Skip to content

cua-driver: NSMenu key equivalents on backgrounded apps, overlay z-order, focus fixes - #1437

Merged
ddupont808 merged 6 commits into
mainfrom
cua-driver/overlay-z-order-and-focus-fixes
May 4, 2026
Merged

cua-driver: NSMenu key equivalents on backgrounded apps, overlay z-order, focus fixes#1437
ddupont808 merged 6 commits into
mainfrom
cua-driver/overlay-z-order-and-focus-fixes

Conversation

@ddupont808

@ddupont808 ddupont808 commented May 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • NSMenu shortcut fix: hotkey/press_key with window_id now fires NSMenu key equivalents (Cmd+N, Cmd+S, Cmd+W, …) on backgrounded native AppKit apps. Previously, SkyLight's auth-message envelope routed events onto a direct-mach path that bypassed IOHIDPostEvent, so NSMenu never saw them. Fix: call FocusWithoutRaise.activateForMenuShortcut (SLPSSetFrontProcessWithOptions with kCPSNoWindows = 0x400) then post without the auth envelope — events route through IOHIDPostEvent and NSApplication.sendEvent dispatches NSMenu key equivalents normally.
  • macOS 26 compat: Replaces the deprecated GetProcessForPID Carbon API (removed in macOS 26 / Darwin 25.x) with SLSGetWindowOwner + SLSGetConnectionPSN for window-ID → PSN lookup.
  • Overlay z-order: Agent cursor overlay stays above target windows correctly.
  • Background automation focus: activateWithoutRaise now uses window-ID for PSN lookup (compatible with macOS 26).
  • New tools: drag, page, set_agent_cursor_style; browser JS execution via CDP/WebKit/WebInspector.
  • Tests: 9 Hermes form-fill tests, overlay z-order test, background menu shortcut test (verifies 0 focus losses without window_id, 1 focus loss + new document with window_id).

Test plan

  • scripts/test.sh test_background_menu_shortcut — both cases pass (Cmd+Z no-window_id → 0 focus losses; Cmd+N with window_id → 1 focus loss + TextEdit window count +1)
  • scripts/test.sh test_overlay_z_order — overlay stays on top
  • scripts/test.sh test_hermes_form_fill — all 9 Hermes tests pass
  • Build succeeds clean on macOS 26 (Darwin 25.x)

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Added window-id-based focus activation that avoids raising windows or triggering Space follow behavior
    • Added optional window_id parameter to hotkey and key press tools for menu shortcut support
  • Improvements

    • Enhanced keyboard input with configurable authorization message handling
    • Improved focus control when activating backgrounded applications
  • Tests

    • Added integration tests for background menu shortcut behavior on macOS

cua and others added 3 commits April 24, 2026 13:10
…d hermes form-fill tests

## Overlay z-ordering (NSWindowLevel.normal)
Changed the agent-cursor overlay from NSWindowLevel.floating (above all
normal windows) to NSWindowLevel.normal so it is sandwiched just above
the target window: [target, overlay, fg-windows]. Foreground windows
above the target now correctly occlude the cursor, making it visually
clear which window the agent is interacting with.

## Continuous repin loop (30 fps)
Replaced the sparse one-shot defensive-repin schedule [60, 180, 360,
600, 900, 1200ms] with a continuous ~30 fps loop. macOS sometimes
raises a background window's z-level asynchronously after an AX action;
the old schedule left a gap of up to 300ms where the overlay sat below
the target. At 33ms the overlay snaps back within one frame.

## Background Safari form-fill fixes
- ClickTool: 800ms post-AXPress delay for AXTextField/AXTextArea so
  WebKit establishes DOM focus before type_text_chars fires (email/text
  field race condition).
- ClickTool: detect AXPopUpButton clicks, list available options and
  hint to use set_value instead of click (native popup closes on bg).
- SetValueTool: two-strategy popup selection — AX child AXPress (native
  AppKit) with JavaScript injection fallback for Safari/WebKit selects
  that expose no AX children when the popup is closed.
- AXInput: added screenBoundingRect(), children(), stringAttribute()
  helpers used by ClickTool and SetValueTool.

## Agent cursor focus rect
AgentCursorRenderer/View/Cursor: draw a cyan glowing rounded rect around
the targeted AX element's screen bbox after each click so the user sees
which element the agent targeted.

## FocusMonitorApp enhancements
Added TrackingTextField (NSTextField subclass) that holds keyboard focus
throughout the test run and tracks field-level resignFirstResponder
events. Three-level focus-loss counters (app, window key, text field)
written to /tmp/focus_monitor_{losses,key_losses,field_losses}.txt.

## Structured content for list_windows / launch_app
Both tools now return structuredContent alongside their human-readable
text so integration tests can access window/app data without parsing.

## New integration tests
- test_hermes_form_fill.py: 9 tests driving individual HTML input types
  (text, password, email, number, textarea, checkbox, select, radio,
  submit) in a backgrounded Safari window via the hermes CLI, asserting
  no focus is stolen from FocusMonitorApp.
- test_overlay_z_order.py: asserts overlay appears at layer 0 (not
  floating/layer 3) and is sandwiched between the target and any
  foreground windows above it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- AgentCursor: call clearFocusRect() in hide() so the focus rect does
  not linger after the cursor auto-hides (the helper had a docstring
  explicitly saying it should be called there but was never wired up)

- SetValueTool: fix value escaping in JS/AppleScript injection path.
  Previously only single-quotes were escaped, leaving double-quotes and
  backslashes able to break the outer AppleScript string or inject
  arbitrary commands. Switch to percent-encoding with unreserved chars
  only and decode via decodeURIComponent() in the JS, eliminating the
  double-escape problem entirely. Also add a 10-second polling timeout
  around proc.waitUntilExit() so a stuck Safari/osascript does not block
  the MCP tool handler indefinitely.

- FocusMonitorApp: update doc-comment from "two kinds" to "three kinds"
  of focus loss to match the implementation (app-level, window key
  status, and text-field first-responder, the third being the newest
  addition tracked via TrackingTextField).

- test_hermes_form_fill: change module-level os.environ["ANTHROPIC_API_KEY"]
  to os.environ.get(..., "") so test discovery / lint / collect-only
  passes do not raise KeyError. setUpClass now skips the suite cleanly
  when the key is absent.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t SPIs

Fix hotkey/press_key silently swallowing NSMenu key equivalents (Cmd+N,
Cmd+S, Cmd+W, …) when the target app is backgrounded.

Root cause: SLEventPostToPid with the SkyLight auth-message envelope forks
onto a direct-mach delivery path that bypasses IOHIDPostEvent. NSMenu key
equivalents are dispatched in NSApplication.sendEvent: which only processes
the IOHIDPostEvent stream — so auth-enveloped events are silently ignored.

Fix (when window_id is supplied to hotkey/press_key):
1. FocusWithoutRaise.activateForMenuShortcut — calls
   SLPSSetFrontProcessWithOptions(kCPSNoWindows = 0x400) to make the target
   WindowServer-frontmost without raising its window or triggering Space follow.
2. Post via SLEventPostToPid WITHOUT the auth-message envelope
   (attachAuthMessage: false) — routes through IOHIDPostEvent so
   NSApplication.sendEvent: dispatches NSMenu key equivalents.

The Chromium/renderer path (no window_id) is unchanged: auth-message envelope
stays on since Chromium requires it for its keyboard pipeline.

Also replaces the deprecated GetProcessForPID Carbon API (removed in macOS 26)
with SLSGetWindowOwner + SLSGetConnectionPSN for window-ID → PSN lookup.

Test: test_background_menu_shortcut.py — verifies via FocusMonitorApp:
- hotkey WITHOUT window_id → 0 focus losses (no activation, Chromium path)
- hotkey WITH window_id → 1 focus loss (activateForMenuShortcut) + new
  TextEdit document opens (NSMenu Cmd+N fires correctly)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview May 4, 2026 6:31pm

Request Review

@coderabbitai

coderabbitai Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bf0eba22-6a98-475b-9ede-ea97169b5188

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR enhances menu key equivalent handling for backgrounded applications by adding window-id-based focus activation (without raising or stealing focus), introducing an attachAuthMessage parameter to keyboard input APIs, and integrating these capabilities into the hotkey and key-press tools. A new integration test validates the behavior.

Changes

Background Menu Shortcut Support

Layer / File(s) Summary
SkyLight SPI Bindings & PSN Resolution
libs/cua-driver/Sources/CuaDriverCore/Input/SkyLightEventPost.swift
New private SPI resolvers for SLSGetWindowOwner and SLSGetConnectionPSN enable window-to-PSN lookup. Added public getProcessPSN(forWindowId:into:) overload and setFrontProcessNoWindows(psn:windowID:) to activate target process at WindowServer level without raising windows. Updated isFocusWithoutRaiseAvailable to account for modern window-based resolution path.
Focus-Without-Raise APIs
libs/cua-driver/Sources/CuaDriverCore/Input/FocusWithoutRaise.swift
Updated activateWithoutRaise to use window-id-based PSN resolution. Added new activateForMenuShortcut(targetPid:targetWid:) method that resolves target PSN from window ID and calls setFrontProcessNoWindows to activate without raising. Updated recipe documentation to reflect window-id resolution.
Keyboard Input Auth Message Control
libs/cua-driver/Sources/CuaDriverCore/Input/KeyboardInput.swift
Added attachAuthMessage: Bool = true parameter to public press and hotkey methods, threading it through to sendKey. Updated internal sendKey to use SkyLightEventPost.postToPid(..., attachAuthMessage:) with fallback. Ensured sendUnicodeCharacter clears CGEvent.flags for each synthesized unicode event to isolate modifier state.
Tool Schema & Wiring — Hotkey
libs/cua-driver/Sources/CuaDriverServer/Tools/HotkeyTool.swift
Added optional window_id (CGWindowID) to hotkey tool schema. In invoke, when window_id is provided and non-zero, activates target window via FocusWithoutRaise.activateForMenuShortcut, sleeps briefly, then posts hotkey with attachAuthMessage: false; otherwise uses existing auth-message behavior.
Tool Schema & Wiring — Key Press
libs/cua-driver/Sources/CuaDriverServer/Tools/PressKeyTool.swift
Extended window_id handling: when provided without element_index, invokes FocusWithoutRaise.activateForMenuShortcut, sleeps, then sends key via KeyboardInput.press(..., attachAuthMessage: false). Preserved prior direct-press behavior when window_id is absent or zero. Updated schema description to document menu-shortcut pathway.
Documentation Update
libs/cua-driver/Sources/CuaDriverServer/Tools/ScreenshotTool.swift
Updated window_id parameter description to direct users to list_windows instead of get_accessibility_tree.
Integration Test
libs/cua-driver/Tests/integration/test_background_menu_shortcut.py
Added TestBackgroundMenuShortcut class validating menu key behavior on backgrounded apps. Tests: (1) Cmd+Z without window_id does not steal focus from FocusMonitorApp sentinel; (2) Cmd+N with window_id activates TextEdit at WindowServer level, opens a new document, and steals focus exactly once. Includes helper utilities for focus-loss detection via /tmp/focus_monitor_losses.txt counter and on-screen window filtering.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Menu shortcuts bloom from windowsills,
No raise, no fuss, just focused wills.
A rabbit hops through auth messages true,
And tests confirm: backgrounded apps shine through!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: NSMenu key equivalents on backgrounded apps, overlay z-order fixes, and focus-related improvements across multiple files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cua-driver/overlay-z-order-and-focus-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Conflicts resolved by taking main's additions for all hunks:
- .bumpversion.cfg / CuaDriverCore.swift: version 0.1.1 → 0.1.2
- ScreenshotTool.swift: window_id is now required
- CuaDriverCommand.swift: add --claude-code-computer-use-compat flag
- CLIDocExtractor.swift: document the new compat flag
- install.sh: add claude-code-computer-use-compat install note
- uninstall.sh: add cache dir + Claude MCP registration cleanup

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
libs/cua-driver/Sources/CuaDriverCore/Input/FocusWithoutRaise.swift (1)

50-69: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the PID fallback for pre-macOS 15.

Both activation paths now depend entirely on getProcessPSN(forWindowId:), but that helper returns false when SLSGetWindowOwner / SLSGetConnectionPSN are unavailable. Since targetPid is still available here, older systems lose background focus/menu-shortcut support unnecessarily.

Suggested fix
     let targetOk = targetPSN.withUnsafeMutableBytes { raw in
-        SkyLightEventPost.getProcessPSN(forWindowId: targetWid, into: raw.baseAddress!)
+        SkyLightEventPost.getProcessPSN(forWindowId: targetWid, into: raw.baseAddress!)
+            || SkyLightEventPost.getProcessPSN(forPid: targetPid, into: raw.baseAddress!)
     }
     guard targetOk else { return false }

Apply the same fallback in activateForMenuShortcut(...) as well.

Also applies to: 127-136

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/cua-driver/Sources/CuaDriverCore/Input/FocusWithoutRaise.swift` around
lines 50 - 69, In activateWithoutRaise, retain the pre-macOS15 PID fallback: if
SkyLightEventPost.getProcessPSN(forWindowId:into:) returns false, fall back to
constructing a PSN (process serial number) or using the PID path used previously
(same approach as activateForMenuShortcut) so the call path still works on older
systems where SLSGetWindowOwner/SLSGetConnectionPSN are unavailable; update both
activateWithoutRaise and activateForMenuShortcut to attempt
getProcessPSN(forWindowId:into:) first and if it fails use the targetPid-based
PSN/pid-fallback code path (refer to prevPSN, targetPSN, getFrontProcess, and
getProcessPSN symbols to locate where to insert the fallback).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/HotkeyTool.swift`:
- Around line 102-117: The branch silently proceeds when rawWindowId is nil/out
of UInt32 range or when
FocusWithoutRaise.activateForMenuShortcut(targetPid:targetWid:) fails; change
the logic in the block that checks rawWindowId and calls
FocusWithoutRaise.activateForMenuShortcut so it fails fast: if rawWindowId is
nil or UInt32(exactly: rawWindowId) is nil, throw or return an error immediately
instead of falling back to the non-windowed hotkey path, and after calling
FocusWithoutRaise.activateForMenuShortcut, check its boolean result and
throw/return on false before calling KeyboardInput.hotkey(keys, toPid: pid,
attachAuthMessage: false) so callers see the real failure rather than a silent
no-op.

In `@libs/cua-driver/Sources/CuaDriverServer/Tools/PressKeyTool.swift`:
- Around line 160-171: FocusWithoutRaise.activateForMenuShortcut can fail
silently yet you proceed to call KeyboardInput.press with
attachAuthMessage:false; update the PressKeyTool logic to check the boolean
result of FocusWithoutRaise.activateForMenuShortcut (when rawWindowId -> UInt32
path is taken) and handle failure explicitly: if activateForMenuShortcut returns
false, do not call KeyboardInput.press(..., attachAuthMessage: false) — instead
either call the fallback KeyboardInput.press(key, modifiers: modifiers, toPid:
pid) or propagate/throw an error so the caller sees the failure; ensure the
change is applied around the same conditional that converts rawWindowId to
UInt32 and references FocusWithoutRaise.activateForMenuShortcut and
KeyboardInput.press.

In `@libs/cua-driver/Tests/integration/test_background_menu_shortcut.py`:
- Around line 107-121: The current _launch_focus_app() blocks on
proc.stdout.readline() and can hang; modify the loop to poll the subprocess
stdout with a timeout (e.g., using select.select on proc.stdout.fileno() or
os.poll) and only call proc.stdout.readline() when data is available, handle EOF
(empty read) by terminating the process and raising, and preserve the existing
4s total timeout/loop count and the returned (proc, pid) behavior; update
_launch_focus_app to use select/select-like readiness checks before reading, and
ensure proc.terminate() is still called on timeout or EOF.

---

Outside diff comments:
In `@libs/cua-driver/Sources/CuaDriverCore/Input/FocusWithoutRaise.swift`:
- Around line 50-69: In activateWithoutRaise, retain the pre-macOS15 PID
fallback: if SkyLightEventPost.getProcessPSN(forWindowId:into:) returns false,
fall back to constructing a PSN (process serial number) or using the PID path
used previously (same approach as activateForMenuShortcut) so the call path
still works on older systems where SLSGetWindowOwner/SLSGetConnectionPSN are
unavailable; update both activateWithoutRaise and activateForMenuShortcut to
attempt getProcessPSN(forWindowId:into:) first and if it fails use the
targetPid-based PSN/pid-fallback code path (refer to prevPSN, targetPSN,
getFrontProcess, and getProcessPSN symbols to locate where to insert the
fallback).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a67516df-5acd-4230-832a-baf0934ab8f3

📥 Commits

Reviewing files that changed from the base of the PR and between e5098af and ae86c06.

📒 Files selected for processing (7)
  • libs/cua-driver/Sources/CuaDriverCore/Input/FocusWithoutRaise.swift
  • libs/cua-driver/Sources/CuaDriverCore/Input/KeyboardInput.swift
  • libs/cua-driver/Sources/CuaDriverCore/Input/SkyLightEventPost.swift
  • libs/cua-driver/Sources/CuaDriverServer/Tools/HotkeyTool.swift
  • libs/cua-driver/Sources/CuaDriverServer/Tools/PressKeyTool.swift
  • libs/cua-driver/Sources/CuaDriverServer/Tools/ScreenshotTool.swift
  • libs/cua-driver/Tests/integration/test_background_menu_shortcut.py

Comment on lines +102 to +117
if let rawWid = rawWindowId, rawWid != 0,
let wid = UInt32(exactly: rawWid)
{
FocusWithoutRaise.activateForMenuShortcut(
targetPid: pid, targetWid: CGWindowID(wid))
usleep(50_000)
// Post WITHOUT the SkyLight auth-message envelope.
// With the envelope, SLEventPostToPid forks onto a direct-mach
// path that bypasses IOHIDPostEvent — NSMenu never sees those
// events. Without the envelope the path goes through IOHIDPostEvent
// so NSApplication.sendEvent: dispatches NSMenu key equivalents.
// SLPSSetFrontProcessWithOptions (called inside activateForMenuShortcut)
// already made the target WindowServer-frontmost.
try KeyboardInput.hotkey(keys, toPid: pid, attachAuthMessage: false)
} else {
try KeyboardInput.hotkey(keys, toPid: pid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when window_id is invalid or activation fails.

This branch silently degrades if window_id is out of UInt32 range or activateForMenuShortcut(...) returns false, but it still reports success. For the NSMenu path that turns a real targeting failure into a silent no-op.

Suggested fix
-                if let rawWid = rawWindowId, rawWid != 0,
-                   let wid = UInt32(exactly: rawWid)
-                {
-                    FocusWithoutRaise.activateForMenuShortcut(
-                        targetPid: pid, targetWid: CGWindowID(wid))
+                if let rawWid = rawWindowId {
+                    guard rawWid != 0, let wid = UInt32(exactly: rawWid) else {
+                        return errorResult(
+                            "window_id \(rawWid) is outside the supported UInt32 range.")
+                    }
+                    guard FocusWithoutRaise.activateForMenuShortcut(
+                        targetPid: pid, targetWid: CGWindowID(wid))
+                    else {
+                        return errorResult(
+                            "Failed to activate window_id \(rawWid) for NSMenu shortcut delivery.")
+                    }
                     usleep(50_000)
                     // Post WITHOUT the SkyLight auth-message envelope.
                     // With the envelope, SLEventPostToPid forks onto a direct-mach
@@
-                } else {
+                } else {
                     try KeyboardInput.hotkey(keys, toPid: pid)
                 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/cua-driver/Sources/CuaDriverServer/Tools/HotkeyTool.swift` around lines
102 - 117, The branch silently proceeds when rawWindowId is nil/out of UInt32
range or when FocusWithoutRaise.activateForMenuShortcut(targetPid:targetWid:)
fails; change the logic in the block that checks rawWindowId and calls
FocusWithoutRaise.activateForMenuShortcut so it fails fast: if rawWindowId is
nil or UInt32(exactly: rawWindowId) is nil, throw or return an error immediately
instead of falling back to the non-windowed hotkey path, and after calling
FocusWithoutRaise.activateForMenuShortcut, check its boolean result and
throw/return on false before calling KeyboardInput.hotkey(keys, toPid: pid,
attachAuthMessage: false) so callers see the real failure rather than a silent
no-op.

Comment on lines +160 to +171
if let rawWid = rawWindowId, rawWid != 0,
let wid = UInt32(exactly: rawWid)
{
FocusWithoutRaise.activateForMenuShortcut(
targetPid: pid, targetWid: CGWindowID(wid))
usleep(50_000)
// Same recipe as HotkeyTool — see its inline comment.
try KeyboardInput.press(
key, modifiers: modifiers, toPid: pid, attachAuthMessage: false)
} else {
try KeyboardInput.press(
key, modifiers: modifiers, toPid: pid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle activation failure before posting the key.

This has the same silent-failure mode as HotkeyTool: an invalid/stale window_id, or a false return from activateForMenuShortcut(...), still leads to a success result even though the NSMenu-targeted path was not established.

Suggested fix
-                    if let rawWid = rawWindowId, rawWid != 0,
-                       let wid = UInt32(exactly: rawWid)
-                    {
-                        FocusWithoutRaise.activateForMenuShortcut(
-                            targetPid: pid, targetWid: CGWindowID(wid))
+                    if let rawWid = rawWindowId {
+                        guard rawWid != 0, let wid = UInt32(exactly: rawWid) else {
+                            return errorResult(
+                                "window_id \(rawWid) is outside the supported UInt32 range.")
+                        }
+                        guard FocusWithoutRaise.activateForMenuShortcut(
+                            targetPid: pid, targetWid: CGWindowID(wid))
+                        else {
+                            return errorResult(
+                                "Failed to activate window_id \(rawWid) for NSMenu key delivery.")
+                        }
                         usleep(50_000)
                         // Same recipe as HotkeyTool — see its inline comment.
                         try KeyboardInput.press(
                             key, modifiers: modifiers, toPid: pid, attachAuthMessage: false)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/cua-driver/Sources/CuaDriverServer/Tools/PressKeyTool.swift` around
lines 160 - 171, FocusWithoutRaise.activateForMenuShortcut can fail silently yet
you proceed to call KeyboardInput.press with attachAuthMessage:false; update the
PressKeyTool logic to check the boolean result of
FocusWithoutRaise.activateForMenuShortcut (when rawWindowId -> UInt32 path is
taken) and handle failure explicitly: if activateForMenuShortcut returns false,
do not call KeyboardInput.press(..., attachAuthMessage: false) — instead either
call the fallback KeyboardInput.press(key, modifiers: modifiers, toPid: pid) or
propagate/throw an error so the caller sees the failure; ensure the change is
applied around the same conditional that converts rawWindowId to UInt32 and
references FocusWithoutRaise.activateForMenuShortcut and KeyboardInput.press.

Comment on lines +107 to +121
def _launch_focus_app() -> tuple[subprocess.Popen, int]:
proc = subprocess.Popen(
[_FOCUS_APP_EXE],
stdout=subprocess.PIPE,
stderr=subprocess.DEVNULL,
text=True,
)
for _ in range(40):
line = proc.stdout.readline().strip()
if line.startswith("FOCUS_PID="):
pid = int(line.split("=", 1)[1])
return proc, pid
time.sleep(0.1)
proc.terminate()
raise RuntimeError("FocusMonitorApp did not print FOCUS_PID in time")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Python's subprocess module, does Popen.stdout.readline() block until it receives a newline or EOF, and what non-blocking pattern is recommended for implementing a timeout while waiting for child process output?

💡 Result:

Yes—Popen.stdout.readline blocks until it can return a complete “line” (a trailing newline in text mode, or a newline byte in binary mode) or until it reaches EOF. How to implement a timeout for waiting on child output (non-blocking pattern): don’t try to make readline non-blocking directly. Instead, use a readiness wait (select/poll) or an asynchronous/threaded reader that you can time out. Recommended approach (works with line-delimited output): 1) Use select.select with a timeout to wait until the pipe is readable, then call readline (which will not block because data is ready). Example: import subprocess, select p = subprocess.Popen(cmd, stdout=subprocess.PIPE, text=True, bufsize=1) deadline = 10 # seconds end = time.time + deadline buf = '' while True: remaining = end - time.time if remaining <= 0: raise TimeoutError('Timed out waiting for child output') r, _, _ = select.select([p.stdout], [], [], remaining) if not r: raise TimeoutError('Timed out waiting for child output') line = p.stdout.readline # now safe: pipe is readable if line == '': # EOF break print(line, end='') 2) If you already use asyncio, the subprocess-aware timeout pattern is to await readline with asyncio.wait_for(process.stdout.readline, timeout). Example (asyncio): line = await asyncio.wait_for(process.stdout.readline, timeout) Evidence notes: - subprocess pipes should be read/handled carefully; the official docs recommend communicate to avoid deadlocks when using stdout/stderr pipes, and they describe that communicate reads until EOF. - For timeouts while reading lines from a PIPE, the documented non-blocking pattern is to use select/poll to wait for readability before calling readline, rather than calling readline directly (which otherwise blocks waiting for newline/EOF).

Citations:


This timeout loop can hang forever on the first readline().

proc.stdout.readline() blocks until a newline is received or EOF is reached. If FocusMonitorApp fails before printing FOCUS_PID=..., the loop never reaches its intended 4-second timeout and wedges the integration suite instead of failing cleanly.

Suggested fix
+import select
@@
 def _launch_focus_app() -> tuple[subprocess.Popen, int]:
     proc = subprocess.Popen(
         [_FOCUS_APP_EXE],
         stdout=subprocess.PIPE,
@@
-    for _ in range(40):
-        line = proc.stdout.readline().strip()
-        if line.startswith("FOCUS_PID="):
-            pid = int(line.split("=", 1)[1])
-            return proc, pid
-        time.sleep(0.1)
+    deadline = time.time() + 4.0
+    while time.time() < deadline:
+        ready, _, _ = select.select([proc.stdout], [], [], 0.1)
+        if not ready:
+            if proc.poll() is not None:
+                break
+            continue
+        line = proc.stdout.readline().strip()
+        if line.startswith("FOCUS_PID="):
+            pid = int(line.split("=", 1)[1])
+            return proc, pid
     proc.terminate()
     raise RuntimeError("FocusMonitorApp did not print FOCUS_PID in time")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/cua-driver/Tests/integration/test_background_menu_shortcut.py` around
lines 107 - 121, The current _launch_focus_app() blocks on
proc.stdout.readline() and can hang; modify the loop to poll the subprocess
stdout with a timeout (e.g., using select.select on proc.stdout.fileno() or
os.poll) and only call proc.stdout.readline() when data is available, handle EOF
(empty read) by terminating the process and raising, and preserve the existing
4s total timeout/loop count and the returned (proc, pid) behavior; update
_launch_focus_app to use select/select-like readiness checks before reading, and
ensure proc.terminate() is still called on timeout or EOF.

cua and others added 2 commits May 4, 2026 10:18
- Call unhide() after launch so windows appear on screen in the background
- If app already running with no on-screen windows (e.g. Finder windows on
  a different Space), re-send an oapp AppleEvent to create a new window on
  the current Space — same behavior as clicking the Dock icon
- Update tool description: "Launches in the background" (was "Launches hidden")

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ri to about:blank

- Finder fallback: open ~/ via application(_:open:) when no on-screen windows
- Safari fallback: open about:blank instead of ~/ to avoid file:// URL side effects
- Scope fallback to com.apple.finder and com.apple.Safari only (avoids unintended
  side effects on document apps, Calculator, etc.)
- Add integration test: test_launch_app_visible.py covering TextEdit cold-launch,
  Calculator focus-steal suppression, and Finder no-URL window creation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ddupont808
ddupont808 merged commit 8a551a8 into main May 4, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant