diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c5ad23a..e359d6f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 without waiting for a table refresh; providers whose SDK exposes no pricing (e.g. the Anthropic API) fall back to the table. ([#265](https://github.com/microsoft/conductor/issues/265)) +- **`conductor doctor` — provider & environment diagnostics** — a safe, + read-only command that reports which providers are installed, their + capability tier (`stable` / `experimental`), which credential environment + variables are detected (presence only — values are never printed), plus + Conductor version / update status and configured registries. Offline by + default; `--check` tests provider connections, `--models` lists available + models, `--provider NAME` scopes to one provider, and `--json` emits + machine-readable output for CI. Exit code is `1` only when `--check` is set + and the scoped provider (default `copilot`) fails to connect. Also adds a + public `list_models()` method to the provider interface (implemented for + Copilot and Claude). ([#274](https://github.com/microsoft/conductor/issues/274)) ### Fixed diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 5520670c..06e0f621 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -8,6 +8,7 @@ Complete command-line reference for Conductor. - [`conductor stop`](#conductor-stop) - [`conductor gate-respond`](#conductor-gate-respond) - [`conductor validate`](#conductor-validate) +- [`conductor doctor`](#conductor-doctor) - [`conductor registry`](#conductor-registry) ## `conductor run` @@ -325,6 +326,88 @@ for f in examples/*.yaml; do conductor validate "$f"; done **Warnings** (validation passes with notes): - **Undeclared dependencies in explicit mode** — agent prompt references `{{ a.output.val }}` but doesn't declare `a.output` in its `input:` list +## `conductor doctor` + +Report provider and environment diagnostics — a safe, read-only health check +for your Conductor setup. Answers "is my setup healthy?" without running a +workflow: which providers are installed, their stability tier, which +credential environment variables are detected, plus Conductor version / +update status and configured registries. + +```bash +conductor doctor [SECTION] [OPTIONS] +``` + +`SECTION` (optional positional) limits output to one of `providers`, +`registries`, or `env`. When omitted, all three sections are shown. + +**Offline by default** — no providers are instantiated and no credentials are +required. The only default network access is the GitHub-releases update check +in the `env` section (cache-first, short timeout, silent, and skipped when +`CONDUCTOR_NO_UPDATE_CHECK` is set). + +### Options + +| Option | Short | Description | +|--------|-------|-------------| +| `--check` | | Instantiate each provider and test its connection via `validate_connection()` (performs network I/O). | +| `--models` | | List each provider's available models (implies `--check`). | +| `--provider NAME` | `-p` | Scope the providers section to a single provider. | +| `--json` | | Emit machine-readable JSON instead of Rich tables (for CI). | + +### Sections + +- **env** — Conductor version, Python version, OS/platform, and update + availability. +- **providers** — for each known provider (`copilot`, `claude`, + `claude-agent-sdk`, `hermes`, `openai-agents`): whether the SDK is + installed, the capability tier (`stable` / `experimental`), which + credential environment variables are **present** (presence only — values + are never printed), and — with `--check` / `--models` — connection status + and available models. `openai-agents` is surfaced as "not yet implemented". +- **registries** — configured workflow registries and which is the default + (see [`conductor registry`](#conductor-registry)). + +### Credential detection + +Only the **presence** of credential environment variables is reported — +values are never read or printed. Detected variables per provider: + +| Provider | Environment variables | +|----------|-----------------------| +| `copilot` | `GITHUB_TOKEN`, `GH_TOKEN`, `COPILOT_PROVIDER_API_KEY`, `COPILOT_PROVIDER_BEARER_TOKEN` | +| `claude` | `ANTHROPIC_API_KEY`, `ANTHROPIC_AUTH_TOKEN` | +| `claude-agent-sdk` | `ANTHROPIC_API_KEY` | +| `hermes` | *(none — endpoint / API key are passed explicitly)* | + +### Exit codes + +- `0` — success (the default for offline runs; missing credentials for an + *optional* provider never fail the command). +- `1` — an invalid `SECTION`/`--provider` was given, **or** `--check` was set + and the **scoped** provider failed to connect. The scoped provider is the + one named by `--provider`, or `copilot` (the default) when `--provider` is + omitted. + +### Examples + +```bash +# Full offline report (all sections) +conductor doctor + +# Just the providers section +conductor doctor providers + +# Actually test provider connections (network) +conductor doctor --check + +# List available models for a single provider +conductor doctor --models --provider claude + +# Machine-readable output for CI +conductor doctor --json +``` + ## `conductor registry` Manage workflow registries — named sources (GitHub repos or local directories) for shared workflows. diff --git a/src/conductor/cli/app.py b/src/conductor/cli/app.py index 4e961447..160905cc 100644 --- a/src/conductor/cli/app.py +++ b/src/conductor/cli/app.py @@ -251,10 +251,12 @@ def main( if console.is_terminal and verbosity != ConsoleVerbosity.SILENT: import sys - # Skip when the subcommand is 'update' + # Skip when the subcommand is 'update' or 'doctor' — both surface + # update status in their own output (doctor in its env section), so + # the startup hint would be redundant noise. args = sys.argv[1:] subcommand = next((a for a in args if not a.startswith("-")), None) - if subcommand != "update": + if subcommand not in ("update", "doctor"): from conductor.cli.update import check_for_update_hint check_for_update_hint(console) @@ -1439,3 +1441,82 @@ def update( except Exception as e: print_error(e) raise typer.Exit(code=1) from None + + +@app.command() +def doctor( + section: Annotated[ + str | None, + typer.Argument( + help="Section to show: providers | registries | env. Default: all sections.", + ), + ] = None, + check: Annotated[ + bool, + typer.Option( + "--check", + help="Instantiate providers and test their connections (network).", + ), + ] = False, + models: Annotated[ + bool, + typer.Option( + "--models", + help="List available models for each provider (implies --check).", + ), + ] = False, + provider: Annotated[ + str | None, + typer.Option( + "--provider", + "-p", + help="Scope the providers section to a single provider.", + ), + ] = None, + as_json: Annotated[ + bool, + typer.Option( + "--json", + help="Emit machine-readable JSON instead of tables.", + ), + ] = False, +) -> None: + """Report provider & environment diagnostics. + + A safe, read-only health check for your Conductor setup: which providers + are installed, their stability tier, which credential environment + variables are detected (presence only — values are never printed), plus + Conductor version / update status and configured registries. + + Offline by default — no providers are instantiated and no credentials are + required. (The default env section does a cache-first GitHub update check; + set CONDUCTOR_NO_UPDATE_CHECK to disable it.) Use --check to actually test + provider connections, and --models to list each provider's available + models. + + \b + Examples: + conductor doctor # all sections + conductor doctor providers # providers section only + conductor doctor --check # test provider connections + conductor doctor --models -p claude # list Claude's models + conductor doctor --json # machine-readable output + """ + from conductor.cli.doctor import run_doctor + + try: + exit_code = run_doctor( + section=section, + provider=provider, + check=check, + models=models, + as_json=as_json, + console=output_console, + err_console=console, + ) + except Exception as e: + print_error(e) + raise typer.Exit(code=1) from None + + if exit_code != 0: + raise typer.Exit(code=exit_code) diff --git a/src/conductor/cli/doctor.py b/src/conductor/cli/doctor.py new file mode 100644 index 00000000..7dd69e82 --- /dev/null +++ b/src/conductor/cli/doctor.py @@ -0,0 +1,290 @@ +"""Implementation of the ``conductor doctor`` command. + +Renders the provider/environment diagnostics gathered by +:mod:`conductor.providers.diagnostics` as Rich tables (human-readable) or a +JSON document (``--json``, for CI). All data gathering lives in the +diagnostics module; this file is a thin, presentation-only layer. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import logging +from collections.abc import Iterator +from typing import TYPE_CHECKING + +from rich.markup import escape +from rich.table import Table + +from conductor.providers.capabilities import known_provider_names +from conductor.providers.diagnostics import ( + ALL_SECTIONS, + DoctorReport, + EnvDiagnostic, + ProviderDiagnostic, + RegistryDiagnostic, + gather, +) + +if TYPE_CHECKING: + from rich.console import Console + + from conductor.providers.diagnostics import Section + + +_CHECK = "[green]✓[/green]" +_CROSS = "[red]✗[/red]" +_DASH = "[dim]—[/dim]" + + +def run_doctor( + *, + section: str | None, + provider: str | None, + check: bool, + models: bool, + as_json: bool, + console: Console, + err_console: Console, +) -> int: + """Gather and render diagnostics, returning a process exit code. + + Args: + section: Positional section filter (``env`` / ``providers`` / + ``registries``), or ``None`` for all sections. + provider: Scope the providers section to a single provider name. + check: Instantiate providers and probe ``validate_connection()``. + models: List available models (implies ``check``). + as_json: Emit a JSON document instead of Rich tables. + console: Console for primary output (stdout). + err_console: Console for error messages (stderr). + + Returns: + ``0`` on success; ``1`` when ``section``/``provider`` is invalid, or + when ``check`` is set and the scoped provider (``--provider`` when + given, else ``copilot``) fails to connect. + """ + # --models implies --check. + check = check or models + + if section is not None and section not in ALL_SECTIONS: + err_console.print( + f"[bold red]Error:[/bold red] Unknown section {section!r}. " + f"Choose from: {', '.join(ALL_SECTIONS)}." + ) + return 1 + + if provider is not None and provider not in known_provider_names(): + err_console.print( + f"[bold red]Error:[/bold red] Unknown provider {provider!r}. " + f"Known providers: {', '.join(known_provider_names())}." + ) + return 1 + + sections: tuple[Section, ...] = ALL_SECTIONS if section is None else (section,) # type: ignore[assignment] + + report = _gather_report(sections=sections, provider=provider, check=check, models=models) + + if as_json: + console.print_json(data=report.to_dict()) + return _compute_exit_code(report.providers, check=check, provider=provider) + + 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) + if report.registries is not None: + _render_registries(report.registries, console) + + return _compute_exit_code(report.providers, check=check, provider=provider) + + +def _compute_exit_code( + providers: list[ProviderDiagnostic] | None, + *, + check: bool, + provider: str | None, +) -> int: + """Return ``1`` when a checked scoped provider failed to connect, else ``0``. + + Offline runs (``check`` is False) and runs that did not gather the + providers section always return ``0`` — an unhealthy *optional* provider + never fails the command. Only the scoped provider (``--provider`` when + given, otherwise the ``copilot`` default) drives a non-zero exit. + """ + if not check or not providers: + return 0 + scoped = provider or "copilot" + for diag in providers: + if diag.name == scoped and diag.connection_ok is False: + return 1 + return 0 + + +@contextlib.contextmanager +def _suppressed_logging(active: bool) -> Iterator[None]: + """Silence log records while probing providers, restoring the prior level. + + Constructing and validating providers can emit INFO/ERROR log records to + stderr (e.g. the Claude provider logs "Connection validation failed" then + returns ``False``). During ``doctor`` the rendered report is the single + source of truth, so this noise is suppressed for the duration of the + probes. A no-op when ``active`` is ``False`` (offline runs stay pristine). + """ + if not active: + yield + return + previous = logging.root.manager.disable + logging.disable(logging.CRITICAL) + try: + yield + finally: + logging.disable(previous) + + +def _gather_report( + *, + sections: tuple[Section, ...], + provider: str | None, + check: bool, + models: bool, +) -> DoctorReport: + """Run the async gather, suppressing provider log noise during checks.""" + with _suppressed_logging(check or models): + return asyncio.run( + gather(sections=sections, provider=provider, check=check, list_models=models) + ) + + +def _render_env(env: EnvDiagnostic, console: Console) -> None: + """Render the environment section.""" + table = Table(title="Environment", show_header=False, box=None, padding=(0, 2)) + table.add_column("Key", style="dim") + table.add_column("Value") + + table.add_row("Conductor", f"v{env.conductor_version}") + table.add_row("Python", env.python_version) + table.add_row("Platform", env.platform) + + if not env.update_checked: + update = "[dim]check skipped (CONDUCTOR_NO_UPDATE_CHECK)[/dim]" + elif env.update_available is None: + update = "[dim]unavailable (offline?)[/dim]" + elif env.update_available: + update = f"[yellow]v{env.latest_version} available[/yellow]" + else: + update = "[green]up to date[/green]" + table.add_row("Update", update) + + console.print(table) + + +def _render_providers( + providers: list[ProviderDiagnostic], + console: Console, + *, + check: bool, + models: bool, +) -> None: + """Render the providers section as a table (columns adapt to flags).""" + table = Table(title="Providers", show_lines=True) + table.add_column("Provider", style="cyan", no_wrap=True) + table.add_column("Installed", justify="center") + table.add_column("Tier") + table.add_column("Credentials", no_wrap=True) + if check: + table.add_column("Connection") + if models: + table.add_column("Models") + table.add_column("Notes") + + for diag in providers: + row = [ + diag.name, + _CHECK if diag.installed else _CROSS, + _tier_cell(diag.tier), + _credentials_cell(diag), + ] + if check: + row.append(_connection_cell(diag)) + if models: + row.append(_models_cell(diag)) + row.append(diag.note or _DASH) + table.add_row(*row) + + console.print(table) + + +def _tier_cell(tier: str | None) -> str: + """Format the tier cell.""" + if tier is None: + return _DASH + if tier == "experimental": + return "[yellow]experimental[/yellow]" + return tier + + +def _credentials_cell(diag: ProviderDiagnostic) -> str: + """Format credential env-var presence (presence only, never values).""" + if not diag.credential_env_vars: + return _DASH + lines = [ + f"{_CHECK} {cred.name}" if cred.present else f"[dim]{_CROSS} {cred.name}[/dim]" + for cred in diag.credential_env_vars + ] + return "\n".join(lines) + + +def _connection_cell(diag: ProviderDiagnostic) -> str: + """Format the connection-check result cell.""" + if not diag.checked or diag.connection_ok is None: + return _DASH + if diag.connection_ok: + return f"{_CHECK} connected" + if diag.connection_error: + return f"{_CROSS} [dim]{escape(diag.connection_error)}[/dim]" + return f"{_CROSS} [dim]connection failed[/dim]" + + +def _models_cell(diag: ProviderDiagnostic) -> str: + """Format the models cell. + + Lists every model id (comma-separated); Rich wraps the cell to the column + width. The leading count makes long lists scannable. ``n/a`` when models + is None (not enumerated), ``(none)`` for an empty list. + """ + if diag.models_error: + return f"{_CROSS} [dim]{escape(diag.models_error)}[/dim]" + if diag.models is None: + return "[dim]n/a[/dim]" + if not diag.models: + return "[dim](none)[/dim]" + shown = ", ".join(escape(model) for model in diag.models) + return f"[dim]{len(diag.models)}:[/dim] {shown}" + + +def _render_registries(registries: RegistryDiagnostic, console: Console) -> None: + """Render the registries section.""" + if registries.error is not None: + console.print(f"{_CROSS} [dim]failed to load registries: {escape(registries.error)}[/dim]") + return + if not registries.registries: + console.print("[dim]No registries configured.[/dim]") + return + + table = Table(title="Registries", show_lines=False) + table.add_column("Name", style="cyan") + table.add_column("Type") + table.add_column("Source") + table.add_column("Default", justify="center") + + for reg in registries.registries: + table.add_row( + escape(reg.name), + escape(reg.type), + escape(reg.source), + _CHECK if reg.is_default else _DASH, + ) + + console.print(table) diff --git a/src/conductor/providers/base.py b/src/conductor/providers/base.py index 988359e7..5ed77b74 100644 --- a/src/conductor/providers/base.py +++ b/src/conductor/providers/base.py @@ -335,3 +335,29 @@ async def get_model_pricing(self, model: str) -> ModelPricing | None: ``model``, otherwise ``None``. """ return None + + async def list_models(self) -> list[str] | None: + """Return the model identifiers the provider can enumerate, if any. + + Used by ``conductor doctor --models`` to surface the models a + provider exposes. Implementations should query their SDK's + model-listing endpoint and return the resulting identifiers. + + Implementations should: + + * Return a list of model id strings on success (possibly empty). + * Return ``None`` when the provider cannot enumerate models — either + because the SDK is unavailable, the provider has no model-listing + concept, or the listing call failed. + * Never raise — diagnostics are best-effort and must not interrupt + the caller. + + The default implementation returns ``None`` so providers that have no + model-enumeration concept (e.g. those delegating to an external CLI) + are reported as "n/a" rather than an error. + + Returns: + A list of available model identifiers, or ``None`` when the + provider does not enumerate models. + """ + return None diff --git a/src/conductor/providers/claude.py b/src/conductor/providers/claude.py index a0050fa3..9b158530 100644 --- a/src/conductor/providers/claude.py +++ b/src/conductor/providers/claude.py @@ -482,6 +482,24 @@ async def get_max_prompt_tokens(self, model: str) -> int | None: matched_id = match_model_id(model, cache.keys()) return cache.get(matched_id) if matched_id is not None else None + async def list_models(self) -> list[str] | None: + """Return the model ids advertised by the Anthropic API. + + Enumerates ``client.models.list()`` and returns each entry's ``id``. + Used by ``conductor doctor --models``. + + Returns ``None`` when the SDK is unavailable, the client has not been + constructed, or the listing call fails — diagnostics must never raise. + """ + if not ANTHROPIC_SDK_AVAILABLE or self._client is None: + return None + try: + page = await self._client.models.list() + except Exception as e: # noqa: BLE001 - diagnostics must never raise + logger.debug("Failed to list Anthropic models: %s", e) + return None + return [model.id for model in page.data] + async def _ensure_mcp_connected(self) -> None: """Connect to MCP servers if configured. diff --git a/src/conductor/providers/copilot.py b/src/conductor/providers/copilot.py index 37972d52..ff220f37 100644 --- a/src/conductor/providers/copilot.py +++ b/src/conductor/providers/copilot.py @@ -2435,6 +2435,29 @@ def _per_mtok(credits_per_batch: float) -> float: cache_write_per_mtok=0.0, ) + async def list_models(self) -> list[str] | None: + """Return the model ids advertised by the Copilot SDK. + + Queries ``client.list_models()`` and returns the ``id`` of every + entry. Used by ``conductor doctor --models``. + + Returns ``None`` in mock-handler mode, when the SDK is unavailable, + or when the SDK call fails — diagnostics must never raise. + + Catches ``Exception`` (not ``BaseException``) at the SDK boundary so + ``asyncio.CancelledError``/``KeyboardInterrupt``/``SystemExit`` still + propagate, mirroring :meth:`get_max_prompt_tokens`. + """ + if self._mock_handler is not None or not COPILOT_SDK_AVAILABLE: + return None + try: + await self._ensure_client_started() + models = await self._client.list_models() + except Exception as e: + logger.debug("Failed to list Copilot models: %s", e) + return None + return [info.id for info in models] + async def _validate_reasoning_effort_for_model( self, model: str, effort: ReasoningEffort ) -> None: diff --git a/src/conductor/providers/diagnostics.py b/src/conductor/providers/diagnostics.py new file mode 100644 index 00000000..de1d4d5c --- /dev/null +++ b/src/conductor/providers/diagnostics.py @@ -0,0 +1,468 @@ +"""Provider & environment diagnostics for ``conductor doctor``. + +Keyless, Typer-free data-gathering layer behind the ``conductor doctor`` +command (issue #274). It answers "is my setup healthy?" without running a +workflow: which providers are installed, whether they can connect, what +models they expose, plus Conductor version / update status and configured +registries. + +Design contract: + +* **Never raises.** Every probe degrades gracefully — a missing SDK, an + unreadable config file, or a failing connection is captured as data, not + an exception. Callers can render whatever was gathered. +* **Offline by default.** No provider is instantiated and no backend is + contacted unless ``check=True`` (connection probes) or ``list_models=True`` + (which implies a check). The only default network touch is the GitHub + Releases update check in :func:`gather_env`, which is cache-first, uses a + short timeout, fails silently, and honors ``CONDUCTOR_NO_UPDATE_CHECK``. +* **No secrets.** Credential environment variables are reported by + *presence only* — their values are never read into the report. +""" + +from __future__ import annotations + +import contextlib +import os +import platform +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Any, Literal, cast + +from conductor import __version__ +from conductor.providers.capabilities import get_capabilities, known_provider_names + +if TYPE_CHECKING: + from conductor.providers.factory import ProviderType + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +Section = Literal["env", "providers", "registries"] +"""A ``conductor doctor`` output section.""" + +ALL_SECTIONS: tuple[Section, ...] = ("env", "providers", "registries") +"""Default set of sections rendered when no positional ``SECTION`` is given.""" + +# Provider names that are known to the schema/factory but not yet implemented. +# Surfaced as an informational note, not an error. +_NOT_IMPLEMENTED: frozenset[str] = frozenset({"openai-agents"}) + +# Per-provider credential environment variables whose *presence* (never +# value) is reported in the offline diagnostic. Copilot authenticates via +# the GitHub/Copilot CLI login on disk, so its GitHub-token vars are +# best-effort hints rather than hard requirements. +_CREDENTIAL_ENV_VARS: dict[str, tuple[str, ...]] = { + "copilot": ( + "GITHUB_TOKEN", + "GH_TOKEN", + "COPILOT_PROVIDER_API_KEY", + "COPILOT_PROVIDER_BEARER_TOKEN", + ), + "claude": ("ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN"), + "claude-agent-sdk": ("ANTHROPIC_API_KEY",), + "hermes": (), + "openai-agents": (), +} + +# Update-check opt-out env var (mirrors cli/update.py so diagnostics does not +# depend on a private symbol there). +_UPDATE_DISABLE_ENV = "CONDUCTOR_NO_UPDATE_CHECK" + + +# --------------------------------------------------------------------------- +# Dataclasses +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class CredentialEnvVar: + """Presence of a single credential environment variable (value never read).""" + + name: str + present: bool + + def to_dict(self) -> dict[str, Any]: + """Return a JSON-safe representation.""" + return {"name": self.name, "present": self.present} + + +@dataclass +class ProviderDiagnostic: + """Diagnostic snapshot for a single provider.""" + + name: str + installed: bool + implemented: bool + tier: str | None + credential_env_vars: list[CredentialEnvVar] = field(default_factory=list) + checked: bool = False + connection_ok: bool | None = None + connection_error: str | None = None + models: list[str] | None = None + models_error: str | None = None + note: str | None = None + + def to_dict(self) -> dict[str, Any]: + """Return a JSON-safe representation.""" + return { + "name": self.name, + "installed": self.installed, + "implemented": self.implemented, + "tier": self.tier, + "credential_env_vars": [c.to_dict() for c in self.credential_env_vars], + "checked": self.checked, + "connection_ok": self.connection_ok, + "connection_error": self.connection_error, + "models": self.models, + "models_error": self.models_error, + "note": self.note, + } + + +@dataclass +class EnvDiagnostic: + """Diagnostic snapshot of the Conductor install and host environment.""" + + conductor_version: str + python_version: str + platform: str + update_checked: bool + update_available: bool | None + latest_version: str | None + + def to_dict(self) -> dict[str, Any]: + """Return a JSON-safe representation.""" + return { + "conductor_version": self.conductor_version, + "python_version": self.python_version, + "platform": self.platform, + "update_checked": self.update_checked, + "update_available": self.update_available, + "latest_version": self.latest_version, + } + + +@dataclass +class RegistryInfo: + """A single configured registry entry.""" + + name: str + type: str + source: str + is_default: bool + + def to_dict(self) -> dict[str, Any]: + """Return a JSON-safe representation.""" + return { + "name": self.name, + "type": self.type, + "source": self.source, + "is_default": self.is_default, + } + + +@dataclass +class RegistryDiagnostic: + """Diagnostic snapshot of configured workflow registries.""" + + default: str | None + registries: list[RegistryInfo] = field(default_factory=list) + error: str | None = None + """Set when the registries config could not be loaded (e.g. malformed + TOML). Distinguishes a load *failure* from a genuinely empty config so + ``doctor`` surfaces the problem instead of reporting "no registries".""" + + def to_dict(self) -> dict[str, Any]: + """Return a JSON-safe representation.""" + return { + "default": self.default, + "registries": [r.to_dict() for r in self.registries], + "error": self.error, + } + + +@dataclass +class DoctorReport: + """Aggregated diagnostics. Sections not requested are left as ``None``.""" + + env: EnvDiagnostic | None = None + providers: list[ProviderDiagnostic] | None = None + registries: RegistryDiagnostic | None = None + + def to_dict(self) -> dict[str, Any]: + """Return a JSON-safe representation, omitting sections not gathered.""" + out: dict[str, Any] = {} + if self.env is not None: + out["env"] = self.env.to_dict() + if self.providers is not None: + out["providers"] = [p.to_dict() for p in self.providers] + if self.registries is not None: + out["registries"] = self.registries.to_dict() + return out + + +# --------------------------------------------------------------------------- +# Small helpers (never raise) +# --------------------------------------------------------------------------- + + +def _format_error(exc: BaseException) -> str: + """Render an exception as a compact one-line string for the report.""" + msg = str(exc).strip() + return msg if msg else type(exc).__name__ + + +def _sdk_available(name: str) -> bool: + """Return the provider's SDK-availability flag, or ``False`` on any error.""" + try: + if name == "copilot": + from conductor.providers.copilot import COPILOT_SDK_AVAILABLE + + return COPILOT_SDK_AVAILABLE + if name == "claude": + from conductor.providers.claude import ANTHROPIC_SDK_AVAILABLE + + return ANTHROPIC_SDK_AVAILABLE + if name == "claude-agent-sdk": + from conductor.providers.claude_agent_sdk import CLAUDE_AGENT_SDK_AVAILABLE + + return CLAUDE_AGENT_SDK_AVAILABLE + if name == "hermes": + from conductor.providers.hermes import HERMES_SDK_AVAILABLE + + return HERMES_SDK_AVAILABLE + except Exception: # noqa: BLE001 - diagnostics must never raise + return False + return False + + +def _provider_tier(name: str) -> str | None: + """Return the provider's stability tier, or ``None`` when undeterminable.""" + try: + return get_capabilities(name).tier + except Exception: # noqa: BLE001 - diagnostics must never raise + return None + + +def _credential_env_vars(name: str) -> list[CredentialEnvVar]: + """Return presence flags for the provider's credential env vars.""" + return [ + CredentialEnvVar(var, bool(os.environ.get(var))) + for var in _CREDENTIAL_ENV_VARS.get(name, ()) + ] + + +def _update_check_disabled() -> bool: + """Return ``True`` if the user opted out of update checks via env var.""" + val = os.environ.get(_UPDATE_DISABLE_ENV, "").strip().lower() + return val in {"1", "true", "yes"} + + +def _check_update() -> tuple[bool, bool | None, str | None]: + """Determine update availability (cache-first, silent, best-effort). + + Returns: + A ``(checked, available, latest_version)`` tuple. + ``checked`` is ``False`` when the check was skipped via + ``CONDUCTOR_NO_UPDATE_CHECK``. When ``checked`` is ``True`` but the + result could not be determined (offline / parse failure), + ``available`` is ``None`` and ``latest_version`` is ``None``. + """ + if _update_check_disabled(): + return False, None, None + try: + from conductor.cli.update import ( + fetch_latest_version, + is_newer, + read_cache, + write_cache, + ) + + cached = read_cache() + if cached is not None: + remote = cached.get("version", "") + else: + result = fetch_latest_version() + if result is None: + return True, None, None + remote, tag_name, url = result + # Persisting the fetched version is best-effort: a non-writable + # HOME (common in CI) must NOT discard an already-successful + # fetch and misreport "offline". + with contextlib.suppress(Exception): + write_cache(remote, tag_name, url) + if not remote: + return True, None, None + return True, is_newer(remote, __version__), remote + except Exception: # noqa: BLE001 - diagnostics must never raise + return True, None, None + + +# --------------------------------------------------------------------------- +# Gather functions +# --------------------------------------------------------------------------- + + +def gather_env() -> EnvDiagnostic: + """Gather Conductor version, host, and update-availability diagnostics.""" + checked, available, latest = _check_update() + return EnvDiagnostic( + conductor_version=__version__, + python_version=platform.python_version(), + platform=platform.platform(), + update_checked=checked, + update_available=available, + latest_version=latest, + ) + + +def gather_registries() -> RegistryDiagnostic: + """Gather configured workflow registries (never raises). + + A load failure (e.g. malformed ``registries.toml``) is captured in the + returned ``error`` field rather than swallowed — a corrupt config must be + surfaced, not reported as "no registries configured". + """ + try: + from conductor.registry.config import load_config + + config = load_config() + except Exception as e: # noqa: BLE001 - diagnostics must never raise + return RegistryDiagnostic(default=None, registries=[], error=_format_error(e)) + + registries = [ + RegistryInfo( + name=reg_name, + type=str(entry.type), + source=entry.source, + is_default=(reg_name == config.default), + ) + for reg_name, entry in config.registries.items() + ] + return RegistryDiagnostic(default=config.default, registries=registries) + + +async def gather_provider( + name: str, + *, + check: bool = False, + list_models: bool = False, +) -> ProviderDiagnostic: + """Gather diagnostics for a single provider (never raises). + + Offline fields (``installed`` / ``tier`` / credential presence) are + always populated. When ``check`` (or ``list_models``, which implies a + check) is set and the provider is implemented and installed, the + provider is constructed and ``validate_connection()`` is called; with + ``list_models`` its ``list_models()`` is also queried. + + Args: + name: Provider name (e.g. ``"copilot"``). + check: Instantiate the provider and probe ``validate_connection()``. + list_models: Also enumerate available models (implies ``check``). + + Returns: + A fully-populated :class:`ProviderDiagnostic`. + """ + implemented = name not in _NOT_IMPLEMENTED + installed = _sdk_available(name) if implemented else False + + diag = ProviderDiagnostic( + name=name, + installed=installed, + implemented=implemented, + tier=_provider_tier(name), + credential_env_vars=_credential_env_vars(name), + note=None if implemented else "not yet implemented", + ) + + do_check = check or list_models + if not do_check or not implemented: + return diag + + diag.checked = True + if not installed: + diag.connection_ok = False + diag.connection_error = "SDK not installed" + return diag + + from conductor.providers.factory import create_provider + + provider = None + try: + provider = await create_provider(cast("ProviderType", name), validate=False) + except Exception as e: # noqa: BLE001 - diagnostics must never raise + diag.connection_ok = False + diag.connection_error = _format_error(e) + return diag + + try: + try: + diag.connection_ok = bool(await provider.validate_connection()) + except Exception as e: # noqa: BLE001 - diagnostics must never raise + diag.connection_ok = False + diag.connection_error = _format_error(e) + + if list_models and diag.connection_ok: + try: + models = await provider.list_models() + diag.models = list(models) if models is not None else None + except Exception as e: # noqa: BLE001 - diagnostics must never raise + diag.models_error = _format_error(e) + finally: + with contextlib.suppress(Exception): + await provider.close() + + return diag + + +async def gather( + *, + sections: tuple[Section, ...] = ALL_SECTIONS, + provider: str | None = None, + check: bool = False, + list_models: bool = False, +) -> DoctorReport: + """Gather a full :class:`DoctorReport` for the requested sections. + + Args: + sections: Which sections to include. Defaults to all. + provider: When set, scope the ``providers`` section to this one name. + check: Probe provider connections (``providers`` section only). + list_models: Enumerate provider models (implies ``check``). + + Returns: + A :class:`DoctorReport`; sections not requested remain ``None``. + """ + report = DoctorReport() + + if "env" in sections: + report.env = gather_env() + + if "providers" in sections: + names = [provider] if provider is not None else list(known_provider_names()) + report.providers = [ + await gather_provider(pname, check=check, list_models=list_models) for pname in names + ] + + if "registries" in sections: + report.registries = gather_registries() + + return report + + +__all__ = [ + "ALL_SECTIONS", + "CredentialEnvVar", + "DoctorReport", + "EnvDiagnostic", + "ProviderDiagnostic", + "RegistryDiagnostic", + "RegistryInfo", + "Section", + "gather", + "gather_env", + "gather_provider", + "gather_registries", +] diff --git a/tests/test_cli/test_doctor.py b/tests/test_cli/test_doctor.py new file mode 100644 index 00000000..8eecb55f --- /dev/null +++ b/tests/test_cli/test_doctor.py @@ -0,0 +1,393 @@ +"""Tests for the ``conductor doctor`` CLI command (issue #274). + +Exercises rendering, JSON output, exit-code semantics, and error handling. +Data gathering is patched at ``conductor.cli.doctor.gather`` for the +flag/exit-code cases; one test runs the real offline path end-to-end. +""" + +from __future__ import annotations + +import importlib +import json +from unittest.mock import AsyncMock + +import pytest +import typer.main +from rich.console import Console +from typer.testing import CliRunner + +from conductor.cli.app import app +from conductor.providers.diagnostics import ( + CredentialEnvVar, + DoctorReport, + EnvDiagnostic, + ProviderDiagnostic, + RegistryDiagnostic, + RegistryInfo, +) + +runner = CliRunner() + +# The submodule ``conductor.cli.app`` is shadowed by the ``app`` Typer object +# it exports (``conductor/cli/__init__.py`` does ``from conductor.cli.app +# import app``), so the string path / plain import resolves to the Typer, not +# the module. Grab the real module object explicitly for console patching. +_app_module = importlib.import_module("conductor.cli.app") + + +@pytest.fixture(autouse=True) +def _no_update_check(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the CLI offline and render at a fixed wide width. + + The doctor command renders through the module-level ``output_console`` / + ``console`` in ``conductor.cli.app``, whose width tracks the ambient + terminal. CI runs with a narrow non-TTY width that wraps and truncates + Rich table cells, which would break substring assertions on the rendered + output. Pinning both consoles to a fixed width makes rendering + deterministic regardless of the environment. + """ + monkeypatch.setenv("CONDUCTOR_NO_UPDATE_CHECK", "1") + monkeypatch.setattr(_app_module, "output_console", Console(width=200)) + monkeypatch.setattr(_app_module, "console", Console(stderr=True, width=200)) + + +def _prov( + name: str, + *, + installed: bool = True, + implemented: bool = True, + tier: str | None = "stable", + creds: list[CredentialEnvVar] | None = None, + checked: bool = False, + connection_ok: bool | None = None, + connection_error: str | None = None, + models: list[str] | None = None, + models_error: str | None = None, + note: str | None = None, +) -> ProviderDiagnostic: + return ProviderDiagnostic( + name=name, + installed=installed, + implemented=implemented, + tier=tier, + credential_env_vars=creds or [], + checked=checked, + connection_ok=connection_ok, + connection_error=connection_error, + models=models, + models_error=models_error, + note=note, + ) + + +def _patch_gather( + monkeypatch: pytest.MonkeyPatch, + report: DoctorReport, + captured: dict[str, object] | None = None, +) -> None: + """Patch ``conductor.cli.doctor.gather`` to return *report*.""" + + async def _fake_gather(**kwargs: object) -> DoctorReport: + if captured is not None: + captured.update(kwargs) + return report + + monkeypatch.setattr("conductor.cli.doctor.gather", _fake_gather) + + +# --------------------------------------------------------------------------- +# Help / basic wiring +# --------------------------------------------------------------------------- + + +class TestDoctorHelp: + def test_help_runs(self) -> None: + result = runner.invoke(app, ["doctor", "--help"]) + assert result.exit_code == 0 + + def test_options_are_registered(self) -> None: + # Inspect the command's registered parameters rather than parsing the + # rendered help text: Rich wraps/truncates the options panel at narrow + # (CI non-TTY) widths, so a substring check on the help output is + # fragile. Param inspection verifies the flags actually exist. + doctor_cmd = typer.main.get_command(app).commands["doctor"] + opts = {opt for param in doctor_cmd.params for opt in (*param.opts, *param.secondary_opts)} + for token in ("--check", "--models", "--provider", "--json"): + assert token in opts + + +# --------------------------------------------------------------------------- +# Offline rendering (real end-to-end) +# --------------------------------------------------------------------------- + + +class TestDoctorOffline: + def test_default_all_sections(self, monkeypatch: pytest.MonkeyPatch, tmp_path: object) -> None: + monkeypatch.setenv("CONDUCTOR_HOME", str(tmp_path)) + result = runner.invoke(app, ["doctor"]) + assert result.exit_code == 0 + assert "Environment" in result.output + assert "copilot" in result.output + assert "claude" in result.output + + def test_section_filter(self, monkeypatch: pytest.MonkeyPatch) -> None: + captured: dict[str, object] = {} + _patch_gather( + monkeypatch, DoctorReport(registries=RegistryDiagnostic(default=None)), captured + ) + result = runner.invoke(app, ["doctor", "registries"]) + assert result.exit_code == 0 + assert captured["sections"] == ("registries",) + + +# --------------------------------------------------------------------------- +# JSON output +# --------------------------------------------------------------------------- + + +class TestDoctorJson: + def test_json_is_parseable(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport( + env=EnvDiagnostic( + conductor_version="1.2.3", + python_version="3.12.0", + platform="test", + update_checked=False, + update_available=None, + latest_version=None, + ), + providers=[_prov("copilot")], + ) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "--json"]) + assert result.exit_code == 0 + data = json.loads(result.stdout) + assert data["env"]["conductor_version"] == "1.2.3" + assert data["providers"][0]["name"] == "copilot" + + def test_json_never_leaks_secret_values(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport( + providers=[ + _prov( + "claude", + creds=[ + CredentialEnvVar("ANTHROPIC_API_KEY", True), + CredentialEnvVar("ANTHROPIC_AUTH_TOKEN", False), + ], + ) + ] + ) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "--json"]) + assert result.exit_code == 0 + data = json.loads(result.stdout) + creds = data["providers"][0]["credential_env_vars"] + # Only name + present are ever serialized (no value field). + assert creds[0] == {"name": "ANTHROPIC_API_KEY", "present": True} + assert all(set(c) == {"name", "present"} for c in creds) + + def test_json_with_check_failure_exits_one(self, monkeypatch: pytest.MonkeyPatch) -> None: + # The primary CI use case: emit machine-readable JSON AND signal a + # non-zero exit when the scoped provider fails to connect. + report = DoctorReport(providers=[_prov("copilot", checked=True, connection_ok=False)]) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "providers", "--check", "--json"]) + assert result.exit_code == 1 + data = json.loads(result.stdout) # JSON still valid despite exit 1 + assert data["providers"][0]["connection_ok"] is False + + def test_json_includes_registries_error(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport(registries=RegistryDiagnostic(default=None, error="malformed TOML")) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "registries", "--json"]) + assert result.exit_code == 0 + data = json.loads(result.stdout) + assert data["registries"]["error"] == "malformed TOML" + + +# --------------------------------------------------------------------------- +# Secret-leak safety (end-to-end, real environment) +# --------------------------------------------------------------------------- + + +class TestDoctorSecretLeakEndToEnd: + """A real secret in the environment must never reach stdout (presence only).""" + + _CANARY = "sk-ant-LEAK-CANARY-DO-NOT-PRINT" + + def test_offline_json_does_not_leak_env_secret(self, monkeypatch: pytest.MonkeyPatch) -> None: + # Real gather (NOT patched) with a real secret env var set. + monkeypatch.setenv("ANTHROPIC_API_KEY", self._CANARY) + result = runner.invoke(app, ["doctor", "providers", "--json"]) + assert result.exit_code == 0 + assert self._CANARY not in result.output + data = json.loads(result.stdout) + claude = next(p for p in data["providers"] if p["name"] == "claude") + present = {c["name"]: c["present"] for c in claude["credential_env_vars"]} + assert present["ANTHROPIC_API_KEY"] is True # detected by presence + assert all("value" not in c for c in claude["credential_env_vars"]) + + def test_check_json_does_not_leak_env_secret(self, monkeypatch: pytest.MonkeyPatch) -> None: + # --check must not echo the secret even while probing; patch provider + # construction so no real network I/O happens. + monkeypatch.setenv("ANTHROPIC_API_KEY", self._CANARY) + fake = AsyncMock() + fake.validate_connection.return_value = False + fake.list_models.return_value = None + fake.close.return_value = None + monkeypatch.setattr( + "conductor.providers.factory.create_provider", + AsyncMock(return_value=fake), + ) + result = runner.invoke( + app, ["doctor", "providers", "--provider", "claude", "--check", "--json"] + ) + assert result.exit_code == 1 # scoped claude fails to connect + assert self._CANARY not in result.output + data = json.loads(result.stdout) + assert data["providers"][0]["connection_ok"] is False + + +# --------------------------------------------------------------------------- +# Exit-code semantics +# --------------------------------------------------------------------------- + + +class TestDoctorExitCodes: + def test_offline_exit_zero(self, monkeypatch: pytest.MonkeyPatch) -> None: + _patch_gather(monkeypatch, DoctorReport(providers=[_prov("copilot")])) + result = runner.invoke(app, ["doctor", "providers"]) + assert result.exit_code == 0 + + def test_scoped_default_failure_exits_one(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport(providers=[_prov("copilot", checked=True, connection_ok=False)]) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "providers", "--check"]) + assert result.exit_code == 1 + + def test_optional_provider_failure_exits_zero(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport( + providers=[ + _prov("copilot", checked=True, connection_ok=True), + _prov("claude", checked=True, connection_ok=False), + ] + ) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "providers", "--check"]) + assert result.exit_code == 0 + + def test_scoped_provider_failure_exits_one(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport(providers=[_prov("claude", checked=True, connection_ok=False)]) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "providers", "--provider", "claude", "--check"]) + assert result.exit_code == 1 + + +# --------------------------------------------------------------------------- +# Flags +# --------------------------------------------------------------------------- + + +class TestDoctorFlags: + def test_models_implies_check(self, monkeypatch: pytest.MonkeyPatch) -> None: + captured: dict[str, object] = {} + _patch_gather(monkeypatch, DoctorReport(providers=[_prov("copilot")]), captured) + result = runner.invoke(app, ["doctor", "--models"]) + assert result.exit_code == 0 + assert captured["check"] is True + assert captured["list_models"] is True + + def test_models_rendered(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport( + providers=[ + _prov("copilot", checked=True, connection_ok=True, models=["gpt-5", "gpt-4"]) + ] + ) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "--models"]) + assert result.exit_code == 0 + assert "gpt-5" in result.output + + def test_all_models_shown_no_truncation(self, monkeypatch: pytest.MonkeyPatch) -> None: + # Every model id must appear — no "(+N more)" cap. Use a wide render so + # the assertion isn't defeated by cell wrapping mid-identifier. + models = [f"model-{i:02d}" for i in range(20)] + report = DoctorReport( + providers=[_prov("copilot", checked=True, connection_ok=True, models=models)] + ) + _patch_gather(monkeypatch, report) + monkeypatch.setattr(_app_module, "output_console", Console(width=1000)) + result = runner.invoke(app, ["doctor", "--models"]) + assert result.exit_code == 0 + assert "more)" not in result.output + for model in models: + assert model in result.output + + +# --------------------------------------------------------------------------- +# Error handling +# --------------------------------------------------------------------------- + + +class TestDoctorErrors: + def test_unknown_provider(self) -> None: + result = runner.invoke(app, ["doctor", "--provider", "bogus"]) + assert result.exit_code == 1 + assert "Unknown provider" in (result.stderr or result.output) + + def test_unknown_section(self) -> None: + result = runner.invoke(app, ["doctor", "bogus"]) + assert result.exit_code == 1 + assert "Unknown section" in (result.stderr or result.output) + + +class TestDoctorMarkupSafety: + """Free-form strings with Rich markup metacharacters must not crash rendering.""" + + def test_bracketed_connection_error_renders(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport( + providers=[ + _prov( + "claude", + checked=True, + connection_ok=False, + connection_error="[Errno 2] No such file [/Users/x]", + ) + ] + ) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "providers", "--provider", "claude", "--check"]) + # Rendering the whole table happens in one console.print; if the + # bracketed error weren't escaped, Rich would raise MarkupError and the + # error text would never reach stdout. Its presence proves no crash. + assert result.exit_code == 1 + assert "Errno 2" in result.output + + def test_bracketed_registry_source_renders(self, monkeypatch: pytest.MonkeyPatch) -> None: + report = DoctorReport( + registries=RegistryDiagnostic( + default="local", + registries=[ + RegistryInfo(name="local", type="path", source="[/weird/path]", is_default=True) + ], + ) + ) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "registries"]) + assert result.exception is None + assert result.exit_code == 0 + assert "weird/path" in result.output + + def test_registries_load_error_renders(self, monkeypatch: pytest.MonkeyPatch) -> None: + # A corrupt registries config is surfaced (not shown as "no registries") + # and bracketed error text does not crash Rich rendering. + report = DoctorReport( + registries=RegistryDiagnostic(default=None, error="bad TOML at [line 3]") + ) + _patch_gather(monkeypatch, report) + result = runner.invoke(app, ["doctor", "registries"]) + assert result.exception is None + assert result.exit_code == 0 + assert "failed to load registries" in result.output + assert "line 3" in result.output + assert "No registries configured" not in result.output diff --git a/tests/test_providers/test_diagnostics.py b/tests/test_providers/test_diagnostics.py new file mode 100644 index 00000000..b1193b16 --- /dev/null +++ b/tests/test_providers/test_diagnostics.py @@ -0,0 +1,349 @@ +"""Tests for the keyless diagnostics layer behind ``conductor doctor`` (#274). + +These tests assert the gather layer never raises, honors the offline +contract (no provider instantiation unless ``check``/``list_models``), and +maps providers/credentials/registries into the report dataclasses correctly. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock + +import pytest + +from conductor.providers import diagnostics as d + +# --------------------------------------------------------------------------- +# Environment section +# --------------------------------------------------------------------------- + + +class TestGatherEnv: + """`gather_env` reports version/host and update availability.""" + + def test_basic_fields(self) -> None: + env = d.gather_env() + assert env.conductor_version + assert env.python_version + assert env.platform + + def test_update_check_disabled(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("CONDUCTOR_NO_UPDATE_CHECK", "1") + env = d.gather_env() + assert env.update_checked is False + assert env.update_available is None + assert env.latest_version is None + + def test_update_available_from_cache(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("CONDUCTOR_NO_UPDATE_CHECK", raising=False) + monkeypatch.setattr( + "conductor.cli.update.read_cache", + lambda: {"version": "999.0.0"}, + ) + env = d.gather_env() + assert env.update_checked is True + assert env.update_available is True + assert env.latest_version == "999.0.0" + + def test_update_check_network_failure_is_silent(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("CONDUCTOR_NO_UPDATE_CHECK", raising=False) + monkeypatch.setattr("conductor.cli.update.read_cache", lambda: None) + monkeypatch.setattr("conductor.cli.update.fetch_latest_version", lambda: None) + env = d.gather_env() + assert env.update_checked is True + assert env.update_available is None + + def test_cold_cache_fetch_and_up_to_date(self, monkeypatch: pytest.MonkeyPatch) -> None: + # No cache -> fetch, persist, and report "up to date" for an older remote. + monkeypatch.delenv("CONDUCTOR_NO_UPDATE_CHECK", raising=False) + monkeypatch.setattr("conductor.cli.update.read_cache", lambda: None) + monkeypatch.setattr( + "conductor.cli.update.fetch_latest_version", + lambda: ("0.0.1", "v0.0.1", "url"), + ) + wrote: list[tuple[str, str, str]] = [] + monkeypatch.setattr( + "conductor.cli.update.write_cache", + lambda v, t, u: wrote.append((v, t, u)), + ) + env = d.gather_env() + assert env.update_checked is True + assert env.update_available is False # remote 0.0.1 is older than current + assert env.latest_version == "0.0.1" + assert wrote == [("0.0.1", "v0.0.1", "url")] + + def test_cache_write_failure_does_not_discard_fetch( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + # A non-writable HOME (common in CI) must NOT collapse a successful + # fetch into a misleading "offline" result. + monkeypatch.delenv("CONDUCTOR_NO_UPDATE_CHECK", raising=False) + monkeypatch.setattr("conductor.cli.update.read_cache", lambda: None) + monkeypatch.setattr( + "conductor.cli.update.fetch_latest_version", + lambda: ("999.0.0", "v999.0.0", "url"), + ) + + def _boom(*_args: Any) -> None: + raise OSError("read-only HOME") + + monkeypatch.setattr("conductor.cli.update.write_cache", _boom) + env = d.gather_env() + assert env.update_checked is True + assert env.update_available is True # fetch preserved despite write failure + assert env.latest_version == "999.0.0" + + +# --------------------------------------------------------------------------- +# Registries section +# --------------------------------------------------------------------------- + + +class TestGatherRegistries: + """`gather_registries` reflects the registries config, never raises.""" + + def test_reads_config(self, monkeypatch: pytest.MonkeyPatch) -> None: + fake = SimpleNamespace( + default="team", + registries={ + "team": SimpleNamespace(type="github", source="org/repo"), + "local": SimpleNamespace(type="path", source="/tmp/wf"), + }, + ) + monkeypatch.setattr("conductor.registry.config.load_config", lambda: fake) + result = d.gather_registries() + assert result.default == "team" + assert result.error is None + assert {r.name for r in result.registries} == {"team", "local"} + team = next(r for r in result.registries if r.name == "team") + assert team.is_default is True + assert team.type == "github" + local = next(r for r in result.registries if r.name == "local") + assert local.is_default is False + + def test_load_failure_is_surfaced_not_swallowed(self, monkeypatch: pytest.MonkeyPatch) -> None: + # A corrupt config must be reported via ``error`` — NOT collapsed into + # an empty result that reads as "no registries configured". + def _raise() -> Any: + raise RuntimeError("malformed TOML at line 3") + + monkeypatch.setattr("conductor.registry.config.load_config", _raise) + result = d.gather_registries() + assert result.default is None + assert result.registries == [] + assert result.error == "malformed TOML at line 3" + assert result.to_dict()["error"] == "malformed TOML at line 3" + + +# --------------------------------------------------------------------------- +# Provider section — offline +# --------------------------------------------------------------------------- + + +class TestGatherProviderOffline: + """Offline provider probes populate static fields and never connect.""" + + async def test_installed_and_tier(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.copilot.COPILOT_SDK_AVAILABLE", True) + diag = await d.gather_provider("copilot") + assert diag.installed is True + assert diag.implemented is True + assert diag.tier == "stable" + assert diag.checked is False + assert diag.connection_ok is None + + async def test_not_installed(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.hermes.HERMES_SDK_AVAILABLE", False) + diag = await d.gather_provider("hermes") + assert diag.installed is False + + async def test_openai_agents_not_implemented(self) -> None: + diag = await d.gather_provider("openai-agents") + assert diag.implemented is False + assert diag.installed is False + assert diag.note == "not yet implemented" + + async def test_credential_presence(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-test") + monkeypatch.delenv("ANTHROPIC_AUTH_TOKEN", raising=False) + diag = await d.gather_provider("claude") + by_name = {c.name: c.present for c in diag.credential_env_vars} + assert by_name == {"ANTHROPIC_API_KEY": True, "ANTHROPIC_AUTH_TOKEN": False} + + async def test_credential_values_never_stored(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-super-secret") + diag = await d.gather_provider("claude") + # Only presence booleans are recorded — never the secret value. + for cred in diag.credential_env_vars: + assert isinstance(cred.present, bool) + assert "sk-super-secret" not in repr(cred) + + async def test_offline_never_instantiates(self, monkeypatch: pytest.MonkeyPatch) -> None: + create = AsyncMock() + monkeypatch.setattr("conductor.providers.factory.create_provider", create) + await d.gather_provider("copilot", check=False) + create.assert_not_called() + + async def test_tier_failure_degrades(self, monkeypatch: pytest.MonkeyPatch) -> None: + def _raise(_name: str) -> Any: + raise RuntimeError("capabilities boom") + + monkeypatch.setattr("conductor.providers.diagnostics.get_capabilities", _raise) + diag = await d.gather_provider("copilot") + assert diag.tier is None # degrades, does not raise + + +# --------------------------------------------------------------------------- +# Provider section — with --check / --models +# --------------------------------------------------------------------------- + + +def _fake_provider( + *, + ok: bool = True, + validate_error: Exception | None = None, + models: list[str] | None = None, + models_error: Exception | None = None, +) -> Any: + """Build an AsyncMock provider for check/model probes.""" + provider = AsyncMock() + if validate_error is not None: + provider.validate_connection.side_effect = validate_error + else: + provider.validate_connection.return_value = ok + if models_error is not None: + provider.list_models.side_effect = models_error + else: + provider.list_models.return_value = models + provider.close.return_value = None + return provider + + +class TestGatherProviderCheck: + """`check` / `list_models` probe connections; failures never raise.""" + + async def test_connection_ok(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.copilot.COPILOT_SDK_AVAILABLE", True) + provider = _fake_provider(ok=True) + monkeypatch.setattr( + "conductor.providers.factory.create_provider", + AsyncMock(return_value=provider), + ) + diag = await d.gather_provider("copilot", check=True) + assert diag.checked is True + assert diag.connection_ok is True + provider.close.assert_awaited_once() + + async def test_not_installed_skips_construction(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.claude.ANTHROPIC_SDK_AVAILABLE", False) + create = AsyncMock() + monkeypatch.setattr("conductor.providers.factory.create_provider", create) + diag = await d.gather_provider("claude", check=True) + assert diag.checked is True + assert diag.connection_ok is False + assert diag.connection_error == "SDK not installed" + create.assert_not_called() + + async def test_construction_error_captured(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.claude.ANTHROPIC_SDK_AVAILABLE", True) + monkeypatch.setattr( + "conductor.providers.factory.create_provider", + AsyncMock(side_effect=RuntimeError("no api key")), + ) + diag = await d.gather_provider("claude", check=True) + assert diag.connection_ok is False + assert "no api key" in (diag.connection_error or "") + + async def test_validate_returns_false(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.claude.ANTHROPIC_SDK_AVAILABLE", True) + provider = _fake_provider(ok=False) + monkeypatch.setattr( + "conductor.providers.factory.create_provider", + AsyncMock(return_value=provider), + ) + diag = await d.gather_provider("claude", check=True) + assert diag.connection_ok is False + + async def test_validate_raises_is_caught(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.copilot.COPILOT_SDK_AVAILABLE", True) + provider = _fake_provider(validate_error=RuntimeError("kaboom")) + monkeypatch.setattr( + "conductor.providers.factory.create_provider", + AsyncMock(return_value=provider), + ) + diag = await d.gather_provider("copilot", check=True) + assert diag.connection_ok is False + assert "kaboom" in (diag.connection_error or "") + provider.close.assert_awaited_once() + + async def test_models_implies_check(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.copilot.COPILOT_SDK_AVAILABLE", True) + provider = _fake_provider(ok=True, models=["gpt-5", "gpt-4"]) + monkeypatch.setattr( + "conductor.providers.factory.create_provider", + AsyncMock(return_value=provider), + ) + diag = await d.gather_provider("copilot", list_models=True) + assert diag.checked is True + assert diag.models == ["gpt-5", "gpt-4"] + + async def test_models_none_is_na(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.copilot.COPILOT_SDK_AVAILABLE", True) + provider = _fake_provider(ok=True, models=None) + monkeypatch.setattr( + "conductor.providers.factory.create_provider", + AsyncMock(return_value=provider), + ) + diag = await d.gather_provider("copilot", list_models=True) + assert diag.models is None + + async def test_models_error_captured(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.copilot.COPILOT_SDK_AVAILABLE", True) + provider = _fake_provider(ok=True, models_error=RuntimeError("list boom")) + monkeypatch.setattr( + "conductor.providers.factory.create_provider", + AsyncMock(return_value=provider), + ) + diag = await d.gather_provider("copilot", list_models=True) + assert diag.models is None + assert "list boom" in (diag.models_error or "") + + +# --------------------------------------------------------------------------- +# Top-level gather +# --------------------------------------------------------------------------- + + +class TestGather: + """`gather` orchestrates section selection and provider scoping.""" + + async def test_all_sections(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("CONDUCTOR_NO_UPDATE_CHECK", "1") + report = await d.gather() + assert report.env is not None + assert report.providers is not None + assert report.registries is not None + names = {p.name for p in report.providers} + assert {"copilot", "claude", "openai-agents"} <= names + + async def test_single_section(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("CONDUCTOR_NO_UPDATE_CHECK", "1") + report = await d.gather(sections=("registries",)) + assert report.env is None + assert report.providers is None + assert report.registries is not None + + async def test_provider_scoping(self) -> None: + report = await d.gather(sections=("providers",), provider="claude") + assert report.providers is not None + assert [p.name for p in report.providers] == ["claude"] + + async def test_report_to_dict_omits_missing_sections( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("CONDUCTOR_NO_UPDATE_CHECK", "1") + report = await d.gather(sections=("env",)) + as_dict = report.to_dict() + assert set(as_dict) == {"env"} diff --git a/tests/test_providers/test_list_models.py b/tests/test_providers/test_list_models.py new file mode 100644 index 00000000..7ee9dce3 --- /dev/null +++ b/tests/test_providers/test_list_models.py @@ -0,0 +1,103 @@ +"""Tests for the public ``list_models()`` provider method (issue #274). + +Covers the base default (``None``) plus the Copilot and Claude overrides, +including their never-raise behavior at the SDK boundary. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock + +import pytest + +from conductor.providers.base import AgentProvider +from conductor.providers.claude import ClaudeProvider +from conductor.providers.copilot import CopilotProvider + + +class _StubProvider(AgentProvider, abstract=True): + """Minimal concrete provider to exercise the base ``list_models`` default.""" + + async def execute(self, *args: Any, **kwargs: Any) -> Any: # noqa: D102 + raise NotImplementedError + + async def validate_connection(self) -> bool: # noqa: D102 + return True + + async def close(self) -> None: # noqa: D102 + return None + + +class TestBaseListModels: + """The base implementation returns ``None`` (no enumeration).""" + + async def test_default_returns_none(self) -> None: + provider = _StubProvider() + assert await provider.list_models() is None + + +class TestCopilotListModels: + """Copilot enumerates model ids via ``client.list_models()``.""" + + async def test_returns_model_ids(self) -> None: + provider = CopilotProvider() + provider._ensure_client_started = AsyncMock() # type: ignore[method-assign] + fake_client = SimpleNamespace( + list_models=AsyncMock( + return_value=[SimpleNamespace(id="gpt-5"), SimpleNamespace(id="claude-x")] + ) + ) + provider._client = fake_client # type: ignore[assignment] + + assert await provider.list_models() == ["gpt-5", "claude-x"] + + async def test_mock_handler_mode_returns_none(self) -> None: + provider = CopilotProvider(mock_handler=lambda *a, **k: {}) + assert await provider.list_models() is None + + async def test_sdk_unavailable_returns_none(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("conductor.providers.copilot.COPILOT_SDK_AVAILABLE", False) + provider = CopilotProvider() + assert await provider.list_models() is None + + async def test_sdk_error_returns_none(self) -> None: + provider = CopilotProvider() + provider._ensure_client_started = AsyncMock() # type: ignore[method-assign] + fake_client = SimpleNamespace(list_models=AsyncMock(side_effect=RuntimeError("boom"))) + provider._client = fake_client # type: ignore[assignment] + + assert await provider.list_models() is None + + +class TestClaudeListModels: + """Claude enumerates model ids via ``client.models.list()``.""" + + async def test_returns_model_ids(self) -> None: + provider = ClaudeProvider(api_key="test-key") + page = SimpleNamespace( + data=[SimpleNamespace(id="claude-a"), SimpleNamespace(id="claude-b")] + ) + provider._client = SimpleNamespace( # type: ignore[assignment] + models=SimpleNamespace(list=AsyncMock(return_value=page)) + ) + + assert await provider.list_models() == ["claude-a", "claude-b"] + + async def test_client_none_returns_none(self) -> None: + provider = ClaudeProvider(api_key="test-key") + provider._client = None + assert await provider.list_models() is None + + async def test_sdk_error_returns_none(self) -> None: + provider = ClaudeProvider(api_key="test-key") + provider._client = SimpleNamespace( # type: ignore[assignment] + models=SimpleNamespace(list=AsyncMock(side_effect=RuntimeError("boom"))) + ) + assert await provider.list_models() is None + + async def test_sdk_unavailable_returns_none(self, monkeypatch: pytest.MonkeyPatch) -> None: + provider = ClaudeProvider(api_key="test-key") + monkeypatch.setattr("conductor.providers.claude.ANTHROPIC_SDK_AVAILABLE", False) + assert await provider.list_models() is None