fix(codex): place -C working-dir flag before exec subcommand (#959)#984
fix(codex): place -C working-dir flag before exec subcommand (#959)#984chr1syy wants to merge 1 commit into
Conversation
…tro#959) Codex CLI parses -C / --cd as a root-level global flag and silently ignores it when it appears after the `exec` subcommand. Both `buildAgentArgs` and the CLI's `spawnJsonLineAgent` were appending workingDirArgs after batchModePrefix, producing `codex exec ... -C <dir> ...` which left Codex running in the wrong cwd — most visible on Windows where Codex also has process-cwd resolution bugs (openai/codex#9671). Add an opt-in `workingDirArgsBeforeBatchPrefix` flag on the agent definition and set it on Codex. Subcommand-scoped working-dir flags (Droid `--cwd`, Gemini `--include-directories`) keep their current append-after-subcommand position. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR fixes issue ChangesCodex working-directory argument ordering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes a silent argument-ordering bug where Codex's
Confidence Score: 4/5Safe to merge — the core ordering fix is correct in both code paths and is well-covered by new tests. The spawnJsonLineAgent CLI path uses a hardcoded toolType === 'codex' guard rather than the generic workingDirArgsBeforeBatchPrefix flag introduced in this PR. Any future agent that sets the flag will work correctly through buildAgentArgs but will have its working-dir args silently misplaced through the CLI path until a developer notices and manually adds another condition. src/cli/services/agent-spawner.ts — the hardcoded toolType check diverges from the generic flag-based approach used everywhere else. Important Files Changed
Reviews (1): Last reviewed commit: "fix(codex): place -C working-dir flag be..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (toolType === 'codex' && def?.workingDirArgs) { | ||
| preOverrideArgs.push(...def.workingDirArgs(cwd)); | ||
| } |
There was a problem hiding this comment.
def?.workingDirArgsBeforeBatchPrefix flag instead of a hardcoded toolType === 'codex' check would keep this path consistent with buildAgentArgs and make the opt-in self-describing — any future agent with the flag set would be handled automatically.
| if (toolType === 'codex' && def?.workingDirArgs) { | |
| preOverrideArgs.push(...def.workingDirArgs(cwd)); | |
| } | |
| if (def?.workingDirArgsBeforeBatchPrefix && def?.workingDirArgs) { | |
| preOverrideArgs.push(...def.workingDirArgs(cwd)); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/services/agent-spawner.ts`:
- Around line 577-586: The hardcoded check for toolType === 'codex' should be
replaced by the definition flag; update the conditional in agent-spawner.ts to
use def?.workingDirArgsBeforeBatchPrefix (and def?.workingDirArgs) instead of
toolType === 'codex' so that when a definition opts into
workingDirArgsBeforeBatchPrefix you call
preOverrideArgs.push(...def.workingDirArgs(cwd)); keep the existing push
behavior and null-safe checks (use def?.workingDirArgs) and do not change
ordering logic beyond switching the guard to
def.workingDirArgsBeforeBatchPrefix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f54f8187-3d2d-4bef-acf5-0e53a1f2044a
📒 Files selected for processing (6)
src/__tests__/cli/services/agent-spawner.test.tssrc/__tests__/main/agents/definitions.test.tssrc/__tests__/main/utils/agent-args.test.tssrc/cli/services/agent-spawner.tssrc/main/agents/definitions.tssrc/main/utils/agent-args.ts
|
|
||
| // Codex's `-C` / `--cd` is a root-level global flag and is silently ignored | ||
| // when it appears after the `exec` subcommand (issue #959 / | ||
| // https://github.com/openai/codex/issues/9671). Prepend the working-dir | ||
| // args BEFORE batchModePrefix so the final ordering is | ||
| // `codex -C <dir> exec ...` instead of `codex exec ... -C <dir>`. | ||
| if (toolType === 'codex' && def?.workingDirArgs) { | ||
| preOverrideArgs.push(...def.workingDirArgs(cwd)); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use workingDirArgsBeforeBatchPrefix flag instead of hardcoding agent type.
The hardcoded check toolType === 'codex' defeats the purpose of introducing the generic workingDirArgsBeforeBatchPrefix configuration flag in definitions.ts. If another agent needs the same behavior in the future, this code would require modification.
♻️ Refactor to use the flag
- // Codex's `-C` / `--cd` is a root-level global flag and is silently ignored
- // when it appears after the `exec` subcommand (issue `#959` /
- // https://github.com/openai/codex/issues/9671). Prepend the working-dir
- // args BEFORE batchModePrefix so the final ordering is
- // `codex -C <dir> exec ...` instead of `codex exec ... -C <dir>`.
- if (toolType === 'codex' && def?.workingDirArgs) {
+ // Some agents (e.g., Codex) require workingDirArgs to precede the batch-mode
+ // subcommand because the flag is parsed as a root-level global. Prepend here
+ // when workingDirArgsBeforeBatchPrefix is set (see definitions.ts and `#959`).
+ if (def?.workingDirArgs && def?.workingDirArgsBeforeBatchPrefix) {
preOverrideArgs.push(...def.workingDirArgs(cwd));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Codex's `-C` / `--cd` is a root-level global flag and is silently ignored | |
| // when it appears after the `exec` subcommand (issue #959 / | |
| // https://github.com/openai/codex/issues/9671). Prepend the working-dir | |
| // args BEFORE batchModePrefix so the final ordering is | |
| // `codex -C <dir> exec ...` instead of `codex exec ... -C <dir>`. | |
| if (toolType === 'codex' && def?.workingDirArgs) { | |
| preOverrideArgs.push(...def.workingDirArgs(cwd)); | |
| } | |
| // Some agents (e.g., Codex) require workingDirArgs to precede the batch-mode | |
| // subcommand because the flag is parsed as a root-level global. Prepend here | |
| // when workingDirArgsBeforeBatchPrefix is set (see definitions.ts and `#959`). | |
| if (def?.workingDirArgs && def?.workingDirArgsBeforeBatchPrefix) { | |
| preOverrideArgs.push(...def.workingDirArgs(cwd)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/services/agent-spawner.ts` around lines 577 - 586, The hardcoded
check for toolType === 'codex' should be replaced by the definition flag; update
the conditional in agent-spawner.ts to use def?.workingDirArgsBeforeBatchPrefix
(and def?.workingDirArgs) instead of toolType === 'codex' so that when a
definition opts into workingDirArgsBeforeBatchPrefix you call
preOverrideArgs.push(...def.workingDirArgs(cwd)); keep the existing push
behavior and null-safe checks (use def?.workingDirArgs) and do not change
ordering logic beyond switching the guard to
def.workingDirArgsBeforeBatchPrefix.
1 participant
Summary
Fixes #959. The Codex CLI parses
-C/--cdas a root-level global flag and silently ignores it when it appears after theexecsubcommand. BothbuildAgentArgs(desktop / Auto Run / Cue) and the CLI'sspawnJsonLineAgent(maestro-cli send / playbooks) were appendingworkingDirArgsafterbatchModePrefix, producing:Codex silently ignored the
-Cand fell back to its own cwd resolution — most visible on Windows where Codex also has process-cwd resolution bugs (openai/codex#9671).After:
Implementation
workingDirArgsBeforeBatchPrefix?: booleanon the agent definition. Codex sets it totrue.buildAgentArgsnow prependsworkingDirArgsbeforebatchModePrefixwhen this flag is set; otherwise it preserves the current append-after-prefix behavior.spawnJsonLineAgentdoes the same for Codex — preserves the existing Codex-only scoping so Droid's CLI path is unchanged (Droid's--cwdis a subcommand-scoped flag and was never applied by the CLI path).--cwd) and Gemini (--include-directories) remain unaffected.Tests
agent-args.test.ts: new tests verifying (1) Codex ordering with-Cbeforeexec, (2) Droid ordering preserves--cwdafterexec, (3) no duplication when flag is set.definitions.test.ts: asserts Codex hasworkingDirArgsBeforeBatchPrefix: trueand Droid does not.agent-spawner.test.ts: extended the existing Codex spawn test to assert-Cprecedesexec.Test plan
npx vitest run src/__tests__/main/utils/agent-args.test.ts src/__tests__/main/agents/definitions.test.ts— 101 passingnpx vitest run src/__tests__/cli/services/agent-spawner.test.ts— 113 passingnpx vitest run src/__tests__/main/ipc/handlers/agents.test.ts src/__tests__/integration/provider-integration.test.ts src/__tests__/integration/group-chat-integration.test.ts— 73 passing🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests