feat: fire session reset hooks for daily and idle resets by salvormallow · Pull Request #61675 · openclaw/openclaw

salvormallow · 2026-04-06T04:15:36Z

Summary

Problem: Internal hooks and before_reset plugin hook only fire for manual /new and /reset commands, not for daily (4AM) or idle-timeout session resets. This means session-memory never saves on daily/idle resets.
Why it matters: 4 community issues report lost session memory on daily/idle resets (Feature: session:end internal hook event #10142, session-memory hook should support session:end events #31266, Session reset hooks: idle/daily reset do not trigger session-memory save #50891, Daily reset doesn't trigger session-memory hook flush #43524)
What changed: Wire both hook types into the lazy staleness path in initSessionState(), update session-memory registration and filter
What did NOT change (scope boundary): No new hook event names, no changes to plugin SDK API, no extraction/refactor — only export added to existing functions

Change Type (select all)

Bug fix
Feature

Scope (select all touched areas)

Memory / storage

Linked Issue/PR

Closes Feature: session:end internal hook event #10142
Closes session-memory hook should support session:end events #31266
Closes Session reset hooks: idle/daily reset do not trigger session-memory save #50891
Closes Daily reset doesn't trigger session-memory hook flush #43524
This PR fixes a bug or regression

Root Cause (if applicable)

Root cause: initSessionState() fires session_end/session_start plugin hooks for lazy resets but never fires triggerInternalHook or hookRunner.runBeforeReset. These only fire via emitResetCommandHooks() in commands-core.ts, which is only called for manual /new and /reset commands.
Missing detection / guardrail: No test asserted that internal hooks fire for lazy resets
Contributing context (if known): The lazy evaluation design means sessions aren't proactively killed — staleness is checked on next message arrival, a code path separate from manual commands

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
Target test or file: src/auto-reply/reply/session.stale-hooks.test.ts (new), src/hooks/bundled/session-memory/handler.test.ts (extended)
Scenario the test should lock in: Internal hook fires with action: "daily" / "idle" when session is stale; before_reset plugin hook fires for lazy resets; no double-fire on manual reset
Why this is the smallest reliable guardrail: Mocks triggerInternalHook and hookRunner to assert dispatch without side-effects
Existing test that already covers this (if any): session-hooks-context.test.ts covers session_end/session_start but not internal hooks or before_reset

User-visible / Behavior Changes

session-memory now saves session summaries on daily (4AM) and idle-timeout resets, not just manual /new and /reset. Plugin before_reset hook now fires for lazy resets, giving plugins pre-archive transcript access.

Diagram (if applicable)

Before:
[lazy reset] -> session_end + session_start hooks only

After:
[lazy reset] -> internal hook (command:daily/idle) + before_reset + session_end + session_start

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: Any
Runtime/container: Node.js 24
Integration/channel (if any): Any channel with daily or idle reset configured

Steps

Configure a session with daily reset (default 4AM)
Have an active session with messages before 4AM
Send a message after 4AM (triggering lazy staleness detection)

Expected

session-memory saves a summary of the previous session
before_reset plugin hook fires with reason: "daily" and transcript messages

Actual (before this PR)

session-memory does not fire — no summary saved
before_reset plugin hook does not fire

Evidence

Failing test/log before + passing after

Human Verification (required)

Verified scenarios: Daily reset with stale session triggers session-memory save; idle reset triggers save; manual /new does not double-fire
Edge cases checked: First-ever session (no previous entry), system events (heartbeat), fresh session
What you did not verify: End-to-end with real LLM slug generation (mocked in tests)

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.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Risks and Mitigations

Risk: before_reset receives the post-archive sessionFile path (after renameSync), not the original path. loadBeforeResetTranscript handles ENOENT by scanning for the latest .reset.* archived sibling.
- Mitigation: This matches the manual reset path behavior. The stable post-archive path is intentional — it avoids racing transcript reads against the archive rename.

AI Disclosure

Mark as AI-assisted in the PR title or description
Note the degree of testing: fully tested — 10 unit tests (7 new + 3 extended) + manual daily/idle reset verification
Confirm: I understand what this code does and have verified the hook dispatch logic, guard conditions, and fire-and-forget patterns

🤖 AI-assisted with Claude Code

cc @vincentkoc (hooks/plugins CODEOWNER)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d004142bd1

ℹ️ 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".

greptile-apps · 2026-04-06T04:21:13Z

Greptile Summary

Wires triggerInternalHook and hookRunner.runBeforeReset into the lazy staleness path in initSessionState() so daily (4AM) and idle-timeout resets fire the same hook sequence as manual /new and /reset. Both new blocks are correctly gated on previousSessionEndReason and !resetTriggered to prevent misfires and double-fires. The three helper functions in commands-core.ts are exported (no behavior change), session-memory's filter and HOOK.md events list are widened to command:daily and command:idle, and 7 new unit tests lock in the dispatch logic across all key scenarios.

Confidence Score: 5/5

Safe to merge — both new hook blocks are correctly guarded, fire-and-forget semantics match existing patterns, and tests cover all key scenarios including no-double-fire on manual resets.

All previously flagged issues are resolved. The missing previousSessionEndReason guard on before_reset (called out in prior review threads) is already present at line 788 in the current code, symmetric with the internal-hook block at line 762. No P0/P1 findings remain.

No files require special attention.

_{Greploops — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.
Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.}

_{Reviews (2): Last reviewed commit: "feat: fire session reset hooks for daily..." | Re-trigger Greptile}

salvormallow · 2026-04-06T09:50:59Z

Addressed all three review comments in 86ec2f8:

@chatgpt-codex-connector P2 — Internal hooks gated behind hookRunner:
Moved the internal hook block above the if (hookRunner && isNewSession) guard so it fires independently of the plugin hook runner — matching how emitResetCommandHooks() fires triggerInternalHook without checking hookRunner.

@greptile-apps P1 — Missing previousSessionEndReason guard:
Added && previousSessionEndReason to the before_reset guard. Also removed the ?? "daily" fallback since with the guard, previousSessionEndReason is always defined inside the block. Both blocks are now symmetric.

@greptile-apps P2 — Long line:
Split isResetCommand predicate across multiple lines.

All 34 tests pass.

salvormallow · 2026-04-06T12:18:13Z

(Updated after rebase onto latest main — the review fixes from the previous comment are now in 7e627e9. Also resolved merge conflict in commands-core.ts where upstream extracted reset hooks into commands-reset-hooks.ts; our export additions now target that file instead.)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e627e9e55

ℹ️ 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".

salvormallow · 2026-04-06T12:41:22Z

@chatgpt-codex-connector Fixed in fbbf7bb — now uses the already-resolved agentId from resolveSessionAgentId() (config-aware) instead of re-parsing via resolveAgentIdFromSessionKey(). Note: the manual path in emitResetCommandHooks() has the same pattern — out of scope for this PR but worth a follow-up.

chatgpt-codex-connector · 2026-04-06T12:41:32Z

To use Codex here, create a Codex account and connect to github.

salvormallow · 2026-04-08T04:34:04Z

CI note: All failures on this PR are pre-existing upstream breakage on main, not from this PR's changes:

extensions/discord/src/proxy-request-client.ts(36,7) — RequestClientOptions now requires scheduler and runtimeProfile, but the Discord extension wasn't updated. Breaks check, build-artifacts, build-smoke, checks-fast-contracts-protocol.
src/plugins/provider-runtime.test.ts(611,51) — "demo-cli" is no longer a valid ExternalOAuthManager value. Breaks check, check-additional.

These cascade into the extension shard cancellations. This PR only touches src/auto-reply/reply/, src/hooks/bundled/session-memory/, and src/commands/commands-reset-hooks.ts — none of the failing files.

Happy to rebase once main is green again. ready for review whenever you get a chance 🙏

FeSens · 2026-04-08T22:48:19Z

Amazing work! We need this fix!

Wire internal hooks and before_reset plugin hook into the lazy staleness path so they fire on daily (4AM) and idle-timeout resets, not just manual /new and /reset commands. Both hooks fire as fire-and-forget to avoid blocking the user's message response (session-memory's LLM slug generation can take up to 15s). commands-core is dynamically imported inside the fire-and-forget block to avoid pulling its heavy dependency tree into session.ts's static imports. Export loadBeforeResetTranscript from commands-core for use by session.ts. Update bundled session-memory hook registration and filter to accept "daily" and "idle" actions. Fixes openclaw#10142, openclaw#31266, openclaw#50891, openclaw#43524 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

clawsweeper · 2026-04-28T18:55:59Z

Codex review: needs real behavior proof before merge. Reviewed June 6, 2026, 12:46 AM ET / 04:46 UTC.

Summary
Review failed before ClawSweeper could summarize the requested change.

PR surface: Source +64, Tests +429, Docs +1. Total +494 across 8 files.

Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path.

Review metrics: none identified.

Merge readiness
Overall: 🌊 off-meta tidepool
Proof: 🌊 off-meta tidepool
Patch quality: 🌊 off-meta tidepool
Result: rating does not apply to this item.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] No close action taken because the review did not complete.

Maintainer options:

Decide the mitigation before merge
Retry the Codex review after fixing the execution failure.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] Review did not complete, so no work-lane recommendation was made.

Review details

Best possible solution:

Retry the Codex review after fixing the execution failure.

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

Unclear. The review failed before ClawSweeper could establish a reproduction path.

Is this the best way to solve the issue?

Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 9313471fa579.

Label changes

Label changes:

remove P2: Current review triage priority is none.
remove merge-risk: 🚨 compatibility: Current PR review selected no merge-risk labels.
remove merge-risk: 🚨 session-state: Current PR review selected no merge-risk labels.

Label justifications:

rating: 🌊 off-meta tidepool: Overall readiness is 🌊 off-meta tidepool; proof is 🌊 off-meta tidepool and patch quality is 🌊 off-meta tidepool.

Evidence reviewed

PR surface:

Source +64, Tests +429, Docs +1. Total +494 across 8 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	69	5	+64
Tests	2	429	0	+429
Docs	3	9	8	+1
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	8	507	13	+494

What I checked:

failure reason: codex execution failed.
codex failure detail: Codex review failed for this PR with exit 1.
codex stdout: Per-item Codex failure; continuing with the rest of the shard.

Likely related people:

unknown: Codex failed before it could trace repository history. (role: review did not complete; confidence: low)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

clawsweeper · 2026-05-20T12:04:06Z

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-05T04:53:09Z

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

openclaw-barnacle Bot added the size: M label Apr 6, 2026

chatgpt-codex-connector Bot reviewed Apr 6, 2026

View reviewed changes

Comment thread src/auto-reply/reply/session.ts

greptile-apps Bot reviewed Apr 6, 2026

View reviewed changes

Comment thread src/auto-reply/reply/session.ts Outdated

Comment thread src/hooks/bundled/session-memory/handler.ts Outdated

salvormallow force-pushed the feat/session-reset-hooks-lazy branch from d004142 to 86ec2f8 Compare April 6, 2026 09:50

salvormallow force-pushed the feat/session-reset-hooks-lazy branch from 86ec2f8 to 6682f77 Compare April 6, 2026 12:09

openclaw-barnacle Bot added size: L and removed size: M labels Apr 6, 2026

salvormallow force-pushed the feat/session-reset-hooks-lazy branch from 6682f77 to 7e627e9 Compare April 6, 2026 12:15

openclaw-barnacle Bot added size: M and removed size: L labels Apr 6, 2026

chatgpt-codex-connector Bot reviewed Apr 6, 2026

View reviewed changes

Comment thread src/auto-reply/reply/session.ts Outdated

salvormallow force-pushed the feat/session-reset-hooks-lazy branch from 7e627e9 to fbbf7bb Compare April 6, 2026 12:41

simon-inkie mentioned this pull request Apr 9, 2026

[Bug]: 2026.4.8 regression: daily atHour reset no longer fires, causing unbounded session growth #63732

Closed

vincentkoc added clownfish:human-review labels Apr 28, 2026

vincentkoc added clawsweeper Tracked by ClawSweeper automation and removed clownfish:merge-ready labels Apr 28, 2026

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

clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label May 19, 2026

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

openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 6, 2026

Uh oh!

Conversation

salvormallow commented Apr 6, 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 (before this PR)

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

AI Disclosure

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

greptile-apps Bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

Uh oh!

salvormallow commented Apr 6, 2026

Uh oh!

salvormallow commented Apr 6, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

salvormallow commented Apr 6, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 6, 2026

Uh oh!

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

Uh oh!

Uh oh!

FeSens commented Apr 8, 2026

Uh oh!

clawsweeper Bot commented Apr 28, 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 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

greptile-apps Bot commented Apr 6, 2026 •

edited

Loading

salvormallow commented Apr 8, 2026 •

edited

Loading

clawsweeper Bot commented Apr 28, 2026 •

edited

Loading