2 Commits

Author	SHA1	Message	Date
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	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