fix(desktop): stop Runtime Host after launcher loss - #4114
Conversation
Bind Desktop-spawned ephemeral Runtime Hosts to the launcher's IPC lifecycle so an abrupt Electron exit cannot leave an orphan blocking the next launch. Preserve the reusable lifetime of generic detached launches. Generated-by: OpenAI Codex
0550a9b to
07018fd
Compare
jackwener
left a comment
There was a problem hiding this comment.
I found one blocking issue at 07018fd3336200f71f97a50410ceb723c6341655.
[P1] The Desktop launch path bypasses the new launcher-lifetime guard
Desktop always creates and passes candidateLaunchBarrier from
runtime-host-boot.ts, so local startup takes the branch at
runtime-host-desktop-candidate.ts:309-310. The barrier's production dependency
then launches with launchOwnedRuntimeHostCandidate, not
launchDetachedRuntimeHostCandidate.
The new closeOnLauncherExit: true value reaches that owned launcher, but
launcher.ts:103-108 still enables its IPC guard only when
inheritableAuthorityLeaseFd is present. A normal Desktop launch has no such
lease descriptor. As a result, the child receives neither an IPC channel nor
MAKA_RUNTIME_HOST_LAUNCH_OWNER_GUARD, and killing the Electron main process
still leaves the Runtime Host alive.
I reproduced this with the real candidate process by switching the new fixture
from the detached launcher to the owned launcher used by the production
barrier: after killing the launcher, the Host connection remained open and the
new five-second assertion failed with launcher-owned detached Host survived its launcher. Extending the owned launcher's guard condition to
inheritableAuthorityLeaseFd !== undefined || closeOnLauncherExit === true
made the same probe pass while preserving the authority-supervised control.
Please make the owned/barrier launch path honor closeOnLauncherExit and run
the regression through that path. The current Host-kernel test calls the
detached launcher directly, while the Desktop barrier test only asserts the
forwarded root path, so both pass without exercising the production launcher.
The current branch is directly based on 2c066316545e432459e327734c634dd2cb61fa40;
the merge result is clean. The exact-head hosted test, package, and
windows_recovery checks passed. I also ran the full affected build, the full
Runtime Host suite (1,346 passed, 9 skipped), and 54 focused Desktop tests. I did
not approve or merge this head because the reported orphaning behavior remains
reachable.
Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。
Summary
Bind Desktop-spawned ephemeral Runtime Hosts to the Desktop launcher lifecycle. If the Electron main process exits abruptly, loss of the inherited IPC channel now closes the owned Host instead of leaving it to block a later launch. Generic detached Runtime Hosts keep their existing reusable lifetime.
Fixes #4111
Verification
2c066316:node --test --test-name-pattern="launcher-owned detached Host" packages/runtime-host/dist/__tests__/host-kernel.test.js— failed because the Host connection stayed open after launcher exit.npm --workspace @maka/runtime-host test— 1,346 passed, 9 skipped.npm --workspace @maka/desktop run build:main— passed.node --test apps/desktop/dist/main/__tests__/runtime-host-desktop-candidate.test.js apps/desktop/dist/main/__tests__/runtime-host-desktop-manager.test.js— 54 passed.npm run lint— passed.npm run format:check— passed.npm --workspace @maka/desktop run build:workspace-depsreached the unchanged@maka/uiworkspace and failed on existing Astryx API type mismatches (settledText,conversationKey,unlockAutoFollow, andtrailingAction); the affected Runtime Host and Desktop main-process builds above pass.AI use
Select exactly one:
Tool(s) and scope: OpenAI Codex assisted with implementation and regression-test drafting; I reviewed the design, diff, and verification.
Checklist
Does this PR entail a change in behavior?