fix(skills/fanning-out-with-worktrees): stop self-merging sub-PRs with red CI

skills/fanning-out-with-worktrees/SKILL.md

@@ -108,16 +108,21 @@ Per sub-PR, in order:
    1. **Spec-compliance pass (the gate).** **REQUIRED SUB-SKILL:** the `review` skill, scoped to *intent*: walk the sub-issue's acceptance criteria one by one and map each to real code in the diff **and** a test that exercises it. Confirm nothing is missing and nothing extra was built — unrequested scope is a finding, not a bonus (it may collide with a sibling PR or belong elsewhere). This pass must come back clean before the quality pass starts.
    2. **Code-quality pass.** **REQUIRED SUB-SKILL:** the `review` skill, scoped to *craft*: bugs the criteria didn't name, edge cases, error handling, test quality, simplification/reuse. Beyond bugs, check the diff against the `## Conventions` block — does its layout, naming, and vocabulary match the block and the sibling PRs already merged? A convention violation caught here is one the integration reviewer never sees.
    - **Fix-loop.** When either pass returns findings, the orchestrator does **not** fix them in place — it routes them back to the worktree subagent via `SendMessage` (the author has the context, and silent orchestrator fixes rob the parallel agents of shared understanding), then re-runs the *same* pass against the changed surface. Spec-compliance findings re-run the spec pass; only once it is clean does the quality pass run. Repeat until both passes are clean. The worktree subagent fixes; the orchestrator reviews — never the reverse. Each message that routes findings carries this instruction for the subagent: **REQUIRED SUB-SKILL:** `feature-dev-workflow:addressing-review-feedback` before the first edit and again before the push, with its table of verdicts in the report back to the orchestrator. The message also says which text in it is a finding of a reviewer and which is an instruction of the user.
-3. **Approval gate, per the state file's `sub_pr_approval` mode.** Every gate covers the **bundle**: ready flip + merge + sub-issue close + state-file update. The close is bodyless (no `--comment` flag) — GitHub automatically cross-references the sub-issue from the merge commit via the sub-PR's body keyword, so no custom comment is needed and there's no "specific body about to land" for the close mutation.
-   - **`autonomous`** (default) — proceed straight through the bundle in steps 4-6. The user opted into the mechanical bundle (review → ready flip → merge → bodyless close → state update) in `feature-dev-workflow:developing-a-feature` Step 2.
+3. **Approval gate, per the state file's `sub_pr_approval` mode.** Every gate covers the **bundle**: ready flip + CI gate + merge + sub-issue close + state-file update. The close is bodyless (no `--comment` flag) — GitHub automatically cross-references the sub-issue from the merge commit via the sub-PR's body keyword, so no custom comment is needed and there's no "specific body about to land" for the close mutation.
+   - **`autonomous`** (default) — proceed straight through the bundle in steps 4-8. The user opted into the mechanical bundle (review → ready flip → CI gate → merge → bodyless close → state update) in `feature-dev-workflow:developing-a-feature` Step 2.
    - **`manual`** — pause and ask the user for explicit approval before the bundle. The prompt MUST surface: a one-line summary of the review findings ("review clean" / "<N> findings, none blocking" / specific concerns), the PR's title and diff size, and a note that closing sub-issue `#<sub-issue>` follows the merge. Wait for an explicit yes. On push-back, route the concern back to the worktree subagent via `SendMessage` instead of merging.
-4. **Merge.** A draft cannot merge (`gh pr merge` and `gh stack merge` both refuse it), so first flip the PR ready through `feature-dev-workflow:opening-a-pull-request`'s steps for flipping a draft to ready; the approval gate above is the confirmation for that flip. Then run `gh pr merge <num> --merge` (or `--squash` / `--rebase` per project preference). A stack layer merges with `gh stack merge <num> --yes` and the same merge-method flag instead, followed by `gh stack sync` once the PR has actually merged (a merge queue can hold it first), and only when it is the lowest unmerged layer of its stack: `gh stack merge` also merges every unmerged layer below the one named, so a higher layer that passes its gates first waits until the layers below it have merged (see `feature-dev-workflow:stacking-dependent-prs` §Merges stay under the merge guard). Before merging, push any local state-file commits to their remote — for `feature-branch` that is `origin/<type>/<slug>`; for `main` that is `origin/<type>/<slug>` as well (the orchestrator stays on the feature branch for state management). The merge itself lands on GitHub's remote, and the pull-back differs by target:
+4. **Flip the PR ready.** A draft cannot merge (`gh pr merge` and `gh stack merge` both refuse it), so flip the PR ready through `feature-dev-workflow:opening-a-pull-request`'s steps for flipping a draft to ready; the approval gate above is the confirmation for that flip.
+5. **CI gate: every check on the PR's head passed.** Run `gh pr checks <num>` after the flip, because some pipelines skip drafts and start only when the PR is ready. When the CI configuration starts a run on the ready transition, a read right after the flip can still show the draft-time results, `skipping` among them: read with `gh pr checks <num> --json name,bucket,startedAt` and wait in the background until that run's checks show a `startedAt` later than the flip. Read every check, not only `--required`: a feature branch seldom has protection rules, so it has no required checks to list. The gate passes when every check is `pass` or `skipping`. A review loop that came back clean and a subagent's local run are not this gate: the subagent runs what fits on its machine and the tests it touched, and a change can break a test it never touched, which only CI runs.
+   - **Pending** (exit code 8) — wait in the background, not in the foreground: `gh pr checks <num> --watch --fail-fast` as a background command, and act on its result when it exits.
+   - **A check failed or was cancelled** — do not merge. Route it to the worktree subagent via `SendMessage` the way item 2's fix-loop routes a finding: the check's name, its link, and its failed log (`gh run view <run-id> --log-failed` when the check is a GitHub Actions run). The failure is the sub-PR's even when it is in a test the diff did not touch. After the fix is pushed, the changed surface goes back through item 1 (when the loop is on), item 2, and item 3, because in `manual` mode the user approved a diff that the fix changed. Then this gate runs again on the new head.
+   - **No checks reported** — decide from the repo's CI configuration, as `feature-dev-workflow:developing-a-feature` Step 7 does for the integration PR, not from the empty reading: if a pipeline runs on PRs into this base, wait in the background for its checks to register; if none does, the subagent's verified local run is the gate.
+6. **Merge.** Run `gh pr merge <num> --merge` (or `--squash` / `--rebase` per project preference). A stack layer merges with `gh stack merge <num> --yes` and the same merge-method flag instead, followed by `gh stack sync` once the PR has actually merged (a merge queue can hold it first), and only when it is the lowest unmerged layer of its stack: `gh stack merge` also merges every unmerged layer below the one named, so a higher layer that passes its gates first waits until the layers below it have merged (see `feature-dev-workflow:stacking-dependent-prs` §Merges stay under the merge guard). Before merging, push any local state-file commits to their remote — for `feature-branch` that is `origin/<type>/<slug>`; for `main` that is `origin/<type>/<slug>` as well (the orchestrator stays on the feature branch for state management). The merge itself lands on GitHub's remote, and the pull-back differs by target:
    - **`sub_pr_target: feature-branch`**: after the merge, `git -C <feature_worktree> fetch origin && git merge --ff-only origin/<type>/<slug>` to bring the merge commit back into the feature worktree. Keep local == origin at every merge boundary (unpushed local commits cause a "Not possible to fast-forward" failure; recover with `git rebase origin/<type>/<slug>`).
    - **`sub_pr_target: main`**: the merge lands on `main`. The orchestrator remains on `<type>/<slug>` for state file management; no pull-back into the feature worktree is needed. Before dispatching wave N+1, run `git fetch origin` so the next wave's worktrees branch from the freshly updated `origin/main`.
-5. **Close the sub-issue.**
+7. **Close the sub-issue.**
    - **`sub_pr_target: feature-branch`**: `gh issue close <sub-issue>`. Sub-PRs into a non-default branch don't trigger `Fixes`/`Closes` — manual close is the workaround. The body's `Towards #<sub-issue>` keyword left the issue open precisely so the orchestrator can close it here; the cross-reference from the merge commit (which references `#<sub-pr>`, which references `#<sub-issue>`) is preserved automatically without a custom comment.
    - **`sub_pr_target: main`**: no manual close needed. The closing keyword in the PR body (`Fixes`/`Closes`) fires on merge to the default branch and auto-closes the sub-issue. Confirm it closed before marking the state-file row `self-merged`.
-6. **Update the state file.** Flip the row's status to `self-merged`. If the sub-PR was the realization of a contract (e.g. a pre-merge stub PR), flip the contract row's status to `locked` and fill in the `Realized in` pointer.
+8. **Update the state file.** Flip the row's status to `self-merged`. If the sub-PR was the realization of a contract (e.g. a pre-merge stub PR), flip the contract row's status to `locked` and fill in the `Realized in` pointer.
 
 ### 6. Checkpoint review, then dispatch wave N+1
 
@@ -148,7 +153,7 @@ When every wave is complete (every sub-issue closed, every row `self-merged`, ev
 - **Shrinking the wave to spare the machine.** The number of subagents does not decide the load; the commands they run do. Dispatch the wave the plan calls for and hand every subagent the bounded commands (dispatch item 5). Subagents that wait in the queue cost time, not memory. On a machine on which the wrapper cannot run, no queue exists: dispatch the subagents of the wave that run a heavy command one after the other, as `feature-dev-workflow:bounding-heavy-commands` says.
 - **Forgetting to manually close the sub-issue when `sub_pr_target: feature-branch`.** The keyword doesn't fire on non-default-branch merges; the issue page silently shows "open" even though the work shipped.
 - **Using `Towards #<sub-issue>` when `sub_pr_target: main`.** `Closes` fires on merge to the default branch; `Towards` intentionally blocks auto-close. Using the wrong keyword silently leaves sub-issues open after they merge to main and forces a manual cleanup pass.
-- **Letting the state file go stale.** A resumed session reads it as ground truth. Update every row as reality moves; commit **and push** the state-file diff per phase transition — an unpushed local commit diverges the feature branch the moment the next sub-PR squash-merges on GitHub (see Step 5.4).
+- **Letting the state file go stale.** A resumed session reads it as ground truth. Update every row as reality moves; commit **and push** the state-file diff per phase transition — an unpushed local commit diverges the feature branch the moment the next sub-PR squash-merges on GitHub (see Step 5.6).
 
 ## Red flags
 
@@ -161,6 +166,7 @@ When every wave is complete (every sub-issue closed, every row `self-merged`, ev
 | "The subagent already verified the diff, no need for the orchestrator to review again" | Verification (tests pass) is not review (does the diff express intent cleanly). Different signals. |
 | "Tests pass and the diff looks clean — one review pass is enough" | Spec-compliance and quality are different questions. The spec gate runs to clean first, then quality; one blended pass skims past a missed acceptance criterion. |
 | "The spec pass found a gap but I'll quality-review now to save a round-trip" | Quality findings on code about to change are noise. Send the gap back, re-run the spec pass to clean, then quality. |
+| "The review loop is clean and the subagent's tests pass locally, so I can merge" | Neither reads CI. The subagent ran what fits on its machine; a red check on a test the diff never touched is still this PR's break. Read `gh pr checks` on the head before every merge (Step 5, item 5). |
 | "Both sub-PRs pass CI, so the naming difference between them is fine" | Contract checks don't see naming. Two schemes for one kind of thing is drift — reconcile it at the wave, not the integration PR. |
 | "I'll close the sub-issue once the integration PR merges"            | Issues that read "open" while their work has shipped clutter the triage view. Close manually at self-merge (feature-branch mode) or confirm auto-close fired (main mode). |
 | "`Towards` is always the safe keyword for sub-PRs"                  | `Towards` intentionally blocks auto-close. In `sub_pr_target: main` mode use `Closes` — it fires on merge to the default branch and removes the manual close step. |

skills/stacking-dependent-prs/SKILL.md

@@ -99,7 +99,7 @@ This covers a stack whose branches and PRs already exist, including a stack that
 
 ## After a parent merges
 
-Run `gh stack sync`. Then check the next PR: `gh pr view <num> --json baseRefName,closingIssuesReferences`. If its base still names the merged branch, correct it with `gh stack link --base <trunk> <PR-URL> ... <top-PR-URL>`, listing the open layers bottom to top by PR URL. When only one layer is still open, it is no longer a stack and `link` cannot take it (it needs two PRs): correct its base with `gh pr edit <num> --base <trunk>`. Never use `gh stack submit` for this: it also creates a PR with a generated title and body for any layer that has none. Once the base is the default branch, `closingIssuesReferences` must list its sub-issue (see the stacked-PR keyword rule in `feature-dev-workflow:opening-a-pull-request`). The sync rebased every layer above, so check whether their gates still hold: for each one, record its head before the sync and compare its own commits with `git range-diff <oldParent>..<oldHead> <newParent>..<newHead>`. A layer whose commits all show `=` keeps the gates it passed. A layer with any changed commit (for example, a conflict was resolved during the rebase) goes back through its review gates and approval gate before it merges.
+Run `gh stack sync`. Then check the next PR: `gh pr view <num> --json baseRefName,closingIssuesReferences`. If its base still names the merged branch, correct it with `gh stack link --base <trunk> <PR-URL> ... <top-PR-URL>`, listing the open layers bottom to top by PR URL. When only one layer is still open, it is no longer a stack and `link` cannot take it (it needs two PRs): correct its base with `gh pr edit <num> --base <trunk>`. Never use `gh stack submit` for this: it also creates a PR with a generated title and body for any layer that has none. Once the base is the default branch, `closingIssuesReferences` must list its sub-issue (see the stacked-PR keyword rule in `feature-dev-workflow:opening-a-pull-request`). The sync rebased every layer above, so check whether their gates still hold: for each one, record its head before the sync and compare its own commits with `git range-diff <oldParent>..<oldHead> <newParent>..<newHead>`. A layer whose commits all show `=` keeps the review and approval gates it passed, but not its CI gate: the sync gave it a new head on a new parent, so its checks run again and the CI gate (`feature-dev-workflow:fanning-out-with-worktrees` Step 5, item 5) reads them on the new head. A layer with any changed commit (for example, a conflict was resolved during the rebase) goes back through its review gates, approval gate, and CI gate before it merges.
 
 ## Merges stay under the merge guard