Skip to content

feat: add /openai-reset command to spend rate-limit reset credits - #50

Merged
ualtinok merged 1 commit into
cortexkit:mainfrom
iceteaSA:feat/openai-reset-command
Aug 13, 2026
Merged

feat: add /openai-reset command to spend rate-limit reset credits#50
ualtinok merged 1 commit into
cortexkit:mainfrom
iceteaSA:feat/openai-reset-command

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Adds /openai-reset: an operator-confirmed modal command that spends one Codex rate-limit reset credit to un-exhaust an OAuth account (main or any enabled fallback). Codex grants these credits per account (wham/rate-limit-reset-credits); until now the only way to spend one was a hand-built curl.

What it does

  • Preview (L1): one row per account — used %, applicable/available credit counts, eligibility. Ineligible rows (not exhausted, no applicable credits, per-account failures) render disabled with the reason; a disabled row cannot dispatch an action.
  • Confirm (L2): explicit destructive confirmation ("SPENDS 1 of N reset credits — irreversible") bound to the preview-time account identity. Confirm re-resolves identity fresh and refuses if the account changed, was removed/disabled, or the main account was switched since preview.
  • Result (L3): honest outcome rendering. No-ops (nothing_to_reset, no_credit) report the exact server code and never claim success. Unknown outcomes (http_error, network ambiguity) say so, and the retry reuses the same persisted redeem_request_id + credit_id rather than minting new ones.
  • Post-reset verification refreshes only the reset account's quota (new per-account selector on refreshAllQuota) and updates the sidebar.

Safety model

The credit spend is irreversible, so the redemption path is a small state machine persisted in config storage:

  • Credit selection and UUID minting happen inside one mutateAccounts lock; a second overlapping confirm (double-click, second process) reuses the persisted claim instead of minting a new UUID. Proven by a forced-overlap test that holds both invocations pre-finalize with a deferred consume, plus a direct two-concurrent-claims probe — both go red if the reuse branch is removed.
  • The consume POST always carries an explicit credit_id (earliest-expiring eligible credit) and aborts at 60 s, strictly below the 5-minute in-flight TTL, so a hung request cannot race a fresh claim.
  • Only the four documented server codes are terminal. Malformed/unknown responses classify as ambiguous and preserve the in-flight pair for an idempotent retry.
  • Corrupt persisted in-flight records resolve locally (ambiguous_local) without touching the wire; prototype-polluting account keys are rejected at normalization, mutation, and arg-decode.
  • Server-side dedup of a repeated redeem_request_id is documented as assumed, not verified — the retry copy never promises a free retry (verification-debt note in ARCHITECTURE.md).

Tests

776 pass / 0 fail (163 new assertions across reset wire client, state machine, storage normalization, command flow, and dialog). Safety tests are red-first: forced-overlap single-UUID, 60 s hanging consume, ambiguous/http_error state preservation, stale-modal and identity-change refusals, per-account isolation, corrupt-record local resolution. build, tsc, and Biome clean; lefthook passes.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Adds /openai-reset: a modal-only command to spend one Codex rate‑limit reset credit and un‑exhaust an OAuth account (main or any enabled fallback). Includes per‑account quota refresh, strict identity checks with fresh identity re‑resolution at confirm, and safe, idempotent redemption with clear outcomes.

  • New Features

    • Core (packages/opencode/src/core/reset-credits.ts): lists credits, checks eligibility via WHAM applicable_available_count, picks earliest‑expiring, and consumes with explicit credit_id; 60s consume timeout, 5‑min in‑flight TTL, cooldowns; persisted under reset and validated with isSafeResetAccountKey.
    • TUI: three‑stage modal (preview, confirm, result) with confirm‑time identity re‑resolution; honest outcomes and idempotent retries; errors via packages/opencode/src/util/error.ts.
    • Quota: refreshAllQuota accepts { accountKey }, reads the live main at call time, refreshes only the target, and clears only that account’s healthy mark; sidebar updates after reset.
    • RPC/TUI: protocol adds openai-reset; client supports per‑call timeouts (default 2s); server splits request‑delivery receipt timeout from socket inactivity (handler) timeout, tuned so a 60s consume can complete; TUI calls openai-reset with 90s.
    • Quota normalize: adds resetCreditsApplicable and preserves it across per‑turn pushes when omitted.
  • Tests

    • Cover credit listing/consumption, preconditions and cooldowns, persisted state and idempotency, per‑account refresh and identity checks, RPC client/server timeout behavior (including slow apply and stalled delivery), command registration, and end‑to‑end reset flows.

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

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 17 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread packages/opencode/src/core/reset-credits.ts
Comment thread packages/opencode/src/index.ts Outdated
Comment thread ARCHITECTURE.md Outdated
Comment thread packages/opencode/src/core/reset-credits.ts
Comment thread packages/opencode/src/tui/command-dialogs.tsx
@iceteaSA
iceteaSA force-pushed the feat/openai-reset-command branch 2 times, most recently from 035fbab to eeac9c0 Compare July 18, 2026 01:31
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Live-testing the branch surfaced two real bugs that all prior test fixtures missed, both now fixed and folded into the single commit:

1. The reset-credit list endpoint's wire shape was wrong in the original fixtures. The real GET /backend-api/wham/rate-limit-reset-credits response (captured live) uses a top-level credits array — not rate_limit_reset_credits — and carries no applicable_available_count at all; that count only exists on wham/usage (rate_limit_reset_credits.{available_count,applicable_available_count}). The parser now matches the live shape, and the applicable count is sourced exclusively from the normalized usage snapshot (resetCreditsApplicable) in both the preview and the confirm-time precondition re-check. All fixtures replaced with the captured response; a regression test pins the obsolete shape to zero credits.

2. The host DialogSelect hides disabled options entirely (live-verified) — ineligible accounts and the result text row were invisible, leaving a bare Refresh/Close dialog. Ineligible accounts now render as selectable rows that route to an informational result (never the spend confirm — enforced at both the dialog and command layer), and the result text renders as a visible inert row.

Verified end-to-end in a live TUI session: exhausted main shows eligible (3/3 applicable), non-exhausted fallback visible with its reason, confirm preview correct (spend/expiry/reset dates, identity-bound), cancel path clean — no credit was spent during verification. Full gate: 787 tests, build/typecheck/biome clean.

@iceteaSA
iceteaSA force-pushed the feat/openai-reset-command branch 4 times, most recently from 5521939 to 8e00305 Compare July 19, 2026 10:22
@ualtinok

Copy link
Copy Markdown
Contributor

Thanks for the implementation. This needs revision before merge:

  1. Rebase onto current main (v0.4.2).
  2. The TUI client allows 90 seconds, but the RPC server still destroys an apply socket after 2 seconds. Give reset apply a matching server-side timeout or move it to an async job, and add a delayed-apply regression test.
  3. Derive the main account identity from the resolved live access token, not persisted storage. Add a stale-storage plus changed-JWT test so confirmation cannot spend a newly switched main account's credit.
  4. Verify the backend's redeem idempotency directly: replay a consumed (redeem_request_id, credit_id) and prove it returns an already-consumed result without decrementing another credit. Do not rely on an unverified server-deduplication assumption for client retries.
  5. Preserve resetCreditsApplicable through quota normalization, account storage, and later transport pushes. Bound preview/listing calls with timeout coverage, and document the command in the root README.

Please rerun the full gate after the rebase.

@iceteaSA
iceteaSA force-pushed the feat/openai-reset-command branch from 8e00305 to 1ba4e37 Compare July 22, 2026 18:32
@ualtinok

Copy link
Copy Markdown
Contributor

Thanks for the revision. The RPC deadline, live main-token identity, and applicable-credit metadata paths are now addressed.

Two blockers remain:

  1. listResetCredits() still issues an unbounded GET. Apply a bounded abort signal and add timeout coverage for the preview/listing path.
  2. The retry tests still mock already_redeemed. The implementation itself accurately says a reused request ID does not prove the server did nothing. Before merging a credit-spending retry path, run a safe direct backend verification: consume once, replay the exact (redeem_request_id, credit_id), and prove the replay returns a terminal already-consumed result without decrementing another credit. Record that evidence in the PR.

Everything else from the prior review is resolved.

@iceteaSA
iceteaSA force-pushed the feat/openai-reset-command branch from 1ba4e37 to 293cddc Compare July 22, 2026 20:10
@iceteaSA

Copy link
Copy Markdown
Contributor Author

On the two remaining blockers:

  1. listResetCredits() bounded — fixed in 293cddc. The list GET now carries signal: AbortSignal.timeout(RESET_LIST_TIMEOUT_MS) (15s, matching the quota-fetch bound from fix(quota): bound and log the active quota fetch path #52); an abort surfaces through the existing catch as ResetCreditError('http_error'). Timeout coverage added for the preview/listing path (fake-timer test fails pre-fix on the missing signal, fires at exactly 15,000ms).

  2. Live dedup verification — agreed the already_redeemed mock proves only the client contract, not server behavior, and I won't claim it as verified. This is the one step I can't do unattended: it requires spending a real credit on a live account, then replaying the exact (redeem_request_id, credit_id) to confirm a terminal already-consumed result with no second decrement. The code is explicit that dedup is assumed-not-verified (ARCHITECTURE.md verification-debt note), and the retry path stays gated behind operator-confirmed interactive use until that live check is recorded. I'll run it against a live account and post the captured before/after credit counts + both responses here as the evidence — will update this thread once done.

@iceteaSA

Copy link
Copy Markdown
Contributor Author

Live backend dedup verification — run against the main account (plan: plus), which was genuinely at 100% on the primary window with 2 reset credits available. One real credit spent.

Sequence: consume once → replay the exact same (redeem_request_id, credit_id) → snapshot wham/usage after each.

redeem_request_id = 013119ed-2336-4f92-912f-6a4702a454f2
credit_id         = RateLimitResetCredit_cf1a69b7a4608191a1190ce84e86b58c

BEFORE                     primary_used_pct=100  available_count=2  applicable=2

STEP 1  first consume  →   { "code": "reset",            "windows_reset": 1, credit.status: "redeemed" }
STEP 2  usage          →   primary_used_pct=0    available_count=1  applicable=0

STEP 3  REPLAY (same)  →   { "code": "already_redeemed", "windows_reset": 0, credit.status: "redeemed" }
STEP 4  usage          →   primary_used_pct=0    available_count=1  applicable=0

Conclusion — server-side idempotency confirmed:

  • The replay returned already_redeemed with windows_reset: 0 (no reset applied a second time).
  • available_count stayed at 1 across the replay (Step 2 → Step 4) — no second credit was decremented.
  • The credit's redeemed_at on the replay response matched the first consume's timestamp (2026-07-23T19:49:33.226507Z), i.e. the server returned the original redemption record, not a new one.

This is the empirical basis the retry path relied on: replaying a consumed (redeem_request_id, credit_id) is safe — the server dedupes on the identifier and does not spend another credit. The assumed-not-verified caveat in ARCHITECTURE.md can now be updated to verified (dated, with this evidence). I'll fold that doc update into the branch.

@iceteaSA
iceteaSA force-pushed the feat/openai-reset-command branch from 293cddc to cb1e05a Compare July 23, 2026 19:50
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Doc updated in cb1e05a — the ARCHITECTURE.md verification-debt note now reads as verified (2026-07-23) with the live evidence above, rather than assumed-from-source. Both remaining blockers now closed: bounded listResetCredits() GET (293cddc) + live-verified server dedup.

@iceteaSA
iceteaSA force-pushed the feat/openai-reset-command branch from cb1e05a to 9a08f45 Compare August 11, 2026 09:26
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Rebased onto main at v0.5.1 — 9a08f45, single commit, 1048 pass / 0 fail, typecheck and biome clean.

This branch was cut in July against v0.4.x, so it collided with everything that landed since: sticky-balanced routing, the background quota poll, admission exhaustion-skipping, and the RPC state sweep. Ten conflicted files. Resolved by taking upstream wholesale and re-applying only what /openai-reset adds — nothing already reviewed and merged should come back through this door.

Two of the conflicts needed judgement rather than a union:

refresh-all-quota.ts — this PR's original main-refresh block used codexRefreshFn + client.auth.set directly, and upstream has since replaced that path with lease-orchestrated refreshMainWithLease. The old block is unreachable now, so it was dropped rather than merged. That also made refreshMainWithLease a mandatory dependency the reset call site had to supply (index.ts, refreshResetTargetQuota) — without it, typecheck fails.

rpc-client.test.ts — a genuine add/add: both sides created the file independently. Unioned both suites into one file (your sessionId test plus this PR's two timeout tests) with a single teardown handling both fixtures' cleanup conventions. No test dropped.

Verified present after resolution rather than assumed: the admission identity gate in its fileAccountId === currentAccountId form (not the undefined || form this branch predates), isReplayableRequest gating quotaBlocksMain, killswitchPasses threading, the RPC sweep with its openai-auth-<hash> dirs and beforeWrite seam, /openai-cachekeep sustain, and /openai-routing reset.

One thing to flag rather than quietly keep: rpc-server.ts carries a 90s default where v0.5.1's base has 2s. That is this PR's own change from your round-1 review — it survived the rebase untouched because upstream never edited that line. The explicit-timeout test still passes since a per-call timeoutMs overrides the default. If you have since settled on 2s as a hard cap and want the 90s confined to the client's per-call override, say so and I will narrow it.

@ualtinok

Copy link
Copy Markdown
Contributor

The live dedup run is exactly what was missing — replay returning already_redeemed with windows_reset: 0, available_count unchanged at 1 across steps 2→4, and redeemed_at matching the first consume. That last detail is the one that proves it: the server returned the original redemption record rather than minting a second one. Blocker closed, and thank you for spending a real credit to get it.

I spot-checked the rebase rather than taking the summary: the admission gate is in its fileAccountId === currentAccountId form at index.ts:2087, and isReplayableRequest still gates quotaBlocksMain at index.ts:3164. Dropping the dead codexRefreshFn block in refresh-all-quota.ts was the right call over merging it.

On your rpc-server.ts question — you asked whether to confine the 90s to the client's per-call override. Neither option is right, and I had the semantics wrong when I first read it, so I measured on Node 24.16:

  1. With requestTimeout/headersTimeout at 500ms, a socket timeout of 90s, and a 3s handler: 200 OK at 3016ms. Those two bound request receipt only, never handler execution.
  2. With receipt bounds at 120s and req.setTimeout at 1s against the same 3s handler: socket destroyed at 1010ms, handler's write lands on a dead socket.

So your 90s is load-bearing and the client override cannot substitute for it — a 2s default would destroy the socket 58 seconds into a 60s consume, server-side, no matter what the client passes. Your reasoning was right.

But the three do not need the same value, and giving them one couples an unrelated guard to the reset path. requestTimeout/headersTimeout only limit how long a client may take to deliver ≤1 MiB over loopback; 90s there is pure slack for every endpoint, while the handler bound is the only one that needs to outlast a consume. Please split them: keep server.requestTimeout and server.headersTimeout at the 2s receipt bound, and let the req.setTimeout inactivity timer carry the 90s. Probe 1 is exactly that shape and passes.

Worth a comment on the split too, since a future reader will otherwise "tidy" the three back into one value and silently reintroduce the 2s abort.

That is the only change I want. Everything else is ready.

An account with reset credits had no way to spend them: the quota sat
exhausted until its window rolled over, while credits that exist to
short-circuit exactly that wait went unused.

/openai-reset lists each account with its eligibility, and consumes a
credit against the one chosen. Redemption carries a per-attempt UUID as
the idempotency key, so a retry after a timeout resolves to the original
redemption rather than spending a second credit - verified live against
the API, where a replayed request returned already_redeemed with the
first attempt's timestamp and an unchanged credit count. Attempt state
is claimed under the existing account file lock, so two processes cannot
redeem concurrently.

Credits are spent oldest-first. They expire, so preferring the credit
nearest expiry wastes the least.

The RPC server's two timeouts are deliberately different values.
requestTimeout and headersTimeout bound how long a client may take to
DELIVER a request and never bound handler execution - a 500ms receipt
bound still returns 200 for a three-second handler. The socket
inactivity timer is the one that can destroy a socket out from under a
working handler, so it must outlast the slowest legitimate apply: a
redemption is bounded by a 60s consume call, and a 2s timer there would
kill the connection 58 seconds in regardless of what deadline the client
passed. Collapsing the three into one value reintroduces one of two
bugs, so the split carries a comment saying so.
@iceteaSA
iceteaSA force-pushed the feat/openai-reset-command branch from 9a08f45 to cabc6eb Compare August 12, 2026 11:02
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Split done in cabc6eb, also rebased onto v0.5.2 (no overlap with the WS pool fix). 1052 pass / 0 fail.

server.requestTimeout and server.headersTimeout now take a separate receiptTimeoutMs defaulting to 2s; the socket inactivity timer keeps the 90s. Comment on the split explains why they cannot be one value, in both directions — a receipt bound that aborts a slow reset, or a socket timer that leaves every endpoint holding a dead connection.

Your probe led somewhere I did not expect, and it invalidated one of the existing tests.

enforces request timeout passed timeoutMs: 100, sent a half body, and asserted the abort. I read that as testing requestTimeout — so after the split it should have needed receiptTimeoutMs. It went from ~100ms to a 10s failure, which did not match.

Measuring it directly: requestTimeout: 100 does not abort a half-sent body at all. Not at 6s, and not with connectionsCheckingInterval dropped to 50ms:

receipt=100, checking=default(30s) → NO ABORT after 6000ms
receipt=100, checking=50ms         → NO ABORT after 6000ms
req.setTimeout(100)                → aborted at 105ms

Node samples requestTimeout on the connectionsCheckingInterval tick — 30s by default — so it is a coarse ceiling, not the mechanism that reclaims a stalled socket. That test was never testing what its name said: it passed because timeoutMs also set req.setTimeout(100), and the inactivity timer did the work. The old single value hid the distinction by making both true at once.

Renamed it to destroys a socket that stalls part-way through sending a request, kept it on timeoutMs, and recorded why requestTimeout cannot serve that role. It runs at ~4s under Bun rather than the ~105ms plain Node gives — coarser socket-timer granularity — so I mutation-checked it rather than trusting the green: removing timeoutMs entirely makes it fail at 10s, so the parameter is load-bearing and the pass is real.

So the split changed nothing about which mechanism guards a stalled connection. It only stopped one value from standing in for two, and made the test say which one it depends on.

@ualtinok
ualtinok merged commit 0fc111f into cortexkit:main Aug 13, 2026
5 checks passed
@iceteaSA
iceteaSA deleted the feat/openai-reset-command branch August 13, 2026 20:47
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