From c4997af4d1802b74f6da380a4e763befdbfab04f Mon Sep 17 00:00:00 2001 From: Multica Eve Date: Tue, 7 Jul 2026 20:49:42 +0800 Subject: [PATCH] fix: archive autopilots on delete (#5042) Co-authored-by: Eve Co-authored-by: multica-agent --- server/internal/handler/autopilot.go | 29 ++------ .../internal/handler/autopilot_list_test.go | 50 +++++++++++++ .../handler/autopilot_subscriber_test.go | 70 +++++++++++++++---- server/pkg/db/generated/autopilot.sql.go | 25 ++++--- server/pkg/db/queries/autopilot.sql | 12 ++-- 5 files changed, 133 insertions(+), 53 deletions(-) diff --git a/server/internal/handler/autopilot.go b/server/internal/handler/autopilot.go index af4769c09..1080889e7 100644 --- a/server/internal/handler/autopilot.go +++ b/server/internal/handler/autopilot.go @@ -971,31 +971,10 @@ func (h *Handler) DeleteAutopilot(w http.ResponseWriter, r *http.Request) { return } - // autopilot_subscriber carries no DB-level foreign key/cascade (repo rule: - // referential cleanup lives in the application layer), so delete the - // subscriber template alongside the autopilot in one transaction. Without - // this, deleting an autopilot would orphan its subscriber rows. - tx, err := h.TxStarter.Begin(r.Context()) - if err != nil { - writeError(w, http.StatusInternalServerError, "failed to delete autopilot") - return - } - defer tx.Rollback(r.Context()) - qtx := h.Queries.WithTx(tx) - - if err := qtx.DeleteAutopilotSubscribersForAutopilot(r.Context(), idUUID); err != nil { - writeError(w, http.StatusInternalServerError, "failed to delete autopilot") - return - } - if err := qtx.DeleteAutopilotCollaboratorsForAutopilot(r.Context(), idUUID); err != nil { - writeError(w, http.StatusInternalServerError, "failed to delete autopilot") - return - } - if err := qtx.DeleteAutopilot(r.Context(), idUUID); err != nil { - writeError(w, http.StatusInternalServerError, "failed to delete autopilot") - return - } - if err := tx.Commit(r.Context()); err != nil { + // Product "delete" is archival: stop future triggers and hide the + // autopilot from default lists while preserving runs, tasks, webhook + // deliveries, subscribers, and collaborators as execution history. + if err := h.Queries.ArchiveAutopilot(r.Context(), idUUID); err != nil { writeError(w, http.StatusInternalServerError, "failed to delete autopilot") return } diff --git a/server/internal/handler/autopilot_list_test.go b/server/internal/handler/autopilot_list_test.go index b1704c181..c8bb6b9e0 100644 --- a/server/internal/handler/autopilot_list_test.go +++ b/server/internal/handler/autopilot_list_test.go @@ -114,3 +114,53 @@ func TestListAutopilots_DerivedFields(t *testing.T) { } } } + +func TestListAutopilots_DefaultExcludesArchived(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + ctx := context.Background() + + agentID := createHandlerTestAgent(t, "autopilot-list-archived-agent", []byte(`[]`)) + archived := insertListTestAutopilot(t, agentID, "list-archived-hidden") + if _, err := testPool.Exec(ctx, `UPDATE autopilot SET status = 'archived' WHERE id = $1`, archived); err != nil { + t.Fatalf("archive autopilot fixture: %v", err) + } + + w := httptest.NewRecorder() + testHandler.ListAutopilots(w, newRequest("GET", "/api/autopilots", nil)) + if w.Code != 200 { + t.Fatalf("ListAutopilots default: expected 200, got %d: %s", w.Code, w.Body.String()) + } + var body struct { + Autopilots []map[string]any `json:"autopilots"` + } + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("decode default list: %v", err) + } + for _, row := range body.Autopilots { + if row["id"] == archived { + t.Fatalf("archived autopilot %s appeared in default list", archived) + } + } + + w = httptest.NewRecorder() + testHandler.ListAutopilots(w, newRequest("GET", "/api/autopilots?status=archived", nil)) + if w.Code != 200 { + t.Fatalf("ListAutopilots archived: expected 200, got %d: %s", w.Code, w.Body.String()) + } + body.Autopilots = nil + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("decode archived list: %v", err) + } + found := false + for _, row := range body.Autopilots { + if row["id"] == archived { + found = true + break + } + } + if !found { + t.Fatalf("archived autopilot %s missing from status=archived list", archived) + } +} diff --git a/server/internal/handler/autopilot_subscriber_test.go b/server/internal/handler/autopilot_subscriber_test.go index 11aa3c0d7..2bb4d00ce 100644 --- a/server/internal/handler/autopilot_subscriber_test.go +++ b/server/internal/handler/autopilot_subscriber_test.go @@ -597,22 +597,30 @@ func TestAutopilotDispatchSkipsInboxWhenNoSubscribers(t *testing.T) { } } -// TestDeleteAutopilotRemovesSubscribers guards the app-layer cleanup that -// replaced the dropped autopilot_subscriber → autopilot ON DELETE CASCADE: -// deleting an autopilot must also delete its subscriber template rows in the -// same transaction, leaving no orphans behind. -func TestDeleteAutopilotRemovesSubscribers(t *testing.T) { +// TestDeleteAutopilotArchivesAndPreservesHistory guards the delete endpoint's +// product contract: user-facing delete archives the autopilot, hiding it from +// the default list and stopping future triggers, but preserves historical +// runs/tasks and subscriber configuration. +func TestDeleteAutopilotArchivesAndPreservesHistory(t *testing.T) { ctx := context.Background() var autopilotID string + var taskID string defer func() { + if taskID != "" { + testPool.Exec(ctx, `DELETE FROM agent_task_queue WHERE id = $1`, taskID) + } if autopilotID != "" { testPool.Exec(ctx, `DELETE FROM autopilot_subscriber WHERE autopilot_id = $1`, autopilotID) testPool.Exec(ctx, `DELETE FROM autopilot WHERE id = $1`, autopilotID) } }() - var agentID string - if err := testPool.QueryRow(ctx, `SELECT id FROM agent WHERE workspace_id = $1 LIMIT 1`, testWorkspaceID).Scan(&agentID); err != nil { + var agentID, runtimeID string + if err := testPool.QueryRow(ctx, ` + SELECT id, runtime_id FROM agent + WHERE workspace_id = $1 AND runtime_id IS NOT NULL + LIMIT 1 + `, testWorkspaceID).Scan(&agentID, &runtimeID); err != nil { t.Fatalf("load test agent: %v", err) } @@ -643,6 +651,24 @@ func TestDeleteAutopilotRemovesSubscribers(t *testing.T) { t.Fatalf("subscriber rows before delete = %d, want 1", before) } + var runID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO autopilot_run (autopilot_id, source, status) + VALUES ($1, 'manual', 'completed') + RETURNING id + `, autopilotID).Scan(&runID); err != nil { + t.Fatalf("create run: %v", err) + } + if err := testPool.QueryRow(ctx, ` + INSERT INTO agent_task_queue ( + agent_id, runtime_id, status, priority, autopilot_run_id + ) + VALUES ($1, $2, 'completed', 0, $3) + RETURNING id + `, agentID, runtimeID, runID).Scan(&taskID); err != nil { + t.Fatalf("create linked task: %v", err) + } + w = httptest.NewRecorder() req = newRequest("DELETE", "/api/autopilots/"+autopilotID+"?workspace_id="+testWorkspaceID, nil) req = withURLParam(req, "id", autopilotID) @@ -651,19 +677,35 @@ func TestDeleteAutopilotRemovesSubscribers(t *testing.T) { t.Fatalf("DeleteAutopilot: expected 204, got %d: %s", w.Code, w.Body.String()) } + var status string + if err := testPool.QueryRow(ctx, `SELECT status FROM autopilot WHERE id = $1`, autopilotID).Scan(&status); err != nil { + t.Fatalf("load autopilot after delete: %v", err) + } + if status != "archived" { + t.Fatalf("autopilot status after delete = %q, want archived", status) + } + var after int if err := testPool.QueryRow(ctx, `SELECT count(*) FROM autopilot_subscriber WHERE autopilot_id = $1`, autopilotID).Scan(&after); err != nil { t.Fatalf("count subscribers after delete: %v", err) } - if after != 0 { - t.Fatalf("subscriber rows after delete = %d, want 0 (app-layer cleanup)", after) + if after != 1 { + t.Fatalf("subscriber rows after delete = %d, want 1 (archival preserves config)", after) } - var autopilotRows int - if err := testPool.QueryRow(ctx, `SELECT count(*) FROM autopilot WHERE id = $1`, autopilotID).Scan(&autopilotRows); err != nil { - t.Fatalf("count autopilot after delete: %v", err) + var runRows int + if err := testPool.QueryRow(ctx, `SELECT count(*) FROM autopilot_run WHERE id = $1 AND autopilot_id = $2`, runID, autopilotID).Scan(&runRows); err != nil { + t.Fatalf("count run after delete: %v", err) } - if autopilotRows != 0 { - t.Fatalf("autopilot rows after delete = %d, want 0", autopilotRows) + if runRows != 1 { + t.Fatalf("autopilot_run rows after delete = %d, want 1 (archival preserves history)", runRows) + } + + var taskRunID string + if err := testPool.QueryRow(ctx, `SELECT autopilot_run_id::text FROM agent_task_queue WHERE id = $1`, taskID).Scan(&taskRunID); err != nil { + t.Fatalf("load linked task after delete: %v", err) + } + if taskRunID != runID { + t.Fatalf("task autopilot_run_id after delete = %q, want %q", taskRunID, runID) } } diff --git a/server/pkg/db/generated/autopilot.sql.go b/server/pkg/db/generated/autopilot.sql.go index b0b86b984..073a007c9 100644 --- a/server/pkg/db/generated/autopilot.sql.go +++ b/server/pkg/db/generated/autopilot.sql.go @@ -81,6 +81,17 @@ func (q *Queries) AdvanceTriggerNextRun(ctx context.Context, arg AdvanceTriggerN return err } +const archiveAutopilot = `-- name: ArchiveAutopilot :exec +UPDATE autopilot +SET status = 'archived', updated_at = now() +WHERE id = $1 +` + +func (q *Queries) ArchiveAutopilot(ctx context.Context, id pgtype.UUID) error { + _, err := q.db.Exec(ctx, archiveAutopilot, id) + return err +} + const createAutopilot = `-- name: CreateAutopilot :one INSERT INTO autopilot ( workspace_id, title, description, assignee_type, assignee_id, @@ -333,15 +344,6 @@ func (q *Queries) CreateAutopilotTrigger(ctx context.Context, arg CreateAutopilo return i, err } -const deleteAutopilot = `-- name: DeleteAutopilot :exec -DELETE FROM autopilot WHERE id = $1 -` - -func (q *Queries) DeleteAutopilot(ctx context.Context, id pgtype.UUID) error { - _, err := q.db.Exec(ctx, deleteAutopilot, id) - return err -} - const deleteAutopilotCollaborator = `-- name: DeleteAutopilotCollaborator :exec DELETE FROM autopilot_collaborator WHERE autopilot_id = $1 AND user_type = $2 AND user_id = $3 @@ -888,7 +890,10 @@ SELECT ), '')::text AS last_run_status FROM autopilot a WHERE a.workspace_id = $1 - AND ($2::text IS NULL OR a.status = $2) + AND ( + ($2::text IS NULL AND a.status <> 'archived') + OR a.status = $2 + ) ORDER BY a.created_at DESC ` diff --git a/server/pkg/db/queries/autopilot.sql b/server/pkg/db/queries/autopilot.sql index 347cd9258..fa7a98cdf 100644 --- a/server/pkg/db/queries/autopilot.sql +++ b/server/pkg/db/queries/autopilot.sql @@ -30,7 +30,10 @@ SELECT ), '')::text AS last_run_status FROM autopilot a WHERE a.workspace_id = $1 - AND (sqlc.narg('status')::text IS NULL OR a.status = sqlc.narg('status')) + AND ( + (sqlc.narg('status')::text IS NULL AND a.status <> 'archived') + OR a.status = sqlc.narg('status') + ) ORDER BY a.created_at DESC; -- name: GetAutopilot :one @@ -66,8 +69,10 @@ UPDATE autopilot SET WHERE id = $1 RETURNING *; --- name: DeleteAutopilot :exec -DELETE FROM autopilot WHERE id = $1; +-- name: ArchiveAutopilot :exec +UPDATE autopilot +SET status = 'archived', updated_at = now() +WHERE id = $1; -- name: UpdateAutopilotLastRunAt :exec UPDATE autopilot SET last_run_at = now(), updated_at = now() @@ -451,4 +456,3 @@ SELECT EXISTS ( -- Powers the per-row can_write flag on the list endpoint without an N+1. SELECT autopilot_id FROM autopilot_collaborator WHERE user_type = 'member' AND user_id = $1; -