pal-e-docs

Review: Bug: post-merge hook false alarm on squash merge

review-505-2026-03-28 Review

review ready
Project

Verdict: READY

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — References #173 (closed), PR #177 (original fix)
  • [x] Repo — forgejo_admin/claude-custom
  • [x] What Broke — Specific: false negative on squash merge, suppresses /update-docs
  • [x] Repro Steps — 4 clear steps
  • [x] Expected Behavior — Defined
  • [x] Environment — All 3 hooks listed with line ranges
  • [x] Acceptance Criteria — 5 items, debug-first approach
  • [x] Related — #173, project-pal-e-agency, triggering PR identified

Traceability

  • [x] story:pm-scope label — PM scope management story
  • [x] arch:hooks label — hooks architecture component
  • [x] Forgejo issue — forgejo_admin/claude-custom#189, open

File Targets

  • [x] hooks/remind-update-docs.sh:21-24 — verified: lines 21-24 contain two-stage jq merge detection (.tool_response.merged then .tool_response.result | .merged)
  • [x] hooks/post-mcp-merge-rebase.sh:13-16 — verified: lines 13-16 contain identical two-stage jq merge detection pattern
  • [x] hooks/board-item-on-merge.sh:37-40 — verified: lines 37-40 contain identical two-stage jq merge detection pattern

Repo Placement

OK. Issue filed on forgejo_admin/claude-custom. All 3 hook files live in ~/claude-custom/hooks/. Single-repo fix.

Dependencies

  • Board item #426 (issue #173 — original bug, "merge hook false positive") is in done. This is a recurrence, not a duplicate.
  • No blockers in in_progress or next_up that overlap.
  • No cross-repo dependencies — fix is entirely within claude-custom hooks.

Acceptance Criteria

All 5 AC are verifiable by an agent:
  • AC1 (debug capture to /tmp/hook-debug.json) — executable, smart diagnostic-first approach prevents guessing at the JSON shape
  • AC2 (all 3 hooks detect merged:true) — testable after fix, same pattern in all 3 files
  • AC3 (squash merge triggers /update-docs) — testable via MCP merge of a real PR
  • AC4 (failed merge shows error) — testable via intentional 405/409 scenario
  • AC5 (no regression for rebase/regular) — testable but requires multiple merge method PRs; may need to be validated opportunistically in production rather than synthetically
Note: AC5 is broad — rebase and regular merge regression testing may exceed a single agent pass if done synthetically. Recommend validating AC5 opportunistically (next rebase-method merge confirms no regression) rather than blocking on it.

Blast Radius

  • label-on-pr.sh and label-on-branch.sh also parse .tool_response.result but for different fields (PR URL, issue number) and on different trigger tools (submit_pr, create_issue_and_branch). Not directly affected by this bug, but if the overall PostToolUse JSON envelope changed, they could have similar parsing fragility. Consider a follow-up audit ticket if the debug capture (AC1) reveals an envelope-level change.
  • block-mcp-merge.sh is PreToolUse only — not affected.
  • No downstream consumers of these hooks' output beyond agent context injection.

Decomposition

3 file targets, 1 repo, 5 AC — but all AC target a single duplicated parse pattern. The fix is one jq expression replicated across 3 files. Estimated agent time: under 5 minutes. No decomposition needed.

Recommendation

No action needed. Ticket is well-scoped with accurate file targets, complete traceability, debug-first AC ordering, and clear lineage to #173. Ready for next_up.