diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ced41311..a29b930ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,8 +100,12 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 steps: + # The layering ratchets (R6/R9/R10) measure the merge-base with origin/main, which a + # shallow checkout cannot reach. - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 # The layering gate parses production sources with `oxc-parser`, so # dependencies are required; keep install-deps enabled. @@ -117,7 +121,7 @@ jobs: # Model tests for the dependency-graph report and its blast-radius query. The report # reads the gate's model (scripts/layering/model.ts) and applies the gate's own R6 - # counting rule, so it is not a second measurement of TYPE_INVERSION_BASELINE. + # counting rule, so it is not a second measurement of the R6 ratchet. - name: Check the depgraph report model uses: ./.github/actions/run-gate with: { gate: depgraph } diff --git a/docs/dependency-graph-findings.md b/docs/dependency-graph-findings.md index 825925797..0e8884890 100644 --- a/docs/dependency-graph-findings.md +++ b/docs/dependency-graph-findings.md @@ -22,8 +22,8 @@ const files = listSourceFiles(); const sources = new Map(files.map((f) => [f, fs.readFileSync(f, 'utf8')])); const edges = resolveImportEdges(sources); -// e.g. R6 inversions per zone pair, deduplicated by file pair — reproduces -// TYPE_INVERSION_BASELINE, so a mismatch means one of the two is stale. +// e.g. R6 inversions per zone pair, deduplicated by file pair — the same count the gate +// ratchets against the merge-base with origin/main. const seen = new Set(); const byPair = new Map(); for (const edge of edges) { @@ -126,8 +126,9 @@ narrow name replaced both. from `daemon-command-registry.ts` to key an exhaustive owner-file map; that remaining inversion is the commands-zone consumer, not a second source of truth for the union. -All remaining inversions are argued at `TYPE_INVERSION_BASELINE` in `scripts/layering/check.ts`, next -to the numbers they explain. +All remaining inversions are argued here. R6 (`scripts/layering/type-inversion-ratchet.ts`) records +no numbers of its own: its reference is the same count taken at the merge-base with `origin/main`, +so a zone pair can only shrink. ## 0b. The biggest structural finding is not an inversion @@ -156,13 +157,13 @@ but it is a comprehension one, and it is the single largest obstacle to reading isolation. At the current measured commit it spans `commands` (33), `daemon-server` (30), `platforms` (19), `core` (12), root composition (5), `contracts` (2), and `client` (1). -Now ratcheted for growth by **R9** (`TYPE_CYCLE_BASELINE`, derived from the zone ceilings in -`scripts/layering/daemon-modularity.ts`), so it cannot get worse +Now ratcheted for growth by **R9** (`scripts/layering/daemon-modularity.ts`), so it cannot get worse while nobody is looking — a type-only import that closes a new loop fails the gate, verified by adding one type-only import that closes a loop and watching the gate reject it. It was growth-only -here; #1781 A6 made it an equality pin, so a baseline left above the measured size fails too and a -shrink is banked by the change that earns it. The refactor itself is still deliberately not -attempted; it starts at those four hubs. +here; #1781 A6 made it an equality pin, and the pin is now the merge-base's own measurement +(`scripts/layering/ratchet-reference.ts`), so a shrink is banked the moment it merges and there is +no slack left to spend. The refactor itself is still deliberately not attempted; it starts at those +four hubs. ### The facade cycle: investigated, no narrower port exists @@ -183,14 +184,15 @@ duplicate the public API shape — a second source of truth for it — or derive carry the same dependency. Those four files are therefore the minimum number of naming sites, not an accident: they are the -choke point. Accepted as a position, argued at `TYPE_INVERSION_BASELINE`. The option this section +choke point. Accepted as a position, argued in §0 above. The option this section used to hold open — moving `NAVIGATION_COMMAND_PROJECTIONS` out of `commands/` — was answered by deleting it: five direct signatures replaced the registry, so there is no longer a projection registry whose home is in question. ## 1. The two remaining type-inversion clusters -`TYPE_INVERSION_BASELINE` in `scripts/layering/check.ts` holds both, with the reasoning inline. +§0 above holds both, with the reasoning inline; the gate measures them against the merge-base +rather than recording them. **28 + 1 edges → `client/client-types.ts`** — *done, mostly.* Now 5 edges. The vocabulary moved into the `contracts/client-*.ts` family files — one file per command/domain family, largest 137 LOC — diff --git a/scripts/__tests__/committed-source-tree.ts b/scripts/__tests__/committed-source-tree.ts index ed87b15e1..72190ceb3 100644 --- a/scripts/__tests__/committed-source-tree.ts +++ b/scripts/__tests__/committed-source-tree.ts @@ -44,6 +44,56 @@ export function renamedSince(repoRoot: string, base: string): ReadonlyMap/src/`, and workspace package manifests. One definition, so a consumer reading + * a committed tree cannot classify it differently from the walker. + */ +function committedSourceSet(tracked: readonly string[]): { + sources: string[]; + manifests: string[]; +} { + return { + sources: tracked.filter((file) => WALKED_SOURCE.test(file) && isProductionSourceFile(file)), + manifests: tracked.filter((file) => WALKED_MANIFEST.test(file)), + }; +} + +/** Contents of `files` at `treeish`, through ONE long-lived `git cat-file --batch`. */ +function readCommittedBlobs( + repoRoot: string, + treeish: string, + files: readonly string[], +): Map { + if (files.length === 0) return new Map(); + const requests = files.map((file) => `${treeish}:${file}\n`).join(''); + return parseCatFileBatch(git(repoRoot, ['cat-file', '--batch'], requests), files); +} + +/** + * The same enumeration and blob read as `createCommittedSourceTree`, handed over as text: the + * production sources and workspace manifests committed at `treeish`. A whole-tree measurement + * (the layering ratchets) needs the corpus rather than a reader, and taking it from here is what + * keeps its file set identical to the closure walker's. + */ +// fallow-ignore-next-line unused-export -- consumed by scripts/layering, outside fallow's scope +export function readCommittedSources( + repoRoot: string, + treeish: string, +): { sources: Map; manifests: Map } { + const { sources, manifests } = committedSourceSet(listCommittedTree(repoRoot, treeish)); + const blobs = readCommittedBlobs(repoRoot, treeish, [...sources, ...manifests]); + const only = (files: readonly string[]) => + new Map(files.flatMap((file) => (blobs.has(file) ? [[file, blobs.get(file)!] as const] : []))); + return { sources: only(sources), manifests: only(manifests) }; +} + /** * ` blob \n\n` per hit and ` missing\n` per miss, in request * order. Sizes are bytes, so this walks the raw buffer rather than a string offset. @@ -82,15 +132,11 @@ function directoriesOf(files: ReadonlySet): Set { * whole tree, never one per file. A read outside that set is a widening request, not a fallback. */ export function createCommittedSourceTree(repoRoot: string, treeish: string): SourceTreeReader { - const listing = git(repoRoot, ['ls-tree', '-r', '--name-only', '-z', treeish]).toString('utf8'); - const tracked = new Set(listing.split('\0').filter(Boolean)); + const listing = listCommittedTree(repoRoot, treeish); + const tracked = new Set(listing); const directories = directoriesOf(tracked); - const walked = [...tracked].filter( - (file) => - WALKED_MANIFEST.test(file) || (WALKED_SOURCE.test(file) && isProductionSourceFile(file)), - ); - const requests = walked.map((file) => `${treeish}:${file}\n`).join(''); - const contents = parseCatFileBatch(git(repoRoot, ['cat-file', '--batch'], requests), walked); + const { sources, manifests } = committedSourceSet(listing); + const contents = readCommittedBlobs(repoRoot, treeish, [...sources, ...manifests]); const relative = (file: string) => path.relative(repoRoot, file).split(path.sep).join('/'); return { exists: (file) => tracked.has(relative(file)) || directories.has(relative(file)), diff --git a/scripts/depgraph/README.md b/scripts/depgraph/README.md index 45b8352ac..11a81b77f 100644 --- a/scripts/depgraph/README.md +++ b/scripts/depgraph/README.md @@ -81,9 +81,10 @@ it returns an empty list, which is the gate passing, not a broken query. `pnpm check:layering` is. The report reads the same model (`scripts/layering/model.ts`) and applies the gate's own counting rule — `typeInversionsByPair` counts once per file pair over the raw edges, -exactly as `checkTypeInversions` in `scripts/layering/check.ts` does — so `typeInversions` reproduces -`TYPE_INVERSION_BASELINE` by construction, not by a second measurement. CI used to assert that -equality; it was a duplicate detector of the same code path and was removed. In particular the count +exactly as `typeInversionCounts` in `scripts/layering/model.ts` does — so `typeInversions` reproduces +the gate's R6 measurement by construction, not by a second measurement. The gate compares that +measurement with the merge-base's; CI used to assert the report agreed with a recorded baseline, +which was a duplicate detector of the same code path and was removed. In particular the count does NOT come from the collapsed edge list, where `dynamic` outranks `type` and a module imported both lazily and for its types would drop out. diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index 08715e17f..d61b0def5 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -22,10 +22,9 @@ // record is store-owned mutable state that any daemon module can write; and the terminal // concrete-platform boundary (R65), which rejects every import form into the retired // src/platforms path or a platform package. -// - Over the TYPE GRAPH: the largest type-level import cycle is pinned by -// equality (R9). R4 keeps the value graph acyclic, so these cycles are free at -// runtime but bound what can be read in isolation; growth fails, and so does a -// baseline left above the measured size. +// - Over the TYPE GRAPH: the largest type-level import cycle may not grow past the +// merge-base (R9). R4 keeps the value graph acyclic, so these cycles are free at +// runtime but bound what can be read in isolation. // - Across the DAEMON MODULARITY MIGRATION: R7 ownership pressure and external // daemon/types.ts importers only shrink, R9 zone membership cannot grow or absorb // engine files, and planned logical modules start with zero forbidden/internal imports (R10). @@ -37,6 +36,9 @@ // code cannot manufacture or repair a narrowed runtime proof (R66). // - Over CONTRACTS PRODUCTION SOURCE: contracts owns vocabulary only — host, process, and timer // mechanics belong in capture-kit or an adapter (R18). +// R6, R9, and the R10 R7 counts are ratchets with no written-down reference: each is the same +// measurement taken over the merge-base with origin/main (`ratchet-reference.ts`), so growth +// fails, a shrink needs no edit, and no change can bank headroom. // `(root)` holds entrypoints and composition roots. The retired `src/utils` zone is deliberately // outside the spine and is rejected separately by R14; extracted workspace package zones are // classified separately and held behind R11 instead of the src folder spine. @@ -56,13 +58,19 @@ import { import { backEdgePair, findValueImportCycles, - largestTypeCycleMembers, + memoizedImportParser, resolveImportEdges, topFolder, - typeInversionPair, type LayeringViolation, type ResolvedImportEdge, } from './model.ts'; +import { checkTypeInversions } from './type-inversion-ratchet.ts'; +import { + measureRatchets, + mergeBaseRatchets, + type LayeringRatchets, + type MergeBaseRatchets, +} from './ratchet-reference.ts'; import { checkDaemonModularityRatchets, checkRetiredInteractionPaths, @@ -224,110 +232,6 @@ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation }); } -// Catches: a type-only import against the ranked spine's declared order — a design-level -// dependency (zone A is stated in terms of zone B) that R5 is blind to because it costs -// nothing at runtime, so nothing else flags "the type shape leaks the wrong direction." -// Evidence: the R5-adjacent commits in check.ts's history introduced this ratchet; the 61-to-5 -// reduction and the two remaining deliberate inversions are recorded below and in -// docs/dependency-graph-findings.md. -// Cost: not attributed (folded into check.ts's whole-graph pass; no standalone module or test -// file to size separately). -// Kill criterion: none enforced today; retire only by maintainer decision that type-only spine -// inversions no longer matter. Reaching zero remaining inversions does not retire it: at zero -// the ratchet is what keeps the count from regrowing, and tsc never rejects a type-only edge. -// -// R6 ratchet: type-only spine inversions, per zone pair. R5 cannot see these (a type-only import -// is free at runtime), but "zone A is declared in terms of zone B" is still a boundary claim, and -// ranking type edges surfaced 61 of them. Down to 5, and every one of the 5 is now a deliberate -// architectural position rather than a misplaced declaration: -// -// commands/mcp -> client (4) `AgentDeviceClient`, used as an opaque handle ("the client this -// command runs against"). The facade no longer reaches back into -// commands/ — the navigation projection it was once built from is -// retired — so this is no longer a zone-level cycle, just a port -// that would have to cover the whole facade: 4 files NAME it, but 26 -// call sites use methods across 13 of its namespaces, so any port -// would re-declare the public API. R5 is zero here: nothing imports -// the client at runtime, only its type. -// -// commands -> daemon-server (1) `DaemonCommandRoute` is declared in core so descriptors can -// name a route without importing the daemon. `command-explain.ts` -// still type-imports the re-export from `daemon-command-registry.ts` -// to key an exhaustive `Record` of -// owner files; that remaining inversion is the commands-zone -// consumer, not a second source of truth for the union. -// -// See docs/dependency-graph-findings.md §0 for the long form. The counts may only go DOWN. Fixing edges without lowering the number fails too, so the baseline -// cannot quietly stop describing the tree. -// -// This gate is the sole owner of the ratchet. The depgraph report reuses the shared inversion -// classifier for observability, but does not compare its report output with this baseline. -export const TYPE_INVERSION_BASELINE: Readonly> = { - 'commands -> client': 3, - 'commands -> daemon-server': 1, - 'mcp -> client': 1, -}; - -function checkTypeInversions(edges: readonly ResolvedImportEdge[]): LayeringViolation[] { - const seen = new Set(); - const countsByPair = new Map(); - const firstEdgeByPair = new Map(); - for (const edge of edges) { - const pair = typeInversionPair(edge); - if (!pair) continue; - const identity = `${edge.file} -> ${edge.target}`; - if (seen.has(identity)) continue; - seen.add(identity); - countsByPair.set(pair, (countsByPair.get(pair) ?? 0) + 1); - if (!firstEdgeByPair.has(pair)) firstEdgeByPair.set(pair, edge); - } - - const violations: LayeringViolation[] = []; - for (const [pair, count] of [...countsByPair].sort(([left], [right]) => - left.localeCompare(right), - )) { - const allowed = TYPE_INVERSION_BASELINE[pair]; - const edge = firstEdgeByPair.get(pair)!; - if (allowed === undefined) { - violations.push({ - rule: 'R6 type-spine-inversion', - file: edge.file, - line: edge.line, - message: - `new type-only ${pair} inversion (${count} edge(s), e.g. ${edge.file} -> ${edge.target}). ` + - `Declare the shared type below both zones instead of adding it to TYPE_INVERSION_BASELINE.`, - }); - continue; - } - if (count > allowed) { - violations.push({ - rule: 'R6 type-spine-inversion', - file: edge.file, - line: edge.line, - message: - `type-only ${pair} inversions grew to ${count} (baseline ${allowed}). ` + - `Move the shared type below both zones; the baseline may only shrink.`, - }); - } - } - - for (const [pair, allowed] of Object.entries(TYPE_INVERSION_BASELINE)) { - const count = countsByPair.get(pair) ?? 0; - if (count >= allowed) continue; - const message = - count === 0 - ? `type-only ${pair} inversions are all gone — delete this entry from TYPE_INVERSION_BASELINE.` - : `type-only ${pair} inversions dropped to ${count} — lower TYPE_INVERSION_BASELINE to ${count}.`; - violations.push({ - rule: 'R6 type-spine-inversion', - file: 'scripts/layering/check.ts', - line: 1, - message, - }); - } - return violations; -} - function checkSessionStateOwnership(sources: ReadonlyMap): LayeringViolation[] { const types = sources.get('src/daemon/types.ts'); if (!types) { @@ -438,17 +342,23 @@ function checkSessionStateOwnership(sources: ReadonlyMap): Layer function report( files: readonly string[], violations: readonly LayeringViolation[], - typeCycle: number, + ratchets: LayeringRatchets, + reference: MergeBaseRatchets, ): number { if (violations.length === 0) { + const inversions = Object.values(ratchets.typeInversions).reduce( + (sum, count) => sum + count, + 0, + ); process.stdout.write( `Layering guard: OK — ${files.length} source files satisfy R2 and contain no ` + `value-import cycles (both checked globally); the ranked target spine contains no ` + - `back-edges; the ranked spine's type-only inversions match the R6 ratchet (${Object.values(TYPE_INVERSION_BASELINE).reduce((sum, count) => sum + count, 0)} remaining); ` + + `back-edges; the ranked spine's type-only inversions hold at or under the merge-base ` + + `${reference.ref.slice(0, 10)} per zone pair (R6, ${inversions} remaining); ` + `${RETIRED_PATH_RULES.R14.rule} permits no tracked paths under retired src/utils; ` + `all ${sessionStateFieldCount()} SessionState fields are classified and every write is ` + - `inside its declared owner (R7); the largest type-level cycle is ${typeCycle} files ` + - `(R9); ${daemonModularitySummary()}; ` + + `inside its declared owner (R7); the largest type-level cycle is ` + + `${ratchets.largestTypeCycle.length} files (R9); ${daemonModularitySummary(reference)}; ` + `${packageBoundariesSummary(repoRoot)}; ${platformPackagePolicySummary()}; ` + `runtime facts remain the only device-command admission authority and daemon code cannot ` + `manufacture narrowed runtime proof (R66); and R65 keeps production src/daemon free of ` + @@ -485,7 +395,10 @@ export type LayeringContext = Readonly<{ allTypeScriptSources: ReadonlyMap; trackedSrcUtilsFiles: readonly string[]; edges: readonly ResolvedImportEdge[]; - typeCycleMembers: readonly string[]; + /** The ratcheted measurements of this tree. */ + ratchets: LayeringRatchets; + /** The same measurements at the merge-base with origin/main. */ + reference: LayeringRatchets; }>; export type LayeringRule = (context: LayeringContext) => LayeringViolation[]; @@ -544,10 +457,11 @@ export const LAYERING_RULES: Readonly> = { 'selector-pipeline-ownership': (context) => selectorPipelineOwnershipViolations(context.edges, workspaceSpecifierTargets(repoRoot)), 'back-edges': (context) => checkBackEdges(context.edges), - 'type-spine-inversions': (context) => checkTypeInversions(context.edges), + 'type-spine-inversions': (context) => + checkTypeInversions(context.edges, context.reference.typeInversions), 'session-state-ownership': (context) => checkSessionStateOwnership(context.sources), 'daemon-modularity-ratchets': (context) => [ - ...checkDaemonModularityRatchets(context.edges, context.typeCycleMembers), + ...checkDaemonModularityRatchets(context.edges, context.ratchets, context.reference), ...checkRetiredSessionLifecyclePaths(context.sourceFiles), ...checkRetiredSessionObservabilityPaths(context.sourceFiles), ...checkRetiredSnapshotExecutionPaths(context.sourceFiles), @@ -579,20 +493,24 @@ export function main(): number { const sources = readSources(sourceFiles); const allTypeScriptSources = readSources(listTypeScriptFiles()); const trackedSrcUtilsFiles = listTrackedSrcUtilsFiles(repoRoot); - const edges = resolveImportEdges(sources, workspaceSpecifierTargets(repoRoot)); - // Computed once and threaded: the rule and the success line must report the same number. - const typeCycleMembers = largestTypeCycleMembers(edges); - const typeCycle = typeCycleMembers.length; + // One memoizing parser for both trees: every file the merge-base shares with the working tree + // is parsed once, whichever scan reaches it first. + const parse = memoizedImportParser(); + const edges = resolveImportEdges(sources, workspaceSpecifierTargets(repoRoot), parse); + // Measured once and threaded: the rules and the success line must report the same numbers. + const ratchets = measureRatchets(sources, edges); + const reference = mergeBaseRatchets(repoRoot, parse); const context: LayeringContext = { sourceFiles, sources, allTypeScriptSources, trackedSrcUtilsFiles, edges, - typeCycleMembers, + ratchets, + reference, }; const violations = Object.values(LAYERING_RULES).flatMap((rule) => rule(context)); - return report(sourceFiles, violations, typeCycle); + return report(sourceFiles, violations, ratchets, reference); } if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { diff --git a/scripts/layering/daemon-modularity.test.ts b/scripts/layering/daemon-modularity.test.ts index 464341c04..1b9cef021 100644 --- a/scripts/layering/daemon-modularity.test.ts +++ b/scripts/layering/daemon-modularity.test.ts @@ -7,10 +7,9 @@ import { checkRetiredSessionObservabilityPaths, checkRetiredSnapshotExecutionPaths, DAEMON_MODULARITY_BASELINE, - TYPE_CYCLE_BASELINE, } from './daemon-modularity.ts'; -import { SESSION_STATE_FIELD_OWNERS } from './session-state.ts'; import { resolveImportEdges, targetDagZone, type ResolvedImportEdge } from './model.ts'; +import type { LayeringRatchets } from './ratchet-reference.ts'; function importEdge(file: string, target: string): ResolvedImportEdge { return { @@ -45,13 +44,8 @@ const ZONE_DIRECTORY: Readonly> = { 'provider-webdriver': 'packages/provider-webdriver/src/', }; -/** - * A cycle membership that exactly fills the zone ceilings. R9 is equality-pinned, so a test - * probing anything else starts from the pinned size the way `baselineEdges` starts from the - * pinned edges; `overrides` re-counts one zone without disturbing the others. - */ -function baselineTypeCycleMembers(overrides: Readonly> = {}): string[] { - const zones = { ...DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers, ...overrides }; +/** A cycle membership of `count` files per zone, so a zone count becomes file paths. */ +function typeCycleMembers(zones: Readonly>): string[] { return Object.entries(zones).flatMap(([zone, count]) => Array.from( { length: count }, @@ -60,18 +54,55 @@ function baselineTypeCycleMembers(overrides: Readonly> = ); } -test('daemon modularity baseline records the measured R7 ownership pressure', () => { - assert.equal( - Object.keys(SESSION_STATE_FIELD_OWNERS).length, - DAEMON_MODULARITY_BASELINE.sessionState.writerOwnedFields, +/** + * The merge-base measurement every test ratchets against. R9 and R10's R7 counts have no recorded + * numbers any more, so a test states its own reference tree instead of importing one. + */ +const REFERENCE: LayeringRatchets = { + typeInversions: {}, + largestTypeCycle: typeCycleMembers({ 'provider-webdriver': 6 }), + sessionState: { writerOwnedFields: 19, ownerFileClaims: 22 }, +}; + +/** The same measurement as the reference except where a test moves one number. */ +function measured(overrides: Partial = {}): LayeringRatchets { + return { ...REFERENCE, ...overrides }; +} + +test('R10 rejects R7 ownership pressure that grew past the merge-base', () => { + const grownFields = checkDaemonModularityRatchets( + baselineEdges(), + measured({ sessionState: { writerOwnedFields: 20, ownerFileClaims: 23 } }), + REFERENCE, ); - assert.equal( - Object.values(SESSION_STATE_FIELD_OWNERS).reduce((sum, owners) => sum + owners.length, 0), - DAEMON_MODULARITY_BASELINE.sessionState.ownerFileClaims, + assert.deepEqual( + grownFields.map(({ rule, message }) => ({ rule, message })), + [ + { + rule: 'R10 daemon-modularity', + message: + 'R7 writerOwnedFields grew to 20 (baseline 19 at the merge-base). Route the new write ' + + 'through an existing owner instead.', + }, + { + rule: 'R10 daemon-modularity', + message: + 'R7 ownerFileClaims grew to 23 (baseline 22 at the merge-base). Route the new write ' + + 'through an existing owner instead.', + }, + ], + ); +}); + +test('R10 banks an R7 shrink with no edit anywhere', () => { + assert.deepEqual( + checkDaemonModularityRatchets( + baselineEdges(), + measured({ sessionState: { writerOwnedFields: 18, ownerFileClaims: 20 } }), + REFERENCE, + ), + [], ); - assert.equal(TYPE_CYCLE_BASELINE, 6); - assert.equal(DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers['provider-webdriver'], 6); - assert.equal('daemon-server' in DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers, false); }); test('external daemon/types.ts importer membership changes require the baseline to change', () => { @@ -84,14 +115,16 @@ test('external daemon/types.ts importer membership changes require the baseline const violations = checkDaemonModularityRatchets( [...baselineEdges(), ...edges], - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.equal(violations.length, 1); assert.match(violations[0]!.message, /may only shrink from the recorded 2/); const removed = checkDaemonModularityRatchets( baselineDaemonTypesEdges().slice(1), - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.equal(removed.length, 1); assert.match(removed[0]!.message, /delete it from externalDaemonTypesImporters/); @@ -110,7 +143,8 @@ test('logical modules reject forbidden imports', () => { const violations = checkDaemonModularityRatchets( [...baselineEdges(), ...edges], - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.equal(violations.length, 1); assert.match(violations[0]!.message, /replay-test must not import/); @@ -138,7 +172,8 @@ test('replay-test rejects request-global and engine-internal imports', () => { const violations = checkDaemonModularityRatchets( [...baselineEdges(), ...edges], - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.deepEqual( violations.map(({ message }) => message.replace(/;.*/, '')), @@ -162,7 +197,7 @@ test('replay-test may still import its own files inside the package', () => { ); assert.deepEqual( - checkDaemonModularityRatchets([...baselineEdges(), ...edges], baselineTypeCycleMembers()), + checkDaemonModularityRatchets([...baselineEdges(), ...edges], REFERENCE, REFERENCE), [], ); }); @@ -177,7 +212,8 @@ test('internal trees reject deep imports globally, including from daemon', () => const violations = checkDaemonModularityRatchets( [...baselineEdges(), ...edges], - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.equal(violations.length, 1); assert.match(violations[0]!.message, /must not import maestro's internal tree/); @@ -218,7 +254,8 @@ test('daemon replay rejects handler, owner, session-store, and engine deep edges const violations = checkDaemonModularityRatchets( [...baselineEdges(), ...edges], - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.deepEqual( violations.map(({ file, line, message }) => ({ @@ -283,7 +320,8 @@ test('session lifecycle rejects handler deep imports in both directions', () => const violations = checkDaemonModularityRatchets( [...baselineEdges(), ...edges], - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.deepEqual( violations.map(({ file, line, message }) => ({ @@ -348,7 +386,8 @@ test('interaction rejects handler crossings and deep imports around its facade', const violations = checkDaemonModularityRatchets( [...baselineEdges(), ...edges], - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.equal(violations.length, 5); assert.ok( @@ -393,7 +432,8 @@ test('session observability rejects handler deep imports in both directions', () const violations = checkDaemonModularityRatchets( [...baselineEdges(), ...edges], - baselineTypeCycleMembers(), + REFERENCE, + REFERENCE, ); assert.deepEqual( violations.map(({ file, line, message }) => ({ @@ -552,17 +592,19 @@ test('session observability rejects restored handler paths', () => { ); }); -test('R9 records zone ceilings and keeps engine files outside the largest component', () => { +test('R9 holds each zone to the merge-base and keeps engine files outside the component', () => { // One commands file and one engine file traded for two provider-webdriver ones, so the - // total stays at the baseline and only the per-zone claims are on trial. - const zones = DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers; + // total stays at the merge-base's size and only the per-zone claims are on trial. const violations = checkDaemonModularityRatchets( baselineEdges(), - baselineTypeCycleMembers({ - commands: 1, - 'ad-replay': 1, - 'provider-webdriver': zones['provider-webdriver']! - 2, + measured({ + largestTypeCycle: typeCycleMembers({ + commands: 1, + 'ad-replay': 1, + 'provider-webdriver': 4, + }), }), + REFERENCE, ); assert.equal(violations.length, 3); @@ -573,42 +615,54 @@ test('R9 records zone ceilings and keeps engine files outside the largest compon // #1837: the zone violation used to name the alphabetically-first zone member — a file that had // been in the cycle all along — so the +1 was found only by diffing member lists between commits. -// The ceiling records a count, not a membership, so the message lists every zone member instead. -test('R10 zone overflow lists the whole zone so the joining member is visible', () => { - const zones = DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers; - // Sorts after the provider-webdriver probes: the old first-member pick could not name it by luck. +// The merge-base carries membership, so the message names exactly the files that joined. +test('R10 zone overflow names the member that joined the cycle', () => { + // Sorts after the provider-webdriver probes: a first-member pick could not name it by luck. const joined = 'src/daemon/snapshot-interactor-capture.ts'; - const members = [ - ...baselineTypeCycleMembers({ 'provider-webdriver': zones['provider-webdriver']! - 1 }), - joined, - ].sort(); - const daemonMembers = members.filter((member) => member.startsWith('src/daemon/')); - assert.deepEqual(daemonMembers, [joined]); + const members = [...typeCycleMembers({ 'provider-webdriver': 5 }), joined].sort(); - const violations = checkDaemonModularityRatchets(baselineEdges(), members); + const violations = checkDaemonModularityRatchets( + baselineEdges(), + measured({ largestTypeCycle: members }), + REFERENCE, + ); assert.equal(violations.length, 1); const [violation] = violations; assert.equal(violation!.rule, 'R10 daemon-modularity'); assert.equal(violation!.file, 'scripts/layering/daemon-modularity.ts'); - assert.match(violation!.message, /contains 1 daemon-server file\(s\) \(baseline 0\)/); - for (const member of daemonMembers) { - assert.ok(violation!.message.includes(member), `${member} missing from: ${violation!.message}`); - } - assert.match(violation!.message, /1 over the ceiling — the member\(s\) that joined are among/); + assert.match( + violation!.message, + /contains 1 daemon-server file\(s\) \(baseline 0 at the merge-base\)/, + ); + assert.match( + violation!.message, + new RegExp(`1 over the merge-base — the daemon-server file\\(s\\) that joined: ${joined}\\.`), + ); }); -// Growth was always rejected; a baseline left ABOVE the measured size used to be a suggestion -// in the success line, which is headroom the next change spends without a number moving. -test('R9 rejects a baseline left above the measured cycle', () => { - const zones = DAEMON_MODULARITY_BASELINE.largestTypeCycle.zoneMembers; +test('R9 rejects a cycle grown past the merge-base', () => { const violations = checkDaemonModularityRatchets( baselineEdges(), - baselineTypeCycleMembers({ 'provider-webdriver': zones['provider-webdriver']! - 1 }), + measured({ largestTypeCycle: typeCycleMembers({ 'provider-webdriver': 7 }) }), + REFERENCE, ); - assert.equal(violations.length, 1); + assert.equal(violations.length, 2); assert.match(violations[0]!.rule, /^R9 /); - assert.match(violations[0]!.message, /dropped to 5 files \(baseline 6\)/); - assert.match(violations[0]!.message, /Lower LARGEST_TYPE_CYCLE_ZONE_CEILINGS by the same 1/); + assert.match(violations[0]!.message, /grew to 7 files \(baseline 6 at the merge-base\)/); + assert.match(violations[1]!.rule, /^R10 /); +}); + +// The shrink direction used to need an edit in the same change, or the ceiling kept headroom the +// next change could spend. Measuring the merge-base banks it on merge, with nothing to lower. +test('R9 banks a shrink with no edit anywhere', () => { + assert.deepEqual( + checkDaemonModularityRatchets( + baselineEdges(), + measured({ largestTypeCycle: typeCycleMembers({ 'provider-webdriver': 5 }) }), + REFERENCE, + ), + [], + ); }); diff --git a/scripts/layering/daemon-modularity.ts b/scripts/layering/daemon-modularity.ts index 22d08673e..d7a304e30 100644 --- a/scripts/layering/daemon-modularity.ts +++ b/scripts/layering/daemon-modularity.ts @@ -9,35 +9,18 @@ import { type LogicalModulePolicy, } from './architecture-ownership.ts'; import { targetDagZone, type LayeringViolation, type ResolvedImportEdge } from './model.ts'; -import { SESSION_STATE_FIELD_OWNERS } from './session-state.ts'; - -const LARGEST_TYPE_CYCLE_ZONE_CEILINGS: Readonly> = { - // co-defined-contract pair (`capabilities.ts` ↔ `runtime.ts` and the four files they - // pull in). The standard shrink is a third module holding the shared type. - 'provider-webdriver': 6, -}; +import type { LayeringRatchets } from './ratchet-reference.ts'; +// R7 ownership pressure and the largest type cycle (whole and per zone) are ratcheted against the +// merge-base with origin/main (`ratchet-reference.ts`); the importer membership below stays a +// recorded list, because it names files rather than counting them. export const DAEMON_MODULARITY_BASELINE = { - sessionState: { - // R60 moved `audioProbe` to store-owned. R64 does the same for the neutral `perfCapture` - // and `lastPerfProfile` records after retiring the two platform-specific perf fields. - writerOwnedFields: 19, - ownerFileClaims: 22, - }, - largestTypeCycle: { - zoneMembers: LARGEST_TYPE_CYCLE_ZONE_CEILINGS, - }, externalDaemonTypesImporters: [ 'src/client/client-normalizers.ts', 'src/remote/daemon-artifacts.ts', ], } as const; -export const TYPE_CYCLE_BASELINE = Object.values(LARGEST_TYPE_CYCLE_ZONE_CEILINGS).reduce( - (sum, count) => sum + count, - 0, -); - const ENGINE_FILE_PREFIXES = [ 'packages/ad-replay/src/', 'packages/maestro/src/', @@ -55,17 +38,18 @@ const ENGINE_FILE_PREFIXES = [ * Cost: 937 LOC total for the file (323 rule + 614 test; shared with R9's checkTypeCycleBaseline * below, not attributed separately). * Kill criterion: none enforced today; retire only by maintainer decision that the daemon - * modularity baselines (SessionState field-owner counts, logical-module import policies and - * facades, the external daemon/types.ts importer list, per-zone cycle ceilings) no longer + * modularity measurements (SessionState field-owner counts, logical-module import policies and + * facades, the external daemon/types.ts importer list, per-zone cycle membership) no longer * matter. Every one is a count or an import edge the compiler accepts either way. */ export function checkDaemonModularityRatchets( edges: readonly ResolvedImportEdge[], - largestTypeCycleMembers: readonly string[], + measured: LayeringRatchets, + reference: LayeringRatchets, ): LayeringViolation[] { return [ - ...checkSessionStateBaseline(), - ...checkTypeCycleBaseline(largestTypeCycleMembers), + ...checkSessionStateBaseline(measured.sessionState, reference.sessionState), + ...checkTypeCycleBaseline(measured.largestTypeCycle, reference.largestTypeCycle), ...checkDaemonTypesImporters(edges), ...checkLogicalModuleImports(edges), ]; @@ -132,94 +116,74 @@ export function checkRetiredInteractionPaths(sourceFiles: readonly string[]): La ); } -function checkSessionStateBaseline(): LayeringViolation[] { - const actual = { - writerOwnedFields: Object.keys(SESSION_STATE_FIELD_OWNERS).length, - ownerFileClaims: Object.values(SESSION_STATE_FIELD_OWNERS).reduce( - (sum, owners) => sum + owners.length, - 0, - ), - }; +function checkSessionStateBaseline( + measured: LayeringRatchets['sessionState'], + reference: LayeringRatchets['sessionState'], +): LayeringViolation[] { const violations: LayeringViolation[] = []; for (const metric of ['writerOwnedFields', 'ownerFileClaims'] as const) { - const baseline = DAEMON_MODULARITY_BASELINE.sessionState[metric]; - if (actual[metric] === baseline) continue; + if (measured[metric] <= reference[metric]) continue; violations.push({ rule: 'R10 daemon-modularity', file: 'scripts/layering/daemon-modularity.ts', line: 1, message: - actual[metric] > baseline - ? `R7 ${metric} grew to ${actual[metric]} (baseline ${baseline}). Route the new write through an existing owner instead.` - : `R7 ${metric} dropped to ${actual[metric]} — lower the daemon modularity baseline in the same capability move so it cannot regrow.`, + `R7 ${metric} grew to ${measured[metric]} (baseline ${reference[metric]} at the ` + + `merge-base). Route the new write through an existing owner instead.`, }); } return violations; } /** - * Catches: the largest type-only import cycle growing past its pinned size, or the baseline - * shrinking without the ceiling being lowered to match — R4 keeps the value graph acyclic, so - * these cycles cost nothing at runtime, but an ungoverned type cycle can grow without bound - * while every individual edge still looks locally reasonable. + * Catches: the largest type-only import cycle growing past what the merge-base holds, whole or + * in any one zone — R4 keeps the value graph acyclic, so these cycles cost nothing at runtime, + * but an ungoverned type cycle can grow without bound while every individual edge still looks + * locally reasonable. * Evidence: 6984a1e095 (#1852) fixed R10's zone listing when this ceiling trips, evidence the - * check fires in practice; ef6ec2995b (#1825, #1781 A6) made the R9 shrink direction - * mandatory rather than advisory. + * check fires in practice; ef6ec2995b (#1825, #1781 A6) made a banked shrink mandatory rather + * than advisory, which measuring the merge-base now does without an edit. * Cost: 937 LOC total for the file (323 rule + 614 test; shared with R10's ratchets above, not * attributed separately). * Kill criterion: none enforced today; retire only by maintainer decision that a bounded - * type-only cycle size no longer matters. tsc never rejects a type-only cycle, and emptying - * LARGEST_TYPE_CYCLE_ZONE_CEILINGS pins the size at zero rather than retiring the check. + * type-only cycle size no longer matters. tsc never rejects a type-only cycle, and a merge-base + * with no cycle pins the size at zero rather than retiring the check. */ -function checkTypeCycleBaseline(members: readonly string[]): LayeringViolation[] { +function checkTypeCycleBaseline( + members: readonly string[], + referenceMembers: readonly string[], +): LayeringViolation[] { const violations: LayeringViolation[] = []; - const baseline = DAEMON_MODULARITY_BASELINE.largestTypeCycle; - if (members.length > TYPE_CYCLE_BASELINE) { + if (members.length > referenceMembers.length) { violations.push({ rule: 'R9 type-cycle-size', file: 'scripts/layering/daemon-modularity.ts', line: 1, message: `the largest type-level import cycle grew to ${members.length} files (baseline ` + - `${TYPE_CYCLE_BASELINE}). A type-only import that closes a loop makes every file in the ` + - `loop unreadable in isolation. Declare the shared type below both modules, or if the growth ` + - `is genuinely warranted, raise the zone ceilings in the same commit and say why.`, - }); - } else if (members.length < TYPE_CYCLE_BASELINE) { - // A ceiling left above the measured size is headroom a later change spends without a - // reviewer ever seeing a number move, so the shrink is recorded in the change that earns - // it — the same equality pin R6 and the R10 R7 counts already carry. - violations.push({ - rule: 'R9 type-cycle-size', - file: 'scripts/layering/daemon-modularity.ts', - line: 1, - message: - `the largest type-level import cycle dropped to ${members.length} files (baseline ` + - `${TYPE_CYCLE_BASELINE}). Lower LARGEST_TYPE_CYCLE_ZONE_CEILINGS by the same ${TYPE_CYCLE_BASELINE - members.length} ` + - `in this change so the cycle cannot regrow into slack nobody chose.`, + `${referenceMembers.length} at the merge-base). A type-only import that closes a loop makes ` + + `every file in the loop unreadable in isolation. Declare the shared type below both modules.`, }); } - const membersByZone = groupBy(members, targetDagZone); - for (const [zone, zoneMembers] of membersByZone) { - const allowed = baseline.zoneMembers[zone] ?? 0; + const referenceByZone = groupBy(referenceMembers, targetDagZone); + for (const [zone, zoneMembers] of groupBy(members, targetDagZone)) { + const referenceZoneMembers = new Set(referenceByZone.get(zone) ?? []); + const allowed = referenceZoneMembers.size; if (zoneMembers.length <= allowed) continue; - // The ceiling records a count, not a membership, so the gate cannot name the file that - // joined; naming the alphabetically-first member instead sent #1837's diagnosis to a file - // that had been in the cycle all along. List the whole zone so the joining edge is one - // diff away from the author, who knows which of these files the change touched. The - // overflow is net growth (a join and a departure cancel out), so it bounds nothing about - // how many members are new — only that at least one of the listed files is. + // A ceiling recorded a count, so the gate could only list the whole zone and #1837's + // diagnosis landed on a file that had been in the cycle all along. The merge-base carries + // membership, so the files that joined are named exactly. + const joined = zoneMembers.filter((member) => !referenceZoneMembers.has(member)); violations.push({ rule: 'R10 daemon-modularity', file: 'scripts/layering/daemon-modularity.ts', line: 1, message: `the largest type cycle now contains ${zoneMembers.length} ${zone} file(s) (baseline ` + - `${allowed}); extraction must not trade one zone's locality for another's. ` + - `${zoneMembers.length - allowed} over the ceiling — the member(s) that joined are among ` + - `these ${zone} files: ${zoneMembers.join(', ')}. Cut the edge that pulled them in ` + - `rather than raising the ceiling.`, + `${allowed} at the merge-base); extraction must not trade one zone's locality for ` + + `another's. ${zoneMembers.length - allowed} over the merge-base — the ${zone} file(s) ` + + `that joined: ${joined.join(', ')}. Cut the edge that pulled them in.`, }); } @@ -340,11 +304,11 @@ function groupBy( return groups; } -export function daemonModularitySummary(): string { - const session = DAEMON_MODULARITY_BASELINE.sessionState; +export function daemonModularitySummary(reference: LayeringRatchets): string { + const session = reference.sessionState; return ( - `R10 pins R7 at ${session.writerOwnedFields} writer-owned fields / ` + - `${session.ownerFileClaims} owner claims, R9 at ${TYPE_CYCLE_BASELINE} files with zone ceilings, ` + + `R10 holds R7 at the merge-base's ${session.writerOwnedFields} writer-owned fields / ` + + `${session.ownerFileClaims} owner claims, R9 at its ${reference.largestTypeCycle.length} files per zone, ` + `${DAEMON_MODULARITY_BASELINE.externalDaemonTypesImporters.length} external daemon/types.ts importers, ` + 'and zero forbidden logical-module imports' ); diff --git a/scripts/layering/model.ts b/scripts/layering/model.ts index 7cb355160..4209ffea2 100644 --- a/scripts/layering/model.ts +++ b/scripts/layering/model.ts @@ -325,14 +325,33 @@ function resolveTargetFile( return candidates.find((candidate) => sourceFiles.has(candidate)) ?? null; } +export type ImportParser = (source: string) => ImportEdge[]; + +/** + * `parseImports` memoized by source text. A ratchet parses two trees that share almost every + * file, so the second tree costs a parse only where its text differs from the first. + */ +export function memoizedImportParser(): ImportParser { + const edgesBySource = new Map(); + return (source) => { + let edges = edgesBySource.get(source); + if (!edges) { + edges = parseImports(source); + edgesBySource.set(source, edges); + } + return edges; + }; +} + export function resolveImportEdges( sources: ReadonlyMap, workspaceExportTargets?: ReadonlyMap, + parse: ImportParser = parseImports, ): ResolvedImportEdge[] { const sourceFiles = new Set(sources.keys()); const edges: ResolvedImportEdge[] = []; for (const [file, source] of sources) { - for (const edge of parseImports(source)) { + for (const edge of parse(source)) { const target = resolveTargetFile(file, edge.spec, sourceFiles, workspaceExportTargets); if (!target) continue; edges.push({ @@ -452,12 +471,34 @@ export function backEdgePair(edge: ResolvedImportEdge): string | null { // a type-only import costs nothing at runtime and does not affect cold start — but a // type-only edge still says "this zone is declared in terms of that one", and that IS a // boundary claim. Ranking them found 61 inversions the gate had never seen, which is why -// they are ratcheted rather than merely reported: see `TYPE_INVERSION_BASELINE`. +// they are ratcheted rather than merely reported against the merge-base with origin/main: see +// `typeInversionCounts` and scripts/layering/type-inversion-ratchet.ts. export function typeInversionPair(edge: ResolvedImportEdge): string | null { if (edge.dynamic || !edge.typeOnly) return null; return spineInversionPair(edge); } +/** + * R6's measurement: distinct type-only spine inversions per zone pair, keyed `from -> to` and + * sorted by pair. The ratchet compares this record across two trees, so it is a pure function of + * the edge set rather than a count taken inside the rule. + */ +export function typeInversionCounts( + edges: readonly ResolvedImportEdge[], +): Readonly> { + const seen = new Set(); + const counts = new Map(); + for (const edge of edges) { + const pair = typeInversionPair(edge); + if (!pair) continue; + const identity = `${edge.file} -> ${edge.target}`; + if (seen.has(identity)) continue; + seen.add(identity); + counts.set(pair, (counts.get(pair) ?? 0) + 1); + } + return Object.fromEntries([...counts].sort(([left], [right]) => left.localeCompare(right))); +} + export function collectBackEdges(edges: readonly ResolvedImportEdge[]): BackEdgeMap { const identitiesByPair = new Map>(); for (const edge of edges) { diff --git a/scripts/layering/package-boundaries.ts b/scripts/layering/package-boundaries.ts index ab0beffe4..cb8c462c4 100644 --- a/scripts/layering/package-boundaries.ts +++ b/scripts/layering/package-boundaries.ts @@ -80,11 +80,24 @@ export function specifierSites(file: string, source: string): SpecifierSite[] { * filtering the output. */ export function readWorkspacePackages(repoRoot: string): WorkspacePackage[] { + return workspacePackagesFromManifests( + new Map( + listTrackedPackageManifests(repoRoot).map((manifestFile) => [ + manifestFile, + fs.readFileSync(path.join(repoRoot, manifestFile), 'utf8'), + ]), + ), + ); +} + +/** The same package model over manifest sources already in hand, e.g. read from a git ref. */ +export function workspacePackagesFromManifests( + manifests: ReadonlyMap, +): WorkspacePackage[] { const packages: WorkspacePackage[] = []; - for (const manifestFile of listTrackedPackageManifests(repoRoot).sort()) { + for (const manifestFile of [...manifests.keys()].sort()) { const entry = path.posix.basename(path.posix.dirname(manifestFile)); - const manifestPath = path.join(repoRoot, manifestFile); - const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')) as { + const manifest = JSON.parse(manifests.get(manifestFile)!) as { name?: string; private?: boolean; exports?: Record; @@ -334,8 +347,19 @@ function walkTsFiles(repoRoot: string, relativeDir: string): string[] { /** Flat `specifier -> repo-relative source` map across all workspace packages. */ export function workspaceSpecifierTargets(repoRoot: string): Map { + return specifierTargetsOf(readWorkspacePackages(repoRoot)); +} + +/** The same flat map for a manifest set read elsewhere, e.g. at a git ref. */ +export function workspaceSpecifierTargetsFromManifests( + manifests: ReadonlyMap, +): Map { + return specifierTargetsOf(workspacePackagesFromManifests(manifests)); +} + +function specifierTargetsOf(packages: readonly WorkspacePackage[]): Map { const targets = new Map(); - for (const pkg of readWorkspacePackages(repoRoot)) { + for (const pkg of packages) { for (const [specifier, target] of pkg.exportTargets) targets.set(specifier, target); } return targets; diff --git a/scripts/layering/ratchet-reference.test.ts b/scripts/layering/ratchet-reference.test.ts new file mode 100644 index 000000000..d4b2b361a --- /dev/null +++ b/scripts/layering/ratchet-reference.test.ts @@ -0,0 +1,95 @@ +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { test } from 'node:test'; +import { memoizedImportParser, resolveImportEdges } from './model.ts'; +import { measureRatchets, mergeBaseRatchets } from './ratchet-reference.ts'; +import { sessionStateWritePressure } from './session-state.ts'; +import { listTrackedProductionSources } from './tracked-sources.ts'; +import { readCommittedSources } from '../__tests__/committed-source-tree.ts'; + +const SESSION_TYPES = [ + 'export type SessionState = {', + ' snapshot?: string;', + ' trace?: string;', + ' name: string;', + '};', +].join('\n'); + +function tree(extra: Record = {}) { + return new Map([ + ['src/daemon/types.ts', SESSION_TYPES], + ['src/daemon/session-snapshot.ts', 'session.snapshot = "a"; nextSession.snapshot = "b";'], + ['src/daemon/ref-frame.ts', 'session.snapshot = undefined;'], + ['src/daemon/handlers/trace-runtime.ts', 'session.trace ??= "t";'], + ['src/client/client.ts', "import type { Loop } from '../commands/loop.ts';"], + ['src/commands/loop.ts', "import type { Client } from '../client/client.ts';"], + ...Object.entries(extra), + ]); +} + +const repoRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { + encoding: 'utf8', +}).trim(); + +test('sessionStateWritePressure counts written fields and (field, writer) claims, not writes', () => { + assert.deepEqual(sessionStateWritePressure(tree()), { + writerOwnedFields: 2, + ownerFileClaims: 3, + }); + assert.deepEqual( + sessionStateWritePressure(tree({ 'src/daemon/other.ts': 'session[key] = 1;' })), + { + writerOwnedFields: 2, + ownerFileClaims: 3, + }, + ); + assert.deepEqual(sessionStateWritePressure(new Map()), { + writerOwnedFields: 0, + ownerFileClaims: 0, + }); +}); + +test('measureRatchets reports all three ratchets from one tree', () => { + const sources = tree(); + assert.deepEqual(measureRatchets(sources, resolveImportEdges(sources)), { + typeInversions: { 'commands -> client': 1 }, + largestTypeCycle: ['src/client/client.ts', 'src/commands/loop.ts'], + sessionState: { writerOwnedFields: 2, ownerFileClaims: 3 }, + }); +}); + +test('memoizedImportParser parses each distinct source text once', () => { + let parses = 0; + const a = "import type { Client } from '../client/client.ts';"; + const sources = new Map([ + ['src/commands/a.ts', a], + ['src/commands/b.ts', `${a.slice(0, 5)}${a.slice(5)}`], + ['src/client/client.ts', 'export type Client = {};'], + ]); + const parse = memoizedImportParser(); + assert.deepEqual(resolveImportEdges(sources, undefined, parse), resolveImportEdges(sources)); + resolveImportEdges(sources, undefined, (source) => { + parses++; + return parse(source); + }); + assert.equal(parses, 3); + assert.equal(parse(a), parse(a)); +}); + +// The reference is a committed tree read through the shared committed-tree reader, so its file +// set must be the one the working-tree scan uses. A divergence here means the git-side +// enumeration or blob read no longer matches the layering scan input. +test('the committed enumeration at HEAD is the working-tree scan input', () => { + const { sources, manifests } = readCommittedSources(repoRoot, 'HEAD'); + assert.deepEqual([...sources.keys()], listTrackedProductionSources(repoRoot)); + assert.ok(manifests.has('packages/kernel/package.json')); +}); + +test('the merge-base reference names its ref and measures the real tree', () => { + const reference = mergeBaseRatchets(repoRoot); + assert.match(reference.ref, /^[0-9a-f]{40}$/); + assert.ok(reference.largestTypeCycle.length >= 1); + assert.ok(reference.sessionState.writerOwnedFields > 0); + assert.ok(reference.sessionState.ownerFileClaims >= reference.sessionState.writerOwnedFields); + assert.ok(Object.keys(reference.typeInversions).length > 0); +}); diff --git a/scripts/layering/ratchet-reference.ts b/scripts/layering/ratchet-reference.ts new file mode 100644 index 000000000..45317bb35 --- /dev/null +++ b/scripts/layering/ratchet-reference.ts @@ -0,0 +1,60 @@ +// The three ratcheted measurements (R6 type-spine inversions, R9 largest type cycle, R10's R7 +// ownership pressure) and where their reference numbers come from: the merge-base with +// origin/main, measured by the same functions that measure the working tree. Growth fails, a +// shrink needs no edit, and no recorded number can sit above what main actually holds. +// +// The base tree is read through the shared committed-tree reader (one `git ls-tree`, one +// `git cat-file --batch`), never a second checkout and never a read per file. + +import { mergeBaseWithMain, readCommittedSources } from '../__tests__/committed-source-tree.ts'; +import { + largestTypeCycleMembers, + memoizedImportParser, + resolveImportEdges, + typeInversionCounts, + type ImportParser, + type ResolvedImportEdge, +} from './model.ts'; +import { workspaceSpecifierTargetsFromManifests } from './package-boundaries.ts'; +import { sessionStateWritePressure, type SessionStateWritePressure } from './session-state.ts'; + +export type LayeringRatchets = Readonly<{ + /** R6: distinct type-only spine inversions per `from -> to` zone pair. */ + typeInversions: Readonly>; + /** R9 and R10's zone membership: sorted members of the largest type-level cycle. */ + largestTypeCycle: readonly string[]; + /** R10: R7 ownership pressure. */ + sessionState: SessionStateWritePressure; +}>; + +export type MergeBaseRatchets = LayeringRatchets & Readonly<{ ref: string }>; + +export function measureRatchets( + sources: ReadonlyMap, + edges: readonly ResolvedImportEdge[], +): LayeringRatchets { + return { + typeInversions: typeInversionCounts(edges), + largestTypeCycle: largestTypeCycleMembers(edges), + sessionState: sessionStateWritePressure(sources), + }; +} + +/** + * The reference measurement: the merge-base tree, enumerated and classified by the same reader + * the eager-closure ratchet uses, then measured exactly like the working tree. A file whose text + * is byte-identical at both ends costs no second parse, because `parse` memoizes by source text. + */ +export function mergeBaseRatchets( + repoRoot: string, + parse: ImportParser = memoizedImportParser(), +): MergeBaseRatchets { + const ref = mergeBaseWithMain(repoRoot); + const { sources, manifests } = readCommittedSources(repoRoot, ref); + const edges = resolveImportEdges( + sources, + workspaceSpecifierTargetsFromManifests(manifests), + parse, + ); + return { ref, ...measureRatchets(sources, edges) }; +} diff --git a/scripts/layering/retired-paths-policy.test.ts b/scripts/layering/retired-paths-policy.test.ts index d97b53536..675459bc6 100644 --- a/scripts/layering/retired-paths-policy.test.ts +++ b/scripts/layering/retired-paths-policy.test.ts @@ -11,9 +11,16 @@ import { listTrackedTypeScriptFiles, } from './tracked-sources.ts'; import { RETIRED_PATH_RULES, retiredPathRuleViolations } from './retired-paths-policy.ts'; +import type { LayeringRatchets } from './ratchet-reference.ts'; const repoRoot = path.resolve(import.meta.dirname, '../..'); +const EMPTY_RATCHETS: LayeringRatchets = { + typeInversions: {}, + largestTypeCycle: [], + sessionState: { writerOwnedFields: 0, ownerFileClaims: 0 }, +}; + function contextWithFiles( files: Partial>, ): LayeringContext { @@ -23,7 +30,8 @@ function contextWithFiles( allTypeScriptSources: new Map(), trackedSrcUtilsFiles: [], edges: [], - typeCycleMembers: [], + ratchets: EMPTY_RATCHETS, + reference: EMPTY_RATCHETS, ...files, }; } diff --git a/scripts/layering/session-state.ts b/scripts/layering/session-state.ts index 31719407b..a51199bbc 100644 --- a/scripts/layering/session-state.ts +++ b/scripts/layering/session-state.ts @@ -274,3 +274,30 @@ export function findSessionStateWrites( (left, right) => left.file.localeCompare(right.file) || left.line - right.line, ); } + +export type SessionStateWritePressure = Readonly<{ + /** Declared fields that some daemon module writes directly. */ + writerOwnedFields: number; + /** Distinct (field, writing module) pairs — what `SESSION_STATE_FIELD_OWNERS` claims. */ + ownerFileClaims: number; +}>; + +/** + * R10's measurement of R7 pressure: how many declared fields have a direct writer, and how many + * module claims that takes. Read from the tree rather than from the ownership table, so the same + * function measures a merge-base tree whose table is not in scope. On a tree R7 accepts, both + * numbers equal the table's own size. + */ +export function sessionStateWritePressure( + sources: ReadonlyMap, +): SessionStateWritePressure { + const types = sources.get('src/daemon/types.ts'); + if (!types) return { writerOwnedFields: 0, ownerFileClaims: 0 }; + const writes = findSessionStateWrites(sources, sessionStateFields(types)).filter( + (write) => write.field !== '[computed]', + ); + return { + writerOwnedFields: new Set(writes.map((write) => write.field)).size, + ownerFileClaims: new Set(writes.map((write) => `${write.field}\0${write.file}`)).size, + }; +} diff --git a/scripts/layering/type-inversion-ratchet.test.ts b/scripts/layering/type-inversion-ratchet.test.ts new file mode 100644 index 000000000..4c5e091e8 --- /dev/null +++ b/scripts/layering/type-inversion-ratchet.test.ts @@ -0,0 +1,50 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { resolveImportEdges, typeInversionCounts } from './model.ts'; +import { checkTypeInversions } from './type-inversion-ratchet.ts'; + +/** `commandsFiles` commands files type-importing the client, plus one value import R6 ignores. */ +function inversionEdges(commandsFiles: number) { + const sources = new Map([ + ['src/client/client.ts', 'export type AgentDeviceClient = { run(): void };'], + ['src/commands/value-user.ts', "import { run } from '../client/client.ts';"], + ]); + for (let index = 0; index < commandsFiles; index++) { + sources.set( + `src/commands/typed-${index}.ts`, + "import type { AgentDeviceClient } from '../client/client.ts';", + ); + } + return resolveImportEdges(sources); +} + +test('typeInversionCounts keys distinct type-only inversions by zone pair, sorted', () => { + assert.deepEqual(typeInversionCounts(inversionEdges(2)), { 'commands -> client': 2 }); + assert.deepEqual(typeInversionCounts([]), {}); +}); + +test('R6 is quiet at the merge-base count, and banks a shrink with no edit', () => { + assert.deepEqual(checkTypeInversions(inversionEdges(2), { 'commands -> client': 2 }), []); + assert.deepEqual(checkTypeInversions(inversionEdges(1), { 'commands -> client': 2 }), []); + assert.deepEqual(checkTypeInversions(inversionEdges(0), { 'commands -> client': 2 }), []); +}); + +test('R6 rejects a pair that grew past the merge-base and names the first edge', () => { + const violations = checkTypeInversions(inversionEdges(3), { 'commands -> client': 2 }); + assert.equal(violations.length, 1); + assert.equal(violations[0]!.rule, 'R6 type-spine-inversion'); + assert.equal(violations[0]!.file, 'src/commands/typed-0.ts'); + assert.match( + violations[0]!.message, + /type-only commands -> client inversions grew to 3 \(baseline 2 at the merge-base\)/, + ); +}); + +test('R6 rejects a pair the merge-base does not have at all', () => { + const violations = checkTypeInversions(inversionEdges(1), {}); + assert.equal(violations.length, 1); + assert.match( + violations[0]!.message, + /new type-only commands -> client inversion \(1 edge\(s\), e\.g\. src\/commands\/typed-0\.ts -> src\/client\/client\.ts\); the merge-base has none/, + ); +}); diff --git a/scripts/layering/type-inversion-ratchet.ts b/scripts/layering/type-inversion-ratchet.ts new file mode 100644 index 000000000..0015aca4b --- /dev/null +++ b/scripts/layering/type-inversion-ratchet.ts @@ -0,0 +1,69 @@ +// R6: type-only spine inversions, per zone pair. R5 cannot see these (a type-only import is free +// at runtime), but "zone A is declared in terms of zone B" is still a boundary claim, and ranking +// type edges surfaced 61 of them. The survivors are argued in docs/dependency-graph-findings.md +// §0; the reference is the same count taken at the merge-base with origin/main, so a pair can +// only shrink and no change can bank headroom by recording a number above the tree. +// +// Catches: a type-only import against the ranked spine's declared order — a design-level +// dependency (zone A is stated in terms of zone B) that R5 is blind to because it costs +// nothing at runtime, so nothing else flags "the type shape leaks the wrong direction." +// Evidence: the R5-adjacent commits in check.ts's history introduced this ratchet; the 61-to-5 +// reduction and the surviving deliberate inversions are recorded in +// docs/dependency-graph-findings.md. +// Cost: 118 LOC (61 rule + 57 test), plus the shared merge-base measurement in +// ratchet-reference.ts. +// Kill criterion: none enforced today; retire only by maintainer decision that type-only spine +// inversions no longer matter. Reaching zero remaining inversions does not retire it: at zero +// the ratchet is what keeps the count from regrowing, and tsc never rejects a type-only edge. + +import { typeInversionPair, type LayeringViolation, type ResolvedImportEdge } from './model.ts'; + +const RULE = 'R6 type-spine-inversion'; + +export function checkTypeInversions( + edges: readonly ResolvedImportEdge[], + reference: Readonly>, +): LayeringViolation[] { + const seen = new Set(); + const countsByPair = new Map(); + const firstEdgeByPair = new Map(); + for (const edge of edges) { + const pair = typeInversionPair(edge); + if (!pair) continue; + const identity = `${edge.file} -> ${edge.target}`; + if (seen.has(identity)) continue; + seen.add(identity); + countsByPair.set(pair, (countsByPair.get(pair) ?? 0) + 1); + if (!firstEdgeByPair.has(pair)) firstEdgeByPair.set(pair, edge); + } + + const violations: LayeringViolation[] = []; + for (const [pair, count] of [...countsByPair].sort(([left], [right]) => + left.localeCompare(right), + )) { + const allowed = reference[pair]; + const edge = firstEdgeByPair.get(pair)!; + if (allowed === undefined) { + violations.push({ + rule: RULE, + file: edge.file, + line: edge.line, + message: + `new type-only ${pair} inversion (${count} edge(s), e.g. ${edge.file} -> ${edge.target}); ` + + `the merge-base has none. Declare the shared type below both zones.`, + }); + continue; + } + if (count > allowed) { + violations.push({ + rule: RULE, + file: edge.file, + line: edge.line, + message: + `type-only ${pair} inversions grew to ${count} (baseline ${allowed} at the merge-base). ` + + `Move the shared type below both zones; the count may only shrink.`, + }); + } + } + return violations; +}