feat: add per_user_headers bootstrap verification flow via submitHandler abstraction - #3875
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds per-user MCP header verification through a new API mutation. Refactors ChangesMCP per-user headers verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClientsTable
participant MCPHeadersAuthorizer
participant verifyMCPClientHeaders
participant MCPVerifyHeadersEndpoint
MCPClientsTable->>MCPHeadersAuthorizer: Open headers dialog for per_user_headers client
MCPHeadersAuthorizer->>verifyMCPClientHeaders: Submit user header values
verifyMCPClientHeaders->>MCPVerifyHeadersEndpoint: POST user_headers
MCPVerifyHeadersEndpoint-->>verifyMCPClientHeaders: Return verification result
verifyMCPClientHeaders-->>MCPClientsTable: Invalidate MCPClients cache
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
|
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (7): Last reviewed commit: "feat: ui support for config json per use..." | Re-trigger Greptile |
ca292aa to
4e7e80c
Compare
aeb0a59 to
f12bc87
Compare
4e7e80c to
bfc4323
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx`:
- Around line 1509-1533: The MCPHeadersAuthorizer usage in mcpClientSheet is
missing conflict handling for the verified-already case. Update the
MCPHeadersAuthorizer props here to pass an onConflict callback, similar to the
create-flow in mcpClientForm, so a 409 from verifyMCPClientHeaders/handleRunTest
surfaces a clear toast or message instead of silently resetting to the input
step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f352ded-454b-4b1c-97b3-a1056e345729
📒 Files selected for processing (4)
ui/app/workspace/mcp-registry/views/mcpClientForm.tsxui/app/workspace/mcp-registry/views/mcpClientSheet.tsxui/app/workspace/mcp-registry/views/mcpHeadersAuthorizer.tsxui/lib/store/apis/mcpApi.ts
f12bc87 to
c3ddfde
Compare
bfc4323 to
4cd233a
Compare
4cd233a to
4236b38
Compare
c3ddfde to
22b8e7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx`:
- Around line 531-537: The pending-verification description in
mcpClientSheet.tsx is still grouping non-per-user-oauth auth types into the
OAuth message. Update the conditional inside SheetDescription so mcpClient.state
=== "pending_verification" distinguishes mcpClient.config.auth_type ===
"per_user_headers" from "oauth" as well as "per_user_oauth", and returns the
correct copy for each auth type. Use the existing mcpClient.state and
mcpClient.config.auth_type branching in mcpClientSheet.tsx to keep the behavior
aligned with the PR intent.
- Around line 893-916: The tool_execution_timeout field in mcpClientSheet.tsx
only checks for integers, so negative values can still be entered even though
the control is meant to represent a non-negative timeout. Update the Input
onChange logic in the mcpClientSheet form to reject values less than 0 before
calling field.onChange, alongside the existing empty/integer handling, so the
tool_execution_timeout value stays non-negative.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd6dae03-9c13-4e85-9ee9-5e3301060e17
📒 Files selected for processing (5)
ui/app/workspace/mcp-registry/library/views/mcpLibraryInstallSheet.tsxui/app/workspace/mcp-registry/views/mcpClientForm.tsxui/app/workspace/mcp-registry/views/mcpClientSheet.tsxui/app/workspace/mcp-registry/views/mcpHeadersAuthorizer.tsxui/lib/store/apis/mcpApi.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- ui/app/workspace/mcp-registry/views/mcpClientForm.tsx
- ui/lib/store/apis/mcpApi.ts
- ui/app/workspace/mcp-registry/views/mcpHeadersAuthorizer.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx`:
- Around line 531-537: The pending-verification description in
mcpClientSheet.tsx is still grouping non-per-user-oauth auth types into the
OAuth message. Update the conditional inside SheetDescription so mcpClient.state
=== "pending_verification" distinguishes mcpClient.config.auth_type ===
"per_user_headers" from "oauth" as well as "per_user_oauth", and returns the
correct copy for each auth type. Use the existing mcpClient.state and
mcpClient.config.auth_type branching in mcpClientSheet.tsx to keep the behavior
aligned with the PR intent.
- Around line 893-916: The tool_execution_timeout field in mcpClientSheet.tsx
only checks for integers, so negative values can still be entered even though
the control is meant to represent a non-negative timeout. Update the Input
onChange logic in the mcpClientSheet form to reject values less than 0 before
calling field.onChange, alongside the existing empty/integer handling, so the
tool_execution_timeout value stays non-negative.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd6dae03-9c13-4e85-9ee9-5e3301060e17
📒 Files selected for processing (5)
ui/app/workspace/mcp-registry/library/views/mcpLibraryInstallSheet.tsxui/app/workspace/mcp-registry/views/mcpClientForm.tsxui/app/workspace/mcp-registry/views/mcpClientSheet.tsxui/app/workspace/mcp-registry/views/mcpHeadersAuthorizer.tsxui/lib/store/apis/mcpApi.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- ui/app/workspace/mcp-registry/views/mcpClientForm.tsx
- ui/lib/store/apis/mcpApi.ts
- ui/app/workspace/mcp-registry/views/mcpHeadersAuthorizer.tsx
🛑 Comments failed to post (2)
ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx (2)
531-537: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pending-verification copy doesn't distinguish
per_user_headersfromoauth.For
pending_verificationstate, onlyper_user_oauthgets its own copy; every other auth type (includingper_user_headers) falls through to the OAuth-specific message "needs a one-time OAuth authorization before it can be used." This is factually wrong forper_user_headersclients, which need header verification, not OAuth. The PR intends to distinguishper_user_headersandoauthcopy (per PR objectives), but this description block still lumps them together.🐛 Proposed fix
<SheetDescription> {mcpClient.state === "pending_verification" ? mcpClient.config.auth_type === "per_user_oauth" ? "This client was declared in config.json. A one-time admin test login is needed to verify the OAuth setup and discover tools — each user will authenticate individually afterward." - : "This client was declared in config.json and needs a one-time OAuth authorization before it can be used." + : mcpClient.config.auth_type === "per_user_headers" + ? "This client was declared in config.json and needs a one-time header verification before it can be used." + : "This client was declared in config.json and needs a one-time OAuth authorization before it can be used." : "MCP server configuration and available tools"} </SheetDescription>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<SheetDescription> {mcpClient.state === "pending_verification" ? mcpClient.config.auth_type === "per_user_oauth" ? "This client was declared in config.json. A one-time admin test login is needed to verify the OAuth setup and discover tools — each user will authenticate individually afterward." : mcpClient.config.auth_type === "per_user_headers" ? "This client was declared in config.json and needs a one-time header verification before it can be used." : "This client was declared in config.json and needs a one-time OAuth authorization before it can be used." : "MCP server configuration and available tools"} </SheetDescription>🤖 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 `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx` around lines 531 - 537, The pending-verification description in mcpClientSheet.tsx is still grouping non-per-user-oauth auth types into the OAuth message. Update the conditional inside SheetDescription so mcpClient.state === "pending_verification" distinguishes mcpClient.config.auth_type === "per_user_headers" from "oauth" as well as "per_user_oauth", and returns the correct copy for each auth type. Use the existing mcpClient.state and mcpClient.config.auth_type branching in mcpClientSheet.tsx to keep the behavior aligned with the PR intent.
893-916: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing non-negative guard on
tool_execution_timeoutinput.The
onChangehandler rejects non-integers but not negatives —min="0"on the<input type="number">doesn't prevent typing a negative value directly. The tooltip states leaving empty/0uses the global setting, implying negative values are not meaningful for a timeout.🐛 Proposed fix
onChange={(e) => { if (e.target.value === "") { field.onChange(undefined); return; } const n = Number(e.target.value); - if (!Number.isInteger(n)) return; + if (!Number.isInteger(n) || n < 0) return; field.onChange(n); }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<FormControl> <Input type="number" className={`w-24 ${isUsingGlobal ? "text-muted-foreground" : ""}`} placeholder="0" value={field.value === 0 || field.value === undefined ? "" : String(field.value)} onChange={(e) => { if (e.target.value === "") { field.onChange(undefined); return; } const n = Number(e.target.value); if (!Number.isInteger(n) || n < 0) return; field.onChange(n); }} min="0" step="1" data-testid="mcp-tool-execution-timeout" /> </FormControl> </FormItem> ); }} />🤖 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 `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx` around lines 893 - 916, The tool_execution_timeout field in mcpClientSheet.tsx only checks for integers, so negative values can still be entered even though the control is meant to represent a non-negative timeout. Update the Input onChange logic in the mcpClientSheet form to reject values less than 0 before calling field.onChange, alongside the existing empty/integer handling, so the tool_execution_timeout value stays non-negative.
4236b38 to
6a892e7
Compare
4487741 to
cd269f9
Compare
f6e41a6 to
4f06391
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
cd269f9 to
976665c
Compare
4f06391 to
40f4550
Compare
Merge activity
|
The base branch was changed.
…_type The Authorize/Verify label ternary keyed off auth_type === 'oauth', but handleStartBootstrap's OAuth-based path is the same one-time admin verification step for every pending_verification client — oauth and per_user_oauth alike. Drop the ternary and always show "Authorize".
40f4550 to
25e4d41
Compare

Summary
Adds support for verifying
per_user_headersMCP clients that were bootstrapped viaconfig.jsonand are sitting inpending_verificationstate. Previously, only OAuth-based clients had a verification flow in the sheet view. This PR introduces averifyMCPClientHeadersAPI endpoint and wires up theMCPHeadersAuthorizerdialog for the bootstrap path, allowing an admin to submit sample header values once to verify the connection and discover tools.Changes
MCPHeadersAuthorizerto accept a genericsubmitHandlercallback instead of a hardcodedpayload+createMCPClientcall, decoupling the dialog from the specific API endpoint it targets. The Create flow passes a handler that callscreateMCPClient, while the bootstrap/verify flow passes one that callsverifyMCPClientHeaders.verifyMCPClientHeadersRTK mutation thatPOSTs to/mcp/client/{id}/verify-headerswithuser_headers, invalidating theMCPClientscache tag on success.bootstrapHeadersOpenstate toMCPClientSheetto drive theMCPHeadersAuthorizerdialog forper_user_headersclients inpending_verification.handleStartBootstrapnow short-circuits forper_user_headersauth type, opening the headers dialog directly instead of callinginitiateVerification.onOpenChangehandler now also guards against closing while the headers authorizer dialog is open.pending_verificationdescription text to correctly reflectper_user_headersvsoauthauth types.Type of change
Affected areas
How to test
per_user_headersMCP client inconfig.jsonso it is created inpending_verificationstate.MCPHeadersAuthorizerdialog should open.pending_verification, tools are discovered, and a success toast appears.mcpClientForm) still works end-to-end forper_user_headersclients.cd ui pnpm i pnpm buildScreenshots/Recordings
Add before/after screenshots of the MCP client sheet for a
per_user_headersclient inpending_verificationstate.Breaking changes
Related issues
Link related issues here.
Security considerations
Sample header values submitted during verification are sent directly to the backend's
/api/mcp/client/{id}/verify-headersendpoint. These values may contain API keys or credentials and should be treated as secrets. No values are persisted in frontend state beyond the lifetime of the dialog.Checklist
docs/contributing/README.mdand followed the guidelines