From 47302becafaa11ab094f9ad92e2166884676589a Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 9 Nov 2026 01:42:46 -0600 Subject: [PATCH] fix(chat): refuse chat endpoints while their experiment is off, and say so in search A run asked to chat over iMessage while Chat connectors was off. Search did not surface iMessage for a longer query, and POST chat-endpoints created a draft endpoint for a provider the instance has turned off. - Chat endpoint creation now returns 403 with the Experimental settings page while the provider's experiment (Chat connectors, or GitHub review bots for GitHub) is off. AgentMail is unchanged. - Hidden chat channels match on their description too, so worded requests find the unavailable result. - The own-server instruction says the person enters the key and the agent must not create the connection itself. - The interaction listing says only the addressee resolves a connection request card. Co-Authored-By: Claude Opus 5.5 --- .../chat-channels.integration.test.ts | 22 +++++++++++++++++++ .../connection-intents-service.test.ts | 4 +++- .../person-only-page-guidance.test.ts | 14 ++++++++++-- server/src/routes/chat-channels.ts | 12 ++++++++++ server/src/routes/openapi.ts | 5 ++++- server/src/services/connection-intents.ts | 13 ++++++----- 6 files changed, 61 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index aa8aa1a61fb..e5af378da22 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -9607,7 +9607,27 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }; } + it("refuses to create a chat endpoint while its provider experiment is off", async () => { + const settings = instanceSettingsService(db); + const previous = await settings.getExperimental(); + try { + await settings.updateExperimental({ enableChatConnectors: false, enableGitHubReviewBots: false }); + const fixture = await seedCompany(); + const { service } = createService(); + const app = routesApp(db, fixture.companyId, service); + for (const provider of ["imessage-photon", "slack", "github"]) { + const response = await request(app).post(`/api/companies/${fixture.companyId}/chat-endpoints`) + .send({ provider, assignedAgentId: fixture.assignedAgentId }).expect(403); + expect(response.body.error).toContain("//company/settings/instance/experimental"); + } + expect(await service.list(fixture.companyId)).toEqual([]); + } finally { + await settings.updateExperimental({ enableChatConnectors: previous.enableChatConnectors, enableGitHubReviewBots: previous.enableGitHubReviewBots }); + } + }); + it("creates a channel-purpose connection and rejects attempts to change its assigned agent", async () => { + await instanceSettingsService(db).updateExperimental({ enableChatConnectors: true }); const fixture = await seedCompany(); const { service } = createService(); const app = routesApp(db, fixture.companyId, service); @@ -9662,6 +9682,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { const signingSecret = "signing-secret-canary"; const clientSecret = "client-secret-canary"; it("saves advanced app details with the initial draft and rejects them for other providers", async () => { + await instanceSettingsService(db).updateExperimental({ enableChatConnectors: true }); const company = await seedCompany(); const { service } = createService(); const app = routesApp(db, company.companyId, service); @@ -11808,6 +11829,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }); it("rejects chat endpoints for non-invokable agents", async () => { + await instanceSettingsService(db).updateExperimental({ enableChatConnectors: true }); const fixture = await seedCompany(); const { service } = createService(); const app = routesApp(db, fixture.companyId, service); diff --git a/server/src/__tests__/connection-intents-service.test.ts b/server/src/__tests__/connection-intents-service.test.ts index e5075374513..3233fae8705 100644 --- a/server/src/__tests__/connection-intents-service.test.ts +++ b/server/src/__tests__/connection-intents-service.test.ts @@ -232,6 +232,8 @@ describeEmbeddedPostgres("connectionIntentService", () => { const service = connectionIntentService(db); try { await settings.updateExperimental({ enableChatConnectors: false }); + // A longer request, as an agent words it, still finds the hidden channel. + expect((await service.search(claims, "iMessage chat messaging with a contact")).results.find(item => item.service === "imessage-photon")?.state).toBe("unavailable"); const off = await service.search(claims, "iMessage"); const imessage = off.results.find(item => item.service === "imessage-photon"); expect(imessage).toMatchObject({ state: "unavailable", methods: [] }); @@ -251,7 +253,7 @@ describeEmbeddedPostgres("connectionIntentService", () => { it("points a service the catalog does not list at the secure own-server setup page", async () => { const result = await connectionIntentService(db).search(claims, "Synthetic notes service"); expect(result.instruction).toMatch(/\[Connect your own MCP server\]\(\/[A-Z0-9]+\/apps\/byo\)/); - expect(result.instruction).toContain("never ask for a key in chat"); + expect(result.instruction).toContain("Do not create the connection yourself, ask for the key in chat"); }); it("creates one addressed request, resolves after install, and then reports ready for the responsible user", async () => { diff --git a/server/src/__tests__/person-only-page-guidance.test.ts b/server/src/__tests__/person-only-page-guidance.test.ts index 8752d579f2f..747e7fd16b8 100644 --- a/server/src/__tests__/person-only-page-guidance.test.ts +++ b/server/src/__tests__/person-only-page-guidance.test.ts @@ -30,6 +30,11 @@ it.each([ expect(operation!.description).toContain("No agent can call a quarantined action"); }); +it("says chat endpoint creation needs the experiment, and links where it is turned on", () => { + const create = searchRunnerApi({ query: "create chat endpoint imessage", limit: 10 }).results.find(row => row.operationId === "POST /api/companies/{companyId}/chat-endpoints"); + expect(create?.description).toContain("[Experimental settings](//company/settings/instance/experimental)"); +}); + it.each([ "complete connection request for someone else", "connection intent resolve decline", @@ -42,16 +47,21 @@ it.each([ } }); +it("says on the interaction listing that only the addressee resolves a connection request card", () => { + const list = searchRunnerApi({ query: "GET /api/issues/{id}/interactions" }).results.find(row => row.operationId === "GET /api/issues/{id}/interactions"); + expect(list?.description).toContain("Only that person can connect or decline it"); +}); + // The catalog text is the only place these page paths live on the server, so a renamed UI route // would leave agents linking a page that no longer exists. Every page named must be mounted. it("names only pages that the board UI mounts", () => { const appRoutes = readFileSync(path.join(repoRoot, "ui/src/App.tsx"), "utf8"); const pages = new Set( runnerApiCatalog() - .filter(row => /^\/api\/(mcp\/(setup|requests\/\{id\}\/consent)$|connection-intents\/|tool-connections\/\{connectionId\}\/(catalog|review-counts)$|companies\/\{companyId\}\/tools\/apps\/attention$)/.test(row.path)) + .filter(row => /^\/api\/(mcp\/(setup|requests\/\{id\}\/consent)$|connection-intents\/|tool-connections\/\{connectionId\}\/(catalog|review-counts)$|companies\/\{companyId\}\/(tools\/apps\/attention|chat-endpoints)$)/.test(row.path)) .flatMap(row => [...(row.description ?? "").matchAll(/\/\/([a-z0-9/<>-]+)/gi)].map(match => match[1]!)), ); - expect([...pages].sort()).toEqual(["apps//permissions", "apps/assistant-connection", "issues/"]); + expect([...pages].sort()).toEqual(["apps//permissions", "apps/assistant-connection", "company/settings/instance/experimental", "issues/"]); const mounted = (page: string) => page .replace("apps//permissions", "apps/:connectionId/:tab") .replace("issues/", "issues/:issueId"); diff --git a/server/src/routes/chat-channels.ts b/server/src/routes/chat-channels.ts index 162c2c3829b..ad02e08164c 100644 --- a/server/src/routes/chat-channels.ts +++ b/server/src/routes/chat-channels.ts @@ -144,6 +144,16 @@ export function chatChannelRoutes(db: Db, options: ChatChannelRouteOptions) { if (!enabled) throw forbidden("This connector is not enabled on this instance"); } + function assertChatProviderEnabledForCreate( + provider: string, + experimental: { enableChatConnectors: boolean; enableGitHubReviewBots: boolean }, + ) { + if (provider === "agentmail") return; + if (provider === "github" ? experimental.enableGitHubReviewBots : experimental.enableChatConnectors) return; + const setting = provider === "github" ? "GitHub review bots" : "Chat connectors"; + throw forbidden(`${setting} is turned off on this instance, so this chat endpoint cannot be created. An instance admin turns it on in Experimental settings at //company/settings/instance/experimental. Agents cannot change instance settings.`); + } + async function assertConnectionManager( req: ExpressRequest, companyId: string, @@ -195,6 +205,8 @@ export function chatChannelRoutes(db: Db, options: ChatChannelRouteOptions) { async (req, res) => { const companyId = req.params.companyId as string; await assertConnectionManager(req, companyId); + // Setup pages are hidden while the provider's experiment is off; the API must not create the endpoint either. + assertChatProviderEnabledForCreate(req.body.provider, await instanceSettingsService(db).getExperimental()); if (req.body.provider === "slack" && !req.body.slackSetupMethod) { req.body.slackSetupMethod = service.slackRegistration.managerGrants.defaultMethod(); } diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index f7be3bc7157..97da67811fd 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -2369,7 +2369,9 @@ registry.registerPath({ tags: ["chat-channels"], summary: "Create a chat endpoint", description: - "Creates one provider bot endpoint bound permanently to one Paperclip agent. Provider setup and verification happen in later calls.", + "Creates one provider bot endpoint bound permanently to one Paperclip agent. Provider setup and verification happen in later calls. " + + "Chat providers need Chat connectors, and GitHub needs GitHub review bots, turned on by an instance admin in Experimental settings at //company/settings/instance/experimental. " + + "While that setting is off, connections_search shows the provider as unavailable and this call is refused; agents cannot change it. Link that page as [Experimental settings](//company/settings/instance/experimental) and reply.", request: { params: z.object({ companyId: z.string().uuid() }), body: jsonBody(createChatEndpointSchema), @@ -8348,6 +8350,7 @@ registry.registerPath({ path: "/api/issues/{id}/interactions", tags: ["issues"], summary: "List issue thread interactions", + description: "A connection_intent interaction is a connection request card addressed to one person, its addresseeUserId. Only that person can connect or decline it; no other member, board user or agent can resolve it for them.", request: { params: z.object({ id: z.string() }) }, responses: { 200: r.ok(), 401: r.unauthorized }, }); diff --git a/server/src/services/connection-intents.ts b/server/src/services/connection-intents.ts index 57afeaadf09..047af685d0e 100644 --- a/server/src/services/connection-intents.ts +++ b/server/src/services/connection-intents.ts @@ -543,13 +543,15 @@ export function connectionIntentService(db: Db) { connectionId: ready?.id ?? null, }}); } - // A chat channel hidden by the instance setting stays findable by name, so the agent can say why - // it is unavailable and where it is turned on instead of searching for a route that is not offered. + // A chat channel hidden by the instance setting stays findable, so the agent can say why it is + // unavailable and where it is turned on instead of searching for a route that is not offered. if (!settings.enableChatConnectors) for (const app of APP_STORE_DEFINITIONS) { if (!isConnectorFeatureEnabled(app, settings) || discoveryMethods(app).length || !getAvailableConnectionMethods(app).some(method => method.purpose === "channel" && method.provider !== "github")) continue; - const { score, nameScore } = scoreConnectionSearch(preparedQuery, [app.slug, app.name], ""); - if (!nameScore) continue; + const channelMethods = getAvailableConnectionMethods(app).filter(method => method.purpose === "channel"); + const { score, nameScore } = scoreConnectionSearch(preparedQuery, [app.slug, app.name], + `${app.description ?? ""} ${channelMethods.map(method => `${method.whenToUse} ${method.label ?? ""}`).join(" ")}`); + if (!score) continue; candidates.push({ score, nameScore, item: { service: app.slug, name: app.name, description: app.description ?? null, logoUrl: app.branding.logoUrl ?? null, methods: [], source: "catalog", state: "unavailable", connectionId: null, @@ -713,7 +715,8 @@ export function connectionIntentService(db: Db) { function directSearchResult(query: string, results: ConnectionSearchResultItem[], prefix: string, suggestions = false): ConnectionsSearchResult { // A service the catalog does not list can still be the person's own MCP server; name its secure setup page. - const ownServer = `they can add it as their own MCP server on [Connect your own MCP server](/${prefix}/apps/byo), which takes the URL and keeps any API key in a secure field. Link that page; never ask for a key in chat.`; + const ownServer = `the person adds it as their own MCP server on [Connect your own MCP server](/${prefix}/apps/byo): they enter the URL and type any API key into that page's secure field themselves. ` + + "An agent cannot enter the key. Do not create the connection yourself, ask for the key in chat or search for another route; link that page and reply."; return { version: 1, query, results, instruction: !results.length ? `No verified connection route was found. If the person gave the service's own server URL, ${ownServer} Otherwise explain that support could not be verified; do not invent a provider route or request unsupported services.` : !suggestions && isRemoteMcpConnectorId(results[0]!.service) && results[0]!.state !== "unavailable"