Repository navigation
Conversation
#480) truncate_index() used to `: > "$tsv"` the REAL, committed 00-index.tsv before a single row of the new one existed. On macOS 27 the built-in bash can kill forked awk children with "Bus error: 10" partway through rebuild-tsv.sh -- not reproducible under Homebrew bash, and too rare in this plugin's own short test runs to ever show up there. The run already detected one shape of that crash and reported jit_rc 2, but kept writing into the already-truncated file, so "detected" did not mean "preserved": 7 indexes in the reporter's own project ended up empty, the largest going from 3,714 rows to 1,430. Every index is now built into a temp file in the same directory (truncate_index) and swapped in with one mv (commit_index), only when JIT_RC2_COUNT did not advance while that index's own loop ran -- a helper dying anywhere inside it leaves the previously committed index untouched rather than replaced by a partial one. A real subshell bug surfaced while wiring this up: truncate_index()'s own jit_rc(2) calls run inside the $( ) that captures its temp path, so they never reach the caller's JIT_RC -- every call site now raises jit_rc 2 explicitly on a truncate_index failure. test-rebuild-exit-codes.sh's section G is updated for the resulting, deliberate behaviour change: mv's rename() only needs write permission on the directory, not on the file it replaces, so a chmod'd-444 index file no longer blocks a rebuild -- the genuine "cannot write" case moved to the directory. New test-rebuild-atomic-swap-480.sh drives a real SIGBUS-killed awk child via a shimmed PATH and asserts the committed index is byte-identical before and after. Closes #480 Co-Authored-By: Claude Sonnet 5 <[email protected]>
…on a killed process (#480) Self-review (Explore + oss:auditor, spawned against the first commit) both independently flagged the same regression: COUNT was read from the temp build before commit_index() ever decided whether to keep it, and the per-builder success line was logged unconditionally regardless of commit_index()'s verdict -- so a crashed, discarded build still printed a success-shaped "label: N rules" line to pipeline.log and stdout, directly below the FATAL saying it was discarded. Now logged conditionally on commit_index()'s own return value. Explore additionally reproduced a second gap: nothing cleaned up a temp index file if the whole rebuild-tsv.sh PROCESS (not merely a forked awk helper) is killed mid-build -- SIGTERM, an agent session cut off, an OOM kill -- between truncate_index() creating it and commit_index() ever running. A real SIGTERM 1s into an 800-entry fixture reproduced a leftover `.00-index.tsv.XXXXXX` that nothing on the next run cleans up either. An EXIT trap over a JIT_TEMP_FILES array now removes every temp file this run ever created, swapped in or not. Co-Authored-By: Claude Sonnet 5 <[email protected]>
CI red on PR #487 (6 legs, all the same root cause): test-assertion-helpers.sh requires every suite to carry at least one `# jit-drive:` declaration line, and the new test-rebuild-atomic-swap-480.sh had none -- "declares nothing". Declared assert_contains (its (desc, OUTPUT, needle) signature is the documented "capture" source, "contains" semantic); assert_rc stays undeclared since it compares exit codes rather than payload content, which is reported in the harness's own third-outcome list rather than failing. Verified locally: bash tests/test-assertion-helpers.sh now exits 0, 445 passed / 0 failed (previously failing on exactly this suite). Co-Authored-By: Claude Sonnet 5 <[email protected]>
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
1 participant
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.
On macOS 27, the built-in bash's forked awk helper children can die with Bus error: 10 partway through a rebuild. rebuild-tsv.sh previously truncated each committed 00-index.tsv in place before writing the new content, so a crash left the committed index empty or half-written -- a silently dead entry on a fresh clone, block rules included.
Fix: each of the 4 builders (build_tool_tsv, build_vocab_tsv, build_vocab_path_tsv, build_path_tsv) now builds into a temp file in the same directory and swaps it into place with one mv via a new commit_index() function, only when no jit_rc(2) fired while that index's own build loop ran (tracked via a new JIT_RC2_COUNT global snapshotted before/after each builder). A follow-up commit fixes both findings from self-review: the per-builder success log line is now gated on commit_index()'s own return value instead of printing unconditionally even when the build was discarded as crashed, and a JIT_TEMP_FILES + EXIT trap cleans up a leftover temp file if the whole rebuild-tsv.sh process (not just a forked awk helper) is killed mid-build.
tests/test-rebuild-exit-codes.sh section G was rewritten: the atomic mv-based swap only needs write permission on the directory, not on the file being replaced, so the write-guard probe moved from chmod 444 on the index file to chmod 555 on the directory. This is an intentional behavior change, not a regression.
New test: tests/test-rebuild-atomic-swap-480.sh drives a real SIGBUS-killed awk child via a shimmed PATH and asserts the committed index is byte-identical before/after the crash, exit code 2, stderr names the crash, and no stray temp file is left.
Closes #480.
🤖 Generated with Claude Code
[AI-generated]