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
This commit is contained in:
@@ -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<string> {
|
||||
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<string[]> {
|
||||
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<void> {
|
||||
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<string> {
|
||||
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<string> {
|
||||
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) });
|
||||
|
||||
@@ -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<Uint8Array>({
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user