fix(mcp): reproduce #86 — truthful logout, and a sign-in name that resolves - #87
Draft
christianhpoe wants to merge 2 commits into
Draft
fix(mcp): reproduce #86 — truthful logout, and a sign-in name that resolves#87christianhpoe wants to merge 2 commits into
christianhpoe wants to merge 2 commits into
Conversation
…rowser Clicking "Log in" on an MCP connector often did nothing visible: no browser window, no password prompt, no error anyone could act on. Issue #86 reports it against a custom connector; it also affects eleven catalog entries, Microsoft SharePoint and Google Workspace among them. The connector's name was being derived three different ways. Connect slugifies the display name and writes that key into opencode.jsonc. Sign-in instead ran the *display* name through validateMcpServerName, which exists to reject strings that are not already valid server names — so every name containing a space threw "server_name must be alphanumeric" before the authorization URL was ever requested. The throw was caught and shown as a generic failure, which is why the symptom reads as the browser refusing to open rather than as a bug. Names with an underscore failed differently: sign-in asked the engine for "iron-crow" while the server was registered as "iron_crow", so the engine reported it missing. The derivation now lives in one dependency-free module, and sign-in composes it with the validator rather than substituting one for the other: a display name is slugified, and only the resulting key is validated. authorizeMcp also pins the opencode.jsonc key onto the entry it hands the modal, so a catalog entry's display name can no longer stand in for the server's identity, and it matches catalog entries on that key instead of on a slugified title. Verified against the pinned opencode engine and the repo's mock OAuth server: a connector named "Iron Crow" now returns an authorization URL and completes the flow to connected, where before it failed without contacting the engine. Closes #86 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EwVKKSbK7HTqnjeWmtPdK
Reproduced from issue #86 against the pinned engine. A remote MCP server that accepts the unauthenticated handshake and rejects only tools/call is reported by the engine as plain `connected`, so the connector reads as Ready without anyone signing in. "Log out" then clears a credential that never existed, the badge drops to Paused, and the next launch silently restores Ready. From the outside that is indistinguishable from a stale session surviving a logout, which is what the issue describes. Verified with a control arm: restarting the engine without logging out produces the same Ready state, so the logout contributes nothing. The engine offers no way to ask whether a server is authenticated — McpStatusConnected is bare and no endpoint reports stored credentials. The signal is to reconnect once the credentials are gone, which is what the next launch does anyway: a server that needed them comes back needs_auth, one that never did comes back connected. Confirmed both ways against the real engine. Probing during logout also leaves the badge showing the truth immediately rather than a Paused state that flips to Ready on the next start. Logout now says what happened instead of always claiming success. A first attempt at this watched for a spontaneous reconnect instead of asking for one; it passed its unit tests and did nothing at all against a real engine, because the server stays disabled until something reconnects it. Refs #86 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EwVKKSbK7HTqnjeWmtPdK
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
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.
Summary
Issue #86 reproduces in full against the pinned engine (v1.17.18). Two defects, both fixed here.
Why
The reproduction
The reporter's server accepts the unauthenticated handshake and rejects only
tools/call, with a 403 and an HTML body. All four reported symptoms follow from that, and all four reproduce:connected, auth store emptyconnected, never opens the modalconnected~2s after restartArm B is the important one: the restart alone is sufficient, so the logout contributes nothing. Nothing stale is being restored — there was never anything to restore. The issue's own diagnosis ("reloading a stale or invalid token on startup") is not what happens; the auth store is empty throughout.
That makes the LegalWork defect narrower and more concrete than the title suggests:
logoutMcpAuthreported success unconditionally. That false confirmation is what makes this read as a stale-session bug.Detecting it
The engine offers no way to ask whether a server is authenticated —
McpStatusConnectedis bare, and no endpoint reports stored credentials. The signal is to reconnect once the credentials are gone, which is exactly what the next launch does, only now instead of later:disabledconnected→ logout was a no-opdisabledneeds_auth→ logout workedProbing during logout also leaves the badge showing the truth immediately, instead of a Paused state that quietly flips to Ready on the next start — the very transition the reporter experienced.
The sign-in name
Connect slugifies the display name and writes that key into
opencode.jsonc. Sign-in ran the display name throughvalidateMcpServerName, whose job is to reject strings that are not already valid server names. Any character outside[A-Za-z0-9_-]threwserver_name must be alphanumeric with '-' or '_', caught and shown as a generic failure — which is why it reads as the browser refusing to open. Nine remote catalog connectors are affected, path-dependent:sharepointoauth: true)google-cloud-storageoauth: true)highqoauth: true)google-workspacecbinsightslegaldatahuntersolveintelligencetechgcdnb-riskLegalWork UI Control and Computer Use trip the same validator but are
type: local, andauthorizeMcpalready refuses non-remote entries, so they never reach sign-in and are not counted.This matters for #86 because sign-in is the remedy for an anonymously-connected server: the "Log in" button is the only way to force authentication. Whether the reporter hit it depends on their connector name, which the issue does not state — see below.
Issue
What is and is not established
disconnect+auth removethe entry is gone frommcp-auth.jsonand does not come back.iron-crow_predict_101_rejection, implying a config key ofiron-crow. That comes from a display name of either "Iron Crow" (breaks) or "iron-crow" (does not). "IronCrow" would have producedironcrow. Circumstantial; not settled. Confirming the name as typed would close it.connectedfor an anonymous handshake is engine behaviour and is not changed here. This PR makes LegalWork stop misreporting it.Scope
apps/app/src/app/mcp-auth-state.ts:detectReconnectWithoutAuth, the reconnect probe.apps/app/src/app/mcp-identity.ts: one derivation for a connector's server name, replacing three.store.tslogoutMcpAuthreports the probe's verdict instead of unconditional success;authorizeMcpmatches catalog entries on their identity key and carries the config key to the modal.mcp-auth-modal.tsxresolves the server name from the entry rather than re-slugifying its display name.constants.ts/mcp.tsdelegate to the shared identity module.mcp.logout_no_credentialsstring (en; other locales fall back, as with existing keys).mcp-auth-identity.test.ts(7),mcp-logout-truthfulness.test.ts(12).Out of scope
Testing
Ran
bun test tests/(apps/app)pnpm typecheck(apps/app)node scripts/i18n-audit.mjs --ciscripts/mock-oauth-mcp-server.mjsResult
Manual verification
Both fixes were validated end to end against the real engine, not just unit tests.
Logout, two arms:
Sign-in, a connector named
Iron Crow:A note on process, since it shaped the result: the first logout implementation watched for a spontaneous reconnect. It passed twelve unit tests and did nothing whatsoever against a real engine, because the server stays
disableduntil something reconnects it. The end-to-end arms above are what caught that, and are why the probe now asks for the reconnect rather than waiting for one.Evidence
tools/call).Risk
POST /mcp/{name}/connectduring logout. On a server that genuinely required credentials this lands onneeds_auth, which is the correct post-logout state and more accurate than the previousdisabled. A failed or unreadable probe falls back to the existing message, so an uncertain answer never accuses a working logout.opencode.jsoncare unchanged.getMcpIdentityKeynow preserves underscores to match what connect writes, correcting a latent mismatch; no catalog entry uses one.Rollback