mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-25 03:55:27 +02:00
feat(channel): prune channel bindings on member removal + chat session delete
MUL-3515 §4 dropped every channel_* foreign key, so the old ON DELETE CASCADE that cleared a user's channel_user_binding when they left a workspace, and a chat's channel_chat_session_binding when its chat_session was deleted, no longer fires. Re-establish that integrity in the application layer, inside the existing transactions: revokeAndRemoveMember -> DeleteChannelUserBindingsByWorkspaceMember, DeleteChatSession -> DeleteChannelChatSessionBindingBySession. Adds real-DB tests for both paths, including a scoping check that a remaining member's binding survives the prune. Verified on PG17: both new tests plus the existing revocation tests and the full handler package pass. MUL-3515 Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -348,6 +348,14 @@ func (h *Handler) DeleteChatSession(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// channel_chat_session_binding used to carry a chat_session FK with
|
||||
// ON DELETE CASCADE; MUL-3515 §4 dropped every channel_* foreign key, so
|
||||
// prune the binding here in the same tx that deletes its chat_session.
|
||||
if err := qtx.DeleteChannelChatSessionBindingBySession(r.Context(), session.ID); err != nil {
|
||||
writeError(w, http.StatusInternalServerError, "failed to delete chat session binding")
|
||||
return
|
||||
}
|
||||
|
||||
if err := qtx.DeleteChatSession(r.Context(), db.DeleteChatSessionParams{
|
||||
ID: session.ID,
|
||||
WorkspaceID: session.WorkspaceID,
|
||||
|
||||
@@ -444,3 +444,64 @@ func TestListChatMessagesPage_RejectsInvalidLimit(t *testing.T) {
|
||||
t.Fatalf("ListChatMessagesPage invalid limit: expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// 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) {
|
||||
agentID := createHandlerTestAgent(t, "ChatDeleteBindingAgent", []byte("[]"))
|
||||
sessionID := createHandlerTestChatSession(t, agentID)
|
||||
ctx := context.Background()
|
||||
|
||||
const appID = "cli_chat_delete_binding"
|
||||
const channelChatID = "oc_chat_delete_binding"
|
||||
|
||||
// channel_* rows have no FK to chat_session/workspace (MUL-3515 §4), so
|
||||
// they outlive the helper's chat_session cleanup; clear by deterministic
|
||||
// key before and after.
|
||||
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_installation WHERE channel_type = 'feishu' AND config->>'app_id' = $1`, appID)
|
||||
}
|
||||
cleanChannel()
|
||||
t.Cleanup(cleanChannel)
|
||||
|
||||
var installID string
|
||||
if err := testPool.QueryRow(ctx, `
|
||||
INSERT INTO channel_installation (workspace_id, agent_id, channel_type, config, installer_user_id)
|
||||
VALUES ($1, $2, 'feishu', jsonb_build_object('app_id', $3::text), $4)
|
||||
RETURNING id
|
||||
`, testWorkspaceID, agentID, appID, testUserID).Scan(&installID); err != nil {
|
||||
t.Fatalf("insert channel_installation: %v", err)
|
||||
}
|
||||
|
||||
if _, err := testPool.Exec(ctx, `
|
||||
INSERT INTO channel_chat_session_binding (chat_session_id, installation_id, channel_type, channel_chat_id, chat_type)
|
||||
VALUES ($1, $2, 'feishu', $3, 'p2p')
|
||||
`, sessionID, installID, channelChatID); err != nil {
|
||||
t.Fatalf("insert channel_chat_session_binding: %v", err)
|
||||
}
|
||||
|
||||
req := httptest.NewRequest(http.MethodDelete, "/api/chat/sessions/"+sessionID, nil)
|
||||
req.Header.Set("X-User-ID", testUserID)
|
||||
req = withURLParam(req, "sessionId", sessionID)
|
||||
req = withChatTestWorkspaceCtx(t, req)
|
||||
w := httptest.NewRecorder()
|
||||
testHandler.DeleteChatSession(w, req)
|
||||
|
||||
if w.Code != http.StatusNoContent {
|
||||
t.Fatalf("DeleteChatSession: expected 204, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
var bindingExists bool
|
||||
if err := testPool.QueryRow(ctx,
|
||||
`SELECT EXISTS (SELECT 1 FROM channel_chat_session_binding WHERE channel_chat_id = $1)`, channelChatID).Scan(&bindingExists); err != nil {
|
||||
t.Fatalf("query chat session binding: %v", err)
|
||||
}
|
||||
if bindingExists {
|
||||
t.Fatal("deleted chat session's channel_chat_session_binding was not pruned")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -112,6 +112,19 @@ func (h *Handler) revokeAndRemoveMember(ctx context.Context, workspaceID, userID
|
||||
}
|
||||
}
|
||||
|
||||
// channel_user_binding used to carry a member FK with ON DELETE CASCADE, so
|
||||
// a removed member's IM bindings vanished automatically. MUL-3515 §4 dropped
|
||||
// every channel_* foreign key, moving that integrity rule to the application
|
||||
// layer: prune the bindings here, in the same tx as the member-row delete.
|
||||
// The inbound path also re-checks membership (see ChannelStore.IsWorkspaceMember),
|
||||
// but pruning stops a stale binding from lingering across a remove/re-add.
|
||||
if err := qtx.DeleteChannelUserBindingsByWorkspaceMember(ctx, db.DeleteChannelUserBindingsByWorkspaceMemberParams{
|
||||
WorkspaceID: workspaceID,
|
||||
MulticaUserID: userID,
|
||||
}); err != nil {
|
||||
return empty, err
|
||||
}
|
||||
|
||||
// Member row deletion lives inside the same tx so a successful revoke is
|
||||
// never followed by a failed member-delete (which would leave the user
|
||||
// still a member with a dead runtime), and a failed revoke never leaves
|
||||
|
||||
@@ -596,6 +596,86 @@ func TestDeleteMember_RevokesTargetRuntimes(t *testing.T) {
|
||||
assertRevoked(t, fx)
|
||||
}
|
||||
|
||||
// TestDeleteMember_PrunesChannelUserBindings verifies the application-layer
|
||||
// replacement for the channel_user_binding member-FK cascade (MUL-3515 §4):
|
||||
// removing a member prunes that member's channel bindings, in the same tx as
|
||||
// the member-row delete, while leaving a remaining member's binding intact.
|
||||
func TestDeleteMember_PrunesChannelUserBindings(t *testing.T) {
|
||||
fx := setupRevocationFixture(t, "handler-tests-revoke-binding", "daemon-revoke-binding")
|
||||
ctx := context.Background()
|
||||
|
||||
const appID = "cli_revoke_binding"
|
||||
const removedOpenID = "ou_revoke_binding_removed"
|
||||
const keepOpenID = "ou_revoke_binding_keep"
|
||||
|
||||
// channel_* rows have no FK to workspace (MUL-3515 §4), so the fixture's
|
||||
// workspace-delete cleanup never reaches them; clear by deterministic key
|
||||
// both before (in case a prior run was killed mid-test) and after.
|
||||
cleanChannel := func() {
|
||||
_, _ = testPool.Exec(context.Background(),
|
||||
`DELETE FROM channel_user_binding WHERE channel_user_id = ANY($1)`,
|
||||
[]string{removedOpenID, keepOpenID})
|
||||
_, _ = testPool.Exec(context.Background(),
|
||||
`DELETE FROM channel_installation WHERE channel_type = 'feishu' AND config->>'app_id' = $1`, appID)
|
||||
}
|
||||
cleanChannel()
|
||||
t.Cleanup(cleanChannel)
|
||||
|
||||
var installID string
|
||||
if err := testPool.QueryRow(ctx, `
|
||||
INSERT INTO channel_installation (workspace_id, agent_id, channel_type, config, installer_user_id)
|
||||
VALUES ($1, $2, 'feishu', jsonb_build_object('app_id', $3::text), $4)
|
||||
RETURNING id
|
||||
`, fx.WorkspaceID, fx.AgentID, appID, testUserID).Scan(&installID); err != nil {
|
||||
t.Fatalf("insert channel_installation: %v", err)
|
||||
}
|
||||
|
||||
// Binding for the member being removed — must be pruned.
|
||||
if _, err := testPool.Exec(ctx, `
|
||||
INSERT INTO channel_user_binding (workspace_id, multica_user_id, installation_id, channel_type, channel_user_id)
|
||||
VALUES ($1, $2, $3, 'feishu', $4)
|
||||
`, fx.WorkspaceID, fx.TargetUserID, installID, removedOpenID); err != nil {
|
||||
t.Fatalf("insert removed-member binding: %v", err)
|
||||
}
|
||||
|
||||
// Binding for the requester (an owner who stays) — must survive, proving
|
||||
// the prune is scoped to the removed user, not the whole workspace.
|
||||
if _, err := testPool.Exec(ctx, `
|
||||
INSERT INTO channel_user_binding (workspace_id, multica_user_id, installation_id, channel_type, channel_user_id)
|
||||
VALUES ($1, $2, $3, 'feishu', $4)
|
||||
`, fx.WorkspaceID, testUserID, installID, keepOpenID); err != nil {
|
||||
t.Fatalf("insert remaining-member binding: %v", err)
|
||||
}
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := newRequest("DELETE", "/api/workspaces/"+fx.WorkspaceID+"/members/"+fx.MemberID, nil)
|
||||
req.Header.Set("X-Workspace-ID", fx.WorkspaceID)
|
||||
req = withURLParams(req, "id", fx.WorkspaceID, "memberId", fx.MemberID)
|
||||
testHandler.DeleteMember(w, req)
|
||||
|
||||
if w.Code != http.StatusNoContent {
|
||||
t.Fatalf("DeleteMember: expected 204, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
var removedExists bool
|
||||
if err := testPool.QueryRow(ctx,
|
||||
`SELECT EXISTS (SELECT 1 FROM channel_user_binding WHERE channel_user_id = $1)`, removedOpenID).Scan(&removedExists); err != nil {
|
||||
t.Fatalf("query removed-member binding: %v", err)
|
||||
}
|
||||
if removedExists {
|
||||
t.Fatal("removed member's channel_user_binding was not pruned")
|
||||
}
|
||||
|
||||
var keepExists bool
|
||||
if err := testPool.QueryRow(ctx,
|
||||
`SELECT EXISTS (SELECT 1 FROM channel_user_binding WHERE channel_user_id = $1)`, keepOpenID).Scan(&keepExists); err != nil {
|
||||
t.Fatalf("query remaining-member binding: %v", err)
|
||||
}
|
||||
if !keepExists {
|
||||
t.Fatal("remaining member's channel_user_binding was wrongly pruned")
|
||||
}
|
||||
}
|
||||
|
||||
// TestLeaveWorkspace_RevokesOwnRuntimes is the self-removal counterpart: when
|
||||
// a member leaves a workspace voluntarily, their own runtimes are revoked
|
||||
// with the same atomic write set as DeleteMember.
|
||||
|
||||
Reference in New Issue
Block a user