diff --git a/apps/ai/src/mcp/app.test.ts b/apps/ai/src/mcp/app.test.ts index 55148f672d..de253e4155 100644 --- a/apps/ai/src/mcp/app.test.ts +++ b/apps/ai/src/mcp/app.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it } from "@effect/vitest" import { OrgId, UserId } from "@maple/domain/http" +import { INVESTIGATION_READ_FAMILIES } from "@maple/domain/http/v2" import { ConfigProvider, Context, @@ -661,6 +662,77 @@ describe("MCP HTTP authorization", () => { }) }) +describe("MCP tools/list per credential", () => { + it("hides write tools from a read-only key, in the same isolate that shows them to a full key", async () => { + const db = createTestDb(createdDbs) + const base = Layer.mergeAll(db.layer, Env.layer.pipe(Layer.provide(testConfig()))) + const services = Layer.mergeAll( + ApiKeysService.layer, + ErrorActorsService.layer, + AuthService.layer, + makeMcpToolExecutorStubLayer(), + makeRateLimiterStubLayer(), + ).pipe(Layer.provideMerge(base)) + const orgId = Schema.decodeSync(OrgId)("org_test") + const userId = Schema.decodeSync(UserId)("user_test") + const [full, readOnly] = await Effect.runPromise( + Effect.gen(function* () { + const apiKeys = yield* ApiKeysService + return [ + yield* apiKeys.create(orgId, userId, { name: "Full key", kind: "mcp" }), + yield* apiKeys.create(orgId, userId, { + name: "Read-only key", + scopes: Arr.map(INVESTIGATION_READ_FAMILIES, (family) => `${family}:read`), + }), + ] as const + }).pipe(Effect.provide(services)), + ) + const { handler, dispose } = HttpRouter.toWebHandler(McpLive.pipe(Layer.provideMerge(services)), { + disableLogger: true, + }) + const listFor = async (secret: string) => { + const response = await handler( + new Request("https://api.example.com/mcp", { + method: "POST", + headers: { + authorization: `Bearer ${secret}`, + accept: "application/json, text/event-stream", + "content-type": "application/json", + host: "api.example.com", + "x-forwarded-proto": "https", + "mcp-protocol-version": "2025-06-18", + }, + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list" }), + }), + Context.empty(), + ) + const names = Arr.map((await readBody(response)).result?.tools ?? [], (tool) => tool.name) + return { + status: response.status, + write: names.includes("create_alert_rule"), + read: names.includes("inspect_trace"), + feedback: names.includes("send_maple_feedback"), + } + } + try { + expect(await listFor(full.secret)).toEqual({ + status: 200, + write: true, + read: true, + feedback: true, + }) + expect(await listFor(readOnly.secret)).toEqual({ + status: 200, + write: false, + read: true, + feedback: true, + }) + } finally { + await dispose() + } + }) +}) + const ToolCallBody = Schema.Struct({ id: Schema.Number, result: Schema.optionalKey( diff --git a/apps/ai/src/mcp/dispatcher.test.ts b/apps/ai/src/mcp/dispatcher.test.ts index b3cb3eed66..30f3bc169f 100644 --- a/apps/ai/src/mcp/dispatcher.test.ts +++ b/apps/ai/src/mcp/dispatcher.test.ts @@ -237,6 +237,39 @@ describe("MCP dispatcher", () => { ) expect(error._tag).toBe("@maple/mcp/ToolNotFoundError") expect(error.toolName).toBe("not_a_maple_tool") + expect(error.message).toContain("tools/list") + }), + ) + + it.effect("names the tools an unknown name most likely meant", () => + Effect.gen(function* () { + const executor = yield* makeValidationExecutor + const error = yield* Effect.flip( + executor.execute(TENANT, "uptime_monitors", {}, "mcp") as Effect.Effect< + never, + McpToolNotFoundError + >, + ) + expect(error.message).toContain("Did you mean `list_uptime_monitors`") + }), + ) + + // On /mcp the visibility gate hides these tools first; this is the registry's own refusal. + it.effect("answers a read-only key's write call as a permission problem, not a bug", () => + Effect.gen(function* () { + const executor = yield* makeValidationExecutor + const result = yield* executor.execute( + { ...TENANT, readOnly: true }, + "create_alert_rule", + {}, + "mcp", + ) + expect({ + isError: result.isError, + category: result.failureCategory, + lead: Str.startsWith("Permission denied:")(result.content[0]?.text ?? ""), + bugHint: Str.includes("send_maple_feedback")(result.content[0]?.text ?? ""), + }).toEqual({ isError: true, category: "permission", lead: true, bugHint: false }) }), ) @@ -410,6 +443,8 @@ describe("MCP dispatcher", () => { if (dispatchSpan.status._tag === "Ended") expect(dispatchSpan.status.exit._tag).toBe("Failure") expect(dispatchSpan.attributes.get("maple.mcp.error.category")).toBe("query") + expect(dispatchSpan.attributes.get("maple.mcp.error.message")).toBe("boom") + expect(result.content[0]?.text).toContain("report it now with `send_maple_feedback`") }), ) diff --git a/apps/ai/src/mcp/dispatcher.ts b/apps/ai/src/mcp/dispatcher.ts index 3713d385fc..bdfc7266e2 100644 --- a/apps/ai/src/mcp/dispatcher.ts +++ b/apps/ai/src/mcp/dispatcher.ts @@ -54,7 +54,7 @@ import * as Cloudflare from "@alchemy.run/cloudflare" // BOUNDARY: This module owns unparsed external values and narrows them before domain use. import type { McpToolNotFoundError } from "@maple/domain/mcp-tool-contract" import type { McpToolSurface } from "@maple/domain/mcp-manifest" -import { Context, Effect, Layer, Array as Arr } from "effect" +import { Context, Effect, Layer, Array as Arr, String as Str } from "effect" import { FetchHttpClient } from "effect/http" import type { McpToolResult } from "./tools/types" import type { McpToolRuntimeRequirements } from "./tools/runtime-requirements" @@ -76,12 +76,13 @@ const failureResult = (text: string, category: string): McpToolResult => ({ /** * Appended to failures that are likely Maple's fault, on the public transport only: an outside - * agent has the motive and the details to report it right when the call fails. Maple's own + * agent has the motive and the details to report it right when the call fails. "Offer the user" + * was read literally and no report was ever sent, so the agent sends it itself. Maple's own * agents (chat, bot) are not asked to report on Maple. */ export const withFeedbackHint = (text: string, name: string, surface: McpToolSurface): string => surface === "mcp" && name !== "send_maple_feedback" - ? `${text}\nIf this looks like a bug in Maple rather than in your call, offer the user to report it with \`send_maple_feedback\`.` + ? `${text}\nIf this looks like a bug in Maple rather than in your call, report it now with \`send_maple_feedback\` (kind "bug", related_to "${name}", what you called and what came back, no user data), then tell the user in one line that you did.` : text /** @@ -95,6 +96,9 @@ const REAL_FAILURE_CATEGORY = new Map([ ["@maple/mcp/errors/McpInvalidTenantError", "tenant"], ]) +/** Matches `expected-failures.ts`, so both kinds of failure carry the same message bound. */ +const MAX_SPAN_MESSAGE_CHARS = 500 + const dispatchInSpan = Effect.fn("McpToolDispatcher.call")(function* ( name: string, input: unknown, @@ -160,9 +164,14 @@ const dispatchInSpan = Effect.fn("McpToolDispatcher.call")(function* ( recordExpectedMcpFailure(error, "MCP authentication failed").pipe( Effect.as(failureResult(`Authentication required: ${error.message}`, "auth")), ), + // A credential that is valid but may not do this is a permission answer, not a bug. "@maple/mcp/errors/McpAuthInvalidError": (error) => recordExpectedMcpFailure(error, "MCP authentication failed").pipe( - Effect.as(failureResult(`Authentication failed: ${error.message}`, "auth")), + Effect.as( + error.reason === "insufficient_scope" + ? failureResult(`Permission denied: ${error.message}`, "permission") + : failureResult(`Authentication failed: ${error.message}`, "auth"), + ), ), }), // After the catchTags above, so a failure they converted into an in-band @@ -187,7 +196,12 @@ const dispatchInSpan = Effect.fn("McpToolDispatcher.call")(function* ( const category = REAL_FAILURE_CATEGORY.get(error._tag) return category === undefined ? Effect.void - : Effect.annotateCurrentSpan({ "result.isError": true, "maple.mcp.error.category": category }) + : Effect.annotateCurrentSpan({ + "result.isError": true, + "maple.mcp.error.category": category, + // As expected failures record it: a tool's failure reasons are one query, not a log join. + "maple.mcp.error.message": Str.slice(0, MAX_SPAN_MESSAGE_CHARS)(error.message), + }) }), Effect.annotateLogs({ "maple.mcp.tool": name }), ) diff --git a/apps/ai/src/mcp/lib/map-http-error.test.ts b/apps/ai/src/mcp/lib/map-http-error.test.ts new file mode 100644 index 0000000000..85f67ff82b --- /dev/null +++ b/apps/ai/src/mcp/lib/map-http-error.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from "@effect/vitest" +import { AlertForbiddenError, AlertPersistenceError, AlertValidationError } from "@maple/domain/http" +import { WarehouseQueryError } from "@maple/domain" +import { toMcpHttpError } from "./map-http-error" + +describe("toMcpHttpError", () => { + it("reads a 403 as a permission problem in public words, never the internal tag", () => { + const mapped = toMcpHttpError("create_alert_rule")( + new AlertForbiddenError({ message: "Only admins can create alert rules" }), + ) + expect({ tag: mapped._tag, reason: "reason" in mapped ? mapped.reason : undefined }).toEqual({ + tag: "@maple/mcp/errors/McpAuthInvalidError", + reason: "insufficient_scope", + }) + expect(mapped.message).not.toContain("@maple/") + }) + + it("reads a 400 as input the model fixes", () => { + const mapped = toMcpHttpError("create_alert_rule")( + new AlertValidationError({ message: "threshold must be positive", details: [] }), + ) + expect(mapped._tag).toBe("@maple/mcp/errors/McpInvalidInputError") + }) + + it("keeps a retryable 5xx a real failure, saying a retry may work", () => { + const mapped = toMcpHttpError("create_alert_rule")( + new AlertPersistenceError({ message: "connection reset" }), + ) + expect(mapped._tag).toBe("@maple/mcp/errors/McpQueryError") + expect(mapped.message).toContain("Retrying in about 5s may work.") + }) + + it("keeps a non-retryable 5xx a query failure, without the tag in its text", () => { + const mapped = toMcpHttpError("list_users")( + new WarehouseQueryError({ message: "boom", pipeName: "list_users" }), + ) + expect(mapped._tag).toBe("@maple/mcp/errors/McpQueryError") + expect(mapped.message).not.toContain("@maple/") + }) +}) diff --git a/apps/ai/src/mcp/lib/map-http-error.ts b/apps/ai/src/mcp/lib/map-http-error.ts index ccf1587073..eb08d524b3 100644 --- a/apps/ai/src/mcp/lib/map-http-error.ts +++ b/apps/ai/src/mcp/lib/map-http-error.ts @@ -1,12 +1,59 @@ import type { SelfDescribingHttpError } from "@maple/domain/http" -import { McpQueryError } from "../tools/types" +import { + McpAuthInvalidError, + McpInvalidInputError, + McpNotReadyError, + McpQueryError, + McpUnavailableError, +} from "../tools/types" -/** Adapt an HTTP-domain failure at the MCP protocol boundary without reclassifying its tag. */ +export type McpHttpError = + | McpAuthInvalidError + | McpInvalidInputError + | McpNotReadyError + | McpQueryError + | McpUnavailableError + +/** When a retryable error names no delay. */ +const DEFAULT_RETRY_SECONDS = 5 + +/** + * Adapt an HTTP-domain failure at the MCP boundary by its public category, so a 403 reads as a + * permission problem and a 400 as a fix to the call, not as a Maple bug. The text is the error's + * public message: internal tags never reach the model. Only 5xx stay `McpQueryError`. + */ export const toMcpHttpError = (pipeName: string) => - (error: SelfDescribingHttpError): McpQueryError => - new McpQueryError({ - message: `${error._tag}: ${error.error.message}`, - pipeName, - cause: error, - }) + (error: SelfDescribingHttpError): McpHttpError => { + const body = error.error + const message = body.message + switch (body.type) { + case "invalid_request_error": + case "not_found_error": + case "conflict_error": + return new McpInvalidInputError({ + message, + ...(body.param === undefined ? undefined : { parameter: body.param }), + }) + case "authentication_error": + return new McpAuthInvalidError({ message, reason: "invalid_credentials" }) + case "permission_error": + return new McpAuthInvalidError({ message, reason: "insufficient_scope" }) + case "payment_error": + return new McpUnavailableError({ message, capability: "plan" }) + case "rate_limit_error": + return new McpNotReadyError({ + message, + retryAfterSeconds: body.retry_after_seconds ?? DEFAULT_RETRY_SECONDS, + }) + // A 5xx stays a real failure (Error span, cause kept) even when a retry may help. + case "api_error": + return new McpQueryError({ + message: body.retryable + ? `${message} Retrying in about ${body.retry_after_seconds ?? DEFAULT_RETRY_SECONDS}s may work.` + : message, + pipeName, + cause: error, + }) + } + } diff --git a/apps/ai/src/mcp/lib/map-warehouse-error.test.ts b/apps/ai/src/mcp/lib/map-warehouse-error.test.ts index ff56febe66..ccda856c90 100644 --- a/apps/ai/src/mcp/lib/map-warehouse-error.test.ts +++ b/apps/ai/src/mcp/lib/map-warehouse-error.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from "vitest" -import { WarehouseQueryError, WarehouseResultDecodeError, WarehouseSchemaDriftError } from "@maple/domain" -import { toMcpQueryError } from "./map-warehouse-error" +import { + WarehouseConfigError, + WarehouseQueryError, + WarehouseResultDecodeError, + WarehouseSchemaDriftError, +} from "@maple/domain" +import { toMcpQueryError, toMcpReadError } from "./map-warehouse-error" describe("toMcpQueryError", () => { it("forwards plain query errors verbatim with the pipe label", () => { @@ -38,3 +43,23 @@ describe("toMcpQueryError", () => { expect(mcp.message).not.toContain("schema apply") }) }) + +describe("toMcpReadError", () => { + it("reads one of Maple's tables missing for the org as not ready", () => { + const err = new WarehouseConfigError({ + message: "Resource 'user_activity_hourly' not found", + pipeName: "list_users", + }) + const mcp = toMcpReadError("list_users")(err) + expect(mcp._tag).toBe("@maple/mcp/errors/McpNotReadyError") + expect(mcp.message).toContain("user_activity_hourly") + }) + + it("leaves a table Maple does not define as a query failure", () => { + const err = new WarehouseConfigError({ + message: "Resource 'spans' not found", + pipeName: "list_users", + }) + expect(toMcpReadError("list_users")(err)._tag).toBe("@maple/mcp/errors/McpQueryError") + }) +}) diff --git a/apps/ai/src/mcp/lib/map-warehouse-error.ts b/apps/ai/src/mcp/lib/map-warehouse-error.ts index cd825d6f5a..057b7c3122 100644 --- a/apps/ai/src/mcp/lib/map-warehouse-error.ts +++ b/apps/ai/src/mcp/lib/map-warehouse-error.ts @@ -4,7 +4,9 @@ import { warehouseHandlers, warehouseReadHandlers, } from "@maple/backend/services/warehouse/warehouse-error-handlers" -import { McpQueryBudgetError, McpQueryError } from "../tools/types" +import { missingWarehouseTable } from "@maple/backend/services/warehouse/missing-table" +import { describeWarehouseTable } from "@maple/backend/services/warehouse/warehouse-catalog" +import { McpNotReadyError, McpQueryBudgetError, McpQueryError } from "../tools/types" export { warehouseHandlers, warehouseReadHandlers } @@ -51,15 +53,52 @@ export const toMcpQueryError = }) : new McpQueryError({ message: warehouseErrorText(error), pipeName: pipe, cause: error }) +/** + * The same, for SQL Maple wrote: one of Maple's own tables not deployed for the org yet (a deploy + * still rolling out, an unapplied BYO schema) is not the call's fault and not a bug, so it reads + * as not ready. Caller SQL keeps {@link toMcpQueryError}: a table it names may just be a typo. + */ +export const toMcpReadError = + (pipe: string) => + (error: WarehouseError): McpQueryError | McpQueryBudgetError | McpNotReadyError => { + const table = missingWarehouseTable(error) + return table === undefined || describeWarehouseTable(table) === null + ? toMcpQueryError(pipe)(error) + : new McpNotReadyError({ + message: missingTableText(table), + retryAfterSeconds: MISSING_TABLE_RETRY_SECONDS, + }) + } + +/** A missing table stays visible in logs: if it never clears, a misrouted token may be the cause. */ +const failRead = ( + pipe: string, + error: WarehouseError, +): Effect.Effect => { + const mapped = toMcpReadError(pipe)(error) + return mapped._tag === "@maple/mcp/errors/McpNotReadyError" + ? Effect.logWarning("Maple warehouse table missing for org").pipe( + Effect.annotateLogs({ "maple.mcp.pipe": pipe, "error.message": error.message }), + Effect.andThen(Effect.fail(mapped)), + ) + : Effect.fail(mapped) +} + +/** Long enough for a deploy to finish creating the table. */ +const MISSING_TABLE_RETRY_SECONDS = 120 + +const missingTableText = (table: string): string => + `The \`${table}\` table this tool reads is not deployed for this org yet (usually a deploy still rolling out). ` + + "If the org runs its own ClickHouse, an admin applies the schema from the ClickHouse settings page." + /** * `Effect.catchTags` handler map that converts every warehouse error tag into an - * `McpQueryError` (with the schema-drift hint), leaving any non-warehouse errors + * MCP error ({@link toMcpReadError}: the schema-drift hint, a missing table as not ready), leaving any non-warehouse errors * (e.g. the MCP auth errors from `CurrentMcpTenant`) untouched. Apply inline so the * residual error channel infers from the caught tags: * `effect.pipe(Effect.catchTags(warehouseToMcpHandlers("pipe_label")))`. */ -export const warehouseToMcpHandlers = (pipe: string) => - warehouseHandlers((error) => Effect.fail(toMcpQueryError(pipe)(error))) +export const warehouseToMcpHandlers = (pipe: string) => warehouseHandlers((error) => failRead(pipe, error)) /** * The same, for reads that compile their own SQL (the AI agent-session and @@ -68,4 +107,4 @@ export const warehouseToMcpHandlers = (pipe: string) => * {@link warehouseToMcpHandlers} does not typecheck against them. */ export const warehouseReadToMcpHandlers = (pipe: string) => - warehouseReadHandlers((error) => Effect.fail(toMcpQueryError(pipe)(error))) + warehouseReadHandlers((error) => failRead(pipe, error)) diff --git a/apps/ai/src/mcp/lib/slos.ts b/apps/ai/src/mcp/lib/slos.ts index 0f5296029a..237c43f974 100644 --- a/apps/ai/src/mcp/lib/slos.ts +++ b/apps/ai/src/mcp/lib/slos.ts @@ -18,7 +18,8 @@ import { } from "@maple/domain/slo" import type { SloNotFoundError, SloPersistenceError, SloValidationError } from "@maple/domain/http" import { SlosService, type SloRecord, type SloStatus } from "@maple/backend/services/slos/SlosService" -import { McpInvalidInputError, McpQueryError } from "../tools/types" +import { McpInvalidInputError } from "../tools/types" +import { toMcpHttpError } from "./map-http-error" import { doc, type NextCall } from "./tool-doc" export const SLI_KINDS = ["availability", "latency"] as const @@ -155,11 +156,7 @@ export const toMcpSloError = (tool: string) => (error: SloServiceError) => { parameter: "slo", }) case "@maple/http/errors/SloPersistenceError": - return new McpQueryError({ - message: `${error._tag}: ${error.message}`, - pipeName: tool, - cause: error, - }) + return toMcpHttpError(tool)(error) } } diff --git a/apps/ai/src/mcp/lib/tool-suggestions.test.ts b/apps/ai/src/mcp/lib/tool-suggestions.test.ts new file mode 100644 index 0000000000..34b0803b28 --- /dev/null +++ b/apps/ai/src/mcp/lib/tool-suggestions.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from "@effect/vitest" +import { suggestToolNames, unknownToolMessage } from "./tool-suggestions" + +const TOOLS = [ + "list_uptime_monitors", + "create_uptime_monitor", + "list_alert_incidents", + "acknowledge_alert_incident", + "get_incident_timeline", + "search_traces", + "inspect_trace", + "list_alert_rules", +] + +describe("suggestToolNames", () => { + // Names agents sent in production that matched no tool. + it("finds the tool behind a name missing its verb", () => { + expect(suggestToolNames("uptime_monitors", TOOLS)[0]).toBe("list_uptime_monitors") + }) + + it("reads a REST path as the tool for the same resource", () => { + expect(suggestToolNames("GET /v2/uptime_monitors", TOOLS)[0]).toBe("list_uptime_monitors") + }) + + it("offers the incident tools for a singular incident getter", () => { + expect(suggestToolNames("get_alert_incident", TOOLS)).toEqual( + expect.arrayContaining(["list_alert_incidents", "acknowledge_alert_incident"]), + ) + }) + + it("catches a misspelling", () => { + expect(suggestToolNames("serch_traces", TOOLS)[0]).toBe("search_traces") + }) + + it("suggests nothing for a CLI flag", () => { + expect(suggestToolNames("--help", TOOLS)).toEqual([]) + }) +}) + +describe("unknownToolMessage", () => { + it("always says where the full list is", () => { + expect(unknownToolMessage("--help", TOOLS)).toBe( + "Unknown tool `--help`. Call tools/list to see the available tools.", + ) + expect(unknownToolMessage("uptime_monitors", TOOLS)).toContain("tools/list has every tool") + }) +}) diff --git a/apps/ai/src/mcp/lib/tool-suggestions.ts b/apps/ai/src/mcp/lib/tool-suggestions.ts new file mode 100644 index 0000000000..018659dbeb --- /dev/null +++ b/apps/ai/src/mcp/lib/tool-suggestions.ts @@ -0,0 +1,65 @@ +import { Array as Arr, Order, String as Str } from "effect" + +/** Verbs every other tool starts with: sharing one says nothing about which tool was meant. */ +const VERBS: ReadonlySet = new Set(["get", "list", "find", "search", "show", "fetch", "v2"]) + +const MAX_SUGGESTIONS = 3 + +/** `GET /v2/uptime_monitors` -> `["uptime", "monitor"]`: lower-case words, plurals folded. */ +const words = (name: string): ReadonlyArray => + Arr.map( + Arr.filter(Str.split(/[^a-z0-9]+/)(Str.toLowerCase(name)), (word) => word.length > 1), + (word) => (word.length > 3 && Str.endsWith("s")(word) ? Str.slice(0, -1)(word) : word), + ) + +const editDistance = (a: string, b: string): number => { + const previous = Arr.makeBy(b.length + 1, (index) => index) + for (let i = 1; i <= a.length; i++) { + let diagonal = previous[0] ?? 0 + previous[0] = i + for (let j = 1; j <= b.length; j++) { + const above = previous[j] ?? 0 + const left = previous[j - 1] ?? 0 + previous[j] = Math.min(above + 1, left + 1, diagonal + (a[i - 1] === b[j - 1] ? 0 : 1)) + diagonal = above + } + } + return previous[b.length] ?? 0 +} + +interface Scored { + readonly name: string + readonly shared: number + readonly distance: number +} + +const byRelevance = Order.combine( + Order.mapInput(Order.flip(Order.Number), (scored: Scored) => scored.shared), + Order.mapInput(Order.Number, (scored: Scored) => scored.distance), +) + +/** + * The tools a caller most likely meant by a name that does not exist: shared words first + * (`uptime_monitors` -> `list_uptime_monitors`), then close spellings (`serch_traces`). + */ +export const suggestToolNames = (name: string, available: ReadonlyArray): ReadonlyArray => { + const asked = Arr.filter(words(name), (word) => !VERBS.has(word)) + const lower = Str.toLowerCase(name) + const scored = Arr.flatMap(available, (candidate): ReadonlyArray => { + const candidateWords = words(candidate) + const shared = Arr.filter(asked, (word) => candidateWords.includes(word)).length + const distance = editDistance(lower, candidate) + const close = distance <= Math.max(2, Math.floor(candidate.length / 4)) + return shared > 0 || close ? [{ name: candidate, shared, distance }] : [] + }) + return Arr.map(Arr.take(Arr.sort(scored, byRelevance), MAX_SUGGESTIONS), (scored) => scored.name) +} + +/** What a model reads for an unknown tool name: the likely tools, and where the full list is. */ +export const unknownToolMessage = (name: string, available: ReadonlyArray): string => { + const suggestions = suggestToolNames(name, available) + const listed = Arr.map(suggestions, (suggestion) => `\`${suggestion}\``) + return suggestions.length === 0 + ? `Unknown tool \`${name}\`. Call tools/list to see the available tools.` + : `Unknown tool \`${name}\`. Did you mean ${Arr.join(listed, ", ")}? tools/list has every tool.` +} diff --git a/apps/ai/src/mcp/lib/uptime-monitors.ts b/apps/ai/src/mcp/lib/uptime-monitors.ts index 4087d71a2a..20138125b5 100644 --- a/apps/ai/src/mcp/lib/uptime-monitors.ts +++ b/apps/ai/src/mcp/lib/uptime-monitors.ts @@ -119,11 +119,7 @@ export const toMcpUptimeError = (tool: string) => (error: UptimeServiceError) => capability: "uptime_monitor_allowance", }) case "@maple/http/errors/UptimeMonitorPersistenceError": - return new McpQueryError({ - message: `${error._tag}: ${error.message}`, - pipeName: tool, - cause: error, - }) + return new McpQueryError({ message: error.message, pipeName: tool, cause: error }) } } diff --git a/apps/ai/src/mcp/server-instructions.ts b/apps/ai/src/mcp/server-instructions.ts index efb3654418..eb2fe5bf08 100644 --- a/apps/ai/src/mcp/server-instructions.ts +++ b/apps/ai/src/mcp/server-instructions.ts @@ -6,7 +6,7 @@ import { Array as Arr } from "effect" export const MAPLE_MCP_SERVER_INSTRUCTIONS = Arr.join( [ "Maple is an OpenTelemetry observability platform. These tools read and manage the user's traces, logs, metrics, errors, alerts and dashboards. The `maple://instructions` resource has the detailed usage guide.", - "If Maple itself gets in your way (a tool errors in a way that looks like Maple's fault, returns something wrong, is missing a capability you needed, or its description misled you), offer the user to report it with `send_maple_feedback`: show them a one-line summary and send it once they agree, or right away if their instructions already allow it. Describe Maple's behaviour, not the user's data.", + "If Maple itself gets in your way (a tool errors in a way that looks like Maple's fault, returns something wrong, is missing a capability you needed, or its description misled you), report it with `send_maple_feedback` yourself, unless the user's instructions say not to, then tell the user in one line. The report is visible to the Maple team and the user's organization: describe Maple's behaviour, not the user's data.", ], "\n\n", ) diff --git a/apps/ai/src/mcp/server.ts b/apps/ai/src/mcp/server.ts index a8add694f6..d757e8465b 100644 --- a/apps/ai/src/mcp/server.ts +++ b/apps/ai/src/mcp/server.ts @@ -3,7 +3,7 @@ import { Context, Effect, Layer, Array as Arr, String as Str } from "effect" import { cachedRecoverable } from "@maple/infra/cached-recoverable" import { McpToolExecutorLoader, McpToolsUnavailableError } from "./executor-loader" import { CurrentMcpRequestTenant } from "./lib/query-warehouse" -import type { McpToolResult } from "./tools/types" +import { refusesReadOnlyCredential, type McpToolResult } from "./tools/types" import type { McpToolDescriptor } from "@maple/domain/mcp-tool-contract" /** @@ -64,6 +64,17 @@ const annotateToolFailure = (result: McpToolResult) => }) : Effect.void +/** + * Hides a tool from a read-only credential's `tools/list`, and answers a call to it as unknown: + * the registry would refuse every such call anyway, and a listed tool invites it. + */ +const HiddenFromReadOnlyCredentials = EffectMcpServer.visibility(() => + Effect.gen(function* () { + const tenant = yield* CurrentMcpRequestTenant + return tenant?.readOnly !== true + }), +) + const toBoundaryErrorResult = (error: { readonly _tag: string; readonly message: string }) => toCallToolResult({ isError: true, @@ -87,8 +98,12 @@ export class McpToolRegistration extends Context.Service< make: Effect.gen(function* () { const server = yield* EffectMcpServer.McpServer const loader = yield* McpToolExecutorLoader + const readOnlyGate = yield* Layer.build(HiddenFromReadOnlyCredentials.layer) const addTool = (descriptor: McpToolDescriptor) => server.addTool({ + ...(refusesReadOnlyCredential(descriptor.name, descriptor.annotations?.readOnlyHint === true) + ? { services: readOnlyGate } + : undefined), tool: new McpSchema.Tool({ name: descriptor.name, title: descriptor.title, @@ -112,6 +127,10 @@ export class McpToolRegistration extends Context.Service< }) } + // The hub tells clients apart by this; the stateless transport issues no session id. + if (tenant.mcpClientName !== undefined) { + yield* Effect.annotateCurrentSpan("maple.mcp.client.name", tenant.mcpClientName) + } // Already built by the transport before the call; this reads the cached executor. const executor = yield* loader.load return yield* executor diff --git a/apps/ai/src/mcp/tool-descriptors.generated.ts b/apps/ai/src/mcp/tool-descriptors.generated.ts index 04c0320ec5..3b17257b8a 100644 --- a/apps/ai/src/mcp/tool-descriptors.generated.ts +++ b/apps/ai/src/mcp/tool-descriptors.generated.ts @@ -124,7 +124,7 @@ export const MCP_TOOL_DESCRIPTORS: ReadonlyArray = [ {"name":"link_pull_request","title":"Link Pull Request","description":"Attach a GitHub pull request to an error issue without proposing new work. When it merges, Maple opens a verification window sized by the issue's severity and occurrence rate, then closes the issue if the error stopped. Leave the issue alone after linking: the merge moves it to `verifying` and the verdict moves it on. propose_fix does the same link when given `pr_url`, and also moves the issue to `in_review`.","inputSchema":{"type":"object","properties":{"issue_id":{"type":"string","description":"The error issue ID from list_error_issues (a fingerprint from find_errors is not an issue id): its UUID or its REST id (iss_\u2026)"},"pull_request_url":{"type":"string","description":"Full GitHub pull request URL, e.g. https://github.com/owner/repo/pull/123"}},"required":["issue_id","pull_request_url"],"additionalProperties":false},"outputSchema":{"type":"object","properties":{"pullRequestId":{"type":"string"},"issueId":{"type":"string"},"repoFullName":{"type":"string"},"number":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"url":{"type":"string"},"state":{"type":"string","enum":["open","merged","closed"]}},"required":["pullRequestId","issueId","repoFullName","number","url","state"],"additionalProperties":true},"annotations":{"title":"Link Pull Request","readOnlyHint":false,"destructiveHint":false,"idempotentHint":true,"openWorldHint":true}}, {"name":"list_error_issue_events","title":"List Error Issue Events","description":"List the audit-log events for an issue (state transitions, claims, comments, agent notes, fix proposals) in reverse-chronological order.","inputSchema":{"type":"object","properties":{"issue_id":{"type":"string","description":"The error issue ID from list_error_issues (a fingerprint from find_errors is not an issue id): its UUID or its REST id (iss_\u2026)"},"limit":{"type":"number","description":"Max events to return (default 100, max 500)"}},"required":["issue_id"],"additionalProperties":false},"outputSchema":{"type":"object","properties":{"issueId":{"type":"string"},"events":{"type":"array","items":{"type":"object","properties":{"id":{"type":"string"},"type":{"type":"string"},"fromState":{"anyOf":[{"type":"string"},{"type":"null"}]},"toState":{"anyOf":[{"type":"string"},{"type":"null"}]},"actorId":{"anyOf":[{"type":"string"},{"type":"null"}]},"createdAt":{"type":"string"},"payload":{"type":"object"},"actor":{"type":"string"}},"required":["id","type","fromState","toState","actorId","createdAt","payload","actor"],"additionalProperties":true}},"total":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"limit":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]}},"required":["issueId","events","total","limit"],"additionalProperties":true},"annotations":{"title":"List Error Issue Events","readOnlyHint":true,"destructiveHint":false,"idempotentHint":true,"openWorldHint":false}}, {"name":"register_agent","title":"Register Agent","description":"Register an LLM agent with the error-issue system so it can claim and transition issues. Must be called from a human session (not an agent API key). Returns an actor ID to pin via API-key metadata.","inputSchema":{"type":"object","properties":{"name":{"type":"string","description":"Unique agent name within the org (1..100 chars)"},"model":{"type":"string","description":"Model identifier, e.g. 'claude-opus-4.7'"},"capabilities_json":{"type":"string","contentMediaType":"application/json","contentSchema":{"type":"array","items":{"type":"string"}},"description":"JSON array of capability tags, e.g. [\"auto-triage\",\"patch-proposer\"]"}},"required":["name"],"additionalProperties":false},"outputSchema":{"type":"object","properties":{"id":{"type":"string"},"agentName":{"anyOf":[{"type":"string"},{"type":"null"}]},"model":{"anyOf":[{"type":"string"},{"type":"null"}]},"capabilities":{"type":"array","items":{"type":"string"}}},"required":["id","agentName","model","capabilities"],"additionalProperties":true},"annotations":{"title":"Register Agent","readOnlyHint":false,"destructiveHint":false,"idempotentHint":false,"openWorldHint":false}}, - {"name":"send_maple_feedback","title":"Send Maple Feedback","description":"Send feedback about Maple itself (this MCP server, its tools, API, or docs) to the Maple team: a bug you hit, a tool or capability you were missing, a misleading description, or something that worked well. Use it when Maple got in the way of your task, not for problems in the user's own services. The report goes to the Maple team and is visible to the user's organization, so ask the user first with a one-line summary, unless their instructions already allow sending Maple feedback. Describe Maple's behaviour, not the user's data: leave out secrets, customer data, and raw log or span contents.","inputSchema":{"type":"object","properties":{"kind":{"type":"string","enum":["bug","feature_request","improvement","documentation","praise","other"],"description":"bug (broken or wrong), feature_request (missing capability), improvement (works but could be better), documentation (a description or doc misled you), praise, or other"},"summary":{"type":"string","description":"One line: what happened or what you want (max 200 chars)"},"reason":{"type":"string","description":"Why you are sending this: what you were trying to do, what got in the way, and why it matters (max 4000 chars)"},"details":{"type":"string","description":"Reproduction steps, the call you made, expected versus actual (max 10000 chars)"},"impact":{"type":"string","enum":["blocking","degraded","minor"],"description":"blocking (could not finish), degraded (finished with a workaround), or minor"},"related_to":{"type":"string","description":"The tool, API endpoint, or page this is about, e.g. 'search_traces'"},"agent_type":{"type":"string","enum":["coding_agent","chat_assistant","autonomous_agent","ci","other"],"description":"coding_agent (Claude Code, Cursor, Codex), chat_assistant, autonomous_agent, ci, or other"},"agent_name":{"type":"string","description":"Your client or agent name, e.g. 'claude-code'"},"model":{"type":"string","description":"The model you run on, e.g. 'claude-opus-5-5'"},"agent_version":{"type":"string","description":"Your client or agent version"}},"required":["kind","summary","reason","agent_type"],"additionalProperties":false},"outputSchema":{"type":"object","properties":{"id":{"type":"string"},"kind":{"type":"string"},"impact":{"anyOf":[{"type":"string"},{"type":"null"}]},"summary":{"type":"string"},"agentType":{"type":"string"},"agentName":{"anyOf":[{"type":"string"},{"type":"null"}]},"agentModel":{"anyOf":[{"type":"string"},{"type":"null"}]},"createdAt":{"type":"string"}},"required":["id","kind","impact","summary","agentType","agentName","agentModel","createdAt"],"additionalProperties":true},"annotations":{"title":"Send Maple Feedback","readOnlyHint":false,"destructiveHint":false,"idempotentHint":false,"openWorldHint":false}}, + {"name":"send_maple_feedback","title":"Send Maple Feedback","description":"Report a problem with Maple itself (this MCP server, its tools, API, or docs) to the Maple team: a bug, a missing capability, a misleading description, or something that worked well. Send it yourself, without asking first, when a Maple tool fails in a way that looks like Maple's fault or Maple got in the way of the task; skip it only if the user's instructions say not to send Maple feedback. Not for problems in the user's own services. The report is visible to the Maple team and to the user's organization, so tell the user in one line that you sent it. Describe Maple's behaviour, not the user's data: leave out secrets, customer data, and raw log or span contents.","inputSchema":{"type":"object","properties":{"kind":{"type":"string","enum":["bug","feature_request","improvement","documentation","praise","other"],"description":"bug (broken or wrong), feature_request (missing capability), improvement (works but could be better), documentation (a description or doc misled you), praise, or other"},"summary":{"type":"string","description":"One line: what happened or what you want (max 200 chars)"},"reason":{"type":"string","description":"Why you are sending this: what you were trying to do, what got in the way, and why it matters (max 4000 chars)"},"details":{"type":"string","description":"Reproduction steps, the call you made, expected versus actual (max 10000 chars)"},"impact":{"type":"string","enum":["blocking","degraded","minor"],"description":"blocking (could not finish), degraded (finished with a workaround), or minor"},"related_to":{"type":"string","description":"The tool, API endpoint, or page this is about, e.g. 'search_traces'"},"agent_type":{"type":"string","enum":["coding_agent","chat_assistant","autonomous_agent","ci","other"],"description":"coding_agent (Claude Code, Cursor, Codex), chat_assistant, autonomous_agent, ci, or other (the default)"},"agent_name":{"type":"string","description":"Your client or agent name, e.g. 'claude-code'. Default: the name your client sent when it connected"},"model":{"type":"string","description":"The model you run on, e.g. 'claude-opus-5-5'"},"agent_version":{"type":"string","description":"Your client or agent version"}},"required":["kind","summary","reason"],"additionalProperties":false},"outputSchema":{"type":"object","properties":{"id":{"type":"string"},"kind":{"type":"string"},"impact":{"anyOf":[{"type":"string"},{"type":"null"}]},"summary":{"type":"string"},"agentType":{"type":"string"},"agentName":{"anyOf":[{"type":"string"},{"type":"null"}]},"agentModel":{"anyOf":[{"type":"string"},{"type":"null"}]},"createdAt":{"type":"string"}},"required":["id","kind","impact","summary","agentType","agentName","agentModel","createdAt"],"additionalProperties":true},"annotations":{"title":"Send Maple Feedback","readOnlyHint":false,"destructiveHint":false,"idempotentHint":false,"openWorldHint":false}}, {"name":"list_error_incidents","title":"List Error Incidents","description":"Error incidents: flare-ups of an error issue (alert-rule breaches are list_alert_incidents). An issue gets a 'first_seen' incident when it opens and a 'regression' incident when it fires after being resolved; incidents auto-resolve after ~30 min of silence. No time window: without issue_id or issue_ids, every currently open incident in the org; with them, those issues' incidents, open and resolved, newest first. Polling many issues? Pass them all as issue_ids in one call.","inputSchema":{"type":"object","properties":{"issue_id":{"type":"string","description":"Only this issue's incidents, resolved ones included. Omit for open incidents org-wide.: its UUID or its REST id (iss_\u2026)"},"issue_ids":{"type":"array","items":{"type":"string"},"description":"Only these issues' incidents, resolved ones included, in one read (at most 100). Each is its UUID or its REST id (iss_\u2026)"},"limit":{"type":"number","description":"Max incidents to return (default 100, max 500)"}},"additionalProperties":false},"outputSchema":{"type":"object","properties":{"incidents":{"type":"array","items":{"type":"object","properties":{"id":{"type":"string"},"issueId":{"type":"string"},"status":{"type":"string"},"reason":{"type":"string"},"firstTriggeredAt":{"type":"string"},"lastTriggeredAt":{"type":"string"},"resolvedAt":{"anyOf":[{"type":"string"},{"type":"null"}]},"occurrenceCount":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]}},"required":["id","issueId","status","reason","firstTriggeredAt","lastTriggeredAt","resolvedAt","occurrenceCount"],"additionalProperties":true}},"total":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"openCount":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"issueId":{"type":"string"},"issueIds":{"type":"array","items":{"type":"string"}},"issuesWithoutIncidents":{"type":"array","items":{"type":"string"}},"limit":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"hasMore":{"type":"boolean"}},"required":["incidents","total","openCount"],"additionalProperties":true},"annotations":{"title":"List Error Incidents","readOnlyHint":true,"destructiveHint":false,"idempotentHint":true,"openWorldHint":false}}, {"name":"get_error_notification_policy","title":"Get Error Notification Policy","description":"Read the org-wide error notification policy: whether it is on, which alert destinations it delivers to (name, UUID and REST id), which events notify, min_occurrence_count and the occurrence count a new error's first notification actually waits for. update_error_notification_policy changes it.","inputSchema":{"type":"object","properties":{},"additionalProperties":false},"outputSchema":{"type":"object","properties":{"enabled":{"type":"boolean"},"destinationIds":{"type":"array","items":{"type":"string"}},"destinationNamesUnavailable":{"type":"boolean"},"destinations":{"type":"array","items":{"type":"object","properties":{"id":{"type":"string"},"publicId":{"type":"string"},"name":{"anyOf":[{"type":"string"},{"type":"null"}]},"type":{"anyOf":[{"type":"string"},{"type":"null"}]},"enabled":{"type":"boolean"}},"required":["id","publicId","name","type","enabled"],"additionalProperties":true}},"notifyOnFirstSeen":{"type":"boolean"},"notifyOnRegression":{"type":"boolean"},"notifyOnResolve":{"type":"boolean"},"minOccurrenceCount":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"firstNotificationThreshold":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"severity":{"type":"string"},"updatedAt":{"type":"string"},"updatedBy":{"type":"string"}},"required":["enabled","destinationIds","destinations","notifyOnFirstSeen","notifyOnRegression","notifyOnResolve","minOccurrenceCount","firstNotificationThreshold","severity","updatedAt","updatedBy"],"additionalProperties":true},"annotations":{"title":"Get Error Notification Policy","readOnlyHint":true,"destructiveHint":false,"idempotentHint":true,"openWorldHint":false}}, {"name":"update_error_notification_policy","title":"Update Error Notification Policy","description":"Configure the org-wide error notification policy: whether error incidents (first seen, regression, auto-resolve) are delivered to alert destinations, and which ones. Org admins and engineers only. Omit a field to leave it unchanged; get_error_notification_policy reads the current policy. A new error becomes an issue (and can notify) only after 3 occurrences; min_occurrence_count cannot lower that.","inputSchema":{"type":"object","properties":{"enabled":{"type":"boolean","description":"Enable notifications overall"},"destination_ids":{"type":"array","items":{"type":"string"},"description":"Alert destinations (from list_alert_destinations) to deliver to, replacing the current ones. Pass [] to clear them. Each is its UUID or its REST id (dest_\u2026)"},"notify_on_first_seen":{"type":"boolean","description":"Notify when a new error becomes an issue (after its 3rd occurrence)"},"notify_on_regression":{"type":"boolean","description":"Notify when a resolved issue re-occurs"},"notify_on_resolve":{"type":"boolean","description":"Notify when an incident is auto-resolved after the silence window"},"min_occurrence_count":{"type":"number","description":"Only notify when an incident opens with at least this many occurrences, checked once when it opens. New errors already need 3 to become an issue, so values of 3 or less only affect regressions"},"severity":{"type":"string","enum":["warning","critical"],"description":"Severity label carried on the notifications"}},"additionalProperties":false},"outputSchema":{"type":"object","properties":{"enabled":{"type":"boolean"},"destinationIds":{"type":"array","items":{"type":"string"}},"destinationNamesUnavailable":{"type":"boolean"},"destinations":{"type":"array","items":{"type":"object","properties":{"id":{"type":"string"},"publicId":{"type":"string"},"name":{"anyOf":[{"type":"string"},{"type":"null"}]},"type":{"anyOf":[{"type":"string"},{"type":"null"}]},"enabled":{"type":"boolean"}},"required":["id","publicId","name","type","enabled"],"additionalProperties":true}},"notifyOnFirstSeen":{"type":"boolean"},"notifyOnRegression":{"type":"boolean"},"notifyOnResolve":{"type":"boolean"},"minOccurrenceCount":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"firstNotificationThreshold":{"anyOf":[{"type":"number"},{"type":"string","enum":["Infinity","-Infinity","NaN"]}]},"severity":{"type":"string"},"updatedAt":{"type":"string"},"updatedBy":{"type":"string"}},"required":["enabled","destinationIds","destinations","notifyOnFirstSeen","notifyOnRegression","notifyOnResolve","minOccurrenceCount","firstNotificationThreshold","severity","updatedAt","updatedBy"],"additionalProperties":true},"annotations":{"title":"Update Error Notification Policy","readOnlyHint":false,"destructiveHint":false,"idempotentHint":true,"openWorldHint":false}}, diff --git a/apps/ai/src/mcp/tools/acknowledge-alert-incident.ts b/apps/ai/src/mcp/tools/acknowledge-alert-incident.ts index bbe82483a7..106aa1a9f4 100644 --- a/apps/ai/src/mcp/tools/acknowledge-alert-incident.ts +++ b/apps/ai/src/mcp/tools/acknowledge-alert-incident.ts @@ -1,4 +1,5 @@ -import { McpInvalidInputError, McpQueryError, type McpToolRegistrar } from "./types" +import { McpInvalidInputError, type McpToolRegistrar } from "./types" +import { toMcpHttpError } from "../lib/map-http-error" import { Effect, Schema } from "effect" import { AcknowledgeAlertIncidentOutput } from "@maple/domain/mcp-outputs" import { AlertIncidentAckService } from "@maple/backend/services/alerts/AlertIncidentAckService" @@ -57,13 +58,7 @@ export function registerAcknowledgeAlertIncidentTool(server: McpToolRegistrar) { new McpInvalidInputError({ message: error.message, parameter: "incident_id" }), ), "@maple/http/errors/AlertPersistenceError": (error) => - Effect.fail( - new McpQueryError({ - message: `${error._tag}: ${error.message}`, - pipeName: "acknowledge_alert_incident", - cause: error, - }), - ), + Effect.fail(toMcpHttpError("acknowledge_alert_incident")(error)), }), ) diff --git a/apps/ai/src/mcp/tools/delete-alert-rule.ts b/apps/ai/src/mcp/tools/delete-alert-rule.ts index b4e41c6307..cb0865aab5 100644 --- a/apps/ai/src/mcp/tools/delete-alert-rule.ts +++ b/apps/ai/src/mcp/tools/delete-alert-rule.ts @@ -1,4 +1,5 @@ -import { McpInvalidInputError, McpQueryError, type McpToolRegistrar } from "./types" +import { McpInvalidInputError, type McpToolRegistrar } from "./types" +import { toMcpHttpError } from "../lib/map-http-error" import { Effect, Schema } from "effect" import { DeleteAlertRuleOutput } from "@maple/domain/mcp-outputs" import { CurrentMcpTenant } from "../lib/query-warehouse" @@ -39,21 +40,9 @@ export function registerDeleteAlertRuleTool(server: McpToolRegistrar) { "@maple/http/errors/AlertValidationError": (error) => Effect.fail(new McpInvalidInputError({ message: error.message })), "@maple/http/errors/AlertForbiddenError": (error) => - Effect.fail( - new McpQueryError({ - message: `${error._tag}: ${error.message}`, - pipeName: "delete_alert_rule", - cause: error, - }), - ), + Effect.fail(toMcpHttpError("delete_alert_rule")(error)), "@maple/http/errors/AlertPersistenceError": (error) => - Effect.fail( - new McpQueryError({ - message: `${error._tag}: ${error.message}`, - pipeName: "delete_alert_rule", - cause: error, - }), - ), + Effect.fail(toMcpHttpError("delete_alert_rule")(error)), }), ) diff --git a/apps/ai/src/mcp/tools/integration-writes.ts b/apps/ai/src/mcp/tools/integration-writes.ts index cf5de03a31..f40e9239ca 100644 --- a/apps/ai/src/mcp/tools/integration-writes.ts +++ b/apps/ai/src/mcp/tools/integration-writes.ts @@ -43,7 +43,7 @@ const permitted = (tool: string, permission: Policy.Permission) => }) const queryError = (tool: string) => (error: { readonly _tag: string; readonly message: string }) => - new McpQueryError({ message: `${error._tag}: ${error.message}`, pipeName: tool, cause: error }) + new McpQueryError({ message: error.message, pipeName: tool, cause: error }) const INTERVAL = Schema.Int.check(Schema.isGreaterThanOrEqualTo(5), Schema.isLessThanOrEqualTo(300)) const decodeInterval = Schema.decodeUnknownEffect(ScrapeIntervalSeconds) diff --git a/apps/ai/src/mcp/tools/registry.test.ts b/apps/ai/src/mcp/tools/registry.test.ts index f16ca73365..ef2af4a141 100644 --- a/apps/ai/src/mcp/tools/registry.test.ts +++ b/apps/ai/src/mcp/tools/registry.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest" import { Effect, Exit, Schema, Array as Arr, String as Str } from "effect" -import { mapleToolCatalog, mapleToolPhrase, toInputSchema } from "./registry" +import { mapleToolCatalog, mapleToolPhrase, readToolCall, toInputSchema } from "./registry" import * as P from "../lib/params" const jsonOf = (schema: unknown): string => JSON.stringify(schema) @@ -162,3 +162,48 @@ describe("mapleToolPhrase", () => { expect(mapleToolPhrase("submit_diagnosis")).toBeUndefined() }) }) + +describe("enum values as models spell them", () => { + const feedback = (kind: string) => + readToolCall("send_maple_feedback", { kind, summary: "s", reason: "r" }) + + it("reads case, spaces and hyphens as the published value", () => { + expect(feedback("Feature Request")?.args.kind).toBe("feature_request") + expect(feedback("feature-request")?.decodes).toBe(true) + }) + + it("reads a declared alias as its target", () => { + expect(feedback("docs")?.args.kind).toBe("documentation") + expect(feedback("bug_report")?.args.kind).toBe("bug") + expect(feedback("enhancement")?.decodes).toBe(true) + }) + + it("maps every declared value alias to a value its parameter publishes", () => { + const broken = Arr.flatMap(mapleToolCatalog, (tool) => + Arr.flatMap(Object.entries(tool.valueAliases), ([parameter, aliases]) => { + const properties = toInputSchema(tool.schema).properties + const property = + typeof properties === "object" && properties !== null + ? Object.entries(properties).find(([name]) => name === parameter)?.[1] + : undefined + const published = + typeof property === "object" && + property !== null && + "enum" in property && + Arr.isArray(property.enum) + ? property.enum + : [] + return Arr.flatMap(Object.entries(aliases), ([alias, target]) => + published.includes(target) && !published.includes(alias) + ? [] + : [`${tool.name}.${parameter}: ${alias}`], + ) + }), + ) + expect(broken).toEqual([]) + }) + + it("still rejects a value nothing maps to", () => { + expect(feedback("telepathy")?.decodes).toBe(false) + }) +}) diff --git a/apps/ai/src/mcp/tools/registry.ts b/apps/ai/src/mcp/tools/registry.ts index 8de7928f14..6738c0cfa0 100644 --- a/apps/ai/src/mcp/tools/registry.ts +++ b/apps/ai/src/mcp/tools/registry.ts @@ -121,7 +121,9 @@ import { type McpToolPhrases, type McpToolRegistrar, type McpToolResult, + refusesReadOnlyCredential, } from "./types" +import { unknownToolMessage } from "../lib/tool-suggestions" import { argumentWarnings, enumValues, @@ -157,6 +159,7 @@ export interface MapleToolCatalogEntry { readonly outputSchema: Schema.Codec readonly hints: McpToolHints readonly aliases: Readonly> + readonly valueAliases: Readonly>>> readonly audience: McpToolAudience readonly scopeFamily?: string readonly phrases: McpToolPhrases @@ -387,6 +390,7 @@ const collectMapleToolDefinitions = (): ReadonlyArray => { outputSchema: spec.output, hints: spec.hints, aliases: spec.aliases ?? {}, + valueAliases: spec.valueAliases ?? {}, audience: spec.audience ?? "public", ...(spec.scopeFamily === undefined ? undefined : { scopeFamily: spec.scopeFamily }), phrases: spec.phrases, @@ -552,9 +556,35 @@ const parameterNames = (entry: MapleToolCatalogEntry): ReadonlyArray => return typeof properties === "object" && properties !== null ? Rec.keys(properties) : [] } -/** How a call's arguments are read before decoding: aliases applied, enum case fixed, unknowns dropped. */ +const enumSpelling = (value: string): string => Str.replace(/[\s-]+/g, "_")(Str.toLowerCase(Str.trim(value))) + +/** + * An enum value as the model spelled it, read as the published one it means: `Feature Request` + * and `feature-request` are `feature_request`, a declared value alias is its target. Anything + * else is left for the decoder to reject with the allowed values. + */ +const canonicalEnumArguments = (entry: MapleToolCatalogEntry, input: unknown): unknown => { + if (!isArgumentRecord(input)) return input + const enums = enumValues(inputSchemaOf(entry)) + return Rec.map(input, (value, key) => { + const allowed = enums.get(key) + if (typeof value !== "string" || allowed === undefined || allowed.includes(value)) return value + const spelled = enumSpelling(value) + if (allowed.includes(spelled)) return spelled + const target = entry.valueAliases[key]?.[spelled] + return target !== undefined && allowed.includes(target) ? target : value + }) +} + +/** How a call's arguments are read before decoding: aliases applied, enum spellings fixed, unknowns dropped. */ const normalizeFor = (entry: MapleToolCatalogEntry, input: unknown, nowMs?: number) => - normalizeArguments(input, parameterNames(entry), entry.aliases, enumValues(inputSchemaOf(entry)), nowMs) + normalizeArguments( + canonicalEnumArguments(entry, input), + parameterNames(entry), + entry.aliases, + enumValues(inputSchemaOf(entry)), + nowMs, + ) const isArgumentRecord = (value: unknown): value is Readonly> => typeof value === "object" && value !== null && !Arr.isArray(value) @@ -683,12 +713,14 @@ export const executeRegisteredMcpToolUnscoped = Effect.fn("McpToolRegistry.execu } return yield* new McpToolNotFoundError({ toolName: name, - message: `Unknown MCP tool: ${name}`, + message: unknownToolMessage( + name, + Arr.map(mapleToolCatalogFor(surface), (entry) => entry.name), + ), }) } - // A read-only key reaches every read tool; feedback stays open to it, as on v2. - if (!definition.hints.readOnly && definition.name !== "send_maple_feedback") { + if (refusesReadOnlyCredential(definition.name, definition.hints.readOnly)) { const tenant = yield* CurrentMcpTenant if (tenant.readOnly === true) { return yield* new McpAuthInvalidError({ diff --git a/apps/ai/src/mcp/tools/send-maple-feedback.ts b/apps/ai/src/mcp/tools/send-maple-feedback.ts index b720adf37b..f629c8c36b 100644 --- a/apps/ai/src/mcp/tools/send-maple-feedback.ts +++ b/apps/ai/src/mcp/tools/send-maple-feedback.ts @@ -1,5 +1,5 @@ import { McpInvalidInputError, McpNotReadyError, type McpToolRegistrar } from "./types" -import { DateTime, Effect, Schema } from "effect" +import { DateTime, Effect, Schedule, Schema, String as Str } from "effect" import { AgentFeedbackImpact, AgentFeedbackKind, AgentType } from "@maple/domain/http" import { AgentFeedbackPublicId } from "@maple/domain/http/v2" import { SendMapleFeedbackOutput } from "@maple/domain/mcp-outputs" @@ -39,9 +39,9 @@ export function registerSendMapleFeedbackTool(server: McpToolRegistrar) { name: "send_maple_feedback", title: "Send Maple Feedback", description: - "Send feedback about Maple itself (this MCP server, its tools, API, or docs) to the Maple team: a bug you hit, a tool or capability you were missing, a misleading description, or something that worked well. " + - "Use it when Maple got in the way of your task, not for problems in the user's own services. " + - "The report goes to the Maple team and is visible to the user's organization, so ask the user first with a one-line summary, unless their instructions already allow sending Maple feedback. " + + "Report a problem with Maple itself (this MCP server, its tools, API, or docs) to the Maple team: a bug, a missing capability, a misleading description, or something that worked well. " + + "Send it yourself, without asking first, when a Maple tool fails in a way that looks like Maple's fault or Maple got in the way of the task; skip it only if the user's instructions say not to send Maple feedback. Not for problems in the user's own services. " + + "The report is visible to the Maple team and to the user's organization, so tell the user in one line that you sent it. " + "Describe Maple's behaviour, not the user's data: leave out secrets, customer data, and raw log or span contents.", parameters: Schema.Struct({ kind: P.oneOf( @@ -60,14 +60,55 @@ export function registerSendMapleFeedbackTool(server: McpToolRegistrar) { "blocking (could not finish), degraded (finished with a workaround), or minor", ), related_to: P.optionalText("The tool, API endpoint, or page this is about, e.g. 'search_traces'"), - agent_type: P.oneOf( + agent_type: P.optionalOneOf( AgentType.literals, - "coding_agent (Claude Code, Cursor, Codex), chat_assistant, autonomous_agent, ci, or other", + "coding_agent (Claude Code, Cursor, Codex), chat_assistant, autonomous_agent, ci, or other (the default)", + ), + agent_name: P.optionalText( + "Your client or agent name, e.g. 'claude-code'. Default: the name your client sent when it connected", ), - agent_name: P.optionalText("Your client or agent name, e.g. 'claude-code'"), model: P.optionalText("The model you run on, e.g. 'claude-opus-5-5'"), agent_version: P.optionalText("Your client or agent version"), }), + // Values agents send for `kind` and `agent_type` that mean a published one. + valueAliases: { + kind: { + bug_report: "bug", + error: "bug", + issue: "bug", + broken: "bug", + feature: "feature_request", + feature_req: "feature_request", + request: "feature_request", + missing_feature: "feature_request", + enhancement: "improvement", + suggestion: "improvement", + ux: "improvement", + usability: "improvement", + performance: "improvement", + docs: "documentation", + doc: "documentation", + description: "documentation", + positive: "praise", + compliment: "praise", + kudos: "praise", + feedback: "other", + general: "other", + }, + agent_type: { + coding: "coding_agent", + coding_assistant: "coding_agent", + code_agent: "coding_agent", + cli: "coding_agent", + ide: "coding_agent", + assistant: "chat_assistant", + chat: "chat_assistant", + chatbot: "chat_assistant", + autonomous: "autonomous_agent", + agent: "autonomous_agent", + ci_cd: "ci", + }, + }, output: SendMapleFeedbackOutput, hints: { readOnly: false, destructive: false, idempotent: false }, phrases: ["Sending feedback"], @@ -91,20 +132,27 @@ export function registerSendMapleFeedbackTool(server: McpToolRegistrar) { details: params.details, relatedTo: params.related_to, agent: { - type: params.agent_type, - name: params.agent_name, + type: params.agent_type ?? "other", + // The client's self-declared name is unchecked input: bounded like the parameter. + name: + params.agent_name ?? + (tenant.mcpClientName === undefined + ? undefined + : Str.slice(0, LIMITS.label)(tenant.mcpClientName)), model: params.model, version: params.agent_version, }, source: "mcp", }) .pipe( + // A report is often sent right after something else failed: one blip must not lose it. + Effect.retry({ times: 2, schedule: Schedule.exponential("200 millis") }), Effect.catchTag( "@maple/http/errors/AgentFeedbackPersistenceError", () => new McpNotReadyError({ - message: "Feedback could not be saved right now.", - retryAfterSeconds: 5, + message: "Feedback could not be saved right now; nothing was recorded.", + retryAfterSeconds: 10, }), ), ) @@ -145,7 +193,9 @@ export function registerSendMapleFeedbackTool(server: McpToolRegistrar) { [output.agentType, output.agentName, output.agentModel].filter(Boolean).join(" · "), ], ]), - doc.text("Thanks. The Maple team reads every submission."), + doc.text( + "Sent. Tell the user in one line that you reported this to the Maple team; their organization can see it too.", + ), ], }), }) diff --git a/apps/ai/src/mcp/tools/status-pages.ts b/apps/ai/src/mcp/tools/status-pages.ts index 574bdd0486..2317953c62 100644 --- a/apps/ai/src/mcp/tools/status-pages.ts +++ b/apps/ai/src/mcp/tools/status-pages.ts @@ -85,7 +85,7 @@ const toMcpError = parameter: "if_version", }) default: - return new McpQueryError({ message: `${error._tag}: ${error.message}`, pipeName: tool }) + return new McpQueryError({ message: error.message, pipeName: tool, cause: error }) } } diff --git a/apps/ai/src/mcp/tools/types.ts b/apps/ai/src/mcp/tools/types.ts index 60e22646a1..286f7b0ac5 100644 --- a/apps/ai/src/mcp/tools/types.ts +++ b/apps/ai/src/mcp/tools/types.ts @@ -113,6 +113,13 @@ export interface McpToolHints { readonly openWorld?: boolean } +/** + * Whether a read-only credential is refused this tool, and so never shown it: every tool that + * changes data, except feedback, which stays open to read-only keys as on v2. + */ +export const refusesReadOnlyCredential = (name: string, readOnly: boolean): boolean => + !readOnly && name !== "send_maple_feedback" + /** * Who a tool is for. * @@ -189,6 +196,11 @@ export interface McpToolSpec< readonly phrases: McpToolPhrases /** Retired parameter names mapped to their current name. Accepted, never published. */ readonly aliases?: Readonly> + /** + * Per enum parameter, other spellings of a published value (`feature` for `feature_request`). + * Accepted, never published. Case, spaces and hyphens are already forgiven for every enum. + */ + readonly valueAliases?: Readonly>>> /** * Refuse keys a nested object parameter does not declare, instead of dropping them. For * documents that replace state, where a dropped typo would reset a field to its default. diff --git a/packages/backend/src/services/warehouse/missing-table.ts b/packages/backend/src/services/warehouse/missing-table.ts index e04b918797..c81d6fa347 100644 --- a/packages/backend/src/services/warehouse/missing-table.ts +++ b/packages/backend/src/services/warehouse/missing-table.ts @@ -1,3 +1,5 @@ +import { Array as Arr, Predicate, String as Str } from "effect" + /** * Detect "this table is missing on the org's cluster" so a caller can read a * raw-source equivalent (or degrade) instead of 502ing. @@ -29,6 +31,21 @@ const isMissingTable = ) } +const MISSING_TABLE_NAME = + /Resource '([^']+)' not found|Unknown table expression identifier '([^']+)'|Table (\S+) does not exist/i + +/** + * The table a `WarehouseConfigError` says is missing, when it says so; undefined for every other + * error. A table that is not deployed yet (deploy ordering, an unapplied BYO schema) clears later. + */ +export const missingWarehouseTable = (error: unknown): string | undefined => { + if (!isMissingTable(MISSING_TABLE_NAME)(error) || !Predicate.hasProperty(error, "message")) + return undefined + const match = Predicate.isString(error.message) ? MISSING_TABLE_NAME.exec(error.message) : null + const name = match?.[1] ?? match?.[2] ?? match?.[3] + return name === undefined ? undefined : Arr.lastNonEmpty(Str.split(".")(name)) +} + /** * Only page-view queries may degrade on this one — raw `session_events` holds * the same browser rows. Funnels have no raw counterpart (server and mobile diff --git a/packages/query-engine/src/execution/errors.test.ts b/packages/query-engine/src/execution/errors.test.ts index dd57a4d2c8..af3abab410 100644 --- a/packages/query-engine/src/execution/errors.test.ts +++ b/packages/query-engine/src/execution/errors.test.ts @@ -317,6 +317,10 @@ describe("mapWarehouseError", () => { // Neither names a table the caller chose, so both stay config errors // regardless of who wrote the SQL. expect(classify("p", "Resource 'product_events' not found")).toBeInstanceOf(WarehouseConfigError) + // Tinybird sends it as a 403; the status must not turn it into a credentials failure. + expect( + classify("p", { message: "Resource 'user_activity_hourly' not found", status: 403 }, "maple"), + ).toBeInstanceOf(WarehouseConfigError) expect(classify("p", "Invalid URL")).toBeInstanceOf(WarehouseConfigError) expect(classify("p", { message: "x", type: "UNKNOWN_SETTING" })).toBeInstanceOf( WarehouseConfigError, diff --git a/packages/query-engine/src/execution/errors.ts b/packages/query-engine/src/execution/errors.ts index 53f083e9e4..d0f874844d 100644 --- a/packages/query-engine/src/execution/errors.ts +++ b/packages/query-engine/src/execution/errors.ts @@ -169,8 +169,14 @@ type ClassificationRule = { // Ordered rules — first match wins. A raw error can satisfy several patterns // (e.g. a 503 carrying an HTML body, or schema-drift text inside a transient // failure), so the order encodes precedence: -// auth > upstream > config > client > schema_drift > (default) query. +// missing datasource > auth > upstream > config > client > schema_drift > (default) query. const CLASSIFICATION_RULES: ReadonlyArray = [ + { + // Tinybird answers a missing datasource with a 403, so the auth rule's status match read a + // table still being deployed as rejected credentials. The text names the failure; it wins. + pattern: /Resource '[^']*' not found/i, + make: (base) => new WarehouseConfigError(base), + }, { status: (s) => s === 401 || s === 403, types: new Set(["AUTHENTICATION_FAILED", "ACCESS_DENIED", "USER_DOESNT_EXIST", "REQUIRED_PASSWORD"]),