pal-e-docs

Review: Convention enforcement

review-377-2026-03-27 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] ### Type — Feature
  • [x] ### Lineage
  • [x] ### Repo
  • [x] ### User Story
  • [x] ### Context
  • [x] ### File Targets
  • [x] ### Acceptance Criteria
  • [x] ### Test Expectations
  • [x] ### Constraints
  • [x] ### Checklist
  • [x] ### Related

Traceability

  • [x] story:pm-scope label — PM scoping story
  • [x] arch:enforcement label — enforcement architecture component
  • [x] Forgejo issue — forgejo_admin/claude-custom#166, open

File Targets

  • [x] settings.json — verified at ~/claude-custom/settings.json. Contains the PreToolUse matchers described.
  • [ ] GroupMe matcher status — ISSUE: The ticket states the GroupMe entries are "already split (fixed during incident)" but they are NOT split. Line 103 still reads "mcp__groupme__send_message|mcp__groupme__add_member|mcp__groupme__remove_member" as a single pipe-separated matcher. The agent implementing this will need to split them.
  • [x] mcp__forgejo__create_issue|mcp__forgejo__create_issue_and_branch (line 130) — verified present. Returns "deny", NOT "ask". Per the Constraints section, only "ask" matchers are affected, so this one is safe. Issue body correctly identifies it as an audit target.
  • [x] Write|Edit|NotebookEdit (line 81) — verified present. Returns "deny", NOT "ask". Safe per the same constraint.
  • [x] pal-e-docs convention note — to be CREATED as convention-hook-matcher-pattern. Deliverable is clear.

Repo Placement

Issue filed on forgejo_admin/claude-custom — correct for settings.json changes. The second deliverable (convention note in pal-e-docs) is created via MCP API, not a repo change, so single-repo placement is acceptable.

Dependencies

  • Board #357 (claude-custom#160, GroupMe incident) — done. This is the incident that exposed the bug. No blocking dependency.
  • Board #375 (claude-custom#164, auto-pull hook) — done. Related but not blocking.
  • Board #395 (claude-custom#167, story label upgrade) — done. Related enforcement work, not blocking.
  • No active blockers found.

Acceptance Criteria

  • [x] AC1: "All PreToolUse matchers that return permissionDecision: ask use separate entries" — Testable. Agent can grep for pipe + ask patterns.
  • [x] AC2: "Convention note convention-hook-matcher-pattern created in pal-e-docs" — Testable via API.
  • [x] AC3: "Convention documents the specific failure mode" — Testable by reading note content.
  • [x] AC4: "Existing pipe-separated PreToolUse matchers audited" — Testable. The full audit is: only the GroupMe matcher (line 103) uses pipe + ask. The two deny matchers (lines 81, 130) are safe.
Criteria are testable and complete.

Blast Radius

  • Agent config files (agents/penny.md, agents/dottie.md, agents/qa.md) use pipe-separated PreToolUse matchers, but all return "deny", not "ask". The Constraints section correctly limits scope to "ask" matchers only.
  • PostToolUse matchers with pipes (e.g., line 185 mcp__forgejo__create_issue_and_branch) are explicitly excluded by the Constraints section. No blast radius concern.
  • SubagentStart matcher (line 246) uses pipe — this is neither PreToolUse nor returns "ask". Safe.

Recommendation

One factual error must be corrected before this is READY:
  • Fix the GroupMe claim: The issue body says the GroupMe entries are "already split (fixed during incident)" — this is false. Line 103 of settings.json still has the pipe-separated matcher. Update the File Targets section to say the GroupMe matcher NEEDS splitting (not that it's already done). This is important because an implementing agent reading "already split" may skip the split and only audit the other two matchers.
Once that correction is made, verdict upgrades to READY. The scope is clean, all other targets are verified, and the work fits in a single agent pass.