fix(hooks): live-probe gate fires under subdir project dir + surfaces WARN to agents (v4.4.32)#996
Closed
michael-wojcik wants to merge 5 commits into
Closed
Conversation
… WARN to agents _resolve_repo_root returned CLAUDE_PROJECT_DIR unconditionally, so a process whose CLAUDE_PROJECT_DIR is the pact-plugin subdir resolved a doubled pact-plugin/pact-plugin marker path, found no plugin marker, and silently self-disabled — shadowing the git-common-dir fallback. The early return is now guarded on the plugin marker resolving at that root; otherwise it falls through to the existing git-common-dir-parent path, then cwd. The repo-root and unset cases are unchanged. _emit_warn printed its advisory to stderr on exit 0, which PreToolUse does not surface to an agent operator (only exit-2 deny stderr surfaces). The advisory now emits on stdout as hookSpecificOutput.additionalContext (hookEventName PreToolUse), reaching agent-driven merges. Advisory text unchanged; the gate still always exits 0 (non-blocking). Adds resolver-matrix and surfacing-shape verification tests; re-points the dogfood-probe runbook VERIFY procedure to the new channel.
Adds the both-project-dir resolver matrix (repo-root / subdir / unset) plus edge cases — a marker-bearing project dir trusted regardless of depth, the cwd third-tier fallback when the marker is absent and git is unresolvable, and an end-to-end check that main() WARNs rather than silently self-disabling under a subdir project dir. Adds the WARN-path surfacing-shape assertions, advisory-prose byte-equivalence, and the silent-vs-WARN suppressOutput asymmetry negative control. Non-vacuity is established by a source-only revert with clean per-finding failure attribution; full hook suite 9247 passed, 0 failed, 0 errors. Test-only; source byte-identical to the prior commit.
…verage Folds in review findings: the silent-when-satisfied test now spies on _has_satisfied_row to assert it is called with the resolved repo root (isolating the fall-through instead of passing green-both-trees); the byte-equivalence test docstring is reframed as an accurate green-tree drift-detector; and a new end-to-end test drives main() through the waiver_ok=True path. Non-vacuity re-proven by source-only revert.
Inline comments folding in reviewer findings: the WARN rests solely on the additionalContext channel (no backstop — acceptable for a non-blocking advisory; runtime delivery confirmed by the post-merge re-probe); the git-common-dir fallback intentionally resolves to the main checkout, not a worktree; and the repo-root-case second marker read is intentional. Comments only, no logic change.
Collaborator
Author
|
Superseded by #997. The hook-audit this session confirmed |
This was referenced Jun 20, 2026
Closed
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.
Summary
Fixes the two efficacy defects in the live-probe gate (
pact-plugin/hooks/live_probe_gate.py) surfaced by the #924 tmux dogfood. Implements the #932 code fixes (Finding A + Finding B); closure of #932 and #924 is pending the tmux re-probe perpact-plugin/tests/runbooks/924-locus-b-dogfood-probe.md— that requires a real tmux teammate-mode session and is out of scope for this in-process change.What landed
Finding A — gate self-disabled under a subdir
CLAUDE_PROJECT_DIR._resolve_repo_rootreturnedCLAUDE_PROJECT_DIRunconditionally, so a process whoseCLAUDE_PROJECT_DIRis thepact-pluginsubdir resolved a doubledpact-plugin/pact-pluginmarker path, found no plugin marker, and silently self-disabled — shadowing the correct git-common-dir fallback. The early return is now guarded on the plugin marker actually resolving at that root; otherwise it falls through to the existing git-common-dir-parent path, then cwd. One guarded branch — the repo-root and unset cases are behaviorally unchanged.Finding B — WARN advisory invisible to agent operators.
_emit_warnprinted its advisory to stderr on exit 0, which PreToolUse does not surface to an agent (only an exit-2 deny's stderr surfaces). The advisory now emits on stdout ashookSpecificOutput.additionalContext(hookEventName: "PreToolUse"), mirroring the verified siblingtask_claim_gate.py. Advisory prose is byte-unchanged and the gate still always exits 0 (non-blocking). NosuppressOutputco-emit on the WARN path (it would re-hide the advisory).Tests
main()-WARNs-under-subdir integration test (proves the gate reconnects, not just the resolver unit).suppressOutputasymmetry negative control.Why
The gate exists to remind an operator to run the post-merge live-probe before closing a hook-infra issue. Both defects let it silently no-op for the dominant agent-driven-merge case — defeating its purpose. These fixes restore the intended advisory behavior.
Version: PATCH 4.4.31 → 4.4.32.
Not closing yet
This PR does not close #932 or #924. Their closure is gated on a clean tmux re-probe per
924-locus-b-dogfood-probe.md(which logs a satisfied both-mode row) — a real tmux teammate-mode session, sequenced post-merge.