Skip to content

feat: add daemon stop lifecycle#1323

Merged
thymikee merged 6 commits into
mainfrom
agent/daemon-stop-stage0
Jul 17, 2026
Merged

feat: add daemon stop lifecycle#1323
thymikee merged 6 commits into
mainfrom
agent/daemon-stop-stage0

Conversation

@thymikee

@thymikee thymikee commented Jul 17, 2026

Copy link
Copy Markdown
Member

Summary

Implements Stage 0 of issue #1320: safe daemon stop and shutdown lease finalization.

  • Verify daemon PID/start-time identity, request graceful shutdown, and escalate only after the bounded timeout.
  • Support --clean for daemon-owned retained Apple runner processes and leases.
  • Finalize active session provider leases during graceful shutdown, drain pending releases, and report known versus forced/unknown cleanup state.

Part of #1320

Validation

  • pnpm typecheck
  • Focused daemon stop, shutdown-report, provider-lease-expiry, and CLI daemon tests
  • pnpm test:integration:progress:check
  • pnpm check:layering

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-17 16:21 UTC

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.8 MB 1.8 MB +9.4 kB
JS gzip 564.7 kB 569.7 kB +5.1 kB
npm tarball 680.9 kB 684.3 kB +3.4 kB
npm unpacked 2.4 MB 2.4 MB +9.4 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 22.8 ms 23.0 ms +0.2 ms
CLI --help 47.2 ms 47.4 ms +0.1 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/runner-disposal.js +47.8 kB +15.5 kB
dist/src/runner-client.js -46.3 kB -13.5 kB
dist/src/cli.js +3.3 kB +976 B
dist/src/internal/daemon.js +1.9 kB +562 B
dist/src/registry.js +159 B +57 B

@thymikee

Copy link
Copy Markdown
Member Author

Not ready yet; three Stage 0 blockers remain on head 8ab03c2:

  1. Bounded shutdown is bypassed by each session release. teardownDaemonSession() awaits finalizeDaemonSessionLease(), which awaits expiredProviderLeaseReleaser.release(). Both hosted and recoverable provider calls can hang without a timeout, and the 2s drain() is reached only after all session finalizers finish. A hung provider therefore drives daemon stop into SIGKILL instead of reaching the bounded graceful drain/report. Journal first where recovery is supported, bound each per-session attempt, and let the final drain/report always run.

  2. The graceful report drops successful per-session releases. release() completes them through retryPending() without the released accumulator; drain() creates a new accumulator later, after those leases were removed. Consequently providerReleases.released only contains work completed during the final drain, not allocations successfully released while finalizing sessions. Preserve the shutdown-wide released set so the result enumerates all released and pending allocations as feat: add cross-worktree device ownership and safe recovery #1320 requires.

  3. This PR must not close the umbrella. feat: add cross-worktree device ownership and safe recovery #1320 explicitly owns stages 0–5; this implements Stage 0 only. Replace Closes #1320 with Refs/Part of #1320, or close a dedicated Stage-0 child issue.

CI is otherwise green; one iOS smoke job is still running.

@thymikee

Copy link
Copy Markdown
Member Author

Re-review of head 200d811: the umbrella reference is corrected and shutdown-wide release tracking fixes the prior missing-released report, but one blocker remains.

The 2s final drain does not wait for the provider attempt that timed out at the per-session layer. finalizeDaemonSessionLease() returns after 1s while its release()/retryPending() remains in flight with retrying === true. drain() then calls retryPending(), which immediately returns because a retry is already running, so the daemon snapshots pending and advances toward exit without using the intended 2s drain window. A release that would finish during that window is therefore missed and can be cut off by shutdown. Track/join the active retry promise (or active provider attempts), race that shared completion against the drain timeout, and only then snapshot released/pending.

Add a regression where the provider resolves after the 1s per-session timeout but before the 2s drain deadline, preferably through the daemon shutdown route; the current permanently-hung helper test encodes immediate pending and cannot catch this gap.

CI is still in progress. Live hosted-provider shutdown evidence is still required before merge readiness.

@thymikee

Copy link
Copy Markdown
Member Author

The latest head fixes joining one in-flight release, but shutdown still does not attempt every active allocation. retryPendingLiveLeases() awaits leases serially, while all later release() calls join the same activeRetry. If session A's provider release hangs, session B is added to pendingLiveLeases but its provider release is never invoked before the session timeouts and final 2s drain expire—even when B would release immediately. That violates #1320 Stage 0's requirement to explicitly finalize every active allocation. Please make release attempts independently bounded/concurrent (or otherwise ensure one hung provider call cannot starve later leases) and add a startDaemonRuntime().shutdown() regression with two provider-backed sessions: A hangs permanently, B resolves, and B must be invoked and recorded within the shutdown budget. Current CI is green; live hosted-provider shutdown evidence is also still required after the code fix.

@thymikee

Copy link
Copy Markdown
Member Author

Live device validation completed against the freshly built branch:

  • iOS — iPhone 17 Pro simulator: opened Settings, captured a healthy XCTest snapshot, then daemon stop --clean completed gracefully with cleanupConfidence: known, no warnings, and no pending releases.
  • Android — Pixel 9 Pro XL API 37 emulator: opened Settings, captured a snapshot through android-helper, then the same clean stop completed gracefully with known cleanup and no warnings.
  • iOS retained-runner branch: after a normal close, the daemon-owned XCTest runner remained alive as expected. daemon stop --clean removed that runner and cleared the daemon metadata.

The local sessions had no hosted provider allocations, so this is live coverage for the local iOS/Android lifecycle and clean-up paths; hosted-provider lease finalization remains covered by the automated tests.

@thymikee
thymikee marked this pull request as ready for review July 17, 2026 12:29
@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 17, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Fixed the remaining shutdown starvation blocker on head 54c0e6d59. Provider release attempts are now deduplicated per lease instead of sharing one global retry promise, so one permanently hung provider call remains bounded/pending without preventing later allocations from being invoked, released, and reported. Added a two-session fake-timer regression that leaves lease A hung, proves lease B is still called and released, and verifies the final drain reports A pending / B released; it fails on the prior implementation. Independent post-push review is clean. Fresh format, typecheck, lint, focused tests (7/7), build, layering, and Fallow passed; the broad local suite's unrelated host-load/sandbox flakes pass in isolation, and current PR CI is in progress with early gates green. Code review is now clean.

@thymikee
thymikee force-pushed the agent/daemon-stop-stage0 branch from 54c0e6d to 630e0bb Compare July 17, 2026 16:11
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto current origin/main (12500ddc75). The conflict was reconciled by preserving main’s recording-aware shutdown teardown budget while keeping provider lease finalization before session deletion. New head: 630e0bb65.

Local validation passed: format, typecheck, lint, Fallow, and 19 focused daemon shutdown/lease/CLI tests. The branch is mergeable; CI has restarted on the rebased head.

@thymikee
thymikee merged commit d022799 into main Jul 17, 2026
24 checks passed
@thymikee
thymikee deleted the agent/daemon-stop-stage0 branch July 17, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant