Skip to content

feat(mesh): add programmatic agent launch boundary - #11224

Merged
yiliang114 merged 2 commits into
codex/multi-agent-mesh-foundationfrom
codex/mesh-step-4-runtime-launch
Sep 6, 2026
Merged

feat(mesh): add programmatic agent launch boundary#11224
yiliang114 merged 2 commits into
codex/multi-agent-mesh-foundationfrom
codex/mesh-step-4-runtime-launch

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

This stacked prerequisite adds the runtime seam required by mesh step 4. It launches a configured background agent with a fixed mesh identity, preserves the definition's tool restrictions under the mesh read-only ceiling, returns typed capacity and launch outcomes, and restores the same capability boundary after a cold resume.

Why it's needed

The mesh launcher must reuse the existing background-agent lifecycle without putting mesh-specific dispatch logic into that shared path. Keeping this change in a separate child PR also keeps the shared runtime hot path out of the step-4 host-session PR, as required by the implementation workflow.

Reviewer Test Plan

How to verify

Run the four named core test files. Confirm that a saturated background registry returns capacity_wait without queueing, a missing definition creates no runtime, a valid definition is converted before launch, definition-level execution/disallow restrictions cannot be widened, and a cold-resumed mesh runtime still rejects write tools.

Evidence (Before & After)

N/A — internal runtime contract only.

Tested on

OS Status
🍏 macOS ✅ 218 named tests; targeted ACP bridge package build
🪟 Windows ⚠️ pending #11206 whole-branch CI
🐧 Linux ⚠️ pending #11206 whole-branch CI

Environment (optional)

Node.js workspace with the existing repository dependencies.

Risk & Scope

  • Main risk or tradeoff: This touches the shared background-agent launch and cold-resume paths; normal Agent tool behavior remains on the existing branch and the new path is opt-in.
  • Not validated / out of scope: Hidden host ownership, dispatcher wiring, thread tools, and live-model execution stay in later stacked steps.
  • Breaking changes / migration notes: None.

Linked Issues

Stacked prerequisite for mesh step 4 in #11206.

中文说明

这个 PR 做什么

这是 mesh 第 4 步的 runtime 前置子 PR。它使用固定 mesh 身份启动已配置的后台 agent,在 mesh 只读上限内保留 agent definition 自身的工具收窄规则,返回带类型的容量与启动结果,并在冷恢复后重新建立同一能力边界。

为什么需要

mesh launcher 需要复用现有后台 agent 生命周期,但不应把 mesh 的派发逻辑塞进共享 runtime 路径。按实现工作流把这部分单独放在子 PR,也能确保第 4 步的 host-session PR 不直接修改共享 runtime 热路径。

Reviewer Test Plan

如何验证

运行四个指定的 core 测试文件。确认后台 registry 饱和时返回 capacity_wait 且不入队;definition 缺失时不创建 runtime;有效 definition 在启动前经过转换;definition 自身的 execution/disallow 限制不会被放宽;mesh runtime 冷恢复后仍拒绝写工具。

前后证据

不适用——仅内部 runtime 契约。

测试环境

OS 状态
🍏 macOS ✅ 218 个指定测试;定向 ACP bridge package build
🪟 Windows ⚠️ 等待 #11206 整分支 CI
🐧 Linux ⚠️ 等待 #11206 整分支 CI

环境(可选)

Node.js workspace,使用仓库现有依赖。

风险与范围

  • 主要风险或取舍:改动了共享后台 agent 启动与冷恢复路径;普通 Agent 工具仍走原分支,新路径只在显式调用时启用。
  • 未验证 / 不在范围:隐藏 host 所有权、dispatcher 接线、thread 工具和真实模型运行留在后续 stacked 步骤。
  • 破坏性变更 / 迁移说明:无。

关联事项

#11206 中 mesh 第 4 步的 stacked 前置 PR。

@yiliang114
yiliang114 merged commit 4f21280 into codex/multi-agent-mesh-foundation Sep 6, 2026
60 of 61 checks passed
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 6, 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!

First, where this actually stands: #11224 is already merged — by you, 35 seconds after it was opened — into the stacked branch codex/multi-agent-mesh-foundation, not into main. So this run is a post-merge audit. The gate can't block anything at this point, and I'm not going to approve or request changes on a merged PR. The value here is handing the findings forward to #11206, which is the PR that actually reaches main and the only place this code will ever meet CI.

Template looks good ✓ — every required heading is present and filled in, the Tested-on table is honest about what's still pending, and the Chinese section is a full translation rather than a summary.

Problem: this is a feat, not a fix, so there's no reproduction to ask for and no bug being claimed. The need is architectural: mesh step 4 has to start a durable background agent under a fixed identity without pushing mesh dispatch logic into the shared Agent tool path. That's a real ordering constraint with plan docs committed behind it on the branch, not a hypothetical.

Direction: coherent, and the split is the right call. Keeping the runtime seam in its own child PR is exactly what stops #11206 from becoming one unreviewable change spanning a 4,500-line shared hot path plus the entire mesh subsystem. The one direction-level question I'd want settled before #11206 reaches main is in the review below — the read-only ceiling bounds what a mesh agent can write but not what it can read. That's a product decision, not a code defect, and it may well be intentional at this step.

Size: all 10 files sit under packages/core/src/**, so this is squarely core infrastructure. Breakdown: 296 production logic lines (280 added / 16 removed, across agent.ts, mesh/capability.ts, mesh/launcher.ts, background-agent-resume.ts, config.ts, agents/index.ts, agent-transcript.ts) and 276 test lines (270 added / 6 removed, across three test files). No generated or schema files. It's a feat, so no size block applies, and 296 is under the 500-line escalation threshold and well under the 1000-line advisory.

Approach: the scope feels right. I sketched my own design before reading the diff — thread a narrow programmatic option through the existing invocation instead of duplicating spawn logic, intersect the definition's tools with the mesh ceiling rather than unioning them, persist a mesh marker in the meta sidecar and re-derive the boundary on cold resume — and this is essentially that design, which is a good sign rather than a reason to rubber-stamp it. Nothing drive-by, no formatting churn, no speculative configurability. One modelling question carries into the review: launch_failed currently absorbs both "mesh policy refused this launch shape" and "the runtime genuinely failed", and a step-4/5 dispatcher deciding retry vs. abandon will need those apart.

Risk: no elevated path signals — none of the changed files match the revert-correlated high-risk set. But review depth goes up anyway for a different reason: this commit has no CI signal whatsoever, and the launch-side half of the new security boundary is not covered by any test. Both are detailed in the review below.

Moving on to code review. 🔍

中文说明

感谢贡献!

先说明当前状态:#11224 已经合并——由你在开启 35 秒后自行合并——目标是 stacked 分支 codex/multi-agent-mesh-foundation,而不是 main。所以本次运行属于合并后审计。此时 gate 已无法拦截任何东西,我也不会对已合并的 PR 做 approve 或 request changes。这里的价值在于把结论前移交给 #11206,那才是真正会进入 main、也是这段代码唯一会遇到 CI 的 PR。

模板完整 ✓ ——所有必需小标题都存在且已填写,Tested-on 表格如实标注了仍待验证的项,中文部分是完整翻译而非摘要。

**问题:**这是 feat 而不是 fix,因此不需要复现,也没有声称修复任何 bug。需求是架构性的:mesh 第 4 步必须以固定身份启动一个持久后台 agent,同时不能把 mesh 派发逻辑塞进共享的 Agent tool 路径。这是真实的顺序约束,分支上有相应的规划文档支撑,不是假设。

方向:思路连贯,拆分也是对的。把 runtime seam 单独放进子 PR,正是避免 #11206 变成一个同时横跨 4500 行共享热路径和整个 mesh 子系统、无法评审的巨型改动的关键。在 #11206 进入 main 之前,我希望先有一个答案的方向性问题写在下面的评审里——只读上限限制的是 mesh agent 能写什么,而不是能什么。那是产品决策,不是代码缺陷,在这一步也很可能是有意为之。

**规模:**10 个文件全部位于 packages/core/src/**,属于不折不扣的核心基础设施。明细:296 行生产逻辑(新增 280 / 删除 16,涉及 agent.tsmesh/capability.tsmesh/launcher.tsbackground-agent-resume.tsconfig.tsagents/index.tsagent-transcript.ts),276 行测试(新增 270 / 删除 6,涉及三个测试文件)。无生成或 schema 文件。类型为 feat,因此不适用规模拦截;296 行低于 500 行的升级阈值,也远低于 1000 行的大 PR 提醒线。

**方案:**范围合理。我在读 diff 之前先勾画了自己的设计——在现有 invocation 上穿一个很窄的 programmatic 选项,而不是复制一套 spawn 逻辑;对 definition 的工具做交集而非并集;在 meta sidecar 里持久化 mesh 标记,冷恢复时重新推导边界——而这个 PR 基本就是那套设计。这是个好迹象,但不构成直接放行的理由。没有夹带无关改动,没有格式化噪音,没有投机性的可配置项。有一个建模疑问带入评审:launch_failed 目前同时吸收了"mesh 策略拒绝这种启动形态"和"runtime 真的失败了"两种情况,而第 4/5 步需要决定重试还是放弃的 dispatcher 必须能区分二者。

**风险:**无路径层面的升级信号——改动文件均未命中与 revert 相关的高风险路径集合。但审查深度仍然提高,原因是另一件事:这个 commit 完全没有任何 CI 信号,并且新安全边界的启动侧没有任何测试覆盖。两者都在下面的评审中详述。

进入代码审查 🔍

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

I went in expecting to find a hole in the capability boundary and mostly didn't — the design is careful, and several things that look risky on a first read turn out to be handled. Let me record what I cleared as well as what I didn't, because the cleared list is where a reviewer of #11206 would otherwise spend their time.

Checked and cleared:

  • The buildMeshToolConfig signature change (readonly string[]ToolConfig) is an incompatible change, but it has no consumers to break: every call site on the branch is inside the mesh feature's own new files. ToolConfig.tools is a required field, so definition.tools.includes('*') can't throw, and the FunctionDeclaration.name optionality is handled properly by the name is string predicate rather than cast away.
  • Narrowing is genuinely intersective. executionAllowedTools and disallowedTools from the definition both filter the mesh allow set, so a definition can tighten but never widen past the ceiling, and thread tools are explicitly protected from being disallowed. That's the right shape and it's tested.
  • MCP tools fail closed three times over: classifyMeshTool returns 'deny' for anything absent from the map, so mcp__* never enters tools, never enters executionAllowedTools, and is independently refused by the invocation guard.
  • toolConfig = this.programmatic?.toolConfig sits in the non-fork branch where toolConfig was previously left undefined, so for ordinary Agent tool calls it assigns undefined to a variable that was already undefined. No regression to the normal path.
  • I suspected a bad one here: shouldRunInBackground = backgroundRequested && isTopLevelSession() means a background request downgrades to foreground in a nested session, which would have run the whole agent synchronously and then reported launch_failed because programmaticStatus is only set to 'started' inside the background branch. It isn't reachable — agent.ts rejects an explicit run_in_background: true from a nested session several hundred lines earlier, so a programmatic launch in that context fails closed with the real reason. Worth knowing it's the early guard, not the downgrade path, that saves this.
  • deriveConfig applies overrides generically (Object.create + defineProperty per key), so adding getToolInvocationGuard to the DerivedConfigOverrides union is sufficient — no separate plumbing was missed.
  • Capacity handling doesn't double-book: the canStartBackgroundAgent pre-check is non-booking, and if the internal reservation fails anyway, the programmatic path sets capacity_wait and returns a spawn-blocked result without queueing. The test asserts getQueuedCount() stays 0.

What I'd actually flag:

1. The launch-side half of the boundary has no test. This is the one I'd want addressed before #11206 merges. launcher.ts composes the guard with deriveConfig and hands the result to launchProgrammaticBackgroundAgent, but launcher.test.ts does vi.mock('../../tools/agent/agent.js') and replaces that function wholesale — so the real seam is never crossed in any test. Meanwhile agent.ts gained 72 lines with six distinct this.programmatic branch points (the subagentConfig override, the capacity_wait early return, the agentId override, the toolConfigOverride, the meshAgentId meta write, and the widened executionAllowedTools meta condition) plus a new exported function, and this PR touches no test for it — agent.test.ts exists on the branch and was extended by earlier steps, but has no programmatic-launch coverage. The consequence is specific: the chain deriveConfig(guardedConfig)createApprovalModeOverridecreateAgentHeadless is what makes the mesh guard reach the child scheduler, and it is asserted only on the resume path (background-agent-resume.test.ts calls getToolInvocationGuard() on the config it captured and checks that edit is refused). The launch path — the one that runs first, in production, every time — rests on inheritance working correctly through two config derivations, unverified.

2. No CI ran on this commit, so nothing above was machine-checked. Not a code defect, but it changes what the review is worth: I read this statically, and typecheck / lint / format / the four named test files have never executed against 21c77d04. ci.yml's pull_request trigger is branch-filtered to main and release/**, and this PR targets codex/multi-agent-mesh-foundation, so the suite never fires on stacked child PRs by construction. That's a deliberate tradeoff, but it means a feat touching a shared 4,500-line hot path can merge with zero automated signal — the entire safety net is deferred to #11206. Flagging it here because it's the reason finding 1 stays open rather than being settled by a green run.

3. launch_failed conflates policy refusal with runtime failure. programmaticStatus defaults to 'launch_failed', and execute() has roughly ten buildSpawnBlockedResult early returns that never set it. So "you launched from a nested session, which mesh policy forbids" and "the runtime blew up" both surface as launch_failed, distinguished only by free-text error. The union already models agent_unavailable and capacity_wait as first-class, so a dispatcher in step 4/5 that has to choose retry vs. abandon vs. re-parent will be parsing strings. Cheap to fix now, annoying to fix after callers exist.

4. Question for the product decision, not the code: the ceiling bounds writes, not read scope. checkMeshShellCommand delegates to classifyShellCommandSafetyInDirectory(command, cwd), and inside classifyInternal the cwd is used only for localGitConfigMakesCommandUnsafe — it does not confine which paths a command may touch. Alongside that, read_file, grep, glob and ls are all classified allow with no root restriction. So a mesh agent can read anything the user can (~/.ssh/id_rsa, .env) and then thread_post it to a thread that is by design visible to every participant. The PR explicitly scopes host ownership and workspace binding to later steps, so this may be entirely intentional at this stage — but it should be a named decision on #11206 rather than an emergent property of the classification table, because the shared-thread visibility is what turns ordinary read access into disclosure.

Launch and cold-resume flow, with the tested and untested halves marked
sequenceDiagram
    participant P1 as launchMeshAgent (launcher.ts)
    participant P2 as deriveConfig mesh guard
    participant P3 as launchProgrammaticBackgroundAgent
    participant P4 as AgentToolInvocation.execute
    participant P5 as createApprovalModeOverride
    participant P6 as createAgentHeadless
    participant P7 as BackgroundAgentResumeService

    P1->>P1: loadSubagent then convertToRuntimeConfig
    P1->>P1: buildMeshToolConfig, definition intersect ceiling
    P1->>P2: wrap getToolInvocationGuard
    P2->>P3: guardedConfig
    P3->>P4: new invocation with programmatic options
    P4->>P4: canStartBackgroundAgent, else capacity_wait
    P4->>P5: derive approval config from guardedConfig
    P5->>P6: agentConfig and toolConfigOverride
    P6-->>P3: programmaticStatus started
    Note over P3,P6: no test crosses this seam - launcher.test.ts mocks agent.js
    P7->>P7: meta.meshAgentId triggers guard re-derive
    P7->>P6: toolConfigOverride from buildMeshToolConfig
    Note over P7,P6: covered - resume test asserts the guard denies edit
Loading
Files changed (10 of 10)
File What changed Coverage in this PR
packages/core/src/tools/agent/agent.ts New exported programmatic launch entry, plus a programmatic option threaded through six branch points of the shared invocation none — no test file touched
packages/core/src/agents/mesh/launcher.ts New mesh entry point: loads and converts the definition, checks capacity, composes the guard, returns a typed result unit-tested, but agent.js is mocked out entirely
packages/core/src/agents/mesh/capability.ts Tool-config builder now takes a ToolConfig and intersects execution and disallow restrictions; new invocation guard unit-tested directly
packages/core/src/agents/background-agent-resume.ts Cold resume re-derives the guard and rebuilds the tool override when the meta carries a mesh id unit-tested, asserts a real guard invocation
packages/core/src/agents/agent-transcript.ts Adds an optional durable mesh id to the persisted agent meta none — reached indirectly by the resume test
packages/core/src/config/config.ts Adds the guard accessor to the derived-config override surface none
packages/core/src/agents/index.ts Exports the launcher and mesh types n/a
packages/core/src/agents/mesh/launcher.test.ts New: fixed identity, disabled agent, missing definition, capacity wait
packages/core/src/agents/mesh/capability.test.ts Extended for the new signature, definition restrictions, and the guard
packages/core/src/agents/background-agent-resume.test.ts Extended with the cold-resume ceiling test

Test evidence

This section carries no CI evidence, because none exists for this commit — that is the finding, not a gap in my reporting. Two independent API queries confirm it: actions/runs?head_sha=21c77d04…&event=pull_request returns 0 runs, and grouping all runs on that SHA by event yields 11 runs, every one of them pull_request_target bot orchestration. None of the 73 check-runs on the head belong to Qwen Code CI or E2E Tests.

The cause is structural, not a fluke: ci.yml triggers pull_request only for main and release/**, and this PR's base is codex/multi-agent-mesh-foundation.

Check Conclusion
Qwen Code CI (test, lint_and_static, typecheck) never triggered — base branch outside the main / release/** filter
E2E Tests never triggered — push-to-main and nightly only
pull_request-event workflow runs on head 0
Assign PR owner (pull_request_target) success
PR self-report label (pull_request_target) success
PR Asset Branch Cleanup (pull_request_target) success
🧐 Qwen Pull Request Review (pull_request_target) cancelled
Qwen Triage (pull_request_target) in_progress — this run

Not verified, and why: typecheck, lint, format, and all four named test files — no CI fired on this head. The PR was self-merged by an admin 35 seconds after opening, so there was no window for a run either. The only test numbers in play are the author's own claim of 218 named tests on macOS; that is the author's local result, not evidence I can stand behind, and the PR itself marks Windows and Linux as pending #11206. I also did not execute anything — the review is static by policy, so I have not reproduced the author's run.

Sandboxed verification would settle the open half of this: @qwen-code /verify — that the mesh invocation guard actually reaches the spawned background agent's scheduler on the launch path is not observable from the diff and is asserted by no test, since launcher.test.ts mocks agent.js out and the guard chain crosses two config derivations (deriveConfigcreateApprovalModeOverride) before reaching createAgentHeadless. A load-bearing A/B would be: launch a mesh agent, have it attempt edit and a write-classified shell command, and confirm both are refused — then confirm the same run passes with the guard removed, which is exactly the "green suite that doesn't pin the change" case. Worth aiming at #11206 rather than this merged child, since that PR has a main base and real CI to A/B against. There is no TUI surface here, so /tmux has nothing to drive.

中文说明

代码评审

我本来预期能在能力边界上找到漏洞,结果基本没有——设计是细致的,有几处初看有风险的地方其实已经处理妥当。下面把"已排除"和"未排除"的都记录下来,因为已排除的那部分正是 #11206 的评审者否则会花时间的地方。

已核查并排除:

  • buildMeshToolConfig 的签名变更(readonly string[]ToolConfig)是不兼容改动,但没有消费者会被破坏:分支上所有调用点都在 mesh 特性自己的新文件里。ToolConfig.tools 是必填字段,所以 definition.tools.includes('*') 不会抛异常;FunctionDeclaration.name 的可选性也通过 name is string 谓词正确处理,而不是被强转掩盖。
  • 收窄确实是交集语义。definition 的 executionAllowedToolsdisallowedTools 都是对 mesh allow 集合做过滤,因此 definition 只能收紧、绝不可能放宽越过上限,并且 thread 工具被显式保护、不可被禁用。形态是对的,也有测试。
  • MCP 工具三重失败关闭:classifyMeshTool 对映射表中不存在的任何名称返回 'deny',所以 mcp__* 既进不了 tools,也进不了 executionAllowedTools,还会被调用守卫独立拒绝。
  • toolConfig = this.programmatic?.toolConfig 位于非 fork 分支,而该分支此前从不给 toolConfig 赋值,所以对普通 Agent tool 调用而言,这是把一个本来就是 undefined 的变量再赋成 undefined。常规路径无回归。
  • 我一度怀疑这里有问题:shouldRunInBackground = backgroundRequested && isTopLevelSession() 意味着嵌套会话中后台请求会降级为前台,那样会同步跑完整个 agent,然后又因为 programmaticStatus 只在后台分支里被置为 'started' 而报告 launch_failed。但它不可达——agent.ts 在几百行之前就已拒绝来自嵌套会话的显式 run_in_background: true,所以该场景下的 programmatic 启动会带着真实原因失败关闭。值得知道救下它的是那个前置守卫,而不是降级路径。
  • deriveConfig 以通用方式应用覆盖(Object.create + 逐键 defineProperty),所以把 getToolInvocationGuard 加进 DerivedConfigOverrides 联合类型就足够了——没有漏掉额外接线。
  • 容量处理不会重复预占:canStartBackgroundAgent 预检查不占用名额;即使内部预占失败,programmatic 路径也会置为 capacity_wait 并返回 spawn-blocked 结果而不入队。测试断言 getQueuedCount() 保持为 0。

我真正要提出的:

1. 边界的启动侧完全没有测试。 这是我希望在 #11206 合并前解决的一条。launcher.tsderiveConfig 组合守卫并把结果交给 launchProgrammaticBackgroundAgent,但 launcher.test.ts 执行了 vi.mock('../../tools/agent/agent.js') 把该函数整体替换掉——所以真实的接缝在任何测试中都没有被跨过。同时 agent.ts 新增了 72 行、含六个不同的 this.programmatic 分支点(subagentConfig 覆盖、capacity_wait 提前返回、agentId 覆盖、toolConfigOverridemeshAgentId meta 写入,以及被放宽的 executionAllowedTools meta 条件)外加一个新的导出函数,而本 PR 没有动它的任何测试——agent.test.ts 在分支上存在且被更早的步骤扩展过,但没有 programmatic 启动的覆盖。后果很具体:让 mesh 守卫抵达子调度器的链条是 deriveConfig(guardedConfig)createApprovalModeOverridecreateAgentHeadless,而它在恢复路径上被断言(background-agent-resume.test.ts 在它捕获的 config 上调用 getToolInvocationGuard() 并检查 edit 被拒绝)。启动路径——生产中每次都会先走的那条——依赖两次 config 派生之间的继承正确工作,却未被验证。

2. 这个 commit 没有跑过 CI,因此上面所有内容都未经机器校验。 这不是代码缺陷,但它改变了本次评审的分量:我是静态阅读的,typecheck / lint / format / 四个指定测试文件从未在 21c77d04 上执行过。ci.ymlpull_request 触发被限定在 mainrelease/**,而本 PR 目标是 codex/multi-agent-mesh-foundation,所以 stacked 子 PR 从构造上就不会触发套件。这是有意取舍,但意味着一个改动共享 4500 行热路径的 feat 可以在零自动信号的情况下合并——整个安全网都被推迟到 #11206。在这里提出,是因为这正是第 1 条保持未决、无法被一次绿灯运行解决的原因。

3. launch_failed 把策略拒绝和运行时失败混为一谈。 programmaticStatus 默认值是 'launch_failed',而 execute() 有大约十处 buildSpawnBlockedResult 提前返回从不设置它。于是"你从嵌套会话启动,这是 mesh 策略禁止的"和"运行时崩了"都表现为 launch_failed,只能靠自由文本 error 区分。联合类型已经把 agent_unavailablecapacity_wait 建模为一等公民,所以第 4/5 步需要在重试、放弃、改换父级之间做选择的 dispatcher 将不得不解析字符串。现在改很便宜,等调用方出现之后再改就麻烦了。

4. 一个属于产品决策而非代码的疑问:上限限制的是写,不是读的范围。 checkMeshShellCommand 委托给 classifyShellCommandSafetyInDirectory(command, cwd),而在 classifyInternal 内部,cwd 只用于 localGitConfigMakesCommandUnsafe——它并不限制命令可以触及哪些路径。与此同时 read_filegrepglobls 全部被分类为 allow 且无根目录约束。所以 mesh agent 可以读取用户能读的任何东西(~/.ssh/id_rsa.env),然后 thread_post 到一个按设计对所有参与者可见的 thread。PR 明确把 host 所有权和 workspace 绑定划到后续步骤,所以此阶段这可能是完全有意的——但它应该在 #11206 上成为一个被明确写下的决策,而不是分类表的衍生属性,因为正是共享 thread 的可见性把普通的读取权限变成了信息泄露。

测试证据

本节不携带任何 CI 证据,因为这个 commit 根本没有 CI 证据——这本身就是结论,而不是我汇报上的缺口。两次独立的 API 查询确认了这一点:actions/runs?head_sha=21c77d04…&event=pull_request 返回 0 条运行;把该 SHA 上所有运行按事件分组得到 11 条,每一条都是 pull_request_target 的机器人编排。head 上的 73 个 check-run 没有一个属于 Qwen Code CIE2E Tests

原因是结构性的,不是偶发:ci.yml 只对 mainrelease/** 触发 pull_request,而本 PR 的 base 是 codex/multi-agent-mesh-foundation

未验证项及原因:typecheck、lint、format,以及全部四个指定测试文件——这个 head 上没有触发任何 CI。 该 PR 由一位 admin 在开启 35 秒后自行合并,所以也不存在让运行完成的窗口。唯一存在的测试数字是作者自己声明的 macOS 上 218 个指定测试;那是作者的本地结果,不是我能背书的证据,而且 PR 自己已把 Windows 和 Linux 标为等待 #11206。我也没有执行任何东西——按策略评审是静态的,因此我没有复现作者的运行。

沙箱验证可以解决这里悬而未决的一半:@qwen-code /verify —— mesh 调用守卫在启动路径上是否真的抵达被 spawn 的后台 agent 的调度器,从 diff 上看不出来,也没有任何测试断言,因为 launcher.test.tsagent.js mock 掉了,而守卫链在到达 createAgentHeadless 之前要跨过两次 config 派生(deriveConfigcreateApprovalModeOverride)。一个有承重价值的 A/B 是:启动一个 mesh agent,让它尝试 edit 和一条被分类为写操作的 shell 命令,确认两者都被拒绝;然后确认把守卫移除后同一次运行会通过——这正是"绿灯套件但没有钉住改动"的情形。建议指向 #11206 而不是这个已合并的子 PR,因为那个 PR 的 base 是 main,有真实 CI 可供 A/B。这里没有 TUI 界面,所以 /tmux 无可驱动。

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 3/5 — the design is sound and I cleared eight things I initially suspected, but the launch-side guard chain is asserted by no test and no CI ever ran on this commit, so the bar for auto-approving a core change isn't met.

Going back to the proposal I wrote before reading the diff: this is essentially the design I'd have reached, which I want to be careful not to over-read as evidence of correctness — it mostly means we both looked at the same constraints and picked the obvious seam. Where it exceeds my sketch is the cold-resume half. I had assumed the boundary would be re-derived from the definition at resume; persisting meshAgentId in the meta sidecar and using it as the trigger is better than my version, because it makes the boundary survive a process restart without storing the whole tool config, and the new resume test actually calls the guard and asserts edit is refused rather than just checking the override shape. That's the strongest-tested part of the PR and the part I'd hold up as the model for the rest.

What I can't get past is that I never saw this run, and nobody else has either. That's not hedging — it's the specific shape of the gap. The resume path is tested. The launch path, which is the one that executes first in production and every time thereafter, is tested only against a mock that replaces the function it calls. So the claim "the mesh read-only ceiling holds for a freshly launched mesh agent" is currently supported by reading, by symmetry with the resume path, and by the author's local macOS run. Reading is what I did, and reading is also how I nearly reported a false Critical: I was confident the nested-session background downgrade would silently run a mesh agent to completion in the foreground and then report launch_failed, and it took going back to agent.ts to find the early guard a few hundred lines up that makes it unreachable. If I got one load-bearing inference wrong in a static pass, the honest conclusion is that the pass isn't a substitute for the run, not that I caught everything else.

Six months from now I think I'd mostly thank the author rather than curse them. The typed result unions are the right instinct, the narrowing is intersective so it can only ever tighten, and MCP tools fail closed three separate ways. The maintenance cost is real but proportionate: this.programmatic now changes behaviour at six points inside a 4,500-line class, so AgentToolInvocation has acquired a second personality, and a future reader has to know that. A short doc comment on the field naming all six sites would pay for itself. Related, programmaticStatus is public so the free function can read it back, which is a slightly awkward channel for a result the function already returns — folding it into the return value would be tidier, but that's a preference, not a defect.

The pattern question matters more than this PR does. This is a stacked child of #11206, and steps 5, 6 and beyond will have the same shape: a feat into a feature branch, which ci.yml's main / release/** filter means gets no CI, self-merged by a maintainer within a minute, with the entire safety net deferred to the parent. That's a legitimate way to build a large subsystem, and the parent will get real CI. But it accumulates: a whole subsystem can reach the point of being reviewed on #11206 having never been typechecked or tested as a unit at any intermediate step, and a defect introduced here is then much cheaper to find in this diff than in a 6,800-line one. Worth deciding deliberately whether stacked children should get a workflow_dispatch CI run on demand, rather than discovering the answer when #11206 goes red for a reason that landed three steps ago.

Am I being a pushover here? The temptation is genuine and worth naming: the code is good, the author has admin, it's already merged so nothing I do can block it, and it's "just a prerequisite". That combination is exactly where a gate should not soften, so I'm not approving — and I'm also not requesting changes, because the PR is merged and a CHANGES_REQUESTED review on merged code is theatre that notifies nobody usefully. The findings belong on #11206, where they can still change an outcome.

Deferring rather than approving, and escalating for a human call. The reasons I can't close this out myself: (1) the launch-path guard chain crosses two config derivations before reaching the child scheduler and no test crosses that seam; (2) zero automated checks ran on the merged commit, so I have no machine signal to weigh against my reading; (3) whether the read-only ceiling should also bound read scope — a mesh agent can currently read ~/.ssh or .env and post it to a thread visible to every participant — is a product decision I shouldn't make by inference from a classification table.

I tried to resolve an accountable maintainer deterministically and nothing came back: $QWEN_MAINTAINER_HANDLE is unset, the PR carries no labels so the area map has nothing to match, and there are no reviews on record to fall back to. Per the workflow I'm posting this without an @mention rather than guessing a login — the author (@yiliang114) is the admin who merged this and who opened #11206, so the escalation lands with them by construction, and #11206 is where I'd want findings 1, 3 and 4 addressed before it reaches main.

中文说明

信心度:3/5 —— 设计是扎实的,我排除了八处最初的怀疑,但启动侧的守卫链没有任何测试断言,且这个 commit 从未跑过 CI,因此自动批准核心改动的门槛没有达到。

回到我在读 diff 之前写下的方案:这基本就是我会得出的设计。对此我要小心,不要过度解读为正确性的证据——它主要说明我们俩看到了相同的约束并选择了那个显而易见的接缝。它超出我构想的地方在冷恢复那一半。我原本假设边界会在恢复时从 definition 重新推导;而在 meta sidecar 中持久化 meshAgentId 并用它作为触发条件,比我的版本更好,因为它让边界在进程重启后仍然存活,又不必存储整个 tool config;而且新的恢复测试是真的调用了守卫并断言 edit 被拒绝,而不只是检查 override 的形状。这是整个 PR 中测试最扎实的部分,也是我认为是其余部分应当对齐的范式。

我无法越过的一点是:我从未见过它运行,别人也没有。这不是含糊其辞——这正是缺口的具体形状。恢复路径有测试。启动路径——生产中第一个执行、且此后每次都执行的那条——只针对一个替换掉了它所调用函数的 mock 做了测试。所以"mesh 只读上限对新启动的 mesh agent 成立"这个论断,目前的支撑是阅读、与恢复路径的对称性推断,以及作者在 macOS 上的本地运行。阅读是我做的事,而阅读也正是我差点报出一个假 Critical 的方式:我曾确信嵌套会话的后台降级会静默地在前台把一个 mesh agent 跑完、然后报告 launch_failed,直到回到 agent.ts 才发现几百行之上有一个前置守卫使它不可达。如果我在一遍静态审查中就错了一个承重推断,那么诚实的结论是这遍审查不能替代真实运行,而不是我把其余一切都抓住了。

六个月后,我想我大体上会感谢作者而不是埋怨。带类型的结果联合是正确的直觉,收窄是交集语义所以只可能收紧,MCP 工具以三种独立方式失败关闭。维护成本是真实的但相称:this.programmatic 现在在一个 4500 行的类里的六个位置改变行为,所以 AgentToolInvocation 获得了第二重人格,未来的读者必须知道这一点。在该字段上加一段简短文档注释、点名全部六处,会是值得的。相关地,programmaticStatus 是公开的,以便那个自由函数回读,而作为该函数本就已返回的结果的传递通道,这略显别扭——把它折进返回值会更干净,但那是偏好,不是缺陷。

模式问题比这个 PR 本身更重要。这是 #11206 的 stacked 子 PR,而第 5、6 及后续步骤会是同样的形状:一个 feat 进入特性分支,因 ci.ymlmain / release/** 过滤而得不到任何 CI,由维护者在一分钟内自行合并,整个安全网都推迟到父 PR。这是构建大型子系统的合理方式,父 PR 会拿到真实 CI。但它是会累积的:整个子系统可能在到达 #11206 评审时,在任何中间步骤都从未作为一个整体被 typecheck 或测试过,而此处引入的缺陷在这个 diff 里发现,远比在 6800 行的 diff 里发现便宜。值得刻意决定 stacked 子 PR 是否应该获得一个按需的 workflow_dispatch CI 运行,而不是等到 #11206 因为三步之前落地的原因变红时才发现答案。

我在这里是不是太好说话了?这种诱惑是真实的,也值得点名:代码是好的,作者有 admin 权限,它已经合并所以我做什么都无法拦截,而且它"只是个前置 PR"。这个组合正是 gate 不该软化的地方,所以我不批准——同时我也不 request changes,因为 PR 已合并,对已合并代码提交 CHANGES_REQUESTED 是一场对谁都无用的表演。这些结论应该落在 #11206 上,在那里它们仍然能改变结果。

选择 defer 而非批准,并上升为需要人来判断。 我无法自行收尾的原因:(1)启动路径的守卫链在抵达子调度器之前跨过两次 config 派生,而没有测试跨过这个接缝;(2)已合并的 commit 上零自动检查运行,所以我没有任何机器信号来与我的阅读相互权衡;(3)只读上限是否也应约束读取范围——mesh agent 目前可以读取 ~/.ssh.env 并 post 到一个对所有参与者可见的 thread——是一个产品决策,我不应该靠对分类表的推断来替它做主。

我尝试以确定方式解析出应负责的维护者,但没有结果:$QWEN_MAINTAINER_HANDLE 未设置,PR 不带任何标签所以区域映射无从匹配,也没有可回退的评审记录。按工作流要求,我不猜测登录名,而是不带 @mention 发布本条——作者(@yiliang114)正是合并此 PR 并开启 #11206 的 admin,因此这次上升按构造就落到他们那里,而 #11206 正是我希望在进入 main 之前解决第 1、3、4 条的地方。

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

Reviewed at 21c77d0441753b209376462170a59c6ecc614bbd · 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