Guided-auth removal, OAuth smoke testing, and TUI/CLI support#1531
Guided-auth removal, OAuth smoke testing, and TUI/CLI support#1531BobDickinson wants to merge 6 commits into
Conversation
- Replaced `runGuidedAuth` with `authenticate` in the InspectorClient OAuth end-to-end tests to support quick authentication with CIMD pre-registration. - Updated comments to reflect the change in authentication method and its support for HTTP test metadata URLs. - Ensured that the tests continue to validate the expected authorization URL after the change. Additionally, imported `ensureCimdClientRegistration` in the OAuthManager to streamline client registration during the authentication process.
Resolve Code.ts theme conflict: keep both block margin reset and nowrap ellipsis variant from v2/main. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@claude review |
|
Claude finished @BobDickinson's task in 4m 17s —— View job PR Review — Guided-auth removal, CIMD, OAuth smoke testingI read the diff against A few things worth a look before merge — mostly test-coverage gaps rather than logic bugs. FindingsCorrectness / behavior (looks right, noting for the record)
Test coverage gaps (CI runs
|
Review actions and feedbackTest coverage gaps1. Added Shared wiring lives in 2. No dedicated unit test. Covered transitively by 3. Per-server Acknowledged. Round-trip is tested for the in-memory store ( Minor / nits4. Port 0 / Added JSDoc on 5. Fixed default port 6276 / concurrent TUI OAuth Documented in 6. Correct for current web (browser storage). Will revisit with the 7. Agreed — |
|
This is the next step in the auth cleanup/evolution. When this is merged, I will finish up #1514 (which should be more focussed), then do #1526 (which will handle mid-session auth and step up, but also addresses other common issues like invalid token on initial auth, token refresh, etc). All of that combined should put our OAuth support in pretty good shape. |






How this PR grew
This PR started as a narrow cleanup: remove guided OAuth from v2. Guided auth (
beginGuidedAuth,runGuidedAuth,proceedOAuthStep, andcore/auth/state-machine.ts) was a debugging-oriented, step-through flow that duplicated what the normal OAuth path already does; product flows useauthenticate()andcompleteOAuthFlow()only. Removing it simplifiedOAuthManager,InspectorClient, and the test surface.The next step was to validate standard OAuth against real MCP servers and authorization servers—not only the in-repo
TestServerHttpfixtures. That work included:specification/v2_auth_smoke_testing.md— manual procedures for header/PAT bypass, static (preregistered) client, DCR, and CIMD against hosted endpoints (MCP Example, GitHub MCP, Stytch demo, xaa.dev EMA, etc.)..gitignorenow includes/configs/so developers can keep local catalog and client-config files for smokes without checking them in. The smoke spec describes how to set those up (--client-config,--catalog, default paths under~/.mcp-inspector/).Running those smokes on web required restoring two v1/v1.5 capabilities that had not yet been ported to v2:
client.jsonand wire it intoInspectorClient(needed for Stytch and similar ASes).TUI work to run the same smokes:
client.jsonat startup (EMA IdP, CIMD) vialoadRunnerClientConfig/buildRunnerClientAuthOptions.127.0.0.1:6276(default runner port; T9 “MCPO”).loadServerEntries()so TUI/CLI readoauth.clientSecretfrom the OS keychain after web Server Settings saved it.authProviderwhen no tokens exist (so the SDK does not navigate before the callback server listens);dropCachedTransport()so connect → OAuth → connect works on the sameInspectorClient.CLI — shared flags (
--client-config, CIMD/static overrides,--callback-url), asyncloadServerEntries, token reuse from~/.mcp-inspector/storage/oauth.json. Interactive OAuth (callback server, 401 retry) remains a Phase 4 follow-up; see CLI README and smoke spec known gaps.EMA — same runner/client-config path; Phase 4 checklist and smoke cross-links updated in
v2_auth_ema.md.The diff (~90 files) spans guided-auth removal, smoke-test documentation, web parity, TUI/CLI runners, core connect fixes, and specs.
Summary
Remove guided OAuth and enable production-style OAuth smoke testing across web, TUI, and CLI: install-level CIMD, per-server OAuth clear, TUI connect-time auth with loopback callback, shared
client.jsonand keychain-aware catalog load, and auth display including client registration kind (static / DCR / CIMD).Core / auth
beginGuidedAuth,runGuidedAuth,proceedOAuthStep,core/auth/state-machine.ts,oauthStepChange); singleauthenticate()/completeOAuthFlow()path.authProviderwhen no stored tokens; attach when authorized.transportHasAuthProvider+dropCachedTransport()for connect → OAuth → connect on the same instance.core/client/runner.ts,core/auth/node/runner-oauth-callback.ts(default callback 6276).rehydrateMcpConfigFromKeychain()inloadServerEntries()for TUI/CLI (same effective secrets as webGET /api/servers).OAuthClientRegistrationKind; CIMDregistrationKindonsaveClientInformation.Web
InspectorClientat connect.clearServerOAuthState).TUI
client.jsonat startup; OAuth on HTTP/SSE servers.sse/streamable-http(isOAuthCapableServerConfiginApp.tsx).CLI
--client-config,--client-id,--client-secret,--client-metadata-url,--callback-url; asyncloadServerEntries.~/.mcp-inspector/storage/oauth.json; interactive OAuth deferred (README).Specs / docs
specification/v2_auth_smoke_testing.md— real-server matrix, TUI/CLI procedures..gitignore—/configs/for optional local dev config (not shipped).v2_auth_ema.md,v2_servers_file.md,v2_scope.md— runner/keychain updates.clients/tui/README.md,clients/cli/README.md.Removed / replaced
specification/v2_enterprise_managed_auth.md→v2_auth_ema.md.clients/web/src/test/integration/auth/state-machine.test.ts.Test plan
npm run validate(web, CLI, TUI, launcher)npm run test:integration(web) — OAuth e2e including connect retry without disconnectManual procedures:
specification/v2_auth_smoke_testing.md.Follow-ups (not in this PR)
RemoteOAuthStorage/oauth.jsonparity with TUI/CLI).