From 4703d7cd5af294a48b05ff388f9b874c6b41dec8 Mon Sep 17 00:00:00 2001 From: Adam Crocker <77861255+captcrock@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:21:34 -0500 Subject: [PATCH] TD-5231: agent-desktop's cursor works from the packaged app Two bugs from the TD-5227 live check against the packaged build: - Two sessions whose first element command ran at the same moment each started an agent-desktop session; one session's refs then resolved in the other's namespace (SNAPSHOT_NOT_FOUND). Start, turn-off and end of the cursor session now run under one asyncio lock; commands themselves still run in parallel. - The agent-desktop copy bundled inside TST Desk reports "TST Desk" as the owner of its cursor windows, so captures that left out the owner name "agent-desktop" kept them. The host and the Python fallback now also match the owning process's executable (proc_pidpath), which keeps TST Desk's own windows. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HUthoCdmzg2hy89ssk2cSF --- DECISIONS.md | 13 ++++ core/tests/test_cu_agent_cursors.py | 24 +++++++ core/tstd/desktop/ax_cursor.py | 19 +++++- docs/tst-desk-backlog.md | 19 ++++++ .../src/tst_cu_mcp/backends/darwin_capture.py | 46 ++++++++++++-- .../tests/test_darwin_capture_exclude.py | 32 ++++++++++ shell/src/cu_capture.rs | 62 ++++++++++++++++++- 7 files changed, 208 insertions(+), 7 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index c1ce74f9..e86b1844 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -19826,3 +19826,16 @@ helper answers (the rules controller assumes a lit ring is lit, and would need a replay path for nothing the probe does not already give); starting the helper only when the overlay is first needed (that is the path that failed). + +## 2026-11-03 — TD-5231: agent-desktop's windows are found by executable, and its session start is serialized (Class A; amends TD-5227 item 8) + +**Decision:** TST Desk's capture treats a window as agent-desktop's when its +owner name is `agent-desktop` or its owning process (`kCGWindowOwnerPID`, +resolved with `proc_pidpath`) runs an executable named `agent-desktop`. +The cursor session's start, turn-off and end run under one asyncio lock. + +**Rationale:** the copy bundled inside TST Desk (TD-5228) reports the app's +name as the owner of its windows, so a name check missed exactly the copy +TST Desk ships. The executable check must not catch TST Desk's own windows, +which share that owner name. The lock covers set-up only, never the command, +so concurrent sessions still act in parallel. diff --git a/core/tests/test_cu_agent_cursors.py b/core/tests/test_cu_agent_cursors.py index 1060b68c..c6f42f8f 100644 --- a/core/tests/test_cu_agent_cursors.py +++ b/core/tests/test_cu_agent_cursors.py @@ -234,6 +234,30 @@ async def test_stop_turns_cursors_off_now_and_shutdown_ends_the_session(self) -> ["session", "end", "run-1"], ] + async def test_two_first_commands_at_once_start_one_session(self) -> None: + """TD-5231: two sessions acting at once must share one namespace, or + a ref from one session's snapshot is looked up in another.""" + + class SlowStart(FakeCli): + async def run_with(self, prefix: list[str], command: str, *args: str) -> Any: + if command == "session": + await asyncio.sleep(0.05) + return await super().run_with(prefix, command, *args) + + cli, cursor = SlowStart(), self._cursor([True]) + + async def first(sid: str) -> list[str]: + token = current_cu_agent.set(CuAgent(sid)) + try: + return await cursor.scope(cli) + finally: + current_cu_agent.reset(token) + + a, b = await asyncio.gather(first("aaaa"), first("bbbb")) + starts = [argv for argv in cli.argv if argv[:2] == ["session", "start"]] + assert len(starts) == 1 + assert a[:2] == b[:2] == ["--session", "run-1"] + async def test_a_failed_start_runs_without_a_cursor_and_does_not_retry(self) -> None: cli, cursor = FakeCli(start_fails=True), self._cursor([True]) assert await cursor.scope(cli) == [] diff --git a/core/tstd/desktop/ax_cursor.py b/core/tstd/desktop/ax_cursor.py index c0aa4927..62fe663e 100644 --- a/core/tstd/desktop/ax_cursor.py +++ b/core/tstd/desktop/ax_cursor.py @@ -26,6 +26,7 @@ from __future__ import annotations +import asyncio from collections.abc import Awaitable, Callable from typing import Any, Protocol @@ -57,6 +58,10 @@ def __init__( self._enabled: set[str] = set() self._overlay_on = False self._failed = False + # Sessions act concurrently: two first commands at once must start + # one agent-desktop session, not two (TD-5231). Only the set-up is + # held, never the command itself. + self._lock = asyncio.Lock() @property def session_id(self) -> str | None: @@ -64,6 +69,10 @@ def session_id(self) -> str | None: async def scope(self, cli: ScopedRunner) -> list[str]: """Global flags for the next command; turns the cursor on or off first.""" + async with self._lock: + return await self._scope(cli) + + async def _scope(self, cli: ScopedRunner) -> list[str]: agent = current_cu_agent.get() or DEFAULT_AGENT wanted = self._wanted() if self._session is None: @@ -74,7 +83,7 @@ async def scope(self, cli: ScopedRunner) -> list[str]: return [] prefix = ["--session", self._session, "--agent-id", agent.agent_id] if not wanted: - await self.off(cli) + await self._off(cli) return prefix try: if not self._overlay_on: @@ -95,6 +104,10 @@ async def scope(self, cli: ScopedRunner) -> list[str]: async def off(self, cli: ScopedRunner) -> None: """Remove every agent's cursor (Stop, or a switch turned off).""" + async with self._lock: + await self._off(cli) + + async def _off(self, cli: ScopedRunner) -> None: if self._session is None or not self._overlay_on: return try: @@ -106,6 +119,10 @@ async def off(self, cli: ScopedRunner) -> None: async def end(self, cli: ScopedRunner) -> None: """End the session at shutdown; its cursors go with it.""" + async with self._lock: + await self._end(cli) + + async def _end(self, cli: ScopedRunner) -> None: session, self._session = self._session, None self._overlay_on = False self._enabled.clear() diff --git a/docs/tst-desk-backlog.md b/docs/tst-desk-backlog.md index a5ec5c09..ed4147b5 100644 --- a/docs/tst-desk-backlog.md +++ b/docs/tst-desk-backlog.md @@ -13042,6 +13042,25 @@ step 1 and the Mac app since TD-5228; on Windows the binary comes from --- +### TD-5231 — agent-desktop's cursor works from the packaged app +**Size:** 2 · **Depends on:** TD-5227, TD-5228 +**Status:** In review 2026-11-03. + +Two bugs found in the TD-5227 live check against the packaged build: + +1. Two sessions whose first element command ran at the same moment each + started their own agent-desktop session. One session's snapshot refs then + resolved in the other's namespace, and its action failed with + `SNAPSHOT_NOT_FOUND`. +2. The agent-desktop copy bundled inside TST Desk reports the app's name, + "TST Desk", as the owner of its cursor windows. TST Desk's screenshots + left out windows by the owner name `agent-desktop`, so the bundled copy's + cursors stayed in every screenshot. + +**Acceptance criteria:** +- [x] Starting, turning off and ending agent-desktop's cursor session is serialized: two first commands at once start one session (`test_two_first_commands_at_once_start_one_session`, fails without the lock) +- [x] A capture leaves out a window whose owning process runs the `agent-desktop` executable, whatever owner name it reports, and keeps TST Desk's own windows (host `cu_capture.rs` and the Python fallback; unit tests on both) +- [x] Live with the packaged binary: two sessions' clicks land in one agent-desktop session; while the bundled copy's cursor is on screen, the capture drops exactly its window (reported owner "TST Desk", its own pid) and keeps the TST Desk window ### TD-5230 — The packaged app's ring and cursors survive the helper's slow start **Size:** 1 · **Depends on:** TD-4834, TD-5226, TD-5227 **Status:** In review 2026-11-03. diff --git a/mcp/tst-cu-mcp/src/tst_cu_mcp/backends/darwin_capture.py b/mcp/tst-cu-mcp/src/tst_cu_mcp/backends/darwin_capture.py index ad7b677f..42d50740 100644 --- a/mcp/tst-cu-mcp/src/tst_cu_mcp/backends/darwin_capture.py +++ b/mcp/tst-cu-mcp/src/tst_cu_mcp/backends/darwin_capture.py @@ -6,11 +6,18 @@ screen, the image is composed from the on-screen window list without them; otherwise the plain capture runs unchanged. Mirrors the host's ``cu_capture.rs``. Function-local pyobjc so the module imports anywhere. + +A window is agent-desktop's when its owner name is ``agent-desktop`` or its +owning process runs an executable of that name: the copy bundled inside an +app (TST Desk ships one) reports the app's name as the owner, so the name +alone misses it (TD-5231). """ from __future__ import annotations -from collections.abc import Iterable +import ctypes +import os +from collections.abc import Callable, Iterable from typing import Any EXCLUDED_OWNER = "agent-desktop" @@ -72,18 +79,47 @@ def _kept_on_screen() -> list[int] | None: return kept if len(kept) < len(windows) else None -def _owners(ids: Iterable[Any], described: Iterable[Any]) -> list[tuple[int, str]]: - """``(id, owner name)`` for each id, in the ids' order. A window with no - description (it closed meanwhile) gets an empty owner, so it is kept.""" +def process_name(pid: int) -> str: + """The file name of *pid*'s executable, or "" when it cannot be read.""" + try: + libproc = ctypes.CDLL("libproc.dylib") + buffer = ctypes.create_string_buffer(4096) + length = libproc.proc_pidpath(ctypes.c_int(pid), buffer, ctypes.c_uint32(4096)) + except (OSError, AttributeError): + return "" + if length <= 0: + return "" + return os.path.basename(buffer.value.decode("utf-8", "replace")) + + +def _owners( + ids: Iterable[Any], + described: Iterable[Any], + process_name: Callable[[int], str] = process_name, +) -> list[tuple[int, str]]: + """``(id, owner)`` for each id, in the ids' order; the owner reads + ``agent-desktop`` when the owning process runs that executable. A window + with no description (it closed meanwhile) gets an empty owner, so it is + kept.""" names: dict[int, str] = {} + executables: dict[int, str] = {} for entry in described: getter = getattr(entry, "get", None) if getter is None: continue try: - names[int(getter("kCGWindowNumber"))] = str(getter("kCGWindowOwnerName") or "") + window_id = int(getter("kCGWindowNumber")) + owner = str(getter("kCGWindowOwnerName") or "") + raw_pid = getter("kCGWindowOwnerPID") + pid = int(raw_pid) if raw_pid is not None else None except (TypeError, ValueError): continue + if pid is not None and pid > 0: + if pid not in executables: + executables[pid] = process_name(pid) + if executables[pid] == EXCLUDED_OWNER: + owner = EXCLUDED_OWNER + names[window_id] = owner windows: list[tuple[int, str]] = [] for raw in ids: try: diff --git a/mcp/tst-cu-mcp/tests/test_darwin_capture_exclude.py b/mcp/tst-cu-mcp/tests/test_darwin_capture_exclude.py index d2e8f603..3bf01fa5 100644 --- a/mcp/tst-cu-mcp/tests/test_darwin_capture_exclude.py +++ b/mcp/tst-cu-mcp/tests/test_darwin_capture_exclude.py @@ -147,3 +147,35 @@ def png(image: Any) -> bytes: assert darwin._cg_capture_png((0, 0, 800, 600)) == b"\x89PNG fake" assert encoded == [FROM_ARRAY] assert fake.calls == [("from_array", [3, 40, 12, 2])] + + +def test_a_bundled_copy_is_found_by_its_executable_not_its_owner_name() -> None: + """TD-5231: inside an app bundle agent-desktop's windows report the app's + name as their owner. The app's own window, same owner name, stays.""" + from tst_cu_mcp.backends.darwin_capture import _owners + + described = [ + {"kCGWindowNumber": 7, "kCGWindowOwnerName": "TST Desk", "kCGWindowOwnerPID": 501}, + {"kCGWindowNumber": 8, "kCGWindowOwnerName": "TST Desk", "kCGWindowOwnerPID": 777}, + {"kCGWindowNumber": 9, "kCGWindowOwnerName": "Finder", "kCGWindowOwnerPID": 300}, + ] + names = {501: "tst-desk", 777: "agent-desktop", 300: "Finder"} + asked: list[int] = [] + + def process_name(pid: int) -> str: + asked.append(pid) + return names[pid] + + windows = _owners([7, 8, 9], described, process_name) + assert kept_window_ids(windows) == [7, 9] + assert sorted(asked) == [300, 501, 777] + + +@pytest.mark.skipif(sys.platform != "darwin", reason="libproc is macOS") +def test_process_name_reads_this_process() -> None: + import os + + from tst_cu_mcp.backends.darwin_capture import process_name + + assert process_name(os.getpid()) == os.path.basename(os.path.realpath(sys.executable)) + assert process_name(-1) == "" diff --git a/shell/src/cu_capture.rs b/shell/src/cu_capture.rs index 8d517c90..cd561db9 100644 --- a/shell/src/cu_capture.rs +++ b/shell/src/cu_capture.rs @@ -5,6 +5,11 @@ //! agent's cursor in another agent's screenshot. When such a window is on //! screen, the image is composed from the on-screen window list without //! them; otherwise the plain capture runs unchanged. +//! +//! A window is agent-desktop's when its owner name is `agent-desktop` or its +//! owning process runs an executable of that name. The copy bundled inside +//! TST Desk reports the app's name as its owner, so the name alone misses it +//! (TD-5231); the executable check must not catch TST Desk's own windows. /// The window ids a capture draws: every window in `windows` (id and owner /// name, front to back) except agent-desktop's, in the same order. @@ -72,8 +77,17 @@ mod macos { ) -> *mut c_void; static kCGWindowNumber: *const c_void; static kCGWindowOwnerName: *const c_void; + static kCGWindowOwnerPID: *const c_void; + } + + // libproc, part of libSystem. + extern "C" { + fn proc_pidpath(pid: i32, buffer: *mut c_void, buffersize: u32) -> i32; } + const PROC_PIDPATHINFO_MAXSIZE: usize = 4096; + const EXCLUDED_EXECUTABLE: &str = "agent-desktop"; + #[link(name = "CoreFoundation", kind = "framework")] extern "C" { fn CFGetTypeID(cf: *const c_void) -> usize; @@ -139,6 +153,7 @@ mod macos { /// gets an empty owner, so it is kept. unsafe fn owners(ids: *const c_void) -> Vec<(u32, String)> { let mut names: HashMap = HashMap::new(); + let mut executables: HashMap = HashMap::new(); if let Some(described) = Owned::adopt(CGWindowListCreateDescriptionFromArray(ids)) { for entry in cu_cf::items(described.as_ptr()) { let entry = entry.as_ptr(); @@ -148,7 +163,18 @@ mod macos { let number = cu_cf::number_value(CFDictionaryGetValue(entry, kCGWindowNumber)); if let Some(id) = number.and_then(window_id) { let owner = CFDictionaryGetValue(entry, kCGWindowOwnerName); - names.insert(id, cu_cf::text(owner)); + let mut name = cu_cf::text(owner); + let pid = cu_cf::number_value(CFDictionaryGetValue(entry, kCGWindowOwnerPID)) + .and_then(owner_pid); + if let Some(pid) = pid { + let bundled = *executables.entry(pid).or_insert_with(|| { + executable_name(pid).as_deref() == Some(EXCLUDED_EXECUTABLE) + }); + if bundled { + name = EXCLUDED_EXECUTABLE.to_string(); + } + } + names.insert(id, name); } } } @@ -182,6 +208,29 @@ mod macos { )) } + fn owner_pid(number: f64) -> Option { + let whole = number.fract() == 0.0 && (1.0..=f64::from(i32::MAX)).contains(&number); + whole.then_some(number as i32) + } + + /// The file name of `pid`'s executable, or `None` when it cannot be read. + fn executable_name(pid: i32) -> Option { + let mut buffer = vec![0u8; PROC_PIDPATHINFO_MAXSIZE]; + // SAFETY: the buffer outlives the call and its size is what we pass. + let written = unsafe { + proc_pidpath( + pid, + buffer.as_mut_ptr().cast(), + PROC_PIDPATHINFO_MAXSIZE as u32, + ) + }; + let length = usize::try_from(written).ok().filter(|&n| n > 0)?; + let path = String::from_utf8_lossy(&buffer[..length.min(buffer.len())]).into_owned(); + std::path::Path::new(&path) + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + } + fn window_id(number: f64) -> Option { let whole = number.fract() == 0.0 && (0.0..=f64::from(u32::MAX)).contains(&number); whole.then_some(number as u32) @@ -191,6 +240,17 @@ mod macos { mod tests { use super::*; + #[test] + fn executable_name_reads_this_process_and_refuses_nonsense() { + let own = std::env::current_exe().unwrap(); + let expected = own.file_name().unwrap().to_string_lossy().into_owned(); + assert_eq!(executable_name(std::process::id() as i32), Some(expected)); + assert_eq!(executable_name(-1), None); + assert_eq!(owner_pid(0.0), None); + assert_eq!(owner_pid(42.5), None); + assert_eq!(owner_pid(501.0), Some(501)); + } + #[test] fn window_ids_survive_the_pointer_sized_array() { let ids = [3, 10_238, u32::MAX];