Skip to content

fix(ci): bypass the changeset requirement on mid-stack PRs - #83

Merged
thecodedrift merged 5 commits into
mainfrom
docs/stack-changeset-guidance
Aug 3, 2026
Merged

fix(ci): bypass the changeset requirement on mid-stack PRs#83
thecodedrift merged 5 commits into
mainfrom
docs/stack-changeset-guidance

Conversation

@thecodedrift

Copy link
Copy Markdown
Member

Records how changesets behave on a merge-down stack, learned while landing the partition-rules-by-engine stack (#79#80#81#73).

require-changeset.yml is on: pull_request: branches: [main] and looks for a .changeset/*.md added in that PR's own diff. So a mid-stack PR never runs the check at all, and the bottom PR is the only place a changeset can satisfy it — one added on the tip is invisible to the diff of the PR that actually merges. Labelling mid-stack PRs skip-changeset "fixes" a check that was never going to run, while implying the change ships no release note.

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

The guidance also asks for the changeset to be grown as the stack lands rather than written up front to describe the whole future change, so a reviewer evaluates a release note that matches the diff in front of them.

Separately, iterate-pr now names the other structural red check on this shape: a lower PR can be legitimately failing because the unit that completes it has not landed yet. The instruction is to verify that against the change's own tasks by running the suite on the branch — not to assume it, and not to write throwaway assertions to make an intermediate state look green.

@thecodedrift thecodedrift added the skip-changeset PR intentionally ships no release note (bypasses the changeset requirement) label Aug 2, 2026
@thecodedrift thecodedrift changed the title docs: record how changesets work on a merge-down stack fix(ci): bypass the changeset requirement on mid-stack PRs Aug 2, 2026
@thecodedrift thecodedrift added skip-changeset PR intentionally ships no release note (bypasses the changeset requirement) and removed skip-changeset PR intentionally ships no release note (bypasses the changeset requirement) labels Aug 2, 2026
Require Changeset runs only on PRs based on `main` and reads the PR's own diff,
so a mid-stack PR never runs it and the bottom PR is the only place a changeset
can satisfy it. Labelling mid-stack PRs `skip-changeset` implies a decision that
was never needed; putting the changeset at the base means every branch above
inherits it.

Grow that one file as the stack lands, so a reviewer evaluates a release note
matching the diff in front of them rather than the whole future change.

Also notes in iterate-pr that a lower PR on a merge-down stack can be
legitimately red, with the instruction to verify that against the change's own
tasks rather than assume it.
On a stack, one changeset describes the whole change and lives on the bottom PR
— the one targeting `main`, which is where the check has to pass. Branches above
inherit that file, so it is never *added* in a child's own diff and every
mid-stack PR failed. Labelling them `skip-changeset` records a deliberate "ships
no release note," which is false, so the workflow bypasses on the base ref
instead.

The `on: branches: [main]` filter does not hold for stacked PRs in practice —
the workflow still runs on a PR based on a feature branch — so the base is
re-checked inside the step rather than trusted. Observed on #80, #81, and #73,
all of which ran and failed the check with a non-main base.
@thecodedrift
thecodedrift force-pushed the docs/stack-changeset-guidance branch from 256f6f2 to 2944a99 Compare August 2, 2026 23:36
…k branch

Branching "from main" immediately after a propagate picks up the last rebased
branch instead, so the new PR carries the entire stack and an OpenSpec change
directory it never touched. Hit while opening this PR.
@thecodedrift
thecodedrift marked this pull request as ready for review August 3, 2026 13:56
Copilot AI review requested due to automatic review settings August 3, 2026 13:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates repository guidance and CI behavior around changesets in merge-down stacks, aiming to ensure only the stack’s bottom PR (targeting main) is responsible for adding and evolving the single changeset while mid-stack PRs don’t get pressured into using skip-changeset.

Changes:

  • Add workflow logic to bypass the changeset requirement when the PR base branch is not main.
  • Document “one changeset on the bottom PR, grown incrementally” guidance in CLAUDE.md.
  • Extend the iterate-pr agent skill docs with stack-specific failure interpretation and workflow hygiene notes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
CLAUDE.md Adds documented guidance for changeset placement/growth on merge-down stacks.
.github/workflows/require-changeset.yml Adds a base-branch guard to bypass the changeset requirement for non-main base refs.
.agents/skills/iterate-pr/SKILL.md Documents stack-related “structural” failures and safe next-steps for iterate-pr.

Comment thread .github/workflows/require-changeset.yml Outdated
Comment thread .agents/skills/iterate-pr/SKILL.md Outdated
Comment thread CLAUDE.md Outdated
@thecodedrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @thecodedrift's task in 4m 19s —— View job


Review of #83

Summary: This PR is documentation + a defensive CI guard, all correct in intent (one changeset lives on the bottom PR of a merge-down stack and is inherited by children; iterate-pr should distinguish structural red checks from real regressions). No functional risk to the require-changeset.yml check — the new bypass is inert, not harmful. I found one factual inaccuracy repeated in three places (already flagged by the automated Copilot review, which I independently confirmed) and one new inaccuracy in the propagate_stack.py behavior claim that I verified against the actual script source.

1. The "branches: [main] filter doesn't hold for mid-stack PRs" claim is incorrect (3 locations)

For a pull_request trigger, GitHub's branches: filter matches the PR's base branch. require-changeset.yml's on: pull_request: branches: [main] therefore means the workflow never runs at all for a PR whose base isn't main — there's no scenario where it "still runs on a PR based on a feature branch" that the new BASE_REF check needs to catch. Concretely:

  • .github/workflows/require-changeset.yml:47-54 — the code addition itself is harmless (it can just never take the != main branch given the trigger), but the comment justifying it asserts something false about GitHub Actions trigger semantics.
  • CLAUDE.md:96 — repeats the same claim as prose guidance.
  • .agents/skills/iterate-pr/SKILL.md:285 — repeats it again as advice for what to do if a mid-stack PR fails this check.

Left as-is, this teaches an incorrect mental model to whoever (human or agent) reads it next. Recommend rewording all three to describe the BASE_REF check as a defensive/redundant guard rather than implying the trigger filter is unreliable — I posted inline comments with suggested phrasing on each.

2. The propagate_stack.py "leaves you on a stack branch" claim doesn't match the current script (SKILL.md:105-111)

I read propagate_stack.py to verify this before commenting. On a normal completed run — success or most early-return failures (conflict, balloon guard, push failure) — the script explicitly checkouts back to the branch it started on before returning. The one path that doesn't restore the start branch is a checkout failure for a child mid-loop, which is a specific failure mode, not the general case the new text implies. Suggest either narrowing the doc to that specific condition, or better, fixing the actual gap in propagate_stack.py (the missing restore-to-start on that one return path) rather than documenting a workaround for it. Detail and line references in the inline comment.

Other notes

  • No .changeset/*.md is added by this PR, but it's correctly labelled skip-changeset (CI/docs-only change, consistent with the new guidance being documented).
  • CLAUDE.md and SKILL.md sections are well-placed within their existing "Stacked PRs" structure and don't duplicate content elsewhere.
  • No security, performance, or test-coverage concerns — this is docs/CI-comment only, no code paths that need test coverage.

Nothing here blocks merging on its own (the workflow guard is inert, not broken), but I'd fix the three repeated inaccuracies before they propagate further as "how our CI works" folklore.

Comment thread .github/workflows/require-changeset.yml Outdated
Comment thread CLAUDE.md Outdated
Comment thread .agents/skills/iterate-pr/SKILL.md
Comment thread .agents/skills/iterate-pr/SKILL.md Outdated
…laim

Review pushed back on three statements, and two of them were wrong.

`propagate_stack.py` does return you to the branch you started on — normal
completion, rebase conflict, balloon guard, and push failure all check it out
before returning. Only a failure to check out a *child* (exit 6, typically that
branch being held by a worktree) returns early without it. That is the path that
actually bit, so the warning now names it instead of describing the script's
normal behavior.

The `branches: [main]` claim is reframed rather than dropped. It is documented to
filter on the base branch, and the guard should be unreachable — but #73, #80,
and #81 each produced a failing `Require a changeset` check run with an
`openspec/partition-engine-*` base, and a workflow that never triggers produces
no check run at all. The comment now cites that evidence and presents the check
as a defensive guard, without teaching that Actions filters are unreliable in
general.

Also replaces the "structural failures are acceptable" guidance with the opposite
rule: a mid-stack PR that is red is incomplete, and the fix belongs in it. That
advice let this stack sit on 20 failures that turned out to be two real defects.
…l target

`branches: [main]` reads as "only PRs whose base is main," and filtered that way
when a PR had one base. Under GitHub's stacked-PR support a stacked PR targets
`main` eventually, so the filter matches the eventual target and these workflows
run on mid-stack PRs too — which is why #73, #80, and #81 each produced a failing
`Require a changeset` run with an `openspec/partition-engine-*` base.

That makes the base-ref guard required rather than defensive, and generalizes: a
workflow whose correctness depends on "is this the PR that merges to main" has to
establish that itself. Corrects the OpenSpec archive-check section too, which
told readers a stacked PR would not run that check at all.
@thecodedrift

Copy link
Copy Markdown
Member Author

Re: @claude[bot] — "Claude finished @thecodedrift's task — Review of #83 … one factual inaccuracy repeated in three places … and one new inaccuracy in the propagate_stack.py behavior claim"
#83

Both findings were right, and both are fixed. The propagate_stack.py claim was simply wrong — the script does restore the starting branch on normal completion, conflict, balloon guard, and push failure; only the child-checkout failure (exit 6) returns without it. That is the path that actually caused this, since the tip branch was held by a worktree, so the warning now names it.

The branches: [main] finding landed differently than either of us framed it. Your reasoning about the documented semantics is correct, and the guard is nonetheless load-bearing: #73, #80, and #81 each produced a failing Require a changeset check run with openspec/partition-engine-* bases, and a workflow that never triggers produces no check run at all. The mechanism is GitHub's stacked-PR support — a PR in a stack targets main eventually, and the filter matches that eventual target, so these workflows do run mid-stack.

So the conclusion inverts: branches: can no longer be relied on to scope a workflow to the bottom of a stack, and any job whose correctness depends on "is this the PR that merges to main" has to establish that itself. All three sites now say that, and the OpenSpec archive-check section — which told readers a stacked PR would not run that check at all — is corrected on the same grounds.

— AI Coding Agent

@thecodedrift

Copy link
Copy Markdown
Member Author

Re: @copilot-pull-request-reviewer — "## Pull request overview — Updates repository guidance and CI behavior around changesets in merge-down stacks…"

Both of your findings are addressed (replied inline and resolved). The branches: [main] wording is corrected in all three places, though not in the direction the review suggested: rather than being redundant, the base-ref check turns out to be required, because GitHub's stacked-PR support means a stacked PR matches branches: [main] on its eventual target. Evidence is on the inline threads — #73, #80, and #81 each produced a failing check run with a feature-branch base.

— AI Coding Agent

@thecodedrift
thecodedrift merged commit 7466145 into main Aug 3, 2026
5 checks passed
@thecodedrift
thecodedrift deleted the docs/stack-changeset-guidance branch August 3, 2026 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR intentionally ships no release note (bypasses the changeset requirement)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants