Bug: merge_approved_pr has no approval gate hook #163

New issue

Open

opened 2026-03-26 03:39:39 +00:00 by forgejo_admin · 2 comments

forgejo_admin commented

2026-03-26 03:39:39 +00:00

Owner

Type

Bug

Lineage

Standalone — discovered during svelte-playground deploy (pal-e-deployments PR #48)

Repo

ldraney/claude-custom

What Broke

mcp__forgejo__merge_approved_pr has no PreToolUse hook enforcing explicit user approval. Betty Sue called the merge tool and it went through immediately — no confirmation required. The tool description says "Only call after explicit user approval" but nothing enforces this. The behavioral rule (feedback_no_merge_without_approval: "QA approved ≠ merge approved") is memory-only, not hook-enforced.

Repro Steps

QA approves a PR via the review-fix loop
Agent calls mcp__forgejo__merge_approved_pr without user saying "merge"
Observe: PR merges successfully with no hook blocking

Expected Behavior

A PreToolUse hook on mcp__forgejo__merge_approved_pr should block execution and require explicit user confirmation before the merge proceeds. Similar to how check-issue-template.sh gates issue creation.

Environment

Cluster/namespace: local (claude-custom hooks)
Service version/commit: current main of ~/claude-custom
Related alerts: none — this is a process gap, not a runtime error

Acceptance Criteria

Calling mcp__forgejo__merge_approved_pr triggers a PreToolUse hook
Hook blocks with a message requiring explicit user approval
No false blocks when user has explicitly said "merge"

feedback_no_merge_without_approval — the behavioral rule this should enforce
sop-hook-block-recovery — recovery procedure when hooks block unexpectedly
project-pal-e-agency — hooks/config infrastructure

### Type Bug ### Lineage Standalone — discovered during svelte-playground deploy (pal-e-deployments PR #48) ### Repo `ldraney/claude-custom` ### What Broke `mcp__forgejo__merge_approved_pr` has no PreToolUse hook enforcing explicit user approval. Betty Sue called the merge tool and it went through immediately — no confirmation required. The tool description says "Only call after explicit user approval" but nothing enforces this. The behavioral rule (`feedback_no_merge_without_approval`: "QA approved ≠ merge approved") is memory-only, not hook-enforced. ### Repro Steps 1. QA approves a PR via the review-fix loop 2. Agent calls `mcp__forgejo__merge_approved_pr` without user saying "merge" 3. Observe: PR merges successfully with no hook blocking ### Expected Behavior A PreToolUse hook on `mcp__forgejo__merge_approved_pr` should block execution and require explicit user confirmation before the merge proceeds. Similar to how `check-issue-template.sh` gates issue creation. ### Environment - Cluster/namespace: local (claude-custom hooks) - Service version/commit: current main of `~/claude-custom` - Related alerts: none — this is a process gap, not a runtime error ### Acceptance Criteria - [ ] Calling `mcp__forgejo__merge_approved_pr` triggers a PreToolUse hook - [ ] Hook blocks with a message requiring explicit user approval - [ ] No false blocks when user has explicitly said "merge" ### Related - `feedback_no_merge_without_approval` — the behavioral rule this should enforce - `sop-hook-block-recovery` — recovery procedure when hooks block unexpectedly - `project-pal-e-agency` — hooks/config infrastructure

forgejo_admin commented

2026-03-27 21:05:59 +00:00

Author

Owner

Scope Review: BLOCK

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

The bug described does not exist — the hook block-mcp-merge.sh was implemented on 2026-02-24 and is correctly wired in settings.json as a PreToolUse matcher on mcp__forgejo__merge_approved_pr.

All 3 acceptance criteria are already satisfied by existing code in ~/claude-custom
Repo placement mismatch: issue body says Repo: ldraney/claude-custom but this issue is filed under forgejo_admin/pal-e-platform
Scope is fundamentally invalid: no code change is needed; the described problem was solved over a month ago

Recommend closing as invalid. If the hook was genuinely bypassed in a session, a new issue should be filed with specific repro evidence against forgejo_admin/claude-custom.

## Scope Review: BLOCK Review note: `review-363-2026-03-27` The bug described does not exist — the hook `block-mcp-merge.sh` was implemented on 2026-02-24 and is correctly wired in `settings.json` as a PreToolUse matcher on `mcp__forgejo__merge_approved_pr`. - **All 3 acceptance criteria are already satisfied** by existing code in `~/claude-custom` - **Repo placement mismatch**: issue body says `Repo: ldraney/claude-custom` but this issue is filed under `forgejo_admin/pal-e-platform` - **Scope is fundamentally invalid**: no code change is needed; the described problem was solved over a month ago Recommend closing as invalid. If the hook was genuinely bypassed in a session, a new issue should be filed with specific repro evidence against `forgejo_admin/claude-custom`.

forgejo_admin commented

2026-03-27 21:11:11 +00:00

Author

Owner

Closing — Bug Does Not Exist

Scope review (review-363-2026-03-27) confirmed the hook infrastructure is already in place:

~/claude-custom/hooks/block-mcp-merge.sh — uses permissionDecision: "ask" to force user confirmation
~/claude-custom/settings.json lines 94-101 — PreToolUse matcher on mcp__forgejo__merge_approved_pr wired correctly
Both merge vectors (Bash CLI + MCP tool) are gated

All 3 acceptance criteria are already met by existing code. Closing as invalid.

## Closing — Bug Does Not Exist Scope review (`review-363-2026-03-27`) confirmed the hook infrastructure is already in place: - `~/claude-custom/hooks/block-mcp-merge.sh` — uses `permissionDecision: "ask"` to force user confirmation - `~/claude-custom/settings.json` lines 94-101 — PreToolUse matcher on `mcp__forgejo__merge_approved_pr` wired correctly - Both merge vectors (Bash CLI + MCP tool) are gated All 3 acceptance criteria are already met by existing code. Closing as invalid.