feat(submit): register stacks on GitHub after submit - #22
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds GitHub Stack REST API integration, linear branch-segment detection, remote stack reconciliation, persisted stack numbers, and synchronization calls across submission flows after successful non-dry-run pushes. ChangesGitHub Stack Synchronization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SubmitFlow
participant pushBranch
participant syncGitHubStacks
participant GitHub
SubmitFlow->>pushBranch: push branch
pushBranch-->>SubmitFlow: record successful non-dry-run branch
SubmitFlow->>syncGitHubStacks: synchronize pushed branches
syncGitHubStacks->>GitHub: reconcile GitHub stacks
GitHub-->>syncGitHubStacks: stack response or error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/engine/ghstack.go (1)
241-283: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
existingsilently picks the first non-zeroStackNumberin a segment.If segment membership changes across submits (e.g., after a local fold/reorder), branches within one segment could carry different recorded
StackNumbers, but only the first (bottommost) non-zero value is used asexisting; any others are discarded without comment. In practice this is caught downstream byreconcileStack's divergence check (a mismatched remote top routes toresolveDivergedStack), so it's not a silent-corruption risk — but the resulting divergence warning won't mention that the local segment itself already disagreed about stack membership, making it harder to diagnose.Consider detecting and surfacing this case explicitly for a clearer warning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/engine/ghstack.go` around lines 241 - 283, Detect conflicting non-zero StackNumber values while iterating branches in syncGitHubStacks, rather than silently retaining only the first value in existing. Surface a clear warning that the local segment has inconsistent stack membership, including the affected branch sequence, and preserve the current reconciliation flow for segments without conflicting values.internal/engine/ghstack_test.go (1)
1-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLGTM! Segmentation tests are thorough and correctly cover linear runs, forks, partial submits, and unsubmitted siblings.
Separately, worth noting
reconcileStack/resolveDivergedStack(the merged-PR suffix matching and divergence detection) have no test coverage sinceghStackAPIshells out directly togh. Extracting it behind a small function-variable seam would let that logic be unit tested without a realghbinary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/engine/ghstack_test.go` around lines 1 - 141, Add a small function-variable seam around the ghstack API’s external gh invocation, then use it from reconcileStack and resolveDivergedStack instead of shelling out directly. Add unit tests through the seam covering merged-PR suffix matching and divergence detection, without requiring a real gh binary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/engine/ghstack.go`:
- Around line 150-155: Explicitly suppress the unused-symbol lint warning for
ghUnstack while leaving its current implementation unchanged. Use the
repository’s established Go lint-suppression convention directly on ghUnstack,
since resolveDivergedStack is not yet wired to call it.
- Around line 24-44: Update reconcileStack to treat an existing remote GHStack
as dissolved when either remote.Open is false or remote.PullRequests is empty,
and follow the existing dissolved-stack restart path instead of POSTing to
/stacks/{number}/add. Preserve current reconciliation behavior for open stacks
containing pull requests.
---
Nitpick comments:
In `@internal/engine/ghstack_test.go`:
- Around line 1-141: Add a small function-variable seam around the ghstack API’s
external gh invocation, then use it from reconcileStack and resolveDivergedStack
instead of shelling out directly. Add unit tests through the seam covering
merged-PR suffix matching and divergence detection, without requiring a real gh
binary.
In `@internal/engine/ghstack.go`:
- Around line 241-283: Detect conflicting non-zero StackNumber values while
iterating branches in syncGitHubStacks, rather than silently retaining only the
first value in existing. Surface a clear warning that the local segment has
inconsistent stack membership, including the affected branch sequence, and
preserve the current reconciliation flow for segments without conflicting
values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f59722e-e1ed-4fa4-a72f-75125c55cf8f
📒 Files selected for processing (4)
internal/engine/ghstack.gointernal/engine/ghstack_test.gointernal/engine/submit.gointernal/store/pr_info.go
A stack whose PRs have all merged is not deleted — it stays queryable with open:false and an emptied member list. The nil check only caught an explicit 404, so a finished stack fell through to POST /add against a closed stack instead of starting a fresh one. Extracts stackNeedsRebuild and prsAboveTop as pure functions so the reconciliation decision is testable without the network, and names GHStackPR instead of an inline anonymous struct. Found by CodeRabbit on #22.
A stack whose PRs have all merged is not deleted — it stays queryable with open:false and an emptied member list. The nil check only caught an explicit 404, so a finished stack fell through to POST /add against a closed stack instead of starting a fresh one. Extracts stackNeedsRebuild and prsAboveTop as pure functions so the reconciliation decision is testable without the network, and names GHStackPR instead of an inline anonymous struct. Found by CodeRabbit on #22.
A stack whose PRs have all merged is not deleted — it stays queryable with open:false and an emptied member list. The nil check only caught an explicit 404, so a finished stack fell through to POST /add against a closed stack instead of starting a fresh one. Extracts stackNeedsRebuild and prsAboveTop as pure functions so the reconciliation decision is testable without the network, and names GHStackPR instead of an inline anonymous struct. Found by CodeRabbit on #22.
…nserted below the current bottom Two bugs compounded to leave a restacked PR pointed at the wrong base indefinitely, confirmed against the actual amustafa/stackr repo (PR #22 stayed based on main after am/sync-worktree-cleanup was inserted below it): 1. reconcileStack/prsAboveTop only ever checked whether remote's *top* PR appeared somewhere in the local segment, then appended whatever sat above it. It never verified the full remote sequence actually lined up, so a PR inserted *below* the stack's current bottom (e.g. a restack onto a new base branch) was indistinguishable from "nothing changed" — the old top was still there, so reconcileStack returned early and never touched GitHub. Replaced with classifyAgainstRemote, which locates remote's whole PR list as a contiguous run and reports what's below and above it separately. 2. Nothing ever synced an existing PR's base ref to GitHub after the local parent changed — pushBranch only force-pushed commits and updated stackr's own local cache. Added ghUpdatePRBase (a raw REST PATCH; `gh pr edit --base` fails outright via an unrelated GraphQL project-cards deprecation) and call it from pushBranch whenever the recorded base drifts from the local parent. Those two don't compose for free: GitHub refuses to retarget a PR's base while it's grouped into a stack, and refuses to create a stack containing a PR that's already grouped into another one. So when classifyAgainstRemote's newBelow PRs are still open (a real insertion, not just GitHub having already dropped a merged one), reconcileStack now unstacks the old group, retries the base retarget for every PR in the chain, and only then rebuilds.
Adds the GitHub stacked-PRs REST integration (public preview, X-GitHub-Api-Version 2026-03-10). stackr already creates each PR with --base set to its parent branch, which is exactly the chain GitHub validates, so registering a stack is one API call after submit. - ghstack.go: create/get/add/unstack via 'gh api --input -' - linearSegments: decompose stackr's branch tree into the maximal linear runs GitHub's model requires, cutting at every fork so each child starts a fresh stack based on the fork point - syncGitHubStacks: best-effort, runs on every submit path, scoped to what was actually pushed; a repo without the preview warns instead of failing a submit whose PRs already exist - BranchPR.StackNumber persists the stack so later submits extend it The reconciliation policy for a stack that has diverged remotely is left as a documented TODO in resolveDivergedStack.
A stack whose PRs have all merged is not deleted — it stays queryable with open:false and an emptied member list. The nil check only caught an explicit 404, so a finished stack fell through to POST /add against a closed stack instead of starting a fresh one. Extracts stackNeedsRebuild and prsAboveTop as pure functions so the reconciliation decision is testable without the network, and names GHStackPR instead of an inline anonymous struct. Found by CodeRabbit on #22.
…nserted below the current bottom Two bugs compounded to leave a restacked PR pointed at the wrong base indefinitely, confirmed against the actual amustafa/stackr repo (PR #22 stayed based on main after am/sync-worktree-cleanup was inserted below it): 1. reconcileStack/prsAboveTop only ever checked whether remote's *top* PR appeared somewhere in the local segment, then appended whatever sat above it. It never verified the full remote sequence actually lined up, so a PR inserted *below* the stack's current bottom (e.g. a restack onto a new base branch) was indistinguishable from "nothing changed" — the old top was still there, so reconcileStack returned early and never touched GitHub. Replaced with classifyAgainstRemote, which locates remote's whole PR list as a contiguous run and reports what's below and above it separately. 2. Nothing ever synced an existing PR's base ref to GitHub after the local parent changed — pushBranch only force-pushed commits and updated stackr's own local cache. Added ghUpdatePRBase (a raw REST PATCH; `gh pr edit --base` fails outright via an unrelated GraphQL project-cards deprecation) and call it from pushBranch whenever the recorded base drifts from the local parent. Those two don't compose for free: GitHub refuses to retarget a PR's base while it's grouped into a stack, and refuses to create a stack containing a PR that's already grouped into another one. So when classifyAgainstRemote's newBelow PRs are still open (a real insertion, not just GitHub having already dropped a merged one), reconcileStack now unstacks the old group, retries the base retarget for every PR in the chain, and only then rebuilds.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
internal/engine/ghstack.go (5)
224-231: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueBound the extension loop with
claimed.The inner loop follows child links without checking
claimed. The segmentation logic is correct for a tree, but a cycle in the persisted graph makes this loop append forever and growsegmentwithout bound. The graph is read from a state file, so a corrupted parent/child pair is reachable. Add the guard to keep the loop bounded.🛡️ Proposed guard
for { kids := childrenInSet(segment[len(segment)-1]) - if len(kids) != 1 { + if len(kids) != 1 || claimed[kids[0]] { break } segment = append(segment, kids[0]) claimed[kids[0]] = true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/engine/ghstack.go` around lines 224 - 231, Update the inner extension loop in the segment-building logic to stop when the next child is already present in claimed, before appending it. Preserve the existing single-child condition and mark each newly appended child in claimed so cycles in the persisted graph cannot grow segment indefinitely.
273-277: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRoute the warning through the same output policy as the success line.
The success message at Line 286 respects
quiet. This warning does not. The same applies to the warnings at Line 427 inreconcileStack. Ifquietexists to keep scripted submits clean, these lines break that contract; if warnings are intentionally always visible, the success line is the inconsistent one. Pick one policy and apply it to both.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/engine/ghstack.go` around lines 273 - 277, Align the warning output in the stack sync flow with the existing quiet-aware success output, using the same output policy for both. Apply that policy consistently to the warning messages in the surrounding sync logic and the warnings in reconcileStack, preserving the current message content and error handling.
350-367: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA single failed PR lookup aborts reconciliation.
anyPROpenreturns on the first error. If one PR innewBelowis inaccessible or deleted,gh apireturns 404,reconcileStackpropagates the error, andsyncGitHubStacksprints a warning without recording anything. A PR that GitHub cannot find is not open, so the safer reading is to skip it and keep checking.The calls are also sequential with a 15s timeout each.
newBelowis normally short, so this is acceptable today, but consider a note or a bound if segments can grow.♻️ Proposed change
for _, n := range prNumbers { out, err := ghStackAPI("GET", fmt.Sprintf("repos/{owner}/{repo}/pulls/%d", n), nil) if err != nil { - return false, err + // A PR we cannot read is not an open PR blocking the rebuild. + if strings.Contains(err.Error(), "404") || strings.Contains(err.Error(), "Not Found") { + continue + } + return false, err }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/engine/ghstack.go` around lines 350 - 367, Update anyPROpen to skip inaccessible or deleted PR lookups instead of returning the first ghStackAPI error, treating those PRs as not open and continuing through prNumbers. Preserve JSON parsing errors and successful open-state detection, and consider documenting or bounding the sequential lookup behavior if segment sizes may grow.
457-470: 📐 Maintainability & Code Quality | 🔵 Trivial
resolveDivergedStackreturns an error on every submit for a diverged stack.The
TODO(implement)leaves this path permanently failing.syncGitHubStacksdowngrades the error to a warning, so submits still succeed, but the user sees the same message on every submit and has no way to clear it except editing the state file. The PR description acknowledges this.Do you want me to open a tracking issue for the reconciliation policy, or draft the rebuild variant that clears the recorded
StackNumberwhenghCreateStackfails afterghUnstack?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/engine/ghstack.go` around lines 457 - 470, The unimplemented resolveDivergedStack path must reconcile divergent stacks instead of returning an error on every submit. Implement the chosen reconciliation policy in resolveDivergedStack, including clearing the recorded stack number if unstacking succeeds but ghCreateStack fails, and preserve correct state for successful rebuilds; use the existing submit flow and GHStack helpers rather than leaving the TODO error path.
96-102: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInclude the exit error and make status detection explicit.
The error message contains only stderr. Two consequences:
- If
ghwrites nothing to stderr, the message ends withfailed:and no cause remains.ghGetStackdecides "stack no longer exists" by substring matching404orNot Foundon this message (Line 127). That couples control flow togh's human-readable stderr text.Wrap the underlying error and return a typed status so callers do not parse prose.
♻️ Proposed change
+// ghAPIError carries the failed invocation so callers can branch on status +// instead of matching stderr text. +type ghAPIError struct { + Method, Path string + Stderr string + Err error +} + +func (e *ghAPIError) Error() string { + return fmt.Sprintf("gh api %s %s failed: %s", e.Method, e.Path, e.Stderr) +} + +func (e *ghAPIError) Unwrap() error { return e.Err } + +func (e *ghAPIError) notFound() bool { + return strings.Contains(e.Stderr, "HTTP 404") +}if err := cmd.Run(); err != nil { if ctx.Err() != nil { return nil, fmt.Errorf("gh api %s timed out after %s", path, ghTimeout) } - return nil, fmt.Errorf("gh api %s %s failed: %s", method, path, strings.TrimSpace(stderr.String())) + return nil, &ghAPIError{ + Method: method, Path: path, + Stderr: strings.TrimSpace(stderr.String()), + Err: err, + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/engine/ghstack.go` around lines 96 - 102, Update the command execution error path in ghGetStack’s underlying gh API helper to preserve the cmd.Run exit error and expose an explicit typed status for callers, rather than relying on formatted stderr text. Ensure the returned error retains stderr context while wrapping the underlying error, and update ghGetStack’s “stack no longer exists” handling to inspect the typed status instead of matching “404” or “Not Found” in error prose.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/engine/ghstack.go`:
- Around line 249-272: Update the segment scan around existing and
reconcileStack to collect all distinct non-zero StackNumber values instead of
retaining only the first one. When a segment spans multiple recorded stacks,
route it through the rebuild path by passing the designated multi-stack sentinel
(existing < 0), and update reconcileStack to dissolve every recorded stack
before creating the unified stack. Preserve the existing behavior for segments
belonging to zero or one recorded stack.
- Around line 291-303: Move the opening paragraph describing reconcileStack from
the comment above stackNeedsRebuild to the comment immediately preceding
reconcileStack. Keep the stackNeedsRebuild-specific explanation attached to
stackNeedsRebuild, and leave the baseByPR note with the reconcileStack
documentation as appropriate.
- Around line 381-383: Update the stackNeedsRebuild branch in the stack handling
flow to dissolve or unstack the existing remote stack before calling
ghCreateStack(prNumbers). Match the cleanup behavior used by the newBelow path,
then preserve creation of the replacement stack and registration flow.
---
Nitpick comments:
In `@internal/engine/ghstack.go`:
- Around line 224-231: Update the inner extension loop in the segment-building
logic to stop when the next child is already present in claimed, before
appending it. Preserve the existing single-child condition and mark each newly
appended child in claimed so cycles in the persisted graph cannot grow segment
indefinitely.
- Around line 273-277: Align the warning output in the stack sync flow with the
existing quiet-aware success output, using the same output policy for both.
Apply that policy consistently to the warning messages in the surrounding sync
logic and the warnings in reconcileStack, preserving the current message content
and error handling.
- Around line 350-367: Update anyPROpen to skip inaccessible or deleted PR
lookups instead of returning the first ghStackAPI error, treating those PRs as
not open and continuing through prNumbers. Preserve JSON parsing errors and
successful open-state detection, and consider documenting or bounding the
sequential lookup behavior if segment sizes may grow.
- Around line 457-470: The unimplemented resolveDivergedStack path must
reconcile divergent stacks instead of returning an error on every submit.
Implement the chosen reconciliation policy in resolveDivergedStack, including
clearing the recorded stack number if unstacking succeeds but ghCreateStack
fails, and preserve correct state for successful rebuilds; use the existing
submit flow and GHStack helpers rather than leaving the TODO error path.
- Around line 96-102: Update the command execution error path in ghGetStack’s
underlying gh API helper to preserve the cmd.Run exit error and expose an
explicit typed status for callers, rather than relying on formatted stderr text.
Ensure the returned error retains stderr context while wrapping the underlying
error, and update ghGetStack’s “stack no longer exists” handling to inspect the
typed status instead of matching “404” or “Not Found” in error prose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ace0f69c-63a6-4d53-88ff-eccb896f89fa
📒 Files selected for processing (5)
internal/engine/ghstack.gointernal/engine/ghstack_test.gointernal/engine/github.gointernal/engine/submit.gointernal/store/pr_info.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/engine/ghstack_test.go
- internal/store/pr_info.go
- internal/engine/submit.go
Addresses three CodeRabbit findings on #22 that share one root cause: stackr created a GitHub stack without first dissolving the stacks its PRs already belonged to, and GitHub allows a PR in only one stack. - The segment scan kept only the first non-zero StackNumber, so a segment spanning two recorded stacks picked one and failed /add forever. - The stackNeedsRebuild path created without unstacking, but a closed stack retains its members until explicitly unstacked. - reconcileStack's doc comment was attached to stackNeedsRebuild.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/engine/ghstack_test.go (1)
228-238: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the complete
mapSegmentresult.
TestMapSegment_CollectsEveryRecordedStackchecks onlylen(seg.prNumbers).TestMapSegment_SkipsBranchesWithoutAPRchecks onlyseg.branches. A wrong PR-number mapping can pass both tests.Assert
seg.prNumbersequals[]int{42, 43, 44}and[]int{42, 44}. Also assertbaseByPR[44] == "b"in the first test.Also applies to: 248-256
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/engine/ghstack_test.go` around lines 228 - 238, Strengthen the mapSegment assertions in TestMapSegment_CollectsEveryRecordedStack and TestMapSegment_SkipsBranchesWithoutAPR: compare seg.prNumbers against the exact expected slices []int{42, 43, 44} and []int{42, 44}, respectively, rather than checking only their lengths. In the first test, also assert that seg.baseByPR[44] equals "b".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/engine/ghstack_test.go`:
- Around line 284-298: Tighten isMissingStack so it recognizes only ghStackAPI
errors matching the failed unstack command shape and a 404/Not Found response,
rather than any message containing those strings. Preserve rejection of
unrelated 404 output and existing non-404 cases, and extend TestIsMissingStack
with coverage for an unrelated 404 response.
---
Nitpick comments:
In `@internal/engine/ghstack_test.go`:
- Around line 228-238: Strengthen the mapSegment assertions in
TestMapSegment_CollectsEveryRecordedStack and
TestMapSegment_SkipsBranchesWithoutAPR: compare seg.prNumbers against the exact
expected slices []int{42, 43, 44} and []int{42, 44}, respectively, rather than
checking only their lengths. In the first test, also assert that
seg.baseByPR[44] equals "b".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6814d6a8-b6e8-453d-a44a-b78c71280977
📒 Files selected for processing (2)
internal/engine/ghstack.gointernal/engine/ghstack_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/engine/ghstack.go
Integrates with GitHub stacked pull requests, which entered public preview on 2026-07-30.
Why this is small
GitHub's stack is a first-class server-side object, not just a UI convention over chained base branches. Creating one requires that each PR's base ref equals the previous PR's head ref — otherwise the API returns 422.
stackr already guarantees exactly that:
ghCreatePRpasses--base b.ParentBranchName. So registering a stack is one API call after submit, not a restructuring.What's here
internal/engine/ghstack.go—ghCreateStack/ghGetStack/ghAddToStack/ghUnstack, overgh api --input -. JSON on stdin keeps array ordering exact and avoidsgh's field-flag syntax;ghexpands the{owner}/{repo}placeholders itself, so stackr never resolves the remote. Pinned toX-GitHub-Api-Version: 2026-03-10.linearSegments— stackr's graph is a tree; a GitHub stack is strictly linear and a PR may belong to only one stack. The two don't map one-to-one, so forks are cut and each child starts a fresh run based on the fork point:[c]and[d]are single PRs, so they stay unregistered until they grow (GitHub requires two or more PRs per stack). Their base refs already point atbeither way. The trade-off — GitHub can't show a relationship between[a b]and what sits aboveb— is documented on the function.syncGitHubStacks— runs on every submit path, scoped to what was actually pushed. Best-effort by design, matchingghMergedHeadBranches: the PRs already exist by the time it runs, so a repo without the preview, an offline machine, or an olderghwarns rather than failing the submit. Dry runs contribute nothing.store.BranchPR.StackNumber— persists the stack so a later submit extends it instead of duplicating.cloneBranchPRdoes a struct copy, somerge.gocarries it for free.Deliberately left open
resolveDivergedStackhandles the case where the recorded stack's top PR isn't in the segment at all — aftersr reorder/sr move/sr fold, or a manual regroup in the web UI. It currently returns an error, with the trade-off written up: rebuilding (unstack+create) mirrors local but destroys remote intent and isn't atomic; warning and leaving it never destroys anything but lets GitHub drift.Testing
7 tests over
linearSegments: linear stacks, fork-cutting, forks that extend upward, partial submits, and an unsubmitted sibling correctly not counting as a fork.Not verified against live GitHub — endpoint shapes come from the docs. This PR's own submit is the first real exercise.
Summary by CodeRabbit
New Features
Bug Fixes