Skip to content

refactor(daemon): close embedded runtimes only from their host - #3850

Open
thymikee wants to merge 7 commits into
mainfrom
refactor/embedded-runtime-owner-closes
Open

thymikee wants to merge 7 commits into
mainfrom
refactor/embedded-runtime-owner-closes

Conversation

@thymikee

@thymikee thymikee commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Summary

Embedded runtimes are closed by the host that starts them. daemon stop refuses a live embedded or unknown owner without a request or signal, and retires a registration only after proving its recorded host lifetime exited. In-process close() returns ShutdownReport; unfinished work retains the registration and lock.

const host = await startAgentDevice({ stateDir: '/tmp/device-host' });
const report = await host.close();

Remove external embedded shutdown, completion publication/parsing, and the outside waiter. Remove unused exports and stale guidance. Keep missing stateDir as plain INVALID_ARGS with its hint. The patch changeset names all four accidentally released reasons. Localize internal types and declare TestMu v2 capabilities in code. Released v1 migration remains in #3759.

36 files; +453/−1,869 (2,322 gross). Follows the cleanup handoff and maintainer decisions. Part of #3747.

Validation

Head 86dee15e8610c7800e82889b2331c307e8379de2; base 421dd7e8e7.

  • Exact-head affected gate: all 36 runnable checks passed; 4,143 related tests and 146 Node integrations passed. Round 3 evidence.
  • Production unused-exports check passed twice; independent reviews found no regressions.
  • At 7dc1986: built-package live check: live host refused, zero signals, still serving; exited host registration retired.
  • At 7dc1986: v0.21.29 CLI against that head: error, zero signals, host still serving.
  • CI: 17 successful checks, one skipped, one neutral; coverage, integration, compatibility and Android/Apple/Linux smoke passed. No manual device run required for this lifecycle change.

thymikee and others added 2 commits October 20, 2026 18:12
Remove outside shutdown admission, completion records and stop polling. Keep direct close reports and unfinished-work registration fences. Localize internal types and move TestMu v2 capabilities into code.

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

Copy link
Copy Markdown
Member Author

Built-package live verification at 86ae7b77b39ea249b7e04bc12263fe99163d6034, no device:

The host process imported startAgentDevice from agent-device (the package’s built main entry), using a fresh temporary state directory:

{"phase":"started","pid":73781,"stateDir":"/Users/michal/.t3/worktrees/agent-device/plugin-v2-cleanup/.tmp/plugin-v2/live-state-nM6GoL","sessions":[]}

From a separate shell:

node /Users/michal/.t3/worktrees/agent-device/plugin-v2-cleanup/bin/agent-device.mjs daemon stop --state-dir /Users/michal/.t3/worktrees/agent-device/plugin-v2-cleanup/.tmp/plugin-v2/live-state-nM6GoL --json

Exit: 1. Output:

{
  "success": false,
  "error": {
    "code": "COMMAND_FAILED",
    "message": "A host process owns this runtime, so it was not retired.",
    "hint": "The runtime is owned by host process 73781. Close it there with the handle returned by startAgentDevice(). If the host has exited, call ensureDaemon() for this state directory to recover its abandoned registration.",
    "diagnosticId": "mvgw06wf-8d7ff679",
    "details": {
      "reason": "daemon-owner-embedded",
      "stateDir": "/Users/michal/.t3/worktrees/agent-device/plugin-v2-cleanup/.tmp/plugin-v2/live-state-nM6GoL",
      "daemon": { "pid": 73781, "processStartTime": "Tue Oct 20 18:17:53 2026" }
    }
  }
}

The host then queried its client again, called close(), printed the returned report, and exited 0:

{"phase":"still-serving","sessions":[]}
{"phase":"close","report":{"providerReleases":{"completed":[],"pending":[],"unknown":[]},"requests":{"drained":0,"abandoned":0},"platformOwners":"closed","runners":{"stopped":[],"handedOff":[]},"sessions":{"closed":0,"failed":0},"plugins":{"disposed":["browserstack","aws-device-farm"],"failed":[],"pending":[]}}}

Local exact-head receipt run-hWxhHW: 36 passed, 25 explicit skips, 2 deduplicated; exactCommit=true, checkoutChanged=false. Related tests: 4,137 passed / 551 files. Node integrations: 146 passed / 12 opt-in skips. Eager-closure budget: 853 passed at the same head. Skipped GitHub-owned checks are CI obligations, not local passes.

The final independent review accepted the unconditional refusal required by the handoff. Its suggestion to restore dead-owner stop success was declined because embedded and unknown registrations must refuse; existing managed startup retains abandoned-owner recovery. The lower-level retirement ownership guard remains for direct callers.

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-3850/

Built to branch gh-pages at 2026-10-20 23:06 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.47 MB 5.46 MB -5.4 kB
Package (unpacked) 5.47 MB 5.46 MB -5.4 kB
Package (download) 1.66 MB 1.66 MB -1.8 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 30.3 ms 32.9 ms +2.6 ms
CLI --help 59.6 ms 58.2 ms -1.4 ms

@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the PR. The code at 86ae7b7 has no blocking problems, and all 19 checks pass. The live check comes from your PR comment, and I did not rerun it. One design question below is still open. I also did not run a mixed-version case, where a 0.21.29 CLI sends daemon_shutdown to an embedded runtime built from this head. From the code, I expect an error and no signal, but I did not execute it.

The head test "stop refuses an embedded registration even after its host exits" may not record a start time. If so, it does not cover the case where the shared retirement could prove the host exited. Please confirm that, or add a case that records the start time.

Not blocking, and you can take or leave these: the new embedded/unknown branch in daemon-client-stop.ts rebuilds the refusal that stopDaemon already produces through ownerRefusal, so two places now emit the same reason with different hints. daemon stop on an embedded runtime used to exit 0 and now fails with daemon-owner-embedded, and the removed reasons daemon-shutdown-unconfirmed, daemon-embedded-stop-unreachable and daemon-not-embedded shipped in v0.21.29, so a patch changeset should name them. The host_state_dir_required reason that host.ts no longer returns also shipped in v0.21.29, so please keep it or note its removal in the changeset. Nothing reads DaemonStopIdentity.record at head any more, so it can go from daemon-registration.ts.

Could stopRecordedDaemon keep only the unreadable-record refusal and pass observedDaemonIdentityOfRegistration(record) to stopDaemon for every readable owner, as scripts/clean-daemon.ts does? Then ownerRefusal would be the single source of the owner refusals and would carry the embeddedCloseHint wording. Live hosts would still get no request and no signal, and a host that provably exited would be retired, as takeover already does. This needs one decision first: may daemon stop retire an embedded record whose host provably exited? If it must refuse, please encode that in the retirement owner, not in a second guard.

Before merge, please settle that question and add the changeset for the daemon stop change. I know of no conflicts.

thymikee and others added 4 commits October 20, 2026 19:57
Centralize stop owner refusals and preserve released host diagnostics. Document the stop behavior and removed reasons in a patch changeset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep helpers private after the shared stop route removes their external callers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep missing stateDir as plain INVALID_ARGS with its hint, per the maintainer decision. Name all four accidentally released reasons in the patch changeset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Align ownership comments, the client reason reference, and ADR 0034 with live-host refusal and proven-exit retirement. Keep cleanup comments about detached process-tree authority.

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

Copy link
Copy Markdown
Member Author

Round 2 settles the decision: YES, daemon stop may retire an embedded or unknown registration after proving the recorded host lifetime exited. A live owner receives no shutdown request or terminating signal; its owner closes it.

Item-to-commit mapping:

  • Single readable-owner route through observedDaemonIdentityOfRegistration() / stopDaemon(), shared ownerRefusal hint, and removal of the duplicate stop guard: eb3db8f.
  • The old exited-host test recorded only a PID, not a start time. Replaced it with live/exited embedded and unknown cases, each with/without recorded start time; only proven exited lifetimes retire: eb3db8f.
  • Patch changeset names the live embedded stop failure (daemon-owner-embedded) and owner-close rule: eb3db8f; final four-reason removal wording: 96d8b87.
  • Superseding maintainer decision: host_state_dir_required stays removed; missing stateDir is plain INVALID_ARGS with its hint. Changeset explicitly names all four accidentally released reasons, including daemon-shutdown-unconfirmed, daemon-embedded-stop-unreachable, and daemon-not-embedded: 96d8b87.
  • Remove unused DaemonStopIdentity.record: eb3db8f.
  • Mixed-version check: executed against the built head; v0.21.29 CLI exits 1 with SESSION_NOT_FOUND, host stays responsive and records zero signals.

Second-order removals (one per line):

  • Remove the public export of module-local daemonNotRunningStopResult: ff383a6.
  • Remove the public export of module-local daemonRecordOwnerUnrecognizedError: ff383a6.
  • Remove the now-unused stop snapshot record field and its construction: eb3db8f.

Doc/comment fixes (one per line):

  • Versioned CLI help: live-owner refusal and proven-exit registration retirement: eb3db8f.
  • Website command reference: live embedded/unknown refusal and proven-exit retirement: eb3db8f.
  • Website client lifecycle guidance: owner closes live runtime; stop recovers proven exited registration: eb3db8f.
  • Website embedded-runtime guidance: qualify stop refusal as live-host behavior: eb3db8f.
  • Website daemon-owner-embedded reference: qualify owner as live: 7dc1986.
  • ADR 0034: current ownership decision, proven-exit exception, rejected outside-shutdown alternative and decision-history rationale under FORMAT.md: eb3db8f, 7dc1986.
  • Daemon ownership contract comment: remove unconditional outside-stop refusal: 7dc1986.
  • CLI cleanup comments: state detached process-tree authority without obsolete stop routing claims: 7dc1986.

Sweep evidence: production unused-exports gate passed twice; Fallow audit passed after export cleanup. Whole-repo git grep plus per-removed-symbol searches found no remaining completion-record parser/schema/types, shutdown lifecycle mode/command/admission branch, external waiter/helpers, LogEntry, or HostClientConfig. The four removed reasons occur only in the changeset. Remaining asyncDispose and retained-close references belong to active resource/runner APIs; agent-device/host remains only in a regression asserting that removed subpath cannot import. The #3797 mention in dated performance evidence records a historical rebase, not current API guidance. docs/agents/domain.md is absent on this branch; CONTEXT.md, agent docs, routing cards, README and examples contain no stale references. No compatibility shims or test-only exports added.

Full PR: 2,266 gross changed lines, under 2,500; no follow-up stack needed. Independent read-only reviews found no regressions at the final head.

Validation at 7dc1986fac72f772ea7b58c7a673163967025e65: pnpm check:affected --run --head HEAD passed all 36 runnable checks; receipt run-LqdDxK, exactCommit=true, checkoutChanged=false. 4,142 related tests / 551 files passed; Node integration: 146 passed, 12 opt-in skips. GitHub-owned checks remain CI obligations.

CI: pending at publication; this comment will be updated when it settles.

Full live and mixed-version output

Built-package live check at 7dc1986fac72f772ea7b58c7a673163967025e65 (separate CLI processes; isolated temporary state directory).

{
  "phase": "ready",
  "pid": 89094,
  "sessions": []
}
{
  "phase": "registration",
  "pid": 89094,
  "processStartTime": "Tue Oct 20 20:04:33 2026",
  "ownership": "embedded",
  "stateDir": "/Users/michal/.t3/worktrees/agent-device/plugin-v2-cleanup/.tmp/plugin-v2/live-round2-state-VIr3WK"
}
{
  "phase": "head-cli-live",
  "exit": 1,
  "stdout": "{\n  \"success\": false,\n  \"error\": {\n    \"code\": \"COMMAND_FAILED\",\n    \"message\": \"The daemon was not stopped, because its owner ends it.\",\n    \"hint\": \"The runtime is owned by host process 89094. Close it there with the handle returned by startAgentDevice(). If the host has exited, use daemon stop or ensureDaemon() for this state directory to recover its abandoned registration.\",\n    \"diagnosticId\": \"mvgztadz-05d19b0c\",\n    \"details\": {\n      \"pid\": 89094,\n      \"processStartTime\": \"Tue Oct 20 20:04:33 2026\",\n      \"reason\": \"daemon-owner-embedded\",\n      \"terminationReason\": \"embedded-owner\",\n      \"retirement\": {\n        \"status\": \"retained\",\n        \"reason\": \"embedded-owner\",\n        \"termination\": {\n          \"status\": \"retained\",\n          \"reason\": \"embedded-owner\"\n        },\n        \"removedInfo\": false\n      }\n    }\n  }\n}\n",
  "stderr": ""
}
{
  "phase": "probe",
  "signals": [],
  "sessions": []
}
{
  "phase": "v0.21.29-cli-live",
  "exit": 1,
  "stdout": "{\n  \"success\": false,\n  \"error\": {\n    \"code\": \"SESSION_NOT_FOUND\",\n    \"message\": \"No active session. Run open first.\",\n    \"hint\": \"Run open first or pass an explicit device selector.\",\n    \"diagnosticId\": \"mvgztewg-89b9f540\",\n    \"logPath\": \"/Users/michal/.t3/worktrees/agent-device/plugin-v2-cleanup/.tmp/plugin-v2/live-round2-state-VIr3WK/sessions/default/requests/b01b55cc0eaa0e82.ndjson\",\n    \"diagnosticsRecord\": {\n      \"session\": \"default\",\n      \"requestId\": \"b01b55cc0eaa0e82\"\n    }\n  }\n}\n",
  "stderr": ""
}
{
  "phase": "probe",
  "signals": [],
  "sessions": []
}
{
  "phase": "head-cli-exited",
  "exit": 0,
  "stdout": "{\n  \"success\": true,\n  \"data\": {\n    \"stopped\": false,\n    \"mode\": \"not-running\",\n    \"cleanupConfidence\": \"known\",\n    \"claimsReleased\": [],\n    \"claimsOrphaned\": [],\n    \"claimsSuperseded\": [],\n    \"claimsUnattributable\": [],\n    \"providerReleases\": {\n      \"status\": \"completed\",\n      \"released\": [],\n      \"pending\": []\n    },\n    \"warnings\": [],\n    \"clean\": false\n  }\n}\n",
  "stderr": "",
  "registrationExists": false
}

@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the update. Commit 7dc1986 fixes most of what the earlier review at 86ae7b7 found, and all 19 checks pass at this head. One problem remains.

The --clean refusal hints in src/cli/commands/daemon.ts still read as they do on main. For an embedded record, daemon stop --clean fails with daemon-clean-not-detached and tells the operator to "Stop it without --clean, which drains it through its own close". For an unknown owner (line 93), the hint says a plain stop "asks the running process to leave and reports what it answers". With this PR, a plain stop sends no request to either owner. It refuses a live owner with daemon-owner-embedded or daemon-owner-unrecognized. So an operator who follows the hint hits a second refusal, which is the loop the doc comment at lines 77-82 says the hint prevents. That comment also still says stop "asks the running process to leave". Please make ownerRefusal and embeddedCloseHint in daemon-contracts the only source of next-step wording for embedded and unknown owners. For an embedded record, return embeddedCloseHint(pid), or text that says to close it in its host and that a plain stop retires only a proven-exited registration. For an unknown record, reuse the unrecognized hint from ownerRefusal. Then fix the doc comment. The assertions in daemon.test.ts at lines 333 and 340 match only /Stop it without --clean/, so please tighten them to assert the new wording.

I did not rerun the built-package live check or the v0.21.29 mixed-version check from comment 6297850312, and I ran no tests. The proveExitWithoutSignaling change also reaches the takeover, startup and timeout callers. For an embedded record without a start time, they now get an embedded-owner refusal instead of exit-unconfirmed. Those callers already handle embedded-owner (daemon-lifecycle-errors.ts:100), but no test in this change goes through them. Could you add one that does? Once the hint wording is fixed and tested, this should be ready to merge.

…rt time

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

thymikee commented Oct 20, 2026 •

Copy link
Copy Markdown
Member Author

Round 3 addressed in 86dee15:

  1. Hint wording: --clean now reuses daemon-contracts ownerRefusal / embeddedCloseHint, including the host PID. Embedded guidance says to close in the host and permits registration retirement only after proving that host lifetime exited. Unknown ownership reuses the unrecognized-owner remedy. Updated the doc comment and tightened CLI plus host assertions.
  2. Caller regression: takeover of an embedded registration without processStartTime now has a test through retireDaemonForTakeover and the real retirement path, asserting daemon-owner-embedded, preserved registration, and no signals or spawn.
  3. Stale wording sweep: searched source, docs, website, help, skills, and comments; removed the remaining “asks the running process” / “drains it through its own close” guidance. No further stale matches.

Validation at this exact head: pnpm check:affected --run --head 86dee15e8610c7800e82889b2331c307e8379de2 passed all runnable checks; receipt run-K3Z9bE, exactCommit=true, checkoutChanged=false. Includes 4,143 related tests in 551 files and 146 Node integration tests (12 opt-in skips). Independent review: no findings. The first gate run caught the old exact hint expectation in src/host.test.ts; updated it and reran successfully. CI settled at this head: 17 successful checks, one skipped native-cache matrix job, and one neutral Cubic check. Coverage, Integration Tests, Repo Guards & Compatibility, and Android/Apple/Linux smoke all passed. Waited in the foreground. Re-read maintainer and inline comments after CI; no new requests.

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.

1 participant