fix(labels): sweep resource-label junctions on bulk deletes (MUL-4479) (#5348)

* fix(labels): sweep resource-label junctions on bulk deletes (MUL-4479)

Runtime, runtime-profile, and workspace deletion hard-delete their agents
and skills without clearing agent_to_label / skill_to_label. Migration 173
dropped the junction foreign keys, so these rows are no longer cascade-cleaned;
once resource labels are enabled, every labelled agent/skill removed through one
of these bulk paths leaves a permanent, invisible orphan junction row.

Clear the junctions in the same transaction as the owner delete, before the
owning rows disappear:

- DeleteAgentRuntime / ArchiveAgentsAndDeleteRuntime / DeleteRuntimeProfile:
  DeleteAgentLabelAssignmentsByRuntime, ahead of the archived-agent hard-delete.
- DeleteWorkspace: DeleteAgentLabelAssignmentsByWorkspace +
  DeleteSkillLabelAssignmentsByWorkspace, ahead of the workspace cascade.

Adds regression tests for all four delete paths.

Follow-up to #5345 (Elon review). Resource labels must stay disabled until this
lands.

Co-authored-by: multica-agent <github@multica.ai>

* fix(labels): make workspace cleanup atomic

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: Eve <eve@multica-ai.local>
This commit is contained in:
Bohan Jiang
2026-07-13 20:07:24 +08:00
committed by GitHub
parent 8567ebffd9
commit 45ff984518
7 changed files with 390 additions and 14 deletions

View File

@@ -0,0 +1,305 @@
package handler
import (
"context"
"net/http"
"net/http/httptest"
"testing"
"github.com/google/uuid"
)
// Resource-label junction tables (agent_to_label / skill_to_label) deliberately
// carry no foreign keys, so every bulk hard-delete entry point that removes the
// owning agents/skills must clear their label links in the same transaction.
// These tests pin that cleanup on the four batch paths that never pass through a
// per-entity delete: runtime delete (strict + cascade), runtime-profile delete,
// and workspace delete. Without the sweep, a labelled agent/skill leaves a
// permanent, invisible orphan row once resource labels are enabled.
// insertLabelRow creates a real issue_label so the seeded junction row is valid
// regardless of whether a given database still carries the pre-release label_id
// foreign key. Registers cleanup.
func insertLabelRow(t *testing.T, ctx context.Context, workspaceID, resourceType string) string {
t.Helper()
var labelID string
if err := testPool.QueryRow(ctx, `
INSERT INTO issue_label (workspace_id, resource_type, name, color)
VALUES ($1, $2, $3, '#3b82f6')
RETURNING id
`, workspaceID, resourceType, resourceType+"-"+uuid.NewString()[:8]).Scan(&labelID); err != nil {
t.Fatalf("insert issue_label: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM issue_label WHERE id = $1`, labelID)
})
return labelID
}
func seedAgentLabel(t *testing.T, ctx context.Context, workspaceID, agentID string) {
t.Helper()
labelID := insertLabelRow(t, ctx, workspaceID, "agent")
if _, err := testPool.Exec(ctx,
`INSERT INTO agent_to_label (agent_id, label_id) VALUES ($1, $2)`,
agentID, labelID); err != nil {
t.Fatalf("seed agent_to_label: %v", err)
}
}
func seedSkillLabel(t *testing.T, ctx context.Context, workspaceID, skillID string) {
t.Helper()
labelID := insertLabelRow(t, ctx, workspaceID, "skill")
if _, err := testPool.Exec(ctx,
`INSERT INTO skill_to_label (skill_id, label_id) VALUES ($1, $2)`,
skillID, labelID); err != nil {
t.Fatalf("seed skill_to_label: %v", err)
}
}
func countAgentLabelAssignments(t *testing.T, ctx context.Context, agentID string) int {
t.Helper()
var n int
if err := testPool.QueryRow(ctx,
`SELECT count(*) FROM agent_to_label WHERE agent_id = $1`, agentID).Scan(&n); err != nil {
t.Fatalf("count agent_to_label: %v", err)
}
return n
}
func countSkillLabelAssignments(t *testing.T, ctx context.Context, skillID string) int {
t.Helper()
var n int
if err := testPool.QueryRow(ctx,
`SELECT count(*) FROM skill_to_label WHERE skill_id = $1`, skillID).Scan(&n); err != nil {
t.Fatalf("count skill_to_label: %v", err)
}
return n
}
// TestDeleteAgentRuntime_CleansAgentLabelAssignments: the strict runtime delete
// hard-deletes the archived agent bound to the runtime; its label link must go
// with it in the same transaction.
func TestDeleteAgentRuntime_CleansAgentLabelAssignments(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
runtimeID := seedIsolatedRuntime(t, "Label Cleanup Runtime")
agentID := seedAgentOnRuntime(t, runtimeID, "Label Cleanup Archived Agent", true)
seedAgentLabel(t, ctx, testWorkspaceID, agentID)
w := httptest.NewRecorder()
req := newRequest("DELETE", "/api/runtimes/"+runtimeID, nil)
req = withURLParam(req, "runtimeId", runtimeID)
testHandler.DeleteAgentRuntime(w, req)
if w.Code != http.StatusOK {
t.Fatalf("DeleteAgentRuntime: expected 200, got %d: %s", w.Code, w.Body.String())
}
if agentExists(t, agentID) {
t.Fatalf("archived agent should have been hard-deleted with its runtime")
}
if n := countAgentLabelAssignments(t, ctx, agentID); n != 0 {
t.Fatalf("agent_to_label rows survived runtime delete: %d (orphaned once resource labels ship)", n)
}
}
// TestArchiveAgentsAndDeleteRuntime_CleansAgentLabelAssignments: the cascade
// endpoint archives the active agent and hard-deletes it with the runtime, so
// the label link must be swept too.
func TestArchiveAgentsAndDeleteRuntime_CleansAgentLabelAssignments(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
runtimeID := createCascadeFixtureRuntime(t, ctx, "Label Cascade Runtime")
agentID := createCascadeFixtureAgent(t, ctx, runtimeID, "Label Cascade Agent")
seedAgentLabel(t, ctx, testWorkspaceID, agentID)
w := httptest.NewRecorder()
req := newRequest("POST", "/api/runtimes/"+runtimeID+"/archive-agents-and-delete",
map[string]any{"expected_active_agent_ids": []string{agentID}})
req = withURLParam(req, "runtimeId", runtimeID)
testHandler.ArchiveAgentsAndDeleteRuntime(w, req)
if w.Code != http.StatusOK {
t.Fatalf("ArchiveAgentsAndDeleteRuntime: expected 200, got %d: %s", w.Code, w.Body.String())
}
if n := countAgentLabelAssignments(t, ctx, agentID); n != 0 {
t.Fatalf("agent_to_label rows survived cascade runtime delete: %d", n)
}
}
// TestDeleteRuntimeProfile_CleansAgentLabelAssignments: deleting a runtime
// profile hard-deletes the archived agents on each of its runtimes; their label
// links must go with them.
func TestDeleteRuntimeProfile_CleansAgentLabelAssignments(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
profileID := insertRuntimeProfileFixture(t, ctx, "Label Cleanup Profile", "codex", "company-codex-label")
runtimeID := insertProfileRuntimeFixture(t, ctx, profileID, "Label Cleanup Profile Runtime", "codex")
agentID := createCascadeFixtureAgent(t, ctx, runtimeID, "Label Cleanup Profile Agent")
if _, err := testPool.Exec(ctx, `UPDATE agent SET archived_at = now() WHERE id = $1`, agentID); err != nil {
t.Fatalf("archive agent: %v", err)
}
seedAgentLabel(t, ctx, testWorkspaceID, agentID)
w := httptest.NewRecorder()
req := newRequest("DELETE", "/api/workspaces/"+testWorkspaceID+"/runtime-profiles/"+profileID, nil)
req = withURLParams(req, "id", testWorkspaceID, "profileId", profileID)
testHandler.DeleteRuntimeProfile(w, req)
if w.Code != http.StatusNoContent {
t.Fatalf("DeleteRuntimeProfile: expected 204, got %d: %s", w.Code, w.Body.String())
}
if agentExists(t, agentID) {
t.Fatalf("archived agent should have been hard-deleted with its runtime profile")
}
if n := countAgentLabelAssignments(t, ctx, agentID); n != 0 {
t.Fatalf("agent_to_label rows survived runtime-profile delete: %d", n)
}
}
func seedWorkspaceResourceLabelFixture(t *testing.T, ctx context.Context, slug string) (string, string, string) {
t.Helper()
_, _ = testPool.Exec(ctx, `DELETE FROM workspace WHERE slug = $1`, slug)
var wsID string
if err := testPool.QueryRow(ctx, `
INSERT INTO workspace (name, slug, description)
VALUES ($1, $2, $3)
RETURNING id
`, "Handler Test Delete Labels", slug, "resource-label atomic cleanup test").Scan(&wsID); err != nil {
t.Fatalf("create workspace: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM workspace WHERE id = $1`, wsID)
})
if _, err := testPool.Exec(ctx,
`INSERT INTO member (workspace_id, user_id, role) VALUES ($1, $2, 'owner')`,
wsID, testUserID); err != nil {
t.Fatalf("create owner member: %v", err)
}
// agent.runtime_id is NOT NULL, so the labelled agent needs a runtime in the
// same workspace. Both cascade away with the workspace.
var runtimeID string
if err := testPool.QueryRow(ctx, `
INSERT INTO agent_runtime (
workspace_id, daemon_id, name, runtime_mode, provider, status,
device_info, metadata, owner_id, last_seen_at
)
VALUES ($1, NULL, 'ws-label-runtime', 'cloud', 'ws-label-test', 'online', 'dev', '{}'::jsonb, $2, now())
RETURNING id
`, wsID, testUserID).Scan(&runtimeID); err != nil {
t.Fatalf("insert runtime: %v", err)
}
var agentID string
if err := testPool.QueryRow(ctx, `
INSERT INTO agent (
workspace_id, name, description, runtime_mode, runtime_config,
runtime_id, visibility, max_concurrent_tasks, owner_id
)
VALUES ($1, 'ws-label-agent', '', 'cloud', '{}'::jsonb, $2, 'workspace', 1, $3)
RETURNING id
`, wsID, runtimeID, testUserID).Scan(&agentID); err != nil {
t.Fatalf("insert agent: %v", err)
}
var skillID string
if err := testPool.QueryRow(ctx, `
INSERT INTO skill (workspace_id, name, description, content, config, created_by)
VALUES ($1, 'ws-label-skill', 'fixture', '# x', '{}'::jsonb, $2)
RETURNING id
`, wsID, testUserID).Scan(&skillID); err != nil {
t.Fatalf("insert skill: %v", err)
}
seedAgentLabel(t, ctx, wsID, agentID)
seedSkillLabel(t, ctx, wsID, skillID)
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent_to_label WHERE agent_id = $1`, agentID)
_, _ = testPool.Exec(context.Background(), `DELETE FROM skill_to_label WHERE skill_id = $1`, skillID)
_, _ = testPool.Exec(context.Background(), `DELETE FROM workspace WHERE id = $1`, wsID)
})
return wsID, agentID, skillID
}
// TestDeleteWorkspace_CleansResourceLabelAssignments: workspace delete cascades
// away the agents and skills, but the junction tables have no workspace_id and
// no foreign key, so both must be swept before the cascade or they orphan.
func TestDeleteWorkspace_CleansResourceLabelAssignments(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
wsID, agentID, skillID := seedWorkspaceResourceLabelFixture(t, ctx, "handler-tests-delete-labels")
w := httptest.NewRecorder()
req := newRequest("DELETE", "/api/workspaces/"+wsID, nil)
req = withURLParam(req, "id", wsID)
testHandler.DeleteWorkspace(w, req)
if w.Code != http.StatusNoContent {
t.Fatalf("DeleteWorkspace: expected 204, got %d: %s", w.Code, w.Body.String())
}
if n := countAgentLabelAssignments(t, ctx, agentID); n != 0 {
t.Fatalf("agent_to_label rows survived workspace delete: %d", n)
}
if n := countSkillLabelAssignments(t, ctx, skillID); n != 0 {
t.Fatalf("skill_to_label rows survived workspace delete: %d", n)
}
}
// TestDeleteWorkspace_RollsBackResourceLabelCleanup verifies the cleanup and
// final workspace delete share one database statement. A restrictive test-only
// foreign key makes the final delete fail; both junction rows must remain.
func TestDeleteWorkspace_RollsBackResourceLabelCleanup(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
wsID, agentID, skillID := seedWorkspaceResourceLabelFixture(t, ctx, "handler-tests-delete-labels-rollback")
const guardTable = "workspace_delete_resource_label_rollback_guard"
_, _ = testPool.Exec(ctx, `DROP TABLE IF EXISTS `+guardTable)
if _, err := testPool.Exec(ctx, `
CREATE TABLE `+guardTable+` (
workspace_id UUID NOT NULL REFERENCES workspace(id)
)
`); err != nil {
t.Fatalf("create workspace delete guard: %v", err)
}
if _, err := testPool.Exec(ctx, `INSERT INTO `+guardTable+` (workspace_id) VALUES ($1)`, wsID); err != nil {
t.Fatalf("insert workspace delete guard: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DROP TABLE IF EXISTS `+guardTable)
})
w := httptest.NewRecorder()
req := newRequest("DELETE", "/api/workspaces/"+wsID, nil)
req = withURLParam(req, "id", wsID)
testHandler.DeleteWorkspace(w, req)
if w.Code != http.StatusInternalServerError {
t.Fatalf("DeleteWorkspace: expected 500, got %d: %s", w.Code, w.Body.String())
}
var workspaceExists bool
if err := testPool.QueryRow(ctx, `SELECT EXISTS (SELECT 1 FROM workspace WHERE id = $1)`, wsID).Scan(&workspaceExists); err != nil {
t.Fatalf("check workspace after failed delete: %v", err)
}
if !workspaceExists {
t.Fatal("workspace was removed despite the injected delete failure")
}
if n := countAgentLabelAssignments(t, ctx, agentID); n != 1 {
t.Fatalf("agent_to_label rows after failed workspace delete = %d, want 1", n)
}
if n := countSkillLabelAssignments(t, ctx, skillID); n != 1 {
t.Fatalf("skill_to_label rows after failed workspace delete = %d, want 1", n)
}
}

View File

@@ -772,6 +772,13 @@ func (h *Handler) DeleteAgentRuntime(w http.ResponseWriter, r *http.Request) {
writeError(w, http.StatusInternalServerError, "failed to clean up chat pins")
return
}
// agent_to_label has no agent_id FK, so clear the runtime's agents' label
// links before those agents are hard-deleted below; otherwise they survive
// as invisible orphan rows once resource labels are enabled.
if err := qtx.DeleteAgentLabelAssignmentsByRuntime(r.Context(), rt.ID); err != nil {
writeError(w, http.StatusInternalServerError, "failed to clean up agent label assignments")
return
}
if err := qtx.DeleteArchivedAgentsByRuntime(r.Context(), rt.ID); err != nil {
writeError(w, http.StatusInternalServerError, "failed to clean up archived agents")
return
@@ -1032,6 +1039,13 @@ func (h *Handler) ArchiveAgentsAndDeleteRuntime(w http.ResponseWriter, r *http.R
writeError(w, http.StatusInternalServerError, "failed to clean up chat pins")
return
}
// agent_to_label has no agent_id FK, so clear the runtime's agents' label
// links before those agents are hard-deleted below; otherwise they survive
// as invisible orphan rows once resource labels are enabled.
if err := qtx.DeleteAgentLabelAssignmentsByRuntime(r.Context(), rt.ID); err != nil {
writeError(w, http.StatusInternalServerError, "failed to clean up agent label assignments")
return
}
if err := qtx.DeleteArchivedAgentsByRuntime(r.Context(), rt.ID); err != nil {
writeError(w, http.StatusInternalServerError, "failed to clean up archived agents")
return

View File

@@ -467,6 +467,13 @@ func (h *Handler) DeleteRuntimeProfile(w http.ResponseWriter, r *http.Request) {
writeError(w, http.StatusInternalServerError, "failed to clean up channel installations")
return
}
// agent_to_label has no agent_id FK; clear the runtime's agents' label
// links before the archived agents are hard-deleted so they don't leak
// as orphan rows once resource labels are enabled.
if err := qtx.DeleteAgentLabelAssignmentsByRuntime(r.Context(), rid); err != nil {
writeError(w, http.StatusInternalServerError, "failed to clean up agent label assignments")
return
}
if err := qtx.DeleteArchivedAgentsByRuntime(r.Context(), rid); err != nil {
writeError(w, http.StatusInternalServerError, "failed to clean up archived agents")
return

View File

@@ -152,6 +152,25 @@ func (q *Queries) DeleteAgentLabelAssignmentsByLabel(ctx context.Context, labelI
return err
}
const deleteAgentLabelAssignmentsByRuntime = `-- name: DeleteAgentLabelAssignmentsByRuntime :exec
DELETE FROM agent_to_label
WHERE agent_id IN (SELECT id FROM agent WHERE runtime_id = $1)
`
// The single-entity cleanups above cover one agent/skill at a time. The runtime
// variant below covers runtime and runtime-profile bulk hard deletes, where the
// owning agents disappear without passing through a per-entity delete.
// Workspace-wide cleanup lives in DeleteWorkspace so it is atomic with that
// workspace's existing multi-table teardown.
// Runtime teardown hard-deletes every agent bound to the runtime (archived and
// system; active agents are refused by a 409 guard). Clear their label links by
// runtime so none survive the agent hard-delete.
func (q *Queries) DeleteAgentLabelAssignmentsByRuntime(ctx context.Context, runtimeID pgtype.UUID) error {
_, err := q.db.Exec(ctx, deleteAgentLabelAssignmentsByRuntime, runtimeID)
return err
}
const deleteIssueLabelAssignmentsByLabel = `-- name: DeleteIssueLabelAssignmentsByLabel :exec
DELETE FROM issue_to_label WHERE label_id = $1

View File

@@ -55,6 +55,18 @@ const deleteWorkspace = `-- name: DeleteWorkspace :exec
WITH ws_installations AS (
SELECT id FROM channel_installation WHERE workspace_id = $1
),
ws_agents AS (
SELECT id FROM agent WHERE workspace_id = $1
),
ws_skills AS (
SELECT id FROM skill WHERE workspace_id = $1
),
cleared_agent_label_assignments AS (
DELETE FROM agent_to_label WHERE agent_id IN (SELECT id FROM ws_agents)
),
cleared_skill_label_assignments AS (
DELETE FROM skill_to_label WHERE skill_id IN (SELECT id FROM ws_skills)
),
cleared_chat_sessions AS (
DELETE FROM channel_chat_session_binding WHERE installation_id IN (SELECT id FROM ws_installations)
RETURNING chat_session_id
@@ -90,13 +102,10 @@ deleted_pending_check_suites AS (
DELETE FROM workspace WHERE workspace.id = $1
`
// The channel_* tables carry NO FK to workspace (MUL-3515 §4), so — unlike the
// CASCADE-backed tables the DELETE below sweeps — they are not cleaned up
// implicitly. Remove this workspace's channel installations, and every dependent
// row of each, here so a deleted workspace never leaves an orphaned installation
// occupying its bot's (channel_type, config->>'app_id') routing slot, which would
// make that bot un-rebindable anywhere until an operator hand-deletes the row
// (#4810). All in one statement so it commits atomically with the workspace row.
// The channel_* tables (MUL-3515 §4) and resource-label junctions carry NO FK to
// workspace, so — unlike the CASCADE-backed tables the DELETE below sweeps —
// they are not cleaned up implicitly. Remove their workspace-owned rows here so
// they commit or roll back atomically with the workspace row.
func (q *Queries) DeleteWorkspace(ctx context.Context, id pgtype.UUID) error {
_, err := q.db.Exec(ctx, deleteWorkspace, id)
return err

View File

@@ -55,6 +55,19 @@ DELETE FROM agent_to_label WHERE agent_id = $1;
-- name: DeleteSkillLabelAssignmentsBySkill :exec
DELETE FROM skill_to_label WHERE skill_id = $1;
-- The single-entity cleanups above cover one agent/skill at a time. The runtime
-- variant below covers runtime and runtime-profile bulk hard deletes, where the
-- owning agents disappear without passing through a per-entity delete.
-- Workspace-wide cleanup lives in DeleteWorkspace so it is atomic with that
-- workspace's existing multi-table teardown.
-- name: DeleteAgentLabelAssignmentsByRuntime :exec
-- Runtime teardown hard-deletes every agent bound to the runtime (archived and
-- system; active agents are refused by a 409 guard). Clear their label links by
-- runtime so none survive the agent hard-delete.
DELETE FROM agent_to_label
WHERE agent_id IN (SELECT id FROM agent WHERE runtime_id = $1);
-- name: AttachLabelToIssue :exec
-- Workspace-guarded INSERT: the WHERE EXISTS clauses ensure both the issue
-- and the label belong to the given workspace. A future caller that forgets

View File

@@ -39,16 +39,25 @@ WHERE id = $1
RETURNING issue_counter;
-- name: DeleteWorkspace :exec
-- The channel_* tables carry NO FK to workspace (MUL-3515 §4), so — unlike the
-- CASCADE-backed tables the DELETE below sweeps — they are not cleaned up
-- implicitly. Remove this workspace's channel installations, and every dependent
-- row of each, here so a deleted workspace never leaves an orphaned installation
-- occupying its bot's (channel_type, config->>'app_id') routing slot, which would
-- make that bot un-rebindable anywhere until an operator hand-deletes the row
-- (#4810). All in one statement so it commits atomically with the workspace row.
-- The channel_* tables (MUL-3515 §4) and resource-label junctions carry NO FK to
-- workspace, so — unlike the CASCADE-backed tables the DELETE below sweeps —
-- they are not cleaned up implicitly. Remove their workspace-owned rows here so
-- they commit or roll back atomically with the workspace row.
WITH ws_installations AS (
SELECT id FROM channel_installation WHERE workspace_id = $1
),
ws_agents AS (
SELECT id FROM agent WHERE workspace_id = $1
),
ws_skills AS (
SELECT id FROM skill WHERE workspace_id = $1
),
cleared_agent_label_assignments AS (
DELETE FROM agent_to_label WHERE agent_id IN (SELECT id FROM ws_agents)
),
cleared_skill_label_assignments AS (
DELETE FROM skill_to_label WHERE skill_id IN (SELECT id FROM ws_skills)
),
cleared_chat_sessions AS (
DELETE FROM channel_chat_session_binding WHERE installation_id IN (SELECT id FROM ws_installations)
RETURNING chat_session_id