pal-e-docs

Review: Worktree flow — auto-rebase branches when main advances

review-241-2026-03-27-v2 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [ ] All sections — ISSUE: The entire issue body is the literal string $NEW_BODY. A previous refinement attempt (comment #9200, 2026-03-27T22:14:05Z) used a shell variable that was never expanded, destroying the ticket content. The issue has zero usable sections — no Type, no User Story, no File Targets, no Acceptance Criteria. The ticket is unexecutable in its current state.

Traceability

  • [x] story:dev-execute label — present on board item #241
  • [x] arch:hooks label — present on board item #241 (added after first review)
  • [x] Forgejo issue — claude-custom#136, open
  • [x] scope:discovered, scope:worktree labels — present

File Targets

Cannot assess — issue body is $NEW_BODY. The previous review (review-241-2026-03-27) identified these issues with the original targets:
  • [ ] plugins/worktree-rebase/ — INVALID. The plugins/ directory contains only Claude Code plugin manager config files (blocklist.json, config.json, installed_plugins.json). All custom automation lives in hooks/.
  • [ ] hooks/post-merge-rebase-check.sh — naming conflict with existing hooks/post-merge-rebase.sh. Relationship was undocumented.

Repo Placement

OK — filed on forgejo_admin/claude-custom, work targets hooks in that repo.

Dependencies

The landscape has changed significantly since the first review. Three related tickets have been completed:
  • #193 (pre-spawn freshness hook) — marked done but NOT delivered. Board item #507 is in done column, but hooks/pre-spawn-freshness.sh does not exist in the repo, and no reference to it exists in settings.json. This is either a false completion or the PR was not merged.
  • #194 (post-merge worktree cleanup) — done and delivered. Integrated into both post-merge-rebase.sh (lines 66-83) and post-mcp-merge-rebase.sh (lines 62-85). Worktrees for merged branches are now auto-removed.
  • #184 (worktree isolation enforcement gaps) — marked done. Covered freshness check in check-agent-spawn.sh, cleanup coverage, and SOP alignment.
  • #418 (cross-repo worktree isolation, pal-e-platform#188) — still in todo. Complementary but not blocking.
Existing infrastructure covers much of #136's original intent:
  • post-merge-rebase.sh — PostToolUse on gh pr merge: fetches and fast-forwards local main after the agent's own merges.
  • post-mcp-merge-rebase.sh — PostToolUse on mcp__forgejo__merge_approved_pr: same for MCP merges.
  • check-claude-custom-clean.sh — SessionStart: auto-pulls claude-custom main on session start, warns on divergence.
  • cleanup-worktrees.sh — SessionStart: removes stale worktrees older than 7 days.
The remaining uncovered gap is narrow: detecting when main advances due to external merges (other agents, CI) during an active session, after SessionStart has already run. This is specifically a mid-session freshness problem.

Acceptance Criteria

Cannot assess current AC — body is destroyed. Previous review found all 3 original AC ambiguous (trigger event unspecified, "or" between auto-rebase and notification).

Blast Radius

  • Auto-rebase risk remains the core design question. Rebasing an in-progress branch mid-session can corrupt the working tree or introduce silent merge conflicts. The pre-spawn freshness approach (#193) was safer — it ensures freshness at branch creation time, not mid-work.
  • Double-rebase risk: Adding another rebase mechanism alongside post-merge-rebase.sh and post-mcp-merge-rebase.sh creates risk of conflicting fast-forward logic.
  • Scope overlap with completed work: If #193 (pre-spawn freshness) is truly delivered (just not visible on main yet), then #136's remaining scope is very narrow — only mid-session detection of external merges.

Decomposition

Cannot assess until the body is restored and scope is re-evaluated against completed work. If the remaining gap (mid-session external merge detection) is confirmed as the sole scope, this is likely a single-file hook — no decomposition needed. If the scope is broader, reassessment required.

Recommendation

  • [BODY] Restore the issue body. The current body is the literal string $NEW_BODY — a shell variable that was never expanded during the refinement update. The entire ticket spec is gone. Restore from the original content (referenced in review-241-2026-03-27) and apply the refinements from that first review.
  • [BODY] Re-scope against completed work. Issues #193, #194, and #184 have all been completed since the original ticket was written. The Context and File Targets sections must acknowledge what now exists and define only the remaining gap.
  • [SCOPE] Verify #193 completion status. Board item #507 (pre-spawn freshness hook) is marked done, but hooks/pre-spawn-freshness.sh does not exist in the repo. If the PR was not actually merged, #136's scope may need to absorb that work — or #193 needs to be reopened.
  • [SCOPE] Resolve the auto-rebase vs. notification design question. The previous review flagged this and the AC still used ambiguous "or" language. Pick one approach and document the rationale.
  • [LABEL] Labels are now correct — no label changes needed.