From 3441906fb98127fea6deb385963d6a7aa69e0480 Mon Sep 17 00:00:00 2001 From: ernestprovo23 Date: Mon, 8 Jun 2026 22:22:52 -0400 Subject: [PATCH] cli: exit-code contract for zero-answer runs + close pooled httpx client closes #17: establish a documented exit-code contract on `conclave ask`. A run that produces zero *usable* member answers now exits non-zero (1) on both the human and `--json` paths. Under `--json` the full JSON result is still emitted to stdout first, so a script can parse the payload and detect failure via the exit code. Usage/config errors keep their distinct code 2. The check is computed once (`result.successful_answers`) and applied to both paths, so an all-members-failed run is also treated as a failure (previously the human path keyed off `result.answers` non-emptiness and the JSON path always exited 0). closes #20: wire the pooled httpx client lifecycle. The sync wrappers (`ask_sync`/`debate_sync`/`adversarial_sync`) now close the shared client in a `finally` inside the same event loop that used it, so the CLI never leaks the pool and no "unclosed client" warning fires. Added `Council.aclose()` / `Council.close_sync()` and re-exported `conclave.aclose` so long-lived library/server consumers can release the pool on shutdown. tests: new tests/test_cli.py (Typer CliRunner) pins the exit-code contract (zero members, all-failed, success, bad mode, empty council) and verifies `transport.aclose()` is actually invoked on sync run completion (no leaked client). cli.py coverage 0% -> 43%; total 82.7% (floor 75%). --- src/conclave/__init__.py | 9 +++ src/conclave/cli.py | 25 +++++- src/conclave/council.py | 54 +++++++++++-- tests/test_cli.py | 163 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 242 insertions(+), 9 deletions(-) create mode 100644 tests/test_cli.py diff --git a/src/conclave/__init__.py b/src/conclave/__init__.py index 8734993..d866e08 100644 --- a/src/conclave/__init__.py +++ b/src/conclave/__init__.py @@ -21,6 +21,13 @@ latency, token usage, and any error) plus the merged synthesis. For ``debate`` it also carries per-round answers (``rounds``); for ``adversarial`` it carries the proposal/critique/verdict structure (``adversarial``). + +The ``*_sync`` wrappers close the pooled HTTP client automatically. Long-lived +consumers that drive the async API on their own event loop (e.g. a server) must +release the process-wide connection pool on shutdown:: + + import conclave + await conclave.aclose() # or: await council.aclose() """ from __future__ import annotations @@ -34,6 +41,7 @@ ModelAnswer, TokenUsage, ) +from .transport import aclose __version__ = "0.1.0" @@ -46,5 +54,6 @@ "AdversarialResult", "ConclaveConfig", "load_config", + "aclose", "__version__", ] diff --git a/src/conclave/cli.py b/src/conclave/cli.py index aaca8e6..379b740 100644 --- a/src/conclave/cli.py +++ b/src/conclave/cli.py @@ -166,7 +166,17 @@ def ask( False, "--json", help="Emit the full result as JSON instead of panels." ), ) -> None: - """Fan PROMPT out to a council and synthesize, debate, or adversarially review.""" + """Fan PROMPT out to a council and synthesize, debate, or adversarially review. + + Exit codes: + + * 0 -- the run produced at least one usable member answer. + * 1 -- the run completed but produced zero usable member answers (e.g. no + council member had an API key, or every member failed). Under ``--json`` + the full JSON result is still emitted to stdout first, so a script can both + parse the payload and detect the failure via the non-zero exit code. + * 2 -- a usage/config error (unknown mode, or no members resolved). + """ mode_lower = mode.lower() if mode_lower not in _VALID_MODES: err_console.print( @@ -188,13 +198,22 @@ def ask( else: result = c.ask_sync(prompt, synthesize=(mode_lower == "synthesize")) + # A run that produced no usable member answers is a failure for scripting + # purposes regardless of output format. We compute this once and apply the + # same exit-code contract to both the JSON and human paths. + no_usable_answers = not result.successful_answers + if as_json: + # Always emit valid JSON to stdout so a consumer can parse the payload, + # then signal failure via the exit code if nothing usable came back. console.print_json(json.dumps(_result_to_dict(result))) + if no_usable_answers: + raise typer.Exit(code=1) return - if not result.answers: + if no_usable_answers: err_console.print( - "[red]No council members had keys available. Run 'conclave providers' to check.[/red]" + "[red]No usable council answers. Run 'conclave providers' to check keys.[/red]" ) raise typer.Exit(code=1) diff --git a/src/conclave/council.py b/src/conclave/council.py index 47af852..51c1742 100644 --- a/src/conclave/council.py +++ b/src/conclave/council.py @@ -15,6 +15,7 @@ import asyncio from collections.abc import Callable +from . import transport from .config import ConclaveConfig, load_config from .logging import get_logger from .models import CouncilResult, ModelAnswer @@ -247,16 +248,57 @@ async def adversarial(self, prompt: str, proposer: str | None = None) -> Council return await run_adversarial(self, prompt, proposer=proposer) + async def aclose(self) -> None: + """Close the shared pooled HTTP client. + + Library users running their own event loop (e.g. a server) should call + this on shutdown so the process-wide connection pool is released and no + "unclosed client" warning is emitted under strict asyncio. It is safe to + call more than once; the pooled client is recreated lazily on next use. + + The synchronous wrappers (:meth:`ask_sync`, :meth:`debate_sync`, + :meth:`adversarial_sync`) already close the client automatically before + their event loop ends, so CLI/sync callers do not need to call this. + """ + await transport.aclose() + + def close_sync(self) -> None: + """Synchronous wrapper around :meth:`aclose` for non-async callers.""" + self._run_sync(self.aclose, "close_sync", close_client=False) + @staticmethod - def _run_sync(coro_factory: Callable[[], asyncio.Future | object], label: str): - """Run an async council method synchronously, guarding nested loops.""" + def _run_sync( + coro_factory: Callable[[], asyncio.Future | object], + label: str, + *, + close_client: bool = True, + ): + """Run an async council method synchronously, guarding nested loops. + + ``asyncio.run`` creates (and tears down) a fresh event loop per call. The + pooled httpx client is bound to whichever loop first used it, so we close + it inside that same loop's ``finally`` before the loop is destroyed -- + otherwise the pool leaks and asyncio emits an "unclosed client" warning. + ``close_client=False`` is used by :meth:`close_sync` itself to avoid + recursively re-closing. + """ try: asyncio.get_running_loop() except RuntimeError: - return asyncio.run(coro_factory()) - raise RuntimeError( - f"{label}() called from within a running event loop; await the async method instead" - ) + pass + else: + raise RuntimeError( + f"{label}() called from within a running event loop; await the async method instead" + ) + + async def _runner(): + try: + return await coro_factory() + finally: + if close_client: + await transport.aclose() + + return asyncio.run(_runner()) def ask_sync(self, prompt: str, synthesize: bool = True) -> CouncilResult: """Synchronous wrapper around :meth:`ask`. diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..20af28c --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,163 @@ +"""Tests for the CLI exit-code contract and HTTP-client lifecycle wiring. + +These exercise ``conclave.cli.ask`` through Typer's ``CliRunner`` (no real keys, +no network). Two concerns are pinned here: + +* **Exit-code contract (#17).** A run that produces zero *usable* member answers + exits non-zero (code 1) on both the human and ``--json`` paths, and under + ``--json`` the full JSON payload is still emitted to stdout so a script can + parse the result *and* detect the failure via the exit code. A run with at + least one usable answer exits 0. +* **Pooled-client lifecycle (#20).** The synchronous council wrappers close the + shared httpx client when the run completes, so ``transport.aclose()`` is + actually invoked and no client leaks past the CLI command. +""" + +from __future__ import annotations + +import json + +import pytest +from typer.testing import CliRunner + +from conclave import cli +from conclave.config import ConclaveConfig + +runner = CliRunner() + + +def _config() -> ConclaveConfig: + """A deterministic config independent of any on-disk ~/.conclave file.""" + return ConclaveConfig( + models={ + "grok": "xai/grok-4.3", + "gemini": "gemini/gemini-2.5-pro", + "claude": "anthropic/claude-sonnet-4-6", + "perplexity": "perplexity/sonar-pro", + }, + councils={"default": ["grok", "gemini", "claude", "perplexity"]}, + synthesizer="claude", + ) + + +@pytest.fixture +def patch_cli_config(monkeypatch): + """Make ``conclave.cli.load_config`` return the deterministic test config.""" + monkeypatch.setattr(cli, "load_config", _config) + + +def test_no_members_human_exits_one(clear_keys, patch_cli_config): + """Plain (human) path: zero usable answers -> exit code 1.""" + result = runner.invoke(cli.app, ["ask", "hello"]) + assert result.exit_code == 1 + assert "No usable council answers" in result.output + + +def test_no_members_json_exits_one_but_emits_json(clear_keys, patch_cli_config): + """JSON path: zero usable answers -> exit 1, yet valid JSON still on stdout.""" + result = runner.invoke(cli.app, ["ask", "hello", "--json"]) + assert result.exit_code == 1 + payload = json.loads(result.stdout) + assert payload["answers"] == [] + assert payload["prompt"] == "hello" + + +def test_all_members_failed_exits_one(monkeypatch, patch_cli_config, patch_call_model): + """Keys present but every member errors -> zero usable answers -> exit 1.""" + for var in ("XAI_API_KEY", "GEMINI_API_KEY", "ANTHROPIC_API_KEY", "PERPLEXITY_API_KEY"): + monkeypatch.setenv(var, "dummy-key") + + def handler(model, messages, **kwargs): + raise RuntimeError("provider down") + + patch_call_model(handler) + result = runner.invoke(cli.app, ["ask", "hello", "--council", "grok,gemini", "--mode", "raw"]) + assert result.exit_code == 1 + + +def test_successful_run_exits_zero(monkeypatch, patch_cli_config, patch_call_model): + """At least one usable answer -> exit 0, JSON payload carries the answers.""" + for var in ("XAI_API_KEY", "GEMINI_API_KEY", "ANTHROPIC_API_KEY", "PERPLEXITY_API_KEY"): + monkeypatch.setenv(var, "dummy-key") + + def handler(model, messages, **kwargs): + from tests.conftest import make_response + + return make_response(f"answer from {model}") + + patch_call_model(handler) + result = runner.invoke( + cli.app, + ["ask", "hello", "--council", "grok,gemini", "--mode", "raw", "--json"], + ) + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert len(payload["answers"]) == 2 + assert all(a["answer"].startswith("answer from") for a in payload["answers"]) + + +def test_unknown_mode_exits_two(patch_cli_config): + """Usage error (bad mode) keeps its distinct exit code 2.""" + result = runner.invoke(cli.app, ["ask", "hello", "--mode", "bogus"]) + assert result.exit_code == 2 + + +def test_unresolved_council_exits_two(patch_cli_config): + """Usage error (empty council selector resolves to zero members) -> exit 2.""" + result = runner.invoke(cli.app, ["ask", "hello", "--council", " , "]) + assert result.exit_code == 2 + assert "No council members resolved" in result.output + + +def test_sync_run_closes_pooled_client(monkeypatch, patch_call_model): + """The sync wrapper invokes transport.aclose() so the client never leaks.""" + import conclave.council as council_mod + from conclave import Council + + for var in ("XAI_API_KEY", "GEMINI_API_KEY", "ANTHROPIC_API_KEY", "PERPLEXITY_API_KEY"): + monkeypatch.setenv(var, "dummy-key") + + calls = {"aclose": 0} + + async def fake_aclose(): + calls["aclose"] += 1 + + monkeypatch.setattr(council_mod.transport, "aclose", fake_aclose) + + def handler(model, messages, **kwargs): + from tests.conftest import make_response + + return make_response("ok") + + patch_call_model(handler) + Council(models=["grok"], synthesizer="grok", config=_config()).ask_sync( + "hello", synthesize=False + ) + assert calls["aclose"] == 1 + + +def test_close_sync_invokes_aclose(monkeypatch): + """Council.close_sync drives transport.aclose without re-closing recursively.""" + import conclave.council as council_mod + from conclave import Council + + calls = {"aclose": 0} + + async def fake_aclose(): + calls["aclose"] += 1 + + monkeypatch.setattr(council_mod.transport, "aclose", fake_aclose) + Council(models=["grok"], config=_config()).close_sync() + # close_sync passes close_client=False, so aclose fires exactly once (the body), + # not twice (body + finally). + assert calls["aclose"] == 1 + + +async def test_aclose_really_closes_real_client(): + """End-to-end: a real pooled client gets created then closed by aclose().""" + from conclave import transport + + client = transport._get_client() + assert not client.is_closed + await transport.aclose() + assert client.is_closed