diff --git a/docs/architecture.md b/docs/architecture.md index 33ebf937..80345dc6 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -60,7 +60,7 @@ The standalone manager normalizes bare account-manager commands, so both `codex- - Handles multi-auth `auth` subcommands locally. - Forwards non-auth commands to official Codex. - For request-bearing sessions with runtime rotation enabled, creates a temporary shadow `CODEX_HOME`, writes a local provider (`codex-multi-auth-runtime-proxy`), and starts a loopback proxy for that process. -- For interactive TUI sessions, `resume`/`fork`, and `codex app-server`, keeps the canonical `CODEX_HOME` and passes the same provider as `-c` overrides instead, so session history and SQLite state are not copied into a shadow and reindexed on every launch. A resident `app-server` needs the canonical home for a stronger reason than convenience: it cannot start at all on a shadow home whose `app-server-control` is a symlink, and a shadow thread index would stay frozen for the life of the process (#659). +- For interactive TUI sessions (with or without the optional initial `[PROMPT]`), `resume`/`fork`, and `codex app-server`, keeps the canonical `CODEX_HOME` and passes the same provider as `-c` overrides instead, so session history and SQLite state are not copied into a shadow and reindexed on every launch. A resident `app-server` needs the canonical home for a stronger reason than convenience: it cannot start at all on a shadow home whose `app-server-control` is a symlink, and a shadow thread index would stay frozen for the life of the process (#659). - Keeps forwarded sessions on file-backed auth state unless the caller opts out. - Supports ephemeral force-pin: `codex-multi-auth-codex --account ` (or `CODEX_MULTI_AUTH_FORCE_ACCOUNT`) for a single invocation only — never mutates the persisted `switch` pin. diff --git a/docs/configuration.md b/docs/configuration.md index 4b5c3b7a..7332c15c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -142,7 +142,7 @@ Keep these enabled for most environments: ## Runtime Rotation Proxy -`codexRuntimeRotationProxy` is enabled by default. When enabled through defaults, settings, `codex-multi-auth rotation enable`, or `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=1`, the `codex-multi-auth-codex` wrapper starts a localhost-only Responses proxy for forwarded official Codex sessions, including CLI request commands, `codex app-server`, and `codex app` launches through the wrapper. For non-interactive request commands and `codex app`, the wrapper writes a temporary shadow `CODEX_HOME/config.toml` that selects a custom provider named `codex-multi-auth-runtime-proxy`, launches the official Codex surface against that provider, and removes the shadow home after the owning process exits. Interactive TUI sessions, `resume`/`fork`, and `codex app-server` instead stay on the canonical `CODEX_HOME` and receive the same provider through `-c` overrides, which avoids reindexing session history on every launch and leaves the real `config.toml` untouched. Set `codexRuntimeRotationProxy=false`, run `codex-multi-auth rotation disable`, or set `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0` to bypass the proxy. +`codexRuntimeRotationProxy` is enabled by default. When enabled through defaults, settings, `codex-multi-auth rotation enable`, or `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=1`, the `codex-multi-auth-codex` wrapper starts a localhost-only Responses proxy for forwarded official Codex sessions, including CLI request commands, `codex app-server`, and `codex app` launches through the wrapper. For non-interactive request commands and `codex app`, the wrapper writes a temporary shadow `CODEX_HOME/config.toml` that selects a custom provider named `codex-multi-auth-runtime-proxy`, launches the official Codex surface against that provider, and removes the shadow home after the owning process exits. Interactive TUI sessions (with or without the optional initial prompt), `resume`/`fork`, and `codex app-server` instead stay on the canonical `CODEX_HOME` and receive the same provider through `-c` overrides, which avoids reindexing session history on every launch and leaves the real `config.toml` untouched. Set `codexRuntimeRotationProxy=false`, run `codex-multi-auth rotation disable`, or set `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0` to bypass the proxy. A single forwarded run can be pinned to one account with `codex-multi-auth-codex --account ` (or `CODEX_MULTI_AUTH_FORCE_ACCOUNT`). The pin is applied per-invocation by that run's own proxy instance, so it never touches the persisted `switch` pin and cannot leak across concurrent sessions. Because the proxy is required for the pin to take effect, `--account` fails hard when the proxy is disabled rather than silently using a rotated account. See [Force an account for one invocation](reference/commands.md#force-an-account-for-one-invocation). diff --git a/docs/development/ARCHITECTURE.md b/docs/development/ARCHITECTURE.md index cc679624..c8ca585e 100644 --- a/docs/development/ARCHITECTURE.md +++ b/docs/development/ARCHITECTURE.md @@ -55,7 +55,7 @@ Official Codex CLI Runtime rotation enabled -> one of two homes, over four branches | - |- interactive TUI (no forwarded subcommand), resume, fork + |- interactive TUI (no forwarded subcommand, or a root [PROMPT]), resume, fork | canonical CODEX_HOME + ephemeral -c provider overrides | (no shadow copy, no provider/transport rewrite of config.toml, | detach on exit; the auth-store reconcile above still applies) @@ -171,7 +171,7 @@ Policy evaluation (`lib/policy/runtime-policy.ts`) can block paused/drained acco | Branch | Predicate | Transport | | --- | --- | --- | - | Interactive TUI | `isCodexInteractiveTuiCommand` — no forwarded subcommand at all | App runtime helper with `useCanonicalHome: true` and `detachOnExit: true`. Runs against the **canonical** `CODEX_HOME`; the provider is passed as ephemeral `-c model_providers.*` overrides. No shadow copy and no state sync-back. Nothing **provider- or transport-related** is written into `config.toml` on this path — the only top-level key the wrapper still reconciles there is `cli_auth_credentials_store`, which is transport-independent (see step 4 note). | + | Interactive TUI | `isCodexInteractiveTuiCommand` — no forwarded root subcommand: bare `codex [OPTIONS]`, and `codex [OPTIONS] [PROMPT]` with the optional initial prompt, including one forced by `--`. A first positional that does not name a real root subcommand is that prompt, not a command (#673). | App runtime helper with `useCanonicalHome: true` and `detachOnExit: true`. Runs against the **canonical** `CODEX_HOME`; the provider is passed as ephemeral `-c model_providers.*` overrides. No shadow copy and no state sync-back. Nothing **provider- or transport-related** is written into `config.toml` on this path — the only top-level key the wrapper still reconciles there is `cli_auth_credentials_store`, which is transport-independent (see step 4 note). | | Interactive `resume` / `fork` | `isCodexInteractiveResumeCommand` — forwarded command is `resume` or `fork` | Same transport and options as the interactive TUI above. These open a TUI against an existing thread, so they must see the canonical thread index. | | `app-server` | `isCodexAppServerCommand` — forwarded command is `app-server` | App runtime helper with `useCanonicalHome: true`, `detachOnExit: false`, `installAppServerShim: false`, and `proxyAppServerAccountRead: true`. Same canonical home as the interactive branches; the differences are that a resident server owns its proxy for its whole lifetime rather than detaching it, and that the account-response rewriting is requested explicitly because no shim is present to set the label env. | | `codex app` | `isCodexAppCommand` — forwarded command is `app` | App runtime helper process with a shadow `CODEX_HOME`, plus the app-server CLI shim. | diff --git a/docs/development/CONFIG_FLOW.md b/docs/development/CONFIG_FLOW.md index b9197968..b63ec8f4 100644 --- a/docs/development/CONFIG_FLOW.md +++ b/docs/development/CONFIG_FLOW.md @@ -70,10 +70,10 @@ For dashboard display values: ## 6) Runtime Rotation Flow 1. Resolve `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY`; if unset, read `pluginConfig.codexRuntimeRotationProxy`, which defaults to enabled. -2. If disabled or the forwarded command is help/non-requesting, forward directly to official Codex. +2. If disabled or the forwarded command is help/non-requesting, forward directly to official Codex. This includes a help flag on a root launch that also carries a prompt (`codex "prompt" --help`): it prints help and exits clean, and the interactive branch detaches its helper on a clean exit, so a helper started just to print help would idle until its detached timeout. The scan stops at `--`, so help-looking text inside a forced prompt still routes normally (#673). 3. If enabled, start a loopback Responses proxy with a per-process client token. 4. Select a transport from the forwarded argv: - - **No forwarded subcommand (interactive TUI)** — keep the canonical `CODEX_HOME` and pass `codex-multi-auth-runtime-proxy` as ephemeral `-c model_providers.*` overrides. Nothing is copied, no provider or transport config is written into `config.toml`, and the helper detaches on exit. The transport-independent `cli_auth_credentials_store` reconcile below still applies. + - **No forwarded root subcommand (interactive TUI)** — bare `codex [OPTIONS]`, and `codex [OPTIONS] [PROMPT]` carrying the optional initial prompt (including one forced by `--`), whose provider overrides are injected ahead of that prompt rather than appended (#673). Keep the canonical `CODEX_HOME` and pass `codex-multi-auth-runtime-proxy` as ephemeral `-c model_providers.*` overrides. Nothing is copied, no provider or transport config is written into `config.toml`, and the helper detaches on exit. The transport-independent `cli_auth_credentials_store` reconcile below still applies. - **`resume` / `fork`** — the same canonical-home transport as the interactive TUI. These resume an existing thread, and the shadow home omits the runtime SQLite state, so the shadow transport could not see the requested thread (#647). - **`codex app`** — run the app runtime helper against a shadow `CODEX_HOME`. - **Any other request-bearing command** — create a temporary shadow `CODEX_HOME` and rewrite its `config.toml` to use `codex-multi-auth-runtime-proxy`. diff --git a/scripts/codex.js b/scripts/codex.js index 1ddedc59..a245cba7 100755 --- a/scripts/codex.js +++ b/scripts/codex.js @@ -421,6 +421,14 @@ function extractForcedAccountFlag(args) { let sawFlag = false; for (let index = 0; index < args.length; index += 1) { const arg = args[index]; + // `--` ends option parsing: everything after it is Codex's root + // `[PROMPT]` text. A `--account` spelled in there is the user's prompt, + // not a launcher flag — stripping it would both mangle the prompt and + // silently pin the run to an account nobody selected. + if (arg === "--") { + strippedArgs.push(...args.slice(index)); + break; + } if (arg === "--account") { sawFlag = true; const next = args[index + 1]; @@ -1365,6 +1373,9 @@ function replaceRequestedModel(args, nextModel) { const nextArgs = [...args]; for (let i = 0; i < nextArgs.length; i += 1) { const arg = nextArgs[i]; + // Past `--` the tokens are the user's prompt. Rewriting one in place + // would edit the prompt text itself before re-forwarding it. + if (arg === "--") break; if ((arg === "--model" || arg === "-m") && typeof nextArgs[i + 1] === "string") { nextArgs[i + 1] = nextModel; return nextArgs; @@ -1859,6 +1870,11 @@ async function forwardToRealCodex(codexBin, rawArgs, baseEnv = process.env) { function hasCliAuthCredentialsStoreOverride(args) { for (let i = 0; i < args.length; i += 1) { const arg = args[i]; + // Everything after `--` is the root `[PROMPT]` positional, not options: + // a prompt whose text spells `--config=cli_auth_credentials_store=...` is + // not a caller override, and treating it as one would silently drop the + // file auth store this wrapper depends on. + if (arg === "--") break; if (arg === "-c" || arg === "--config") { const next = args[i + 1]; if (!next || !next.includes("=")) continue; @@ -2357,6 +2373,10 @@ function coerceReasoningEffortForModel(model, effort) { function extractRequestedModel(args) { for (let i = 0; i < args.length; i += 1) { const arg = args[i]; + // Past `--` every token is root `[PROMPT]` text; a `-m` in there names + // no model, and reading one would coerce reasoning config for a model + // the user never requested. + if (arg === "--") break; if (arg === "--model" || arg === "-m") { const next = args[i + 1]; if (typeof next === "string" && next.trim().length > 0) { @@ -2427,6 +2447,8 @@ function rewriteReasoningConfigArgs(rawArgs) { const nextArgs = [...rawArgs]; for (let i = 0; i < nextArgs.length; i += 1) { const arg = nextArgs[i]; + // Past `--` a `-c key=value` pair is prompt text, not a config override. + if (arg === "--") break; if ((arg === "-c" || arg === "--config") && typeof nextArgs[i + 1] === "string") { nextArgs[i + 1] = rewriteReasoningConfigAssignment( nextArgs[i + 1], @@ -4903,13 +4925,20 @@ async function createRuntimeRotationAppHelperContext( } }; + const injectedArgs = [ + ...helperArgs, + "-c", + `model_provider=${configTomlModule.tomlStringLiteral(RUNTIME_ROTATION_PROXY_PROVIDER_ID)}`, + ]; return { - args: [ - ...baseContext.args, - ...helperArgs, - "-c", - `model_provider=${configTomlModule.tomlStringLiteral(RUNTIME_ROTATION_PROXY_PROVIDER_ID)}`, - ], + // Root TUI argv can end in the optional `[PROMPT]` positional, and `--` + // binds everything after it to that positional — overrides appended + // there would become prompt text, so the interactive branch asks for + // them to be injected ahead of the prompt instead. + args: + options.injectArgsBeforeRootPrompt === true + ? insertArgsBeforeRootPrompt(baseContext.args, injectedArgs) + : insertArgsBeforeForwardedSeparator(baseContext.args, injectedArgs), env: { ...baseContext.env, ...helperEnv, @@ -4991,13 +5020,19 @@ async function createRuntimeRotationProxyContextIfEnabled( if (isCodexAppCommand(rawArgs)) { return startAppHelperContext(); } - if ( - isCodexInteractiveTuiCommand(rawArgs) || - isCodexInteractiveResumeCommand(rawArgs) - ) { + // Classify once: each predicate re-walks the whole argv, and the root-TUI + // answer is needed twice below. + const isRootTuiLaunch = isCodexInteractiveTuiCommand(rawArgs); + if (isRootTuiLaunch || isCodexInteractiveResumeCommand(rawArgs)) { return startAppHelperContext({ detachOnExit: true, useCanonicalHome: true, + // Root TUI launches may carry the optional `[PROMPT]` positional + // (possibly forced by `--`); the provider overrides must stay on the + // option side of it. `resume`/`fork` keep the appended ordering their + // argv has always forwarded with — their tokens are subcommand args, + // not the root prompt. + injectArgsBeforeRootPrompt: isRootTuiLaunch, }); } @@ -5088,90 +5123,257 @@ function isPureHelpOrVersionArgs(rawArgs) { ); } +// Hoisted: every argv walk calls consumesNextArg once per token, and a Set +// rebuilt per call allocated this list on every one of them. +const OPTIONS_CONSUMING_NEXT_ARG = new Set([ + "-c", + "--config", + "--enable", + "--disable", + "--listen", + "--remote", + "--remote-auth-token-env", + "--ws-auth", + "--ws-token-file", + "--ws-token-sha256", + "--ws-shared-secret-file", + "--ws-issuer", + "--ws-audience", + "--ws-max-clock-skew-seconds", + "--download-url", + "-i", + "--image", + "-m", + "--model", + "--local-provider", + "-p", + "--profile", + "-s", + "--sandbox", + "-a", + "--ask-for-approval", + "-C", + "--cd", + "--add-dir", + "--output-schema", + "--color", + "-o", + "--output-last-message", +]); + function consumesNextArg(arg) { - return new Set([ - "-c", - "--config", - "--enable", - "--disable", - "--listen", - "--remote", - "--remote-auth-token-env", - "--ws-auth", - "--ws-token-file", - "--ws-token-sha256", - "--ws-shared-secret-file", - "--ws-issuer", - "--ws-audience", - "--ws-max-clock-skew-seconds", - "--download-url", - "-i", - "--image", - "-m", - "--model", - "--local-provider", - "-p", - "--profile", - "-s", - "--sandbox", - "-a", - "--ask-for-approval", - "-C", - "--cd", - "--add-dir", - "--output-schema", - "--color", - "-o", - "--output-last-message", - ]).has(arg); + return OPTIONS_CONSUMING_NEXT_ARG.has(arg); } -function findForwardedCommand(rawArgs) { - if (!Array.isArray(rawArgs) || rawArgs.length === 0) { - return null; +// Codex's root grammar is `codex [OPTIONS] [PROMPT]` / `codex [OPTIONS] +// [ARGS]`, so the first positional is only a command when it names a +// real root subcommand — otherwise it is the optional initial prompt of an +// interactive TUI launch. The allowlist mirrors `codex --help`; treating an +// unknown positional as a prompt matches native Codex, where an unrecognized +// token also falls through to the prompt positional rather than erroring as a +// bad subcommand. Routing predicates all resolve through this map so aliases +// classify like their canonical command. +// +// A real root subcommand missing from this set is misrouted, not merely +// misnamed: it classifies as an interactive prompt and takes the app-helper +// branch with `detachOnExit: true`, so a short-lived command leaves a detached +// helper and its loopback proxy idling until the rotation idle timeout. Keep +// the set in sync with the upstream grammar — clap publishes the authoritative +// root list in its own completions, so +// codex completion bash | grep -oE 'opts="[^"]*"' | grep ' exec ' +// prints every root command (and alias) for the installed codex-cli. + +const CODEX_ROOT_COMMAND_ALIASES = new Map([ + ["e", "exec"], + ["a", "apply"], +]); +const CODEX_ROOT_COMMANDS = new Set([ + "exec", + "review", + "login", + "logout", + "mcp", + "plugin", + "mcp-server", + "app-server", + "remote-control", + "app", + "completion", + "update", + "doctor", + "sandbox", + "debug", + "apply", + "resume", + "archive", + "delete", + "migrate-rollouts", + "unarchive", + "fork", + "cloud", + "exec-server", + "features", + "help", + "auth", + // Hidden root subcommands: absent from `codex --help`, but real — each is + // documented by `codex help `. + "responses-api-proxy", + "cloud-tasks", + "execpolicy", + // Internal stdio/UDS relay. Hidden like the three above, and short-lived: + // omitting it classified `codex stdio-to-uds ` as a root prompt, + // which took the detached app-helper branch and leaked a helper per call. + "stdio-to-uds", +]); + +function resolveCodexRootCommand(token) { + const command = CODEX_ROOT_COMMAND_ALIASES.get(token) ?? token; + return CODEX_ROOT_COMMANDS.has(command) ? command : null; +} + +// `-i/--image` is greedy (`...`): native Codex consumes every following +// free token up to the next option-like token, so a prompt can only follow the +// image list after another option or a `--`. Every argv walker below skips the +// same span, or the wrapper would disagree with native Codex about where the +// positional side starts. +function consumesRemainingFreeArgs(arg) { + return arg === "-i" || arg === "--image"; +} + +function skipOptionValueSpan(args, i) { + const arg = args[i]; + if (consumesRemainingFreeArgs(arg)) { + let end = i; + while ( + end + 1 < args.length && + typeof args[end + 1] === "string" && + args[end + 1].length > 0 && + (args[end + 1] === "-" || !args[end + 1].startsWith("-")) + ) { + end += 1; + } + return end; } - for (let i = 0; i < rawArgs.length; i += 1) { - const arg = rawArgs[i]; + return consumesNextArg(arg) ? i + 1 : i; +} + +/** + * The single definition of where an argv's option side ends. Walking from + * `startIndex`, skips option flags and the values they consume — including a + * greedy `-i a.png b.png` span — and reports what stopped the walk: + * + * - `index`: the first free (positional) token, or null when there is none. + * - `terminatorIndex`: the bare `--` that ended option parsing, or null. + * - `boundary`: where option-side content ends, i.e. the positional, the + * `--`, or `args.length`. This is where injected overrides belong. + * + * Callers keep their own policy for `--` (the root grammar binds everything + * after it to `[PROMPT]`; a subcommand's does not), but they must not keep + * their own copy of the walk: a classifier and an injector that disagree by + * one token put a `-c` override into prompt text or read prompt text as a + * subcommand. + */ +function findRootPositional(args, startIndex = 0) { + if (!Array.isArray(args)) { + return { index: null, terminatorIndex: null, boundary: 0 }; + } + for (let i = startIndex; i < args.length; i += 1) { + const arg = args[i]; if (typeof arg !== "string" || arg.length === 0) continue; if (arg === "--") { - return i + 1 < rawArgs.length - ? { command: rawArgs[i + 1], index: i + 1 } - : null; + return { index: null, terminatorIndex: i, boundary: i }; } if (arg.startsWith("--config=")) { continue; } if (arg.startsWith("--") || (arg.startsWith("-") && arg !== "-")) { - if (consumesNextArg(arg)) { - i += 1; - } + i = skipOptionValueSpan(args, i); continue; } - return { command: arg, index: i }; + return { index: i, terminatorIndex: null, boundary: i }; + } + return { index: null, terminatorIndex: null, boundary: args.length }; +} + +function findForwardedCommand(rawArgs) { + if (!Array.isArray(rawArgs) || rawArgs.length === 0) { + return null; + } + // `--` ends option parsing: everything after it binds to the root + // `[PROMPT]` positional, never to a subcommand, even when the text happens + // to spell a command name. `findRootPositional` reports it as a terminator + // and yields no positional, which settles the launch as interactive. + // + // Native Codex fills the root `[PROMPT]` slot with the first free token and + // still tries a later one as a subcommand (`codex hello exec …` runs exec), + // so an unknown positional keeps the scan going. + for ( + let cursor = findRootPositional(rawArgs, 0); + cursor.index !== null; + cursor = findRootPositional(rawArgs, cursor.index + 1) + ) { + const command = resolveCodexRootCommand(rawArgs[cursor.index]); + if (command !== null) { + return { command, index: cursor.index }; + } } return null; } +// Inserts option-side args ahead of the root `[PROMPT]` positional (or the +// `--` that forces one), so injected overrides cannot be read as prompt text. +// With no positional at all this is a plain append, which keeps bare-TUI argv +// byte-identical to the pre-insertion shape. +function insertArgsBeforeRootPrompt(baseArgs, injectedArgs) { + const { boundary } = findRootPositional(baseArgs); + return [ + ...baseArgs.slice(0, boundary), + ...injectedArgs, + ...baseArgs.slice(boundary), + ]; +} + +// Inserts args before a `--` separator when one is present, else appends. +// Subcommand argv keeps its leading subcommand token — `resume`/`fork` argv is +// pinned to it — but anything appended past a `--` lands in the subcommand's +// own positional list: Codex rejects that outright for `exec`/`review` +// ("unexpected argument '-c' found"), and worse, silently forwards it as +// arguments for `sandbox` and into the stored config for `mcp add`. The +// separator bounds the append. +function insertArgsBeforeForwardedSeparator(baseArgs, injectedArgs) { + // Deliberately not `findRootPositional`: that reports where the option side + // ends, stopping at the first positional, and a subcommand's own arguments + // are positionals — it would answer `exec` for `exec -- hi`. The question + // here is only where the separator sits, and a bare `indexOf` answers it + // exactly: clap never consumes `--` as an option value (`codex --cd -- hi` + // is "a value is required for '--cd'"), so the first `--` is always the + // separator, and it always follows the subcommand token because + // `findForwardedCommand` returns null the moment it sees one. + const separatorIndex = baseArgs.indexOf("--"); + if (separatorIndex === -1) { + return [...baseArgs, ...injectedArgs]; + } + return [ + ...baseArgs.slice(0, separatorIndex), + ...injectedArgs, + ...baseArgs.slice(separatorIndex), + ]; +} + function findForwardedSubcommand(rawArgs, commandIndex) { - for (let i = commandIndex + 1; i < rawArgs.length; i += 1) { - const arg = rawArgs[i]; - if (typeof arg !== "string" || arg.length === 0) continue; - if (arg === "--") { - return i + 1 < rawArgs.length ? rawArgs[i + 1] : null; - } - if (arg.startsWith("--config=")) { - continue; - } - if (arg.startsWith("--") || (arg.startsWith("-") && arg !== "-")) { - if (consumesNextArg(arg)) { - i += 1; - } - continue; - } - return arg; + const { index, terminatorIndex } = findRootPositional( + rawArgs, + commandIndex + 1, + ); + // Unlike the root grammar, a subcommand's `--` still precedes its own + // positional args, so the token after it is the nested subcommand. + if (terminatorIndex !== null) { + return terminatorIndex + 1 < rawArgs.length + ? rawArgs[terminatorIndex + 1] + : null; } - return null; + return index === null ? null : rawArgs[index]; } function hasHelpFlagAfterCommand(rawArgs, commandIndex) { @@ -5179,8 +5381,8 @@ function hasHelpFlagAfterCommand(rawArgs, commandIndex) { const arg = rawArgs[i]; if (arg === "--") return false; if (arg === "--help" || arg === "-h" || arg === "help") return true; - if (typeof arg === "string" && consumesNextArg(arg)) { - i += 1; + if (typeof arg === "string") { + i = skipOptionValueSpan(rawArgs, i); } } return false; @@ -5194,6 +5396,11 @@ function isCodexAppServerCommand(rawArgs) { return findForwardedCommand(rawArgs)?.command === "app-server"; } +// True for every root TUI launch: bare `codex [OPTIONS]`, and `codex +// [OPTIONS] [PROMPT]` with the optional initial prompt — including a prompt +// forced by `--`. Both shapes start the same interactive session and need the +// canonical home, or Codex stalls rebuilding the omitted thread index inside +// the shadow mirror before it can submit the already-delivered prompt. function isCodexInteractiveTuiCommand(rawArgs) { return findForwardedCommand(rawArgs) === null; } @@ -5219,7 +5426,14 @@ function shouldUseRuntimeRoutingForForwardedArgs(rawArgs) { const command = findForwardedCommand(rawArgs); if (!command) { - return true; + // A help flag alongside a root prompt (`codex "prompt" --help`) prints + // help and exits clean, so it must skip the transport for the same + // reason as the request-command help forms below: the interactive + // branch detaches its helper on a clean exit, and a helper started + // just to print help would idle until its detached timeout. The scan + // stops at `--`, so help-looking text inside a forced prompt still + // routes normally. + return !hasHelpFlagAfterCommand(rawArgs, -1); } const requestCommands = new Set(["exec", "review", "resume", "fork", "app"]); @@ -5450,12 +5664,16 @@ function buildForwardArgs(rawArgs) { return { args: compatibilityArgs, requestedModel }; } + const authStoreArgs = ["-c", 'cli_auth_credentials_store="file"']; return { - args: [ - ...compatibilityArgs, - "-c", - 'cli_auth_credentials_store="file"', - ], + // A root launch may end in the `[PROMPT]` positional — possibly forced + // by `--`, where an appended `-c` pair would be read as prompt text — + // so the override is inserted on the option side of the prompt there. + // Subcommand argv keeps the appended ordering it has always forwarded. + args: + findForwardedCommand(compatibilityArgs) === null + ? insertArgsBeforeRootPrompt(compatibilityArgs, authStoreArgs) + : insertArgsBeforeForwardedSeparator(compatibilityArgs, authStoreArgs), requestedModel, }; } diff --git a/test/codex-bin-wrapper.test.ts b/test/codex-bin-wrapper.test.ts index 0b0b2aed..2a4abaf6 100644 --- a/test/codex-bin-wrapper.test.ts +++ b/test/codex-bin-wrapper.test.ts @@ -1287,6 +1287,76 @@ describe("codex bin wrapper", () => { expect(output).not.toContain("--account"); }); + // `--` makes everything after it the root `[PROMPT]` positional, so a + // launcher-only flag spelled inside that prompt is the user's text. The + // stripper walked the whole argv, so it both mangled the prompt and pinned + // the run to an account nobody selected — or hard-failed on a range check + // for a launch that contained no flag at all. + it("leaves a `--account` inside a `--`-forced prompt as prompt text", () => { + const fixtureRoot = createWrapperFixture(); + createRuntimeRotationProxyFixtureModule(fixtureRoot); + const originalHome = join(fixtureRoot, "codex-home"); + mkdirSync(originalHome, { recursive: true }); + writeAccountsFixture(originalHome, 3); + const fakeBin = createCustomFakeCodexBin(fixtureRoot, [ + "#!/usr/bin/env node", + 'console.log(`FORWARDED:${process.argv.slice(2).join(" ")}`);', + 'console.log(`FORCED_INDEX:${process.env.CODEX_MULTI_AUTH_FORCE_ACCOUNT_INDEX ?? ""}`);', + "process.exit(0);", + ]); + + const result = runWrapper( + fixtureRoot, + ["exec", "--", "--account", "3", "is", "wrong"], + { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + CODEX_HOME: originalHome, + CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "0", + OPENAI_API_KEY: undefined, + }, + ); + + const output = combinedOutput(result); + expect(result.status).toBe(0); + // The prompt reaches Codex intact, flag tokens and all. + expect(output).toContain("-- --account 3 is wrong"); + // And no pin was inferred from the user's prose. + expect(output).toContain("FORCED_INDEX:"); + expect(output).not.toContain("FORCED_INDEX:2"); + }); + + // Same boundary, different scanner: a `-m` inside the prompt named no + // model, but the wrapper read one and then rewrote it in place on the + // unsupported-model retry — editing the user's prompt text before + // re-forwarding it. + it("does not read or rewrite a `-m` inside a `--`-forced prompt", () => { + const fixtureRoot = createWrapperFixture(); + createRuntimeRotationProxyFixtureModule(fixtureRoot); + const originalHome = join(fixtureRoot, "codex-home"); + mkdirSync(originalHome, { recursive: true }); + writeAccountsFixture(originalHome, 1); + const fakeBin = createCustomFakeCodexBin(fixtureRoot, [ + "#!/usr/bin/env node", + 'console.log(`FORWARDED:${process.argv.slice(2).join(" ")}`);', + "process.exit(0);", + ]); + + const result = runWrapper( + fixtureRoot, + ["exec", "--", "-m", "gpt-4", "please", "fix"], + { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + CODEX_HOME: originalHome, + CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "0", + OPENAI_API_KEY: undefined, + }, + ); + + const output = combinedOutput(result); + expect(result.status).toBe(0); + expect(output).toContain("-- -m gpt-4 please fix"); + }); + it("repairs local session index and suppresses known Codex rollout-store noise", () => { const fixtureRoot = createWrapperFixture(); const codexHome = join(fixtureRoot, "codex-home"); @@ -4329,6 +4399,287 @@ describe("codex bin wrapper", () => { ); }); + // The root grammar is `codex [OPTIONS] [PROMPT]`: an initial prompt makes the + // launch no less interactive, but the classifier used to read the first + // positional as a subcommand, so prompt-bearing launches fell through to the + // shadow-home transport. The shadow mirror deliberately omits the runtime + // SQLite state, so native Codex rebuilt the entire thread index from + // rollouts — a startup stall that scales with session history — before it + // submitted the already-delivered prompt (#673). + for (const promptForm of [ + { + label: "a prompt after value-consuming options", + args: [ + "-c", + 'model_reasoning_effort="high"', + "-i", + "notes.png", + // `--cd` ends the greedy image span; without an option (or `--`) + // between them, native Codex reads the prompt as a second image. + "--cd", + ".", + "Reply with exactly READY, then stop.", + ], + prompt: "Reply with exactly READY, then stop.", + // The provider overrides are injected before the prompt, so the + // prompt stays the final argument with the last override beside it. + penultimateArg: 'model_provider="codex-multi-auth-runtime-proxy"', + }, + { + // `--` forces the positional, so prompt text that spells a real + // command name must still launch the TUI: native Codex parses + // `codex -- exec` as the prompt "exec", not as the exec subcommand. + label: "a `--`-forced prompt that spells a command name", + args: ["--", "exec"], + prompt: "exec", + // The overrides must land on the option side of the `--`, or Codex + // would read them as prompt text. + penultimateArg: "--", + }, + { + // `-i/--image` is greedy: native Codex consumes every free token up + // to the next option, so the image list must stay contiguous and the + // injected overrides must land after it — an override pair splitting + // the list would demote the second image to the prompt and make the + // real prompt an "unexpected argument" hard error. + label: "a prompt after a greedy multi-image option", + args: ["-i", "a.png", "b.png", "--cd", ".", "look at these"], + prompt: "look at these", + penultimateArg: 'model_provider="codex-multi-auth-runtime-proxy"', + }, + ] as const) { + it(`uses the canonical Codex home for a TUI launch with ${promptForm.label} (#673)`, async () => { + const fixtureRoot = createWrapperFixture(); + createRuntimeRotationProxyFixtureModule(fixtureRoot); + const fakeBin = createCustomFakeCodexBin(fixtureRoot, [ + "#!/usr/bin/env node", + 'const fs = require("node:fs");', + 'const path = require("node:path");', + "const args = process.argv.slice(2);", + 'if (args[0] === "app-server") {', + ' console.log(`APP_SERVER_FORWARDED:${args.join(" ")}`);', + " process.exit(0);", + "}", + "console.log(`PROMPT_HOME_IS_ORIGINAL:${process.env.CODEX_HOME === process.env.ORIGINAL_CODEX_HOME}`);", + // The thread index only exists in the canonical home; the shadow + // mirror omits it, which is what forced the startup rebuild. + 'const statePath = path.join(process.env.CODEX_HOME ?? "", "state_5.sqlite");', + "console.log(`PROMPT_SEES_THREAD_INDEX:${fs.existsSync(statePath)}`);", + `console.log(\`PROMPT_ARG_VERBATIM:\${args.includes(${JSON.stringify( + promptForm.prompt, + )})}\`);`, + "console.log(`PROMPT_LAST_ARG:${args[args.length - 1]}`);", + "console.log(`PROMPT_PENULTIMATE_ARG:${args[args.length - 2]}`);", + 'console.log(`PROMPT_HAS_BASE_URL_OVERRIDE:${args.some((arg) => arg.includes("model_providers.codex-multi-auth-runtime-proxy.base_url="))}`);', + "console.log(`PROMPT_CLI_PATH_IS_SHIM:${(process.env.CODEX_CLI_PATH ?? \"\").includes(\"app-server-shims\")}`);", + "process.exit(0);", + ]); + const originalHome = join(fixtureRoot, "codex-home"); + const markerPath = join(fixtureRoot, "proxy-marker.txt"); + mkdirSync(originalHome, { recursive: true }); + writeFileSync( + join(originalHome, "config.toml"), + 'model_provider = "openai"\n', + "utf8", + ); + writeFileSync( + join(originalHome, "state_5.sqlite"), + "canonical-thread-index\n", + "utf8", + ); + + const result = runWrapper(fixtureRoot, [...promptForm.args], { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + CODEX_HOME: originalHome, + ORIGINAL_CODEX_HOME: originalHome, + CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "1", + CODEX_MULTI_AUTH_APP_ROTATION_IDLE_MS: "200", + CODEX_MULTI_AUTH_TEST_PROXY_MARKER: markerPath, + OPENAI_API_KEY: undefined, + }); + + const output = combinedOutput(result); + if (result.status !== 0) { + throw new Error(output); + } + expect(output).toContain("PROMPT_HOME_IS_ORIGINAL:true"); + expect(output).toContain("PROMPT_SEES_THREAD_INDEX:true"); + expect(output).toContain("PROMPT_ARG_VERBATIM:true"); + expect(output).toContain(`PROMPT_LAST_ARG:${promptForm.prompt}`); + expect(output).toContain( + `PROMPT_PENULTIMATE_ARG:${promptForm.penultimateArg}`, + ); + // Rotation is still active: the proxy overrides ride along as `-c` args. + expect(output).toContain("PROMPT_HAS_BASE_URL_OVERRIDE:true"); + expect(output).toContain("PROMPT_CLI_PATH_IS_SHIM:true"); + // The canonical home is never rewritten on disk. + expect(readFileSync(join(originalHome, "config.toml"), "utf8")).toBe( + 'model_provider = "openai"\n', + ); + await waitForFileText(markerPath, "start:http://127.0.0.1:4567\nclose\n"); + }); + } + + // Native Codex fills the root `[PROMPT]` slot with the first free token and + // still tries a later one as a subcommand: `codex hello exec …` runs exec. + // The classifier scans past unknown positionals the same way, so this stays + // a noninteractive exec on the isolated shadow home (#673). + it("classifies a command after a filled prompt slot like native Codex (#673)", () => { + const fixtureRoot = createWrapperFixture(); + createRuntimeRotationProxyFixtureModule(fixtureRoot); + const fakeBin = createCustomFakeCodexBin(fixtureRoot, [ + "#!/usr/bin/env node", + "console.log(`SHADOW_HOME_IS_ORIGINAL:${process.env.CODEX_HOME === process.env.ORIGINAL_CODEX_HOME}`);", + "process.exit(0);", + ]); + const originalHome = join(fixtureRoot, "codex-home"); + const markerPath = join(fixtureRoot, "proxy-marker.txt"); + mkdirSync(originalHome, { recursive: true }); + writeFileSync( + join(originalHome, "config.toml"), + 'model_provider = "openai"\n', + "utf8", + ); + + const result = runWrapper(fixtureRoot, ["hello", "exec", "echo hi"], { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + CODEX_HOME: originalHome, + ORIGINAL_CODEX_HOME: originalHome, + CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "1", + CODEX_MULTI_AUTH_TEST_PROXY_MARKER: markerPath, + OPENAI_API_KEY: undefined, + }); + + const output = combinedOutput(result); + if (result.status !== 0) { + throw new Error(output); + } + expect(output).toContain("SHADOW_HOME_IS_ORIGINAL:false"); + }); + + // The alias resolves through the same allowlist as its canonical command, so + // `codex e ...` must classify exactly like `codex exec ...` — a real + // noninteractive request on the isolated shadow home, not a prompt (#673). + it("keeps the `e` exec alias on the shadow Codex home (#673)", () => { + const fixtureRoot = createWrapperFixture(); + createRuntimeRotationProxyFixtureModule(fixtureRoot); + const fakeBin = createCustomFakeCodexBin(fixtureRoot, [ + "#!/usr/bin/env node", + "console.log(`SHADOW_HOME_IS_ORIGINAL:${process.env.CODEX_HOME === process.env.ORIGINAL_CODEX_HOME}`);", + "process.exit(0);", + ]); + const originalHome = join(fixtureRoot, "codex-home"); + const markerPath = join(fixtureRoot, "proxy-marker.txt"); + mkdirSync(originalHome, { recursive: true }); + writeFileSync( + join(originalHome, "config.toml"), + 'model_provider = "openai"\n', + "utf8", + ); + + const result = runWrapper(fixtureRoot, ["e", "do something"], { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + CODEX_HOME: originalHome, + ORIGINAL_CODEX_HOME: originalHome, + CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "1", + CODEX_MULTI_AUTH_TEST_PROXY_MARKER: markerPath, + OPENAI_API_KEY: undefined, + }); + + const output = combinedOutput(result); + if (result.status !== 0) { + throw new Error(output); + } + expect(output).toContain("SHADOW_HOME_IS_ORIGINAL:false"); + }); + + // The other alias: `a` resolves to `apply`, which makes no model requests, + // so it must skip the rotation transport entirely — exactly like a spelled- + // out `codex apply` always did, instead of routing as an unknown token (#673). + it("forwards the `a` apply alias without starting the rotation transport (#673)", () => { + const fixtureRoot = createWrapperFixture(); + createRuntimeRotationProxyFixtureModule(fixtureRoot); + const fakeBin = createCustomFakeCodexBin(fixtureRoot, [ + "#!/usr/bin/env node", + 'console.log(`FORWARDED:${process.argv.slice(2).join(" ")}`);', + "console.log(`ALIAS_HOME_IS_ORIGINAL:${process.env.CODEX_HOME === process.env.ORIGINAL_CODEX_HOME}`);", + "process.exit(0);", + ]); + const originalHome = join(fixtureRoot, "codex-home"); + const markerPath = join(fixtureRoot, "proxy-marker.txt"); + mkdirSync(originalHome, { recursive: true }); + writeFileSync( + join(originalHome, "config.toml"), + 'model_provider = "openai"\n', + "utf8", + ); + + const result = runWrapper(fixtureRoot, ["a"], { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + CODEX_HOME: originalHome, + ORIGINAL_CODEX_HOME: originalHome, + CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "1", + CODEX_MULTI_AUTH_TEST_PROXY_MARKER: markerPath, + OPENAI_API_KEY: undefined, + }); + + const output = combinedOutput(result); + if (result.status !== 0) { + throw new Error(output); + } + expect(existsSync(markerPath)).toBe(false); + expect(output).toContain("ALIAS_HOME_IS_ORIGINAL:true"); + expect(output).toContain("FORWARDED:a"); + expect(output).not.toContain("model_provider="); + }); + + // A hidden root subcommand (absent from `codex --help`, real per + // `codex help `) is still a command, not a prompt: it keeps the + // routing fall-through it always had instead of spinning up an interactive + // canonical-home helper that would outlive its short clean exit (#673). + // + // The allowlist is what decides this, so a real root command missing from + // it is misrouted rather than merely misnamed. `stdio-to-uds` is that case: + // it is a genuine hidden root subcommand of codex-cli, and every invocation + // classified as a prompt left a detached helper idling behind it. + for (const args of [ + ["execpolicy", "check"], + ["stdio-to-uds", "/tmp/codex.sock"], + ] as const) { + it(`classifies the hidden \`${args[0]}\` subcommand as a command, not a prompt (#673)`, () => { + const fixtureRoot = createWrapperFixture(); + createRuntimeRotationProxyFixtureModule(fixtureRoot); + const fakeBin = createCustomFakeCodexBin(fixtureRoot, [ + "#!/usr/bin/env node", + "console.log(`SHADOW_HOME_IS_ORIGINAL:${process.env.CODEX_HOME === process.env.ORIGINAL_CODEX_HOME}`);", + "process.exit(0);", + ]); + const originalHome = join(fixtureRoot, "codex-home"); + const markerPath = join(fixtureRoot, "proxy-marker.txt"); + mkdirSync(originalHome, { recursive: true }); + writeFileSync( + join(originalHome, "config.toml"), + 'model_provider = "openai"\n', + "utf8", + ); + + const result = runWrapper(fixtureRoot, [...args], { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + CODEX_HOME: originalHome, + ORIGINAL_CODEX_HOME: originalHome, + CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "1", + CODEX_MULTI_AUTH_TEST_PROXY_MARKER: markerPath, + OPENAI_API_KEY: undefined, + }); + + const output = combinedOutput(result); + if (result.status !== 0) { + throw new Error(output); + } + expect(output).toContain("SHADOW_HOME_IS_ORIGINAL:false"); + }); + } + // `resume`/`fork` are interactive TUI entry points, but they carry a forwarded // subcommand, so they used to fall through to the shadow-home transport. The // shadow mirror omits the runtime SQLite state, so the requested thread was @@ -4402,6 +4753,55 @@ describe("codex bin wrapper", () => { }); } + // The rotation helper injects its own options (shim args plus + // `-c model_provider=...`) into subcommand argv, and appending those past a + // `--` hands them to the subcommand as positionals instead. The separator + // bounds that injection too (#673). + it("injects rotation helper args before a `--` on the resume path (#673)", async () => { + const fixtureRoot = createWrapperFixture(); + createRuntimeRotationProxyFixtureModule(fixtureRoot); + const fakeBin = createCustomFakeCodexBin(fixtureRoot, [ + "#!/usr/bin/env node", + "const args = process.argv.slice(2);", + 'if (args[0] === "app-server") process.exit(0);', + "const sep = args.indexOf(\"--\");", + "console.log(`SEP_INDEX:${sep}`);", + "console.log(`AFTER_SEP:${args.slice(sep + 1).join(\" \")}`);", + 'console.log(`OVERRIDE_BEFORE_SEP:${args.slice(0, sep).some((a) => a.startsWith("model_provider="))}`);', + "console.log(`FIRST_ARG:${args[0]}`);", + "process.exit(0);", + ]); + const originalHome = join(fixtureRoot, "codex-home"); + const markerPath = join(fixtureRoot, "proxy-marker.txt"); + mkdirSync(originalHome, { recursive: true }); + writeFileSync( + join(originalHome, "config.toml"), + 'model_provider = "openai"\n', + "utf8", + ); + + const result = runWrapper(fixtureRoot, ["resume", "--last", "--", "do it"], { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + CODEX_HOME: originalHome, + ORIGINAL_CODEX_HOME: originalHome, + CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY: "1", + CODEX_MULTI_AUTH_APP_ROTATION_IDLE_MS: "200", + CODEX_MULTI_AUTH_TEST_PROXY_MARKER: markerPath, + OPENAI_API_KEY: undefined, + }); + + const output = combinedOutput(result); + if (result.status !== 0) { + throw new Error(output); + } + // The subcommand still leads, the injected overrides sit on the option + // side, and only the user's own text survives past the separator. + expect(output).toContain("FIRST_ARG:resume"); + expect(output).toContain("AFTER_SEP:do it"); + expect(output).toContain("OVERRIDE_BEFORE_SEP:true"); + await waitForFileText(markerPath, "start:http://127.0.0.1:4567\nclose\n"); + }); + // Printing help makes no model requests, so it must not pay for the rotation // transport at all. This matters most for resume/fork now that they are // interactive: that branch detaches its helper on a clean exit, so a helper @@ -5104,6 +5504,47 @@ describe("codex bin wrapper", () => { ).toHaveLength(1); }); + // Everything after `--` is the root prompt, so prompt text that happens to + // spell the override is not a caller override. Scanning past `--` would let + // it suppress the injection and silently drop the file auth store (#673). + it("still injects the file auth store when the prompt text spells the override (#673)", () => { + const fixtureRoot = createWrapperFixture(); + const fakeBin = createFakeCodexBin(fixtureRoot); + // A single token, which is the only runnable shape that tripped the old + // scan: a two-token `-c ` prompt is surplus positionals that + // Codex rejects before any of this matters. + const result = runWrapper( + fixtureRoot, + ["--", '--config=cli_auth_credentials_store="keyring"'], + { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + }, + ); + + expect(result.status).toBe(0); + // The wrapper's own override is injected ahead of the `--`, and the + // prompt text reaches Codex untouched behind it. + expect(result.stdout).toContain( + 'FORWARDED:-c cli_auth_credentials_store="file" -- --config=cli_auth_credentials_store="keyring"', + ); + }); + + // Appending past a `--` puts the injected pair in the subcommand's own + // positional list, which Codex rejects with "unexpected argument '-c'". + // The separator bounds the append for subcommand argv too (#673). + it("injects the file auth store before a subcommand's `--` separator (#673)", () => { + const fixtureRoot = createWrapperFixture(); + const fakeBin = createFakeCodexBin(fixtureRoot); + const result = runWrapper(fixtureRoot, ["exec", "--", "do the thing"], { + CODEX_MULTI_AUTH_REAL_CODEX_BIN: fakeBin, + }); + + expect(result.status).toBe(0); + expect(result.stdout).toContain( + 'FORWARDED:exec -c cli_auth_credentials_store="file" -- do the thing', + ); + }); + it("propagates downstream file-store write errors from forwarded wrapper execution", () => { const fixtureRoot = createWrapperFixture(); const fakeBin = createCustomFakeCodexBin(fixtureRoot, [