feat(mesh): add hidden host session launcher - #11225
Conversation
|
Thanks for the PR! Template looks good ✓ — every required heading is filled in, including the Risk & Scope bullets and the Chinese translation. Problem: this is planned work rather than a bug claim, so there is no reproduction to ask for. It is stacked step 4 of #11206, and the design and acceptance documents it updates are already committed on the foundation branch. The prerequisite it names — one reusable runtime owner per workspace before a dispatcher can bind runs to background agents — is a real ordering constraint, not a hypothetical. Direction: aligned. Multi-agent mesh is a maintainer-driven initiative with committed design docs under Size: core paths are touched — this spans Approach: scope feels right and genuinely minimal. Every edit earns its place: the new host module, the private launch route, the catalog and count exclusions, and the exports that tie them together. Two things I'd flag as not scope creep but easy to misread — the I raised two scope-level questions on the first pass, and both were answered by force-pushed follow-ups while the review was running: Risk: elevated on two axes, so review depth is increased rather than skipped:
Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必填小标题都填了，包括 Risk & Scope 和中文翻译。 问题： 这是计划内的工作，不是 bug 报告，所以不需要复现。它是 #11206 的 stacked step 4，其更新的设计文档与验收文档已经提交在 foundation 分支上。它提出的前置条件——dispatcher 把 run 绑定到后台 agent 之前，每个工作区需要一个可复用的 runtime owner——是真实的顺序约束，不是假设。 方向： 对齐。多 agent mesh 是维护者主导、 规模： 触及核心路径——横跨 方案： 范围合理且确实最小化。每处改动都有必要：新的 host 模块、私有启动路由、catalog 与计数排除，以及把它们串起来的导出。有两点想说明——它们不是范围蔓延，但容易被误读： 我在第一轮提了两个规模层面的疑问，两个都在审查过程中被 force-push 的后续提交回答了： 风险： 两个维度都有升级，因此加深审查而不是跳过：
进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Reviewed at head Code reviewI wrote my independent proposal before opening the diff: a workspace-scoped host module that claims under the existing mesh lock, reuses a stored session, launches through the typed launcher, keeps alive by reusing the scheduled-task resume primitive with a deadline and a single-flight guard, and excludes All four findings from the first pass are now resolved. I re-checked each against the current head rather than taking the commit messages at face value: 1. Count/list asymmetry — fixed. 2. Post-paging filter — fixed. The outer 3. Stale-host recovery — now explicitly scheduled and gated. This was my "nothing ever clears a stale 4. String-literal coupling — fixed. The constant moved to a new single-purpose module Things I checked that came out clean, since the missing CI means nobody else will:
I have no remaining code-level findings. What I cannot give you is execution evidence — see Testing. sequenceDiagram
participant P1 as Caller (step 6 dispatcher)
participant P2 as startMeshHostSessionOwner
participant P3 as mesh-store
participant P4 as AcpSessionBridge
participant P5 as acpAgent child
P1->>P2: launch(agent, prompt)
P2->>P3: readMeshWorkspace(cwd)
P3-->>P2: hostSessionId or empty
alt host already claimed
P2->>P4: recordHeartbeat(hostSessionId)
P4-->>P2: throws when reaped
P2->>P4: resumeSession under deadline (single flight)
else no host yet
P2->>P4: spawnOrAttach(sourceType mesh)
P2->>P3: claimMeshHostSession under workspace lock
P3-->>P2: winner id (loser closes its own session)
end
P2->>P4: launchMeshAgent(sessionId, agentId, prompt)
P4->>P5: ext method session mesh-agent launch
P5-->>P4: started, capacity_wait, agent_unavailable, or launch_failed
P4-->>P2: typed result
P2-->>P1: typed result
Files changed (16)
TestingThis PR has no test signal from its own CI, and that is structural rather than a flake. So the numbers in the description — core 218, host/keepalive 33, bridge 914, ACP child 623, and the 4.3 ms reload after a 20 ms reap — are the author's local claims on macOS, not verified evidence. They also predate both force-pushes, so they do not account for the pagination or totals tests either. The PR labels them honestly, marking Windows and Linux as pending #11206's whole-branch CI, and the acceptance doc repeats the same caveat. I am not disputing them; I am noting that nothing in this thread independently confirms them, and the gate that would (the #11206 whole-branch run) has not happened. Not verified: any test, lint, typecheck, or build result — no CI lane covers this base branch, so "it compiles and the suite passes" is an inference from reading, not an observation. Not verified: Windows and Linux behaviour, per the author's own table. Not verified: every behavioural claim below, because the module has no production caller yet and its suite is the only thing that exercises it.
One consequence worth being explicit about, because it changes what "wait for green" would mean here: deferring approval until CI lands is not a usable remedy on this branch. The only CI that will ever complete for this PR is Sandboxed verification would settle this: 中文说明本次审查基于 head 代码审查我在打开 diff 之前先写了自己的独立方案：一个工作区级 host 模块，在现有 mesh 锁内完成认领，复用已存储的 session，通过类型化 launcher 启动，靠复用 scheduled-task 的 resume 原语（带 deadline 与单飞守卫）保活，并把 第一轮的四个发现现在都已解决。 我针对当前 head 逐条复核，而不是只看提交信息： 1. 计数/列表不对称——已修。 2. 分页后过滤——已修。 对 3. 失效 host 恢复——现已明确排期并设 gate。 这是我那条"没有任何地方清除失效 4. 字符串字面量耦合——已修。 常量搬到了新的单一职责模块 由于 CI 缺失、别人不会去查，我把查过且干净的几点列出来：
我没有剩余的代码层面发现。我给不出的是执行证据——见"测试"。 测试这个 PR 从自身 CI 拿不到任何测试信号，而且这是结构性的，不是 flake。 因此描述里的数字——core 218、host/keepalive 33、bridge 914、ACP child 623，以及 20 ms reap 后 4.3 ms reload——是作者在 macOS 上的本地声明，不是已验证的证据。它们也早于两次 force-push，因此同样没有把分页或计数测试计入。PR 对此标注得很诚实，把 Windows 和 Linux 标为等待 #11206 的整分支 CI，验收文档也重复了同样的 caveat。我不是在质疑这些数字；我是在说明本线程里没有任何东西独立确认过它们，而本该确认它们的那道门（#11206 整分支运行）还没有发生。 未验证：任何测试、lint、typecheck 或构建结果——没有 CI lane 覆盖这个基线分支，所以"它能编译、套件能通过"是阅读得出的推断，不是观测。未验证：Windows 与 Linux 行为，依据作者自己的表格。未验证：下面所有行为性声明，因为该模块尚无生产调用方，其测试套件是唯一会执行它的东西。 有一点值得说清楚，因为它改变了"等绿"在这里的含义：在这个分支上，把批准推迟到 CI 落地并不是一个可用的补救。本 PR 唯一会完成的 CI 是 沙箱化验证可以定这件事： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Addressed three of the four stage-2 findings:
Verified at the current head with |
|
Confidence: 3/5 — the code review came out clean and I have no remaining findings; the cap is evidentiary, not doubt about quality. Nothing in CI compiles or runs this change, so approving would attest to a build and test result that does not exist. Stepping back: this is a well-built stacked step, and it beats the baseline I wrote before opening the diff. I had proposed reusing the scheduled-task resume primitive; the PR went further and extracted What I'd most credit is how the review landed. The head moved twice while I was reading it, and both pushes were direct answers to findings rather than unrelated churn: the pagination fix moved the exclusion ahead of paging in all five catalog paths, and the totals fix closed the count/list asymmetry and collapsed six scattered So why am I still not approving? There is no CI evidence to approve against, and on this branch there never will be. And the PR's own declared gate has not run. The description and the acceptance doc both name #11206's whole-branch CI as the bar for this step. #11206 is still open against I am deliberately not requesting changes — there is nothing left to request. This is a defer on evidence, not on merit. ⏸️ Deferring to @qqqys — you own the 中文说明Confidence: 3/5 —— 代码审查结果是干净的，我没有剩余发现；这个封顶是证据层面的，不是对质量的怀疑。CI 里没有任何东西会编译或运行这次改动，所以批准将等于为一个并不存在的构建与测试结果背书。 退一步看：这是一个构建良好的 stacked step，并且优于我在打开 diff 之前写下的基线方案。我原本的设想是复用 scheduled-task 的 resume 原语；这个 PR 更进一步，抽出了 我最想肯定的是这轮审查的落地方式。审查过程中 head 移动了两次，而两次推送都是对发现的直接回应，不是无关的翻动：分页修复把排除移到了全部五条 catalog 路径的分页之前，计数修复关闭了计数/列表不对称，并把散落在六处的 那为什么我仍然不批准？ 没有可供批准所依据的 CI 证据，而且在这个分支上永远不会有。 而且 PR 自己声明的那道门还没有运行。 描述和验收文档都把 #11206 的整分支 CI 命名为本步骤的标准。#11206 对 我刻意不请求变更——已经没有什么可请求的了。这是一次基于证据的 defer，不是基于价值的否定。 ⏸️ 转交 @qqqys —— 你拥有本 PR 触及的 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — the harness's own test phase never ran because the packages/cli build gate failed first, so there is no harness testScope; the diff-covering suites were run by hand instead (all green) and the full packages/core suite's 75 failures were measured as pre-existing by re-running the same 11 files on the built merge base (empty net-new failing-file set).
Not reviewed: test-efficacy probe — harnessValidated: null, i.e. the positive control produced no verdict because no probe file was green in the unmutated baseline; 0 mutants and 0 hunk probes ran (2 mutants and 19 hunks skipped for cap, 3 and 6 for baseline), so no probe-kit survivor evidence backs the coverage findings — each of those was instead established by a verifier running the mutation by hand.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 3)": re-measuring the four core test counts the docs record (52 background-agent-resume , 150 background-tasks , 12 capability , 4 launcher = 218) — packages/c…; "agent reverse-audit (round 3)": whether a mesh agent launched into the clientless hidden host can raise a tool-permission request no client can answer — I read createMeshToolInvocationGuard …; "agent reverse-audit (round 3)": the launch RPC's timeout margin — requestSessionStatus defaults to initTimeoutMs ( bridge.ts:6588-6592 , DEFAULT_INIT_TIMEOUT_MS ≈ 10 s per the comment at…; "agent reverse-audit (round 1)": did not read the cold-task register(entry) path in background-tasks.ts to confirm how a duplicate agentId collides in the this.agents task map (only the…; "agent reverse-audit (round 1)": did not verify DEFAULT_QUEUE_LIMIT / whether dispatch-policy.ts would in practice hold a second run for the same agent in a queue rather than launching it c…, and 1 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：build-and-test — the harness's own test phase never ran because the packages/cli build gate failed first, so there is no harness testScope; the diff-covering suites were run by hand instead (all green) and the full packages/core suite's 75 failures were measured as pre-existing by re-running the same 11 files on the built merge base (empty net-new failing-file set).
未审查（原文为英文）：test-efficacy probe — harnessValidated: null, i.e. the positive control produced no verdict because no probe file was green in the unmutated baseline; 0 mutants and 0 hunk probes ran (2 mutants and 19 hunks skipped for cap, 3 and 6 for baseline), so no probe-kit survivor evidence backs the coverage findings — each of those was instead established by a verifier running the mutation by hand.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 3)"：re-measuring the four core test counts the docs record (52 background-agent-resume , 150 background-tasks , 12 capability , 4 launcher = 218) — packages/c…；"agent reverse-audit (round 3)"：whether a mesh agent launched into the clientless hidden host can raise a tool-permission request no client can answer — I read createMeshToolInvocationGuard …；"agent reverse-audit (round 3)"：the launch RPC's timeout margin — requestSessionStatus defaults to initTimeoutMs ( bridge.ts:6588-6592 , DEFAULT_INIT_TIMEOUT_MS ≈ 10 s per the comment at…；"agent reverse-audit (round 1)"：did not read the cold-task register(entry) path in background-tasks.ts to confirm how a duplicate agentId collides in the this.agents task map (only the…；"agent reverse-audit (round 1)"：did not verify DEFAULT_QUEUE_LIMIT / whether dispatch-policy.ts would in practice hold a second run for the same agent in a queue rather than launching it c…，另有 1 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| }); | ||
| resident = false; | ||
| const startedAt = performance.now(); | ||
| await owner.tick(); |
There was a problem hiding this comment.
[Suggestion] R1-20: The periodic keepalive — this module's stated purpose, and the driver of step-4 gate (d) — never fires in any test. All six owners are built with intervalMs: 60_000 (lines 113, 159, 164, 207, 329, 352), none installs fake timers, and every revive is pulled by an explicit owner.tick() or owner.launch(). So the setInterval body at mesh-host-session.ts:107-115 is never entered, and three separate guards inside it are unpinned. The sharpest consequence is the swallowed rejection: test 6 is itself proof that tick() rejects (rejects.toThrow(/timed out/)) whenever a resume misses its deadline, so removing .catch(() => {}) leaves that rejection unhandled inside a timer body — and Node's default --unhandled-rejections=throw then terminates the process, meaning one slow host revive would take down the whole serve daemon. With the 70 s default resume timeout at 2.33× the 30 s interval, overlapping ticks are the normal case, and nothing pins that they are dropped either.
Witness:
Three mutants, each leaving the suite fully green (Tests 6 passed (6)):
delete `.catch(() => {})` (mesh-host-session.ts:111)
delete `if (running) return;` (:108)
delete the entire setInterval registration + clearInterval(timer) (:106-116)
The Node consequence, measured rather than assumed -- repo floor is Node >=22 (.nvmrc, engines):
Error: tick failed 1
at Timeout._onTimeout (file:///tmp/probe-unhandled.mjs:4:23)
Node.js v22.23.2
EXIT=1 <- the `SURVIVED 500ms` line never printed
In-repo corroboration that the serve daemon installs no handler of its own (run-qwen-serve.ts:6582-6587):
"the serve fast path installs no process-level unhandledRejection handler before this runs, and
Node's default for one is to exit -- a failed chunk load ... would take down every runtime,
session, and connection the daemon serves."
Why the existing tests cannot see it: test 3 measures its reload INSIDE owner.launch() (:216-220)
and test 1 inside an explicit owner.tick() (:123) -- never on a timer edge.
Add a case that installs vi.useFakeTimers(), starts the owner with a small intervalMs/resumeTimeoutMs, lets the reaper (or a throwing recordHeartbeat mock) mark the host gone, advances past one interval and asserts resumeSession was called with no explicit tick()/launch(); then advance a second time while the first revive is still in flight and assert recordHeartbeat/resumeSession were entered once (the running guard), and that a rejecting tick does not escape the timer — a process.on('unhandledRejection') spy, or simply that the owner is still usable on the next tick.
Three constraints on that test: DEFAULT_MESH_RESUME_TIMEOUT_MS = 70_000 is more than double DEFAULT_MESH_KEEPALIVE_INTERVAL_MS = 30_000 (mesh-host-session.ts:17-18), so it must advance across an interval boundary while a tick is still in flight rather than assuming ticks are short; it must advance by the intervalMs it injects, not the 30 s default; and it must not depend on the timer holding the process open, because timer.unref?.() (:116) means a real-timer variant can never be observed after the test body returns. startMeshHostSessionOwner has no production caller yet, so the crash is latent until step 6 wires the owner — that is a severity consideration and the reason this stays a Suggestion, not a reason to leave the module's only production path untested.
Acceptance criterion: that test must go red when if (running) return; is removed (two overlapping resumes recorded), when .catch(() => {}) is removed (unhandled rejection observed), and when the setInterval registration is removed (no revive on clock advance). No existing test fails under any of the three.
中文说明
周期性 keepalive——本模块声明的目的、也是第 4 步 gate (d) 的驱动者——在任何测试中都不会触发。全部六个 owner 都以 intervalMs: 60_000 构建（113、159、164、207、329、352 行），没有一个安装假定时器，每次恢复都是由显式的 owner.tick() 或 owner.launch() 拉动的。所以 mesh-host-session.ts:107-115 的 setInterval 主体从未被进入，其中三道各自的守卫都没有被钉住。
最尖锐的后果是被吞掉的拒绝：测试 6 本身就证明了只要 resume 错过 deadline，tick() 就会拒绝（rejects.toThrow(/timed out/)），所以移除 .catch(() => {}) 会让该拒绝在定时器主体内无人处理——而 Node 默认的 --unhandled-rejections=throw 随后会终止进程，意味着一次缓慢的 host 恢复就会拖垮整个 serve daemon。由于默认 70 秒的 resume 超时是 30 秒周期的 2.33 倍，重叠的 tick 是常态，而这一点同样没有被钉住。
为什么现有测试看不见它：测试 3 在 owner.launch() 内部测量它的重载（:216-220），测试 1 在显式的 owner.tick() 内部测量（:123）——从来不在定时器边沿上。
修法：新增一个用例，安装 vi.useFakeTimers()，以较小的 intervalMs/resumeTimeoutMs 启动 owner，让 reaper（或一个会抛错的 recordHeartbeat mock）标记 host 已消失，推进超过一个周期并断言 resumeSession 在没有显式 tick()/launch() 的情况下被调用；然后在第一次恢复仍在进行时再推进一次，断言 recordHeartbeat/resumeSession 只被进入一次（running 守卫），并且一次拒绝的 tick 不会逃出定时器——用 process.on('unhandledRejection') spy，或简单地断言 owner 在下一个 tick 仍可用。
该测试的三个约束：DEFAULT_MESH_RESUME_TIMEOUT_MS = 70_000 是 DEFAULT_MESH_KEEPALIVE_INTERVAL_MS = 30_000 的两倍多（mesh-host-session.ts:17-18），所以它必须在某个 tick 仍在进行时跨越一个周期边界推进，而不是假设 tick 很短；它必须按自己注入的 intervalMs 推进，而不是 30 秒默认值；它也不能依赖定时器把进程挂住，因为 timer.unref?.()（:116）意味着真实定时器的变体在测试主体返回后永远无法被观测到。startMeshHostSessionOwner 目前还没有生产调用方，所以崩溃在第 6 步接线之前是潜伏的——这是严重性上的考量，也是它保持为 Suggestion 的原因，但不是让该模块唯一的生产路径不被测试的理由。
验收标准：当 if (running) return; 被移除（记录到两次重叠恢复）、当 .catch(() => {}) 被移除（观测到未处理拒绝）、以及当 setInterval 注册被移除（时钟推进时没有恢复）时，该测试都必须变红。三种情况下现有测试都不会失败。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| expect(spawnCount).toBe(1); | ||
| expect(resumes).toEqual(['mesh-host-1']); | ||
| expect(launches).toEqual(['mesh-host-1:ag_alice', 'mesh-host-1:ag_alice']); | ||
| expect(reloadMs).toBeLessThan(1_000); |
There was a problem hiding this comment.
[Suggestion] R1-34: Both wall-clock assertions in this file use the same literal (1_000) as the resumeTimeoutMs the tests were constructed with, but the window they measure is strictly wider than the window that timeout governs — so the bound is set by the code under test, and it fails on terms the deadline never polices. beginKeepaliveSessionResume wraps only bridge.resumeSession(request) in withTimeout (scheduled-task-keepalive.ts:118-131, :637-644). reloadMs here brackets owner.tick() → readMeshWorkspace plus ensure()'s own readMeshWorkspace — two real withWorkspaceLock acquisitions, each fs.mkdir + lockfile.lock with a ~655 ms retry budget (mesh-store.ts:46-53) — plus the resume; in test 3 it additionally brackets bridge.launchMeshAgent's full channel round-trip. Neither the store reads nor the launch RPC is inside withTimeout. Both directions were measured: a 900 ms regression in the exact branch these two test names advertise keeps both assertions green, while a 1,100 ms launch RPC with a perfectly healthy resume fails the assertion naming no slow subsystem.
Witness:
Baseline (5 runs, unmodified PR): test 1 reloadMs 1.5/1.4/1.4/1.5/1.9 ms; test 3 4.3/4.2/4.1/4.5/4.1 ms
(the acceptance doc's "4.3 ms" reproduces). Headroom to the 1,000 ms bound ~995 ms.
Direction 1 -- 900 ms delay inserted at mesh-host-session.ts:57, in the revive branch BEFORE
beginKeepaliveSessionResume (so outside withTimeout):
{"test":"1-revive","reloadMs":902.6} {"test":"3-reaper","reloadMs":904.4}
✓ owns one hidden session and revives it before the next launch 927ms
✓ reloads after the daemon bridge reaper closes the host 980ms
Tests 6 passed (6) <- a 900 ms regression, both assertions green
Direction 2 -- a 1,100 ms delay inside test 3's fake-child extMethodImpl for sessionMeshAgentLaunch
only, resume untouched and healthy:
AssertionError: expected 1105.4093300000004 to be less than 1000 (:239)
...while line 227's resolves.toMatchObject passed -- the deadline never fired.
Direction 2 is reported with one declared config change: the run raised this test's own sessionIdleTimeoutMs from the shipped 20 to 5_000, because at 20 ms the reaper kills the freshly-resumed channel mid-request. That affects only which term trips the assertion, not the structural point, and Direction 1 needed no config change at all.
Decouple the two numbers — resumeTimeoutMs: 5_000 with expect(reloadMs).toBeLessThan(200) — or drop the wall-clock assertions entirely: test 1 already pins spawnCount === 1 and resumes, test 3 already pins handles.length === 2 and handles[0].killed, so the speed property is the only thing they add and it is currently bounded by nothing.
A raised resumeTimeoutMs must stay at or below MAX_SESSION_RESTORE_TIMEOUT_MS = 2_147_483_647 (session-restore-timeout.ts:8), because withTimeout begins with if (ms > MAX_SESSION_RESTORE_TIMEOUT_MS) return p; (scheduled-task-keepalive.ts:638) and returns the raw non-abortable promise — above the ceiling there is no deadline at all, and test 6's two rejects.toThrow(/timed out/) assertions depend on one existing. 5_000 is far below it.
Acceptance criterion: with the bound decoupled, inserting await new Promise((r) => setTimeout(r, 400)) into ensure()'s revive branch (mesh-host-session.ts:58, right after recordHeartbeat throws) turns test 1 red; against the shipped toBeLessThan(1_000) the same delay leaves all 6 green — measured at 900 ms above.
中文说明
本文件中两处墙钟断言都使用了与测试构造时所用 resumeTimeoutMs 相同的字面量（1_000），但它们测量的窗口严格宽于该超时所管辖的窗口——于是这个界限是由被测代码设定的，而且它会在 deadline 从不监察的项上失败。beginKeepaliveSessionResume 只把 bridge.resumeSession(request) 包进 withTimeout（scheduled-task-keepalive.ts:118-131、:637-644）。这里的 reloadMs 括住的是 owner.tick() → readMeshWorkspace 加上 ensure() 自己的 readMeshWorkspace——两次真实的 withWorkspaceLock 获取，每次都是 fs.mkdir + lockfile.lock，重试预算约 655 ms（mesh-store.ts:46-53）——再加上 resume；在测试 3 中它还额外括住了 bridge.launchMeshAgent 的完整 channel 往返。store 读取和 launch RPC 都不在 withTimeout 之内。
两个方向都做了实测：在这两个测试名所宣称的那个分支里插入 900 ms 回归，两处断言仍然全绿；而一次 1,100 ms 的 launch RPC、resume 完全健康，却会让断言失败，并且指不出任何慢的子系统。
方向 2 的报告附带一处已声明的配置改动：该次运行把本测试自己的 sessionIdleTimeoutMs 从出厂的 20 提高到 5_000，因为在 20 ms 下 reaper 会在请求进行中杀掉刚恢复的 channel。这只会影响哪一项触发断言，不影响结构性结论；而方向 1 完全不需要配置改动。
修法：把两个数字解耦——resumeTimeoutMs: 5_000 配 expect(reloadMs).toBeLessThan(200)——或者干脆去掉墙钟断言：测试 1 已经钉住 spawnCount === 1 与 resumes，测试 3 已经钉住 handles.length === 2 与 handles[0].killed，所以这两处断言唯一新增的就是速度属性，而它目前没有任何界限。
约束：提高后的 resumeTimeoutMs 必须不高于 MAX_SESSION_RESTORE_TIMEOUT_MS = 2_147_483_647（session-restore-timeout.ts:8），因为 withTimeout 以 if (ms > MAX_SESSION_RESTORE_TIMEOUT_MS) return p; 开头（scheduled-task-keepalive.ts:638），会返回原始的、不可中止的 promise——超过该上限就完全没有 deadline，而测试 6 的两条 rejects.toThrow(/timed out/) 断言依赖于存在一个 deadline。5_000 远低于它。
验收标准：在界限解耦之后，向 ensure() 的恢复分支插入 await new Promise((r) => setTimeout(r, 400))（mesh-host-session.ts:58，紧跟 recordHeartbeat 抛错之后）会让测试 1 变红；而在出厂的 toBeLessThan(1_000) 下同样的延迟会让 6 个测试全绿——上文已在 900 ms 下实测。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| }); | ||
| let spawnCount = 0; | ||
| const bridge = { | ||
| recordHeartbeat: vi.fn(), |
There was a problem hiding this comment.
[Suggestion] R1-35: This is the suite's only concurrency test, and it gives both owners ONE shared bridge whose recordHeartbeat never throws and whose resumeSession is vi.fn() — so the loser's post-closeSession recursion is exercised only in its benign form, and the production shape the claim protocol exists for (two daemons, two bridges) is unreachable here. In production each daemon owns its bridge, so after closeSession the loser's return ensure() (mesh-host-session.ts:88-89) re-reads the winner's hostSessionId and heartbeats a session its own bridge never held, which throws SessionNotFoundError, and the bare catch turns that into a resumeSession of the winner's session in the loser's own bridge. This test cannot enter that branch, so ensure() returns the winner's id and expect(new Set(sessionIds).size).toBe(1) passes — and the stubs could not represent it if it were entered. The cost is that the green result on "two owners start concurrently" is evidence about a shared-bridge unit only: when that cross-bridge behaviour is fixed no test in this file goes red, and if it regresses none goes red either.
Witness:
(a) read-confirmed: test 2 uses `recordHeartbeat: vi.fn()` -- returns undefined, never throws -- so the
recursion re-enters ensure(), passes the heartbeat at :55 and returns at :77; the revive branch at
:56-76 is never entered. Both owners are handed the SAME bridge object (:156-166).
(b) the stubs could not represent the divergent path:
{"resumeSessionCalls":1,"outcome":{"status":"rejected","name":"TypeError",
"message":"Cannot read properties of undefined (reading 'finally')"}}
at beginKeepaliveSessionResume (scheduled-task-keepalive.ts:125) <- ensure (mesh-host-session.ts:58)
(c) the two-bridge probe, built here -- two real makeBridge instances over one workspace dir, one owner
each, both ensureResident() concurrently:
{"phase":"bridge-state","aSessions":["hostA:/tmp/.../workspace"],
"bSessions":["hostA:/tmp/.../workspace"],"bResumeExtCalls":[]}
Bridge B, which never spawned that session, ends with a LIVE in-memory entry for the winner's id,
and both owners report the same id -- so `new Set(sessionIds).size === 1` passes in the divergent
world exactly as in the healthy one.
Declaration: (c) models two daemons as two real bridges in ONE process; the divergent half
(recordHeartbeat/resumeSession/byId) is per-bridge in-memory state and does not cross a process
boundary, while the cross-process half (claimMeshHostSession's withWorkspaceLock) ran for real.
One correction to how this was first described: the first throw is not withTimeout's p.then — it is void started.completion.finally(...) at mesh-host-session.ts:69 throwing synchronously out of the catch; p.then produces a second, unhandled TypeError.
Construct one bridge per owner whose recordHeartbeat throws for any id that owner did not itself spawn and whose resumeSession returns a promise recording the id, then assert the intended post-condition — exactly one owner ends up driving a live host, and the loser never resumes a session it never owned — instead of only new Set(sessionIds).size === 1, which still passes on the shipped code since both owners return the winner's id. If the two-daemon shape is judged out of scope for step 4, the cheaper honest move is to rename the test so it no longer advertises concurrency coverage it does not have.
Any per-owner stub must return a promise from resumeSession, since bridgeTypes.ts:1523-1525 declares resumeSession(req: BridgeRestoreSessionRequest): Promise<BridgeRestoredSession> and mesh-host-session.ts:19-26 types the bridge as Pick<AcpSessionBridge, ...> — the same contract the 8 TS2322 build errors are raised against, so this fix must not add a ninth.
Acceptance criterion: the rewritten test must go red against mesh-host-session.ts:81-90 as shipped — the loser resumes the winner's session in its own bridge, which is what bSessions above records — and go green only once the claim is resolved before spawnOrAttach.
中文说明
这是套件中唯一的并发测试，而它给两个 owner 的是同一个共享 bridge，其 recordHeartbeat 从不抛错、其 resumeSession 是 vi.fn()——因此落败方在 closeSession 之后的递归只以良性形态被执行，而认领协议为之存在的生产形态（两个 daemon、两个 bridge）在这里不可达。
在生产中每个 daemon 拥有自己的 bridge，所以 closeSession 之后落败方的 return ensure()（mesh-host-session.ts:88-89）会重新读取胜者的 hostSessionId，并对一个自己的 bridge 从未持有过的 session 发心跳，这会抛 SessionNotFoundError，而那个裸 catch 会把它变成在落败方自己的 bridge 中对胜者 session 的一次 resumeSession。本测试无法进入该分支，所以 ensure() 返回胜者的 id，expect(new Set(sessionIds).size).toBe(1) 通过——而且即使进入了该分支，这些 stub 也无法表达它。
代价是：「两个 owner 并发启动」上的绿色结果只是关于一个共享 bridge 单元的证据；当那个跨 bridge 行为被修复时，本文件没有测试会变红，而当它回归时也没有测试会变红。
对最初描述的一处更正：第一次抛错不是 withTimeout 的 p.then——而是 mesh-host-session.ts:69 的 void started.completion.finally(...) 从 catch 中同步抛出；p.then 会产生第二个、未被处理的 TypeError。
修法：为每个 owner 构造一个自己的 bridge，其 recordHeartbeat 对任何不是该 owner 自己 spawn 的 id 抛错，其 resumeSession 返回一个记录该 id 的 promise；然后断言预期的后置条件——恰好一个 owner 最终驱动一个存活 host，且落败方绝不会恢复一个它从未拥有过的 session——而不只是 new Set(sessionIds).size === 1（在出厂代码上它仍然通过，因为两个 owner 都返回胜者的 id）。如果认为双 daemon 形态超出第 4 步范围，更省的诚实做法是重命名该测试，使它不再宣称自己并不具备的并发覆盖。
约束：任何按 owner 划分的 stub 都必须从 resumeSession 返回一个 promise，因为 bridgeTypes.ts:1523-1525 声明了 resumeSession(req: BridgeRestoreSessionRequest): Promise<BridgeRestoredSession>，而 mesh-host-session.ts:19-26 把 bridge 类型定为 Pick<AcpSessionBridge, ...>——正是那 8 个 TS2322 构建错误所针对的同一契约，所以本修复不能添第九个。
验收标准：改写后的测试必须针对出厂的 mesh-host-session.ts:81-90 变红——落败方在自己的 bridge 中恢复了胜者的 session，这正是上面 bSessions 所记录的——并且只有在认领于 spawnOrAttach 之前完成后才变绿。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| }), | ||
| ]; | ||
| try { | ||
| const sessionIds = await Promise.all( |
There was a problem hiding this comment.
[Suggestion] R1-28: No test ever issues two overlapping calls on ONE owner, so the single-flight ensuring ??= ensure() guard (mesh-host-session.ts:94-97) — the only thing stopping a concurrent launch burst from minting one host session per caller — is unpinned. This Promise.all spreads one call each across two owners, which have separate ensuring closures; tests 1, 3 and 6 await every call before starting the next. With the guard gone, two mesh agents dispatched in the same turn (or one 30 s tick() overlapping a launch()) both enter ensure(), both await readMeshWorkspace, both see hostSessionId === undefined, and both call spawnOrAttach — two real ACP children where one suffices. The loser is then closed by the claim race, which can itself throw and orphan the session; each transient child also occupies a bridge admission slot, and at the cap the second spawn throws SessionLimitExceededError (bridgeErrors.ts:224-226), so launch() rejects even though a good host was one await away. Nothing observable in the result betrays it — both callers still end on the same session id — only the spawn count does.
Witness:
INTACT: mesh-host-session.test.ts 6 passed
PROBE-R1-28 spawnOrAttach=1 closeSession=[] sessionIds=["mesh-host-1","mesh-host-1"]
MUT28 : mesh-host-session.test.ts 6 passed <- the guard is unpinned
PROBE-R1-28 spawnOrAttach=2 closeSession=[<loser>] sessionIds=["mesh-host-1","mesh-host-1"]
MUT28 = ensureResident reduced to `return ensure();` (dropping `ensuring ??=` / `return ensuring`,
mesh-host-session.ts:95-98). Probe: ONE owner, two overlapping launch() calls.
Add a test that builds ONE owner and calls Promise.all([owner.launch(AGENT, 'a'), owner.launch(AGENT, 'b')]) — or two ensureResident() calls — against a spawnOrAttach mock that hands out a distinct id per call, asserting spawnOrAttach was called exactly once and both results carry the same sessionId.
The new mock must satisfy MeshHostBridge = Pick<AcpSessionBridge, 'recordHeartbeat' | 'resumeSession' | 'spawnOrAttach' | 'closeSession' | 'launchMeshAgent'> (mesh-host-session.ts:20-27), because this file's existing mocks already fail that shape and break the build with 8 TS2322 — a new mock must not add a ninth. Please keep this separate from the reviving-reset and running-guard coverage gaps: they are three different lines, three different mutations and three different consequences (duplicate ACP children vs duplicate resumes vs overlapping ticks), and merging them would misstate the coverage, since reviving already has partial coverage from test 6 (resumeSession called once across two timed-out ticks).
Acceptance criterion: expect(bridge.spawnOrAttach).toHaveBeenCalledTimes(1) across two concurrent same-owner calls; the mutation above (spawn count becomes 2) must make it red, and the existing 6 tests staying green under it is the gap.
中文说明
没有任何测试在同一个 owner 上发起两次重叠的调用，因此单飞守卫 ensuring ??= ensure()（mesh-host-session.ts:94-97）——阻止并发启动爆发按调用方各铸一个 host session 的唯一机制——没有被钉住。这个 Promise.all 把调用分别摊在两个 owner 上，而它们各自有独立的 ensuring 闭包；测试 1、3、6 则在开始下一次调用前 await 了每一次调用。
守卫消失后，同一轮中被派发的两个 mesh agent（或一次 30 秒 tick() 与一次 launch() 重叠）都会进入 ensure()，都 await readMeshWorkspace，都看到 hostSessionId === undefined，都调用 spawnOrAttach——在本只需一个的地方产生两个真实 ACP 子进程。落败方随后被认领竞争关闭，而那一步自己也可能抛错并遗留孤儿 session；每个瞬时子进程还会占用一个 bridge 准入槽位，而在上限处第二次 spawn 会抛 SessionLimitExceededError（bridgeErrors.ts:224-226），于是尽管一个好 host 只差一次 await，launch() 仍会拒绝。结果上没有任何可观察的东西暴露它——两个调用方最终仍得到同一个 session id——只有 spawn 次数会。
修法：新增一个测试，构建一个 owner 并调用 Promise.all([owner.launch(AGENT, 'a'), owner.launch(AGENT, 'b')])——或两次 ensureResident()——配一个每次调用给出不同 id 的 spawnOrAttach mock，断言 spawnOrAttach 恰好被调用一次、且两个结果携带相同的 sessionId。
约束：新 mock 必须满足 MeshHostBridge = Pick<AcpSessionBridge, 'recordHeartbeat' | 'resumeSession' | 'spawnOrAttach' | 'closeSession' | 'launchMeshAgent'>（mesh-host-session.ts:20-27），因为本文件现有的 mock 已经不满足该形态并以 8 个 TS2322 破坏构建——新 mock 不能添第九个。请把这一条与 reviving 重置、running 守卫的覆盖缺口分开处理：它们是三处不同的代码行、三种不同的变异、三种不同的后果（重复的 ACP 子进程 vs 重复的恢复 vs 重叠的 tick），把它们合并会错误陈述覆盖情况，因为 reviving 已经从测试 6 获得了部分覆盖（在两次超时的 tick 中 resumeSession 只被调用一次）。
验收标准：两次同 owner 并发调用下 expect(bridge.spawnOrAttach).toHaveBeenCalledTimes(1)；上述变异（spawn 次数变为 2）必须让它变红，而现有 6 个测试在该变异下仍全绿正是缺口所在。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| }); | ||
| const reloadMs = performance.now() - startedAt; | ||
|
|
||
| expect(handles).toHaveLength(2); |
There was a problem hiding this comment.
[Suggestion] R1-19: Every test reaps the host at most once, so the reviving reset that makes a second revive possible is unexercised — deleting it keeps all six tests green while making the host permanently unrevivable in production. mesh-host-session.ts:69-73 clears the in-flight revive only in the completion's .finally (if (reviving === started) reviving = undefined;). Remove those lines and no test fails: test 1 revives once and then hits the healthy recordHeartbeat path, test 3 has a single reap cycle, and test 6 ends while the completion is still pending. In the daemon the second reap of a long-lived host then takes the if (!reviving) false branch, await reviving.deadline resolves instantly against the stale, already-settled deadline, and ensure() returns a hostSessionId the bridge no longer holds — launch() forwards bridge.launchMeshAgent to a dead session and every mesh launch fails from then on, with no revive ever attempted again until the daemon restarts. The mesh host is by design the longest-lived session in the workspace and survives many reaper cycles, so this is the normal path rather than the rare one.
Witness:
Coverage arm: replacing mesh-host-session.ts:69-73 with `void started.completion.catch(() => {});`
-- removing ONLY the reset, keeping rejection suppression -- leaves `Tests 6 passed (6)`.
Mechanism arm, a discriminating probe over two reaper cycles (recordHeartbeat throws while
resident === false; resumeSession sets it back):
INTACT: resumeSession calls cycle1=1 cycle2=2 residentAfterCycle2=true -> 1 passed
MUTANT: AssertionError: expected 1 to be 2 -> 1 failed
The second reap never revives under the mutant: once `completion` settles, `deadline` is a settled
promise, so with `reviving` never cleared the second reap awaits it and returns instantly.
Extend test 3, or add a case, with a second reap-and-revive cycle: after the first reload succeeds, wait for bridge.sessionCount to drop to 0 again, launch once more, and assert handles reaches length 3 with handles[1].killed === true.
The reset fires on the completion's settlement (mesh-host-session.ts:71), so the new test must let the first revive fully settle before reaping again — the verifying probe needed one setImmediate between cycles for exactly that reason. A still-pending first revive is legitimately suppressed, which is the behaviour test 6 already pins, so the new case must not duplicate that one.
Acceptance criterion: that second cycle must go red when the .finally cleanup at mesh-host-session.ts:69-73 is removed — the second launch would return the dead session id instead of producing a third channel.
中文说明
每个测试至多 reap host 一次，因此让第二次恢复成为可能的 reviving 重置从未被执行——删除它会让六个测试全部保持绿色，同时在生产中让 host 永久无法恢复。mesh-host-session.ts:69-73 只在 completion 的 .finally 中清除进行中的恢复（if (reviving === started) reviving = undefined;）。移除这几行后没有测试会失败：测试 1 恢复一次后就走上健康的 recordHeartbeat 路径，测试 3 只有一个 reap 周期，测试 6 在 completion 仍挂起时结束。
在 daemon 中，一个长生命周期 host 的第二次 reap 随后会走 if (!reviving) 的假分支，await reviving.deadline 会针对那个陈旧的、已经结束的 deadline 立即解析，于是 ensure() 返回一个 bridge 已不再持有的 hostSessionId——launch() 会把 bridge.launchMeshAgent 转发给一个死掉的 session，此后每次 mesh 启动都失败，并且在 daemon 重启之前再也不会尝试恢复。按设计，mesh host 是工作区中生命周期最长的 session、会经历多个 reaper 周期，所以这是常态路径而不是罕见路径。
修法：扩展测试 3，或新增一个用例，加入第二个 reap-and-revive 周期：第一次重载成功后，等待 bridge.sessionCount 再次降到 0，再启动一次，并断言 handles 达到长度 3 且 handles[1].killed === true。
约束：该重置是在 completion 结束时触发的（mesh-host-session.ts:71），所以新测试必须让第一次恢复完全结束之后再 reap——验证用的 probe 正是为此在两个周期之间需要一个 setImmediate。仍在挂起的第一次恢复被合法地抑制，这正是测试 6 已经钉住的行为，所以新用例不能与它重复。
验收标准：当 mesh-host-session.ts:69-73 的 .finally 清理被移除时，那个第二周期必须变红——第二次启动会返回死掉的 session id，而不是产生第三个 channel。
— qwen3.8-max via Qwen Code /review (v0.23.0)
Both tui-parity jobs died in `tsc --build` before running anything: the four inline bridge mocks in mesh-host-session.test.ts are partial, so they no longer satisfy MeshHostBridge = Pick<AcpSessionBridge, ...> now that this branch adds launchMeshAgent and the bridge methods carry their real signatures (recordHeartbeat returns BridgeHeartbeatResult, spawnOrAttach/resumeSession return BridgeSession/BridgeRestoredSession). Eight TS2322 errors, at 111:7, 157:9, 162:9, 322:9, 323:36, 324:36, 346:36 and 347:9. Use the `as unknown as AcpSessionBridge` escape hatch this file already applies to its other two bridge literals (and the convention across src/serve/*.test.ts for this interface, which is far too wide to mock in full). Type-level only: emitted JS is unchanged, so no runtime behaviour moves. Verified locally with `npm run typecheck --workspace=packages/cli`: the same environment reproduces all eight errors on the pre-fix file at the identical line:col, and after the fix reports zero for this file — a diff of exactly 8 errors removed and 0 introduced. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtqkvx55o8
… codex/mesh-step-4-launcher # Conflicts: # packages/cli/src/serve/mesh/mesh-host-session.test.ts
|
已推送 c1636a1，收口本轮隐藏宿主会话审查。 已处理：R1-1/R1-2 两个 Critical；公开创建与 ACP 创建保留 mesh source；session list、search、live-state、goals、branch conflict、CLI list/continue、TUI resume、completion、ACP list、realtime context 全部排除宿主；SessionService 在分页前过滤；launch 回查 workspace.hostSessionId；store 失败返回 typed launch_failed；claim/loser清理 best-effort；计数扫描复用 tail buffer；协议和验收证据同步更新。共享 source 常量移到 runtime 叶子模块，避免 ACP 反向依赖 serve。 定向观测：acpAgent 629/629；SessionService 253/253；mesh host 6/6；multi-workspace 单独 153/153；commands/config/realtime/completion/goals/Standalone picker 合计 483/483；server 新增路径 4/4；定向 ESLint 通过。批量并行跑 7 个命名文件时 multi-workspace 两个既有路由用例出现 405/404，共享状态串扰；同文件单独复跑 153/153。useSessionPicker 与 OpenTUI render 用例在本 worktree 因缺少可选 UI runtime 未收集，留给 #11206 整分支 CI。按本轮约束未运行 build/typecheck。 未在本 PR 扩张：R1-6/R1-21/R1-22/R1-35 属于第 8 步跨进程与生命周期可靠性；R1-10/R1-19/R1-20/R1-28/R1-29/R1-34 是额外的 mutation/timer/时序测试加固，保留给对应可靠性步骤，避免隐藏宿主 PR 继续膨胀。 |
3a81ed8
into
codex/multi-agent-mesh-foundation
3 participants
What this PR does
This stacked step adds the workspace-scoped hidden mesh host and its private launch route. The host is claimed under the mesh workspace lock, reused across launches, kept alive through the existing scheduled-task resume primitive, restored after the bridge reaper removes it, and excluded before pagination from persisted/live and live-only public session catalogs, including their public aggregate counts.
It also routes configured identities through the typed launcher from #11224 and returns
started | capacity_wait | agent_unavailable | launch_failedwithout adding dispatcher behavior.Why it's needed
Persistent mesh identities need one reusable runtime owner per workspace before the dispatcher and thread tools can safely bind runs to background agents. The host must survive bridge reaping without becoming a user-visible chat, while runtime launch remains a private daemon-to-child operation.
The acceptance document previously required a daemon-process observation in this step. There is intentionally no server-bootstrap caller until the dispatcher is added, so this PR records the equivalent real-
AcpSessionBridgereaper test in-process with a fake ACP child and moves the daemon-process observation to step 6/7 instead of adding unused wiring.Reviewer Test Plan
How to verify
capacity_waitand creates no queued booking.agent_unavailablewithout starting a runtime.started.Evidence (Before & After)
N/A — internal runtime and daemon bridge contracts only.
Tested on
The in-process bridge reaper used a 20 ms idle timeout. Reload completed in 4.3 ms against a 1,000 ms resume deadline, and the next launch returned
startedwithout recreating the bridge.Environment (optional)
Node.js workspace with the existing repository dependencies. Targeted core and ACP bridge package builds passed.
Risk & Scope
sourceType=meshas internal and removes it from public list results; direct bridge ownership APIs still retain the full internal view.Linked Issues
Stacked step 4 for #11206. Runtime prerequisite: #11224.
中文说明
这个 PR 做什么
这个 stacked 步骤增加工作区级隐藏 mesh host 及其私有启动路由。host 在 mesh 工作区锁内完成唯一认领，在多次启动间复用，通过现有 scheduled-task resume primitive 保活，在 bridge reaper 移除后恢复，并从 persisted/live 与 live-only 两条公开 session catalog 路径中隐藏。
它还通过 #11224 的类型化 launcher 启动已配置身份，返回
started | capacity_wait | agent_unavailable | launch_failed，但不提前加入 dispatcher 行为。为什么需要
持久 mesh 身份需要一个可复用的工作区 runtime owner，dispatcher 和 thread 工具才能安全地把 run 绑定到后台 agent。host 必须能在 bridge 回收后恢复，同时不能显示成用户会话；runtime 启动只允许 daemon 到 child 的私有调用。
验收文档原先要求本步骤做 daemon 进程观测。当前在 dispatcher 加入前刻意没有 server-bootstrap 调用方，因此这个 PR 使用真实
AcpSessionBridgereaper 加 fake ACP child 做等价的进程内测试，并把 daemon 进程观测移到第 6/7 步，避免提前增加无调用方的 wiring。Reviewer Test Plan
如何验证
capacity_wait且不创建 queued booking。agent_unavailable，且不启动 runtime。started。前后证据
不适用——仅内部 runtime 与 daemon bridge 契约。
测试环境
进程内 bridge reaper 使用 20 ms idle timeout。相对 1,000 ms resume deadline，reload 用时 4.3 ms；下一次启动返回
started，bridge 没有重建。环境（可选）
Node.js workspace，使用仓库现有依赖。定向 core 与 ACP bridge package build 通过。
风险与范围
sourceType=mesh视为内部来源并从公开列表结果移除；bridge 的直接 owner API 仍保留完整内部视图。关联事项
#11206 的 stacked 第 4 步。runtime 前置 PR：#11224。