From 2fc02935869d2eff0688cd6c9cae1beccc0605cb Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 00:08:31 -0600 Subject: [PATCH] fix(api): tell agents only the person can revoke assistant connections, and where (#20170) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - People connect external assistants to Paperclip, and each person manages their own assistant connections in the browser > - Chat agents find API routes through `search_api` and call them with `call_api` > - The catalog entries for `/api/mcp/connections` did not say who can use them or which page holds the control > - `call_api` refuses these routes as protocol endpoints, and its message did not say the refusal is final > - A chat agent asked to "disconnect my old assistant" searched for an agent route for its whole turn and did not reply > - This pull request names the person-only boundary and the page in the catalog, and makes the refusal final > - The benefit is that the agent replies at once with a link to the page where the person revokes the connection ## Linked Issues or Issue Description No public issue exists. This is a bug report. **What happened** A person asked the chat agent: "Disconnect my old assistant, but leave the website task running." The agent found `DELETE /api/mcp/connections/{id}`. `call_api` refused the list route with "This endpoint requires its existing protocol client; call_api supports REST only". The agent then searched the API for other routes (dot bindings, agents, "mcp", "assistant") for two minutes. The turn timed out with no reply. The agent wrote in its reasoning that there "must be a supported path". **Expected behavior** The agent tells the person that only they can revoke the connection, links the Assistant connections page, and says the delegated task keeps running. **Steps to reproduce** 1. Connect two assistants to a company as one person. Let the first assistant create a task for an agent. 2. In chat, ask the CEO: "Disconnect my old assistant, but leave the website task running." 3. See that the agent searches the API until the turn ends and does not reply. ## What Changed - `server/src/routes/openapi.ts`: the `GET /api/mcp/connections` and `DELETE /api/mcp/connections/{id}` entries now have a description. It says only the signed-in person can list or revoke their assistant connections, links `[Assistant connections](//apps/assistant-connection)`, notes that the connected assistant itself can revoke its own token through OAuth token revocation, says not to search for another route, and says that tasks the assistant already created keep running. - `server/src/services/native-runtime/runner-api-client.ts`: the `call_api` protocol refusal now says no other API route substitutes for the endpoint. It points to the catalog description only when that operation has one, so undocumented protocol routes such as `POST /api/mcp/project-tools` do not send the agent to an empty description. - `server/src/__tests__/assistant-connections-guidance.test.ts`: new regression test. It uses the exact search queries from the failed run and covers the refusal with and without a description. - `server/src/__tests__/public-mcp.test.ts`: agent and non-session board actors get 401 on listing and revoking assistant connections. ## Verification - `cd server && pnpm exec vitest run src/__tests__/assistant-connections-guidance.test.ts src/__tests__/primary-agent-guidance.test.ts src/__tests__/openapi-routes.test.ts src/services/native-runtime/runner-api.test.ts` passes (2111 tests). After the review fixes, `src/__tests__/assistant-connections-guidance.test.ts src/__tests__/public-mcp.test.ts src/services/native-runtime/runner-api.test.ts` passes (2157 tests), and the server `tsc --noEmit` passes. - The new test fails on `master` without the change (2 failures) and passes with it. - The end-to-end check is the `assistant-connections.revoke-personal-grant` chat eval with this commit cherry-picked. ## Risks - Low risk. The change is catalog text and one error message. No route, permission or data behavior changes. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .../assistant-connections-guidance.test.ts | 34 +++++++++++++++++++ server/src/__tests__/public-mcp.test.ts | 2 ++ server/src/routes/openapi.ts | 7 ++++ .../native-runtime/runner-api-client.ts | 2 +- 4 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 server/src/__tests__/assistant-connections-guidance.test.ts diff --git a/server/src/__tests__/assistant-connections-guidance.test.ts b/server/src/__tests__/assistant-connections-guidance.test.ts new file mode 100644 index 00000000000..d03719da11a --- /dev/null +++ b/server/src/__tests__/assistant-connections-guidance.test.ts @@ -0,0 +1,34 @@ +import { expect, it } from "vitest"; +import { searchRunnerApi } from "../services/native-runtime/runner-api-catalog.js"; +import { validateRunnerApiCall } from "../services/native-runtime/runner-api-client.js"; + +const context = { companyId: "00000000-0000-4000-8000-000000000121", issueId: "00000000-0000-4000-8000-000000000122", runId: "00000000-0000-4000-8000-000000000123", workMode: "standard" }; + +it("tells agents searching for assistant disconnection that only the person can do it, and where", () => { + for (const query of ["chat client assistant disconnect connection", "assistant connections revoke list", "mcp connection revoke REST"]) { + const found = searchRunnerApi({ query, limit: 10 }).results; + const revoke = found.find(row => row.operationId === "DELETE /api/mcp/connections/{id}"); + expect(revoke, query).toBeDefined(); + expect(revoke!.description).toContain("[Assistant connections](//apps/assistant-connection)"); + expect(revoke!.description).toContain("only the signed-in person"); + expect(revoke!.description).toContain("Do not search for another route"); + expect(revoke!.description).toContain("OAuth token revocation sent by the connected assistant itself"); + expect(revoke!.description).toMatch(/already created keep their assignees and running work/); + } + const list = searchRunnerApi({ query: "GET /api/mcp/connections" }).results[0]!; + expect(list.description).toContain("[Assistant connections](//apps/assistant-connection)"); +}); + +it("says the call_api protocol rejection is final instead of inviting another search", () => { + expect(() => validateRunnerApiCall({ operationId: "GET /api/mcp/connections" }, context)) + .toThrow(/no other API route substitutes for it\. Its catalog description says who can use it\./); +}); + +it("points at the catalog description only when the protocol endpoint has one", () => { + const undocumented = searchRunnerApi({ query: "POST /api/mcp/project-tools" }).results.find(row => row.operationId === "POST /api/mcp/project-tools"); + expect(undocumented?.description).toBe(""); + let message = ""; + try { validateRunnerApiCall({ operationId: "POST /api/mcp/project-tools" }, context); } catch (error) { message = (error as Error).message; } + expect(message).toContain("no other API route substitutes for it."); + expect(message).not.toContain("catalog description"); +}); diff --git a/server/src/__tests__/public-mcp.test.ts b/server/src/__tests__/public-mcp.test.ts index 808bb56847c..c5e406b360a 100644 --- a/server/src/__tests__/public-mcp.test.ts +++ b/server/src/__tests__/public-mcp.test.ts @@ -466,6 +466,8 @@ describe.skipIf(!support.supported)("public MCP OAuth and tool boundary", () => ] as Request["actor"][]) { actor = candidate; expect((await request(app).get("/api/mcp/setup")).status).toBe(401); + expect((await request(app).get("/api/mcp/connections")).status).toBe(401); + expect((await request(app).delete(`/api/mcp/connections/${randomUUID()}`).set("Origin", config.origin)).status).toBe(401); } for (const source of ["session", "cloud_tenant"] as const) { actor = { type: "board", source, userId: randomUUID() }; diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index b3a9bb92797..5efd8ba619b 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -12957,14 +12957,21 @@ registerCurrentRoute({ body: mcpConsentSchema.extend({ userCode: z.string().min(8).max(12) }), responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden }, }); +const ASSISTANT_CONNECTIONS_PERSON_ONLY = "Personal and person-only: only the signed-in person can list or revoke their own assistant connections, in their browser on " + + "[Assistant connections](//apps/assistant-connection), with this conversation's company prefix as . " + + "An agent cannot do it for them: these routes accept only the person's own signed-in browser session, and the only other revocation is OAuth token revocation sent by the connected assistant itself with its own token. " + + "Do not search for another route; link that page and tell the person which connection to revoke. " + + "Revoking stops that assistant's further calls; tasks it already created keep their assignees and running work."; registerCurrentRoute({ method: "get", path: "/api/mcp/connections", tags: ["tool-gateway"], summary: "List the signed-in person's assistant connections", + description: ASSISTANT_CONNECTIONS_PERSON_ONLY, responses: { 200: r.ok(), 401: r.unauthorized }, }); registerCurrentRoute({ method: "delete", path: "/api/mcp/connections/{id}", tags: ["tool-gateway"], summary: "Revoke an assistant connection using a same-origin browser session", + description: ASSISTANT_CONNECTIONS_PERSON_ONLY, responses: { 204: { description: "Connection revoked" }, 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden }, }); diff --git a/server/src/services/native-runtime/runner-api-client.ts b/server/src/services/native-runtime/runner-api-client.ts index 2f051b7309b..42a1af9ad19 100644 --- a/server/src/services/native-runtime/runner-api-client.ts +++ b/server/src/services/native-runtime/runner-api-client.ts @@ -54,7 +54,7 @@ export function validateRunnerApiCall(value: unknown, context: RunnerApiContext) if (!parsed.success) throw badRequest("Invalid call_api arguments", { issues: parsed.error.issues }); const input = parsed.data; const operation = runnerApiOperation(input.operationId); - if (operation.transport !== "rest") throw unprocessable("This endpoint requires its existing protocol client; call_api supports REST only"); + if (operation.transport !== "rest") throw unprocessable(`This endpoint requires its existing protocol client; call_api supports REST only and no other API route substitutes for it.${operation.description.trim() ? " Its catalog description says who can use it." : ""}`); if (input.responseText && operation.method !== "GET") throw badRequest("responseText requires a GET operation; read the saved artifact instead of repeating a mutation"); if (!operation.allowedModes.includes(context.workMode)) throw forbidden("call_api permits only reads in Ask and Plan modes; use the permitted dedicated tools"); if (input.pathParams?.companyId && input.pathParams.companyId !== context.companyId) throw forbidden("API call belongs to another company");