fix: hybrid-memory cron maintenance outcomes and exit ledger validation - #1204
Conversation
Changed Files SummarySource (5)
Tests (1)
Docs (2)
Total: 8 files changed |
Forge Feedback Loop
|
- Create cron-exit-validator.ts with structured validation - Add validate-cron-exit CLI command for internal use - Update cron job bash harness with validation instructions - Add comprehensive tests for exit ledger parsing and validation - Register new validation command in CLI Agent-Logs-Url: https://github.com/markus-lassfolk/openclaw-hybrid-memory/sessions/cd60b52e-d486-4166-b77f-63a3040389a2 Co-authored-by: markus-lassfolk <3661143+markus-lassfolk@users.noreply.github.com>
- Update existing cron job messages when normalizeExisting=true - Remove obsolete command references (e.g., consolidate-episodes) - Convert tests to vitest format - All 14 tests passing Agent-Logs-Url: https://github.com/markus-lassfolk/openclaw-hybrid-memory/sessions/cd60b52e-d486-4166-b77f-63a3040389a2 Co-authored-by: markus-lassfolk <3661143+markus-lassfolk@users.noreply.github.com>
- Document validation behavior and expected outcomes - Explain migration path for existing installations - Detail config-based skipping mechanism - List acceptance criteria and testing coverage Agent-Logs-Url: https://github.com/markus-lassfolk/openclaw-hybrid-memory/sessions/cd60b52e-d486-4166-b77f-63a3040389a2 Co-authored-by: markus-lassfolk <3661143+markus-lassfolk@users.noreply.github.com>
…gelog - Pin CI/security to Node 22.16 (node:sqlite + FTS5, nodejs/node#57621) - engines, .nvmrc, docs: require >=22.16.0 - Tests: storageGrowth nulls vs expect.anything(); EventLog CHECK relaxed - CHANGELOG [Unreleased]: cron exit validation (#1203) + Node bump - Biome-format cron exit validator / harness / install paths Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da95333ac5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… error) Co-authored-by: Cursor <cursoragent@cursor.com>
Empty or step-less HM_EXIT cannot distinguish guard skip from abort before hm_step; report failure so validate-cron-exit surfaces the error. Co-authored-by: chatgpt-codex-connector <noreply@openai.com> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfa27f2657
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sting Apply overrides to payload.message for agentTurn jobs and skip canonical message replacement when a per-job override is provided. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2ed444fdb
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "", | ||
| "VALIDATION & GUARD UPDATE (Issue: cron jobs report OK despite failures)", | ||
| `After the bash script completes, validate that ALL required steps [${requiredStepsList}] appear in HM_EXIT with exit=0.`, | ||
| "- If ANY required step is missing from HM_EXIT, has exit≠0, or the log contains 'unknown command', this job has FAILED.", |
There was a problem hiding this comment.
Allow full-job config skips in cron validation flow
The new generic validation rule marks any missing required step as FAILED, which conflicts with several job preambles that explicitly instruct a full skip when a feature is disabled (for example self-correction-analysis and weekly-reflection in cmd-install.ts). In that disabled-feature scenario, no hm_step lines are expected in HM_EXIT, so the current wording forces a failed outcome instead of the intended skipped outcome and can incorrectly increment error counts for legitimate guard/config skips.
Useful? React with 👍 / 👎.
…gated skip Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e81e3d1f1
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `After the bash script completes, validate that ALL required steps [${requiredStepsList}] appear in HM_EXIT with exit=0.`, | ||
| "- If ANY required step is missing from HM_EXIT, has exit≠0, or the log contains 'unknown command', this job has FAILED.", | ||
| "- If a step is replaced with a config-skip variant (e.g., 'distill-skipped' exit=0 when distill.enabled is false), that counts as present.", | ||
| "- Only after ALL required steps are validated successful: perform the GUARD CHECK timestamp write.", | ||
| "- If validation fails, do NOT update the guard file.", |
There was a problem hiding this comment.
Enforce exit-ledger validation in the execution path
The updated cron template only describes validation after the bash block, but it never runs openclaw hybrid-mem validate-cron-exit (or any other command that can fail the turn). When the model omits or misreports that manual validation step, cron can still mark the run successful even if required HM_EXIT steps are missing/failed, which recreates the false-green behavior this fix is meant to prevent.
Useful? React with 👍 / 👎.
| if (text.length === 0) return false; | ||
| if (/\bjob was skipped\b/.test(text)) return true; | ||
| if (/\bskip(ped|ping)?\b.*\bhybrid[- ]memory\b.*\bconfig\b/.test(text)) return true; | ||
| if (/\bself[- ]correction\b.*\b(disabled|skipped)\b/.test(text)) return true; |
There was a problem hiding this comment.
Match camelCase feature names in skip-log heuristics
The skip detector only matches self-correction with a hyphen/space, but several cron preambles use camelCase config keys such as selfCorrection.enabled and nightlyCycle.enabled (cmd-install.ts). If logs follow those key names (for example, "selfCorrection.enabled is false, skipping"), this regex does not match, so an intentionally skipped full job with an empty ledger is incorrectly marked failed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e427f30db0
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| for (const line of lines) { | ||
| // Match patterns like: "error: unknown command 'consolidate-episodes'" | ||
| const match = line.match(/(?:error|Error):\s*unknown command\s+['"]([^'"]+)['"]/); |
There was a problem hiding this comment.
Limit unknown-command scan to failing step output
checkForUnknownCommands fails the whole run whenever any HM_LOG line contains error: unknown command ..., but this scans all log text, not just the executed command failure stream. The maintenance-log-analyzer cron job (defined in cmd-install.ts) intentionally prints excerpts from prior maintenance logs, so a successful analyzer run can legitimately include historical unknown command strings and be misclassified as failed. This creates false failures and can inflate cron error counts even when required steps exited 0.
Useful? React with 👍 / 👎.
Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.