From 219dfd64e2a7da7eda4b0df61b9f579cee073ea8 Mon Sep 17 00:00:00 2001 From: baggiiiie Date: Thu, 3 Sep 2026 15:29:15 +0800 Subject: [PATCH 1/2] Return artifact source from show-artifact --- .changeset/artifact-source-text.md | 5 ++ e2e/scenarios/artifacts.test.ts | 7 ++- .../hosts/mcp/src/artifacts-tools.test.ts | 50 ++++++++++++++++++- packages/hosts/mcp/src/tool-server.ts | 35 ++++++++++--- 4 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 .changeset/artifact-source-text.md 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/artifacts.test.ts b/e2e/scenarios/artifacts.test.ts index fa9a82d03b..4749dc0317 100644 --- a/e2e/scenarios/artifacts.test.ts +++ b/e2e/scenarios/artifacts.test.ts @@ -187,6 +187,7 @@ scenario( const suffix = uniqueSuffix(); const title = `Release Readiness ${suffix}`; const marker = `artifact-ok-${suffix}`; + const source = artifactSource(marker).trim(); // Tracked so cleanup runs even when an assertion below fails. let artifactId: ArtifactId | undefined; @@ -215,7 +216,7 @@ scenario( ); const rendered = yield* session.call("create-artifact", { - code: artifactSource(marker), + code: source, title, description: "Whether the current release is ready to ship", }); @@ -552,6 +553,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`."), From 5ce4d63d51c252e23f1a36a87a1df137264f86ae Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:27:53 -0700 Subject: [PATCH 2/2] Preserve artifact deletion regression coverage --- e2e/scenarios/artifacts.test.ts | 86 ++++++++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 6 deletions(-) diff --git a/e2e/scenarios/artifacts.test.ts b/e2e/scenarios/artifacts.test.ts index 89328c0770..2b8f700dba 100644 --- a/e2e/scenarios/artifacts.test.ts +++ b/e2e/scenarios/artifacts.test.ts @@ -606,8 +606,10 @@ scenario( const suffix = uniqueSuffix(); const originalTitle = `Draft Dashboard ${suffix}`; const renamedTitle = `Quarterly Dashboard ${suffix}`; + const listDeleteTitle = `List Card ${suffix}`; let artifactId: ArtifactId | undefined; + let listArtifactId: ArtifactId | undefined; yield* Effect.gen(function* () { const rendered = yield* session.call("create-artifact", { @@ -657,26 +659,88 @@ scenario( originalTitle, ); + // A second artifact for the gallery card's own delete path: the detail + // page steps below consume the renamed one, and the card's hover → + // Delete affordance is a separate surface the console must keep working. + const listed = yield* session.call("create-artifact", { + code: artifactSource(`list-delete-${suffix}`), + title: listDeleteTitle, + description: "A dashboard the user will delete from the gallery", + }); + expect(listed.ok, `create-artifact succeeded: ${listed.text}`).toBe(true); + listArtifactId = structuredOf(listed).artifactId as ArtifactId; + expect(listArtifactId, "the second artifact was persisted").toBeTruthy(); + yield* browser.session(identity, async ({ page, step }) => { - await step("Delete the artifact from the list", async () => { + await step("Delete the artifact from its gallery card", async () => { await visit(page, "/artifacts"); + // Card actions reveal on hover; the card is the link's enclosing tile. const card = page.locator('[data-slot="artifact-card"]').filter({ - hasText: renamedTitle, + hasText: listDeleteTitle, }); await card.waitFor({ timeout: 20_000 }); await card.hover(); await card.getByRole("button", { name: "Delete" }).click(); const confirm = page.getByRole("alertdialog"); - await confirm.getByRole("heading", { name: `Delete ${renamedTitle}?` }).waitFor(); + await confirm.getByRole("heading", { name: `Delete ${listDeleteTitle}?` }).waitFor(); await confirm.getByRole("button", { name: "Delete Artifact" }).click(); await confirm.waitFor({ state: "hidden", timeout: 20_000 }); + await page + .getByRole("link", { name: `Open artifact ${listDeleteTitle}` }) + .waitFor({ state: "detached", timeout: 20_000 }); }); - await step("The artifact is gone from the list", async () => { - await page + let releaseListRefresh = () => {}; + let markListRefreshStarted = () => {}; + const listRefreshGate = new Promise((resolve) => { + releaseListRefresh = resolve; + }); + const listRefreshStarted = new Promise((resolve) => { + markListRefreshStarted = resolve; + }); + + await step("Open the artifact and delete it from its detail page", async () => { + await visit(page, "/artifacts"); + await page.getByRole("link", { name: `Open artifact ${renamedTitle}` }).click(); + await page.getByRole("heading", { name: renamedTitle }).waitFor({ timeout: 20_000 }); + + // Hold the post-delete list refresh open. The redirected gallery must + // carry the optimistic removal across the route handoff rather than + // relying on a fast canonical response to hide a stale-cache flash. + await page.route("**/artifacts", async (route) => { + if (route.request().method() !== "GET") { + await route.continue(); + return; + } + markListRefreshStarted(); + await listRefreshGate; + await route.continue(); + }); + + await page.getByRole("button", { name: "Delete" }).click(); + const confirm = page.getByRole("alertdialog"); + await confirm.getByRole("heading", { name: `Delete ${renamedTitle}?` }).waitFor(); + await confirm.getByRole("button", { name: "Delete Artifact" }).click(); + }); + + await step("The redirected gallery already omits the deleted artifact", async () => { + await page.waitForURL((url) => /\/artifacts\/?$/.test(url.pathname), { + timeout: 20_000, + }); + await page.getByRole("heading", { name: "Saved artifacts" }).waitFor({ timeout: 20_000 }); + await listRefreshStarted; + + const deletedCardCount = await page .getByRole("link", { name: `Open artifact ${renamedTitle}` }) - .waitFor({ state: "detached", timeout: 20_000 }); + .count(); + releaseListRefresh(); + await page.unrouteAll({ behavior: "wait" }); + + expect( + deletedCardCount, + "the optimistic delete survives navigation while the list refresh is pending", + ).toBe(0); }); }); @@ -684,12 +748,22 @@ scenario( expect(afterDelete.text, "the agent no longer offers the deleted artifact").not.toContain( renamedTitle, ); + expect(afterDelete.text, "nor the artifact deleted from its gallery card").not.toContain( + listDeleteTitle, + ); const missing = yield* session.call("show-artifact", { id: artifactId }); expect(missing.ok, "fetching a deleted artifact is an error, not an empty render").toBe( false, ); }).pipe( + Effect.ensuring( + Effect.suspend(() => + listArtifactId === undefined + ? Effect.void + : client.artifacts.remove({ params: { artifactId: listArtifactId } }), + ).pipe(Effect.ignore), + ), Effect.ensuring( Effect.suspend(() => artifactId === undefined