Skip to content

Commit 447e7cf

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 d04e213 commit 447e7cf

23 files changed

Lines changed: 339 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: none enforced today; retire only by maintainer decision that the
9+
// gateway/application-resources/IME-activation trio having one writer no longer matters.
10+
// The trio are plain mutable fields, so an outside write type-checks.
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: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
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: none enforced today; retire only by maintainer decision that bin.ts's --help
10+
// fast path no longer needs to delegate to the real alias registry — moot once the fast path
11+
// is deleted or its resolution is inlined into commands/cli-command-aliases.ts, leaving no
12+
// second call site.
13+
//
114
// R12 bin-alias-fast-path.
215
//
316
// `bin.ts`'s `--help` fast path resolves a command alias (`tap`, `launch`, …) to its canonical

scripts/layering/check.ts

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

159+
/**
160+
* Catches: a production import cycle — A imports B imports A at the value level — that a
161+
* file-by-file review cannot see because each edge looks locally fine; only walking the
162+
* whole graph exposes the loop. No other gate looks at cycles at all.
163+
* Evidence: 3d70943550 (#984) introduced the import-direction DAG gate this cycle check
164+
* anchors; f19864e486 (#1410) added the dependency-graph report built on the same model.
165+
* Cost: not attributed (folded into check.ts's whole-graph pass; no standalone module or
166+
* test file to size separately).
167+
* Kill criterion: none enforced today; retire only by maintainer decision that an acyclic
168+
* value-import graph no longer matters. tsc rejects a cyclic `references` edge between
169+
* projects, but no tsconfig declares references today and the A4 spike found they are a
170+
* build-cache mechanism, not a boundary: value imports inside one project are never
171+
* cycle-checked, and a cross-package edge resolves through root node_modules with no error.
172+
*/
159173
function checkCycles(edges: readonly ResolvedImportEdge[]): LayeringViolation[] {
160174
return findValueImportCycles(edges).map((cycle) => ({
161175
rule: 'R4 value-import-cycle',
@@ -190,6 +204,19 @@ function checkRecordRuntimeOwnership(sources: ReadonlyMap<string, string>): Laye
190204
});
191205
}
192206

207+
/**
208+
* Catches: a value import that runs against the ranked target spine's declared order (a lower
209+
* zone importing a higher one) — the runtime-consequential half of what R6 also checks for
210+
* type-only edges; neither zone-policy.ts's table nor the cycle check names direction.
211+
* Evidence: 3d70943550 (#984) introduced the ranked spine and its back-edge check; docs/
212+
* dependency-graph-findings.md tracks the count this rule ratchets.
213+
* Cost: not attributed (folded into check.ts's whole-graph pass; no standalone module or
214+
* test file to size separately).
215+
* Kill criterion: none enforced today; retire only by maintainer decision that the ranked spine's
216+
* import direction no longer matters. Splitting zones into packages would not replace it: the
217+
* A4 spike found an undeclared workspace package still resolves through root node_modules and
218+
* a relative tunnel into another package's src still compiles.
219+
*/
193220
function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation[] {
194221
const seen = new Set<string>();
195222
return edges.flatMap((edge) => {
@@ -208,6 +235,18 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation
208235
});
209236
}
210237

238+
// Catches: a type-only import against the ranked spine's declared order — a design-level
239+
// dependency (zone A is stated in terms of zone B) that R5 is blind to because it costs
240+
// nothing at runtime, so nothing else flags "the type shape leaks the wrong direction."
241+
// Evidence: the R5-adjacent commits in check.ts's history introduced this ratchet; the 61-to-5
242+
// reduction and the two remaining deliberate inversions are recorded below and in
243+
// docs/dependency-graph-findings.md.
244+
// Cost: not attributed (folded into check.ts's whole-graph pass; no standalone module or test
245+
// file to size separately).
246+
// Kill criterion: none enforced today; retire only by maintainer decision that type-only spine
247+
// inversions no longer matter. Reaching zero remaining inversions does not retire it: at zero
248+
// the ratchet is what keeps the count from regrowing, and tsc never rejects a type-only edge.
249+
//
211250
// R6 ratchet: type-only spine inversions, per zone pair. R5 cannot see these (a type-only import
212251
// is free at runtime), but "zone A is declared in terms of zone B" is still a boundary claim, and
213252
// 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: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
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: none enforced today; retire only by maintainer decision that contracts owning
9+
// vocabulary only no longer matters. package.json cannot replace it: node: built-ins are not
10+
// dependencies and timer globals need no import. The A4 spike found `types: []` would break
11+
// contracts' AbortSignal/URL/Buffer/process uses while every lib that supplies them also
12+
// supplies setTimeout, and the file-level bans (interaction-outcome, snapshot-quality-warnings,
13+
// network-traffic and ios-snapshot type-only statements, kernel-only imports) have no tsc form.
14+
115
import { parseSync } from 'oxc-parser';
216
import type { LayeringViolation } from './model.ts';
317

scripts/layering/daemon-modularity.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,20 @@ 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: none enforced today; retire only by maintainer decision that the daemon
58+
* modularity baselines (SessionState field-owner counts, logical-module import policies and
59+
* facades, the external daemon/types.ts importer list, per-zone cycle ceilings) no longer
60+
* matter. Every one is a count or an import edge the compiler accepts either way.
61+
*/
4862
export function checkDaemonModularityRatchets(
4963
edges: readonly ResolvedImportEdge[],
5064
largestTypeCycleMembers: readonly string[],
@@ -143,6 +157,20 @@ function checkSessionStateBaseline(): LayeringViolation[] {
143157
return violations;
144158
}
145159

160+
/**
161+
* Catches: the largest type-only import cycle growing past its pinned size, or the baseline
162+
* shrinking without the ceiling being lowered to match — R4 keeps the value graph acyclic, so
163+
* these cycles cost nothing at runtime, but an ungoverned type cycle can grow without bound
164+
* while every individual edge still looks locally reasonable.
165+
* Evidence: 6984a1e095 (#1852) fixed R10's zone listing when this ceiling trips, evidence the
166+
* check fires in practice; ef6ec2995b (#1825, #1781 A6) made the R9 shrink direction
167+
* mandatory rather than advisory.
168+
* Cost: 937 LOC total for the file (323 rule + 614 test; shared with R10's ratchets above, not
169+
* attributed separately).
170+
* Kill criterion: none enforced today; retire only by maintainer decision that a bounded
171+
* type-only cycle size no longer matters. tsc never rejects a type-only cycle, and emptying
172+
* LARGEST_TYPE_CYCLE_ZONE_CEILINGS pins the size at zero rather than retiring the check.
173+
*/
146174
function checkTypeCycleBaseline(members: readonly string[]): LayeringViolation[] {
147175
const violations: LayeringViolation[] = [];
148176
const baseline = DAEMON_MODULARITY_BASELINE.largestTypeCycle;

scripts/layering/daemon-platform-boundary.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,20 @@ 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: none enforced today; retire only by maintainer decision that src/daemon staying
15+
* free of concrete-platform imports no longer matters. A package boundary would not replace
16+
* it: @agent-device/platform-* resolves from src/daemon through root node_modules whether
17+
* declared or not (A4 spike), and the retired src/platforms path is a plain relative import.
18+
*/
19+
620
/**
721
* R65 rejects every concrete-platform dependency from tracked production daemon sources.
822
*

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
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: none enforced today; retire only by maintainer decision that the engine.ts →
9+
// runner-presentation.ts call topology (presentIosSnapshot and publishIosSnapshot each make
10+
// their one delegating call; the runner file never folds, validates, or builds the
11+
// presentation itself) no longer matters. An exports map cannot replace it: both files sit
12+
// in one package and call each other by relative import, which no manifest restricts.
13+
114
import { parseSync } from 'oxc-parser';
215
import type { LayeringViolation } from './model.ts';
316
import { memberPath, visitAst } from './layering-ast.ts';

scripts/layering/package-boundaries.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
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: none enforced today; retire only by maintainer decision that the workspace
9+
// boundary no longer matters. `pnpm typecheck` already covers two branches for src/ and
10+
// packages/ importers (NodeNext rejects a non-exported subpath with TS2307; composite rootDir
11+
// rejects a package→root relative escape with TS6059), but the A4 spike found no compiler
12+
// mechanism for an undeclared workspace:* dependency, a root→packages/*/src relative tunnel,
13+
// or any scripts/ import: project references are a build-cache mechanism, not a boundary.
14+
//
115
// R11 package-boundaries: the workspace rules of #1490, as data the gate walks.
216
//
317
// 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: none enforced today; retire only by maintainer decision that lazy platform
12+
// composition in src/platform-runtime.ts no longer matters. The build cannot tell an eager
13+
// import from the lazy seam; both resolve and type-check identically.
14+
115
import { parseSync } from 'oxc-parser';
216
import { parseImports, type LayeringViolation } from './model.ts';
317

scripts/layering/platform-package-policy.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
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: none enforced today; retire only by maintainer decision that platform-family
11+
// isolation (private manifests with exact exports, static imports only from the composition
12+
// root, no sibling-family reach-ins, no top-level loadInventory/loadRuntime) no longer
13+
// matters. Publishing the families separately would not replace it: the A4 spike found an
14+
// undeclared workspace package still resolves through root node_modules and a relative tunnel
15+
// into a sibling's src still compiles; project references are a build cache, not a boundary.
16+
117
import path from 'node:path';
218
import { PLATFORMS } from '@agent-device/kernel/device';
319
import { parseImports, type LayeringViolation } from './model.ts';

0 commit comments

Comments
 (0)