Skip to content

test(structure): per-package eager-closure budgets — the ADR-0019 loading-shape probe (#1739) - #1965

Merged
thymikee merged 5 commits into
mainfrom
test/1960-eager-closure-budgets
Aug 24, 2026
Merged

test(structure): per-package eager-closure budgets — the ADR-0019 loading-shape probe (#1739)#1965
thymikee merged 5 commits into
mainfrom
test/1960-eager-closure-budgets

Conversation

@thymikee

@thymikee thymikee commented Aug 22, 2026

Copy link
Copy Markdown
Member

Summary

ADR-0019's "Implementation-laziness" section requires platform-package façades to stay implementation-lazy and is explicit that a startup threshold alone is not a substitute for preserving the loading shape: "the tracking issue owns the exact probe and planted-red procedure." #1950 built the AST-level walker (eager-import-closure.fixtures.ts); #1959/#1969 fixed two more instances of the regression class by hand. This PR generalizes the proof into a data-driven budget table so every workspace-package entry surface — and any designated hub module — gets an eager-closure ceiling without a bespoke test file.

Three files:

  • src/__tests__/eager-closure-budgets.ts — the data module: the budget table, the platform-implementation patterns, manifest-derived entry discovery, and the import-chain formatter.
  • src/__tests__/eager-closure-budgets.test.ts — iterates the table and asserts.
  • src/__tests__/eager-import-closure.fixtures.ts — gains eagerClosureGraphOf (predecessor-recording BFS) and per-file edge memoization; eagerClosureOf keeps its contract and is expressed in terms of it.

Discovery has one owner, shared with R11

facadeEntryFiles(repoRoot) lives in scripts/layering/package-boundaries.ts, and both R11's façade gate and this budget table call it — it is one function, not two implementations of the same rule. It takes each package's package.json exports targets, then adds every production source under a src/facades/ directory, walking recursively so a nested façade file cannot be covered by one gate and missed by the other.

(The first version of this PR claimed to share R11's discovery while actually carrying its own one-level readdir. That scan missed nested façade files, and missed every manifest-only entry surface — including all six platform façades. It is now genuinely one owner, and R11's façade test is green on it.)

This matters concretely: the six packages/platform-*/src/index.ts façades — the exact subject of ADR-0019's implementation-laziness rule — have no facades/ directory at all. A directory-only scan omits them, and the gate would claim to prove the loading shape while never looking at it. A dedicated non-vacuity test asserts all six are discovered.

Each platform façade evaluates exactly one module: itself. Its metadata is inline, its contract imports are import type (erased), and every implementation loads behind a function-scoped await import. A budget of 1 is the tightest possible statement of "metadata-eager and implementation-lazy", and a single static value import would destroy it while every other gate stayed green — R3/R13 govern import direction, never evaluation weight.

Pins are exact ratchets, asserted with equality

Every pin equals the measured closure size on 04e4c23b9, asserted with equality, not <=. Growing fails, and shrinking also fails until the row is lowered in the same PR — which is what "only ever ratchets down" requires, and what <= does not give: under <=, the moment an entry legitimately shrinks the unchanged row silently becomes headroom for the next regression. No slack in either direction. This matches how the repo pins R9 type-cycle size, R10 writer/owner counts, and test-file line counts — "existing pins only shrink; a new pin requires measured justification" (docs/agents/testing.md). Slack is not neutral for this gate: the regression it catches is a single static import dragging a subtree in, measured by #1969 at 5–12% of suite import work each, and a ceiling carrying "a few files" of spare room silently absorbs the small end of exactly that. Growth stays allowed — it just has to be a visible number change in the diff of the PR that causes it.

103 rows: 95 package entry surfaces + 8 designated hubs. The table is two Record<string, number> literals keyed by entry path, so a duplicate row inside either is a TypeScript error (TS1117) rather than something a later Set conversion would swallow. Notable values — platform-{apple,android,harmonyos,vega,linux,web} 1 each; contracts/facades/platform.ts 42; contracts/facades/interaction.ts 25; most granularized contracts entries 15; maestro 104; src/cli.ts 361; src/platform-runtime.ts 31 (the ADR-0019 composition root); src/daemon/session-teardown.ts 89. The hub set includes the five #1969 moved off the wide façades (dispatch, capabilities, registry, platform-execution-entry, register-builtins).

Violations name a bounded offending route

eagerClosureGraphOf records which file first pulled each module in. A failure prints the entry's heaviest direct edges — ranked by how many modules enter through each, capped at 4 edges and 2 representative routes apiece — so a newly added import is the first thing on screen instead of one chain per evaluated module (which would be 361 lines at src/cli.ts scale):

Where the weight comes from (heaviest direct edges, capped -- this attributes by shortest
import route, it does not diff against a recorded baseline):
  packages/platform-apple/src/index.ts -- 58 module(s) enter through this edge:
    src/platform-runtime.ts
      -> packages/platform-apple/src/index.ts
      -> packages/platform-apple/src/runtime.ts
      -> packages/platform-apple/src/logs/runtime.ts
      ...
  (+2 more direct edge(s), smaller)

What it does not do is diff against a recorded baseline: this gate persists each entry's module count, not its module identity, so it attributes by shortest import route rather than naming which modules are new. For the common case — a new import on the entry itself — that edge is new, its whole subtree is attributed to it, and it sorts first. For a regression deep inside an already-large subtree it names the top-level edge containing the growth. Storing a true delta would mean checking in ~1,500 module paths and rewriting them on every contracts refactor; the docstring states this trade explicitly rather than implying precision the data does not support.

The walk is breadth-first, so every printed chain is the shortest route to that file. Memoizing per-file edges also cut the existing pins' runtime (cli-startup-import-closure 2336ms → ~550ms).

Every rule has a failing-direction test

A real tree that satisfies its pins cannot distinguish a correct rule from a vacuous one, so each rule the gate rests on carries its own planted-red test: the equality ratchet (sabotage: ===<=), the bounded attribution (cap 4 → 999), recursive discovery (recursive → one-level, against a temp fixture tree with a nested façade), and row uniqueness (same path in both records; plus TS1117 for a duplicate key within one). All four were observed red and reverted.

Per the lane rule

Planted-red proof (reverted before commit)

Replaced the lazy await import('./runtime.ts') in packages/platform-apple/src/index.ts with a static namespace import — the flagship regression this gate exists to catch. Three tests fired:

× 'packages/platform-apple/src/index.ts' evaluates at most 1 modules
  → evaluates 65 modules on import, over its budget of 1.

× 'packages/platform-apple/src/index.ts' never evaluates a concrete platform implementation
  → packages/platform-apple/src/index.ts
        -> packages/platform-apple/src/runtime.ts
        -> packages/platform-apple/src/logs/runtime.ts
        -> packages/platform-apple/src/logs/backend.ts

× 'src/platform-runtime.ts' evaluates at most 31 modules
  → evaluates 80 modules on import, over its budget of 31.

The third is the composition root picking up the blast radius through its own chain — the cascade a flat offender set would not have explained. Sabotage reverted; the diff contains only the three intended files.

On #1969

#1969 is merged and this branch is rebased on it. Its contracts-entry-closure.test.ts is left in place: both of its tests state properties this table cannot (a specific hub must not reach two named façades; no source file value-imports them — a whole-tree scan, not an entry walk). The five hubs it names are now also budgeted here, so they carry both the specific pin and the general ceiling.

Validation

  • pnpm vitest run --project unit-core on the new file — 206/206 passing in ~700ms.
  • Together with the three existing closure pins — 230/230 passing, no slow-test gate output.
  • pnpm typecheck — clean.
  • pnpm check:layering — 181/181 passing (R11's façade gate now runs on the shared facadeEntryFiles); R11 reports 95 exported subpaths.
  • pnpm check:fallow — no issues in the changed files.
  • pnpm check:affected --run — all runnable checks passed.
  • Planted-red proof performed and reverted (above).

Closes #1960

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.39 MB 2.39 MB 0 B
JS gzip 801.1 kB 801.1 kB 0 B
npm tarball 923.3 kB 923.3 kB 0 B
npm unpacked 3.20 MB 3.20 MB 0 B

npm unpacked components

Component Base Current Diff
JS / dist source 2.54 MB 2.54 MB 0 B
Apple runner source/project 564.3 kB 564.3 kB 0 B
macOS helper source 54.5 kB 54.5 kB 0 B
Android helper artifacts 0 B 0 B 0 B
Other package files 44.5 kB 44.5 kB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.2 ms 29.8 ms +1.6 ms
CLI --help 83.4 ms 84.3 ms +0.9 ms

Top changed chunks: no changes in the largest emitted chunks.

Top changed packed files

No changed packed files.

@thymikee

Copy link
Copy Markdown
Member Author

Not ready at 0352dba3 despite green CI:

  1. discoverFacadeFiles() scans only packages/*/src/facades/*.ts, while the repository's actual workspace-façade gate starts from every package manifest export target and then adds /src/facades/. This misses platform package entry façades such as packages/platform-*/src/index.ts, so the generalized ADR-0019 loading-shape claim is incomplete and the body’s “same discovery” statement is false.
  2. The budgets are seeded before perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump #1969. Merging both unchanged leaves stale slack (platform 42/48, interaction 25/30) large enough to reintroduce the clump perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump #1969 removes. Rebase/stack after perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump #1969 and reseed tight budgets.
  3. test(structure): per-package eager-closure budgets — the ADR-0019 loading-shape probe (#1739) #1960 asks for the offending edge chain; current failures print only a sorted closure/offender set, which does not identify the transitive import path.

Please derive façade roots from manifest ownership, add predecessor-chain diagnostics, and reseed post-#1969 with minimal headroom.

thymikee added a commit that referenced this pull request Aug 22, 2026
… reseed tight

Review findings on #1965:

1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every
   package that publishes its entry surface straight from the manifest —
   including all six `packages/platform-*/src/index.ts` façades, the exact
   subject of ADR-0019's implementation-laziness rule. Discovery now derives
   from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/`
   files, reusing the R11 helper rather than reimplementing it so the two gates
   cannot disagree about what an entry surface is. The table grows from 14
   façades + 2 hubs to 95 entry surfaces + 8 hubs.

2. Budgets carried a few files of slack each. They are now exact ratchets with
   no headroom, matching how the repo pins R9/R10 and test-file size: growth is
   allowed, it just has to be a visible number change in the diff of the PR that
   causes it. Every budget is reseeded from post-#1969 measurement.

3. Violations printed a flat sorted set, which named the offender but not the
   route. `eagerClosureGraphOf` records each file's discoverer, so failures now
   print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps
   its contract and is expressed in terms of the new walk; per-file edges are
   memoized, which also cuts the existing pins' runtime (cli closure test
   2336ms -> ~550ms).

The platform-package façades evaluate exactly one module each — themselves —
so their budget of 1 is the tightest statement of "metadata-eager,
implementation-lazy" the walker can make.
@thymikee
thymikee force-pushed the test/1960-eager-closure-budgets branch from 0352dba to 55d4846 Compare August 22, 2026 13:50
@thymikee

Copy link
Copy Markdown
Member Author

All three addressed in 55d4846, rebased onto 03c398406 (#1969 merged). PR body corrected too — the "same discovery" claim was wrong as written, and I've replaced it rather than softened it.

1. Discovery now derives from manifest ownership

You were right that this was a correctness gap and not just a wording problem. discoverFacadeEntryFiles now starts from readWorkspacePackages(...).exportTargets and then adds /src/facades/ files — the same composition R11 uses, and importing the same helper rather than reimplementing it, so the two gates cannot drift into disagreeing about what an entry surface is.

The omission was worse than "incomplete": all six packages/platform-*/src/index.ts façades have no facades/ directory at all, so the previous scan never looked at the exact files ADR-0019's implementation-laziness rule is about. The gate claimed to prove the loading shape while missing its actual subject.

What they measure is the payoff — each platform façade evaluates exactly one module: itself. Metadata inline, contract imports all import type (erased), every implementation behind a function-scoped await import. So those six budgets are 1, which is the tightest statement of "metadata-eager and implementation-lazy" the walker can make. There's a dedicated non-vacuity test asserting all six are discovered, so the gate can't quietly lose its subject again.

Table grew 14 façades + 2 hubs → 95 entry surfaces + 8 hubs.

2. Budgets reseeded post-#1969, with zero headroom

Reseeded from measurement on 03c398406, and I dropped the headroom concept entirely rather than tightening it — budgets are now exact ratchets asserted as <=. That matches how the repo already pins R9, R10, and test-file size, and docs/agents/testing.md's "existing pins only shrink; a new pin requires measured justification".

Your point about slack not being neutral is the reason: the regression class is a single static import dragging a subtree in, #1969 measured it at 5–12% of suite import work per instance, and "a few files" of spare room silently absorbs the small end of exactly that. Growth is still fine — it just has to show up as a number change in the diff of the PR causing it.

facades/platform.ts is now pinned at its true 42 and interaction.ts at 25 (both unchanged by #1969 — it moved consumers off them rather than shrinking them). src/cli.ts 386 → 361 and session-teardown.ts 121 → 89 did move, which is exactly the stale-seed problem you flagged.

I also added the five hubs #1969 moved off the wide façades (dispatch, capabilities, registry, platform-execution-entry, register-builtins) plus src/platform-runtime.ts, the composition root.

3. Edge chains, not offender sets

Added eagerClosureGraphOf to the shared fixture — a predecessor-recording BFS — and failures now print the transitive route:

packages/platform-apple/src/index.ts
    -> packages/platform-apple/src/runtime.ts
    -> packages/platform-apple/src/logs/runtime.ts
    -> packages/platform-apple/src/logs/backend.ts

Breadth-first, so each chain is the shortest route to that file. eagerClosureOf keeps its contract and is now expressed in terms of the graph walk; iteration order went depth- → breadth-first, which nothing depended on (documented on the function, and the three existing pins verify it).

I also memoized per-file edges while I was in there, since ~100 entries share most of their subtrees. Side benefit: cli-startup-import-closure went 2336ms → ~550ms, and the whole 201-test budget file runs in ~430ms — comfortably inside the 2.5s per-test unit budget you warned about. My scan walks closures from entries rather than parsing the whole tree, so it doesn't need the text-filter your #1969 test does.

Planted red, re-run

Replaced the lazy await import('./runtime.ts') in packages/platform-apple/src/index.ts with a static namespace import. Three tests fired:

× 'packages/platform-apple/src/index.ts' evaluates at most 1 modules
  → evaluates 65 modules on import, over its budget of 1.

× 'packages/platform-apple/src/index.ts' never evaluates a concrete platform implementation
  → (chain above)

× 'src/platform-runtime.ts' evaluates at most 31 modules
  → evaluates 80 modules on import, over its budget of 31.

The third is the composition root picking up the blast radius through its own chain — the cascade a flat offender set wouldn't have explained, which is a fair argument for your finding 3. Sabotage reverted; diff is three files.

One thing I did not do

Left contracts-entry-closure.test.ts in place. Both its tests state properties this table can't: a named hub must not reach two specific façades, and no source file value-imports them — a whole-tree scan rather than an entry walk. Its five hubs are now budgeted here too, so they carry both the specific pin and the general ceiling. Happy to fold it in if you'd rather have one mechanism, but it looked like a genuine loss of coverage rather than duplication.

CI green (27/27).

@thymikee

Copy link
Copy Markdown
Member Author

Not ready at 55d48467. The rebase/reseed, manifest targets, and predecessor chains fix important prior gaps, but four correctness holes remain in the gate itself: (1) graph.size <= budget does not enforce an exact ratchet—after any shrink it leaves silent headroom; require equality or fail under-budget rows too. (2) Numeric over-budget failures dump chains for every closure module, not the newly offending edge; record baseline identity/edges or emit a bounded delta-oriented route. (3) discovery still duplicates only immediate src/facades/*.ts scanning while R11 walks recursively; reuse one canonical discovery owner so nested façades cannot drift out. (4) Set-based exhaustiveness collapses duplicate budget rows; explicitly detect duplicate ids/paths. Exact-head CI is green and the slice is test-only, but these gaps allow the structural proof to pass vacuously or with stale slack.

@thymikee

Copy link
Copy Markdown
Member Author

The primary static-import planted red is credible and CI is green, but the generalized gate still has four correctness holes:

  1. Rows are documented as exact ratchets, but the assertion is graph.size <= budget; a shrink silently creates future headroom. Require equality or fail under-budget rows until deliberately reseeded.
  2. An over-budget failure prints chains for every evaluated module (hundreds for src/cli.ts) rather than the new offending delta. Persist enough baseline identity/edge data to report a bounded offending route.
  3. discoverFacadeEntryFiles reimplements a one-level src/facades scan, while canonical R11 discovery is recursive. Nested façade files can be covered by R11 yet silently omitted here; reuse one discovery owner.
  4. Converting budget rows to Sets hides duplicate ids/entry paths despite the “exactly one” claim. Assert duplicates explicitly or encode uniqueness structurally.

Please add planted-red/non-vacuity tests for these failure modes and correct the PR text that currently claims exactness and shared R11 discovery.

thymikee added a commit that referenced this pull request Aug 22, 2026
… reseed tight

Review findings on #1965:

1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every
   package that publishes its entry surface straight from the manifest —
   including all six `packages/platform-*/src/index.ts` façades, the exact
   subject of ADR-0019's implementation-laziness rule. Discovery now derives
   from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/`
   files, reusing the R11 helper rather than reimplementing it so the two gates
   cannot disagree about what an entry surface is. The table grows from 14
   façades + 2 hubs to 95 entry surfaces + 8 hubs.

2. Budgets carried a few files of slack each. They are now exact ratchets with
   no headroom, matching how the repo pins R9/R10 and test-file size: growth is
   allowed, it just has to be a visible number change in the diff of the PR that
   causes it. Every budget is reseeded from post-#1969 measurement.

3. Violations printed a flat sorted set, which named the offender but not the
   route. `eagerClosureGraphOf` records each file's discoverer, so failures now
   print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps
   its contract and is expressed in terms of the new walk; per-file edges are
   memoized, which also cuts the existing pins' runtime (cli closure test
   2336ms -> ~550ms).

The platform-package façades evaluate exactly one module each — themselves —
so their budget of 1 is the tightest statement of "metadata-eager,
implementation-lazy" the walker can make.
thymikee added a commit that referenced this pull request Aug 22, 2026
…le-owner

Second review pass on #1965 found four holes, two of which were places the
PR text claimed a property the code did not have.

1. Rows were documented as exact ratchets but asserted with `<=`, so a shrink
   silently became headroom a later regression could grow back into. The
   comparison is now equality, in a pure `classifyBudget` with a separate
   message for each direction ("lower its pin to N in this PR so the ratchet
   keeps the gain"), matching test-file-size-ratchet.ts and the R9/R10 pins.

2. An over-pin failure printed a chain per evaluated module — 361 of them for
   src/cli.ts. It now prints a bounded attribution: the entry's heaviest direct
   edges (capped at 4) with a couple of representative deep routes each, ranked
   so a newly added import sorts first. The comment states plainly that this
   attributes by shortest import route and does NOT diff against a recorded
   baseline; naming a true delta would mean checking in ~1,500 module paths and
   rewriting them on every contracts refactor.

3. Discovery reimplemented a one-level `src/facades` scan while canonical R11
   discovery is recursive, so a nested façade file could be covered by R11 and
   silently missing here. `facadeEntryFiles` is now a single exported owner in
   package-boundaries.ts that both R11's façade gate and this table consume.

4. Rows were converted to Sets before any uniqueness check, so a duplicate was
   unobservable. The table is now two `Record<string, number>` literals keyed by
   path, making an in-record duplicate a TypeScript error (ts1117); the only
   remaining case — one path in both records — is asserted on the array.

Each of the four holes gets a test that fails when the rule is broken, since a
tree that happens to satisfy its pins cannot distinguish a correct rule from a
vacuous one. Writing those found a real bug in the duplicate check itself
(`Set.add` returns the Set, so the filter never matched).

Pins reseeded on 04e4c23.
@thymikee
thymikee force-pushed the test/1960-eager-closure-budgets branch from 55d4846 to 6d337d2 Compare August 22, 2026 16:35
@thymikee

Copy link
Copy Markdown
Member Author

All four fixed in 6d337d27f, rebased onto 04e4c23b9. Taking the two false-claim ones first, since you're right that those are the worse category — I wrote "exact ratchets" and "same discovery as R11" over code that did neither.

1. "Exact ratchet" is now actually equality

It was graph.size <= budget. So a shrink passed silently and converted the gain into headroom, which is the opposite of a ratchet — exactly what you said.

Now classifyBudget(id, actual, budget) is a pure comparator asserting equality, with a separate message per direction, modelled on test-file-size-ratchet.test.ts:

x.ts evaluates 40 modules on import, pinned at 42. It shrank -- lower its pin to 40 in this
PR so the ratchet keeps the gain instead of leaving headroom a later regression could grow
back into.

Pulling the rule into a pure function is what makes it testable in its failing direction, which matters here: over the real tree, <= and === are indistinguishable while every entry happens to match its pin.

2. Bounded attribution instead of a chain dump

You're right that 361 chains for src/cli.ts is not a diagnostic. Failures now print the entry's heaviest direct edges — ranked by how many modules enter through them, capped at 4 edges × 2 routes — so the offending import is the first thing on screen. From the re-run planted red on the composition root:

src/platform-runtime.ts evaluates 80 modules on import, pinned at 31. ...

Where the weight comes from (heaviest direct edges, capped -- this attributes by shortest
import route, it does not diff against a recorded baseline):
  packages/platform-apple/src/index.ts -- 58 module(s) enter through this edge:
    src/platform-runtime.ts
      -> packages/platform-apple/src/index.ts
      -> packages/platform-apple/src/runtime.ts
      -> packages/platform-apple/src/logs/runtime.ts
      -> packages/capture-kit/src/index.ts
      ...
  src/platform-runtime-gateway.ts -- 8 module(s) enter through this edge:
    ...
  (+2 more direct edge(s), smaller)

One place I did not follow the direction, and want your call. You asked me to persist per-row module identity so the failure can name the newly added modules. I didn't, and the docstring says so in those words rather than implying a delta it can't compute. The reason: naming a true delta means checking in ~1,500 module paths (that's the sum of all 103 closures) and rewriting them on every contracts refactor — a second source of truth with real churn, right after #1969 moved 259 imports. What I shipped attributes by shortest import route instead, which lands on the same answer for the common case (a new import on the entry is a new edge, so its whole subtree is attributed to it and it sorts first) and degrades to "this top-level edge contains the growth" for a regression deep inside an existing subtree.

If you'd rather have the exact delta and accept the baseline file, say so and I'll add it — the walker already produces the data, it's a fallow-baselines/-shaped artifact.

3. One discovery owner, and it's the recursive one

facadeEntryFiles(repoRoot) now lives in scripts/layering/package-boundaries.ts next to readWorkspacePackages, and both R11's façade gate and the budget table call it. My module just re-exports it. The recursive walkTsFiles it uses is the one already in that file, filtered to production sources so facades/x.test.ts doesn't become an entry surface.

R11's façade test is 13/13 green on the shared function, and the full layering suite is 181/181.

4. Duplicates are now unwritable, not merely unasserted

The table is two Readonly<Record<string, number>> literals keyed by entry path. A repeated key inside one is a TypeScript error, so it can't be committed — verified by planting one:

src/__tests__/eager-closure-budgets.ts(236,3): error TS1117: An object literal cannot have
multiple properties with the same name.

The one duplicate still expressible is the same path in both records, and that's asserted on the array before any Set.

Planted red for each of the four

Added a failing-direction test per hole, since "a hole no test can fail" was your objection:

Sabotage Test that fired
===<= the ratchet fails an entry that SHRANK, not only one that grew
REPORTED_EDGES 4 → 999 closure pressure is attributed to the heaviest direct edges and is bounded
recursive → one-level scan discovery is recursive, so a NESTED facade file cannot hide from the gate
same path in both records no entry path is budgeted twice, checked before any Set could absorb it
duplicate key in one record pnpm typecheck (TS1117)

Each was observed red, then reverted. The nested-façade one runs against a fixture tree in a temp dir, so it holds even though the real repo has no nested façade today.

Worth flagging: writing #4's sabotage found a bug in my own check — I had ids.filter((id) => !seen.add(id)), and Set.add returns the Set, so the filter never matched anything and the test passed with a duplicate present. It only surfaced because I planted the duplicate instead of trusting the green. Fixed to an explicit loop.

Also

describeClosurePressure has a case for entries that evaluate only themselves — the six platform façades — so they print "(no eager edges: this entry evaluates only itself)" rather than an empty section.

Validation

pnpm typecheck, pnpm check:layering (181/181), pnpm check:fallow, pnpm run lint, pnpm run format:check, and pnpm check:affected --run all pass. The budget file is 206 tests in ~700ms; together with the three existing closure pins, 230 tests with no slow-test gate output.

CI: one Smoke Tests shard failed on the live iOS simulator scenario (click id="automation-close-sheet"selector_not_found in smoke:automation-input), which is a live-device lane flake unrelated to this diff — the branch changes no production code at all, only src/__tests__/ and scripts/layering/. I've re-run that job; will confirm when it lands.

@thymikee

thymikee commented Aug 22, 2026

Copy link
Copy Markdown
Member Author

CI is green — 28/28.

The Smoke Tests (iOS) failure I flagged was indeed the live-device lane, confirmed by re-run: it failed twice with different symptoms (click id="automation-close-sheet"selector_not_found, then typed email suffix should be observable with the field holding a.test — a truncated/dropped-keystroke value), then passed unchanged on the third attempt. The same lane failed and passed on claude/agent-device-request-bound-migrat in the same window, and this branch changes no production code at all (src/__tests__/ and scripts/layering/ only; Bundle Size reports 0 B).

For the record, the new gate in CI: 206 tests, 1595 ms for the whole file, no slow-test gate output.

@thymikee

Copy link
Copy Markdown
Member Author

Not ready at 6d337d27 despite green CI. The equality ratchet, shared recursive owner, duplicate enforcement, performance, and decision not to persist ~1,500 module-identity paths are sound. Two gate-design issues remain:

  1. facadeEntryFiles() changed R11 from tracked-source discovery (listSourceFiles()) to a raw filesystem walk and accepts every production-looking src/facades/**/*.ts; the temp test explicitly proves untracked files are discovered. Layering scans are required to read tracked files only, so a host-local scratch facade can now fail both R11 and the budget gate. Keep one recursive owner, but feed it tracked production paths; plant a git fixture with a tracked nested facade plus an untracked sibling that must be ignored.
  2. Only numeric-pressure diagnostics are bounded. The platform-implementation assertion emits every offender plus a full chain for every path, so one eager platform subtree can produce hundreds of paths/chains. Reuse capped owning-edge/representative-route output with an omitted count and add a wide planted-red fixture.

Exact-head CI is 28/28 green and package growth is 0 B.

thymikee added a commit that referenced this pull request Aug 24, 2026
… reseed tight

Review findings on #1965:

1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every
   package that publishes its entry surface straight from the manifest —
   including all six `packages/platform-*/src/index.ts` façades, the exact
   subject of ADR-0019's implementation-laziness rule. Discovery now derives
   from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/`
   files, reusing the R11 helper rather than reimplementing it so the two gates
   cannot disagree about what an entry surface is. The table grows from 14
   façades + 2 hubs to 95 entry surfaces + 8 hubs.

2. Budgets carried a few files of slack each. They are now exact ratchets with
   no headroom, matching how the repo pins R9/R10 and test-file size: growth is
   allowed, it just has to be a visible number change in the diff of the PR that
   causes it. Every budget is reseeded from post-#1969 measurement.

3. Violations printed a flat sorted set, which named the offender but not the
   route. `eagerClosureGraphOf` records each file's discoverer, so failures now
   print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps
   its contract and is expressed in terms of the new walk; per-file edges are
   memoized, which also cuts the existing pins' runtime (cli closure test
   2336ms -> ~550ms).

The platform-package façades evaluate exactly one module each — themselves —
so their budget of 1 is the tightest statement of "metadata-eager,
implementation-lazy" the walker can make.
thymikee added a commit that referenced this pull request Aug 24, 2026
…le-owner

Second review pass on #1965 found four holes, two of which were places the
PR text claimed a property the code did not have.

1. Rows were documented as exact ratchets but asserted with `<=`, so a shrink
   silently became headroom a later regression could grow back into. The
   comparison is now equality, in a pure `classifyBudget` with a separate
   message for each direction ("lower its pin to N in this PR so the ratchet
   keeps the gain"), matching test-file-size-ratchet.ts and the R9/R10 pins.

2. An over-pin failure printed a chain per evaluated module — 361 of them for
   src/cli.ts. It now prints a bounded attribution: the entry's heaviest direct
   edges (capped at 4) with a couple of representative deep routes each, ranked
   so a newly added import sorts first. The comment states plainly that this
   attributes by shortest import route and does NOT diff against a recorded
   baseline; naming a true delta would mean checking in ~1,500 module paths and
   rewriting them on every contracts refactor.

3. Discovery reimplemented a one-level `src/facades` scan while canonical R11
   discovery is recursive, so a nested façade file could be covered by R11 and
   silently missing here. `facadeEntryFiles` is now a single exported owner in
   package-boundaries.ts that both R11's façade gate and this table consume.

4. Rows were converted to Sets before any uniqueness check, so a duplicate was
   unobservable. The table is now two `Record<string, number>` literals keyed by
   path, making an in-record duplicate a TypeScript error (ts1117); the only
   remaining case — one path in both records — is asserted on the array.

Each of the four holes gets a test that fails when the rule is broken, since a
tree that happens to satisfy its pins cannot distinguish a correct rule from a
vacuous one. Writing those found a real bug in the duplicate check itself
(`Set.add` returns the Set, so the filter never matched).

Pins reseeded on 04e4c23.
thymikee added a commit that referenced this pull request Aug 24, 2026
…atform diagnostics

Two findings from the third #1965 review.

1. R11 REGRESSION, introduced by the previous commit in this PR. Extracting one
   recursive facade-discovery owner was right, but it was implemented as a raw
   filesystem walk, which silently changed R11 from tracked-source discovery
   (listSourceFiles) to "whatever is on disk". A layering gate describes
   committed state, so an uncommitted scratch facade under a scanned path must
   be invisible; instead it could fail both R11 and the budget gate on a
   contributor's working directory. Worse, the test asserted the new behaviour,
   pinning the regression in place.

   The tracked-source enumerator now lives in a leaf module,
   scripts/layering/tracked-sources.ts, that both check.ts and
   package-boundaries.ts consume (check.ts imports package-boundaries.ts, so the
   dependency cannot run the other way). facadeEntryFiles reads
   listTrackedProductionSources, keeping the single recursive owner while
   restoring tracked-only scope.

   The temp-dir test is replaced by a real git fixture following
   platform-package-repository.test.ts: a tracked nested facade that must be
   discovered, plus untracked siblings — one beside it and one beside the nested
   file — that must not be. Reverting to the filesystem walk turns it red.

2. Only the numeric-pressure diagnostic was bounded; the platform-implementation
   assertion still emitted every offender with a full chain, so one eager
   platform subtree buried the single import that caused it under hundreds of
   lines. Both diagnostics now share one renderer that groups targets by the
   entry's own import they arrived through, caps edges and routes, and states
   the omitted counts. A wide planted-red fixture (300 offenders under one
   owning edge plus extra edges) proves the cap engages rather than assuming it.

src/cli.ts reseeded 361 -> 362 for drift from the rebase onto 7aaa559.
@thymikee
thymikee force-pushed the test/1960-eager-closure-budgets branch from 6d337d2 to 7815272 Compare August 24, 2026 08:14
@thymikee

Copy link
Copy Markdown
Member Author

Both fixed in 781527274, rebased onto 7aaa559e2.

1. This was an R11 regression I introduced, not a new-gate nit

Stating that plainly because the framing matters: extracting one recursive discovery owner was the right move, but I implemented it as a raw walkTsFiles filesystem walk. That silently changed R11 itself from tracked-source discovery (listSourceFiles()) to "whatever is on disk" — so an uncommitted scratch facades/experiment.ts could fail R11 and the budget gate on someone's working directory, for a gate whose entire job is to describe committed state. And my test asserted the new behaviour, so the regression was pinned in the wrong direction. That's the worst part: it wasn't just wrong, it was defended.

The fix keeps the single owner and feeds it tracked paths. check.ts already imports package-boundaries.ts, so the dependency can't run the other way; the enumerator therefore moved to a leaf module, scripts/layering/tracked-sources.ts:

listTrackedTypeScriptFiles(repoRoot)   // git ls-files, same pathspecs as before
isProductionSourceFile(file)
listTrackedProductionSources(repoRoot) // the canonical layering scan input

check.ts's listTypeScriptFiles() / listSourceFiles() now delegate to it (same zero-arg signatures, existing callers untouched), and facadeEntryFiles reads listTrackedProductionSources(repoRoot). One owner, recursive, tracked-only, and parameterised by repo root so it's testable.

The temp-dir test is gone, replaced by a real git fixture following scripts/layering/platform-package-repository.test.tsgit init, commit, then write untracked files:

  • packages/demo/src/facades/nested/deep.ts — tracked, nested → must be discovered
  • packages/demo/src/facades/scratch.ts — untracked → must be ignored
  • packages/demo/src/facades/nested/scratch.ts — untracked, sitting right beside the tracked nested façade → must be ignored
  • packages/demo/src/facades/skip.test.ts — tracked test source → not an entry surface

Planted red: reverting facadeEntryFiles to the filesystem walk fails it on exactly the right assertion —

AssertionError: an untracked scratch façade is not committed state:
  expected [ 'packages/demo/src/entry.ts', …(4) ] to not include
  'packages/demo/src/facades/scratch.ts'

R11 is green on the shared function (pnpm check:layering 181/181, 1374 source files).

2. Platform-implementation diagnostics now share the bounded renderer

You're right that bounding only the numeric side was half a fix. Both diagnostics now go through one renderOwningEdges, which groups targets by the entry's own import they arrived through — the edge you'd actually make lazy — caps at 4 edges × 2 representative routes, and states what it dropped. Re-running the live planted red (static-importing ./runtime.ts in platform-apple/src/index.ts):

packages/platform-apple/src/index.ts evaluates 18 concrete platform implementation module(s)
before discovery or binding selects an owner ... Move the owning edge behind a
function-scoped `await import`.
Where they come in (capped):
  packages/platform-apple/src/runtime.ts -- 18 platform implementation module(s) under this edge:
    packages/platform-apple/src/index.ts
      -> packages/platform-apple/src/runtime.ts
      -> packages/platform-apple/src/logs/runtime.ts
      -> packages/platform-apple/src/logs/doctor.ts
      -> packages/platform-apple/src/logs/coredevice-console.ts
    ...
    (+16 more platform implementation module(s) under this edge)

Previously that was 18 offenders × a full chain each; on a wider subtree it would have been hundreds.

The wide fixture you asked for: 300 offenders under one owning edge, plus extra edges to force the group tail. It asserts the omitted counts appear (+299 more … under this edge, +3 more owning edge(s), 3 …), that the output stays under 30 lines, and that a mid-subtree module is not printed. Setting either cap to 9999 turns it red.

One thing the fixture taught me: my first version chained the 300 modules linearly, and owningEdgeOf has a 64-hop guard, so the deep ones fell out of their group. Rebuilt broad-and-shallow, which is also the shape a real platform subtree has.

Also

formatImportChain is no longer exported (both callers are now in-module) — fallow caught it.

Validation

pnpm typecheck, pnpm check:layering (181/181), pnpm check:fallow, pnpm run lint, pnpm run format:check, pnpm check:affected --run — all pass. 207 tests in the budget file, 231 with the three existing closure pins, no slow-test gate output (the git fixture is the slowest test at ~630 ms, well inside the 2.5 s unit budget).

src/cli.ts reseeded 361 → 362 for drift from the rebase.

@thymikee

Copy link
Copy Markdown
Member Author

CI green — 28/28.

One Smoke Tests shard (Android emulator, not iOS this time) failed on wait text "Alert result: cancelled" timing out after 10 s in the live automation scenario, then passed unchanged on re-run. Unlike the earlier iOS case the Android lane had been green on other branches all morning, so I re-ran rather than calling it a flake up front — but the branch still contains zero production code (scripts/layering/ and src/__tests__/ only, Bundle Size 0 B), so there is no mechanism by which it could affect a live emulator's UI timing.

On the check you'd expect to be slow to report: Mutation Affected does not appear on this PR, and that is correct rather than pending — the workflow is path-gated to scripts/mutation/**, scripts/lib/**, stryker.config.json, and its own file, none of which this diff touches.

In CI: 207 tests in the budget file, 1477 ms for the whole file, no slow-test gate output.

@thymikee

Copy link
Copy Markdown
Member Author

Request changes: the tracked-only discovery fix is incomplete for manifest-derived entry surfaces. facadeEntryFiles() intersects src/facades/ discovery with listTrackedProductionSources(), but it adds every export target from readWorkspacePackages() first; that helper reads raw on-disk packages/*/package.json, and the final existsSync admits an untracked package or an untracked target newly named by a dirty manifest. That can make R11 and the exhaustive budget gate fail on scratch work even though the gate claims to describe committed state. The planted-red fixture only covers untracked files under src/facades/, so it misses this exact path. Intersect manifest-derived targets with the canonical tracked production set (and preferably restrict manifests to tracked package manifests), then add a fixture for an untracked manifest export target. The exact ratchet, shared R11 discovery, bounded diagnostics, and size rationale otherwise look sound; all current checks are green.

ADR-0019 requires platform-package façades to stay implementation-lazy
and is explicit that a startup threshold alone is not a substitute for
preserving the loading shape. #1950 built the AST-level walker
(eager-import-closure.fixtures.ts) and proved the planted-red
procedure on one file (session-teardown.ts's android-helper
denylist); this generalizes it into a data-driven budget table so any
workspace-package façade -- or a designated hub module -- can get an
eager-closure ceiling without a bespoke test.

Seeds a budget for every packages/*/src/facades/*.ts file (discovered
the same way package-boundaries.test.ts discovers façades, not
hand-listed) from its measured current closure size, plus a
platform-implementation denylist for façades whose contract is
implementation-neutral vocabulary. Also demonstrates the mechanism on
two designated hub modules (cli.ts, session-teardown.ts) alongside
their existing, more specific ad hoc pins.

Closes #1960
… reseed tight

Review findings on #1965:

1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every
   package that publishes its entry surface straight from the manifest —
   including all six `packages/platform-*/src/index.ts` façades, the exact
   subject of ADR-0019's implementation-laziness rule. Discovery now derives
   from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/`
   files, reusing the R11 helper rather than reimplementing it so the two gates
   cannot disagree about what an entry surface is. The table grows from 14
   façades + 2 hubs to 95 entry surfaces + 8 hubs.

2. Budgets carried a few files of slack each. They are now exact ratchets with
   no headroom, matching how the repo pins R9/R10 and test-file size: growth is
   allowed, it just has to be a visible number change in the diff of the PR that
   causes it. Every budget is reseeded from post-#1969 measurement.

3. Violations printed a flat sorted set, which named the offender but not the
   route. `eagerClosureGraphOf` records each file's discoverer, so failures now
   print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps
   its contract and is expressed in terms of the new walk; per-file edges are
   memoized, which also cuts the existing pins' runtime (cli closure test
   2336ms -> ~550ms).

The platform-package façades evaluate exactly one module each — themselves —
so their budget of 1 is the tightest statement of "metadata-eager,
implementation-lazy" the walker can make.
…le-owner

Second review pass on #1965 found four holes, two of which were places the
PR text claimed a property the code did not have.

1. Rows were documented as exact ratchets but asserted with `<=`, so a shrink
   silently became headroom a later regression could grow back into. The
   comparison is now equality, in a pure `classifyBudget` with a separate
   message for each direction ("lower its pin to N in this PR so the ratchet
   keeps the gain"), matching test-file-size-ratchet.ts and the R9/R10 pins.

2. An over-pin failure printed a chain per evaluated module — 361 of them for
   src/cli.ts. It now prints a bounded attribution: the entry's heaviest direct
   edges (capped at 4) with a couple of representative deep routes each, ranked
   so a newly added import sorts first. The comment states plainly that this
   attributes by shortest import route and does NOT diff against a recorded
   baseline; naming a true delta would mean checking in ~1,500 module paths and
   rewriting them on every contracts refactor.

3. Discovery reimplemented a one-level `src/facades` scan while canonical R11
   discovery is recursive, so a nested façade file could be covered by R11 and
   silently missing here. `facadeEntryFiles` is now a single exported owner in
   package-boundaries.ts that both R11's façade gate and this table consume.

4. Rows were converted to Sets before any uniqueness check, so a duplicate was
   unobservable. The table is now two `Record<string, number>` literals keyed by
   path, making an in-record duplicate a TypeScript error (ts1117); the only
   remaining case — one path in both records — is asserted on the array.

Each of the four holes gets a test that fails when the rule is broken, since a
tree that happens to satisfy its pins cannot distinguish a correct rule from a
vacuous one. Writing those found a real bug in the duplicate check itself
(`Set.add` returns the Set, so the filter never matched).

Pins reseeded on 04e4c23.
…atform diagnostics

Two findings from the third #1965 review.

1. R11 REGRESSION, introduced by the previous commit in this PR. Extracting one
   recursive facade-discovery owner was right, but it was implemented as a raw
   filesystem walk, which silently changed R11 from tracked-source discovery
   (listSourceFiles) to "whatever is on disk". A layering gate describes
   committed state, so an uncommitted scratch facade under a scanned path must
   be invisible; instead it could fail both R11 and the budget gate on a
   contributor's working directory. Worse, the test asserted the new behaviour,
   pinning the regression in place.

   The tracked-source enumerator now lives in a leaf module,
   scripts/layering/tracked-sources.ts, that both check.ts and
   package-boundaries.ts consume (check.ts imports package-boundaries.ts, so the
   dependency cannot run the other way). facadeEntryFiles reads
   listTrackedProductionSources, keeping the single recursive owner while
   restoring tracked-only scope.

   The temp-dir test is replaced by a real git fixture following
   platform-package-repository.test.ts: a tracked nested facade that must be
   discovered, plus untracked siblings — one beside it and one beside the nested
   file — that must not be. Reverting to the filesystem walk turns it red.

2. Only the numeric-pressure diagnostic was bounded; the platform-implementation
   assertion still emitted every offender with a full chain, so one eager
   platform subtree buried the single import that caused it under hundreds of
   lines. Both diagnostics now share one renderer that groups targets by the
   entry's own import they arrived through, caps edges and routes, and states
   the omitted counts. A wide planted-red fixture (300 offenders under one
   owning edge plus extra edges) proves the cap engages rather than assuming it.

src/cli.ts reseeded 361 -> 362 for drift from the rebase onto 7aaa559.
@thymikee
thymikee force-pushed the test/1960-eager-closure-budgets branch from 7815272 to ccc1e0e Compare August 24, 2026 09:35
The previous commit intersected the `src/facades/` walk with tracked sources
but left manifest-derived entry surfaces bypassing that intersection entirely
— the same bug class, on the path it did not close. Two distinct leaks:

1. `readWorkspacePackages` enumerated `packages/` with `readdirSync`, so an
   uncommitted scratch package contributed a name, export targets, and
   dependency edges to every R11 rule built on it, not just to facade
   discovery. Closed at the source: it now reads tracked manifests via
   `listTrackedPackageManifests`, mirroring how R13's
   `readTrackedPlatformPackageDeclarations` has always enumerated its own.

2. A TRACKED manifest can still name an uncommitted target in its working-tree
   content, which the trailing `existsSync` admitted. `facadeEntryFiles` now
   intersects manifest-derived targets with the tracked production set as well,
   so every path it returns is committed whatever its origin.

Three fixtures, each verified failing before the fix and passing after:

- `readWorkspacePackages reads tracked manifests only` (in R11's own suite,
  since leak 1 affects every rule that reads packages, not only this gate) —
  red when manifest enumeration reverts to `readdirSync`.
- `an untracked PACKAGE contributes no entry surface` — red when both fixes
  are reverted.
- `a dirty manifest naming an UNTRACKED target contributes no entry surface` —
  red when the target intersection is dropped.

The shared git fixture builder is factored out of the existing discovery test
so all four use one committed baseline.

Pins reseeded on 8d280a2: six new contracts entry surfaces from #1993, which
the exhaustiveness gate caught on its own, plus dispatch 100 -> 94 and
platform-runtime 31 -> 36.
@thymikee

Copy link
Copy Markdown
Member Author

Fixed in 2baf7ef9a, rebased onto 8d280a208. You're right, and it's the same bug class as the last round on the path I didn't close: I intersected the src/facades/ walk with tracked sources and never looked at where the other half of the set comes from.

Leak 1 — untracked package directory, closed at the source

readWorkspacePackages enumerated packages/ with readdirSync + existsSync, so an uncommitted scratch package contributed a name, export targets and dependency edges to every rule built on it. That is wider than façade discovery — checkPackageBoundaries, checkRootSites and the workspace-dependency rules all read it, so filtering my gate's output would have left R11 itself failing on scratch work. Taking your "preferably" as the actual fix:

export function readWorkspacePackages(repoRoot: string): WorkspacePackage[] {
  for (const manifestFile of listTrackedPackageManifests(repoRoot).sort()) {  }

listTrackedPackageManifests is git ls-files 'packages/*/package.json' in the shared tracked-sources.ts. R13's readTrackedPlatformPackageDeclarations has always enumerated its manifests exactly this way — R11 was the inconsistent one.

I checked the blast radius before widening it: scripts/mutation/ownership.ts and test-scope.ts also call readWorkspacePackages, but scripts/mutation/run.ts already resolves its repo root with git rev-parse and selects with git diff, so git was already unconditional there. No new dependency.

Leak 2 — tracked manifest, uncommitted target

Restricting manifests doesn't close this one: the manifest file is committed, the working-tree edit names a target that isn't, and existsSync waved it through. So manifest-derived targets are now intersected too:

const tracked = new Set(listTrackedProductionSources(repoRoot));
for (const pkg of readWorkspacePackages(repoRoot)) {
  for (const target of pkg.exportTargets.values()) {
    if (tracked.has(target)) found.add(target);
  }
}

Every path the function returns is committed, whatever produced it.

Fixtures — verified red before the fix, not assumed

Writing these was worth it, because the first version of the untracked-package fixture passed against the un-fixed readdirSync code: the target intersection alone already suppressed it at facadeEntryFiles. That would have shipped a fixture that looked like it covered leak 1 while actually only re-testing leak 2's fix. So the source-level assertion went where the behaviour lives — R11's own suite:

Fixture Sabotage that turns it red
readWorkspacePackages reads tracked manifests only (package-boundaries.test.ts) manifest enumeration back to readdirSync
an untracked PACKAGE contributes no entry surface both fixes reverted
a dirty manifest naming an UNTRACKED target contributes no entry surface target intersection dropped

Each was observed failing on the exact assertion, then restored. The R11 one asserts on package names, so it covers the dependency-edge exposure too, not just export targets. The git fixture builder is factored out so all four discovery tests share one committed baseline.

R11's own behaviour after the change

Confirmed, per your note — pnpm check:layering is 181/181 with 17 workspace packages behind 101 exported subpaths, unchanged from before except for the six new contracts entries #1993 added. On a clean tree tracked manifests and on-disk manifests are the same set, so the only behaviour that changed is the dirty-tree case this was about.

Pins reseeded on 8d280a2

The exhaustiveness gate caught #1993's six new contracts entry surfaces by itself, which is the mechanism working as intended:

+ "packages/contracts/src/back-runtime.ts"
+ "packages/contracts/src/home-runtime.ts"
+ "packages/contracts/src/interactor-operation-catalog.ts"
+ "packages/contracts/src/keyboard-runtime.ts"
+ "packages/contracts/src/orientation-runtime.ts"
+ "packages/contracts/src/tv-remote-runtime.ts"

Also src/core/dispatch.ts 100 → 94 and src/platform-runtime.ts 31 → 36.

Validation

pnpm typecheck, pnpm check:layering (181/181), pnpm check:fallow, pnpm run lint, pnpm run format:check, pnpm check:affected --run — all pass. 221 tests in the budget file, 245 with the three existing closure pins, no slow-test gate output.

One flake worth naming so it isn't mistaken for this change: scripts/node-test-tmpdir.test.ts's killed-owner/live-child case failed once under full-suite contention and passed standalone and on re-run. It is an assertion failure rather than a timeout, so I checked it rather than waving it through — my diff touches no tmpdir machinery.

(Noting #1990 is stacked behind this — happy to land as soon as you're satisfied.)

@thymikee

Copy link
Copy Markdown
Member Author

Clean at 2baf7ef. The prior tracked-only blocker is resolved at both owner seams: workspace discovery now enumerates tracked package manifests, and manifest-derived entry targets are intersected with tracked production sources. The real-git regressions cover both an entirely untracked package and a dirty tracked manifest naming an untracked target; reverting either admission direction makes the corresponding test fail. The design reuses the tracked-source owner and adds no parallel manifest registry.

Deterministic CI is green; Android and iOS smoke are still running. Code review is ready for human.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 24, 2026
@thymikee
thymikee merged commit cc521de into main Aug 24, 2026
28 checks passed
@thymikee
thymikee deleted the test/1960-eager-closure-budgets branch August 24, 2026 10:40
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-24 10:40 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(structure): per-package eager-closure budgets — the ADR-0019 loading-shape probe (#1739)

1 participant