From 0110a9673fe5f88b7cb3f1c261657613f399217c Mon Sep 17 00:00:00 2001 From: JeremyFunk Date: Fri, 31 Jul 2026 03:58:39 +0200 Subject: [PATCH] Effect v4 review fixes: apps/api MCP domain Fixes from a full effect-review-v4 pass over apps/api/src/mcp (110 files): - Replace Schema.decodeUnknownSync on agent-supplied input with effectful decoding so invalid ids/filters return actionable tool errors instead of escaping as defects (update-dashboard, query-data) - Replace try/catch JSON.parse with Schema.fromJsonString decoding (resolve-tenant, create-dashboard); untrace the auth hot path - Log the cause before the inspect-widget catchCause fallbacks; map warehouse errors per-tag instead of collapsing the union - Annotate orgId + tool on every tool span centrally in the dispatcher (annotateCurrentSpan + annotateSpans); drop dead McpTenantError - Make resolveTimeRange/resolveDashboardTimeRange read the Effect Clock instead of DateTime.nowUnsafe, updating all tool call sites; time.test now asserts exact values under TestClock - queryWarehouse gains an optional rowSchema for validated rows - Migrate MCP tests to @effect/vitest idioms (it.effect, assert.*) - Add test coverage for inspect-widget helpers (33 tests) and the resolve-tenant auth path (29 tests) Co-Authored-By: Claude Fable 5 --- apps/api/src/mcp/__evals__/regression.test.ts | 97 +-- apps/api/src/mcp/app.test.ts | 150 ++--- apps/api/src/mcp/dispatcher.test.ts | 23 +- apps/api/src/mcp/dispatcher.ts | 34 +- .../src/mcp/lib/dashboard-mutations.test.ts | 10 +- apps/api/src/mcp/lib/inspect-widget.test.ts | 583 ++++++++++++++++++ apps/api/src/mcp/lib/inspect-widget.ts | 53 +- apps/api/src/mcp/lib/query-warehouse.ts | 28 +- .../mcp/lib/resolve-dashboard-time-range.ts | 26 +- apps/api/src/mcp/lib/resolve-tenant.test.ts | 500 +++++++++++++++ apps/api/src/mcp/lib/resolve-tenant.ts | 16 +- apps/api/src/mcp/lib/time.test.ts | 125 ++-- apps/api/src/mcp/lib/time.ts | 45 +- .../__tests__/dashboard-concurrency.test.ts | 31 +- apps/api/src/mcp/tools/compare-periods.ts | 2 +- apps/api/src/mcp/tools/create-alert-rule.ts | 19 +- apps/api/src/mcp/tools/create-dashboard.ts | 10 +- apps/api/src/mcp/tools/diagnose-service.ts | 2 +- apps/api/src/mcp/tools/error-detail.ts | 2 +- apps/api/src/mcp/tools/explore-attributes.ts | 2 +- apps/api/src/mcp/tools/find-errors.ts | 2 +- apps/api/src/mcp/tools/find-slow-traces.ts | 2 +- .../get-instrumentation-recommendations.ts | 2 +- .../mcp/tools/get-service-top-operations.ts | 2 +- apps/api/src/mcp/tools/inspect-chart-data.ts | 6 +- apps/api/src/mcp/tools/list-metrics.ts | 2 +- apps/api/src/mcp/tools/list-services.ts | 2 +- apps/api/src/mcp/tools/mine-log-patterns.ts | 2 +- apps/api/src/mcp/tools/query-data.ts | 83 ++- apps/api/src/mcp/tools/run-sql.ts | 2 +- apps/api/src/mcp/tools/search-logs.ts | 2 +- apps/api/src/mcp/tools/search-sessions.ts | 2 +- apps/api/src/mcp/tools/search-traces.ts | 2 +- apps/api/src/mcp/tools/service-map.ts | 2 +- apps/api/src/mcp/tools/types.ts | 5 - apps/api/src/mcp/tools/update-dashboard.ts | 13 +- apps/api/src/routes/chat.http.ts | 1 - 37 files changed, 1558 insertions(+), 332 deletions(-) create mode 100644 apps/api/src/mcp/lib/inspect-widget.test.ts create mode 100644 apps/api/src/mcp/lib/resolve-tenant.test.ts diff --git a/apps/api/src/mcp/__evals__/regression.test.ts b/apps/api/src/mcp/__evals__/regression.test.ts index b4719ccd7..a431b901e 100644 --- a/apps/api/src/mcp/__evals__/regression.test.ts +++ b/apps/api/src/mcp/__evals__/regression.test.ts @@ -1,4 +1,5 @@ -import { afterAll, beforeAll, describe, expect, it } from "vitest" +import { afterAll, assert, beforeAll, describe, it } from "@effect/vitest" +import { Effect } from "effect" import { installFakeWarehouse, restoreWarehouse, type FixtureRule } from "./fake-warehouse" import { makeEvalRuntime, runToolDirect, type EvalRuntime } from "./eval-runtime" import { @@ -58,48 +59,64 @@ afterAll(async () => { }) describe("inspect_trace bounded-overview rendering", () => { - it("renders a small trace in full (no truncation note)", async () => { - const result = await runToolDirect(rt, "inspect_trace", { trace_id: SMALL_TRACE_ID }) - const text = renderedText(result) - expect(text).not.toContain("Showing") - expect(text).toContain("GET /api/orders") - // Span ids are surfaced at the end of each line for follow-up lookups. - expect(text).toContain("span=") - }) + it.effect("renders a small trace in full (no truncation note)", () => + Effect.gen(function* () { + const result = yield* Effect.promise(() => + runToolDirect(rt, "inspect_trace", { trace_id: SMALL_TRACE_ID }), + ) + const text = renderedText(result) + assert.notInclude(text, "Showing") + assert.include(text, "GET /api/orders") + // Span ids are surfaced at the end of each line for follow-up lookups. + assert.include(text, "span=") + }), + ) - it("caps a large trace and keeps the error span + omitted marker", async () => { - const result = await runToolDirect(rt, "inspect_trace", { trace_id: FIXTURES.traceId }) - const text = renderedText(result) - // Bounded overview note. - expect(text).toContain(`of ${LARGE_TRACE_SPAN_COUNT} spans (errors and longest first)`) - // The single error span survives selection even though it's low-duration. - expect(text).toContain("[Error]") - expect(text).toContain("db.query users") - // Dropped siblings are surfaced, not silently hidden. - expect(text).toContain("more spans") - // Full span ids remain available for inspect_span pivots. - expect(text).toContain("span=") - }) + it.effect("caps a large trace and keeps the error span + omitted marker", () => + Effect.gen(function* () { + const result = yield* Effect.promise(() => + runToolDirect(rt, "inspect_trace", { trace_id: FIXTURES.traceId }), + ) + const text = renderedText(result) + // Bounded overview note. + assert.include(text, `of ${LARGE_TRACE_SPAN_COUNT} spans (errors and longest first)`) + // The single error span survives selection even though it's low-duration. + assert.include(text, "[Error]") + assert.include(text, "db.query users") + // Dropped siblings are surfaced, not silently hidden. + assert.include(text, "more spans") + // Full span ids remain available for inspect_span pivots. + assert.include(text, "span=") + }), + ) }) describe("inspect_span drill-down", () => { - it("returns the full attribute set for a known span", async () => { - const result = await runToolDirect(rt, "inspect_span", { - trace_id: SPAN_DETAIL_TRACE_ID, - span_id: SPAN_DETAIL_SPAN_ID, - }) - const text = renderedText(result) - expect(text).toContain("http.method") - expect(text).toContain("POST") - expect(text).toContain("/api/checkout") - }) + it.effect("returns the full attribute set for a known span", () => + Effect.gen(function* () { + const result = yield* Effect.promise(() => + runToolDirect(rt, "inspect_span", { + trace_id: SPAN_DETAIL_TRACE_ID, + span_id: SPAN_DETAIL_SPAN_ID, + }), + ) + const text = renderedText(result) + assert.include(text, "http.method") + assert.include(text, "POST") + assert.include(text, "/api/checkout") + }), + ) - it("reports a friendly message for an unknown span (no crash)", async () => { - const result = await runToolDirect(rt, "inspect_span", { - trace_id: SPAN_DETAIL_TRACE_ID, - span_id: MISSING_SPAN_ID, - }) - const text = renderedText(result) - expect(text.toLowerCase()).toContain("not found") - }) + it.effect("reports a friendly message for an unknown span (no crash)", () => + Effect.gen(function* () { + const result = yield* Effect.promise(() => + runToolDirect(rt, "inspect_span", { + trace_id: SPAN_DETAIL_TRACE_ID, + span_id: MISSING_SPAN_ID, + }), + ) + const text = renderedText(result) + assert.include(text.toLowerCase(), "not found") + }), + ) }) diff --git a/apps/api/src/mcp/app.test.ts b/apps/api/src/mcp/app.test.ts index 5c0dab005..8fed751cf 100644 --- a/apps/api/src/mcp/app.test.ts +++ b/apps/api/src/mcp/app.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it } from "@effect/vitest" +import { afterEach, assert, describe, it } from "@effect/vitest" import { OrgId, UserId } from "@maple/domain/http" import { ConfigProvider, Context, Effect, Layer, Schema } from "effect" import { HttpRouter } from "effect/unstable/http" @@ -26,47 +26,50 @@ const testConfig = () => ) describe("MCP HTTP authorization", () => { - it("challenges unauthenticated clients before MCP initialization", async () => { + it.effect("challenges unauthenticated clients before MCP initialization", () => { const db = createTestDb(createdDbs) const base = Layer.mergeAll(db.layer, Env.layer.pipe(Layer.provide(testConfig()))) const services = Layer.mergeAll(ApiKeysService.layer, AuthService.layer).pipe( Layer.provideMerge(base), ) const routes = McpLive.pipe(Layer.provideMerge(services)) - const { handler, dispose } = HttpRouter.toWebHandler(routes, { disableLogger: true }) - try { - const response = await handler( - new Request("https://api.example.com/mcp", { - method: "POST", - headers: { - "content-type": "application/json", - host: "api.example.com", - "x-forwarded-proto": "https", - }, - body: JSON.stringify({ - jsonrpc: "2.0", - id: 1, - method: "initialize", - params: { - protocolVersion: "2025-11-25", - capabilities: {}, - clientInfo: { name: "test", version: "1.0.0" }, + + return Effect.gen(function* () { + const { handler, dispose } = HttpRouter.toWebHandler(routes, { disableLogger: true }) + const response = yield* Effect.promise(() => + handler( + new Request("https://api.example.com/mcp", { + method: "POST", + headers: { + "content-type": "application/json", + host: "api.example.com", + "x-forwarded-proto": "https", }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-11-25", + capabilities: {}, + clientInfo: { name: "test", version: "1.0.0" }, + }, + }), }), - }), - Context.empty() as never, - ) - expect(response.status).toBe(401) - expect(response.headers.get("www-authenticate")).toContain( + Context.empty() as never, + ), + ).pipe(Effect.ensuring(Effect.promise(() => dispose()))) + + assert.strictEqual(response.status, 401) + assert.include( + response.headers.get("www-authenticate") ?? "", 'resource_metadata="https://api.example.com/.well-known/oauth-protected-resource/mcp"', ) - expect(response.headers.get("www-authenticate")).toContain('scope="mcp:tools"') - } finally { - await dispose() - } + assert.include(response.headers.get("www-authenticate") ?? "", 'scope="mcp:tools"') + }) }) - it("accepts an audience-bound OAuth key behind a forwarded HTTPS proxy", async () => { + it.effect("accepts an audience-bound OAuth key behind a forwarded HTTPS proxy", () => { const db = createTestDb(createdDbs) const base = Layer.mergeAll(db.layer, Env.layer.pipe(Layer.provide(testConfig()))) const services = Layer.mergeAll(ApiKeysService.layer, AuthService.layer).pipe( @@ -74,51 +77,50 @@ describe("MCP HTTP authorization", () => { ) const orgId = Schema.decodeUnknownSync(OrgId)("org_test") const userId = Schema.decodeUnknownSync(UserId)("user_test") - const key = await Effect.runPromise( - Effect.gen(function* () { - const apiKeys = yield* ApiKeysService - return yield* apiKeys.create(orgId, userId, { - name: "OAuth MCP test", - kind: "mcp", - scopes: ["mcp:tools"], - metadataJson: { - source: "maple_mcp_oauth", - roles: ["org:member"], - clientId: "client_test", - resource: "https://api.example.com/mcp", - }, - }) - }).pipe(Effect.provide(services)), - ) - const routes = McpLive.pipe(Layer.provideMerge(services)) - const { handler, dispose } = HttpRouter.toWebHandler(routes, { disableLogger: true }) - try { - const response = await handler( - new Request("http://internal-worker.invalid/mcp", { - method: "POST", - headers: { - authorization: `Bearer ${key.secret}`, - "content-type": "application/json", - host: "internal-worker.invalid", - "x-forwarded-host": "api.example.com", - "x-forwarded-proto": "https", - }, - body: JSON.stringify({ - jsonrpc: "2.0", - id: 1, - method: "initialize", - params: { - protocolVersion: "2025-11-25", - capabilities: {}, - clientInfo: { name: "test", version: "1.0.0" }, + + return Effect.gen(function* () { + const apiKeys = yield* ApiKeysService + const key = yield* apiKeys.create(orgId, userId, { + name: "OAuth MCP test", + kind: "mcp", + scopes: ["mcp:tools"], + metadataJson: { + source: "maple_mcp_oauth", + roles: ["org:member"], + clientId: "client_test", + resource: "https://api.example.com/mcp", + }, + }) + + const routes = McpLive.pipe(Layer.provideMerge(services)) + const { handler, dispose } = HttpRouter.toWebHandler(routes, { disableLogger: true }) + const response = yield* Effect.promise(() => + handler( + new Request("http://internal-worker.invalid/mcp", { + method: "POST", + headers: { + authorization: `Bearer ${key.secret}`, + "content-type": "application/json", + host: "internal-worker.invalid", + "x-forwarded-host": "api.example.com", + "x-forwarded-proto": "https", }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-11-25", + capabilities: {}, + clientInfo: { name: "test", version: "1.0.0" }, + }, + }), }), - }), - Context.empty() as never, - ) - expect(response.status).toBe(200) - } finally { - await dispose() - } + Context.empty() as never, + ), + ).pipe(Effect.ensuring(Effect.promise(() => dispose()))) + + assert.strictEqual(response.status, 200) + }).pipe(Effect.provide(services)) }) }) diff --git a/apps/api/src/mcp/dispatcher.test.ts b/apps/api/src/mcp/dispatcher.test.ts index 710f7e4de..024b17d4e 100644 --- a/apps/api/src/mcp/dispatcher.test.ts +++ b/apps/api/src/mcp/dispatcher.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "@effect/vitest" +import { assert, describe, it } from "@effect/vitest" import { Effect, Schema } from "effect" import type { InternalRpcToolNotFoundError } from "@maple/domain/internal-rpc" import { callMcpTool, listMcpTools } from "./dispatcher" @@ -16,13 +16,14 @@ describe("MCP dispatcher", () => { })) .filter(({ type }) => type !== "object") - expect(invalidSchemas).toEqual([]) + assert.deepStrictEqual(invalidSchemas, []) }) it.effect("publishes the same names, descriptions, and schemas used by HTTP MCP", () => Effect.gen(function* () { const descriptors = yield* listMcpTools - expect(descriptors).toEqual( + assert.deepStrictEqual( + descriptors, mapleToolDefinitions.map((definition) => ({ name: definition.name, description: definition.description, @@ -35,7 +36,7 @@ describe("MCP dispatcher", () => { it("normalizes an empty Struct root and rejects a non-object root", () => { // Effect emits `{ anyOf: [{type:"object"},{type:"array"}] }` — no `type` — // for a no-parameter tool; that exact shape is normalized. - expect(toInputSchema(Schema.Struct({}))).toEqual({ + assert.deepStrictEqual(toInputSchema(Schema.Struct({})), { type: "object", properties: {}, additionalProperties: false, @@ -43,8 +44,8 @@ describe("MCP dispatcher", () => { // Anything else with a non-object root has parameters an empty object // schema would erase, so registration fails loudly instead. - expect(() => toInputSchema(Schema.Literals(["a", "b"]))).toThrow(/object root/) - expect(() => toInputSchema(Schema.Array(Schema.String))).toThrow(/object root/) + assert.throws(() => toInputSchema(Schema.Literals(["a", "b"])), /object root/) + assert.throws(() => toInputSchema(Schema.Array(Schema.String)), /object root/) }) it.effect("returns MCP validation feedback for invalid model tool input", () => @@ -57,9 +58,9 @@ describe("MCP dispatcher", () => { never, never > - expect(result.isError).toBe(true) - expect(result.content[0]?.text).toContain("Invalid parameters") - expect(result.content[0]?.text).toContain("inspect_trace") + assert.strictEqual(result.isError, true) + assert.include(result.content[0]?.text ?? "", "Invalid parameters") + assert.include(result.content[0]?.text ?? "", "inspect_trace") }), ) @@ -72,8 +73,8 @@ describe("MCP dispatcher", () => { never >, ) - expect(error._tag).toBe("@maple/internal-rpc/ToolNotFoundError") - expect(error.name).toBe("not_a_maple_tool") + assert.strictEqual(error._tag, "@maple/internal-rpc/ToolNotFoundError") + assert.strictEqual(error.name, "not_a_maple_tool") }), ) }) diff --git a/apps/api/src/mcp/dispatcher.ts b/apps/api/src/mcp/dispatcher.ts index 822348b58..fb942b68e 100644 --- a/apps/api/src/mcp/dispatcher.ts +++ b/apps/api/src/mcp/dispatcher.ts @@ -1,5 +1,6 @@ import { InternalRpcToolNotFoundError, type InternalMcpToolDescriptor } from "@maple/domain/internal-rpc" -import { Effect, Schema } from "effect" +import { Cause, Effect, Option, Schema } from "effect" +import { CurrentMcpTenant } from "./lib/query-warehouse" import { mapleToolDefinitions, toInputSchema, type MapleToolDefinition } from "./tools/registry" import type { McpToolResult } from "./tools/types" @@ -8,8 +9,10 @@ class McpDecodeError extends Schema.TaggedErrorClass()("@maple/m }) {} const toErrorMessage = (error: unknown): string => { - if (error instanceof Error && "error" in error && error.error != null) { - const inner = error.error + // `Effect.try`/`tryPromise` wrap thrown values in `Cause.UnknownError`; report + // the thrown value rather than the wrapper's generic message. + if (Cause.isUnknownError(error) && error.cause != null) { + const inner = error.cause return inner instanceof Error ? inner.message : String(inner) } if (error instanceof Error) return error.message @@ -42,7 +45,13 @@ export const callMcpTool = Effect.fn("McpToolDispatcher.call")(function* (name: } const execute = Effect.gen(function* () { - yield* Effect.annotateCurrentSpan({ tool: definition.name }) + const tenant = yield* Effect.serviceOption(CurrentMcpTenant) + const spanAnnotations = Option.match(tenant, { + onNone: () => ({ tool: definition.name }), + onSome: ({ orgId }) => ({ tool: definition.name, orgId }), + }) + yield* Effect.annotateCurrentSpan(spanAnnotations) + const decoded = yield* Effect.try({ try: () => Schema.decodeUnknownSync(definition.schema)(input), catch: (error) => error, @@ -55,7 +64,14 @@ export const callMcpTool = Effect.fn("McpToolDispatcher.call")(function* (name: ), ) - return yield* definition.handler(decoded).pipe(Effect.tap(() => Effect.logInfo("Tool completed"))) + // The tool's own span is opened inside `definition.handler` (every handler is + // an `Effect.fn`), so the annotations are attached with `annotateSpans` — + // inherited by every span created below — rather than `annotateCurrentSpan`, + // which would only reach this dispatch span. + return yield* definition.handler(decoded).pipe( + Effect.annotateSpans(spanAnnotations), + Effect.tap(() => Effect.logInfo("Tool completed")), + ) }) return yield* execute.pipe( @@ -82,14 +98,6 @@ export const callMcpTool = Effect.fn("McpToolDispatcher.call")(function* (name: content: [{ type: "text", text: `${error._tag}: ${error.message}` }], } satisfies McpToolResult), ), - "@maple/mcp/errors/McpTenantError": (error) => - Effect.logError(`Tool error: ${error.message}`).pipe( - Effect.annotateLogs({ errorTag: error._tag }), - Effect.as({ - isError: true, - content: [{ type: "text", text: `${error._tag}: ${error.message}` }], - } satisfies McpToolResult), - ), "@maple/mcp/errors/McpAuthMissingError": (error) => Effect.logError(`Auth error: ${error.message}`).pipe( Effect.annotateLogs({ errorTag: error._tag }), diff --git a/apps/api/src/mcp/lib/dashboard-mutations.test.ts b/apps/api/src/mcp/lib/dashboard-mutations.test.ts index 454f24035..1802791d1 100644 --- a/apps/api/src/mcp/lib/dashboard-mutations.test.ts +++ b/apps/api/src/mcp/lib/dashboard-mutations.test.ts @@ -100,7 +100,8 @@ describe("dashboard mutations on tag-less / description-less dashboards", () => const layer = makeLayer(testDb) return Effect.gen(function* () { - yield* DashboardPersistenceService.upsert(asOrgId(ORG), asUserId("seed-user"), seed()) + const dashboards = yield* DashboardPersistenceService + yield* dashboards.upsert(asOrgId(ORG), asUserId("seed-user"), seed()) const result = yield* withDashboardMutation(DASHBOARD, "update_dashboard_widget", (widgets) => Effect.succeed([...widgets, widget("w-new")]), @@ -108,7 +109,7 @@ describe("dashboard mutations on tag-less / description-less dashboards", () => assert.strictEqual(result.ok, true) - const listed = yield* DashboardPersistenceService.list(asOrgId(ORG)) + const listed = yield* dashboards.list(asOrgId(ORG)) assert.strictEqual(listed.dashboards.length, 1) assert.deepStrictEqual( listed.dashboards[0]!.widgets.map((w) => w.id), @@ -132,13 +133,14 @@ describe("dashboard mutations on tag-less / description-less dashboards", () => const invoke = handler as unknown as ToolHandler return Effect.gen(function* () { - yield* DashboardPersistenceService.upsert(asOrgId(ORG), asUserId("seed-user"), seed()) + const dashboards = yield* DashboardPersistenceService + yield* dashboards.upsert(asOrgId(ORG), asUserId("seed-user"), seed()) const result = yield* invoke({ dashboard_id: DASHBOARD, name: "Renamed" }) assert.notStrictEqual(result.isError, true) - const listed = yield* DashboardPersistenceService.list(asOrgId(ORG)) + const listed = yield* dashboards.list(asOrgId(ORG)) assert.strictEqual(listed.dashboards[0]!.name, "Renamed") }).pipe(Effect.provide(layer)) }) diff --git a/apps/api/src/mcp/lib/inspect-widget.test.ts b/apps/api/src/mcp/lib/inspect-widget.test.ts new file mode 100644 index 000000000..db73c7b8e --- /dev/null +++ b/apps/api/src/mcp/lib/inspect-widget.test.ts @@ -0,0 +1,583 @@ +// Unit tests for the pure decision logic inside inspect-widget.ts. +// +// `applyReduceToValue`, `isSingleAllGroup` and `summarizeOutcome` are module-private +// helpers; they are exercised through the smallest exported surface that reaches +// them (`inspectWidget` / `inspectWidgetsAfterMutation`) with stub +// QueryEngineService / WarehouseQueryService layers, so no warehouse is needed. + +import { assert, describe, it } from "@effect/vitest" +import { Effect, Layer } from "effect" +import type { QueryEngineResult } from "@maple/query-engine" +import { QueryEngineService, type QueryEngineServiceShape } from "@/services/QueryEngineService" +import { WarehouseQueryService, type WarehouseQueryServiceShape } from "@/lib/WarehouseQueryService" +import type { TenantContext } from "@/lib/tenant-context" +import type { DashboardDocument } from "@maple/domain/http" +import { + inspectWidget, + inspectWidgetsAfterMutation, + type DashboardWidget, + type InspectWidgetTimeRange, +} from "./inspect-widget" + +const tenant = { orgId: "org_test", userId: "user_test", roles: [], authMode: "self_hosted" } as TenantContext + +const timeRange: InspectWidgetTimeRange = { + startTime: "2026-04-01 00:00:00", + endTime: "2026-04-01 06:00:00", + source: "dashboard", +} + +const TIMESERIES_ENDPOINT = "custom_query_builder_timeseries" +const BREAKDOWN_ENDPOINT = "custom_query_builder_breakdown" + +// --- stub layers ----------------------------------------------------------- + +/** QueryEngineService that always resolves to `result`. */ +const engineReturning = (result: QueryEngineResult) => + Layer.succeed(QueryEngineService, { + execute: () => Effect.succeed({ result }), + } as unknown as QueryEngineServiceShape) + +/** QueryEngineService whose `execute` throws synchronously (a defect). */ +const engineDefect = Layer.succeed(QueryEngineService, { + execute: () => { + throw new Error("engine exploded") + }, +} as unknown as QueryEngineServiceShape) + +/** WarehouseQueryService whose `list_metrics` lookup returns `metricNames`. */ +const catalogWith = (metricNames: ReadonlyArray) => + Layer.succeed(WarehouseQueryService, { + query: () => Effect.succeed({ data: metricNames.map((metricName) => ({ metricName })) }), + } as unknown as WarehouseQueryServiceShape) + +/** WarehouseQueryService whose catalog lookup fails (we must assume "exists"). */ +const catalogFailing = Layer.succeed(WarehouseQueryService, { + query: () => Effect.fail(new Error("catalog down")), +} as unknown as WarehouseQueryServiceShape) + +const stubs = (result: QueryEngineResult, metricNames: ReadonlyArray = []) => + Layer.mergeAll(engineReturning(result), catalogWith(metricNames)) + +// --- fixtures -------------------------------------------------------------- + +const tracesDraft = (over: Record = {}) => ({ + id: "q1", + name: "Query 1", + dataSource: "traces", + aggregation: "count", + ...over, +}) + +const makeWidget = (over: { + endpoint?: string + params?: unknown + transform?: Record + id?: string + title?: string +}): DashboardWidget => + ({ + id: over.id ?? "w1", + visualization: "chart", + dataSource: { + endpoint: over.endpoint ?? TIMESERIES_ENDPOINT, + ...(over.params !== undefined ? { params: over.params } : {}), + ...(over.transform !== undefined ? { transform: over.transform } : {}), + }, + display: over.title !== undefined ? { title: over.title } : {}, + layout: { x: 0, y: 0, w: 6, h: 4 }, + }) as unknown as DashboardWidget + +const timeseries = (points: ReadonlyArray<{ bucket: string; series: Record }>) => + ({ kind: "timeseries", source: "traces", data: points }) satisfies QueryEngineResult + +const breakdown = (rows: ReadonlyArray<{ name: string; value: number }>) => + ({ kind: "breakdown", source: "traces", data: rows }) satisfies QueryEngineResult + +const run = (widget: DashboardWidget, layer: Layer.Layer) => + inspectWidget({ tenant, dashboardName: "dash", widget, timeRange }).pipe(Effect.provide(layer)) + +/** Inspect a stat widget and return its single query's `reducedValue`. */ +const reduceWith = (aggregate: string | undefined, field: string, result: QueryEngineResult) => + Effect.gen(function* () { + const isBreakdown = result.kind === "breakdown" + const outcome = yield* run( + makeWidget({ + endpoint: isBreakdown ? BREAKDOWN_ENDPOINT : TIMESERIES_ENDPOINT, + // A breakdown query must group by something to be buildable. + params: { queries: [isBreakdown ? groupedDraft(["service"]) : tracesDraft()] }, + transform: { reduceToValue: { field, ...(aggregate !== undefined && { aggregate }) } }, + }), + stubs(result), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return undefined + return outcome.data.queries[0]?.reducedValue + }) + +const THREE_POINTS = timeseries([ + { bucket: "2026-04-01 00:00:00", series: { count: 2 } }, + { bucket: "2026-04-01 01:00:00", series: { count: 4 } }, + { bucket: "2026-04-01 02:00:00", series: { count: 9 } }, +]) + +// --- applyReduceToValue ---------------------------------------------------- + +describe("applyReduceToValue (via inspectWidget transform.reduceToValue)", () => { + it.effect("sums timeseries values", () => + Effect.gen(function* () { + assert.strictEqual(yield* reduceWith("sum", "count", THREE_POINTS), 15) + }), + ) + + it.effect("averages timeseries values", () => + Effect.gen(function* () { + assert.strictEqual(yield* reduceWith("avg", "count", THREE_POINTS), 5) + }), + ) + + it.effect("defaults to avg when no aggregate is configured", () => + Effect.gen(function* () { + assert.strictEqual(yield* reduceWith(undefined, "count", THREE_POINTS), 5) + }), + ) + + it.effect("supports min / max / first / count", () => + Effect.gen(function* () { + assert.strictEqual(yield* reduceWith("min", "count", THREE_POINTS), 2) + assert.strictEqual(yield* reduceWith("max", "count", THREE_POINTS), 9) + assert.strictEqual(yield* reduceWith("first", "count", THREE_POINTS), 2) + assert.strictEqual(yield* reduceWith("count", "count", THREE_POINTS), 3) + }), + ) + + it.effect("returns null for an unknown aggregate", () => + Effect.gen(function* () { + assert.strictEqual(yield* reduceWith("median", "count", THREE_POINTS), null) + }), + ) + + it.effect("falls back to the first numeric field when the configured field is absent", () => + Effect.gen(function* () { + // The renderer auto-picks the first numeric column for stat tiles, so the + // inspector must reduce that same column rather than report a false null. + assert.strictEqual(yield* reduceWith("sum", "not_a_column", THREE_POINTS), 15) + }), + ) + + it.effect("returns null when there is no numeric value at all", () => + Effect.gen(function* () { + assert.strictEqual(yield* reduceWith("sum", "count", timeseries([])), null) + }), + ) + + it.effect("reduces breakdown rows through the `value` field", () => + Effect.gen(function* () { + const rows = breakdown([ + { name: "api", value: 10 }, + { name: "web", value: 5 }, + ]) + assert.strictEqual(yield* reduceWith("sum", "value", rows), 15) + assert.strictEqual(yield* reduceWith("max", "value", rows), 10) + }), + ) + + it.effect("reduces a breakdown row selected by name", () => + Effect.gen(function* () { + const rows = breakdown([ + { name: "api", value: 10 }, + { name: "web", value: 5 }, + ]) + assert.strictEqual(yield* reduceWith("sum", "web", rows), 5) + }), + ) + + it.effect("omits reducedValue entirely when the widget has no reduceToValue transform", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ params: { queries: [tracesDraft()] } }), + stubs(THREE_POINTS), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.isUndefined(outcome.data.queries[0]?.reducedValue) + }), + ) +}) + +// --- isSingleAllGroup / EMPTY_GROUPING ------------------------------------- + +const groupedDraft = (groupBy: ReadonlyArray) => + tracesDraft({ + addOns: { groupBy: true, having: false, orderBy: false, limit: false, legend: false }, + groupBy: [...groupBy], + }) + +describe("isSingleAllGroup (via the EMPTY_GROUPING flag)", () => { + it.effect("flags a requested grouping that collapsed to a single `all` series", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ params: { queries: [groupedDraft(["service"])] } }), + stubs( + timeseries([ + { bucket: "2026-04-01 00:00:00", series: { all: 3 } }, + { bucket: "2026-04-01 01:00:00", series: { all: 7 } }, + ]), + ), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.include(outcome.data.flags, "EMPTY_GROUPING") + }), + ) + + it.effect("flags a breakdown whose only row is `all`", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ + endpoint: BREAKDOWN_ENDPOINT, + params: { queries: [groupedDraft(["service"])] }, + }), + stubs(breakdown([{ name: "all", value: 42 }])), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.include(outcome.data.flags, "EMPTY_GROUPING") + }), + ) + + it.effect("does not flag real group series", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ params: { queries: [groupedDraft(["service"])] } }), + stubs( + timeseries([ + { bucket: "2026-04-01 00:00:00", series: { api: 3, web: 1 } }, + { bucket: "2026-04-01 01:00:00", series: { api: 4, web: 2 } }, + ]), + ), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.notInclude(outcome.data.flags, "EMPTY_GROUPING") + }), + ) + + it.effect("does not flag an intentionally ungrouped chart that yields one `all` series", () => + Effect.gen(function* () { + // No groupBy add-on requested — one "all" series is the expected total. + const outcome = yield* run( + makeWidget({ params: { queries: [tracesDraft()] } }), + stubs( + timeseries([ + { bucket: "2026-04-01 00:00:00", series: { all: 3 } }, + { bucket: "2026-04-01 01:00:00", series: { all: 7 } }, + ]), + ), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.notInclude(outcome.data.flags, "EMPTY_GROUPING") + }), + ) + + it.effect("does not flag a groupBy list holding only the `none`/`all` sentinels", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ params: { queries: [groupedDraft(["none"])] } }), + stubs( + timeseries([ + { bucket: "2026-04-01 00:00:00", series: { all: 3 } }, + { bucket: "2026-04-01 01:00:00", series: { all: 7 } }, + ]), + ), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.notInclude(outcome.data.flags, "EMPTY_GROUPING") + }), + ) +}) + +// --- METRIC_NOT_FOUND ------------------------------------------------------ + +const metricsWidget = makeWidget({ + params: { + queries: [ + { + id: "q1", + name: "Metric query", + dataSource: "metrics", + aggregation: "avg", + metricName: "my.custom.metric", + metricType: "gauge", + }, + ], + }, +}) + +describe("METRIC_NOT_FOUND detection", () => { + it.effect("replaces EMPTY with METRIC_NOT_FOUND when the metric is absent from the catalog", () => + Effect.gen(function* () { + const outcome = yield* run(metricsWidget, stubs(timeseries([]), ["some.other.metric"])) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.include(outcome.data.flags, "METRIC_NOT_FOUND") + assert.notInclude(outcome.data.flags, "EMPTY") + }), + ) + + it.effect("keeps EMPTY when the metric does exist in the catalog", () => + Effect.gen(function* () { + const outcome = yield* run(metricsWidget, stubs(timeseries([]), ["my.custom.metric"])) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.include(outcome.data.flags, "EMPTY") + assert.notInclude(outcome.data.flags, "METRIC_NOT_FOUND") + }), + ) + + it.effect("assumes the metric exists when the catalog lookup fails", () => + Effect.gen(function* () { + const outcome = yield* run( + metricsWidget, + Layer.mergeAll(engineReturning(timeseries([])), catalogFailing), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.include(outcome.data.flags, "EMPTY") + assert.notInclude(outcome.data.flags, "METRIC_NOT_FOUND") + }), + ) + + it.effect("never raises METRIC_NOT_FOUND for a non-metrics query", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ params: { queries: [tracesDraft()] } }), + stubs(timeseries([]), []), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.include(outcome.data.flags, "EMPTY") + assert.notInclude(outcome.data.flags, "METRIC_NOT_FOUND") + }), + ) +}) + +// --- non-supported outcomes ------------------------------------------------ + +describe("inspectWidget outcomes", () => { + it.effect("returns `unsupported` for a predefined endpoint", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ endpoint: "service_overview", params: {} }), + stubs(timeseries([])), + ) + assert.strictEqual(outcome.kind, "unsupported") + if (outcome.kind !== "unsupported") return + assert.strictEqual(outcome.endpoint, "service_overview") + }), + ) + + it.effect("skips a widget with no params", () => + Effect.gen(function* () { + const outcome = yield* run(makeWidget({}), stubs(timeseries([]))) + assert.strictEqual(outcome.kind, "skipped") + if (outcome.kind !== "skipped") return + assert.strictEqual(outcome.reason, "no_params") + }), + ) + + it.effect("skips a widget whose params don't decode", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ params: { queries: [{ nope: true }] } }), + stubs(timeseries([])), + ) + assert.strictEqual(outcome.kind, "skipped") + if (outcome.kind !== "skipped") return + assert.strictEqual(outcome.reason, "decode_failed") + }), + ) + + it.effect("skips a widget with no enabled queries", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ params: { queries: [tracesDraft({ enabled: false })] } }), + stubs(timeseries([])), + ) + assert.strictEqual(outcome.kind, "skipped") + if (outcome.kind !== "skipped") return + assert.strictEqual(outcome.reason, "no_enabled_queries") + }), + ) + + it.effect("skips a widget with more than 5 enabled queries", () => + Effect.gen(function* () { + const queries = Array.from({ length: 6 }, (_, i) => tracesDraft({ id: `q${i}`, name: `Q${i}` })) + const outcome = yield* run(makeWidget({ params: { queries } }), stubs(timeseries([]))) + assert.strictEqual(outcome.kind, "skipped") + if (outcome.kind !== "skipped") return + assert.strictEqual(outcome.reason, "too_many_queries") + }), + ) + + it.effect("returns `inspection_error` (catchCause fallback) when the engine defects", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ params: { queries: [tracesDraft()] } }), + Layer.mergeAll(engineDefect, catalogWith([])), + ) + assert.strictEqual(outcome.kind, "inspection_error") + if (outcome.kind !== "inspection_error") return + assert.include(outcome.message, "engine exploded") + }), + ) + + it.effect("records a per-query error (not a failure) when a query spec cannot be built", () => + Effect.gen(function* () { + // Logs only support `count`; the builder returns `query: null` + error. + const outcome = yield* run( + makeWidget({ + params: { queries: [{ id: "q1", name: "Q1", dataSource: "logs", aggregation: "avg" }] }, + }), + stubs(timeseries([])), + ) + assert.strictEqual(outcome.kind, "supported") + if (outcome.kind !== "supported") return + assert.strictEqual(outcome.data.queries[0]?.status, "error") + assert.include(outcome.data.flags, "EMPTY") + }), + ) + + it.effect("skips a raw_sql_chart widget that has no params.sql", () => + Effect.gen(function* () { + const outcome = yield* run( + makeWidget({ endpoint: "raw_sql_chart", params: {} }), + stubs(timeseries([])), + ) + assert.strictEqual(outcome.kind, "skipped") + if (outcome.kind !== "skipped") return + assert.strictEqual(outcome.reason, "no_params") + }), + ) +}) + +// --- summarizeOutcome (via inspectWidgetsAfterMutation) -------------------- + +const dashboardWith = (widgets: ReadonlyArray) => + ({ + id: "dash-1", + name: "Dash", + timeRange: { type: "relative", value: "6h" }, + widgets, + }) as unknown as DashboardDocument + +describe("summarizeOutcome (via inspectWidgetsAfterMutation)", () => { + it.effect("returns the skipped summary when validate is false", () => + Effect.gen(function* () { + const widget = makeWidget({ params: { queries: [tracesDraft()] } }) + const summary = yield* inspectWidgetsAfterMutation({ + tenant, + dashboard: dashboardWith([widget]), + widgetIds: [widget.id], + validate: false, + }).pipe(Effect.provide(stubs(THREE_POINTS))) + + assert.isFalse(summary.ran) + assert.deepStrictEqual([...summary.inspected], []) + assert.isFalse(summary.capped) + }), + ) + + it.effect("summarizes a healthy supported widget", () => + Effect.gen(function* () { + const widget = makeWidget({ params: { queries: [tracesDraft()] }, title: "Requests" }) + const summary = yield* inspectWidgetsAfterMutation({ + tenant, + dashboard: dashboardWith([widget]), + widgetIds: [widget.id], + validate: true, + }).pipe(Effect.provide(stubs(THREE_POINTS))) + + assert.isTrue(summary.ran) + assert.strictEqual(summary.inspected.length, 1) + assert.strictEqual(summary.inspected[0]?.widgetId, "w1") + assert.strictEqual(summary.inspected[0]?.title, "Requests") + assert.strictEqual(summary.inspected[0]?.verdict, "looks_healthy") + assert.strictEqual(summary.healthyCount, 1) + assert.strictEqual(summary.skippedCount, 0) + // A dashboard timeRange resolved successfully. + assert.strictEqual(summary.timeRange?.source, "dashboard") + }), + ) + + it.effect("maps unsupported / skipped / error outcomes to their verdicts", () => + Effect.gen(function* () { + const unsupported = makeWidget({ id: "w-unsup", endpoint: "service_overview", params: {} }) + const skipped = makeWidget({ id: "w-skip" }) + const summary = yield* inspectWidgetsAfterMutation({ + tenant, + dashboard: dashboardWith([unsupported, skipped]), + widgetIds: ["w-unsup", "w-skip"], + validate: true, + }).pipe(Effect.provide(stubs(THREE_POINTS))) + + assert.strictEqual(summary.inspected[0]?.verdict, "unsupported") + assert.include(summary.inspected[0]?.note ?? "", "service_overview") + assert.strictEqual(summary.inspected[1]?.verdict, "skipped") + assert.include(summary.inspected[1]?.note ?? "", "cannot inspect") + // Both unsupported and skipped land in `skippedCount`. + assert.strictEqual(summary.skippedCount, 2) + }), + ) + + it.effect("maps an inspection defect to the `error` verdict", () => + Effect.gen(function* () { + const widget = makeWidget({ params: { queries: [tracesDraft()] } }) + const summary = yield* inspectWidgetsAfterMutation({ + tenant, + dashboard: dashboardWith([widget]), + widgetIds: [widget.id], + validate: true, + }).pipe(Effect.provide(Layer.mergeAll(engineDefect, catalogWith([])))) + + assert.strictEqual(summary.inspected[0]?.verdict, "error") + assert.include(summary.inspected[0]?.note ?? "", "Inspection failed") + assert.strictEqual(summary.skippedCount, 1) + }), + ) + + it.effect("counts a suspicious widget and ignores unknown widget ids", () => + Effect.gen(function* () { + const widget = makeWidget({ params: { queries: [tracesDraft()] } }) + const summary = yield* inspectWidgetsAfterMutation({ + tenant, + dashboard: dashboardWith([widget]), + widgetIds: [widget.id, "does-not-exist"], + validate: true, + }).pipe(Effect.provide(stubs(timeseries([])))) + + assert.strictEqual(summary.inspected.length, 1) + assert.strictEqual(summary.suspiciousCount + summary.brokenCount, 1) + }), + ) + + it.effect("caps the inspected widget list at maxWidgets", () => + Effect.gen(function* () { + const widgets = Array.from({ length: 3 }, (_, i) => + makeWidget({ id: `w${i}`, params: { queries: [tracesDraft()] } }), + ) + const summary = yield* inspectWidgetsAfterMutation({ + tenant, + dashboard: dashboardWith(widgets), + widgetIds: widgets.map((w) => w.id), + validate: true, + maxWidgets: 2, + }).pipe(Effect.provide(stubs(THREE_POINTS))) + + assert.isTrue(summary.capped) + assert.strictEqual(summary.inspected.length, 2) + }), + ) +}) diff --git a/apps/api/src/mcp/lib/inspect-widget.ts b/apps/api/src/mcp/lib/inspect-widget.ts index 1b2e98ab6..76b1a9be4 100644 --- a/apps/api/src/mcp/lib/inspect-widget.ts +++ b/apps/api/src/mcp/lib/inspect-widget.ts @@ -25,6 +25,7 @@ import { import { resolveDashboardTimeRange, type DashboardTimeRangeInput } from "./resolve-dashboard-time-range" import { resolveTimeRange } from "./time" import { autoBucketSeconds, runRawSql } from "./run-raw-sql" +import { toMcpQueryError } from "./map-warehouse-error" import type { DashboardDocument, DashboardWidgetSchema } from "@maple/domain/http" import type { InspectChartDataData, @@ -34,6 +35,7 @@ import type { WidgetInspectionEntry, WidgetInspectionSummary, WidgetInspectionVerdict, + WarehouseError, } from "@maple/domain" import type { TenantContext } from "@/lib/tenant-context" @@ -264,6 +266,11 @@ const metricExistsInCatalog = Effect.fn("metricExistsInCatalog")(function* ( (m) => m.metricName === metricName, ), ), + Effect.tapCause((cause) => + Effect.logDebug("[inspect-widget] metric catalog lookup failed; assuming metric exists").pipe( + Effect.annotateLogs({ metricName, metricType, cause }), + ), + ), Effect.orElseSucceed(() => true), ) }) @@ -308,6 +315,9 @@ export interface InspectWidgetInput { timeRange: InspectWidgetTimeRange } +/** `@maple/http/errors/WarehouseAuthError` → `WarehouseAuthError`. */ +const warehouseErrorLabel = (error: WarehouseError): string => error._tag.split("/").pop() ?? error._tag + /** * Inspect a raw_sql_chart widget by running its stored SQL through the exact * same macro-expansion + safety pass + warehouse execution as the dashboard UI, @@ -346,7 +356,15 @@ const inspectRawSqlWidget = Effect.fn("inspectRawSqlWidget")(function* ( Effect.catchTag("@maple/http/errors/RawSqlValidationError", (error) => Effect.succeed({ ok: false as const, error: `${error.code}: ${error.message}` }), ), - Effect.catch((error) => Effect.succeed({ ok: false as const, error: error.message })), + // Everything left is a `WarehouseSqlError`; keep the tag so the agent can + // tell auth/config/quota apart from a genuinely bad query, and reuse the + // shared mapper so BYO-ClickHouse schema drift still gets its hint. + Effect.catch((error) => + Effect.succeed({ + ok: false as const, + error: `${warehouseErrorLabel(error)}: ${toMcpQueryError(RAW_SQL_ENDPOINT)(error).message}`, + }), + ), ) if (!result.ok) { @@ -714,10 +732,13 @@ export const inspectWidget = Effect.fn("inspectWidget")( return { kind: "supported", data } satisfies InspectionOutcome }, Effect.catchCause((cause) => - Effect.succeed({ - kind: "inspection_error", - message: Cause.pretty(cause), - }), + Effect.logDebug("[inspect-widget] widget inspection failed").pipe( + Effect.annotateLogs({ cause }), + Effect.as({ + kind: "inspection_error", + message: Cause.pretty(cause), + }), + ), ), ) @@ -838,13 +859,14 @@ export const inspectWidgetsAfterMutation = Effect.fn("inspectWidgetsAfterMutatio const capped = targets.length > maxWidgets const toInspect = capped ? targets.slice(0, maxWidgets) : targets - const resolved = resolveDashboardTimeRange(dashboard.timeRange as DashboardTimeRangeInput) - const timeRange: InspectWidgetTimeRange = resolved - ? { startTime: resolved.startTime, endTime: resolved.endTime, source: "dashboard" } - : (() => { - const fallback = resolveTimeRange(undefined, undefined, 6) - return { startTime: fallback.st, endTime: fallback.et, source: "fallback" as const } - })() + const resolved = yield* resolveDashboardTimeRange(dashboard.timeRange as DashboardTimeRangeInput) + let timeRange: InspectWidgetTimeRange + if (resolved) { + timeRange = { startTime: resolved.startTime, endTime: resolved.endTime, source: "dashboard" } + } else { + const fallback = yield* resolveTimeRange(undefined, undefined, 6) + timeRange = { startTime: fallback.st, endTime: fallback.et, source: "fallback" } + } const outcomes = yield* Effect.forEach( toInspect, @@ -894,7 +916,12 @@ export const inspectWidgetsAfterMutation = Effect.fn("inspectWidgetsAfterMutatio } return summary }, - Effect.catchCause(() => Effect.succeed(SKIPPED_SUMMARY)), + Effect.catchCause((cause) => + Effect.logError("[inspect-widget] post-mutation widget validation failed").pipe( + Effect.annotateLogs({ cause }), + Effect.as(SKIPPED_SUMMARY), + ), + ), ) /** diff --git a/apps/api/src/mcp/lib/query-warehouse.ts b/apps/api/src/mcp/lib/query-warehouse.ts index b0e33a53e..a8180dc5b 100644 --- a/apps/api/src/mcp/lib/query-warehouse.ts +++ b/apps/api/src/mcp/lib/query-warehouse.ts @@ -1,10 +1,10 @@ import { HttpServerRequest } from "effect/unstable/http" import type { WarehouseQueryName } from "@maple/domain" -import { Context, Effect } from "effect" +import { Context, Effect, Schema } from "effect" import { resolveMcpTenantContext } from "@/mcp/lib/resolve-tenant" import type { TenantContext } from "@/lib/tenant-context" import { toMcpQueryError } from "@/mcp/lib/map-warehouse-error" -import { McpAuthMissingError } from "@/mcp/tools/types" +import { McpAuthMissingError, McpQueryError } from "@/mcp/tools/types" import { WarehouseQueryService } from "@/lib/WarehouseQueryService" import { WarehouseExecutor } from "@maple/query-engine/observability" import { makeWarehouseExecutorFromTenant } from "@/lib/WarehouseQueryService" @@ -25,14 +25,19 @@ export const resolveTenant = CurrentMcpTenant /** Infrastructure binding: resolves tenant and provides WarehouseExecutor layer. */ export const withTenantExecutor = (effect: Effect.Effect) => - Effect.fn("withTenantExecutor")(function* () { + Effect.gen(function* () { const tenant = yield* resolveTenant return yield* Effect.provide(effect, makeWarehouseExecutorFromTenant(tenant)) - })() + }).pipe(Effect.withSpan("withTenantExecutor")) +/** + * Pass `rowSchema` to validate the warehouse rows; without it the rows are + * returned as an unchecked `T[]` cast. + */ export const queryWarehouse = Effect.fn("queryWarehouse")(function* ( pipe: WarehouseQueryName, params?: Record, + rowSchema?: Schema.ConstraintDecoder, ) { const tenant = yield* resolveTenant const service = yield* WarehouseQueryService @@ -40,5 +45,18 @@ export const queryWarehouse = Effect.fn("queryWarehouse")(function* ( .query(tenant, { pipeName: pipe, params }) .pipe(Effect.mapError(toMcpQueryError(pipe))) - return { data: response.data as T[] } + if (rowSchema === undefined) return { data: response.data as T[] } + + const rows = yield* Schema.decodeUnknownEffect(Schema.Array(rowSchema))(response.data).pipe( + Effect.mapError( + (error) => + new McpQueryError({ + message: `Unexpected ${pipe} response shape: ${String(error)}`, + pipeName: pipe, + cause: error, + }), + ), + ) + + return { data: rows as T[] } }) diff --git a/apps/api/src/mcp/lib/resolve-dashboard-time-range.ts b/apps/api/src/mcp/lib/resolve-dashboard-time-range.ts index c0987b2b3..b00275986 100644 --- a/apps/api/src/mcp/lib/resolve-dashboard-time-range.ts +++ b/apps/api/src/mcp/lib/resolve-dashboard-time-range.ts @@ -1,4 +1,5 @@ import * as DateTime from "effect/DateTime" +import { Effect, Option } from "effect" const formatUtc = (dt: DateTime.DateTime): string => DateTime.formatIso(dt).replace("T", " ").slice(0, 19) @@ -37,32 +38,31 @@ export type DashboardTimeRangeInput = * Returns `null` for unrecognized relative shorthands so the caller can fall * back to a sensible default window. */ -export function resolveDashboardTimeRange(timeRange: DashboardTimeRangeInput): ResolvedTimeRange | null { +export function resolveDashboardTimeRange( + timeRange: DashboardTimeRangeInput, +): Effect.Effect { if (timeRange.type === "absolute") { const start = DateTime.make(timeRange.startTime) const end = DateTime.make(timeRange.endTime) - if (start._tag === "None" || end._tag === "None") return null - return { + if (Option.isNone(start) || Option.isNone(end)) return Effect.succeed(null) + return Effect.succeed({ startTime: formatUtc(start.value), endTime: formatUtc(end.value), - } + }) } const trimmed = timeRange.value.trim().toLowerCase() - if (!trimmed) return null + if (!trimmed) return Effect.succeed(null) const match = trimmed.match(RELATIVE_PATTERN) - if (!match) return null + if (!match) return Effect.succeed(null) const amount = Number.parseInt(match[1], 10) - if (!Number.isFinite(amount) || amount <= 0) return null + if (!Number.isFinite(amount) || amount <= 0) return Effect.succeed(null) const unit = match[2] as RelativeUnit - const now = DateTime.nowUnsafe() - const start = subtractRelative(now, amount, unit) - - return { - startTime: formatUtc(start), + return Effect.map(DateTime.now, (now) => ({ + startTime: formatUtc(subtractRelative(now, amount, unit)), endTime: formatUtc(now), - } + })) } diff --git a/apps/api/src/mcp/lib/resolve-tenant.test.ts b/apps/api/src/mcp/lib/resolve-tenant.test.ts new file mode 100644 index 000000000..dcf87b801 --- /dev/null +++ b/apps/api/src/mcp/lib/resolve-tenant.test.ts @@ -0,0 +1,500 @@ +// Tests for the MCP transport authentication path: internal-service tokens, +// API-key / MCP-OAuth bearer tokens (audience binding + scope rules), agent +// actor resolution, and the Clerk/session fallback. Every McpAuth* failure +// branch is asserted through `Exit.findErrorOption` so the tagged error type +// (not just "it failed") is checked. + +import { assert, describe, it } from "@effect/vitest" +import { Effect, Exit, Layer, Option, Redacted, Schema } from "effect" +import { OrgId, RoleName, UserId } from "@maple/domain/http" +import { Env, type EnvShape } from "@/lib/Env" +import { AuthService, type AuthServiceShape } from "@/services/AuthService" +import { ApiKeysService, type ResolvedApiKey } from "@/services/ApiKeysService" +import type { TenantContext } from "@/lib/tenant-context" +import { mcpResourceForRequest, resolveMcpTenantContext } from "./resolve-tenant" + +const asOrgId = Schema.decodeUnknownSync(OrgId) +const asUserId = Schema.decodeUnknownSync(UserId) +const asRoleName = Schema.decodeUnknownSync(RoleName) + +const MCP_URL = "https://api.maple.dev/mcp" +const RESOURCE = "https://api.maple.dev/mcp" +const INTERNAL_TOKEN = "s3cret-internal" +const ACTOR_UUID = "11111111-2222-4333-8444-555555555555" +const OTHER_ACTOR_UUID = "99999999-8888-4777-8666-555555555555" + +const request = (headers: Record = {}, url = MCP_URL) => new Request(url, { headers }) + +// --- stub layers ----------------------------------------------------------- + +const envLayer = (over: Partial = {}) => + Layer.succeed(Env, { + INTERNAL_SERVICE_TOKEN: Option.none(), + MAPLE_ORG_ID_OVERRIDE: Option.none(), + ...over, + } as unknown as EnvShape) + +const withInternalToken = (token = INTERNAL_TOKEN, orgOverride?: string) => + envLayer({ + INTERNAL_SERVICE_TOKEN: Option.some(Redacted.make(token)), + MAPLE_ORG_ID_OVERRIDE: orgOverride === undefined ? Option.none() : Option.some(orgOverride), + }) + +const die = () => Effect.die(new Error("not stubbed for this test")) + +const apiKeysLayer = (resolveByBearer: (token: string | undefined) => Effect.Effect) => + Layer.succeed(ApiKeysService, { + get: die, + list: die, + create: die, + roll: die, + revoke: die, + resolveByKey: die, + resolveByBearer, + touchLastUsed: die, + } as never) + +/** No API key matches the bearer — the request falls through to session auth. */ +const noApiKey = apiKeysLayer(() => Effect.succeed(Option.none())) + +const apiKeyReturning = (key: Partial) => + apiKeysLayer(() => + Effect.succeed( + Option.some({ + orgId: asOrgId("org_key"), + userId: asUserId("user_key"), + keyId: "key_1", + kind: "api", + metadataJson: null, + scopes: null, + roles: null, + cliManaged: false, + mcpOAuthResource: null, + ...key, + } as ResolvedApiKey), + ), + ) + +const authLayer = (resolveMcpTenant: AuthServiceShape["resolveMcpTenant"]) => + Layer.succeed(AuthService, { + resolveTenant: die, + resolveMcpTenant, + loginSelfHosted: die, + getUserEmail: die, + getCustomerData: die, + } as unknown as AuthServiceShape) + +const sessionTenant: TenantContext = { + orgId: asOrgId("org_session"), + userId: asUserId("user_session"), + roles: [asRoleName("admin")], + authMode: "clerk", +} + +const sessionOk = authLayer(() => Effect.succeed(sessionTenant)) +const sessionFails = authLayer(() => + Effect.fail({ message: "no session cookie", _tag: "UnauthorizedError" } as never), +) + +/** Everything the resolver can need; individual tests override a slice. */ +const layers = (over: { + env?: Layer.Layer + apiKeys?: Layer.Layer + auth?: Layer.Layer +} = {}) => Layer.mergeAll(over.env ?? envLayer(), over.apiKeys ?? noApiKey, over.auth ?? sessionOk) + +const resolve = (req: Request, layer: Layer.Layer) => + resolveMcpTenantContext(req).pipe(Effect.provide(layer)) + +const failureOf = (exit: Exit.Exit) => Option.getOrUndefined(Exit.findErrorOption(exit)) + +// --- mcpResourceForRequest ------------------------------------------------- + +describe("mcpResourceForRequest", () => { + it("derives the resource from the request URL", () => { + assert.strictEqual(mcpResourceForRequest(request()), "https://api.maple.dev/mcp") + }) + + it("prefers forwarded proto/host over the URL", () => { + const req = request({ "x-forwarded-proto": "https", "x-forwarded-host": "edge.maple.dev" }) + assert.strictEqual(mcpResourceForRequest(req), "https://edge.maple.dev/mcp") + }) + + it("uses only the first value of a comma-separated forwarded header", () => { + const req = request({ + "x-forwarded-proto": "https, http", + "x-forwarded-host": "edge.maple.dev, internal", + }) + assert.strictEqual(mcpResourceForRequest(req), "https://edge.maple.dev/mcp") + }) +}) + +// --- internal service token ------------------------------------------------ + +describe("internal service auth", () => { + it.effect("resolves the tenant from x-org-id when the token matches", () => + Effect.gen(function* () { + const tenant = yield* resolve( + request({ authorization: `Bearer maple_svc_${INTERNAL_TOKEN}`, "x-org-id": "org_abc" }), + layers({ env: withInternalToken() }), + ) + assert.strictEqual(tenant.orgId, "org_abc") + assert.strictEqual(tenant.userId, "internal-service") + assert.strictEqual(tenant.authMode, "self_hosted") + assert.deepStrictEqual([...tenant.roles], []) + }), + ) + + it.effect("prefers MAPLE_ORG_ID_OVERRIDE over the x-org-id header", () => + Effect.gen(function* () { + const tenant = yield* resolve( + request({ authorization: `Bearer maple_svc_${INTERNAL_TOKEN}`, "x-org-id": "org_header" }), + layers({ env: withInternalToken(INTERNAL_TOKEN, "org_override") }), + ) + assert.strictEqual(tenant.orgId, "org_override") + }), + ) + + it.effect("fails with McpAuthMissingError when the server token is unconfigured", () => + Effect.gen(function* () { + const exit = yield* resolve( + request({ authorization: `Bearer maple_svc_${INTERNAL_TOKEN}`, "x-org-id": "org_abc" }), + layers(), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpAuthMissingError") + assert.include(error?.message ?? "", "INTERNAL_SERVICE_TOKEN") + }), + ) + + it.effect("fails with McpAuthMissingError when no org id is available", () => + Effect.gen(function* () { + const exit = yield* resolve( + request({ authorization: `Bearer maple_svc_${INTERNAL_TOKEN}` }), + layers({ env: withInternalToken() }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpAuthMissingError") + assert.include(error?.message ?? "", "x-org-id") + }), + ) + + it.effect("fails with McpInvalidTenantError for a malformed org id", () => + Effect.gen(function* () { + // Header values are trimmed by `Headers`, so the untrimmed org id has to + // come from the env override to reach the decoder in a bad shape. + const exit = yield* resolve( + request({ authorization: `Bearer maple_svc_${INTERNAL_TOKEN}` }), + layers({ env: withInternalToken(INTERNAL_TOKEN, " padded ") }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpInvalidTenantError") + assert.strictEqual((error as { field?: string }).field, "orgId") + }), + ) + + it.effect("fails with McpAuthInvalidError on a same-length token mismatch", () => + Effect.gen(function* () { + // Same length as INTERNAL_TOKEN so the comparison reaches timingSafeEqual. + const wrong = "x".repeat(INTERNAL_TOKEN.length) + const exit = yield* resolve( + request({ authorization: `Bearer maple_svc_${wrong}`, "x-org-id": "org_abc" }), + layers({ env: withInternalToken() }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpAuthInvalidError") + assert.include(error?.message ?? "", "mismatch") + }), + ) + + it.effect("fails with McpAuthInvalidError (not a defect) on a different-length token", () => + Effect.gen(function* () { + // timingSafeEqual throws on unequal buffer lengths — the length guard must + // short-circuit before it is reached. + const exit = yield* resolve( + request({ authorization: "Bearer maple_svc_short", "x-org-id": "org_abc" }), + layers({ env: withInternalToken() }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpAuthInvalidError") + }), + ) +}) + +// --- API key / MCP OAuth --------------------------------------------------- + +const bearer = (token = "maple_key_live") => request({ authorization: `Bearer ${token}` }) + +describe("api key auth", () => { + it.effect("accepts a legacy full-access key and defaults roles to root", () => + Effect.gen(function* () { + const tenant = yield* resolve(bearer(), layers({ apiKeys: apiKeyReturning({}) })) + assert.strictEqual(tenant.orgId, "org_key") + assert.strictEqual(tenant.userId, "user_key") + assert.deepStrictEqual([...tenant.roles], ["root"]) + assert.isUndefined(tenant.actorId) + }), + ) + + it.effect("keeps the key's pinned roles when present", () => + Effect.gen(function* () { + const tenant = yield* resolve( + bearer(), + layers({ apiKeys: apiKeyReturning({ roles: [asRoleName("member")] }) }), + ) + assert.deepStrictEqual([...tenant.roles], ["member"]) + }), + ) + + it.effect("rejects a scoped (restricted) non-OAuth key", () => + Effect.gen(function* () { + const exit = yield* resolve( + bearer(), + layers({ apiKeys: apiKeyReturning({ scopes: ["traces:read"] }) }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpAuthInvalidError") + assert.strictEqual((error as { reason?: string }).reason, "insufficient_scope") + }), + ) + + it.effect("accepts an audience-bound MCP OAuth token", () => + Effect.gen(function* () { + const tenant = yield* resolve( + bearer(), + layers({ + apiKeys: apiKeyReturning({ + kind: "mcp" as ResolvedApiKey["kind"], + mcpOAuthResource: RESOURCE, + scopes: ["mcp:tools"], + }), + }), + ) + assert.strictEqual(tenant.orgId, "org_key") + }), + ) + + it.effect("rejects an OAuth token bound to a different resource", () => + Effect.gen(function* () { + const exit = yield* resolve( + bearer(), + layers({ + apiKeys: apiKeyReturning({ + kind: "mcp" as ResolvedApiKey["kind"], + mcpOAuthResource: "https://evil.example.com/mcp", + scopes: ["mcp:tools"], + }), + }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpAuthInvalidError") + assert.strictEqual((error as { reason?: string }).reason, "invalid_target") + }), + ) + + it.effect("rejects an OAuth token without the mcp:tools scope", () => + Effect.gen(function* () { + const exit = yield* resolve( + bearer(), + layers({ + apiKeys: apiKeyReturning({ + kind: "mcp" as ResolvedApiKey["kind"], + mcpOAuthResource: RESOURCE, + scopes: ["traces:read"], + }), + }), + ).pipe(Effect.exit) + + assert.strictEqual( + (failureOf(exit) as { reason?: string })?.reason, + "invalid_target", + ) + }), + ) + + it.effect("rejects an OAuth-resource token whose kind is not `mcp`", () => + Effect.gen(function* () { + const exit = yield* resolve( + bearer(), + layers({ + apiKeys: apiKeyReturning({ mcpOAuthResource: RESOURCE, scopes: ["mcp:tools"] }), + }), + ).pipe(Effect.exit) + + assert.strictEqual( + (failureOf(exit) as { reason?: string })?.reason, + "invalid_target", + ) + }), + ) + + it.effect("binds the audience to the forwarded host, not the raw URL", () => + Effect.gen(function* () { + const tenant = yield* resolve( + request({ + authorization: "Bearer maple_key_live", + "x-forwarded-proto": "https", + "x-forwarded-host": "edge.maple.dev", + }), + layers({ + apiKeys: apiKeyReturning({ + kind: "mcp" as ResolvedApiKey["kind"], + mcpOAuthResource: "https://edge.maple.dev/mcp", + scopes: ["mcp:tools"], + }), + }), + ) + assert.strictEqual(tenant.orgId, "org_key") + }), + ) + + it.effect("fails with McpAuthInvalidError when the key lookup itself fails", () => + Effect.gen(function* () { + const exit = yield* resolve( + bearer(), + layers({ apiKeys: apiKeysLayer(() => Effect.fail({ message: "db down" } as never)) }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpAuthInvalidError") + assert.strictEqual((error as { reason?: string }).reason, "api_key_lookup") + assert.include(error?.message ?? "", "db down") + }), + ) + + it.effect("fails with McpInvalidTenantError for a key holding a malformed org id", () => + Effect.gen(function* () { + const exit = yield* resolve( + bearer(), + layers({ apiKeys: apiKeyReturning({ orgId: " bad " as ResolvedApiKey["orgId"] }) }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpInvalidTenantError") + assert.strictEqual((error as { field?: string }).field, "orgId") + }), + ) + + it.effect("fails with McpInvalidTenantError for a key holding a malformed user id", () => + Effect.gen(function* () { + const exit = yield* resolve( + bearer(), + layers({ apiKeys: apiKeyReturning({ userId: "" as ResolvedApiKey["userId"] }) }), + ).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpInvalidTenantError") + assert.strictEqual((error as { field?: string }).field, "userId") + }), + ) +}) + +// --- actor id resolution --------------------------------------------------- + +describe("agent actor resolution", () => { + it.effect("uses the key's pinned agentActorId metadata", () => + Effect.gen(function* () { + const tenant = yield* resolve( + bearer(), + layers({ + apiKeys: apiKeyReturning({ + metadataJson: JSON.stringify({ agentActorId: ACTOR_UUID }), + }), + }), + ) + assert.strictEqual(tenant.actorId, ACTOR_UUID) + }), + ) + + it.effect("prefers the x-maple-agent-id header over the key metadata", () => + Effect.gen(function* () { + const tenant = yield* resolve( + request({ + authorization: "Bearer maple_key_live", + "x-maple-agent-id": OTHER_ACTOR_UUID, + }), + layers({ + apiKeys: apiKeyReturning({ + metadataJson: JSON.stringify({ agentActorId: ACTOR_UUID }), + }), + }), + ) + assert.strictEqual(tenant.actorId, OTHER_ACTOR_UUID) + }), + ) + + it.effect("drops a malformed actor id instead of failing the request", () => + Effect.gen(function* () { + const tenant = yield* resolve( + request({ authorization: "Bearer maple_key_live", "x-maple-agent-id": "not-a-uuid" }), + layers({ apiKeys: apiKeyReturning({}) }), + ) + assert.isUndefined(tenant.actorId) + }), + ) + + it.effect("ignores unparseable metadata JSON", () => + Effect.gen(function* () { + const tenant = yield* resolve( + bearer(), + layers({ apiKeys: apiKeyReturning({ metadataJson: "{not json" }) }), + ) + assert.isUndefined(tenant.actorId) + }), + ) + + it.effect("ignores metadata JSON that is not an object or lacks agentActorId", () => + Effect.gen(function* () { + const arrayMeta = yield* resolve( + bearer(), + layers({ apiKeys: apiKeyReturning({ metadataJson: JSON.stringify([ACTOR_UUID]) }) }), + ) + assert.isUndefined(arrayMeta.actorId) + + const wrongType = yield* resolve( + bearer(), + layers({ apiKeys: apiKeyReturning({ metadataJson: JSON.stringify({ agentActorId: 7 }) }) }), + ) + assert.isUndefined(wrongType.actorId) + }), + ) +}) + +// --- session fallback ------------------------------------------------------ + +describe("session auth fallback", () => { + it.effect("falls back to AuthService when no API key matches", () => + Effect.gen(function* () { + const tenant = yield* resolve(request({ cookie: "__session=abc" }), layers()) + assert.strictEqual(tenant.orgId, "org_session") + assert.strictEqual(tenant.userId, "user_session") + assert.strictEqual(tenant.authMode, "clerk") + assert.deepStrictEqual([...tenant.roles], ["admin"]) + }), + ) + + it.effect("fails with McpAuthInvalidError(session_auth_fallback) when the session is invalid", () => + Effect.gen(function* () { + const exit = yield* resolve(request(), layers({ auth: sessionFails })).pipe(Effect.exit) + + const error = failureOf(exit) + assert.strictEqual(error?._tag, "@maple/mcp/errors/McpAuthInvalidError") + assert.strictEqual((error as { reason?: string }).reason, "session_auth_fallback") + assert.include(error?.message ?? "", "no session cookie") + }), + ) + + it.effect("treats a non-Bearer authorization header as no token", () => + Effect.gen(function* () { + const tenant = yield* resolve(request({ authorization: "Basic abc123" }), layers()) + assert.strictEqual(tenant.orgId, "org_session") + }), + ) +}) diff --git a/apps/api/src/mcp/lib/resolve-tenant.ts b/apps/api/src/mcp/lib/resolve-tenant.ts index f71afa4d2..78fec8d06 100644 --- a/apps/api/src/mcp/lib/resolve-tenant.ts +++ b/apps/api/src/mcp/lib/resolve-tenant.ts @@ -16,16 +16,14 @@ const apiKeyDefaultRoles = [decodeRoleNameSync("root")] const AGENT_ACTOR_HEADER = "x-maple-agent-id" +const decodeJsonValue = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.Unknown)) + const extractAgentActorIdFromMetadata = (metadataJson: string | null): string | null => { if (!metadataJson) return null - try { - const parsed = JSON.parse(metadataJson) - if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { - const candidate = (parsed as Record).agentActorId - return typeof candidate === "string" ? candidate : null - } - } catch { - // fall through + const parsed = Option.getOrUndefined(decodeJsonValue(metadataJson)) + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + const candidate = (parsed as Record).agentActorId + return typeof candidate === "string" ? candidate : null } return null } @@ -61,7 +59,7 @@ export const mcpResourceForRequest = (request: Request) => { return `${protocol}://${host}/mcp` } -export const resolveMcpTenantContext = Effect.fn("resolveMcpTenantContext")(function* (request: Request) { +export const resolveMcpTenantContext = Effect.fnUntraced(function* (request: Request) { const token = getBearerToken(request.headers) // Internal service auth (e.g. chat agent) diff --git a/apps/api/src/mcp/lib/time.test.ts b/apps/api/src/mcp/lib/time.test.ts index bb7788c4a..66fe5359f 100644 --- a/apps/api/src/mcp/lib/time.test.ts +++ b/apps/api/src/mcp/lib/time.test.ts @@ -1,6 +1,12 @@ -import { describe, expect, it } from "vitest" +import { assert, describe, it } from "@effect/vitest" +import { Effect } from "effect" +import { TestClock } from "effect/testing" +import { expect } from "vitest" import { formatClampNote, normalizeTime, resolveTimeRange } from "./time" +// 2026-03-30 12:00:00 UTC — every default-window assertion derives from this. +const NOW = Date.UTC(2026, 2, 30, 12, 0, 0) + describe("normalizeTime", () => { it("passes through already-correct format", () => { expect(normalizeTime("2026-03-30 14:30:00")).toBe("2026-03-30 14:30:00") @@ -36,57 +42,72 @@ describe("normalizeTime", () => { }) describe("resolveTimeRange", () => { - it("normalizes both provided values", () => { - const { st, et } = resolveTimeRange("2026-03-30T10:00:00Z", "2026-03-30T16:00:00Z") - expect(st).toBe("2026-03-30 10:00:00") - expect(et).toBe("2026-03-30 16:00:00") - }) - - it("returns default window when neither is provided", () => { - const { st, et } = resolveTimeRange(undefined, undefined) - // Both should match YYYY-MM-DD HH:mm:ss format - expect(st).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/) - expect(et).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/) - // Default window is 6 hours - const startMs = new Date(st.replace(" ", "T") + "Z").getTime() - const endMs = new Date(et.replace(" ", "T") + "Z").getTime() - const diffHours = (endMs - startMs) / (1000 * 60 * 60) - expect(diffHours).toBeCloseTo(6, 0) - }) - - it("normalizes start and uses default end when only start provided", () => { - const { st, et } = resolveTimeRange("2026-03-30T10:00:00+09:00", undefined) - expect(st).toBe("2026-03-30 01:00:00") - expect(et).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/) - }) - - it("uses default start and normalizes end when only end provided", () => { - const { st, et } = resolveTimeRange(undefined, "2026-03-30T16:00:00Z") - expect(st).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/) - expect(et).toBe("2026-03-30 16:00:00") - }) - - it("preserves numeric third-arg as defaultHours (back-compat)", () => { - const { st, et } = resolveTimeRange(undefined, undefined, 1) - const startMs = new Date(st.replace(" ", "T") + "Z").getTime() - const endMs = new Date(et.replace(" ", "T") + "Z").getTime() - expect((endMs - startMs) / 3600_000).toBeCloseTo(1, 0) - }) - - it("clamps start when range exceeds maxHours", () => { - const result = resolveTimeRange("2026-03-01T00:00:00Z", "2026-03-30T00:00:00Z", { maxHours: 24 * 7 }) - expect(result.clamped).toBe(true) - expect(result.et).toBe("2026-03-30 00:00:00") - expect(result.st).toBe("2026-03-23 00:00:00") - expect(result.maxHours).toBe(24 * 7) - }) - - it("does not clamp when range is within maxHours", () => { - const result = resolveTimeRange("2026-03-29T00:00:00Z", "2026-03-30T00:00:00Z", { maxHours: 24 * 7 }) - expect(result.clamped).toBe(false) - expect(result.st).toBe("2026-03-29 00:00:00") - expect(result.et).toBe("2026-03-30 00:00:00") - }) + it.effect("normalizes both provided values", () => + Effect.gen(function* () { + const { st, et } = yield* resolveTimeRange("2026-03-30T10:00:00Z", "2026-03-30T16:00:00Z") + assert.strictEqual(st, "2026-03-30 10:00:00") + assert.strictEqual(et, "2026-03-30 16:00:00") + }), + ) + + it.effect("returns default 6h window when neither is provided", () => + Effect.gen(function* () { + yield* TestClock.setTime(NOW) + const { st, et } = yield* resolveTimeRange(undefined, undefined) + assert.strictEqual(st, "2026-03-30 06:00:00") + assert.strictEqual(et, "2026-03-30 12:00:00") + }), + ) + + it.effect("normalizes start and uses the clock for end when only start provided", () => + Effect.gen(function* () { + yield* TestClock.setTime(NOW) + const { st, et } = yield* resolveTimeRange("2026-03-30T10:00:00+09:00", undefined) + assert.strictEqual(st, "2026-03-30 01:00:00") + assert.strictEqual(et, "2026-03-30 12:00:00") + }), + ) + + it.effect("uses default start and normalizes end when only end provided", () => + Effect.gen(function* () { + yield* TestClock.setTime(NOW) + const { st, et } = yield* resolveTimeRange(undefined, "2026-03-30T16:00:00Z") + assert.strictEqual(st, "2026-03-30 06:00:00") + assert.strictEqual(et, "2026-03-30 16:00:00") + }), + ) + + it.effect("preserves numeric third-arg as defaultHours (back-compat)", () => + Effect.gen(function* () { + yield* TestClock.setTime(NOW) + const { st, et } = yield* resolveTimeRange(undefined, undefined, 1) + assert.strictEqual(st, "2026-03-30 11:00:00") + assert.strictEqual(et, "2026-03-30 12:00:00") + }), + ) + + it.effect("clamps start when range exceeds maxHours", () => + Effect.gen(function* () { + const result = yield* resolveTimeRange("2026-03-01T00:00:00Z", "2026-03-30T00:00:00Z", { + maxHours: 24 * 7, + }) + assert.isTrue(result.clamped) + assert.strictEqual(result.et, "2026-03-30 00:00:00") + assert.strictEqual(result.st, "2026-03-23 00:00:00") + assert.strictEqual(result.maxHours, 24 * 7) + }), + ) + + it.effect("does not clamp when range is within maxHours", () => + Effect.gen(function* () { + const result = yield* resolveTimeRange("2026-03-29T00:00:00Z", "2026-03-30T00:00:00Z", { + maxHours: 24 * 7, + }) + assert.isFalse(result.clamped) + assert.strictEqual(result.st, "2026-03-29 00:00:00") + assert.strictEqual(result.et, "2026-03-30 00:00:00") + }), + ) }) describe("formatClampNote", () => { diff --git a/apps/api/src/mcp/lib/time.ts b/apps/api/src/mcp/lib/time.ts index b847d155c..d4cbdd6df 100644 --- a/apps/api/src/mcp/lib/time.ts +++ b/apps/api/src/mcp/lib/time.ts @@ -1,5 +1,5 @@ import * as DateTime from "effect/DateTime" -import { Option } from "effect" +import { Effect, Option } from "effect" const formatUtc = (dt: DateTime.DateTime): string => DateTime.formatIso(dt).replace("T", " ").slice(0, 19) @@ -25,8 +25,7 @@ export function normalizeTime(input: string): string { const DEFAULT_HOURS = 6 -function defaultTimeRange(hours = DEFAULT_HOURS) { - const now = DateTime.nowUnsafe() +function defaultTimeRange(now: DateTime.Utc, hours = DEFAULT_HOURS) { const start = DateTime.subtract(now, { hours }) return { startTime: formatUtc(start), @@ -74,28 +73,30 @@ export function resolveTimeRange( startTime: string | undefined, endTime: string | undefined, opts: ResolveTimeRangeOptions | number = {}, -): ResolvedTimeRange { - const { defaultHours = DEFAULT_HOURS, maxHours } = - typeof opts === "number" ? { defaultHours: opts, maxHours: undefined } : opts - - const defaults = defaultTimeRange(defaultHours) - let st = startTime ? normalizeTime(startTime) : defaults.startTime - let et = endTime ? normalizeTime(endTime) : defaults.endTime - - let clamped = false - if (maxHours !== undefined && maxHours > 0) { - const stMs = toEpochMs(st) - const etMs = toEpochMs(et) - if (stMs !== undefined && etMs !== undefined) { - const maxMs = maxHours * 3600 * 1000 - if (etMs - stMs > maxMs) { - st = fromEpochMs(etMs - maxMs) - clamped = true +): Effect.Effect { + return Effect.map(DateTime.now, (now) => { + const { defaultHours = DEFAULT_HOURS, maxHours } = + typeof opts === "number" ? { defaultHours: opts, maxHours: undefined } : opts + + const defaults = defaultTimeRange(now, defaultHours) + let st = startTime ? normalizeTime(startTime) : defaults.startTime + let et = endTime ? normalizeTime(endTime) : defaults.endTime + + let clamped = false + if (maxHours !== undefined && maxHours > 0) { + const stMs = toEpochMs(st) + const etMs = toEpochMs(et) + if (stMs !== undefined && etMs !== undefined) { + const maxMs = maxHours * 3600 * 1000 + if (etMs - stMs > maxMs) { + st = fromEpochMs(etMs - maxMs) + clamped = true + } } } - } - return { st, et, clamped, maxHours } + return { st, et, clamped, maxHours } + }) } /** diff --git a/apps/api/src/mcp/tools/__tests__/dashboard-concurrency.test.ts b/apps/api/src/mcp/tools/__tests__/dashboard-concurrency.test.ts index e7bfa8dec..50197e4d3 100644 --- a/apps/api/src/mcp/tools/__tests__/dashboard-concurrency.test.ts +++ b/apps/api/src/mcp/tools/__tests__/dashboard-concurrency.test.ts @@ -91,10 +91,11 @@ describe("dashboard concurrency", () => { const layer = makeLayer(testDb) return Effect.gen(function* () { - yield* DashboardPersistenceService.upsert(ORG, USER, seed()) + const dashboards = yield* DashboardPersistenceService + yield* dashboards.upsert(ORG, USER, seed()) const addWidget = (widgetId: string) => - DashboardPersistenceService.mutate(ORG, USER, DASHBOARD, (existing) => + dashboards.mutate(ORG, USER, DASHBOARD, (existing) => Effect.succeed( new DashboardDocument({ ...existing, @@ -106,7 +107,7 @@ describe("dashboard concurrency", () => { yield* Effect.all([addWidget("w-a"), addWidget("w-b")], { concurrency: 2 }) - const listed = yield* DashboardPersistenceService.list(ORG) + const listed = yield* dashboards.list(ORG) assert.strictEqual(listed.dashboards.length, 1) const widgets = listed.dashboards[0]!.widgets.map((w) => w.id).sort() @@ -119,8 +120,10 @@ describe("dashboard concurrency", () => { const layer = makeLayer(testDb) return Effect.gen(function* () { + const dashboards = yield* DashboardPersistenceService + // Establish baseline at version=1. - yield* DashboardPersistenceService.upsert(ORG, USER, seed({ name: "Initial" })) + yield* dashboards.upsert(ORG, USER, seed({ name: "Initial" })) // Fire two upserts concurrently. Both will read the same version // before either writes. The first commit wins the CAS; the second @@ -128,7 +131,7 @@ describe("dashboard concurrency", () => { const exits = yield* Effect.all( [ Effect.exit( - DashboardPersistenceService.upsert( + dashboards.upsert( ORG, USER, seed({ @@ -140,7 +143,7 @@ describe("dashboard concurrency", () => { ), ), Effect.exit( - DashboardPersistenceService.upsert( + dashboards.upsert( ORG, USER, seed({ @@ -155,7 +158,7 @@ describe("dashboard concurrency", () => { { concurrency: 2 }, ) - const exitsAndListed = { exits, listed: yield* DashboardPersistenceService.list(ORG) } + const exitsAndListed = { exits, listed: yield* dashboards.list(ORG) } const successes = exitsAndListed.exits.filter(Exit.isSuccess) const failures = exitsAndListed.exits.filter(Exit.isFailure) @@ -181,7 +184,9 @@ describe("dashboard concurrency", () => { const layer = makeLayer(testDb) return Effect.gen(function* () { - yield* DashboardPersistenceService.upsert(ORG, USER, seed({ name: "Initial" })) + const dashboards = yield* DashboardPersistenceService + + yield* dashboards.upsert(ORG, USER, seed({ name: "Initial" })) // Race two upserts so we deterministically observe at least one // CAS conflict. (`upsert` re-reads on every call, so the only way @@ -190,7 +195,7 @@ describe("dashboard concurrency", () => { const exits = yield* Effect.all( [ Effect.exit( - DashboardPersistenceService.upsert( + dashboards.upsert( ORG, USER, seed({ @@ -202,7 +207,7 @@ describe("dashboard concurrency", () => { ), ), Effect.exit( - DashboardPersistenceService.upsert( + dashboards.upsert( ORG, USER, seed({ @@ -220,10 +225,10 @@ describe("dashboard concurrency", () => { // Recovery path: refetch fresh state and re-apply the loser's // edit on top of it. This is exactly what the web hook does in // response to a `DashboardConcurrencyError`. - const fresh = yield* DashboardPersistenceService.list(ORG) + const fresh = yield* dashboards.list(ORG) const current = fresh.dashboards[0]! - yield* DashboardPersistenceService.upsert( + yield* dashboards.upsert( ORG, USER, new DashboardDocument({ @@ -233,7 +238,7 @@ describe("dashboard concurrency", () => { }), ) - const listed = yield* DashboardPersistenceService.list(ORG) + const listed = yield* dashboards.list(ORG) // At least one writer should have hit a CAS conflict. We don't assert // on which — under serialized scheduling either A or B can win — only diff --git a/apps/api/src/mcp/tools/compare-periods.ts b/apps/api/src/mcp/tools/compare-periods.ts index 0e6df4ad5..9c06c6fab 100644 --- a/apps/api/src/mcp/tools/compare-periods.ts +++ b/apps/api/src/mcp/tools/compare-periods.ts @@ -53,7 +53,7 @@ export function registerComparePeriodsTool(server: McpToolRegistrar) { curEt = new Date(center.getTime() + halfWindow).toISOString().replace("T", " ").slice(0, 19) } else { // Resolve current period - const current = resolveTimeRange(current_start, current_end, 1) + const current = yield* resolveTimeRange(current_start, current_end, 1) curSt = current.st curEt = current.et diff --git a/apps/api/src/mcp/tools/create-alert-rule.ts b/apps/api/src/mcp/tools/create-alert-rule.ts index d03cd33ad..57d01d2ae 100644 --- a/apps/api/src/mcp/tools/create-alert-rule.ts +++ b/apps/api/src/mcp/tools/create-alert-rule.ts @@ -106,7 +106,7 @@ function buildAlertRuleRequest( let signalType = params.signal_type let comparator = params.comparator let threshold = params.threshold - let windowMinutes = params.window_minutes ?? 5 + const windowMinutes = params.window_minutes ?? 5 let templateDefaults: Record = {} if (params.template && params.template !== "custom") { @@ -349,16 +349,15 @@ export function registerCreateAlertRuleTool(server: McpToolRegistrar) { const alerts = yield* AlertsService const rule = yield* alerts.createRule(tenant.orgId, tenant.userId, tenant.roles, decoded).pipe( - Effect.catchTag("@maple/http/errors/AlertValidationError", (error) => - Effect.fail( - new McpQueryError({ - message: `${error._tag}: ${error.message}\n${error.details.join("\n")}`, - pipeName: "create_alert_rule", - cause: error, - }), - ), - ), Effect.catchTags({ + "@maple/http/errors/AlertValidationError": (error) => + Effect.fail( + new McpQueryError({ + message: `${error._tag}: ${error.message}\n${error.details.join("\n")}`, + pipeName: "create_alert_rule", + cause: error, + }), + ), "@maple/http/errors/AlertForbiddenError": (error) => Effect.fail( new McpQueryError({ diff --git a/apps/api/src/mcp/tools/create-dashboard.ts b/apps/api/src/mcp/tools/create-dashboard.ts index 765e37f68..7137763e8 100644 --- a/apps/api/src/mcp/tools/create-dashboard.ts +++ b/apps/api/src/mcp/tools/create-dashboard.ts @@ -1,5 +1,5 @@ import { McpQueryError, optionalStringParam, requiredStringParam, type McpToolRegistrar } from "./types" -import { Effect, Schema } from "effect" +import { Effect, Option, Schema } from "effect" import { createDualContent } from "../lib/structured-output" import { resolveTenant } from "@/mcp/lib/query-warehouse" import { DashboardPersistenceService } from "@/services/DashboardPersistenceService" @@ -22,6 +22,7 @@ import type { TemplateParameterValues, WidgetDef } from "@/dashboard-templates" const decodePortableDashboard = Schema.decodeUnknownEffect(PortableDashboardDocument) const PortableDashboardFromJson = Schema.fromJsonString(PortableDashboardDocument) const decodeParamKey = Schema.decodeUnknownSync(DashboardTemplateParameterKey) +const decodeJsonValue = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.Unknown)) // --------------------------------------------------------------------------- // Simplified widget specs path — MCP-only, parses JSON tool input @@ -246,12 +247,11 @@ function computeAutoLayout(specs: SimpleWidgetSpec[]): Array<{ x: number; y: num } function parseSimpleWidgets(json: string): WidgetDef[] | string { - let specs: SimpleWidgetSpec[] - try { - specs = JSON.parse(json) - } catch { + const parsed = decodeJsonValue(json) + if (Option.isNone(parsed)) { return "Invalid widgets JSON. Expected a JSON array of widget specs." } + const specs = parsed.value as SimpleWidgetSpec[] if (!Array.isArray(specs) || specs.length === 0) { return "widgets must be a non-empty JSON array." diff --git a/apps/api/src/mcp/tools/diagnose-service.ts b/apps/api/src/mcp/tools/diagnose-service.ts index 33d6210a9..c97e16cb7 100644 --- a/apps/api/src/mcp/tools/diagnose-service.ts +++ b/apps/api/src/mcp/tools/diagnose-service.ts @@ -20,7 +20,7 @@ export function registerDiagnoseServiceTool(server: McpToolRegistrar) { environment: optionalStringParam("Filter by deployment environment (e.g. production, staging)"), }), Effect.fn("McpTool.diagnoseService")(function* ({ service_name, start_time, end_time, environment }) { - const { st, et } = resolveTimeRange(start_time, end_time) + const { st, et } = yield* resolveTimeRange(start_time, end_time) const tenant = yield* resolveTenant const result = yield* diagnoseService({ diff --git a/apps/api/src/mcp/tools/error-detail.ts b/apps/api/src/mcp/tools/error-detail.ts index e123a22f1..bf5055209 100644 --- a/apps/api/src/mcp/tools/error-detail.ts +++ b/apps/api/src/mcp/tools/error-detail.ts @@ -39,7 +39,7 @@ export function registerErrorDetailTool(server: McpToolRegistrar) { include_timeseries, limit, }) { - const { st, et } = resolveTimeRange(start_time, end_time) + const { st, et } = yield* resolveTimeRange(start_time, end_time) const tenant = yield* resolveTenant const result = yield* errorDetail({ diff --git a/apps/api/src/mcp/tools/explore-attributes.ts b/apps/api/src/mcp/tools/explore-attributes.ts index d0faa09b7..bf9767e4d 100644 --- a/apps/api/src/mcp/tools/explore-attributes.ts +++ b/apps/api/src/mcp/tools/explore-attributes.ts @@ -35,7 +35,7 @@ export function registerExploreAttributesTool(server: McpToolRegistrar) { limit: optionalNumberParam("Max results (default 50)"), }), Effect.fn("McpTool.exploreAttributes")(function* (params) { - const range = resolveTimeRange(params.start_time, params.end_time, { maxHours: 24 * 30 }) + const range = yield* resolveTimeRange(params.start_time, params.end_time, { maxHours: 24 * 30 }) const { st, et } = range const lim = clampLimit(params.limit, { defaultValue: 50, max: 500 }) const scope = (params.scope ?? "span") as "span" | "resource" diff --git a/apps/api/src/mcp/tools/find-errors.ts b/apps/api/src/mcp/tools/find-errors.ts index ed37f469f..7fbab80a3 100644 --- a/apps/api/src/mcp/tools/find-errors.ts +++ b/apps/api/src/mcp/tools/find-errors.ts @@ -21,7 +21,7 @@ export function registerFindErrorsTool(server: McpToolRegistrar) { limit: optionalNumberParam("Max results (default 20)"), }), Effect.fn("McpTool.findErrors")(function* ({ start_time, end_time, service, environment, limit }) { - const { st, et } = resolveTimeRange(start_time, end_time) + const { st, et } = yield* resolveTimeRange(start_time, end_time) const tenant = yield* resolveTenant const errors = yield* findErrors({ diff --git a/apps/api/src/mcp/tools/find-slow-traces.ts b/apps/api/src/mcp/tools/find-slow-traces.ts index d6ddc44d0..546b36092 100644 --- a/apps/api/src/mcp/tools/find-slow-traces.ts +++ b/apps/api/src/mcp/tools/find-slow-traces.ts @@ -27,7 +27,7 @@ export function registerFindSlowTracesTool(server: McpToolRegistrar) { environment, limit, }) { - const range = resolveTimeRange(start_time, end_time, { maxHours: 24 * 7 }) + const range = yield* resolveTimeRange(start_time, end_time, { maxHours: 24 * 7 }) const { st, et } = range const lim = clampLimit(limit, { defaultValue: 10, max: 100 }) diff --git a/apps/api/src/mcp/tools/get-instrumentation-recommendations.ts b/apps/api/src/mcp/tools/get-instrumentation-recommendations.ts index 3372079b3..2282eda81 100644 --- a/apps/api/src/mcp/tools/get-instrumentation-recommendations.ts +++ b/apps/api/src/mcp/tools/get-instrumentation-recommendations.ts @@ -147,7 +147,7 @@ export function registerGetInstrumentationRecommendationsTool(server: McpToolReg // Coverage degrades gracefully: if the warehouse is unavailable the issue list // (possibly stale) still renders, with the coverage section marked unavailable. const wantCoverage = include_coverage !== false - const range = resolveTimeRange(undefined, undefined, { defaultHours: 24 }) + const range = yield* resolveTimeRange(undefined, undefined, { defaultHours: 24 }) const resourceKeysOpt = wantCoverage ? yield* exploreAttributeKeys({ source: "traces", diff --git a/apps/api/src/mcp/tools/get-service-top-operations.ts b/apps/api/src/mcp/tools/get-service-top-operations.ts index fd0301ec4..447277cd7 100644 --- a/apps/api/src/mcp/tools/get-service-top-operations.ts +++ b/apps/api/src/mcp/tools/get-service-top-operations.ts @@ -40,7 +40,7 @@ export function registerGetServiceTopOperationsTool(server: McpToolRegistrar) { end_time, limit, }) { - const range = resolveTimeRange(start_time, end_time, { maxHours: 24 * 7 }) + const range = yield* resolveTimeRange(start_time, end_time, { maxHours: 24 * 7 }) const { st, et } = range const metricOption = metric === undefined ? Option.some("count" as const) : decodeTracesMetric(metric) diff --git a/apps/api/src/mcp/tools/inspect-chart-data.ts b/apps/api/src/mcp/tools/inspect-chart-data.ts index 43b8f5550..48376bb4f 100644 --- a/apps/api/src/mcp/tools/inspect-chart-data.ts +++ b/apps/api/src/mcp/tools/inspect-chart-data.ts @@ -243,10 +243,10 @@ export function registerInspectChartDataTool(server: McpToolRegistrar) { let timeRange: InspectWidgetTimeRange if (start_time && end_time) { - const range = resolveTimeRange(start_time, end_time) + const range = yield* resolveTimeRange(start_time, end_time) timeRange = { startTime: range.st, endTime: range.et, source: "override" } } else { - const resolved = resolveDashboardTimeRange(dashboard.timeRange as DashboardTimeRangeInput) + const resolved = yield* resolveDashboardTimeRange(dashboard.timeRange as DashboardTimeRangeInput) if (resolved) { timeRange = { startTime: resolved.startTime, @@ -254,7 +254,7 @@ export function registerInspectChartDataTool(server: McpToolRegistrar) { source: "dashboard", } } else { - const fallback = resolveTimeRange(undefined, undefined, 6) + const fallback = yield* resolveTimeRange(undefined, undefined, 6) timeRange = { startTime: fallback.st, endTime: fallback.et, source: "fallback" } } } diff --git a/apps/api/src/mcp/tools/list-metrics.ts b/apps/api/src/mcp/tools/list-metrics.ts index f1b1c4dc4..2d352fa69 100644 --- a/apps/api/src/mcp/tools/list-metrics.ts +++ b/apps/api/src/mcp/tools/list-metrics.ts @@ -31,7 +31,7 @@ export function registerListMetricsTool(server: McpToolRegistrar) { offset, limit, }) { - const range = resolveTimeRange(start_time, end_time, { maxHours: 24 * 30 }) + const range = yield* resolveTimeRange(start_time, end_time, { maxHours: 24 * 30 }) const { st, et } = range const lim = clampLimit(limit, { defaultValue: 50, max: 500 }) const off = clampOffset(offset, { max: 10_000 }) diff --git a/apps/api/src/mcp/tools/list-services.ts b/apps/api/src/mcp/tools/list-services.ts index f4d4dc436..eb682c5f8 100644 --- a/apps/api/src/mcp/tools/list-services.ts +++ b/apps/api/src/mcp/tools/list-services.ts @@ -19,7 +19,7 @@ export function registerListServicesTool(server: McpToolRegistrar) { environment: optionalStringParam("Filter by deployment environment (e.g. production, staging)"), }), Effect.fn("McpTool.listServices")(function* ({ start_time, end_time, environment }) { - const { st, et } = resolveTimeRange(start_time, end_time) + const { st, et } = yield* resolveTimeRange(start_time, end_time) const tenant = yield* resolveTenant yield* Effect.annotateCurrentSpan({ orgId: tenant.orgId, diff --git a/apps/api/src/mcp/tools/mine-log-patterns.ts b/apps/api/src/mcp/tools/mine-log-patterns.ts index 6018912fb..efedab370 100644 --- a/apps/api/src/mcp/tools/mine-log-patterns.ts +++ b/apps/api/src/mcp/tools/mine-log-patterns.ts @@ -37,7 +37,7 @@ export function registerMineLogPatternsTool(server: McpToolRegistrar) { sample_size, limit, }) { - const range = resolveTimeRange(start_time, end_time, { maxHours: 24 }) + const range = yield* resolveTimeRange(start_time, end_time, { maxHours: 24 }) const { st, et } = range const sampleSize = Math.min(Math.max(Number(sample_size) || 10_000, 1), 50_000) const lim = Math.min(Math.max(Number(limit) || 50, 1), 200) diff --git a/apps/api/src/mcp/tools/query-data.ts b/apps/api/src/mcp/tools/query-data.ts index 1ee72a921..da64ab717 100644 --- a/apps/api/src/mcp/tools/query-data.ts +++ b/apps/api/src/mcp/tools/query-data.ts @@ -8,7 +8,7 @@ import { type McpToolResult, } from "./types" import { resolveTimeRange } from "../lib/time" -import { Effect, Match, Schema } from "effect" +import { Effect, Match, Option, Schema } from "effect" import { resolveTenant } from "@/mcp/lib/query-warehouse" import { QueryEngineService } from "@/services/QueryEngineService" import { @@ -35,11 +35,11 @@ import { type QueryDataQueryContext, } from "@maple/domain" -const asServiceName = Schema.decodeUnknownSync(ServiceName) -const asSpanName = Schema.decodeUnknownSync(SpanName) -const asDeploymentEnvironment = Schema.decodeUnknownSync(DeploymentEnvironment) -const asCommitSha = Schema.decodeUnknownSync(CommitSha) -const asMetricName = Schema.decodeUnknownSync(MetricName) +const asServiceName = Schema.decodeUnknownOption(ServiceName) +const asSpanName = Schema.decodeUnknownOption(SpanName) +const asDeploymentEnvironment = Schema.decodeUnknownOption(DeploymentEnvironment) +const asCommitSha = Schema.decodeUnknownOption(CommitSha) +const asMetricName = Schema.decodeUnknownOption(MetricName) const queryDataSchema = Schema.Struct({ source: Schema.Literals(["traces", "logs", "metrics"]).annotate({ @@ -119,7 +119,7 @@ export function registerQueryDataTool(server: McpToolRegistrar) { queryDataDescription, queryDataSchema, Effect.fn("McpTool.queryData")(function* (params) { - const { st, et } = resolveTimeRange(params.start_time, params.end_time) + const { st, et } = yield* resolveTimeRange(params.start_time, params.end_time) // Validate attribute params if (params.attribute_value && !params.attribute_key) { @@ -146,6 +146,54 @@ export function registerQueryDataTool(server: McpToolRegistrar) { } } + // Branded filter values are decoded up front so a malformed value comes + // back as an actionable tool error naming the field instead of a defect. + const invalidFilters: string[] = [] + const decodeFilter = ( + decode: (input: unknown) => Option.Option, + field: string, + value: string, + ): A | undefined => { + const decoded = decode(value) + if (Option.isNone(decoded)) { + invalidFilters.push(`${field}="${value}"`) + return undefined + } + return decoded.value + } + const decodeFilterList = ( + decode: (input: unknown) => Option.Option, + field: string, + value: string, + ): Array => + splitCsv(value).flatMap((entry) => { + const decoded = decodeFilter(decode, field, entry) + return decoded === undefined ? [] : [decoded] + }) + + const serviceName = params.service_name + ? decodeFilter(asServiceName, "service_name", params.service_name) + : undefined + const spanName = params.span_name + ? decodeFilter(asSpanName, "span_name", params.span_name) + : undefined + const environments = params.environments + ? decodeFilterList(asDeploymentEnvironment, "environments", params.environments) + : undefined + const commitShas = params.commit_shas + ? decodeFilterList(asCommitSha, "commit_shas", params.commit_shas) + : undefined + const metricName = params.metric_name + ? decodeFilter(asMetricName, "metric_name", params.metric_name) + : undefined + + if (invalidFilters.length > 0) { + return validationError( + `Invalid filter value(s): ${invalidFilters.join(", ")}. Use list_services, explore_attributes, or list_metrics to discover valid values.`, + 'service_name="checkout-api" environments="production"', + ) + } + // Track defaults applied for transparency const decisions: string[] = [] @@ -169,17 +217,11 @@ export function registerQueryDataTool(server: McpToolRegistrar) { } const filters: TracesFilters = { - ...(params.service_name && { serviceName: asServiceName(params.service_name) }), - ...(params.span_name && { spanName: asSpanName(params.span_name) }), + ...(serviceName && { serviceName }), + ...(spanName && { spanName }), ...(params.root_spans_only && { rootSpansOnly: params.root_spans_only }), - ...(params.environments && { - environments: splitCsv(params.environments).map((env) => - asDeploymentEnvironment(env), - ), - }), - ...(params.commit_shas && { - commitShas: splitCsv(params.commit_shas).map((sha) => asCommitSha(sha)), - }), + ...(environments && { environments }), + ...(commitShas && { commitShas }), ...(params.group_by === "attribute" && params.attribute_key && { groupByAttributeKeys: [params.attribute_key] }), ...(attributeFilters.length > 0 && { attributeFilters }), @@ -228,7 +270,7 @@ export function registerQueryDataTool(server: McpToolRegistrar) { if (!params.metric) decisions.push(`metric: fixed to "count" (only option for logs)`) const filters: LogsFilters = { - ...(params.service_name && { serviceName: asServiceName(params.service_name) }), + ...(serviceName && { serviceName }), ...(params.severity && { severity: params.severity }), } const hasFilters = Object.keys(filters).length > 0 @@ -264,7 +306,6 @@ export function registerQueryDataTool(server: McpToolRegistrar) { }), Match.when("metrics", (): QuerySpecType => { // metric_name presence is enforced by the validation above. - const metricName = params.metric_name ?? "" const metricType = params.metric_type ?? "sum" const metricsAttributeFilters: Array<{ @@ -282,9 +323,9 @@ export function registerQueryDataTool(server: McpToolRegistrar) { } const filters: MetricsFilters = { - metricName: asMetricName(metricName), + metricName: metricName as MetricName, metricType, - ...(params.service_name && { serviceName: asServiceName(params.service_name) }), + ...(serviceName && { serviceName }), ...(params.group_by === "attribute" && params.attribute_key && { groupByAttributeKey: params.attribute_key }), ...(metricsAttributeFilters.length > 0 && { diff --git a/apps/api/src/mcp/tools/run-sql.ts b/apps/api/src/mcp/tools/run-sql.ts index 144ee007a..cf1a64b3d 100644 --- a/apps/api/src/mcp/tools/run-sql.ts +++ b/apps/api/src/mcp/tools/run-sql.ts @@ -54,7 +54,7 @@ export function registerRunSqlTool(server: McpToolRegistrar) { runSqlSchema, Effect.fn("McpTool.runSql")(function* (params) { const tenant = yield* resolveTenant - const { st, et } = resolveTimeRange(params.start_time, params.end_time) + const { st, et } = yield* resolveTimeRange(params.start_time, params.end_time) const granularitySeconds = params.granularity_seconds ?? autoBucketSeconds(st, et) const outcome = yield* runRawSql({ diff --git a/apps/api/src/mcp/tools/search-logs.ts b/apps/api/src/mcp/tools/search-logs.ts index 929ad408c..6702c3e59 100644 --- a/apps/api/src/mcp/tools/search-logs.ts +++ b/apps/api/src/mcp/tools/search-logs.ts @@ -40,7 +40,7 @@ export function registerSearchLogsTool(server: McpToolRegistrar) { offset, limit, }) { - const range = resolveTimeRange(start_time, end_time, { maxHours: 24 * 7 }) + const range = yield* resolveTimeRange(start_time, end_time, { maxHours: 24 * 7 }) const { st, et } = range const lim = clampLimit(limit, { defaultValue: 30, max: 200 }) const off = clampOffset(offset, { max: 10_000 }) diff --git a/apps/api/src/mcp/tools/search-sessions.ts b/apps/api/src/mcp/tools/search-sessions.ts index b53146543..612e967da 100644 --- a/apps/api/src/mcp/tools/search-sessions.ts +++ b/apps/api/src/mcp/tools/search-sessions.ts @@ -49,7 +49,7 @@ export function registerSearchSessionsTool(server: McpToolRegistrar) { limit: optionalNumberParam("Max results (default 25)"), }), Effect.fn("McpTool.searchSessions")(function* (params) { - const range = resolveTimeRange(params.start_time, params.end_time, { maxHours: 24 * 7 }) + const range = yield* resolveTimeRange(params.start_time, params.end_time, { maxHours: 24 * 7 }) const { st, et } = range const lim = clampLimit(params.limit, { defaultValue: 25, max: 200 }) const off = clampOffset(params.offset, { max: 10_000 }) diff --git a/apps/api/src/mcp/tools/search-traces.ts b/apps/api/src/mcp/tools/search-traces.ts index 4502d4092..fbb34e604 100644 --- a/apps/api/src/mcp/tools/search-traces.ts +++ b/apps/api/src/mcp/tools/search-traces.ts @@ -45,7 +45,7 @@ export function registerSearchTracesTool(server: McpToolRegistrar) { limit: optionalNumberParam("Max results (default 20)"), }), Effect.fn("McpTool.searchTraces")(function* (params) { - const range = resolveTimeRange(params.start_time, params.end_time, { maxHours: 24 * 7 }) + const range = yield* resolveTimeRange(params.start_time, params.end_time, { maxHours: 24 * 7 }) const { st, et } = range const lim = clampLimit(params.limit, { defaultValue: 20, max: 200 }) const off = clampOffset(params.offset, { max: 10_000 }) diff --git a/apps/api/src/mcp/tools/service-map.ts b/apps/api/src/mcp/tools/service-map.ts index 420bb8edf..6f7a3a0ce 100644 --- a/apps/api/src/mcp/tools/service-map.ts +++ b/apps/api/src/mcp/tools/service-map.ts @@ -19,7 +19,7 @@ export function registerServiceMapTool(server: McpToolRegistrar) { environment: optionalStringParam("Filter by deployment environment"), }), Effect.fn("McpTool.serviceMap")(function* ({ start_time, end_time, service_name, environment }) { - const { st, et } = resolveTimeRange(start_time, end_time) + const { st, et } = yield* resolveTimeRange(start_time, end_time) const tenant = yield* resolveTenant yield* Effect.annotateCurrentSpan({ orgId: tenant.orgId, diff --git a/apps/api/src/mcp/tools/types.ts b/apps/api/src/mcp/tools/types.ts index 9ba8b3325..db0d19c5d 100644 --- a/apps/api/src/mcp/tools/types.ts +++ b/apps/api/src/mcp/tools/types.ts @@ -1,10 +1,6 @@ import type { Effect } from "effect" import { Schema } from "effect" -class McpTenantError extends Schema.TaggedErrorClass()("@maple/mcp/errors/McpTenantError", { - message: Schema.String, -}) {} - export class McpAuthMissingError extends Schema.TaggedErrorClass()( "@maple/mcp/errors/McpAuthMissingError", { message: Schema.String, header: Schema.optionalKey(Schema.String) }, @@ -26,7 +22,6 @@ export class McpQueryError extends Schema.TaggedErrorClass()( ) {} export type McpToolError = - | McpTenantError | McpAuthMissingError | McpAuthInvalidError | McpInvalidTenantError diff --git a/apps/api/src/mcp/tools/update-dashboard.ts b/apps/api/src/mcp/tools/update-dashboard.ts index fe1951369..a3725fe50 100644 --- a/apps/api/src/mcp/tools/update-dashboard.ts +++ b/apps/api/src/mcp/tools/update-dashboard.ts @@ -8,7 +8,7 @@ import { IsoDateTimeString } from "@maple/domain" const PortableDashboardFromJson = Schema.fromJsonString(PortableDashboardDocument) const decodeIsoDateTimeString = Schema.decodeUnknownSync(IsoDateTimeString) -const decodeDashboardId = Schema.decodeUnknownSync(DashboardId) +const decodeDashboardId = Schema.decodeUnknownEffect(DashboardId) const TIME_RANGE_MAP: Record = { "1h": "1h", @@ -55,7 +55,16 @@ export function registerUpdateDashboardTool(server: McpToolRegistrar) { ) : null - const dashboardIdBranded = decodeDashboardId(dashboard_id) + const dashboardIdBranded = yield* decodeDashboardId(dashboard_id).pipe( + Effect.mapError( + (cause) => + new McpQueryError({ + message: `Invalid dashboard_id: ${dashboard_id}. Use list_dashboards to find available dashboard IDs.`, + pipeName: "update_dashboard", + cause, + }), + ), + ) const nowMillis = yield* Clock.currentTimeMillis const now = decodeIsoDateTimeString(new Date(nowMillis).toISOString()) diff --git a/apps/api/src/routes/chat.http.ts b/apps/api/src/routes/chat.http.ts index c23d4eab7..a23139593 100644 --- a/apps/api/src/routes/chat.http.ts +++ b/apps/api/src/routes/chat.http.ts @@ -57,7 +57,6 @@ export const HttpChatLive = HttpApiBuilder.group(MapleApi, "chat", (handlers) => const result = yield* definition.handler(decoded).pipe( Effect.catchTags({ "@maple/mcp/errors/McpQueryError": (e) => Effect.succeed(errorResult(e._tag, e.message)), - "@maple/mcp/errors/McpTenantError": (e) => Effect.succeed(errorResult(e._tag, e.message)), "@maple/mcp/errors/McpAuthMissingError": (e) => Effect.succeed(errorResult(e._tag, e.message)), "@maple/mcp/errors/McpAuthInvalidError": (e) =>