fix(snapshot): a signal ends the run; copies land whole or not at all - #186
Merged
chuycepeda merged 2 commits intoSep 25, 2026
Merged
Conversation
- `trap release EXIT INT TERM` released the lock on a signal and the loop kept archiving the remaining files with no exclusion. INT and TERM now release and exit (130 / 143). - Copies went straight to the final name, so a copy cut short stayed as a valid-looking snapshot that every later run compared against. `put` copies to a hidden temp file in the same directory and renames it into place; the temp file is removed on failure and on release. tests/aios-snapshot.test.sh: a cp shim that fails halfway, one that SIGKILLs the archiver mid-copy, and TERM/INT during a slow copy (job control on so INT reaches the background job). Each case also runs against the hook pinned at the pre-change commit, where it reproduces the defect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
chuycepeda
added a commit
that referenced
this pull request
Sep 25, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LCjeYxWVWosFKnRiCf8Ae7
Member
|
Shipped in v0.8.5 with your commits and authorship intact. Thank you. It went in as you wrote it: 22/22 under bash 5 and 3.2, and 6 of them fail on the old hook. Looking forward to the lock-reclaim PR across the three hooks. |
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.
What
Two ways an interrupted
hooks/aios-snapshotrun leaves the archive wrong. Both are silent.trap release EXIT INT TERMreleased the lock on INT or TERM, and the script then continued: bash runs the handler and resumes the loop. The remaining files were archived with no lock, which is the one thing the lock exists to prevent.cpor a killed process, stayed under that name. Every later run compared against the truncated file, and it read as history.Fix
trap release EXIT, plustrap 'release; exit 130' INTandtrap 'release; exit 143' TERM. A signal that arrives during a foregroundcpis handled when thecpreturns. The handler discards that copy and exits.putcopies to.aios-snapshot.<pid>.tmpin the snapshot directory, then renames it to the final name. The rename is atomic within one filesystem, so a final name only ever holds a complete copy. The temp name starts with a dot and cannot match the variant globs. The temp file is removed on failure and inrelease. Callers still check under the lock that the destination does not exist, so the collision logic is unchanged.A power cut is not covered: nothing calls
fsync. What is covered is every interruption where the process stops but the filesystem survives.Proof
tests/aios-snapshot.test.shgains a section withcpshims onPATH:cpthat writes three bytes and fails. Nothing is left under a snapshot name, and no temp file remains.cpthat writes three bytes and SIGKILLs the archiver. No trap runs, so only the write path decides what stays. No snapshot name exists afterwards.cp. Each exits 143 or 130 and archives nothing more. The lock is released and no temp file remains. Job control is enabled for that invocation, because a non-interactive shell starts background jobs with SIGINT ignored.2026-08-14-obs.md, and after TERM the second file is still archived. These cases are skipped with a message if the commit is missing.exit, INT withoutexit, a direct copy instead ofput, and no temp cleanup each fail their case.22 pass under bash 5 and bash 3.2. The existing concurrency race, with the lock as the only difference between variants, still passes, and its control still fires. The full
tests/run passes.Not in this PR
The lock-reclaim race (two processes reading the same dead pid) has the same shape in
aios-commit,aios-snapshotandaios-note-append. It gets its own PR across all three.Scope
hooks/aios-snapshot, its test, and the CHANGELOG entry. Part of the hooks series announced in #158.🤖 Generated with Claude Code