@qwen-code /triage

@qwen-code /triage

Thanks for the PR!

Template looks good ✓

On direction: clearly aligned. Cron is a first-class feature (CHANGELOG: "durable cron jobs" #5004, "enable loop/cron tools by default" #4950), and silent normalization of malformed input is a real correctness bug — users can end up with jobs running at unintended cadences. The linked issue #5348 lays it out well.

On approach: scope is tight and appropriate. Four files, +59/-12, all directly related to the fix. The whole-token regex validation before parseInt() is the right minimal fix. No scope creep, no drive-by refactors.

Moving on to code review. 🔍

— Qwen Code · qwen3.7-max

Code Review

The fix is clean and correct. Both cronParser.ts and cronDisplay.ts adopt the same pattern — a ^\d+$ regex check before parseInt() — which is exactly the minimal change needed. The parser correctly distinguishes error types: Invalid value for bad single tokens, Invalid range for malformed/multi-segment ranges, Invalid step for bad step values.

One minor observation: INTEGER_TOKEN_RE and the positive-integer parsing helper are duplicated across cronParser.ts and cronDisplay.ts. Not a blocker — the two modules are independent and the duplication is a few lines — but worth flagging for the contributor to consider.

No correctness bugs, no security issues, no regressions. Scope is tight.

Testing

Before (main, without PR)

Bug confirmed — malformed inputs silently normalized:

$ npx tsx -e "import { parseCron } from './packages/core/src/utils/cronParser'; ..."
=== Before (main, no PR) ===
BUG CONFIRMED: 5x * * * * accepted (silently normalized)
BUG CONFIRMED: 1-2-3 accepted (silently normalized)
BUG CONFIRMED: */15garbage accepted (silently normalized)

After (PR #5352 applied)

All malformed inputs rejected, valid inputs still work:

$ npx tsx -e "import { parseCron } from './packages/core/src/utils/cronParser'; import { humanReadableCron } from './packages/core/src/utils/cronDisplay'; ..."
=== After (PR #5352 applied) ===
FIXED: 5x rejected - Invalid value: "5x"
FIXED: 1-2-3 rejected - Invalid range: "1-2-3"
FIXED: */15garbage rejected - Invalid step: "15garbage"
VALID: */15 accepted ✓
VALID: 1-5 accepted ✓

Display: */15x => */15x * * * *
Display: */15  => Every 15 minutes

Unit Tests (27/27 pass)

$ cd packages/core && npx vitest run src/utils/cronParser.test.ts src/utils/cronDisplay.test.ts

 RUN  v3.2.4 /home/runner/work/qwen-code/qwen-code/packages/core

 ✓ src/utils/cronDisplay.test.ts (2 tests) 5ms
 ✓ src/utils/cronParser.test.ts (25 tests) 18ms

 Test Files  2 passed (2)
      Tests  27 passed (27)
   Duration  4.29s

Results match the PR's claims exactly. ✅

— Qwen Code · qwen3.7-max

This is a textbook bugfix PR: well-scoped, correctly implemented, with regression tests that actually cover the failure modes.

My independent proposal would have been the same — add a ^\d+$ regex guard before every parseInt() call in the cron field parser. The PR goes slightly further by also fixing humanReadableCron(), which had the identical bug on the display side. That's the right call — fix both paths in one PR rather than leaving a known-good-but-actually-broken display function.

The before/after testing is unambiguous: on main, parseCron('5x * * * *') silently succeeds (the bug). With the PR, it throws Invalid value: "5x". Valid expressions like */15 and 1-5 continue to work. All 27 unit tests pass. The results match the PR's claims exactly.

The minor duplication of INTEGER_TOKEN_RE across the two files is not worth extracting — they're in independent modules and the shared logic is three lines. If a future change needs to update the validation, the duplication will be obvious.

Approving. ✅

— Qwen Code · qwen3.7-max

✅ Local runtime verification (real tmux, real CronCreateTool) — PR #5352

Verdict: fixes #5348 as intended, no regressions, type-checks & lints clean — safe to merge.

I drove the real cron functions and the real CronCreateTool + CronScheduler against the malformed inputs from #5348, inside an isolated tmux session. A/B baseline is origin/main (1 commit behind; the two changed files are byte-identical to the merge-base → clean apply).

What the PR does

parseCron() used bare parseInt(), which silently drops trailing junk, so malformed fields were normalized instead of rejected. The PR adds a whole-token /^\d+$/ check before conversion for single values (Invalid value), range endpoints / over-segmented ranges (Invalid range), and steps (Invalid step). humanReadableCron() gets the same check via parsePositiveInteger, so malformed step patterns fall back to the raw string instead of being summarized.

Evidence — PR head

Static checks

vitest cronParser.test.ts + cronDisplay.test.ts → 27 passed   VITEST_EXIT=0
eslint (all 4 changed files)                    → ESLINT_EXIT=0
typecheck core (tsc --noEmit)                   → TYPECHECK_CORE_EXIT=0

Real runtime — parseCron rejects every malformed token:

5x * * * *          THROW Invalid value: "5x"
1-5x * * * *        THROW Invalid range: "1-5x"
1-2-3 * * * *       THROW Invalid range: "1-2-3"
*/15garbage * * * * THROW Invalid step:  "15garbage"
1-10/3x * * * *     THROW Invalid step:  "3x"
5/2x * * * *        THROW Invalid step:  "2x"
valid controls (5, */15, 0 9 * * 1-5, 1-5) → all OK (no regression)

humanReadableCron falls back to the raw string for malformed steps
(*/15 * * * * still → Every 15 minutes; */15x, */0, 0 */2x, 0 0 */3x → raw).

End-to-end through the real CronCreateTool (+ real CronScheduler):

cron_create("5x * * * *")  → jobs_created=0  error="Invalid value: \"5x\""   (rejected)
cron_create("*/5 * * * *") → jobs_created=1  "Scheduled <id> (Every 5 minutes)"  (valid)

A/B vs origin/main — the bug, demonstrated

                                  PR head                    origin/main (base)
parseCron("5x * * * *")           THROW Invalid value        PARSED (silently accepted)
nextFireTime("5x * * * *")        THROW                      2026-06-19T12:05:00Z
nextFireTime("5  * * * *")        2026-06-19T12:05:00Z       2026-06-19T12:05:00Z
  → malformed fires like "5"?     n/a (rejected)             YES  (identical fire time)
humanReadableCron("*/15x …")      "*/15x * * * *" (raw)      "Every 15 minutes"  (wrong)
humanReadableCron("*/0 …")        "*/0 * * * *" (raw)        "Every 0 minutes"   (nonsensical)
CronCreateTool("5x * * * *")      0 jobs + error             1 job: "Scheduled <id> (5x * * * *)"

The headline: on base, cron_create("5x * * * *") silently creates a job that fires at minute 5 — exactly the "unintended cadence" risk #5348 describes. On head it fails fast.

The new tests have teeth — both fail on base:

× parseCron     > throws on malformed numeric tokens   → expected [Function] to throw an error
× humanReadableCron > falls back for malformed step …  → expected 'Every 15 minutes' to be '*/15x * * * *'
Tests 2 failed | 25 skipped (27)   TEETH_VITEST_EXIT=1

Notes

• Scoped purely to numeric-field validation; well-formed expressions are unaffected (valid controls all still parse and cron_create still schedules them).
• */0 now also falls back in display (parsePositiveInteger requires > 0), matching parseCron's long-standing Invalid step rejection of zero steps — consistent.
• Verified on Linux.

vitest cronParser.test.ts + cronDisplay.test.ts → 27 passed   VITEST_EXIT=0
eslint（4 个改动文件）                          → ESLINT_EXIT=0
typecheck core (tsc --noEmit)                   → TYPECHECK_CORE_EXIT=0

5x * * * *          THROW Invalid value: "5x"
1-5x * * * *        THROW Invalid range: "1-5x"
1-2-3 * * * *       THROW Invalid range: "1-2-3"
*/15garbage * * * * THROW Invalid step:  "15garbage"
1-10/3x * * * *     THROW Invalid step:  "3x"
5/2x * * * *        THROW Invalid step:  "2x"
合法对照（5、*/15、0 9 * * 1-5、1-5）→ 全部 OK（无回归）

cron_create("5x * * * *")  → jobs_created=0  error="Invalid value: \"5x\""   （被拒绝）
cron_create("*/5 * * * *") → jobs_created=1  "Scheduled <id> (Every 5 minutes)"  （合法）

                                  PR head                    origin/main (base)
parseCron("5x * * * *")           THROW Invalid value        PARSED（被静默接受）
nextFireTime("5x * * * *")        THROW                      2026-06-19T12:05:00Z
nextFireTime("5  * * * *")        2026-06-19T12:05:00Z       2026-06-19T12:05:00Z
  → 非法表达式是否像 "5" 一样触发？  不适用（已拒绝）          是（触发时间完全相同）
humanReadableCron("*/15x …")      "*/15x * * * *"（原样）     "Every 15 minutes"（错误）
humanReadableCron("*/0 …")        "*/0 * * * *"（原样）       "Every 0 minutes"（无意义）
CronCreateTool("5x * * * *")      0 个任务 + 报错             1 个任务："Scheduled <id> (5x * * * *)"

× parseCron     > throws on malformed numeric tokens   → expected [Function] to throw an error
× humanReadableCron > falls back for malformed step …  → expected 'Every 15 minutes' to be '*/15x * * * *'
Tests 2 failed | 25 skipped (27)   TEETH_VITEST_EXIT=1