Repository navigation
refactor(daemon): move CLI-shared daemon modules out of the daemon package and gate CLI reachability (R78) - #3625
Conversation
|
There was a problem hiding this comment.
All reported issues were addressed across 118 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Size Report
Startup median (7 runs, lower is better):
|
|
The reach walk in R78 duplicates R82's walk, so this needs one change before merge. The other code looks fine at 6b96029, and CI is green (43 checks, 0 failing, including check:layering and the eager-closure budget test). I did not run check:layering or typecheck locally, so "the CLI reaches no daemon module" rests on that CI result, and I did not recount the subpath exports or check removed subpaths for leftover importers beyond what typecheck proves.
Should daemon-contracts own the device-claim reconciler at all? It now depends on capture-kit at runtime because the reconciler drives app-log, audio-probe and perf-capture recovery, and it holds a process-lifetime On the open review threads, the realpath skip for a missing dirname (P1) and emitDiagnostic returning early without a scope (P2) both exist on the base commit and are moved verbatim here. They are fine as separate fixes. Two lower-priority threads still apply: the missing dynamic-import case in the folder-ban tests, and the ADR 0032 "flat ban" wording. These do not apply, so please resolve them: ADR 0033 and type-only edges (the ADR describes the folder ban, which does cover type-only edges), require()/createRequire (no production use exists, and this is a limit of the shared import parser, not R78), and OwnerScopedClaimRecovery missing from the facade (no consumer names it). |
…ckage The device-claim record, store, inspection and settlement, the session artifact paths and the stop path now live in daemon-contracts, which the CLI and the daemon both read. The claim reconciler, its owner-scoped composer and the recovery platform scope sit beside them; the app-log durable resource, its recovery and the app-log file policy sit in capture-kit with the other durable captures. The device identity decode moves to kernel, so no contracts module needs capture-kit for it. `device release --stale` and the platform runtimes it composes reach no daemon module, and `DaemonCommandRoute` is read from command-registry. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
R78 walks static and dynamic edges from src/bin.ts and src/cli.ts and fails on any reached module outside the client-side roots that imports the daemon package, naming the chain. R82 and R78 share one walker, process-reach.ts. The client-side policy in LOGICAL_MODULE_POLICIES now covers the CLI, typed client, remote transports and daemon client. The two moved entries that carry the durable-capture machinery and the claim fixtures entry are approved over the new-entry ceiling, and the ADRs say what the rules do now. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
6b96029 to
3ce8ac0
Compare
|
Addressed at
|
|
The code at 3ce8ac0 looks good to me. The earlier findings from 6b96029 (#3625 (comment)) are fixed: ADR 0033 now separates the client-side direct-edge policy from R78's reach hop, the dynamic-import test runs for each of the five client-side roots, and the ADR 0032 note says where the flat ban lives. Not blocking: the new device-claim-recovery.ts row in docs/adr/0033-collocation-decision-table.md:148 cites ADR 0019 section 1, but the rule that root composition supplies only the gateway is in section 10, and ADR 0019:850-851 still names the old packages/daemon path for claim-recovery-gateway.ts, so you can cite section 10 and update that path to daemon-contracts, or leave it. I reviewed only the new changes in 3ce8ac0 and did not run check:layering, typecheck or the R78 and R82 tests myself, so "the real tree passes R78" rests on your check:affected run. All 14 jobs that did not pass were cancelled and none failed, so CI shows no failing step, and the changes touch only scripts/layering, the eager-closure budget table and ADR text. Please rerun CI on 3ce8ac0 so the checks produce a result before merge. |
Summary
L1b of the daemon single-entrypoint stack, on top of #3616: the CLI process reaches no module of
@agent-device/daemon, and R78 now proves it by reachability instead of a folder ban.daemon-contracts: the device-claim cluster (record, store, inspection, reboot, settlement, claims),session-artifact-paths(with the managed-path check),daemon-stop, and the claim reconciler + owner-scoped composer + recovery platform scope.capture-kit: the app-log durable resource, its store and recovery, and the app-log file policy (they sit with the other durable captures).kernel/device:decodeDeviceIdentity(was in capture-kit), sodaemon-contractsdoes not depend on capture-kit for claim records.DaemonCommandRouteis read fromcommand-registry.device release --stalecomposes from root platform runtimes (src/platform-runtime-claim-recovery.ts) and reaches no daemon module.src/cli/commands/device-release.tsis gone.daemon-client-entry) is now reach-only: a breadth-first walk over static and dynamic edges fromsrc/bin.ts/src/cli.tsthat fails on any reached module outside the client-side roots that imports the daemon package, naming the chain. R82 and R78 share one walker (scripts/layering/process-reach.ts). The folder ban oversrc/cli.ts,src/cli/,src/client/,src/remote/andsrc/daemon-client/is theclient-sideentry inLOGICAL_MODULE_POLICIES, which refactor(daemon): move wire request and response types out of the daemon package #3616 introduced forsrc/daemon-client/. The tests plant type-only, value and dynamic imports on every root, the exact laundering chaincli.ts → cli/commands/device.ts → platform-runtime-claim-recovery.ts → daemon, a type-only hop that must not count, and a stale-entry case. On the first run the walk also foundplatform-runtime-app-log-output.ts/-process.tsimporting daemon helpers; both moved too.AGENTS.mdsession-artifact-paths pointer follows the move.Subpath exports
daemondaemon-contractsdevice-claims,device-claims-fixtures,device-claim-recovery,daemon-stop,session-artifact-paths)capture-kitapp-log-durable-resource,app-log-resource-recovery,app-log-files)kernel,host-kit,test-kitNet: 210 → 207. The device-claim cluster is one explicit-name entry (
device-claims) instead of seven files exported separately. Two judgment calls to review:daemon-stopkeeps its own subpath. It lazily importsdaemon-registration-owner, and its test mocks that seam, so merging it into the owner module (or re-exporting it from one) would either make a static cycle or break the test.daemon-contracts(ADR 0033 records why: the daemon and the daemonless release both run it, and ADR 0019 section 1 keeps the transaction with the shared code), which depends oncapture-kit. The reverse (capture-kit → daemon-contracts) is a TypeScript project-reference cycle throughcommand-registry → selectors → capture-kit, and there is no package above both. The dependency stays acyclic; if the layering intent is "contracts below capture", the reconciler needs a new package, which I did not create.Eager closure
scripts/__tests__/eager-closure-budgets.test.tspasses with no growth on any existing entry. Three new entries carryAPPROVED_OVER_CEILINGrows (app-log-durable-resource.ts,device-claim-recovery.ts,device-claims.fixtures.ts): each is code moved verbatim from a daemon entry of the same weight, and only the daemon and the lazily loadeddevice release --stalepath import them. The first draft widenedcapture-kit's root barrel (32 → 63 modules) anddaemon-resolution; both were reverted to separate entries for that reason. Four tests that needtest-kitstay underpackages/daemon(test-kit depends on daemon-contracts, and capture-kit/daemon-contracts tests cannot import it): the claim, settlement, reconciliation and app-log-files tests exercise the moved modules through their public entries.Diff size
118 files, +744 −582 over #3616: over the 1,000-line budget. Most of it is moves counted twice (rename detection does not pair the merged or split files: the kernel decode move is +85/−90, the claim reconciler, owner recovery and scope moves another ~300) and import rewrites across the daemon tests. If you want this under budget I would split the kernel/capture-kit moves (
decodeDeviceIdentity, app-log durable resource and file policy) into a lower layer.Validation
pnpm check:affected --runpasses on3ce8ac06c(L1b) and on824077b38(#3616), both rebased on origin/maine649bc0;pnpm check:daemon-wire-compatpasses on both. Earlier L1b attempts failed on the eager-closure budget,production-exportsandfallow(all fixed above) and twice ontmpdir-leaks-modelwhile two other threads' gates ran on this host; that check passes alone twice and in the final runs. A fresh-context review of the first L1b draft found only stale path references (fixed); the maintainer review of this shape asked for the shared walker and the policy entry, both done.No device-facing behavior changes;
device release --staleruns the same reconciler code, composed from the same root gateway.🤖 Generated with Claude Code