fix(gateway): log errors when terminal persistence rejects in chat-abort cleanup by SunnyShu0925 · Pull Request #97813 · openclaw/openclaw

SunnyShu0925 · 2026-06-29T14:31:39Z

Related to #97795.

What Problem This Solves

persistGatewaySessionLifecycleEvent failures in createAgentEventHandler are silently swallowed — the terminal lifecycle handler catches rejection only to broadcast a fallback session snapshot, without logging the failure. Operators cannot observe session-store write failures in production.

Why This Change Was Made

Log the rejected persistence at the owner boundary (server-chat.ts) where the promise is created and its rejection is already caught for fallback projection. This is the single correct site — it owns both the persistence promise and the fallback projection. Logging in cleanup consumers (chat-abort.ts, server-runtime-subscriptions.ts) would be downstream duplicates that miss the owner/fallback case and risk double-reporting.

A module-level createSubsystemLogger("gateway/server-chat") follows the established gateway pattern.

Root Cause

introduced by commit 7c47904bb42d (joshavant). The original owner catch only broadcast a fallback session snapshot; it never logged the write failure.

Confidence: clear — the .catch(() => { broadcastSessionChange(evt); }) handler visibly omits error recording.

User Impact

None — this is an observability fix for operators.

Evidence

Behavior addressed: When persistGatewaySessionLifecycleEvent rejects, the gateway logs the failure with the run ID and error before broadcasting the fallback session snapshot. Logging happens once at the persistence owner boundary.

Real environment tested: Linux (kernel 4.19), Node v22, branch fix/chat-abort-persistence-log-97795 (HEAD 803e4d5499), based on origin/main 68ead4dd80.

Exact steps or command run after this patch:

$ node scripts/run-oxlint.mjs src/gateway/server-chat.ts src/gateway/server-chat.agent-events.test.ts
EXIT=0

$ npx oxfmt --check src/gateway/server-chat.ts src/gateway/server-chat.agent-events.test.ts
All matched files use the correct format.

Evidence after fix:

Persistence outcome	Before	After
`persistGatewaySessionLifecycleEvent` resolves	`markPersisted()` + broadcast (existing)	unchanged
`persistGatewaySessionLifecycleEvent` rejects	silent fallback broadcast only	`log.error` → then fallback broadcast

Test output — assertion added to existing fallback test:

$ npx vitest run src/gateway/server-chat.agent-events.test.ts --reporter=verbose

 ✓ |gateway-server| agent event handler > broadcasts a terminal fallback snapshot when persistence fails  5ms
 ✓ |gateway-server| agent event handler > does not clear a same-id retry when an old restart terminal arrives  3ms
 ✓ |gateway-server| agent event handler > ...

 Test Files  1 passed (1)
      Tests  95 passed (95)
   Duration  10.24s

The existing test "broadcasts a terminal fallback snapshot when persistence fails" now additionally asserts mockLogError was called with "gateway: failed to persist terminal session lifecycle event for run run-failed-write: Error: disk full".

Observed result after fix: When persistGatewaySessionLifecycleEventMock rejects with new Error("disk full"), the owner catch logs the error via log.error and still broadcasts the fallback sessions.changed snapshot. The markTrackedRunTerminalPersisted callback is still not called (persistence failed). All 95 existing tests pass.

What was not tested:

Forcing an actual disk write failure in a live gateway session store — the vitest suite exercises the real createAgentEventHandler production code and asserts the log is emitted from the exact same .catch() block that the compiled gateway runs

Regression Test Plan

$ npx vitest run src/gateway/server-chat.agent-events.test.ts --reporter=verbose

 Test Files  1 passed (1)
      Tests  95 passed (95)

Modified test broadcasts a terminal fallback snapshot when persistence fails:

persistGatewaySessionLifecycleEventMock rejects with new Error("disk full")
Send terminal lifecycle event via handler({...})
Assert fallback sessions.changed broadcast (existing)
Assert mockLogError called with message containing run ID and error text (new)

Changes

src/gateway/server-chat.ts: +8/−1 lines — createSubsystemLogger import, module-level logger, log.error in the persistence owner .catch()
src/gateway/server-chat.agent-events.test.ts: +11 lines — vi.mock for logging subsystem, mockLogError assertion in the existing fallback snapshot test

clawsweeper · 2026-06-29T14:35:05Z

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

This PR has the right owner-boundary idea, but it is now superseded by an open sibling that targets the same server-chat.ts catch, uses the gateway formatter for unknown errors, and includes stronger real-runtime proof.

Root-cause cluster
Relationship: superseded
Canonical: #97839
Summary: This PR and the sibling PRs address the same rejected terminal persistence logging gap; the open canonical sibling is the safer owner-boundary landing candidate because it uses gateway formatting and has stronger proof.

Members:

canonical: fix: log terminal session persistence failures #97839 - It targets the same server-chat.ts owner catch, uses formatForLog(err), includes redaction coverage, is open, and is mergeable.
fixed_by_candidate: [Bug]: Unhandled persistence failures in chat-abort.ts silently swallow errors and risk memory leaks #97795 - The open issue describes the same swallowed terminal persistence failure and can be fixed by the canonical PR once merged.
partial_overlap: fix(gateway): log terminal persistence failures in chat abort cleanup #97856 - It targets cleanup-consumer logging for the same issue, but changes downstream callers instead of the owner catch.
partial_overlap: fix(gateway): log terminal session persistence failures in chat abort cleanup #97863 - It expands logging to downstream cleanup/runtime-subscription catches for the same report, but still uses raw String(err) and lacks the canonical owner-boundary proof shape.
superseded: fix(gateway): log chat-abort terminal persistence failures instead of swallowing #97852 - It is a closed unmerged cleanup-consumer attempt for the same issue and is not a viable replacement.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this branch and review the formatted, proof-positive owner-boundary implementation in https://github.com/openclaw/openclaw/pull/97839 as the canonical fix for https://github.com/openclaw/openclaw/issues/97795.

So I’m closing this here and keeping the remaining discussion on #97839 and #97795.

Review details

Best possible solution:

Close this branch and review the formatted, proof-positive owner-boundary implementation in https://github.com/openclaw/openclaw/pull/97839 as the canonical fix for https://github.com/openclaw/openclaw/issues/97795.

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

Yes. Source inspection shows a rejected persistGatewaySessionLifecycleEvent reaches the current-main owner catch, which only broadcasts fallback sessions.changed state and does not log the failed write.

Is this the best way to solve the issue?

No. The owner-boundary location is the right fix shape, but this branch is no longer the best landing path because the sibling PR uses formatForLog(err) and includes stronger real-runtime proof.

Security review:

Security review needs attention: The diff adds a low-severity log-boundary concern by rendering unknown persistence errors without the gateway formatter.

[low] Unknown rejection bypasses gateway formatter — src/gateway/server-chat.ts:716
The new log message formats an arbitrary rejected value with String(err) instead of formatForLog(err), weakening gateway redaction and length-capping for persistence errors.
Confidence: 0.86

AGENTS.md: found and applied where relevant.

What I checked:

Repository policy read: Root and scoped gateway AGENTS.md were read fully; the review applied the gateway owner-boundary, proof, and security guidance. (AGENTS.md:1, 6de357ad4772)
Current main still has the silent owner catch: Current main creates and tracks persistGatewaySessionLifecycleEvent, then catches rejection only to broadcast a fallback session snapshot without logging the failed write. (src/gateway/server-chat.ts:719, 6de357ad4772)
This PR logs raw unknown rejection text: The PR adds a log.error call at the owner catch, but renders the unknown rejection with String(err). (src/gateway/server-chat.ts:716, 803e4d5499d0)
Gateway formatter contract: formatForLog is the existing gateway helper for formatting, redacting, and capping arbitrary values before gateway logs. (src/gateway/ws-log.ts:114, 6de357ad4772)
Canonical sibling is a better landing path: The open sibling PR uses formatForLog(err) at the same owner catch, adds a redaction assertion, and its body includes copied real runtime EPERM proof plus fallback sessions.changed output. (src/gateway/server-chat.ts:717, f3abe1365048)
Related work cluster: GitHub search and live PR views show the open issue and multiple overlapping PRs; https://github.com/openclaw/openclaw/pull/97839 is the focused owner-boundary branch, while the other open cleanup-consumer branches overlap the same report.

Likely related people:

Alix-007: Current checkout blame for the terminal persistence owner catch and sibling cleanup paths points to 5f86c3a90f69e; confidence is low because the commit title is not gateway-specific. (role: current line provenance; confidence: low; commits: 5f86c3a90f69; files: src/gateway/server-chat.ts, src/gateway/chat-abort.ts, src/gateway/server-runtime-subscriptions.ts)
openperf: Merged gateway lifecycle persistence work around persistGatewaySessionLifecycleEvent is directly adjacent to the rejected persistence path being logged here. (role: adjacent lifecycle persistence contributor; confidence: medium; commits: 613f51a7aa91; files: src/gateway/session-lifecycle-state.ts, src/gateway/session-lifecycle-state.test.ts)
steipete: Adjacent stale lifecycle snapshot work and PR search results tie this person to the same gateway session lifecycle persistence surface. (role: adjacent lifecycle persistence contributor; confidence: medium; commits: ec13a2873cf9; files: src/gateway/session-lifecycle-state.ts, src/gateway/session-lifecycle-state.test.ts)
Jordan Baker: Authored nearby gateway/runtime error-detail formatting work, which is relevant because the blocking issue is how unknown errors are rendered at the gateway log boundary. (role: adjacent formatter contributor; confidence: low; commits: 86c1622a3a3c; files: src/gateway/ws-log.ts, src/gateway/ws-log.test.ts)

Codex review notes: model internal, reasoning high; reviewed against 6de357ad4772.

SunnyShu0925 · 2026-06-29T15:09:40Z

@clawsweeper re-review

clawsweeper · 2026-06-29T15:09:43Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

SunnyShu0925 · 2026-06-29T15:40:09Z

@clawsweeper re-review

clawsweeper · 2026-06-29T15:40:13Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

SunnyShu0925 · 2026-06-29T15:57:31Z

@clawsweeper re-review

SunnyShu0925 · 2026-06-29T16:29:06Z

@clawsweeper re-review

clawsweeper · 2026-06-29T16:29:10Z

🦞🧹
ClawSweeper could not start a re-review for this item.

Reason: re-review requires an open issue or PR.

openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Jun 29, 2026

openclaw-barnacle Bot added size: S and removed size: XS labels Jun 29, 2026

SunnyShu0925 force-pushed the fix/chat-abort-persistence-log-97795 branch from 87f37e8 to 4d29d9b Compare June 29, 2026 15:17

clawsweeper Bot mentioned this pull request Jun 29, 2026

fix: log terminal session persistence failures #97839

Merged

SunnyShu0925 force-pushed the fix/chat-abort-persistence-log-97795 branch from 4d29d9b to 803e4d5 Compare June 29, 2026 15:39

openclaw-barnacle Bot added size: XS and removed size: S labels Jun 29, 2026

clawsweeper Bot mentioned this pull request Jun 29, 2026

fix(gateway): log chat-abort terminal persistence failures instead of swallowing #97852

Closed

clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. label Jun 29, 2026

clawsweeper Bot mentioned this pull request Jun 29, 2026

fix(gateway): log terminal persistence failures in chat abort cleanup #97856

Closed

luoyanglang mentioned this pull request Jun 29, 2026

fix(gateway): log terminal session persistence failures in chat abort cleanup #97863

Closed

SunnyShu0925 closed this Jun 29, 2026

SunnyShu0925 force-pushed the fix/chat-abort-persistence-log-97795 branch from 803e4d5 to 6de357a Compare June 29, 2026 16:28

fuller-stack-dev had a problem deploying to qa-live-shared June 29, 2026 16:45 — with GitHub Actions Failure

fuller-stack-dev temporarily deployed to qa-live-shared June 29, 2026 16:46 — with GitHub Actions Inactive

This was referenced Jun 29, 2026

fix: log failed terminal session persistence #97884

Closed

fix(gateway): log chat-abort terminal persistence failures #98034

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(gateway): log errors when terminal persistence rejects in chat-abort cleanup#97813

fix(gateway): log errors when terminal persistence rejects in chat-abort cleanup#97813
SunnyShu0925 wants to merge 0 commit into
openclaw:mainfrom
SunnyShu0925:fix/chat-abort-persistence-log-97795

SunnyShu0925 commented Jun 29, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

SunnyShu0925 commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

Root Cause

User Impact

Evidence

Regression Test Plan

Changes

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

SunnyShu0925 commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SunnyShu0925 commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading