preflight check for stacked PR availability in submit#44
preflight check for stacked PR availability in submit#44skarim wants to merge 5 commits intoskarim/fix-onto-rebasefrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a “stacked PRs availability” preflight to gh stack submit, introducing a dedicated exit code (9) and updating docs/site assets to reflect the new behavior and improve docs presentation/metadata.
Changes:
- Add
ErrStacksUnavailable(exit code 9) and asubmitpreflight that can skip stack syncing when stacks aren’t available. - Add test coverage for the new preflight paths and a
ForceInteractiveconfig knob to reliably exercise interactive behavior in tests. - Update docs/skill content and the docs site (new inline SVG component, social card, and OpenGraph/Twitter metadata).
Show a summary per file
| File | Description |
|---|---|
| skills/gh-stack/SKILL.md | Documents submit’s behavior when stacked PRs aren’t available and exit code 9. |
| internal/config/config.go | Adds ForceInteractive to control IsInteractive() in tests. |
| docs/src/content/docs/reference/cli.md | Documents new exit code 9 in the CLI reference. |
| docs/src/content/docs/index.mdx | Switches stack diagram rendering to a component. |
| docs/src/components/StackDiagram.astro | New inline-SVG diagram component to avoid Safari theme issues. |
| docs/src/assets/stack-diagram.svg | Removes the old SVG asset. |
| docs/public/github-social-card.jpg | Adds a social preview image for OpenGraph/Twitter cards. |
| docs/astro.config.mjs | Updates site description and adds OG/Twitter meta tags. |
| cmd/utils.go | Introduces ErrStacksUnavailable (exit code 9). |
| cmd/submit_test.go | Adds tests for submit preflight scenarios. |
| cmd/submit.go | Implements stacks availability preflight and skips syncStack when unavailable. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
cmd/submit_test.go:961
TestSyncStack_SkippedWhenStacksUnavailabledoesn’t actually verify behavior: it assertscreateCalledis false after explicitly not callingsyncStack, which will always pass regardless of production logic. This test should either be removed or rewritten to exerciserunSubmitin the “stacks unavailable but user proceeds” path and assert the GitHub client’sCreateStack/UpdateStackare not called.
func TestSyncStack_SkippedWhenStacksUnavailable(t *testing.T) {
// Verify that syncStack is not called when stacksAvailable is false.
// This is the core behavior enabling unstacked PR creation.
s := &stack.Stack{
Trunk: stack.BranchRef{Branch: "main"},
Branches: []stack.BranchRef{
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
},
}
createCalled := false
mock := &github.MockClient{
CreateStackFn: func(prNumbers []int) (int, error) {
createCalled = true
return 42, nil
},
}
cfg, _, errR := config.NewTestConfig()
// When stacksAvailable=true, syncStack should be called.
syncStack(cfg, mock, s)
assert.True(t, createCalled, "syncStack should call CreateStack when invoked")
// When stacksAvailable=false, the caller (runSubmit) skips syncStack
// entirely — verified by the submit_test integration tests above.
// Here we just confirm the contract: if syncStack is NOT called,
// CreateStack is NOT called.
createCalled = false
// (not calling syncStack)
assert.False(t, createCalled, "CreateStack should not be called when syncStack is skipped")
- Files reviewed: 9/11 changed files
- Comments generated: 4
| inR, inW, _ := os.Pipe() | ||
| inW.Close() | ||
|
|
||
| cfg, _, errR := config.NewTestConfig() | ||
| cfg.In = inR | ||
| cfg.ForceInteractive = true |
There was a problem hiding this comment.
TestSubmit_PreflightCheck_404_Interactive_UserDeclinesAborts creates an os.Pipe() and assigns cfg.In = inR, but the read end is never closed. Please defer inR.Close() (and handle the pipe creation error) to avoid leaking file descriptors across the test suite.
This issue also appears on line 929 of the same file.
There was a problem hiding this comment.
Good catch — fixed with defer inR.Close() in 9c58a54.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a branch name was previously used for a merged PR, syncStackPRs would adopt the old merged PR for the new branch, falsely showing it as merged in gh stack view. Now, branches with no recorded PR only adopt open PRs from the API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pre-flight check for stacked PR availability in
gh stack submitWhen running
gh stack submitfor the first time on a new stack, the command would push branches and create PRs before discovering that stacked PRs aren't enabled for the repository — wasting time and creating orphaned PRs.Fix: Before pushing or creating PRs,
submitnow callsListStacks()to verify stacked PR support (skipped if a stack ID already exists locally). If stacks aren't available, the user is prompted interactively to create regular (unstacked) PRs instead. In non-interactive mode, the command exits with a new dedicated exit code (9). This preserves the workflow for users who use the CLI without access to stacked PRs.Resolves #22