Auth hardening CI gaps + SDK consolidation (EMA leg 2 / scopes)

[BobDickinson]

Closes #1527

Summary

• Close remaining [MCP 2026-07-28] Authorization hardening #1527 acceptance gaps with automated tests: SEP-2468 iss reject paths, SEP-2207 offline_access on authorize, SEP-2351 discovery URL shapes, SEP-2352 AS-migration re-authorize, plus EraNegotiationFailed → 401 unwrap in isUnauthorizedError.
• Lean EMA leg 2 and scope helpers on the v2 SDK (discoverAndRequestJwtAuthGrant, computeScopeUnion / isStrictScopeSuperset) and document the consolidation path.
• Drop deprecated TypeScript baseUrl from shared/client tsconfigs (TS 6 deprecation).

Test plan

• npm run ci (validate + coverage + smoke + Storybook) passed locally
• CI green on the PR
• Spot-check OAuth e2e paths still complete with callback iss from the local test AS

Made with Cursor

[BobDickinson]

This PR was covers two types of auth cleanup. First, I wanted to ensure that after switching to the TypeScript v2 SDK that we were appropriately using the auth capabilities of the SDK instead of rolling our own (it turns out that most of that got handled in the v2 SDK port). There were a couple of small items left that were addressed in this PR. Second, I wanted to make sure we had covered all of the auth hardening issues from the 2026-07-28 spec release (per #1527). It turns out that those were all handled either before or during the v2 SDK port also, and the only gaps (other than one small fix) were in test coverage / validation for the auth hardening issues. This PR basically gets our auth support up-to-date and clean with respect to the v2 SDK and the 2026-07-28 spec.

[BobDickinson]

@claude review

[claude]

Claude finished @BobDickinson's task in 4m 22s —— View job

---

Claude PR Review

• Read PR diff against the base
• Review core auth changes (wire.ts, scopes.ts, utils.ts, emaFlow.ts)
• Review test changes & test-server changes
• Check tsconfig / baseUrl removal
• Post review feedback

Reviewed all 27 files. This is a clean, well-scoped PR — mostly test/coverage hardening plus a tidy consolidation onto the v2 SDK. Nothing blocking; a few observations below, mostly around one maintainability risk.

---

✅ What looks good

• isUnauthorizedError deep-walk (core/auth/utils.ts) is solid: UnauthorizedError.isInstance short-circuit, status/code === 401, message regex, and recursion through both native cause and data.cause, with a seen set guarding cycles. The new unit tests cover each shape (SDK UnauthorizedError, EraNegotiationFailed wrapping in data.cause, native Error.cause, and a negative case for an unrelated SdkError).
• scopes.ts re-export — collapsing to the SDK's computeScopeUnion / isStrictScopeSuperset is safe here because the existing scopes.test.ts (unchanged) still asserts the observable behaviors — dedupe/order-preservation, undefined on all-empty, and the "missing token scope → force re-auth" edge. Those act as a parity guard against SDK-semantics drift. Good that they weren't deleted.
• emaFlow.ts resource fallback (ctx.resourceUrl?.href ?? ctx.resourceMetadata.resource) correctly matches wire.ts now requiring a non-empty resource.
• test-server iss support is RFC 9207-correct: advertises authorization_response_iss_parameter_supported and appends iss (issuer with trailing slash stripped, matching metadata issuer). The SEP-2468 accept / mismatch / missing-iss trio and the SEP-2352 AS-migration test are genuinely end-to-end and read well.
• baseUrl removal across the tsconfigs is done right — paths are made explicitly config-relative (./core/*, ../../core/*) so resolution is unchanged.

🟡 Maintainability — string-matched SDK error mapping (core/auth/ema/wire.ts)

exchangeIdJag distinguishes SDK failure modes by regex-matching the error message:

if (/Failed to discover token endpoint/i.test(message)) { … "IdP metadata missing token_endpoint" }
if (/Invalid token exchange response/i.test(message))   { … "did not return an ID-JAG" }

This couples us to the SDK's internal wording. The concern is compounded by the tests (wire.test.ts) now mocking the SDK helper and fabricating those exact strings — so if the SDK changes its message text, production silently degrades to the generic EMA leg 2 (…) wrapper and the tests won't catch it (they assert against the strings they invented, not the SDK's real output).

Two options, in order of preference:

1. If discoverAndRequestJwtAuthGrant throws typed errors (e.g. SdkError with a code), branch on the type/code instead of the message.
2. Otherwise, add a note that the regexes mirror SDK wording and are drift-sensitive, and consider one non-mocked test that lets the real SDK produce the "missing token_endpoint" error (e.g. all-404 discovery) so a wording change surfaces.

Fix this →

🟢 Minor notes (non-blocking)

• Leg 2 test coverage narrowed to mocks. Delegating to the SDK is the right call, but wire.test.ts no longer exercises a real HTTP round-trip for leg 2 (leg 3 still does). The EMA e2e paths don't cover leg 2 either, so leg 2 is now mock-only. Acceptable trade-off for consolidation — just flagging the reduced integration confidence.
• isUnauthorizedError data shape assumption. The walk requires "cause" in data. If a future SDK nests the UnauthorizedError directly as data (rather than data.cause), it won't be caught. Current SDK shapes are covered by tests, so fine for now.

Verdict

Looks good to merge once the string-matching risk is either addressed or consciously accepted. Coverage additions are thorough and the SDK consolidation is clean.

[BobDickinson]

Good catch on the mock/production drift risk.

We kept the message-regex remaps (the SDK still throws plain Error here, not typed codes) and added two real-SDK canaries in wire.test.ts so a wording change in the SDK fails CI instead of silently falling through to the generic EMA leg 2 wrapper:

1. Discovery all-404 → remapped IdP metadata missing token_endpoint
2. Valid IdP metadata + invalid token-exchange body → remapped did not return an ID-JAG

Short comments in wire.ts (above the regexes) and above those tests note the coupling to current SDK wording.

[BobDickinson]

I validated the EMA smoke test cases (I did have to update the EMA test server config because we are now pickier about requesting unsupported scopes). EMA is working (with the bundled ToDo test server and our EMA composable test server).

[cliffhall]

Smoke test results — v2/main merged into this branch

Merged the latest v2/main into v2/auth-sdk-consolidation locally and ran the full smoke suite. No merge conflicts (ort auto-merge; the branch was 2 commits behind and the incoming pagination/logging work didn't touch any auth files). All claims below verified against the merged tree.

✅ Build / typecheck / lint (proves the baseUrl removal)

npm run validate across all four clients (web → cli → tui → launcher) — green: format:check + lint + build + tsc + unit tests. This confirms dropping the deprecated baseUrl from tsconfig.base.json and the client configs still resolves the @inspector/core/* (and web react/pino) path aliases correctly.

✅ Auth hardening CI gaps (#1527 acceptance)

Named tests present and passing in inspectorClient-oauth-e2e.test.ts (integration project, 32/32 in-file; 920/920 across the whole integration suite):

Claim	Test	Result
SEP-2468 iss accept	accepts completeOAuthFlow with matching callback iss	✅
SEP-2468 iss reject-mismatch	rejects mismatched callback iss before storing tokens	✅
SEP-2468 iss reject-missing	rejects missing iss when AS requires authorization_response iss	✅
SEP-2207 offline_access	requests offline_access when AS scopes_supported includes it	✅
SEP-2352 AS migration	does not silently reuse issuer-A tokens when discovery resolves issuer B	✅

EraNegotiationFailed → 401 unwrap (isUnauthorizedError, utils.test.ts):

• returns true when EraNegotiationFailed wraps UnauthorizedError in data.cause ✅
• returns true when UnauthorizedError is on native Error.cause ✅
• returns true for SDK UnauthorizedError / returns false for unrelated SdkError ✅

✅ EMA leg 2 / scopes lean on the v2 SDK

core/auth/ema/wire.ts (wire.test.ts):

• delegates to the SDK ID-JAG helper and returns the grant ✅ (now uses discoverAndRequestJwtAuthGrant)
• maps real SDK missing-token-endpoint errors (canary) ✅
• maps real SDK invalid token-exchange responses (canary) ✅
• rejects an empty resource identifier before calling the SDK ✅
• throws when the SDK returns success without a jwtAuthGrant ✅

core/auth/scopes.ts now re-exports the SDK's computeScopeUnion / isStrictScopeSuperset — exercised end-to-end in the browser smoke below.

✅ Live browser smoke — mid-session step-up (SEP-2350 scope union + 401 recovery)

Drove the real UI (web dev server on :6274) against the local composable OAuth server (oauth-step-up-demo, test-servers/configs/oauth-step-up-demo.json) with catalog scopes mcp tools:read only, so get_temp (requires weather:read) forces a step-up. This exercises the consolidated scopes.ts union logic and the utils.ts 401 detection on the live path.

1. Connect → DCR + PKCE consent — authorize page requests mcp, tools:read.

2. Connected after OAuth — tools/list OK, protocol handshake in the log.

3. get_temp → step-up modal (SEP-2350 union) — modal lists mcp (already granted), tools:read (already granted), weather:read (new).

4. Re-authorize carries the union on the wire — authorize URL = scope=mcp tools:read weather:read.

5. Step-up success toast — "Step-up authorization succeeded. Retry your action."; session kept (no disconnect).

6. get_temp succeeds after step-up — "The temperature in NYC is 25 degrees C"; tools/call OK.

Summary

Merge is clean, validate is green across all clients, every SEP hardening / EMA-consolidation / baseUrl claim is backed by a named passing test, and the live scope-union step-up flow works end-to-end on the merged tree.

_{🤖 Smoke-tested with Claude Code}

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 45s —— View job

---

Claude PR Review (re-review for @cliffhall)

• Read PR diff against origin/v2/main
• Review core auth changes (wire.ts, scopes.ts, utils.ts, emaFlow.ts)
• Review test & test-server changes
• Check tsconfig / baseUrl removal
• Post review feedback

Re-reviewed the 28 auth/spec/test files. This is in good shape — the maintainability concern from the prior round (string-matched SDK error mapping) has been addressed properly, and @cliffhall's merge-and-smoke run confirms the SEP claims against a live tree. Nothing blocking.

---

✅ Prior review concern is resolved

The earlier flag was that exchangeIdJag regex-matches SDK error wording and the tests mocked those exact strings — so SDK drift would silently degrade to the generic EMA leg 2 (…) wrapper without failing CI. The fix is the right one:

• core/auth/ema/wire.ts:53-57 now documents the coupling explicitly and points at the canaries.
• wire.test.ts adds two real-SDK canaries (useRealDiscoverAndRequestJwtAuthGrant routes the mock through vi.importActual(...).discoverAndRequestJwtAuthGrant): all-404 discovery → IdP metadata missing token_endpoint, and a valid-metadata-but-invalid-exchange-body → did not return an ID-JAG. These drive the real SDK code path, so a wording change now fails CI instead of falling through silently. That's exactly the "option 2" mitigation and it's the correct call given the SDK still throws plain Errors (not typed codes) here.

✅ Core auth changes

• isUnauthorizedError deep-walk (utils.ts:182-222) — UnauthorizedError.isInstance short-circuit, status/code === 401, transport failed …(401) regex, and recursion through both native cause and data.cause with a seen cycle guard. Covers the EraNegotiationFailed unwrap cleanly, and utils.test.ts exercises each shape plus a negative case.
• scopes.ts — collapsing to the SDK's computeScopeUnion / isStrictScopeSuperset is safe; the unchanged scopes.test.ts still asserts the observable behaviors (dedupe/order, undefined-on-empty, force-reauth edge) as a parity guard. The Inspector-only resolvePersistedScopeAfterGrant / resolveEffectiveGrantedScope helpers stay local — correct scoping.
• emaFlow.ts:70-71 — ctx.resourceUrl?.href ?? ctx.resourceMetadata.resource correctly satisfies wire.ts now requiring a non-empty resource (which trims + throws on empty). Good defensive pairing.

✅ Test-server iss support (RFC 9207 / SEP-2468)

test-server-oauth.ts advertises authorization_response_iss_parameter_supported: true and appends iss to the redirect, derived from the same config.issuerUrl ?? new URL(requestBaseUrl) (trailing-slash stripped) that builds the metadata issuer — so accept/mismatch/missing-iss all exercise a consistent issuer. The three SEP-2468 e2e cases assert tokens are not stored on reject, and the SEP-2352 AS-migration test is genuinely end-to-end (two live AS servers, discovery flipped to issuer B, AS grants cleared so refresh-reuse can't mask the bug, then asserts re-authorize targets B's origin). These read well and are real integration coverage.

✅ baseUrl removal

Done right across tsconfig.base.json + client configs: paths are made explicitly config-relative (./core/*, ../../core/*) so resolution is unchanged, with an inline note about the TS 6 deprecation. validate (which runs build + tsc) being green across all four clients is the proof.

🟢 Minor, non-blocking observations

• Leg 2 is now mock-only for the happy path. The "delegates to the SDK ID-JAG helper" test asserts the full call args including fetchFn: undefined — slightly brittle (an argument-shape change would fail it) but harmless, and the canaries + leg 3's real round-trip keep integration confidence reasonable. Acceptable trade-off for the consolidation.
• isUnauthorizedError data.cause assumption still requires "cause" in data; a future SDK nesting UnauthorizedError directly as data (not data.cause) wouldn't be caught. Current SDK shapes are covered — fine for now, just noting the shape is load-bearing.

Verdict

LGTM. The consolidation is clean, the coverage additions are thorough and honest (canaries over fabricated strings), and the merged-tree smoke run backs every SEP claim. Good to merge.
· branch v2/auth-sdk-consolidation

[cliffhall]

Thanks for the re-review — appreciate the thorough pass. LGTM confirmed, nothing blocking.

Acknowledging the two non-blocking observations (no code changes made, agreeing they're acceptable as-is):

1. Leg-2 happy-path is mock-only and asserts fetchFn: undefined. Agreed it's slightly brittle to call-arg shape. Leaving as-is: the two real-SDK canaries drive the actual discoverAndRequestJwtAuthGrant path and leg 3 does a real round-trip, so the mock-only happy path is just asserting we hand the SDK the right inputs. Worth revisiting if/when the SDK grows a typed leg-2 result.

2. isUnauthorizedError data.cause shape is load-bearing. Correct — the deep-walk currently assumes UnauthorizedError lands on native cause or data.cause, not directly as data. All current SDK/EraNegotiationFailed shapes are covered; if a future SDK nests it as data itself we'd extend the walk (and the canary-style tests would be the place to pin it).

Both are captured here as follow-up notes rather than blockers. No changes to the diff.