mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-03 03:03:56 +02:00
fix(chat): prune orphaned outbound card messages on chat-session delete (#4810)
The standalone chat-session delete path pruned channel_chat_session_binding but not channel_outbound_card_message. Both are keyed by chat_session_id with no FK (MUL-3515 §4) and no reaper, so deleting a chat session left the card rows as permanent orphans — the same no-FK-orphan class as the #4810 installation fix, which already covers the workspace-delete / runtime-teardown / reclaim paths. Add DeleteChannelOutboundCardMessagesBySession and call it in the same tx as the binding prune; extend the delete-chat-session test to assert both are swept. Follow-up nit from the #5103 review (Elon). MUL-3937 Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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)")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
-- =====================
|
||||
|
||||
Reference in New Issue
Block a user