Skip to content

feat(mesh): run the dispatch loop inside the host session, unattended - #11263

Closed
yiliang114 wants to merge 1 commit into
codex/multi-agent-mesh-foundationfrom
codex/mesh-step-7-supervisor
Closed

feat(mesh): run the dispatch loop inside the host session, unattended#11263
yiliang114 wants to merge 1 commit into
codex/multi-agent-mesh-foundationfrom
codex/mesh-step-7-supervisor

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

Closes step 6 and makes step 7 runnable without a human driving it. supervisor.ts ticks dispatchOnce inside the hidden host session; acpAgent.ts starts it for any session whose source type is the mesh host and stops it when that session is discarded; server.ts keeps one host session resident per trusted workspace whenever the roster is non-empty. Two stale assertions from earlier steps are aligned with #11252.

Why it's needed

Nothing ticked the dispatcher. dispatchOnce and startMeshHostSessionOwner were referenced only by their tests, and the first live slice recorded in §0.2 was driven by hand. Without a loop there is no unattended demo.

Where the loop runs is a decision, and it is now made: in the host session, not the daemon. The launcher and the background-agent registry live in that session's process, so ticking there makes inspect a local registry read and a start a local call. A daemon-side loop would have paid an ACP round trip per candidate per tick for nothing. The daemon's whole job is to keep the host resident. Recorded in the design's §5.2 step 6 as decision (a), with (b) and why it was rejected.

Two guards make it safe unattended. The supervisor dispatches only while its session still holds the workspace's host claim, so a stale duplicate the reaper did not reach cannot give one agent two bodies. Ticks coalesce rather than overlap: a tick requested mid-pass joins that pass and receives its outcome, so a caller asking for "the state after the next pass" always gets one and the port is never entered twice for the same queued runs.

It polls on purpose. Posts written by the daemon's REST route land in the store, not in this process, and the design treats in-process notifications as hints rather than truth. Two seconds of latency is the price of never missing a durable trigger.

Reviewer Test Plan

How to verify

cd packages/core
npx vitest run src/agents/mesh/supervisor.test.ts   # claim guard, empty roster, coalesced ticks
npx vitest run src/agents/mesh/                      # 14 files

Then the live recipe from the acceptance doc: qwen serve, POST /mesh/agents for alice and bob, POST /mesh/threads assigned to alice. Within one keepalive interval the daemon spawns the host; within 2 s the supervisor launches alice. qwen serve --debug shows MESH_SUPERVISOR ticks.

Evidence (Before & After)

Before: booked runs stayed queued until someone called dispatchOnce by hand. After: they start on their own, only from the claimed host.

Tested on

OS Status
🍏 macOS N/A
🪟 Windows N/A
🐧 Linux

Environment (optional)

Named Vitest files only. Observed: supervisor.test.ts 3 tests; src/agents/mesh/ 14 files / 139 tests. mesh-host-session.test.ts could not load on the review machine (its node_modules resolves an older @qwen-code/acp-bridge without ./sessionSource); CI is the gate for it, once #11261 unbreaks the build.

Risk & Scope

  • Main risk or tradeoff: the daemon-side wiring (server.ts) has no automated test on this branch; it mirrors the scheduled-task keepalive block line for line and is exercised only by the live recipe. The child-side wiring is one if on the session's source type.
  • Not validated / out of scope: the live recipe has not been run from this branch on a build-capable machine. Restart and stall recovery, queueExternalInput(false) rebook, and cancellation remain step 8.
  • Breaking changes / migration notes: none. MeshTickOutcome is new; dispatchOnce and createMeshDispatchPort are now exported from core for the ACP child.

Linked Issues

Parent delivery PR: #11206. Depends on #11261 for a green build. Pairs with #11260 for the visible demo.

Nothing ticked the dispatcher: dispatchOnce and startMeshHostSessionOwner were
referenced only by their tests, and the first live slice was driven by hand.
The loop now runs inside the hidden host session. The launcher and the
background-agent registry live in that process, so ticking there makes
inspect a local registry read and a start a local call; a daemon-side loop
would have paid a process boundary per candidate per tick for nothing. The
daemon's whole job is to keep the host resident, which it does for any
trusted workspace whose roster is non-empty.

The supervisor dispatches only while its session still holds the workspace's
host claim, so a stale duplicate the reaper did not reach cannot give one
agent two bodies. Ticks coalesce rather than overlap, and a tick requested
mid-pass joins that pass and receives its outcome. It polls rather than
waiting for notifications because posts written by the daemon's REST route
land in the store, not in this process.

Two stale assertions are aligned with changes that landed after they were
written: thread tool names now live in ToolNames, and the prompt window is
committed when the runtime accepts the turn rather than at booking.
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required section is filled in, including the OS table and Risk & Scope. One nit: the template's 中文说明 <details> block is missing from the body.

Problem: real, and verifiable rather than theoretical. startMeshHostSessionOwner had no production caller before this PR — it was exported and exercised only by its own test — and dispatchOnce was in the same state. So the mesh had a dispatcher and a host owner with nothing connecting them, which matches the description's claim that the first live slice had to be driven by hand. This is wiring up dead code on a tracked plan, not hardening against a hypothetical.

Direction: aligned. This is step 6 of the committed mesh implementation plan, stacked on the still-open parent #11206, so nothing here reaches main users yet. The placement decision is the interesting part and I think it is argued correctly: the dispatch port needs config.getBackgroundTaskRegistry(), which only exists in the host session's process, so a daemon-side loop would pay an ACP round trip per candidate per tick to reach state it does not own. Ticking where the state lives is the right call, and rejecting option (b) on those grounds is sound.

Size: core paths are touched (packages/core/src/agents/** plus cross-package wiring in packages/cli). Breakdown of the 439 changed lines: 230 production (supervisor.ts 141, server.ts 54, acpAgent.ts 25, agents/index.ts 10), 189 test (4 files), 20 docs (2 plan files). Title type is feat, so no size gate applies, and 230 is well under the 500-line escalation threshold.

Approach: scope feels right — one new 141-line module, two wiring edits, and two stale assertions realigned. No drive-by refactors. Two things I'd want answered before this merges, both about how faithfully the daemon side mirrors the pattern it copies:

  • server.ts parks stopMeshHosts on app.locals with a comment saying the shutdown sequence can invoke it, but nothing does — neither the app-drain block below it nor stopRuntimeAppProducers in the serve runner reads that key. The scheduled-task keepalive it mirrors is stopped from both. Details in the review comment.
  • The mirrored block also parks per-workspace start/stop hooks that the runtime lifecycle consumes when a workspace is added or drained. The mesh host parks neither, so a workspace registered after startup never gets a host, and a drained one keeps its roster timer.

Risk: Stage 1e matched a high-risk path — packages/cli/src/acp-integration/acpAgent.ts (acp-integration, 10 of 31 historically reverted PRs touched these paths vs 5 of 60 controls). That does not block anything, but it means full Stage 2 enrichment and CI evidence before approval. Note also that this PR's own CI is almost entirely skipped because the base is a feature branch rather than main — the test evidence section covers what that leaves us with.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 各必填部分都写了,包括操作系统表格和 Risk & Scope。一个小问题:模板要求的 中文说明 <details> 段落缺失。

**问题:**真实存在,且可验证,不是理论性加固。在本 PR 之前 startMeshHostSessionOwner 没有任何生产调用方——只被导出并被自己的测试覆盖——dispatchOnce 同样如此。也就是说 mesh 有调度器和 host owner,但两者之间没有连接,这与描述里"第一次联调需要手动驱动"一致。这是把一个已立项计划里的死代码接通,不是针对假想场景的加固。

**方向:**对齐。这是已提交的 mesh 实施计划里的 step 6,叠在仍未合并的父 PR #11206 之上,因此目前不会影响 main 上的用户。最有意思的是循环放在哪里的决策,我认为论证是正确的:dispatch port 需要 config.getBackgroundTaskRegistry(),而它只存在于 host session 进程内,所以放在 daemon 侧每个 tick、每个候选都要付一次 ACP 往返去访问并不属于自己的状态。在状态所在处 tick 是对的,据此否掉方案 (b) 也站得住。

**规模:**触及核心路径(packages/core/src/agents/**,以及 packages/cli 的跨包接线)。439 行改动明细:生产代码 230 行supervisor.ts 141、server.ts 54、acpAgent.ts 25、agents/index.ts 10)、测试 189 行(4 个文件)、文档 20 行(2 个计划文件)。标题类型是 feat,因此不触发规模硬门禁,230 行也远低于 500 行的升级阈值。

**方案:**范围合理——一个新模块 141 行、两处接线改动、两条过期断言的对齐,没有顺手重构。合并前我想先确认两点,都是关于 daemon 侧对其所复制模式的还原是否完整:

  • server.tsstopMeshHosts 挂在 app.locals 上,注释说关闭流程可以调用它,但实际上没有任何地方调用——它下面的 app-drain 代码块和 serve runner 里的 stopRuntimeAppProducers 都没读这个键。它所模仿的 scheduled-task keepalive 在两处都被停掉了。详见审查评论。
  • 被模仿的代码块还挂了按 workspace 的 start/stop 钩子,供 runtime 生命周期在 workspace 新增或摘除时调用。mesh host 两个都没挂,因此启动后新注册的 workspace 永远拿不到 host,被摘除的 workspace 的 roster 定时器也仍在跑。

**风险:**Stage 1e 命中高风险路径——packages/cli/src/acp-integration/acpAgent.tsacp-integration;历史回滚统计中 31 个被回滚 PR 有 10 个触及这些路径,对照组 60 个中只有 5 个)。这不构成阻断,但意味着要走完整的 Stage 2 补充审查,并在批准前拿到 CI 证据。另外请注意:本 PR 自身的 CI 几乎全是 skipped,因为 base 是特性分支而不是 main——测试证据一节会说明这留下了什么。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 775d37abcc1baad020d153b5cbd4183e20533753 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

The core of this PR is good. The claim guard is the right safety property, tick coalescing via inFlight + .finally is correct, runPass turning a thrown port error into a typed error outcome means the loop survives a broken registry, and both timers are unref()'d in line with the surrounding house style. I verified the wiring resolves: packages/core/src/index.ts:623 re-exports ./agents/index.js, so the new symbols reach acpAgent.ts's import from @qwen-code/qwen-code-core, and dispatchOnce(input.projectRoot, input.port) matches the signature at dispatcher.ts:172.

My independent read before opening the diff was that the loop belonged in the existing MeshHostSessionOwner, which already has a 30 s timer and a tick(). The PR's placement is better than that and I'll say so: the port needs config.getBackgroundTaskRegistry(), which only exists in the child, so a daemon-side loop would cross a process boundary per candidate per tick to reach state it does not own.

What I could not confirm is the lifecycle wiring. Four findings, all in the part the description itself flags as untested.

1. closeStoredSession removes a session without stopping its supervisor. The stop lives at acpAgent.ts:4518-4519, inside discardStoredSessionIfCurrent — but the session map entry is actually dropped in removeStoredSessionEntry (4149, delete at 4178), which closeStoredSession calls directly at 4474. closeStoredSession is reached without the wrapper from the ACP sessionClose control method at 10686. There is a concrete path that hits this: in mesh-host-session.ts, when ensure() loses the claim race it calls bridge.closeSession(spawned.sessionId) on a session it spawned with sourceType: MESH_HOST_SESSION_SOURCE_TYPE — i.e. one that started a supervisor at 14570. That supervisor keeps ticking for the life of the child process, and its meshSupervisors entry is never deleted.

The damage is contained, and contained by your own guard: the orphan reads workspace.hostSessionId !== input.sessionId, returns not_claimed_host, and never dispatches, so no agent gets two bodies. The cost is an unbounded Map plus a permanent 2 s poll doing readMeshWorkspace + readMeshAgents per orphan. Moving the stop into removeStoredSessionEntry — the single place the entry goes away — fixes this and also covers the other two callers at 3750 and 14661.

2. stopMeshHosts is parked and never read. Written at server.ts:3298, with a comment saying the shutdown sequence in the serve runner can invoke it. Nothing does. The app-drain block 40 lines below reads stopScheduledTaskKeepalive, stopWorkspaceGitState, stopExtensionGenerationReconciler (3544-3551); stopRuntimeAppProducers reads stopScheduledTaskKeepalive, stopWorkspaceGitState, stopLiveCoordinator, stopWebTerminalRegistry, subSessionStoppers (run-qwen-serve.ts:4569-4593). Grepping all of packages/cli/src finds no other reference to the key. So on a graceful drain the roster timer keeps firing ensureIfRostered()owner.ensureResident()bridge.spawnOrAttach(...) while and after the bridges are being shut down at the end of that same drain block. The keepalive it mirrors is stopped first precisely to avoid spawning into a draining bridge. Both timers are unref()'d, so this is not a hang — it is an ordering hazard.

3. The mesh host is not wired to workspace add/drain. The mirrored block also parks startScheduledTaskKeepaliveForWorkspace and stopScheduledTaskKeepaliveForWorkspace (server.ts:3315-3327), and the runtime lifecycle consumes them: runtimeAdded starts a keepalive for a newly registered workspace (run-qwen-serve.ts:7138-7141) and the drain paths stop and restart it per workspace (7185-7189, 7221-7225, 7342-7357). startMeshHostForWorkspace is parked nowhere, so a workspace registered after createServeApp never gets a host — its roster is never checked and its dispatch loop never runs — while a drained workspace's owner and roster timer keep going. The description says this mirrors the keepalive block "line for line"; it mirrors the startup loop but not the lifecycle hooks.

4. The deleted watermark assertion points at coverage that does not exist. The removal in dispatcher.test.ts is itself correct: bindRunSession now commits committedThroughSequence only when consumedOnStart is true (thread-actions.ts:474-486) and the test's mock returns no consumedOnStart, so committedThroughSequence === 1 genuinely no longer holds. But the replacement comment says the watermark "is asserted there (see runtime-bridge.ts)". runtime-bridge.ts is production code with no assertions, and there is no runtime-bridge.test.ts. At this commit committedThroughSequence appears in mesh tests only in prompt.test.ts:144,166,183, and only as input fixture state feeding contextThroughSequence. Nothing asserts the write side — neither the consumedOnStart: true branch nor consumeRunDelivery's commit at thread-actions.ts:562; thread-actions.test.ts does not mention bindRunSession or consumeRunDelivery at all. Per AGENTS.md a missing test is a Suggestion, but a comment naming coverage that isn't there will mislead the next reader, which is worse than no comment.

Two smaller things: DEFAULT_MESH_SUPERVISOR_INTERVAL_MS is exported at agents/index.ts:45 with no reader — acpAgent.ts starts the supervisor without intervalMs — and onTick, documented as "for the daemon's status surface", has no production caller either (only supervisor.test.ts:155). onTick also sits outside runPass's try/catch, so a throwing observer would reject tick() despite its "never throws" doc. Both latent, both the shape AGENTS.md asks you to grep read sites for. And each tick reads the workspace and roster twice, since dispatchOnce re-reads both plus listThreads (dispatcher.ts:179-181) after the guards already did — fine at 2 s, worth knowing if that interval ever tightens.

On the two realigned assertions: both check out. The six thread tools really are in ToolNames now (tool-names.ts:70-75), so capability.test.ts excluding them by name is aligning with reality rather than masking a regression — and buildMeshToolConfig / createMeshToolInvocationGuard still gate by classification, so the read-only boundary stays pinned by the assertions that remain. The storage.getProjectDir mock addition is a straightforward fixture fix for #11255's sidecar relocation.

sequenceDiagram
    participant P1 as Daemon server.ts roster timer
    participant P2 as MeshHostSessionOwner
    participant P3 as ACP bridge
    participant P4 as Host child acpAgent.ts
    participant P5 as MeshSupervisor
    participant P6 as dispatchOnce plus port
    P1->>P1: every keepalive, readMeshAgents
    P1->>P2: ensureResident() when roster non-empty
    P2->>P3: spawnOrAttach(sourceType mesh host)
    P3->>P4: new session with mesh host source type
    P4->>P5: startMeshSupervisor(projectRoot, sessionId, port)
    loop every 2 s
        P5->>P5: readMeshWorkspace, claim guard
        P5->>P6: dispatchOnce(projectRoot, port)
        P6->>P6: selectCandidates, inspect, claimRun
        P6-->>P5: DispatchRecord list
    end
    P4->>P5: stop() on discardStoredSessionIfCurrent
Loading
Files changed (10)
File What changed
packages/core/src/agents/mesh/supervisor.ts New. The 2 s dispatch loop: claim guard, roster short-circuit, coalesced ticks, typed outcomes. The substance of the PR and the part that reads best.
packages/core/src/agents/mesh/supervisor.test.ts New. Three tests: stranger session refuses to dispatch, empty roster no-ops, ticks coalesce and the loop survives a throwing port.
packages/cli/src/serve/server.ts Per-trusted-workspace host owner plus a roster timer. Carries findings 2 and 3: the stop fn is never read and the lifecycle hooks are not parked.
packages/cli/src/acp-integration/acpAgent.ts Starts a supervisor for mesh-host sessions and stops it on discard. Carries finding 1: the stop is on the wrapper, not on the path that actually removes the session.
packages/core/src/agents/index.ts Re-exports the dispatcher, port factory and supervisor for the ACP child. Reachable via the package root.
packages/core/src/agents/mesh/dispatcher.test.ts Drops the two watermark assertions. Correct to drop, but the replacement comment cites coverage that does not exist (finding 4).
packages/core/src/agents/mesh/capability.test.ts Excludes the thread tools by name before asserting nothing else classifies as thread. Legitimate realignment.
packages/core/src/agents/mesh/dispatch-port.test.ts Adds a storage mock so the sidecar lookup resolves. Fixture fix.
docs/plans/2026-09-06-multi-agent-board-collaboration.md Records decision (a) and why (b) was rejected. Useful, and it matches the code.
docs/plans/2026-09-07-mesh-implementation-acceptance.md Closes step 6 and adds the live demo recipe.

Testing

This is an unattended CI run, so I executed nothing from this PR — no build, no test, no gh pr checkout. The evidence below is the PR's own CI, read through the API for commit 775d37a.

The load-bearing fact: the repository's real gates do not run on this PR at all. ci.yml fires pull_request only for main and release/** (ci.yml:21-24), and this PR's base is codex/multi-agent-mesh-foundation. So there is no lint, typecheck, build or unit-test check on this commit — not failing, not pending, simply never triggered. tui-parity.yml has no branch filter, which is why its two checks are the only substantive ones present.

That means there is currently no automated evidence that supervisor.test.ts passes, or that the mesh suite is at 14 files / 139 tests. The numbers in the description — 3 supervisor tests, 139 mesh tests, and mesh-host-session.test.ts failing to load because the review machine resolves an older @qwen-code/acp-bridge without ./sessionSource — are the author's claim from a local run, not evidence I can corroborate, and the description says as much. The description also states plainly that the daemon-side wiring has no automated test on this branch, which is where findings 1-3 all sit.

Not verified: that a booked run actually starts unattended; that the claim guard holds against a real stale duplicate rather than a mocked one; that qwen serve keeps exactly one host resident per trusted workspace; and the shutdown-ordering hazard in finding 2. None of these are observable from the diff, and no check on this commit observes them either.

Check Conclusion
Qwen Code CI (lint, static analysis, unit tests) not triggered — base branch is not main or release/**
TUI parity snapshots (ink vs opentui) in_progress
OpenTUI no-flicker gate in_progress
assign success
label success
authorize success (2 runs)
triage in_progress (this run)
delay-automatic-review waiting
review-pr, tmux-testing, verify, publish-resolution, publish-tmux, publish-verify, resolve-pr, precheck-pr, review-config, ack-review-request, fallback-comment skipped — bot orchestration, not triggered

The two in-progress checks are TUI parity and a no-flicker gate. Neither touches the mesh, so green results there would say nothing about this PR's claim. tui-parity is the one pending pull_request workflow run on this commit.

Sandboxed verification would settle this, and you have write access so both lanes are available: @qwen-code /verify — that a booked run actually transitions queuedrunning with no human calling dispatchOnce, and that a second supervisor for the same workspace returns not_claimed_host against real store state rather than a mocked port, is not observable from this diff and no check on this commit observes it. @qwen-code /tmux — for the daemon side: qwen serve, two POST /mesh/agents, one POST /mesh/threads, and whether the host spawns and the agent starts within the claimed intervals. That is also the only lane that would surface findings 2 and 3, since both are lifecycle behaviours that a unit suite on this branch does not cover. Worth noting the description's own recipe has not been run from this branch on a build-capable machine.

Real-scenario testing (2c): N/A — unattended CI run, which never drives the product locally.

中文说明

代码审查

PR 的核心部分是好的。claim guard 是正确的安全属性,通过 inFlight + .finally 实现的 tick 合并是正确的,runPass 把 port 抛出的错误转成类型化的 error 结果意味着循环能在注册表损坏时存活,两个定时器都按周边代码风格做了 unref()。接线我验证过是通的:packages/core/src/index.ts:623 重新导出了 ./agents/index.js,因此新符号能被 acpAgent.ts@qwen-code/qwen-code-core 导入,dispatchOnce(input.projectRoot, input.port) 也与 dispatcher.ts:172 的签名一致。

我在看 diff 之前的独立判断是:循环应该放进已有的 MeshHostSessionOwner,它已经有 30 s 定时器和 tick()。本 PR 的放置比我的想法更好,我直说:port 需要 config.getBackgroundTaskRegistry(),而它只存在于子进程里,所以 daemon 侧的循环每个 tick、每个候选都要跨一次进程边界去访问并不属于自己的状态。

我无法确认的是生命周期接线。四条发现,都落在描述自己承认未测试的那部分。

1. closeStoredSession 移除 session 时没有停掉它的 supervisor。 stop 在 acpAgent.ts:4518-4519,位于 discardStoredSessionIfCurrent 内部——但 session map 条目实际是在 removeStoredSessionEntry4149,删除在 4178)里摘掉的,而 closeStoredSession4474 直接调用它。closeStoredSession 可以不经过那层 wrapper 被调到,入口是 10686 的 ACP sessionClose 控制方法。有一条具体路径会命中:mesh-host-session.tsensure() 抢 claim 失败时,会对一个自己用 sourceType: MESH_HOST_SESSION_SOURCE_TYPE 生成的 session 调用 bridge.closeSession(spawned.sessionId)——也就是在 14570 启动过 supervisor 的那个。该 supervisor 会在子进程整个生命周期里继续 tick,且它的 meshSupervisors 条目永不被删除。

影响是被控制的,而且是被你自己的 guard 控制的:孤儿读到 workspace.hostSessionId !== input.sessionId,返回 not_claimed_host,永不派发,所以不会出现一个 agent 两个 body。代价是 Map 无界增长,外加每个孤儿一个永久的 2 s 轮询在做 readMeshWorkspace + readMeshAgents 文件读。把 stop 移进 removeStoredSessionEntry——条目消失的唯一位置——即可修好,同时覆盖 375014661 两个调用方。

2. stopMeshHosts 挂上去了但没人读。 写在 server.ts:3298,注释说 serve runner 的关闭流程可以调用它。实际没有任何地方调用。下方 40 行的 app-drain 代码块读的是 stopScheduledTaskKeepalivestopWorkspaceGitStatestopExtensionGenerationReconciler3544-3551);stopRuntimeAppProducers 读的是 stopScheduledTaskKeepalivestopWorkspaceGitStatestopLiveCoordinatorstopWebTerminalRegistrysubSessionStoppersrun-qwen-serve.ts:4569-4593)。在整个 packages/cli/src 里 grep 不到这个键的其他引用。于是优雅 drain 时,roster 定时器会继续触发 ensureIfRostered()owner.ensureResident()bridge.spawnOrAttach(...),而这发生在同一个 drain 代码块末尾正在关闭 bridge 期间甚至之后。它所模仿的 keepalive 之所以先被停掉,正是为了避免向正在 drain 的 bridge 生成 session。两个定时器都 unref() 了,所以这不是挂死——是时序隐患。

3. mesh host 没有接入 workspace 的新增/摘除生命周期。 被模仿的代码块还挂了 startScheduledTaskKeepaliveForWorkspacestopScheduledTaskKeepaliveForWorkspaceserver.ts:3315-3327),runtime 生命周期会消费它们:runtimeAdded 为新注册的 workspace 启动 keepalive(run-qwen-serve.ts:7138-7141),drain 路径按 workspace 停止并重启(7185-71897221-72257342-7357)。startMeshHostForWorkspace 哪里都没挂,因此 createServeApp 之后注册的 workspace 永远拿不到 host——它的 roster 不会被检查,派发循环也不会运行——而被摘除的 workspace 的 owner 和 roster 定时器仍在跑。描述说这里是"逐行照搬 keepalive 代码块";照搬了启动循环,但没照搬生命周期钩子。

4. 被删除的 watermark 断言指向了并不存在的覆盖。 dispatcher.test.ts 里的删除本身是对的:bindRunSession 现在只在 consumedOnStart 为 true 时提交 committedThroughSequencethread-actions.ts:474-486),而测试的 mock 不返回 consumedOnStart,所以 committedThroughSequence === 1 确实不再成立。但替换的注释说 watermark "在那里被断言(见 runtime-bridge.ts)"。runtime-bridge.ts 是生产代码,没有任何断言,也不存在 runtime-bridge.test.ts。在此 commit 上,committedThroughSequence 在 mesh 测试里只出现在 prompt.test.ts:144,166,183,而且只作为喂给 contextThroughSequence输入 fixture 状态。没有任何测试断言写入侧——既没有 consumedOnStart: true 分支,也没有 consumeRunDeliverythread-actions.ts:562 的提交;thread-actions.test.ts 完全没提到 bindRunSessionconsumeRunDelivery。按 AGENTS.md,缺测试算 Suggestion,但一条指名了不存在覆盖的注释会误导下一个读者,这比没有注释更糟。

另外两件小事:DEFAULT_MESH_SUPERVISOR_INTERVAL_MSagents/index.ts:45 被导出但没有读者——acpAgent.ts 启动 supervisor 时没传 intervalMsonTick 被文档描述为"给 daemon 的状态面用",同样没有生产调用方(只有 supervisor.test.ts:155)。onTick 还在 runPass 的 try/catch 之外,因此一个抛异常的 observer 会让 tick() reject,与它"never throws"的文档矛盾。两者都只是潜在问题,但正是 AGENTS.md 要求 grep 读取点的那类形态。还有一点:每个 tick 会把 workspace 和 roster 读两遍,因为 guard 读完之后 dispatchOnce 又会把两者加 listThreads 重读一遍(dispatcher.ts:179-181)——2 s 间隔下没问题,但如果以后收紧间隔就值得注意。

关于两条被对齐的断言:都核实过。六个 thread 工具现在确实在 ToolNames 里(tool-names.ts:70-75),所以 capability.test.ts 按名字排除它们是对齐现实,不是掩盖回归——而 buildMeshToolConfig / createMeshToolInvocationGuard 仍按分类做门禁,因此只读边界依然由保留下来的断言钉住。storage.getProjectDir mock 的新增是针对 #11255 sidecar 位置迁移的直接 fixture 修复。

测试

这是无人值守的 CI 运行,因此我没有执行本 PR 的任何东西——没有构建、没有测试、没有 gh pr checkout。下面的证据是 PR 自己的 CI,通过 API 读取 commit 775d37a 得到。

关键事实:本仓库真正的门禁在这个 PR 上根本没有运行。 ci.ymlpull_request 只对 mainrelease/** 触发(ci.yml:21-24),而本 PR 的 base 是 codex/multi-agent-mesh-foundation。所以这个 commit 上没有 lint、typecheck、build 或单元测试检查——不是失败,不是等待中,而是从未触发。tui-parity.yml 没有分支过滤,这就是为什么它的两个检查是仅有实质性存在的检查。

这意味着目前没有任何自动化证据表明 supervisor.test.ts 通过,或者 mesh 测试套件是 14 文件 / 139 测试。描述里的数字——3 个 supervisor 测试、139 个 mesh 测试,以及 mesh-host-session.test.ts 因审查机器解析到不含 ./sessionSource 的旧版 @qwen-code/acp-bridge 而无法加载——都是作者本地运行的自述,不是我能佐证的证据,描述本身也是这么说的。描述也明确讲了 daemon 侧接线在本分支上没有自动化测试,而发现 1-3 全部落在那里。

未验证:booked run 是否真的无人值守地启动;claim guard 面对真实的过期副本(而非 mock)是否成立;qwen serve 是否对每个受信任 workspace 只保持一个 host 常驻;以及发现 2 的关闭时序隐患。这些都无法从 diff 观察,此 commit 上也没有任何检查观察它们。

两个进行中的检查是 TUI parity 和 no-flicker gate,都不触及 mesh,因此它们变绿也说明不了本 PR 的主张。tui-parity 是此 commit 上唯一待定的 pull_request 工作流运行。

沙箱验证可以定论这件事,而且你有写权限,两条通道都可用:@qwen-code /verify —— booked run 是否真的在无人调用 dispatchOnce 的情况下从 queued 变成 running,以及同一 workspace 的第二个 supervisor 面对真实 store 状态(而非 mock 的 port)是否返回 not_claimed_host,这些从 diff 观察不到,此 commit 上也没有检查观察。@qwen-code /tmux —— 用于 daemon 侧:qwen serve、两次 POST /mesh/agents、一次 POST /mesh/threads,看 host 是否生成、agent 是否在声称的间隔内启动。这也是唯一能暴露发现 2 和 3 的通道,因为两者都是本分支单元测试未覆盖的生命周期行为。顺带一提,描述里的联调步骤本身尚未在本分支的可构建机器上跑过。

真实场景测试(2c):N/A —— 无人值守 CI 运行,从不在本地驱动产品。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 775d37abcc1baad020d153b5cbd4183e20533753 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 3/5 — the supervisor itself is clean and the placement decision is better than the one I would have made; the daemon-side lifecycle wiring is incomplete in three specific places, and nothing on this commit can tell us whether the loop actually runs.

⏸️ Deferring to @yiliang114 — not approving. You own this roadmap line, so the call is yours, but I don't think this should merge as-is, and I want to be explicit about why rather than wave it through because the author is a maintainer.

Stepping back: the problem is real, and I verified it rather than taking the description's word — startMeshHostSessionOwner had no production caller before this PR, so the mesh genuinely had a dispatcher and a host owner with nothing between them. The design question this PR actually settles is the interesting one, and it settles it correctly. I went in expecting to argue the loop belonged in the existing MeshHostSessionOwner, which already has a timer and a tick(). That would have been worse: the port needs the child's background-task registry, so a daemon-side loop crosses a process boundary every tick to read state it does not own. Recording (a) versus (b) in the plan doc, with the reason (b) lost, is exactly the kind of thing that makes this maintainable later.

supervisor.ts reads well and I'd thank whoever wrote it in six months. The claim guard is the right invariant, and it earns its keep immediately — it is the only reason finding 1 stays a leak instead of becoming one agent with two bodies. Coalescing is correct, a thrown port error becomes a typed outcome instead of wedging the loop, and the three tests pin the properties that matter rather than the implementation.

Where I stop is the wiring, and it stops for a reason the description already half-concedes: the daemon side has no automated test on this branch. All three findings live in that untested gap, and all three are the same mistake — the block copies the scheduled-task keepalive's startup loop but not its lifecycle. stopMeshHosts is parked on app.locals for a shutdown sequence that never reads it, so a graceful drain can keep calling spawnOrAttach into a bridge that is being shut down. startMeshHostForWorkspace is parked nowhere, so a workspace registered after startup silently never gets a host — that is a functional hole, not a tidy-up, and it would be inherited by #11206 on merge. And the supervisor's stop sits on discardStoredSessionIfCurrent while the session entry is actually removed one level down in removeStoredSessionEntry, which closeStoredSession reaches directly; the claim-race loser in mesh-host-session.ts walks straight into that. None of the three are hard to fix — the first is a key in a list, the second is parking two hooks the mirrored block already parks, the third is moving two lines down a level — but each one is the kind of thing that looks fine in a diff and only shows up in a long-lived daemon.

Finding 4 is smaller but I'd still fix the comment: the deleted watermark assertion is right to delete, and it points the next reader at runtime-bridge.ts for coverage that does not exist anywhere in the suite. A stale pointer in a test comment outlives the person who wrote it.

On evidence, I want to be blunt about how thin it is. ci.yml fires pull_request only for main and release/**, so this base branch gets no lint, typecheck, build or unit-test run at all — the only in-progress checks are TUI parity and a no-flicker gate, neither of which touches the mesh. The central claim of this PR is behavioural ("booked runs start on their own, only from the claimed host"), and right now it rests entirely on the description's local numbers plus a live recipe the description says has not been run from this branch. I executed nothing — unattended CI run, and this gate never runs PR code. So: not "tests pass", but "no test ran".

Two ways to close that, both available to you: @qwen-code /verify for the claim that a queued run transitions without a hand calling dispatchOnce and that a second supervisor returns not_claimed_host against real store state; @qwen-code /tmux for the daemon side, which is also the only lane that would surface the drain-ordering and late-workspace gaps. Either would move me off 3/5 quickly if it comes back clean alongside the three wiring fixes.

I'm not requesting changes, because the feature's direction and its core module are right and this is stacked work on an unmerged branch where you may well prefer to fold the fixes into step 8's restart-and-stall pass. But I'm not approving either: three concrete lifecycle defects plus zero automated evidence on the commit is not a state I'd attest to.

中文说明

信心度:3/5 —— supervisor 本身很干净,放置决策比我原本会做的更好;但 daemon 侧的生命周期接线有三处具体缺失,而且此 commit 上没有任何东西能告诉我们这个循环是否真的跑起来了。

⏸️ 转交 @yiliang114,不予批准。 这条 roadmap 归你,所以决定权在你。但我不认为它应该按现状合并,我也想明确说清原因,而不是因为作者是维护者就放行。

退一步看:问题是真实的,而且是我自己核实的,不是照抄描述——在本 PR 之前 startMeshHostSessionOwner 没有生产调用方,所以 mesh 确实是有调度器和 host owner、中间什么都没有。本 PR 真正定下来的设计问题才是有意思的部分,而且定得对。我一开始准备论证循环应该放进已有的 MeshHostSessionOwner,它已经有定时器和 tick()。那样会更糟:port 需要子进程的 background-task 注册表,所以 daemon 侧的循环每个 tick 都要跨一次进程边界去读不属于自己的状态。把方案 (a) 与 (b) 连同 (b) 落选的原因记进计划文档,正是让这东西日后还可维护的做法。

supervisor.ts 读起来很舒服,半年后我会感谢写它的人。claim guard 是正确的不变量,而且立刻就体现价值——它正是发现 1 只停留在"泄漏"而没有变成"一个 agent 两个 body"的唯一原因。合并逻辑正确,port 抛错会变成类型化结果而不是卡死循环,三个测试钉住的是关键属性而非实现细节。

我停下来的地方是接线,而停下来的理由描述里已经承认了一半:daemon 侧在本分支上没有自动化测试。三条发现全部落在这个未测试的缺口里,而且是同一个错误——这段代码照搬了 scheduled-task keepalive 的启动循环,却没照搬它的生命周期。stopMeshHosts 被挂在 app.locals 上,等一个从不读它的关闭流程,于是优雅 drain 时可能持续对正在关闭的 bridge 调用 spawnOrAttachstartMeshHostForWorkspace 哪里都没挂,所以启动之后注册的 workspace 会静默地永远拿不到 host——这是功能空洞,不是整理工作,而且合并时会被 #11206 继承。supervisor 的 stop 放在 discardStoredSessionIfCurrent 上,而 session 条目实际是在下一层的 removeStoredSessionEntry 里摘掉的,closeStoredSession 会直接走到那里;mesh-host-session.ts 里抢 claim 失败的那个正好撞上。三条都不难修——第一条是往列表里加一个键,第二条是挂上被模仿代码块本来就挂的两个钩子,第三条是把两行往下挪一层——但每一条都属于在 diff 里看着没事、只在长时间运行的 daemon 中才暴露的那类问题。

发现 4 更小,但我仍会改那条注释:删掉 watermark 断言是对的,而它把下一个读者指向 runtime-bridge.ts 去寻找整个测试套件里都不存在的覆盖。测试注释里的过期指针会比写它的人活得更久。

关于证据,我想直说它有多薄。ci.ymlpull_request 只对 mainrelease/** 触发,所以这个 base 分支完全没有 lint、typecheck、build 或单元测试运行——唯一进行中的检查是 TUI parity 和 no-flicker gate,两者都不触及 mesh。本 PR 的核心主张是行为性的("booked run 会自行启动,且只从持有 claim 的 host 启动"),而现在它完全依赖描述里的本地数字,外加一个描述自己说尚未在本分支上跑过的联调步骤。我什么都没执行——无人值守 CI 运行,而且这个门禁从不运行 PR 代码。所以结论不是"测试通过",而是"没有测试运行"。

有两条路可以补上,你都有权限:@qwen-code /verify 用来验证排队的 run 是否真的在无人调用 dispatchOnce 时发生状态转移,以及第二个 supervisor 面对真实 store 状态是否返回 not_claimed_host@qwen-code /tmux 用来验证 daemon 侧,那也是唯一能暴露 drain 时序和后注册 workspace 缺口的通道。任一条如果跑干净、并且三处接线问题一并修好,我会很快从 3/5 上调。

我没有提交 request changes,因为功能方向和核心模块是对的,而且这是叠在未合并分支上的工作,你完全可能更愿意把这些修复并入 step 8 的重启与卡死恢复那一轮。但我也不批准:三处具体的生命周期缺陷,加上此 commit 上零自动化证据,不是一个我愿意背书的状态。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 775d37abcc1baad020d153b5cbd4183e20533753 · re-run with @qwen-code /triage

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Converted to draft, not closed. Folded into #11294, which now carries this commit and is the single PR against the mesh branch. Kept open for its review history; please review #11294 instead.

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Closing: superseded by work already on the mesh branch.

@yiliang114 yiliang114 closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants