ldraney/claude-custom

Fork 0

feat: auto-delete worktrees on PR merge #199

Merged

forgejo_admin merged 2 commits from 194-post-merge-worktree-cleanup into main

2026-03-28 05:40:30 +00:00

forgejo_admin commented

2026-03-28 05:29:03 +00:00

Contributor

Summary

Extends both post-merge hooks to automatically remove worktrees for merged branches. After the existing fast-forward logic, each hook extracts the merged branch name and walks git worktree list --porcelain to find and remove the matching worktree. Prevents stale worktree accumulation (700MB incident on 2026-03-13).

Changes

hooks/post-merge-rebase.sh — Added worktree cleanup after fast-forward. Extracts PR number from gh pr merge command, uses gh pr view to resolve head branch name, walks worktrees, removes match via git worktree remove --force + git branch -D.
hooks/post-mcp-merge-rebase.sh — Added worktree cleanup after fast-forward. Extracts owner/repo/pull_number from MCP tool_input, uses Forgejo API (token from $HOME/secrets/pal-e-services/forgejo.env) to resolve head branch, same worktree walk and cleanup.

Test Plan

bash -n syntax check passes on both files
gh path mock merge: fast-forward works, cleanup exits silently (no matching worktree)
MCP path mock merge: fast-forward works, cleanup exits silently (no matching worktree)
Failed merge (exitCode 1) exits silently — no regression
Non-merge commands exit silently — no regression
Non-merged MCP response exits silently — no regression
Manual: verify actual worktree removal after real PR merge

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #194
Spec: pal-e-platform/docs/superpowers/specs/2026-03-28-worktree-lifecycle-enforcement-design.md
project-pal-e-agency — agent infrastructure project

## Summary Extends both post-merge hooks to automatically remove worktrees for merged branches. After the existing fast-forward logic, each hook extracts the merged branch name and walks `git worktree list --porcelain` to find and remove the matching worktree. Prevents stale worktree accumulation (700MB incident on 2026-03-13). ## Changes - `hooks/post-merge-rebase.sh` — Added worktree cleanup after fast-forward. Extracts PR number from `gh pr merge` command, uses `gh pr view` to resolve head branch name, walks worktrees, removes match via `git worktree remove --force` + `git branch -D`. - `hooks/post-mcp-merge-rebase.sh` — Added worktree cleanup after fast-forward. Extracts owner/repo/pull_number from MCP tool_input, uses Forgejo API (token from `$HOME/secrets/pal-e-services/forgejo.env`) to resolve head branch, same worktree walk and cleanup. ## Test Plan - [x] `bash -n` syntax check passes on both files - [x] gh path mock merge: fast-forward works, cleanup exits silently (no matching worktree) - [x] MCP path mock merge: fast-forward works, cleanup exits silently (no matching worktree) - [x] Failed merge (exitCode 1) exits silently — no regression - [x] Non-merge commands exit silently — no regression - [x] Non-merged MCP response exits silently — no regression - [ ] Manual: verify actual worktree removal after real PR merge ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - Closes #194 - Spec: `pal-e-platform/docs/superpowers/specs/2026-03-28-worktree-lifecycle-enforcement-design.md` - `project-pal-e-agency` — agent infrastructure project

forgejo_admin added 1 commit

2026-03-28 05:29:03 +00:00

feat: auto-delete worktrees on PR merge in both post-merge hooks 50f0a2f5e7

After the existing fast-forward logic, extract the merged branch name
and remove any matching worktree. gh path uses `gh pr view` for the
branch name; MCP path uses the Forgejo API. Best-effort: silent exit
if no matching worktree exists.

Closes #194

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-28 05:30:07 +00:00

Author

Contributor

PR Review: #199 — auto-delete worktrees on PR merge

Acceptance Criteria Check

Criterion	Status
gh path: extracts PR number, uses `gh pr view` for head branch	PASS
MCP path: extracts owner/repo/pull_number, uses Forgejo API for head branch	PASS
Walks `git worktree list --porcelain`, finds worktree on merged branch	PASS
Removes matching worktree via `git worktree remove --force`	PASS
Deletes local branch ref via `git branch -D`	PASS
Logs `[post-merge-cleanup] Removed worktree for branch <name> at <path>`	PASS
Best-effort: silent exit if no matching worktree found	PASS
Existing fast-forward logic unchanged	PASS (0 deletions)

Code Quality

Both hooks follow identical cleanup patterns (porcelain walk, case statement, trailing-entry handler). Consistent with the spec's "~10 lines per hook, not enough duplication to justify shared library" decision.
MCP path correctly sources forgejo.env only when needed, and guards on both $FORGEJO_TOKEN and $FORGEJO_URL before making the API call.
gh path correctly cds to $CWD before calling gh pr view, ensuring the gh CLI resolves the correct repo context.
All cleanup operations are wrapped in null-guards ([ -n "$MERGED_BRANCH" ], [ -n "$worktree_path" ]), making failure silent.
No changes to settings.json or cleanup-worktrees.sh, matching the issue's "Files the agent should NOT touch" constraint.

Test Results (from PR body)

bash -n syntax validation: PASS on both files
Mock gh merge JSON: PASS (fast-forward works, cleanup silent)
Mock MCP merge JSON: PASS (fast-forward works, cleanup silent)
Failed merge / non-merge / non-merged edge cases: all PASS

Findings

No issues found. Implementation matches the spec exactly with no scope creep.

VERDICT: APPROVE

## PR Review: #199 — auto-delete worktrees on PR merge ### Acceptance Criteria Check | Criterion | Status | |-----------|--------| | gh path: extracts PR number, uses `gh pr view` for head branch | PASS | | MCP path: extracts owner/repo/pull_number, uses Forgejo API for head branch | PASS | | Walks `git worktree list --porcelain`, finds worktree on merged branch | PASS | | Removes matching worktree via `git worktree remove --force` | PASS | | Deletes local branch ref via `git branch -D` | PASS | | Logs `[post-merge-cleanup] Removed worktree for branch <name> at <path>` | PASS | | Best-effort: silent exit if no matching worktree found | PASS | | Existing fast-forward logic unchanged | PASS (0 deletions) | ### Code Quality - Both hooks follow identical cleanup patterns (porcelain walk, case statement, trailing-entry handler). Consistent with the spec's "~10 lines per hook, not enough duplication to justify shared library" decision. - MCP path correctly sources `forgejo.env` only when needed, and guards on both `$FORGEJO_TOKEN` and `$FORGEJO_URL` before making the API call. - gh path correctly `cd`s to `$CWD` before calling `gh pr view`, ensuring the gh CLI resolves the correct repo context. - All cleanup operations are wrapped in null-guards (`[ -n "$MERGED_BRANCH" ]`, `[ -n "$worktree_path" ]`), making failure silent. - No changes to settings.json or cleanup-worktrees.sh, matching the issue's "Files the agent should NOT touch" constraint. ### Test Results (from PR body) - `bash -n` syntax validation: PASS on both files - Mock gh merge JSON: PASS (fast-forward works, cleanup silent) - Mock MCP merge JSON: PASS (fast-forward works, cleanup silent) - Failed merge / non-merge / non-merged edge cases: all PASS ### Findings No issues found. Implementation matches the spec exactly with no scope creep. **VERDICT: APPROVE**

forgejo_admin commented

2026-03-28 05:32:43 +00:00

Author

Contributor

PR #199 Review

DOMAIN REVIEW

Tech stack: Bash shell hooks (Claude Code PostToolUse hooks), jq for JSON parsing, curl for Forgejo API, gh CLI for GitHub/Forgejo CLI.

What this PR does: After a successful PR merge (via either gh pr merge or mcp__forgejo__merge_approved_pr), the hooks now resolve the merged branch name and walk git worktree list --porcelain to find and --force remove the matching worktree, then delete the local branch. This addresses the 700MB stale worktree accumulation problem from 2026-03-13.

The approach is sound. Both hooks already run after confirmed successful merges, so the cleanup logic triggers at the right time. Error handling is fail-open (all git/curl commands redirect stderr to /dev/null), which is correct for a PostToolUse hook -- cleanup failures must never block agent workflow.

BLOCKERS

1. DRY violation: worktree walk-and-remove logic duplicated verbatim across two hooks

The worktree parsing block (the while IFS= read -r line loop, the case statement, the trailing-entry handler, and the git worktree remove --force + git branch -D cleanup) is identical in both post-merge-rebase.sh and post-mcp-merge-rebase.sh -- approximately 30 lines copy-pasted.

This codebase already has a pattern for shared logic: forgejo-helper.sh (sourced by board-item-on-merge.sh and others). The worktree walk-and-remove block should be extracted into a shared helper function (e.g., in a worktree-helper.sh or added to an existing shared file) and sourced by both hooks.

Note: cleanup-worktrees.sh also has a very similar porcelain-parsing loop (with age-based filtering). That is a pre-existing DRY issue outside this PR's scope, but it reinforces the need for a shared function now rather than tripling the duplication.

Why this is a blocker, not a nit: The worktree walk logic parses a specific porcelain format. If that format changes or a bug is found in the parser, fixing it in one hook but not the other creates silent divergence -- exactly the kind of divergence risk that DRY violations in operational paths produce. Two hooks doing the same --force removal from the same parsed data must share the same code.

2. Inconsistent auth pattern: token auth vs. existing basic auth convention

The new code in post-mcp-merge-rebase.sh sources forgejo.env directly and uses Authorization: token $FORGEJO_TOKEN:

source "$HOME/secrets/pal-e-services/forgejo.env" 2>/dev/null
curl -s -H "Authorization: token $FORGEJO_TOKEN" ...

Every other hook in this codebase (board-item-on-merge.sh, label-on-verdict.sh, etc.) uses the shared forgejo-helper.sh with _load_forgejo_creds and basic auth (-u "${FORGEJO_USER}:${FORGEJO_PASSWORD}"). This PR introduces a second, incompatible auth pattern.

This is a DRY/divergence blocker for the same reason: two authentication paths to the same API will diverge when credentials rotate. Use forgejo-helper.sh and the existing basic auth pattern, or refactor forgejo-helper.sh to support token auth for all consumers. Do not introduce a parallel path.

NITS

1. Uninitialized MERGED_BRANCH in post-merge-rebase.sh

In the MCP hook, MERGED_BRANCH="" is explicitly initialized. In the gh hook, it is not -- it relies on bash treating unset variables as empty in [ -n ]. This works but is inconsistent between the two hooks. Initialize it explicitly for clarity and consistency.

2. Edge case: gh pr merge --squash without explicit PR number

When gh pr merge is invoked without a PR number (gh infers it from the current branch), the regex grep -oE 'gh\s+pr\s+merge\s+([0-9]+)' will not match, so cleanup silently skips. This is safe (fail-open) but worth a comment in the code noting this limitation, since gh pr merge without an explicit number is a valid invocation pattern.

3. Missing --connect-timeout / --max-time on curl

The existing board-item-on-merge.sh uses curl -s --connect-timeout 5 --max-time 10. The new curl in post-mcp-merge-rebase.sh uses curl -s without timeouts. In a PostToolUse hook, a hanging curl blocks the agent. Add timeouts consistent with the existing pattern.

4. Log tag inconsistency

Both hooks emit [post-merge-cleanup] on successful removal. The fast-forward messages use [post-merge-rebase] and [post-mcp-merge-rebase] respectively. For log grep-ability, the cleanup messages should differentiate which hook they came from (e.g., [post-merge-cleanup:gh] and [post-merge-cleanup:mcp]), or at minimum match the hook's existing prefix convention.

SOP COMPLIANCE

Branch named after issue: 194-post-merge-worktree-cleanup references #194
PR body follows template: Summary, Changes, Test Plan, Related sections present
Related references parent issue: Closes #194
Related references spec: 2026-03-28-worktree-lifecycle-enforcement-design.md
No secrets committed: credentials sourced at runtime from ~/secrets/
No unnecessary file changes: only the two relevant hook files modified
Commit messages are descriptive

PROCESS OBSERVATIONS

Change failure risk: Low for the core logic (fail-open design, --force remove is idempotent). Medium for the auth divergence -- token auth may silently fail if FORGEJO_TOKEN is not set or expires, with no fallback.
Deployment frequency impact: Positive. Automatic worktree cleanup on merge reduces the manual cleanup-worktrees.sh dependency and prevents the 700MB disk bloat that previously required manual intervention.
Technical debt: This PR adds a third copy of porcelain-parsing logic to the hooks directory. Extracting it now prevents compounding debt.

VERDICT: NOT APPROVED

Two blockers must be addressed:

Extract the duplicated worktree walk-and-remove logic into a shared helper function.
Use the existing forgejo-helper.sh auth pattern instead of introducing a parallel token-based auth path.

## PR #199 Review ### DOMAIN REVIEW **Tech stack**: Bash shell hooks (Claude Code PostToolUse hooks), `jq` for JSON parsing, `curl` for Forgejo API, `gh` CLI for GitHub/Forgejo CLI. **What this PR does**: After a successful PR merge (via either `gh pr merge` or `mcp__forgejo__merge_approved_pr`), the hooks now resolve the merged branch name and walk `git worktree list --porcelain` to find and `--force` remove the matching worktree, then delete the local branch. This addresses the 700MB stale worktree accumulation problem from 2026-03-13. **The approach is sound.** Both hooks already run after confirmed successful merges, so the cleanup logic triggers at the right time. Error handling is fail-open (all git/curl commands redirect stderr to `/dev/null`), which is correct for a PostToolUse hook -- cleanup failures must never block agent workflow. ### BLOCKERS **1. DRY violation: worktree walk-and-remove logic duplicated verbatim across two hooks** The worktree parsing block (the `while IFS= read -r line` loop, the `case` statement, the trailing-entry handler, and the `git worktree remove --force` + `git branch -D` cleanup) is **identical** in both `post-merge-rebase.sh` and `post-mcp-merge-rebase.sh` -- approximately 30 lines copy-pasted. This codebase already has a pattern for shared logic: `forgejo-helper.sh` (sourced by `board-item-on-merge.sh` and others). The worktree walk-and-remove block should be extracted into a shared helper function (e.g., in a `worktree-helper.sh` or added to an existing shared file) and sourced by both hooks. Note: `cleanup-worktrees.sh` also has a very similar porcelain-parsing loop (with age-based filtering). That is a pre-existing DRY issue outside this PR's scope, but it reinforces the need for a shared function now rather than tripling the duplication. **Why this is a blocker, not a nit**: The worktree walk logic parses a specific porcelain format. If that format changes or a bug is found in the parser, fixing it in one hook but not the other creates silent divergence -- exactly the kind of divergence risk that DRY violations in operational paths produce. Two hooks doing the same `--force` removal from the same parsed data must share the same code. **2. Inconsistent auth pattern: token auth vs. existing basic auth convention** The new code in `post-mcp-merge-rebase.sh` sources `forgejo.env` directly and uses `Authorization: token $FORGEJO_TOKEN`: ```bash source "$HOME/secrets/pal-e-services/forgejo.env" 2>/dev/null curl -s -H "Authorization: token $FORGEJO_TOKEN" ... ``` Every other hook in this codebase (`board-item-on-merge.sh`, `label-on-verdict.sh`, etc.) uses the shared `forgejo-helper.sh` with `_load_forgejo_creds` and basic auth (`-u "${FORGEJO_USER}:${FORGEJO_PASSWORD}"`). This PR introduces a second, incompatible auth pattern. This is a DRY/divergence blocker for the same reason: two authentication paths to the same API will diverge when credentials rotate. Use `forgejo-helper.sh` and the existing basic auth pattern, or refactor `forgejo-helper.sh` to support token auth for all consumers. Do not introduce a parallel path. ### NITS **1. Uninitialized `MERGED_BRANCH` in `post-merge-rebase.sh`** In the MCP hook, `MERGED_BRANCH=""` is explicitly initialized. In the `gh` hook, it is not -- it relies on bash treating unset variables as empty in `[ -n ]`. This works but is inconsistent between the two hooks. Initialize it explicitly for clarity and consistency. **2. Edge case: `gh pr merge --squash` without explicit PR number** When `gh pr merge` is invoked without a PR number (gh infers it from the current branch), the regex `grep -oE 'gh\s+pr\s+merge\s+([0-9]+)'` will not match, so cleanup silently skips. This is safe (fail-open) but worth a comment in the code noting this limitation, since `gh pr merge` without an explicit number is a valid invocation pattern. **3. Missing `--connect-timeout` / `--max-time` on curl** The existing `board-item-on-merge.sh` uses `curl -s --connect-timeout 5 --max-time 10`. The new curl in `post-mcp-merge-rebase.sh` uses `curl -s` without timeouts. In a PostToolUse hook, a hanging curl blocks the agent. Add timeouts consistent with the existing pattern. **4. Log tag inconsistency** Both hooks emit `[post-merge-cleanup]` on successful removal. The fast-forward messages use `[post-merge-rebase]` and `[post-mcp-merge-rebase]` respectively. For log grep-ability, the cleanup messages should differentiate which hook they came from (e.g., `[post-merge-cleanup:gh]` and `[post-merge-cleanup:mcp]`), or at minimum match the hook's existing prefix convention. ### SOP COMPLIANCE - [x] Branch named after issue: `194-post-merge-worktree-cleanup` references #194 - [x] PR body follows template: Summary, Changes, Test Plan, Related sections present - [x] Related references parent issue: `Closes #194` - [x] Related references spec: `2026-03-28-worktree-lifecycle-enforcement-design.md` - [x] No secrets committed: credentials sourced at runtime from `~/secrets/` - [x] No unnecessary file changes: only the two relevant hook files modified - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS - **Change failure risk**: Low for the core logic (fail-open design, `--force` remove is idempotent). Medium for the auth divergence -- token auth may silently fail if `FORGEJO_TOKEN` is not set or expires, with no fallback. - **Deployment frequency impact**: Positive. Automatic worktree cleanup on merge reduces the manual `cleanup-worktrees.sh` dependency and prevents the 700MB disk bloat that previously required manual intervention. - **Technical debt**: This PR adds a third copy of porcelain-parsing logic to the hooks directory. Extracting it now prevents compounding debt. ### VERDICT: NOT APPROVED Two blockers must be addressed: 1. Extract the duplicated worktree walk-and-remove logic into a shared helper function. 2. Use the existing `forgejo-helper.sh` auth pattern instead of introducing a parallel token-based auth path.

forgejo_admin referenced this pull request from a commit

2026-03-28 05:37:07 +00:00

fix: extract shared worktree cleanup + use consistent Forgejo auth

forgejo_admin added 1 commit

2026-03-28 05:37:07 +00:00

fix: extract shared worktree cleanup + use consistent Forgejo auth 3a435b33e4

Address 2 QA blockers and 4 nits from PR #199 review:

Blockers:
- Extract ~30-line worktree walk-and-remove logic into
  remove_worktree_for_branch() in forgejo-helper.sh, sourced by both hooks
- Replace token auth (Authorization: token) with existing _load_forgejo_creds
  basic auth pattern in post-mcp-merge-rebase.sh

Nits:
- Initialize MERGED_BRANCH="" in post-merge-rebase.sh
- Add comment about gh pr merge without explicit PR number silently skipping
- Add --connect-timeout 5 --max-time 10 to the Forgejo API curl call
- Differentiate log tags: [post-merge-cleanup:gh] and [post-merge-cleanup:mcp]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-28 05:39:58 +00:00

Author

Contributor

PR #199 Review (Re-review)

DOMAIN REVIEW

Tech stack: Bash shell hooks (PostToolUse hooks for Claude Code), Forgejo API integration, git worktree management.

Shared function extraction (forgejo-helper.sh lines 276-319): remove_worktree_for_branch() is well-structured. It accepts three parameters (main_worktree, branch, log_tag), validates inputs with an early return, parses git worktree list --porcelain correctly, and handles the edge case where porcelain output does not end with a blank line (lines 309-312). The --force flag on worktree remove is appropriate since merged branches have no uncommitted work. The branch -D cleanup after worktree removal prevents dangling refs.

Auth pattern (post-mcp-merge-rebase.sh lines 74-79): Now uses _load_forgejo_creds + -u "${FORGEJO_USER}:${FORGEJO_PASSWORD}" basic auth, matching the established pattern in forgejo_set_label, forgejo_comment_on_issue, and all other helper functions. Curl timeouts (--connect-timeout 5 --max-time 10) are consistent with existing conventions.

GH CLI path (post-merge-rebase.sh lines 71-78): The gh pr view call to resolve the head branch name is appropriate. The NOTE comment (lines 72-73) documenting the limitation when gh pr merge is invoked without an explicit PR number is good defensive documentation.

Error handling: Both hooks initialize MERGED_BRANCH="" before conditionals, so remove_worktree_for_branch receives an empty string on failure paths and silently returns via the guard clause at line 285. All git and curl commands redirect stderr to /dev/null where appropriate. The exit 0 at the end of both hooks ensures the hook never blocks the main merge flow.

BLOCKERS

None. Both previously identified blockers are resolved:

DRY violation (RESOLVED): Worktree walk logic extracted into remove_worktree_for_branch() in forgejo-helper.sh. Both hooks call the shared function with differentiated log tags.
Inconsistent auth pattern (RESOLVED): MCP hook now uses _load_forgejo_creds + basic auth (-u flag). Token-based auth is fully removed -- confirmed via grep across the entire hooks directory.

NITS

All four previous nits are resolved:

Curl timeouts: present (--connect-timeout 5 --max-time 10)
MERGED_BRANCH initialization: both hooks set MERGED_BRANCH="" before conditionals
Differentiated log tags: post-merge-cleanup:gh vs post-merge-cleanup:mcp

One new minor nit:

Missing shellcheck source directive: Both hooks source "${HOOK_DIR}/forgejo-helper.sh" without the # shellcheck source=forgejo-helper.sh comment that board-item-on-merge.sh uses (line 30 of that file). Non-blocking, but would silence SC1091 if shellcheck is ever run across hooks.

SOP COMPLIANCE

Branch named after issue: 194-post-merge-worktree-cleanup references #194
PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present
Related references spec: links to docs/superpowers/specs/2026-03-28-worktree-lifecycle-enforcement-design.md (verified exists) and project-pal-e-agency
Closes directive: Closes #194
No secrets committed: confirmed no hardcoded credentials (only FORGEJO_PASSWORD default empty string in credential loader)
No unnecessary file changes: 3 files changed, all directly related to the feature
Commit messages are descriptive (per PR description)

PROCESS OBSERVATIONS

Clean fix commit addressing all review feedback. The extraction of remove_worktree_for_branch() into forgejo-helper.sh is the right architectural choice -- it sits alongside the other shared Forgejo functions and can be reused by future cleanup hooks. The auth pattern is now fully consistent across the entire helper library.

The manual test plan item (actual worktree removal after real PR merge) is still unchecked. This is expected -- it can only be validated post-merge with a real worktree present. Recommend Lucas validate this on the next PR merge after this lands.

VERDICT: APPROVED

## PR #199 Review (Re-review) ### DOMAIN REVIEW **Tech stack**: Bash shell hooks (PostToolUse hooks for Claude Code), Forgejo API integration, git worktree management. **Shared function extraction** (`forgejo-helper.sh` lines 276-319): `remove_worktree_for_branch()` is well-structured. It accepts three parameters (main_worktree, branch, log_tag), validates inputs with an early return, parses `git worktree list --porcelain` correctly, and handles the edge case where porcelain output does not end with a blank line (lines 309-312). The `--force` flag on `worktree remove` is appropriate since merged branches have no uncommitted work. The `branch -D` cleanup after worktree removal prevents dangling refs. **Auth pattern** (`post-mcp-merge-rebase.sh` lines 74-79): Now uses `_load_forgejo_creds` + `-u "${FORGEJO_USER}:${FORGEJO_PASSWORD}"` basic auth, matching the established pattern in `forgejo_set_label`, `forgejo_comment_on_issue`, and all other helper functions. Curl timeouts (`--connect-timeout 5 --max-time 10`) are consistent with existing conventions. **GH CLI path** (`post-merge-rebase.sh` lines 71-78): The `gh pr view` call to resolve the head branch name is appropriate. The NOTE comment (lines 72-73) documenting the limitation when `gh pr merge` is invoked without an explicit PR number is good defensive documentation. **Error handling**: Both hooks initialize `MERGED_BRANCH=""` before conditionals, so `remove_worktree_for_branch` receives an empty string on failure paths and silently returns via the guard clause at line 285. All git and curl commands redirect stderr to `/dev/null` where appropriate. The `exit 0` at the end of both hooks ensures the hook never blocks the main merge flow. ### BLOCKERS None. Both previously identified blockers are resolved: 1. **DRY violation (RESOLVED)**: Worktree walk logic extracted into `remove_worktree_for_branch()` in `forgejo-helper.sh`. Both hooks call the shared function with differentiated log tags. 2. **Inconsistent auth pattern (RESOLVED)**: MCP hook now uses `_load_forgejo_creds` + basic auth (`-u` flag). Token-based auth is fully removed -- confirmed via grep across the entire hooks directory. ### NITS All four previous nits are resolved: - Curl timeouts: present (`--connect-timeout 5 --max-time 10`) - MERGED_BRANCH initialization: both hooks set `MERGED_BRANCH=""` before conditionals - Differentiated log tags: `post-merge-cleanup:gh` vs `post-merge-cleanup:mcp` One new minor nit: 1. **Missing shellcheck source directive**: Both hooks `source "${HOOK_DIR}/forgejo-helper.sh"` without the `# shellcheck source=forgejo-helper.sh` comment that `board-item-on-merge.sh` uses (line 30 of that file). Non-blocking, but would silence SC1091 if shellcheck is ever run across hooks. ### SOP COMPLIANCE - [x] Branch named after issue: `194-post-merge-worktree-cleanup` references #194 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present - [x] Related references spec: links to `docs/superpowers/specs/2026-03-28-worktree-lifecycle-enforcement-design.md` (verified exists) and `project-pal-e-agency` - [x] Closes directive: `Closes #194` - [x] No secrets committed: confirmed no hardcoded credentials (only `FORGEJO_PASSWORD` default empty string in credential loader) - [x] No unnecessary file changes: 3 files changed, all directly related to the feature - [x] Commit messages are descriptive (per PR description) ### PROCESS OBSERVATIONS Clean fix commit addressing all review feedback. The extraction of `remove_worktree_for_branch()` into `forgejo-helper.sh` is the right architectural choice -- it sits alongside the other shared Forgejo functions and can be reused by future cleanup hooks. The auth pattern is now fully consistent across the entire helper library. The manual test plan item (actual worktree removal after real PR merge) is still unchecked. This is expected -- it can only be validated post-merge with a real worktree present. Recommend Lucas validate this on the next PR merge after this lands. ### VERDICT: APPROVED

forgejo_admin merged commit c0c534fb27 into main

2026-03-28 05:40:30 +00:00

forgejo_admin deleted branch 194-post-merge-worktree-cleanup

2026-03-28 05:40:30 +00:00

forgejo_admin referenced this pull request from a commit

2026-03-28 05:40:32 +00:00

feat: auto-delete worktrees on PR merge (#199)

forgejo_admin referenced this pull request

2026-03-28 12:08:55 +00:00

Validate session 2026-03-28 merges (9 PRs) #208