Repository navigation
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
The daemon refactor looks sound, but one test gap needs fixing before merge. At c3a81fd, 18 of 18 checks pass, and I know of no conflicts. The test fixture at src/daemon/tests/daemon-runtime-fixture.ts:26 copies the body of the Could the pair go away entirely if the server took a factory instead of built deps? The server would keep owning paths, the policy load (with its typed stderr and exit) and the Not blocking, take or leave: I did not run the layering, eager-closure or fallow gates, so the R76/R9 results and the 468 to 469 closure claim rest on the PR body and green CI. I also did not reproduce the live |
…cess root startDaemonRuntime now receives its composed dependencies instead of building them: provider runtimes, the platform gateway and inventory gateways, request platform providers, host diagnostics, credential fingerprinting, the platform lifecycle owners, the platform-services and claim-recovery compositions, and the command-surface factory all move to src/daemon-composition.ts, which src/daemon.ts composes and injects. The operational server keeps startup sequencing, listeners, shutdown, the metadata watch, idle reap, and request-handler creation, with the same order on both paths. DaemonProviderCredentials moves to @agent-device/daemon-contracts/provider-credentials, the #2559 shared home beside daemon-policy-file; the fingerprint implementation stays in the root. Tests boot the runtime through one shared fixture. No production file under src/daemon/ now reads a non-package src/ module except the direct-ios-selector edge owned by #2273/#2274.
…s as a factory startDaemonRuntime now takes a composeDeps factory and keeps the daemon paths, the ADR 0029 policy load (typed stderr and exit included), the SessionStore, the session callbacks, and the assertShutdownAllowed derivation in the server, so the process root and every runtime test share one assembly and the second resolveDaemonPaths and the policy round-trip go away. Skipped provider runtimes are data on the deps and the server emits provider_runtime_skipped through emitDaemonDiagnostic; the lease-lifecycle facts, the runtime gateway, the resource-cleanup surface, and the claim-recovery factory are declared once at the top level of DaemonRuntimeDeps. The fixture is a thin call through the real factory, and a new assembly test proves a dropped shutdown gate, a mis-wired store, or an unreadable policy cannot pass silently.
The daemon-runtime rows of the R76 inventory no longer describe tree reality once the composition moved out of src/daemon/, so the stale entries go and the planted tests retarget the one surviving classified pair. daemon-composition is declared in the platform-runtime zone (a composition, not an executable), and ADRs 0019/0022/0033 record where the composition and the extracted credential type landed.
c3a81fd to
3ebaa58
Compare
|
Rebased onto main (
Closure stays at main's 468: the entry reaches the composition through a function-scoped |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 31 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/adr/0022-daemon-platform-runtime-coupling.md">
<violation number="1" location="docs/adr/0022-daemon-platform-runtime-coupling.md:14">
P3: Section 2 follows this amendment, so “above” points readers in the wrong direction. Change it to “below.”</violation>
<violation number="2" location="docs/adr/0022-daemon-platform-runtime-coupling.md:14">
P3: This R76 importer path omits the `src/` prefix, so the amended ADR points readers to a nonexistent path. Use the repository-relative path `src/daemon/direct-ios-selector.ts`.</violation>
</file>
<file name="src/__tests__/daemon-composition-assembly.test.ts">
<violation number="1" location="src/__tests__/daemon-composition-assembly.test.ts:100">
P2: The runtime is shut down only after all assertions, so a failed assertion leaves its HTTP server active and can hang the Vitest worker. Keep the controller outside `try` and shut it down in `finally`.</violation>
<violation number="2" location="src/__tests__/daemon-composition-assembly.test.ts:103">
P3: These directories come from `mkdtempForTestSync`, whose contract delegates removal to run-level TMPDIR teardown; remove both per-test `rmSync` calls.
(Based on your team's feedback about Vitest temporary-directory cleanup.)</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| outputPath: store.resolveAppLogPath('web-assembly'), | ||
| pidPath: store.resolveAppLogPidPath('web-assembly'), | ||
| }); | ||
| await runtime?.shutdown(); |
There was a problem hiding this comment.
P2: The runtime is shut down only after all assertions, so a failed assertion leaves its HTTP server active and can hang the Vitest worker. Keep the controller outside try and shut it down in finally.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/__tests__/daemon-composition-assembly.test.ts, line 100:
<comment>The runtime is shut down only after all assertions, so a failed assertion leaves its HTTP server active and can hang the Vitest worker. Keep the controller outside `try` and shut it down in `finally`.</comment>
<file context>
@@ -0,0 +1,140 @@
+ outputPath: store.resolveAppLogPath('web-assembly'),
+ pidPath: store.resolveAppLogPidPath('web-assembly'),
+ });
+ await runtime?.shutdown();
+ assert.deepEqual(exits, [0]);
+ } finally {
</file context>
| hubs, and the platform-services and claim-recovery dynamic edges) moved out of `src/daemon/` into | ||
| the process-root composition module `src/daemon-composition.ts`, which R76 does not see because it | ||
| is no longer a daemon-zone importer. The R76 inventory now carries only the leaked | ||
| selector-query edge (`daemon/direct-ios-selector.ts`); the section-2 outcomes above remain the |
There was a problem hiding this comment.
P3: Section 2 follows this amendment, so “above” points readers in the wrong direction. Change it to “below.”
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0022-daemon-platform-runtime-coupling.md, line 14:
<comment>Section 2 follows this amendment, so “above” points readers in the wrong direction. Change it to “below.”</comment>
<file context>
@@ -6,6 +6,14 @@ Accepted (2026-09-06). Implements the audit deliverables of issue #2278. The cla
+hubs, and the platform-services and claim-recovery dynamic edges) moved out of `src/daemon/` into
+the process-root composition module `src/daemon-composition.ts`, which R76 does not see because it
+is no longer a daemon-zone importer. The R76 inventory now carries only the leaked
+selector-query edge (`daemon/direct-ios-selector.ts`); the section-2 outcomes above remain the
+record of how each was classified.
+
</file context>
| selector-query edge (`daemon/direct-ios-selector.ts`); the section-2 outcomes above remain the | |
| selector-query edge (`daemon/direct-ios-selector.ts`); the section-2 outcomes below remain the |
| await runtime?.shutdown(); | ||
| assert.deepEqual(exits, [0]); | ||
| } finally { | ||
| fs.rmSync(stateDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
P3: These directories come from mkdtempForTestSync, whose contract delegates removal to run-level TMPDIR teardown; remove both per-test rmSync calls.
(Based on your team's feedback about Vitest temporary-directory cleanup.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/__tests__/daemon-composition-assembly.test.ts, line 103:
<comment>These directories come from `mkdtempForTestSync`, whose contract delegates removal to run-level TMPDIR teardown; remove both per-test `rmSync` calls.
(Based on your team's feedback about Vitest temporary-directory cleanup.) </comment>
<file context>
@@ -0,0 +1,140 @@
+ await runtime?.shutdown();
+ assert.deepEqual(exits, [0]);
+ } finally {
+ fs.rmSync(stateDir, { recursive: true, force: true });
+ }
+});
</file context>
| hubs, and the platform-services and claim-recovery dynamic edges) moved out of `src/daemon/` into | ||
| the process-root composition module `src/daemon-composition.ts`, which R76 does not see because it | ||
| is no longer a daemon-zone importer. The R76 inventory now carries only the leaked | ||
| selector-query edge (`daemon/direct-ios-selector.ts`); the section-2 outcomes above remain the |
There was a problem hiding this comment.
P3: This R76 importer path omits the src/ prefix, so the amended ADR points readers to a nonexistent path. Use the repository-relative path src/daemon/direct-ios-selector.ts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/adr/0022-daemon-platform-runtime-coupling.md, line 14:
<comment>This R76 importer path omits the `src/` prefix, so the amended ADR points readers to a nonexistent path. Use the repository-relative path `src/daemon/direct-ios-selector.ts`.</comment>
<file context>
@@ -6,6 +6,14 @@ Accepted (2026-09-06). Implements the audit deliverables of issue #2278. The cla
+hubs, and the platform-services and claim-recovery dynamic edges) moved out of `src/daemon/` into
+the process-root composition module `src/daemon-composition.ts`, which R76 does not see because it
+is no longer a daemon-zone importer. The R76 inventory now carries only the leaked
+selector-query edge (`daemon/direct-ios-selector.ts`); the section-2 outcomes above remain the
+record of how each was classified.
+
</file context>
| selector-query edge (`daemon/direct-ios-selector.ts`); the section-2 outcomes above remain the | |
| +selector-query edge (`src/daemon/direct-ios-selector.ts`); the section-2 outcomes above remain the |
|
At 3ebaa58 I found no blocking problems, and all 19 checks pass. This follows the earlier review at c3a81fd, whose evidence request is now met. I ran no gates myself. The layering, di-seams, eager-closure (the 468-module claim), fallow, provider integration and concurrency torture results rest on your report and green CI. I did not reproduce the built-daemon smoke or the Not blocking, take or leave: Four Cubic threads still apply. The P2 on the assembly test not shutting down its runtime in finally is at #3431 (comment). It only leaves a listener open in a failing test and production is unaffected, but please add the shutdown in finally or reply on the thread before merge. Three P3 threads also apply: #3431 (comment), #3431 (comment) and #3431 (comment). No conflicts are known. Apart from that P2 thread, nothing blocks merge; the three P3 notes are optional. |
This branch has not been deployed
1 participant
Summary
PR 6 of the daemon extraction (#3276 umbrella): the daemon server no longer builds its own dependencies.
startDaemonRuntimetakes{ env, sessionStore, deps: DaemonRuntimeDeps }; everything that names a platform runtime, provider runtime, interactor, host diagnostics, credential fingerprinting, resource cleanup, lifecycle owners, or the platform-services / claim-recovery compositions moves to the new root composition modulesrc/daemon-composition.ts, whichsrc/daemon.tscalls and injects. The operational server keeps startup sequencing, listeners, shutdown, metadata watch, idle reap, and request-handler creation, with the same order on both paths.DaemonProviderCredentials(type only) lands in@agent-device/daemon-contracts/provider-credentials— the #2559 daemon⇄client shared home besidedaemon-policy-file.ts;contractswas rejected under R18 (the fingerprint implementation keeps reading the plugin manifest in root, and the type is daemon⇄client vocabulary, not app-domain vocabulary). Runtime tests boot through one shared fixture (src/daemon/__tests__/daemon-runtime-fixture.ts). R76 loses its 8 daemon-runtime rows (only thedirect-ios-selectoredge from #2273/#2274 remains); ADRs 0019/0022/0033 amended. 30 files.Validation
Commit c3a81fd. Local:
pnpm install --frozen-lockfile && pnpm build,pnpm format,check:quick,check:layering(320/0),check:di-seams, eager-closure budgets (802/0;src/daemon.tsclosure 468→469 modules = the new composition file itself, weight 5117→5116 — containment + novelty + flat weight, the split tolerance; every base module stays eager), test-file-size-ratchet,test:integration:provider(213),test:concurrency-torture(4),check:affected --run, daemon-scope unit run (462 files / 3455 tests).pnpm depgraph: spine back-edges 0, type-only inversions 11 (unchanged). Edge query over graph.json fromdaemon/**nodes to non-packages/nodes outside the zone: exactlydaemon/direct-ios-selector.ts -> platform-runtime-apple-resources.ts [value]. Smoke:node bin/agent-device.mjs devices+session listagainst the composed daemon, cleandaemon stop. Provider integration/coverage run remotely on this head.