diff --git a/.changeset/artifact-source-text.md b/.changeset/artifact-source-text.md new file mode 100644 index 0000000000..b87f7d8bba --- /dev/null +++ b/.changeset/artifact-source-text.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +**Fix: `show-artifact` now returns the saved component source to MCP clients that cannot render Apps.** Agents can read the current source and make targeted edits instead of receiving only a link to the artifact. diff --git a/e2e/scenarios/artifact-source-roundtrip.test.ts b/e2e/scenarios/artifact-source-roundtrip.test.ts new file mode 100644 index 0000000000..e8a056a7a7 --- /dev/null +++ b/e2e/scenarios/artifact-source-roundtrip.test.ts @@ -0,0 +1,77 @@ +import { expect } from "@effect/vitest"; +import { Effect, Schema } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { ArtifactId } from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Mcp, Target } from "../src/services"; +import { visit } from "../src/surfaces/browser"; + +const api = composePluginApi([] as const); +const savedArtifact = Schema.Struct({ artifactId: ArtifactId, url: Schema.String }); +const sourceResult = Schema.Struct({ code: Schema.String }); +const structured = Schema.Struct({ structuredContent: Schema.Unknown }); + +scenario( + "Artifacts · text-only clients read current source and edit the same artifact", + { timeout: 120_000 }, + Effect.gen(function* () { + const target = yield* Target; + const mcp = yield* Mcp; + const browser = yield* Browser; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const session = mcp.session(identity); + const source = "function App() { return

Original source marker

; }"; + const created = yield* session.call("create-artifact", { + title: "Source round trip", + code: source, + }); + expect(created.ok).toBe(true); + const envelope = yield* Schema.decodeUnknownEffect(structured)(created.raw); + const saved = yield* Schema.decodeUnknownEffect(savedArtifact)(envelope.structuredContent); + yield* Effect.gen(function* () { + const shown = yield* session.call("show-artifact", { id: saved.artifactId }); + expect(shown.ok).toBe(true); + const shownEnvelope = yield* Schema.decodeUnknownEffect(structured)(shown.raw); + const current = yield* Schema.decodeUnknownEffect(sourceResult)( + shownEnvelope.structuredContent, + ); + expect(current.code).toBe(source); + expect(shown.text).toContain(current.code); + const updated = current.code.replace("Original source marker", "Updated source marker"); + const edited = yield* session.call("edit-artifact", { + artifactId: saved.artifactId, + edits: [{ oldText: current.code, newText: updated }], + }); + expect(edited.ok, edited.text).toBe(true); + const afterEdit = yield* session.call("show-artifact", { id: saved.artifactId }); + expect(afterEdit.ok).toBe(true); + const afterEnvelope = yield* Schema.decodeUnknownEffect(structured)(afterEdit.raw); + const afterSource = yield* Schema.decodeUnknownEffect(sourceResult)( + afterEnvelope.structuredContent, + ); + expect(afterSource.code).toBe(updated); + expect(afterEdit.text).toContain(updated); + expect(afterEdit.text).not.toContain("Original source marker"); + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the artifact edited from its returned source", async () => { + await visit(page, saved.url); + await page + .frameLocator('[data-testid="artifact-shell-frame"]') + .frameLocator("iframe") + .getByText("Updated source marker", { exact: true }) + .waitFor({ timeout: 30_000 }); + }); + }); + }).pipe( + Effect.ensuring( + client.artifacts.remove({ params: { artifactId: saved.artifactId } }).pipe( + // oxlint-disable-next-line executor/no-effect-escape-hatch -- boundary: cleanup must fail the scenario if the API cannot remove the fixture + Effect.orDie, + ), + ), + ); + }), +); diff --git a/e2e/scenarios/artifacts.test.ts b/e2e/scenarios/artifacts.test.ts index 57cf10c195..2b8f700dba 100644 --- a/e2e/scenarios/artifacts.test.ts +++ b/e2e/scenarios/artifacts.test.ts @@ -149,14 +149,14 @@ const recordHandshakeOrdering = async (page: Page): Promise => { const readHandshakeOrdering = (page: Page): Promise> => page.evaluate(() => globalThis.__handshakeOrder ?? []); -const readConsoleStyle = (page: Page): Promise<{ primary: string; buttonBg: string }> => - page.evaluate(() => { - const button = document.querySelector("button"); - return { - primary: getComputedStyle(document.documentElement).getPropertyValue("--primary").trim(), - buttonBg: button ? getComputedStyle(button).backgroundColor : "", - }; - }); +const readConsoleStyle = async (page: Page): Promise<{ primary: string; buttonBg: string }> => { + const button = page.getByRole("button", { name: "Rename", exact: true }); + await button.waitFor(); + return button.evaluate((element) => ({ + primary: getComputedStyle(document.documentElement).getPropertyValue("--primary").trim(), + buttonBg: getComputedStyle(element).backgroundColor, + })); +}; // The shell's compiled stylesheet declares `--mcp-apps-shell-stylesheet: 1` // on `:root` as a provenance marker (see the shell's globals.css): the shell's @@ -193,6 +193,7 @@ scenario( const title = `Release Readiness ${suffix}`; const marker = `artifact-ok-${suffix}`; const pullRequestUrl = new URL("/policies?from=artifact-link", target.baseUrl).toString(); + const source = artifactSource(marker, pullRequestUrl).trim(); // Tracked so cleanup runs even when an assertion below fails. let artifactId: ArtifactId | undefined; @@ -221,7 +222,7 @@ scenario( ); const rendered = yield* session.call("create-artifact", { - code: artifactSource(marker, pullRequestUrl), + code: source, title, description: "Whether the current release is ready to ship", }); @@ -571,6 +572,10 @@ scenario( String(structuredOf(shown).url ?? shown.text), "show-artifact delivers the same deep link for a non-Apps client", ).toContain(String(artifactId)); + expect( + shown.text, + "show-artifact includes the current source in its text result for a non-Apps client", + ).toContain(`Source:\n\`\`\`tsx\n${source}\n\`\`\``); }).pipe( Effect.ensuring( Effect.suspend(() => diff --git a/packages/hosts/mcp/src/artifacts-tools.test.ts b/packages/hosts/mcp/src/artifacts-tools.test.ts index bc0a958b20..a761f12bb8 100644 --- a/packages/hosts/mcp/src/artifacts-tools.test.ts +++ b/packages/hosts/mcp/src/artifacts-tools.test.ts @@ -182,6 +182,13 @@ const structuredOf = (result: Awaited>): Record>): string => (result.content as Array<{ type: string; text: string }>)[0].text; +/** Assert source is available through both MCP result channels. */ +const expectArtifactSource = (result: Awaited>, code: string) => { + expect(structuredOf(result).code).toBe(code); + expect(textOf(result)).toContain("Source:"); + expect(textOf(result)).toContain(code); +}; + const toolNames = async (client: Client): Promise => (await client.listTools()).tools.map((tool) => tool.name); @@ -688,8 +695,11 @@ describe("MCP host — create-artifact", () => { url: "https://executor.test/artifacts/art_1", artifactId: "art_1", }); - // The model needs to be told to hand the URL over. + // The model needs to be told to hand the URL over. Source is a + // show-artifact read, not part of the create confirmation. expect(textOf(result)).toContain("https://executor.test/artifacts/art_1"); + expect(textOf(result)).not.toContain("Source:"); + expect(structuredOf(result)).not.toHaveProperty("code"); // Persistence is what makes the fallback possible at all. expect(store.calls).toHaveLength(1); expect(store.rows.get("art_1")?.code).toBe(COUNTER_CODE); @@ -1285,6 +1295,10 @@ describe("MCP host — artifact retrieval", () => { code: COUNTER_CODE, artifactId: "art_1", }); + // Apps-capable hosts still need the source on the text channel: a + // later restore or a client that starts advertising apps must not + // make `show-artifact` unusable for `edit-artifact`. + expectArtifactSource(shown, COUNTER_CODE); }, { artifacts: store.port }, ); @@ -1368,7 +1382,12 @@ describe("MCP host — artifact retrieval", () => { status: "fallback_url", url: "https://executor.test/artifacts/art_1", artifactId: "art_1", + code: COUNTER_CODE, }); + // The URL instruction stays; the source rides after it so a text-only + // host can copy `oldText` for `edit-artifact` from this result. + expect(textOf(shown)).toContain("https://executor.test/artifacts/art_1"); + expectArtifactSource(shown, COUNTER_CODE); }, { artifacts: store.port, @@ -1377,6 +1396,35 @@ describe("MCP host — artifact retrieval", () => { ); }); + it("returns show-artifact source when the client has no apps support and no web UI", async () => { + const store = makeArtifactStore(); + await Effect.runPromise( + store.port.save({ + title: "Saved earlier", + description: null, + code: COUNTER_CODE, + }), + ); + await withClient( + makeStubEngine({}), + NO_APPS_CAPS, + async (client) => { + const shown = await client.callTool({ + name: "show-artifact", + arguments: { id: "art_1" }, + }); + expect(structuredOf(shown)).toEqual({ + status: "fallback_unavailable", + reason: "mcp_apps_unsupported", + artifactId: "art_1", + code: COUNTER_CODE, + }); + expectArtifactSource(shown, COUNTER_CODE); + }, + { artifacts: store.port }, + ); + }); + it("reports a miss as an error result rather than failing the tool call", async () => { const store = makeArtifactStore(); await withClient( diff --git a/packages/hosts/mcp/src/tool-server.ts b/packages/hosts/mcp/src/tool-server.ts index 730c5bfd89..c78c2e8adb 100644 --- a/packages/hosts/mcp/src/tool-server.ts +++ b/packages/hosts/mcp/src/tool-server.ts @@ -900,6 +900,8 @@ const startMarker = (name: string, attributes: Record): Effect. // user as an inline widget when the client renders MCP Apps, and as a link into // the web app when it doesn't. Both carry `artifactId`, because either way the // artifact was saved and can be reopened later. +// `show-artifact` returns source on both channels; create/edit only confirm +// saves. const renderRejectedResult = (reason: string): McpToolResult => ({ content: [{ type: "text", text: `create-artifact rejected: ${reason}` }], @@ -983,6 +985,24 @@ const bindingUnresolvedResult = (input: { isError: true, }); +/** Format the stored source for the text result channel. */ +const artifactSourceText = (code: string): string => `Source:\n\`\`\`tsx\n${code}\n\`\`\``; + +/** Add source to both MCP result channels. */ +const withArtifactSource = (result: McpToolResult, code: string): McpToolResult => { + const source = artifactSourceText(code); + const content = result.content.map((block, index) => + index === 0 && block.type === "text" + ? { type: "text" as const, text: `${block.text}\n\n${source}` } + : block, + ); + return { + ...result, + content, + structuredContent: { ...result.structuredContent, code }, + }; +}; + const renderedInAppResult = (input: { readonly code: string; readonly artifactId: string; @@ -2042,11 +2062,14 @@ export const createExecutorMcpServer = ( .pipe(Effect.catchCause(() => Effect.succeed(null))); if (!artifact) return artifactNotFoundResult(id); yield* notifyArtifactUsage("viewed"); - return deliverArtifact({ - code: artifact.code, - artifactId: artifact.id, - title: artifact.title, - }); + return withArtifactSource( + deliverArtifact({ + code: artifact.code, + artifactId: artifact.id, + title: artifact.title, + }), + artifact.code, + ); }).pipe( Effect.withSpan("mcp.host.tool.show_artifact", { attributes: { "mcp.tool.name": "show-artifact", "mcp.artifact.id": id }, @@ -2245,7 +2268,7 @@ export const createExecutorMcpServer = ( description: [ "Re-render a saved UI artifact by id.", "Use `list-artifacts` first to find the id whose title or description matches what the user asked for.", - "Clients that cannot display MCP apps receive a link to the artifact instead.", + "Returns the artifact's current source. Clients that cannot display MCP apps also receive a link to the artifact; pass it to the user.", ].join("\n"), inputSchema: { id: z.string().trim().min(1).describe("The artifact id from `list-artifacts`."),