diff --git a/docs/architecture.md b/docs/architecture.md index b22328b..e800980 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -36,6 +36,38 @@ The plugin README documents the tool surface, the inbound `` 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. diff --git a/packages/mcp/disconnect-exit.fixture.ts b/packages/mcp/disconnect-exit.fixture.ts index c9603da..8c8bec2 100644 --- a/packages/mcp/disconnect-exit.fixture.ts +++ b/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): Effect.Effect => - 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, ), diff --git a/packages/mcp/memory-substrate.ts b/packages/mcp/memory-substrate.ts new file mode 100644 index 0000000..f8a53c6 --- /dev/null +++ b/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 + +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, +}) diff --git a/packages/mcp/server.integration.test.ts b/packages/mcp/server.integration.test.ts index 0799a34..ae36913 100644 --- a/packages/mcp/server.integration.test.ts +++ b/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 } 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( diff --git a/packages/mcp/server.test.ts b/packages/mcp/server.test.ts index 6041948..e2cebe5 100644 --- a/packages/mcp/server.test.ts +++ b/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, 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',