feat: forward CODEX_PLUGIN_CC_ARGS to codex launches#419
Conversation
Add a CODEX_PLUGIN_CC_ARGS environment variable. When set, its value is tokenized (shell-style) and prepended to every codex invocation the plugin makes: the app-server runtime spawn and the availability checks. This lets users force codex global overrides (e.g. a custom provider via `-c model_provider=...` or `--profile`) without editing config.toml, and without a per-command flag. Previously only --model/--effort were forwarded and the app-server was always spawned as `codex app-server`. - args.mjs: getCodexPassthroughArgs(env) + CODEX_PLUGIN_ARGS_ENV - app-server.mjs: prepend args to the `codex app-server` spawn - codex.mjs: prepend args to `--version` / `app-server --help` checks - fake codex fixture: tolerate leading global flags, record boot argv - tests: unit tests for the parser + e2e passthrough assertion - README: document the variable Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da5b881948
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (typeof raw !== "string" || !raw.trim()) { | ||
| return []; | ||
| } | ||
| return splitRawArgumentString(raw); |
There was a problem hiding this comment.
When users put a global argument whose value contains literal backslashes in CODEX_PLUGIN_CC_ARGS—for example the documented --add-dir path flag with a Windows path like --add-dir 'C:\work\repo'—this new env path calls splitRawArgumentString(), which treats \ as an escape even inside single quotes; the argv becomes ["--add-dir","C:workrepo"] before spawning Codex. That launches Codex with the wrong directory/config value, so the passthrough should use a parser that preserves quoted backslashes or otherwise handle Windows paths explicitly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in f6f1ab7. splitRawArgumentString now handles single-quote context before the escape check, so single-quoted content is literal (POSIX semantics) and --add-dir 'C:\work\repo' survives intact. Double-quote escaping is unchanged. Added unit tests covering single-quote backslashes, double-quote escaping, and unquoted escapes.
splitRawArgumentString treated `\` as an escape even inside single quotes, so CODEX_PLUGIN_CC_ARGS values like `--add-dir 'C:\work\repo'` were mangled into `C:workrepo`. Handle single-quote context before the escape check so single-quoted content is literal (POSIX semantics); double-quote escaping is unchanged. Addresses the Codex review comment on openai#419. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6f1ab7714
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pushValue(args, "--scope", input.scope); | ||
| pushValue(args, "--model", input.model); | ||
| pushValue(args, "--cwd", input.cwd); | ||
| args.push("--background", "--json"); |
There was a problem hiding this comment.
Here codex_review maps MCP tool calls to review --background --json, but that flag does not enqueue a review job in the companion: handleReviewCommand parses --background and then unconditionally awaits runForegroundCommand (checked in plugins/codex/scripts/codex-companion.mjs). The existing slash-command flow detaches reviews through Claude's Bash run_in_background; an MCP tool call has no such wrapper, so longer reviews block the MCP request and return the final review payload instead of the advertised job id/status. Please either implement a real background review path for this invocation or make the MCP review tool foreground/timeout-aware.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks — but this file was included in the PR by mistake. plugins/codex/mcp/** and tests/mcp-server.test.mjs are unrelated to this change (which only adds the CODEX_PLUGIN_CC_ARGS passthrough); they got swept into a git add -A for a follow-up commit. I've rewritten that commit and force-pushed, so these files are no longer part of the PR. This MCP background-review concern is therefore out of scope here — happy to open a separate issue for it if it's a real problem on main.
| escaping = true; | ||
| continue; | ||
| } | ||
|
|
||
| if (quote) { | ||
| if (quote === "\"") { |
There was a problem hiding this comment.
Fresh evidence after the single-quote fix: in the current code, double-quoted values still hit the generic backslash escape before quote === "\"" is handled. With CODEX_PLUGIN_CC_ARGS='--add-dir "C:\\work\\repo"', splitRawArgumentString() returns ['--add-dir','C:workrepo'], even though the README promises shell-like parsing and POSIX double quotes keep backslashes before ordinary characters. This launches Codex with the wrong path/config whenever users choose double quotes around a Windows path; handle double-quote escaping selectively or preserve \ before non-special chars.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f75ad1. splitRawArgumentString now applies POSIX double-quote semantics: a backslash only escapes " \ $ and stays literal before any other character, so--add-dir "C:\work\repo" is preserved ("a"b"still yieldsa"b`). Both single- and double-quote backslash cases now have unit tests. Note the fix commit was rewritten (force-push) to drop some files that were included by mistake — see the reply on the other thread.
splitRawArgumentString treated `\` as an escape regardless of quote context, so CODEX_PLUGIN_CC_ARGS values with backslashes were mangled — e.g. `--add-dir 'C:\work\repo'` and `--add-dir "C:\work\repo"` both became `C:workrepo`. Apply POSIX quoting semantics: - single quotes: everything literal, including backslashes - double quotes: backslash only escapes " \ $ ` ; before any other char it stays literal (so `"a\"b"` still yields `a"b`) Adds unit tests for single/double-quote backslash handling. Addresses the Codex review comments on openai#419. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
klmklmnb
force-pushed
the
feat/codex-plugin-cc-args
branch
from
f6f1ab7 to
2f75ad1
Compare
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
What
Adds a
CODEX_PLUGIN_CC_ARGSenvironment variable. When set, its value is tokenized (shell-style, reusing the existingsplitRawArgumentString) and prepended to everycodexinvocation the plugin makes:codex app-serverruntime spawn (app-server.mjs), which covers both direct and broker modescodex --versionandcodex app-server --help(codex.mjs)Example:
Why
Closes #418. Previously only
--model/--effortwere forwarded and the app-server was always spawned bare, so there was no way to pass Codex global overrides (custommodel_provider,--profile, arbitrary-c key=value) without editingconfig.toml. This makes it opt-in via a single env var, reusing Codex's own config-override surface.Changes
scripts/lib/args.mjs: newgetCodexPassthroughArgs(env)+CODEX_PLUGIN_ARGS_ENVconstantscripts/lib/app-server.mjs: prepend args to thecodex app-serverspawn (reads the same env passed to the child)scripts/lib/codex.mjs: prepend args to the availability checkstests/fake-codex-fixture.mjs: tolerate leading global flags before the subcommand; record boot argvtests/args.test.mjs(new): unit tests for the parsertests/runtime.test.mjs: e2e test asserting the args reach the codex processREADME.md: documents the variable, quoting, and the shared-runtime caveatTesting
npm test— 95/95 passing locally.Notes
-c/--profile.