feat(core): let a workflow agent pin a directory and outlive the default bounds - #8972
Conversation
…ult bounds
Three gaps that together keep workflow subagents to short, in-place work.
**`agent({workingDir})`.** A script had no way to run an agent inside a
directory. `isolation: 'worktree'` is not a substitute: it CREATES a
worktree from the current tree and refuses to run when the parent tree is
dirty — the opposite of pinning an agent to a directory whose uncommitted
state is the point (a review worktree, a checkout a previous step
provisioned). `workingDir` is the same contract `AgentTool` already
exposes as `working_dir`: an existing, caller-owned worktree that the
harness neither creates nor removes.
Two details are easy to get wrong and both are covered:
- The fast path hands `config` to `AgentHeadless` untouched and cannot
honour a rebind, so `workingDir` forces the override path. Left on the
fast path it would be dropped in silence and the agent would run in the
parent tree — the failure the option exists to prevent.
- `canonicalizeAgentOpts` now projects `workingDir`. The same prompt run
against two worktrees is two different questions; without the
projection a resume that changed only the directory would replay the
previous tree's answers as this one's.
The validation moves to `agents/worktree-pin.ts`, shared with `AgentTool`
rather than duplicated: the path comes from a model either way, and
pinning replaces the child's `WorkspaceContext` wholesale, so it must
resolve inside the repository and be a registered linked worktree. The
caller passes the parameter name so errors say `workingDir` to a script
and `working_dir` to a tool call.
**Tunable per-subagent bounds.** `max_turns: 50` / `max_time_minutes: 10`
were hard-pinned at both dispatch sites with no override, while the three
other workflow bounds all have one. A build-and-test agent, or an
analysis of a 2 000-line file, exceeds them routinely — and under the
GOAL-terminal contract being cut off surfaces as a `null` element, an
agent that silently went missing rather than one that visibly failed.
Both are now env-tunable and clamped, and the doc comment states how
`stallMs`, `max_time_minutes` and the run wall clock differ, since
raising one without the others just moves which limit kills the run.
**Headless regression test.** A foreground `Workflow` call must complete
with no interactive session and no completion channel: `qwen --prompt`
has no TUI, no approval bridge and a closed stdin, so anything reaching
for interactivity inside the tool or runner would hang on a prompt nobody
can answer. The background half was already refused explicitly; this
pins the foreground half.
Part of QwenLM#8769.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the PR, @qqqys — the write-up itself is genuinely detailed, but it doesn't follow the repository's PR template, so the gate has to stop here before code review. This is a formatting gate, not a code concern.
The body is missing all of the required sections from pull_request_template.md (it currently uses a custom agent({workingDir}) / Tunable per-subagent bounds / Headless regression test / Tests structure):
- What this PR does — prose description of the change
- Why it's needed — the motivation is already in your description; it just needs this section
- Reviewer Test Plan, with its three subsections:
- How to verify — the behaviors a reviewer should confirm and what to expect: e.g. a
Workflowcall withworkingDirpins the subagent to that worktree and stays off the fast path, an invalid path aborts the dispatch with the cause named, the env-tunable bounds clamp/reject as documented, and which suites pin that - Evidence (Before & After) — this is internal workflow-engine behavior rather than a TUI surface change, so
N/Ais fine here per the template, with commands and output under How to verify - Tested on — the OS matrix (🍏/🪟/🐧 with ✅/
⚠️ /N/A); right now it's unclear where your verification ran
- How to verify — the behaviors a reviewer should confirm and what to expect: e.g. a
- Risk & Scope — the three bullets: main risk or tradeoff (e.g. forcing the override path when
workingDiris set) / not validated / breaking changes - Linked Issues — reference #8769 without a closing keyword, since this is part of that proposal
- The
<details>Chinese translation of the body
Could you restructure the body to follow the template? The content you already wrote is good — most of it can be moved into the right sections as-is. Please keep each paragraph or list item as one long line (the template notes that GitHub renders single newlines as <br>, so hard-wrapped text displays as a narrow column).
Once the body is updated, a maintainer can re-run triage with @qwen-code /triage to continue.
中文说明
感谢提交 PR,@qqqys——描述本身写得很详细,但没有遵循仓库的 PR 模板,所以 gate 在代码审查之前先停在这里。这是一次格式上的拦截,而不是对代码的质疑。
正文缺少 pull_request_template.md 要求的所有章节(目前使用了自定义的 agent({workingDir}) / Tunable per-subagent bounds / Headless regression test / Tests 结构):
- What this PR does——用散文描述改动
- Why it's needed——动机在你的描述里已经写了,只需要放到这个章节
- Reviewer Test Plan,包含三个子章节:
- How to verify——评审者应确认的行为和预期结果:例如带
workingDir的Workflow调用会把子代理固定到该 worktree 并绕开 fast path、非法路径会中止分发并说明原因、环境变量可调上限按文档所述钳制/拒绝,以及哪些测试套件固定了这些行为 - Evidence (Before & After)——这是 workflow 引擎内部行为而非 TUI 界面改动,按模板写
N/A即可,命令与输出放在 How to verify 下 - Tested on——操作系统矩阵(🍏/🪟/🐧 加 ✅/
⚠️ /N/A);目前无法判断你的验证是在哪个平台上进行的
- How to verify——评审者应确认的行为和预期结果:例如带
- Risk & Scope——三个要点:主要风险或权衡(例如设置
workingDir时强制走 override 路径)/ 未验证项 / 破坏性变更 - Linked Issues——引用 #8769(不使用关闭关键字,因为这是该提案的一部分)
- 正文的
<details>中文翻译
能否按模板重构正文?你已经写好的内容是好的——多数可以直接挪到对应章节。请保持每个段落或列表项为一长行(模板注明 GitHub 会把单个换行渲染成 <br>,硬换行的文字会显示成窄列)。
正文更新后,维护者可以用 @qwen-code /triage 重新触发 triage 继续流程。
— Qwen Code · qwen3.8-max
已被后续 commit 取代,当前 head 需重新 review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more.
— qwen3.8-max via Qwen Code /review (v0.21.10)
| } else if (typeof opts.workingDir === 'string' && opts.workingDir) { | ||
| // Caller-owned worktree: same rebind, no provisioning and no cleanup. |
There was a problem hiding this comment.
[Suggestion] Guard asymmetry: a defined-but-falsy workingDir skips the rebind in silence.
The fast-path gate above treats opts.workingDir !== undefined as "must take the override path" (the fast-path condition requires opts.workingDir === undefined), but this branch only pins when typeof opts.workingDir === 'string' && opts.workingDir is truthy. A defined-but-falsy workingDir ('' or a non-string) therefore bypasses the fast path, fails this truthiness check, and the subagent runs in the parent working tree with no error — the exact silent misdirection this option exists to prevent.
Failure scenario: a caller invoking the exported WorkflowAgentDispatch seam with { workingDir: '' } gets an agent silently running in the parent tree — probe-reproduced at this commit: dispatch went through the override path with the parent Config untouched, resolver never consulted, no error. Latent today because the sandbox validates non-empty strings upstream.
Make the guards symmetric — branch on opts.workingDir !== undefined and throw for an invalid value:
} else if (opts.workingDir !== undefined) {
if (typeof opts.workingDir !== 'string' || !opts.workingDir) {
throw new Error(
'agent({workingDir}): must be a non-empty string naming an existing git worktree of this repository.',
);
}— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
已修复:主机侧 dispatch 现在也会拒绝空 workingDir,避免绕过 sandbox 时静默落回父工作区。验证证据:orchestrator 141/141、sandbox 151/151 通过;Core typecheck、ESLint、Prettier、diff check 通过。
| await expect( | ||
| sandbox.run(`return agent("x", { workingDir: 7 });`), | ||
| ).rejects.toThrow(/workingDir.*non-empty string/); |
There was a problem hiding this comment.
[Suggestion] The empty-string half of the sandbox guard has no test — only the non-string half (workingDir: 7) is exercised here, and no test in packages/core exercises workingDir: ''.
The clause is load-bearing: if || agentOpts.workingDir.length === 0 were dropped from workflow-sandbox.ts, agent("x", { workingDir: "" }) would pass sandbox validation and reach the orchestrator — where the fast-path gate is false for '' but the rebind branch is truthy-gated — so no rebind, no error, and the agent silently runs in the parent working tree. No test would fail.
| await expect( | |
| sandbox.run(`return agent("x", { workingDir: 7 });`), | |
| ).rejects.toThrow(/workingDir.*non-empty string/); | |
| await expect( | |
| sandbox.run(`return agent("x", { workingDir: 7 });`), | |
| ).rejects.toThrow(/workingDir.*non-empty string/); | |
| await expect( | |
| sandbox.run(`return agent("x", { workingDir: "" });`), | |
| ).rejects.toThrow(/workingDir.*non-empty string/); |
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
已修复:sandbox 回归现在同时覆盖非字符串和空字符串 workingDir。验证证据:workflow-sandbox 151/151 通过。
| expect( | ||
| resolveSubagentMaxTurns({ QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS: '120' }), | ||
| ).toBe(120); |
There was a problem hiding this comment.
[Suggestion] The env-tunable bounds are only tested at resolver level — the dispatch wiring survives a revert mutation.
Mutation check at this commit: replacing resolveSubagentMaxTurns() / resolveSubagentMaxTimeMinutes() with the DEFAULT_* constants at both call sites (fast path ~547-548, override path ~885-886) keeps all 139 tests in this file green — the pre-existing wiring test asserts exactly { max_turns: 50, max_time_minutes: 10 } (identical for constants and resolvers in a clean env), and the override path's captured runConfigOverrides is recorded but never asserted.
Failure scenario: an operator sets QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS=120 to stop long agents being cut to null in parallel(); a future refactor silently ignores the env var at the dispatch sites, and the suite blesses it.
Add one test per path that stubs the env and asserts the dispatched runConfig reflects the override — this shape was probe-verified to fail against the mutated code and pass against the original:
vi.stubEnv('QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS', '120');
// assert created[0].runConfig.max_turns === 120 (fast path)
// and the captured runConfigOverrides.max_turns === 120 (override path)— qwen3.8-max via Qwen Code /review (v0.21.10)
| it('foreground execute() completes with no interactive session or completion channel', async () => { | ||
| const registry = new WorkflowRunRegistry(); | ||
| const config = { |
There was a problem hiding this comment.
[Suggestion] This test pins only the tool layer — the end-to-end contract issue #8769 P0 #2 names is not exercised anywhere.
P0 #2 requires a foreground Workflow call to work end-to-end non-interactively under the appropriate approval mode: getDefaultPermission() is 'ask', resolved by the scheduler against the run's approval mode. This test constructs the tool directly with isInteractive: () => false and calls execute() — it never exercises approval-mode resolution of the 'ask' default permission, and no integration test covers a headless Workflow run. The comment above calls this "the regression test for that contract".
Failure scenario: if a future change introduces an interactive prompt into the scheduler/permission path for ask-default tools in headless runs, the qwen --prompt path (CI, cron — closed stdin) hangs on a prompt nobody can answer while this unit test stays green.
Consider adding — or tracking as an explicit follow-up before Phase 1 — an end-to-end headless check, e.g. an integration test running a Workflow call via qwen --prompt under yolo/auto approval with stdin closed, since the unit layer cannot represent approval-mode resolution.
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
Deferred as an explicit follow-up before Phase 1, rather than implemented in this round. The unit-layer contract this PR owns — a foreground Workflow call completing with no interactive session and no completion channel — is pinned by the headless regression test added here. What the finding correctly notes is that approval-mode resolution of the 'ask' default permission happens in the scheduler against the run's approval mode, which the unit layer cannot represent; a genuine end-to-end check needs new integration-harness scaffolding (bundled CLI running a Workflow call via --prompt under yolo/auto approval with stdin closed, against a mock model endpoint — no Workflow integration harness exists today). Adding that harness would grow this PR well past its original intent, so it is tracked as a follow-up instead of being silently dropped.
中文说明
作为 Phase 1 之前的明确后续事项推迟,本轮不实现。本 PR 所保证的单元层契约——前台 Workflow 调用在没有交互式会话、没有 completion channel 的情况下完成——已经由此处新增的 headless 回归测试固定下来。该发现正确指出:'ask' 默认权限的 approval-mode 解析发生在调度器中、针对本次运行的 approval mode 进行,而单元层无法表达这一点;真正的端到端检查需要新的集成测试脚手架(通过 --prompt 在 yolo/auto 审批模式下、stdin 关闭、对着 mock 模型端点运行一次 Workflow 调用的打包 CLI——目前并不存在 Workflow 集成脚手架)。加入该脚手架会使本 PR 远超其原始意图,因此改为作为后续事项跟踪,而不是被静默丢弃。
| for (const k of [ | ||
| 'schema', | ||
| 'model', | ||
| 'isolation', | ||
| 'agentType', | ||
| 'workingDir', | ||
| ] as const) { |
There was a problem hiding this comment.
[Suggestion] The fileoverview contract description contradicts this projection change.
This diff adds workingDir to the projection (here), and the function-level doc argues it is load-bearing for cache correctness — but the file's @fileoverview (lines 23-26) still asserts the projection "keeps only the dispatch-affecting opts (schema, model, isolation, agentType)" — the exact opposite of the implemented behavior, for the one opt where cache-correctness is a safety property. The fileoverview is the module's authoritative description of resume-key derivation ("Key derivation (matches upstream v2)"), and it is what a reader meets first.
Failure scenario: a maintainer diagnosing resume-cache behavior — "why did changing only workingDir force a re-run?" — reads the fileoverview, concludes workingDir is cosmetic and projected away, and "fixes" the key by removing it — restoring the cross-directory replay hole this PR closes.
Update the fileoverview sentence to the new set: "keeps only the dispatch-affecting opts (schema, model, isolation, agentType, workingDir)".
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
已修复:journal fileoverview 已把 workingDir 纳入 dispatch-affecting canonical options。验证证据:Prettier、ESLint、diff check 通过。
| ); | ||
| if ('error' in resolved) { | ||
| throw new Error( | ||
| `agent({workingDir: ${JSON.stringify(opts.workingDir)}}): ${resolved.error}`, |
There was a problem hiding this comment.
[Suggestion] This refusal message escapes opts.workingDir via JSON.stringify but interpolates resolved.error raw — and every refusal variant in worktree-pin.ts embeds the script-supplied resolvedPath unescaped, so control characters reach the error text anyway.
Probe-reproduced: workingDir: 'foo\r\n[audit] injected' passes the sandbox's only validation (non-empty string) and the thrown error carries raw CRLF — surfaced to logs/display/OTLP — even though the adjacent agentType branch carries an explicit SECURITY comment applying sanitizeForErrorMessage for exactly this fragmentation class, and the first half of this very message is escaped.
| `agent({workingDir: ${JSON.stringify(opts.workingDir)}}): ${resolved.error}`, | |
| `agent({workingDir: ${JSON.stringify(opts.workingDir)}}): ${sanitizeForErrorMessage(resolved.error)}`, |
Ideally also sanitize resolvedPath where the worktree-pin error strings are built, so both surfaces are covered (the AgentTool surface shares this module).
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
已修复:resolver 返回的模型可控错误文本在拼接前会移除控制字符,并增加 CR/LF/NUL 回归。验证证据:workflow-orchestrator 141/141 通过;ESLint 通过。
| * methods would otherwise still resolve through the prototype to the parent. | ||
| */ | ||
| function createWorktreeConfigOverride(base: Config, wtPath: string): Config { | ||
| function createDirScopedConfigOverride(base: Config, wtPath: string): Config { |
There was a problem hiding this comment.
[Suggestion] The shared dir-scoped rebind drops user-configured customIgnoreFiles — a mirror gap vs the AgentTool block the doc comment claims to mirror.
The body below (~1199) builds new FileDiscoveryService(wtPath) without customIgnoreFiles, while AgentTool's inline rebind (agent.ts ~2864-2867) passes this.config.getFileFilteringOptions().customIgnoreFiles. Probe-demonstrated with a temp-dir fixture (.cursorignore listing secret.txt): this helper's construction shape yields shouldQwenIgnoreFile('secret.txt') = false — the secret surfaces in ls/read-file/grep inside the pinned worktree — while the AgentTool shape yields true. The identical one-liner pre-existed for isolation: 'worktree', but this PR extracts it into this helper, wires the new workingDir surface through it, and asserts surface parity in the doc comment. AgentTool has a regression test for the carry-through (agent.test.ts ~2083-2127); the workflow surface has none.
const wtFileService = new FileDiscoveryService(
wtPath,
base.getFileFilteringOptions().customIgnoreFiles,
);— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
已修复:目录重绑定会继承 base Config 的 customIgnoreFiles,并增加 .cursorignore 传递断言。验证证据:workflow-orchestrator 141/141 通过;Core typecheck 通过。
| it('refuses a path outside the repository', async () => { | ||
| const result = await resolveExternalWorktreeDir(config, '/elsewhere/tree'); |
There was a problem hiding this comment.
[Suggestion] The symlink/realpath half of the containment guard has zero coverage in this new test file — every test stubs GitWorktreeService with plain strings; there is no node:fs/promises mock or symlink fixture. /repo does not exist during the tests, so both fs.realpath calls reject and the .catch() fallbacks degrade containment to a string comparison — the canonical-path logic never executes.
Probe-demonstrated: deleting both fs.realpath calls from worktree-pin.ts keeps all 7 tests in this file green.
Failure scenario: a future "simplification" removes the canonical-path comparison, and a model-supplied in-repo path that is a symlink to a registered worktree outside the repo then passes both string containment and isRegisteredLinkedWorktree (which realpaths its own input and matches the target's registry entry) — re-binding the child's WorkspaceContext outside the repository, the exact escape the guard exists to stop.
Add a case (temp-dir fixture or an fs.realpath mock) where the in-repo path canonicalizes outside the repo, asserting the resolves outside this repository error.
— qwen3.8-max via Qwen Code /review (v0.21.10)
| * operator-tunable via env, on the same pattern as the three other workflow | ||
| * bounds (`QWEN_CODE_MAX_WORKFLOW_AGENTS`, `QWEN_CODE_WORKFLOW_STALL_SECONDS`, | ||
| * `QWEN_CODE_MAX_WORKFLOW_SECONDS`), each clamped to a hard ceiling. |
There was a problem hiding this comment.
[Suggestion] This clause is factually wrong about two of the three named bounds.
Probe at this commit ran all four resolvers with oversized input: only QWEN_CODE_MAX_WORKFLOW_AGENTS clamps (→ ceiling 10000). QWEN_CODE_WORKFLOW_STALL_SECONDS is applied verbatim by resolveStallMs (999999999s → 999999999000ms; no ceiling branch in the function) and QWEN_CODE_MAX_WORKFLOW_SECONDS is applied verbatim by resolveMaxWallClockMs (the repo's own test shows even '0.1' honored).
Failure scenario: a maintainer bounding worst-case workflow wall time from this rationale comment concludes a misconfigured env cannot exceed a ceiling, when QWEN_CODE_MAX_WORKFLOW_SECONDS=999999999 is honored unclamped (~31.7 years); the claim also makes the new HARD_* clamps look like universal house style, inviting a wrong "consistency" change in either direction.
| * operator-tunable via env, on the same pattern as the three other workflow | |
| * bounds (`QWEN_CODE_MAX_WORKFLOW_AGENTS`, `QWEN_CODE_WORKFLOW_STALL_SECONDS`, | |
| * `QWEN_CODE_MAX_WORKFLOW_SECONDS`), each clamped to a hard ceiling. | |
| * operator-tunable via env, on the same env-override pattern as the other | |
| * workflow bounds; like `QWEN_CODE_MAX_WORKFLOW_AGENTS` (and unlike | |
| * `QWEN_CODE_WORKFLOW_STALL_SECONDS` / `QWEN_CODE_MAX_WORKFLOW_SECONDS`, | |
| * which apply valid overrides verbatim), clamped to a hard ceiling. |
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const relToRepo = path.relative(realRepoRoot, realResolved); | ||
| if (relToRepo.startsWith('..') || path.isAbsolute(relToRepo)) { |
There was a problem hiding this comment.
[Suggestion] startsWith('..') also matches legitimate names beginning with .. — spuriously refusing registered worktrees like ..hidden-wt.
Probe-reproduced end-to-end against real git: git worktree add ..hidden-wt succeeds and registers; driving this module's exact logic refuses the pin with resolves outside this repository, because path.relative(repoRoot, '<repo>/..hidden-wt') is '..hidden-wt' and startsWith('..') is true. Fails closed (false refusal + misleading error), not a bypass. Moved verbatim from agent.ts, but newly reachable from workflow scripts.
Failure scenario: a user who keeps a worktree under a dot-dot-prefixed name (legal on POSIX; only . and .. exactly are reserved) cannot pin it, and the error sends debugging in the wrong direction.
Test traversal segments, not the string prefix (flip-verified: accepts ..hidden-wt, still refuses genuine ../ traversal):
| const relToRepo = path.relative(realRepoRoot, realResolved); | |
| if (relToRepo.startsWith('..') || path.isAbsolute(relToRepo)) { | |
| const relToRepo = path.relative(realRepoRoot, realResolved); | |
| if ( | |
| relToRepo === '..' || | |
| relToRepo.startsWith(`..${path.sep}`) || | |
| path.isAbsolute(relToRepo) | |
| ) { |
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
已修复:containment 只拒绝精确 .. 或 ../ 路径段,不再误拒 ..hidden-wt,并增加回归。验证证据:worktree-pin 8/8 通过;Core typecheck 通过。
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
Address the remaining review findings on the agent workingDir pin. Containment anchored at `--show-toplevel`, which from inside a linked worktree answers with the worktree's own root — spuriously refusing registered sibling worktrees, the documented review-pipeline setup. Resolve the repository's main working tree via the first entry of `git worktree list --porcelain` (new GitWorktreeService helper) and anchor there, keeping the toplevel answer as fallback. Add dispatch-site wiring tests for the env-tunable subagent bounds at both the fast and the override path: with a clean env the DEFAULT_* constants and the resolvers are indistinguishable, so a revert mutation at either call site kept every existing test green. Cover the fs.realpath half of the containment guard with a real symlink fixture (plain-string stubs made both realpath calls reject, so the canonical-path logic never executed), and the sibling-anchor fix with a unit case. Correct the bounds doc comment: only QWEN_CODE_MAX_WORKFLOW_AGENTS and the subagent bounds clamp to a ceiling; the stall and wall-clock env overrides apply valid values verbatim.
…/qwen-code into workflow/agent-working-dir
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review feedback round — PR #8972Commit: Findings and dispositionsFixed this round
Re-verified from the previous round (still holding in HEAD)
Deferred (not resolved — recorded reply posted on the thread)
Reviewer test-plan note ("no such file or directory" for six test files) — these were reviewer-side paths missing the Failed check analysis:
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Test (macos-latest / windows-latest, Node 22.x) was skipped in CI and the unit suite did not run locally on those OSes.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more.
中文说明
已审查。 建议见行内评论。
未审查:build-and-test — Test (macos-latest / windows-latest, Node 22.x) was skipped in CI and the unit suite did not run locally on those OSes。
未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:none — all checks above completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks above completed within budget.。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan(非阻断):src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| async getMainWorktreePath(): Promise<string | null> { | ||
| try { | ||
| const out = await ( | ||
| await this.getGit() | ||
| ).raw(['worktree', 'list', '--porcelain']); | ||
| const firstLine = out.split('\n', 1)[0]?.trim() ?? ''; |
There was a problem hiding this comment.
[Suggestion] The new getMainWorktreePath() has zero direct test coverage — the only test depending on its semantics (the sibling-worktree acceptance test in worktree-pin.test.ts) stubs the method out wholesale, and gitWorktreeService.test.ts never references it. This method is the anchor of the PR's central re-anchoring fix.
Failure scenario: a regression making it always return null (e.g. inverting the startsWith('worktree ') check) silently falls back to getRepoTopLevel() in worktree-pin.ts — from inside a linked worktree that answers the worktree's own root, so sibling pins like ../review-pr-1-base are spuriously refused as outside the repository: exactly the regression this PR fixes. Probe-verified at this commit: under that mutation all 287 tests in the three relevant suites stay green, while the unit tests sketched below flip 2/4.
// packages/core/src/services/gitWorktreeService.test.ts — existing hoistedMockRaw pattern
it('parses the first porcelain entry as the main worktree path', async () => {
hoistedMockRaw.mockResolvedValueOnce('worktree /repo\nworktree /repo/wt\n');
expect(await service.getMainWorktreePath()).toBe('/repo');
});
// plus: first line without the `worktree ` prefix -> null; raw() rejecting -> null; empty output -> null中文说明
新增的 getMainWorktreePath() 没有任何直接测试覆盖——唯一依赖其语义的测试(worktree-pin.test.ts 中的兄弟 worktree 接受测试)把该方法整个 stub 掉了,gitWorktreeService.test.ts 也从未引用它。该方法是本 PR 核心「重新锚定」修复的锚点。
失败场景:某个使其恒返回 null 的回归(例如把 startsWith('worktree ') 检查写反)会静默回退到 worktree-pin.ts 中的 getRepoTopLevel()——在 linked worktree 内部运行时它回答的是该 worktree 自己的根,于是像 ../review-pr-1-base 这样的兄弟钉住会被误判为「在仓库之外」而遭拒绝:这正是本 PR 要修复的回归。已在当前提交上用探针验证:该变异下三个相关套件的全部 287 个测试仍保持全绿,而上面草拟的单元测试会有 2/4 翻红(草图使用 gitWorktreeService.test.ts 现有的 hoistedMockRaw 模式:porcelain 首条目 → 主树路径;首行无 worktree 前缀 → null;raw() 拒绝 → null;空输出 → null)。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| * The `canonicalOpts` projection keeps only the dispatch-affecting opts | ||
| * (`schema`, `model`, `isolation`, `agentType`) with object keys sorted, so | ||
| * cosmetic opt differences (a re-ordered schema, a `label` change) don't | ||
| * bust the cache. | ||
| * (`schema`, `model`, `isolation`, `agentType`, `workingDir`) with object keys | ||
| * sorted, so cosmetic opt differences (a re-ordered schema, a `label` change) | ||
| * don't bust the cache. |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (deterministic): reverting this documentation-only hunk on its own leaves every test green — nothing gates this comment against code drift. The companion code hunk (adding 'workingDir' to the canonicalizeAgentOpts projection) WAS killed by the probe and the whole-file revert is gated by workflow-journal.test.ts, so the behaviour itself is covered — only this comment is ungated. Note round 1's R1-5 was exactly this drift class on this same file, so the risk is not hypothetical. Measured coverage observation, not a behavioural defect — no code change is required for this PR.
Failure scenario: a future change dropping workingDir from the projection (or adding another dispatch-affecting opt) leaves this JSDoc stale with nothing flagging the mismatch — a misleading contract document on a cache-correctness module.
中文说明
测试有效性探针(确定性结果):单独回退这个纯文档 hunk 后所有测试仍然全绿——没有任何东西把这条注释与代码钉在一起以防漂移。配套的代码 hunk(把 'workingDir' 加入 canonicalizeAgentOpts 投影)被探针杀死,整文件回退也被 workflow-journal.test.ts 拦截,所以行为本身是有覆盖的——只有这条注释没有被钉住。注意第一轮的 R1-5 正是同一文件上同一类「文档与投影漂移」,因此这里的漂移风险并非假设。这是测量得出的覆盖观察,不是行为缺陷——本 PR 无需改动代码。
失败场景:未来的改动若把 workingDir 从投影中移除(或新增另一个影响派发的选项),这段 JSDoc 会在没有任何东西提示不一致的情况下过期——而这是位于缓存正确性攸关模块上的一份契约文档。
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
Declined — no code change this round. Your own deterministic probe records that the behaviour is already gated: adding 'workingDir' to the canonicalizeAgentOpts projection is mutation-killed, and a whole-file revert is caught by workflow-journal.test.ts. Only the prose comment is ungated, and the finding explicitly states no code change is required for this PR. Gating the prose itself would mean a brittle assertion on comment text, so per the repo's simplicity rule we are not growing the diff for it. The residual drift risk (a future change dropping workingDir from the projection leaving this JSDoc stale) is acknowledged and recorded here; the projection assertion in workflow-journal.test.ts remains the behavioural gate.
中文说明
拒绝——本轮不做代码改动。您自己的确定性探针已确认行为本身有测试拦截:把 'workingDir' 加入 canonicalizeAgentOpts 投影会被变异杀死,整文件回退也被 workflow-journal.test.ts 拦截。只有这段文字注释没有被钉住,且该发现明确写明本 PR 无需改动代码。要钉住文字本身,只能靠对注释文本的脆弱断言,因此按本仓库的简洁性原则不为此扩大 diff。残余的漂移风险(未来改动把 workingDir 从投影中移除、这段 JSDoc 随之过期而无人提示)在此记录在案;workflow-journal.test.ts 中的投影断言仍是行为层面的拦截。
| 'cannot serve. Mutually exclusive with `isolation`. The path must live ' + | ||
| 'inside the repository and appear in `git worktree list`. ' + |
There was a problem hiding this comment.
[Suggestion] The model-facing workingDir description states an eligibility condition that is not sufficient: "appear in git worktree list". The authoritative gate isRegisteredLinkedWorktree rejects the main working tree even though it is always listed first in git worktree list, and it also rejects stale-but-still-listed registry entries via its liveness probe. The refusal text itself names "it is the main working tree" — the implementation knows the documented condition is insufficient.
Failure scenario: probe-verified against real git at this commit — resolveExternalWorktreeDir(config, '.', 'workingDir') and the absolute main-tree path are both refused while the linked review worktree is accepted. A script author following this description — which earlier invites pinning with "its uncommitted state is the point" — pins the dirty main checkout and gets a runtime refusal; inside parallel() (documented in the next paragraph as errors-as-data) the throwing thunk becomes null at its index, so a documented-contract script silently degrades to a null result instead of an agent result.
| 'cannot serve. Mutually exclusive with `isolation`. The path must live ' + | |
| 'inside the repository and appear in `git worktree list`. ' + | |
| 'cannot serve. Mutually exclusive with `isolation`. The path must live ' + | |
| 'inside the repository and be a linked worktree registered via ' + | |
| '`git worktree add` — the main checkout is not eligible. ' + |
中文说明
面向模型的 workingDir 描述给出了一个不充分的资格条件:「出现在 git worktree list 中」。权威关卡 isRegisteredLinkedWorktree 会拒绝主工作树——尽管它总是排在 git worktree list 的第一条——并且会通过存活探测拒绝那些仍在列表中但已失效的登记条目。拒绝文案自己都写着 "it is the main working tree"——实现明知文档给出的条件不够。
失败场景:已在当前提交上用真实 git 探针验证——resolveExternalWorktreeDir(config, '.', 'workingDir') 与主树绝对路径都被拒绝,而 linked 的 review worktree 被接受。脚本作者按照这段描述(前文还在用「其未提交状态正是重点」邀请钉住)把子 agent 钉在脏的主检出上,得到运行时拒绝;而在 parallel()(下一段明确描述为「错误即数据」)里,抛错的 thunk 会变成其下标上的 null——按文档契约写出的脚本静默退化为一个 null 结果,而不是 agent 结果。建议修复:收紧表述为「在仓库内部、且是通过 git worktree add 登记的 linked worktree——主检出不可作为钉住目标」。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| expect(canonicalizeAgentOpts({ workingDir: 'wt' })).toBe( | ||
| JSON.stringify({ workingDir: 'wt' }), | ||
| ); | ||
| expect(a).not.toBe(b); |
There was a problem hiding this comment.
[Suggestion] The new workingDir journal-key test gates only the MISS direction (different dirs ⇒ different keys); the HIT direction (same workingDir ⇒ same key ⇒ resume actually replays from cache) is asserted nowhere — deriveAgentKey's determinism tests use only {}/{ model } opts, and every P6 end-to-end resume test dispatches bare opts.
Failure scenario: probe-verified mutation at this commit — a per-call nonce in the workingDir branch of deriveAgentKey keeps all 15 journal tests green (not.toBe stays green, the projection assertion pins canonicalizeAgentOpts rather than the hash, and the determinism test never passes a workingDir). Result: every resumeFromRunId of a workingDir-using workflow silently misses the journal and re-runs all dispatches live — full token/time re-spend with no error — defeating the resume cache for exactly the workflow shape this PR introduces. The symmetric assertion below fails under the mutation and passes on the correct code.
| expect(canonicalizeAgentOpts({ workingDir: 'wt' })).toBe( | |
| JSON.stringify({ workingDir: 'wt' }), | |
| ); | |
| expect(a).not.toBe(b); | |
| expect(canonicalizeAgentOpts({ workingDir: 'wt' })).toBe( | |
| JSON.stringify({ workingDir: 'wt' }), | |
| ); | |
| expect(a).not.toBe(b); | |
| expect( | |
| deriveAgentKey('', 'review it', { | |
| workingDir: '.qwen/tmp/review-pr-1', | |
| }), | |
| ).toBe( | |
| deriveAgentKey('', 'review it', { | |
| workingDir: '.qwen/tmp/review-pr-1', | |
| }), | |
| ); |
中文说明
这个新的 workingDir 日志键测试只钉住了 MISS 方向(不同目录 ⇒ 不同键);HIT 方向(相同 workingDir ⇒ 相同键 ⇒ resume 确实从缓存重放)没有任何断言——deriveAgentKey 的确定性测试只用 {}/{ model } 选项,P6 的端到端 resume 测试全部以裸选项派发。
失败场景:已在当前提交上做变异探针——在 deriveAgentKey 的 workingDir 分支里加入每次调用不同的 nonce,15 个 journal 测试全部保持绿(not.toBe 仍绿;投影断言钉的是 canonicalizeAgentOpts 而不是哈希;确定性测试从不传 workingDir)。结果是:使用 workingDir 的 workflow 每次 resumeFromRunId 都会静默 miss 日志、全部派发重新实跑——token 与时间被完整重花且没有任何报错——恰好在本 PR 引入的这种 workflow 形态上击穿了 resume 缓存。下面的对称断言在该变异下会失败、在正确代码上通过。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const repoRoot = | ||
| (await probe.getMainWorktreePath()) ?? | ||
| (await probe.getRepoTopLevel()) ?? | ||
| parentCwd; |
There was a problem hiding this comment.
[Suggestion] Probe-reproduced containment edge introduced by the new main-tree anchor: getMainWorktreePath() parses newline-delimited porcelain, so a main-tree path containing a newline truncates the anchor to a shorter prefix that may resolve inside a different repository, against whose registry isRegisteredLinkedWorktree then validates. The probe built that layout with real git: a pin to the other repo's live registered worktree was accepted, rebinding the child outside the invoking repository; replacing the anchor with the pre-PR getRepoTopLevel() chain refused the same input (the probe flips) — the escape is introduced by this re-anchor. It also falsifies the new JSDoc's claim that "consumers fail closed on the bad anchor and the authoritative registration checks never consult this value": the consumer accepted, and the registration check consults the anchor via wtService's sourceRepoPath.
Severity is Suggestion rather than Critical: the trigger is pathological and self-inflicted (cloning into a path containing a newline, nested inside another repo that has a registered worktree under the truncated prefix), the pin path remains model-supplied, and a pin is a cwd rebind, not a sandbox. The far more probable outcome of a newline-bearing clone path is the benign direction: truncated prefix inside no repo ⇒ everything fails closed and legitimate pins are spuriously refused.
Failure scenario: repo R1 cloned into /a/<LF>R1 where /a is inside repo R2 — running inside R1, getMainWorktreePath() splits on the newline and returns /a; containment then checks the model-supplied pin against /a, and GitWorktreeService('/a') validates against R2's registry, so a pin to R2's live worktree /a/wtR2 passes both gates — the child's WorkspaceContext rebinds outside the invoking repository, exactly what the containment comment above this block forbids.
Suggested fix (author's choice): cross-check the parsed anchor before trusting it (e.g. require agreement with git rev-parse --show-toplevel when cwd is in the main tree); fall back to getRepoTopLevel() when the porcelain first entry does not consume cleanly; or parse with -z on git ≥ 2.36. Do not simply revert to getRepoTopLevel() — that regresses the linked-worktree sibling-pin case this PR deliberately fixes.
中文说明
探针复现的包含边界问题,由新的「主树锚点」引入:getMainWorktreePath() 解析以换行分隔的 porcelain 输出,因此包含换行的主树路径会把锚点截断成更短的前缀,而该前缀可能落在另一个仓库内部,随后 isRegisteredLinkedWorktree 会对着那个仓库的登记册做校验。探针用真实 git 搭出了这个布局:钉到对方仓库「存活且已登记」的 worktree 被接受了,子 agent 被重绑定到调用仓库之外;把锚点换回 PR 之前的 getRepoTopLevel() 链后,同样的输入被拒绝(探针翻转)——这个逃逸是本次重新锚定引入的。它同时也证伪了新 JSDoc 的说法(「消费者在坏锚点上会失败关闭、且权威登记检查从不使用该值」):消费者接受了,而登记检查经由 wtService 的 sourceRepoPath 确实使用了该锚点。
严重度定为 Suggestion 而非 Critical:触发条件是病态且自伤的(把仓库克隆进一个含换行的路径,且其嵌套在另一个仓库内、截断前缀之下恰好有对方已登记的 worktree);钉住路径仍然由模型提供;且钉住是 cwd 重绑定,不是沙箱。含换行的克隆路径更常见的结果是良性方向:截断前缀不在任何仓库内 ⇒ 全部失败关闭,合法的钉住被误拒。
失败场景:仓库 R1 克隆到 /a/<LF>R1,而 /a 位于仓库 R2 内部——在 R1 内运行时,getMainWorktreePath() 按换行切分、返回 /a;包含检查随后以 /a 为基准校验模型给出的钉住路径,GitWorktreeService('/a') 对照 R2 的登记册校验,于是钉到 R2 的存活 worktree /a/wtR2 两关全过——子 agent 的 WorkspaceContext 被重绑定到调用仓库之外,正是上方注释明令禁止的情形。
建议修复(由作者选择):在信任解析出的锚点前做交叉校验(例如 cwd 在主树时要求与 git rev-parse --show-toplevel 一致);porcelain 首条目未被完整消费时回退到 getRepoTopLevel();或在 git ≥ 2.36 上用 -z 解析。请不要简单回退为 getRepoTopLevel()——那会让本 PR 刻意修复的「linked worktree 内钉住兄弟 worktree」场景回归。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: 🤖 No code changes this round — the failed check was investigated to the limit available in this environment and is not reproducible from anything this PR owns. · 本轮未改动代码 —— 失败的检查已在当前环境能力范围内彻查,无法从本 PR 拥有的任何代码路径复现。 Review feedback round — PR #8972Commit: Failed check analysis:
|
QwenLM#8972) A main working tree whose path contains a newline splits the porcelain first entry of `git worktree list`, and the truncated prefix could resolve inside a different repository — re-anchoring the pin's containment and registration checks against that repo's worktree registry. Detect the malformed first record (a path remainder where a record attribute belongs) and fall back to `--show-toplevel`, whose single-value answer keeps interior newlines intact. Also pins down round-2 review findings: direct unit and real-git coverage for `getMainWorktreePath()` (whose semantics were only exercised through a stub), the symmetric journal-key HIT direction for `workingDir` resumes, and the model-facing `workingDir` eligibility description (the main checkout is not a valid pin target even though it appears in `git worktree list`).
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Round summaryAddressed 4 of the 5 inline findings from the automated reviewer's round-2 pass and declined 1 with a recorded reason. All changes are confined to Feedback points and decisions
Conflict notesNone — VerificationCommands actually run (results at the pushed commit
Environment note on the still-red 中文说明本轮摘要已处理自动化审查者第二轮行内发现中的 4 项,另有 1 项以记录在案的理由拒绝。全部改动限于 各反馈点及处理决定
冲突说明无—— 验证实际执行的命令(结果为推送提交
关于仍为红色的 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: reverse audit — reached the round cap (5) without two consecutive dry rounds; round 5 still surfaced findings.
Not reviewed: build-and-test — Test (macos-latest / windows-latest, Node 22.x) was skipped in CI and the unit suite did not run locally on those OSes.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the e2e suite did not run locally.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: (none — all planned checks completed within budget); chunk 2: none — all checks I intended completed within budget..
Test Plan (not a blocker): src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more.
中文说明
已审查。 建议见行内评论。
未审查:reverse audit — reached the round cap (5) without two consecutive dry rounds; round 5 still surfaced findings。
未审查:build-and-test — Test (macos-latest / windows-latest, Node 22.x) was skipped in CI and the unit suite did not run locally on those OSes。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the e2e suite did not run locally。
未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:(none — all planned checks completed within budget);chunk 2:none — all checks I intended completed within budget.。
Test Plan(非阻断):src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| for (const line of lines.slice(1)) { | ||
| const attr = line.trim(); | ||
| if (attr === '') break; // blank line ends the first record | ||
| if (!isWorktreeListPorcelainAttribute(attr)) return null; |
There was a problem hiding this comment.
[Suggestion] The round-2 newline-truncation detector is bypassable: a truncated first porcelain record is ACCEPTED — getMainWorktreePath() returns the truncated prefix as the main-tree anchor — when the path remainder after an interior newline is attribute-shaped (detached, bare, locked, prunable, HEAD …, branch …), or when the path ends with a newline (the blank remainder hits the break). Probe-reproduced against real git end-to-end: a pin to a neighbouring repo's registered worktree was accepted, rebinding the child outside the invoking repository. Bounded at Suggestion because the trigger requires the invoking repo's own path to contain a newline (self-inflicted, not model-producible) and a pin is documented as a cwd pin, not a sandbox — but it defeats a protection this PR itself added with an explicit test and threat comment.
Failure scenario: repo R1 cloned into /outer/sub/<LF>detached with /outer/sub inside another repo R2 — porcelain emits worktree /outer/sub/ + detached + HEAD …, every continuation line passes isWorktreeListPorcelainAttribute, the anchor becomes /outer/sub/, and GitWorktreeService('/outer/sub') validates a model-supplied pin against R2's registry — the child's workspace rebinds outside the invoking repository.
Suggested fix: round-trip-validate the parsed anchor before trusting it (git -C <anchor> rev-parse --git-common-dir must agree with this repo's common dir), or anchor via rev-parse --git-common-dir, or parse --porcelain -z on Git ≥ 2.36. Anchor validation also closes the whitespace vector in the sibling comment.
中文说明
第二轮加入的换行截断检测器可被绕过:当路径在内部换行之后的残余部分呈属性形状(detached、bare、locked、prunable、HEAD …、branch …),或路径以换行结尾(空残余命中 break)时,被截断的 porcelain 首条目会被接受——getMainWorktreePath() 把截断前缀当作主树锚点返回。已用真实 git 做端到端探针复现:钉到相邻仓库已登记的 worktree 被接受,子 agent 被重绑定到调用仓库之外。触发条件要求调用仓库自身路径含换行(自伤布局,模型无法制造),且钉住按文档只是 cwd 钉住而非沙箱,因此定为 Suggestion;但它击败的是本 PR 自己用显式测试和威胁注释加上的防护。
失败场景:仓库 R1 克隆到 /outer/sub/<LF>detached,而 /outer/sub 位于另一仓库 R2 内——porcelain 输出 worktree /outer/sub/ + detached + HEAD …,每条续行都通过 isWorktreeListPorcelainAttribute,锚点变为 /outer/sub/,GitWorktreeService('/outer/sub') 会对照 R2 的登记册校验模型提供的钉住路径——子 agent 的工作区被重绑定到调用仓库之外。
建议修复:在信任解析出的锚点前做往返校验(git -C <锚点> rev-parse --git-common-dir 必须与本仓库的 common dir 一致),或改用 rev-parse --git-common-dir 取锚点,或在 Git ≥ 2.36 上用 --porcelain -z 解析。锚点往返校验同时能堵住相邻评论中的空白字符向量。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| } else if (opts.workingDir !== undefined) { | ||
| if (typeof opts.workingDir !== 'string' || !opts.workingDir) { |
There was a problem hiding this comment.
[Suggestion] The two workingDir entrances disagree on workingDir + isolation together: the sandbox entrance throws "incompatible options", but this entrance resolves the combination by precedence — isolation: 'worktree' wins and workingDir is silently dropped, the opposite of AgentTool's documented working_dir-wins semantics. The sandbox gate is additionally probe-verified bypassable from model-authored vm scripts: the gates read the raw agentOpts BEFORE the JSON revival, so an enumerable getter on isolation returning undefined for the two validation reads and 'worktree' at stringify time slips the combination through to dispatch.
Failure scenario: a script passes {workingDir: 'wt'} plus a read-counted isolation getter → the host dispatch receives {workingDir:'wt', isolation:'worktree'} → a fresh isolation worktree is provisioned and workingDir is never read — the agent runs somewhere other than the directory the caller named, and cleanup later removes that worktree while the script believes state accumulated in the pinned tree. The harm is bounded (same-repo cwd deviation, no boundary escape), but this is exactly the silent-winner failure the sandbox gate's own comment says it exists to name.
Suggested fix (probe-flip verified): throw in runOverridePath when both are defined — the host layer sees the revived plain object, so this is TOCTOU-proof:
if (opts.isolation !== undefined && opts.workingDir !== undefined) {
throw new Error('agent({workingDir, isolation}): incompatible options. ...');
}Optionally also re-gate safeOpts in the sandbox so the script gets the named error instead of a dispatch-time refusal.
中文说明
workingDir 的两个入口对 workingDir 与 isolation 同时出现的处理不一致:sandbox 入口抛出 "incompatible options",而此入口按优先级裁决——isolation: 'worktree' 胜出、workingDir 被静默丢弃,与 AgentTool 文档声明的 working_dir 优先语义相反。此外,sandbox 关卡已被探针验证可被模型编写的 vm 脚本绕过:关卡在 JSON 复活之前读取原始 agentOpts,因此给 isolation 一个可枚举 getter(前两次验证读取返回 undefined、stringify 时返回 'worktree')即可让组合溜进派发层。
失败场景:脚本传入 {workingDir: 'wt'} 加一个计数读取的 isolation getter → 宿主派发收到 {workingDir:'wt', isolation:'worktree'} → 现场新建隔离 worktree、workingDir 从未被读取——agent 跑在调用方指定目录之外的地方,cleanup 随后删除该 worktree,而脚本以为状态积累在被钉住的树里。危害有界(同仓库内 cwd 偏离、无边界逃逸),但这正是 sandbox 关卡注释自称存在目的就是要点名的「静默胜出」失败。
建议修复(探针已验证翻转):在 runOverridePath 中两者同时定义时抛错——宿主层看到的是复活后的纯对象,因此没有 TOCTOU 问题;可选地同时在 sandbox 里对 safeOpts 复检,让脚本拿到点名错误而非派发期拒绝。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| 'agent() opts: `{ label?, phase?, schema?, model?, agentType?, isolation?, workingDir?, stallMs? }`. ' + | ||
| '`schema` (JSON Schema object): the subagent must deliver its result ' + |
There was a problem hiding this comment.
[Suggestion] Two model-facing copies of the agent() opts drift in this PR. (1) The opts list advertised here includes stallMs, but it is the only opt with no explanatory paragraph — its no-progress semantics, the 60s default (DEFAULT_STALL_MS), and the 0 kill-switch ("A stallMs of 0 means 'no watchdog'", workflow-stall.ts) are undiscoverable from the schema a script author or script-writing model actually reads. (2) The tool-level copy — WORKFLOW_TOOL_DESCRIPTION's capability enumeration, ~line 552, "Per-call agent({ schema, agentType, model, isolation: 'worktree' }) covers … git-worktree-isolated subagents" — still presents isolation as the entire worktree story and never names workingDir or stallMs, at the moment this PR introduces workingDir with text explicitly documenting that isolation "cannot serve" the pinning case.
Failure scenario: a script needing a legitimately quiet long dispatch cannot learn that stallMs: 0 disables the watchdog — it guesses a huge number (watchdog stays armed) or misreads the opt as a wall-clock cap, yielding spurious aborts on a healthy run; and a model authoring from the tool description passes isolation: 'worktree' for exactly the pre-existing caller-owned-worktree case workingDir exists for — refused on a dirty parent or provisioned as a fresh checkout lacking the uncommitted state that is the point — while workingDir stays undiscoverable. The file's docblock calls this prose load-bearing and interpolates the caps precisely to prevent hand-sync drift; the opts enumeration is hand-synced, and this PR is the drift event.
Suggested fix: add a stallMs paragraph alongside the others here (stall detector, not a wall-clock cap; 0 disables the watchdog), and extend the tool-level enumeration to agent({ schema, agentType, model, isolation: 'worktree', workingDir, stallMs }), naming pinning to a caller-owned worktree and the stall watchdog.
中文说明
本 PR 让面向模型的两份 agent() 选项文案发生了漂移。(1) 此处列出的选项已包含 stallMs,但它是唯一没有解释段落的选项——其无进展语义、60 秒默认值(DEFAULT_STALL_MS)以及 0 关闭开关("A stallMs of 0 means 'no watchdog'",workflow-stall.ts)在脚本作者或写脚本的模型实际阅读的 schema 中均不可发现。(2) 工具级文案——WORKFLOW_TOOL_DESCRIPTION 的能力枚举(约第 552 行,"Per-call agent({ schema, agentType, model, isolation: 'worktree' }) covers … git-worktree-isolated subagents")——仍把 isolation 呈现为 worktree 故事的全部,只字未提 workingDir 与 stallMs;而本 PR 恰恰在引入 workingDir 时明确写着 isolation "cannot serve" 钉住场景。
失败场景:需要合法静默长派发的脚本无法得知 stallMs: 0 可关闭看门狗——要么猜一个巨大数值(看门狗仍然在位),要么把它误读为墙钟上限,健康运行被误杀;从工具描述出发的模型会对「既有、调用方自有 worktree」这一 workingDir 正是为之存在的场景传 isolation: 'worktree'——在脏父树上被拒绝,或拿到一个缺少关键未提交状态的全新检出——而 workingDir 始终不可发现。该文件的 docblock 自称这些文字是承重件,并特意用插值处理各项上限以避免手工同步漂移;选项枚举靠手工同步,而本 PR 正是漂移事件。
建议修复:在此为 stallMs 补一段(停滞检测器而非墙钟上限;0 关闭看门狗),并把工具级枚举扩为 agent({ schema, agentType, model, isolation: 'worktree', workingDir, stallMs }),点名「钉住到调用方自有 worktree」与停滞看门狗。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // about who owns the directory's lifetime, so name it here rather than | ||
| // silently letting one win. | ||
| if (agentOpts.workingDir !== undefined) { | ||
| if (typeof agentOpts.workingDir !== 'string' || agentOpts.workingDir.length === 0) { |
There was a problem hiding this comment.
[Suggestion] Whitespace-only workingDir passes both workflow entrance gates — this length === 0 check and the orchestrator's !opts.workingDir — and is refused only deep in the registration gate with a message blaming the directory ("not a registered linked worktree"), while the equivalent AgentTool input is trimmed and refused up front ("must be a non-empty string"). Probe-confirmed through the real sandbox: dispatch received {"workingDir":" "}. Fail-closed, but the three surfaces now demonstrably disagree on which layer rejects a blank-ish value, and the deep message misdirects.
Failure scenario: agent('x', { workingDir: ' ' }) clears both non-empty checks, resolves to /repo/ , passes containment, and fails isRegisteredLinkedWorktree with a registration-status diagnosis for a blank value — sending the model to fix worktree registration instead of the argument.
| if (typeof agentOpts.workingDir !== 'string' || agentOpts.workingDir.length === 0) { | |
| if (typeof agentOpts.workingDir !== 'string' || agentOpts.workingDir.trim().length === 0) { |
(Apply the same trim-based check at the orchestrator entrance.)
中文说明
纯空白的 workingDir 能通过两个 workflow 入口关卡——此处的 length === 0 检查与 orchestrator 的 !opts.workingDir——只在登记关卡深处被拒绝,且错误消息怪的是目录("not a registered linked worktree");而等价的 AgentTool 输入会先被 trim 并在上层以 "must be a non-empty string" 拒绝。已通过真实 sandbox 探针确认:派发收到了 {"workingDir":" "}。虽然失败关闭,但三个面现在确凿地在「哪一层拒绝空白值」上互相矛盾,且深层消息具有误导性。
失败场景:agent('x', { workingDir: ' ' }) 通过两处非空检查、解析为 /repo/ 、通过包含检查,然后在 isRegisteredLinkedWorktree 处以「登记状态」诊断拒绝一个空白值——模型会去修 worktree 登记而不是修参数。
建议修复如上(对 orchestrator 入口同样采用 trim 检查)。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const mainPath = firstLine.slice('worktree '.length).trim(); | ||
| return mainPath.length > 0 ? mainPath : null; |
There was a problem hiding this comment.
[Suggestion] getMainWorktreePath() silently mutates a main-tree path with leading/trailing whitespace: both lines[0]?.trim() (~line 360) and this payload .trim() strip the space/tab/CR that git's porcelain output preserves verbatim — producing a wrong anchor with NO residue line, so the newline-truncation detector can never fire (the record is well-formed). Probe-verified against real git (a trailing-space repo; porcelain preserves the space) and end-to-end: with a sibling repo at the trimmed path, containment anchored there and its registry validated a pin to a worktree outside the invoking repository; the two-site fix flipped the probe. Same trigger class as the sibling finding (self-inflicted path plus a sibling repo at the trimmed path), hence Suggestion.
Failure scenario: invoking repo at /srv/proj (trailing space) → the method returns /srv/proj; if that sibling exists as another repo with registered worktrees, a model-supplied pin to one of them passes both gates and the child rebinds outside the invoking repository. Without the sibling, pins fail closed with confusing refusals.
Suggested fix: strip only the line terminator, at both sites — const firstLine = (lines[0] ?? '').replace(/\r$/, ''); and take firstLine.slice('worktree '.length) without .trim() (keep the length > 0 guard). The alternative round-trip anchor validation closes this and the detector bypass above; note the getRepoTopLevel() fallback has the same trailing-whitespace trim, so cover the whole fallback chain.
中文说明
getMainWorktreePath() 会静默改变带前导/尾随空白的主树路径:lines[0]?.trim()(约第 360 行)与这里的载荷 .trim() 都会去掉 git porcelain 输出原样保留的空格/制表符/CR——产生一个没有任何残余行的错误锚点,换行截断检测器因此永远不会触发(记录是良构的)。已用真实 git 探针验证(尾随空格仓库;porcelain 保留空格)并端到端复现:当修剪后的路径处存在兄弟仓库时,包含检查以该处为锚、其登记册验证通过了钉到调用仓库之外 worktree 的钉住;修复两处 .trim() 后探针翻转。触发条件与相邻发现同类(自伤路径加修剪路径处的兄弟仓库),故定为 Suggestion。
失败场景:调用仓库位于 /srv/proj (尾随空格)→ 方法返回 /srv/proj;若该兄弟路径是另一个有已登记 worktree 的仓库,模型钉到其中之一可两关全过,子 agent 被重绑定到调用仓库之外;若兄弟不存在,钉住以令人困惑的拒绝失败关闭。
建议修复:两处都只去掉行终止符——const firstLine = (lines[0] ?? '').replace(/\r$/, '');,取 firstLine.slice('worktree '.length) 而不再 .trim()(保留 length > 0 守卫)。锚点往返校验可同时堵住此处与上面的检测器绕过;注意 getRepoTopLevel() 回退也有同样的尾随空白 trim,需覆盖整条回退链。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const realRepoRoot = await fs.realpath(repoRoot).catch(() => repoRoot); | ||
| const realResolved = await fs | ||
| .realpath(resolvedPath) | ||
| .catch(() => resolvedPath); |
There was a problem hiding this comment.
[Suggestion] Containment compares the two sides in different representations when only one realpath succeeds — canonical realRepoRoot against the verbatim spelling of a resolvedPath whose realpath rejected (absent path). Under a symlinked repo ancestor (macOS /tmp → /private/tmp, /var/folders, autofs homes), a nonexistent pin target yields path.relative('/private/tmp/repo', '/tmp/repo/…') → '../../…' → refusal "resolves outside this repository (/private/tmp/repo)" — naming a canonical root the caller never typed and hiding the true cause (the path does not exist). Probe-reproduced with real symlinked temp dirs; the fix flipped the probe to the accurate registration-gate message. Fails closed; the reverse direction cannot false-accept.
Failure scenario: session repo under /tmp; the model passes .qwen/tmp/review-pr-2 when only review-pr-1 exists → a misleading containment refusal instead of the registration gate's accurate "absent from git worktree list", so the model retries against a nonexistent containment problem (e.g. /private/… spellings) instead of correcting the path.
Suggested fix: keep both sides in one representation — on realpath(resolvedPath) rejection, run the containment comparison against the un-canonicalized repoRoot (migration-free for the existing stub tests); or reject early with an explicit "does not exist" error (that variant needs the plain-string stub tests moved to real temp dirs).
中文说明
当只有一侧 realpath 成功时,包含检查用不同表示比较两侧——规范化后的 realRepoRoot 对比 realpath 失败(路径不存在)的 resolvedPath 原文拼写。在仓库祖先为符号链接时(macOS /tmp → /private/tmp、/var/folders、autofs 家目录),不存在的钉住目标会得到 path.relative('/private/tmp/repo', '/tmp/repo/…') → '../../…' → 拒绝 "resolves outside this repository (/private/tmp/repo)"——点名了一个调用方从未输入的规范化根,掩盖了真实原因(路径不存在)。已用真实符号链接临时目录探针复现;修复后探针翻转为登记关卡的准确消息。失败关闭;反向不可能误接受。
失败场景:会话仓库位于 /tmp 下;模型在只有 review-pr-1 时传入 .qwen/tmp/review-pr-2 → 得到误导性的包含拒绝而非登记关卡准确的 "absent from git worktree list",模型会针对一个不存在的包含问题反复重试(如改用 /private/… 拼写)而不是纠正路径。
建议修复:让两侧保持同一表示——realpath(resolvedPath) 失败时改用未规范化的 repoRoot 做包含比较(现有 stub 测试无需迁移);或提前以明确的 "does not exist" 错误拒绝(该变体需把纯字符串 stub 测试迁到真实临时目录)。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const repoRoot = | ||
| (await probe.getMainWorktreePath()) ?? | ||
| (await probe.getRepoTopLevel()) ?? |
There was a problem hiding this comment.
[Suggestion] When getMainWorktreePath() correctly returns null (truncation detected — the good path of the round-2 fix — or worktree list unreadable) and the parent runs inside a linked worktree, this fallback chain degrades the anchor to the current worktree's own root (getRepoTopLevel() from inside a linked worktree answers that worktree's root), so every legitimate sibling-worktree pin is over-refused with guidance that cannot be satisfied. Real-git probe: the mislabeled refusal reproduces; the flip arm (clean main-tree path) succeeds on the identical layout; the registration gate would have passed the sibling (it reads gitdir files directly, newline-immune) — the refusal is purely the degraded anchor's. The rationale comment above names exactly this harm for the getMainWorktreePath()-success state but does not answer for the fallback state.
Failure scenario: a repo whose main-tree path contains a newline, two registered sibling worktrees wt1/wt2, cwd inside wt1 (the documented normal state for /review pipelines), the model pins wt2 → "working_dir … resolves outside this repository (…/wt1). Pass a worktree that lives inside the repository." — the target genuinely lives inside the repository and no registered worktree exists inside the current one, so every retry fails and the pipeline stalls. Fails closed — no escape.
Suggested fix: track which arm produced the anchor; when it fell back past getMainWorktreePath(), say so in the refusal — e.g. "the repository's main working tree could not be determined (git worktree list unreadable or its path malformed), so containment was checked against the current worktree root."
中文说明
当 getMainWorktreePath() 正确地返回 null(检测到截断——第二轮修复的良性路径——或 worktree list 不可读)且父会话运行在 linked worktree 内时,这条回退链把锚点退化为当前 worktree 自己的根(从 linked worktree 内调用 getRepoTopLevel() 回答的是该 worktree 的根),于是每一个合法的兄弟 worktree 钉住都被过度拒绝,且指引无法满足。真实 git 探针:误标拒绝可复现;翻转臂(干净主树路径)在完全相同的布局下成功;登记关卡本可通过该兄弟(它直接读 gitdir 文件,不受换行影响)——拒绝纯粹来自退化的锚点。上方的动机注释恰好为 getMainWorktreePath() 成功的状态点名了此害,却没有为回退状态兜底。
失败场景:主树路径含换行的仓库、两个已登记兄弟 worktree wt1/wt2、cwd 在 wt1(/review 流水线的文档化常态),模型钉住 wt2 → "working_dir … resolves outside this repository (…/wt1). Pass a worktree that lives inside the repository."——目标确实在仓库内,且当前 worktree 内不存在任何已登记 worktree,任何重试都会失败,流水线卡死。失败关闭——无逃逸。
建议修复:记录锚点出自哪一支;当回退到 getMainWorktreePath() 之后时,在拒绝文案中说明——例如 "the repository's main working tree could not be determined (git worktree list unreadable or its path malformed), so containment was checked against the current worktree root."
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // check rejects the main tree, a plain sub-directory (including a stale | ||
| // registry record whose directory was recreated), a worktree belonging to | ||
| // another repo, and a hand-crafted directory carrying a copied `.git` file. | ||
| if (!(await wtService.isRegisteredLinkedWorktree(resolvedPath))) { |
There was a problem hiding this comment.
[Suggestion] The validator resolves the pin path twice and returns the lexical spelling: containment computes realResolved (~line 106), then isRegisteredLinkedWorktree re-resolves independently (its first statement is a fresh fs.realpath, gitWorktreeService.ts:1330), and the resolver returns path: resolvedPath (~line 149) — which the rebind block binds as the child's cwd. Probe-verified deterministically (no race needed): with a same-repo-registered worktree living OUTSIDE the repo (git worktree add may place worktrees anywhere; the entry lands in this repo's .git/worktrees/) and a symlink passed as the pin, re-pointing the symlink after validation moved the spawned child's pwd outside the repository while both gates had passed; the resolve-once fix flipped the probe. WorkspaceContext snapshots the canonical root at construction (file tools largely absorbed), so the observable drift is the shell cwd. Bounded at Suggestion: registering the outside worktree requires same-repo write access (which a workflow subagent with shell plausibly has), and pinning is documented as a cwd pin — but the pin itself silently landing outside despite passing validation is not the "explicit absolute path" case the JSDoc carves out.
Failure scenario: a model with write access runs git worktree add /tmp/evil-wt, passes a symlink /repo/link → /repo/wt-in as workingDir; both gates pass on wt-in; the symlink is re-pointed to /tmp/evil-wt any time before or during the child run; the child's shell cwd resolves outside the repository while the run believes it is pinned inside.
Suggested fix (probe-verified, all 10 existing worktree-pin tests stay green): resolve once and thread the single resolution through — pass realResolved (not resolvedPath) to isRegisteredLinkedWorktree/getRegisteredWorktreeBranch, and return path: realResolved so the child binds to the exact directory object that was validated.
中文说明
校验器对钉住路径解析了两次并返回词法拼写:包含检查计算 realResolved(约第 106 行),随后 isRegisteredLinkedWorktree 独立地重新解析(其第一条语句就是新的 fs.realpath,gitWorktreeService.ts:1330),而 resolver 返回 path: resolvedPath(约第 149 行)——重绑定块把它绑为子 agent 的 cwd。已用确定性探针验证(无需竞态):把一个登记在本仓库、但实际位于仓库之外的 worktree(git worktree add 可以把 worktree 放在任何地方;条目落在本仓库的 .git/worktrees/)配合一个作为钉住目标的符号链接,在校验通过后重新指向符号链接,即可让派生出的子进程 pwd 落到仓库之外——而两道关卡都曾通过;「只解析一次」的修复使探针翻转。WorkspaceContext 在构造时对规范化根做快照(文件工具基本被吸收),可观察的漂移是 shell 的 cwd。定为 Suggestion:登记仓库外的 worktree 需要同仓库写权限(带 shell 的 workflow 子 agent 很可能有),且钉住按文档只是 cwd 钉住——但「通过校验的钉住却静默落在仓库外」并不是 JSDoc 明确排除的「显式绝对路径」情形。
失败场景:有写权限的模型执行 git worktree add /tmp/evil-wt,把符号链接 /repo/link → /repo/wt-in 作为 workingDir 传入;两道关卡都按 wt-in 通过;符号链接在子运行之前或期间被重新指向 /tmp/evil-wt;子 agent 的 shell cwd 解析到仓库之外,而运行以为自己被钉在仓库内。
建议修复(探针已验证,现有 10 个 worktree-pin 测试全部保持绿):只解析一次并把单一结果贯穿到底——把 realResolved(而非 resolvedPath)传给 isRegisteredLinkedWorktree/getRegisteredWorktreeBranch,并返回 path: realResolved,使子 agent 绑定到被校验过的那个目录对象。
— qwen3.8-max via Qwen Code /review (v0.21.10)
- Round-trip-validate the porcelain main-tree anchor (git-common-dir must
agree) so attribute-shaped or trailing-newline truncations cannot aim
the pin gate at a different repository's worktree registry
- Preserve legitimate path whitespace when parsing the anchor and the
--show-toplevel fallback (terminator-only strip, untrimmed raw output)
- Thread one canonical realpath through both pin gates and the rebind so
a re-pointed symlink cannot land the child where neither gate looked
- Canonicalise both containment sides or neither, so an absent target
reaches the registration gate's accurate message instead of a
manufactured outside-the-repository refusal
- Name the degraded anchor in the containment refusal when the main
working tree could not be determined
- Throw on agent({workingDir, isolation}) at the orchestrator entrance
(revived plain object — not evadable by the sandbox getter trick)
- Trim-based blank check for workingDir at both workflow entrances
- Document stallMs in the workflow schema and extend the tool-level
capability enumeration to workingDir and stallMs
Local real-stack verification (maintainer)I built both arms from source and ran the real bundled CLI against real git worktrees, because the PR's own risk section says the pin was never exercised end to end:
That run now exists. The pin works, the gate holds, and both dispatch guards are load-bearing. Details, evidence and three non-blocking notes below. Verdict: LGTM to merge. Setup
The fixture is a real repository containing: the main working tree; a registered linked worktree carrying an uncommitted file (the "its uncommitted state is the whole point" case); a detached-HEAD worktree; a registered worktree living outside the repo directory; a plain sub-directory; a second repository with its own worktree; a directory carrying a copied 1. The pin actually pinsSame script, both arms. The pinned agent reports the worktree and reads the file that only exists there uncommitted; the unpinned control (which takes the fast dispatch path) reports the parent tree.
The detached and outside-the-repo rows matter: they confirm the branch label is not a gate, and that the doc correction ("may live anywhere on disk", replacing "must live inside the repository") describes the code. 2. The gate refuses everything it claims to — and creates nothing when it doesTwelve refusals, each an end-to-end run. In all twelve the provider journal shows zero subagent requests — the dispatch aborts before an agent exists, which is what the unit test asserts and what actually happens.
3. Both dispatch guards are load-bearing (mutation tests)I removed each guard, rebuilt, and re-ran the same scenario:
Both mutations are caught by the PR's own unit tests (9 failures, including 4. The resume key change, end to endTwo consecutive
5. The bounds are genuinely operator-tunableA subagent scripted to never terminate; the turn cap is the only thing that stops it. Provider turns counted from the journal:
Resolver behaviour, read off the built code:
And the motivation checks out: under 6. No regression on the shared surfaces
The "byte-identical error text" claim in the PR body holds literally. 7. The PR's own test plan, on macOS
The PR marks 🍏 macOS as Three non-blocking notes1. The two new env variables are not in the tool description, unlike every other workflow bound. 2. The
I think it's the right call, but the body says "Breaking changes / migration notes: none", and this does change behaviour for an existing script that passes a non-number. Worth one line in the body so a reviewer isn't surprised by it in the diff. 3. Cosmetic: the refused path is echoed twice. 中文说明本地真实环境验证(维护者)我从源码分别构建了两条腿,用真实打包的 CLI 跑真实 git worktree,因为 PR 自己的风险章节写着:
这次运行补上了。钉住是真的生效、闸门守得住、两处派发保护都是承重的。 结论:同意合并,另有三条不阻塞的说明。 环境
夹具是一个真实仓库,包含:主工作树;一个带未提交文件的已登记 linked worktree(正是"未提交状态才是重点"那个场景);一个 detached HEAD 的 worktree;一个位于仓库目录之外的已登记 worktree;一个普通子目录;另一个仓库及其自己的 worktree;一个携带指向真实注册表条目的 1. 钉住是真的生效同一份脚本、两条腿。被钉住的 agent 报告 worktree 路径,并读到了只在那里未提交存在的文件;未钉住的对照组(走快速派发路径)报告父工作树。
detached 与"仓库外"这两行很关键:它们确认了分支只是标签而非关卡,并且文档改动(把"必须在仓库内部"改为"可以在磁盘任何位置")描述的是代码的真实行为。 2. 闸门该拒的全拒了,而且拒的时候什么都不创建十二种拒绝,每种都是一次端到端运行。十二次的 provider 台账里子 agent 请求数都是 0——派发在 agent 存在之前就中止了,这正是单测断言的行为,也是实际发生的行为。
3. 两处派发保护都是承重的(变异测试)我逐个删掉保护、重新构建、再跑同一场景:
两个变异都被 PR 自带的单测抓住(9 条失败,其中包括 4. resume key 改动的端到端验证同一次会话里连续两次
5. 上限确实变成了运维可调一个被脚本化成永不终止的子 agent,只有轮数上限能停下它。轮数从台账里数出来:
从构建产物上读出的解析行为:
动机也验证到了:在 6. 共享面零回归
PR 正文里"错误文本逐字节一致"的说法,字面成立。 7. PR 自己的测试方案,在 macOS 上
PR 把 🍏 macOS 标为 三条不阻塞的说明1. 两个新环境变量没有进工具描述,而其余所有 workflow 上限都进了。 2.
我认为这个改动是对的,但正文写着*"破坏性变更 / 迁移说明:无"*,而对一个已经在传非数字 3. 观感问题:被拒的路径回显了两次。 |
|
🤖 Addressed the latest review feedback (round 10/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 10/100 轮)。改动内容与我反驳保留之处如下: Review-round summary — PR #8972The single issue-level comment this round is the maintainer's real-stack verification report with an LGTM-to-merge verdict and three explicitly non-blocking notes. No failed checks, no inline comments, no conflict resolution requested ( Feedback points and dispositions1. The two new env variables are not in the tool description — ImplementedClaim verified before changing anything: Change (one sentence, per the maintainer's "one-line fix" scope): the Runtime paragraph of the tool description now states, in the sentence cluster that already lists the other bounds — "Each subagent attempt is separately capped at 50 turns ( 2. The
|
|
📊 Takeover milestone — round 10/100, in the current window. Census: 10 pushed fix(es), 7 no-change review(s), 0 timeout(s), 0 rejected attempt(s), 0 other round(s) (crash / model error / gate error / infra), 3 base update(s). This many rounds deserves a human look. Options: keep going (fine — nothing changes), split or reduce the PR if rounds keep accumulating, or release takeover (remove the 中文说明📊 接管里程碑 —— 第 10/100 轮(当前窗口)。统计:推送修复 10 次、审阅无需改动 7 次、超时 0 次、验证拒绝 0 次、其他轮次(崩溃/模型错误/门错误/infra)0 次、base 更新 3 次。 轮次到这个量值得人工看一眼。可选:继续(无需操作);若轮次持续累积,考虑拆分或缩减 PR;或释放接管(移除 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — reached the round cap (5) without two consecutive dry rounds; round 5 surfaced only re-detections of already-open PR comments.
Not reviewed: build-and-test — Test (macos-latest / windows-latest, Node 22.x) was skipped in CI (merge_group-only lanes) and the unit suite did not run locally on those OSes; the Windows lane is the one that would exercise this PR's platform path-arithmetic test fixes (the R4-5 class).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the e2e suite did not run locally.
Test Plan (not a blocker): src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — reached the round cap (5) without two consecutive dry rounds; round 5 surfaced only re-detections of already-open PR comments。
未审查:build-and-test — Test (macos-latest / windows-latest, Node 22.x) was skipped in CI (merge_group-only lanes) and the unit suite did not run locally on those OSes; the Windows lane is the one that would exercise this PR's platform path-arithmetic test fixes (the R4-5 class)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the e2e suite did not run locally。
Test Plan(非阻断):src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // Strip only the line terminator (a CRLF carriage return included): | ||
| // git preserves a path's leading/trailing whitespace verbatim, and a | ||
| // trim would silently mutate it into a different, wrong anchor. | ||
| const firstLine = (lines[0] ?? '').replace(/\r$/, ''); |
There was a problem hiding this comment.
[Critical] R8-4: Escalated from the round-8 Suggestion (the thread at line 327) with executed probe evidence. The terminator strips introduced by this PR strip a trailing CR that is legitimately PART of the directory name — git's command stdout is LF-terminated on all platforms, so /\r?\n$/ (line 327) and /\r$/ (this line) guard a phantom while performing exactly the mutation the adjacent comments forbid ("a trim would silently mutate it into a different, wrong anchor"). getMainWorktreePath()'s round-trip validation refuses the mutated anchor, but the fallback getRepoTopLevel() reintroduces it WITHOUT any round-trip. — Failure scenario: a repository rooted at a directory named with a trailing CR (legal on Linux/macOS) → getRepoTopLevel() returns the nonexistent CR-less sibling path; when another repository exists at that sibling path, the pin gate probes git there and reads THAT repository's worktree registry — a foreign repo's registered worktree is accepted as a pin target, a demonstrated bypass of the cross-repo registry gate the round-trip validation exists to prevent.
Witness (real git, this commit):
PROBE getRepoTopLevel : "/tmp/crprobe-a1toBS/repo" (real repo path: ".../repo\r") top === repo : false
PROBE pin S-worktree from R: {"path":"…/swt","branch":"sbranch","slug":"swt","repoRoot":"…/repo"}
← foreign repo S's worktree ACCEPTED; the gate read S's registry
Flip: with /\n$/-only strips → top === repo: true, R's own worktree accepted, S's worktree refused
Suggested fix: strip only the LF — out.replace(/\n$/, '') at line 327 and drop the /\r$/ strip here; a stray CR then survives as path data, the round-trip compares true anchors, and the fallback keeps the path verbatim.
中文说明
R8-4:由第 8 轮的建议(line 327 处的讨论串)升级为阻断,附本次执行的探针证据。本 PR 引入的行终止符剥离会剥掉合法属于目录名一部分的尾部 CR——git 命令输出在所有平台上都以裸 LF 结尾,因此 /\r?\n$/(327 行)与 /\r$/(本行)防的是一个幻影场景,却恰恰执行了相邻注释明令禁止的变异(「trim 会把路径悄悄变异成另一个错误的锚点」)。getMainWorktreePath() 的 round-trip 校验会拒绝被变异的锚点,但回退路径 getRepoTopLevel() 又在没有任何 round-trip 的情况下把它重新引入。失败场景:仓库根目录名带尾部 CR(Linux/macOS 上合法)→ getRepoTopLevel() 返回一个不存在的、去掉 CR 的同名兄弟路径;若该兄弟路径上恰好存在另一个仓库,钉住门禁就会在那个路径上探测 git 并读取那个仓库的 worktree 登记表——外部仓库的已登记 worktree 会被接受为钉住目标,这是对 round-trip 校验本要防御的跨仓库登记表混淆的实际绕过。
见证(真实 git,本提交):见上方探针输出——getRepoTopLevel 返回去掉 CR 的路径(top === repo : false);从 R 钉住 S 的 worktree 被接受(门禁读取了 S 的登记表);改为仅剥 /\n$/ 后探针翻转(R 自己的 worktree 被接受,S 的被拒绝)。
建议修复:只剥 LF——327 行改为 out.replace(/\n$/, ''),并去掉此处的 /\r$/ 剥离;游离的 CR 将作为路径数据保留,round-trip 比较的是真实锚点,回退路径保留路径原文。(第 8 轮以「悄悄错误的锚点」的定性将此处报为建议;本轮探针证明了门禁绕过后果,故升级。)
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(created[0]!.runConfig).toEqual({ | ||
| max_turns: DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS, |
There was a problem hiding this comment.
[Suggestion] R9-1 (1/2): The per-subagent safety floors are now anchored nowhere — this diff replaced the T11 test's literal max_turns: 50, max_time_minutes: 10 assertion with references to the same DEFAULT_* constants production reads, so every assertion moves together with the constant, and resolveSubagentBound never validates its own default (the < 1 check applies only to env overrides). — Failure scenario: mutation-probed at this commit, DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS = 50 → 5000 and → 0 each keep 188/188 tests green (the 0 case flows straight into runConfig, dissolving the safety floor); a future edit of the constant (turns/minutes mix-up, stray zero) ships silently — subagents either run 100× longer than the intended floor or die instantly, surfacing as null slots in parallel(). See (2/2) for the mirror-image ceiling gap.
| expect(created[0]!.runConfig).toEqual({ | |
| max_turns: DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS, | |
| expect(created[0]!.runConfig).toEqual({ | |
| max_turns: 50, |
(or anchor the literals once in the resolver describe block: expect(DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS).toBe(50); expect(DEFAULT_WORKFLOW_SUBAGENT_MAX_TIME_MINUTES).toBe(10);)
中文说明
R9-1(2 处之 1):单个子 agent 的安全下限值现在没有任何字面量锚点——本 diff 把 T11 测试中的字面量断言 max_turns: 50, max_time_minutes: 10 换成了与生产代码读取的同一组 DEFAULT_* 常量的引用,于是所有断言随常量一起移动,而 resolveSubagentBound 从不校验自己的默认值(< 1 检查只作用于环境变量覆盖)。失败场景:已在本提交上做变异探针——DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS = 50 → 5000 与 → 0 均保持 188/188 测试全绿(0 的情形会直接流入 runConfig,安全下限被消解);未来对常量的任何编辑(轮数/分钟数混淆、多打一个零)都会悄无声息地发布——子 agent 要么运行得比预期下限长 100 倍,要么瞬间终止,在 parallel() 中表现为 null 槽位。镜像的上限缺口见(2 处之 2)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| resolveSubagentMaxTurns({ | ||
| QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS: '999999', | ||
| }), | ||
| ).toBe(HARD_WORKFLOW_SUBAGENT_MAX_TURNS_CEILING); |
There was a problem hiding this comment.
[Suggestion] R9-1 (2/2): Same tautological anchoring for the HARD ceilings — the clamp tests compare resolver output against the very constants they clamp to, so HARD_WORKFLOW_SUBAGENT_MAX_TURNS_CEILING (500) and HARD_WORKFLOW_SUBAGENT_MAX_MINUTES_CEILING (100) are asserted only against themselves; grep finds no literal 500/100 anchor for these bounds anywhere in packages/core. — Failure scenario: mutation-probed at this commit, 500 → 5000 and 100 → 1000 keep 203/203 tests green — a 10× relocation of both safety ceilings produces zero test signal. The one-line fix leaves the rest of the constant-referencing convention untouched.
| resolveSubagentMaxTurns({ | |
| QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS: '999999', | |
| }), | |
| ).toBe(HARD_WORKFLOW_SUBAGENT_MAX_TURNS_CEILING); | |
| resolveSubagentMaxTurns({ | |
| QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS: '999999', | |
| }), | |
| ).toBe(500); |
(and likewise expect(HARD_WORKFLOW_SUBAGENT_MAX_MINUTES_CEILING).toBe(100); for the minutes assertion below)
中文说明
R9-1(2 处之 2):HARD 上限是同样的自指锚定——夹紧测试把 resolver 的输出与它夹向的同一组常量比较,因此 HARD_WORKFLOW_SUBAGENT_MAX_TURNS_CEILING(500)与 HARD_WORKFLOW_SUBAGENT_MAX_MINUTES_CEILING(100)只对着它们自己断言;全 packages/core 内 grep 不到这两个上限的任何字面量 500/100 锚点。失败场景:已在本提交上做变异探针——500 → 5000 与 100 → 1000 均保持 203/203 测试全绿——两个安全上限被放大 10 倍却没有任何测试信号。一行修复即可,且不影响其余引用常量的既有惯例。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| 'subagent time cap, not this watchdog); the timer is suspended ' + | ||
| 'while a tool is in flight, so a legitimately slow tool is not ' + | ||
| 'a stall. ' + | ||
| `Default ${DEFAULT_STALL_MS} (override via \`${MAX_WORKFLOW_STALL_MS_ENV}\`, whole seconds); \`0\` disables the watchdog. Wall time ` + |
There was a problem hiding this comment.
[Suggestion] R9-4: The doc comment above WORKFLOW_TOOL_DESCRIPTION (~lines 545-562) enumerates the description's interpolated constants — "The agent cap and the two env knobs are interpolated from the orchestrator's exported constants (DEFAULT_MAX_AGENTS_PER_RUN, MAX_WORKFLOW_AGENTS_ENV, MAX_WORKFLOW_CONCURRENCY_ENV) in both halves" — and was exact at the merge base, but this diff adds seven interpolation sites without touching the comment: the runtime half gains the four subagent-bound constants, and the script half gains DEFAULT_STALL_MS, MAX_STALL_ATTEMPTS and MAX_WORKFLOW_STALL_MS_ENV (this line). — Concrete cost: the comment is the maintainer-facing map of which constants back model-visible numbers (its stated purpose is preventing hand-sync drift); a maintainer editing the caps or auditing the description trusts an enumeration that now undercounts five env knobs as two and misdescribes the script half's sourcing (now also workflow-stall.js constants) — in the very PR whose test comments call the description anchors load-bearing.
Suggested fix: update the enumeration, or replace the fixed list with a pointer to the two halves (four orchestrator subagent-bound constants in the runtime half; three workflow-stall constants in the script half).
中文说明
R9-4:WORKFLOW_TOOL_DESCRIPTION 上方(约 545-562 行)的文档注释列举了描述文本所插值的常量——「agent 上限与两个 env 旋钮由 orchestrator 导出的常量(DEFAULT_MAX_AGENTS_PER_RUN、MAX_WORKFLOW_AGENTS_ENV、MAX_WORKFLOW_CONCURRENCY_ENV)在两个半区插值」——在合并基上它是精确的,但本 diff 新增了七处插值却没有改动该注释:runtime 半区新增四个子 agent 上限常量,script 半区新增 DEFAULT_STALL_MS、MAX_STALL_ATTEMPTS 与 MAX_WORKFLOW_STALL_MS_ENV(本行)。具体代价:该注释是维护者了解「哪些常量支撑模型可见数字」的索引(其声明目的就是防止手工同步漂移);修改上限或审查描述的维护者会信任一份现在把五个 env 旋钮少算成两个、且错述 script 半区来源(现在还包括 workflow-stall.js 的常量)的列举——而恰恰是这个 PR,其测试注释声称描述锚点是承重的。
建议修复:更新列举内容,或把固定清单换成指向两个半区的说明(runtime 半区的四个 orchestrator 子 agent 上限常量;script 半区的三个 workflow-stall 常量)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
🤖 Addressed the latest review feedback (round 11/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 11/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8972One commit: Feedback points and dispositions[Critical] R8-4 — trailing-CR strips mutate directory-name data and bypass the cross-repo registry gate (gitWorktreeService.ts:374, thread at :327) — RESOLVED (fixed) Reproduced before changing anything, per the source-blind rule. A real-git regression test (added to Fix (the reviewer's suggested one): strip only the LF — [Suggestion] R9-1 (1/2) — per-subagent safety floors anchored nowhere (workflow-orchestrator.test.ts:1922) — RESOLVED (fixed) Mutation-probed at the pre-fix commit: [Suggestion] R9-1 (2/2) — HARD ceilings asserted against themselves (workflow-orchestrator.test.ts:2526) — RESOLVED (fixed) Same probe run mutated [Suggestion] R9-4 — stale constant enumeration in the WORKFLOW_TOOL_DESCRIPTION doc comment (workflow.ts:135) — RESOLVED (fixed) Verified the claim by grepping every interpolation site: the script half interpolates the three workflow-stall constants ( Deferred non-Critical feedbackCritical-only mode is active; the VerificationCommands actually run (working tree before commit = the committed diff):
中文说明Autofix 审查轮次 — PR #8972单个提交: 反馈点与处理[Critical] R8-4 — 尾部 CR 剥离会变异目录名数据,并绕过跨仓库登记表门禁(gitWorktreeService.ts:374,及 :327 处的讨论串)— 已解决(已修复) 按来源盲规则,先复现再改动。在 修复(即审查者建议的方案):只剥 LF—— [Suggestion] R9-1(2 处之 1)— 单个子 agent 的安全下限没有任何锚点(workflow-orchestrator.test.ts:1922)— 已解决(已修复) 已在修复前提交上做变异探针: [Suggestion] R9-1(2 处之 2)— HARD 上限对着自己断言(workflow-orchestrator.test.ts:2526)— 已解决(已修复) 同一次探针还将 [Suggestion] R9-4 — WORKFLOW_TOOL_DESCRIPTION 文档注释中的常量列举已过时(workflow.ts:135)— 已解决(已修复) 通过 grep 全部插值点核实了该发现:script 半区除共享的三个常量外还插值三个 workflow-stall 常量(第 129/135 行的 延后的非 Critical 反馈仅处理 Critical 的模式已生效; 验证实际执行的命令(提交前的工作区 = 已提交的 diff):
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (macos-latest / windows-latest, Node 22.x) was skipped in CI (merge_group-only lanes) and the unit suite did not run locally on those OSes; the Windows lane is the one that would exercise this PR's platform path-arithmetic test fixes (the R4-5 class).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the e2e suite did not run locally.
Test Plan (not a blocker): src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Test (macos-latest / windows-latest, Node 22.x) was skipped in CI (merge_group-only lanes) and the unit suite did not run locally on those OSes; the Windows lane is the one that would exercise this PR's platform path-arithmetic test fixes (the R4-5 class)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the e2e suite did not run locally。
Test Plan(非阻断):src/agents/worktree-pin.test.ts — no such file or directory; src/agents/runtime/workflow-orchestrator.test.ts — no such file or directory; src/agents/runtime/workflow-sandbox.test.ts — no such file or directory; src/agents/runtime/workflow-journal.test.ts — no such file or directory; src/tools/workflow/workflow.test.ts — no such file or directory; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round: no action needed (PR #8972)Critical-only mode is active (5 change-producing rounds are already complete), and this round's actionable sections are all empty:
What was deferred by the workflow (audit record only, left open for human follow-up per Critical-only rules — no code changes, thread resolutions, or replies were made for it):
Result: no code changes this round; the branch head is unchanged ( 中文说明Autofix 本轮:无需处理(PR #8972)当前已处于仅处理 Critical 的模式(此前已完成 5 个产生改动的轮次),本轮所有可执行区域均为空:
以下内容被工作流延后处理(仅为审计记录,按 Critical-only 规则留待人工跟进——未对其做任何代码改动、线程解决或回复):
结果: 本轮未做任何代码改动;分支头保持不变( Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Local real-stack re-verification at the new tip (maintainer)Two commits landed after my previous verification at
I rebuilt a third arm at the new tip and ran both the delta and the whole previously-verified surface again, against real git worktrees with the real bundled CLI. Both new commits do what they say, both are covered by load-bearing tests, and nothing regressed. Verdict: LGTM to merge. Three non-blocking notes, and one correction to my own earlier comment, below. Setup
1.
|
| env knob the model can discover | base | prev | head |
|---|---|---|---|
QWEN_CODE_MAX_WORKFLOW_AGENTS / …_CONCURRENCY / …_SECONDS |
yes | yes | yes |
QWEN_CODE_WORKFLOW_STALL_SECONDS |
no | yes | yes |
QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS |
no | no | yes |
QWEN_CODE_WORKFLOW_AGENT_MAX_MINUTES |
no | no | yes |
The numbers are interpolated, not prose — and I checked that the interpolation is live rather than a coincidence. Mutating DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS from 50 to 7 in the built bundle moved both halves at once:
- the description the model received became "capped at 7 turns", and
- the looping subagent was cut off at exactly 7 provider turns.
So the model-visible number cannot drift from the enforced one. The new assertion is load-bearing too: deleting the added sentence fails workflow.test.ts > description carries both the runtime facts and the orchestration policy, and because it anchors through the exported constants, a rename on the runtime side fails there as well.
2. 0e365af8 — the CR fix is real, and the pin only works with it
First, the premise. \r is a legal byte in a POSIX filename, and for a repository whose directory name ends with one, real git answers:
$ git rev-parse --show-toplevel | xxd | tail -1
... 2f 72 65 70 6f 0d 0a /repo\r\n
The \r is path data; the \n alone is the terminator. That is exactly the claim the commit's new comment makes, and it holds on real git here.
I built a repository whose main working tree path ends with CR, with a registered linked worktree carrying an uncommitted marker, then measured the two readers the commit touches from each arm's built core:
| arm | getRepoTopLevel() |
getMainWorktreePath() |
|---|---|---|
base 8e0033d |
…/repo — CR eaten by .trim() |
(method does not exist yet) |
prev eaa6d6b |
…/repo — CR eaten by /\r?\n$/ |
null (round-trip validation rejects the mangled anchor) |
head 0e365af |
…/repo\r (…2f7265706f0d) |
…/repo\r (…2f7265706f0d) |
End to end, same script, same repository — the agent reports where it actually is:
| arm | agent({workingDir: "/var/tmp/pr8972cr/wt\r"}) |
agent's real pwd |
marker it read | subagents created |
|---|---|---|---|---|
| head | accepted | …/wt^M — last bytes 2f77740d = /wt\r |
CR_PINNED_WORKTREE |
1 (+1 unpinned control → CR_MAIN_TREE) |
| prev | refused | — | — | 0 |
Neither /var/tmp/pr8972cr/wt nor …/repo exists without the CR, so the marker file is the proof: the agent is in the exact directory object, not a lookalike. Note the refusal text on prev echoes workingDir "/var/tmp/pr8972cr/wt" — the mangling, visible in the message.
The new tests are load-bearing. Reverting both sites to the pre-0e365af8 behaviour fails 3 tests, one of them against real git:
FAIL gitWorktreeService.linked.integ.test.ts > (real git) > preserves a trailing CR in the repository directory name
FAIL gitWorktreeService.test.ts > getMainWorktreePath > preserves a trailing CR in the main worktree path
FAIL gitWorktreeService.test.ts > getRepoTopLevel > preserves a trailing CR in the repository top-level path
3. The platform claim is load-bearing — and if it is ever wrong, it fails closed
0e365af8 rests on one assertion: "git's stdout is LF-terminated on every platform." I cannot falsify that on macOS with real git, so I manufactured the counterfactual — a git shim on PATH that re-terminates the two touched commands with CRLF, passing everything else through untouched — and asked the only question that matters:
| arm, under a git whose stdout really is CRLF-terminated | getMainWorktreePath() |
end-to-end pin | subagents created |
|---|---|---|---|
prev eaa6d6b |
correct path | pinned, PINNED_WORKTREE |
2 |
head 0e365af |
null |
refused | 0 |
So the trade is explicit: head gives up tolerance of a hypothetical CRLF-emitting git in exchange for correctness on a CR-in-path, and on such a platform workingDir would refuse every path rather than pin the wrong tree. That is the safe direction, and it is why this is a note rather than a blocker — see note 2.
4. Everything from the last review still holds at the new tip
Re-ran the whole suite against 0e365af:
| surface | result |
|---|---|
| pin accepted: relative, absolute, detached-HEAD, outside-the-repo, non-sandbox | 5/5, each agent in the right tree with the right marker |
gate refusals: main tree, plain dir, other repo's worktree, copied .git file, stale registry record, missing, traversal, workingDir+isolation (incl. hidden behind a getter), non-string, whitespace, DEL/NEL control chars |
12/12 refused, 0 subagents created in every one |
| resume, identical script | 1 dispatch total across both runs (run 2 fully cached) |
resume, only workingDir changed |
2 dispatches, answer switches to DETACHED_WORKTREE |
turn cap: unset / =3 / =999999 / =2.5 |
50 / 3 / 500 (clamped) / 50 (rejected → default) |
parallel() of capped agents |
[null, null] — the motivating failure reproduces |
AgentTool working_dir refusal text, base vs head |
sha256 identical (84ef604ab34f6d87, 410 bytes) |
isolation:'worktree' |
unchanged |
stallMs type gate |
base runs, head refuses (see note 4) |
5. Tests, on macOS
| command | result |
|---|---|
the PR's own plan — npx vitest run src/agents/ src/tools/ |
127 files passed, 1 skipped · 4278 passed, 6 skipped (4284) |
npx vitest run src/services/gitWorktreeService |
4 files, 71 passed |
The PR marks 🍏 macOS as Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) are skipping; only the ubuntu leg runs.
Notes (none blocking)
1. The last commit's own tests are outside the PR's stated verify command.
The Reviewer Test Plan says cd packages/core && npx vitest run src/agents/ src/tools/. All three CR tests 0e365af8 adds live in src/services/, so a reviewer who follows the plan exactly runs none of them — including the real-git integration test, which is the strongest one in the commit. Adding src/services/gitWorktreeService to that command line is a one-line fix.
2. Windows is where the platform claim actually bites, and nothing exercises it.
The CR change is correct on macOS and Linux, where git demonstrably emits LF. Windows is marked workingDir refusing every path with a "not a registered linked worktree" message that names the wrong cause. Worth a sentence in the doc comment, or a Windows smoke test before anyone relies on workingDir there.
3. The body's test totals are stale.
It states 4250 passed | 6 skipped (4256). On this tip the same command gives 4278 passed | 6 skipped (4284) — tests were added since the body was written. Cosmetic, but it is the number a reviewer diffs against.
4. Two notes from my last review are still open. Neither is new, both are body-only:
- The
stallMstype gate is a real behaviour change (baseruns a script passingstallMs: 'soon',headrefuses it) that the body does not list, while stating "Breaking changes / migration notes: none". - The refused path is echoed twice in the refusal message — the outer frame adds the spelling the script wrote, the resolver message already carries the resolved path. Cosmetic.
Correction to my previous comment
I listed QWEN_CODE_MAX_TOKENS_PER_WORKFLOW among the workflow bounds already named in the tool description. It is not there, on any arm — the description mentions the token budget only via budget.total, and the env variable is surfaced to the user in a TUI run notice instead ("Workflows have no per-run token cap. Set QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=<n>…"), not to the model. The substance of that note — that the two subagent knobs the PR argues hardest for were the ones the model could not discover — was correct, and 49bfc9f5 has fixed it.
中文说明
新顶端的本地真实环境复验(维护者)
在我上一次验证(eaa6d6b)之后又落了两个 commit:
49bfc9f5—— 把两个子 agent 环境变量上限写进 workflow 工具描述0e365af8—— 保留 git worktree 路径答案里结尾的 CR
我按新顶端重新构建了第三条腿,既跑了增量,也把上次验证过的全部面重新跑了一遍,用真实打包 CLI 对真实 git worktree 执行。两个新 commit 都名副其实,都有承重的测试覆盖,且没有回归。49bfc9f5 正好关掉了我上次的第 1 条说明。
结论:同意合并。 另有三条不阻塞的说明,以及一处对我自己上一条评论的更正。
环境
| 三条腿 | head = 0e365af(PR 顶端)· prev = eaa6d6b(我上次验证时的顶端)· base = 8e0033d(与 main 的 merge-base,未变) |
| 构建 | 每条腿独立 npm ci → npm run build → npm run bundle;所有运行都执行该腿的 dist/cli.js |
| 调用方式 | qwen --prompt … --approval-mode yolo < /dev/null(headless、stdin 关闭)+ 3 次真实交互式 TUI 会话 |
| 模型侧 | 记录型 OpenAI 兼容 mock —— 这次还会把 CLI 实际发出的 tools[] 数组原文落盘,所以"模型能不能发现这个旋钮"是从线上报文读出来的,不是从源码推的 |
| 子 agent 干的活 | 真实 run_shell_command,把 pwd 过一遍 cat -v、打印 pwd 末尾字节的十六进制、以及 cat WHERE_AM_I.txt |
| 规模 | 40 次 headless CLI 运行 + 4 次终端截屏,757 条记录的 provider 请求 |
| 主机 | macOS 26.6 arm64,Node v24.18.1,git 2.55.0 |
1. 49bfc9f5 —— 两个旋钮现在模型可见了,第 1 条说明关闭
我上次说:PR 正文声称这两个环境变量*"均已写入工具描述与代码注释"*,而实际上只在代码里。三条腿的 provider 请求实测:
| 模型能发现的环境变量 | base | prev | head |
|---|---|---|---|
QWEN_CODE_MAX_WORKFLOW_AGENTS / …_CONCURRENCY / …_SECONDS |
有 | 有 | 有 |
QWEN_CODE_WORKFLOW_STALL_SECONDS |
无 | 有 | 有 |
QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS |
无 | 无 | 有 |
QWEN_CODE_WORKFLOW_AGENT_MAX_MINUTES |
无 | 无 | 有 |
数值是插值出来的、不是手写文案 —— 而且我验证了这个插值是活的而非巧合。把构建产物里的 DEFAULT_WORKFLOW_SUBAGENT_MAX_TURNS 从 50 改成 7,两处同时跟着动:
- 模型收到的描述变成*"capped at 7 turns"*,并且
- 那个死循环子 agent 恰好在第 7 轮 provider 请求被切断。
也就是说,模型看到的数字不可能与实际执行的数字漂移。新加的断言也是承重的:删掉新增的那句话会让 workflow.test.ts > description carries both the runtime facts and the orchestration policy 失败;而且它是通过导出常量锚定的,所以运行时侧改名同样会在这里失败。
2. 0e365af8 —— CR 修复是真的,没有它这个钉法就不成立
先说前提。\r 是 POSIX 文件名里的合法字节;对一个目录名以 CR 结尾的仓库,真实 git 的回答是:
$ git rev-parse --show-toplevel | xxd | tail -1
... 2f 72 65 70 6f 0d 0a /repo\r\n
\r 是路径数据,只有 \n 是终止符。这正是该 commit 新注释所主张的,并且在真实 git 上成立。
我造了一个主工作树路径以 CR 结尾的仓库,并在其中挂了一个带未提交标记文件的已登记 linked worktree,然后从每条腿的构建产物里直接测量该 commit 触及的两个读取器:
| 腿 | getRepoTopLevel() |
getMainWorktreePath() |
|---|---|---|
base 8e0033d |
…/repo —— CR 被 .trim() 吃掉 |
(方法尚不存在) |
prev eaa6d6b |
…/repo —— CR 被 /\r?\n$/ 吃掉 |
null(往返校验把被篡改的锚点挡了下来) |
head 0e365af |
…/repo\r(…2f7265706f0d) |
…/repo\r(…2f7265706f0d) |
端到端,同一份脚本、同一个仓库 —— agent 报告它实际在哪:
| 腿 | agent({workingDir: "/var/tmp/pr8972cr/wt\r"}) |
agent 真实 pwd |
读到的标记 | 创建的子 agent |
|---|---|---|---|---|
| head | 接受 | …/wt^M —— 末尾字节 2f77740d 即 /wt\r |
CR_PINNED_WORKTREE |
1(外加 1 个未钉住的对照 → CR_MAIN_TREE) |
| prev | 拒绝 | — | — | 0 |
去掉 CR 之后的 /var/tmp/pr8972cr/wt 和 …/repo 都不存在,所以标记文件就是铁证:agent 落在那个精确的目录对象上,而不是一个长得像的路径。另外注意 prev 的拒绝文本回显的是 workingDir "/var/tmp/pr8972cr/wt" —— 被篡改的结果,直接写在消息里。
新增测试是承重的。把两处都还原成 0e365af8 之前的行为,会失败 3 条测试,其中一条是打真实 git 的:
FAIL gitWorktreeService.linked.integ.test.ts > (real git) > preserves a trailing CR in the repository directory name
FAIL gitWorktreeService.test.ts > getMainWorktreePath > preserves a trailing CR in the main worktree path
FAIL gitWorktreeService.test.ts > getRepoTopLevel > preserves a trailing CR in the repository top-level path
3. 平台假设是承重的 —— 而且万一它是错的,失败方向是安全的
0e365af8 建立在一个论断上:"git 的 stdout 在每个平台上都是 LF 结尾"。我在 macOS 上用真实 git 没法证伪它,于是我造了反事实 —— 在 PATH 上放一个 git 垫片,把该 commit 触及的两条命令重新用 CRLF 结尾,其余原样透传 —— 然后只问那个真正重要的问题:
| 腿(在一个 stdout 真的是 CRLF 结尾的 git 下) | getMainWorktreePath() |
端到端钉住 | 创建的子 agent |
|---|---|---|---|
prev eaa6d6b |
正确路径 | 钉住,PINNED_WORKTREE |
2 |
head 0e365af |
null |
拒绝 | 0 |
所以取舍是明确的:head 放弃了对"假想中会输出 CRLF 的 git"的容忍,换来对"路径里带 CR"的正确性;而在那样的平台上,workingDir 会拒绝所有路径,而不是钉错树。这是安全的方向,也正因如此它是一条说明而不是阻塞项 —— 见第 2 条说明。
4. 上次验证的结论在新顶端依然全部成立
针对 0e365af 重跑了整套:
| 面 | 结果 |
|---|---|
| 钉住被接受:相对路径、绝对路径、detached HEAD、仓库目录之外、非沙箱 | 5/5,每个 agent 都在正确的树里、读到正确的标记 |
闸门拒绝:主工作树、普通子目录、别的仓库的 worktree、拷贝的 .git 文件、陈旧注册表记录、路径不存在、路径穿越、workingDir+isolation(含藏在 getter 后面)、非字符串、纯空白、DEL/NEL 控制字符 |
12/12 全拒,每一次创建的子 agent 都是 0 |
| resume,脚本逐字节相同 | 两次运行合计只派发 1 次(第二次全部命中缓存) |
resume,只改了 workingDir |
派发 2 次,答案切换为 DETACHED_WORKTREE |
轮数上限:不设 / =3 / =999999 / =2.5 |
50 / 3 / 500(夹紧)/ 50(拒绝→默认) |
撞上限的 agent 放进 parallel() |
[null, null] —— PR 论证的那个失败场景可复现 |
AgentTool working_dir 拒绝文本,base vs head |
sha256 完全一致(84ef604ab34f6d87,410 字节) |
isolation:'worktree' |
无变化 |
stallMs 类型闸门 |
base 照跑,head 拒绝(见第 4 条说明) |
5. 测试,在 macOS 上
| 命令 | 结果 |
|---|---|
PR 自己的方案 —— npx vitest run src/agents/ src/tools/ |
127 文件通过、1 跳过 · 4278 通过、6 跳过(4284) |
npx vitest run src/services/gitWorktreeService |
4 个文件,71 条通过 |
PR 把 🍏 macOS 标为 Test (macos-latest, Node 22.x) 与 Test (windows-latest, Node 22.x) 都是 skipping,只有 ubuntu 那条腿真跑。
三条说明(均不阻塞)
1. 最后一个 commit 自己的测试落在 PR 声明的验证命令之外。
Reviewer Test Plan 写的是 cd packages/core && npx vitest run src/agents/ src/tools/。而 0e365af8 新增的 3 条 CR 测试全部在 src/services/ 下,所以严格照着方案走的审阅者一条都不会跑到 —— 包括那条打真实 git 的集成测试,而它恰恰是该 commit 里最强的一条。在那行命令后面补上 src/services/gitWorktreeService 即可。
2. 平台假设真正会咬人的地方是 Windows,而目前没有任何东西在验证它。
CR 改动在 macOS 与 Linux 上是正确的 —— git 在这两处确实输出 LF。而 Windows 在 PR 里标着 workingDir 拒绝所有路径,并给出一句把原因说错的"not a registered linked worktree"。建议在文档注释里补一句,或者在有人真正依赖 Windows 上的 workingDir 之前补一个冒烟测试。
3. 正文里的测试数字已经过期。
正文写的是 4250 passed | 6 skipped (4256)。在这个顶端上同一条命令给出的是 4278 passed | 6 skipped (4284) —— 正文写完之后又加了测试。属于观感问题,但这正是审阅者拿来对照的那个数字。
4. 我上次的两条说明仍然没有关闭。 两条都不是新问题,且都只涉及正文:
stallMs类型闸门是一处真实的行为变化(base会照跑一个传stallMs: 'soon'的脚本,head会拒绝),正文没有列出它,却写着*"破坏性变更 / 迁移说明:无"*。- 拒绝消息里被拒路径回显了两次 —— 外层帧补上脚本写的拼法,解析器消息里已经带了解析后的路径。观感问题。
对我上一条评论的更正
我把 QWEN_CODE_MAX_TOKENS_PER_WORKFLOW 列进了"工具描述里已经点名的 workflow 上限"。它并不在里面,三条腿都不在 —— 描述里只通过 budget.total 提到了 token 预算,而那个环境变量是通过 TUI 的运行提示面向用户暴露的("Workflows have no per-run token cap. Set QWEN_CODE_MAX_TOKENS_PER_WORKFLOW=<n>…"),并不面向模型。那条说明的实质 —— PR 最卖力论证的两个子 agent 旋钮恰恰是模型发现不了的 —— 是成立的,而 49bfc9f5 已经把它修好了。
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round: no action takenNo actionable feedback arrived this round, so no code changes were made and nothing was committed. What the round saw:
Why nothing was done: Critical-only mode is active (5 change-producing rounds are complete in this window). The only feedback item — the PR comment by @wenshao (issuecomment-5311310749) — was placed in the deferred non-Critical section by the workflow's deterministic brake. That section is an audit record, not work: deferred items are intentionally left open for human follow-up and are not addressed by this bot. Diff growth remains well within budget (source 18 / test 114 net lines vs 400/400 budgets). To bring the deferred item back into scope, a maintainer can tag it [Critical], submit a Request changes review, or comment 中文说明Autofix 本轮:未采取任何操作本轮没有收到任何可处理的反馈,因此未做任何代码改动,也没有提交任何内容。 本轮看到的情况:
未采取任何操作的原因: 当前处于仅处理 Critical 的模式(本窗口已完成 5 个产生改动的轮次)。唯一的反馈条目——@wenshao 的 PR 评论(issuecomment-5311310749)——被工作流的确定性刹车机制放入了"延后的非 Critical 反馈"区域。该区域是审计记录,不是工作项:被延后的条目会刻意保持开放状态,留待人工跟进,本机器人不会处理它们。 Diff 增长仍在预算之内(源码净增 18 行 / 测试净增 114 行,预算为 400/400)。 如需将延后的条目重新纳入处理范围,维护者可以为其加上 [Critical] 标签、提交 Request changes 评审,或评论 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 8709 passed · 0 failed · 8709 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:8709 通过 · 0 失败 · 8709 总计 Verification reportPR #8972 Deep Verification —
|
| # | Cell | Oracle | Base (d5b26b4) |
Head (0e365af) |
|---|---|---|---|---|
| A1 | script agent("x", {workingDir: …}) |
dispatch receives opt / error text | ❌ unknown option (as predicted) | ✅ opt delivered to dispatch |
| A2 | workingDir + isolation |
error | ❌ unknown option | ❌ incompatible options (named contradiction) |
| A3/A4 | " " / 7 as workingDir |
error | ❌ unknown option | ❌ non-empty string |
| A5 | stallMs: "0" (string) |
dispatch payload | ✅ silently passed through (pre-fix drop) | ❌ finite number refused loudly |
| J1–J5 | resume keys, opts differing only in workingDir |
key equality | same key across dirs (cross-dir replay bug) | different keys; same-dir HIT intact |
| B1–B6 | QWEN_CODE_WORKFLOW_AGENT_MAX_TURNS/MINUTES matrix |
resolver output | no knob; hard-coded = 50 literal in dist |
defaults 50/10, 120/45 honored, clamp 500/100, rejects 0/abc/2.5/0x10/1e3; both dispatch sites call the resolvers |
| R1–R20 | pin resolver vs real git (registered in/out of repo, detached, unregistered subdir, main tree, foreign repo's worktree, symlink→registered, symlink→outside, absent, non-repo parent, sibling pin from inside a linked worktree) | resolve / refusal | module does not exist (surface was AgentTool-private) | 12/12 behaviors correct; canonical realpath threaded through gate and result |
| C1–C4 | repo dir ending in \r, hostile CR-less sibling repo with its own worktree |
anchor + gate verdict | getRepoTopLevel() strips CR → anchor mutates to sibling → gate accepts the foreign repo's worktree |
CR preserved → gate refuses foreign, accepts own |
| C8–C13 | newline / attribute-shaped / trailing-newline paths; trailing space; main-tree answer from inside a linked worktree | getMainWorktreePath / getRepoTopLevel |
n/a (method new) | truncated anchors refused (null), round-trip catches attribute-shaped remainder, whitespace/CR preserved, main tree answered from linked worktree |
Harness 1: 36/36 scripted assertions. Harness 2 (mock-free, real git binary against repos the harness creates, incl. \r/\n/trailing-space directory names): 25/25. Expected base failures are encoded as passing assertions per the verdict contract.
The Agent-tool behavior-preservation claim ("error text byte-identical") was checked directly: base dist agent.js inline template vs head worktree-pin.js template are identical modulo the ${label} interpolation whose default is working_dir (5/5 scripted comparisons: both templates found, equality modulo label, and both companion messages present).
Test pinning — mutation matrix (scratch worktree at HEAD)
Unmutated control in the same tree: 267/267 green across the 5 files below (subset of the gate scope; not double-counted in the verdict totals). Witness: 03-mutation-matrix-and-gate.png.
| # | Mutation (file) | Suite | Result |
|---|---|---|---|
| M1 | registration gate → if (false) — positive control (worktree-pin.ts) |
worktree-pin.test.ts | KILLED 4F/6P — exactly the refusal-path tests; accept path stays green |
| M2 | drop opts.workingDir === undefined from fast-path condition (workflow-orchestrator.ts) |
workflow-orchestrator.test.ts | KILLED 8F/153P — exactly the 8 workingDir tests (forced-route, rebind, refusals, scrubbing) |
| M3 | remove 'workingDir' from canonical projection (workflow-journal.ts) |
workflow-journal.test.ts | KILLED 1F/14P — keeps workingDir, so a resume cannot hit across directories |
| M4 | LF-only strip → .trim() in getRepoTopLevel (gitWorktreeService.ts) |
unit + real-git integ | KILLED 4F/49P — 2 unit + 2 real-git CR/whitespace tests |
| M5 | remove if (mainPath.length === 0) return null (gitWorktreeService.ts) |
unit + real-git integ | SURVIVES 53/53 — see Findings |
| M6 | foreground runs require interactivity (workflow.ts) |
workflow.test.ts | KILLED — target test foreground execute() completes with no interactive session or completion channel red (broad mutation: 33 red, siblings share the isInteractive: false fixture trait) |
Every red quoted is the intended behavioral assertion (expected-versus-actual values), not an import/compile break; M1 provides the positive control proving the harness can turn the suites red.
Targeted gates
| Scope | Tree | Result |
|---|---|---|
src/agents src/tools + the 2 gitWorktreeService test files |
head | 4360 passed | 6 skipped (129 files + 1 skipped file), 0 failed |
Author's plan scope src/agents src/tools |
head | 4307 passed | 6 skipped (127 files + 1 skipped) — file counts match the author's claim exactly |
| Same scope | base | 4277 passed | 6 skipped (126 files + 1 skipped) |
| Δ base→head | — | +30 tests, +1 file, +0 failing — all PR-new tests, zero regressions |
The author's cited 4250 passed was measured against an older base; the current base tip already carries 4277, so the delta (not the absolute) is the verified quantity. Skipped file on both arms: workflow-p4a-meta-live.live.test.ts (6 live tests) — identical on both sides.
Corrections
- The conversation-start git snapshot labeled this change "fix(core): preserve trailing CR in git worktree path answers" — that is the PR's final commit subject (the branch's bot convention appends
(#8972)to every commit). The PR itself is theworkingDirpinning feature; the CR fix is one of its hardening commits. No code action — labeling clarification only. - The metadata snapshot's
baseRefOid(8e0033d…) is stale relative to the actual merge-ref base tip (d5b26b4…); verification therefore ran against the merge ref — i.e. what actually lands.
Findings (non-blocking)
- Survivor M5 — dead-leaning defensive clause, no fixture. Deleting
if (mainPath.length === 0) return null;fromgetMainWorktreePath()leaves all 53 gitWorktreeService tests green. Classification: the clause fires only on a porcelain first line that is exactlyworktree(empty path) — unreachable from a realgitbinary, and the unit mocks never feed it. It is a defense against malformed git output rather than a load-bearing gate; reported as completeness, not a merge condition. If pinned, the fixture ishoistedMockRaw.mockResolvedValueOnce('worktree \n')expectingnull. - Pre-existing ESM circular-init TDZ (not introduced by this PR). Importing compiled
gitWorktreeService.jsas an entry module crashes withCannot access 'AGENT_WORKTREE_SLUG_PATTERN' before initialization:worktreeCleanup.jsconsumes that binding at top level while the cycle is still evaluating. Verified pre-existing:worktreeCleanup.jsis byte-identical between base and head dist, and the same edge exists at base. Real CLI/vitest entry orders never hit it (the graph is entered elsewhere first); my harness had to preload the leaf. Recorded so nobody attributes it to this PR, and becauseworktree-pin.tsadds one more importer of this graph.
Not covered
- No live model-driven workflow run with a real pinned worktree (the PR discloses the same). The sub-model-boundary seams are proven instead: sandbox gate, orchestrator routing/rebind (via PR tests + M2), resolver (real git), journal keys, bounds. The "scenario reaches the code" check holds at each seam (dispatch counters in harness 1 are non-zero; the headless test asserts real result content).
- Per-commit attribution: 20 commits in the metadata snapshot, only the tip reachable at depth 2 (
git rev-list HEAD^1..HEAD^2= 1 across the shallow boundary — the known false-small). Verified the aggregateHEAD^1..HEADdiff. - Repo-wide gate — only the affected surface ran (agents/tools/services). Typecheck/build relied on the lane's pre-run
npm run build(head dist present and driven throughout; basepackages/corerebuilt cleanly in the A/B worktree). - TOCTOU after validation — the pin resolves once and threads the canonical path, but a symlink re-pointed between validation and child start is inherent to pinning; the PR documents this ("a cwd pin, not a filesystem sandbox").
- Windows/macOS: real-git cells ran on Linux only (the
\r/\npath shapes are unrepresentable on Win32; the PR's tests skip there too). - Approval behavior of tools called inside a headless workflow subagent (unchanged per PR disclosure).
- Environment note (not a PR property): this container's root
node_modulesdiverges from the lockfile-declared versions; correct versions live in nestedpackages/core/node_modules. Fresh checkouts ofpackages/core(base worktree, mutation worktree) failed to build/run until the nested tree was linked. This is a lane-setup artifact; both the head and base arms of every comparison used identical dependency realpaths.
Methodology
Environment: CI verify container (node:22-bookworm), merge-ref checkout at depth 2; head dist prebuilt; base packages/core rebuilt at d5b26b4 in a scratch worktree wired to the same node_modules (realpath-checked; driven modules import no workspace packages). Harnesses (harness/*.mjs, kept for rerun) drive compiled dist output through configuration seams — real git invocations for the resolver/anchor cells, recorded dispatch stubs downstream of the units under test — with every expectation scripted so an intended base-side red counts as a pass. Mutation runs executed in a second scratch worktree at HEAD with one mutation applied and reverted per run, against an unmutated 267-test green control. Raw logs: harness/01-run.log, 02-run.log/02-direct.log, mut-M1..M6.log, gate-head.log, gate-author-scope.log, gate-base-scope.log, base-build.log. Both scratch worktrees were removed after capture; the main tree is clean.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Thanks for the PR — and for reworking the body after the earlier template gate; it reads well now.
Moving on to code review. 🔍 中文说明感谢贡献——也感谢在模板门禁之后重写了 PR 正文,现在写得很清楚。
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo critical blockers. My independent proposal for this problem was close to what landed here — shared resolver, override-path routing, resume-key projection, env bounds on the existing override contract — and the places the PR goes further (anchor round-tripping, error-message scrubbing) are justified by the threat model, not decoration. Detail:
Dispatch flow for a pinned agent, since the gate ordering is the point: sequenceDiagram
participant P1 as workflow script sandbox
participant P2 as orchestrator override path
participant P3 as worktree-pin resolver
participant P4 as GitWorktreeService
participant P5 as dir-scoped Config override
participant P6 as workflow subagent
P1->>P2: agent prompt, opts with workingDir
Note over P1,P2: sandbox gate refuses workingDir combined with isolation
P2->>P2: refuse non-string or empty workingDir
P2->>P3: resolve with the caller's own parameter name
P3->>P4: git available, is a repository
P3->>P4: main-tree anchor, round-trip validated
P3->>P3: realpath once, thread the single resolution
P3->>P4: isRegisteredLinkedWorktree on the canonical path
P4-->>P3: registered, or fail closed
P3-->>P2: resolved pin or refusal
P2->>P5: rebind every cwd surface to the pinned path
P2->>P6: dispatch via SubagentManager with bounded runConfig
Files changed (15 of 15 shown)
TestingEvidence here is the PR's own CI at the reviewed SHA, read through the API — triage does not execute PR code. At
Sandboxed verification would settle the remaining behavioural gap: 中文说明代码审查无阻断性问题。我对这个问题的独立方案与 PR 的做法基本一致——共享校验器、override 路径路由、resume key 投影、沿用既有覆盖契约的环境变量上限——PR 走得更远的地方(锚点 round-trip 校验、错误消息清洗)由威胁模型决定,不是装饰。细节:
测试此处的证据是被审 SHA 上 PR 自己的 CI(经 API 读取)——triage 不执行 PR 代码。 沙箱验证可以补上剩下的行为缺口: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean gate, clean review, green CI at the reviewed SHA; the leftovers are description-accuracy nits and an absent live end-to-end run, none of it blocking. Stepping back: this one earns its merge. The gate's default is skepticism, and this PR walked through the skeptical path cleanly —
Non-blocking nits, for the record: the description's "changes no behaviour for the Agent tool" overclaims two small, beneficial, tested behavior changes (main-tree anchoring, canonical path return); the One process note: Stage 0 flags a core 中文说明置信度:4/5 —— 门禁干净、审查干净、被审 SHA 上 CI 全绿;剩下的只有描述准确性的小问题与缺失的端到端实跑,均不阻断。 退一步看:这个 PR 配得上合并。门禁的默认姿态是怀疑,而它走完了整条怀疑路径——
非阻断小问题,留档:描述中「对 Agent 工具行为没有任何改变」说重了两处小而有益、且有测试的行为变化(主树锚定、返回规范化路径); 一点流程说明:Stage 0 对生产行数 500+ 的核心 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.14. |
…le claim Round 8 raised three Criticals, all one root cause: the routing layer stated capability facts that the runtime it dispatches into had already outgrown, and three tests pinned each stale claim in place. R5-3 — the PR-worktree blocker said `agent()` "takes no working directory". It does: `WorkflowAgentOpts.workingDir` is in `KNOWN_AGENT_OPTS` (workflow-sandbox.ts:1368, validated at 1407-1420) and the orchestrator rebinds the subagent cwd through the shared `worktree-pin.ts` (workflow-orchestrator.ts:937-974). QwenLM#8972, which this PR's description names as the unblocker, is merged. The gap was never the runtime — the generated fan-out simply did not pass the pin. So it passes it now: `buildReviewWorkflowScript` bakes a `WORKING_DIR` literal beside the roster and each dispatch carries `workingDir` when the plan has a worktree, which is the same value `agent-prompt --roster` tells the orchestrator to put in `working_dir`. Omitted entirely when there is none — `agent({workingDir})` refuses an empty string rather than reading it as "no pin". Never alongside `isolation`; the two are mutually exclusive and passing both fails every agent. R8-1 — the territory blocker claimed the script "expresses the Step 3A roster only". This PR's own builder disproves that: `requiredAgents` emits `chunk-<id>` entries, `buildFanOutRoster` has a chunk branch, `buildLaunch` bakes the full per-territory contract into the prompt, and the serializer is topology-agnostic. What actually bounds 3B is delivery, and that bound is real: a workflow returns every agent's text inside ONE tool result under the scheduler's global budget, where the hand-launched path gets 32 000 chars per agent (`AgentTool.maxOutputChars`; `WorkflowTool` declares no override). A 3A roster is bounded at 14; a 3B roster grows one agent per chunk, so the truncation grows with the diff. The blocker now says that, and shrinks when the result gets a fan-out-sized budget rather than when the roster changes shape. R8-2 — SKILL.md asserted parity ("What differs is who launches them") and sent Exit 0 straight to Step 3D. Both are false at that same delivery boundary, and Step 3D cannot catch it: it verifies dispatch against on-disk transcripts, not against what reached the reader, so a truncated result certifies as complete with whole dimensions cut out of the middle. The parity sentence now names the difference, and the Exit 0 branch requires reading the spill file before Step 3D — the rule the legacy `--roster` redirect has always had, for the same reason. BEHAVIOR FLIP: a PR-worktree review with both gates open now routes to the workflow path instead of falling back to legacy. That is the flagship case — every same-repo PR review — and it was blocked only by the false premise above. It stays behind both opt-in gates (`QWEN_CODE_ENABLE_WORKFLOWS=1` and `QWEN_REVIEW_WORKFLOW=1`), so no default changes. The three tests that pinned the old premises are rewritten to pin the new facts, not deleted: the worktree-refusal tests now assert the roster IS emitted and that every dispatch carries the pin, and the territory tests assert the reason names the delivery bound and does NOT carry the disproved expressibility claim. Verified: each of the three fixes reverted in turn turns a test red (workingDir dropped -> "pins every agent to the review worktree" fails; old territory string restored -> 2 fail; PR-worktree blocker restored -> 3 fail). `vitest run src/commands/review` — 3854 passed, 1 failed: `run-ledger.test.ts` "refuses to append over a ledger it could not read", which fails identically on the unmodified branch (chmod does not stop uid 0). `tsc -p packages/cli` reports the same 86 pre-existing errors with and without this change. eslint and prettier clean on every changed file.










What this PR does
Three changes that together let a workflow subagent do work that is neither short nor in-place.
A workflow script can pin an agent to a directory.
agent({workingDir})runs that agent inside an existing git worktree the caller already owns — nothing is created, nothing is removed, and the child's cwd surfaces are rebound so its file, shell and search tools resolve inside it. This is the same contract the Agent tool already exposes asworking_dir, and it reuses the same validation: the path must resolve inside the repository and must be a worktree git actually knows about.Two details make the difference between working and silently not working, and both are handled. The fast dispatch path hands the run's Config to the agent untouched and has no way to honour a directory rebind, so
workingDirforces the override path — left on the fast path it would be dropped without a word and the agent would run in the parent working tree, which is the exact failure the option exists to prevent. And the resume key projection now includesworkingDir, because the same prompt run against two worktrees is two different questions; without it, a resume that changed only the directory would replay the previous tree's answers as this one's.The validation itself moves out of the Agent tool into a small shared module. It is the load-bearing half and not worth two copies: the path comes from a model either way — a tool call's argument, or a line in a workflow script — and pinning replaces the child's workspace boundary wholesale. The caller passes the name of its own parameter, so a script reads
workingDir "…"in the error and a tool call readsworking_dir "…".The per-subagent resource bounds become operator-tunable. A workflow subagent was capped at 50 turns and 10 minutes, hard-coded at both dispatch sites with no override, while the three other workflow bounds all have one. Both now honour an env override with a hard ceiling, on the same contract as the agent cap: a non-integer or sub-1 value is rejected with a warning and the default is used, and a value above the ceiling is clamped. The doc comment also states how the three time bounds differ from one another, since raising one without the others just moves which limit kills the run.
A regression test pins the headless foreground contract. A foreground workflow call must complete with no interactive session and no completion channel.
Why it's needed
isolation: 'worktree'is not a substitute for pinning. It creates a worktree from the current tree and refuses to run when the parent tree is dirty. That is the opposite of what a caller needs when the directory already exists and its uncommitted state is the whole point — a review worktree, a scratch checkout a previous step provisioned, anything whose lifetime the caller owns. Today a script simply cannot express that, so any workflow whose agents must work somewhere other than the session's own directory has no way to say so.The turn and time bounds matter for the same class of work. A build-and-test agent, or an analysis of a two-thousand-line file that has to page through large reads, exceeds 50 turns or 10 minutes routinely. Under the terminal-state contract, being cut off does not surface as a visible failure: the dispatch throws,
parallel()turns it into anullelement, and the caller sees an agent that silently went missing. An operator who hits this has no knob at all today — the two ceilings are the only workflow bounds without one, and at the override site they beat the agent type's own configuration.The headless test guards a path with no signal today.
qwen --prompt— CI, cron, any unattended run — has no TUI, no approval bridge and a closed stdin. The workflow tool's default permission isask, which the scheduler resolves against the run's approval mode; that is fine, but it means nothing inside the tool or the runner may reach for interactivity, or the foreground call would hang forever on a prompt nobody can answer. The background half is already refused explicitly with a clear error; the foreground half had nothing pinning it.Reviewer Test Plan
How to verify
Expected: all pass. On this branch,
4250 passed | 6 skipped (4256),127 files passed | 1 skipped.New tests, and what each is for:
src/agents/worktree-pin.test.tscovers the shared resolver against a mocked git worktree service — a registered worktree inside the repo resolves; a path outside the repository, an unregistered directory, missing git tooling, and a non-repository parent each refuse with the actual cause named rather than a generic message; a detached-HEAD worktree with no branch is accepted, since the branch is a label and never a gate; and the caller's parameter name appears in the error text.src/agents/runtime/workflow-orchestrator.test.tstreats that resolver as a seam and asserts what the orchestrator does with its verdict: aworkingDirdispatch leaves the fast path (the subagent is created through the manager, and the fast-path constructor is never called), the subagent's Config answers with the pinned directory rather than the parent's, a refusal aborts the dispatch without creating an agent at all, and the resolver is told the workflow opt's own name. The same file covers the two env-tunable bounds — defaults, valid overrides, clamping above the ceiling, and rejection of0/abc/2.5/0x10/1e3.src/agents/runtime/workflow-sandbox.test.tscovers the script-facing surface:workingDirreaches dispatch, a non-string is refused, andworkingDirtogether withisolationis refused as a contradiction rather than resolved by precedence — a script that got a silent winner would believe it was isolated when it was pinned, or the reverse.src/agents/runtime/workflow-journal.test.tsasserts two dispatches identical but forworkingDirderive different resume keys.src/tools/workflow/workflow.test.tsruns a foreground workflow to completion against a config with no interactive session and no completion channel.To exercise the pin by hand: create a worktree with
git worktree add ../wt-demo, then run a workflow withQWEN_CODE_ENABLE_WORKFLOWS=1whose script callsagent('run pwd and report it', { workingDir: '../wt-demo' }). The agent reports the worktree path; without this change the same script is rejected as an unknown option.Evidence (Before & After)
N/A — no user-visible or TUI change. The user-facing surface is a new
agent()option and two new env variables, both documented in the tool description and the code.Tested on
Environment (optional)
Unit tests only (vitest, Node 22, Linux).
Risk & Scope
workingDirrebinds the subagent's workspace boundary, so the validation is the security-relevant part of this PR — it is shared with the Agent tool rather than reimplemented precisely so the two cannot drift apart, but a reviewer should readagents/worktree-pin.tsas the load-bearing file. Raising the per-subagent bounds lets a single agent burn more tokens and wall clock than before; the defaults are unchanged, both overrides are clamped, and the run-level agent cap and wall clock still bound the whole run.workingDiris additive and rejected in combination withisolation; both env variables are opt-in; moving the resolver into a shared module changes no behaviour for the Agent tool, whose error text is byte-identical because the default parameter name isworking_dir.Linked Issues
Part of #8769.
中文说明
这个 PR 做了什么
三处改动,合起来让 workflow 子 agent 能做既不短、也不在原地的工作。
workflow 脚本可以把一个 agent 钉在某个目录上。
agent({workingDir})让该 agent 在调用方已经拥有的、既有的 git worktree 里运行——不创建、不删除,并且子 agent 的「我在哪」相关面被重新绑定,使其文件、shell 与搜索工具都落在该目录内。这与 Agent 工具已经暴露的working_dir是同一套契约,并复用同一套校验:路径必须落在仓库内部,且必须是 git 真正登记过的 worktree。有两个细节决定了它是「能用」还是「悄悄不生效」,两者都处理了。快速派发路径把运行时 Config 原样交给 agent,没有任何办法执行目录重绑定,所以
workingDir会强制走 override 路径——如果留在快速路径上,它会被一声不吭地丢弃,agent 转而在父工作树里运行,而这正是这个选项要防止的失败。另外,resume key 的投影现在包含workingDir,因为同一个 prompt 跑在两个 worktree 上是两个不同的问题;否则一次只改了目录的 resume 会把上一棵树的答案当成这一棵树的答案重放。校验逻辑本身从 Agent 工具中移入一个小的共享模块。它是承重的那一半,不值得存在两份:无论哪条路径,路径都来自模型——工具调用的参数,或 workflow 脚本里的一行——而钉住会整体替换子 agent 的工作区边界。调用方传入自己那一侧的参数名,因此脚本在错误里读到的是
workingDir "…",工具调用读到的是working_dir "…"。单个子 agent 的资源上限变为运维可调。 workflow 子 agent 此前被限制在 50 轮与 10 分钟,在两个派发点硬编码且没有任何覆盖手段,而其余三个 workflow 上限都有。现在两者都支持带硬上限的环境变量覆盖,契约与 agent 数量上限一致:非整数或小于 1 的值会被拒绝并给出警告、回退到默认值,高于硬上限的值会被夹紧。文档注释同时说明了三个时间上限彼此的分工,因为只抬高其中一个而不管其余,只是换成另一个上限来杀掉这次运行。
一个回归测试钉住 headless 前台契约。 前台的 workflow 调用必须在没有交互式会话、也没有完成通道的情况下跑完。
为什么需要
isolation: 'worktree'不能替代「钉住」。它是从当前树新建一个 worktree,并且在父树有未提交改动时拒绝运行。当目录已经存在、而且其未提交状态正是重点时——一个 review worktree、上一步准备好的临时检出、任何生命周期由调用方掌握的目录——这恰恰是相反的语义。今天脚本根本无法表达这一点,因此任何需要让 agent 在会话自身目录之外工作的 workflow,都没有办法说出这个需求。轮数与时间上限影响的是同一类工作。一个构建与测试的 agent,或者对一个两千行文件的分析(需要分页读完大段内容),例行地会超过 50 轮或 10 分钟。在终态契约下,被切断不会表现为可见的失败:派发抛错,
parallel()把它变成一个null元素,调用方看到的是一个悄悄消失的 agent。今天撞上这一点的运维方没有任何旋钮——这两个上限是唯一没有覆盖手段的 workflow 上限,而且在 override 站点它们会盖过 agent 类型自身的配置。headless 测试守护的是一条今天没有任何信号的路径。
qwen --prompt——CI、cron、任何无人值守的运行——没有 TUI、没有审批桥接、stdin 是关闭的。workflow 工具的默认权限是ask,由调度器结合运行的审批模式解析;这没有问题,但它意味着工具与 runner 内部不得有任何地方去索取交互,否则前台调用会永远挂在一个没人能回答的确认框上。后台那一半已经用明确的错误拒绝掉了;前台这一半此前没有任何东西钉住。审阅者验证方案
如何验证
预期全部通过。本分支上为
4250 passed | 6 skipped (4256),127 files passed | 1 skipped。新增测试,以及各自的用途:
src/agents/worktree-pin.test.ts针对被 mock 的 git worktree 服务覆盖共享校验器——仓库内已登记的 worktree 可以解析通过;仓库之外的路径、未登记的目录、缺失的 git 工具、非仓库的父目录,各自以真实原因而非笼统消息拒绝;处于 detached HEAD、没有分支的 worktree 会被接受,因为分支只是标签、从来不是关卡;并且调用方的参数名会出现在错误文本里。src/agents/runtime/workflow-orchestrator.test.ts把该校验器当作接缝,断言编排器拿到裁决后的行为:带workingDir的派发会离开快速路径(子 agent 经由 manager 创建,快速路径的构造函数完全没有被调用)、子 agent 的 Config 回答的是被钉住的目录而不是父目录、被拒绝时派发中止且根本不创建 agent、以及校验器被告知的是 workflow 侧选项自己的名字。同一文件覆盖两个可调上限——默认值、有效覆盖、超上限夹紧,以及对0/abc/2.5/0x10/1e3的拒绝。src/agents/runtime/workflow-sandbox.test.ts覆盖面向脚本的接口:workingDir能到达派发层、非字符串被拒绝、workingDir与isolation同时出现时作为矛盾被拒绝而不是按优先级择一——如果脚本拿到一个无声的胜者,它会以为自己被隔离了而实际是被钉住,或者相反。src/agents/runtime/workflow-journal.test.ts断言两次仅workingDir不同的派发会派生出不同的 resume key。src/tools/workflow/workflow.test.ts针对一个没有交互式会话、也没有完成通道的 config,把前台 workflow 跑到结束。若要手动验证钉住效果:用
git worktree add ../wt-demo创建一个 worktree,然后用QWEN_CODE_ENABLE_WORKFLOWS=1跑一个脚本调用agent('run pwd and report it', { workingDir: '../wt-demo' })的 workflow。agent 会报告该 worktree 路径;没有本改动时,同一脚本会因未知选项被拒绝。证据(前后对比)
N/A —— 没有用户可见或 TUI 变化。面向用户的接口是一个新的
agent()选项和两个新的环境变量,均已写入工具描述与代码注释。测试环境
环境(可选)
仅单元测试(vitest,Node 22,Linux)。
风险与范围
workingDir会重新绑定子 agent 的工作区边界,所以校验是本 PR 中与安全相关的部分——它与 Agent 工具共享而非重新实现,正是为了两者不会漂移,但审阅者应把agents/worktree-pin.ts当作承重文件来读。抬高单个子 agent 的上限意味着一个 agent 可以比以前烧掉更多 token 与墙钟时间;默认值未变,两个覆盖都会被夹紧,运行级的 agent 数量上限与墙钟仍然约束整次运行。workingDir是增量的,且与isolation同时出现时会被拒绝;两个环境变量都是选择性启用;把校验器移入共享模块对 Agent 工具的行为没有任何改变,其错误文本逐字节一致,因为默认参数名就是working_dir。关联 Issue
Part of #8769.