Skip to content

fix(daemon): finalize every host-owned close through registration - #3797

Open
thymikee wants to merge 1 commit into
fix/daemon-recorded-owner-safetyfrom
feat/agent-device-host
Open

thymikee wants to merge 1 commit into
fix/daemon-recorded-owner-safetyfrom
feat/agent-device-host

Conversation

@thymikee

@thymikee thymikee commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Summary

Hosted runtimes now close through an authenticated, instance-fenced stop. The registration owner writes every hosted close to one daemon-shutdown.json, beside the legacy report. No request ledger, separate completion file, release facade, or expiry policy remains.

Stops keep the original token, PID and process start time. They read the matching completion and report together. A direct close answers stops arriving after listener shutdown. Retained completion answers later stops while its host lives. stopped: true with warnings and unknown cleanup confidence means serving stopped; cleanup remains unconfirmed.

agent-device daemon stop --state-dir /tmp/agent-device

This 27-file, 2,480-gross-line layer depends on the ownership-safety prerequisite. The combined stack is 4,186 gross lines. Part of #3747 and #3757; public host entry and binding stay in #3757. Shipped quiesce, report readers and startup budgets remain intact.

Validation

Tested 4999683a84a20ed2946a7a6663c1ad77b4fd3bdd against parent b7df117c00df0e2ff4d828d3c4089998113018f4. The identical real-runtime released-close regression times out on 6fce5ec6e2 and passes here in 1.87 s, including two concurrent stops and zero host signals. This preserves the later bc45f60 fix.

Foreground affected gate: 10 checks and 4,029 related tests passed. Receipt run-w9iUs6: exactCommit=true, checkoutChanged=false. Independent code and sentence reviews passed. CI-owned checks remain pending.

@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-19 07:09 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.41 MB 5.41 MB +5.0 kB
Package (unpacked) 5.41 MB 5.41 MB +5.0 kB
Package (download) 1.63 MB 1.63 MB +1.7 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 34.6 ms 34.2 ms -0.4 ms
CLI --help 67.6 ms 65.3 ms -2.4 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 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

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.

@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.

@thymikee

Copy link
Copy Markdown
Member Author

The earlier gap is fixed. At 4f68cd7 all 20 checks pass, including Coverage, which failed at b3460e2 on the eager-closure probe for daemon-shutdown-report.ts. This push removes the host-kit/file import that caused it. The required evidence is now in place, and I did not run the embedded stop route or the timing case below myself.

One problem remains in the stop path. A retained close keeps daemon.json, but the completion record only answers for 60 s, and a record is only written after an accepted shutdown (daemon-client-stop.ts#L125, daemon-shutdown-report.ts#L289). So a stop sent more than 60 s after a retained close, or after a host closed its own runtime, finds no matching record. The stop then waits the full 60 s and throws daemon_shutdown_unconfirmed at a runtime whose listener is already closed. The rule to enforce: every close that leaves its registration in place publishes an instance-fenced record, and that record answers every later stop of that instance until the next acquisition clears it. Acquisition already clears it, so the age bound adds nothing to the pid and start-time fence for the same instance. Please either write the record on every retained close, or skip the age check for a retained record whose instance matches the registration still on disk. Then answer the thread with a test that ages the record past 60 s.

The cubic-dev-ai thread on this exact wait still holds, narrowed to retained closes: #3797 (comment). It resolves with the fix above or a reply that gives the reason.

Not blocking: nothing in production produces embedded ownership until #3757, so only tests that start startDaemonRuntime directly reach this route; you can take or leave that as a note in the PR. Before merge, fix the retained-stop wait or reply to the thread with the reason. The split question open since b3460e2 also needs an answer.

@thymikee

Copy link
Copy Markdown
Member Author

Round three. Split done, and findings 1–8 are in. New head 1c2e9172ed, pnpm check:affected --run passed there with exactCommit=true (receipt .tmp/validation/run-pcv7bF/receipt.json).

The split. daemonBinding and the judgeStartupExit refactor are gone from this branch: the binding module, its contract, the client option, the attach branch and their tests are deleted here and carried to the agent-device/host PR, which is where the pin gets its producer. waitForDaemonStartup is back to the shape it has on origin/main. judgeStartupExit had no other consumer on this branch, so removing it removes the refactor too.

That lands the change the split ask describes: 19 production files at 1,176 net / 1,670 gross lines, plus 8 lines of docs and CHANGELOG. The remaining 2,909 gross lines are tests for that behavior. Gross total is 4,587, still over the 2,500 line cap, and the only cuts left would separate the ownership read from the stop route that consumes it — which your review asked to keep together. Say the word if you want a third layer anyway; the seam would be ownership + admission in one PR and the completion record plus daemon stop in the next.

In the CLI, identity, ownership and the --clean lifetime now come from the one read readDaemonStopIdentity returns, so the stop went from four reads of daemon.json to at most two, and stopDaemon lost the stopEmbedded callback that used to carry ownership across the gap between them.

(1) daemon-record-unreadable. A record that is present and cannot be read or parsed is no longer an absence: stop refuses it and signals nothing, where it used to answer that nothing runs. daemon stop and the embedded route share the one error, so a file that is not JSON says the same thing on either route, and daemon-embedded-stop-unreachable now means only what it says — a readable record with no way to reach the runtime it names. CHANGELOG, help daemon, commands.md and the client-API reason table all name it. Verified against the built CLI: a daemon.json of not json refuses instead of answering not-running.

(2) Covered by the split paragraph above. readStopCleanupOwner derives the cleanup lifetime from the same read, which closed a real gap: on origin/main the --clean walk followed the CLI's read while the retirement re-read the file inside stopDaemon, so a record rewritten in between sent the two steps to different lifetimes. readDaemonOwnershipFromRecord became dead and is gone. The identity/ownership pair replaces the two reads the ask names; the transport and token the embedded route needs stay a second read, since a record can state an owner and still omit both, and that case has its own refusal.

(3) DaemonRuntimeOwnership = Exclude<DaemonOwnership, 'unknown'> on startDaemonRuntime, createDaemonIdleReap and createDaemonLifecycleAdmission. unknown is what a reader answers for a record it cannot place — a process always knows whether it owns itself — so the daemon's unreachable unknown refusal branch is deleted, along with the test-only ownerUnrecognized() helper, and DAEMON_LIFECYCLE_REASONS.ownerUnrecognized became DAEMON_RECORD_OWNER_UNRECOGNIZED_REASON on the reader side. The tests now rewrite daemon.json under a live runtime, exactly as you suggested: bootRuntimeWithUnplaceableRecordOwner() writes ownership: 'sideways' with everything else about the instance intact, and asserts the reader answers unknown before the stop runs.

(4) One JSDoc block per function, both at the two places you named and checked across the branch.

(5) One --clean refusal statement in cli-help.ts:512, and the sentence now covers the three records it refuses (host-owned, unrecognized, unreadable) plus the fact that stopping without --clean still drains the first two. The --clean hint is record-aware for the same reason: a record that cannot be read is refused by that stop too, so pointing at it would have sent the operator to a second refusal.

(6) Renamed to kebab-case: daemon-embedded-stop-unreachable, daemon-clean-not-detached, daemon-shutdown-unconfirmed. Help, docs and the CHANGELOG follow. The older snake_case reasons (daemon_exit_unconfirmed, daemon_retirement_unconfirmed, daemon_shutting_down) are shipped on the wire and stay.

(7) Adopted, with the consequence you did not name taken as well. The hint for a detached answer against an unreadable owner names the record repair and never daemon stop, asserted both ways in src/daemon-client-embedded-stop.test.ts. And the unknown record now gets the authenticated shutdown request rather than a refusal: asking is free, and the runtime's answer is the proof of whose process this is — under an embedded runtime it agrees and closes itself, under a detached one it answers daemon-not-embedded and nothing is signaled.

(8) The embedded-owner test now stubs @agent-device/daemon-contracts/daemon-process's stopDaemonProcess to record attempts, so the assertion is "no signal attempt was made" rather than "the identity gate declined first". A control row proves the signal path is live for the same pid under a detached record, which is what makes the negative row mean something.

One gate fix worth reporting: making the router the sole owner of stop routing left stopEmbeddedDaemon with no production consumer, and the dead-code gate caught it. It is module-private now rather than suppressed or allowlisted.

The stop-wait thread stays open, with the receipts and the case that still bothers me in its reply. Not merged. Watching for the next pass.

@thymikee

Copy link
Copy Markdown
Member Author

Fourth round: 00b82dc6ce.

The retained-stop gap from your 15:33 comment is fixed, both halves. Rule enforced as stated: every close that leaves its registration in place publishes an instance-fenced record, and that record answers every later stop of that instance until the next acquisition clears it.

  • The age bound no longer applies to a retained record. A retained close leaves daemon.json behind and only a new acquisition clears the completion file, so for that outcome the lifetime fence is the whole test — the age added nothing. released keeps its 60 s window, since its job is to answer the stop that asked. daemonShutdownCompletionStillRetained carries the distinction, and the wait proves the writer is still live before trusting an aged record, so a dead host's stale file still answers not-running.
  • createDaemonShutdownRelease publishes retention without the ledger, and registration.finish() takes the close's own unfinished lines so a failed unlink says the same thing with no ledger answer. Both gated on host-owned: a detached daemon's stop signals its process and reads the shutdown report, so a completion record there is a fact nobody reads.

Receipts, each red at the previous shape (expected null to match object { outcome: 'retained' }; a 5 s test timeout inside the 60 s budget) are on the stop-wait thread, which now resolves on the condition you set. Docs, CHANGELOG and the cubic thread — the same single violation as the 11:30 review — are covered. The split answer you asked for went out at 16:47, and the "no production producer until #3757" note is in the PR body.

pnpm check:affected --run on this head: exactCommit=true, 11/11 runnable checks.

@thymikee
thymikee force-pushed the feat/agent-device-host branch from 00b82dc to 6fce5ec Compare October 18, 2026 18:13
@thymikee

thymikee commented Oct 18, 2026 •

Copy link
Copy Markdown
Member Author

Rebased onto main (11de319aad → ca016ec4f5): head 00b82dc6ce → 6fce5ec6e2. The PR had gone CONFLICTING, and docs/agents/pull-requests.md says to rebase for conflicts.

What upstream moved, and where this branch followed it

  • fix(daemon): replace fixed 15s startup kill with progress-aware startup budget #3791 split src/daemon-client/daemon-client-lifecycle.ts into itself + daemon-client-startup.ts, and split packages/daemon/src/server/http/daemon-runtime.ts into itself + daemon-startup-diagnostics.ts (loaded function-scoped, ADR 0019). This branch's stop-routing edits are re-anchored to whichever module now owns the code:
    • resolveTimeoutRecovery (this PR's own function) is anchored in daemon-client-lifecycle.ts, next to the DaemonClientSettings it judges and the dynamic import its caller reaches it through; the readReusableLocalDaemon refusals follow that function into daemon-client-startup.ts;
    • createDaemonShutdownRelease keeps its home in daemon-runtime.ts and now receives finishDaemonRegistration as a parameter, because fix(daemon): replace fixed 15s startup kill with progress-aware startup budget #3791 moved that function behind the runtime's function-scoped await import. A static import would have re-grown the daemon entry's eager closure, which is what that move exists to prevent.
  • chore: remove CHANGELOG.md again and gitignore it #3803 deleted CHANGELOG.md and gitignored it, in favour of gh release --generate-notes. This branch's changelog entries are therefore dropped, not moved. Every user-visible statement they carried lives in the docs this PR also changes (client-api.md, commands.md, the reason table and the versioned CLI help).

History: all four commits, same order, same messages. git range-diff 11de319aad..00b82dc6ce origin/main..HEAD reports only context shifts in the hunks listed above — no review content changed, so the threads that are resolved stay resolved at this head.

Validation at this head: pnpm check:affected --run passes with exactCommit=true. Earlier evidence remains attributed to 00b82dc6ce and its predecessors; nothing from those runs is claimed for 6fce5ec6e2 except this one.

@thymikee

Copy link
Copy Markdown
Member Author

The earlier retained-close finding is fixed in 6fce5ec (daemon-client-stop.ts:176-182), and CI is green with 20 checks and none failing, including Coverage. One defect remains, so this is not ready to merge yet.

A host that closes its own embedded runtime with nothing in hand still hangs a daemon stop for 60 s. No shutdown was accepted, so ledger.take returns undefined and closeReport && (completion || !released) at daemon-registration-owner.ts:190 writes no record for a released close. closeSteps closes the listener first, then drains leases, sessions, owners, plugins and runners before it removes the registration. A stop sent in that window still finds daemon.json, meets a refused connection, gets 'gone', and enters waitForClose (daemon-client-stop.ts:154). That loop matches only a completion record or an ended instance. The host stays alive, so the operator waits the full 60 s and then gets a false daemon-shutdown-unconfirmed ("it may still be serving") for a runtime that released cleanly. This is the same class as the retained-close finding, now for the released outcome. The rule: every close of a host-owned runtime publishes one instance-fenced record of its disposition, released or retained, and that record answers every stop of that instance that reached the wait. DaemonRegistrationOwner.finish and recordShutdownRetention own that. Please write the released record on every host-owned close and drop the ledger gate and the completion parameter. Alternatively, make waitForClose treat daemon.json no longer naming the instance as not running. Please add a regression test: an embedded runtime with a held close step, runtime.close() not awaited, and stopRecordedDaemon called once the listener is closed. It should settle in under 10 s without daemon-shutdown-unconfirmed, and it should fail at the current head.

Would it be simpler to make the record a fact about the state dir, as the commit message says, and not an answer to a request? Every host-owned close would write one record from finish, released or retained, with its unfinished list. A released record keeps its 60 s window, and a retained one stays bound to the instance's lifetime. Then createDaemonShutdownAnswerLedger, its accept() wiring in the admission drain, the completion parameter and the (completion || !released) condition can all go, and waitForClose keeps only the instance match and the lifetime probe. That removes one concept and closes the finding above. The cost is that a host's own close leaves a released record nobody asked for, so the test 'a host that closes its own runtime with nothing in hand leaves no stop to answer' would have to change, since it asserts null today.

On the open threads, the cubic-dev-ai thread on the released-close wait still applies, even though it is marked resolved. The retained part is fixed, but a refused connection during a released host close still waits 60 s, as described above.

Not blocking: createDaemonShutdownRelease in daemon-runtime.ts:283 takes finishRegistration as a parameter only to keep it out of the eager closure, and finish() now takes two overlapping report arguments (completion.unfinished and unfinished). You could move it beside finishDaemonRegistration in the function-scoped module and collapse the two arguments into one close report, or leave both as they are.

I reviewed 6fce5ec by reading the code and did not run the race, so the mechanism comes from the close ordering, the 'gone' mapping and waitForClose. I also did not run the eager-closure probe for the injected finishDaemonRegistration, so that rests on green CI. The embedded route has no production caller until #3757, so only tests that call startDaemonRuntime directly reach it. I compared the rebase against 4f68cd7 using the range-diff, not against 00b82dc. No conflicts are known. Before merge, the released-close stop must settle quickly, with the regression test above proving it.

@thymikee
thymikee force-pushed the feat/agent-device-host branch from 6fce5ec to bc45f60 Compare October 19, 2026 06:53
@thymikee

Copy link
Copy Markdown
Member Author

Fixed in bc45f60. I took the second framing: the record is a fact about the state dir, not an answer to a request.

What changed

  • DaemonRegistrationOwner.finish(outcome, unfinished) — the completion parameter and the (completion || !released) condition are both gone. Every close of a host-owned runtime writes its disposition, released or retained, from the removal the call just proved.
  • createDaemonShutdownAnswerLedger, its accept() wiring in the drain, and DaemonShutdownStopAnswer are deleted. waitForClose keeps only the instance match and the lifetime probe, as you described.
  • DaemonShutdownCompletionQuery stays; buildDaemonShutdownCompletion now takes the close's unfinished directly.

Regression test

a stop that arrives while its host is closing settles on the record that close writes (src/daemon-client-embedded-stop.test.ts). It holds createPlatformRuntimeGateway().shutdown open — that step runs after await serversClosed and before the release, so entering it proves the listener is shut while daemon.json is still published — starts stopRecordedDaemon without awaiting runtime.close(), then releases the step. It settles in 160 ms. I ran it against 6fce5ec with the source otherwise reverted: it fails there, hitting the test's own 5 s cap before the record ever appears.

The test you predicted

a host that closes its own runtime with nothing in hand leaves no stop to answer asserted null and is now a host that closes its own runtime records the release nobody asked for, asserting outcome: 'released', unfinished: []. Two neighbouring claims said the same thing in prose: the DaemonShutdownCompletion header ("a record nobody asked for is not evidence about a stop made now") and the drain contract in daemon-idle-reap.ts ("a shutdown answers the stop that asked for it, and a quiesce answers nobody"). Both now state the fact-about-the-dir rule. I left the seam's mode argument alone — nothing reads it since the ledger went, and its test pins it, so that is a separate call if you want it.

I did not touch waitForClose's "record names the instance" rule, so option B is not needed.

Non-blocking item

Collapsed: finish() takes one close report now (outcome, unfinished). I kept the injection. Moving createDaemonShutdownRelease into daemon-startup-diagnostics.ts would make daemon-runtime.ts import that module eagerly to reach the factory, which is the ADR-0019 property the function-scoped load exists to hold. So the factory stays where the close that calls it lives, and the injection stays its one parameter. Say the word if you read that boundary differently and I'll move it.

Gate: pnpm check:affected --run passed with exactCommit=true at bc45f60. Same four commits, only the fourth changed.

@thymikee
thymikee force-pushed the feat/agent-device-host branch from bc45f60 to 4999683 Compare October 19, 2026 06:56
@thymikee thymikee changed the title feat(daemon): let a host own a runtime and answer its stop fix(daemon): finalize every host-owned close through registration Oct 19, 2026
@thymikee
thymikee changed the base branch from main to fix/daemon-recorded-owner-safety October 19, 2026 06:56
@thymikee
thymikee added this pull request to stack #3807 October 19, 2026 06:56
@thymikee

Copy link
Copy Markdown
Member Author

Published the approved split: #3806 owns signal and cleanup safety. #3797 now builds on it with authenticated close and completion. Each actual PR diff fits the cap: 2,036 and 2,480 gross lines. The combined stack is 4,186 lines.

The registration owner now finalizes every host-owned close. One shutdown file holds the report and completion. The request ledger, separate completion file, release facade, duplicate release flags, and second address read are removed. Stops keep the original token, PID and process birth. The cleanup script also preserves recorded ownership; an omitted caller field cannot grant signal authority.

Local validation passed at b7df117c00 for #3806 and 4999683a84 for #3797. Receipts run-a3LXIt and run-w9iUs6 have exactCommit=true and checkoutChanged=false. Independent code and different-provider sentence reviews passed.

The released-close regression fails on 6fce5ec6e2 and passes on 4999683a84. It preserves the later bc45f60 fix and checks two concurrent stops. CI is pending at the new heads. Neither PR is reported merge-ready yet.

@thymikee
thymikee force-pushed the feat/agent-device-host branch from 4999683 to 2d5eef0 Compare October 19, 2026 07:05
@thymikee

Copy link
Copy Markdown
Member Author

Coordination update: the maintainer approved the two-layer stack, with #3806 below this PR. The later push to 2d5eef0694 restored the earlier branch history instead of building on #3806, which caused these conflicts. I am restoring the reviewed stack and preserving that push’s new post-close detached-completion assertion and direct-close documentation. Further changes to this PR must start from the stacked head above fix/daemon-recorded-owner-safety. The restoration is being validated before push.

@thymikee
thymikee force-pushed the feat/agent-device-host branch from 2d5eef0 to 4999683 Compare October 19, 2026 07:09

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