feat(storage): restore quarantined archived sessions (phase 2.1 of #42) - #558
Merged
Conversation
…dge-jun#42) Add trash list/restore APIs, Storage GUI quarantine panel, and round-trip tests so default cleanup can be undone without Phase 3 purge.
This comment was marked as outdated.
This comment was marked as outdated.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… races Keep satellite backups for quarantine restore, snapshot full thread rows and state dependents, remap satellite DB paths, and make satellite failures retryable. Drop duplicate trash probing so Storage abort/generation ownership stays correct.
Use link+unlink no-replace for trash restore so a raced dest cannot be overwritten, and raise timeouts for multi-satellite cleanup tests that overrun 5s on Windows CI.
Reject malformed manifests wholesale, gate stage deletion on restore completeness, and reconstruct production thread rows from rollout session_meta when satellite snapshots are missing.
Holding BEGIN IMMEDIATE across writeFileSync let Windows CI disk/AV latency stall the watermark concurrent-restore test past bun's 5s default. Persist consolidatePostImage after COMMIT and raise the two concurrent restore test budgets to match the other multi-satellite cases.
Restage files then delete rows this restore committed so satellite/leftover failures leave a clean retryable trash entry.
…xisting rows Track exact conflict-ignored inserts, require compensating DB txs to commit before restaging, and leave accurate partial counts when compensation is busy.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ation Late failures after file moves now persist restore-pending.json and keep restored files in place, so retries accept destinations and finish only missing metadata without deleting rows before a guaranteed restage.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Persist the resume marker with stage-local temp+fsync+rename before any rollout leaves quarantine, distinguish missing/valid/invalid markers, and fail closed when a resume still owes satellite work but satellite-backup.json is gone.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Once restore-pending is durable, a later rename failure must not reverse successful moves or drop the planned acceptedDestRels marker so resume can finish staged files.
…sume, tombstone finalize, worker job) Fail closed per owed satellite section on resume, finalize successful restores via non-listable tombstone rename, and run restoreTrashEntry in a serialized Bun Worker so the event loop stays responsive.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Route manual cleanup and trash restore through one per-CODEX_HOME coordinator that returns 409 storage_mutation_busy instead of queuing. Stub runPolicyStorageMutation for Phase 3 policy worker integration.
Map worker rejections to distinct restore error codes end-to-end (API, GUI, i18n) instead of generic restore_failed, and register the restore test-stream endpoint only when OPENCODEX_CLEANUP_TEST_HOOKS=1.
Keep only restore worker error types in cleanup.ts; remove calls to undefined pending-restore guard helpers introduced in the prior commit.
Reject cleanup when selected archives overlap acceptedDestRels from valid restore-pending markers, fail closed on unfinished satellite sections, and address tip CodeRabbit nits (healthz flake, preview status).
Resolve conflicts keeping Phase 3 auto-cleanup policy alongside trash restore UX, overlap guards, and mutation coordinator integration.
Raise injected satellite rollback test timeout for slow Windows SQLite reconcile, and delay concurrent policy PUT until the worker has loaded the enabled snapshot inside holdAfterLoadMs.
…g-safe selection Acquire the shared CODEX_HOME mutation slot in the policy job controller before spawning the worker and release it on success, failure, timeout, and abort. Pending-restore destinations no longer consume percent or reduceToBytes selection budget; cleanup backfills with the next oldest safe archives. Adds API regressions for policy, restore, and manual cleanup contention in both orderings.
The request handler assignment overwrote executeJob's inflight=null on mutation-busy returns, leaving a settled Promise latched forever. Clear inflight in a finally on the assigned job promise instead.
Resolve api-storage-policy.test.ts conflict: keep Windows CI worker spawn timing (blockMs 1500, sawRunning gate, 800ms hold margin).
4 tasks
Wibias
added a commit
to Wibias/opencodex
that referenced
this pull request
Jul 27, 2026
…follow-up Loop short writes to completion and rename via renameAtomicFile so Windows sharing violations cannot replace a durable temp with a truncated backup.
Wibias
added a commit
that referenced
this pull request
Jul 27, 2026
* fix(storage): harden #558 satellite backup writes and zst restore Make every satellite-backup.json update crash-safe (tmp + fsync + rename + best-effort dir fsync) so a failed post-memories rewrite cannot truncate the last valid snapshot. Restore legacy compressed-only quarantine entries by decompressing a lone .jsonl.zst in memory and reconstructing the thread row. * fix(storage): harden satellite-backup write retries for #558 follow-up Loop short writes to completion and rename via renameAtomicFile so Windows sharing violations cannot replace a durable temp with a truncated backup.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /api/storage/trashandPOST /api/storage/trash/restoreto list and restore Phase 2 quarantine entries underCODEX_HOME/.trash/<epoch>/(relative paths only).satellite-backup.jsonis present), removes the trash entry on success, and returns 409 when Codex DBs are busy.Closes part of #42 (Phase 2.1 only — no Phase 3 auto-policy/purge).
Test plan
bun test --isolateforstorage-cleanup,api-storage-cleanup,api-storage,storage-scannerbun run typecheckstate_*.sqlite, confirm restore returns busy / 409Summary by CodeRabbit