mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-27 21:33:41 +02:00
fix(channel): skip orphaned installations in hub-boot active scan
Preflight deploy review: channel_installation dropped the workspace/agent FK (MUL-3515 §4), so unlike lark_installation it does not cascade away when its workspace is deleted or its agent is hard-deleted (e.g. runtime teardown). The hub-boot query then keeps opening a WebSocket for a bot whose owner is gone. JOIN ListActiveChannelInstallations to live workspace + agent so an orphaned installation is never connected, uniformly for every deletion path. The JOIN matches the old ON DELETE CASCADE semantics (row existence, not agent archival), so an archived-but-present agent's installation is still listed; the orphaned row's encrypted secret is thereby never decrypted/used. Tests: a real-DB handler test asserts a deleted-workspace/agent installation and a non-Feishu one are both excluded; the lark scope test's active-list assertion moved there since the JOIN now needs real workspace/agent fixtures. (Physically deleting dormant orphaned channel rows on workspace/agent deletion is a separate app-layer-cleanup follow-up.) MUL-3515 Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
@@ -160,3 +161,60 @@ func TestListLarkInstallations_NotConfigured_HardCodedInstallSupportedFalse(t *t
|
||||
t.Fatalf("install_supported must be false in the early-return branch even with a non-nil APIClient")
|
||||
}
|
||||
}
|
||||
|
||||
// TestListActiveLarkInstallations_SkipsOrphans pins the MUL-3515 hub-boot
|
||||
// guard: ListActiveChannelInstallations is JOINed to live workspace + agent,
|
||||
// so an active channel_installation whose workspace or agent has been deleted
|
||||
// (channel_* has no FK cascade) is never returned — otherwise the Hub would
|
||||
// keep opening a WebSocket for a bot whose owner is gone. It also stays
|
||||
// channel_type='feishu'-scoped. Runs against the real test DB.
|
||||
func TestListActiveLarkInstallations_SkipsOrphans(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
agentID := createHandlerTestAgent(t, "LarkActiveScopeAgent", []byte("[]"))
|
||||
|
||||
const (
|
||||
liveApp = "cli_active_live"
|
||||
orphanApp = "cli_active_orphan"
|
||||
slackApp = "cli_active_slack"
|
||||
// Deliberately non-existent workspace/agent so the JOIN drops the row.
|
||||
orphanWS = "5d0a0000-0000-4000-8000-000000000001"
|
||||
orphanAg = "5d0a0000-0000-4000-8000-000000000002"
|
||||
)
|
||||
clean := func() {
|
||||
_, _ = testPool.Exec(context.Background(),
|
||||
`DELETE FROM channel_installation WHERE config->>'app_id' = ANY($1)`,
|
||||
[]string{liveApp, orphanApp, slackApp})
|
||||
}
|
||||
clean()
|
||||
t.Cleanup(clean)
|
||||
|
||||
seed := func(ws, ag, channelType, app string) {
|
||||
if _, err := testPool.Exec(ctx, `
|
||||
INSERT INTO channel_installation (workspace_id, agent_id, channel_type, config, installer_user_id, status)
|
||||
VALUES ($1, $2, $3, jsonb_build_object('app_id', $4::text), $5, 'active')
|
||||
`, ws, ag, channelType, app, testUserID); err != nil {
|
||||
t.Fatalf("seed %s installation: %v", app, err)
|
||||
}
|
||||
}
|
||||
seed(testWorkspaceID, agentID, "feishu", liveApp) // live workspace + agent -> listed
|
||||
seed(orphanWS, orphanAg, "feishu", orphanApp) // deleted workspace + agent -> dropped
|
||||
seed(testWorkspaceID, agentID, "slack", slackApp) // wrong channel_type -> dropped
|
||||
|
||||
active, err := lark.NewChannelStore(testHandler.Queries).ListActiveLarkInstallations(ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("ListActiveLarkInstallations: %v", err)
|
||||
}
|
||||
seen := map[string]bool{}
|
||||
for _, inst := range active {
|
||||
seen[inst.AppID] = true
|
||||
}
|
||||
if !seen[liveApp] {
|
||||
t.Fatal("expected the live-workspace/agent Feishu installation to be listed")
|
||||
}
|
||||
if seen[orphanApp] {
|
||||
t.Fatal("orphaned installation (deleted workspace/agent) must not be listed — the hub would connect a dead bot")
|
||||
}
|
||||
if seen[slackApp] {
|
||||
t.Fatal("non-Feishu installation must not be listed by the Feishu hub")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -139,26 +139,9 @@ VALUES ($1, $2, 'slack', 'oc_scope_slack', 'om_scope_slack', 'pending')
|
||||
t.Fatalf("ListLarkInstallationsByWorkspace: got apps=%v, want exactly [%s]", apps, feishuApp)
|
||||
}
|
||||
|
||||
// list-active (hub boot): includes the Feishu install, never the Slack one
|
||||
active, err := store.ListActiveLarkInstallations(ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("ListActiveLarkInstallations: %v", err)
|
||||
}
|
||||
var sawFeishu, sawSlack bool
|
||||
for _, r := range active {
|
||||
switch r.AppID {
|
||||
case feishuApp:
|
||||
sawFeishu = true
|
||||
case slackApp:
|
||||
sawSlack = true
|
||||
}
|
||||
}
|
||||
if !sawFeishu {
|
||||
t.Fatal("ListActiveLarkInstallations: missing the active Feishu installation")
|
||||
}
|
||||
if sawSlack {
|
||||
t.Fatal("ListActiveLarkInstallations: returned a Slack installation (hub would supervise another channel)")
|
||||
}
|
||||
// (ListActiveLarkInstallations channel-type + live workspace/agent scoping
|
||||
// is covered by TestListActiveLarkInstallations_SkipsOrphans in the handler
|
||||
// package, which has real workspace/agent fixtures the JOIN now requires.)
|
||||
|
||||
// --- outbound reads: a Slack binding/card must not be seen as Feishu ---
|
||||
|
||||
|
||||
@@ -601,15 +601,26 @@ func (q *Queries) GetChannelUserBindingByUserID(ctx context.Context, arg GetChan
|
||||
}
|
||||
|
||||
const listActiveChannelInstallations = `-- name: ListActiveChannelInstallations :many
|
||||
SELECT id, workspace_id, agent_id, channel_type, config, status, ws_lease_token, ws_lease_expires_at, installer_user_id, installed_at, created_at, updated_at FROM channel_installation
|
||||
WHERE status = 'active'
|
||||
AND channel_type = $1
|
||||
ORDER BY created_at ASC
|
||||
SELECT ci.id, ci.workspace_id, ci.agent_id, ci.channel_type, ci.config, ci.status, ci.ws_lease_token, ci.ws_lease_expires_at, ci.installer_user_id, ci.installed_at, ci.created_at, ci.updated_at FROM channel_installation ci
|
||||
JOIN workspace w ON w.id = ci.workspace_id
|
||||
JOIN agent a ON a.id = ci.agent_id
|
||||
WHERE ci.status = 'active'
|
||||
AND ci.channel_type = $1
|
||||
ORDER BY ci.created_at ASC
|
||||
`
|
||||
|
||||
// Boot path for a per-channel-type inbound hub: every active installation of
|
||||
// the given channel_type, so a hub claims leases and opens connections only
|
||||
// for its own platform and never supervises another channel's installation.
|
||||
//
|
||||
// The JOINs require the owning workspace and agent rows to still exist.
|
||||
// channel_installation has no FK (MUL-3515 §4), so unlike the old
|
||||
// lark_installation (which cascaded away on workspace/agent deletion) an
|
||||
// installation can be orphaned when its workspace is deleted or its agent is
|
||||
// hard-deleted (e.g. runtime teardown). Without this guard the hub would keep
|
||||
// opening a WebSocket for a bot whose workspace/agent is gone. The JOIN matches
|
||||
// the old ON DELETE CASCADE semantics: it filters on row existence, not agent
|
||||
// archival, so an archived-but-present agent's installation is still listed.
|
||||
func (q *Queries) ListActiveChannelInstallations(ctx context.Context, channelType string) ([]ChannelInstallation, error) {
|
||||
rows, err := q.db.Query(ctx, listActiveChannelInstallations, channelType)
|
||||
if err != nil {
|
||||
|
||||
@@ -75,10 +75,21 @@ ORDER BY created_at ASC;
|
||||
-- Boot path for a per-channel-type inbound hub: every active installation of
|
||||
-- the given channel_type, so a hub claims leases and opens connections only
|
||||
-- for its own platform and never supervises another channel's installation.
|
||||
SELECT * FROM channel_installation
|
||||
WHERE status = 'active'
|
||||
AND channel_type = sqlc.arg('channel_type')
|
||||
ORDER BY created_at ASC;
|
||||
--
|
||||
-- The JOINs require the owning workspace and agent rows to still exist.
|
||||
-- channel_installation has no FK (MUL-3515 §4), so unlike the old
|
||||
-- lark_installation (which cascaded away on workspace/agent deletion) an
|
||||
-- installation can be orphaned when its workspace is deleted or its agent is
|
||||
-- hard-deleted (e.g. runtime teardown). Without this guard the hub would keep
|
||||
-- opening a WebSocket for a bot whose workspace/agent is gone. The JOIN matches
|
||||
-- the old ON DELETE CASCADE semantics: it filters on row existence, not agent
|
||||
-- archival, so an archived-but-present agent's installation is still listed.
|
||||
SELECT ci.* FROM channel_installation ci
|
||||
JOIN workspace w ON w.id = ci.workspace_id
|
||||
JOIN agent a ON a.id = ci.agent_id
|
||||
WHERE ci.status = 'active'
|
||||
AND ci.channel_type = sqlc.arg('channel_type')
|
||||
ORDER BY ci.created_at ASC;
|
||||
|
||||
-- name: SetChannelInstallationStatus :exec
|
||||
UPDATE channel_installation
|
||||
|
||||
Reference in New Issue
Block a user