fix(gateway): emit before_reset on session reset by VACInc · Pull Request #53872 · openclaw/openclaw

VACInc · 2026-03-24T18:14:05Z

Summary

emit the typed before_reset plugin hook from the gateway session reset path
pass transcript messages and session context through to the hook runner
add a regression test covering sessions.reset -> before_reset emission

Why

Gateway /new and /reset already emitted the legacy internal session hook, but they did not emit the newer typed plugin before_reset hook. That meant plugins migrated to before_reset were skipped on the gateway reset path even though the chat command path already supported the hook.

Testing

pnpm exec vitest run src/gateway/server.sessions.gateway-server-sessions-a.test.ts -t "sessions.reset emits"

greptile-apps · 2026-03-24T18:17:31Z

Greptile Summary

This PR brings the gateway /new and /reset paths to parity with the chat-command path by emitting the typed before_reset plugin hook from performGatewaySessionReset. The implementation is mostly clean and well-tested, but there is one concrete reliability issue and one consistency concern to address.

Key changes:

New emitGatewayBeforeResetPluginHook helper in session-reset-service.ts reads transcript messages synchronously, then fires hookRunner.runBeforeReset as fire-and-forget (matching the existing commands-core.ts pattern).
A regression test verifies that sessions.reset triggers the hook with the correct transcript content and session context.

Issues found:

The synchronous readSessionMessages call inside emitGatewayBeforeResetPluginHook is not wrapped in a try-catch. A TOCTOU race between fs.existsSync and fs.readFileSync (or a permission error) would throw synchronously, propagate up into performGatewaySessionReset, and abort the reset. The existing commands-core.ts path wraps the equivalent file I/O in a try-catch to prevent this. This should be addressed before merging.
readSessionMessages enriches messages with __openclaw metadata and includes synthetic compaction entries — the chat-command path pushes raw entry.message objects without either. Plugins that handle before_reset from both paths will receive structurally different messages arrays.

Confidence Score: 3/5

Not safe to merge until the unguarded synchronous readSessionMessages call is wrapped in a try-catch to prevent file I/O errors from aborting the session reset.
The core goal of the PR is correct and the test is well-structured, but there is a concrete bug: a synchronous fs.readFileSync call without error handling can propagate out of the hook helper and into the main reset flow, causing a session reset to fail silently from a transcript read error. This is a primary-path reliability concern. A simple try-catch is the only blocker; once that is resolved the PR should be straightforward to approve.
src/gateway/session-reset-service.ts — specifically lines 272–275 where readSessionMessages is called without error handling.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/gateway/session-reset-service.ts
Line: 272-275

Comment:
**Unguarded synchronous file I/O can abort the session reset**

`readSessionMessages` calls `fs.readFileSync` synchronously on line 108 of `session-utils.fs.ts`. There is a TOCTOU window between the `fs.existsSync` guard inside that function and the actual `readFileSync` call — if the file is removed in that window (e.g. during rapid session cycling or a concurrent archive step), `readFileSync` throws. Because the call here is outside the `void hookRunner.runBeforeReset(...).catch(...)` async boundary, that synchronous exception propagates straight up through `emitGatewayBeforeResetPluginHook` and into `performGatewaySessionReset`, aborting the reset entirely.

Compare this to the equivalent path in `commands-core.ts` (line 87–120), which wraps everything — including the async file read — inside a try-catch so that any I/O failure is just logged and the reset continues.

Suggested fix: wrap the `readSessionMessages` call in a try-catch so that file I/O failures degrade gracefully to an empty messages list rather than throwing:

```
  let messages: unknown[] = [];
  try {
    if (typeof sessionId === "string" && sessionId.trim().length > 0) {
      messages = readSessionMessages(sessionId, params.storePath, sessionFile);
    }
  } catch {
    // log and continue with empty messages
    logVerbose(`before_reset: failed to read session messages for ${sessionId ?? "(none)"}`);
  }
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/gateway/session-reset-service.ts
Line: 272-275

Comment:
**Message shape diverges from the chat-command `before_reset` path**

`readSessionMessages` enriches each message with `__openclaw` metadata via `attachOpenClawTranscriptMeta` and also synthesises extra `role: "system"` entries for compaction checkpoints (see `session-utils.fs.ts` lines 130–144). The parallel path in `commands-core.ts` pushes raw `entry.message` objects directly, with no extra metadata and no compaction entries.

Plugins that already handle `before_reset` from the chat-command path (and therefore expect undecorated message objects with no compaction entries) will receive a different shape when the reset is triggered via the gateway. Consider either aligning `commands-core.ts` to also use `readSessionMessages`, or strip the `__openclaw` metadata before passing messages to the hook so the contract is consistent across both paths.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(gateway): emit before_reset on sessi..." | Re-trigger Greptile}

greptile-apps · 2026-03-24T18:17:37Z

+  const messages =
+    typeof sessionId === "string" && sessionId.trim().length > 0
+      ? readSessionMessages(sessionId, params.storePath, sessionFile)
+      : [];


Message shape diverges from the chat-command before_reset path

readSessionMessages enriches each message with __openclaw metadata via attachOpenClawTranscriptMeta and also synthesises extra role: "system" entries for compaction checkpoints (see session-utils.fs.ts lines 130–144). The parallel path in commands-core.ts pushes raw entry.message objects directly, with no extra metadata and no compaction entries.

Plugins that already handle before_reset from the chat-command path (and therefore expect undecorated message objects with no compaction entries) will receive a different shape when the reset is triggered via the gateway. Consider either aligning commands-core.ts to also use readSessionMessages, or strip the __openclaw metadata before passing messages to the hook so the contract is consistent across both paths.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/gateway/session-reset-service.ts Line: 272-275 Comment: **Message shape diverges from the chat-command `before_reset` path** `readSessionMessages` enriches each message with `__openclaw` metadata via `attachOpenClawTranscriptMeta` and also synthesises extra `role: "system"` entries for compaction checkpoints (see `session-utils.fs.ts` lines 130–144). The parallel path in `commands-core.ts` pushes raw `entry.message` objects directly, with no extra metadata and no compaction entries. Plugins that already handle `before_reset` from the chat-command path (and therefore expect undecorated message objects with no compaction entries) will receive a different shape when the reset is triggered via the gateway. Consider either aligning `commands-core.ts` to also use `readSessionMessages`, or strip the `__openclaw` metadata before passing messages to the hook so the contract is consistent across both paths. How can I resolve this? If you propose a fix, please make it concise.

Leaving this one out for now to keep the patch minimal. The concrete bug here was that before_reset was not reliably firing/saving on the reset paths we traced; the current fix is limited to preserving hook execution and avoiding false-positive/failed resets. Aligning the messages payload shape across chat-command and gateway paths would widen the hook contract surface, and this gateway path was not functioning end-to-end before this fix anyway. For the current consumer (memory-lancedb-pro), the shape difference is not material because it only summarizes normal user/assistant messages. If maintainers want the hook contract normalized, I would prefer to do that as a follow-up change.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3faba69c09

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

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b3806c95d

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

VACInc · 2026-03-24T19:21:53Z

Addressed the follow-up P1 in 97dca56ee6.

This moves gateway before_reset emission until after updateSessionStore(...) succeeds, and captures the reset source from currentEntry inside that locked mutation callback. That keeps the hook tied to the transcript that was actually reset and avoids firing it for stale pre-lock state or a reset that never commits.

I also added a focused regression that mutates the store under the lock and proves before_reset now sees the under-lock entry rather than the earlier loadSessionEntry(...) snapshot.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e537bd0e53

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

chatgpt-codex-connector · 2026-04-01T08:31:58Z

+    if (typeof sessionId === "string" && sessionId.trim().length > 0) {
+      messages = readSessionMessages(sessionId, params.storePath, sessionFile);


Move transcript read off reset request thread

When before_reset hooks are enabled, sessions.reset now performs readSessionMessages(...) synchronously in the RPC path, and that helper walks the whole transcript via sync filesystem I/O and JSON parsing. On large transcripts this blocks the Node event loop, delaying unrelated gateway websocket traffic and making reset latency spike; this is especially visible in production where sessions can be long-lived. The hook dispatch can stay fire-and-forget, but the transcript load should not run as synchronous work on the request path.

Useful? React with 👍 / 👎.

jalehman · 2026-04-01T19:46:58Z

Merged via squash.

Prepared head SHA: a47894ef16e14e07b0a3e47fe84a93c872b59250
Merge commit: 711c9e7249398edf808b0afa9fa64b2fc09bd144

Thanks @VACInc!

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a47894ef16

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

chatgpt-codex-connector · 2026-04-01T19:50:57Z

+  const sessionId = params.entry?.sessionId;
+  const sessionFile = params.entry?.sessionFile;
+  const agentId = normalizeAgentId(params.target.agentId ?? resolveDefaultAgentId(params.cfg));
+  const workspaceDir = resolveAgentWorkspaceDir(params.cfg, agentId);


Use session workspace in before_reset hook context

emitGatewayBeforeResetPluginHook always sets workspaceDir from the agent default workspace, but gateway sessions can carry a per-session spawnedWorkspaceDir (for subagent sessions) that is preserved across resets. In that case, before_reset plugins receive the wrong workspace path and can read/write memory or artifacts in the parent agent workspace instead of the session’s actual workspace when sessions.reset is called on subagent keys.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-01T19:50:57Z

+    const archived = (await fs.readdir(dir))
+      .filter((name) => name.startsWith(resetPrefix))
+      .toSorted();
+    const latest = archived[archived.length - 1];


Match archived transcript to the reset source session

The ENOENT fallback picks the lexicographically newest *.reset.* sibling, which is not tied to the specific previousSessionEntry being processed. Because emitResetCommandHooks runs this work fire-and-forget, back-to-back /new or /reset commands can produce multiple archives before an earlier hook reads, so that earlier hook may ingest a later reset’s transcript and persist/dedupe against the wrong conversation history.

Useful? React with 👍 / 👎.

@jalehman