conductor(track): superpowers review section 12 — finishing-a-development-branch (brief)
This commit is contained in:
@@ -723,7 +723,74 @@ The project has the discipline's core elements (per-task atomic commits, git not
|
||||
|
||||
## 12. Finishing a Development Branch
|
||||
|
||||
<!-- Section 12 brief (50-150 LOC). Skill: finishing-a-development-branch. Verdict pending. -->
|
||||
## 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/<id>/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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user