[Bug]: Non-atomic cleanup manifest writes can strand archived sessions after a crash #3778

Resolved in dev

#3786 (43970794fe) publishes cleanup manifests atomically. The original manifest-publication defect is resolved; physical power-loss durability is not claimed.

Verified against dev 5759d9ea2f1e7281cdc01eb9628f2e0a123fb59c. Original report by @turin-dev. The attribution record was added in #3811.

Client or integration

OpenCodex dashboard

Area

Other

Summary

Phase 2 archived-session cleanup can strand user sessions after a process or power loss while writing its recovery manifest.

writePrivateFile writes directly to the final manifest.json with writeFileSync(path, content, "utf8"). executeArchivedCleanup calls the final writeManifest() only after every rollout has already been moved into .trash/<epoch> and immediately before the state/satellite database reconciliation. Because the destination is truncated in place and the write is neither temporary nor fsynced, an interruption can leave a zero-length or truncated manifest.

On restart, restoreTrashEntry rejects the entire stage as invalid_trash, so the staged rollout files cannot be restored through the API or dashboard. In the pre-DB-deletion window, the database row still points at the now-missing archived_sessions/...jsonl; if the interruption happens after a later database commit (for example during the permanent-mode purgeIncomplete manifest rewrite), the row can already be gone. In both cases the normal recovery path is lost even though the rollout bytes remain in .trash.

This breaks the crash-recovery guarantee that the cleanup journal is intended to provide. The existing satellite backup and restore-pending files already use temp + fsync + atomic rename, but the manifest does not.

Reproduction

The following uses a temporary CODEX_HOME and a synthetic state database. The second writeFileSync deterministically simulates the torn/partial manifest that a process or power loss can leave while the real code is rewriting the destination in place.

import { Database } from "bun:sqlite";
import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import {
  executeArchivedCleanup,
  previewArchivedCleanup,
  restoreTrashEntry,
} from "./src/storage/cleanup.ts";

const home = mkdtempSync(join(tmpdir(), "ocx-cleanup-crash-"));
mkdirSync(join(home, "archived_sessions"), { recursive: true });
writeFileSync(join(home, "archived_sessions", "rollout-old.jsonl"), "conversation data");

const db = new Database(join(home, "state_1.sqlite"));
db.exec("CREATE TABLE threads (id TEXT PRIMARY KEY, rollout_path TEXT NOT NULL, archived INTEGER, archived_at INTEGER, history_mode TEXT)");
db.exec("INSERT INTO threads VALUES ('t1','archived_sessions/rollout-old.jsonl',1,1,'legacy')");
db.close();

const preview = previewArchivedCleanup(100, home);
const cleanup = executeArchivedCleanup({
  percent: 100,
  mode: "quarantine",
  digest: preview.digest,
  codexHome: home,
  now: 1700000000000,
});
const trashId = cleanup.trashDir!;
const stage = join(home, ...trashId.split("/"));

const dbAfter = new Database(join(home, "state_1.sqlite"), { readonly: true });
const deleted = dbAfter.query("SELECT id FROM threads WHERE id='t1'").get() === null;
dbAfter.close();

// Stand-in for a crash during the final in-place manifest rewrite.
writeFileSync(join(stage, "manifest.json"), "{\"entries\":[");

const restore = restoreTrashEntry(trashId, { codexHome: home });
console.log(JSON.stringify({
  cleanupOk: cleanup.ok,
  deleted,
  stagedFileStillThere: existsSync(join(stage, "rollout-old.jsonl")),
  restore,
  restoredFile: existsSync(join(home, "archived_sessions", "rollout-old.jsonl")),
}));

rmSync(home, { recursive: true, force: true });

Run with bun run repro-cleanup-manifest.ts against the checked-out source. Observed output:

{"cleanupOk":true,"deleted":true,"stagedFileStillThere":true,"restore":{"ok":false,"trashDir":".trash/1700000000000","count":0,"bytes":0,"restoredPaths":[],"error":"invalid_trash"},"restoredFile":false}

The cleanup test file still passes its existing 68 tests; it has failure injection for a manifest write that throws before the write, but no test for an interrupted/truncated write after staging.

Version

OpenCodex dev commit bd1cda99c162e3b4b41b14f6ad5ca2cf6f1a1f03 (package 2.44.0), checked 2026-09-06

Operating system

Ubuntu 24.04.3 LTS, Linux x64; Bun 1.4.0

Provider and model

No response (provider-independent local storage path)

Logs or error output

cleanupOk=true
deleted=true
stagedFileStillThere=true
restore.error=invalid_trash
restoredFile=false

Screenshots and supporting files

• Direct in-place writer truncates and rewrites the destination manifest.
• Final manifest write runs after all rollout files are staged and before database deletion.
• Permanent purge rewrite rewrites the recovery manifest after a successful DB commit when purge is incomplete.
• Restore rejection maps a malformed/truncated manifest to invalid_trash before any staged file move.
• Atomic pattern used by satellite backup provides a suitable implementation model.

Suggested fix: write the manifest to a private temp file in the same stage directory, write all bytes, fsync the file, then atomically rename it over the old manifest and fsync the directory. Keep the previous valid manifest until the replacement succeeds, and add a regression that simulates an interrupted final write and verifies restore remains possible.

Redacted configuration

No provider configuration is required. All identifiers, paths, message text, and database values in the reproduction are synthetic.

Checks

• I searched existing issues and documentation.
• I removed secrets, tokens, account details, request credentials, and personal data.