mirror of
https://github.com/github/gh-stack.git
synced 2026-09-14 20:26:28 +08:00
9cc827dc78
* 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
144 lines
4.9 KiB
Go
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
|
|
}
|