Skip to content

docs(pull-request-workflow): two preconditions a stack has to meet - #229

Merged
CybotTM merged 2 commits into
mainfrom
docs/stacked-pr-preconditions
Aug 25, 2026
Merged

docs(pull-request-workflow): two preconditions a stack has to meet#229
CybotTM merged 2 commits into
mainfrom
docs/stacked-pr-preconditions

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 25, 2026

Copy link
Copy Markdown
Member

Problem

The stacked-PR material covers what happens once a stack exists: merge order, retargeting, --delete-branch closing the child, approvals lost when the base merges. Two things that decide whether the stack can exist at all were not written down, and both were paid for in one afternoon.

A base must be a branch in the target repository. Contributing from a fork, the feature branch lives in the fork, so upstream has nothing to point at:

$ gh pr create --base feature/x --head myorg:follow-up
Base ref must be a branch (createPullRequest)

The check is one call — gh api "repos/$UPSTREAM" --jq '.permissions.push' — and it belongs before the branch is built, not after the PR is refused.

A split is only real if both halves stand on their own. A test and the CI change that lets it pass read like two things and are one. Worse, the coupling is usually already proven at the moment the split is proposed: if you diagnosed a CI failure where one half causes it and the other fixes it, you have your answer and the split is a promise you cannot keep.

Change

Two sections ahead of the existing stacked-PR material, since both are preconditions for it:

  • A stack needs push access to the repository the PRs target — the constraint, the error it produces, the one-line check, and what the options actually are on a fork (single PR, or a follow-up that waits and targets the default branch). Also names the trap of proposing a split and a stack in the same breath, where only the merge order is deliverable.
  • Before splitting a PR, run each half without the other — build each half, run the gates on it, and check your own diagnosis first.

Observed

phpDocumentor/guides#1345, 2026-08-25. A review round grew the PR past its subject, so the extra work was split onto its own branch and committed — then the upstream PR could not be opened (push: false on the base repo), and the halves turned out to be inseparable anyway: any test loading the class hits the CI failure the other half fixes, in every suite. Branch, PR and description were rebuilt around the split and then rebuilt again to undo it.

Assisted by claude-code:claude-opus-5 — Session

The stacked-PR material covers merge order, retargeting and lost approvals -
everything that happens once the stack exists. Two things that decide whether it
can exist at all were missing.

A pull request's base must be a branch in the repository it targets. Contributing
from a fork, the base branch lives in the fork, so `gh pr create --base` answers
"Base ref must be a branch" and there is no stack to be had. One API call on
.permissions.push settles it before the branch is built.

And a split is only real if both halves stand on their own. A test and the CI
change that lets it pass read like two things and are one; where a diagnosed CI
failure has one half causing it and the other fixing it, the question is already
answered and the split cannot be delivered.

Both were paid for on the same day: a split proposed for phpDocumentor/guides#1345,
built as a branch and a PR, then reversed - the halves were inseparable, and the
follow-up could not have been stacked upstream anyway.

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_014H1xwaAmrQRWUA3vx8bJcD
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@github-actions github-actions Bot added documentation Improvements or additions to documentation skill labels Aug 25, 2026
… clause

The note quoted `Base ref must be a branch` alone. GitHub answers with four
clauses, and a reader matching their own output against one of them will not
recognise it. One of the others actively misleads: `No commits between …` is
false in the sense it suggests - the branches do differ - and checking that is
exactly the detour this note exists to prevent.

Quote all four and name which one is the cause.

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_014H1xwaAmrQRWUA3vx8bJcD
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
@CybotTM
CybotTM marked this pull request as ready for review August 25, 2026 18:53
Copilot AI lite review requested due to automatic review settings August 25, 2026 18:53
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@CybotTM

CybotTM commented Aug 25, 2026

Copy link
Copy Markdown
Member Author

Copilot answered the ruleset's request on e7528d50 with "unable to review … reached their quota limit" and no comments, so there is no bot review on this head. Reviewed it myself.

It caught one thing. The note quoted Base ref must be a branch as though that were GitHub's answer; the answer has four clauses, and a reader matching their own output against one of them will not recognise it. One of the others actively misleads — No commits between … is false in the sense it suggests, the branches do differ, and checking that is exactly the detour the note exists to prevent. Fixed in e7528d5: all four quoted, with the operative one named.

Verified on the head: pre-commit run over the changed file passes, scripts/verify-harness.sh reports Level 3 COMPLETE with 0 errors, scripts/spec-cleanup-guard.sh reports clean. Branch is not behind main. No review threads. Reference text only — no SKILL.md, plugin manifest or version surface touched.

The .permissions.push check the section recommends was run against a real repository today (phpDocumentor/guides, push: false), which is what produced the case this documents.

Self-review: e7528d5

Assisted by claude-code:claude-opus-5 — Session

@CybotTM
CybotTM merged commit 1ab4487 into main Aug 25, 2026
23 of 24 checks passed
@CybotTM
CybotTM deleted the docs/stacked-pr-preconditions branch August 25, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants