test: close wire-surface overclaim and make the closure fail closed (#1432)

Addresses both review P1s on #1717.

P1 — the manifest materially overclaimed ADR 0006 coverage. It quoted all four
bullets while digesting only the payload TYPES, so the producer and consumer
seams could break a skewed peer without moving a listed digest. Now listed on
both sides of every boundary: JSON-RPC method sets and the projections that
turn each method's params into a DaemonRequest, createRpcError/sendJson/
writeRpcResponseEnvelope, resolveToken and the auth-hook types, upload
preflight/finalize/308 handlers and the resumable ticket shape, artifact route
and download/inventory framing, REST error mapping, and the client's own
payload builder, lease-method mapping, response parser and error projection.
57 -> 117 declarations.

What stays out is now named rather than implied: createDaemonHttpServer's
dispatch wiring and the /health and /rpc literals inside it. Everything it
dispatches WITH is digested individually, and a moved route 404s at connect
time rather than misparsing — the loud failure, not the silent one.

P1 — imported and re-exported payload shapes escaped the closure.
declarationHomes() scanned only the manifest's own files and the walk
continued silently when a name could not be placed, so a listed type could
gain foo?: ImportedShape from a new module and stay green. Resolution is now
explicit and fails closed: relative imports, workspace specifiers (through the
owning package's own exports map, so a re-pointed export cannot drop a type),
and facade re-export chains. Every referenced name must land on a listed
declaration, a waiver with a written reason, a declared external module, or the
TS/Node global set. Fixed two extractor blind spots the walk exposed: a
declaration's own generic parameters and `as const` were being reported as
references.

Planted-red proofs (wire-mutations.test.ts): 13 cases independently mutate
method naming, response serialization, response parsing, auth projection,
upload ticket shape, 308 framing, artifact framing, REST error mapping, and
progress framing, each asserting the digest moves; 3 probes prove the closure
really reaches across a package boundary, a facade re-export, and a plain
relative import. Mutations apply inside the declaration's own span — a
whole-file replace silently hit a sibling sharing the substring, which is how
the first draft of one case passed vacuously.

The largest waiver pair (InternalRequestOptions, CommandFlags) rests on ADR
0006's own additive rule: they reach the peer inside DaemonRequest's untyped
flags/input bags, and the decision says a new flag needs no bump. Digesting
them would fire the gate on every new CLI flag and train reviewers to
rubber-stamp acks.

Author: claude

test/wire-compat/README.md

@@ -23,13 +23,26 @@ interoperate" — `agent-device proxy`, cloud/limrun providers, and a remote mac
 skewed on purpose, and package version is explicitly *not* the compatibility gate there. The one
 place version skew is designed in was the one place with no gate.
 
-## The two gates
+## The three gates
 
 | | Runs | Answers |
 | --- | --- | --- |
-| `wire-compat.test.ts` (`unit-core`) | every PR, offline, shallow-safe | does the ledger still match the source it describes? |
+| `wire-compat.test.ts` (`unit-core`) | every PR, offline, shallow-safe | does the ledger still match the source it describes, and is the manifest closed? |
+| `wire-mutations.test.ts` (`unit-core`) | every PR | does the gate actually **catch** each class of wire break? |
 | `pnpm check:daemon-wire-compat` (`Released-Surface Compatibility`) | own `fetch-depth: 0` job | did the drift **since the last released tag** come with a bump or an ack? |
 
+The mutation lane exists because the first two answer "the manifest describes
+today's source", never "the manifest describes the *right* source". A gate can
+list 117 declarations, pass every check, and still miss the seam that breaks a
+skewed peer — which is exactly what review found in the first version of this
+directory. Each case there mutates one real listed declaration the way a real
+break would (method renamed, envelope unframed, auth header dropped, upload
+ticket field renamed, 308 downgraded, artifact route narrowed) and asserts the
+digest moves. Two guards keep them honest: the mutation is applied **inside the
+declaration's own span** so it cannot silently hit a sibling that shares the
+substring, and the unmutated digest must equal the ledger's so the case is
+pinned to the declaration the gate really watches.
+
 The split is forced, not stylistic. From a single commit a bumped ledger and an unbumped one are
 both just an edited file, so only a baseline read out of git can tell them apart — the same reason
 the replay-compat corpus splits its provenance verifier out of the unit lane.
@@ -75,20 +88,46 @@ bullet is only partly digestible the group carries an `uncovered` note saying wh
 reviewer-owned and why — a gate that implies coverage it does not provide is worse than one that
 admits the gap (AGENTS.md, "a registry claim is not a semantic check").
 
-The one such gap today: the `/health` and `/rpc` path literals sit inside `http-server.ts` request
-handlers, whose bodies churn for reasons that are not protocol changes. They stay reviewer-owned
-because their failure mode is the loud one — a moved route 404s at connect time, before any payload
-is exchanged. Everything digested here can misparse *silently*, which is the whole point.
+**Both sides of every boundary are listed.** The first version of this directory digested only the
+payload *types* while quoting all four bullets, which review correctly called an overclaim: method
+sets, response serialization, auth projection, upload ticket and 308 framing, artifact framing, and
+the client's own parsers could all break a skewed peer without moving a listed digest. A
+client-only change breaks an older daemon just as surely as the reverse, so producer and consumer
+seams are both here.
+
+The one remaining gap: `createDaemonHttpServer`, the 200-line dispatcher, and the `/health` and
+`/rpc` path literals inside it. Everything it dispatches *with* is digested individually, so what is
+uncovered is the wiring — and its failure mode is the loud one, a 404 at connect time before any
+payload is exchanged. Everything digested here can misparse *silently*, which is the whole point.
 
 Digests ignore comments and formatting, so reflowing a type or rewriting the prose above a field
 does not move them; only the declaration's tokens do.
 
-## Adding to the surface
+## The closure, and why it fails closed
+
+`wire-compat.test.ts` walks each listed declaration's AST and resolves every type name it
+references — through relative imports, workspace specifiers (via the owning package's own `exports`
+map), and façade re-export chains. Every name must land somewhere someone wrote down:
 
-The manifest's closure is checked, not asserted: `wire-compat.test.ts` walks each digested
-declaration's AST and fails if it references a type declared in a manifest file that the manifest
-itself omits. So adding `foo?: NewShape` to a wire type tells you to list `NewShape` rather than
-letting the field that decides what the peer parses sit outside the gate.
+- a **listed** declaration, or
+- a **waiver** in `closure-policy.ts` — a repo declaration that is genuinely not payload, with the
+  reason it cannot change what a peer parses, or
+- a **declared external module** (`node:http`, `undici`) with no declaration site here, or
+- the TypeScript/Node global set.
+
+Anything else fails. The earlier version skipped names it could not place, which made the whole
+claim hollow: a listed type could grow `foo?: ImportedShape` from an unlisted module and stay green.
+Three probes in `wire-mutations.test.ts` prove the walk really reaches across each boundary form —
+drop a listed declaration from the claimed set and the closure must report it.
+
+Challenge the waivers first when reviewing this directory; they are where coverage is traded away.
+The largest pair (`InternalRequestOptions`, `CommandFlags`) rests on ADR 0006's own additive rule:
+those reach the peer inside `DaemonRequest`'s untyped `flags`/`input` bags, and the decision says a
+new flag needs no bump. If a flag ever becomes a typed field, list the type instead of widening the
+waiver.
+
+## Adding to the surface
 
-To add a declaration by hand, put it in the group whose ADR 0006 bullet it serves and paste its
-digest from the failure message.
+Put the declaration in the group whose ADR 0006 bullet it serves and paste its digest from the
+failure message. If it introduces a new break class, add a case to `wire-mutations.test.ts` — a
+listed declaration with no proof that mutating it fails is a claim, not a gate.

test/wire-compat/closure-policy.ts

@@ -0,0 +1,80 @@
+/**
+ * What the wire-surface closure is allowed to stop at (#1432, review P1).
+ *
+ * The closure walk fails closed: every type name a listed declaration
+ * references must resolve to a listed declaration, or to an entry here. There
+ * is no silent skip, because the silent skip WAS the defect — a listed type
+ * could grow `foo?: ImportedShape` from an unlisted module and stay green.
+ *
+ * Two kinds of stop, kept apart because they are different claims:
+ *
+ * - an EXTERNAL MODULE is outside this repository, so there is no declaration
+ *   to digest and no way for a change to it to ship in our tarball;
+ * - a WAIVER is a repo declaration the walk reaches that is genuinely not
+ *   payload — transport plumbing, a local error class, a callback type. Each
+ *   one is a claim someone has to defend in review, which is the point.
+ *
+ * A waiver stops the walk. If a waived type later starts carrying payload,
+ * moving it into `surface.ts` is the fix, not widening this file.
+ */
+
+/**
+ * Module specifiers whose types have no declaration site in this repo. Prefix
+ * match, so `node:http` covers `node:http2` only if spelled out — keep them
+ * exact enough to mean something.
+ */
+const WIRE_EXTERNAL_MODULES: Readonly<Record<string, string>> = {
+  'node:http': 'Node HTTP request/response objects are the transport, not the payload on it.',
+  'node:stream': 'Stream plumbing carries bytes; the bytes themselves are digested elsewhere.',
+  undici: 'HTTP client types belong to the transport library, not to our wire contract.',
+};
+
+/**
+ * Repo declarations the closure reaches that are deliberately not wire
+ * surface. Key is `<file>#<name>`; the value has to say why a change to it
+ * cannot change what a skewed peer parses.
+ */
+export const WIRE_CLOSURE_WAIVERS: Readonly<Record<string, string>> = {
+  'packages/kernel/src/errors.ts#AppError':
+    'The local error CLASS. Only its projection onto the wire is contractual, and that projection is DaemonError, which is listed.',
+  'packages/kernel/src/errors.ts#AppErrorCode':
+    'Error-code vocabulary is carried as a plain string in DaemonError.code; adding a code cannot change the envelope a peer parses.',
+  'packages/kernel/src/errors.ts#AppErrorDetails':
+    'Local detail bag behind AppError; the wire form is DaemonError.details, typed as Record<string, unknown> and listed.',
+  'src/daemon/client/daemon-client-metadata.ts#DaemonInfo':
+    'Client-side record of where a daemon is listening (pid, ports, state dir). Never serialized into a request or response.',
+  'src/daemon/types.ts#DaemonRequest':
+    'Re-export alias of the listed kernel DaemonRequest; the declaration that fixes the shape is packages/kernel/src/contracts.ts#DaemonRequest.',
+  'src/daemon/types.ts#DaemonResponse':
+    'Re-export alias of the listed kernel DaemonResponse; the shape is fixed by packages/kernel/src/contracts.ts#DaemonResponse.',
+  'packages/contracts/src/request-progress.ts#RequestProgressSink':
+    'A callback type (event) => void. The event it receives is RequestProgressEvent, which is listed; the sink itself never crosses the wire.',
+
+  // The CLI flag/option vocabulary embedded in DaemonCommandRequest. This is
+  // the one waiver pair that stops a large subtree, so it is the one to
+  // challenge first — the reasoning is ADR 0006's own, not convenience.
+  //
+  // These types are the CLI-SIDE projection. They do not cross the boundary as
+  // typed fields: DaemonRequest carries `input?: Record<string, unknown>` and
+  // `flags?: Record<string, unknown>`, both listed, and the flag vocabulary
+  // lands inside those bags as plain keys. ADR 0006 then classifies a new flag
+  // as additive ("new optional request fields or flags that older daemons can
+  // ignore or reject with a normal command error"), explicitly NOT a bump. So
+  // digesting them would fire the gate on every new CLI flag for a change the
+  // decision says needs no acknowledgment — and a gate that cries wolf gets
+  // its acks rubber-stamped, which costs more than the coverage buys.
+  //
+  // If a flag ever becomes a typed field on DaemonRequest, that changes
+  // DaemonRequest's own digest, and the right fix is to list the type here
+  // rather than widen this waiver.
+  'packages/contracts/src/client-request.ts#InternalRequestOptions':
+    'CLI-side option projection; reaches the peer inside DaemonRequest.input/flags (Record<string, unknown>, both listed), and ADR 0006 calls new flags additive.',
+  'packages/contracts/src/command-flags.ts#CommandFlags':
+    'CLI-side flag vocabulary; reaches the peer inside DaemonRequest.flags (Record<string, unknown>, listed), and ADR 0006 calls new flags additive.',
+};
+
+export function isExternalWireSpecifier(specifier: string): boolean {
+  return Object.keys(WIRE_EXTERNAL_MODULES).some(
+    (prefix) => specifier === prefix || specifier.startsWith(`${prefix}/`),
+  );
+}

test/wire-compat/closure.ts

@@ -0,0 +1,66 @@
+/**
+ * The closure walk: does every type a listed wire declaration references land
+ * somewhere someone wrote down? (#1432, review P1.)
+ *
+ * Extracted from the gate test so the planted-red proofs in
+ * `wire-mutations.test.ts` exercise the SAME walk the gate runs. A red proof
+ * against a second copy of the logic proves nothing about the gate.
+ */
+
+import { readTopLevelDeclarationNames, readTypeReferences } from './declaration-digest.ts';
+import { createOriginResolver } from './module-resolution.ts';
+import type { WireDeclarationRef } from './surface.ts';
+
+export type ClosureGaps = {
+  /** Repo declarations reached by the walk that are neither listed nor waived. */
+  omitted: string[];
+  /** Names nothing could place — the fail-closed case. */
+  unresolved: string[];
+};
+
+export function findClosureGaps(options: {
+  repoRoot: string;
+  readSource: (file: string) => string;
+  declarations: readonly WireDeclarationRef[];
+  /** Keys (`<file>#<name>`) treated as covered; normally every listed declaration. */
+  claimed: ReadonlySet<string>;
+  waivers: Readonly<Record<string, string>>;
+  isExternalSpecifier: (specifier: string) => boolean;
+}): ClosureGaps {
+  const { repoRoot, readSource, declarations, claimed, waivers, isExternalSpecifier } = options;
+
+  const declaredNames = new Map<string, Set<string>>();
+  const declaresLocally = (file: string, name: string): boolean => {
+    let names = declaredNames.get(file);
+    if (!names) {
+      names = new Set(readTopLevelDeclarationNames(file, readSource(file)));
+      declaredNames.set(file, names);
+    }
+    return names.has(name);
+  };
+
+  const resolveOrigin = createOriginResolver({
+    repoRoot,
+    readSource,
+    declaresLocally,
+    isExternalSpecifier,
+  });
+
+  const omitted = new Set<string>();
+  const unresolved = new Set<string>();
+  for (const ref of declarations) {
+    for (const name of readTypeReferences(ref.file, readSource(ref.file), ref.name)) {
+      const origin = resolveOrigin(ref.file, name);
+      if (origin.kind === 'global' || origin.kind === 'external') continue;
+      if (origin.kind === 'unresolved') {
+        unresolved.add(`${name} (referenced by ${ref.name} in ${ref.file})`);
+        continue;
+      }
+      const key = `${origin.file}#${origin.name}`;
+      if (claimed.has(key) || key in waivers) continue;
+      omitted.add(`${key} (reached from ${ref.name} in ${ref.file})`);
+    }
+  }
+
+  return { omitted: [...omitted].sort(), unresolved: [...unresolved].sort() };
+}

test/wire-compat/declaration-digest.ts

@@ -129,6 +129,35 @@ export function digestDeclaration(file: string, source: string, name: string): s
     .digest('hex')}`;
 }
 
+/**
+ * Replaces `from` with `to` **inside `name`'s declaration only**, returning the
+ * whole mutated source.
+ *
+ * The planted-red proofs need this rather than `source.replace`: sibling
+ * declarations in the same file share substrings (two functions in
+ * `request-progress-protocol.ts` both end their template with the same framing),
+ * so a whole-file replace silently mutates the FIRST match and leaves the
+ * declaration under test untouched — a vacuous proof that looks like a real one.
+ * Throws when `from` is absent from the span, so that case fails loudly.
+ */
+export function replaceInDeclaration(
+  file: string,
+  source: string,
+  name: string,
+  from: string,
+  to: string,
+): string {
+  const { span } = findDeclaration(file, source, name);
+  const declaration = source.slice(span.start, span.end);
+  if (!declaration.includes(from)) {
+    throw new Error(
+      `${file}#${name} does not contain ${JSON.stringify(from)}, so a mutation built on it would ` +
+        `be a no-op. Re-point the case at the declaration's current text.`,
+    );
+  }
+  return source.slice(0, span.start) + declaration.replace(from, to) + source.slice(span.end);
+}
+
 function walk(node: unknown, visit: (node: Node) => void): void {
   if (!node || typeof node !== 'object') return;
   if (Array.isArray(node)) {
@@ -162,7 +191,16 @@ function rootTypeName(typeName: unknown): string | null {
 export function readTypeReferences(file: string, source: string, name: string): string[] {
   const { node } = findDeclaration(file, source, name);
   const names = new Set<string>();
+  // A declaration's own generic parameters (`JsonRpcRequestEnvelope<TParams>`)
+  // read as type references but have no declaration site to gate — they are
+  // bound right here. Collected across the whole subtree so a nested generic
+  // helper's parameters drop out too.
+  const typeParameters = new Set<string>();
   walk(node, (child) => {
+    if (child.type === 'TSTypeParameter') {
+      const bound = (child.name as { name?: unknown } | undefined)?.name;
+      if (typeof bound === 'string') typeParameters.add(bound);
+    }
     if (child.type === 'TSTypeReference') {
       const referenced = rootTypeName(child.typeName);
       if (referenced) names.add(referenced);
@@ -173,5 +211,8 @@ export function readTypeReferences(file: string, source: string, name: string):
     }
   });
   names.delete(name);
+  // `x as const` parses as a type reference to the contextual keyword.
+  names.delete('const');
+  for (const bound of typeParameters) names.delete(bound);
   return [...names].sort();
 }