diff --git a/internal/agent/loop_history.go b/internal/agent/loop_history.go index 91f20e48..8d432447 100644 --- a/internal/agent/loop_history.go +++ b/internal/agent/loop_history.go @@ -162,8 +162,13 @@ func (l *Loop) buildMessages(ctx context.Context, history []providers.Message, s // tools. Otherwise lookupMCPDescFromUserTools surfaces descriptions from // any user's cache → LLM sees tools it can't actually call (executeToolForActor // scoped to actorUserID returns "tool not found"). Compute actor via - // resolveActorUserID — same key the agent loop uses to fetch per-user MCP creds. - actorUserID := resolveActorUserID(userID, store.SenderIDFromContext(ctx), peerKind, channelType) + // CredentialUserID (merged tenant_user identity) to match the cache key + // used by getUserMCPTools. Fall back to resolveActorUserID for channels + // without merge resolution. + actorUserID := store.CredentialUserIDFromContext(ctx) + if actorUserID == "" { + actorUserID = resolveActorUserID(userID, store.SenderIDFromContext(ctx), peerKind, channelType) + } mcpToolDescs := l.buildMCPToolDescs(toolNames, actorUserID) // Bootstrap DM mode: only restrict tools for open agents (identity being created). diff --git a/internal/agent/loop_mcp_user_test.go b/internal/agent/loop_mcp_user_test.go index 2e03da79..081a766e 100644 --- a/internal/agent/loop_mcp_user_test.go +++ b/internal/agent/loop_mcp_user_test.go @@ -475,3 +475,127 @@ func TestGetUserMCPToolsNilPoolEarlyReturn(t *testing.T) { type errTokenExpiredForTest string func (e errTokenExpiredForTest) Error() string { return string(e) } + +// TestGetUserMCPToolsGroupCredentialResolution verifies that when a Telegram user +// posts in a group, the group's credentials (CredentialUserID) are used for MCP +// tool lookup instead of the individual sender's credentials. +// +// This is the fix for the calendar event creation bug: the user's calendar email +// was known but the MCP tools were being looked up with the wrong user ID, +// causing the tools to appear blocked by system policy. +func TestGetUserMCPToolsGroupCredentialResolution(t *testing.T) { + // Simulate a group chat where the group has its own MCP credentials + // (e.g., a Google Calendar MCP server configured for the group). + groupID := "group:telegram:-100456" + serverID := uuid.New() + + // Track which user ID is used for credential lookup + var lookupUserID string + st := &trackingMCPServerStore{ + userCreds: &store.MCPUserCredentials{ + APIKey: "group-calendar-api-key", + Headers: map[string]string{"Authorization": "Bearer group-calendar-api-key"}, + }, + onGetUserCredentials: func(_ context.Context, _ uuid.UUID, userID string) { + lookupUserID = userID + }, + } + + pool := newTestPool(t) + l := &Loop{ + tenantID: uuid.New(), + mcpPool: pool, + mcpStore: st, + mcpUserCredSrvs: []store.MCPAccessInfo{ + {Server: store.MCPServerData{ + BaseModel: store.BaseModel{ID: serverID}, + Name: "google-calendar", + Transport: "http", + URL: "http://127.0.0.1:1", + }}, + }, + } + + // When CredentialUserID is set in context (resolved group identity), + // it should be used for MCP credential lookup. + ctx := store.WithCredentialUserID(context.Background(), groupID) + _ = l.getUserMCPTools(ctx, groupID) + + if lookupUserID != groupID { + t.Errorf("expected MCP credentials looked up with group ID %q, got %q", groupID, lookupUserID) + } +} + +// trackingMCPServerStore is a test store that tracks GetUserCredentials calls. +type trackingMCPServerStore struct { + userCreds *store.MCPUserCredentials + onGetUserCredentials func(_ context.Context, _ uuid.UUID, userID string) +} + +func (m *trackingMCPServerStore) GetUserCredentials(_ context.Context, _ uuid.UUID, userID string) (*store.MCPUserCredentials, error) { + if m.onGetUserCredentials != nil { + m.onGetUserCredentials(context.Background(), uuid.Nil, userID) + } + return m.userCreds, nil +} + +func (m *trackingMCPServerStore) CreateServer(_ context.Context, _ *store.MCPServerData) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) GetServer(_ context.Context, _ uuid.UUID) (*store.MCPServerData, error) { + panic("not implemented") +} +func (m *trackingMCPServerStore) GetServerByName(_ context.Context, _ string) (*store.MCPServerData, error) { + panic("not implemented") +} +func (m *trackingMCPServerStore) ListServers(_ context.Context) ([]store.MCPServerData, error) { + panic("not implemented") +} +func (m *trackingMCPServerStore) UpdateServer(_ context.Context, _ uuid.UUID, _ map[string]any) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) DeleteServer(_ context.Context, _ uuid.UUID) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) GrantToAgent(_ context.Context, _ *store.MCPAgentGrant) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) RevokeFromAgent(_ context.Context, _, _ uuid.UUID) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) ListAgentGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { + panic("not implemented") +} +func (m *trackingMCPServerStore) ListServerGrants(_ context.Context, _ uuid.UUID) ([]store.MCPAgentGrant, error) { + panic("not implemented") +} +func (m *trackingMCPServerStore) GrantToUser(_ context.Context, _ *store.MCPUserGrant) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) RevokeFromUser(_ context.Context, _ uuid.UUID, _ string) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) CountAgentGrantsByServer(_ context.Context) (map[uuid.UUID]int, error) { + panic("not implemented") +} +func (m *trackingMCPServerStore) ListAccessible(_ context.Context, _ uuid.UUID, _ string) ([]store.MCPAccessInfo, error) { + panic("not implemented") +} +func (m *trackingMCPServerStore) CreateRequest(_ context.Context, _ *store.MCPAccessRequest) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) ListPendingRequests(_ context.Context) ([]store.MCPAccessRequest, error) { + panic("not implemented") +} +func (m *trackingMCPServerStore) ReviewRequest(_ context.Context, _ uuid.UUID, _ bool, _, _ string) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) SetUserCredentials(_ context.Context, _ uuid.UUID, _ string, _ store.MCPUserCredentials) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) DeleteUserCredentials(_ context.Context, _ uuid.UUID, _ string) error { + panic("not implemented") +} +func (m *trackingMCPServerStore) CacheToolDescriptions(_ context.Context, _ uuid.UUID, _ map[string]store.CachedToolInfo) error { + return nil +} diff --git a/internal/agent/loop_pipeline_callbacks.go b/internal/agent/loop_pipeline_callbacks.go index dd117613..c20af8f7 100644 --- a/internal/agent/loop_pipeline_callbacks.go +++ b/internal/agent/loop_pipeline_callbacks.go @@ -231,17 +231,22 @@ func (l *Loop) makeBuildFilteredTools(req *RunRequest) func(state *pipeline.RunS // Servers with require_user_credentials are deferred at startup and // connected per-request here with the actual user's credentials. // - // Use resolveActorUserID — the gateway consumer rewrites UserID in - // two scenarios (group chats AND DM with merged contact), both of - // which break per-user MCP credential lookup. ChannelType discriminates - // Bitrix24 (always prefer SenderID) from other channels (group-only - // rewrite recovery). See resolveActorUserID docstring for full rationale. - actorUserID := resolveActorUserID( - state.Input.UserID, - state.Input.SenderID, - state.Input.PeerKind, - state.Input.ChannelType, - ) + // Prefer CredentialUserID from context — resolveCredentialUserID already + // resolved the merged tenant_user identity (e.g. Telegram group merged + // to a tenant_user via UI). Using the raw SenderID or group composite + // as the cache key would miss the credential row keyed by the merged + // tenant_user UUID. Fall back to resolveActorUserID for channels without + // merge resolution (backward compat). See resolveActorUserID docstring + // for the group-rewrite recovery rationale. + actorUserID := store.CredentialUserIDFromContext(state.Ctx) + if actorUserID == "" { + actorUserID = resolveActorUserID( + state.Input.UserID, + state.Input.SenderID, + state.Input.PeerKind, + state.Input.ChannelType, + ) + } userTools := l.getUserMCPTools(state.Ctx, actorUserID) slog.Debug("mcp.user_tools_context", "peer_kind", state.Input.PeerKind, diff --git a/internal/agent/loop_pipeline_tool_callbacks.go b/internal/agent/loop_pipeline_tool_callbacks.go index d7829b9d..60280c11 100644 --- a/internal/agent/loop_pipeline_tool_callbacks.go +++ b/internal/agent/loop_pipeline_tool_callbacks.go @@ -75,7 +75,12 @@ func (l *Loop) makeExecuteToolCall(req *RunRequest, bridgeRS *runState) func(ctx // C2 fix: route through executeToolForActor so per-user MCP tools // resolve to the calling user's BridgeTool (not the first user's // BridgeTool leaked via shared registry). - actorUserID := resolveActorUserID(req.UserID, req.SenderID, req.PeerKind, req.ChannelType) + // Prefer CredentialUserID (merged tenant_user identity) over raw + // resolveActorUserID so the cache key matches what getUserMCPTools used. + actorUserID := store.CredentialUserIDFromContext(ctx) + if actorUserID == "" { + actorUserID = resolveActorUserID(req.UserID, req.SenderID, req.PeerKind, req.ChannelType) + } result := l.executeToolForActor(ctx, registryName, tc.Arguments, req.Channel, req.ChatID, req.PeerKind, req.SessionKey, actorUserID) toolDuration := time.Since(toolStart) @@ -146,7 +151,12 @@ func (l *Loop) makeExecuteToolRaw(req *RunRequest) func(ctx context.Context, tc // C2 fix (parallel path): route through executeToolForActor for per-user // MCP tool isolation. Same rationale as makeExecuteToolCall above. - actorUserID := resolveActorUserID(req.UserID, req.SenderID, req.PeerKind, req.ChannelType) + // Prefer CredentialUserID (merged tenant_user identity) over raw + // resolveActorUserID so the cache key matches what getUserMCPTools used. + actorUserID := store.CredentialUserIDFromContext(ctx) + if actorUserID == "" { + actorUserID = resolveActorUserID(req.UserID, req.SenderID, req.PeerKind, req.ChannelType) + } result := l.executeToolForActor(ctx, registryName, tc.Arguments, req.Channel, req.ChatID, req.PeerKind, req.SessionKey, actorUserID) dur := time.Since(start) diff --git a/internal/agent/user_identity_resolver.go b/internal/agent/user_identity_resolver.go index b12fdcd7..6f5784b8 100644 --- a/internal/agent/user_identity_resolver.go +++ b/internal/agent/user_identity_resolver.go @@ -60,7 +60,17 @@ func (l *Loop) resolveCredentialUserID(ctx context.Context, req RunRequest) stri return req.UserID } - // Group: try individual sender first (most specific) + // Group: try group contact first (group has its own credentials), + // fall back to individual sender if no group credential exists. + if chatID := extractGroupChatID(req.UserID); chatID != "" && channelType != "" { + resolved, err := l.userResolver.ResolveTenantUserID(ctx, channelType, chatID) + if err != nil { + slog.Debug("credential_resolve.group_contact_failed", "chatID", chatID, "channel", channelType, "error", err) + } else if resolved != "" { + return resolved + } + } + if req.SenderID != "" && channelType != "" { senderNumeric := req.SenderID if idx := strings.IndexByte(senderNumeric, '|'); idx > 0 { @@ -74,16 +84,6 @@ func (l *Loop) resolveCredentialUserID(ctx context.Context, req RunRequest) stri } } - // Group: try group contact resolution (group chatID merged to tenant user) - if chatID := extractGroupChatID(req.UserID); chatID != "" && channelType != "" { - resolved, err := l.userResolver.ResolveTenantUserID(ctx, channelType, chatID) - if err != nil { - slog.Debug("credential_resolve.group_contact_failed", "chatID", chatID, "channel", channelType, "error", err) - } else if resolved != "" { - return resolved - } - } - return req.UserID } diff --git a/internal/agent/user_identity_resolver_test.go b/internal/agent/user_identity_resolver_test.go index c28108c6..7e2904d7 100644 --- a/internal/agent/user_identity_resolver_test.go +++ b/internal/agent/user_identity_resolver_test.go @@ -62,13 +62,13 @@ func TestResolveCredentialUserID(t *testing.T) { wantUserID: "team-acme@co.com", }, { - name: "Group: both sender and group merged — sender takes priority", + name: "Group: both sender and group merged — group takes priority", req: RunRequest{UserID: "group:tg:-100456", SenderID: "12345", ChannelType: "telegram", PeerKind: "group"}, mergedMap: map[string]string{ "telegram:12345": "john@co.com", "telegram:-100456": "team-acme@co.com", }, - wantUserID: "john@co.com", // individual > group + wantUserID: "team-acme@co.com", // group > individual }, { name: "Group: nobody merged — returns group composite",