Skip to content

fix(computer-use): follow-ups to #141 — foreground selection and live-test robustness - #142

Merged
echoVic merged 5 commits into
mainfrom
fix/computer-use-followups
Oct 20, 2026
Merged

echoVic merged 5 commits into
mainfrom
fix/computer-use-followups

Conversation

@echoVic

@echoVic echoVic commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

Follow-ups to #141: one driver fix from CodeRabbit's second round, plus three live-test robustness fixes found while verifying #141 on a real Mac.

Approach

  • fix(computer-use) 66dd148: in the foreground path, element type keeps the selection only when the target was focused originally and is still focused at the click decision. Otherwise the selection collapses to the end. This also covers a target that the background attempt has just focused, where Cocoa may select all. It addresses fix(computer-use): live-verified macOS typing, focus and overlay fixes #141's CodeRabbit comment at foreground.rs:318, without the suggested diff, which would have stopped collapsing those freshly focused targets.

  • test a2903af: the direct-click probe in textedit_save_panel_uses_foreground_for_typing accepts either safe outcome of the ownership check:

    • refused, with no mouse-down;
    • allowed, with exactly one mouse-down.

    In both cases the same test-owned filename field must stay focused and unchanged. On macOS 26.6.2 the AX hit test for the Save panel field reports either TextEdit or its view service; 2 of 5 runs refused. This assertion comes from main (30354e5).

  • test 6411938: live helpers retry AXError::CannotComplete the same way as InvalidUIElement (up to 5 attempts, 100 ms apart). This fixes one_failed_frame_does_not_abort_observation when it reads AXMainWindow right after launch.

  • test d109461: live cleanup confirms that every owned process actually exits.

    • It checks with kill(pid, 0) and escalates terminate → forceTerminate → SIGKILL, or SIGTERM → SIGKILL when there is no AppKit handle.
    • It registers every started PID in the fixture's private log, including late receipts.
    • It reports any survivor as a restoration failure.
    • It refuses to launch a fixture while an unowned CUFixture is alive, and never terminates that instance.

    In one full run, a fixture left behind by observe_captures_a_window_covered_by_another caused 11 later tests to act on the wrong instance.

Related Issue

Follow-up to #141.

Verification

All checks ran on macOS 26.6.2 (Apple Silicon) with stable rustc 1.97.1.

  • Live GUI suite at d109461: 92 of 92 passed with no retries or user-activity skips. observe_of_an_electron_app_waits_for_its_tree skips without ORCA_CU_ELECTRON_APP.
    • Command: cargo test -p orca-computer-use --test live_macos --locked -- --ignored --test-threads=1 --nocapture.
    • Cleanup confirmed 109 owned processes gone. No SIGKILL was needed, and no fixture was left behind.
  • At 6411938: textedit_save_panel_uses_foreground_for_typing and one_failed_frame_does_not_abort_observation each passed 4 times: once in a full run and 3 repeats.
  • cargo test -p orca-computer-use --locked: 221 unit, 8 cleanup contract, 3 harness contract and 8 default live_macos tests pass.
  • env -u NODE_OPTIONS cargo test -p orca-runtime --lib --locked -- computer_use: 58 pass, 2 ignored.
  • cargo fmt --all -- --check and git diff --check are clean.
  • cargo clippy -p orca-computer-use --all-targets --locked reports no warnings in changed files.
  • cargo check -p orca-computer-use --tests --locked passes for x86_64-unknown-linux-gnu and x86_64-pc-windows-msvc.
  • Not run locally: the full workspace test and clippy gates, left to CI.

Impact

  • Public protocol/CLI: none.
  • Persisted data/migration: none.
  • Security/permissions: none. The driver change only decides whether to collapse a selection before foreground typing; recipient and password checks are unchanged.
  • Dependencies: none.
  • Documentation: none.

User Interface

Not applicable.

Checklist

  • The change is focused and contains no unrelated refactors.
  • Tests cover the change where appropriate.
  • cargo fmt --all -- --check passes.
  • cargo test --workspace --all-targets -- --test-threads=1 passes. (Left to CI.)
  • If credentials, platform limits, or external services block the full test gate, the Verification section lists the largest relevant test subset run and explains the blocker.
  • cargo clippy --workspace --all-targets passes. (Left to CI.)
  • If credentials, platform limits, or external services block the full clippy gate, the Verification section lists the largest relevant subset run and explains the blocker.
  • Public behavior changes are reflected in documentation.
  • No secrets or sensitive data are included.
  • No version or release artifacts are included unless explicitly requested by a maintainer.

Summary by CodeRabbit

  • Bug Fixes
    • Foreground typing now preserves a text selection only when the target remains focused; otherwise, typing starts at the end of the text.
  • Tests
    • Improved macOS interaction checks for transient accessibility errors, click behavior, and cleanup of test-launched processes.

Address PR #141 CodeRabbit second-round item B. Preserve an element Type
selection only when the target was originally focused and is still focused
at the foreground click decision. A newly focused target still collapses
its selection, including Cocoa's select-all behavior on refocus.

Keep the click-decision result until selection handling. If no click decision
ran, resolve keyboard_target_is_focused again immediately before collapsing.
Add a pure decision helper and unit cases for lost, newly acquired, and
retained focus. The lost-focus case failed against the old snapshot-only
rule before the fix.

Checked: stable rustc 1.97.1; cargo +stable fmt -p orca-computer-use;
cargo +stable test -p orca-computer-use (221 unit, 3 harness contract,
8 default live tests passed; 92 live tests ignored); cargo +stable clippy
-p orca-computer-use --all-targets (existing warnings only); cargo +stable
test -p orca-computer-use --test live_macos --no-run; git diff --check.
No ignored GUI or runtime native/real tests were run.
Keep the direct click probe in textedit_save_panel_uses_foreground_for_typing,
but accept either AX attribution for the test-owned filename field. A refused
click must report a covering window and emit no mouse-down; an allowed click
must emit exactly one mouse-down. Log which outcome the native guard returned.

Check the test filename and focused field before clicking only that field's
native point. After either outcome require the same filename field to remain
focused with its exact value, proving the panel remains open. Preserve the
non-text key no-click assertion, foreground/pointer restoration, and Escape
cleanup. Do not change the production ownership guard.

Checked: stable rustc 1.97.1; cargo +stable fmt -p orca-computer-use;
cargo +stable test -p orca-computer-use (221 unit, 3 harness contract,
8 default live tests passed; 92 live tests ignored); cargo +stable clippy
-p orca-computer-use --all-targets (existing warnings only); cargo +stable
test -p orca-computer-use --test live_macos --no-run; git diff --check.
The ignored TextEdit GUI test was compiled only, never run.
Treat AXError::CannotComplete like InvalidUIElement in the live-test native
read helper. A newly launched fixture can be temporarily busy despite its
messaging timeout. Retry only these two errors, rediscover the handle before
the next read, and retain the five-attempt ceiling and 100ms retry delay.
Production accessibility reads and other error handling are unchanged.

Extend the existing default stale_ax_reads tests to cover both errors,
mixed transient failures, recovery after handle rediscovery, and the exact
retry ceiling. Keep immediate returns for successful and non-transient reads.
Both CannotComplete regressions failed before updating the retry predicate.

Checked: stable rustc 1.97.1; cargo +stable fmt -p orca-computer-use;
cargo +stable test -p orca-computer-use (221 unit, 3 harness contract,
8 default live tests passed; 92 live tests ignored); cargo +stable clippy
-p orca-computer-use --all-targets (existing warnings only); cargo +stable
test -p orca-computer-use --test live_macos --no-run; git diff --check.
No ignored GUI or runtime native/real tests were run.
Register every valid started PID from each fixture's private log, arm log
tracking before open, and re-read the logs during Drop to catch late starts.
Use a separate log sequence so missing or duplicate receipts cannot reuse a
previous launch's log. Preserve the preexisting-process and session ownership
filters; never discover extra processes globally for termination.

Confirm exit with kill(pid, 0) after terminate and forceTerminate, regardless
of AppKit's isTerminated state. Use SIGTERM when the native app handle is
missing, escalate a still-live owned process to SIGKILL, and confirm again.
Bound each wait and late-receipt cleanup rounds. Log each PID's native handle,
stop requests, process probes, and final outcome; report inspection or final
termination failures through RESTORATION_FAILURES so the suite cannot pass
silently with a known survivor.

Fail clearly before launching a fixture if a foreign fixture is still alive,
without terminating it. Probe the kernel to ignore stale AppKit entries.
Keep Live::start permissive so legitimate Live sessions can coexist and the
other-session preservation test retains its semantics.

Add eight default, GUI-free cleanup contract tests for all/late receipts,
ownership isolation, launch conflicts, ordered escalation, missing app handles,
confirmed exit, persistent survivors, and probe errors. Five regressions failed
against the extracted old first-receipt/native-only cleanup behavior before
the fixes; all eight pass now. The exact cause of the reported live leak was
not reproduced locally because desktop tests were intentionally not run.

Checked: stable rustc 1.97.1; cargo +stable fmt -p orca-computer-use;
cargo +stable test -p orca-computer-use (221 unit, 8 cleanup contract,
3 harness contract, 8 default live tests passed; 92 live GUI tests ignored);
cargo +stable clippy -p orca-computer-use --all-targets (existing warnings
only); cargo +stable test -p orca-computer-use --test live_macos --no-run;
git diff --check. No ignored GUI or runtime native/real tests were run,
and no existing desktop processes were signalled during these checks.
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 595e59e8-f634-46f2-b962-0477934cf09c
📥 Commits

Reviewing files that changed from the base of the PR and between d109461 and 533b56c.

📒 Files selected for processing: 1
  • docs/superpowers/specs/2026-07-28-native-windows-platform-foundation.manifest.json

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The changes update focus checks for foreground typing and revise the macOS live-test harness. The harness tracks fixture processes through receipt logs, checks launch conflicts, and escalates cleanup actions. It also retries two transient accessibility errors and verifies Save-panel click outcomes.

Changes: Foreground Typing

Layer / File(s) Summary
Focus-aware selection handling
crates/orca-computer-use/src/driver/macos/foreground.rs
Typing preserves the target selection only when the target was focused before the action and at the click. Tests cover the focus combinations.

Live macOS Test Harness

Layer / File(s) Summary
Cleanup helpers and contract tests
crates/orca-computer-use/tests/support/live_cleanup.rs, crates/orca-computer-use/tests/live_cleanup_contract.rs
Helpers parse started-process receipts, select cleanup candidates, detect conflicts, and choose cleanup actions from liveness probes. Contract tests cover these behaviors and errors.
Receipt-based fixture tracking
crates/orca-computer-use/tests/live_macos.rs, docs/superpowers/specs/2026-07-28-native-windows-platform-foundation.manifest.json
The harness checks for unowned running fixtures before launch, discovers PIDs from receipt logs, and retries cleanup for attributed processes. The manifest records a deferred portability boundary.
Process probes and termination
crates/orca-computer-use/tests/live_macos.rs
The harness validates process IDs, probes liveness, waits for exit with action-specific timeouts, and applies native termination or signals.
Retry transient accessibility reads
crates/orca-computer-use/tests/live_macos.rs
Reads retry InvalidUIElement and CannotComplete, re-resolve between attempts, and stop after five reads. Tests cover individual and mixed errors.
Save-panel click ownership assertions
crates/orca-computer-use/tests/live_macos.rs
The test checks mouse-down counts for refused and allowed clicks, then verifies the filename field retains focus and its value.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 533b5

The supplied evidence establishes no unresolved concrete failure in the foreground-typing or macOS test-harness changes. No additional merge-blocking risk is identified.

### Architecture Summary

Architecture risk: 🔵 Low · up to 533b5

The change affects 2 systems.

Changed systems: crates, docs

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — crates (service) was modified; 4 changed files map to changed impact.
  • observed — docs (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in crates/orca-computer-use/src/driver/macos/foreground.rs: Added preserve_foreground_type_selection, which returns true only when the target was focused both before the action and at the later check, plus tests for the focus combinations.
  • observed — Modified behavior in crates/orca-computer-use/src/driver/macos/foreground.rs: Typing now stores the focus state observed before the optional click in focus_at_click.
  • observed — Modified behavior in crates/orca-computer-use/src/driver/macos/foreground.rs: The pre-click focus result is retained for the subsequent selection decision.
  • observed — Modified behavior in crates/orca-computer-use/src/driver/macos/foreground.rs: For element-target typing, selection collapse now depends on both the target’s original focus and its focus at the click; the previous check used only the original focus. The click-time state is reused when available, or queried afterward when no click location was captured.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 4 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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 clearly summarizes the main changes: foreground selection handling and live-test robustness. It is concise and specific.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 30.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

d109461 added process-level cleanup checks to live_macos.rs: two libc::kill
calls and the SIGTERM and SIGKILL constants. Record the four matches counted
by the Windows platform-boundary validator as a windows-test-portability
entry named computer-use-live-macos-process-cleanup. ESRCH and EPERM classify
probe failures but are not part of that validator's operation pattern.

The entire fixture is cfg(target_os = "macos") and is not compiled on Windows.
Keep the reviewed boundary explicit without changing the validator or the
cleanup behavior. The other inventories need no additions for this branch.

Checked: node scripts/validate-windows-platform-boundaries.mjs;
node scripts/test-validate-windows-platform-boundaries.mjs;
node scripts/validate-runtime-surface-contract.mjs;
node scripts/test-validate-runtime-surface-contract.mjs;
node scripts/validate-execution-broker-boundary.mjs;
node scripts/test-validate-execution-broker-boundary.mjs;
node scripts/validate-http-client-boundary.mjs;
node scripts/test-validate-http-client-boundary.mjs;
node scripts/test-repository-hygiene.mjs; git diff --check.
All checks passed. No live GUI tests were run.
@echoVic
echoVic merged commit a016619 into main Oct 20, 2026
4 checks passed
@echoVic
echoVic deleted the fix/computer-use-followups branch October 20, 2026 15:10
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