fix(runtime-host): await initial connection cleanup on close

[chihumyum]

Summary

Closing Desktop during its first Host connection could finish before acquisition and cancellation cleanup. A late ready candidate could then fail IPC registration after the router closed, leaving the candidate unclosed and publishing stale failure state.

Make the shared reconnect lifecycle own and join initial acquisition, including late resource cleanup. Register tasks before callbacks can reenter, and complete closed through one teardown path. Desktop closes candidates rejected before handoff and checks target generation ownership before activation or state publication. Explicit shutdown cancellation no longer reports a fatal startup failure.

Verification

• Added behavior regressions first ran red: 6 failures across lifecycle/manager cases, with 2 passing controls. Final source-built connection lifecycle suite: 31 passed; Desktop manager suite: 63 passed. Coverage includes late ready resources, delayed abort cleanup, synchronous close reentry, repeated start, unstarted initial resources, and permanent failure.
• Full Runtime Host suite: 2,234 passed, 12 skipped, 0 failed. Full Desktop suite: 3,258 passed, 0 failed. Tests use temporary roots and controlled/local candidate fixtures; model catalog DNS was blocked for the final Runtime Host run.
• An initial run under an extra macOS network sandbox caused 3 platform failures involving native sandboxing and ps spawning (EPERM). Those cases passed in a controlled rerun without the outer sandbox, followed by the passing full Runtime Host run above. No product changes were made to suppress them.
• Passed root build, lint, format check, typecheck; Desktop/UI Knip; strict renderer architecture; Astryx inventory/tests/theme; Windows inventory; ASF headers; model metadata; protocol epoch and staged diff checks.
• Real Desktop quit, SSH, and Windows execution were not exercised; manager fixtures are not native UI acceptance.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Codex, including GPT-6 Astra design/review subagents, contributed investigation, implementation, behavior tests, validation, and this description. The commit includes Generated-by: Codex. Final review and merge remain with human maintainers.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Full review of 51961a91 (one commit; +398/-35, 4 files). This PR hasn't been reviewed before, and it doesn't link an issue.

What changed.

• RuntimeHostReconnectLifecycle now owns the initial acquisition as #startTask, registered before any callback can re-enter. close() joins it, alongside the reconnect task and discard chain. close() also closes an unstarted initial resource.
• #failPermanently routes through close(), so closed resolves only after the acquisition tasks settle.
• In Desktop, the ready branch of connect checks abort and target ownership before completeRegistration(), and closes a candidate rejected before handoff.
• A new #ownsTarget guard (manager not closed, and the generation is still the mapped one) gates #markUnavailable, onFatalError, #activate, and #publishState.

Correctness and races. I walked through the re-entrancy cases:

• close() from a listener during #install. The task is already registered, #close awaits #startTask, and #start returns right after #install, so there is no cycle.
• Permanent failure inside #start. #failPermanently → void close() → #close awaits #startTask, which settles when #start rethrows. The same holds inside #reconnect. No deadlock in either case.
• Late resource after abort. #install sees #closed and queues #discard, and #close awaits #discardTask only after joining the start and reconnect tasks. The ordering is correct.
• Errors after close(). waitForCurrent still prefers #terminalError over the generic closed error, so callers keep the real failure.
• Desktop. #removeTarget deletes the map entry before lifecycle.close(). The late #markUnavailable / onFatalError from a cancelled start is therefore suppressed, while observations are still closed by #removeTarget. This matches the intent that explicit shutdown no longer publishes a fatal startup failure. The per-attempt IPC handlers belong to the candidate, so candidate.close() on the new throw path releases them.

Shutdown ordering and hangs. Manager close() now waits for an in-flight initial connect, where it previously returned without joining it. disposeRuntimeHostDesktop (runtime-host-boot.ts:2117) has no timeout around runtimeHostManager.close(). For local targets, connectOrSpawnRuntimeHost receives the signal plus connect and handshake timeouts, and the reconnect path already relied on abort-aware connects. Residual risk remains in createDesktopRuntimeHostCandidate's post-handshake RPCs, which take no signal. See the inline note (P3).

Findings:

• P3: reconnect-lifecycle.ts:317. close() now has no upper bound on the initial-acquisition join, and Desktop quit has no outer timeout.
• P3 (tests): runtime-host-desktop-manager.ts:1532, and likewise :757 and :1447. The new #ownsTarget guards are only exercised with the manager closed. Nothing covers a replaced generation while the manager is still open (retryLocalStart, enable replacing an existing profile, disable): a stale fatal error, a stale #activate, or a stale state publication. #activate now returns silently instead of activating or throwing. That is fine for the current callers, but a regression there would be invisible.

Local verification. I built at the head in a temporary worktree:

• reconnecting-connection.test.js passes 31/31, and Desktop runtime-host-desktop-manager.test.js passes 63/63.
• With reconnect-lifecycle.ts swapped back to main, 5 of the new lifecycle tests fail: caller and re-entrant close, abort cleanup, unstarted initial, and repeated start. The permanent-failure case passes as a control. This is consistent with the PR body's "6 failures across lifecycle/manager".

Protocol: not touched, so no epoch change is needed.

Status:

• Mergeability: MERGEABLE (merge state BLOCKED, review required). The head is on current main 3597abe8; git merge-tree and git diff --check are clean.
• CI: hosted test was still in progress at review time (label passes). This review does not reflect its result.

I found no blocking issues. This review is not merge approval.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review 51961a91 → e197faf5 (one commit: e197faf58, tests only). The merge-base is still 3597abe8 (current main).

Prior findings

• P3, unbounded initial-connection join in close() (reconnect-lifecycle.ts:317): not changed, and the reason given is accepted. I checked the author's argument. Liveness probing starts when the connection is constructed after the handshake (connection.ts:475, timeouts of 8 s local and 45 s peer). So the post-handshake RPCs under startCandidate reject when the Host is wedged. A bounded join here would let closed resolve before the start settles. A quit-time bound in disposeRuntimeHostDesktop is a reasonable follow-up.
• P3 (tests), stale-generation guards only exercised with the manager closed: partly fixed.
  • The new test (runtime-host-desktop-manager.test.ts:1962) has retryLocalStart replace a failed Local generation while that generation's #markUnavailable is still pending and the manager stays open.
  • It asserts that the late unavailable state is not published, that the old epoch is inactive and not owned, and that the replacement stays ready. Per the author, it fails without the #publishState guard.
  • Still uncovered: a replaced generation's late ready result reaching #activate (:1532), and replacement through enable or disable of a remote profile. This doesn't block.

New code: test only, and I found no issues.

Protocol: not touched, so no epoch change is needed.

Status: MERGEABLE (merge state BLOCKED, review required). git merge-tree and git diff --check are clean. At review time, hosted CI test on e197faf5 was still in progress (run 37709625046), so this review does not reflect its result. I did not run the suites locally. This review is not merge approval.

[chihumyum]

On the remaining guard coverage: with the manager open, a replaced generation's late ready reaching #activate (:1532), and replacement through enable/disable of a remote profile, are not reachable through the public API today. Each replacement awaits the old lifecycle's close, which now joins its initial start, and replacements of one profile are serialized. The unserialized initial Local start keeps its generation valid until it settles, so retryLocalStart is a no-op while it runs. The added test covers the one open-manager overlap that is reachable: a failed generation's pending #markUnavailable. Removing any of the other early returns (:757, :1192, :1447, :1532) one at a time keeps the suite green, because they overlap with #publishState, signal.throwIfAborted() and the lifecycle's #closed checks. They are defense in depth rather than separately reachable paths.

🤖 Addressed by Claude Code