fix: split pipe-separated GroupMe PreToolUse matchers #178

Merged
forgejo_admin merged 1 commit from 166-split-groupme-pretooluse-matchers into main 2026-03-27 21:57:30 +00:00
Contributor

Summary

Pipe-separated PreToolUse matchers silently swallow permissionDecision: "ask" in --dangerously-skip-permissions mode. This splits the single GroupMe entry into three separate PreToolUse entries so each tool fires the approval prompt correctly.

Changes

  • settings.json -- Split mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member into three separate PreToolUse entries, each with its own matcher and the same block-groupme-send.sh hook command
  • hooks/block-groupme-send.sh -- Updated comment header to document the split pattern and reference #166

Audit of Other Pipe-Separated Matchers

  • Write|Edit|NotebookEdit (line 81): block-claude-custom-main-edit.sh returns deny, check-issue.sh uses exit 2. Neither returns ask. No split needed.
  • mcp__forgejo__create_issue|mcp__forgejo__create_issue_and_branch (line 130): check-issue-template.sh returns deny, check-domain-label.sh returns allow. Neither returns ask. No split needed.

Test Plan

  • Manual: verify each of mcp__groupme__send_message, mcp__groupme__add_member, mcp__groupme__remove_member fires the approval prompt in --dangerously-skip-permissions mode
  • Verify settings.json is valid JSON (validated during implementation)

Review Checklist

  • All PreToolUse matchers returning permissionDecision: "ask" use separate entries
  • Existing pipe-separated PreToolUse matchers audited
  • settings.json validates as valid JSON
  • No unrelated changes
  • Convention note convention-hook-matcher-pattern is called out in the issue but owned by pal-e-docs (out of scope for this repo PR)
  • Closes #166
  • Root cause: claude-custom#160 (GroupMe incident 2026-03-25)
## Summary Pipe-separated PreToolUse matchers silently swallow `permissionDecision: "ask"` in `--dangerously-skip-permissions` mode. This splits the single GroupMe entry into three separate PreToolUse entries so each tool fires the approval prompt correctly. ## Changes - `settings.json` -- Split `mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member` into three separate PreToolUse entries, each with its own matcher and the same `block-groupme-send.sh` hook command - `hooks/block-groupme-send.sh` -- Updated comment header to document the split pattern and reference #166 ## Audit of Other Pipe-Separated Matchers - `Write|Edit|NotebookEdit` (line 81): `block-claude-custom-main-edit.sh` returns `deny`, `check-issue.sh` uses `exit 2`. Neither returns `ask`. **No split needed.** - `mcp__forgejo__create_issue|mcp__forgejo__create_issue_and_branch` (line 130): `check-issue-template.sh` returns `deny`, `check-domain-label.sh` returns `allow`. Neither returns `ask`. **No split needed.** ## Test Plan - Manual: verify each of `mcp__groupme__send_message`, `mcp__groupme__add_member`, `mcp__groupme__remove_member` fires the approval prompt in `--dangerously-skip-permissions` mode - Verify `settings.json` is valid JSON (validated during implementation) ## Review Checklist - [x] All PreToolUse matchers returning `permissionDecision: "ask"` use separate entries - [x] Existing pipe-separated PreToolUse matchers audited - [x] `settings.json` validates as valid JSON - [x] No unrelated changes ## Related Notes - Convention note `convention-hook-matcher-pattern` is called out in the issue but owned by pal-e-docs (out of scope for this repo PR) ## Related - Closes #166 - Root cause: claude-custom#160 (GroupMe incident 2026-03-25)
Pipe-separated matchers silently swallow permissionDecision "ask" in
--dangerously-skip-permissions mode. Split the single GroupMe entry into
three separate PreToolUse entries (send_message, add_member, remove_member)
so each fires the approval prompt correctly.

Audited all other pipe-separated PreToolUse matchers:
- Write|Edit|NotebookEdit: returns deny/exit 2 (not ask) -- no split needed
- create_issue|create_issue_and_branch: returns deny/allow (not ask) -- no split needed

Closes #166

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

QA Review

Scope: 2 files, +22/-2. Config-only change with a comment update.

Findings

settings.json

  • Single pipe-separated matcher correctly split into three separate PreToolUse entries
  • Each entry points to the same block-groupme-send.sh hook -- correct, the script's case statement already handles each tool name independently
  • JSON validated as syntactically correct
  • No other PreToolUse entries were modified -- audit of Write|Edit|NotebookEdit and create_issue|create_issue_and_branch documented in PR body with sound reasoning (they return deny/allow, not ask)

hooks/block-groupme-send.sh

  • Comment-only change. No logic modifications.
  • Documents the split pattern and references #166 for traceability

Nits

None.

VERDICT: APPROVE

## QA Review **Scope:** 2 files, +22/-2. Config-only change with a comment update. ### Findings **settings.json** - Single pipe-separated matcher correctly split into three separate PreToolUse entries - Each entry points to the same `block-groupme-send.sh` hook -- correct, the script's `case` statement already handles each tool name independently - JSON validated as syntactically correct - No other PreToolUse entries were modified -- audit of `Write|Edit|NotebookEdit` and `create_issue|create_issue_and_branch` documented in PR body with sound reasoning (they return `deny`/`allow`, not `ask`) **hooks/block-groupme-send.sh** - Comment-only change. No logic modifications. - Documents the split pattern and references #166 for traceability ### Nits None. ### VERDICT: APPROVE
forgejo_admin deleted branch 166-split-groupme-pretooluse-matchers 2026-03-27 21:57:30 +00:00
Sign in to join this conversation.
No description provided.