Merge PR #8770: fix(tui): prefer macOS file URL over Finder icon bitmap on image paste (@roboomp)
This commit is contained in:
can1357
@@ -72,6 +72,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed thinking-loop aborts (`AIError.Flag.ThinkingLoop`) walking `retry.fallbackChains` and switching to another model family on attempt 1, so a healthy planning turn on Grok 4.6 (SuperGrok / Cursor OAuth) no longer gets replaced by whatever the chain lists next. The loop guard now re-samples the same model with its `thinking-loop-redirect` notice, and no longer parks the model selector on a fallback cooldown. ([#8760](https://github.com/can1357/oh-my-pi/issues/8760))
|
||||
### 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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user