From 5bc796d4cb926eb1295fb81a0e4e974ebef4faa9 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 08:40:14 +0000 Subject: [PATCH] fix(clipboard): read the system clipboard via Bun.spawn instead of execSync readTextFromClipboard called execSync for pbpaste, termux-clipboard-get, wl-paste, and xclip; readMacFileUrlsFromClipboard did the same for osascript; copyToClipboard for termux-clipboard-set. execSync parks the event loop until the child exits or the 2000ms timeout fires, so a hung clipboard daemon froze the TUI render loop for the whole budget on every paste and copy chord (input-controller handleImagePaste and handleClipboardTextRawPaste). A new spawnCapture helper wraps Bun.spawn with the same 2000ms guard, stdout-to-string decoding, and non-zero-exit/timeout throw semantics the outer try/catch already assumed. Every synchronous clipboard shell-out now yields to the event loop while the child runs. Regression test under readTextFromClipboard runs a slow fake pbpaste and asserts a concurrent setInterval keeps ticking; the pre-fix code delivered zero ticks. Fixes #4235 --- packages/coding-agent/src/utils/clipboard.ts | 64 ++++++++++--- .../coding-agent/test/utils/clipboard.test.ts | 96 +++++++++++++++---- 2 files changed, 131 insertions(+), 29 deletions(-) diff --git a/packages/coding-agent/src/utils/clipboard.ts b/packages/coding-agent/src/utils/clipboard.ts index ecebc3545..74848f761 100644 --- a/packages/coding-agent/src/utils/clipboard.ts +++ b/packages/coding-agent/src/utils/clipboard.ts @@ -1,8 +1,52 @@ -import { execSync } from "node:child_process"; import type { ClipboardImage } from "@oh-my-pi/pi-natives"; import * as native from "@oh-my-pi/pi-natives"; import { logger } from "@oh-my-pi/pi-utils"; +/** + * Run a subprocess and capture its stdout without blocking the event loop. + * + * `readTextFromClipboard`, `readMacFileUrlsFromClipboard`, and the Termux copy + * path all shell out to CLI clipboard tools. The synchronous `execSync` API + * parks the render loop until the child exits or the timeout fires, so a hung + * clipboard daemon freezes the TUI for the full 2000ms budget (#4235). This + * helper mirrors the previous semantics — read stdout as UTF-8, throw on + * non-zero exit or timeout, forward optional stdin — but yields to the event + * loop while the child runs. + * + * @throws Error when the child fails to spawn, is killed by the timeout, or + * exits with a non-zero status. Callers rely on this to fall through to the + * outer catch and return an empty string / empty list. + */ +async function spawnCapture( + cmd: string[], + options: { input?: string; timeoutMs?: number } = {}, +): Promise { + const timeoutMs = options.timeoutMs ?? 2000; + const proc = Bun.spawn(cmd, { + stdout: "pipe", + stderr: "ignore", + stdin: options.input !== undefined ? Buffer.from(options.input) : "ignore", + }); + let timedOut = false; + const timer = setTimeout(() => { + timedOut = true; + proc.kill(); + }, timeoutMs); + try { + const stdout = await new Response(proc.stdout).text(); + await proc.exited; + if (timedOut) { + throw new Error(`${cmd[0]} timed out after ${timeoutMs}ms`); + } + if (proc.exitCode !== 0) { + throw new Error(`${cmd[0]} exited with code ${proc.exitCode}`); + } + return stdout; + } finally { + clearTimeout(timer); + } +} + function hasDisplay(): boolean { return process.platform !== "linux" || Boolean(process.env.DISPLAY || process.env.WAYLAND_DISPLAY); } @@ -53,11 +97,7 @@ const MAC_FILE_URL_SCRIPT = [ export async function readMacFileUrlsFromClipboard(): Promise { if (process.platform !== "darwin") return []; try { - const stdout = execSync("osascript -", { - input: MAC_FILE_URL_SCRIPT, - encoding: "utf8", - timeout: 2000, - }).toString(); + const stdout = await spawnCapture(["osascript", "-"], { input: MAC_FILE_URL_SCRIPT }); return stdout .split(/\r?\n/) .map(line => line.trim()) @@ -110,7 +150,7 @@ export async function copyToClipboard(text: string): Promise { try { if (process.env.TERMUX_VERSION) { try { - execSync("termux-clipboard-set", { input: text, timeout: 5000 }); + await spawnCapture(["termux-clipboard-set"], { input: text, timeoutMs: 5000 }); return; } catch { // Fall through to native @@ -286,13 +326,13 @@ export async function readTextFromClipboard(): Promise { try { const p = process.platform; if (p === "darwin") { - return execSync("pbpaste", { encoding: "utf8", timeout: 2000 }).toString(); + return await spawnCapture(["pbpaste"]); } if (p === "win32") { return (await readTextViaPowerShell()) ?? ""; } if (process.env.TERMUX_VERSION) { - return execSync("termux-clipboard-get", { encoding: "utf8", timeout: 2000 }).toString(); + return await spawnCapture(["termux-clipboard-get"]); } if (isWsl()) { const text = await readTextViaPowerShell(); @@ -303,14 +343,14 @@ export async function readTextFromClipboard(): Promise { const hasX11Display = Boolean(process.env.DISPLAY); if (hasWaylandDisplay) { try { - return execSync("wl-paste --type text/plain --no-newline", { encoding: "utf8", timeout: 2000 }).toString(); + return await spawnCapture(["wl-paste", "--type", "text/plain", "--no-newline"]); } catch { if (hasX11Display) { - return execSync("xclip -selection clipboard -o", { encoding: "utf8", timeout: 2000 }).toString(); + return await spawnCapture(["xclip", "-selection", "clipboard", "-o"]); } } } else if (hasX11Display) { - return execSync("xclip -selection clipboard -o", { encoding: "utf8", timeout: 2000 }).toString(); + return await spawnCapture(["xclip", "-selection", "clipboard", "-o"]); } } catch (error) { logger.warn("clipboard: failed to read clipboard text", { error: String(error) }); diff --git a/packages/coding-agent/test/utils/clipboard.test.ts b/packages/coding-agent/test/utils/clipboard.test.ts index 91a4d796a..5e7b5f53d 100644 --- a/packages/coding-agent/test/utils/clipboard.test.ts +++ b/packages/coding-agent/test/utils/clipboard.test.ts @@ -1,5 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; -import { readImageFromClipboard, readMacFileUrlsFromClipboard } from "@oh-my-pi/pi-coding-agent/utils/clipboard"; +import { + readImageFromClipboard, + readMacFileUrlsFromClipboard, + readTextFromClipboard, +} from "@oh-my-pi/pi-coding-agent/utils/clipboard"; import * as native from "@oh-my-pi/pi-natives"; import type { Subprocess } from "bun"; @@ -189,36 +193,94 @@ describe("readImageFromClipboard dispatch", () => { describe("readMacFileUrlsFromClipboard", () => { it("returns an empty list on non-darwin platforms without spawning osascript", async () => { setPlatform("linux"); - const cp = await import("node:child_process"); - const execSpy = vi.spyOn(cp, "execSync"); + const spawnSpy = vi.spyOn(Bun, "spawn"); expect(await readMacFileUrlsFromClipboard()).toEqual([]); - expect(execSpy).not.toHaveBeenCalled(); + expect(spawnSpy).not.toHaveBeenCalled(); }); it("splits osascript output into one path per non-empty line on darwin", async () => { setPlatform("darwin"); - const cp = await import("node:child_process"); - const execSpy = vi - .spyOn(cp, "execSync") - .mockReturnValue("/Users/me/Pictures/photo.png\n/Users/me/Pictures/clip.jpg\n\n"); + const calls: SpawnCall[] = []; + spyPowershell(calls, "/Users/me/Pictures/photo.png\n/Users/me/Pictures/clip.jpg\n\n"); const paths = await readMacFileUrlsFromClipboard(); expect(paths).toEqual(["/Users/me/Pictures/photo.png", "/Users/me/Pictures/clip.jpg"]); - expect(execSpy).toHaveBeenCalledTimes(1); - const [cmd, options] = execSpy.mock.calls[0] ?? []; - expect(cmd).toBe("osascript -"); - expect((options as { input?: string } | undefined)?.input).toContain("«class furl»"); + expect(calls).toHaveLength(1); + expect(calls[0]?.cmd).toEqual(["osascript", "-"]); + // AppleScript payload is piped as stdin; the fix uses Bun.spawn with a + // Buffer so the child receives it without blocking the event loop. + const stdin = calls[0]?.options.stdin; + expect(Buffer.isBuffer(stdin)).toBe(true); + expect((stdin as Buffer).toString("utf8")).toContain("«class furl»"); }); - it("returns an empty list when osascript throws (e.g. binary missing)", async () => { + it("returns an empty list when osascript exits non-zero (e.g. binary missing)", async () => { setPlatform("darwin"); - const cp = await import("node:child_process"); - vi.spyOn(cp, "execSync").mockImplementation(() => { - throw new Error("osascript: command not found"); - }); + spyPowershell([], "", 127); expect(await readMacFileUrlsFromClipboard()).toEqual([]); }); }); + +describe("readTextFromClipboard", () => { + it("returns pbpaste stdout on darwin without touching execSync", async () => { + setPlatform("darwin"); + const calls: SpawnCall[] = []; + spyPowershell(calls, "hello from pbpaste"); + + expect(await readTextFromClipboard()).toBe("hello from pbpaste"); + expect(calls).toHaveLength(1); + expect(calls[0]?.cmd).toEqual(["pbpaste"]); + }); + + it("returns an empty string when the subprocess exits non-zero", async () => { + setPlatform("darwin"); + spyPowershell([], "", 1); + + expect(await readTextFromClipboard()).toBe(""); + }); + + it("keeps the event loop responsive while the clipboard tool runs (#4235)", async () => { + setPlatform("darwin"); + + // Simulate a slow pbpaste: its stdout stream only emits after a real + // setTimeout, so the event loop must be free during the read. Under the + // pre-fix execSync path, this would spin the child synchronously and + // starve every setInterval tick. + const DELAY_MS = 80; + const slowProc = { + pid: 1, + stdout: new ReadableStream({ + async start(controller) { + await Bun.sleep(DELAY_MS); + controller.enqueue(new TextEncoder().encode("payload")); + controller.close(); + }, + }), + stderr: streamOf(""), + exitCode: 0, + exited: (async () => { + await Bun.sleep(DELAY_MS); + return 0; + })(), + kill: () => true, + } as unknown as Subprocess; + vi.spyOn(Bun, "spawn").mockReturnValue(slowProc); + + let ticks = 0; + const timer = setInterval(() => { + ticks += 1; + }, 10); + try { + const text = await readTextFromClipboard(); + expect(text).toBe("payload"); + } finally { + clearInterval(timer); + } + // If the read blocked the loop, ticks would stay at 0. A yielding + // implementation fires several ticks in the ~80ms window. + expect(ticks).toBeGreaterThanOrEqual(2); + }); +});