Skip to content

refactor(contracts): move single-owner modules out of @agent-device/contracts - #2357

Open
thymikee wants to merge 4 commits into
claude/2336-command-registry-packagefrom
claude/2339-contracts-single-owner-moves
Open

refactor(contracts): move single-owner modules out of @agent-device/contracts#2357
thymikee wants to merge 4 commits into
claude/2336-command-registry-packagefrom
claude/2339-contracts-single-owner-moves

Conversation

@thymikee

@thymikee thymikee commented Sep 6, 2026

Copy link
Copy Markdown
Member

Summary

Closes #2339. Stacked on #2348 (child A) — base is claude/2336-command-registry-package, not main.

Nine modules leave @agent-device/contracts as git renames, no re-export shim at any old path. Contracts production source drops 18,217 → 16,992 lines (−1,225) across 190 → 180 files (find packages/contracts/src -name '*.ts' ! -name '*.test.ts' ! -path '*__tests__*' | xargs wc -l, base vs. HEAD). Exports map 118 → 109 subpaths, snapshot regenerated. 76 files changed; every moved module is R091R100 under git diff -M90% --name-status origin/claude/2336-command-registry-package...HEAD.

Corrections to the issue's tables, verified against the tree:

  • wait-runtime-plan.ts kept. A's packages/command-registry/src/__tests__/wait-runtime-execution.test.ts imports it, so it is not root-only. Moving it would make a package import root src/.
  • snapshot-presentation.ts has 2 platform-android consumers, not 6 (snapshot-presentation.ts, ui-hierarchy-builder.ts). Still single-package; moved.
  • Two consequences of the moves: android-touch-plan lands as touch-plan-lowering.ts and retires the package-local touch-plan.ts re-export barrel it fed; APPLE_OS_DISPLAY_NAMES folds into gesture-admission.ts, its one remaining contracts caller, so both gesture refusals share one copy of the wording without a new subpath. fast-check follows the property tests from contracts to platform-android.
  • replay-divergence.ts: implementation + its test to src/core, types stay on the /divergence subpath. ReplayVarScrubEntry follows the implementation — it is the sanitizer's parameter shape, not wire vocabulary. A split is not expressible as a rename.

interaction-guarantees.ts: kept. AGENTS.md names it a declaration site and points readers at that path; its non-daemon consumers are the interaction-contract test project and scripts/depgraph/affected-run.ts, i.e. repo tooling rather than a root zone. The ≥1,000-line target is met without it.

Validation

Tested SHA 6bf0b3d37c5c8541f6faffbfdb8fb5e659cab58e (the pushed head), after pnpm install --frozen-lockfile && pnpm build.

pnpm check:affected --run: format, lint, typecheck, layering, di-seams, fallow, mcp-metadata, build, package (R11 package-boundaries), integration-node, macos-coverage all pass. vitest-related: 9696 passed, 3 failed — app-log-session-resource, durable-capture-resource, capture-kit/durable-capture/adoption, all asserting a chmod 0500 directory is unwritable, which this container ignores because it runs as uid 0. All three are untouched by this branch and fail identically on the base.

The gates after that point were run individually and pass: replay-compat, daemon-wire-compat (after git fetch --unshallow --tags), affected-selector, gate-manifest, depgraph, tmpdir-leaks, coverage-model, wire-compat-model, production-exports, bundle-owner-files, command-docs, agent-guidance, xctest-selection, provider-integration, integration-progress, maestro-conformance, mutation-model, fixture-cache.

Fallow reports one pre-existing 19-line clone (src/commands/output/error.tssrc/mcp/tool-error.ts), byte-identical on the base and only in scope because both files changed one import line. No gate suppressed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SdXHhx9NkfH1PT4XjYe7tE


Generated by Claude Code

… zone

Six @agent-device/contracts modules had no consumer outside the root zones,
so the shared vocabulary package carried types only the daemon and root
composition ever read. Each one moves to the zone that owns it and every
consumer switches to the owning module; no re-export stays behind at the old
contracts path.

- perf-runtime-plan, snapshot-timeout-evidence, platform-resource-cleanup ->
  src/daemon
- daemon-owner-cleanup -> src/
- interaction-error -> src/core

wait-runtime-plan stays in contracts: @agent-device/command-registry consumes
it, so it is not root-only after the registry package landed.

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

Four modules in @agent-device/contracts had exactly one consuming package, so
the shared vocabulary carried Android- and Apple-specific shapes no other zone
could use. Each moves into the package that owns it, with every consumer
switched to the owning module and no re-export left at the old contracts path.

- android-helper-artifacts -> platform-android/src/helper-artifacts.ts
- android-touch-plan -> platform-android/src/touch-plan-lowering.ts, which
  also retires the package-local touch-plan.ts re-export barrel that existed
  only to give the contracts module a local name
- snapshot-presentation -> platform-android/src/snapshot-presentation-node.ts
  (renamed to keep the package's existing Android-specific
  snapshot-presentation.ts distinct)
- apple-multitouch-support -> platform-apple/src/multitouch-support.ts

APPLE_OS_DISPLAY_NAMES folds into gesture-admission.ts, its one remaining
contracts caller, so both gesture refusals still share one copy of the wording
without a new contracts subpath for a table its own doc calls non-public.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdXHhx9NkfH1PT4XjYe7tE
replay-divergence.ts mixed the wire vocabulary every zone reads with the
sanitizing, bounding and reporting implementation only root zones call. The
ten value consumers are all root (daemon replay, the session replay
coordinator, the daemon client lifecycle, the replay-test reporter, the
command error projection, and the MCP tool error), so the implementation moves
to src/core/replay-divergence.ts and carries its test unchanged.

The types stay in contracts and keep the @agent-device/contracts/divergence
subpath, which packages/ad-replay and packages/selectors type-import.
ReplayVarScrubEntry follows the implementation: it is the sanitizer's own
parameter shape, not part of the divergence wire report.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdXHhx9NkfH1PT4XjYe7tE
The nine relocated modules no longer live in @agent-device/contracts, so its
exports map drops their subpaths (118 -> 109) and
scripts/layering/contracts-exports.snapshot.json is regenerated from the
manifest, which is what R11 package-boundaries diffs the live surface against.

The two resolution assertions naming the retired snapshot-presentation and
snapshot-timeout-evidence subpaths go with them; interaction, snapshot and
react-native-overlay still cover both the direct-module and facade shapes the
assertions were there to prove.

The property tests that needed fast-check left with snapshot-presentation and
replay-divergence, so the dependency moves too: contracts drops it and
platform-android declares it, as fallow's unused-devDependency check reports.

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

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-2357/

Built to branch gh-pages at 2026-09-06 10:24 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.50 MB 4.49 MB -1.9 kB
Package (unpacked) 4.49 MB 4.49 MB -1.9 kB
Package (download) 1.33 MB 1.33 MB -702 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.5 ms 26.5 ms +0.0 ms
CLI --help 80.0 ms 76.7 ms -3.3 ms

@thymikee thymikee left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review from the #2335 coordinator, at 6bf0b3d. I re-derived the load-bearing claims from the tree. Everything checks out, and the wait-runtime-plan catch is the most valuable thing in this PR.

Verified independently against base fc40090:

  • Contracts production source 18,217 → 16,992 lines (−1,225), recomputed with my own count, not yours — matches your figure exactly and clears the ≥1,000 requirement.
  • All nine moved modules are absent from packages/contracts/src/: no re-export shim survives at any old path.
  • Zero suppressions anywhere in the diff — no fallow-ignore, no APPROVED_OVER_CEILING, no ts-expect-error, no disable comments, no skipped tests.
  • Commit structure is exactly right: three implementation commits, then one final chore(gates).

The wait-runtime-plan.ts correction is right, and it is subtler than the PR body says.

I confirmed it: packages/command-registry/src/__tests__/wait-runtime-execution.test.ts:3 imports @agent-device/contracts/wait-runtime-plan. Moving that module to src/daemon as #2339's table instructs would make a workspace package import root src/ — an R11 violation, and a gate failure you would then have been tempted to explain away.

What makes it worth calling out: #2339's table was correct when it was written and became wrong underneath you. It was measured on main at 27a97ee, where that test lived at src/core/command-descriptor/__tests__/ — genuinely root-only. A's packaging moved the test into a package and changed the fact. You verified against your actual base instead of the issue, which is the only reason this PR does not ship an R11 break.

That generalises, and I am carrying it forward: every table in this umbrella was measured at 27a97ee, and main has since absorbed #2349 and #2347, with #2348 and #2346 close behind. #2341 and #2342 inherit tables with the same staleness risk, and #2342's seam list is hand-written in precisely this style. Re-verification against the real base is now a standing requirement for the rest of the wave, not a nicety.

Your snapshot-presentation.ts correction (2 platform-android consumers, not 6) is the third issue-table defect this wave after #2340's two wrong filenames. Worth noting the pattern rather than fixing each silently — which you did, correctly, in the body.

Judgement calls I agree with: keeping interaction-guarantees.ts (AGENTS.md names it a declaration site; its non-daemon consumers are repo tooling, not a root zone, and the line target is met without it); folding APPLE_OS_DISPLAY_NAMES into gesture-admission.ts rather than minting a subpath for one caller; and saying plainly that the replay-divergence.ts implementation/type split "is not expressible as a rename" instead of dressing it up as one. ReplayVarScrubEntry following the implementation is right — it is the sanitizer's parameter shape, not wire vocabulary.

The pre-existing 19-line error.tstool-error.ts clone that fallow surfaces only because both files changed an import line is correctly left alone; adopting it here would be scope creep into a move PR.

Nothing blocking. CI is still settling — the chmod 0500 trio is the known uid-0 artifact of this container and fails identically on the base, and the alert-related device lanes have been intermittently red across this whole wave independently of any of these branches.


Generated by Claude Code

thymikee commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Smoke Tests failure on 6bf0b3d is not this PR's. Re-running it once; no code change from me.

Failing check: Smoke Testspackages/platform-apple/src/snapshot-source/native-runtime.test.ts > native snapshot foreground ownership: Error: Hook timed out in 60000ms at the beforeAll on line 9, which compiles a native binary from path.resolve(import.meta.dirname, '../../../../…'). 1 failed suite, 5 tests skipped, 61.6 s.

Why it is not this PR's — I checked the mechanism rather than assuming, because this PR does move modules into packages/platform-apple and a broken path there would be a plausible culprit:

  • A timeout is not a path break. An unresolvable nativeRoot fails immediately with ENOENT; this burned the full 60 s doing work, which is a native compile that did not finish in budget.
  • This PR's entire platform-apple footprint is 3 files / +52 lines: it adds the moved multitouch-support.ts and retargets two import lines in gesture-facts.ts and interactions.ts. It does not touch packages/platform-apple/package.json, vitest.config.ts, any vitest project definition, or anything under src/snapshot-source/ — so it cannot have moved that ../../../../… path or changed which project the test runs in or with what parallelism.
  • The same suite passed on this PR's base. refactor(core): move the command descriptor registry into its own workspace package #2348 at fc40090 — the exact base commit here — went fully green across the same unit-core project minutes earlier.

That leaves a slow native compile on a loaded macOS runner. Re-running once; if it fails again I will say so rather than re-running a second time.

For the record, this is the third distinct device/native lane failure across this wave (#2346 Android emulator wait-timeout, #2346 iOS alert XCUITest, this one a native-build hook timeout), on three branches that between them are a type split, a contracts move and a registry move. None share a mechanism with their failures. The macOS/device lanes look load-sensitive right now, which is worth a look independently of #2335.


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants