fix(server): release only our own extension claim, and stop guessing scoped names

[chrischall]

The three nits from #218 — the post-merge review of #213 — on a fresh branch off the squashed main.

The claim cleanup could drop someone else's claim

extensionClaim exists so a second extension hello cannot interleave with the awaited pin read. Its own cleanup could defeat it: the abandon and refusal paths cleared the claim unconditionally, but the close handler may have released this socket's claim already and a newer connection may hold it by now.

A claims → A closes (handler clears A's claim) → B claims
        → A's handler resumes, clears B's claim → C passes the guard
        → B and C both reach `extensionWs = ws`

Self-healing and improbable — it needs a close inside a file read plus two reconnects — but it is the exact interleaving the claim was added to prevent, so all four sites now check === ws before releasing.

The test helper's waiters were keyed by nothing

waitForServerHello drained every waiter on each hello and dropped the ones whose mcpId didn't match, so a caller waiting on two MCPs could only settle by timing out. No current caller waits on two ids; the next person to write one would have paid for it with a 5-second mystery. Keyed by mcpId now.

serverNameFromPinFile guessed where it cannot know

_ is legal on both sides of a scope's / (SAFE_SCOPED), so @my_org/tool-mcp and @my/org_tool-mcp share the stem @my_org_tool-mcp and neither is recoverable from it. My docblock claimed the mapping was unambiguous; it isn't, and printing a guess names a package that may not exist.

It now translates only the unambiguous case (leading @, exactly one _) and shows the stem otherwise — and because a bare stem is the one string clear used to reject, clear now accepts a stem as well as a server name. Everything list prints stays usable, rather than being legible and rejected at the same time.

Testing

1232 tests, three consecutive clean runs, tsc -b across all six workspaces. New test covers the ambiguous-stem path end to end: list shows the stem, does not show a fabricated name, and clear takes the stem.

Closes #218

[claude]

Claude finished @chrischall's task in 3m 57s —— View job

---

Review of PR #219

All three follow-ups from #218 are addressed correctly:

• host.ts — all four extensionClaim = null sites (unreadable-pin, refused, dead-on-arrival, and the close handler at host.ts:475) now guard with if (extensionClaim === ws) before clearing, exactly matching the interleaving described in the PR body. Verified by reading each site: packages/server/src/host.ts:226,239,256,475.
• mock-extension.ts — helloWaiters is now a Map<mcpId, wake[]> instead of a flat array, so a hello for one mcpId no longer drains/drops waiters for a different one. Drained and deleted correctly on match.
• trust.ts — serverNameFromPinFile no longer guesses on >1 underscore (ambiguous scoped stems are shown as-is), and clear now accepts that stem via the new clearPin fallback so nothing list prints becomes unusable. New test (trust.test.ts:139-161) exercises the ambiguous-stem path end to end and matches the described behavior.

One 🟡 nit left inline: clearPin's fallback catch in trust.ts:109-120 treats any error from clearExtensionPin as "not a legal name" and retries via stem lookup. For a scoped name, a genuine filesystem error (e.g. a permission-denied unlink) gets masked into a misleading "no extension pin for X" (EXIT.USAGE) instead of surfacing the real error (EXIT.BRIDGE), because the stem lookup for a slash-form name never matches the underscore-form pinFile. Narrow, edge-case, but a real behavior change worth a second look.

Note: I could not execute npm test / tsc -b in this sandboxed review environment (commands required interactive approval unavailable here), so the PR's own testing claims (1232 tests, tsc -b across six workspaces) are unverified by me — flagging as an open question, not a finding.

Verdict: warn

[claude]

🟡 Nit: The bare catch here conflates two different failure modes from clearExtensionPin: (1) nameOrStem isn't a legal server name (safeIdentityFileBase throws synchronously), which is the case this fallback is meant to handle, and (2) a legitimate filesystem error from unlink on an otherwise-valid name (e.g. EACCES), which clearExtensionPin rethrows (extension-trust.ts:231-234).

For a scoped name like @fetchproxy/example-mcp, case 2 is silently masked: the fallback looks up pins.find(p => p.pinFile === nameOrStem), but the stored pinFile is the underscore form (@fetchproxy_example-mcp), so a slash-form nameOrStem never matches. match is undefined, and the real error (a permission problem, say) is discarded in favor of UsageError('no extension pin for @fetchproxy/example-mcp') — EXIT.USAGE with a misleading "nothing to clear" message, instead of the actual EXIT.BRIDGE "unexpected error" that would correctly point at the filesystem issue.

(For unscoped names this mostly self-corrects — the retried unlink(match.file) on the fallback path hits the same real error and it propagates uncaught — but that doesn't hold for the scoped case.)

Might be worth narrowing the catch to the "unsafe name" case specifically, e.g. by checking the error message/type from safeIdentityFileBase, rather than catching anything clearExtensionPin throws.

[github-actions]

🟡 Auto-review verdict: warn — The three targeted fixes (claim-release race in host.ts, mcpId-keyed test waiters, non-guessing scoped-name recovery in trust.ts) are all correctly implemented and covered by tests, with one narrow edge-case nit in the new clearPin fallback.
📋 Tracking follow-ups: #220