fix(oauth): preserve Anthropic intent across uncertain refresh outcomes

A timeout, lost response, unreadable body, or credential-store failure can
happen after Anthropic consumed and rotated a refresh token. Clearing the
durable intent lets the next attempt replay the old token, risking reuse
handling and forced reauthentication. The store's uncertain flag does not
cover these request outcomes.

Track the pre-dispatch boundary explicitly. Clear a non-terminal intent only
before dispatch or after the adapter reports an explicit non-success HTTP
response; retain it after all other post-dispatch outcomes and after provider
success followed by persistence failure.

Intent cleanup is secondary to the refresh result. A failed unlink now leaves
the replay guard in place without replacing the original provider error,
pre-dispatch abort, terminal login result, or successfully persisted
credential.

Cover definite 503 retry, uncertain post-dispatch failure, pre-dispatch abort,
cleanup failure, post-provider persistence failure, and post-persist cleanup
failure.

Author: luvs01

src/oauth/index.ts

@@ -562,6 +562,45 @@ function terminal(error:unknown):boolean{
   if (error instanceof RefreshIntentIOError) return false;
   return isTerminalRefreshError(error);
 }
+
+/**
+ * True when the token endpoint definitively answered and rejected the request.
+ *
+ * The Anthropic adapter attaches an HTTP status only to an explicit non-success response,
+ * which is the retryable rejection this PR handles. Everything else (timeout, dropped
+ * connection, a body that could not be read or parsed, or a local persistence fault) leaves
+ * the outcome unknown: the server may already have rotated the token, and a blind replay
+ * could trip refresh-token-reuse revocation. Those cases must keep the refresh intent.
+ *
+ * Deliberately narrower than `terminal()`, which asks whether the CREDENTIAL is dead.
+ * This asks the different question of whether the ATTEMPT is known to have failed.
+ */
+function definitivelyAnswered(error: unknown): boolean {
+  if (error instanceof AnthropicTokenError) return error.httpStatus !== undefined;
+  return false;
+}
+
+/**
+ * Intent cleanup is secondary to the refresh outcome it protects.
+ *
+ * A filesystem failure here must not replace a provider error, a pre-dispatch abort, or a
+ * successfully persisted credential. Leaving the intent in place is the conservative fallback:
+ * it blocks replay until the local persistence problem is repaired.
+ */
+function clearAnthropicRefreshIntentBestEffort(
+  provider: string,
+  accountId: string,
+  generation: string,
+): boolean {
+  try {
+    return clearOAuthRefreshIntent(provider, accountId, generation);
+  } catch {
+    console.warn(
+      "[opencodex] Anthropic refresh intent cleanup failed; preserving the durable replay guard.",
+    );
+    return false;
+  }
+}
 function authoritative(stored:OAuthCredentials,active:boolean,now:()=>number):OAuthCredentials{if(stored.source!=="local-cli")return stored;const disk=detectGrokCliToken();if(!disk)return stored;const allowed=isSameGrokIdentity(stored,disk)||(active&&!hasComparableGrokIdentity(stored,disk));return allowed&&shouldAdoptGrokGeneration(stored,disk,now(),REFRESH_SKEW_MS)?disk:stored;}
 function merged(fresh: OAuthCredentials, previous: OAuthCredentials): OAuthCredentials {
   return {
@@ -677,39 +716,50 @@ export async function refreshAnthropicAccountWithLock(
       return stored.access;
     }
 
+    let refreshMayHaveReachedProvider = false;
     try {
       writeOAuthRefreshIntent(provider, accountId, generation, now(), deps.flight?.flightId);
       if (deps.signal?.aborted) throw deps.signal.reason;
+      // From this point on, even a synchronous client error is conservatively post-dispatch:
+      // the provider may have received and rotated the refresh token before the caller learned
+      // the outcome.
+      refreshMayHaveReachedProvider = true;
       if (deps.flight) deps.flight.dispatched = true;
       const fresh = merged(await def.refresh(stored.refresh, deps.signal), stored);
       const outcome = await mergeAccountCredential(provider, accountId, fresh, {
         expectedGeneration: generation,
         afterPrePersistRead: deps.afterPrePersistRead,
       });
       if (outcome.superseded) {
-        clearOAuthRefreshIntent(provider, accountId, generation);
+        clearAnthropicRefreshIntentBestEffort(provider, accountId, generation);
         if (outcome.stored.expires > now() + REFRESH_SKEW_MS) return outcome.stored.access;
         throw new OAuthLoginRequiredError(provider);
       }
-      clearOAuthRefreshIntent(provider, accountId, generation);
+      // The rotated credential is durable now. A cleanup failure must not turn that committed
+      // success into a refresh failure; the old-generation intent remains a conservative guard.
+      clearAnthropicRefreshIntentBestEffort(provider, accountId, generation);
       return fresh.access;
     } catch (error) {
       if (error instanceof OAuthMutationBusyError || error instanceof OAuthTokenRefreshStaleError) throw error;
       if (!terminal(error)) {
-        // A non-terminal failure means the credential was never rejected, so the caller is
-        // told to retry. Leaving the intent behind contradicted that: the next attempt hit
-        // the pending-intent branch above and raised OAuthLoginRequiredError, so one 503 or
-        // timeout locked the account out of refresh entirely until manual re-auth — even
-        // once upstream recovered. Clear it so the promised retry can actually happen.
+        // A non-terminal failure tells the caller to retry, but the intent outlived it, so
+        // the next attempt hit the pending-intent branch above and raised
+        // OAuthLoginRequiredError. One 503 locked the account out of refresh until manual
+        // re-auth even after upstream recovered.
         //
-        // The replay guard is preserved by `uncertain`: a refresh whose outcome is genuinely
-        // unknown surfaces as an uncertain intent from the store, which this path never
-        // clears, and a superseded owner still leaves through OAuthTokenRefreshStaleError.
-        clearOAuthRefreshIntent(provider, accountId, generation);
+        // Only clear the intent when the server DEFINITIVELY answered and rejected the
+        // request. The adapter attaches an HTTP status only to that explicit non-success
+        // response. A timeout, a dropped connection, or an unreadable/unparseable body
+        // carries no status: the server may already have
+        // rotated the token, and replaying it could trip refresh-token-reuse revocation.
+        // Those outcomes keep the intent so the guard still refuses a blind replay.
+        if (!refreshMayHaveReachedProvider || definitivelyAnswered(error)) {
+          clearAnthropicRefreshIntentBestEffort(provider, accountId, generation);
+        }
         throw error;
       }
       await markAccountNeedsReauthIfGeneration(provider, accountId, generation, writerGeneration);
-      clearOAuthRefreshIntent(provider, accountId, generation);
+      clearAnthropicRefreshIntentBestEffort(provider, accountId, generation);
       throw new OAuthLoginRequiredError(provider);
     }
   } finally {

tests/oauth-refresh.test.ts

@@ -518,13 +518,12 @@ describe("oauth refresh hardening", () => {
   });
 
   /**
-   * A non-terminal refresh failure is reported as retryable, but the refresh intent outlived
-   * it. The next attempt then hit the pending-intent branch and raised OAuthLoginRequiredError,
-   * so a single 503 or timeout locked the account out of refresh until manual re-authentication
-   * even after upstream recovered. The replay guard is unaffected: an intent whose outcome is
-   * genuinely unknown is reported `uncertain` by the store and is still never cleared here.
+   * A definitive non-terminal HTTP failure is retryable: the endpoint answered with a failure,
+   * so the durable intent must not turn that promised retry into OAuthLoginRequiredError.
+   * A timeout or unreadable response is different — the provider may already have rotated the
+   * token, so that post-dispatch intent remains as the replay guard.
    */
-  test("a transient Anthropic failure leaves the account refreshable", async () => {
+  test("a definitive transient Anthropic HTTP failure leaves the account refreshable", async () => {
     await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
     const id = getAccountSet("anthropic")!.activeAccountId;
     const transient = new AnthropicTokenError("server", 503, undefined);
@@ -542,6 +541,120 @@ describe("oauth refresh hardening", () => {
     expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined();
   });
 
+  test("an Anthropic refresh with an uncertain post-dispatch outcome preserves its intent", async () => {
+    await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
+    const id = getAccountSet("anthropic")!.activeAccountId;
+    const credential = getAccountCredential("anthropic", id)!;
+    const generation = credentialGeneration(credential);
+    const timeout = new AnthropicTokenError("timeout", undefined, undefined);
+
+    await expect(refreshAnthropicAccountWithLock("anthropic", id, {
+      ...OAUTH_PROVIDERS.anthropic!,
+      refresh: async () => { throw timeout; },
+    }, credential)).rejects.toBe(timeout);
+
+    expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation });
+    expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined();
+  });
+
+  test("a pre-dispatch Anthropic abort clears its unconsumed refresh intent", async () => {
+    await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
+    const id = getAccountSet("anthropic")!.activeAccountId;
+    const credential = getAccountCredential("anthropic", id)!;
+    const aborted = new Error("aborted before dispatch");
+    const controller = new AbortController();
+    controller.abort(aborted);
+    let calls = 0;
+
+    await expect(refreshAnthropicAccountWithLock("anthropic", id, {
+      ...OAUTH_PROVIDERS.anthropic!,
+      refresh: async () => { calls += 1; throw new Error("must not run"); },
+    }, credential, { signal: controller.signal })).rejects.toBe(aborted);
+
+    expect(calls).toBe(0);
+    expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined();
+  });
+
+  test("intent cleanup failure preserves the original Anthropic HTTP error", async () => {
+    await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
+    const id = getAccountSet("anthropic")!.activeAccountId;
+    const credential = getAccountCredential("anthropic", id)!;
+    const generation = credentialGeneration(credential);
+    const transient = new AnthropicTokenError("server", 503, undefined);
+    const clearFailure = new Error("intent unlink failed");
+    const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntent").mockImplementation(() => {
+      throw clearFailure;
+    });
+    const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
+    try {
+      await expect(refreshAnthropicAccountWithLock("anthropic", id, {
+        ...OAUTH_PROVIDERS.anthropic!,
+        refresh: async () => { throw transient; },
+      }, credential)).rejects.toBe(transient);
+
+      expect(clearSpy).toHaveBeenCalled();
+      expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation });
+      expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined();
+    } finally {
+      warnSpy.mockRestore();
+      clearSpy.mockRestore();
+    }
+  });
+
+  test("Anthropic persistence failure after provider success preserves the replay guard", async () => {
+    await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
+    const id = getAccountSet("anthropic")!.activeAccountId;
+    const credential = getAccountCredential("anthropic", id)!;
+    const generation = credentialGeneration(credential);
+    const persistenceFailure = new Error("credential persistence failed");
+    const mergeSpy = spyOn(storeModule, "mergeAccountCredential").mockImplementation(async () => {
+      throw persistenceFailure;
+    });
+    try {
+      await expect(refreshAnthropicAccountWithLock("anthropic", id, {
+        ...OAUTH_PROVIDERS.anthropic!,
+        refresh: async () => ({
+          access: "fresh",
+          refresh: "rt-fresh",
+          expires: Date.now() + 3_600_000,
+        }),
+      }, credential)).rejects.toBe(persistenceFailure);
+
+      expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation });
+      expect(getAccountCredential("anthropic", id)).toEqual(credential);
+      expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined();
+    } finally {
+      mergeSpy.mockRestore();
+    }
+  });
+
+  test("post-persist intent cleanup failure does not turn Anthropic refresh success into failure", async () => {
+    await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
+    const id = getAccountSet("anthropic")!.activeAccountId;
+    const credential = getAccountCredential("anthropic", id)!;
+    const oldGeneration = credentialGeneration(credential);
+    const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntent").mockImplementation(() => {
+      throw new Error("intent unlink failed");
+    });
+    const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
+    try {
+      await expect(refreshAnthropicAccountWithLock("anthropic", id, {
+        ...OAUTH_PROVIDERS.anthropic!,
+        refresh: async () => ({
+          access: "fresh",
+          refresh: "rt-fresh",
+          expires: Date.now() + 3_600_000,
+        }),
+      }, credential)).resolves.toBe("fresh");
+
+      expect(getAccountCredential("anthropic", id)?.access).toBe("fresh");
+      expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation: oldGeneration });
+    } finally {
+      warnSpy.mockRestore();
+      clearSpy.mockRestore();
+    }
+  });
+
   test("Anthropic post-dispatch stale flight replacement stays retryable without replay or reauth", async () => {
     await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
     const id = getAccountSet("anthropic")!.activeAccountId;