fix(sessions): recover store from stale temp artifacts safely by wAngByg · Pull Request #79910 · openclaw/openclaw

wAngByg · 2026-05-09T17:54:11Z

Summary

Problem: an interrupted session-store atomic write can leave the primary sessions.json zero-byte or malformed while an older writer temp artifact still contains valid session metadata.
Fix: loadSessionStore can now recover from stale session-store temp artifacts produced by the real atomic writer (sessions.json.<pid>.<uuid>.tmp and legacy sessions.json.<uuid>.tmp).
Safety change from earlier revisions: load-time recovery no longer promotes sessions.json.bak and no longer rewrites the primary store from the read path. This avoids inventing a .bak recovery contract and removes the race where recovery could overwrite a newer primary written concurrently.
TOCTOU hardening: candidate bytes are read through the opened file descriptor after fstat, so the integrity/staleness checks are bound to the file actually read.
What did NOT change: normal valid-store loading, the save hot path, the writer queue, and valid empty stores still short-circuit recovery.

Change Type

Scope

Linked Issue/PR

Closes [Bug] sessions.json can become 0 bytes (empty), breaking session aggregation with no recovery mechanism #77783

Real behavior proof

Behavior or issue addressed: corrupted or zero-byte primary session store load can recover usable session entries from stale atomic temp artifacts, without overwriting a newer primary store and without treating .bak as an automatic recovery source.
Real environment tested: Windows host, Node v24.15.0, pnpm 11.2.2 via Corepack, PR branch rebased onto current openclaw/openclaw:main, head 036916aa5a37fb883bf4a3198ec2ac77f67e3b50.
Exact steps or command run after this patch:

# Live filesystem behavior proof, not CI output.
node_modules\.bin\tsx -e "<script creates malformed sessions.json, stale sessions.json.<pid>.<uuid>.tmp, then calls real loadSessionStore>"

# Focused regression and hygiene checks after the same rebase.
node node_modules/vitest/vitest.mjs run src/config/sessions/store-load.recovery.test.ts --config test/vitest/vitest.runtime-config.config.ts --reporter verbose --pool forks --maxWorkers=1 --fileParallelism=false --testTimeout=10000 --hookTimeout=10000
node_modules\.bin\oxfmt --check --threads=1 src/config/sessions/store-load.ts src/config/sessions/store-load.recovery.test.ts
node_modules\.bin\oxlint src/config/sessions/store-load.ts src/config/sessions/store-load.recovery.test.ts

Evidence after fix: copied live console output from the real loadSessionStore path on the PR branch after rebase:

[sessions/store] loaded session store from stale tmp artifact
{
  "command": "node_modules/.bin/tsx -e <live session-store recovery script>",
  "tempArtifactShape": "sessions.json.13152.<uuid>.tmp",
  "recoveredKeys": [
    "live-recovered"
  ],
  "primaryBefore": "{bad json",
  "primaryAfter": "{bad json",
  "primaryRewrittenByLoad": false,
  "bakOnlyRecoveryKeys": [],
  "bakAutoPromoted": false
}

Observed result after fix: the actual loader returned the session from the stale atomic temp artifact, left the malformed primary file unchanged, and ignored .bak as a recovery source. Focused regression coverage also passed: store-load.recovery.test.ts 17/17, oxfmt --check passed for the changed files, and direct oxlint on the changed files exited successfully.
What was not tested: full repository CI on every platform was left to GitHub Actions; the focused local proof covered the session-store recovery behavior changed by this PR.

Notes for reviewers

This revision intentionally narrows the previous branch:

Removed automatic .bak promotion because current main does not have a demonstrated sessions.json.bak writer contract.
Removed load-time self-heal writes, so recovery cannot race a normal save and replace a newer valid primary with older recovered data.
Kept recovery useful by returning the recovered temp artifact in memory; any later normal persistence still goes through the existing session-store writer path.

clawsweeper · 2026-05-09T17:57:35Z

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 11:46 PM ET / 03:46 UTC.

Summary
The PR adds in-memory recovery from stale sessions.json.<pid>.<uuid>.tmp and legacy session-store temp artifacts when the primary session store is zero-byte or malformed.

PR surface: Source +174, Tests +254. Total +428 across 4 files.

Reproducibility: yes. from source inspection: current main returns the initialized empty store after a zero-byte or malformed sessions store instead of recovering from sibling temp artifacts. I did not run a local filesystem repro because this review is read-only.

Review metrics: 2 noteworthy metrics.

Session Recovery Fallback: 1 added in-memory fallback, 0 primary rewrites. The central behavior change returns recovered metadata from stale temp artifacts without rewriting the corrupt primary file.
Sibling Temp Lifecycle PR: 1 open related PR. The open stale-temp sweep PR touches the same artifact family and can affect whether recovery candidates still exist at load time.

Stored data model
Persistent data-model change detected: serialized state: src/config/sessions/store-load.recovery.test.ts, serialized state: src/config/sessions/store-load.ts, unknown-data-model-change: src/config/sessions/store-load.recovery.test.ts, unknown-data-model-change: src/config/sessions/store-load.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #77783
Summary: This PR is the active candidate fix for the canonical corrupt sessions.json recovery issue; earlier broad recovery attempts are superseded, while diagnostics and orphan-temp sweeping remain partial or adjacent work.

Members:

canonical: [Bug] sessions.json can become 0 bytes (empty), breaking session aggregation with no recovery mechanism #77783 - The PR body uses closing syntax and implements stale-temp recovery for the corrupt primary-store case reported there.
superseded: fix(sessions): recover session store from bak/tmp after corrupted write #77915 - Closed unmerged earlier recovery attempt included broader .bak, fresh-temp fallback, and primary rewrite behavior now removed from this narrower PR.
superseded: fix(sessions): recover store from backup and tmp artifacts #79319 - Closed unmerged backup/tmp recovery attempt was narrowed into this stale-temp-only branch.
partial_overlap: fix(sessions): sweep orphan store temp files #90503 - It touches the same stale session-store temp artifact family but solves orphan deletion rather than corrupt-primary recovery.
partial_overlap: feat(sessions): add read-only diagnose command #96113 - It targets read-only session diagnostics, which overlaps the canonical issue's diagnostic scope but not stale-temp recovery.
1 more in the report.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

Confirm whether this PR should close the broader linked issue or leave doctor/rebuild diagnostics as follow-up.
Coordinate merge order or combined behavior with fix(sessions): sweep orphan store temp files #90503.

Risk before merge

[P2] Recovering from the newest validated stale temp can surface older session metadata when the primary store is unusable; maintainers need to accept that fallback semantic explicitly.
[P1] The linked canonical issue at [Bug] sessions.json can become 0 bytes (empty), breaking session aggregation with no recovery mechanism #77783 also asks for rebuild-from-session-directory and doctor diagnostics, while this PR intentionally implements only stale-temp recovery.
[P1] Open PR fix(sessions): sweep orphan store temp files #90503 deletes stale session-store temp files on load; if both land, recovery must inspect candidates before deletion or the sweep must preserve candidates while the primary is corrupt.
[P1] Open PR feat(sessions): add read-only diagnose command #96113 appears to target read-only session diagnostics, so this recovery PR should not be treated as owning all diagnostic scope without maintainer confirmation.

Maintainer options:

Approve Narrow Recovery Semantics (recommended)
A maintainer can accept returning the newest validated stale temp as the intended in-memory fallback for an unusable primary store and decide how much of the linked broader issue this closes.
Coordinate With Temp Sweep
Before landing both PRs, require the sweep path to preserve recovery candidates or run only after corrupt-primary recovery has had a chance to inspect them.
Pause For Durable Recovery Design
Pause this branch if maintainers want doctor diagnostics, transcript rebuild, quarantine, or durable self-heal in the first accepted recovery path.

Next step before merge

[P2] The remaining action is maintainer acceptance of recovery semantics, linked-issue scope, and temp-sweep coordination rather than a narrow automated repair.

Security
Cleared: No concrete security or supply-chain concern found; the diff reads local same-directory temp artifacts with regular-file, hardlink, staleness, and fd-bound checks and adds no dependencies, commands, network calls, or secret handling.

Review details

Best possible solution:

Land the narrowed stale-temp recovery after maintainers accept the fallback semantics, coordinate it with stale-temp deletion behavior, and track doctor/rebuild diagnostics separately if still required.

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

Yes from source inspection: current main returns the initialized empty store after a zero-byte or malformed sessions store instead of recovering from sibling temp artifacts. I did not run a local filesystem repro because this review is read-only.

Is this the best way to solve the issue?

Yes for the narrowed stale-temp failure, with maintainer acceptance required. The PR uses the existing atomic-writer artifact contract and avoids .bak promotion or primary rewrites, but it does not implement the full doctor/rebuild recovery requested by the linked issue.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P1: The PR addresses a corrupt core session metadata store failure that can break session listing, recovery, and active channel workflows.
merge-risk: 🚨 session-state: The diff changes session metadata loading to return state from a stale temp artifact when the primary store is empty or malformed.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live Windows console output from the real loadSessionStore path showing stale-temp recovery, unchanged primary bytes, .bak refusal, and focused checks.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live Windows console output from the real loadSessionStore path showing stale-temp recovery, unchanged primary bytes, .bak refusal, and focused checks.

Evidence reviewed

PR surface:

Source +174, Tests +254. Total +428 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	182	8	+174
Tests	1	254	0	+254
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	436	8	+428

What I checked:

Repository policy applied: Root AGENTS.md was read fully; its session-state, storage, proof, and whole-surface PR review guidance applies to this session-store PR. (AGENTS.md:24, 9d68f877ac3e)
Current main corrupt-store behavior: Current loadSessionStore initializes an empty store and catches final read/parse failure, so zero-byte or malformed primary content still collapses to an empty in-memory store on main. (src/config/sessions/store-load.ts:397, 9d68f877ac3e)
Existing atomic temp contract: Current main writes the session store through writeTextAtomic with tempPrefix: path.basename(storePath), producing the temp artifact family the PR recovers from. (src/config/sessions/store.ts:1659, 9d68f877ac3e)
Existing temp matcher contract: Current main has a basename-aware matcher for current and legacy session-store temp names, which the PR reuses as the recovery candidate boundary. (src/config/sessions/artifacts.ts:53, 9d68f877ac3e)
PR recovery implementation: The PR only runs recovery after the primary file was readable but unusable, rejects .bak, validates stale temp candidates, reads through the opened file descriptor, and avoids cache ownership for recovered data. (src/config/sessions/store-load.ts:484, 036916aa5a37)
PR regression coverage: The added tests cover stale current and legacy temp recovery, .bak refusal, no primary rewrite, concurrent newer-primary preservation, fd-bound reads, fresh-temp refusal, and invalid candidate skipping. (src/config/sessions/store-load.recovery.test.ts:80, 036916aa5a37)

Likely related people:

openperf: Merged PR 85967 added store-specific temp names, the artifact matcher, and stale-temp cleanup primitives that this PR builds on. (role: introduced temp-artifact contract; confidence: high; commits: 89aea9b84333; files: src/config/sessions/store.ts, src/config/sessions/artifacts.ts, src/config/sessions/disk-budget.ts)
Shakker: Commit c2e93c7 moved the session-store loading path into the current store-load module, including the empty-store fallback surface this PR changes. (role: introduced loader split; confidence: medium; commits: c2e93c76bd43; files: src/config/sessions/store-load.ts, src/config/sessions/store.ts)
steipete: PR 91322 substantially refactored session metadata storage around the same loader/writer boundary. (role: adjacent session-storage contributor and merger; confidence: medium; commits: 538d36eaaaa6; files: src/config/sessions/store-load.ts, src/config/sessions/store.ts)
vincentkoc: Commit 7f1d82a reverted/deferred the SQLite session metadata path and preserved the current file-backed session-store surface this PR repairs. (role: recent adjacent contributor; confidence: medium; commits: 7f1d82ab2518; files: src/config/sessions/store-load.ts, src/config/sessions/store.ts, src/config/sessions/disk-budget.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.

MertBasar0 · 2026-05-11T00:35:03Z

Re-review would be useful after the latest fixes here.

The remaining items from the previous review looked narrow and file-local:

validate .bak recovery candidates before reading, using the same local regular-file/symlink/hardlink
safety gate as tmp candidates
align the fs-safe temp recovery fixtures with the pinned .fs-safe-replace.<pid>.<uuid>.t mp writer
contract

If those are now addressed, this looks like a small, mergeable recovery hardening PR with useful
real-behavior proof

clawsweeper · 2026-05-16T15:20:44Z

🦞🧹
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.

Re-review progress:

State: Failed
Detail: The targeted re-review did not finish cleanly. Check the workflow run for details.
Run: https://github.com/openclaw/clawsweeper/actions/runs/25965769419
Updated: 2026-05-16T15:36:47.694Z

clawsweeper · 2026-05-16T17:21:09Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/25968183066
Updated: 2026-05-16T17:27:55.723Z

clawsweeper · 2026-05-17T01:01:01Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/25977503433
Updated: 2026-05-17T01:07:17.246Z

clawsweeper · 2026-05-26T10:43:42Z

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

clawsweeper · 2026-05-26T11:17:40Z

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

RomneyDa · 2026-05-29T06:31:00Z

Heads up: this PR needs to be updated against current main before the new required Dependency Guard check can pass.

clawsweeper · 2026-05-30T05:16:38Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26675481162
Updated: 2026-05-30T05:30:11.661Z

clawsweeper · 2026-05-31T09:34:57Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26709097124
Updated: 2026-05-31T09:45:05.695Z

clawsweeper · 2026-05-31T10:29:34Z

🦞🧹
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.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26710152583
Updated: 2026-05-31T10:37:11.038Z

wAngByg · 2026-06-11T02:28:16Z

@clawsweeper re-review

P1/P2 findings addressed in 036916aa5a37fb883bf4a3198ec2ac77f67e3b50:

removed load-time self-heal writes, so recovery can no longer overwrite a newer primary installed by a concurrent normal save;
removed automatic sessions.json.bak promotion because current main does not define a .bak writer contract;
read temp recovery candidates through an opened fd with fstat/staleness validation bound to the bytes read;
added regressions for no primary rewrite, no concurrent-newer-primary overwrite, fd-bound candidate reads, .bak ignored, and stale tmp recovery.

Current-head proof/checks: Real behavior proof is green; focused store-load.recovery.test.ts passed 17/17; oxfmt --check and direct oxlint passed for the changed files. The remaining red checks-node-agentic-* shards are unrelated src/agents/* tests and also reproduce on #80681.

clawsweeper · 2026-06-11T02:28:19Z

🦞🧹
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.

steipete · 2026-07-11T23:08:29Z

Obsolete: the runtime no longer performs atomic sessions.json writes, so new stale .tmp artifacts cannot accumulate; the legacy sweep that remains (disk-budget artifact cleanup) already reclaims old ones.
Sessions and transcripts moved from JSON/JSONL files to per-agent SQLite on main (0a8e3604ba24, plus follow-up 4eef09dc3f5d), and the storage rework removed or rewrote the code paths implicated here. We're closing storage-implicated reports optimistically as part of that cleanup since we can't re-verify every historical environment. If this still reproduces on current main or the next release, please open a fresh issue with new logs and link back to this one.

openclaw-barnacle Bot added size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 9, 2026

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 10, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 10, 2026

wAngByg force-pushed the new-pr-session-recovery branch from 337c561 to 52e391d Compare May 10, 2026 08:24

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 10, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 10, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 10, 2026

wAngByg force-pushed the new-pr-session-recovery branch from 390a36b to 4e8f6e1 Compare May 10, 2026 11:12

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 10, 2026

clawsweeper Bot mentioned this pull request May 10, 2026

[Bug]: Telegram channel can be wedged by bad persisted session JSON; status should detect and offer recovery #43549

Open

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 10, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 11, 2026

wAngByg force-pushed the new-pr-session-recovery branch from 4e8f6e1 to fefaab1 Compare May 11, 2026 02:36

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 11, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 11, 2026

github-actions Bot mentioned this pull request May 11, 2026

🦞 OpenClaw 生态日报 2026-05-11 ivanweng2077/big_model_radar#26

Open

wAngByg force-pushed the new-pr-session-recovery branch from fefaab1 to 83d2b00 Compare May 11, 2026 03:40