diff --git a/server/src/__tests__/agent-refusal-finality.test.ts b/server/src/__tests__/agent-refusal-finality.test.ts new file mode 100644 index 00000000000..55e01273c5e --- /dev/null +++ b/server/src/__tests__/agent-refusal-finality.test.ts @@ -0,0 +1,94 @@ +import express from "express"; +import request from "supertest"; +import { describe, expect, it } from "vitest"; + +import { HttpError } from "../errors.js"; +import { errorHandler } from "../middleware/index.js"; +import { getAccessibleResource, issueNotFoundBody, ISSUE_NOT_VISIBLE_CODE } from "../routes/authz.js"; +import { buildOpenApiDocument } from "../routes/openapi.js"; +import { withMcpConnectionFailure } from "../services/mcp-connection-failure.js"; +import { unreachableServerRemediation } from "../services/tool-access/catalog/failures.js"; + +const COMPANY_ID = "10000000-0000-4000-8000-000000000001"; +const OTHER_COMPANY_ID = "10000000-0000-4000-8000-000000000009"; +const CONNECTION_ID = "30000000-0000-4000-8000-000000000003"; + +const agentActor: Express.Request["actor"] = { type: "agent", source: "agent_key", agentId: "agent-1", companyId: COMPANY_ID }; +const boardActor: Express.Request["actor"] = { type: "board", source: "local_implicit", userId: "local-board" }; + +function lookupApp(actor: Express.Request["actor"], issue: { companyId: string } | null) { + const app = express(); + app.use((req, _res, next) => { + req.actor = actor; + next(); + }); + app.get("/api/issues/:id", async (req, res) => { + const found = await getAccessibleResource(req, res, issue, "Issue not found", issueNotFoundBody(req)); + if (found) res.json(found); + }); + app.use(errorHandler); + return app; +} + +async function refusedConnection(code: string) { + return withMcpConnectionFailure(async () => { + throw new TypeError("fetch failed", { cause: Object.assign(new Error("connect failed"), { code }) }); + }).catch((error: unknown) => error); +} + +describe("agent refusals that are final", () => { + it("tells an agent a task lookup is final, with one body for missing and other-company tasks", async () => { + const missing = await request(lookupApp(agentActor, null)).get("/api/issues/COM-2").expect(404); + const otherCompany = await request(lookupApp(agentActor, { companyId: OTHER_COMPANY_ID })).get("/api/issues/OTH-1").expect(404); + expect(missing.body).toEqual(otherCompany.body); + expect(missing.body).toEqual(issueNotFoundBody({ actor: agentActor } as express.Request)); + expect(missing.body.error).toBe("Issue not found"); + expect(missing.body.code).toBe(ISSUE_NOT_VISIBLE_CODE); + expect(missing.body.remediation).toContain("this lookup is final"); + expect(missing.body.remediation).toContain("do not open a similarly named task in its place"); + expect(missing.body.remediation).toContain("Reply now"); + }); + + it("keeps the plain task 404 for people", async () => { + const res = await request(lookupApp(boardActor, null)).get("/api/issues/COM-2").expect(404); + expect(res.body).toEqual({ error: "Issue not found" }); + }); + + it("tells an agent an unreachable MCP server is the person's to fix, and not to probe other addresses", async () => { + const error = await refusedConnection("ECONNREFUSED"); + const { remediation } = unreachableServerRemediation(error, CONNECTION_ID, { actorType: "agent" }) as { remediation: string }; + expect(remediation).toContain("nothing accepted the connection"); + expect(remediation).toContain("they must correct the URL, or start the server"); + expect(remediation).toContain("you must not contact other hosts, ports or network addresses yourself"); + expect(remediation).toContain("Reply now"); + expect(remediation).toContain(`[Open the connection](//apps/${CONNECTION_ID}/permissions)`); + }); + + it("adds no MCP remediation for people or for failures that reached the server", async () => { + const error = await refusedConnection("ECONNREFUSED"); + expect(unreachableServerRemediation(error, CONNECTION_ID, { actorType: "user" })).toEqual({}); + expect(unreachableServerRemediation(new Error("tool list was invalid"), CONNECTION_ID, { actorType: "agent" })).toEqual({}); + }); + + it("returns the MCP remediation in the error body an agent receives", async () => { + const source = await refusedConnection("ENOTFOUND"); + const routed = express(); + routed.post("/check", () => { + throw new HttpError(502, "fetch failed", { code: "runtime_error", ...unreachableServerRemediation(source, CONNECTION_ID, { actorType: "agent" }) }); + }); + routed.use(errorHandler); + const res = await request(routed).post("/check").expect(502); + expect(res.body.code).toBe("runtime_error"); + expect(res.body.remediation).toContain("its host name did not resolve"); + }); + + it("points the API catalog at the checked MCP setup route and the complete model lists", () => { + const paths = buildOpenApiDocument().paths; + expect(paths["/api/companies/{companyId}/tools/connections"].post.description).toContain("POST /api/companies/{companyId}/tools/apps/connect with link"); + const models = paths["/api/companies/{companyId}/adapters/{type}/models"].get; + expect(models.description).toContain("type opencode_local and provider=openrouter"); + expect(models.description).toContain("These calls are the complete answer"); + const provider = models.parameters.find((row: { name: string }) => row.name === "provider"); + expect(provider.schema.enum).toContain("openrouter"); + }); +}); diff --git a/server/src/__tests__/board-access-denial.test.ts b/server/src/__tests__/board-access-denial.test.ts index 9518032f254..b8481f00127 100644 --- a/server/src/__tests__/board-access-denial.test.ts +++ b/server/src/__tests__/board-access-denial.test.ts @@ -127,6 +127,8 @@ describe("board-only denial", () => { expect(AGENT_CHAT_DIRECTIVE).toContain("so stop searching for one"); expect(AGENT_CHAT_DIRECTIVE).toContain("tell the person which control in the Paperclip web app to use, link its page when you know the route"); expect(AGENT_CHAT_DIRECTIVE).not.toContain("use permitted discovery to identify the supported operator path"); + expect(AGENT_CHAT_DIRECTIVE).toContain("Reply now instead of making more calls"); + expect(AGENT_CHAT_DIRECTIVE).not.toContain("Complete other permitted work"); }); it("says in the API catalog who sets up a Claude Managed Agent profile", () => { diff --git a/server/src/__tests__/issue-access-grants-routes.test.ts b/server/src/__tests__/issue-access-grants-routes.test.ts index b642460a120..c6e5dae4c73 100644 --- a/server/src/__tests__/issue-access-grants-routes.test.ts +++ b/server/src/__tests__/issue-access-grants-routes.test.ts @@ -298,6 +298,14 @@ describeEmbeddedPostgres("issue access grant and locked edge routes", { concurre )).get(`/api/issues/${fixture.privateChild.id}`); expect(deniedAfterRevoke.status).toBe(404); expect(deniedAfterRevoke.body.error).toBe("Issue not found"); + // A private task and a missing one must give an agent the same final answer. + const missing = await request(await createApp( + db, + agentActor(fixture.companyId, fixture.assignedAgent.id), + )).get(`/api/issues/${randomUUID()}`); + expect(missing.status).toBe(404); + expect(deniedAfterRevoke.body).toEqual(missing.body); + expect(deniedAfterRevoke.body.code).toBe("issue_not_visible"); }, 20_000); it("creates and enriches explicit grants idempotently while rejecting unauthorized and open shares", async () => { diff --git a/server/src/routes/authz.ts b/server/src/routes/authz.ts index 8b39815fc45..e744ceb15d3 100644 --- a/server/src/routes/authz.ts +++ b/server/src/routes/authz.ts @@ -108,6 +108,23 @@ export function boardAccessRequired(req: Request) { }); } +export const ISSUE_NOT_VISIBLE_CODE = "issue_not_visible"; + +/** + * The 404 for a task lookup. A deleted task, a private task the caller cannot + * see and another company's task get this same body, so it never says which. + * Agents also learn that the lookup is final, so they reply instead of + * searching other routes for the task. + */ +export function issueNotFoundBody(req: Request): Record { + if (req.actor.type !== "agent") return { error: "Issue not found" }; + return { + error: "Issue not found", + code: ISSUE_NOT_VISIBLE_CODE, + remediation: "No task with this ID or identifier is visible to you. A deleted task, a private task you were not given and a task in another company all get this same answer, so this lookup is final: do not search for the task through other routes, lists, activity, files or hosts, and do not open a similarly named task in its place. Reply now: tell the person you cannot open it here, and that if it still exists they can share it with you from its sharing controls or paste the parts they need.", + }; +} + export function assertBoard(req: Request) { if (req.actor.type !== "board") { throw boardAccessRequired(req); @@ -263,10 +280,11 @@ export async function getAccessibleResource( res: Response, resource: T | null | undefined | Promise, notFoundMessage: string, + notFoundBody: Record = { error: notFoundMessage }, ): Promise { const resolved = await resource; if (!resolved || !hasCompanyAccess(req, resolved.companyId)) { - res.status(404).json({ error: notFoundMessage }); + res.status(404).json(notFoundBody); return null; } assertCompanyAccess(req, resolved.companyId); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index e6b32c4847f..157e889068b 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -286,6 +286,7 @@ import { assertCompanyAccess, getAccessibleResource, getActorInfo, + issueNotFoundBody, } from "./authz.js"; import { assertNoAgentHostWorkspaceCommandMutation, @@ -5226,7 +5227,7 @@ export function issueRoutes( req: Request, res: Response, issue: Parameters[1] & { identifier?: string | null }, - options: { allowBreakGlass?: boolean } = {}, + options: { allowBreakGlass?: boolean; finalForAgents?: boolean } = {}, ) { const decision = await decideIssueAccess(req, issue, "issue:read"); if (decision.allowed) return true; @@ -5239,7 +5240,9 @@ export function issueRoutes( await recordIssueBreakGlassRead(req, issue); return true; } - res.status(404).json({ error: "Issue not found" }); + // Must match the missing-task body on the same route, or the 404 would + // reveal that a private task exists. + res.status(404).json(options.finalForAgents ? issueNotFoundBody(req) : { error: "Issue not found" }); return false; } @@ -9774,9 +9777,10 @@ export function issueRoutes( res, timing.time("lookup", () => getIssueById(req, id)), "Issue not found", + issueNotFoundBody(req), ); if (!issue) return; - if (!(await timing.time("authorization", () => assertIssueReadAllowed(req, res, issue, { allowBreakGlass: true })))) return; + if (!(await timing.time("authorization", () => assertIssueReadAllowed(req, res, issue, { allowBreakGlass: true, finalForAgents: true })))) return; // Saved cards are structural conversation data. Include their local // projection before first paint, independently of slow recovery, workspace // and related-task enrichment. This never refreshes external providers. diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 0ce9f81adcf..e6b5761491b 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -4125,11 +4125,12 @@ registry.registerPath({ path: "/api/companies/{companyId}/adapters/{type}/models", tags: ["adapters"], summary: "List models for an adapter type and runner provider", + description: "Answers which models an agent's runtime can select. For a paperclip_runner agent, pass its adapterConfig.provider and its defaultEnvironmentId; the response is the runtime's own list. An OpenCode agent whose model starts with openrouter/ uses an OpenRouter key: list those choices with type opencode_local and provider=openrouter. That list is OpenRouter's public catalog, so whether a model works depends on the key's account and is not a qualification. These calls are the complete answer: report the options with that caveat and reply, rather than enumerating providers through other routes, the shell or the web.", request: { params: z.object({ companyId: z.string(), type: z.string() }), query: z.object({ provider: z - .enum(["codex", "acpx", "opencode", "claude_managed", "aws_agentcore"]) + .enum(["codex", "acpx", "opencode", "claude_managed", "aws_agentcore", "openrouter"]) .optional(), environmentId: z.string().optional(), refresh: z.string().optional(), @@ -11816,6 +11817,7 @@ registerCurrentRoute({ path: "/api/companies/{companyId}/tools/connections", tags: ["tool-access"], summary: "Create a tool connection", + description: "Saves a connection as given without checking that its server answers. To connect an MCP server from its URL, use POST /api/companies/{companyId}/tools/apps/connect with link instead: it checks the server before saving and keeps an unreachable server as an editable draft.", body: createToolConnectionSchema, responses: { 201: r.ok(), diff --git a/server/src/services/agent-conversations.ts b/server/src/services/agent-conversations.ts index 2f81391892c..04d35092920 100644 --- a/server/src/services/agent-conversations.ts +++ b/server/src/services/agent-conversations.ts @@ -93,7 +93,7 @@ export const AGENT_CHAT_DIRECTIVE = `You are in an ongoing conversation with the When changing defaults or policy for new or future work, update the resource that owns those defaults. Leave existing items and active work unchanged unless the user explicitly includes them in the requested change. This boundary includes metadata and association changes, even when they seem harmless or helpful. Do not broaden the request to align related existing items. Explain which subsequent work inherits the saved defaults and whether existing work changed. -If an operation needs board or instance-operator identity that this execution lacks (its API catalog authorization actor is board, or a call returns 403 "Board access required"), that boundary is final: no agent credential, tool or other route can do it, so stop searching for one. Complete other permitted work, then tell the person which control in the Paperclip web app to use, link its page when you know the route (a denial's remediation may name it), and end your response. A user's request or confirmation does not expand your credential's scope. Treat permission denials as boundaries: do not probe host processes, private control files, or unrelated endpoints for alternate access. Distinguish a requested action from a verified action. +If an operation needs board or instance-operator identity that this execution lacks (its API catalog authorization actor is board, or a call returns 403 "Board access required"), that boundary is final: no agent credential, tool or other route can do it, so stop searching for one. Reply now instead of making more calls: say what you already finished, tell the person which control in the Paperclip web app to use, link its page when you know the route (a denial's remediation may name it), and end your response. Do not call other routes to look for a way around it. A user's request or confirmation does not expand your credential's scope. Treat permission denials as boundaries: do not probe host processes, private control files, or unrelated endpoints for alternate access. Distinguish a requested action from a verified action. Research, clarify, and develop full plans here using the conversation's plan document. Revise the draft as the discussion develops. Planning alone does not create execution tasks. Put implementation and substantial execution into separate tasks. diff --git a/server/src/services/mcp-connection-failure.ts b/server/src/services/mcp-connection-failure.ts index b3cbb816e0b..16e6413544a 100644 --- a/server/src/services/mcp-connection-failure.ts +++ b/server/src/services/mcp-connection-failure.ts @@ -61,3 +61,9 @@ export function retainMcpConnectionFailure(source: unknown, tar export function isExpectedMcpConnectionFailure(error: unknown): boolean { return error instanceof HttpError && error.status === 502 && !hasUnclassifiedRemoteConnectionAttempts(error) && failures.has(error); } + +/** Why an outbound MCP request did not reach its server, when that is known. */ +export function mcpConnectionFailureReason(error: unknown): Reason | null { + if (!(error instanceof Error) || hasUnclassifiedRemoteConnectionAttempts(error)) return null; + return failures.get(error)?.reason ?? readRemoteConnectionFailure(error); +} diff --git a/server/src/services/tool-access/catalog/failures.ts b/server/src/services/tool-access/catalog/failures.ts index ff1fe8a14bf..166ecd82eca 100644 --- a/server/src/services/tool-access/catalog/failures.ts +++ b/server/src/services/tool-access/catalog/failures.ts @@ -1,5 +1,7 @@ import type { ToolConnectionHealthStatus } from "@paperclipai/shared"; import { HttpError, unprocessable } from "../../../errors.js"; +import { mcpConnectionFailureReason } from "../../mcp-connection-failure.js"; +import type { ActorInfo } from "../types.js"; function asRecord(value: unknown): Record { if (value && typeof value === "object" && !Array.isArray(value)) @@ -153,3 +155,31 @@ export function sanitizeHttpFailure(error: unknown): { code: "runtime_error", }; } + +const UNREACHABLE_SERVER_REASONS: Readonly> = { + connection_refused: "nothing accepted the connection", + dns_failure: "its host name did not resolve", + network_unreachable: "its network address could not be reached", + connection_reset: "the connection was dropped", + connection_timeout: "it did not answer in time", + tls_failure: "its TLS certificate was not accepted", +}; + +/** + * For an agent, a check that could not reach the MCP server is final until the + * person fixes the URL or the server. Say so, so the agent hands it back instead + * of retrying or contacting other network addresses as a workaround. + */ +export function unreachableServerRemediation( + source: unknown, + connectionId: string, + actor?: Pick, +): { remediation: string } | Record { + if (actor?.actorType !== "agent") return {}; + const reason = mcpConnectionFailureReason(source); + const detail = reason ? UNREACHABLE_SERVER_REASONS[reason] : undefined; + if (!detail) return {}; + return { + remediation: `Paperclip could not reach this MCP server at its saved URL: ${detail}. This result is final until the person fixes it: they must correct the URL, or start the server or make it reachable from Paperclip, and then check it again. Retrying now will not connect it, and you must not contact other hosts, ports or network addresses yourself as a workaround. Reply now: tell the person the server could not be reached and what they need to fix, do not say it is connected, and link the connection as [Open the connection](//apps/${connectionId}/permissions), with this conversation's company prefix as .`, + }; +} diff --git a/server/src/services/tool-access/catalog/health.ts b/server/src/services/tool-access/catalog/health.ts index 9de76a44c64..cc8647e79ec 100644 --- a/server/src/services/tool-access/catalog/health.ts +++ b/server/src/services/tool-access/catalog/health.ts @@ -21,7 +21,7 @@ import type { ConnectionCredentialAccess } from "../credential-access.js"; import type { ConnectionSetupPolicy } from "../connections/setup-policy.js"; import { toConnection } from "../connections/projection.js"; import { isAgentMailConnection, type ToolCatalogDiscovery } from "./discovery.js"; -import { healthFailureHttpStatus, sanitizeHttpFailure, unsupportedToolConnectionTransport } from "./failures.js"; +import { healthFailureHttpStatus, sanitizeHttpFailure, unreachableServerRemediation, unsupportedToolConnectionTransport } from "./failures.js"; function asRecord(value: unknown): Record { if (value && typeof value === "object" && !Array.isArray(value)) return value as Record; @@ -226,6 +226,7 @@ export function createConnectionHealth(db: Db, { now, audit, getConnectionRow, d setupUrl: connectionSetupUrl(connection), reconnectUrl: connectionReconnectUrl(connection), ...githubRecovery, + ...unreachableServerRemediation(error, connection.id, actor), })); } } diff --git a/server/src/services/tool-access/catalog/publication.ts b/server/src/services/tool-access/catalog/publication.ts index cfb25f98386..20d97192f45 100644 --- a/server/src/services/tool-access/catalog/publication.ts +++ b/server/src/services/tool-access/catalog/publication.ts @@ -27,7 +27,7 @@ import { isAgentMailConnection, type ToolCatalogDiscovery } from "./discovery.js import { classifyRisk, shouldQuarantineNewEntries, isGoogleWorkspaceToolAllowed, type McpToolDescriptor } from "./descriptors.js"; import { toConnection } from "../connections/projection.js"; import { toCatalogEntry } from "./projection.js"; -import { healthFailureHttpStatus, sanitizeHttpFailure } from "./failures.js"; +import { healthFailureHttpStatus, sanitizeHttpFailure, unreachableServerRemediation } from "./failures.js"; function asRecord(value: unknown): Record { if (value && typeof value === "object" && !Array.isArray(value)) return value as Record; @@ -98,6 +98,7 @@ export function createToolCatalogPublication(db: Db, { now, audit, getConnection code: failure.code, setupUrl: connectionSetupUrl(connection), reconnectUrl: connectionReconnectUrl(connection), + ...unreachableServerRemediation(error, connection.id, actor), })); }