Skip to content

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

Open
thymikee wants to merge 34 commits into
refactor/host-environmentfrom
refactor/daemon-runtime-no-process-globals
Open

thymikee wants to merge 34 commits into
refactor/host-environmentfrom
refactor/daemon-runtime-no-process-globals

Conversation

@thymikee

@thymikee thymikee commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

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 0b83711eb1 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.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

thymikee and others added 4 commits October 17, 2026 01:35
… process

startDaemonRuntime no longer calls process.exit or installs process
handlers; src/daemon.ts owns signals, fatal errors and the exit code.
close() is idempotent and resolves to a ShutdownReport. A runtime that
fails during startup rolls back what it acquired.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… uploads and requests

Provider admission, interactor resolution, artifact and upload tracking,
resumable uploads, materialized paths, session-open locks and request
cancellation are owned by the runtime instance instead of module scope.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The Apple runner's sessions, locks, recycle ledger and owner state belong
to the daemon runtime that composed them. The PNG worker is held by every
runtime that uses it and stops with the last one.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

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

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 32.7 ms 33.4 ms +0.7 ms
CLI --help 62.2 ms 63.2 ms +1.0 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/daemon/src/server/http/daemon-runtime.ts Outdated
Comment thread packages/daemon/src/server/http/daemon-runtime-isolation.test.ts Outdated
Comment thread packages/daemon/src/server/http/daemon-runtime-close.test.ts
Comment thread packages/daemon/src/sessions/materialized-path-registry.ts
Comment thread packages/daemon/src/server/resumable-upload.ts Outdated
Comment thread src/daemon.ts Outdated
Comment thread packages/daemon/src/resources/artifact-tracking.ts
Comment thread packages/capture-kit/src/png-worker-client.ts
Comment thread packages/daemon/src/server/http/daemon-runtime-composition.test.ts Outdated
…moves temp files

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread packages/daemon/src/server/http/daemon-runtime-close.test.ts
Comment thread packages/daemon/src/server/resumable-upload.ts
Comment thread packages/daemon/src/resources/artifact-tracking.ts
…one removal helper

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

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 and others added 3 commits October 17, 2026 07:31
…s to the runtime

Closing one daemon runtime no longer retires another runtime's start
admissions or signals its prep builds.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The runtime context, the per-runtime state list and the session teardown
helpers move beside the other shutdown mechanics.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ranch

* refactor/host-environment:
  fix(host): resolve every client read from the client's host
@thymikee

Copy link
Copy Markdown
Member Author

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.

…-no-process-globals

* refactor/host-environment:
  style: format the daemon policy reuse test
  fix(host): resolve the daemon policy path against the host working directory

# Conflicts:
#	packages/daemon/src/server/http/daemon-runtime.ts
@thymikee

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member Author

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 thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 17, 2026
… migration

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thymikee
thymikee added this pull request to stack #3778 October 17, 2026 08:11
thymikee and others added 2 commits October 17, 2026 10:13
…refactor/daemon-runtime-no-process-globals

* origin/refactor/host-environment:
  test: keep the registration-owner test under its size ratchet
  fix(host): start a launched daemon in the launching host's working directory
  test: merge the duplicate test-kit import in the handler chain routing test
  test: move the daemon-composition tests off the daemon's testing exports (#3742)
  fix(android): ignore emulator log lines in -list-avds output (#3743)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

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 and others added 2 commits October 17, 2026 11:02
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…refactor/daemon-runtime-no-process-globals

* origin/refactor/host-environment:
  refactor(daemon): read configuration from the request host instead of the ambient scope
  test: merge the duplicate test-kit import in the stale provider admission scenarios
  feat(cli): ensure the daemon explicitly before attaching in the CLI and MCP (#3766)
  feat(client): daemonLifecycle attach mode and ensureDaemon() (#3765)
  feat(daemon): quiesce request that retires an idle daemon atomically (#3764)
  fix(daemon): answer a named missing session on the session, not the device (#3701)
  test: route test/ and scripts/ off the daemon's testing exports (#3733)
  refactor(plugins): make the plugin framework provider-independent (#3722)
  fix(apple): name an Xcode too old for its host macOS as a runner startup failure (#3739)

# Conflicts:
#	packages/daemon/src/server/http/daemon-runtime.ts
@thymikee

Copy link
Copy Markdown
Member Author

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 thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 17, 2026
…refactor/daemon-runtime-no-process-globals

* origin/refactor/host-environment:
  fix(daemon): drop the unused host from install-from-source and tighten test envs
@thymikee

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member Author

This PR is ready at 5a20a1c. The earlier findings from b29df19 are fixed: the bounded-close log assertion now matches the emit path, and the shutdown report carries the provider-release outcome to stderr. Not blocking: the loop in src/daemon.ts that writes the unfinished-work lines to stderr is only covered through the pure function test, so you could extend src/daemon-entrypoint.test.ts with a closed report that has abandoned: 1 and check its line reaches stderr, or leave it.

The two cubic threads are fixed at this head and you can resolve them: the close path now releases the PNG worker, plugins, registration and lock even when a step fails (#3772 (comment)), and the shutdown error now reaches stderr before the exit code is set (#3772 (comment)).

I read the diff and ran no tests or gates myself, and I took the check:affected pass on 5a20a1c from your notes. There are no conflicts. Smoke Tests is still running, and it starts and stops a real daemon, so it exercises this PR's close and exit path. It needs to finish green before merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 17, 2026
thymikee and others added 2 commits October 17, 2026 13:54
…e and its test share

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…refactor/daemon-runtime-no-process-globals

* origin/refactor/host-environment:
  test: pin the one-transport CLI run and one-host MCP call; document the bound host
@thymikee

Copy link
Copy Markdown
Member Author

Pushed fff0337b83 (check:affected passed on it) for the 11:38Z recommendations.

  • Closed report reaches stderr, tested. The executable's close handling is now reportDaemonClose({ closed, stderr }) (in server-shutdown.ts, exported from @agent-device/daemon); src/daemon.ts calls it. src/daemon-entrypoint.test.ts drives it with a closed report that has requests.abandoned: 1 and asserts its line reaches stderr, that a clean report writes nothing, and that a failed close writes Daemon shutdown error and reports failure.
  • Cubic threads. None are open on this PR.
  • Merge. Contains feat(host): capture HostEnvironment once at entry points #3763 at 9a4e0812f4 (field doc, one-transport and one-host tests).

…refactor/daemon-runtime-no-process-globals

* origin/refactor/host-environment:
  test: keep the failure that explains a missed materialization in the transport test
…-no-process-globals

* refactor/host-environment:
  feat(provider-testmu): publish @agent-device/testmu on plugin contract v2 (#3770)
  docs(plugins): document plugin contract v2, the inventory and ADR 0034 (#3769)
  feat(daemon): daemon status lists the registered provider plugins (#3768)
  feat(plugins): plugin contract v2 and one provider registry (#3767)

# Conflicts:
#	packages/daemon/src/server/http/daemon-runtime-composition.test.ts
#	packages/daemon/src/server/http/daemon-runtime-metadata-ownership.test.ts
#	packages/daemon/src/server/http/daemon-runtime.ts
#	src/daemon-composition.ts
thymikee and others added 4 commits October 17, 2026 15:00
…etirements and shell probes per runtime

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…eside Apple's runner-owner

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…me-no-process-globals

* origin/main:
  fix(ios-runner): stop wide sliders from wedging tree snapshots (#3776)
… report

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

The PR is ready from the code side. At 794c048 the shutdown-failure thread is fixed: src/daemon.ts sets the exit code to 1 when the close report says it failed, and server-shutdown.ts writes "Daemon shutdown error". Not blocking: the reportDaemonClose case in src/daemon-entrypoint.test.ts would fit better next to the describeUnfinishedShutdownWork test in daemon-runtime-bounded-close.test.ts, where you can reuse cleanReport, and you can take or leave that.

Checks are green: 17 pass and none fail at 794c048. I read the one-line wiring in src/daemon.ts (close report, then exit code, then process exit) but no test drives the runDaemon path through a real signal or idle-reap close. I did not run the tests myself.

The PR has merge conflicts with its base. Before merge, please rebase onto refactor/host-environment at c9aa2f2, which brought in plugin contract v2. Then resolve the four conflicting files: packages/daemon/src/server/http/daemon-runtime.ts, src/daemon-composition.ts, daemon-runtime-composition.test.ts and daemon-runtime-metadata-ownership.test.ts. I did not review that resolution, since it does not exist yet.

The cubic-dev-ai thread about swallowed shutdown failures no longer applies, so please resolve it: #3772 (comment)

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 17, 2026
thymikee and others added 4 commits October 17, 2026 15:53
…bals

* commit 'db55a5dfbd':
  test: give the iOS recording scenario's daemon host the PATH and Swift cache its process already had
  refactor(daemon): read the host cwd where the deploy handlers use it, leaving their shared prologue untouched
… the real entrypoint through a signal and an idle reap

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…bals

* commit 'd8a9bd40c0':
  fix(daemon): abandon a transient device claim under the same env that cleared it
  fix(daemon): scope a transient device claim to the session that holds it (#3714)
  test: keep the scenario harness's state dir and transport out of reach of its env override
  test: fail the transport test on an unexpected exit or throw and always remove its workspace
…e the helper already loads

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

Pushed 2f4e815f26 (check:affected passed on it): merges the current refactor/host-environment (d8a9bd40c0, with current main) and applies the coordinator's two #3757 items plus the review recommendations.

  • Android helper state is per runtime. The snapshot-helper sessions, failed starts, pending retirements and adb shell-protocol probes live in per-runtime slots (kept in adb-shell-protocol.ts, exported as @agent-device/platform-android/helper-runtime-state and registered by the composition next to the Apple runner state). A close tears down only its own runtime. snapshot-helper-session-runtime-isolation.test.ts closes runtime A while B's helper session stays reused with no respawn, and checks B's close does not clear a retirement A holds pending; both fail if the state is made global again.
  • ShutdownReport.plugins comes from the registry's close() outcomes, once. The separate raw-runtime loop is gone. createPluginClose calls deps.shutdownProviderRuntimes (the registry's idempotent close()), bounded by the plugin budget, and maps the per-plugin outcomes to disposed, failed (with the step and source) and pending. Startup rollback and the final release go through the same memoized call, and composition failure is already closed by the composition itself.
  • Close report tests together. The reportDaemonClose case moved next to the describeUnfinishedShutdownWork test in daemon-runtime-bounded-close.test.ts and reuses cleanReport.
  • The entrypoint is driven through real closes. daemon-entrypoint.test.ts runs src/daemon.ts through a SIGTERM close and a new idle-reap close, asserting exit 0, nothing on stderr and a released registration.
  • Gates. The new Android subpath is enumerated beside Apple's runner-owner in the platform package policy, and the state sits in a module the helper already loads, so platform-android/mechanics's eager closure does not grow. The plugin close is split into its budget race and its report to stay under the complexity gate.
  • Threads. None are open on this PR.

thymikee and others added 2 commits October 17, 2026 16:55
…ost module, not a seventh subpath

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…bals

* commit '870cf663c4':
  fix: align replay, SDK, and session boundaries with ADRs (#3762)
  refactor(host): read host settings from HostEnvironment in root src and remaining packages (#3775)
  fix(daemon-client): document the drain timeout and tighten ensureDaemon follow-ups (#3783)
@thymikee

Copy link
Copy Markdown
Member Author

Pushed 0b2939c3a9 (check:affected passed on it): current main (#3762, #3783) and refactor/host-environment at 870cf663c4 (with #3775) merged.

  • No seventh platform-android export. ./helper-runtime-state is gone. The per-runtime helper state (createAndroidHelperRuntimeState, helperRuntimeSlot) now lives in adb-host.ts, the module that already owns the bound adb host and its async-local scope, and is exported through the existing ./adb-host subpath. adb-shell-protocol.ts, the helper session lifecycle and the retirement module read their slots from it, and the composition registers it next to the Apple runner state.
  • Why not a new module. platform-android/mechanics has a no-growth eager-closure budget, and a new helper-runtime-state.ts added one module to it. adb-host.ts is already in that closure, so the budget holds.
  • Policy. The platform package policy still enumerates exactly the existing Android subpaths. The one change is that src/daemon-composition.ts may import ./adb-host besides the host binding, to register the state; the closure budgets, layering and the isolation tests (snapshot-helper-session-runtime-isolation.test.ts) pass.
  • Nothing new on the review threads; none are open.

…refactor/daemon-runtime-no-process-globals

* origin/refactor/host-environment:
  test: build the scenario gateway on the daemon host's env and pin the host claims directory on a failed release
@thymikee

Copy link
Copy Markdown
Member Author

Pushed 2568e52f54 (check:affected passed on it): current with main and refactor/host-environment at 9f3e58ffbb. It brings in #3763's latest (the scenario gateway on the daemon host's env, the host-claims-directory test for a failed transient-claim release, the updated PR body) and nothing new in this PR's own code. No review thread is open.

…refactor/daemon-runtime-no-process-globals

* origin/refactor/host-environment:
  build(package): stop shipping Swift test sources (#3785)
  fix(scroll): take edge hints from a same-rect scrollable ancestor (#3784)
  docs(agents): write agent docs in ASD-STE100 and remove misleading docs (#3723)
  fix(daemon): retain cleanup locks and preserve healed snapshots (#3760)
  test: say which package still reads PATH from the process in the recording scenario
@thymikee

Copy link
Copy Markdown
Member Author

Pushed f8251daeec (check:affected passed on it): current with main (#3760, #3762, #3784, #3785) and refactor/host-environment at cb17bb3051. The merge with #3760's cleanup-lock changes was textually clean, and the daemon server, sessions and core suites plus the entrypoint tests pass on it. The iOS Smoke Tests failure on 2568e52f54 was one Swift runner test (testSynthesizedReplacementLeavesAMaskedFieldUnverified) in code this PR does not touch; I re-ran it. No review thread is open.

@thymikee

Copy link
Copy Markdown
Member Author

Attribution for the iOS Smoke Tests failure on 2568e52f54 and f8251daeec (RunnerTests.testSynthesizedReplacementLeavesAMaskedFieldUnverified, 124 tests, 1 failure): nothing in this PR can reach it.

  • Same Swift tree as main. git diff origin/main at f8251daeec has no .swift, .m, .h, apple/, scripts/build-xcuitest-apple.sh, xcuitest build-settings or workflow file. The test is Swift code identical to main's.
  • The failing step runs no TypeScript from this PR. Run targeted iOS runner XCTest regressions is xcodebuild test-without-building against an xctestrun that setup-apple-runner-build produced with scripts/build-xcuitest-apple.sh. The test ids come from check-xctest-selection.ts, and the derived path and runner environment come from the workflow. The runner TypeScript that this PR changed (runner-artifact.ts, runner-cache.ts, the session and recycle modules) is not executed in that step, and no environment it sets reaches it.
  • Why the job runs here at all. apple-ci-impact.ts treats any change under packages/platform-apple/src/runner/ as an XCTest input, so this PR triggers the Swift tests that a docs-only PR skips. main runs them on every push.
  • It is intermittent on identical inputs. The same Swift passed on this branch at 0b2939c3a9 (iOS check green) and on main at a60fe5a64 and 6e8c2a046c, and failed on 2568e52f54 and f8251daeec. The log of the failure shows keyboard-existence retries before the masked-field assertion (13 bullets kept instead of 8), which is simulator keyboard timing.

I re-ran only the failed Smoke job on f8251daeec (run 39052247147). Unrelated to this change; the reply will follow if that run still fails.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed f8251da. The conflict from 794c048 is gone, and the cubic P2 thread on the daemon exit code is fixed at this head, so you can resolve it (#3772 (comment)). Two things block this head.

The new slot code in adb-host.ts is a near copy of runner-owner-state.ts; only the names differ. That makes eight copies of the "entered store, else standalone store" pattern (daemon x5, host-kit request-cancel, Apple, Android), so a fix to the fallback, to dispose, or to enterWith timing needs a change in each copy. The earlier question about one owner is still open. Could one createRuntimeSlots() (or runtimeSlot(key, create, dispose?) plus a store factory) next to RuntimeStateHandle, in a small dependency-free entry, replace both copies? Apple and Android would then declare only their slot keys, and the daemon copies could fold in later. That would also shrink the 744 net production lines. The first step would be to move RuntimeStateHandle out of the host-kit request-cancel module so platform-android can import it as a value. If that is not the right shape, what made you reject it?

The Android snapshot-helper path now finds its warm session, failed-start backoff, pending retirement and shell-protocol probe through a slot that daemon-composition.ts registers. No test or live run covers this route. The daemon fixture passes runtimeStates: [], so deleting line 136 keeps every test green, and the helper silently falls back to the process-global store. If the context is lost on the real route, sessions and the close-time reset can land in different stores, and the device helper stays claimed. Please run an Android emulator through the daemon at this head: take two snapshots in one session (the second shows metadata.sessionReused true and one helper spawn in the daemon log), then after daemon stop, adb shell pidof com.callstack.agentdevice.snapshothelper prints nothing and device status shows no claim. A daemon-composition.test.ts check that runtimeStates includes the Android and Apple handles would also help. The live run in the PR body covers only the iOS simulator.

The earlier iOS Smoke failure (testSynthesizedReplacementLeavesAMaskedFieldUnverified) passed on rerun and is likely unrelated, because this patch has no Swift change. Smoke Tests is running again now. There are no conflicts.

@thymikee

Copy link
Copy Markdown
Member Author

Update: the re-run of the failed iOS Smoke Tests job (attempt 2 of run 39052247147) passed on f8251daeec, with no code change, and all 17 checks are green. That confirms the earlier failure of testSynthesizedReplacementLeavesAMaskedFieldUnverified was an intermittent simulator run on Swift code identical to main's, as attributed above.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(daemon): daemon runtime owns no process globals

1 participant