feat(arch): gate adapter containment and prove restart rebuilds from checkpoints - #962
Closed
chughtapan wants to merge 4 commits into
Closed
Conversation
…bin map Only "clean-slate cannot import production" was enforced. Three rules now hold the shape the cutover produced: adapters may reach @moltzap/client only through its published adapter-facing subpaths, no source may import daemon-side machinery by name, and the client's export and bin maps are pinned. The subpath rule alone was insufficient — channel-base re-exported MoltZapChannelCore and ChannelService, so any adapter could have taken the long way round. That barrel now exports the seven presentation names its consumers actually use; both classes have no consumer outside packages/client. The rules scan shipped sources only. Test scaffolding legitimately drives a peer agent and registers fixtures, and none of it reaches a user. The three client-side conformance configs matched no tests and ran passWithNoTests, so four CI steps reported green for nothing. Deleted rather than left standing in for an unmet criterion; the deferral is recorded in #926.
The socket support module went with the Unix RPC plane; its JSDoc header outlived it and now labels the wrong export.
`20260801-harness-client-owns-runtime-context` states the client "stores stable per-conversation presentation checkpoints locally. After restart it uses search and history reads to rebuild context from those positions", "presents context at most once during normal operation", and that "history reads rebuild context only and never recreate reply authority". Nothing tested any of it. Three client lifetimes against one slot. Each spawns its own daemon; only the checkpoint directory survives between them. The second lifetime must not re-present the content the first already carried, and its bound reply must still land in the live conversation. Deleting the checkpoint directory restores the full re-read, so the narrowing is the checkpoints and not a recency window. A turn's reply is bound to the MCP client that produced it, so the reply round trip runs inside the lifetime that owns that client rather than escaping its scope.
chughtapan
force-pushed
the
impl/architecture-rules-and-restart
branch
from
August 5, 2026 21:30
b8d15a6 to
79a50a6
Compare
Owner
Author
|
Superseded by #974, which collapses this stack into one PR against |
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
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.
Step 13, complete: the architecture rules and the restart proof.
Architecture rules
Before this, exactly one rule was enforced: "clean-slate cannot import production." Three now hold the shape the cutover produced.
Adapter containment. The two adapter packages may reach
@moltzap/clientonly through its published adapter-facing subpaths, and no shipped source may import daemon-side machinery by name (MoltZapService,MoltZapChannelCore,MoltZapAgentClient,ChannelService,acquireMoltzapd,runMoltzapd).The subpath rule alone was insufficient, as the plan predicted:
channel-base/index.tsre-exportedMoltZapChannelCoreandChannelService, so an adapter could take the long way round through a legal subpath. That barrel now exports the seven presentation names its consumers actually use. Both classes have zero consumers outsidepackages/client.assertExportMap+assertBinMappin the client's seven export subpaths and its singlemoltzapdbin.Rules scan shipped sources only — test scaffolding legitimately drives a peer agent and registers fixtures, and none of it reaches a user. Two real files hit the rule on the first run and this is why; the exclusion is stated in the script, not silently allowlisted.
Three conformance configs deleted.
client,openclaw-channel,nanoclaw-channeleach ranpassWithNoTests: trueagainst a directory with zero files — four CI steps green for nothing. The server's suite is real and untouched.Non-vacuity probe — five for five
MoltZapServicecontext-log.ts:1@moltzap/client/dist/moltzapd.jsmoltzapbin keyexpected moltzapd, got moltzap, moltzapd./smuggleexport subpathwould pass vacuouslyRestart proof
Written against what the record actually says, not a paraphrase.
20260801-harness-client-owns-runtime-context:Verified against the implementation first:
projectTurnsetsreply: turn.replyfrom the live MCP_metaroute whilereconstructHarnessContextproduces only context, andsourceConversationIdsexcludes the target conversation. Both guarantees hold structurally; nothing tested either.Three client lifetimes against one slot, each spawning its own daemon, only the checkpoint directory surviving between them:
Probes
drainConversationReadignores prior checkpointsexpected […] to not include 'source content before the restart'harnessClientForProfileuseslayerMemoryinstead oflayerFileSystemThe second probe matters: it is the one that distinguishes durable across process restart from within one lifetime. My first attempt at it silently no-op'd — the
assertchecked that a substring existed rather than that the replacement applied, so the test "passed" against unmodified code. Re-run with a post-condition on the edit, it fails correctly.Two corrections
Ledger row 5 is withdrawn. It claimed
check-architecture-boundaries.jsis stale for naming@moltzap/v2-endpointwhile "v2 renamed to-harness". Verified: v2 renamedendpoint→harnesson its own branch and updated its own copy of the script. Main still hasv2/endpoint,docs/architecture/components.mdon main agrees, and main's table matches main's tree. Expected pre-cutover divergence per20260729-v2-authority-lives-with-v2, not staleness.The plan's metric was a proxy. It asked for "strictly fewer
messagesReadcalls than a cold run." The ADR's actual claim is at-most-once presentation; with pagination a smaller delta can still be one call, so the test measures content presented into the turn. Stronger and directly observable.Authority gap, stated plainly
docs/architecture/harness-implementation-slate.mddoes not exist on the main line, andcheckpointappears in no main-resident ADR. The record this test pins is v2-owned, and it says so itself: "Production adoption ismain-owned." So the test pins behavior the production implementation was built to, with no main-resident record binding it. That is step 5's hole, not something this PR can close.Verification
nx run workspace:precommit— exit 0node scripts/check-architecture-boundaries.js— OK, 470 v1 sources, 12 adapter sources, 6 v2 packages, 64 v2 sourcesEnrichedSenderas newly dead once the barrel narrowed; made internal, sincechannel-core.tsstill uses it