pal-e-docs

Review: Remove gh CLI references, fix post-merge hook for Forgejo MCP merges

review-1529-2026-06-20b Review

review ready
Project

Verdict: APPROVED

Re-review of board item #1529 after scope refinement. Previous review review-1529-2026-06-20 returned NEEDS_REFINEMENT because the issue described building hooks that already exist. The rewritten issue correctly scopes this as a deletion/cleanup task.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, discovered during PR #265
  • [x] Repo — ldraney/claude-custom
  • [x] User Story — present (As the Overseer...)
  • [x] Context — thorough explanation of migration to Forgejo, MCP replacements
  • [x] File Targets — detailed with DELETE/MODIFY/NOT-TOUCH categories
  • [x] Feature Flag — None (correct for cleanup task)
  • [x] Acceptance Criteria — 8 criteria
  • [x] Test Expectations — 4 items with concrete commands
  • [x] Constraints — clear guardrails (no new hooks, don't touch MCP hooks)
  • [x] Checklist — present
  • [x] Related — present

Traceability

  • [x] story:operational-reference label — verified on board item
  • [x] story note verified — found in project-claude-custom user-stories section ("As the Overseer or any agent working in this repo, I can find which files to modify...")
  • [x] arch:hooks label — present on board item
  • [ ] arch note MISSING — no arch-hooks note found in pal-e-docs. However, this is a deletion/cleanup task that removes dead code. The arch note gap is pre-existing and not introduced by this ticket. Acceptable to proceed without it.
  • [x] Forgejo issue — ldraney/claude-custom#268, open

File Targets

Files to DELETE (all verified to exist):
  • [x] hooks/post-merge-rebase.sh — exists, confirmed dead (replaced by post-mcp-merge-rebase.sh)
  • [x] hooks/block-pr-merge.sh — exists, confirmed dead (replaced by block-mcp-merge.sh)
  • [x] hooks/remind-review-loop.sh — exists, confirmed dead (replaced by remind-mcp-review-loop.sh)
  • [x] hooks/block-upstream.sh — exists, blocks gh commands never used post-migration
Files to MODIFY (all verified):
  • [x] hooks/check-issue.sh — confirmed: lines 105-111 (github) case) and lines 118-124 (*) fallback) contain gh issue view calls. Issue says "~lines 82-123" which is approximate but scope is correct.
  • [x] settings.json — confirmed: 4 hook wiring entries at lines 56, 68, 223, 227. Also has Bash(gh api:*) permission at line 4.
  • [x] settings.local.json — confirmed: 15 Bash(gh ...:*) permission entries
Files to PRESERVE (verified):
  • [x] hooks/post-mcp-merge-rebase.sh — exists, active
  • [x] hooks/block-mcp-merge.sh — exists, active
  • [x] hooks/remind-mcp-review-loop.sh — exists, active
  • [x] WebFetch(domain:github.com) — confirmed at settings.json line 6
  • [x] WebFetch(domain:raw.githubusercontent.com) — confirmed at settings.json line 7

Repo Placement

OK — issue filed on ldraney/claude-custom, all file targets are in the same repo.

Dependencies

No blocking dependencies. Board item #1526 (docs/ directory, PR #265) is done. No other in-progress items conflict.

Acceptance Criteria

All 8 AC are concrete and agent-verifiable:
  • AC 1-2: file deletion + settings cleanup — verifiable with ls and grep
  • AC 3: check-issue.sh cleanup — verifiable with grep
  • AC 4-5: permission cleanup — verifiable with grep
  • AC 6: preservation — verifiable with grep
  • AC 7: MCP hooks untouched — verifiable with git diff
  • AC 8: syntax check — verifiable with bash -n
Test command is concrete and correct: bash -n hooks/*.sh && grep -l "post-merge-rebase\|block-pr-merge\|remind-review-loop\|block-upstream" settings.json settings.local.json

Blast Radius

Docs references (not in scope but noted): The 4 dead hooks are referenced in 3 docs files:
  • docs/hooks.md — 4 references (lines 31, 34, 85, 86)
  • docs/filetree.md — 4 references (lines 41, 42, 68, 72)
  • docs/operations.md — 1 reference (line 38)
The issue does NOT include these docs files in its File Targets. This is acceptable because the /update-docs skill runs post-merge and will catch stale doc references. However, an efficient agent could clean these in the same PR.
MCP hook comments: post-mcp-merge-rebase.sh (line 5, 67) and remind-mcp-review-loop.sh (line 5) reference the old hooks in comments. These become stale after deletion but are harmless. Issue correctly says "Do not modify any *-mcp-* hooks."
No functional cross-references: No other hook sources or imports the 4 dead hooks. All references are in settings wiring (being removed), docs (post-merge cleanup), and comments (harmless).

Decomposition Assessment

8 AC across 7 file targets in 1 repo. AC count exceeds the >5 threshold. However:
  • This is a pure deletion/cleanup task — no new code to write
  • All operations are mechanical: rm, grep-and-delete lines, bash -n verify
  • Single repo, no cross-repo coordination
  • Estimated agent time: 3-4 minutes
No decomposition needed. The 5-minute rule exists to prevent complex multi-system changes from being bundled. This ticket's high AC count reflects thoroughness of verification, not complexity of implementation.

Recommendation

No action needed. Scope is clean and well-defined for a single agent pass.
Note: The missing arch-hooks note is a pre-existing gap, not introduced by this ticket. Creating it is out of scope for this cleanup task but could be tracked separately.