Skip to content

Commit 92bd3dc

Browse files
committed
docs(layering): kill criteria on every rule module
Adds a four-line Catches/Evidence/Cost/Kill-criterion header to every layering rule module for R2, R4-R7, R9-R14, R16, R18, R19, R65-R73, and the rule-id uniqueness gate, so each structural check states what it catches, why no other gate sees it, its LOC cost, and the concrete condition under which it gets deleted. No behavior change.
1 parent e624ef9 commit 92bd3dc

24 files changed

Lines changed: 319 additions & 0 deletions

scripts/layering/application-lifecycle-policy.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
// Catches: application-lifecycle state (the platform-runtime-gateway/application-resources/
2+
// IME-activation trio) mutated from outside its declared owner file — an app-boot or IME
3+
// readiness race that only shows up as an intermittent device-facing flake, because the type
4+
// system sees a legal write to a legal field regardless of which module made it.
5+
// Evidence: d8a7d03faf (#1759) routed application lifecycle through runtime facts, the migration
6+
// this ownership check protects; 7b48531d3b (#2081) retired the cutover scaffolding around it.
7+
// Cost: 201 LOC (119 rule + 82 test).
8+
// Kill criterion: delete when the gateway/application-resources/IME-activation trio expose only
9+
// typed transition methods (no exported mutable field), making an outside write a compile
10+
// error instead of an AST finding.
11+
112
import { parseSync } from 'oxc-parser';
213
import { memberName, visitAst } from './layering-ast.ts';
314
import type { LayeringViolation } from './model.ts';

scripts/layering/bin-alias-fast-path.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
// Catches: bin.ts's --help fast path re-declaring its own alias table instead of calling the
2+
// real registry — the exact silent-drift bug #1618-adjacent produced, where tap/launch/
3+
// relaunch fell out of a hand-written table and paid a full CLI bootstrap for static help
4+
// text. bin.ts runs unconditionally on import, so no unit test can import and exercise it
5+
// directly; only reading its source text structurally can catch a regression.
6+
// Evidence: d85072d935 (#1641) routed command aliases through the help fast path; 74a70f1764
7+
// (#2046) removed next-major compatibility surfaces bin.ts once carried alongside it.
8+
// Cost: 650 LOC (339 rule + 311 test).
9+
// Kill criterion: delete when bin.ts's --help fast path is deleted or its alias resolution is
10+
// inlined into commands/cli-command-aliases.ts itself, leaving no second call site whose
11+
// delegation needs checking.
12+
//
113
// R12 bin-alias-fast-path.
214
//
315
// `bin.ts`'s `--help` fast path resolves a command alias (`tap`, `launch`, …) to its canonical

scripts/layering/check.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,18 @@ function checkLayeringRules(edges: readonly ResolvedImportEdge[]): LayeringViola
157157
return violations;
158158
}
159159

160+
/**
161+
* Catches: a production import cycle — A imports B imports A at the value level — that a
162+
* file-by-file review cannot see because each edge looks locally fine; only walking the
163+
* whole graph exposes the loop. No other gate looks at cycles at all.
164+
* Evidence: 3d70943550 (#984) introduced the import-direction DAG gate this cycle check
165+
* anchors; f19864e486 (#1410) added the dependency-graph report built on the same model.
166+
* Cost: not attributed (folded into check.ts's whole-graph pass; no standalone module or
167+
* test file to size separately).
168+
* Kill criterion: delete when the build graph itself enforces acyclicity (e.g. project
169+
* references per zone reject a cyclic reference edge at tsc time) so a static walk here
170+
* is redundant with a compiler error.
171+
*/
160172
function checkCycles(edges: readonly ResolvedImportEdge[]): LayeringViolation[] {
161173
return findValueImportCycles(edges).map((cycle) => ({
162174
rule: 'R4 value-import-cycle',
@@ -191,6 +203,18 @@ function checkRecordRuntimeOwnership(sources: ReadonlyMap<string, string>): Laye
191203
});
192204
}
193205

206+
/**
207+
* Catches: a value import that runs against the ranked target spine's declared order (a lower
208+
* zone importing a higher one) — the runtime-consequential half of what R6 also checks for
209+
* type-only edges; neither zone-policy.ts's table nor the cycle check names direction.
210+
* Evidence: 3d70943550 (#984) introduced the ranked spine and its back-edge check; docs/
211+
* dependency-graph-findings.md tracks the count this rule ratchets.
212+
* Cost: not attributed (folded into check.ts's whole-graph pass; no standalone module or
213+
* test file to size separately).
214+
* Kill criterion: delete when the spine's zones become physically separate packages (R11's
215+
* package-boundaries model) so an inverted import fails to resolve rather than needing a
216+
* graph walk to catch.
217+
*/
194218
function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation[] {
195219
const seen = new Set<string>();
196220
return edges.flatMap((edge) => {
@@ -209,6 +233,18 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation
209233
});
210234
}
211235

236+
// Catches: a type-only import against the ranked spine's declared order — a design-level
237+
// dependency (zone A is stated in terms of zone B) that R5 is blind to because it costs
238+
// nothing at runtime, so nothing else flags "the type shape leaks the wrong direction."
239+
// Evidence: the R5-adjacent commits in check.ts's history introduced this ratchet; the 61-to-5
240+
// reduction and the two remaining deliberate inversions are recorded below and in
241+
// docs/dependency-graph-findings.md.
242+
// Cost: not attributed (folded into check.ts's whole-graph pass; no standalone module or test
243+
// file to size separately).
244+
// Kill criterion: delete when the two remaining inversions (commands/mcp -> client,
245+
// commands -> daemon-server) are resolved by moving the projection registry or the route
246+
// union to a lower zone, leaving the ratchet at zero with nothing left to hold down.
247+
//
212248
// R6 ratchet: type-only spine inversions, per zone pair. R5 cannot see these (a type-only import
213249
// is free at runtime), but "zone A is declared in terms of zone B" is still a boundary claim, and
214250
// ranking type edges surfaced 61 of them. Down to 5, and every one of the 5 is now a deliberate

scripts/layering/contracts-implementation-policy.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
// Catches: packages/contracts production source calling host, process, or timer mechanics
2+
// directly — contracts owns vocabulary only, and a mechanic call there means an adapter's
3+
// concern leaked into the shared-vocabulary package every zone imports, invisible to
4+
// consumers because the call itself is fully typed and legal Node code.
5+
// Evidence: 8f98d23f14 (#1750) gave R18 its own number after an id collision; 057ab1c82d (#1746)
6+
// fixed double-reporting in this same authority check.
7+
// Cost: 337 LOC (206 rule + 131 test).
8+
// Kill criterion: delete when packages/contracts has no runtime dependency on node: built-ins
9+
// at all (checked by its own package.json having zero such dependencies), so a mechanic call
10+
// fails module resolution instead of needing a source-text scan.
11+
112
import { parseSync } from 'oxc-parser';
213
import type { LayeringViolation } from './model.ts';
314

scripts/layering/daemon-modularity.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,19 @@ const ENGINE_FILE_PREFIXES = [
4545
'packages/replay-test/src/',
4646
] as const;
4747

48+
/**
49+
* Catches: the daemon modularity migration regressing quietly — a SessionState field losing
50+
* its owner, a logical module gaining a forbidden or internal import, or an external
51+
* daemon/types.ts importer count creeping up — any of which erodes the wave-by-wave
52+
* extraction #1478/#1478-P5 already paid for, and nothing enforces the wave order itself.
53+
* Evidence: 2316fd32c5 (#1487) pinned the migration contracts this ratchet grew from;
54+
* 6984a1e095 (#1852) fixed the R10 zone-listing message when the type-cycle ceiling trips.
55+
* Cost: 937 LOC total for the file (323 rule + 614 test; shared with R9's checkTypeCycleBaseline
56+
* below, not attributed separately).
57+
* Kill criterion: delete when architecture-ownership.ts's LOGICAL_MODULE_POLICIES list is empty
58+
* (every planned daemon module has landed) and SESSION_STATE_FIELD_OWNERS needs no baseline
59+
* floor because ownership is enforced structurally instead of by ratchet.
60+
*/
4861
export function checkDaemonModularityRatchets(
4962
edges: readonly ResolvedImportEdge[],
5063
largestTypeCycleMembers: readonly string[],
@@ -143,6 +156,20 @@ function checkSessionStateBaseline(): LayeringViolation[] {
143156
return violations;
144157
}
145158

159+
/**
160+
* Catches: the largest type-only import cycle growing past its pinned size, or the baseline
161+
* shrinking without the ceiling being lowered to match — R4 keeps the value graph acyclic, so
162+
* these cycles cost nothing at runtime, but an ungoverned type cycle can grow without bound
163+
* while every individual edge still looks locally reasonable.
164+
* Evidence: 6984a1e095 (#1852) fixed R10's zone listing when this ceiling trips, evidence the
165+
* check fires in practice; ef6ec2995b (#1825, #1781 A6) made the R9 shrink direction
166+
* mandatory rather than advisory.
167+
* Cost: 937 LOC total for the file (323 rule + 614 test; shared with R10's ratchets above, not
168+
* attributed separately).
169+
* Kill criterion: delete when LARGEST_TYPE_CYCLE_ZONE_CEILINGS is empty — the provider-webdriver
170+
* co-defined-contract pair splits its shared type into a third module and no zone carries a
171+
* pinned type cycle any more.
172+
*/
146173
function checkTypeCycleBaseline(members: readonly string[]): LayeringViolation[] {
147174
const violations: LayeringViolation[] = [];
148175
const baseline = DAEMON_MODULARITY_BASELINE.largestTypeCycle;

scripts/layering/daemon-platform-boundary.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,19 @@ import { parseSync } from 'oxc-parser';
33
import { isProductionSourceFile } from './tracked-sources.ts';
44
import type { LayeringViolation } from './model.ts';
55

6+
/**
7+
* Catches: src/daemon importing a concrete platform package or the retired src/platforms path,
8+
* in any form — static, dynamic, or type-only — the terminal boundary the R11/R13 package
9+
* split exists to make possible; a daemon module that reaches around a platform's package
10+
* facade defeats the composition-root discipline R13 enforces on the other side.
11+
* Evidence: c794c11d7e (#2072) closed the daemon platform boundary this rule pins; 1522126f1f
12+
* (#2212) kept close lifecycle behind the session facade rather than reopening the boundary.
13+
* Cost: 694 LOC (402 rule + 292 test).
14+
* Kill criterion: delete when src/daemon has no filesystem adjacency to packages/platform-* at
15+
* all (a separate package boundary, per R11, makes a concrete-platform import a module-
16+
* resolution error instead of a source-text scan finding).
17+
*/
18+
619
/**
720
* R65 rejects every concrete-platform dependency from tracked production daemon sources.
821
*

scripts/layering/ios-snapshot-engine-policy.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
// Catches: an iOS snapshot caller reaching the XCTest/Limrun/Appium runners directly instead of
2+
// through the one converged engine.ts — the fragmentation #2222's "converge Limrun snapshots
3+
// through engine" and #758's "bulk-snapshot DEPTH limit" both trace back to, where each
4+
// backend's snapshot path could silently diverge from the others' presentation contract.
5+
// Evidence: a8ee397168 (#2213) added the snapshot engine conformance gates this policy
6+
// enforces; 6c8c0508d9 (#2222) converged Limrun snapshots through the engine it protects.
7+
// Cost: 267 LOC (229 rule + 38 test).
8+
// Kill criterion: delete when engine.ts is the only exported symbol from
9+
// packages/capture-kit/src/ios-snapshot-engine/ (runner-presentation.ts folded into it or made
10+
// unexported), so a direct-runner call fails to resolve instead of needing an AST scan.
11+
112
import { parseSync } from 'oxc-parser';
213
import type { LayeringViolation } from './model.ts';
314
import { memberPath, visitAst } from './layering-ast.ts';

scripts/layering/package-boundaries.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
// Catches: a package reaching back into root src/, a root file tunnelling into packages/*/src
2+
// with a relative path, an undeclared workspace import, or a subpath the exports map does not
3+
// name — bypasses Node's own resolution error cannot see, because a relative route resolves
4+
// fine even though it duplicates the module under its specifier form.
5+
// Evidence: 76453add71 (#1494, #1490 W0) established the workspace split this rule protects;
6+
// 83322a3f2f (#1574) pinned exact facade symbols for every workspace package.
7+
// Cost: 1239 LOC (363 rule + 876 test).
8+
// Kill criterion: delete when every workspace package enforces its own boundary at the
9+
// TypeScript project-reference level (composite tsconfig references replacing the manual
10+
// manifest/exports-map walk), so an undeclared or tunnelled import fails tsc directly.
11+
//
112
// R11 package-boundaries: the workspace rules of #1490, as data the gate walks.
213
//
314
// Package resolution already makes a deep `@agent-device/*` specifier a runtime

scripts/layering/platform-composition-policy.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
// Catches: src/platform-runtime.ts, the one canonical composition root, wiring a platform
2+
// package's implementation eagerly instead of through the lazy provider-composition seam —
3+
// a startup-cost regression (every platform's code loading on every process start) that only
4+
// shows up as a perf number, not a type error.
5+
// Evidence: 03f0f408c2 (#2070) moved platform provider composition out of the daemon into this
6+
// root; c7f42ccedc (#2117) moved the Android family behind package exports the composition
7+
// file now targets.
8+
// Cost: 103 LOC (no dedicated test file; exercised through platform-package-policy.test.ts);
9+
// shares rule id R13 with platform-package-policy.ts (1030 LOC) and
10+
// platform-package-source-policy.ts (230 LOC).
11+
// Kill criterion: delete when src/platform-runtime.ts's composition becomes generated from the
12+
// platform package manifests rather than hand-wired, so eager loading is structurally
13+
// impossible instead of merely checked for.
14+
115
import { parseSync } from 'oxc-parser';
216
import { parseImports, type LayeringViolation } from './model.ts';
317

scripts/layering/platform-package-policy.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
// Catches: a platform package's private implementation loaded before the one canonical
2+
// composition root assembles it, or a cross-boundary edge into another platform family's
3+
// private surface — the six platform packages moved behind package facades (#2116-#2125)
4+
// specifically to make eager loading and cross-family reach-ins visible, and only a source
5+
// walk over every package's imports can confirm the boundary actually held.
6+
// Evidence: 838ed223b5 (#2116) moved the six W6 platform families behind package facades;
7+
// ed26b31c94 (#2125) contracted the Apple platform surface to match.
8+
// Cost: 1030 LOC (377 rule + 653 test); shared with platform-composition-policy.ts (103 LOC)
9+
// and platform-package-source-policy.ts (230 LOC), which this module orchestrates.
10+
// Kill criterion: delete when the platform families are separate npm-published packages with
11+
// their own dependency graph (rather than workspace packages sharing one repo's build), so
12+
// premature loading fails as a missing dependency instead of needing a structural check.
13+
114
import path from 'node:path';
215
import { PLATFORMS } from '@agent-device/kernel/device';
316
import { parseImports, type LayeringViolation } from './model.ts';

0 commit comments

Comments
 (0)