fix(desktop): connect the loopback services concurrently - #5074
Conversation
Service startup was a method that mutated the shared `ServiceSet` as it went, which only works while the connections run one after another. Move the per-service work into `connect_service` and have it report a `ServiceOutcome` instead: the child this App spawned, whether a stale listener was replaced, and the result. Ownership now travels with the outcome in both directions. A connection that spawned a process and then failed still hands that child back, so `ServiceSet::collect` stops it exactly as the previous shared-state loop did rather than leaking it. Behaviour is unchanged: services are still connected in order, progress is still published before each one, and the first error still stops the set. The diff is mostly the de-indentation of the moved body. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The App connected the status service and then the chat service, so the window waited for their sum. The two own separate ports, commands and processes and neither reads the other's readiness, so nothing required that order. A start that follows a runtime update pays the difference twice over: each stale listener is replaced and then warms a fresh interpreter before it answers a readiness probe. Connect them together and wait for the slowest instead. On this host the status service reached readiness in 2.17s and chat in 2.99s from cold, so a start that took their sum now takes the larger of the two. The `connecting` phase can no longer name one service while both are in flight, so progress now reports the services still being waited on: the loopback set while they run together, then whichever connection outlives its peer. The boot page already rendered an unnamed service as "正在连接 本地服务" and keeps naming a single pending service, so a stalled connection stays diagnosable. Concurrency is asserted as a contract rather than a timing coincidence: every connection must observe its peer in flight, and a sequential implementation fails the bounded peer wait instead of hanging. The failure path is covered too, since splitting the shared loop is exactly what could strand a spawned child without an owner. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed head: 8b000a0a8e383416a80aae15cae58c45347cca1f; base: 3e443ad7c285973c40be883293970be6c0c51e06. Whole-PR review under policy revision 11, including both commits. 未发现阻塞性问题。
动机
旧 ServiceSet::start 串行连接 status 再连接 chat，两个独立端口的冷启动/替换等待会相加。目标是让同一桌面窗口更快到达真实的双服务 ready，而不是缩短超时、隐藏错误或新增 daemon。
反对直接并行的最强理由是生命周期：status 失败时，chat 可能已生成子进程；如果失败仅返回错误而丢失 child owner，后续 supervisor 重试会留下孤儿、重复服务或误杀借用服务。我通过真实 HTTP 与进程组回放检查这一反例，不把 injected connect helper 的通过当作完整证明。
改动思路
沿用已有 Desktop 服务所有者，没有新 runtime、任务状态或对外协议。active caller 是 maintenance::start_services，仍先经过 runtime 配对和 supervision gate，再进入 ServiceSet::start。两条 scoped thread 各执行原有连接/身份验证；全部 outcome 汇合后才允许发布 ready。
并行是明确的默认行为变化：原先首个失败会短路，现在已启动的另一个连接仍按已有截止时间完成，然后一起清理。这是接受的有界代价：两个被请求的服务分别拥有原有 port/runtime 检查，已有 15 秒阶段 deadline 保留，错误仍按 status→chat 固定优先级返回，失败后自动重试归原 supervisor，不以 fail-fast 为由遗失 ownership。没有降低 runtime 配对或 foreign-process 拒绝边界。
具体改动
- services.rs:136 的
ServiceSet::start接入并行连接；connect_all:193 用 typedServiceKind数组及 mutex 派生剩余集合，先报 local，随后报告仍在等待的单个服务，不提前 ready。 - ServiceOutcome / collect:143 携带 child、healed 和 result；全部 ownership 被接收，即使某个结果失败也会停止本 App 创建的进程组，借用/launchd 所有者不被冒充为 App-owned。
- connect_service:232 是原 ensure 路径的提取：匹配复用、registry unreadable 的完整修复提示、foreign 拒绝、stale PID 二次验证、launchd 单所有者、spawn/probe deadline 均保留。
- maintenance.rs 的 callback 改为 pending slice →
pending_label→ 原有 connecting snapshot。未修改的static/boot.js:22已有 local/status/chat 消费文案，不需要新配置 editor、CLI 或 Lark companion。 - services_tests.rs 新增并行重叠、pending label、失败收集后子进程清理；lib.rs 固定既有启动页三种文案。整 PR 4 文件 +318/-103：production 主体在 services.rs（+191/-101）、maintenance（+3/-2）；其余 124 行为测试，不是新持久状态。
正向路径：runtime 已配对 → 两服务连接重叠 → local/剩余服务进度 → matching 双成功 → Maintenance ready → 工作区。负向路径：status 的 registry readiness 失败且 chat 也 foreign → 两结果汇合 → status 的完整 remediation 保留 → owned child/descendant 清理 → service_error → 已有重试流程恢复。
对主干的风险
已执行的本地验证：
| 验证 | base | exact head / 结论 |
|---|---|---|
准备 exact-source runtime bundle 后 cargo test --locked |
69 passed，2 ignored；签名包测试另 1 ignored | 72 passed，2 ignored；签名包测试另 1 ignored |
真实 ServiceSet::start 冷启动、已有服务复用、失败清理、失败后重试、双拒绝优先级、stale 替换 |
6 场景通过；冷启动无重叠 | 同样 6 场景通过；两 child 在任一个 ready 前均已启动 |
| Drop / failure 进程组 | child 与合成 descendant 已退出；借用 listener 保留 | 同样通过，无 orphan；失败返回前已清理 |
| 故意改回串行的 mutation | 历史顺序被观察到 | 同 public entrypoint 的 overlap oracle 失败（exit 1） |
| 故意丢弃 outcome child 的 mutation | — | 同 public entrypoint 的 drained oracle 失败（exit 1） |
| 启动页实际资产的浏览器回读 | consumer 代码未变 | local/status/chat、service_error 重试诊断、ready 均正确显示 |
cargo fmt --check |
exit 1，maintenance.rs 六处既有格式差异 | exit 1，归一化后六处逐字同一签名；非本 PR 回归 |
| diff / public-boundary | — | diff check 通过；定向扫描无错误 |
真实进程回放仅替换 transport 端口和未加载的 fixture launchd label，其余生产连接、HTTP probe、spawn、PID 检查、收集与 Drop 路径不变；fixture 使用匹配 release identity 的合成服务和真实子进程/descendant。没有碰正在运行的用户服务，没有执行安装、真实 Goal 写入或签名发布。immutable fixture SHA-256 为 4ade03d24b63452d2275f84f03f2e25a4fb356d1cfe38ecbbd62bfa211b2b5f7；base/head 观察指纹分别为 ad19ca20d28c6d81578781bd359a537a4302a0d5048e8bdeb3fded17c1c3cf75 / e54b2dbca23fdeaae259d86964c5bdd1b741dd4f4ad800afe0d5f2f00956cbe5，差异是已披露的 overlap、进度以及失败时完成 peer attempt。
浏览器通过可控 IPC snapshot 验证未变的真实 boot.js 消费面，不用它证明 native ownership；native ownership 来自上面的进程回放。未测试 signed/notarized 安装包或 Windows/Linux，既有 ignored 测试不能算通过，也不把这次局部 native 验证包装成跨平台发布资格。初次 Cargo 构建缺 runtime-source.tar.gz 是 checkout 准备缺口，补齐 source bundle 后已恢复。六处格式失败的同一签名为 c66bc12947784cd2bf1060f76d6b56425eb8de81064920671fb52f362f6d716d，与改动不相交，不强迫本 PR 做无关格式修复。远端 CI 未查询。
我的整体评价
APPROVE。 本次交付的是完整的“双服务并行且 ownership 不丢失”阶段，长期 retry 不增加 orphan，错误和 runtime 配对恢复仍可用；不是仅以启动变快或 helper 测试为依据。与已合入 #4166 的进度投影、#4523 的配对阶段保留契约一致。
Future-facing pass 已体现在同域提取：per-service 决策保留一个 owner，汇合只管 transient ownership/结果，没有复制连接规则或增加第二份 registry truth。无需更大的 scheduler/async 框架。非阻塞建议：把 reviewer 运行的真实 ServiceSet::start 生命周期/descendant 场景固化进仓库回归，补强目前 helper-oriented 的新增测试。
cargo clippy --all-targets --locked -- -D warnings、desktop icon 和 release-workflow 两项本地 smoke 均通过。APPROVE 是评审结论，不是 merge-readiness 或自合并授权。
English verdict: APPROVE - 8b000a0. The existing per-service authority is retained while connection attempts overlap; matched real-process baseline/head replay verifies failure cleanup, borrowed-service preservation and deterministic diagnostics. Both semantic mutations fail their independent oracles; pre-existing formatting failures do not block this review.
1 participant
Problem
The App connects the status service and then the chat service, so the boot
window waits for their sum. The two own separate ports, commands and
processes, and neither reads the other's readiness, so nothing required that
order.
A start that follows a runtime update pays the difference twice over: both
listeners are stale, so each one is replaced and then warms a fresh
interpreter before it answers a readiness probe.
Measured on one macOS host, from cold to a passing readiness probe:
serve-status(8766)chat(8767)The probes themselves are not the cost (4–10ms for status, 12–92ms for chat),
and the recovery-diagnostics environment probe is already cached behind a 30s
TTL. The wait is the sum of two interpreter warm-ups.
Change
Connect both services together and wait for the slowest, so a start that took
their sum now takes the larger of the two.
The
connectingphase can no longer name one service while both are inflight, so progress reports the services still being waited on: the loopback
set while they run together, then whichever connection outlives its peer. The
boot page already rendered an unnamed service as
正在连接本地服务and stillnames a single pending service, so a stalled connection stays diagnosable.
Commits
Split so the mechanical move is reviewable apart from the behaviour change:
refactor(desktop): give each service connection its own outcome—behaviour-preserving. Per-service work moves out of a method that mutated
the shared
ServiceSetintoconnect_service, reporting aServiceOutcome. Still sequential; the pre-existing suite passes unchangedat this commit. Most of the diff is de-indentation, so
git diff -wis theshorter read.
fix(desktop): connect the loopback services concurrently— the actualbehaviour change, the pending-service phase label, and the tests.
Ownership, the risk this refactor carries
Splitting a shared-state loop into independent tasks is exactly what could
strand a spawned child with no owner: previously a failed service was still
stopped because it had already been pushed onto
ServiceSet. Ownership nowtravels with every outcome in both directions, and
a_failed_service_set_stops_the_child_its_peer_startedspawns a real processand asserts it does not outlive the failed set.
Validation
cargo testinapps/desktop/loopx-control-plane/src-tauri: 72 passed, 0failed, 2 ignored (they require a signed build and isolated installer
paths). The first commit passes the pre-existing 69 on its own.
coincidence: every connection must observe its peer in flight. Reverting
connect_allto a sequential fan-out makesservice_connections_run_concurrently_and_name_the_remaining_servicefailwith
[(Status, false), (Chat, true)], bounded by a 10s peer wait ratherthan hanging the suite. Confirmed by temporarily applying that regression.
states (
local, thenchat). Layout, mark, progress bar and the recoverydisclosure are unchanged; only the status line differs.
cargo fmt --checkreports the same six pre-existingfindings before and after. A repo-wide
cargo fmtreformats unrelated codeunder this toolchain, so it was reverted and the changed lines were matched
to the surrounding style by hand;
maintenance.rsis a five-line change.Scope
ServiceSet::starthas one caller (maintenance.rs) anddetails.servicehas one consumer (
boot.js), both updated here.Not included, and deliberately: navigating to the workspace as soon as chat is
ready. The workspace is served by chat on 8767, and chat is the slower of the
two, so once the services run concurrently it is already the critical path —
that change would buy no time while adding a state where the workspace is open
after the status service failed.
The runtime-install phase that dominates a post-update first start is
unchanged; it was optimised separately in #4166.