fix(coderd/x/chatd): recover timed out agents (#27254)

Closes CODAGT-802 Coder Agents only escalated failed workspace dials when the agent had connected and later disconnected. An agent that never connected and had already exceeded its `connection_timeout` stayed on the soft retry error indefinitely, so a chat could keep attempting tools against an unhealthy workspace. To fix, we'll classify the latest agent after a failed dial and return stop/start recovery guidance when its status is `timeout`. Agents still connecting, including templates with `connection_timeout = 0`, keep the existing retryable behaviour. ## Before <img width="843" height="229" alt="image" src="https://github.com/user-attachments/assets/d659a376-c8d4-4983-b7d7-d1a699770dfb" /> ## After <img width="848" height="250" alt="image" src="https://github.com/user-attachments/assets/26b88f72-f67c-4d9e-87b1-51d701b7352f" />
2026-09-01 14:53:15 +08:00 · 2026-07-16 12:52:41 +10:00
parent de716f89dc
commit 21d08241e9
2 changed files with 122 additions and 125 deletions
@@ -120,6 +120,12 @@ var (
 			"to stop the workspace, then start_workspace to start it " +
 			"again",
 	)
+	errChatAgentNeverConnected = xerrors.New(
+		"workspace agent never connected and its connection timeout has " +
+			"elapsed, so it cannot execute tools. To recover, call " +
+			"stop_workspace to stop the workspace, then start_workspace " +
+			"to start it again",
+	)
 	errChatDialTimeout = xerrors.New(
 		"connection to the workspace agent timed out. " +
 			"The agent may still be reachable on the next attempt.",
@@ -846,17 +852,17 @@ func agentDisconnectedFor(now time.Time, agent database.WorkspaceAgent, inactive
 	return disconnectedFor, true
 }

-func (c *turnWorkspaceContext) latestWorkspaceAgentNeedsRestart(
+func (c *turnWorkspaceContext) latestWorkspaceAgentRecoveryError(
 	ctx context.Context,
 	workspaceID uuid.UUID,
-) (bool, error) {
+) error {
 	agentID, err := c.latestWorkspaceAgentID(ctx, workspaceID)
 	if err != nil {
 		if xerrors.Is(err, errChatHasNoWorkspaceAgent) {
-			return false, err
+			return err
 		}
 		c.server.logger.Warn(ctx, "failed to resolve latest agent for timeout classification", slog.Error(err))
-		return false, nil
+		return errChatDialTimeout
 	}

 	agent, err := c.server.db.GetWorkspaceAgentByID(ctx, agentID)
@@ -865,11 +871,24 @@ func (c *turnWorkspaceContext) latestWorkspaceAgentNeedsRestart(
 			slog.F("agent_id", agentID),
 			slog.Error(err),
 		)
-		return false, nil
+		return errChatDialTimeout
 	}

-	disconnectedFor, disconnected := agentDisconnectedFor(c.server.clock.Now(), agent, c.server.agentInactiveDisconnectTimeout)
-	return disconnected && disconnectedFor >= agentDisconnectedRecoveryThreshold, nil
+	now := c.server.clock.Now()
+	status := agent.Status(now, c.server.agentInactiveDisconnectTimeout)
+	recoveryErr := errChatDialTimeout
+	if status.Status == database.WorkspaceAgentStatusTimeout {
+		recoveryErr = errChatAgentNeverConnected
+	} else if status.Status == database.WorkspaceAgentStatusDisconnected && status.DisconnectedAt != nil {
+		disconnectedFor := now.Sub(*status.DisconnectedAt)
+		if disconnectedFor < 0 {
+			disconnectedFor = 0
+		}
+		if disconnectedFor >= agentDisconnectedRecoveryThreshold {
+			recoveryErr = errChatAgentDisconnected
+		}
+	}
+	return c.externalAgentError(ctx, agent, recoveryErr)
 }

 func (c *turnWorkspaceContext) externalAgentError(
@@ -1002,14 +1021,7 @@ func (c *turnWorkspaceContext) getWorkspaceConn(ctx context.Context) (workspaces
 			// propagate unchanged so the chatloop can detect it.
 			if ctx.Err() == nil && errors.Is(context.Cause(dialCtx), errChatDialTimeout) {
 				c.clearCachedWorkspaceState()
-				needsRestart, statusErr := c.latestWorkspaceAgentNeedsRestart(ctx, chatSnapshot.WorkspaceID.UUID)
-				if statusErr != nil {
-					return nil, statusErr
-				}
-				if needsRestart {
-					return nil, c.externalAgentError(ctx, agent, errChatAgentDisconnected)
-				}
-				return nil, c.externalAgentError(ctx, agent, errChatDialTimeout)
+				return nil, c.latestWorkspaceAgentRecoveryError(ctx, chatSnapshot.WorkspaceID.UUID)
 			}
 			return nil, err
 		}
@@ -344,8 +344,11 @@ func TestChatWorkspaceRecoveryErrorsDifferentiateSignalStrength(t *testing.T) {
 	require.Contains(t, disconnected, "start_workspace")
 	require.NotContains(t, disconnected, "ask_user_question")

-	// Dial timeout alone is a weak signal. The model should not
-	// escalate to lifecycle tools without DB-confirmed disconnect.
+	neverConnected := errChatAgentNeverConnected.Error()
+	require.Contains(t, neverConnected, "stop_workspace")
+	require.Contains(t, neverConnected, "start_workspace")
+	require.NotContains(t, neverConnected, "ask_user_question")
+
 	dialTimeout := errChatDialTimeout.Error()
 	require.NotContains(t, dialTimeout, "ask_user_question")
 	require.NotContains(t, dialTimeout, "stop_workspace")
@@ -2380,10 +2383,6 @@ func TestGetWorkspaceConn_StatusCheck(t *testing.T) {

 	tests := []testCase{
 		{
-			// Agent never connected and the connection timeout
-			// has elapsed. This should not trigger lifecycle
-			// recovery because the agent did not connect and
-			// then disconnect.
 			name: "TimedOutAgentCacheHit",
 			buildAgent: func(now time.Time) database.WorkspaceAgent {
 				return database.WorkspaceAgent{
@@ -2514,34 +2513,83 @@ func TestGetWorkspaceConn_StatusCheck(t *testing.T) {
 }

 func TestGetWorkspaceConn_DialTimeoutDisconnectedRecoveryThreshold(t *testing.T) {
-	// The recovery sentinel requires a failed dial and a fresh
-	// disconnected status check past the recovery threshold. A
-	// disconnected DB row alone is not enough to trigger stop/start
-	// recovery.
 	t.Parallel()

+	buildDisconnectedAgent := func(disconnectedFor time.Duration) func(time.Time) database.WorkspaceAgent {
+		return func(now time.Time) database.WorkspaceAgent {
+			return database.WorkspaceAgent{
+				FirstConnectedAt: sql.NullTime{
+					Time:  now.Add(-10 * time.Minute),
+					Valid: true,
+				},
+				LastConnectedAt: sql.NullTime{
+					Time:  now.Add(-10 * time.Minute),
+					Valid: true,
+				},
+				DisconnectedAt: sql.NullTime{
+					Time:  now.Add(-disconnectedFor),
+					Valid: true,
+				},
+			}
+		}
+	}
+
+	buildTimedOutAgent := func(now time.Time) database.WorkspaceAgent {
+		return database.WorkspaceAgent{
+			CreatedAt:                now.Add(-10 * time.Minute),
+			ConnectionTimeoutSeconds: 60,
+		}
+	}
+
 	testCases := []struct {
-		name            string
-		disconnectedFor time.Duration
-		wantErr         error
-		wantRecovery    bool
+		name                 string
+		buildAgent           func(now time.Time) database.WorkspaceAgent
+		staleExternalBinding bool
+		wantErr              error
 	}{
 		{
-			name:            "RecentDisconnectReturnsDialTimeout",
-			disconnectedFor: agentDisconnectedRecoveryThreshold / 2,
-			wantErr:         errChatDialTimeout,
-			wantRecovery:    false,
+			name:       "RecentDisconnectReturnsDialTimeout",
+			buildAgent: buildDisconnectedAgent(agentDisconnectedRecoveryThreshold / 2),
+			wantErr:    errChatDialTimeout,
 		},
 		{
-			name:            "PastThresholdEscalates",
-			disconnectedFor: agentDisconnectedRecoveryThreshold,
-			wantErr:         errChatAgentDisconnected,
-			wantRecovery:    true,
+			name:       "PastThresholdEscalates",
+			buildAgent: buildDisconnectedAgent(agentDisconnectedRecoveryThreshold),
+			wantErr:    errChatAgentDisconnected,
+		},
+		{
+			name:       "NeverConnectedTimeoutEscalates",
+			buildAgent: buildTimedOutAgent,
+			wantErr:    errChatAgentNeverConnected,
+		},
+		{
+			name:                 "StaleExternalBindingUsesLatestInternalAgent",
+			buildAgent:           buildTimedOutAgent,
+			staleExternalBinding: true,
+			wantErr:              errChatAgentNeverConnected,
+		},
+		{
+			name: "NeverConnectedWithinTimeoutStaysSoft",
+			buildAgent: func(now time.Time) database.WorkspaceAgent {
+				return database.WorkspaceAgent{
+					CreatedAt:                now.Add(-30 * time.Second),
+					ConnectionTimeoutSeconds: 120,
+				}
+			},
+			wantErr: errChatDialTimeout,
+		},
+		{
+			name: "NeverConnectedNoTimeoutStaysSoft",
+			buildAgent: func(now time.Time) database.WorkspaceAgent {
+				return database.WorkspaceAgent{
+					CreatedAt: now.Add(-10 * time.Minute),
+				}
+			},
+			wantErr: errChatDialTimeout,
 		},
 	}

 	for _, tc := range testCases {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()

@@ -2568,28 +2616,28 @@ func TestGetWorkspaceConn_DialTimeoutDisconnectedRecoveryThreshold(t *testing.T)
 			delayTrap := clock.Trap().NewTimer("chatd", dialValidationDelayTimerTag)
 			defer delayTrap.Close()
 			now := clock.Now()
-			disconnectedAgent := database.WorkspaceAgent{
-				ID: agentID,
-				FirstConnectedAt: sql.NullTime{
-					Time:  now.Add(-10 * time.Minute),
-					Valid: true,
-				},
-				LastConnectedAt: sql.NullTime{
-					Time:  now.Add(-10 * time.Minute),
-					Valid: true,
-				},
-				DisconnectedAt: sql.NullTime{
-					Time:  now.Add(-tc.disconnectedFor),
-					Valid: true,
-				},
+			boundAgent := tc.buildAgent(now)
+			boundAgent.ID = agentID
+			latestAgent := boundAgent
+			latestAgent.ID = uuid.New()
+			latestAgentLookups := 1
+			if tc.staleExternalBinding {
+				boundAgent.ResourceID = uuid.New()
+				latestAgent.ResourceID = uuid.Nil
+				latestAgentLookups++
+				db.EXPECT().GetWorkspaceResourceByID(gomock.Any(), boundAgent.ResourceID).
+					Return(database.WorkspaceResource{Type: chattool.ExternalAgentResourceType}, nil).
+					AnyTimes()
 			}
-
-			db.EXPECT().GetWorkspaceAgentByID(gomock.Any(), agentID).
-				Return(disconnectedAgent, nil).
-				Times(2)
-			db.EXPECT().GetWorkspaceAgentsInLatestBuildByWorkspaceID(gomock.Any(), workspaceID).
-				Return([]database.WorkspaceAgent{disconnectedAgent}, nil).
+			db.EXPECT().GetWorkspaceAgentByID(gomock.Any(), boundAgent.ID).
+				Return(boundAgent, nil).
 				Times(1)
+			db.EXPECT().GetWorkspaceAgentByID(gomock.Any(), latestAgent.ID).
+				Return(latestAgent, nil).
+				Times(1)
+			db.EXPECT().GetWorkspaceAgentsInLatestBuildByWorkspaceID(gomock.Any(), workspaceID).
+				Return([]database.WorkspaceAgent{latestAgent}, nil).
+				Times(latestAgentLookups)

 			server := &Server{
 				db:                             db,
@@ -2648,11 +2696,12 @@ func TestGetWorkspaceConn_DialTimeoutDisconnectedRecoveryThreshold(t *testing.T)
 			}
 			require.Nil(t, result.conn)
 			require.ErrorIs(t, result.err, tc.wantErr)
-			if tc.wantRecovery {
-				require.ErrorIs(t, result.err, errChatAgentDisconnected)
-			} else {
+			if !xerrors.Is(tc.wantErr, errChatAgentDisconnected) {
 				require.NotErrorIs(t, result.err, errChatAgentDisconnected)
 			}
+			if !xerrors.Is(tc.wantErr, errChatAgentNeverConnected) {
+				require.NotErrorIs(t, result.err, errChatAgentNeverConnected)
+			}

 			workspaceCtx.mu.Lock()
 			defer workspaceCtx.mu.Unlock()
@@ -2880,70 +2929,6 @@ func TestGetWorkspaceConn_DialTimeout(t *testing.T) {
 	require.ErrorIs(t, err, errChatDialTimeout)
 }

-func TestGetWorkspaceConn_DialTimeoutStatusTimeoutDoesNotEscalate(t *testing.T) {
-	// Agents that never connected are startup failures, not
-	// disconnected recovery cases. A dial timeout should stay a
-	// retry/escalation error rather than stop/start guidance.
-	t.Parallel()
-
-	ctrl := gomock.NewController(t)
-	db := dbmock.NewMockStore(ctrl)
-
-	workspaceID := uuid.New()
-	agentID := uuid.New()
-	chat := database.Chat{
-		ID: uuid.New(),
-		WorkspaceID: uuid.NullUUID{
-			UUID:  workspaceID,
-			Valid: true,
-		},
-		AgentID: uuid.NullUUID{
-			UUID:  agentID,
-			Valid: true,
-		},
-	}
-
-	timedOutAgent := database.WorkspaceAgent{
-		ID:                       agentID,
-		CreatedAt:                time.Now().Add(-10 * time.Minute),
-		ConnectionTimeoutSeconds: 60,
-	}
-
-	db.EXPECT().GetWorkspaceAgentByID(gomock.Any(), agentID).
-		Return(timedOutAgent, nil).
-		Times(2)
-	db.EXPECT().GetWorkspaceAgentsInLatestBuildByWorkspaceID(gomock.Any(), workspaceID).
-		Return([]database.WorkspaceAgent{timedOutAgent}, nil).
-		Times(1)
-
-	server := &Server{
-		db:                             db,
-		clock:                          quartz.NewReal(),
-		agentInactiveDisconnectTimeout: 30 * time.Second,
-		dialTimeout:                    10 * time.Millisecond,
-	}
-	server.agentConnFn = func(ctx context.Context, _ uuid.UUID) (workspacesdk.AgentConn, func(), error) {
-		<-ctx.Done()
-		return nil, nil, ctx.Err()
-	}
-
-	chatStateMu := &sync.Mutex{}
-	currentChat := chat
-	workspaceCtx := turnWorkspaceContext{
-		server:           server,
-		chatStateMu:      chatStateMu,
-		currentChat:      &currentChat,
-		loadChatSnapshot: func(context.Context, uuid.UUID) (database.Chat, error) { return database.Chat{}, nil },
-	}
-	defer workspaceCtx.close()
-
-	ctx := testutil.Context(t, testutil.WaitShort)
-	gotConn, err := workspaceCtx.getWorkspaceConn(ctx)
-	require.Nil(t, gotConn)
-	require.ErrorIs(t, err, errChatDialTimeout)
-	require.NotErrorIs(t, err, errChatAgentDisconnected)
-}
-
 func TestGetWorkspaceConn_DialTimeoutParentCanceled(t *testing.T) {
 	// When the parent context is canceled, the parent's error
 	// must propagate unchanged (not wrapped as a dial timeout).