diff --git a/conductor/tracks/superpowers_review_20260619/report.md b/conductor/tracks/superpowers_review_20260619/report.md index e491d7cd..2e0955d2 100644 --- a/conductor/tracks/superpowers_review_20260619/report.md +++ b/conductor/tracks/superpowers_review_20260619/report.md @@ -723,7 +723,74 @@ The project has the discipline's core elements (per-task atomic commits, git not ## 12. Finishing a Development Branch - +## 12. Finishing a Development Branch + +### 12.1 What the skill prescribes + +The `finishing-a-development-branch` skill is the post-implementation decision flow. Core principle: "Verify tests → Detect environment → Present options → Execute choice → Clean up." The 6-step Process: + +1. **Verify Tests** — Before presenting options, verify tests pass. If tests fail, stop. Don't proceed to Step 2. +2. **Detect Environment** — Determine workspace state before presenting options. `GIT_DIR == GIT_COMMON` → normal repo; `GIT_DIR != GIT_COMMON` + named branch → worktree; `GIT_DIR != GIT_COMMON` + detached HEAD → externally managed. +3. **Determine Base Branch** — Try `git merge-base HEAD main` or `git merge-base HEAD master`. +4. **Present Options** — Normal repo and named-branch worktree: present exactly 4 options (Merge locally, Push and create PR, Keep as-is, Discard). Detached HEAD: present exactly 3 options (no merge option). +5. **Execute Choice** — Per the user's choice: Option 1 = merge to base + delete branch + cleanup worktree; Option 2 = push + create PR (no worktree cleanup); Option 3 = keep as-is (no worktree cleanup); Option 4 = require typed "discard" confirmation + force-delete + cleanup worktree. +6. **Cleanup Workspace** — Only for Options 1 and 4. Use `git worktree remove` + `git worktree prune`. Provenance check: only clean up worktrees under `.worktrees/`, `worktrees/`, or `~/.config/superpowers/worktrees/`. Harness-owned worktrees are NOT cleaned up. + +Common Mistakes: Skipping test verification; open-ended questions ("What should I do next?"); cleaning up worktree for Option 2; deleting branch before removing worktree; running `git worktree remove` from inside the worktree; cleaning up harness-owned worktrees; no confirmation for discard. + +Red Flags: Never proceed with failing tests; never merge without verifying tests on result; never delete work without confirmation; never force-push without explicit request; never remove a worktree before confirming merge success; never clean up worktrees you didn't create (provenance check); never run `git worktree remove` from inside the worktree. + +### 12.2 Mapping to the project's existing pattern + +The project works on master directly (per Section 8). The skill's branch management doesn't apply. The closest equivalents are the Phase Completion Verification and the project's track archival flow. + +| Skill rule | Project equivalent | Where | +|---|---|---| +| "Verify Tests before presenting options" | Phase Completion Verification §3 "Execute Automated Tests in Batches" | `conductor/workflow.md` | +| "Detect Environment: GIT_DIR vs GIT_COMMON" | Project works on master directly (per Section 8's ARCH-DIFF); no worktree detection needed | `conductor/workflow.md` | +| "Determine Base Branch" | Master is the only base branch in the project; no merge-base question | `conductor/workflow.md` | +| "Present Options: Merge locally / Push and Create PR / Keep as-is / Discard" | The project's Phase 10 finalize is simpler: register in tracks.md + update metadata.json. No merge/PR/keep/discard options. | `conductor/tracks//plan.md` Phase 10 | +| "Option 1: Merge Locally + cleanup worktree" | Project does not merge; commits go directly to master. Archival is via `git mv` to `conductor/archive/`. | `conductor/tracks.md` §"Notes" — Archiving a track (3 steps) | +| "Option 2: Push and Create PR" | Project does not use GitHub PR workflow; commits go to master via per-task atomic commits | Project-wide | +| "Option 3: Keep as-is" | Implicit — every track stays in `conductor/tracks/` until the user archives it | Project-wide | +| "Option 4: Discard (with typed confirmation)" | The "Inherited-Cruft Pattern" + HARD BANs on git restore/reset/stash (per `AGENTS.md`) | `AGENTS.md` §"Critical Anti-Patterns" | +| "Cleanup Workspace: provenance check" | n/a — project doesn't use worktrees | — | +| "Never merge without verifying tests on result" | The project's per-task atomic commits enforce "tests pass before commit"; this is upstream of the merge question | `conductor/workflow.md` §"Task Workflow" step 9 | +| "Require typed 'discard' confirmation" | The HARD BANs on `git restore`, `git reset`, `git stash*` (per AGENTS.md) effectively require explicit user permission for any destructive operation | `AGENTS.md` §"Critical Anti-Patterns" | +| "Skipping test verification" | The Isolated-Pass Verification Fallacy rule catches this upstream | `conductor/workflow.md` §"Isolated-Pass Verification Fallacy" | + +### 12.3 Where the project already follows the discipline + +- **The "Verify Tests before presenting options" rule** is enforced via Phase Completion Verification §3. +- **The "Require typed discard confirmation" rule** is enforced via the HARD BANs on git restore/reset/stash + the Inherited-Cruft Pattern (ASK the user before doing destructive work). +- **The "never merge without verifying tests" rule** is upstream-enforced via per-task atomic commits (tests pass before commit, not before merge). +- **The "Open-ended questions" anti-pattern** is implicitly avoided: the Phase Completion Verification protocol uses specific failure reporting. + +### 12.4 Where the project doesn't follow the discipline + +- **No 4-option menu** because the project doesn't use branch management. Phase 10 is a simpler artifact. +- **No worktree cleanup** because no worktrees. +- **No "Merge locally" option** because project doesn't merge. +- **No "Push and create PR" option** because project doesn't use GitHub PRs. + +### 12.5 Recommendations summary + +The project's master-direct workflow is a deliberate ARCH-DIFF from the skill. The discipline elements (verify tests, no destructive operations without confirmation, no skipping) are present via other rules. The deferred rebuild may want to: + +- **LOW:** Document the ARCH-DIFF explicitly in `conductor/workflow.md` §"Task Workflow" step 9: "The project works on master directly. The finishing-a-development-branch skill's 4-option menu (merge/PR/keep/discard) does not apply. Instead, the project's track archival flow (3 steps per `conductor/tracks.md` §Notes) handles track completion." + +**Verdict.** + +| Field | Value | +|---|---| +| **Primary** | `ARCH-DIFF` | +| **Integration tag** | `INTEGRATED` | +| **Section size** | brief | +| **Cross-refs** | nagent_review_20260608 §5 (durable work + disposable workers); fable_review_20260617 §13 (project's archival flow as "genuinely useful"); intent_dsl_survey_20260612 §6 | + +**Rationale.** The project's master-direct workflow is an architectural difference from the skill's branch-management workflow. The discipline elements (verify tests, no destructive ops without confirmation, no skipping) are present via other rules. The skill's 4-option menu doesn't apply. + +**Recommended change.** Document the ARCH-DIFF explicitly in `conductor/workflow.md` §"Task Workflow" step 9; consider promoting the track archival flow to a `conductor/workflow.md` §"Track Archival" section. ## 13. Using Git Worktrees