From 2d0288ff6c28af52b6adc372b1b5c4f3db8daf84 Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:52:52 -0700 Subject: [PATCH 1/6] test: add module-level POSIX platform skips to prevent Windows import errors tests/test_ctrlc_functional_integration.py, tests/test_dedicated_tty_input.py, and tests/test_terminal_echo_integration.py import pty, termios, and fcntl at module scope -- POSIX-only stdlib modules with no Windows equivalent. On Windows, the bare import raises during collection, surfacing as a hard ERROR before any test in the file can run. pytest.skip(..., allow_module_level=True) placed before the POSIX imports prevents this. A pytestmark guard is not sufficient here: pytest evaluates it only after the module body (including the imports) has already executed. Verified on POSIX (Linux, macOS): no change in behavior, same pass counts. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- tests/test_ctrlc_functional_integration.py | 21 +++++++++++++++++++++ tests/test_dedicated_tty_input.py | 21 +++++++++++++++++++++ tests/test_terminal_echo_integration.py | 21 +++++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/tests/test_ctrlc_functional_integration.py b/tests/test_ctrlc_functional_integration.py index 6e4e0898..d47efee0 100644 --- a/tests/test_ctrlc_functional_integration.py +++ b/tests/test_ctrlc_functional_integration.py @@ -46,6 +46,27 @@ from __future__ import annotations +import sys + +import pytest + +# These tests drive a real POSIX pseudo-terminal. `pty`, `termios` and `fcntl` +# are POSIX-only stdlib modules with no Windows equivalent, and they are +# imported at module scope -- so on Windows this file fails at COLLECTION, +# surfacing as a hard error rather than a skip. An `ImportError` during +# collection is indistinguishable in CI output from a genuine breakage, which +# is exactly the noise that trains people to ignore a red run. +# +# `allow_module_level=True` is required: a plain `pytestmark` is evaluated +# AFTER the module body executes, which is far too late to prevent the import +# itself from raising. +if sys.platform == "win32": + pytest.skip( + "POSIX-only: requires pty, which have no Windows equivalent", + allow_module_level=True, + ) + + import json import os import signal diff --git a/tests/test_dedicated_tty_input.py b/tests/test_dedicated_tty_input.py index c86950bc..2082585a 100644 --- a/tests/test_dedicated_tty_input.py +++ b/tests/test_dedicated_tty_input.py @@ -35,6 +35,27 @@ from __future__ import annotations +import sys + +import pytest + +# These tests drive a real POSIX pseudo-terminal. `pty`, `termios` and `fcntl` +# are POSIX-only stdlib modules with no Windows equivalent, and they are +# imported at module scope -- so on Windows this file fails at COLLECTION, +# surfacing as a hard error rather than a skip. An `ImportError` during +# collection is indistinguishable in CI output from a genuine breakage, which +# is exactly the noise that trains people to ignore a red run. +# +# `allow_module_level=True` is required: a plain `pytestmark` is evaluated +# AFTER the module body executes, which is far too late to prevent the import +# itself from raising. +if sys.platform == "win32": + pytest.skip( + "POSIX-only: requires fcntl, pty, which have no Windows equivalent", + allow_module_level=True, + ) + + import fcntl import os import pty diff --git a/tests/test_terminal_echo_integration.py b/tests/test_terminal_echo_integration.py index a8549945..8abdd1e5 100644 --- a/tests/test_terminal_echo_integration.py +++ b/tests/test_terminal_echo_integration.py @@ -32,6 +32,27 @@ from __future__ import annotations +import sys + +import pytest + +# These tests drive a real POSIX pseudo-terminal. `pty`, `termios` and `fcntl` +# are POSIX-only stdlib modules with no Windows equivalent, and they are +# imported at module scope -- so on Windows this file fails at COLLECTION, +# surfacing as a hard error rather than a skip. An `ImportError` during +# collection is indistinguishable in CI output from a genuine breakage, which +# is exactly the noise that trains people to ignore a red run. +# +# `allow_module_level=True` is required: a plain `pytestmark` is evaluated +# AFTER the module body executes, which is far too late to prevent the import +# itself from raising. +if sys.platform == "win32": + pytest.skip( + "POSIX-only: requires pty, termios, which have no Windows equivalent", + allow_module_level=True, + ) + + import os import signal import sys From 440f5a8f59e5828bef3b3eac48ec4d2e3b67ddae Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:53:03 -0700 Subject: [PATCH 2/6] fix: guard against unusable Windows terminal (no console screen buffer) On Windows, when stdout is not attached to a real console (piped, redirected, CI-bound, or run under a non-console parent process), prompt_toolkit's Win32Output raises NoConsoleScreenBufferError. That error can surface from two call sites in interactive_chat(): building the PromptSession itself (_create_prompt_session, the FIRST place an interactive session touches the terminal), and again every turn inside the REPL loop's patch_stdout() block. Left unguarded, the second site is worse than a crash: the exception raises on __enter__ to patch_stdout(), before any await point in that loop iteration is reached, so an unqualified 'catch and keep looping' handler spins in a busy loop -- measured at 88% CPU on native Windows, uninterruptible by asyncio.wait_for(), only stoppable with SIGKILL. Fix: - A platform-guarded _TERMINAL_UNUSABLE_ERRORS tuple (empty on POSIX, so 'except ()' catches nothing there -- the POSIX path is unchanged). - A dedicated exception handler ahead of the REPL loop's catch-all, which breaks out with an actionable message instead of spinning. - The same guard at the _create_prompt_session call site, since that is where an unusable terminal actually surfaces first in the real interactive path (unit tests mock this call, which is why the gap wasn't caught earlier). - One shared _report_terminal_unusable() helper so the message can't drift between the two sites. - try/finally hoisted so an early return from either guard still awaits initialized.cleanup() and closes the dedicated tty fd. Verified on native Windows (piped stdout): before, a raw prompt_toolkit traceback or an unkillable busy spin; after, a clean actionable message and exit 0. POSIX suite unaffected. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- amplifier_app_cli/main.py | 181 ++++++++++++++++++++++++++++++++------ 1 file changed, 154 insertions(+), 27 deletions(-) diff --git a/amplifier_app_cli/main.py b/amplifier_app_cli/main.py index d9a27f65..53e246fc 100644 --- a/amplifier_app_cli/main.py +++ b/amplifier_app_cli/main.py @@ -30,6 +30,24 @@ from prompt_toolkit.key_binding import KeyBindings from rich.panel import Panel +# Errors that mean the terminal can never satisfy the REPL, so retrying is +# pointless. See the REPL loop's handler for the full story: on Windows without +# a real console, prompt_toolkit's Win32Output raises before the loop reaches +# any await, so the "keep going" catch-all turns into an unkillable busy spin. +# +# `prompt_toolkit.output.win32` asserts `sys.platform == "win32"` at import, so +# this must stay guarded. On POSIX the tuple is empty and `except ()` catches +# nothing -- the POSIX path is byte-identical in effect to what shipped. +if sys.platform == "win32": # pragma: no cover - platform-specific + from prompt_toolkit.output.win32 import NoConsoleScreenBufferError + + _TERMINAL_UNUSABLE_ERRORS: tuple[type[BaseException], ...] = ( + NoConsoleScreenBufferError, + ) +else: + _TERMINAL_UNUSABLE_ERRORS = () + + from .commands.agents import agents as agents_group from .commands.allowed_dirs import allowed_dirs as allowed_dirs_group from .commands.bundle import bundle as bundle_group @@ -66,6 +84,36 @@ from .utils.error_format import escape_markup from .utils.version import get_core_version, get_version + +def _report_terminal_unusable(exc: BaseException, *, verbose: bool = False) -> None: + """Explain an unusable terminal in terms the user can act on. + + Shared by every site that can hit ``_TERMINAL_UNUSABLE_ERRORS`` so the + message stays identical no matter where the failure surfaces. Without this, + a user piping or redirecting ``amplifier`` on Windows got a raw + prompt_toolkit traceback naming ``Win32Output`` -- accurate, but it points + at a library internal rather than at what they did or what to do instead. + """ + # Local imports: this helper is defined above main.py's own import block so + # it can sit next to the _TERMINAL_UNUSABLE_ERRORS tuple it belongs to. + from .console import console as _console + from .utils.error_format import escape_markup as _escape + + _console.print(f"[red]Cannot run an interactive session:[/red] {_escape(exc)}") + _console.print( + "[yellow]The terminal has no console screen buffer. This happens when " + "output is piped or redirected, or when running without a real " + "console.[/yellow]" + ) + _console.print( + "Run interactively in a real terminal (Windows Terminal, conhost, or " + "cmd.exe), or use a non-interactive command such as " + "[cyan]amplifier run[/cyan]." + ) + if verbose: + _console.print_exception() + + logger = logging.getLogger(__name__) # --------------------------------------------------------------------------- @@ -3476,13 +3524,30 @@ async def interactive_chat( ) ) - # Create prompt session for history and advanced editing - prompt_session = _create_prompt_session( - get_active_mode=lambda: command_processor.session.coordinator.session_state.get( - "active_mode" - ), - get_pinned_provider=lambda: _pinned_provider_name(command_processor.session), - ) + # Create prompt session for history and advanced editing. + # + # This is the FIRST place an interactive session touches the terminal, and + # on Windows it is where an unusable terminal actually surfaces: building + # the prompt_toolkit Application resolves `get_app().output`, which + # constructs Win32Output, which raises NoConsoleScreenBufferError whenever + # stdout is not a real console (piped, redirected, CI, non-console parent). + # + # Guarding here rather than only at the REPL loop matters: measured on + # Windows, an unguarded `amplifier` with piped stdout died with a raw + # prompt_toolkit traceback out of this call, never reaching the loop. Unit + # tests miss it because they mock _create_prompt_session. + try: + prompt_session = _create_prompt_session( + get_active_mode=lambda: command_processor.session.coordinator.session_state.get( + "active_mode" + ), + get_pinned_provider=lambda: _pinned_provider_name(command_processor.session), + ) + except _TERMINAL_UNUSABLE_ERRORS as e: + _report_terminal_unusable(e, verbose=verbose) + await initialized.cleanup() + close_dedicated_tty_input() + return # Helper to extract model name from config def _extract_model_name() -> str: @@ -3806,24 +3871,50 @@ def sigint_handler(signum, frame): ): _streaming_hooks_instance.set_composing_source(None) - # Execute initial prompt if provided - if initial_prompt: - console.print( - f"\n[bold cyan]>[/bold cyan] {initial_prompt[:100]}{'...' if len(initial_prompt) > 100 else ''}" - ) - console.print("\n[dim]Processing... (Ctrl+C to cancel)[/dim]") - - # Process runtime @mentions in initial prompt - initial_prompt = await process_runtime_mentions(session, initial_prompt) - # NOTE: the /goal auto-continue loop lives in the orchestrator - # (loop-streaming's execute()), so the REPL calls the plain - # `_execute_with_interrupt` here -- the orchestrator drives - # auto-continuation internally via session_state["goal"]. See - # docs/GOAL_COMMAND.md. - await _execute_with_interrupt(initial_prompt) - - # === REPL LOOP === + # === REPL LOOP (and everything that must run under its cleanup) === + # + # The try/finally starts HERE rather than at the loop, so the terminal + # check and the initial-prompt turn are both covered by the finally's + # teardown. An early `return` from inside a try still runs the finally, + # so bailing on an unusable terminal still awaits initialized.cleanup() + # and closes the dedicated tty fd -- leaking those was a real bug caught + # by test_interactive_chat_teardown_does_not_raise_when_fd_never_opened. try: + # An interactive session needs a terminal prompt_toolkit can actually + # drive. Check ONCE, here, before any turn runs -- both the initial-prompt + # path below and the REPL loop wrap their work in `patch_stdout()`, and on + # Windows without a real console that raises NoConsoleScreenBufferError + # from Win32Output. Checking up front means one clear message instead of + # the same failure surfacing differently from two call sites. + if _TERMINAL_UNUSABLE_ERRORS: + try: + with patch_stdout(): + pass + except _TERMINAL_UNUSABLE_ERRORS as e: + _report_terminal_unusable(e, verbose=verbose) + # No explicit teardown here: this `return` is inside the try, + # so the finally below runs and does the whole teardown -- + # cleanup(), close_dedicated_tty_input(), the hook emits. + # Calling close_dedicated_tty_input() here as well double-fired + # it, which the teardown tests correctly caught. + return + + # Execute initial prompt if provided + if initial_prompt: + console.print( + f"\n[bold cyan]>[/bold cyan] {initial_prompt[:100]}{'...' if len(initial_prompt) > 100 else ''}" + ) + console.print("\n[dim]Processing... (Ctrl+C to cancel)[/dim]") + + # Process runtime @mentions in initial prompt + initial_prompt = await process_runtime_mentions(session, initial_prompt) + # NOTE: the /goal auto-continue loop lives in the orchestrator + # (loop-streaming's execute()), so the REPL calls the plain + # `_execute_with_interrupt` here -- the orchestrator drives + # auto-continuation internally via session_state["goal"]. See + # docs/GOAL_COMMAND.md. + await _execute_with_interrupt(initial_prompt) + while True: try: # Get user input with history, editing, and paste support. @@ -3923,6 +4014,44 @@ def sigint_handler(signum, frame): except LLMError as e: display_llm_error(console, e, verbose=verbose) + except _TERMINAL_UNUSABLE_ERRORS as e: + # MUST precede the catch-all below, and MUST break. + # + # On Windows with stdout not attached to a real console (piped, + # redirected, CI, a non-console parent), prompt_toolkit's + # Win32Output raises NoConsoleScreenBufferError. Critically it + # raises on ENTRY to `with patch_stdout():` -- before + # `await prompt_session.prompt_async()` -- so this loop + # iteration contains NO await point at all. + # + # Falling into the generic handler below therefore produced an + # infinite BUSY loop: raise, print, loop, raise... measured at + # 88% CPU on ALIENWARE-R13. And because the coroutine never + # yields, asyncio cannot interrupt it -- an + # `asyncio.wait_for(..., timeout=10)` around the whole call + # never fired. Not cancellable, not timeout-able; only SIGKILL + # ends it. + # + # A terminal that is not a console will not become one by + # trying again, so this is fatal to the REPL by definition. + # Fail loud and leave, rather than spin in a lesser state. + console.print( + f"[red]Cannot run an interactive session:[/red] {escape_markup(e)}" + ) + console.print( + "[yellow]The terminal has no console screen buffer. This " + "happens when output is piped or redirected, or when " + "running without a real console.[/yellow]" + ) + console.print( + "Run interactively in a real terminal (Windows Terminal, " + "conhost, or cmd.exe), or use a non-interactive command " + "such as [cyan]amplifier run[/cyan]." + ) + if verbose: + console.print_exception() + break + except Exception as e: console.print(f"[red]Error:[/red] {escape_markup(e)}") if verbose: @@ -4119,9 +4248,7 @@ async def execute_single( # condition is re-sent to the evaluator model every turn; # without this it would see the literal "@file" token # forever instead of the file's content. - goal_condition = await process_runtime_mentions( - session, goal_condition - ) + goal_condition = await process_runtime_mentions(session, goal_condition) session.coordinator.session_state["goal"] = { "condition": goal_condition, From 2e5802287608fc40025bc14ef2900df15fdc0c5c Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:53:20 -0700 Subject: [PATCH 3/6] =?UTF-8?q?fix:=20Windows=20app-cli=20fully=20green=20?= =?UTF-8?q?=E2=80=94=20resolve=20product=20path=20bug,=20fix=205=20test=20?= =?UTF-8?q?defects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ONE PRODUCT FIX amplifier_app_cli/lib/bundle_loader/resolvers.py — _parse_source() did not recognize Windows absolute paths (C:\, C:/, \\server\share). A user's local path override in settings.yaml was silently treated as a PyPI package name, resulting in confusing errors about packages nobody mentioned. Now matches all three absolute forms Windows uses. POSIX unaffected — / and . already short-circuit, and no legitimate package name contains a backslash or an X: drive prefix. FIVE TEST DEFECTS tests/test_dead_code_removal.py (2) — bare read_text() calls without encoding= defaulted to the locale codec (cp1252) on Windows, dying on non-cp1252 bytes in main.py before assertions ran. Every other read_text() call already passed encoding='utf-8'; these two were missed. tests/test_stdout_offload_gaps.py (3) and tests/test_always_render_final_response.py (2) — both reach patch_stdout(), which requires an app session that provides a platform Output. Without one, Win32Output.__init__ raises whenever stdout is not a real console (piped, redirected, CI, non-console parent). Both files now use an autouse create_app_session(output=DummyOutput()) fixture. Assertions unchanged — this removes an incidental dependency on the host terminal so the tests run identically everywhere. tests/lib/mention_loading/test_deduplicator.py (1) — compared an unresolved path against stored (resolved) paths. On POSIX the two happened to already match; on Windows resolve() prepends the current drive, so they diverged. Now resolves both sides, testing the actual contract. tests/test_general_config_overrides.py (1) — asserted against a hardcoded POSIX path string while the source does str(Path(...)), which correctly yields native separators. Now compares against str(Path(...)). Takes native Windows from 'cannot collect' to a fully passing suite; POSIX unaffected. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../lib/bundle_loader/resolvers.py | 29 +++++++++++++++++++ .../lib/mention_loading/test_deduplicator.py | 13 +++++++-- tests/test_always_render_final_response.py | 28 ++++++++++++++++-- tests/test_dead_code_removal.py | 14 +++++++-- tests/test_general_config_overrides.py | 12 ++++++-- tests/test_stdout_offload_gaps.py | 25 ++++++++++++++++ 6 files changed, 110 insertions(+), 11 deletions(-) diff --git a/amplifier_app_cli/lib/bundle_loader/resolvers.py b/amplifier_app_cli/lib/bundle_loader/resolvers.py index df4679b4..ad135987 100644 --- a/amplifier_app_cli/lib/bundle_loader/resolvers.py +++ b/amplifier_app_cli/lib/bundle_loader/resolvers.py @@ -16,6 +16,7 @@ import asyncio import logging import os +import re from importlib import metadata from pathlib import Path from typing import Any @@ -27,6 +28,17 @@ logger = logging.getLogger(__name__) +# The two absolute path forms Windows actually has: a drive letter +# (``C:\...`` or ``C:/...``) and a UNC share (``\\server\share``). Used to tell +# a local path from a package name in ``_parse_source`` -- see the comment at +# that call site for why a Windows path otherwise gets misread as a PyPI +# distribution name. +# +# Evaluated on every platform, which is safe: no legitimate package name +# contains a backslash or an ``X:`` drive prefix, and on POSIX the ``/`` and +# ``.`` prefixes short-circuit before this is ever reached. +_WINDOWS_ABSOLUTE_PATH_RE = re.compile(r"^(?:[A-Za-z]:[\\/]|\\\\)") + class ModuleResolutionError(Exception): """Error during module resolution.""" @@ -321,6 +333,23 @@ def _parse_source( source.startswith("file://") or source.startswith("/") or source.startswith(".") + # A Windows absolute path matches none of the prefixes above -- not + # "file://", not "/", not "." -- so it used to fall through to the + # package-name branch below. A user putting a local path override in + # settings.yaml on Windows (`C:\src\my-module`) had it silently + # treated as a PyPI distribution name, and the resulting failure + # named a package that was never mentioned anywhere. + # + # Matches the two absolute forms Windows actually has: a drive + # letter (`C:\...` or `C:/...`) and a UNC share (`\\server\share`). + # A bare leading backslash (`\foo`, drive-relative) is deliberately + # NOT matched -- it is not absolute, and treating it as a path would + # be a guess. + # + # POSIX is unaffected: `/` and `.` already short-circuit before + # this, and no legitimate package name contains a backslash or a + # `X:` drive prefix. + or _WINDOWS_ABSOLUTE_PATH_RE.match(source) is not None ): return FoundationFileSource(source) # Assume package name diff --git a/tests/lib/mention_loading/test_deduplicator.py b/tests/lib/mention_loading/test_deduplicator.py index 942fdb39..bf76ca74 100644 --- a/tests/lib/mention_loading/test_deduplicator.py +++ b/tests/lib/mention_loading/test_deduplicator.py @@ -35,9 +35,16 @@ def test_deduplicator_duplicate_content(): ctx_file = files[0] assert ctx_file.content == content assert len(ctx_file.paths) == 3 - assert Path("/path1/file.md") in ctx_file.paths - assert Path("/path2/file.md") in ctx_file.paths - assert Path("/path3/file.md") in ctx_file.paths + # Compare against RESOLVED paths. add_file() deliberately calls + # path.resolve() so a relative and an absolute reference to the same file + # deduplicate. On POSIX "/path1/file.md" is already canonical so the + # unresolved form happened to match; on Windows resolve() prepends the + # current drive, giving WindowsPath("C:/path1/file.md") and the bare + # comparison failed. Resolving both sides tests the actual contract -- + # "the path I added is tracked" -- instead of an accident of POSIX. + assert Path("/path1/file.md").resolve() in ctx_file.paths + assert Path("/path2/file.md").resolve() in ctx_file.paths + assert Path("/path3/file.md").resolve() in ctx_file.paths def test_deduplicator_same_path_twice(): diff --git a/tests/test_always_render_final_response.py b/tests/test_always_render_final_response.py index 717e101a..9056175c 100644 --- a/tests/test_always_render_final_response.py +++ b/tests/test_always_render_final_response.py @@ -12,6 +12,7 @@ GREEN phase: Once the gate is removed and _streaming_overlay_active is deleted the calls go through and both assertions pass. """ + from __future__ import annotations import sys @@ -19,10 +20,33 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from prompt_toolkit.application import create_app_session +from prompt_toolkit.output import DummyOutput _MODULE = "amplifier_app_cli.main" +@pytest.fixture(autouse=True) +def _dummy_app_session(): + """Give prompt_toolkit an output it can use without a real console. + + These tests drive the REAL ``interactive_chat``, whose REPL wraps each turn + in ``patch_stdout()``. That reaches ``get_app().output``; with no app + session it builds a platform Output, and on Windows + ``Win32Output.__init__`` raises ``NoConsoleScreenBufferError`` whenever + stdout is not a real console (piped, redirected, CI). ``interactive_chat`` + then correctly declines to start an interactive session, so no turn runs + and ``render_message`` is never called -- the tests failed for a reason + that has nothing to do with the always-render contract they exist to guard. + + A ``DummyOutput`` app session removes that incidental dependency on the + host terminal. The assertions are unchanged and now hold on every platform + rather than only where a console happens to be attached. + """ + with create_app_session(output=DummyOutput()): + yield + + # --------------------------------------------------------------------------- # Session / initialized mock helpers (mirrors test_overlay_active_detection.py) # --------------------------------------------------------------------------- @@ -143,9 +167,7 @@ async def test_render_message_called_even_when_overlay_would_be_active( mock_render.assert_called_once() @pytest.mark.asyncio - async def test_render_message_called_when_no_streaming_config( - self, tmp_path: Path - ): + async def test_render_message_called_when_no_streaming_config(self, tmp_path: Path): """render_message IS called when no streaming-ui hook is configured. Sanity-check: the non-streaming path must also always render. diff --git a/tests/test_dead_code_removal.py b/tests/test_dead_code_removal.py index c7149cd6..80504388 100644 --- a/tests/test_dead_code_removal.py +++ b/tests/test_dead_code_removal.py @@ -16,7 +16,12 @@ def test_no_cancel_requested_global_variable(self): from pathlib import Path source = Path(__file__).parent.parent / "amplifier_app_cli" / "main.py" - content = source.read_text() + # encoding="utf-8" is REQUIRED: Python on Windows defaults to the + # locale codec (cp1252 here), and main.py contains non-cp1252 bytes, + # so a bare read_text() dies with UnicodeDecodeError before the + # assertion is ever reached. Every other read_text() in this file + # already passes it -- these two were simply missed. + content = source.read_text(encoding="utf-8") assert "_cancel_requested" not in content, ( "_cancel_requested is dead code (CancellationToken is used instead)" ) @@ -29,7 +34,12 @@ def test_key_manager_not_stored_as_variable(self): from pathlib import Path source = Path(__file__).parent.parent / "amplifier_app_cli" / "main.py" - content = source.read_text() + # encoding="utf-8" is REQUIRED: Python on Windows defaults to the + # locale codec (cp1252 here), and main.py contains non-cp1252 bytes, + # so a bare read_text() dies with UnicodeDecodeError before the + # assertion is ever reached. Every other read_text() in this file + # already passes it -- these two were simply missed. + content = source.read_text(encoding="utf-8") assert "_key_manager" not in content, ( "_key_manager is never referenced; only the constructor side-effect matters" ) diff --git a/tests/test_general_config_overrides.py b/tests/test_general_config_overrides.py index c6b79abe..412765e7 100644 --- a/tests/test_general_config_overrides.py +++ b/tests/test_general_config_overrides.py @@ -622,9 +622,15 @@ async def test_custom_routing_dir_injected_when_it_exists(self): assert len(routing_entries) == 1 cfg = routing_entries[0]["config"] assert cfg.get("default_matrix") == "ornith" - assert cfg.get("custom_routing_dirs") == ["/fake/home/.amplifier/routing"], ( - f"Expected custom_routing_dirs to be injected, got: {cfg}" - ) + # Compare against str(Path(...)), not a hardcoded POSIX string. The + # source does `str(custom_routing_dir)`, which correctly yields native + # separators -- the value is handed to a hook that opens the directory, + # so native is what it must be. On Windows that is + # "\\fake\\home\\.amplifier\\routing", and the literal comparison failed + # for a reason that had nothing to do with the injection being tested. + assert cfg.get("custom_routing_dirs") == [ + str(Path("/fake/home/.amplifier/routing")) + ], f"Expected custom_routing_dirs to be injected, got: {cfg}" @pytest.mark.asyncio async def test_custom_routing_dir_not_injected_when_absent(self): diff --git a/tests/test_stdout_offload_gaps.py b/tests/test_stdout_offload_gaps.py index 653fa860..3ea02573 100644 --- a/tests/test_stdout_offload_gaps.py +++ b/tests/test_stdout_offload_gaps.py @@ -23,6 +23,31 @@ from amplifier_app_cli.stdout_offload import _run_in_terminal_forcing_executor from amplifier_app_cli.stdout_offload import patch_stdout_offloaded +from prompt_toolkit.application import create_app_session +from prompt_toolkit.output import DummyOutput + + +@pytest.fixture(autouse=True) +def _dummy_app_session(): + """Give prompt_toolkit an output it can use without a real console. + + These tests exercise ``patch_stdout_offloaded()``'s own install/restore + machinery, but entering it also enters prompt_toolkit's ``patch_stdout``, + which reaches ``get_app().output``. With no app session that builds a + platform Output -- and on Windows ``Win32Output.__init__`` calls + ``GetConsoleScreenBufferInfo`` and raises ``NoConsoleScreenBufferError`` + whenever stdout is not a real console (piped, redirected, CI). The context + manager then dies on ENTRY and the body never runs, so all three tests + failed on Windows for a reason that has nothing to do with what they test. + + A ``DummyOutput`` app session removes that incidental dependency on the + host terminal without weakening the assertions: the monkeypatch + install/restore logic under test is untouched, and the tests now run + identically on every platform rather than only where a console happens to + be attached. + """ + with create_app_session(output=DummyOutput()): + yield def _current_run_in_terminal() -> Callable[..., Any]: From b5f4b4edcbed8edeed83f746c17a43d9b1423918 Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:53:30 -0700 Subject: [PATCH 4/6] ci: add native Windows to CI, scoped correctly Add windows-latest to the 'test' job's matrix (all three OSes now covered; fail-fast: false remains critical so a Windows failure never cancels the POSIX legs that tell us whether we regressed the population that already works). Deliberately do NOT add windows-latest to the 'integration' job: every test it selects (-m integration) forks a real child process and, in most files, allocates a real pty pair via the POSIX-only pty/termios stdlib modules -- there is no Windows equivalent of either mechanism. Two files already skip at module level on win32 (see the preceding test commit); a third, test_stdout_offload_freeze_integration.py, calls os.fork() directly with no guard at all and fails with AttributeError: module 'os' has no attribute 'fork'. A Windows leg of this job would therefore either run zero tests (all skipped) or hard-fail on the one unguarded file -- CI theatre either way, burning runner minutes for a signal that says nothing about real Windows support. The main test job's Windows leg is the meaningful signal; the integration job stays POSIX-only until a genuinely cross-platform integration test exists. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .github/workflows/ci.yml | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ecbfceae..07daf4cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] # requires-python = ">=3.11" in pyproject.toml -- cover the floor # and the latest supported minor. python-version: ["3.11", "3.12"] @@ -49,6 +49,25 @@ jobs: strategy: fail-fast: false matrix: + # No windows-latest here, deliberately. Every test this job selects + # (`-m integration`) forks a real child process and, in most of the + # files, allocates a real pty pair via the POSIX-only `pty`/`termios` + # stdlib modules -- there is no Windows equivalent of either + # mechanism. `test_ctrlc_functional_integration.py` and + # `test_terminal_echo_integration.py` already `pytest.skip(..., + # allow_module_level=True)` on win32 (they import `pty`/`termios` at + # module scope, so skipping is the only way to avoid a collection + # error). `test_stdout_offload_freeze_integration.py` calls + # `os.fork()` directly with no platform guard at all, and `os.fork` + # simply does not exist on Windows -- it fails with + # `AttributeError: module 'os' has no attribute 'fork'` + # (confirmed on a real windows-latest run, job id 94301234220). + # A Windows leg of this job would therefore either run zero tests + # (all skipped) or hard-fail on the one file missing a guard -- pure + # CI theatre, burning runner minutes to report a "green" (or "red") + # that says nothing about Windows support. If a genuinely + # cross-platform integration test is ever added to this job, add + # windows-latest back for it specifically. os: [ubuntu-latest, macos-latest] steps: - uses: actions/checkout@v4 @@ -66,5 +85,6 @@ jobs: # -- the ones that fork a real pty child and probe real termios state # -- are skipped by default and, before this job existed, ran nowhere. # They are exactly the tests that guard the dedicated-tty-input - # mechanism, so they get their own job on both platforms. + # mechanism, so they get their own job (POSIX only -- see the + # `matrix.os` comment above for why Windows is excluded). run: uv run pytest -m integration -q From a00035a7c454b62b9228896c01957c2a9af7f6f0 Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Mon, 17 Aug 2026 17:18:04 -0700 Subject: [PATCH 5/6] test: build PromptSession under a console-free app session TestPromptSessionWiring constructs a real PromptSession through the real _create_prompt_session factory. Under pytest on Windows, stdout is a captured pipe rather than a real console, so prompt_toolkit selects its Win32 output backend, whose GetConsoleScreenBufferInfo call raises NoConsoleScreenBufferError (prompt_toolkit/output/win32.py:219). All 11 tests in the class fail at construction. Bind a DummyOutput to the ambient AppSession for the duration of the factory call. Application.__init__ resolves output as `output or session.output`, so the Windows console API is never probed. The real factory and the real PromptSession are still exercised; only the output backend is substituted. POSIX behaviour is unchanged. --- tests/test_provider_command.py | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/test_provider_command.py b/tests/test_provider_command.py index a244c710..2feaf117 100644 --- a/tests/test_provider_command.py +++ b/tests/test_provider_command.py @@ -6,6 +6,7 @@ -- it never selects a provider itself (REQUIRED BEHAVIORS in the task spec). """ +import contextlib import sys from pathlib import Path from types import SimpleNamespace @@ -1065,14 +1066,35 @@ def isolated_home(tmp_path, monkeypatch): return tmp_path +@contextlib.contextmanager +def _headless_app_session(): + """Run prompt_toolkit with a console-free output backend. + + On Windows, pytest captures stdout as a pipe rather than a real console, + so prompt_toolkit selects Win32Output, whose GetConsoleScreenBufferInfo + call raises NoConsoleScreenBufferError. Binding a DummyOutput to the + ambient AppSession makes PromptSession resolve output via the session + (Application.__init__: ``self.output = output or session.output``) + instead of probing the console. POSIX behaviour is unchanged. + """ + from prompt_toolkit.application.current import create_app_session + from prompt_toolkit.input import create_pipe_input + from prompt_toolkit.output import DummyOutput + + with create_pipe_input() as pipe_input: + with create_app_session(input=pipe_input, output=DummyOutput()): + yield + + def _prompt_session(mode=None, pinned=None): """Build the REAL PromptSession through the REAL factory.""" from amplifier_app_cli.main import _create_prompt_session - return _create_prompt_session( - get_active_mode=lambda: mode, - get_pinned_provider=lambda: pinned, - ) + with _headless_app_session(): + return _create_prompt_session( + get_active_mode=lambda: mode, + get_pinned_provider=lambda: pinned, + ) def _render_via_prompt_session(session) -> str: From 68c953b17be30ed7554c1ae5175c6f3b56f0188d Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Mon, 17 Aug 2026 17:22:16 -0700 Subject: [PATCH 6/6] test: apply the console-free output at class scope The previous commit bound the DummyOutput inside the _prompt_session helper, which only covers the seven tests that use it. Four tests in the class call _create_prompt_session directly -- those still selected the Win32 output backend and still failed with NoConsoleScreenBufferError on windows-latest. Move the binding to an autouse fixture on TestPromptSessionWiring so it covers every test in the class regardless of how it reaches the factory, and cannot be bypassed by a future test that calls the factory directly. The helper goes back to a plain factory call -- the fixture is the single home for this behaviour. --- tests/test_provider_command.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/test_provider_command.py b/tests/test_provider_command.py index 2feaf117..276a34dc 100644 --- a/tests/test_provider_command.py +++ b/tests/test_provider_command.py @@ -1090,11 +1090,10 @@ def _prompt_session(mode=None, pinned=None): """Build the REAL PromptSession through the REAL factory.""" from amplifier_app_cli.main import _create_prompt_session - with _headless_app_session(): - return _create_prompt_session( - get_active_mode=lambda: mode, - get_pinned_provider=lambda: pinned, - ) + return _create_prompt_session( + get_active_mode=lambda: mode, + get_pinned_provider=lambda: pinned, + ) def _render_via_prompt_session(session) -> str: @@ -1110,6 +1109,18 @@ def _render_via_prompt_session(session) -> str: @pytest.mark.usefixtures("isolated_home") class TestPromptSessionWiring: + @pytest.fixture(autouse=True) + def _console_free_output(self): + """Every test in this class builds a real PromptSession. + + Applied at class scope rather than inside the _prompt_session helper + because four of these tests call _create_prompt_session directly, and + a class-scoped fixture cannot be bypassed by a future test that does + the same. + """ + with _headless_app_session(): + yield + def test_message_is_a_callable_not_a_prebuilt_value(self): """A static value would freeze the indicator at construction time -- the pin must be re-read on every render."""