fix(computer-use): win32 backend reports PowerShell failures truthfully - #5903
Conversation
Every win32 action now runs self-contained in its own PowerShell process: the User32 Add-Type definition travels with each invocation instead of a bootstrap process whose type registration could never reach the fresh powershell.exe spawned per action (issue #5896 defect 1). withUser32()/psJson() now go through psOk(), which rejects nonzero exits, timeouts, and spawn failures, so input methods can no longer return action_sent:true after a failed subprocess (defect 2). left_mouse_down's ternary no longer swallows the LEFTDOWN press when a target is supplied - a targeted press both moves and presses (defect 3). list_windows now parses its JSON via psJson instead of mapping the raw run result, and write_clipboard fails truthfully on error. Tests: fake powershell.exe on PATH captures decoded -EncodedCommand payloads; covers exit!=0 -> error, spawn failure -> error, success -> success, and targeted mouse-down emitting SetCursorPos + LEFTDOWN in one self-contained command. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e18b73ce-e4f5-411a-84af-5fb28c6487d9) |
|
Claude finished @Hmbown's task in 2m 29s —— View job Review complete
SummaryThis correctly fixes all three defects named in #5896, and the fix logic checks out against the code, not just the PR description. 1. Bootstrap-process race (fixed correctly). Every 2. Silent failures (fixed correctly). 3. Minor — reuse/simplification (non-blocking):
Tests: the 6 new tests in Provider/model route safety: not applicable — this diff is confined to the Windows computer-use backend and its input plumbing, no model/provider selection logic touched. No security concerns: scripts still travel exclusively via |
There was a problem hiding this comment.
🟢 Approval recommended
The core behavioral fixes match the issue’s defects and are backed by focused tests, with only a minor naming clarity nit noted.
Pull request overview
This PR fixes correctness in the computer-use Win32 backend so PowerShell failures no longer surface as { action_sent: true }, and User32-backed input works reliably by making each PowerShell invocation self-contained.
Changes:
- Replace the one-shot “bootstrap” PowerShell type registration with a per-action
USER32_PRELUDEso every spawnedpowershell.exehas the neededUser32type. - Add
psOk()(truthful PowerShell execution) and route key call sites (withUser32,write_clipboard,cursor_position,list_windows) through it /psJson(). - Add a Node-only test suite that fakes
powershell.exeto assert self-containment, error propagation, and the targetedleft_mouse_downbehavior.
File summaries
| File | Description |
|---|---|
| crates/tui/plugins/computer-use/src/backends/win32.mjs | Makes PowerShell/User32 invocations self-contained and fail truthfully; fixes targeted left_mouse_down press logic. |
| crates/tui/plugins/computer-use/tests/win32-input.test.mjs | Adds regression tests using a fake powershell.exe to verify error semantics and per-action self-containment. |
Review details
Suppressed comments (1)
crates/tui/plugins/computer-use/src/backends/win32.mjs:100
- In probe(), the boolean
psOkvariable now shares a name with the newpsOk()helper, which makes the code harder to read and can lead to confusion during future edits. Renaming the boolean to something likepowershellOkkeeps intent clear.
const psOk = await ps("Write-Output 'ok'").then((r) => r.code === 0).catch(() => false);
return {
platform: "win32",
powershell: psOk,
capabilities: { screenshot: psOk, accessibility_tree: psOk, clipboard: psOk, recording: ffmpeg, raw_input: psOk },
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebb706fc29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const mjs = path.join(dir, "ps-fake.mjs"); | ||
| fs.writeFileSync(mjs, FAKE_PS_MJS); | ||
| const bin = path.join(dir, "powershell.exe"); | ||
| fs.writeFileSync(bin, `#!/bin/sh\nexec node ${JSON.stringify(mjs)} "$@"\n`); |
There was a problem hiding this comment.
On native Windows—the platform this backend targets—child_process.spawn("powershell.exe", ...) requires a Windows executable, but this fixture writes a POSIX /bin/sh script and chmod cannot make it executable there. Consequently the five onPath: true tests fail with a spawn/invalid-executable error rather than exercising the backend; the repository's computer-use plugin CI job runs only on Ubuntu, so this escapes CI but breaks npm test for Windows contributors. Use an injectable process runner or create a platform-appropriate launcher.
Useful? React with 👍 / 👎.
| async function psOk(script, opts = {}) { | ||
| const r = await ps(script, opts); | ||
| if (r.timedOut) throw new ExecError(`powershell timed out after ${opts.timeoutMs ?? 25_000}ms`, r); | ||
| if (r.code !== 0) throw new ExecError(`powershell.exe exited ${r.code}: ${(r.stderr || r.stdout).trim().slice(0, 300)}`, r); |
There was a problem hiding this comment.
When Add-Type fails—for example because constrained-language policy rejects dynamic compilation—the new prelude suppresses that failure with -ErrorAction SilentlyContinue; subsequent missing-User32 errors can continue to each action's unconditional Write-Output '{"ok": true}'. PowerShell's documented Continue behavior “displays the error message and continues executing”, so the process can exit 0 and this check still returns action_sent: true although no input was sent. Set $ErrorActionPreference = 'Stop', stop suppressing prelude failures, or explicitly validate the operation before emitting success.
Useful? React with 👍 / 👎.
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
The PR correctly makes Windows User32-backed actions self-contained and truthful by adding psOk/psJson error propagation, removing the ineffective bootstrap process, and fixing left_mouse_down target press logic. Tests cover key paths with a fake powershell.exe.
Findings
- [INFO] list_windows and write_clipboard error paths are not covered by the new tests (
crates/tui/plugins/computer-use/tests/win32-input.test.mjs)
The PR description states list_windows now parses via psJson and write_clipboard now uses psOk, but the new win32-input.test.mjs only checks nonzero exit for left_mouse_down, mouse_move, and key, and never exercises list_windows or write_clipboard. A regression in either path would go unnoticed. - [WARNING] Fake powershell.exe may truncate stdout/stderr by calling process.exit immediately (
crates/tui/plugins/computer-use/tests/win32-input.test.mjs:27)
In the fake PowerShell helper, process.stdout.write and process.stderr.write are asynchronous when writing to a pipe, but process.exit is called immediately after. This can truncate captured output and make tests flaky. Setting process.exitCode allows Node to flush pending stdio before exiting.
Suggestions
-
crates/tui/plugins/computer-use/tests/win32-input.test.mjs:27— Use process.exitCode instead of process.exit so pending stdout/stderr writes flush before the fake process exits, preventing flaky test output truncation.process.exitCode = Number(process.env.CU_FAKE_PS_EXIT || 0);
Assessment
The PR is correct and materially improves error truthfulness and Windows input reliability. The new tests are well-designed, though coverage could be extended to list_windows and write_clipboard. The suggested test reliability fix is minor and high-confidence.
Advisory review by Codewhale (codewhale review --pr 5903 --post, head ebb706fc2947a3391ffc62294c83663cdfb2ead4). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| } | ||
| if (process.env.CU_FAKE_PS_STDOUT) process.stdout.write(process.env.CU_FAKE_PS_STDOUT); | ||
| if (process.env.CU_FAKE_PS_STDERR) process.stderr.write(process.env.CU_FAKE_PS_STDERR); | ||
| process.exit(Number(process.env.CU_FAKE_PS_EXIT || 0)); |
There was a problem hiding this comment.
[WARNING] Fake powershell.exe may truncate stdout/stderr by calling process.exit immediately
In the fake PowerShell helper, process.stdout.write and process.stderr.write are asynchronous when writing to a pipe, but process.exit is called immediately after. This can truncate captured output and make tests flaky. Setting process.exitCode allows Node to flush pending stdio before exiting.
| } | ||
| if (process.env.CU_FAKE_PS_STDOUT) process.stdout.write(process.env.CU_FAKE_PS_STDOUT); | ||
| if (process.env.CU_FAKE_PS_STDERR) process.stderr.write(process.env.CU_FAKE_PS_STDERR); | ||
| process.exit(Number(process.env.CU_FAKE_PS_EXIT || 0)); |
There was a problem hiding this comment.
Use process.exitCode instead of process.exit so pending stdout/stderr writes flush before the fake process exits, preventing flaky test output truncation.
| process.exit(Number(process.env.CU_FAKE_PS_EXIT || 0)); | |
| process.exitCode = Number(process.env.CU_FAKE_PS_EXIT || 0); |
… exec }) Follow-up to the closed Hmbown#5910, per the maintainer's invitation after the bug itself was fixed and merged in Hmbown#5903. This PR is refactor-only: it keeps main's merged behavior and adds a cleaner test seam. Previously the win32 backend's PowerShell runner (ps/psOk/psJson) lived at module scope, so backend tests had to fake powershell.exe on PATH (or run on Windows). Move the runner helpers into create() as instance-local functions that default to the production runner but can be overridden with create({ exec: { run } }) — the same injection pattern the harmonyos backend already uses. tests/win32.test.mjs now exercises the win32 backend on any host through the injected runner: actions run without powershell, nonzero exits reject (fail-truthful), and targeted mouse-down is verified to move + press with the User32 type defined in-process. Verified on macOS with no Windows involved: node --test tests/win32.test.mjs (4/4) and tests/backends.test.mjs (10/10, no regression). Refs Hmbown#5903, Hmbown#5910. Signed-off-by: EvanProgramming <evangonggyf@gmail.com>
Closes #5896.
The verified issue named three defects in the Windows backend:
create()registered the User32 C# type in a one-shot bootstrap PowerShell process, but every action spawns a freshpowershell.exe— so actions ran without the type definition they depended on.withUser32()returned rawps()results, soleft_mouse_down,mouse_move,scroll,key,clickAt,write_clipboardand friends returned{action_sent:true}even on exit≠0 or spawn failure (reproduced with a fakepowershell.exeexiting 1).left_mouse_down, ternary/concatenation precedence put themouse_event(LEFTDOWN)expression in the no-target branch — a targeted mouse-down moved the cursor and never pressed.Fix: every User32-backed invocation is now self-contained (full
Add-Typeprelude per action);psOk/psJsonthrowExecErroron timeout, nonzero exit, and spawn failure;left_mouse_downnow always emits the press after the optional move;list_windowsparses viapsJson; bootstrap process removed.Tests (
tests/win32-input.test.mjs, 6 new): a fakepowershell.exeon PATH decodes and captures each-EncodedCommand— exit≠0 → error; spawn failure → error; success → success; targeted down = SetCursorPos + LEFTDOWN in one self-contained command; per-action self-containment for all 10 User32 actions;cursor_positionJSON parsing.Evidence:
node --test tests/win32-input.test.mjs→ 6/6 pass, exit 0. Full plugin suite 42/43 on macOS with the single failure reproduced identically at baseline (pre-existing ssh-probe test that requires AppleEvents and assertsprocess.platform === 'linux'in its error branch — unrelated to this change, not weakened). Native Windows desktop acceptance remains a separate required receipt per the issue.Note
Medium Risk
Changes Windows desktop automation input and error semantics; failures now surface as errors instead of silent success, which is correct but may affect callers that ignored errors.
Overview
Fixes the Windows computer-use backend so mouse/keyboard actions no longer claim success when PowerShell fails, and so User32 input actually runs in each spawned process.
Self-contained PowerShell invocations: The one-shot bootstrap that registered
User32in a separatepowershell.exeis removed. Every User32-backed action (andcursor_position) now prefixes scripts with an inlineAdd-Typeprelude (USER32_PRELUDE), because each action still spawns a fresh process.Truthful errors: New
psOkthrowsExecErroron timeout, nonzero exit, and spawn failure;psJsonbuilds on that.withUser32,write_clipboard, andlist_windowsuse these paths so callers get errors instead of{ action_sent: true }when PowerShell fails.left_mouse_downbug: Optional cursor move andLEFTDOWNare composed explicitly so a targeted press both moves and presses (fixes ternary/concatenation precedence).Tests: Adds
win32-input.test.mjswith a fakepowershell.exeon PATH to assert encoded scripts, per-action self-containment, error propagation, and the targeted mouse-down behavior—without a Windows GUI.Reviewed by Cursor Bugbot for commit ebb706f. Bugbot is set up for automated code reviews on this repo. Configure here.