fix(cli): resolve doctor's glyph set per console encoding (#401)

CHANGELOG.md

@@ -40,6 +40,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   as fatal.
 - `runtime.default_reasoning_effort` was silently dropped at run time for every
   provider and is now forwarded through `ProviderRegistry`.
+- **`conductor doctor`'s table output no longer dies part-written on a
+  `cp1252` console** (#401). The Installed/Credentials/Connection/Models
+  columns hardcoded `✓`/`✗`/`○`/`⚠`, none of which cp1252 can encode, so a
+  run on a legacy Windows console raised `UnicodeEncodeError` mid-table,
+  after the Environment section had already printed. `conductor doctor`
+  now resolves each glyph once per invocation against the output console's
+  stream encoding, falling back to `OK`/`X`/`o`/`!` when the Unicode
+  glyphs cannot be encoded; the `--json` path was already safe and is
+  unchanged.
 - **MCP tool discovery and structured tool results no longer break with MCP
   2.0** (#419). MCP 2.0 renamed the Python field on `mcp.types.Tool` from
   `inputSchema` to `input_schema` and on `mcp.types.CallToolResult` from

src/conductor/cli/doctor.py

@@ -12,7 +12,7 @@
 import contextlib
 import logging
 from collections.abc import Iterator
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, NamedTuple
 
 from rich.table import Table
 from rich.text import Text
@@ -35,20 +35,75 @@
     from conductor.providers.diagnostics import Section
 
 
-_CHECK = Text.from_markup("[green]✓[/green]")
-_CROSS = Text.from_markup("[red]✗[/red]")
-_DASH = Text.from_markup("[dim]—[/dim]")
-_OPTIONAL_MARK = "○"
-"""Neutral glyph for an absent *optional* credential — deliberately not the
-red ``✗`` used for a genuinely missing required credential (issue #319).
+class _Glyphs(NamedTuple):
+    """The status glyphs a table render uses, resolved for one console."""
 
-.. note::
-   ``✓``, ``✗`` and ``○`` are not encodable in cp1252, so the default *table*
-   output of ``conductor doctor`` still fails on such a console — see #401.
-   This module's ``--json`` path is safe (``ensure_ascii=True``); the table path
-   is deliberately out of scope here because every glyph consumer would need the
-   console threaded through it. The em-dash is encodable in cp1252.
-"""
+    check: Text
+    cross: Text
+    dash: Text
+    warn: Text
+    optional: str
+    """Neutral glyph for an absent *optional* credential — deliberately not
+    ``cross``, which is reserved for a genuinely missing required credential
+    (issue #319)."""
+
+
+_UNICODE_GLYPHS = _Glyphs(
+    check=Text.from_markup("[green]✓[/green]"),
+    cross=Text.from_markup("[red]✗[/red]"),
+    dash=Text.from_markup("[dim]—[/dim]"),
+    warn=Text.from_markup("[yellow]⚠[/yellow]"),
+    optional="○",
+)
+_ASCII_GLYPHS = _Glyphs(
+    check=Text.from_markup("[green]OK[/green]"),
+    cross=Text.from_markup("[red]X[/red]"),
+    dash=Text.from_markup("[dim]-[/dim]"),
+    warn=Text.from_markup("[yellow]![/yellow]"),
+    optional="o",
+)
+
+
+def _encodable(text: str, encoding: str | None) -> bool:
+    """Whether *text* can be encoded to *encoding*.
+
+    A falsy ``encoding`` is treated as capable so an in-memory buffer is not
+    needlessly downgraded. ``io.StringIO`` has an ``.encoding`` of ``None``;
+    rich's ``NULL_FILE`` has no such attribute at all. This is a deliberate
+    fail-open: a stream that is lossy *and* silent about its encoding (e.g.
+    ``codecs.getwriter``) will still raise.
+    """
+    if not encoding:
+        return True
+    try:
+        text.encode(encoding)
+    except (UnicodeEncodeError, LookupError):
+        return False
+    return True
+
+
+def _resolve_glyphs(console: MarkupFreeConsole) -> _Glyphs:
+    """Pick Unicode or ASCII-safe glyphs for *console*'s stream encoding.
+
+    Rich hands a rendered line straight to the underlying file's ``write()``;
+    it does not check whether the target encoding can represent it. A legacy
+    Windows console (``cp1252``) cannot encode ``✓``/``✗``/``○``/``⚠``, so the
+    table dies mid-write, part-printed (issue #401). Resolved once per
+    ``run_doctor`` call and passed down rather than re-checked per cell, so
+    every cell in one report agrees.
+
+    Probed per glyph rather than through rich's ``ConsoleOptions.ascii_only``,
+    which is a ``startswith("utf")`` prefix test: ``gb18030`` encodes all of
+    these and that check would downgrade it for nothing.
+    """
+    encoding = console.encoding
+    return _Glyphs(
+        check=_UNICODE_GLYPHS.check if _encodable("✓", encoding) else _ASCII_GLYPHS.check,
+        cross=_UNICODE_GLYPHS.cross if _encodable("✗", encoding) else _ASCII_GLYPHS.cross,
+        dash=_UNICODE_GLYPHS.dash if _encodable("—", encoding) else _ASCII_GLYPHS.dash,
+        warn=_UNICODE_GLYPHS.warn if _encodable("⚠", encoding) else _ASCII_GLYPHS.warn,
+        optional=_UNICODE_GLYPHS.optional if _encodable("○", encoding) else _ASCII_GLYPHS.optional,
+    )
 
 
 def run_doctor(
@@ -109,14 +164,15 @@ def run_doctor(
         console.print_json(data=report.to_dict(), ensure_ascii=True)
         return _compute_exit_code(report.providers, check=check, provider=provider)
 
+    glyphs = _resolve_glyphs(console)
     if report.env is not None:
         _render_env(report.env, console)
     if report.providers is not None:
-        _render_providers(report.providers, console, check=check, models=models)
+        _render_providers(report.providers, console, glyphs, check=check, models=models)
         if models:
-            _render_models(report.providers, console)
+            _render_models(report.providers, console, glyphs)
     if report.registries is not None:
-        _render_registries(report.registries, console)
+        _render_registries(report.registries, console, glyphs)
 
     return _compute_exit_code(report.providers, check=check, provider=provider)
 
@@ -204,6 +260,7 @@ def _render_env(env: EnvDiagnostic, console: Console) -> None:
 def _render_providers(
     providers: list[ProviderDiagnostic],
     console: MarkupFreeConsole,
+    glyphs: _Glyphs,
     *,
     check: bool,
     models: bool,
@@ -223,30 +280,30 @@ def _render_providers(
     for diag in providers:
         row = [
             diag.name,
-            _CHECK if diag.installed else _CROSS,
-            _tier_cell(diag.tier),
-            _credentials_cell(diag),
+            glyphs.check if diag.installed else glyphs.cross,
+            _tier_cell(diag.tier, glyphs),
+            _credentials_cell(diag, glyphs),
         ]
         if check:
-            row.append(_connection_cell(diag))
+            row.append(_connection_cell(diag, glyphs))
         if models:
-            row.append(_models_cell(diag))
-        row.append(diag.note or _DASH)
+            row.append(_models_cell(diag, glyphs))
+        row.append(diag.note or glyphs.dash)
         table.add_row(*row)
 
     console.print(table)
 
 
-def _tier_cell(tier: str | None) -> Text:
+def _tier_cell(tier: str | None, glyphs: _Glyphs) -> Text:
     """Format the tier cell."""
     if tier is None:
-        return _DASH
+        return glyphs.dash
     if tier == "experimental":
         return Text.from_markup("[yellow]experimental[/yellow]")
     return Text(tier)
 
 
-def _credentials_cell(diag: ProviderDiagnostic) -> Text:
+def _credentials_cell(diag: ProviderDiagnostic, glyphs: _Glyphs) -> Text:
     """Format credential env-var presence (presence only, never values).
 
     A present credential is a green ``✓``. An absent credential renders as a
@@ -257,32 +314,32 @@ def _credentials_cell(diag: ProviderDiagnostic) -> Text:
     accompanying auth-path note is surfaced in the Notes column (issue #319).
     """
     if not diag.credential_env_vars:
-        return _DASH
+        return glyphs.dash
     lines: list[Text] = []
     for cred in diag.credential_env_vars:
         if cred.present:
-            lines.append(styled("{} {}", _CHECK, cred.name))
+            lines.append(styled("{} {}", glyphs.check, cred.name))
         elif diag.credentials_optional:
-            lines.append(styled("[dim]{} {}[/dim]", _OPTIONAL_MARK, cred.name))
+            lines.append(styled("[dim]{} {}[/dim]", glyphs.optional, cred.name))
         else:
-            lines.append(styled("[dim]{} {}[/dim]", _CROSS, cred.name))
+            lines.append(styled("[dim]{} {}[/dim]", glyphs.cross, cred.name))
     return join("\n", lines)
 
 
-def _connection_cell(diag: ProviderDiagnostic) -> Text:
+def _connection_cell(diag: ProviderDiagnostic, glyphs: _Glyphs) -> Text:
     """Format the connection-check result cell."""
     if not diag.checked or diag.connection_ok is None:
-        return _DASH
+        return glyphs.dash
     if diag.connection_ok and diag.connection_note:
-        return styled("[yellow]⚠[/yellow] {}", diag.connection_note)
+        return styled("{} {}", glyphs.warn, diag.connection_note)
     if diag.connection_ok:
-        return styled("{} connected", _CHECK)
+        return styled("{} connected", glyphs.check)
     if diag.connection_error:
-        return styled("{} [dim]{}[/dim]", _CROSS, diag.connection_error)
-    return styled("{} [dim]connection failed[/dim]", _CROSS)
+        return styled("{} [dim]{}[/dim]", glyphs.cross, diag.connection_error)
+    return styled("{} [dim]connection failed[/dim]", glyphs.cross)
 
 
-def _models_cell(diag: ProviderDiagnostic) -> Text:
+def _models_cell(diag: ProviderDiagnostic, glyphs: _Glyphs) -> Text:
     """Format the models cell in the Providers summary table.
 
     Shows a count/status only — per-model reasoning-effort and
@@ -291,19 +348,19 @@ def _models_cell(diag: ProviderDiagnostic) -> Text:
     models is ``None`` (not enumerated), ``(none)`` for an empty list.
     """
     if diag.models_error:
-        return styled("{} [dim]{}[/dim]", _CROSS, diag.models_error)
+        return styled("{} [dim]{}[/dim]", glyphs.cross, diag.models_error)
     if diag.models is None:
         return Text.from_markup("[dim]n/a[/dim]")
     count = len(diag.models)
     if not count:
         return Text.from_markup("[dim](none)[/dim]")
-    return styled("{} {} model{}", _CHECK, count, "s" if count != 1 else "")
+    return styled("{} {} model{}", glyphs.check, count, "s" if count != 1 else "")
 
 
-def _format_tokens(value: int | None) -> Text:
+def _format_tokens(value: int | None, glyphs: _Glyphs) -> Text:
     """Format a token-limit value with grouped digits, or ``—`` when unknown."""
     if value is None:
-        return _DASH
+        return glyphs.dash
     return Text(f"{value:,}")
 
 
@@ -320,10 +377,10 @@ def _efforts_cell(model: ModelDiagnostic) -> Text:
     return Text(", ".join(model.supported_reasoning_efforts))
 
 
-def _default_effort_cell(model: ModelDiagnostic) -> Text:
+def _default_effort_cell(model: ModelDiagnostic, glyphs: _Glyphs) -> Text:
     """Format the default-reasoning-effort cell."""
     if model.default_reasoning_effort is None:
-        return _DASH
+        return glyphs.dash
     return Text(model.default_reasoning_effort)
 
 
@@ -337,20 +394,22 @@ def _default_effort_cell(model: ModelDiagnostic) -> Text:
 (issue #386), plus the synthetic ``"error"`` key used for ``None`` (pricing
 resolution itself failed). Built as module-level constants to avoid
 re-parsing the same markup literal on every table row (matching the
-``_CHECK``/``_CROSS``/``_DASH`` constants above) — each markup argument is
-still a literal template, not an interpolated value, keeping this inside the
-repo's console rules (see AGENTS.md "Console Output")."""
+``_UNICODE_GLYPHS``/``_ASCII_GLYPHS`` constants above) — each markup argument
+is still a literal template, not an interpolated value, keeping this inside
+the repo's console rules (see AGENTS.md "Console Output"). Every value here
+is pure ASCII, so no fallback applies: a property of these four literals,
+not a rule that anything outside :class:`_Glyphs` is safe to print."""
 
 
-def _rate_cell(value: float | None) -> Text:
+def _rate_cell(value: float | None, glyphs: _Glyphs) -> Text:
     """Format a per-Mtok rate, or ``—`` when unknown.
 
     Deliberately never renders ``0.00`` for ``None`` — a zero would read as
     "free", which is exactly the silent-wrong-number class of bug issue
     #386 is about.
     """
     if value is None:
-        return _DASH
+        return glyphs.dash
     return Text(f"{value:,.2f}")
 
 
@@ -368,7 +427,7 @@ def _pricing_source_cell(model: ModelDiagnostic) -> Text:
     return _PRICING_SOURCE_CELLS.get(model.pricing_source, Text(model.pricing_source))
 
 
-def _render_models(providers: list[ProviderDiagnostic], console: Console) -> None:
+def _render_models(providers: list[ProviderDiagnostic], console: Console, glyphs: _Glyphs) -> None:
     """Render a per-provider Models detail table (``--models`` only).
 
     One table per provider that returned at least one model, with columns
@@ -380,7 +439,7 @@ def _render_models(providers: list[ProviderDiagnostic], console: Console) -> Non
     for diag in providers:
         if not diag.models:
             continue
-        table = Table(title=f"Models — {diag.name}", show_lines=True)
+        table = Table(title=f"Models {glyphs.dash.plain} {diag.name}", show_lines=True)
         table.add_column("Model", style="cyan", no_wrap=True)
         table.add_column("Reasoning efforts")
         table.add_column("Default")
@@ -395,23 +454,23 @@ def _render_models(providers: list[ProviderDiagnostic], console: Console) -> Non
             table.add_row(
                 model.id,
                 _efforts_cell(model),
-                _default_effort_cell(model),
-                _format_tokens(model.max_prompt_tokens),
-                _format_tokens(model.max_output_tokens),
-                _format_tokens(model.max_context_window_tokens),
-                _rate_cell(model.input_per_mtok),
-                _rate_cell(model.output_per_mtok),
+                _default_effort_cell(model, glyphs),
+                _format_tokens(model.max_prompt_tokens, glyphs),
+                _format_tokens(model.max_output_tokens, glyphs),
+                _format_tokens(model.max_context_window_tokens, glyphs),
+                _rate_cell(model.input_per_mtok, glyphs),
+                _rate_cell(model.output_per_mtok, glyphs),
                 _pricing_source_cell(model),
             )
 
         console.print(table)
 
 
-def _render_registries(registries: RegistryDiagnostic, console: Console) -> None:
+def _render_registries(registries: RegistryDiagnostic, console: Console, glyphs: _Glyphs) -> None:
     """Render the registries section."""
     if registries.error is not None:
         console.print(
-            styled("{} [dim]failed to load registries: {}[/dim]", _CROSS, registries.error)
+            styled("{} [dim]failed to load registries: {}[/dim]", glyphs.cross, registries.error)
         )
         return
     if not registries.registries:
@@ -429,7 +488,7 @@ def _render_registries(registries: RegistryDiagnostic, console: Console) -> None
             reg.name,
             reg.type,
             reg.source,
-            _CHECK if reg.is_default else _DASH,
+            glyphs.check if reg.is_default else glyphs.dash,
         )
 
     console.print(table)