Commit Graph

15 Commits

Author SHA1 Message Date
Michał Pierzchała 99754dc69c fix(layering): resolve relative imports inside workspace packages referencing #1781 (#1872)
* fix(layering): resolve relative imports inside workspace packages

resolveTargetFile() dropped any relative import whose resolved path
didn't start with src/. Since #1490 W0 added packages/*/src/** to the
source set, every intra-package relative import (e.g. a facade
re-exporting a sibling file) was silently invisible to the layering
graph — R4 value-cycle rejection and depgraph reverse reachability
both stopped at the package facade.

Resolve relative specifiers that land under packages/<name>/src/ too,
while still refusing anything outside src/ and packages/*/src/.

Verified against the real tree: 448 previously-invisible value edges
and 436 type-only edges now resolve, but none of them close a new R4
value cycle or grow the R9 type-cycle SCC, so the R6/R9 baselines are
unchanged.

Refs #1781

* style: apply oxfmt to regression test
2026-08-19 13:54:34 +02:00
Michał Pierzchała ef6ec2995b chore(layering): document R12/R18/R19, retire R8, make R9 shrink mandatory (#1781 A6) (#1825)
* chore(layering): document R12/R18/R19, retire R8, make R9 shrink mandatory (#1781 A6)

The A6 review kept `check:layering` in full (15/15 planted violations fired,
no other enforcer exists) and left four follow-throughs.

R12 bin-alias-fast-path, R18 contracts-implementation-authority and R19
selector-pipeline-ownership were live rules with no ADR or CONTEXT anchor —
they now carry one each, in the same list as R7/R9/R10/R13.

R8 zero-dep-job-closure is retired: no CI job sets `install-deps: false` and
ci.yml records why each keeps it enabled, so the invariant has no subjects.
R11's relative-into-packages exception existed only because a zero-dep closure
cannot coexist with specifier loads, so it retires with R8; the route is now
closed to every caller. R1 was retired the same way at #1490.

R9 was growth-only and merely suggested lowering the ceiling, which is
headroom the next change spends without a number moving. It is now an equality
pin like R6 and the R10 R7 counts, and the committed baseline drops 47 -> 46
(daemon-server ceiling 17 -> 16) to match the measurement.

ADR 0019 §6 now says each runtime-command-cutover row is deleted when that
command's migration is declared closed.

* chore(layering): rename R9 to type-cycle-size now that it fails both ways (#1781 A6)
2026-08-18 15:35:46 +02:00
Michał Pierzchała c7565cb1f8 refactor(snapshot): clean snapshot ownership (#1754)
* refactor(snapshot): clean snapshot ownership

* fix(snapshot): address ownership review feedback
2026-08-12 13:44:21 +02:00
Michał Pierzchała 4f8dc3f31e refactor: move selector engine into workspace package (#1589)
* refactor: move selector engine into workspace package

* refactor(selectors): trim the package façade to its real consumers

Follow-up to the selector-package cutover, from a structural review of it.

- Drop 15 façade symbols with no consumer anywhere in the repo:
  selectorUsesKey (added by the cutover, never called), isNodeVisible /
  isNodeEditable (the real helpers are contracts/snapshot's), normalizeText,
  splitIsSelectorArgs, IS_PREDICATE_REQUIRED_MESSAGE, four nested Replay
  types, SelectorDisambiguationDisclosure, and the four kernel type
  re-exports every consumer already imports from kernel directly.
- Delete SelectorCapturePolicyInput.selectorExpression, which
  deriveSelectorCapturePolicy never read; the policy varies only by
  predicate, so it takes one now. Two of the four tests asserted that the
  unread parameter had no effect and could not fail; they go with it.
- Return the Maestro export vocabulary to the maestro package. The cutover
  inlined MAESTRO_TEXT/STATE_SELECTOR_KEYS' values into the CLI call site,
  leaving both constants dead in the package that owns the concept and no
  gate over the two copies. MAESTRO_SELECTOR_PROJECTION is now the one
  statement of it.
- Dedupe SelectorDiagnostics and SelectorDisambiguationDisclosure, declared
  character-for-character twice across the AST/string seam, and name the two
  shared option shapes once instead of five inline copies. The parser-side
  resolution types take an Ast prefix so the twins read as twins.
- Delete three identity wrappers: parsePrivateSelector,
  selectorExpressionToMaestro, and the formatSelectorFailure forwarder —
  nothing passes it a chain any more, so the SelectorChain | string union
  and its branch go too.
- Delete internal/index.ts, an AST barrel whose only consumer was one test
  in the same directory (renamed to engine.test.ts), and the match.ts
  pass-through that existed to feed it.
- ReplaySelectorGrammar had three variants for two behaviors; 'wait' and
  'ordinary' were the same path. It is 'is' | 'positional' now.
- Drop the deleted src/sdk/selectors.ts from .fallowrc.json's entry list.

Behavior unchanged. pnpm check green: 598 unit files / 5278 tests, smoke
35 passed / 3 live skipped, layering 71/71, depgraph 22/22, mutation config
45/45, fallow clean, package smoke sound. Counterfactual: pointing
MAESTRO_SELECTOR_PROJECTION.textKeys at the state keys turns three
replay-maestro-export cells red; restored before commit.

* test(selectors): split the engine aggregation test by source concept

`internal/index.test.ts` (renamed `engine.test.ts` when its barrel went away)
was a 708-line aggregation over the whole engine — past the 500-line tripwire
and mirroring no source module, so it also ran as one serial unit.

It becomes five files that each mirror what they test, plus the parser cells
folded into the existing parse test:

  resolve.test.ts                 alternative fallback, strict uniqueness,
                                  first-match existence
  resolve-disambiguation.test.ts  ADR 0012 ranking: deepest, smallest-area,
                                  winner-vs-challenger disclosure, tie fallback
  resolve-viewport.test.ts        the visibility half: on-screen beats
                                  off-screen, including inside an off-screen
                                  scroll container
  match.test.ts                   per-key matching semantics (text, role,
                                  focused, appname/windowtitle, decoded
                                  newline labels)
  arguments.test.ts               where the selector ends and the command's
                                  positionals begin, both grammars
  parse.test.ts                   +6 grammar/escape cells beside the existing
                                  property tests

The login-form tree shared by resolve.test.ts and match.test.ts moves to
`__tests__/login-form-nodes.ts` rather than being copied into both.

All 27 cells are carried over unchanged and still pass; no file now exceeds
224 lines. pnpm check green: 602 unit files / 5278 tests, layering 71/71,
depgraph 22/22, mutation config 45/45, fallow clean over 127 changed files.

* revert(selectors): keep agent-device/selectors public, behind one AST subpath

The cutover removed the `agent-device/selectors` public subpath as part of
tightening the API. It is in use, so the removal is reverted: the subpath ships
the same ten symbols v0.20.5 shipped, with the same signatures.

That has to coexist with the reason the package façade is string-only, so the
AST leaves through one named door instead of the main one:

  @agent-device/selectors        string-in/string-out; every in-repo consumer
  @agent-device/selectors/ast    the published parser surface; one consumer,
                                 src/sdk/selectors.ts

`packages/selectors/src/ast.ts` re-exports parseSelectorChain,
tryParseSelectorChain, isSelectorToken, the AST-taking findSelectorChainMatch
and resolveSelectorChain, isNodeVisible, isNodeEditable, and types
SelectorChain / SelectorDiagnostics. `formatSelectorFailure` keeps its
published `SelectorChain | string` first parameter as a shim here rather than
widening internal/resolve.ts back to a union — the compatibility obligation
sits at the boundary that owes it.

This is strictly narrower than main, where the AST was reachable from anywhere
in src/ via src/selectors/*. Two gates hold it there: facade-symbols.ts pins
./ast to exactly the v0.20.5 list, and package-boundaries.test.ts asserts
src/sdk/selectors.ts is the only file outside the package that imports it.

Restored alongside: the ./selectors export and tsdown entry/chunk group, the
.fallowrc.json entry, the package-exports supported-subpath list, and both
client-api.md sections. No CHANGELOG entry — nothing is removed any more.

pnpm check green: 602 unit files / 5278 tests, smoke 35 passed / 3 live
skipped, layering 71/71 (10 packages, 32 subpaths), depgraph 22/22, mutation
config 45/45, fallow clean over 129 changed files, package smoke imported all
12 published entry points with publint and attw passing. Verified functionally
against the built dist: the doc's parse -> findSelectorChainMatch example
returns the same shapes as before, resolveSelectorChain still returns an AST
`selector`, and formatSelectorFailure still accepts a chain.

* fix(selectors): correct the two expectations that still assume the removal

Review P1s on a792415a: restoring the public subpath left two gates asserting
it was gone.

- installed-package-metro.test.ts moved `agent-device/selectors` into the
  blocked-specifier list. It goes back to the subpath smoke set, running the
  same `isSelectorToken('||')` + `parseSelectorChain` check it ran before the
  removal, so the file's only remaining delta from main is a formatter reflow.
- owner-files-no-leak.test.ts asserted `dist/src/sdk-selectors.js` was absent.
  It requires the stable named chunk again, and still rejects an auto-numbered
  `selectors2.js` fallback — the pair is what proves the restored tsdown chunk
  group is doing its job, verified against a clean build.

PR body corrected: the removal is no longer described as intentional API
tightening.

* refactor(selectors): satisfy the widened fallow scope after rebase

main's #1591 (the follow-up filed from this review) removed `packages/**` from
.fallowrc.json's ignorePatterns, so the new package is audited for the first
time. Everything below is a finding fallow could not previously see.

Dead surface, all confirmed consumer-free:

- 12 type re-exports from the `.` façade whose shapes consumers only ever
  reach structurally.
- MAESTRO_TEXT_SELECTOR_KEYS / MAESTRO_STATE_SELECTOR_KEYS, orphaned by this
  branch's own MAESTRO_SELECTOR_PROJECTION change, and the test-util
  SELECTOR_VALUE_HAZARDS. All three are module-local now.
- IS_PREDICATE_USAGE_HINT fails --production because its only consumer is the
  is-argument-surface parity test. It gets a commented `ignoreExports` entry
  rather than deletion: the constant is what makes the daemon and CLI raise
  ONE hint instead of two copied strings (ADR 0010), so the test asserting
  that is the point, not an accident.

`fast-check` is now declared by the package that imports it.

Duplication, split by what could be proven:

- `isUsefulVisibilityAnchor` existed character-for-character in both
  packages/selectors and packages/maestro. Moved to
  @agent-device/contracts/snapshot, which both already depend on and which
  already owns this vocabulary. Safe because the `normalizeType` each copy
  called is itself character-identical to the contracts one — checked before
  moving, since a different normalizer would have silently changed which
  nodes anchor.
- maestro additionally reimplemented `normalizeType`, `buildSnapshotNodeMap`
  (as `buildSnapshotNodeByIndex`) and `findSnapshotAncestor`, all
  character-identical to contracts'. Deleted in favour of the shared ones.
- The three scroll-ancestor walks are NOT deduped. They are structurally the
  same walk but each uses a different scrollable predicate, and I have no
  evidence the three agree; collapsing them would be a Maestro-conformance
  change, not a cleanup. Both maestro sites now say so, and the work is filed
  separately.

`projectSelectorExpression` (15 cyclomatic / 22 cognitive, written by the
cutover) splits into a dispatcher plus `readAgreedTextValue` and
`projectSelectorTerms`; all three are under threshold.

Rebase note: the one conflict, in package-boundaries.test.ts, resolved to
NEITHER side — #1591 had already deleted `AdReplayVerifiedTargetGuard` as an
unused export, and this branch deletes the seven ReplaySelectorPort names, so
the conflicting block is empty.

* build: record fast-check for packages/selectors in the lockfile

Declaring the dependency in packages/selectors/package.json without
regenerating pnpm-lock.yaml made every CI job fail in its install step with
ERR_PNPM_OUTDATED_LOCKFILE. My local `pnpm install --frozen-lockfile` printed
"+ 1 dependencies were added: fast-check@^4.9.0" and exited 0, which read as
success but was the same mismatch CI refuses.

Regenerated with the pinned pnpm 11.17.0, not the 11.5.3 on this machine:
11.5.3 rewrites peer-dependency resolution keys repo-wide (dropping
`(supports-color@7.2.0)` suffixes) and produced a 222-line diff. With the
pinned version the diff is the 4 lines this change actually needs, plus
pnpm's alphabetical re-sort of the root selectors entry.
2026-08-04 19:06:39 +02:00
Michał Pierzchała 0ee2a86129 refactor: extract contracts workspace package (#1499)
* refactor: extract contracts workspace package

* fix: preserve screenshot diff result contract

* test: stabilize Android keyboard smoke
2026-07-30 17:07:46 +02:00
Michał Pierzchała 76453add71 refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) (#1494)
* refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0)

Extend the workspace with packages/* and move the kernel behind an
enforced public API: packages/kernel with nine consumer-earned subpath
exports (errors, device, snapshot, contracts, collections, rect,
redaction, daemon-error, bounds — the last absorbed from utils as Rect
vocabulary). Every kernel import repo-wide becomes the
@agent-device/kernel/<sub> specifier; kernel tests move to
src/__tests__/kernel/ and exercise the package surface. The root
declares the package in devDependencies (workspace:*), tsdown bundles
it (noExternal) so the published artifact and its runtime dependency
manifest are unchanged.

Gate rewiring in the same change, per the W0 brief:
- R1 kernel-sink retires (physically subsumed); new R11
  package-boundaries guards no-root-back-imports, relative tunnelling
  past exports maps, undeclared workspace deps, and non-exported
  subpaths, with runtime resolution pins via import.meta.resolve.
- resolveImportEdges and mutation ownership follow workspace
  specifiers through exports maps, keeping R4 cycle checks, depgraph,
  and derived test ownership connected across the seam (kernel-errors
  still owns 495 tests). listSourceFiles includes packages/*/src.
- kernel becomes an unranked zone; mutation registry, stryker mutate
  globs, and the mutation-affected workflow path filter move to
  packages/kernel/src/errors.ts.
- check:affected gains packages/ ownership (manifests fail open);
  vitest and coverage include packages/*/src; fallow ignores
  packages/** (its resolver cannot follow workspace specifiers).
- The affected-selector CI job installs dependencies: its closure now
  crosses workspace specifiers, and the R8 relative exception is
  unsafe for production src files (Node ESM does not realpath, so dual
  specifier/relative loads would instantiate modules twice). The R8
  zero-dep set is pinned empty with that rationale.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep

* fix: address W0 review — mutation sandbox, exports-map resolution, tsc -b

Review findings on #1494, all five:

1. contracts-schema-public.test.ts reads the kernel source at its
   packages/ path (fs access invisible to the codemod and typecheck).
2. Mutation lane: Stryker sandboxes the tree but pnpm's node_modules
   symlink resolves @agent-device/* back to the real repo, so mutants
   in the sandbox never load and vitest.related finds no tests.
   vitest.mutation.config.ts now aliases each EXPORTED specifier to
   its source (derived from exports maps, never a wildcard), keeping
   resolution inside the mutated tree. Validated: kernel-errors module
   runs end to end (dry run 3,984 tests, mutants killed, exit 0).
3. Layering/depgraph resolve workspace specifiers through the
   exports-derived map (workspaceSpecifierTargets) instead of
   reconstructing paths, so '.'-facade packages resolve; the
   positional fallback remains only for map-less fixtures (P0 pin).
4. Per-package project references implemented: packages/kernel is
   composite (emitDeclarationOnly -> dist-types, gitignored), the root
   references it, and typecheck becomes tsc -b — probed to catch type
   errors on both sides under TypeScript 7 native.
5. R11's relative-route exception now requires membership in an actual
   R8 zero-dep job closure (zeroDepClosureFiles walks entries), not
   mere scripts/ placement — closing the dual-instantiation bypass.

Also from review discussion: daemon-error moves out of the kernel
package to src/client/ — its consumers (cli, client facade) rehydrate
wire DaemonErrors client-side; the daemon only produces them. Kernel
drops to 8 exported subpaths before any of them ship.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep

* refactor: one exports-map reader for mutation alias and ownership

Fallow flagged workspaceExportAliases (cognitive 15, CRAP 90). The
manifest-reading logic already exists as workspaceSpecifierTargets in
scripts/layering/package-boundaries.ts, so both the Stryker sandbox
alias table and the mutation ownership walker now consume it instead
of carrying near-clones. Behavior unchanged; mutation suite 45/45 and
changed-code fallow green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep

* fix: composite kernel without a root references edge

FreeRange runs plain `tsc -p tsconfig.json`, and a root `references`
entry makes non-build-mode TypeScript demand the referenced project's
built declarations (TS6305) — a standing "build first" tax on every
plain -p consumer (fr, editors). Keep the per-package composite
project and build it in typecheck (`tsc -b packages/kernel` before the
root and examples/sdk passes), but drop the root references edge: root
consumption resolves through exports to source, identical to runtime
and to the bundler. Probed: plain -p green with no prebuilt output;
kernel-side type errors still caught by its own build.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep

* fix: R11 uses the layering parser; mutation config is a fallow entry

Review blockers on #1494:

- R11's private single-quote regex could miss a double-quoted or
  re-export route into packages/*/src. specifierSites now delegates to
  the layering model's parseImports (both quote styles, side-effect
  imports, re-exports, dynamic imports), with direct regressions for
  each formerly-invisible form.
- vitest.mutation.config.ts becomes a declared fallow entry instead of
  a tolerated unused-file finding: the full-repo audit now reports it
  reachable (unused files 2 -> 1; the remainder predates this PR).

FreeRange clean-checkout evidence: with packages/kernel/dist-types and
every *.tsbuildinfo deleted, `pnpm check:freerange` reports 0 findings
on this head — the TS6305 topology died with the root references edge
in the previous commit; check:freerange has no build precondition.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-30 12:12:46 +02:00
Michał Pierzchała 2316fd32c5 test: pin daemon modularity migration contracts (#1487)
* test: pin daemon modularity migration contracts

* refactor: tighten daemon modularity ratchets

* test: pin reporter live hook isolation
2026-07-29 18:05:00 +02:00
Michał Pierzchała 255deb6c28 ci: fold single-grep jobs into steps, call named pnpm scripts (#1465)
* ci: fold single-grep jobs into steps, call named pnpm scripts

- Merge ios-runner-swift-compat and no-test-di-seams (each just
  checkout + one rg assertion) into steps of a new static-checks job,
  keeping each step's own failure message. Removes two job-scheduling/
  checkout overheads and two PR status-check lines.
- Replace the layering-guard job's inlined copies of check:layering and
  depgraph:test with the named pnpm scripts, removing the silent-drift
  risk between the workflow and package.json.
- Fix the same drift in conformance-regenerate.yml, which inlined
  maestro:conformance:regenerate byte-for-byte.
- Leave affected-selector's inline node invocation as-is: R8's zero-dep
  closure check (scripts/layering/zero-dep-jobs.ts) finds a job's entry
  scripts by matching literal paths in the run: block, so switching to
  `pnpm check:affected:test` would zero out its entries and make R8
  fail closed. Documented inline why this one stays inlined.
- Leave publish-mcp-registry.yml's sync-mcp-metadata --check alone: that
  job never runs the setup-node-pnpm action, so pnpm isn't provisioned
  there at all.

Refs #1462

* ci: teach R8 to resolve pnpm script names, drop affected-selector's inline copy

R8's zero-dep-job entry scan matched literal script paths in a run: block,
so a bare `pnpm <script>` invocation found zero entries and R8 failed
closed — the reason affected-selector kept an inline node command instead
of calling pnpm check:affected:test (#1462). zeroDepJobs now also resolves
a pnpm script name against package.json and scans the resolved command for
entry paths, so affected-selector can call the named script like every
other job.

Also replaced the other workflows' inlined copies of named package.json
scripts (test:replay:*, perf, perf:android, maestro:conformance:differential,
check:mcp-metadata, size) with their pnpm names, keeping each job's
CI-specific trailing flags — found via a repo-wide sweep for any run: block
whose text duplicates a scripts entry.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L67kDSTAJwaoLCwANEJFRM

* fix: revert publish-mcp-registry pnpm regression, recurse R8 alias resolution

publish-mcp-registry.yml's job only provisions Node via actions/setup-node,
never the repo's setup-node-pnpm action, so pnpm is never installed there —
the earlier sweep's `pnpm check:mcp-metadata` would have broken the release
path. Reverted to the direct node invocation with a comment explaining why,
matching the PR's own stated rationale for leaving it alone.

zeroDepJobs' pnpm-alias resolution only expanded one level: a resolved
script that itself invoked another named pnpm script had its entries
silently dropped from R8's closure. resolveRunEntries now recurses through
chained aliases with a per-chain visited set, so a nested alias's entries
are found and a cycle stops re-expanding a repeated name instead of
recursing forever. Added coverage for both the chained and cyclic cases.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L67kDSTAJwaoLCwANEJFRM

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-28 20:56:19 +02:00
devin-ai-integration[bot] edca35d122 chore(deps): Renovate config, packageManager-derived pnpm in CI, repo-wide format (#1444)
* chore(deps): add Renovate config and enforce packageManager pnpm version in CI

Refs #1422

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* chore: bump pnpm to 11.17.0 and format the whole repo with oxfmt

format/format:check drop their hand-maintained path list: oxfmt already skips
node_modules and honors .gitignore, so the only exclusion list is
.oxfmtrc.json ignorePatterns.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* test(mutation): accept either quote style in the affected-lane path filter

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* chore(deps): keep fixture-app runtime deps as individual Renovate PRs

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-07-28 13:50:39 +02:00
Michał Pierzchała fcaa6c995c refactor(contracts): declare shared vocabulary below its consumers, ratchet what remains (#1435)
* refactor(contracts): declare the public API vocabulary below its consumers

The layering gate's largest remaining cluster was 28 type-only inversions from a
single edge: `commands/` declaring itself in terms of `client/client-types.ts`.
R2 forbids the reverse import, so a shape both surfaces need has to sit below
both. The command/device vocabulary — connection config, the device and session
views, and every per-command Options/Result — now lives in
`contracts/client-api.ts`; `client/client-types.ts` keeps the `AgentDeviceClient`
facade and re-exports the rest through one wildcard.

R6 total: 42 -> 18. No new inversion in any pair.

The published surface is unchanged, and that is verified rather than asserted:
the built `index.d.ts` exports the same 216 type names as main, byte-identical.

Eight shapes deliberately did NOT move, because each is stated in terms of a
HIGHER-ranked zone: `ScrollOptions` (ScrollInputDirection, commands/), the four
navigation Options plus `AgentDeviceCommandClient` (navigation-projection,
commands/), and the two Metro result aliases (metro/). Declaring those in
contracts/ would trade 28 commands->client edges for contracts->commands and
contracts->metro ones — the foundation depending on the layers above it, worse in
kind even though fewer in number. This is measured, not assumed: moving the whole
file to contracts/ first took the gate from 42 to 48, which is how the floor was
found.

Two keystone moves made the other 84 movable:

- `RemoteConnectionProfileFields` joined its sibling `CloudProviderProfileFields`
  in contracts/remote-config-fields.ts. It was the root of the base chain
  (AgentDeviceClientConfig -> AgentDeviceRequestOverrides ->
  DeviceCommandBaseOptions -> every per-command Options), so one rank-4
  declaration was pinning ~80 shapes up with it.
- `DaemonBatchStep` moved to contracts/batch-step.ts. Its `runtime` field was
  written `DaemonRequest['runtime']`, dragging the whole daemon request type in to
  say `SessionRuntimeHints` — the same type, three zones lower.

`CompanionTunnelScope`/`MetroBridgeScope` also moved to contracts/, since the
vocabulary needs the scope shape and it sat next to client-local env-var names.

Six pass-through re-exports in client-types.ts are suppressed per-name with the
reason inline: they exist only to publish contracts/kernel types through the
package entrypoint wildcard, every internal consumer imports them from the
declaring module, so "no consumer" is correct and not actionable — deleting them
would remove names from the public types.

`pnpm check` green, 4488 unit tests. Findings doc records the sequencing for the
last 5: the upstream declarations have to come down before the shapes that need
them can.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs: drop the graph viewer, keep the query that replaces it

The rendered dependency-graph viewer is not being merged (PR #1409 closed). It cost
~2200 lines plus a Fallow exemption for a 920-line canvas renderer, and nobody —
human or agent — reached a conclusion from the picture. Every finding in this
document came from short queries against the gate's own model.

This file pointed at the `claude/depgraph-viewer` branch for the tooling, which
would have dangled once that branch is deleted. Replaced with the thing that was
actually load-bearing: a throwaway probe script, inlined, that re-derives the
numbers from `scripts/layering/model.ts` and nothing else. Verified verbatim — it
reproduces TYPE_INVERSION_BASELINE exactly, which is also the check that tells you
whether either side has gone stale.

Two numbers in the summary table were stale, describing an intermediate state
rather than what shipped: R6 said "35 across 4" (actually 18 across 5 after the
vocabulary move) and ranked coverage said "729 of 894" (actually 888 of 901). Both
corrected, along with the file/edge counts in the header.

Also notes the deduplication detail that makes the query agree with the gate: each
file pair counts once, so a raw edge count reads higher.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* refactor(contracts): move the four keystones that pinned the rest of the inversions

R6 type-only spine inversions: 18 -> 7, and every one of the 7 that remains is a
deliberate architectural position rather than a misplaced declaration.

Four keystones moved to contracts/, each of which was pinning a much larger set:

- `CommandFlags` (was core/dispatch-context.ts). One rank-2 declaration holding the
  daemon's request type and every recorded action above it. Its last non-contracts
  dependency was `DaemonBatchStep`, already moved in 3fdbfe0.
- `SessionAction` (was daemon/types.ts). replay/ (6 modules) and compat/maestro/
  read and write session scripts; declaring the shape inside the daemon made both
  depend on the server to describe a file format neither asks it to produce. The
  daemon still owns the recording — only the shape moved.
- `TargetAnnotationV1` shape (was replay/target-identity.ts). ADR 0012 target
  evidence, written by 8 daemon modules and read by commands/; the parsing and
  classification logic stays in replay/.
- `ScrollInputDirection` and the Metro prepare/reload result payloads, which
  unblocked `ScrollOptions` and `MetroPrepareResult`/`MetroReloadResult`.

`DaemonRequest` also split into the three shapes it had been conflating: the
kernel WIRE shape (`flags?: Record<string, unknown>`, because a process boundary
cannot enforce a vocabulary), the new `contracts/command-request.ts`
`CommandRequest` (wire shape with flags typed — what a command surface needs), and
the daemon's own refinement (+ `internal?: DaemonRequestInternal`, carrying
SessionState callbacks and the admitted lease). core/command-descriptor/ had been
importing the third to read `command`, `positionals` and `flags`.

Two things deliberately NOT moved, because moving them would add coupling rather
than remove it, and the baseline now argues both:

- `DaemonCommandDescriptor`/`DaemonCommandRoute` — the route type is
  `keyof typeof DAEMON_ROUTE_HANDLERS`, derived from what the server implements.
  Moving it down means re-declaring route names in contracts plus a gate to prove
  the handler map still covers them. ADR 0003/0008 own that boundary.
- `AgentDeviceClient` — used as an opaque handle by 4 files. The facade is built
  from commands/'s own NAVIGATION_COMMAND_PROJECTIONS, so this is a genuine
  zone-level cycle; breaking it is a design call about where that registry belongs.
  R5 is zero here: nothing imports the client at runtime, only its type.

Also records the largest structural finding, which R6 does not measure: cycles by
edge kind are 1 (value only), 87 (value + type-only), 1 (value + dynamic), 213
(all). At runtime the graph is a clean DAG; the 87-file type-level cluster means
no one of those files' types can be read in isolation. Hubs are
runtime-contract.ts, commands/runtime-types.ts, backend.ts,
commands/runtime-common.ts. Not attempted here — it is a different and much larger
change.

`pnpm check` green, 4488 unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* feat(layering): ratchet type-cycle growth (R9), and rule out a narrower client port

R9: the largest strongly-connected component over value + type-only edges may not
grow. R4 keeps the VALUE graph acyclic, so every cycle counted here is created by
type-only imports - free at runtime, invisible to R5/R6, and the largest single
obstacle to reading a subsystem in isolation: inside a component of 102 files, no
file has a self-contained slice.

Baseline set to 102, which is what THIS branch achieves - main carries 107 and the
boundary moves here bring it to 102. An earlier revision baselined 87, measured
against an older main; after rebasing onto f19864e the real figure was 102 and the
new rule fired on its own stale baseline. Worth stating because the failure looked
like a regression and was not: attribution showed main at 107 and this branch
reducing it, which is the check working rather than complaining.

Growth-only, deliberately unlike R6. Reducing 102 is a real refactor rather than a
file move, so a hard equality would turn every unrelated improvement into a baseline
edit. A shrunk tree is reported in the success line instead of failing. Verified at
the new baseline by adding one type-only import that closes a loop and watching 102
become 108 and the gate reject it.

The refactor itself is still not attempted. Hubs by in-component dependents are
runtime-contract.ts, commands/runtime-types.ts, backend.ts,
commands/runtime-common.ts; a pass starts there.

Separately, investigated the narrower-port idea for the 4 remaining -> client
inversions and it does not work. Measured first:

  files NAMING AgentDeviceClient (the inversions)   4
  files CALLING client methods                     26
  distinct facade namespaces reached               13

The narrowness is an artifact of where the type is named, not of what is used.
Making those four generic over the client type pushes the concrete type into the 26
implementations, turning 4 inversions into up to 26. A port spanning 13 namespaces
is the whole facade, so it would either duplicate the public API shape - a second
source of truth for it - or derive from the facade and carry the same dependency.

So the four are the minimum number of naming sites rather than an accident: they are
the choke point. Recorded as a position with the numbers behind it. The remaining
option is the question underneath it - whether NAVIGATION_COMMAND_PROJECTIONS
belongs in commands/ - and that is a design decision about the command surface, not
a dependency cleanup.

pnpm check green, 4535 unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* fix(layering): test R9, specify its floor, and drop two duplications

Adversarial self-review of #1435 found four things worth fixing.

R9 shipped with no unit test. Every other rule in this gate has one (R5 back-edges,
R6 inversions, R7 session state, R8 zero-dep closures); R9's only verification was a
manual injection CI cannot repeat. Added tests for the three distinctions it depends
on: a type-only loop counts, a dynamic-only loop does not, a value loop still does.

Writing that test immediately found an undocumented edge case, which is the argument
for it. largestTypeCycleSize returns 1 for an acyclic graph that has non-dynamic
edges but 0 when every edge is dynamic, because only edge-participating files enter
the walk. Immaterial to a growth ratchet, but an inconsistent floor nobody had
written down. Now specified in the doc comment and pinned by the test, so 0 and 1
cannot later be read as a meaningful difference.

largestTypeCycleMembers was exported with no consumer - speculative API, and
scripts/layering is in Fallow's ignorePatterns so nothing would have flagged it.
Same pattern review caught on the previous head with MaestroRuntimeFlags and
TargetRect. Made module-private.

ResolvedMetroKind was declared twice after the Metro payload move: exported from
contracts/metro.ts and still private in metro/client-metro.ts. client-metro.ts now
imports it.

The gate computed the SCC twice per run, once in the rule and once for the success
line. Computed once and threaded, so the two can no longer disagree.

Also re-verified the claim this PR rests on, with a stronger check than the one in
the body: comparing DECLARATION names in index.d.ts counts inlined internals, and by
that measure this branch appears to lose five names (PrepareMetroRuntimeResult,
ReloadMetroResult, ResolvedMetroKind, SCROLL_INPUT_DIRECTIONS, ScrollInputDirection).
All five are declared-but-not-exported helpers. The real surface - exported names
across all eleven published entrypoints - is 69 on both sides, identical. Also proved
DaemonRequest structurally equal to its pre-split shape with a type-level assertion
rather than by reasoning, and confirmed SessionAction, CommandFlags and
TargetAnnotationV1 moved byte-identically.

pnpm check green, 4535 unit tests, 24 layering tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* refactor(contracts): one file per command family, one name per request

Addresses review on #1435.

contracts/client-api.ts was 1,064 LOC and grouped session, app, interaction,
replay, observability and recording contracts together, so it answered no one
question and crossed the >1,000-LOC architecture-debt tripwire in AGENTS.md:124.
Split it into 14 domain-family files by the command families that already exist
(client-connection, client-device-view, client-session, client-lease, client-app,
client-capture, client-target, client-gesture, client-selector-read,
client-replay, client-observability, client-settings, client-system,
client-request); the four Metro client shapes went into the existing
contracts/metro.ts so one file answers the Metro question. Largest resulting
file is 137 LOC. client/client-types.ts re-exports one wildcard per family, so
the published import path is unchanged.

Published surface verified unchanged against main two ways: the exported-name
set of all 11 published entrypoints is identical (70 names), and every
declaration in the built index.d.ts is byte-identical after normalization -- 0
names added, 0 shapes changed. index.d.ts got smaller (1,726 -> 1,682 lines):
10 declarations main duplicated into it now resolve through a shared chunk.

Also, from re-examining the two findings the review flagged as blind spots:

- CommandRequest was a third name for "a request" that no consumer needed.
  Every core/command-descriptor/ use read only command/positionals/flags, in two
  spellings (the full type and a Pick of it). Replaced by
  contracts/dispatched-command.ts DispatchedCommand -- those three fields and
  nothing else, with command/positionals Picked from the wire type so they
  cannot drift. daemon/types.ts DaemonRequest now extends the wire shape
  directly. Two request shapes again, at two ranks.
- The 7 remaining R6 inversions each get a mechanical reason rather than an
  appeal to an ADR: the 4 AgentDeviceClient edges are a real zone-level cycle
  (client-types.ts imports ProjectedNavigationCommandClient from commands/), and
  no narrower port exists (26 call sites across 13 namespaces); the 2
  DaemonCommandDescriptor edges are unavoidable because that shape is stated in
  terms of the server-private DaemonRequest; the 1 DaemonCommandRoute edge is
  unavoidable because the type is computed from the daemon's handler table.

Cleanups found on the way: three doc comments this branch had orphaned from
their declarations (SettleCommandOptions, RecordControlOptions,
ReloadMetroResult -- the last had drifted onto an unrelated type it
misdescribed) are reattached; intra-contracts imports normalized from
'../contracts/x.ts' to './x.ts', which is what the duplicate-import lint caught;
and stale references to the deleted file removed from the docs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-27 17:22:39 +02:00
Michał Pierzchała 56b72c5cf7 refactor(boundaries): put shared contracts below their consumers, gate the result (#1405)
* refactor(boundaries): move shared contracts below their consumers

Acts on the depgraph findings: type-only edges are invisible to R5, so
vocabulary that everything depends on had drifted above the zones that use it.

- contracts/: the four platform-plugin facet tags (LogBackend,
  RecordingBackendTag, PerfMetricsSamplerTag, PlatformGatedProviderResolverKey)
  now live beside the plugin contract itself, which also moves out of core/;
  NetworkEntry moves next to the command surface that renders it; and the
  click-button, recording-export-quality, interactor-types and
  runner-lease-context vocabularies move down out of core/.
- (root) drops from 29 files to 13: the internal *-contract/output/annotation
  modules move into contracts/, kernel/ (daemon-error, observability-redaction
  beside kernel/redaction), core/ (batch-policy, an ADR 0008 projection),
  commands/ (cli-command-aliases) and remote/ (upload-progress, upload-stream).
  What remains is entrypoints and the composition roots that R2 requires to
  sit outside the spine.
- utils/ joins the ranked spine at rank 1 after its only two upward files move
  to the zones they were reaching for (cli/resolve-cli-options,
  cli-schema/cli-config), putting ~336 value edges under the gate.
- Internal imports that routed types through the client-types re-export hub now
  name their real source.

Type-only spine inversions drop from 61 to 35; the remainder is two clusters
(client/client-types.ts and the ADR 0003 daemon facet). No behaviour change:
4470 unit tests and the layering gate pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* style: merge the duplicate contract imports the tag moves created

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* refactor(imports): name the declaring module, share find's argument rules

Two follow-ups from re-measuring the graph after the boundary moves.

1. 89 type imports across 79 files routed through a re-export hub in another
   zone: `CliFlags` reached through commands/cli-grammar/flag-types.ts (52) when
   it is declared in contracts/cli-flags.ts, the replay suite result types
   reached through daemon/types.ts when they are declared in contracts/replay.ts,
   the doctor types through a daemon handler module, and so on. Each hop invented
   a cross-zone edge the architecture never asked for — including every apparent
   replay -> daemon and utils -> commands dependency. They now name the module
   that declares them. Within-zone hops are left alone; those are a local style
   choice, not a boundary claim.

2. `find`'s three positional/flag checks existed in both daemon entry points with
   hand-repeated messages, and the copy in dispatchFindReadOnlyViaRuntime was
   unreachable — its only caller validates first. Both now call checkFindArgs in
   selectors/find.ts, beside parseFindArgs and isReadOnlyFindAction, for the
   reason that module's own comment already gives: so the two paths cannot
   disagree. The refusal is returned rather than thrown, because the two
   mechanisms are not observationally identical in the session event log.

Type-only spine inversions: 61 -> 35. 4470 unit tests and every gate pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* feat(layering): ratchet type-only spine inversions (R6)

R5 ignores type-only edges by design — they cost nothing at runtime and do not
affect cold start — so nothing was watching the direction they point. Ranking
them the same way found 61 inversions, including contracts/ and utils/ declared
in terms of rank-4 zones. 26 are fixed by the preceding commits; R6 pins the
rest per zone pair so they can only shrink, and a new pair fails outright rather
than being added to the baseline.

The two remaining clusters each need their own change, and the baseline says so:
the per-command Options/Result vocabulary declared inside the public Node-client
surface, and the ADR 0003 daemon facet shape that core's descriptor registry
composes.

Both ratchet directions are covered: growth fails, and shrinking without
lowering the number fails too, so the baseline cannot quietly stop describing
the tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs: record the import-graph findings behind this refactor

A dated snapshot, not a normative document: when it disagrees with
scripts/layering/, the gate wins. The graph tool that produced it lives on the
claude/depgraph-viewer branch, deliberately out of this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* refactor(selectors): state the shared selector argument rules once

R2 (commands-floor) forbids the daemon from importing commands/, and that is the
right call: commands/ is the client-side surface — its only consumers are cli/,
cli-schema/, mcp/, client/ and the composition roots — while the daemon is the
executor on the other side of the wire. ADR 0008 protects exactly that seam.
Relaxing R2 would let the executor depend on a client projection and pull CLI
grammar and output formatting into the daemon's bundle.

But the rule does force duplication: the daemon must validate independently
because it accepts requests from any client, so 10 refusal messages existed in
both zones. The only place a shared rule can live is below both, and selectors/
already held the parsers (splitIsSelectorArgs, splitSelectorFromArgs,
isSupportedPredicate) and even the `is` predicate message — just not the checks
that use them.

Three drifts had already appeared in the `is` predicate rule alone:

- commands/interaction/selectors.ts re-implemented the predicate list as an
  inlined seven-way `!==` chain while importing the message and hint from
  selectors/predicates.ts, so adding a predicate to the shared list would not
  have reached the CLI grammar.
- That inlined chain compared the raw token, so the CLI rejected `is TEXT ...`
  while the daemon it hands the command to accepts it. The CLI now matches the
  executor; this is an intentional alignment, not an accident.
- isCommand raised the same refusal without IS_PREDICATE_USAGE_HINT, so whether
  an agent got recovery guidance depended on which layer noticed first — the
  failure mode ADR 0010's audit calls out.

checkIsPredicate, checkIsArgs, checkGetFormat, checkElementTargetArgs and
checkWaitText now hold those rules, each beside the parser it wraps, and report
a refusal rather than choosing how to raise it: the daemon returns a response,
the command surface throws. Those mechanisms are not interchangeable — they
write different session events — so the shared check stays out of that decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* feat(daemon): give ADR 0014's ref frame one transition, pin SessionState owners

`SessionStore.get()` returns the live record out of a private Map and `set()`
re-puts the same reference, so every `session.<field> = …` in the daemon is a
durable write to store-owned state: 57 of them across 17 files, against 26
`set()` calls that are therefore ceremonial. Nothing at the store boundary can
check what those writes are supposed to keep true.

Measuring which module writes which field showed the problem is narrower than
the raw count suggests — 16 of 27 fields already have exactly one writer. The
sharp case is ADR 0014's ref frame: `refFrameState`, `refFrameScope`,
`refFrameTree` and `refFrameGeneration` must move together or the frame is
incoherent (an `active` state with a stale tree resolves refs against a
namespace nobody authorized), yet complete issuance wrote them in ref-frame.ts
and partial issuance wrote the same four in session-snapshot.ts. ref-frame.ts's
own header claims to be "the single owner of the frame's transitions", and
session-snapshot.ts documented itself as the exception. Both forms now go
through `activateRefFrame`; they differ only in scope.

`recordSession` deliberately moves alone in two paths (recording without arming
a publication), so the save-script cluster gets no invented abstraction — it
gets ownership instead. R7 records every field's owner and stops the set from
growing quietly: a new SessionState field must declare one, a foreign write
fails naming the owner to call, and an owner that stops writing must be removed
so the table cannot drift into fiction. Field names are read out of the
`SessionState` declaration, so a daemon module with an unrelated local named
`session` — a provider or runner session — cannot trip it.

4475 unit tests and every gate pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs: record the reference semantics and refresh the findings

SessionStore.get/set now document that the record is handed out live, since that
is the fact behind R7. The findings snapshot picks up the resolved R2 question,
the ref-frame consolidation and the two new gate scopes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* refactor(boundaries): rank every satellite zone, extract the provider port

Second-order effect of the earlier rounds. With `utils` on the spine and
`(root)` emptied of shared contracts, the eleven zones that were unranked
"because ranking them would invent an order the architecture had not committed
to" turned out to have a consistent rank already — the order was there,
unasserted. Solving the constraint system showed one blocker: `utils/remote-config.ts`
projected a remote-config profile into `CliFlags` while reaching up into
`remote/`, and its only three consumers were in `cli/`. It moves there as
`cli/remote-config-flags.ts`, and every satellite zone joins the spine.

Ranked coverage goes from 730/895 files to 882/895. Only `(root)` stays out, and
now for one stated reason: R2 forbids `daemon/` from importing `commands/`, so
the files that wire them compose the spine from above.

Ranking them exposed 22 type-only inversions R6 had never been able to see, and
they were concentrated rather than scattered:

- The device-provider port. `providers/` and `cloud-webdriver/` implement what
  the daemon calls, so both sides name `DeviceLease`, `LeaseLifecycleProvider`,
  `LeaseLifecycleContext` and `DeviceInventoryProvider` — now declared in
  contracts/device-provider.ts, below both. The adapters also imported the
  daemon's NARROWED `DaemonRequest` while only ever reading `req.flags`; they now
  name the public one from kernel/contracts.
- `MetroPrepareKind` and the remote-config profile field groups move to
  contracts/ for the same reason: the command surface validates them and
  contracts/cli-flags.ts is composed from them.

Two clusters remain, ratcheted with their reasons in TYPE_INVERSION_BASELINE:
the client-types vocabulary, and `SessionAction`, which needs `CommandFlags` and
`DaemonBatchStep` to move with it.

Also fixes two things CI caught: the eight type re-exports my earlier import
redirection orphaned (none published through any src/sdk/* entrypoint, so no
public surface changes) and `isSupportedPredicate`, now module-private since
`checkIsPredicate` is the admission API. `fallow-baselines/health.json` is keyed
by path, so the moved cli-config entry moves with the file rather than being
regenerated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* fix(selectors): use the admitted predicate, not the raw option

Review finding. `isCommand` called `checkIsPredicate` and then kept reading
`options.predicate` for the capture policy, the `exists` branch,
`evaluateIsPredicate`, the failure message and the returned result. Admission
normalizes case, so an upper-case predicate was let past the gate and then
evaluated against lower-case branches: `EXISTS` skipped its own branch and fell
through to the generic path, and the result echoed the raw token. I widened
admission at that surface without threading the normalized value through it —
the CLI-grammar surface in the same change does use the admitted value.

Every decision after admission now reads it.

Two tests, both verified to fail without the fix:

- a production-route regression driving `device.selectors.is` with
  `EXISTS`/`TEXT`, plus one pinning that an unknown predicate is still refused
  WITH the ADR 0010 usage hint;
- a surface parity gate (selectors/__tests__/is-argument-surface-parity.test.ts)
  in the repo's existing parity style, asserting the daemon and CLI-grammar
  surfaces reach the same verdict and hand the same normalized predicate
  downstream across an input table. A helper-only test cannot catch a surface
  that admits correctly and then discards the result, which is what happened
  here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs: name the pre-push gate, and the formatter's path allowlist

Both misses in this PR's review were process, not judgement, and the docs
pointed the wrong way for both.

AGENTS.md said "prefer the aggregate package.json scripts" without naming which
aggregate, and CONTRIBUTING listed `pnpm test` and the targeted checks but never
`pnpm check`. `check:tooling` looks like the gate and is a subset of it: it stops
before the Fallow audit, so the dead exports this PR introduced passed a clean
`check:tooling` and failed CI. Both files now name `pnpm check`, say what it
covers, and say what it cannot (the device matrix).

The same gap produced a second mistake twice: `oxfmt <path>` reformats whatever
you point it at, while the repo's `format` script is an allowlist that excludes
`scripts/` and every `.md`. One run reformatted 50 unrelated script files into a
commit; the next nearly did it to AGENTS.md. AGENTS.md now says to run
`pnpm format`, never `oxfmt <path>`.

It also records the rule that cost a CI cycle: Fallow's baselines are keyed by
path, so a renamed file needs its baseline entry moved, not the baselines
regenerated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* revert: undo stray formatter output across docs and scripts

Three separate `oxfmt <path>` runs in this branch reformatted files the repo's
`format` script deliberately excludes: 55 files under scripts/maestro-conformance
plus scripts/perf, sync-mcp-metadata and the slow-test reporter, and 12 markdown
files including six ADRs and docs/agents/. All of it was whitespace, quote style
and markdown table padding — no content — but it inflated the diff a reviewer has
to read and would have rewritten prose ownership across files this change has no
business touching.

All 70 are back to their origin/main content, so the diff outside src/ is now
exactly this change's scope: three docs, scripts/layering, the Fallow baseline,
and five provider integration tests.

The rule this violated is now in AGENTS.md: run `pnpm format`, never
`oxfmt <path>`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* style: reformat two provider tests with the repo's pinned oxfmt

`pnpm format:check` failed in CI on the two files whose imports I merged by hand.
The repo pins oxfmt 0.42.0 as a devDependency and both `format` scripts invoke
`./node_modules/oxfmt/bin/oxfmt`; I had reformatted with `npx oxfmt`, which
resolved 0.60.0, and the two versions disagree about wrapping a 100-column import.

This is the rule AGENTS.md already states — run `pnpm format`, never oxfmt
directly — so there is nothing to add to the docs, only to do.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* fix(ci): install deps for the layering guard, and gate the zero-dep contract

The Layering Guard job failed with ERR_MODULE_NOT_FOUND on `oxc-parser`. The job
ran with `install-deps: false` — no `pnpm install`, so no `node_modules` — and R7
had started parsing the daemon with oxc-parser instead of matching assignment
operators with a regex. `pnpm check:layering` passed on every local run, because
locally `node_modules` is always there.

The job now installs dependencies. The alternative was to put R7 back on a regex,
which cannot see `??=` or a computed `session[key] =` write, so it would trade a
correct rule for a fast job.

That leaves the interesting part: the zero-dep contract is real for the jobs that
keep it, and it is invisible to every local run, which is the worst combination a
constraint can have. R8 makes it checkable. It reads the zero-dep job list out of
`.github/workflows/` rather than restating it — declaring a job zero-dep is what
puts it under the rule — walks each job's entry scripts and their whole
relative-import closure, and requires every specifier to be a Node builtin or
another repo file. A zero-dep job whose entry scripts the scan cannot identify
fails too, so the rule cannot be escaped by changing how the job invokes them.

Specifiers come from oxc-parser's module record, not a line scan. The closures
include `--test` files, and a test about imports naturally embeds import syntax in
a fixture string; the line scanner reported two such phantom violations in
model.test.ts before the switch, which is how a gate stops being trusted.

Verified by re-running the real gate against three injected regressions: the
layering job back on `install-deps: false` (reproduces the exact CI failure,
pointing at session-state.ts:24), a package import added to the still-zero-dep
affected-selector closure, and a zero-dep job whose run step names no script.

Also corrects the CONTEXT.md spine paragraph, which still described the satellite
zones as deliberately unranked after they had all joined the ranked spine.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* fix(layering): make R7 exhaustive, and follow session records through aliases

Review finding: `SESSION_STATE_FIELD_OWNERS` covered 27 of `SessionState`'s 42
fields and nothing asserted parity, so a new field could be added and pass the
gate by being invisible to it. R7's advertised claim — "every SessionState write
is inside its declared owner" — was broader than what it checked.

Investigating that turned up a second, larger gap the finding did not name: the
scan only recognized a binding literally named `session`. The daemon names these
records by role, so `nextSession`, `provisionalSession`, `completedSession`,
`preRunSession` and `preEntrySession` were all invisible — and three of those
writes were genuine violations R7 existed to catch:

  src/daemon/snapshot-runtime.ts:256  nextSession.snapshotScopeSource
  src/daemon/snapshot-runtime.ts:265  nextSession.snapshotGeneration
  src/daemon/handlers/session-replay-runtime.ts:707
                                      preEntrySession.pendingRecordAndHeal

The first two are the #1076 versioned-ref invariant: the generation advances
exactly when the stored tree is replaced. That rule lived in `setSessionSnapshot`
and had acquired a second statement of itself in snapshot-runtime.ts, whose own
comment admitted the bypass. It now goes through `setSnapshotLineage` in the
owning module. The third clears a watermark stamped by session-replay-resume.ts;
`clearPendingRecordAndHealWatermark` puts the clear beside the stamp.

Gate changes:
- Binding detection accepts aliases, paired with the existing declared-field
  filter so an unrelated `…Session` local only registers if it also writes a
  field SessionState owns — where the remedy is the same anyway.
- `fieldClassificationDrift` asserts parity in all three directions:
  unclassified, in-both, and naming a field SessionState no longer declares.
- `STORE_OWNED_SESSION_STATE_FIELDS` classifies the 11 fields the store
  establishes at construction. It is a positive claim, so a direct write to one
  fails and names both remedies.
- Four fields the widened scan made visible (`lease`, `deviceClaim`, `appName`,
  `saveScriptComplete`) got real owners.

`nextSnapshotGeneration` is now module-private: replacing its only external call
site orphaned the export, which `pnpm check` caught via Fallow.

Verified against three injected regressions: a new SessionState field with no
direct write (the reviewer's exact scenario), a foreign write through an alias
binding, and a direct write to a store-established field. All three rejected.
`pnpm check` green, 4486 unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

* docs(daemon): correct the snapshot-lineage claim, and pin the real contract

Device verification of the snapshot-lineage route found that a ref pinned before
a `diff` keeps resolving with no pinned-ref warning. That is the designed ADR
0014 behaviour, not a regression — the comment describing it was wrong, and I
propagated it.

`main`'s comment in snapshot-runtime.ts said a diff "leaves client refs pinned to
the previous generation, which is exactly what the pinned warning diagnoses". The
counter and the authorization epoch are different clocks:

  - `diff` passes `issuesRefsToClient: false`, so it never reactivates the frame;
  - `resolveRefStalenessWarning` compares a pin against the frame EPOCH, not the
    observation counter, and its own comment says why — a capture that bumped the
    counter must not make a valid pin from the issuing frame look stale.

So advancing the counter is not the same as invalidating client refs, and the
observable the comment promised does not exist. I carried the sentence into
`setSnapshotLineage`'s doc when the transition moved, and then into a hardware
verification request, which cost a reviewer a device run against a false claim.

`setSnapshotLineage` itself is unchanged and was a pure move: same expressions,
same inputs as the inline assignments it replaced, so this route behaves exactly
as it does on main.

A comment that contradicts the code should be an assertion instead, so the
contract is now pinned in session-snapshot.test.ts: the diff advances the counter,
preserves the epoch, leaves the pre-diff pin resolving without a warning, and
still warns for a pin from a different frame. Verified to fail when the epoch
comparison is swapped for the counter. A second test covers the keep-current
branch, which had no coverage.

`pnpm check` green, 4488 unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-27 08:08:34 +02:00
devin-ai-integration[bot] 4e06304b12 fix: align layering claims, remove app-log ineffective dynamic import, refresh architecture records (#1222) (#1227)
* fix: align layering claims, remove app-log ineffective dynamic import, refresh architecture records (#1222)

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* docs: fix layering spine ordering (cli top), ADR 0009 deferred-scope wording, terse app-log-request-scope comment (#1222)

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* docs: show full rank groups in layering spine diagrams; clarify back-edge order vs literal imports (#1222)

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* docs: mark replay ADR context historical

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* docs: refresh replay repair and app-log boundary status

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-07-13 12:22:17 +02:00
devin-ai-integration[bot] d585d74172 chore: close out architecture experiments (#1213)
* chore: close out architecture experiments

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* docs: record unavailable live experiment

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* ci: make Android perf script atomic

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* docs: explain atomic perf workflow

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* chore: simplify back-edge diagnostics

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-07-11 15:07:32 +02:00
devin-ai-integration[bot] 0a8ea3a57b refactor: consolidate architecture ownership and client results (#1210)
* refactor: consolidate architecture ownership and client results

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: keep selector parse chunk grouping current

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: update moved architecture breadcrumbs

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: enforce moved selector architecture

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: keep selector guarantee ownership current

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* docs: update selector ownership references

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-07-11 09:40:24 +02:00
devin-ai-integration[bot] ae74c51abd chore: add agent-efficiency regression guards (#1174)
* chore: ratchet architecture dependency graph

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* test: ratchet agent-facing output economy

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* feat: derive command navigation explanations

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: keep efficiency checks fallow-clean

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(layering): enforce back-edge ceiling monotonicity and cover root src files

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* ci(layering): pin back-edge-ceiling ratchet to PR merge-base

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* test(output-economy): baseline-independent actionability floors, policy-derived error, like-for-like screenshot surfaces

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(explain): resolve true CLI aliases, canonical usage, and derived owners

Surface true CLI aliases from parser normalization (long-press, metrics,
tap, launch, relaunch) distinct from catalog keys, preserving implied-flag
semantics (relaunch => open --relaunch). Extract the canonical single-line
usage builder to src/utils/cli-usage.ts so schemas without usageOverride
include positionals and flags. Replace guessed handler paths with a
completeness-checked daemon-route owner map keyed by the closed
DaemonCommandRoute union, fixing silently-dropped non-kebab routes
(reactNative, recordTrace) and generic dispatch. Add table-driven coverage
for aliases, synthesized usage, split-family/route-variant/dispatch owners,
structured output, and explain:command CLI exit/stdout/stderr.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: enforce exact ratchets and compact command explain

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: colocate command ownership metadata

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: bind daemon owners to production routes

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: preserve generic dispatch bundling

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix: enforce monotonic output budgets

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-07-10 11:54:09 +02:00