Round 3, from two independent re-reviews of the round-2 fix commit. The fix for
the lost-output-on-SIGTERM defect introduced a worse one than the defect.
The signal handler ended in `exit "$?"`, and `$?` at signal time is the status of
whatever command was running — normally 0 for a guard still gathering evidence.
Measured on the real guard, `prepush-donewhen.sh` SIGTERMed mid-curl with an
unticked Done-when box:
uninstrumented rc=-15 git ABORTS the push
instrumented rc=0 git PUSHES to main
That converts a killed guard into consent, in the permissive direction, in the
hook that protects `main`. The handler now flushes, restores the default
disposition and re-raises the signal, so the process dies the way it would have
died. test_output_survives_a_SIGTERM could not see it: it asserts stdout content,
never reads the return code, and drives a capture-mode hook so no exit-code guard
is ever signalled.
Also from this round:
- The flush ran TWICE on the signal path (signal handler, then EXIT trap),
writing two contradictory exit records for one invocation — a real decision
then a phantom no-op — so the report mis-histogrammed the very case the trap
was added for. It is idempotent now.
- `exec 4<&- 2>/dev/null`: the round-1 blocker pattern again, eight lines below
the comment forbidding it. Rather than fix the third instance and wait for a
fourth, test_no_exec_in_the_sink_carries_a_STDERR_REDIRECT now enforces it.
- The log write leaked `Operation not permitted` to stderr on an unwritable log,
because `>> "$file" 2>/dev/null` opens the file BEFORE redirecting stderr. The
fail-open test used a path that dies at mkdir and never reached this.
- Classification: an unrecognised value is no longer lowercased into a valid
decision, exit 2 now dominates a printed `allow` (it is the harness's block
channel, and reporting a permit for a refused call is the one direction this
log must never be wrong in), and a non-zero exit no longer annotates a real
decision with an invented `+error`.
- Classification and replay read the same bytes; they disagreed when a hook used
fd 4, so the harness saw `deny` while the log recorded `no-op`.
- The stderr normaliser rewrote `line N:` globally, collapsing genuinely
different diagnostics; it is anchored to the script-path prefix now.
- docs 5.4 claimed the two zero rows were "trigger did not fire", justified by
unit tests that invoke the scripts directly and therefore prove nothing about
harness dispatch. It now separates what is established from what is not, and
stops contradicting its own decision record.
Refs: #776
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
timothyandClaude Opus 5
b9f031f52a