docs: add token_exchange auth type, per-client TLS/sync/timeout fields, tool list persistence, and reconnect per-call clarifications to MCP docs and OpenAPI spec - #5977
Conversation
|
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe documentation now covers token-exchange MCP configuration and lifecycle behavior, persistence of changed tool-sync results, and reconnect restrictions across MCP guides and OpenAPI descriptions. ChangesMCP documentation behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merge activity
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/deployment-guides/config-json/schema-reference.mdx`:
- Line 293: The schema table entry for needs_session_stickiness must explicitly
include token_exchange, per_user_oauth, and per_user_headers in the
always-per-call rule, and change “Cannot be false” to “This field cannot be
false” while preserving the existing HTTP and SSE/stdio behavior.
- Line 298: Update the tls_config schema-reference description to state that it
applies only to HTTP/SSE connections, insecure_skip_verify takes precedence over
ca_cert_pem when enabled, and insecure_skip_verify is discouraged in production;
replace the ambiguous “and/or” wording so it does not imply custom CA
verification remains effective when verification is disabled.
- Around line 295-296: Update the documentation entries for tool_sync_interval
and tool_execution_timeout to match transports/config.schema.json: document the
supported duration-string formats and units, retain integer compatibility for
tool_execution_timeout, and document legacy nanosecond compatibility for
tool_sync_interval. Ensure the described fallback and disabling behavior remains
accurate.
- Line 311: Clarify the documentation paragraph describing auth_type
"token_exchange" so it states that the API/Web UI declaration path is available
only on enterprise deployments and cannot enable this authentication type on
OSS. Remove the contradictory reference to using that path in a non-enterprise
config.json environment while preserving the existing OSS rejection behavior.
- Line 306: Update the token_exchange entry in the schema reference table to
remove the unsupported authorization_server_url field, keeping only audience,
client_id, optional client_secret, and optional scopes so the documentation
matches transports/config.schema.json and provider behavior.
🪄 Autofix
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: d9850591-deca-4fcc-bdb7-0348d4f81d90
📒 Files selected for processing (7)
docs/deployment-guides/config-json/schema-reference.mdxdocs/mcp/auth/per-user-headers.mdxdocs/mcp/auth/per-user-oauth.mdxdocs/mcp/connecting-to-servers.mdxdocs/mcp/gateway.mdxdocs/openapi/openapi.jsondocs/openapi/paths/management/mcp.yaml
| | `tools_to_auto_execute` | array | Subset of `tools_to_execute` that runs without user confirmation | | ||
| | `headers` | object | Static admin headers (used by `headers` and as additions on `per_user_headers`) | | ||
| | `is_code_mode_client` | boolean | Wrap tools as Python code-mode helpers instead of raw tool calls | | ||
| | `needs_session_stickiness` | boolean | HTTP-only, and only meaningful for `oauth`/`headers`/`none` (per-user auth types are always per-call). `true` holds one persistent upstream connection reused for every tool call; `false`/omitted (default) dials fresh per tool call. Cannot be `false` for `connection_type` `"sse"`/`"stdio"` — both are always sticky. See [Session Stickiness](/mcp/connecting-to-servers#session-stickiness-http-only). | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the per-call rule explicit for token_exchange.
token_exchange exchanges each caller's identity token at tool-use time. Name it with per_user_oauth and per_user_headers in the always-per-call rule. Also change “Cannot be false...” to “This field cannot be false...” so the restriction has an explicit subject.
As per path instructions, this page must document HTTP session stickiness and per-call behavior. The supplied transports/bifrost-http/handlers/mcp.go context, Lines [629]-[774], shows caller tokens are exchanged on each tool use.
🧰 Tools
🪛 LanguageTool
[style] ~293-~293: To form a complete sentence, be sure to include a subject.
Context: ...ed (default) dials fresh per tool call. Cannot be false for connection_type `"sse"...
(MISSING_IT_THERE)
🤖 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 `@docs/deployment-guides/config-json/schema-reference.mdx` at line 293, The
schema table entry for needs_session_stickiness must explicitly include
token_exchange, per_user_oauth, and per_user_headers in the always-per-call
rule, and change “Cannot be false” to “This field cannot be false” while
preserving the existing HTTP and SSE/stdio behavior.
Sources: Path instructions, Linters/SAST tools
| | `tool_sync_interval` | integer | Per-client tool-list sync interval in minutes. `0`/omitted falls back to the global `client.mcp_tool_sync_interval`; negative disables periodic sync for this client. | | ||
| | `tool_execution_timeout` | integer | Per-client tool execution timeout in seconds. `0`/omitted falls back to the global `client.mcp_tool_execution_timeout`. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Match the documented duration formats to the schema.
Lines [295]-[296] declare integer-only minute/second fields. The supplied schema guidance requires duration strings where supported. It also requires integer compatibility for tool_execution_timeout and legacy nanoseconds for tool_sync_interval. Update the documented types and units before users copy these values into config.json.
As per coding guidelines, documentation configuration examples must match transports/config.schema.json. As per path instructions, documentation must match the schema's duration and compatibility behavior.
🤖 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 `@docs/deployment-guides/config-json/schema-reference.mdx` around lines 295 -
296, Update the documentation entries for tool_sync_interval and
tool_execution_timeout to match transports/config.schema.json: document the
supported duration-string formats and units, retain integer compatibility for
tool_execution_timeout, and document legacy nanosecond compatibility for
tool_sync_interval. Ensure the described fallback and disabling behavior remains
accurate.
Sources: Coding guidelines, Path instructions
| | `tool_sync_interval` | integer | Per-client tool-list sync interval in minutes. `0`/omitted falls back to the global `client.mcp_tool_sync_interval`; negative disables periodic sync for this client. | | ||
| | `tool_execution_timeout` | integer | Per-client tool execution timeout in seconds. `0`/omitted falls back to the global `client.mcp_tool_execution_timeout`. | | ||
| | `allow_on_all_virtual_keys` | boolean | When `true`, every virtual key can use this client without an explicit allowlist entry. | | ||
| | `tls_config` | object | `{ "insecure_skip_verify": bool, "ca_cert_pem": string }` — skip TLS verification (development only) and/or trust a custom CA certificate for this client's connection. `ca_cert_pem` supports `env.VAR_NAME`. | |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Document TLS scope and precedence.
tls_config applies only to HTTP/SSE. If insecure_skip_verify is true, it takes precedence over ca_cert_pem. Add both rules and state that insecure_skip_verify is discouraged in production. The current “and/or” wording can imply that the custom CA still protects a connection when verification is disabled.
As per path instructions, TLS scope, precedence, and production guidance must be documented.
🤖 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 `@docs/deployment-guides/config-json/schema-reference.mdx` at line 298, Update
the tls_config schema-reference description to state that it applies only to
HTTP/SSE connections, insecure_skip_verify takes precedence over ca_cert_pem
when enabled, and insecure_skip_verify is discouraged in production; replace the
ambiguous “and/or” wording so it does not imply custom CA verification remains
effective when verification is disabled.
Source: Path instructions
| |-------|-----------|-------------| | ||
| | `oauth_config` | `oauth`, `per_user_oauth` | Optional inline OAuth provider block. The whole block can be omitted, and any inner field (`client_id`, `client_secret`, `authorize_url`, `token_url`, `registration_url`, `scopes`) can be omitted individually — RFC 8414 metadata discovery + RFC 7591 dynamic client registration fill the gaps off `connection_string` at admin-click time. `client_id` / `client_secret` support `env.VAR_NAME` and `vault.path` references (resolved at runtime, reference stored); the other fields take literal values (encrypted at rest, redacted in API responses). | | ||
| | `per_user_header_keys` | `per_user_headers` | Required, non-empty. Array of header names each end-user must supply. | | ||
| | `token_exchange` | `token_exchange` | Required. `{ "audience": string, "client_id": SecretVar, "client_secret": SecretVar (optional, public clients), "scopes": string[] (optional), "authorization_server_url": string (optional) }`. `client_id`/`client_secret` support `env.VAR_NAME`/`vault.path` references. Include `"offline_access"` in `scopes` where the identity provider supports it to keep the retained admin discovery credential self-renewing instead of expiring into `needs_reauth`. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Remove the unsupported authorization_server_url field.
transports/config.schema.json, Lines [4846]-[4874], defines token_exchange with audience, client_id, optional client_secret, and optional scopes. It sets additionalProperties to false. A user who follows this table will fail schema validation.
As per coding guidelines, documentation configuration examples must match transports/config.schema.json. As per path instructions, documentation must remain in parity with the schema and provider behavior.
Suggested correction
-... `"scopes": string[] (optional), "authorization_server_url": string (optional) }`.
+... `"scopes": string[] (optional) }`.📝 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.
| | `token_exchange` | `token_exchange` | Required. `{ "audience": string, "client_id": SecretVar, "client_secret": SecretVar (optional, public clients), "scopes": string[] (optional), "authorization_server_url": string (optional) }`. `client_id`/`client_secret` support `env.VAR_NAME`/`vault.path` references. Include `"offline_access"` in `scopes` where the identity provider supports it to keep the retained admin discovery credential self-renewing instead of expiring into `needs_reauth`. | | |
| | `token_exchange` | `token_exchange` | Required. `{ "audience": string, "client_id": SecretVar, "client_secret": SecretVar (optional, public clients), "scopes": string[] (optional) }`. `client_id`/`client_secret` support `env.VAR_NAME`/`vault.path` references. Include `"offline_access"` in `scopes` where the identity provider supports it to keep the retained admin discovery credential self-renewing instead of expiring into `needs_reauth`. | |
🤖 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 `@docs/deployment-guides/config-json/schema-reference.mdx` at line 306, Update
the token_exchange entry in the schema reference table to remove the unsupported
authorization_server_url field, keeping only audience, client_id, optional
client_secret, and optional scopes so the documentation matches
transports/config.schema.json and provider behavior.
Sources: Coding guidelines, Path instructions
| The schema enforces these pairings: `oauth_config` is rejected on non-OAuth auth types, `per_user_header_keys` is rejected on any auth type other than `per_user_headers`, and `token_exchange` is rejected on any auth type other than `token_exchange` — a misplaced block fails `$schema` validation instead of being silently ignored. | ||
|
|
||
| <Warning> | ||
| **Enterprise only:** `auth_type: "token_exchange"` in `config.json` is rejected on OSS — the client is skipped entirely at boot with an error logged naming it. Declare `token_exchange` clients via the API/Web UI on an enterprise deployment instead if you need them in a non-enterprise `config.json` environment during a migration. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify the OSS migration condition.
The warning says OSS rejects token_exchange, then says to use API/Web UI on an enterprise deployment “if you need them in a non-enterprise config.json environment.” This is contradictory. State that the API/Web UI path applies only to enterprise deployments and cannot enable this auth type on OSS.
As per path instructions, documentation must match provider behavior. The PR objective defines token_exchange as enterprise-only.
🤖 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 `@docs/deployment-guides/config-json/schema-reference.mdx` at line 311, Clarify
the documentation paragraph describing auth_type "token_exchange" so it states
that the API/Web UI declaration path is available only on enterprise deployments
and cannot enable this authentication type on OSS. Remove the contradictory
reference to using that path in a non-enterprise config.json environment while
preserving the existing OSS rejection behavior.
Source: Path instructions
ae5a1d1 to
ab01d0e
Compare
2 participants
Summary
This PR updates the MCP documentation to reflect new client configuration fields, the
token_exchangeauth type (enterprise only), clarified reconnect behavior for per-call clients, and tool list persistence across restarts.Changes
token_exchangeas a validauth_typein theconfig.jsonschema reference, including its requiredtoken_exchangeblock fields, enterprise-only restriction, and the newPOST /api/mcp/client/{id}/verify-exchangeverification endpoint.client_configsfields:is_ping_available,tool_sync_interval,tool_execution_timeout,allow_on_all_virtual_keys, andtls_config.400not only for per-user auth clients but for any per-call client (including shared clients withneeds_session_stickinessfalse/omitted), since none hold a shared upstream connection to re-establish. Updated this inconnecting-to-servers.mdx,gateway.mdx, the OpenAPI YAML, and the generatedopenapi.json.connectedtohealthyand addedtoken_exchangeto the set of auth types that boot intopending_verification.gateway.mdx,per-user-headers.mdx, andper-user-oauth.mdxclarifying that every discovered tool list is persisted to the database, so a restart does not revert the tool list to what was found at original bootstrap verification. Persistence is skipped when the result is byte-identical to what is already stored.token_exchangeblock rejection on non-token_exchangeauth types.Type of change
Affected areas
How to test
Review the rendered documentation pages for:
docs/deployment-guides/config-json/schema-reference.mdx— confirm new fields andtoken_exchangeauth type appear correctly, including the enterprise warning block.docs/mcp/connecting-to-servers.mdx— confirm the reconnect400note accurately describes per-call clients.docs/mcp/gateway.mdx,docs/mcp/auth/per-user-headers.mdx,docs/mcp/auth/per-user-oauth.mdx— confirm the tool list persistence note is present.docs/openapi/paths/management/mcp.yamlanddocs/openapi/openapi.json— confirm the reconnect endpoint description matches the prose documentation.Breaking changes
Related issues
Security considerations
The
token_exchangeauth type is explicitly documented as enterprise-only and is rejected at boot on OSS deployments with an error logged. Thetls_config.insecure_skip_verifyfield is documented as development-only. Thetoken_exchangeblock fields (client_id,client_secret) supportenv.VAR_NAMEandvault.pathreferences, consistent with existing secret handling patterns.Checklist
docs/contributing/README.mdand followed the guidelines