ldraney/claude-custom
1
0
Fork 0
Code Issues 32 Pull requests Projects Releases Packages Wiki Activity Actions

Bug: merge_approved_pr post-hook reports false positive failure #173

New issue
Closed
opened 2026-03-27 01:59:29 +00:00 by forgejo_admin · 2 comments
forgejo_admin commented 2026-03-27 01:59:29 +00:00
Contributor
Copy link

Type

Bug

Lineage

Discovered during pal-e-platform PR #192 merge (2026-03-27).

Repo

forgejo_admin/claude-custom

What Broke

The PostToolUse:mcp__forgejo__merge_approved_pr hook emits "Merge was not successful. Do NOT run /update-docs." even when the MCP tool returns a successful merge response: {"merged": true, "method": "squash", "message": "PR #192 merged via squash"}.

This causes confusion about whether the merge actually happened and blocks the /update-docs SOP flow.

Repro Steps

  1. Have a QA-approved PR ready to merge
  2. Call mcp__forgejo__merge_approved_pr via MCP — merge succeeds
  3. Observe: hook fires with "Merge was not successful" despite merged: true in response
  4. Verify via mcp__forgejo__review_pr — PR is confirmed closed+merged

Expected Behavior

When the merge API returns merged: true, the post-hook should NOT report failure. It should either stay silent or emit a success message.

Environment

  • Cluster/namespace: local (claude-custom hooks run on archbox)
  • Service version: current main branch of claude-custom
  • Related alerts: none — this is a developer tooling issue

Acceptance Criteria

  • Hook correctly detects merged: true in MCP response
  • Hook does NOT emit false positive failure on successful squash merge
  • Hook still correctly reports actual merge failures (405, 409, etc.)

Related

  • project-pal-e-agency — hooks are agency infrastructure
  • forgejo_admin/pal-e-platform#192 — PR where false positive was observed
### Type Bug ### Lineage Discovered during pal-e-platform PR #192 merge (2026-03-27). ### Repo `forgejo_admin/claude-custom` ### What Broke The `PostToolUse:mcp__forgejo__merge_approved_pr` hook emits "Merge was not successful. Do NOT run /update-docs." even when the MCP tool returns a successful merge response: `{"merged": true, "method": "squash", "message": "PR #192 merged via squash"}`. This causes confusion about whether the merge actually happened and blocks the /update-docs SOP flow. ### Repro Steps 1. Have a QA-approved PR ready to merge 2. Call `mcp__forgejo__merge_approved_pr` via MCP — merge succeeds 3. Observe: hook fires with "Merge was not successful" despite `merged: true` in response 4. Verify via `mcp__forgejo__review_pr` — PR is confirmed closed+merged ### Expected Behavior When the merge API returns `merged: true`, the post-hook should NOT report failure. It should either stay silent or emit a success message. ### Environment - Cluster/namespace: local (claude-custom hooks run on archbox) - Service version: current main branch of claude-custom - Related alerts: none — this is a developer tooling issue ### Acceptance Criteria - [ ] Hook correctly detects `merged: true` in MCP response - [ ] Hook does NOT emit false positive failure on successful squash merge - [ ] Hook still correctly reports actual merge failures (405, 409, etc.) ### Related - `project-pal-e-agency` — hooks are agency infrastructure - `forgejo_admin/pal-e-platform#192` — PR where false positive was observed
forgejo_admin commented 2026-03-27 21:32:31 +00:00
Author
Contributor
Copy link

Scope Review: READY

Review note: review-426-2026-03-27

Root cause identified: all 3 PostToolUse hooks for mcp__forgejo__merge_approved_pr use .tool_response.merged but MCP tool responses wrap the return value inside .tool_response.result as a JSON string. The jq expression never finds merged at the expected path.

Blast radius — 3 hooks affected, not 1:

  • remind-update-docs.sh (line 20) — emits false positive "Merge was not successful" (the reported symptom)
  • post-mcp-merge-rebase.sh (line 12) — silently skips local main fast-forward
  • board-item-on-merge.sh (line 36) — silently skips auto-move to done

Before moving to next_up:

  • Add story:dev-execute label (traceability gap)
  • Update issue body to name all 3 affected files
## Scope Review: READY Review note: `review-426-2026-03-27` Root cause identified: all 3 PostToolUse hooks for `mcp__forgejo__merge_approved_pr` use `.tool_response.merged` but MCP tool responses wrap the return value inside `.tool_response.result` as a JSON string. The jq expression never finds `merged` at the expected path. **Blast radius — 3 hooks affected, not 1:** - `remind-update-docs.sh` (line 20) — emits false positive "Merge was not successful" (the reported symptom) - `post-mcp-merge-rebase.sh` (line 12) — silently skips local main fast-forward - `board-item-on-merge.sh` (line 36) — silently skips auto-move to done **Before moving to next_up:** - Add `story:dev-execute` label (traceability gap) - Update issue body to name all 3 affected files
forgejo_admin commented 2026-03-27 21:48:08 +00:00
Author
Contributor
Copy link

Validation: PASS

Validation note: validation-173-2026-03-27
5 checks: 5 PASS, 0 FAIL

All acceptance criteria verified against live main branch (commit a742d5f, PR #177). All 3 merge hooks (remind-update-docs.sh, post-mcp-merge-rebase.sh, board-item-on-merge.sh) now have identical two-stage jq parsing for .tool_response.result, matching the label-on-pr.sh reference pattern. False positive eliminated; failure path still correct.

## Validation: PASS Validation note: `validation-173-2026-03-27` 5 checks: 5 PASS, 0 FAIL All acceptance criteria verified against live main branch (commit a742d5f, PR #177). All 3 merge hooks (remind-update-docs.sh, post-mcp-merge-rebase.sh, board-item-on-merge.sh) now have identical two-stage jq parsing for `.tool_response.result`, matching the label-on-pr.sh reference pattern. False positive eliminated; failure path still correct.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
217-enhance-skill-review-ticket-verify-arch
86-epilogue-nits-hook-updates-dottie-config
76-add-sessionstart-hook-to-detect-missing
69-fix-block-docs-writes-sh-security-gap-ad
51-phase-3-agent-label-behavior-posttooluse
No results found.
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ldraney/claude-custom#173
No description provided.