docs: explain saved MCP connection tests and permissions - #138
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![mogplex[bot]](https://avatars.githubusercontent.com/in/3107927?s=60&v=4){kind=small}
There was a problem hiding this comment.
Mogplex PR Review
Status: Attention needed
Docs-only PR (3 MDX files) documenting saved-MCP connection tests and tool permissions. I compared the head file against main and checked internal consistency: the new ### Test a saved connection / ### Set tool permissions headings produce the anchors used by the two updated cross-links (#set-tool-permissions in connection-scope-and-overrides.mdx and in the "Asking before a connection's tools run" section), the six-second test deadline matches the existing six-second saved-server discovery budget, the permissions table maps cleanly onto the preserved Extra JSON example (enabled_tools/disabled_tools/default_tools_approval_mode/per-tool approval_mode), and the Control-only rule for prompted tools stays consistent with the "Where connection tools load" table and the new settings.mdx lines. No broken links, no contradictions with retained content, no security or correctness concerns. Three suggestion-level notes below: a troubleshooting cause that was dropped, a slightly self-contradictory pair of sentences about invalid permissions, and a link label that now over-promises. Also worth confirming the stated merge gating on the companion app PR, since this text describes behavior that is not yet deployed.
Suggestions
- Troubleshooting row drops the startup-deadline cause (content/docs/configure-and-extend/connections-and-mcp.mdx)
The "Saved MCP tool missing in web chat" row was rewritten from "Confirm Enabled is on, the URL is public, and the server supports Streamable HTTP. Check saved tool restrictions and the startup deadline." to "Choose Test connection. Check the result and each tool's permission. Enable the server to use its allowed tools."
The new section itself states that test results "do not monitor runtime health" and that "Chat still discovers tools at the start of each turn," and the page elsewhere documents the six-second discovery budget plus Control's eight-second wait. So a server that passes Test connection can still be dropped from a turn by the startup deadline — that cause is now unreachable from the troubleshooting table.
Suggestion: append something like "A server that misses the startup deadline is left out of that turn even when the test succeeds." to the row.
- "The app rejects invalid permissions" reads as contradicting the next sentence (content/docs/configure-and-extend/connections-and-mcp.mdx)
The lines "The app rejects invalid permissions. Correct invalid permission fields in Extra JSON. Invalid policies never become permissive defaults." are hard to parse together: if the app rejects invalid permissions, a reader will wonder where the invalid fields they must correct come from (presumably JSON written earlier by the CLI or by hand, which the permissions UI cannot represent).
Suggestion: name the source, e.g. "The permission controls reject invalid values on save. If a server already has invalid permission fields from an earlier CLI or manual edit, fix them in Extra JSON; Mogplex never falls back to a permissive default for an invalid policy."
- Retargeted link no longer covers "availability" (content/docs/web/guides/connection-scope-and-overrides.mdx)
The link now reads "See saved server tool permissions for that catalog's permissions and availability." The#set-tool-permissionssubsection covers permissions only; availability (enabled state, public HTTP requirement, which surfaces load the tools) lives in the parent#use-a-saved-server-in-web-chatsection.
Suggestion: either point back at #use-a-saved-server-in-web-chat, or keep the new anchor and trim the sentence to "...for that catalog's tool permissions."
There was a problem hiding this comment.
Mogplex PR Review
Status: Attention needed
Docs-only PR documenting saved MCP connection tests and tool-permission controls. It is accurate and internally consistent as far as I can verify from the repo: the new ### Set tool permissions heading matches the #set-tool-permissions anchor used by both the in-file link and the cross-file link in connection-scope-and-overrides.mdx, the old #use-a-saved-server-in-web-chat anchor still exists so no other pages break, the Extra JSON example and blocklist-wins semantics are preserved, the new UI table maps cleanly onto the same JSON fields, the troubleshooting rows retain the startup-deadline cause, and the settings.mdx additions agree with the detailed page. No security or correctness problems, and no credentials in the examples. Two suggestion-level clarity notes only: (1) the six-second discovery plus two-second teardown budgets in the test section sit right at Control's eight-second wait described later in the same page, so a reader may wonder whether teardown eats the chat-path margin; (2) "the permission controls reject invalid values on save" immediately followed by "fix them in Extra JSON" reads as slightly self-contradictory. Also minor: the new link text in connection-scope-and-overrides.mdx reads "saved server tool permissions ... for that catalog's tool permissions", which is repetitive and drops the previous mention of availability. Merge ordering matters here — the PR body says to hold until Mogplex/mogplex#523 merges and deploys, so this should not be auto-merged ahead of the app change.
2 findings were added inline.
Mogplex PR ReviewStatus: No material issues found Summary Documentation-only change across three MDX files. It replaces Extra JSON-only guidance for saved MCP servers with two new sections — "Test a saved connection" and "Set tool permissions" — while preserving the Extra JSON reference for advanced/CLI use. I found no security, correctness, or link-integrity problems. This reads approve-ready. What I verified
Questions / minor notes (non-blocking)
Verdict ✅ APPROVE — documentation is accurate, internally consistent, and link-safe; hold for the stated deployment ordering. Affected files:
|
This branch was successfully deployed
1 participant
Saved MCP servers now have connection tests and permission controls in the app. Update the Connections guide and Settings reference with tool counts, safe failure guidance, default and per-tool approvals, allow/block lists, and the Control-only requirement for prompted tools.
Explain that results describe the last manual test during the page visit, reset when settings change or the page reloads, and do not monitor runtime health. Preserve the Extra JSON reference for advanced configurations and CLI compatibility.
Validation: lint, typecheck, 18 tests, production build, and copy review passed.
Deployment order: hold this PR until the companion app change in Mogplex/mogplex#523 passes review, merges, and deploys. Review findings must be addressed before merge.
Review wording fixes preserve the runtime startup-deadline troubleshooting cause, clarify earlier invalid CLI/manual policies, narrow the tool-permissions link label, and document the separate six-second discovery/two-second teardown budgets.