diff --git a/README.md b/README.md index 0895fba..6dd1482 100644 --- a/README.md +++ b/README.md @@ -469,12 +469,24 @@ Global `--json` can be placed before a subcommand for machine-readable output, f | `answerable demo [causal\|grain\|maturity]` | Runs a built-in adversarial case end to end. | | `answerable assess --data ... --question --output ` | Executes the full assessment and writes an Evidence Warrant. | | `answerable warrant verify --warrant ` | Verifies warrant integrity. | -| `answerable warrant show \| export` | Inspects or exports a warrant. | +| `answerable warrant show \| export` | Not available; exits 5. Read the JSON/Markdown artifacts produced by `assess`. | | `answerable benchmark mutations --output ` | Runs the live 112-pair mutation benchmark. | | `answerable benchmark --freeze --output ` | Produces a hash-addressed frozen benchmark release. | -| `answerable source add \| test` | Registers and health-checks supported read-only data connectors. | +| `answerable source add \| test` | Not available; exits 5. | | `answerable mcp` | Starts the packaged FastMCP stdio server. | +`frame`, `plan`, `execute` and `inspect` are also reserved CLI names, not +implemented operations; they return `command_unavailable` and exit 5. Use +`init` for question scaffolding, `assess` for the full run, and the documented +MCP tools for inspection and reading assessment artifacts. + +`warrant verify` requires `--warrant`. A missing, unreadable or malformed file +returns an explicit error (exit 2), never success. A checked but invalid warrant +returns `valid: false` (exit 3), including in human output. A valid warrant +returns exit 0; this verifies integrity, not the truth of its claims. +With `--json`, these errors are structured JSON on stdout; human errors go to +stderr. Existing parser usage errors follow argparse's stderr/exit-2 behavior. + External-agent benchmark utilities live under `scripts/` and preserve blind case export, raw agent runs, decision scoring, statistical comparison and SVG regeneration. ## Engineering evidence diff --git a/docs/adr/0004-cli-fail-closed.md b/docs/adr/0004-cli-fail-closed.md new file mode 100644 index 0000000..bf12962 --- /dev/null +++ b/docs/adr/0004-cli-fail-closed.md @@ -0,0 +1,35 @@ +# ADR 0004 — CLI commands must not report success without an operation + +Status: implemented; CI confirmation required before merge. + +## Context + +FR-CLI-001 acceptance probes found eight reserved command/action combinations +returning exit 0 and `status: ok` without performing any operation. `warrant +verify` without a path did the same. Invalid warrants returned exit 3 but the +human-readable output still said `ok`. Existing tests preserved one of these +incorrect success cases instead of testing an actual verification. + +## Decision + +- Reserved `frame`, `plan`, `execute`, `inspect`, `source add/test` and + `warrant show/export` return exit 5 and `command_unavailable`. Help and README + explicitly mark them unavailable. This change does not implement them. +- Verification requires a path. Missing, unreadable and malformed inputs return + exit 2 with `warrant_required` or `warrant_unreadable`. No `valid` field is + emitted when verification could not be performed. +- A checked warrant retains exit 0 / `valid: true` or exit 3 / `valid: false`. + Human output distinguishes valid from INVALID rather than printing `ok`. +- JSON command errors go to stdout; human errors go to stderr. Parser usage + errors retain argparse's existing behavior. The fallback is failure, so a + future command cannot silently become successful without a handler. + +## Compatibility and validation + +No warrant schema or historical artifact changes. Automation relying on fake +success now receives a failure by design. FR-CLI-001 remains implemented until +CI confirms the candidate; its previous interface test is strengthened. +Subprocess tests exercise exit codes, both output formats, absent/malformed +inputs, no side effects for unavailable operations, and real generated/tampered +warrants. Coverage thresholds remain unchanged. Stable-product readiness still +requires the separate release criteria, including independent and agent studies. diff --git a/requirements/traceability.yaml b/requirements/traceability.yaml index e142115..8057dd6 100644 --- a/requirements/traceability.yaml +++ b/requirements/traceability.yaml @@ -366,7 +366,7 @@ requirements: status: verified FR-CLI-001: implementation: ["src/answerable/cli.py", "src/answerable/__init__.py"] - tests: ["tests/test_interfaces.py", "tests/test_distribution_version.py"] + tests: ["tests/test_interfaces.py", "tests/test_distribution_version.py", "tests/test_cli_acceptance.py"] status: implemented FR-MCP-001: implementation: ["src/answerable/interfaces/mcp.py"] diff --git a/src/answerable/cli.py b/src/answerable/cli.py index 6e1d765..c7efbc5 100644 --- a/src/answerable/cli.py +++ b/src/answerable/cli.py @@ -17,6 +17,8 @@ EXIT_BLOCKED = 2 EXIT_INVALID_WARRANT = 3 EXIT_BENCHMARK_FAILED = 4 +EXIT_UNAVAILABLE = 5 +EXIT_USAGE = 2 def build_parser() -> argparse.ArgumentParser: @@ -27,7 +29,7 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--json", action="store_true", dest="json_output") subparsers = parser.add_subparsers(dest="command", required=True) for command in COMMANDS: - subparsers.add_parser(command) + subparsers.add_parser(command, help="Not available in this release (exit 5).") subparsers.add_parser("doctor", help="Check that the local Answerable runtime is ready.") @@ -59,10 +61,14 @@ def build_parser() -> argparse.ArgumentParser: assess.add_argument("--format", choices=("json", "markdown", "both"), default="both") warrant = subparsers.add_parser("warrant") - warrant.add_argument("action", choices=("show", "export", "verify")) + warrant.add_argument( + "action", + choices=("show", "export", "verify"), + help="verify requires --warrant; show/export are not available (exit 5).", + ) warrant.add_argument("--warrant", type=Path, default=None) - source = subparsers.add_parser("source") + source = subparsers.add_parser("source", help="Not available in this release (exit 5).") source.add_argument("action", choices=("add", "test")) subparsers.add_parser( @@ -278,11 +284,25 @@ def _mcp() -> int: return 0 -def _verify(path: Path) -> tuple[int, dict[str, object]]: +def _verify(path: Path | None) -> tuple[int, dict[str, object]]: from answerable.application.assessment_runner import load_warrant from answerable.public import verify_warrant - valid = verify_warrant(load_warrant(path)) + if path is None: + return EXIT_USAGE, { + "status": "error", + "code": "warrant_required", + "message": "warrant verify requires --warrant ; nothing was verified.", + } + try: + valid = verify_warrant(load_warrant(path)) + except (OSError, ValueError, TypeError, KeyError, AttributeError): + return EXIT_USAGE, { + "status": "error", + "code": "warrant_unreadable", + "warrant": str(path), + "message": "Cannot read or decode the warrant; nothing was verified.", + } return (0 if valid else EXIT_INVALID_WARRANT), {"warrant": str(path), "valid": valid} @@ -300,15 +320,25 @@ def main(argv: Sequence[str] | None = None) -> int: code, payload = _benchmark(args, json_output=args.json_output) elif args.command == "doctor": code, payload = _doctor() - elif args.command == "warrant" and args.action == "verify" and args.warrant is not None: + elif args.command == "warrant" and args.action == "verify": code, payload = _verify(args.warrant) else: - code, payload = 0, {"status": "ok"} + code, payload = ( + EXIT_UNAVAILABLE, + { + "status": "error", + "code": "command_unavailable", + "message": "This command is not available in this release; " + "no operation was performed.", + }, + ) payload["command"] = args.command if getattr(args, "action", None): payload["action"] = args.action if args.json_output: print(json.dumps(payload, sort_keys=True, ensure_ascii=False)) + elif payload.get("status") == "error": + print(f"answerable: {payload['message']}", file=sys.stderr) elif args.command == "doctor": print(f"Answerable {payload['version']}") print(f"Python {payload['python']}") @@ -317,6 +347,10 @@ def main(argv: Sequence[str] | None = None) -> int: marker = "+" if status == "ok" else "x" print(f"{marker} {dependency}: {status}") print(f"Status: {payload['status']}") - elif args.command not in {"assess", "demo", "benchmark", "init"}: - print(f"answerable {args.command}: ok") + elif args.command == "warrant": + label = "valid" if payload["valid"] else "INVALID" + print( + f"Warrant {payload['warrant']}: {label}", + file=sys.stdout if payload["valid"] else sys.stderr, + ) return code diff --git a/tests/test_cli_acceptance.py b/tests/test_cli_acceptance.py new file mode 100644 index 0000000..d8d50b4 --- /dev/null +++ b/tests/test_cli_acceptance.py @@ -0,0 +1,119 @@ +"""FR-CLI-001: a successful process must reflect an actual operation.""" + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +from answerable.application.assessment_runner import AssessmentRunner +from answerable.application.spec_loader import load_spec + +ENTRY = [sys.executable, "-c", "from answerable.cli import main; raise SystemExit(main())"] +UNAVAILABLE = [ + ("frame",), + ("plan",), + ("execute",), + ("inspect",), + ("source", "add"), + ("source", "test"), + ("warrant", "show"), + ("warrant", "export"), +] + + +@pytest.mark.parametrize("args", UNAVAILABLE) +@pytest.mark.parametrize("json_mode", [False, True]) +def test_FR_CLI_001_unavailable_commands_fail_without_side_effects( + tmp_path: Path, args: tuple[str, ...], json_mode: bool +) -> None: + result = subprocess.run( + [*ENTRY, *(["--json"] if json_mode else []), *args], + cwd=tmp_path, + capture_output=True, + text=True, + timeout=15, + ) + assert result.returncode == 5 + assert list(tmp_path.iterdir()) == [] + if json_mode: + payload = json.loads(result.stdout) + assert payload["status"] == "error" + assert payload["code"] == "command_unavailable" + assert result.stderr == "" + else: + assert result.stdout == "" + assert "not available" in result.stderr + + +@pytest.mark.parametrize("json_mode", [False, True]) +def test_FR_CLI_001_verify_requires_a_file(tmp_path: Path, json_mode: bool) -> None: + result = subprocess.run( + [*ENTRY, *(["--json"] if json_mode else []), "warrant", "verify"], + cwd=tmp_path, + capture_output=True, + text=True, + timeout=15, + ) + assert result.returncode == 2 + if json_mode: + payload = json.loads(result.stdout) + assert payload["code"] == "warrant_required" + assert "valid" not in payload + else: + assert "--warrant" in result.stderr + assert result.stdout == "" + + +@pytest.mark.parametrize("content", [None, "{", "{}", "[]"]) +def test_FR_CLI_001_unreadable_warrant_is_a_structured_error( + tmp_path: Path, content: str | None +) -> None: + path = tmp_path / "input.json" + if content is not None: + path.write_text(content, encoding="utf-8") + result = subprocess.run( + [*ENTRY, "--json", "warrant", "verify", "--warrant", str(path)], + capture_output=True, + text=True, + timeout=15, + ) + assert result.returncode == 2 + assert json.loads(result.stdout)["code"] == "warrant_unreadable" + assert "Traceback" not in result.stderr + + +def test_FR_CLI_001_real_warrant_and_tampering_have_distinct_exit_codes(tmp_path: Path) -> None: + example = Path(__file__).resolve().parents[1] / "examples/campaign_retention" + run = AssessmentRunner().run( + data_sources=(example / "customers.csv",), + spec=load_spec(example / "question.yaml"), + output_directory=tmp_path, + ) + path = run.artifacts["warrant"] + for tampered, expected in [(False, 0), (True, 3)]: + if tampered: + record = json.loads(path.read_text()) + record["canonical_json"] += " " + path.write_text(json.dumps(record)) + for json_mode in [False, True]: + result = subprocess.run( + [ + *ENTRY, + *(["--json"] if json_mode else []), + "warrant", + "verify", + "--warrant", + str(path), + ], + capture_output=True, + text=True, + timeout=15, + ) + assert result.returncode == expected + if json_mode: + assert json.loads(result.stdout)["valid"] is (not tampered) + else: + assert ("INVALID" if tampered else "valid") in (result.stdout + result.stderr) + assert ": ok" not in result.stdout diff --git a/tests/test_interfaces.py b/tests/test_interfaces.py index 763d39b..abeb96e 100644 --- a/tests/test_interfaces.py +++ b/tests/test_interfaces.py @@ -31,8 +31,11 @@ def test_phase_15_cli_json_and_exit_code_contract(self) -> None: output = StringIO() with redirect_stdout(output): code = main(("--json", "warrant", "verify")) - self.assertEqual(code, 0) - self.assertEqual(json.loads(output.getvalue())["action"], "verify") + self.assertEqual(code, 2) + payload = json.loads(output.getvalue()) + self.assertEqual(payload["action"], "verify") + self.assertEqual(payload["code"], "warrant_required") + self.assertNotIn("valid", payload) def test_phase_15_mcp_returns_structured_redacted_content(self) -> None: server = MCPServer(