modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 4

test: make the not-an-OAuthError test assert its own title (review round 3)

Browse source 
The test was named "is not an OAuthError, which is why the retry path must not
claim it" and asserted only `name` and `typeof tokenEndpoint`. Neither touches
the OAuth hierarchy, so a future SDK reparenting the class would leave it
passing with its title now false — and the comment promising the #2280 handling
would be revisited if that changed was hollow. A test that has quietly stopped
testing its subject is worse than no test.

It now asserts against the hierarchy both ways, `OAuthError.isInstance(err)`
and `instanceof`. Verified by mutation rather than assumed: flipping the
assertion to simulate the reparenting fails exactly this test and nothing else
(1 failed, 15 passed).

Also fixes a doubled "terminal terminal", collateral from the round-2 SEP label
removal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018MvNbmAtQuWhDvzi4yXDju
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-09-07 10:45:26 -04:00
commit 8ca00fa2d6
2 changed files with 12 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

2
clients/web/src/hooks/useOAuthRecovery.ts
View file

@ -392,7 +392,7 @@ export function useOAuthRecovery({
);
/**
* Report a terminal terminal token-endpoint refusal (#2280) and clear any re-auth banner.
* Report a terminal token-endpoint refusal (#2280) and clear any re-auth banner.
*
* Every arm goes through this rather than calling the notice helper directly.
* The banner clear is not incidental: a banner left from an *earlier* failure

12
clients/web/src/test/core/auth/insecureTokenEndpoint.test.ts
View file

@ -1,5 +1,8 @@
import { describe, it, expect } from "vitest";
import { InsecureTokenEndpointError } from "@modelcontextprotocol/client";
import {
InsecureTokenEndpointError,
OAuthError,
} from "@modelcontextprotocol/client";
import { findInsecureTokenEndpoint } from "@inspector/core/auth/insecureTokenEndpoint.js";
const ENDPOINT = "http://tenant.example.localhost:3300/api/oauth/token";
@ -21,6 +24,13 @@ describe("SDK brand placement", () => {
// do not treat it as a transient authorization failure. If a future SDK
// changes that, the #2280 handling should be revisited rather than silently
// keeping a now-wrong justification.
//
// Asserted against the hierarchy itself, both ways. Checking only `name`
// and `tokenEndpoint` would leave this passing unchanged if the class were
// reparented — the test would keep its title while having stopped testing
// it, which is worse than not having it.
expect(OAuthError.isInstance(err)).toBe(false);
expect(err instanceof OAuthError).toBe(false);
expect(err.name).toBe("InsecureTokenEndpointError");
expect(typeof err.tokenEndpoint).toBe("string");
});