fix: parse MCP-wrapped tool_response.result in 3 merge hooks #177

Merged

forgejo_admin merged 1 commit from 173-fix-mcp-response-parsing into main

2026-03-27 21:37:10 +00:00

forgejo_admin commented

2026-03-27 21:36:22 +00:00

Contributor

Summary

Three PostToolUse hooks that fire on mcp__forgejo__merge_approved_pr were parsing .tool_response.merged directly, but MCP wraps responses in .tool_response.result as a JSON string. This caused false positive "merge not successful" messages even when merges succeeded (observed on pal-e-platform PR #192).

Changes

hooks/remind-update-docs.sh -- try .tool_response.merged first, fall back to parsing .tool_response.result JSON string
hooks/post-mcp-merge-rebase.sh -- same fix
hooks/board-item-on-merge.sh -- same fix

All three now follow the same pattern already used by label-on-pr.sh and label-on-branch.sh.

Test Plan

Verified with MCP-wrapped payload {"tool_response":{"result":"{\"merged\": true, ...}"}} -- all 3 hooks correctly detect success
Verified with direct payload {"tool_response":{"merged":true}} -- still works (backward compatible)
Verified with failed merge payload {"merged": false} -- correctly reports failure
Next real merge via MCP will confirm end-to-end

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

forgejo_admin/claude-custom#173 -- the Forgejo issue this PR implements
project-pal-e-agency -- hooks are agency infrastructure

Closes #173

## Summary Three PostToolUse hooks that fire on `mcp__forgejo__merge_approved_pr` were parsing `.tool_response.merged` directly, but MCP wraps responses in `.tool_response.result` as a JSON string. This caused false positive "merge not successful" messages even when merges succeeded (observed on pal-e-platform PR #192). ## Changes - `hooks/remind-update-docs.sh` -- try `.tool_response.merged` first, fall back to parsing `.tool_response.result` JSON string - `hooks/post-mcp-merge-rebase.sh` -- same fix - `hooks/board-item-on-merge.sh` -- same fix All three now follow the same pattern already used by `label-on-pr.sh` and `label-on-branch.sh`. ## Test Plan - [x] Verified with MCP-wrapped payload `{"tool_response":{"result":"{\"merged\": true, ...}"}}` -- all 3 hooks correctly detect success - [x] Verified with direct payload `{"tool_response":{"merged":true}}` -- still works (backward compatible) - [x] Verified with failed merge payload `{"merged": false}` -- correctly reports failure - [ ] Next real merge via MCP will confirm end-to-end ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - `forgejo_admin/claude-custom#173` -- the Forgejo issue this PR implements - `project-pal-e-agency` -- hooks are agency infrastructure Closes #173

forgejo_admin added 1 commit

2026-03-27 21:36:22 +00:00

fix: parse MCP-wrapped tool_response.result in 3 merge hooks b72feb9932

MCP wraps tool responses in .tool_response.result as a JSON string,
but remind-update-docs.sh, post-mcp-merge-rebase.sh, and
board-item-on-merge.sh only checked .tool_response.merged (which
doesn't exist in the MCP response structure). This caused false
positive "merge not successful" messages even when merges succeeded.

Now all three hooks try .tool_response.merged first (direct format),
then fall back to parsing .tool_response.result as JSON -- matching
the pattern already used by label-on-pr.sh and label-on-branch.sh.

Closes #173

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-27 21:36:48 +00:00

Author

Contributor

Self-Review

Verdict: PASS

What was reviewed

3 files changed, 21 additions, 3 deletions
Identical 6-line fix applied to all 3 hooks: remind-update-docs.sh, post-mcp-merge-rebase.sh, board-item-on-merge.sh

Findings

Correctness: The fix tries .tool_response.merged first (direct format), then falls back to parsing .tool_response.result as a JSON string. This handles both MCP-wrapped and hypothetical direct response formats.
Consistency: All 3 hooks now use the same pattern already established by label-on-pr.sh and label-on-branch.sh.
Fail-open preserved: If both jq extractions fail, $MERGED remains empty, which means != "true" evaluates correctly -- merge is treated as failed, matching the original fail-safe behavior.
No secrets, no unrelated changes, no regressions.

Test results

MCP-wrapped success payload: all 3 hooks emit correct success output
Direct success payload: backward compatible, all 3 hooks emit correct success output
Failed merge payload: all 3 hooks correctly detect failure

No issues found. Ready for human review.

## Self-Review **Verdict: PASS** ### What was reviewed - 3 files changed, 21 additions, 3 deletions - Identical 6-line fix applied to all 3 hooks: `remind-update-docs.sh`, `post-mcp-merge-rebase.sh`, `board-item-on-merge.sh` ### Findings - **Correctness**: The fix tries `.tool_response.merged` first (direct format), then falls back to parsing `.tool_response.result` as a JSON string. This handles both MCP-wrapped and hypothetical direct response formats. - **Consistency**: All 3 hooks now use the same pattern already established by `label-on-pr.sh` and `label-on-branch.sh`. - **Fail-open preserved**: If both jq extractions fail, `$MERGED` remains empty, which means `!= "true"` evaluates correctly -- merge is treated as failed, matching the original fail-safe behavior. - **No secrets, no unrelated changes, no regressions.** ### Test results - MCP-wrapped success payload: all 3 hooks emit correct success output - Direct success payload: backward compatible, all 3 hooks emit correct success output - Failed merge payload: all 3 hooks correctly detect failure No issues found. Ready for human review.