Convention: PreToolUse hook matchers must use separate entries, not pipe-separated

forgejo_admin commented

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

Contributor

Type

Feature

Lineage

Discovered scope from GroupMe incident remediation (2026-03-25). Four restart cycles to diagnose.

Repo

forgejo_admin/claude-custom

User Story

As platform operator,
I want a documented convention and audit for PreToolUse hook matchers,
So that future hooks work correctly in --dangerously-skip-permissions mode without multi-restart debugging cycles.

Context

Finding (2026-03-25): PreToolUse hooks with pipe-separated matchers (e.g., mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member) do NOT enforce permissionDecision: "ask" in --dangerously-skip-permissions mode. The hook fires (sentinel file proves it), the JSON output is correct, but the permission prompt is silently swallowed.

Splitting the same matcher into three separate entries fixes it. Single-tool matchers like mcp__forgejo__merge_approved_pr always worked — the pipe separator is the problem.

This cost four session restarts, multiple test messages to the wrong group, and hours of debugging. The fix is simple but the pattern needs to be documented so no future hook repeats this mistake.

File Targets

Files to modify:

settings.json — AUDIT all existing pipe-separated PreToolUse matchers. Split any that use permissionDecision: "ask" into separate entries. Currently affected: the groupme entries NEEDS splitting — line 103 still pipe-separated. Check if mcp__forgejo__create_issue|mcp__forgejo__create_issue_and_branch or Write|Edit|NotebookEdit have the same issue.
pal-e-docs convention note — CREATE convention-hook-matcher-pattern documenting: (1) pipe-separated matchers work for PostToolUse but NOT reliably for PreToolUse permissionDecision: "ask" in bypass mode, (2) always use separate entries for PreToolUse approval hooks

Files NOT to touch:

Hook scripts themselves — the scripts are fine, it's the matcher config

Acceptance Criteria

All PreToolUse matchers that return permissionDecision: "ask" use separate entries (one tool per matcher)
Convention note convention-hook-matcher-pattern created in pal-e-docs
Convention documents the specific failure mode: pipe-separated + ask + bypass mode = silently swallowed
Existing pipe-separated PreToolUse matchers audited — split if they return "ask"

Test Expectations

Manual test: verify each split matcher fires the approval prompt in --dangerously-skip-permissions mode
Run command: N/A (manual session tests)

Constraints

PostToolUse matchers with pipes appear to work fine — don't split those unnecessarily
Only PreToolUse matchers returning permissionDecision: "ask" are affected
This may be a Claude Code bug — document as a known workaround, not a permanent convention

Checklist

PR opened
Tests pass
No unrelated changes

claude-custom#160 — GroupMe incident that exposed this
claude-custom#164 — auto-pull hook (related deployment gap)
project-pal-e-agency — enforcement architecture

### Type Feature ### Lineage Discovered scope from GroupMe incident remediation (2026-03-25). Four restart cycles to diagnose. ### Repo `forgejo_admin/claude-custom` ### User Story As platform operator, I want a documented convention and audit for PreToolUse hook matchers, So that future hooks work correctly in `--dangerously-skip-permissions` mode without multi-restart debugging cycles. ### Context **Finding (2026-03-25):** PreToolUse hooks with pipe-separated matchers (e.g., `mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member`) do NOT enforce `permissionDecision: "ask"` in `--dangerously-skip-permissions` mode. The hook fires (sentinel file proves it), the JSON output is correct, but the permission prompt is silently swallowed. **Splitting the same matcher into three separate entries fixes it.** Single-tool matchers like `mcp__forgejo__merge_approved_pr` always worked — the pipe separator is the problem. This cost four session restarts, multiple test messages to the wrong group, and hours of debugging. The fix is simple but the pattern needs to be documented so no future hook repeats this mistake. ### File Targets Files to modify: - `settings.json` — AUDIT all existing pipe-separated PreToolUse matchers. Split any that use `permissionDecision: "ask"` into separate entries. Currently affected: the groupme entries NEEDS splitting — line 103 still pipe-separated. Check if `mcp__forgejo__create_issue|mcp__forgejo__create_issue_and_branch` or `Write|Edit|NotebookEdit` have the same issue. - pal-e-docs convention note — CREATE `convention-hook-matcher-pattern` documenting: (1) pipe-separated matchers work for PostToolUse but NOT reliably for PreToolUse `permissionDecision: "ask"` in bypass mode, (2) always use separate entries for PreToolUse approval hooks Files NOT to touch: - Hook scripts themselves — the scripts are fine, it's the matcher config ### Acceptance Criteria - [ ] All PreToolUse matchers that return `permissionDecision: "ask"` use separate entries (one tool per matcher) - [ ] Convention note `convention-hook-matcher-pattern` created in pal-e-docs - [ ] Convention documents the specific failure mode: pipe-separated + ask + bypass mode = silently swallowed - [ ] Existing pipe-separated PreToolUse matchers audited — split if they return "ask" ### Test Expectations - [ ] Manual test: verify each split matcher fires the approval prompt in `--dangerously-skip-permissions` mode - Run command: N/A (manual session tests) ### Constraints - PostToolUse matchers with pipes appear to work fine — don't split those unnecessarily - Only PreToolUse matchers returning `permissionDecision: "ask"` are affected - This may be a Claude Code bug — document as a known workaround, not a permanent convention ### Checklist - [ ] PR opened - [ ] Tests pass - [ ] No unrelated changes ### Related - `claude-custom#160` — GroupMe incident that exposed this - `claude-custom#164` — auto-pull hook (related deployment gap) - `project-pal-e-agency` — enforcement architecture

forgejo_admin commented

2026-03-26 05:43:40 +00:00

Author

Contributor

Scope Review: READY

Review note: review-377-2026-03-25
Scope is solid — all template sections present, file targets verified, no blocking dependencies. Audit finding: the two remaining pipe-separated PreToolUse matchers (Write|Edit|NotebookEdit and create_issue|create_issue_and_branch) do NOT return permissionDecision: "ask", so they are not affected by this bug and do not need splitting. Remaining work is convention note creation + optional config annotation.

## Scope Review: READY Review note: `review-377-2026-03-25` Scope is solid — all template sections present, file targets verified, no blocking dependencies. Audit finding: the two remaining pipe-separated PreToolUse matchers (`Write|Edit|NotebookEdit` and `create_issue|create_issue_and_branch`) do NOT return `permissionDecision: "ask"`, so they are not affected by this bug and do not need splitting. Remaining work is convention note creation + optional config annotation.

forgejo_admin commented

2026-03-27 21:08:18 +00:00

Author

Contributor

Scope Review: NEEDS_REFINEMENT

Review note: review-377-2026-03-27
Template complete, traceability complete, acceptance criteria testable, blast radius clean.

One factual error blocks READY status:

File Targets section claims GroupMe entries are "already split (fixed during incident)" — verified false. Line 103 of settings.json still has the pipe-separated matcher mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member. An implementing agent may skip the split if it reads "already done."

Fix needed: Update the File Targets bullet for settings.json to say the GroupMe matcher NEEDS splitting, not that it's already split. Once corrected, this upgrades to READY.

## Scope Review: NEEDS_REFINEMENT Review note: `review-377-2026-03-27` Template complete, traceability complete, acceptance criteria testable, blast radius clean. **One factual error blocks READY status:** - File Targets section claims GroupMe entries are "already split (fixed during incident)" — verified false. Line 103 of `settings.json` still has the pipe-separated matcher `mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member`. An implementing agent may skip the split if it reads "already done." **Fix needed:** Update the File Targets bullet for `settings.json` to say the GroupMe matcher NEEDS splitting, not that it's already split. Once corrected, this upgrades to READY.

forgejo_admin commented

2026-03-27 21:24:44 +00:00

Author

Contributor

Scope Review Corrections Needed

From review-377-2026-03-27:

Fix 1: File Targets section says GroupMe matcher entries are "already split (fixed during incident)" — this is incorrect. Line 103 of settings.json still has the pipe-separated matcher: mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member. Update the issue body to say this NEEDS splitting.

Once fixed → READY for next_up.

## Scope Review Corrections Needed From `review-377-2026-03-27`: **Fix 1:** File Targets section says GroupMe matcher entries are "already split (fixed during incident)" — this is **incorrect**. Line 103 of `settings.json` still has the pipe-separated matcher: `mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member`. Update the issue body to say this NEEDS splitting. Once fixed → READY for next_up.

forgejo_admin commented

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

Author

Contributor

Issue body updated per scope review corrections.