submit: Refuse to overwrite commits pushed to a branch by someone else

.changes/unreleased/Fixed-20260620-071248.yaml

@@ -0,0 +1,3 @@
+kind: Fixed
+body: 'submit: Refuse to overwrite commits added to a branch by someone else since git-spice last pushed it; the offending commits are listed. Run ''gs branch sync'' to integrate them, or use --force to overwrite.'
+time: 2026-06-20T07:12:48.071369-04:00

branch_sync.go

@@ -7,12 +7,18 @@ import (
 	"go.abhg.dev/gs/internal/git"
 	"go.abhg.dev/gs/internal/handler/branchsync"
 	"go.abhg.dev/gs/internal/silog"
+	"go.abhg.dev/gs/internal/spice"
 	"go.abhg.dev/gs/internal/text"
 )
 
 type branchSyncCmd struct {
 	Branch string `placeholder:"NAME" help:"Branch to sync" predictor:"trackedBranches"`
 	Rebase bool   `help:"On divergence (both local and remote have new commits since the last push), replay remote-side commits onto local. Conflicts surface as a normal interrupted rebase; resume with 'gs rebase --continue'."`
+
+	// RecordPushed is set only by the rebase continuation that finishes a
+	// conflicted '--rebase' sync: it records the integrated remote head as
+	// the last-pushed baseline without re-running the rebase.
+	RecordPushed string `hidden:"" help:"Record the given hash as the last-pushed baseline and exit."`
 }
 
 func (*branchSyncCmd) Help() string {
@@ -45,14 +51,31 @@ func (cmd *branchSyncCmd) AfterApply(ctx context.Context, wt *git.Worktree) erro
 func (cmd *branchSyncCmd) Run(
 	ctx context.Context,
 	log *silog.Logger,
+	svc *spice.Service,
 	handler *branchsync.Handler,
 ) error {
 	req := branchsync.SyncRequest{Branch: cmd.Branch}
 	if cmd.Rebase {
 		req.Mode = branchsync.ModeRebase
 	}
+	if cmd.RecordPushed != "" {
+		req.RecordPushed = git.Hash(cmd.RecordPushed)
+	}
+
 	res, err := handler.Sync(ctx, req)
 	if err != nil {
+		// A '--rebase' sync can be interrupted by a conflict. Once the user
+		// resolves it and runs 'gs rebase continue', re-running the rebase
+		// would conflict again, so the continuation only records the
+		// integrated remote head (res.ToHash) as the last-pushed baseline.
+		if res != nil && !res.ToHash.IsZero() {
+			return svc.RebaseRescue(ctx, spice.RebaseRescueRequest{
+				Err:     err,
+				Command: []string{"branch", "sync", "--record-pushed", res.ToHash.String()},
+				Branch:  cmd.Branch,
+				Message: "interrupted: sync branch " + cmd.Branch,
+			})
+		}
 		return err
 	}
 

doc/src/guide/cr.md

@@ -191,6 +191,18 @@ if the operation could result in data loss.
 To override these safety checks
 and push to a branch anyway, use the `--force` flag.
 
+<!-- gs:version unreleased -->
+
+In particular, if someone else has pushed commits to a branch
+since git-spice last pushed it
+(for example, a maintainer adding changes to your CR),
+submitting will stop and list those commits
+instead of overwriting them.
+Use $$gs branch sync$$ to bring the commits into your branch,
+then submit again.
+This protection is based on the commit git-spice last pushed,
+so it holds even after the branch has been restacked.
+
 ### Update existing CRs only
 
 <!-- gs:version v0.10.0 -->

internal/git/integration_test.go

@@ -70,16 +70,18 @@ func TestIntegrationCommitListing(t *testing.T) {
 
 		assert.Equal(t, []git.CommitDetail{
 			{
-				Hash:       "9045e63b1d4d4e6e2db3fa03d4eb98a6ed653f3a",
-				ShortHash:  "9045e63",
-				Subject:    "Add feature2",
-				AuthorDate: time.Date(2024, 8, 27, 22, 10, 11, 0, time.UTC),
+				Hash:        "9045e63b1d4d4e6e2db3fa03d4eb98a6ed653f3a",
+				ShortHash:   "9045e63",
+				Subject:     "Add feature2",
+				AuthorEmail: "test@example.com",
+				AuthorDate:  time.Date(2024, 8, 27, 22, 10, 11, 0, time.UTC),
 			},
 			{
-				Hash:       "acca66548dc31594dc3bc669c804e98eda1edc3d",
-				ShortHash:  "acca665",
-				Subject:    "Add feature1",
-				AuthorDate: time.Date(2024, 8, 27, 21, 52, 12, 0, time.UTC),
+				Hash:        "acca66548dc31594dc3bc669c804e98eda1edc3d",
+				ShortHash:   "acca665",
+				Subject:     "Add feature1",
+				AuthorEmail: "test@example.com",
+				AuthorDate:  time.Date(2024, 8, 27, 21, 52, 12, 0, time.UTC),
 			},
 		}, commits)
 	})

internal/git/rev_list.go

@@ -37,6 +37,9 @@ type CommitDetail struct {
 	// Subject is the first line of the commit message.
 	Subject string
 
+	// AuthorEmail is the email address of the commit author.
+	AuthorEmail string
+
 	// AuthorDate is the time the commit was authored.
 	AuthorDate time.Time
 }
@@ -48,7 +51,9 @@ func (cd *CommitDetail) String() string {
 // ListCommitsDetails returns details about commits matched by the given range.
 func (r *Repository) ListCommitsDetails(ctx context.Context, commits CommitRange) iter.Seq2[CommitDetail, error] {
 	return func(yield func(CommitDetail, error) bool) {
-		for line, err := range r.listCommitsFormat(ctx, commits, "%H %h %at %s") {
+		// %ae (author email) carries no spaces, so it stays a single
+		// space-delimited field ahead of the free-form subject.
+		for line, err := range r.listCommitsFormat(ctx, commits, "%H %h %at %ae %s") {
 			if err != nil {
 				yield(CommitDetail{}, err)
 				return
@@ -66,7 +71,7 @@ func (r *Repository) ListCommitsDetails(ctx context.Context, commits CommitRange
 				continue
 			}
 
-			epochstr, subject, ok := strings.Cut(line, " ")
+			epochstr, line, ok := strings.Cut(line, " ")
 			if !ok {
 				r.log.Warn("Bad rev-list output", "line", line, "error", "missing an time")
 				continue
@@ -77,11 +82,18 @@ func (r *Repository) ListCommitsDetails(ctx context.Context, commits CommitRange
 				continue
 			}
 
+			authorEmail, subject, ok := strings.Cut(line, " ")
+			if !ok {
+				r.log.Warn("Bad rev-list output", "line", line, "error", "missing an author email")
+				continue
+			}
+
 			if !yield(CommitDetail{
-				Hash:       Hash(hash),
-				ShortHash:  Hash(shortHash),
-				Subject:    subject,
-				AuthorDate: time.Unix(epoch, 0),
+				Hash:        Hash(hash),
+				ShortHash:   Hash(shortHash),
+				Subject:     subject,
+				AuthorEmail: authorEmail,
+				AuthorDate:  time.Unix(epoch, 0),
 			}, nil) {
 				return
 			}

internal/handler/branchsync/handler.go

@@ -125,6 +125,16 @@ const (
 type SyncRequest struct {
 	Branch string
 	Mode   Mode
+
+	// RecordPushed, when set, makes Sync skip all fetching and rebasing and
+	// only record the given hash as the branch's last-pushed baseline.
+	//
+	// It is used to finish a '--rebase' sync that was interrupted by a
+	// conflict: once the user resolves it and runs 'gs rebase continue', the
+	// remote commits are integrated, but the baseline still needs to be
+	// recorded. Re-running the full sync would try to rebase again, so the
+	// continuation records the baseline directly instead.
+	RecordPushed git.Hash
 }
 
 // Sync syncs a single tracked branch according to the request. Returns
@@ -138,6 +148,26 @@ func (h *Handler) Sync(ctx context.Context, req SyncRequest) (*SyncResult, error
 		return &SyncResult{Branch: branch, Action: ActionSkipped, SkipReason: "trunk is synced by 'gs repo sync'"}, nil
 	}
 
+	// Finish an interrupted '--rebase' sync: just record the baseline.
+	if !req.RecordPushed.IsZero() {
+		lookup, err := h.Store.LookupBranch(ctx, branch)
+		if err != nil {
+			return nil, fmt.Errorf("lookup branch: %w", err)
+		}
+		if lookup.UpstreamBranch == "" {
+			return nil, ErrNoUpstream
+		}
+		if err := h.recordPushedHash(ctx, branch, lookup.UpstreamBranch, req.RecordPushed); err != nil {
+			return nil, err
+		}
+		return &SyncResult{
+			Branch:   branch,
+			Action:   ActionRebased,
+			FromHash: req.RecordPushed,
+			ToHash:   req.RecordPushed,
+		}, nil
+	}
+
 	lookup, err := h.Store.LookupBranch(ctx, branch)
 	if err != nil {
 		return nil, fmt.Errorf("lookup branch: %w", err)
@@ -243,7 +273,11 @@ func (h *Handler) Sync(ctx context.Context, req SyncRequest) (*SyncResult, error
 	// already guarantees. This is what lets remote-side commits survive
 	// even after the branch has been restacked locally.
 	if err := h.rebase(ctx, branch, lookup.UpstreamBranch, localHash, remoteHash, pHash); err != nil {
-		return nil, err
+		// The rebase was interrupted (typically a conflict). Surface the
+		// integration target so the caller can record it as the baseline
+		// once the user resolves the conflict and resumes.
+		res.ToHash = remoteHash
+		return res, err
 	}
 	if err := h.recordPushedHash(ctx, branch, lookup.UpstreamBranch, remoteHash); err != nil {
 		log.Warn("Could not record pushed hash", "error", err)

internal/handler/split/handler.go

@@ -37,6 +37,7 @@ type Store interface {
 	Trunk() string
 	Remote() (state.Remote, error)
 	BeginBranchTx() *state.BranchTx
+	LookupBranch(ctx context.Context, name string) (*state.LookupResponse, error)
 }
 
 var _ Store = (*state.Store)(nil)
@@ -354,6 +355,15 @@ func (h *Handler) prepareChangeMetadataTransfer(
 
 	toUpstreamBranch := cmp.Or(upstreamBranch, fromBranch)
 
+	// Carry the last-pushed baseline along with the upstream branch, so a
+	// later 'submit' of toBranch can still tell whether the remote has been
+	// touched by someone else. Without it, toBranch would have no baseline
+	// and submit would refuse to push.
+	var lastPushed *git.Hash
+	if resp, err := h.Store.LookupBranch(ctx, fromBranch); err == nil && !resp.UpstreamLastPushedHash.IsZero() {
+		lastPushed = &resp.UpstreamLastPushedHash
+	}
+
 	var empty string
 	if err := tx.Upsert(ctx, state.UpsertRequest{
 		Name:           fromBranch,
@@ -364,10 +374,11 @@ func (h *Handler) prepareChangeMetadataTransfer(
 	}
 
 	if err := tx.Upsert(ctx, state.UpsertRequest{
-		Name:           toBranch,
-		ChangeMetadata: metaJSON,
-		ChangeForge:    forgeID,
-		UpstreamBranch: &toUpstreamBranch,
+		Name:                   toBranch,
+		ChangeMetadata:         metaJSON,
+		ChangeForge:            forgeID,
+		UpstreamBranch:         &toUpstreamBranch,
+		UpstreamLastPushedHash: lastPushed,
 	}); err != nil {
 		return nil, fmt.Errorf("set change metadata on %v: %w", toBranch, err)
 	}

internal/handler/submit/errors.go

@@ -0,0 +1,89 @@
+package submit
+
+import (
+	"fmt"
+
+	"go.abhg.dev/gs/internal/git"
+)
+
+// RemoteAdvancedError indicates that the remote upstream branch gained
+// commits on top of what git-spice last pushed, so a force-push would
+// discard them.
+//
+// The user can bring those commits into their branch with 'gs branch sync'
+// and submit again, or use --force to overwrite them.
+type RemoteAdvancedError struct {
+	// Branch is the local branch being submitted.
+	Branch string
+
+	// Remote is the name of the remote being pushed to.
+	Remote string
+
+	// UpstreamBranch is the name of the branch on the remote.
+	UpstreamBranch string
+
+	// Commits are the commits present on the remote
+	// that are not in the local branch, newest first.
+	Commits []git.CommitDetail
+}
+
+func (e *RemoteAdvancedError) Error() string {
+	return fmt.Sprintf(
+		"%v/%v has %d commit(s) not in branch %v",
+		e.Remote, e.UpstreamBranch, len(e.Commits), e.Branch,
+	)
+}
+
+// RemoteDivergedError indicates that the remote upstream branch was rewritten
+// away from what git-spice last pushed, so we cannot identify what a
+// force-push would discard.
+type RemoteDivergedError struct {
+	// Branch is the local branch being submitted.
+	Branch string
+
+	// Remote is the name of the remote being pushed to.
+	Remote string
+
+	// UpstreamBranch is the name of the branch on the remote.
+	UpstreamBranch string
+
+	// LastPushed is the commit git-spice last pushed to the upstream branch.
+	LastPushed git.Hash
+
+	// RemoteHash is the upstream branch's current head.
+	RemoteHash git.Hash
+}
+
+func (e *RemoteDivergedError) Error() string {
+	return fmt.Sprintf(
+		"%v/%v was rewritten away from the last pushed commit %v",
+		e.Remote, e.UpstreamBranch, e.LastPushed.Short(),
+	)
+}
+
+// RemoteNoBaselineError indicates that git-spice has no record of what it last
+// pushed to the upstream branch, and the remote differs from the local branch,
+// so it cannot prove that a force-push is safe.
+type RemoteNoBaselineError struct {
+	// Branch is the local branch being submitted.
+	Branch string
+
+	// Remote is the name of the remote being pushed to.
+	Remote string
+
+	// UpstreamBranch is the name of the branch on the remote.
+	UpstreamBranch string
+
+	// RemoteHash is the upstream branch's current head.
+	RemoteHash git.Hash
+
+	// LocalHash is the local branch's head.
+	LocalHash git.Hash
+}
+
+func (e *RemoteNoBaselineError) Error() string {
+	return fmt.Sprintf(
+		"no record of the last commit pushed to %v/%v",
+		e.Remote, e.UpstreamBranch,
+	)
+}