test(integration): migrate acp-cron to fake-openai-server (#8076) - #8082
Conversation
E2E Test ReportRoot CauseThe FixMigrated the test to use
Also added The cron scheduler still operates on real minute-boundary timing — only model responses are deterministic. Test ResultVerification
中文说明E2E 测试报告根本原因
修复方案将测试迁移为使用
同时添加了 cron 调度器仍然使用真实的分钟边界计时——只有模型响应是确定性的。 测试结果验证
🧠 Handled by Qwen Code · model/模型 |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical — issue #8076 documents the Direction: well-aligned. Migrating flaky real-model integration tests to Size: not applicable — single test file, no core paths. Approach: scope is right. The diff does one thing — swap real model calls for scripted responses and add Risk: no elevated risk signals — the only changed file is a Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到，非理论性——issue #8076 记录了 方向：完全对齐。将不稳定的真实模型集成测试迁移到 规模：不适用——仅单个测试文件，未触及核心路径。 方案：范围合理。diff 只做一件事——将真实模型调用替换为脚本化响应并添加 风险：无升级风险信号——唯一变更文件是 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: swap real model calls for Comparison with the diff: the PR does exactly this and follows the repo's established integration-test conventions. Verified against the helper API ( The No critical blockers. No convention violations. TestingThis is a test-only change with no user-visible behavior — real-scenario testing is N/A. CI evidence for
Latest run per check; the material skipped jobs are shown so the integration-test gap is visible. / 每个检查名取最新一次运行；列出关键的 skipped 任务以便看到集成测试缺口。 中文说明代码审查独立方案： 将真实模型调用替换为 与 diff 对比： PR 完全按此实现，并遵循仓库既有的集成测试约定。已对照 helper API 核实（
无关键阻塞。无规范违反。 测试仅测试变更，无用户可见行为——真实场景测试不适用。
— Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean, focused test migration that follows the repo's own This is a textbook flaky-test fix. The problem is observed (main CI failure, issue #8076), the solution follows the established pattern (#7934), and the diff does one thing. Since the last review the author pushed three follow-up commits — a comment / no-op-cleanup correction, diagnostics + isolation hardening, and clearing the stale What settles it for me is @wenshao's independent mutation testing in this thread: flipping The only thing this test no longer validates is that a real model can interpret the CI is green on 中文说明置信度：5/5 — 干净、专注的测试迁移，遵循仓库自身的 这是一个教科书式的不稳定测试修复。问题已观测到（主分支 CI 失败，issue #8076），方案遵循既有模式（#7934），diff 只做一件事。上次 review 之后作者推了三个后续提交——注释/空操作清理修正、诊断与隔离加固、setup 前清理陈旧 真正让我放心的是本线程中 @wenshao 的独立变异测试：翻转 该测试不再验证真实模型能否解释 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
github-actions
Bot
added
the
review/self-reported
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. |
|
Review: PR #8082 — test(integration): migrate acp-cron to fake-openai-server Clean, well-scoped test-only change. Single file, follows the established No actionable issues found. A few observations:
LGTM, ship it. |
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.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix Review Response — PR #8082No changes made this round. Feedback triage
No inline comments, no failed checks, no changes-requested reviews. 中文说明Autofix 审查回应 — PR #8082本轮未做任何更改。 反馈分类
无行内评论、无失败检查、无要求更改的审查。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Local verification round — real build, real ACP session, no credentialsVerdict: Built and run on a real local macOS environment ( 中文完整版本（点击展开）本地验证轮次 —— 真实构建、真实 ACP 会话、无凭据结论：可以合并（ 在真实的本地 macOS 环境完成构建与运行（ 核心主张与 A/B 对照这个 PR 要让一件事成立：
B 格是承重的那一格：同一产物、同一批断言、同样的时序，只把模型回复换掉，测试就变红， 次要主张"没有真实模型调用"。 这是在接缝上测到的，不是靠读代码推断：两次插桩运行中 fake server 每个 agent 独立的 对 PR 描述的更正
发现1.（低，非阻塞）新增注释里
也就是说，cron 触发的那一轮拿到的是 2.（低，非阻塞）"Part 4: Clean up the cron job" 现在是空操作。 handler 只在 3.（信息性，既有问题 —— 与本 PR 无关） 门禁
未覆盖范围
方法macOS Darwin 25.6.0，Node v24.18.1。通过 PR 的测试文件从
Central claim and A/BThe PR exists to make one thing true: the outcome of
Cell B is the load-bearing one: identical bundle, identical assertions,
Secondary claims"No real model calls." Confirmed at the seam, not by inspection: across two Per-agent Corrections to the PR description
Findings1. (Low, non-blocking) The new
So the cron-fired turn is answered with the
2. (Low, non-blocking) "Part 4: Clean up the cron job" is now a no-op. The 3. (Informational, pre-existing — not caused by this PR) Gates
Not covered
MethodologymacOS Darwin 25.6.0, Node v24.18.1. Real build via The PR's test file was extracted from
Local maintainer verification round; advisory evidence, not an approval. 11 test executions, raw logs and harness generators kept locally. |
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/10 轮）。改动内容与我反驳保留之处如下： Review feedback — round summaryFinding 1 (Low, non-blocking):
|
Review: test(integration): migrate acp-cron to fake-openai-server (#8082)OverviewConverts I verified the mechanics that matter:
Issues1. The comment is honest that later indices don't map 1:1, but index 0 is load-bearing and carries the same risk it disclaims: if the CLI ever issues any model call before the first user prompt (session-title generation, a warm-up, a classifier pass during which points at the cron scheduler, not at the dispatch that actually broke. Cheap fix — fail fast right after Part 1: expect(
fakeServer.requests[0]?.body['messages'],
'requestIndex 0 was not the cron_create prompt — dispatch shifted',
).toBeDefined();
// or, stronger: assert a tool_call sessionUpdate for cron_create arrivedAlternatively dispatch on content instead of index (match the last user message against the cron marker), which is immune to index drift entirely. 2.
3.
4.
Coverage & description
TimingWorst case is ~66 s: up to 60 s to the minute boundary plus ≤6 s recurring jitter (10 % of period, VerdictApprove once issue 1 is addressed (or consciously accepted) — it's the difference between a clear failure and a 75 s red herring. Issues 2–4 are small cleanups. |
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. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
Fail fast when the cron_create tool call is not served to the first user prompt instead of timing out opaquely 75s later, and dump the fake server request log on failure so a dispatch shift is quick to diagnose. Close the fake server even when test setup throws, drop the dead FAKE_SERVER_OPTIONS (container mode is skipped by IS_SANDBOX), and move QWEN_HOME out of the agent workspace cwd so workspace scans never see it.
|
🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/10 轮）。改动内容与我反驳保留之处如下： Review feedback addressedAll changes are in Inline comments (automated reviewer)
Issue-level review (@wenshao)
Not changed (recorded)
Verification
中文说明已处理的审查反馈所有改动均在 行内评论（自动审查器）
Issue 级审查（@wenshao）
未改动（已记录）
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 AutoFix could not start — a setup step failed (or the run was cancelled) before the agent ran, so no fix was attempted. This is normally a transient infra issue, a broken base build, or a cancelled run — not this PR. It will retry on the next scan. AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why). Run log: https://github.com/QwenLM/qwen-code/actions/runs/30553093684 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No code changes this roundThe only feedback was an informational Because this PR is itself an integration-test change ( VerificationCommands run this round (all on the PR head, sandbox disabled as in the CI suite):
The integration suite the reviewer could not verify now runs and passes locally, so the previously-unverified 中文说明本轮不做代码改动唯一的反馈是自动审查机器人的一条信息性 由于本 PR 本身就是一处集成测试改动（ 验证本轮运行的命令（均在 PR head 上执行，并像 CI 套件一样禁用了沙箱）：
审查者此前无法验证的集成测试套件，如今已能在本地运行并通过，因此本分支上原先未经验证的 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
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. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 4/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/10 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[rc:3684242502] Stale
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix Review Round — No Action NeededThe only feedback this round is a This is informational, not a defect finding or change request:
The integration test suite ( No code changes are warranted this round. 中文说明Autofix 审查轮次 — 无需操作本轮唯一的反馈是自动审查机器人的一条 这是一条信息性说明，而非缺陷发现或变更请求：
集成测试套件（ 本轮无需进行代码变更。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 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
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action requiredThe only feedback this round is a COMMENTED review from the automated reviewer (
There is nothing actionable to address in the code. The integration test skip is a CI scheduling/trigger condition outside the scope of this PR's code changes. 中文说明无需操作本轮唯一的反馈是自动审查器（
代码中没有需要处理的可操作项。集成测试被跳过是 CI 调度/触发条件的问题，不在本 PR 代码变更的范围内。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Local verification — built and actually ran this test
Setup: Verdict: LGTM, safe to merge. One non-blocking nit at the end. 1. Determinism — 8/8 with
|
| # | Mutation | Result |
|---|---|---|
| M1 | Session.ts: source: job.cronExpr === '@wakeup' ? 'loop' : 'cron' → 'loop' |
❌ caught at acp-cron.test.ts:408 in 33s |
| M2 | Session.ts: drop the cron user_message_chunk echo |
❌ caught at step 3a (75s window) |
| M3 | 20s model latency on the cron-fired turn only (CLI untouched) | ❌ caught at step 3b (15s window) |
| — | pristine bundle, same test | ✅ passes |
M3 is the interesting one — it identifies what actually broke on main. Both the run the PR cites (30506000682) and the most recent recurrence (30628967374) failed the same way:
Error: Timed out waiting for sessionUpdate: agent_message_chunk after cron fire
(after 15000ms, saw 53 updates: [… agent_message_chunk, user_message_chunk])
The last update seen is user_message_chunk — the cron did fire and did echo; what blew the budget was the real model's first response chunk not landing inside the 15s post-fire window. The description frames the flake mostly as tool-call variance ("may not call the right tool, may use wrong parameters"); the observed failures were the third clause, model latency. Same fix either way — scripted responses come back in ~ms — and M3 shows that 15s budget is still enforced, so the migration removes the cause without removing the assertion. Worth correcting in the description if you touch it again.
3. The QWEN_HOME isolation is load-bearing, not cosmetic
Both variants pass, so the difference is contamination, not the result. I exported an ambient QWEN_HOME to make the shared/global config dir observable, then ran the PR's file and a copy with only the QWEN_HOME line removed:
- With the PR's line: 0 files.
- Without it: 9 files, including
settings.jsonrewritten tosecurity.auth.selectedType: "openai",installation_id,usage_record.jsonl, andtmp/<project-hash>/scheduled_tasks.lock— the global cron-scheduler lock, held while other test files in the same repo run concurrently (fileParallelism,maxThreads: 4). That is a concrete cross-test interference channel, and it is now gone.
4. Other checks
- Cold start: passes with an empty global config dir (no pre-seeded auth/settings), so it does not depend on a warm developer machine.
- Hostile proxy env: with ambient
HTTP_PROXY/HTTPS_PROXY/ALL_PROXYpointed at a dead port, the test still passes — theNO_PROXY-only defence is sufficient here (sibling tests likegen-ai-telemetry.test.tsalso unset the proxy vars; not needed for this one). - No leakage: after runs, no orphan
--acpprocesses and no leftover<testdir>-homedirectory. - Dropping Part 4 is safe: the tool result from the scripted call reads "Session-only (not written to disk, dies when Qwen exits)", so with the arguments now pinned there is no durable job to clean up — the
cron_deleteprompt really was a no-op. Under the old real-model version the model chosedurableitself, so pinning the arguments is what makes that removal safe. eslint --max-warnings 0andprettier --checkclean on the changed file.
5. Nit (non-blocking, follow-up at most) — index-keyed dispatch
Instrumenting the fake server shows 6 model calls per run; only i=0 is the user prompt, and i=1/i=2/i=4 are internal (tool-call classification, post-tool turn, suggestion mode) — exactly as the added comment describes.
The requests[0] fail-fast guard does work: inserting one extra session/prompt before Part 1 fails the test in 1.2s with requestIndex 0 was not the cron_create prompt — dispatch shifted, instead of an opaque 75s timeout.
But note i=1 — the classification pass quotes the cron_create arguments, so its payload also contains CRONFIRE7742. A future internal call that lands ahead of the first prompt while quoting the prompt or the tool arguments (title generation is the obvious candidate) would satisfy the guard and still shift dispatch, handing back the opaque timeout the guard exists to prevent. Keying on message content instead removes the class — I ran this with the same inserted turn and it passes:
const fakeServer = await startFakeOpenAIServer(({ body }) => {
const msgs = (body['messages'] ?? []) as Array<Record<string, unknown>>;
const lastText = JSON.stringify(msgs[msgs.length - 1]?.['content'] ?? '');
if (lastText.includes('Call cron_create with cron expression')) {
return { toolCalls: [fakeToolCall('cron_create', { /* … */ })] };
}
return { content: 'Done.' };
});Not worth another round on this PR — the current form is correct today and fails loudly for the common shift.
中文说明
本地验证 —— 实际构建并运行了这个测试
Integration Tests (CLI, No Sandbox) 以 merge_group 为触发条件，因此在 PR 事件上被跳过；本 PR 的每一轮自动评审最后都写着 "Not reviewed: build-and-test — … was skipped in CI and its suite did not run locally." 所以我基于 PR head e7d63fb 做了构建，真实运行了迁移后的测试，并补充了反向对照实验，确认它仍然能发现问题。
环境： 在 PR head 执行 npm run bundle → 测试驱动真实的打包 CLI（dist/cli.js --acp）通过 ACP 与 fake-openai-server 交互。Linux、Node v22.22.2、QWEN_SANDBOX=false。除特别说明处外，PR 自身的测试文件均未改动。
结论：LGTM，可以合并。 末尾有一个不阻塞合并的小建议。
1. 确定性 —— retry: 0 下 8/8 通过
连续 8 次运行，每次 rc=0，耗时 20.5s–64.5s。耗时差异完全来自真实的分钟边界等待——在 :45 启动的run 21 秒结束，在 :01 启动的需要 65 秒——说明调度器仍在做真实的计时工作，与 PR 描述一致。差异中没有任何一部分来自模型延迟。
2. 迁移后的测试在管道被破坏时仍然会失败
改写后的测试变绿本身说明不了什么，所以我从三个位置破坏了生产代码路径，并用 PR 未改动的测试文件重新运行：
| # | 变异 | 结果 |
|---|---|---|
| M1 | Session.ts：source: job.cronExpr === '@wakeup' ? 'loop' : 'cron' → 'loop' |
❌ 33 秒内在 acp-cron.test.ts:408 捕获 |
| M2 | Session.ts：去掉 cron 的 user_message_chunk 回显 |
❌ 在 3a 步（75s 窗口）捕获 |
| M3 | 仅对 cron 触发的那一轮注入 20s 模型延迟（CLI 未改） | ❌ 在 3b 步（15s 窗口）捕获 |
| — | 干净的 bundle，同一测试 | ✅ 通过 |
M3 最有价值——它定位了 main 上真正的失败原因。 PR 引用的那次运行（30506000682）和最近一次复现（30628967374）失败方式完全相同：
Error: Timed out waiting for sessionUpdate: agent_message_chunk after cron fire
(after 15000ms, saw 53 updates: [… agent_message_chunk, user_message_chunk])
最后一条更新是 user_message_chunk——也就是 cron 确实触发了、也确实回显了；真正超时的是真实模型的首个响应分片没能落在 15 秒的触发后窗口内。PR 描述主要把这次不稳定归因于工具调用的方差（"可能不调用正确的工具、使用错误的参数"）；而实际观测到的失败属于第三种情况，即模型延迟。两者的修复方式相同——脚本化响应在毫秒级返回——而且 M3 表明那个 15 秒预算仍然有效，所以这次迁移是在不削弱断言的前提下消除了诱因。如果之后还要改描述，建议顺手修正这一点。
3. QWEN_HOME 隔离是实质性的，不是装饰
两个变体都能通过，所以差异在于污染而非结果。我导出了一个环境级 QWEN_HOME，让共享/全局配置目录可观测，然后分别运行 PR 的文件和一个仅删除 QWEN_HOME 那一行的副本：
- 带 PR 这一行：0 个文件。
- 不带：9 个文件，包括被改写为
security.auth.selectedType: "openai"的settings.json、installation_id、usage_record.jsonl，以及tmp/<project-hash>/scheduled_tasks.lock—— 也就是全局的 cron 调度锁，而它是在同一仓库中其他测试文件并发运行（fileParallelism、maxThreads: 4）期间被持有的。这是一条实实在在的跨测试干扰通道，现在被消除了。
4. 其他检查
- 冷启动： 在空的全局配置目录（没有预置的认证/设置）下通过，说明它不依赖"热"的开发机环境。
- 恶劣代理环境： 把环境级
HTTP_PROXY/HTTPS_PROXY/ALL_PROXY指向一个死端口后测试仍然通过——这里只设NO_PROXY的做法是够用的（gen-ai-telemetry.test.ts等同类测试还会清空这些代理变量，本例不需要）。 - 无残留： 运行结束后没有游离的
--acp进程，也没有残留的<testdir>-home目录。 - 去掉 Part 4 是安全的： 脚本化调用返回的工具结果写着 "Session-only (not written to disk, dies when Qwen exits)"，因此在参数已被固定的情况下没有需要清理的持久化任务——那条
cron_delete提示确实是空操作。而在旧的真实模型版本里，durable是由模型自己决定的，所以正是"固定参数"这一点让这次删除变得安全。 - 变更文件上
eslint --max-warnings 0与prettier --check均干净。
5. 小建议（不阻塞，最多作为后续跟进）—— 基于索引的分发
对 fake server 加上埋点后可以看到每次运行有 6 次模型调用；只有 i=0 是用户提示，i=1/i=2/i=4 都是内部调用（工具调用分类、工具结果后的一轮、建议模式）——与新增注释描述的完全一致。
requests[0] 的快速失败守卫确实有效：在 Part 1 之前插入一次额外的 session/prompt 后，测试在 1.2 秒内以 requestIndex 0 was not the cron_create prompt — dispatch shifted 失败，而不是给出一个 75 秒的模糊超时。
但请注意 i=1——分类那一轮会引用 cron_create 的参数，所以它的载荷里同样包含 CRONFIRE7742。将来若有内部调用排到首个提示之前，并且其中引用了提示词或工具参数（标题生成是最典型的候选），就会同时满足这个守卫、又确实发生了分发错位，于是又退回到守卫本想避免的那种模糊超时。改为按消息内容匹配可以整体消除这一类问题——我在同样插入额外一轮的情况下验证过，可以通过：
const fakeServer = await startFakeOpenAIServer(({ body }) => {
const msgs = (body['messages'] ?? []) as Array<Record<string, unknown>>;
const lastText = JSON.stringify(msgs[msgs.length - 1]?.['content'] ?? '');
if (lastText.includes('Call cron_create with cron expression')) {
return { toolCalls: [fakeToolCall('cron_create', { /* … */ })] };
}
return { content: 'Done.' };
});不值得为此再走一轮——当前写法在今天是正确的，并且对常见的错位会明确报错。
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
|
@qwen-code /triage |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Review triage — no changes this roundPR #8082 (issue #8076) received one issue-level comment from maintainer @wenshao: a thorough local verification report (determinism 8/8, three negative-control mutations all caught, Feedback classification
§5 nit — index-keyed dispatch → DeclineThe suggestion is to key the fake server's response dispatch on message content ( Declined because:
If this becomes a real problem, it is a straightforward follow-up change. 中文说明评审分类 — 本轮无变更PR #8082（issue #8076）收到维护者 @wenshao 的一条 issue 级评论：一份详尽的本地验证报告（确定性 8/8、三个反向变异均被捕获、 反馈分类
§5 小建议 — 基于索引的分发 → 拒绝该建议将 fake server 的响应分发从按请求索引（ 拒绝理由：
如果这成为实际问题，是一个简单的后续跟进变更。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
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. ✅
|
Released in v0.21.3. |
4 participants
What this PR does
Migrates the
acp-cronintegration test from real model API calls to deterministicfake-openai-serverscripted responses. The test now controls exactly what the model returns at each step (acron_createtool call, confirmation text, interactive response, cron-fired response, and cleanup), eliminating model output variance as a failure source. Also adds per-agentQWEN_HOMEisolation to prevent concurrent test interference with auth/model config.Why it's needed
The test failed on main CI (run 30506000682) because it depended on real model inference to correctly call
cron_createwith specific parameters and return specific content. Real-model tests are inherently non-deterministic: the model may not call the right tool, may use wrong parameters, or may respond slowly enough to exceed timeouts. This is the same class of flakiness addressed in #7934, which migrated 39 test cases tofake-openai-server. The cron scheduler still uses real minute-boundary timing, so the test continues to validate the actual cron fire → sessionUpdate → streaming pipeline.Reviewer Test Plan
How to verify
Run the integration test locally and confirm it passes deterministically:
Expected: 1 test passed in ~55-65 seconds (the wait is the real cron minute-boundary timing).
Evidence (Before & After)
N/A — test-only change, no user-visible behavior.
Tested on
Environment (optional)
Integration test with
fake-openai-server; no real model calls.Risk & Scope
Linked Issues
Fixes #8076
中文说明
本 PR 做了什么
将
acp-cron集成测试从真实模型 API 调用迁移为确定性的fake-openai-server脚本化响应。测试现在精确控制模型在每一步返回的内容（cron_create工具调用、确认文本、交互式响应、cron 触发响应和清理），消除了模型输出方差作为失败源。同时添加了每个 agent 独立的QWEN_HOME隔离，防止并发测试在认证/模型配置上的干扰。为什么需要
该测试在主分支 CI（run 30506000682）上失败，因为它依赖真实模型推理来正确调用
cron_create并传入特定参数、返回特定内容。真实模型测试本质上是不确定的：模型可能不调用正确的工具、使用错误的参数，或响应过慢导致超时。这与 #7934 中解决的不稳定性属于同一类别，该 PR 将 39 个测试用例迁移到了fake-openai-server。cron 调度器仍然使用真实的分钟边界计时，因此测试继续验证实际的 cron 触发 → sessionUpdate → 流式传输管道。审阅者测试计划
如何验证
在本地运行集成测试并确认其确定性通过：
预期：1 个测试在约 55-65 秒内通过（等待时间是真实的 cron 分钟边界计时）。
证据（修改前后）
N/A — 仅测试变更，无用户可见行为。
测试平台
环境（可选）
使用
fake-openai-server的集成测试；无真实模型调用。风险与范围
关联 Issue
修复 #8076