diff --git a/cmd/gateway.go b/cmd/gateway.go index c41e90bc..e2a3b1cb 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -750,8 +750,9 @@ func runGateway() { // the one used by pg.NewPGStores → NewPGBitrixPortalStore. bitrixEncKey := os.Getenv("GOCLAW_ENCRYPTION_KEY") // Use the MCP-aware factory variant so channels that opt into - // lazy per-user credential provisioning (via mcp_server_name + - // mcp_base_url in their instance config) can reach the partner's + // lazy per-user credential provisioning (via mcp_server_id — or + // the legacy mcp_server_name + mcp_base_url pair — in their + // instance config) can reach the partner's // MCPServerStore. The MCP server authenticates each onboard call // via the caller-supplied Bitrix access_token (the "Bitrix24 // OAuth → existing mcp_user_credentials bridge" — Bitrix-specific diff --git a/internal/channels/bitrix24/factory.go b/internal/channels/bitrix24/factory.go index be7e2c2f..5e21e1f6 100644 --- a/internal/channels/bitrix24/factory.go +++ b/internal/channels/bitrix24/factory.go @@ -88,31 +88,43 @@ type bitrixInstanceConfig struct { // Optional MCP lazy-provisioning binding (Phase C). // - // When MCPServerName + MCPBaseURL are set AND the factory variant that - // accepts a MCPServerStore is used (FactoryWithPortalStoreAndMCP), the - // channel tries to mint per-user MCP credentials on first message: + // When MCPServerID (or the legacy MCPServerName+MCPBaseURL pair) is set + // AND the factory variant that accepts a MCPServerStore is used + // (FactoryWithPortalStoreAndMCP), the channel tries to mint per-user MCP + // credentials on first message: // // 1. Channel receives message from user U with OAuth tokens in event. // 2. Channel looks up MCPUserCredentials(serverID, senderID). Present - // → skip. Absent → POST /api/auto-onboard on MCPBaseURL forwarding - // U's OAuth tokens. MCP server authenticates the call via Bitrix - // `profile` against the supplied access_token — no shared admin - // secret required — and responds with a per-user api_key, which - // channel stores via SetUserCredentials (the "Bitrix24 OAuth → - // existing mcp_user_credentials bridge" — Bitrix-specific glue, - // not a generic MCP architecture pattern). + // → skip. Absent → POST /api/auto-onboard on the resolved base URL + // forwarding U's OAuth tokens. MCP server authenticates the call + // via Bitrix `profile` against the supplied access_token — no + // shared admin secret required — and responds with a per-user + // api_key, which channel stores via SetUserCredentials (the + // "Bitrix24 OAuth → existing mcp_user_credentials bridge" — + // Bitrix-specific glue, not a generic MCP architecture pattern). // 3. Agent pipeline downstream reads those creds naturally. // // Best-effort: if any step fails, channel logs a warning and forwards // the message anyway — agent loop will just see no creds and skip // that MCP server's tools. User gets a response, albeit without MCP. // - // Half-config fails at factory load: both fields set or both empty. - // // Skipped entirely for Open Channel bots (bot_type=O) — transient // customers don't map to tenant_users. - MCPServerName string `json:"mcp_server_name,omitempty"` // mcp_servers.name - MCPBaseURL string `json:"mcp_base_url,omitempty"` // HTTPS root + // + // MCPServerID is the preferred wiring since v3.15: admin picks an MCP + // server from the dashboard dropdown (filtered to + // require_user_credentials=true rows) and the channel resolves both + // name and URL from the mcp_servers row. + MCPServerID string `json:"mcp_server_id,omitempty"` // mcp_servers.id (UUID) + + // Deprecated: MCPServerName + MCPBaseURL are kept for backward-compat + // with configs written before the MCPServerID rollout. If MCPServerID + // is empty and both legacy fields are set the channel falls back to + // GetServerByName and uses MCPBaseURL as the base URL. New configs + // should only set MCPServerID; Phase 5 of the refactor migrates every + // existing row and drops these fields. + MCPServerName string `json:"mcp_server_name,omitempty"` // legacy — mcp_servers.name + MCPBaseURL string `json:"mcp_base_url,omitempty"` // legacy — HTTPS root } // Factory is the base channels.ChannelFactory signature. Bitrix24 requires a @@ -198,13 +210,17 @@ func FactoryWithPortalStoreAndMCP(portalStore store.BitrixPortalStore, mcpStore return nil, fmt.Errorf("bitrix24: invalid bot_type %q (must be \"B\" or \"O\")", ic.BotType) } - // MCP provisioning config is all-or-nothing. Catching half-config here - // prevents a silent "provisioning disabled but you meant to enable it" - // surprise — admin either sets both or neither. + // MCP provisioning config uses mcp_server_id (preferred) or the + // legacy mcp_server_name + mcp_base_url pair. Catching half-config + // here prevents a silent "provisioning disabled but you meant to + // enable it" surprise when someone only fills one of the legacy + // fields. The new mcp_server_id path is self-contained (server + // name + URL both derived from the mcp_servers row at Start()). + hasServerID := strings.TrimSpace(ic.MCPServerID) != "" hasServerName := strings.TrimSpace(ic.MCPServerName) != "" hasBaseURL := strings.TrimSpace(ic.MCPBaseURL) != "" - if hasServerName != hasBaseURL { - return nil, errors.New("bitrix24: mcp_server_name and mcp_base_url must both be set, or both empty") + if !hasServerID && hasServerName != hasBaseURL { + return nil, errors.New("bitrix24: mcp_server_name and mcp_base_url must both be set, or both empty (or use mcp_server_id)") } // Shared process-wide router. InitWebhookRouter uses sync.Once so the diff --git a/internal/channels/bitrix24/provisioner.go b/internal/channels/bitrix24/provisioner.go index 109592f6..5b156c91 100644 --- a/internal/channels/bitrix24/provisioner.go +++ b/internal/channels/bitrix24/provisioner.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log/slog" + "net/url" "strings" "sync" "time" @@ -94,41 +95,89 @@ func (c *Channel) initMCPProvisioner(ctx context.Context) error { "channel", c.Name()) return nil } - if strings.TrimSpace(c.cfg.MCPServerName) == "" || strings.TrimSpace(c.cfg.MCPBaseURL) == "" { - slog.Debug("bitrix24 mcp: provisioning disabled (mcp_server_name or mcp_base_url empty)", + + hasServerID := strings.TrimSpace(c.cfg.MCPServerID) != "" + hasLegacy := strings.TrimSpace(c.cfg.MCPServerName) != "" && strings.TrimSpace(c.cfg.MCPBaseURL) != "" + if !hasServerID && !hasLegacy { + slog.Debug("bitrix24 mcp: provisioning disabled (no mcp_server_id and legacy fields empty)", "channel", c.Name()) return nil } - // Resolve server name → UUID once at startup. If the server name is - // wrong or the row doesn't exist yet, log and disable provisioning — - // don't block channel startup. Admin can create the server + reload - // the channel later. + // Resolve the mcp_servers row. Preferred path: mcp_server_id (UUID + // dashboards write since v3.15). Fallback: legacy name lookup for + // configs written before the refactor and not yet migrated. // - // PGMCPServerStore.GetServerByName scopes the lookup by tenant_id from - // context (multi-tenant isolation). Channel.Start receives ctx from the - // instance loader without that scope set — wrap it explicitly with the - // channel's own tenant id so the lookup matches the row a tenant admin - // created via `bitrix-portal create` / dashboard. + // GetServer / GetServerByName both scope by tenant_id from context. + // Channel.Start receives ctx from the instance loader without that + // scope set — wrap it explicitly with the channel's own tenant id so + // the lookup matches the row a tenant admin created via the dashboard + // or `bitrix-portal create`. lookupCtx := ctx if tid := c.TenantID(); tid != uuid.Nil { lookupCtx = store.WithTenantID(ctx, tid) } - server, err := c.mcpStore.GetServerByName(lookupCtx, c.cfg.MCPServerName) + + var ( + server *store.MCPServerData + err error + ) + if hasServerID { + serverUUID, parseErr := uuid.Parse(strings.TrimSpace(c.cfg.MCPServerID)) + if parseErr != nil { + slog.Warn("bitrix24 mcp: provisioning disabled — invalid mcp_server_id", + "channel", c.Name(), "mcp_server_id", c.cfg.MCPServerID, "err", parseErr) + return nil + } + server, err = c.mcpStore.GetServer(lookupCtx, serverUUID) + } else { + server, err = c.mcpStore.GetServerByName(lookupCtx, c.cfg.MCPServerName) + } if err != nil || server == nil { slog.Warn("bitrix24 mcp: provisioning disabled — server not found", - "channel", c.Name(), "mcp_server_name", c.cfg.MCPServerName, "err", err) + "channel", c.Name(), + "mcp_server_id", c.cfg.MCPServerID, + "mcp_server_name", c.cfg.MCPServerName, + "err", err) + return nil + } + + // Base URL sourcing: for the id-based path we derive the ORIGIN + // (scheme://host[:port]) from the mcp_servers row. The row's URL is + // the MCP JSON-RPC endpoint (e.g. https://mcp.example.com/mcp), but + // the /api/auto-onboard REST call the provisioner will POST expects + // the origin only — appending "/api/auto-onboard" to the JSON-RPC + // path would 404. Legacy path stays on MCPBaseURL, which historically + // was already the origin (admin gave us "https://mcp.example.com"), + // so a half-migrated fleet keeps working until Phase 5 rewrites configs. + baseURL := strings.TrimSpace(c.cfg.MCPBaseURL) + if hasServerID { + derived, deriveErr := deriveAutoOnboardBaseURL(server.URL) + if deriveErr != nil { + slog.Warn("bitrix24 mcp: provisioning disabled — mcp_servers.url unparseable", + "channel", c.Name(), + "mcp_server", server.Name, + "url", server.URL, + "err", deriveErr) + return nil + } + baseURL = derived + } + if baseURL == "" { + slog.Warn("bitrix24 mcp: provisioning disabled — mcp_servers row has empty url", + "channel", c.Name(), "mcp_server", server.Name) return nil } c.mcpServerID = server.ID - c.mcpClient = newMCPClient(c.cfg.MCPBaseURL, 10*time.Second) + c.mcpClient = newMCPClient(baseURL, 10*time.Second) c.mcpDebounce = make(map[mcpDebounceKey]time.Time) slog.Info("bitrix24 mcp: provisioning enabled", "channel", c.Name(), - "mcp_server", c.cfg.MCPServerName, - "mcp_server_id", server.ID) + "mcp_server", server.Name, + "mcp_server_id", server.ID, + "require_user_credentials", server.RequireUserCredentials) return nil } @@ -342,6 +391,47 @@ func (c *Channel) selfRefreshUserCreds(ctx context.Context, userID string, exist return nil } +// deriveAutoOnboardBaseURL strips the path/query/fragment from an MCP server +// URL so what's left is safe to append "/api/auto-onboard" to. The +// mcp_servers.url column stores the JSON-RPC endpoint the agent loop dials +// for tool calls (which usually lives under a subpath like /mcp), while the +// per-user credential-minting REST call the Bitrix24 channel makes lives at +// the origin. This helper bridges the two conventions with a single URL +// column so operators don't have to fill in a second field. +// +// Examples: +// - "https://mcp.example.com/mcp" → "https://mcp.example.com" +// - "https://mcp.example.com/mcp/" → "https://mcp.example.com" +// - "https://mcp.example.com/" → "https://mcp.example.com" +// - "https://mcp.example.com" → "https://mcp.example.com" +// - "http://localhost:8080/some/path" → "http://localhost:8080" +// +// Returns an error when the string cannot be parsed as an absolute URL or +// carries no host — either case would produce a nonsensical base URL for +// the auto-onboard client and we prefer to disable provisioning rather than +// send credentials to a bogus origin. +func deriveAutoOnboardBaseURL(raw string) (string, error) { + trimmed := strings.TrimSpace(raw) + if trimmed == "" { + return "", errors.New("mcp_servers.url is empty") + } + u, err := url.Parse(trimmed) + if err != nil { + return "", fmt.Errorf("parse mcp_servers.url: %w", err) + } + if u.Scheme == "" || u.Host == "" { + return "", fmt.Errorf("mcp_servers.url %q has no scheme or host", trimmed) + } + // Reset every component that could turn the origin back into a full URL + // so a future field addition here doesn't silently break the invariant. + u.Path = "" + u.RawPath = "" + u.RawQuery = "" + u.Fragment = "" + u.Opaque = "" + return u.String(), nil +} + // tryAcquireMCPProvision atomically checks the debounce window for // (serverID, userID) and, if clear, records a fresh attempt — all under a // single lock so concurrent webhook-retry events can't both pass the gate diff --git a/internal/channels/bitrix24/provisioner_test.go b/internal/channels/bitrix24/provisioner_test.go index 900cb829..4dfd30bd 100644 --- a/internal/channels/bitrix24/provisioner_test.go +++ b/internal/channels/bitrix24/provisioner_test.go @@ -30,6 +30,7 @@ type fakeMCPStore struct { mu sync.Mutex serversByName map[string]*store.MCPServerData + serversByID map[uuid.UUID]*store.MCPServerData userCreds map[string]store.MCPUserCredentials // key = serverID + ":" + userID getUserCallCount int @@ -39,6 +40,7 @@ type fakeMCPStore struct { func newFakeMCPStore() *fakeMCPStore { return &fakeMCPStore{ serversByName: map[string]*store.MCPServerData{}, + serversByID: map[uuid.UUID]*store.MCPServerData{}, userCreds: map[string]store.MCPUserCredentials{}, } } @@ -81,8 +83,13 @@ func (f *fakeMCPStore) SetUserCredentials(_ context.Context, serverID uuid.UUID, func (f *fakeMCPStore) CreateServer(_ context.Context, _ *store.MCPServerData) error { return nil } -func (f *fakeMCPStore) GetServer(_ context.Context, _ uuid.UUID) (*store.MCPServerData, error) { - return nil, nil +func (f *fakeMCPStore) GetServer(_ context.Context, id uuid.UUID) (*store.MCPServerData, error) { + f.mu.Lock() + defer f.mu.Unlock() + if s, ok := f.serversByID[id]; ok { + return s, nil + } + return nil, nil // partner's contract: nil + nil when absent } func (f *fakeMCPStore) ListServers(_ context.Context) ([]store.MCPServerData, error) { return nil, nil } func (f *fakeMCPStore) UpdateServer(_ context.Context, _ uuid.UUID, _ map[string]any) error { @@ -607,6 +614,141 @@ func TestInitMCPProvisioner_DisabledModes(t *testing.T) { t.Errorf("missing server row should leave provisioner off") } }) + + // mcp_server_id path (v3.15+ dashboard wiring): the UUID string must + // parse and resolve to a live mcp_servers row. Broken UUID or missing + // row silently disables provisioning — no fallback to legacy name. + t.Run("mcp_server_id_invalid_uuid", func(t *testing.T) { + fs := newFakeStore() + resetWebhookRouterForTest() + defer resetWebhookRouterForTest() + + mcpStore := newFakeMCPStore() + + fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") + ch, _ := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_id":"not-a-uuid"}`), + bus.New(), nil) + bc := ch.(*Channel) + if err := bc.initMCPProvisioner(context.Background()); err != nil { + t.Fatalf("init: %v", err) + } + if bc.mcpClient != nil || bc.mcpServerID != uuid.Nil { + t.Errorf("invalid mcp_server_id UUID should leave provisioner off") + } + }) + + t.Run("mcp_server_id_row_not_found", func(t *testing.T) { + fs := newFakeStore() + resetWebhookRouterForTest() + defer resetWebhookRouterForTest() + + mcpStore := newFakeMCPStore() + // Intentionally do NOT seed serversByID — GetServer returns nil. + + fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") + ch, _ := fn("b1", nil, json.RawMessage(`{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_id":"00000000-0000-0000-0000-000000000042"}`), + bus.New(), nil) + bc := ch.(*Channel) + if err := bc.initMCPProvisioner(context.Background()); err != nil { + t.Fatalf("init: %v", err) + } + if bc.mcpClient != nil || bc.mcpServerID != uuid.Nil { + t.Errorf("missing mcp_servers row for mcp_server_id should leave provisioner off") + } + }) +} + +// TestDeriveAutoOnboardBaseURL guards the fix for the Phase 2 regression +// where mcp_servers.url (JSON-RPC endpoint under /mcp) was passed verbatim +// to mcp_client.newMCPClient — which appends "/api/auto-onboard" to it, +// producing "…/mcp/api/auto-onboard" (404). The origin extraction is the +// contract the Bitrix24 auto-onboard client expects. +func TestDeriveAutoOnboardBaseURL(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"jsonrpc subpath", "https://mcp.example.com/mcp", "https://mcp.example.com"}, + {"jsonrpc trailing slash", "https://mcp.example.com/mcp/", "https://mcp.example.com"}, + {"origin trailing slash", "https://mcp.example.com/", "https://mcp.example.com"}, + {"origin only", "https://mcp.example.com", "https://mcp.example.com"}, + {"deep path with query", "http://localhost:8080/some/path?x=1#frag", "http://localhost:8080"}, + {"prod b24 syn mcp", "https://b24-mcp-dev.synity.so/mcp", "https://b24-mcp-dev.synity.so"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := deriveAutoOnboardBaseURL(tc.in) + if err != nil { + t.Fatalf("deriveAutoOnboardBaseURL(%q) unexpected error: %v", tc.in, err) + } + if got != tc.want { + t.Errorf("deriveAutoOnboardBaseURL(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } + + errCases := []struct { + name string + in string + }{ + {"empty", ""}, + {"whitespace only", " \t"}, + {"no scheme", "mcp.example.com/mcp"}, + {"no host", "https:///path"}, + } + for _, tc := range errCases { + t.Run("err_"+tc.name, func(t *testing.T) { + if _, err := deriveAutoOnboardBaseURL(tc.in); err == nil { + t.Errorf("deriveAutoOnboardBaseURL(%q) expected error, got nil", tc.in) + } + }) + } +} + +// TestInitMCPProvisioner_MCPServerID exercises the v3.15+ id-based wiring: +// mcp_server_id resolves to a live mcp_servers row whose URL becomes the +// base URL for /api/auto-onboard. Legacy MCPServerName + MCPBaseURL are +// ignored on this path (single source of truth). +func TestInitMCPProvisioner_MCPServerID(t *testing.T) { + fs := newFakeStore() + resetWebhookRouterForTest() + defer resetWebhookRouterForTest() + + mcpStore := newFakeMCPStore() + serverID := uuid.MustParse("019df803-ec13-76c3-b1f5-60b0e80d3eec") + // URL carries a /mcp subpath because that's how partner MCP servers + // really configure the JSON-RPC endpoint. initMCPProvisioner must + // strip it before handing the value to mcp_client (which appends + // /api/auto-onboard). Regression guard for the Phase 2 fix. + mcpStore.serversByID[serverID] = &store.MCPServerData{ + BaseModel: store.BaseModel{ID: serverID}, + Name: "b24-syn-mcp", + URL: "https://b24-mcp.example.test/mcp", + Enabled: true, + // Field promoted from settings JSONB in Phase 89 — the provisioner + // reads it to log accurately and downstream provisionIfMissing + // respects it via manager-level checks. + RequireUserCredentials: true, + } + + fn := FactoryWithPortalStoreAndMCP(fs, mcpStore, "") + cfg := `{"portal":"p","bot_code":"c","bot_name":"n","mcp_server_id":"019df803-ec13-76c3-b1f5-60b0e80d3eec"}` + ch, err := fn("b1", nil, json.RawMessage(cfg), bus.New(), nil) + if err != nil { + t.Fatalf("factory: %v", err) + } + bc := ch.(*Channel) + if err := bc.initMCPProvisioner(context.Background()); err != nil { + t.Fatalf("init: %v", err) + } + + if bc.mcpClient == nil { + t.Fatalf("mcp_server_id path should wire mcpClient — got nil") + } + if bc.mcpServerID != serverID { + t.Errorf("mcpServerID = %s, want %s", bc.mcpServerID, serverID) + } } // newBareChannelForNotifyTest builds a Channel that's wired enough for diff --git a/internal/http/validate.go b/internal/http/validate.go index 8f8c0ab8..999bf051 100644 --- a/internal/http/validate.go +++ b/internal/http/validate.go @@ -74,7 +74,8 @@ var mcpServerAllowedFields = map[string]bool{ "name": true, "display_name": true, "transport": true, "command": true, "args": true, "url": true, "api_key": true, "env": true, "headers": true, "enabled": true, "tool_prefix": true, "timeout_sec": true, - "agent_id": true, "config": true, "settings": true, + "require_user_credentials": true, + "agent_id": true, "config": true, "settings": true, } var channelInstanceAllowedFields = map[string]bool{ diff --git a/internal/mcp/manager.go b/internal/mcp/manager.go index e1374a62..04442e67 100644 --- a/internal/mcp/manager.go +++ b/internal/mcp/manager.go @@ -280,7 +280,10 @@ func (m *Manager) resolveServerCredentials(ctx context.Context, info store.MCPAc } // Skip server if it requires scoped/user credentials and none are present. - if requireUserCreds(srv.Settings) { + // Prefer the top-level column (Phase 89 backfilled from settings JSONB); + // fall back to the legacy JSONB entry for cases where a caller mutated + // settings directly without touching the column. + if srv.RequireUserCredentials || requireUserCreds(srv.Settings) { if userID == "" { return nil } @@ -368,7 +371,7 @@ func (m *Manager) resolveServerCredentials(ctx context.Context, info store.MCPAc } tenantID := store.TenantIDFromContext(ctx) oauthUserID := "" - if requireUserCreds(srv.Settings) { + if srv.RequireUserCredentials || requireUserCreds(srv.Settings) { oauthUserID = userID } token, err2 := m.oauthTokenProvider.GetValidToken(ctx, srv.ID, tenantID, oauthUserID) @@ -441,7 +444,7 @@ func (m *Manager) LoadForAgent(ctx context.Context, agentID uuid.UUID, userID st for _, info := range accessible { // When loading at startup (userID=""), store servers requiring per-user // credentials for later per-request resolution instead of skipping them. - if userID == "" && requireUserCreds(info.Server.Settings) && info.Server.Enabled { + if userID == "" && (info.Server.RequireUserCredentials || requireUserCreds(info.Server.Settings)) && info.Server.Enabled { m.userCredServers = append(m.userCredServers, info) slog.Debug("mcp.server.deferred_user_creds", "server", info.Server.Name) continue diff --git a/internal/store/mcp_store.go b/internal/store/mcp_store.go index a49ee46d..1bc6e9b1 100644 --- a/internal/store/mcp_store.go +++ b/internal/store/mcp_store.go @@ -24,7 +24,13 @@ type MCPServerData struct { TimeoutSec int `json:"timeout_sec" db:"timeout_sec"` Settings json.RawMessage `json:"settings,omitempty" db:"settings"` Enabled bool `json:"enabled" db:"enabled"` - CreatedBy string `json:"created_by" db:"created_by"` + // RequireUserCredentials marks servers that mint credentials per-user at + // message time (e.g. Bitrix24 channel auto-onboard) instead of sharing a + // single admin api_key across every caller. Promoted from + // settings.require_user_credentials (JSONB) to a top-level column so + // channel factories can filter mcp_servers directly. + RequireUserCredentials bool `json:"require_user_credentials" db:"require_user_credentials"` + CreatedBy string `json:"created_by" db:"created_by"` } // MCPAgentGrant represents an MCP server grant to an agent. diff --git a/internal/store/pg/mcp_export_queries.go b/internal/store/pg/mcp_export_queries.go index 730c0376..cc05cb22 100644 --- a/internal/store/pg/mcp_export_queries.go +++ b/internal/store/pg/mcp_export_queries.go @@ -20,9 +20,10 @@ type MCPServerExport struct { Args json.RawMessage `json:"args,omitempty"` URL string `json:"url,omitempty"` ToolPrefix string `json:"tool_prefix,omitempty"` - TimeoutSec int `json:"timeout_sec"` - Settings json.RawMessage `json:"settings,omitempty"` - Enabled bool `json:"enabled"` + TimeoutSec int `json:"timeout_sec"` + Settings json.RawMessage `json:"settings,omitempty"` + Enabled bool `json:"enabled"` + RequireUserCredentials bool `json:"require_user_credentials,omitempty"` } // MCPGrantWithKey references an MCP agent grant by server_name and agent_key (portable). @@ -51,7 +52,7 @@ func ExportMCPServers(ctx context.Context, db *sql.DB) ([]MCPServerExport, error rows, err := db.QueryContext(ctx, "SELECT name, COALESCE(display_name,''), transport,"+ " COALESCE(command,''), args, COALESCE(url,''),"+ - " COALESCE(tool_prefix,''), timeout_sec, settings, enabled"+ + " COALESCE(tool_prefix,''), timeout_sec, settings, enabled, require_user_credentials"+ " FROM mcp_servers WHERE 1=1"+tc+ " ORDER BY name", tcArgs..., @@ -71,7 +72,7 @@ func ExportMCPServers(ctx context.Context, db *sql.DB) ([]MCPServerExport, error if err := rows.Scan( &srv.Name, &srv.DisplayName, &srv.Transport, &srv.Command, &argsRaw, &srv.URL, - &srv.ToolPrefix, &srv.TimeoutSec, &settings, &srv.Enabled, + &srv.ToolPrefix, &srv.TimeoutSec, &settings, &srv.Enabled, &srv.RequireUserCredentials, ); err != nil { slog.Warn("mcp_export.servers.scan", "error", err) continue @@ -185,12 +186,12 @@ func ImportMCPServer(ctx context.Context, db *sql.DB, srv MCPServerExport, creat _, err = db.ExecContext(ctx, `INSERT INTO mcp_servers (id, name, display_name, transport, command, args, url, - tool_prefix, timeout_sec, settings, enabled, + tool_prefix, timeout_sec, settings, enabled, require_user_credentials, created_by, created_at, updated_at, tenant_id) - VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,NOW(),NOW(),$13)`, + VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,NOW(),NOW(),$14)`, id, srv.Name, srv.DisplayName, srv.Transport, srv.Command, jsonOrNull(srv.Args), srv.URL, - srv.ToolPrefix, srv.TimeoutSec, jsonOrNull(srv.Settings), srv.Enabled, + srv.ToolPrefix, srv.TimeoutSec, jsonOrNull(srv.Settings), srv.Enabled, srv.RequireUserCredentials, createdBy, tid, ) if err != nil { diff --git a/internal/store/pg/mcp_servers.go b/internal/store/pg/mcp_servers.go index 461dfd26..f0974c55 100644 --- a/internal/store/pg/mcp_servers.go +++ b/internal/store/pg/mcp_servers.go @@ -56,12 +56,12 @@ func (s *PGMCPServerStore) CreateServer(ctx context.Context, srv *store.MCPServe _, err := s.db.ExecContext(ctx, `INSERT INTO mcp_servers (id, name, display_name, transport, command, args, url, headers, env, - api_key, tool_prefix, timeout_sec, settings, enabled, created_by, created_at, updated_at, tenant_id) - VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18)`, + api_key, tool_prefix, timeout_sec, settings, enabled, require_user_credentials, created_by, created_at, updated_at, tenant_id) + VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19)`, srv.ID, srv.Name, nilStr(srv.DisplayName), srv.Transport, nilStr(srv.Command), jsonOrEmpty(srv.Args), nilStr(srv.URL), encHeaders, encEnv, nilStr(apiKey), nilStr(srv.ToolPrefix), srv.TimeoutSec, - jsonOrEmpty(srv.Settings), srv.Enabled, srv.CreatedBy, now, now, tenantID, + jsonOrEmpty(srv.Settings), srv.Enabled, srv.RequireUserCredentials, srv.CreatedBy, now, now, tenantID, ) return err } @@ -69,7 +69,7 @@ func (s *PGMCPServerStore) CreateServer(ctx context.Context, srv *store.MCPServe const mcpServerSelectCols = `id, name, COALESCE(display_name, '') AS display_name, transport, COALESCE(command, '') AS command, args, COALESCE(url, '') AS url, headers, env, COALESCE(api_key, '') AS api_key, COALESCE(tool_prefix, '') AS tool_prefix, - timeout_sec, settings, enabled, created_by, created_at, updated_at` + timeout_sec, settings, enabled, require_user_credentials, created_by, created_at, updated_at` func (s *PGMCPServerStore) GetServer(ctx context.Context, id uuid.UUID) (*store.MCPServerData, error) { q := `SELECT ` + mcpServerSelectCols + ` FROM mcp_servers WHERE id = $1` diff --git a/internal/store/sqlitestore/mcp_servers.go b/internal/store/sqlitestore/mcp_servers.go index b2185d68..e36aca7c 100644 --- a/internal/store/sqlitestore/mcp_servers.go +++ b/internal/store/sqlitestore/mcp_servers.go @@ -17,7 +17,7 @@ import ( ) const mcpServerSelectCols = `id, name, display_name, transport, command, args, url, headers, env, - api_key, tool_prefix, timeout_sec, settings, enabled, created_by, created_at, updated_at` + api_key, tool_prefix, timeout_sec, settings, enabled, require_user_credentials, created_by, created_at, updated_at` // SQLiteMCPServerStore implements store.MCPServerStore backed by SQLite. type SQLiteMCPServerStore struct { @@ -59,12 +59,12 @@ func (s *SQLiteMCPServerStore) CreateServer(ctx context.Context, srv *store.MCPS _, err := s.db.ExecContext(ctx, `INSERT INTO mcp_servers (id, name, display_name, transport, command, args, url, headers, env, - api_key, tool_prefix, timeout_sec, settings, enabled, created_by, created_at, updated_at, tenant_id) - VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`, + api_key, tool_prefix, timeout_sec, settings, enabled, require_user_credentials, created_by, created_at, updated_at, tenant_id) + VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)`, srv.ID, srv.Name, nilStr(srv.DisplayName), srv.Transport, nilStr(srv.Command), jsonOrEmpty(srv.Args), nilStr(srv.URL), encHeaders, encEnv, nilStr(apiKey), nilStr(srv.ToolPrefix), srv.TimeoutSec, - jsonOrEmpty(srv.Settings), srv.Enabled, srv.CreatedBy, now, now, tenantID, + jsonOrEmpty(srv.Settings), srv.Enabled, srv.RequireUserCredentials, srv.CreatedBy, now, now, tenantID, ) return err } diff --git a/internal/store/sqlitestore/schema.go b/internal/store/sqlitestore/schema.go index ce220203..173f56bc 100644 --- a/internal/store/sqlitestore/schema.go +++ b/internal/store/sqlitestore/schema.go @@ -16,7 +16,7 @@ var schemaSQL string // SchemaVersion is the current SQLite schema version. // Bump this when adding new migration steps below. -const SchemaVersion = 55 +const SchemaVersion = 57 // migrations maps version → SQL to apply when upgrading FROM that version. // schema.sql always represents the LATEST full schema (for fresh DBs). @@ -898,6 +898,45 @@ ALTER TABLE usage_event_rollups ADD COLUMN thinking_tokens BIGINT NOT NULL DEFAU CREATE INDEX IF NOT EXISTS idx_channel_pending_messages_parent ON channel_pending_messages(channel_name, parent_history_key) WHERE parent_history_key <> '';`, + // Version 55 → 56: promote require_user_credentials from settings JSONB + // to a top-level column so channel factories can filter directly. + // Backfill reads the legacy JSONB via json_extract so no admin needs to + // re-tick after upgrading. Mirrors PG migration 000092. Idempotent-guarded + // via idempotentColumnMigration(55). + 55: `ALTER TABLE mcp_servers ADD COLUMN require_user_credentials BOOLEAN NOT NULL DEFAULT 0; +UPDATE mcp_servers + SET require_user_credentials = COALESCE(CAST(json_extract(settings, '$.require_user_credentials') AS INTEGER), 0) + WHERE settings IS NOT NULL + AND json_extract(settings, '$.require_user_credentials') IS NOT NULL;`, + // Version 56 → 57: backfill Bitrix24 channel_instances.config with + // mcp_server_id by resolving the legacy mcp_server_name against + // mcp_servers (matched on the channel's agent tenant_id since + // channel_instances doesn't carry tenant_id directly). Mirrors PG + // migration 000093. Idempotent — only touches rows without an + // existing mcp_server_id key. + 56: `UPDATE channel_instances + SET config = json_set( + COALESCE(config, '{}'), + '$.mcp_server_id', + (SELECT srv.id + FROM mcp_servers srv + WHERE srv.name = json_extract(channel_instances.config, '$.mcp_server_name') + AND srv.tenant_id = ( + SELECT a.tenant_id FROM agents a WHERE a.id = channel_instances.agent_id + ) + LIMIT 1) + ), + updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') + WHERE channel_type = 'bitrix24' + AND json_extract(config, '$.mcp_server_name') IS NOT NULL + AND json_extract(config, '$.mcp_server_id') IS NULL + AND EXISTS ( + SELECT 1 FROM mcp_servers srv + WHERE srv.name = json_extract(channel_instances.config, '$.mcp_server_name') + AND srv.tenant_id = ( + SELECT a.tenant_id FROM agents a WHERE a.id = channel_instances.agent_id + ) + );`, } const addUsageEventAnalyticsTables = ` @@ -1549,6 +1588,8 @@ func idempotentColumnMigration(version int) (string, string, bool) { return "secure_cli_binaries", "adapter_name", true case 51: return "webhook_calls", "last_heartbeat_at", true + case 55: + return "mcp_servers", "require_user_credentials", true default: return "", "", false } diff --git a/internal/store/sqlitestore/schema.sql b/internal/store/sqlitestore/schema.sql index e49e35c9..c6856b46 100644 --- a/internal/store/sqlitestore/schema.sql +++ b/internal/store/sqlitestore/schema.sql @@ -692,6 +692,13 @@ CREATE TABLE IF NOT EXISTS mcp_servers ( timeout_sec INT DEFAULT 60, settings TEXT NOT NULL DEFAULT '{}', enabled BOOLEAN NOT NULL DEFAULT 1, + -- require_user_credentials mirrors settings.require_user_credentials but + -- promoted to a top-level column so the Bitrix24 channel factory can + -- filter mcp_servers directly (indexable) rather than parse the full + -- JSONB per row. false = shared admin api_key applies to every caller. + -- true = the server mints credentials per-user at message time (Bitrix24 + -- auto-onboard etc.). + require_user_credentials BOOLEAN NOT NULL DEFAULT 0, created_by VARCHAR(255) NOT NULL, tenant_id TEXT NOT NULL REFERENCES tenants(id), created_at TEXT DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), diff --git a/internal/store/sqlitestore/sqlx_scan_structs.go b/internal/store/sqlitestore/sqlx_scan_structs.go index ffedd8c3..64a1144e 100644 --- a/internal/store/sqlitestore/sqlx_scan_structs.go +++ b/internal/store/sqlitestore/sqlx_scan_structs.go @@ -104,28 +104,30 @@ type mcpServerRow struct { ToolPrefix *string `json:"tool_prefix" db:"tool_prefix"` TimeoutSec int `json:"timeout_sec" db:"timeout_sec"` Settings json.RawMessage `json:"settings" db:"settings"` - Enabled bool `json:"enabled" db:"enabled"` - CreatedBy string `json:"created_by" db:"created_by"` - CreatedAt sqliteTime `json:"created_at" db:"created_at"` - UpdatedAt sqliteTime `json:"updated_at" db:"updated_at"` + Enabled bool `json:"enabled" db:"enabled"` + RequireUserCredentials bool `json:"require_user_credentials" db:"require_user_credentials"` + CreatedBy string `json:"created_by" db:"created_by"` + CreatedAt sqliteTime `json:"created_at" db:"created_at"` + UpdatedAt sqliteTime `json:"updated_at" db:"updated_at"` } func (r *mcpServerRow) toMCPServerData() store.MCPServerData { return store.MCPServerData{ - BaseModel: store.BaseModel{ID: r.ID, CreatedAt: r.CreatedAt.Time, UpdatedAt: r.UpdatedAt.Time}, - Name: r.Name, - DisplayName: derefStr(r.DisplayName), - Transport: r.Transport, - Command: derefStr(r.Command), - Args: r.Args, - URL: derefStr(r.URL), - Headers: r.Headers, - Env: r.Env, - APIKey: derefStr(r.APIKey), - ToolPrefix: derefStr(r.ToolPrefix), - TimeoutSec: r.TimeoutSec, - Settings: r.Settings, - Enabled: r.Enabled, - CreatedBy: r.CreatedBy, + BaseModel: store.BaseModel{ID: r.ID, CreatedAt: r.CreatedAt.Time, UpdatedAt: r.UpdatedAt.Time}, + Name: r.Name, + DisplayName: derefStr(r.DisplayName), + Transport: r.Transport, + Command: derefStr(r.Command), + Args: r.Args, + URL: derefStr(r.URL), + Headers: r.Headers, + Env: r.Env, + APIKey: derefStr(r.APIKey), + ToolPrefix: derefStr(r.ToolPrefix), + TimeoutSec: r.TimeoutSec, + Settings: r.Settings, + Enabled: r.Enabled, + RequireUserCredentials: r.RequireUserCredentials, + CreatedBy: r.CreatedBy, } } diff --git a/internal/upgrade/version.go b/internal/upgrade/version.go index a4e76ba7..89320e8b 100644 --- a/internal/upgrade/version.go +++ b/internal/upgrade/version.go @@ -2,4 +2,4 @@ package upgrade // RequiredSchemaVersion is the schema migration version this binary requires. // Bump this whenever adding a new SQL migration file. -const RequiredSchemaVersion uint = 91 +const RequiredSchemaVersion uint = 93 diff --git a/migrations/000092_add_require_user_credentials_to_mcp_servers.down.sql b/migrations/000092_add_require_user_credentials_to_mcp_servers.down.sql new file mode 100644 index 00000000..266d20cb --- /dev/null +++ b/migrations/000092_add_require_user_credentials_to_mcp_servers.down.sql @@ -0,0 +1 @@ +ALTER TABLE mcp_servers DROP COLUMN IF EXISTS require_user_credentials; diff --git a/migrations/000092_add_require_user_credentials_to_mcp_servers.up.sql b/migrations/000092_add_require_user_credentials_to_mcp_servers.up.sql new file mode 100644 index 00000000..a68e104b --- /dev/null +++ b/migrations/000092_add_require_user_credentials_to_mcp_servers.up.sql @@ -0,0 +1,16 @@ +-- Promote the require_user_credentials flag from settings JSONB to a top-level +-- column so channel factories can filter mcp_servers directly (indexable) and +-- reduce the "read the whole JSONB" cost paid on every message. +-- +-- The JSONB entry stays in place for one release cycle for legacy readers +-- (internal/mcp/manager.go:requireUserCreds); a later migration removes it +-- once all callers are on the column. +ALTER TABLE mcp_servers + ADD COLUMN IF NOT EXISTS require_user_credentials BOOLEAN NOT NULL DEFAULT false; + +-- Backfill the column from existing settings blobs so no admin has to +-- re-tick the checkbox after upgrading. +UPDATE mcp_servers + SET require_user_credentials = COALESCE((settings->>'require_user_credentials')::boolean, false) + WHERE settings IS NOT NULL + AND settings ? 'require_user_credentials'; diff --git a/migrations/000093_backfill_bitrix24_mcp_server_id.down.sql b/migrations/000093_backfill_bitrix24_mcp_server_id.down.sql new file mode 100644 index 00000000..6c8bfbbe --- /dev/null +++ b/migrations/000093_backfill_bitrix24_mcp_server_id.down.sql @@ -0,0 +1,9 @@ +-- Revert the backfill by dropping the mcp_server_id key from Bitrix24 +-- channel_instances configs. The legacy mcp_server_name + mcp_base_url pair +-- was never touched by the up migration, so provisioning falls back to it +-- automatically once the new key is gone. +UPDATE channel_instances + SET config = config - 'mcp_server_id', + updated_at = NOW() + WHERE channel_type = 'bitrix24' + AND config ? 'mcp_server_id'; diff --git a/migrations/000093_backfill_bitrix24_mcp_server_id.up.sql b/migrations/000093_backfill_bitrix24_mcp_server_id.up.sql new file mode 100644 index 00000000..3d2f78d8 --- /dev/null +++ b/migrations/000093_backfill_bitrix24_mcp_server_id.up.sql @@ -0,0 +1,25 @@ +-- Backfill channel_instances.config -> mcp_server_id for Bitrix24 channels that +-- currently store the legacy mcp_server_name. Resolves the name to a UUID via +-- mcp_servers (scoped to the channel agent's tenant) so the new MCP dropdown +-- and factory path (Phase 89) can drive lookups without touching the legacy +-- mcp_server_name / mcp_base_url pair. +-- +-- Idempotent: only touches rows where config already carries the legacy +-- mcp_server_name AND does not yet carry mcp_server_id. Rows whose name does +-- not resolve to any mcp_servers row in the same tenant are left alone -- the +-- factory falls back to the legacy pair for them until an admin opens the +-- channel form and picks a server from the dropdown. +UPDATE channel_instances ci + SET config = jsonb_set( + COALESCE(ci.config, '{}'::jsonb), + '{mcp_server_id}', + to_jsonb(srv.id::text) + ), + updated_at = NOW() + FROM mcp_servers srv, agents a + WHERE ci.channel_type = 'bitrix24' + AND ci.config ? 'mcp_server_name' + AND NOT (ci.config ? 'mcp_server_id') + AND ci.config->>'mcp_server_name' = srv.name + AND ci.agent_id = a.id + AND a.tenant_id = srv.tenant_id; diff --git a/ui/desktop/frontend/src/types/mcp.ts b/ui/desktop/frontend/src/types/mcp.ts index 22322916..c2edc4d8 100644 --- a/ui/desktop/frontend/src/types/mcp.ts +++ b/ui/desktop/frontend/src/types/mcp.ts @@ -12,6 +12,8 @@ export interface MCPServerData { timeout_sec: number settings?: { require_user_credentials?: boolean } enabled: boolean + /** Top-level twin of `settings.require_user_credentials` (Phase 89 rollout). */ + require_user_credentials?: boolean created_by: string agent_count?: number created_at: string @@ -30,6 +32,8 @@ export interface MCPServerInput { tool_prefix?: string timeout_sec?: number enabled?: boolean + /** Top-level flag written since Phase 89; backends still accept the legacy JSONB entry. */ + require_user_credentials?: boolean } export interface MCPAgentGrant { diff --git a/ui/web/src/pages/channels/bitrix24/mcp-server-select.tsx b/ui/web/src/pages/channels/bitrix24/mcp-server-select.tsx new file mode 100644 index 00000000..5b29b0f2 --- /dev/null +++ b/ui/web/src/pages/channels/bitrix24/mcp-server-select.tsx @@ -0,0 +1,70 @@ +import { useMemo } from "react"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { useMCP } from "@/pages/mcp/hooks/use-mcp"; +import type { MCPServerData } from "@/types/mcp"; + +/** + * Dropdown picker over MCP servers whose `require_user_credentials` flag is + * true — the population channels can meaningfully auto-onboard against. + * + * The list comes from the same `useMCP()` query the MCP admin page uses so + * cache is shared and the filter is a cheap client-side pass (~10 servers + * per portal on realistic setups). Legacy `mcp_server_name` fallback is + * handled at the Bitrix24 factory layer; this component only writes UUIDs. + */ +interface MCPServerSelectProps { + value: string; + onChange: (value: string | undefined) => void; + placeholder?: string; + disabled?: boolean; +} + +const CLEAR_VALUE = "__none__"; + +export function MCPServerSelect({ value, onChange, placeholder, disabled }: MCPServerSelectProps) { + const { servers, loading } = useMCP(); + + const perUserServers = useMemo( + () => + (servers ?? []).filter( + (s) => s.require_user_credentials || s.settings?.require_user_credentials, + ), + [servers], + ); + + const selectValue = value && value !== "" ? value : CLEAR_VALUE; + + return ( +
+ +
+ ); +} diff --git a/ui/web/src/pages/channels/channel-fields.tsx b/ui/web/src/pages/channels/channel-fields.tsx index 4716df13..7e3727fb 100644 --- a/ui/web/src/pages/channels/channel-fields.tsx +++ b/ui/web/src/pages/channels/channel-fields.tsx @@ -17,6 +17,7 @@ import { SkillNameSelect } from "@/components/shared/skill-name-select"; import { ProviderModelSelect } from "@/components/shared/provider-model-select"; import { InfoLabel } from "@/components/shared/info-label"; import { BitrixPortalSelect } from "./bitrix24/bitrix-portal-select"; +import { MCPServerSelect } from "./bitrix24/mcp-server-select"; import { deliveryModelKey, isDeliveryModelKey, isDeliveryProviderKey } from "./channel-delivery-provider-fields"; import type { FieldDef } from "./channel-schemas"; @@ -424,6 +425,19 @@ function FieldRenderer({ ); + case "mcp-select": + return ( +
+ + + {inlineHelp &&

{inlineHelp}

} +
+ ); + case "tags": return (
diff --git a/ui/web/src/pages/channels/channel-schemas.ts b/ui/web/src/pages/channels/channel-schemas.ts index 37938ee7..e1d9ccf0 100644 --- a/ui/web/src/pages/channels/channel-schemas.ts +++ b/ui/web/src/pages/channels/channel-schemas.ts @@ -5,7 +5,7 @@ import { reasoningDeliveryOptions } from "./reasoning-delivery-config"; export interface FieldDef { key: string; label: string; - type: "text" | "password" | "number" | "boolean" | "select" | "multi-select" | "tags" | "tristate" | "textarea" | "tool-select" | "skill-select"; + type: "text" | "password" | "number" | "boolean" | "select" | "multi-select" | "tags" | "tristate" | "textarea" | "tool-select" | "skill-select" | "mcp-select"; placeholder?: string; required?: boolean; defaultValue?: string | number | boolean | string[]; @@ -298,8 +298,14 @@ export const configSchema: Record = { { key: "allow_from", label: "Allowed Users (DM)", type: "tags", help: "Bitrix24 user IDs allowed to DM the bot. Empty = no allowlist filter." }, { key: "group_allow_from", label: "Allowed Users (Group)", type: "tags", help: "Separate allowlist for group senders." }, ...chatBehaviorOverrideFields, - { key: "mcp_server_name", label: "MCP Server Name", type: "text", advanced: true, placeholder: "bitrix24-prod", help: "Optional — name from mcp_servers table. Must be set together with MCP Base URL to enable per-user MCP credential auto-onboard. Leave both empty to disable." }, - { key: "mcp_base_url", label: "MCP Base URL", type: "text", advanced: true, placeholder: "https://mcp.example.com", help: "Optional — HTTPS root of the partner MCP server. Channel POSTs {mcp_base_url}/api/auto-onboard to mint per-user credentials on first-sight. The MCP server authenticates each call via the caller's Bitrix access_token, so no admin secret is required." }, + // Preferred single-select input backed by mcp_servers.require_user_credentials. + // Base URL is resolved from the selected row at Start() — no separate field needed. + { key: "mcp_server_id", label: "MCP Server", type: "mcp-select", help: "Optional — pick a per-user MCP server (only servers with \"Require user credentials\" ticked appear here). Channel POSTs {server.url}/api/auto-onboard on first-sight to mint the caller's per-user credential. Leave empty to disable MCP provisioning for this channel." }, + // Legacy string fields — kept for backward-compat with configs written before + // the mcp_server_id rollout. Phase 5 migrates every existing channel to + // mcp_server_id and drops these entries. + { key: "mcp_server_name", label: "MCP Server Name (legacy)", type: "text", advanced: true, placeholder: "bitrix24-prod", help: "Deprecated — use \"MCP Server\" dropdown instead. Kept for backward-compat with pre-Phase-89 configs; will be removed once every channel is migrated." }, + { key: "mcp_base_url", label: "MCP Base URL (legacy)", type: "text", advanced: true, placeholder: "https://mcp.example.com", help: "Deprecated — see the \"MCP Server\" dropdown. Legacy base URL used only when the new dropdown is empty and the legacy name is set." }, ], }; diff --git a/ui/web/src/pages/mcp/mcp-form-dialog.tsx b/ui/web/src/pages/mcp/mcp-form-dialog.tsx index 07254a85..2f3d3df3 100644 --- a/ui/web/src/pages/mcp/mcp-form-dialog.tsx +++ b/ui/web/src/pages/mcp/mcp-form-dialog.tsx @@ -100,7 +100,10 @@ export function MCPFormDialog({ open, onOpenChange, server, onSubmit, onTest, on toolPrefix: server?.tool_prefix ?? "", timeout: server?.timeout_sec ?? 60, enabled: server?.enabled ?? true, - requireUserCreds: server?.settings?.require_user_credentials ?? false, + // Prefer the top-level column (Phase 89 backfilled from settings JSONB); + // fall back to the legacy settings entry so cached responses from + // pre-upgrade backends still render the checkbox with the right value. + requireUserCreds: server?.require_user_credentials ?? server?.settings?.require_user_credentials ?? false, toolHintsGlobal: server?.settings?.tool_hints?.global ?? "", toolHintsTools: server?.settings?.tool_hints?.tools ?? {}, oauthEnabled: oauth?.auth_type === "oauth", @@ -190,6 +193,10 @@ export function MCPFormDialog({ open, onOpenChange, server, onSubmit, onTest, on timeout_sec: data.timeout, settings, enabled: data.enabled, + // Send the promoted top-level flag alongside the legacy settings + // twin so both backends (pre- and post-Phase 89) stay in sync during + // the migration window. Phase 5 drops the settings entry. + require_user_credentials: data.requireUserCreds, }; }; diff --git a/ui/web/src/types/mcp.ts b/ui/web/src/types/mcp.ts index 2f88299b..3877ec2a 100644 --- a/ui/web/src/types/mcp.ts +++ b/ui/web/src/types/mcp.ts @@ -41,6 +41,14 @@ export interface MCPServerData { timeout_sec: number; settings?: MCPServerSettings; enabled: boolean; + /** + * True when the server mints credentials per-user at message time + * (Bitrix24 auto-onboard etc.). Promoted from `settings.require_user_credentials` + * to a top-level column; the settings entry stays for one release cycle + * so legacy readers keep working. Prefer this field over the settings + * entry when both are present. + */ + require_user_credentials?: boolean; created_by: string; agent_count?: number; created_at: string; @@ -60,6 +68,12 @@ export interface MCPServerInput { timeout_sec?: number; settings?: MCPServerSettings; enabled?: boolean; + /** + * Top-level twin of `settings.require_user_credentials`. Send both while + * the server backend is still on the migration window (readers may hit + * either field); after Phase 5 lands the settings entry disappears. + */ + require_user_credentials?: boolean; } export interface MCPToolInfo {