skills/github__gh-stack
1
0
Fork 0
mirror of https://github.com/github/gh-stack.git synced 2026-09-14 20:26:28 +08:00
Code Issues Packages Projects Releases Wiki Activity
54 Commits 28 Branches 10 Tags
v0.0.6
Commit Graph

6 Commits

Author SHA1 Message Date
Sameen Karim 756002b4c9 ensure local trunk branch for required operations (#127) 
* Ensure trunk branch exists locally before commands that need it

When a user starts a stack after renaming their initial branch
(e.g. `git branch -m newbranch`), the trunk branch (e.g. main) may
not exist as a local branch. Commands that pass the trunk name to git
operations like merge-base, rebase, or rev-parse then fail with:

  fatal: Not a valid object name main

Add an `ensureLocalTrunk` helper that checks whether the trunk branch
exists locally and, if not, fetches it from the remote and creates a
local tracking branch. This mirrors the pattern already used in the
checkout command for importing stacks.

Commands updated:
- modify: call ensureLocalTrunk before the linearity check in
  CheckStackLinearity, which uses IsAncestor(trunk, branch). This was
  the originally reported failure.
- rebase: call ensureLocalTrunk after fetch and before fastForwardTrunk
  and the cascade rebase. git rebase requires a locally resolvable ref;
  the remote tracking ref alone is not sufficient.
- trunk: call ensureLocalTrunk before CheckoutBranch so that
  `gh stack trunk` works even when trunk was never created locally.
- checkout: refactor the existing inline BranchExists + CreateBranch
  block to use the shared helper.

Also fix an incorrect comment in fastForwardTrunk that claimed "the
remote tracking ref is sufficient for rebasing" — verified empirically
that `git rebase main` fails when main has no local branch, even after
fetching origin/main.

Commands that were already safe and required no changes:
- sync: fetches trunk explicitly and fastForwardTrunk guards with
  BranchExists
- push, switch, navigate, unstack: do not reference trunk
- add, submit: do not require trunk as a local git ref
- view: handles IsAncestor errors gracefully (false positive is
  acceptable since rebase will fix it)

* add check to avoid unnecessary remote selection prompt
 2026-06-15 13:54:20 -04:00
Sameen Karim 49a753708c insert branches with modify (#107) 
* add insert branch operation to modify TUI

Add `i` (insert below) and `I` (insert above) key bindings to the
interactive modify view, allowing users to insert new empty branches
into an existing stack. This follows Vim-inspired semantics where
lowercase `i` inserts below the cursor and uppercase `I` inserts above.

## TUI behavior

When the user presses `i` or `I`, the TUI enters an insert input mode
(similar to rename mode) where they type a new branch name. The input
is validated against git ref naming rules, local branch uniqueness, and
in-stack name collisions. On confirm, a placeholder node is inserted at
the correct position in the branch list with a green "✚ insert"
annotation badge and green connector styling.

Insert is a structure operation — it works alongside fold, rename, and
drop, but is mutually exclusive with reorder (consistent with existing
mode exclusivity rules). Undo (`z`) removes the inserted node cleanly.

## Apply engine

At apply time (Step 2 in the pipeline, between renames and folds), the
engine creates the new git branch at the parent branch's tip via
`git.CreateBranch` and inserts a `BranchRef` into the stack metadata at
the correct position. If the insertion changes the base of a branch
that has an open PR, `affectsPRs` is set to trigger a required
`gh stack submit` afterward.

## Header shortcut updates

- Combined the fold shortcuts into a single line: `d/u - fold down/up`
- Added insert shortcuts on their own line: `i/I - insert below/above`
- Reordered fold references throughout to list "down" before "up" for
  consistency with the insert shortcut ordering

## Files changed

- types.go: ActionInsertBelow/ActionInsertAbove types, IsInserted field,
  InsertedBranches in ApplyResult
- model.go: key bindings, insert input mode, undo, mode exclusivity,
  annotation, styling, header shortcuts, effective-index tracking to
  prevent false reorder detection when inserts shift node positions
- styles.go: green insert badge/branch/connector styles
- status.go: insert counting in pending change summary
- help.go: new "Insert below / above" section, reordered fold heading
- apply.go: BuildPlan and ApplyPlan handle insert actions
- modify.go: updated command description and success summary
- README.md: updated keybindings table

## Test coverage

- 16 new TUI tests: insert below/above, top/bottom edges, undo, mode
  exclusivity, merged branch guard, cancel/empty input, duplicate name
  validation, pending summary counting, annotation rendering, mixed
  operations with drop/fold, apply acceptance
- 4 new apply tests: BuildPlan produces correct insert actions,
  ApplyPlan creates branches and updates stack metadata, insert at
  stack start uses trunk as parent, affectsPRs triggered when inserting
  before a branch with an open PR

* update add error msg to direct users to modify for inserting branches

* docs updates

* fix insert branch bugs in modify TUI

Fix three bugs with the insert branch feature in the modify TUI, and
adjust rename behavior on inserted nodes.

## Bug 1: False "moved" annotations on existing branches

After inserting a branch, all branches below the insertion point
displayed "↕ moved 1 layer down" annotations. This happened because
`nodeAnnotation` and `toNodeData` compared each node's
`OriginalPosition` against its raw array index, which gets shifted
when an inserted node is added to the slice.

Fix: introduce an `effectiveIdx` parameter that counts only
non-inserted nodes, so position comparisons reflect the original
ordering. The View loop computes effective indices by incrementing
only for non-inserted nodes and passes them to the rendering
functions.

## Bug 2: Header branch count inflated by staged inserts

The branch count in the header ("N branches") included inserted
placeholder nodes, making it appear as though the stack had grown
before changes were applied.

Fix: `buildHeaderConfig` now excludes `IsInserted` nodes from the
branch count. The count reflects only the original branches in the
stack.

## Bug 3: Operations allowed on inserted placeholder nodes

Inserted nodes could be folded into other branches, which makes no
sense for a placeholder with no commits. Additionally, the "last
branch" guard counted inserted nodes as active, allowing users to
drop all original branches and bypass the empty-stack check.

Fix:
- `fold()` rejects inserted nodes with a descriptive error message.
- `toggleDrop()` on an inserted node removes it entirely and pops
  the original insert action from the undo stack (clean cancellation
  rather than a separate undo entry).
- All three "active branch" guards (`toggleDrop`, `fold`, `tryApply`)
  now exclude `IsInserted` nodes, ensuring at least one original
  branch always remains in the stack.

## Rename on inserted branches

Instead of blocking renames on inserted nodes, pressing `r` now
enters rename mode and updates the insert action's name in place.
The node's `Ref.Branch` and `PendingAction.NewName` are both updated
directly — no separate rename action is created in the undo stack.
This lets users fix a typo without having to drop and re-insert.

## Tests added

- `TestInsertDoesNotShowMovedAnnotation` — verifies no false move
  annotations appear on existing branches after an insert
- `TestBranchCountExcludesInserts` — verifies header count stays
  stable after insert
- `TestCannotFoldInsertedBranch` — verifies fold is blocked
- `TestCannotRenameInsertedBranch` — verifies rename updates the
  insert name in place
- `TestDropInsertedBranchRemovesIt` — verifies drop removes the node
- `TestDropInsertedBranchCanBeUndone` — verifies drop pops the
  original insert from the undo stack
- `TestCannotDropAllOriginalBranchesWithInsert` — verifies the
  empty-stack guard excludes inserted nodes

* ensure cannot fold into an inserted branch

* rm dead code

* delete inserted branches during abort
 2026-05-26 17:39:38 -04:00
Sameen Karim 9cc827dc78 modify: only require submit when changes affect PRs (#106) 
* modify: only require submit when changes affect PRs

Previously, `gh stack modify` always transitioned to `PhasePendingSubmit`
after completing on any stack with a remote ID (`s.ID != ""`). This blocked
the user from running another `modify` until they ran `gh stack submit`,
even when the modifications only touched local branches without PRs.

This was overly restrictive. If a user is working at the top of their stack
with branches that haven't been pushed or had PRs created yet, restructuring
those branches is a purely local operation — there is no remote state to
reconcile, and no reason to force a submit before allowing further modifies.

## What changed

The condition for entering `PhasePendingSubmit` is now
`s.ID != "" && affectsPRs` instead of just `s.ID != ""`.

A new `affectsPRs` flag is tracked throughout the apply process. It is set
to `true` when any of the following occurs:

- A **renamed** branch has a `PullRequest` ref
- A **folded** branch (source or target) has a `PullRequest` ref
- A **dropped** branch has a `PullRequest` ref
- A **rebased** branch (during cascading rebase) has a `PullRequest` ref

If none of these conditions are met, the modify state file is cleared
immediately — no pending-submit lock, no "run `gh stack submit`" prompt.

## Changes by file

**`internal/modify/state.go`**
- Added `AffectsPRs bool` field to `StateFile`. This persists the flag
  across conflict boundaries so that `ContinueApply` knows whether
  actions applied before the conflict already affected PR branches.

**`internal/modify/apply.go`**
- `ApplyPlan`: tracks `affectsPRs` through each step (rename, fold, drop,
  rebase). Saves the flag into conflict state when a conflict occurs.
  Uses `s.ID != "" && affectsPRs` for the pending-submit decision.
- `ContinueApply`: initializes `affectsPRs` from the saved state file,
  then checks the conflict branch and remaining branches for PRs during
  the cascading rebase. Uses the same combined condition.
- Both functions set `result.NeedsSubmit` / show the "run submit" message
  only when the flag is true.

**`internal/tui/modifyview/types.go`**
- Added `NeedsSubmit bool` to `ApplyResult` so the caller can use it
  for the success message.

**`cmd/modify.go`**
- `printModifySuccess` now takes its cue from `result.NeedsSubmit`
  instead of `s.ID != ""`. The "run `gh stack submit`" hint is only
  shown when PR branches were actually affected.

**`internal/modify/apply_test.go`**
- Updated `TestApplyPlan_PendingSubmitForRemoteStack` to use branches
  with PRs and trigger an actual rebase, validating the pending-submit
  path correctly.
- Added `TestApplyPlan_ClearsStateForRemoteStackWithNoPRBranches`:
  remote stack where no branches have PRs → state is cleared.
- Added `TestApplyPlan_PendingSubmitOnlyWhenPRBranchesAffected`:
  remote stack with a mix of PR and non-PR branches, only the non-PR
  branch is renamed → state is cleared, `NeedsSubmit` is false.

## Behavior summary

| Scenario | Before | After |
|---|---|---|
| Modify on local stack (no remote ID) | State cleared | State cleared (unchanged) |
| Modify on remote stack, PR branches affected | `PhasePendingSubmit` | `PhasePendingSubmit` (unchanged) |
| Modify on remote stack, only local branches affected | `PhasePendingSubmit` ❌ | State cleared ✅ |

The `CheckStateGuard` function (used by `add`, `push`, `sync`, `unstack`,
`rebase`) already did not block on `PhasePendingSubmit`, so those commands
are unaffected by this change.

* clear state after saving stack

* clarify submit requirement in description

* assign value directly
 2026-05-26 17:39:38 -04:00
Sameen Karim 8a9f1c86f8 commands help text (#88) 
* more help text for commands

* improved root help

* fix typo

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* updated examples for add cmd

Co-authored-by: Sameen Karim <skarim@github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-05-15 14:01:02 -04:00
Sameen Karim 8cddfd4e21 Optimize view/modify TUI load time with parallel fetching (#79) 
- Deduplicate API calls: syncStackPRs now returns PRDetails for
  LoadBranchNodes to reuse, eliminating redundant FindPRDetailsForBranch calls
- Parallelize API calls in syncStackPRs (capped at 6 concurrent requests)
- Parallelize git operations in LoadBranchNodes (capped at 4 concurrent)
- Show "Loading stack..." indicator for interactive sessions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-05-11 11:09:32 -04:00
Sameen Karim 7a268fc380 modify command (#72) 
* git primitives for modify cmd

* extract reusable TUI parts

* modify cmd

* recreate stack after modify

* add checks to prevent other modifications while modify is applying

* modify continue for resuming after resolving conflicts

* fix bug with duplicate stack entries after modifying

* reuse conflict resolution help msg from rebase

* additional confirmation before overwriting stack on remote

* fix recreate order of operations

Co-authored-by: Copilot <copilot@github.com>

* move base commit instead of cherry picking for fold up

* check to ensure we aren't left with zero branches

* unify and dedupe across view and modify tui

* more detailed help instructions

Co-authored-by: Copilot <copilot@github.com>

* only recommend submit if stack exists on remote

Co-authored-by: Copilot <copilot@github.com>

* tests for modify tui, apply modifications, submit modifications

* refactor submit for regular and pending modifications

* rename recover to abort

Co-authored-by: Copilot <copilot@github.com>

* docs for modify cmd

* tui styling updates

* updated tui screenshot

* addressing review comments

* Fix 4 bugs from code review

Bug 1: Move RevParseMap error check before using originalRefs.
The error from git.RevParseMap() was deferred past iteration of
originalRefs, which could panic on a nil map.

Bug 2: Differentiate cherry-pick vs rebase conflicts in modify.
Cherry-pick conflicts don't save state as 'conflict' phase, so
--continue won't work. Now prints --abort-only instructions for
cherry-pick conflicts.

Bug 3: Unwind now cleans up branches created by renames.
After restoring snapshot branches, Unwind deletes renamed branch
names that don't belong to the original snapshot.

Bug 4: Simplify push message in submit command.
Changed from 'Pushing N branches to remote...' to 'Pushing to
remote...' since individual branches may fail.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix 7 nit issues from code review

11: Add named constants for phase strings (PhaseApplying, PhaseConflict,
PhasePendingSubmit) in state.go; replace remaining raw literals in
state.go CheckStateGuard.

14: Fix bottomLines comment mismatch — listed 3 items but value is 2.

15: Extract magic number 88 to MinWidthForArt constant in header.go.

16: Remove unused stackview import anchor in model.go — the import
is used via types.go where BranchNode is embedded.

17: Simplify CheckStackLinearity parent resolution — ActiveBaseBranch
already handles skipping merged branches.

18: Fix rename undo matching any rename — add NewName check so only
the specific rename being undone is matched.

20: Add TestUndoRename and TestUndoRename_DoesNotAffectOtherRenames
to validate rename undo behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Make cherry-pick conflicts recoverable via --continue

Previously, cherry-pick conflicts during fold-down operations could only
be resolved with --abort. Now they save full conflict state (phase,
conflict type, fold branch/target, remaining branches) to the state file,
enabling recovery via 'gh stack modify --continue'.

Changes:
- Add ConflictType field to StateFile (rebase or cherry_pick)
- Add FoldBranch/FoldTarget fields for cherry-pick context
- Add CherryPickContinue to git package (cherry-pick --continue)
- Save cherry-pick conflict state in ApplyPlan with remaining branches
- ContinueApply handles both rebase and cherry-pick conflicts
- Unified conflict messaging in cmd/modify.go (both types show --continue)
- Updated test to verify cherry-pick conflict state is saved correctly

* Apply suggestions from code review

Co-authored-by: Luke Ghenco <lukeghenco@github.com>
Co-authored-by: Sameen Karim <skarim@github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Luke Ghenco <lukeghenco@github.com>
 2026-05-04 22:34:42 -04:00