fix(gateway): log start session persistence failures by masatohoshino · Pull Request #100313 · openclaw/openclaw

masatohoshino · 2026-07-05T11:46:20Z

What Problem This Solves

Resolves an issue where operators lose all visibility when a gateway session's
start lifecycle event fails to persist. When the session-lifecycle store
rejects the write for a "start" event, the run's start marker is silently
dropped from restart-recovery accounting with no log line, so an operator
investigating a session that never resumed after a restart has no trace of the
failed write.

The sibling terminal-phase persistence failure was made observable in
#97839 (log terminal session persistence failures), but the start-phase path
was left swallowing its rejection.

Why This Change Was Made

The start-phase persist was fired as
void persistGatewaySessionLifecycleEvent({...}).catch(() => undefined) — the
rejection was discarded with zero logging, while the terminal-phase sibling in
the same file already logs the equivalent failure. This change mirrors #97839:
the start-phase catch now emits a redacted logError with the same message
shape (via formatForLog), so both lifecycle phases surface persistence
failures consistently. The fire-and-forget semantics are unchanged — this is
limited to adding the missing log line, with no change to recovery behavior.

User Impact

Operators and maintainers now get a logged, redacted diagnostic
(gateway: start session persistence failed session=… run=… error=…) when a
session's start marker fails to persist, matching the existing terminal-phase
log. This closes an asymmetric blind spot in restart-recovery diagnostics. No
user-visible runtime behavior changes.

Evidence

Real behavior proof (genuine read-only-filesystem rejection, no mocks)

Drove the shipped createAgentEventHandler end-to-end against a real
session-store persistence failure — the production analog of a read-only disk
or filesystem holding session state. A real on-disk session entry is seeded
via the prod upsertSessionEntry API while the fs is writable, then its
directory is remounted read-only (EROFS) inside a user+mount namespace.
Reads still succeed (so loadSessionEntry finds the entry); the real atomic
write (@openclaw/fs-safe replaceFileAtomic → fs.writeFile temp) then fails
with EROFS, requireWriteSuccess: true propagates the rejection, and the
handler's start-phase .catch fires. No vi.mock; the persistence function and
logError/formatForLog are the real production modules.

EROFS (not chmod) is required because @openclaw/fs-safe chmods the target
directory back to 0o700 immediately before every write, which defeats
permission-based injection on a directory the process owns. A read-only mount
cannot be chmod'd away.

[setup] seeded real entry: <sandbox>/.openclaw/agents/main/sessions/sessions.json
[setup] canonicalKey=agent:main:proof-session sessionId=sess-proof-1
[setup] remounted read-only (EROFS): <sandbox>/.openclaw/agents/main/sessions
[setup] direct write to dir blocked by EROFS: YES

=== A. persistGatewaySessionLifecycleEvent rejection (real EROFS write) ===
persist REJECTED: code=EROFS
  Error: EROFS: read-only file system, open '<sandbox>/.../sessions.json.<pid>.<uuid>.tmp'

=== B. Shipped createAgentEventHandler `.catch` -> logError (real stderr) ===
[gateway] start session persistence failed session=agent:main:proof-session run=run-proof-start error=Error: EROFS: read-only file system, open '<sandbox>/.../sessions.json.<pid>.<uuid>.tmp': code=EROFS

=== RESULT ===
underlying failure was a real EROFS fs write error: YES
shipped handler emitted the redacted start-persist log: YES
PROOF PASS

Before this change the same run produces no log line (the rejection is
swallowed by .catch(() => undefined)); after it, the redacted diagnostic above
is emitted, mirroring the terminal-phase behavior shipped in #97839.

Focused regression test

A colocated regression test (server-chat.agent-events.test.ts), modeled on the
existing terminal-failure test and the existing start-persist success test,
forces the start persist to reject and asserts the redacted log fires exactly
once:

✓ agent event handler > logs when start session persistence fails
 Test Files  2 passed (2)
      Tests  200 passed (200)

Static checks

oxfmt --check — clean; oxlint — clean
tsgo:core — exit 0

The change is confined to src/gateway/server-chat.ts (the start-phase catch)
plus its colocated test. No config, migration, or protocol surface touched.

clawsweeper · 2026-07-05T12:10:29Z

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 9:21 AM ET / 13:21 UTC.

Summary
The branch adds a gateway log for rejected start session lifecycle persistence and a colocated regression test.

PR surface: Source +7, Tests +26. Total +33 across 2 files.

Reproducibility: yes. at source level. Current main catches rejected start lifecycle persistence writes with catch(() => undefined), and the updated PR body includes a real EROFS terminal transcript showing the patched handler emits the redacted log.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

[P2] Rerun or triage the current QA Smoke CI runtime-inventory-drift-check failure before merge.

Risk before merge

[P1] Current-head QA Smoke CI failed runtime-inventory-drift-check; it may be unrelated to this narrow gateway log diff, but merge readiness still needs a rerun or triage.

Maintainer options:

Decide the mitigation before merge
Land the narrow gateway log and regression test after the QA Smoke CI failure is rerun or triaged, keeping session persistence semantics unchanged.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

The remaining action is maintainer validation or rerun of the failing QA Smoke CI check; there is no narrow code defect for ClawSweeper repair.

Security
Cleared: No security or supply-chain concern was found; the diff only adds a gateway diagnostic log through the existing redacting formatter and a colocated test.

Review details

Best possible solution:

Land the narrow gateway log and regression test after the QA Smoke CI failure is rerun or triaged, keeping session persistence semantics unchanged.

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

Yes, at source level. Current main catches rejected start lifecycle persistence writes with catch(() => undefined), and the updated PR body includes a real EROFS terminal transcript showing the patched handler emits the redacted log.

Is this the best way to solve the issue?

Yes. Logging once at the start-persistence catch in createAgentEventHandler matches the terminal sibling and is narrower than moving logging into the shared persistence helper, which could blur caller ownership or double-log other paths.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The updated PR body includes after-fix terminal output from a real EROFS-backed session-store write failure showing the start persistence rejection log.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The updated PR body includes after-fix terminal output from a real EROFS-backed session-store write failure showing the start persistence rejection log.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is a normal-priority gateway observability bug fix with limited blast radius and no evidence of active outage, data loss, or security impact.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The updated PR body includes after-fix terminal output from a real EROFS-backed session-store write failure showing the start persistence rejection log.
proof: sufficient: Contributor real behavior proof is sufficient. The updated PR body includes after-fix terminal output from a real EROFS-backed session-store write failure showing the start persistence rejection log.

Evidence reviewed

PR surface:

Source +7, Tests +26. Total +33 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	8	1	+7
Tests	1	26	0	+26
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	34	1	+33

What I checked:

Current main still swallows start persistence failures: Current main fire-and-forgets the start lifecycle persistence write with a rejection handler that discards the error, so the central problem is not already implemented on main. (src/gateway/server-chat.ts:1437, f9e194e4a122)
PR implementation mirrors the terminal sibling: The PR changes the start-phase catch to log session, run, and formatted error while preserving fire-and-forget semantics. (src/gateway/server-chat.ts:1439, e8f4c93c6643)
Sibling terminal path already logs the same class of failure: The terminal lifecycle persistence path logs rejected writes through logError and formatForLog before preserving fallback projection behavior. (src/gateway/server-chat.ts:720, f9e194e4a122)
Persistence callee can reject real write failures: persistGatewaySessionLifecycleEvent calls updateSessionStoreEntry with requireWriteSuccess: true, so storage write failures propagate to the handler catch path. (src/gateway/session-lifecycle-state.ts:279, f9e194e4a122)
Log redaction boundary is reused: formatForLog handles Error values by rendering the error chain, redacting sensitive text, and capping length before values reach gateway logs. (src/gateway/ws-log.ts:115, f9e194e4a122)
Regression test added at the affected boundary: The PR adds a focused agent-event-handler test that rejects the mocked start lifecycle persistence call and asserts the redacted gateway log is emitted exactly once. (src/gateway/server-chat.agent-events.test.ts:3177, e8f4c93c6643)

Likely related people:

LZY3538: Authored the merged terminal-phase persistence logging PR that established the sibling gateway invariant this PR mirrors. (role: sibling fix author; confidence: high; commits: 7d593bb9ae54; files: src/gateway/server-chat.ts, src/gateway/server-chat.agent-events.test.ts)
steipete: Appears in recent gateway session lifecycle and state-helper history, and authored the final terminal log identity change included in the related merged PR. (role: recent adjacent contributor and merger; confidence: high; commits: 715a99416658, 4d8502804d70, b6f4cf5ac432; files: src/gateway/server-chat.ts, src/gateway/session-lifecycle-state.ts, src/gateway/ws-log.ts)
ooiuuii: Recent merged restart-recovery work touched the gateway server-chat and session lifecycle area adjacent to this persistence/recovery accounting path. (role: feature-history contributor; confidence: medium; commits: d20fdf3b3842; files: src/gateway/server-chat.ts, src/gateway/session-lifecycle-state.ts)
hexsprite: Authored adjacent gateway lifecycle error formatting/redaction work that the new diagnostic relies on through formatForLog. (role: adjacent formatter contributor; confidence: medium; commits: 86c1622a3a3c; files: src/gateway/ws-log.ts, src/gateway/ws-log.test.ts)

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.

Review history (1 earlier review cycle)

reviewed 2026-07-05T12:13:16.356Z sha 64cca25 :: needs real behavior proof before merge. :: none

The gateway session-lifecycle "start" event persistence (persistGatewaySessionLifecycleEvent, which surfaces write failures via requireWriteSuccess) was fired as void ...catch(() => undefined), swallowing the rejection with zero logging. A failed start-marker write silently dropped the run's start record from restart-recovery accounting, with no operator-visible trace. The sibling terminal-phase catch already logs this since openclaw#97839; the start path was the unfixed sibling. Mirror that fix: log the swallowed start-phase persistence failure with the same redacted message shape via formatForLog, keeping the fire-and-forget semantics unchanged. Adds a focused regression test asserting the log fires on start-persist rejection.

steipete · 2026-07-05T18:13:42Z

Thank you — this fix landed through maintainer batch PR #100399 in commit b22c36f, with contributor credit and changelog thanks preserved. Closing this source PR as superseded.

openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Jul 5, 2026

masatohoshino force-pushed the pr/gateway-start-session-persist-log branch from 64cca25 to e8f4c93 Compare July 5, 2026 12:44

steipete self-assigned this Jul 5, 2026

This was referenced Jul 5, 2026

fix: land ten small reliability fixes #100383

Closed

fix: land ten small reliability fixes #100395

Closed

fix: land ten small reliability fixes #100399

Merged

steipete closed this Jul 5, 2026

clawsweeper Bot mentioned this pull request Jul 7, 2026

[Bug]: Unawaited persistGatewaySessionLifecycleEvent swallows write failures and drops session start markers #101383

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(gateway): log start session persistence failures#100313

fix(gateway): log start session persistence failures#100313
masatohoshino wants to merge 1 commit into
openclaw:mainfrom
masatohoshino:pr/gateway-start-session-persist-log

masatohoshino commented Jul 5, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

masatohoshino commented Jul 5, 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

User Impact

Evidence

Real behavior proof (genuine read-only-filesystem rejection, no mocks)

Focused regression test

Static checks

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

masatohoshino commented Jul 5, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading