diff --git a/.changeset/artifact-hyphenated-tool-paths.md b/.changeset/artifact-hyphenated-tool-paths.md new file mode 100644 index 0000000000..c659cf6787 --- /dev/null +++ b/.changeset/artifact-hyphenated-tool-paths.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +Render artifacts that call integrations or tools with hyphenated slugs. diff --git a/apps/cloud/src/mcp/session-build-semaphore.test.ts b/apps/cloud/src/mcp/session-build-semaphore.test.ts index 3d4ad76343..584b65ee0e 100644 --- a/apps/cloud/src/mcp/session-build-semaphore.test.ts +++ b/apps/cloud/src/mcp/session-build-semaphore.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, beforeEach } from "@effect/vitest"; +import { describe, expect, it, beforeEach, afterEach, vi } from "@effect/vitest"; import { acquireBuildSlot, @@ -13,6 +13,10 @@ describe("session-build-semaphore", () => { resetBuildSlotsForTest(); }); + afterEach(() => { + vi.useRealTimers(); + }); + it("grants up to the cap immediately, with no wait", async () => { const results = await Promise.all([ acquireBuildSlot().promise, @@ -214,6 +218,7 @@ describe("session-build-semaphore", () => { }); it("proceeds without a slot when the queue wait exceeds the timeout, and does not count it as active", async () => { + vi.useFakeTimers(); await Promise.all([ acquireBuildSlot().promise, acquireBuildSlot().promise, @@ -223,6 +228,10 @@ describe("session-build-semaphore", () => { expect(currentActiveBuildsForTest()).toBe(4); const timedOutHandle = acquireBuildSlot(10); + await vi.advanceTimersByTimeAsync(9); + expect(currentQueueLengthForTest()).toBe(1); + expect(currentActiveBuildsForTest()).toBe(4); + await vi.advanceTimersByTimeAsync(1); const result = await timedOutHandle.promise; expect(result).toEqual({ acquired: false, waitMs: expect.any(Number), timedOut: true }); diff --git a/e2e/scenarios/artifact-hyphenated-path.test.ts b/e2e/scenarios/artifact-hyphenated-path.test.ts new file mode 100644 index 0000000000..d2ec32b5df --- /dev/null +++ b/e2e/scenarios/artifact-hyphenated-path.test.ts @@ -0,0 +1,109 @@ +import { randomBytes } from "node:crypto"; +import { expect } from "@effect/vitest"; +import { Effect, Schema } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { + ArtifactId, + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, +} from "@executor-js/sdk/shared"; + +import { createEmulatorInstance } from "../src/emulator-instance"; +import { scenario } from "../src/scenario"; +import { Api, Browser, Mcp, Target } from "../src/services"; +import { visit } from "../src/surfaces/browser"; + +const api = composePluginApi([openApiHttpPlugin()] as const); +const decodeCreatedArtifact = Schema.decodeUnknownSync( + Schema.Struct({ structuredContent: Schema.Struct({ artifactId: ArtifactId }) }), +); + +scenario( + "Artifacts · a hyphenated integration renders live tool data through a bracket path", + { timeout: 180_000 }, + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const mcp = yield* Mcp; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const session = mcp.session(identity); + const slug = IntegrationSlug.make(`artifact-schema-${randomBytes(4).toString("hex")}`); + const baseUrl = yield* createEmulatorInstance("resend", "artifact-path"); + let artifactId: ArtifactId | undefined; + + yield* Effect.gen(function* () { + // The emulator's public schema is a real JSON endpoint. Its descriptive + // title gives the rendered query a stable caller-visible result. + yield* client.openapi.addSpec({ + payload: { + slug, + baseUrl, + spec: { + kind: "blob", + value: JSON.stringify({ + openapi: "3.0.3", + info: { title: "Service schema", version: "1" }, + servers: [{ url: baseUrl }], + paths: { + "/openapi.json": { + get: { + operationId: "readSchema", + responses: { "200": { description: "Schema" } }, + }, + }, + }, + }), + }, + }, + }); + yield* client.connections.create({ + payload: { + owner: "org", + name: ConnectionName.make("public"), + integration: slug, + template: AuthTemplateSlug.make("none"), + values: {}, + }, + }); + const created = yield* session.call("create-artifact", { + title: `Service schema ${slug}`, + code: `function App() { + const query = useQuery(tools['${slug}'].openapiJson.readSchema.queryOptions({})); + return 
{query.isPending ? "Loading schema" : JSON.stringify(query.data ?? query.error)}
; + }`, + }); + expect(created.ok, created.text).toBe(true); + const envelope = decodeCreatedArtifact(created.raw); + artifactId = envelope.structuredContent.artifactId; + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the artifact and read the service schema", async () => { + await visit(page, `/artifacts/${artifactId}`); + const data = page + .frameLocator('[data-testid="artifact-shell-frame"]') + .frameLocator("iframe") + .getByTestId("live-schema"); + await data.waitFor({ timeout: 30_000 }); + await data.filter({ hasText: /Resend/i }).waitFor({ timeout: 30_000 }); + expect(await data.textContent()).toContain("openapi"); + }); + }); + }).pipe( + Effect.ensuring( + Effect.gen(function* () { + if (artifactId !== undefined) + yield* client.artifacts.remove({ params: { artifactId } }).pipe(Effect.ignore); + yield* client.connections + .remove({ + params: { owner: "org", integration: slug, name: ConnectionName.make("public") }, + }) + .pipe(Effect.ignore); + yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); + }), + ), + ); + }), +); diff --git a/packages/hosts/mcp-apps-shell/src/shell/proxy.ts b/packages/hosts/mcp-apps-shell/src/shell/proxy.ts index 6d7744d0c6..b0983344d9 100644 --- a/packages/hosts/mcp-apps-shell/src/shell/proxy.ts +++ b/packages/hosts/mcp-apps-shell/src/shell/proxy.ts @@ -33,12 +33,16 @@ export type RequestTrustedInteraction = ( interaction: TrustedInteraction, ) => Promise; -const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$]*$/; +const TOOL_PATH_IDENTIFIER = /^[A-Za-z_$][\w$]*$/; +const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$-]*$/; + +const formatToolPathSegment = (segment: string): string => + TOOL_PATH_IDENTIFIER.test(segment) ? `.${segment}` : `[${JSON.stringify(segment)}]`; /** * The ONE grammar the shell ever puts on the `execute-action` wire: * - * return await tools.("")?(.)*() + * return await tools("")?*() * * A single proxy-shaped tool call, nothing else — no statements, no loops, no * composition. The server parses `execute-action` against exactly this shape @@ -71,10 +75,12 @@ export function toolCallCode( if (role !== undefined && (typeof role !== "string" || role.length === 0)) { throw new Error("Invalid tool role."); } - const [head, ...rest] = parts; + const head = parts[0]; + if (head === undefined) throw new Error("Invalid tool path."); + const rest = parts.slice(1); const tag = role === undefined ? "" : `(${JSON.stringify(role)})`; - const trailer = rest.length > 0 ? `.${rest.join(".")}` : ""; - return `return await tools.${head}${tag}${trailer}(${JSON.stringify(args[0] ?? {})})`; + const target = `${formatToolPathSegment(head)}${tag}${rest.map(formatToolPathSegment).join("")}`; + return `return await tools${target}(${JSON.stringify(args[0] ?? {})})`; } /** diff --git a/packages/hosts/mcp-apps-shell/src/shell/tool-call-grammar.pin.test.ts b/packages/hosts/mcp-apps-shell/src/shell/tool-call-grammar.pin.test.ts index ed71c22ced..433858fedf 100644 --- a/packages/hosts/mcp-apps-shell/src/shell/tool-call-grammar.pin.test.ts +++ b/packages/hosts/mcp-apps-shell/src/shell/tool-call-grammar.pin.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "@effect/vitest"; -import { parseToolCallCode } from "@executor-js/host-mcp/tool-call-code"; +import { formatToolCallCode, parseToolCallCode } from "@executor-js/host-mcp/tool-call-code"; import { toolCallCode } from "./proxy"; @@ -30,6 +30,11 @@ describe("execute-action tool-call grammar", () => { path: ["search"], args: [{ query: "github issues", limit: 12 }], }, + { + label: "a hyphenated integration slug", + path: ["cloudflare-bindings", "d1_database_query"], + args: [{ database_id: "db", sql: "SELECT 1" }], + }, { label: "an argument with a $ in an identifier-ish key", path: ["mongo", "org", "main", "find"], @@ -82,6 +87,12 @@ describe("execute-action tool-call grammar", () => { }); } + it("formats a resolved hyphenated integration safely", () => { + expect(formatToolCallCode(["cloudflare-bindings", "org", "default", "query"], {})).toBe( + 'return await tools["cloudflare-bindings"].org.default.query({})', + ); + }); + it("refuses to emit a path that would not parse", () => { expect(() => toolCallCode([], [])).toThrow("Invalid tool path."); expect(() => toolCallCode(["github", "issues; drop"], [])).toThrow("Invalid tool path."); diff --git a/packages/hosts/mcp/src/artifact-bindings.test.ts b/packages/hosts/mcp/src/artifact-bindings.test.ts index bbbe7c630a..44e2fafd03 100644 --- a/packages/hosts/mcp/src/artifact-bindings.test.ts +++ b/packages/hosts/mcp/src/artifact-bindings.test.ts @@ -29,6 +29,21 @@ describe("extractArtifactRoles", () => { expect(roles).toEqual([{ role: "vercel", integration: "vercel" }]); }); + it("reads a hyphenated integration from a bracket reference", () => { + const roles = extractArtifactRoles( + `useQuery(tools["cloudflare-bindings"].d1_database_query.queryOptions({ sql: "SELECT 1" }));`, + ); + expect(roles).toEqual([{ role: "cloudflare-bindings", integration: "cloudflare-bindings" }]); + }); + + it("reads a hyphenated integration from single-quoted bracket references", () => { + expect( + extractArtifactRoles( + `useQuery(tools['cloudflare-bindings']('production').query.queryOptions({}));`, + ), + ).toEqual([{ role: "production", integration: "cloudflare-bindings" }]); + }); + it("collapses repeated references to one role", () => { const roles = extractArtifactRoles( `useQuery(tools.linear.issues.list.queryOptions({})); diff --git a/packages/hosts/mcp/src/artifact-bindings.ts b/packages/hosts/mcp/src/artifact-bindings.ts index 9be7a15c39..d513aca047 100644 --- a/packages/hosts/mcp/src/artifact-bindings.ts +++ b/packages/hosts/mcp/src/artifact-bindings.ts @@ -92,14 +92,17 @@ const withCommentsBlanked = (code: string): string => code.replace(/\/\/[^\n]*|\/\*[\s\S]*?\*\//g, (text) => text.replace(/[^\n]/g, " ")); /** - * A `tools.` reference, with the optional role call that follows it. + * A tools root reference, with the optional role call that follows it. * * The role is captured from either quote flavour. Anything else after the root * — property access, a call with an object — is left to the caller's own path * handling; extraction only cares which integration slot is being reached. */ -const TOOLS_REFERENCE = - /(? { const scannable = withCommentsBlanked(code); const found = new Map(); for (const match of scannable.matchAll(TOOLS_REFERENCE)) { - const integration = match[1]; + const integration = match[1] ?? match[2] ?? match[3]; if (integration === undefined || RESERVED_TOOL_ROOTS.has(integration)) continue; - const role = match[2] ?? match[3] ?? integration; + const role = match[4] ?? match[5] ?? integration; if (role.length === 0) continue; if (!found.has(role)) found.set(role, { role, integration }); } diff --git a/packages/hosts/mcp/src/tool-call-code.ts b/packages/hosts/mcp/src/tool-call-code.ts index b95933dcd1..852791a6c0 100644 --- a/packages/hosts/mcp/src/tool-call-code.ts +++ b/packages/hosts/mcp/src/tool-call-code.ts @@ -12,13 +12,13 @@ * the shell ever writes any. So the server parses `execute-action` against the * one grammar the proxy emits: * - * return await tools.("")?(.)*() + * return await tools("")?*() * * One awaited tool call, one JSON-literal argument, nothing else — no * statements, no loops, no composition. `execute` (the model-facing codemode * tool) is untouched; this constraint is only for the app-originated channel. * - * The leading identifier is an INTEGRATION, not a connection: artifact paths + * The leading segment is an INTEGRATION, not a connection: artifact paths * carry no tier and no connection name (see `artifact-bindings.ts`). The * optional string call right after it is the integration ROLE, which is how an * artifact using two accounts of one integration says which it means. Both are @@ -32,16 +32,26 @@ import { Option, Schema } from "effect"; -const TOOL_CALL_CODE = - /^return await tools\.([A-Za-z_$][\w$]*)(?:\((("(?:[^"\\]|\\.)*"))\))?((?:\.[A-Za-z_$][\w$]*)*)\((.*)\);?$/s; +const JSON_STRING_LITERAL = String.raw`"(?:[^"\\]|\\.)*"`; +const IDENTIFIER = String.raw`[A-Za-z_$][\w$]*`; +const SLUG = String.raw`[A-Za-z_$][\w$-]*`; +const PATH_SEGMENT = String.raw`(?:\.${IDENTIFIER}|\["${SLUG}"\])`; +const TOOL_CALL_CODE = new RegExp( + String.raw`^return await tools(${PATH_SEGMENT})(?:\((${JSON_STRING_LITERAL})\))?((?:${PATH_SEGMENT})*)\((.*)\);?$`, + "s", +); +const PATH_SEGMENT_MATCHER = new RegExp(String.raw`(?:\.(${IDENTIFIER})|\["(${SLUG})"\])`, "g"); /** The proxy's argument is always `JSON.stringify` output, so anything that * does not decode is, by construction, not something the proxy emitted. */ const decodeArgs = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.Unknown)); const decodeRole = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.String)); +const decodePath = (serialized: string): readonly string[] => + Array.from(serialized.matchAll(PATH_SEGMENT_MATCHER), (match) => match[1] ?? match[2] ?? ""); + export type ParsedToolCall = { - /** The dotted path segments under `tools`, e.g. `["github", "issues", "create"]`. + /** The path segments under `tools`, e.g. `["github", "issues", "create"]`. * The head is an integration slug (or a system-tool root); it is never a * tier or a connection name. */ readonly path: readonly string[]; @@ -56,7 +66,7 @@ export type ParsedToolCall = { /** The message handed back to the iframe when its code is not a tool call. */ export const TOOL_CALL_CONTRACT_MESSAGE = [ "execute-action accepts a single tool call, not arbitrary code.", - 'The only accepted form is `return await tools.("")?.()` —', + 'The only accepted form is `return await tools("")?()` —', "exactly what the shell's `tools.*` proxy emits.", "Interactive UI reaches integrations declaratively:", "`tools...queryOptions(...)` / `.infiniteQueryOptions(...)` for reads,", @@ -72,14 +82,14 @@ export const parseToolCallCode = (code: string): ParsedToolCall | null => { const match = TOOL_CALL_CODE.exec(code.trim()); if (!match) return null; - const [, root, serializedRole, , dottedRest, serializedArgs] = match; - if (root === undefined || dottedRest === undefined || serializedArgs === undefined) return null; + const [, root, serializedRole, serializedRest, serializedArgs] = match; + if (root === undefined || serializedRest === undefined || serializedArgs === undefined) + return null; const args = decodeArgs(serializedArgs); if (Option.isNone(args)) return null; - const rest = dottedRest.length > 0 ? dottedRest.slice(1).split(".") : []; - const path = [root, ...rest]; + const path = decodePath(`${root}${serializedRest}`); if (serializedRole === undefined) return { path, args: args.value }; @@ -91,7 +101,11 @@ export const parseToolCallCode = (code: string): ParsedToolCall | null => { return { path, role: role.value, args: args.value }; }; -const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$]*$/; +const TOOL_PATH_IDENTIFIER = /^[A-Za-z_$][\w$]*$/; +const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$-]*$/; + +const formatToolPathSegment = (segment: string): string => + TOOL_PATH_IDENTIFIER.test(segment) ? `.${segment}` : `[${JSON.stringify(segment)}]`; /** * Build the codemode call for a RESOLVED address — the full @@ -115,5 +129,5 @@ export const formatToolCallCode = (path: readonly string[], args: unknown): stri throw new Error("Invalid resolved tool path."); } } - return `return await tools.${path.join(".")}(${JSON.stringify(args ?? {})})`; + return `return await tools${path.map(formatToolPathSegment).join("")}(${JSON.stringify(args ?? {})})`; };