Repository navigation
blob fetch: keep a job that wrote until it is forgotten - #35823
Conversation
Red: retire_finished drops the oldest finished jobs past 32, whether or not they wrote, so after enough abandoned jobs a later push can no longer read what a dead client's fetch committed (#35652, item 2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c76279e2406909277323
retire_finished dropped the oldest finished jobs past 32 whether or not they committed files. A client that died after its fetch committed but before it saved a manifest left the only record of those files in that job; enough later jobs erased it, and the next push could leave behind fetched files its tree dropped. A finished job that wrote now stays until blob.fetch.forget; jobs that wrote nothing are still capped at 32 (#35652, item 2). Red at bf296ad (run 41878247406): the cancel named no jobs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c76279e2406909277323
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Shipped in:
|
|
Owned by an agent session · session |
|
Review (codex, router-review): SHIP at b9c5da2 Reviewer: codex gpt-6.1-sol xhigh, fast tier. SHIP at b9c5da2 Visual verdict: unchanged by this diff The retention and commit/cancel locking logic is consistent; no correctness or regression findings. Cargo was unavailable locally, so I relied on static review and the reported hosted test. Follow-ups: none |
|
Found 1 test failure on Blacksmith runners: Failure
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Follow-up 2 of #35652, from the Codex review that gave SHIP on #34604.
What it fixes
retire_finishedincmux-tui-core/src/blob_fetch.rskept at most 32 finished fetch jobs and dropped the oldest, whether or not they had written files. Say a client dies after its fetch commits but before it saves its manifest. The only record of those files is then the job, which the next push to the folder reads throughblob.fetch.cancelandstatus written_from. Once more than 32 other jobs finished, that record was gone, and a later push could leave behind fetched files its tree had dropped.Now a finished job that committed at least one file is never evicted. It stays until a push forgets it (
blob.fetch.forget, after its manifest names the files). Finished jobs that wrote nothing are still capped at 32. This is the second fix the issue suggested ("never evict a job that wrote files until it is forgotten"). It needs no new state on disk.A job's
writtenlist is final once it has leftRunning: a cancel marks it failed and waits for any commit in progress to finish, and a commit checkscancelledunder the same lock. So readingwritten.is_empty()while retiring is stable.Memory: a job that wrote and was never forgotten now stays until the next push to its folder forgets it, or the daemon restarts. Each such job holds its
(path, revision)list, which is what a push needs to recover.Test
cargo test -p cmux-tui-core --lib a_job_that_wrote_is_kept_until_forgotten(hosted,./scripts/verify-cmux-tui-hosted.sh --filter). One job writes, then 40 jobs under another folder fail with nothing written. The cancel under the first folder must still name the job that wrote, and itswrittenlist must still be readable. The 40 empty jobs are still capped at 32, and once forgotten the job goes."jobs": [], but the test expects the job that wrote.blob_fetchrun (result in a comment below).Observed / Not run
link_sync_storetest, which runs fewer than 32 jobs and does not reach this path.risk: normal (machine daemon retention policy).
Changelog
none
Agent session: agent session
🤖 Generated with Claude Code