diff --git a/crates/tui/plugins/computer-use/src/backends/win32.mjs b/crates/tui/plugins/computer-use/src/backends/win32.mjs index 40c8c07202..910c3bd5c9 100644 --- a/crates/tui/plugins/computer-use/src/backends/win32.mjs +++ b/crates/tui/plugins/computer-use/src/backends/win32.mjs @@ -10,21 +10,9 @@ import crypto from "node:crypto"; import { spawn } from "node:child_process"; import { run, runOk, ExecError, tryJson } from "../exec.mjs"; -function ps(script, opts = {}) { - const encoded = Buffer.from(script, "utf16le").toString("base64"); - return run("powershell.exe", ["-NoProfile", "-NonInteractive", "-EncodedCommand", encoded], { - timeoutMs: opts.timeoutMs ?? 25_000, - maxBuffer: 32 * 1024 * 1024, - }); -} - -async function psJson(script, opts = {}) { - const r = await ps(script, opts); - const out = r.stdout.trim(); - const j = tryJson(out, null); - if (!j) throw new ExecError(`powershell did not return JSON: ${(r.stderr || out).trim().slice(0, 300)}`, r); - return j; -} +// One-shot PowerShell runner is defined per `create()` instance below so a +// test can inject a fake runner; the shared bootstrap type and every action +// script are assembled against that instance-local runner. const USER32 = ` using System; @@ -51,6 +39,14 @@ public static class User32 { } }`; +// Prepend this to every User32-backed script. Each `ps()` spawns a FRESH +// powershell.exe process, so the type must be (re)defined in-process — the +// bootstrap process's Add-Type does NOT carry over. Defining it inline is what +// makes every action self-contained (see issue #5896). +const USER32_DEF = `Add-Type -TypeDefinition @' +${USER32} +'@ -ErrorAction SilentlyContinue; [User32] | Out-Null;`; + function recordingsDir() { return process.env.CODEWHALE_CU_RECORDINGS_DIR || path.join(os.homedir(), ".codewhale-cu", "recordings"); } @@ -65,16 +61,44 @@ const VK = { }; const MODVK = { ctrl: 0x11, control: 0x11, alt: 0x12, shift: 0x10, win: 0x5b, meta: 0x5b, cmd: 0x5b }; -export function create() { +export function create(opts = {}) { + // Allow tests (and other embedders) to inject a runner so no real + // powershell.exe is spawned. Defaults to the production runner. + const injectedRun = opts.exec && typeof opts.exec.run === "function" ? opts.exec.run : null; + const runner = injectedRun ?? run; + + async function ps(script, o = {}) { + const encoded = Buffer.from(script, "utf16le").toString("base64"); + return runner("powershell.exe", ["-NoProfile", "-NonInteractive", "-EncodedCommand", encoded], { + timeoutMs: o.timeoutMs ?? 25_000, + maxBuffer: 32 * 1024 * 1024, + }); + } + + async function psJson(script, o = {}) { + const r = await ps(script, o); + const out = r.stdout.trim(); + const j = tryJson(out, null); + if (!j) throw new ExecError(`powershell did not return JSON: ${(r.stderr || out).trim().slice(0, 300)}`, r); + return j; + } + const bootstrapped = (async () => { await ps(`Add-Type -TypeDefinition @'\n${USER32}\n'@ -ErrorAction SilentlyContinue; [User32] | Out-Null`, { timeoutMs: 30_000 }); })(); let lastRaster = null; let recording = null; // {id, pid, file, startedAt, mode} - async function withUser32(script, opts) { + // Every User32-backed action runs in its own powershell.exe, so the type must + // be defined in that process (USER32_DEF). A nonzero exit means the input was + // NOT delivered — fail truthfully instead of reporting success (#5896). + async function withUser32(script, o) { await bootstrapped; - return ps(script, opts); + const r = await ps(`${USER32_DEF}\n${script}`, o); + if (r.code !== 0) { + throw new ExecError(`win32 input action failed: ${(r.stderr || r.stdout || "").trim().slice(0, 300)}`, r); + } + return r; } return { @@ -256,7 +280,10 @@ Write-Output '{"ok": true}';`, { timeoutMs: 20_000 }); return { action_sent: true, from, to }; }, left_mouse_down: async ({ target }) => { - await withUser32(target ? `[User32]::SetCursorPos(${Math.round(target.x)}, ${Math.round(target.y)}) | Out-Null;` : "" + `[User32]::mouse_event([User32]::LEFTDOWN, 0, 0, 0, [UIntPtr]::Zero); Write-Output '{"ok": true}'`); + const script = target + ? `[User32]::SetCursorPos(${Math.round(target.x)}, ${Math.round(target.y)}) | Out-Null; [User32]::mouse_event([User32]::LEFTDOWN, 0, 0, 0, [UIntPtr]::Zero); Write-Output '{"ok": true}'` + : `[User32]::mouse_event([User32]::LEFTDOWN, 0, 0, 0, [UIntPtr]::Zero); Write-Output '{"ok": true}'`; + await withUser32(script); return { action_sent: true }; }, left_mouse_up: async () => { @@ -408,9 +435,11 @@ Write-Output '{"ok": true}';`, { timeoutMs: 10_000 }); }, cursor_position: async () => { await bootstrapped; - const j = await psJson(`$p = New-Object User32+POINT; + const r = await withUser32(`$p = New-Object User32+POINT; [void][User32]::GetCursorPos([ref]$p); Write-Output ('{"x": ' + $p.X + ', "y": ' + $p.Y + '}');`); + const j = tryJson(r.stdout.trim(), null); + if (!j) throw new ExecError("win32 cursor_position did not return JSON", r); return { x: j.x, y: j.y }; }, recordingStart: async ({ fps = 15, region } = {}) => { diff --git a/crates/tui/plugins/computer-use/tests/win32.test.mjs b/crates/tui/plugins/computer-use/tests/win32.test.mjs new file mode 100644 index 0000000000..ff44e36a02 --- /dev/null +++ b/crates/tui/plugins/computer-use/tests/win32.test.mjs @@ -0,0 +1,68 @@ +// Regression tests for the Windows backend (issue #5896). +// +// Before the fix: +// 1. Every User32-backed action spawned a fresh powershell.exe that never had +// the `User32` P/Invoke type defined, so the call silently failed. +// 2. `withUser32` returned the subprocess result without checking the exit +// code, so a failed action still reported `{ action_sent: true }`. +// 3. `left_mouse_down({ target })` only moved the cursor and dropped the +// button press due to a ternary/concatenation precedence bug. +// +// These tests inject a fake runner so no real powershell.exe is spawned and the +// generated PowerShell scripts can be asserted directly. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +function decodeScript(args) { + const i = args.indexOf("-EncodedCommand"); + if (i === -1) return null; + return Buffer.from(args[i + 1], "base64").toString("utf16le"); +} + +function mockExec({ fail = false } = {}) { + const calls = []; + const run = async (_cmd, args) => { + calls.push({ script: decodeScript(args) }); + if (fail) return { code: 1, stdout: "", stderr: "simulated powershell failure" }; + return { code: 0, stdout: "", stderr: "" }; + }; + return { run, calls }; +} + +test("win32: User32 actions define the type in their own process", async () => { + const { run, calls } = mockExec(); + const mod = await import("../src/backends/win32.mjs"); + const b = mod.create({ exec: { run } }); + await b.left_click({ target: { x: 5, y: 6 } }); + assert.ok(calls.at(-1).script.includes("public static class User32"), "User32 type must be defined in the action process"); +}); + +test("win32: input actions fail truthfully on a nonzero exit (no false success)", async () => { + const { run } = mockExec({ fail: true }); + const mod = await import("../src/backends/win32.mjs"); + const b = mod.create({ exec: { run } }); + await assert.rejects(() => b.left_click({ target: { x: 1, y: 2 } }), /failed/); + await assert.rejects(() => b.left_mouse_down({ target: { x: 1, y: 2 } }), /failed/); + await assert.rejects(() => b.scroll({ target: { x: 1, y: 2 } }), /failed/); + await assert.rejects(() => b.key({ text: "a" }), /failed/); +}); + +test("win32: left_mouse_down with a target both moves and presses", async () => { + const { run, calls } = mockExec(); + const mod = await import("../src/backends/win32.mjs"); + const b = mod.create({ exec: { run } }); + await b.left_mouse_down({ target: { x: 12, y: 34 } }); + const script = calls.at(-1).script; + assert.ok(script.includes("SetCursorPos(12, 34)"), "must move the cursor to the target"); + assert.ok(script.includes("LEFTDOWN"), "must press the left button (precedence fix)"); + assert.ok(script.includes("SetCursorPos") && script.includes("mouse_event"), "both move and press must be present"); +}); + +test("win32: cursor_position loads the User32 type in-process and never lies", async () => { + const { run, calls } = mockExec(); + const mod = await import("../src/backends/win32.mjs"); + const b = mod.create({ exec: { run } }); + // Empty stdout -> no JSON -> must throw, not report a position. + await assert.rejects(() => b.cursor_position()); + assert.ok(calls.at(-1).script.includes("public static class User32"), "cursor_position must define User32 in-process"); +});