fix(desktop): let the launch-owner guard retire the owned Host on quit

[me2seeks]

Closes #4730. Review context: #3709 comment confirming the guard-detach root cause and the corrected scope after #4709.

Summary

• Stop releasing the candidate launch barrier in RuntimeHostDesktopManager.#close(). Releasing detached the launch-owner guard at the exact moment it was the close authority, so the guard ignored the IPC disconnect that would otherwise have closed the Host — an owned ephemeral Host could survive a full quit.
• Quit no longer drives retirement. prepareRuntimeHostQuit only probes Host activity to feed the interruption-consent dialog; the guard closes the owned Host after the Desktop process exits. The synchronous retirement drive, waitForProcessExit PID polling, and the force-terminate recovery path are removed from the quit flow.
• The Host reports upgradeBlockingActivity in host.diagnostics.query (required field; the epoch gate refuses mixed-version peers, so there is no wire case where it is absent) so the consent question stays answered by the same authority that gates host.upgrade.prepare. The predicate takes a selfCommands argument because host.upgrade.prepare counts itself as an in-flight command while the diagnostics query does not.
• A guard-triggered close now records the retirement reason, so launcher loss exits 0 instead of looking like a crash.

Protocol epoch moves 119 → 120 for the additive diagnostics key (rebased; the number tracks whatever main has taken at merge time): older Clients reject the unknown key on exact decode, so the pair refuses at the handshake.

Why

#4730's guaranteed-timeout symptom was neutralized by #4709's observation margin, but the root cause was untouched: #close() still released the barrier unconditionally. The driven retirement #3706 added becomes a transitional wrapper; this PR deletes it from the quit path while keeping retireOwnedLocalHost for the Desktop update-install and managed-service setup flows, which genuinely need a driven retirement.

Behavior change

• Quit with a clear owned Host: no retirement drive, no exit wait; the guard closes the Host after process exit.
• Quit with active work: unchanged consent dialog (copy now also promises durable recovery); on confirm, quit proceeds and the Host drains via the guard with its own shutdown deadline as the bound.
• Probe failure (wedged Host) no longer blocks quit — the guard plus the Host-side deadline bound the outcome, so the retry/force/cancel recovery dialog is removed.

Verification

• npm run build
• npm --workspace @maka/desktop run typecheck
• npx biome check on all changed files
• node --test apps/desktop/dist/main/__tests__/*.test.js (2092 passed)
• node --test packages/runtime-host/dist/__tests__/*.test.js (1677 passed, 12 platform skips)

AI use

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

Tool(s) and scope: Maka assisted investigation, implementation, tests, and review; the commit includes the required trailer.

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 — quit no longer drives or waits for Host retirement; the launch-owner guard is the close authority.
• No

[Astro-Han]

Reviewed current head f98e93ace880bd392608dbc3bad7b00fc3d1c4e3 (OPEN, MERGEABLE). No P0–P2; one P3 below. Gate stays NO-GO on red hosted test, which fails on an untouched spec.

P3 — the PR body understates the protocol epoch bump

The body says protocol epoch 109 → 110, but the exact diff is packages/runtime-host/src/protocol/index.ts:104 going 112 → 113. Please correct the description so reviewers checking compatibility against the stated numbers do not look at the wrong range.

Gate note (not attributed to this change)

Hosted test fails on the untouched apps/desktop/e2e/quote-selection.spec.ts:22,79-81; windows_recovery passes. Until that red is re-run or waived by maintainers, the gate cannot close either way.

What was checked on this head

Exit cleanup, launch-owner ownership, host resource reclamation, concurrent/reentrant and failure-recovery paths. What I could not judge: real Windows quit-path behavior with a live owned host was not reproduced locally.

---

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

简体中文

本条结论全部来自 @Luna-Deep-Qronos 的审查。我自己没有读这份 diff；我核的是当前 head 有没有漂移、以及 exact-head 的 CI 状态。当前 head 是 f98e93a，可合并。P3 是正文写的 epoch 数字跟实际 diff 对不上，改一下描述。另外线上 test 红在没动过的用例上，门禁等维护者处理。

[me2seeks]

Rebased onto current main (851407f). The two hosted test failures on the previous head were both Desktop transcript e2e specs untouched by this change (quote-selection, then transcript-scroll-cost on the re-run); main has since landed #4782 (dev renderer dep-optimizer generation) and #4831 (scroll assertion fix), so this run carries those. Epoch stays 112 → 113. P3 from the review (stated epoch numbers) was fixed in the body earlier.

[Astro-Han]

Re-reviewed at 851407f against main (a5a99a633), +245/−381 over 13 files, CI green at this head. The branch conflicts with main on one file only, packages/runtime-host/src/protocol/index.ts: the epoch constant and the ledger under it. main is at 117 and the head still declares 113, so merging as is would lower the epoch. Rebase, declare 118, move your entry to the top of the ledger; everything else auto-merges.

The fix is at the owner and is a net removal. #close() on main releases the candidate launch barrier unconditionally (runtime-host-desktop-manager.ts:977), which flips the guard to released and closes its lease, so the Desktop's disconnect no longer retires the owned ephemeral Host, which is #4730. Dropping that release lets the guard do its job; the force-quit / retry dialog and forceTerminateOwnedLocalHost were the mechanism that compensated, and they go in the same change with the #3709 discussion linked. I followed guard close → close({ reason: 'retirement' }) → shutdownReason → exit code 0, and the reopen-after-quit election with draining retrying a fresh candidate under the 45s deadline. The previous P3 (body said 109 → 110) is fixed; the earlier red gates were untouched transcript e2e and are green after the rebase.

P3:

• The quit probe reads #hasUpgradeBlockingActivity() from host.diagnostics.query, a mode: 'query' call, but the predicate's #activeCommandOperations > 1 was calibrated for host.upgrade.prepare, a command that counts itself. From the query path one in-flight command from another client reads as idle and quit does not ask. Give the predicate a selfCommands: 0 | 1 argument and pass 0 from the diagnostics path (host-kernel.ts:696,881).
• During a reconnect target.lifecycle?.current is undefined and the probe returns not_owned (runtime-host-desktop-manager.ts:773-776), so quit does not ask even if the Host is busy; the old quiesce() covered that window with a timeout. If that is the accepted trade, say so in the probe's comment.
• host-status.ts:87 says a missing field means unknown, not idle; the only reader treats missing as clear. With the epoch bump a mixed-version handshake cannot happen, so pick one: drop the optionality or fix the comment.
• forceTerminateRegisteredRuntimeHost and its WithDependencies variant (client/registered-host-termination.ts:85, exported from client/index.ts:49) have no caller left outside their own test once forceTerminateOwnedLocalHost is gone. Delete them with it, or say why they stay.

Evidence boundary: static read; not run on a real Electron quit, so the timing of an owned candidate that is no longer unref'd at release, and the reopen election duration, are unverified.

AI-assisted review: drafted with Maka; I verified the conflicting file, the epoch values and the orphaned export myself.

[me2seeks]

Rebased and review addressed in eb6b7c26 (epoch 119 → 120; main took 114–119 while this sat).

• selfCommands: #hasUpgradeBlockingActivity(selfCommands: 0 | 1); host.upgrade.prepare passes 1 (it counts itself as an in-flight command), the diagnostics query passes 0. One in-flight command from another client now correctly reads as blocking on the quit probe.
• Reconnect gap: accepted and documented at the probe — lifecycle.current is undefined mid-reconnect, so a busy Host reads as not_owned and quit does not ask; the guard still closes the owned Host truthfully after exit, settling or interrupting its work under the retirement contract.
• Field optionality: dropped. upgradeBlockingActivity is required in HostDiagnosticsResult; the epoch gate already refuses mixed-version peers, so the optional decode path was dead weight. The earlier comment was the stale half.
• Orphaned export: forceTerminateRegisteredRuntimeHost and its WithDependencies variant are deleted (client/registered-host-termination.ts, client/index.ts), along with their test. The observed-identity variant from fix(desktop): stop stale ephemeral Host at startup #4781 is untouched.

Local: full build; desktop main suite 2304 pass; runtime-host kernel 73 pass; protocol suites pass.

[Astro-Han]

Reviewed exact head eb6b7c261a12bf2fb0a24c12b8abfa2399225123.

Result: I found no new P0–P2 correctness, permission, concurrency, protocol, or Host-lifecycle issue in the production path. I verified:

• packages/runtime-host/src/server/host-kernel.ts:693-728,879-889 keeps the diagnostics self-command count separate from the host.upgrade.prepare self-command count; the late-client gate and diagnostics report the same blocking fact.
• packages/runtime-host/src/protocol/host-status.ts:145-218 requires boolean upgradeBlockingActivity; packages/runtime-host/src/protocol/index.ts:104-109 advances the compatibility epoch from 119 to 120, and packages/runtime-host/src/__tests__/host-kernel.test.ts:2561-2599 verifies that the older epoch is rejected during handshake.
• apps/desktop/src/main/runtime-host-desktop-manager.ts:945-958 no longer releases the launch barrier during quit. packages/runtime-host/src/candidate-entry.ts:105-108 lets the launch-owner guard close the owned Host with the retirement reason, while the Host/process lifecycle supplies the shutdown deadline.
• The former owned force-termination implementation, exports, and call sites are absent.

Two P3 contract/documentation inconsistencies remain:

1. apps/desktop/src/main/__tests__/runtime-host-desktop-manager.test.ts:327-337 still constructs a fake status without upgradeBlockingActivity and treats the result as clear. That bypasses the production decoder, where the field is mandatory. Update the fixture to use a valid wire payload or explicitly test decoder rejection.
2. The PR description still says that upgradeBlockingActivity is optional and decoded as unknown when absent. That contradicts the mandatory field and epoch-120 handshake contract implemented by this head.

Hosted test and windows_recovery are successful; the merge-tree and whitespace checks are clean. I could not independently rerun local build/typecheck/dist suites because this worktree has no tsc or vitest, and I did not run a real Electron quit/guard-close-failure smoke test. This is a COMMENT, not an approval.

Automated review notice: This review was generated with AI assistance and cannot replace independent human review.

[me2seeks]

Both P3s addressed in d7e9726e:

• The probe harness now always returns a valid wire payload (the field is required), and the idle-case test uses an explicit upgradeBlockingActivity: false report. The missing-field case is covered where it can actually happen: decoder rejection in protocol.test.ts, and the probe's decode/transport failure path in probe failure never blocks quit.
• The PR description now matches the mandatory field and the epoch-120 handshake contract, and mentions the selfCommands calibration.

[jackwener]

Approving at exact head eb6b7c26, at @me2seeks's direction. No P0–P2; two P3s carried forward, one of which should still be corrected before this merges even though it does not block.

I handed this one over mid-review rather than duplicating the deep pass, so this approval rests on that published review plus what I verified myself, and I checked that it covers the two angles I had flagged as unfinished:

• The epoch bump refuses cleanly rather than failing obscurely. protocol/index.ts:104-109 advances 119 → 120, host-status.ts:145-218 makes upgradeBlockingActivity mandatory, and host-kernel.test.ts:2561-2599 asserts the older epoch is rejected during the handshake — so a mismatched pair refuses at the door instead of surfacing as something else later.
• Removing the force-terminate path does not leave the outcome unbounded. runtime-host-desktop-manager.ts:945-958 no longer releases the launch barrier, candidate-entry.ts:105-108 has the guard close the owned Host with a retirement reason, and the Host/process lifecycle supplies the deadline. The former owned force-termination implementation, its exports and its call sites are gone rather than left dormant.

What I verified before handing over still stands: the problem is a third-party report (#4730, opened by someone other than this PR's author) with the root cause confirmed structurally in #3709, and the fix lands on that root cause — #close() deliberately not releasing owned candidates is what keeps the launch-owner guard armed, with #ipcMain.close() then supplying the disconnect the guard acts on.

The two P3s, and why one of them is worth doing anyway

A fixture that bypasses the decoder. runtime-host-desktop-manager.test.ts:327-337 builds a status without upgradeBlockingActivity and treats the result as clear, which the production decoder would not accept now that the field is mandatory. Worth fixing as a contract assertion rather than as a fixture edit — either use a valid wire payload or assert the decoder rejects the invalid one. A fixture standing in for the decoder cannot show that the decoder agrees with it, and that is precisely the class of gap that lets a contract drift unnoticed.

The description contradicts the implementation. It still says upgradeBlockingActivity is optional and decoded as unknown when absent; at this head the field is mandatory and that is what the epoch-120 handshake enforces. This repository squash-merges, so the title and body become the commit record on main — a reader of git log would be told the opposite of what the code does. It is P3 in severity and still worth correcting before merge for that reason alone.

One path nobody exercised

No real Electron quit / guard-close-failure smoke was run — by me or by the deep pass. The reasoning that the guard plus the Host-side deadline bounds the outcome is sound on the code, and the unit coverage is real, but the end-to-end behaviour on a genuinely wedged Host remains argued rather than observed. Recording it so it is a known gap rather than an assumed pass.

简体中文

在 exact head eb6b7c26 上批准,依 @me2seeks 的指示。无 P0–P2;两条 P3 带走,其中一条虽不阻塞、但仍建议在合并前改掉。

这一单我中途交接出去、没有重复做深审,所以这条批准建立在那份已发布的评审加上我自己核过的部分之上;我确认过它覆盖了我先前标为未完成的那两个角度:

• epoch 抬升是干净地拒绝,而不是以别的形式怪异地失败。 protocol/index.ts:104-109 把 119 推到 120,host-status.ts:145-218 将 upgradeBlockingActivity 定为必填,而 host-kernel.test.ts:2561-2599 断言旧 epoch 在握手阶段即被拒绝 —— 版本不匹配的配对在门口就被挡住,而不是稍后以别的样子冒出来。
• 移除强制终止路径并没有让结果失去边界。 runtime-host-desktop-manager.ts:945-958 不再释放启动屏障,candidate-entry.ts:105-108 由 guard 带着退休原因关闭 owned Host,而 deadline 由 Host/进程生命周期提供。原先 owned 强制终止的实现、导出与调用点是被移除而不是留着不用。

我交接前核过的内容依然成立:问题是第三方报告(#4730 由本 PR 作者以外的人提出),根因在 #3709 中被结构性确认,而修复正落在该根因上 —— #close() 刻意不再释放 owned candidate,才让 launch-owner guard 保持 armed,随后的 #ipcMain.close() 提供 guard 据以动作的那个断开信号。

两条 P3,以及其中一条为何仍值得做

一个绕过解码器的夹具。 runtime-host-desktop-manager.test.ts:327-337 构造了不含 upgradeBlockingActivity 的状态并当作「无阻塞」处理,而在该字段已成必填的今天,生产解码器不会接受它。建议按合同断言来修,而不是改夹具 —— 要么用一份合法的 wire 载荷,要么直接断言解码器会拒绝非法载荷。夹具顶替解码器,证明不了解码器与它意见一致,而这正是让合同悄悄漂移的那一类缺口。

描述与实现相矛盾。 正文仍写着 upgradeBlockingActivity 是可选、缺失时解码为 unknown;而在本 head 上它是必填,epoch-120 的握手正是据此强制的。本仓库采用 squash 合并,所以标题与正文会成为 main 上的提交记录 —— 读 git log 的人会被告知与代码相反的事。就严重度而言它是 P3,但仅凭这一点就值得在合并前改掉。

一条没人练过的路径

没有跑过真实的 Electron quit / guard 关闭失败冒烟 —— 我没跑,深审那一遍也没跑。「guard 加 Host 侧 deadline 兜住结果」这个推理在代码上成立,单元覆盖也是真的,但在一个真正卡死的 Host 上的端到端行为,目前是被论证的、而不是被观察到的。 记录在此,使它成为已知缺口而不是默认通过。

---

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

[jackwener]

Correction to my approval: it says eb6b7c26, but GitHub attached it to d7e9726e — the head moved while I was posting. Everything in it was derived from eb6b7c26.

That matters here in the PR's favour, so it is worth stating rather than leaving: both P3s I carried forward are already resolved on d7e9726e, the head the approval actually binds to.

• The fixture no longer bypasses the decoder. runtime-host-desktop-manager.test.ts now always returns a valid wire payload — { pid: 42, upgradeBlockingActivity: options.upgradeBlockingActivity ?? false } — with a comment stating the reason: the field is required on the wire, so the harness mirrors the production decoder contract. The test's name changed with it, from "treats a missing activity field as clear" to "treats an idle activity report as clear", which is the more honest description of what it now proves. This is the contract-assertion shape rather than a fixture edit.
• The description no longer contradicts the implementation. The "optional; decoded as unknown when absent" wording is gone, so the squash-merge commit record will not tell a git log reader the opposite of what the code does.

The one gap I recorded stands unchanged, because nothing in this delta touches it: no real Electron quit / guard-close-failure smoke was run, by me or by the deep pass. The guard-plus-deadline reasoning holds on the code and the unit coverage is real, but end-to-end behaviour against a genuinely wedged Host is still argued rather than observed.

简体中文

对我那条批准的更正:正文写的是 eb6b7c26,但 GitHub 把它挂在了 d7e9726e —— 发布过程中 head 移动了。其中所有内容都是基于 eb6b7c26 得出的。

这一点在本 PR 上是朝有利方向的,所以值得说明而不是略过:我带走的两条 P3,在批准实际绑定的 d7e9726e 上都已解决。

• 夹具不再绕过解码器。 runtime-host-desktop-manager.test.ts 现在总是返回合法的 wire 载荷 —— { pid: 42, upgradeBlockingActivity: options.upgradeBlockingActivity ?? false } —— 并附注明理由:该字段在 wire 上是必填的,所以 harness 与生产解码器合同保持一致。测试名也随之从「把缺失的活动字段当作无阻塞」改为「把空闲的活动报告当作无阻塞」,那才是它现在真正证明的东西。 这是合同断言的形状,而不是改夹具。
• 描述不再与实现矛盾。 「可选;缺失时解码为 unknown」的措辞已经去掉,所以 squash 合并后的提交记录不会告诉读 git log 的人与代码相反的事。

我记录的那一处缺口维持不变,因为本次增量没有触及它:没有跑过真实的 Electron quit / guard 关闭失败冒烟 —— 我没跑,深审那一遍也没跑。guard 加 deadline 的推理在代码上成立、单元覆盖也是真的,但面对一个真正卡死的 Host 的端到端行为,目前仍是被论证的、而不是被观察到的。

---

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