[AI-1707] Make daemon shutdown crash-free: idempotent dispose + contained teardown

[realtonyyoung]

Problem

Every daemon shutdown on 0.11.10 ends in SIGABRT: ObjectDisposedException from ServerConnection.DisposeAsync surfacing through DI host teardown into the NativeAOT unhandled path. Four crash reports on 2026-08-02 alone; each abort strands every disposable ordered after ServerConnection in the DI walk and stretched supervisor relaunch gaps to ~2 minutes (windows of no_daemon_available flow failures). Same defect class as AI-1624 (#419) — this is the unswept sibling: #419 is an ancestor of v0.11.10 but was scoped to LocalPermissionBridge only.

Root cause

DaemonRunner disposes the same ServerConnection singleton twice by construction — an explicit connection.DisposeAsync() in the finally, then DisposeHostAsync(host) walking the container's tracked disposables. DisposeAsync had no run-once guard, disposed _terminalSenderCts without nulling it, and the second pass cancelled the disposed linked CTS. Nothing on the teardown path catches, so the throw aborts the process.

Fix (per the reviewed spec/plan on AI-1707)

• ServerConnection.DisposeAsync: Interlocked run-once guard; per-resource containment for faultable awaits; mandatory release of CTS/HttpClient/hub in finally, each step individually guarded; ODE-guarded cancel.
• DaemonRunner: teardown steps run through a new RunTeardownAsync coordinator — a throw in any step is logged with the step name and later steps still run, order pinned by test (daemon-lock → orchestrator → server-connection → host-stop → host-dispose). DI-walk limitation + the double-teardown sweep rationale documented in code.
• AgentOrchestrator.DisposeAsync (sweep item): same run-once + containment treatment, and its _shutdownCts — previously never disposed at all — is now disposed in a guaranteed finally after all token-dependent cleanup (stays readonly).
• Bonus defect found by the SIGTERM E2E: SIGTERM during the initial ConnectAsync retry loop let TaskCanceledException escape RunAsync → a second, pre-existing shutdown SIGABRT. Now caught when ApplicationStopping has fired (unrelated cancellations still propagate).

Tests

New durable-contract suites (not just no-throw): body-ran-once counters, CTS ends cancelled+disposed, faulting-await containment with later-cleanup assertion, the exact production shape (explicit dispose → host stop → host dispose over a container-tracked instance), order-pinning + throw-at-each-position coordinator tests, and the up-front vacuous-test check on AgentOrchestrator required by the spec review. Targeted classes all green (ServerConnectionDisposeTests 5/5, DaemonHostDisposalTests 8/8, ConnectWithRetryTests 7/7, LocalPermissionBridgeTests 46/46, AgentOrchestratorVendorTests 166/166). A pre-directive full unit run showed zero new failures vs a detached-origin/main baseline (47 ⊆ 50 known pre-existing macOS failures). Full suites delegated to CI per Tony. AOT publish (Release, osx-arm64, both csprojs) clean.

E2E on published binaries with isolated config dir: SIGTERM → Cleanup: completed, daemon exiting, no Unhandled exception banner, no new .ips. (Note for reviewers: kcap daemon stop SIGKILLs the tree, so it never exercises dispose — direct SIGTERM does.)

Spec + plan + codex spec-review findings: https://linear.app/kurrent/issue/AI-1707

🤖 Generated with Claude Code

[linear-code]

AI-1707

[qodo-code-review]

PR Summary by Qodo

Make daemon shutdown crash-free with idempotent disposals and resilient teardown

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Make ServerConnection and AgentOrchestrator DisposeAsync run-once and non-throwing under
 double-dispose.
• Add a teardown coordinator so shutdown steps always run, with step-level failure logging.
• Add regression tests for production-shaped DI re-dispose, teardown ordering, and cancellation
 during startup.

Diagram

graph TD
  DR["DaemonRunner (finally)"] --> RT["RunTeardownAsync"] --> DL["DaemonLock.Dispose"] --> AO["AgentOrchestrator.DisposeAsync"] --> SC["ServerConnection.DisposeAsync"] --> HS["Host.StopAsync"] --> HD["Host.DisposeAsync"] --> DI["DI dispose walk"]
  DI --> AO
  DI --> SC

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Remove explicit singleton disposals; rely only on host disposal

• ➕ Avoids structural double-dispose entirely
• ➕ Less bespoke shutdown code in DaemonRunner
• ➖ May lose the intentionally pinned teardown order (explicit dispose before host stop) relied on for deterministic retirement/cleanup
• ➖ Still requires every tracked singleton disposal to be non-throwing to prevent partial DI disposal/hangs

2. Stop DI from tracking these singletons (custom factory/ownership)

• ➕ Eliminates the DI re-dispose pass, reducing surface area for teardown bugs
• ➖ Non-idiomatic in Microsoft.Extensions.DependencyInjection; increases lifecycle complexity and risk of leaks
• ➖ Requires careful auditing of all registrations/hosted-service patterns to preserve stop/dispose semantics

3. Global catch around the finally cleanup block

• ➕ Prevents NativeAOT aborts from teardown exceptions with minimal refactoring
• ➖ Does not ensure later teardown steps run after an early failure unless steps are individually contained
• ➖ Can mask resource leaks (e.g., host not disposed) and still leave foreground threads alive

Recommendation: Keep the PR’s approach: a step-level teardown coordinator plus run-once, non-throwing disposals on DI-tracked singletons. This directly addresses the root cause (structural double-dispose + uncontained teardown exceptions) while preserving the required shutdown ordering and ensuring host disposal still occurs even when individual cleanup steps fail.

Files changed (6) +538 / -60

Bug fix (3) +265 / -60

DaemonRunner.csContain startup cancellations and coordinate shutdown teardown steps +80/-5

Contain startup cancellations and coordinate shutdown teardown steps

• Swallows OperationCanceledException during startup when ApplicationStopping is requested to prevent NativeAOT aborts on cooperative shutdown. Replaces direct finally teardown calls with RunTeardownAsync, ensuring each named step is logged/contained and later steps still run, without changing exit codes on cleanup failures.

src/Capacitor.Cli.Daemon/DaemonRunner.cs

AgentOrchestrator.csMake AgentOrchestrator.DisposeAsync idempotent and failure-contained +99/-38

Make AgentOrchestrator.DisposeAsync idempotent and failure-contained

• Adds an Interlocked run-once guard and durable test seams (body-run counter, shutdown CTS exposure). Refactors DisposeAsync into contained sub-steps with mandatory cleanup in a finally, and disposes the shutdown CTS after token-dependent teardown completes.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

ServerConnection.csMake ServerConnection.DisposeAsync run-once with per-step containment and mandatory release +86/-17

Make ServerConnection.DisposeAsync run-once with per-step containment and mandatory release

• Adds an Interlocked run-once guard plus test seams (body-run counter, CTS exposure, injectable awaited task). Wraps faultable awaits and cancellation in per-step try/catch logging, and guarantees CTS/HttpClient/hub release in a guarded finally to prevent DI-teardown crashes under double-dispose.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

Tests (3) +273 / -0

AgentOrchestratorDisposeTests.csAdd idempotent-dispose contract tests for AgentOrchestrator +63/-0

Add idempotent-dispose contract tests for AgentOrchestrator

• Introduces tests that assert DisposeAsync can be invoked twice (explicit + DI walk) without throwing, does not re-enter the body, and leaves the shutdown CTS cancelled and disposed. Uses durable assertions to prevent vacuous pass conditions.

test/Capacitor.Cli.Tests.Unit/AgentOrchestratorDisposeTests.cs

DaemonHostDisposalTests.csTest teardown coordinator ordering and containment behavior +76/-0

Test teardown coordinator ordering and containment behavior

• Extends existing host-disposal regression suite with tests for DaemonRunner.RunTeardownAsync. Pins the production step order and verifies a failure in any position is logged and does not prevent later steps from executing.

test/Capacitor.Cli.Tests.Unit/Daemon/DaemonHostDisposalTests.cs

ServerConnectionDisposeTests.csAdd production-shaped regression tests for ServerConnection double-dispose and containment +134/-0

Add production-shaped regression tests for ServerConnection double-dispose and containment

• Adds a focused suite verifying DisposeAsync is idempotent across explicit and DI-driven re-dispose, cancels/disposes the terminal-sender CTS, and logs/contains failures from awaited pipeline tasks while still performing later cleanup. Includes an integration-style test reproducing the real host/DI disposal shape.

test/Capacitor.Cli.Tests.Unit/Daemon/ServerConnectionDisposeTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Orchestrator cancel not contained ✓ Resolved 🐞 Bug ☼ Reliability

Description

AgentOrchestrator.DisposeAsync only catches ObjectDisposedException around
_shutdownCts.CancelAsync(); any other CancelAsync failure can escape DisposeAsync and skip the
subsequent agent termination/cleanup loops while the run-once guard prevents a retry.

Code

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[R3107-3110]

            try {
-                await agent.ReadCts.CancelAsync();
-                await agent.Runtime.TerminateAsync(TimeSpan.FromSeconds(5));
-            } catch {
-                /* best-effort */
+                await _shutdownCts.CancelAsync();
+            } catch (ObjectDisposedException) {
+                // Already torn down elsewhere — nothing left to cancel.

Evidence

The dispose method explicitly states faultable awaits must be contained, yet
_shutdownCts.CancelAsync() is only ODE-guarded, so other exceptions would escape and skip later
cleanup. The same class already treats other CancelAsync calls as best-effort (broad catch),
indicating cancellation is considered faultable on teardown paths.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[3089-3142]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[1862-1865]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`AgentOrchestrator.DisposeAsync()` intends to be non-throwing, but `_shutdownCts.CancelAsync()` is only guarded for `ObjectDisposedException`. If `CancelAsync` faults for any other reason, the exception escapes the method and prevents the later agent-teardown loops from running; because `_disposeOnce` is already set, DI’s second dispose pass cannot re-enter to finish cleanup.

### Issue Context
Other `CancelAsync` sites in the orchestrator are treated as best-effort and fully contained, but the top-level shutdown CTS cancellation is not.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[3094-3142]

### Suggested fix
Wrap `_shutdownCts.CancelAsync()` in a broad `catch (Exception ex)` (keeping the ODE comment if desired), log via `LogDisposeStepFailed(ex, "shutdown-cancel")`, and continue so the rest of teardown (lane drain + agent termination + CleanupAgentAsync loop) still executes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Host dispose failure may hang ✓ Resolved 🐞 Bug ☼ Reliability

Description

RunTeardownAsync swallows exceptions from the host-dispose step; if DisposeHostAsync throws partway
through the DI dispose walk, later disposables (including UnixSpawnerThread) may not be disposed,
and its non-background thread can keep the process alive after shutdown completes.

Code

src/Capacitor.Cli.Daemon/DaemonRunner.cs[R629-632]

+                await action();
+            } catch (Exception ex) {
+                allSucceeded = false;
+                LogTeardownStepFailed(logger, ex, name);

Evidence

RunTeardownAsync catches and continues for any step failure, and it is used to invoke
DisposeHostAsync(host) as the final teardown step. UnixSpawnerThread clearly starts a
non-background thread and documents that Dispose() is what retires it; if host disposal aborts
before disposing it, the process may not terminate.

src/Capacitor.Cli.Daemon/DaemonRunner.cs[567-573]
src/Capacitor.Cli.Daemon/DaemonRunner.cs[623-637]
src/Capacitor.Cli.Daemon/Pty/Unix/UnixSpawnerThread.cs[27-43]
src/Capacitor.Cli.Daemon/Pty/Unix/UnixSpawnerThread.cs[103-114]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DaemonRunner.RunTeardownAsync` intentionally contains step exceptions. However, if the `host-dispose` step throws (e.g., a DI-tracked singleton throws during disposal), the DI disposal walk can abort early and skip later disposables. Because `UnixSpawnerThread` runs a foreground (`IsBackground=false`) thread that is retired in `Dispose()`, a partial/aborted host disposal can leave that thread alive and prevent process exit.

### Issue Context
The PR comments already note the DI-walk limitation (a throwing disposable aborts the internal walk). Containing the `host-dispose` exception prevents a NativeAOT abort, but can trade it for a shutdown hang if the foreground thread wasn’t disposed.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/DaemonRunner.cs[567-580]
- src/Capacitor.Cli.Daemon/DaemonRunner.cs[623-637]

### Suggested fix
Treat `host-dispose` as a special case:
- Option A: If the `host-dispose` step fails, log and then force termination with a clean exit (e.g., `Environment.Exit(0)`), to avoid a foreground-thread hang.
- Option B: Add an explicit, guarded teardown step to retire critical foreground-thread resources (e.g., make `UnixSpawnerThread.Dispose()` idempotent and dispose it explicitly before `host-dispose`), so even a partial DI walk can’t strand the process.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[realtonyyoung]

NO FINDINGS