Skip to content

Fix link failure when existing PR in stack is queued#171

Open
skarim wants to merge 1 commit into
skarim/modify-rebase-abortfrom
skarim/link-failing-queued-prs
Open

Fix link failure when existing PR in stack is queued#171
skarim wants to merge 1 commit into
skarim/modify-rebase-abortfrom
skarim/link-failing-queued-prs

Conversation

@skarim

@skarim skarim commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

gh stack link refused to add PRs to a stack whose bottom PR was already in the merge queue. Because linking is additive — every existing stack PR must be listed again or the update is rejected for dropping them — including the queued PR triggered an eligibility failure, while omitting it triggered the drop guard:

✗ PR #439655 cannot be added to a stack: it is queued for merge

PR #439655 was already part of the target stack, so being in the merge queue should not have blocked adding new PRs on top of it. The eligibility check ran before link fetched the repository's stacks, so validatePREligibility had no way to know which PRs were already stacked and rejected every queued PR unconditionally. The same catch-22 applied to merged, closed, and auto-merge-enabled PRs already in the stack.

Behavior

link now resolves the target stack before validating eligibility and exempts PRs that are already members of it:

  • Already in the target stack — eligibility checks (queued, auto-merge, merged, closed) are skipped, since the PR is only being re-listed, not added. This lets new PRs be linked on top of a stack whose existing PRs are queued.
  • Not in the target stack — the previous strict checks still apply, so a queued or auto-merge PR that is not yet stacked is still rejected.
  • Creating a new stack — unchanged; with no target stack, every PR is validated as before.

Changes

cmd/link.go:

  • runLink: fetch the stacks and resolve the target stack (findMatchingStack) before eligibility validation instead of after, and pass it down.
  • validatePREligibility: takes the target stack and skips all eligibility checks for any PR already in it.
  • prevalidateStack: takes the already-resolved target stack instead of looking it up a second time.

cmd/link_test.go — four new tests:

  • TestLink_AllowsQueuedPRAlreadyInStack: a queued PR already in the stack no longer blocks adding new PRs; the stack is updated with the full ordered list.
  • TestLink_AllowsMergedPRAlreadyInStack / TestLink_AllowsAutoMergePRAlreadyInStack: the same exemption for merged and auto-merge-enabled members.
  • TestLink_RejectsQueuedPRNotInStack_WhenAddingToExistingStack: a queued PR not yet part of the target stack is still rejected.

Stack created with GitHub Stacks CLIGive Feedback 💬

Copilot AI review requested due to automatic review settings July 10, 2026 16:09

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

This PR fixes a gh stack link failure mode when the target stack already contains ineligible PR states (queued for merge, merged, closed, or auto-merge-enabled). The fix ensures link resolves the target stack before running eligibility checks, and exempts PRs that are already members of that target stack from those checks—allowing additive updates without getting stuck in the “must re-list but can’t validate” loop.

Changes:

  • Resolve the target stack earlier in runLink, then pass it into validation/prevalidation so existing stacked PRs can be exempted appropriately.
  • Update validatePREligibility to skip queued/merged/closed/auto-merge checks for PRs already present in the resolved target stack.
  • Add regression tests covering queued/merged/auto-merge exemptions for already-stacked PRs, plus a negative test ensuring queued PRs not in the stack are still rejected.
Show a summary per file
File Description
cmd/link.go Reorders stack resolution ahead of eligibility checks; threads the resolved stack into validation and simplifies prevalidation to use the already-matched stack.
cmd/link_test.go Adds targeted regression tests to ensure queued/merged/auto-merge PRs already in the target stack don’t block linking new PRs, while preserving rejection for queued PRs not yet in the stack.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Low

gh stack link validated PR eligibility before it fetched the repository's
stacks, so validatePREligibility rejected any queued PR unconditionally —
including one already a member of the stack being updated. Because link is
additive (every existing stack PR must be re-listed or the update is refused
for dropping them), a stack whose bottom PR was in the merge queue could never
take new PRs on top: re-listing the queued PR failed eligibility, and omitting
it failed the drop check.

Fetch the stacks and resolve the target stack before validating eligibility,
then skip the eligibility checks (queued, auto-merge, merged, closed) for any
PR that is already a member of that stack — those PRs are not being added, so
the checks don't apply. PRs not already in the stack, and brand-new stacks,
keep the previous strict behavior. prevalidateStack now takes the resolved
stack instead of looking it up a second time.

Add coverage for linking new PRs onto a stack whose existing PR is queued,
merged, or has auto-merge enabled, and for still rejecting a queued PR that is
not yet part of the target stack.
@skarim skarim force-pushed the skarim/link-failing-queued-prs branch from 6806de7 to b3250b9 Compare July 10, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gh stack link fails when existing stack contains queued PRs

2 participants