From a98d895b671b8843eee3f71ee577089d72b598e4 Mon Sep 17 00:00:00 2001 From: Henry Date: Mon, 7 Sep 2026 04:27:27 +0800 Subject: [PATCH] chore(backend): remove dead office-session index classifier (#3455) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio --- .../sqlite/office_session_race_guard_test.go | 17 ++- .../sqlite/office_task_session_uniqueness.go | 54 -------- ...e_task_session_uniqueness_postgres_test.go | 92 -------------- .../office_task_session_uniqueness_test.go | 116 ------------------ 4 files changed, 13 insertions(+), 266 deletions(-) delete mode 100644 apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness.go delete mode 100644 apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness_postgres_test.go delete mode 100644 apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness_test.go diff --git a/apps/backend/internal/task/repository/sqlite/office_session_race_guard_test.go b/apps/backend/internal/task/repository/sqlite/office_session_race_guard_test.go index fb4efe5e2a..8c57956057 100644 --- a/apps/backend/internal/task/repository/sqlite/office_session_race_guard_test.go +++ b/apps/backend/internal/task/repository/sqlite/office_session_race_guard_test.go @@ -18,10 +18,19 @@ import ( "github.com/kandev/kandev/internal/testutil" ) -// insertOfficeSessionWithStartedAt is insertOfficeSession -// (office_task_session_uniqueness_test.go) with an explicit started_at, so -// ordering tests can control which row is "newer" independent of insertion -// order. +// officeSessionSeed describes a task_sessions row to insert directly, +// bypassing CreateTaskSession so tests can construct rows with a specific +// (task_id, agent_profile_id, state) shape. +type officeSessionSeed struct { + id string + taskID string + agentProfileID string + state models.TaskSessionState +} + +// insertOfficeSessionWithStartedAt inserts a task_sessions row directly with +// an explicit started_at, so ordering tests can control which row is "newer" +// independent of insertion order. func insertOfficeSessionWithStartedAt(t *testing.T, db *sqlx.DB, s officeSessionSeed, startedAt time.Time) { t.Helper() now := time.Now().UTC() diff --git a/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness.go b/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness.go deleted file mode 100644 index 271f74ad45..0000000000 --- a/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness.go +++ /dev/null @@ -1,54 +0,0 @@ -package sqlite - -import ( - "errors" - "strings" - - "github.com/jackc/pgx/v5/pgconn" -) - -// officeTaskSessionIndexName names the partial unique index that would -// enforce at most one "live" task_sessions row per (task_id, -// agent_profile_id) pair. It is not wired into base_schema.go yet — see -// ErrOfficeSessionRaceConflict's doc comment in errors.go for why — but -// naming it here keeps isOfficeTaskSessionUniqueViolation attributable to -// this constraint specifically once it lands. Mirrors the (workspace_id, -// external_id) classification in task_external_id.go. -const officeTaskSessionIndexName = "uniq_office_task_session" - -// sqliteOfficeTaskSessionViolationMessage is the substring go-sqlite3 puts in -// a UNIQUE-constraint error for this specific index. SQLite's driver exposes -// no typed access to the violated index's name, only the column list of the -// failing index ("UNIQUE constraint failed: task_sessions.task_id, -// task_sessions.agent_profile_id"); that column pair is unique to -// uniq_office_task_session in this table (the only other unique constraint is -// the id primary key), so matching it attributes the violation correctly — a -// bare "UNIQUE constraint failed" match would also fire on the primary key. -// -// SQLite lists the columns in the index's own DEFINITION order, not table -// column order (verified directly: an index declared ON t(b, a) reports -// "t.b, t.a"). This message is therefore contingent on the eventual -// uniq_office_task_session DDL declaring its columns as -// (task_id, agent_profile_id) in that order — flipping the order in the -// migration silently deadens this match again, with the code, tests, and -// comments all still reading correct. -const sqliteOfficeTaskSessionViolationMessage = "UNIQUE constraint failed: task_sessions.task_id, task_sessions.agent_profile_id" - -// isOfficeTaskSessionUniqueViolation reports whether err is a violation of -// uniq_office_task_session specifically, not any unique violation. On -// PostgreSQL it inspects the typed pgconn.PgError's constraint name; on -// SQLite (no typed access to the constraint name) it matches the column-list -// message documented above. A plain strings.Contains(err.Error(), the index -// name) is not sufficient by itself: Postgres embeds the constraint name in -// its error text, but SQLite never does, so that check alone would leave -// this classification dead on SQLite even with the index in place. -func isOfficeTaskSessionUniqueViolation(err error) bool { - if err == nil { - return false - } - var pgErr *pgconn.PgError - if errors.As(err, &pgErr) { - return pgErr.Code == "23505" && pgErr.ConstraintName == officeTaskSessionIndexName - } - return strings.Contains(err.Error(), sqliteOfficeTaskSessionViolationMessage) -} diff --git a/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness_postgres_test.go b/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness_postgres_test.go deleted file mode 100644 index 79b82d8e95..0000000000 --- a/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness_postgres_test.go +++ /dev/null @@ -1,92 +0,0 @@ -package sqlite - -import ( - "testing" - "time" - - "github.com/kandev/kandev/internal/testutil" -) - -// TestIsOfficeTaskSessionUniqueViolation_ClassifiesPostgresConstraint -// exercises isOfficeTaskSessionUniqueViolation directly against a real -// PostgreSQL unique-constraint violation. The classifier's two production -// call sites (createTaskSession's INSERT and updateTaskSessionWithStateGuard's -// UPDATE) were deleted per the system design's "Baseline" instruction — -// office session-uniqueness enforcement now lives only in -// CreateOfficeTaskSession's in-transaction guard (session.go), which never -// inspects driver error types — so this function is intentionally -// referenced only by this test and its SQLite sibling -// (TestIsOfficeTaskSessionUniqueViolation_ClassifiesSQLiteMessage). -// uniq_office_task_session is not wired into base_schema.go, so this test -// builds and drops its own scoped index rather than relying on production -// schema, and drives the raw insert directly instead of through -// CreateTaskSession/UpdateTaskSessionIfCurrentState, since neither -// production call path translates this error anymore. -func TestIsOfficeTaskSessionUniqueViolation_ClassifiesPostgresConstraint(t *testing.T) { - db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) - repo, err := NewWithDB(db, db, nil) - if err != nil { - t.Fatalf("init postgres schema: %v", err) - } - const taskID = "task-classify-pg" - seedPostgresTask(t, repo, taskID) - agent := "agent-classify-pg" - - if _, err := repo.db.Exec(` - CREATE UNIQUE INDEX uniq_office_task_session - ON task_sessions(task_id, agent_profile_id) - WHERE agent_profile_id IS NOT NULL AND state IN ( - 'CREATED', 'STARTING', 'RUNNING', 'IDLE', 'WAITING_FOR_INPUT' - ) - `); err != nil { - t.Fatalf("create scoped index: %v", err) - } - t.Cleanup(func() { - _, _ = repo.db.Exec(`DROP INDEX IF EXISTS uniq_office_task_session`) - }) - - now := time.Now().UTC() - if _, err := repo.db.Exec(repo.db.Rebind(` - INSERT INTO task_sessions (id, task_id, agent_profile_id, state, started_at, updated_at) - VALUES (?, ?, ?, 'CREATED', ?, ?) - `), "sess-classify-pg-1", taskID, agent, now, now); err != nil { - t.Fatalf("seed first office session: %v", err) - } - - _, err = repo.db.Exec(repo.db.Rebind(` - INSERT INTO task_sessions (id, task_id, agent_profile_id, state, started_at, updated_at) - VALUES (?, ?, ?, 'CREATED', ?, ?) - `), "sess-classify-pg-2", taskID, agent, now, now) - if err == nil { - t.Fatal("expected inserting a second live row for the same pair to violate the scoped index") - } - if !isOfficeTaskSessionUniqueViolation(err) { - t.Fatalf("expected err to classify as an office task session unique violation, got: %v", err) - } -} - -// TestIsOfficeTaskSessionUniqueViolation_IgnoresUnrelatedPostgresViolation -// asserts the classifier does not fire on an unrelated PostgreSQL -// unique-constraint violation (the tasks primary key here), mirroring the -// SQLite sibling's negative case. -func TestIsOfficeTaskSessionUniqueViolation_IgnoresUnrelatedPostgresViolation(t *testing.T) { - db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) - repo, err := NewWithDB(db, db, nil) - if err != nil { - t.Fatalf("init postgres schema: %v", err) - } - const taskID = "task-classify-pg-2" - seedPostgresTask(t, repo, taskID) - - now := time.Now().UTC() - _, err = repo.db.Exec(repo.db.Rebind(` - INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) - VALUES (?, ?, ?, ?, ?) - `), taskID, "ws-"+taskID, taskID, now, now) - if err == nil { - t.Fatal("expected duplicate task id to violate the tasks primary key") - } - if isOfficeTaskSessionUniqueViolation(err) { - t.Fatalf("expected primary-key violation not to be misclassified as an office task session unique violation: %v", err) - } -} diff --git a/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness_test.go b/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness_test.go deleted file mode 100644 index a548318998..0000000000 --- a/apps/backend/internal/task/repository/sqlite/office_task_session_uniqueness_test.go +++ /dev/null @@ -1,116 +0,0 @@ -package sqlite - -import ( - "testing" - "time" - - "github.com/jmoiron/sqlx" - - "github.com/kandev/kandev/internal/task/models" -) - -// officeSessionSeed describes a task_sessions row to insert directly, -// bypassing CreateTaskSession so tests can construct rows with a specific -// (task_id, agent_profile_id, state) shape. -type officeSessionSeed struct { - id string - taskID string - agentProfileID string - state models.TaskSessionState -} - -func insertOfficeSession(t *testing.T, db *sqlx.DB, s officeSessionSeed) { - t.Helper() - now := time.Now().UTC() - _, err := db.Exec(` - INSERT INTO task_sessions ( - id, task_id, agent_profile_id, executor_id, executor_profile_id, environment_id, - repository_id, base_branch, base_commit_sha, - agent_profile_snapshot, executor_snapshot, environment_snapshot, repository_snapshot, - state, error_message, metadata, started_at, completed_at, updated_at, - is_primary, review_status, is_passthrough, task_environment_id - ) VALUES (?, ?, ?, '', '', '', '', '', '', - '{}', '{}', '{}', '{}', - ?, '', '{}', ?, NULL, ?, - 0, '', 0, '') - `, s.id, s.taskID, s.agentProfileID, string(s.state), now, now) - if err != nil { - t.Fatalf("insert office session %s: %v", s.id, err) - } -} - -// TestIsOfficeTaskSessionUniqueViolation_ClassifiesSQLiteMessage exercises -// isOfficeTaskSessionUniqueViolation directly against a real SQLite -// UNIQUE-constraint error. The classifier's two production call sites -// (createTaskSession's INSERT and updateTaskSessionWithStateGuard's UPDATE) -// were deleted per the system design's "Baseline" instruction — office -// session-uniqueness enforcement lives only in CreateOfficeTaskSession's -// in-transaction guard now (session.go), which never parses driver error -// text — so this function is intentionally referenced only by this test -// file. uniq_office_task_session is not wired into base_schema.go — a -// table-wide partial index on this pair broke live kanban-relaunch and -// workflow-replacement flows (see the follow-up card linked from -// ErrOfficeSessionRaceConflict's doc comment) — so this test builds and -// drops its own scoped index rather than relying on production schema. -func TestIsOfficeTaskSessionUniqueViolation_ClassifiesSQLiteMessage(t *testing.T) { - repo := newRepoForHealTests(t) - insertTask(t, repo.db, "task-classify-1") - agent := "agent-classify" - - if _, err := repo.db.Exec(` - CREATE UNIQUE INDEX uniq_office_task_session_probe - ON task_sessions(task_id, agent_profile_id) - WHERE agent_profile_id IS NOT NULL AND state IN ( - 'CREATED', 'STARTING', 'RUNNING', 'IDLE', 'WAITING_FOR_INPUT' - ) - `); err != nil { - t.Fatalf("create scoped index: %v", err) - } - t.Cleanup(func() { - _, _ = repo.db.Exec(`DROP INDEX IF EXISTS uniq_office_task_session_probe`) - }) - - insertOfficeSession(t, repo.db, officeSessionSeed{ - id: "sess-classify-1", taskID: "task-classify-1", agentProfileID: agent, - state: models.TaskSessionStateCreated, - }) - - _, err := repo.db.Exec(` - INSERT INTO task_sessions ( - id, task_id, agent_profile_id, executor_id, executor_profile_id, environment_id, - repository_id, base_branch, base_commit_sha, - agent_profile_snapshot, executor_snapshot, environment_snapshot, repository_snapshot, - state, error_message, metadata, started_at, completed_at, updated_at, - is_primary, review_status, is_passthrough, task_environment_id - ) VALUES (?, 'task-classify-1', ?, '', '', '', '', '', '', - '{}', '{}', '{}', '{}', - 'CREATED', '', '{}', datetime('now'), NULL, datetime('now'), - 0, '', 0, '') - `, "sess-classify-2", agent) - if err == nil { - t.Fatal("expected inserting a second live row for the same pair to violate the scoped index") - } - if !isOfficeTaskSessionUniqueViolation(err) { - t.Fatalf("expected err to classify as an office task session unique violation, got: %v", err) - } -} - -// TestIsOfficeTaskSessionUniqueViolation_IgnoresUnrelatedViolation asserts -// the classifier does not fire on an unrelated UNIQUE-constraint error (the -// tasks primary key here), which a bare "UNIQUE constraint failed" substring -// match would have misclassified. -func TestIsOfficeTaskSessionUniqueViolation_IgnoresUnrelatedViolation(t *testing.T) { - repo := newRepoForHealTests(t) - insertTask(t, repo.db, "task-classify-2") - - _, err := repo.db.Exec(` - INSERT INTO tasks (id, workspace_id, workflow_id, workflow_step_id, title, description, state, created_at, updated_at) - VALUES ('task-classify-2', '', '', '', 'dup', '', 'todo', datetime('now'), datetime('now')) - `) - if err == nil { - t.Fatal("expected duplicate task id to violate the tasks primary key") - } - if isOfficeTaskSessionUniqueViolation(err) { - t.Fatalf("expected primary-key violation not to be misclassified as an office task session unique violation: %v", err) - } -}