From 01b21dbc8e4e8b3885891c0e0980ff0667ef2f1c Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 29 Sep 2026 12:32:45 -0500 Subject: [PATCH] fix(terminal): report the open outcome from the final observed frontmost state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ladder now ends by observing frontmost after the settle and re-assert: the tool output says whether the opened document actually ended up in front, and when another application holds focus it names that application — a focus race can no longer read as plain success (#95261 requirement 2). The not-front branch gets one extra activate+0.4s so the report reads the state after a last raise attempt, not before it. --- tests/tools/test_terminal_macos_open.py | 29 +++++++++++++++++++++++++ tools/terminal_tool_macos_open.py | 20 ++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_terminal_macos_open.py b/tests/tools/test_terminal_macos_open.py index 2add36aeea..09f2823b7a 100644 --- a/tests/tools/test_terminal_macos_open.py +++ b/tests/tools/test_terminal_macos_open.py @@ -131,3 +131,32 @@ def test_compound_command_not_transformed(): def test_none_input_returns_none(): assert macos_open._transform_macos_open_command(None, system="Darwin") is None + + +# --- Outcome reporting: distinct open-but-not-front outcome ---------------- + +def test_ladder_reports_distinct_outcomes_from_final_state(): + transformed = _assert_transformed("open -a Preview /path/to/file.pdf") + # Success and open-but-not-front are two distinct, observable outcomes + # read from the FINAL frontmost state — a focus race can never read as + # plain success (issue #95261 requirement 2). + assert "is now frontmost" in transformed + assert "opened but another application holds focus" in transformed + # The not-front branch names whichever application actually holds focus. + assert "name of first application process whose frontmost is true" in transformed + +def test_ladder_is_valid_shell_syntax_for_quoting_edge_cases(): + import subprocess + + for command in ( + "open /path/to/file.pdf", + "open '/Users/some one/file name.pdf'", + 'open -a "Google Chrome" "/path/to/file.pdf"', + "open -a Preview file.txt", + ): + transformed = macos_open._transform_macos_open_command(command, system="Darwin") + assert transformed is not None and transformed != command, command + result = subprocess.run( + ["bash", "-n", "-c", transformed], capture_output=True, text=True + ) + assert result.returncode == 0, (command, result.stderr) diff --git a/tools/terminal_tool_macos_open.py b/tools/terminal_tool_macos_open.py index 1eaa112f73..18625f0b62 100644 --- a/tools/terminal_tool_macos_open.py +++ b/tools/terminal_tool_macos_open.py @@ -96,6 +96,23 @@ def _build_macos_open_raise_ladder(app: str | None, file: str) -> str: # by macOS as pure activation. reopen = f"open -a \"$_H_APP\" {file_q} 2>/dev/null" + # Outcome reporting from the FINAL observed frontmost state. The two + # branches are deliberately distinct so a focus race can never read as + # success: the tool output tells the agent (and the user) whether the + # opened document actually ended up in front, and if not, which + # application holds focus. ``file_q`` is a self-contained quoted token, so + # the messages survive paths with spaces or shell metacharacters. + frontmost_name = ( + "osascript -e 'tell application \"System Events\" to get name of " + "first application process whose frontmost is true' 2>/dev/null" + ) + report = ( + f'if {frontmost}; then echo open: {file_q} is now frontmost; ' + f'else _H_FOCUS=$({frontmost_name}); ' + f'echo open: {file_q} opened but another application holds focus; ' + f'echo "frontmost: $_H_FOCUS"; fi' + ) + return ( f"{app_assign}; " f"if ! {frontmost}; then " @@ -108,7 +125,8 @@ def _build_macos_open_raise_ladder(app: str | None, file: str) -> str: f"fi; " f"fi; " f"sleep 0.8; " - f"if ! {frontmost}; then {activate}; fi" + f"if ! {frontmost}; then {activate}; sleep 0.4; fi; " + f"{report}" )