feat(server): lazy bridge connect — listen() loads identity only

[chrischall]

Summary

Defers the fetchproxy bridge connection (role election + WS bind/dial) from FetchproxyServer.listen() to the first verb call. listen() is now just a cheap identity load + mcpId generation.

Why

Today: every configured-but-unused MCP under Claude Desktop claims bridge resources at MCP-client startup. When several MCPs start in parallel they all race to bind port 37149 — only one wins host, the rest dial as peers. While the race is in flight, the browser extension's reconnect cycle produces a stream of ERR_CONNECTION_REFUSED errors in DevTools because it may reach the port before any MCP has bound it. And MCPs that never have a tool called still hold a peer WebSocket open to the host for the whole session.

After this PR: nothing touches the network until something actually needs it. Boot is quiet, idle MCPs cost zero socket descriptors, and the first-call latency in exchange is a one-time elect + handshake (sub-second on a warm extension).

API

• listen() — unchanged signature, behavior reduced to identity load. MCP authors don't need to change anything to get the new behavior.
• connect() — new public method. Idempotent wrapper around the internal ensureConnected. Useful for callers that want to surface the role / connection outcome at boot (most production MCPs won't need this; test harnesses do).
• Each verb method (fetch, readCookies, readLocalStorage, readSessionStorage, captureRequestHeader, readIndexedDb) now awaits ensureConnected() at the top. The existing throwIfPendingPair / pair-code error path runs unchanged once the handle is up.
• Concurrent first-callers share a connectingPromise mutex so two parallel verb calls don't race the port bind (mutex test included).

Tests

Two new in ws-server.test.ts:

• listen() does not bind the port — connection is deferred — proves the role-null gap after listen() and that another instance can take the host slot in that window.
• concurrent first verb calls share one connection election — fires two connect() calls in parallel and asserts the mutex prevents a double election.

Existing tests adapted: integration tests added await X.connect() after each await X.listen() so their mock extensions can dial a bound port. This verifies the new explicit eager path is functionally equivalent to the pre-0.5.3 implicit one.

Total: 486 tests passing (was 484, +2 new).

Test plan

• npm test — 486/486 pass
• npm run typecheck — clean
• npm run build --workspaces --if-present — clean
• Manual: launch Claude Desktop with several MCPs configured but call none — confirm none of the MCPs are connected to port 37149 (lsof -iTCP:37149). Then call one tool and confirm exactly that MCP is now bound/connected.

🤖 Generated with Claude Code

[claude]

Claude finished @chrischall's task in 4m 10s —— View job

---

PR Review: feat(server): lazy bridge connect

• Read CLAUDE.md conventions
• Read PR diff and changed files
• Review for correctness, edge cases, security
• Check test coverage of new code paths
• Post inline comments for findings
• Post summary verdict

---

Summary

The lazy-connect design is sound and the implementation is clean. The connectingPromise mutex correctly serialises concurrent first-callers — the concurrent test in ws-server.test.ts exercises this well. Existing integration tests are adapted correctly (all add await X.connect() after await X.listen() so their mock extensions can dial a bound port). No correctness bugs or security issues found.

Two 🟡 nits, no 🔴 important findings.

Findings

🟡 Stale docstrings (flagged inline on line 314) — the class-level JSDoc still says "On listen(), the server races the configured port" and the role field doc says "Set after listen() succeeds." Both describe the pre-0.5.3 eager behavior and should be updated to reflect that the port race now happens lazily.

🟡 close() + in-flight doConnect() race (flagged inline on line 453) — if a verb call triggers doConnect() concurrently with close(), close() exits before the handle is established and never closes it. doConnect() then writes this.hostHandle after close() has returned, leaking the socket. Fixing it requires close() to await and clear the in-flight connectingPromise. Narrow race, not blocking.

Verdict: warn — no blocking issues, two nits.

[claude]

🟡 Nit — close() called while doConnect() is in flight can leak a handle

The finally correctly clears connectingPromise so a failed connection is retryable, but close() doesn't set connectingPromise = null before it exits. If a verb call triggers doConnect() and close() races it, close() will see hostHandle == null (the handle hasn't been set yet), so hostHandle.close() never runs. doConnect() then finishes and writes this.hostHandle = <live handle> after close() has already returned — the handle leaks.

One simple fix: have close() also clear connectingPromise (and optionally await it first so any in-flight election gets shut down cleanly):

async close(): Promise<void> {
  this.rejectAllPending();
  // Wait for any in-flight election before tearing down handles.
  if (this.connectingPromise) {
    await this.connectingPromise.catch(() => {});
  }
  if (this.hostHandle) await this.hostHandle.close();
  if (this.peerHandle) this.peerHandle.close();
  this.hostHandle = null;
  this.peerHandle = null;
  this.role = null;
  this.connectingPromise = null;
}

This is a narrow race (requires concurrent close() + verb call) so not blocking, but worth a follow-up.

[claude]

🟡 Nit — two stale docstrings elsewhere in the class

Not in the diff but worth flagging here since this PR introduced the semantic change that makes them wrong:

1. Class-level JSDoc (line ~260): "On listen(), the server races the configured port…" — this is the old eager behavior. Should say the port race happens lazily on the first verb call (or explicit connect()).
2. role field JSDoc (line ~274): "Set after listen() succeeds. Null while not listening." — role is now null after listen() and only set after connect() / first verb call.

Fix this →

[claude]

Claude finished @chrischall's task in 2m 34s —— View job

---

Claude finished re-reviewing PR #51 nit fixes —

---

Re-review: address PR #51 nits

• Read CLAUDE.md conventions
• Read updated ws-server.ts
• Verify close()/doConnect() race fix
• Verify stale docstrings fix
• Post verdict

---

Summary

The close() race fix is correct: it awaits the in-flight connectingPromise.catch(...) before tearing down handles, so a doConnect() that completes mid-close() will have its handle picked up and properly closed. connectingPromise = null is also cleared at the end of close() (the ensureConnected finally will already have cleared it, but the redundant clear is harmless). The role field docstring is correctly updated.

One new 🟡 nit: the docstring fix accidentally inserted a bare /** on line 257, immediately before the /** that starts the class-level JSDoc on line 258. JavaScript block comments don't nest, so the outer /** (line 257) swallows the entire description — FetchproxyServer ends up with no JSDoc and IDE hover docs are silently lost. Flagged inline.

Findings

🟡 Nit — stray /** on line 257 strips FetchproxyServer of its class JSDoc (flagged inline). Remove the extra /** so the description is re-attached to the class.

Verdict: warn — close() race and role docstring are fixed correctly; one minor regression in the docstring edit itself.

[claude]

🟡 Nit — stray /** strips the class of its JSDoc

This line was added by the docstring fix but shouldn't be here. JavaScript block comments don't nest: the /** on line 257 opens a block comment, and the */ on line 274 closes it — which means the content from lines 258–274 (the entire updated class description) is inside that outer comment, not attached to FetchproxyServer. The class ends up with no JSDoc, so IDE hover docs (VS Code, etc.) won't show the description.

Suggested change

	/**
	/**
	* The MCP-facing handle for the fetchproxy bridge.

Simply remove the extra /** on line 257 (i.e., collapse the two back into one /** on line 258).

[claude]

Claude finished @chrischall's task in 2m 29s —— View job

---

Re-review: lazy bridge connect (post-nit-fix sync)

• Read CLAUDE.md conventions
• Read updated ws-server.ts — all three previous nits resolved
• Read test files — new close() race test added, coverage complete
• Post verdict

---

Summary

All three findings from the previous review cycles are fully resolved in this sync:

🟡 Stray /** (bd4947e) — Removed. The class-level JSDoc is now a single /** block on line 257; FetchproxyServer has its description back in IDEs. ✓

🟡 Stale docstrings (f6f8582) — Both updated correctly. The class-level JSDoc now reads "listen() loads identity and reserves nothing. The first verb call (or an explicit connect()) races the configured port…" and the role field says "null until the first verb call (or an explicit connect()) — listen() no longer triggers the role election as of 0.5.3+." ✓

🟡 close() + in-flight doConnect() race (f6f8582) — Fixed correctly. close() now awaits this.connectingPromise.catch(() => undefined) before checking handles, so any in-flight doConnect() completes (writing this.hostHandle) before the teardown reads it. A new unit test (close() awaits an in-flight connect so no handle leaks, port 41059) covers this path directly by asserting hostHandle, peerHandle, and connectingPromise are all null post-close regardless of race order. ✓

No new issues found. Verdict: pass.