Skip to content

refactor: daemon and contracts decoupling, wave 3 (registry package, type split, leaf moves) #2335

Closed

Description

@thymikee

Purpose

Third decoupling wave for src/daemon and @agent-device/contracts, following #2316, #2317 and #2318. Measured on main at 27a97ee619 with a production-only import graph (1,629 files, 8,805 edges):

  • src/daemon: 350 production files, 56.8k lines; 197 flat top-level files (30k lines). The value graph is acyclic and the daemon has zero edges into platform packages. It has 154 value edges into 67 root src/ files; the top hubs are command-catalog.ts (15 daemon files), core/command-descriptor/registry.ts (13), core/dispatch-resolve.ts (12), provider-device-runtime.ts (11).
  • @agent-device/contracts: 190 modules, 18.2k lines, 118 subpaths. 106 modules export runtime code (13k lines). 146 modules are reachable from a platform/provider package; 44 are not; 19 are reachable from no package at all.

The command descriptor registry is the keystone: it is package-ready today (its only root dependency is core/parse-timeout.ts) and it is what forbids packaging the daemon client and the session event journal under R11.

Children (dependency order)

Rules for every child

Evaluated and not scheduled

  • @agent-device/gestures (gesture-plan, android-touch-plan, apple-multitouch-support, gesture-admission; ~780 lines with a clean DAG below the contracts hubs): viable, deferred for new-package cost.
  • Client-contracts audience split (client-* ×14 + facades/client.ts, 1.4k lines, all type-only consumers): buys ownership clarity, not coupling reduction.
  • src/daemon/session-target-evidence.ts: needs capture-kit and ad-script; no package depends on both.
  • HTTP artifact transfer cluster (15 top-level files, 1,984 lines, 0 root deps): a logical module, not a package; belongs with refactor(daemon): audit and retire residual semantic platform coupling #2278's ownership work.
  • Maestro adapter fold into replay/: rejected, R9 engine-cycle prefix.

Completion

All seven children are merged and both measurements have been re-run and recorded — see the close-out comment for the numbers, the filter they were measured under, and the findings carried forward. Closing.

Activity

  1. thymikee commented on Sep 6, 2026

    @thymikee
    MemberAuthor

    Reconciliation and execution plan

    Cross-checked all seven children against the umbrella checklist. The declared dependencies are mutually consistent — every Dependencies section agrees with the reverse edge stated by its blocker (A names D/F1/F2; C names F1/F2; E names F2's code-signature need; B and F2 agree on doctor-progress.ts). No contradictions to resolve, so the graph below is the umbrella's as written.

    main ──┬── A #2336  registry package ──┬── D #2339  contracts single-owner moves
           │                               │
           ├── C #2338  types.ts split ────┴── F1 #2341  session event journal
           │
           ├── B #2337  commands-side rendering ──┐
           │                                      ├── F2 #2342  daemon client relocation
           └── E #2340  four leaf moves ──────────┘
    

    Waves

    Wave 1 — in flight, all based on main, developed in parallel:

    issue branch
    A #2336 claude/2336-command-registry-package
    B #2337 claude/2337-commands-rendering-out-of-daemon
    C #2338 claude/2338-daemon-types-split
    E #2340 claude/2340-daemon-leaf-moves

    Wave 2 — D stacks directly on A's branch. F1 stacks on an integration branch merging A and C, because it needs A's catalog/registry package and C's wire-only request shape.

    Wave 3 — F2 stacks on an integration branch merging A, B, C and E.

    Integration branches are cut by the coordinator once their inputs are pushed, so the dependent PRs review as their own diff rather than re-showing their bases.

    Interlocks being watched

    • C → F1. F1's completion condition (no path from a journal input type to SessionState or DeviceLease) is only satisfiable if C's wire-only shape actually lands. F1's fallback to @agent-device/kernel/contracts public request types is a fallback, not the plan.
    • A → D. D must not move anything A's new package imports; A leaves contracts untouched, which keeps D's root-only set stable.
    • B → F2. F2 assumes src/daemon/client/doctor-progress.ts is already deleted, not deprecated.
    • E → F2. F2's remaining-seam inventory assumes code-signature*.ts has already left src/daemon.

    The two graph measurements in the umbrella's Purpose section get re-run and recorded here once every child is closed.


    Generated by Claude Code

  2. thymikee commented on Sep 6, 2026

    @thymikee
    MemberAuthor

    Wave 1 status and four corrections to this issue

    All five wave-1/2 children are in flight. B (#2337) is merged as #2349.

    issue PR state
    A #2336 #2348 resolving a conflict with the new main
    B #2337 #2349 merged
    C #2338 #2346 rebased onto main, CI re-running
    D #2339 — in progress, stacked on A's branch
    E #2340 #2347 clean against main, CI green so far

    Four things this issue gets wrong or leaves underspecified. Each is verified against the tree, not inferred.

    1. E is a prerequisite for A. This issue declares them independent.

    walkDaemonCodeGraph in the daemon code-signature followed relative import specifiers only. On main that covers 619 of the daemon's modules — the rest of the daemon, reached by workspace specifier, is invisible to the signature a client compares a running daemon against. Any child of this umbrella that moves code into a workspace package therefore removes that code from the daemon's signature.

    E hit this first (its four leaf moves took the walk to 611 and pushed the walker itself out of the graph it walks, which is the property code-signature-cache.ts's format guard rests on) and fixed it at the construction path in #2347: scoped specifiers resolve through the owning package's exports map, and the manifest is stamped so an exports retarget invalidates without either endpoint changing.

    A moves 12 modules into a brand-new package and is exposed to exactly the same defect. So is F1, which creates another package. E must land before A, or A ships the same blind signature. Sequencing accordingly.

    2. F1's dependency on C is not the one written here.

    #2341 requires the journal package to import nothing under src/ (R11). C's wire-only shape, DaemonWireRequest, is declared at src/daemon/daemon-request-wire.ts — under src/ — so a workspace package may not import it. As exported it cannot be F1's input type.

    It also is not needed. @agent-device/kernel/contracts already exports DaemonRequest and DaemonRequestMeta (packages/kernel/src/contracts.ts:100 and :73) carrying exactly the four fields the journal reads — command, flags, meta.requestId, meta.clientArtifactPaths — with no internal key and no path to SessionState or DeviceLease. C's module is itself built by narrowing that type.

    So #2341's "or @agent-device/kernel/contracts public request/meta types" clause is the path that actually applies, not the fallback it reads as. C still satisfies #2338 as written. F1 stays sequenced behind C only for conflict avoidance — C's 408-file diff touches the session-event-*.ts files F1 relocates.

    3. #2340's consumer list names two files that do not import the subject.

    #2340 lists sparse-fallback-screenshot.ts and generic-runtime-execution.ts as screenshot-overlay consumers. On main neither imports it; the real consumers are src/daemon/screenshot-runtime.ts and src/daemon/android-snapshot-timeout-evidence.ts. E followed the code, correctly.

    This matters beyond E: #2342 hand-lists its remaining seams the same way, and #2339 its consumer counts. Both are being told to verify every entry against the tree and report corrections, rather than treating the tables as authoritative.

    4. The closing measurement needs one definition, chosen now.

    This issue's Purpose states 154 daemon → root src/ value edges. A measured 150 → 114 with scripts/depgraph/build.ts (delta −36, exactly as predicted, ceiling met either way). The original figure was taken over a "production-only import graph (1,629 files, 8,805 edges)". Two definitions are in play and the Completion section promises this number gets re-run here. Pinning which filter counts — before the last child closes — avoids comparing two different measurements at the end.


    Generated by Claude Code

  3. thymikee commented on Sep 7, 2026

    @thymikee
    MemberAuthor

    Wave 3 complete — all seven children merged

    child issue PR commit
    A — command descriptor registry package #2336 #2348 2ec4e91
    B — commands-side rendering out of the daemon #2337 #2349 6768a04
    C — split daemon/types.ts #2338 #2346 dcd8b65
    D — contracts single-owner module moves #2339 #2357 bd08e6e
    E — four pure leaves to their kits #2340 #2347 b4ebd77
    F1 — session event journal package #2341 #2361 233a34d
    F2 — relocate the daemon client #2342 #2360 51ed621

    main is at 233a34d. Below are the two measurements re-run, per Completion.

    Measurement 1 — daemon → root src/

    150 → 113 value edges (−37), reaching 57 → 49 distinct root modules.

    Filter, stated because it decides the number: the checked-in pnpm depgraph graph (scripts/depgraph/build.ts), counting kind: value edges whose source is under src/daemon/ and whose target is a root src/ file outside src/daemon/. That graph carries no test files at all (0 of 1,660 nodes), so it is production-only by construction.

    It does not reproduce this issue's opening 154 / 67. At the same commit 27a97ee, pnpm depgraph reports 1,653 files and 8,484 edges where the issue's graph had 1,629 and 8,805 — a differently configured graph, so the endpoints are not comparable across the two. Both endpoints above are measured with the one filter named here. The −37 delta is consistent with the −36 the children predicted between them.

    One filter subtlety worth recording: F2 moved the daemon client to src/daemon-client/, which is outside src/daemon/ and so is now eligible to count as a root target. It contributes 0 — no daemon file imports the client, which was F2's premise.

    Remaining top hubs: provider-device-runtime.ts (11), core/dispatch-resolve.ts (10), core/replay-divergence.ts (6), core/lease-scope.ts (5), request/device-inventory-context.ts (5), runtime.ts (5).

    Measurement 2 — @agent-device/contracts

    27a97ee 233a34d
    production modules 190 180
    production lines 18,217 16,999
    exported subpaths 118 109
    modules reachable from a platform/provider package 164 159
    modules not so reachable 26 21

    The reachability rows use the same pnpm depgraph graph and likewise do not reproduce the issue's 146 / 44 / 19; the direction and the shrink are what the row is good for, not the absolute against a differently configured graph.

    Findings worth carrying forward

    Six stale facts in the children's issue tables, all caught by checking the tree rather than the issue text. #2340 named two wrong consumer filenames. #2339 claimed wait-runtime-plan.ts was root-only (following it would have broken R11, since A's package test imports it) and put snapshot-presentation.ts at 6 consumers where the tree had 2. #2342 omitted core/replay-divergence.ts and named src/commands/management/output.ts as a consumer it is not. All were measured at 27a97ee and drifted as the wave merged. Recommend future waves re-verify each table against the real base before starting, and that children post corrections rather than following the table.

    One dependency this issue did not declare: E before A. walkDaemonCodeGraph did not follow workspace subpaths, so A's new package was invisible to the daemon code signature until E's fix landed. The stated order had them independent.

    Cost that landed with it. E's signature fix grew the daemon source graph from ~620 to ~1,461 modules; the cached per-CLI-invocation signature went ~2.9 ms → ~7.5 ms. Correctness, paid for in startup.

    Bonus finds. F2's session-artifact-paths.ts extraction dropped src/cli.ts's eager closure 379 → 365 modules — the CLI had been pulling in the whole session store for one pure path helper. Separately, the wave's CI runs exposed a real defect in smoke:automation-system: it revealed its canaries with a blind scroll down <amount> and acted immediately, where iOS probes visibility and scrolls again. Fixed in #2370, which also covers the post-alert canary #2356 lists as still open.

    Deferred, not done

    • native-runtime.test.ts exceeds its 45 s xcrun budget on loaded macOS runners; one-line patch proposed on refactor(contracts): move single-owner modules out of @agent-device/contracts #2357, deliberately not applied inside a move PR.
    • Three of C's test files remain over the file-size ratchet.
    • The "evaluated and not scheduled" list above is unchanged; nothing in this wave made @agent-device/gestures or the client-contracts audience split more attractive.

    Leaving this issue open for a maintainer to close, since the completion condition is yours to accept.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions