Skip to content

fix(sessions): sweep orphan store temp files#90503

Merged
steipete merged 1 commit into
openclaw:mainfrom
sahibzada-allahyar:fastino-89520-atomic-tmp-sweep-b
Jul 6, 2026
Merged

fix(sessions): sweep orphan store temp files#90503
steipete merged 1 commit into
openclaw:mainfrom
sahibzada-allahyar:fastino-89520-atomic-tmp-sweep-b

Conversation

@sahibzada-allahyar

@sahibzada-allahyar sahibzada-allahyar commented Jun 5, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Fixes #89520.

What Problem This Solves

A hard-killed atomic session-store write can leave sessions.json.<pid>.<uuid>.tmp artifacts behind indefinitely. Repeated crashes accumulate files in each agent session directory.

Why This Change Was Made

The original PR scanned the session directory during uncached store reads. This maintainer rewrite moves cleanup to the existing Gateway/embedded-TUI startup migration lifecycle, so ordinary reads stay free of directory I/O.

The startup pass:

  • reuses the canonical session-store target discovery and the existing five-minute stale threshold;
  • bounds stat/delete concurrency;
  • removes only matching stale atomic-write temps;
  • preserves every candidate if the primary store is missing, corrupt, or not a JSON object;
  • isolates discovery/cleanup errors so startup still proceeds.

User Impact

Crash-orphaned session temp files are reclaimed on the next Gateway or embedded-TUI startup without risking the only recoverable copy and without adding work to session reads.

Evidence

  • 91 focused session, Gateway-startup, and embedded-TUI tests passed.
  • Full pnpm build passed.
  • pnpm check:changed passed for all six touched files, including core and core-test typechecks.
  • Blacksmith Testbox lease: tbx_01kww1cytcgctj7rzjm3g6rczw.
  • Live probe used the real atomic writer, killed it with SIGKILL after temp staging, backdated the orphan, and verified the startup migration removed it.
  • Fresh autoreview reported no actionable findings.

@openclaw-barnacle openclaw-barnacle Bot added size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 5, 2026
@clawsweeper

clawsweeper Bot commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 12:05 PM ET / 16:05 UTC.

Summary
The PR adds a synchronous session-store load/cache-miss sweep for stale <store>.<pid>.<uuid>.tmp files and focused tests for stale, fresh, unrelated, missing-directory, and custom-basename cases.

PR surface: Source +53, Tests +75. Total +128 across 2 files.

Reproducibility: yes. at source level. Current main creates identifiable session-store atomic temp files and has explicit stale-temp cleanup primitives, but ordinary store load does not automatically sweep them; the PR body also supplies a real checkout probe for the proposed sweep.

Review metrics: 1 noteworthy metric.

  • Runtime Deletion Path: 1 added. The diff adds one production unlink sweep to the session-store load path, so the cleanup boundary needs maintainer attention before merge.

Stored data model
Persistent data-model change detected: serialized state: src/config/sessions/store-load.test.ts, serialized state: src/config/sessions/store-load.ts, unknown-data-model-change: src/config/sessions/store-load.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: #89520
Summary: This PR is the focused candidate fix for the canonical automatic session-store temp sweep report.

Members:

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:

  • none.

Risk before merge

  • [P1] The PR adds a production unlink path during session-store load/cache miss, so maintainers should explicitly accept deleting stale matching temp files from active session directories at that boundary.
  • [P1] The PR repeats the five-minute stale-temp cutoff already used by disk-budget cleanup, so the two cleanup paths could drift unless maintainers accept the duplication or ask for a shared rule.

Maintainer options:

  1. Accept Load-Time Cleanup Boundary (recommended)
    If maintainers accept deleting stale matching temp files during session-store load, proceed with normal merge validation.
  2. Share The Stale-Temp Rule First
    Extract or reuse the existing five-minute stale-temp rule so disk-budget cleanup and load-time cleanup cannot diverge.
  3. Pause For Explicit Cleanup-Only Policy
    Pause or close this PR if maintainers want stale temp deletion limited to doctor, explicit cleanup, or disk-budget flows rather than ordinary load.

Next step before merge

  • [P2] The remaining action is maintainer acceptance of the production session-load deletion boundary, not an automated code repair.

Security
Cleared: No concrete security or supply-chain concern was found; the diff touches local session-store cleanup code and tests only.

Review details

Best possible solution:

Land the narrow load-time sweep if maintainers accept the session-state cleanup boundary, ideally sharing the existing stale-temp window with disk-budget cleanup to prevent drift.

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

Yes, at source level. Current main creates identifiable session-store atomic temp files and has explicit stale-temp cleanup primitives, but ordinary store load does not automatically sweep them; the PR body also supplies a real checkout probe for the proposed sweep.

Is this the best way to solve the issue?

Yes, this is a narrow maintainable fix for the linked leak because it covers the active store path wherever the store is first loaded. Sharing the existing stale-temp cutoff would make the implementation cleaner, but no line-level correctness bug was found.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR addresses a bounded but production-observed session-store disk accumulation bug with a focused code change and tests.
  • merge-risk: 🚨 session-state: The diff deletes stale atomic-write sidecars from production session directories, so an incorrect cleanup boundary could affect session persistence.
  • 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 (terminal): The PR body includes terminal output from a real checkout probe showing the changed sweep removes a stale matching temp while preserving fresh and unrelated files.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a real checkout probe showing the changed sweep removes a stale matching temp while preserving fresh and unrelated files.
Evidence reviewed

PR surface:

Source +53, Tests +75. Total +128 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 53 0 +53
Tests 1 75 0 +75
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 128 0 +128

What I checked:

  • Repository policy read: Root AGENTS.md was read fully and applied; no scoped AGENTS.md exists under src/config, so root session-state/storage review policy governs this PR. (AGENTS.md:1, 6cb82eaab865)
  • PR diff adds load-time cleanup: The PR adds sweepOrphanSessionStoreTemps and calls it before the session-store disk read on cache miss. (src/config/sessions/store-load.ts:375, c68ca1c842f9)
  • Current main still lacks automatic load sweep: Current main's loadSessionStore goes from cache lookup into file read/retry without any store-temp sweep, so the central requested behavior is not already implemented on main. (src/config/sessions/store-load.ts:378, 6cb82eaab865)
  • Current main has the basename-aware temp matcher: isSessionStoreTempArtifactName already recognizes current <store>.<pid>.<uuid>.tmp and legacy <store>.<uuid>.tmp names while tracking custom store basenames. (src/config/sessions/artifacts.ts:59, 6cb82eaab865)
  • Existing cleanup is explicit, not load-time: Disk-budget and unreferenced-artifact cleanup already reclaim stale store temps using a five-minute window, but those paths are explicit cleanup flows rather than ordinary store load. (src/config/sessions/disk-budget.ts:312, 6cb82eaab865)
  • Current writer creates matchable temp names: Session-store writes pass tempPrefix: path.basename(params.storePath), producing the temp files this PR intends to sweep after a crash between write and rename. (src/config/sessions/store.ts:1684, 6cb82eaab865)

Likely related people:

  • openperf: Authored the merged session-store temp naming, matcher, and stale-temp reclaim work reused by this PR. (role: recent session temp cleanup author; confidence: high; commits: 89aea9b84333; files: src/config/sessions/artifacts.ts, src/config/sessions/disk-budget.ts, src/config/sessions/store.ts)
  • steipete: Merged PR 85967 and has repeated recent commits in session-store load, artifact, disk-budget, and store-facade files. (role: recent adjacent contributor and merger; confidence: high; commits: 89aea9b84333, 538d36eaaaa6, 3f16b96ddc08; files: src/config/sessions/store-load.ts, src/config/sessions/artifacts.ts, src/config/sessions/disk-budget.ts)
  • jalehman: Recent main history shows multiple session lifecycle and accessor seam changes in the same store-facade ownership area. (role: recent session seam contributor; confidence: medium; commits: ef47dd610c87, 127e174c9e4d, c41bc58cf612; files: src/config/sessions/store.ts, src/config/sessions/session-accessor.ts)
  • vincentkoc: Recent main history includes session-store load and store-facade changes, including the post-SQLite revert that preserved current file-backed behavior. (role: recent adjacent contributor; confidence: medium; commits: 7f1d82ab2518, 09a159c9139d; 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.
Review history (1 earlier review cycle)
  • reviewed 2026-06-21T18:17:02.475Z sha c68ca1c :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 5, 2026
@sahibzada-allahyar

sahibzada-allahyar commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 5, 2026
@hxy91819 hxy91819 mentioned this pull request Jun 5, 2026
Merged
@clawsweeper clawsweeper Bot mentioned this pull request Jun 15, 2026
Closed
@openclaw-clownfish openclaw-clownfish Bot mentioned this pull request Jun 15, 2026
Closed
3 tasks
@clawsweeper clawsweeper Bot added merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. and removed merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 20, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 24, 2026
Closed
14 tasks
@sahibzada-allahyar

sahibzada-allahyar commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

This is green and mergeable — it sweeps orphaned atomic-write temp files at gateway startup so stale *.tmp artifacts can't accumulate. Would appreciate a review when someone has bandwidth.

@clawsweeper clawsweeper Bot mentioned this pull request Jul 5, 2026
Closed
@steipete steipete self-assigned this Jul 6, 2026
@steipete
steipete force-pushed the fastino-89520-atomic-tmp-sweep-b branch from c68ca1c to 1ec6f9c Compare July 6, 2026 16:00
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: M and removed size: S labels Jul 6, 2026
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Maintainer rewrite and land-ready verification complete.

  • Moved cleanup out of loadSessionStore into the shared Gateway/embedded-TUI startup migration.
  • Preserves stale temps whenever the primary store is missing or invalid; fresh/unrelated files remain untouched.
  • Bounded filesystem concurrency and reused the canonical five-minute stale threshold.

Proof:

Contributor credit is preserved in the rewritten commit and PR.

@steipete
steipete merged commit 5e22787 into openclaw:main Jul 6, 2026
116 of 119 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 6, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 7, 2026
@sahibzada-allahyar @steipete
fix(sessions): clean stale atomic-write temps on startup (openclaw#90503
71c25f3 
)

Co-authored-by: Peter Steinberger <[email protected]>
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
@sahibzada-allahyar @steipete @giodl73-repo
fix(sessions): clean stale atomic-write temps on startup (openclaw#90503
352b506 
)

Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

gateway Gateway runtime merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Atomic write helper leaks orphan .tmp files; gateway never sweeps them (51 GB observed in the wild)

2 participants

@sahibzada-allahyar @steipete