Repository navigation
fix(migrations): a leftover --replace aside is refused by name and never deleted blindly (panel F6) - #637
Merged
Conversation
…ver deleted blindly (panel F6) A kill between `--replace` setting the old migration aside and deleting it left the only copy in a dot-directory the reader skipped, and a later --replace deleted it. Every directory read now refuses a Ferro-shaped leftover aside (interrupted_replace) or half-written staging directory (interrupted_write), naming it and the recovery: rename it back while the replacement is absent, delete it once the replacement is complete. The aside names its replacement (.NNNN_x.MMMM_y.replaced when renumbered), a renumbered replace also sets the old directory aside first, and an existing aside or staging directory is refused instead of removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
0x054
commented
Oct 12, 2026
0x054
left a comment
Contributor
Author
There was a problem hiding this comment.
Scoped re-review: F6
Verdict: ADDRESSED at b870726.
- (a) Walked the sequence in
generate.write(generate.py:259-275) andwrite_migration(layout.py:259-270). Beforeold.rename(aside), the old directory is readable. After it, the reader refuses on the aside,complete=false, with rename-back. During staging, the aside is refused first and then the.partial. After the staging rename, the new directory is whole and the aside readscomplete=true, with delete. A kill during the finalrmtreeis covered by the same state. - (b) The texts name the fix (directory.rs:530-566). The kinds
interrupted_replaceandinterrupted_writeare stable, and the golden test pins both texts. - (c)
interrupted_writes(directory.rs:846-893) matches only.NNNN_<name>[.MMMM_<new>].replacedand.NNNN_<name>.partial, viasplit_numbered. Other dot-names stay ignored. - (d) The diff leaves
DuplicateNumberand every existing text unchanged. No path deletes an aside or a.partial: both are refused (generate.py:263, layout.py:261).
No new findings. A concurrent check during a live new will refuse transiently, which is acceptable.
Verify at b870726: maturin develop 0; cargo check --all-targets 0; cargo test -p ferro-migrate 0 (427 passed); pytest sqlite+postgres 0 (186 passed, 6 skipped); prek 0. The merge preview was "Already up to date" and cargo check --all-targets passed (0), so there was no merge to abort.
1 of 3 tasks
0x054
added a commit
that referenced
this pull request
Oct 12, 2026
…ver deleted blindly (panel F6) (#637) Panel finding F6 on PR #636: `ferro migrate new --replace` set the old directory aside under a hidden name before writing the new one, and a hard kill in between left the only copy where the directory reader skipped it and a later `--replace` deleted it first. Every directory read now refuses a ferro-named leftover (`interrupted_replace`, `interrupted_write`) naming the recovery, a renumbered replacement's aside carries the new directory's name so the reader can tell whether it completed, and nothing deletes an existing aside. Part of #623 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR
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
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.
Part of #623 (panel fix round for PR #636; #629's files)
F6:
--replacein place, a hard kill leaves the migration hidden andcheckdoes not name itBefore:
--replacerenamed0002_add_xto.0002_add_x.replacedand then wrote the new directory. If the process was killed between those two steps, the only copy of the migration (hand-written and data steps included) sat in a dot-directory that the reader skips.checkandstatusreported nothing, and a later--replaceranrmtreeon the aside.After: every directory read (
check,up,down,status,new,new --replace) refuses a leftover that Ferro wrote. The refusal names the directory and how to recover:What changed:
crates/ferro-migrate/src/directory.rs: two newDirectoryErrorkinds,InterruptedReplace(interrupted_replace) andInterruptedWrite(interrupted_write).MigrationsDir::readandread_replacingboth check for them before any other check. An aside is reported before a partial because it may hold the only copy. Hidden names that Ferro did not write are still ignored.src/ferro/migrations/generate.py(write): the aside's name now includes the replacement's name when the number changes (.0002_add_x.0003_add_x.replaced), so the refusal can check whether the replacement is in place. Because the new directory appears with a single rename, its presence means it is complete. A renumbered replace now also moves the old directory aside before writing, so a kill mid-cleanup never leaves a half-deleted directory under a visible name. At every moment, either a readable migration sits under a visible name or the reader names what was in flight. If an aside already exists,writerefuses instead of deleting it.src/ferro/migrations/layout.py(write_migration): if a.partialstaging directory already exists, it refuses instead of deleting it.DuplicateNumberand its--replacelines are untouched.Covering tests:
tests/test_migrate_replace.py:test_a_replace_killed_after_setting_aside_is_named_by_every_reader: a simulated crash (aside by hand, no0002_add_x).checkexits 3 and names the aside and the recovery.new --replaceand plainnewrefuse and leave the tree byte-for-byte unchanged. After renaming the aside back,checkexits 0 and the replace succeeds.test_a_renumbered_replace_killed_before_its_cleanup_names_the_aside_to_deletetest_a_renumbered_replace_killed_before_writing_names_the_rename_backtest_a_write_killed_halfway_names_the_partial_to_deletetest_a_replace_leaves_nothing_hidden_behindcrates/ferro-migrate/src/directory.rs:a_leftover_replace_aside_or_partial_write_is_refused_by_every_read, which checks the kinds, the exact texts for both reads, and that the aside is reported before the partial.RED before the fix: the four crash tests failed.
checkexited 0, or reportedungenerated, andnew --replaceexited 1 for a different reason. The Rust test failed becausereadreturnedOk. All are GREEN after the fix.Verify
Run on b870726 with
FERRO_POSTGRES_URL=postgresql://postgres:password@127.0.0.1:5432/postgres?sslmode=disable UV_FROZEN=1:Outside Files owned
None.
migrate new --replace, foreign defaults as drift (#623) #636 and closes nothing.🤖 Generated with Claude Code
https://claude.ai/code/session_014ERLk8yFhJy7rdDyU7rGdR