From a26416bc1d6cf3c7deab7c2cb68b005bb0e62dcd Mon Sep 17 00:00:00 2001 From: ernestprovo23 Date: Tue, 21 Jul 2026 06:06:25 -0400 Subject: [PATCH] feat(cli): persist durable council results --- CHANGELOG.md | 7 + DOCUMENTATION_INDEX.md | 1 + README.md | 12 ++ docs/PRODUCT_DESIGN_DOCUMENT.md | 5 + .../2026-07-21-durable-json-output-design.md | 52 +++++ src/conclave/cli.py | 66 ++++++- tests/test_cli.py | 185 +++++++++++++++++- 7 files changed, 324 insertions(+), 4 deletions(-) create mode 100644 docs/plans/2026-07-21-durable-json-output-design.md diff --git a/CHANGELOG.md b/CHANGELOG.md index c332f9c..58e53f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **Durable buffered CLI results (#58).** `conclave ask --json-output PATH` atomically + persists the complete `CouncilResult` JSON with user-private temporary-file semantics + while preserving stdout and exit-code behavior. Persistence remains opt-in and is + rejected with streaming; write failures retain normal stdout before exiting 1. + ## [1.2.0] - 2026-07-18 ### Added diff --git a/DOCUMENTATION_INDEX.md b/DOCUMENTATION_INDEX.md index 0f1dd80..7d870fb 100644 --- a/DOCUMENTATION_INDEX.md +++ b/DOCUMENTATION_INDEX.md @@ -36,6 +36,7 @@ the canonical authority spec on top of those. | **H1 Synthetic QA Pack** | [`studies/elite_qa_v1/README.md`](studies/elite_qa_v1/README.md) | Balanced 24-task open-book harness fixture, QA protocol, and confirmatory preregistration template. | | **H1 Live Runner Design** | [`docs/plans/2026-07-18-h1-live-evaluation-runner-design.md`](docs/plans/2026-07-18-h1-live-evaluation-runner-design.md) | Sequential paid-exploratory execution, hash-bound UTF-8 estimates, USD 10 cap, authenticated checkpoints, and no-repeat resume. | | **H1 Live Runner Plan** | [`docs/plans/2026-07-18-h1-live-evaluation-runner.md`](docs/plans/2026-07-18-h1-live-evaluation-runner.md) | Exact TDD tasks for the six live conditions, dry-run estimator, replay fixtures, CLI gate, and correctness-only paid smoke. | +| **Durable JSON Output Design** | [`docs/plans/2026-07-21-durable-json-output-design.md`](docs/plans/2026-07-21-durable-json-output-design.md) | Opt-in atomic user-private result persistence for long buffered council runs and detached supervisors. | --- diff --git a/README.md b/README.md index a328eea..766deab 100644 --- a/README.md +++ b/README.md @@ -132,8 +132,20 @@ conclave ask "Should we adopt a service mesh for 8 services?" \ # Machine-readable output (works for every mode; carries elite phase artifacts too) conclave ask "..." -c grok,perplexity --mode debate --json + +# Durable buffered output for supervisors that may detach before a long run finishes +conclave ask "..." -c grok,gemini,claude --mode adversarial \ + --json --json-output /secure/local/run-result.json ``` +`--json-output PATH` is opt-in and available for buffered runs. It atomically +persists the complete result with user-private temporary-file semantics (`0600` on +POSIX) while leaving stdout and exit codes unchanged. The parent directory must +already exist, existing files are replaced atomically, and `--stream` cannot be +combined with durable output. Result files contain the prompt and model answers, so +choose a secure local path. A persistence error still renders normal stdout, then +exits 1. + Published v1.2 mode flags are `--mode synthesize|raw|debate|adversarial|vote|elite`. `--rounds N` (default 2) is the *maximum* round count for `debate`; `--converge-threshold FLOAT` (or `--converge`/`--no-converge`) optionally stops a debate early once answers diff --git a/docs/PRODUCT_DESIGN_DOCUMENT.md b/docs/PRODUCT_DESIGN_DOCUMENT.md index a79f38c..f968a5e 100644 --- a/docs/PRODUCT_DESIGN_DOCUMENT.md +++ b/docs/PRODUCT_DESIGN_DOCUMENT.md @@ -111,6 +111,11 @@ markup, no middleman) and a security property. absent is skipped with a warning and recorded in `CouncilResult.skipped`. Unknown providers (no static env-var mapping) are *not* pre-emptively skipped — the live call is attempted and any auth error is captured as a `ModelAnswer.error`. +6. **Result persistence is explicit and local.** Buffered CLI runs persist nothing by + default. `--json-output PATH` opts into an atomic user-private file (`0600` on POSIX) + containing the same secret-free `CouncilResult` JSON; normal stdout remains available + even if persistence fails. The file still contains the prompt and answers, so operators + must select a secure path. **Residual considerations:** a provider error could in principle echo a key fragment. Since v0.3 every provider/transport error is passed through `redact()` before it reaches diff --git a/docs/plans/2026-07-21-durable-json-output-design.md b/docs/plans/2026-07-21-durable-json-output-design.md new file mode 100644 index 0000000..d5cc55d --- /dev/null +++ b/docs/plans/2026-07-21-durable-json-output-design.md @@ -0,0 +1,52 @@ +# Durable JSON Output Design + +## Problem + +`conclave ask --json` writes its result only to stdout after the full council run. +Long adversarial runs can outlive a supervising shell or agent tool's output session, +so the council succeeds but the caller loses the final JSON. Increasing a caller +timeout is brittle, and a wrapper cannot guarantee that every integration handles +process detachment correctly. + +## Decision + +Add an opt-in `--json-output PATH` option to `conclave ask`. After a council result +is complete, Conclave serializes the same payload used by `--json` and writes it to +the requested path before applying the existing exit-code contract. The option does +not require `--json`: callers may retain human terminal output while polling the +durable file. Existing stdout behavior remains unchanged. + +The file is written atomically through a securely created temporary sibling and +replaced only after the complete JSON has been flushed. It uses user-private +temporary-file semantics (`0600` on POSIX). The parent directory must already exist; +Conclave will not create directory trees or silently redirect output. A write failure +is reported without exposing the result body, preserves normal stdout, and exits 1. + +Alternatives rejected: + +- Raising orchestration timeouts still loses output when the supervisor detaches. +- A repository-specific shell wrapper duplicates lifecycle and security behavior. +- Always persisting results would violate Conclave's current no-storage default. + +## Data Flow and Safety + +The council runs exactly as it does today. The completed `CouncilResult` is converted +once with `_result_to_dict`, encoded as JSON, optionally written to the owner-only +path, and then rendered to stdout or Rich panels. API keys remain outside the result +model and are never written. Because prompts and model answers may be sensitive, +persistence stays explicit and the documentation warns operators to select a secure +local path. + +Streaming remains unchanged: `--json-output` is rejected with `--stream`, because the +streaming branch currently returns before constructing the shared rendering path. +Supporting both would enlarge the fix without solving the detached adversarial-run +failure. + +## Verification and Rollback + +Regression tests cover successful durable output, user-private permissions, identical +stdout/file payloads under `--json`, persistence-failure stdout, portability, and +pre-call rejection of unsupported destinations or streaming. Existing CLI and full +offline suites must remain green, followed by lint and secret scanning. Rollback is a +single option/helper removal; no persisted format, cache, config, or public Python API +changes. diff --git a/src/conclave/cli.py b/src/conclave/cli.py index 137e5e0..2f5b88e 100644 --- a/src/conclave/cli.py +++ b/src/conclave/cli.py @@ -14,6 +14,9 @@ from __future__ import annotations import json +import os +import tempfile +from pathlib import Path import typer from rich.console import Console @@ -41,6 +44,34 @@ def _result_to_dict(result: CouncilResult) -> dict: return result.model_dump(mode="json") +def _write_json_output(path: Path, payload: dict) -> None: + """Atomically persist a complete JSON payload with user-private permissions.""" + # mkstemp avoids name races and creates a user-private file on supported + # platforms; replacing the destination preserves those permissions. + fd, temp_name = tempfile.mkstemp( + dir=path.parent, + prefix=f".{path.name}.", + suffix=".tmp", + ) + temp_path = Path(temp_name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as handle: + json.dump(payload, handle) + handle.write("\n") + handle.flush() + os.fsync(handle.fileno()) + os.replace(temp_path, path) + finally: + try: + os.close(fd) + except OSError: + pass + try: + temp_path.unlink(missing_ok=True) + except OSError: + pass + + def _answer_panel(ans, *, border: str = "cyan") -> Panel: """Build a rich panel for a single ModelAnswer (ok or failed).""" if ans.ok: @@ -493,6 +524,14 @@ def ask( as_json: bool = typer.Option( False, "--json", help="Emit the full result as JSON instead of panels." ), + json_output: Path | None = typer.Option( # noqa: B008 + None, + "--json-output", + help=( + "Persist the complete result JSON to PATH with user-private permissions. " + "Buffered modes only; stdout rendering is unchanged." + ), + ), cache: bool | None = typer.Option( None, "--cache/--no-cache", @@ -538,6 +577,14 @@ def ask( ) raise typer.Exit(code=2) + if stream and json_output is not None: + err_console.print("[red]--json-output cannot be combined with --stream.[/red]") + raise typer.Exit(code=2) + + if json_output is not None and (not json_output.parent.is_dir() or json_output.is_dir()): + err_console.print("[red]--json-output requires an existing parent directory.[/red]") + raise typer.Exit(code=2) + if mode_lower == "vote" and not choices: err_console.print( "[red]--mode vote requires --choices (e.g. --choices 'Yes,No,Abstain').[/red]" @@ -587,11 +634,22 @@ def ask( result.elite is None or result.elite.decision_readiness != "ready" ) + payload = _result_to_dict(result) if as_json or json_output is not None else None + json_output_failed = False + if json_output is not None: + try: + _write_json_output(json_output, payload or {}) + except Exception: + # The completed result must remain available on its normal stdout path + # even when the optional persistence side effect fails. + err_console.print("[red]Could not write --json-output.[/red]") + json_output_failed = True + 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 or elite_not_ready: + console.print_json(json.dumps(payload)) + if json_output_failed or no_usable_answers or elite_not_ready: raise typer.Exit(code=1) return @@ -611,6 +669,8 @@ def ask( f"{result.elite.decision_readiness} ({reasons})[/red]" ) raise typer.Exit(code=1) + if json_output_failed: + raise typer.Exit(code=1) return if no_usable_answers: @@ -620,6 +680,8 @@ def ask( raise typer.Exit(code=1) _RENDERERS[result.mode](result) + if json_output_failed: + raise typer.Exit(code=1) @app.command() diff --git a/tests/test_cli.py b/tests/test_cli.py index ff3fa02..7a431a3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,13 +1,16 @@ -"""Tests for the CLI exit-code contract and HTTP-client lifecycle wiring. +"""Tests for CLI exit codes, durable output, and HTTP-client lifecycle wiring. These exercise ``conclave.cli.ask`` through Typer's ``CliRunner`` (no real keys, -no network). Two concerns are pinned here: +no network). Three concerns are pinned here: * **Exit-code contract (#17).** A run that produces zero *usable* member answers, or an Elite run whose ``decision_readiness`` is not ``ready``, exits non-zero (code 1) on both the human and ``--json`` paths. 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. Other runs with a usable answer exit 0. +* **Durable result output (#58).** Buffered runs can atomically persist the same + complete JSON payload with user-private permissions, including before a failure exit. + Invalid destinations and streaming combinations fail before council construction. * **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. @@ -16,6 +19,7 @@ from __future__ import annotations import json +import stat import pytest from typer.testing import CliRunner @@ -98,6 +102,183 @@ def handler(model, messages, **kwargs): assert all(a["answer"].startswith("answer from") for a in payload["answers"]) +def test_json_output_persists_same_payload_owner_only( + monkeypatch, patch_cli_config, patch_call_model, tmp_path +): + """A detached caller can recover the complete JSON from an owner-only file.""" + 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) + output_path = tmp_path / "result.json" + result = runner.invoke( + cli.app, + [ + "ask", + "hello", + "--council", + "grok,gemini", + "--mode", + "raw", + "--json", + "--json-output", + str(output_path), + ], + ) + + assert result.exit_code == 0 + assert json.loads(output_path.read_text(encoding="utf-8")) == json.loads(result.stdout) + assert stat.S_IMODE(output_path.stat().st_mode) == 0o600 + + +def test_json_output_with_stream_is_rejected_before_council_creation( + monkeypatch, patch_cli_config, tmp_path +): + """Durable buffered output cannot silently take the streaming branch.""" + created = False + + class ForbiddenCouncil: + def __init__(self, **kwargs): + nonlocal created + created = True + + monkeypatch.setattr(cli, "Council", ForbiddenCouncil) + result = runner.invoke( + cli.app, + ["ask", "hello", "--stream", "--json-output", str(tmp_path / "result.json")], + ) + + assert result.exit_code == 2 + assert created is False + assert "--json-output" in result.output + assert "--stream" in result.output + + +def test_json_output_persists_failure_payload_before_exit(clear_keys, patch_cli_config, tmp_path): + """The durable payload is complete even when the existing exit contract returns 1.""" + output_path = tmp_path / "result.json" + result = runner.invoke( + cli.app, + ["ask", "hello", "--json", "--json-output", str(output_path)], + ) + + assert result.exit_code == 1 + assert json.loads(output_path.read_text(encoding="utf-8")) == json.loads(result.stdout) + assert json.loads(result.stdout)["answers"] == [] + + +def test_json_output_rejects_missing_parent_before_council_creation( + monkeypatch, patch_cli_config, tmp_path +): + """A bad destination fails before any provider-capable Council is created.""" + created = False + + class ForbiddenCouncil: + def __init__(self, **kwargs): + nonlocal created + created = True + + monkeypatch.setattr(cli, "Council", ForbiddenCouncil) + output_path = tmp_path / "missing" / "result.json" + result = runner.invoke( + cli.app, + ["ask", "hello", "--json-output", str(output_path)], + ) + + assert result.exit_code == 2 + assert created is False + assert "existing parent directory" in result.output + + +def test_json_output_write_failure_preserves_stdout( + monkeypatch, patch_cli_config, patch_call_model, tmp_path +): + """Persistence failure changes the exit code, not the completed stdout payload.""" + for var in ("XAI_API_KEY", "GEMINI_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}") + + def fail_write(path, payload): + raise OSError("simulated disk failure") + + patch_call_model(handler) + monkeypatch.setattr(cli, "_write_json_output", fail_write) + result = runner.invoke( + cli.app, + [ + "ask", + "hello", + "--council", + "grok,gemini", + "--mode", + "raw", + "--json", + "--json-output", + str(tmp_path / "result.json"), + ], + ) + + assert result.exit_code == 1 + assert len(json.loads(result.stdout)["answers"]) == 2 + assert "Could not write --json-output" in result.output + + +def test_json_output_does_not_require_posix_fchmod( + monkeypatch, patch_cli_config, patch_call_model, tmp_path +): + """Secure temporary-file persistence works when POSIX-only fchmod is unavailable.""" + for var in ("XAI_API_KEY", "GEMINI_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) + monkeypatch.delattr(cli.os, "fchmod") + output_path = tmp_path / "result.json" + result = runner.invoke( + cli.app, + [ + "ask", + "hello", + "--council", + "grok,gemini", + "--mode", + "raw", + "--json-output", + str(output_path), + ], + ) + + assert result.exit_code == 0 + assert json.loads(output_path.read_text(encoding="utf-8"))["prompt"] == "hello" + + +def test_json_output_cleans_partial_temp_on_serialization_failure(monkeypatch, tmp_path): + """Sensitive partial JSON is removed when serialization fails mid-write.""" + + def fail_dump(payload, handle): + handle.write('{"partial":') + raise ValueError("simulated serialization failure") + + monkeypatch.setattr(cli.json, "dump", fail_dump) + with pytest.raises(ValueError, match="simulated serialization failure"): + cli._write_json_output(tmp_path / "result.json", {"prompt": "sensitive"}) + + assert list(tmp_path.iterdir()) == [] + + 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"])