fix(core): prevent cron scheduler from firing on creation minute#4946
Conversation
The CronScheduler fired jobs immediately when the current minute matched the cron expression, because lastFiredAt was undefined and the double-fire guard was bypassed. Combined with /loop's intentional immediate execution (SKILL.md step 3), this caused duplicate execution of the prompt. Initialize lastFiredAt to the creation minute-start so the scheduler treats it as already-fired and waits for the next matching cron window.
E2E Test ReportSetuptmux interactive session, macOS, built from this branch. Test:
|
|
Thanks for the PR! Template looks good ✓ On direction: this is a clear-cut correctness bug — the cron scheduler double-fires on the creation minute because On approach: this is about as minimal as a fix gets — one line seeds Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这是一个明确的正确性 bug——cron 调度器在任务创建分钟内会重复触发,因为 方案:这是最简化的修复方式——一行代码将 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. The fix is correct and minimal — lastFiredAt is seeded to the creation minute boundary so the existing double-fire guard prevents same-minute execution. Test coverage is solid. — qwen3.7-plus via Qwen Code /review
Code ReviewThe fix is elegant and minimal — exactly what I'd propose independently. One line seeds Correctness verified:
No blockers. Code follows project conventions (TypeScript strict, ESM, collocated tests). Unit TestsAll 27 tests in Real-Scenario TestingTested with the PR bundle ( Analysis: The job was created at ~08:27:50 (minute 27). The agent executed the intentional first WriteFile immediately. The first A second Note: I couldn't reproduce the exact "before" bug because the installed SummaryThe fix works as designed. The scheduler no longer fires in the creation minute, preventing the double-execution bug described in the PR. The implementation is minimal, well-tested, and follows project conventions. 中文说明代码审查修复优雅且最小化——正是我会独立提出的方案。一行代码将 正确性验证:
无阻塞问题。代码遵循项目规范(TypeScript strict、ESM、测试与源码同目录)。 单元测试
真实场景测试使用 PR 构建包测试( 任务创建于 ~08:27:50(第 27 分钟)。Agent 立即执行了预期的首次 WriteFile。第一个 第二个 注意: 无法完全复现"修复前"的 bug,因为已安装的 总结修复按设计工作。调度器不再在创建分钟内触发,防止了 PR 描述的双重执行 bug。实现最小化、测试充分、遵循项目规范。 — Qwen Code · qwen3.7-max |
|
This is a clean bug fix that does exactly what it says — seeds The unit test definitively proves the fix (job created at 10:30:15, tick at 10:30:59 → zero fires, tick at 10:31:59 → one fire). The tmux test confirms it works in the real product — the scheduler fires starting from the next minute, not the creation minute. No concerns. Ships the fix cleanly. ✅ 中文说明这是一个干净的 bug 修复,完全按描述工作——在创建时初始化 单元测试明确证明了修复(任务创建于 10:30:15,在 10:30:59 tick → 零次触发,在 10:31:59 tick → 一次触发)。tmux 测试确认在真实产品中有效——调度器从下一分钟开始触发,而非创建分钟。 无顾虑。干净地交付了修复。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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. |
Runtime verification — cron creation-minute double-fire fix (head
|
What this PR does
Initializes
lastFiredAtto the creation minute-start when a cron job is created, so the scheduler's double-fire guard treats the creation minute as "already fired" and waits for the next matching cron window.Why it's needed
When
/loopschedules a cron job, the SKILL.md instructs the agent to execute the prompt immediately (step 3, by design). However, theCronScheduler.tick()could also fire the same job within the same minute —lastFiredAtwasundefinedon creation, so the guard check (lastFiredAt === matchedMinuteMs) was bypassed whenever the current minute happened to match the cron expression. This caused duplicate execution of the prompt: once by the agent (intentional) and once by the scheduler (bug).For example,
/loop 2m check the buildwould write the file immediately, then the scheduler would fire● Cron: check the buildseconds later and write it again.The fix seeds
lastFiredAtto the floor of the creation minute so the existing double-fire guard handles it naturally.Reviewer Test Plan
How to verify
npm run build && npm run bundleQWEN_CODE_ENABLE_CRON=1 node dist/cli.js --approval-mode yolo/loop 2m write the word TEST to /tmp/looptest.txt● Cron:triggered duplicateEvidence (Before & After)
Before — scheduler fires immediately after the agent's execution:
After — only the intentional execution, prompt returns to idle:
Tested on
Environment (optional)
E2E verified via tmux interactive session with
QWEN_CODE_ENABLE_CRON=1 node dist/cli.js --approval-mode yolo. Unit tests vianpx vitest run.Risk & Scope
lastFiredAtwas previously unused at creation time; the change piggybacks on the existing double-fire guard./loop) also benefit — they no longer fire on the creation minute either.中文说明
这个 PR 做了什么
在创建 cron 任务时,将
lastFiredAt初始化为创建时间所在分钟的起始时间戳,使调度器的防重复触发机制将创建分钟视为"已触发",从而等待下一个匹配的 cron 窗口。为什么需要这个改动
使用
/loop调度 cron 任务时,SKILL.md 指示 agent 立即执行提示词(步骤 3,设计如此)。但CronScheduler.tick()也会在同一分钟内触发同一任务——因为创建时lastFiredAt为undefined,防重复触发检查(lastFiredAt === matchedMinuteMs)被绕过。这导致提示词被重复执行:agent 执行一次(预期行为),调度器又执行一次(bug)。风险与范围
lastFiredAt在创建时此前未使用;此改动利用了现有的防重复触发机制。/loop)也受益——不再在创建分钟触发。