fix: resolve issues #652–#656 - #658
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthroughthis is a major account-management and runtime-routing fix set, not a blocker. focused regression tests cover all five reported fixes. no direct security or data-loss regression is reported, but persisted token invalidation and process ownership checks require careful review. concurrency and some windows failure paths remain limited. key changes
review focus and risks
Walkthroughthis pr persists oauth invalidation state, applies policy-aware selection, adds window-aware quota deferral, hardens runtime shutdown, supports legacy package cleanup, fixes oauth callback guidance, and protects direct storage-path overrides. Changesaccount invalidation and selection
quota scheduling
runtime lifecycle and storage state
legacy package lifecycle and oauth guidance
estimated code review effort: 5 (critical) | ~120 minutes possibly related prs
suggested labels: suggested reviewers: 📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
|
![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: 11
🤖 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 `@lib/accounts.ts`:
- Around line 1434-1435: Update the routine account-save reconciliation around
the token-field merge to preserve a valid persisted auth invalidation marker
when the in-memory snapshot is marker-free; do not clear disk invalidation
during ordinary saves. Keep the successful refresh transaction as the explicit
path that removes the marker, and add a deterministic two-AccountManager
concurrency regression test ordering invalidation before the stale routine save,
covering the Windows filesystem path.
In `@lib/codex-manager.ts`:
- Around line 430-462: Update the policy handling in the selection-trace flow
around evaluateRuntimePolicy and selectHybridAccountTraced so policy.allowed ===
false prevents any account from being selected, either by blocking every
candidate or returning a no-selection trace. Preserve existing per-account auth
invalidation handling, and add Vitest regression coverage in
codex-manager-selection-diagnostics.test.ts for globally denied models and
budget-blocked requests, including auth-rotation routing coverage as required.
In `@lib/runtime/app-bind.ts`:
- Around line 715-719: Correct the error handling in the SIGKILL catch around
kill and the analogous SIGTERM catch: return immediately only for ignorable
ESRCH/EPERM cases, while allowing unexpected errors to propagate or be reported
through options.log. Remove the dead platform !== "win32" guard and ensure ESRCH
does not proceed to another wait because the process has already exited.
- Around line 623-638: Update runWindowsTaskkill to enforce a finite timeout
while waiting for taskkill, ensuring the promise resolves even if neither error
nor close fires; clear the timer when finishing and preserve single-settlement
behavior. Capture and report the taskkill outcome, including timeout, error, and
nonzero exit status, so stopDetachedProcess and
unbindCodexAppRuntimeRotationLocked can distinguish unsuccessful termination
from success.
- Around line 991-999: Update the cleanup loop around helperStatusPath so it
only unlinks the helper status file after the helper has been confirmed stopped.
Preserve the existing behavior for bind and log paths, and retain helper status
files when stop logic skips a different kind or reports that the helper is still
alive.
- Around line 117-130: Decide whether the new exported symbols
DetachedProcessStopOptions, RuntimeRotationAppHelperStatus, and the related
app-bind stop functions are public APIs; if so, re-export them through
lib/index.ts or an appropriate package subpath, otherwise remove their public
exports and keep them scoped to existing app-bind consumers.
- Around line 691-710: Update the process termination flow around kill and
waitForDetachedProcessExit to bypass kill(pid, "SIGTERM") on Windows and invoke
taskkill(pid) directly, ensuring the detached tree is terminated even if the
target exits immediately. Preserve the existing non-Windows signal and fallback
behavior, and add a Windows regression in the relevant app-bind test covering
immediate liveness loss and asserting taskkill is called.
In `@scripts/install-codex-auth-utils.js`:
- Line 87: Add a deterministic Vitest case covering both resolveInstallPaths and
resolveUninstallPaths with platform set to "win32" and matching
APPDATA/LOCALAPPDATA values. Assert that the current and legacy cache paths
produced by the two resolvers match, including the cacheLegacyNodeModules path,
without relying on process.platform.
In `@scripts/preuninstall.js`:
- Around line 207-214: Extend the Vitest coverage in the preuninstall test suite
to seed both cacheNodeModules and cacheLegacyNodeModules, inject one transient
EBUSY, EPERM, or ENOTEMPTY failure during removal, and assert that both
directories are ultimately deleted. Keep the test deterministic and exercise the
withFileOperationRetry path used by the legacy-cache cleanup.
In `@test/app-bind.test.ts`:
- Around line 336-415: Add integration regression cases around
unbindCodexAppRuntimeRotation that place runtime-rotation-app-helper.json beside
the bind directory: verify a matching running helper is discovered and its
status file removed, a foreign kind leaves the file intact, and malformed or
absent JSON still completes while restoring config.toml. Also add coverage for
the injected kill path where kill throws an ESRCH error, preserving the
ignorable-error behavior in stopRuntimeRotationAppHelperProcess.
In `@test/preemptive-quota-scheduler.test.ts`:
- Around line 239-258: Add a regression test alongside the existing
stale/reset-data cases for a snapshot with updatedAt set to now + 1, and assert
getDeferral returns defer: true with waitMs equal to the configured
maxDeferralMs and reason "quota-near-exhaustion". Use PreemptiveQuotaScheduler
and the existing update/getDeferral setup to verify future-dated snapshots use
the fallback delay.
🪄 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: b41ff91b-4397-4647-ac0e-72ab98f7d5fe
📒 Files selected for processing (38)
docs/development/CONFIG_FIELDS.mdlib/accounts.tslib/codex-manager.tslib/codex-manager/account-pool-write.tslib/codex-manager/backend-settings-schema.tslib/codex-manager/commands/status.tslib/codex-manager/commands/uninstall.tslib/codex-manager/commands/why-selected.tslib/codex-manager/health-check.tslib/codex-manager/login-oauth.tslib/config.tslib/forecast.tslib/parallel-probe.tslib/policy/runtime-policy.tslib/preemptive-quota-scheduler.tslib/rotation.tslib/runtime-rotation-proxy.tslib/runtime/app-bind.tslib/runtime/rotation-account-selection.tslib/runtime/rotation-proxy-state.tslib/runtime/rotation-token-refresh.tslib/schemas.tslib/storage/flagged-storage.tslib/storage/public-types.tsscripts/install-codex-auth-utils.jsscripts/postinstall.jsscripts/preuninstall.jstest/app-bind.test.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-manager-status-command.test.tstest/login-oauth-callback-guidance.test.tstest/postinstall.test.tstest/preemptive-quota-scheduler.test.tstest/rotation-proxy-state.test.tstest/rotation-token-refresh.test.tstest/runtime-rotation-proxy.test.tstest/uninstall-command.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (27)
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/rotation-proxy-state.test.tstest/codex-manager-status-command.test.tstest/postinstall.test.tstest/rotation-token-refresh.test.tstest/preemptive-quota-scheduler.test.tstest/uninstall-command.test.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/app-bind.test.tstest/login-oauth-callback-guidance.test.tstest/runtime-rotation-proxy.test.ts
test/**/rotation*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Test account selection and rotation logic in rotation.test.ts and rotation-integration.test.ts
Files:
test/rotation-proxy-state.test.tstest/rotation-token-refresh.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/rotation-proxy-state.test.tsscripts/preuninstall.jslib/parallel-probe.tslib/codex-manager/backend-settings-schema.tslib/runtime/rotation-token-refresh.tslib/forecast.tslib/runtime/rotation-proxy-state.tstest/codex-manager-status-command.test.tslib/storage/flagged-storage.tslib/codex-manager/login-oauth.tstest/postinstall.test.tslib/codex-manager/health-check.tslib/codex-manager/commands/status.tslib/storage/public-types.tslib/codex-manager/commands/why-selected.tslib/config.tsscripts/postinstall.jstest/rotation-token-refresh.test.tslib/codex-manager/account-pool-write.tstest/preemptive-quota-scheduler.test.tstest/uninstall-command.test.tslib/runtime/rotation-account-selection.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/app-bind.test.tslib/codex-manager.tslib/rotation.tslib/schemas.tsscripts/install-codex-auth-utils.jstest/login-oauth-callback-guidance.test.tslib/runtime-rotation-proxy.tslib/policy/runtime-policy.tslib/accounts.tslib/codex-manager/commands/uninstall.tslib/preemptive-quota-scheduler.tstest/runtime-rotation-proxy.test.tslib/runtime/app-bind.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/rotation-proxy-state.test.tstest/codex-manager-status-command.test.tstest/postinstall.test.tstest/rotation-token-refresh.test.tstest/preemptive-quota-scheduler.test.tstest/uninstall-command.test.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/app-bind.test.tstest/login-oauth-callback-guidance.test.tstest/runtime-rotation-proxy.test.ts
**/*
📄 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:
test/rotation-proxy-state.test.tsscripts/preuninstall.jslib/parallel-probe.tslib/codex-manager/backend-settings-schema.tslib/runtime/rotation-token-refresh.tslib/forecast.tslib/runtime/rotation-proxy-state.tstest/codex-manager-status-command.test.tslib/storage/flagged-storage.tslib/codex-manager/login-oauth.tstest/postinstall.test.tsdocs/development/CONFIG_FIELDS.mdlib/codex-manager/health-check.tslib/codex-manager/commands/status.tslib/storage/public-types.tslib/codex-manager/commands/why-selected.tslib/config.tsscripts/postinstall.jstest/rotation-token-refresh.test.tslib/codex-manager/account-pool-write.tstest/preemptive-quota-scheduler.test.tstest/uninstall-command.test.tslib/runtime/rotation-account-selection.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/app-bind.test.tslib/codex-manager.tslib/rotation.tslib/schemas.tsscripts/install-codex-auth-utils.jstest/login-oauth-callback-guidance.test.tslib/runtime-rotation-proxy.tslib/policy/runtime-policy.tslib/accounts.tslib/codex-manager/commands/uninstall.tslib/preemptive-quota-scheduler.tstest/runtime-rotation-proxy.test.tslib/runtime/app-bind.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/rotation-proxy-state.test.tsscripts/preuninstall.jslib/parallel-probe.tslib/codex-manager/backend-settings-schema.tslib/runtime/rotation-token-refresh.tslib/forecast.tslib/runtime/rotation-proxy-state.tstest/codex-manager-status-command.test.tslib/storage/flagged-storage.tslib/codex-manager/login-oauth.tstest/postinstall.test.tslib/codex-manager/health-check.tslib/codex-manager/commands/status.tslib/storage/public-types.tslib/codex-manager/commands/why-selected.tslib/config.tsscripts/postinstall.jstest/rotation-token-refresh.test.tslib/codex-manager/account-pool-write.tstest/preemptive-quota-scheduler.test.tstest/uninstall-command.test.tslib/runtime/rotation-account-selection.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/app-bind.test.tslib/codex-manager.tslib/rotation.tslib/schemas.tsscripts/install-codex-auth-utils.jstest/login-oauth-callback-guidance.test.tslib/runtime-rotation-proxy.tslib/policy/runtime-policy.tslib/accounts.tslib/codex-manager/commands/uninstall.tslib/preemptive-quota-scheduler.tstest/runtime-rotation-proxy.test.tslib/runtime/app-bind.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/rotation-proxy-state.test.tstest/codex-manager-status-command.test.tstest/postinstall.test.tstest/rotation-token-refresh.test.tstest/preemptive-quota-scheduler.test.tstest/uninstall-command.test.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/app-bind.test.tstest/login-oauth-callback-guidance.test.tstest/runtime-rotation-proxy.test.ts
scripts/**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Windows-sensitive cleanup and write operations must retry transient
EBUSY,EPERM, andENOTEMPTYfailures where applicable.
Files:
scripts/preuninstall.jsscripts/postinstall.jsscripts/install-codex-auth-utils.js
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: Route all public exports throughlib/index.tsor documented package subpaths.
Keep module dependencies acyclic and preserve the layeringtypes/constants → storage → accounts → runtime → manager/CLI; lower layers must not import higher layers.
Preserve runtime rotation pass-through semantics except for intentionally changed auth or provider headers.
Deduplicate emails usingnormalizeEmailKey(), which trims and lowercases the email.
Use classes for state requiring multiple independent instances or dependency injection, includingAccountManager,CircuitBreaker,SessionAffinityStore, and theCodexErrorhierarchy. Reserve module-level state for genuinely process-global concerns and provide a test reset helper for such state.
Never import fromdist/in source tests or library code.
Never suppress type errors.
Never patch official Codex application binaries for desktop routing.
Never use bare recursive cleanup in Windows-sensitive paths without retry handling.
Files:
lib/parallel-probe.tslib/codex-manager/backend-settings-schema.tslib/runtime/rotation-token-refresh.tslib/forecast.tslib/runtime/rotation-proxy-state.tslib/storage/flagged-storage.tslib/codex-manager/login-oauth.tslib/codex-manager/health-check.tslib/codex-manager/commands/status.tslib/storage/public-types.tslib/codex-manager/commands/why-selected.tslib/config.tslib/codex-manager/account-pool-write.tslib/runtime/rotation-account-selection.tslib/codex-manager.tslib/rotation.tslib/schemas.tslib/runtime-rotation-proxy.tslib/policy/runtime-policy.tslib/accounts.tslib/codex-manager/commands/uninstall.tslib/preemptive-quota-scheduler.tslib/runtime/app-bind.ts
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/parallel-probe.tslib/codex-manager/backend-settings-schema.tslib/runtime/rotation-token-refresh.tslib/forecast.tslib/runtime/rotation-proxy-state.tslib/storage/flagged-storage.tslib/codex-manager/login-oauth.tslib/codex-manager/health-check.tslib/codex-manager/commands/status.tslib/storage/public-types.tslib/codex-manager/commands/why-selected.tslib/config.tslib/codex-manager/account-pool-write.tslib/runtime/rotation-account-selection.tslib/codex-manager.tslib/rotation.tslib/schemas.tslib/runtime-rotation-proxy.tslib/policy/runtime-policy.tslib/accounts.tslib/codex-manager/commands/uninstall.tslib/preemptive-quota-scheduler.tslib/runtime/app-bind.ts
lib/{runtime-rotation-proxy.ts,runtime/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Runtime rotation must fail open to normal official Codex forwarding when startup helpers are unavailable.
Files:
lib/runtime/rotation-token-refresh.tslib/runtime/rotation-proxy-state.tslib/runtime/rotation-account-selection.tslib/runtime-rotation-proxy.tslib/runtime/app-bind.ts
lib/runtime/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not patch official Codex app binaries; use the reversible app-bind or launcher-helper mechanisms instead.
Files:
lib/runtime/rotation-token-refresh.tslib/runtime/rotation-proxy-state.tslib/runtime/rotation-account-selection.tslib/runtime/app-bind.ts
lib/{storage/**/*.ts,storage.ts,runtime-paths.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/{storage/**/*.ts,storage.ts,runtime-paths.ts}: Resolve project storage identity withresolveProjectStorageIdentityRoot; never derive project pools directly from raw worktree paths.
Never key project storage directly by worktree path.
Files:
lib/storage/flagged-storage.tslib/storage/public-types.ts
lib/storage/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not key project storage by worktree path; use
resolveProjectStorageIdentityRoot.
Files:
lib/storage/flagged-storage.tslib/storage/public-types.ts
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/development/CONFIG_FIELDS.md
docs/development/CONFIG_FIELDS.md
📄 CodeRabbit inference engine (docs/development/RUNBOOK_ADD_CONFIG_FIELD.md)
Update
docs/development/CONFIG_FIELDS.mdwith field inventory details when adding new configuration fieldsMaintain full field inventory in
docs/development/CONFIG_FIELDS.md
Files:
docs/development/CONFIG_FIELDS.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/CONFIG_FIELDS.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/CONFIG_FIELDS.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/CONFIG_FIELDS.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/development/CONFIG_FIELDS.md
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/development/CONFIG_FIELDS.md
test/**/codex-manager-cli.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Test CLI settings management across 5 panels with Q cancel handling in codex-manager-cli.test.ts
Files:
test/codex-manager-cli.test.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts,request/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Do not forward stale decoded
content-encodingmetadata when Node fetch has already decoded response bytes.
Files:
lib/runtime-rotation-proxy.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/{runtime-rotation-proxy.ts,local-bridge.ts}: Runtime proxy client-facing headers and responses must never expose account emails or tokens.
Never include account emails or tokens in runtime proxy client responses.
Files:
lib/runtime-rotation-proxy.ts
lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (AGENTS.md)
lib/runtime-rotation-proxy.ts: Keep runtime rotation enabled by default, use loopback-only networking, and use a per-process client token.
Do not expose account emails or tokens in runtime proxy response headers or logs.
The runtime proxy may forward only Responses API and model-discovery requests.
Files:
lib/runtime-rotation-proxy.ts
lib/{accounts.ts,accounts/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Maintain account health on a 0–100 scale and update it through account manager APIs.
Files:
lib/accounts.ts
lib/accounts.ts
📄 CodeRabbit inference engine (AGENTS.md)
Email deduplication must be case-insensitive using
normalizeEmailKey()(trim and lowercase).
Files:
lib/accounts.ts
🧠 Learnings (2)
📚 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/rotation-proxy-state.test.tstest/codex-manager-status-command.test.tstest/postinstall.test.tstest/rotation-token-refresh.test.tstest/preemptive-quota-scheduler.test.tstest/uninstall-command.test.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/app-bind.test.tstest/login-oauth-callback-guidance.test.tstest/runtime-rotation-proxy.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/rotation-proxy-state.test.tstest/codex-manager-status-command.test.tstest/postinstall.test.tstest/rotation-token-refresh.test.tstest/preemptive-quota-scheduler.test.tstest/uninstall-command.test.tstest/codex-manager-cli.test.tstest/codex-manager-selection-diagnostics.test.tstest/app-bind.test.tstest/login-oauth-callback-guidance.test.tstest/runtime-rotation-proxy.test.ts
🪛 ast-grep (0.45.0)
lib/runtime/app-bind.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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 (40)
lib/codex-manager/login-oauth.ts (1)
265-265: LGTM!Also applies to: 309-325
test/login-oauth-callback-guidance.test.ts (1)
8-66: LGTM!Also applies to: 110-113, 155-190
lib/preemptive-quota-scheduler.ts (1)
1-1: LGTM!Also applies to: 32-60, 322-335
test/preemptive-quota-scheduler.test.ts (1)
177-237: LGTM!Also applies to: 260-274
lib/codex-manager/backend-settings-schema.ts (1)
279-279: LGTM!lib/config.ts (1)
1821-1823: LGTM!docs/development/CONFIG_FIELDS.md (1)
161-163: LGTM!test/rotation-proxy-state.test.ts (1)
3-3: LGTM!Also applies to: 67-67
lib/accounts.ts (1)
242-271: LGTM!Also applies to: 520-521, 650-679, 796-796, 831-831, 889-889, 946-946, 1256-1308, 1501-1555, 1604-1605
lib/rotation.ts (1)
372-373: LGTM!Also applies to: 405-406, 484-487, 627-631, 659-677
lib/codex-manager/commands/why-selected.ts (1)
33-33: LGTM!Also applies to: 141-141
lib/codex-manager/commands/status.ts (1)
2-2: LGTM!Also applies to: 91-96
lib/codex-manager/health-check.ts (1)
2-2: LGTM!Also applies to: 45-54, 180-180, 223-230, 295-295, 309-322
lib/forecast.ts (1)
1-5: LGTM!Also applies to: 211-219
lib/parallel-probe.ts (1)
145-149: LGTM!test/codex-manager-cli.test.ts (1)
170-193: LGTM!test/codex-manager-selection-diagnostics.test.ts (1)
1-172: LGTM!test/codex-manager-status-command.test.ts (1)
11-11: LGTM!Also applies to: 213-232
test/rotation-token-refresh.test.ts (1)
41-59: LGTM!Also applies to: 71-78, 258-302
lib/codex-manager/account-pool-write.ts (1)
138-139: LGTM!Also applies to: 196-197
lib/schemas.ts (1)
178-179: LGTM!lib/storage/public-types.ts (1)
47-50: LGTM!lib/storage/flagged-storage.ts (1)
85-94: LGTM!Also applies to: 134-135
lib/runtime/rotation-token-refresh.ts (1)
105-107: LGTM!lib/runtime-rotation-proxy.ts (1)
23-26: LGTM!Also applies to: 52-55, 711-720, 751-751, 1074-1096, 1209-1228, 1355-1358, 1448-1454
lib/policy/runtime-policy.ts (1)
42-42: LGTM!Also applies to: 151-151, 194-200, 237-237, 251-251
lib/runtime/rotation-account-selection.ts (1)
161-162: LGTM!lib/runtime/rotation-proxy-state.ts (1)
2-2: LGTM!Also applies to: 36-36
test/runtime-rotation-proxy.test.ts (1)
1415-1437: LGTM!Also applies to: 2403-2406, 2490-2493
lib/runtime/app-bind.ts (4)
4-11: LGTM!
371-371: LGTM!Also applies to: 535-537, 878-878
729-754: LGTM!
928-932: 🗄️ Data Integrity & Integrationno change needed.
lib/runtime/app-bind.ts:421-429resolvespaths.bindDirunder the samemode = (env.CODEX_MULTI_AUTH_APP_BIND_CODEX_HOME ?? "").trim() || join(home, ".codex"); multiAuthDir = (...) || getCodexMultiAuthDir();sodirname(paths.bindDir)maps to the canonical multi-auth helper-status location.lib/codex-manager/commands/uninstall.ts (1)
9-10: LGTM!Also applies to: 31-36, 47-49, 83-83, 290-292, 304-308, 355-357
scripts/install-codex-auth-utils.js (2)
9-10: LGTM!Also applies to: 98-100
109-111: 🎯 Functional Correctnessno change needed.
test/install-codex-auth.test.ts:65-74covers duplicate entries and unrelated entries, andtest/install-codex-auth.test.ts:76-81covers preservation of other entries throughremovePluginFromList, which exercises the same legacy-removal logic used bynormalizePluginList.scripts/postinstall.js (1)
36-37: LGTM!scripts/preuninstall.js (1)
192-194: LGTM!test/uninstall-command.test.ts (1)
61-70: LGTM!Also applies to: 166-166, 245-261, 263-278, 299-299, 308-308
test/postinstall.test.ts (1)
1-24: LGTM!
4426904 to
baf7bef
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@lib/runtime-rotation-proxy.ts`:
- Around line 177-179: Update the email branch of the schedule-key construction
to use normalizeEmailKey() instead of local trim().toLowerCase() logic, while
preserving the existing refresh-token fallback and email: key prefix.
In `@lib/runtime/app-bind.ts`:
- Around line 690-741: Update the POSIX branch of readProcessIdentity to parse
ps lstart output explicitly using its documented format instead of relying on
Date.parse. Ensure valid lstart values produce the correct timestamp so
verifyRuntimeHelperProcessIdentity can validate live helpers, and add coverage
using representative actual lstart output.
In `@test/app-bind.test.ts`:
- Around line 362-390: Strengthen the Windows regression test for
stopRuntimeRotationAppHelperProcess by making the kill mock set alive to false
immediately when SIGTERM is issued, then continue asserting that
runWindowsTaskkill is called with PID 4343. Preserve the existing SIGTERM
assertion so the test verifies taskkill still runs after the process exits
during the graceful wait.
In `@test/runtime-rotation-proxy.test.ts`:
- Around line 2448-2451: Extend the regression coverage around the token-refresh
behavior in the relevant runtime rotation test by adding a 401 fixture with a
distinct invalidation code recognized by isTokenInvalidationError(). Trigger the
account invalidation flow and assert that accountManager.getAccountByIndex(0)
persists that exact code in authInvalidationErrorCode, alongside
authInvalidatedAt, rather than only covering the token_invalidated fallback.
🪄 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: 56efd8b3-fc0d-4206-9fd8-c13025f2d27a
📒 Files selected for processing (4)
lib/runtime-rotation-proxy.tslib/runtime/app-bind.tstest/app-bind.test.tstest/runtime-rotation-proxy.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (13)
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/runtime-rotation-proxy.test.tstest/app-bind.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/runtime-rotation-proxy.test.tstest/app-bind.test.tslib/runtime-rotation-proxy.tslib/runtime/app-bind.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/runtime-rotation-proxy.test.tstest/app-bind.test.ts
**/*
📄 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:
test/runtime-rotation-proxy.test.tstest/app-bind.test.tslib/runtime-rotation-proxy.tslib/runtime/app-bind.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/runtime-rotation-proxy.test.tstest/app-bind.test.tslib/runtime-rotation-proxy.tslib/runtime/app-bind.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/runtime-rotation-proxy.test.tstest/app-bind.test.ts
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: Route all public exports throughlib/index.tsor documented package subpaths.
Keep module dependencies acyclic and preserve the layeringtypes/constants → storage → accounts → runtime → manager/CLI; lower layers must not import higher layers.
Preserve runtime rotation pass-through semantics except for intentionally changed auth or provider headers.
Deduplicate emails usingnormalizeEmailKey(), which trims and lowercases the email.
Use classes for state requiring multiple independent instances or dependency injection, includingAccountManager,CircuitBreaker,SessionAffinityStore, and theCodexErrorhierarchy. Reserve module-level state for genuinely process-global concerns and provide a test reset helper for such state.
Never import fromdist/in source tests or library code.
Never suppress type errors.
Never patch official Codex application binaries for desktop routing.
Never use bare recursive cleanup in Windows-sensitive paths without retry handling.
Files:
lib/runtime-rotation-proxy.tslib/runtime/app-bind.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts,request/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Do not forward stale decoded
content-encodingmetadata when Node fetch has already decoded response bytes.
Files:
lib/runtime-rotation-proxy.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/{runtime-rotation-proxy.ts,local-bridge.ts}: Runtime proxy client-facing headers and responses must never expose account emails or tokens.
Never include account emails or tokens in runtime proxy client responses.
Files:
lib/runtime-rotation-proxy.ts
lib/{runtime-rotation-proxy.ts,runtime/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Runtime rotation must fail open to normal official Codex forwarding when startup helpers are unavailable.
Files:
lib/runtime-rotation-proxy.tslib/runtime/app-bind.ts
lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (AGENTS.md)
lib/runtime-rotation-proxy.ts: Keep runtime rotation enabled by default, use loopback-only networking, and use a per-process client token.
Do not expose account emails or tokens in runtime proxy response headers or logs.
The runtime proxy may forward only Responses API and model-discovery requests.
Files:
lib/runtime-rotation-proxy.ts
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/runtime-rotation-proxy.tslib/runtime/app-bind.ts
lib/runtime/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not patch official Codex app binaries; use the reversible app-bind or launcher-helper mechanisms instead.
Files:
lib/runtime/app-bind.ts
🧠 Learnings (2)
📚 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/runtime-rotation-proxy.test.tstest/app-bind.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/runtime-rotation-proxy.test.tstest/app-bind.test.ts
🪛 ast-grep (0.45.0)
lib/runtime/app-bind.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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 (13)
lib/runtime-rotation-proxy.ts (1)
23-26: LGTM!Also applies to: 52-55, 725-765, 1088-1114, 1227-1246, 1373-1376, 1466-1472
test/runtime-rotation-proxy.test.ts (1)
9-14: LGTM!Also applies to: 1417-1481, 2535-2538
lib/runtime/app-bind.ts (9)
121-140: still unrouted.lib/runtime/app-bind.ts:121andlib/runtime/app-bind.ts:134exportDetachedProcessStopOptionsandRuntimeRotationAppHelperStatus, andlib/runtime/app-bind.ts:780/lib/runtime/app-bind.ts:849export the stop functions, but no barrel or package subpath exposes them. route them throughlib/index.tsor keep them internal.As per coding guidelines: "Route all public exports through
lib/index.tsor documented package subpaths."Source: Coding guidelines
633-648: unchanged from the last round.lib/runtime/app-bind.ts:634resolves only onerrororclose, so a wedgedtaskkillblocksunbindCodexAppRuntimeRotationLockedatlib/runtime/app-bind.ts:1036forever on windows. the exit code is also discarded, so access-denied looks like a clean tree kill. bound the wait and report the outcome.
794-813: still ordered wrong for windows.lib/runtime/app-bind.ts:795sendsSIGTERMbefore anytaskkill. node terminates only the target pid on win32 and leaves detached descendants running. once the target exits,waitForDetachedProcessExitreturns true atlib/runtime/app-bind.ts:806and the function returns before thetaskkillatlib/runtime/app-bind.ts:811, so the tracked tree is orphaned. calltaskkill(pid)directly on win32.
818-822: the inverted branch is still here. atlib/runtime/app-bind.ts:821an unexpectedkillfailure returns silently, whileESRCH/EPERMfalls through to a second wait. invert it: return for ignorable codes, and propagate or log unexpected errors throughoptions.log. the same shape sits in theSIGTERMcatch atlib/runtime/app-bind.ts:801.
1102-1114: unchanged.lib/runtime/app-bind.ts:1106unlinkshelperStatusPathunconditionally, even when the gate atlib/runtime/app-bind.ts:1048-1051skipped a foreignkind, and even when the warning atlib/runtime/app-bind.ts:1053-1057reported that the helper is still alive. that file lives outsidepaths.bindDirand is the shared discovery artifact, so deleting it strands a live helper. gate the unlink on a confirmed stop.
4-11: LGTM!Also applies to: 22-25
381-381: LGTM!Also applies to: 545-547
832-869: LGTM!Also applies to: 993-993
1115-1121: 🩺 Stability & Availabilityno change needed: the app bind lock does not live inside
paths.bindDir.
withAppBindLock(paths.bindDir, ...)atlib/runtime/app-bind.ts:906andlib/runtime/app-bind.ts:1024uses thepaths.bindDirstring only as a shared async lock key; no lock artifacts are created underpaths.bindDir, so the recursive removal does not delete the held lock.> Likely an incorrect or invalid review comment.test/app-bind.test.ts (2)
392-462: coverage gap is unchanged from the last round.test/app-bind.test.ts:392-462exercisesstopRuntimeRotationAppHelperProcessin isolation with injected handlers only. the code this pr added for#653is still untested: status-file discovery atlib/runtime/app-bind.ts:1043-1047, the kind/state gate atlib/runtime/app-bind.ts:1048-1051, and the status-file removal atlib/runtime/app-bind.ts:1106. add unbind cases that write aruntime-rotation-app-helper.jsonfixture next to the bind directory, plus anESRCHcase where the injectedkillthrows, to lock in the ignorable-error contract atlib/runtime/app-bind.ts:743-749.As per path instructions for
test/**: "demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior."Source: Path instructions
8-16: LGTM!Also applies to: 333-334
baf7bef to
4735b99
Compare
a580528 to
2f47e24
Compare
2f47e24 to
dabb578
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
lib/runtime/app-bind.ts (2)
1200-1208: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winthe cleanup loop deletes
helperStatusPathandpaths.statusPatheven when the process is still running. two paths reach here with a live process: the identity gate atlib/runtime/app-bind.ts:654andlib/runtime/app-bind.ts:678returns false whenever the probe times out orps/powershell.exeis unavailable, and the warnings atlib/runtime/app-bind.ts:1131-1135andlib/runtime/app-bind.ts:1147-1151only log. after the unlink plus the recursiverm(paths.bindDir)atlib/runtime/app-bind.ts:1211, no laterunbind-appor status command can find that orphan router or helper.gate both status removals on a confirmed stop.
stopRuntimeRotationRouterProcessandstopRuntimeRotationAppHelperProcessalready return that boolean; capture it inunbindCodexAppRuntimeRotationLockedand keep the status file when the process is still alive or the identity check refused to act. the same applies to a foreignkind, whichlib/runtime/app-bind.ts:1142-1145deliberately skips.🤖 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 `@lib/runtime/app-bind.ts` around lines 1200 - 1208, Update unbindCodexAppRuntimeRotationLocked to capture the confirmed-stop booleans returned by stopRuntimeRotationRouterProcess and stopRuntimeRotationAppHelperProcess. Gate removal of helperStatusPath and paths.statusPath on the corresponding process having been confirmed stopped, preserving those files when identity checks time out, process probes are unavailable, the process remains alive, or a foreign kind is skipped; ensure the subsequent bind-directory cleanup cannot remove these status files in those cases.
887-894: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winthe windows ordering issue is not fixed.
lib/runtime/app-bind.ts:876sendsSIGTERM, which node maps to an unconditional kill of that pid only. if the root exits inside the graceful window,waitForDetachedProcessExitreturns true atlib/runtime/app-bind.ts:887and the function returns before thetaskkillatlib/runtime/app-bind.ts:892, so detached descendants survive. skip the signal whenplatform === "win32"and calltaskkill(pid)first.the windows test at
test/app-bind.test.ts:363-391keepsalivetrue untiltaskkillruns, so it cannot catch this. add a case wherekillflipsaliveto false and still asserttaskkillran.🐛 tree kill first on windows
if (!isAlive(pid)) return; + if (platform === "win32") { + // node's SIGTERM emulation kills only the target pid, so detached + // descendants would be orphaned. taskkill /T owns the tree. + await taskkill(pid); + await waitForDetachedProcessExit(pid, options); + return; + } try { kill(pid, "SIGTERM");🤖 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 `@lib/runtime/app-bind.ts` around lines 887 - 894, Update the detached-process shutdown flow around waitForDetachedProcessExit so Windows skips the initial SIGTERM and invokes taskkill(pid) first, ensuring the process tree is terminated even when the root exits during the graceful wait. Preserve the existing non-Windows graceful shutdown path. Extend the Windows coverage in the relevant app-bind tests with a case where kill sets alive to false, and still assert taskkill executes.
🤖 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 `@scripts/codex.js`:
- Around line 3660-3671: The Windows shim replacement in the executable setup
must retry transient filesystem failures. Update the preceding stale-shim
deletion and the Windows copy in the visible setup flow to use the existing
retry handling for EBUSY, EPERM, and ENOTEMPTY where applicable, preserving the
independent copied executable behavior. Add a deterministic Vitest regression in
the app-router tests covering startup with a locked stale shim.
In `@test/app-bind.test.ts`:
- Around line 525-603: Update the “validates legacy router timestamps and
command identity before stopping” test to inject deterministic mock
process-identity handlers into stopRuntimeRotationRouterProcess, preventing
calls to the host ps/PowerShell probe while preserving the timestamp and
command-mismatch assertions. Keep the real spawned subprocess and final
successful stop assertion, and retain createTempRoot cleanup using the existing
withFileOperationRetry pattern.
In `@test/storage.test.ts`:
- Around line 78-83: Replace the cleanup call to setStoragePathDirectState with
setStoragePathState, ensuring directStorageStateOverride is cleared rather than
installing a null override. Preserve deterministic test cleanup so later tests
and async-context concurrency cases use the correct storage state.
---
Duplicate comments:
In `@lib/runtime/app-bind.ts`:
- Around line 1200-1208: Update unbindCodexAppRuntimeRotationLocked to capture
the confirmed-stop booleans returned by stopRuntimeRotationRouterProcess and
stopRuntimeRotationAppHelperProcess. Gate removal of helperStatusPath and
paths.statusPath on the corresponding process having been confirmed stopped,
preserving those files when identity checks time out, process probes are
unavailable, the process remains alive, or a foreign kind is skipped; ensure the
subsequent bind-directory cleanup cannot remove these status files in those
cases.
- Around line 887-894: Update the detached-process shutdown flow around
waitForDetachedProcessExit so Windows skips the initial SIGTERM and invokes
taskkill(pid) first, ensuring the process tree is terminated even when the root
exits during the graceful wait. Preserve the existing non-Windows graceful
shutdown path. Extend the Windows coverage in the relevant app-bind tests with a
case where kill sets alive to false, and still assert taskkill executes.
🪄 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: 532e8aae-06fa-4bae-a66d-0c11500dd3d7
📒 Files selected for processing (9)
lib/runtime/app-bind.tslib/storage.tslib/storage/path-state.tsscripts/codex-app-router.jsscripts/codex.jstest/app-bind.test.tstest/codex-app-router.test.tstest/codex-bin-wrapper.test.tstest/storage.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (16)
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-app-router.test.tstest/codex-bin-wrapper.test.tstest/storage.test.tstest/app-bind.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-app-router.test.tstest/codex-bin-wrapper.test.tslib/storage.tstest/storage.test.tslib/storage/path-state.tsscripts/codex.jsscripts/codex-app-router.jstest/app-bind.test.tslib/runtime/app-bind.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-app-router.test.tstest/codex-bin-wrapper.test.tstest/storage.test.tstest/app-bind.test.ts
**/*
📄 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:
test/codex-app-router.test.tstest/codex-bin-wrapper.test.tslib/storage.tstest/storage.test.tslib/storage/path-state.tsscripts/codex.jsscripts/codex-app-router.jstest/app-bind.test.tslib/runtime/app-bind.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-app-router.test.tstest/codex-bin-wrapper.test.tslib/storage.tstest/storage.test.tslib/storage/path-state.tsscripts/codex.jsscripts/codex-app-router.jstest/app-bind.test.tslib/runtime/app-bind.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-app-router.test.tstest/codex-bin-wrapper.test.tstest/storage.test.tstest/app-bind.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
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: Route all public exports throughlib/index.tsor documented package subpaths.
Keep module dependencies acyclic and preserve the layeringtypes/constants → storage → accounts → runtime → manager/CLI; lower layers must not import higher layers.
Preserve runtime rotation pass-through semantics except for intentionally changed auth or provider headers.
Deduplicate emails usingnormalizeEmailKey(), which trims and lowercases the email.
Use classes for state requiring multiple independent instances or dependency injection, includingAccountManager,CircuitBreaker,SessionAffinityStore, and theCodexErrorhierarchy. Reserve module-level state for genuinely process-global concerns and provide a test reset helper for such state.
Never import fromdist/in source tests or library code.
Never suppress type errors.
Never patch official Codex application binaries for desktop routing.
Never use bare recursive cleanup in Windows-sensitive paths without retry handling.
Files:
lib/storage.tslib/storage/path-state.tslib/runtime/app-bind.ts
lib/{storage/**/*.ts,storage.ts,runtime-paths.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/{storage/**/*.ts,storage.ts,runtime-paths.ts}: Resolve project storage identity withresolveProjectStorageIdentityRoot; never derive project pools directly from raw worktree paths.
Never key project storage directly by worktree path.
Files:
lib/storage.tslib/storage/path-state.ts
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/storage.tslib/storage/path-state.tslib/runtime/app-bind.ts
test/**/storage*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Test V3 storage, worktree migration, and concurrent load scenarios in storage.test.ts and storage-async.test.ts
Files:
test/storage.test.ts
lib/storage/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not key project storage by worktree path; use
resolveProjectStorageIdentityRoot.
Files:
lib/storage/path-state.ts
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.jsscripts/codex-app-router.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.jsscripts/codex-app-router.js
lib/{runtime-rotation-proxy.ts,runtime/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Runtime rotation must fail open to normal official Codex forwarding when startup helpers are unavailable.
Files:
lib/runtime/app-bind.ts
lib/runtime/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not patch official Codex app binaries; use the reversible app-bind or launcher-helper mechanisms instead.
Files:
lib/runtime/app-bind.ts
🧠 Learnings (2)
📚 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-app-router.test.tstest/codex-bin-wrapper.test.tstest/storage.test.tstest/app-bind.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-app-router.test.tstest/codex-bin-wrapper.test.tstest/storage.test.tstest/app-bind.test.ts
🪛 ast-grep (0.45.0)
test/codex-app-router.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 { spawn, spawnSync, type ChildProcessWithoutNullStreams } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
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)
lib/runtime/app-bind.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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 (13)
lib/storage.ts (1)
111-111: LGTM!Also applies to: 610-615
lib/storage/path-state.ts (1)
10-73: LGTM!test/storage.test.ts (1)
7-12: LGTM!Also applies to: 50-76
lib/runtime/app-bind.ts (4)
122-141: the new public shapesDetachedProcessStopOptionsandRuntimeRotationAppHelperStatus, plusstopDetachedProcessatlib/runtime/app-bind.ts:861andstopRuntimeRotationAppHelperProcessatlib/runtime/app-bind.ts:930, are still exported without a barrel route. add them tolib/index.tsor a documented subpath, or drop theexportkeyword and keep them internal to app-bind consumers.as per coding guidelines: "Route all public exports through
lib/index.tsor documented package subpaths."Source: Coding guidelines
693-708:runWindowsTaskkillatlib/runtime/app-bind.ts:693-708still resolves only onerrororclose, with no timer.stopDetachedProcessawaits it, andunbindCodexAppRuntimeRotationLockedatlib/runtime/app-bind.ts:1126awaits that, so a wedgedtaskkillhangs unbind on windows. the exit code is also discarded, so access-denied looks identical to a clean tree kill.
778-788: the posix branch still callsDate.parseonps -o lstart=output atlib/runtime/app-bind.ts:783. that format is not the ecmascript date-time string format, so parsing is implementation-defined. on any runtime that returnsNaN,verifyRuntimeProcessIdentityreturns false andstopRuntimeRotationAppHelperProcessskips a live helper. preferps -o lstart=replaced by an epoch-based field (for exampleps -o etimes=orlstartparsed explicitly) and add a case with reallstarttext.#!/bin/bash # check whether any test exercises the real posix identity probe output rg -n 'lstart|etimes|readProcessIdentity|verifyRuntimeProcessIdentity' test lib --type=ts
4-11: LGTM!Also applies to: 22-25, 78-78, 370-370, 383-383, 547-549, 628-683, 824-859, 913-957, 1081-1083
scripts/codex-app-router.js (1)
136-142: LGTM!Also applies to: 158-158, 242-242, 252-258, 289-295
test/app-bind.test.ts (1)
6-17: LGTM!Also applies to: 334-335, 337-361, 393-464, 466-523, 775-775, 804-807, 832-832
test/codex-app-router.test.ts (1)
135-135: LGTM!test/codex-bin-wrapper.test.ts (1)
496-499: LGTM!scripts/codex.js (2)
4161-4165: LGTM!
4179-4194: 🩺 Stability & Availabilityno change needed: helper shutdown is bounded.
waitForRuntimeRotationAppHelperExitinscripts/codex.js:4006-4017uses a 2s timeout beforestopRuntimeRotationAppHelpersendsSIGKILL, so an ignoredSIGTERMdoes not leave cleanup awaiting indefinitely. no regression case is required here.
dabb578 to
5439882
Compare
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 `@lib/runtime-rotation-proxy.ts`:
- Around line 1377-1380: In lib/runtime-rotation-proxy.ts lines 1377-1380, after
accountManager.markAuthInvalidated(), await the durable account-storage save
before sending the 401 invalidation response. In
test/runtime-rotation-proxy.test.ts lines 2497-2534, reload persisted storage
through a fresh AccountManager and assert that authInvalidatedAt and the
upstream error code are preserved.
In `@lib/runtime/app-bind.ts`:
- Around line 753-786: Update lib/runtime/app-bind.ts lines 753-786 around
runProcessIdentityProbe and unbindCodexAppRuntimeRotation to report probe
failures through options.log, distinguish unavailable probes from identity
mismatches, and increase PROCESS_IDENTITY_PROBE_TIMEOUT_MS for the Windows
PowerShell path; add an AppBindOptions identity-verifier injection seam. Update
test/app-bind.test.ts lines 715-775 to inject the verifier instead of spawning a
real child through the host ps or powershell.exe.
In `@scripts/codex.js`:
- Around line 113-121: Coerce the parsed environment-variable counters to a
finite nonnegative value when initializing
appServerShimFileCleanupBusyFailuresRemaining and
appServerShimCopyBusyFailuresRemaining. Ensure malformed values such as NaN
become zero so maybeThrowSimulatedAppServerShimFileError can terminate normally,
while preserving valid configured failure counts.
🪄 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: 20ca5b80-1fae-47ba-bf27-40addbcd32f2
📒 Files selected for processing (16)
lib/accounts.tslib/codex-manager.tslib/index.tslib/runtime-rotation-proxy.tslib/runtime/app-bind.tsscripts/codex-app-router.jsscripts/codex.jstest/accounts.test.tstest/app-bind.test.tstest/codex-bin-wrapper.test.tstest/codex-manager-selection-diagnostics.test.tstest/preemptive-quota-scheduler.test.tstest/preuninstall.test.tstest/runtime-rotation-proxy.test.tstest/storage.test.tstest/uninstall-command.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (19)
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: Route all public exports throughlib/index.tsor documented package subpaths.
Keep module dependencies acyclic and preserve the layeringtypes/constants → storage → accounts → runtime → manager/CLI; lower layers must not import higher layers.
Preserve runtime rotation pass-through semantics except for intentionally changed auth or provider headers.
Deduplicate emails usingnormalizeEmailKey(), which trims and lowercases the email.
Use classes for state requiring multiple independent instances or dependency injection, includingAccountManager,CircuitBreaker,SessionAffinityStore, and theCodexErrorhierarchy. Reserve module-level state for genuinely process-global concerns and provide a test reset helper for such state.
Never import fromdist/in source tests or library code.
Never suppress type errors.
Never patch official Codex application binaries for desktop routing.
Never use bare recursive cleanup in Windows-sensitive paths without retry handling.
Files:
lib/index.tslib/codex-manager.tslib/runtime-rotation-proxy.tslib/accounts.tslib/runtime/app-bind.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:
lib/index.tstest/storage.test.tstest/preuninstall.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-bin-wrapper.test.tstest/accounts.test.tstest/uninstall-command.test.tstest/preemptive-quota-scheduler.test.tslib/codex-manager.tslib/runtime-rotation-proxy.tsscripts/codex.jstest/runtime-rotation-proxy.test.tsscripts/codex-app-router.jslib/accounts.tslib/runtime/app-bind.tstest/app-bind.test.ts
**/*
📄 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:
lib/index.tstest/storage.test.tstest/preuninstall.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-bin-wrapper.test.tstest/accounts.test.tstest/uninstall-command.test.tstest/preemptive-quota-scheduler.test.tslib/codex-manager.tslib/runtime-rotation-proxy.tsscripts/codex.jstest/runtime-rotation-proxy.test.tsscripts/codex-app-router.jslib/accounts.tslib/runtime/app-bind.tstest/app-bind.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:
lib/index.tstest/storage.test.tstest/preuninstall.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-bin-wrapper.test.tstest/accounts.test.tstest/uninstall-command.test.tstest/preemptive-quota-scheduler.test.tslib/codex-manager.tslib/runtime-rotation-proxy.tsscripts/codex.jstest/runtime-rotation-proxy.test.tsscripts/codex-app-router.jslib/accounts.tslib/runtime/app-bind.tstest/app-bind.test.ts
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/index.tslib/codex-manager.tslib/runtime-rotation-proxy.tslib/accounts.tslib/runtime/app-bind.ts
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/storage.test.tstest/preuninstall.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-bin-wrapper.test.tstest/accounts.test.tstest/uninstall-command.test.tstest/preemptive-quota-scheduler.test.tstest/runtime-rotation-proxy.test.tstest/app-bind.test.ts
test/**/storage*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Test V3 storage, worktree migration, and concurrent load scenarios in storage.test.ts and storage-async.test.ts
Files:
test/storage.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/storage.test.tstest/preuninstall.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-bin-wrapper.test.tstest/accounts.test.tstest/uninstall-command.test.tstest/preemptive-quota-scheduler.test.tstest/runtime-rotation-proxy.test.tstest/app-bind.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/storage.test.tstest/preuninstall.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-bin-wrapper.test.tstest/accounts.test.tstest/uninstall-command.test.tstest/preemptive-quota-scheduler.test.tstest/runtime-rotation-proxy.test.tstest/app-bind.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
lib/{runtime-rotation-proxy.ts,local-bridge.ts,request/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Do not forward stale decoded
content-encodingmetadata when Node fetch has already decoded response bytes.
Files:
lib/runtime-rotation-proxy.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/{runtime-rotation-proxy.ts,local-bridge.ts}: Runtime proxy client-facing headers and responses must never expose account emails or tokens.
Never include account emails or tokens in runtime proxy client responses.
Files:
lib/runtime-rotation-proxy.ts
lib/{runtime-rotation-proxy.ts,runtime/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Runtime rotation must fail open to normal official Codex forwarding when startup helpers are unavailable.
Files:
lib/runtime-rotation-proxy.tslib/runtime/app-bind.ts
lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (AGENTS.md)
lib/runtime-rotation-proxy.ts: Keep runtime rotation enabled by default, use loopback-only networking, and use a per-process client token.
Do not expose account emails or tokens in runtime proxy response headers or logs.
The runtime proxy may forward only Responses API and model-discovery requests.
Files:
lib/runtime-rotation-proxy.ts
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.jsscripts/codex-app-router.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.jsscripts/codex-app-router.js
lib/{accounts.ts,accounts/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Maintain account health on a 0–100 scale and update it through account manager APIs.
Files:
lib/accounts.ts
lib/accounts.ts
📄 CodeRabbit inference engine (AGENTS.md)
Email deduplication must be case-insensitive using
normalizeEmailKey()(trim and lowercase).
Files:
lib/accounts.ts
lib/runtime/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not patch official Codex app binaries; use the reversible app-bind or launcher-helper mechanisms instead.
Files:
lib/runtime/app-bind.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T13:13:48.685Z
Learning: Use `~/.codex/multi-auth/settings.json` as the canonical settings file, with top-level `version`, `dashboardDisplaySettings`, and `pluginConfig` fields.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T13:13:48.685Z
Learning: Treat `pluginConfig` as the persisted compatibility name for runtime settings, and support its documented environment-variable overrides through `lib/config.ts` accessors.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T13:13:48.685Z
Learning: Do not manually edit refresh lease/state files or active shadow-home lock metadata while the CLI is running.
📚 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/storage.test.tstest/preuninstall.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-bin-wrapper.test.tstest/accounts.test.tstest/uninstall-command.test.tstest/preemptive-quota-scheduler.test.tstest/runtime-rotation-proxy.test.tstest/app-bind.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/storage.test.tstest/preuninstall.test.tstest/codex-manager-selection-diagnostics.test.tstest/codex-bin-wrapper.test.tstest/accounts.test.tstest/uninstall-command.test.tstest/preemptive-quota-scheduler.test.tstest/runtime-rotation-proxy.test.tstest/app-bind.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)
lib/runtime/app-bind.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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 OpenGrep (1.26.0)
lib/runtime/app-bind.ts
[ERROR] 846-848: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (19)
lib/accounts.ts (1)
242-244: LGTM!Also applies to: 270-271, 520-521, 650-650, 663-679, 796-796, 831-831, 889-889, 946-946, 1256-1276, 1308-1308, 1367-1384, 1452-1453, 1519-1520, 1537-1539, 1560-1573, 1622-1623
lib/codex-manager.ts (1)
2-3: LGTM!Also applies to: 66-70, 397-472
lib/index.ts (1)
36-36: LGTM!lib/runtime-rotation-proxy.ts (1)
1-1: LGTM!Also applies to: 23-26, 52-58, 170-186, 729-738, 769-769, 1092-1118, 1231-1250, 1470-1476
test/accounts.test.ts (1)
35-35: LGTM!Also applies to: 2427-2427, 2461-2505
test/codex-manager-selection-diagnostics.test.ts (1)
1-234: LGTM!test/preemptive-quota-scheduler.test.ts (1)
260-292: LGTM!test/runtime-rotation-proxy.test.ts (1)
9-14: LGTM!Also applies to: 1417-1531, 2612-2615
lib/runtime/app-bind.ts (2)
638-714: 📐 Maintainability & Code Qualitynew public exports still bypass
lib/index.ts.
stopRuntimeRotationRouterProcess,parsePosixProcessStartTime, andstopDetachedProcessare exported fromlib/runtime/app-bind.tsbut no barrel or documented subpath exposes them. this repeats the earlier finding on this file.as per coding guidelines, "Route all public exports through
lib/index.tsor documented package subpaths."Also applies to: 844-844, 1012-1017
Source: Coding guidelines
22-26: LGTM!Also applies to: 79-83, 127-149, 393-393, 557-559, 716-746, 844-884, 965-1010, 1079-1115, 1314-1429
scripts/codex-app-router.js (2)
136-144: LGTM!Also applies to: 160-166, 250-250, 260-268
296-309: 🗄️ Data Integrity & Integrationrouter argv includes
--status; no change needed.
lib/runtime/app-bind.ts:572-595spawns and regenerates startup/launch-agent args with--status "${state.statusPath}"alongside--state.scripts/codex.js (2)
203-215: LGTM!Also applies to: 3698-3726, 3904-3904, 4217-4221, 4235-4250
183-201: 🩺 Stability & Availabilityretry predicate already covers Windows file operation codes.
scripts/codex.js:34definesRETRYABLE_SHADOW_HOME_CLEANUP_CODESas["EBUSY", "EPERM", "ENOTEMPTY"], andscripts/codex.js:153proxies it throughisRetryableShadowHomeCleanupErrorused byscripts/codex.js:3698andscripts/codex.js:3714.test/app-bind.test.ts (1)
6-27: LGTM!Also applies to: 82-107, 160-173, 383-469, 471-560, 562-619, 621-713, 777-806, 837-861, 1033-1033, 1062-1065, 1090-1090
test/codex-bin-wrapper.test.ts (1)
496-499: LGTM!Also applies to: 2405-2405, 2477-2479
test/preuninstall.test.ts (1)
9-34: LGTM!Also applies to: 93-126
test/storage.test.ts (1)
7-12: LGTM!Also applies to: 50-86
test/uninstall-command.test.ts (1)
5-5: LGTM!Also applies to: 62-71, 112-128, 184-184, 263-279, 286-296, 317-317, 326-326
5439882 to
cfbec35
Compare
cfbec35 to
cf1148c
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@lib/accounts.ts`:
- Around line 1321-1325: Normalize invalidation timestamps across
lib/accounts.ts:1321-1325, lib/schemas.ts:179-180, and
lib/storage/flagged-storage.ts:85-94: update isAccountAuthInvalidated to require
a finite value greater than zero, and reject or remove zero and negative
timestamps during account-state loading and flagged-account normalization. Add
regression coverage for zero and negative timestamps, including auth rotation,
Windows filesystem I/O, and concurrency paths.
In `@lib/runtime/app-bind.ts`:
- Around line 1503-1523: Update the helper ownership decision in the runtime
rotation cleanup flow around stopRuntimeRotationAppHelperProcess so a live
helper with no identityToken can be stopped after verifyRuntimeProcessIdentity
succeeds, even when helperOwner is null. Preserve strict identityToken ownership
matching when the status includes a token, and update the existing token-less
test plus add coverage for stopping a verified live token-less helper.
In `@scripts/codex.js`:
- Around line 4168-4189: The runtime rotation helper owner token is written to a
shared fixed path before spawn, allowing concurrent launchers to overwrite one
another and orphan a running helper. Update writeRuntimeRotationAppHelperOwner
and the spawn/ready flow to use a launcher-, helper-, or PID-specific ownership
key, or persist ownership only after the helper successfully reports ready.
Preserve ownership matching in the app-binding flow, and add a regression test
covering concurrent launches that previously replaced the owner token.
In `@test/app-bind.test.ts`:
- Around line 894-924: Update the affected app-bind tests around
unbindCodexAppRuntimeRotation to use a short-lived sleeper child’s PID instead
of process.pid in runtime-helper status fixtures, and ensure the child is
cleaned up. In the stop-failure case around the replacement/ownerless fixture,
inject verifyProcessIdentity so the test does not invoke host ps or
powershell.exe; preserve the assertions that validate the intended rejection
behavior.
- Around line 824-892: Update the owned-helper test around child creation and
unbindCodexAppRuntimeRotation to await the fixture child’s exit event after
signaling it to stop, before invoking unbind. Preserve the existing cleanup
fallback for failures, and keep the status-file assertion deterministic without
relying on process reaping timing.
🪄 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: d20e1574-15c3-445d-8599-4bee9ced217f
📒 Files selected for processing (13)
README.mddocs/reference/storage-paths.mdlib/accounts.tslib/runtime-constants.tslib/runtime-rotation-proxy.tslib/runtime/app-bind.tslib/schemas.tslib/storage/flagged-storage.tslib/storage/public-types.tsscripts/codex-app-router.jsscripts/codex.jstest/app-bind.test.tstest/runtime-rotation-proxy.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (23)
**/*
📄 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:
README.mdlib/runtime-constants.tslib/storage/public-types.tsdocs/reference/storage-paths.mdscripts/codex-app-router.jstest/runtime-rotation-proxy.test.tslib/schemas.tsscripts/codex.jslib/runtime-rotation-proxy.tstest/app-bind.test.tslib/storage/flagged-storage.tslib/accounts.tslib/runtime/app-bind.ts
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: Route all public exports throughlib/index.tsor documented package subpaths.
Keep module dependencies acyclic and preserve the layeringtypes/constants → storage → accounts → runtime → manager/CLI; lower layers must not import higher layers.
Preserve runtime rotation pass-through semantics except for intentionally changed auth or provider headers.
Deduplicate emails usingnormalizeEmailKey(), which trims and lowercases the email.
Use classes for state requiring multiple independent instances or dependency injection, includingAccountManager,CircuitBreaker,SessionAffinityStore, and theCodexErrorhierarchy. Reserve module-level state for genuinely process-global concerns and provide a test reset helper for such state.
Never import fromdist/in source tests or library code.
Never suppress type errors.
Never patch official Codex application binaries for desktop routing.
Never use bare recursive cleanup in Windows-sensitive paths without retry handling.
Files:
lib/runtime-constants.tslib/storage/public-types.tslib/schemas.tslib/runtime-rotation-proxy.tslib/storage/flagged-storage.tslib/accounts.tslib/runtime/app-bind.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:
lib/runtime-constants.tslib/storage/public-types.tsscripts/codex-app-router.jstest/runtime-rotation-proxy.test.tslib/schemas.tsscripts/codex.jslib/runtime-rotation-proxy.tstest/app-bind.test.tslib/storage/flagged-storage.tslib/accounts.tslib/runtime/app-bind.ts
**/*.{js,ts,mjs,cjs}
📄 CodeRabbit inference engine (README.md)
**/*.{js,ts,mjs,cjs}: Keep OAuth credentials and account state local; runtime rotation and local bridges must be loopback-only.
Do not patch official Codex app binaries; use reversible packaged app-bind and user-level launcher helpers instead.
Protect local bridge access with hashed local client tokens.
Use statelessstore=falseResponses routing by default; enable statefulstore=trueonly when callers explicitly opt intobackground: truecompatibility.
Never run npm install or update commands automatically; version checks may only display a manual notice on an interactive TTY or whenCODEX_MULTI_AUTH_DEBUG=1.
Experimental synchronization and backup flows must be non-destructive by default: preview before applying, preserve destination-only accounts, and fail safely on filename collisions.
Bound forwarded request retries and account rotation; disable whole-pool replay when all accounts are rate-limited and apply cooldowns after repeated cross-account 5xx bursts.
Stagger proactive credential refreshes to reduce background refresh bursts.
Files:
lib/runtime-constants.tslib/storage/public-types.tsscripts/codex-app-router.jstest/runtime-rotation-proxy.test.tslib/schemas.tsscripts/codex.jslib/runtime-rotation-proxy.tstest/app-bind.test.tslib/storage/flagged-storage.tslib/accounts.tslib/runtime/app-bind.ts
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/runtime-constants.tslib/storage/public-types.tslib/schemas.tslib/runtime-rotation-proxy.tslib/storage/flagged-storage.tslib/accounts.tslib/runtime/app-bind.ts
lib/{storage/**/*.ts,storage.ts,runtime-paths.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/{storage/**/*.ts,storage.ts,runtime-paths.ts}: Resolve project storage identity withresolveProjectStorageIdentityRoot; never derive project pools directly from raw worktree paths.
Never key project storage directly by worktree path.
Files:
lib/storage/public-types.tslib/storage/flagged-storage.ts
lib/storage/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not key project storage by worktree path; use
resolveProjectStorageIdentityRoot.
Files:
lib/storage/public-types.tslib/storage/flagged-storage.ts
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/storage-paths.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/storage-paths.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/storage-paths.md
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/storage-paths.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-app-router.jsscripts/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-app-router.jsscripts/codex.js
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/runtime-rotation-proxy.test.tstest/app-bind.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/runtime-rotation-proxy.test.tstest/app-bind.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/runtime-rotation-proxy.test.tstest/app-bind.test.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts,request/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Do not forward stale decoded
content-encodingmetadata when Node fetch has already decoded response bytes.
Files:
lib/runtime-rotation-proxy.ts
lib/{runtime-rotation-proxy.ts,local-bridge.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/{runtime-rotation-proxy.ts,local-bridge.ts}: Runtime proxy client-facing headers and responses must never expose account emails or tokens.
Never include account emails or tokens in runtime proxy client responses.
Files:
lib/runtime-rotation-proxy.ts
lib/{runtime-rotation-proxy.ts,runtime/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Runtime rotation must fail open to normal official Codex forwarding when startup helpers are unavailable.
Files:
lib/runtime-rotation-proxy.tslib/runtime/app-bind.ts
lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (AGENTS.md)
lib/runtime-rotation-proxy.ts: Keep runtime rotation enabled by default, use loopback-only networking, and use a per-process client token.
Do not expose account emails or tokens in runtime proxy response headers or logs.
The runtime proxy may forward only Responses API and model-discovery requests.
Files:
lib/runtime-rotation-proxy.ts
lib/{accounts.ts,accounts/**/*.ts}
📄 CodeRabbit inference engine (lib/AGENTS.md)
Maintain account health on a 0–100 scale and update it through account manager APIs.
Files:
lib/accounts.ts
lib/accounts.ts
📄 CodeRabbit inference engine (AGENTS.md)
Email deduplication must be case-insensitive using
normalizeEmailKey()(trim and lowercase).
Files:
lib/accounts.ts
lib/runtime/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not patch official Codex app binaries; use the reversible app-bind or launcher-helper mechanisms instead.
Files:
lib/runtime/app-bind.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T13:59:48.826Z
Learning: Use the official OpenAI installation path for the `codex` command and invoke this package through `codex-multi-auth` or the intentional forwarding wrappers.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T13:59:48.826Z
Learning: The package is intended for personal development use; production or commercial workloads should use the OpenAI Platform API.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T13:59:57.574Z
Learning: Do not manually edit refresh lease/state files while the CLI is running.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T13:59:57.574Z
Learning: Existing stateless pipelines should leave `backgroundResponses` disabled; enable it only for callers that require stateful background responses and validate one end-to-end request before broad rollout.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Treat `~/.codex/multi-auth` contents as project-managed, while `~/.codex/accounts.json`, `~/.codex/auth.json`, and `~/.codex/config.toml` remain official Codex CLI files; do not overwrite unrelated ownership boundaries.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Never read or write the OS keychain or `security` CLI; use the file-backed Codex auth store and preserve official CLI file locations.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Reconcile the persisted top-level `cli_auth_credentials_store` value to `"file"` at first-run setup, wrapper startup, and `doctor --fix`, while leaving profile-level values unchanged and preserving line endings and accepted TOML quote styles.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Honor `CODEX_MULTI_AUTH_FORCE_FILE_AUTH_STORE=0` and `CODEX_MULTI_AUTH_ENFORCE_CLI_FILE_AUTH_STORE=0` with their documented scopes: the former disables wrapper injection and startup reconciliation; the latter disables config rewrites while retaining per-invocation overrides.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Use atomic writes and appropriate retry handling for configuration and account state; failures during first-run setup or wrapper reconciliation must be swallowed or debug-logged so they do not block the user command.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Exclude `.reset-intent` markers and cache-like artifacts from recovery candidates; suppress flagged-account backup recovery while the flagged reset marker remains present.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Named backup exports must use only letters, numbers, `_`, and `-`; reject path separators, `..`, `.rotate.`, `.tmp`, and `.wal`, append `.json` when omitted, and do not overwrite existing files except through an explicit lower-level force path.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: The local bridge must be loopback-only, expose only `/health`, `/v1/models`, and `/v1/responses`, and persist token hashes rather than plaintext tokens; plaintext tokens may be shown only during create or rotate commands.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Enforce pause and drain entries from `account-policies.json` at selection time through `evaluateRuntimePolicy`, excluding blocked accounts from hybrid rotation.
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-08-08T14:00:08.875Z
Learning: Maintain compatibility paths only for migration reads; canonical new storage must use the documented `~/.codex/multi-auth` layout and its canonical filenames.
📚 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/runtime-rotation-proxy.test.tstest/app-bind.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/runtime-rotation-proxy.test.tstest/app-bind.test.ts
🪛 ast-grep (0.45.0)
lib/runtime/app-bind.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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } 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 { spawn } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 OpenGrep (1.26.0)
lib/runtime/app-bind.ts
[ERROR] 936-938: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (31)
README.md (1)
261-261: LGTM!docs/reference/storage-paths.md (1)
43-43: LGTM!lib/accounts.ts (1)
2-2: LGTM!Also applies to: 113-142, 273-303, 528-556, 570-577, 612-619, 701-701, 714-730, 847-847, 882-882, 940-940, 997-997, 1307-1319, 1359-1359, 1418-1435, 1486-1505, 1571-1625, 1674-1675
lib/schemas.ts (1)
164-164: LGTM!Also applies to: 180-180
lib/storage/flagged-storage.ts (1)
96-100: LGTM!Also applies to: 138-139
lib/storage/public-types.ts (1)
25-26: LGTM!Also applies to: 49-52
lib/runtime-rotation-proxy.ts (1)
1-1: LGTM!Also applies to: 23-26, 52-58, 170-199, 742-782, 1105-1131, 1244-1263, 1390-1403, 1486-1492
test/runtime-rotation-proxy.test.ts (1)
9-14: LGTM!Also applies to: 1417-1603, 2569-2626, 2703-2706
lib/runtime-constants.ts (1)
6-9: LGTM!lib/runtime/app-bind.ts (12)
4-14: LGTM!Also applies to: 25-30, 74-75, 85-91, 122-128, 140-169
323-323: LGTM!Also applies to: 358-358, 402-405, 418-418
518-518: LGTM!Also applies to: 538-539, 611-612
584-586: LGTM!
667-757: LGTM!
759-789: LGTM!
796-916: LGTM!
918-1000: LGTM!
1002-1088: LGTM!
1090-1202: LGTM!
1204-1301: LGTM!
1380-1380: LGTM!Also applies to: 1426-1432, 1474-1502, 1524-1550, 1593-1614
scripts/codex-app-router.js (1)
39-39: LGTM!Also applies to: 62-66, 142-175, 259-259, 269-278, 309-319
scripts/codex.js (4)
83-84: LGTM!Also applies to: 115-125, 138-154, 192-223
3707-3735: LGTM!
3779-3783: LGTM!Also applies to: 3897-3912, 3937-3938, 3956-3956, 3973-3973
4259-4263: LGTM!Also applies to: 4277-4292, 5295-5295
test/app-bind.test.ts (5)
6-28: LGTM!Also applies to: 83-138, 191-204
372-372: LGTM!Also applies to: 389-390, 414-415
417-728: LGTM!
730-822: LGTM!
926-955: LGTM!Also applies to: 986-1010, 1182-1182, 1211-1214, 1239-1239
cf1148c to
0f6038a
Compare
| if (!verified) { | ||
| return false; | ||
| } | ||
| return stopDetachedProcess(router.pid, platform, options); |
There was a problem hiding this comment.
if the verified router exits and its pid is reused before stopDetachedProcess signals it, the replacement process is terminated because ownership is not revalidated after the fresh liveness check. on windows, taskkill /t /f terminates the replacement process and its entire process tree.
Knowledge Base Used: Runtime Services
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/runtime/app-bind.ts
Line: 747
Comment:
**pid reuse bypasses ownership verification**
if the verified router exits and its pid is reused before `stopDetachedProcess` signals it, the replacement process is terminated because ownership is not revalidated after the fresh liveness check. on windows, `taskkill /t /f` terminates the replacement process and its entire process tree.
**Knowledge Base Used:** [Runtime Services](https://app.greptile.com/zeian/-/custom-context/knowledge-base/ndycode/codex-multi-auth/-/docs/runtime-services.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
1 participant
Summary
This PR implements and tests the five related account-management and runtime-rotation issues:
why-selectedawaits live selection and applies the same runtime availability gates, policy blocks, and score boosts as production routing.Follow-up validation fixes included in this PR:
AsyncLocalStoragecontexts created by earlier async work.codex.execopy instead of a hard link to the running Node image, allowing graceful cleanup withoutEPERMlocks.Verification
npm run build✅npm run typecheck✅npm run typecheck:scripts✅npm run lint:ts✅npm run lint:scripts✅git diff --check✅npm test✅ — 337 test files passed, 1 skipped; 5,313 tests passed, 4 skippedFixes #652
Fixes #653
Fixes #654
Fixes #655
Fixes #656
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
this pr addresses account rotation, token invalidation, quota scheduling, oauth guidance, uninstall cleanup, and runtime process ownership.
Confidence Score: 4/5
the pr is not yet safe to merge because unbind can still terminate an unrelated process if pid reuse occurs between ownership verification and signaling.
process ownership is checked before a separate pid-only stop operation, leaving a concurrency window where a replacement process can receive sigterm or sigkill and, on windows, taskkill can terminate its full process tree.
Files Needing Attention: lib/runtime/app-bind.ts
Important Files Changed
Sequence Diagram
Prompt To Fix All With AI
Reviews (10): Last reviewed commit: "fix: resolve issues 652 through 656" | Re-trigger Greptile
Context used (3)