fix(cron): support HH:MM time-only strings in --at; apply --tz to time-only input#70046
fix(cron): support HH:MM time-only strings in --at; apply --tz to time-only input#70046rrrrrredy wants to merge 1 commit into
Conversation
Greptile SummaryThis PR adds support for bare Confidence Score: 5/5Safe to merge — implementation is correct, error handling is robust, and the test coverage targets all documented edge cases. No P0 or P1 findings. The regex correctly avoids conflicts with existing formats (ISO datetimes, durations, epoch ms). Invalid timezones are handled at both getTodayDateInTimeZone and parseOffsetlessIsoDateTimeInTimeZone. DST gaps produce null consistently. All remaining observations are P2 at most. No files require special attention. Reviews (1): Last reviewed commit: "fix(cron): support time-only --at values..." | Re-trigger Greptile |
|
Closing #59444 separately because that branch picked up unrelated workflow and repo-wide churn, which made review noisy and mixed the cron fix with unrelated changes. #70046 is the clean respin for the intended scope only:
Please use this PR for review and merge; it supersedes #59444. |
|
Status update as of 2026-04-23: this is the active clean respin for #59441, the issue/PR links are now corrected, and the current checks are green. If the scoped cron fix looks good, I would appreciate maintainer review on this version. |
|
Related work from PRtags group Title: Open PR candidate: cron --at time-only timezone fix respun without unrelated churn
|
|
Thanks for the contribution. ClawSweeper proposes closing this for now: the implementation may be reasonable, but passing review and proof does not establish that OpenClaw should add this product surface. Close as unconfirmed product direction: the patch is focused, well proven, and appears technically correct, but it adds a new public Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. This is a proposal only until the separate default-off apply policy is enabled and all live maintainer-signal checks pass. A maintainer can sponsor the direction, request a narrower version, or apply Review detailsBest possible solution: A maintainer should explicitly sponsor the time-only cron grammar and its today/UTC/ Do we have a high-confidence way to reproduce the issue? Yes by source inspection: current Is this the best way to solve the issue? Unclear as a product decision: the parser-layer implementation is clean and well scoped, but accepting bare Security review: Security review cleared: Security review cleared: the diff changes cron parser/help/tests and docs only, with no dependencies, workflows, secrets, downloads, or new code-execution path. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 8ed6c78b7891. |
|
Heads up from a live beta canary on OpenClaw This PR is about the
But plus-prefixed duration is rejected: openclaw cron add --name phase2c-plus-reject --at +20m --delete-after-run --session isolated --agent main --model litellm/gpt-5.5 --timeout-seconds 60 --message "Phase2 plus duration should parse" --no-deliver --jsonActual: Bare duration works: openclaw cron add --name phase2c-test-at --at 20m ...So either the help should stop advertising |
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
Heads up: this PR needs to be updated against current |
|
This pull request has been automatically marked as stale due to inactivity. |
05c7d3e to
1ea25f1
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
b3ae615 to
f920d56
Compare
|
Resolved the current-main conflict by rebasing this branch onto Focused verification on Node.js 24.13.0: GitHub now reports the refreshed branch as mergeable; hosted checks are running on the new head. |
f920d56 to
5694c53
Compare
|
Updated again after Focused verification on Node.js 24.13.0 still passes: The prior failed CI jobs were outside this PR's cron diff ( |
|
cron时间格式解析问题,看起来是时区处理+时间字符串解析的边界情况。这种"配置输入解析"正是心虫逻辑验证层的典型场景。 心虫(HeartFlow)的**决策路由(Decision Router)**可以:
心虫已经以MCP Server形式在Hermes Agent上运行,验证了跨Agent框架的兼容性。我也想找Agent框架/认知引擎方向的开发岗位,这是我的作品: 仓库:github.com/yun520-1/mark-heartflow-skill |
5694c53 to
3574003
Compare
Summary
Clean respin of #59444 with the diff scoped to the cron
--atparser and the public help/docs surfaces that describe that parser.HH:MM/HH:MM:SSvalues incron add --atviaparseAt(input, tz?).--tzwhen provided.nullfor invalid IANA timezone strings and out-of-range time-only values (24:00,12:60,12:30:60).20m, and+20msemantics.HH:MM[:SS]accepted format incron add/cron edithelp and cron docs.Scope
This PR intentionally stays within the current cron CLI schedule parsing and public documentation surface:
src/cli/cron-cli/shared.tssrc/cli/cron-cli/shared.test.tssrc/cli/cron-cli/register.cron-add.tssrc/cli/cron-cli/register.cron-edit.tssrc/cli/cron-cli/schedule-options.tssrc/cli/cron-cli.test.tsdocs/cli/cron.mddocs/automation/cron-jobs.mdCurrent
mainalready supports both20mand+20mduration input. This PR does not change that duration branch.Real Behavior Proof
cron add --atnow accepts bare time-onlyHH:MM/HH:MM:SSvalues, applies--tzto those values, rejects invalid time-only values, preserves duration behavior, and exposes the new accepted format in CLI help/docs.npx -y [email protected], branch headb3ae6154ed92fca4356698d587e6a708cd9ca611.npx -y [email protected] scripts/test-projects.mjs src/cli/cron-cli/shared.test.ts;npx -y [email protected] scripts/test-projects.mjs src/cli/cron-cli.test.ts;npx -y [email protected] node_modules/oxfmt/bin/oxfmt --check --threads=1 --config .oxfmtrc.jsonc src/cli/cron-cli/register.cron-add.ts src/cli/cron-cli/register.cron-edit.ts src/cli/cron-cli/schedule-options.ts src/cli/cron-cli.test.ts docs/cli/cron.md docs/automation/cron-jobs.md;npx -y [email protected] scripts/check-docs-mdx.mjs docs/cli/cron.md docs/automation/cron-jobs.md;npx -y [email protected] node_modules/oxlint/bin/oxlint --tsconfig config/tsconfig/oxlint.core.json src/cli/cron-cli/register.cron-add.ts src/cli/cron-cli/register.cron-edit.ts src/cli/cron-cli/schedule-options.ts src/cli/cron-cli.test.ts; parser and CLI commands shown below.parseAtconsole output, and realnode openclaw.mjs cron addcommands using a dummy gateway URL.nullor fail before gateway access, valid09:00and--tz Asia/ShanghaiCLI inputs pass parser validation and reach the dummy gateway, and help/docs checks pass.>=22.19.0engine requirement; all accepted verification below used Node.js v24.13.0.Runtime used for verification:
Targeted test runs:
Targeted lint and docs checks:
Parser proof with
Date.now()fixed at2026-06-15T00:00:00Z:Real CLI proof using a dummy gateway URL so no job can be created:
That command reaches the gateway path, proving
09:00is accepted by--atparsing. The invalid time-only case still fails before any gateway call:The
--tztime-only path also reaches the safe dummy gateway instead of failing parser validation:Covered cases include:
09:0009:00:30--tz Asia/Shanghainull24:00,12:60,12:30:60+30mand30mduration behaviorHH:MM[:SS]Related