refactor(capture-kit): move durable-capture resource mechanics out of the daemon - #2320
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Sentinel at efd5d86: coordinator + independent Terra/high review found one P2 regression-test gap. In src/daemon/tests/durable-capture-resource.test.ts, the new confirmed failed-adoption cleanup test starts with an unblocked ledger; deleting clearUndurableCleanup from the newly moved composition mapping still leaves assertStartAllowed green. Seed blockUndurableCleanup(context.device, ...) before adoption, then assert confirmed cleanup clears that block. Validate the test red with the clear mapping removed and green restored. This is a missing regression for the relocated policy seam, not a claimed production behavior bug. Otherwise the mechanics preserve fence/recovery/error/cleanup ordering; the 30-module subpath exception has evidence that actual consumer closures shrink, and package growth is only +481 B. Exact-head CI including Coverage and Integration is green. Hold readiness label until the mapping regression is effective. No fixes or merge performed. |
|
Fixed in 85d1485. The clear mapping is now effective: the test seeds |
…anup report The durable-capture mechanics reached two daemon-owned authorities directly: the concrete `SessionStore` class plus `SessionState`, and the admission ledger, which `recoverFailedAdoption` called to block or clear a replacement start. Both are daemon policy, so neither can travel with the mechanics. Replace them with a two-member `DurableCaptureSessionStore<S>` port and a session type parameter, and let the mechanics report what they observed — `DurableCaptureCleanupOutcome` — while `createDurableCaptureResource` keeps the clear/block decision and the reason text. Recovery takes the session directory resolver from its caller instead of importing `safeSessionName`. Splitting `DurableCaptureRecordDefinition` out of the definition says which half needs a session at all: recovery, finish-recovered, and start preflight terminalize a persisted record with no session in hand.
… the daemon The daemon held two halves of one mechanism. capture-kit already owned the durable-resource envelope, JSON, and descriptor codec; the fence, transition, adoption, and recovery mechanics that operate on that envelope still sat in `src/daemon` as eight files. Move them behind the store port and cleanup report the previous commit introduced, exposed through one new `@agent-device/capture-kit/durable-capture` subpath — not the `.` index, which is the eager closure every platform runtime imports. Admission, start preflight, runtime binding, the kind stamps, and the composition root that wires the mechanics to the admission ledger stay daemon policy. The moved tests exercise the mechanics through a resource kind and session type of their own, so what they prove is that the mechanics need neither the daemon's closed kind set nor `SessionState`. The composition root keeps the admission mapping the adoption test used to assert, now in `durable-capture-resource.test.ts` where the ledger lives.
…export The daemon's store and adoption modules were the `.` index's only production consumers of `decodeDurableResourceEnvelope`; both now sit beside the encoder inside capture-kit and import it directly.
…ed surface `tsc -b` cannot name the fixture spy's inferred type across the package boundary, and five re-exported vocabulary types had no consumer.
The confirmed-cleanup test started with an unblocked ledger, so deleting `clearUndurableCleanup` from the relocated composition mapping still left `assertStartAllowed` green. Seed a block first, so the assertion is that the mapping lifted it.
|
The test gap is fixed at No new findings. The remaining blockers are conflicts with main and missing CI coverage on this commit. Please resolve the conflicts and run the required checks. |
85d1485 to
e186d72
Compare
|
Rebased onto
Re-validated on the new base: One update to the pre-existing failure noted in the description: |
|
Closes #2317.
The daemon held two halves of one mechanism.
@agent-device/capture-kitalready owned the durable-resource envelope, JSON, and descriptor codec; the fence, transition, adoption, and recovery mechanics that operate on that envelope still lived insrc/daemon.The four seams, and what replaced them
SessionStore.set/.resolveSessionDirDurableCaptureSessionStore<S>— a two-member portSessionStateS; only a definition's ownsessionSlotlooks inside itsafeSessionNameresolveSessionDir(sessionId)from its callerDurableCaptureAdmissionLedgerDurableCaptureCleanupOutcome, reported by the mechanics; the daemon decides admissionrecoverFailedAdoptioncomputesconfirmedexactly as before —(!persisted && cleanupError === undefined) || transition.confirmed— and reports it.createDurableCaptureResourcemapsconfirmed: truetoclearUndurableCleanupandconfirmed: falsetoblockUndurableCleanup(device, reason)with the same reason string, still applied before the primary error is rethrown. The admission ledger is never named inside capture-kit.Splitting
DurableCaptureRecordDefinitionout ofDurableCaptureResourceDefinitionsays which half needs a session at all: recovery, finish-recovered, and start preflight terminalize a persisted record with no session in hand, so they name the record definition and carry no session type. That also drops the handle parameter from those signatures, which was only ever there to reachsessionSlot.What moved, and what stayed
Eight files (1,390 lines) left
src/daemonforpackages/capture-kit/src/durable-capture/, behind one new@agent-device/capture-kit/durable-capturesubpath. It is deliberately not on the.index — that index is the eager closure every platform runtime imports.The daemon keeps five
durable-capture-*files, 297 lines: the admission ledger, start preflight, runtime binding, the closed kind set, and the composition root that wires the moved mechanics to the ledger and the session store's naming rule. All sixteen kind stamps stay.git show -M90%reads every production move as a rename (90–100% similarity);git show -M90% 50f9a5dhas the summary.Tests
The mechanics tests moved beside their sources and now run against a resource kind and a session type of their own, so what they prove is that the mechanics need neither the daemon's closed kind set nor
SessionState. Their fixture handle reproduces the shipped live-handle contract — finish and cleanup run once, and disposing an unconfirmed cleanup throws — because that throw is the signal a failed adoption reports.The admission mapping the adoption test used to assert moved to
durable-capture-resource.test.ts, where the ledger lives: one test for the clear, one for the block.Planted-red proof
No new structural rule, so the proof is that the existing ones bite.
withDurableCaptureResourceFence→fence.test.ts > rejects a stale fence before its side effectfails by name.src/daemon/types.tsfrom the moved adoption module → R11 red ('../../../../src/daemon/types.ts' escapes packages/capture-kit/), plus R10 on the daemon/types.ts importer count.durable-capture-admission-ledger.tsfrom the moved adoption module → R11 red the same way.All three removed.
Validation
Green:
pnpm check:tooling(format, lint, typecheck, layering, depgraph, gate-manifest, production-exports, tmpdir-leaks, xctest-selection, mcp-metadata, build, bundle-owner-files, package), fullvitest --project unit-core(1200 files, 8980 tests),src/__tests__/cli-startup-import-closure.test.ts,scripts/__tests__/eager-closure-budgets.test.ts.Two things worth a reviewer's eye:
APPROVED_OVER_CEILINGrow. The new subpath is a first-introduceddomain-facadeentry evaluating 30 modules against a ceiling of 20. The entry surface is new; the weight is not — the store publishes throughhost-kit/fileand adoption validates through the envelope codec, and both edges came along unchanged fromsrc/daemon. The gate itself confirms no existing entry grew.check:affectedfails on one unrelated test.session-open-runtime.test.ts > open --metro-port alone stays host-ambiguous on a physical Android devicetrips the hermetic-signal guard whenvitest relatedschedules it alongside this file set. It passes alone and in the fullunit-corerun, and it fails identically onmainat 80997b6 with a comparablevitest relatedselection, so it is pre-existing and not caused by this change.Two small cleanups the move forced:
decodeDurableResourceEnvelopeleft the capture-kit.index (the daemon's store and adoption modules were its only production consumers, and they now sit beside the encoder), and the four kind-stamp recovery files had their duplicated subpath imports merged.