[low] fix(usage): only clear the in-flight lock this render wrote (#624)

* fix(usage): only clear the in-flight lock this render wrote

A successful fetch removed usage.lock unconditionally. When two renders
fetch concurrently and one receives a 429, the other's later success
deleted the Retry-After lock, so the next render after the 180s cache
expiry fetched again inside the server's backoff window.

Clear the lock only while it still holds this render's own in-flight
record.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(usage): keep a rate-limit lock written during an in-flight fetch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: elhoim

src/utils/__tests__/usage-fetch.test.ts

@@ -42,6 +42,7 @@ interface ProbeOptions {
     claudeConfigDir?: string;
     home: string;
     httpsProxy?: string;
+    lockWrittenDuringRequest?: string;
     lowercaseHttpsProxy?: string;
     mode?: 'error' | 'status' | 'success' | 'unexpected';
     nowMs: number;
@@ -103,6 +104,12 @@ https.request = (...args) => {
         },
         destroy() {},
         end() {
+            // Simulates a concurrent render replacing the lock while this
+            // request is in flight.
+            if (process.env.TEST_LOCK_DURING_REQUEST) {
+                fs.writeFileSync(lockFile, process.env.TEST_LOCK_DURING_REQUEST);
+            }
+
             if (mode === 'error') {
                 const handlers = requestHandlers.get('error') || [];
                 for (const handler of handlers) {
@@ -226,6 +233,10 @@ process.stdout.write(JSON.stringify({
             env.HTTPS_PROXY = options.httpsProxy;
         }
 
+        if (options.lockWrittenDuringRequest !== undefined) {
+            env.TEST_LOCK_DURING_REQUEST = options.lockWrittenDuringRequest;
+        }
+
         if (options.lowercaseHttpsProxy !== undefined) {
             env.https_proxy = options.lowercaseHttpsProxy;
         }
@@ -891,6 +902,35 @@ describe('fetchUsageData error handling', () => {
         }
     });
 
+    it('keeps a rate-limit lock that a concurrent render wrote during the fetch', () => {
+        const harness = createProbeHarness();
+
+        try {
+            const home = harness.createTokenHome('success-keeps-foreign-lock');
+            // Another render got a 429 while this request was in flight. Deleting
+            // its lock would let the next render fetch inside the Retry-After window.
+            const rateLimitedLock = JSON.stringify({
+                blockedUntil: Math.floor(nowMs / 1000) + 300,
+                error: 'rate-limited'
+            });
+            const result = harness.runProbe({
+                claudeConfigDir: home.claudeConfig,
+                home: home.home,
+                lockWrittenDuringRequest: rateLimitedLock,
+                mode: 'success',
+                nowMs,
+                pathDir: home.bin,
+                responseBody: successResponseBody
+            });
+
+            expect(result.requestCount).toBe(1);
+            expect(result.cacheExists).toBe(true);
+            expect(result.lockContents).toBe(rateLimitedLock);
+        } finally {
+            harness.cleanup();
+        }
+    });
+
     it('ignores a lock whose deadline is implausibly far in the future', () => {
         const harness = createProbeHarness();
 

src/utils/usage-fetch.ts

@@ -670,18 +670,26 @@ function readStaleUsageCache(cacheIdentity: UsageCacheIdentity | null): UsageDat
     }
 }
 
-function writeUsageLock(blockedUntil: number, error: UsageLockError): void {
+function writeUsageLock(blockedUntil: number, error: UsageLockError): string {
+    const contents = JSON.stringify({ blockedUntil, error });
     try {
         ensureCacheDirExists();
-        fs.writeFileSync(LOCK_FILE, JSON.stringify({ blockedUntil, error }));
+        fs.writeFileSync(LOCK_FILE, contents);
     } catch {
         // Ignore lock file errors
     }
+    return contents;
 }
 
-function clearUsageLock(): void {
+// Remove the lock only while it still holds the in-flight record this process
+// wrote. A concurrent render may have replaced it with a 429 Retry-After lock
+// in the meantime; deleting that would let the next render fetch inside the
+// server's backoff window.
+function clearOwnUsageLock(ownContents: string): void {
     try {
-        fs.rmSync(LOCK_FILE, { force: true });
+        if (fs.readFileSync(LOCK_FILE, 'utf8') === ownContents) {
+            fs.rmSync(LOCK_FILE, { force: true });
+        }
     } catch {
         // Ignore lock file errors
     }
@@ -896,7 +904,7 @@ export async function fetchUsageData(options: FetchUsageDataOptions = {}): Promi
         );
     }
 
-    writeUsageLock(now + LOCK_MAX_AGE, 'timeout');
+    const inFlightLock = writeUsageLock(now + LOCK_MAX_AGE, 'timeout');
 
     // Fetch from API using Node's https module
     try {
@@ -935,7 +943,7 @@ export async function fetchUsageData(options: FetchUsageDataOptions = {}): Promi
         // the caller's requested fields. Incomplete 200 responses are cached but
         // still need the short throttle so later renders do not refetch every time.
         if (hasRequiredUsageFields(usageData, requiredFields)) {
-            clearUsageLock();
+            clearOwnUsageLock(inFlightLock);
         }
 
         return cacheUsageData(usageData, now);