fix(app-server): pass -c model="..." to codex app-server so options.model takes effect#408
fix(app-server): pass -c model="..." to codex app-server so options.model takes effect#408hiirott wants to merge 2 commits into
-c model="..." to codex app-server so options.model takes effect#408Conversation
…ns.model takes effect
When `runAppServerTurn` / `runAppServerReview` are invoked with a `model`
option, that value was only delivered via the `thread/start` and
`turn/start` requests. However, the spawn of `codex app-server` itself
did not include a `-c model="..."` override, so the codex CLI session
booted with the CLI's built-in default model. On a ChatGPT account where
the default (`gpt-5.3-codex`) is unsupported, every task launched via
`codex-companion.mjs task --background --model <m>` failed with:
```
Codex error: 'gpt-5.3-codex' model is not supported when using Codex
with a ChatGPT account.
```
even though the user passed `--model gpt-5.4`.
This change threads the model option through to the spawn site:
- `lib/app-server.mjs`: when `options.model` is set, add
`-c model=${JSON.stringify(model)}` to the spawned argv. JSON quoting
yields a valid TOML string literal that the codex CLI's `-c` parser
accepts unambiguously (e.g. `model="gpt-5.4"`).
- `lib/codex.mjs` `withAppServer`: accept a third `clientOptions`
argument and forward it to `CodexAppServerClient.connect` for both
the primary and the direct-retry connection.
- `lib/codex.mjs` `runAppServerReview` / `runAppServerTurn`: pass
`{ model: options.model }` through to `withAppServer`.
`interruptAppServerTurn` and `findLatestTaskThread` still create their
own short-lived clients without a model override; they are not exercised
by the failing `task --background --model` path and are left for a
follow-up.
Verified locally on macOS / codex-cli 0.130.0 with:
```
node companion.mjs task --background --fresh --model gpt-5.4 "say hi"
```
Before this patch the job failed during `turn/start`; after, the job
completes in ~7s with the assistant reply.
![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: 05313567d9
ℹ️ 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".
| let client = null; | ||
| try { | ||
| client = await CodexAppServerClient.connect(cwd); | ||
| client = await CodexAppServerClient.connect(cwd, clientOptions); |
There was a problem hiding this comment.
When no shared session exists, this call enters the default broker path before any direct app-server is spawned. CodexAppServerClient.connect only passes env into ensureBrokerSession, and app-server-broker.mjs starts its inner direct app-server with { disableBroker: true }, so clientOptions.model never becomes the new -c model=... argv; I verified a task --model gpt-5.4 launch records codex app-server with no -c. The ChatGPT-account failure described in the commit therefore still happens for the default shared-runtime path, and only the direct retry path gets the override.
Useful? React with 👍 / 👎.
…kground tasks honor --model The 2026-06-30 fix (0531356) only covered the direct/fallback client path. The broker path used by task --background dropped options.model at ensureBrokerSession, so the broker's long-lived codex app-server child always started on the CLI default model. Thread model through connect -> ensureBrokerSession -> spawnBrokerProcess -> app-server-broker serve --model -> inner connect. RCA: 根因=broker 子プロセス起動時に model が渡っていなかった（6/30 修正は direct 経路のみ） 再発防止=~/.claude/patches/ registry を4ファイル版に更新・README に broker 経路の注意書き Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 133f88f1b3
ℹ️ 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".
| endpoint, | ||
| pidFile, | ||
| logFile, | ||
| model: options.model, |
There was a problem hiding this comment.
When a ready broker session already exists, ensureBrokerSession returns it before reaching this new model spawn argument, so a later task --model gpt-5.4 can still attach to an app-server that was originally started without -c model=.... In the ChatGPT-account scenario described by this fix, that leaves the unsupported default model in effect until the user ends/cleans up the shared session; store/compare the broker's model or bypass/restart the broker when a model override is requested.
Useful? React with 👍 / 👎.
|
Nice catch on the root cause — confirming it from a different angle in case it helps this land. I independently hit the same wall while trying to forward Two things worth folding in / coordinating on:
Happy to test/help if you want to extend this to cover |
2 participants
Summary
runAppServerTurn/runAppServerReviewaccept amodeloption that is forwarded to thethread/startandturn/startrequests. However, the spawn ofcodex app-serveritself (inlib/app-server.mjs) never adds a corresponding-c model=\"…\"config override, so the codex CLI session boots with the CLI's built-in default model.On a ChatGPT account where the default (
gpt-5.3-codex) is unsupported, every task launched via:node scripts/codex-companion.mjs task --background --model gpt-5.4 "<prompt>"fails during the first
turn/startwith:even though the user explicitly passed
--model gpt-5.4.codex exec --model gpt-5.4 "say hi"works fine — the regression is specific to the app-server path used by the companion.Fix
Thread the
modeloption through to the spawn site:lib/app-server.mjs— whenthis.options.modelis set, add-c model=${JSON.stringify(model)}to the spawned argv. JSON quoting yields a valid TOML string literal the codex CLI's-cparser accepts unambiguously (e.g.model=\"gpt-5.4\").lib/codex.mjswithAppServer— accept a thirdclientOptionsargument and forward it toCodexAppServerClient.connectfor both the primary and the direct-retry connection.lib/codex.mjsrunAppServerReview/runAppServerTurn— pass{ model: options.model }through towithAppServer.When the caller does not set
options.model, behavior is unchanged (no-cflag is appended).interruptAppServerTurnandfindLatestTaskThreadstill create their own short-lived clients without a model override; they are not exercised by the failingtask --background --modelpath and are left for a follow-up.Test plan
Local verification on macOS / codex-cli 0.130.0:
node companion.mjs task --background --fresh --model gpt-5.4 \"say hi\"→ job fails with thegpt-5.3-codex is not supportederror duringturn/start.completed, ~7 s, assistant returns "Hi". Job idtask-mqzvgcqz-7o506t, thread019f15cb-b60a-7013-ace1-ed235e61de96.--modelis omitted, no-c model=...flag is appended (verified by inspecting the spawned argv) — behavior unchanged for callers that rely on the CLI default.Notes
CodexAppServerClient.connect(..., { disableBroker: true })path inapp-server-broker.mjs, so this fix is effective for both Spawned and Broker transports.