fix: harden onboarding completion and state invariants

Author: binarylogic

internal/app/chat/empty_state_poll_test.go

@@ -78,4 +78,3 @@ func TestEmptyStateSummaryMessageUpdatesState(t *testing.T) {
 		t.Fatalf("unexpected service count: got %d want %d", m.policySummary.ServiceCount, 5)
 	}
 }
-

internal/app/onboarding/auth/authenticate_test.go

@@ -21,7 +21,6 @@ func TestAuthPollInterval(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			got := authPollInterval(tc.interval)

internal/app/onboarding/datadog/appkey_test.go

@@ -41,7 +41,6 @@ func TestAppKeyErrorMessage(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			got := appKeyErrorMessage(tc.err)

internal/app/onboarding/datadog/discovery_test.go

@@ -82,4 +82,3 @@ func TestDiscoveryStatusReadyCompletesStep(t *testing.T) {
 		t.Fatalf("expected DatadogDiscoveryComplete message")
 	}
 }
-

internal/app/onboarding/errorfmt/errorfmt_test.go

@@ -43,7 +43,6 @@ func TestUserFacing(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			got := UserFacing(tc.err, tc.fallback)

internal/app/onboarding/gate_requirements.go

@@ -10,7 +10,7 @@ func (m *Model) rewindGateFor(target Gate) Gate {
 		return target
 	}
 	rewind := bootstrap.RewindGate(
-		bootstrap.Gate(target),
+		target,
 		def.requirement,
 		bootstrap.State{
 			User:      m.state.user,
@@ -22,5 +22,5 @@ func (m *Model) rewindGateFor(target Gate) Gate {
 			DDAccount: m.state.ddAccount,
 		},
 	)
-	return Gate(rewind)
+	return rewind
 }

internal/app/onboarding/gate_requirements_test.go

@@ -22,7 +22,6 @@ func TestRewindGateFor(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			m := newTestModel(t)

internal/app/onboarding/transition_handlers.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/usetero/cli/internal/app/onboarding/msgs"
 	"github.com/usetero/cli/internal/core/bootstrap"
+	"github.com/usetero/cli/internal/domain"
 )
 
 func (m *Model) toCoreState() bootstrap.State {
@@ -29,6 +30,15 @@ func (m *Model) applyCoreState(state bootstrap.State) {
 	m.state.ddSite = state.DDSite
 	m.state.ddAPIKey = state.DDAPIKey
 	m.state.ddAccount = state.DDAccount
+	m.syncServicesToState()
+}
+
+func (m *Model) syncServicesToState() {
+	accountID := domain.AccountID("")
+	if m.state.account != nil {
+		accountID = m.state.account.ID
+	}
+	m.services = m.services.WithAccountID(accountID)
 }
 
 func (m *Model) handlePreflightResolved(msg msgs.PreflightResolved) TransitionOutcome {
@@ -56,14 +66,8 @@ func (m *Model) handlePreflightResolved(msg msgs.PreflightResolved) TransitionOu
 			Account:      msg.State.Account,
 		},
 	)
-	nextGate := Gate(next)
+	nextGate := next
 	m.applyCoreState(nextState)
-	if m.state.org != nil && m.state.account == nil {
-		m.services = m.services.WithAccountID("")
-	}
-	if m.state.account != nil {
-		m.services = m.services.WithAccountID(m.state.account.ID)
-	}
 	m.scope.Info("preflight decision",
 		slog.String("outcome", string(msg.State.Outcome)),
 		slog.String("next_gate", nextGate.String()))
@@ -74,129 +78,137 @@ func (m *Model) handleAuthenticated(msg msgs.Authenticated) TransitionOutcome {
 	m.scope.Info("user authenticated", "user_id", msg.User.ID)
 	nextState, next := bootstrap.ApplyAuthenticated(m.toCoreState(), msg.User)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleRoleSelected(msg msgs.RoleSelected) TransitionOutcome {
 	m.scope.Info("role selected", slog.String("role", msg.Role))
 	nextState, next := bootstrap.ApplyRoleSelected(m.toCoreState(), msg.Role)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleOrgSelected(msg msgs.OrgSelected) TransitionOutcome {
 	m.scope.Info("organization selected", slog.String("org_id", msg.Org.ID.String()))
 	nextState, next := bootstrap.ApplyOrgSelected(m.toCoreState(), msg.Org)
 	m.applyCoreState(nextState)
-	m.services = m.services.WithAccountID("")
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleNoOrgs() TransitionOutcome {
 	m.scope.Debug("no organizations found")
 	nextState, next := bootstrap.ApplyNoOrgs(m.toCoreState())
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleOrgCreated(msg msgs.OrgCreated) TransitionOutcome {
 	m.scope.Info("organization created", slog.String("org_id", msg.Org.ID.String()))
 	nextState, next := bootstrap.ApplyOrgCreated(m.toCoreState(), msg.Org)
 	m.applyCoreState(nextState)
-	m.services = m.services.WithAccountID("")
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleAccountSelected(msg msgs.AccountSelected) TransitionOutcome {
 	m.scope.Info("account selected", slog.String("account_id", msg.Account.ID.String()))
 	nextState, next := bootstrap.ApplyAccountSelected(m.toCoreState(), msg.Org, msg.Account)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleNoAccounts(msg msgs.NoAccounts) TransitionOutcome {
 	m.scope.Debug("no accounts found")
 	nextState, next := bootstrap.ApplyNoAccounts(m.toCoreState(), msg.Org)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleAccountCreated(msg msgs.AccountCreated) TransitionOutcome {
 	m.scope.Info("account created", slog.String("account_id", msg.Account.ID.String()))
 	nextState, next := bootstrap.ApplyAccountCreated(m.toCoreState(), msg.Org, msg.Account)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleRuntimeReady(msg msgs.RuntimeReady) TransitionOutcome {
 	m.scope.Info("runtime initialized", slog.String("account_id", msg.Account.ID.String()))
 	nextState, next := bootstrap.ApplyRuntimeReady(m.toCoreState(), msg.Org, msg.Account)
 	m.applyCoreState(nextState)
-	m.services = m.services.WithAccountID(msg.Account.ID)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleDatadogReady() TransitionOutcome {
 	m.scope.Debug("datadog ready")
 	nextState, next := bootstrap.ApplyDatadogReady(m.toCoreState())
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleDatadogNeeded() TransitionOutcome {
 	m.scope.Debug("datadog setup needed")
 	nextState, next := bootstrap.ApplyDatadogNeeded(m.toCoreState())
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleDatadogRegionSelected(msg msgs.DatadogRegionSelected) TransitionOutcome {
 	m.scope.Info("datadog region selected", slog.String("site", string(msg.Site)))
 	nextState, next := bootstrap.ApplyDatadogRegionSelected(m.toCoreState(), msg.Site)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleDatadogAPIKeyEntered(msg msgs.DatadogAPIKeyEntered) TransitionOutcome {
 	m.scope.Debug("datadog api key validated")
 	nextState, next := bootstrap.ApplyDatadogAPIKeyEntered(m.toCoreState(), msg.APIKey)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleDatadogAccountCreated(msg msgs.DatadogAccountCreated) TransitionOutcome {
 	m.scope.Info("datadog account created", slog.String("datadog_account_id", msg.DatadogAccountID.String()))
 	nextState, next := bootstrap.ApplyDatadogAccountCreated(m.toCoreState(), msg.DatadogAccountID)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleDatadogDiscoveryComplete() TransitionOutcome {
 	m.scope.Info("datadog discovery complete")
 	nextState, next := bootstrap.ApplyDatadogDiscoveryComplete(m.toCoreState())
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleWorkspaceSelected(msg msgs.WorkspaceSelected) TransitionOutcome {
 	m.scope.Info("workspace selected", slog.String("workspace_id", string(msg.Workspace.ID)))
 	nextState, next := bootstrap.ApplyWorkspaceSelected(m.toCoreState(), msg.Workspace)
 	m.applyCoreState(nextState)
-	return advance(Gate(next))
+	return advance(next)
 }
 
 func (m *Model) handleSyncComplete() TransitionOutcome {
+	completion, ok := bootstrap.CompleteOnboarding(m.toCoreState())
+	if !ok {
+		m.scope.Error("sync completed without required onboarding state",
+			slog.Bool("has_user", m.state.user != nil),
+			slog.Bool("has_org", m.state.org != nil),
+			slog.Bool("has_account", m.state.account != nil),
+			slog.Bool("has_workspace", m.state.workspace != nil),
+		)
+		return noop()
+	}
+
 	m.scope.Info("onboarding complete",
-		slog.String("org_id", m.state.org.ID.String()),
-		slog.String("account_id", m.state.account.ID.String()),
-		slog.String("workspace_id", string(m.state.workspace.ID)),
+		slog.String("org_id", completion.Org.ID.String()),
+		slog.String("account_id", completion.Account.ID.String()),
+		slog.String("workspace_id", string(completion.Workspace.ID)),
 	)
 	return advanceWith("", func() tea.Msg {
 		return msgs.OnboardingComplete{
-			User:      m.state.user,
-			Org:       *m.state.org,
-			Account:   *m.state.account,
-			Workspace: *m.state.workspace,
+			User:      completion.User,
+			Org:       completion.Org,
+			Account:   completion.Account,
+			Workspace: completion.Workspace,
 		}
 	})
 }

internal/app/onboarding/transitions_test.go

@@ -79,7 +79,6 @@ func TestHandleTransitionPreflightRouting(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			m := newTestModel(t)
@@ -114,7 +113,6 @@ func TestHandleTransitionDatadogBranchRouting(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			m := newTestModel(t)
@@ -189,6 +187,40 @@ func TestHandleTransitionRuntimeReady(t *testing.T) {
 	}
 }
 
+func TestHandleTransitionRuntimeReadySyncsServiceAccountScope(t *testing.T) {
+	t.Parallel()
+
+	m, client := newTestModelWithClient(t)
+	var scopedAccountID domain.AccountID
+	client.SetAccountIDFunc = func(accountID domain.AccountID) {
+		scopedAccountID = accountID
+	}
+
+	org := ptrOrg("org-1")
+	account := ptrAccount("acc-1")
+
+	_ = m.handleTransition(msgs.RuntimeReady{Org: *org, Account: *account})
+	if scopedAccountID != "acc-1" {
+		t.Fatalf("scoped account id = %q, want %q", scopedAccountID, "acc-1")
+	}
+}
+
+func TestHandleTransitionOrgSelectedClearsServiceAccountScope(t *testing.T) {
+	t.Parallel()
+
+	m, client := newTestModelWithClient(t)
+	var scopedAccountID domain.AccountID
+	client.SetAccountIDFunc = func(accountID domain.AccountID) {
+		scopedAccountID = accountID
+	}
+
+	m.state.account = ptrAccount("acc-1")
+	_ = m.handleTransition(msgs.OrgSelected{Org: *ptrOrg("org-2")})
+	if scopedAccountID != "" {
+		t.Fatalf("scoped account id = %q, want empty", scopedAccountID)
+	}
+}
+
 func TestHandleTransitionSyncComplete(t *testing.T) {
 	t.Parallel()
 
@@ -212,8 +244,28 @@ func TestHandleTransitionSyncComplete(t *testing.T) {
 	}
 }
 
+func TestHandleTransitionSyncCompleteMissingStateNoops(t *testing.T) {
+	t.Parallel()
+
+	m := newTestModel(t)
+	m.state.user = ptrUser("user-1")
+	m.state.org = ptrOrg("org-1")
+	// Missing account/workspace should not panic or emit completion payload.
+
+	cmd := m.handleTransition(msgs.SyncComplete{})
+	if cmd != nil {
+		t.Fatal("expected nil command when completion state is incomplete")
+	}
+}
+
 func newTestModel(t *testing.T) *Model {
 	t.Helper()
+	m, _ := newTestModelWithClient(t)
+	return m
+}
+
+func newTestModelWithClient(t *testing.T) (*Model, *apitest.MockClient) {
+	t.Helper()
 
 	scope := logtest.NewScope(t)
 	client := apitest.NewMockClient()
@@ -225,7 +277,7 @@ func newTestModel(t *testing.T) *Model {
 
 	m := New(context.Background(), styles.NewTheme(true), services, userPrefs, orgPrefs, authSvc, syncer, scope)
 	m.SetSize(120, 40)
-	return m
+	return m, client
 }
 
 func ptrOrg(id string) *domain.Organization {

internal/app/onboarding/visibility_test.go

@@ -68,11 +68,11 @@ type fixedTestStep struct {
 	view string
 }
 
-func (s fixedTestStep) Init() tea.Cmd                { return nil }
-func (s fixedTestStep) Update(msg tea.Msg) tea.Cmd   { return nil }
-func (s fixedTestStep) View() string                 { return s.view }
-func (s fixedTestStep) SetSize(width, height int)    {}
-func (s fixedTestStep) ShortHelp() []key.Binding     { return nil }
+func (s fixedTestStep) Init() tea.Cmd              { return nil }
+func (s fixedTestStep) Update(msg tea.Msg) tea.Cmd { return nil }
+func (s fixedTestStep) View() string               { return s.view }
+func (s fixedTestStep) SetSize(width, height int)  {}
+func (s fixedTestStep) ShortHelp() []key.Binding   { return nil }
 
 type hiddenTestStep struct {
 	fixedTestStep