History

Fix hyphenated tool paths in artifacts (#1884)

* test: cover hyphenated paths in browser * refactor: narrow hyphenated artifact path fix * Test queue timeout with a controlled clock * Handle single-quoted artifact integration paths * Use the catalog path for the artifact schema query ---------

Tony(Gijung) Kim committed 1 month ago

Commit 3eea03b1 parent 3d9ac6a7 files · +186 −23Browse files

Unified · Split

7 files
  1. .changeset/artifact-hyphenated-tool-paths.md +5 −0
  2. e2e/scenarios/artifact-hyphenated-path.test.ts +109 −0
  3. packages/hosts/mcp-apps-shell/src/shell/proxy.ts +11 −5
  4. packages/hosts/mcp-apps-shell/src/shell/tool-call-grammar.pin.test.ts +12 −1
  5. packages/hosts/mcp/src/artifact-bindings.test.ts +15 −0
  6. packages/hosts/mcp/src/artifact-bindings.ts +8 −5
  7. packages/hosts/mcp/src/tool-call-code.ts +26 −12
.changeset/artifact-hyphenated-tool-paths.md+5−0
@@ -0,0 +1,5 @@
1---
2"executor": patch
3---
4
5Render artifacts that call integrations or tools with hyphenated slugs.
e2e/scenarios/artifact-hyphenated-path.test.ts+109−0
@@ -0,0 +1,109 @@
1import { randomBytes } from "node:crypto";
2import { expect } from "@effect/vitest";
3import { Effect, Schema } from "effect";
4import { composePluginApi } from "@executor-js/api/server";
5import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
6import {
7 ArtifactId,
8 AuthTemplateSlug,
9 ConnectionName,
10 IntegrationSlug,
11} from "@executor-js/sdk/shared";
12
13import { createEmulatorInstance } from "../src/emulator-instance";
14import { scenario } from "../src/scenario";
15import { Api, Browser, Mcp, Target } from "../src/services";
16import { visit } from "../src/surfaces/browser";
17
18const api = composePluginApi([openApiHttpPlugin()] as const);
19const decodeCreatedArtifact = Schema.decodeUnknownSync(
20 Schema.Struct({ structuredContent: Schema.Struct({ artifactId: ArtifactId }) }),
21);
22
23scenario(
24 "Artifacts · a hyphenated integration renders live tool data through a bracket path",
25 { timeout: 180_000 },
26 Effect.gen(function* () {
27 const target = yield* Target;
28 const browser = yield* Browser;
29 const mcp = yield* Mcp;
30 const { client: makeClient } = yield* Api;
31 const identity = yield* target.newIdentity();
32 const client = yield* makeClient(api, identity);
33 const session = mcp.session(identity);
34 const slug = IntegrationSlug.make(`artifact-schema-${randomBytes(4).toString("hex")}`);
35 const baseUrl = yield* createEmulatorInstance("resend", "artifact-path");
36 let artifactId: ArtifactId | undefined;
37
38 yield* Effect.gen(function* () {
39 // The emulator's public schema is a real JSON endpoint. Its descriptive
40 // title gives the rendered query a stable caller-visible result.
41 yield* client.openapi.addSpec({
42 payload: {
43 slug,
44 baseUrl,
45 spec: {
46 kind: "blob",
47 value: JSON.stringify({
48 openapi: "3.0.3",
49 info: { title: "Service schema", version: "1" },
50 servers: [{ url: baseUrl }],
51 paths: {
52 "/openapi.json": {
53 get: {
54 operationId: "readSchema",
55 responses: { "200": { description: "Schema" } },
56 },
57 },
58 },
59 }),
60 },
61 },
62 });
63 yield* client.connections.create({
64 payload: {
65 owner: "org",
66 name: ConnectionName.make("public"),
67 integration: slug,
68 template: AuthTemplateSlug.make("none"),
69 values: {},
70 },
71 });
72 const created = yield* session.call("create-artifact", {
73 title: `Service schema ${slug}`,
74 code: `function App() {
75 const query = useQuery(tools['${slug}'].openapiJson.readSchema.queryOptions({}));
76 return <pre data-testid="live-schema">{query.isPending ? "Loading schema" : JSON.stringify(query.data ?? query.error)}</pre>;
77 }`,
78 });
79 expect(created.ok, created.text).toBe(true);
80 const envelope = decodeCreatedArtifact(created.raw);
81 artifactId = envelope.structuredContent.artifactId;
82 yield* browser.session(identity, async ({ page, step }) => {
83 await step("Open the artifact and read the service schema", async () => {
84 await visit(page, `/artifacts/${artifactId}`);
85 const data = page
86 .frameLocator('[data-testid="artifact-shell-frame"]')
87 .frameLocator("iframe")
88 .getByTestId("live-schema");
89 await data.waitFor({ timeout: 30_000 });
90 await data.filter({ hasText: /Resend/i }).waitFor({ timeout: 30_000 });
91 expect(await data.textContent()).toContain("openapi");
92 });
93 });
94 }).pipe(
95 Effect.ensuring(
96 Effect.gen(function* () {
97 if (artifactId !== undefined)
98 yield* client.artifacts.remove({ params: { artifactId } }).pipe(Effect.ignore);
99 yield* client.connections
100 .remove({
101 params: { owner: "org", integration: slug, name: ConnectionName.make("public") },
102 })
103 .pipe(Effect.ignore);
104 yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore);
105 }),
106 ),
107 );
108 }),
109);
packages/hosts/mcp-apps-shell/src/shell/proxy.ts+11−5
@@ -33,12 +33,16 @@ export type RequestTrustedInteraction = ( · 32 unmodified lines
33 interaction: TrustedInteraction,33 interaction: TrustedInteraction,
34) => Promise<TrustedInteractionResponse>;34) => Promise<TrustedInteractionResponse>;
35 35
36const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$]*$/;36const TOOL_PATH_IDENTIFIER = /^[A-Za-z_$][\w$]*$/;
37const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$-]*$/;
38
39const formatToolPathSegment = (segment: string): string =>
40 TOOL_PATH_IDENTIFIER.test(segment) ? `.${segment}` : `[${JSON.stringify(segment)}]`;
37 41
38/**42/**
39 * The ONE grammar the shell ever puts on the `execute-action` wire:43 * The ONE grammar the shell ever puts on the `execute-action` wire:
40 *44 *
41 * return await tools.<ident>("<role>")?(.<ident>)*(<JSON>)45 * return await tools<segment>("<role>")?<segment>*(<JSON>)
42 *46 *
43 * A single proxy-shaped tool call, nothing else — no statements, no loops, no47 * A single proxy-shaped tool call, nothing else — no statements, no loops, no
44 * composition. The server parses `execute-action` against exactly this shape48 * composition. The server parses `execute-action` against exactly this shape
@@ -71,10 +75,12 @@ export function toolCallCode( · 26 unmodified lines
71 if (role !== undefined && (typeof role !== "string" || role.length === 0)) {75 if (role !== undefined && (typeof role !== "string" || role.length === 0)) {
72 throw new Error("Invalid tool role.");76 throw new Error("Invalid tool role.");
73 }77 }
74 const [head, ...rest] = parts;78 const head = parts[0];
79 if (head === undefined) throw new Error("Invalid tool path.");
80 const rest = parts.slice(1);
75 const tag = role === undefined ? "" : `(${JSON.stringify(role)})`;81 const tag = role === undefined ? "" : `(${JSON.stringify(role)})`;
76 const trailer = rest.length > 0 ? `.${rest.join(".")}` : "";82 const target = `${formatToolPathSegment(head)}${tag}${rest.map(formatToolPathSegment).join("")}`;
77 return `return await tools.${head}${tag}${trailer}(${JSON.stringify(args[0] ?? {})})`;83 return `return await tools${target}(${JSON.stringify(args[0] ?? {})})`;
78}84}
79 85
80/**86/**
packages/hosts/mcp-apps-shell/src/shell/tool-call-grammar.pin.test.ts+12−1
@@ -1,5 +1,5 @@
1import { describe, expect, it } from "@effect/vitest";1import { describe, expect, it } from "@effect/vitest";
2import { parseToolCallCode } from "@executor-js/host-mcp/tool-call-code";2import { formatToolCallCode, parseToolCallCode } from "@executor-js/host-mcp/tool-call-code";
3 3
4import { toolCallCode } from "./proxy";4import { toolCallCode } from "./proxy";
5 5
@@ -30,6 +30,11 @@ describe("execute-action tool-call grammar", () => { · 24 unmodified lines
30 path: ["search"],30 path: ["search"],
31 args: [{ query: "github issues", limit: 12 }],31 args: [{ query: "github issues", limit: 12 }],
32 },32 },
33 {
34 label: "a hyphenated integration slug",
35 path: ["cloudflare-bindings", "d1_database_query"],
36 args: [{ database_id: "db", sql: "SELECT 1" }],
37 },
33 {38 {
34 label: "an argument with a $ in an identifier-ish key",39 label: "an argument with a $ in an identifier-ish key",
35 path: ["mongo", "org", "main", "find"],40 path: ["mongo", "org", "main", "find"],
@@ -82,6 +87,12 @@ describe("execute-action tool-call grammar", () => { · 46 unmodified lines
82 });87 });
83 }88 }
84 89
90 it("formats a resolved hyphenated integration safely", () => {
91 expect(formatToolCallCode(["cloudflare-bindings", "org", "default", "query"], {})).toBe(
92 'return await tools["cloudflare-bindings"].org.default.query({})',
93 );
94 });
95
85 it("refuses to emit a path that would not parse", () => {96 it("refuses to emit a path that would not parse", () => {
86 expect(() => toolCallCode([], [])).toThrow("Invalid tool path.");97 expect(() => toolCallCode([], [])).toThrow("Invalid tool path.");
87 expect(() => toolCallCode(["github", "issues; drop"], [])).toThrow("Invalid tool path.");98 expect(() => toolCallCode(["github", "issues; drop"], [])).toThrow("Invalid tool path.");
packages/hosts/mcp/src/artifact-bindings.test.ts+15−0
@@ -29,6 +29,21 @@ describe("extractArtifactRoles", () => { · 28 unmodified lines
29 expect(roles).toEqual([{ role: "vercel", integration: "vercel" }]);29 expect(roles).toEqual([{ role: "vercel", integration: "vercel" }]);
30 });30 });
31 31
32 it("reads a hyphenated integration from a bracket reference", () => {
33 const roles = extractArtifactRoles(
34 `useQuery(tools["cloudflare-bindings"].d1_database_query.queryOptions({ sql: "SELECT 1" }));`,
35 );
36 expect(roles).toEqual([{ role: "cloudflare-bindings", integration: "cloudflare-bindings" }]);
37 });
38
39 it("reads a hyphenated integration from single-quoted bracket references", () => {
40 expect(
41 extractArtifactRoles(
42 `useQuery(tools['cloudflare-bindings']('production').query.queryOptions({}));`,
43 ),
44 ).toEqual([{ role: "production", integration: "cloudflare-bindings" }]);
45 });
46
32 it("collapses repeated references to one role", () => {47 it("collapses repeated references to one role", () => {
33 const roles = extractArtifactRoles(48 const roles = extractArtifactRoles(
34 `useQuery(tools.linear.issues.list.queryOptions({}));49 `useQuery(tools.linear.issues.list.queryOptions({}));
packages/hosts/mcp/src/artifact-bindings.ts+8−5
@@ -92,14 +92,17 @@ const withCommentsBlanked = (code: string): string => · 91 unmodified lines
92 code.replace(/\/\/[^\n]*|\/\*[\s\S]*?\*\//g, (text) => text.replace(/[^\n]/g, " "));92 code.replace(/\/\/[^\n]*|\/\*[\s\S]*?\*\//g, (text) => text.replace(/[^\n]/g, " "));
93 93
94/**94/**
95 * A `tools.<root>` reference, with the optional role call that follows it.95 * A tools root reference, with the optional role call that follows it.
96 *96 *
97 * The role is captured from either quote flavour. Anything else after the root97 * The role is captured from either quote flavour. Anything else after the root
98 * — property access, a call with an object — is left to the caller's own path98 * — property access, a call with an object — is left to the caller's own path
99 * handling; extraction only cares which integration slot is being reached.99 * handling; extraction only cares which integration slot is being reached.
100 */100 */
101const TOOLS_REFERENCE =101const TOOL_ROOT = String.raw`(?:\.\s*([A-Za-z_$][\w$]*)|\[\s*(?:"([A-Za-z_$][\w$-]*)"|'([A-Za-z_$][\w$-]*)')\s*\])`;
102 /(?<![.\w$])tools\s*\.\s*([A-Za-z_$][\w$]*)\s*(?:\(\s*(?:"((?:[^"\\]|\\.)*)"|'((?:[^'\\]|\\.)*)')\s*\))?/g;102const TOOLS_REFERENCE = new RegExp(
103 String.raw`(?<![.\w$])tools\s*${TOOL_ROOT}\s*(?:\(\s*(?:"((?:[^"\\]|\\.)*)"|'((?:[^'\\]|\\.)*)')\s*\))?`,
104 "g",
105);
103 106
104/**107/**
105 * An old-style address: a tier literal in the segment right after the108 * An old-style address: a tier literal in the segment right after the
@@ -138,9 +141,9 @@ export const extractArtifactRoles = (code: string): readonly ArtifactRole[] => { · 32 unmodified lines
138 const scannable = withCommentsBlanked(code);141 const scannable = withCommentsBlanked(code);
139 const found = new Map<string, ArtifactRole>();142 const found = new Map<string, ArtifactRole>();
140 for (const match of scannable.matchAll(TOOLS_REFERENCE)) {143 for (const match of scannable.matchAll(TOOLS_REFERENCE)) {
141 const integration = match[1];144 const integration = match[1] ?? match[2] ?? match[3];
142 if (integration === undefined || RESERVED_TOOL_ROOTS.has(integration)) continue;145 if (integration === undefined || RESERVED_TOOL_ROOTS.has(integration)) continue;
143 const role = match[2] ?? match[3] ?? integration;146 const role = match[4] ?? match[5] ?? integration;
144 if (role.length === 0) continue;147 if (role.length === 0) continue;
145 if (!found.has(role)) found.set(role, { role, integration });148 if (!found.has(role)) found.set(role, { role, integration });
146 }149 }
packages/hosts/mcp/src/tool-call-code.ts+26−12
@@ -12,13 +12,13 @@ · 11 unmodified lines
12 * the shell ever writes any. So the server parses `execute-action` against the12 * the shell ever writes any. So the server parses `execute-action` against the
13 * one grammar the proxy emits:13 * one grammar the proxy emits:
14 *14 *
15 * return await tools.<ident>("<role>")?(.<ident>)*(<JSON>)15 * return await tools<segment>("<role>")?<segment>*(<JSON>)
16 *16 *
17 * One awaited tool call, one JSON-literal argument, nothing else — no17 * One awaited tool call, one JSON-literal argument, nothing else — no
18 * statements, no loops, no composition. `execute` (the model-facing codemode18 * statements, no loops, no composition. `execute` (the model-facing codemode
19 * tool) is untouched; this constraint is only for the app-originated channel.19 * tool) is untouched; this constraint is only for the app-originated channel.
20 *20 *
21 * The leading identifier is an INTEGRATION, not a connection: artifact paths21 * The leading segment is an INTEGRATION, not a connection: artifact paths
22 * carry no tier and no connection name (see `artifact-bindings.ts`). The22 * carry no tier and no connection name (see `artifact-bindings.ts`). The
23 * optional string call right after it is the integration ROLE, which is how an23 * optional string call right after it is the integration ROLE, which is how an
24 * artifact using two accounts of one integration says which it means. Both are24 * artifact using two accounts of one integration says which it means. Both are
@@ -32,16 +32,26 @@ · 7 unmodified lines
32 32
33import { Option, Schema } from "effect";33import { Option, Schema } from "effect";
34 34
35const TOOL_CALL_CODE =35const JSON_STRING_LITERAL = String.raw`"(?:[^"\\]|\\.)*"`;
36 /^return await tools\.([A-Za-z_$][\w$]*)(?:\((("(?:[^"\\]|\\.)*"))\))?((?:\.[A-Za-z_$][\w$]*)*)\((.*)\);?$/s;36const IDENTIFIER = String.raw`[A-Za-z_$][\w$]*`;
37const SLUG = String.raw`[A-Za-z_$][\w$-]*`;
38const PATH_SEGMENT = String.raw`(?:\.${IDENTIFIER}|\["${SLUG}"\])`;
39const TOOL_CALL_CODE = new RegExp(
40 String.raw`^return await tools(${PATH_SEGMENT})(?:\((${JSON_STRING_LITERAL})\))?((?:${PATH_SEGMENT})*)\((.*)\);?$`,
41 "s",
42);
43const PATH_SEGMENT_MATCHER = new RegExp(String.raw`(?:\.(${IDENTIFIER})|\["(${SLUG})"\])`, "g");
37 44
38/** The proxy's argument is always `JSON.stringify` output, so anything that45/** The proxy's argument is always `JSON.stringify` output, so anything that
39 * does not decode is, by construction, not something the proxy emitted. */46 * does not decode is, by construction, not something the proxy emitted. */
40const decodeArgs = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.Unknown));47const decodeArgs = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.Unknown));
41const decodeRole = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.String));48const decodeRole = Schema.decodeUnknownOption(Schema.fromJsonString(Schema.String));
42 49
50const decodePath = (serialized: string): readonly string[] =>
51 Array.from(serialized.matchAll(PATH_SEGMENT_MATCHER), (match) => match[1] ?? match[2] ?? "");
52
43export type ParsedToolCall = {53export type ParsedToolCall = {
44 /** The dotted path segments under `tools`, e.g. `["github", "issues", "create"]`.54 /** The path segments under `tools`, e.g. `["github", "issues", "create"]`.
45 * The head is an integration slug (or a system-tool root); it is never a55 * The head is an integration slug (or a system-tool root); it is never a
46 * tier or a connection name. */56 * tier or a connection name. */
47 readonly path: readonly string[];57 readonly path: readonly string[];
@@ -56,7 +66,7 @@ export type ParsedToolCall = { · 8 unmodified lines
56/** The message handed back to the iframe when its code is not a tool call. */66/** The message handed back to the iframe when its code is not a tool call. */
57export const TOOL_CALL_CONTRACT_MESSAGE = [67export const TOOL_CALL_CONTRACT_MESSAGE = [
58 "execute-action accepts a single tool call, not arbitrary code.",68 "execute-action accepts a single tool call, not arbitrary code.",
59 'The only accepted form is `return await tools.<integration>("<role>")?.<path>(<json>)` —',69 'The only accepted form is `return await tools<integration>("<role>")?<path>(<json>)` —',
60 "exactly what the shell's `tools.*` proxy emits.",70 "exactly what the shell's `tools.*` proxy emits.",
61 "Interactive UI reaches integrations declaratively:",71 "Interactive UI reaches integrations declaratively:",
62 "`tools.<integration>.<tool>.queryOptions(...)` / `.infiniteQueryOptions(...)` for reads,",72 "`tools.<integration>.<tool>.queryOptions(...)` / `.infiniteQueryOptions(...)` for reads,",
@@ -72,14 +82,14 @@ export const parseToolCallCode = (code: string): ParsedToolCall | null => { · 9 unmodified lines
72 const match = TOOL_CALL_CODE.exec(code.trim());82 const match = TOOL_CALL_CODE.exec(code.trim());
73 if (!match) return null;83 if (!match) return null;
74 84
75 const [, root, serializedRole, , dottedRest, serializedArgs] = match;85 const [, root, serializedRole, serializedRest, serializedArgs] = match;
76 if (root === undefined || dottedRest === undefined || serializedArgs === undefined) return null;86 if (root === undefined || serializedRest === undefined || serializedArgs === undefined)
87 return null;
77 88
78 const args = decodeArgs(serializedArgs);89 const args = decodeArgs(serializedArgs);
79 if (Option.isNone(args)) return null;90 if (Option.isNone(args)) return null;
80 91
81 const rest = dottedRest.length > 0 ? dottedRest.slice(1).split(".") : [];92 const path = decodePath(`${root}${serializedRest}`);
82 const path = [root, ...rest];
83 93
84 if (serializedRole === undefined) return { path, args: args.value };94 if (serializedRole === undefined) return { path, args: args.value };
85 95
@@ -91,7 +101,11 @@ export const parseToolCallCode = (code: string): ParsedToolCall | null => { · 5 unmodified lines
91 return { path, role: role.value, args: args.value };101 return { path, role: role.value, args: args.value };
92};102};
93 103
94const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$]*$/;104const TOOL_PATH_IDENTIFIER = /^[A-Za-z_$][\w$]*$/;
105const TOOL_PATH_SEGMENT = /^[A-Za-z_$][\w$-]*$/;
106
107const formatToolPathSegment = (segment: string): string =>
108 TOOL_PATH_IDENTIFIER.test(segment) ? `.${segment}` : `[${JSON.stringify(segment)}]`;
95 109
96/**110/**
97 * Build the codemode call for a RESOLVED address — the full111 * Build the codemode call for a RESOLVED address — the full
@@ -115,5 +129,5 @@ export const formatToolCallCode = (path: readonly string[], args: unknown): stri · 17 unmodified lines
115 throw new Error("Invalid resolved tool path.");129 throw new Error("Invalid resolved tool path.");
116 }130 }
117 }131 }
118 return `return await tools.${path.join(".")}(${JSON.stringify(args ?? {})})`;132 return `return await tools${path.map(formatToolPathSegment).join("")}(${JSON.stringify(args ?? {})})`;
119};133};