mcp tests: keep the real Zulip adapter out of above-port tests

docs/architecture.md

@@ -36,6 +36,38 @@ The plugin README documents the tool surface, the inbound `<channel>` event
 format, the boot/identity model, and troubleshooting. Read it for anything about
 running commy *inside Claude Code* specifically.
 
+## Test architecture
+
+The ports are the seam, so tests sit on one side of them or the other:
+
+- **Port contract tests** (`@commy/testing`) pin the behaviour every adapter
+  must honour. They run against *both* the real Zulip adapter and the in-memory
+  adapter, which is how `@commy/memory` earns the right to stand in for Zulip
+  elsewhere — it is a *proven* contract-equivalent, not a hopeful mock.
+
+- **Above-the-port unit tests** (in `@commy/mcp` — `server.test.ts`,
+  `server.integration.test.ts`, tools tests) exercise the driving adapter:
+  bootstrap, identity lifecycle, the event pump, tool dispatch. **They use the
+  in-memory adapter (or a hand-rolled port fake) only — never the real Zulip
+  adapter.** A boot or tool-dispatch test that needs the real Zulip adapter to
+  pass is testing the wrong thing: the contract suite already owns Zulip's
+  behaviour, so above the port we depend on the *contract*, served by the fast
+  in-memory double. This keeps these tests realm-free, fast, and immune to
+  Zulip rate limits.
+
+  Two narrow exceptions are legitimate and are **not** real-adapter usage:
+  `bootstrap.test.ts` wires the real adapter *from config* (it tests the wiring,
+  not the adapter's I/O), and the live suite (`*.live.test.ts`) deliberately
+  hits a real realm and is excluded from default discovery.
+
+  Note: `server.test.ts` / `server.integration.test.ts` still reference
+  `@commy/zulip` for the `ZulipAdapter` *type* and the `UserUploadPath` brand
+  (`decodeUserUploadPathSync`). That is type/brand coupling, not behaviour — the
+  `SubstrateAdapter` port the driving adapter depends on is currently *typed as*
+  `ZulipAdapter`, so a provided in-memory double must be completed to that shape.
+  Substrate-neutralising that port (so above-port code names no Zulip type at
+  all) is tracked separately; it does not change the rule above.
+
 ## Substrate rationale and contracts
 
 - [Why Zulip](why-zulip.md) — why the V1 driven adapter is backed by a Zulip realm.

packages/mcp/disconnect-exit.fixture.ts

@@ -17,14 +17,14 @@
  * reproduction of the very race under test).
  */
 import { stderrLoggerLayer } from '@commy/core/logging'
-import { type MemoryAdapter, memoryAdapter } from '@commy/memory/adapter'
-import type { ZulipAdapter } from '@commy/zulip/adapter'
+import { memoryAdapter } from '@commy/memory/adapter'
 import { FetchHttpClient } from '@effect/platform'
 import { BunFileSystem, BunRuntime } from '@effect/platform-bun'
 import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'
 import { ConfigProvider, Effect, Layer, Option } from 'effect'
 import { substrateAdapterLayer } from './bootstrap.ts'
 import { CursorStoreTag } from './cursor-store.ts'
+import { completeAsSubstrate } from './memory-substrate.ts'
 import { clientDisconnect, makeProgram } from './server.ts'
 
 const inMemoryCursorStore = {
@@ -33,23 +33,20 @@ const inMemoryCursorStore = {
 }
 
 /**
- * The in-memory substrate is a `MemoryAdapter`; complete it to the
- * `ZulipAdapter` shape the program expects. `reconcileMinterSubscriptions`
- * (boot) and `close` (shutdown finalizer) are exercised, so they are real
- * no-ops; `uploadFile`/`downloadFile` are never reached without an MCP
- * client driving tools.
+ * Complete the in-memory substrate to the `ZulipAdapter` shape the program
+ * expects. `reconcileMinterSubscriptions` (boot) and `close` (shutdown
+ * finalizer) are exercised, so the helper's inert no-ops suffice;
+ * `uploadFile`/`downloadFile` are never reached without an MCP client driving
+ * tools, so they die loudly if anything calls them.
  */
-const asZulipAdapter = (adapter: Effect.Effect<MemoryAdapter>): Effect.Effect<ZulipAdapter> =>
-  Effect.map(
-    adapter,
-    (base): ZulipAdapter => ({
-      ...base,
-      reconcileMinterSubscriptions: () => Effect.succeed({ added: [], error: undefined }),
+const substrate = memoryAdapter().pipe(
+  Effect.map((base) =>
+    completeAsSubstrate(base, {
       uploadFile: () => Effect.die(new Error('disconnect-exit fixture: uploadFile unused')),
       downloadFile: () => Effect.die(new Error('disconnect-exit fixture: downloadFile unused')),
-      close: async () => {},
     }),
-  )
+  ),
+)
 
 let attached = 0
 const armedStdin = {
@@ -71,7 +68,7 @@ BunRuntime.runMain(
     Effect.provide(
       Layer.provideMerge(
         Layer.mergeAll(
-          substrateAdapterLayer(asZulipAdapter(memoryAdapter())),
+          substrateAdapterLayer(substrate),
           Layer.succeed(CursorStoreTag, inMemoryCursorStore),
           stderrLoggerLayer,
         ),

packages/mcp/memory-substrate.ts

@@ -0,0 +1,60 @@
+import type { AgentComms } from '@commy/core/ports'
+import type { ZulipAdapter } from '@commy/zulip/adapter'
+import { decodeUserUploadPathSync } from '@commy/zulip/http'
+import { Effect } from 'effect'
+
+/**
+ * The single seam where above-the-port tests touch a Zulip type or brand.
+ *
+ * Above-port unit tests (`server.test.ts`, `server.integration.test.ts`, the
+ * `disconnect-exit` fixture) drive the substrate through the in-memory adapter
+ * or a hand-rolled port fake — never the real Zulip adapter (see
+ * docs/architecture.md § Test architecture). But the `SubstrateAdapter` port
+ * those programs depend on is currently *typed as* {@link ZulipAdapter}, so any
+ * provided double must be completed from the universal {@link AgentComms} core
+ * to that Zulip-shaped aggregate: `reconcileMinterSubscriptions`,
+ * `downloadFile`, `uploadFile`, `close`. Concentrating that completion — and the
+ * lone `UserUploadPath` brand mint it needs — here keeps the rule self-enforcing:
+ * `@commy/zulip` appears in exactly one test-side module, this one.
+ *
+ * `ZulipAdapter` is re-exported so callers annotate their doubles without
+ * naming `@commy/zulip` themselves.
+ */
+export type { ZulipAdapter } from '@commy/zulip/adapter'
+
+/** The four Zulip-shaped members that complete `AgentComms` to a `ZulipAdapter`. */
+type SubstrateExtras = Pick<
+  ZulipAdapter,
+  'reconcileMinterSubscriptions' | 'downloadFile' | 'uploadFile' | 'close'
+>
+
+/**
+ * Per-member overrides. Anything omitted falls back to an inert default: a
+ * no-op reconcile report, an empty download, a stub upload result, a no-op
+ * close. Tests override only the member whose behaviour they actually assert.
+ */
+type SubstrateExtrasOverrides = Partial<SubstrateExtras>
+
+const inertExtras: SubstrateExtras = {
+  reconcileMinterSubscriptions: () => Effect.succeed({ added: [], error: undefined }),
+  downloadFile: () =>
+    Effect.succeed({ data: new Uint8Array([]), contentType: 'application/octet-stream' }),
+  uploadFile: () =>
+    Effect.succeed({ url: decodeUserUploadPathSync('/user_uploads/0/stub'), filename: 'stub' }),
+  close: async () => {},
+}
+
+/**
+ * Complete an {@link AgentComms} core (the in-memory adapter, or a hand-rolled
+ * port fake) to the `ZulipAdapter` shape the `SubstrateAdapter` port expects.
+ * Pass overrides for the Zulip-shaped members a given test asserts on (e.g. a
+ * counting `close`); the rest stay inert.
+ */
+export const completeAsSubstrate = (
+  base: AgentComms,
+  overrides: SubstrateExtrasOverrides = {},
+): ZulipAdapter => ({
+  ...base,
+  ...inertExtras,
+  ...overrides,
+})

packages/mcp/server.integration.test.ts

@@ -28,14 +28,17 @@ import {
   InboxError,
 } from '@commy/core/ports'
 import { memoryAdapter } from '@commy/memory/adapter'
-import type { ZulipAdapter } from '@commy/zulip/adapter'
-import { decodeUserUploadPathSync } from '@commy/zulip/http'
 import { Client } from '@modelcontextprotocol/sdk/client/index.js'
 import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'
 import { Deferred, Effect, FiberId, Layer, Option, Stream } from 'effect'
 import { parseEnv, substrateAdapterLayer } from './bootstrap.ts'
 import type { CursorStore } from './cursor-store.ts'
 import { CursorStoreTag } from './cursor-store.ts'
+// Above-port unit tests drive the substrate through the in-memory adapter only —
+// never the real Zulip adapter (see docs/architecture.md § Test architecture).
+// `completeAsSubstrate` is the single seam that completes it to the Zulip-shaped
+// `SubstrateAdapter` port; no Zulip type or brand is named directly here.
+import { completeAsSubstrate } from './memory-substrate.ts'
 import { makeProgram } from './server.ts'
 import { testPlatformLayer } from './test-platform.ts'
 
@@ -271,24 +274,20 @@ const buildHarness = async (overrides: AdapterOverrides = {}): Promise<Harness>
   }
   const composedDirectory: Directory = { ...base.directory, ...overrides.directoryOverrides }
 
-  const adapter: ZulipAdapter = {
-    identity: composedIdentity,
-    publisher: composedPublisher,
-    inbox: composedInbox,
-    history: composedHistory,
-    directory: composedDirectory,
-    reconcileMinterSubscriptions: () => Effect.succeed({ added: [], error: undefined }),
-    downloadFile: () =>
-      Effect.succeed({
-        data: new Uint8Array([]),
-        contentType: 'application/octet-stream',
-      }),
-    uploadFile: () =>
-      Effect.succeed({ url: decodeUserUploadPathSync('/user_uploads/0/stub'), filename: 'stub' }),
-    close: async () => {
-      closes.count += 1
+  const adapter = completeAsSubstrate(
+    {
+      identity: composedIdentity,
+      publisher: composedPublisher,
+      inbox: composedInbox,
+      history: composedHistory,
+      directory: composedDirectory,
     },
-  }
+    {
+      close: async () => {
+        closes.count += 1
+      },
+    },
+  )
 
   const [serverTransport, clientTransport] = InMemoryTransport.createLinkedPair()
   const client = new Client(

packages/mcp/server.test.ts

@@ -26,8 +26,6 @@ import {
   InboxError,
 } from '@commy/core/ports'
 import { memoryAdapter } from '@commy/memory/adapter'
-import type { ZulipAdapter } from '@commy/zulip/adapter'
-import { decodeUserUploadPathSync } from '@commy/zulip/http'
 import {
   Cause,
   Duration,
@@ -44,6 +42,13 @@ import { EnvConfigError, NotInRepo, parseEnv, substrateAdapterLayer } from './bo
 import type { CursorStore } from './cursor-store.ts'
 import { CursorStoreTag } from './cursor-store.ts'
 import type { IdentityCache } from './identity-cache.ts'
+// Above-port unit tests drive the substrate through hand-rolled port fakes and
+// the in-memory adapter only — never the real Zulip adapter (see
+// docs/architecture.md § Test architecture). `completeAsSubstrate` is the single
+// seam that completes either to the Zulip-shaped `SubstrateAdapter` port, and it
+// re-exports the `ZulipAdapter` type so this file names no `@commy/zulip` module
+// directly.
+import { completeAsSubstrate, type ZulipAdapter } from './memory-substrate.ts'
 import { clientDisconnect, forkIdleSweep, makeProgram, type ProgramParams } from './server.ts'
 import { testPlatformLayer } from './test-platform.ts'
 
@@ -178,29 +183,20 @@ const buildFakeAdapter = (
     added: [] as ReadonlyArray<ChannelName>,
     error: undefined as string | undefined,
   }
-  const adapter: ZulipAdapter = {
-    identity: identityPort,
-    publisher,
-    inbox,
-    history,
-    directory,
-    reconcileMinterSubscriptions: () =>
-      Effect.sync(() => {
-        events.push('reconcile')
-        reconcileCalls.count += 1
-        return options.reconcileReport ?? defaultReconcileReport
-      }),
-    downloadFile: () =>
-      Effect.succeed({
-        data: new Uint8Array([]),
-        contentType: 'application/octet-stream',
-      }),
-    uploadFile: () =>
-      Effect.succeed({ url: decodeUserUploadPathSync('/user_uploads/0/stub'), filename: 'stub' }),
-    close: async () => {
-      closes.count += 1
+  const adapter = completeAsSubstrate(
+    { identity: identityPort, publisher, inbox, history, directory },
+    {
+      reconcileMinterSubscriptions: () =>
+        Effect.sync(() => {
+          events.push('reconcile')
+          reconcileCalls.count += 1
+          return options.reconcileReport ?? defaultReconcileReport
+        }),
+      close: async () => {
+        closes.count += 1
+      },
     },
-  }
+  )
   return { adapter, calls: { acquired, closes, subscribed, reconcileCalls, events } }
 }
 
@@ -314,21 +310,14 @@ test('main drives a real memory adapter through acquire + env subscribe + close'
     }).pipe(Effect.flatMap(() => realSubscribe(target)))
   let closes = 0
   const oneShotEvents: MessageInbox['events'] = () => Stream.empty
-  const memoryAdapterAsZulipShape: ZulipAdapter = {
-    ...adapter,
-    inbox: { ...adapter.inbox, subscribe: spy, events: oneShotEvents },
-    reconcileMinterSubscriptions: () => Effect.succeed({ added: [], error: undefined }),
-    downloadFile: () =>
-      Effect.succeed({
-        data: new Uint8Array([]),
-        contentType: 'application/octet-stream',
-      }),
-    uploadFile: () =>
-      Effect.succeed({ url: decodeUserUploadPathSync('/user_uploads/0/stub'), filename: 'stub' }),
-    close: async () => {
-      closes += 1
+  const memoryAdapterAsZulipShape = completeAsSubstrate(
+    { ...adapter, inbox: { ...adapter.inbox, subscribe: spy, events: oneShotEvents } },
+    {
+      close: async () => {
+        closes += 1
+      },
     },
-  }
+  )
   const env = {
     ...validEnv,
     COMMY_SUBSCRIBE: 'channel:home,thread:home/payments,mentions',