fix(platform): address review R1 - fix docstring, stale closures, shared formatCents

- Fix misleading "Fails open" docstring in reset_daily_usage (it's
  fail-closed for billed operations).
- Use refs in useResetRateLimit to avoid stale closure in mutation
  callbacks.
- Replace eslint-disable with useRef pattern in RateLimitResetDialog.
- Export and share formatCents between dialog and panel components.
- Add clarifying comment for omitted rate_limit_reset_cost in inner
  get_usage_status call.
This commit is contained in:
Zamil Majdy
2026-03-27 11:03:11 +07:00
parent 137edb3e6e
commit 78cd14d501
5 changed files with 27 additions and 9 deletions

View File

@@ -550,6 +550,8 @@ async def reset_copilot_usage(
try:
# Verify the user is actually at or over their daily limit.
# (rate_limit_reset_cost intentionally omitted — this object is only
# used for limit checks, not returned to the client.)
usage_status = await get_usage_status(
user_id=user_id,
daily_token_limit=daily_limit,

View File

@@ -161,8 +161,9 @@ async def reset_daily_usage(user_id: str, daily_token_limit: int = 0) -> bool:
daily_token_limit: The configured daily token limit. When positive,
the weekly counter is reduced by this amount.
Fails open: returns False if Redis is unavailable (consistent with
the fail-open design of this module).
Returns False if Redis is unavailable so the caller can handle
compensation (fail-closed for billed operations, unlike the read-only
rate-limit checks which fail-open).
"""
now = datetime.now(UTC)
try:

View File

@@ -4,7 +4,7 @@ import { Button } from "@/components/atoms/Button/Button";
import { Text } from "@/components/atoms/Text/Text";
import { Dialog } from "@/components/molecules/Dialog/Dialog";
import { useRouter } from "next/navigation";
import { useEffect } from "react";
import { useEffect, useRef } from "react";
import { useResetRateLimit } from "../../hooks/useResetRateLimit";
interface Props {
@@ -18,7 +18,7 @@ interface Props {
onCreditChange?: () => void;
}
function formatCents(cents: number): string {
export function formatCents(cents: number): string {
return `$${(cents / 100).toFixed(2)}`;
}
@@ -38,11 +38,16 @@ export function RateLimitResetDialog({
});
const router = useRouter();
// Stable ref for the callback so the effect only re-fires when
// `isOpen` changes, not when the function reference changes.
const onCreditChangeRef = useRef(onCreditChange);
onCreditChangeRef.current = onCreditChange;
// Refresh the credit balance each time the dialog opens so we never
// block a valid reset due to a stale client-side balance.
useEffect(() => {
if (isOpen) onCreditChange?.();
}, [isOpen]); // eslint-disable-line react-hooks/exhaustive-deps
if (isOpen) onCreditChangeRef.current?.();
}, [isOpen]);
// Whether to hide the reset button entirely
const cannotReset = isWeeklyExhausted || hasInsufficientCredits;

View File

@@ -1,6 +1,7 @@
import type { CoPilotUsageStatus } from "@/app/api/__generated__/models/coPilotUsageStatus";
import { Button } from "@/components/atoms/Button/Button";
import Link from "next/link";
import { formatCents } from "../RateLimitResetDialog/RateLimitResetDialog";
import { useResetRateLimit } from "../../hooks/useResetRateLimit";
export function formatResetTime(
@@ -91,7 +92,7 @@ function ResetButton({
>
{isPending
? "Resetting..."
: `Reset daily limit for $${(cost / 100).toFixed(2)}`}
: `Reset daily limit for ${formatCents(cost)}`}
</Button>
);
}

View File

@@ -5,11 +5,20 @@ import {
import { toast } from "@/components/molecules/Toast/use-toast";
import { ApiError } from "@/lib/autogpt-server-api";
import { useQueryClient } from "@tanstack/react-query";
import { useRef } from "react";
export function useResetRateLimit(options?: {
onSuccess?: () => void;
onCreditChange?: () => void;
}) {
// Use refs so mutation callbacks always see the latest options,
// avoiding stale-closure issues when the caller re-renders with
// different callback references.
const onSuccessRef = useRef(options?.onSuccess);
onSuccessRef.current = options?.onSuccess;
const onCreditChangeRef = useRef(options?.onCreditChange);
onCreditChangeRef.current = options?.onCreditChange;
const queryClient = useQueryClient();
const { mutate: resetUsage, isPending } = usePostV2ResetCopilotUsage({
mutation: {
@@ -20,13 +29,13 @@ export function useResetRateLimit(options?: {
await queryClient.invalidateQueries({
queryKey: getGetV2GetCopilotUsageQueryKey(),
});
options?.onCreditChange?.();
onCreditChangeRef.current?.();
toast({
title: "Rate limit reset",
description:
"Your daily usage limit has been reset. You can continue working.",
});
options?.onSuccess?.();
onSuccessRef.current?.();
},
onError: (error: unknown) => {
const message =