From 5add0389db31592a8261f5627c0aa0d9467a977a Mon Sep 17 00:00:00 2001
From: DavidKoleczek <45405824+DavidKoleczek@users.noreply.github.com>
Date: Mon, 17 Aug 2026 16:13:00 -0400
Subject: [PATCH 1/7] fix(cli): guard the fcntl import so the CLI starts on
 Windows
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`amplifier_agent_lib/migration.py` imported `fcntl` unconditionally, and it
sits on the CLI's eager import path (`__main__.py` -> `admin/migrate.py` ->
here). `__main__.py` registers all 15 commands up front, so a module needed by
exactly one subcommand took down every subcommand, `--version` included, with
`ModuleNotFoundError: No module named 'fcntl'`.

The module was always scoped to Unix and says so in its own docstring. The
defect was expressing that scope as a bare import rather than a runtime check.

`fcntl` is now imported optionally and `file_lock` raises the new
`MigrationUnsupportedError` when it is absent, so the limitation is enforced at
call time and stays scoped to the one command that cannot work there.
`amplifier-agent migrate` on such a platform exits 1 with
`{"error": "migration-unsupported: ..."}` in JSON mode and a message on stderr
in text mode. That key is distinct from the two existing failure keys because
nothing failed and nothing was moved; reporting it as a failed migration would
suggest a partial one.

Refusing rather than locking as a no-op is deliberate. Both migrations move
user data and re-check their preconditions under the lock precisely because a
concurrent process could otherwise move the same tree twice, so running
unlocked would trade a documented platform limitation for a data-loss race.

Verified on a Windows 11 guest running the same ref: unpatched upstream
reproduces the three-hop traceback and exits 1; with this change `--version`,
`doctor` (11/11), and `verify` all exit 0, and `migrate` refuses cleanly in
both output formats. No behavior change on Linux or macOS.

This bug class has no automated regression coverage. The e2e harness
provisions Linux, where `fcntl` imports fine, so any e2e case asserting
`--version` exit 0 is green both before and after this change.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
---
 CHANGELOG.md                             | 18 +++++++++++++
 docs/spec/cli.md                         |  3 +++
 docs/spec/storage-and-workspace.md       | 10 +++++++
 src/amplifier_agent_cli/admin/migrate.py | 20 ++++++++++++++
 src/amplifier_agent_lib/migration.py     | 34 +++++++++++++++++++++++-
 5 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 86d55dc1..837875c4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **Every CLI command failed on Windows with `ModuleNotFoundError: No module
+  named 'fcntl'`**, `--version` included. `amplifier_agent_lib/migration.py`
+  imported `fcntl` unconditionally, and it sits on the CLI's eager import path
+  (`__main__.py` -> `admin/migrate.py` -> here), so a module that only one
+  subcommand needs took down every subcommand. The module was always scoped to
+  Unix and says so in its own docstring; the defect was expressing that scope
+  as a bare import. `fcntl` is now imported optionally and `file_lock` raises
+  `MigrationUnsupportedError` when it is absent, so the limitation is enforced
+  at call time and stays scoped to the one command that cannot work there.
+  `amplifier-agent migrate` on such a platform now exits 1 with
+  `{"error": "migration-unsupported: ..."}` rather than crashing the CLI.
+  Refusing rather than locking as a no-op is deliberate: the migrations move
+  user data and re-check their preconditions under the lock, so running
+  unlocked would trade a documented platform limitation for a data-loss race.
+  No behavior change on Linux or macOS.
+
 ## [0.12.0] — 2026-07-29
 
 ### Added
diff --git a/docs/spec/cli.md b/docs/spec/cli.md
index aa03f7b0..53e3d086 100644
--- a/docs/spec/cli.md
+++ b/docs/spec/cli.md
@@ -130,6 +130,9 @@ migrate [--output text|json]
     ~/.amplifier-agent). Idempotent. Exit 1 if either migration raises, else 0.
     JSON payload: {"sessions_migration": {migrated, skipped, collided},
                    "xdg_migration": {migrated, skipped, collided, from_xdg}}
+    Unix only. On a platform without flock it refuses at CALL time, exit 1 (see
+    storage-and-workspace.md). Its unavailability is scoped to this one subcommand:
+    every other subcommand, including `--version`, still dispatches normally there.
 
 version [--json]
     Plain: `amplifier-agent <version> (wire <protocolVersion>)`.
diff --git a/docs/spec/storage-and-workspace.md b/docs/spec/storage-and-workspace.md
index a3935bd0..a5ff80e2 100644
--- a/docs/spec/storage-and-workspace.md
+++ b/docs/spec/storage-and-workspace.md
@@ -207,6 +207,12 @@ observe the completed state. A lock file is created if absent, and it is release
 is killed, so a crashed run never strands it. Migration is supported on Unix only; Windows is out of
 scope for both.
 
+That scope is enforced at CALL time, not at import time. Where `flock` is unavailable, both
+migrations refuse rather than run unlocked: these migrations move user data and re-check their
+preconditions under the lock, so an unlocked run would trade a documented platform limitation for a
+data-loss race. Refusing is also what keeps the limitation scoped to this one command instead of
+breaking CLI dispatch for every other subcommand.
+
 Sessions migration:
 
 ```
@@ -251,6 +257,10 @@ Exit 0 on success, including the nothing-to-do case. Exit 1 on either migration
 `{"error": "sessions-migration-failed: ..."}` or `{"error": "xdg-migration-failed: ..."}` in JSON
 mode and a message on stderr in text mode.
 
+Exit 1 with `{"error": "migration-unsupported: ..."}` where the platform has no `flock`. A distinct
+key from the two above because nothing failed and nothing was moved: the command is unavailable
+here, and reporting it as a failed migration would suggest a partial one.
+
 Logging cadence:
 
 ```
diff --git a/src/amplifier_agent_cli/admin/migrate.py b/src/amplifier_agent_cli/admin/migrate.py
index ca27e26e..77c18ab6 100644
--- a/src/amplifier_agent_cli/admin/migrate.py
+++ b/src/amplifier_agent_cli/admin/migrate.py
@@ -12,10 +12,12 @@
 
 import json
 import sys
+from typing import NoReturn
 
 import click
 
 from amplifier_agent_lib.migration import (
+    MigrationUnsupportedError,
     maybe_migrate_legacy_xdg_storage,
     migrate_legacy_sessions_if_needed,
 )
@@ -23,6 +25,20 @@
 __all__ = ["migrate_command"]
 
 
+def _exit_unsupported(exc: MigrationUnsupportedError, output: str) -> NoReturn:
+    """Report an unsupported platform and exit non-zero.
+
+    Distinct from the generic failure path: nothing went wrong and nothing was
+    moved, so the message says the command is unavailable here rather than
+    implying a partial or failed migration.
+    """
+    if output == "json":
+        click.echo(json.dumps({"error": f"migration-unsupported: {exc}"}))
+    else:
+        click.echo(f"Error: {exc}", err=True)
+    sys.exit(1)
+
+
 @click.command(name="migrate")
 @click.option(
     "--output",
@@ -35,6 +51,8 @@ def migrate_command(output: str) -> None:
     """Migrate legacy storage layouts to current. Idempotent; safe to run multiple times. Reports what was moved."""
     try:
         sessions = migrate_legacy_sessions_if_needed()
+    except MigrationUnsupportedError as exc:
+        _exit_unsupported(exc, output)
     except Exception as exc:
         if output == "json":
             click.echo(json.dumps({"error": f"sessions-migration-failed: {exc}"}))
@@ -44,6 +62,8 @@ def migrate_command(output: str) -> None:
 
     try:
         xdg = maybe_migrate_legacy_xdg_storage()
+    except MigrationUnsupportedError as exc:
+        _exit_unsupported(exc, output)
     except Exception as exc:
         if output == "json":
             click.echo(json.dumps({"error": f"xdg-migration-failed: {exc}"}))
diff --git a/src/amplifier_agent_lib/migration.py b/src/amplifier_agent_lib/migration.py
index 7dffee50..7ab0cb2e 100644
--- a/src/amplifier_agent_lib/migration.py
+++ b/src/amplifier_agent_lib/migration.py
@@ -14,12 +14,18 @@
      Idempotent via sentinel file at <home>/.migrated_from_xdg.
 
 Unix-only (fcntl.flock). AAA targets Linux/macOS; Windows is out of scope.
+
+That scope is enforced at CALL time, not at import time.  This module sits on
+the CLI's eager import path (__main__.py -> admin/migrate.py -> here), so an
+unguarded ``import fcntl`` would take the entire CLI down on Windows rather
+than just the one command that cannot work there.  The import is therefore
+optional, and ``file_lock`` raises MigrationUnsupportedError when locking is
+unavailable.
 """
 
 from __future__ import annotations
 
 import contextlib
-import fcntl
 import logging
 import os
 import shutil
@@ -30,11 +36,28 @@
 
 from amplifier_agent_lib.persistence import _home, amplifier_agent_home, state_root
 
+try:
+    import fcntl
+except ImportError:  # pragma: no cover - Windows; see module docstring
+    fcntl = None  # type: ignore[assignment]
+
 logger = logging.getLogger(__name__)
 
 LEGACY_WORKSPACE = "_legacy"
 
 
+class MigrationUnsupportedError(RuntimeError):
+    """Raised when migration cannot run safely on this platform.
+
+    Migration moves user data between storage roots, and both migrations
+    re-check their preconditions under an exclusive lock precisely because a
+    concurrent process could otherwise move the same tree twice.  Without
+    ``fcntl.flock`` there is no lock, so the correct behavior is to refuse
+    rather than to run unguarded: a silent no-op lock would turn a documented
+    platform limitation into a data-loss race.
+    """
+
+
 @dataclass
 class MigrationResult:
     """Outcome of a migration attempt."""
@@ -52,7 +75,16 @@ def file_lock(lock_path: Path) -> Iterator[None]:
     The lock file is created if absent. The kernel releases the lock when the
     file descriptor closes (on context exit or process death), so a killed
     process never strands the lock.
+
+    Raises:
+        MigrationUnsupportedError: if ``fcntl`` is unavailable (Windows).
     """
+    if fcntl is None:
+        raise MigrationUnsupportedError(
+            "Storage migration requires file locking (fcntl.flock), which is "
+            "not available on this platform. amplifier-agent migrate is "
+            "supported on Linux and macOS only."
+        )
     lock_path.parent.mkdir(parents=True, exist_ok=True)
     fd = open(lock_path, "w")
     try:

From f0f32770d155b3b9d1c8138caf1874db90e5186f Mon Sep 17 00:00:00 2001
From: DavidKoleczek <45405824+DavidKoleczek@users.noreply.github.com>
Date: Mon, 17 Aug 2026 16:35:23 -0400
Subject: [PATCH 2/7] fix(cli): skip the SC-B session-leader step where setsid
 does not exist
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`run` called `os.getsid(0)` and `os.setsid()` unconditionally at the top of its
callback, guarded by `except (OSError, PermissionError)`. Neither name exists on
Windows and the raised `AttributeError` was not in that tuple, so every `run`
died before reaching any engine code:

    AttributeError: module 'os' has no attribute 'getsid'

The step is now gated on `hasattr(os, "setsid")`, and the
`AMPLIFIER_AGENT_DEBUG_SIDLOG` branch reports `n/a` rather than raising from the
same missing name. The capability is checked rather than the platform, because
what the code needs is the call, not the OS.

On POSIX nothing changes: the engine still becomes session leader, confirmed by
`AMPLIFIER_AGENT_DEBUG_SIDLOG` reporting `sid == pid`.

This deliberately does not give Windows an equivalent guarantee. Windows has no
session groups, and the containment primitive is a Job Object, a different
mechanism on both sides of the wrapper boundary rather than a flag on this one.
Swallowing the error silently would leave a documented contract reading as
satisfied on a platform that cannot satisfy it, so the gap is written down
instead: cancellation on Windows reaches the engine, and MCP children may
outlive it. See docs/spec/wrapper-contract.md.

Verified on a Windows 11 guest: unpatched reproduces the AttributeError at
single_turn.py:669 and exits 1; with this change `run` completes a real LLM
turn and exits 0. Linux e2e `run` suite passes.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
---
 CHANGELOG.md                                 | 16 +++++++++++++
 docs/spec/wrapper-contract.md                |  7 ++++++
 src/amplifier_agent_cli/modes/single_turn.py | 25 +++++++++++++-------
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 837875c4..e3fcf030 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`amplifier-agent run` crashed on Windows with `AttributeError: module 'os'
+  has no attribute 'getsid'`.** The SC-B session-leader step at the top of the
+  `run` callback called `os.getsid`/`os.setsid` unconditionally, and
+  `AttributeError` was not in its `except` tuple. Neither name exists on
+  Windows, so every `run` died before reaching any engine code. The step is now
+  gated on `hasattr(os, "setsid")` and the debug SIDLOG branch reports `n/a`
+  instead of raising. On POSIX the behavior is unchanged: the engine still
+  becomes session leader, verified by `AMPLIFIER_AGENT_DEBUG_SIDLOG` reporting
+  `sid == pid`.
+
+  This does not give Windows an equivalent guarantee, and pretending otherwise
+  would be worse than the crash. Windows has no session groups; the containment
+  primitive is a Job Object, a different mechanism on both sides of the wrapper
+  boundary. Until that is built, cancellation on Windows reaches the engine but
+  MCP children may outlive it. Recorded in `docs/spec/wrapper-contract.md`.
+
 - **Every CLI command failed on Windows with `ModuleNotFoundError: No module
   named 'fcntl'`**, `--version` included. `amplifier_agent_lib/migration.py`
   imported `fcntl` unconditionally, and it sits on the CLI's eager import path
diff --git a/docs/spec/wrapper-contract.md b/docs/spec/wrapper-contract.md
index b795800e..305bf1d1 100644
--- a/docs/spec/wrapper-contract.md
+++ b/docs/spec/wrapper-contract.md
@@ -274,6 +274,13 @@ Cancellation is idempotent, and dispose is an alias for it.
 Signaling only the engine PID would orphan the engine's MCP children, holding file descriptors,
 ports, and sockets open until the OS reaps them minutes later.
 
+POSIX only, on both sides. Windows has no session groups and provides neither `os.getsid` nor
+`os.setsid`, so the engine skips the session-leader step there, and a negative-PID group signal is
+not a Windows operation. The containment primitive on Windows is a Job Object, a different
+mechanism on both sides of the boundary rather than a flag on this one. Until that is built, the
+guarantee above does not hold on Windows: cancelling reaches the engine, and MCP children may
+outlive it. Engine startup and single-turn `run` are unaffected.
+
 ## Non-goals
 
 - **The wrapper never sees or configures the bundle.** No mount plan crosses the boundary, no
diff --git a/src/amplifier_agent_cli/modes/single_turn.py b/src/amplifier_agent_cli/modes/single_turn.py
index 74a1bc65..3d18a600 100644
--- a/src/amplifier_agent_cli/modes/single_turn.py
+++ b/src/amplifier_agent_cli/modes/single_turn.py
@@ -665,16 +665,25 @@ def run(
     # SC-B — engine becomes session leader so MCP child processes spawned via
     # tool-mcp.mount() inherit a shared session group. The wrapper kills the
     # group on cancel so children die with the parent.
-    try:
-        if os.getsid(0) != os.getpid():
-            os.setsid()
-    except (OSError, PermissionError):
-        # Best-effort — running under a debugger or test harness that already
-        # owns a session may make setsid() fail; tolerate.
-        pass
+    #
+    # POSIX only. Windows has no session groups and provides neither os.getsid
+    # nor os.setsid, so this is skipped there rather than guessed at: the
+    # equivalent containment primitive is a Job Object, which is a different
+    # mechanism on both sides of the wrapper boundary. The consequence is
+    # recorded in docs/spec/wrapper-contract.md — on Windows, MCP children are
+    # not guaranteed to die with the engine on cancel.
+    if hasattr(os, "setsid"):
+        try:
+            if os.getsid(0) != os.getpid():
+                os.setsid()
+        except (OSError, PermissionError):
+            # Best-effort — running under a debugger or test harness that already
+            # owns a session may make setsid() fail; tolerate.
+            pass
     if os.environ.get("AMPLIFIER_AGENT_DEBUG_SIDLOG"):
         try:
-            sys.stderr.write(f"engine-sid-ok pid={os.getpid()} sid={os.getsid(0)}\n")
+            sid = os.getsid(0) if hasattr(os, "getsid") else "n/a"
+            sys.stderr.write(f"engine-sid-ok pid={os.getpid()} sid={sid}\n")
         except OSError:
             pass
 

From 3ff7eef0bb0598d197009f4405f169b5037a91b8 Mon Sep 17 00:00:00 2001
From: DavidKoleczek <45405824+DavidKoleczek@users.noreply.github.com>
Date: Mon, 17 Aug 2026 16:44:51 -0400
Subject: [PATCH 3/7] fix(cli): force UTF-8 stdio so a non-ASCII reply cannot
 crash the turn
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Python selects the console code page for stdio. On Windows that is a legacy
single-byte encoding (cp1252 on the guests we test), so writing a reply
containing any character outside that page raised at the final write:

    _real_stdout.write(result.get("reply", "") + "\n")
      File ".../encodings/cp1252.py", line 19, in encode
    UnicodeEncodeError: 'charmap' codec can't encode character '\u2713'

The failure lands after the model call has run and been billed, which is the
worst available moment: the work is done, paid for, and then discarded.

`main()` now reconfigures stdout and stderr to UTF-8 before dispatch. An
explicit PYTHONIOENCODING wins, because an operator who named an encoding
outranks this default even when the one they named is narrow. Streams that are
already UTF-8, or that cannot be reconfigured, are skipped rather than fought.

Not gated on the platform. The trigger is a stdio encoding that cannot
represent the payload, not the OS: POSIX under
`LC_ALL=C PYTHONCOERCECLOCALE=0 PYTHONUTF8=0` selects ASCII and fails
identically. On a normal UTF-8 host the call is a no-op.

Two corrections to the original Windows report, found while confirming this.

The exposed surface is one line, not the general stdout path. The JSON envelope
writes go through `json.dumps`, whose default `ensure_ascii=True` makes them
ASCII by construction; `jsonrpc.write_message` encodes to UTF-8 bytes itself;
`click.echo` does its own encoding and never raises. Only the text-mode reply
write is exposed.

An em dash is not the trigger. U+2014 is representable in cp1252, so em dashes
in --help and in skill descriptions degrade to a replacement character rather
than crashing. Reproducing the reported failure requires a character with no
cp1252 mapping, such as U+2713 or CJK.

Verified on a Windows 11 guest, console code page 437, Python stdout cp1252:
unpatched reproduces the UnicodeEncodeError at the reply write and exits 1;
with this change the same prompt returns `DONE <U+2713> OK` and exits 0. Linux
`make check` clean and e2e `run` + `modes` suites pass (16).

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
---
 CHANGELOG.md                        | 24 ++++++++++++++++++
 src/amplifier_agent_cli/__main__.py | 38 +++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e3fcf030..ce6e09bf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`amplifier-agent run` crashed with `UnicodeEncodeError` after the turn had
+  already completed.** Python picks the console code page for stdio, which on
+  Windows is a legacy single-byte encoding (cp1252 on the guests we test), so
+  writing a reply containing any character outside that page raised at the
+  final write -- after the model call had run and been billed. `main()` now
+  reconfigures stdout and stderr to UTF-8 before dispatch, honoring an explicit
+  `PYTHONIOENCODING` and skipping streams that are already UTF-8 or cannot be
+  reconfigured.
+
+  Not gated on the platform, because the trigger is a stdio encoding that
+  cannot represent the payload rather than the OS: POSIX under
+  `LC_ALL=C PYTHONCOERCECLOCALE=0 PYTHONUTF8=0` selects ASCII and fails
+  identically. On a normal UTF-8 host this is a no-op.
+
+  Two corrections to the original Windows report while confirming this. First,
+  the exposed surface is exactly one line, `single_turn.py`'s text-mode reply
+  write: the JSON envelope paths go through `json.dumps`, whose default
+  `ensure_ascii=True` makes them ASCII by construction; `jsonrpc.write_message`
+  encodes to UTF-8 bytes itself; and `click.echo` does its own encoding and
+  never raises. Second, an em dash is *not* the trigger -- U+2014 is
+  representable in cp1252. Em dashes in `--help` and in skill descriptions
+  degrade to a replacement character rather than crashing. What actually
+  crashes is a character with no cp1252 mapping, such as U+2713 or CJK.
+
 - **`amplifier-agent run` crashed on Windows with `AttributeError: module 'os'
   has no attribute 'getsid'`.** The SC-B session-leader step at the top of the
   `run` callback called `os.getsid`/`os.setsid` unconditionally, and
diff --git a/src/amplifier_agent_cli/__main__.py b/src/amplifier_agent_cli/__main__.py
index 71303ac4..0d8aa0d3 100644
--- a/src/amplifier_agent_cli/__main__.py
+++ b/src/amplifier_agent_cli/__main__.py
@@ -27,6 +27,7 @@
 
 from __future__ import annotations
 
+import os
 import sys
 
 import click
@@ -75,8 +76,45 @@ def cli(ctx: click.Context) -> None:
 cli.add_command(_providers_group, name="providers")
 
 
+def _force_utf8_io() -> None:
+    """Make stdout and stderr UTF-8 unless the operator picked an encoding.
+
+    Python selects the console code page for stdio, which on Windows is a
+    legacy single-byte encoding (cp1252 on the guests we test). Any non-ASCII
+    character then raises UnicodeEncodeError at write time -- after the turn has
+    run and been paid for, which is the worst possible moment to fail. This is
+    not an edge case: em dashes appear in this CLI's own --help text and in
+    bundled skill descriptions, so a reply quoting either one is enough.
+
+    Not Windows-specific, and not gated on the platform. POSIX under LC_ALL=C
+    selects ASCII for the same reason and fails the same way; the trigger is a
+    stdio encoding that cannot represent the payload, not the OS.
+
+    PYTHONIOENCODING is honored when set. An operator who named an encoding
+    outranks this default, including when they name a narrow one deliberately.
+
+    Best-effort by construction: a replaced or non-reconfigurable stream is
+    skipped rather than fought, because failing here would break the CLI in
+    exactly the environments this is meant to protect.
+    """
+    if os.environ.get("PYTHONIOENCODING"):
+        return
+    for stream in (sys.stdout, sys.stderr):
+        reconfigure = getattr(stream, "reconfigure", None)
+        if reconfigure is None:
+            continue
+        current = (getattr(stream, "encoding", "") or "").lower().replace("-", "_")
+        if current in ("utf_8", "utf8"):
+            continue
+        try:
+            reconfigure(encoding="utf-8")
+        except (OSError, ValueError):  # pragma: no cover - stream refused
+            pass
+
+
 def main() -> None:
     """Entry point referenced by pyproject.toml [project.scripts]."""
+    _force_utf8_io()
     try:
         cli(standalone_mode=True)
     except KeyboardInterrupt:

From 16af8632a844111d70cf11210c1926b601e7d05c Mon Sep 17 00:00:00 2001
From: DavidKoleczek <45405824+DavidKoleczek@users.noreply.github.com>
Date: Mon, 17 Aug 2026 17:04:07 -0400
Subject: [PATCH 4/7] feat(bundle): mount tool-pwsh instead of tool-bash on
 Windows
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`tool-bash` cannot find a shell on Windows. It resolves via
`shutil.which("bash")`, and Git for Windows installs `bash.exe` into
`C:\Program Files\Git\bin` while adding only `C:\Program Files\Git\cmd` to
PATH. The lookup returns None and commands fall through to a branch that execs
the command name as a binary. Driving the tool directly on a Windows guest:

    shutil.which("bash") -> None
    tool-bash.execute({"command": "pwd"})
      success = False
      output  = '[WinError 2] The system cannot find the file specified'

No mention of bash, no remedy, nothing the model can act on. The original report
described the model retrying and concluding the tool was broken. What we
measured is worse: given that error the model did not report a failure at all,
it fabricated a plausible working directory and presented it as command output.
An unactionable error does not reliably surface as a visible failure; it can
surface as a confident wrong answer, which is the more expensive outcome.

bundle.md now declares both shell modules and `shell_tool.select_shell_tool`
drops the wrong one at the bundle-prep seam -- one place, reached by both the
CLI and HTTP faces, running before the host-config merge so every later step and
the kernel see the final roster.

Declaring both is what makes this expressible. The manifest is static and its
sha256 is the prepared-cache key, so there is no conditional form to hang a
platform predicate on. Both are therefore installed during cold-prepare, costing
one extra clone of a zero-dependency package on POSIX.

The swap is not a rename, and that is deliberate. The tool is named `pwsh`
because the tool name is a strong prior on the syntax the model emits; a `bash`
tool that runs PowerShell would invite bash syntax and fail differently. The
cost is that anything matching the shell by literal tool name must name both, so
the shipped `plan` and `brainstorm` modes now list `bash` and `pwsh` in their
tool policies. Without that the shell would be entirely unpoliced on Windows,
which is a worse failure than the one being fixed. Sub-agents need no change:
they inherit the already-filtered parent roster.

`tool-pwsh` is pinned to a commit rather than `@main`.
amplifier-bundle-windows-shell is a single-commit proof-of-concept with no tags
and no CI, so `@main` would let an upstream force-push change what users install
with no signal here.

Verified on a Windows 11 guest: cold-prepare installs tool-pwsh and primes the
cache; `select_shell_tool` reports `dropped: tool-bash, roster: ['tool-pwsh',
...]`; driving tool-pwsh directly returns success=True with real stdout; and an
agent turn shows `[tool/started] pwsh` returning the true working directory.
PowerShell 7 is absent on that guest and the documented 5.1 fallback is what
runs, so no PATH repair or extra install is required.

Linux: `make check` clean; full e2e 44 passed. The 11 github_copilot failures
are the harness's own documented consequence of GITHUB_TOKEN being unset, and
the single shadowing error was a port-9098 collision that passes 6/6 on re-run.
The DTU log confirms the filter live: `shell tool: kept tool-bash, dropped
tool-pwsh (platform=linux)`.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
---
 CHANGELOG.md                                  | 42 ++++++++++
 docs/spec/bundle-and-cache.md                 | 27 ++++++-
 src/amplifier_agent_lib/_runtime.py           |  8 ++
 src/amplifier_agent_lib/bundle/bundle.md      | 15 ++++
 .../bundle/modes/brainstorm.md                |  4 +
 src/amplifier_agent_lib/bundle/modes/plan.md  |  4 +
 src/amplifier_agent_lib/shell_tool.py         | 81 +++++++++++++++++++
 7 files changed, 180 insertions(+), 1 deletion(-)
 create mode 100644 src/amplifier_agent_lib/shell_tool.py

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ce6e09bf..667ac793 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,8 +7,50 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- **PowerShell shell tool on Windows.** `bundle.md` now declares both
+  `tool-bash` and `tool-pwsh` (from `amplifier-bundle-windows-shell`, pinned to
+  a commit rather than `@main` because that repo is a single-commit
+  proof-of-concept with no tags and no CI). Exactly one is mounted, chosen by
+  `amplifier_agent_lib/shell_tool.py` at the bundle-prep seam: `tool-pwsh` on
+  Windows, `tool-bash` everywhere else.
+
+  The swap is not a rename. The tool is named `pwsh`, because the tool name is a
+  strong prior on the syntax the model emits, so the model gets a PowerShell
+  tool rather than a `bash` tool with a surprising backend. The cost is that
+  anything matching the shell by literal tool name has to name both: the shipped
+  `plan` and `brainstorm` modes now list `bash` and `pwsh` in their tool
+  policies, which they must, or the shell would be unpoliced on Windows.
+
+  Declaring both is what makes the swap expressible at all -- the manifest is
+  static and its sha256 is the prepared-cache key, so there is no conditional
+  form available. Both are therefore installed during cold-prepare, costing one
+  extra clone of a zero-dependency package on POSIX. Nothing changes for
+  existing POSIX users beyond that clone.
+
 ### Fixed
 
+- **The shell tool was unusable on Windows, and failed in a way that produced
+  confidently wrong answers.** `tool-bash` resolves its shell with
+  `shutil.which("bash")`. Git for Windows installs `bash.exe` into
+  `C:\Program Files\Git\bin` but only adds `C:\Program Files\Git\cmd` to PATH,
+  so the lookup returns `None` and commands fall through to a branch that tries
+  to exec the command name as a binary. Driving the tool directly on a Windows
+  guest returns `success=False` with
+  `[WinError 2] The system cannot find the file specified` -- no mention of
+  bash, no remedy, nothing the model can act on.
+
+  The original report described the model retrying and concluding the tool was
+  broken. What we measured is worse: given that error the model did not report a
+  failure at all, it fabricated a plausible working directory and presented it
+  as command output. A tool that fails unactionably does not reliably produce a
+  visible error; it can produce a silent wrong answer.
+
+  Fixed by mounting `tool-pwsh` instead on Windows (see Added). PowerShell needs
+  no PATH repair: `tool-pwsh` finds PowerShell 7 when present and falls back to
+  Windows PowerShell 5.1, which ships with the OS.
+
 - **`amplifier-agent run` crashed with `UnicodeEncodeError` after the turn had
   already completed.** Python picks the console code page for stdio, which on
   Windows is a legacy single-byte encoding (cp1252 on the guests we test), so
diff --git a/docs/spec/bundle-and-cache.md b/docs/spec/bundle-and-cache.md
index b427f0c3..19bb165a 100644
--- a/docs/spec/bundle-and-cache.md
+++ b/docs/spec/bundle-and-cache.md
@@ -37,7 +37,8 @@ session.context:      context-simple       max_tokens 300000, auto_compact
 session.provider:     the anthropic provider, as the runtime default entry
 
 tools:
-  tool-filesystem, tool-bash, tool-web, tool-search, tool-todo, tool-apply-patch,
+  tool-filesystem, tool-web, tool-search, tool-todo, tool-apply-patch,
+  tool-bash | tool-pwsh  (both declared, exactly one mounted -- see below),
   tool-delegate (self_delegation, session_resume, context_inheritance, provider_selection;
                  excludes tool-delegate from sub-agents),
   tool-mcp, tool-skills, tool-mode, tool-recipes
@@ -54,6 +55,30 @@ Agents declare no `tools:` blocks; they inherit the parent tool roster through t
 `context_inheritance`. Modules referenced only by agent definitions are installed alongside the
 top-level ones, so a delegated session can always mount what its agent declares.
 
+### Shell tool selection
+
+The manifest declares both shell modules; exactly one reaches the kernel. The selection runs at the
+bundle-prep seam, before host-config merge, so every later step and the kernel see the final roster:
+
+```
+Windows    tool-pwsh mounted, tool-bash dropped
+otherwise  tool-bash mounted, tool-pwsh dropped
+```
+
+Both are declared because the manifest is static and its sha256 is the cache key, so there is no
+conditional form available here. Both are therefore also installed during cold-prepare, which costs
+one extra clone of a zero-dependency package on POSIX.
+
+The swap is not a rename: the tool is named `pwsh`, and the model sees a PowerShell tool rather than
+a `bash` tool with a different backend. The name is a strong prior on the syntax the model emits, so
+this is deliberate. The consequence is that anything matching the shell by literal tool name must
+name both -- the shipped modes list `bash` and `pwsh` in their tool policies for exactly this
+reason. Sub-agents are unaffected: they inherit the already-filtered parent roster.
+
+On Windows the selection is unconditional and does not probe for PowerShell first. Windows PowerShell
+5.1 is present on every supported Windows install, and `tool-pwsh` falls back to it when PowerShell 7
+is absent.
+
 Four upstream modules are deliberately absent relative to the upstream behavioral-anchor bundle:
 `hooks-streaming-ui` and `hooks-todo-display` would break the JSON-stdout contract,
 `behaviors/logging.yaml` is replaced by `hook-context-intelligence`, and `hooks-approval` is dropped
diff --git a/src/amplifier_agent_lib/_runtime.py b/src/amplifier_agent_lib/_runtime.py
index e3ae9600..5b898dd4 100644
--- a/src/amplifier_agent_lib/_runtime.py
+++ b/src/amplifier_agent_lib/_runtime.py
@@ -27,6 +27,7 @@
 from amplifier_agent_lib.incremental_save import IncrementalSaveHook
 from amplifier_agent_lib.persistence import state_root
 from amplifier_agent_lib.session_store import SessionStore
+from amplifier_agent_lib.shell_tool import select_shell_tool
 from amplifier_agent_lib.skill_dispatch import USER_TURN_ROLE, dispatch_skill_or_execute
 from amplifier_agent_lib.wire_approval_provider import WireApprovalProvider
 
@@ -118,6 +119,13 @@ def prepare_bundle_for_session(
     the prepared bundle's mount_plan BEFORE calling. A future clone-return
     variant is on the design backlog.
     """
+    # Shell tool selection. bundle.md declares both tool-bash and tool-pwsh;
+    # exactly one belongs on this platform. Runs FIRST so every step below --
+    # and the kernel -- sees the final tool roster rather than one that still
+    # contains a module about to be removed. See shell_tool.py for why the
+    # manifest declares both.
+    select_shell_tool(prepared.mount_plan or {})
+
     # D4: mcp.configPath -> AMPLIFIER_MCP_CONFIG env var.
     mcp_block = (host_config or {}).get("mcp")
     if isinstance(mcp_block, dict):
diff --git a/src/amplifier_agent_lib/bundle/bundle.md b/src/amplifier_agent_lib/bundle/bundle.md
index 878e60a1..984ee92a 100644
--- a/src/amplifier_agent_lib/bundle/bundle.md
+++ b/src/amplifier_agent_lib/bundle/bundle.md
@@ -104,8 +104,23 @@ tools:
   # Core tools (inherited by all sub-agents via tool-delegate)
   - module: tool-filesystem
     source: git+https://github.com/microsoft/amplifier-module-tool-filesystem@main
+  # Shell. BOTH are declared; exactly one is mounted per platform by
+  # amplifier_agent_lib/shell_tool.py, which runs at the bundle-prep seam:
+  #   Windows   -> tool-pwsh    (tool-bash cannot find a shell there)
+  #   elsewhere -> tool-bash    (tool-pwsh is dropped)
+  # Declaring both is what makes the swap possible at all: this manifest is
+  # static and its sha256 is the prepared-cache key, so there is no conditional
+  # form to express "Windows only" here. The cost is one extra clone of a
+  # zero-dependency package during cold-prepare on POSIX.
   - module: tool-bash
     source: git+https://github.com/microsoft/amplifier-module-tool-bash@main
+  # Pinned to a commit, not @main: amplifier-bundle-windows-shell is a
+  # single-commit proof-of-concept with no tags and no CI, so @main would let an
+  # upstream force-push change what users install with no signal here.
+  - module: tool-pwsh
+    source: git+https://github.com/microsoft/amplifier-bundle-windows-shell@fd58d71418304da9116c92bc9211986f1a7b19c8#subdirectory=modules/tool-pwsh
+    config:
+      safety_profile: standard
   - module: tool-web
     source: git+https://github.com/microsoft/amplifier-module-tool-web@main
   - module: tool-search
diff --git a/src/amplifier_agent_lib/bundle/modes/brainstorm.md b/src/amplifier_agent_lib/bundle/modes/brainstorm.md
index 6ed050ac..878d145d 100644
--- a/src/amplifier_agent_lib/bundle/modes/brainstorm.md
+++ b/src/amplifier_agent_lib/bundle/modes/brainstorm.md
@@ -17,7 +17,11 @@ mode:
       - delegate
       - recipes
     warn:
+      # Both shell tools. Only one is ever mounted (tool-bash on POSIX,
+      # tool-pwsh on Windows), but tool policies match on the TOOL NAME, so
+      # naming only `bash` would leave the shell unpoliced on Windows.
       - bash
+      - pwsh
 
   default_action: block
 ---
diff --git a/src/amplifier_agent_lib/bundle/modes/plan.md b/src/amplifier_agent_lib/bundle/modes/plan.md
index ff8fee34..1c4da254 100644
--- a/src/amplifier_agent_lib/bundle/modes/plan.md
+++ b/src/amplifier_agent_lib/bundle/modes/plan.md
@@ -18,7 +18,11 @@ mode:
       - delegate
       - recipes
     warn:
+      # Both shell tools. Only one is ever mounted (tool-bash on POSIX,
+      # tool-pwsh on Windows), but tool policies match on the TOOL NAME, so
+      # naming only `bash` would leave the shell unpoliced on Windows.
       - bash
+      - pwsh
   
   default_action: block
 ---
diff --git a/src/amplifier_agent_lib/shell_tool.py b/src/amplifier_agent_lib/shell_tool.py
new file mode 100644
index 00000000..ec6ec0b5
--- /dev/null
+++ b/src/amplifier_agent_lib/shell_tool.py
@@ -0,0 +1,81 @@
+"""Platform selection for the shell tool.
+
+``bundle.md`` declares BOTH shell modules. Exactly one survives into any given
+session's mount plan, and this module is what picks it:
+
+    Windows   ->  tool-pwsh    (PowerShell, from amplifier-bundle-windows-shell)
+    elsewhere ->  tool-bash
+
+Why declare both and filter, rather than declare one conditionally: the bundle
+manifest is static and its sha256 IS the prepared-cache key, so there is no
+conditional syntax to hang this on. Declaring both also means
+``bundle.prepare(install_deps=True)`` installs both, which costs one extra clone
+of a zero-dependency package on POSIX and buys a mount plan that cannot be
+missing its shell tool on either platform.
+
+Why swap rather than mount both: on Windows ``tool-bash`` does not find a shell.
+Git for Windows ships ``bash.exe`` in a directory its installer leaves off PATH,
+so ``shutil.which("bash")`` returns None and commands fall through to a branch
+that tries to exec ``pwd`` as a binary. The model then sees an unactionable
+``[WinError 2]`` and concludes the tool is broken. Leaving both mounted would
+keep that trap one wrong choice away, and would spend context on two shell tool
+descriptions to do it.
+
+The name difference is deliberate upstream and is load-bearing here. The tool is
+``pwsh``, not ``bash`` backed by PowerShell, because the tool name is a strong
+prior on the SYNTAX the model emits. Anything that refers to the shell tool by
+literal name therefore has to name both; the shipped modes do (see
+``bundle/modes/*.md``).
+"""
+
+from __future__ import annotations
+
+import logging
+import sys
+from typing import Any
+
+logger = logging.getLogger(__name__)
+
+__all__ = ["POSIX_SHELL_MODULE", "WINDOWS_SHELL_MODULE", "select_shell_tool"]
+
+WINDOWS_SHELL_MODULE = "tool-pwsh"
+POSIX_SHELL_MODULE = "tool-bash"
+
+
+def select_shell_tool(mount_plan: dict[str, Any], *, is_windows: bool | None = None) -> str | None:
+    """Drop the shell module that does not belong on this platform, in place.
+
+    Args:
+        mount_plan: A mount plan dict. ``mount_plan["tools"]`` is a list of
+            ``{module, config, source}`` entries, the shape
+            ``Bundle.to_mount_plan()`` produces.
+        is_windows: Platform override, for tests. Defaults to the real platform.
+
+    Returns:
+        The module id that was dropped, or ``None`` if nothing was dropped
+        (either it was not declared, or the plan has no tools).
+
+    Tolerates a manifest that declares neither module, or only the one being
+    kept. A missing entry is not an error here: this function's job is to
+    remove, and an operator who has already narrowed the roster has made a
+    choice worth respecting rather than second-guessing at mount time.
+    """
+    if is_windows is None:
+        is_windows = sys.platform == "win32"
+
+    drop = POSIX_SHELL_MODULE if is_windows else WINDOWS_SHELL_MODULE
+    tools = mount_plan.get("tools")
+    if not isinstance(tools, list):
+        return None
+
+    remaining = [entry for entry in tools if entry.get("module") != drop]
+    if len(remaining) == len(tools):
+        return None
+
+    # Mutate the SAME list object. Callers hold references to
+    # ``prepared.mount_plan["tools"]`` and rebinding the key would leave them
+    # pointing at the unfiltered list.
+    tools[:] = remaining
+    kept = WINDOWS_SHELL_MODULE if is_windows else POSIX_SHELL_MODULE
+    logger.info("shell tool: kept %s, dropped %s (platform=%s)", kept, drop, sys.platform)
+    return drop

From 96d42d7b63665a5e3bcd9fd2aaf00428347c5cb2 Mon Sep 17 00:00:00 2001
From: sadlilas <11658960+sadlilas@users.noreply.github.com>
Date: Tue, 18 Aug 2026 12:31:04 -0700
Subject: [PATCH 5/7] Revert "feat(bundle): mount tool-pwsh instead of
 tool-bash on Windows"

This reverts commit 16af8632a844111d70cf11210c1926b601e7d05c.
---
 CHANGELOG.md                                  | 39 ---------
 docs/spec/bundle-and-cache.md                 | 27 +------
 src/amplifier_agent_lib/_runtime.py           |  8 --
 src/amplifier_agent_lib/bundle/bundle.md      | 15 ----
 .../bundle/modes/brainstorm.md                |  4 -
 src/amplifier_agent_lib/bundle/modes/plan.md  |  4 -
 src/amplifier_agent_lib/shell_tool.py         | 81 -------------------
 7 files changed, 1 insertion(+), 177 deletions(-)
 delete mode 100644 src/amplifier_agent_lib/shell_tool.py

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4edf3fcb..887e3f5b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,48 +27,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   selects the server, and `CHAT_COMPLETIONS_API_KEY` (optional) is sent only when set,
   since local servers commonly need none. Both are environment-only; the persisted
   credentials file is not consulted for this provider. Default model is `default`.
-- **PowerShell shell tool on Windows.** `bundle.md` now declares both
-  `tool-bash` and `tool-pwsh` (from `amplifier-bundle-windows-shell`, pinned to
-  a commit rather than `@main` because that repo is a single-commit
-  proof-of-concept with no tags and no CI). Exactly one is mounted, chosen by
-  `amplifier_agent_lib/shell_tool.py` at the bundle-prep seam: `tool-pwsh` on
-  Windows, `tool-bash` everywhere else.
-
-  The swap is not a rename. The tool is named `pwsh`, because the tool name is a
-  strong prior on the syntax the model emits, so the model gets a PowerShell
-  tool rather than a `bash` tool with a surprising backend. The cost is that
-  anything matching the shell by literal tool name has to name both: the shipped
-  `plan` and `brainstorm` modes now list `bash` and `pwsh` in their tool
-  policies, which they must, or the shell would be unpoliced on Windows.
-
-  Declaring both is what makes the swap expressible at all -- the manifest is
-  static and its sha256 is the prepared-cache key, so there is no conditional
-  form available. Both are therefore installed during cold-prepare, costing one
-  extra clone of a zero-dependency package on POSIX. Nothing changes for
-  existing POSIX users beyond that clone.
 
 ### Fixed
 
-- **The shell tool was unusable on Windows, and failed in a way that produced
-  confidently wrong answers.** `tool-bash` resolves its shell with
-  `shutil.which("bash")`. Git for Windows installs `bash.exe` into
-  `C:\Program Files\Git\bin` but only adds `C:\Program Files\Git\cmd` to PATH,
-  so the lookup returns `None` and commands fall through to a branch that tries
-  to exec the command name as a binary. Driving the tool directly on a Windows
-  guest returns `success=False` with
-  `[WinError 2] The system cannot find the file specified` -- no mention of
-  bash, no remedy, nothing the model can act on.
-
-  The original report described the model retrying and concluding the tool was
-  broken. What we measured is worse: given that error the model did not report a
-  failure at all, it fabricated a plausible working directory and presented it
-  as command output. A tool that fails unactionably does not reliably produce a
-  visible error; it can produce a silent wrong answer.
-
-  Fixed by mounting `tool-pwsh` instead on Windows (see Added). PowerShell needs
-  no PATH repair: `tool-pwsh` finds PowerShell 7 when present and falls back to
-  Windows PowerShell 5.1, which ships with the OS.
-
 - **`amplifier-agent run` crashed with `UnicodeEncodeError` after the turn had
   already completed.** Python picks the console code page for stdio, which on
   Windows is a legacy single-byte encoding (cp1252 on the guests we test), so
diff --git a/docs/spec/bundle-and-cache.md b/docs/spec/bundle-and-cache.md
index 3c06e5d1..53787a53 100644
--- a/docs/spec/bundle-and-cache.md
+++ b/docs/spec/bundle-and-cache.md
@@ -38,8 +38,7 @@ session.context:      context-simple       max_tokens 300000, auto_compact
 session.provider:     the anthropic provider, as the runtime default entry
 
 tools:
-  tool-filesystem, tool-web, tool-search, tool-todo, tool-apply-patch,
-  tool-bash | tool-pwsh  (both declared, exactly one mounted -- see below),
+  tool-filesystem, tool-bash, tool-web, tool-search, tool-todo, tool-apply-patch,
   tool-delegate (self_delegation, session_resume, context_inheritance, provider_selection;
                  excludes tool-delegate from sub-agents),
   tool-mcp, tool-skills, tool-mode, tool-recipes
@@ -56,30 +55,6 @@ Agents declare no `tools:` blocks; they inherit the parent tool roster through t
 `context_inheritance`. Modules referenced only by agent definitions are installed alongside the
 top-level ones, so a delegated session can always mount what its agent declares.
 
-### Shell tool selection
-
-The manifest declares both shell modules; exactly one reaches the kernel. The selection runs at the
-bundle-prep seam, before host-config merge, so every later step and the kernel see the final roster:
-
-```
-Windows    tool-pwsh mounted, tool-bash dropped
-otherwise  tool-bash mounted, tool-pwsh dropped
-```
-
-Both are declared because the manifest is static and its sha256 is the cache key, so there is no
-conditional form available here. Both are therefore also installed during cold-prepare, which costs
-one extra clone of a zero-dependency package on POSIX.
-
-The swap is not a rename: the tool is named `pwsh`, and the model sees a PowerShell tool rather than
-a `bash` tool with a different backend. The name is a strong prior on the syntax the model emits, so
-this is deliberate. The consequence is that anything matching the shell by literal tool name must
-name both -- the shipped modes list `bash` and `pwsh` in their tool policies for exactly this
-reason. Sub-agents are unaffected: they inherit the already-filtered parent roster.
-
-On Windows the selection is unconditional and does not probe for PowerShell first. Windows PowerShell
-5.1 is present on every supported Windows install, and `tool-pwsh` falls back to it when PowerShell 7
-is absent.
-
 Four upstream modules are deliberately absent relative to the upstream behavioral-anchor bundle:
 `hooks-streaming-ui` and `hooks-todo-display` would break the JSON-stdout contract,
 `behaviors/logging.yaml` is replaced by `hook-context-intelligence`, and `hooks-approval` is dropped
diff --git a/src/amplifier_agent_lib/_runtime.py b/src/amplifier_agent_lib/_runtime.py
index 5b898dd4..e3ae9600 100644
--- a/src/amplifier_agent_lib/_runtime.py
+++ b/src/amplifier_agent_lib/_runtime.py
@@ -27,7 +27,6 @@
 from amplifier_agent_lib.incremental_save import IncrementalSaveHook
 from amplifier_agent_lib.persistence import state_root
 from amplifier_agent_lib.session_store import SessionStore
-from amplifier_agent_lib.shell_tool import select_shell_tool
 from amplifier_agent_lib.skill_dispatch import USER_TURN_ROLE, dispatch_skill_or_execute
 from amplifier_agent_lib.wire_approval_provider import WireApprovalProvider
 
@@ -119,13 +118,6 @@ def prepare_bundle_for_session(
     the prepared bundle's mount_plan BEFORE calling. A future clone-return
     variant is on the design backlog.
     """
-    # Shell tool selection. bundle.md declares both tool-bash and tool-pwsh;
-    # exactly one belongs on this platform. Runs FIRST so every step below --
-    # and the kernel -- sees the final tool roster rather than one that still
-    # contains a module about to be removed. See shell_tool.py for why the
-    # manifest declares both.
-    select_shell_tool(prepared.mount_plan or {})
-
     # D4: mcp.configPath -> AMPLIFIER_MCP_CONFIG env var.
     mcp_block = (host_config or {}).get("mcp")
     if isinstance(mcp_block, dict):
diff --git a/src/amplifier_agent_lib/bundle/bundle.md b/src/amplifier_agent_lib/bundle/bundle.md
index de6ce6ab..cca2655a 100644
--- a/src/amplifier_agent_lib/bundle/bundle.md
+++ b/src/amplifier_agent_lib/bundle/bundle.md
@@ -108,23 +108,8 @@ tools:
   # Core tools (inherited by all sub-agents via tool-delegate)
   - module: tool-filesystem
     source: git+https://github.com/microsoft/amplifier-module-tool-filesystem@main
-  # Shell. BOTH are declared; exactly one is mounted per platform by
-  # amplifier_agent_lib/shell_tool.py, which runs at the bundle-prep seam:
-  #   Windows   -> tool-pwsh    (tool-bash cannot find a shell there)
-  #   elsewhere -> tool-bash    (tool-pwsh is dropped)
-  # Declaring both is what makes the swap possible at all: this manifest is
-  # static and its sha256 is the prepared-cache key, so there is no conditional
-  # form to express "Windows only" here. The cost is one extra clone of a
-  # zero-dependency package during cold-prepare on POSIX.
   - module: tool-bash
     source: git+https://github.com/microsoft/amplifier-module-tool-bash@main
-  # Pinned to a commit, not @main: amplifier-bundle-windows-shell is a
-  # single-commit proof-of-concept with no tags and no CI, so @main would let an
-  # upstream force-push change what users install with no signal here.
-  - module: tool-pwsh
-    source: git+https://github.com/microsoft/amplifier-bundle-windows-shell@fd58d71418304da9116c92bc9211986f1a7b19c8#subdirectory=modules/tool-pwsh
-    config:
-      safety_profile: standard
   - module: tool-web
     source: git+https://github.com/microsoft/amplifier-module-tool-web@main
   - module: tool-search
diff --git a/src/amplifier_agent_lib/bundle/modes/brainstorm.md b/src/amplifier_agent_lib/bundle/modes/brainstorm.md
index 878d145d..6ed050ac 100644
--- a/src/amplifier_agent_lib/bundle/modes/brainstorm.md
+++ b/src/amplifier_agent_lib/bundle/modes/brainstorm.md
@@ -17,11 +17,7 @@ mode:
       - delegate
       - recipes
     warn:
-      # Both shell tools. Only one is ever mounted (tool-bash on POSIX,
-      # tool-pwsh on Windows), but tool policies match on the TOOL NAME, so
-      # naming only `bash` would leave the shell unpoliced on Windows.
       - bash
-      - pwsh
 
   default_action: block
 ---
diff --git a/src/amplifier_agent_lib/bundle/modes/plan.md b/src/amplifier_agent_lib/bundle/modes/plan.md
index 1c4da254..ff8fee34 100644
--- a/src/amplifier_agent_lib/bundle/modes/plan.md
+++ b/src/amplifier_agent_lib/bundle/modes/plan.md
@@ -18,11 +18,7 @@ mode:
       - delegate
       - recipes
     warn:
-      # Both shell tools. Only one is ever mounted (tool-bash on POSIX,
-      # tool-pwsh on Windows), but tool policies match on the TOOL NAME, so
-      # naming only `bash` would leave the shell unpoliced on Windows.
       - bash
-      - pwsh
   
   default_action: block
 ---
diff --git a/src/amplifier_agent_lib/shell_tool.py b/src/amplifier_agent_lib/shell_tool.py
deleted file mode 100644
index ec6ec0b5..00000000
--- a/src/amplifier_agent_lib/shell_tool.py
+++ /dev/null
@@ -1,81 +0,0 @@
-"""Platform selection for the shell tool.
-
-``bundle.md`` declares BOTH shell modules. Exactly one survives into any given
-session's mount plan, and this module is what picks it:
-
-    Windows   ->  tool-pwsh    (PowerShell, from amplifier-bundle-windows-shell)
-    elsewhere ->  tool-bash
-
-Why declare both and filter, rather than declare one conditionally: the bundle
-manifest is static and its sha256 IS the prepared-cache key, so there is no
-conditional syntax to hang this on. Declaring both also means
-``bundle.prepare(install_deps=True)`` installs both, which costs one extra clone
-of a zero-dependency package on POSIX and buys a mount plan that cannot be
-missing its shell tool on either platform.
-
-Why swap rather than mount both: on Windows ``tool-bash`` does not find a shell.
-Git for Windows ships ``bash.exe`` in a directory its installer leaves off PATH,
-so ``shutil.which("bash")`` returns None and commands fall through to a branch
-that tries to exec ``pwd`` as a binary. The model then sees an unactionable
-``[WinError 2]`` and concludes the tool is broken. Leaving both mounted would
-keep that trap one wrong choice away, and would spend context on two shell tool
-descriptions to do it.
-
-The name difference is deliberate upstream and is load-bearing here. The tool is
-``pwsh``, not ``bash`` backed by PowerShell, because the tool name is a strong
-prior on the SYNTAX the model emits. Anything that refers to the shell tool by
-literal name therefore has to name both; the shipped modes do (see
-``bundle/modes/*.md``).
-"""
-
-from __future__ import annotations
-
-import logging
-import sys
-from typing import Any
-
-logger = logging.getLogger(__name__)
-
-__all__ = ["POSIX_SHELL_MODULE", "WINDOWS_SHELL_MODULE", "select_shell_tool"]
-
-WINDOWS_SHELL_MODULE = "tool-pwsh"
-POSIX_SHELL_MODULE = "tool-bash"
-
-
-def select_shell_tool(mount_plan: dict[str, Any], *, is_windows: bool | None = None) -> str | None:
-    """Drop the shell module that does not belong on this platform, in place.
-
-    Args:
-        mount_plan: A mount plan dict. ``mount_plan["tools"]`` is a list of
-            ``{module, config, source}`` entries, the shape
-            ``Bundle.to_mount_plan()`` produces.
-        is_windows: Platform override, for tests. Defaults to the real platform.
-
-    Returns:
-        The module id that was dropped, or ``None`` if nothing was dropped
-        (either it was not declared, or the plan has no tools).
-
-    Tolerates a manifest that declares neither module, or only the one being
-    kept. A missing entry is not an error here: this function's job is to
-    remove, and an operator who has already narrowed the roster has made a
-    choice worth respecting rather than second-guessing at mount time.
-    """
-    if is_windows is None:
-        is_windows = sys.platform == "win32"
-
-    drop = POSIX_SHELL_MODULE if is_windows else WINDOWS_SHELL_MODULE
-    tools = mount_plan.get("tools")
-    if not isinstance(tools, list):
-        return None
-
-    remaining = [entry for entry in tools if entry.get("module") != drop]
-    if len(remaining) == len(tools):
-        return None
-
-    # Mutate the SAME list object. Callers hold references to
-    # ``prepared.mount_plan["tools"]`` and rebinding the key would leave them
-    # pointing at the unfiltered list.
-    tools[:] = remaining
-    kept = WINDOWS_SHELL_MODULE if is_windows else POSIX_SHELL_MODULE
-    logger.info("shell tool: kept %s, dropped %s (platform=%s)", kept, drop, sys.platform)
-    return drop

From 2193283b96d4a7e26c5e1431580fece3b5cc1436 Mon Sep 17 00:00:00 2001
From: sadlilas <11658960+sadlilas@users.noreply.github.com>
Date: Tue, 18 Aug 2026 12:32:11 -0700
Subject: [PATCH 6/7] docs: declare git as a prerequisite and check for it in
 install.sh

git was a silent, undocumented requirement. install.sh checked for uv and
curl but not git, and neither README.md nor docs/INSTALL.md listed it.

git is not a build-time nicety. Bundles and modules are fetched by cloning
git repositories, so a machine without git on PATH fails twice: while
priming the bundle cache during install, and every subsequent time a bundle
is mounted at run time. On a bare Windows host, where git is not present by
default, this surfaced as an opaque clone failure that named neither the
missing dependency nor the remedy.

- install.sh now refuses up front when git is absent, with a per-platform
  install hint (xcode-select / package manager / Git for Windows).
- README.md and docs/INSTALL.md list git alongside uv and curl.
- docs/INSTALL.md notes that git is needed at run time, and that Git for
  Windows supplies both git and the bash the shell tool looks for.

Co-Authored-By: Amplifier <amplifier@microsoft.com>
---
 CHANGELOG.md    | 12 ++++++++++++
 README.md       |  2 +-
 docs/INSTALL.md |  4 +++-
 install.sh      | 15 +++++++++++++++
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 887e3f5b..239b2ecb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,6 +30,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **`git` was a silent, undocumented requirement.** The installer checked for
+  `uv` and `curl` but not `git`, and neither `README.md` nor `docs/INSTALL.md`
+  listed it. `git` is not a build-time nicety: bundles and modules are fetched
+  by cloning git repositories, so a machine without `git` on `PATH` fails both
+  while priming the cache during install and every subsequent time a bundle is
+  mounted. On a bare Windows host -- where `git` is not present by default --
+  this surfaced as an opaque clone failure with no statement of the missing
+  dependency. `install.sh` now refuses up front with a per-platform install
+  hint, and both `README.md` and `docs/INSTALL.md` list `git` alongside `uv`
+  and `curl`, noting that it is needed at run time and that Git for Windows
+  supplies both `git` and the `bash` the shell tool looks for.
+
 - **`amplifier-agent run` crashed with `UnicodeEncodeError` after the turn had
   already completed.** Python picks the console code page for stdio, which on
   Windows is a legacy single-byte encoding (cp1252 on the guests we test), so
diff --git a/README.md b/README.md
index e6f82a79..b482b6fd 100644
--- a/README.md
+++ b/README.md
@@ -55,7 +55,7 @@ Public integrations run opencode, paperclip, and NanoClaw on it: see [who has in
 curl -fsSL https://raw.githubusercontent.com/microsoft/amplifier-agent/main/install.sh | bash
 ```
 
-Installs the latest release and primes the bundle cache so your first run is instant. Requires [`uv`](https://docs.astral.sh/uv/) and `curl`; the installer tells you what is missing rather than bootstrapping silently.
+Installs the latest release and primes the bundle cache so your first run is instant. Requires [`uv`](https://docs.astral.sh/uv/), `curl`, and `git`; the installer tells you what is missing rather than bootstrapping silently. `git` is a runtime dependency too -- bundles and modules are fetched by cloning git repositories.
 
 To review the script first, pin a version, install without the script, or uninstall, see [`docs/INSTALL.md`](docs/INSTALL.md).
 
diff --git a/docs/INSTALL.md b/docs/INSTALL.md
index a6cfdee5..991c6f30 100644
--- a/docs/INSTALL.md
+++ b/docs/INSTALL.md
@@ -2,7 +2,9 @@
 
 `amplifier-agent` is a Python tool installed with [`uv`](https://docs.astral.sh/uv/). The installer resolves the latest tagged release and installs from it.
 
-**Prerequisites:** `uv` and `curl`. The installer tells you exactly what to install if either is missing. It will not bootstrap them silently.
+**Prerequisites:** `uv`, `curl`, and `git`. The installer tells you exactly what to install if any is missing. It will not bootstrap them silently.
+
+`git` is needed at run time, not just at install time: bundles and modules are fetched by cloning git repositories, so a machine without `git` on `PATH` can neither prime the cache nor mount a bundle. On Windows, installing [Git for Windows](https://git-scm.com/download/win) satisfies this and also provides the `bash` that the shell tool looks for.
 
 ## Recommended
 
diff --git a/install.sh b/install.sh
index a10b2efc..654fa412 100755
--- a/install.sh
+++ b/install.sh
@@ -125,6 +125,21 @@ if ! command -v curl > /dev/null 2>&1; then
     exit 1
 fi
 
+# Require git -- the bundle system clones module and bundle repositories, both
+# while priming the cache below and every time a bundle is mounted at run time.
+if ! command -v git > /dev/null 2>&1; then
+    printf 'error: git is required but not found on PATH.\n\n' >&2
+    printf 'amplifier-agent fetches modules and bundles by cloning git repositories,\n' >&2
+    printf 'both while priming the cache during this install and every time a bundle\n' >&2
+    printf 'is mounted at run time. It is a runtime dependency, not just a build one.\n\n' >&2
+    printf 'Install git:\n' >&2
+    printf '  macOS:    xcode-select --install\n' >&2
+    printf '  Linux:    your package manager, e.g. apt install git\n' >&2
+    printf '  Windows:  https://git-scm.com/download/win (Git for Windows)\n\n' >&2
+    printf 'Then re-run this script.\n' >&2
+    exit 1
+fi
+
 # Require uv — never silently bootstrap it; direct the user instead.
 if ! command -v uv > /dev/null 2>&1; then
     printf 'error: uv is required but not found on PATH.\n\n' >&2

From e79afaf6949b90ad5ac98efb69c030cb48df85ba Mon Sep 17 00:00:00 2001
From: sadlilas <11658960+sadlilas@users.noreply.github.com>
Date: Tue, 18 Aug 2026 12:43:58 -0700
Subject: [PATCH 7/7] docs: correct the git-prerequisite note about what the
 installer check covers

The prior wording juxtaposed the bare-Windows failure with the new
install.sh check, implying the installer is what rescues that user. It
cannot be: install.sh is a bash script, and on Windows bash is supplied
by Git for Windows, which also supplies git -- so anyone able to run the
installer already has git and the check never fires for them.

The README and INSTALL.md entries are what carry this on Windows. The
install.sh check covers the environments that genuinely have bash without
git: minimal Linux containers, slim CI images, and fresh WSL distributions.

Co-Authored-By: Amplifier <amplifier@microsoft.com>
---
 CHANGELOG.md | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 239b2ecb..93c7b74b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,10 +37,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   while priming the cache during install and every subsequent time a bundle is
   mounted. On a bare Windows host -- where `git` is not present by default --
   this surfaced as an opaque clone failure with no statement of the missing
-  dependency. `install.sh` now refuses up front with a per-platform install
-  hint, and both `README.md` and `docs/INSTALL.md` list `git` alongside `uv`
-  and `curl`, noting that it is needed at run time and that Git for Windows
-  supplies both `git` and the `bash` the shell tool looks for.
+  dependency.
+
+  `README.md` and `docs/INSTALL.md` now list `git` alongside `uv` and `curl`,
+  noting that it is needed at run time and that Git for Windows supplies both
+  `git` and the `bash` the shell tool looks for. The documentation is what
+  carries this on Windows: `install.sh` is a bash script, and on Windows bash
+  is itself supplied by Git for Windows, so a user who can run the installer
+  necessarily already has `git`. `install.sh` also gained an up-front check
+  with a per-platform install hint, which covers the environments where bash
+  is present without `git` -- minimal Linux containers, slim CI images, and
+  fresh WSL distributions.
 
 - **`amplifier-agent run` crashed with `UnicodeEncodeError` after the turn had
   already completed.** Python picks the console code page for stdio, which on