modify: only require submit when changes affect PRs#106
Draft
skarim wants to merge 1 commit into
Draft
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously,
gh stack modifyalways transitioned toPhasePendingSubmitafter completing on any stack with a remote ID (
s.ID != ""). This blockedthe user from running another
modifyuntil they rangh 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
PhasePendingSubmitis nows.ID != "" && affectsPRsinstead of justs.ID != "".A new
affectsPRsflag is tracked throughout the apply process. It is setto
truewhen any of the following occurs:PullRequestrefPullRequestrefPullRequestrefPullRequestrefIf 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.goAffectsPRs boolfield toStateFile. This persists the flagacross conflict boundaries so that
ContinueApplyknows whetheractions applied before the conflict already affected PR branches.
internal/modify/apply.goApplyPlan: tracksaffectsPRsthrough each step (rename, fold, drop,rebase). Saves the flag into conflict state when a conflict occurs.
Uses
s.ID != "" && affectsPRsfor the pending-submit decision.ContinueApply: initializesaffectsPRsfrom the saved state file,then checks the conflict branch and remaining branches for PRs during
the cascading rebase. Uses the same combined condition.
result.NeedsSubmit/ show the "run submit" messageonly when the flag is true.
internal/tui/modifyview/types.goNeedsSubmit booltoApplyResultso the caller can use itfor the success message.
cmd/modify.goprintModifySuccessnow takes its cue fromresult.NeedsSubmitinstead of
s.ID != "". The "rungh stack submit" hint is onlyshown when PR branches were actually affected.
internal/modify/apply_test.goTestApplyPlan_PendingSubmitForRemoteStackto use brancheswith PRs and trigger an actual rebase, validating the pending-submit
path correctly.
TestApplyPlan_ClearsStateForRemoteStackWithNoPRBranches:remote stack where no branches have PRs → state is cleared.
TestApplyPlan_PendingSubmitOnlyWhenPRBranchesAffected:remote stack with a mix of PR and non-PR branches, only the non-PR
branch is renamed → state is cleared,
NeedsSubmitis false.Behavior summary
PhasePendingSubmitPhasePendingSubmit(unchanged)PhasePendingSubmit❌The
CheckStateGuardfunction (used byadd,push,sync,unstack,rebase) already did not block onPhasePendingSubmit, so those commandsare unaffected by this change.
Stack created with GitHub Stacks CLI • Give Feedback 💬