-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(server): transcode HEIC attachments instead of failing the turn #5229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| // @effect-diagnostics nodeBuiltinImport:off | ||
| import * as NodeChildProcess from "node:child_process"; | ||
| import * as NodeFSP from "node:fs/promises"; | ||
| import * as NodeOS from "node:os"; | ||
| import * as NodePath from "node:path"; | ||
|
|
||
| import { describe, expect, it } from "vite-plus/test"; | ||
|
|
||
| import { | ||
| isTranscodableImageMimeType, | ||
| TRANSCODED_IMAGE_MIME_TYPE, | ||
| transcodeImageToJpeg, | ||
| } from "./imageTranscode.ts"; | ||
|
|
||
| // 8x8 RGB PNG, used as the source for the HEIC fixture below. | ||
| const SAMPLE_PNG_BASE64 = | ||
| "iVBORw0KGgoAAAANSUhEUgAAAAgAAAAICAIAAABLbSncAAAAbElEQVR4nA3JQQEAMAgDMZRUCUqqpEpQgoh7o2jLN1WFii5cpJhiiyuqhEQLi4gRK04/GjXduEkzzTbXP4xMG5uYMWvOP4JCB4eECRsuPwYNPXjIMMMONz8WLb14yTLLLrc/Dh19+Mgxxx53PKaVZoFj4h8/AAAAAElFTkSuQmCC"; | ||
|
|
||
| const JPEG_START_OF_IMAGE = [0xff, 0xd8, 0xff]; | ||
|
|
||
| function runSips(args: Array<string>): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| NodeChildProcess.execFile("sips", args, (error) => | ||
| error === null ? resolve() : reject(error), | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| // `sips` is the macOS transcoder, and also the only way to build a real HEIC | ||
| // fixture without shipping a binary in the repo. Probing for it keeps this | ||
| // suite green on hosts that do not have it. | ||
| const SIPS_AVAILABLE = await runSips(["--version"]).then( | ||
| () => true, | ||
| () => false, | ||
| ); | ||
|
|
||
| async function makeHeicFixture(): Promise<Uint8Array> { | ||
| const workingDir = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-heic-fixture-")); | ||
| const pngPath = NodePath.join(workingDir, "source.png"); | ||
| const heicPath = NodePath.join(workingDir, "source.heic"); | ||
|
|
||
| try { | ||
| await NodeFSP.writeFile(pngPath, Buffer.from(SAMPLE_PNG_BASE64, "base64")); | ||
| await runSips(["-s", "format", "heic", pngPath, "--out", heicPath]); | ||
| return new Uint8Array(await NodeFSP.readFile(heicPath)); | ||
| } finally { | ||
| await NodeFSP.rm(workingDir, { recursive: true, force: true }).catch(() => {}); | ||
| } | ||
| } | ||
|
|
||
| describe("imageTranscode", () => { | ||
| it("recognizes the mime types Claude cannot ingest", () => { | ||
| expect(isTranscodableImageMimeType("image/heic")).toBe(true); | ||
| expect(isTranscodableImageMimeType("image/heif")).toBe(true); | ||
| expect(isTranscodableImageMimeType("IMAGE/HEIC")).toBe(true); | ||
| expect(isTranscodableImageMimeType(" image/heic ")).toBe(true); | ||
| }); | ||
|
|
||
| it("leaves natively supported mime types alone", () => { | ||
| for (const mimeType of ["image/jpeg", "image/png", "image/gif", "image/webp"]) { | ||
| expect(isTranscodableImageMimeType(mimeType)).toBe(false); | ||
| } | ||
| }); | ||
|
|
||
| it("targets jpeg, which every provider accepts", () => { | ||
| expect(TRANSCODED_IMAGE_MIME_TYPE).toBe("image/jpeg"); | ||
| }); | ||
|
|
||
| it.skipIf(!SIPS_AVAILABLE)("converts HEIC bytes into JPEG bytes", async () => { | ||
| const heic = await makeHeicFixture(); | ||
| // Guards against the fixture silently degrading into a non-HEIC file. | ||
| expect(Buffer.from(heic.subarray(4, 12)).toString("ascii")).toBe("ftypheic"); | ||
|
|
||
| const jpeg = await transcodeImageToJpeg({ | ||
| bytes: heic, | ||
| platform: "darwin", | ||
| }); | ||
|
|
||
| expect(jpeg.byteLength).toBeGreaterThan(0); | ||
| expect([...jpeg.subarray(0, 3)]).toEqual(JPEG_START_OF_IMAGE); | ||
| }); | ||
|
|
||
| it.skipIf(!SIPS_AVAILABLE)("rejects bytes that are not a decodable image", async () => { | ||
| await expect( | ||
| transcodeImageToJpeg({ | ||
| bytes: new TextEncoder().encode("not an image"), | ||
| platform: "darwin", | ||
| }), | ||
| ).rejects.toThrow(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| // @effect-diagnostics nodeBuiltinImport:off | ||
| /** | ||
| * Transcodes image formats the model providers cannot ingest (HEIC/HEIF) into | ||
| * JPEG, which every provider accepts. | ||
| * | ||
| * iPhones capture HEIC by default, so pasting or attaching a photo straight | ||
| * from an Apple device otherwise fails at the provider boundary even though the | ||
| * attachment itself was stored just fine. | ||
| * | ||
| * Transcoding shells out to a tool that ships with the host rather than pulling | ||
| * in a HEIC decoder dependency: `sips` on macOS (always present) and | ||
| * `heif-convert` from libheif elsewhere (packaged on most desktop Linux). When | ||
| * neither is available the caller surfaces the original "unsupported type" | ||
| * failure, so this is strictly additive. | ||
| * | ||
| * @module imageTranscode | ||
| */ | ||
| import * as NodeChildProcess from "node:child_process"; | ||
| import * as NodeFSP from "node:fs/promises"; | ||
| import * as NodeOS from "node:os"; | ||
| import * as NodePath from "node:path"; | ||
|
|
||
| export const TRANSCODABLE_IMAGE_MIME_TYPES: ReadonlySet<string> = new Set([ | ||
| "image/heic", | ||
| "image/heic-sequence", | ||
| "image/heif", | ||
| "image/heif-sequence", | ||
| ]); | ||
|
|
||
| export const TRANSCODED_IMAGE_MIME_TYPE = "image/jpeg"; | ||
|
|
||
| /** Whether `mimeType` is one we can convert into a provider-supported format. */ | ||
| export function isTranscodableImageMimeType(mimeType: string): boolean { | ||
| return TRANSCODABLE_IMAGE_MIME_TYPES.has(mimeType.trim().toLowerCase()); | ||
| } | ||
|
|
||
| interface Transcoder { | ||
| readonly command: string; | ||
| readonly args: (input: { | ||
| readonly inputPath: string; | ||
| readonly outputPath: string; | ||
| }) => Array<string>; | ||
| } | ||
|
|
||
| const SIPS_TRANSCODER: Transcoder = { | ||
| command: "sips", | ||
| args: ({ inputPath, outputPath }) => ["-s", "format", "jpeg", inputPath, "--out", outputPath], | ||
| }; | ||
|
|
||
| const HEIF_CONVERT_TRANSCODER: Transcoder = { | ||
| command: "heif-convert", | ||
| args: ({ inputPath, outputPath }) => [inputPath, outputPath], | ||
| }; | ||
|
|
||
| function transcodersFor(platform: NodeJS.Platform): ReadonlyArray<Transcoder> { | ||
| return platform === "darwin" ? [SIPS_TRANSCODER] : [HEIF_CONVERT_TRANSCODER]; | ||
| } | ||
|
|
||
| function runTranscoder(input: { | ||
| readonly transcoder: Transcoder; | ||
| readonly inputPath: string; | ||
| readonly outputPath: string; | ||
| }): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| NodeChildProcess.execFile( | ||
| input.transcoder.command, | ||
| input.transcoder.args({ | ||
| inputPath: input.inputPath, | ||
| outputPath: input.outputPath, | ||
| }), | ||
| // A photo is a bounded workload; the cap only guards against a wedged | ||
| // helper process holding the turn open forever. | ||
| { timeout: 30_000, maxBuffer: 1024 * 1024 }, | ||
| (error) => (error === null ? resolve() : reject(error)), | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Converts HEIC/HEIF bytes to JPEG bytes. | ||
| * | ||
| * Rejects when no transcoder is available on the host or the conversion fails, | ||
| * so callers can fall back to reporting the attachment as unsupported. | ||
| */ | ||
| export async function transcodeImageToJpeg(input: { | ||
| readonly bytes: Uint8Array; | ||
| readonly platform: NodeJS.Platform; | ||
| }): Promise<Uint8Array> { | ||
| const workingDir = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-image-transcode-")); | ||
| const inputPath = NodePath.join(workingDir, "input"); | ||
| const outputPath = NodePath.join(workingDir, "output.jpg"); | ||
|
|
||
| try { | ||
| await NodeFSP.writeFile(inputPath, input.bytes); | ||
|
|
||
| let lastError: unknown = new Error("No image transcoder is available on this host."); | ||
| for (const transcoder of transcodersFor(input.platform)) { | ||
| try { | ||
| await runTranscoder({ transcoder, inputPath, outputPath }); | ||
| const converted = await NodeFSP.readFile(outputPath); | ||
| if (converted.byteLength === 0) { | ||
| // Some builds of `sips` exit 0 after writing nothing when the input | ||
| // is not decodable, so an empty result has to count as a failure. | ||
| throw new Error(`${transcoder.command} produced an empty image.`); | ||
| } | ||
| return new Uint8Array(converted); | ||
| } catch (error) { | ||
| lastError = error; | ||
| } | ||
| } | ||
|
|
||
| throw lastError; | ||
| } finally { | ||
| await NodeFSP.rm(workingDir, { recursive: true, force: true }).catch(() => {}); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import { | |
| type ModelUsage, | ||
| } from "@anthropic-ai/claude-agent-sdk"; | ||
| import { parseCliArgs } from "@t3tools/shared/cliArgs"; | ||
| import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; | ||
| import { | ||
| ApprovalRequestId, | ||
| type CanonicalItemType, | ||
|
|
@@ -69,6 +70,11 @@ import * as Stream from "effect/Stream"; | |
|
|
||
| import { resolveAttachmentPath } from "../../attachmentStore.ts"; | ||
| import { ServerConfig } from "../../config.ts"; | ||
| import { | ||
| isTranscodableImageMimeType, | ||
| TRANSCODED_IMAGE_MIME_TYPE, | ||
| transcodeImageToJpeg, | ||
| } from "../../imageTranscode.ts"; | ||
| import * as McpProviderSession from "../../mcp/McpProviderSession.ts"; | ||
| import { resolveClaudeSdkExecutablePath } from "../Drivers/ClaudeExecutable.ts"; | ||
| import { makeClaudeEnvironment } from "../Drivers/ClaudeHome.ts"; | ||
|
|
@@ -951,7 +957,11 @@ const buildUserMessageEffect = Effect.fn("buildUserMessageEffect")(function* ( | |
| continue; | ||
| } | ||
|
|
||
| if (!SUPPORTED_CLAUDE_IMAGE_MIME_TYPES.has(attachment.mimeType)) { | ||
| // HEIC/HEIF is the default capture format on Apple devices, so it arrives | ||
| // constantly via paste and the mobile composer. Convert it instead of | ||
| // failing the turn. | ||
| const needsTranscode = isTranscodableImageMimeType(attachment.mimeType); | ||
| if (!needsTranscode && !SUPPORTED_CLAUDE_IMAGE_MIME_TYPES.has(attachment.mimeType)) { | ||
| return yield* new ProviderAdapterRequestError({ | ||
| provider: PROVIDER, | ||
| method: "turn/start", | ||
|
|
@@ -983,6 +993,28 @@ const buildUserMessageEffect = Effect.fn("buildUserMessageEffect")(function* ( | |
| ), | ||
| ); | ||
|
|
||
| if (needsTranscode) { | ||
| const hostPlatform = yield* HostProcessPlatform; | ||
| const converted = yield* Effect.tryPromise({ | ||
|
Comment on lines
+996
to
+998
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the service boundary where the imperative helper is pulled in. Once Posted via Macroscope — Effect Service Conventions |
||
| try: () => transcodeImageToJpeg({ bytes, platform: hostPlatform }), | ||
| catch: (cause) => | ||
| new ProviderAdapterRequestError({ | ||
| provider: PROVIDER, | ||
| method: "turn/start", | ||
| detail: `Unsupported Claude image attachment type '${attachment.mimeType}' and converting it to JPEG failed.`, | ||
| cause, | ||
| }), | ||
| }); | ||
|
|
||
| sdkContent.push( | ||
| buildClaudeImageContentBlock({ | ||
| mimeType: TRANSCODED_IMAGE_MIME_TYPE, | ||
| bytes: converted, | ||
| }), | ||
| ); | ||
| continue; | ||
| } | ||
|
|
||
| sdkContent.push( | ||
| buildClaudeImageContentBlock({ | ||
| mimeType: attachment.mimeType, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module spawns a child process and does temp-file I/O with raw Node APIs, and it is then consumed from inside the
ClaudeAdapterEffect service viaEffect.tryPromise. Per the dependency-acquisition convention, an imperative Promise adapter shouldn't become a dependency of another Effect service — runtime-backed dependencies should be acquired from the environment.The server already models both of these as services:
ProcessRunner(apps/server/src/processRunner.ts, withrun, timeouts, and taggedProcessSpawnError/ProcessTimeoutError/… failures) andFileSystem.FileSystem(makeTempDirectoryScoped, used elsewhere for exactly this scratch-dir pattern, e.g.apps/server/src/atomicWrite.ts).Suggested shape: make
transcodeImageToJpeganEffectthat doesyield* ProcessRunner.ProcessRunnerandyield* FileSystem.FileSystem, and fail with aSchema.TaggedErrorClass(e.g.ImageTranscodeErrorcarrying the transcoder command and the underlyingcause) instead ofthrow new Error(...)/throw lastError. Theplatformparameter can stay as-is — that's pure configuration, not service injection. That also lets thenodeBuiltinImport:offsuppression at the top of the file be dropped.Posted via Macroscope — Effect Service Conventions