Fix/cron parse undefined by Eruditi · Pull Request #72449 · openclaw/openclaw

Eruditi · 2026-04-27T00:35:51Z

Summary

Describe the problem and fix in 2–5 bullets:

If this PR fixes a plugin beta-release blocker, title it fix(<plugin-id>): beta blocker - <summary> and link the matching Beta blocker: <plugin-name> - <summary> issue labeled beta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.

Problem:
Why it matters:
What changed:
What did NOT change (scope boundary):

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related #
This PR fixes a bug or regression

Root Cause (if applicable)

For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write N/A. If the cause is unclear, write Unknown.

Root cause:
Missing detection / guardrail:
Contributing context (if known):

Regression Test Plan (if applicable)

For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write N/A.

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file:
Scenario the test should lock in:
Why this is the smallest reliable guardrail:
Existing test that already covers this (if any):
If no new test is added, why not:

User-visible / Behavior Changes

List user-visible changes (including defaults/config).
If none, write None.

Diagram (if applicable)

For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write N/A.

Before:
[user action] -> [old state]

After:
[user action] -> [new state] -> [result]

Security Impact (required)

New permissions/capabilities? (Yes/No)
Secrets/tokens handling changed? (Yes/No)
New/changed network calls? (Yes/No)
Command/tool execution surface changed? (Yes/No)
Data access scope changed? (Yes/No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS:
Runtime/container:
Model/provider:
Integration/channel (if any):
Relevant config (redacted):

Steps

Expected

Actual

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
Edge cases checked:
What you did not verify:

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

Backward compatible? (Yes/No)
Config/env changes? (Yes/No)
Migration needed? (Yes/No)
If yes, exact upgrade steps:

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk:
- Mitigation:

The parseAbsoluteTimeMs function would throw 'Cannot read properties of undefined (reading trim)' when called with undefined or null input. This could happen when cron jobs had malformed or missing schedule data. Fixes openclaw#69547

The openclaw update command would run a full npm install even when the installed version already matched the latest registry version. This caused unnecessary work, wasted time, and could be dangerous on resource-constrained systems. This fix adds a version comparison before invoking the package manager. If the current version equals the target version (and no explicit tag was requested), the update is skipped with an informative message. Fixes openclaw#69412

greptile-apps · 2026-04-27T00:37:15Z

Greptile Summary

This PR fixes a crash in parseAbsoluteTimeMs by widening its signature to accept null | undefined and adds a matching test suite. It also adds an early-exit path in update-command.ts to skip a redundant package install when the resolved target version already matches the current version.

Confidence Score: 4/5

Safe to merge; the cron parse fix is correct and well-tested, and the update-command change is low-risk with one minor labelling issue.

Only a P2 style issue was found (hardcoded (latest) label in the already-up-to-date message). No logic errors or security concerns.

src/cli/update-cli/update-command.ts — verify the channel-label messaging.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/cli/update-cli/update-command.ts
Line: 1348

Comment:
**Hardcoded `(latest)` label may be misleading**

The message `Already on ${currentVersion} (latest). Nothing to update.` is shown whenever `currentVersion === targetVersion && !explicitTag`, regardless of which channel the user is tracking. If the user is on a `beta` or `next` channel, the resolved `targetVersion` could be a pre-release that is not "latest" in the npm `latest` tag sense, making the message confusing.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Merge branch 'openclaw:main' into fix/cr..." | Re-trigger Greptile}

greptile-apps · 2026-04-27T00:37:18Z

+    !explicitTag
+  ) {
+    if (!opts.json) {
+      defaultRuntime.log(theme.success(`Already on ${currentVersion} (latest). Nothing to update.`));


Hardcoded (latest) label may be misleading

The message Already on ${currentVersion} (latest). Nothing to update. is shown whenever currentVersion === targetVersion && !explicitTag, regardless of which channel the user is tracking. If the user is on a beta or next channel, the resolved targetVersion could be a pre-release that is not "latest" in the npm latest tag sense, making the message confusing.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/cli/update-cli/update-command.ts Line: 1348 Comment: **Hardcoded `(latest)` label may be misleading** The message `Already on ${currentVersion} (latest). Nothing to update.` is shown whenever `currentVersion === targetVersion && !explicitTag`, regardless of which channel the user is tracking. If the user is on a `beta` or `next` channel, the resolved `targetVersion` could be a pre-release that is not "latest" in the npm `latest` tag sense, making the message confusing. How can I resolve this? If you propose a fix, please make it concise.

clawsweeper · 2026-04-27T07:46:50Z

Thanks for the contribution. I reviewed the branch, and this PR is not a good landing base for OpenClaw.

Close: this branch is not a safe landing candidate because it mixes a small cron parser guard with an unrelated update early-exit that conflicts with current main's same-version package refresh contract, has no real behavior proof, and is currently conflicting.

So I’m closing this PR rather than keeping an unmergeable branch open. A new narrow PR that carries only the useful part is welcome.

Review details

Best possible solution:

Close this mixed branch, keep current main's package-refresh semantics, and accept a new narrow cron-only PR only if it includes a current stack trace or real repro for nullish parser input.

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

Unclear for the claimed cron crash: the linked report was closed as a docs-link path and current cron call sites guard schedule.at before parsing. The update regression introduced by this PR is source-reproducible against current tests that require same-version package refreshes.

Is this the best way to solve the issue?

No. The maintainable path is a narrow cron-only change with current proof, while preserving the current package-update refresh contract.

Security review:

Security review cleared: The diff changes CLI update control flow but adds no dependencies, workflow changes, secret handling, network sources, or new supply-chain execution surface beyond the functional update-path regression.

AGENTS.md: found and applied where relevant.

What I checked:

Repository policy read: Root AGENTS.md was read fully and applied; no scoped AGENTS.md owns src/cli/update-cli or src/cron. (AGENTS.md:1, 20534c57b7e8)
PR state and proof: Live GitHub reports the PR head 3d1f3abb0cbc844baf1dbfbeeb8dca1693180235, mergeStateStatus DIRTY, mergeable CONFLICTING, and labels include triage: needs-real-behavior-proof. (3d1f3abb0cbc)
Unrelated update hunk: The PR inserts an early exit(0) for same-version package updates before the normal package install, post-core plugin sync, channel persistence, restart, and doctor paths. (src/cli/update-cli/update-command.ts:1521, 3d1f3abb0cbc)
Current update contract: Current main computes packageAlreadyCurrent but dry-run still describes a refresh, and the runtime still calls runPackageInstallUpdate for package updates instead of skipping them. (src/cli/update-cli/update-command.ts:3410, 20534c57b7e8)
Current update tests: Current tests require same-version package-manager updates to run the install and plugin sync, and a second test requires same-version package installs to refresh the installed package tree. (src/cli/update-cli.test.ts:2145, 20534c57b7e8)
Cron report already closed on a different root cause: The linked cron trim issue was closed by maintainer comment as covered by merged PR fix(terminal): tolerate undefined path in formatDocsLink #67086, which hardened the onboarding/config docs-link path that was actually throwing trim.

Likely related people:

Alix-007: Authored the merged parser validation PR that now owns src/cron/parse.ts and src/cron/parse.test.ts on current main. (role: recent parser contributor; confidence: high; commits: 4559a8d73655; files: src/cron/parse.ts, src/cron/parse.test.ts)
vincentkoc: Provided maintainer verification on the merged parser validation PR and is credited as co-author in the merge commit metadata. (role: recent parser reviewer/co-author; confidence: medium; commits: 4559a8d73655; files: src/cron/parse.ts, src/cron/parse.test.ts)
steipete: Closed the linked update no-op issue with the current main behavior rationale, which conflicts with this PR's early-exit update hunk. (role: linked update behavior decision-maker; confidence: medium; files: src/cli/update-cli/update-command.ts, src/cli/update-cli.test.ts)

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

clawsweeper · 2026-05-20T19:59:25Z

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?

The egg game starts only after the PR passes the real-behavior proof check.
Before that, no creature or rarity is rolled. The treat waits for real proof.
This is still just collectible flavor: proof affects review readiness, not creature quality.

openclaw-barnacle · 2026-06-14T05:01:51Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

clawsweeper · 2026-06-17T22:00:59Z

ClawSweeper applied the proposed close for this PR.

Action: closed this PR.
Close reason: low-signal unmergeable PR.
Evidence: durable ClawSweeper review.

Eruditi and others added 20 commits April 21, 2026 11:26

Merge branch 'openclaw:main' into fix/cron-parse-undefined

03f4bf7

Merge branch 'openclaw:main' into fix/cron-parse-undefined

2e76d24

Merge branch 'openclaw:main' into fix/cron-parse-undefined

25d0ddc

Merge branch 'openclaw:main' into fix/cron-parse-undefined

2b0b3ea

Merge branch 'openclaw:main' into fix/cron-parse-undefined

702aaff

Merge branch 'openclaw:main' into fix/cron-parse-undefined

1b9406b

Merge branch 'main' into fix/cron-parse-undefined

710878e

Merge branch 'openclaw:main' into fix/cron-parse-undefined

98a0d63

Merge branch 'main' into fix/cron-parse-undefined

4c62086

Merge branch 'openclaw:main' into fix/cron-parse-undefined

0254bab

Merge branch 'main' into fix/cron-parse-undefined

fae98ed

Merge branch 'openclaw:main' into fix/cron-parse-undefined

328a9d6

Merge branch 'openclaw:main' into fix/cron-parse-undefined

29a2920

Merge branch 'openclaw:main' into fix/cron-parse-undefined

3c383cd

Merge branch 'openclaw:main' into fix/cron-parse-undefined

86dd586

Merge branch 'openclaw:main' into fix/cron-parse-undefined

056792d

Merge branch 'openclaw:main' into fix/cron-parse-undefined

f408d61

Merge branch 'openclaw:main' into fix/cron-parse-undefined

8b5d1c8

openclaw-barnacle Bot added cli CLI command changes size: S triage: blank-template Candidate: PR template appears mostly untouched. triage: refactor-only Candidate: refactor/cleanup-only PR without maintainer context. labels Apr 27, 2026

greptile-apps Bot reviewed Apr 27, 2026

View reviewed changes

Eruditi added 3 commits April 28, 2026 10:34

Merge branch 'openclaw:main' into fix/cron-parse-undefined

e27329b

Merge branch 'openclaw:main' into fix/cron-parse-undefined

e042198

Merge branch 'openclaw:main' into fix/cron-parse-undefined

80a0b61

Merge branch 'openclaw:main' into fix/cron-parse-undefined

3d1f3ab

openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 20, 2026

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 14, 2026

Alix-007 mentioned this pull request Jun 17, 2026

fix(cron): reject invalid absolute timestamps #93903

Merged

clawsweeper Bot closed this Jun 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix/cron parse undefined#72449

Fix/cron parse undefined#72449
Eruditi wants to merge 24 commits into
openclaw:mainfrom
Eruditi:fix/cron-parse-undefined

Eruditi commented Apr 27, 2026

Uh oh!

greptile-apps Bot commented Apr 27, 2026

Uh oh!

greptile-apps Bot Apr 27, 2026

Uh oh!

clawsweeper Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 20, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 14, 2026

Uh oh!

clawsweeper Bot commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

Eruditi commented Apr 27, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps Bot Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

clawsweeper Bot commented May 20, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 14, 2026

Uh oh!

clawsweeper Bot commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Apr 27, 2026 •

edited

Loading