fix(runtime): emit consistent token_invalidated body on upstream-401 path - #497
Conversation
…path The upstream-401 invalidation path forwarded the raw OpenAI body, so clients keying off error.code saw a stable "token_invalidated" code on the refresh-failure path but not here (Greptile P3 on #496). - Add buildTokenInvalidationBody(): wraps both paths in the same { error: { message, code: "token_invalidated" } } shape, preserving the upstream message when present and falling back to a stable message for non-JSON bodies (no markup echoed to clients). - Force content-type: application/json and drop content-length/-encoding via responseHeadersForClient since the body is rewritten. - Document that applyMonotonicAuthCooldown intentionally uses Date.now() (not the injected now()) to match the markAccountCoolingDown/nowMs() write domain, guarding against a regression that would defeat the monotonic cooldown race protection. - Strengthen existing upstream-401 and html-body tests to assert the client contract (code + message). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
📝 WalkthroughWalkthroughthe proxy now normalizes upstream 401 token-invalidation responses into a consistent json error contract Changestoken invalidation and auth flow updates
Sequence DiagramsequenceDiagram
participant Upstream
participant Proxy
participant Client
Upstream->>Proxy: 401 (json or non-json body)
Proxy->>Proxy: buildTokenInvalidationBody(bodyText)
Proxy->>Proxy: extract top-level message or error.message or use fallback
Proxy->>Proxy: apply cooldown and clear session affinity
Proxy->>Client: 401 with application/json and { error: { code: "token_invalidated", message } }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
review notes:
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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 (2)
lib/runtime-rotation-proxy.ts (1)
1558-1566: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueconsider reusing
TOKEN_INVALIDATED_FALLBACK_MESSAGEconstant here.the refresh-failure path at
lib/runtime-rotation-proxy.ts:1559hardcodes the same message that's now defined as a constant at line 852. using the constant keeps both invalidation paths in sync if the message ever changes.♻️ proposed fix
res.end( JSON.stringify({ error: { - message: "OAuth token has been invalidated. Please re-login.", - code: "token_invalidated", + message: TOKEN_INVALIDATED_FALLBACK_MESSAGE, + code: TOKEN_INVALIDATED_CODE, }, }), );🤖 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 1558 - 1566, Replace the hardcoded invalidation message in the refresh-failure response (the block that calls res.writeHead/ res.end to return the OAuth token invalidated JSON) with the existing TOKEN_INVALIDATED_FALLBACK_MESSAGE constant so both invalidation paths remain in sync; update the JSON construction to use TOKEN_INVALIDATED_FALLBACK_MESSAGE for the error.message field and keep the code property "token_invalidated" unchanged.test/runtime-rotation-proxy.test.ts (1)
1846-1848: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winmissing test for top-level
messageextraction path.
lib/runtime-rotation-proxy.ts:868-870checksparsed.messagebefore falling through toparsed.error.message. current tests only cover the nestederror.messagepath and the non-json fallback. a regression test with{ message: "top-level msg" }(noerrorwrapper) would cover the branch at line 869.🧪 suggested test case
it("extracts top-level message from upstream 401 invalidation body", async () => { const now = Date.now(); const accountManager = new AccountManager(undefined, createStorage(now, 2)); const invalidationBody = JSON.stringify({ message: "Token has been invalidated by admin", }); const { calls, fetchImpl } = createRecordingFetch(() => new Response(invalidationBody, { status: HTTP_STATUS.UNAUTHORIZED, headers: { "content-type": "application/json" }, }), ); const proxy = await startProxy({ accountManager, fetchImpl }); const response = await postResponses(proxy, { model: "gpt-5-codex" }); const body = (await response.json()) as { error: { message: string; code: string } }; expect(response.status).toBe(HTTP_STATUS.UNAUTHORIZED); expect(body.error.code).toBe("token_invalidated"); expect(body.error.message).toBe("Token has been invalidated by admin"); expect(calls).toHaveLength(1); });🤖 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 1846 - 1848, Add a regression test in test/runtime-rotation-proxy.test.ts that covers the top-level parsed.message extraction path (the branch checked in lib/runtime-rotation-proxy.ts where parsed.message is used before parsed.error.message). Create an AccountManager and Recording fetch that returns a JSON body like { message: "Token has been invalidated by admin" } with HTTP_STATUS.UNAUTHORIZED, start the proxy via startProxy, call postResponses (model: "gpt-5-codex"), and assert the response status is HTTP_STATUS.UNAUTHORIZED, that the returned body.error.code equals "token_invalidated" and body.error.message equals the top-level message, and that the upstream fetch was called exactly once (use existing helpers createRecordingFetch, createStorage, AccountManager, startProxy, postResponses, HTTP_STATUS to mirror other tests).
🤖 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 `@lib/runtime-rotation-proxy.ts`:
- Around line 1558-1566: Replace the hardcoded invalidation message in the
refresh-failure response (the block that calls res.writeHead/ res.end to return
the OAuth token invalidated JSON) with the existing
TOKEN_INVALIDATED_FALLBACK_MESSAGE constant so both invalidation paths remain in
sync; update the JSON construction to use TOKEN_INVALIDATED_FALLBACK_MESSAGE for
the error.message field and keep the code property "token_invalidated"
unchanged.
In `@test/runtime-rotation-proxy.test.ts`:
- Around line 1846-1848: Add a regression test in
test/runtime-rotation-proxy.test.ts that covers the top-level parsed.message
extraction path (the branch checked in lib/runtime-rotation-proxy.ts where
parsed.message is used before parsed.error.message). Create an AccountManager
and Recording fetch that returns a JSON body like { message: "Token has been
invalidated by admin" } with HTTP_STATUS.UNAUTHORIZED, start the proxy via
startProxy, call postResponses (model: "gpt-5-codex"), and assert the response
status is HTTP_STATUS.UNAUTHORIZED, that the returned body.error.code equals
"token_invalidated" and body.error.message equals the top-level message, and
that the upstream fetch was called exactly once (use existing helpers
createRecordingFetch, createStorage, AccountManager, startProxy, postResponses,
HTTP_STATUS to mirror other tests).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 28c3c90f-694c-436e-a500-283957547e1e
📒 Files selected for processing (2)
lib/runtime-rotation-proxy.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 (10)
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only (
"type": "module"), Node >= 18
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
as any,@ts-ignore, or@ts-expect-errorTypeScript assertions
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.ts
{**/scripts/**/*.js,**/test/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use bare recursive delete logic in Windows-sensitive scripts/tests without retry handling for transient
EBUSY/EPERM/ENOTEMPTYerrors
Files:
test/runtime-rotation-proxy.test.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/runtime-rotation-proxy.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (README.md)
**/*.{ts,tsx,js,jsx}: Implement default-on runtime Responses rotation for request-bearing forwarded Codex CLI/app sessions, with opt-out support viaCODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0
Store project-scoped accounts under~/.codex/multi-auth/projects/<project-key>/openai-codex-accounts.jsonfor repo-specific workflows
Support environment variable overrides for configuration includingCODEX_MULTI_AUTH_DIR,CODEX_MODE,CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY,CODEX_TUI_COLOR_PROFILE, and others as documented
Implement account health checks withcodex-multi-auth checkcommand that validates saved account credentials and state
Implement quota forecasting and budget guards to prevent exhausting account quota within a session
Record local usage in a ledger at~/.codex/multi-auth/usage/usage-ledger.jsonlfor per-project tracking and reporting
Implement bounded outbound request budget so one prompt cannot walk the full account pool indefinitely
Trigger short cooldown instead of continuing aggressive rotation when repeated cross-account 5xx bursts are detected
Stagger proactive token refresh to reduce background refresh bursts across the account pool
Expose recent runtime request metrics incodex-multi-auth statustext output and machine-readable metrics incodex-multi-auth report --json
Make OAuth callback listen on port 1455 for login flows
Support device authorization flow viacodex-multi-auth login --device-authas an alternate to browser-based OAuth for headless environments
Implement dashboard hotkeys including Up/Down for navigation, Enter for selection, 1-9 for quick switch, / for search, ? for help, Q to back, S to set account, R to refresh, E to enable/disable, and D to delete
Support named local pool backup export with filename prompt in the Settings > Experimental menu
Implementcodex-multi-auth doctor --fixcommand to diagnose and apply the safest fixes for storage or account state issues
Implementcodex-multi-auth fix --dry-run...
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.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.ts
**/lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/lib/runtime-rotation-proxy.ts: Keep runtime rotation default-on behavior aligned with explicit release and migration documentation
Do not expose account emails or tokens in runtime proxy client response headers or logs
Files:
lib/runtime-rotation-proxy.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-rotation-proxy.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
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/runtime-rotation-proxy.ts
🔇 Additional comments (4)
lib/runtime-rotation-proxy.ts (3)
669-677: LGTM!
851-884: LGTM!
1785-1791: LGTM!test/runtime-rotation-proxy.test.ts (1)
1860-1866: LGTM!Also applies to: 2037-2041
…ion builder + unit-test branches
Addresses two Greptile P2 comments on this PR:
- The refresh-failure invalidation exit still hardcoded the body literals
the new constants were meant to centralise, reopening the exact drift
risk this PR closes. Route it through buildTokenInvalidationBody(""),
which yields the same { error: { message: <fallback>, code:
"token_invalidated" } } so both paths can no longer diverge.
- Export buildTokenInvalidationBody and add 8 unit tests covering all
message-extraction branches: empty input, top-level message, nested
error.message, top-level-wins priority, blank-top-level -> nested
fallback, non-JSON body, and no-usable-message fallback.
Full suite: 4062 tests pass (4054 + 8). typecheck/lint/build clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed both P2 comments in
Full suite now 4062 passing (4054 + 8 new); typecheck/lint/build clean. |
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 (2)
test/runtime-rotation-proxy.test.ts (1)
1844-2045: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winadd a regression for the rewritten 401 headers.
lib/runtime-rotation-proxy.ts:1784-1787now rewrites both headers and body, buttest/runtime-rotation-proxy.test.ts:1844-2045only asserts the json payload. please also assertcontent-typeis forced to json and stalecontent-length/content-encodingare absent, so this contract does not regress silently.🤖 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 1844 - 2045, The tests that validate rewritten 401 responses (e.g., the "returns 401 to client and does not rotate when upstream explicitly invalidates the token" case and similar 401 cases) only assert the JSON payload but not the rewritten headers; since lib/runtime-rotation-proxy.ts at the rewrite block (around the header/body rewrite code) now forces JSON and strips stale headers, update these tests (locate the response returned from postResponses(proxy, ...)) to also assert response.headers.get("content-type") === "application/json" and that response.headers.get("content-length") and response.headers.get("content-encoding") are null/absent so the header-rewrite contract is covered. Ensure you add the same assertions in the other 401-related tests in this diff that expect rewritten 401 bodies.lib/runtime-rotation-proxy.ts (1)
861-884:⚠️ Potential issue | 🟠 Major | ⚡ Quick winsanitize json-carried invalidation messages before echoing them.
lib/runtime-rotation-proxy.ts:868-875still reflects any string found inmessageorerror.messageverbatim. if upstream sends json with an html fragment in that field, this helper will return it unchanged, which breaks the "no markup echoed" contract on the rewritten 401 path. fall back to the stable message for markup-like content here, and add a vitest regression neartest/runtime-rotation-proxy.test.ts:2064-2102.🤖 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 861 - 884, The helper buildTokenInvalidationBody currently trusts JSON fields verbatim; update it to reject markup-like messages by validating any extracted direct or nested message (from parsed.message or parsed.error.message) for HTML/markup (e.g., presence of '<'/'>' or a tag-like pattern like /<[^>]+>/ or common doctype/script fragments) and only accept it if it’s plain text; otherwise keep TOKEN_INVALIDATED_FALLBACK_MESSAGE. Ensure the returned JSON still uses TOKEN_INVALIDATED_CODE. Add a vitest regression near test/runtime-rotation-proxy.test.ts that sends upstream JSON with an HTML fragment in message/error.message and asserts the response body uses TOKEN_INVALIDATED_FALLBACK_MESSAGE (not the markup).
🤖 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 `@lib/runtime-rotation-proxy.ts`:
- Around line 861-884: The helper buildTokenInvalidationBody currently trusts
JSON fields verbatim; update it to reject markup-like messages by validating any
extracted direct or nested message (from parsed.message or parsed.error.message)
for HTML/markup (e.g., presence of '<'/'>' or a tag-like pattern like /<[^>]+>/
or common doctype/script fragments) and only accept it if it’s plain text;
otherwise keep TOKEN_INVALIDATED_FALLBACK_MESSAGE. Ensure the returned JSON
still uses TOKEN_INVALIDATED_CODE. Add a vitest regression near
test/runtime-rotation-proxy.test.ts that sends upstream JSON with an HTML
fragment in message/error.message and asserts the response body uses
TOKEN_INVALIDATED_FALLBACK_MESSAGE (not the markup).
In `@test/runtime-rotation-proxy.test.ts`:
- Around line 1844-2045: The tests that validate rewritten 401 responses (e.g.,
the "returns 401 to client and does not rotate when upstream explicitly
invalidates the token" case and similar 401 cases) only assert the JSON payload
but not the rewritten headers; since lib/runtime-rotation-proxy.ts at the
rewrite block (around the header/body rewrite code) now forces JSON and strips
stale headers, update these tests (locate the response returned from
postResponses(proxy, ...)) to also assert response.headers.get("content-type")
=== "application/json" and that response.headers.get("content-length") and
response.headers.get("content-encoding") are null/absent so the header-rewrite
contract is covered. Ensure you add the same assertions in the other 401-related
tests in this diff that expect rewritten 401 bodies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 18e3dc02-7892-4d7b-860b-9f84d94ef5d0
📒 Files selected for processing (2)
lib/runtime-rotation-proxy.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 (10)
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only (
"type": "module"), Node >= 18
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
as any,@ts-ignore, or@ts-expect-errorTypeScript assertions
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.ts
{**/scripts/**/*.js,**/test/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use bare recursive delete logic in Windows-sensitive scripts/tests without retry handling for transient
EBUSY/EPERM/ENOTEMPTYerrors
Files:
test/runtime-rotation-proxy.test.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/runtime-rotation-proxy.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (README.md)
**/*.{ts,tsx,js,jsx}: Implement default-on runtime Responses rotation for request-bearing forwarded Codex CLI/app sessions, with opt-out support viaCODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY=0
Store project-scoped accounts under~/.codex/multi-auth/projects/<project-key>/openai-codex-accounts.jsonfor repo-specific workflows
Support environment variable overrides for configuration includingCODEX_MULTI_AUTH_DIR,CODEX_MODE,CODEX_MULTI_AUTH_RUNTIME_ROTATION_PROXY,CODEX_TUI_COLOR_PROFILE, and others as documented
Implement account health checks withcodex-multi-auth checkcommand that validates saved account credentials and state
Implement quota forecasting and budget guards to prevent exhausting account quota within a session
Record local usage in a ledger at~/.codex/multi-auth/usage/usage-ledger.jsonlfor per-project tracking and reporting
Implement bounded outbound request budget so one prompt cannot walk the full account pool indefinitely
Trigger short cooldown instead of continuing aggressive rotation when repeated cross-account 5xx bursts are detected
Stagger proactive token refresh to reduce background refresh bursts across the account pool
Expose recent runtime request metrics incodex-multi-auth statustext output and machine-readable metrics incodex-multi-auth report --json
Make OAuth callback listen on port 1455 for login flows
Support device authorization flow viacodex-multi-auth login --device-authas an alternate to browser-based OAuth for headless environments
Implement dashboard hotkeys including Up/Down for navigation, Enter for selection, 1-9 for quick switch, / for search, ? for help, Q to back, S to set account, R to refresh, E to enable/disable, and D to delete
Support named local pool backup export with filename prompt in the Settings > Experimental menu
Implementcodex-multi-auth doctor --fixcommand to diagnose and apply the safest fixes for storage or account state issues
Implementcodex-multi-auth fix --dry-run...
Files:
test/runtime-rotation-proxy.test.tslib/runtime-rotation-proxy.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.ts
**/lib/runtime-rotation-proxy.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/lib/runtime-rotation-proxy.ts: Keep runtime rotation default-on behavior aligned with explicit release and migration documentation
Do not expose account emails or tokens in runtime proxy client response headers or logs
Files:
lib/runtime-rotation-proxy.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-rotation-proxy.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
lib/**
⚙️ CodeRabbit configuration file
focus on auth rotation, windows filesystem IO, and concurrency. verify every change cites affected tests (vitest) and that new queues handle EBUSY/429 scenarios. check for logging that leaks tokens or emails.
Files:
lib/runtime-rotation-proxy.ts
Prerelease that ships the cascade OAuth token-invalidation fix from issue #495 to npm under the `beta` dist-tag: the 401 handler now detects explicit token-invalidation responses and returns them to the client instead of rotating through every account (the rotation itself was tripping OpenAI's anti-abuse detection and invalidating accounts in sequence). Invalidated accounts get a monotonic 5-minute cooldown, session affinity is cleared, and both invalidation exit paths emit a consistent token_invalidated error body. Also adds a configurable minRotationIntervalMs sticky window. Carries forward multi-workspace support (beta.1) and the pinned-account 503 diagnostic (beta.0). Stable v2.1.13 will land once the issue #486 root cause is identified and patched. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Follow-up to #496. Addresses the unresolved Greptile review concerns from that PR's post-merge audit.
The core anti-cascade behavior shipped in #496 is correct, but two contract/robustness items remained:
1. Asymmetric 401 body (Greptile P3 → fixed)
The refresh-failure invalidation path synthesised
{error:{message,code:"token_invalidated"}}, but the upstream-401 invalidation path forwarded the raw OpenAI body verbatim — which carries no guaranteedcode. A client (or operator tooling) keying offerror.codeto distinguish "explicitly invalidated" from a generic 401 would catch one path and silently miss the other.buildTokenInvalidationBody()wraps both paths in the same{error:{message,code:"token_invalidated"}}shape.messagewhen present (error.messageor top-levelmessage); falls back to a stable message for non-JSON bodies — no HTML/markup is echoed back to clients.content-type: application/jsonand drops stalecontent-length/content-encoding(via the existingresponseHeadersForClient) since the body is rewritten.2. Clock-domain guard (Greptile P2 on
now()→ documented, no behavior change)applyMonotonicAuthCooldowndeliberately usesDate.now()rather than the proxy's injectednow(), because it compares againstcoolingDownUntil, which is written viamarkAccountCoolingDown→nowMs()(==Date.now()). Both sides must share the real-wall-clock domain; switching to the injectednow()would mis-compare an injected-clock value against a real-clock deadline and silently defeat the monotonic cooldown race protection. Added a comment so this isn't "fixed" into a regression.Test plan
error.code === "token_invalidated"and that the upstream message is preserved.codeand the stable fallback message (no markup leaked).tsc --noEmit: cleaneslint(ts + scripts): cleannpm run build: cleanvitest run --maxWorkers=1)🤖 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
this pr closes the two unresolved contract items from the #496 post-merge audit: it unifies the 401 response body across both token-invalidation vectors (refresh-failure and upstream-401) through a shared
buildTokenInvalidationBody()builder, and documents whyapplyMonotonicAuthCooldownintentionally usesDate.now()rather than the injected clock.buildTokenInvalidationBody()normalises both exit paths to{ error: { message, code: "token_invalidated" } }, extracting the upstream human-readable message (top-level or nestederror.message) and falling back to a stable string for non-json/html bodies;content-lengthandcontent-encodingare stripped by the existingresponseHeadersForClienthelper before the rewritten body is sent.buildTokenInvalidationBody("")instead of inline literals, so the two paths share a single source of truth for both the code string and the fallback message.buildTokenInvalidationBodycover all extraction branches, and the two integration tests are strengthened to asserterror.codeand the preserved/fallback message.Confidence Score: 5/5
safe to merge — both invalidation exit paths now share a single builder with consistent output, all extraction branches are unit-tested, and neither the clock-domain guard nor header handling introduce regressions.
the change is narrow and surgical: a pure body-normalisation layer that falls back safely for any upstream input, with the existing
responseHeadersForClientcorrectly stripping stalecontent-lengthandcontent-encodingbefore the rewritten json body is sent. both previously reported contract gaps are closed, unit coverage now exercises all message-extraction branches, and the two integration tests verify end-to-end shape on both vectors.no files require special attention
Important Files Changed
buildTokenInvalidationBody()builder with correct message extraction and fallback; wires both invalidation exit paths through it; adds a clock-domain comment onapplyMonotonicAuthCooldown; no new issues founderror.code/error.message; adds a dedicatedbuildTokenInvalidationBodydescribe block covering all five extraction branches including the previously untested nested and priority casesSequence Diagram
sequenceDiagram participant Client participant Proxy as RuntimeRotationProxy participant OpenAI as Upstream (OpenAI) Note over Proxy,OpenAI: Path A — refresh-failure invalidation Proxy->>OpenAI: POST /token/refresh OpenAI-->>Proxy: 4xx invalidated Proxy->>Proxy: buildTokenInvalidationBody("") Note right of Proxy: fallback message, code:"token_invalidated" Proxy-->>Client: "401 {error:{message,code:"token_invalidated"}}" Note over Proxy,OpenAI: Path B — upstream-401 invalidation Proxy->>OpenAI: POST /responses OpenAI-->>Proxy: "401 {error:{message:"Encountered invalidated…"}}" Proxy->>Proxy: isTokenInvalidationError(bodyText) Proxy->>Proxy: buildTokenInvalidationBody(bodyText) Note right of Proxy: extracts nested error.message,<br/>code:"token_invalidated",<br/>strips content-length/encoding via<br/>responseHeadersForClient Proxy-->>Client: "401 {error:{message,code:"token_invalidated"}}"Comments Outside Diff (2)
lib/runtime-rotation-proxy.ts, line 1558-1566 (link)TOKEN_INVALIDATED_CODEandTOKEN_INVALIDATED_FALLBACK_MESSAGEare introduced in this PR but never consumed at the refresh-failure exit point (line 1558–1565). if either constant drifts (even a capitalisation fix), the two invalidation paths silently emit different strings again — exactly the asymmetry this PR exists to prevent. the fix is to replace the inline literals with the new constants (or callbuildTokenInvalidationBody("")to also go through the shared builder).Prompt To Fix With AI
test/runtime-rotation-proxy.test.ts, line 1857-1870 (link)buildTokenInvalidationBodybranchesthe new function has five distinct message-extraction branches (top-level
message, nestederror.message, whitespace-only non-empty string, blank/falsymessagewith populatederror.message, non-JSON body). the integration tests added here exercise the top-level-message and non-JSON-fallback paths, but the nestederror.messagebranch and the "top-level wins over nested" priority rule are untested at unit level. if the extraction priority is ever adjusted, no unit test will catch the regression.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (2): Last reviewed commit: "refactor(runtime): route refresh-failure..." | Re-trigger Greptile