Skip to content

feat(cli): handle merge queues#257

Merged
lararosekelley merged 3 commits into
mainfrom
feat/255-merge-queues-and-trains
Jun 30, 2026
Merged

feat(cli): handle merge queues#257
lararosekelley merged 3 commits into
mainfrom
feat/255-merge-queues-and-trains

Conversation

@lararosekelley

@lararosekelley lararosekelley commented Jun 30, 2026

Copy link
Copy Markdown
Owner

feat(cli): handle merge queues

Closes #255


Stack managed by git-stk

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

Claude finished @lararosekelley's task in 8m 37s —— View job


Code Review - Confidence: 75/100

Adds merge-queue / merge-train awareness to restack: frozen branches are skipped on rebase and push, with a reactive backstop in push_force_with_lease that downgrades a mid-run queue rejection to a warning.

  • Bug (CI Blocking)src/git.rs:159: cargo fmt --check fails — bail! macro needs line-wrapping. (inline)
  • Bug (CI Blocking)src/providers/github.rs:263: rustfmt wants a line break in the value.get(…).and_then(…) chain. (inline)
  • Bug (CI Blocking)src/providers/github.rs:285: rustfmt wants the &[…] slice on one line. (inline)
  • Bugsrc/stack/restack.rs:287: Freezing a middle branch (main → A → B(frozen)) still rebases and pushes A, invalidating B's queue entry. Freeze must propagate to every branch below a frozen one in the restack order. (inline)
  • Missing Coveragetests/restack.rs:515: No integration test for git stk continue through a conflict when frozen branches are present; the new RestackState.frozen serialization path is untested end-to-end. (inline)

The CI failure is three cargo fmt issues — trivial to fix with cargo fmt. The semantic bug (ancestor branches of a frozen node not being frozen) is the most important finding: it silently leaves the queue entry invalid when any non-bottom branch is frozen. The merge_queue_rejection heuristic and the parse_merge_queue_entry JSON path are both well-targeted and the unit tests for them are solid.

Comment thread src/git.rs Outdated
Comment thread src/providers/github.rs
Comment thread src/providers/github.rs Outdated
Comment thread src/stack/restack.rs
Comment thread tests/restack.rs
@lararosekelley

Copy link
Copy Markdown
Owner Author

@bot review

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

Claude finished @lararosekelley's task in 5m 45s —— View job


Code Review — Confidence: 82/100 (+7 vs prior review)

Adds merge-queue / merge-train awareness to restack: frozen branches are skipped on rebase and push, with ancestor-freeze propagation (with_frozen_ancestors) and a reactive backstop in push_tolerating_merge_queue for mid-run queue enrollments.


  • Gather context and read changed files
  • Review src/git.rs changes
  • Review src/providers/github.rs changes
  • Review src/providers/gitlab.rs changes
  • Review src/providers/mod.rs changes
  • Review src/stack/restack.rs changes
  • Review tests/restack.rs changes
  • Summarize findings

  • Output Regressionsrc/stack/restack.rs:415: When the push_tolerating_merge_queue backstop fires (a branch enrolled mid-run), it prints a "was not updated" warning then returns Ok(()). finish_restack then prints "pushed feature/a feature/b to origin" for every branch in pushable — including the one just warned as held back. The user sees contradictory output. Fix: change push_force_with_lease to return the held-back set and subtract it before printing. (inline)

  • Missing Coveragetests/restack.rs:570: No integration test for git stk continue when frozen branches are in the saved state. The frozen=\t-joined serialization and strip_prefix("frozen=") deserialization are untested through a conflict → continue flow. If deserialization returned an empty set silently, continue_restack would rebase and push queued branches with nothing to catch it. (inline)


Fixes from prior review confirmed: the ancestor-freeze bug is resolved — with_frozen_ancestors correctly propagates the frozen set to every stack-parent of a queued branch, and the new restack_freezing_a_branch_also_freezes_everything_below_it integration test covers the key scenario. The merge_queue_rejection heuristic, parse_merge_queue_entry JSON path, and active_train_source_branches parsing are all well-targeted and well-tested. The proactive GitLab merge-train check via glab api --paginate is correctly scoped (returns empty on 404, appropriate for a Premium-only feature).

Comment thread src/stack/restack.rs Outdated
Comment thread tests/restack.rs
@lararosekelley

Copy link
Copy Markdown
Owner Author

@bot review

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

Claude finished @lararosekelley's task in 6m 8s —— View job


Code Review — Confidence: 90/100 (+8 vs prior review)

Adds merge-queue / merge-train awareness to restack: frozen branches are skipped on rebase and push, ancestor-freeze propagates via with_frozen_ancestors, and push_force_with_lease returns only the branches that actually landed so finish_restack never reports a branch as both held and pushed.


  • Gather context and read changed files
  • Review src/git.rs changes
  • Review src/providers/github.rs changes
  • Review src/providers/gitlab.rs changes
  • Review src/providers/mod.rs changes
  • Review src/stack/restack.rs changes
  • Review tests/restack.rs changes
  • Check CI / cargo fmt / cargo clippy status
  • Summarize findings

Fixes from prior reviews confirmed:

  • cargo fmt — all three rustfmt failures from the first review are resolved.
  • ✅ Ancestor-freeze bug — with_frozen_ancestors correctly walks the parent chain and freezes every stack-ancestor of a queued branch, including the multi-queued-branch case where the short-circuit !frozen.insert(parent) correctly stops traversal at a shared ancestor.
  • ✅ Output regression — push_force_with_lease now returns Vec<String> (the branches that actually landed). finish_restack uses the returned pushed vec rather than pushable, so the "pushed … to origin" line never names a branch that was warned as held back by the mid-run backstop.
  • continue serialization round-trip — continue_preserves_frozen_branches_read_back_from_the_state_file exercises the full frozen=\t-joined write → state file → strip_prefix("frozen=") read path. The final assert_eq!(repo.remote_sha(&bare, "feature/a"), frozen_a) / assert_eq!(repo.remote_sha(&bare, "feature/b"), frozen_b) checks serve as the effective oracle: if deserialization silently returned an empty set, the continue would push those branches and the SHA assertions would fail.

No new actionable findings. The merge_queue_rejection heuristic is well-scoped: the three negative markers (stale info, non-fast-forward, fetch first) prevent a stale-lease failure from being downgraded, and the unit tests cover the exact GitHub rejection shape plus a "queue mention in an unrelated banner" false-positive. The with_frozen_ancestors short-circuit (!frozen.insert(parent)) is correct for both the single-queued-branch and multiple-queued-branch cases. push_force_with_lease is the only caller of the new return type; no partial migration. CI is in-progress on the latest push.


@lararosekelley lararosekelley merged commit 48a6cc1 into main Jun 30, 2026
13 checks passed
@lararosekelley lararosekelley deleted the feat/255-merge-queues-and-trains branch June 30, 2026 21:34
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.

Handle merge queues / merge trains: freeze queued branches in restack & sync (GH006 hard-fail)

1 participant