Skip to content

feat(daemon): let a host own a runtime and answer its stop - #3797

Open
thymikee wants to merge 2 commits into
mainfrom
feat/agent-device-host
Open

thymikee wants to merge 2 commits into
mainfrom
feat/agent-device-host

Conversation

@thymikee

@thymikee thymikee commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Summary

An embedder that keeps the runtime in its own process must say so where every other process looks: the registration. This adds ownership to the daemon record, makes every client path that could signal or replace a runtime respect it, and gives both stop paths one admission seam. Part of #3747. 43 files.

  • A host-owned runtime stops only by an authenticated, instance-fenced shutdown request, which it answers with its own close() — so host.close() and daemon stop produce identical teardown. Nothing outside signals it: not a managed client, timeout recovery, a forced stop, or --clean (refused with details.reason: daemon_clean_not_detached).
  • Ownership reads three states — detached, embedded, unknown — and every decision that could signal a pid requires a proven detached. A record that is unreadable, or states an owner this build cannot recognize, answers unknown and is refused: retirement, takeover, timeout recovery, --clean, and both lifecycle requests.
  • daemon quiesce (stop-if-idle) and daemon_shutdown (drain) share one request builder, fence and admission check, so one owner cannot grant what the other refuses. The shipped quiesce wire shape is unchanged.
  • A closing runtime publishes one completion record, fenced to its instance (pid plus process start time), so every stop addressed to that instance reads one answer. daemon stop waits on that record plus the instance's lifetime, and reports what the close could not finish as warnings.
  • Scope expansion: a client can pin one instance with daemonBinding: { token, pid, processStartTime } and then dispatches only to it, with its token. That is the issue's requirement 4; it lands here because a host hands the pin back.
const client = createAgentDeviceClient({ stateDir, daemonBinding: hostDaemon });
// every request goes to that pid with its token; a state directory holding any other
// instance, or a record that cannot be read as an address, is refused instead of being
// started or replaced.

daemon status names such a record host-owned, or owner-not-recognized; daemon stop stops it without signaling the host.

Size: 1546 net production lines over 25 files, over the 700 threshold in docs/agents/review.md. Two independent review rounds read this branch; this push answers round two (33 findings), all resolved on the threads.

Layout: the admission seam sits in daemon-idle-reap.ts (same leave judgement, one census), the completion envelope in daemon-shutdown-report.ts (the on-disk shutdown record pair). The eager-closure gate's remedy is a home in an already-evaluated module; renaming the first file is a follow-up (TODO(#3757)).

Reconciled with #3795, which merged onto the same region. One owner per gap: its settleWithinBudget is the only bounded-shutdown helper here (the local copy this branch carried is gone), its openDaemonServers and startup rollback keep the listener-ownership rule, and its pending-platform-owner rule now also decides the completion record — a close that cannot release the holds because a platform owner is still running reports retained, not released, because the registration really is still held. hasUnfinishedWork() covers abandoned requests and pending owners on both exits.

Validation

Base 11de319aad, head 4f68cd7607. pnpm check:affected --run passed there with exactCommit=true: format, lint, typecheck, layering, fallow, build, vitest-related, command-docs, docs-links, docs-prose. Replay and web-smoke lanes are GitHub-authoritative or parked — CI owns them.

Risks: no live device run, only CI lanes. agent-device/host lands next, so embedded is unreachable from a published entrypoint yet.

View guided diff Turn on auto-fix

@github-actions

github-actions Bot commented Oct 18, 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-3797/

Built to branch gh-pages at 2026-10-18 14:44 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.40 MB 5.41 MB +14.4 kB
Package (unpacked) 5.40 MB 5.41 MB +14.4 kB
Package (download) 1.63 MB 1.63 MB +4.7 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 33.9 ms 34.8 ms +0.9 ms
CLI --help 62.6 ms 63.5 ms +0.9 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.

1 issue found across 41 files

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.

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

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

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/daemon-client/daemon-client-stop.ts">

<violation number="1" location="src/daemon-client/daemon-client-stop.ts:108">
P2: After a refused connection, this waits for the host process to exit. A host that keeps running after its runtime closed makes every such stop wait the full 30s, then report that the runtime may still be serving. Confirm the stop's answer for a closed runtime, or report the retained state directly.</violation>
</file>

Comment thread src/daemon-client/daemon-client-attach.ts Outdated
Comment thread packages/daemon-contracts/src/daemon-registration.ts Outdated
Comment thread src/agent-device-client.ts Outdated
Comment thread packages/daemon-contracts/src/daemon-shutdown-report.ts Outdated
Comment thread src/daemon-client/daemon-client-stop.ts Outdated
Comment thread packages/daemon-contracts/src/daemon-stop.ts Outdated
Comment thread packages/daemon/src/server/http/daemon-runtime.ts Outdated
Comment thread src/daemon-client/daemon-lifecycle-errors.ts Outdated
Comment thread src/cli/commands/__tests__/daemon.test.ts Outdated
Comment thread website/docs/docs/client-api.md Outdated
@thymikee
thymikee force-pushed the feat/agent-device-host branch 4 times, most recently from 5cf114c to b3460e2 Compare October 18, 2026 13:05
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto main at 11de319aad (#3795) and reconciled with it; new head b3460e275e, pnpm check:affected --run passed there with exactCommit=true.

One owner per gap after the merge:

  • fix(daemon): close startup and shutdown ownership gaps #3795's exported settleWithinBudget is now the only bounded-shutdown helper on this branch — the local copy this branch carried is removed. Its openDaemonServers and startup rollback keep the listener-ownership rule; this branch adds nothing there.
  • fix(daemon): close startup and shutdown ownership gaps #3795's rule "a platform owner still shutting down keeps the registration and lock" now also decides the completion record. A close that cannot release the holds because an owner is pending, or because a request ignored cancellation, answers retained rather than released — the registration really is still held, so the record says what the state directory shows. hasUnfinishedWork() is the one predicate both exits read.
  • One consequence changed an assertion here: with a platform owner whose shutdown() rejects, the record is now retained and daemon.json stays, where this branch used to report released. The test asserts the registration's presence too, so the pairing is pinned rather than inferred.
  • Fallow's complexity gate read startDaemonRuntimeOnHost at 21 after the merge, so two decisions moved to owners that already hold the vocabulary: the idle window reads ownership itself (resolveDaemonIdleReapMs(env, ownership); a non-detached runtime gets no window), and the accept-once / answer-once state is createDaemonShutdownAnswerLedger() in daemon-shutdown-report.ts. Behaviour unchanged; the reap test now drives ownership instead of an injected window.

Not merged. Watching for the next pass.

@thymikee

Copy link
Copy Markdown
Member Author

The Coverage job (unit-ci) fails at b3460e2, and it also failed on the previous head. The log excerpts stop before the vitest summary, so I could not name the failing test or threshold. This PR adds lifecycle suites that run in that job, so it may be related. Please find the failure in the full job log and fix it or say what causes it. Apart from the open stop-wait thread named at the end, I found no blocking code defect, and there are no conflicts.

Not blocking, and you can take or leave these. (1) An unreadable or invalid daemon.json now reads as "unknown", so daemon stop throws daemon_embedded_stop_unreachable where it used to answer not-running; give that refusal its own reason such as daemon_record_unreadable, and add the CHANGELOG and help text for it (https://github.com/callstack/agent-device/blob/b3460e2/packages/daemon-contracts/src/daemon-stop.ts#L63). (2) One daemon stop reads daemon.json up to four times; readDaemonStopIdentity can return ownership from the same read, which removes the stopEmbedded callback and its unreachable error (https://github.com/callstack/agent-device/blob/b3460e2/src/cli/commands/daemon.ts#L45). (3) DaemonRuntimeOptions.ownership should exclude 'unknown', and the test can rewrite daemon.json to simulate an unrecognized owner (https://github.com/callstack/agent-device/blob/b3460e2/packages/daemon/src/server/http/daemon-runtime.ts#L230). (4) Two JSDoc blocks sit back to back above isOwnerRefusal and above publishDaemonShutdownCompletion, so one function lost its doc. (5) The --clean refusal is stated twice in one sentence in cli-help.ts:512. (6) The new reasons mix kebab-case and snake_case; the client-lifecycle kebab-case convention fits while the API is unreleased. (7) When the record is unrecognized and the daemon says it is detached, the hint should name the record repair, not daemon stop, because that command never signals anything there (https://github.com/callstack/agent-device/blob/b3460e2/src/daemon-client/daemon-lifecycle-errors.ts#L158). (8) In the embedded-owner test, the observed pid is the vitest worker, which the daemon-identity check already refuses to signal, so the "no signal" assertion passes without the ownership gate; stub that check to accept the pid so only the ownership gate stands in the way (https://github.com/callstack/agent-device/blob/b3460e2/src/__tests__/daemon-registration-embedded-owner.test.ts#L64).

The gross diff is +4064/-687, over the 2,500-line cap in docs/agents/pull-requests.md. Could the daemonBinding pin (client option, binding module, attach branch, contracts type, docs and tests) and the judgeStartupExit refactor move to the #3757 host PR, where the pin gets its producer? Ownership, the daemon_shutdown mode on the existing admission seam, and the instance-fenced completion record are the stop requirement and stay together. In the CLI, reading identity and ownership once and routing straight to stopDaemon or stopEmbeddedDaemon would drop two redundant reads. For that, readDaemonStopIdentity and DaemonRetirementInput.observed must carry ownership from the same record read, so routing and the funnel judge one observation.

I did not run daemon stop against a real detached daemon on this head; the detached route only gained ownership reads, and the CI lifecycle suites cover it. The embedded route has no production producer until #3757, so only tests that boot startDaemonRuntime directly reach it. I also have no receipt for the claim that pnpm check:affected passed at b3460e2.

The open inline thread on the stop wait (#3797 (comment)) still stands. The other review threads no longer apply and you can resolve them: r4306845790, r4306845794, r4306845798, r4306845809, r4306845813, r4306845820, r4306845823, r4306845825, r4306845831, r4306845836, r4306845840, r4306845843, r4306845852, r4306845854, r4306845858, r4306845861, r4306845865, r4306845868, r4306845870, r4306845878, r4306845884, r4306845890, r4306845893, r4306845897, r4306845902, r4306845904, r4306845906, r4306845912, r4306845916, r4306845920, r4306845922 and r4306845927. The code now handles each of these, and the comment-fix threads are already done.

A runtime that runs inside another process cannot be stopped by signalling that
process, so its registration now records who owns it and the runtime answers a stop
for itself.

`ownership: 'embedded' | 'detached'` is published in `daemon.json`, read back as the
default `detached` by every reader, and decides three things: an idle timer never ends
an embedded runtime, a shutdown request is answered only by an embedded runtime and a
quiesce only by a detached one, and retiring an embedded record proves the lifetime is
gone without signalling anything.

Both requests are answered at one admission seam that shares the token check, the
pid-plus-processStartTime fence, the answer-before-dispatch rule, and the runtime's one
close owner. A shutdown drains work in hand rather than refusing it, and the close then
answers the stop that asked for it through a bounded completion record written under the
registration claim, before that claim is released.
What the registration now records lets a client tell whether it may end the process a
state directory names, so nothing outside a host touches one it does not own.

A managed client that finds an embedded runtime refuses with `daemon-owner-embedded`
instead of replacing it, and a request timeout retires nothing for it or for a client
pinned to one. `daemon stop` reaches such a runtime over its socket, waits for the
completion record its own close writes, and reports what that close could not finish as
warnings; a stop with no route to that socket is refused rather than falling back to a
signal. `--clean` is refused for it, because that cleanup walks the owner's whole process
tree and a host process runs more than the daemon.

`daemonBinding` pins a client to one instance: its token, pid and process start time. The
pin is compared before dispatch and never after, so a replaced or absent instance is a
typed refusal and never a reason to start, replace, or redirect anything.
@thymikee
thymikee force-pushed the feat/agent-device-host branch from b3460e2 to 4f68cd7 Compare October 18, 2026 14:44
@thymikee

Copy link
Copy Markdown
Member Author

Fixed the red Coverage job and cleared the round-two threads. New head 4f68cd7607, pnpm check:affected --run passed there with exactCommit=true (receipt .tmp/validation/run-XolT0k/receipt.json).

What Coverage actually failed on. The step's visible log ends at "building macOS helper" because that is where the runner stops streaming; the verdict is further down the job log. One test failed, and it is a real one: the ADR-0019 eager-closure probe (scripts/__tests__/eager-closure-budgets.test.ts) reported that packages/daemon-contracts/src/daemon-shutdown-report.ts evaluates 14 modules on import where the merge-base evaluated 1, and adds 386 top-level statements of eager work. It reproduced locally in two seconds.

The cause was mine: publishDaemonShutdownCompletion reached for publishFileSync, which made every importer of that vocabulary module load host-kit/file and its seven internals eagerly. No timeout, no hung lifecycle test. The two pre-flight coverage gates passed, and the changed-line and CRAP verdicts never ran, because both are gated on the coverage step's own success.

The fix is the remedy the gate prescribes rather than a suppression: the write moved to daemon-registration-owner.ts, the module that already holds the claim, already calls publishFileSync, and was already in every affected closure at the merge-base. daemon-shutdown-report.ts is vocabulary and reads again, and the entry evaluates exactly what it did at the merge-base. writeShutdownCompletion keeps the same rule — assert the claim, one atomic publish, 0600 — and nothing about the record's shape or fencing changed.

The cubic thread on that test's mode assertion is resolved by the same move, in the honest direction: the contract test now plants a record as a fixture and says so, and the 0600 assertion moved to src/__tests__/daemon-registration-embedded-owner.test.ts, where the assertion runs against the production write inside a real held claim. A fixture can no longer appear to prove a rule the publisher owns.

All 33 round-two threads are resolved. Each fix was already in the head, in b3460e275e or earlier; I verified every one against the code rather than against the replies, including the four that read as absent to a grep (stopDaemonProcess's corrected JSDoc, daemon-idle-reap's removal of requestId, the assert.ok(paths.infoPath) row, and the rejected-close comment) and the two where the flagged text is now deliberately not present. Nothing was declined, so nothing is left open.

Not merged. Watching for the next pass.

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