Skip to content

fix(credential-pool): honor Codex quota across internal requests - #1889

Open
trac3r00 wants to merge 6 commits into
code-yeongyu:mainfrom
trac3r00:fix/codex-quota-pr
Open

trac3r00 wants to merge 6 commits into
code-yeongyu:mainfrom
trac3r00:fix/codex-quota-pr

Conversation

@trac3r00

@trac3r00 trac3r00 commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Check Codex OAuth account quota before automatic selection. Included quota
    takes priority; unknown, cooling, and leased accounts cannot accidentally
    authorize paid usage.
  • Preserve explicit API-key bypass while title generation, compaction, branch
    summaries, side queries, vision, and worker requests retain account rotation
    and session-configured model fallback.
  • Surface safe account-switch notices. Sync/async listener failures and
    self-unsubscription cannot interrupt routing or peer observers.
  • Preserve remote compaction's local-summary fallback, including legacy runtime
    callers, while retaining numeric HTTP status for active account rotation.

Prerequisite / overlap

This branch includes the implementation and regressions from #1769 as a
separate attributed prerequisite commit (29b8846e), preserving newer
upstream tests. It replaces an earlier competing classifier expression.
Please integrate #1769 first, or retain that credited prerequisite here.
It can be dropped from this branch once #1769 lands.

The feature was integrated on cd317ee6472dde1658a75e644637691ba5d41bc1.
Current-main fallback defaults, deferred model switches, context repair, and
TUI changes are preserved.

Validation

  • npm ci --ignore-scripts with unchanged manifests and lockfiles.
  • npm run check: clean formatting, dependency/import/lock checks,
    root/scripts typechecks, and browser smoke.
  • npm run build:npm: all seven workspace phases passed.
  • 33 prerequisite tests passed after observing the expected failing cases.
  • 146 focused tests passed, including the new upstream Model switch refuses or silently skips a model that one compaction would make usable; fallback rejects the rung instead of repairing it #1873 regressions.
  • 669 adjacent title/compaction/side-query/vision/documentation tests passed.
  • Real isolated CLI/TUI notice rendering, native memory-supervisor launch,
    generated worker RPC, title generation, and clean worker exit were exercised
    with synthetic credentials and faux generation.
  • Independent integration review: APPROVE.

Tests ran with inherited ZAI_API_KEY removed. No personal auth/settings or
generated distribution files are included. Paid-credit admission was simulated,
not verified by deliberately spending credits. TUI evidence covers rendering,
not a shutdown pass.

The portable regression files live under
packages/coding-agent/test/suite/{codex-quota,codex-runtime-routing,codex-auxiliary-routing,account-notices,internal-model-request}.test.*.


Summary by cubic

Automatic Codex OAuth account selection now checks included quota before routing a request, so exhausted, cooling, leased, or unknown-quota accounts cannot reach paid usage. It admits only confirmed eligible accounts by default, while explicit API-key requests remain pinned and auxiliary requests use session-scoped fallback without changing the active chat model.

Routing and notices

  • Title generation, compaction, branch summaries, /btw, vision, and worker requests retain account rotation and configured model fallback.
  • Internal requests without a fallback chain preserve the original error identity instead of rewriting it.
  • Remote Codex compaction preserves its local-summary fallback, including for legacy callers.
  • Subscription-limit prose now triggers credential failover instead of ending the request, incorporating the requirement from #1769.
  • Provider ids are normalized through normalizeProviderId so quota admission and notices follow the chatgpt-subscription rename while legacy openai-codex ids keep routing.
  • Account-switch notices contain no credentials, and failing or self-removing listeners cannot interrupt routing or other observers.
  • Interactive and print modes render account-switch and internal-model fallback notices.
  • Rotation hook options are type-complete on the rebased base, keeping isStreamStart and event typing in sync.

Written for commit d7bcb84. Summary will update on new commits.

Review in cubic

@trac3r00

Copy link
Copy Markdown
Contributor Author

PR 1889 CI status (post-fix)

PR: #1889
Head at push: 84fc79a; fix commit added on top preserves internal error identity.

My regression (fixed)

test/suite/agent-session-compaction.test.ts > notifies extensions when auto-compaction fails
was re-labeled Summarization failed: summary generator blew up because the
internal helper lazy-wrapped the native stream even when no fallback chain
existed. Fixed by returning the native stream directly when fallback is
disabled or the model has no configured chain (RetryFallbackController.hasConfiguredChain()).
Local: the test now passes.

Pre-existing failures on the base (NOT from this PR)

Reproduced at clean origin/main (cd317ee6) with zero local edits from this PR:

  1. packages/ai/test/zai-coding-plan-models.test.ts > uses API-equivalent reference costs

    • expected {input:0.075, output:0.25, cacheRead:0.015}, got {input:0.15, output:0.5, cacheRead:0.03}.
    • Deterministic static catalog-cost mismatch in packages/ai (glm-5.3-flash).
    • This PR touches zero files under packages/ai (verified: git diff origin/main...HEAD -- packages/ai is empty).
  2. packages/coding-agent/test/suite/agent-session-compaction.test.ts > compacts trailing tool results before provider call 2 when they push context over the threshold

    • expected 4014 to be less than or equal to 4000.
    • Fails locally at clean main; passed in CI's coding-agent 1/3 shard, so this is a
      boundary-fragile token assertion, not a CI-failing PR change.

orientpine and others added 5 commits September 22, 2026 20:00
Incorporate the combined implementation and regressions from senpi PR code-yeongyu#1769
as a separately reviewable prerequisite for quota-aware internal routing.
Preserve newer upstream tests and document the dependency.

Constraint: Reuse code-yeongyu#1769 rather than maintain a competing classifier
Directive: Drop this prerequisite when PR code-yeongyu#1769 is integrated
Confidence: high
Scope-risk: narrow
Prefer confirmed included quota before paid credits, including cooling and
leased accounts, and isolate account-switch observers from request execution.
Route auxiliary model calls through session-scoped fallback while retaining
explicit credential overrides and remote compaction's local fallback.

Add portable regressions, safe session notices, API documentation, and fork
change tracking. Preserve current-main model-switch and fallback behavior.

Constraint: Unknown quota must never authorize paid usage
Constraint: Explicit request keys retain upstream bypass semantics
Rejected: Rotate requests carrying explicit external keys | violates caller intent
Directive: Keep notification failures observational and credential-free
Confidence: high
Scope-risk: moderate
Not-tested: Paid credits were simulated; no deliberate live credit consumption
The subscription provider id was renamed from `openai-codex` to
`chatgpt-subscription` (senpi#1989) after this branch's base, so the quota
admission, the account-switch notices and the internal-request OAuth strip now
compare through pi-ai's `normalizeProviderId`, and the account-id header
rebuild uses the renamed `extractChatGptSubscriptionAccountId` helper. Legacy
ids keep resolving through the same helper, so the existing fixtures still
exercise the alias path.

Constraint: Upstream renamed the provider id after this branch's base
Rejected: Comparing the canonical id directly | legacy ids in fixtures and stored configs would stop routing
Confidence: high
Scope-risk: narrow
Not-tested: Windows-only CI jobs
runRotation's option Pick omitted isStreamStart, so the rotation-events
threading and the non-streaming wrapper failed to typecheck on the rebased
base. The Pick now names all three hooks and the non-streaming call states its
event type explicitly.

Constraint: Upstream's RunCredentialFailoverOptions gained isStreamStart
Confidence: high
Scope-risk: narrow

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants