Repository navigation
Add Codex reset credit picker - #7154
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a detailed Codex reset credits management modal, allowing users to view and explicitly select which credit to redeem (ordered by expiration) rather than just automatically consuming the soonest-expiring one. It also refactors the responses input sanitizer to handle nested output parts correctly. Feedback on the changes highlights a potential runtime crash in codexResetCredits.ts if the payload is null, a violation of Repository Style Guide Rule 8 regarding local Zod schema definitions in the API route, and potential hydration mismatches in the new modal due to server-side rendering of localized dates and relative times.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const payloadRecord = toRecord(payload); | ||
| const reportedCount = Number(payloadRecord.available_count ?? payloadRecord.availableCount); |
There was a problem hiding this comment.
Potential runtime crash (TypeError) if payload is not an object or is null. toRecord(payload) can return null, and accessing payloadRecord.available_count directly without a null check will throw an error. Please add a defensive check to ensure payloadRecord is not null before accessing its properties.
const payloadRecord = toRecord(payload);
const reportedCount = payloadRecord
? Number(payloadRecord.available_count ?? payloadRecord.availableCount)
: Number.NaN;| const ConnectionIdSchema = z.string().trim().min(1).max(256); | ||
|
|
||
| const CodexResetCreditBodySchema = z.object({ | ||
| connectionId: z.string().optional(), | ||
| idempotencyKey: z.string().optional(), | ||
| connectionId: ConnectionIdSchema, | ||
| idempotencyKey: z.string().trim().min(1).max(256), | ||
| creditId: z.string().trim().min(1).max(512).optional(), | ||
| }); |
There was a problem hiding this comment.
Defining Zod schemas locally in the route file violates Rule 8 of the Repository Style Guide, which states: 'Always validate inputs with Zod schemas from src/shared/validation/schemas.ts'. Please move ConnectionIdSchema and CodexResetCreditBodySchema to src/shared/validation/schemas.ts and import them here.
| const ConnectionIdSchema = z.string().trim().min(1).max(256); | |
| const CodexResetCreditBodySchema = z.object({ | |
| connectionId: z.string().optional(), | |
| idempotencyKey: z.string().optional(), | |
| connectionId: ConnectionIdSchema, | |
| idempotencyKey: z.string().trim().min(1).max(256), | |
| creditId: z.string().trim().min(1).max(512).optional(), | |
| }); | |
| import { ConnectionIdSchema, CodexResetCreditBodySchema } from "@/shared/validation/schemas"; |
References
- Always validate inputs with Zod schemas from src/shared/validation/schemas.ts. (link)
| @@ -0,0 +1,277 @@ | |||
| "use client"; | |||
|
|
|||
| import { useMemo, useState } from "react"; | |||
| function CreditSummary({ | ||
| credit, | ||
| recommended = false, | ||
| tr, | ||
| }: { | ||
| credit: CodexResetCreditView; | ||
| recommended?: boolean; | ||
| tr: (key: string, fallback: string, values?: UsageTranslationValues) => string; | ||
| }) { | ||
| const { absolute: expiry, relative: relativeExpiry } = getCodexResetCreditExpiryLabel( | ||
| credit.expiresAt | ||
| ); | ||
|
|
||
| return ( | ||
| <div className="min-w-0 flex-1"> | ||
| <div className="flex flex-wrap items-center gap-2"> | ||
| <span className="font-medium text-text-main"> | ||
| {credit.title || tr("resetCreditDefaultTitle", "Full reset")} | ||
| </span> | ||
| {recommended && ( | ||
| <span className="rounded-full bg-primary/10 px-2 py-0.5 text-[10px] font-semibold text-primary"> | ||
| {tr("resetCreditExpiresFirst", "Expires first")} | ||
| </span> | ||
| )} | ||
| </div> | ||
| {credit.description && ( | ||
| <p className="mt-1 text-xs leading-relaxed text-text-muted">{credit.description}</p> | ||
| )} | ||
| <div className="mt-2 flex items-center gap-1.5 text-xs text-text-muted"> | ||
| <span className="material-symbols-outlined text-[15px]">schedule</span> | ||
| {expiry ? ( | ||
| <span> | ||
| {tr("resetCreditExpiresAt", `Expires ${expiry}`, { date: expiry })} | ||
| {relativeExpiry ? ` (${relativeExpiry})` : ""} | ||
| </span> | ||
| ) : ( | ||
| <span>{tr("resetCreditNoExpiry", "No expiration date")}</span> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Rendering localized dates (toLocaleString) and relative times based on Date.now() during server-side pre-rendering (SSR) will cause hydration mismatches because the server and client times/timezones differ. To prevent this, use a mounted state to defer rendering the schedule/expiry section until the component has mounted on the client.
function CreditSummary({
credit,
recommended = false,
tr,
}: {
credit: CodexResetCreditView;
recommended?: boolean;
tr: (key: string, fallback: string, values?: UsageTranslationValues) => string;
}) {
const [mounted, setMounted] = useState(false);
useEffect(() => {
setMounted(true);
}, []);
const { absolute: expiry, relative: relativeExpiry } = useMemo(() => {
if (!mounted) return { absolute: null, relative: null };
return getCodexResetCreditExpiryLabel(credit.expiresAt);
}, [credit.expiresAt, mounted]);
return (
<div className="min-w-0 flex-1">
<div className="flex flex-wrap items-center gap-2">
<span className="font-medium text-text-main">
{credit.title || tr("resetCreditDefaultTitle", "Full reset")}
</span>
{recommended && (
<span className="rounded-full bg-primary/10 px-2 py-0.5 text-[10px] font-semibold text-primary">
{tr("resetCreditExpiresFirst", "Expires first")}
</span>
)}
</div>
{credit.description && (
<p className="mt-1 text-xs leading-relaxed text-text-muted">{credit.description}</p>
)}
{mounted && (
<div className="mt-2 flex items-center gap-1.5 text-xs text-text-muted">
<span className="material-symbols-outlined text-[15px]">schedule</span>
{expiry ? (
<span>
{tr("resetCreditExpiresAt", `Expires ${expiry}`, { date: expiry })}
{relativeExpiry ? ` (${relativeExpiry})` : ""}
</span>
) : (
<span>{tr("resetCreditNoExpiry", "No expiration date")}</span>
)}
</div>
)}
</div>
);
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d0e1900e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| throw new CodexResetCreditError( | ||
| 409, | ||
| "selected_credit_unavailable", | ||
| "The selected Codex reset credit is no longer available." |
There was a problem hiding this comment.
Preserve idempotent retries after a consumed selection
When a selected redemption succeeds upstream but the request fails before the client receives success (for example during the post-consume usage refresh or a dropped response), the hook retries with the same idempotency key and creditId. At that point the selected credit may no longer appear in the inventory, so this conflict is returned before calling the consume endpoint that could resolve the same redeem_request_id as alreadyRedeemed; the UI remains failed even though the reset credit was consumed. Allow the same selected idempotency key to reach the consume call (or record the attempted credit before the inventory check) so retries are actually idempotent.
Useful? React with 👍 / 👎.
|
This is good work on both fronts — the reset-credit picker is well-tested (listing, expiry ordering, explicit selection, conflict-on-redeem races are all covered), and the |
4d0e190 to
e9d940a
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Done — I split the two concerns and removed the unrelated CI/baseline changes:
I also removed the unrelated responsive quota-layout and integration-test changes from #7154. The picker branch passes its 32 focused tests, core/dashboard type gates, ESLint, and the complexity ratchet without a baseline change. Thanks for the clear split suggestion. |
|
Thanks @JxnLexn! Merged into release/v3.8.49 after combined merge-train validation (static gates + tests + vitest green on the merged tree). Where quality gates flagged growth we decomposed/extracted inside your branch keeping you as author — including renumbering the reasoning-routing migration slot and the ProviderDetailPageClient rebaseline annotation. |
Summary
Validation
node --import tsx/esm --test tests/unit/codex-reset-credits.test.ts tests/unit/provider-limits-ui.test.ts(32 passing)npm run typecheck:corenpm run check:dashboard-typechecknpm run check:complexity-ratchetsSplit
The independent Responses-content normalization fix now lives in #7269. CI/baseline and unrelated quota-layout changes were removed from this PR.