diff --git a/packages/cli/src/acp/content.ts b/packages/cli/src/acp/content.ts index c8b511a092a..66861659002 100644 --- a/packages/cli/src/acp/content.ts +++ b/packages/cli/src/acp/content.ts @@ -24,28 +24,13 @@ export function contentBlockToParts(block: ContentBlock): PromptPart[] { } return [{ type: "text", text: block.text }] } - case "image": - if (block.data) { - return [ - { - type: "file", - url: `data:${block.mimeType};base64,${block.data}`, - filename: filenameFromUri(block.uri ?? undefined) ?? "image", - mime: block.mimeType, - }, - ] - } - if (block.uri?.startsWith("data:") || block.uri?.startsWith("http://") || block.uri?.startsWith("https://")) { - return [ - { - type: "file", - url: block.uri, - filename: filenameFromUri(block.uri) ?? "image", - mime: block.mimeType, - }, - ] - } - return [] + case "image": { + const url = block.data ? `data:${block.mimeType};base64,${block.data}` : block.uri + if (!url) return [] + const filename = filenameFromUri(block.uri ?? undefined) ?? "image" + if (url.startsWith("data:")) return [{ type: "file", url, filename, mime: block.mimeType }] + return [resourceLinkToPart({ uri: url, name: filename, mimeType: block.mimeType })] + } case "resource_link": return [resourceLinkToPart(block)] case "resource": @@ -180,7 +165,10 @@ export function linkReference(name: string | undefined, uri: string): PromptPart function filenameFromUri(uri: string | undefined): string | undefined { if (!uri || uri.startsWith("data:")) return undefined - if (URL.canParse(uri)) return path.basename(new URL(uri).pathname) || undefined + if (URL.canParse(uri)) { + const url = new URL(uri) + return path.basename((url.protocol === "zed:" && url.searchParams.get("path")) || url.pathname) || undefined + } return path.basename(uri) || undefined } diff --git a/packages/cli/src/acp/turn.ts b/packages/cli/src/acp/turn.ts index 58dda1b8009..3cbee315d1b 100644 --- a/packages/cli/src/acp/turn.ts +++ b/packages/cli/src/acp/turn.ts @@ -374,6 +374,9 @@ export const make = Effect.fnUntraced(function* (input: { const attached = yield* input.sessions.require(params.sessionId) const catalog = yield* input.catalog.get(attached.cwd) const childUpdates = (yield* Ref.get(input.capabilities)).childSessionUpdates + if (params.prompt.some((block) => block.type === "image" && !block.data && !block.uri)) { + return yield* new ACPError.InvalidRequestError({ message: "image content has no data or uri", field: "prompt" }) + } const parts = yield* Effect.forEach(promptContentToParts(params.prompt), referenceUnreadableFile, { concurrency: "unbounded", }) diff --git a/packages/cli/test/acp/content.test.ts b/packages/cli/test/acp/content.test.ts index 16588ee9110..6bcee6d2526 100644 --- a/packages/cli/test/acp/content.test.ts +++ b/packages/cli/test/acp/content.test.ts @@ -45,22 +45,48 @@ describe("acp content conversion", () => { ]) }) - test("image block with http URI becomes a file part", () => { + test("image block with only a data URL URI becomes a file part", () => { + expect( + contentBlockToParts({ type: "image", data: "", mimeType: "image/png", uri: "data:image/png;base64,AAAA" }), + ).toEqual([{ type: "file", url: "data:image/png;base64,AAAA", filename: "image", mime: "image/png" }]) + }) + + test("image block with only an http URI becomes a markdown link", () => { expect( contentBlockToParts({ type: "image", data: "", mimeType: "image/jpeg", - uri: "http://example.com/assets/photo.jpg", + uri: "https://example.com/assets/photo.jpg", }), - ).toEqual([ - { - type: "file", - url: "http://example.com/assets/photo.jpg", - filename: "photo.jpg", - mime: "image/jpeg", - }, + ).toEqual([{ type: "text", text: "[photo.jpg](https://example.com/assets/photo.jpg)" }]) + expect( + contentBlockToParts({ type: "image", data: "", mimeType: "image/png", uri: "https://example.com/" }), + ).toEqual([{ type: "text", text: "[image](https://example.com/)" }]) + expect( + contentBlockToParts({ type: "image", data: "", mimeType: "image/png", uri: "http://example.com/a.png" }), + ).toEqual([{ type: "text", text: "[a.png](http://example.com/a.png)" }]) + }) + + test("image block with only a URI of another scheme or a bare path becomes a markdown link", () => { + expect(contentBlockToParts({ type: "image", data: "", mimeType: "image/png", uri: "ftp://host/a.png" })).toEqual([ + { type: "text", text: "[a.png](ftp://host/a.png)" }, ]) + expect(contentBlockToParts({ type: "image", data: "", mimeType: "image/png", uri: "assets/b.png" })).toEqual([ + { type: "text", text: "[b.png](assets/b.png)" }, + ]) + }) + + test("image block with only a file URI becomes a file part", () => { + expect( + contentBlockToParts({ type: "image", data: "", mimeType: "image/png", uri: "file:///tmp/screenshot.png" }), + ).toEqual([{ type: "file", url: "file:///tmp/screenshot.png", filename: "screenshot.png", mime: "image/png" }]) + }) + + test("image block with only a zed path becomes a file part named after the path", () => { + expect( + contentBlockToParts({ type: "image", data: "", mimeType: "image/png", uri: "zed://file?path=/tmp/shot.png" }), + ).toEqual([{ type: "file", url: pathToFileURL("/tmp/shot.png").href, filename: "shot.png", mime: "image/png" }]) }) test("resource_link file URL becomes a file part with name and fallback mime", () => { diff --git a/packages/cli/test/acp/prompt-content.subprocess.test.ts b/packages/cli/test/acp/prompt-content.subprocess.test.ts index 74ad7e89319..ab6ab2053da 100644 --- a/packages/cli/test/acp/prompt-content.subprocess.test.ts +++ b/packages/cli/test/acp/prompt-content.subprocess.test.ts @@ -7,7 +7,7 @@ import { createAcpFixture, expectOk, initialize, newSession } from "./subprocess const tinyPng = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+ip1sAAAAASUVORK5CYII=" describe("acp prompt content subprocess", () => { - test("accepts embedded text resource image and file resource link prompt content", async () => { + test("accepts embedded text resource, inline and remote image, and file resource link prompt content", async () => { await using fixture = await createAcpFixture() await Bun.write(path.join(fixture.home, "README.md"), "# ACP content smoke\n") const acp = fixture.spawn() @@ -41,6 +41,16 @@ describe("acp prompt content subprocess", () => { }), ) + const remote = expectOk( + await acp.request("session/prompt", { + sessionId: session.sessionId, + prompt: [ + { type: "text", text: "Use this remote image." }, + { type: "image", mimeType: "image/png", data: "", uri: "https://example.com/remote.png" }, + ], + }), + ) + const linked = expectOk( await acp.request("session/prompt", { sessionId: session.sessionId, @@ -70,8 +80,9 @@ describe("acp prompt content subprocess", () => { }), ) + expect(remote.stopReason).toBe("end_turn") expect(linked.stopReason).toBe("end_turn") expect(missing.stopReason).toBe("end_turn") - expect(fixture.llm.requests.length).toBeGreaterThanOrEqual(4) + expect(fixture.llm.requests.length).toBeGreaterThanOrEqual(5) }, 60_000) }) diff --git a/packages/cli/test/acp/prompt.test.ts b/packages/cli/test/acp/prompt.test.ts index 0e768d4daab..ca94d586d53 100644 --- a/packages/cli/test/acp/prompt.test.ts +++ b/packages/cli/test/acp/prompt.test.ts @@ -142,6 +142,44 @@ describe("acp prompt turns over the wire", () => { ]) }) + test("attaches local image links and references remote or unreadable ones in place", async () => { + await using dir = await tmpdir() + const local = pathToFileURL(path.join(dir.path, "local.png")).href + const missing = pathToFileURL(path.join(dir.path, "missing.png")).href + await Bun.write(path.join(dir.path, "local.png"), "png") + await using acp = await startSession() + + const response = await acp.prompt(acp.sessionId, [ + { type: "text", text: "compare" }, + { type: "image", data: "", mimeType: "image/png", uri: "https://example.com/remote.png" }, + { type: "image", data: "", mimeType: "image/png", uri: local }, + { type: "image", data: "", mimeType: "image/png", uri: missing }, + ]) + + expect(response.stopReason).toBe("end_turn") + expect(acp.server.submissions).toEqual([ + expect.objectContaining({ + kind: "prompt", + text: `compare\n[remote.png](https://example.com/remote.png)\n[missing.png](${missing})`, + files: [{ uri: local, name: "local.png" }], + }), + ]) + }) + + test("rejects an image with neither data nor uri as invalid params", async () => { + await using acp = await startSession() + + expect( + await rpcError( + acp.prompt(acp.sessionId, [ + { type: "text", text: "look" }, + { type: "image", data: "", mimeType: "image/png" }, + ]), + ), + ).toMatchObject({ code: -32602, message: expect.stringContaining("image content has no data or uri") }) + expect(acp.server.submissions).toEqual([]) + }) + test("returns turn usage and publishes current context usage with cumulative session cost", async () => { const assistantTokens = { input: 100, output: 40, reasoning: 7, cache: { read: 11, write: 13 } } await using acp = await startSession({