test(integration): deflake tool-control permission cases - #7725
Conversation
Migrate 5 flaky tool-control E2E cases from real model to fake-openai-server, eliminating timeout flakes caused by non-deterministic model tool-call behavior. Add a flake detection step to the autofix workflow that closes self-resolved CI failure issues before entering the expensive develop phase. Add a verification step to the triage workflow that surfaces the silent gap when the agent produces a summary but fails to post the stage comment.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR — re-run after the scope trim. Template looks good ✓ Problem: observed flakiness. The Docker-sandbox E2E job was repeatedly timing out when the real model didn't choose the expected tool calls. Linked to #7616. This is a real, reproduced CI reliability issue, not theoretical hardening. Direction: well-aligned. These five cases exist to verify permission enforcement (excludeTools, allowedTools, canUseTool routing), not model planning. Replacing real-model variance with deterministic fake-server tool calls is the right call — it keeps the load-bearing assertions while removing the flake source. Size: not applicable — single integration test file, no core paths touched. Approach: the scope is now tight. The earlier version bundled three unrelated concerns (test deflake, autofix flake gate, triage verification); this revision keeps only the test migration, which addresses the scope feedback. The Moving on to code review. 🔍 中文说明感谢贡献——范围缩减后的 re-run。 模板完整 ✓ 问题：已观测到的 CI 不稳定。Docker sandbox E2E 多次因真实模型未选择预期工具调用而超时，关联 #7616。是真实的、已复现的 CI 可靠性问题。 方向：对齐。这 5 个用例验证的是权限执行（excludeTools、allowedTools、canUseTool 路由），而非模型规划。用确定性的 fake server 工具调用替换真实模型波动是正确做法。 规模：不适用——仅一个集成测试文件，未触及核心路径。 方案：范围已收紧。早期版本捆绑了三个无关改动，本次修订只保留测试迁移。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: I'd do exactly what this PR does — replace real-model calls with the existing Comparison with the diff: the PR matches this approach cleanly. No correctness bugs, no security concerns, no regressions. What changed since the last review round:
The five migrated cases all follow the same clean pattern: fake server emits fixed tool calls → SDK processes them through the real permission layer → assertions check specific tool call IDs for No blockers. No AGENTS.md violations. TestingFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Ubuntu unit test is still running. macOS/Windows tests and integration tests were skipped (likely gated on the ubuntu result). No failures so far. The Not verified: real-scenario tmux testing — N/A, this is a test-infrastructure change with no user-visible behavior. 中文说明代码审查独立方案： 与本 PR 完全一致——用现有的 与 diff 的对比： PR 干净地实现了这一方案。无正确性 bug、无安全问题、无回归。 上一轮 review 以来的变化：prettier 格式已修正（wenshao #1 阻塞项）；多轮 edit 用例改为按请求内容分发而非 5 个迁移用例遵循相同的干净模式：fake server 发出固定工具调用 → SDK 通过真实权限层处理 → 按工具调用 ID 断言 无阻塞项。无 AGENTS.md 违规。 测试Ubuntu 单元测试仍在运行中，目前无失败。macOS/Windows 测试和集成测试被跳过（可能依赖 ubuntu 结果）。真实场景 tmux 测试：不适用——这是测试基础设施变更，无用户可见行为。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, well-motivated CI reliability fix; all prior blocking feedback addressed; only non-blocking nit is the Read-pattern coverage drift. This revision is a different PR from what was reviewed before. The scope trim to a single test file is the right call — the test deflake stands on its own and doesn't need to carry the workflow changes. The five migrated cases are genuinely better than what they replace: the old assertions ( The one reservation is the wenshao has already approved. CI is still running (ubuntu unit test in progress) — approval deferred until CI lands green on 中文说明置信度：4/5 — 干净、动机充分的 CI 可靠性修复；所有先前阻塞反馈已解决；唯一的非阻塞项是 Read 模式覆盖面漂移。 本次修订与之前 review 的版本是不同的 PR。范围缩减到单个测试文件是正确做法——测试 deflake 本身是独立的，不需要捆绑 workflow 改动。5 个迁移用例确实比原来更好：旧断言在模型不配合时会空过，新断言通过确定性的工具调用 ID 精确验证了被阻止和允许的路径。wenshao 的变异测试确认了断言是有承载力的。 唯一的保留意见是 allowedTools 路径模式用例中 wenshao 已 approve。CI 仍在运行（ubuntu 单元测试进行中）——批准延迟到 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Review + Linux verification report (real build + bundle, migrated tests executed)Verdict: the direction (script the model, test the permission layer) is right, but as committed this PR replaces flaky tests with deterministically failing ones — 3 of the 7 fake-server cases fail on every run in a clean Linux environment, and I can prove the root cause for the edit case. Needs a fix before merge. Execution evidence (commit b380fde,
|
| Migrated test | Result |
|---|---|
| block read operations with excludeTools | ✅ pass |
| plan blocks all write tools | ✅ pass |
| block shell commands with prefix pattern | ✅ pass |
| allow shell commands with pattern matching | ✅ pass |
| block edit operations with excludeTools | ❌ fail (3/3 runs) |
| auto-approve path patterns with allowedTools | ❌ fail |
| auto-approve shell commands with pattern matching | ❌ fail |
Root cause (proven for the edit case)
The scripted fake model emits edit tool calls without a prior read_file, and the edit tool enforces read-before-edit. Probe output from the failing assertion's actual tool result:
"toolUseId": "edit-readme",
"content": "File …/readme.md has not been read in this session. Use the read_file tool first to load the current content … before editing it.",
"isError": true
So readmeResult.isError is true for a reason unrelated to excludeTools — the test now fails deterministically where it used to flake. A real model satisfies this precondition naturally (it reads first); a scripted transcript must do it explicitly: have requestIndex 0 return read_file calls for the files, requestIndex 1 return the edit calls. The two auto-approve failures show the same shape (expected false to be true — the scripted call never executed); the same staging fix likely covers them, worth verifying each.
Note the PR's own Tested-on table says neither macOS nor Linux was exercised — this is exactly the gap. A deterministic red is worse for the docker E2E job than a 50% flake: it fails every run and will re-trigger the very autofix loop this PR is trying to save from wasted cycles.
Workflow changes — two smaller findings
- The flake-close comment can overclaim.
is_flake=truehas two triggers — (a) a later green run on main, (b) the triggering commit touched only test files — but the posted close comment unconditionally says "a subsequent run on main has already passed", which is untrue when only (b) fired. Same class of issue as fix(review): don't overclaim a submit-gate bypass on a same-account write #7718: the detector may use heuristics, the human-facing copy must match the evidence. Split the comment per branch (and note that (b) alone is shaky — a bad test-only commit legitimately breaks CI). - The Reviewer Test Plan contradicts the step's own gate. Step 2 says to verify via
workflow_dispatchwithdry_run: true, but the Flake detection step's condition isdry_run != 'true'— in dry-run the step is skipped entirely and logs nothing. Either let the step run in dry-run with writes gated, or fix the test plan. - The triage "verify stage comment" step is warning-only with no side effects — that part looks good, and
is_flakebeing empty when the step skips correctly passes the!= 'true'guards downstream.
Happy to re-verify once the scripted transcripts stage a read_file turn first.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查——无阻断问题。 建议见行内评论。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— qwen3.7-max via Qwen Code /review
Re-verification report, round 2 (Linux, commit 37ad026, full rebuild)Status: 2 of the 3 deterministic failures are fixed and verified green 3/3; all three workflow findings are properly addressed. One test remains deterministically red — Previous findings — status at this head
The remaining failure: the staged
|
Round-3 status check (head = +
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
Round-4 status check (head = +
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] tool-control.test.ts excludeTools test: multi-turn fake-server flow never reaches requestIndex === 1 — the SDK does not make a second model call after processing the first turn's read_file results, so the scripted edit turn never fires. expect(srcEditResult).toBeDefined() fails deterministically. This is the round-2/3/4 blocker from @gwinthis (comment 5080789331), confirmed still standing: file sha1 at HEAD matches the cited sha1, test unchanged. Fix requires content-keyed fake-server routing or equivalent to drive the second turn.
— qwen3.7-max via Qwen Code /review
|
Review closeout: no code changes were needed. I resolved the two script-test CI wiring suggestions as already satisfied: |
|
Re-checked all review suggestions — they are all already addressed or false positives:
No code changes needed. |
ReviewThree independent changes: (1) five I built the branch and ran the migrated tests and the shell logic rather than reading them. The migration itself holds up well — one blocker and a handful of correctness/calibration items below. What I ran — worktree at
|
| Check | Result |
|---|---|
6 fake-server cases, isolated HOME, 3 consecutive runs |
6/6 pass every run, ~8.6s total per run |
Mutation: drop excludeTools: ['Edit(/src/**)'] |
test goes red ✅ |
Mutation: allowedTools: ['Edit(*.json)'] → ['Edit(*.md)'] |
test goes red ✅ |
vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow qwen-triage-workflow |
119/119 pass |
actionlint with the repo's exact flags, both workflows |
clean |
yamllint config review (no line-length rule; quoted-strings: single/required) |
new YAML is gate-compatible |
prettier --check A/B, base vs head, same paths |
regression — #1 |
Mocked-gh harness over the Flake-detection shell, 9 scenarios |
fail-open paths all correct; see #3, #6 |
The new assertions are genuinely load-bearing — both mutations produce real failures, not just weaker passes. Nice upgrade over the old if (x.length > 0) { expect(...) } shape, which passed vacuously whenever the model didn't cooperate.
1. Blocking — tool-control.test.ts no longer passes prettier --check
A/B in the same path, base vs head:
$ git checkout $(git merge-base origin/main HEAD) -- integration-tests/sdk-typescript/tool-control.test.ts
$ npx prettier --check integration-tests/sdk-typescript/tool-control.test.ts
All matched files use Prettier code style!
$ git checkout HEAD -- integration-tests/sdk-typescript/tool-control.test.ts
$ npx prettier --check integration-tests/sdk-typescript/tool-control.test.ts
[warn] integration-tests/sdk-typescript/tool-control.test.ts
prettier --write rewrites ~357 lines. Both causes are side effects of the refactor:
- hoisting
FAKE_SERVER_OPTIONSshortened the second argument enough that prettier collapses everystartFakeOpenAIServer(...)call tail to}, FAKE_SERVER_OPTIONS); - several new
.find()callbacks now fit on one line, e.g.const rmResult = shellResults.find((r) => r.toolUseId === 'shell-rm');
npm run format fixes it. (.github/workflows/qwen-triage.yml also fails prettier, but it already failed at the merge-base — pre-existing, not yours.)
2. requestIndex keying is brittle for the one multi-turn case
should block edit operations… is the only migrated case that needs two conversation turns (requestIndex === 0 → reads, === 1 → edits). That counter counts HTTP requests, not conversation turns, so any auxiliary model call the CLI makes in between silently shifts the edits out of the way.
This isn't hypothetical — I hit it on the first run. With a populated ~/.qwen/memories, the CLI issues a memory-recall call between the two turns:
DEBUG_REQS 3 tools-per-request: [8, 1, 8]
DEBUG_AUX {"tool":"respond_in_schema", ...} <- request #1 is memory recall, not the model turn
DEBUG_CALLS [{"n":"read_file","id":"read-src"},{"n":"read_file","id":"read-readme"}]
^ no edit calls at all — turn 2 fell through to `return { content: 'Done.' }`
AssertionError: expected undefined to be defined
❯ tool-control.test.ts:534 expect(srcEditResult).toBeDefined();
With HOME isolated it passes 3/3, so CI runners are fine today — but the failure mode is precisely the one this PR set out to remove: a green-path assumption that turns into expected undefined to be defined when something unrelated changes. Memory recall, a next-speaker check, compression, title generation — any of them re-breaks this case, and the message won't point at the cause.
Suggest dispatching on request shape instead of the counter, e.g.:
const hasReadResult = (req) =>
(req.body.messages ?? []).some((m) => m.role === 'tool' && m.tool_call_id === 'read-src');
// …then: if (!hasReadResult(req)) → reads; else if (no edit result yet) → edits; else → 'Done.'The other four migrated cases are single-turn and unaffected.
3. Flake detection: a failed gh issue close aborts the step mid-way
gh issue comment and gh issue close have no || true (unlike the --add-label line right below), and is_flake=true is written to $GITHUB_OUTPUT after them. Under set -euo pipefail a transient API failure on the close gives: comment posted, issue still open, is_flake never emitted, job red. Reproduced with a mocked gh where only issue close fails:
✅ Subsequent E2E Tests run(s) on main succeeded — this is a flake.
gh: issue close failed (simulated)
STEP EXIT=1
GITHUB_OUTPUT= <- empty
Write the output first, then treat the three mutations as best-effort:
if [[ "${IS_FLAKE}" == "true" ]]; then
echo "is_flake=true" >> "${GITHUB_OUTPUT}"
if [[ "${DRY_RUN}" != 'true' ]]; then
gh issue comment ... || true
gh issue close ... || true
gh issue edit ... 2>/dev/null || true
fi
else
echo "is_flake=false" >> "${GITHUB_OUTPUT}"
fiEverything else in that step fails open correctly — I exercised all nine paths (non-CI title, issue-view failure, missing run ID, run-fetch failure, non-main / non-push / non-failure run, no later green, runs-list failure, dry-run) and every one exits 0 with is_flake=false and zero mutations. Dry-run correctly suppresses all three writes.
4. Triage verification will warn falsely on issues: edited / reopened
The triage job runs unconditionally for github.event_name == 'issues', and the workflow listens on ['opened', 'edited', 'reopened']. On those unattended re-events SKILL.md → Duplicate Guard tells the agent to exit without posting when a prior <!-- qwen-triage stage=N --> marker exists. Then:
Check triage responsepasses (the agent still returns a summary),- the prior stage=1 comment's
updated_atpredatesstarted_at, - →
::warning title=Triage silent gapfires on a perfectly correct run.
Issue bodies get edited often, so this is the common case, not the rare one — and it gets more likely once #7723 lands and the status comment stops satisfying the guard. Suggested shape: warn only when no stage comment exists at all, and keep the since freshness check for explicit reruns (issue_comment / workflow_dispatch) where fresh output really is required.
5. Marker-gated dedup will double-post on PRs blocked before this lands
pr-workflow.md now filters the existing-terminal-review check on startswith("<!-- qwen-triage terminal-review -->"). Terminal reviews already submitted have no marker, so they become invisible to that check and the next re-run of an already-blocked PR submits a second CHANGES_REQUESTED — the exact duplicate the guard exists to prevent. Either keep a legacy fallback for one cycle:
select(.user.login=="qwen-code-ci-bot" and .state=="CHANGES_REQUESTED" and
((.body | startswith("<!-- qwen-triage terminal-review -->")) or (.body | contains("qwen-triage stage=1"))))
or note in the PR body that the first re-run of each pre-existing blocked PR is expected to double-post.
6. Design: one green run is thin evidence for not planned + autofix/skip
Any single later same-workflow green on main closes the issue with "No code fix needed" and stamps autofix/skip, which permanently excludes it from future scans. The PR's own premise is that this suite fails ~50% of the time — so an intermittently-reproducing real regression also almost always has a later green, and gets auto-closed with the same wording. Worth considering:
- require K ≥ 2 consecutive later greens rather than
> 0; - label
ci/flakeinstead of (or alongside)autofix/skip, so flake volume stays countable — that measurement is what Do we really need this many E2E tests? #7616 actually needs; - soften the comment: the evidence supports "main is green again", not "no code fix needed".
Two smaller things in the same step:
ISSUE_JSON="$(gh issue view ... || true)"→ empty →TITLE=""→ logs"Not a CI failure issue; skipping flake detection."(confirmed as scenario 2 of my harness). If the PAT ever loses issue-read scope, detection quietly stops and the log says nothing. A separate-z "${ISSUE_JSON}"branch costs two lines.- the
LATER_GREENquery is correct without--paginateonly because the API returns runs newest-first, so any later green is guaranteed to be on page 1. Worth a comment — that reasoning isn't obvious.
7. Coverage drift the PR body doesn't mention
should auto-approve specific path patterns with allowedTools previously exercised Read-rule path matching (Read(.json), Read(.txt)) against read_file. It now exercises Edit(*.json) → write_file (the Edit meta-category). The new assertions are much stronger — exact canUseToolCalls, [Operation Cancelled] content, on-disk effect — but Read(<pattern>) auto-approval loses its E2E coverage entirely. Either keep a read-pattern case or call the swap out under Risk & Scope.
8. Scope
Three unrelated concerns in one PR. The repo's own Stage 1d asks for exactly this to be split, and here it has a practical cost: the flake gate is the change most likely to need reverting, and today it can't be reverted without also reverting the deflake work.
Nits
scripts/tests/qwen-autofix-workflow.test.js:79—readDecisionStep's regex still terminates at- name: 'Claim issue', so it now swallows the entire Flake-detection step. Harmless today (every assertion on it istoContain), but it should terminate at- name: 'Flake detection'before someone adds anot.toContain.- The pre-existing fake-server case at ~L371 still carries its
envblock inline;fakeModelOptions()was introduced but not applied there. - PR body says flake detection also fires when "触发 commit 只改了测试文件" — that heuristic was dropped (the test even pins its absence via
not.toContain('COMMIT_FILES')). Worth updating the description. - The Tested on table says "not tested" on every OS. The 6 fake-server cases run in ~9s with no API key — cheap enough to fill in.
- fix(ci): rename triage status marker to avoid duplicate-guard collision #7723 touches both
.github/workflows/qwen-triage.ymlandscripts/tests/qwen-triage-workflow.test.js; expect a conflict in the test file (both addit()blocks) whichever lands second.
中文说明
三块相互独立的改动：E2E 用例迁移到 fake server、autofix 的 flake 预检、triage 的产出校验 + skill marker 契约。我把分支实际构建并跑了迁移后的用例和 shell 逻辑，迁移本身质量不错，以下是一个阻塞项和若干正确性/标定问题。
已验证： 隔离 HOME 下 6 个 fake-server 用例连续跑 3 轮全绿（每轮约 8.6s）；两次变异（删掉 excludeTools: ['Edit(/src/**)']、把 Edit(*.json) 改成 Edit(*.md)）都能让用例变红，说明新断言是有承载力的，比原来 if (x.length > 0) { expect(...) } 那种模型不配合就空过的写法强很多。workflow 契约测试 119/119 通过，actionlint 干净，新增 YAML 符合 yamllint 门禁。
1. 阻塞： tool-control.test.ts 在 base 上 prettier 干净，在 PR head 上不再通过（prettier --write 改动约 357 行）。原因是抽出 FAKE_SERVER_OPTIONS 后第二个参数变短，prettier 会把调用尾部折叠成 }, FAKE_SERVER_OPTIONS);，以及若干 .find() 回调可单行放下。跑 npm run format 即可。（qwen-triage.yml 的 prettier 失败在 merge-base 上就存在，与本 PR 无关。）
2. requestIndex 分发对唯一的多轮用例很脆。 该计数器数的是 HTTP 请求而不是对话轮次。我第一次跑就踩到了：本机 ~/.qwen/memories 非空时，CLI 会在两轮之间插入一次 memory recall（respond_in_schema），于是 requestIndex === 1 被它吃掉，edit 那一轮落到 'Done.' 分支，用例报 expected undefined to be defined。隔离 HOME 后 3/3 通过，所以 CI runner 目前没问题——但这正是本 PR 想消灭的那类失败：任何新增的中间模型调用（memory recall、next-speaker 检查、压缩、标题生成）都会让它再次假红，且报错信息完全指不到原因。建议按请求内容分发（例如判断 messages 里是否已有 read-src 的 tool 结果），而不是按计数器。
3. Flake 步骤： gh issue comment / gh issue close 没有 || true，且 is_flake=true 写在这些写操作之后。close 一旦瞬时失败，就会出现"评论已发、issue 未关、is_flake 未输出、job 变红"的半成品状态（已用 mock gh 复现）。建议先写 GITHUB_OUTPUT 再把三个写操作降级为 best-effort。该步骤其余 9 条失败路径我都跑过，全部正确地 fail-open，dry-run 也确实抑制了全部写操作。
4. Triage 校验会误报： issues 事件监听了 edited / reopened，而 SKILL.md 的 Duplicate Guard 要求 agent 在这类非交互事件 + 已有 stage marker 时直接退出、不发任何评论，此时新校验步骤会误报 Triage silent gap。建议只有在完全不存在 stage 评论时才 warn，since 新鲜度判断只用于显式重跑。
5. Marker 去重： 存量 terminal review 没有 <!-- qwen-triage terminal-review --> 前缀，新过滤条件看不到它们，已被 block 的老 PR 再跑一次会重复提交第二条 CHANGES_REQUESTED。建议保留一轮 legacy 兜底。
6. 标定： 只要有一次后续 main run 变绿就以"No code fix needed"关单并打 autofix/skip（永久排除）。但本 PR 的前提就是这套 E2E 有 ~50% 失败率——那么间歇性复现的真实回归同样几乎必然有后续绿色，从而被同样的话术自动关掉。建议：要求连续 K≥2 次绿；用 ci/flake 标签代替/补充 autofix/skip，让 flake 数量可统计（#7616 真正需要的正是这个度量）；措辞改成"main 已恢复"而非"无需修复"。
7. 覆盖面漂移： should auto-approve specific path patterns with allowedTools 原来验证 Read 规则路径匹配，现在换成 Edit(*.json) → write_file。新断言强得多，但 Read(<pattern>) 的 E2E 覆盖被整体去掉了，建议补一个 read 用例或在 Risk & Scope 里写明。
其余小项：readDecisionStep 的正则现在会吞掉整个 Flake 步骤；L371 的老用例没改用 fakeModelOptions()；PR 描述里"只改了测试文件"的启发式实际已删除；Tested on 全填"未测试"，但这 6 个用例不需要 API key、9 秒就能跑完，值得补上；#7723 与本 PR 都改了 qwen-triage-workflow.test.js，后落地的一方会有冲突。
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Unresolved, please confirm: [Critical] @gwinthis round-4 (comment 5080789331): excludeTools test 'requestIndex === 1 never reached' — reported failing 3/3 without HOME isolation, but @wenshao verified 6/6 pass with isolated HOME (CI). Cannot definitively rule without reproducing in both environments. [Critical] @qwen-code-ci-bot review 4780347041: 'SDK does not make a second model call after processing the first turn's read_file results' — verifier argues the 'block read' test proves the SDK does make a second call, but the prior reviewer observed failures. Cannot confirm without direct test execution.
Not reviewed: Agent 0: Issue fidelity & root-cause ownership, Agent 4: Performance & efficiency, Agent 7: Build & test verification — its prompt was built, but no agent on record was launched with it.
— qwen3.7-max via Qwen Code /review
yiliang114
changed the title
|
Re-review closeout:
Verification: the five migrated cases pass 5/5; repository build and typecheck also pass. The blocker raised by @doudouOUC is addressed by |
Round-5 re-verification (head
|
| # | Finding (round 1) | Severity | Status |
|---|---|---|---|
| 1 | tool-control.test.ts fails prettier --check |
Blocker | Fixed — A/B verified |
| 2 | requestIndex === 1 eaten by a memory-recall side-query → excludeTools case fails |
Critical | Fixed — transcript-based dispatch, proven load-bearing |
| 3 | Workflow findings (autofix flake pre-check, triage verify step, closeout labels) | mixed | Moot — dropped from scope in bd02c267 |
Prettier A/B in the real path (config resolution is path-relative), production code byte-identical at both heads:
17c4e20 → [warn] integration-tests/sdk-typescript/tool-control.test.ts (fails)
bd02c267 → All matched files use Prettier code style! (passes)
eslint also clean on the file.
Adjudicating the contested [Critical]
Same dist/cli.js (no packages/ delta between the two heads), swapping only the test file, ×2 HOME environments:
HOME with ~/.qwen/memories |
HOME without | |
|---|---|---|
pre-fix test file (17c4e20) |
✗ expected undefined to be defined |
✓ 5/5 |
this head (bd02c267) |
✓ 5/5 (3 consecutive runs) | ✓ 5/5 |
That reproduces @gwinthis's round-2/3/4 failure exactly (same assertion message) and @yiliang114's pass, from one variable. Root cause: client.ts:2135 fires an auto-memory recall side-query on every user turn — enableManagedAutoMemory defaults to true (config.ts:2284) — and its respond_in_schema HTTP request lands at requestIndex === 1, eating the slot the old handler used for turn 2.
Instrumented request log (requestIndex, body.tools.length) confirms it; the 1-tool request is the recall call:
with memories: [8, 1, 8, 8] ← index 1 is the side-query
without memories: [8, 8, 8]
The new body['messages']-shape dispatch is immune to it. Note this only reproduces with valid topic docs — my first attempt at the control was vacuous because scan.ts:68 drops any file without a type: frontmatter key, so no recall call fired at all.
New findings
1. [Suggestion] Two of the five migrated cases don't exercise allowedTools at all
Mutation matrix (empty out the parameter each case names, run that case):
| mutant | case | verdict |
|---|---|---|
excludeTools: ['Edit(/src/**)'] → [] |
block edit on path patterns | RED ✓ |
excludeTools: ['Bash(rm *)'] → [] |
block shell prefix | RED ✓ |
allowedTools: ['ShellTool(echo )', 'ShellTool(ls )'] → [] |
allow shell w/ pattern matching | GREEN ✗ |
allowedTools: ['Edit(*.json)'] → [] |
auto-approve path patterns | RED ✓ |
allowedTools: ['ShellTool(echo *)'] → [] |
auto-approve shell w/ pattern matching | GREEN ✗ |
Cause: permission-manager.ts:500 auto-allows read-only shell commands before allowedTools is consulted, and both echo and ls are in READ_ONLY_ROOT_COMMANDS (shellReadOnlyChecker.ts:25). State dump with allowedTools: []:
echo= {"content":"test","isError":false} ← ran anyway
touch= {"content":"[Operation Cancelled] Reason: Non-allowed tools ...","isError":true}
canUseToolCalls= ["run_shell_command"] ← only the touch
So these two assert the read-only auto-allow path, not pattern matching. Not introduced here — the merge-base (596abd9) assertions were weaker still (expect(input.command).toMatch(/^(echo |ls )/) asserts what the model chose), and the kill-set is identical at 17c4e20, so the assertion refactor regressed nothing. But the PR rewrites exactly these assertions, and the description claims they "still exercise the actual permission and routing behavior" — for these two, they don't.
Repair, verified in both directions (green with the pattern, RED with allowedTools: []) — make the subject a non-read-only command:
- { command: 'echo test' },
+ { command: 'touch allowed.txt' },
- { command: 'touch blocked.txt' },
+ { command: 'mkdir blockeddir' },
- allowedTools: ['ShellTool(echo *)'],
+ allowedTools: ['ShellTool(touch *)'],
- expect(helper.fileExists('blocked.txt')).toBe(false);
+ expect(helper.fileExists('allowed.txt')).toBe(true);
+ expect(helper.fileExists('blockeddir')).toBe(false);2. [Nit] The other four handlers keep the requestIndex coupling the fix removed
Four migrated cases still dispatch on requestIndex === 0. Empirically safe — the side-query lands at index 1, never 0: directly observed for all 5 handlers in the instrumented run, and implied by 3 further passing runs on the memory-populated HOME. The main turn's POST is already in flight while recall is still scanning disk. Recording it as a nit rather than a defect; the same body['messages'] dispatch would make it uniform.
3. [Nit] fakeModelOptions() not adopted at the two pre-existing call sites
Lines ~400 and ~1241 still inline model/env/authType blocks that are byte-identical to what the helper returns — and this commit touches both (moving FAKE_SERVER_OPTIONS in) without switching them over. Same point the bot raised at stage 2. bd02c267 also dropped the helper's doc comment (/** Shared query options fragment for tests backed by a fake OpenAI server. */) — worth keeping.
4. [Nit] Reviewer Test Plan omits the bundle step
TEST_CLI_PATH resolves to dist/cli.js (globalSetup.ts:98), which npm run bundle produces — not npm run build. The plan lists build + typecheck only, so following it verbatim leaves no CLI to drive.
5. [Nit] rm file.txt has no filesystem oracle
The other two migrated cases use one (.env / blocked.txt must not exist). file.txt is never created, so this case rests solely on the message regex. Creating it and asserting it survives would close the "denied text but ran anyway" gap.
Also verified
- 5/5 green, ~7 s, no API key, on both HOMEs; 3 consecutive runs each.
- The two pre-existing fake-server cases (touched only by the reformat) still pass.
eslintexit 0 with no output, and the linter is genuinely live — a planted unused var makes it exit 1.prettier --checkclean.- The two-turn read-then-edit is required, not gratuitous — I rewrote the excludeTools case as a single-turn edit-only handler and it fails (
editneeds a prior read in-session). Worth stating since the extra turn is what forced the transcript dispatch. toMatchObjectonfindToolResult'snullreturn still fails the case (confirmed in vitest) — droppingexpect(...).toBeDefined()does not create a false pass, only a less specific message.- CI red is unrelated infra:
Audit critical runtime dependencies→npm auditregistry400 Bad Request/ "This endpoint is being retired". feat(core): add Goal v3 worker tools #7729 fails on the identical step; this PR touches no lockfile.
Harness (reproducible)
git worktree add wt7725 bd02c267 && cd wt7725
npm ci && npm run build && npm run bundle # dist/cli.js
# isolated HOMEs: h-clean = .qwen/settings.json only
# h-mem = + .qwen/memories/{feedback,user,project}/*.md
# each WITH a top-level `type:` frontmatter key (else scan.ts:68 drops it)
HOME=$PWD/../h-mem QWEN_SANDBOX=false npx vitest run --root ./integration-tests \
--poolOptions.threads.maxThreads 2 sdk-typescript/tool-control.test.ts \
-t '<5-case alternation>' --retry=0Side-query detector: log (requestIndex, body['tools'].length) inside each handler — the 1-tool request is the recall call. Load-bearing check: same dist, swap only the test file between git show 17c4e20:<path> and git show bd02c267:<path>.
中文说明
第 5 轮复核（head bd02c267，本地真实构建，5 个用例全部执行）
接我第一轮评审（head 17c4e20）。范围已收窄为纯测试改动，仅 1 个文件。
结论：可以合并。 第一轮的两个问题都已修复并验证。阻塞本 PR 的争议性 [Critical]（@doudouOUC 的 CHANGES_REQUESTED）已解决——@gwinthis 与 @yiliang114 两边都没错，只是环境不同。其余为建议/细节级。
上一轮问题 → 当前状态
| # | 问题 | 级别 | 状态 |
|---|---|---|---|
| 1 | prettier --check 失败 |
Blocker | 已修复（A/B 验证：旧 head 报 warn，新 head 通过） |
| 2 | requestIndex === 1 被 memory-recall 旁路调用吃掉 |
Critical | 已修复，且证明该修复是必要的 |
| 3 | workflow 相关问题 | 混合 | 已失效——bd02c267 已移出范围 |
争议 [Critical] 的裁定
同一个 dist/cli.js（两个 head 的 packages/ 完全一致），只替换测试文件，× 两种 HOME：
HOME 有 ~/.qwen/memories |
HOME 没有 | |
|---|---|---|
修复前（17c4e20） |
✗ expected undefined to be defined |
✓ 5/5 |
| 当前 head | ✓ 5/5（连续 3 次） | ✓ 5/5 |
根因：client.ts:2135 每个用户回合都会发起自动记忆召回旁路查询（enableManagedAutoMemory 默认为 true，config.ts:2284），它的 HTTP 请求落在 requestIndex === 1，正好占掉旧代码用于第二回合的槽位。请求日志（requestIndex, tools 数量）：有记忆时 [8, 1, 8, 8]，无记忆时 [8, 8, 8]——那个 1 个工具的请求就是召回调用。新的按 body['messages'] 内容分发不受影响。
注意：只有带 type: frontmatter 的记忆文件才会触发（scan.ts:68 会丢弃没有该字段的文件），我第一次搭的对照组因此是空跑的。
新发现
1.（建议）5 个用例里有 2 个完全没有验证 allowedTools。 把它们各自声称测试的参数清空后仍然全绿：allowedTools: ['ShellTool(echo )','ShellTool(ls )'] → [] 与 allowedTools: ['ShellTool(echo *)'] → [] 两个变异体存活；另外 3 个变异体都被杀死。原因是 permission-manager.ts:500 在读取 allowedTools 之前就直接放行只读 shell 命令，而 echo、ls 都在 READ_ONLY_ROOT_COMMANDS（shellReadOnlyChecker.ts:25）里。所以这两个用例实际验证的是只读自动放行路径，不是模式匹配。
这不是本 PR 引入的（merge-base 的断言更弱），且与 17c4e20 的变异杀伤集完全一致，说明断言重构没有造成退化。但本 PR 恰好重写了这两处断言，而描述里写的是断言"仍覆盖真实的权限与路由逻辑"——对这两个用例并不成立。修复方案（已双向验证：带模式通过、清空后变红）：把主体换成非只读命令，例如 touch allowed.txt + allowedTools: ['ShellTool(touch *)']，再用 mkdir blockeddir 作为反向对照。
2.（细节）另外 4 个 handler 仍然耦合 requestIndex === 0。 实测安全——旁路调用稳定落在 index 1，从不落在 0（5/5 用例 × 4 次运行），因为主回合的请求早已在途、而召回还在扫盘。记为细节而非缺陷。
3.（细节）fakeModelOptions() 没有在两处既有调用点采用（约 400 行、1241 行），两段内联代码与该 helper 的返回值逐字节相同，而本次提交恰好动了这两处。另外 bd02c267 删掉了该 helper 的文档注释，建议保留。
4.（细节）Reviewer Test Plan 缺少 bundle 步骤。 TEST_CLI_PATH 指向 dist/cli.js（globalSetup.ts:98），由 npm run bundle 产出，而非 npm run build。
5.（细节）rm file.txt 缺少文件系统对照。 另两个用例都有（.env / blocked.txt 必须不存在），而 file.txt 从未创建，该用例只依赖消息正则。
其他已验证
- 5/5 通过，约 7 秒，无需真实 API key；两种 HOME 各连续 3 次。
- 仅被格式化影响的 2 个既有 fake-server 用例仍通过；
eslint、prettier均干净。 - 两回合"先读后改"是必需的：我把该用例改成单回合直接 edit 会失败（
edit需要会话内先 read）。这也正是必须改用内容分发的原因。 findToolResult返回null时toMatchObject仍会让用例失败（已在 vitest 中确认），不会假通过。- CI 变红与本 PR 无关：
Audit critical runtime dependencies步骤npm audit收到400 Bad Request（该接口正在下线）。feat(core): add Goal v3 worker tools #7729 在同一步骤同样失败，而本 PR 未改动 lockfile。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Unresolved, please confirm: [Critical] .github/workflows/qwen-autofix.yml — gh issue view failure leaves ISSUE_JSON empty under set -euo pipefail (file not in this PR's diff; cannot verify at reviewed commit)
中文说明
已审查。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 未决，请确认：[Critical] .github/workflows/qwen-autofix.yml — gh issue view failure leaves ISSUE_JSON empty under set -euo pipefail (file not in this PR's diff; cannot verify at reviewed commit)
— qwen3.7-max via Qwen Code /review
|
No action on the latest autofix Critical: |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Unresolved, please confirm: [Critical] .github/workflows/qwen-autofix.yml — gh issue view failure leaves ISSUE_JSON empty under set -euo pipefail (file not in this PR's diff; cannot verify at reviewed commit)
中文说明
已审查。 建议见行内评论。 未决，请确认：[Critical] .github/workflows/qwen-autofix.yml — gh issue view failure leaves ISSUE_JSON empty under set -euo pipefail (file not in this PR's diff; cannot verify at reviewed commit)
— qwen3.7-max via Qwen Code /review
|
Review closeout:
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Unresolved, please confirm: [Critical] .github/workflows/qwen-autofix.yml — gh issue view failure leaves ISSUE_JSON empty under set -euo pipefail (file not in this PR's diff; the workflow changes were removed from scope)
中文说明
已审查。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 未决，请确认：[Critical] .github/workflows/qwen-autofix.yml — gh issue view failure leaves ISSUE_JSON empty under set -euo pipefail (file not in this PR's diff; the workflow changes were removed from scope)
— qwen3.7-max via Qwen Code /review
Round-6 re-verification (head
|
| run | HOME | result |
|---|---|---|
| 1, 2, 4 | clean | 5/5 ✓ (~12s) |
| 3, 5 | ~/.qwen/memories populated |
5/5 ✓ (~15s) |
The memories HOME is positive-controlled, not decorative: checking out the pre-fix test file (17c4e20) into the same build reproduces the original failure exactly —
FAIL ... should block edit operations on specific path patterns with excludeTools
AssertionError: expected undefined to be defined
— while f1034bd7 is green under that same HOME. That closes out the round-2 ↔ round-5 reviewer disagreement for good: the transcript-content dispatch really is what fixes it, not luck.
Mutation matrix — 4/5 mutants die
Each cell empties the one parameter the test claims to verify:
| test | mutant | result |
|---|---|---|
block edit … excludeTools |
excludeTools: [] |
✗ dies |
| block shell prefix | excludeTools: [] |
✗ dies |
allow shell pattern (touch/mkdir) |
allowedTools: [] |
✗ dies ← the R5 fix, confirmed load-bearing |
auto-approve path (Edit(*.json)) |
allowedTools: [] |
✗ dies |
auto-approve shell (echo) |
allowedTools: [] |
The touch/mkdir swap is a real repair, not cosmetic: READ_ONLY_ROOT_COMMANDS (packages/core/src/utils/shellReadOnlyChecker.ts:25) contains echo and ls but not touch/mkdir, and permission-manager.ts:500 returns allow for read-only shell commands before allowedTools is consulted.
Remaining (non-blocking): tool-control.test.ts:949 is still vacuous for the same reason
should auto-approve specific shell commands with pattern matching (line 918) still pairs allowedTools: ['ShellTool(echo *)'] with { command: 'echo test' }. echo is auto-allowed by the read-only rule, so expect(canUseToolCalls).toEqual(['run_shell_command']) is satisfied whether or not the pattern exists — emptying allowedTools leaves the test green. What it does still prove is the canUseTool deny path for mkdir/touch; what it does not prove is the thing in its name.
Same one-line-shaped repair as the sibling test. I applied and ran this exact patch — it passes as written and goes red under allowedTools: []:
- { command: 'echo test' },
- 'shell-echo-test',
+ { command: 'touch approved.txt' },
+ 'shell-approved',
...
- { command: 'touch blocked.txt' },
- 'shell-touch',
+ { command: 'mkdir blocked-dir' },
+ 'shell-blocked',
...
- // Auto-approve echo commands
- allowedTools: ['ShellTool(echo *)'],
+ // Auto-approve touch commands
+ allowedTools: ['ShellTool(touch *)'],
...
- expect(findToolResult(messages, 'shell-echo-test')).toMatchObject({
+ expect(findToolResult(messages, 'shell-approved')).toMatchObject({
isError: false,
});
- expect(findToolResult(messages, 'shell-touch')).toMatchObject({
+ expect(helper.fileExists('approved.txt')).toBe(true);
+ expect(findToolResult(messages, 'shell-blocked')).toMatchObject({
...
- expect(helper.fileExists('blocked.txt')).toBe(false);
+ expect(helper.fileExists('blocked-dir')).toBe(false);(The prompt string on line 942 needs the matching update.)
Nits
tool-control.test.ts:722-727— the allow-shell case asserts onlyisError: false; it never checks thatallowed.txt/allowed-diractually appeared. A one-lineexpect(helper.fileExists('allowed.txt')).toBe(true)turns "the scheduler reported success" into "the command really ran", matching the filesystem oracle you already added to the write_file case.fakeModelOptionsduplication — the helper added at line 46 is a good extraction, but lines 400-410 and 1241-1252 still inline the identical 12-line block. I take the "out of scope" call, though since the helper is introduced by this PR the cleanup is arguably part of it. Non-blocking either way.- Pre-existing, not yours:
npx tsc -p integration-tests/tsconfig.jsonfails withTS5063: Substitutions for pattern '//' should be an array(the"//"comment key insidepaths), and esbuild warns on the same line.
Checked and fine (recording so the next round doesn't re-litigate)
requestIndex === 0in the four single-turn handlers is safe. I instrumented one handler to log(requestIndex, body.tools.length): with memories the sequence is14 → 1 → 14(the 1-tool request is therespond_in_schemarecall side-query), without memories14 → 14. The side-query never lands at index 0, so only the multi-turn edit case needed transcript dispatch — the asymmetry in this file is correct, not an oversight.- The 2-turn read-then-edit script in the excludeTools case is necessary, not gratuitous.
editrequires a priorread_filein-session; collapsing it to a single edit-only turn fails onedit-readme. Please don't "simplify" it later. allowedTools: ['Edit(*.json)']auto-approving awrite_filecall is intended behavior, not an accident —Editis a documented meta-category coveringedit/write_file/notebook_edit(permission-manager.test.ts:127,:2593).- Prettier is clean at head and the check is positive-controlled (planting bad formatting makes it exit 1). The R1 formatting blocker stays fixed.
中文说明
第 6 轮复核（head f1034bd7，本地构建 + 5 个用例全部执行 + 5 格变异矩阵）
承接第 5 轮评审（head bd02c267）。此后唯一的改动是 f1034bd7：把某个用例里的 echo/ls 换成 touch/mkdir，正是第 5 轮变异测试发现的问题。
结论：可以合并。 第 5 轮的问题已修复且经变异验证。另一个同类用例仍存在相同弱点（下面给出已验证的补丁），但不阻塞合并：该用例仍然通过，并且确实覆盖了真实的拒绝路径。
验证方式
git diff --name-only 0f56e35c f1034bd7 只有测试文件一项，说明 PR head 的 packages/ 与 merge-base 完全一致。我在干净 worktree 中从 head 构建（npm run build && npm run bundle），以 --retry=0、无 API key 运行 5 个聚焦用例：干净 HOME 下 3 次 5/5 通过（约 12s），带 ~/.qwen/memories 的 HOME 下 2 次 5/5 通过（约 15s）。
带 memories 的 HOME 是有正对照的：把修复前的测试文件（17c4e20）放进同一份构建，能精确复现原始失败 expected undefined to be defined；而 f1034bd7 在同样 HOME 下全绿。这也彻底了结了第 2 轮与第 5 轮评审者结论相反的争议——真正起作用的是按会话内容路由，而不是运气。
变异矩阵：5 个中 4 个被杀死
把每个用例声称验证的那个参数清空：excludeTools: []（编辑拦截、shell 前缀拦截）、allowedTools: []（touch/mkdir 放行、Edit(*.json) 自动批准）四格全部变红；只有 echo 自动批准那格依然通过。
touch/mkdir 的替换是实质修复而非表面调整：READ_ONLY_ROOT_COMMANDS（shellReadOnlyChecker.ts:25）包含 echo、ls 但不含 touch/mkdir，而 permission-manager.ts:500 会在读取 allowedTools 之前就对只读 shell 命令返回 allow。
遗留（不阻塞）：tool-control.test.ts:949 仍然是空转断言
should auto-approve specific shell commands with pattern matching（第 918 行）仍然用 allowedTools: ['ShellTool(echo *)'] 搭配 { command: 'echo test' }。echo 被只读规则自动放行，因此有没有这条模式该用例都会通过——清空 allowedTools 依旧全绿。它真正证明的是 mkdir/touch 的 canUseTool 拒绝路径，而不是用例名字所说的那件事。
修法与已修好的兄弟用例同形。上面的补丁我已实际应用并运行：原样通过，且在 allowedTools: [] 下变红（第 942 行的 prompt 字符串需同步修改）。
小建议
- 第 722-727 行：放行用例只断言
isError: false，没有检查allowed.txt/allowed-dir是否真的产生。补一行expect(helper.fileExists('allowed.txt')).toBe(true)，与 write_file 用例里已有的文件系统证据保持一致。 fakeModelOptions重复：第 46 行抽出的 helper 很好，但第 400-410 与 1241-1252 行仍然内联了同样的 12 行。"超出范围"的判断我接受；只是该 helper 正是本 PR 引入的，顺手统一也说得通。不阻塞。- 既有问题，与本 PR 无关：
npx tsc -p integration-tests/tsconfig.json报TS5063（paths里的"//"注释键），esbuild 也在同一行告警。
已核查且没有问题（记录下来，避免下一轮重复讨论）
- 四个单轮 handler 里的
requestIndex === 0是安全的。 我给某个 handler 加了(requestIndex, body.tools.length)日志：带 memories 时序列是14 → 1 → 14（那次 1 个工具的请求就是respond_in_schema记忆召回旁路查询），不带时是14 → 14。旁路查询永远不会落在 index 0，所以只有多轮的编辑用例需要按会话内容路由——本文件里的这种不对称是正确设计，不是疏漏。 - 编辑拦截用例里"先读后改"的两轮脚本是必要的，不是多余的。
edit需要同一会话内先有read_file；改成单轮纯编辑会在edit-readme上失败。请后续不要"简化"它。 - 用
allowedTools: ['Edit(*.json)']自动批准write_file是预期行为，不是巧合——Edit是覆盖edit/write_file/notebook_edit的元类别（permission-manager.test.ts:127、:2593）。 - head 上 Prettier 检查通过，且该检查有正对照（故意破坏格式会让它退出码为 1）。第 1 轮的格式化阻塞问题保持已修复。
…n tests echo and ls are in READ_ONLY_ROOT_COMMANDS, so permission-manager auto-allows them before allowedTools is consulted. The 'auto-approve shell commands with pattern matching' test used echo test for the allow case, which never exercised allowedTools pattern matching. Switch to touch (non-read-only) so the allow path genuinely tests ShellTool(touch *) matching. Also replace inline model/env/authType blocks in two pre-existing tests with the fakeModelOptions helper for consistency with the migrated tests.
|
@qwen-code /triage |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.7-max via Qwen Code /review
|
Released in v0.21.1. |
5 participants
What this PR does
Migrates five flaky tool-control integration cases from real-model tool selection to deterministic fake-model tool calls. The assertions still exercise the actual permission and routing behavior while removing model-choice variance.
Why it is needed
The Docker-sandbox E2E job was repeatedly timing out when the real model did not choose the expected edit or shell call. These cases are meant to verify permission enforcement, not model planning, so deterministic inputs make the signal reliable.
Reviewer Test Plan
How to verify
Run
QWEN_SANDBOX=false npx vitest run --root ./integration-tests sdk-typescript/tool-control.test.ts -t "should block edit operations on specific path patterns with excludeTools|should block specific shell commands with prefix pattern|should allow specific shell commands with pattern matching|should auto-approve specific path patterns with allowedTools|should auto-approve specific shell commands with pattern matching" --retry=0. All five migrated cases should pass without a real model API key.Evidence (Before & After)
Before: the cases depended on a real model choosing exact tool calls and could time out. After: a fake model emits fixed tool calls, and the five focused cases pass locally.
Tested on
Environment (optional)
Local run with sandbox disabled and an isolated Qwen home.
npm run buildandnpm run typecheckalso pass.Risk & Scope
Linked Issues
Part of #7616
中文说明
本 PR 做了什么
把 5 个不稳定的工具控制集成用例从真实模型选工具迁移为 fake model 固定发出工具调用。断言仍覆盖真实的权限与路由逻辑，同时消除模型决策波动。
为什么需要
Docker sandbox E2E 曾多次因为真实模型没有选择预期的编辑或 shell 调用而超时。这些用例要验证的是权限执行，而不是模型规划，所以应使用确定性的输入。
Reviewer Test Plan
如何验证
运行上面的聚焦命令；5 个迁移用例应全部通过，且无需真实模型 API key。
前后证据
Before：用例依赖真实模型选择精确工具调用，可能超时。After：fake model 发出固定工具调用，本地 5 个聚焦用例全部通过。
测试环境
macOS 已验证；Windows 与 Linux 未在本地验证。sandbox 关闭并使用隔离的 Qwen home；
npm run build与npm run typecheck均通过。风险与范围
关联 #7616。