Skip to content

feat(mesh): unattended dispatch loop, mid-run steering, and the build fix - #11294

Closed
yiliang114 wants to merge 4 commits into
codex/multi-agent-mesh-foundationfrom
codex/mesh-midrun-delivery
Closed

feat(mesh): unattended dispatch loop, mid-run steering, and the build fix#11294
yiliang114 wants to merge 4 commits into
codex/multi-agent-mesh-foundationfrom
codex/mesh-midrun-delivery

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

Moves direct running delivery and unconsumed-trigger reconciliation out of step 8 and into the working system. Three changes: the dispatcher pushes pending triggers into a running body before it looks for idle work; acceptRunDelivery records that the runtime queue took them; and the terminal write rebooks anything the run was told to answer and never read.

Stacked on #11263.

Why it's needed

A post into a running turn was being silently swallowed. Admission returns coalesce(running) for it, appends it to the live run's triggerMessageIds, and charges a turn. Nothing then delivered it — queueExternalInput had no caller anywhere in mesh — and nothing rebooked it: finishRun had no reconciliation, so when the run ended the message was gone. That is the single failure mode this design says it will not have, and it sat on the path the demo is built around: "the person interjects mid-run" is step 4 of §8.

It is also the one thing this system does that Multica cannot. §0.1 rests on it, and §7's comparison table names it as the only place we are ahead. Leaving it in step 8, behind restart recovery and outbox replay, meant shipping the claim before the capability.

Acceptance is not consumption. The queue taking the input says nothing about the model having read it, so acceptRunDelivery writes acceptedMessageIds and extends the run's context window, and deliberately does not touch consumedMessageIds or the delivery watermark. The correlated drain event in runtime-bridge.ts is still what commits, matched by delivery id rather than guessed at from the text.

The terminal write is the backstop, because it is the last moment at which "this run will never read it" becomes true. A refused delivery therefore costs latency and never a message, which is what makes the at-least-once contract in decision 9 real rather than aspirational.

Only human and system triggers are replayed. An agent-authored post that missed has its author still on the thread, and the turn gate exists precisely to stop two agents re-triggering each other, so replaying one would spend budget repeating a conversation nobody is waiting on.

Reviewer Test Plan

How to verify

cd packages/core
npx vitest run src/agents/mesh/

New cases: steering a running agent (delivery id is the run id; accepted but not consumed; never delivered twice); a refused delivery recorded as delivery_race and rebooked at finish; a consumed trigger not replayed; an agent post not replayed while the human post beside it is; no rebook onto a thread a person closed.

Evidence (Before & After)

Before: post into a running turn → charged, recorded on the run, never delivered, never rebooked. After: delivered at the next tool boundary, or rebooked when the runtime refuses.

Tested on

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

Environment (optional)

Named Vitest files only. Observed: src/agents/mesh/ 14 files / 144 tests (was 139).

Risk & Scope

  • Main risk or tradeoff: deliver is optional on the port, so a future runtime that cannot reach a live body degrades to "wait for the next run" rather than failing. The rebook path covers it either way.
  • Not validated / out of scope: no live run. The drain-event half is exercised by runtime-bridge.ts's existing correlation, not by a new test here.
  • Pre-existing and untouched: runtime-bridge.ts:100 trips no-unsafe-finally (a throw inside finally, from feat(mesh): bind dispatcher runs to agent turns #11252). It is one of the reasons Lint is red on feat(mesh): add persistent shared-thread agent collaboration #11206 and is left for a focused fix rather than mixed in here.

Linked Issues

Parent delivery PR: #11206. Stacked on #11263. Implements the mid-run half of decision 9 and §4's accepted_running / delivery_race.

…of search

#11225 replaced 'if (item)' with 'if (item?.sourceType !== MESH_HOST...)'.
The optional chain yields undefined when the read found nothing, and
undefined !== 'mesh', so a session that vanished between the search hit and
this read now enters the branch and is added as an undefined summary. It
fails tsc, which is why every build on the branch is red, and it would have
been a real regression even if it had compiled: the guard it replaced existed
to drop exactly that case.

Both conditions, in this order.
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.
…d not read

A person posting while their agent is mid-turn is the one thing this system
does that Multica cannot, and it was scheduled with the reliability work in
step 8. That left the path looking supported while it was not: admission
coalesced the post onto the running run, charged a turn for it, and nothing
ever delivered or rebooked it. The message was silently swallowed, which is
the single failure mode this design says it will not have.

The dispatcher now pushes pending triggers into a running body before it
looks for idle work, using the structured input path with the run id as the
delivery id so the drain event can be matched back rather than guessed at
from the text. Acceptance is recorded separately from consumption: the queue
took it, the model has not read it, and the correlated drain event is still
what commits the watermark.

The terminal write is the backstop, because it is the last moment at which
'this run will never read it' becomes true. Anything the run was told to
answer and did not consume is rebooked as one fresh run. Human and system
triggers only: an agent post that missed its target has its author still on
the thread, and the turn gate exists precisely to stop two agents
re-triggering each other, so replaying one would spend budget repeating a
conversation nobody is waiting on.
@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 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@yiliang114 yiliang114 changed the title feat(mesh): deliver into a running turn, and never drop what a run did not read feat(mesh): unattended dispatch loop, mid-run steering, and the build fix Sep 7, 2026
@yiliang114
yiliang114 changed the base branch from codex/mesh-step-7-supervisor to codex/multi-agent-mesh-foundation September 7, 2026 09:26
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Closing: redundant. The foundation already has all three — queueExternalInput in dispatch-port.ts, rebookUndeliveredTriggers in dispatcher.ts, and dispatchOnce driven from acpAgent.ts. I built the same things in parallel without checking first; that is my mistake, not a gap in the branch.

@yiliang114 yiliang114 closed this Sep 7, 2026
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template headings all present ✓. The 中文说明 <details> block from the template is missing — not gating on that, but it is part of the template.

One note before the gate findings, because it shaped this review: the PR changed underneath the review. It started as feat(mesh): deliver into a running turn…, +415/−10 across 8 files against codex/mesh-step-7-supervisor. It is now feat(mesh): unattended dispatch loop, mid-run steering, and the build fix, +851/−19 across 16 files against codex/multi-agent-mesh-foundation. This pass is against the current head, a3146b04.

Problem: observed, and the tree already documented the gap. Not theoretical hardening. bindRunSession on the base branch already carries a doc comment saying the initial prompt "is consumed the moment the turn starts — unlike input pushed into a running turn, which is committed only when the runtime reports draining it", and ThreadRun already had acceptedMessageIds / consumedMessageIds / contextThroughSequence for exactly that split. But nothing ever pushed input into a running turn: I checked every caller of queueExternalInput and they are all in agent.ts, background-agent-resume.ts and background-tasks.ts — none under mesh/. The semantics were specified, the fields were provisioned, the delivery path was absent, and a post into a running turn was charged and then dropped.

Direction: aligned. It closes the gap the design already claims (§0.1, decision 9, §4's accepted_running / delivery_race) and it is the capability the demo path rests on. Worth naming rather than blocking: this PR also reorders the plan of record, moving three items out of step 8 to land ahead of step 7 and editing both plan docs to justify it. You own those docs and the argument in the acceptance note holds, but a plan resequence landing inside an implementation PR is easy to lose track of.

Size: core paths, and now cross-package (packages/core + packages/cli). Production logic 443 lines — supervisor.ts 141, run-lifecycle.ts 64, dispatcher.ts 63, thread-actions.ts 62, serve/server.ts 54, acpAgent.ts 25, dispatch-port.ts 18, agents/index.ts 10, session-list.ts 6 — plus tests 395 and docs 32. Under the 500-line threshold, feat-typed so no Tier 1 block applies, and as a maintainer-authored PR it is exempt from the two-tier core gate in any case. No size escalation.

Approach: the mid-run steering half is what I would have written. I sketched a fix from the title and the "Why it's needed" section before opening the diff — optional deliver on the port, acceptance recorded separately from consumption, terminal write rebooking unread triggers, human/system only — and the implementation matches it, reusing queueExternalInput, consumeRunDelivery, generateRunId, allocateRunSequence and the pre-existing run fields rather than inventing a parallel mechanism.

The scope question is the bundling, and I'd like to ask it plainly rather than block on it: the body still describes the old, narrower PR. It says "Stacked on #11263" and lists three changes, but the diff now absorbs #11263 outright (supervisor.ts +141 and supervisor.test.ts +167), carries the ACP host-session wiring, the daemon-side host owner loop, the barrel exports, and what looks like #11261's session-list.ts existence-check fix — none of which appear anywhere in the description. Cutting the PR back to the mid-run steering it describes would make it reviewable on its own terms; if the bundle is deliberate, the body needs to say so and #11263 / #11261 need to be marked superseded rather than left open beside it.

Two smaller things: the continueResidentAgent(...) reflow in dispatch-port.ts is unrelated formatting churn, and finishRunInTransaction now awaits allocateRunSequence() unconditionally, which persists the workspace file on every finish even when nothing is rebooked.

Risk: two signals, one of them hard. Stage 1e matches packages/cli/src/acp-integration/acpAgent.ts, a revert-correlated path — worth a reviewer's attention on the supervisor lifecycle wiring (start/stop per session, and the discardStoredSessionIfCurrent teardown). More importantly, ci.yml triggers pull_request only on main and release/**, so no unit/lint/typecheck CI runs on this base branch — but tui-parity is not branch-filtered, it did run, and it is red: packages/core does not compile. Details and the exact errors in the review comment.

Flagging these before going deeper — the build break is the one that has to move first.

中文说明

感谢贡献!

模板标题齐全 ✓。模板里的 中文说明 <details> 段落缺失——不以此作为阻塞项,但它确实是模板的一部分。

在门禁结论之前先说明一点,因为它影响了本次审查:这个 PR 在审查过程中发生了变化。 它最初是 feat(mesh): deliver into a running turn…,基于 codex/mesh-step-7-supervisor,8 个文件 +415/−10。现在是 feat(mesh): unattended dispatch loop, mid-run steering, and the build fix,基于 codex/multi-agent-mesh-foundation,16 个文件 +851/−19。本次审查针对当前 head a3146b04

问题:已观测到,而且代码库里早就记录了这个缺口。 不是理论性加固。基线分支上 bindRunSession 的文档注释就写着:初始 prompt "在 turn 开始的那一刻即被视为已消费——不同于推入正在执行 turn 的输入,后者只有在运行时报告已 drain 时才提交",而 ThreadRun 也早就为这个区分准备了 acceptedMessageIds / consumedMessageIds / contextThroughSequence 字段。但从来没有任何代码把输入推进正在执行的 turn:我查了 queueExternalInput 的全部调用方,都在 agent.tsbackground-agent-resume.tsbackground-tasks.tsmesh/ 下一个都没有。语义已定义、字段已就位、投递路径缺失,于是一次发往运行中 turn 的消息被计入 turn 之后就被丢掉了。

方向:一致。 它补上了设计文档已经声称的能力(§0.1、decision 9、§4 的 accepted_running / delivery_race),也正是 demo 路径所依赖的能力。需要点明但不作为阻塞:本 PR 同时调整了计划的顺序,把三个条目从 step 8 移出、提前到 step 7 之前,并修改了两份 plan 文档来说明理由。这些文档由你维护,acceptance 说明里的论证也站得住,但计划重排混在实现 PR 里,后续容易失去线索。

规模: 触及核心路径,且现在是跨包改动(packages/core + packages/cli)。生产逻辑 443 行——supervisor.ts 141、run-lifecycle.ts 64、dispatcher.ts 63、thread-actions.ts 62、serve/server.ts 54、acpAgent.ts 25、dispatch-port.ts 18、agents/index.ts 10、session-list.ts 6——另有测试 395 行、文档 32 行。低于 500 行阈值,类型为 feat 因此不触发 Tier 1 硬阻塞,且作为维护者提交的 PR 本就不受两层核心门禁约束。无需按规模升级。

方案:mid-run steering 这一半换成我也会这么写。 我在打开 diff 之前只根据标题和"Why it's needed"草拟了修法——port 上可选的 deliver、接受与消费分开记录、终态写入时重订未读触发消息、仅限 human/system——实现与之吻合,并且复用了 queueExternalInputconsumeRunDeliverygenerateRunIdallocateRunSequence 和已有的 run 字段,而不是另造一套机制。

范围上的问题在于打包方式,我想直接提出来而不是以此阻塞:PR 描述仍然是那个更窄的旧版本。 它写着"Stacked on #11263"并列出三项改动,但 diff 现在已经把 #11263 整个吸收进来(supervisor.ts +141、supervisor.test.ts +167),还带上了 ACP host-session 接线、daemon 侧的 host owner 循环、barrel 导出,以及看起来属于 #11261session-list.ts 存在性检查修复——这些在描述里一处都没有提到。把 PR 收回它所描述的 mid-run steering 范围,会让它能按自身条款被审查;如果打包是有意的,描述需要说明,并且 #11263 / #11261 应标记为被取代,而不是与它并列继续 open。

两个小点:dispatch-port.tscontinueResidentAgent(...) 的重排属于无关的格式化改动;finishRunInTransaction 现在无条件 await allocateRunSequence(),即使没有需要重订的内容,每次结束都会落盘写一次 workspace 文件。

风险:两个信号,其中一个是硬性的。 Stage 1e 命中 packages/cli/src/acp-integration/acpAgent.ts,这是与回滚相关的路径——supervisor 生命周期的接线值得审查者留意(按 session 启停,以及 discardStoredSessionIfCurrent 的清理)。更重要的是,ci.ymlpull_request 只在 mainrelease/** 上触发,所以这个 base 分支上不会跑单元/lint/类型检查 CI——但 tui-parity 没有分支过滤,它确实跑了,而且是红的:packages/core 编译不过。 具体错误见代码审查评论。

先把这些提出来再往下深入——编译失败是必须最先解决的那一项。

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

Evidence carried here: the PR's own CI checks and job logs, read through the API, plus a static read of the tree at the reviewed commit. I did not build, run or execute any PR-derived code — this review is static, per the gate's no-execute rule.

Blocker: packages/core does not compile

src/agents/mesh/run-lifecycle.ts(398,8): error TS2552: Cannot find name 'ThreadRun'. Did you mean 'Thread'?
src/agents/mesh/run-lifecycle.ts(405,14): error TS7006: Parameter 'id' implicitly has an 'any' type.
src/agents/mesh/run-lifecycle.ts(406,11): error TS7006: Parameter 'id' implicitly has an 'any' type.
src/agents/mesh/run-lifecycle.ts(408,8): error TS7006: Parameter 'message' implicitly has an 'any' type.
src/agents/mesh/run-lifecycle.ts(414,19): error TS2552: Cannot find name 'ThreadRun'. Did you mean 'Thread'?
src/agents/mesh/run-lifecycle.ts(418,36): error TS7006: Parameter 'message' implicitly has an 'any' type.

I confirmed this against a3146b04 rather than taking the log's word for it. run-lifecycle.ts:36 reads import type { Thread, ThreadEvent, ThreadMessage } from './types.js';ThreadRun is not in that list — while the new rebookUnconsumedTriggers uses it twice, at line 398 (run: ThreadRun) and line 414 (const rebooked: ThreadRun = {). The four TS7006 errors are the cascade: with run unresolved, run.consumedMessageIds and run.triggerMessageIds are any, so the id and message callback parameters lose their types under noImplicitAny. Adding ThreadRun to that one import should clear all six.

This is worth dwelling on because it explains how the PR's own test plan looked green. npx vitest run src/agents/mesh/ transpiles with esbuild, which strips types without checking them, so all 144 tests can pass while tsc fails — the reported "14 files / 144 tests" and this build break are both true. AGENTS.md's workflow step 3 asks for npm run build && npm run typecheck alongside the unit tests for exactly this reason. It also means the type-correctness of the packages/cli half is unknown, not clean: the build died in packages/core, so acpAgent.ts, serve/server.ts and session-list.ts were never reached by the compiler.

Ironic given the title advertises "the build fix" — the two test fixes bundled here (capability.test.ts, dispatch-port.test.ts) do repair base-branch breakage, but the PR's own new code introduces a fresh compile error.

The mechanism itself is sound

I traced the steering path end to end against the base-branch machinery rather than accepting the description. deliver passes deliveryId: run.id; runtime-bridge.ts:39 correlates the drain on exactly that (event.deliveryId === input.context.runId); consumeRunDelivery then promotes acceptedMessageIds wholesale into consumedMessageIds, which is what rebookUnconsumedTriggers filters against. So a steer the model actually read does get committed and does not get rebooked. Dedup in the dispatcher rides on the accepted set rather than the watermark, which is the right choice — see the second note below. attempts: 0 on the rebooked run matches the existing convention (runs are created at 0, claimRun increments), and bindRunSession's consumedOnStart path means the launch trigger is already consumed, so the rebook won't replay it.

  • The invariant that separates this from answering every interjection twice is untested. Every new test drives a fake port whose deliver never causes a drain event, so the accepted → consumed promotion is never exercised. The refused-delivery test covers the rebook path; the "nothing is delivered twice" assertion only counts driver.deliver calls inside one dispatcher pass. So "a steered message ends up in consumedMessageIds and is therefore not rebooked at finish" has no test. The body is upfront about this ("the drain-event half is exercised by runtime-bridge.ts's existing correlation, not by a new test here") — I verified that correlation exists and lines up, but that is a static read, not evidence.

Non-blocking, in rough order of how much I'd want them addressed:

  • acceptRunDelivery throws on a race that is bound to happen. It requires run.status === 'running' strictly, while its sibling consumeRunDelivery accepts 'running' || 'finishing'. It is called with the stale run snapshot read by listThreads() at the top of dispatchOnce, after await port.deliver(...) and after acquiring the workspace lock. If the run reached finishing or terminal in that window — the registry can still report running, so deliver returns true — it throws. The steering pass runs before the candidate loop and dispatcher.ts has no try/catch, so dispatchOnce rejects and the whole tick aborts: runPass catches it into {kind:'error'}, logs dispatch pass failed, and every queued candidate behind it waits for the next tick. Self-heals in ~2 s and the message is still rebooked at finish, so this is latency plus recurring log noise rather than loss — but recording a delivery miss instead of throwing would remove it, and would make the finishing asymmetry between the two functions deliberate rather than accidental.
  • consumeRunDelivery regresses contextThroughSequence. It writes back through = context.contextThroughSequence ?? run.contextThroughSequence, and context is the launch-time binding, so the value acceptRunDelivery just extended is overwritten with the older snapshot (2 → 1 in the simplest case). Inert today — the run is about to go terminal, prompt assembly reads deliveryByAgent.committedThroughSequence rather than run.contextThroughSequence, and dedup rides on acceptedMessageIds — but it makes a documented high-water mark non-monotonic, and a future change that trusts the watermark instead of the accepted set would silently double-deliver. Worth a comment at minimum.
  • The rebooked run bypasses the budget gate. It is written straight into the store as queued, and selectCandidates applies no budget check either, so it starts even if the thread tree has since exhausted its token budget. Defensible — the work was already admitted and charged once, and not re-charging autoTurnsUsed is correct, matching the human-post path — but dispatch-policy.ts calls the budget rules "the difference between a working mesh and a token fire" and says they fail closed. One line confirming the deliberate overspend would settle it.
  • capability.test.ts loosens the assertion it repairs. It was "no tool in ToolNames classifies as thread"; it is now "no tool in ToolNames other than the named thread tools classifies as thread", which excludes by name the very things it would have caught and leaves the second assertion as the only real check. If thread tools genuinely moved into ToolNames, filtering is the honest correction — but say so in the PR body, since a build fix that narrows a boundary test is the shape worth a second look.
  • Some of the new barrel exports have no consumer. agents/index.ts adds eight; acpAgent.ts imports three (createMeshDispatchPort, startMeshSupervisor, MeshSupervisor). dispatchOnce, DispatchRecord, MeshTickOutcome and DEFAULT_MESH_SUPERVISOR_INTERVAL_MS are exported across the package boundary with nothing reading them yet.
  • allocateRunSequence() is now unconditional per finish, and it persists the workspace file on every call, so every run completion writes the workspace record even when there is nothing to rebook. Minor write amplification; allocating lazily would need the missed set computed first.

Verified rather than accepted: the no-unsafe-finally problem the body calls pre-existing is genuinely pre-existing — this PR does not touch runtime-bridge.ts, and the throw inside finally is present at the base commit.

sequenceDiagram
    participant P1 as Person posting mid-turn
    participant P2 as Admission
    participant P3 as Dispatcher steering pass
    participant P4 as Runtime queue
    participant P5 as Running agent body
    participant P6 as runtime-bridge drain
    participant P7 as Terminal write
    P1->>P2: post into a thread with a live run
    P2->>P2: coalesce(running), append trigger, charge a turn
    P3->>P4: deliver(text, deliveryId = run id)
    alt queue took it
        P4-->>P3: true
        P3->>P3: acceptRunDelivery - accepted ids plus context window, not consumed
        P4->>P5: drained between tool rounds
        P5->>P6: EXTERNAL_MESSAGE with that delivery id
        P6->>P6: consumeRunDelivery - accepted promoted to consumed
        P6->>P7: finishRun sees the trigger consumed, no rebook
    else queue refused (run finishing or body moved)
        P4-->>P3: false, recorded as delivery_race
        P3->>P7: trigger never accepted
        P7->>P7: rebookUnconsumedTriggers - human and system only, never onto a done thread
    end
Loading
Files changed (16)
File What changed
packages/core/src/agents/mesh/run-lifecycle.ts Adds the terminal-write rebook; does not compileThreadRun used but never imported
packages/core/src/agents/mesh/thread-actions.ts Adds acceptRunDelivery: records acceptance and extends the context window, deliberately not consumption
packages/core/src/agents/mesh/dispatcher.ts New steering pass ahead of the candidate loop, plus the deliver port method and two result kinds
packages/core/src/agents/mesh/dispatch-port.ts Implements deliver over queueExternalInput using the structured form so the delivery id survives; unrelated reformat of one call
packages/core/src/agents/mesh/supervisor.ts The whole dispatch loop from #11263, now folded into this PR rather than stacked under it
packages/core/src/agents/mesh/supervisor.test.ts Tests for that loop — also #11263's
packages/core/src/agents/index.ts Barrel exports for the dispatcher, port and supervisor; three of the eight have a consumer
packages/cli/src/acp-integration/acpAgent.ts Starts and stops one supervisor per mesh host session — a Stage 1e revert-correlated path
packages/cli/src/serve/server.ts Daemon-side host owner: keeps one hidden host resident per rostered, trusted workspace on the keepalive cadence
packages/cli/src/serve/server/session-list.ts Restores the existence check when filtering host sessions out — looks like #11261
packages/core/src/agents/mesh/capability.test.ts Repairs a boundary assertion by filtering thread tool names out first
packages/core/src/agents/mesh/dispatch-port.test.ts Adds the sidecar storage mock the base branch started needing
packages/core/src/agents/mesh/dispatcher.test.ts Steering and refused-delivery cases
packages/core/src/agents/mesh/run-lifecycle.test.ts Rebook cases: consumed not replayed, agent post not replayed, nothing rebooked onto a done thread
docs/plans/2026-09-06-multi-agent-board-collaboration.md Resequences step 8, moving three items ahead of step 7
docs/plans/2026-09-07-mesh-implementation-acceptance.md Records that mid-run steering landed early, with the observed test count

Test evidence — the PR's own CI

ci.yml triggers pull_request only on base branches main and release/**, so with this PR based on codex/multi-agent-mesh-foundation the unit, lint and typecheck lanes never fired. tui-parity is not branch-filtered, so it did run — and both of its jobs failed while building, which is the only automated signal this PR has and it is red. Nothing was polled or waited on; this is the complete picture as of the fetch.

Check Conclusion
TUI parity snapshots (ink vs opentui) failurenpm run build --workspace=packages/core exit 1, 6 TS errors in run-lifecycle.ts
OpenTUI no-flicker gate failure — same 6 TS errors, same build step
test (unit, ci.yml) not triggered — base branch outside main / release/**
lint_and_static (ci.yml) not triggered — base branch outside main / release/**
classify_pr (ci.yml) not triggered — base branch outside main / release/**
assign success
label success
authorize success
Remind on force-push success
delete-asset-branch success
delay-automatic-review cancelled (superseded by the force-push)
review-pr cancelled (superseded by the force-push)

The two failures are the same defect reproduced independently by two jobs, not flake: identical file, identical six errors, identical failing build step, on the current head. Classified as PR-caused from the diff and the compiler output — run-lifecycle.ts is a file this PR modifies and the missing symbol is one this PR introduces — not from anything the log body claims.

Not verified: that packages/cli typechecks (the build never reached it); that the mesh suite passes on a clean install (the author's 144-test count is from vitest, which does not typecheck, and is the author's claim about their own machine rather than evidence this run can stand behind); and every behavioural claim above, since no lane ran the changed code.

Sandboxed verification would settle the behavioural half: @qwen-code /verify — that a message steered into a running turn reaches consumedMessageIds and is therefore not rebooked at finish is the whole difference between at-least-once and answering every interjection twice, and it is observable neither from the diff nor from this PR's tests, since every fake port here returns without emitting a drain event. Note that a /verify run would also have caught the compile error in its base build step. @qwen-code /tmux is the lane for driving the mid-run interjection as a real user against the live host, and matters more than usual here given the acp-integration wiring is new. Neither will substitute for fixing the build first — and until this PR targets main or a release/** branch, ci.yml will keep not running on it at all.

中文说明

代码审查

本评论携带的证据:通过 API 读取的本 PR 自身 CI 检查与作业日志,外加对审查 commit 上代码树的静态阅读。我没有构建、运行或执行任何源自 PR 的代码——依据 gate 的「不执行」规则,本次审查为静态审查。

阻塞项:packages/core 编译不过

(六条编译错误见上方代码块,中文不重复。)

我是对照 a3146b04 核实过的,没有只采信日志。run-lifecycle.ts:36import type { Thread, ThreadEvent, ThreadMessage } from './types.js';——列表里没有 ThreadRun——而新增的 rebookUnconsumedTriggers 用了它两次,分别在 398 行(run: ThreadRun)和 414 行(const rebooked: ThreadRun = {)。四条 TS7006 是连带结果:run 无法解析,run.consumedMessageIdsrun.triggerMessageIds 就成了 any,于是在 noImplicitAnyidmessage 回调参数失去类型。把 ThreadRun 加进那一条 import,六条错误应该都会消失。

这一点值得多说几句,因为它解释了 PR 自己的测试计划为什么看起来是绿的。npx vitest run src/agents/mesh/ 用 esbuild 转译,只剥离类型而不做检查,所以 144 条测试全过的同时 tsc 可以失败——报告里的「14 files / 144 tests」和这个编译失败两件事同时为真。AGENTS.md 工作流第 3 步要求在单元测试之外一并跑 npm run build && npm run typecheck,正是因为这个原因。这也意味着 packages/cli 那一半的类型正确性是未知,而非干净:构建死在 packages/core,编译器根本没有走到 acpAgent.tsserve/server.tssession-list.ts

考虑到标题里写着「the build fix」,这有点讽刺——这里打包的两个测试修复(capability.test.tsdispatch-port.test.ts)确实修好了基线分支的破损,但 PR 自己新增的代码引入了一个新的编译错误。

机制本身是成立的

我对照基线分支的既有机制端到端追了 steering 链路,没有只采信描述。deliver 传入 deliveryId: run.idruntime-bridge.ts:39 正是用它做 drain 关联(event.deliveryId === input.context.runId);随后 consumeRunDeliveryacceptedMessageIds 整体提升进 consumedMessageIds,而这正是 rebookUnconsumedTriggers 的过滤依据。所以模型确实读到的 steer 会被提交,也不会被重订。dispatcher 里的去重依赖 accepted 集合而非 watermark,这个选择是对的——原因见下面第二条。重订 run 上的 attempts: 0 与既有约定一致(run 创建时为 0,由 claimRun 递增),而 bindRunSessionconsumedOnStart 路径意味着启动触发消息已被消费,因此重订不会把它重放一遍。

  • 区分「至少一次」与「每次插话都回答两遍」的那条不变量没有测试。 所有新增测试用的都是 fake port,其 deliver 从不触发 drain 事件,因此 accepted → consumed 的提升路径从未被执行到。投递被拒的测试覆盖了 rebook 路径;「不会重复投递」的断言只统计了单个 dispatcher pass 内 driver.deliver 的调用次数。所以「被 steer 的消息最终会进入 consumedMessageIds、因而在结束时不会被重订」没有测试。描述对此是坦率的(「drain 事件那一半由 runtime-bridge.ts 既有的关联逻辑覆盖,这里没有新增测试」)——我确认了这个关联确实存在且对得上,但那是静态阅读,不是证据。

以下为非阻塞项,大致按我希望被处理的优先级排列:

  • acceptRunDelivery 会在一个必然会发生的竞态上抛异常。 它严格要求 run.status === 'running',而姊妹函数 consumeRunDelivery 接受 'running' || 'finishing'。它被调用时传入的是 dispatchOnce 开头 listThreads() 读到的过期 run 快照,中间还经过了 await port.deliver(...) 和获取 workspace 锁。如果在这个窗口内 run 进入了 finishing 或终态——registry 仍可能报告 running,于是 deliver 返回 true——它就会抛错。steering pass 位于 candidate 循环之前,而 dispatcher.ts 没有任何 try/catch,所以 dispatchOnce 整体 reject、本次 tick 中止:runPass 把它兜成 {kind:'error'}、打出 dispatch pass failed,排在后面的所有 queued candidate 都要等下一个 tick。约 2 秒后自愈,消息在结束时仍会被重订,所以这是延迟加反复的日志噪音而非丢失——但把投递未命中记录下来而不是抛错就能消除它,也能让两个函数之间对 finishing 的处理差异变成有意为之。
  • consumeRunDelivery 会让 contextThroughSequence 回退。 它写回 through = context.contextThroughSequence ?? run.contextThroughSequence,而 context 是启动时的 binding,于是 acceptRunDelivery 刚扩展的值被更旧的快照覆盖(最简单场景下 2 → 1)。今天无害——run 即将终态,prompt 组装读的是 deliveryByAgent.committedThroughSequence 而非 run.contextThroughSequence,去重也依赖 acceptedMessageIds——但这让一个被文档描述为高水位的字段变成非单调的;将来若有改动信任 watermark 而非 accepted 集合,就会静默重复投递。至少值得加一句注释。
  • 重订的 run 绕过了预算门禁。 它被直接以 queued 写入 store,而 selectCandidates 也不做预算检查,所以即便该 thread tree 的 token 预算已耗尽仍会启动。可以辩护——这份工作此前已被 admit 并计费过一次,且重复增加 autoTurnsUsed 是正确的,与 human 发帖路径一致——但 dispatch-policy.ts 把预算规则称为「一个能用的 mesh 与一场 token 火灾之间的区别」并声明其 fail closed。补一句确认这个超支是有意为之即可。
  • capability.test.ts 在修复断言的同时放松了它。 原来是「ToolNames 里没有任何工具被分类为 thread」;现在是「ToolNames 里除了这些被点名的 thread 工具之外,没有工具被分类为 thread」,也就是按名字排除了它本来要抓的东西,只剩第二条断言是真正的检查。如果 thread 工具确实迁进了 ToolNames,那么过滤是诚实的修正——但请在 PR 描述里说明,因为一个顺手收窄了边界测试的「build fix」正是值得多看一眼的形状。
  • 部分新增的 barrel 导出没有消费者。 agents/index.ts 新增了八个,acpAgent.ts 只导入三个(createMeshDispatchPortstartMeshSupervisorMeshSupervisor)。dispatchOnceDispatchRecordMeshTickOutcomeDEFAULT_MESH_SUPERVISOR_INTERVAL_MS 被导出到包边界之外,目前无人读取。
  • allocateRunSequence() 现在每次 finish 都无条件调用,而它每次调用都会落盘 workspace 文件,因此即使没有需要重订的内容,每次 run 结束也都会写一次 workspace 记录。属于轻微写放大;若要惰性分配,需要先把 missed 集合算出来。

核实而非采信: 描述中称为既有问题的 no-unsafe-finally 确实是既有问题——本 PR 没有改动 runtime-bridge.ts,而 finally 里的 throw 在基线 commit 上就已存在。

(时序图与文件清单见上,中文不重复:人在 turn 执行中发帖 → admission 合并到运行中的 run 并计入一次 turn → dispatcher 的 steering pass 以 run id 为 deliveryId 投递 → 队列接受则记为 accepted 并扩展上下文窗口(非 consumed),body 在 tool 轮次之间 drain,runtime-bridge 按 delivery id 关联并把 accepted 提升为 consumed,结束时不重订;队列拒绝则记为 delivery_race,由终态写入重订,且仅限 human/system、不会重订到已 done 的 thread。)

测试证据 —— PR 自身的 CI

ci.ymlpull_request 只在 base 为 mainrelease/** 时触发,因此本 PR 基于 codex/multi-agent-mesh-foundation,单元、lint 与类型检查通道全都没有触发。tui-parity 没有分支过滤,所以它确实跑了——而它的两个作业都在构建阶段失败。这是本 PR 唯一的自动化信号,而且它是红的。没有做任何轮询或等待;以上就是抓取时刻的完整情况。

(CI 表格见上方机器可读区域,中文不重复。)

这两个失败是同一个缺陷被两个作业各自独立复现,不是 flake:相同的文件、相同的六条错误、相同的失败构建步骤,且都在当前 head 上。判定为 PR 引入,依据是 diff 与编译器输出——run-lifecycle.ts 是本 PR 改动的文件,缺失的符号也是本 PR 引入的——而不是日志正文里的任何说法。

未验证: packages/cli 能否通过类型检查(构建没有走到);mesh 测试套件在干净安装下是否通过(作者的 144 条测试来自不做类型检查的 vitest,且那是作者关于自己机器的说法,不是本次运行能够背书的证据);以及上面所有行为性结论,因为没有任何通道运行过被改动的代码。

沙箱验证可以了结行为性那一半:@qwen-code /verify——一条被 steer 进运行中 turn 的消息是否会进入 consumedMessageIds、从而在结束时被重订,正是「至少一次」与「每次插话都回答两遍」之间的全部区别;它既无法从 diff 观察到,也无法从本 PR 的测试观察到,因为这里每个 fake port 都是直接返回、不发 drain 事件。需要说明的是,一次 /verify 运行本身也会在它的基线构建步骤里抓到这个编译错误。@qwen-code /tmux 是以真实用户身份针对 live host 驱动运行中插话的通道,考虑到这里的 acp-integration 接线是新增的,它比平时更重要。两者都不能替代先把构建修好——而且在这个 PR 改为面向 mainrelease/** 分支之前,ci.yml 会一直完全不运行。

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 2/5 — the design is good and I could follow every argument it makes, but it does not compile, and that is not a judgement call.

Stepping back: the mid-run steering half is the best-reasoned part of this stack. I wrote my own proposal before opening the diff and the implementation matched it, then beat it on reuse — it wires into queueExternalInput, consumeRunDelivery and run fields that already existed rather than building a parallel delivery mechanism, and the distinction between acceptance and consumption is exactly right and exactly what the base branch's own doc comment had already promised. I traced the drain correlation and it holds. If the question were "is this the right design", the answer is yes.

The question is not that, though. packages/core fails to build with six TypeScript errors from one missing type import, reproduced independently by both tui-parity jobs on the current head. A PR that does not compile cannot merge, and the interesting part is how it got here: the test plan in the body is npx vitest run src/agents/mesh/, and vitest transpiles without typechecking, so a green 144-test run and a broken build are entirely compatible. The one lane that would have caught it — ci.yml's build and static analysis — does not run on this PR at all, because ci.yml only triggers pull_request against main and release/**. So this is not carelessness so much as a stack that has no gate underneath it, and it happened on the PR whose title advertises a build fix.

That is the thing I would want a human to weigh, beyond the one-line fix. This PR started as a focused 8-file change and is now a 16-file, two-package bundle that has absorbed #11263's supervisor wholesale and what looks like #11261's session-list fix, while its body still says "Stacked on #11263" and still describes only the three original changes. Those two PRs are still open beside it. Nobody reading the description can tell what is actually under review, and the diff now touches acp-integration, a path this repo's own revert history flags. I am not blocking on scope — the bundling may well be the pragmatic way to move a stack that cannot get CI — but the description has to match the diff before anyone can review it honestly, and the superseded PRs need closing or marking.

Six months from now I would thank the author for the comments in this code; they explain why acceptance is not consumption and why the terminal write is the backstop, which is the part a future maintainer actually needs. I would also want the stack to stop absorbing its own neighbours silently.

What would move this to an approve: fix the ThreadRun import and confirm npm run build && npm run typecheck passes across both packages, not just vitest; bring the body in line with the diff and settle #11263 / #11261; and add one test that drives the drain event, so the accepted → consumed → not-rebooked chain is pinned by something rather than by my reading of it. The non-blocking notes in the review comment — the acceptRunDelivery throw on a predictable race, the regressing watermark, the budget bypass — are worth a look but none of them should hold the PR up on their own.

Requesting changes on the build break. Everything else is discussion.

中文说明

Confidence: 2/5 —— 设计是好的,它提出的每一个论点我都能跟上,但它编译不过,而这一点没有商量余地。

退一步看:mid-run steering 这一半是整个堆叠里论证最扎实的部分。我在打开 diff 之前先写了自己的方案,实现与之一致,而且在复用上做得比我的方案更好——它接入了既有的 queueExternalInputconsumeRunDelivery 和已经存在的 run 字段,而不是另建一套投递机制;「接受」与「消费」的区分完全正确,也正是基线分支自己的文档注释早就承诺过的语义。drain 关联我追过了,成立。如果问题是「这个设计对不对」,答案是对。

但问题不在这里。packages/core 构建失败,六条 TypeScript 错误全部来自一个缺失的类型 import,并且在当前 head 上被 tui-parity 的两个作业各自独立复现。一个编译不过的 PR 无法合并,而更有意思的是它怎么走到这一步的:描述里的测试计划是 npx vitest run src/agents/mesh/,而 vitest 只转译不做类型检查,所以「144 条测试全绿」和「构建失败」完全可以同时成立。唯一本该抓到它的通道——ci.yml 的构建与静态分析——在这个 PR 上根本不运行,因为 ci.ymlpull_request 只在 base 为 mainrelease/** 时触发。所以这与其说是疏忽,不如说是这个堆叠底下没有任何门禁,而它恰好发生在一个标题里写着 build fix 的 PR 上。

这才是我希望人来权衡的地方,而不只是那一行修复。这个 PR 最初是一个聚焦的 8 文件改动,现在是 16 文件、跨两个包的打包,已经把 #11263 的 supervisor 整个吸收进来,还带上了看起来属于 #11261 的 session-list 修复,而它的描述仍然写着「Stacked on #11263」、仍然只描述原来的三项改动。那两个 PR 还在旁边 open 着。任何人读描述都无法判断实际被审查的是什么,而 diff 现在触及了 acp-integration——本仓库自己的回滚历史标记过的路径。我不以范围为由阻塞——对于一个拿不到 CI 的堆叠来说,打包也许正是务实的推进方式——但在任何人能够诚实审查之前,描述必须与 diff 一致,被取代的 PR 也需要关闭或标注。

六个月后我会感谢作者写在这些代码里的注释;它们解释了「为什么接受不等于消费」、「为什么终态写入是最后的兜底」,而这正是未来维护者真正需要的部分。我也会希望这个堆叠不要再静默地把自己的邻居吸收进来。

能把它推向 approve 的是:修掉 ThreadRun 的 import,并确认 npm run build && npm run typecheck 在两个包上都通过,而不只是 vitest;让描述与 diff 一致,并把 #11263 / #11261 处理掉;再加一个真正驱动 drain 事件的测试,让 accepted → consumed → 不重订 这条链路被某个东西钉住,而不是只靠我的阅读。审查评论里那些非阻塞项——acceptRunDelivery 在可预见竞态上抛异常、watermark 回退、绕过预算门禁——值得看一眼,但任何一项都不足以单独卡住这个 PR。

以编译失败为由请求修改。其余都是讨论。

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

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

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a fix before this can go anywhere — packages/core does not compile.

run-lifecycle.ts:36 imports { Thread, ThreadEvent, ThreadMessage } but the new rebookUnconsumedTriggers uses ThreadRun at lines 398 and 414, so tsc fails with six errors (two TS2552, four cascading TS7006 under noImplicitAny). Adding ThreadRun to that import should clear all of them. Both tui-parity jobs hit this independently on a3146b04.

The reason it slipped through is worth noting: npx vitest run src/agents/mesh/ transpiles with esbuild and never typechecks, so 144 green tests and a broken build are both true. Please confirm npm run build && npm run typecheck across packages/core and packages/cli — the build died in core, so the CLI half (acpAgent.ts, serve/server.ts, session-list.ts) has never been compiled by any gate.

Second thing, not a blocker but it needs settling before anyone can review this honestly: the body still describes the earlier, narrower PR. It says "Stacked on #11263" and lists three changes, while the diff has absorbed #11263's supervisor.ts outright, picked up what looks like #11261's session-list.ts fix, added the ACP and daemon wiring, and grown from 8 files to 16 across two packages. Both of those PRs are still open beside this one.

The design itself is good — I traced the acceptance-is-not-consumption chain through runtime-bridge.ts and it holds. Full notes, including the untested drain path and four non-blocking observations, are in the review comment above.

中文说明

这个 PR 需要先修一处才能继续——packages/core 编译不过。

run-lifecycle.ts:36 导入的是 { Thread, ThreadEvent, ThreadMessage },但新增的 rebookUnconsumedTriggers 在 398 行和 414 行用到了 ThreadRun,于是 tsc 报出六条错误(两条 TS2552,四条在 noImplicitAny 下连带产生的 TS7006)。把 ThreadRun 加进那条 import 应该就能全部消除。tui-parity 的两个作业都在 a3146b04 上独立撞到了这个问题。

它之所以漏过去,原因值得记一下:npx vitest run src/agents/mesh/ 用 esbuild 转译、从不做类型检查,所以「144 条测试全绿」和「构建失败」两件事同时为真。请确认 npm run build && npm run typecheckpackages/core packages/cli 上都通过——构建死在 core,所以 CLI 那一半(acpAgent.tsserve/server.tssession-list.ts)从未被任何门禁编译过。

第二件事,不是阻塞项,但在任何人能够诚实审查之前需要先理清:PR 描述仍然写的是那个更早、更窄的版本。它写着「Stacked on #11263」并列出三项改动,而 diff 已经把 #11263supervisor.ts 整个吸收进来、带上了看起来属于 #11261session-list.ts 修复、新增了 ACP 与 daemon 接线,并从 8 个文件跨两个包增长到 16 个文件。这两个 PR 都还在旁边 open 着。

设计本身是好的——我沿着「接受不等于消费」这条链路追过了 runtime-bridge.ts,它是成立的。完整意见(包括没有测试覆盖的 drain 路径和四条非阻塞观察)见上方的审查评论。

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

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

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