fix(sessions): sweep orphan store temp files#90503
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 4, 2026, 9:53 PM ET / 01:53 UTC. Summary PR surface: Source +53, Tests +75. Total +128 across 2 files. Reproducibility: yes. source-reproducible: current session writes stage matching temp files, and current main lacks an unconditional load-time sweep, so a stale temp beside sessions.json can remain until cleanup or disk-budget maintenance runs. I did not run a hard-kill reproduction because this review is read-only. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the narrow load-time sweep using the existing temp filename contract and freshness guard; treat periodic cleanup as a separate defense-in-depth follow-up only if maintainers want it. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current session writes stage matching temp files, and current main lacks an unconditional load-time sweep, so a stale temp beside sessions.json can remain until cleanup or disk-budget maintenance runs. I did not run a hard-kill reproduction because this review is read-only. Is this the best way to solve the issue? Yes, this is an acceptable minimal fix: it reuses the existing session-store temp filename predicate and preserves fresh temps with the same five-minute safety window current cleanup already uses. A broader periodic sweep can be considered separately, but it is not required for this PR to solve startup cleanup. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 6868cde4d45f. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +53, Tests +75. Total +128 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Fixes #89520.
What Changed
Real behavior proof
<store>.<pid>.<uuid>.tmpfiles older than five minutes, while preserving fresh or unrelated files.Darwin 25.5.0), Nodev26.0.0.Platforms tested
v26.0.0.Additional real checkout proof
After ClawSweeper asked for proof outside the Vitest-only path, I ran a real checkout probe that imports the actual changed
sweepOrphanSessionStoreTempsfunction from this branch, creates a realsessions.jsondirectory with stale, fresh, and unrelated temp files, then runs the sweep.Exact command:
Output:
Observed result: the stale
sessions.json.<pid>.<uuid>.tmpfile was removed, while the fresh matching temp file and unrelated temp file were preserved.