Skip to content

feat(codex): show and use banked resets - #7813

Draft
Bil0000 wants to merge 4 commits into
pingdotgg:mainfrom
Bil0000:t3code/codex-banked-resets-visualization
Draft

feat(codex): show and use banked resets#7813
Bil0000 wants to merge 4 commits into
pingdotgg:mainfrom
Bil0000:t3code/codex-banked-resets-visualization

Conversation

@Bil0000

@Bil0000 Bil0000 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Why

Codex users can earn banked resets, but T3 Code did not show them or let users redeem one.

What changed

  • Read Codex limit windows and reset credits through the existing app-server probe.
  • Show 5-hour, weekly, and banked reset status at the top of Usage.
  • Redeem a reset through a typed, authorized RPC with confirmation and clear outcomes.
  • Keep retries safe with one idempotency key, a bounded timeout, and best-effort snapshot refresh.
  • Label each panel with its environment for remote and multi-account setups.

Test plan

  • Focused server, web, contracts, and client-runtime typechecks.
  • Focused lint for all changed files.
  • 6 focused test files, 93 tests passing.
  • Independent read-only code review: ready to merge, no findings.

Visual check

Design mockup was reviewed before implementation. Browser screenshots were not captured because computer use was not authorized for this run.

Built with GPT-5.6 Sol via Codex in T3 Code.

Note

Add banked rate-limit reset support for Codex providers

  • Adds contracts modeling provider rate limits, reset credits, and the server.consumeProviderRateLimitReset RPC in rpc.ts and server.ts
  • Implements consumeRateLimitResetCredit in the Codex adapter (CodexAdapter.ts) and ProviderRegistry (ProviderRegistry.ts), which consumes a banked reset then performs a best-effort refresh
  • Provider probe and status checks now surface rateLimits from the Codex app-server API in snapshots (CodexProvider.ts)
  • Adds a CodexLimitsPanel UI component and wires a confirmation modal in UsagePage.tsx to trigger the reset via the new client-runtime command
  • Risk: serverConsumeProviderRateLimitReset RPC requires AuthOrchestrationOperateScope in RpcAuthorization.ts; existing clients without this scope will get authorization errors

Macroscope summarized 533a8ca.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bc3da65f-bdec-44dc-8d5b-45b66a723be0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XL 500-999 changed lines (additions + deletions). labels Aug 21, 2026

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the new banked-reset service/RPC path against the Effect service conventions. Three findings, all in the error-modeling and change-discipline rules; the adapter/registry wiring and layer usage otherwise look consistent with the existing provider modules.

Posted via Macroscope — Effect Service Conventions

"ServerProviderRateLimitResetError",
{
instanceId: ProviderInstanceId,
reason: TrimmedNonEmptyString,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Every construction of this error wraps a real failure at the RPC boundary, but there is no cause field, so the underlying provider error cannot be preserved. Suggest adding an optional defect cause, matching ServerProviderUpdateError in this file.

Suggested change
reason: TrimmedNonEmptyString,
reason: TrimmedNonEmptyString,
cause: Schema.optional(Schema.Defect()),

Posted via Macroscope — Effect Service Conventions

const connectCodexAppServer = Effect.fn("connectCodexAppServer")(function* (
input: CodexAppServerConnectionInput,
) {
const resolvedHomePath = input.homePath ? expandHomePath(input.homePath) : undefined;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The invariant comment explaining why homePath is expanded here (~ is not shell-expanded for env vars passed to child_process.spawn) was dropped while this code moved into connectCodexAppServer. Suggest restoring it so the reason for expandHomePath stays documented.

Suggested change
const resolvedHomePath = input.homePath ? expandHomePath(input.homePath) : undefined;
// `~` is not shell-expanded when env vars are set via `child_process.spawn`,
// so `CODEX_HOME=~/.codex_work` would reach codex verbatim and trip
// "CODEX_HOME points to '~/.codex_work', but that path does not exist".
// Expand here for parity with `CodexTextGeneration`/`CodexSessionRuntime`.
const resolvedHomePath = input.homePath ? expandHomePath(input.homePath) : undefined;

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/ws.ts
Comment on lines +1490 to +1494
Effect.mapError(
(cause) =>
new ServerProviderRateLimitResetError({
instanceId: input.instanceId,
reason: cause.message,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The wrapper's reason is copied from cause.message and the underlying error is then discarded, so the caller-visible message is derived from the cause and the error chain/stack is lost. Consider a stable reason derived from this boundary (as providerMaintenanceRunner.ts does for ServerProviderUpdateError) and passing the original failure through as cause, e.g. reason: "Codex rejected the banked reset request.", cause. Requires the cause field noted on packages/contracts/src/server.ts.

Posted via Macroscope — Effect Service Conventions

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One finding on the new banked-reset UI. Everything else lines up with the shared primitives: Button/AlertDialog* are used instead of raw controls, the AlertDialogClose render={<Button variant="outline" />} pattern matches existing confirm dialogs, and the panel keeps only contextual layout/color at the call site without overriding primitive geometry.

Posted via Macroscope — UI Consistency

Comment on lines +534 to +537
open={resetTarget !== null}
onOpenChange={(open) => {
if (!open && !usingReset) setResetTarget(null);
}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The dialog's open state and its content are the same resetTarget value, so clearing it (here on Cancel, and on success at line 163) blanks the copy while the popup is still visible: AlertDialogPopup composes DIALOG_POPUP_CLASS, which animates out over 200ms via data-ending-style, so the description briefly renders as "This uses one reset for . It resets both limits…".

Suggest keeping the target until the close completes, as the other in-flight confirm dialogs do (ConnectionsSettings.tsx defers clearing its pending value in onOpenChangeComplete): drive open from a separate boolean, and clear resetTarget in onOpenChangeComplete instead of in onOpenChange/the success path.

Posted via Macroscope — UI Consistency

@macroscopeapp

macroscopeapp Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a complete new feature: displaying Codex rate limits and enabling users to consume banked resets. It spans contracts, server RPC with authorization, provider adapters, client state, and a new UI component — new user-facing capability warrants human review.

You can add or adjust custom eligibility rules. Learn more.

@Bil0000
Bil0000 marked this pull request as draft August 21, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL 500-999 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant