From f9b11e584fc9a3dfd64875f985defabf31bcece9 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 16 Aug 2026 20:33:55 +1000 Subject: [PATCH 1/2] Keep an image's format when downscaling it Conditioning re-encoded every image as PNG. PNG is lossless, so a photograph comes out larger than the JPEG it came from despite having fewer pixels: ten photos totalling 7.6 MB became 30.6 MB, and the request carrying them was refused for exceeding the maximum size. A webp source is the one that cannot keep its format, because sips reads webp but cannot write it. --- .../test/CommandIntentExecutor.spec.ts | 2 +- .../test/CommandKeyHandler.spec.ts | 2 +- apps/claude-sdk-cli/test/ViewHost.spec.ts | 2 +- packages/claude-core/CHANGELOG.md | 1 + packages/claude-core/changes.jsonl | 1 + .../claude-core/src/image/NodeSipsBridge.ts | 8 +- packages/claude-core/src/image/SipsBridge.ts | 5 +- .../claude-core/src/image/conditionImage.ts | 27 ++++-- .../claude-core/test/conditionImage.spec.ts | 97 ++++++++++++++++--- .../claude-sdk-tools/test/ReadFile.spec.ts | 14 +-- packages/claude-sdk-tools/test/helpers.ts | 2 +- 11 files changed, 123 insertions(+), 38 deletions(-) diff --git a/apps/claude-sdk-cli/test/CommandIntentExecutor.spec.ts b/apps/claude-sdk-cli/test/CommandIntentExecutor.spec.ts index 3f7db8e4..c295971f 100644 --- a/apps/claude-sdk-cli/test/CommandIntentExecutor.spec.ts +++ b/apps/claude-sdk-cli/test/CommandIntentExecutor.spec.ts @@ -38,7 +38,7 @@ import { MemoryObjectStore } from './MemoryObjectStore.js'; /** Test double: sips unavailable, so pasted images pass through unconditioned. */ const passthroughSips: SipsBridge = { dimensions: () => Promise.reject(new Error('no sips in tests')), - resizeToPng: () => Promise.reject(new Error('no sips in tests')), + resize: () => Promise.reject(new Error('no sips in tests')), }; /** Test double: a logger that discards everything, so the executor resolves without the app's logger. */ diff --git a/apps/claude-sdk-cli/test/CommandKeyHandler.spec.ts b/apps/claude-sdk-cli/test/CommandKeyHandler.spec.ts index 49d771f5..be216d32 100644 --- a/apps/claude-sdk-cli/test/CommandKeyHandler.spec.ts +++ b/apps/claude-sdk-cli/test/CommandKeyHandler.spec.ts @@ -41,7 +41,7 @@ const flush = () => new Promise((resolve) => setImmediate(resolve)); /** Test double: sips unavailable, so pasted images pass through unconditioned. */ const passthroughSips: SipsBridge = { dimensions: () => Promise.reject(new Error('no sips in tests')), - resizeToPng: () => Promise.reject(new Error('no sips in tests')), + resize: () => Promise.reject(new Error('no sips in tests')), }; /** Test double: a logger that discards everything, so the executor resolves without the app's logger. */ diff --git a/apps/claude-sdk-cli/test/ViewHost.spec.ts b/apps/claude-sdk-cli/test/ViewHost.spec.ts index 3dced99b..21300c9e 100644 --- a/apps/claude-sdk-cli/test/ViewHost.spec.ts +++ b/apps/claude-sdk-cli/test/ViewHost.spec.ts @@ -81,7 +81,7 @@ function makeTurnClock(): ITurnClock { /** Test double: sips unavailable, so pasted images pass through unconditioned. */ const passthroughSips: SipsBridge = { dimensions: () => Promise.reject(new Error('no sips in tests')), - resizeToPng: () => Promise.reject(new Error('no sips in tests')), + resize: () => Promise.reject(new Error('no sips in tests')), }; /** Test double: a logger that discards everything, so the executor resolves without the app's logger. */ diff --git a/packages/claude-core/CHANGELOG.md b/packages/claude-core/CHANGELOG.md index 4f2b4fc7..0c544688 100644 --- a/packages/claude-core/CHANGELOG.md +++ b/packages/claude-core/CHANGELOG.md @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A downscaled image keeps its source format rather than being re-encoded as PNG, which inflated photographs about fourfold. A webp is the exception and becomes a png, because sips reads webp but cannot write it - Config merge now recurses arbitrarily deep instead of stopping after one nested level, so a local override several levels down (e.g. one entry of a nested record) no longer silently replaces its whole containing object and drops its siblings - Fix a perceived ~500ms lag on every Escape keypress: a raw stdin chunk containing only the ESC byte is now emitted immediately as an escape KeyAction instead of waiting on readline's internal CSI-sequence disambiguation timeout - Fix absent-file and inode-swap defects in config file watching diff --git a/packages/claude-core/changes.jsonl b/packages/claude-core/changes.jsonl index 802ac550..d78dd8b0 100644 --- a/packages/claude-core/changes.jsonl +++ b/packages/claude-core/changes.jsonl @@ -28,3 +28,4 @@ {"description":"IFileSystem gains tmpdir, uid, mkdir with an explicit mode, lstat, and a synchronous readlinkSync, so code that needs a temporary directory, or has to create one and check who owns it, can reach all of it through the filesystem seam instead of node:os and node:fs directly","category":"added"} {"description":"StatResult now carries uid and mode, so a caller can tell who owns a path and who else can reach it","category":"added"} {"description":"canonicalisePath resolves a path to where it actually lands, following symlinks even when the target does not exist yet, for callers that must decide on the destination rather than the string they were handed","category":"added"} +{"description":"A downscaled image keeps its source format rather than being re-encoded as PNG, which inflated photographs about fourfold. A webp is the exception and becomes a png, because sips reads webp but cannot write it","category":"fixed"} diff --git a/packages/claude-core/src/image/NodeSipsBridge.ts b/packages/claude-core/src/image/NodeSipsBridge.ts index ec61d667..412b4c7c 100644 --- a/packages/claude-core/src/image/NodeSipsBridge.ts +++ b/packages/claude-core/src/image/NodeSipsBridge.ts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { promisify } from 'node:util'; import { buildDimensionArgs, buildResizeArgs, parseDimensions } from './conditionImage.js'; -import { type ImageDimensions, SipsBridge } from './SipsBridge.js'; +import { type ImageDimensions, SipsBridge, type SipsFormat } from './SipsBridge.js'; const execFileAsync = promisify(execFile); const SIPS_TIMEOUT_MS = 10_000; @@ -26,12 +26,12 @@ export class NodeSipsBridge extends SipsBridge { }); } - public async resizeToPng(input: Buffer): Promise { + public async resize(input: Buffer, format: SipsFormat): Promise { return this.#withTempDir(async (dir) => { const inputPath = join(dir, 'input'); - const outputPath = join(dir, 'output.png'); + const outputPath = join(dir, `output.${format}`); await writeFile(inputPath, input); - await execFileAsync('sips', buildResizeArgs(inputPath, outputPath), { timeout: SIPS_TIMEOUT_MS }); + await execFileAsync('sips', buildResizeArgs(inputPath, outputPath, format), { timeout: SIPS_TIMEOUT_MS }); return readFile(outputPath); }); } diff --git a/packages/claude-core/src/image/SipsBridge.ts b/packages/claude-core/src/image/SipsBridge.ts index f6f3f54c..304070e0 100644 --- a/packages/claude-core/src/image/SipsBridge.ts +++ b/packages/claude-core/src/image/SipsBridge.ts @@ -1,5 +1,8 @@ export type ImageDimensions = { readonly width: number; readonly height: number }; +/** Formats sips can write. No webp: sips reads it but exits 13 when asked to produce it. */ +export type SipsFormat = 'png' | 'jpeg' | 'gif'; + /** * The child-process boundary for image conditioning — the one place the real `sips` binary runs. * Injected so `conditionImage` is tested without the binary. Every method rejects when sips is @@ -7,5 +10,5 @@ export type ImageDimensions = { readonly width: number; readonly height: number */ export abstract class SipsBridge { public abstract dimensions(input: Buffer): Promise; - public abstract resizeToPng(input: Buffer): Promise; + public abstract resize(input: Buffer, format: SipsFormat): Promise; } diff --git a/packages/claude-core/src/image/conditionImage.ts b/packages/claude-core/src/image/conditionImage.ts index ca9f2d96..0a62332d 100644 --- a/packages/claude-core/src/image/conditionImage.ts +++ b/packages/claude-core/src/image/conditionImage.ts @@ -1,5 +1,5 @@ import type { ILogger } from '../logging/ILogger.js'; -import type { SipsBridge } from './SipsBridge.js'; +import type { SipsBridge, SipsFormat } from './SipsBridge.js'; /** The image media types both attach paths already emit (paste: clipboard.ts detectMediaType; ReadFile: file-type sniff). */ export type ImageMediaType = 'image/png' | 'image/jpeg' | 'image/gif' | 'image/webp'; @@ -15,10 +15,18 @@ export function buildDimensionArgs(inputPath: string): string[] { return ['-g', 'pixelWidth', '-g', 'pixelHeight', inputPath]; } -/** sips argument vector that downscales to a <=2000px long edge (aspect kept) and re-encodes as PNG. +/** Re-encode target per source type. webp maps to png because sips reads webp but cannot write it. */ +export const RESIZE_OUTPUT = { + 'image/jpeg': { format: 'jpeg', mediaType: 'image/jpeg' }, + 'image/png': { format: 'png', mediaType: 'image/png' }, + 'image/gif': { format: 'gif', mediaType: 'image/gif' }, + 'image/webp': { format: 'png', mediaType: 'image/png' }, +} as const satisfies Record; + +/** sips argument vector that downscales to a <=2000px long edge (aspect kept) and re-encodes as `format`. * `-Z` never enlarges *once gated by dimensions* — it is only ever invoked here for an oversized image. */ -export function buildResizeArgs(inputPath: string, outputPath: string): string[] { - return ['-Z', String(MAX_LONG_EDGE), '-s', 'format', 'png', inputPath, '--out', outputPath]; +export function buildResizeArgs(inputPath: string, outputPath: string, format: SipsFormat): string[] { + return ['-Z', String(MAX_LONG_EDGE), '-s', 'format', format, inputPath, '--out', outputPath]; } /** Parse `pixelWidth: N` / `pixelHeight: N` out of `sips -g` stdout. @@ -34,8 +42,8 @@ export function parseDimensions(stdout: string): { width: number; height: number } /** - * Condition an image for attachment: downscale to a <=2000px long edge as PNG when it is larger, - * otherwise leave it exactly as-is. Any sips problem (absent, not invocable, or a failure on this + * Condition an image for attachment: downscale to a <=2000px long edge, keeping its source format, + * when it is larger, otherwise leave it exactly as-is. Any sips problem (absent, not invocable, or a failure on this * image) degrades to the original bytes and media type — a conditioner must never block an attachment. * * Each outcome is logged the moment it happens (not aggregated at the call site) so the timestamp is @@ -50,9 +58,10 @@ export async function conditionImage(input: Buffer, mediaType: ImageMediaType, s logger.debug(`image conditioning: long edge is within ${MAX_LONG_EDGE}px, attaching unchanged`); return { data: input, mediaType }; } - const data = await sips.resizeToPng(input); - logger.debug(`image conditioning: long edge exceeds ${MAX_LONG_EDGE}px, downscaled to a ${MAX_LONG_EDGE}px long edge and re-encoded as PNG (${input.length} -> ${data.length} bytes)`); - return { data, mediaType: 'image/png' }; + const output = RESIZE_OUTPUT[mediaType]; + const data = await sips.resize(input, output.format); + logger.debug(`image conditioning: long edge exceeds ${MAX_LONG_EDGE}px, downscaled to a ${MAX_LONG_EDGE}px long edge as ${output.format} (${input.length} -> ${data.length} bytes)`); + return { data, mediaType: output.mediaType }; } catch (error) { logger.warn(`image conditioning: sips unavailable or failed, attaching image unchanged (${input.length} bytes)`, { error }); return { data: input, mediaType }; diff --git a/packages/claude-core/test/conditionImage.spec.ts b/packages/claude-core/test/conditionImage.spec.ts index dc7a07fa..182e781b 100644 --- a/packages/claude-core/test/conditionImage.spec.ts +++ b/packages/claude-core/test/conditionImage.spec.ts @@ -1,37 +1,62 @@ import { describe, expect, it } from 'vitest'; +import type { ImageMediaType } from '../src/image/conditionImage'; import { buildDimensionArgs, buildResizeArgs, conditionImage, parseDimensions } from '../src/image/conditionImage'; -import type { SipsBridge } from '../src/image/SipsBridge'; +import type { SipsBridge, SipsFormat } from '../src/image/SipsBridge'; import type { ILogger } from '../src/logging/ILogger'; const noopLogger: ILogger = { trace: () => {}, debug: () => {}, info: () => {}, warn: () => {}, error: () => {} }; -const PNG_BYTES = Buffer.from('conditioned-png-bytes'); +const RESIZED_BYTES = Buffer.from('conditioned-bytes'); const resizes: SipsBridge = { dimensions: () => Promise.resolve({ width: 4000, height: 3000 }), - resizeToPng: () => Promise.resolve(PNG_BYTES), + resize: () => Promise.resolve(RESIZED_BYTES), }; const smallEnough: SipsBridge = { dimensions: () => Promise.resolve({ width: 1500, height: 800 }), - resizeToPng: () => Promise.reject(new Error('resize must not be called for a small image')), + resize: () => Promise.reject(new Error('resize must not be called for a small image')), }; const absent: SipsBridge = { dimensions: () => Promise.reject(new Error('spawn sips ENOENT')), - resizeToPng: () => Promise.reject(new Error('spawn sips ENOENT')), + resize: () => Promise.reject(new Error('spawn sips ENOENT')), }; const notInvocable: SipsBridge = { dimensions: () => Promise.reject(new Error('spawn sips EACCES')), - resizeToPng: () => Promise.reject(new Error('spawn sips EACCES')), + resize: () => Promise.reject(new Error('spawn sips EACCES')), }; const failsOnImage: SipsBridge = { dimensions: () => Promise.resolve({ width: 4000, height: 3000 }), - resizeToPng: () => Promise.reject(new Error('sips exited 13')), + resize: () => Promise.reject(new Error('sips exited 13')), +}; + +/** Records the format sips was asked to produce, so the mapping can be asserted at the boundary. */ +const recordingBridge = () => { + const formats: SipsFormat[] = []; + const bridge: SipsBridge = { + dimensions: () => Promise.resolve({ width: 4000, height: 3000 }), + resize: (_input, format) => { + formats.push(format); + return Promise.resolve(RESIZED_BYTES); + }, + }; + return { bridge, formats }; +}; + +const formatAskedFor = async (mediaType: ImageMediaType): Promise => { + const { bridge, formats } = recordingBridge(); + await conditionImage(Buffer.from('orig'), mediaType, bridge, noopLogger); + return formats[0]; +}; + +const mediaTypeReturned = async (mediaType: ImageMediaType): Promise => { + const { mediaType: actual } = await conditionImage(Buffer.from('orig'), mediaType, resizes, noopLogger); + return actual; }; describe('buildResizeArgs', () => { - it('builds a 2000px downscale-to-PNG sips invocation', () => { - const expected = ['-Z', '2000', '-s', 'format', 'png', '/tmp/in', '--out', '/tmp/out.png']; - const actual = buildResizeArgs('/tmp/in', '/tmp/out.png'); + it('builds a 2000px downscale invocation in the requested format', () => { + const expected = ['-Z', '2000', '-s', 'format', 'jpeg', '/tmp/in', '--out', '/tmp/out.jpeg']; + const actual = buildResizeArgs('/tmp/in', '/tmp/out.jpeg', 'jpeg'); expect(actual).toEqual(expected); }); }); @@ -54,14 +79,60 @@ describe('parseDimensions', () => { describe('conditionImage — resizes an oversized image', () => { it('uses the conditioned bytes', async () => { - const expected = PNG_BYTES; + const expected = RESIZED_BYTES; const { data: actual } = await conditionImage(Buffer.from('orig'), 'image/jpeg', resizes, noopLogger); expect(actual).toBe(expected); }); +}); + +describe('conditionImage — the format it asks sips to produce', () => { + it('re-encodes a jpeg as jpeg', async () => { + const expected = 'jpeg'; + const actual = await formatAskedFor('image/jpeg'); + expect(actual).toBe(expected); + }); + + it('re-encodes a png as png', async () => { + const expected = 'png'; + const actual = await formatAskedFor('image/png'); + expect(actual).toBe(expected); + }); + + it('re-encodes a gif as gif', async () => { + const expected = 'gif'; + const actual = await formatAskedFor('image/gif'); + expect(actual).toBe(expected); + }); + + it('re-encodes a webp as png, which sips can write', async () => { + const expected = 'png'; + const actual = await formatAskedFor('image/webp'); + expect(actual).toBe(expected); + }); +}); + +describe('conditionImage — the media type it reports after resizing', () => { + it('reports image/jpeg for a jpeg', async () => { + const expected = 'image/jpeg'; + const actual = await mediaTypeReturned('image/jpeg'); + expect(actual).toBe(expected); + }); + + it('reports image/png for a png', async () => { + const expected = 'image/png'; + const actual = await mediaTypeReturned('image/png'); + expect(actual).toBe(expected); + }); + + it('reports image/gif for a gif', async () => { + const expected = 'image/gif'; + const actual = await mediaTypeReturned('image/gif'); + expect(actual).toBe(expected); + }); - it('reports image/png after resizing', async () => { + it('reports image/png for a webp', async () => { const expected = 'image/png'; - const { mediaType: actual } = await conditionImage(Buffer.from('orig'), 'image/jpeg', resizes, noopLogger); + const actual = await mediaTypeReturned('image/webp'); expect(actual).toBe(expected); }); }); diff --git a/packages/claude-sdk-tools/test/ReadFile.spec.ts b/packages/claude-sdk-tools/test/ReadFile.spec.ts index 616d90c8..1a12bb34 100644 --- a/packages/claude-sdk-tools/test/ReadFile.spec.ts +++ b/packages/claude-sdk-tools/test/ReadFile.spec.ts @@ -516,7 +516,7 @@ import type { SipsBridge } from '@shellicar/claude-core/image/SipsBridge'; const neverSips: SipsBridge = { dimensions: () => Promise.reject(new Error('sips must not run for a non-image')), - resizeToPng: () => Promise.reject(new Error('sips must not run for a non-image')), + resize: () => Promise.reject(new Error('sips must not run for a non-image')), }; describe('createReadFile — conditioning leaves non-images alone', () => { @@ -532,29 +532,29 @@ describe('createReadFile — conditioning leaves non-images alone', () => { }); }); -const conditionedPng = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0xaa, 0xbb]); +const conditionedBytes = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0xaa, 0xbb]); const resizes: SipsBridge = { dimensions: () => Promise.resolve({ width: 4000, height: 3000 }), - resizeToPng: () => Promise.resolve(conditionedPng), + resize: () => Promise.resolve(conditionedBytes), }; describe('createReadFile — conditions an oversized image', () => { - it('emits the conditioned PNG bytes', async () => { + it('emits the conditioned bytes', async () => { const fs = new MemoryFileSystem({ '/images/big.jpg': jpegMagic }); const ReadFile = createReadFile(fs, resizes, noopLogger); const result = await callFull(ReadFile, { path: '/images/big.jpg', mimeType: 'image/*' }); - const expected = conditionedPng.toString('base64'); + const expected = conditionedBytes.toString('base64'); const actual = result.attachments?.[0]?.source.data; expect(actual).toBe(expected); }); - it('re-labels the conditioned image as image/png', async () => { + it('keeps a conditioned jpeg labelled as image/jpeg', async () => { const fs = new MemoryFileSystem({ '/images/big.jpg': jpegMagic }); const ReadFile = createReadFile(fs, resizes, noopLogger); const result = await callFull(ReadFile, { path: '/images/big.jpg', mimeType: 'image/*' }); - const expected = 'image/png'; + const expected = 'image/jpeg'; const actual = result.attachments?.[0]?.source.media_type; expect(actual).toBe(expected); }); diff --git a/packages/claude-sdk-tools/test/helpers.ts b/packages/claude-sdk-tools/test/helpers.ts index ba2023ee..b10feb0b 100644 --- a/packages/claude-sdk-tools/test/helpers.ts +++ b/packages/claude-sdk-tools/test/helpers.ts @@ -6,7 +6,7 @@ import type { z } from 'zod'; /** Test double: sips unavailable, so ReadFile images pass through unconditioned. */ export const passthroughSips: SipsBridge = { dimensions: () => Promise.reject(new Error('no sips in tests')), - resizeToPng: () => Promise.reject(new Error('no sips in tests')), + resize: () => Promise.reject(new Error('no sips in tests')), }; /** Test double: a logger that discards everything, so the tool builds without the app's logger. */ From c8f5ec2a517779e8f594e38ba8d79af3923dc54b Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 16 Aug 2026 20:44:54 +1000 Subject: [PATCH 2/2] Say the change in five words --- packages/claude-core/CHANGELOG.md | 2 +- packages/claude-core/changes.jsonl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/claude-core/CHANGELOG.md b/packages/claude-core/CHANGELOG.md index 0c544688..087d6e1c 100644 --- a/packages/claude-core/CHANGELOG.md +++ b/packages/claude-core/CHANGELOG.md @@ -39,10 +39,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- A downscaled image keeps its source format rather than being re-encoded as PNG, which inflated photographs about fourfold. A webp is the exception and becomes a png, because sips reads webp but cannot write it - Config merge now recurses arbitrarily deep instead of stopping after one nested level, so a local override several levels down (e.g. one entry of a nested record) no longer silently replaces its whole containing object and drops its siblings - Fix a perceived ~500ms lag on every Escape keypress: a raw stdin chunk containing only the ESC byte is now emitted immediately as an escape KeyAction instead of waiting on readline's internal CSI-sequence disambiguation timeout - Fix absent-file and inode-swap defects in config file watching - Fix version metadata - Package now publishes CJS alongside ESM with working sourcemaps +- Preserve image format when downscaling - Re-establish ANSI colour state on wrapped continuation lines diff --git a/packages/claude-core/changes.jsonl b/packages/claude-core/changes.jsonl index d78dd8b0..d46051a4 100644 --- a/packages/claude-core/changes.jsonl +++ b/packages/claude-core/changes.jsonl @@ -28,4 +28,4 @@ {"description":"IFileSystem gains tmpdir, uid, mkdir with an explicit mode, lstat, and a synchronous readlinkSync, so code that needs a temporary directory, or has to create one and check who owns it, can reach all of it through the filesystem seam instead of node:os and node:fs directly","category":"added"} {"description":"StatResult now carries uid and mode, so a caller can tell who owns a path and who else can reach it","category":"added"} {"description":"canonicalisePath resolves a path to where it actually lands, following symlinks even when the target does not exist yet, for callers that must decide on the destination rather than the string they were handed","category":"added"} -{"description":"A downscaled image keeps its source format rather than being re-encoded as PNG, which inflated photographs about fourfold. A webp is the exception and becomes a png, because sips reads webp but cannot write it","category":"fixed"} +{"description":"Preserve image format when downscaling","category":"fixed"}