fix(cron): add deterministic test seam for cron-interactive E2E - #6987
Merged
Conversation
Add forceFireJob(id) method and QWEN_CODE_TEST_CRON_FAST env var to CronScheduler. When enabled, newly created session-only jobs auto-fire after 5s (configurable via QWEN_CODE_TEST_CRON_DELAY_MS) instead of waiting up to 60s for the wall-clock minute boundary. This removes the timing-flakiness from cron-interactive.test.ts without changing any production behavior (seam is env-gated and inactive by default). Refs #6982
Enable the CronScheduler test seam in cron-interactive tests via QWEN_CODE_TEST_CRON_FAST=1 in makeEnv(). This makes newly created cron jobs auto-fire after 5s instead of waiting for the wall-clock minute boundary. Reduce all waitForScreen timeouts from 90s to 30s since the fire is now deterministic (~5s after job creation + model round-trip). Refs #6982
qwen-code-ci-bot
requested changes
Jul 15, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the PR, @yiliang114! The change itself looks well-scoped — a clean test seam for cron-interactive flakiness with zero production impact. But the PR body is missing several required headings from the PR template:
## What this PR does(you have## Summaryinstead — close, but not the template heading)## Why it's needed(the motivation is spread across sections but has no dedicated heading)## Reviewer Test Plan— this is the most important miss. Reviewers need a### How to verifysection with steps, and a### Tested ontable so we know which platforms you validated.## Risk & Scope## Linked Issues(you have## Related— same idea, different heading)
Could you restructure the body to match the template? The content is already good — it just needs to land under the right headings so reviewers can scan it efficiently. In particular, a ### How to verify with something like "run npm run test:integration:interactive:sandbox:none and confirm cron-interactive tests pass reliably across multiple runs" would save the reviewer time.
中文说明
感谢 PR，@yiliang114！改动本身范围合理——为 cron-interactive 测试添加了干净的测试接缝，对生产环境零影响。但 PR body 缺少 PR 模板 中的几个必填标题：
## What this PR does（你用了## Summary，接近但不是模板标题）## Why it's needed（动机分散在各节中，没有专门的标题）## Reviewer Test Plan——这是最重要的缺失。审查者需要### How to verify部分来说明验证步骤，以及### Tested on表格来标明你验证了哪些平台。## Risk & Scope## Linked Issues（你用了## Related，意思一样但标题不同）
请按照模板重新组织 body。内容本身已经很好，只需要放到正确的标题下，方便审查者快速浏览。特别是加一个 ### How to verify，比如写"运行 npm run test:integration:interactive:sandbox:none 并确认 cron-interactive 测试多次运行均稳定通过"，可以节省审查者的时间。
— Qwen Code · qwen3.7-max
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. |
wenshao
approved these changes
Jul 15, 2026
Collaborator
|
@qwen-code /triage |
Collaborator
|
Thanks for the PR, @yiliang114! Template looks good ✓ — all required headings present ( Problem: real, well-documented flakiness in Direction: aligned — fixing test flakiness at the root by making fires deterministic is the right call. Env-gated test seam with zero production impact is a clean pattern. Size: touches Approach: minimal and well-scoped. Moving on to code review. 🔍 中文说明感谢 PR，@yiliang114！ 模板完整 ✓ ——所有必填标题均已包含。之前 review 轮次的模板问题已修复。 问题：真实存在、有充分记录的 方向：对齐——通过使触发确定性化来从根源修复测试 flaky 是正确做法。env 门控的测试钩子对生产零影响，是一个干净的模式。 规模：触及 方案：最小且范围合理。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal: To fix cron-interactive timing flakiness, I'd add a method to bypass the wall-clock check and fire a job immediately, gate it behind an env var, and have the test file set that var with shorter timeouts. Cleanup of any test-only state on Comparison: The PR's approach matches this exactly — No correctness bugs, security issues, or regressions found. Specifically:
Real-Scenario TestingBuild & typecheck: ✅ both clean. Unit tests: 123/123 cronScheduler tests pass. 49/49 related tool tests pass (cron-create, cron-delete, cron-list, loop-wakeup). Test seam verification (direct import of the built module):
The full E2E integration tests ( 中文说明代码审查独立方案： 为修复 cron-interactive 的 timing flaky，我会添加一个绕过墙钟检查立即触发 job 的方法，用环境变量门控，并让测试文件设置该变量并缩短超时。测试状态的清理在 对比： PR 的方案与此完全一致—— 未发现正确性 bug、安全问题或回归。 真实场景测试构建和类型检查： ✅ 均通过。 单元测试： 123/123 cronScheduler 测试通过。49/49 相关工具测试通过。 测试钩子验证： 4 项验证全部通过——无 env 时零影响、env 设置后自动触发、forceFireJob 直接调用正常、stop() 正确清理 timer。 完整的 E2E 集成测试需要 LLM 后端和 node-pty，当前 CI 环境不可用。PR 上的 CI 将在三个平台上验证。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — Clean, minimal fix for a well-documented flaky test. Every change earns its place. This is a textbook example of a good fix: real problem with months of history, minimal code surface, env-gated seam with zero production impact, thorough cleanup paths. The The only thing I'd note for awareness (not blocking): Approving. ✅ 中文说明置信度：5/5 — 干净、最小化的修复，解决了有充分记录的 flaky 测试。每处改动都有必要。 这是一个教科书式的好修复：真实问题有数月的历史记录，最小的代码变更面，env 门控的钩子对生产零影响，清理路径完善。 唯一值得注意的（不阻塞）： 批准。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 15, 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. ✅
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
Add a deterministic test seam to
CronSchedulerso cron-interactive E2E tests no longer depend on wall-clock minute boundaries. A newforceFireJob(id)method fires a job immediately, and aQWEN_CODE_TEST_CRON_FAST=1environment variable triggers auto-fire 5s after job creation. The test file is updated to use this seam with reduced timeouts (90s → 30s).Why it's needed
cron-interactive.test.tshas been intermittently flaky for ~3.5 months because it waits up to 60s for a real wall-clock cron fire plus model round-trip, within a 90s budget. This is the root fix for #6982. Without it, even with quarantine (#6986), nightly runs still hit the same timing race. The test seam makes fires deterministic without changing any production scheduler logic.Reviewer Test Plan
How to verify
forceFireJob(id)is additive — the normaltick()path is untouched, existing behavior preserved.QWEN_CODE_TEST_CRON_FASTseam is env-gated and inactive by default (no production impact).stop()anddestroy()— no leaked setTimeout handles.QWEN_CODE_TEST_CRON_FAST=1and verify the cron notification appears within ~10s instead of up to 60s.Evidence (Before & After)
Before:
cron_createwith*/1 * * * *→ wait up to 60s for wall-clock minute boundary → occasional 90s timeout on slow runners.After: with
QWEN_CODE_TEST_CRON_FAST=1→ auto-fire at ~5s → cron notification appears within ~10s total (5s delay + model round-trip).Tested on
Not locally tested (CI-only environment available). CI run on this PR will validate across all three platforms.
Environment (optional)
Node.js v22.x, sandbox:none.
Risk & Scope
CronScheduler.create(), but it is strictly gated behindQWEN_CODE_TEST_CRON_FAST=1and has zero impact when unset. TheforceFireJobmethod is additive and doesn't modify any existing fire logic.!job.durablecheck). Full CronScheduler refactor with injectable clock is a larger effort not covered here.forceFireJobis a new public method, no existing API changed.Linked Issues
Closes #6982
Refs: #6986, #2731, #3402, #3992, #6016
中文说明
这个 PR 做了什么
给
CronScheduler添加确定性测试钩子，使 cron-interactive E2E 测试不再依赖墙钟整分钟边界。新增forceFireJob(id)方法立即触发 job，以及QWEN_CODE_TEST_CRON_FAST=1环境变量在 job 创建后 5 秒自动触发。测试文件更新为使用该钩子并降低超时（90s → 30s）。为什么需要
cron-interactive.test.ts已经间歇性 flaky 约 3.5 个月，因为它需要等待最多 60 秒的真实墙钟 cron 触发加上模型往返，在 90 秒预算内。这是 #6982 的根改法。没有它，即使有隔离（#6986），nightly 运行仍然会遇到同样的 timing race。测试钩子使触发确定性化，不改变任何生产调度器逻辑。风险与范围
CronScheduler.create()中的新代码路径，但严格门控在QWEN_CODE_TEST_CRON_FAST=1后面，不设置时零影响。forceFireJob是增量方法，不修改任何现有触发逻辑。!job.durable检查保护）。带可注入时钟的完整 CronScheduler 重构是更大的工作，不在此 PR 范围内。forceFireJob是新公开方法，没有现有 API 变更。