feat(daemon): the runtime owns its state and never the process

[thymikee]

Summary

Part of #3747, stacked on #3763. Closes #3752.

A daemon runtime no longer assumes it owns the process.

• Process ownership: startDaemonRuntime never calls process.exit and installs no process.on/once handler. src/daemon.ts owns signals, fatal errors and the exit code. A startup refusal rejects with details.exitCode (daemonStartupExitCode).
• Shutdown: close() is idempotent: concurrent calls share one promise that resolves to a ShutdownReport (provider releases, runners, sessions, plugins). closed settles whoever closes (idle reap). Provider runtimes dispose exactly once; a startup that fails rolls back what it acquired. daemon stop --json still reports providerReleases.pending.
• Per-runtime state: provider admission, interactor resolution, artifact/upload tracking, resumable uploads, materialized paths, session-open locks and request cancellation, plus the Apple runner's sessions, locks, recycle ledger and owner state, live in the runtime. Accepted sockets and HTTP handlers keep the runtime explicitly (AsyncResource.bind). The PNG worker is held by every runtime that uses it and stops with the last.
• Not moved: the Apple runner host binding is a stateless function table (rebinding the same reference is a no-op).

Validation

pnpm check:affected --run --head 66f0b27ea4 passed (exact commit). New tests: two runtimes in one process serve, close and keep state independently; idempotent close() and rollback; runner state per runtime; PNG worker refcount.
Live on iPhone 17e simulator (clean daemon, /private/tmp state dir): open, snapshot, close; graceful daemon stop handed the runner off (ios_runner_session_detached, runner kept alive) and a new daemon adopted it with no second xcodebuild; daemon stop --clean and device status left no claims or runners.

Live Android evidence (daemon route, source head 0658db9)

Headless emulator emulator-5554 (ReactNative API 35, API 35), isolated state dir under /private/tmp, no device claim held beforehand (~/.agent-device/device-claims empty), helper built with pnpm build:android. Commands were run through the built CLI against a freshly started daemon.

open com.android.settings --platform android --serial emulator-5554 --session live3772 --state-dir <dir>   -> success
snapshot -i --json (1st)  androidSnapshot: backend android-helper, helperTransport persistent-session, helperSessionReused false, installReason mismatched (fresh APK), nodeCount 177
snapshot -i --json (2nd)  androidSnapshot: backend android-helper, helperTransport persistent-session, helperSessionReused true,  installReason current,    nodeCount 177
snapshot -i --json (3rd)  helperTransport persistent-session, helperSessionReused true
pidof com.callstack.agentdevice.snapshothelper: 14162 after the 1st, 14162 before and after the 3rd (one process)
logcat since the open: one "Start proc 14162:com.callstack.agentdevice.snapshothelper ... start instr" (plus the reinstall's force-stop)
daemon stop --state-dir <dir> (session still open)  -> "Daemon stopped (graceful)."
adb shell pidof com.callstack.agentdevice.snapshothelper  -> no output, exit 1
device status --platform android --serial emulator-5554    -> "No local device claims found."
ls ~/.agent-device/device-claims                           -> empty

So the warm session is found again by the second and third snapshots through the root slot accessor in the daemon's entered store, and the close-time reset in the same store stops the helper and releases the claim. The daemon log at its default level records none of this, so the process id, logcat and the helperSessionReused field are the evidence; the three source-level changes since this run only move imports (RuntimeStateHandle's home) and approval wiring in the closure gate.

🤖 Generated with Claude Code

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.38 MB	5.39 MB	+7.9 kB
Package (unpacked)	5.38 MB	5.39 MB	+7.9 kB
Package (download)	1.62 MB	1.62 MB	+2.6 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	41.7 ms	43.4 ms	+1.7 ms
CLI --help	80.4 ms	84.3 ms	+3.9 ms

[cubic-dev-ai]

All reported issues were addressed across 52 files

_{You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 9 files (changes from recent commits).

_{You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[thymikee]

Closing one runtime can still kill another runtime's Apple runner build, so this needs one more change before merge (reviewed at c047776). CI is green (12 checks, none failing), and I know of no other merge issue.

The Apple runner state in runner-artifact.ts is still process-wide. deviceStartStates (line 122) and runnerPrepProcessLedger (line 363) are module-level. When runtime A closes, stopAllIosRunnerSessions retires start admissions for every device in the process, then signals every prep child with no device filter (runner-disposal.ts:95). So A's close sends SIGINT, SIGTERM and SIGKILL to runtime B's in-flight xcodebuild build-for-testing, and B's open then fails with a teardown refusal. #3752 needs two runtimes in one process that close independently. The rule: every runner registry that a per-runtime sweep (close, abort-all, retire-all) enumerates must itself be per-runtime. Please move these two behind runnerRuntimeSlot, as runnerSessions already is, and audit the rest of runner/ with the same rule (path-keyed build locks and pure caches can stay). Please add a runner-session test that starts a prep in runtime B, stops all in runtime A, and asserts B's child survives. The isolation test cannot catch this today because its fixture has no Apple runner.

I found this by reading the code. I did not run two runtimes with an Apple runner. The PR body shows no artifacts for the live simulator run, and that run covers one runtime only, so per-runtime Apple isolation rests on runner-owner-state.test.ts. I also did not check whether anything in daemon composition reads Apple runner state before the late enterWith. A read there would land in the standalone store. The signal-to-exit-code glue in src/daemon.ts (75/78 on a startup refusal, 1 on a fatal error or failed close) has no direct test, and I did not run it.

Not blocking, and fine to take or leave: daemon-runtime.ts grows from 993 to 1099 lines, and AGENTS.md says files past 1,000 lines should be split before adding behavior, for example by moving RuntimeContext, createDaemonRuntimeStates and the ShutdownReport assembly beside server-shutdown.ts. Also, the same AsyncLocalStorage boilerplate (store, standalone fallback, getStore() ?? standalone, {run, enter, dispose}) appears in seven host-kit and daemon modules, Apple has a second mechanism in runnerRuntimeSlot, and best-effort rmSync is copied inline in resumable-upload.ts and materialized-path-registry.ts.

Could runnerRuntimeSlot become the single runtime-state owner in host-kit instead? One AsyncLocalStorage would hold a per-runtime store. runtimeSlot(key, create, dispose?) would return that runtime's slot and register its disposer, and createRuntimeStore() would return { run, dispose }. startDaemonRuntime would enter it once before composeDeps. That would delete RuntimeStateHandle, the six per-module storages, RuntimeContext.extend/enterWith and the DaemonRuntimeDeps.runtimeStates field. Each stateful module would shrink to one runtimeSlot call, and the Apple prep and start-admission registries would join through the same call. It needs two decisions first. The standalone fallback outside a runtime would live in that one module, and RuntimeStateHandle would move out of request-cancel.ts.

On the open review threads, all are addressed at this head except none still apply as live issues. These can be resolved: PNG/plugins/lock release in closeOnHost, isolation test cleanup, close test assertions, materialized-path dispose, resumable-upload dispose, shutdown error in daemon.ts, artifact-tracking dispose, releasePngWorker, composition test resolver, infoPath and lockPath assertions, removeBestEffort, all fixed at this head. The dispose ordering thread is benign: dispose runs after the servers close and in-flight work settles, and only a force-cut handler could still touch the temp dir, with its connection already gone.

Before merge, Apple start admissions and the prep-process ledger must be per runtime, with a regression test showing B's prep child survives A's stop-all.

[thymikee]

Pushed 817930fcf4 (check:affected passed on it) to answer the 02:43Z review.

• Apple runner state is now per runtime. deviceStartStates, runnerPrepProcessLedger and the bad-artifact marks live in runnerRuntimeSlot in core/runner-owner-state.ts, so closing runtime A's runners no longer retires B's start admissions or signals B's prep children.
• New test. runner/__tests__/runner-session-runtime-isolation.test.ts runs two runtimes, stops all of A's runners, and asserts B's prep and admissions survive.
• Size. A refactor(move) commit keeps daemon-runtime.ts under the size limit.
• Base. The branch merges refactor/host-environment (which now resolves every client read from the client's host), so no force push was needed.

Not done here: consolidating the per-module runtime-state owners into one is a follow-up. src/daemon.ts's exit-code glue is still untested.

[thymikee]

The PR is ready at 817930f. Not blocking, and you can take or leave these: the four test files daemon-runtime-device-claims, -recording-teardown, -snapshot-shutdown and -web-close-teardown still carry the empty import {} from './daemon-runtime.ts'; that the move commit left, and the close listener in https://github.com/callstack/agent-device/blob/817930f/packages/platform-apple/src/runner/runner-artifact.ts#L392 reads runnerPrepProcessLedger() when the event fires, so capturing the ledger once in registerRunnerPrepProcess and using it in both the add and the close listener would remove the dependence on the emitter's async context.

This follows the earlier review at c047776 (#3772 (comment)). That review's findings are fixed. All 12 checks pass at 817930f.

One thing I did not run: two daemon runtimes with a real xcodebuild prep in one process. Isolation rests on runner-session-runtime-isolation.test.ts, whose fake child never emits 'close'. The live simulator run in the PR body predates this delta, so the ALS lookups for prep and admission in a single live runtime are not live-validated. They do match the runnerSessions slot pattern that run covered. I also have not confirmed whether daemon composition reads Apple runner state before runtimeContext.extend at packages/daemon/src/server/http/daemon-runtime.ts#L368, which would land in the standalone store. I did not run the tests or any gate.

The PR has merge conflicts with main. Before merge, the base branch refactor/host-environment has to land, and the PR has to be rebased to clear the conflicts.

On the open inline threads, no P1 or P2 thread still applies, and no lower-priority threads still apply. These threads no longer apply, so please resolve them: #3772 (comment), #3772 (comment), #3772 (comment), #3772 (comment), #3772 (comment), #3772 (comment), #3772 (comment), #3772 (comment), #3772 (comment), #3772 (comment) and #3772 (comment) are fixed, since the patches they cover are unchanged in this update. #3772 (comment) is harmless, because dispose runs after the servers close and in-flight work settles, so only a force-cut handler whose connection is already gone could touch the temp dir.

The branch has a merge conflict with refactor/host-environment, so it needs a rebase before it can merge.

[thymikee]

This PR is ready at 012b50d, and the earlier findings from c047776 are now fixed. Not blocking: import {} from './daemon-runtime.ts'; is left in four tests (device-claims, recording-teardown, snapshot-shutdown, web-close-teardown) after the move, and it binds nothing, so delete those four empty imports or leave them, as you prefer (first one: https://github.com/callstack/agent-device/blob/012b50d/packages/daemon/src/server/http/daemon-runtime-device-claims.test.ts#L1).

Smoke Tests is still running at 012b50d and no job has failed. The diff touches the smoke route, in daemon startup (the policy-load merge resolution) and in close (the moved teardownDaemonSessionForShutdown and RuntimeContext), so any failure there needs a look before you clear it. No conflicts. Smoke Tests must finish green, and refactor/host-environment must land before this stacked PR can merge.

Some limits on what the review covered. I read the code and did not run tests or gates. I did not run two daemon runtimes with a real xcodebuild prep in one process. The fake child in the isolation test never emits 'close'. The 'close' listener at runner-artifact.ts:392 looks up the ledger when the event fires, so it relies on the child-process handle carrying the spawn's async context. That holds in Node, but nobody has exercised it here. The simulator run in the PR body predates 22cea00, so no live cold start (prep build) has run at this head. The same runnerRuntimeSlot pattern, used for runnerSessions, was covered by that run. The signal-to-exit-code glue in src/daemon.ts has no direct test, as you note. packages/daemon/src still reads process.env and process.cwd() (app-events.ts:103-107, session-script-*, record-runtime.ts:230). That is outside the exit/on/once scope of #3752, belongs to #3763 HostEnvironment, and is not in this delta.

On the open review threads, I could not read who wrote them. Ten fixed ones can be resolved because the code at this head already does what they asked: #3772 (comment) (closeOnHost finally still releases the PNG worker, plugins, registration and lock config), #3772 (comment) (isolation test unchanged since c047776), #3772 (comment) (close test unchanged), #3772 (comment) (materialized-path-registry dispose still in createRuntimeContext), #3772 (comment) (resumable-upload unchanged), #3772 (comment) (src/daemon.ts:33-35 still reports the shutdown error and sets exit code 1), #3772 (comment) (artifact-tracking unchanged), #3772 (comment) (releasePngWorker still idempotent and called in the finally), #3772 (comment) (composition test unchanged), #3772 (comment) (infoPath/lockPath assertions unchanged). Two more can be resolved too: #3772 (comment) (benign, since runtimeContext.dispose() still runs last in the finally at daemon-runtime.ts:894, after servers close and in-flight work settles) and #3772 (comment) (shared removal helper unchanged since c047776).

[thymikee]

Pushed 7eedaf61b7 (check:affected passed on it).

• Merges the final refactor/host-environment (ab5ee6e3fe: spawn cwd fix, main merge), which clears the conflict notice.
• Deletes the four empty import {} from './daemon-runtime.ts' lines from the device-claims, recording-teardown, snapshot-shutdown and web-close-teardown tests.
• Moves the Limrun public-HTTP test that main added onto the runtime shutdown contract (close(), no exit option, array-returning detach and finalize).

Nothing else changed since the review at 012b50d.

[thymikee]

The code at 7eedaf6 looks right, and the earlier review of 012b50d still holds. CI is green on all 6 checks at that commit, and the new changes touch only tests and an upstream base merge. The PR has merge conflicts with main. The stack base refactor/host-environment has also moved to 7eb7379, and merging it conflicts in packages/daemon/src/server/http/daemon-runtime.ts, so please merge it, resolve that file, and re-run check:affected before merge.

Not blocking, and you can take or leave it: the base now carries daemon-runtime-quiesce.test.ts from #3764, and it still passes exit and registerProcessHandlers: false (https://github.com/callstack/agent-device/blob/7eedaf6/packages/daemon/src/server/http/daemon-runtime-quiesce.test.ts#L116), calls runtime.shutdown(), and waits on an exit promise, which this PR removes. When you resolve the conflict, route the quiesce path's void shutdown() to close(), drop those two options, call runtime.close(), and await runtime.closed.

Is the per-runtime state handle (RuntimeStateHandle) in the right place? Its type sits in host-kit's request-cancel module, and createRuntimeContext sits in server-shutdown.ts. The same store, fallback and handle code repeats in seven modules, and kernel/scoped-provider.ts already owns the store-or-local-fallback part. Could one factory such as createRuntimeState(create, dispose), returning { current(), handle }, live in kernel or in its own host-kit entry, so each module declares only its state shape and dispose step? createRuntimeContext could then move to a daemon module named for the runtime context.

The evidence is not complete yet. I did not run the Limrun test locally, so that it passes at 7eedaf6 rests on the green CI and on your check:affected run. I also did not inspect the conflicting hunks of daemon-runtime.ts against 7eb7379, and I assumed no review threads were resolved since the last review, so any open inline thread that still applies still stands.

[thymikee]

Pushed b29df19162 (check:affected passed on it).

• Bounded close (Codex review P1). close() cancels every in-flight request and drains them against a 5s budget (shutdownBudgets.requestDrainMs). A request that ignores cancellation is reported as ShutdownReport.requests.abandoned, the registration and lock stay held while the process lives, and the listeners are force-closed at the same budget. Plugin shutdown is bounded the same way, and a plugin still shutting down is reported in plugins.pending. daemon-runtime-bounded-close.test.ts covers a hung request and a hung plugin.
• Rollback (Codex review P1). Not changed here. The composeDeps failure leak is closed by feat(plugins): plugin contract v2 and one provider registry #3767's transactional composition with registry.close() as the single cleanup owner. After it lands I will consume its per-plugin outcomes for ShutdownReport.plugins and delete the runtime-shutdown loop in server-shutdown.ts; until then this PR adds no second disposer.
• Merge with refactor/host-environment. Includes main's quiesce stack (feat(daemon): quiesce request that retires an idle daemon atomically #3764): the quiesce drain calls close(), the shutting-down response and startingUp flag are kept, and main's new quiesce test and the Limrun and provider-scenario harness tests moved onto close()/closed.
• Explicit host. The packages/daemon readers take the request host explicitly (from feat(host): capture HostEnvironment once at entry points #3763).

[thymikee]

f8251da now conflicts with main after #3786 and #3788 landed. Please rebase. The code verdict from the review above is unchanged: the duplicated slot pattern still needs an answer before this is ready.

[thymikee]

Pushed d5682c76fa (check:affected passed on it): #3777 (platform and capture packages on the host) merged into refactor/host-environment, so this branch is merged with it and with current main (#3786, #3788). Two import-list conflicts in the Apple runner kept both hostEnvironment and the per-runtime runnerRuntimeSlot. Two tests followed the merge: the capture-kit unavailable-worker test now uses retainPngWorker (this PR's replacement for prewarmPngWorker), and the iOS e2e live runner passes its host to sendRequest. The platform-apple, platform-android, capture-kit and daemon suites, the entrypoint and composition tests, the layering guard and the eager-closure budgets pass; fallow flags nothing introduced. No review thread is open.

[thymikee]

Pushed 5a8f1cd22c (check:affected passed on it): current with main and refactor/host-environment at fab82cc123 (#3775 and #3777 merged). It answers the f8251da review's two blockers.

• One slot store, no Android copy. The Android package no longer has its own async-local store. The root keeps the one store (defineRuntimeSlots in @agent-device/host-kit/runtime-slots) and binds its slot accessor into the adb host port (AndroidAdbHost.runtimeSlot); Android declares only its slot keys. Daemon composition registers the same store's handle from a small root module (platform-runtime-android-helper-state.ts) without loading the Android mechanics. The seventh Android subpath and its layering exception are gone.
• Why Apple, host-kit's request-cancel and the daemon copies did not move onto it. I tried: with the shared module imported by Apple's runner-owner state and by request-cancel, the eager-closure no-growth budgets failed on about a dozen entries (for example daemon/index.ts 372 to 373 modules, host-kit/request.ts 5 to 6, the Apple lifecycle and doctor facades each +1), because a new module is evaluated eagerly where each of those had its state inline. I reverted those and kept the state where it already lives. Folding them in needs either a budget change the maintainer approves, or moving the Apple state behind its host port the way Android now is; I left that as a decision rather than widening this PR.
• Composition test. daemon-composition.test.ts now asserts a composition registers two state handles (Apple runner, Android helper) and that the Android handle scopes the root's slot accessor, so deleting the registration fails a test.
• Not done: the live Android emulator run. I have no emulator available. The route is covered by the stub-host isolation tests (a close of one runtime leaves the other's warm session and pending retirement) and the composition test, not by a live daemon run; that check remains to be done by someone with an emulator.

[thymikee]

Thanks for the update. At 5a8f1cd the daemon exit-code fix still holds, but two problems remain, and one needs live evidence before merge.

Android snapshot-helper warm sessions, failed-start backoff, pending retirements and shell-protocol probes are now found through a root slot store bound into AndroidAdbHost.runtimeSlot (snapshot-helper-session-lifecycle.ts#L99). They are not entered per request. The only coverage is the stub-host isolation test, which uses a separate defineRuntimeSlots instance, and a composition test that calls androidHelperRuntimeSlot directly. No run shows that the real daemon route reads and resets helper state in the same entered store. If the async context is lost on that route, reads fall back to the standalone store. Then the warm session is not reused, or the close-time reset misses it, and the helper stays claimed on the device. No test would show this. Please run an Android emulator through the daemon at this head and take two snapshots in one session. The second must show metadata.sessionReused true, and the daemon log must show one helper spawn. Then run daemon stop. After it, adb shell pidof com.callstack.agentdevice.snapshothelper must print nothing, and device status must show no claim. Please attach that output to the PR body.

The new defineRuntimeSlots is the owner of the pattern, but only Android uses it. Apple's runnerRuntimeStorage, standalone fallback, createAppleRunnerRuntimeState and runnerRuntimeSlot (runner-owner-state.ts#L34) are still a near-copy of runtime-slots.ts lines 27-47. request-cancel.ts lines 8-32 is the same copy next to the owner. The earlier review asked Apple and Android to declare only keys, and this update moved one of the eight copies. A fix to the fallback, dispose or enterWith timing still needs edits in seven hand-written copies plus the owner. Apple's dispose also differs from the owner's: it clears slots and runs no disposers. The rule should be that every module that keeps per-runtime state gets it from defineRuntimeSlots, and no other module builds an AsyncLocalStorage with a standalone fallback for runtime state. A grep for getStore() ?? outside runtime-slots.ts should then return nothing. As you note, this needs a maintainer decision to accept a +1-module eager-closure budget for runtime-slots.ts. After that, please move runner-owner-state.ts and request-cancel.ts onto it, and fold in the five daemon modules or file them as an explicit follow-up. I did not run the eager-closure budget gate, so the budget numbers are unverified on my side.

Would one defineRuntimeSlots() instance as the store be simpler? Host-kit would export runtimeSlot(key, create, dispose?) and createRuntimeStore(). Every stateful module (the five daemon modules, request-cancel, Apple runner-owner-state and the Android adb port accessor) would call runtimeSlot with its own key, and startDaemonRuntime would enter the store once before composeDeps. That would delete the per-module handles, the DaemonRuntimeDeps.runtimeStates list and RuntimeContext.extend. A forgotten registration would then be impossible instead of a silent fallback, and the production net should drop below the 700-line threshold. The maintainer budget decision above would have to come first.

Not blocking: AndroidAdbHost.runtimeSlot (adb-host.ts#L49) declares a dispose parameter that helperRuntimeSlot (line 125) never passes, and the doc says a close "tears down" the helper while the handle's dispose only forgets the slots. The composition test also checks androidHelperRuntimeSlot directly instead of the accessor bound into the port. Either drop the parameter or use it, and assert through helperRuntimeSlot after importing the binding module. Take or leave these.

Eight checks were still running when I reviewed: the four Smoke Tests, CodeQL Analyze, Repo Guards & Compatibility, Coverage and Integration Tests. None has failed yet. The diff overlaps their routes (daemon runtime close and composition, the Android helper slots, and the layering policy and host-kit exports), so a failure there needs a look. I did not inspect the merge commit d5682c7 hunk by hunk beyond the slot files, and the range-diff shows commits 1-19 unchanged.

Before merge, the Android emulator run above must pass, and a maintainer must decide on the +1-module budget so Apple and request-cancel can move onto defineRuntimeSlots.

[thymikee]

Decision on the budget: I accept the +1-module eager-closure budget for runtime-slots.ts. Please move runner-owner-state.ts (Apple) and request-cancel.ts onto defineRuntimeSlots, so that each of them declares only its keys and no other module builds its own AsyncLocalStorage with a standalone fallback. The Android emulator run from the review above is still required.

[thymikee]

Pushed 3ef9ace0ff (check:affected passed on it) for the maintainer's decision on the budget.

• Apple and request-cancel are on defineRuntimeSlots. runner-owner-state.ts and host-kit's request-cancel.ts no longer build their own AsyncLocalStorage with a standalone fallback: each keeps its state in slots (runnerRuntimeSlot, the request-cancel slot with its disposer aborting in-flight controllers) and exposes createHandle() from the shared store. Apple's lease-owner state dir and claim probe are now a slot too, and the Apple handle's dispose now runs slot disposers like the owner's.
• The +1-module addition is recorded in the gate, not waived. The closure gate had no way to carry a maintainer approval, so I added APPROVED_CLOSURE_ADDITIONS to eager-closure-budgets.ts with one row: packages/host-kit/src/runtime-slots.ts, refactor(daemon): daemon runtime owns no process globals #3752, owner thymikee, with the reason. It excuses only that module from an existing entry's no-growth count; any other added module is judged as before. closure-growth-rule.test.ts covers an approved module passing, a second unapproved one still failing, and the same growth failing without approval; another test fails a row whose module is gone.
• Take-or-leave notes. AndroidAdbHost.runtimeSlot no longer declares a disposer parameter nobody passed, and its doc now says the close-time reset stops the helper while the slots only forget state. I did not assert through helperRuntimeSlot in the composition test: layering R13 lets only the adb binding file import that facet, so the test checks the root accessor directly.
• Still not done. The five daemon modules (provider-device-admission, interactor-resolution, artifact-tracking, resumable-upload, materialized-path-registry) keep their own copies; I will fold them onto the store next if you want it in this PR. The live Android emulator run is also still outstanding, since I have no emulator.

[cubic-dev-ai]

All reported issues were addressed across 8 files (changes from recent commits).

_{You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[thymikee]

Pushed 0658db9b9e (check:affected passed on it).

• The five daemon modules are on the shared store too. provider-device-admission, interactor-resolution, artifact-tracking, resumable-upload and materialized-path-registry keep their state in defineRuntimeSlots slots; the expiry-timer and temp-file cleanup each module did on dispose now runs as the slot's disposer. With Apple (runner-owner-state), host-kit's request-cancel and the Android root accessor already on it, no per-runtime state module builds its own AsyncLocalStorage with a standalone fallback any more. The remaining AsyncLocalStorage uses are request or execution scopes (diagnostics, command host, request progress, provider scope), not per-runtime state.
• The approved-addition mechanism got the two cubic fixes above.
• Still not done: the live Android emulator run, since I have no emulator; and the per-module createXRuntimeState handles and DaemonRuntimeDeps.runtimeStates list stay, so a forgotten registration is still possible, though the composition test now pins the registered handles. Collapsing them to one store the runtime enters once would be the follow-up.

[thymikee]

Thanks for the update. The earlier review of 5a8f1cd is mostly addressed at 0658db9: the two eager-closure threads from the other reviewer are fixed, and CI shows 20/20 checks passing. One item still needs a real run before merge. I did not run tests or the eager-closure gate myself, and I took the budget numbers and the check:affected pass from the CI size report and your notes.

Android helper state (warm sessions, failed-start backoff, pending retirements, shell-protocol probes) is now reached through the root slot accessor bound into AndroidAdbHost.runtimeSlot in snapshot-helper-session-lifecycle.ts. If the async context is lost on the real daemon route, reads fall back to the standalone store. The second snapshot then starts a new helper, or the close-time reset misses the warm session. Today the tests use the stub host's own defineRuntimeSlots instance or call androidHelperRuntimeSlot directly, and the PR body has no Android run output, so no test would catch this. Please run an Android emulator through the daemon at this head and take two snapshots in one session. The second must show metadata.sessionReused true, and the daemon log must show one helper spawn. Then run daemon stop: adb shell pidof com.callstack.agentdevice.snapshothelper must print nothing, and device status must show no claim. Please attach that output to the PR body. A leaked helper stays claimed on the device after stop, and warm-session reuse could regress silently.

Not blocking, take or leave: daemon-runtime.ts:75 and server-shutdown.ts:10 still import RuntimeStateHandle through the host-kit/request re-export while seven other files moved to @agent-device/host-kit/runtime-slots, so one import path and no re-export would be cleaner; and the APPROVED_CLOSURE_ADDITIONS row at eager-closure-budgets.ts:148 does nothing once runtime-slots.ts is in the merge-base closure, yet the validity test only checks that the file exists, so the stale row would stay.

The two eager-closure threads from the other reviewer are fixed at this head, so please resolve them: closure-growth-rule rename mapping and budget row reason check.

Before merge, the Android emulator output above must be in the PR body.

[thymikee]

Pushed 70c901d671 (check:affected passed on the exact commit; it includes the merge of refactor/host-environment at 4f6dd3e722).

• Live Android evidence is in the PR body: second and third snapshots report helperSessionReused: true with one helper process (pid unchanged) and one Start proc in logcat. After a graceful daemon stop with the session open, pidof com.callstack.agentdevice.snapshothelper prints nothing and device status shows no claim. It was taken at 0658db9b9e; the later commits only move imports and the closure-gate approval wiring.
• RuntimeStateHandle is now imported from host-kit/runtime-slots in daemon-runtime.ts and server-shutdown.ts. The unused re-export in host-kit/src/request.ts is gone.
• Remaining follow-ups, not in this PR: narrowing commandHostEnvironment() (about 60 call sites in root platform-runtime files and platform packages), and collapsing the per-module handles and runtimeStates into one entered store.

[thymikee]

The code at 70c901d looks good, and the live Android run in the PR body meets my earlier evidence request. The run was taken at 0658db9; the only change since then is type-only imports, so runtime behavior is the same. The earlier Cubic findings are fixed at this head. All 20 checks pass, and there are no conflicts. I ran no tests myself.

One question on simplicity before this is ready for review. Apple keeps its runtime store in its own package: runner-owner-state.ts calls defineRuntimeSlots() and exports createAppleRunnerRuntimeState and runnerRuntimeSlot. Android keeps its store in the root (src/platform-runtime-android-helper-state.ts) and reaches it through a new port facet, AndroidAdbHost.runtimeSlot, plus helperRuntimeSlot in adb-host.ts. The earlier reason was the eager-closure budget, but the +1-module budget is now accepted and Apple calls defineRuntimeSlots directly. adb-host.ts imports only types and node:async_hooks. Could adb-host.ts call defineRuntimeSlots() and export createAndroidHelperRuntimeState and helperRuntimeSlot itself, as Apple does? That removes the port facet, the root helper-state module and the stub store in adb-host.fixtures.ts, and the Android isolation test would then use the same store as production. If something blocks that shape, please say what.

This PR is based on refactor/host-environment (#3763), so it also waits for that one.

[thymikee]

Pushed 029e82d666 (check:affected passed on the exact commit; includes #3763 at 595fa4822c).

• adb-host.ts now calls defineRuntimeSlots() itself and exports createAndroidHelperRuntimeState and helperRuntimeSlot, as runner-owner-state.ts does. Deleted: the AndroidAdbHost.runtimeSlot port facet, the root src/platform-runtime-android-helper-state.ts module (and its zone entry), and the stub store in adb-host.fixtures.ts. The isolation test uses the production store.
• R13 allowed platform-android/adb-host only for the root binding file, so src/daemon-composition.ts and its test are now in that importer set (policy and policy test updated).
• The merge with feat(host): capture HostEnvironment once at entry points #3763 conflicted on the PNG worker: I kept feat(daemon): the runtime owns its state and never the process #3772's retainPngWorker and put the unavailable-worker report and the daemon diagnostics scope on it (withDaemonDiagnosticsScope({ logPath }, retainPngWorker)).
• The Android emulator evidence in the body was taken before this refactor and the behavior it shows is unchanged (same slot store, now owned by the adb host module); I did not re-run it.

[thymikee]

The code at 029e82d is clean. The earlier concern from 70c901d is fixed: the runtime now owns its state, and the merge keeps the PNG worker leak fix. All 20 checks pass at 029e82d, and the author reports check:affected passed on the same commit. I ran no tests. I judged the isolation and composition tests and the layering policy from the diff. The live Android emulator run is from 0658db9 and was not re-run at 029e82d. It still counts, because the later change only moves where the slot store is defined and touches no adb or device call. The PNG worker report behavior comes from #3763, and I checked only the merge resolution here, not that PR's own logic.

Not blocking: bindAndroidAdbHostStub() and its import in adb-shell-protocol.test.ts are now unused, because helperRuntimeSlot no longer needs a bound host. You can delete both or leave them.

The cubic-dev-ai P2 thread on the PNG worker leak no longer applies, because retainPngWorker is held at daemon-runtime.ts:829 and releasePngWorker() runs in the finally at daemon-runtime.ts:967. You can resolve it: #3772 (comment)

This PR is stacked on #3763 (refactor/host-environment). #3763 must land first, and then this PR needs a retarget onto main.

[thymikee]

Pushed c58e2b05d0 (check:affected passed on the exact commit; includes #3763 at d1f3001848).

• Removed the unused bindAndroidAdbHostStub() and its import from adb-shell-protocol.test.ts.
• The cubic leak thread (r4300418786) was already resolved against c3baaec.
• Merge with feat(host): capture HostEnvironment once at entry points #3763: retainPngWorker now takes the unavailable-worker sink, registers it per holder, drops it on release, and tells a holder that registers after the worker already failed. The daemon passes the same emitDaemonDiagnostic sink.

[thymikee]

Smoke Tests failed at c58e2b05d0 on the Swift runner XCTest testPlanMovesPastATreeTierWhoseCollapsedTabQueryCannotStart (RunnerTests+SnapshotCapturePlanOccupancyTests.swift:529: got capture-failed, expected budget; run 39106555059, job 117613540676). This is a processing-slice timing assertion inside the runner's own XCTest, the same class as the earlier masked-field flake. Nothing in this PR reaches it: the diff against main contains no .swift, .pbxproj or .xctestplan file, and no runner build script or workflow, and the test runs the compiled Swift sources without the daemon, the host environment or the Node runtime. I re-ran only that job.