From 6f6d7539c88c6dd8cce89fd9878bd5f964198b23 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Mon, 13 Jul 2026 20:28:28 +0200 Subject: [PATCH] feat: remove unused chat statuses pending, paused, and completed (#27064) The chatd state machine only recognizes `waiting`, `running`, `error`, `requires_action`, and `interrupting`. Remove the unused `pending`, `paused`, and `completed` values from the database enum, backend, SDK, frontend, generated queries, and API docs. Migration `000543_chat_status_remove_unused` remaps existing `pending` rows to `running`, remaps `paused` and `completed` rows to `waiting`, drops the obsolete `idx_chats_pending` index, and recreates `chats_expanded` around the enum swap. It also removes the dead `AcquireChats` query and all remaining query literals for the deleted statuses. **NOTE**: The enum swap can break chat queries from older replicas during a mixed-version rollout because they still reference `'pending'::chat_status`. Chats are experimental, so this PR accepts that limited rollout window instead of adding a two-release expand and contract sequence. > This PR was authored by Mux (AI agent) on Mike's behalf. --- coderd/apidoc/docs.go | 6 - coderd/apidoc/swagger.json | 6 - coderd/coderdtest/chat.go | 2 +- coderd/database/dbauthz/dbauthz.go | 9 - coderd/database/dbauthz/dbauthz_test.go | 10 - coderd/database/dbmetrics/querymetrics.go | 8 - coderd/database/dbmock/dbmock.go | 15 -- coderd/database/dump.sql | 5 - .../000543_chat_status_remove_unused.down.sql | 5 + .../000543_chat_status_remove_unused.up.sql | 91 +++++++++ coderd/database/models.go | 9 - coderd/database/querier.go | 3 - coderd/database/querier_test.go | 16 +- coderd/database/queries.sql.go | 172 +----------------- coderd/database/queries/chats.sql | 97 +--------- coderd/exp_chats_test.go | 130 ++----------- coderd/telemetry/telemetry_test.go | 6 +- coderd/x/chatd/chatd.go | 3 - coderd/x/chatd/chatd_test.go | 88 ++------- coderd/x/chatd/chatstate/state.go | 5 +- coderd/x/chatd/integration_test.go | 3 +- coderd/x/chatd/quickgen.go | 4 - coderd/x/chatd/quickgen_internal_test.go | 1 - coderd/x/chatd/recording_internal_test.go | 4 +- coderd/x/chatd/subagent.go | 10 +- coderd/x/chatd/subagent_catalog.go | 4 +- coderd/x/chatd/turn_summary_internal_test.go | 85 --------- codersdk/chats.go | 3 - .../agents/tasks-to-chats-migration.md | 25 +-- docs/reference/api/chats.md | 2 +- docs/reference/api/schemas.md | 6 +- site/src/api/queries/chats.test.ts | 28 +-- site/src/api/typesGenerated.ts | 6 - .../AgentsPage/AgentChatPage.stories.tsx | 24 +-- .../pages/AgentsPage/AgentChatPage.test.ts | 4 +- site/src/pages/AgentsPage/AgentChatPage.tsx | 2 +- .../AgentsPage/AgentChatPageView.stories.tsx | 6 +- site/src/pages/AgentsPage/AgentsPage.tsx | 6 +- .../AgentsPage/AgentsPageView.stories.tsx | 6 +- .../chatStore.createStore.test.ts | 56 ++---- .../ChatConversation/chatStore.test.tsx | 48 ++--- .../components/ChatConversation/chatStore.ts | 23 +-- .../ChatConversation/useChatStore.ts | 11 +- .../ChatElements/tools/SubagentTool.tsx | 2 +- .../AgentsPage/components/ChatPageContent.tsx | 5 +- .../components/ChatTopBar.stories.tsx | 2 +- .../ChatsSidebar/ChatsSidebar.stories.tsx | 43 +---- .../dialogs/ChatSearchDialog.stories.tsx | 2 +- .../ChatsSidebar/tree/ChatTreeNode.tsx | 5 +- .../ChatsSidebar/tree/statusConfig.ts | 17 +- site/src/pages/AgentsPage/utils/chime.test.ts | 16 +- site/src/pages/AgentsPage/utils/chime.ts | 24 +-- site/src/testHelpers/chatEntities.ts | 2 +- 53 files changed, 293 insertions(+), 878 deletions(-) create mode 100644 coderd/database/migrations/000543_chat_status_remove_unused.down.sql create mode 100644 coderd/database/migrations/000543_chat_status_remove_unused.up.sql diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index ef440849cb..aae3e1f8a0 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -17695,20 +17695,14 @@ const docTemplate = `{ "type": "string", "enum": [ "waiting", - "pending", "running", - "paused", - "completed", "error", "requires_action", "interrupting" ], "x-enum-varnames": [ "ChatStatusWaiting", - "ChatStatusPending", "ChatStatusRunning", - "ChatStatusPaused", - "ChatStatusCompleted", "ChatStatusError", "ChatStatusRequiresAction", "ChatStatusInterrupting" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index f1cd626e60..7f170c96ea 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -15944,20 +15944,14 @@ "type": "string", "enum": [ "waiting", - "pending", "running", - "paused", - "completed", "error", "requires_action", "interrupting" ], "x-enum-varnames": [ "ChatStatusWaiting", - "ChatStatusPending", "ChatStatusRunning", - "ChatStatusPaused", - "ChatStatusCompleted", "ChatStatusError", "ChatStatusRequiresAction", "ChatStatusInterrupting" diff --git a/coderd/coderdtest/chat.go b/coderd/coderdtest/chat.go index acaa7352e9..3f67c7d0ac 100644 --- a/coderd/coderdtest/chat.go +++ b/coderd/coderdtest/chat.go @@ -117,7 +117,7 @@ func waitForChatTerminalState( if err != nil { return false } - return chat.Status != database.ChatStatusPending && chat.Status != database.ChatStatusRunning + return chat.Status != database.ChatStatusRunning }, testutil.WaitLong, testutil.IntervalFast) } diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 10f6bcd855..4d23b86a09 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1669,15 +1669,6 @@ func scopedOrgRoleIdentifiers(names []string, orgID uuid.UUID) []rbac.RoleIdenti return out } -func (q *querier) AcquireChats(ctx context.Context, arg database.AcquireChatsParams) ([]database.Chat, error) { - // AcquireChats is a system-level operation used by the chat processor. - // Authorization is done at the system level, not per-user. - if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceChat); err != nil { - return nil, err - } - return q.db.AcquireChats(ctx, arg) -} - func (q *querier) AcquireLock(ctx context.Context, id int64) error { return q.db.AcquireLock(ctx, id) } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 67c7cee24c..d8fa56e6d7 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -546,16 +546,6 @@ func (s *MethodTestSuite) TestConnectionLogs() { } func (s *MethodTestSuite) TestChats() { - s.Run("AcquireChats", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { - arg := database.AcquireChatsParams{ - StartedAt: dbtime.Now(), - WorkerID: uuid.New(), - NumChats: 1, - } - chat := testutil.Fake(s.T(), faker, database.Chat{}) - dbm.EXPECT().AcquireChats(gomock.Any(), arg).Return([]database.Chat{chat}, nil).AnyTimes() - check.Args(arg).Asserts(rbac.ResourceChat, policy.ActionUpdate).Returns([]database.Chat{chat}) - })) s.Run("HydrateAgentChatsContext", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { arg := database.HydrateAgentChatsContextParams{AgentID: uuid.New()} dbm.EXPECT().HydrateAgentChatsContext(gomock.Any(), arg).Return(nil).AnyTimes() diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index a2592c270e..b8b5397a7e 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -105,14 +105,6 @@ func (m queryMetricsStore) DeleteOrganization(ctx context.Context, id uuid.UUID) return r0 } -func (m queryMetricsStore) AcquireChats(ctx context.Context, arg database.AcquireChatsParams) ([]database.Chat, error) { - start := time.Now() - r0, r1 := m.s.AcquireChats(ctx, arg) - m.queryLatencies.WithLabelValues("AcquireChats").Observe(time.Since(start).Seconds()) - m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "AcquireChats").Inc() - return r0, r1 -} - func (m queryMetricsStore) AcquireLock(ctx context.Context, pgAdvisoryXactLock int64) error { start := time.Now() r0 := m.s.AcquireLock(ctx, pgAdvisoryXactLock) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index ba2884a0d4..f56b257f3a 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -45,21 +45,6 @@ func (m *MockStore) EXPECT() *MockStoreMockRecorder { return m.recorder } -// AcquireChats mocks base method. -func (m *MockStore) AcquireChats(ctx context.Context, arg database.AcquireChatsParams) ([]database.Chat, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AcquireChats", ctx, arg) - ret0, _ := ret[0].([]database.Chat) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// AcquireChats indicates an expected call of AcquireChats. -func (mr *MockStoreMockRecorder) AcquireChats(ctx, arg any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AcquireChats", reflect.TypeOf((*MockStore)(nil).AcquireChats), ctx, arg) -} - // AcquireLock mocks base method. func (m *MockStore) AcquireLock(ctx context.Context, pgAdvisoryXactLock int64) error { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index fae68a1e3e..8c9855b2ed 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -362,10 +362,7 @@ CREATE TYPE chat_reasoning_effort AS ENUM ( CREATE TYPE chat_status AS ENUM ( 'waiting', - 'pending', 'running', - 'paused', - 'completed', 'error', 'requires_action', 'interrupting' @@ -4763,8 +4760,6 @@ CREATE INDEX idx_chats_owner ON chats USING btree (owner_id); CREATE INDEX idx_chats_parent_chat_id ON chats USING btree (parent_chat_id); -CREATE INDEX idx_chats_pending ON chats USING btree (status) WHERE (status = 'pending'::chat_status); - CREATE INDEX idx_chats_root_chat_id ON chats USING btree (root_chat_id); CREATE INDEX idx_chats_worker_acquisition_candidates ON chats USING btree (status, updated_at, id) WHERE (archived = false); diff --git a/coderd/database/migrations/000543_chat_status_remove_unused.down.sql b/coderd/database/migrations/000543_chat_status_remove_unused.down.sql new file mode 100644 index 0000000000..7d2e83454f --- /dev/null +++ b/coderd/database/migrations/000543_chat_status_remove_unused.down.sql @@ -0,0 +1,5 @@ +-- No-op: the removed enum values are not restored, matching prior art such +-- as 000377 and 000384. Restoring them would require another +-- rename-create-cast-drop cycle, and the data cannot be restored anyway: +-- rows remapped to 'running' or 'waiting' by the up migration keep their +-- new status. diff --git a/coderd/database/migrations/000543_chat_status_remove_unused.up.sql b/coderd/database/migrations/000543_chat_status_remove_unused.up.sql new file mode 100644 index 0000000000..2e77366f60 --- /dev/null +++ b/coderd/database/migrations/000543_chat_status_remove_unused.up.sql @@ -0,0 +1,91 @@ +-- Remove legacy chat statuses that the chatd state machine treats as +-- invalid. 'pending', 'paused', and 'completed' are never written by the +-- backend anymore; the valid set is exactly what the state machine +-- recognizes: waiting, running, error, requires_action, interrupting. + +-- Remap any historical rows to the closest valid status. The column type +-- is still the original chat_status here. +-- +-- 'pending' meant queued work that no runner had picked up yet, so remap +-- it to 'running': the worker acquisition query picks up 'running' chats +-- without a worker and services them. +UPDATE chats SET status = 'running' +WHERE status = 'pending'; + +-- 'paused' and 'completed' were settled states; 'waiting' is the idle +-- resting state and the column default. +UPDATE chats SET status = 'waiting' +WHERE status IN ('paused', 'completed'); + +-- The partial index's WHERE clause references 'pending', which is being +-- removed. The index is obsolete now that the legacy AcquireChats query +-- is gone. +DROP INDEX idx_chats_pending; + +-- The view selects c.status, so it must be dropped before the column's +-- type can be altered. It is recreated verbatim below. +DROP VIEW chats_expanded; + +-- Recreate the enum without the removed values using the +-- rename-create-cast-drop pattern. +ALTER TYPE chat_status RENAME TO chat_status_old; +CREATE TYPE chat_status AS ENUM ( + 'waiting', + 'running', + 'error', + 'requires_action', + 'interrupting' +); +ALTER TABLE chats ALTER COLUMN status DROP DEFAULT; +ALTER TABLE chats ALTER COLUMN status TYPE chat_status USING status::text::chat_status; +ALTER TABLE chats ALTER COLUMN status SET DEFAULT 'waiting'; +DROP TYPE chat_status_old; + +CREATE VIEW chats_expanded AS + SELECT c.id, + c.owner_id, + c.workspace_id, + c.title, + c.status, + c.worker_id, + c.started_at, + c.heartbeat_at, + c.created_at, + c.updated_at, + c.parent_chat_id, + c.root_chat_id, + c.last_model_config_id, + c.last_reasoning_effort, + c.archived, + c.last_error, + c.mode, + c.mcp_server_ids, + c.labels, + c.build_id, + c.agent_id, + c.pin_order, + c.last_read_message_id, + c.dynamic_tools, + c.organization_id, + c.plan_mode, + c.client_type, + c.last_turn_summary, + c.snapshot_version, + c.history_version, + c.queue_version, + c.generation_attempt, + c.retry_state, + c.retry_state_version, + c.runner_id, + c.requires_action_deadline_at, + COALESCE(root.user_acl, c.user_acl) AS user_acl, + COALESCE(root.group_acl, c.group_acl) AS group_acl, + owner.username AS owner_username, + owner.name AS owner_name, + c.context_aggregate_hash, + c.context_dirty_since, + c.context_dirty_resources, + c.context_error + FROM ((chats c + LEFT JOIN chats root ON ((root.id = COALESCE(c.root_chat_id, c.parent_chat_id)))) + JOIN visible_users owner ON ((owner.id = c.owner_id))); diff --git a/coderd/database/models.go b/coderd/database/models.go index 0018fe1efe..7edc3e6f4c 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1725,10 +1725,7 @@ type ChatStatus string const ( ChatStatusWaiting ChatStatus = "waiting" - ChatStatusPending ChatStatus = "pending" ChatStatusRunning ChatStatus = "running" - ChatStatusPaused ChatStatus = "paused" - ChatStatusCompleted ChatStatus = "completed" ChatStatusError ChatStatus = "error" ChatStatusRequiresAction ChatStatus = "requires_action" ChatStatusInterrupting ChatStatus = "interrupting" @@ -1772,10 +1769,7 @@ func (ns NullChatStatus) Value() (driver.Value, error) { func (e ChatStatus) Valid() bool { switch e { case ChatStatusWaiting, - ChatStatusPending, ChatStatusRunning, - ChatStatusPaused, - ChatStatusCompleted, ChatStatusError, ChatStatusRequiresAction, ChatStatusInterrupting: @@ -1787,10 +1781,7 @@ func (e ChatStatus) Valid() bool { func AllChatStatusValues() []ChatStatus { return []ChatStatus{ ChatStatusWaiting, - ChatStatusPending, ChatStatusRunning, - ChatStatusPaused, - ChatStatusCompleted, ChatStatusError, ChatStatusRequiresAction, ChatStatusInterrupting, diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 218688f7b4..3d5ae5ee62 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -13,9 +13,6 @@ import ( ) type sqlcQuerier interface { - // Acquires up to @num_chats pending chats for processing. Uses SKIP LOCKED - // to prevent multiple replicas from acquiring the same chat. - AcquireChats(ctx context.Context, arg AcquireChatsParams) ([]Chat, error) // Blocks until the lock is acquired. // // This must be called from within a transaction. The lock will be automatically diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 008b6ef4db..9e709f590b 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -1280,11 +1280,11 @@ func TestChatContextHydration(t *testing.T) { hashH := []byte{0x01, 0x02, 0x03} hashOther := []byte{0xff, 0xee} - chatNull := newChat(database.ChatStatusWaiting, agent.ID) // never hydrated - chatMatch := newChat(database.ChatStatusRunning, agent.ID) // already at hashH - chatDrift := newChat(database.ChatStatusRunning, agent.ID) // drifted, active - chatTerminal := newChat(database.ChatStatusCompleted, agent.ID) // drifted, terminal - chatArchived := newChat(database.ChatStatusRunning, agent.ID) // drifted, archived + chatNull := newChat(database.ChatStatusWaiting, agent.ID) // never hydrated + chatMatch := newChat(database.ChatStatusRunning, agent.ID) // already at hashH + chatDrift := newChat(database.ChatStatusRunning, agent.ID) // drifted, active + chatTerminal := newChat(database.ChatStatusError, agent.ID) // drifted, terminal + chatArchived := newChat(database.ChatStatusRunning, agent.ID) // drifted, archived chatOtherAgent := newChat(database.ChatStatusRunning, otherAgent.ID) // Pin starting hashes; chatNull is intentionally left NULL. @@ -13019,7 +13019,7 @@ func TestChatPinOrderConstraints(t *testing.T) { parent, err := db.InsertChat(ctx, database.InsertChatParams{ OrganizationID: org.ID, - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, ClientType: database.ChatClientTypeUi, OwnerID: owner.ID, LastModelConfigID: modelCfg.ID, @@ -13029,7 +13029,7 @@ func TestChatPinOrderConstraints(t *testing.T) { child, err := db.InsertChat(ctx, database.InsertChatParams{ OrganizationID: org.ID, - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, ClientType: database.ChatClientTypeUi, OwnerID: owner.ID, LastModelConfigID: modelCfg.ID, @@ -13050,7 +13050,7 @@ func TestChatPinOrderConstraints(t *testing.T) { chat, err := db.InsertChat(ctx, database.InsertChatParams{ OrganizationID: org.ID, - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, ClientType: database.ChatClientTypeUi, OwnerID: owner.ID, LastModelConfigID: modelCfg.ID, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index cb2663638c..6bd29e7df5 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5606,165 +5606,6 @@ func (q *sqlQuerier) UpdateChatModelConfig(ctx context.Context, arg UpdateChatMo return i, err } -const acquireChats = `-- name: AcquireChats :many -WITH acquired_chats AS ( -UPDATE - chats -SET - status = 'running'::chat_status, - started_at = $1::timestamptz, - heartbeat_at = $1::timestamptz, - updated_at = $1::timestamptz, - worker_id = $2::uuid -WHERE - id = ANY( - SELECT - id - FROM - chats - WHERE - status = 'pending'::chat_status - AND archived = false - ORDER BY - updated_at ASC - FOR UPDATE - SKIP LOCKED - LIMIT - $3::int - ) -RETURNING id, owner_id, workspace_id, title, status, worker_id, started_at, heartbeat_at, created_at, updated_at, parent_chat_id, root_chat_id, last_model_config_id, archived, last_error, mode, mcp_server_ids, labels, build_id, agent_id, pin_order, last_read_message_id, dynamic_tools, organization_id, plan_mode, client_type, last_turn_summary, user_acl, group_acl, snapshot_version, history_version, queue_version, generation_attempt, retry_state, retry_state_version, runner_id, requires_action_deadline_at, context_aggregate_hash, context_dirty_since, context_dirty_resources, context_error, last_reasoning_effort -), -chats_expanded AS ( - SELECT - acquired_chats.id, - acquired_chats.owner_id, - acquired_chats.workspace_id, - acquired_chats.title, - acquired_chats.status, - acquired_chats.worker_id, - acquired_chats.started_at, - acquired_chats.heartbeat_at, - acquired_chats.created_at, - acquired_chats.updated_at, - acquired_chats.parent_chat_id, - acquired_chats.root_chat_id, - acquired_chats.last_model_config_id, - acquired_chats.last_reasoning_effort, - acquired_chats.archived, - acquired_chats.last_error, - acquired_chats.mode, - acquired_chats.mcp_server_ids, - acquired_chats.labels, - acquired_chats.build_id, - acquired_chats.agent_id, - acquired_chats.pin_order, - acquired_chats.last_read_message_id, - acquired_chats.dynamic_tools, - acquired_chats.organization_id, - acquired_chats.plan_mode, - acquired_chats.client_type, - acquired_chats.last_turn_summary, - acquired_chats.snapshot_version, - acquired_chats.history_version, - acquired_chats.queue_version, - acquired_chats.generation_attempt, - acquired_chats.retry_state, - acquired_chats.retry_state_version, - acquired_chats.runner_id, - acquired_chats.requires_action_deadline_at, - COALESCE(root.user_acl, acquired_chats.user_acl) AS user_acl, - COALESCE(root.group_acl, acquired_chats.group_acl) AS group_acl, - owner.username AS owner_username, - owner.name AS owner_name, - acquired_chats.context_aggregate_hash, - acquired_chats.context_dirty_since, - acquired_chats.context_dirty_resources, - acquired_chats.context_error - FROM - acquired_chats - LEFT JOIN chats root ON root.id = COALESCE(acquired_chats.root_chat_id, acquired_chats.parent_chat_id) - JOIN visible_users owner ON owner.id = acquired_chats.owner_id -) -SELECT id, owner_id, workspace_id, title, status, worker_id, started_at, heartbeat_at, created_at, updated_at, parent_chat_id, root_chat_id, last_model_config_id, last_reasoning_effort, archived, last_error, mode, mcp_server_ids, labels, build_id, agent_id, pin_order, last_read_message_id, dynamic_tools, organization_id, plan_mode, client_type, last_turn_summary, snapshot_version, history_version, queue_version, generation_attempt, retry_state, retry_state_version, runner_id, requires_action_deadline_at, user_acl, group_acl, owner_username, owner_name, context_aggregate_hash, context_dirty_since, context_dirty_resources, context_error -FROM chats_expanded -` - -type AcquireChatsParams struct { - StartedAt time.Time `db:"started_at" json:"started_at"` - WorkerID uuid.UUID `db:"worker_id" json:"worker_id"` - NumChats int32 `db:"num_chats" json:"num_chats"` -} - -// Acquires up to @num_chats pending chats for processing. Uses SKIP LOCKED -// to prevent multiple replicas from acquiring the same chat. -func (q *sqlQuerier) AcquireChats(ctx context.Context, arg AcquireChatsParams) ([]Chat, error) { - rows, err := q.db.QueryContext(ctx, acquireChats, arg.StartedAt, arg.WorkerID, arg.NumChats) - if err != nil { - return nil, err - } - defer rows.Close() - var items []Chat - for rows.Next() { - var i Chat - if err := rows.Scan( - &i.ID, - &i.OwnerID, - &i.WorkspaceID, - &i.Title, - &i.Status, - &i.WorkerID, - &i.StartedAt, - &i.HeartbeatAt, - &i.CreatedAt, - &i.UpdatedAt, - &i.ParentChatID, - &i.RootChatID, - &i.LastModelConfigID, - &i.LastReasoningEffort, - &i.Archived, - &i.LastError, - &i.Mode, - pq.Array(&i.MCPServerIDs), - &i.Labels, - &i.BuildID, - &i.AgentID, - &i.PinOrder, - &i.LastReadMessageID, - &i.DynamicTools, - &i.OrganizationID, - &i.PlanMode, - &i.ClientType, - &i.LastTurnSummary, - &i.SnapshotVersion, - &i.HistoryVersion, - &i.QueueVersion, - &i.GenerationAttempt, - &i.RetryState, - &i.RetryStateVersion, - &i.RunnerID, - &i.RequiresActionDeadlineAt, - &i.UserACL, - &i.GroupACL, - &i.OwnerUsername, - &i.OwnerName, - &i.ContextAggregateHash, - &i.ContextDirtySince, - &i.ContextDirtyResources, - &i.ContextError, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const acquireStaleChatDiffStatuses = `-- name: AcquireStaleChatDiffStatuses :many WITH acquired AS ( UPDATE @@ -6036,7 +5877,7 @@ WITH to_archive AS ( -- Redundant filter helps the planner use the partial index on created_at. AND c.created_at < $1::timestamptz -- New active statuses must be added here to prevent archiving. - AND c.status NOT IN ('running', 'pending', 'paused', 'requires_action') + AND c.status NOT IN ('running', 'requires_action') AND COALESCE(activity.last_activity_at, c.created_at) < $1::timestamptz -- Sorting by created_at lets Postgres drive the scan from the -- partial index instead of evaluating every LATERAL subquery @@ -6443,10 +6284,9 @@ SELECT id, owner_id, workspace_id, title, status, worker_id, started_at, heartbe FROM chats_expanded WHERE agent_id = $1::uuid AND archived = false - -- Active statuses only: waiting, pending, running, paused, - -- requires_action. - -- Excludes completed and error (terminal states). - AND status IN ('waiting', 'running', 'paused', 'pending', 'requires_action') + -- Active statuses only: waiting, running, requires_action. + -- Excludes error (terminal state) and interrupting. + AND status IN ('waiting', 'running', 'requires_action') ORDER BY updated_at DESC ` @@ -6538,8 +6378,6 @@ WHERE AND chats_expanded.status NOT IN ( 'running'::chat_status, 'interrupting'::chat_status, - 'pending'::chat_status, - 'paused'::chat_status, 'requires_action'::chat_status ) AND COALESCE(activity.last_activity_at, chats_expanded.created_at) < $1::timestamptz @@ -10400,7 +10238,7 @@ UPDATE chats SET context_dirty_since = $1 WHERE agent_id = $2::uuid AND archived = false - AND status IN ('waiting', 'running', 'paused', 'pending', 'requires_action') + AND status IN ('waiting', 'running', 'requires_action') AND context_aggregate_hash IS NOT NULL AND context_aggregate_hash IS DISTINCT FROM $3 AND context_dirty_since IS NULL diff --git a/coderd/database/queries/chats.sql b/coderd/database/queries/chats.sql index 97d2eeb9f7..4cdd2f9037 100644 --- a/coderd/database/queries/chats.sql +++ b/coderd/database/queries/chats.sql @@ -1469,7 +1469,7 @@ UPDATE chats SET context_dirty_since = @dirty_since WHERE agent_id = @agent_id::uuid AND archived = false - AND status IN ('waiting', 'running', 'paused', 'pending', 'requires_action') + AND status IN ('waiting', 'running', 'requires_action') AND context_aggregate_hash IS NOT NULL AND context_aggregate_hash IS DISTINCT FROM @aggregate_hash AND context_dirty_since IS NULL @@ -1540,90 +1540,6 @@ SELECT (SELECT COUNT(*)::int FROM genuinely_new) - (SELECT COUNT(*)::int FROM inserted) AS rejected_new_files; --- name: AcquireChats :many --- Acquires up to @num_chats pending chats for processing. Uses SKIP LOCKED --- to prevent multiple replicas from acquiring the same chat. -WITH acquired_chats AS ( -UPDATE - chats -SET - status = 'running'::chat_status, - started_at = @started_at::timestamptz, - heartbeat_at = @started_at::timestamptz, - updated_at = @started_at::timestamptz, - worker_id = @worker_id::uuid -WHERE - id = ANY( - SELECT - id - FROM - chats - WHERE - status = 'pending'::chat_status - AND archived = false - ORDER BY - updated_at ASC - FOR UPDATE - SKIP LOCKED - LIMIT - @num_chats::int - ) -RETURNING * -), -chats_expanded AS ( - SELECT - acquired_chats.id, - acquired_chats.owner_id, - acquired_chats.workspace_id, - acquired_chats.title, - acquired_chats.status, - acquired_chats.worker_id, - acquired_chats.started_at, - acquired_chats.heartbeat_at, - acquired_chats.created_at, - acquired_chats.updated_at, - acquired_chats.parent_chat_id, - acquired_chats.root_chat_id, - acquired_chats.last_model_config_id, - acquired_chats.last_reasoning_effort, - acquired_chats.archived, - acquired_chats.last_error, - acquired_chats.mode, - acquired_chats.mcp_server_ids, - acquired_chats.labels, - acquired_chats.build_id, - acquired_chats.agent_id, - acquired_chats.pin_order, - acquired_chats.last_read_message_id, - acquired_chats.dynamic_tools, - acquired_chats.organization_id, - acquired_chats.plan_mode, - acquired_chats.client_type, - acquired_chats.last_turn_summary, - acquired_chats.snapshot_version, - acquired_chats.history_version, - acquired_chats.queue_version, - acquired_chats.generation_attempt, - acquired_chats.retry_state, - acquired_chats.retry_state_version, - acquired_chats.runner_id, - acquired_chats.requires_action_deadline_at, - COALESCE(root.user_acl, acquired_chats.user_acl) AS user_acl, - COALESCE(root.group_acl, acquired_chats.group_acl) AS group_acl, - owner.username AS owner_username, - owner.name AS owner_name, - acquired_chats.context_aggregate_hash, - acquired_chats.context_dirty_since, - acquired_chats.context_dirty_resources, - acquired_chats.context_error - FROM - acquired_chats - LEFT JOIN chats root ON root.id = COALESCE(acquired_chats.root_chat_id, acquired_chats.parent_chat_id) - JOIN visible_users owner ON owner.id = acquired_chats.owner_id -) -SELECT * -FROM chats_expanded; - -- name: UpdateChatStatus :one WITH updated_chat AS ( UPDATE @@ -2534,10 +2450,9 @@ SELECT * FROM chats_expanded WHERE agent_id = @agent_id::uuid AND archived = false - -- Active statuses only: waiting, pending, running, paused, - -- requires_action. - -- Excludes completed and error (terminal states). - AND status IN ('waiting', 'running', 'paused', 'pending', 'requires_action') + -- Active statuses only: waiting, running, requires_action. + -- Excludes error (terminal state) and interrupting. + AND status IN ('waiting', 'running', 'requires_action') ORDER BY updated_at DESC; -- name: SoftDeleteContextFileMessages :exec @@ -2630,8 +2545,6 @@ WHERE AND chats_expanded.status NOT IN ( 'running'::chat_status, 'interrupting'::chat_status, - 'pending'::chat_status, - 'paused'::chat_status, 'requires_action'::chat_status ) AND COALESCE(activity.last_activity_at, chats_expanded.created_at) < @archive_cutoff::timestamptz @@ -2999,7 +2912,7 @@ WITH to_archive AS ( -- Redundant filter helps the planner use the partial index on created_at. AND c.created_at < @archive_cutoff::timestamptz -- New active statuses must be added here to prevent archiving. - AND c.status NOT IN ('running', 'pending', 'paused', 'requires_action') + AND c.status NOT IN ('running', 'requires_action') AND COALESCE(activity.last_activity_at, c.created_at) < @archive_cutoff::timestamptz -- Sorting by created_at lets Postgres drive the scan from the -- partial index instead of evaluating every LATERAL subquery diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index eb2ec800a5..48418a4666 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -1069,15 +1069,13 @@ func TestListChats(t *testing.T) { require.Equal(t, firstUser.UserID, chat.OwnerID) require.Equal(t, modelConfig.ID, chat.LastModelConfigID) - // The chat may have been picked up by the background - // processor (via signalWake) before we list, so - // accept any active status. + // The chat may have been picked up by the chat worker + // before we list, so accept any status it may have + // reached by now. require.Contains(t, []codersdk.ChatStatus{ - codersdk.ChatStatusPending, codersdk.ChatStatusRunning, codersdk.ChatStatusError, codersdk.ChatStatusWaiting, - codersdk.ChatStatusCompleted, }, chat.Status, "unexpected chat status: %s", chat.Status) require.NotZero(t, chat.CreatedAt) require.NotZero(t, chat.UpdatedAt) @@ -1100,9 +1098,8 @@ func TestListChats(t *testing.T) { // The list is already verified as sorted by UpdatedAt // descending (loop above). We intentionally do NOT // compare positions using the creation-time UpdatedAt - // values because signalWake() may trigger background - // processing that mutates UpdatedAt between CreateChat - // and ListChats. + // values because the chat worker may pick up a chat and + // mutate UpdatedAt between CreateChat and ListChats. memberChats, err := memberClient.ListChats(ctx, nil) require.NoError(t, err) @@ -1133,21 +1130,21 @@ func TestListChats(t *testing.T) { OwnerID: owner.ID, LastModelConfigID: modelConfig.ID, Title: "owner created chat", - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) sharedChat := dbgen.Chat(t, db, database.Chat{ OrganizationID: firstUser.OrganizationID, OwnerID: member.ID, LastModelConfigID: modelConfig.ID, Title: "member shared chat", - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) unsharedReadableChat := dbgen.Chat(t, db, database.Chat{ OrganizationID: firstUser.OrganizationID, OwnerID: firstUser.UserID, LastModelConfigID: modelConfig.ID, Title: "unshared readable chat", - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) err := db.UpdateChatACLByID(dbauthz.As(ctx, rbac.Subject{ @@ -1268,7 +1265,7 @@ func TestListChats(t *testing.T) { OwnerID: firstUser.UserID, LastModelConfigID: modelConfig.ID, Title: fmt.Sprintf("chat-%d", i), - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) createdChatIDs = append(createdChatIDs, dbChat.ID) } @@ -1351,7 +1348,7 @@ func TestListChats(t *testing.T) { OwnerID: firstUser.UserID, LastModelConfigID: modelConfig.ID, Title: "pinned-chat", - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) // Fill page 1 with newer chats so the pinned chat @@ -1364,7 +1361,7 @@ func TestListChats(t *testing.T) { OwnerID: firstUser.UserID, LastModelConfigID: modelConfig.ID, Title: fmt.Sprintf("filler-%d", i), - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) } @@ -1411,7 +1408,7 @@ func TestListChats(t *testing.T) { OwnerID: firstUser.UserID, LastModelConfigID: modelConfig.ID, Title: fmt.Sprintf("cursor-pin-chat-%d", i), - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) createdChatIDs = append(createdChatIDs, dbChat.ID) } @@ -6552,7 +6549,7 @@ func TestChatPinOrder(t *testing.T) { OwnerID: firstUser.UserID, LastModelConfigID: modelConfig.ID, Title: "child chat", - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, ParentChatID: uuid.NullUUID{UUID: parentChat.ID, Valid: true}, RootChatID: uuid.NullUUID{UUID: parentChat.ID, Valid: true}, }) @@ -8659,8 +8656,7 @@ func TestPatchChatMessage(t *testing.T) { if getErr != nil { return false } - return c.Status != codersdk.ChatStatusPending && - c.Status != codersdk.ChatStatusRunning + return c.Status != codersdk.ChatStatusRunning }, testutil.IntervalFast, "initial chat processing did not finish") messagesResult, err := client.GetChatMessages(ctx, chat.ID, nil) @@ -8696,8 +8692,7 @@ func TestPatchChatMessage(t *testing.T) { if getErr != nil { return false } - return c.Status != codersdk.ChatStatusPending && - c.Status != codersdk.ChatStatusRunning + return c.Status != codersdk.ChatStatusRunning }, testutil.IntervalFast, "post-edit chat processing did not finish") updatedChat, err := client.GetChat(ctx, chat.ID) @@ -8997,7 +8992,7 @@ func TestRegenerateChatTitle(t *testing.T) { _, err = db.UpdateChatStatus(dbauthz.AsSystemRestricted(ctx), database.UpdateChatStatusParams{ ID: chat.ID, - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, @@ -9020,47 +9015,6 @@ func TestRegenerateChatTitle(t *testing.T) { ) }) - t.Run("PendingWithoutWorker", func(t *testing.T) { - t.Parallel() - - ctx := testutil.Context(t, testutil.WaitLong) - client, db := newChatClientWithDatabase(t) - user := coderdtest.CreateFirstUser(t, client.Client) - modelConfig := createTitleGenerationModelConfig(t, client) - - chat := dbgen.Chat(t, db, database.Chat{ - OrganizationID: user.OrganizationID, - OwnerID: user.UserID, - LastModelConfigID: modelConfig.ID, - Title: "pending chat without worker", - }) - seedManualTitleSourceMessage(t, db, chat, modelConfig.ID) - - var err error - chat, err = db.UpdateChatStatus(dbauthz.AsSystemRestricted(ctx), database.UpdateChatStatusParams{ - ID: chat.ID, - Status: database.ChatStatusPending, - WorkerID: uuid.NullUUID{}, - StartedAt: sql.NullTime{}, - HeartbeatAt: sql.NullTime{}, - LastError: pqtype.NullRawMessage{}, - }) - require.NoError(t, err) - - // Pending chats are never acquired - // (GetChatWorkerAcquisitionCandidates excludes the status), so - // manual title regeneration must still proceed. - updated, err := client.RegenerateChatTitle(ctx, chat.ID) - require.NoError(t, err) - require.Equal(t, "Test Chat", updated.Title) - - persisted, err := db.GetChatByID(dbauthz.AsSystemRestricted(ctx), chat.ID) - require.NoError(t, err) - require.Equal(t, "Test Chat", persisted.Title) - require.Equal(t, database.ChatStatusPending, persisted.Status) - require.False(t, persisted.WorkerID.Valid) - }) - t.Run("PasteOnlyChat", func(t *testing.T) { t.Parallel() @@ -9074,7 +9028,7 @@ func TestRegenerateChatTitle(t *testing.T) { OwnerID: user.UserID, LastModelConfigID: modelConfig.ID, Title: "New Chat", - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) // The chat's only user message is a synthetic pasted-text // attachment with no text parts. @@ -9158,7 +9112,7 @@ func TestRegenerateChatTitle(t *testing.T) { _, err = db.UpdateChatStatus(dbauthz.AsSystemRestricted(ctx), database.UpdateChatStatusParams{ ID: chat.ID, - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, @@ -9247,52 +9201,6 @@ func TestProposeChatTitle(t *testing.T) { requireSDKError(t, err, http.StatusUnauthorized) }) - t.Run("PendingWithoutWorker", func(t *testing.T) { - t.Parallel() - - ctx := testutil.Context(t, testutil.WaitLong) - client, db := newChatClientWithDatabase(t) - user := coderdtest.CreateFirstUser(t, client.Client) - modelConfig := createTitleGenerationModelConfig(t, client) - - chat := dbgen.Chat(t, db, database.Chat{ - OrganizationID: user.OrganizationID, - OwnerID: user.UserID, - LastModelConfigID: modelConfig.ID, - Title: "pending chat without worker", - }) - seedManualTitleSourceMessage(t, db, chat, modelConfig.ID) - - var err error - chat, err = db.UpdateChatStatus(dbauthz.AsSystemRestricted(ctx), database.UpdateChatStatusParams{ - ID: chat.ID, - Status: database.ChatStatusPending, - WorkerID: uuid.NullUUID{}, - StartedAt: sql.NullTime{}, - HeartbeatAt: sql.NullTime{}, - LastError: pqtype.NullRawMessage{}, - }) - require.NoError(t, err) - - before, err := db.GetChatByID(dbauthz.AsSystemRestricted(ctx), chat.ID) - require.NoError(t, err) - - // Pending chats are never acquired - // (GetChatWorkerAcquisitionCandidates excludes the status), so - // title proposal must still proceed. - resp, err := client.ProposeChatTitle(ctx, chat.ID) - require.NoError(t, err) - require.Equal(t, "Test Chat", resp.Title) - - persisted, err := db.GetChatByID(dbauthz.AsSystemRestricted(ctx), chat.ID) - require.NoError(t, err) - require.Equal(t, before.Title, persisted.Title, - "propose must not persist the suggested title") - require.Equal(t, database.ChatStatusPending, persisted.Status) - require.False(t, persisted.WorkerID.Valid) - require.True(t, persisted.UpdatedAt.Equal(before.UpdatedAt)) - }) - t.Run("DoesNotBumpHistoryVersion", func(t *testing.T) { t.Parallel() @@ -9467,7 +9375,7 @@ func TestManualTitleEndpointsPassCallerAPIKeyToAIGateway(t *testing.T) { OwnerID: firstUser.UserID, LastModelConfigID: modelConfig.ID, Title: "initial title", - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) content, err := chatprompt.MarshalParts([]codersdk.ChatMessagePart{ codersdk.ChatMessageText("manual title source"), diff --git a/coderd/telemetry/telemetry_test.go b/coderd/telemetry/telemetry_test.go index ef82029b01..e6f05e9f8e 100644 --- a/coderd/telemetry/telemetry_test.go +++ b/coderd/telemetry/telemetry_test.go @@ -1681,7 +1681,7 @@ func TestChatsTelemetry(t *testing.T) { OwnerID: user.ID, LastModelConfigID: modelCfg2.ID, Title: "Child Chat", - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, ParentChatID: uuid.NullUUID{UUID: rootChat.ID, Valid: true}, RootChatID: uuid.NullUUID{UUID: rootChat.ID, Valid: true}, }) @@ -1852,7 +1852,7 @@ func TestChatsTelemetry(t *testing.T) { require.NotNil(t, foundChild.RootChatID) assert.Equal(t, rootChat.ID, *foundChild.RootChatID) assert.Nil(t, foundChild.WorkspaceID) - assert.Equal(t, "completed", foundChild.Status) + assert.Equal(t, "waiting", foundChild.Status) assert.Equal(t, modelCfg2.ID, foundChild.LastModelConfigID) assert.Nil(t, foundChild.Mode) assert.False(t, foundChild.Archived) @@ -1969,7 +1969,7 @@ func TestChatDiffStatusSummaryTelemetry(t *testing.T) { OwnerID: user.ID, LastModelConfigID: modelCfg.ID, Title: "Chat " + state, - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, }) now := dbtime.Now() _, chatErr := db.UpsertChatDiffStatus(ctx, database.UpsertChatDiffStatusParams{ diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 3e4d17d944..1a3616d69e 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -4366,9 +4366,6 @@ func (p *Server) maybeFinalizeTurnStatusLabelAndPush( case database.ChatStatusWaiting: p.finalizeSuccessfulTurnStatusLabelAndPush(ctx, chat, status, runResult, logger) - case database.ChatStatusPending: - p.setLastTurnSummaryAsync(ctx, chat, fallbackTurnStatusLabel(status), logger) - case database.ChatStatusError: p.clearLastTurnSummaryAsync(ctx, chat, logger) if p.webpushConfigured() { diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index d06972a138..7a6eae5fdc 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -1817,7 +1817,7 @@ func TestSendMessageRejectsInvalidQueuedModelConfigID(t *testing.T) { chat := dbgen.Chat(t, db, database.Chat{ OrganizationID: org.ID, - Status: database.ChatStatusPending, + Status: database.ChatStatusRunning, OwnerID: user.ID, LastModelConfigID: modelConfig.ID, Title: "reject invalid queued model config", @@ -2934,17 +2934,17 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) { require.Equal(t, wantPayload, requireChatLastErrorPayload(t, fromDB.LastError)) // Verify the error is cleared when the chat transitions to a - // non-error status (e.g. pending after a retry). + // non-error status (e.g. running after a retry). chat, err = db.UpdateChatStatus(ctx, database.UpdateChatStatusParams{ ID: chat.ID, - Status: database.ChatStatusPending, + Status: database.ChatStatusRunning, WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) - require.Equal(t, database.ChatStatusPending, chat.Status) + require.Equal(t, database.ChatStatusRunning, chat.Status) require.False(t, chat.LastError.Valid) fromDB, err = db.GetChatByID(ctx, chat.ID) @@ -5250,16 +5250,10 @@ func TestHeartbeatNoWorkspaceNoBump(t *testing.T) { require.Equal(t, 0, count, "expected no workspaces to be flushed when chat has no workspace") } -// waitForChatProcessed waits for a wake-triggered processOnce to -// fully complete for the given chat. It polls until the chat leaves -// both pending and running states (meaning processChat has finished -// its cleanup and updated the DB), then calls WaitUntilIdleForTest. -// -// Waiting for a terminal state (not just "not pending") avoids a -// WaitGroup Add/Wait race: AcquireChats changes the DB status to -// running before processOnce calls inflight.Add(1). If we only -// waited for status != pending, we could call Wait() while Add(1) -// hasn't happened yet. +// waitForChatProcessed waits for the chat worker to fully handle a +// wake for the given chat. It polls until the chat leaves the running +// state (the worker has finished the turn and updated the DB), then +// calls WaitUntilIdleForTest so tracked background work settles. func waitForChatProcessed( ctx context.Context, t *testing.T, @@ -5273,18 +5267,18 @@ func waitForChatProcessed( if err != nil { return false } - // Wait until the chat reaches a terminal state. Neither - // pending (waiting to be acquired) nor running (being - // processed). This guarantees that inflight.Add(1) has - // already been called by processOnce. - return c.Status != database.ChatStatusPending && - c.Status != database.ChatStatusRunning + // Wait until the chat reaches a settled state (not being + // processed). This guarantees the wake was picked up and its + // background work registered before WaitUntilIdleForTest + // checks for idleness. + return c.Status != database.ChatStatusRunning }, testutil.WaitShort, testutil.IntervalFast) chatd.WaitUntilIdleForTest(server) } -// newTestServer creates a passive server that never calls -// processOnce on its own. +// newTestServer creates a passive server whose periodic chat +// acquisition is effectively disabled, so chats are only processed in +// response to explicit wakes. func newTestServer( t *testing.T, db database.Store, @@ -8405,7 +8399,7 @@ func TestProposeChatTitle_DebugRun(t *testing.T) { chat := dbgen.Chat(t, db, database.Chat{ OrganizationID: org.ID, - Status: database.ChatStatusCompleted, + Status: database.ChatStatusWaiting, ClientType: database.ChatClientTypeUi, OwnerID: user.ID, Title: "original title", @@ -11823,54 +11817,6 @@ func TestPromoteQueuedPreservesReasoningEffort(t *testing.T) { require.Equal(t, database.ChatReasoningEffortHigh, storedChat.LastReasoningEffort.ChatReasoningEffort) } -// TestPromoteQueuedWhileRequiresActionMixedTools guards against -func TestAcquireChatsSkipsArchivedPendingChat(t *testing.T) { - t.Parallel() - - db, ps := dbtestutil.NewDB(t) - _ = newTestServer(t, db, ps, uuid.New()) - - ctx := testutil.Context(t, testutil.WaitLong) - user, org, model := seedChatDependencies(t, db) - - archivedChat := dbgen.Chat(t, db, database.Chat{ - OwnerID: user.ID, - OrganizationID: org.ID, - Title: "acquire-skip-archived", - LastModelConfigID: model.ID, - }) - - // Archive the chat, then force it to pending. - _, err := db.ArchiveChatByID(ctx, archivedChat.ID) - require.NoError(t, err) - - _, err = db.UpdateChatStatus(ctx, database.UpdateChatStatusParams{ - ID: archivedChat.ID, - Status: database.ChatStatusPending, - }) - require.NoError(t, err) - - // Insert a second, non-archived pending chat so the result - // slice is non-empty and the assertion is not vacuously true. - activeChat := dbgen.Chat(t, db, database.Chat{ - OwnerID: user.ID, - OrganizationID: org.ID, - Title: "acquire-active", - LastModelConfigID: model.ID, - Status: database.ChatStatusPending, - }) - - now := time.Now() - acquired, err := db.AcquireChats(ctx, database.AcquireChatsParams{ - WorkerID: uuid.New(), - StartedAt: now, - NumChats: 10, - }) - require.NoError(t, err) - require.Len(t, acquired, 1, "only the non-archived chat should be acquired") - require.Equal(t, activeChat.ID, acquired[0].ID) -} - // TestAdvisorGating_ExperimentDisabled verifies that the advisor tool is // not attached when the chat-advisor experiment is absent from the // experiments list, even if the DB-stored advisor config has Enabled=true. diff --git a/coderd/x/chatd/chatstate/state.go b/coderd/x/chatd/chatstate/state.go index 4d39d27329..180e8fbf98 100644 --- a/coderd/x/chatd/chatstate/state.go +++ b/coderd/x/chatd/chatstate/state.go @@ -113,9 +113,8 @@ func (s ExecutionState) QueueNonEmpty() bool { // // The classifier is a single flat switch over the valid (status, // archived, queue) tuples in the chat execution state model. Anything -// outside that set (legacy pending/paused/completed statuses, archived -// busy states, waiting with a non-empty queue, future enum values) -// falls through to [StateInvalid]. +// outside that set (archived busy states, waiting with a non-empty +// queue, future enum values) falls through to [StateInvalid]. // //nolint:revive // queueNonEmpty/exists are simple classifier inputs. func ClassifyExecutionState(chat database.Chat, queueNonEmpty, exists bool) ExecutionState { diff --git a/coderd/x/chatd/integration_test.go b/coderd/x/chatd/integration_test.go index 9bf30b4e3e..8fd89420dc 100644 --- a/coderd/x/chatd/integration_test.go +++ b/coderd/x/chatd/integration_test.go @@ -243,8 +243,7 @@ func waitForChatDone( if event.Status != nil { t.Logf("[%s] status → %s", label, event.Status.Status) switch event.Status.Status { - case codersdk.ChatStatusWaiting, - codersdk.ChatStatusCompleted: + case codersdk.ChatStatusWaiting: return case codersdk.ChatStatusError: require.FailNow(t, label+" ended with error status") diff --git a/coderd/x/chatd/quickgen.go b/coderd/x/chatd/quickgen.go index 47b6556746..4657a95d00 100644 --- a/coderd/x/chatd/quickgen.go +++ b/coderd/x/chatd/quickgen.go @@ -1087,8 +1087,6 @@ func turnStatusLabelStateContext(status database.ChatStatus) string { switch status { case database.ChatStatusWaiting: return "The turn finished and the chat is idle." - case database.ChatStatusPending: - return "Another user message is queued and the chat will continue." case database.ChatStatusRequiresAction: return "The chat is waiting for user input or action." case database.ChatStatusError: @@ -1102,8 +1100,6 @@ func fallbackTurnStatusLabel(status database.ChatStatus) string { switch status { case database.ChatStatusWaiting: return "Finished latest turn" - case database.ChatStatusPending: - return "Still working on request" case database.ChatStatusRequiresAction: return "Waiting for user input" case database.ChatStatusError: diff --git a/coderd/x/chatd/quickgen_internal_test.go b/coderd/x/chatd/quickgen_internal_test.go index d96eceec4c..530ec2cbd4 100644 --- a/coderd/x/chatd/quickgen_internal_test.go +++ b/coderd/x/chatd/quickgen_internal_test.go @@ -863,7 +863,6 @@ func TestFallbackTurnStatusLabel(t *testing.T) { want string }{ {status: database.ChatStatusWaiting, want: "Finished latest turn"}, - {status: database.ChatStatusPending, want: "Still working on request"}, {status: database.ChatStatusRequiresAction, want: "Waiting for user input"}, {status: database.ChatStatusError, want: "Hit an error"}, {status: database.ChatStatus("unknown"), want: "Updated chat status"}, diff --git a/coderd/x/chatd/recording_internal_test.go b/coderd/x/chatd/recording_internal_test.go index 8cf2b37d20..8d8b4d9b25 100644 --- a/coderd/x/chatd/recording_internal_test.go +++ b/coderd/x/chatd/recording_internal_test.go @@ -105,7 +105,7 @@ func createComputerUseParentChild( AgentID: uuid.NullUUID{UUID: agent.ID, Valid: true}, LastModelConfigID: model.ID, Title: parentTitle, - Status: database.ChatStatusPending, + Status: database.ChatStatusRunning, }) // Insert the child chat directly via DB to avoid triggering @@ -121,7 +121,7 @@ func createComputerUseParentChild( LastModelConfigID: model.ID, Title: childTitle, Mode: database.NullChatMode{ChatMode: database.ChatModeComputerUse, Valid: true}, - Status: database.ChatStatusPending, + Status: database.ChatStatusRunning, }) return parent, child diff --git a/coderd/x/chatd/subagent.go b/coderd/x/chatd/subagent.go index ccb7e1a26d..f5c2575ce7 100644 --- a/coderd/x/chatd/subagent.go +++ b/coderd/x/chatd/subagent.go @@ -800,8 +800,7 @@ func (p *Server) subagentTools( interrupted := false if args.Interrupt && targetChatInfo != nil { - interrupted = targetChatInfo.Status == database.ChatStatusRunning || - targetChatInfo.Status == database.ChatStatusPending + interrupted = targetChatInfo.Status == database.ChatStatusRunning } return toolJSONResponse(withSubagentType(map[string]any{ "chat_id": targetChat.ID.String(), @@ -862,8 +861,8 @@ func (p *Server) subagentTools( "sort order is best-effort: an agent's position may shift "+ "if its updated_at changes between calls. Each "+ "agent has chat_id, title, type, status, created_at, "+ - "updated_at. Status: pending/running = working, "+ - "interrupting = transient, waiting/completed = idle, "+ + "updated_at. Status: running = working, "+ + "interrupting = transient, waiting = idle, "+ "error = stopped on error.", func(ctx context.Context, args listAgentsArgs, _ fantasy.ToolCall) (fantasy.ToolResponse, error) { if currentChat == nil { @@ -1434,8 +1433,7 @@ func (p *Server) checkSubagentCompletion( // waiting (no queued messages) or running (queued messages). // Treat it as not-done so the agent settles before // classification, avoiding stale partial output. - if chat.Status == database.ChatStatusPending || - chat.Status == database.ChatStatusRunning || + if chat.Status == database.ChatStatusRunning || chat.Status == database.ChatStatusInterrupting { return chat, "", false, nil } diff --git a/coderd/x/chatd/subagent_catalog.go b/coderd/x/chatd/subagent_catalog.go index 59b7237a85..42893f741b 100644 --- a/coderd/x/chatd/subagent_catalog.go +++ b/coderd/x/chatd/subagent_catalog.go @@ -304,8 +304,8 @@ func buildSpawnAgentDescription( "After spawning, use wait_agent to retrieve the result. Agents persist " + "after completion; reuse an agent via message_agent for follow-up work " + "when it already has relevant context. Spawned agents are your " + - "responsibility: do not abandon one in a working state (pending or " + - "running); retrieve its result, redirect it with message_agent, or stop " + + "responsibility: do not abandon one in a working state (running); " + + "retrieve its result, redirect it with message_agent, or stop " + "it with interrupt_agent." if currentChat.PlanMode.Valid && currentChat.PlanMode.ChatPlanMode == database.ChatPlanModePlan { description += " During plan mode, type=\"" + subagentTypeGeneral + diff --git a/coderd/x/chatd/turn_summary_internal_test.go b/coderd/x/chatd/turn_summary_internal_test.go index d5d545b2ed..06f5fc3f69 100644 --- a/coderd/x/chatd/turn_summary_internal_test.go +++ b/coderd/x/chatd/turn_summary_internal_test.go @@ -7,7 +7,6 @@ import ( "sync/atomic" "testing" - "charm.land/fantasy" "github.com/google/uuid" "github.com/stretchr/testify/require" @@ -17,7 +16,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" "github.com/coder/coder/v2/coderd/x/chatd/chatstate" - "github.com/coder/coder/v2/coderd/x/chatd/chattest" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) @@ -116,89 +114,6 @@ func TestUpdateLastTurnSummaryRejectsStaleWrites(t *testing.T) { require.Equal(t, sql.NullString{String: "fresh summary", Valid: true}, fetched.LastTurnSummary) } -func TestPendingChatPersistsSummaryButSkipsWebPush(t *testing.T) { - t.Parallel() - - db, ps := dbtestutil.NewDB(t) - ctx := testutil.Context(t, testutil.WaitMedium) - owner := dbgen.User(t, db, database.User{}) - org := dbgen.Organization(t, db, database.Organization{}) - dbgen.OrganizationMember(t, db, database.OrganizationMember{ - UserID: owner.ID, - OrganizationID: org.ID, - }) - - provider := dbgen.ChatProvider(t, db, database.ChatProvider{ - Provider: "openai", - DisplayName: "OpenAI", - APIKey: "test-key", - Enabled: true, - }) - - modelCfg, err := db.InsertChatModelConfig(ctx, database.InsertChatModelConfigParams{ - AIProviderID: uuid.NullUUID{UUID: provider.ID, Valid: true}, - Model: "test-model", - DisplayName: "Test Model", - CreatedBy: uuid.NullUUID{UUID: owner.ID, Valid: true}, - UpdatedBy: uuid.NullUUID{UUID: owner.ID, Valid: true}, - Enabled: true, - IsDefault: true, - ContextLimit: 128000, - CompressionThreshold: 80, - Options: json.RawMessage(`{}`), - }) - require.NoError(t, err) - - chat, err := db.InsertChat(ctx, database.InsertChatParams{ - OrganizationID: org.ID, - Status: database.ChatStatusPending, - ClientType: database.ChatClientTypeUi, - OwnerID: owner.ID, - LastModelConfigID: modelCfg.ID, - Title: "summary-pending-chat", - }) - require.NoError(t, err) - - const summary = "Still working on request" - var generateCalls atomic.Int32 - model := &chattest.FakeModel{ - ProviderName: "openai", - ModelName: "test-model", - GenerateFn: func(_ context.Context, _ fantasy.Call) (*fantasy.Response, error) { - generateCalls.Add(1) - return &fantasy.Response{ - Content: fantasy.ResponseContent{ - fantasy.TextContent{Text: "Unexpected label"}, - }, - }, nil - }, - } - - dispatcher := &recordingWebpushDispatcher{} - logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) - server := &Server{ctx: t.Context(), db: db, pubsub: ps, webpushDispatcher: dispatcher} - server.maybeFinalizeTurnStatusLabelAndPush( - context.WithoutCancel(ctx), - chat, - database.ChatStatusPending, - "", - runChatResult{ - FinalAssistantText: "I finished the queued turn.", - StatusLabelModel: model, - FallbackProvider: model.Provider(), - FallbackModel: model.Model(), - }, - logger, - ) - server.drainInflight() - - fetched, err := db.GetChatByID(ctx, chat.ID) - require.NoError(t, err) - require.Equal(t, sql.NullString{String: summary, Valid: true}, fetched.LastTurnSummary) - require.Equal(t, int32(0), generateCalls.Load()) - require.Equal(t, int32(0), dispatcher.dispatchCount.Load()) -} - func TestSuccessfulChildChatOutcomeSkipsSummaryAndWebPush(t *testing.T) { t.Parallel() diff --git a/codersdk/chats.go b/codersdk/chats.go index 6e3664c717..9e62d6f73a 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -88,10 +88,7 @@ type ChatStatus string const ( ChatStatusWaiting ChatStatus = "waiting" - ChatStatusPending ChatStatus = "pending" ChatStatusRunning ChatStatus = "running" - ChatStatusPaused ChatStatus = "paused" - ChatStatusCompleted ChatStatus = "completed" ChatStatusError ChatStatus = "error" ChatStatusRequiresAction ChatStatus = "requires_action" ChatStatusInterrupting ChatStatus = "interrupting" diff --git a/docs/ai-coder/agents/tasks-to-chats-migration.md b/docs/ai-coder/agents/tasks-to-chats-migration.md index 8fb2b86322..f512fdffd4 100644 --- a/docs/ai-coder/agents/tasks-to-chats-migration.md +++ b/docs/ai-coder/agents/tasks-to-chats-migration.md @@ -191,21 +191,21 @@ client already has. Task and chat statuses use different values. The Chats API status set is defined in `codersdk.ChatStatus`: -| Tasks API status | Chats API status | Notes | -|------------------|-------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `pending` | `pending` | Queued for processing. | -| `running` | `running` | Agent is actively working. | -| `complete` | `waiting` | Idle. Newly created, finished successfully, or interrupted. This is the default idle state. | -| `paused` | n/a | The Tasks API pause stops the workspace; the Chats API equivalent is `interrupt` plus separate workspace lifecycle. The `paused` enum value exists in code but no production path on `main` transitions a chat into it today. | -| `failed` | `error` | Agent encountered an error. | -| n/a | `requires_action` | Agent invoked a client-provided tool and is waiting for the result before continuing. | +| Tasks API status | Chats API status | Notes | +|------------------|-------------------|---------------------------------------------------------------------------------------------------------------------| +| `pending` | `running` | Chats have no separate queued state; a chat that hasn't been picked up yet reports `running`. | +| `running` | `running` | Agent is actively working. | +| `complete` | `waiting` | Idle. Newly created, finished successfully, or interrupted. This is the default idle state. | +| `paused` | n/a | The Tasks API pause stops the workspace; the Chats API equivalent is `interrupt` plus separate workspace lifecycle. | +| `failed` | `error` | Agent encountered an error. | +| n/a | `requires_action` | Agent invoked a client-provided tool and is waiting for the result before continuing. | +| n/a | `interrupting` | An interrupt was requested and the agent is winding down the current run. | The Chats API uses `waiting` as the default idle state (not `complete`). A chat enters `waiting` when it is first created (before any message is queued) and again whenever a run finishes or is interrupted, so treat `waiting` as "the agent is not currently working" rather than only "the -agent just finished." The `completed` enum value is also defined but is -not currently set by any production code path on `main`. +agent just finished." ### 6. Replace delete with archive @@ -530,8 +530,9 @@ curl -s -X POST https://coder.example.com/api/experimental/chats \ }' | jq '{id, status, title}' ``` -You should receive a `Chat` object with `status` set to `"waiting"` or -`"pending"`. Save the `id` for subsequent steps. +You should receive a `Chat` object with `status` set to `"running"`, +since new chats begin processing immediately. Save the `id` for +subsequent steps. ### 3. Stream the response diff --git a/docs/reference/api/chats.md b/docs/reference/api/chats.md index 40bbd32637..a9bd9bd042 100644 --- a/docs/reference/api/chats.md +++ b/docs/reference/api/chats.md @@ -229,7 +229,7 @@ Status Code **200** |---------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `client_type` | `api`, `ui` | | `kind` | `auth`, `config`, `generic`, `instruction_file`, `mcp_config`, `mcp_server`, `missing_key`, `overloaded`, `provider_disabled`, `rate_limit`, `skill`, `stream_silence_timeout`, `timeout`, `usage_limit` | -| `status` | `completed`, `error`, `excluded`, `interrupting`, `invalid`, `ok`, `oversize`, `paused`, `pending`, `requires_action`, `running`, `unreadable`, `waiting` | +| `status` | `error`, `excluded`, `interrupting`, `invalid`, `ok`, `oversize`, `requires_action`, `running`, `unreadable`, `waiting` | | `plan_mode` | `plan` | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index f50ad6d0fa..53547ca044 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -3406,9 +3406,9 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in #### Enumerated Values -| Value(s) | -|----------------------------------------------------------------------------------------------------| -| `completed`, `error`, `interrupting`, `paused`, `pending`, `requires_action`, `running`, `waiting` | +| Value(s) | +|------------------------------------------------------------------| +| `error`, `interrupting`, `requires_action`, `running`, `waiting` | ## codersdk.ChatStreamActionRequired diff --git a/site/src/api/queries/chats.test.ts b/site/src/api/queries/chats.test.ts index 7b1a948d8d..7789d4877b 100644 --- a/site/src/api/queries/chats.test.ts +++ b/site/src/api/queries/chats.test.ts @@ -2201,7 +2201,7 @@ describe("mergeWatchedChatSummary", () => { it("leaves context untouched for non-context events", () => { const context = { dirty: true, dirty_since: "2025-01-02T00:00:00.000Z" }; const cachedChat = makeChat("chat-1", { - status: "pending", + status: "waiting", updated_at: "2025-01-01T00:00:00.000Z", context, }); @@ -2220,7 +2220,7 @@ describe("mergeWatchedChatSummary", () => { it("merges fresh status updates without clobbering a newer title snapshot", () => { const cachedChat = makeChat("chat-1", { - status: "pending", + status: "waiting", title: "Fresh title", last_model_config_id: "model-old", updated_at: "2025-01-01T00:00:00.000Z", @@ -2314,7 +2314,7 @@ describe("mergeWatchedChatSummary", () => { it("compares updated_at values as instants instead of strings", () => { const cachedChat = makeChat("chat-1", { - status: "pending", + status: "waiting", last_model_config_id: "model-old", updated_at: "2025-01-01T00:00:00.12Z", }); @@ -2342,7 +2342,7 @@ describe("mergeWatchedChatSummary", () => { updated_at: "2025-01-01T00:00:00.000Z", }); const watchedChat = makeChat("chat-1", { - status: "completed", + status: "waiting", title: "Updated title", updated_at: "2025-01-01T00:05:00.000Z", }); @@ -2364,7 +2364,7 @@ describe("mergeWatchedChatSummary", () => { updated_at: "2025-01-01T00:10:00.000Z", }); const watchedChat = makeChat("chat-1", { - status: "completed", + status: "waiting", title: "Newer generated title", updated_at: "2025-01-01T00:05:00.000Z", }); @@ -2414,7 +2414,7 @@ describe("mergeWatchedChatSummary", () => { updated_at: "2025-01-01T00:00:00.000Z", }); const watchedChat = makeChat("chat-1", { - status: "completed", + status: "waiting", title: "Stale title", diff_status: watchedDiffStatus, updated_at: "2025-01-01T00:05:00.000Z", @@ -2465,7 +2465,7 @@ describe("mergeWatchedChatSummary", () => { updated_at: "2025-01-01T00:10:00.000Z", }); const watchedChat = makeChat("chat-1", { - status: "completed", + status: "waiting", title: "Stale title", diff_status: watchedDiffStatus, updated_at: "2025-01-01T00:05:00.000Z", @@ -2489,7 +2489,7 @@ describe("mergeWatchedChatSummary", () => { updated_at: "2025-01-01T00:00:00.000Z", }); const watchedChat = makeChat("chat-1", { - status: "completed", + status: "waiting", updated_at: "2025-01-01T00:05:00.000Z", }); @@ -2526,7 +2526,7 @@ describe("mergeWatchedChatSummary", () => { updated_at: "2025-01-01T00:00:00.000Z", }); const watchedChat = makeChat("chat-1", { - status: "completed", + status: "waiting", updated_at: "2025-01-01T00:05:00.000Z", }); @@ -2544,7 +2544,7 @@ describe("mergeWatchedChatIntoCaches", () => { const queryClient = createTestQueryClient(); const chatId = "chat-1"; const cachedChat = makeChat(chatId, { - status: "pending", + status: "waiting", last_model_config_id: "model-old", updated_at: "2025-01-01T00:00:00.000Z", }); @@ -2581,7 +2581,7 @@ describe("mergeWatchedChatIntoCaches", () => { const cachedChild = makeChat(childId, { parent_chat_id: "parent-1", root_chat_id: "parent-1", - status: "pending", + status: "waiting", last_model_config_id: "model-old", updated_at: "2025-01-01T00:00:00.000Z", }); @@ -2619,7 +2619,7 @@ describe("mergeWatchedChatIntoCaches", () => { const queryClient = createTestQueryClient(); const chatId = "chat-1"; const cachedChat = makeChat(chatId, { - status: "completed", + status: "waiting", title: "Fresh title", last_model_config_id: "model-new", workspace_id: "workspace-new", @@ -2643,7 +2643,7 @@ describe("mergeWatchedChatIntoCaches", () => { }); expect(readInfiniteChats(queryClient)?.[0]).toMatchObject({ - status: "completed", + status: "waiting", title: "Fresh title", last_model_config_id: "model-new", workspace_id: "workspace-new", @@ -2653,7 +2653,7 @@ describe("mergeWatchedChatIntoCaches", () => { expect( queryClient.getQueryData(chatKey(chatId)), ).toMatchObject({ - status: "completed", + status: "waiting", title: "Fresh title", last_model_config_id: "model-new", workspace_id: "workspace-new", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 8d6f8da48a..42a16dbaa6 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3018,21 +3018,15 @@ export interface ChatSourcePart { // From codersdk/chats.go export type ChatStatus = - | "completed" | "error" | "interrupting" - | "paused" - | "pending" | "requires_action" | "running" | "waiting"; export const ChatStatuses: ChatStatus[] = [ - "completed", "error", "interrupting", - "paused", - "pending", "requires_action", "running", "waiting", diff --git a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx index 487e5ccf45..e798e15e03 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx @@ -856,7 +856,7 @@ export const WithMessageHistory: Story = { id: CHAT_ID, ...baseChatFields, title: "Markdown rendering showcase", - status: "completed", + status: "waiting", }, { messages: [ @@ -1273,7 +1273,7 @@ export const RootChatShareActionAvailable: Story = { id: CHAT_ID, ...baseChatFields, title: "Shareable root chat", - status: "completed", + status: "waiting", }, { messages: [], queued_messages: [], has_more: false }, { diffUrl: undefined }, @@ -1332,7 +1332,7 @@ export const OtherUserChatReadOnly: Story = { owner_username: "OtherUser", owner_name: "Other User", title: "Other user's chat", - status: "completed", + status: "waiting", }, { messages: [], queued_messages: [], has_more: false }, { diffUrl: undefined }, @@ -1362,7 +1362,7 @@ export const OtherUserChatWithMessages: Story = { owner_username: "OtherUser", owner_name: "Other User", title: "Other user's chat with messages", - status: "completed", + status: "waiting", }, { messages: [ @@ -1436,7 +1436,7 @@ export const ArchivedOtherUserChat: Story = { owner_username: "OtherUser", owner_name: "Other User", title: "Archived other user's chat", - status: "completed", + status: "waiting", }, { messages: [], queued_messages: [], has_more: false }, { diffUrl: undefined }, @@ -1496,7 +1496,7 @@ export const PlanModeFromChatState: Story = { id: CHAT_ID, ...baseChatFields, title: "Plan mode persists", - status: "completed", + status: "waiting", plan_mode: "plan", }, { messages: [], queued_messages: [], has_more: false }, @@ -1540,7 +1540,7 @@ export const CompletedWithDiffPanel: Story = { id: CHAT_ID, ...baseChatFields, title: "Build a feature", - status: "completed", + status: "waiting", }, { messages: [], queued_messages: [], has_more: false }, { diffUrl: "https://github.com/coder/coder/pull/123" }, @@ -1600,7 +1600,7 @@ export const WithSubagentCards: Story = { result: { chat_id: "child-chat-1", title: "Child agent", - status: "pending", + status: "running", }, }, ], @@ -1711,7 +1711,7 @@ export const WithMixedSubagentTranscript: Story = { id: CHAT_ID, ...baseChatFields, title: "Mixed subagent transcript", - status: "completed", + status: "waiting", }, { messages: [ @@ -1837,7 +1837,7 @@ export const WithReasoningInline: Story = { id: CHAT_ID, ...baseChatFields, title: "Reasoning title", - status: "completed", + status: "waiting", }, { messages: [ @@ -1941,7 +1941,7 @@ export const SidebarWithPRAndRepos: Story = { id: CHAT_ID, ...baseChatFields, title: "Full sidebar demo", - status: "completed", + status: "waiting", }, { messages: [], queued_messages: [], has_more: false }, { diffUrl: "https://github.com/coder/coder/pull/456" }, @@ -2122,7 +2122,7 @@ export const SidebarWithSingleRepo: Story = { id: CHAT_ID, ...baseChatFields, title: "Single repo sidebar", - status: "completed", + status: "waiting", }, { messages: [], queued_messages: [], has_more: false }, { diffUrl: undefined }, diff --git a/site/src/pages/AgentsPage/AgentChatPage.test.ts b/site/src/pages/AgentsPage/AgentChatPage.test.ts index 011ca0a6da..945a5724e6 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.test.ts +++ b/site/src/pages/AgentsPage/AgentChatPage.test.ts @@ -192,7 +192,7 @@ describe("restoreOptimisticRequestSnapshot", () => { store.batch(() => { store.setQueuedMessages([]); - store.setChatStatus("pending"); + store.setChatStatus("waiting"); store.clearStreamState(); store.clearStreamError(); }); @@ -247,7 +247,7 @@ describe("runPromoteQueuedMessage", () => { const snapshot = store.getSnapshot(); expect(snapshot.queuedMessages.map((m) => m.id)).toEqual([a.id, c.id]); expect(snapshot.suppressedQueuedMessageIDs.has(b.id)).toBe(true); - expect(snapshot.chatStatus).toBe("pending"); + expect(snapshot.chatStatus).toBe("running"); }); it("rolls back queue and status, clears suppression, and rethrows on API error", async () => { diff --git a/site/src/pages/AgentsPage/AgentChatPage.tsx b/site/src/pages/AgentsPage/AgentChatPage.tsx index ac645dc296..2bcd8b5293 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.tsx @@ -200,7 +200,7 @@ export const runPromoteQueuedMessage = async (params: { ); store.clearStreamState(); store.clearStreamError(); - store.setChatStatus("pending"); + store.setChatStatus("running"); }); if (agentId) { clearChatErrorReason(agentId); diff --git a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx index d7569404f4..cc58a59997 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx @@ -852,7 +852,7 @@ const buildMessage = ( const buildStoreWithMessages = ( msgs: TypesGen.ChatMessage[], - status: TypesGen.ChatStatus = "completed", + status: TypesGen.ChatStatus = "waiting", ) => { const store = createChatStore(); store.replaceMessages(msgs); @@ -1098,7 +1098,7 @@ const resetScrollStoryStore = ( count = 80, ) => { store.replaceMessages(buildLongConversation(count)); - store.setChatStatus("completed"); + store.setChatStatus("waiting"); }; const inverseScrollStore = buildStoreWithMessages(buildLongConversation(80)); @@ -1425,7 +1425,7 @@ export const StickyUserMessageClipUpdatesWhilePinned: Story = { render: () => , play: async ({ canvasElement }) => { stickyClipUpdateStore.replaceMessages(buildTallStickyConversation(30)); - stickyClipUpdateStore.setChatStatus("completed"); + stickyClipUpdateStore.setChatStatus("waiting"); const canvas = within(canvasElement); const scrollContainer = canvas.getByTestId("scroll-container"); diff --git a/site/src/pages/AgentsPage/AgentsPage.tsx b/site/src/pages/AgentsPage/AgentsPage.tsx index b706153711..178ef19b13 100644 --- a/site/src/pages/AgentsPage/AgentsPage.tsx +++ b/site/src/pages/AgentsPage/AgentsPage.tsx @@ -343,8 +343,12 @@ const AgentsPage: FC = () => { (archiveAndDeleteMutation.isPending ? archiveAndDeleteMutation.variables?.chatId : undefined); + // A chat in any of these statuses has an in-flight run that + // archiving would interrupt, so ask for confirmation first. const isActiveChat = (chat: TypesGen.Chat | undefined) => - chat?.status === "pending" || chat?.status === "running"; + chat?.status === "running" || + chat?.status === "interrupting" || + chat?.status === "requires_action"; const requestArchiveAgent = (chatId: string) => { if (isArchiving) { return; diff --git a/site/src/pages/AgentsPage/AgentsPageView.stories.tsx b/site/src/pages/AgentsPage/AgentsPageView.stories.tsx index 26b67ca164..fa0004b74d 100644 --- a/site/src/pages/AgentsPage/AgentsPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentsPageView.stories.tsx @@ -535,7 +535,7 @@ export const WithChatList: Story = { buildChat({ id: "chat-1", title: "Refactor authentication module", - status: "completed", + status: "waiting", updated_at: todayTimestamp, }), buildChat({ @@ -564,13 +564,13 @@ export const WithChatList: Story = { buildChat({ id: "chat-5", title: "Implement WebSocket handler", - status: "completed", + status: "requires_action", updated_at: todayTimestamp, }), buildChat({ id: "chat-6", title: "Debug memory leak in worker", - status: "paused", + status: "interrupting", updated_at: todayTimestamp, }), ], diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.createStore.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.createStore.test.ts index c65b16a09c..02ea146779 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.createStore.test.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.createStore.test.ts @@ -376,10 +376,10 @@ describe("setSubagentStatusOverride", () => { it("overwrites an existing override for the same chatID", () => { const store = createChatStore(); store.setSubagentStatusOverride("sub-1", "running"); - store.setSubagentStatusOverride("sub-1", "completed"); + store.setSubagentStatusOverride("sub-1", "waiting"); expect(store.getSnapshot().subagentStatusOverrides.get("sub-1")).toBe( - "completed", + "waiting", ); }); }); @@ -707,24 +707,13 @@ describe("selectIsAwaitingFirstStreamChunk", () => { expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(false); }); - it("returns true during pending status when latest message is from user", () => { + it("returns false during waiting status when latest message is from user", () => { const store = createChatStore(); - store.setChatStatus("pending"); + store.setChatStatus("waiting"); store.upsertDurableMessage(makeMessage(1, "user", "hello")); - // "pending" with a user message as latest means the user - // just submitted and is waiting for the server to start. - expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(true); - }); - - it("returns false during pending status when latest message is from assistant", () => { - const store = createChatStore(); - store.setChatStatus("pending"); - store.upsertDurableMessage(makeMessage(1, "user", "hello")); - store.upsertDurableMessage(makeMessage(2, "assistant", "calling tool")); - - // "pending" with an assistant message as latest means a - // tool-call cycle is in progress, not a fresh user send. + // "waiting" means the chat is idle; nothing is generating, + // so no Thinking indicator should show. expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(false); }); @@ -745,27 +734,15 @@ describe("selectIsAwaitingFirstStreamChunk", () => { expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(true); }); - it("returns false when latest message is a tool result during pending", () => { - const store = createChatStore(); - store.setChatStatus("pending"); - store.upsertDurableMessage(makeMessage(1, "user", "hello")); - store.upsertDurableMessage(makeMessage(2, "assistant", "calling tool")); - store.upsertDurableMessage(makeMessage(3, "tool", "tool result")); - - // During "pending", the transport cannot deliver parts, so - // we should not be in a "starting" state. - expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(false); - }); - it("returns true after optimistic send: clearStreamState + setChatStatus('running') + upsertDurableMessage", () => { const store = createChatStore(); - // Simulate a completed previous turn: assistant replied, - // then server transitioned to "pending". + // Simulate a settled previous turn: assistant replied, + // then server transitioned to "waiting". store.upsertDurableMessage(makeMessage(1, "user", "first question")); store.upsertDurableMessage(makeMessage(2, "assistant", "first answer")); - store.setChatStatus("pending"); + store.setChatStatus("waiting"); - // Verify baseline: not awaiting during pending. + // Verify baseline: not awaiting while idle. expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(false); // Simulate handleSend after POST returns (non-queued). @@ -777,15 +754,14 @@ describe("selectIsAwaitingFirstStreamChunk", () => { expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(true); }); - it("returns true when WS delivers user message + status:pending (fresh send)", () => { + it("returns true when WS delivers user message + status:running (fresh send)", () => { const store = createChatStore(); - // Simulate the WS batch: [message(user), status:pending]. - // This is the exact event order from the server when the - // user sends a message. The Thinking indicator must appear during - // the pending phase so there is no visual gap before the - // server transitions to running. + // Simulate the WS batch: [message(user), status:running]. + // This is the event order from the server when the user + // sends a message. The Thinking indicator must appear + // before the first stream chunk arrives. store.upsertDurableMessage(makeMessage(1, "user", "sweet ty")); - store.setChatStatus("pending"); + store.setChatStatus("running"); store.clearStreamState(); expect(selectIsAwaitingFirstStreamChunk(store.getSnapshot())).toBe(true); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx index a1c7ce0f29..b4a87870b3 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx @@ -1221,7 +1221,7 @@ describe("useChatStore", () => { }); }); - it("ignores message_part updates while chat is pending", async () => { + it("ignores message_part updates while chat is waiting", async () => { immediateAnimationFrame(); const chatID = "chat-1"; @@ -1284,12 +1284,12 @@ describe("useChatStore", () => { mockSocket.emitData({ type: "status", chat_id: chatID, - status: { status: "pending" }, + status: { status: "waiting" }, }); }); await waitFor(() => { - // Stream state is preserved after status=pending (the + // Stream state is preserved after status=waiting (the // durable message event handles cleanup via // needsStreamReset). Only new message_parts should be // blocked by the shouldApplyMessagePart gate. @@ -1315,7 +1315,7 @@ describe("useChatStore", () => { await waitFor(() => { // The late message_part should not be applied because - // shouldApplyMessagePart gates on pending/waiting. + // shouldApplyMessagePart gates on waiting. // Stream state still shows the original "first". expect(result.current.streamState?.blocks).toEqual([ { type: "response", text: "first" }, @@ -2622,13 +2622,13 @@ describe("useChatStore", () => { mockSocket.emitData({ type: "status", chat_id: subagentChatID, - status: { status: "completed" }, + status: { status: "waiting" }, }); }); await waitFor(() => { expect(result.current.subagentStatusOverrides.get(subagentChatID)).toBe( - "completed", + "waiting", ); }); // Main chat status should remain "running" from the initial @@ -2868,10 +2868,10 @@ describe("useChatStore", () => { }); }); - it("does not surface reconnectState for completed chats", async () => { + it("does not surface reconnectState for settled chats", async () => { immediateAnimationFrame(); - const chatID = "chat-disconnect-completed"; + const chatID = "chat-disconnect-settled"; const mockSocket = createMockSocket(); mockWatchChatReturn(mockSocket); @@ -2883,7 +2883,7 @@ describe("useChatStore", () => { const { store } = useChatStore({ chatID, chatMessages: [], - chatRecord: { ...buildChat(chatID), status: "completed" }, + chatRecord: { ...buildChat(chatID), status: "waiting" }, chatMessagesData: { messages: [], queued_messages: [], @@ -2902,7 +2902,7 @@ describe("useChatStore", () => { ); await waitFor(() => { - expect(result.current.chatStatus).toBe("completed"); + expect(result.current.chatStatus).toBe("waiting"); expect(watchChat).toHaveBeenCalledWith(chatID, undefined); }); @@ -2912,7 +2912,7 @@ describe("useChatStore", () => { await waitFor(() => { expect(result.current.reconnectState).toBeNull(); - expect(result.current.chatStatus).toBe("completed"); + expect(result.current.chatStatus).toBe("waiting"); }); }); it("uses exponential backoff on consecutive disconnects", async () => { @@ -3539,9 +3539,9 @@ describe("useChatStore", () => { expect(result.current.chatStatus).toBe("running"); }); - // Simulate a stale REST refetch returning "pending". + // Simulate a stale REST refetch returning "waiting". rerender({ - chatRecord: { ...buildChat(chatID), status: "pending" }, + chatRecord: { ...buildChat(chatID), status: "waiting" }, }); // The store must ignore the stale REST value because the @@ -3926,9 +3926,9 @@ describe("thinking indicator event ordering", () => { expect(watchChat).toHaveBeenCalledWith(chatID, 1); }); - // Server sends message_part then immediately transitions to pending. - // The buffered parts must be discarded (not applied) because - // pending status clears stream state. + // Server sends message_part then immediately transitions to + // waiting. The buffered parts must be discarded (not applied) + // because waiting status clears stream state. act(() => { mockSocket.emitDataBatch([ { @@ -3941,13 +3941,13 @@ describe("thinking indicator event ordering", () => { { type: "status", chat_id: chatID, - status: { status: "pending" }, + status: { status: "waiting" }, }, ]); }); await waitFor(() => { - expect(result.current.chatStatus).toBe("pending"); + expect(result.current.chatStatus).toBe("waiting"); expect(result.current.streamState).toBeNull(); }); @@ -4014,13 +4014,13 @@ describe("updateSidebarChat via stream events", () => { mockSocket.emitData({ type: "status", chat_id: chatID, - status: { status: "completed" }, + status: { status: "waiting" }, }); }); await waitFor(() => { const sidebarChats = readInfiniteChats(queryClient); - expect(sidebarChats?.[0].status).toBe("completed"); + expect(sidebarChats?.[0].status).toBe("waiting"); }); }); @@ -4211,14 +4211,14 @@ describe("updateSidebarChat via stream events", () => { mockSocket.emitData({ type: "status", chat_id: chatID, - status: { status: "completed" }, + status: { status: "waiting" }, }); }); await waitFor(() => { const sidebarChats = readInfiniteChats(queryClient); expect(sidebarChats?.find((c) => c.id === chatID)?.status).toBe( - "completed", + "waiting", ); }); @@ -4351,14 +4351,14 @@ describe("updateSidebarChat via stream events", () => { mockSocket.emitData({ type: "status", chat_id: chatID, - status: { status: "completed" }, + status: { status: "waiting" }, }); }); await waitFor(() => { const sidebarChats = readInfiniteChats(queryClient); // Status should update, but updated_at must stay untouched. - expect(sidebarChats?.[0].status).toBe("completed"); + expect(sidebarChats?.[0].status).toBe("waiting"); expect(sidebarChats?.[0].updated_at).toBe(initialChat.updated_at); }); }); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.ts b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.ts index 44020f9e10..5f82c01c0a 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.ts @@ -142,8 +142,7 @@ const reconnectStatesEqual = ( export const isActiveChatStatus = ( status: TypesGen.ChatStatus | null, -): boolean => - status === "running" || status === "pending" || status === "interrupting"; +): boolean => status === "running" || status === "interrupting"; export type ChatStoreState = { messagesByID: Map; @@ -663,25 +662,13 @@ export const selectIsAwaitingFirstStreamChunk = ( const latestMessageNeedsAssistantResponse = !latestMessage || latestMessage.role !== "assistant"; // Show the Thinking indicator when the store has no stream - // data yet and the conversation is waiting for an assistant - // response. For "running" status we use the existing broad - // check (any non-assistant latest message). For "pending" we - // restrict to the case where the latest message is explicitly - // a user message — this covers the fresh-send flow (user just - // submitted and the server hasn't started streaming yet) while - // avoiding a spurious indicator during multi-turn tool-call - // cycles, where the latest durable message is a tool result - // and the assistant response is still being assembled. + // data yet, the chat is running, and the conversation is + // waiting for an assistant response (any non-assistant latest + // message). if (state.streamState !== null || !latestMessageNeedsAssistantResponse) { return false; } - if (state.chatStatus === "running") { - return true; - } - if (state.chatStatus === "pending" && latestMessage?.role === "user") { - return true; - } - return false; + return state.chatStatus === "running"; }; export const useChatSelector = ( diff --git a/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts b/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts index b61e9b0514..e887682d58 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts @@ -86,7 +86,7 @@ export const useChatStore = ( // source for chatStatus and the REST-fetched chatRecord.status // must not overwrite it. Without this guard, a React Query // refetch (e.g. on window focus) can regress chatStatus to a - // stale value like "pending", causing shouldApplyMessagePart() + // stale value like "waiting", causing shouldApplyMessagePart() // to drop all incoming parts. const wsStatusReceivedRef = useRef(false); const activeChatIDRef = useRef(null); @@ -388,8 +388,7 @@ export const useChatStore = ( const historyReplacementBuf: TypesGen.ChatMessage[] = []; const shouldApplyMessagePart = (): boolean => { - const currentStatus = store.getSnapshot().chatStatus; - return currentStatus !== "pending" && currentStatus !== "waiting"; + return store.getSnapshot().chatStatus !== "waiting"; }; const schedulePartsFlush = () => { @@ -584,12 +583,8 @@ export const useChatStore = ( wsStatusReceivedRef.current = true; store.clearRetryState(); store.setChatStatus(nextStatus); - if (nextStatus === "pending" || nextStatus === "waiting") { + if (nextStatus === "waiting") { discardBufferedParts(); - store.clearRetryState(); - } - if (nextStatus === "running") { - store.clearRetryState(); } if (nextStatus !== "error") { clearChatErrorReasonEvent(chatID); diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/SubagentTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/SubagentTool.tsx index 9fc4aaa26f..83a292bd63 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/SubagentTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/SubagentTool.tsx @@ -99,7 +99,7 @@ function getSubagentLabel( * Resolves a sub-agent status string and tool-level status into a * display icon. The sub-agent status in the tool result is a * snapshot from when the tool returned and may be stale (e.g. a - * background sub-agent records "pending" forever). The icon is + * background sub-agent records "running" forever). The icon is * therefore driven primarily by the tool-call status itself. */ const SubagentStatusIcon: React.FC<{ diff --git a/site/src/pages/AgentsPage/components/ChatPageContent.tsx b/site/src/pages/AgentsPage/components/ChatPageContent.tsx index 0d7ca6c468..02d5590e1b 100644 --- a/site/src/pages/AgentsPage/components/ChatPageContent.tsx +++ b/site/src/pages/AgentsPage/components/ChatPageContent.tsx @@ -80,7 +80,7 @@ export const ChatPageTimeline: FC = ({ store, selectIsAwaitingFirstStreamChunk, ); - const isChatCompleted = !hasStream && chatStatus !== "pending"; + const isChatCompleted = !hasStream; const messages = orderedMessageIDs .map((messageID) => { @@ -422,8 +422,7 @@ export const ChatPageInput: FC = ({ wasEditingRef.current = isEditing; }, [isEditing, resetEditAttachments]); - const isStreaming = - hasStreamState || chatStatus === "running" || chatStatus === "pending"; + const isStreaming = hasStreamState || chatStatus === "running"; const inputElement = ( { const canvas = within(canvasElement); - await expect(canvas.getAllByText("GPT-4o streaming…")).toHaveLength(2); + await expect(canvas.getByText("GPT-4o streaming…")).toBeInTheDocument(); expect( canvas.queryByText("Added Docker and Terraform validation"), ).not.toBeInTheDocument(); @@ -402,41 +396,6 @@ export const RunningDelegatedChat: Story = { }, }; -export const PendingDelegatedChat: Story = { - args: { - chats: [ - buildChat({ - id: "root-pending", - title: "Root agent", - children: [ - buildChat({ - id: "child-pending", - title: "Pending child", - status: "pending", - parent_chat_id: "root-pending", - root_chat_id: "root-pending", - }), - ], - }), - ], - }, - parameters: { - reactRouter: reactRouterParameters({ - location: { - path: "/agents/child-pending", - pathParams: { agentId: "child-pending" }, - }, - routing: agentsRouting, - }), - }, - play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - await expect( - canvas.getByTestId("agents-tree-executing-child-pending"), - ).toBeInTheDocument(); - }, -}; - export const ExpandCollapse: Story = { args: { chats: [ diff --git a/site/src/pages/AgentsPage/components/ChatsSidebar/dialogs/ChatSearchDialog.stories.tsx b/site/src/pages/AgentsPage/components/ChatsSidebar/dialogs/ChatSearchDialog.stories.tsx index 3bf3b96c37..ce71561950 100644 --- a/site/src/pages/AgentsPage/components/ChatsSidebar/dialogs/ChatSearchDialog.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatsSidebar/dialogs/ChatSearchDialog.stories.tsx @@ -24,7 +24,7 @@ const mockChat: Chat = { owner_id: "owner-1", owner_username: "jaayden", title: "Fix race condition in auth middleware", - status: "completed", + status: "waiting", last_model_config_id: "model-1", mcp_server_ids: [], labels: {}, diff --git a/site/src/pages/AgentsPage/components/ChatsSidebar/tree/ChatTreeNode.tsx b/site/src/pages/AgentsPage/components/ChatsSidebar/tree/ChatTreeNode.tsx index c60daa84c4..5dfce96d40 100644 --- a/site/src/pages/AgentsPage/components/ChatsSidebar/tree/ChatTreeNode.tsx +++ b/site/src/pages/AgentsPage/components/ChatsSidebar/tree/ChatTreeNode.tsx @@ -71,8 +71,7 @@ export const ChatTreeNode: FC = ({ chat, isChildNode }) => { ); const hasChildren = childIDs.length > 0; const isDelegated = Boolean(getParentChatID(chat)); - const isDelegatedExecuting = - isDelegated && (chat.status === "pending" || chat.status === "running"); + const isDelegatedExecuting = isDelegated && chat.status === "running"; const modelName = getModelDisplayName( chat.last_model_config_id, modelConfigs, @@ -83,7 +82,7 @@ export const ChatTreeNode: FC = ({ chat, isChildNode }) => { ? chatErrorReasons[chat.id] || chat.last_error?.message || undefined : undefined; const lastTurnSummary = asNonEmptyString(chat.last_turn_summary); - const isStreaming = chat.status === "running" || chat.status === "pending"; + const isStreaming = chat.status === "running"; const streamingSubtitle = isStreaming ? `${modelName} streaming…` : undefined; const staleTurnSummaryReleaseMs = 10_000; const [streamingSummary, setStreamingSummary] = useState( diff --git a/site/src/pages/AgentsPage/components/ChatsSidebar/tree/statusConfig.ts b/site/src/pages/AgentsPage/components/ChatsSidebar/tree/statusConfig.ts index 187e7f63ac..30130480c1 100644 --- a/site/src/pages/AgentsPage/components/ChatsSidebar/tree/statusConfig.ts +++ b/site/src/pages/AgentsPage/components/ChatsSidebar/tree/statusConfig.ts @@ -18,17 +18,14 @@ type ChatIconConfig = { const statusConfig = { waiting: { icon: CheckIcon, className: "text-content-secondary" }, - pending: { icon: Loader2Icon, className: "text-content-link animate-spin" }, running: { icon: Loader2Icon, className: "text-content-link animate-spin" }, - paused: { icon: PauseIcon, className: "text-content-warning" }, interrupting: { icon: PauseIcon, className: "text-content-warning" }, requires_action: { icon: PauseIcon, className: "text-content-warning" }, error: { icon: AlertTriangleIcon, className: "text-content-destructive" }, - completed: { icon: CheckIcon, className: "text-content-secondary" }, } as const; const getStatusConfig = (status: ChatStatus): ChatIconConfig => { - return statusConfig[status] ?? statusConfig.completed; + return statusConfig[status] ?? statusConfig.waiting; }; const getPRIconConfig = ( @@ -63,10 +60,10 @@ const getChatDiffStatus = (chat: Chat): ChatDiffStatus | undefined => { /** * Returns the icon and styling that represents a chat's current state. * - * Combines `getStatusConfig` and `getPRIconConfig`: when the chat is in a - * settled state (`waiting` or `completed`) and has a linked PR, the PR icon - * takes precedence so list rows surface the merge / closed / draft state - * instead of the generic status icon. + * Combines `getStatusConfig` and `getPRIconConfig`: when the chat is in the + * settled `waiting` state and has a linked PR, the PR icon takes precedence + * so list rows surface the merge / closed / draft state instead of the + * generic status icon. */ export const getChatDisplayConfig = ( chat: Chat, @@ -78,9 +75,7 @@ export const getChatDisplayConfig = ( const diffStatus = getChatDiffStatus(chat); const baseConfig = getStatusConfig(chat.status); const prConfig = - chat.status === "waiting" || chat.status === "completed" - ? getPRIconConfig(diffStatus) - : undefined; + chat.status === "waiting" ? getPRIconConfig(diffStatus) : undefined; const config = prConfig ?? baseConfig; return { icon: config.icon, diff --git a/site/src/pages/AgentsPage/utils/chime.test.ts b/site/src/pages/AgentsPage/utils/chime.test.ts index 35ac4fec64..9d0e4ca303 100644 --- a/site/src/pages/AgentsPage/utils/chime.test.ts +++ b/site/src/pages/AgentsPage/utils/chime.test.ts @@ -124,18 +124,6 @@ describe("maybePlayChime", () => { expect(playSpy).toHaveBeenCalledTimes(1); }); - it("chimes on running → pending when viewing a different chat", async () => { - vi.spyOn(document, "hidden", "get").mockReturnValue(false); - await triggerAndSettle("running", "pending", "chat-1", "chat-2"); - expect(playSpy).toHaveBeenCalledTimes(1); - }); - - it("chimes on pending → waiting (watchChats skips running)", async () => { - vi.spyOn(document, "hidden", "get").mockReturnValue(false); - await triggerAndSettle("pending", "waiting", "chat-1", "chat-2"); - expect(playSpy).toHaveBeenCalledTimes(1); - }); - it("chimes on running → waiting when tab is hidden (same chat)", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(true); await triggerAndSettle("running", "waiting", "chat-1", "chat-1"); @@ -193,9 +181,9 @@ describe("maybePlayChime", () => { expect(playSpy).not.toHaveBeenCalled(); }); - it("does NOT chime on pending → pending (no change)", async () => { + it("does NOT chime on interrupting → waiting (interrupted, not finished)", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(true); - await triggerAndSettle("pending", "pending", "chat-1", "chat-2"); + await triggerAndSettle("interrupting", "waiting", "chat-1", "chat-2"); expect(playSpy).not.toHaveBeenCalled(); }); diff --git a/site/src/pages/AgentsPage/utils/chime.ts b/site/src/pages/AgentsPage/utils/chime.ts index 63b94df13b..e8f9ef61c5 100644 --- a/site/src/pages/AgentsPage/utils/chime.ts +++ b/site/src/pages/AgentsPage/utils/chime.ts @@ -110,15 +110,8 @@ function playChime(chatID: string): void { /** * Check whether a chat status transition should trigger a chime - * and play it if so. The chime fires on these transitions: - * - * running → waiting (normal completion via per-chat WS) - * running → pending (normal completion via per-chat WS) - * pending → waiting (watchChats WS skipped "running") - * - * Note that "pending" appears as both a source and a target: - * it is an active state when the agent is queued, and a resting - * state after the agent finishes. The chime is suppressed when + * and play it if so. The chime fires on the running → waiting + * transition (normal completion). The chime is suppressed when * the chat is currently visible to the user. */ export function maybePlayChime( @@ -131,18 +124,13 @@ export function maybePlayChime( return; } - // Terminal states that indicate the agent finished. - const isTerminal = nextStatus === "waiting" || nextStatus === "pending"; - if (!isTerminal) { + // Terminal state that indicates the agent finished. + if (nextStatus !== "waiting") { return; } - // Only chime when transitioning from a non-terminal state. - // "running" is the expected previous state, but "pending" can - // appear when the watchChats WebSocket skips the intermediate - // "running" status (it only publishes the final state change). - const wasActive = prevStatus === "running" || prevStatus === "pending"; - if (!wasActive) { + // Only chime when transitioning from the active state. + if (prevStatus !== "running") { return; } diff --git a/site/src/testHelpers/chatEntities.ts b/site/src/testHelpers/chatEntities.ts index 2c634ecbb1..efd5e52be6 100644 --- a/site/src/testHelpers/chatEntities.ts +++ b/site/src/testHelpers/chatEntities.ts @@ -18,7 +18,7 @@ export const MockChat: Chat = { owner_name: MockUserOwner.name, last_model_config_id: "model-config-1", title: "Agent", - status: "completed", + status: "waiting", last_turn_summary: null, created_at: MOCK_TIMESTAMP, updated_at: MOCK_TIMESTAMP,