pal-e-docs

Review: Cross-repo worktree isolation for parallel agents

review-418-2026-03-25 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- present ("Feature")
  • [x] Lineage -- present (standalone, discovered-scope)
  • [x] Repo -- present (pal-e-platform, claude-custom)
  • [x] User Story -- present and well-formed
  • [x] Context -- present, thorough, includes incident details
  • [x] File Targets -- present with both "modify" and "should NOT touch" lists
  • [x] Acceptance Criteria -- present (6 items)
  • [x] Test Expectations -- present (4 items)
  • [x] Constraints -- present (5 items)
  • [x] Checklist -- present
  • [x] Related -- present (4 items)
All required sections for the Feature template are present. Template is complete.

Traceability

  • [ ] story:X label -- MISSING on board item #418. Labels are type:bug,arch:ci-pipeline,discovered-scope,priority:high. No story label. This is enforcement-layer infrastructure -- story:dev-execute would be appropriate (matches the sibling worktree item #241).
  • [x] arch:X label -- present: arch:ci-pipeline. Reasonable for the hook/enforcement work, though arch:agent-spawn might be more precise since this is about agent isolation, not CI pipelines specifically.
  • [x] Forgejo issue -- valid, open: forgejo_admin/pal-e-platform#188

Type Mismatch

ISSUE: The Forgejo issue ### Type says Feature, but the board item label says type:bug. These must agree. Given the incident context (three agents clobbered each other), this IS a bug fix for a missing capability. However, the work is mostly additive (new SOP section, new hook, new agent instructions) which leans Feature. The router should decide, but the mismatch must be resolved before this moves to next_up.

File Targets

  • [x] hooks/cross-repo-isolation.sh (new) -- confirmed: file does NOT exist yet. Path is valid within claude-custom/hooks/.
  • [x] agents/dev.md -- verified: exists at ~/claude-custom/agents/dev.md. Currently has no cross-repo isolation instructions. The isolation: worktree frontmatter is present (line 7) but only covers spawning repo.
  • [x] worktree-workflow SOP (pal-e-docs) -- verified: exists, is active. Has no "Cross-Repo Isolation" section. TOC confirms gap: sections cover how-it-works, freshness, cleanup, location, remotes, rules -- all scoped to spawning repo only.
  • [x] agent-spawn-conventions (pal-e-docs) -- verified: exists, is active. Pre-Spawn Checklist has 4 items, none mention cross-repo isolation.
  • [x] terraform/ (should NOT touch) -- confirmed: exists, no changes needed.
  • [x] salt/ (should NOT touch) -- confirmed: exists, no changes needed.
All file targets verified. The existing CLAUDE.md in claude-custom already has a Worktree Isolation section (lines 9-17) documenting the /tmp/ clone pattern for claude-custom specifically -- the issue correctly identifies this as precedent to generalize.

Repo Placement

ISSUE: The Forgejo issue is filed on pal-e-platform but the actual code changes live in claude-custom (hooks, agent profiles) and pal-e-docs (SOP updates via Dottie). The issue body acknowledges this: "This issue is filed here because the SOP gap was discovered during platform operations." However, the Dev agent implementing the hook and agent profile changes will need to work on the claude-custom repo. This means the Forgejo issue should ideally be filed on claude-custom to match the repo where the PR will land. Or, at minimum, the issue should clearly state which repo gets the PR. Currently it is ambiguous -- the Checklist says "likely multiple PRs across repos" but does not specify which issue tracks which PR.
Recommendation: Either move the issue to claude-custom (primary code repo), or create a child issue on claude-custom for the hook/agent work while keeping this as the umbrella. The SOP updates via Dottie/Betty Sue can reference this issue directly.

Dependencies

  • Board item #241 (#136: Worktree flow -- auto-rebase branches when main advances) in todo -- related but not blocking. Both address worktree isolation gaps but are independently deliverable.
  • Board item #347 (arch:agent-spawn bug, backlog) -- potentially related. Same domain (agent spawn mechanics).
  • No explicit depends: or blocks: labels on #418. None documented in scope. This appears correct -- this ticket is independent.

Acceptance Criteria

Six criteria. Assessment:
  • AC 1-2 (isolation behavior) -- behavioral/convention criteria, not directly testable by automated means. They depend on agents following the SOP. The hook (AC 5) provides partial enforcement. Acceptable given the nature of the work.
  • AC 3-4 (SOP updates) -- verifiable by reading the updated notes. Clear and testable.
  • AC 5 (PreToolUse hook) -- verifiable by running the hook with mock inputs. Clear and testable.
  • AC 6 (Dev agent profile) -- verifiable by reading agents/dev.md. Clear and testable.
Test Expectations are reasonable. The "manual test" items are appropriate for this kind of infrastructure work. The bash hooks/cross-repo-isolation.sh command with mock inputs is testable.
Minor gap: No acceptance criterion verifies that the existing CLAUDE.md Worktree Isolation section is updated to reference the generalized pattern (currently it only covers claude-custom). The issue's Constraints section references it as precedent but does not require updating it.

Blast Radius

  • All 5 agent profiles (betty-sue.md, dev.md, dottie.md, penny.md, qa.md) could potentially do cross-repo work. The issue only targets dev.md. QA agents also use isolation: worktree and could face the same problem. Consider whether the hook should protect all agent types or just Dev.
  • The check-issue.sh hook already has cross-repo detection logic (lines 49-58) that resolves git context from file paths. The new hook should be aware of this existing pattern to avoid conflicts.
  • The block-claude-custom-main-edit.sh hook (line 38) already references cd ~/claude-custom && git checkout in its error message. If the new hook detects cd ~/repo && git checkout broadly, it needs to not conflict with existing hook guidance.
  • The /tmp/ clone pattern creates a cleanup concern -- who cleans up /tmp/repo-branch after the agent finishes? The existing cleanup-worktrees.sh only handles .claude/worktrees/. This is not addressed in the scope.

Recommendation

Three issues must be resolved before READY:
  • Type mismatch: Board item says type:bug, issue says ### Type: Feature. Pick one and align both.
  • Missing story label: Add story:dev-execute (or appropriate story) to board item #418.
  • Repo placement ambiguity: Clarify which repo gets the PR. The hook and agent profile changes are in claude-custom. Either move the issue there, or explicitly document in the issue body: "PR will be submitted to claude-custom. SOP updates are a separate Dottie task referencing this issue."
Two additional items to consider (non-blocking but recommended):
  • Add an acceptance criterion for /tmp/ cleanup (who removes /tmp/repo-branch after agent work completes?).
  • Consider whether QA agents also need cross-repo isolation in their profile, or document why Dev-only is sufficient.