fix: read Windows-authored agent .md + credentials.json as UTF-8

src/amplifier_agent_cli/admin/auth.py

@@ -95,13 +95,39 @@ def _load_credentials() -> dict[str, Any]:
     Tolerant of legacy shapes: if the existing file is missing the
     ``version`` envelope, treat the whole body as the ``providers`` dict
     and silently upgrade on next write. Raises ``click.ClickException``
-    on JSON-decode failure with a clear remediation hint.
+    with a clear remediation hint on either failure mode -- bytes that
+    are not valid UTF-8, or text that is not valid JSON. Neither is
+    allowed to surface as a raw traceback.
     """
     path = credentials_path()
     if not path.exists():
         return {"version": CREDENTIALS_VERSION, "providers": {}}
     try:
-        data = json.loads(path.read_text() or "{}")
+        # encoding="utf-8-sig", matching hydrate_agent_overlay: a file touched
+        # by Windows tooling (Notepad's "UTF-8 with BOM", Windows PowerShell
+        # 5.1 Set-Content/Out-File) is UTF-8 WITH a BOM. Read as plain "utf-8"
+        # the leading U+FEFF survives into the string and json.loads then dies
+        # with "Unexpected UTF-8 BOM (decode using utf-8-sig)" -- surfaced to
+        # the user as "not valid JSON", which is both wrong and unactionable.
+        # utf-8-sig strips a leading BOM if present and is identical to utf-8
+        # for files without one. We always write plain utf-8 (_atomic_write),
+        # so this only ever forgives a file some other tool re-saved.
+        raw = path.read_text(encoding="utf-8-sig")
+    except UnicodeDecodeError as exc:
+        # Reachable whenever the bytes on disk are not valid UTF-8: a file
+        # written by an older build under a non-UTF-8 locale default (Windows
+        # cp1252), hand-edited in a legacy encoding, or truncated mid-sequence.
+        # Without this branch the UnicodeDecodeError escapes uncaught and
+        # every `auth` command -- plus each provider credential lookup behind
+        # `run`/`models`/`serve` -- dies with a raw traceback.
+        raise click.ClickException(
+            f"Credentials file at {path} is not valid UTF-8 ({exc}). "
+            "It was likely written by an older build under a non-UTF-8 locale. "
+            "Re-save the file as UTF-8, or clear all stored credentials with "
+            "`amplifier-agent auth clear --force` and re-add them."
+        ) from exc
+    try:
+        data = json.loads(raw or "{}")
     except json.JSONDecodeError as exc:
         raise click.ClickException(
             f"Credentials file at {path} is not valid JSON ({exc}). "
@@ -139,7 +165,7 @@ def _atomic_write(path: Path, payload: dict[str, Any]) -> None:
         pass
 
     tmp_path = path.with_suffix(path.suffix + ".tmp")
-    tmp_path.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n")
+    tmp_path.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n", encoding="utf-8")
     os.chmod(tmp_path, CREDENTIALS_FILE_MODE)
     os.replace(tmp_path, path)
 
@@ -159,25 +185,51 @@ def _save_credentials(data: dict[str, Any]) -> Path:
 # Resolver helper (consumed by provider_sources._resolve_env_credential)
 # ---------------------------------------------------------------------------
 
+# Module-level latch for _warn_credentials_unreadable_once. A single command
+# resolves credentials once per provider (and Azure-style providers resolve an
+# extra endpoint field), so an unlatched warning would print a dozen identical
+# lines for one broken file.
+_credentials_warning_emitted = False
+
+
+def _warn_credentials_unreadable_once(exc: click.ClickException) -> None:
+    """Surface an unreadable credentials file to the user exactly once.
+
+    The resolvers below deliberately never raise: one bad write must not
+    brick every subsequent invocation. But degrading silently is its own
+    failure -- with only a DEBUG log, a corrupt file makes every provider
+    report ``<not set>`` and exit 0, which reads as "no credentials
+    configured" rather than "your credentials file is broken." The user
+    then re-adds keys that were never actually lost.
+
+    So: resilient *and* visible. Emit the remediation hint (which carries
+    the path and the specific parse failure) once per process on stderr,
+    leaving stdout clean for callers that parse it.
+    """
+    global _credentials_warning_emitted
+    logger.debug("credentials.json unreadable; resolving as empty (%s)", exc.message)
+    if _credentials_warning_emitted:
+        return
+    _credentials_warning_emitted = True
+    click.echo(f"Warning: {exc.message}", err=True)
+
 
 def resolve_credential_from_file(provider_name: str) -> str:
     """Look up ``provider_name``'s ``api_key`` in the credentials file.
 
     Returns ``""`` if no file exists or the entry is missing. Never
-    raises -- a malformed file is logged at DEBUG and treated as empty,
-    so a one-time bad write doesn't break every subsequent invocation.
+    raises -- a malformed file is treated as empty so a one-time bad
+    write doesn't break every subsequent invocation -- but the failure is
+    reported once per process via :func:`_warn_credentials_unreadable_once`
+    so the degradation is never silent.
 
     The caller (``_resolve_env_credential``) chains this AFTER the env var
     lookup so shell env always wins.
     """
     try:
         data = _load_credentials()
     except click.ClickException as exc:
-        logger.debug(
-            "credentials.json unreadable; resolving %r as empty (%s)",
-            provider_name,
-            exc.message,
-        )
+        _warn_credentials_unreadable_once(exc)
         return ""
     providers = data.get("providers") or {}
     entry = providers.get(provider_name) or {}
@@ -191,11 +243,13 @@ def resolve_field_from_file(provider_name: str, field: str) -> str:
     """Read an arbitrary string field for a provider entry.
 
     Used by Azure-style providers that store endpoint URLs alongside
-    the api_key. Returns ``""`` when absent.
+    the api_key. Returns ``""`` when absent. Shares the never-raise /
+    warn-once contract of :func:`resolve_credential_from_file`.
     """
     try:
         data = _load_credentials()
-    except click.ClickException:
+    except click.ClickException as exc:
+        _warn_credentials_unreadable_once(exc)
         return ""
     providers = data.get("providers") or {}
     entry = providers.get(provider_name) or {}

src/amplifier_agent_lib/spawn.py

@@ -102,7 +102,13 @@ def hydrate_agent_overlay(agent_md_path: Path) -> dict[str, Any]:
     """
     import yaml
 
-    text = agent_md_path.read_text(encoding="utf-8")
+    # encoding="utf-8-sig": a Windows-authored agent .md (Notepad, PowerShell
+    # Out-File/Set-Content) is UTF-8 WITH a BOM. Read as plain "utf-8" the
+    # leading U+FEFF survives, so `text.startswith("---")` is False and the
+    # whole file is silently treated as a plain instruction -- tools, hooks,
+    # model_role, meta are dropped with no error. utf-8-sig strips a leading BOM
+    # if present and is identical to utf-8 for files without one.
+    text = agent_md_path.read_text(encoding="utf-8-sig")
 
     # Split on the YAML frontmatter delimiters: ---\nYAML\n---
     if not text.startswith("---"):