fix(desktop): wait for idle worker exit before restart (#3330)

[nothingyuancando]

这次改了什么

摘要

修复按需 Node worker 空闲回收后的同 key 重启竞态：worker 从业务表移除后，只有在旧 UtilityProcess 真实触发 exit 后，才允许下一次请求 fork 替代进程；若旧进程在有界时间内未退出，返回停止失败而不双开。

变更类型

• fix 缺陷修复
• feat 新功能
• refactor / perf 重构或性能优化
• docs / test / chore 文档、测试或工程维护
• 其他：

范围

• 关联 Issue / 需求：bug(desktop): Windows 按需插件 Node worker 空闲回收后二次启动超时 #3330
• 本 PR 包含：按 worker key 保存停止退出屏障；下一次启动等待真实退出或返回有界错误；延迟退出回归测试。
• 明确不包含：插件业务代码、启动超时重试策略、全局 pi-package-home 或其它 worker 类型。
• 用户可见变化：按需插件空闲回收后再次调用时，不会因旧进程尚未退出而启动冲突进程并卡在 bootstrap；若进程无法停止，会收到明确启动失败。
• 是否存在 breaking change：无

UI 变化

不涉及 UI。

• 引用的设计规范：不涉及。

怎么验证的

自动验证

pnpm --filter desktop exec vitest run src/main/cindy-brain/__tests__/nodeRuntimeBroker.test.ts
结果：55 个测试全部通过

pnpm --filter desktop exec tsc --noEmit
结果：TypeScript 检查通过

回归覆盖“首次请求 → 空闲停止 → 旧进程延迟退出 → 第二次请求”，并断言退出前不 fork 同 key 新进程。

手工验证

不涉及；真实 Windows packaged utilityProcess.fork 验证留给 CI/发布验收。

未执行的验证

未在 Windows packaged 应用中执行现场复现；当前使用进程假体覆盖延迟退出、强杀屏障和同 key 重启时序。

风险

风险分类

• 无已知风险
• SQLite / migration
• system prompt
• 协议兼容
• 权限 / 安全 / 用户数据
• 存量插件兼容（批准状态 / 指纹 / manifest 校验 / 安装布局 / 包格式）
• 原生层 / fingerprint / OTA
• 跨平台差异
• 其他：

影响与回滚

• 影响范围：仅影响按需 Node worker 停止与同 key 后续启动的时序；resident worker、插件协议和正常复用路径不变。
• 回滚 / 降级方式：回滚本 PR 即恢复原有停止后立即允许重启的行为；无数据迁移。停止超时仍沿用现有有界等待和 SIGKILL 兜底。

提交前检查

• 已 review 完整 diff
• 每个 commit 都带 DCO 签名（git commit -s，见 DCO）
• UI 改动已在「UI 变化」注明引用的设计规范章节（不涉及 UI 则跳过）
• 未提交凭证、令牌或授权文件
• 已补充必要文档
• 已确认测试结果或说明未执行原因

[greptile-apps]

Greptile Summary

本 PR 为按需 Node worker 增加按 key 的退出屏障，确保空闲回收后的替代进程仅在旧 UtilityProcess 实际退出后启动。

• 停止 worker 时保存有界的进程退出 Promise，并在真实 exit 后清理屏障
• 同 key 的后续启动会共享并等待该屏障，超时则返回明确的启动失败
• 新增延迟退出、并发重启及超时后恢复的生命周期回归测试

Confidence Score: 5/5

当前变更看起来可以安全合并。

未发现仍然存在的阻塞性失败。

Important Files Changed

Filename	Overview
apps/desktop/src/main/cindy-brain/nodeRuntimeBroker.ts	在停止与重新启动之间加入按 worker key 管理的有界退出屏障，并保留现有单飞启动机制。
apps/desktop/src/main/cindy-brain/tests/nodeRuntimeBroker.test.ts	新增旧进程延迟退出、并发请求共享屏障及超时后恢复启动的回归覆盖。

Sequence Diagram

sequenceDiagram
    participant R as 后续请求
    participant B as Node Runtime Broker
    participant O as 旧 UtilityProcess
    participant N as 新 UtilityProcess
    B->>B: 从 workers 移除旧 worker
    B->>B: 注册同 key 退出屏障
    B->>O: SIGTERM
    R->>B: 请求同 key worker
    B->>B: 等待退出屏障
    O-->>B: exit
    B->>B: 清理退出屏障
    B->>N: fork 替代进程
    N-->>B: spawn / ready
    B-->>R: 返回请求结果

Loading

_{Reviews (2): Last reviewed commit: "test(desktop): harden node worker exit b..." | Re-trigger Greptile}

[MagicLizi]

这个 PR 改了插件 worker 空闲回收后的重启时序，属于插件基座，已进入维护者确认（基座改动不因 bugfix 豁免）。

请维护者在本 PR 上 Approve；需要修改请 Request Changes。讨论 issue：#3351

[Battleplus]

Nice implementation — cleaner than my #3343 approach of using a custom drainingExits map. Reusing waitForProcessExit is the right call.

I did an adversarial review and found 3 test gaps worth considering:

1. Concurrent requests during barrier wait

The most critical scenario the current test doesn't cover: two requests arrive while the old process is still exiting.

Request A → ensureWorker → await stopping (barrier pending)
Request B → ensureWorker → await stopping (same barrier)

Currently the test only verifies a single request waits for exit. But with two concurrent requests, both should share the same barrier and both should resolve to a single startingWorkers entry after the process exits. Worth a targeted test.

2. Barrier timeout → eventual recovery

If the barrier rejects (process didn't exit within grace period), the next request gets WorkerStartError. But the process might eventually exit. At that point stoppingWorkers.delete(key) fires from the exit listener. The test should verify:

• barrier rejects → request fails with clear error
• old process eventually exits → stoppingWorkers entry cleaned up
• subsequent request → can start a new process normally

This proves the system recovers after a timeout failure, not just fails permanently.

3. void barrier.catch(() => undefined) swallows rejection silently

The comment says this is intentional to avoid unhandled rejection, but it means the rejection reason is lost until the next request awaits the barrier. If the rejection contains useful diagnostic info (e.g. "process X did not exit within 2500ms"), it might be worth logging it at the point of rejection for observability, even if the promise itself is suppressed.

Minor point — the current behavior is correct, just a suggestion for debuggability.

---

These are not blockers — the core design is sound and the single-request delayed exit test is the right primary regression. Just ideas for hardening.

[Battleplus]

CI update: Linux unit tests (2/2) failed in makerChatStoreActiveView.test.ts with ReferenceError: window is not defined in makerTransport.ts:236.

This test is not related to your changes — it's a renderer-side test that doesn't touch nodeRuntimeBroker.ts. My #3343 (same issue, different implementation) passes all 11/11 including this shard. The failure looks like a shard-specific or flaky test environment issue.

You can verify by re-running the failed job, or pushing an empty commit to trigger a fresh CI run.