Add Codex launch arguments setting#2892
Conversation
📝 WalkthroughWalkthroughThis PR adds support for configurable launch arguments to the Codex provider. The ChangesCodex Launch Arguments
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
ApprovabilityVerdict: Needs human review This PR introduces a new user-facing configuration capability that modifies how the Codex subprocess is spawned by allowing arbitrary CLI arguments. Changes that affect subprocess execution warrant human review to verify security and behavioral implications. You can customize Macroscope's approvability policy. Learn more. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/contracts/src/settings.ts (1)
209-213: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd
"launchArgs"to theorderarray for consistency.The
orderarray should include"launchArgs"to ensure the field appears in the intended position in the settings form.ClaudeSettings(line 255) includeslaunchArgsin its order array, and the test expectation inProviderSettingsForm.test.ts(line 21) assumeslaunchArgsappears aftershadowHomePath.♻️ Proposed fix
}, { - order: ["binaryPath", "homePath", "shadowHomePath"], + order: ["binaryPath", "homePath", "shadowHomePath", "launchArgs"], }, );🤖 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 `@packages/contracts/src/settings.ts` around lines 209 - 213, The settings schema object currently sets order: ["binaryPath", "homePath", "shadowHomePath"] but omits "launchArgs", causing form ordering and tests to fail; update that object's order array to include "launchArgs" after "shadowHomePath" so it matches ClaudeSettings and the expectation in ProviderSettingsForm.test.ts, ensuring the "order" array change is applied to the same settings object in packages/contracts/src/settings.ts.
🤖 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 `@apps/server/src/provider/Layers/CodexSessionRuntime.ts`:
- Around line 721-724: The CodexAppServerSpawnError message currently logs a
hardcoded "${options.binaryPath} app-server" which can mismatch the actual
command; update the error construction where CodexAppServerSpawnError is thrown
(after the spawner.spawn call) to include the computed appServerArgs (from
codexAppServerArgs(options.launchArgs)) along with options.binaryPath so the
logged command reflects ChildProcess.make(options.binaryPath, appServerArgs,
...) and any flags from options.launchArgs; ensure you reference appServerArgs
and options.binaryPath in the error message so spawn failures show the real
command invoked.
---
Outside diff comments:
In `@packages/contracts/src/settings.ts`:
- Around line 209-213: The settings schema object currently sets order:
["binaryPath", "homePath", "shadowHomePath"] but omits "launchArgs", causing
form ordering and tests to fail; update that object's order array to include
"launchArgs" after "shadowHomePath" so it matches ClaudeSettings and the
expectation in ProviderSettingsForm.test.ts, ensuring the "order" array change
is applied to the same settings object in packages/contracts/src/settings.ts.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b83f8476-9f14-45be-9c09-5bab8a90c3d4
📒 Files selected for processing (12)
apps/server/src/provider/Layers/CodexAdapter.test.tsapps/server/src/provider/Layers/CodexAdapter.tsapps/server/src/provider/Layers/CodexProvider.tsapps/server/src/provider/Layers/CodexSessionRuntime.tsapps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.tsapps/server/src/provider/Layers/ProviderRegistry.test.tsapps/server/src/provider/Layers/codexLaunchArgs.test.tsapps/server/src/serverSettings.test.tsapps/web/src/components/KeybindingsToast.browser.tsxapps/web/src/components/settings/ProviderSettingsForm.test.tspackages/contracts/src/settings.test.tspackages/contracts/src/settings.ts
| const appServerArgs = codexAppServerArgs(options.launchArgs); | ||
| const child = yield* spawner | ||
| .spawn( | ||
| ChildProcess.make(options.binaryPath, ["app-server"], { | ||
| ChildProcess.make(options.binaryPath, appServerArgs, { |
There was a problem hiding this comment.
Error message doesn't reflect configured launch arguments.
The CodexAppServerSpawnError message on line 736 shows a fixed "${options.binaryPath} app-server" command, but the actual spawn uses appServerArgs which may contain additional flags from options.launchArgs. When debugging spawn failures with custom launch args, the error message won't match the actual command.
📝 Suggested fix to include computed args in error message
- new CodexErrors.CodexAppServerSpawnError({
- command: `${options.binaryPath} app-server`,
- cause,
- }),
+ new CodexErrors.CodexAppServerSpawnError({
+ command: `${options.binaryPath} ${appServerArgs.join(" ")}`,
+ cause,
+ }),🤖 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 `@apps/server/src/provider/Layers/CodexSessionRuntime.ts` around lines 721 -
724, The CodexAppServerSpawnError message currently logs a hardcoded
"${options.binaryPath} app-server" which can mismatch the actual command; update
the error construction where CodexAppServerSpawnError is thrown (after the
spawner.spawn call) to include the computed appServerArgs (from
codexAppServerArgs(options.launchArgs)) along with options.binaryPath so the
logged command reflects ChildProcess.make(options.binaryPath, appServerArgs,
...) and any flags from options.launchArgs; ensure you reference appServerArgs
and options.binaryPath in the error message so spawn failures show the real
command invoked.
What Changed
launchArgssetting to the shared settings schema and provider settings form.codex app-serverfor provider probes and live session startup.Why
Codex instances need the same kind of configurable launch arguments Claude already exposes, so users can pass app-server CLI flags like
-c model_provider="codex-lb"without changing the binary path. The field is split into argv tokens directly and never shell-executed.UI Changes
Adds a small
Launch argumentstext field to Codex provider settings.Screenshots will be added separately.
Checklist
Validation:
bun fmtbun lintbun typecheckNote
Medium Risk
Changes how the Codex child process is spawned for every session and health probe; misconfigured flags could break connectivity, but there is no new auth or data-path logic.
Overview
Adds a Codex
launchArgssetting (schema, settings patch, and provider form field) so users can pass extra flags tocodex app-server, mirroring Claude’s launch-arguments UX.Configured values are trimmed, split on whitespace into argv tokens via
codexAppServerArgs(always prefixed withapp-server), and forwarded on provider status probes, live session spawn (CodexSessionRuntime), and adapter → runtime wiring. Tests cover argv building, probe/adapter propagation, and settings trimming/visibility.Reviewed by Cursor Bugbot for commit 0d0a277. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add
launchArgssetting to Codex provider for passing custom CLI argumentslaunchArgsstring field toCodexSettingsandCodexSettingsPatchin settings.ts, trimmed on decode with an empty string default.launchArgsthrough the provider stack:checkCodexProviderStatus→probeCodexAppServerProvider→makeCodexSessionRuntime, which appends the whitespace-split args to theapp-serverCLI invocation.codexAppServerArgshelper in CodexProvider.ts and CodexSessionRuntime.ts to build the final arg list consistently.launchArgsfield.Macroscope summarized 0d0a277.
Summary by CodeRabbit