Preserve shared hook decision reasons and verify output parsing

Merged
VA
3d ago
+591-236

Problem

A valid hook reply containing a legacy block decision and a permission ask consumes their shared explanation for the block, replacing the approval explanation with a generic message. Hook-output decisions and normalized HookSpec data also lack shrinking round-trip properties and a checked-in fuzz target.

Design

Extract the existing pure HookOutput implementation into hooks/output.rs and place HookEvent with the normalized spec types. The runner keeps the same public exports and calls the same implementation. Preserve a shared block/approval explanation in both outputs; the block remains effective. Copy the explanation only when both owned outputs need it. The fuzz target compiles these production source files directly, using the harness Error/Result types; it needs no native runner, subprocess, model, or public testing API.

Deletions

Remove the former inline output decoder and event definition from hooks/mod.rs. No duplicate parser or decision algorithm remains.

Decisions

  • Start from origin/main at 597a2d75abb8, including #594. This is independent from the pinned-manifest allocation fix in #590.
  • A block-plus-ask reply clones its shared reason only when the approval slot is empty. When an earlier hook supplied approval, the denial takes ownership of its decoded reason. The allocation property compares the production merge with JSON decoding alone and rejects a deliberately added reason copy through the same measurement harness.
  • The production extraction preserves the former implementation except for internal module visibility and the shared block/approval reason fix. Hook matching, imports, process execution and failure policies remain unchanged.
  • State remains the existing volume-loaded HookSpec configuration and per-invocation output. No new API, store, protocol, polling, mutex or concurrency algorithm.
  • Shrinking properties assert structured-output semantic round trips, atomic rejection of every recognized invalid field type, failure-policy application including Stop, monotonic first-denial preservation, and normalized-rule serialization/validation stability.
  • Fuzzing covers arbitrary output bytes for all four events and both failure policies; canonical accepted JSON must preserve the result. Normalized rules round-trip every field and preserve validation outcomes. Seven checked-in fuzz seeds cover deny, approval/replacement arguments, post-tool feedback, invalid types, malformed syntax, executable HookSpec data and the shared-reason counterexample. The shrinking counterexample seed is also checked in for deterministic replay before random property cases.

Tests run

Signed head: 72f7265facd067ab345098c8ad073dc2501ab4ae. Full change +591/-236, net +355 lines; most additions are properties/fuzz coverage, while the former inline parser and event definitions move to their sole modules.

All builds and execution through Q:/lanes/bs.ps1, warm target, incremental defaults. Native closure: filesystem-local,native-execution,terminal,mcp-http,test-support (includes hooks, mcp-stdio, mcp-http and defaults).

  • Before the fix, the actual production-path property failed and shrank to PreToolUse with decision:block, permissionDecision:ask and an empty shared reason: approval became the generic fallback. The exact persisted seed is checked in.
  • Full native touched-feature compile: cargo test --no-run -p acyclic-harness --lib with that closure passed (228.54 s). Built binary directly: hooks 26/26, 0.22 s wall; native settings 13/13, 0.06 s wall.
  • Initial pre-push compile with the identical feature closure passed (21.92 s). Verified the persisted regression file is present, then ran the built output-property tests directly: 3/3, 0.01 s. No test count reduction, sleeps or polling.
  • ASan cargo-fuzz target hook_output compiles the actual production output/spec source and uses the real harness Error/Result. Rust 1.98.1 with RUSTC_BOOTSTRAP=1 only for sanitizer instrumentation. Initial target build 51.55 s; seven checked-in seeds plus 512 runs passed in 0.94 s.
  • Strengthened the fuzz oracle to assert approval reason preservation explicitly. Final target rebuild 1.66 s; seed corpus plus 512 runs passed in 0.77 s (4096-byte input / 3-second fuzz bound). No sanitizer findings.
  • Shrinking properties cover semantic JSON round trips, approval/block reasons, post-tool feedback ordering, nested replacement arguments, every recognized invalid field, atomic rejection and failure policies including Stop, first-denial preservation, and normalized HookRule round-trip/validation stability.
  • rustfmt --check, git diff --check, LF/NUL and source-size checks passed. Every execution suite is under 1 s. No new synchronization primitive; concurrency implementation remains unchanged.

Native/fuzz gate: /home/var/log/buildbox/2026-10-16T001853033Z-3795688.log (rc=0).
Final persisted-seed/oracle gate: /home/var/log/buildbox/2026-10-16T002612059Z-3826241.log (rc=0).
Baseline counterexample: /home/var/log/buildbox/2026-10-16T000822238Z-3710324.log.

Review-fix validation at 72f7265facd067ab345098c8ad073dc2501ab4ae, through the same bs warm target and feature closure:

  • Full touched-feature lib-test compile passed in 71.91 s. Built binary directly: all 27 hook tests passed in 0.03 s, including the new shrinking multi-hook allocation property and its extra-copy negative control.
  • The new property uses real sequential hook replies, asserts that the first approval and later denial retain their respective reasons, and requires both allocation count and allocated bytes to equal JSON decoding alone. The deliberate extra reason copy must fail that same detector.
  • Extended the existing production-source fuzz target to merge each input after an earlier approval, preserving approval and applying the same denial. ASan target rebuild passed in 1.13 s; seed corpus plus 512 runs passed in 0.86 s.
  • rustfmt, diff checks and LF/NUL checks passed. Review-fix diff +57/-1, net +56 lines. No new state, APIs, processes or synchronization.
  • Gate: /home/var/log/buildbox/2026-10-16T003955314Z-3882040.log (rc=0).