fix(codex): route app-server through the canonical home (#659) - #660
fix(codex): route app-server through the canonical home (#659)#660possibilities wants to merge 5 commits into
Conversation
`codex-multi-auth-codex app-server` fell through to the shadow-home transport, and a resident server cannot live there. It usually cannot even start. Codex refuses when `<CODEX_HOME>/app-server-control` exists and is not a directory, and the shadow mirror symlinks that directory like any other, so on any home that has already run an app-server the child exits immediately with `socket directory path exists and is not a directory`. A server that does start is worse off than one that fails: the mirror snapshots the runtime SQLite state rather than linking it, so every attached client drives threads against a frozen, throwaway index — the same divergence that hung `resume` on a blank screen (ndycode#647), except an app-server holds it for the whole life of the process and hands it to every client that attaches. Classify `app-server` with the predicate that already exists for it and route it to the canonical-home app helper, alongside the interactive entry points. Two details differ from that branch: - `detachOnExit: false`. A resident server owns its proxy for its whole lifetime, so the helper stops when the server does. Leaving it to idle out would strand one proxy helper per supervised restart. - `proxyAppServerAccountRead` is threaded through the helper context and set for this branch, so moving the transport does not silently drop the `account/read` / `getAuthStatus` / `account/rateLimits/read` rewriting that stdio clients already rely on. Only the shadow branch used to set it. Verified end to end against codex-cli 0.147.0 with two managed accounts: the pinned account governs the server's model calls, proved by reading each turn's rollout `rate_limits` back against the accounts' distinct quota windows, and rollouts land in the canonical `sessions` directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0134wwvqFQA87kf1K2cLeEk6
`docs/reference/commands.md` and `docs/configuration.md` both stated that `codex app-server` runs against a shadow `CODEX_HOME`. It now stays on the canonical home like the interactive TUI, and for app-server that placement is required rather than an optimization, so both pages say why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0134wwvqFQA87kf1K2cLeEk6
Review catch. `detachOnExit: false` was not actually honored for a short-lived server: the grace clause detaches on any clean exit inside the window, so an app-server that a client attaches to, queries once, and disconnects from within five seconds still left its helper and proxy running for the full 12h idle timeout — with the owner gone and nothing refreshing the activity clock. That is precisely the stranding the branch claims to prevent. The window exists for callers that hand off and return immediately; `codex app` is the real one. So it now applies only when the caller has not decided for itself: `detachOnExit === true` detaches past the window, an unset `detachOnExit` still detaches inside it, and an explicit `false` never detaches. TUI passes true and `codex app` passes neither, so both keep their behavior exactly. Both app-server tests drop the shortened idle timeout they needed to see the proxy close, which turns each of them into a regression test for this: they fail against the old clause and pass against the new one. Also restores the `cli_auth_credentials_store="file"` assertion the earlier test rewrite dropped for app-server, and corrects docs/architecture.md, which still described the canonical home as interactive-only and labelled the sole runtime path as shadow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0134wwvqFQA87kf1K2cLeEk6
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
📝 Walkthroughseverity: minor. the change routes reviewers should focus on the architectural choice to set
Walkthroughapp-server runtime rotation now uses canonical Changesapp-server runtime rotation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AppServer
participant RuntimeAppHelper
participant CanonicalCodexHome
Client->>AppServer: connect to the requested listen address
AppServer->>RuntimeAppHelper: start with provider overrides
RuntimeAppHelper->>CanonicalCodexHome: read and update canonical thread state
RuntimeAppHelper-->>AppServer: proxy provider and account-read traffic
AppServer-->>Client: serve sessions and threads
AppServer->>RuntimeAppHelper: signal resident-server exit
RuntimeAppHelper->>RuntimeAppHelper: terminate asynchronously
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/configuration.md`:
- Line 143: The app-server routing documentation must apply only to resident
app-server runs and explicitly note that app-server --help, help, generate-ts,
and generate-json-schema bypass the proxy. Update the paragraph in
docs/configuration.md lines 143-143 and the corresponding command-reference text
in docs/reference/commands.md lines 550-550; qualify both the proxy and -c
override statements consistently, with no direct changes required elsewhere.
In `@docs/development/ARCHITECTURE.md`:
- Line 172: Update the architecture documentation’s transport count from three
to four and revise the system diagram to include the canonical-home app-server
branch identified by isCodexAppServerCommand, matching the routing contract in
scripts/codex.js. Ensure the diagram shows app-server following its resident
proxy ownership path rather than the legacy shadow-home path.
- Around line 187-188: Extend test/codex-bin-wrapper.test.ts with regression
coverage for app-server lifecycle: verify the app-server helper shuts down after
the server exits, and add Windows-specific coverage that exercises cleanup retry
behavior during the app-server process-exit flow. Reuse the existing canonical
CODEX_HOME, concurrent helper ownership, stale stdio shutdown, and shadow
cleanup test patterns where applicable, while asserting app-server-specific
process exit and cleanup behavior.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4eaa7314-7c42-4d7a-8108-713db27a2612
📒 Files selected for processing (6)
docs/architecture.mddocs/configuration.mddocs/development/ARCHITECTURE.mddocs/reference/commands.mdscripts/codex.jstest/codex-bin-wrapper.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (17)
docs/**/*.md
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
docs/**/*.md: User-facing documentation should follow the page template: Title and one-line lead, Quick path commands, Core operational workflow, Troubleshooting or failure handling, and Related links
Use short sections and scan-friendly tables in documentation where they improve clarity
Prefer direct, actionable language in documentation
Use runnable command examples in documentation
Explain expected outcomes after critical commands in documentation
Keep terminology consistent with runtime names in documentation
Avoid speculative language when behavior is deterministic in documentation
Put the user problem in the first paragraph before implementation detail
Use descriptive page titles such ascodex-multi-auth Featuresinstead of generic titles on public docs
Do not repeat keyword lists in every section; search terms should appear only where they help a developer understand the page
Canonical command family iscodex-multi-auth ...
Canonical runtime root is~/.codex/multi-auth
Runtime rotation must be described as default-on unless the release policy changes
Legacy command/path references belong only in migration contexts in documentation
Compatibility aliases (codex multi auth,codex multi-auth,codex multiauth) belong only in command reference, troubleshooting, or migration contexts
Keep command flags aligned with runtime usage text in documentation
Avoid non-runnable command snippets in documentation
Avoid conflicting path guidance across documentation
Avoid legacy-first onboarding language in documentationOrganize repository documentation according to the defined layers: product entry, user operations, reference, and development.
docs/**/*.md: Do not describecodex-multi-authas replacing@openai/codexor publishing the globalcodexbinary; preserve the official CLI's ownership ofcodex.
Usecodex-multi-authfor account management, and reservecodex-multi-auth-codexormcodexfor intentionally forwarding official Codex commands th...
Files:
docs/configuration.mddocs/architecture.mddocs/development/ARCHITECTURE.mddocs/reference/commands.md
docs/{index.md,getting-started.md,faq.md,architecture.md,features.md,configuration.md,troubleshooting.md,privacy.md,upgrade.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Keep the listed public documentation pages as the canonical sources for operator onboarding, FAQ, architecture, features, configuration, troubleshooting, privacy, and upgrades.
Files:
docs/configuration.mddocs/architecture.md
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/troubleshooting.md)
Document that
codex-multi-auth-codexis the optional forwarding wrapper, whilecodex-multi-authis the canonical account-manager command family; the package does not publish a globalcodexbinary.Document the canonical command names, runtime paths, configuration precedence, storage migration behavior, and upgrade procedures consistently across the referenced documentation.
Files:
docs/configuration.mddocs/architecture.mddocs/development/ARCHITECTURE.mddocs/reference/commands.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Source changes belong in
index.ts,lib/, andscripts/;dist/is generated output and local temporary/cache directories must not be edited.
Files:
docs/configuration.mddocs/architecture.mdtest/codex-bin-wrapper.test.tsdocs/development/ARCHITECTURE.mddocs/reference/commands.mdscripts/codex.js
docs/**
⚙️ CodeRabbit configuration file
keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.
Files:
docs/configuration.mddocs/architecture.mddocs/development/ARCHITECTURE.mddocs/reference/commands.md
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Write Vitest test suites with globals enabled (describe, it, expect)
Maintain 80%+ coverage threshold across statements, branches, functions, and lines
Use removeWithRetry() for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY, EPERM, and ENOTEMPTY errors
Do not rely on dist/ in tests; use source files instead
Do not skip tests without justification
Relax lint rules for test files as configured in eslint.config.js
Files:
test/codex-bin-wrapper.test.ts
test/**/codex-bin-wrapper.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Test bin wrapper lazy-load and missing dist handling with concurrent invocations in codex-bin-wrapper.test.ts
Files:
test/codex-bin-wrapper.test.ts
**/*.{ts,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,js,mjs}: Use ESM modules throughout the project; the package is configured with"type": "module".
Do not useas any,@ts-ignore, or@ts-expect-error.
Files:
test/codex-bin-wrapper.test.tsscripts/codex.js
test/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Windows-sensitive filesystem tests and helpers must use retry handling for transient lock-related cleanup and write failures.
Files:
test/codex-bin-wrapper.test.ts
**/*.{js,ts,mjs,cjs}
📄 CodeRabbit inference engine (README.md)
**/*.{js,ts,mjs,cjs}: Do not publish or replace a globalcodexbinary; official OpenAI installation paths must retain ownership of thecodexcommand.
Keep OAuth credentials local and restrict runtime rotation and local bridges to loopback interfaces.
Require hashed local client tokens to protect the optional loopback bridge.
Responsesbackground: truecompatibility must remain opt-in; requests using it must use statefulstore=truerouting rather than statelessstore=falserouting.
Never run npm install or update commands automatically; only display a manual upgrade notice when appropriate.
Experimental synchronization and backup flows must be non-destructive by default: preview before applying sync, preserve destination-only accounts, and fail safely on backup filename collisions.
Keep account storage project-scoped under the configured multi-auth root when operating in repo-specific workflows.
Files:
test/codex-bin-wrapper.test.tsscripts/codex.js
test/**
⚙️ CodeRabbit configuration file
tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.
Files:
test/codex-bin-wrapper.test.ts
docs/development/**/*.md
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Keep internal architecture, configuration flow, repository ownership, testing, parity, metadata, and audit guidance in development documentation.
Prefer current architecture and reference documentation over historical plans and audit snapshots when describing the present system.
Files:
docs/development/ARCHITECTURE.md
docs/development/**/*
📄 CodeRabbit inference engine (docs/development/CONFIG_FLOW.md)
docs/development/**/*: Resolve the runtime root directory in this order:CODEX_MULTI_AUTH_DIR; explicit non-defaultCODEX_HOME/multi-auth; existing account-storage roots underCODEX_HOMEor~/.codex; canonical~/.codex/multi-auth; and legacy paths only when storage signals exist.
ReaddashboardDisplaySettingsandpluginConfigfromsettings.json, while preserving legacy compatibility loading and migration.
ResolvepluginConfigvalues using this precedence: existingCODEX_MULTI_AUTH_CONFIG_PATHfile, valid unifiedsettings.jsonconfiguration, legacy compatibility configuration, thenDEFAULT_PLUGIN_CONFIG; apply environment-variable overrides afterward.
Ignore a configured but nonexistentCODEX_MULTI_AUTH_CONFIG_PATHduring loading, but create it on the first save while the variable remains set.
Resolve dashboard display values from persisteddashboardDisplaySettings, followed by normalization and fallback defaults.
Resolve account storage by selecting the root directory, using the global accounts file by default, using a project-namespaced path when project-scoped mode is active, and attempting applicable legacy project-file migration.
Normalize standalonecodex-multi-authbare subcommands toauth ...before dispatch; normalize wrapper aliases; run auth-manager commands locally; forward out-of-scope wrapper commands to the official Codex CLI.
For forwarded request-bearing commands, honor runtime rotation: resolveCODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY, thenpluginConfig.codexRuntimeRotationProxy, which defaults to enabled.
When rotation is enabled for a requesting command, use a per-process-token loopback Responses proxy, a temporary shadowCODEX_HOME, and a rewrittenconfig.toml; synchronize refreshed official Codex state on exit and remove the shadow home.
The runtime proxy must select or refresh managed accounts and rotate on rate-limit, authentication, network, or server failures before streaming begins.
The plugin host m...
Files:
docs/development/ARCHITECTURE.md
docs/development/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/development/TESTING.md)
When documentation changes, verify every command snippet is runnable, path references match runtime modules, cross-links are valid, and the feature matrix matches implemented features.
Files:
docs/development/ARCHITECTURE.md
docs/reference/**/*.md
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
New flags/settings/paths must be reflected in
docs/reference/*
docs/reference/**/*.md: Keep command, API, error-contract, settings, and storage-path details in the canonical reference documentation.
Document compatibility aliases (codex multi auth,codex multi-auth, andcodex multiauth) only in command-reference, troubleshooting, or migration sections.
Files:
docs/reference/commands.md
scripts/codex*.js
📄 CodeRabbit inference engine (AGENTS.md)
The wrapper must not reimplement general Codex commands; authentication commands are handled locally and non-authentication commands must forward to the official Codex CLI.
Files:
scripts/codex.js
scripts/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Windows-sensitive cleanup and write operations must retry transient
EBUSY,EPERM, andENOTEMPTYfailures where applicable.
Files:
scripts/codex.js
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: Ignore a set-but-missing `CODEX_MULTI_AUTH_CONFIG_PATH` during load, but use it as the save target when the variable is set.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: When `CODEX_HOME` is non-default, resolve multi-auth strictly under `$CODEX_HOME/multi-auth` without scanning other roots for existing pools.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: `CODEX_MULTI_AUTH_DIR` must re-home all multi-auth-owned settings, accounts, cache, and log directories.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: A per-invocation account selected by `--account` or `CODEX_MULTI_AUTH_FORCE_ACCOUNT` must be ephemeral, fail hard when the rotation proxy is disabled, and never modify or leak the persisted account-switch pin.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: The runtime rotation proxy must preserve request bodies and streaming responses, replace outbound authentication headers, and remove hop-by-hop, private account metadata, and stale decoded `content-encoding` headers from responses.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: When all accounts are unavailable, return a structured pool-exhaustion error that directs users to `codex-multi-auth rotation status` rather than silently failing over indefinitely.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: Explicit OAuth token revocation responses must be returned directly instead of rotating accounts; the affected account must receive the configured token-invalidation cooldown.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: Package installation scripts must remain side-effect-free; postinstall may print only a short notice and must not modify user configuration or install updates.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: First-run self-healing must run only on durable global installs, not `npx` or project-local installs, and must record completion at `~/.codex/multi-auth/first-run-setup.json`.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: Automatic npm version checks may only print a manual update notice; wrappers must never execute npm install or update commands, and notices should be limited to a TTY or debug mode.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: The official Codex CLI credential store must be persisted as `cli_auth_credentials_store = "file"` unless `CODEX_MULTI_AUTH_ENFORCE_CLI_FILE_AUTH_STORE=0` opts out.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:57:49.777Z
Learning: Deprecated selectors such as `gpt-5-codex` and `gpt-5.1-codex*` must be treated as compatibility aliases and retried with the current documented Codex model only after the Codex surface returns a genuine unsupported-model response.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:58:02.778Z
Learning: The runtime rotation proxy is default-on, local, reversible, and loopback-only; explicit opt-out controls must remain available.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:58:02.778Z
Learning: Budget guards are soft under concurrency: evaluations use a pre-request ledger snapshot, and brief concurrent overshoot is intentional rather than treated as a hard distributed quota.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:58:02.778Z
Learning: The canonical account-management command surface remains `codex-multi-auth ...`, while the Codex wrapper preserves official CLI behavior for unrelated commands.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T16:58:11.134Z
Learning: Treat documented public API and command behavior as compatibility contracts; additive JSON surfaces must not invalidate the documented anchor list.
📚 Learning: 2026-06-04T06:14:18.093Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/scheduling-strategy-config.test.ts:1-1
Timestamp: 2026-06-04T06:14:18.093Z
Learning: In ndycode/codex-multi-auth, do not flag explicit imports from "vitest" (e.g., describe, it, expect, beforeEach/afterEach, etc.) in test files as issues—even if the Vitest config sets `globals: true`. The repo’s established convention is to keep these imports for consistency with neighboring tests; removing them would make files outliers.
Applied to files:
test/codex-bin-wrapper.test.ts
📚 Learning: 2026-06-04T06:14:24.975Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/runtime-rotation-proxy.test.ts:2478-2491
Timestamp: 2026-06-04T06:14:24.975Z
Learning: In ndycode/codex-multi-auth test files (e.g. `test/*.test.ts`), when creating V3 storage fixtures for accounts, it’s an intentional convention to use `as never` for deliberately minimal stored-account objects that only include `refreshToken`, `addedAt`, and `lastUsed`. Do not treat `as never` here as a type-safety problem: optional/other fields are expected to be populated by the runtime during execution, and the cast is used solely to keep the fixture minimal and consistent across existing tests.
Applied to files:
test/codex-bin-wrapper.test.ts
🪛 ast-grep (0.45.0)
test/codex-bin-wrapper.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (4)
scripts/codex.js (1)
4289-4299: LGTM!Also applies to: 4334-4334, 4365-4374, 4574-4581
test/codex-bin-wrapper.test.ts (1)
2149-2149: LGTM!Also applies to: 2182-2209, 3170-3241
docs/architecture.md (1)
30-30: LGTM!Also applies to: 63-64, 205-206
docs/reference/commands.md (1)
550-550: 📐 Maintainability & Code Qualityverify the required upgrade note.
This change moves
codex app-serverfrom the shadow home to the canonicalCODEX_HOME. Checkdocs/upgrade.mdand add the migration note. Mention any new npm scripts, or state that none were added.As per path instructions, behavior changes require updated upgrade notes and mention of new npm scripts.
Source: Path instructions
| | 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 `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. | | ||
| | `codex app` | `isCodexAppCommand` — forwarded command is `app` | App runtime helper process with a shadow `CODEX_HOME`. | | ||
| | `app-server` | `isCodexAppServerCommand` — forwarded command is `app-server` | Canonical-home app helper, but `detachOnExit: false`: a resident server owns its proxy for its whole lifetime. `account/read` rewriting stays on. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
update the transport count and system diagram.
This branch adds a fourth runtime transport, but line 165 still says “one of three transports,” and the system diagram omits app-server. Readers can follow the old shadow-home path for this command. Add the canonical-home app-server branch to the diagram and change the count to four. scripts/codex.js:4574-4660 is the routing contract.
As per path instructions, development documentation must prefer the current architecture and verify feature matrices against implemented features.
🤖 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 `@docs/development/ARCHITECTURE.md` at line 172, Update the architecture
documentation’s transport count from three to four and revise the system diagram
to include the canonical-home app-server branch identified by
isCodexAppServerCommand, matching the routing contract in scripts/codex.js.
Ensure the diagram shows app-server following its resident proxy ownership path
rather than the legacy shadow-home path.
Source: Path instructions
There was a problem hiding this comment.
Keeping three, and flagging this as the one open disagreement on the PR so a human can settle it.
The count was already mechanism-based before this branch: the same sentence said "one of three transports" over four branch rows — TUI, resume/fork, app, everything else. So rows and transports were never 1:1 here, and reading a new row as a new transport contradicts the file's own established usage.
app-server reuses the canonical-home app helper the interactive commands already use. detachOnExit is lifecycle and proxyAppServerAccountRead is a wrapper-side protocol rewrite; neither changes home placement nor how traffic reaches the proxy. The three mechanisms are unchanged: canonical-home helper, shadow-home helper for codex app, inline shadow home for everything else.
What was genuinely wrong is that the convention was implicit, so the sentence now states it ("more branches than transports"), and the diagram — which was stale in a way that predates this PR, missing resume/fork since #648 — now lists every branch under the transport it takes, including app-server and the no-transport help path.
@ndycode if you count rows rather than mechanisms in this document, say so and I'll change it to four in one commit.
Review round 2. The app-server tests waited for the proxy's `close` marker to prove the helper stopped rather than detached. That only works where the helper gets to run its SIGTERM handler: on Windows the terminate is unconditional, so the marker never arrives even though cleanup did exactly what it should, and the assertion would hang for five seconds before failing. Both tests now read the helper's pid from the marker and poll until the process is gone, which is true on every platform. They still fail against the previous detach-grace clause, so the regression they were added for is intact. Docs: the runtime-rotation transport diagram listed three branches and had been missing `resume`/`fork` since they moved to the canonical home; it now lists every branch under the transport it takes, including app-server, and records that help and schema generation take no transport at all. `docs/reference/commands.md` and `docs/configuration.md` say the same, so neither reads as though `app-server --help` starts a proxy — while still noting that the transport skip does not stop the transport-independent `cli_auth_credentials_store` reconcile, which `buildForwardArgs` appends on every path. The prose still says three transports, because that count is right: the interactive commands and `app-server` share the canonical-home helper and differ only in options. The document already counted three over four branch rows before this change, so the convention was mechanism-based already; the sentence now says so instead of leaving the reader to infer it from a table that has grown a row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0134wwvqFQA87kf1K2cLeEk6
|
Thanks — four findings, three fixed and one declined with reasoning. Pushed as ✅ greptile P1 —
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/configuration.md`:
- Line 143: Add an upgrade note to docs/upgrade.md documenting the app-server
routing behavior implemented near the codex.js app-server handling: it remains
on the canonical CODEX_HOME while receiving the runtime proxy provider through
-c overrides, unlike surfaces that may use a shadow home. Do not add npm
scripts.
In `@docs/reference/commands.md`:
- Line 550: Update the command reference paragraph describing transport bypasses
to include app-server --help and app-server -h alongside app-server help,
generate-ts, and generate-json-schema. Keep the existing explanation and
behavior unchanged while documenting both flag forms.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: df9c341a-a7b5-4bf0-a50b-133bbd47b759
📒 Files selected for processing (4)
docs/configuration.mddocs/development/ARCHITECTURE.mddocs/reference/commands.mdtest/codex-bin-wrapper.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (15)
docs/**/*.md
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
docs/**/*.md: User-facing documentation should follow the page template: Title and one-line lead, Quick path commands, Core operational workflow, Troubleshooting or failure handling, and Related links
Use short sections and scan-friendly tables in documentation where they improve clarity
Prefer direct, actionable language in documentation
Use runnable command examples in documentation
Explain expected outcomes after critical commands in documentation
Keep terminology consistent with runtime names in documentation
Avoid speculative language when behavior is deterministic in documentation
Put the user problem in the first paragraph before implementation detail
Use descriptive page titles such ascodex-multi-auth Featuresinstead of generic titles on public docs
Do not repeat keyword lists in every section; search terms should appear only where they help a developer understand the page
Canonical command family iscodex-multi-auth ...
Canonical runtime root is~/.codex/multi-auth
Runtime rotation must be described as default-on unless the release policy changes
Legacy command/path references belong only in migration contexts in documentation
Compatibility aliases (codex multi auth,codex multi-auth,codex multiauth) belong only in command reference, troubleshooting, or migration contexts
Keep command flags aligned with runtime usage text in documentation
Avoid non-runnable command snippets in documentation
Avoid conflicting path guidance across documentation
Avoid legacy-first onboarding language in documentationOrganize repository documentation according to the defined layers: product entry, user operations, reference, and development.
docs/**/*.md: Do not describecodex-multi-authas replacing@openai/codexor publishing the globalcodexbinary; preserve the official CLI's ownership ofcodex.
Usecodex-multi-authfor account management, and reservecodex-multi-auth-codexormcodexfor intentionally forwarding official Codex commands th...
Files:
docs/reference/commands.mddocs/configuration.mddocs/development/ARCHITECTURE.md
docs/reference/**/*.md
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
New flags/settings/paths must be reflected in
docs/reference/*
docs/reference/**/*.md: Keep command, API, error-contract, settings, and storage-path details in the canonical reference documentation.
Document compatibility aliases (codex multi auth,codex multi-auth, andcodex multiauth) only in command-reference, troubleshooting, or migration sections.
Files:
docs/reference/commands.md
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/troubleshooting.md)
Document that
codex-multi-auth-codexis the optional forwarding wrapper, whilecodex-multi-authis the canonical account-manager command family; the package does not publish a globalcodexbinary.Document the canonical command names, runtime paths, configuration precedence, storage migration behavior, and upgrade procedures consistently across the referenced documentation.
Files:
docs/reference/commands.mddocs/configuration.mddocs/development/ARCHITECTURE.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Source changes belong in
index.ts,lib/, andscripts/;dist/is generated output and local temporary/cache directories must not be edited.
Files:
docs/reference/commands.mddocs/configuration.mddocs/development/ARCHITECTURE.mdtest/codex-bin-wrapper.test.ts
docs/**
⚙️ CodeRabbit configuration file
keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.
Files:
docs/reference/commands.mddocs/configuration.mddocs/development/ARCHITECTURE.md
docs/{index.md,getting-started.md,faq.md,architecture.md,features.md,configuration.md,troubleshooting.md,privacy.md,upgrade.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Keep the listed public documentation pages as the canonical sources for operator onboarding, FAQ, architecture, features, configuration, troubleshooting, privacy, and upgrades.
Files:
docs/configuration.md
docs/development/**/*.md
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Keep internal architecture, configuration flow, repository ownership, testing, parity, metadata, and audit guidance in development documentation.
Prefer current architecture and reference documentation over historical plans and audit snapshots when describing the present system.
Files:
docs/development/ARCHITECTURE.md
docs/development/**/*
📄 CodeRabbit inference engine (docs/development/CONFIG_FLOW.md)
docs/development/**/*: Resolve the runtime root directory in this order:CODEX_MULTI_AUTH_DIR; explicit non-defaultCODEX_HOME/multi-auth; existing account-storage roots underCODEX_HOMEor~/.codex; canonical~/.codex/multi-auth; and legacy paths only when storage signals exist.
ReaddashboardDisplaySettingsandpluginConfigfromsettings.json, while preserving legacy compatibility loading and migration.
ResolvepluginConfigvalues using this precedence: existingCODEX_MULTI_AUTH_CONFIG_PATHfile, valid unifiedsettings.jsonconfiguration, legacy compatibility configuration, thenDEFAULT_PLUGIN_CONFIG; apply environment-variable overrides afterward.
Ignore a configured but nonexistentCODEX_MULTI_AUTH_CONFIG_PATHduring loading, but create it on the first save while the variable remains set.
Resolve dashboard display values from persisteddashboardDisplaySettings, followed by normalization and fallback defaults.
Resolve account storage by selecting the root directory, using the global accounts file by default, using a project-namespaced path when project-scoped mode is active, and attempting applicable legacy project-file migration.
Normalize standalonecodex-multi-authbare subcommands toauth ...before dispatch; normalize wrapper aliases; run auth-manager commands locally; forward out-of-scope wrapper commands to the official Codex CLI.
For forwarded request-bearing commands, honor runtime rotation: resolveCODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY, thenpluginConfig.codexRuntimeRotationProxy, which defaults to enabled.
When rotation is enabled for a requesting command, use a per-process-token loopback Responses proxy, a temporary shadowCODEX_HOME, and a rewrittenconfig.toml; synchronize refreshed official Codex state on exit and remove the shadow home.
The runtime proxy must select or refresh managed accounts and rotate on rate-limit, authentication, network, or server failures before streaming begins.
The plugin host m...
Files:
docs/development/ARCHITECTURE.md
docs/development/**/*.{md,mdx}
📄 CodeRabbit inference engine (docs/development/TESTING.md)
When documentation changes, verify every command snippet is runnable, path references match runtime modules, cross-links are valid, and the feature matrix matches implemented features.
Files:
docs/development/ARCHITECTURE.md
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Write Vitest test suites with globals enabled (describe, it, expect)
Maintain 80%+ coverage threshold across statements, branches, functions, and lines
Use removeWithRetry() for Windows filesystem cleanup instead of bare fs.rm to handle EBUSY, EPERM, and ENOTEMPTY errors
Do not rely on dist/ in tests; use source files instead
Do not skip tests without justification
Relax lint rules for test files as configured in eslint.config.js
Files:
test/codex-bin-wrapper.test.ts
test/**/codex-bin-wrapper.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Test bin wrapper lazy-load and missing dist handling with concurrent invocations in codex-bin-wrapper.test.ts
Files:
test/codex-bin-wrapper.test.ts
**/*.{ts,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,js,mjs}: Use ESM modules throughout the project; the package is configured with"type": "module".
Do not useas any,@ts-ignore, or@ts-expect-error.
Files:
test/codex-bin-wrapper.test.ts
test/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Windows-sensitive filesystem tests and helpers must use retry handling for transient lock-related cleanup and write failures.
Files:
test/codex-bin-wrapper.test.ts
**/*.{js,ts,mjs,cjs}
📄 CodeRabbit inference engine (README.md)
**/*.{js,ts,mjs,cjs}: Do not publish or replace a globalcodexbinary; official OpenAI installation paths must retain ownership of thecodexcommand.
Keep OAuth credentials local and restrict runtime rotation and local bridges to loopback interfaces.
Require hashed local client tokens to protect the optional loopback bridge.
Responsesbackground: truecompatibility must remain opt-in; requests using it must use statefulstore=truerouting rather than statelessstore=falserouting.
Never run npm install or update commands automatically; only display a manual upgrade notice when appropriate.
Experimental synchronization and backup flows must be non-destructive by default: preview before applying sync, preserve destination-only accounts, and fail safely on backup filename collisions.
Keep account storage project-scoped under the configured multi-auth root when operating in repo-specific workflows.
Files:
test/codex-bin-wrapper.test.ts
test/**
⚙️ CodeRabbit configuration file
tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.
Files:
test/codex-bin-wrapper.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:00.701Z
Learning: The canonical command family is `codex-multi-auth ...`; the package must not publish a global `codex` binary.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:00.701Z
Learning: The desktop app bind must be reversible and must not patch official app files.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:00.701Z
Learning: Default general model routing uses `gpt-5.5`; diagnostic live/quota probes lead with `gpt-5.6-sol`.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Resolve runtime configuration sources in this order: existing `CODEX_MULTI_AUTH_CONFIG_PATH`, valid unified `settings.json` `pluginConfig`, legacy compatibility files, then `DEFAULT_PLUGIN_CONFIG`; apply environment-variable overrides afterward.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Ignore a set-but-missing `CODEX_MULTI_AUTH_CONFIG_PATH` during loading, while continuing to use it as the save target when set.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Honor `CODEX_MULTI_AUTH_DIR` for all multi-auth-owned runtime files; when `CODEX_HOME` is non-default, resolve strictly under `$CODEX_HOME/multi-auth` without scanning other roots.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Keep dashboard display values separate from runtime `pluginConfig`; resolve dashboard values from persisted `dashboardDisplaySettings` followed by normalized defaults.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: A per-invocation `--account` or `CODEX_MULTI_AUTH_FORCE_ACCOUNT` pin must be ephemeral, take precedence over the environment selector, require the runtime rotation proxy, and fail hard when the proxy is disabled.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: The runtime rotation proxy must preserve request bodies and streaming responses, replace outbound authorization with the selected managed account, rotate before streaming on eligible failures, and remove hop-by-hop, private account metadata, and stale decoded content-encoding headers.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: On explicit OAuth token revocation, return the error directly instead of rotating accounts and apply the configured token-invalidation cooldown.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Do not start the runtime proxy or inject provider overrides for `--help`/`-h` and `app-server help`, `generate-ts`, or `generate-json-schema`; the CLI auth-store reconciliation still applies.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: In sequential scheduling, retain the active account until it is exhausted, honor manual pins, and intentionally ignore per-session affinity after an account change.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Keep retry and wait budgets bounded so all-account rate-limit waits cannot exceed the host client's request timeout.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Package install scripts must remain side-effect-free; postinstall may only print a short notice.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Do not patch official Codex app files; use the supported persistent localhost router and restore the backed-up Codex configuration when rotation is disabled or unbound.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: The wrapper must never run npm install or update commands automatically; version checks may only print a manual installation notice.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:14.968Z
Learning: Deprecated selectors such as `gpt-5-codex` and `gpt-5.1-codex*` must be treated as compatibility aliases and retried only after a real unsupported-model response; fallback to `gpt-5.4` only after that response.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-10T17:32:40.515Z
Learning: Compatibility command forms and documented exit-code, JSON, CSV, and flag behavior should remain stable because they constitute the CLI command contract.
📚 Learning: 2026-06-04T06:14:18.093Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/scheduling-strategy-config.test.ts:1-1
Timestamp: 2026-06-04T06:14:18.093Z
Learning: In ndycode/codex-multi-auth, do not flag explicit imports from "vitest" (e.g., describe, it, expect, beforeEach/afterEach, etc.) in test files as issues—even if the Vitest config sets `globals: true`. The repo’s established convention is to keep these imports for consistency with neighboring tests; removing them would make files outliers.
Applied to files:
test/codex-bin-wrapper.test.ts
📚 Learning: 2026-06-04T06:14:24.975Z
Learnt from: ndycode
Repo: ndycode/codex-multi-auth PR: 510
File: test/runtime-rotation-proxy.test.ts:2478-2491
Timestamp: 2026-06-04T06:14:24.975Z
Learning: In ndycode/codex-multi-auth test files (e.g. `test/*.test.ts`), when creating V3 storage fixtures for accounts, it’s an intentional convention to use `as never` for deliberately minimal stored-account objects that only include `refreshToken`, `addedAt`, and `lastUsed`. Do not treat `as never` here as a type-safety problem: optional/other fields are expected to be populated by the runtime during execution, and the cast is used solely to keep the fixture minimal and consistent across existing tests.
Applied to files:
test/codex-bin-wrapper.test.ts
🪛 ast-grep (0.45.0)
test/codex-bin-wrapper.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type SpawnSyncReturns, spawn, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (2)
test/codex-bin-wrapper.test.ts (1)
2177-2177: LGTM!Also applies to: 2210-2224, 3235-3235, 3255-3269
docs/development/ARCHITECTURE.md (1)
58-63: LGTM!Also applies to: 72-75, 171-178, 193-194
Review round 3, both docs-only. The help exception named `app-server help` / `generate-ts` / `generate-json-schema` but scoped the flag forms to request commands. `app-server` is not one of those: the wrapper checks `hasHelpFlagAfterCommand` inside its own branch, so `app-server --help` and `app-server -h` skip the transport too and the reference did not say so. `docs/upgrade.md` gains a bullet under the existing runtime-rotation note. Nothing migrates, which is why this is a bullet rather than a section — that file is topic-scoped, and an app-server transport change is a runtime-rotation topic — but the observable behavior does change for anyone launching app-servers through the wrapper: no shadow home appears for the command, and the proxy moves from running inline in the wrapper to the same detached helper the interactive TUI uses, which stops when the server exits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0134wwvqFQA87kf1K2cLeEk6
|
Both accepted — pushed as ✅ document the app-server help flag bypassCorrect, and my wording did exclude them.
✅ add the app-server routing note to docs/upgrade.mdFair, and I had ticked that box as n/a on the wrong reasoning — I read the file as migration-only, but it is topic-scoped, and this is squarely a runtime-rotation topic. Added as a bullet under the existing Runtime Rotation Upgrade Note rather than a new section, since nothing migrates. Worth noting one correction I made to my own bullet before pushing: I had written that the helper "stops when the server exits rather than idling on," which implies a baseline that never existed — on 2.8.3
|
Replaces the placeholder created by the version bump, now that 2.8.4 is published to npm. Covers both #659 reproductions from the server's point of view (the lstat-strict app-server-control check and the frozen thread index), the three app-helper behaviours that were wrong for a resident server, and the operator-visible consequences: no shadow home, a helper stopped with the server rather than idled out, and a proxy failure that now fails the server instead of running it unrotated. Credits @possibilities for the report and the original patch in #660. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KT5pNBtZ151FSC6KBA32sk
|
Superseded by #662, which is merged ( Verified rather than assumed — this branch is strictly behind
GitHub already marks it |
|
Thank you for this — the report in #659 was one of the best I've received. A reproduction that isolates the trigger ( The fix shipped in v2.8.4 — https://github.com/ndycode/codex-multi-auth/releases/tag/v2.8.4 — via #662, which builds on your branch. You are credited in the release notes and the changelog. I'm closing this one as superseded rather than merging it, because the follow-up review turned up something neither of us had accounted for. What changed from your patchThe app-server CLI shim comes along with the transport.
That first row is the one that matters: it's precisely the silent billing error your Risk section argued hard-fail was worth having. So v2.8.4 gates the shim off for this branch ( A knock-on: with no shim there's no Three smaller ones:
On the transport-count disagreementYou flagged the I went a third way rather than picking a side: the sentence now names both dimensions instead of overloading one number — two homes, canonical and shadow, reached over four selection branches, with a note that the table splits the first branch into its two predicates. The diagram header was updated to match, which is what actually made the page self-contradictory. Also worth your timeThe multi-account balancer you described — one account-pinned app-server per account — has a real problem left in it that v2.8.4 does not fix. Thanks again. |
`codex app-server` took the shadow-`CODEX_HOME` transport, and a resident server cannot live there. Codex applies an lstat-strict check to `<CODEX_HOME>/app-server-control` and refuses to start when it is a symlink, which is what the shadow mirror makes of it; and a server that does start hands every attached client a frozen snapshot of the thread index for the life of the process, writing anything it creates into a copy discarded at exit. Route it to the canonical-home app helper, alongside the interactive TUI and `resume`/`fork` (ndycode#647/ndycode#648). Do not install the app-server CLI shim on that branch. The shim is reachable only from the app helper, so moving the transport silently drags it in, and it stamps `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0`, `CODEX_CLI_PATH`, and a preload `NODE_OPTIONS` onto the environment the forwarded child inherits. Codex passes that environment to shell tools and MCP servers, so every nested wrapper invocation would read rotation as disabled and bill whatever account the official CLI resolved. Its only purpose is to intercept the desktop app spawning its own app-server; a wrapper-invoked server already carries the overrides on its command line. `codex app` and the interactive branches keep it, unchanged. Because no shim means no account-label env, request the `account/read` / `getAuthStatus` / `account/rateLimits/read` rewriting explicitly through `proxyAppServerAccountRead`. Also on the shared helper path: - Honor an explicit `detachOnExit: false`, so a short-lived app-server no longer strands its helper for the full 12h idle timeout. - Start the detach-grace clock when the helper is ready rather than before a launch bounded at 15s, so `codex app` cannot kill the helper it just handed the desktop app off to. - Turn a helper that cannot start into a diagnostic and exit 1 instead of an unhandled rejection with a leaked compatibility home. Hard-fail is deliberate: there is no rotation-off shape to degrade into, and a resident server that quietly loses rotation is a billing error you cannot see. - Stop accumulating the helper startup stdout/stderr buffers once startup settles; the listeners stay attached so the helper never blocks on a full pipe. Reported with a complete reproduction by Mike Bannister (@possibilities) in ndycode#659; the core routing change is from their ndycode#660. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KT5pNBtZ151FSC6KBA32sk
2 participants
Summary
codex app-serverto the canonical-home app helper instead of the shadowCODEX_HOME, alongside the interactive TUI andresume/fork. Closes [bug] codex-multi-auth-codex app-server cannot run on the shadow CODEX_HOME #659.resume/forkfor the same reason. This is the third member of that set.<CODEX_HOME>/app-server-controlis a symlink, which is what the shadow mirror makes of it, and a server that does start hands every attached client a frozen snapshot of the thread index.What Changed
scripts/codex.jscreateRuntimeRotationProxyContextIfEnabledgets anapp-serverbranch above the interactive one, using the canonical-home app helper. Two deliberate differences from that branch:detachOnExit: false— a resident server owns its proxy for its whole lifetime. Leaving it to idle out would strand one helper per supervised restart.proxyAppServerAccountRead: true, threaded throughcreateRuntimeRotationAppHelperContext's options. Only the shadow branch used to set this, so without it the transport move would silently drop theaccount/read/getAuthStatus/account/rateLimits/readrewriting that stdio clients already rely on.whenis unchanged — the gate still requiresisCodexAppServerCommand, and theCODEX_MULTI_AUTH_APP_SERVER_ACCOUNT_LABELpath is untouched.cleanupdetached on any clean exit inside the 5s window regardless ofdetachOnExit: false, so a short-lived app-server — a client attaches, runs one query, disconnects — still stranded its helper for the full 12h idle timeout with nobody left to refresh the activity clock.detachOnExit === truedetaches past the window; an unsetdetachOnExitstill detaches inside it; an explicitfalsenever does. The TUI passestrueandcodex apppasses neither, so neither changes behavior.test/codex-bin-wrapper.test.tsapp-server --listen unix://…runs on the canonical home, sees the real thread index, gets a realapp-server-controldirectory, keeps the rotation overrides, and leavesconfig.tomluntouched.-coverrides withenv_key, canonicalCODEX_HOME,config.tomlbyte-identical) instead of the shadow shape. Its intent is unchanged; only the transport it pins moved.CODEX_MULTI_AUTH_APP_ROTATION_IDLE_MS, so both fail against the old grace clause and pass against the new one — the stranding fix is regression-tested rather than asserted.Docs —
docs/development/ARCHITECTURE.md,docs/architecture.md,docs/reference/commands.md,docs/configuration.md. The last three each stated thatapp-serverruns on a shadow home and would otherwise have shipped false.Validation
npm run lintnpm run typechecknpm testnpm test -- test/documentation.test.ts— 32/32npm run buildnpm testhas pre-existing failures on macOS unrelated to this change — Windows-path tests intest/codex-bin-wrapper.test.ts,test/codex-bin-resolver, andtest/paths.test.ts, plus named-backup tests intest/storage.test.tsandtest/install-codex-auth.test.ts. I compared matched worktrees (upstream7f5c61bvs this branch, samenode_modules) and the failure sets are the same modulo run-to-run variation under load; the composition shifts between runs even on an unchanged tree, and nothing in this diff touches those paths.test/codex-bin-wrapper.test.tsapp-server tests: 11/11.Also verified against real accounts on codex-cli 0.147.0: the pinned account governs the server's model calls, read back from each turn's rollout
rate_limitsagainst two accounts with distinguishable quota windows, and rollouts land in the canonicalsessionsdirectory.Docs and Governance Checklist
docs/getting-started.mdupdated — n/a, onboarding unchangeddocs/features.mdreviewed — no transport claimsdocs/reference/*pages updated —commands.mddocs/upgrade.mdupdated — n/a, no migration behaviorSECURITY.mdandCONTRIBUTING.mdreviewed for alignment — this is a transport-selection fix; it adds no auth surface, no scraping, no rate-limit circumventionRisk and Rollback
Risk level: low-medium. Behavior changes for one command, in the direction the same reasoning already took two others.
Two consequences worth naming rather than burying:
--accountalready fails hard. Happy to add a fallback if you would rather preserve the old shape.-c model_providerprecedence. As with the TUI branch, the wrapper's override is appended last and now wins over a user-supplied one; on the shadow path a user value beat the shadowconfig.toml. Inherited from the accepted branch rather than introduced here.Rollback: revert the commits; nothing persists and no state migrates. The only on-disk difference while running is that app-server no longer creates a shadow home under
runtime-shadow-homes/.Additional Notes
Found while building a multi-account balancer that runs one account-pinned app-server per account. Independently reviewed before submission; the detach-grace defect above came out of that review rather than out of the original patch.
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
routes
codex app-serverthrough the canonical codex home while retaining runtime rotation and account-response rewriting.app-server-controldirectory available.Confidence Score: 5/5
the pr appears safe to merge.
no blocking failure remains.
Important Files Changed
Sequence Diagram
Reviews (3): Last reviewed commit: "docs: cover the app-server help flags an..." | Re-trigger Greptile
Context used: