fix(extension): deliver the pair code from the value in hand, and say when it cannot

[chrischall]

The MCP learns a pair code exists only from the pair-pending frame. When that frame does not arrive, awaitSessionReady times out and reports the not-ready branch, whose hint says "sign in to the target site" — so the user is sent to check a browser session that is fine, while a live XXX-XXX sits in the popup.

That is the shape observed on chrischall/mcp-host#639: four cold starts, each showing a pair code on screen, each reporting pairCode: null at the MCP. The block that sends it had two silent ways to produce exactly that.

1. It re-read the code it already held

After writing the pending record it fetched the record back purely to get pairCode — a value already in scope as result.pairCode. A miss for any reason meant no frame and no log.

The read was never even useful. pendingKey is ${identityHash}:${scopeHash}, so anything stored at that key carries this same identity and therefore this same code (the code is sha256(mcpPub ‖ extPub), pure — no nonce, no mcpId). A stored one can only be equal — or stale, if written under a previous extension identity, in which case the in-hand value is the correct one and the stored one is wrong. Pure downside.

2. It discarded sendOnLink's result

That helper answers false on a socket that is not OPEN. An undelivered code therefore left no trace anywhere, so the MCP's misleading timeout was the only thing anyone had to go on. It now warns, naming the mcpId and the link, and says the code is in the popup — the one line that separates "the extension never asked" from "the user never approved".

What this does not claim

Whether either path is what actually swallowed resy-mcp's frames is not established — I could not observe the extension's service worker. What is established is that both existed and both produce precisely the reported symptom. One is now impossible and the other is loud.

Verification

• npx vitest run — 1451 passed (was 1448; three added). tsc -b clean.
• The happy path was untested before this and now is: an untrusted MCP gets a pair-pending carrying a six-digit code, and no ready.
• A test makes the pending-pair store read back empty and asserts the frame is sent anyway — that one fails on main, which is the regression.
• A test closes the link and asserts the warning names the mcpId.

Refs chrischall/mcp-host#639

🤖 Generated with Claude Code

https://claude.ai/code/session_011ERNU4b1U9wHdN2UWL2vhn

[claude]

Claude finished @chrischall's task in 1m 45s —— View job

---

Review complete ✅

• Read CLAUDE.md conventions
• Read PR diff (git diff origin/main...HEAD)
• Review server-hello.ts changes
• Review test coverage
• Post findings

Summary

This is a tightly-scoped, well-verified fix. I traced both claims in the PR body against the actual code:

1. In-hand pairCode vs. re-read from storage — result.pairCode is typed as a required string (hello.ts:51), and the exact same value is already written verbatim into newPendingRecord.pairCode a few lines earlier (server-hello.ts:283), so sending result.pairCode directly is type-safe and value-identical to what gets stored. I also checked applyNeedsPairRecord (pending-records.ts:138-171): the dedup path (kind === 'pair' already present) never overwrites the existing entry's pairCode, so a stored value can only ever be equal (same pendingKey = same identity = same code) or stale if the entry belongs to a superseded identity — confirming the PR's "pure downside" claim precisely.
2. sendOnLink return value — links.ts:189-193 confirms it returns false only when link.ws is falsy or not WebSocket.OPEN; the new if (!delivered) branch correctly surfaces that with a console.warn, which is fine here since this runs in the extension service worker, not an MCP stdio process (the repo's console.log/debug/info stdio-corruption rule in CLAUDE.md is scoped to MCP processes only).
3. The scope-update branch (kind === 'scope-update', lines ~155-237) is untouched by this PR and never had a pairCode field or storage re-read to begin with — no related inconsistency introduced.

Test coverage — the three new tests in multi-link.test.ts are well-constructed:

• New MCP ids (...cccc, ...dddd, ...eeee) don't collide with any of the file's existing scripted-MCP ids.
• The storage-miss test stubs chrome.storage.local.get to drop only the pendingPair key, which is a faithful reproduction of the old bug's exact failure mode.
• The "link closed before delivery" test sets readyState = 3 synchronously after message() returns (before the handler's awaits resolve), which matches the existing FakeSocket.close()/remoteClose() pattern already used elsewhere in the file (e.g. line 493 mirrors line 72/89).

I was not able to execute npx vitest run or git fetch in this sandboxed review session (both required approval that wasn't available), so I did not attempt to independently confirm the "1451 passed" / "tsc -b clean" claims from the PR body — flagging that as an open question rather than a finding, per review policy. Everything I could statically verify checks out.

No correctness, convention, or security issues found.

Verdict: pass
· Branch

[github-actions]

✅ Auto-review verdict: pass — The fix correctly replaces a redundant, silently-fallible storage re-read with the already-in-scope pairCode value, and adds a loud warning when sendOnLink fails to deliver; both claims were verified against the code and the new tests are sound.