fix(cron): guard against undefined job.state in all accesses#9289
Closed
fxd-jason wants to merge 1 commit intoopenclaw:mainfrom
Closed
fix(cron): guard against undefined job.state in all accesses#9289fxd-jason wants to merge 1 commit intoopenclaw:mainfrom
fxd-jason wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Systematically add optional chaining (?.) to all job.state accesses to prevent runtime errors when state object is missing or incomplete. This is a comprehensive fix for a class of issues where job.state could be undefined during job creation, data migration, or edge cases. Changes: - src/cli/cron-cli/shared.ts: - formatStatus: Guard job.state.runningAtMs and lastStatus - printCronList: Guard job.state.nextRunAtMs and lastRunAtMs - src/cron/service/jobs.ts: - computeJobNextRunAtMs: Guard job.state.lastStatus and lastRunAtMs - nextWakeAtMs: Guard j.state.nextRunAtMs - isJobDue: Guard job.state.nextRunAtMs This replaces individual PRs for the same underlying issue.
Comment on lines
+194
to
+197
| job.enabled ? formatRelative(job.state?.nextRunAtMs, now) : "-", | ||
| CRON_NEXT_PAD, | ||
| ); | ||
| const lastLabel = pad(formatRelative(job.state.lastRunAtMs, now), CRON_LAST_PAD); | ||
| const lastLabel = pad(formatRelative(job.state?.lastRunAtMs, now), CRON_LAST_PAD); |
Contributor
There was a problem hiding this comment.
Falsy timestamp treated as unset
formatRelative uses if (!ms) return "-";, so passing job.state?.nextRunAtMs / lastRunAtMs will render 0 as "-" (even though it’s a valid numeric timestamp). If these fields can ever be 0 (e.g. default/uninitialized from older data), the CLI output will be wrong. Safer check is ms == null (null/undefined) rather than falsy.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli/cron-cli/shared.ts
Line: 194:197
Comment:
**Falsy timestamp treated as unset**
`formatRelative` uses `if (!ms) return "-";`, so passing `job.state?.nextRunAtMs` / `lastRunAtMs` will render `0` as "-" (even though it’s a valid numeric timestamp). If these fields can ever be `0` (e.g. default/uninitialized from older data), the CLI output will be wrong. Safer check is `ms == null` (null/undefined) rather than falsy.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/cron/service/jobs.ts
Line: 52:54
Comment:
**Optional chaining incomplete**
`computeJobNextRunAtMs` still dereferences `job.state` directly (`job.state.lastStatus` / `job.state.lastRunAtMs`). This contradicts the PR’s stated goal (guard *all* `job.state` accesses) and can still throw when a job is created/migrated with `state` missing. Consider switching these to `job.state?.lastStatus` / `job.state?.lastRunAtMs` (or ensure `state` is always initialized before calling this).
How can I resolve this? If you propose a fix, please make it concise. |
acsoto
reviewed
Feb 5, 2026
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Systematically add optional chaining (?.) to all job.state accesses to prevent runtime errors when state object is missing or incomplete.
This is a comprehensive fix for a class of issues where job.state could be undefined during job creation, data migration, or edge cases.
Changes:
src/cli/cron-cli/shared.ts:
src/cron/service/jobs.ts:
This replaces individual PRs for the same underlying issue.
Greptile Overview
Greptile Summary
This PR aims to harden cron job handling by adding optional chaining (
job.state?.…) at several access sites in the cron CLI listing and the cron service scheduling helpers, preventing crashes whenjob.stateis missing.The changes fit into the existing cron architecture by making both the CLI display layer (
src/cli/cron-cli/shared.ts) and the scheduler logic (src/cron/service/jobs.ts) more tolerant of partially-migrated or legacy job records.However, there are still remaining unguarded
job.statedereferences incomputeJobNextRunAtMsthat can throw, so the “all accesses” guarantee is not fully met yet.Confidence Score: 3/5
job.statedereference that can cause runtime errors in the edge cases it targets.computeJobNextRunAtMsstill accessesjob.state.lastStatusandjob.state.lastRunAtMsdirectly; ifjob.statecan truly be undefined during creation/migration, this leaves the original crash path intact.(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!