Repository navigation
docs: browser sign-in for custom MCP servers, no MCP server limit, one install URL - #166
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 whose direction is right (the browser sign-in section, the cap removal, and the single install URL all match the product), but two statements still contradict the change: the page's own troubleshooting table and the sibling scope guide both invoke an MCP connection cap that this PR removes, and the new "every enabled server loads into runs for that repo" claim conflicts with the "Where connection tools load" table on the same page. Fix those two before merging; the remaining notes are optional polish.
3 findings were added inline.
Warnings
-
Cap removal leaves two pages still describing an MCP connection cap (
content/docs/configure-and-extend/connections-and-mcp.mdx)
The new "Keep the tool surface focused" section correctly states "Mogplex does not limit how many MCP servers you connect", but the same page'sTroubleshootingtable still tells readers there is one: the rowIntegration tool works globally but not in one reposays "Check for a project exclusion or a project-specific connection cap." A reader who lands there after the change is told to look for a limit that no longer exists.The same stale claim exists on another page that this PR's reasoning applies to:
content/docs/web/guides/connection-scope-and-overrides.mdxsays "The MCP limit is enforced against the resolved runnable set for that repo, not just the raw account-wide list" and frames exclusion as relief from being "overstuffed". Once #721 ships, that paragraph describes a limit production no longer applies.Suggestion: change the troubleshooting row to point at scope alone (project exclusion or a connection that is not enabled in that repo), and reword the scope-guide paragraph to talk about tool-surface size rather than an enforced limit, so all three pages agree that there is no cap.
|
|
||
| The cap protects runs from receiving an oversized or noisy tool surface. If a | ||
| repo reaches the cap, decide whether to: | ||
| Mogplex does not limit how many MCP servers you connect. Every enabled server |
There was a problem hiding this comment.
Warning: "Every enabled server ... loads into runs for that repo" contradicts the load table on the same page
The replacement text says "Every enabled server in a repo's resolved set loads into runs for that repo." Two rows of the Where connection tools load table higher on the page say otherwise: for Claude Code sandbox runs, saved MCP Servers are "Not loaded from this catalog", and for Codex sandbox runs both columns are "None yet". The same page also documents that tools set to prompt are withheld from workspace chat, the Slack agent, and native-harness runs.
The general point (more servers means a larger tool surface the agent must choose from) is right, but the absolute wording is wrong for the sandbox harnesses and for prompt-gated tools, which is exactly the audience most likely to hit an oversized tool surface.
Suggestion: scope the sentence, e.g. "Every enabled server in a repo's resolved set that the run's surface can load adds tools the agent must choose from", and let the load table carry the exceptions.
| ### Sign in to an MCP server through the browser | ||
|
|
||
| Choose **OAuth (browser sign-in)** when you add an MCP server that signs users | ||
| in with OAuth. Enter only the server's MCP URL. After you select **Add and sign |
There was a problem hiding this comment.
Suggestion: Reconcile "OAuth" and "OAuth (browser sign-in)" terminology with the auth-mode list above
Four lines above the new section, unchanged text still reads "Manual auth modes include none, API key, bearer token, basic auth, and OAuth", and content/docs/web/settings.mdx repeats the same list. The new section then names the control OAuth (browser sign-in) and says it is not a manual mode at all: you supply only the URL and Mogplex discovers and registers.
A reader scanning for the OAuth entry in the manual list will not connect it to the section that now owns the behavior. Suggestion: name the browser sign-in as its own option in that sentence (for example, "Auth modes are none, API key, bearer token, basic auth, OAuth with your own client settings, and OAuth browser sign-in"), and mirror the change in the Settings page's auth-mode bullet.
| Connections when you approve access. | ||
|
|
||
| Mogplex follows the MCP authorization spec, so you do not need to register an | ||
| app with the provider first: |
There was a problem hiding this comment.
Suggestion: New browser sign-in section would benefit from a "verify the sign-in domain" line
The section explains that the MCP server's protected-resource metadata decides which authorization server you get sent to and that Mogplex registers itself there automatically. That is correct, but it also means the destination of the sign-in page is chosen by the server, not by the user, and credentials are then typed into that page.
Existing Security notes on this page cover treating MCP config as .env-like, but nothing tells the reader to confirm the domain on the consent screen before approving. Suggestion: add one sentence, such as "Before you approve, confirm the sign-in page is the provider you expect; the server chooses which authorization server Mogplex uses."
|
Both warnings are fixed in the latest push. The troubleshooting row now points only at scope: a project exclusion, or a connection that is not enabled in that repo. The scope guide no longer describes an enforced MCP limit. It says Mogplex does not limit how many servers you connect, and that every server in a repo's resolved set adds tools the agent must choose from, so excluding a noisy global MCP keeps the tool surface small. The "Keep the tool surface focused" section no longer claims every enabled server loads into every run. It says each server that a run's surface loads adds tools, and it links to the "Where connection tools load" table, which carries the Claude Code and Codex exceptions. I also took both suggestions because they were small. The manual auth-mode sentence now reads "OAuth with your own client settings" and points to browser sign-in as the separate MCP option. The Settings page's auth-mode bullet mentions browser sign-in too. The browser sign-in section now has one line telling readers to confirm the sign-in page belongs to the provider they expect, since the server chooses the authorization server. |
There was a problem hiding this comment.
Mogplex PR Review
Status: Attention needed
Accurate, well-scoped docs corrections; approve with two non-blocking clarifications. The new install URL matches the installation page, the five-server cap is gone everywhere checked, and the new section's anchors resolve — but the new CLI-sync sentence conflicts with the page's own load table about how Integrations reach the CLI, and the sign-in section doesn't name the tab it belongs to.
4 findings were added inline.
| cache so existing local work is not blocked by a transient catalog fetch issue. | ||
|
|
||
| The catalog includes OAuth connections that you signed in to on the web. | ||
| Mogplex refreshes their tokens, so the CLI does not need its own sign-in. |
There was a problem hiding this comment.
Warning: Catalog sync claim conflicts with the page's own load table
The new sentence reads: "The catalog includes OAuth connections that you signed in to on the web. Mogplex refreshes their tokens, so the CLI does not need its own sign-in."
It sits in CLI sync, which documents GET /api/v1/mogplex/mcp/servers and ~/.mogplex/mcp-servers.remote.json, and that section is about Saved MCP Servers.
The "Where connection tools load" table gives Integrations the CLI path "MCP connections, synced as MCP config" and reserves "through catalog sync" for Saved MCP Servers, so the two passages assign different mechanisms to the same object.
The page also describes the catalog as a separate surface whose "Integration scope exclusions do not apply", which reinforces that "connections" and "catalog" are not interchangeable here.
Net effect: a reader arriving from the new sign-in section via "See CLI sync" cannot tell whether a browser-signed-in MCP connection reaches the CLI through the catalog or as synced MCP config.
Was this intentional? If browser-OAuth connections are served by /mcp/servers, the Integrations row of the load table probably needs the same note.
If they arrive as synced MCP config, say "saved servers" here rather than "connections" so the sentence does not read as an Integrations statement.
| Connection credentials are stored server-side and resolved only when the | ||
| connection is tested, listed for CLI sync, or used by a run. | ||
|
|
||
| ### Sign in to an MCP server through the browser |
There was a problem hiding this comment.
Suggestion: Name the tab in the browser sign-in instructions
The text reads "Choose OAuth (browser sign-in) when you add an MCP server that signs users in with OAuth" without saying which surface it happens on.
The page documents two different add flows: the Integrations custom connection form, which is where the auth-mode list appears immediately above at L138, and the separate MCP Servers tab.
The MCP Servers flow in "Use a saved server in web chat" is documented as Add server / Streamable HTTP / headers / Enabled, with no OAuth option, so a reader coming from that tab will look for a control that may not exist there.
Saying "In the Integrations tab, choose OAuth (browser sign-in)" makes it unambiguous.
That wording also matches content/docs/web/settings.mdx L208-L212 in this same PR, which lists "OAuth browser sign-in for MCP servers" as an Integrations-tab auth mode.
If the flow is also available for saved servers, the saved-server section needs a matching sentence.
|
|
||
| The cap protects runs from receiving an oversized or noisy tool surface. If a | ||
| repo reaches the cap, decide whether to: | ||
| Mogplex does not limit how many MCP servers you connect. Each enabled server |
There was a problem hiding this comment.
Suggestion: Removing the cap leaves the per-turn startup budget unexplained
"Mogplex does not limit how many MCP servers you connect. Each enabled server that a run's surface loads adds tools that the agent must choose from." drops the previous explanation of why the cap existed, namely that an oversized surface is what actually hurts runs.
The page already documents the mechanism that bites first: "Control waits up to eight seconds for remote MCP servers" and "A server that misses that deadline is left out", in Where connection tools load and in the Saved MCP tool troubleshooting row.
With no server cap, nothing connects the new advice to that behaviour.
A reader who adds many remote servers can therefore see tools silently disappear from a turn and have no stated reason why.
One sentence pointing at that deadline would close the gap.
| for the full operating model. | ||
|
|
||
| ## MCP limits | ||
| ## Keep the tool surface focused |
There was a problem hiding this comment.
Suggestion: Heading rename drops the #mcp-limits anchor
Renaming "MCP limits" to "Keep the tool surface focused" removes the #mcp-limits anchor.
No inbound links to it were found in web/settings.mdx, web/guides/connection-scope-and-overrides.mdx, guides/troubleshooting.mdx, support.mdx, reference/glossary.mdx, plans-and-billing.mdx, index.mdx, guides/common-use-cases.mdx, or the CLI pages.
CI runs only lint, typecheck, test, and build, so a stale anchor elsewhere would not fail the build.
Worth a repo-wide grep for mcp-limits before merge, since the rename is otherwise invisible to CI.
|
The CLI sync warning is fixed. The sign-in section now names the Integrations tab and its custom connection form, which is where the OAuth (browser sign-in) option lives. The tool-surface section now has one sentence linking it to Control's startup deadline, so a reader who adds many remote servers knows why one can drop out of a turn. I grepped the repo for |
Mogplex PR ReviewStatus: No material issues found Approve-ready docs-only change: the three corrections match the product and the rest of the docs ( Affected files
Suggestions
|
Three corrections to match the product:
https://mogplex.com/install.sh, the same URL as the installation page. It redirects toinstall.mogplex.com, so both work; the docs now show one.Ordering: merge after Mogplex/mogplex#720 and #721 are deployed, so the docs do not describe behavior production lacks.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.