From 6802d551c4cda910b2f5e0f152178293874aa245 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 16 Aug 2026 23:14:18 +0000 Subject: [PATCH] fix(tui): prefer macOS file URL over Finder icon bitmap on image paste Current Finder Cmd+C pasteboards advertise both a public.file-url and a generated 1024x1024 file-icon bitmap. arboard::get_image() succeeded with the icon, so handleImagePaste attached it before readMacFileUrlsFromClipboard was reached, sending a generic PNG document icon to vision models instead of the copied screenshot. Probe the Darwin file URLs before the bitmap representation so an image-file URL wins over the co-advertised icon. Pure bitmap pasteboards (screenshots, browser copies) and non-image file URLs still fall through to the image/text paths. readMacFileUrls is a no-op off Darwin. Fixes #8769 --- packages/coding-agent/CHANGELOG.md | 4 + .../src/modes/controllers/input-controller.ts | 53 ++++--- .../test/issue-8769-repro.test.ts | 148 ++++++++++++++++++ 3 files changed, 184 insertions(+), 21 deletions(-) create mode 100644 packages/coding-agent/test/issue-8769-repro.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4901d84f7..77ff2a79b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the clipboard image-paste keybind attaching Finder's generated file icon instead of the copied image on macOS. Current Finder `Cmd+C` pasteboards advertise both a `public.file-url` and a generated 1024x1024 icon bitmap, so `arboard::get_image()` succeeded with the icon and `InputController.handleImagePaste` attached it before the file-URL branch was ever reached. The handler now probes `readMacFileUrlsFromClipboard()` before the bitmap representation, so an image file URL wins over the co-advertised icon; pure bitmap pasteboards (screenshots, browser copies) and non-image file URLs still fall through to the image/text paths ([#8769](https://github.com/can1357/oh-my-pi/issues/8769)). + ## [17.3.5] - 2026-08-16 ### Added diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index fa59659ea..d0d260fda 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -1637,6 +1637,38 @@ export class InputController { const focusedNow = this.ctx.ui.getFocused(); const promptTarget = focusedNow && focusedNow !== this.ctx.editor && hasPasteText(focusedNow) ? focusedNow : null; + // #8769: On macOS, Finder `Cmd+C` on an image file puts BOTH a + // `public.file-url` representation and a generated 1024x1024 + // file-icon bitmap on the pasteboard. `arboard::get_image()` + // succeeds with the icon, so probing the image representation first + // would attach the generic Finder icon instead of the copied + // screenshot — a vision model then sees a white `PNG` document + // icon. Probe the file URLs before the bitmap and let any that + // resolve to a supported image file win over the icon: the + // authoritative file bytes are what the user copied. + // + // #3506: this branch also recovers file-url-only pasteboards + // (Finder selections, certain screenshot tools) where + // `arboard::get_image()` returns `ContentNotAvailable` and + // `pbpaste` is empty. Every image-shaped path routes through + // {@link handleImagePathPaste}, matching the bracketed-paste + // handler in `CustomEditor.handleInput`; multi-image Finder + // selections must not silently drop after the first attach. + // `readMacFileUrls` returns an empty list off Darwin, so on every + // other platform this is a no-op and the bitmap read below still + // runs first. + const fileUrls = promptTarget ? [] : ((await this.clipboard.readMacFileUrls?.()) ?? []); + let attachedFromFileUrls = false; + for (const url of fileUrls) { + const candidate = extractImagePathFromText(url); + if (!candidate) continue; + await this.handleImagePathPaste(candidate); + attachedFromFileUrls = true; + } + if (attachedFromFileUrls) return true; + // No usable image-file URL (pure bitmap pasteboard: screenshots, + // browser copies, or a non-image Finder selection). Fall to the + // image representation. const image = await this.clipboard.readImage(); if (image) { if (promptTarget) { @@ -1652,27 +1684,6 @@ export class InputController { `Unsupported clipboard image format: ${image.mimeType}`, ); } - // #3506: macOS Finder `Cmd+C` puts only a `public.file-url` - // representation on the pasteboard. `pbpaste` (the backing call - // for `readText` on Darwin) only surfaces plain text / RTF / EPS, - // so it returns empty for file-url-only pasteboards — the smart - // text fallback below would dead-end with "Clipboard is empty". - // Reach the file URL directly via AppleScript and route every - // image-shaped path through {@link handleImagePathPaste}, matching - // the bracketed-paste handler in `CustomEditor.handleInput` which - // iterates every extracted image path. Multi-image Finder - // selections must not silently drop after the first attach. - // `readMacFileUrls` returns an empty list off Darwin, so the - // check is free on every other platform. - const fileUrls = promptTarget ? [] : ((await this.clipboard.readMacFileUrls?.()) ?? []); - let attachedFromFileUrls = false; - for (const url of fileUrls) { - const candidate = extractImagePathFromText(url); - if (!candidate) continue; - await this.handleImagePathPaste(candidate); - attachedFromFileUrls = true; - } - if (attachedFromFileUrls) return true; // Smart paste (#1628): no image on the clipboard — fall back to // pasting its text so the same chord covers both payload kinds. // Hosts that pre-empt the terminal's own paste (VS Code's diff --git a/packages/coding-agent/test/issue-8769-repro.test.ts b/packages/coding-agent/test/issue-8769-repro.test.ts new file mode 100644 index 000000000..136afeea7 --- /dev/null +++ b/packages/coding-agent/test/issue-8769-repro.test.ts @@ -0,0 +1,148 @@ +/** + * Repro for #8769: macOS Finder image paste attaches the generated file icon + * instead of the copied image's bytes. + * + * Current Finder `Cmd+C` pasteboards advertise BOTH a `public.file-url` + * representation and a generated 1024x1024 file-icon bitmap. `arboard::get_image` + * succeeds with the icon, so `InputController.handleImagePaste` — which probed + * the image representation before `readMacFileUrlsFromClipboard` — attached the + * generic Finder icon and never reached the authoritative file URL. A vision + * model then saw a white document icon labelled `PNG` instead of the screenshot. + * + * Defended contract: when the pasteboard exposes a file URL resolving to a + * supported image file, `handleImagePaste` MUST attach that file's bytes and + * NEVER let the co-advertised icon bitmap win. Non-image file URLs and pure + * bitmap pasteboards (screenshots, browser copies) still fall to the image + * representation. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import type { ImageContent } from "@oh-my-pi/pi-ai"; +import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { InputController } from "@oh-my-pi/pi-coding-agent/modes/controllers/input-controller"; +import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; + +// A real, decodable 1x1 PNG standing in for the copied screenshot's bytes. +const FILE_PNG = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M8AAAMBAQDJ/pLvAAAAAElFTkSuQmCC", + "base64", +); + +// A distinct payload standing in for Finder's generated file-icon bitmap that +// `arboard::get_image` returns. Byte-different from FILE_PNG so the test can +// tell which representation was attached. +const ICON_PNG_BASE64 = + "iVBORw0KGgoAAAANSUhEUgAAAAIAAAACCAYAAABytg0kAAAAEklEQVR42mP8z8Dwn4EIwDiqEAAmvwPxaR3sQwAAAABJRU5ErkJggg=="; + +function createCtx() { + const pasteText = vi.fn(); + const insertText = vi.fn(); + const requestRender = vi.fn(); + const showStatus = vi.fn(); + const pendingImages: ImageContent[] = []; + const pendingImageLinks: (string | undefined)[] = []; + const ctx = { + editor: { + pasteText, + insertText, + imageLinks: undefined, + pendingImages, + pendingImageLinks, + } as unknown as InteractiveModeContext["editor"], + ui: { requestRender, getFocused: () => null } as unknown as InteractiveModeContext["ui"], + sessionManager: { + getCwd: () => process.cwd(), + putBlob: async () => ({ hash: "h", path: "/tmp/h.png", displayPath: "/tmp/h.png" }), + } as unknown as InteractiveModeContext["sessionManager"], + showStatus, + } as unknown as InteractiveModeContext; + return { ctx, spies: { pasteText, insertText, requestRender, showStatus, pendingImages, pendingImageLinks } }; +} + +describe("InputController.handleImagePaste (issue #8769)", () => { + let tmpDir: string; + let imgPath: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "issue-8769-")); + imgPath = path.join(tmpDir, "screenshot.png"); + await fs.writeFile(imgPath, FILE_PNG); + resetSettingsForTest(); + await Settings.init({ inMemory: true, overrides: { "images.autoResize": false } }); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + resetSettingsForTest(); + vi.restoreAllMocks(); + }); + + it("attaches the file bytes, not the co-advertised Finder icon bitmap", async () => { + const { ctx, spies } = createCtx(); + const readImage = vi.fn(async () => ({ + // Finder's generated file-icon bitmap; arboard::get_image succeeds with this. + data: Buffer.from(ICON_PNG_BASE64, "base64"), + mimeType: "image/png", + })); + const readMacFileUrls = vi.fn(async () => [imgPath]); + const controller = new InputController(ctx, { + readImage: readImage as unknown as never, + readText: async () => "", + readMacFileUrls, + }); + + const result = await controller.handleImagePaste(); + + expect(result).toBe(true); + // The authoritative file URL MUST be consulted and win over the icon. + expect(readMacFileUrls).toHaveBeenCalled(); + expect(spies.pendingImages.length).toBe(1); + // The attached bytes must be the file's, NOT Finder's icon bitmap. + expect(spies.pendingImages[0]?.data).toBe(FILE_PNG.toString("base64")); + expect(spies.pendingImages[0]?.data).not.toBe(ICON_PNG_BASE64); + }); + + it("still attaches a pure bitmap pasteboard (screenshot/browser copy) with no file URL", async () => { + const { ctx, spies } = createCtx(); + const readMacFileUrls = vi.fn(async () => [] as string[]); + const controller = new InputController(ctx, { + readImage: async () => + ({ + data: Buffer.from(ICON_PNG_BASE64, "base64"), + mimeType: "image/png", + }) as unknown as never, + readText: async () => "", + readMacFileUrls, + }); + + const result = await controller.handleImagePaste(); + + expect(result).toBe(true); + expect(spies.pendingImages.length).toBe(1); + // No usable image-file URL → the bitmap representation is attached. + expect(spies.pendingImages[0]?.data).toBe(ICON_PNG_BASE64); + }); + + it("falls to the bitmap when the file URL is a non-image file", async () => { + const { ctx, spies } = createCtx(); + const readMacFileUrls = vi.fn(async () => ["/Users/me/Documents/report.pdf"]); + const controller = new InputController(ctx, { + readImage: async () => + ({ + data: Buffer.from(ICON_PNG_BASE64, "base64"), + mimeType: "image/png", + }) as unknown as never, + readText: async () => "", + readMacFileUrls, + }); + + const result = await controller.handleImagePaste(); + + expect(result).toBe(true); + expect(spies.pendingImages.length).toBe(1); + expect(spies.pendingImages[0]?.data).toBe(ICON_PNG_BASE64); + }); +});