Add PRD and design document for MCP servers support feature.#4172
Open
genericaccount-de wants to merge 1 commit into
Open
Conversation
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR updates Mini Chat design and product documents to specify MCP server support across architecture, APIs, database tables, runtime behavior, security constraints, metrics, and configuration defaults. ChangesMCP Documentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 4
🤖 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 `@gears/mini-chat/docs/DESIGN.md`:
- Around line 2326-2334: The uniqueness rule on the server registry is not
nullable-safe because tenant_id can be NULL, so duplicate global rows can slip
through for the same source/external_id. Update the constraint definition in
DESIGN.md where the UNIQUE on (tenant_id, source, external_id) is described to
use a nullable-safe approach for global rows, such as a partial unique index for
tenant_id IS NULL or an equivalent constraint that treats NULL tenant_id as a
single global scope, while preserving tenant-scoped uniqueness for non-NULL
tenant_id.
- Around line 3454-3463: The server visibility policy in the effective
resolution rules is referencing fields that are not defined anywhere
authoritative, so the access-control model is ambiguous. Update the resolver
spec to either define `allowed_tenants` and `allowed_roles` in the `mcp_servers`
model and PRD, or remove them and rely on the existing server authorization
fields used by the resolver. Make sure the `apply server visibility policy` step
in the resolution flow points to one clear source of truth and uses consistent
terminology with the `mcp_servers` schema.
In `@gears/mini-chat/docs/PRD.md`:
- Around line 1568-1574: The MCP defaults in the PRD need to match the runtime
spec: update the documented cache TTL in the MCP configuration section so the
default aligns with the 30-second read-through cache behavior. In the MCP
settings list, adjust the `mcp.tool_cache_ttl_secs` value to the spec-consistent
default and keep the surrounding `MCP enabled`, `mcp.max_tools_per_chat`, and
`mcp.call_timeout_secs` entries unchanged.
- Around line 1012-1021: The MCP endpoint table is using a different API base
path than the rest of the PRD, so the route family is inconsistent. Update the
MCP routes and the related use-case steps together to use the same base prefix
as the rest of the public API, and keep the endpoint list, examples, and any
references to these routes in sync so clients target only one route family.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb616838-a88d-41e3-aec3-dac5dff75251
📒 Files selected for processing (2)
gears/mini-chat/docs/DESIGN.mdgears/mini-chat/docs/PRD.md
genericaccount-de
force-pushed
the
feature/mcp-servers-support-mini-chat-design-prd
branch
from
73bf265 to
a6f4ca6
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@gears/mini-chat/docs/DESIGN.md`:
- Around line 3374-3385: The tool routing map currently stores only schema_hash
in McpToolRoute, but validate_arguments() needs the full normalized JSON Schema
to perform pre-dispatch validation. Update the routing design around
McpToolRoutingMap and McpToolRoute so each route can resolve the normalized
schema source of truth, either by storing the normalized schema directly or by
replacing schema_hash with a schema-key/reference into a shared schema registry.
Make sure the validation path can retrieve the actual schema from the
route-related symbols used in the diff.
- Around line 2332-2333: The table CHECK constraint is too strict because it
permanently forces hub-discovered servers to stay disabled, which conflicts with
the approval lifecycle. Update the constraint around the server row invariant so
it no longer hard-codes `source='hub'` to `enabled=false` and
`auto_attach=false`, and keep that initial pending state in the ingest/approval
flow instead; use the `pending_approval`/enablement logic tied to the server
lifecycle rather than the table constraint.
- Around line 3331-3335: The OAGW upstream passthrough allowlist is missing
X-OAGW-Target-Host, which breaks sticky-session routing used by OagwTransport.
Update the McpService upstream registration logic that builds the
create_upstream payload so the header passthrough allowlist includes
X-OAGW-Target-Host alongside Mcp-Protocol-Version and Mcp-Session-Id, and keep
the create_route behavior unchanged. Make sure the change applies wherever the
MCP server upstream headers are assembled so multi-endpoint sessions preserve
affinity after initialize and the 404 re-init flow can repin correctly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 414427f5-e84a-4ea4-98ee-4114be105571
📒 Files selected for processing (2)
gears/mini-chat/docs/DESIGN.mdgears/mini-chat/docs/PRD.md
🚧 Files skipped from review as they are similar to previous changes (1)
- gears/mini-chat/docs/PRD.md
genericaccount-de
force-pushed
the
feature/mcp-servers-support-mini-chat-design-prd
branch
from
a6f4ca6 to
cde4a16
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 `@gears/mini-chat/docs/DESIGN.md`:
- Around line 3543-3545: The estimator cache in the MCP tool budgeting path
should not key only on schema_hash because that ignores name/description changes
and can reuse the wrong token estimate. Update the logic around
EffectiveMcpResolver and the provider cache so the full normalized serialized
tool payload is cached and compared, then use that payload for token estimation
and runtime budget enforcement before provider requests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0fb4f8a-9b0c-4545-8517-32f6818ff2d5
📒 Files selected for processing (2)
gears/mini-chat/docs/DESIGN.mdgears/mini-chat/docs/PRD.md
🚧 Files skipped from review as they are similar to previous changes (1)
- gears/mini-chat/docs/PRD.md
Signed-off-by: ga <7kozlik+github@gmail.com>
genericaccount-de
force-pushed
the
feature/mcp-servers-support-mini-chat-design-prd
branch
from
cde4a16 to
2bbb5d3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add PRD and design document for MCP servers support feature.
Summary by CodeRabbit