docs(adr): retain the current layering engine (ADR 0032)

[thymikee]

Summary

ADR 0032 records the workstream-2 spike of #3276: retain the current layering engine for R2, R4, R5, R6, R77, R78, R14 and R71. Graph-based checks share one normalized import model whose implementation remains replaceable.

The ADR has the edge-set diff, planted-violation parity (23 plants), runtime and lines-deletable tables for dependency-cruiser 18.5 and fallow 3.32. It separates semantic blockers from migration costs:

• Blocking: dependency-cruiser. Its dedupe mislabels 46 pairs and hides an R77 lazy import.
• Blocking: fallow. With no dynamic-import kind, it flags R5's lazy seams and misses R77 import(). Its rule packs skip orphan files.
• Not blocking: TypeScript <7/swc, diagnostics, config comments, and a path check for the filesystem-placement rules R14/R71.

Both engines also find 9 TS import('x').T pairs that parseImports misses. The fix is tracked in #3293.

Every measurement is pinned to d9959f510 (production tree of 7dda0c2bf). The spike harness lives only there, retrieved with git show d9959f510:scripts/layering/boundary-engine-spike.ts.

2 files (ADR and index row). Closes #3278.

Validation

• Tested commit: 081d869a2.
• pnpm check:affected --run: passed (docs-only diff; no local checks selected). pnpm format:check passed.
• The harness ran at d9959f510 on a clean tree and matches every ADR table; git diff 7dda0c2bf d9959f510 -- src packages is empty.
• Docs only; no runtime behavior changes.

🤖 Generated with Claude Code

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.12 MB	5.12 MB	0 B
Package (unpacked)	5.12 MB	5.12 MB	0 B
Package (download)	1.54 MB	1.54 MB	-3 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	18.9 ms	18.6 ms	-0.3 ms
CLI --help	57.6 ms	57.4 ms	-0.2 ms

[cubic-dev-ai]

7 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/adr/README.md">

<violation number="1" location="docs/adr/README.md:36">
P3: This entry describes replacing the custom rules, but ADR 0032 explicitly keeps them. Change “replacing” to “evaluating replacement of” so the index matches the decision.</violation>
</file>

<file name="docs/adr/0032-layering-graph-engine.md">

<violation number="1" location="docs/adr/0032-layering-graph-engine.md:95">
P3: ADR 0010 (error-system.md) governs the user-facing error contract (code, message, hint, details via normalizeError) and never prescribes a `file:line` layout for layering violations. Custom violations do carry `file` and `line` (see `check.ts`, e.g. `{ file: edge.file, line: edge.line }`), but that format is not "required" by ADR 0010, so the citation will mislead a reader tracing the reference. Drop the citation or point at `check.ts` instead.</violation>

<violation number="2" location="docs/adr/0032-layering-graph-engine.md:106">
P2: The harness computes each median from three measured runs, not five. Change the count to three or update the harness to collect five runs so the reported timing is reproducible.</violation>
</file>

<file name="scripts/layering/boundary-engine-spike.ts">

<violation number="1" location="scripts/layering/boundary-engine-spike.ts:91">
P2: R4 still scans the 13 runner `__tests__` helpers that do not end in `.test.ts`, because the global exclude preserves the whole runner subtree for R77. Exclude test-path modules, including intermediate cycle nodes, from R4 while retaining R77 test coverage.</violation>

<violation number="2" location="scripts/layering/boundary-engine-spike.ts:165">
P2: The engine configuration scans every on-disk source under `src` and `packages`, while the custom side reads tracked files only. An unrelated untracked source can alter engine findings and baselines; fail closed on untracked production files or constrain the engine inputs to the tracked scope.</violation>

<violation number="3" location="scripts/layering/boundary-engine-spike.ts:276">
P2: `run()` discards the engines' `stderr` and, outside the stale-gate path, their exit status. When depcruise or fallow fails — invalid config, tsconfig resolution, missing TypeScript 6 — `stdout` is empty or partial and `JSON.parse` in `depcruise()`/`fallow()` throws a bare `SyntaxError: Unexpected end of JSON input`, hiding the engine's actual diagnostic. Return `stderr` from `run()` and include it (plus `status`) in the parse-failure error so the harness surfaces the underlying engine error.</violation>

<violation number="4" location="scripts/layering/boundary-engine-spike.ts:706">
P3: `writePlants()` dirtied the working tree (new `ws2-plant*` files plus edits to `runner-provider.ts`, `runner-artifact.ts`, `runner-cache.ts`, `daemon-client-lease-beat.ts`) and cleanup only runs via the surrounding try/finally. SIGINT/SIGTERM during the multi-minute timed fallow runs kills the process without unwinding, leaving the edited files modified and the plant files on disk — contradicting the header's "removed ... before the script exits" promise and polluting `git status` in a checkout that also runs the layering gate. Register signal handlers that call `restore()` before exiting.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 3 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[thymikee]

I reviewed 3de1186. The ADR reads well and the code review found no problems, but I did not run the harness, so the edge-set, parity, clean-tree and runtime numbers are the author's re-run and not reproduced here. I also did not confirm that the production tree at d9959f5 matches 7dda0c2; I inferred it because the diff touches no src or packages files. I counted only the file-level rows of the deletion table with wc, so the R2, R5 and R6 function-range counts are unchecked. Checks are green (19 reported, none failing), and the diff touches no daemon, backend or surface route, only docs/adr and a script outside every gate. There are no conflicts. The only open item is the question about the harness below.

Not blocking: https://github.com/callstack/agent-device/blob/3de1186/scripts/layering/boundary-engine-spike.ts#L1 is an 859-line harness that no gate covers. It is excluded from typecheck and from fallow, and it imports ten internal symbols from check.ts, model.ts, package-boundaries.ts, ratchet-reference.ts, apple-runner-host-port-policy.ts and tracked-sources.ts. A rename in any of those would break it silently, and the ADR's re-run plan would then find a script that does not compile. You can take or leave this. Either delete the file and cite it as git show d9959f510:scripts/layering/boundary-engine-spike.ts in the Measured and Deletion sections, or add it to a typechecked include.

Could you merge the ADR alone, about 140 lines, and pin the measurement to d9959f5, which the Status line already names? The revisit procedure would then retrieve the harness from that commit. That drops 859 unchecked lines from the production tree and keeps every number reproducible. Nothing has to change first except the Deletion section saying "retrieve from d9959f5" instead of "re-run the in-tree file".

The ten earlier inline threads from cubic-dev-ai are all fixed at this head, so you can resolve them: RUNS = 5, tracked-scope check, test paths in R4, parseOutput failure detail, README row wording, ADR 0010 citation, plant manifest restore, deletion table wording, restorePlants on edited files and Status line production tree.

[thymikee]

Done in 478fbd8. The PR now merges the ADR alone, as you asked: 2 files, 148 lines (ADR 0032 and its index row).

Harness pinned, not in the tree. scripts/layering/boundary-engine-spike.ts is deleted at the head. History was not rewritten, so d9959f510 stays an ancestor of the PR head. This repo only squash-merges, so after merge the commit stays reachable through refs/pull/3286/head. I checked that from an empty repository:

git fetch --filter=blob:none https://github.com/callstack/agent-device pull/3286/head
git show d9959f510:scripts/layering/boundary-engine-spike.ts | wc -l   # 859
git show FETCH_HEAD:scripts/layering/boundary-engine-spike.ts          # fatal: does not exist

ADR wording.

• Status pins every measurement to d9959f510, whose production tree is 7dda0c2bf's.
• Measured retrieves the harness with that git show, after git fetch origin pull/3286/head if the commit is not local.
• Deletion now gives your rationale. The harness imports internal symbols from check.ts, model.ts, package-boundaries.ts, ratchet-reference.ts, apple-runner-host-port-policy.ts and tracked-sources.ts, while tsconfig.json does not include scripts/layering/ and fallow ignores it. So the change that evaluates a revisit trigger retrieves the harness, ports it to that tree's internals, re-runs it and updates the tables.

Only numbers produced at d9959f510. I checked every number against the harness report from the run at d9959f510 (its tree field is d9959f510747f7ea4a1f97782bae15bb9bc229d1).

• Removed, because that run did not emit them:
  • the 280/234 dynamic-pair counts and fallow's 6,136 runtime pairs (from earlier ad-hoc scripts);
  • "with tsc or swc" on the 46 mislabels (that run only timed swc);
  • the "±0.3 s between runs" noise figure, which leaned on an earlier run.
• Relabelled as counts of files at the same commit: the deletion table and .fallowrc.json's 73 comment fields. The file-level deletion rows and 73 recount identically from git show d9959f510:…, and the generator is 122 lines there.
• The 88-edge figure is now attributed to the umbrella's 3.1.1 cross-check.

To answer two points you couldn't check: git diff --stat 7dda0c2bf d9959f510 -- src packages is empty, so the production tree is identical. The R2, R5 and R6 function-range counts were taken by line range at 7dda0c2bf, and those modules are unchanged at d9959f510.

Threads. All ten cubic threads you listed were already resolved, each with a reply naming its fix commit: 7 from the first round, 3 from the second. 10/10 resolved, 0 open.

Validation. pnpm check:affected --run on 478fbd83d passed; the diff is docs-only, so no local checks were selected. pnpm format:check passed too.

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[thymikee]

Thanks for the update. The code review of 0993bb0 is clean: the earlier thread is fixed, because docs/adr/0032-layering-graph-engine.md line 24 now names RANKED_ZONES and zoneRank, which matches the pinned harness. All 8 checks pass, and since the diff is docs only, no CI route overlaps with it. There are no conflicts, and nothing else needs to change before merge. I did not re-run the pinned harness to check the ADR's measurement numbers (the edge counts and the 122-line figure), and I did not confirm that the pinned commit stays reachable after the squash merge. The earlier inline thread (#3286 (comment)) no longer applies at this commit, so you can resolve it.

[thymikee]

Closing the open point on reachability with a measurement. No change was needed, so I pushed nothing and the head stays 0993bb07c.

What survives a squash merge. Three PRs squash-merged earlier today (#3262, #3253, #3217) all have their branches deleted on the remote, yet refs/pull/<n>/head still serves each head commit. In a fresh main clone with lazy fetching disabled (GIT_NO_LAZY_FETCH=1):

• fix(recording): preserve explicit macOS session surfaces #3217's head c98897a99 is absent;
• after git fetch origin pull/3217/head it is present;
• it is not an ancestor of main.

So after a squash merge and branch deletion, the PR ref is the only thing keeping a PR's commits.

The pin today. In the same clone, d9959f510 is absent from main. After git fetch origin pull/3286/head, it is present and git show d9959f510:scripts/layering/boundary-engine-spike.ts returns the 859-line harness.

ADR wording. It already names exactly this path: "Retrieve it with git show d9959f510:…, after git fetch origin pull/3286/head if the commit is not local." So the merge changes nothing for retrieval.

Remaining limit. The PR ref is a GitHub-hosted guarantee. A mirror or migration that drops refs/pull/* would lose the commit, and a squash merge cannot put it on main. Any new commit carrying the file would bring back the unchecked in-tree script we just removed, so I left this as is.

[thymikee]

Design follow-up on 0993bb07c, in the context of #3276: I support keeping the current engine after this spike. Before merging, please narrow what the ADR commits us to:

1. Keep one import model without freezing its implementation. Lines 13–14 and the final revisit trigger conflate retaining custom rules with retaining the custom resolver. LAYERING_RULES includes source-only AST checks and path checks; not all remaining rules consume resolved edges. Others require richer import facts, including symbols. A future engine supplying those facts to the existing consumers is an alternative worth evaluating; the AST rules do not have to disappear first. Replace “about 20 … keep it alive” with the actual remaining consumers/requirements, and allow revisiting when an engine can supply the shared model with less total machinery. This is a wording correction, not a request for another spike in this PR.
2. Separate semantic blockers from migration costs. Missed dynamic imports and orphan violations are correctness gaps. Diagnostic formatting, removing config comments, or keeping a small non-TS retired-path check are migration costs. R14/R71 are filesystem-placement policies, so needing a path check is not itself a failure of an import engine. The measured parity gaps plus limited net deletion already justify the decision.
3. Give the nine missing type-import pairs an actionable follow-up in Architecture quality: replace custom graph/layering code with maintained tools, close guardrail gaps, fix collocation (umbrella) #3276/Architecture WS2: boundary-tool spike — fallow boundaries vs dependency-cruiser (ADR) #3278. parseImports already parses an OXC AST for dynamic imports but scans static imports with regexes. Investigate using that existing AST consistently, including TSImportType, while preserving symbol and edge-kind semantics. Validate with the missed case and mixed type/dynamic imports of the same target; don't add another extractor alongside it.

Suggested decision wording:

Retain the current engine because the evaluated replacements do not provide sufficient semantic parity and net simplification. Graph-based checks share one normalized import model; its implementation remains replaceable. Continue replacing generic algorithms and compiler-enforceable rules with maintained tools. Fix the demonstrated parser gap.

The four evidence tables and the out-of-tree harness are useful as they stand. I checked that the pinned production trees match, but did not rerun the benchmark/parity harness; its measurements remain author-reported.

[thymikee]

Done in 081d869: ADR wording and the index row only. The evidence tables and the pinned harness are unchanged. The title is now "Retain the Current Engine".

1. One import model, implementation not frozen. I classified the 29 LAYERING_RULES entries by what they read. The eight spike rules use 9 entries. Of the other 20:

• Resolved edges, 6 entries: R9/R10 daemon modularity, R19, R73, R74, R75 (through its overlay measurement) and R76. R76 also reads named symbols and bindingResidue from dynamic imports. scripts/depgraph/ also reads resolved edges.
• Raw specifier sites via parseImports, 2 entries: R11 and R13.
• Source ASTs or text only, 11 entries: R7, R16, R18, R65, R66 and the other ownership checks.
• Paths only, 1 entry: the retired-platforms zone.

"About 20 … keep it alive" is replaced by that list of consumers and the facts they need. Rules at a glance now says graph-based checks share one normalized import model, "today built by parseImports and resolveImportEdges", and that its implementation remains replaceable. The last revisit trigger now reads: "An engine can supply the shared import model above to the existing consumers with less total machinery than parseImports and resolveImportEdges. The source-only AST checks do not have to change first."

2. Blockers vs migration costs. The Decision section opens with your statement verbatim, then splits the reasons.

• Semantic blockers:
  • dependency-cruiser's dedupe mislabels 46 pairs and causes the R77 lazy-import() miss;
  • fallow, with no dynamic kind, flags R5's lazy seams and misses R77 import();
  • fallow's rule packs skip orphan files.
• Limited simplification: about 430 lines deletable where dependency-cruiser matches, with the shared model still needed.
• Migration costs, which would not decide it alone:
  • TypeScript <7/swc and a baselineStale wrapper;
  • diagnostics without lines or rule ids;
  • the 73 config comments;
  • R14/R71 as filesystem-placement policies, where a non-TS path needs a path check with either engine.

The engine triggers now name only blockers: a dedupe key that includes the dependency type, and fallow gaining a dynamic kind plus rule-pack coverage of unreachable files.

3. Parser gap. Rules at a glance links #3293 and gives its direction. parseImports already parses an OXC AST for dynamic imports but scans static imports with regexes. The fix reads both from that AST, TSImportType included, keeping symbol and edge-kind semantics, rather than adding a second extractor.

pnpm check:affected --run on 081d869 passed: the diff is docs-only, so no local checks were selected. pnpm format:check passed. d9959f510 is still an ancestor of the head, so the pinned harness stays retrievable.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-07 18:23 UTC