fix(cron): reject sub-millisecond durations by qingminglong · Pull Request #100311 · openclaw/openclaw

qingminglong · 2026-07-05T11:33:48Z

What Problem This Solves

Fixes an issue where users setting cron CLI duration flags could have positive sub-millisecond values silently treated as 0ms when the value was below one millisecond.

Why This Change Was Made

The cron duration parser now rejects values whose millisecond result is non-finite or non-positive after unit conversion and flooring. This keeps the existing overflow guard while also rejecting tiny positive inputs that would otherwise become zero-duration metadata or cooldowns.

User Impact

Cron schedule and failure-alert duration inputs such as 0.5ms are rejected instead of being accepted as an unintended zero-millisecond value.

Evidence

Current head: ab50fb9f03580c368b93e11f26281719c8ebecc3.
Claim proved: positive sub-millisecond cron CLI durations are rejected through the real cron CLI paths while the existing duration parser behavior remains covered.
Changed files:
- src/cli/cron-cli/shared.ts
- src/cli/cron-cli/shared.test.ts
Commands / artifacts:
- git diff --check -> passed after merging current upstream/main (6237580e3cc40d1c05a832428feee048dd3feb44).

ode scripts/run-vitest.mjs src/cli/cron-cli/shared.test.ts -> passed, 1 test file / 30 tests.

node scripts/run-vitest.mjs src/scripts/test-projects.test.ts -> passed, 1 test file / 83 tests; this covers the stale checks-node-compact-small-whole-2 log assertion after the branch merge.
Real CLI proof with isolated temporary OPENCLAW_HOME, OPENCLAW_STATE_DIR, and OPENCLAW_CONFIG_PATH:
- node --import tsx src/entry.ts cron create --name subms-proof --every 0.5ms --system-event tick --json -> Error: Invalid --every. Use a duration like 10m, 1h, or 1d., exit code 1.
- node --import tsx src/entry.ts cron edit proof-job --failure-alert --failure-alert-cooldown 0.5ms --failure-alert-mode announce -> Error: Invalid --failure-alert-cooldown., exit code 1.
python .agents\skills\autoreview\scripts\autoreview --mode local -> clean on the earlier local proof pass.
GitHub PR checks on current head: rerunning after branch update; Real behavior proof, workflow sanity, dependency guard, labeler, and OpenGrep have already reported success, with CI/CodeQL still in progress.
Observed result:
- The real CLI rejects 0.5ms for both cron schedule creation and failure-alert cooldown before creating persistent user cron state.
- Focused Vitest passed: cron parser coverage, 1 test file / 30 tests; tooling route coverage, 1 test file / 83 tests.
- Autoreview clean: no accepted/actionable findings reported.
- PR is mergeable and maintainer edits are enabled.
Not tested / proof gaps:
- No live cron schedule was created; rejection happens before gateway RPC and before persistent cron state creation.
- One broad CI shard, checks-node-compact-small-whole-2, failed in src/scripts/test-projects.test.ts:857 because helper routing received test/scripts/package-git-fixture.test.ts. That failure is outside this PR's cron parser diff; this PR only changes src/cli/cron-cli/shared.ts and src/cli/cron-cli/shared.test.ts.

clawsweeper · 2026-07-05T11:57:55Z

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 2:09 PM ET / 18:09 UTC.

Summary
The PR updates the cron CLI duration parser to reject floored non-positive millisecond results and adds regression tests for sub-millisecond inputs.

PR surface: Source 0, Tests +2. Total +2 across 2 files.

Reproducibility: yes. at source level: current main's cron parser floors 0.5ms to 0, and schedule/failure-alert callers consume that numeric result. I did not execute a local CLI repro because this review is read-only; the PR body supplies after-fix CLI proof.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100306
Summary: This PR is the focused candidate fix for the canonical cron sub-millisecond duration parser issue; the broader batch PR overlaps but is not a safe replacement while it has its own blocker.

Members:

canonical: [Bug]: cron CLI accepts sub-millisecond positive durations as 0ms #100306 - The issue reports the exact parser behavior and this PR uses closing syntax for it.
partial_overlap: fix: land ten small reliability fixes #100399 - The batch PR includes the same cron fix among many unrelated fixes, but its current review has a separate blocking finding and maintainer-sensitive batch decisions.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

No repair lane is needed; the branch already contains the focused fix and tests and should proceed through normal maintainer review or be closed only after a replacement lands.

Security
Cleared: The diff only changes a cron CLI parser condition and colocated tests; it does not touch dependencies, workflows, secrets, package metadata, or code-execution setup.

Review details

Best possible solution:

Land this standalone parser guard with focused regression tests, or close it only if an equivalent fix lands first through the batch PR.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main's cron parser floors 0.5ms to 0, and schedule/failure-alert callers consume that numeric result. I did not execute a local CLI repro because this review is read-only; the PR body supplies after-fix CLI proof.

Is this the best way to solve the issue?

Yes. The shared cron parser guard is the narrowest maintainable fix because it preserves caller contracts and avoids duplicating zero-duration checks in each CLI option path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against c757675f348d.

Label changes

Label justifications:

P2: This is a normal-priority CLI parser bug fix with limited blast radius and focused regression coverage.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output showing real cron create and cron edit --failure-alert-cooldown commands reject 0.5ms.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output showing real cron create and cron edit --failure-alert-cooldown commands reject 0.5ms.

Evidence reviewed

PR surface:

Source 0, Tests +2. Total +2 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	2	2	0
Tests	1	2	0	+2
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	4	2	+2

What I checked:

Root AGENTS.md read: Root repository policy was read in full and applied; no scoped AGENTS.md exists under src/cli/cron-cli. (AGENTS.md:1, c757675f348d)
Current main parser behavior: Current main parses decimal duration strings, floors n * factor, and only rejects non-finite results, so 0.5ms can return numeric 0. (src/cli/cron-cli/shared.ts:205, c757675f348d)
Schedule caller path: --every uses the shared cron parser and rejects only falsy parse results, so the shared parser is the right boundary for interval schedule input. (src/cli/cron-cli/schedule-options.ts:171, c757675f348d)
Failure-alert caller path: --failure-alert-cooldown consumes the same parser and currently allows numeric 0, matching the linked report's second affected path. (src/cli/cron-cli/register.cron-edit.ts:572, c757675f348d)
PR implementation: At the PR head, the parser rejects result <= 0 after flooring, covering positive sub-millisecond values without duplicating checks in callers. (src/cli/cron-cli/shared.ts:230, 700adaeec884)
Regression tests: The PR adds 0.5ms and 0.001ms expectations alongside existing non-positive and malformed duration cases. (src/cli/cron-cli/shared.test.ts:348, 700adaeec884)

Likely related people:

Alix-007: Authored the recent cron parser overflow guard in the same parseDurationMs result-validation block that this PR extends. (role: recent area contributor; confidence: high; commits: 0b807321375f; files: src/cli/cron-cli/shared.ts, src/cli/cron-cli/shared.test.ts)
steipete: Path history shows repeated cron CLI schedule and duration-boundary work, and he is the current assignee on this PR. (role: recent area contributor; confidence: high; commits: 2209faef405f, e0248fc11f1a, 660a6dec7f39; files: src/cli/cron-cli/shared.ts, src/cli/cron-cli/schedule-options.ts, src/cli/cron-cli/register.cron-edit.ts)
anagnorisis2peripeteia: Recently changed the shared cron CLI and schedule option files while adding the on-exit schedule kind. (role: recent area contributor; confidence: medium; commits: 68bfa42b9bfc; files: src/cli/cron-cli/shared.ts, src/cli/cron-cli/schedule-options.ts)
0xbrak: Introduced the configurable cron failure-alert feature family that includes the affected cooldown path. (role: feature introducer; confidence: medium; commits: 4637b90c070f; files: src/cli/cron-cli/register.cron-edit.ts, packages/gateway-protocol/src/schema/cron.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (6 earlier review cycles)

reviewed 2026-07-05T12:00:51.228Z sha 32293c0 :: needs real behavior proof before merge. :: none
reviewed 2026-07-05T14:06:31.620Z sha 32293c0 :: needs maintainer review before merge. :: none
reviewed 2026-07-05T15:13:23.194Z sha ab50fb9 :: needs maintainer review before merge. :: none
reviewed 2026-07-05T16:51:23.643Z sha 5cf1cf8 :: needs maintainer review before merge. :: none
reviewed 2026-07-05T17:30:56.177Z sha db1d76d :: needs maintainer review before merge. :: none
reviewed 2026-07-05T17:47:48.824Z sha 3a993c4 :: needs maintainer review before merge. :: none

qingminglong · 2026-07-05T13:57:08Z

@clawsweeper re-review

clawsweeper · 2026-07-05T13:57:11Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

steipete · 2026-07-05T18:13:37Z

Thank you, @qingminglong. This exact fix and regression coverage landed on main in #100399 at b22c36f while this PR's squash was racing main.

I rechecked the current files: parseDurationMs now rejects finite values that floor to 0ms, and shared.test.ts covers both 0.5ms and 0.001ms. Closing as superseded by the landed equivalent rather than creating duplicate history.

steipete · 2026-07-05T18:13:37Z

Thank you — this fix landed through maintainer batch PR #100399 in commit b22c36f, with contributor credit and changelog thanks preserved. Closing this source PR as superseded.

steipete · 2026-07-05T18:17:53Z

Thanks @qingminglong — this fix and its tests are preserved as the contributor-authored first commit in maintainer takeover #100399, which also carries the changelog credit and broader Testbox proof.

Closing this source PR to avoid two competing branches landing the same cron parser change. #100306 remains open until #100399 actually merges; if that takeover is abandoned, this PR can be reopened.

openclaw-barnacle Bot added cli CLI command changes size: XS labels Jul 5, 2026

clawsweeper Bot mentioned this pull request Jul 5, 2026

[Bug]: cron CLI accepts sub-millisecond positive durations as 0ms #100306

Closed

steipete self-assigned this Jul 5, 2026

openclaw-barnacle Bot added docs Improvements or additions to documentation app: ios App: ios app: web-ui App: web-ui gateway Gateway runtime scripts Repository scripts agents Agent runtime and tooling extensions: qa-lab extensions: policy size: XL and removed size: XS labels Jul 5, 2026

steipete mentioned this pull request Jul 5, 2026

fix: land ten small reliability fixes #100383

Closed

steipete force-pushed the codex/100306-cron-subms-duration branch from 5cf1cf8 to db1d76d Compare July 5, 2026 17:23

openclaw-barnacle Bot removed docs Improvements or additions to documentation app: ios App: ios labels Jul 5, 2026

openclaw-barnacle Bot added size: XS app: web-ui App: web-ui scripts Repository scripts size: M and removed extensions: policy size: XL size: XS labels Jul 5, 2026

steipete force-pushed the codex/100306-cron-subms-duration branch from f19e87b to 3a993c4 Compare July 5, 2026 17:40

openclaw-barnacle Bot added size: XS and removed app: web-ui App: web-ui scripts Repository scripts size: M labels Jul 5, 2026

steipete mentioned this pull request Jul 5, 2026

fix: land ten small reliability fixes #100395

Closed

steipete mentioned this pull request Jul 5, 2026

fix: land ten small reliability fixes #100399

Merged

fix(cron): reject sub-millisecond durations

700adae

steipete force-pushed the codex/100306-cron-subms-duration branch from 3a993c4 to 700adae Compare July 5, 2026 17:57

steipete closed this Jul 5, 2026

steipete mentioned this pull request Jul 5, 2026

docs(changelog): record five landed fixes #100441

Merged

This was referenced Jul 7, 2026

[Bug]: Cron failure-alert cooldown can be saved as 0ms from a positive fractional duration #101770

Closed

fix: reject zero cron failure alert cooldowns #101804

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(cron): reject sub-millisecond durations#100311

fix(cron): reject sub-millisecond durations#100311
qingminglong wants to merge 1 commit into
openclaw:mainfrom
qingminglong:codex/100306-cron-subms-duration

qingminglong commented Jul 5, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

qingminglong commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

qingminglong commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qingminglong commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qingminglong commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading