ldraney/claude-custom

Fork 0

Hook + MCP fix: GroupMe send_message requires name-based resolution + user approval #160

New issue

Closed

opened 2026-03-26 02:43:02 +00:00 by forgejo_admin · 7 comments

forgejo_admin commented

2026-03-26 02:43:02 +00:00

Contributor

Type

Feature

Lineage

Standalone incident-fix — no plan phase. Triggered by 2026-03-25 data exposure incident.

Repo

forgejo_admin/claude-custom (hook + settings.json)
forgejo_admin/groupme-mcp (tool interface change)

User Story

As platform operator,
I want GroupMe messaging to resolve groups by name and require explicit user approval before sending,
So that sensitive data never reaches the wrong group again.

Context

Incident (2026-03-25): Contract status update with parent names and email addresses was sent to the 6-person Coaches & Staff group (113983384) instead of the 2-person WKQ Stakeholders group (113996175). A forked session inherited conversation context and fired mcp__groupme__send_message with the wrong group_id. Marcus caught it and deleted it within 74 seconds. Coaches may have seen it.

Root cause — two failures compounded:

send_message takes raw group_id — caller grabbed wrong ID from a stale docs table that was missing the Stakeholders group
No PreToolUse hook existed to gate GroupMe sends — --dangerously-skip-permissions bypassed all approval

Fix approach: Name-based resolution (eliminate stale IDs) + mandatory human approval (PreToolUse hook). The MCP calls list_groups() live from GroupMe API at send time — no caching, no Postgres sync, no drift.

File Targets

Files to modify or create:

claude-custom:

hooks/block-groupme-send.sh — NEW. PreToolUse hook, follows block-mcp-merge.sh pattern. Extracts group_name from tool_input, formats tool-specific summary, returns permissionDecision: "ask".
settings.json — ADD PreToolUse entry matching mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member
tests/test_block_groupme_send.sh — NEW. Hook unit tests (create tests/ directory).

groupme-mcp:

src/groupme_mcp/server.py — ADD shared _resolve_group(group_name) helper. Calls list_groups(per_page=100) or paginates, fuzzy-matches name, returns group dict. Ambiguous → error with options. No match → error with available names.
src/groupme_mcp/tools/messages.py — MODIFY send_message: group_name replaces group_id
src/groupme_mcp/tools/members.py — MODIFY add_member, remove_member, list_members: group_name replaces group_id
src/groupme_mcp/tools/groups.py — MODIFY get_group: group_name replaces group_id

Files NOT to touch:

groupme-sdk — SDK keeps raw group_id interface, MCP resolves above it

Acceptance Criteria

All 5 group-scoped tools (send_message, add_member, remove_member, list_members, get_group) accept group_name, not raw group_id
Shared _resolve_group() helper in server.py used by all 5 tools
_resolve_group() calls list_groups(per_page=100) or paginates — must not silently miss groups (currently 10 groups, default page size is 10)
Ambiguous or missing group name returns helpful error listing available groups
PreToolUse hook fires on 3 write tools: send_message, add_member, remove_member
Read-only tools (list_groups, list_members, get_group) excluded from hook — they don't modify external state, no approval needed
Hook shows group name and tool-specific action summary in permission prompt:
- send_message: "Send to {group_name}: {text preview}"
- add_member: "Add {nickname} to {group_name}"
- remove_member: "Remove member {membership_id} from {group_name}"
Hook unit tests for all 3 write tool input shapes
groupme-mcp tests updated for all 5 tools' new interface
Deploy order: groupme-mcp first, then claude-custom

Test Expectations

Unit test: send_message with exact group name resolves correctly
Unit test: ambiguous name returns multiple options without sending
Unit test: no match returns error with available group names
Unit test: _resolve_group handles pagination (>10 groups)
Hook test: pipe mock JSON with tool_input.group_name + tool_input.text through block-groupme-send.sh, verify output has permissionDecision: "ask" and reason contains group name
Hook test: pipe add_member shape (group_name + nickname), verify tool-specific summary
Hook test: pipe remove_member shape (group_name + membership_id), verify tool-specific summary
Run command: cd ~/groupme-mcp && pytest tests/ and bash ~/claude-custom/tests/test_block_groupme_send.sh

Constraints

Follow block-mcp-merge.sh pattern exactly for the hook
MCP must call GroupMe API live — no local caching of group names
SDK layer unchanged — only MCP tool interface changes
groupme-mcp uses uv not pip, Forgejo PyPI for groupme-sdk dependency
_resolve_group() must call list_groups(per_page=100) or paginate — default page size of 10 will silently miss groups at current count
Hook tests live in tests/ directory (create it). Each test is a shell script piping mock JSON through the hook and asserting output. Pattern: tests/test_block_groupme_send.sh
Deploy order: groupme-mcp MUST deploy before claude-custom hook. The hook extracts group_name from tool_input. If the hook deploys first while the old MCP still sends group_id, the permission prompt shows empty group name. Sequence: (1) merge + deploy groupme-mcp, (2) merge claude-custom hook.

Checklist

PR opened (groupme-mcp — deploys first)
PR opened (claude-custom — deploys second)
Tests pass
No unrelated changes
pal-e-docs project-groupme-westside updated (remove stale ID table, document live resolution)

project-groupme-westside — GroupMe project page (needs doc update)
project-pal-e-agency — enforcement architecture (hook layer)
sop-claude-config-development — claude-custom development SOP

Labels

story:GM-5, arch:enforcement, type:incident-fix

### Type Feature ### Lineage Standalone incident-fix — no plan phase. Triggered by 2026-03-25 data exposure incident. ### Repo `forgejo_admin/claude-custom` (hook + settings.json) `forgejo_admin/groupme-mcp` (tool interface change) ### User Story As platform operator, I want GroupMe messaging to resolve groups by name and require explicit user approval before sending, So that sensitive data never reaches the wrong group again. ### Context **Incident (2026-03-25):** Contract status update with parent names and email addresses was sent to the 6-person Coaches & Staff group (`113983384`) instead of the 2-person WKQ Stakeholders group (`113996175`). A forked session inherited conversation context and fired `mcp__groupme__send_message` with the wrong group_id. Marcus caught it and deleted it within 74 seconds. Coaches may have seen it. **Root cause — two failures compounded:** 1. `send_message` takes raw `group_id` — caller grabbed wrong ID from a stale docs table that was missing the Stakeholders group 2. No PreToolUse hook existed to gate GroupMe sends — `--dangerously-skip-permissions` bypassed all approval **Fix approach:** Name-based resolution (eliminate stale IDs) + mandatory human approval (PreToolUse hook). The MCP calls `list_groups()` live from GroupMe API at send time — no caching, no Postgres sync, no drift. ### File Targets Files to modify or create: **claude-custom:** - `hooks/block-groupme-send.sh` — NEW. PreToolUse hook, follows `block-mcp-merge.sh` pattern. Extracts `group_name` from `tool_input`, formats tool-specific summary, returns `permissionDecision: "ask"`. - `settings.json` — ADD PreToolUse entry matching `mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member` - `tests/test_block_groupme_send.sh` — NEW. Hook unit tests (create `tests/` directory). **groupme-mcp:** - `src/groupme_mcp/server.py` — ADD shared `_resolve_group(group_name)` helper. Calls `list_groups(per_page=100)` or paginates, fuzzy-matches name, returns group dict. Ambiguous → error with options. No match → error with available names. - `src/groupme_mcp/tools/messages.py` — MODIFY `send_message`: `group_name` replaces `group_id` - `src/groupme_mcp/tools/members.py` — MODIFY `add_member`, `remove_member`, `list_members`: `group_name` replaces `group_id` - `src/groupme_mcp/tools/groups.py` — MODIFY `get_group`: `group_name` replaces `group_id` Files NOT to touch: - `groupme-sdk` — SDK keeps raw `group_id` interface, MCP resolves above it ### Acceptance Criteria - [ ] All 5 group-scoped tools (`send_message`, `add_member`, `remove_member`, `list_members`, `get_group`) accept `group_name`, not raw `group_id` - [ ] Shared `_resolve_group()` helper in `server.py` used by all 5 tools - [ ] `_resolve_group()` calls `list_groups(per_page=100)` or paginates — must not silently miss groups (currently 10 groups, default page size is 10) - [ ] Ambiguous or missing group name returns helpful error listing available groups - [ ] PreToolUse hook fires on 3 write tools: `send_message`, `add_member`, `remove_member` - [ ] Read-only tools (`list_groups`, `list_members`, `get_group`) excluded from hook — they don't modify external state, no approval needed - [ ] Hook shows group name and tool-specific action summary in permission prompt: - send_message: "Send to {group_name}: {text preview}" - add_member: "Add {nickname} to {group_name}" - remove_member: "Remove member {membership_id} from {group_name}" - [ ] Hook unit tests for all 3 write tool input shapes - [ ] groupme-mcp tests updated for all 5 tools' new interface - [ ] Deploy order: groupme-mcp first, then claude-custom ### Test Expectations - [ ] Unit test: `send_message` with exact group name resolves correctly - [ ] Unit test: ambiguous name returns multiple options without sending - [ ] Unit test: no match returns error with available group names - [ ] Unit test: `_resolve_group` handles pagination (>10 groups) - [ ] Hook test: pipe mock JSON with `tool_input.group_name` + `tool_input.text` through `block-groupme-send.sh`, verify output has `permissionDecision: "ask"` and reason contains group name - [ ] Hook test: pipe `add_member` shape (group_name + nickname), verify tool-specific summary - [ ] Hook test: pipe `remove_member` shape (group_name + membership_id), verify tool-specific summary - Run command: `cd ~/groupme-mcp && pytest tests/` and `bash ~/claude-custom/tests/test_block_groupme_send.sh` ### Constraints - Follow `block-mcp-merge.sh` pattern exactly for the hook - MCP must call GroupMe API live — no local caching of group names - SDK layer unchanged — only MCP tool interface changes - groupme-mcp uses `uv` not pip, Forgejo PyPI for groupme-sdk dependency - `_resolve_group()` must call `list_groups(per_page=100)` or paginate — default page size of 10 will silently miss groups at current count - Hook tests live in `tests/` directory (create it). Each test is a shell script piping mock JSON through the hook and asserting output. Pattern: `tests/test_block_groupme_send.sh` - **Deploy order: groupme-mcp MUST deploy before claude-custom hook.** The hook extracts `group_name` from `tool_input`. If the hook deploys first while the old MCP still sends `group_id`, the permission prompt shows empty group name. Sequence: (1) merge + deploy groupme-mcp, (2) merge claude-custom hook. ### Checklist - [ ] PR opened (groupme-mcp — deploys first) - [ ] PR opened (claude-custom — deploys second) - [ ] Tests pass - [ ] No unrelated changes - [ ] pal-e-docs `project-groupme-westside` updated (remove stale ID table, document live resolution) ### Related - `project-groupme-westside` — GroupMe project page (needs doc update) - `project-pal-e-agency` — enforcement architecture (hook layer) - `sop-claude-config-development` — claude-custom development SOP ### Labels story:GM-5, arch:enforcement, type:incident-fix

forgejo_admin commented

2026-03-26 02:46:02 +00:00

Author

Contributor

Scope Review: NEEDS_REFINEMENT

Review note: review-357-2026-03-25
Ticket is well-structured with all template sections present, but name-based resolution scope is incomplete.

add_member and remove_member also take raw group_id — the hook gates all three tools, but the MCP interface fix only targets send_message. Same stale-ID vulnerability applies to member operations. Either extend File Targets to include members.py or document as deliberate Phase 2.
Hook must handle different tool_input shapes — ticket describes extracting group_name + text, but add_member/remove_member have different fields (nickname, membership_id). Hook logic needs to account for this.
Minor: "Hook works under --dangerously-skip-permissions" criterion is misleading — hooks always fire regardless of permission flags. Consider rewording or removing.

## Scope Review: NEEDS_REFINEMENT Review note: `review-357-2026-03-25` Ticket is well-structured with all template sections present, but name-based resolution scope is incomplete. - **add_member and remove_member also take raw group_id** — the hook gates all three tools, but the MCP interface fix only targets `send_message`. Same stale-ID vulnerability applies to member operations. Either extend File Targets to include `members.py` or document as deliberate Phase 2. - **Hook must handle different tool_input shapes** — ticket describes extracting `group_name` + `text`, but `add_member`/`remove_member` have different fields (nickname, membership_id). Hook logic needs to account for this. - Minor: "Hook works under --dangerously-skip-permissions" criterion is misleading — hooks always fire regardless of permission flags. Consider rewording or removing.

forgejo_admin commented

2026-03-26 03:05:36 +00:00

Author

Contributor

Scope Expansion (post-review `review-357-2026-03-25`)

Reviewer flagged two issues — both accepted. Expanding scope:

1. All group-scoped tools get name-based resolution (not just send_message)

add_member, remove_member, and list_members also take raw group_id. An agent could still call add_member(group_id="113983384") with a wrong ID even after send_message is fixed.

Fix: All four tools (send_message, add_member, remove_member, list_members) switch from group_id to group_name. Internal resolution via list_groups() for all.

Updated File Targets for groupme-mcp:

src/groupme_mcp/tools/messages.py — send_message: group_name replaces group_id
src/groupme_mcp/tools/members.py — add_member, remove_member, list_members: group_name replaces group_id
Consider a shared _resolve_group(group_name) helper in src/groupme_mcp/server.py that all tools call

2. Hook handles different tool_input shapes

The hook matches three write tools with different parameters:

send_message → has group_name + text
add_member → has group_name + nickname + contact info
remove_member → has group_name + membership_id

Fix: Hook extracts group_name (common to all) and formats a tool-specific summary:

send_message: "Send to {group_name}: {text preview}"
add_member: "Add {nickname} to {group_name}"
remove_member: "Remove member {membership_id} from {group_name}"

3. Minor: remove redundant acceptance criterion

"Hook works under --dangerously-skip-permissions" removed — inherently true for all hooks.

Updated Acceptance Criteria

All four tools (send_message, add_member, remove_member, list_members) accept group_name, not raw group_id
Ambiguous or missing group name returns helpful error listing available groups
Shared _resolve_group() helper used by all tools
PreToolUse hook fires on send_message, add_member, remove_member with tool-specific summaries
Hook shows group name and action-specific context in permission prompt
Forked/resumed sessions cannot bypass the gate
groupme-mcp tests updated for all four tools' new interface

## Scope Expansion (post-review `review-357-2026-03-25`) Reviewer flagged two issues — both accepted. Expanding scope: ### 1. All group-scoped tools get name-based resolution (not just send_message) `add_member`, `remove_member`, and `list_members` also take raw `group_id`. An agent could still call `add_member(group_id="113983384")` with a wrong ID even after send_message is fixed. **Fix:** All four tools (`send_message`, `add_member`, `remove_member`, `list_members`) switch from `group_id` to `group_name`. Internal resolution via `list_groups()` for all. Updated File Targets for groupme-mcp: - `src/groupme_mcp/tools/messages.py` — `send_message`: `group_name` replaces `group_id` - `src/groupme_mcp/tools/members.py` — `add_member`, `remove_member`, `list_members`: `group_name` replaces `group_id` - Consider a shared `_resolve_group(group_name)` helper in `src/groupme_mcp/server.py` that all tools call ### 2. Hook handles different tool_input shapes The hook matches three write tools with different parameters: - `send_message` → has `group_name` + `text` - `add_member` → has `group_name` + `nickname` + contact info - `remove_member` → has `group_name` + `membership_id` **Fix:** Hook extracts `group_name` (common to all) and formats a tool-specific summary: - send_message: "Send to {group_name}: {text preview}" - add_member: "Add {nickname} to {group_name}" - remove_member: "Remove member {membership_id} from {group_name}" ### 3. Minor: remove redundant acceptance criterion "Hook works under --dangerously-skip-permissions" removed — inherently true for all hooks. ### Updated Acceptance Criteria - [ ] All four tools (`send_message`, `add_member`, `remove_member`, `list_members`) accept `group_name`, not raw `group_id` - [ ] Ambiguous or missing group name returns helpful error listing available groups - [ ] Shared `_resolve_group()` helper used by all tools - [ ] PreToolUse hook fires on `send_message`, `add_member`, `remove_member` with tool-specific summaries - [ ] Hook shows group name and action-specific context in permission prompt - [ ] Forked/resumed sessions cannot bypass the gate - [ ] groupme-mcp tests updated for all four tools' new interface

forgejo_admin commented

2026-03-26 03:10:33 +00:00

Author

Contributor

Scope Review (re-review): NEEDS_REFINEMENT

Review note: review-357-2026-03-25-v2
The two issues from the first review are addressed by the scope expansion. Three new issues found after codebase verification.

get_group in groups.py also takes raw group_id -- 5th tool with the stale-ID vulnerability, not in scope. Either add to name-based resolution or document exclusion rationale.
Hook unit test dropped -- scope expansion's updated acceptance criteria omitted the hook test from the original issue body ("verify JSON output has permissionDecision: ask"). Should be restored.
Deployment ordering undocumented -- groupme-mcp must deploy before claude-custom (hook expects group_name in tool_input). Add to Constraints.

## Scope Review (re-review): NEEDS_REFINEMENT Review note: `review-357-2026-03-25-v2` The two issues from the first review are addressed by the scope expansion. Three new issues found after codebase verification. - **`get_group` in `groups.py` also takes raw `group_id`** -- 5th tool with the stale-ID vulnerability, not in scope. Either add to name-based resolution or document exclusion rationale. - **Hook unit test dropped** -- scope expansion's updated acceptance criteria omitted the hook test from the original issue body ("verify JSON output has `permissionDecision: ask`"). Should be restored. - **Deployment ordering undocumented** -- groupme-mcp must deploy before claude-custom (hook expects `group_name` in `tool_input`). Add to Constraints.

forgejo_admin commented

2026-03-26 03:11:16 +00:00

Author

Contributor

Scope Expansion v2 (post-review `review-357-2026-03-25-v2`)

Three issues flagged — all accepted.

1. `get_group` added to name-based resolution

src/groupme_mcp/tools/groups.py line 42 — get_group(group_id=...) is the 5th tool taking raw group_id. Read-only but same stale-ID vulnerability. Now included.

All 5 group-scoped tools get group_name: send_message, add_member, remove_member, list_members, get_group. All use shared _resolve_group() helper.

Updated File Targets for groupme-mcp:

src/groupme_mcp/tools/messages.py — send_message
src/groupme_mcp/tools/members.py — add_member, remove_member, list_members
src/groupme_mcp/tools/groups.py — get_group
src/groupme_mcp/server.py — shared _resolve_group(group_name) helper

2. Hook unit test restored

Add to Test Expectations:

Hook test: pipe mock JSON with tool_input.group_name + tool_input.text through block-groupme-send.sh, verify output JSON has permissionDecision: "ask" and reason contains group name
Hook test: pipe add_member shape (group_name + nickname), verify tool-specific summary
Hook test: pipe remove_member shape (group_name + membership_id), verify tool-specific summary

3. Deploy ordering constraint

Added to Constraints:

groupme-mcp MUST deploy before claude-custom hook. The hook extracts group_name from tool_input. If the hook deploys first while the old MCP still sends group_id, the permission prompt shows empty group name. Sequence: (1) merge + deploy groupme-mcp, (2) merge claude-custom hook.

4. Read-only exclusion rationale (non-blocking, documenting for implementing agent)

The PreToolUse hook matches write operations only: send_message, add_member, remove_member. Read-only tools (list_groups, list_members, get_group) are excluded from the hook — they don't modify external state and don't need human approval. list_members and get_group still get name-based resolution (consistency), just no approval gate.

Final Consolidated Acceptance Criteria

All 5 group-scoped tools accept group_name, not raw group_id
Shared _resolve_group() helper in server.py used by all 5 tools
Ambiguous or missing group name returns helpful error listing available groups
PreToolUse hook fires on 3 write tools: send_message, add_member, remove_member
Read-only tools (list_groups, list_members, get_group) excluded from hook — documented rationale
Hook shows group name and tool-specific action summary in permission prompt
Hook unit tests for all 3 write tool input shapes
groupme-mcp tests updated for all 5 tools' new interface
Deploy order: groupme-mcp first, then claude-custom

## Scope Expansion v2 (post-review `review-357-2026-03-25-v2`) Three issues flagged — all accepted. ### 1. `get_group` added to name-based resolution `src/groupme_mcp/tools/groups.py` line 42 — `get_group(group_id=...)` is the 5th tool taking raw `group_id`. Read-only but same stale-ID vulnerability. Now included. **All 5 group-scoped tools get `group_name`:** `send_message`, `add_member`, `remove_member`, `list_members`, `get_group`. All use shared `_resolve_group()` helper. Updated File Targets for groupme-mcp: - `src/groupme_mcp/tools/messages.py` — `send_message` - `src/groupme_mcp/tools/members.py` — `add_member`, `remove_member`, `list_members` - `src/groupme_mcp/tools/groups.py` — `get_group` - `src/groupme_mcp/server.py` — shared `_resolve_group(group_name)` helper ### 2. Hook unit test restored Add to Test Expectations: - [ ] Hook test: pipe mock JSON with `tool_input.group_name` + `tool_input.text` through `block-groupme-send.sh`, verify output JSON has `permissionDecision: "ask"` and reason contains group name - [ ] Hook test: pipe `add_member` shape (group_name + nickname), verify tool-specific summary - [ ] Hook test: pipe `remove_member` shape (group_name + membership_id), verify tool-specific summary ### 3. Deploy ordering constraint Added to Constraints: - **groupme-mcp MUST deploy before claude-custom hook.** The hook extracts `group_name` from `tool_input`. If the hook deploys first while the old MCP still sends `group_id`, the permission prompt shows empty group name. Sequence: (1) merge + deploy groupme-mcp, (2) merge claude-custom hook. ### 4. Read-only exclusion rationale (non-blocking, documenting for implementing agent) The PreToolUse hook matches **write operations only**: `send_message`, `add_member`, `remove_member`. Read-only tools (`list_groups`, `list_members`, `get_group`) are excluded from the hook — they don't modify external state and don't need human approval. `list_members` and `get_group` still get name-based resolution (consistency), just no approval gate. ### Final Consolidated Acceptance Criteria - [ ] All 5 group-scoped tools accept `group_name`, not raw `group_id` - [ ] Shared `_resolve_group()` helper in `server.py` used by all 5 tools - [ ] Ambiguous or missing group name returns helpful error listing available groups - [ ] PreToolUse hook fires on 3 write tools: `send_message`, `add_member`, `remove_member` - [ ] Read-only tools (`list_groups`, `list_members`, `get_group`) excluded from hook — documented rationale - [ ] Hook shows group name and tool-specific action summary in permission prompt - [ ] Hook unit tests for all 3 write tool input shapes - [ ] groupme-mcp tests updated for all 5 tools' new interface - [ ] Deploy order: groupme-mcp first, then claude-custom

forgejo_admin commented

2026-03-26 03:14:18 +00:00

Author

Contributor

Scope Review (re-review v3): NEEDS_REFINEMENT

Review note: review-357-2026-03-25-v3
Both v1 and v2 expansion issues are fully addressed. Two new issues found during codebase verification of the consolidated spec.

SDK list_groups pagination unaddressed — _resolve_group() will call SDK's list_groups(page=1, per_page=10) which returns max 10 groups. If the user belongs to >10 groups, name resolution silently misses groups and returns false "no match" errors. Add one line to Constraints: helper must paginate or use per_page=100.
Hook test location unspecified — claude-custom has no tests/ directory. Scope says "pipe mock JSON through hook" but doesn't say where test scripts live. Implementing agent needs a location decision.

## Scope Review (re-review v3): NEEDS_REFINEMENT Review note: `review-357-2026-03-25-v3` Both v1 and v2 expansion issues are fully addressed. Two new issues found during codebase verification of the consolidated spec. - **SDK `list_groups` pagination unaddressed** — `_resolve_group()` will call SDK's `list_groups(page=1, per_page=10)` which returns max 10 groups. If the user belongs to >10 groups, name resolution silently misses groups and returns false "no match" errors. Add one line to Constraints: helper must paginate or use `per_page=100`. - **Hook test location unspecified** — `claude-custom` has no `tests/` directory. Scope says "pipe mock JSON through hook" but doesn't say where test scripts live. Implementing agent needs a location decision.

forgejo_admin commented

2026-03-26 03:17:45 +00:00

Author

Contributor

Scope Expansion v3 (post-review `review-357-2026-03-25-v3`)

Two minor refinements — both accepted.

1. SDK pagination: `_resolve_group()` must paginate

groupme-sdk list_groups(page=1, per_page=10) returns max 10 groups per call. Currently at 10 groups (9 team + 1 stakeholders) — already at the edge.

Fix: Add to Constraints:

_resolve_group() must call list_groups(per_page=100) or implement pagination loop to avoid silently missing groups. Currently 10 groups, default page size is 10 — will break on the 11th group without this.

2. Hook test location specified

claude-custom has no tests/ directory.

Fix: Add to Constraints:

Hook tests live in tests/ directory (create it). Each hook test is a shell script that pipes mock JSON through the hook and asserts output. Pattern: tests/test_block_groupme_send.sh. Run with bash tests/test_block_groupme_send.sh.

## Scope Expansion v3 (post-review `review-357-2026-03-25-v3`) Two minor refinements — both accepted. ### 1. SDK pagination: `_resolve_group()` must paginate `groupme-sdk` `list_groups(page=1, per_page=10)` returns max 10 groups per call. Currently at 10 groups (9 team + 1 stakeholders) — already at the edge. **Fix:** Add to Constraints: - `_resolve_group()` must call `list_groups(per_page=100)` or implement pagination loop to avoid silently missing groups. Currently 10 groups, default page size is 10 — will break on the 11th group without this. ### 2. Hook test location specified `claude-custom` has no `tests/` directory. **Fix:** Add to Constraints: - Hook tests live in `tests/` directory (create it). Each hook test is a shell script that pipes mock JSON through the hook and asserts output. Pattern: `tests/test_block_groupme_send.sh`. Run with `bash tests/test_block_groupme_send.sh`.

forgejo_admin commented

2026-03-26 03:21:14 +00:00

Author

Contributor

Scope Review (re-review v4): READY

Review note: review-357-2026-03-25-v4
All three prior rounds of refinement are fully addressed. Consolidated spec is complete and agent-executable.

Verified:

All 7 file targets confirmed against codebase (line numbers, function signatures, directory existence)
SDK list_groups(page=1, per_page=10) pagination edge confirmed -- currently at 10 groups
block-mcp-merge.sh pattern (19 lines) is a clean template for the new hook
settings.json PreToolUse section uses pipe-delimited matchers -- new entry follows convention
19 existing tests in groupme-mcp use group_id="12345" and will need rewrite (no silent breakage)
No blast radius beyond groupme-mcp -- zero group_id references in claude-custom or other MCP servers
5 points appropriate for scope: 2 repos, 7 files, 5 tool interfaces, shared helper, 3-shape hook, new test dir

## Scope Review (re-review v4): READY Review note: `review-357-2026-03-25-v4` All three prior rounds of refinement are fully addressed. Consolidated spec is complete and agent-executable. **Verified:** - All 7 file targets confirmed against codebase (line numbers, function signatures, directory existence) - SDK `list_groups(page=1, per_page=10)` pagination edge confirmed -- currently at 10 groups - `block-mcp-merge.sh` pattern (19 lines) is a clean template for the new hook - `settings.json` PreToolUse section uses pipe-delimited matchers -- new entry follows convention - 19 existing tests in groupme-mcp use `group_id="12345"` and will need rewrite (no silent breakage) - No blast radius beyond groupme-mcp -- zero `group_id` references in claude-custom or other MCP servers - 5 points appropriate for scope: 2 repos, 7 files, 5 tool interfaces, shared helper, 3-shape hook, new test dir