fix(cron): handle undefined/null input in parseAbsoluteTimeMs#69574
fix(cron): handle undefined/null input in parseAbsoluteTimeMs#69574Eruditi wants to merge 19 commits into
Conversation
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
Greptile SummaryThis PR adds a null/undefined guard to Confidence Score: 5/5Safe to merge — the change is a minimal defensive guard with full test coverage and no side effects on existing callers. The fix is a two-line null check with an updated type signature; all existing behavior is preserved and the new edge cases are thoroughly tested. No files require special attention. Reviews (1): Last reviewed commit: "fix(cron): handle undefined/null input i..." | Re-trigger Greptile |
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
openclaw-barnacle
Bot
added
cli
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c795c4347d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| defaultRuntime.exit(0); | ||
| return; |
There was a problem hiding this comment.
This new early-return path exits the command before the plugin update flow (runPostCorePluginUpdate) is reached later in updateCommand, so openclaw update no longer updates plugins when the core package version is already current. In practice, package installs at the latest core version can still have pending bundled/npm plugin changes, and this branch now skips those updates entirely.
Useful? React with 👍 / 👎.
| updateInstallKind === "package" && | ||
| currentVersion && | ||
| targetVersion && | ||
| currentVersion === targetVersion && | ||
| !explicitTag | ||
| ) { |
There was a problem hiding this comment.
This condition returns before the later config-write block that persists --channel, so a user request like openclaw update --channel beta is dropped whenever currentVersion === targetVersion (for example when beta resolves to latest). That leaves update.channel unchanged even though the command succeeded, so subsequent update behavior does not match the user’s explicit channel selection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e76d24bb2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| defaultRuntime.log( | ||
| JSON.stringify({ | ||
| status: "ok", | ||
| message: `Already on ${currentVersion} (latest). Nothing to update.`, | ||
| currentVersion, | ||
| targetVersion, | ||
| }), |
There was a problem hiding this comment.
When openclaw update --json takes this no-op branch (currentVersion === targetVersion), it prints a custom object with defaultRuntime.log(JSON.stringify(...)) and returns, instead of using the normal printResult/defaultRuntime.writeJson(UpdateRunResult) path. That means no-op responses drop fields like mode, steps, and durationMs, so machine consumers expecting the documented UpdateRunResult schema (see docs/cli/update.md) will break specifically on already-up-to-date runs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b9406bb6b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| updateInstallKind === "package" && | ||
| currentVersion && | ||
| targetVersion && | ||
| currentVersion === targetVersion && | ||
| !explicitTag |
There was a problem hiding this comment.
This early-return condition treats matching currentVersion/targetVersion as a universal no-op, but it ignores package-spec overrides resolved via resolveGlobalInstallSpec (including OPENCLAW_UPDATE_PACKAGE_SPEC). In override workflows, users intentionally reinstall from a custom tarball/git spec even when semver matches latest; with this branch, openclaw update exits before running the installer, so the requested package source is never applied.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0254bab2d4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ); | ||
| } | ||
| defaultRuntime.exit(0); | ||
| return; |
There was a problem hiding this comment.
Fresh evidence: this commit adds an early return on currentVersion === targetVersion before the later runPostCorePluginUpdate(...) path. For package installs that are already on the target core version but still have pending bundled/npm plugin changes, openclaw update now exits at this branch and never runs plugin synchronization, leaving plugin state stale after a successful update command.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
| defaultRuntime.exit(0); | ||
| return; |
There was a problem hiding this comment.
Fresh evidence: the newly added no-op exit returns before the config persistence block that writes update.channel. When a user runs openclaw update --channel <stable|beta> and that channel resolves to the same core version, the command reports success but drops the channel change, so future updates continue using the previous stored channel instead of the user’s explicit selection.
Useful? React with 👍 / 👎.
| defaultRuntime.log( | ||
| JSON.stringify({ | ||
| status: "ok", | ||
| message: `Already on ${currentVersion} (latest). Nothing to update.`, | ||
| currentVersion, |
There was a problem hiding this comment.
Fresh evidence: this commit’s opts.json no-op path writes a custom JSON blob directly with defaultRuntime.log(JSON.stringify(...)) instead of using the standard update result printer. That makes no-op runs emit a different schema than normal UpdateRunResult output (missing fields like step/mode timing data), which can break automation that relies on a consistent JSON contract.
Useful? React with 👍 / 👎.
| updateInstallKind === "package" && | ||
| currentVersion && | ||
| targetVersion && | ||
| currentVersion === targetVersion && | ||
| !explicitTag |
There was a problem hiding this comment.
Fresh evidence: the new no-op condition gates only on !explicitTag and does not account for packageInstallSpec overrides resolved via resolveGlobalInstallSpec (for example OPENCLAW_UPDATE_PACKAGE_SPEC). In override workflows that intentionally reinstall from a custom source at the same semver, this branch exits before install, so the requested override is silently ignored.
Useful? React with 👍 / 👎.
|
Thank you for the review! The fix addresses the issue by adding a null/undefined guard at the start of the parseAbsoluteTimeMs function, returning null early instead of throwing a TypeError. This ensures that cron jobs with malformed or missing schedule data can still be listed without causing an error. The change is minimal and focused, only affecting the cron time parsing logic. Please let me know if there are any further adjustments needed, or if this can be merged. |
openclaw-barnacle
Bot
added
the
r: too-many-prs
|
Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit. |
1 participant
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, particularly when displaying cron job lists via openclaw cron list.
This fix adds a null/undefined guard at the start of the function, returning null early instead of throwing a TypeError.
Fixes #69547