fix(backup): sweep stale temp artifacts before creating new backup by zhfnini-rgb · Pull Request #95600 · openclaw/openclaw

zhfnini-rgb · 2026-06-21T17:30:49Z

What Problem This Solves

Fixes an issue where interrupted backup create runs can leave stale backup staging directories and full-size temporary archive files behind. A hard kill, OOM, host reboot, or process restart skips the in-process cleanup block, so these artifacts can accumulate and consume disk space across later backup runs.

Fixes #95582

Why This Change Was Made

The backup startup path now sweeps only OpenClaw-owned stale backup artifacts older than one hour before creating a new archive. The sweep is scoped to the selected backup staging temp root for openclaw-backup-* directories and to the requested output path's sibling temp files matching <output basename>.<uuid>.tmp, avoiding broad deletion of unrelated shared-temp files.

Active backup runs now write owner markers for both artifact classes. Startup cleanup checks those owner markers and skips artifacts whose owner PID is still live, so long-running or concurrent backups are not removed by an age-only sweep.

User Impact

Users and operators can rerun backup create after a killed or interrupted backup and have old OpenClaw backup temp artifacts cleaned automatically before the new backup starts. Recent artifacts and artifacts owned by a live backup process remain protected.

Evidence

pnpm install --frozen-lockfile
pnpm exec vitest run -c test/vitest/vitest.infra.config.ts src/infra/backup-create.test.ts -t "sweepStaleBackupArtifacts" -> 7 passed, 38 skipped
git diff --check -> passed
Real backup proof with a temporary HOME/state/config and seeded stale artifacts:

beforeStagingExists=True
beforeArchiveTempExists=True
Backup archive: <temp>\backups\proof-backup.tar.gz
Included 2 paths:
- state: <temp>\state
- config: <temp>\openclaw.json
Created <temp>\backups\proof-backup.tar.gz
afterStagingExists=False
afterArchiveTempExists=False
backupExists=True
backupBytes=19407

Earlier broader Windows validation note: node scripts/run-vitest.mjs src/infra/backup-create.test.ts reached 28 passed before existing createBackupArchive tests failed on Windows with EBUSY unlinking temp SQLite files during test cleanup.
Earlier broader Windows validation note: node scripts/run-vitest.mjs src/commands/backup.atomic.test.ts src/commands/backup.test.ts had backup.atomic.test.ts pass; backup.test.ts had 13 passed and 1 Windows EBUSY cleanup failure on a temp SQLite WAL file.

clawsweeper · 2026-06-21T17:32:27Z

Codex review: needs real behavior proof before merge. Reviewed June 29, 2026, 3:26 AM ET / 07:26 UTC.

Summary
The PR adds a pre-backup sweep for stale openclaw-backup-* staging directories and output-adjacent <basename>.<uuid>.tmp archives, with owner sidecars and regression tests.

PR surface: Source +157, Tests +148. Total +305 across 2 files.

Reproducibility: yes. source-level: current main uses timestamped default/directory backup basenames, while the PR only sweeps archive temp files matching the current run's basename. I did not run a destructive hard-kill repro in this read-only review.

Review metrics: 2 noteworthy metrics.

Destructive Cleanup Scope: 2 artifact classes swept; 2 owner sidecars written. The PR deletes stale staging directories and archive temps before writing a backup, so the exact matching and live-owner rules matter before merge.
Default Output Sweep Coverage: 0 tests cover prior timestamped output basenames. The documented default and directory-output flows generate a new basename per run, which is the path currently missed by the diff.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95582
Summary: This PR is a candidate fix for the canonical two-artifact interrupted-backup cleanup issue, but the current branch misses one central archive-temp path for timestamped outputs.

Members:

canonical: backup create leaves multi-GB intermediate temp + staging dir orphaned in tmpdir after a hard kill (no startup sweep of prior runs) #95582 - The PR body uses closing syntax for this issue, and the issue describes stale staging dirs plus output-adjacent archive temps after hard kill.
partial_overlap: fix(backup): clean up stale .tmp archives from interrupted runs before creating new backup #91663 - That open PR targets stale output-adjacent backup .tmp archives but not the complete two-artifact owner-marker cleanup here.
superseded: [Bug] backup create leaves large .tmp files on timeout causing disk space exhaustion #50442 - That closed issue preserves earlier disk-exhaustion evidence for stale backup .tmp files and was superseded by the broader canonical issue.
partial_overlap: openclaw backup create --verify exits 13, leaves corrupt .tmp archive; verifier also rejects older backup hardlinks #89257 - That issue includes corrupt leftover backup temp archives, but also tracks separate --verify exit and verifier compatibility symptoms.
adjacent_distinct: fix(memory): clean stale reindex temp files #92891 - That merged PR fixed the same crash-left-temp cleanup pattern in memory reindexing, not backup creation.
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: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

[P1] Fix archive-temp matching for prior timestamped default and directory-output basenames under the existing age and owner guards.
[P1] Add regression coverage and redacted terminal/live output showing a stale prior timestamped .tar.gz.<uuid>.tmp is removed on a later backup run.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body's live output proves cleanup for a fixed output basename, but it does not show the default or directory-output path where the stale archive temp basename changes between runs. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] Merging as-is leaves the full-size archive-temp leak partially unresolved for the documented default and directory-output backup flows because prior timestamped basenames are not matched.
[P1] The PR intentionally makes one-hour-old matching OpenClaw backup temp artifacts disposable, so maintainers should accept that stale partial archives and staging dirs are not a recovery/debugging contract.
[P1] PID owner sidecars reduce active-writer risk, but PID reuse or permission-limited process checks are not a perfect lease across all hosts.

Maintainer options:

Fix Timestamped Temp Matching (recommended)
Before merge, broaden the archive-temp sweep enough to remove prior OpenClaw timestamped backup temps while keeping UUID, owner, age, and output-directory scoping tight.
Accept Fixed-Path Only Cleanup
Maintainers could intentionally accept that this PR only cleans archive temps for stable file-output basenames and leaves default timestamped archive temps to a follow-up.
Pause For Cleanup Policy
Pause or close if maintainers do not want backup startup to delete stale temp artifacts without a different recovery or debugging policy.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Update the backup stale-temp sweep so a later default or `--output <directory>` run removes older `<timestamp>-openclaw-backup.tar.gz.<uuid>.tmp` files in the selected output directory under the existing one-hour grace and live-owner checks; add focused tests and real terminal/live proof for that path.

Next step before merge

[P1] A narrow automated repair can address the exact-basename bug; maintainer cleanup-policy acceptance remains after the code path covers the documented backup flows.

Security
Cleared: No dependency, workflow, credential, or supply-chain regression was found; the risky behavior is destructive file cleanup and is covered as compatibility and availability risk.

Review findings

[P1] Sweep prior timestamped backup temp names — src/infra/backup-create.ts:747-749

Review details

Best possible solution:

Repair the archive-temp matcher to cover prior OpenClaw timestamped backup temp basenames under the same age and owner guards, then land after maintainer acceptance of the cleanup policy.

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

Yes, source-level: current main uses timestamped default/directory backup basenames, while the PR only sweeps archive temp files matching the current run's basename. I did not run a destructive hard-kill repro in this read-only review.

Is this the best way to solve the issue?

No, not as submitted. The startup sweep is the right layer, but the archive-temp matching is too narrow for the documented default and directory-output flows.

Full review comments:

[P1] Sweep prior timestamped backup temp names — src/infra/backup-create.ts:747-749
The cleanup only matches <current output basename>.<uuid>.tmp. For the documented default and --output <directory> flows, resolveOutputPath() creates a new timestamped basename on every run, so a later backup will not remove the stale archive temp left by a prior run. That leaves the multi-GB .tmp leak unresolved for the main user path; match prior OpenClaw backup basenames under the same age and owner guards.
Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: The PR targets a real backup disk-pressure bug with bounded blast radius around the backup CLI and SDK backup helper.
merge-risk: 🚨 compatibility: Existing operators may currently inspect or manually recover old matching backup temp artifacts that this PR would remove automatically after the grace window.
merge-risk: 🚨 availability: Incorrect stale-temp matching or owner detection can leave the disk-pressure failure unresolved or disrupt an overlapping backup run.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body's live output proves cleanup for a fixed output basename, but it does not show the default or directory-output path where the stale archive temp basename changes between runs. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +157, Tests +148. Total +305 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	157	0	+157
Tests	1	149	1	+148
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	306	1	+305

Acceptance criteria:

[P1] node scripts/run-vitest.mjs src/infra/backup-create.test.ts.
[P1] git diff --check.

What I checked:

AGENTS policy read: Root AGENTS.md was read fully and applied; no scoped AGENTS.md owns src/infra. (AGENTS.md:1, 0ce10d779302)
PR head exact-basename matcher: The PR sweep derives outputBaseName from the current outputPath and only removes archive temp files matching that exact basename plus UUID suffix. (src/infra/backup-create.ts:733, 300d72b58f96)
Current default output is timestamped: Current main builds a timestamped backup basename when no output is given, and directory outputs also receive that fresh basename, so a later run normally has a different basename from a prior failed run. (src/infra/backup-create.ts:204, 0ce10d779302)
Documented main user path: The backup docs show openclaw backup create and openclaw backup create --output ~/Backups; the notes state the default output is a timestamped .tar.gz archive. Public docs: docs/cli/backup.md. (docs/cli/backup.md:14, 0ce10d779302)
PR tests miss timestamp-changing output: The added sweep tests use a fixed backup.tar.gz output basename, so they do not exercise stale archive temps left by previous timestamped default or directory-output runs. (src/infra/backup-create.test.ts:1219, 300d72b58f96)
Current main still has the hard-kill leak shape: Current main creates openclaw-backup-* staging dirs and output-adjacent UUID temp archives, then removes them only in the normal finally path. (src/infra/backup-create.ts:751, 0ce10d779302)

Likely related people:

shichangs: Live PR metadata for the original local backup CLI shows this handle authored the PR that added backup create, backup docs, and the initial atomic backup flow. (role: introduced backup CLI behavior; confidence: high; commits: 0ecfd37b4465; files: src/commands/backup.ts, src/commands/backup.atomic.test.ts, docs/cli/backup.md)
gumadeiras: The original backup CLI PR was merged by this handle, and commit 3ba6491 extracted backup creation into src/infra/backup-create.ts. (role: backup infra refactor contributor and original PR merger; confidence: high; commits: 0ecfd37b4465, 3ba64916599b; files: src/commands/backup.ts, src/infra/backup-create.ts, src/commands/backup-shared.ts)
vincentkoc: Recent backup-path history includes backup test fixture and atomic backup setup changes around the current infra/test surface. (role: recent backup test area contributor; confidence: medium; commits: bf6116af3f, c9e12cbd32, 99e3d4a069; files: src/infra/backup-create.test.ts, src/commands/backup.atomic.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.

zhfnini-rgb · 2026-06-21T18:30:17Z

@clawsweeper re-review

clawsweeper · 2026-06-21T18:30:20Z

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

…w backup On hard kill (SIGKILL, OOM, host reboot), the in-process finally block in createBackupArchive cannot fire, orphaning multi-GB staging dirs (openclaw-backup-*) and temp archives (*.uuid.tmp) in os.tmpdir(). Nothing reaps them, so they accumulate across interrupted runs. Add sweepStaleBackupArtifacts() that runs at the start of each backup create, removing the tool's own stale artifacts older than a 1-hour grace window. This follows the same pattern as openclaw#92891 (memory backend reindex temps). Fixes openclaw#95582

zhfnini-rgb · 2026-06-27T13:44:41Z

Hi maintainers, quick status update on this PR:

GitHub currently reports it as mergeable / clean.
All current checks on 300d72b58f9675560a64bca36f392c7bf0a76d05 are completed with no failures.
The automated review has no contributor-facing blocker left; the remaining decision is maintainer acceptance of the guarded cleanup policy for stale backup temp artifacts.

Could a maintainer take a look when available?

barnacle-openclaw · 2026-07-22T04:44:07Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

openclaw-barnacle Bot added size: S triage: blank-template Candidate: PR template appears mostly untouched. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 21, 2026

zhfnini-rgb force-pushed the fix/backup-sweep-stale-temps branch from 04e6ef6 to 6fe6f0f Compare June 21, 2026 18:03

zhfnini-rgb changed the title ~~fix(backup): sweep stale temp artifacts on startup before creating ne…~~ fix(backup): sweep stale temp artifacts before creating new backup Jun 21, 2026

openclaw-barnacle Bot added size: M and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. size: S labels Jun 21, 2026

totobusnello mentioned this pull request Jun 21, 2026

fix(agents): preserve streamed assistant text when Claude CLI result event is empty #90450

Merged

3 tasks

zhfnini-rgb added 2 commits June 22, 2026 13:40

fix(backup): guard stale artifact sweep owners

300d72b

zhfnini-rgb force-pushed the fix/backup-sweep-stale-temps branch from 57750e7 to 300d72b Compare June 22, 2026 05:50

This was referenced Jun 25, 2026

[Bug] backup create leaves large .tmp files on timeout causing disk space exhaustion #50442

Closed

fix(backup): clean up stale .tmp archives from interrupted runs before creating new backup #91663

Closed

clawsweeper Bot mentioned this pull request Jun 28, 2026

fix(backup): handle ENOENT race when session files are deleted during archive creation (fixes #67417) (AI-assisted) #96911

Closed

1 task

clawsweeper Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jul 1, 2026

barnacle-openclaw Bot added the stale Marked as stale due to inactivity label Jul 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(backup): sweep stale temp artifacts before creating new backup#95600

fix(backup): sweep stale temp artifacts before creating new backup#95600
zhfnini-rgb wants to merge 2 commits into
openclaw:mainfrom
zhfnini-rgb:fix/backup-sweep-stale-temps

zhfnini-rgb commented Jun 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 21, 2026 •

edited

Loading

Uh oh!

zhfnini-rgb commented Jun 21, 2026

Uh oh!

clawsweeper Bot commented Jun 21, 2026 •

edited

Loading

Uh oh!

zhfnini-rgb commented Jun 27, 2026

Uh oh!

barnacle-openclaw Bot commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

zhfnini-rgb commented Jun 21, 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

Uh oh!

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

Uh oh!

Uh oh!

zhfnini-rgb commented Jun 21, 2026

Uh oh!

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

Uh oh!

Uh oh!

zhfnini-rgb commented Jun 27, 2026

Uh oh!

barnacle-openclaw Bot commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zhfnini-rgb commented Jun 21, 2026 •

edited

Loading

clawsweeper Bot commented Jun 21, 2026 •

edited

Loading

clawsweeper Bot commented Jun 21, 2026 •

edited

Loading