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

[turin-dev]

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.

[lidge-jun]

리뷰 · 우선순위 73 / 80

이 이슈는 아카이브 세션 정리(Phase 2 cleanup)가 크래시·전원 끊김 같은 중단을 맞아도 복구 저널로 되돌릴 수 있어야 하는데, 복구의 핵심인 manifest.json 쓰기가 원자적이지 않아서 그 약속을 깨는 버그입니다. 재현 시나리오는 명확합니다. executeArchivedCleanup이 롤아웃 파일을 .trash/<epoch>로 옮긴 뒤, DB 삭제 직전에 최종 매니페스트를 다시 씁니다. 그런데 그 쓰기는 writePrivateFile → writeFileSync(path, content, "utf8")로 최종 경로를 그 자리에서 잘라 덮어씁니다. 중간에 프로세스가 죽으면 매니페스트가 빈 파일이나 잘린 JSON이 되고, 재시작 후 restoreTrashEntry는 파싱 실패를 invalid_trash로 거절합니다. 그러면 휴지통 안에 롤아웃 바이트는 남아 있는데도 API/대시보드 복구 경로로는 못 되돌립니다. quarantine 구간에서는 DB 행이 이미 없어졌거나 archived 경로를 가리키는데 파일은 trash에만 있는 상태가 되고, permanent의 purgeIncomplete 재쓰기 중 끊기면 더 나쁩니다.

지금 dev HEAD는 bd1cda99c(package 2.44.0, #3768 credential 선택 공유까지 들어간 release-244)입니다. 이슈가 가리킨 줄은 HEAD에서도 그대로입니다. src/storage/cleanup.ts L118–L121의 writePrivateFile은 권한만 맞추고 원자 교체는 없습니다. 같은 파일 L1904 근처 writeManifest와 L1986 근처 permanent purgeIncomplete 재쓰기가 둘 다 그 헬퍼를 씁니다. 반면 L1065–L1101 writeSatelliteBackup은 이미 “같은 stage 디렉터리에 private temp → 전체 write + fsync → renameAtomicFile → 디렉터리 fsync” 패턴을 갖고 있고, restore-pending 마커도 비슷한 방향으로 갑니다. 즉 복구 저널의 다른 부품은 원자적인데, 복구의 지도인 매니페스트만 예외입니다. types/config 분할로 무효화될 이슈가 아니고, 로컬 스토리지 크래시 복구 계약 버그라 release-244 비목표(새 프로바이더 등)와도 충돌하지 않습니다. 리포터가 HEAD bd1cda99c 기준으로 재현·제안까지 맞춰 둔 상태라 수락 가치가 큽니다.

라인 118 - writePrivateFile이 writeFileSync로 최종 경로를 in-place truncate/rewrite해서 중단 시 잘린/빈 매니페스트가 남을 수 있음
라인 1904 - 최종 writeManifest()가 롤아웃 stage 이후·DB 삭제 직전에 같은 in-place 경로를 씀
라인 1986 - permanent purgeIncomplete 매니페스트 재쓰기도 동일 헬퍼라 DB 커밋 이후에도 복구 지도가 깨질 수 있음
경로 restoreTrashEntry / invalid_trash - 잘린 매니페스트를 stage 파일 이동 전에 거절해, trash에 바이트가 있어도 정상 복구 API가 막힘
경로 writeSatelliteBackup - 이미 temp+fsync+atomic rename 모델이 있는데 매니페스트만 이를 따르지 않음
경로 기존 cleanup 테스트 - “쓰기 전 throw” 주입은 있으나 “쓰기 중 잘림 후 restore 가능” 회귀가 없음

메인테이너의 판단이 필요한 지점

• 매니페스트만 writeSatelliteBackup과 같은 원자 헬퍼로 올릴지, writePrivateFile 자체를 원자 기본으로 바꿔 다른 호출부까지 같이 묶을지
• 잘린 매니페스트가 이미 있는 기존 trash에 대해 베스트에포트 복구(파일 목록 스캔)를 넣을지, forward-fix만 할지
• permanent purgeIncomplete 재쓰기도 같은 원자 경로를 강제할지(지금도 best-effort catch)
• 회귀를 “truncated 후 restore 성공”으로 고정할지, “rename 전 실패 시 이전 유효 매니페스트 유지”까지 단언할지

너의 추천
수락하세요. manifest.json 쓰기를 satellite backup과 같은 temp + fsync + renameAtomicFile(+ 디렉터리 fsync)로 바꾸고, 최종 쓰기 중 잘림을 시뮬레이션한 뒤 restoreTrashEntry가 성공하는 회귀를 추가하세요. 기존 유효 매니페스트는 rename 성공 전까지 유지되게 하세요. 범위는 src/storage/cleanup.ts와 cleanup 테스트로 좁게 가져가면 release-244에 빨리 들어갈 수 있습니다.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Landed via #3786 at 4397079 — cleanup manifests now publish through the atomic private+fsync+rename path; recovery record no longer truncates on handled pre-replace failure.

이 댓글은 grok-bot이 작성했습니다