submit: Check base CR status before submission

.changes/unreleased/Added-20260512-100734.yaml

@@ -0,0 +1,3 @@
+kind: Added
+body: 'submit: Skip submission when a downstack base''s change has been merged (use --force to bypass)'
+time: 2026-05-12T10:07:34.7611-04:00

downstack_submit.go

@@ -48,10 +48,12 @@ func (cmd *downstackSubmitCmd) Run(
 		return errors.New("nothing to submit below trunk")
 	}
 
-	downstacks, err := svc.ListDownstack(ctx, cmd.Branch)
+	graph, err := svc.BranchGraph(ctx, nil)
 	if err != nil {
-		return fmt.Errorf("list downstack: %w", err)
+		return fmt.Errorf("build branch graph: %w", err)
 	}
+
+	downstacks := slices.Collect(graph.Downstack(cmd.Branch))
 	must.NotBeEmptyf(downstacks, "downstack cannot be empty")
 	slices.Reverse(downstacks)
 
@@ -61,5 +63,6 @@ func (cmd *downstackSubmitCmd) Run(
 		Branches:     downstacks,
 		Options:      &cmd.Options,
 		BatchOptions: &cmd.BatchOptions,
+		BranchGraph:  graph,
 	})
 }

internal/handler/submit/handler.go

@@ -65,9 +65,9 @@ var _ Store = (*state.Store)(nil)
 
 // Service provides access to the Spice service.
 type Service interface {
+	BranchGraph(context.Context, *spice.BranchGraphOptions) (*spice.BranchGraph, error)
 	LoadBranches(context.Context) ([]spice.LoadBranchItem, error)
 	VerifyRestacked(ctx context.Context, name string) error
-	LookupBranch(ctx context.Context, name string) (*spice.LookupBranchResponse, error)
 	UnusedBranchName(ctx context.Context, remote string, branch string) (string, error)
 	ListChangeTemplates(context.Context, string, forge.Repository) ([]*forge.ChangeTemplate, error)
 }
@@ -470,6 +470,13 @@ type BatchRequest struct {
 	Branches     []string // required
 	Options      *Options
 	BatchOptions *BatchOptions // required
+
+	// BranchGraph is an optional graph already built by the command layer.
+	//
+	// Batch submit commands often build the graph to decide which branches are
+	// in scope. Reusing it here avoids loading the same branch state again for
+	// stale-base validation.
+	BranchGraph *spice.BranchGraph
 }
 
 // SubmitBatch submits a batch of branches to a remote repository,
@@ -485,12 +492,25 @@ func (h *Handler) SubmitBatch(ctx context.Context, req *BatchRequest) error {
 		opts.UpdateOnly = &batchOpts.UpdateOnlyDefault
 	}
 
+	graph := req.BranchGraph
+	if graph == nil {
+		var err error
+		graph, err = h.Service.BranchGraph(ctx, nil)
+		if err != nil {
+			return fmt.Errorf("build branch graph: %w", err)
+		}
+	}
+	if err := h.checkStaleSubmissionBases(ctx, graph, req.Branches, opts); err != nil {
+		return err
+	}
+
 	var branchesToComment []string
 	for _, branch := range req.Branches {
 		// Shallow copy the options because submitBranch may modify them.
 		opts := *opts
 		status, err := h.submitBranch(
 			ctx,
+			graph,
 			branch,
 			&submitOptions{Options: &opts},
 		)
@@ -528,15 +548,32 @@ type Request struct {
 
 	// Options are the options for the submit operation.
 	Options *Options // optional
+
+	// BranchGraph is an optional graph already built by the command layer.
+	// If not provided, it may be loaded as required.
+	BranchGraph *spice.BranchGraph
 }
 
 // Submit submits a single branch to a remote repository,
 // creating or updating a change request as needed.
 func (h *Handler) Submit(ctx context.Context, req *Request) error {
 	opts := cmp.Or(req.Options, &Options{})
 	mergeConfiguredOptions(opts)
+	graph := req.BranchGraph
+	if graph == nil {
+		var err error
+		graph, err = h.Service.BranchGraph(ctx, nil)
+		if err != nil {
+			return fmt.Errorf("build branch graph: %w", err)
+		}
+	}
+	if err := h.checkStaleSubmissionBases(ctx, graph, []string{req.Branch}, opts); err != nil {
+		return err
+	}
+
 	status, err := h.submitBranch(
 		ctx,
+		graph,
 		req.Branch,
 		&submitOptions{
 			Options: opts,
@@ -582,6 +619,7 @@ type submitOptions struct {
 
 func (h *Handler) submitBranch(
 	ctx context.Context,
+	graph *spice.BranchGraph,
 	branchToSubmit string,
 	opts *submitOptions,
 ) (status submitStatus, err error) {
@@ -592,9 +630,9 @@ func (h *Handler) submitBranch(
 	svc := h.Service
 	log := h.Log
 
-	branch, err := svc.LookupBranch(ctx, branchToSubmit)
-	if err != nil {
-		return status, fmt.Errorf("lookup branch: %w", err)
+	branch, ok := graph.Lookup(branchToSubmit)
+	if !ok {
+		return status, fmt.Errorf("lookup branch: %w", state.ErrNotExist)
 	}
 
 	// Various code paths down below should call this
@@ -634,9 +672,9 @@ func (h *Handler) submitBranch(
 	// use that name instead.
 	upstreamBase := branch.Base
 	if branch.Base != h.Store.Trunk() {
-		baseBranch, err := svc.LookupBranch(ctx, branch.Base)
-		if err != nil {
-			return status, fmt.Errorf("lookup base branch: %w", err)
+		baseBranch, ok := graph.Lookup(branch.Base)
+		if !ok {
+			return status, fmt.Errorf("lookup base branch: %w", state.ErrNotExist)
 		}
 		upstreamBase = cmp.Or(baseBranch.UpstreamBranch, branch.Base)
 	}

internal/handler/submit/handler_test.go

@@ -14,6 +14,7 @@ import (
 	"go.abhg.dev/gs/internal/forge/forgetest"
 	"go.abhg.dev/gs/internal/silog"
 	"go.abhg.dev/gs/internal/silog/silogtest"
+	"go.abhg.dev/gs/internal/spice"
 	"go.abhg.dev/gs/internal/spice/state"
 	"go.abhg.dev/gs/internal/ui"
 	gomock "go.uber.org/mock/gomock"
@@ -114,6 +115,93 @@ func TestHandler_pushRepositoryID_rejectsDifferentForge(t *testing.T) {
 	assert.Contains(t, err.Error(), "different forge")
 }
 
+func TestHandler_SubmitBatch_rejectsStaleBaseBeforeSubmitting(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+
+	mockStore := NewMockStore(mockCtrl)
+	mockStore.EXPECT().
+		Trunk().
+		Return("main").
+		AnyTimes()
+
+	mockService := NewMockService(mockCtrl)
+	mockService.EXPECT().
+		BranchGraph(gomock.Any(), gomock.Any()).
+		Return(buildStaleBaseTestGraph(t, "main", []spice.LoadBranchItem{
+			{Name: "feat1", Base: "main", Change: submitFakeChange("pr-1")},
+			{Name: "feat2", Base: "feat1", Change: submitFakeChange("pr-2")},
+		}), nil)
+
+	mockRemoteRepo := forgetest.NewMockRepository(mockCtrl)
+	mockRemoteRepo.EXPECT().
+		ChangeStatuses(gomock.Any(), []forge.ChangeID{
+			submitFakeChangeID("pr-1"),
+		}).
+		Return([]forge.ChangeStatus{
+			{State: forge.ChangeMerged},
+		}, nil)
+
+	handler := &Handler{
+		Log:        silog.Nop(),
+		View:       ui.NewFileView(&bytes.Buffer{}),
+		Repository: nil,
+		Worktree:   nil,
+		Store:      mockStore,
+		Service:    mockService,
+		Browser:    &browser.Noop{},
+		FindRemote: func(context.Context) (state.Remote, error) {
+			return state.Remote{Upstream: "origin", Push: "origin"}, nil
+		},
+		OpenRepository: func(
+			context.Context,
+			forge.Forge,
+			forge.RepositoryID,
+		) (forge.Repository, error) {
+			return mockRemoteRepo, nil
+		},
+		ResolveRepository: func(
+			context.Context,
+			string,
+		) (forge.Forge, forge.RepositoryID, error) {
+			return forgetest.NewMockForge(mockCtrl),
+				stubRepositoryID("alice/repo"), nil
+		},
+	}
+
+	err := handler.SubmitBatch(t.Context(), &BatchRequest{
+		Branches:     []string{"feat2"},
+		Options:      &Options{},
+		BatchOptions: &BatchOptions{},
+	})
+
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "1 branches with stale bases were found")
+	assert.ErrorContains(t, err, "gs repo sync")
+	assert.ErrorContains(t, err, "--force")
+}
+
+func TestHandler_checkStaleSubmissionBases_forceSkipsValidation(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+
+	handler := &Handler{
+		Log:               silog.Nop(),
+		View:              ui.NewFileView(&bytes.Buffer{}),
+		Repository:        nil,
+		Worktree:          nil,
+		Store:             NewMockStore(mockCtrl),
+		Service:           NewMockService(mockCtrl),
+		Browser:           &browser.Noop{},
+		FindRemote:        nil,
+		ResolveRepository: nil,
+		OpenRepository:    nil,
+	}
+
+	err := handler.checkStaleSubmissionBases(
+		t.Context(), nil, []string{"feat2"}, &Options{Force: true},
+	)
+	require.NoError(t, err)
+}
+
 func TestReviewersAddWhen_UnmarshalText(t *testing.T) {
 	tests := []struct {
 		name    string
@@ -252,16 +340,6 @@ func TestEffectiveReviewers(t *testing.T) {
 	}
 }
 
-type stubRepositoryID string
-
-func (id stubRepositoryID) String() string {
-	return string(id)
-}
-
-func (id stubRepositoryID) ChangeURL(forge.ChangeID) string {
-	return string(id)
-}
-
 func TestLabelsAddWhen_UnmarshalText(t *testing.T) {
 	tests := []struct {
 		name    string
@@ -399,3 +477,40 @@ func TestEffectiveLabels(t *testing.T) {
 		})
 	}
 }
+
+type stubRepositoryID string
+
+func (id stubRepositoryID) String() string {
+	return string(id)
+}
+
+func (id stubRepositoryID) ChangeURL(forge.ChangeID) string {
+	return string(id)
+}
+
+// submitFakeChangeMetadata is the minimal change metadata needed by submit
+// handler tests that reason about stored branch state.
+type submitFakeChangeMetadata struct {
+	id forge.ChangeID
+}
+
+var _ forge.ChangeMetadata = (*submitFakeChangeMetadata)(nil)
+
+func (m *submitFakeChangeMetadata) ForgeID() string { return "test" }
+func (m *submitFakeChangeMetadata) ChangeID() forge.ChangeID {
+	return m.id
+}
+
+func (m *submitFakeChangeMetadata) NavigationCommentID() forge.ChangeCommentID {
+	return nil
+}
+func (m *submitFakeChangeMetadata) SetNavigationCommentID(forge.ChangeCommentID) {}
+
+// submitFakeChangeID identifies a fake change in submit handler tests.
+type submitFakeChangeID string
+
+func (id submitFakeChangeID) String() string { return string(id) }
+
+func submitFakeChange(id string) forge.ChangeMetadata {
+	return &submitFakeChangeMetadata{id: submitFakeChangeID(id)}
+}