Skip to content

fix(native-chat): apply or refuse saved Codex Arguments instead of silently dropping them - #26059

Closed
brennanb2025 wants to merge 11 commits into
mainfrom
brennanb2025/native-chat-codex-arguments
Closed

brennanb2025 wants to merge 11 commits into
mainfrom
brennanb2025/native-chat-codex-arguments

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

Settings → Agents → Arguments is shared by terminal tabs and native chat. In Codex native chat, many saved Arguments were quietly lost. Some were dropped; others were placed where Codex reads them and then ignores them. A chat could run a different provider, model or config than the terminal, and nothing said so. Now every saved Codex Argument either works in chat the way it does in a terminal tab, is a permission setting (chat takes permissions from Agent Permissions only, see below), or the chat refuses to start and says which argument to change and what to use instead.

What Changed

Before (Codex native chat):

  • --oss --local-provider ollama: the terminal used the local model; chat silently sent prompts to the default cloud provider.
  • -m gpt-5 and --search were accepted but ignored in chat.
  • --profile work was dropped, so chat ran the base config, which can mean a different model, provider or MCP servers.
  • --strict-config, --add-dir, -C, -i, and anything after -- were dropped.
  • The model picker's list was fetched without the saved Arguments, so the picker and the chat could disagree.
  • An unclosed quote in Arguments showed "Codex couldn't start. Send your message to try again." with no hint that Arguments were the cause.

After:

  • Passed to Codex as typed: -c key=value, --enable, --disable, --strict-config.
  • Converted to the config Codex itself derives from them: -m X → -c model="X"; --search → -c web_search="live".
  • Permissions come only from the chat's permission setting, not from launch arguments. Today that setting is Agent Permissions; after feat(native-chat): each Claude/Codex chat gets its own permission mode #26116 it is the chat's own mode and Settings → Chat → "Permissions for new chats". This follows the common pattern: a chat's mode comes from the app's setting plus the per-chat picker, and launch arguments never set it. These are dropped quietly in chat: -s/--sandbox, -a/--ask-for-approval, --approve-for-me, and -c keys for approval_policy, sandbox_mode, approvals_reviewer and sandbox_workspace_write. This is what main already did for most of them. Known cost: a user with -s read-only saved for terminal tabs does not get read-only in chat. Chat runs whatever the chat permission setting (or the chat's picker) says.
  • Refused by name, before anything starts: the chat's error card names the argument and says what to do. For example: "Codex couldn't start. Codex chats can't use --profile from saved Arguments. Remove it from Codex's Arguments in Settings → Agents. To keep the profile's settings, put them in config.toml or pass them with -c." The hints by option:
    • --oss / --local-provider: choose the provider with -c model_provider=… and a local model with -m.
    • -C / --cd / --add-dir / --worktree: a chat already works in its workspace folder.
    • -i: attach the image in the chat instead.
    • --remote, --dangerously-bypass-hook-trust, unknown options and prompts: a plain "remove it".
    • An unclosed quote: "Saved Arguments have a quote that isn't closed." This applies to Claude too, whenever the host starts or restarts a chat: an existing chat after Arguments change, or a chat started from the phone or the CLI. A NEW chat opened from the desktop tab bar is stopped earlier, by the shared terminal-launch planning. That planning tokenizes Arguments for the terminal command and shows the existing toast "Could not build launch command for Codex." This was already the case on main, and terminal tabs behave the same way. Making that toast name Arguments is a follow-up in the shared launch path.
    • Every message points to "Settings → Agents", matching the Command refusal from fix(native-chat): refuse a Command that can't run instead of silently running the stock CLI #25667.
  • Trade-off (decided): a user who keeps one of the refused options in Arguments for terminals can't start a Codex chat until they change it. Arguments is shared, so removing the option there also changes terminal tabs. --profile, --oss, -C and --worktree have replacements that work in both places, and the copy names them. --add-dir has no chat equivalent: extra writable folders are a sandbox setting, and chat takes those from Agent Permissions.
  • Dropped quietly, because they only shape a terminal: --no-alt-screen, --no-daemon, help/version, and the bypass flags (Agent Permissions already reads them).
  • Model picker: its list is fetched with the same translated Arguments as the chat. Limit: an already-cached list refreshes on its normal schedule (up to 10 minutes), because the cache key doesn't include Arguments. Keying it would need a new saved field on every chat record.

Mechanism: codex-structured-launch-args.ts is one table. Each interactive Codex option is passed through, translated to the config Codex derives from it, dropped (terminal-only or a permission setting), or refused by name. The resulting options go after app-server instead of before it. Before the subcommand, Codex accepts every root option but app-server uses only the config ones. After it, an option app-server doesn't take fails loudly. The model-list probe runs app-server with the same options, and its own overrides come last so they still win.

Why

Saved Arguments are written for the interactive codex that a terminal runs, but a chat runs codex app-server. That reads only config (-c, --enable/--disable) and its own few options, and it silently ignores everything else placed before it. The common pattern hands launch arguments to the server process unfiltered after the subcommand, so bad arguments fail visibly. Orca can't do exactly that: its Arguments field is shared with terminals, and its default value (the terminal bypass flag) isn't an app-server option. Passing it through would make every untouched profile fail to start. So the translation step stays (an intended difference), but its job changes from silently filtering to honoring or refusing by name.

Alternatives considered:

  • A separate chat-only Arguments field. Rejected: users expect one setting to work everywhere.
  • Letting -s/-a in Arguments set the chat's permissions. Rejected: permissions have one source, so the permission picker can always name the state the chat is in.

Linked Issue

Follow-up to #25721 and #25667 (saved Command/Arguments in native chat). Merge order: after #26116 (chat permission setting + per-chat picker).

Visual Proof

QA ran on Windows (low spec), driven by a non-Claude agent over the DevTools protocol against a hidden window. Stand-in Codex/Claude CLIs logged every launch argument and JSON-RPC request. The box's real agent config files were unchanged at every step.

A refused saved option names the option and the fix (--profile work; nothing is started, and Retry works after editing Arguments):
r3-S4.jpg

--oss, -C, -i refusals (same wording rules; captured on the previous head, wording unchanged since):
r1-S5.jpg
r1-S6.jpg
r1-S7.jpg

Translated Arguments start the chat. With saved -m gpt-qa-arg --search -c foo.bar=1 --enable qa_feature, Codex was started as app-server -c foo.bar=1 --enable qa_feature -c model="gpt-qa-arg" -c web_search="live". The model-list probe got the same options, then -c approval_policy=never -c features.plugins=false --disable plugins:
r3-S3.jpg

Untouched Arguments (Yolo) start as app-server with thread policy never / danger-full-access. Manual plus saved -s read-only -a untrusted --approve-for-me -c sandbox_mode=read-only: none of those reach Codex, and the thread runs on-request / workspace-write from Agent Permissions:
r3-S1.jpg
r3-S2.jpg

Unclosed quote, existing chat restarting: the new card:
r3-S8b.jpg

Unclosed quote, NEW chat from the tab bar: the existing toast from main's shared launch planning (follow-up, see Notes):
r3-S8m.jpg

Testing

  • Targeted vitest files: the launch-args translator, launch resolution, the model-catalog probe, the configured-args tokenizer, refusal copy, and the native-chat failure notices.
  • tc:node, tc:web and tc:cli typechecks.
  • oxlint and the changed-code quality gate.
  • Electron QA on Windows (low spec) against stand-in CLIs: see Visual Proof.

Review

Pending.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH/WSL: Codex native chat still runs only on the local host. Arguments are read where the chat runs.
  • Mixed versions: the new unclosed-quote refusal is an optional field. An older client shows the host's sentence in an existing chat, and its generic "couldn't start" with Retry on a new chat. The other refusals reuse existing kinds, so an older client shows its own older wording.
  • --strict-config is now passed to app-server. Codex 0.130–0.132 rejects it there, just as a terminal on those versions does.
  • Mobile: shows the host's sentence.
  • Windows: the same tokenizer grammar as before.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…lently dropping them

Settings -> Agents -> Arguments is shared by terminal tabs and Codex native
chat, and is written for the interactive `codex`. The chat launch put the
Arguments before `app-server`, where Codex parses and then ignores -m,
--search, --oss and --local-provider, and Orca itself silently dropped
-s, -a, --profile, --add-dir, -C, -i, --strict-config and permission -c
keys. A chat could run a different model or provider than the terminal, or
a looser sandbox than `-s read-only` asked for, with no sign of it.

Every saved Argument now either reaches Codex the way a terminal applies
it, or the chat refuses to start and names the argument:
- Pass after `app-server`: -c/--config (every key), --enable/--disable,
  --strict-config.
- Translate to the config Codex itself derives: -m/--model -> model,
  --search -> web_search="live", --approve-for-me -> its three overrides.
- Under Manual, -s/-a and -c sandbox_mode/approval_policy become the
  thread policy instead of Manual's defaults (validated); Yolo unchanged.
- Refuse by name: --profile, --oss, --local-provider, --remote*,
  --add-dir, -C, -i, --worktree, --dangerously-bypass-hook-trust,
  prompts (also after --), invalid sandbox/approval values.
- Drop: the bypass flag (Agent Permissions reads it), --yolo, display
  and help/version flags.

The model-list probe launches with the same translated Arguments, its own
config last. An unclosed quote in Arguments is now a named refusal for
both agents, and the Agent Permissions tooltip says what the toggle does.
…w to fix refused Arguments

A resumed Codex chat kept the approvals reviewer it last ran with, because
the reviewer from --approve-for-me or -c approvals_reviewer= reached Codex
only as process config, which a resume's saved setting outranks. Removing
--approve-for-me left the automatic reviewer approving requests. Every
thread start and resume now states the reviewer with the rest of the
policy: the one the Arguments name under Manual, otherwise Codex's
default, user.

approval_policy on-failure (from -a or -c) now reads as on-request, which
is Codex's own alias, instead of refusing the chat.

Refusals now say what to do. A refused option reads "Codex chats can't use
--profile from saved Arguments. Remove it from Codex's Arguments in
Settings → Agents." plus, where one exists, what a chat honors instead
(config.toml or -c for a profile, -c model_provider= for --oss, the chat's
workspace folder for -C/--add-dir/--worktree, attaching an image for -i).
An invalid sandbox, approval policy or reviewer names the setting and its
accepted values. A lone "-" is a prompt, not an unnamed option.
…hat-codex-arguments

# Conflicts:
#	src/main/runtime/orca-runtime-get-worktree-ps.ts
#	src/main/runtime/structured-agent-session-runtime.ts
#	src/renderer/src/components/native-chat/agent-session-failure-words-text.ts
#	src/renderer/src/i18n/locales/en.json
… model in the --oss hint

The chat translated -c approvals_reviewer=guardian_subagent to auto_review
before stating it on the thread. Codex 0.120-0.123 take only
guardian_subagent there, so on those versions the chat failed to start
where the terminal worked. The reviewer now goes to Codex exactly as the
Arguments spell it (user, auto_review or guardian_subagent); every version
accepts the spelling its own config accepts.

--oss also picks a local default model, so the hint for --oss and
--local-provider now says to choose a local model with -m as well.
… alone

Saved Arguments no longer set a Codex chat's sandbox, approval policy or
reviewer. -s/--sandbox, -a/--ask-for-approval and --approve-for-me /
--not-so-yolo are dropped quietly, like the bypass flag (a value option
still consumes its value, and a missing one is still refused), and -c /
--config whose top key is approval_policy, sandbox_mode,
approvals_reviewer or sandbox_workspace_write never reaches app-server.
The thread policy comes from the Agent Permissions setting only, and
still states the reviewer, user, on every start and resume so a resumed
chat never keeps a stale one.

This removes the invalid-value refusal and its copy in every locale, the
value aliases, and the policy parameter the Arguments used to feed.
…f in the model probe

Scope: the per-chat permission picker already states the approvals
reviewer on every thread open and removes the Agent Permissions tooltip,
and this PR now drops every reviewer key from Arguments. So the reviewer
statement, its tests, the tooltip copy and its six translations, and the
permission-policy wiring test come out; those files match main again.

The model-list probe put its own config after the saved Arguments, but
Codex applies --enable/--disable after every -c, so a saved
--enable plugins re-enabled plugin startup in the short-lived probe. The
probe now also passes --disable plugins, which Codex applies last.
@brennanb2025

brennanb2025 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Review summary

Problem. Settings → Agents → Arguments is shared by terminal tabs and native chat. In Codex native chat, saved Arguments were quietly lost:

  • some were dropped: --profile, --add-dir, -C, --strict-config;
  • others were placed before app-server, where Codex parses them and then ignores them: -m, --search, --oss, --local-provider.

So a chat could run a different model, provider or config than the terminal. For example, --oss --local-provider ollama sent chat prompts to the default cloud provider. Nothing told the user. The model picker's list also ignored Arguments.

User-facing change.

  • Every saved Codex Argument now does one of four things:
    • works in chat as in a terminal (-c, --enable/--disable, --strict-config, and -m/--search translated to the config Codex derives from them);
    • is a permission setting, which chat takes from Agent Permissions only;
    • is terminal-only display and is dropped;
    • stops the chat from starting, with a card naming the option and what to use instead.
  • The model picker's list uses the same Arguments.
  • An unclosed quote names Arguments when the host starts or restarts a chat.

What review fixed along the way (5 review rounds plus a readiness pass):

Deferred or accepted:

  • Accepted cost: a terminal -s read-only doesn't make chat read-only.
  • Refused options block chat until changed. This trade-off was decided. --add-dir has no chat equivalent.
  • Model-list cache: the picker's cached list refreshes on its own schedule (up to 10 minutes). Keying the cache by Arguments needs a new persisted field.
  • Unclosed quote in a NEW desktop chat: it still shows main's generic toast, "Could not build launch command for Codex." The shared terminal-launch planning stops first, the same as for terminal tabs. A follow-up should make that toast name Arguments.
  • Nits: a --search=x misuse message; the --add-dir hint; the probe looking up the environment before refusing; singular/plural file names.
  • Separate follow-ups:

Verified:

  • Codex CLI parser behavior, read from the codex-rs source at 0.130–current. Covered: -c/--enable/--disable after app-server, root -m/--oss/--search being ignored, --profile bailing, and the precedence of --enable/--disable after -c.
  • Typechecks tc:node, tc:web and tc:cli; the targeted vitest files; oxlint and the changed-code quality gate.
  • Full CI on the final head 3cf38f27827: PR test LoC, Mobile Checks and unit full all pass (Headless Node skipped). Unit full attempt 1 failed one Windows test, profile-state-writer-stall.electron.test.ts (a SQLite profile-state writer timing out during simulated main-process stalls). This PR touches no persistence code, and the test passed on re-run (attempt 2).
  • Electron QA on Windows (low spec), Codex gpt-5.6-terra driver, stand-in CLIs. Covered: exact argv and thread params for untouched, Manual and translated Arguments; the model-probe argv carrying the saved Arguments; the refusal copy verbatim with nothing started; Retry after editing; the unclosed-quote card on restart; the box's real configs unchanged.

Not verified:

  • A live real Codex binary (by rule; parser behavior is from source).
  • Codex versions below 0.130.
  • macOS/Linux Electron QA (Windows only; the tokenizer and argv paths are platform-neutral apart from the Windows shim test).
  • SSH/WSL: Codex native chat is local-only, unchanged.
  • Resume after an app relaunch at this head (observed at the previous head).

Merge order (updated 2026-10-07): #26116 → this PR. The permission plan changed: chat gets its own "Permissions for new chats" setting (Settings → Chat) and stops reading the terminal Agent Permissions setting (#26116). #24508 is no longer in this stack. Nothing in this PR changes: it already drops permission flags from Arguments and leaves the thread policy to the chat's permission setting.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 7, 2026 18:57
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7d85c037-aa20-4740-97e5-8e6fdc151b0c
📥 Commits

Reviewing files that changed from the base of the PR and between 80d4509 and 3cf38f2.

📒 Files selected for processing (30)
  • src/main/codex-cli/codex-read-only-app-server-args.ts
  • src/main/codex/codex-model-catalog-probe.test.ts
  • src/main/codex/codex-model-catalog-probe.ts
  • src/main/codex/codex-short-lived-app-server-spawn.test.ts
  • src/main/codex/codex-structured-launch-args.test.ts
  • src/main/codex/codex-structured-launch-args.ts
  • src/main/codex/codex-structured-launch-resolution.test.ts
  • src/main/codex/codex-structured-launch-resolution.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-failure-text.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-pre-spawn-first-answer.test.ts
  • src/main/native-chat/structured-agent-arguments-error.ts
  • src/main/native-chat/structured-agent-configured-args.test.ts
  • src/main/native-chat/structured-agent-configured-args.ts
  • src/main/runtime/structured-agent-model-catalog-wiring.ts
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.launch-lifecycle.test.tsx
  • src/renderer/src/components/native-chat/agent-session-arguments-failure-words-text.ts
  • src/renderer/src/components/native-chat/agent-session-failure-words-text.ts
  • src/renderer/src/i18n/en-runtime-required.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
  • src/shared/agent-session-argument-problem-words.ts
  • src/shared/agent-session-argument-problem.ts
  • src/shared/agent-session-failure-copy.ts
  • src/shared/agent-session-failure-words.ts
  • src/shared/agent-session-refusal-notice.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The change updates how saved Codex arguments are normalized and passed to structured app-server launches and the model catalog probe. It converts model and search options to config overrides, filters permission options, and rejects unsupported arguments. It also adds structured reporting for unclosed quotes and argument failures, with updated guidance and translations.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3cf38

No identified issue blocks merging; normal checks remain appropriate.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 23 files. (7 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: applying or refusing saved Codex Arguments instead of silently dropping them.
Description check ✅ Passed The description is complete and aligned with the template. It explains the user impact, implementation, rationale, alternatives, linked issues, visual proof, testing, compatibility considerations, and…
Full details: Docstring Coverage

Explanation

Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 23 files. (7 skipped: 7 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing unmerged. Native chat no longer reads saved Arguments for any agent; they are for terminal launches (#26506). So this per-flag Codex translation isn't needed. Codex chat starts as plain app-server, and chat settings come from ~/.codex/config.toml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant