From d301f13dbc83b93f3b0180295667ba614a949366 Mon Sep 17 00:00:00 2001 From: GijungKim Date: Sun, 30 Aug 2026 10:19:25 -0400 Subject: [PATCH 1/2] test: cover hyphenated paths in browser --- .changeset/artifact-hyphenated-tool-paths.md | 5 ++ .../src/shell/mcp-app.browser.test.ts | 46 ++++++++++++ .../hosts/mcp-apps-shell/src/shell/proxy.ts | 21 ++++-- .../src/shell/tool-call-grammar.pin.test.ts | 13 +++- .../hosts/mcp/src/artifact-bindings.test.ts | 15 ++++ packages/hosts/mcp/src/artifact-bindings.ts | 48 +++++++++---- .../hosts/mcp/src/artifacts-tools.test.ts | 18 +++++ packages/hosts/mcp/src/tool-call-code.ts | 70 +++++++++++++++---- 8 files changed, 201 insertions(+), 35 deletions(-) create mode 100644 .changeset/artifact-hyphenated-tool-paths.md 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/packages/hosts/mcp-apps-shell/src/shell/mcp-app.browser.test.ts b/packages/hosts/mcp-apps-shell/src/shell/mcp-app.browser.test.ts index acf79110e7..921a519aa4 100644 --- a/packages/hosts/mcp-apps-shell/src/shell/mcp-app.browser.test.ts +++ b/packages/hosts/mcp-apps-shell/src/shell/mcp-app.browser.test.ts @@ -1170,6 +1170,11 @@ const HARNESS_BINDINGS: ArtifactBindings = { owner: "org", connection: ConnectionName.make(INVENTORY_CONNECTION), }, + "cloudflare-bindings": { + integration: IntegrationSlug.make("cloudflare-bindings"), + owner: "org", + connection: ConnectionName.make("main"), + }, alt: { integration: IntegrationSlug.make(INVENTORY_SLUG), owner: "org", @@ -1582,6 +1587,47 @@ describe("MCP app generated UI browser isolation", () => { } }, 30_000); + it("serializes a hyphenated integration slug with bracket notation", async () => { + if (!browser || !hostServer) throw new Error("Browser harness did not start."); + const { page, shellFrame } = await openHarness(browser, hostServer.url); + + try { + const innerFrame = await renderGeneratedUi( + page, + shellFrame, + `function App() { + const query = useQuery({ + ...tools["cloudflare-bindings"].d1_database_query.queryOptions({ sql: "SELECT 1" }), + retry: false, + }); + return
{query.isLoading ? "loading" : query.isError ? "error" : "done"}
; + }`, + ); + await innerFrame.locator("#status").waitFor({ timeout: 10_000 }); + await page.waitForFunction(() => + (window as unknown as BrowserHostWindow).__mcpHostState.toolCalls.some( + (call) => + call.arguments?.code === + 'return await tools["cloudflare-bindings"].d1_database_query({"sql":"SELECT 1"})', + ), + ); + + expect((await getHostState(page)).toolCalls).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: "execute-action", + arguments: { + code: 'return await tools["cloudflare-bindings"].d1_database_query({"sql":"SELECT 1"})', + artifactId: HARNESS_ARTIFACT_ID, + }, + }), + ]), + ); + } finally { + await page.close(); + } + }, 30_000); + // An artifact that uses two accounts of one integration tags each call site // with a role. The role has to survive four hops — the inner proxy's `apply` // trap, the TanStack cache key, the postMessage bridge, and the diff --git a/packages/hosts/mcp-apps-shell/src/shell/proxy.ts b/packages/hosts/mcp-apps-shell/src/shell/proxy.ts index 6d7744d0c6..7391d1a2a8 100644 --- a/packages/hosts/mcp-apps-shell/src/shell/proxy.ts +++ b/packages/hosts/mcp-apps-shell/src/shell/proxy.ts @@ -33,12 +33,19 @@ export type RequestTrustedInteraction = ( interaction: TrustedInteraction, ) => Promise; -const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$]*$/; +const TOOL_PATH_IDENTIFIER = /^[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 segment is either `.identifier` or `["JSON-escaped slug"]`. The bracket + * form lets integrations and tools with names such as `cloudflare-bindings` + * use the same narrow channel without making their slug executable source. * * A single proxy-shaped tool call, nothing else — no statements, no loops, no * composition. The server parses `execute-action` against exactly this shape @@ -63,7 +70,7 @@ export function toolCallCode( ): string { if (path.length === 0) throw new Error("Invalid tool path."); const parts = path.map((part) => { - if (typeof part !== "string" || !TOOL_PATH_SEGMENT.test(part)) { + if (typeof part !== "string" || part.length === 0) { throw new Error("Invalid tool path."); } return part; @@ -71,10 +78,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..52bbca5845 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 @@ -30,6 +30,16 @@ 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: "a path segment requiring JSON escaping", + path: ["inventory", 'items"]; return await tools.evil.run({}); //'], + args: [{}], + }, { label: "an argument with a $ in an identifier-ish key", path: ["mongo", "org", "main", "find"], @@ -84,8 +94,7 @@ describe("execute-action tool-call grammar", () => { 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."); - expect(() => toolCallCode(["github", "1bad"], [])).toThrow("Invalid tool path."); + expect(() => toolCallCode(["github", ""], [])).toThrow("Invalid tool path."); expect(() => toolCallCode(["github", "issues"], [], "")).toThrow("Invalid tool role."); }); diff --git a/packages/hosts/mcp/src/artifact-bindings.test.ts b/packages/hosts/mcp/src/artifact-bindings.test.ts index bbbe7c630a..c650a944e0 100644 --- a/packages/hosts/mcp/src/artifact-bindings.test.ts +++ b/packages/hosts/mcp/src/artifact-bindings.test.ts @@ -29,6 +29,13 @@ 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("collapses repeated references to one role", () => { const roles = extractArtifactRoles( `useQuery(tools.linear.issues.list.queryOptions({})); @@ -106,6 +113,14 @@ describe("oldStyleAddressRejection", () => { ).toContain("tools.inventory.org."); }); + it("rejects a bracketed old-style address", () => { + expect( + oldStyleAddressRejection( + `useQuery(tools["cloudflare-bindings"].org.main.d1_database_query.queryOptions({}));`, + ), + ).toContain('tools["cloudflare-bindings"].org.'); + }); + it("accepts the short form", () => { expect( oldStyleAddressRejection(`useQuery(tools.vercel.domains.getDomains.queryOptions({}));`), diff --git a/packages/hosts/mcp/src/artifact-bindings.ts b/packages/hosts/mcp/src/artifact-bindings.ts index 9be7a15c39..75141a76ce 100644 --- a/packages/hosts/mcp/src/artifact-bindings.ts +++ b/packages/hosts/mcp/src/artifact-bindings.ts @@ -41,6 +41,7 @@ import { type ArtifactBindings, type Owner, } from "@executor-js/sdk"; +import { Option, Schema } from "effect"; // --------------------------------------------------------------------------- // Vocabulary @@ -92,14 +93,32 @@ 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.` or `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 identifier = match[1]; + if (identifier !== undefined) return identifier; + const serialized = match[2]; + if (serialized === undefined) return null; + const decoded = decodeJsonString(serialized); + return Option.isSome(decoded) && decoded.value.length > 0 ? decoded.value : null; +}; + +const formatToolRoot = (integration: string): string => + /^[A-Za-z_$][\w$]*$/.test(integration) ? `.${integration}` : `[${JSON.stringify(integration)}]`; /** * An old-style address: a tier literal in the segment right after the @@ -111,18 +130,23 @@ const TOOLS_REFERENCE = * surface: the two words are reserved by the address grammar itself, the shape * is vanishingly rare, and the error says precisely what to write instead. */ -const OLD_STYLE_TIER_SEGMENT = /(? { const match = OLD_STYLE_TIER_SEGMENT.exec(withCommentsBlanked(code)); if (!match) return null; - const [, integration = "", tier = ""] = match; + const integration = integrationFromMatch(match); + const tier = match[3]; + if (integration === null || tier === undefined) return null; + const root = formatToolRoot(integration); return [ - `Artifact code must not name a connection: \`tools.${integration}.${tier}.…\` pins this artifact to one account.`, - `Address the integration only — \`tools.${integration}.(args)\` — and the server binds it to your connection when the artifact runs.`, - `Discovery through \`execute\` still uses the full \`tools.${integration}.${tier}..\` address; only saved artifact code drops the middle segments.`, - `If this artifact needs two accounts of the same integration, tag each one with a role — \`tools.${integration}("prod").(args)\` — and pass \`connections: { "prod": "${integration}.${tier}." }\` to create-artifact.`, + `Artifact code must not name a connection: \`tools${root}.${tier}.…\` pins this artifact to one account.`, + `Address the integration only — \`tools${root}.(args)\` — and the server binds it to your connection when the artifact runs.`, + `Discovery through \`execute\` still uses the full \`tools${root}.${tier}..\` address; only saved artifact code drops the middle segments.`, + `If this artifact needs two accounts of the same integration, tag each one with a role — \`tools${root}("prod").(args)\` — and pass \`connections: { "prod": "${integration}.${tier}." }\` to create-artifact.`, ].join(" "); }; @@ -138,9 +162,9 @@ export const extractArtifactRoles = (code: string): readonly ArtifactRole[] => { const scannable = withCommentsBlanked(code); const found = new Map(); for (const match of scannable.matchAll(TOOLS_REFERENCE)) { - const integration = match[1]; - if (integration === undefined || RESERVED_TOOL_ROOTS.has(integration)) continue; - const role = match[2] ?? match[3] ?? integration; + const integration = integrationFromMatch(match); + if (integration === null || RESERVED_TOOL_ROOTS.has(integration)) continue; + const role = match[3] ?? match[4] ?? integration; if (role.length === 0) continue; if (!found.has(role)) found.set(role, { role, integration }); } diff --git a/packages/hosts/mcp/src/artifacts-tools.test.ts b/packages/hosts/mcp/src/artifacts-tools.test.ts index bc0a958b20..4feda4d53b 100644 --- a/packages/hosts/mcp/src/artifacts-tools.test.ts +++ b/packages/hosts/mcp/src/artifacts-tools.test.ts @@ -2236,6 +2236,24 @@ describe("MCP host — execute-action binding resolution", () => { ]); }); + it("expands a hyphenated integration through the artifact's binding", async () => { + const code = `function App(){ + useQuery(tools["cloudflare-bindings"].d1_database_query.queryOptions({ sql: "SELECT 1" })); + return
; + }`; + const { result, executed } = await runBoundAction({ + code, + available: [conn("cloudflare-bindings", "org", "default")], + action: { + code: 'return await tools["cloudflare-bindings"].d1_database_query({"sql":"SELECT 1"})', + }, + }); + expect(result.isError, textOf(result)).toBeFalsy(); + expect(executed).toEqual([ + 'return await tools["cloudflare-bindings"].org.default.d1_database_query({"sql":"SELECT 1"})', + ]); + }); + it("routes each role to its own connection", async () => { const roleCode = `function App(){ useQuery(tools.linear("prod").issues.list.queryOptions({})); diff --git a/packages/hosts/mcp/src/tool-call-code.ts b/packages/hosts/mcp/src/tool-call-code.ts index b95933dcd1..33422e21e8 100644 --- a/packages/hosts/mcp/src/tool-call-code.ts +++ b/packages/hosts/mcp/src/tool-call-code.ts @@ -12,13 +12,17 @@ * the shell ever writes any. So the server parses `execute-action` against the * one grammar the proxy emits: * - * return await tools.("")?(.)*() + * return await tools("")?*() + * + * A segment is either `.identifier` or a JSON-escaped bracket lookup. The + * latter keeps hyphenated integration and tool slugs inert while preserving + * the one-call grammar. * * 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 +36,47 @@ 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 PATH_SEGMENT = String.raw`(?:\.[A-Za-z_$][\w$]*|\[${JSON_STRING_LITERAL}\])`; +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`(?:\.([A-Za-z_$][\w$]*)|\[(${JSON_STRING_LITERAL})\])`, + "gy", +); /** 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 decodeJsonString = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.String)); + +const decodePath = (serialized: string): readonly string[] | null => { + const path: string[] = []; + let offset = 0; + while (offset < serialized.length) { + PATH_SEGMENT_MATCHER.lastIndex = offset; + const match = PATH_SEGMENT_MATCHER.exec(serialized); + if (!match) return null; + + const identifier = match[1]; + if (identifier !== undefined) { + path.push(identifier); + } else { + const bracketed = match[2]; + if (bracketed === undefined) return null; + const decoded = decodeJsonString(bracketed); + if (Option.isNone(decoded) || decoded.value.length === 0) return null; + path.push(decoded.value); + } + offset = PATH_SEGMENT_MATCHER.lastIndex; + } + return path; +}; 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 +91,8 @@ 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("")?()` —', + "segments use dot notation for identifiers or JSON-string bracket notation otherwise —", "exactly what the shell's `tools.*` proxy emits.", "Interactive UI reaches integrations declaratively:", "`tools...queryOptions(...)` / `.infiniteQueryOptions(...)` for reads,", @@ -72,26 +108,30 @@ 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 (!path || path.length === 0) return null; if (serializedRole === undefined) return { path, args: args.value }; // The role is a JSON string literal for the same reason the args are a JSON // literal: it decodes or it was not the proxy's emission. - const role = decodeRole(serializedRole); + const role = decodeJsonString(serializedRole); if (Option.isNone(role) || role.value.length === 0) return 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 formatToolPathSegment = (segment: string): string => + TOOL_PATH_IDENTIFIER.test(segment) ? `.${segment}` : `[${JSON.stringify(segment)}]`; /** * Build the codemode call for a RESOLVED address — the full @@ -110,10 +150,10 @@ const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$]*$/; */ export const formatToolCallCode = (path: readonly string[], args: unknown): string => { for (const segment of path) { - if (!TOOL_PATH_SEGMENT.test(segment)) { + if (typeof segment !== "string" || segment.length === 0) { // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: unreachable by construction (every segment came from a parsed path or a stored binding); a defect here must not become a malformed emission 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 ?? {})})`; }; From 3069b15579ff054cfc4fd222a3d44d071392310c Mon Sep 17 00:00:00 2001 From: GijungKim Date: Sun, 30 Aug 2026 11:01:32 -0400 Subject: [PATCH 2/2] refactor: narrow hyphenated artifact path fix --- .../src/shell/mcp-app.browser.test.ts | 46 ------------------ .../hosts/mcp-apps-shell/src/shell/proxy.ts | 7 +-- .../src/shell/tool-call-grammar.pin.test.ts | 16 ++++--- .../hosts/mcp/src/artifact-bindings.test.ts | 8 ---- packages/hosts/mcp/src/artifact-bindings.ts | 41 ++++------------ .../hosts/mcp/src/artifacts-tools.test.ts | 18 ------- packages/hosts/mcp/src/tool-call-code.ts | 48 +++++-------------- 7 files changed, 32 insertions(+), 152 deletions(-) diff --git a/packages/hosts/mcp-apps-shell/src/shell/mcp-app.browser.test.ts b/packages/hosts/mcp-apps-shell/src/shell/mcp-app.browser.test.ts index 921a519aa4..acf79110e7 100644 --- a/packages/hosts/mcp-apps-shell/src/shell/mcp-app.browser.test.ts +++ b/packages/hosts/mcp-apps-shell/src/shell/mcp-app.browser.test.ts @@ -1170,11 +1170,6 @@ const HARNESS_BINDINGS: ArtifactBindings = { owner: "org", connection: ConnectionName.make(INVENTORY_CONNECTION), }, - "cloudflare-bindings": { - integration: IntegrationSlug.make("cloudflare-bindings"), - owner: "org", - connection: ConnectionName.make("main"), - }, alt: { integration: IntegrationSlug.make(INVENTORY_SLUG), owner: "org", @@ -1587,47 +1582,6 @@ describe("MCP app generated UI browser isolation", () => { } }, 30_000); - it("serializes a hyphenated integration slug with bracket notation", async () => { - if (!browser || !hostServer) throw new Error("Browser harness did not start."); - const { page, shellFrame } = await openHarness(browser, hostServer.url); - - try { - const innerFrame = await renderGeneratedUi( - page, - shellFrame, - `function App() { - const query = useQuery({ - ...tools["cloudflare-bindings"].d1_database_query.queryOptions({ sql: "SELECT 1" }), - retry: false, - }); - return
{query.isLoading ? "loading" : query.isError ? "error" : "done"}
; - }`, - ); - await innerFrame.locator("#status").waitFor({ timeout: 10_000 }); - await page.waitForFunction(() => - (window as unknown as BrowserHostWindow).__mcpHostState.toolCalls.some( - (call) => - call.arguments?.code === - 'return await tools["cloudflare-bindings"].d1_database_query({"sql":"SELECT 1"})', - ), - ); - - expect((await getHostState(page)).toolCalls).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - name: "execute-action", - arguments: { - code: 'return await tools["cloudflare-bindings"].d1_database_query({"sql":"SELECT 1"})', - artifactId: HARNESS_ARTIFACT_ID, - }, - }), - ]), - ); - } finally { - await page.close(); - } - }, 30_000); - // An artifact that uses two accounts of one integration tags each call site // with a role. The role has to survive four hops — the inner proxy's `apply` // trap, the TanStack cache key, the postMessage bridge, and the diff --git a/packages/hosts/mcp-apps-shell/src/shell/proxy.ts b/packages/hosts/mcp-apps-shell/src/shell/proxy.ts index 7391d1a2a8..b0983344d9 100644 --- a/packages/hosts/mcp-apps-shell/src/shell/proxy.ts +++ b/packages/hosts/mcp-apps-shell/src/shell/proxy.ts @@ -34,6 +34,7 @@ export type RequestTrustedInteraction = ( ) => Promise; 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)}]`; @@ -43,10 +44,6 @@ const formatToolPathSegment = (segment: string): string => * * return await tools("")?*() * - * A segment is either `.identifier` or `["JSON-escaped slug"]`. The bracket - * form lets integrations and tools with names such as `cloudflare-bindings` - * use the same narrow channel without making their slug executable source. - * * A single proxy-shaped tool call, nothing else — no statements, no loops, no * composition. The server parses `execute-action` against exactly this shape * (`parseToolCallCode` in `@executor-js/host-mcp`), so an iframe cannot smuggle @@ -70,7 +67,7 @@ export function toolCallCode( ): string { if (path.length === 0) throw new Error("Invalid tool path."); const parts = path.map((part) => { - if (typeof part !== "string" || part.length === 0) { + if (typeof part !== "string" || !TOOL_PATH_SEGMENT.test(part)) { throw new Error("Invalid tool path."); } return part; 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 52bbca5845..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"; @@ -35,11 +35,6 @@ describe("execute-action tool-call grammar", () => { path: ["cloudflare-bindings", "d1_database_query"], args: [{ database_id: "db", sql: "SELECT 1" }], }, - { - label: "a path segment requiring JSON escaping", - path: ["inventory", 'items"]; return await tools.evil.run({}); //'], - args: [{}], - }, { label: "an argument with a $ in an identifier-ish key", path: ["mongo", "org", "main", "find"], @@ -92,9 +87,16 @@ 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", ""], [])).toThrow("Invalid tool path."); + expect(() => toolCallCode(["github", "issues; drop"], [])).toThrow("Invalid tool path."); + expect(() => toolCallCode(["github", "1bad"], [])).toThrow("Invalid tool path."); expect(() => toolCallCode(["github", "issues"], [], "")).toThrow("Invalid tool role."); }); diff --git a/packages/hosts/mcp/src/artifact-bindings.test.ts b/packages/hosts/mcp/src/artifact-bindings.test.ts index c650a944e0..8cd527c715 100644 --- a/packages/hosts/mcp/src/artifact-bindings.test.ts +++ b/packages/hosts/mcp/src/artifact-bindings.test.ts @@ -113,14 +113,6 @@ describe("oldStyleAddressRejection", () => { ).toContain("tools.inventory.org."); }); - it("rejects a bracketed old-style address", () => { - expect( - oldStyleAddressRejection( - `useQuery(tools["cloudflare-bindings"].org.main.d1_database_query.queryOptions({}));`, - ), - ).toContain('tools["cloudflare-bindings"].org.'); - }); - it("accepts the short form", () => { expect( oldStyleAddressRejection(`useQuery(tools.vercel.domains.getDomains.queryOptions({}));`), diff --git a/packages/hosts/mcp/src/artifact-bindings.ts b/packages/hosts/mcp/src/artifact-bindings.ts index 75141a76ce..92104e901b 100644 --- a/packages/hosts/mcp/src/artifact-bindings.ts +++ b/packages/hosts/mcp/src/artifact-bindings.ts @@ -41,7 +41,6 @@ import { type ArtifactBindings, type Owner, } from "@executor-js/sdk"; -import { Option, Schema } from "effect"; // --------------------------------------------------------------------------- // Vocabulary @@ -93,32 +92,17 @@ const withCommentsBlanked = (code: string): string => code.replace(/\/\/[^\n]*|\/\*[\s\S]*?\*\//g, (text) => text.replace(/[^\n]/g, " ")); /** - * A `tools.` or `tools["root"]` 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 JSON_STRING_LITERAL = String.raw`"(?:[^"\\]|\\.)*"`; -const TOOL_ROOT = String.raw`(?:\.\s*([A-Za-z_$][\w$]*)|\[\s*(${JSON_STRING_LITERAL})\s*\])`; +const TOOL_ROOT = String.raw`(?:\.\s*([A-Za-z_$][\w$]*)|\[\s*"([A-Za-z_$][\w$-]*)"\s*\])`; const TOOLS_REFERENCE = new RegExp( String.raw`(? { - const identifier = match[1]; - if (identifier !== undefined) return identifier; - const serialized = match[2]; - if (serialized === undefined) return null; - const decoded = decodeJsonString(serialized); - return Option.isSome(decoded) && decoded.value.length > 0 ? decoded.value : null; -}; - -const formatToolRoot = (integration: string): string => - /^[A-Za-z_$][\w$]*$/.test(integration) ? `.${integration}` : `[${JSON.stringify(integration)}]`; /** * An old-style address: a tier literal in the segment right after the @@ -130,23 +114,18 @@ const formatToolRoot = (integration: string): string => * surface: the two words are reserved by the address grammar itself, the shape * is vanishingly rare, and the error says precisely what to write instead. */ -const OLD_STYLE_TIER_SEGMENT = new RegExp( - String.raw`(? { const match = OLD_STYLE_TIER_SEGMENT.exec(withCommentsBlanked(code)); if (!match) return null; - const integration = integrationFromMatch(match); - const tier = match[3]; - if (integration === null || tier === undefined) return null; - const root = formatToolRoot(integration); + const [, integration = "", tier = ""] = match; return [ - `Artifact code must not name a connection: \`tools${root}.${tier}.…\` pins this artifact to one account.`, - `Address the integration only — \`tools${root}.(args)\` — and the server binds it to your connection when the artifact runs.`, - `Discovery through \`execute\` still uses the full \`tools${root}.${tier}..\` address; only saved artifact code drops the middle segments.`, - `If this artifact needs two accounts of the same integration, tag each one with a role — \`tools${root}("prod").(args)\` — and pass \`connections: { "prod": "${integration}.${tier}." }\` to create-artifact.`, + `Artifact code must not name a connection: \`tools.${integration}.${tier}.…\` pins this artifact to one account.`, + `Address the integration only — \`tools.${integration}.(args)\` — and the server binds it to your connection when the artifact runs.`, + `Discovery through \`execute\` still uses the full \`tools.${integration}.${tier}..\` address; only saved artifact code drops the middle segments.`, + `If this artifact needs two accounts of the same integration, tag each one with a role — \`tools.${integration}("prod").(args)\` — and pass \`connections: { "prod": "${integration}.${tier}." }\` to create-artifact.`, ].join(" "); }; @@ -162,8 +141,8 @@ export const extractArtifactRoles = (code: string): readonly ArtifactRole[] => { const scannable = withCommentsBlanked(code); const found = new Map(); for (const match of scannable.matchAll(TOOLS_REFERENCE)) { - const integration = integrationFromMatch(match); - if (integration === null || RESERVED_TOOL_ROOTS.has(integration)) continue; + const integration = match[1] ?? match[2]; + if (integration === undefined || RESERVED_TOOL_ROOTS.has(integration)) continue; const role = match[3] ?? match[4] ?? integration; if (role.length === 0) continue; if (!found.has(role)) found.set(role, { role, integration }); diff --git a/packages/hosts/mcp/src/artifacts-tools.test.ts b/packages/hosts/mcp/src/artifacts-tools.test.ts index 4feda4d53b..bc0a958b20 100644 --- a/packages/hosts/mcp/src/artifacts-tools.test.ts +++ b/packages/hosts/mcp/src/artifacts-tools.test.ts @@ -2236,24 +2236,6 @@ describe("MCP host — execute-action binding resolution", () => { ]); }); - it("expands a hyphenated integration through the artifact's binding", async () => { - const code = `function App(){ - useQuery(tools["cloudflare-bindings"].d1_database_query.queryOptions({ sql: "SELECT 1" })); - return
; - }`; - const { result, executed } = await runBoundAction({ - code, - available: [conn("cloudflare-bindings", "org", "default")], - action: { - code: 'return await tools["cloudflare-bindings"].d1_database_query({"sql":"SELECT 1"})', - }, - }); - expect(result.isError, textOf(result)).toBeFalsy(); - expect(executed).toEqual([ - 'return await tools["cloudflare-bindings"].org.default.d1_database_query({"sql":"SELECT 1"})', - ]); - }); - it("routes each role to its own connection", async () => { const roleCode = `function App(){ useQuery(tools.linear("prod").issues.list.queryOptions({})); diff --git a/packages/hosts/mcp/src/tool-call-code.ts b/packages/hosts/mcp/src/tool-call-code.ts index 33422e21e8..852791a6c0 100644 --- a/packages/hosts/mcp/src/tool-call-code.ts +++ b/packages/hosts/mcp/src/tool-call-code.ts @@ -14,10 +14,6 @@ * * return await tools("")?*() * - * A segment is either `.identifier` or a JSON-escaped bracket lookup. The - * latter keeps hyphenated integration and tool slugs inert while preserving - * the one-call grammar. - * * 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. @@ -37,43 +33,22 @@ import { Option, Schema } from "effect"; const JSON_STRING_LITERAL = String.raw`"(?:[^"\\]|\\.)*"`; -const PATH_SEGMENT = String.raw`(?:\.[A-Za-z_$][\w$]*|\[${JSON_STRING_LITERAL}\])`; +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`(?:\.([A-Za-z_$][\w$]*)|\[(${JSON_STRING_LITERAL})\])`, - "gy", -); +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 decodeJsonString = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.String)); - -const decodePath = (serialized: string): readonly string[] | null => { - const path: string[] = []; - let offset = 0; - while (offset < serialized.length) { - PATH_SEGMENT_MATCHER.lastIndex = offset; - const match = PATH_SEGMENT_MATCHER.exec(serialized); - if (!match) return null; - - const identifier = match[1]; - if (identifier !== undefined) { - path.push(identifier); - } else { - const bracketed = match[2]; - if (bracketed === undefined) return null; - const decoded = decodeJsonString(bracketed); - if (Option.isNone(decoded) || decoded.value.length === 0) return null; - path.push(decoded.value); - } - offset = PATH_SEGMENT_MATCHER.lastIndex; - } - return path; -}; +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 path segments under `tools`, e.g. `["github", "issues", "create"]`. @@ -92,7 +67,6 @@ export type ParsedToolCall = { export const TOOL_CALL_CONTRACT_MESSAGE = [ "execute-action accepts a single tool call, not arbitrary code.", 'The only accepted form is `return await tools("")?()` —', - "segments use dot notation for identifiers or JSON-string bracket notation otherwise —", "exactly what the shell's `tools.*` proxy emits.", "Interactive UI reaches integrations declaratively:", "`tools...queryOptions(...)` / `.infiniteQueryOptions(...)` for reads,", @@ -116,19 +90,19 @@ export const parseToolCallCode = (code: string): ParsedToolCall | null => { if (Option.isNone(args)) return null; const path = decodePath(`${root}${serializedRest}`); - if (!path || path.length === 0) return null; if (serializedRole === undefined) return { path, args: args.value }; // The role is a JSON string literal for the same reason the args are a JSON // literal: it decodes or it was not the proxy's emission. - const role = decodeJsonString(serializedRole); + const role = decodeRole(serializedRole); if (Option.isNone(role) || role.value.length === 0) return null; return { path, role: role.value, args: args.value }; }; 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)}]`; @@ -150,7 +124,7 @@ const formatToolPathSegment = (segment: string): string => */ export const formatToolCallCode = (path: readonly string[], args: unknown): string => { for (const segment of path) { - if (typeof segment !== "string" || segment.length === 0) { + if (!TOOL_PATH_SEGMENT.test(segment)) { // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: unreachable by construction (every segment came from a parsed path or a stored binding); a defect here must not become a malformed emission throw new Error("Invalid resolved tool path."); }