Skip to content

Commit 03612ce

Browse files
authored
Merge pull request #3430 from bobleer/codex/fix-computer-use-window-selection
fix(computer-use): discover and target exact application windows
2 parents 20e6a73 + 41b06f9 commit 03612ce

16 files changed

Lines changed: 582 additions & 98 deletions

File tree

‎docs/architecture/computer-use-control.md‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,17 @@ they do not require a user to select the next app. Background AT-SPI startup
149149
does not open a portal picker. Linux portal capture and foreground seat input
150150
still require compositor-owned consent.
151151

152+
`list_windows` is read-only discovery independent of the active capture and AX
153+
tree. macOS returns WindowServer top-level windows including off-screen entries;
154+
Windows returns visible, non-minimized titled top-level windows. Titles and owner
155+
PIDs reveal multi-window and helper-process UIs that an application list cannot.
156+
Hosts advertising `window-target-v1` accept `app: {pid, window_id}` to select an
157+
exact native window. Both IDs must be positive, and native capture validates the
158+
window's ownership before binding it. Omitted window IDs retain an existing
159+
binding. Older hosts and Linux reject exact-window requests explicitly; Linux
160+
keeps its existing AT-SPI and portal selection contracts. No remote-workspace
161+
request falls back to the controller's desktop.
162+
152163
The optional manual-sharing API uses `awaiting_share`, opaque process-bound
153164
choices, observe-first capture and a separate revocable input grant. Its scope
154165
does not apply to normal Agent tasks. Existing remembered preferences and manual
@@ -364,6 +375,13 @@ or screen-capture-permission failures, marked `capture_preparation_error`,
364375
create no binding and attach no prior target's pixels. Lock, stop, ownership,
365376
generation, revocation and unknown errors cannot use that fallback.
366377

378+
An exact-window preparation failure cannot fall back to another window's AX
379+
tree, including a previously bound window of the same PID. App-only preparation
380+
failures suggest native window discovery instead of asserting that the task is
381+
absent. `describe_screen` keeps its text-only default and honors explicit
382+
`include_image: true` for vision-capable models, with the selected image's
383+
reference and geometry in the result. Text-only models receive no image bytes.
384+
367385
A text-only screenshot request performs a real text observation. Tool results
368386
preserve structured identities, AX/OCR facts, errors and geometry in the model's
369387
actual result channel; a short summary is not a replacement. Image attachments
@@ -403,8 +421,8 @@ operation or prove all transient or later application behavior.
403421
### macOS
404422

405423
Retain the selected content window across observations. A sharing-indicator
406-
window appearing earlier in WindowServer order must not replace it. Conflicting
407-
explicit window selection fails; validate a replacement before releasing a live
424+
window appearing earlier in WindowServer order must not replace it. An explicit
425+
window ID can select another window, including within the same PID; validate a replacement before releasing a live
408426
capture. AX focused-window observation resolves the actual bound window, not an
409427
unrelated Sharing dialog. Missing exact identity is an error.
410428

‎scripts/fixtures/computer-use-roundtrip.m‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ - (BOOL)canBecomeKeyWindow { return YES; }
1616
@interface OBFRoundtripEditor : NSView
1717
@end
1818
@implementation OBFRoundtripEditor
19-
- (BOOL)acceptsFirstMouse:(NSEvent *)event { return YES; }
2019
- (BOOL)acceptsFirstResponder { return YES; }
2120
- (BOOL)isAccessibilityElement { return YES; }
2221
- (NSString *)accessibilityRole { return NSAccessibilityTextAreaRole; }
@@ -62,6 +61,13 @@ int main(int argc, const char **argv) { @autoreleasepool {
6261
[window orderFront:nil];
6362
[window makeKeyWindow];
6463
[window makeFirstResponder:editor];
64+
// A second, larger window deliberately wins legacy PID-only capture.
65+
// The tool must discover and explicitly select the smaller task window.
66+
NSWindow *mainWindow = [[NSWindow alloc] initWithContentRect:NSMakeRect(700,100,760,500) styleMask:NSWindowStyleMaskTitled backing:NSBackingStoreBuffered defer:NO];
67+
mainWindow.title = @"Computer Use Large Main Fixture";
68+
mainWindow.releasedWhenClosed = NO;
69+
mainWindow.backgroundColor = NSColor.purpleColor;
70+
[mainWindow orderFront:nil];
6571
publish();
6672
double y = CGDisplayBounds(CGMainDisplayID()).size.height - 170;
6773
printf("READY %d 170 %.0f\n",getpid(),y); fflush(stdout);

‎src/apps/desktop/src/computer_use/control_session.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ fn projected(r: &Resource) -> ControlSnapshot {
165165
s.capabilities.extend(
166166
[
167167
"window-capture",
168+
"window-target-v1",
168169
"background-ax-semantic-input",
169170
"background-directed-keyboard",
170171
"background-directed-scroll",
@@ -177,6 +178,7 @@ fn projected(r: &Resource) -> ControlSnapshot {
177178
s.capabilities.extend(
178179
[
179180
"window-capture",
181+
"window-target-v1",
180182
"background-uia-semantic-input",
181183
"background-window-messages",
182184
"foreground-seat-input",

‎src/apps/desktop/src/computer_use/desktop_host/mod.rs‎

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ use async_trait::async_trait;
1818
use log::debug;
1919
use openbitfun_core::agentic::tools::computer_use_host::{
2020
ActionRecord, AppInfo, AppSelector, AppShortcutsSnapshot, AppStateSnapshot, AppWaitPredicate,
21-
ComputerScreenshot, ComputerUseDisplayInfo, ComputerUseHost, ComputerUseInteractionState,
22-
ComputerUseLastMutationKind, ComputerUsePermissionSnapshot, ComputerUseScreenshotParams,
23-
ComputerUseScreenshotRefinement, ComputerUseSessionSnapshot, InteractiveActionResult,
24-
InteractiveClickParams, InteractiveScrollParams, InteractiveTypeTextParams, InteractiveView,
25-
InteractiveViewOpts, LoopDetectionResult, UiElementLocateQuery, UiElementLocateResult,
26-
VisualActionResult, VisualClickParams, VisualMarkView, VisualMarkViewOpts,
21+
AppWindowInfo, ComputerScreenshot, ComputerUseDisplayInfo, ComputerUseHost,
22+
ComputerUseInteractionState, ComputerUseLastMutationKind, ComputerUsePermissionSnapshot,
23+
ComputerUseScreenshotParams, ComputerUseScreenshotRefinement, ComputerUseSessionSnapshot,
24+
InteractiveActionResult, InteractiveClickParams, InteractiveScrollParams,
25+
InteractiveTypeTextParams, InteractiveView, InteractiveViewOpts, LoopDetectionResult,
26+
UiElementLocateQuery, UiElementLocateResult, VisualActionResult, VisualClickParams,
27+
VisualMarkView, VisualMarkViewOpts,
2728
};
2829
#[cfg(any(target_os = "macos", target_os = "windows"))]
2930
use openbitfun_core::agentic::tools::computer_use_host::{
@@ -928,6 +929,8 @@ impl ComputerUseHost for DesktopComputerUseHost {
928929
}
929930

930931
async fn prepare_control_target(&self, mut app: AppSelector) -> OpenBitFunResult<()> {
932+
app.validate_window_target()
933+
.map_err(OpenBitFunError::tool)?;
931934
if app.is_empty() {
932935
if let Some(pid) = crate::computer_use::control_session::snapshot()
933936
.target
@@ -942,8 +945,13 @@ impl ComputerUseHost for DesktopComputerUseHost {
942945
#[cfg(target_os = "macos")]
943946
{
944947
let pid = resolve_pid_macos(self, &app).await?;
948+
let window_id = app.window_id.map(u32::try_from).transpose().map_err(|_| {
949+
OpenBitFunError::tool(
950+
"[INVALID_PARAMS] window_id exceeds the macOS window ID range",
951+
)
952+
})?;
945953
crate::computer_use::control_session::spawn_blocking(move || {
946-
crate::computer_use::macos_capture::ensure_capture(pid, None)
954+
crate::computer_use::macos_capture::ensure_capture(pid, window_id)
947955
})
948956
.await
949957
.map_err(|e| OpenBitFunError::tool(e.to_string()))?
@@ -966,6 +974,9 @@ impl ComputerUseHost for DesktopComputerUseHost {
966974
}
967975
#[cfg(target_os = "linux")]
968976
{
977+
if app.window_id.is_some() {
978+
return Err(OpenBitFunError::tool("[WINDOW_TARGET_UNSUPPORTED] This host uses AT-SPI app selectors and portal-selected capture, not native window IDs"));
979+
}
969980
if !app.is_empty() {
970981
crate::computer_use::linux_control_ax::bind_app_selector(&app).await?;
971982
}
@@ -1655,6 +1666,44 @@ impl ComputerUseHost for DesktopComputerUseHost {
16551666
}
16561667
}
16571668

1669+
async fn list_windows(&self, app: Option<AppSelector>) -> OpenBitFunResult<Vec<AppWindowInfo>> {
1670+
#[cfg(any(target_os = "macos", target_os = "windows"))]
1671+
{
1672+
let filter = if let Some(app) = app.filter(|app| !app.is_empty()) {
1673+
app.validate_window_target()
1674+
.map_err(OpenBitFunError::tool)?;
1675+
Some((resolve_pid(self, &app).await?, app.window_id))
1676+
} else {
1677+
None
1678+
};
1679+
let windows = tokio::task::spawn_blocking(|| {
1680+
#[cfg(target_os = "macos")]
1681+
{
1682+
crate::computer_use::macos_list_apps::list_windows()
1683+
}
1684+
#[cfg(target_os = "windows")]
1685+
{
1686+
Ok::<_, OpenBitFunError>(crate::computer_use::windows_list_apps::list_windows())
1687+
}
1688+
})
1689+
.await
1690+
.map_err(|e| OpenBitFunError::tool(e.to_string()))??;
1691+
Ok(windows
1692+
.into_iter()
1693+
.filter(|window| {
1694+
filter.is_none_or(|(pid, id)| {
1695+
window.pid == pid && id.is_none_or(|id| window.window_id == id)
1696+
})
1697+
})
1698+
.collect())
1699+
}
1700+
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
1701+
{
1702+
let _ = app;
1703+
Err(OpenBitFunError::tool("[WINDOW_DISCOVERY_UNSUPPORTED] This host exposes AT-SPI applications and portal-selected surfaces; use list_apps and get_app_state"))
1704+
}
1705+
}
1706+
16581707
async fn list_apps(&self, include_hidden: bool) -> OpenBitFunResult<Vec<AppInfo>> {
16591708
#[cfg(target_os = "macos")]
16601709
{
@@ -1798,7 +1847,7 @@ pub(crate) const LINUX_LEGACY_AX_UNAVAILABLE: &str = "[CONTROL_CAPABILITY_UNSUPP
17981847

17991848
#[cfg(target_os = "windows")]
18001849
fn app_selector_is_unspecified(app: &AppSelector) -> bool {
1801-
app.pid.is_none() && app.name.is_none() && app.bundle_id.is_none()
1850+
app.is_empty()
18021851
}
18031852

18041853
#[cfg(any(target_os = "macos", target_os = "windows"))]

‎src/apps/desktop/src/computer_use/desktop_host/native_control_roundtrip_tests.rs‎

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,12 +260,76 @@ pub(crate) async fn run() {
260260
apps.to_string().contains(&pid.to_string()),
261261
"fixture missing from real application discovery"
262262
);
263-
let observation = call(
263+
let before_discovery = host.control_snapshot();
264+
let windows = call(&tool, &context, json!({"action":"list_windows"})).await;
265+
assert_eq!(
266+
host.control_snapshot().generation,
267+
before_discovery.generation
268+
);
269+
assert_eq!(host.control_snapshot().target, before_discovery.target);
270+
let window = windows["windows"]
271+
.as_array()
272+
.unwrap()
273+
.iter()
274+
.find(|window| window["pid"] == pid && window["title"] == "Computer Use Roundtrip Fixture")
275+
.expect("small task window must be discoverable independently of AX and default capture");
276+
let window_id = window["window_id"].as_u64().unwrap();
277+
let main = call(
264278
&tool,
265279
&context,
266280
json!({"action":"get_app_state", "app":{"pid":pid}}),
267281
)
268282
.await;
283+
assert_eq!(
284+
main["app_state"]["window_title"],
285+
"Computer Use Large Main Fixture"
286+
);
287+
let previous_revision = host.control_snapshot().target_revision;
288+
let observation = call(
289+
&tool,
290+
&context,
291+
json!({"action":"get_app_state", "app":{"pid":pid,"window_id":window_id}}),
292+
)
293+
.await;
294+
assert_eq!(
295+
observation["app_state"]["window_title"],
296+
"Computer Use Roundtrip Fixture"
297+
);
298+
assert!(host.control_snapshot().target_revision > previous_revision);
299+
assert_eq!(
300+
host.control_snapshot().target,
301+
Some(format!("pid:{pid}/window:{window_id}"))
302+
);
303+
// A stale/wrong-owner window request must fail without attaching the old
304+
// window's AX or pixels, and without destroying its healthy capture.
305+
let retained = host.control_snapshot();
306+
let wrong_owner = tool.call_impl(&json!({"action":"get_app_state","app":{"pid":std::process::id(),"window_id":window_id}}), &context).await.unwrap_err();
307+
assert!(
308+
wrong_owner
309+
.to_string()
310+
.contains("TARGET_WINDOW_UNAVAILABLE"),
311+
"{wrong_owner}"
312+
);
313+
assert_eq!(host.control_snapshot().target, retained.target);
314+
assert_eq!(
315+
host.control_snapshot().target_revision,
316+
retained.target_revision
317+
);
318+
let described = call_observation(
319+
&tool,
320+
&context,
321+
json!({"action":"describe_screen","include_image":true}),
322+
)
323+
.await;
324+
assert_eq!(described.image_count, 1);
325+
assert_eq!(
326+
described.data["window_title"],
327+
"Computer Use Roundtrip Fixture"
328+
);
329+
assert_eq!(described.data["image_bytes"], true);
330+
assert!(described.data["screenshot_meta"]["screenshot_id"]
331+
.as_str()
332+
.is_some());
269333
let control = host.control_snapshot();
270334
assert_eq!(control.state, "active");
271335
assert!(control.sharing.is_none());

‎src/apps/desktop/src/computer_use/desktop_host/pointer_input.rs‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -435,16 +435,35 @@ fn flash_click_highlight_cg(gx: f64, gy: f64) {
435435
impl DesktopComputerUseHost {
436436
#[cfg(target_os = "windows")]
437437
pub(super) async fn windows_target(&self, app: &AppSelector) -> OpenBitFunResult<(i32, isize)> {
438+
app.validate_window_target()
439+
.map_err(OpenBitFunError::tool)?;
438440
let pid = super::resolve_pid(self, app).await?;
439441
// A sharing indicator or newly opened utility window must not replace
440442
// the window already owned by the control session.
441443
let prefix = format!("pid:{pid}/window:");
442-
if let Some(window) = crate::computer_use::control_session::snapshot()
444+
let control = crate::computer_use::control_session::snapshot();
445+
let bound = control
443446
.target
444447
.as_deref()
445448
.and_then(|target| target.strip_prefix(&prefix))
446-
.and_then(|window| window.parse::<isize>().ok())
449+
.and_then(|window| window.parse::<isize>().ok());
450+
let requested = app
451+
.window_id
452+
.map(isize::try_from)
453+
.transpose()
454+
.map_err(|_| {
455+
OpenBitFunError::tool(
456+
"[INVALID_PARAMS] window_id exceeds the native window handle range",
457+
)
458+
})?;
459+
if control.sharing.is_some()
460+
&& (bound.is_none() || requested.is_some_and(|id| Some(id) != bound))
447461
{
462+
return Err(OpenBitFunError::tool(
463+
"[SHARED_TARGET_ONLY] Window was not shared by the user",
464+
));
465+
}
466+
if let Some(window) = requested.or(bound) {
448467
use windows::Win32::Foundation::HWND;
449468
use windows::Win32::UI::WindowsAndMessaging::{GetWindowThreadProcessId, IsWindow};
450469
let hwnd = HWND(window as *mut std::ffi::c_void);
@@ -453,7 +472,7 @@ impl DesktopComputerUseHost {
453472
GetWindowThreadProcessId(hwnd, Some(&mut actual_pid));
454473
}
455474
if !unsafe { IsWindow(Some(hwnd)) }.as_bool() || actual_pid != pid as u32 {
456-
return Err(OpenBitFunError::tool("[TARGET_WINDOW_UNAVAILABLE] The bound window closed; select a new target explicitly"));
475+
return Err(OpenBitFunError::tool("[TARGET_WINDOW_UNAVAILABLE] The requested window no longer belongs to this process; refresh list_windows and select the intended window"));
457476
}
458477
return Ok((pid, window));
459478
}

‎src/apps/desktop/src/computer_use/macos_capture.rs‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,7 @@ pub(super) fn ensure_capture(pid: i32, mut window_id: Option<u32>) -> Result<(),
191191
"CAPTURE_GENERATION_CHANGED: Previous capture cleanup is still pending".into(),
192192
);
193193
}
194-
if session.pid == pid {
195-
validate_bound_window(session.window_id, window_id)?;
194+
if session.pid == pid && reuse_bound_window(session.window_id, window_id) {
196195
return session.check();
197196
}
198197
// A bad target request must not destroy a healthy stream. Resolve the
@@ -224,12 +223,10 @@ pub(super) fn ensure_capture(pid: i32, mut window_id: Option<u32>) -> Result<(),
224223
Ok(())
225224
}
226225
// The sharing indicator itself may be reported as this application's front
227-
// window. Once selected, a window stays bound until the session is stopped.
228-
fn validate_bound_window(bound: u32, requested: Option<u32>) -> Result<(), String> {
229-
if requested.is_some_and(|id| id != bound) {
230-
return Err("TARGET_WINDOW_CHANGED: Existing capture remains active; use the bound window or start a new control session to select another window".into());
231-
}
232-
Ok(())
226+
// window. Omitted selection keeps the binding; only an explicit window ID may
227+
// request a replacement, validated above before the old stream is released.
228+
fn reuse_bound_window(bound: u32, requested: Option<u32>) -> bool {
229+
requested.is_none_or(|id| id == bound)
233230
}
234231

235232
pub(super) fn bound_window_id(pid: i32) -> Result<u32, String> {
@@ -340,14 +337,12 @@ extern "C" fn obf_control_native_stopped(
340337

341338
#[cfg(test)]
342339
mod tests {
343-
use super::validate_bound_window;
340+
use super::reuse_bound_window;
344341

345342
#[test]
346343
fn sharing_badge_cannot_retarget_an_existing_window_capture() {
347-
assert!(validate_bound_window(100, None).is_ok());
348-
assert!(validate_bound_window(100, Some(100)).is_ok());
349-
assert!(validate_bound_window(100, Some(101))
350-
.unwrap_err()
351-
.starts_with("TARGET_WINDOW_CHANGED:"));
344+
assert!(reuse_bound_window(100, None));
345+
assert!(reuse_bound_window(100, Some(100)));
346+
assert!(!reuse_bound_window(100, Some(101)));
352347
}
353348
}

0 commit comments

Comments
 (0)