Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion e2e/scenarios/artifacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ scenario(

const ARTIFACT_TOOLS = [
"create-artifact",
"edit-artifact",
"list-artifacts",
"show-artifact",
"execute-action",
Expand Down Expand Up @@ -818,7 +819,12 @@ scenario(
// URL, gets everything by default. Without this the assertions above would
// also pass on a server that had simply lost artifacts.
const defaultTools = yield* defaultSession.listTools();
for (const tool of ["create-artifact", "list-artifacts", "show-artifact"] as const) {
for (const tool of [
"create-artifact",
"edit-artifact",
"list-artifacts",
"show-artifact",
] as const) {
expect(defaultTools, `${tool} is present on a default session`).toContain(tool);
}
const defaultSkills = yield* defaultSession.call("skills", {});
Expand Down
34 changes: 22 additions & 12 deletions packages/core/execution/src/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,22 +113,32 @@ const CREATE_ARTIFACT_SKILL_BODY = [
"UPDATING that artifact, not making another one. A second artifact for a revision",
"leaves the user with two near-identical entries and a stale link to the old one.",
"",
"1. `show-artifact({ id })` to get the current source back. Do not rewrite from memory.",
"2. Edit that source.",
"3. `create-artifact` with the same `artifactId` and the complete edited component.",
"For a TWEAK — a new column, a fixed label, a restyled section — use `edit-artifact`:",
"exact find-and-replace patches against the stored source, so you send only the",
"changed lines instead of re-emitting the whole component.",
"",
"```",
"code: '<the whole edited component>'",
'artifactId: "art_..."',
"edits: [{ oldText: '<exact current text>', newText: '<replacement>' }]",
"```",
"",
"`code` fully replaces what was stored — send the whole component, never a fragment",
"or a diff. `title` and `description` are optional on an update: leave them out to",
"keep the current ones, pass them to change them. Connection bindings are",
"re-resolved from the new code, so a call to a new integration binds on the update",
"the same way it would on a create (and pass `connections` if it is ambiguous).",
"The artifact keeps its id and its URL, so anyone holding the link sees the new",
"version.",
"Each `oldText` must match the current source verbatim (whitespace included) and",
"exactly once — include surrounding lines to disambiguate, or set",
"`replaceAll: true` to change every occurrence. Edits apply in order, each seeing",
"the previous one's result, and the batch is atomic: on any miss nothing is saved",
"and the error returns the current source in `structuredContent.code`, so rebuild",
"the retry from that rather than calling `show-artifact` again. If you don't have",
"the source at all (the artifact came from `list-artifacts`), `show-artifact` once",
"and edit from what it returns — never from memory.",
"",
"For a REWRITE, where most of the code changes, edits would be longer than the code:",
"call `create-artifact` with the same `artifactId` and the complete new component.",
"`code` then fully replaces what was stored — the whole component, never a fragment.",
"",
"Both paths validate and smoke-render the result identically, keep the artifact's id",
"and URL, and re-resolve connection bindings from the resulting code (pass",
"`connections` if it is ambiguous). `title` and `description` are optional on both:",
"leave them out to keep the current ones.",
"",
"Only omit `artifactId` when the user genuinely wants a NEW, separate UI.",
"",
Expand Down Expand Up @@ -331,7 +341,7 @@ const CREATE_ARTIFACT_SKILL_BODY = [
"",
"- `list-artifacts` returns every saved artifact's id, title, description and last-updated time.",
"- `show-artifact({ id })` re-renders one and returns its source. Match the user's phrasing against the titles and descriptions from `list-artifacts` rather than guessing an id.",
'- To change one, edit that source and call `create-artifact` with its `artifactId` — see "Changing An Artifact That Already Exists" above.',
'- To change one, patch it with `edit-artifact` (or rewrite it with `create-artifact` + `artifactId`) — see "Changing An Artifact That Already Exists" above.',
"- Clients that cannot display MCP apps get a link to the artifact in the web app instead; pass that URL on to the user verbatim.",
].join("\n");

Expand Down
294 changes: 293 additions & 1 deletion packages/hosts/mcp/src/artifacts-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,10 @@ describe("MCP host — artifact tool visibility", () => {
NO_APPS_CAPS,
async (client) => {
const names = await toolNames(client);
// The model-facing three are always advertised — they degrade to a
// The model-facing tools are always advertised — they degrade to a
// deep link rather than disappearing.
expect(names).toContain("create-artifact");
expect(names).toContain("edit-artifact");
expect(names).toContain("list-artifacts");
expect(names).toContain("show-artifact");
// `execute-action` is only callable from inside a rendered app.
Expand Down Expand Up @@ -491,6 +492,7 @@ describe("MCP host — artifact tool visibility", () => {
async (client) => {
const names = await toolNames(client);
expect(names).not.toContain("create-artifact");
expect(names).not.toContain("edit-artifact");
expect(names).not.toContain("list-artifacts");
expect(names).not.toContain("show-artifact");
expect(names).not.toContain("execute-action");
Expand Down Expand Up @@ -554,6 +556,7 @@ describe("MCP host — artifact tool visibility", () => {
async (client) => {
const names = await toolNames(client);
expect(names).toContain("create-artifact");
expect(names).toContain("edit-artifact");
expect(names).toContain("list-artifacts");
expect(names).toContain("show-artifact");
expect(names).toContain("execute-action");
Expand Down Expand Up @@ -1683,6 +1686,295 @@ describe("MCP host — create-artifact update in place", () => {
});
});

// ---------------------------------------------------------------------------
// edit-artifact — patch-based updates
// ---------------------------------------------------------------------------
//
// The contract: an edit is exact find-and-replace against the stored source,
// applied in order, all-or-nothing, and the result goes through the same
// validate → smoke-render → bind → save pipeline as a full create. A failed
// batch changes nothing and hands the current source back so the retry needs
// no show-artifact round trip.

describe("MCP host — edit-artifact", () => {
const seed = async (client: Client, code: string = COUNTER_CODE) =>
client.callTool({
name: "create-artifact",
arguments: { code, title: "Draft", description: "First pass" },
});

it("patches the stored source and delivers the edited component", async () => {
const store = makeArtifactStore();
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client);
const edited = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [{ oldText: "<div>hi</div>", newText: "<div>hi again</div>" }],
},
});
expect(edited.isError).toBeFalsy();
expect(structuredOf(edited)).toEqual({ code: UPDATED_CODE, artifactId: "art_1" });
expect(store.rows.get("art_1")).toMatchObject({
code: UPDATED_CODE,
// An edit that says nothing about title/description keeps them.
title: "Draft",
description: "First pass",
});
// One artifact, not two.
expect(store.rows.size).toBe(1);
},
{ artifacts: store.port },
);
});

it("applies edits in order, each against the previous result", async () => {
const store = makeArtifactStore();
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client);
const edited = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [
{ oldText: "<div>hi</div>", newText: "<div>hello</div>" },
// Matches only the first edit's output, proving sequencing.
{ oldText: "<div>hello</div>", newText: "<div>hello there</div>" },
],
},
});
expect(edited.isError).toBeFalsy();
expect(store.rows.get("art_1")?.code).toBe(
"function App() { return <div>hello there</div>; }",
);
},
{ artifacts: store.port },
);
});

it("replaces every occurrence only under replaceAll, and refuses ambiguity otherwise", async () => {
const store = makeArtifactStore();
const REPEATED = "function App() { return <div><b>x</b><b>x</b></div>; }";
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client, REPEATED);

const ambiguous = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [{ oldText: "<b>x</b>", newText: "<b>y</b>" }],
},
});
expect(ambiguous.isError).toBe(true);
expect(textOf(ambiguous)).toContain("appears 2 times");
expect(textOf(ambiguous)).toContain("replaceAll");
expect(store.rows.get("art_1")?.code).toBe(REPEATED);

const replaced = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [{ oldText: "<b>x</b>", newText: "<b>y</b>", replaceAll: true }],
},
});
expect(replaced.isError).toBeFalsy();
expect(store.rows.get("art_1")?.code).toBe(
"function App() { return <div><b>y</b><b>y</b></div>; }",
);
},
{ artifacts: store.port },
);
});

it("rejects the whole batch on a miss, returns the current source, and saves nothing", async () => {
const store = makeArtifactStore();
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client);
const missed = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [
{ oldText: "<div>hi</div>", newText: "<div>hello</div>" },
{ oldText: "not in the source", newText: "anything" },
],
},
});
expect(missed.isError).toBe(true);
expect(textOf(missed)).toContain("Edit 2 of 2");
expect(textOf(missed)).toContain("matched nothing");
// The retry material: the stored source rides along on the error.
expect(structuredOf(missed)).toMatchObject({ code: COUNTER_CODE });
// Atomic: the first (valid) edit was not half-applied.
expect(store.rows.get("art_1")?.code).toBe(COUNTER_CODE);
expect(store.calls.length).toBe(1);
},
{ artifacts: store.port },
);
});

it("validates the edited result like a create, and leaves the stored row untouched", async () => {
const store = makeArtifactStore();
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client);
const rejected = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [
{
oldText: "return <div>hi</div>;",
newText: "const Card = 1; return <div>hi</div>;",
},
],
},
});
expect(rejected.isError).toBe(true);
expect(textOf(rejected)).toContain("cannot be redeclared");
expect(store.rows.get("art_1")?.code).toBe(COUNTER_CODE);
expect(store.calls.length).toBe(1);
},
{ artifacts: store.port },
);
});

it("smoke-renders the edited result, so a patch cannot break a working artifact", async () => {
const store = makeArtifactStore();
const smokeRenderArtifact = vi.fn((code: string) =>
Promise.resolve(
code.includes("boom")
? { status: "failed" as const, message: "boom is not defined" }
: { status: "ok" as const },
),
);
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client);
const broken = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [{ oldText: "return", newText: "boom(); return" }],
},
});
expect(broken.isError).toBe(true);
expect(textOf(broken)).toContain("boom is not defined");
expect(store.rows.get("art_1")?.code).toBe(COUNTER_CODE);
// The candidate it rendered was the EDITED source, not the stored one.
expect(smokeRenderArtifact).toHaveBeenLastCalledWith(
expect.stringContaining("boom(); return"),
);
},
{ artifacts: store.port, smokeRenderArtifact },
);
});

it("refuses an artifact id that is not the caller's, without saying whether it exists", async () => {
const store = makeArtifactStore();
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
const result = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_someone_else",
edits: [{ oldText: "a", newText: "b" }],
},
});
expect(result.isError).toBe(true);
// The probe-proof answer, with no stored source riding along.
expect(structuredOf(result)).toMatchObject({ error: "artifact_unavailable" });
expect(structuredOf(result)).not.toHaveProperty("code");
expect(store.calls).toEqual([]);
},
{ artifacts: store.port },
);
});

it("re-resolves bindings when an edit introduces an integration", async () => {
const store = makeArtifactStore();
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client);
expect(store.calls[0]?.bindings).toEqual({});
const edited = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [
{
oldText: "return <div>hi</div>;",
newText:
"const q = useQuery(tools.vercel.domains.getDomains.queryOptions({})); return <div>hi</div>;",
},
],
},
});
expect(edited.isError).toBeFalsy();
expect(store.calls[1]?.bindings).toEqual({
vercel: {
integration: "vercel",
owner: "user",
connection: "personalVercel",
},
});
},
{
artifacts: store.port,
connections: connectionsPort([conn("vercel", "user", "personalVercel")]),
},
);
});

it("updates the title and description when asked, keeping the patched code", async () => {
const store = makeArtifactStore();
await withClient(
makeStubEngine({}),
APPS_CAPS,
async (client) => {
await seed(client);
const edited = await client.callTool({
name: "edit-artifact",
arguments: {
artifactId: "art_1",
edits: [{ oldText: "hi", newText: "hi again" }],
title: "Active users dashboard",
description: "Second pass",
},
});
expect(edited.isError).toBeFalsy();
expect(store.rows.get("art_1")).toMatchObject({
title: "Active users dashboard",
description: "Second pass",
code: UPDATED_CODE,
});
},
{ artifacts: store.port },
);
});
});

// ---------------------------------------------------------------------------
// create-artifact — connection binding
// ---------------------------------------------------------------------------
Expand Down
Loading
Loading