fix(route-insight): never overwrite an earlier backup - #191
Merged
chuycepeda merged 2 commits intoSep 29, 2026
Merged
Conversation
The backup name has one-second resolution, so routing several entries in a row made each backup replace the previous one, and the first pre-image was lost. Claim the name with O_EXCL and take the next free -N suffix; remove the placeholder if the copy fails. Concurrent runs on one file are still not serialised and are left for a separate change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
chuycepeda
added a commit
that referenced
this pull request
Sep 29, 2026
Covers #187-#191 and their follow-ups. Manifests move to 0.8.6. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LCjeYxWVWosFKnRiCf8Ae7
Member
|
Thank you, Matías. This shipped in v0.8.6 (merged via #195). O_EXCL with a -N suffix is the right shape, and cleaning up the placeholder when the copy fails is the detail that is easy to miss. Agreed that serialising concurrent runs on the same file is a separate change. |
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
2 participants
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.
What
hooks/route-insight.pysaves a backup before it rewrites a file, named<file>.routebak-<YYYYmmdd-HHMMSS>from the file's mtime. The name now can never replace an existing backup: it is claimed withO_CREAT | O_EXCL, and when taken the next free-2,-3, … suffix is used. If the copy then fails, the claimed placeholder is removed before the error is re-raised, so an empty file never reads as a backup. The new names still match*.routebak-*.Why
The name has one-second resolution. A close that routes several entries in a row rewrites the file within the same second each time, so from the second run on, each backup landed on the previous one's name and replaced it. Only the last pre-image survived, and the first one, the only copy still holding every entry routed away, was lost with no error.
Proof
New scenario in
tests/route-insight.test.sh: three entries routed with the file's mtime pinned to the same second before each run, which makes the collision deterministic. It checks that three backups exist and that one still holds the first entry./bin/bash)maintooltests/lint-windows-stdout.pytests/*.test.shbatteryclaude -pcheck inheadless-allowlisthung locally and was skipped)Not in this PR
Two runs on the same file at the same time are still not serialised: one can copy a half-written file, or restore its backup over the other's write. Fixing that needs a per-file lock around the whole read-backup-write-verify sequence, which is a larger change than this one. This PR only covers runs in sequence, which is how a close routes entries.
🤖 Generated with Claude Code