diff --git a/apps/ade-cli/src/lib/clipboard.test.ts b/apps/ade-cli/src/lib/clipboard.test.ts new file mode 100644 index 000000000..dd50d92c4 --- /dev/null +++ b/apps/ade-cli/src/lib/clipboard.test.ts @@ -0,0 +1,56 @@ +import { spawnSync } from "node:child_process"; +import { describe, expect, it } from "vitest"; + +import { copyToClipboard, type CopyToClipboardSpawnOptions } from "./clipboard"; + +describe("copyToClipboard", () => { + it("discards the helper's stdout/stderr so a daemonizing wl-copy cannot hold our pipes", () => { + const seen: CopyToClipboardSpawnOptions[] = []; + const ok = copyToClipboard("ade://lane/x", { + platform: "linux", + commandExists: (cmd) => cmd === "wl-copy", + spawn: (cmd, args, opts) => { + expect(cmd).toBe("wl-copy"); + expect(args).toEqual([]); + seen.push(opts); + return { status: 0 }; + }, + }); + expect(ok).toBe(true); + expect(seen[0]?.stdio).toEqual(["pipe", "ignore", "ignore"]); + expect(seen[0]?.timeout).toBeGreaterThan(0); + expect(seen[0]?.input).toBe("ade://lane/x"); + }); + + it("returns the fallback result instead of hanging when the helper never exits", () => { + // The real Wayland failure: the helper keeps running after taking the text. + // Production hands spawnSync a bounded timeout, so the call must come back + // (as `false`, which callers surface as "here's the URL") rather than block. + const started = Date.now(); + const ok = copyToClipboard("ade://lane/x", { + platform: "linux", + commandExists: () => true, + timeoutMs: 300, + // Real child, real spawnSync, production's own options: reads stdin then + // lingers the way a clipboard daemon does. Spawned via `process.execPath` + // rather than `sh -c` so it runs on Windows too, and so the process + // spawnSync's timeout kills IS the lingering one (a shell would fork the + // sleeper and orphan it). + spawn: (_cmd, _args, opts) => + spawnSync(process.execPath, ["-e", "process.stdin.resume(); setTimeout(() => {}, 30_000);"], opts), + }); + expect(ok).toBe(false); + expect(Date.now() - started).toBeLessThan(10_000); + }); + + it("treats a throwing spawn as a copy failure", () => { + expect( + copyToClipboard("x", { + platform: "darwin", + spawn: () => { + throw new Error("EPERM"); + }, + }), + ).toBe(false); + }); +}); diff --git a/apps/ade-cli/src/lib/clipboard.ts b/apps/ade-cli/src/lib/clipboard.ts index 1a76f9de7..98a905826 100644 --- a/apps/ade-cli/src/lib/clipboard.ts +++ b/apps/ade-cli/src/lib/clipboard.ts @@ -3,17 +3,38 @@ // // Picks the right system clipboard binary for darwin (pbcopy), win32 (clip), // or Linux (wl-copy / xclip). Returns `false` when no usable binary is found -// instead of throwing — callers decide how to surface the failure. +// or when the helper does not finish in time, instead of throwing — callers +// decide how to surface the failure (they print the text/URL instead). +// +// Wayland caveat: `wl-copy` forks a daemon that owns the selection until the +// clipboard is overwritten. If that daemon inherits our stdout/stderr pipes, +// `spawnSync` waits for those pipes to close and blocks for as long as the +// selection lives — `ade report-issue --open` and the TUI `/report-issue` +// keybinds would hang forever. Discarding stdout/stderr (so the daemon holds +// no pipe of ours) plus a bounded timeout keeps the call finite on every +// platform. `xclip`, the one X11 fallback we try, daemonizes the same way. // --------------------------------------------------------------------------- import { spawnSync } from "node:child_process"; +/** Upper bound on how long a clipboard helper may run before we give up. */ +const CLIPBOARD_TIMEOUT_MS = 3_000; + +export type CopyToClipboardSpawnOptions = { + input: string; + windowsHide?: boolean; + /** stdin is piped so we can hand over the text; stdout/stderr are discarded. */ + stdio?: Array<"pipe" | "ignore">; + /** Bounded runtime; spawnSync kills the child and reports an error past it. */ + timeout?: number; +}; + export type CopyToClipboardOptions = { /** * Test seam: override the spawn function. The override must return the * same shape as `spawnSync` (status + error). Defaults to `spawnSync`. */ - spawn?: (cmd: string, args: string[], options: { input: string; windowsHide?: boolean }) => { + spawn?: (cmd: string, args: string[], options: CopyToClipboardSpawnOptions) => { error?: Error; status?: number | null; }; @@ -24,12 +45,15 @@ export type CopyToClipboardOptions = { commandExists?: (cmd: string) => boolean; /** Test seam: override `process.platform`. */ platform?: NodeJS.Platform; + /** Test seam: shorten the bounded timeout. */ + timeoutMs?: number; }; export function copyToClipboard(text: string, options: CopyToClipboardOptions = {}): boolean { const platform = options.platform ?? process.platform; const spawn = options.spawn ?? ((cmd, args, opts) => spawnSync(cmd, args, opts)); const commandExists = options.commandExists ?? defaultCommandExists; + const timeout = options.timeoutMs ?? CLIPBOARD_TIMEOUT_MS; let cmd: string; let args: string[]; @@ -50,7 +74,17 @@ export function copyToClipboard(text: string, options: CopyToClipboardOptions = return false; } } - const r = spawn(cmd, args, { input: text, windowsHide: true }); + let r: { error?: Error; status?: number | null }; + try { + r = spawn(cmd, args, { + input: text, + windowsHide: true, + stdio: ["pipe", "ignore", "ignore"], + timeout, + }); + } catch { + return false; + } if (r.error || (typeof r.status === "number" && r.status !== 0)) return false; return true; } @@ -59,6 +93,7 @@ function defaultCommandExists(cmd: string): boolean { const r = spawnSync(process.platform === "win32" ? "where" : "which", [cmd], { stdio: "ignore", windowsHide: true, + timeout: CLIPBOARD_TIMEOUT_MS, }); return !r.error && r.status === 0; } diff --git a/apps/ade-cli/src/tuiClient/__tests__/promptWrap.test.tsx b/apps/ade-cli/src/tuiClient/__tests__/promptWrap.test.tsx index 43cf3889f..fef313355 100644 --- a/apps/ade-cli/src/tuiClient/__tests__/promptWrap.test.tsx +++ b/apps/ade-cli/src/tuiClient/__tests__/promptWrap.test.tsx @@ -56,6 +56,11 @@ function PromptRows({ ); } +/** Strips SGR escapes (e.g. the inverse-video caret cell) for text compares. */ +function stripAnsi(text: string): string { + return text.replace(/\u001B\[[0-9;]*m/g, ""); +} + /** Body lines of the rendered box, with the border/padding stripped. */ function boxBodyLines(frame: string): string[] { return frame @@ -93,8 +98,14 @@ describe("prompt wrap budget", () => { ); const body = boxBodyLines(lastFrame() ?? ""); expect(body).toHaveLength(1); - // Every character survives; the old budget dropped the final one. - expect(body[0]!.trim()).toBe(`› ${text}`); + // The caret is an inverse-video cell rendered *after* the last character; + // PROMPT_ROW_CHROME_CELLS reserves a column for it, so the row still fits + // ONE terminal line. The old assertion used `.trim()`, which cannot strip a + // space wrapped in SGR escapes — so it passed only when chalk detected no + // color support and failed whenever Ink actually emitted escapes. That + // inconsistency was the flake. Strip the escapes so the assertion holds + // either way; asserting the raw SGR bytes would just invert the coupling. + expect(stripAnsi(body[0]!).trim()).toBe(`› ${text}`); }); it("reports when a trailing hint still fits beside a short row", () => { diff --git a/apps/desktop/src/main/services/analytics/productAnalyticsService.test.ts b/apps/desktop/src/main/services/analytics/productAnalyticsService.test.ts index 1debac419..01d276776 100644 --- a/apps/desktop/src/main/services/analytics/productAnalyticsService.test.ts +++ b/apps/desktop/src/main/services/analytics/productAnalyticsService.test.ts @@ -838,15 +838,17 @@ describe("productAnalyticsService", () => { fs.rmSync(harness.root, { recursive: true, force: true }); }); - it("cancels another process's queued client when the shared opt-out marker appears", async () => { + it("cancels another process's queued client on the next status read after the shared opt-out marker appears", async () => { const first = makeHarness(); expect(first.service.capture({ event: "ade_app_opened", surface: "desktop" }).accepted).toBe(true); const second = makeHarness({ root: first.root }); expect(second.service.setEnabled(false)).toMatchObject({ enabled: false, effective: false }); - await vi.waitFor(() => { - expect(first.shutdownArgs).toEqual([[1_500, { flush: false }]]); - }, { timeout: 3_000, interval: 20 }); + // Drive the reconcile explicitly instead of racing the cross-process fs + // watcher: getStatus() re-reads the shared opt-out marker synchronously via + // the same reconcileOptOutMarker() the watcher callback runs. + expect(first.service.getStatus()).toMatchObject({ enabled: false, effective: false }); + expect(first.shutdownArgs).toEqual([[1_500, { flush: false }]]); await expect(first.service.flush()).resolves.toBe(true); await first.service.shutdown(); @@ -854,20 +856,18 @@ describe("productAnalyticsService", () => { fs.rmSync(first.root, { recursive: true, force: true }); }); - it("honors another process's explicit opt-in after a shared opt-out", async () => { + it("honors another process's explicit opt-in after a shared opt-out on the next status read", async () => { const first = makeHarness(); expect(first.service.capture({ event: "ade_app_opened", surface: "desktop" }).accepted).toBe(true); const second = makeHarness({ root: first.root }); expect(second.service.setEnabled(false)).toMatchObject({ enabled: false, effective: false }); - await vi.waitFor(() => { - expect(first.shutdownArgs).toEqual([[1_500, { flush: false }]]); - }, { timeout: 3_000, interval: 20 }); + // Same synchronous reconcile as above rather than a real-timer watcher race. + expect(first.service.getStatus()).toMatchObject({ enabled: false, effective: false }); + expect(first.shutdownArgs).toEqual([[1_500, { flush: false }]]); expect(second.service.setEnabled(true)).toMatchObject({ enabled: true, effective: true }); - await vi.waitFor(() => { - expect(first.service.getStatus()).toMatchObject({ enabled: true, effective: true }); - }, { timeout: 3_000, interval: 20 }); + expect(first.service.getStatus()).toMatchObject({ enabled: true, effective: true }); expect(first.service.capture({ event: "ade_screen_viewed", surface: "desktop" })).toEqual({ accepted: true, reason: "accepted",