From a848e12c7493cc2c80e08689f17ef4530a286a08 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 29 May 2026 22:57:12 +0000 Subject: [PATCH] fix(coderd/database): use monotonic chat goal ordering --- coderd/database/dump.sql | 12 +- .../000510_chat_goal_persistence.up.sql | 3 +- coderd/database/models.go | 1 + coderd/database/querier_test.go | 121 ++++++++++++++++-- coderd/database/queries.sql.go | 28 ++-- coderd/database/queries/chats.sql | 4 +- 6 files changed, 147 insertions(+), 22 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 3d22970323..c733ce1736 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1679,6 +1679,7 @@ CREATE TABLE chat_files ( CREATE TABLE chat_goals ( id uuid DEFAULT gen_random_uuid() NOT NULL, + goal_order bigint NOT NULL, root_chat_id uuid NOT NULL, created_from_chat_id uuid, created_from_message_id bigint, @@ -1702,6 +1703,15 @@ CREATE TABLE chat_goals ( CONSTRAINT chat_goals_replaced_at_status_check CHECK (((status = 'replaced'::chat_goal_status) = (replaced_at IS NOT NULL))) ); +ALTER TABLE chat_goals ALTER COLUMN goal_order ADD GENERATED ALWAYS AS IDENTITY ( + SEQUENCE NAME chat_goals_goal_order_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1 +); + CREATE TABLE chat_messages ( id bigint NOT NULL, chat_id uuid NOT NULL, @@ -4311,7 +4321,7 @@ CREATE INDEX idx_chat_goals_created_from_message_id ON chat_goals USING btree (c CREATE UNIQUE INDEX idx_chat_goals_current ON chat_goals USING btree (root_chat_id) WHERE (status = ANY (ARRAY['active'::chat_goal_status, 'paused'::chat_goal_status])); -CREATE INDEX idx_chat_goals_root_created ON chat_goals USING btree (root_chat_id, created_at DESC, id DESC); +CREATE INDEX idx_chat_goals_root_created ON chat_goals USING btree (root_chat_id, created_at DESC, goal_order DESC); CREATE INDEX idx_chat_messages_chat ON chat_messages USING btree (chat_id); diff --git a/coderd/database/migrations/000510_chat_goal_persistence.up.sql b/coderd/database/migrations/000510_chat_goal_persistence.up.sql index 105932c8fc..8b0421e2b9 100644 --- a/coderd/database/migrations/000510_chat_goal_persistence.up.sql +++ b/coderd/database/migrations/000510_chat_goal_persistence.up.sql @@ -8,6 +8,7 @@ CREATE TYPE chat_goal_status AS ENUM ( CREATE TABLE chat_goals ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + goal_order BIGINT GENERATED ALWAYS AS IDENTITY NOT NULL, root_chat_id UUID NOT NULL REFERENCES chats(id) ON DELETE CASCADE, created_from_chat_id UUID REFERENCES chats(id) ON DELETE SET NULL, created_from_message_id BIGINT REFERENCES chat_messages(id) ON DELETE SET NULL, @@ -36,7 +37,7 @@ CREATE UNIQUE INDEX idx_chat_goals_current WHERE status IN ('active', 'paused'); CREATE INDEX idx_chat_goals_root_created - ON chat_goals(root_chat_id, created_at DESC, id DESC); + ON chat_goals(root_chat_id, created_at DESC, goal_order DESC); CREATE INDEX idx_chat_goals_created_from_message_id ON chat_goals(created_from_message_id) diff --git a/coderd/database/models.go b/coderd/database/models.go index 2c1a8355e3..b5778488d9 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -4786,6 +4786,7 @@ type ChatFileLink struct { type ChatGoal struct { ID uuid.UUID `db:"id" json:"id"` + GoalOrder int64 `db:"goal_order" json:"goal_order"` RootChatID uuid.UUID `db:"root_chat_id" json:"root_chat_id"` CreatedFromChatID uuid.NullUUID `db:"created_from_chat_id" json:"created_from_chat_id"` CreatedFromMessageID sql.NullInt64 `db:"created_from_message_id" json:"created_from_message_id"` diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 8d84a1503c..a1b3d2d6c9 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -12130,16 +12130,16 @@ func TestChatPinOrderConstraints(t *testing.T) { }) } -func TestChatGoalPersistence(t *testing.T) { +func TestChatGoals(t *testing.T) { t.Parallel() if testing.Short() { t.SkipNow() } - setup := func(t *testing.T) (database.Store, context.Context, database.User, database.Chat) { + setup := func(t *testing.T) (database.Store, *sql.DB, context.Context, database.User, database.Chat) { t.Helper() - store, _ := dbtestutil.NewDB(t) + store, _, sqlDB := dbtestutil.NewDBWithSQLDB(t) ctx := testutil.Context(t, testutil.WaitMedium) owner := dbgen.User(t, store, database.User{}) org := dbgen.Organization(t, store, database.Organization{}) @@ -12178,7 +12178,7 @@ func TestChatGoalPersistence(t *testing.T) { }) require.NoError(t, err) - return store, ctx, owner, chat + return store, sqlDB, ctx, owner, chat } insertGoal := func(t *testing.T, store database.Store, ctx context.Context, chat database.Chat, owner database.User, objective string) database.ChatGoal { @@ -12205,10 +12205,25 @@ func TestChatGoalPersistence(t *testing.T) { return goal } + setGoalCreatedAt := func(t *testing.T, sqlDB *sql.DB, ctx context.Context, createdAt time.Time, goals ...database.ChatGoal) { + t.Helper() + + ids := make([]uuid.UUID, 0, len(goals)) + for _, goal := range goals { + ids = append(ids, goal.ID) + } + + result, err := sqlDB.ExecContext(ctx, "UPDATE chat_goals SET created_at = $1 WHERE id = ANY($2)", createdAt, pq.Array(ids)) + require.NoError(t, err) + rowsAffected, err := result.RowsAffected() + require.NoError(t, err) + require.Equal(t, int64(len(ids)), rowsAffected) + } + t.Run("CurrentGoalInvariant", func(t *testing.T) { t.Parallel() - store, ctx, owner, chat := setup(t) + store, _, ctx, owner, chat := setup(t) first := insertGoal(t, store, ctx, chat, owner, "ship goal persistence") require.Equal(t, database.ChatGoalStatusActive, first.Status) @@ -12254,7 +12269,7 @@ func TestChatGoalPersistence(t *testing.T) { t.Run("LifecycleUsesOptimisticGoalID", func(t *testing.T) { t.Parallel() - store, ctx, owner, chat := setup(t) + store, _, ctx, owner, chat := setup(t) goal := insertGoal(t, store, ctx, chat, owner, "complete the task") current, err := store.GetCurrentChatGoalByRootChatID(ctx, chat.ID) @@ -12328,7 +12343,7 @@ func TestChatGoalPersistence(t *testing.T) { t.Run("LatestGoalControlsVisibility", func(t *testing.T) { t.Parallel() - store, ctx, owner, chat := setup(t) + store, _, ctx, owner, chat := setup(t) first := insertGoal(t, store, ctx, chat, owner, "finished goal") completed, err := store.CompleteChatGoalByID(ctx, database.CompleteChatGoalByIDParams{ RootChatID: chat.ID, @@ -12363,10 +12378,100 @@ func TestChatGoalPersistence(t *testing.T) { require.ErrorIs(t, err, sql.ErrNoRows) }) + t.Run("CurrentGoalUsesGoalOrderForCreatedAtTies", func(t *testing.T) { + t.Parallel() + + store, sqlDB, ctx, owner, chat := setup(t) + first := insertGoal(t, store, ctx, chat, owner, "first tied goal") + _, err := store.CompleteChatGoalByID(ctx, database.CompleteChatGoalByIDParams{ + RootChatID: chat.ID, + ID: first.ID, + CompletedByAgent: true, + }) + require.NoError(t, err) + + second := insertGoal(t, store, ctx, chat, owner, "second tied goal") + setGoalCreatedAt(t, sqlDB, ctx, dbtime.Now(), first, second) + + current, err := store.GetCurrentChatGoalByRootChatID(ctx, chat.ID) + require.NoError(t, err) + require.Equal(t, second.ID, current.ID) + require.Equal(t, database.ChatGoalStatusActive, current.Status) + }) + + t.Run("LatestHiddenGoalSuppressesTiedVisibleHistory", func(t *testing.T) { + t.Parallel() + + store, sqlDB, ctx, owner, chat := setup(t) + first := insertGoal(t, store, ctx, chat, owner, "visible tied goal") + _, err := store.CompleteChatGoalByID(ctx, database.CompleteChatGoalByIDParams{ + RootChatID: chat.ID, + ID: first.ID, + CompletedByAgent: true, + }) + require.NoError(t, err) + + second := insertGoal(t, store, ctx, chat, owner, "hidden tied goal") + _, err = store.ClearChatGoalByID(ctx, database.ClearChatGoalByIDParams{ + RootChatID: chat.ID, + ID: second.ID, + }) + require.NoError(t, err) + + setGoalCreatedAt(t, sqlDB, ctx, dbtime.Now(), first, second) + _, err = store.GetCurrentChatGoalByRootChatID(ctx, chat.ID) + require.ErrorIs(t, err, sql.ErrNoRows) + }) + + t.Run("BulkCurrentGoalsUseGoalOrderForCreatedAtTies", func(t *testing.T) { + t.Parallel() + + store, sqlDB, ctx, owner, chat := setup(t) + visibleFirst := insertGoal(t, store, ctx, chat, owner, "bulk visible first") + _, err := store.CompleteChatGoalByID(ctx, database.CompleteChatGoalByIDParams{ + RootChatID: chat.ID, + ID: visibleFirst.ID, + CompletedByAgent: true, + }) + require.NoError(t, err) + visibleSecond := insertGoal(t, store, ctx, chat, owner, "bulk visible second") + setGoalCreatedAt(t, sqlDB, ctx, dbtime.Now(), visibleFirst, visibleSecond) + + hiddenChat, err := store.InsertChat(ctx, database.InsertChatParams{ + OrganizationID: chat.OrganizationID, + Status: database.ChatStatusWaiting, + ClientType: database.ChatClientTypeUi, + OwnerID: owner.ID, + LastModelConfigID: chat.LastModelConfigID, + Title: "goal-test-hidden-" + uuid.NewString(), + }) + require.NoError(t, err) + hiddenFirst := insertGoal(t, store, ctx, hiddenChat, owner, "bulk hidden first") + _, err = store.CompleteChatGoalByID(ctx, database.CompleteChatGoalByIDParams{ + RootChatID: hiddenChat.ID, + ID: hiddenFirst.ID, + CompletedByAgent: true, + }) + require.NoError(t, err) + hiddenSecond := insertGoal(t, store, ctx, hiddenChat, owner, "bulk hidden second") + _, err = store.ClearChatGoalByID(ctx, database.ClearChatGoalByIDParams{ + RootChatID: hiddenChat.ID, + ID: hiddenSecond.ID, + }) + require.NoError(t, err) + setGoalCreatedAt(t, sqlDB, ctx, dbtime.Now(), hiddenFirst, hiddenSecond) + + currentGoals, err := store.GetCurrentChatGoalsByRootChatIDs(ctx, []uuid.UUID{chat.ID, hiddenChat.ID}) + require.NoError(t, err) + require.Len(t, currentGoals, 1) + require.Equal(t, visibleSecond.ID, currentGoals[0].ID) + require.Equal(t, chat.ID, currentGoals[0].RootChatID) + }) + t.Run("ReplaceCurrentGoal", func(t *testing.T) { t.Parallel() - store, ctx, owner, chat := setup(t) + store, _, ctx, owner, chat := setup(t) first := insertGoal(t, store, ctx, chat, owner, "old goal") replaced, err := store.MarkCurrentChatGoalReplacedByRootChatID(ctx, chat.ID) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index b0ed7afc69..d3eca91d5d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -6401,7 +6401,7 @@ WHERE root_chat_id = $1::uuid AND id = $2::uuid AND status IN ('active', 'paused', 'complete') -RETURNING id, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at +RETURNING id, goal_order, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at ` type ClearChatGoalByIDParams struct { @@ -6414,6 +6414,7 @@ func (q *sqlQuerier) ClearChatGoalByID(ctx context.Context, arg ClearChatGoalByI var i ChatGoal err := row.Scan( &i.ID, + &i.GoalOrder, &i.RootChatID, &i.CreatedFromChatID, &i.CreatedFromMessageID, @@ -6459,7 +6460,7 @@ WHERE root_chat_id = $4::uuid AND id = $5::uuid AND status = 'active' -RETURNING id, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at +RETURNING id, goal_order, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at ` type CompleteChatGoalByIDParams struct { @@ -6481,6 +6482,7 @@ func (q *sqlQuerier) CompleteChatGoalByID(ctx context.Context, arg CompleteChatG var i ChatGoal err := row.Scan( &i.ID, + &i.GoalOrder, &i.RootChatID, &i.CreatedFromChatID, &i.CreatedFromMessageID, @@ -8588,7 +8590,7 @@ func (q *sqlQuerier) GetChildChatsByParentIDs(ctx context.Context, arg GetChildC const getCurrentChatGoalByRootChatID = `-- name: GetCurrentChatGoalByRootChatID :one SELECT - chat_goals.id, chat_goals.root_chat_id, chat_goals.created_from_chat_id, chat_goals.created_from_message_id, chat_goals.objective, chat_goals.status, chat_goals.completion_summary, chat_goals.created_by_user_id, chat_goals.completed_by_user_id, chat_goals.completed_by_agent, chat_goals.created_at, chat_goals.updated_at, chat_goals.completed_at, chat_goals.cleared_at, chat_goals.replaced_at + chat_goals.id, chat_goals.goal_order, chat_goals.root_chat_id, chat_goals.created_from_chat_id, chat_goals.created_from_message_id, chat_goals.objective, chat_goals.status, chat_goals.completion_summary, chat_goals.created_by_user_id, chat_goals.completed_by_user_id, chat_goals.completed_by_agent, chat_goals.created_at, chat_goals.updated_at, chat_goals.completed_at, chat_goals.cleared_at, chat_goals.replaced_at FROM chat_goals WHERE @@ -8601,7 +8603,7 @@ WHERE root_chat_id = $1::uuid ORDER BY created_at DESC, - id DESC + goal_order DESC LIMIT 1 ) AND status IN ('active', 'paused', 'complete') @@ -8612,6 +8614,7 @@ func (q *sqlQuerier) GetCurrentChatGoalByRootChatID(ctx context.Context, rootCha var i ChatGoal err := row.Scan( &i.ID, + &i.GoalOrder, &i.RootChatID, &i.CreatedFromChatID, &i.CreatedFromMessageID, @@ -8641,10 +8644,10 @@ WITH latest_goal_ids AS ( ORDER BY root_chat_id, created_at DESC, - id DESC + goal_order DESC ) SELECT - chat_goals.id, chat_goals.root_chat_id, chat_goals.created_from_chat_id, chat_goals.created_from_message_id, chat_goals.objective, chat_goals.status, chat_goals.completion_summary, chat_goals.created_by_user_id, chat_goals.completed_by_user_id, chat_goals.completed_by_agent, chat_goals.created_at, chat_goals.updated_at, chat_goals.completed_at, chat_goals.cleared_at, chat_goals.replaced_at + chat_goals.id, chat_goals.goal_order, chat_goals.root_chat_id, chat_goals.created_from_chat_id, chat_goals.created_from_message_id, chat_goals.objective, chat_goals.status, chat_goals.completion_summary, chat_goals.created_by_user_id, chat_goals.completed_by_user_id, chat_goals.completed_by_agent, chat_goals.created_at, chat_goals.updated_at, chat_goals.completed_at, chat_goals.cleared_at, chat_goals.replaced_at FROM chat_goals JOIN latest_goal_ids ON latest_goal_ids.id = chat_goals.id @@ -8663,6 +8666,7 @@ func (q *sqlQuerier) GetCurrentChatGoalsByRootChatIDs(ctx context.Context, rootC var i ChatGoal if err := rows.Scan( &i.ID, + &i.GoalOrder, &i.RootChatID, &i.CreatedFromChatID, &i.CreatedFromMessageID, @@ -8900,7 +8904,7 @@ INSERT INTO chat_goals ( 'active', $5::uuid ) -RETURNING id, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at +RETURNING id, goal_order, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at ` type InsertActiveChatGoalParams struct { @@ -8922,6 +8926,7 @@ func (q *sqlQuerier) InsertActiveChatGoal(ctx context.Context, arg InsertActiveC var i ChatGoal err := row.Scan( &i.ID, + &i.GoalOrder, &i.RootChatID, &i.CreatedFromChatID, &i.CreatedFromMessageID, @@ -9458,7 +9463,7 @@ SET WHERE root_chat_id = $1::uuid AND status IN ('active', 'paused') -RETURNING id, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at +RETURNING id, goal_order, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at ` func (q *sqlQuerier) MarkCurrentChatGoalReplacedByRootChatID(ctx context.Context, rootChatID uuid.UUID) ([]ChatGoal, error) { @@ -9472,6 +9477,7 @@ func (q *sqlQuerier) MarkCurrentChatGoalReplacedByRootChatID(ctx context.Context var i ChatGoal if err := rows.Scan( &i.ID, + &i.GoalOrder, &i.RootChatID, &i.CreatedFromChatID, &i.CreatedFromMessageID, @@ -9510,7 +9516,7 @@ WHERE root_chat_id = $1::uuid AND id = $2::uuid AND status = 'active' -RETURNING id, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at +RETURNING id, goal_order, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at ` type PauseChatGoalByIDParams struct { @@ -9523,6 +9529,7 @@ func (q *sqlQuerier) PauseChatGoalByID(ctx context.Context, arg PauseChatGoalByI var i ChatGoal err := row.Scan( &i.ID, + &i.GoalOrder, &i.RootChatID, &i.CreatedFromChatID, &i.CreatedFromMessageID, @@ -9719,7 +9726,7 @@ WHERE root_chat_id = $1::uuid AND id = $2::uuid AND status = 'paused' -RETURNING id, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at +RETURNING id, goal_order, root_chat_id, created_from_chat_id, created_from_message_id, objective, status, completion_summary, created_by_user_id, completed_by_user_id, completed_by_agent, created_at, updated_at, completed_at, cleared_at, replaced_at ` type ResumeChatGoalByIDParams struct { @@ -9732,6 +9739,7 @@ func (q *sqlQuerier) ResumeChatGoalByID(ctx context.Context, arg ResumeChatGoalB var i ChatGoal err := row.Scan( &i.ID, + &i.GoalOrder, &i.RootChatID, &i.CreatedFromChatID, &i.CreatedFromMessageID, diff --git a/coderd/database/queries/chats.sql b/coderd/database/queries/chats.sql index 7bf95bcf10..e8b13e2018 100644 --- a/coderd/database/queries/chats.sql +++ b/coderd/database/queries/chats.sql @@ -1817,7 +1817,7 @@ WHERE root_chat_id = @root_chat_id::uuid ORDER BY created_at DESC, - id DESC + goal_order DESC LIMIT 1 ) AND status IN ('active', 'paused', 'complete'); @@ -1833,7 +1833,7 @@ WITH latest_goal_ids AS ( ORDER BY root_chat_id, created_at DESC, - id DESC + goal_order DESC ) SELECT chat_goals.*