Review: Post-merge worktree cleanup

review-508-2026-03-27 Review

review ready

Verdict: READY

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — links to spec and plan in pal-e-platform/docs/superpowers/
  • [x] Repo — forgejo_admin/claude-custom
  • [x] User Story — well-formed (session operator / auto-delete / no stale worktrees)
  • [x] Context — includes 700MB incident motivation and rationale for trigger point
  • [x] File Targets — 2 modify, 2 do-not-touch with reasons
  • [x] Acceptance Criteria — 8 criteria, all specific and verifiable
  • [x] Test Expectations — includes concrete run command with mock JSON
  • [x] Constraints — 4 constraints including token sourcing, insertion point, porcelain edge case, pattern adherence
  • [x] Checklist — standard 3-item
  • [x] Related — project and SOP references

Traceability

  • [x] story:pm-scope label — project management scope enforcement story
  • [x] arch:worktree label — worktree architecture component
  • [x] Forgejo issue — forgejo_admin/claude-custom#194, open

File Targets

  • [x] hooks/post-merge-rebase.sh — verified: exists, 66 lines, PostToolUse hook matching Bash. Currently does fast-forward only. Clean insertion point before final exit 0 on line 66.
  • [x] hooks/post-mcp-merge-rebase.sh — verified: exists, 62 lines, PostToolUse hook matching mcp__forgejo__merge_approved_pr. Same pattern as gh path. Clean insertion point before final exit 0 on line 62.
  • [x] hooks/cleanup-worktrees.sh — verified: correctly listed as do-not-touch. Separate safety net, handled by issue #195.
  • [x] settings.json — verified: correctly listed as do-not-touch. Both hooks already registered (lines 180, 220). No new registrations needed.

Repo Placement

Correct. Issue filed on forgejo_admin/claude-custom, both target files are in claude-custom/hooks/. Single-repo scope.

Dependencies

  • Board item #509 (issue #195, "Fix cleanup-worktrees.sh repo list") — sibling ticket in todo column. No blocking dependency; scopes are cleanly separated (#194 adds new cleanup logic to post-merge hooks, #195 fixes the existing SessionStart cleanup hook).
  • Board item #485 (issue #184, "Worktree isolation enforcement gaps") — broader umbrella in backlog. Touches cleanup-worktrees.sh which #194 explicitly excludes. No conflict.
  • Board item #507 (issue #193, "Pre-spawn freshness hook") — sibling worktree ticket in todo. Independent scope (pre-spawn vs post-merge). No dependency.
  • Board item #241 (issue #136, "Worktree auto-rebase") — complementary but independent. In todo column.
No undocumented dependencies found. All worktree-related siblings are independent.

Acceptance Criteria

8 criteria. All are verifiable by an agent:
  • AC 1-2 (extract PR info): testable by echoing parsed values from mock input
  • AC 3 (walk worktree list): testable with git worktree list --porcelain output parsing
  • AC 4-5 (remove worktree + delete branch): testable in a real or mock scenario
  • AC 6 (log message): testable by checking stderr output
  • AC 7 (best-effort): testable by running with no matching worktree
  • AC 8 (existing logic unchanged): testable by running the provided mock command
Test command is concrete and runnable. No missing criteria identified.

Blast Radius

Low. Only cleanup-worktrees.sh performs similar worktree operations (remove/prune), and it is explicitly excluded from this ticket's scope. No downstream consumers of these hooks — they are leaf hooks in the PostToolUse chain. The new worktree cleanup logic runs AFTER existing fast-forward, so failure in cleanup cannot affect the fast-forward behavior.

Decomposition

2 file targets in 1 repo. 8 AC, but they are tightly coupled — the same logic pattern applied to two parallel hooks (gh path and MCP path). Estimated agent time: 3-4 minutes. No decomposition needed.

Recommendation

No action needed.