Unhandled rejection: the OAuth connection-details refresh voids getOAuthState() with no catch

[cliffhall]

Found in the v2.5.0 milestone-merge review (#2215), in code that shipped on v2/main during the milestone. Filed here rather than fixed in the merge PR, whose tree is byte-identical to origin/v2/main.

The bug

clients/web/src/hooks/useOAuthRecovery.ts, the connection-details refresh effect:

const refresh = (): void => {
  void inspectorClient.getOAuthState().then((state) => {
    if (cancelled) return;
    setConnectionInfoOAuthWhenConnected(
      state ? oauthDetailsFromConnectionState(state) : undefined,
    );
  });
};

getOAuthState() delegates to this.oauthManager.getOAuthState(), which reads the OAuth store — in the browser that is the remote store, so the read is a network request whose response is then parsed. It can reject: a backend that is down or restarting, a 401 on the API token, malformed stored state.

There is no rejection handler on the chain. void silences @typescript-eslint/no-floating-promises without terminating anything, so a rejection becomes an unhandled rejection — on the initial refresh and on every oauthComplete refresh, the latter firing exactly when OAuth has just been exercised and a transient backend failure is most plausible.

Why this is a rule violation, not a judgment call

AGENTS.md is explicit:

Prefer holding and settling the promise. void is an escape hatch, not a fix — it is visible at review time (strictly better than nothing) but still discards the rejection. Reach for it only when the callee already owns its failures … Say which of those it is in a one-line comment; an unexplained void is a review finding.

Neither condition is met. getOAuthState() does not own its failures — it has no catch and surfaces no message — and there is no comment claiming it does. This is also the same class as #2165, whose five void-discarded OAuth recovery promises were fixed in #2190; this call sits in the same file and was missed.

The correct pattern is already used three places in this very file — void runWithCommandAuthRecovery(operation, source).catch(…) at line 794, and .catch((err: unknown) => …) on the IIFEs at 981 and 1351. The flagged call is the outlier.

Worth noting the failure mode is not only user-visible. An unhandled rejection fails the whole vitest run, and attributing it is expensive because it surfaces in a different test with a stack pointing at SDK internals — the #1947 experience, which is why the rule exists.

Fix

Terminate the chain and degrade the panel honestly rather than leaving stale details on screen:

void inspectorClient
  .getOAuthState()
  .then((state) => {
    if (cancelled) return;
    setConnectionInfoOAuthWhenConnected(
      state ? oauthDetailsFromConnectionState(state) : undefined,
    );
  })
  .catch(() => {
    if (cancelled) return;
    setConnectionInfoOAuthWhenConnected(undefined);
  });

Clearing on failure is the right degrade: the panel's job is to report the current OAuth state, and holding the last successful read makes a stale answer indistinguishable from a fresh one. Keep the cancelled guard on the catch too, so a rejection arriving after unmount does not write.

The void may stay once the chain is terminated, with the one-line comment AGENTS.md asks for — this is a synchronous effect body, one of the named cases where the caller genuinely cannot await.

A test can assert it: make getOAuthState reject, render, and check both that the details clear and that no unhandled rejection escapes.

Reported by Copilot on #2215.