fix(cron): keep multi-time cron expressions in cron mode in the job editor (#8469)

[CrysisDeu]

Summary

The cron job detail editor classified any 5-field cron expression with a day-of-week set and dom/month * as a single-time Weekly schedule, without checking that the minute and hour fields are plain integers. A multi-time expression like 0 9,12,15 * * 1-5 opened in Weekly mode showing only 09:00 (parseInt('9,12,15') truncates to 9), and saving from that editor — even to change an unrelated field — re-serialised the schedule from the single time field, silently collapsing the job to one daily run.

This is the "preserve the underlying expression" option from the issue: the weekly detector in parseJobDefaults now requires

• minute and hour to be plain in-range integers (/^\d{1,2}$/, ≤59 / ≤23 — same plain-field grammar as cronClock in cronUtils, so the list view and editor classify the same job the same way), and
• every comma segment of the day-of-week field to be individually representable by expandDow, with numeric tokens in cron's 0–7 range (expandDow('1,3-5/2') silently drops the stepped segment; parseDowToken wraps 8 to Monday via % 7 — both were residual silent-rewrite paths).

Anything else falls through to Cron expression mode, where buildBody round-trips the raw expression verbatim (body.cron = expr). No new UI; genuine weekly expressions (30 9 * * 1,3, 0 13 * * MON-FRI) parse exactly as before.

Closes #8469

Before / after

A job scheduled 0 9,12,15 * * 1-5 (9:00, 12:00, 15:00 on weekdays) opened in the editor:

Before — Weekly mode, only 09:00 survives a save	After — Cron mode, expression preserved

Tests

New website/src/test/JobForm.multiTime.test.ts via the exported parseJobDefaults/buildBody:

• 0 9,12,15 * * 1-5, */15 9 * * 1-5, 0 9-11 * * 1 → cron mode, raw expression preserved
• 0 9 * * 1,3-5/2 (mixed representable + stepped dow segment), 0 9 * * 8 (out-of-range dow), 0 007 * * 1 (zero-padded hour), 60 9 * * 1 / 0 25 * * 1 (out-of-range minute/hour) → cron mode
• 30 9 * * 1,3 and 0 13 * * MON-FRI still parse as weekly (regression guards)
• buildBody on parsed multi-time defaults emits body.cron identical to the original expression

Verification

• npx tsc -b and eslint clean on the changed files; vitest runs in CI
• Two pre-push model-pinned review lanes run: GPT lane blocked on the mixed-dow-segment gap (fixed, per-segment validation); Opus lane passed with two Low advisories (plain-field regex parity with cronClock, dow 0–7 range check) — both applied
• Before/after screenshots captured from an isolated pod running this branch vs main

Pattern harvest

Rule candidate: a form that opens a stored value in a simplified editing mode must first verify the mode can round-trip the value losslessly (parse -> re-serialise -> compare, or a strict representability grammar); any value the mode cannot faithfully represent must open in the raw/advanced mode instead. The defect class — parseInt/lossy parse selecting the simplified mode, then save re-serialising from the truncated view — also existed here for minute, hour, and day-of-week independently, so the check belongs on every field the simplified mode collapses.

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of bd9ee540c6feb7fff48b412668c9d7c5e6770d21 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

The editor now shows the true schedule (raw cron) instead of a weekly widget that silently misrepresented multi-time jobs and destroyed run times on save.

[UX-REVIEWED] bd9ee54

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of bd9ee540c6feb7fff48b412668c9d7c5e6770d21 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Tightening the weekly-mode detector to what weekly mode can actually represent fixes the root cause — silent schedule rewrites — with zero new surface.

Suggestions

• isPlainField/isRepresentableDow restate cronClock's grammar by mirror; hoisting a shared classifier into cronUtils would make list-view/editor drift impossible instead of comment-enforced.

[DESIGN-REVIEWED] bd9ee54

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of bd9ee540c6feb7fff48b412668c9d7c5e6770d21 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] bd9ee54

False positive or not applicable? A repository writer can comment:
/ai-review override gpt bd9ee540c6feb7fff48b412668c9d7c5e6770d21: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed bd9ee540c6feb7fff48b412668c9d7c5e6770d21 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] bd9ee54

Verdict parsed from the review's SHA-scoped output markers for commit bd9ee540c6feb7fff48b412668c9d7c5e6770d21.

False positive or not applicable? A repository writer can comment:
/ai-review override fable bd9ee540c6feb7fff48b412668c9d7c5e6770d21: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of bd9ee540c6feb7fff48b412668c9d7c5e6770d21 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks complete. The fix is cause-level for #8469 (classifier only admits what Weekly mode can round-trip), exports pre-exist, screenshots follow the documented temp-screenshots/ convention, and I found exactly one unfixed sibling of the root cause in the same function: the interval path rounds every_secs=5400 (90 min) to 2 hours and re-serialises it on save — the same silent-rewrite class, different schedule mode.

First-Principles-Verdict: CONCERNS

Cause-level and complete for cron expressions, but the same lossy-reparse-then-save cause has one counted unfixed sibling: interval rounding in the same function.

What this change ships

Intent: stop the job editor from silently collapsing a multi-time cron schedule to one daily run on save (#8469) — a FIX.

1. Multi-time/step/range minute-hour expressions now open in Cron mode, preserved verbatim on save — justified
2. Day-of-week fields with stepped or out-of-range segments now open in Cron mode instead of collapsing/wrapping — justified
3. Out-of-range or zero-padded minute/hour now opens in Cron mode instead of fabricating a time — justified
4. New regression tests for the classifier and round-trip — justified
5. Before/after screenshots committed under temp-screenshots/cron-multi-time/ — justified (documented convention, temp-screenshots/README.md)

Watch

• Same root cause, one unfixed sibling in the same function: parseJobDefaults rounds interval jobs into a lossy unit (website/src/components/JobForm.tsx:61-62 — every_secs: 5400 parses as intUnit: 'hours', intVal: Math.round(1.5) = 2), and buildBody re-serialises it (JobForm.tsx:124), so saving an unrelated field silently rewrites a 90-minute job to 2 hours — exactly the Cron schedule editor drops multi-time cron expressions on save (only first time survives) #8469 shape. Grepped every_secs consumers: this is the only lossy re-serialising path. Interval has no verbatim fallback mode, so the fix differs (pick the largest unit that divides secs evenly) — accepted-and-deferred, not a demand on this PR.

No duplication finding: the /^\d{1,2}$/ plain-field grammar has 4 pre-existing inline spellings in cronUtils.tsx (lines 53, 63, 171, 212), none exported; the new one deliberately mirrors cronClock and follows the repo idiom.

[FIRST-PRINCIPLES-REVIEWED] bd9ee54

[CrysisDeu]

Disposition — First Principles CONCERNS (accepted, deferred with a tracked home):

The Watch finding is correct: interval parsing in the same function is a lossy-reparse-then-save sibling (every_secs: 5400 -> 2 hours). As the lane itself notes, interval mode has no verbatim fallback so the fix is structurally different (largest evenly-dividing unit), and folding it in here would widen a minimal, screenshot-evidenced cron-mode fix. Filed as issue #8647 with the repro and the suggested fix shape so it does not rely on this thread for survival.

[NicholasRBowers]

Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix with a clear root cause (#8469: parseInt silently truncated multi-time cron fields like '9,12,15' to 9 in the job editor's weekly-mode classifier, dropping run times on save) -- JobForm.tsx classifier tightening + new test file + 2 screenshots.

[chenmingwei23]

Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix keeps multi-time cron expressions in cron mode in the job editor (2 code/test files + 2 screenshots), clear root cause #8469 (parseInt truncated a comma/step field and dropped run times on save).

[dwu96]

Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: single-predicate fix in JobForm parseJobDefaults so a cron expression with a minute/hour list, range or step (or a dow field expandDow cannot fully represent) stays in cron mode and round-trips verbatim instead of parseInt silently truncating it and dropping run times on save; one source function plus a new regression test file.

[iamwhatever]

Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: JobForm's weekly-mode detection accepted list/range/step minute and hour fields, so parseInt truncated '9,12,15' to 9 and a save silently dropped the other run times (#8469); the fix requires plain minute/hour fields and a fully-representable day-of-week set before classifying a job as weekly, otherwise falling through to cron mode where the raw expression round-trips verbatim.