mcp tests: keep the real Zulip adapter out of above-port tests - #9
Merged
Merged
Conversation
The above-port unit tests (server.test.ts, server.integration.test.ts) and the disconnect-exit fixture drive the substrate through hand-rolled port fakes and the in-memory adapter only — never the real Zulip adapter. That was already true behaviourally (only server.live.test.ts constructs zulipAdapter(); bootstrap.test.ts wires it from config), but the residual coupling — the ZulipAdapter type and the UserUploadPath brand the SubstrateAdapter port carries — was duplicated across three sites that each completed an AgentComms core to the Zulip-shaped aggregate. Concentrate that completion into one seam, completeAsSubstrate in memory-substrate.ts: it owns the lone UserUploadPath brand mint and re-exports the ZulipAdapter type, so @commy/zulip now appears in exactly one test-side module. The rule (above-port unit tests use the memory adapter / port fakes only) is documented in docs/architecture.md under a new Test architecture section, making it self-enforcing rather than a convention people must remember. comms-e5vm.4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Audits the above-the-port unit tests (
server.test.ts,server.integration.test.ts, tools tests) and thedisconnect-exitfixture to confirm the real Zulip adapter does not appear in them — they drive the substrate through hand-rolled port fakes and the in-memory adapter only. Documents the rule and concentrates the residual Zulip coupling into a single seam.Audit finding
These tests were already compliant behaviourally: only
server.live.test.tsconstructszulipAdapter()(the live suite, out of scope), andbootstrap.test.tswires the real adapter from config (it tests wiring, not I/O — exempt). The above-port targets exercise onlybuildFakeAdapter/ harness overrides and the realmemoryAdapter().The residual
@commy/zulipreferences were theZulipAdaptertype (theSubstrateAdapterport is currently typed asZulipAdapter) and theUserUploadPathbrand (decodeUserUploadPathSync) — type/brand coupling, not adapter behaviour — duplicated across three sites that each completed anAgentCommscore to the Zulip-shaped aggregate.Change
packages/mcp/memory-substrate.ts(new) —completeAsSubstrate(base, overrides?), the single seam that completes anAgentCommscore (in-memory adapter or a port fake) to theZulipAdaptershape. It owns the loneUserUploadPathbrand mint and re-exports theZulipAdaptertype, so@commy/zulipnow appears in exactly one test-side module.server.test.ts×2,server.integration.test.ts,disconnect-exit.fixture.ts) now call the helper instead of inlining the Zulip-shaped members.docs/architecture.md— new Test architecture section documenting the rule (above-port unit tests use the memory adapter / port fakes only, never the real Zulip adapter), why the memory adapter is a proven contract-equivalent, and the two legitimate exceptions.Substrate-neutralising the
SubstrateAdapterport itself (so above-port code names no Zulip type at all) is epic-level and tracked separately; this keeps the boundary self-enforcing in the meantime.Closes comms-e5vm.4 (epic comms-e5vm).