mcp integration suite reports green with every server unreachable — four `expect(true).toBe(true)` escapes

[sroussey]

What

packages/mcp/src/util/__tests__/mcp-servers.integration.test.ts ends four catch blocks with a tautology:

• :157
• :213
• :268
• :282

Each is reached through a broad "expected transport failure" pattern match, so the suite cannot distinguish "every MCP server was unreachable" from "every MCP server passed". A genuine regression inside @workglow/mcp — a broken stdio handshake, a dropped tool listing — surfaces as an exception, matches one of those patterns, and the run is green.

Standing open since 2026-08-03; packages/mcp/src has had zero source change for five consecutive review cycles, so nothing has altered the shape of the code under test in that time either.

The correct pattern already exists in this repo

packages/test/src/contract/creditExhaustedSkip.ts:180 calls ctx.skip(note) — a real vitest skip, reported as skipped rather than passed, with the environment deciding skip-vs-fail and a documented override. It also anchors its matcher so prose like "Item 402(c)" is not scavenged as a status (:44-48), which is the second half of the problem here: the patterns that route a failure into the soft-pass are broad enough to swallow assertion errors.

Proposed fix

1. Replace each expect(true).toBe(true); return; with ctx.skip(<what was unreachable>), following creditExhaustedSkip.ts.
2. Narrow the catch to transport-shaped failures only — an AssertionError from inside the try must propagate, not be classified as "server unreachable".
3. Make the skip conditional on environment the same way creditExhaustedSkip does: skipped in CI, failing locally, so the suite is falsifiable for whoever is actually changing the code.

Acceptance

Running the suite with every configured MCP server pointed at a dead endpoint reports skipped, not passed; deliberately breaking a tool-listing assertion inside one of those try blocks reports failed.

References

• packages/mcp/src/util/__tests__/mcp-servers.integration.test.ts:157,213,268,282
• packages/test/src/contract/creditExhaustedSkip.ts:44-48, :180

Found during the 2026-08-31 review of packages/; standing open since 2026-08-03.

[sroussey]

Still open at origin/main @ fb861bb. Sixth cycle, all four sites byte-identical:

$ grep -rn "expect(true).toBe(true)" packages/mcp/
packages/mcp/src/util/__tests__/mcp-servers.integration.test.ts:157:          expect(true).toBe(true);
packages/mcp/src/util/__tests__/mcp-servers.integration.test.ts:213:          expect(true).toBe(true);
packages/mcp/src/util/__tests__/mcp-servers.integration.test.ts:268:              expect(true).toBe(true);
packages/mcp/src/util/__tests__/mcp-servers.integration.test.ts:282:          expect(true).toBe(true);

packages/mcp/src did move this window — for the first time in six cycles — and the entire diff is the oxfmt reflow of one type annotation:

$ git diff 9a80450 origin/main -- packages/mcp/src
   const configServer = (task.config as Record<string, unknown>)?.server as
-    Record<string, unknown> | undefined;
+    | Record<string, unknown>
+    | undefined;

The remedy has now been in-repo for three cycles (packages/test/src/contract/creditExhaustedSkip.ts:180, a real ctx.skip(note) behind an anchored matcher).

---

Generated by Claude Code