Skip to content

modify: only require submit when changes affect PRs#106

Draft
skarim wants to merge 1 commit into
skarim/switch-after-modifyfrom
skarim/modify-only-block-pr-changes
Draft

modify: only require submit when changes affect PRs#106
skarim wants to merge 1 commit into
skarim/switch-after-modifyfrom
skarim/modify-only-block-pr-changes

Conversation

@skarim
Copy link
Copy Markdown
Collaborator

@skarim skarim commented May 22, 2026

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.


Stack created with GitHub Stacks CLIGive Feedback 💬

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.
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.

1 participant