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>
28 lines
1.2 KiB
TypeScript
28 lines
1.2 KiB
TypeScript
import { describe, it, expect } from "vitest";
|
|
import { oauthMessageToneFor } from "../src/oauthMessageTone.js";
|
|
|
|
const WARNING =
|
|
"Cleared locally, but revoking the grant at the authorization server failed.";
|
|
|
|
describe("oauthMessageToneFor", () => {
|
|
it("warns while the message it was raised for is showing", () => {
|
|
expect(oauthMessageToneFor(WARNING, WARNING)).toBe("warning");
|
|
});
|
|
|
|
// The bug this replaced (#2144): a stored tone survived the message that
|
|
// earned it, so every later ordinary note rendered as a warning.
|
|
it("returns to informational as soon as any other message replaces it", () => {
|
|
expect(oauthMessageToneFor("Authorization updated.", WARNING)).toBe("info");
|
|
});
|
|
|
|
it("is informational when nothing has ever warned", () => {
|
|
expect(oauthMessageToneFor("Authorization updated.", null)).toBe("info");
|
|
});
|
|
|
|
// `null === null` must not read as a warning — with no message on screen
|
|
// there is nothing to colour, and the previous warning is long gone.
|
|
it("is informational when there is no message", () => {
|
|
expect(oauthMessageToneFor(null, null)).toBe("info");
|
|
expect(oauthMessageToneFor(null, WARNING)).toBe("info");
|
|
});
|
|
});
|