fix(oauth-debugger): persist registration strategy + protocol version through Convex#3040
Conversation
… through Convex Saving "Pre-registered" (or CIMD, or an older protocol) in the OAuth Debugger's Configure Server to Test modal silently reverted to DCR/2025-11-25 in authenticated projects: the Convex sync payload only carried oauthScopes/clientId/oauthResourceUrl, the servers table had no columns for the rest of the test profile, and the catalog round-trip rebuilt oauthFlowProfile without it. The localStorage fallback is disabled in hosted mode on both ends, so nothing backstopped it. - sync payload now sends oauthProtocolVersion + oauthRegistrationStrategy (requires mcpjam-backend#658 deployed first — Convex mutations reject unexpected args) - deserializeServersFromConvex restores them into the rebuilt profile, narrowing the string columns back to the closed unions (normalizeOAuthProtocolVersion / normalizeOAuthRegistrationStrategy); unknown wire values read as absent so reader defaults apply - reconnect picks the persisted strategy up for free: the orchestrator already prefers server.oauthFlowProfile — new test drives the real pipeline (Convex row -> deserialize -> reconnect options) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dosubot
Bot
added
the
size:M
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2cb9c5ff-7d4a-492e-846a-ce78779fc727) |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Internal previewPreview URL: https://mcp-inspector-pr-3040.up.railway.app |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
WalkthroughThis change adds persistence and normalization support for OAuth protocol version and registration strategy selections. Two normalizer functions map unknown stored values to known union types, defaulting to undefined for unrecognized inputs. The Changes
Sequence Diagram(s)sequenceDiagram
participant ConvexRow
participant deserializeServersFromConvex
participant normalizeOAuthProtocolVersion
participant normalizeOAuthRegistrationStrategy
participant oauthFlowProfile
ConvexRow->>deserializeServersFromConvex: flat OAuth columns
deserializeServersFromConvex->>normalizeOAuthProtocolVersion: raw protocolVersion value
normalizeOAuthProtocolVersion-->>deserializeServersFromConvex: normalized value or undefined
deserializeServersFromConvex->>normalizeOAuthRegistrationStrategy: raw registrationStrategy value
normalizeOAuthRegistrationStrategy-->>deserializeServersFromConvex: normalized value or undefined
deserializeServersFromConvex->>oauthFlowProfile: persist protocolVersion, registrationStrategy
Estimated code review effort: 3/5 (moderate — touches type contracts, serialization logic, and persistence paths across several files) Related issues: None specified. Related PRs: None specified. Suggested labels: oauth, serialization, testing Suggested reviewers: None specified. Poem Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.1)mcpjam-inspector/client/src/lib/__tests__/project-serialization.test.tsFile contains syntax errors that prevent linting: Line 7: Declarations inside of a 🔧 ESLint
mcpjam-inspector/client/src/hooks/use-server-state.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/hooks/useProjects.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/lib/__tests__/project-serialization.test.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/lib/oauth/profile.ts (1)
34-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLookup arrays aren't compile-time exhaustive against the union types.
OAUTH_PROTOCOL_VERSIONS/OAUTH_REGISTRATION_STRATEGIESare typed asreadonly OAuthProtocolVersion[]/readonly OAuthRegistrationStrategy[], which permits a subset of the union. If a new protocol version or strategy is later added to the union type but the array update is missed, the normalizer would silently treat a legitimate value as "unknown" and fall back to defaults — exactly the failure mode this PR fixes for other cases.Deriving the arrays from a
satisfies Record<Union, true>object would make the compiler flag a missing entry.♻️ Suggested exhaustiveness guard
-const OAUTH_PROTOCOL_VERSIONS: readonly OAuthProtocolVersion[] = [ - "2025-03-26", - "2025-06-18", - "2025-11-25", -]; - -const OAUTH_REGISTRATION_STRATEGIES: readonly OAuthRegistrationStrategy[] = [ - "cimd", - "dcr", - "preregistered", -]; +const OAUTH_PROTOCOL_VERSIONS = Object.keys({ + "2025-03-26": true, + "2025-06-18": true, + "2025-11-25": true, +} satisfies Record<OAuthProtocolVersion, true>) as OAuthProtocolVersion[]; + +const OAUTH_REGISTRATION_STRATEGIES = Object.keys({ + cimd: true, + dcr: true, + preregistered: true, +} satisfies Record<OAuthRegistrationStrategy, true>) as OAuthRegistrationStrategy[];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcpjam-inspector/client/src/lib/oauth/profile.ts` around lines 34 - 63, The lookup lists in normalizeOAuthProtocolVersion and normalizeOAuthRegistrationStrategy are not guaranteed to stay exhaustive with OAuthProtocolVersion and OAuthRegistrationStrategy, so a future union expansion could be silently treated as unknown. Update the OAUTH_PROTOCOL_VERSIONS and OAUTH_REGISTRATION_STRATEGIES definitions in profile.ts to be compiler-checked against the full unions (for example by deriving them from an exhaustiveness-checked source), and keep the existing normalizer functions using those symbols so any missing union member causes a TypeScript error instead of falling back to defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@mcpjam-inspector/client/src/lib/oauth/profile.ts`:
- Around line 34-63: The lookup lists in normalizeOAuthProtocolVersion and
normalizeOAuthRegistrationStrategy are not guaranteed to stay exhaustive with
OAuthProtocolVersion and OAuthRegistrationStrategy, so a future union expansion
could be silently treated as unknown. Update the OAUTH_PROTOCOL_VERSIONS and
OAUTH_REGISTRATION_STRATEGIES definitions in profile.ts to be compiler-checked
against the full unions (for example by deriving them from an
exhaustiveness-checked source), and keep the existing normalizer functions using
those symbols so any missing union member causes a TypeScript error instead of
falling back to defaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26e621fe-fe82-483e-83c8-52ef96c37fab
📒 Files selected for processing (6)
mcpjam-inspector/client/src/hooks/use-server-state.tsmcpjam-inspector/client/src/hooks/useProjects.tsmcpjam-inspector/client/src/lib/__tests__/project-serialization.test.tsmcpjam-inspector/client/src/lib/oauth/profile.tsmcpjam-inspector/client/src/lib/project-serialization.tsmcpjam-inspector/client/src/state/__tests__/oauth-orchestrator.test.ts
1 participant
Inspector half of the OAuth Debugger persistence fix. Depends on MCPJam/mcpjam-backend#658 — the backend must deploy first (Convex mutations reject unexpected args, so sending the new fields against the old deployment would break server saves).
Problem
In authenticated projects, saving "Pre-registered" (or CIMD, or an older protocol version) in the debugger's "Configure Server to Test" modal silently reverts to Dynamic (DCR) / 2025-11-25:
oauthScopes/clientId/oauthResourceUrl— the rest of the test profile had nowhere to go (no columns).activeProject.serversfrom the flat row, reconstructingoauthFlowProfilewithout a strategy → defaults back to DCR.mcp-oauth-config-*) is explicitly disabled in hosted mode on both write and read, so nothing backstops it.This also meant reconnect ran the wrong flow:
ensureAuthorizedForReconnectprefersserver.oauthFlowProfile.registrationStrategy, but that field never survived the catalog round-trip.Change
use-server-state.ts): sendsoauthProtocolVersion+oauthRegistrationStrategyfrom the profile.project-serialization.ts): restores both into the rebuilt profile. The backend columns are plain strings by design (so an old/new client never fails a whole server save over an unknown value); the client narrows them back to the closed unions via newnormalizeOAuthProtocolVersion/normalizeOAuthRegistrationStrategyinlib/oauth/profile.ts— unknown wire values read as absent and fall to the reader-side defaults.deserializeServersFromConvex→ensureAuthorizedForReconnect→initiateOAuthcalled withregistrationStrategy: "preregistered"/protocolVersion: "2025-06-18".RemoteServertype gains the two optional fields.Tests
"quantum","2099-01-01") normalize to absent, legacy rows unchanged.Rollout
🤖 Generated with Claude Code
Note
Medium Risk
Touches OAuth reconnect and Convex server save payloads; rollout depends on the backend schema/mutations accepting the new fields first.
Overview
Fixes hosted OAuth debugger settings silently reverting to Dynamic (DCR) and protocol 2025-11-25 after save when the Convex catalog rebuilds
oauthFlowProfilefrom flat server rows.Convex sync (
use-server-state.ts) now includesoauthProtocolVersionandoauthRegistrationStrategyfrom the in-memory test profile.RemoteServertypes those fields.Deserialize (
project-serialization.ts) maps the flat columns back ontooauthFlowProfile, withnormalizeOAuthProtocolVersion/normalizeOAuthRegistrationStrategyinlib/oauth/profile.tsso unknown wire strings are dropped instead of breaking saves.Reconnect needs no orchestrator change: once the profile survives the round-trip,
ensureAuthorizedForReconnectpasses the saved strategy/version intoinitiateOAuth(new integration test viadeserializeServersFromConvex).Tests cover full round-trip, strategy-only rows, unknown values, and legacy rows without columns. Requires backend #658 deployed first so Convex accepts the new mutation fields.
Reviewed by Cursor Bugbot for commit 9d9fcfc. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Persist the OAuth Debugger’s registration strategy and protocol version through Convex so saved profiles don’t reset to DCR/2025-11-25, and reconnect uses the right flow.
Bug Fixes
oauthProtocolVersionandoauthRegistrationStrategyin the sync payload.oauthFlowProfileon read; unknown values fall back to defaults, legacy rows unchanged.RemoteServerwith optional fields and add round‑trip + reconnect tests.Dependencies
MCPJam/mcpjam-backend#658to be deployed first.Written for commit 9d9fcfc. Summary will update on new commits.