feat(host): capture HostEnvironment once at entry points

[thymikee]

Summary

Part of #3747. Closes #3749.

This branch is the whole HostEnvironment adoption: #3775 (root src/ and the remaining packages) and #3777 (platform and capture packages) are merged into it, on top of this PR's own foundation.

HostEnvironment = { env, cwd } now has one owner. Entry points capture it once, and the daemon runtime, request scope and command spawning receive it instead of reading process. About 260 files change, most of them test call sites.

• Type and capture: HostEnvironment is in @agent-device/kernel/source-value. captureHostEnvironment(process) is in @agent-device/host-kit/process. It copies the environment.
• Entry points: src/bin.ts, src/cli.ts (runCli(argv, deps, host)), src/daemon.ts and createAgentDeviceClient(config, { host }) capture once. A client built without a host captures process and then reads only that host: its default transport, daemon launch, state dir and stateDir() all resolve from it.
• Daemon: startDaemonRuntime requires host. DaemonRuntimeDepsInput.host replaces env. RequestRouterDeps.host and RequestExecutionScope.host carry it. Request handlers, claim writers, lease admission and the reapers in packages/daemon read configuration from that host explicitly (through RequestHandlerChainParams.host or the env their constructor receives), not from the ambient scope.
• Command spawning: withCommandHost({ host, requestEnv }, fn) replaces withRequestCommandEnv. Spawned commands start from host.env. commandHostEnvironment() still gives platform and root code that host and throws outside one; it is meant for command execution only, and their owners (refactor(platforms): read host settings from HostEnvironment in platform and capture packages #3750, refactor: read host settings from HostEnvironment in root src and remaining packages #3751) convert the remaining readers to explicit hosts.
• Explicit inputs: whichCmd, resolveUserPath, expandSessionPath, the device-claim functions, loadDaemonPolicy, launchDaemonProcess and kernel allowlist and simulator-set resolvers take env or cwd as parameters.
• Done when: host-kit, kernel and daemon-contracts non-test sources have no process.env or process.cwd().

Behavior changes: a relative state dir without a cwd is refused (relative_state_dir_without_cwd; callers now pass one), and a relative AGENT_DEVICE_CLAIMS_DIR is refused (relative_claims_dir; the store is shared by every daemon on the host, so it used to depend on where one of them started). Suites that reach host-reading code outside a daemon request or the CLI bind a host with bindTestCommandHost() from test-kit; there is no global test setup.

The root src/ readers (#3775) and the platform and capture packages (#3777) now read host settings from HostEnvironment, merged into this branch. commandHostEnvironment() remains the ambient host for command execution and for a number of platform-side readers that still call it; narrowing it further is a follow-up.

Validation

pnpm check:affected --run --head fab82cc123 (exactCommit) passed, with current main, #3775 and #3777 merged.

Behavior note (PNG worker): packages/capture-kit/src/png-worker-client.ts no longer calls process.emitWarning when the worker is unavailable and starts with env {}. The daemon registers a sink when it pre-warms the worker; every transition to unavailable (spawn failure, missing entry, load error, exit) reaches daemon.log exactly once as png_worker_unavailable with a typed reason and its detail, with or without an active scope. Requests then use the in-process fallback without repeating the event (also in CHANGELOG).

🤖 Generated with Claude Code

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.38 MB	5.38 MB	+5.5 kB
Package (unpacked)	5.38 MB	5.38 MB	+5.5 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	28.2 ms	43.3 ms	+15.1 ms
CLI --help	78.4 ms	83.1 ms	+4.7 ms

[cubic-dev-ai]

All reported issues were addressed across 178 files

_{You’re at about 91% 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 39 files (changes from recent commits).

_{You’re at about 93% 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]

The code needs no changes before merge, but the kernel-errors mutation checks are red and must pass first. I reviewed commit 5effcd7.

All five kernel-errors shards fail the Stryker dry run on one test, host-kit/src/code-signature.test.ts:149 ("the daemon source graph stamps the walker that produced it"). That test is unchanged and passes in the unit lanes. The shards passed in run 38965344815. The related-test set for these shards grew from 1372 to 1607 files, which pulled this test in. The new global setup file (https://github.com/callstack/agent-device/blob/5effcd7/src/__tests__/host-environment-setup.ts#L8) imports host-kit/command and so exec.ts and kernel/errors, so I think it caused that growth. I did not reproduce the failure in the sandbox. The walker probably resolves workspace symlinks outside the sandbox root, but I have not proven that. The "Affected decision-kernel mutants" check fails after the dry run ("13 report(s), expected 18"), and Smoke Tests was still running when I looked. No conflicts.

The next step is to remove the global aroundEach setup, or to make that test work in the sandbox. Re-run the kernel-errors shards after that.

Not blocking, and you can take or leave these. The setup file binds every unit test to the live process.env and process.cwd(), so no test can see host_environment_unscoped (exec.ts:140). Please bind createTestHostEnvironment only in suites that enter through the daemon or CLI, and add one test that calls a commandHostEnvironment() consumer outside a scope and expects that reason. Also, createAgentDeviceClient(config, { host }) at https://github.com/callstack/agent-device/blob/5effcd7/src/agent-device-client.ts#L89 uses host only in stateDir(), while the daemon client still reads process.env and process.cwd() (daemon-client-lifecycle.ts:97-140, 818). Either every resolution the client makes should read the same host, or deps.host can wait for #3751. Finally, RequestExecutionScope.host (https://github.com/callstack/agent-device/blob/5effcd7/packages/daemon/src/server/request-execution-scope.ts#L263) has no production reader except its own test, because daemon handlers use the ambient commandHostEnvironment() (19 call sites).

Would one channel per layer be simpler? host-kit would keep the ALS scope only for spawn env and the DEVELOPER_DIR overlay. Daemon handlers would read host from RequestExecutionScope or the runtime deps they already get. Platform code would take host from its existing host or tool-provider object, as AndroidAdbHost.environment does. That would remove the throwing ambient accessor and the test-wide shim, and a missing host would become a type error. #3750, #3751 and #3752 will copy whatever shape this PR sets. Could we settle in #3747 or an ADR whether HostEnvironment travels explicitly or ambiently before they land?

The open inline threads still apply. The two P2 threads on connection.ts (#3763 (comment) on the home path, #3763 (comment) on provider profile) and the P2 thread on bundletool lookup (#3763 (comment)) are still open. One lower-priority thread also applies: #3763 (comment).

[cubic-dev-ai]

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

_{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>

[thymikee]

Pushed d522d257de: the global test setup is gone, so the related-test set no longer grows. Suites that need a host bind one with bindTestCommandHost() (test-kit); a new test pins host_environment_unscoped outside a host. The kernel-errors 3/5 mutation shard passes locally with 1371 related tests (main: 1369). Waiting on CI.

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://callstack.github.io/agent-device/pr-preview/pr-3763/
Built to branch gh-pages at 2026-10-18 07:01 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[cubic-dev-ai]

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

_{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>

[thymikee]

Thanks for the PR. The host capture is a good direction, but one defect in the client needs a fix before merge. CI is green at d522d25 (23 checks, none failing), and there are no conflicts.

createAgentDeviceClient now accepts deps.host (or captures one at creation, line 89), and client.stateDir() resolves from host.env.AGENT_DEVICE_STATE_DIR and host.cwd (src/agent-device-client.ts#L215). Every command the same client sends goes through sendToDaemon, which still resolves the state dir from live process.env and process.cwd() (daemon-client-lifecycle.ts#L97, L107) and launches the daemon with process.env (line 818). Two inputs show the gap: createAgentDeviceClient({}, { host: { env: { AGENT_DEVICE_STATE_DIR: '/x' }, cwd } }), and setting process.env.AGENT_DEVICE_STATE_DIR (or calling chdir) after the client exists. Before this PR both reads were live and agreed. Now a public Node API client can report one state dir while it starts and talks to a daemon in another, so tooling that cleans or inspects client.stateDir() acts on the wrong daemon. The rule: everything one client reads must resolve from the same host. Either thread host through the transport factory into resolveClientSettings and startDaemon (explicit state dir, cwd, launch env), or drop deps.host and keep stateDir() on live process reads until #3751 moves the daemon client.

Not blocking, and you can take or leave these: RequestExecutionScope.host is set but nothing in production reads it, because daemon handlers call the ambient commandHostEnvironment() and the test only checks the field is the object passed in (request-execution-scope.ts:275); the MCP route runs inside the bin host scope but createClient calls createAgentDeviceClient(config) without deps.host, so it captures process again (src/mcp/command-tools.ts:251); ensureHarmonyToolchainPathConfigured writes env.PATH on the shared captured host, and captureHostEnvironment does not freeze its copy, so the Readonly type does not stop the write (packages/platform-harmonyos/src/hdc.ts:72); a relative AGENT_DEVICE_CLAIMS_DIR used to resolve against cwd and now throws relative_claims_dir, which the PR body and CHANGELOG do not mention, and the body also names a setup file that does not exist and says 178 files where the diff has 262 (device-claim-paths.ts:35); createTestHostEnvironment repeats captureHostEnvironment by hand (packages/test-kit/src/host-environment.ts:13); and the "request meta cwd or daemon cwd" fallback now reads inline process.cwd() in some daemon sites (screenshot-runtime.ts:326, record-runtime.ts:230/239, the session-script files, session-replay-transaction.ts:65) but commandHostEnvironment().cwd in others (session-perf-runtime.ts:149, session-open-execution.ts:432), so one source or a tracking line in #3751/#3752 would help.

Could the AsyncLocalStorage scope keep only what it already owns, which is spawn env (resolveSpawnEnv, commandDeveloperDir, exec trace), while handlers get the host explicitly? Daemon handlers would read scope.host or a host on their params, CLI ClientCommandHandlers would get ctx.host in the context they already receive, and platform modules would take env from their existing host ports, as Android does with AndroidAdbHost.environment. That would remove the throwing ambient getter and the 89 bindTestCommandHost suites, and make the dead scope.host field the real channel. Before #3750 and #3751 build on this, please decide in #3747 whether "bound HostEnvironment" means an ambient locator or explicit context fields.

I did not trace every public SDK export (artifacts, android-adb, limrun, ai-sdk) to prove none reaches commandHostEnvironment() outside a scope, where it would throw host_environment_unscoped. Unit tests cannot catch that, because 89 suites bind a live host. I also did not run code to check that the scope reaches HTTP connection callbacks and every daemon timer, though the request router rebinds per request. I did not run the tests, so the claim that cli-client-commands.test.ts fails without the harness binding is unverified. No live device run was asked for or checked.

This is reviewed at d522d25. Before merge, the client needs to resolve every state dir read from one host, and the scope question for #3750 and #3751 needs an answer.

[cubic-dev-ai]

All reported issues were addressed across 11 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]

Pushed 756cf4e012 for the client finding: createAgentDeviceClient's default transport is now createDaemonTransport(host), so state-dir resolution, remote and transport settings, the policy check, the daemon launch environment, the provider credential fingerprint and the developer dir all read the client's host. sendToDaemon keeps the released process-level default through a boundary wrapper; the CLI run builds its transport from the run's host. A new test (daemon-client-host.test.ts) gives the client a host that disagrees with process.env and checks that stateDir() and the settings the transport resolves agree.

Also taken from the non-blocking list: createTestHostEnvironment goes through captureHostEnvironment, and the PR body now names the relative_claims_dir refusal and no longer mentions the deleted setup file or the old file count. Not changed: the MCP route's createClient (its host is the same capture as the bin host), the Harmony PATH write (it deliberately changes the environment the daemon spawns with), and the inline process.cwd() fallbacks, which stay in the modules where an import would grow an eager closure; #3751/#3752 own migrating them.

On the ambient versus explicit question: the ambient commandHostEnvironment() exists because platform modules had no host to read and R13 forbids process there. I would like it to stay narrow, covering spawn env and the exec diagnostics, and have #3750 give platform modules their host through the ports they already own (as AndroidAdbHost.environment does), with handlers reading scope.host. That would let #3750/#3751 delete the accessor and the bindTestCommandHost suites as they go. It is your call, and I have asked in #3747 for the decision before they build on this.

[thymikee]

Thanks for the update. Since the review at d522d25 (#3763 (comment)), the client now resolves the policy against host.cwd, but one defect remains at f4616b9.

startDaemon passes env: settings.host.env but no cwd (daemon-client-lifecycle.ts#L832). launchDaemonProcess (daemon-registration-owner.ts#L262) spawns without options.cwd, so the daemon inherits process.cwd(). It then captures that as host.cwd (daemon.ts#L9) and resolves the policy at daemon-runtime.ts#L420, while the client resolves the same relative AGENT_DEVICE_DAEMON_POLICY against its own host.cwd (daemon-client-lifecycle.ts#L327). With createAgentDeviceClient({}, { host: { env: { AGENT_DEVICE_DAEMON_POLICY: 'policy.json' }, cwd: '/client/ws' } }), or a process.chdir() after the client exists, the two sides read different files. A Node SDK client then starts a daemon that exits with "cannot read policy JSON" or loads another file, and the client rejects it with DAEMON_POLICY_MISMATCH. Before this PR both sides read the live process cwd and agreed. The rule is that any relative value in the launch env must resolve the same way in the client and in the daemon. Please add cwd to the launchDaemonProcess input and pass cwd: settings.host.cwd to runCmdDetachedMonitored, which already supports options.cwd. That covers the policy path and any other cwd-relative setting the daemon reads. Please also add a regression where host.cwd differs from process.cwd() and the policy path is relative.

Not blocking, and fine to take or leave: CliDeps now has both sendToDaemon and an optional transportForHost (cli.ts#L70) plus resolveRunTransport. In production transportForHost is always set, so DEFAULT_CLI_DEPS.sendToDaemon and the process-capturing wrapper in daemon-client.ts have no production caller.

Could CliDeps take the transport only as transportForHost(host), defaulting to createDaemonTransport, with sendToDaemon and resolveRunTransport deleted? That would give a run one transport source, and launching with cwd: host.cwd fits in the same DaemonClientSettings.host owner. Nothing has to change first, since sendToDaemon is not a public export and tests can inject transportForHost: () => mock.

Of the open inline threads, the cubic-dev-ai thread on the policy path (#3763 (comment)) still applies until the spawn cwd is fixed. These no longer apply and you can resolve them: the createTestHostEnvironment() use in the registration test (#3763 (comment)), the runCli tests with a non-process host (#3763 (comment)), and the docstring that now names the #3751 upload exception (#3763 (comment)).

I did not run any test or daemon; the daemon cwd finding is traced from the spawn options and the entry capture, and I did not check whether other daemon reads also resolve relative env values against the inherited cwd. Remote artifact upload still reads process env and cwd, which I accepted as deferred to #3751 without tracing it. At f4616b9 Smoke Tests are queued, and Repo Guards & Compatibility and Coverage are still running with no failure yet. This change touches the daemon-client route that smoke runs use, so a failure there would likely be related. There are no conflicts. Before merge, please launch the daemon with cwd: settings.host.cwd, add the regression, and let the checks finish green.

[thymikee]

Pushed d8a9bd40c0 (check:affected passed on it) for the 13:35Z recommendations, with current main (#3714 included).

• Transport test hardened. cli-run-transport.test.ts asserts the run completes without exiting, fails on any exit or throw it does not expect, and removes its temp workspace in a finally.
• Harness env override. createProviderScenarioHarness({ env }) lets a scenario set what its daemon host reads; the harness's own state dir and transport are set last so an override cannot desync sessionDir(). The iOS recording scenario uses it (its process-level PATH still hides real tools from code that has not moved to the host yet).
• fix(daemon): scope a transient device claim to the session that holds it #3714 on the explicit host. The new transient-claim code abandons a claim under the same env that cleared it, and the new claim tests pass the env.
• Threads. All inline threads are resolved; cubic reports every issue addressed.
• Still pending: narrowing commandHostEnvironment() to command execution, until refactor(host): read host settings from HostEnvironment in root src and remaining packages #3775 and refactor(platforms): read host settings from HostEnvironment in platform and capture packages #3777 have converted their sites.

[thymikee]

The code is ready for review at 870cf66. The earlier review (#3763 (comment)) found no problems, and the new #3775 changes in this round do not change that. The harness env ordering is now fixed, so the cubic P2 thread on harness.ts:119 no longer applies, so you can resolve it.

Not blocking, and you can take or leave these: the harness builds its gateway with env: process.env at https://github.com/callstack/agent-device/blob/870cf66/test/integration/provider-scenarios/harness.ts#L151 while its daemon host uses the scenario env, so passing input.host.env would match production (src/daemon-composition.ts:95) and let you drop the process-level PATH stub; the abandon call at https://github.com/callstack/agent-device/blob/870cf66/packages/daemon/src/resources/device/device-claim-admission.ts#L131 has no test that fails if it reverts to process.env, so the failed-release test could use a claims dir that differs from process.env and assert the record lands there; and the PR body still says non-owned root src keeps inline reads for #3751 and names validation commit 225e0bd, so it could mention #3775 and the current head.

The only open item is CI. Smoke Tests are queued or running with no failure yet. #3775 changes the root composition env and the CLI handler host, which the smoke route uses, so a smoke failure here would need a look. Smoke Tests must pass at 870cf66 before merge.

I did not run tests. I reviewed #3775's 98 files for new process.env or cwd reads, layer crossings and the resolveHubAppReference contract, but not its test call-site churn line by line. The changes merged from main (#3776, #3714, #3762) are upstream, and I checked them only where they meet host plumbing.

[cubic-dev-ai]

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

_{You’re at about 97% 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]

The code in 9f3e58f is clean. The new commits since 870cf66 only add a daemon unit test, the provider-scenario harness changes, and a comment in the iOS record-trace scenario, and they fix the earlier gap. I read abandonDeviceClaim and writeOwnedDeviceClaim at this head and the new test matches what they do. I did not run the new device-claim-admission test or its mutation.

Smoke Tests is still running and has no failure yet. The new commits do not touch the smoke route, but the PR as a whole changes entry-point host plumbing that smoke exercises, so any failure there needs its own check. The PR has no known conflicts, and Smoke Tests must finish green before merge.

One open inline thread from another reviewer does not apply: the comment in ios-record-trace.test.ts lines 115-116 is accurate. #3775 merged into this branch, but the PATH and Swift-cache readers on this route still read the process environment at head (swift-cache.ts:140 and buildSwiftToolEnv() defaulting to process.env). #3777 migrates them, including src/platform-runtime-screen-recording-finalizer-host.ts. I only checked #3777 through its file list and PR body, not its contents. Please resolve it: #3763 (comment)

[thymikee]

Pushed cb17bb3051 (check:affected passed on it): current with main (#3760, #3762, #3784, #3785). The one conflict was a replay-repair test, where the new close call now passes the host. The cubic thread on the recording scenario's comment is fixed (779dcab21d) and resolved; no thread is open.

[thymikee]

Reviewed cb17bb3. The code looks good, and I found no new problems since the earlier review at 9f3e58f. The range-diff shows the earlier commits unchanged, plus one test-comment commit (779dcab) and the merge with main.

The one merge conflict, in the replay-repair test, is resolved correctly: every handleCloseCommand call there now passes a test host, and the file reads no process.env or process.cwd. The cubic thread on the recording scenario is fixed in 779dcab and resolved.

All checks pass, and there are no conflicts. Nothing else blocks human review.

[thymikee]

This PR is ready. The code at fab82cc looks right to me, and the earlier review at cb17bb3 still holds. All 23 checks pass at fab82cc, and there are no conflicts. I read the diff and ran no tests myself. I could not see the check names, so I can't confirm which device lanes ran. The device-facing spawn changes (runner xcodebuild, snapshot bridge, swiftc, companion) match the old behavior on the daemon route, because host env equals the process env captured at startup and requestEnv still layers on top. Not blocking, and you can take or leave these: readMetroSessionRuntime in https://github.com/callstack/agent-device/blob/fab82cc/packages/react-native/src/metro/metro-session-hints.ts#L124 keeps an outer try/catch around readScopedMetroSessionRuntime, which already catches, so one catch around resolveMetroSessionScope plus the read would be enough; and the PNG worker in https://github.com/callstack/agent-device/blob/fab82cc/packages/capture-kit/src/png-worker-client.ts#L51 no longer calls process.emitWarning and now starts with env {}, which could get a line in the PR body or CHANGELOG. Human merge review is the next step.

[thymikee]

Applied at 4f6dd3e722 (check:affected passed on the exact commit):

• metro-session-hints.ts: dropped the redundant outer try/catch. One catch now wraps resolveMetroSessionScope; the scoped read follows it.
• PNG worker: the CHANGELOG and the PR body now say png-worker-client.ts no longer calls process.emitWarning and starts with env {}. The reason is reported once through png_worker_unavailable.

[thymikee]

The delta from fab82cc to 4f6dd3e looks right. It touches only CHANGELOG.md and one catch in metro-session-hints.ts, and readScopedMetroSessionRuntime already catches its own failures, so moving it outside the catch changes nothing. The earlier review of the other files still stands. All 23 checks pass, and there are no conflicts. I ran no tests myself and relied on CI.

One observability gap, not blocking on its own: in png-worker-client.ts, reportWorkerUnavailable sets a process-wide latch in the first diagnostics scope that reaches the fallback. That is usually a successful screenshot or snapshot request, and its events are dropped, because request-finalization.ts:77 calls flushDiagnosticsToSessionFile() without force. Base wrote process.emitWarning to daemon stderr, which reaches daemon.log. Head removed that write, and the latch blocks any later report. So without --debug, a daemon whose PNG worker cannot start runs on the slower path and leaves no record, although the CHANGELOG says the reason is reported.

On simplicity: the new concept is a one-time, process-wide report that waits for a diagnostics scope, and capture-kit detects a scope by counting the keys of getDiagnosticsMeta(). Daemon-level events without a request scope already have an owner in withDaemonDiagnosticsScope, which force-flushes to daemon.log. Could the daemon wrap prewarmPngWorker in that scope, and could png-worker-client emit png_worker_unavailable in every request scope that takes the fallback, with no latch? That removes the key-count check and the latch, and the reason still reaches a kept log. If you rejected that shape for a reason, a short note is enough. I removed the ready-for-human label until this question is settled.

[cubic-dev-ai]

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

_{You’re at about 99% 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]

Applied at 595fa4822c (check:affected passed on the exact commit): PNG worker unavailability is no longer latched or detected by counting getDiagnosticsMeta() keys. The daemon runs prewarmPngWorker inside withDaemonDiagnosticsScope (force-flushed to daemon.log), and every request scope that takes the fallback emits png_worker_unavailable. New daemon test (daemon-runtime-png-worker.test.ts) starts a runtime whose worker cannot spawn and asserts the event in daemon.log; it fails without the scope. The capture-kit test now pins per-scope reporting. CHANGELOG and PR body match.

[thymikee]

The earlier review at 4f6dd3e is mostly resolved. At 595fa48 the code is not ready yet: one issue remains. All 23 checks pass on 595fa48, and I have no known conflicts to report.

The remaining issue is the open thread on worker failures after the scope closes (#3763 (comment)). The rule: every transition of the PNG worker to unavailable (spawn failure, async load error, or mid-life exit) must reach daemon.log exactly once, with or without an active scope. Today the scope at daemon-runtime.ts closes once obtainWorker() returns. Later events fall into request scopes, and successful requests drop them without --debug. One owner can enforce the rule: markWorkerUnavailable could call a daemon-supplied onUnavailable sink, registered through prewarmPngWorker and wrapped in withDaemonDiagnosticsScope. Please add a daemon test whose worker emits 'error' after construction. The CHANGELOG line about png_worker_unavailable then holds as written.

I did not run the new daemon test or the capture-kit test. I did not reproduce an async worker load failure. I read the code path in handleWorkerFailure and the unforced request flush instead.

Other open threads still apply. The Cubic thread above is the P2 that holds (daemon.log gap). The P2 on repeated events per failed job (#3763 (comment)) holds, but it is noise only, and the once-per-transition sink fixes it. The P3 on send-failure text used as the unavailable reason (#3763 (comment)) holds and does not block.

Not blocking: the CHANGELOG.md:5 entry says a worker that cannot start leaves a png_worker_unavailable event in daemon.log, which is only true once the fix above lands. You can take it or leave it.

[cubic-dev-ai]

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

_{You’re at about 99% 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]

Applied at d1f3001848 (check:affected passed on the exact commit):

• One owner for unavailability: markWorkerUnavailable records the transition once and calls the sink the daemon registers through prewarmPngWorker. The sink writes through emitDaemonDiagnostic (withDaemonDiagnosticsScope, force-flushed), so spawn failures, async load errors and mid-life exits all reach daemon.log exactly once, with or without --debug and with or without an active scope.
• Requests that take the in-process fallback emit nothing, which also removes the repeated-events report (one request used to log several).
• The reason is typed (entry-not-found, spawn-failed, worker-error, worker-exited) plus a detail string; send-failure text is no longer used as a reason, and a failed postMessage neither disables the worker nor reports.
• Tests: daemon tests for a spawn failure and for a worker that emits error then exit after construction, each asserting exactly one event in daemon.log; capture-kit tests for once-per-transition and for a registration after the transition.
• CHANGELOG and PR body match.

[thymikee]

I found no code problems in d1f3001. The earlier findings from 595fa48 (#3763 (comment)) are fixed, and nothing from that review remains.

Smoke Tests is still running on d1f3001, so it has not failed. This delta changes daemon startup (prewarmPngWorker at daemon-runtime.ts:898), and the smoke run goes through that route. Please wait for it to finish green before merge. I did not run the capture-kit or daemon tests. I judged the regression coverage by reading the code from before this change. I also did not check whether a worker 'error' fired inside an active request's AsyncLocalStorage context could write twice to that request's log. The nested withDaemonDiagnosticsScope forces the log path to daemon.log, so I expect it does not.

The cubic-dev-ai P2 thread on the sink surviving terminatePngWorker does not apply, so please resolve it: #3763 (comment). The only production caller is src/daemon.ts, and shutdownOnHost exits the process right after terminate. Any respawn in that window uses the same daemon and log path. A later prewarm in the same process replaces the sink.