fix(security,resilience): pre-release hardening from deep stress test#503
Conversation
Found by a whole-tree adversarial audit + runtime fuzz of the merged main (both #499 and #500). The tree was already well-hardened — these are the residual MEDIUM/LOW gaps, all verified. Egress hardening (never forward inbound client credentials upstream): - runtime-rotation-proxy.ts + local-bridge.ts: strip inbound `cookie` and `proxy-authorization` on the outbound request, alongside the existing authorization/x-api-key stripping. A client cookie would otherwise ride upstream with the managed OAuth Bearer. - runtime-rotation-proxy.ts readErrorBody: the outbound fetch's abort timer is cleared once headers arrive, so a stalled ERROR body (429/5xx path) could hang the handler forever — the success path is per-chunk stall-bounded, the error path was not. Read it via a reader with a per-read idle timeout + a 1MB cap, cancelling the stream on stall/overflow. Windows fs resilience (close the transient-lock gaps the fuzz reproduced): - storage.ts: the secret-bearing WAL write and temp write had NO retry, and the temp cleanup was single-shot — a transient EBUSY/EPERM could fail a valid save or strand a token-bearing *.tmp. Wrap all three in withFileOperationRetry. Also align copyFileWithRetry / renameFileWithRetry to the shared shouldRetryFileOperation taxonomy (adds ENOTEMPTY/EACCES/EAGAIN). The primary commit rename keeps its existing EPERM/EBUSY contract unchanged. - quota-cache.ts: widen RETRYABLE_FS_CODES to the shared set. Least-privilege: - runtime/runtime-observability.ts: persist the snapshot (account id/label) with mode 0o600 and the dir 0o700, matching the other sensitive writers (no-op on win32, owner-only on POSIX). Tests: cookie strip on both egress paths; existing storage WAL/temp/backup retry coverage still green (rename contract unchanged). Also cleaned a stale eslint-disable + unused import in the merged mcodex statusline test. typecheck + lint clean; full suite 4246 passed / 1 skipped.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Review limit reached
More reviews will be available in 3 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughthis pr strips inbound credential headers, bounds upstream error-body reads, consolidates filesystem retry logic, tightens runtime-observability permissions, and updates tests/docs and vitest dev deps. Changesruntime & storage hardening
Sequence DiagramsequenceDiagram
participant client
participant bridge as lib/local-bridge.ts
participant proxy as lib/runtime-rotation-proxy.ts
participant upstream
client->>bridge: request (includes cookie / proxy-authorization)
bridge->>proxy: forwarded request (cookie and proxy-authorization removed)
proxy->>upstream: outbound request with managed Authorization only
upstream-->>proxy: error response (streamable body)
proxy->>proxy: readErrorBody(upstream, streamStallTimeoutMs, maxBytes?)
proxy-->>client: proxied error response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes missing items (explicit):
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
test/runtime-rotation-proxy.test.ts (1)
1120-1135:⚠️ Potential issue | 🟠 Major | ⚡ Quick winassert
proxy-authorizationis stripped on the proxy path.
lib/runtime-rotation-proxy.ts:271-274stripsproxy-authorizationas part of the new hardening, buttest/runtime-rotation-proxy.test.ts:1120-1135only checkscookie. addproxy-authorizationto this request and assert it isnullupstream so this security change cannot regress silently.as per coding guidelines, "test/**: 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."
🤖 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 `@test/runtime-rotation-proxy.test.ts` around lines 1120 - 1135, The test is missing an assertion that the proxy strips the "proxy-authorization" header; update the request in test/runtime-rotation-proxy.test.ts (the block that builds the inbound request before the fetch and the expectations after) to include a "proxy-authorization" header value and add an assertion that calls[0]?.headers.get("proxy-authorization") is null so the hardening in lib/runtime-rotation-proxy.ts (the proxy-authorization stripping logic) is covered and cannot regress.test/local-bridge.test.ts (1)
316-329:⚠️ Potential issue | 🟠 Major | ⚡ Quick wincover
proxy-authorizationstripping too.
lib/local-bridge.ts:101-104now strips bothcookieandproxy-authorization, buttest/local-bridge.test.ts:316-329only proves thecookiebranch. addproxy-authorizationto this request and assert it is absent on the forwarded headers so the new egress hardening is fully pinned by vitest.as per coding guidelines, "lib/**: 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."
🤖 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 `@test/local-bridge.test.ts` around lines 316 - 329, Add an assertion to the existing test that proves the bridge strips the proxy-authorization header: include "proxy-authorization" in the fetch request headers (alongside "authorization", "x-api-key", and "cookie") and then assert that headers.get("proxy-authorization") is null on the forwarded request; use the existing variables (server.baseUrl, calls, forwarded) and the same pattern as the existing cookie/x-api-key checks so the test pins the new stripping behavior implemented in lib/local-bridge.ts (the code that strips cookie and proxy-authorization).lib/runtime-rotation-proxy.ts (1)
899-958:⚠️ Potential issue | 🟠 Major | ⚡ Quick winadd a stalled-error-body regression.
lib/runtime-rotation-proxy.ts:899-958changes the failure semantics for 429/402-403/401/5xx handling, buttest/runtime-rotation-proxy.test.tsdoes not include a case that returns a never-finishing error-body stream and proves the proxy times out and returns instead of hanging. this is the core resilience contract of this change, especially for the 429 path reached fromlib/runtime-rotation-proxy.ts:1793-1816.as per coding guidelines, "lib/**: 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."
🤖 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-rotation-proxy.ts` around lines 899 - 958, The change to readErrorBody alters failure semantics by timing out stalled error-body streams but there is no test proving the proxy doesn't hang on a never-finishing error body (especially for the 429 path); add a vitest in test/runtime-rotation-proxy.test.ts that mocks a Response whose body.getReader().read() never resolves (or delays past the timeout), invokes the proxy path that hits the 429 handling (the code that calls readErrorBody), advances timers if using fake timers, and asserts the proxy returns the timeout/fallback response (or an error) rather than hanging; reference readErrorBody and the 429 handling path in lib/runtime-rotation-proxy.ts when adding the test and ensure the test covers both stream-based bodies and the text() fallback.lib/storage.ts (1)
277-299: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winadd vitest coverage for the widened retry taxonomy.
lib/storage.ts:277,lib/storage.ts:299,lib/storage.ts:1838,lib/storage.ts:1846, andlib/storage.ts:1887now rely on the shared retry set instead of the older narrow checks. the reviewed patch does not include atest/...regression foreagain,enotempty, oreacceson backup copy/rename, wal writes, temp writes, or temp cleanup, so a windows lock regression here will be easy to miss. please add deterministic cases that force those fs calls through the new codes and assert the save path still completes or degrades exactly as intended.As per coding guidelines,
lib/**: "focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios."Also applies to: 1835-1887
🤖 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/storage.ts` around lines 277 - 299, Add deterministic vitest unit tests that mock the filesystem calls exercised by the shared retry taxonomy to cover the widened error set (eagain, enotempty, eacces and EBUSY/429) for the affected operations: the backup copy flow (the function(s) that call shouldRetryFileOperation and use BACKUP_COPY_MAX_ATTEMPTS / BACKUP_COPY_BASE_DELAY_MS), renameFileWithRetry, WAL write logic, temp write logic, and temp cleanup logic; each test should stub fs.copyFile/fs.rename/fs.writeFile/fs.unlink/fs.rmdir (or the module wrappers used) to throw the targeted error codes on the first N attempts and succeed (or fail permanently) to assert the retry behavior, attempt counts, and final outcome, using vitest fake timers or mocks to make delays deterministic and asserting that the save path completes or degrades exactly as intended and that EBUSY/429 are handled by the new queues.
🤖 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/quota-cache.ts`:
- Around line 35-44: Add deterministic vitest regression tests that exercise the
expanded RETRYABLE_FS_CODES behavior and queue ordering: create one test that
stubs the function that reads the quota cache (the read path that consults
RETRYABLE_FS_CODES) to throw EAGAIN once then succeed and assert the read
retried and returned the result; create one test that stubs the rename/replace
path (the rename operation consulted against RETRYABLE_FS_CODES) to throw
ENOTEMPTY or EACCES once then succeed and assert the rename retried; and create
one concurrency test for the write queue (the enqueue/process queue functions in
quota-cache that implement backoff and last-write-wins) that uses vitest fake
timers and stubs for the underlying fs write to force a backoff on an earlier
write while a later write arrives, then assert the later write wins (final
on-disk state matches last update) and queue ordering is preserved. In all tests
use vitest mocks/spies to simulate errors by throwing Error objects with a .code
property matching "EAGAIN","ENOTEMPTY","EACCES", advance timers to drive retries
deterministically, and assert retry counts and final outcomes; reference
RETRYABLE_FS_CODES in test names/comments to tie them to the change.
In `@lib/runtime/runtime-observability.ts`:
- Around line 198-210: The mkdir call using mode 0o700 doesn't change
permissions for pre-existing dirs; after await fs.mkdir(dir, { recursive: true,
mode: 0o700 }) add an explicit await fs.chmod(dir, 0o700) wrapped in a small
retry loop to tolerate transient races (e.g., 2-3 attempts with a short backoff)
and ignore ENOENT/EPERM races where another process is concurrently
creating/removing the dir; place this immediately before writing the snapshot
(around the code that references dir and snapshot/tempPath) and log or capture
any non-transient errors. Also add a regression test that pre-creates
CODEX_MULTI_AUTH_DIR with permissive perms, runs the snapshot persistence path
(the code that writes tempPath and moves it), and then asserts the directory
mode is hardened to 0o700.
---
Outside diff comments:
In `@lib/runtime-rotation-proxy.ts`:
- Around line 899-958: The change to readErrorBody alters failure semantics by
timing out stalled error-body streams but there is no test proving the proxy
doesn't hang on a never-finishing error body (especially for the 429 path); add
a vitest in test/runtime-rotation-proxy.test.ts that mocks a Response whose
body.getReader().read() never resolves (or delays past the timeout), invokes the
proxy path that hits the 429 handling (the code that calls readErrorBody),
advances timers if using fake timers, and asserts the proxy returns the
timeout/fallback response (or an error) rather than hanging; reference
readErrorBody and the 429 handling path in lib/runtime-rotation-proxy.ts when
adding the test and ensure the test covers both stream-based bodies and the
text() fallback.
In `@lib/storage.ts`:
- Around line 277-299: Add deterministic vitest unit tests that mock the
filesystem calls exercised by the shared retry taxonomy to cover the widened
error set (eagain, enotempty, eacces and EBUSY/429) for the affected operations:
the backup copy flow (the function(s) that call shouldRetryFileOperation and use
BACKUP_COPY_MAX_ATTEMPTS / BACKUP_COPY_BASE_DELAY_MS), renameFileWithRetry, WAL
write logic, temp write logic, and temp cleanup logic; each test should stub
fs.copyFile/fs.rename/fs.writeFile/fs.unlink/fs.rmdir (or the module wrappers
used) to throw the targeted error codes on the first N attempts and succeed (or
fail permanently) to assert the retry behavior, attempt counts, and final
outcome, using vitest fake timers or mocks to make delays deterministic and
asserting that the save path completes or degrades exactly as intended and that
EBUSY/429 are handled by the new queues.
In `@test/local-bridge.test.ts`:
- Around line 316-329: Add an assertion to the existing test that proves the
bridge strips the proxy-authorization header: include "proxy-authorization" in
the fetch request headers (alongside "authorization", "x-api-key", and "cookie")
and then assert that headers.get("proxy-authorization") is null on the forwarded
request; use the existing variables (server.baseUrl, calls, forwarded) and the
same pattern as the existing cookie/x-api-key checks so the test pins the new
stripping behavior implemented in lib/local-bridge.ts (the code that strips
cookie and proxy-authorization).
In `@test/runtime-rotation-proxy.test.ts`:
- Around line 1120-1135: The test is missing an assertion that the proxy strips
the "proxy-authorization" header; update the request in
test/runtime-rotation-proxy.test.ts (the block that builds the inbound request
before the fetch and the expectations after) to include a "proxy-authorization"
header value and add an assertion that
calls[0]?.headers.get("proxy-authorization") is null so the hardening in
lib/runtime-rotation-proxy.ts (the proxy-authorization stripping logic) is
covered and cannot regress.
🪄 Autofix (Beta)
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
Run ID: 87845a4e-31a7-4d84-8608-550830c28baa
📒 Files selected for processing (8)
lib/local-bridge.tslib/quota-cache.tslib/runtime-rotation-proxy.tslib/runtime/runtime-observability.tslib/storage.tstest/local-bridge.test.tstest/mcodex-statusline-scope.test.tstest/runtime-rotation-proxy.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (12)
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: All public exports should flow throughlib/index.tsor documented package subpaths
Never import fromdist/in source tests or library code
Never suppress type errors
Files:
lib/quota-cache.tslib/runtime/runtime-observability.tslib/local-bridge.tslib/storage.tslib/runtime-rotation-proxy.ts
{index.ts,lib/**,scripts/**}
📄 CodeRabbit inference engine (AGENTS.md)
Source code must be located in root
index.ts,lib/, andscripts/directories;dist/is generated output and should not be edited
Files:
lib/quota-cache.tslib/runtime/runtime-observability.tslib/local-bridge.tslib/storage.tslib/runtime-rotation-proxy.ts
**/*.{js,ts,mjs,mts}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only (
"type": "module"), Node >= 18
Files:
lib/quota-cache.tslib/runtime/runtime-observability.tstest/local-bridge.test.tslib/local-bridge.tstest/mcodex-statusline-scope.test.tstest/runtime-rotation-proxy.test.tslib/storage.tslib/runtime-rotation-proxy.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
as any,@ts-ignore, or@ts-expect-errortype assertions
Files:
lib/quota-cache.tslib/runtime/runtime-observability.tstest/local-bridge.test.tslib/local-bridge.tstest/mcodex-statusline-scope.test.tstest/runtime-rotation-proxy.test.tslib/storage.tslib/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/quota-cache.tslib/runtime/runtime-observability.tslib/local-bridge.tslib/storage.tslib/runtime-rotation-proxy.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Vitest globals (describe,it,expect) are enabled and should be used without explicit imports
Maintain 80% coverage threshold across statements, branches, functions, and lines
UseremoveWithRetryfor Windows filesystem cleanup instead of barefs.rmto handle EBUSY/EPERM/ENOTEMPTY backoff
Use source files in tests, not compileddist/files; test the source directly
Do not skip tests without justification; include rationale if a test must be skipped
Relax ESLint rules for test files as specified ineslint.config.js
Files:
test/local-bridge.test.tstest/mcodex-statusline-scope.test.tstest/runtime-rotation-proxy.test.ts
{scripts/**,test/**}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use bare recursive delete logic in Windows-sensitive scripts/tests without retry handling
Files:
test/local-bridge.test.tstest/mcodex-statusline-scope.test.tstest/runtime-rotation-proxy.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/local-bridge.test.tstest/mcodex-statusline-scope.test.tstest/runtime-rotation-proxy.test.ts
{lib/storage/**,lib/storage.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Do not key project storage by worktree path; use
resolveProjectStorageIdentityRoot
Files:
lib/storage.ts
{lib/storage/**,lib/storage.ts,lib/config.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Local project-owned state defaults to
~/.codex/multi-auth; official Codex state remains under~/.codex
Files:
lib/storage.ts
lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/runtime-rotation-proxy.ts: Runtime rotation code must preserve pass-through semantics except for auth/provider headers that intentionally change
Runtime proxy client-facing headers must not expose account emails or tokens
Runtime rotation should fail open to normal official Codex forwarding when startup helpers are unavailable
Never add account emails/tokens to runtime proxy client responses
lib/runtime-rotation-proxy.ts: Do not expose account emails or tokens in runtime proxy client response headers or logs
The runtime proxy is loopback-only and uses a per-process client token, forwarding only Responses API and model discovery requests
Files:
lib/runtime-rotation-proxy.ts
{lib/runtime-rotation-proxy.ts,lib/codex-manager/commands/rotation.ts,lib/config.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Runtime rotation is default-on through
codexRuntimeRotationProxy; users can opt out withcodex-multi-auth rotation disableorCODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0
Files:
lib/runtime-rotation-proxy.ts
🔇 Additional comments (1)
test/mcodex-statusline-scope.test.ts (1)
3-3: LGTM!Also applies to: 85-86
- Bump vitest / @vitest/ui / @vitest/coverage-v8 to ^4.1.8 (dev-only), clearing GHSA-5xrq-8626-4rwp so `npm run audit:ci` exits 0. audit:prod was already clean (nothing test-related ships); this unblocks the CI release gate. - Version → 2.1.13-beta.3. - Add docs/releases/v2.1.13-beta.3.md covering the Phase 1 audit (#499), the mcodex launcher + statusline (#500), and the pre-release hardening (#503). Update the docs portal + root README prerelease pointers (documentation.test.ts parity). typecheck + lint + audit:ci clean; full suite 4246 passed / 1 skipped on vitest 4.1.8; documentation parity 25/25.
|
Folded in the two follow-ups from the PR description:
Full suite 4246 passed / 1 skipped on vitest 4.1.8; typecheck + lint + audit:ci clean. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/releases/v2.1.13-beta.3.md (1)
1-108:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winfix word consistency: "prerelease" vs "pre-release".
lines 3-10 use "prerelease" in the heading and body, but line 4 uses "pre-release hardening" and line 97 repeats "pre-release hardening" in the heading. pick one variant and use it throughout.
✍️ proposed fix for consistency
Beta prerelease that lands the Phase 1 correctness/security audit (`#499`), the new `mcodex` launcher + cached statusline (`#500`), and a round of pre-release -hardening surfaced by a whole-tree stress test (`#503`). It carries forward the +prerelease hardening surfaced by a whole-tree stress test (`#503`). It carries forward the cascade OAuth token-invalidation fix from v2.1.13-beta.2, the multi-workspace support from v2.1.13-beta.1, and the pinned-account 503 diagnostic from v2.1.13-beta.0.-## Pre-release hardening (`#503`) +## Prerelease hardening (`#503`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/releases/v2.1.13-beta.3.md` around lines 1 - 108, The document mixes "prerelease" and "pre-release"; standardize to a single variant (use "prerelease") by replacing the occurrences of "pre-release hardening" and any other "pre-release" instances with "prerelease" so the heading "Pre-release hardening (`#503`)" and the body text (e.g., the sentence "This is a **prerelease**. Stable `v2.1.13`..." and the phrase "pre-release hardening surfaced by...") consistently use "prerelease".
🤖 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.
Outside diff comments:
In `@docs/releases/v2.1.13-beta.3.md`:
- Around line 1-108: The document mixes "prerelease" and "pre-release";
standardize to a single variant (use "prerelease") by replacing the occurrences
of "pre-release hardening" and any other "pre-release" instances with
"prerelease" so the heading "Pre-release hardening (`#503`)" and the body text
(e.g., the sentence "This is a **prerelease**. Stable `v2.1.13`..." and the
phrase "pre-release hardening surfaced by...") consistently use "prerelease".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ed2a391a-1fd8-4bcb-b4f5-481a20542e0c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
README.mddocs/README.mddocs/releases/v2.1.13-beta.3.mdpackage.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (13)
{package.json,scripts/codex-multi-auth.js,scripts/codex-routing.js}
📄 CodeRabbit inference engine (AGENTS.md)
Canonical package name is
codex-multi-authand canonical command family iscodex-multi-auth ...
Files:
package.json
{package.json,scripts/codex.js,scripts/codex-routing.js}
📄 CodeRabbit inference engine (AGENTS.md)
The package does not publish a global
codexbin;codex-multi-auth-codexis the explicit wrapper forwarding non-auth commands to official Codex
Files:
package.json
package.json
📄 CodeRabbit inference engine (SECURITY.md)
package.json: Pinhonoto4.12.18or higher (but below4.12.0-4.12.1) to avoid GHSA-xh87-mx6m-69f3 authentication bypass vulnerability
Pinrollupto^4.59.0or higher to avoid vulnerable versions below4.59.0in Vite and Vitest transitive dependencies
Files:
package.json
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 documentation
Files:
docs/releases/v2.1.13-beta.3.mddocs/README.md
docs/**/releases/v*.md
📄 CodeRabbit inference engine (docs/README.md)
Release notes should be organized in a releases directory with versioned files following the naming pattern releases/v[version].md, with beta releases clearly marked
Files:
docs/releases/v2.1.13-beta.3.md
docs/**/releases/*.md
📄 CodeRabbit inference engine (docs/README.md)
Release notes should include release-specific prerelease indicators (e.g., '-beta.3') and link to installation instructions for beta versions (e.g., npm i -g codex-multi-auth@beta)
Files:
docs/releases/v2.1.13-beta.3.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/releases/v2.1.13-beta.3.mddocs/README.md
docs/{README.md,docs/**/*.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
docs/{README.md,docs/**/*.md}: Use canonical package namecodex-multi-authin all documentation
Verify internal links in documentation are valid before merge
Ensure no conflicting guidance exists between README, docs, and governance files before merge
Files:
docs/README.md
docs/{README.md,docs/getting-started.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Update
README.mdanddocs/getting-started.mdfirst when runtime behavior changes
Files:
docs/README.md
docs/{README.md,docs/getting-started.md,docs/configuration.md,docs/troubleshooting.md,docs/reference/**/*.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Verify every documented command is executable as written before merge
Files:
docs/README.md
docs/{docs/**/*.md,README.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Verify paths in documentation match runtime modules (
lib/runtime-paths.ts,lib/storage.ts,lib/config.ts)
Files:
docs/README.md
docs/{README.md,docs/index.md,docs/README.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Use accurate discoverability terms in public landing pages without keyword stuffing or ranking promises
Root README and docs landing pages should naturally include
Codex CLI,multi-account OAuth,account switching,health checks,runtime rotation,diagnostics, andrecoverywhen those topics are in scope
Files:
docs/README.md
docs/{README.md,package.json,docs/development/GITHUB_DISCOVERABILITY.md}
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
Keep the repository description, package description, README lead, and
docs/development/GITHUB_DISCOVERABILITY.mdaligned
Files:
docs/README.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Prefer `npm i -g codex-multi-auth` for standard installation; migrate from legacy scoped prerelease `ndycode/codex-multi-auth` by uninstalling first then installing the new package
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Use `codex-multi-auth login` to add or open the account menu; use `--device-auth` flag for headless shells and `--manual` flag only as a fallback when browser launch or callback is blocked
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Store credentials locally under `~/.codex/multi-auth/` directory structure (override with `CODEX_MULTI_AUTH_DIR` environment variable); use project-scoped accounts under `~/.codex/multi-auth/projects/<project-key>/` for repo-specific workflows
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Enable `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=1` (default-on) for live account rotation inside forwarded Codex CLI/app sessions; disable with `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0` if needed
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Enable `Responses` background mode (`CODEX_AUTH_BACKGROUND_RESPONSES=1` or `backgroundResponses` in settings) only for callers that intentionally send `background: true`, as this switches from stateless `store=false` to stateful `store=true` routing
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Run `codex-multi-auth doctor --fix` as first troubleshooting step; use `codex-multi-auth check` and `codex-multi-auth forecast --live` to validate state after changes
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Disable whole-pool replay by default when every account is rate-limited; use bounded outbound request budget to prevent one prompt from walking the full account pool indefinitely
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Trigger a short cooldown instead of continuing aggressive rotation after repeated cross-account 5xx bursts; stagger proactive refresh to reduce background refresh bursts
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Keep `codex` command owned by the official OpenAI install path; do not publish a global `codex` binary from this package
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:15.142Z
Learning: Use optional `codex-multi-auth-codex` forwarding wrapper only when you intentionally want this package's wrapper behavior; handle `auth ...` subcommands locally and forward all other commands to the official Codex CLI
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:44.161Z
Learning: Maintain comprehensive test coverage with 4,200+ passing tests; ensure `npm run audit:ci` passes clean; maintain typecheck and lint pass status
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:44.161Z
Learning: Support `mcodex` launcher with cached multi-auth status display showing model, reasoning effort, cwd, active account, quota usage, plan, and cache age without calling OpenAI on launch
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:44.161Z
Learning: Support `mcodex --tmux` flag to launch Codex inside a tmux session with mouse scrollback enabled
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:44.161Z
Learning: Support `mcodex --tmux --live-accounts` flag to add a live `codex-multi-auth list` monitor pane during tmux session launch
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:01:44.161Z
Learning: Support `mcodex --monitor` mode for monitor-only operation without launching Codex
🪛 LanguageTool
docs/releases/v2.1.13-beta.3.md
[uncategorized] ~4-~4: Do not mix variants of the same word (‘pre-release’ and ‘prerelease’) within a single text.
Context: ...ached statusline (#500), and a round of pre-release hardening surfaced by a whole-tree stre...
(EN_WORD_COHERENCY)
[style] ~95-~95: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...-readable output, with a stable shape whether or not accounts are configured. ## Pre-releas...
(WHETHER)
[uncategorized] ~97-~97: Do not mix variants of the same word (‘pre-release’ and ‘prerelease’) within a single text.
Context: ...her or not accounts are configured. ## Pre-release hardening (#503) - Strip inbound `cook...
(EN_WORD_COHERENCY)
🔇 Additional comments (3)
README.md (1)
386-386: LGTM!docs/README.md (1)
35-35: LGTM!package.json (1)
3-3: LGTM!Also applies to: 164-165, 172-172
…ins) - runtime/runtime-observability.ts: re-assert dir 0o700 via fs.chmod after mkdir — mkdir's mode is a no-op on a pre-existing multi-auth dir, so the upgrade path kept old (possibly permissive) perms. POSIX-only, best-effort. - Bump .codex-plugin/plugin.json + AGENTS.md to 2.1.13-beta.3 (plugin-manifest test asserts manifest version == package version; the beta.3 bump left them stale). Tests: - local-bridge + runtime-rotation-proxy: assert inbound `proxy-authorization` (alongside cookie) is stripped on the outbound request, so a refactor dropping the delete is caught. - quota-cache: read-retry (EACCES) + rename-retry (ENOTEMPTY) + last-write-wins queue-ordering regressions, pinning the widened retry taxonomy. - runtime-observability-dir-mode (new, POSIX-only): pre-create CODEX_MULTI_AUTH_DIR world-writable, persist a snapshot, assert the dir becomes owner-only — proves the chmod-after-mkdir hardening on the upgrade path. - runtime-observability test fs-mock now exposes chmod. typecheck + lint + audit:ci clean; full suite 4249 passed / 2 skipped.
|
Round 2 — addressed all 3 threads (
Full suite 4249 passed / 2 skipped; typecheck + lint + audit:ci clean. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/local-bridge.test.ts (1)
240-272: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winconsider adding proxy-authorization and cookie to auth-enabled path test for completeness.
the test at line 307-332 now verifies all four credential headers (authorization, x-api-key, cookie, proxy-authorization) are stripped. this auth-enabled runtime-proxy test only checks x-api-key stripping. lib/local-bridge.ts:102-115 deletes cookie and proxy-authorization unconditionally on both paths, so adding those assertions here would ensure the auth-enabled flow has equivalent regression coverage.
🔒 proposed addition to strengthen auth-enabled path coverage
await fetch(`${server.baseUrl}/v1/models`, { headers: { authorization: "Bearer inbound-client-token", "x-api-key": "inbound-secret-key", + cookie: "session=inbound-cookie-secret", + "proxy-authorization": "Basic inbound-proxy-cred", }, }); const forwarded = calls.find((c) => c.url.endsWith("/v1/models")); const headers = new Headers(forwarded?.init?.headers as HeadersInit); // The runtime key is injected as Authorization, but the inbound x-api-key is // still stripped — it must never cross the bridge, runtime key or not. expect(headers.get("authorization")).toBe("Bearer runtime-secret-key"); expect(headers.get("x-api-key")).toBeNull(); + expect(headers.get("cookie")).toBeNull(); + expect(headers.get("proxy-authorization")).toBeNull(); });🤖 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 `@test/local-bridge.test.ts` around lines 240 - 272, The current test "strips an inbound x-api-key on the auth-enabled runtime-proxy path" only asserts x-api-key and authorization; update this test to also assert that "proxy-authorization" and "cookie" are removed when forwarding to the runtime. Locate the test function (the it(...) block) and after constructing headers from forwarded.init.headers (variable forwarded and headers), add assertions similar to expect(headers.get("proxy-authorization")).toBeNull() and expect(headers.get("cookie")).toBeNull() so the auth-enabled runtime-proxy path has parity with the other coverage.test/runtime-observability.test.ts (1)
9-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winadd a win32 regression for the new
chmodbranch.
lib/runtime/runtime-observability.ts:206-208is now platform-specific, buttest/runtime-observability-dir-mode.test.ts:71-95skips on windows and this file only adds the mock. please add a small unit case here that forces the win32 path and asserts persistence still succeeds without callingchmod, so the new windows edge case is covered.As per coding guidelines, "test/**: tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior."
🤖 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 `@test/runtime-observability.test.ts` around lines 9 - 19, Add a new unit test in this file that forces the win32 branch by temporarily stubbing process.platform to "win32" (use Object.defineProperty and restore afterwards), then call the same persistence routine exercised by these tests and assert the mocked chmod (chmodMock) is not called and persistence completes successfully; ensure the test is deterministic and restores platform and mocks after completion so the windows-specific branch in runtime-observability (lines around the chmod logic) is covered without invoking chmod on non-Windows hosts.
♻️ Duplicate comments (1)
lib/runtime/runtime-observability.ts (1)
202-208:⚠️ Potential issue | 🟠 Major | ⚡ Quick windon't swallow every
chmodfailure here.on Line 206 in
lib/runtime/runtime-observability.ts:206-208, this turns anychmoderror into success. that means a pre-existing permissive multi-auth dir can stay open whilelib/runtime/runtime-observability.ts:214-218still writeslastAccountEmailand account ids beneath it. only ignore the expected race codes, retry transient contention, and surface the rest.test/runtime-observability-dir-mode.test.ts:71-95covers the happy path only, so this failure mode is still untested.🤖 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/runtime-observability.ts` around lines 202 - 208, Replace the current blanket suppression of errors on the fs.chmod(dir, 0o700) call with targeted handling: perform the chmod in a small retry loop (e.g., 2–3 attempts) that retries on transient errno values like EINTR and EAGAIN/EWOULDBLOCK, explicitly ignore the benign race errno ENOENT (dir disappeared) but rethrow or surface any other errno (e.g., EACCES/EPERM) instead of swallowing it; update the chmod call site that currently uses .catch(() => undefined) to implement this logic so only expected races are ignored and real failures bubble up.
🤖 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 `@test/runtime-observability-dir-mode.test.ts`:
- Line 1: Remove the explicit Vitest import line that brings in afterEach,
beforeEach, describe, expect, it, vi from "vitest" in the test file; rely on the
configured Vitest globals instead (leave existing usages of
describe/it/expect/vi as-is) so drop the line "import { afterEach, beforeEach,
describe, expect, it, vi } from 'vitest';" and ensure there are no remaining
unused imports or references to that import.
- Around line 19-35: The local retrying remover removeDirectoryWithRetry
reimplements Windows-specific backoff and omits ENOTEMPTY; replace its usage
with the shared helper removeWithRetry by removing the removeDirectoryWithRetry
function and calling removeWithRetry(dir) where used (ensure removeWithRetry is
imported/available in the test), so the test uses the centralized Windows-safe
retry logic instead of the custom rmSync loop.
---
Outside diff comments:
In `@test/local-bridge.test.ts`:
- Around line 240-272: The current test "strips an inbound x-api-key on the
auth-enabled runtime-proxy path" only asserts x-api-key and authorization;
update this test to also assert that "proxy-authorization" and "cookie" are
removed when forwarding to the runtime. Locate the test function (the it(...)
block) and after constructing headers from forwarded.init.headers (variable
forwarded and headers), add assertions similar to
expect(headers.get("proxy-authorization")).toBeNull() and
expect(headers.get("cookie")).toBeNull() so the auth-enabled runtime-proxy path
has parity with the other coverage.
In `@test/runtime-observability.test.ts`:
- Around line 9-19: Add a new unit test in this file that forces the win32
branch by temporarily stubbing process.platform to "win32" (use
Object.defineProperty and restore afterwards), then call the same persistence
routine exercised by these tests and assert the mocked chmod (chmodMock) is not
called and persistence completes successfully; ensure the test is deterministic
and restores platform and mocks after completion so the windows-specific branch
in runtime-observability (lines around the chmod logic) is covered without
invoking chmod on non-Windows hosts.
---
Duplicate comments:
In `@lib/runtime/runtime-observability.ts`:
- Around line 202-208: Replace the current blanket suppression of errors on the
fs.chmod(dir, 0o700) call with targeted handling: perform the chmod in a small
retry loop (e.g., 2–3 attempts) that retries on transient errno values like
EINTR and EAGAIN/EWOULDBLOCK, explicitly ignore the benign race errno ENOENT
(dir disappeared) but rethrow or surface any other errno (e.g., EACCES/EPERM)
instead of swallowing it; update the chmod call site that currently uses
.catch(() => undefined) to implement this logic so only expected races are
ignored and real failures bubble up.
🪄 Autofix (Beta)
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
Run ID: 9eecbda9-bb99-4812-af55-713c602512da
📒 Files selected for processing (8)
.codex-plugin/plugin.jsonAGENTS.mdlib/runtime/runtime-observability.tstest/local-bridge.test.tstest/quota-cache.test.tstest/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (8)
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Vitest globals (describe,it,expect) are enabled and should be used without explicit imports
Maintain 80% coverage threshold across statements, branches, functions, and lines
UseremoveWithRetryfor Windows filesystem cleanup instead of barefs.rmto handle EBUSY/EPERM/ENOTEMPTY backoff
Use source files in tests, not compileddist/files; test the source directly
Do not skip tests without justification; include rationale if a test must be skipped
Relax ESLint rules for test files as specified ineslint.config.js
Files:
test/runtime-rotation-proxy.test.tstest/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/local-bridge.test.tstest/quota-cache.test.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only with
"type": "module"configuration and Node >= 18
Files:
test/runtime-rotation-proxy.test.tstest/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/local-bridge.test.tstest/quota-cache.test.tslib/runtime/runtime-observability.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
as any,@ts-ignore, or@ts-expect-errortype assertions
Files:
test/runtime-rotation-proxy.test.tstest/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/local-bridge.test.tstest/quota-cache.test.tslib/runtime/runtime-observability.ts
{scripts/**/*.js,test/**/*.{ts,js}}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use bare recursive delete logic in Windows-sensitive scripts and tests without retry handling for transient
EBUSY,EPERM, andENOTEMPTYerrors
Files:
test/runtime-rotation-proxy.test.tstest/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/local-bridge.test.tstest/quota-cache.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/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/local-bridge.test.tstest/quota-cache.test.ts
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: All public exports should flow throughlib/index.tsor documented package subpaths
Never import fromdist/in source tests or library code
Never suppress type errors
Files:
lib/runtime/runtime-observability.ts
lib/runtime/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not expose account emails or tokens in runtime proxy client response headers or logs
Runtime rotation provider id must be
codex-multi-auth-runtime-proxy
Files:
lib/runtime/runtime-observability.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/runtime-observability.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:54:44.631Z
Learning: Place source code in root `index.ts`, `lib/`, and `scripts/` directories; do not edit `dist/` or local temp/cache directories
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:54:44.631Z
Learning: Do not bypass the official Codex CLI by reimplementing general Codex commands in the wrapper
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:54:44.631Z
Learning: Keep runtime rotation default-on behavior aligned with explicit release and migration documentation
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Store settings and account configuration in `~/.codex/multi-auth/settings.json` or use `CODEX_MULTI_AUTH_DIR` environment variable to override the root storage path
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Credentials and OAuth tokens must remain locally stored and never transmitted outside loopback during runtime rotation
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Runtime Responses proxy rotation is enabled by default but can be disabled via `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0` environment variable
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Implement account health checks with quota forecasting, automatic failover, and flagged-account recovery mechanisms
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Maintain a usage ledger in `~/.codex/multi-auth/usage/usage-ledger.jsonl` to track local usage by project, account, and time period
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Implement request timeout and stream stall timeout configurations via `CODEX_AUTH_FETCH_TIMEOUT_MS` and `CODEX_AUTH_STREAM_STALL_TIMEOUT_MS` environment variables
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Disable whole-pool replay by default when every account is rate-limited to prevent excessive retry loops
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Implement bounded outbound request budgets to prevent a single prompt from exhausting the entire account pool
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Trigger a short cooldown on repeated cross-account 5xx bursts instead of continuing aggressive rotation
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Stagger proactive token refresh to reduce background refresh bursts and improve performance
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Enable `CODEX_AUTH_BACKGROUND_RESPONSES=1` only for callers that intentionally send `background: true` in requests, as it switches routing from stateless `store=false` to stateful `store=true`
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Use TUI v2 for terminal user interface rendering, configurable via `CODEX_TUI_V2=0/1`, with color profile and glyph style customizable through `CODEX_TUI_COLOR_PROFILE` and `CODEX_TUI_GLYPHS` environment variables
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Implement dashboard hotkey navigation with Up/Down for selection, Enter for action, 1-9 for quick switching, / for search, ? for help, and Q for cancel
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Implement account details view hotkeys: S for set current account, R for refresh, E for enable/disable, D for delete
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Support per-project account storage under `~/.codex/multi-auth/projects/<project-key>/openai-codex-accounts.json` for repo-specific workflows
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Perform best-effort daily npm version checks during wrapper startup and only print manual upgrade notice on interactive TTY or when `CODEX_MULTI_AUTH_DEBUG=1`, never automatically running npm install
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Implement settings menu with an Experimental section for staged features including preview-first sync, named local pool backup export, and refresh guard controls
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:00.830Z
Learning: Make settings sync operations non-destructive by default: sync previews before apply, preserve destination-only accounts, and fail safely on backup filename collisions
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:08.545Z
Learning: Documentation files should follow a hierarchical structure with clear sections for Getting Started, Daily Use, Release History, Repair, Reference, Maintainer Docs, and Governance
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T07:55:08.545Z
Learning: Daily-use documentation should include landing pages, FAQs, feature capability maps, configuration guidance, architecture overview, and privacy information
🔇 Additional comments (4)
.codex-plugin/plugin.json (1)
3-3: LGTM!test/local-bridge.test.ts (1)
321-321: LGTM!Also applies to: 331-331
test/runtime-rotation-proxy.test.ts (1)
1124-1124: LGTM!Also applies to: 1137-1137
test/quota-cache.test.ts (1)
434-545: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/storage.ts (1)
1827-1891:⚠️ Potential issue | 🟠 Major | ⚡ Quick winretry wal cleanup too.
lib/storage.ts:1875still does a singlefs.unlink(walPath)after the save succeeds. that leaves the secret-bearing.walvulnerable to the same transient windows locks you just hardened forwriteJournalandcleanupTemp, so a momentaryebusy/eperm/eagain/enotempty/eaccescan strand stale recovery data on disk.proposed fix
cleanupWal: async () => { try { - await fs.unlink(walPath); + await withFileOperationRetry(() => fs.unlink(walPath)); } catch { // Best effort cleanup. } },please also add the matching vitest regression that forces one retryable unlink failure on the wal path, otherwise this windows-specific path is still unpinned. as per coding guidelines,
lib/**: "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."🤖 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/storage.ts` around lines 1827 - 1891, The cleanupWal implementation currently does a single fs.unlink(walPath) which can leave the secret-bearing .wal stranded on Windows; change cleanupWal to call withFileOperationRetry(() => fs.unlink(walPath)) (same retry taxonomy used by writeJournal and cleanupTemp) and preserve the existing best-effort swallow behavior on persistent failure; then add a vitest regression that stubs/monkeypatches fs.unlink to fail once with a retryable errno (e.g., EBUSY or EPERM) for the walPath and succeed on the next attempt, asserting cleanupWal returns/does not throw and that no sensitive data is logged.test/runtime-rotation-proxy.test.ts (1)
1106-1138:⚠️ Potential issue | 🟠 Major | ⚡ Quick winadd regression tests for bounded upstream error-body handling.
test/runtime-rotation-proxy.test.ts:1123-1137now covers credential stripping, but there is still no direct regression for the new bounded reader inlib/runtime-rotation-proxy.ts:899-958across error branches (lib/runtime-rotation-proxy.ts:1794,lib/runtime-rotation-proxy.ts:1819,lib/runtime-rotation-proxy.ts:1916,lib/runtime-rotation-proxy.ts:1962). please add deterministic tests for (1) stalled 4xx/5xx body timeout and (2) >1mb error-body cap. this is a concurrency/runtime hang risk path.as per coding guidelines:
test/**: tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior.🤖 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 `@test/runtime-rotation-proxy.test.ts` around lines 1106 - 1138, Add two deterministic Vitest regression tests to test/runtime-rotation-proxy.test.ts that exercise the bounded error-body reader in the proxy: (1) simulate an upstream 4xx/5xx response that stalls while sending the error body and assert the proxy times out the read (use createRecordingFetch or a controlled fetchImpl that returns a ReadableStream which stops yielding; use vi.useFakeTimers/advanceTimersToNextTimer to drive the timeout) and (2) simulate an upstream error response that attempts to send >1MB of error bytes and assert the proxy truncates at the 1MB cap and completes the fetch without hanging. Wire these tests to the same helpers used in the file (startProxy, postResponses, createRecordingFetch) and assert that the proxy returns the expected error status/body/truncation and that upstream fetch calls complete (no pending readers), thereby covering the bounded reader behavior used in the runtime-rotation-proxy error branches.test/runtime-observability.test.ts (1)
9-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winadd assertions for the new permission hardening path.
test/runtime-observability.test.ts:9-20addschmodMock, but none of the cases assert the security contract introduced inlib/runtime/runtime-observability.ts:206-217:fs.chmod(dir, 0o700)on posix andfs.writeFile(..., { mode: 0o600, encoding: "utf-8" })for the temp snapshot. as written, those lines can regress without failing this suite.example assertions
it("retries transient rename contention when persisting a snapshot", async () => { process.env.VITEST = ""; let attempts = 0; renameMock.mockImplementation(async () => { @@ await vi.waitFor(() => { expect(renameMock).toHaveBeenCalledTimes(2); }); + expect(chmodMock).toHaveBeenCalledWith("/mock/.codex/multi-auth", 0o700); + expect(writeFileMock).toHaveBeenCalledWith( + expect.stringContaining("runtime-observability.json."), + expect.any(String), + expect.objectContaining({ encoding: "utf-8", mode: 0o600 }), + ); expect(unlinkMock).toHaveBeenCalled(); });as per coding guidelines, "test/**: 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."
🤖 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 `@test/runtime-observability.test.ts` around lines 9 - 20, The test added chmodMock but never asserts the new permission-hardening behavior; update the tests that exercise runtime-observability behavior to assert that chmodMock was called with 0o700 for the directory (fs.chmod(dir, 0o700)) on POSIX flows and that writeFileMock was called with options { mode: 0o600, encoding: "utf-8" } for the temp snapshot write; locate the calls by referencing chmodMock and writeFileMock in runtime-observability tests and add expectations (and where needed simulate POSIX vs Windows behavior) so regressions in the fs.chmod and fs.writeFile mode/encoding contracts will fail the suite.
♻️ Duplicate comments (1)
test/runtime-observability-dir-mode.test.ts (1)
19-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winuse
removeWithRetryhere instead of a local rm loop.
test/runtime-observability-dir-mode.test.ts:19-35reimplements windows cleanup withrmSyncand only retriesEBUSY/EPERM. that diverges from the shared retry taxonomy and still missesENOTEMPTY, so this test can drift from the repo’s windows fs behavior contract.as per coding guidelines, "Use
removeWithRetryfor Windows filesystem cleanup instead of barefs.rmto handle EBUSY/EPERM/ENOTEMPTY backoff".🤖 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 `@test/runtime-observability-dir-mode.test.ts` around lines 19 - 35, The test defines a local rm loop in removeDirectoryWithRetry that uses rmSync and only checks isRetriableFsError, which diverges from the shared retry taxonomy and omits ENOTEMPTY; replace this implementation by using the shared helper removeWithRetry (or delegate to it) so the test uses the repository-wide Windows cleanup/backoff behavior; locate removeDirectoryWithRetry and swap its body to call removeWithRetry(dir) (or import and call removeWithRetry) and remove references to rmSync/isRetriableFsError to ensure the test uses the canonical retry policy.
🤖 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/runtime-observability.ts`:
- Around line 206-208: The current unconditional swallow of fs.chmod(dir, 0o700)
hides real failures; update the catch to only suppress the benign race case
(e.g. the path disappearing) and surface all other errors. Change the .catch(()
=> undefined) on the fs.chmod call so it inspects the caught error and
returns/ignores only when error.code indicates a transient/missing-path race
(such as 'ENOENT'), but rethrow or propagate any other error (EACCES/EPERM/etc.)
so attempts to harden CODEX_MULTI_AUTH_DIR and write runtime-observability.json
fail loudly instead of silently weakening permissions. Ensure this logic is
applied where fs.chmod(dir, 0o700) is called.
---
Outside diff comments:
In `@lib/storage.ts`:
- Around line 1827-1891: The cleanupWal implementation currently does a single
fs.unlink(walPath) which can leave the secret-bearing .wal stranded on Windows;
change cleanupWal to call withFileOperationRetry(() => fs.unlink(walPath)) (same
retry taxonomy used by writeJournal and cleanupTemp) and preserve the existing
best-effort swallow behavior on persistent failure; then add a vitest regression
that stubs/monkeypatches fs.unlink to fail once with a retryable errno (e.g.,
EBUSY or EPERM) for the walPath and succeed on the next attempt, asserting
cleanupWal returns/does not throw and that no sensitive data is logged.
In `@test/runtime-observability.test.ts`:
- Around line 9-20: The test added chmodMock but never asserts the new
permission-hardening behavior; update the tests that exercise
runtime-observability behavior to assert that chmodMock was called with 0o700
for the directory (fs.chmod(dir, 0o700)) on POSIX flows and that writeFileMock
was called with options { mode: 0o600, encoding: "utf-8" } for the temp snapshot
write; locate the calls by referencing chmodMock and writeFileMock in
runtime-observability tests and add expectations (and where needed simulate
POSIX vs Windows behavior) so regressions in the fs.chmod and fs.writeFile
mode/encoding contracts will fail the suite.
In `@test/runtime-rotation-proxy.test.ts`:
- Around line 1106-1138: Add two deterministic Vitest regression tests to
test/runtime-rotation-proxy.test.ts that exercise the bounded error-body reader
in the proxy: (1) simulate an upstream 4xx/5xx response that stalls while
sending the error body and assert the proxy times out the read (use
createRecordingFetch or a controlled fetchImpl that returns a ReadableStream
which stops yielding; use vi.useFakeTimers/advanceTimersToNextTimer to drive the
timeout) and (2) simulate an upstream error response that attempts to send >1MB
of error bytes and assert the proxy truncates at the 1MB cap and completes the
fetch without hanging. Wire these tests to the same helpers used in the file
(startProxy, postResponses, createRecordingFetch) and assert that the proxy
returns the expected error status/body/truncation and that upstream fetch calls
complete (no pending readers), thereby covering the bounded reader behavior used
in the runtime-rotation-proxy error branches.
---
Duplicate comments:
In `@test/runtime-observability-dir-mode.test.ts`:
- Around line 19-35: The test defines a local rm loop in
removeDirectoryWithRetry that uses rmSync and only checks isRetriableFsError,
which diverges from the shared retry taxonomy and omits ENOTEMPTY; replace this
implementation by using the shared helper removeWithRetry (or delegate to it) so
the test uses the repository-wide Windows cleanup/backoff behavior; locate
removeDirectoryWithRetry and swap its body to call removeWithRetry(dir) (or
import and call removeWithRetry) and remove references to
rmSync/isRetriableFsError to ensure the test uses the canonical retry policy.
🪄 Autofix (Beta)
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
Run ID: 27c7e0fb-4921-468a-a963-7e85c7379b11
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.codex-plugin/plugin.jsonAGENTS.mdREADME.mddocs/README.mddocs/releases/v2.1.13-beta.3.mdlib/local-bridge.tslib/quota-cache.tslib/runtime-rotation-proxy.tslib/runtime/runtime-observability.tslib/storage.tspackage.jsontest/local-bridge.test.tstest/mcodex-statusline-scope.test.tstest/quota-cache.test.tstest/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (20)
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: All public exports should flow throughlib/index.tsor documented package subpaths
Never import fromdist/in source tests or library code
Never suppress type errors
Files:
lib/local-bridge.tslib/quota-cache.tslib/runtime/runtime-observability.tslib/runtime-rotation-proxy.tslib/storage.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using
as any,@ts-ignore, or@ts-expect-errortype assertion suppressions
Files:
lib/local-bridge.tstest/mcodex-statusline-scope.test.tstest/runtime-observability.test.tstest/local-bridge.test.tslib/quota-cache.tslib/runtime/runtime-observability.tstest/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.tslib/storage.tstest/runtime-observability-dir-mode.test.tstest/quota-cache.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM-only module syntax (
import/export); the package is configured with"type": "module"
Files:
lib/local-bridge.tstest/mcodex-statusline-scope.test.tstest/runtime-observability.test.tstest/local-bridge.test.tslib/quota-cache.tslib/runtime/runtime-observability.tstest/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.tslib/storage.tstest/runtime-observability-dir-mode.test.tstest/quota-cache.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Normalize email keys using
normalizeEmailKey()(trim + lowercase) for case-insensitive email deduplication
Files:
lib/local-bridge.tstest/mcodex-statusline-scope.test.tstest/runtime-observability.test.tstest/local-bridge.test.tslib/quota-cache.tslib/runtime/runtime-observability.tstest/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.tslib/storage.tstest/runtime-observability-dir-mode.test.tstest/quota-cache.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/local-bridge.tslib/quota-cache.tslib/runtime/runtime-observability.tslib/runtime-rotation-proxy.tslib/storage.ts
docs/{README.md,docs/**/*.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
docs/{README.md,docs/**/*.md}: Use canonical package namecodex-multi-authin all documentation
Verify internal links in documentation are valid before merge
Ensure no conflicting guidance exists between README, docs, and governance files before merge
Files:
docs/README.md
docs/{README.md,docs/getting-started.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Update
README.mdanddocs/getting-started.mdfirst when runtime behavior changes
Files:
docs/README.md
docs/{README.md,docs/getting-started.md,docs/configuration.md,docs/troubleshooting.md,docs/reference/**/*.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Verify every documented command is executable as written before merge
Files:
docs/README.md
docs/{docs/**/*.md,README.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Verify paths in documentation match runtime modules (
lib/runtime-paths.ts,lib/storage.ts,lib/config.ts)
Files:
docs/README.md
docs/{README.md,docs/index.md,docs/README.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Use accurate discoverability terms in public landing pages without keyword stuffing or ranking promises
Root README and docs landing pages should naturally include
Codex CLI,multi-account OAuth,account switching,health checks,runtime rotation,diagnostics, andrecoverywhen those topics are in scope
Files:
docs/README.md
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 documentationDocumentation files should be organized with clear structure: Start Here section, Daily Use section, Release History (with Stable Releases subsection), Repair section, Reference section, Maintainer Docs section, and Governance links
Files:
docs/README.mddocs/releases/v2.1.13-beta.3.md
docs/{README.md,package.json,docs/development/GITHUB_DISCOVERABILITY.md}
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
Keep the repository description, package description, README lead, and
docs/development/GITHUB_DISCOVERABILITY.mdaligned
Files:
docs/README.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/README.mddocs/releases/v2.1.13-beta.3.md
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Vitest globals (describe,it,expect) are enabled and should be used without explicit imports
Maintain 80% coverage threshold across statements, branches, functions, and lines
UseremoveWithRetryfor Windows filesystem cleanup instead of barefs.rmto handle EBUSY/EPERM/ENOTEMPTY backoff
Use source files in tests, not compileddist/files; test the source directly
Do not skip tests without justification; include rationale if a test must be skipped
Relax ESLint rules for test files as specified ineslint.config.js
Files:
test/mcodex-statusline-scope.test.tstest/runtime-observability.test.tstest/local-bridge.test.tstest/runtime-rotation-proxy.test.tstest/runtime-observability-dir-mode.test.tstest/quota-cache.test.ts
test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for test suites; include property tests and chaos tests for critical paths
Files:
test/mcodex-statusline-scope.test.tstest/runtime-observability.test.tstest/local-bridge.test.tstest/runtime-rotation-proxy.test.tstest/runtime-observability-dir-mode.test.tstest/quota-cache.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/mcodex-statusline-scope.test.tstest/runtime-observability.test.tstest/local-bridge.test.tstest/runtime-rotation-proxy.test.tstest/runtime-observability-dir-mode.test.tstest/quota-cache.test.ts
lib/runtime/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not expose account emails or tokens in runtime proxy client response headers or logs
Files:
lib/runtime/runtime-observability.ts
docs/releases/**/*.md
📄 CodeRabbit inference engine (docs/README.md)
Release notes should be organized in a releases/ directory with semantic versioning filenames (e.g., releases/v2.1.12.md, releases/v2.1.13-beta.3.md)
Files:
docs/releases/v2.1.13-beta.3.md
package.json
📄 CodeRabbit inference engine (SECURITY.md)
package.json: Pinhonoto4.12.18or higher (but below4.12.0-4.12.1) to avoid GHSA-xh87-mx6m-69f3 authentication bypass vulnerability
Pinrollupto^4.59.0or higher to avoid vulnerable versions below4.59.0in Vite and Vitest transitive dependencies
Files:
package.json
lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/runtime-rotation-proxy.ts: Runtime rotation code must preserve pass-through semantics except for auth/provider headers that intentionally change
Runtime proxy client-facing headers must not expose account emails or tokens
Runtime rotation should fail open to normal official Codex forwarding when startup helpers are unavailable
Never add account emails/tokens to runtime proxy client responsesRuntime rotation proxy is loopback-only, uses per-process client tokens, and forwards only Responses API and model discovery requests
Files:
lib/runtime-rotation-proxy.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:24.354Z
Learning: Keep runtime rotation default-on behavior (via `codexRuntimeRotationProxy`) aligned with explicit release and migration documentation; users can opt out with `codex-multi-auth rotation disable` or `CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0`
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:24.354Z
Learning: Do not bypass the official Codex CLI by reimplementing general Codex commands in the wrapper; the wrapper's primary role is account management and selective command forwarding
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:24.354Z
Learning: Node runtime requirement is >= 18; ensure all code uses features available in Node 18 or later
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:24.354Z
Learning: The canonical package name is `codex-multi-auth` and the canonical command family is `codex-multi-auth ...`; the package does not publish a global `codex` bin
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: This is an independent open-source project, not an official OpenAI product
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Users are responsible for their own usage and policy compliance when using codex-multi-auth
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: For production/commercial workloads, use the OpenAI Platform API instead of OAuth account credentials through codex-multi-auth
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Keep the official `codex` command owned by the official OpenAI install path; do not use codex-multi-auth to override it
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Credentials should stay local and runtime rotation should be loopback-only in multi-account workflows
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Use health-aware account selection with quota forecasting, automatic failover, and flagged-account recovery
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Whole-pool replay should be disabled by default when every account is rate-limited
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Active requests should use a bounded outbound request budget so one prompt cannot walk the full pool indefinitely
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Repeated cross-account 5xx bursts should trigger a short cooldown instead of continuing aggressive rotation
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Proactive refresh should be staggered to reduce background refresh bursts
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Responses background mode should stay opt-in and only enabled for callers that intentionally send `background: true`
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Runtime rotation should be enabled by default for request-bearing wrapper-launched Codex sessions
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Installed wrappers should perform a best-effort daily npm version check during normal forwarded Codex startup
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:34.577Z
Learning: Version update notices should only print on an interactive TTY or when `CODEX_MULTI_AUTH_DEBUG=1`; never run npm install or update commands automatically
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:01:48.349Z
Learning: Include root-level governance links: project entry (../README.md), contribution policy (../CONTRIBUTING.md), code of conduct (../CODE_OF_CONDUCT.md), and security policy (../SECURITY.md)
🪛 LanguageTool
docs/releases/v2.1.13-beta.3.md
[uncategorized] ~4-~4: Do not mix variants of the same word (‘pre-release’ and ‘prerelease’) within a single text.
Context: ...ached statusline (#500), and a round of pre-release hardening surfaced by a whole-tree stre...
(EN_WORD_COHERENCY)
[style] ~95-~95: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...-readable output, with a stable shape whether or not accounts are configured. ## Pre-releas...
(WHETHER)
[uncategorized] ~97-~97: Do not mix variants of the same word (‘pre-release’ and ‘prerelease’) within a single text.
Context: ...her or not accounts are configured. ## Pre-release hardening (#503) - Strip inbound `cook...
(EN_WORD_COHERENCY)
🔇 Additional comments (12)
.codex-plugin/plugin.json (1)
3-3: LGTM!package.json (1)
3-3: LGTM!Also applies to: 164-166, 172-172
AGENTS.md (1)
6-6: LGTM!README.md (1)
386-386: LGTM!docs/README.md (1)
35-35: LGTM!docs/releases/v2.1.13-beta.3.md (1)
1-108: LGTM!test/mcodex-statusline-scope.test.ts (1)
3-3: LGTM!Also applies to: 85-86
lib/quota-cache.ts (1)
35-44: LGTM!Also applies to: 132-145, 246-255
test/quota-cache.test.ts (1)
434-477: LGTM!Also applies to: 479-514, 516-545
lib/local-bridge.ts (1)
101-104: LGTM!lib/runtime-rotation-proxy.ts (1)
271-274: LGTM!Also applies to: 899-958, 1794-1794, 1819-1819, 1916-1916, 1962-1962
test/local-bridge.test.ts (1)
320-331: LGTM!
…coverage) - runtime/runtime-observability.ts: stop swallowing every chmod failure. Only ENOENT (dir removed by a concurrent process) is ignored; any other failure is surfaced rather than silently leaving a world-readable dir holding account ids/labels. - runtime-observability.test.ts: add a win32 case asserting the snapshot persists WITHOUT calling chmod (POSIX-only branch), and reset the chmod mock per test. - local-bridge.test.ts: extend the auth-enabled runtime-proxy path test to assert cookie + proxy-authorization are stripped too (parity with the no-key path). typecheck + lint clean; full suite 4250 passed / 2 skipped.
|
Round 3 ( |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
test/runtime-rotation-proxy.test.ts (1)
899-958: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winmissing regression test for bounded error-body read.
the new
readErrorBodyatlib/runtime-rotation-proxy.ts:899-958adds idle-timeout and 1mb cap protections, but there's no test that verifies the reader cancels and returns partial data when upstream stalls or sends an oversized error body. consider adding a test that mocks a slow/stalled error body stream and asserts the handler doesn't hang.example scenarios to cover:
- upstream 429 with a body that never completes → handler returns within
streamStallTimeoutMs- upstream 5xx with body exceeding 1mb → reader stops at cap, handler continues
🤖 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 `@test/runtime-rotation-proxy.test.ts` around lines 899 - 958, Add regression tests for readErrorBody behavior by mocking fetch responses that simulate (1) a stalled error body and (2) an oversized (>1MB) error body; use the same test harness helpers (createRecordingFetch / startProxy / postThreadGoal) to inject a Response whose body is a ReadableStream that either never resolves chunks (to trigger streamStallTimeoutMs) or yields >1MB then stops, and assert the handler returns within streamStallTimeoutMs for the stalled case and that the returned error body is truncated (or does not hang) for the oversized case; reference readErrorBody and streamStallTimeoutMs in your assertions to ensure the new timeout and cap protections are exercised.lib/storage.ts (1)
1875-1880:⚠️ Potential issue | 🟠 Major | ⚡ Quick winretry wal cleanup on windows locks.
lib/storage.ts:1875still does a single best-effortunlinkon the secret-bearing wal after a successful save. the same transienteperm/ebusy/eacces/enotempty/eagainlock this patch hardens elsewhere can leave refresh tokens stranded on disk and keep recovery state stale. wrapcleanupWalinwithFileOperationRetry(...), and add a vitest case that makes wal cleanup fail once before succeeding so this path stays pinned down.as per coding guidelines, `**/*.{ts,tsx,js}`: "windows filesystem safety: retry transient `EBUSY`/`EPERM`/`ENOTEMPTY` cleanup and write failures where tests cover windows locks".suggested patch
cleanupWal: async () => { try { - await fs.unlink(walPath); + await withFileOperationRetry(() => fs.unlink(walPath)); } catch { // Best effort cleanup. } },🤖 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/storage.ts` around lines 1875 - 1880, The cleanupWal implementation currently performs a single fs.unlink attempt and must be hardened against transient Windows locks; replace the direct unlink in the cleanupWal async function with a call to withFileOperationRetry(...) wrapping the unlink of walPath (i.e., call withFileOperationRetry(() => fs.unlink(walPath), 'unlink', walPath) or equivalent) so it retries on EBUSY/EPERM/ENOTEMPTY/EACCES/EAGAIN, and add a vitest that stubs/mocks fs.unlink to fail once with a transient error then succeed to prove the retry path executes and the wal is removed.lib/quota-cache.ts (1)
256-263:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winretry temp cleanup after exhausted rename retries.
lib/quota-cache.ts:256only triesunlink(tempPath)once. on windows the same lock that makesrenameback off often makes cleanup fail too, so this can strand.tmpfiles next to the cache and leave the hardening incomplete. reuse the same retry policy here, and add a regression that makesfs.unlinkthrow a transient lock once.as per coding guidelines, `**/*.{ts,tsx,js}`: "windows filesystem safety: retry transient `EBUSY`/`EPERM`/`ENOTEMPTY` cleanup and write failures where tests cover windows locks".suggested patch
} finally { if (!renamed) { - try { - await fs.unlink(tempPath); - } catch { - // Best effort temp cleanup. - } + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + await fs.unlink(tempPath); + break; + } catch (error) { + if (!isRetryableFsError(error) || attempt >= 4) break; + await sleep(10 * 2 ** attempt); + } + } } }🤖 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/quota-cache.ts` around lines 256 - 263, The final cleanup currently calls fs.unlink(tempPath) only once when rename retries exhausted (see variables renamed and tempPath), which can fail on Windows due to transient locks; update the finally block to reuse the same retry policy used for rename (retry on EBUSY/EPERM/ENOTEMPTY with short backoff) to attempt unlink multiple times before giving up, ensure errors other than transient ones still propagate or are logged as "Best effort temp cleanup", and add a regression test that simulates fs.unlink throwing a transient lock error once to verify the retry behavior.lib/runtime/runtime-observability.ts (1)
222-241:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winretry temp snapshot cleanup on windows.
lib/runtime/runtime-observability.ts:236still does a one-shotfs.unlink(tempPath)and swallows the error. after anebusy/epermfailure in the write/rename path, that can strand*.tmpsnapshots under the same transient lock conditions this pr is hardening elsewhere. route cleanup through the shared retry taxonomy and add a vitest regression that forcesunlinkto fail once with a windows lock code. as per coding guidelines, "windows filesystem safety: retry transientEBUSY/EPERM/ENOTEMPTYcleanup and write failures where tests cover windows locks".🤖 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/runtime-observability.ts` around lines 222 - 241, The finalizer currently does a one-shot fs.unlink(tempPath) that swallows errors; change it to use the existing retry taxonomy (RETRYABLE_SNAPSHOT_ERRORS, attempt counter, lastError) so cleanup retries transient Windows lock errors (EBUSY/EPERM/ENOTEMPTY) rather than silently leaving .tmp files: replace the single await fs.unlink(tempPath) in the finally block with a retry loop that checks error.code against RETRYABLE_SNAPSHOT_ERRORS, increments/uses attempt and lastError like the write/rename path, and only stops retrying when non-retryable or max attempts reached; also add a vitest regression that forces fs.unlink to fail once with a Windows lock code and verifies the cleanup eventually succeeds.
🤖 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 `@test/runtime-observability.test.ts`:
- Around line 104-124: The test currently only checks that chmod is not called
on win32; add a POSIX-path assertion that the temp snapshot write uses
owner-only permissions by asserting writeFileMock was called with an options
object including mode: 0o600. After calling mutateRuntimeObservabilitySnapshot
and awaiting renameMock, assert (for the POSIX code path) that writeFileMock was
invoked with the expected filename and options containing mode: 0o600 (use
writeFileMock.mock.calls or toHaveBeenCalledWith/expect.objectContaining to
check the options), leaving the existing platform/win32 assertions and restore
logic intact so a regression in the hardened temp snapshot write will fail the
test.
---
Outside diff comments:
In `@lib/quota-cache.ts`:
- Around line 256-263: The final cleanup currently calls fs.unlink(tempPath)
only once when rename retries exhausted (see variables renamed and tempPath),
which can fail on Windows due to transient locks; update the finally block to
reuse the same retry policy used for rename (retry on EBUSY/EPERM/ENOTEMPTY with
short backoff) to attempt unlink multiple times before giving up, ensure errors
other than transient ones still propagate or are logged as "Best effort temp
cleanup", and add a regression test that simulates fs.unlink throwing a
transient lock error once to verify the retry behavior.
In `@lib/runtime/runtime-observability.ts`:
- Around line 222-241: The finalizer currently does a one-shot
fs.unlink(tempPath) that swallows errors; change it to use the existing retry
taxonomy (RETRYABLE_SNAPSHOT_ERRORS, attempt counter, lastError) so cleanup
retries transient Windows lock errors (EBUSY/EPERM/ENOTEMPTY) rather than
silently leaving .tmp files: replace the single await fs.unlink(tempPath) in the
finally block with a retry loop that checks error.code against
RETRYABLE_SNAPSHOT_ERRORS, increments/uses attempt and lastError like the
write/rename path, and only stops retrying when non-retryable or max attempts
reached; also add a vitest regression that forces fs.unlink to fail once with a
Windows lock code and verifies the cleanup eventually succeeds.
In `@lib/storage.ts`:
- Around line 1875-1880: The cleanupWal implementation currently performs a
single fs.unlink attempt and must be hardened against transient Windows locks;
replace the direct unlink in the cleanupWal async function with a call to
withFileOperationRetry(...) wrapping the unlink of walPath (i.e., call
withFileOperationRetry(() => fs.unlink(walPath), 'unlink', walPath) or
equivalent) so it retries on EBUSY/EPERM/ENOTEMPTY/EACCES/EAGAIN, and add a
vitest that stubs/mocks fs.unlink to fail once with a transient error then
succeed to prove the retry path executes and the wal is removed.
In `@test/runtime-rotation-proxy.test.ts`:
- Around line 899-958: Add regression tests for readErrorBody behavior by
mocking fetch responses that simulate (1) a stalled error body and (2) an
oversized (>1MB) error body; use the same test harness helpers
(createRecordingFetch / startProxy / postThreadGoal) to inject a Response whose
body is a ReadableStream that either never resolves chunks (to trigger
streamStallTimeoutMs) or yields >1MB then stops, and assert the handler returns
within streamStallTimeoutMs for the stalled case and that the returned error
body is truncated (or does not hang) for the oversized case; reference
readErrorBody and streamStallTimeoutMs in your assertions to ensure the new
timeout and cap protections are exercised.
🪄 Autofix (Beta)
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
Run ID: 4ed44716-4fd3-455e-a638-e42c0f45bf4b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.codex-plugin/plugin.jsonAGENTS.mdREADME.mddocs/README.mddocs/releases/v2.1.13-beta.3.mdlib/local-bridge.tslib/quota-cache.tslib/runtime-rotation-proxy.tslib/runtime/runtime-observability.tslib/storage.tspackage.jsontest/local-bridge.test.tstest/mcodex-statusline-scope.test.tstest/quota-cache.test.tstest/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (28)
!dist/**
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in root
index.ts,lib/, andscripts/directories;dist/is generated output and should not be edited
Files:
AGENTS.mddocs/README.md.codex-plugin/plugin.jsonpackage.jsonREADME.mdtest/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.tsdocs/releases/v2.1.13-beta.3.mdlib/quota-cache.tslib/local-bridge.tstest/local-bridge.test.tslib/storage.tstest/quota-cache.test.tslib/runtime/runtime-observability.tslib/runtime-rotation-proxy.tstest/mcodex-statusline-scope.test.ts
docs/{README.md,docs/**/*.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
docs/{README.md,docs/**/*.md}: Use canonical package namecodex-multi-authin all documentation
Verify internal links in documentation are valid before merge
Ensure no conflicting guidance exists between README, docs, and governance files before merge
Files:
docs/README.md
docs/{README.md,docs/getting-started.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Update
README.mdanddocs/getting-started.mdfirst when runtime behavior changes
Files:
docs/README.md
docs/{README.md,docs/getting-started.md,docs/configuration.md,docs/troubleshooting.md,docs/reference/**/*.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Verify every documented command is executable as written before merge
Files:
docs/README.md
docs/{docs/**/*.md,README.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Verify paths in documentation match runtime modules (
lib/runtime-paths.ts,lib/storage.ts,lib/config.ts)
Files:
docs/README.md
docs/{README.md,docs/index.md,docs/README.md}
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Use accurate discoverability terms in public landing pages without keyword stuffing or ranking promises
Root README and docs landing pages should naturally include
Codex CLI,multi-account OAuth,account switching,health checks,runtime rotation,diagnostics, andrecoverywhen those topics are in scope
Files:
docs/README.md
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 documentationDocumentation files should use
.mdextension and follow a hierarchical structure with clear sections for Getting Started, Daily Use, Release History, Repair, Reference, Maintainer Docs, and GovernanceCross-references in documentation should use relative markdown links (e.g.,
[link text](path/to/file.md)) to enable proper documentation navigation and discoverability
Files:
docs/README.mddocs/releases/v2.1.13-beta.3.md
docs/{README.md,package.json,docs/development/GITHUB_DISCOVERABILITY.md}
📄 CodeRabbit inference engine (docs/STYLE_GUIDE.md)
Keep the repository description, package description, README lead, and
docs/development/GITHUB_DISCOVERABILITY.mdaligned
Files:
docs/README.md
docs/**/{DOCUMENTATION.md,CONTRIBUTING.md,CODE_OF_CONDUCT.md,SECURITY.md,README.md}
📄 CodeRabbit inference engine (docs/README.md)
Documentation governance and contribution policies should be maintained at the project root level in files like DOCUMENTATION.md, CONTRIBUTING.md, CODE_OF_CONDUCT.md, SECURITY.md, and README.md
Files:
docs/README.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/README.mddocs/releases/v2.1.13-beta.3.md
package.json
📄 CodeRabbit inference engine (SECURITY.md)
package.json: Pinhonoto4.12.18or higher (but below4.12.0-4.12.1) to avoid GHSA-xh87-mx6m-69f3 authentication bypass vulnerability
Pinrollupto^4.59.0or higher to avoid vulnerable versions below4.59.0in Vite and Vitest transitive dependencies
Files:
package.json
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
test/**/*.test.ts: Vitest globals (describe,it,expect) are enabled and should be used without explicit imports
Maintain 80% coverage threshold across statements, branches, functions, and lines
UseremoveWithRetryfor Windows filesystem cleanup instead of barefs.rmto handle EBUSY/EPERM/ENOTEMPTY backoff
Use source files in tests, not compileddist/files; test the source directly
Do not skip tests without justification; include rationale if a test must be skipped
Relax ESLint rules for test files as specified ineslint.config.js
Files:
test/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.tstest/local-bridge.test.tstest/quota-cache.test.tstest/mcodex-statusline-scope.test.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only (
"type": "module"), Node >= 18
Files:
test/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.tslib/quota-cache.tslib/local-bridge.tstest/local-bridge.test.tslib/storage.tstest/quota-cache.test.tslib/runtime/runtime-observability.tslib/runtime-rotation-proxy.tstest/mcodex-statusline-scope.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
as any,@ts-ignore, or@ts-expect-error
Files:
test/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.tslib/quota-cache.tslib/local-bridge.tstest/local-bridge.test.tslib/storage.tstest/quota-cache.test.tslib/runtime/runtime-observability.tslib/runtime-rotation-proxy.tstest/mcodex-statusline-scope.test.ts
{scripts/**,test/**}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use bare recursive delete logic in Windows-sensitive scripts/tests without retry handling
Files:
test/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.tstest/local-bridge.test.tstest/quota-cache.test.tstest/mcodex-statusline-scope.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Windows filesystem safety: retry transient
EBUSY/EPERM/ENOTEMPTYcleanup and write failures where tests cover Windows locks
Files:
test/runtime-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.tslib/quota-cache.tslib/local-bridge.tstest/local-bridge.test.tslib/storage.tstest/quota-cache.test.tslib/runtime/runtime-observability.tslib/runtime-rotation-proxy.tstest/mcodex-statusline-scope.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-observability-dir-mode.test.tstest/runtime-observability.test.tstest/runtime-rotation-proxy.test.tstest/local-bridge.test.tstest/quota-cache.test.tstest/mcodex-statusline-scope.test.ts
**/*{response,proxy,request,routing}*.{ts,js}
📄 CodeRabbit inference engine (README.md)
Responses background mode (
background: true) should remain opt-in by default and only enable statefulstore=truerouting for callers that explicitly setbackgroundResponsesin settings orCODEX_AUTH_BACKGROUND_RESPONSES=1
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.ts
**/*{rotation,selection,retry,rate-limit}*.{ts,js}
📄 CodeRabbit inference engine (README.md)
Whole-pool replay must be disabled by default when every account is rate-limited to prevent infinite retry loops
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.ts
**/*{rotation,quota,budget,request}*.{ts,js}
📄 CodeRabbit inference engine (README.md)
Active requests must use a bounded outbound request budget to prevent a single prompt from iterating through the entire account pool indefinitely
Files:
test/runtime-rotation-proxy.test.tslib/quota-cache.tstest/quota-cache.test.tslib/runtime-rotation-proxy.ts
**/*{rotation,error,cooldown,backoff}*.{ts,js}
📄 CodeRabbit inference engine (README.md)
Repeated cross-account 5xx bursts must trigger a short cooldown instead of continuing aggressive rotation to prevent hammering upstream services
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.ts
docs/releases/v*.md
📄 CodeRabbit inference engine (docs/README.md)
Release notes documents should follow the naming pattern
releases/v{VERSION}.mdfor stable releases andreleases/v{VERSION}-beta.{NUMBER}.mdfor prerelease versions
Files:
docs/releases/v2.1.13-beta.3.md
lib/**/*.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/**/*.ts: All public exports should flow throughlib/index.tsor documented package subpaths
Never import fromdist/in source tests or library code
Never suppress type errors
Files:
lib/quota-cache.tslib/local-bridge.tslib/storage.tslib/runtime/runtime-observability.tslib/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/quota-cache.tslib/local-bridge.tslib/storage.tslib/runtime/runtime-observability.tslib/runtime-rotation-proxy.ts
**/*{storage,account,project,persist}*.{ts,js}
📄 CodeRabbit inference engine (README.md)
Account storage must support project-scoped storage under
~/.codex/multi-auth/projects/<project-key>/for repo-specific workflows in addition to global account storage
Files:
lib/storage.ts
{lib/auth/**,lib/runtime/**,scripts/**}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hardcode OAuth ports; use existing constants/helpers
Files:
lib/runtime/runtime-observability.ts
{lib/runtime/**,lib/codex-manager/commands/rotation.ts,index.ts}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Runtime rotation is default-on through
codexRuntimeRotationProxy; users can opt out withcodex-multi-auth rotation disableorCODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0
Files:
lib/runtime/runtime-observability.ts
lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (lib/AGENTS.md)
lib/runtime-rotation-proxy.ts: Runtime rotation code must preserve pass-through semantics except for auth/provider headers that intentionally change
Runtime proxy client-facing headers must not expose account emails or tokens
Runtime rotation should fail open to normal official Codex forwarding when startup helpers are unavailable
Never add account emails/tokens to runtime proxy client responses
Files:
lib/runtime-rotation-proxy.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:26:24.990Z
Learning: Keep runtime rotation default-on behavior aligned with explicit release and migration documentation
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:26:45.615Z
Learning: Credentials must stay local and never be transmitted outside the local environment; OAuth account credentials are intended for personal development use only
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:26:45.615Z
Learning: Runtime Responses rotation must be loopback-only; do not expose rotation proxy outside of localhost
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:26:45.615Z
Learning: Official Codex CLI install paths must retain ownership of the `codex` binary; do not override or patch the official `codex` command
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-06-02T08:26:45.615Z
Learning: Settings validation must run after configuration changes to verify integrity; always validate with `codex-multi-auth status`, `codex-multi-auth check`, and `codex-multi-auth forecast --live`
🪛 LanguageTool
docs/releases/v2.1.13-beta.3.md
[uncategorized] ~4-~4: Do not mix variants of the same word (‘pre-release’ and ‘prerelease’) within a single text.
Context: ...ached statusline (#500), and a round of pre-release hardening surfaced by a whole-tree stre...
(EN_WORD_COHERENCY)
[style] ~95-~95: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...-readable output, with a stable shape whether or not accounts are configured. ## Pre-releas...
(WHETHER)
[uncategorized] ~97-~97: Do not mix variants of the same word (‘pre-release’ and ‘prerelease’) within a single text.
Context: ...her or not accounts are configured. ## Pre-release hardening (#503) - Strip inbound `cook...
(EN_WORD_COHERENCY)
🔇 Additional comments (17)
package.json (2)
3-3: LGTM!
164-165: LGTM!Also applies to: 172-172
.codex-plugin/plugin.json (1)
3-3: LGTM!AGENTS.md (1)
6-6: LGTM!README.md (1)
386-386: LGTM!docs/README.md (1)
35-35: LGTM!docs/releases/v2.1.13-beta.3.md (1)
1-108: LGTM!test/mcodex-statusline-scope.test.ts (1)
3-3: LGTM!Also applies to: 84-87
lib/local-bridge.ts (1)
101-104: LGTM!lib/runtime-rotation-proxy.ts (3)
271-274: LGTM!
899-958: LGTM!
1794-1794: LGTM!Also applies to: 1819-1819, 1916-1916, 1962-1962
test/local-bridge.test.ts (2)
263-275: LGTM!
324-335: LGTM!test/runtime-rotation-proxy.test.ts (1)
1123-1137: LGTM!test/runtime-observability-dir-mode.test.ts (2)
1-1: use vitest globals here.
test/runtime-observability-dir-mode.test.ts:1still imports vitest globals explicitly even though*.test.tsuses the global api in this repo.
19-35: reuseremoveWithRetryfor cleanup.
test/runtime-observability-dir-mode.test.ts:19-35reimplements windows delete retry withrmSyncand only coversebusy/eperm. that missesenotemptyand drifts from the shared helper the test guidelines require.
Addresses the #503 round-3 review: the win32 persistence test now also asserts writeFile received mode 0o600, so an owner-only-perms regression on the snapshot file fails the suite.
1 participant
Summary
Pre-release hardening surfaced by a deep, whole-tree stress test of the merged
main(#499 audit + #500 mcodex) — multi-agent adversarial code audit + runtime fuzz. The tree is already well-hardened; these close the residual MEDIUM/LOW gaps. No CRITICAL/HIGH product bugs were found.Changes
Egress (never forward inbound client credentials upstream)
runtime-rotation-proxy.ts+local-bridge.ts: strip inboundcookie+proxy-authorizationon the outbound request (joined the existingauthorization/x-api-keystripping). A client cookie would otherwise travel upstream with the managed OAuth Bearer.runtime-rotation-proxy.tsreadErrorBody: bounded the error-body read (per-read idle timeout + 1 MB cap, cancel on stall). The outbound abort timer is cleared once headers arrive, so a stalled 429/5xx body could previously hang the handler forever — the success path was stall-bounded, the error path wasn't.Windows fs resilience (gaps the runtime fuzz reproduced)
storage.ts: the secret-bearing WAL write and temp write had no retry and temp cleanup was single-shot — a transientEBUSY/EPERMcould fail a valid save or strand a token-bearing.tmp. Wrapped all three inwithFileOperationRetry. AlignedcopyFileWithRetry/renameFileWithRetryto the sharedshouldRetryFileOperationtaxonomy (+ENOTEMPTY/EACCES/EAGAIN). The primary commit rename keeps its existingEPERM/EBUSY5-attempt contract unchanged.quota-cache.ts: widenedRETRYABLE_FS_CODESto the shared set.Least-privilege
runtime/runtime-observability.ts: snapshot now written0o600/ dir0o700(account id/label), matching the other sensitive writers. No-op on win32, owner-only on POSIX.Tests
Cookie-strip regressions on both egress paths; existing storage WAL/temp/backup retry coverage stays green (rename contract intact). Cleaned a stale
eslint-disable+ unused import left in the merged mcodex statusline test.Verification
typecheck clean · lint clean · full suite 4246 passed / 1 skipped.
Out of scope (flagged for follow-up, not in this PR)
mainis the dev-onlyvitest <4.1.0UI advisory (GHSA-5xrq-8626-4rwp).npm audit --omit=devis clean and nothing test-related ships, butnpm run audit:ciexits non-zero — bumpvitest/@vitest/ui/@vitest/coverage-v8to^4.1.0(clean fix) before tagging a release.mcodexbin is user-facing);documentation.test.tsdoesn't enforce PR parity so CI is green, but worth a line before tag.🤖 Generated with Claude Code
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
pre-release hardening pass closing medium/low gaps surfaced by adversarial stress testing on
main. no critical product bugs were found; the changes are targeted and well-scoped.cookieandproxy-authorizationare now stripped alongsideauthorization/x-api-keyon both theruntime-rotation-proxyandlocal-bridgeoutbound paths, with regression coverage in both test files.readErrorBodynow applies a per-chunk idle timeout and a 1 MB size cap so a stalled 429/5xx body can no longer hang the handler indefinitely after headers arrive; the fallback path for non-streamable responses uses the existingwithTimeouthelper.withFileOperationRetry;copyFileWithRetry/renameFileWithRetryandquota-cache.tsalign to the sharedshouldRetryFileOperationtaxonomy.Confidence Score: 4/5
safe to merge with the outstanding cleanupWal retry gap still open; the new readErrorBody logic is correct but untested at the stall/cap boundaries
the egress stripping, fs-retry taxonomy unification, and observability perms changes are all correct and well-covered. the new readErrorBody bounded-read logic is the one area where the suite doesn't exercise the timeout or overflow paths, so a silent regression there is possible. cleanupWal still lacks the retry that cleanupTemp received in this very PR, leaving a token-bearing .wal file potentially stranded on windows
lib/storage.ts (cleanupWal retry gap) and lib/runtime-rotation-proxy.ts (readErrorBody stall/cap test coverage)
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant Bridge as local-bridge / rotation-proxy participant Upstream as OpenAI upstream Client->>Bridge: POST /v1/... (Authorization, x-api-key, Cookie, Proxy-Authorization) Note over Bridge: forwardHeaders() strips:<br/>cookie, proxy-authorization,<br/>x-api-key, authorization (inbound) Bridge->>Bridge: inject managed OAuth Bearer Bridge->>Upstream: POST /v1/... (Authorization: Bearer managed) Upstream-->>Bridge: HTTP 429 / 4xx / 5xx (headers arrive) Note over Bridge: outbound abort timer cleared on headers Bridge->>Bridge: readErrorBody(response, streamStallTimeoutMs) alt "body stalls > streamStallTimeoutMs" Bridge->>Bridge: reader.cancel() return partial text else "body > 1 MB" Bridge->>Bridge: break + reader.cancel() return capped text end Bridge-->>Client: proxied error responseComments Outside Diff (2)
lib/storage.ts, line 1875-1881 (link)cleanupWalskipped the retry treatment thatcleanupTempjust received, even thoughwalPathstores the identical secret-bearing payload — the full serialised account store with refresh tokens. on windows a transient av-scanner or indexer lock on that file will silently swallow the unlink, leaving a0o600wal containing live tokens next to the destination indefinitely. the fix is one line, same pattern ascleanupTemp.Prompt To Fix With AI
lib/storage.ts, line 1875-1881 (link)cleanupWalmissing retry, token-bearing .wal can be stranded on windowscleanupTempreceivedwithFileOperationRetryin this PR, butcleanupWaldid not.walPathholds the identical 0o600 payload — the full serialised account store with refresh tokens. a transient AV/indexer lock on win32 silently swallows the unlink, leaving a secret-bearing.walnext to the destination indefinitely. same one-line fix ascleanupTemp.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (5): Last reviewed commit: "test(observability): assert snapshot tem..." | Re-trigger Greptile