Files
github__gh-stack/internal/modify/state.go
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

144 lines
4.9 KiB
Go

package modify
import (
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
"time"
)
const stateFileName = "gh-stack-modify-state"
const (
PhaseApplying = "applying"
PhaseConflict = "conflict"
PhasePendingSubmit = "pending_submit"
)
// StateFile holds the state of an in-progress or pending-submit modify operation.
// It is stored at .git/gh-stack-modify-state.
type StateFile struct {
SchemaVersion int `json:"schema_version"`
StackName string `json:"stack_name"`
StackIndex int `json:"stack_index"` // index in StackFile.Stacks at modify start
StartedAt time.Time `json:"started_at"`
Phase string `json:"phase"` // "applying", "conflict", or "pending_submit"
PriorRemoteStackID string `json:"prior_remote_stack_id,omitempty"`
Snapshot Snapshot `json:"snapshot"`
Plan []Action `json:"plan"`
// Conflict state — populated when phase is "conflict"
ConflictBranch string `json:"conflict_branch,omitempty"`
ConflictType string `json:"conflict_type,omitempty"` // "rebase" or "cherry_pick"
RemainingBranches []string `json:"remaining_branches,omitempty"`
OriginalBranch string `json:"original_branch,omitempty"`
OriginalRefs map[string]string `json:"original_refs,omitempty"`
// Cherry-pick conflict context — which fold was in progress
FoldBranch string `json:"fold_branch,omitempty"` // branch being folded
FoldTarget string `json:"fold_target,omitempty"` // branch receiving the cherry-pick
// AffectsPRs records whether any action so far has affected a branch with
// a PR. Persisted across conflict boundaries so ContinueApply can combine
// it with checks on remaining branches.
AffectsPRs bool `json:"affects_prs,omitempty"`
}
// Snapshot captures the pre-modify state for unwind/recovery.
type Snapshot struct {
Branches []BranchSnapshot `json:"branches"`
StackMetadata json.RawMessage `json:"stack_metadata"`
}
// BranchSnapshot stores the state of a single branch before modification.
type BranchSnapshot struct {
Name string `json:"name"`
TipSHA string `json:"tip_sha"`
Position int `json:"position"`
}
// Action represents a single staged action from the TUI.
type Action struct {
Type string `json:"type"` // "drop", "fold_down", "fold_up", "move", "rename"
Branch string `json:"branch"`
NewPosition int `json:"new_position,omitempty"` // for "move"
NewName string `json:"new_name,omitempty"` // for "rename"
}
// StatePath returns the full path to the modify state file.
func StatePath(gitDir string) string {
return filepath.Join(gitDir, stateFileName)
}
// LoadState reads the modify state file from the git directory.
// Returns nil, nil if the file does not exist.
func LoadState(gitDir string) (*StateFile, error) {
data, err := os.ReadFile(StatePath(gitDir))
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return nil, nil
}
return nil, fmt.Errorf("reading modify state: %w", err)
}
var state StateFile
if err := json.Unmarshal(data, &state); err != nil {
return nil, fmt.Errorf("parsing modify state: %w", err)
}
return &state, nil
}
// SaveState writes the modify state file atomically (write to temp, then rename).
func SaveState(gitDir string, state *StateFile) error {
data, err := json.MarshalIndent(state, "", " ")
if err != nil {
return fmt.Errorf("marshaling modify state: %w", err)
}
target := StatePath(gitDir)
tmp := target + ".tmp"
if err := os.WriteFile(tmp, data, 0644); err != nil {
return fmt.Errorf("writing modify state: %w", err)
}
// Remove existing target before rename for Windows compatibility
// (os.Rename fails on Windows if the target already exists).
_ = os.Remove(target)
if err := os.Rename(tmp, target); err != nil {
_ = os.Remove(tmp)
return fmt.Errorf("committing modify state: %w", err)
}
return nil
}
// ClearState removes the modify state file.
func ClearState(gitDir string) {
_ = os.Remove(StatePath(gitDir))
}
// StateExists returns true if a modify state file exists.
func StateExists(gitDir string) bool {
_, err := os.Stat(StatePath(gitDir))
return err == nil
}
// CheckStateGuard checks if a modify state file exists with phase "applying"
// and returns an error if so. This is used as a guard at the top of commands that
// should not run while a modify is in progress.
func CheckStateGuard(gitDir string) error {
state, err := LoadState(gitDir)
if err != nil {
return nil // ignore read errors
}
if state == nil {
return nil
}
if state.Phase == PhaseApplying {
return fmt.Errorf("a modify session was interrupted — run `gh stack modify --abort` to restore your stack")
}
if state.Phase == PhaseConflict {
return fmt.Errorf("a modify has unresolved conflicts — run `gh stack modify --continue` or `gh stack modify --abort`")
}
return nil
}