Skip to content

Commit 38cfa89

Browse files
rafaelscostaclaude
andauthored
fix(quality-gates): cross-platform CodeRabbit CLI execution [#731] (#752)
* fix(quality-gates): cross-platform CodeRabbit CLI execution [#731] Closes #731 (reported by @duboneh). The CodeRabbit CLI execution paths were hardcoded to assume Windows + WSL (`wsl bash -c '...'` wrapper, `/mnt/c/...` paths, `installation_mode: wsl`). On macOS/Linux those commands failed silently and the Layer-2 quality-gate automation never enforced the review. The auto-healing loop in @dev / @qa / @devops / @architect / @data-engineer simulated success while doing nothing. ## What changed Three patterns applied across runtime + config + agent surfaces: ### Pattern 1 — Inline cross-platform branching (runtime) - `.aiox-core/core/orchestration/workflow-executor.js` (`runCodeRabbitAnalysis`) - `.aiox-core/core/quality-gates/layer2-pr-automation.js` (`runCodeRabbit`) - `.aiox-core/development/tasks/github-devops-pre-push-quality-gate.md` (`runCodeRabbit` inline code block + error-handler messages) Default behavior: - `installation_mode || (process.platform === 'win32' ? 'wsl' : 'native')`. - `cli_path` defaults to `~/.local/bin/coderabbit`. - Explicit `installation_mode: 'wsl' | 'native'` still wins (lets ops force WSL on macOS for testing, etc). - Explicit raw `command:` string still wins over `cli_path` (back-compat). Error-handler messages now switch on `process.platform`: macOS/Linux users see native recovery commands, Windows users see WSL-wrapped ones. ### Pattern 2 — `wsl_config:` block → `cli_path:` + `platform_notes:` DELETED `installation_mode: wsl` + `wsl_config:` blocks (no overlay; full removal) from: - `.aiox-core/core/quality-gates/quality-gate-config.yaml` - `.aiox-core/development/agents/dev.md` - `.aiox-core/development/agents/qa.md` - `.aiox-core/development/agents/devops.md` Replaced with: ```yaml cli_path: ~/.local/bin/coderabbit platform_notes: macos_linux: "Run cli_path directly from project root (no wrapper)." windows: "Wrap with 'wsl bash -c' and rewrite project paths to /mnt/<drive>/..." ``` ### Pattern 3 — execution_guidelines rewritten neutral Replaced "CRITICAL: CodeRabbit CLI is installed in WSL, not Windows" + the "use 'wsl bash -c' wrapper for ALL commands" prescriptions with cross-platform guidance in 5 agent personas (dev, qa, devops, architect, data-engineer). `commands:` blocks now have explicit `_native` and `_wsl` template suffixes so agents can see both shapes side-by-side. ASCII workflow diagrams updated in: - `.aiox-core/development/tasks/qa-review-story.md` - `.aiox-core/development/tasks/dev-develop-story.md` `environment-bootstrap.md` note also rewritten as cross-platform. ### Regression coverage `tests/unit/quality-gates/cross-platform-coderabbit.test.js` — 8 tests: - macOS (darwin) builds native command - Linux builds native command - Windows wraps with `wsl bash -c` - Explicit `installation_mode: 'native'` override on Windows - Explicit `installation_mode: 'wsl'` override on macOS - Raw `command:` string override (back-compat) - Custom `cli_path` honored on macOS - Custom `cli_path` honored on Windows Test suite: 8/8 pass. ### IDE projection sync `npm run sync:ide` propagated the agent persona changes to: - `.claude/skills/AIOX/agents/{...}/SKILL.md` - `.codex/agents/*.md` - `.gemini/rules/AIOX/agents/*.md` - `.kimi/skills/aiox-{...}/SKILL.md` 109 files synced across 7 IDEs (claude-code, codex, gemini, github-copilot, cursor, antigravity, kimi). Compatibility Parity Gate + IDE Command Sync Validation should be green on this branch. ## Validation - [x] `npm run lint` exit 0 - [x] `npx jest tests/unit/quality-gates/cross-platform-coderabbit.test.js` → 8/8 pass - [x] `grep -rn 'installation_mode: wsl\b\|wsl_config:' .aiox-core/development/agents/*.md` → zero matches - [x] All remaining `wsl bash -c` references are Windows-prefixed examples or `_wsl` template suffixes (verified manually) - [x] `npm run sync:ide` clean — no projection drift ## Out of scope (separate follow-ups) - The `Cross-Platform (${{ matrix.os }}, Node ${{ matrix.node }})` smoke matrix check in CI is `skipping` (unexpanded YAML variable). Unrelated bug. - `qa/MEMORY.md` mentions `installation_mode: wsl` but is a runtime memory file, not config — intentionally left alone (per @advisor guidance). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(ids): regenerate entity-registry after [#731] agent persona changes Pre-push hook regenerated the registry to reflect the cleanup in agent personas (removed `installation_mode: wsl` / `wsl_config:` blocks, reworked usedBy/dependencies for the coderabbit_integration sections). entityCount 815 → 816. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(quality-gates): address CodeRabbit feedback on PR #752 [#731] Two real bugs CodeRabbit caught + one new regression test: ## Bug 1: ${PROJECT_ROOT} placeholder never expanded `layer2-pr-automation.js` left `cd ${PROJECT_ROOT}` as a literal placeholder in the WSL command string. `child_process.spawn` with `shell: true` only expands env vars that ARE set — PROJECT_ROOT is never set at call sites, so the cd target was empty and the command silently `cd`'d into the home directory, then ran coderabbit there instead of in the project root. Fix: substitute `process.cwd()` (or explicit `coderabbit.projectRoot`) in-place and convert to `/mnt/<drive>/<path>` for the WSL host. The resulting command now contains a real absolute path, matching what `workflow-executor.js` already did via `this.projectRoot`. ## Bug 2: Tilde expansion not reliable in spawn contexts Both `workflow-executor.js` (uses `child_process.exec`) and `layer2-pr-automation.js` (uses `spawn { shell: true }`) shipped `cli_path` literally as `~/.local/bin/coderabbit`. Shell expansion works when the call goes through a real shell, but the `workflow-executor` path goes through `execAsync` which is `/bin/sh -c` on POSIX (so OK) but is host-dependent on Windows (cmd.exe does NOT expand `~`). Programmatic expansion via `os.homedir() + path.join` removes the ambiguity. Applied to: - `.aiox-core/core/orchestration/workflow-executor.js` - `.aiox-core/core/quality-gates/layer2-pr-automation.js` - `.aiox-core/development/tasks/github-devops-pre-push-quality-gate.md` (kept in sync — same pattern in the doc's inline JS block) ## Regression test added `cross-platform-coderabbit.test.js`: - "converts Windows project root to /mnt/<drive>/... inside wsl command" - "expands tilde (~) in cli_path via os.homedir() — defensive" Full suite: 10/10 pass (was 8 — +2 new). ## CodeRabbit feedback not addressed (with reason) - **Helper extraction**: deferred. Two call sites + 16 lines duplicated is cheaper to maintain than a new module + import paths + test surface. Per advisor guidance — premature abstraction. - **WSL availability check on Windows**: deferred to a separate issue. If `wsl` is not installed on Windows, the user already sees a clear `'wsl' is not recognized` error from cmd. Adding a `spawnSync('wsl', ['-l'])` probe before every CodeRabbit invocation costs latency on every call and replaces one error message with another. Not a regression introduced by this PR. - **Entity-registry dependency cleanup**: the `dependencies: []` zeroed on agent personas is a CORRECT consequence of removing `wsl_config:` + `installation_mode: wsl` blocks. Those blocks named phantom dependencies (Ubuntu distribution, /mnt paths) that the registry interpreted as edges. Removing the source removed the edges. The registry is consistent with the post-fix state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(quality-gates): CodeRabbit round 2 — WSL tilde + lowercase drive [#731] Two more real bugs CodeRabbit caught on the round-1 fix: ## Bug 3: Pre-expanded tilde poisons the WSL command Round-1 expanded `~` via `os.homedir()` unconditionally — but on Windows that yields `C:\Users\<user>` (a Windows path). Injecting that into a `wsl bash -c '...'` command means the cd path inside WSL points at a Windows path the WSL distribution cannot resolve. The previous round moved the bug, didn't fix it. Fix: split the cli_path handling by mode. - **native mode** (macOS/Linux): expand `~` via `os.homedir() + path.join(...)`. The resolved absolute path is shell-agnostic. - **WSL mode** (Windows or explicit override): keep the literal `~`. The WSL distribution's own bash expands it to the WSL user's HOME (`/home/<wsl-user>`) — the only correct expansion in that context. ## Bug 4: Drive-letter regex only matched uppercase `/^([A-Z]):/` rejects `c:\path` (lowercase drive letter from `process.cwd()` on some configurations / npm test environments). Changed to `/^([A-Za-z]):/`. Replacement still lowercases the captured drive (per WSL convention `/mnt/c/...`). Applied to all three call sites. ## Regression coverage `cross-platform-coderabbit.test.js` — 2 new tests + 1 reshape: - "handles lowercase drive letters when converting to /mnt/<drive>/..." - "keeps tilde (~) literal in cli_path in WSL mode — bash expands it inside WSL" - "wraps the command... (win32)" now asserts the host's homedir-prefixed cli_path does NOT appear in the WSL command (more precise than the previous "no homedir" check which was incidentally tripped by cwd). Full suite: 12/12 pass (was 10 — +2 new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(manifest): regenerate install-manifest hash for entity-registry [#731] CI's Install Manifest Validation step failed because the IDS-Hook regenerated `entity-registry.yaml` in the pre-push (after the round-2 commit) but the matching hash in `install-manifest.yaml` was not updated. `npm run generate:manifest` corrects the drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 536bda9 commit 38cfa89

35 files changed

Lines changed: 1058 additions & 828 deletions

File tree

.aiox-core/core/orchestration/workflow-executor.js

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
const fs = require('fs').promises;
2222
const fsSync = require('fs');
23+
const os = require('os');
2324
const path = require('path');
2425
const yaml = require('js-yaml');
2526

@@ -778,13 +779,30 @@ class WorkflowExecutor {
778779
const { promisify } = require('util');
779780
const execAsync = promisify(exec);
780781

781-
// Build command based on installation mode
782+
// Build command for current platform.
783+
// - Explicit installation_mode: 'wsl' | 'native' wins (lets ops override).
784+
// - Default: Windows hosts wrap via WSL, macOS/Linux run the binary directly.
785+
// - cli_path defaults to ~/.local/bin/coderabbit (matches the CodeRabbit CLI installer default).
786+
// - Tilde handling differs per mode: native expands via os.homedir() so the
787+
// resolved absolute path is shell-agnostic; WSL mode keeps the literal `~`
788+
// so the WSL distribution's own bash expands it (the host's HOME would point
789+
// at a Windows path that WSL cannot resolve).
790+
const rawCliPath = coderabbitConfig.cli_path || '~/.local/bin/coderabbit';
791+
const mode =
792+
coderabbitConfig.installation_mode ||
793+
(process.platform === 'win32' ? 'wsl' : 'native');
782794
let command;
783-
if (coderabbitConfig.installation_mode === 'wsl') {
784-
const wslPath = this.projectRoot.replace(/^([A-Z]):/, (_, drive) => `/mnt/${drive.toLowerCase()}`).replace(/\\/g, '/');
785-
command = `wsl bash -c 'cd "${wslPath}" && ~/.local/bin/coderabbit --prompt-only -t uncommitted 2>&1'`;
795+
if (mode === 'wsl') {
796+
const wslPath = this.projectRoot
797+
.replace(/^([A-Za-z]):/, (_, drive) => `/mnt/${drive.toLowerCase()}`)
798+
.replace(/\\/g, '/');
799+
// Keep literal `~` — WSL bash expands it to the WSL user's HOME.
800+
command = `wsl bash -c 'cd "${wslPath}" && ${rawCliPath} --prompt-only -t uncommitted 2>&1'`;
786801
} else {
787-
command = 'coderabbit --prompt-only -t uncommitted';
802+
const cliPath = rawCliPath.startsWith('~')
803+
? path.join(os.homedir(), rawCliPath.slice(1))
804+
: rawCliPath;
805+
command = `${cliPath} --prompt-only -t uncommitted`;
788806
}
789807

790808
if (this.options.debug) {

.aiox-core/core/quality-gates/layer2-pr-automation.js

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
const { spawn } = require('child_process');
1414
const fs = require('fs').promises;
15+
const os = require('os');
1516
const path = require('path');
1617
const { BaseLayer } = require('./base-layer');
1718

@@ -96,10 +97,39 @@ class Layer2PRAutomation extends BaseLayer {
9697
}
9798

9899
try {
99-
// Check if CodeRabbit is available
100-
const command =
101-
this.coderabbit.command ||
102-
"wsl bash -c 'cd ${PROJECT_ROOT} && ~/.local/bin/coderabbit --prompt-only -t uncommitted'";
100+
// Build command for current platform when no explicit override is present.
101+
// - this.coderabbit.command: explicit string wins (backward compat with old configs).
102+
// - this.coderabbit.installation_mode: 'wsl' | 'native' lets ops override platform detection.
103+
// - Default: Windows hosts wrap via WSL, macOS/Linux run the binary directly.
104+
// - ${PROJECT_ROOT} is resolved programmatically — `child_process.spawn` with
105+
// `shell: true` does shell expansion, but we cannot rely on PROJECT_ROOT
106+
// being set in the env at call sites, so substitute it here.
107+
// - Tilde handling differs per mode: native expands via os.homedir() so the
108+
// resolved absolute path is shell-agnostic; WSL mode keeps the literal `~`
109+
// so the WSL distribution's own bash expands it (the host's HOME would point
110+
// at a Windows path that WSL cannot resolve).
111+
let command;
112+
if (this.coderabbit.command) {
113+
command = this.coderabbit.command;
114+
} else {
115+
const rawCliPath = this.coderabbit.cli_path || '~/.local/bin/coderabbit';
116+
const mode =
117+
this.coderabbit.installation_mode ||
118+
(process.platform === 'win32' ? 'wsl' : 'native');
119+
if (mode === 'wsl') {
120+
const projectRoot = this.coderabbit.projectRoot || process.cwd();
121+
const wslProjectPath = projectRoot
122+
.replace(/\\/g, '/')
123+
.replace(/^([A-Za-z]):/, (_, drive) => `/mnt/${drive.toLowerCase()}`);
124+
// Keep literal `~` — WSL bash expands it to the WSL user's HOME.
125+
command = `wsl bash -c 'cd "${wslProjectPath}" && ${rawCliPath} --prompt-only -t uncommitted'`;
126+
} else {
127+
const cliPath = rawCliPath.startsWith('~')
128+
? path.join(os.homedir(), rawCliPath.slice(1))
129+
: rawCliPath;
130+
command = `${cliPath} --prompt-only -t uncommitted`;
131+
}
132+
}
103133

104134
const result = await this.runCommand(command, timeout);
105135

.aiox-core/core/quality-gates/quality-gate-config.yaml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,16 @@ layer2:
3434
enabled: true
3535
coderabbit:
3636
enabled: true
37-
command: "wsl bash -c 'cd ${PROJECT_ROOT} && ~/.local/bin/coderabbit --prompt-only -t uncommitted'"
37+
# Cross-platform CodeRabbit CLI invocation (Issue #731).
38+
# Runtime resolves the command from cli_path + platform detection:
39+
# - macOS/Linux: run cli_path directly from project root.
40+
# - Windows: wrap with 'wsl bash -c' and rewrite project paths to /mnt/<drive>/...
41+
# Set installation_mode explicitly ('wsl' | 'native') to override platform detection.
42+
# Set command: "<raw shell string>" to bypass detection entirely (back-compat).
43+
cli_path: ~/.local/bin/coderabbit
44+
platform_notes:
45+
macos_linux: "Run binary directly from project root (PATH or cli_path)."
46+
windows: "Wrap with 'wsl bash -c' and use /mnt/<drive>/... project paths."
3847
timeout: 900000 # 15 minutes
3948
blockOn:
4049
- CRITICAL

0 commit comments

Comments
 (0)