fix(computer-use): truthful macOS permission probe, working CGEvent input, and frontmost-guarded keystrokes - #5928
Conversation
…#5917) request_access claimed screenshot/accessibility were available on every host: the probe checked that screencapture and osascript exist, not that the Screen Recording and Accessibility grants are held. It now reads AXIsProcessTrusted() and the CGWindowList name-stripping TCC applies without the Screen Recording grant, reports each grant as granted / denied / unknown, lists what is missing, and names the outermost .app in the process tree (the app TCC holds responsible) in a how_to_fix line. Input scripts fail fast with that remedy after a denied probe instead of hanging on a prompt, and a screencapture 'could not create image from display' names the locked/asleep display instead of hinting at permissions. Separately, every CGEvent input on macOS was broken: cg() took (script, timeoutMs) but keyEvent, type, scroll, and postMouseEvent passed their payload as the second argument, so the script saw no P.code / P.x / P.text and the payload object coerced to a zero timeout — key, type, hold_key, scroll, and clicks all reported 'osascript timed out' instantly. cg() now takes (script, payload, timeoutMs). Tests (injected runner, any host): denied grants surface as missing capabilities with the host app named; a known-denied grant refuses input without spawning osascript; granted state reports no remedy; key/type deliver their payload and a real timeout. Live receipt on this Mac: request_access -> host_app Ghostty, both grants granted; key cmd+q -> ok (it reached the frontmost app, which is #5927). npm test (crates/tui/plugins/computer-use): 51 passed, 0 failed. Signed-off-by: CodeWhale Bot <hmbown@gmail.com>
…st app (#5927) open_application returned launched: true as soon as `open -F` exited, before the app was in front; a following key chord then went to whatever was frontmost — a cleanup cmd+q quit the terminal hosting the founder's session. It now waits up to 3 s for the process to exist and, when activation was asked for, to be frontmost, and returns frontmost: bool, pid, and a note naming the consequence when it could not bring the app forward. key, type, and hold_key accept an optional app_ref and refuse with a named reason when that app is not frontmost at send time; every receipt carries frontmost_app (name, pid, bundle_id) so the transcript says where the event went. Tool schemas document app_ref. Tests (injected runner): activation that never comes forward; guarded key refused with no CGEvent posted; unguarded key receipts the frontmost app; guarded type goes through when the app is in front. npm test (crates/tui/plugins/computer-use): 55 passed, 0 failed. Signed-off-by: CodeWhale Bot <hmbown@gmail.com>
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_8829a29d-3305-4b1e-9c30-dc5adc91a086) |
|
Claude finished @Hmbown's task in 5m 18s —— View job Review: fix(computer-use): truthful macOS permission probe, working CGEvent input, and frontmost-guarded keystrokes
Note: I could not execute SummaryThis is a solid, well-targeted fix for three real bugs ( Two things worth a look (posted as inline comments):
Minor, non-blocking notes:
Good choices worth calling out: switching probe from "did the binary exist" to actual |
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. |
There was a problem hiding this comment.
🟡 Changes recommended
The macOS probe/launch code still has a few confirmed contract/compatibility and bounded-wait issues (fail-closed semantics on unknown, screen_capture rename, host app detection, and open_application timing) that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens the macOS computer-use backend by making request_access report real TCC permission state, fixing CGEvent JXA payload delivery/timeouts so raw input works, and adding optional frontmost app guards + receipts for keystroke tools to reduce “sent input to the wrong app” incidents.
Changes:
- Replace the macOS permission probe with a real TCC-backed check that reports
granted/denied/unknown,missing, andhow_to_fix, and makes input fail fast after a denied probe. - Fix CGEvent input by changing the internal
cg()helper to accept(script, payload, timeoutMs)so callers actually pass keycodes/coords/text and keep real timeouts. - Add
app_refsupport fortype/key/hold_key, require the app to be frontmost when provided, and includefrontmost_appin receipts; improveopen_applicationactivation truthfulness.
File summaries
| File | Description |
|---|---|
| crates/tui/plugins/computer-use/tests/backends.test.mjs | Adds darwin-specific tests for TCC probe reporting, fail-fast input, CGEvent payload delivery, and frontmost guarding behavior. |
| crates/tui/plugins/computer-use/src/tools.mjs | Extends tool schemas/descriptions to accept optional app_ref for keystroke tools. |
| crates/tui/plugins/computer-use/src/backends/darwin.mjs | Implements the new TCC probe, fixes CGEvent payload plumbing, adds frontmost receipts/guards, improves open_application activation handling, and clarifies screencapture failure modes. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // `open -F` returns before the app is in front. Wait for the process to | ||
| // exist and, when activation was asked for, to actually be frontmost; | ||
| // otherwise the next keystroke lands in whatever app is (#5927). | ||
| let p = null; |
| let pid = process.ppid; | ||
| let found = null; | ||
| for (let depth = 0; depth < 12 && pid > 1; depth++) { | ||
| const r = await runL("ps", ["-o", "ppid=,comm=", "-p", String(pid)], { timeoutMs: 4_000 }); |
| async function probe() { | ||
| const caps = { screenshot: true, recording: true, accessibility_tree: true, clipboard: true, displays: true }; | ||
| const caps = { screenshot: true, recording: true, accessibility_tree: true, raw_input: true, clipboard: true, displays: true }; | ||
| const perms = {}; | ||
| try { await jxa(`function run(argv){ return JSON.stringify({n: Application('System Events').applicationProcesses.length}); }`, {}, 8_000); perms.accessibility = "granted"; } | ||
| catch (e) { perms.accessibility = "denied_or_unavailable"; caps.accessibility_tree = false; caps.raw_input = "unreliable"; } | ||
| try { | ||
| const t = os.tmpdir() + `/cu-probe-${crypto.randomBytes(3).toString("hex")}.png`; | ||
| const r = await runL("screencapture", ["-x", "-R0,0,2,2", "-t", "png", t], { timeoutMs: 8_000 }); | ||
| perms.screen_capture = r.code === 0 ? "ok" : "failed"; | ||
| try { fs.rmSync(t, { force: true }); } catch {} | ||
| } catch { perms.screen_capture = "failed"; } | ||
| const hasRecording = fs.existsSync("/usr/sbin/screencapture"); | ||
| return { platform: "darwin", capabilities: caps, permissions: perms, note: "macOS does not expose Screen-Recording TCC state to CLI; a black/empty screenshot means Screen Recording permission is missing. Raw pointer/keyboard events go to whatever is frontmost at the target point — activate the app first for click-type actions." }; | ||
| const missing = []; | ||
| let tcc = {}; |
| // Optional guard for keystroke tools: the app that must be frontmost. | ||
| const inputAppRef = { | ||
| type: "object", | ||
| description: "Refuse to send the keystrokes unless this app is frontmost (name, bundle_id, or pid).", | ||
| properties: { pid: { type: "integer" }, name: { type: "string" }, bundle_id: { type: "string" } }, | ||
| additionalProperties: false, | ||
| }; |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Refuse to post keystrokes when the app the caller named is not the one | ||
| // that would receive them. Returns the frontmost app for the receipt. | ||
| async function guardInput(appRef) { | ||
| const front = await frontmostApp().catch(() => null); |
There was a problem hiding this comment.
guardInput swallows any failure from frontmostApp() into front = null, not just the "no frontmost app" case:
const front = await frontmostApp().catch(() => null);If this JXA call fails for a reason unrelated to the actual frontmost state (a transient osascript timeout, an unrelated JXA error, etc.) while the subsequent findProcess(appRef) call on line 489 happens to succeed, the function falls through to:
if (!front || front.pid !== target.pid) {
throw new ExecError(`refusing to send input: "${target.name}" is not frontmost...`);
}front is null, so this unconditionally reports "${target.name}" is not frontmost — even though the real cause was an unrelated lookup failure, not the named app actually being backgrounded. That's a false negative: a legitimate, safe keystroke request gets refused with a misleading reason, which cuts against this PR's own goal of truthful status reporting (vs. the old request_access that just lied optimistically — this direction, pessimistically, but still not truthful about why).
Consider letting the frontmostApp() error propagate (or at least surface its message) instead of unconditionally collapsing it to "not frontmost".
| // Refuse to post keystrokes when the app the caller named is not the one | ||
| // that would receive them. Returns the frontmost app for the receipt. | ||
| async function guardInput(appRef) { | ||
| const front = await frontmostApp().catch(() => null); | ||
| if (!appRef) return front; | ||
| const target = await findProcess(appRef); | ||
| if (!target.found) throw new ExecError("application not found — call list_apps for exact names/pids"); | ||
| if (!front || front.pid !== target.pid) { | ||
| throw new ExecError(`refusing to send input: "${target.name}" is not frontmost${front ? ` ("${front.name}" is)` : ""}; bring it forward first with open_application { activate: true } or click into it`); | ||
| } | ||
| return front; | ||
| } |
There was a problem hiding this comment.
Nice fix for the timeout/payload bug and the truthful probe. One residual gap worth calling out explicitly (not necessarily a blocker): guardInput only refuses when the caller passes app_ref (tools.mjs lines 222/226/231 make it optional). A key/type/hold_key call made the way the #5927 incident happened — no app_ref — still goes straight through to whatever is frontmost, exactly as before this PR. That's fine as a backend capability (the guard can't force the caller to use it), but since the PR title frames this as fixing "frontmost-guarded keystrokes," it may be worth confirming the tool-calling policy/prompt actually always supplies app_ref for destructive-ish keys (e.g. cmd+q), otherwise the guard added here doesn't change the failure mode that caused the incident.
![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: 46552c6f5c
ℹ️ 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".
| name: "key", description: "Press a key or chord, e.g. 'return', 'cmd+c' (macOS), 'ctrl+c' (Linux/Windows). Repeat with `repeat`. Pass app_ref to refuse unless that app is frontmost; the receipt names frontmost_app either way.", | ||
| inputSchema: { type: "object", required: ["text"], properties: { text: { type: "string" }, repeat: { type: "integer", minimum: 1, maximum: 100 }, app_ref: inputAppRef, computer: computerParam }, additionalProperties: false }, |
There was a problem hiding this comment.
On Linux, Windows, and HarmonyOS—including those platforms reached over SSH—this schema accepts and forwards app_ref, but their type, key, and hold_key implementations destructure only the original arguments and silently send input to the current foreground application. A caller relying on the advertised refusal guard can therefore still send a destructive shortcut to the wrong app; implement the guard on every backend or reject/remove app_ref where it is unsupported.
AGENTS.md reference: AGENTS.md:L38-L40
Useful? React with 👍 / 👎.
| key: async ({ text, repeat = 1 }) => { | ||
| key: async ({ text, repeat = 1, app_ref }) => { | ||
| const { flags, code, key } = parseChord(text); | ||
| const frontmost_app = await guardInput(app_ref); |
There was a problem hiding this comment.
When repeat > 1, this checks the foreground app only once before the loop even though each key event is later posted globally through a separate osascript invocation. If the first shortcut causes the target to quit or lose focus—for example cmd+q—a subsequent repetition can land in the newly frontmost app despite the supplied guard; revalidate the target before each repetition or post the events specifically to the target process.
AGENTS.md reference: AGENTS.md:L28-L29
Useful? React with 👍 / 👎.
| if (typeof d.kCGWindowName === 'string' && d.kCGWindowName.length) named++; | ||
| } | ||
| if (others > 0) out.screen_recording = named > 0; |
There was a problem hiding this comment.
When at least one other layer-0 window exists but every such window has an absent or empty kCGWindowName, this sets screen_recording to false. Window names are optional even when Screen Recording access is granted, so an untitled-window-only desktop is reported as denied, disables screenshot and recording capabilities, and directs the user to change a grant they already hold. The absence of a name can prove neither state here; preserve null unless a named window positively proves access.
AGENTS.md reference: AGENTS.md:L28-L29
Useful? React with 👍 / 👎.
| function run(argv){ var P = JSON.parse(argv[0]); | ||
| ${script} | ||
| }`, {}, timeoutMs); | ||
| }`, payload, timeoutMs); |
There was a problem hiding this comment.
Forwarding the payload here makes left_click_drag execute for the first time, but that path supplies MOUSE.left.dragged, currently numeric event type 7. CoreGraphics defines 6 as kCGEventLeftMouseDragged and 7 as kCGEventRightMouseDragged, so a left-button drag now posts right-drag events between its left-button down and up and can fail or invoke the wrong interaction; correct the event constants before enabling this path.
Useful? React with 👍 / 👎.
| const r = await jxa(`${JXA_PRELUDE} | ||
| var se = Application('System Events'); | ||
| var list = se.applicationProcesses.whose({ frontmost: true })(); |
There was a problem hiding this comment.
On a Mac where the host has the reported Accessibility grant but has not also authorized Apple Events automation of System Events, this new lookup can prompt, time out, or fail even though raw CGEvent input is permitted. Guarded input then cannot succeed, while request_access reports no missing permission because it probes only AX trust and Screen Recording. Read the frontmost/running application through a non-Apple-Events API such as NSWorkspace, or explicitly probe and report the additional Automation boundary.
AGENTS.md reference: AGENTS.md:L28-L29
Useful? React with 👍 / 👎.
| perms.accessibility = tcc.accessibility === true ? "granted" : "unknown"; | ||
| } | ||
| if (tcc.screen_recording === false) { | ||
| perms.screen_recording = "denied"; |
There was a problem hiding this comment.
The probe now emits permissions.screen_recording, but the bundled live smoke check in scripts/smoke.mjs:97 still reads permissions.screen_capture and declares the row passing based only on top-level ok. Consequently the macOS smoke run prints capture=undefined and passes whether the new probe reports granted, denied, or unknown, so it cannot validate the central permission fix; update the consumer and assert the returned state, or preserve a compatible alias.
AGENTS.md reference: AGENTS.md:L120-L122
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 fixes several serious macOS computer-use issues: the permission probe now reports real TCC state and the responsible host app, CGEvent input scripts finally receive their payload with correct timeouts, keystroke tools support an app_ref guard with frontmost receipts, and screenshot errors distinguish locked/asleep displays. The new tests cover the injected darwin backend with host-agnostic mocks.
Findings
- [WARNING] open_application's 3s wait is not actually bounded (
crates/tui/plugins/computer-use/src/backends/darwin.mjs:519)
The polling loop sleeps 300ms per attempt but eachfindProcess(find)call runs to its own timeout (potentially 8s or more when a TCC prompt appears and Accessibility state is unknown). If the app is slow to appear or System Events hangs,open_applicationcan block far longer than the documented 3 seconds. Consider using a deadline orPromise.racearoundfindProcessso total wait is bounded. - [WARNING] app_ref schema allows an empty object (
crates/tui/plugins/computer-use/src/tools.mjs:12)
inputAppRefhas nominPropertiesorrequired, soapp_ref: {}is valid input and reaches the backend. The backend treats the empty object as truthy, callsfindProcess({}), and returns a confusing 'application not found' instead of rejecting the invalid schema. Requiring at least one app identifier prevents this. - [INFO] Screen Recording detection is heuristic and can be inconclusive (
crates/tui/plugins/computer-use/src/backends/darwin.mjs)
The probe classifies screen recording as granted only when it sees at least one other window with a non-emptykCGWindowName. If there are no other visible windows, or every visible window has an empty title, it reportsunknownor evendeniedeven thoughscreencapturemay work. This is a documented best-effort check, but callers should treat screenshot/recording capabilities as uncertain when no positive evidence is available.
Suggestions
-
crates/tui/plugins/computer-use/src/tools.mjs:12— AddminProperties: 1toinputAppRefso an empty object is rejected by schema validation before reaching the backend.type: "object", minProperties: 1, description: "Refuse to send the keystrokes unless this app is frontmost (name, bundle_id, or pid).", properties: { pid: { type: "integer" }, name: { type: "string" }, bundle_id: { type: "string" } }, additionalProperties: false,
Assessment
The PR fixes real and important macOS automation bugs, and the added mock-based tests are valuable. The remaining concerns are mostly edge cases around unbounded polling and schema validation; they should be tightened before relying on guarded keystrokes in production, but the changes are a clear improvement over the previous behavior.
Advisory review by Codewhale (codewhale review --pr 5928 --post, head 46552c6f5c2a1f41ce8ebfa0ef3368b1db6d8843). 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.
| let p = null; | ||
| for (let attempt = 0; attempt < 10; attempt++) { | ||
| await new Promise((res) => setTimeout(res, 300)); | ||
| p = await findProcess(find).catch(() => null); |
There was a problem hiding this comment.
[WARNING] open_application's 3s wait is not actually bounded
The polling loop sleeps 300ms per attempt but each findProcess(find) call runs to its own timeout (potentially 8s or more when a TCC prompt appears and Accessibility state is unknown). If the app is slow to appear or System Events hangs, open_application can block far longer than the documented 3 seconds. Consider using a deadline or Promise.race around findProcess so total wait is bounded.
| // Optional guard for keystroke tools: the app that must be frontmost. | ||
| const inputAppRef = { | ||
| type: "object", | ||
| description: "Refuse to send the keystrokes unless this app is frontmost (name, bundle_id, or pid).", |
There was a problem hiding this comment.
[WARNING] app_ref schema allows an empty object
inputAppRef has no minProperties or required, so app_ref: {} is valid input and reaches the backend. The backend treats the empty object as truthy, calls findProcess({}), and returns a confusing 'application not found' instead of rejecting the invalid schema. Requiring at least one app identifier prevents this.
| description: "Refuse to send the keystrokes unless this app is frontmost (name, bundle_id, or pid).", | ||
| properties: { pid: { type: "integer" }, name: { type: "string" }, bundle_id: { type: "string" } }, | ||
| additionalProperties: false, | ||
| }; |
There was a problem hiding this comment.
Add minProperties: 1 to inputAppRef so an empty object is rejected by schema validation before reaching the backend.
| description: "Refuse to send the keystrokes unless this app is frontmost (name, bundle_id, or pid).", | |
| properties: { pid: { type: "integer" }, name: { type: "string" }, bundle_id: { type: "string" } }, | |
| additionalProperties: false, | |
| }; | |
| type: "object", | |
| minProperties: 1, | |
| description: "Refuse to send the keystrokes unless this app is frontmost (name, bundle_id, or pid).", | |
| properties: { pid: { type: "integer" }, name: { type: "string" }, bundle_id: { type: "string" } }, | |
| additionalProperties: false, |
Closes #5917
Closes #5927
What was wrong (all found by driving the bundled MCP server live on macOS for the #5856 receipt)
request_accesslied. It checked thatscreencapture/osascriptexist, not that the Screen Recording and Accessibility grants are held, and always answeredscreenshot: true.cg()took(script, timeoutMs)butkeyEvent,type,scroll, andpostMouseEventpassed their payload as the second argument: the script saw noP.code/P.x/P.textand the payload object coerced to a zero timeout, sokey,type,hold_key,scroll, and clicks reportedosascript timed outinstantly.open_application { activate: true }reportedlaunched: truebefore the app was in front, andkey/typecarried no target and no receipt of where the event went. A cleanupcmd+qfrom the test client went to the frontmost app and quit the terminal hosting the founder's session (computer-use macOS: key/type go to whatever is frontmost, and open_application(activate) reports launched before the app is in front — a cleanup cmd+q quit the founder's terminal #5927).screencapturefailure ofcould not create image from display(locked or asleep display) was indistinguishable from a permission problem.Change
AXIsProcessTrusted()and the CGWindowList name-stripping TCC applies without the Screen Recording grant (the JXA bridge has noCGPreflightScreenCaptureAccess); reports each grant granted/denied/unknown, listsmissing, and names the outermost.appin the process tree inhow_to_fix— the app TCC holds responsible. Input scripts fail fast with that remedy after a denied probe instead of hanging on a prompt.cg(script, payload, timeoutMs).open_applicationwaits up to 3 s for the process and (when activating) for it to be frontmost; returnsfrontmost,pid, and a note when it could not bring the app forward.key/type/hold_keyacceptapp_ref, refuse when that app is not frontmost, and receiptfrontmost_app.Verified
npm testincrates/tui/plugins/computer-use: 55 passed, 0 failed (9 new tests through the injected runner, host-agnostic).request_access→host_app: Ghostty, both grants granted;screenshotproduces a 15 MB PNG;key cmd+q→ok: true(which is how computer-use macOS: key/type go to whatever is frontmost, and open_application(activate) reports launched before the app is in front — a cleanup cmd+q quit the founder's terminal #5927 was found).app_ref(they destructure named fields); their guard is a follow-up.Note
Medium Risk
Changes macOS raw keyboard/mouse automation and permission gating; misconfiguration could still send events to the frontmost app when
app_refis omitted, but guarded paths reduce accidental quit/key delivery (#5927).Overview
Fixes macOS computer-use reliability around TCC permissions, CGEvent input, and keystrokes hitting the wrong app.
The permission probe (
request_access→probe) now reads real Accessibility and Screen Recording state (viaAXIsProcessTrustedand window-name stripping), identifies the host.appin the process tree, and returnsmissing,how_to_fix, and accurate capability flags. After a denied Accessibility probe, input/a11y scripts fail fast with remediation instead of hanging on TCC prompts.CGEvent input is repaired by changing
cg()to(script, payload, timeoutMs)so keycodes, coordinates, and text reach JXA with proper timeouts (fixing instant falseosascript timed outfailures).Keyboard safety:
type,key, andhold_keyaccept optionalapp_ref(tool schema updated), refuse when that app is not frontmost, and always receiptfrontmost_app.open_applicationwithactivate: truepolls up to ~3s for the app to be frontmost and returnsfrontmost,pid, and a warning note when activation fails. Screenshot errors distinguish locked/asleep display from permission issues.Nine injected darwin backend tests cover probe, fail-fast input, CGEvent payloads, and frontmost guarding.
Reviewed by Cursor Bugbot for commit 46552c6. Bugbot is set up for automated code reviews on this repo. Configure here.