Skip to content
Merged
77 changes: 77 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,83 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
since local servers commonly need none. Both are environment-only; the persisted
credentials file is not consulted for this provider. Default model is `default`.

### 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.

`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
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
`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
(`__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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
4 changes: 3 additions & 1 deletion docs/INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions docs/spec/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>)`.
Expand Down
10 changes: 10 additions & 0 deletions docs/spec/storage-and-workspace.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

```
Expand Down Expand Up @@ -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:

```
Expand Down
7 changes: 7 additions & 0 deletions docs/spec/wrapper-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions src/amplifier_agent_cli/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

from __future__ import annotations

import os
import sys

import click
Expand Down Expand Up @@ -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:
Expand Down
20 changes: 20 additions & 0 deletions src/amplifier_agent_cli/admin/migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,33 @@

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,
)

__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",
Expand All @@ -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}"}))
Expand All @@ -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}"}))
Expand Down
25 changes: 17 additions & 8 deletions src/amplifier_agent_cli/modes/single_turn.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
34 changes: 33 additions & 1 deletion src/amplifier_agent_lib/migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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."""
Expand All @@ -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:
Expand Down