fix(memory-core): retry dreaming cron registration after startup deferral [AI-assisted]#73170
fix(memory-core): retry dreaming cron registration after startup deferral [AI-assisted]#73170luyao618 wants to merge 1 commit into
Conversation
Greptile SummaryAdds a 5-second Confidence Score: 4/5Safe to merge — the fix is narrowly scoped and the deferred retry is well-guarded by existing throttle logic. No P0 or P1 findings. Single P2: the timer handle is not stored, preventing future cancellation. Throttle in No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/memory-core/src/dreaming.ts
Line: 797-803
Comment:
**No handle stored for the deferred `setTimeout`**
The timer ID is discarded, so there is no way to cancel it if the gateway shuts down or the plugin is torn down within the 5-second window. This is harmless in production (it fires once and exits), but it can leave a dangling async operation in tests that don't advance the fake timers past the delay, and it prevents any future cleanup hook from cancelling it.
If a plugin-level teardown / `cleanup` hook exists, storing the handle and calling `clearTimeout(retryHandle)` there would be the complete fix.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(memory-core): retry dreaming cron re..." | Re-trigger Greptile |
| setTimeout(() => { | ||
| void reconcileManagedDreamingCron({ reason: "runtime" }).catch((err) => { | ||
| api.logger.error( | ||
| `memory-core: deferred dreaming cron retry failed: ${formatErrorMessage(err)}`, | ||
| ); | ||
| }); | ||
| }, STARTUP_CRON_RETRY_DELAY_MS); |
There was a problem hiding this comment.
No handle stored for the deferred
setTimeout
The timer ID is discarded, so there is no way to cancel it if the gateway shuts down or the plugin is torn down within the 5-second window. This is harmless in production (it fires once and exits), but it can leave a dangling async operation in tests that don't advance the fake timers past the delay, and it prevents any future cleanup hook from cancelling it.
If a plugin-level teardown / cleanup hook exists, storing the handle and calling clearTimeout(retryHandle) there would be the complete fix.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/memory-core/src/dreaming.ts
Line: 797-803
Comment:
**No handle stored for the deferred `setTimeout`**
The timer ID is discarded, so there is no way to cancel it if the gateway shuts down or the plugin is torn down within the 5-second window. This is harmless in production (it fires once and exits), but it can leave a dangling async operation in tests that don't advance the fake timers past the delay, and it prevents any future cleanup hook from cancelling it.
If a plugin-level teardown / `cleanup` hook exists, storing the handle and calling `clearTimeout(retryHandle)` there would be the complete fix.
How can I resolve this? If you propose a fix, please make it concise.…rral When the cron service is unavailable at gateway_start (startup timing race), schedule a deferred retry after 5 seconds so the managed dreaming cron job registers once sidecars have settled. This prevents a deadlock where the runtime reconciliation path (heartbeat/cron triggered) never fires because the cron was never registered. Closes openclaw#72841 AI-assisted (built with Claude Code via Hermes orchestration).
bb52527 to
68462d5
Compare
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Current main already contains a more complete maintainer implementation of this PR's intended memory-core dreaming cron startup retry. The fix landed through merged PR #73493 as commit e84ebea, with bounded retries, timer cleanup, gateway_stop disposal, tests, and an Unreleased changelog entry for #72841. Best possible solution: Close this PR as superseded by the merged maintainer implementation in #73493. Keep the bounded retry and cleanup behavior already on main, and let the fix ship with the next OpenClaw release. What I checked:
So I’m closing this here and keeping the remaining discussion on the canonical linked item. Codex review notes: model gpt-5.5, reasoning high; reviewed against f256eeba431b; fix evidence: commit e84ebeafbd67. |
Summary
gateway_starthook fires before the cron service is available. The runtime reconciliation path only triggers on heartbeat/cron events, creating a deadlock when the cron was never registered.setTimeout) after the startup deferral so the managed dreaming cron job registers once sidecars have settled.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause
gateway_starthook fires viavoid hookRunner.runGatewayStart()(fire-and-forget) before sidecars fully start, sogetCron()returns null. The existing runtime retry path inbefore_agent_replyonly fires on heartbeat/cron triggers — if the dreaming cron never registered and no heartbeat fires, the reconciliation never runs, creating a deadlock.gatewayContext.getCron()inbefore_agent_reply, but this path depends on heartbeat/cron triggers that may not fire if dreaming cron never registered.Regression Test Plan
extensions/memory-core/src/dreaming.test.tsUser-visible / Behavior Changes
Diagram (if applicable)
N/A
Security Impact (required)