Repository navigation
fix: stop reporting success for unavailable CLI operations - #26
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness/contract issues to address (notably deterministic UTF-8 handling in the new tampering test, plus traceability status alignment with the repo’s “verified == delivered” rule).
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR hardens the answerable CLI to “fail closed” by returning non-zero exit codes and error payloads for reserved/unimplemented operations and for warrant verify misuse, preventing false “success” signals.
Changes:
- Make reserved/unavailable CLI commands/actions return exit 5 with
command_unavailable(and human-mode errors on stderr). - Require
--warrantforwarrant verify, returning exit 2 with structured error codes for missing/unreadable inputs, while preserving exit 0/3 for valid/invalid warrants. - Add subprocess-based CLI acceptance tests and update traceability/README/ADR documentation accordingly.
File summaries
| File | Description |
|---|---|
| tests/test_interfaces.py | Updates the JSON/exit-code contract test for warrant verify with no input. |
| tests/test_cli_acceptance.py | Adds subprocess acceptance coverage for unavailable commands and warrant verification behaviors. |
| src/answerable/cli.py | Implements fail-closed behavior, new exit codes, structured errors, and corrected warrant human output. |
| requirements/traceability.yaml | Adds the new acceptance test to FR-CLI-001 traceability. |
| README.md | Documents reserved command boundaries, exit codes, and warrant verification semantics. |
| docs/adr/0004-cli-fail-closed.md | Records the decision and compatibility notes for failing closed on CLI success paths. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+97
to
+99
| record = json.loads(path.read_text()) | ||
| record["canonical_json"] += " " | ||
| path.write_text(json.dumps(record)) |
| 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 |
| EXIT_INVALID_WARRANT = 3 | ||
| EXIT_BENCHMARK_FAILED = 4 | ||
| EXIT_UNAVAILABLE = 5 | ||
| EXIT_USAGE = 2 |
Jairogelpi
added a commit
that referenced
this pull request
Sep 18, 2026
Rebased onto #26 (e8ecc94), which closed frame/plan/execute/inspect and source add/test to exit 5 / command_unavailable, and made warrant verify require --warrant. This adds the other half: warrant show and warrant export were left unavailable there even though the real logic (load_warrant, WarrantIssuer.export) already existed, so this wires them up for real instead of stubbing them out too. README and the ADR are updated to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184ULwpMiFbdXnY51apP2xF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Eight reserved CLI operations returned
status: okand exit 0 without performing work.warrant verifywithout an input also succeeded, and a tampered warrant printedokin human mode despite its failing exit code.Changes
command_unavailable; help/README state the boundary.Validation
23 new acceptance cases reproduced the missing behavior before the fix. The focused suite now passes 26 tests. All six CI matrix jobs passed (Linux Python 3.11/3.12 and Windows Python 3.12), including full quality, clean-wheel install, real MCP smoke, external evaluation, EMT, schemas and traceability. Full suite: 253 tests; Linux 3.12 branch-inclusive coverage: 95.89%. CodeQL passed. No thresholds changed.
CI: https://github.com/Jairogelpi/answerable_data/actions/runs/35315554564
Scope
This closes false success paths; it does not implement the reserved commands, a hosted web app, or independent/real-agent validation.