diff --git a/server/internal/handler/chat.go b/server/internal/handler/chat.go index b2613157e8..cece329869 100644 --- a/server/internal/handler/chat.go +++ b/server/internal/handler/chat.go @@ -459,6 +459,13 @@ func (h *Handler) DeleteChatSession(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusInternalServerError, "failed to delete chat session binding") return } + // channel_outbound_card_message is also keyed by chat_session_id with no FK + // and no reaper, so prune it in the same tx or a deleted chat session leaves + // permanent orphan card rows (#4810 follow-up). + if err := qtx.DeleteChannelOutboundCardMessagesBySession(r.Context(), session.ID); err != nil { + writeError(w, http.StatusInternalServerError, "failed to delete chat session outbound cards") + return + } if err := qtx.DeleteChatSession(r.Context(), db.DeleteChatSessionParams{ ID: session.ID, diff --git a/server/internal/handler/chat_test.go b/server/internal/handler/chat_test.go index 2d8688620e..2fc50e1e67 100644 --- a/server/internal/handler/chat_test.go +++ b/server/internal/handler/chat_test.go @@ -614,17 +614,20 @@ func TestListChatMessagesPage_RejectsInvalidLimit(t *testing.T) { } } -// TestDeleteChatSession_PrunesChannelChatSessionBinding verifies the -// application-layer replacement for the channel_chat_session_binding -// chat_session-FK cascade (MUL-3515 §4): deleting a chat session prunes its -// channel binding in the same tx that deletes the session row. -func TestDeleteChatSession_PrunesChannelChatSessionBinding(t *testing.T) { +// TestDeleteChatSession_PrunesChannelRows verifies the application-layer +// replacement for the channel_* chat_session-FK cascade (MUL-3515 §4): deleting a +// chat session prunes BOTH its channel_chat_session_binding and its +// channel_outbound_card_message rows in the same tx that deletes the session row. +// Both are keyed by chat_session_id with no FK and no reaper, so a miss leaves a +// permanent orphan (Elon's follow-up on #4810). +func TestDeleteChatSession_PrunesChannelRows(t *testing.T) { agentID := createHandlerTestAgent(t, "ChatDeleteBindingAgent", []byte("[]")) sessionID := createHandlerTestChatSession(t, agentID) ctx := context.Background() const appID = "cli_chat_delete_binding" const channelChatID = "oc_chat_delete_binding" + const cardMsgID = "om_chat_delete_card" // channel_* rows have no FK to chat_session/workspace (MUL-3515 §4), so // they outlive the helper's chat_session cleanup; clear by deterministic @@ -632,6 +635,8 @@ func TestDeleteChatSession_PrunesChannelChatSessionBinding(t *testing.T) { cleanChannel := func() { _, _ = testPool.Exec(context.Background(), `DELETE FROM channel_chat_session_binding WHERE channel_chat_id = $1`, channelChatID) + _, _ = testPool.Exec(context.Background(), + `DELETE FROM channel_outbound_card_message WHERE channel_card_message_id = $1`, cardMsgID) _, _ = testPool.Exec(context.Background(), `DELETE FROM channel_installation WHERE channel_type = 'feishu' AND config->>'app_id' = $1`, appID) } @@ -654,6 +659,13 @@ VALUES ($1, $2, 'feishu', $3, 'p2p') t.Fatalf("insert channel_chat_session_binding: %v", err) } + if _, err := testPool.Exec(ctx, ` +INSERT INTO channel_outbound_card_message (chat_session_id, channel_type, channel_chat_id, channel_card_message_id, status) +VALUES ($1, 'feishu', $2, $3, 'final') +`, sessionID, channelChatID, cardMsgID); err != nil { + t.Fatalf("insert channel_outbound_card_message: %v", err) + } + req := httptest.NewRequest(http.MethodDelete, "/api/chat/sessions/"+sessionID, nil) req.Header.Set("X-User-ID", testUserID) req = withURLParam(req, "sessionId", sessionID) @@ -673,4 +685,13 @@ VALUES ($1, $2, 'feishu', $3, 'p2p') if bindingExists { t.Fatal("deleted chat session's channel_chat_session_binding was not pruned") } + + var cardExists bool + if err := testPool.QueryRow(ctx, + `SELECT EXISTS (SELECT 1 FROM channel_outbound_card_message WHERE channel_card_message_id = $1)`, cardMsgID).Scan(&cardExists); err != nil { + t.Fatalf("query outbound card message: %v", err) + } + if cardExists { + t.Fatal("deleted chat session's channel_outbound_card_message was not pruned (no reaper would ever collect it)") + } } diff --git a/server/pkg/db/generated/channel.sql.go b/server/pkg/db/generated/channel.sql.go index 6e82625b4e..e286881ba9 100644 --- a/server/pkg/db/generated/channel.sql.go +++ b/server/pkg/db/generated/channel.sql.go @@ -447,6 +447,23 @@ func (q *Queries) DeleteChannelInstallationsByArchivedRuntimeAgents(ctx context. return err } +const deleteChannelOutboundCardMessagesBySession = `-- name: DeleteChannelOutboundCardMessagesBySession :exec +DELETE FROM channel_outbound_card_message +WHERE chat_session_id = $1 +` + +// Application-layer integrity (channel_* has no FK/cascade, MUL-3515 §4): drop the +// outbound card-message rows for a chat_session being deleted. They are keyed by +// chat_session_id with no FK and no reaper, so the standalone chat-session delete +// path must prune them here alongside DeleteChannelChatSessionBindingBySession — +// otherwise deleting a chat session leaves them as permanent orphans (Elon's +// follow-up on #4810; the workspace/agent/reclaim sweeps already cover their +// paths). A card that survived its session could only mis-route a later patch. +func (q *Queries) DeleteChannelOutboundCardMessagesBySession(ctx context.Context, chatSessionID pgtype.UUID) error { + _, err := q.db.Exec(ctx, deleteChannelOutboundCardMessagesBySession, chatSessionID) + return err +} + const deleteChannelUserBindingsByInstallation = `-- name: DeleteChannelUserBindingsByInstallation :exec DELETE FROM channel_user_binding WHERE installation_id = $1 diff --git a/server/pkg/db/queries/channel.sql b/server/pkg/db/queries/channel.sql index 8e766a8afb..39aed4f9bf 100644 --- a/server/pkg/db/queries/channel.sql +++ b/server/pkg/db/queries/channel.sql @@ -579,6 +579,17 @@ SET status = $2, last_patched_at = now() WHERE id = $1; +-- name: DeleteChannelOutboundCardMessagesBySession :exec +-- Application-layer integrity (channel_* has no FK/cascade, MUL-3515 §4): drop the +-- outbound card-message rows for a chat_session being deleted. They are keyed by +-- chat_session_id with no FK and no reaper, so the standalone chat-session delete +-- path must prune them here alongside DeleteChannelChatSessionBindingBySession — +-- otherwise deleting a chat session leaves them as permanent orphans (Elon's +-- follow-up on #4810; the workspace/agent/reclaim sweeps already cover their +-- paths). A card that survived its session could only mis-route a later patch. +DELETE FROM channel_outbound_card_message +WHERE chat_session_id = $1; + -- ===================== -- channel_binding_token -- =====================