fix: don't latch truncated tool surface on transient userGroups failure (#803)

`init()` set `toolsRegistered = true` before the one network call registration
depends on, so when `userGroups()` failed both of its attempts it returned the
same empty set a user with no surfaced agents gets, the DO registered without
the coding/apply/repo groups, and the latch kept it that way until eviction —
with nothing thrown, logged or recorded, and no identical retry able to recover it.

- `userGroups()` now throws after its bounded retry (both the returned `{error}`
  and the thrown-fetch path), carrying the last error. The no-token path is
  unchanged: anonymous resolving to no groups is a certainty, not a failure.
- `init()` resolves the roster BEFORE the latch and before the registration
  pipeline is installed. A refused `initialize` therefore leaves nothing latched
  or registered; partyserver resets its status when `onStart` throws and re-runs
  it on the next request, so the client's retry starts from scratch. The latch is
  set right after the lookup, not after the registrations: everything below it is
  synchronous and not re-runnable (the pipeline install wraps `tool`).
- The pinned `/mcp/i/<id>` path keeps its latch-before-lookup order on purpose:
  an unreadable pinned surface already registers one tool that says so, and "not
  your instance" is a permanent answer that must not refuse `initialize` forever.

Tests: the "gives up" case now asserts a refusal with nothing published and
nothing latched; two new cases prove the next `init()` registers the full
surface and that a network-level throw is refused the same way. Harness gains
`deferInit`. File-size pins raised with notes. Docs: workers/mcp/CLAUDE.md and
platform-docs/mcp.md describe the refusal.

Closes #803

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Author: serge-the-dev

platform-docs/mcp.md

@@ -329,6 +329,11 @@ Those three mean the session is gone — a redeploy ends the Durable Object hold
 the fix is to run `initialize` again, not to change the call. Anything else is one of the
 two honest failure shapes above.
 
+One more refusal is possible at `initialize` itself: if the platform API cannot answer which
+agents the account runs (the lookup that decides which agent-specific tools you get), the
+server fails that `initialize` rather than publishing a `tools/list` that silently omits the
+coding, apply and repo groups. Run `initialize` again; the lookup is repeated from scratch.
+
 ## Correct Runtime Flows
 
 Public trial preview:

scripts/check-file-size.mjs

@@ -746,7 +746,13 @@ const PINS = {
 	// shape unreachable (`apiCall` RETURNS a non-2xx as `{error}` rather than throwing, so the
 	// `catch` never fired for the failure it was written for) and states what the retry does NOT
 	// fix — the latch — so the next reader does not mistake a narrowed window for a closed one.
-	"workers/mcp/src/index.ts": 1016,
+	// +23 at #803: the latch the #759 note above said was NOT fixed. `userGroups` now THROWS after
+	// its bounded retry instead of returning the empty set a user with no surfaced agents also
+	// gets, and `init()` resolves it BEFORE `toolsRegistered` and before the pipeline install, so a
+	// refused `initialize` leaves nothing latched or registered and the next request starts over.
+	// Seven of the lines are code (the error carried, the throw, the pinned branch's own latch);
+	// the rest say why the order latch → pipeline → registrations must not change.
+	"workers/mcp/src/index.ts": 1039,
 	// +6 for #324: the "Runs on" machine picker had a <label> that named nothing — a label can
 	// only name one control and what it labels is a GRID of tiles — so it becomes a named group,
 	// which costs a useId, the two lines saying why, and the ignore explaining why not <fieldset>.
@@ -1660,7 +1666,8 @@ const PINS = {
 	// one that refuses to) and these three.
 	// +11 at #804: the tools.ts and coding-session.ts raises above (five and four lines of why)
 	// and these three.
-	"scripts/check-file-size.mjs": 1744,
+	// +8 at #803: the `workers/mcp/src/index.ts` raise above plus this line.
+	"scripts/check-file-size.mjs": 1751,
 };
 
 /**

workers/mcp/CLAUDE.md

@@ -210,15 +210,18 @@ tells you exactly what you changed about it.
 - **Models send JSON strings for object params.** `create_agent` / `update_agent` accept
   `z.union([z.record(z.unknown()), z.string()])` and parse a string, because rejecting it
   turns a working call into a retry loop. Do the same for any new object-shaped argument.
-- **`userGroups()` gets ONE retry, then swallows (#759).** No token → empty set
+- **`userGroups()` gets ONE retry, then THROWS (#759, #803).** No token → empty set
   immediately, which is correct and costs nothing. A failed lookup is retried once after
-  200ms and only then yields empty, i.e. no agent-specific tools for the life of that DO.
-  The retry covers both a thrown fetch AND the `{error}` object `apiCall` returns for a
-  non-2xx — the second was the reachable one, and the original `catch` could not see it.
-  What it does not fix is the LATCH: two failures still register an empty surface until the
-  DO is evicted, because re-running registration would throw `already registered`. So "my
-  tool disappeared" is still usually an auth problem rather than a registration bug — just
-  no longer a single blip away.
+  200ms; a second failure throws out of `init()`. The retry covers both a thrown fetch AND
+  the `{error}` object `apiCall` returns for a non-2xx — the second was the reachable one,
+  and the original `catch` could not see it. The throw is what closed the LATCH: `init()`
+  resolves the roster BEFORE `toolsRegistered = true` and before the pipeline is installed,
+  so a refused `initialize` leaves nothing registered and the next request starts over
+  (partyserver resets its status when `onStart` throws). Before #803 two failures registered
+  the always-on set without the gated groups and kept it until DO eviction, with no error
+  anywhere. Keep that order: latch → pipeline → registrations, all after the one network
+  call, because none of them is re-runnable. So "my tool disappeared" is now an auth or
+  subscription fact, not a registration bug.
 - **`/health`'s `tools` count comes from `src/tool-count.ts`.** It answered a hardcoded
   `41` for months while 124 were registered — and `oauth-provider.test.ts` asserted the
   41, so the test locked the wrong number in rather than catching it. `index.test.ts` now

workers/mcp/src/index.test.ts

@@ -123,6 +123,8 @@ interface HarnessOpts {
 	 * the transient failure this retry exists for.
 	 */
 	rosterFailures?: number;
+	/** Build the instance but do not call `init()` — for tests that drive it themselves (#803). */
+	deferInit?: boolean;
 }
 
 async function setup(opts: HarnessOpts = {}) {
@@ -181,7 +183,7 @@ async function setup(opts: HarnessOpts = {}) {
 	};
 	inst.server = fakeServer; // replace the real McpServer with our capturing double
 
-	await inst.init();
+	if (!opts.deferInit) await inst.init();
 
 	return {
 		inst,
@@ -229,14 +231,16 @@ describe("PagsMcp.init — tool registration", () => {
 	});
 
 	/**
-	 * A transient roster lookup must not cost this connection every gated tool (#759).
+	 * A transient roster lookup must not cost this connection every gated tool (#759), and a
+	 * persistent one must not latch a truncated surface for the life of the DO (#803).
 	 *
-	 * `init()` latches `toolsRegistered = true` and registers ONCE per Durable Object, so an empty
-	 * group set is not a small failure: the whole `coding`, `apply` and `repo` surface disappears
-	 * from `tools/list` for the life of that DO, and the user sees the always-on tools working while
-	 * their coding tools have simply vanished.
+	 * `init()` registers ONCE per Durable Object, so an empty group set is not a small failure: the
+	 * whole `coding`, `apply` and `repo` surface disappears from `tools/list`, and the user sees the
+	 * always-on tools working while their coding tools have simply vanished. #759 gave the lookup
+	 * one retry. #803 made the second failure a refusal — `init()` throws before anything is latched
+	 * or registered — so the next request starts over instead of inheriting the truncation.
 	 */
-	describe("userGroups survives a transient roster failure (#759)", () => {
+	describe("userGroups survives a transient roster failure (#759) and refuses rather than latches (#803)", () => {
 		const rosterCalls = (h: Awaited<ReturnType<typeof setup>>) =>
 			h.fetchStub.calls.filter((c) => c.url.endsWith("/v1/instances/my/instances")).length;
 
@@ -258,15 +262,46 @@ describe("PagsMcp.init — tool registration", () => {
 			expect(h.tools.has("coding_session_capture")).toBe(true);
 		});
 
-		it("gives up after the one retry rather than looping", async () => {
+		it("gives up after the one retry rather than looping — and refuses, rather than publishing a truncated surface (#803)", async () => {
 			// The bound matters as much as the retry. An unbounded loop here would hold every new
 			// connection's `initialize` open against an API that is genuinely down.
-			const h = await setup({ groups: ["coding"], rosterFailures: 99 });
+			const h = await setup({ groups: ["coding"], rosterFailures: 99, deferInit: true });
+			await expect(h.inst.init()).rejects.toThrow(/roster could not be read after 2 attempts/);
 			expect(rosterCalls(h), "exactly two attempts, never more").toBe(2);
-			// And the fallback is the pre-#759 behaviour: no gated tools, but the always-on set is
-			// intact, so the connection still works for everything that does not need a surface.
-			expect(h.tools.has("coding_session_capture")).toBe(false);
+			// The pre-#803 fallback registered the always-on set without the gated groups and latched
+			// it. Now NOTHING is registered and nothing is latched: the DO can be initialised again.
+			expect(h.tools.size, "no tool may be published from a failed roster").toBe(0);
+			expect(h.inst.toolsRegistered).toBe(false);
+		});
+
+		it("a refused initialize is not latched: the next init registers the full surface (#803)", async () => {
+			// Two 500s spend the first init's two attempts; the third lookup succeeds. This is the
+			// whole ticket: an identical retry RECOVERS, where before only DO eviction did.
+			const h = await setup({ groups: ["coding"], rosterFailures: 2, deferInit: true });
+			await expect(h.inst.init()).rejects.toThrow();
+			expect(h.tools.size).toBe(0);
+
+			await h.inst.init();
+			expect(rosterCalls(h), "two failed attempts, then the one that succeeded").toBe(3);
+			expect(h.inst.toolsRegistered).toBe(true);
+			expect(h.tools.has("coding_session_capture")).toBe(true);
 			expect(h.tools.has("list_agents")).toBe(true);
+
+			// And now the latch does the job it exists for: a third init registers nothing twice.
+			const size = h.tools.size;
+			await h.inst.init();
+			expect(h.tools.size).toBe(size);
+		});
+
+		it("a network-level throw is refused the same way as a returned 500 (#803)", async () => {
+			// `apiCall` returns a non-2xx but a failed fetch THROWS; both give-up paths must refuse.
+			const h = await setup({ groups: ["coding"], deferInit: true });
+			vi.stubGlobal("fetch", async () => {
+				throw new TypeError("network down");
+			});
+			await expect(h.inst.init()).rejects.toThrow(/network down/);
+			expect(h.tools.size).toBe(0);
+			expect(h.inst.toolsRegistered).toBe(false);
 		});
 
 		it("does not retry when there is no token — an anonymous connection is legitimately empty", async () => {

workers/mcp/src/index.ts

@@ -156,38 +156,47 @@ export class PagsMcp extends McpAgent<Env, unknown, Props> {
 	 * No token: an unauthenticated connection legitimately has no groups, and retrying would add
 	 * 200ms to every anonymous `initialize` to re-derive a certainty.
 	 *
-	 * ── What this does not fix
+	 * ── Why a second failure THROWS rather than returning empty (#803)
 	 *
-	 * The LATCH. If both attempts fail, the DO still registers with an empty set and stays that way
-	 * until it is evicted. Making registration re-runnable is a larger change — registering a tool
-	 * twice on the same server throws and cancels the MCP stream, which is the failure the latch exists
-	 * to prevent — and it is not what #759 asks for. This narrows the window; it does not close it.
+	 * "Resolved to no groups" and "could not resolve" are different answers, and only the first is
+	 * an empty set. Returning empty for both is what latched a truncated surface: `init()` could not
+	 * tell a blip from a user with no surfaced agents, registered without the gated groups, and the
+	 * latch kept it that way until the DO was evicted — silently, since nothing was thrown, logged or
+	 * recorded, and an identical retry does not recover it. So after the bounded retry this throws.
+	 * `init()` calls it BEFORE the latch and before anything is registered, so the throw fails that
+	 * one `initialize` honestly and leaves the DO able to try again on the next request: partyserver
+	 * resets its status when `onStart` throws and re-runs it on the next fetch. The no-token path is
+	 * untouched — an anonymous connection resolving to no groups is a certainty, not a failure.
 	 */
 	private async userGroups(): Promise<Set<string>> {
-	const groups = new Set<string>();
-	if (!this.userToken) return groups;
-	for (let attempt = 0; attempt < 2; attempt++) {
-		try {
-			const data = (await authedCall("/v1/instances/my/instances", this.userToken, {}, this.env)) as
-				| Array<{ capabilities?: { surfaces?: string[] } }>
-				| { instances?: Array<{ capabilities?: { surfaces?: string[] } }>; error?: string };
-			// The error object `apiCall` returns instead of throwing. Checked BEFORE the list is
-			// read, because `{error}` has no `instances` and would otherwise read as "no agents".
-			if (!Array.isArray(data) && data?.error) {
+		const groups = new Set<string>();
+		if (!this.userToken) return groups;
+		let lastError = "no response";
+		for (let attempt = 0; attempt < 2; attempt++) {
+			try {
+				const data = (await authedCall("/v1/instances/my/instances", this.userToken, {}, this.env)) as
+					| Array<{ capabilities?: { surfaces?: string[] } }>
+					| { instances?: Array<{ capabilities?: { surfaces?: string[] } }>; error?: string };
+				// The error object `apiCall` returns instead of throwing. Checked BEFORE the list is
+				// read, because `{error}` has no `instances` and would otherwise read as "no agents".
+				if (!Array.isArray(data) && data?.error) {
+					lastError = data.error;
+					if (attempt === 0) await new Promise((r) => setTimeout(r, USER_GROUPS_RETRY_MS));
+					continue;
+				}
+				const list = Array.isArray(data) ? data : (data?.instances ?? []);
+				for (const inst of list) for (const s of inst.capabilities?.surfaces ?? []) groups.add(s);
+				return groups;
+			} catch (e) {
+				// A network-level throw. Same treatment as the error object above: one more try.
+				lastError = e instanceof Error ? e.message : String(e);
 				if (attempt === 0) await new Promise((r) => setTimeout(r, USER_GROUPS_RETRY_MS));
-				continue;
 			}
-			const list = Array.isArray(data) ? data : (data?.instances ?? []);
-			for (const inst of list) for (const s of inst.capabilities?.surfaces ?? []) groups.add(s);
-			return groups;
-		} catch {
-			// A network-level throw. Same treatment as the error object above: one more try, then
-			// give up with an empty set — the pre-#759 behaviour, which is still the honest
-			// fallback once a second attempt has also failed.
-			if (attempt === 0) await new Promise((r) => setTimeout(r, USER_GROUPS_RETRY_MS));
 		}
-	}
-	return groups;
+		throw new Error(
+			`mcp: the account's agent roster could not be read after 2 attempts (${lastError}); ` +
+				"refusing to publish a tool surface missing every gated group — retry initialize (#803)",
+		);
 	}
 
 	async init() {
@@ -201,17 +210,31 @@ export class PagsMcp extends McpAgent<Env, unknown, Props> {
 		// same server throws "Tool ... is already registered", which cancels the
 		// MCP stream and makes clients hang until they time out. Register once.
 		if (this.toolsRegistered) return;
-		this.toolsRegistered = true;
 
 		// A `/mcp/i/<id>` session registers ONLY that instance's surface (#783) — nothing below.
-		if (this.props?.pinnedInstance) return this.initPinned(this.props.pinnedInstance);
+		// Latched BEFORE its lookup, unlike the platform-wide path: a pinned surface that cannot be
+		// read registers one tool that says so (`pinned.ts`), and "not your instance" is a permanent
+		// answer that must not refuse `initialize` forever.
+		if (this.props?.pinnedInstance) {
+			this.toolsRegistered = true;
+			return this.initPinned(this.props.pinnedInstance);
+		}
+
+		// Which agent-specific tool groups this user gets — scoped to their agents. Resolved BEFORE
+		// the latch and before the pipeline is installed (#803): this is the only network call
+		// registration depends on, and it throws when the roster cannot be read. Nothing is latched or
+		// registered at that point, so the failed `initialize` is retried from scratch on the next
+		// request instead of a surface missing every gated group being served for the DO's lifetime.
+		const groups = await this.userGroups();
+
+		// Latched HERE, not after the registrations: everything below is synchronous and
+		// deterministic, and the one thing a re-run could do is register a tool twice — the hang the
+		// latch exists to prevent. The pipeline install is also not re-runnable (it wraps `tool`).
+		this.toolsRegistered = true;
 
 		// Must precede every registration below — it wraps the registrar itself.
 		this.installRegistrationPipeline();
 
-		// Which agent-specific tool groups this user gets — scoped to their agents.
-		const groups = await this.userGroups();
-
 		this.server.tool(
 			"list_agents",
 			"List all published agents on ProAgentStore",