fix(test): deflake tool-control E2E content assertions - #7261
Merged
Conversation
The tool-control E2E tests assert on specific file content
(`toContain('modified')`) after asking the model to write to a file.
This is flaky because the model may paraphrase (e.g. write "updated"
instead of "modified") or, in docker sandbox, occasionally not call
write_file at all.
Changes:
- Replace 4× `toContain('modified')` with `not.toBe(originalContent)`.
The tests verify tool-control mechanics (coreTools, allowedTools,
excludeTools), not model wording. Checking that the file changed
from its original content is sufficient and model-agnostic.
- For the excludeTools path-pattern test, remove write_file from
coreTools so the model must use edit, making the
`editCalls.length > 0` assertion deterministic. Also make the
prompt explicitly request the edit tool.
Fixes the recurring flaky failure in
`sdk-typescript/tool-control.test.ts > Combined tool control scenarios
> should work with coreTools + allowedTools` (see #7260, and the
2026-07-15 excludeTools variant).
wenshao
reviewed
Jul 20, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.8-max-preview via Qwen Code /review
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with clear evidence. Two specific CI failures on main are cited (2026-07-19 #7260: Direction: aligned — deflaking E2E assertions that depend on model wording is the right call. The tool-control mechanics (coreTools, excludeTools, allowedTools) are what these tests verify, not whether the model writes the literal word "modified". No CHANGELOG reference needed for test-only changes. Size: not applicable — no core paths touched. Single test file, +16/−10. Approach: scope feels right. Five targeted assertion changes, each replacing a model-wording-dependent check with a model-agnostic one. The excludeTools fix (removing Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有明确证据。PR 引用了两次 main 上的 CI 失败（2026-07-19 #7260： 方向：对齐——将依赖模型措辞的 E2E 断言改为模型无关的断言是正确做法。这些测试验证的是 tool control 机制，不是模型是否写了 "modified" 这个词。测试改动无需 CHANGELOG 参考。 规模：不适用——未触及核心路径。单个测试文件，+16/−10。 方案：范围合理。五处精准的断言修改，每处都将依赖模型措辞的检查替换为模型无关的检查。excludeTools 修复（从 coreTools 移除 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Code ReviewIndependent proposal: Given flaky assertions that depend on model wording, I'd replace Comparison with the diff: The PR does exactly this. All five changes are correct:
No critical blockers. No convention violations. Comments added are appropriate (explaining why the assertion changed). Real-Scenario TestingRan all 5 modified tests individually against a real model endpoint (built from this PR's branch, All 5 passed. The assertions hold with real model responses — the model paraphrases freely and the 中文说明代码审查独立方案： 对于依赖模型措辞的 flaky 断言，我会将 与 diff 对比： PR 完全这样做了。五处改动均正确：
无关键阻塞。无规范违反。添加的注释恰当（解释断言变更的原因）。 真实场景测试对 5 个修改的测试逐一运行（使用真实模型端点，从本 PR 分支构建， — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean deflake with clear evidence, minimal focused fix, all tests verified against a real model. This is exactly what a test deflake should look like. The problem is real and well-documented (two CI failures on main with specific error messages), the fix is minimal (5 assertion changes in one file, no unrelated edits), and the approach is sound — the tool-call assertions are the actual test targets, and the content checks just need to confirm the file changed, not that the model used a specific word. The excludeTools fix (removing Ran all 5 modified tests against a real model endpoint — all passed. The model paraphrases freely and the assertions hold regardless of wording. Ships it. ✅ 中文说明置信度：5/5——干净的 deflake，证据清晰，修复精准，所有测试均通过真实模型验证。 这正是测试 deflake 应有的样子。问题真实且有据可查（main 上两次 CI 失败，有具体错误信息），修复最小化（一个文件中 5 处断言修改，无无关改动），方案合理——工具调用断言是真正的测试目标，内容检查只需确认文件发生了变化，而非模型使用了特定词语。excludeTools 修复（从 coreTools 移除 对 5 个修改的测试运行了真实模型端点测试——全部通过。模型自由措辞，断言无论措辞如何都成立。 可以合并 ✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
approved these changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.8-max-preview via Qwen Code /review
wenshao
approved these changes
Jul 20, 2026
Collaborator
|
Released in v0.20.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Makes the
sdk-typescript/tool-control.test.tsE2E tests robust against model wording variance. Two changes:Content assertions (4 places): Replace
expect(content).toContain('modified')withexpect(content).not.toBe(originalContent). These tests verify tool-control mechanics (coreTools, allowedTools, excludeTools), not whether the model writes the exact word "modified". The model may paraphrase ("updated", "changed", etc.), causing false failures.excludeTools path-pattern test: Remove
write_filefromcoreToolsso the model must useedit, making theeditCalls.length > 0assertion deterministic. Previously the model could choosewrite_fileoveredit, leavingeditCallsempty and failing the assertion. Also make the prompt explicitly request the edit tool.Why it's needed
This test file has caused recurring flaky CI failures on main:
should work with coreTools + allowedTools—expected 'test' to contain 'modified'. The model called write_file but didn't write the literal word "modified".should block edit operations on specific path patterns with excludeTools—expected 0 to be greater than 0. The model used write_file instead of edit, so editCalls was empty.Both failures occurred only in
E2E Test (Linux) - sandbox:docker, with the same commit passing onsandbox:noneand macOS. The triggering commits were unrelated to tool-control.Reviewer Test Plan
How to verify
toContain('read_file'),toContain('write_file'),not.toContain('run_shell_command'), etc.) are unchanged — these are the actual test targets.edit(notwrite_file) in coreTools, so the model has no alternative but to use edit.Evidence (Before & After)
N/A — test-only change, no user-visible behavior.
Tested on
Environment (optional)
E2E tests require a running model endpoint; verified via CI.
Risk & Scope
not.toBe(original)is a weaker assertion thantoContain('modified'). However, the tool-call assertions (toContain('write_file')) already verify the tool was invoked; the content check is a secondary sanity check.Linked Issues
Fixes #7260
中文说明
这个 PR 做了什么
修复
sdk-typescript/tool-control.test.ts中的 flaky 断言。两处改动：toContain('modified')改为not.toBe(原始内容)。这些测试验证的是 tool control 机制，不是模型的具体措辞。模型可能写 "updated" 而非 "modified"，导致误报。editCalls.length > 0断言的不确定性。风险与范围
not.toBe(原始)比toContain('modified')弱，但工具调用断言已验证 write_file 被调用，内容检查只是辅助。