Copilot review round 4. - `getTokens(url, issuer)` falls back to the legacy unkeyed slot when that issuer holds none — right for a connect, wrong for `collectGrants`, which labelled the fallback with the issuer it happened to be iterating and paired it with that issuer's credentials. During a partially migrated flow the clear could have sent an old, unbound token to a newly discovered authorization server. `OAuthStorage.getIssuerTokens` is the exact, no-fallback read; only the ctx-less read still falls back, which is the one place it belongs. - The TUI's warning tone was sticky. It lived in its own state while ~30 call sites set ordinary OAuth notes without resetting it, so one revocation failure left every later "Authorization updated" yellow, across server switches. It is now DERIVED — `oauthMessageToneFor(message, warningText)` — so it cannot outlive the message that earned it, with nothing to remember to clear. Both setters stay plain `useState` setters: a `useCallback` wrapper would be a value `react-hooks/exhaustive-deps` demands in seven dependency arrays for an identity that never changes. - Hook-level tests for the web wiring (previously missed): the per-server opt-out and the backend-proxied fetch are now asserted to reach `clearServerOAuthState`, so handing it the page-origin fetch — which a real authorization server's missing CORS headers would reject — fails a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall <cliff@futurescale.com>
29 lines
1.3 KiB
TypeScript
29 lines
1.3 KiB
TypeScript
/**
|
|
* How the Auth tab should colour the OAuth note it is currently showing.
|
|
*
|
|
* The tone is **derived** from the message rather than stored beside it
|
|
* (#2144). `App.tsx` sets an ordinary OAuth note in around thirty places, none
|
|
* of which would think to reset a tone, so a second piece of state went stale
|
|
* the moment one message raised it: a single revocation failure left every
|
|
* later "Authorization updated" rendering as a warning, including after
|
|
* switching servers.
|
|
*
|
|
* Comparing the shown message against the text that was raised as a warning
|
|
* makes the tone a function of what is on screen, so it cannot outlive it —
|
|
* the next `setOauthMessage` of anything else is informational by
|
|
* construction, with nothing to remember to clear.
|
|
*
|
|
* `warning` is for a *partial* success: the OAuth state really was cleared, so
|
|
* it is not an error status, but the grant may still be live at the
|
|
* authorization server and the informational tone would understate that.
|
|
*/
|
|
export type OAuthMessageTone = "info" | "warning";
|
|
|
|
export function oauthMessageToneFor(
|
|
message: string | null,
|
|
warningText: string | null,
|
|
): OAuthMessageTone {
|
|
// `null === null` must not read as a warning: with no message there is
|
|
// nothing to colour.
|
|
return message !== null && message === warningText ? "warning" : "info";
|
|
}
|