Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 68 additions & 5 deletions .agents/skills/iterate-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,20 @@ Cascade-rebases a branch's descendants onto their parents to carry a fix up the
uv run ${CLAUDE_SKILL_ROOT}/scripts/propagate_stack.py --root <branch> [--dry-run] [--no-push]
```

**One failure path leaves the repo on another branch.** Normal completion, a
rebase conflict, a balloon guard trip, and a push failure all `checkout` the
branch you started on before returning. The exception is when checking out a
_child_ fails (exit 6) — most often because that branch is checked out in a
worktree — which returns immediately, leaving you on whichever branch it had
reached.

That matters because anything you do next inherits it as a base: branching "from
main" right after one of these silently creates a branch on top of the whole
stack, and the resulting PR carries every stack commit plus an OpenSpec change
directory it never touched. After a non-zero exit, run `git checkout main` (or
`git checkout -B <new> origin/main`), and check `git log --oneline origin/main..HEAD`
before opening the PR.

## Workflow

### 1. Identify PR
Expand Down Expand Up @@ -232,11 +246,11 @@ exactly once, at the END of the work — so a PR that still carries an in-progre
change directory will fail this check. Archiving on an intermediate PR is wrong:
it would remove the change docs before the implementation PRs above it merge.

Note the workflow's trigger is currently `pull_request.branches: [main]`, so it
only RUNS on PRs whose base is `main`. A stacked PR based on another feature
branch won't run (or fail) this check at all — so there is nothing to ignore
there. The check matters for PRs that target `main`: typically the bottom of a
stack, plus any PR later retargeted to `main` as the stack merges down.
The workflow's trigger is `pull_request.branches: [main]`, but do NOT read that
as "it only runs on PRs whose base is `main`." Under GitHub's stacked-PR support
a stacked PR targets `main` eventually and the filter matches that eventual
target, so this workflow runs on mid-stack PRs as well. Expect to see the check
on every PR in a stack and decide from stack position, not from the `on:` block.

When the archive check does run and fail, decide ONE thing before treating it as
actionable: **is this PR the last in the chain (the tip)?** A PR is the tip when
Expand Down Expand Up @@ -264,6 +278,55 @@ Then:
This rule applies ONLY to the OpenSpec Archive Check. Every other check is
handled normally regardless of stack position.

#### Stacked PRs: two other failures that are structural, not regressions

**`Require a changeset` on the bottom PR.** The check looks for a
`.changeset/*.md` added in that PR's own diff, so the bottom PR — the one
targeting `main` — is the only place a changeset can satisfy it. If it fails
there, move the changeset DOWN to the bottom branch rather than labelling
anything `skip-changeset`; every branch above inherits it, since a child
contains its ancestors' commits. Extend that one file as later PRs land; never
add a second changeset per PR.

Mid-stack PRs bypass the check on their base ref. If you see one failing it,
Comment thread
thecodedrift marked this conversation as resolved.
look at that guard rather than reaching for the label, which would wrongly
record the change as shipping no release note.

**Do not read `on: pull_request: branches: [main]` as "this only runs on the
bottom PR."** Under GitHub's stacked-PR support a stacked PR targets `main`
eventually, and the filter matches that eventual target — so these workflows run
on mid-stack PRs too. A workflow that must act only on the PR merging to `main`
has to establish that from the base ref or its stack position. When judging
whether a check "should even be running here," check the stack rather than the
`on:` block.

### Never leave a PR on red

**No PR merges with failing tests, including mid-stack.** "Merging down" changes
where a PR merges, not whether its tests pass — and a red branch you merge into
its parent carries that red upward.

It is tempting on a stack to explain a failure as structural: this unit
relocates something and the unit that consumes it lands next, so of course it
fails. **Treat that explanation as a bug report about the split, not a reason to
proceed.** If unit N alone leaves the suite red, unit N is incomplete — the
change that makes it self-consistent belongs _in_ it. In practice that means the
PR that moves something also repoints every reader and updates the tests that
assert the old shape; the next PR then generalizes.

This is not hypothetical. A stack here sat on 20 failures rationalized as
"dispatch arrives in the next PR." The real causes were a migration that
relocated files while every reader kept the old paths, and a command that
discovered rules _before_ running the migration that creates them. Both were
real defects, in the released code path, that the structural story explained
away. An empty scan reports success, so the symptom was "no findings" rather
than an error — which is exactly why it looked like an artifact of the split.

So: never annotate a red check as expected and move on. Check the branch out,
run the suite, and fix the unit until it is green on its own. The only failures
that survive that treatment are ones you can point at a specific missing commit
for — and if that commit is in this stack, it belongs in this PR.

### 5. Fix CI Failures

For each failure in the script output:
Expand Down
27 changes: 27 additions & 0 deletions .github/workflows/require-changeset.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,39 @@ jobs:
- name: Check for a changeset
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
BASE_REF: ${{ github.event.pull_request.base.ref }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
HEAD_REF: ${{ github.event.pull_request.head.ref }}
LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }}
run: |
set -euo pipefail

# Mid-stack PR. On a stack, exactly one changeset describes the whole
# change and it lives on the bottom PR — the one targeting `main`,
# which is where this check has to pass. Every branch above inherits
# that file, so it is never *added* in a child's own diff, and without
# this guard the check below fails every one of them. That must not be
# papered over with a `skip-changeset` label, which would wrongly
# record "this change ships no release note."
#
# This guard is required, not belt-and-braces. `branches: [main]` reads
# like "only PRs whose base is main," and that is how it filtered when
# a PR had exactly one base. Under GitHub's stacked-PR support a PR in
# a stack is understood to target `main` *eventually*, so the filter
# matches on the eventual target and the workflow runs on mid-stack
# PRs too. Observed here: #73, #80, and #81 — bases
# `openspec/partition-engine-*`, never `main` — each produced a failing
# `Require a changeset` check run (e.g. run 30786954198, event
# `pull_request`, head `openspec/partition-engine-2-dispatch`).
#
# So `branches:` no longer scopes a workflow to the bottom of a stack.
# Any job whose correctness depends on "is this the PR that merges to
# main" has to establish that itself, as this one does.
if [ "$BASE_REF" != "main" ]; then
echo "Base is '$BASE_REF', not 'main' — mid-stack PR, so the bottom PR of the stack carries the changeset."
exit 0
fi

# The changesets "Version Packages" PR consumes changesets (removing
# them is its whole job), so it legitimately has none. Bypass it by its
# well-known bot branch name so it needs no manual `skip-changeset`.
Expand Down
15 changes: 15 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,21 @@ The deciding question between forward and down is only this: **can each unit rea

Note how this interacts with the archive gate (see the OpenSpec archive check below): a change is archived exactly once, on whichever PR is the tip. Mid-stack PRs are expected to carry an unarchived change directory and the gate skips them.

### One changeset, at the bottom of the stack, grown as the stack grows

`require-changeset.yml` looks for a `.changeset/*.md` **added in that PR's own diff**. On a stack that means:

- **The changeset belongs on the bottom PR**, the one that targets `main`. That is the only place it can live: a changeset added on the tip is invisible to the bottom PR's diff, so the check would fail on the PR that actually merges.
- **Mid-stack PRs bypass the check**, because the base branch is not `main`. They inherit the base's changeset rather than adding one, so there is nothing for the check to find. **Do not label them `skip-changeset`** — the label records a deliberate "this change ships no release note," which is false here, and the bypass already handles it.

The bypass is an in-step check on the base ref, and it is load-bearing. **`branches: [main]` no longer means "only the PR whose base is `main`."** Under GitHub's stacked-PR support, a PR in a stack is understood to target `main` eventually, so the filter matches on that eventual target and the workflow runs on mid-stack PRs as well — observed here on #73, #80, and #81, all with `openspec/partition-engine-*` bases.

The general rule that follows: **any workflow whose correctness depends on "is this the PR that merges to `main`" must determine that itself** — from the base ref, or by resolving stack position — and cannot lean on the `on:` filter to scope it. If you see a mid-stack PR failing this check, look at that guard rather than reaching for the label.

Put the changeset at the base and every branch above inherits it, since a child contains its ancestors' commits.

**Grow it incrementally as the stack lands.** Each PR extends the changeset with its own scope rather than the base describing the whole future change up front. A reviewer reading the changeset then sees only what has actually landed, and is not asked to evaluate a release note that promises more than the diff in front of them. When you extend it, edit the same file on the branch you are working on — never add a second changeset per PR, or one change becomes several release notes for what merges to `main` exactly once.

### Landing a stack: merge _down_, then one merge to `main`

Merge each PR **down** into its parent's branch, from the tip to the bottom:
Expand Down
Loading