Skip to content

Report Codex cache write failures and clean up orphaned files - #1764

Open
levelupself wants to merge 1 commit into
getagentseal:mainfrom
levelupself:fm/codeburn-upstream-cache-fix
Open

levelupself wants to merge 1 commit into
getagentseal:mainfrom
levelupself:fm/codeburn-upstream-cache-fix

Conversation

@levelupself

Copy link
Copy Markdown

A Codex cache write that fails before the final rename currently leaves the previous disk snapshot in place without a warning, and can leave an empty or partial temporary file behind. Subsequent reports still work, but the persistence failure is hard to diagnose.

This change keeps persistence best effort while warning with the destination path and underlying error. It cleans up owned temporary files after write, sync, close, and rename failures for both the result cache and project index. It also reports abandoned temporary files and reclaims them on a cold result-cache load or flush. New files use exclusive creation and include the writer PID; live writers are left alone, and legacy files without a PID get a 24-hour grace period.

The regression tests cover injected failures, preservation of the last complete snapshot, successful retry, orphan cleanup, live-writer protection, source fingerprint validation, and a cache larger than 60 MB.

Validation used Node 22.23.1 and clean committed trees:

  • Full suite (npm test -- --run --maxWorkers=3): upstream 9a40cc603fcd194caca9daa8c5134a61ea7e0e2b had 5,209 passed, 3 failed, 7 skipped; this branch at bc675005f8edd27b5030935cb6858be47d896b25 had 5,223 passed, 2 failed, 7 skipped. There are no new failures. Both branch failures are the same dashboard never entered background hydration failures in tests/dashboard-process-exit.test.ts seen on upstream. Upstream also failed the degraded-read assertion in tests/session-cache-status-snapshot.test.ts, which passed on the branch.
  • At bc675005f8edd27b5030935cb6858be47d896b25: all 13 new tests and 16 related cache tests passed; npm run test:locks passed all 37 tests; npx tsc --noEmit and npm run build passed.

The new regression assertions are in tests/codex-cache-write.test.ts, exercising src/codex-cache.ts, src/cache-dir.ts, and src/wsl.ts. The related suites cover cache-directory switching, versioned cache adoption, concurrent loads, and WSL flushing. The full-suite commands use the repository's package.json, vitest.config.ts, and test environment isolation setup.

Keep the last complete cache on persistence errors, warn with the affected path, and remove owned temporary files after write, sync, close, or rename failures. Reclaim abandoned legacy and PID-tagged temporary files without deleting live writers.

Cover result and project-index failures, retry recovery, source fingerprint validation, and a cache larger than 60 MB.
@levelupself
levelupself requested a review from iamtoruk as a code owner October 17, 2026 08:47
claude[bot]

This comment was marked as outdated.

@ozymandiashh ozymandiashh added area: cli The codeburn CLI and its core parsing/reporting engine bug Something isn't working needs-real-data-proof PR needs screenshot or output proving it works against real data needs-validation PR requires validation against real-world usage before review provider: codex Codex CLI provider labels Oct 21, 2026

@iamtoruk iamtoruk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the leak is real: a write, fsync or close failure leaves the temp behind and nothing sweeps it. Numbers are identical with your branch, so the fix itself is safe.

The shape is more than the bug needs, and two parts fight the codebase:

  1. Warnings. CodeBurn is silent on stderr unless CODEBURN_VERBOSE is set, because serve turns stderr into progress frames. Your sweep warns on every cold load and flush, and it warns about any fresh temp from another writer before the age check, so a second CodeBurn process mid-write gets reported as stale every run.
  2. PID tagging. The Flatpak CLI shares ~/.cache/codeburn with a host CLI but not its PID namespace, so a host writer's temp looks dead from inside the sandbox and gets deleted at once, and that save is lost. A reused PID keeps a dead writer's leftover forever.

What I would merge: unlink the temp on any failure after open (a few lines in writeFileAtomic), and sweep leftovers by age only, the same rule the session cache already uses, with no warnings. The 60 MB test passes on main and can go, and temp dirs belong under tmpdir() like the other tests. Happy to take that version quickly.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli The codeburn CLI and its core parsing/reporting engine bug Something isn't working needs-real-data-proof PR needs screenshot or output proving it works against real data needs-validation PR requires validation against real-world usage before review provider: codex Codex CLI provider

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants