Repository navigation
feat: fallow's exit-code and JSON output contract, gates, hallow schema - #22
Conversation
…ema`
Exit codes (hallow.exit_codes):
- 0 no enforced gate failed; 1 an enforced gate failed; 2 invalid input or
runtime error; 3 resource unavailable (--config file missing)
- check, security and bare `hallow` exit 1 on error-severity findings by
default; previously only with --ci
- --fail-on-issues counts warn-severity findings too; --ci is
--format sarif --fail-on-issues --quiet (explicit --format wins);
--quiet silences stderr status, never output
- --fail-on-stale-baseline enforces the stale-baseline gate
- errors under --format json are {"error": true, message, exit_code, code}
on stdout; otherwise `error: ...` on stderr; no tracebacks for bad
config, unknown formats or modes, missing files
JSON documents (hallow.output.envelope, pydantic models):
- root kind (check, security, health, dupes, baselines-prune),
schema_version, version, elapsed_ms, gate_outcomes{status, enforced,
observed}, next_steps[{id, command, reason}]
- findings carry finding_id, symbol and actions[{type, auto_fixable,
description, comment|config_key|value}] instead of the internal `fix`
- health and dupes JSON were bare dicts/lists; now typed documents
- MCP `analyze` returns the same check document
`hallow schema` prints commands, flags, formats, exit codes, rules,
suppression syntax and MCP tools, built from the live CLI definitions.
`hallow schema --output-schema` prints the JSON Schema of every document;
docs/output-schema.json is committed and a test fails when it is stale.
Also: SARIF, CodeClimate and GitHub annotations reported each cycle twice
(once from its finding, once from results.cycles) and GitHub annotations
each clone group twice; compact output repeated cycles. They now emit
findings only.
…on-envelope # Conflicts: # src/hallow/cli/main.py
Adversarial review — baseline-orchestrator-codex (Codex, gpt-6-astra), round 1 at 97df907Posted verbatim. Citations of the form What the PR claimsThe reviewed head is The body makes these claims, all recorded in evidence/pr/hallow/22/metadata.json:1:
What the diff showsBacked: Default error enforcement, optional warning enforcement, CI defaults, and explicit-format precedence are implemented by Backed: Baseline filtering precedes gate evaluation; stale enforcement is controlled by the new flag. Quiet status handling leaves the result output intact. Baseline warnings deliberately remain visible. (evidence/pr/hallow/22/diff.patch:1636,1640,1655,1663,2630) Partly backed: Error mapping handles HallowError, FileNotFoundError, TOML decoding, and pydantic validation. It does not cover ordinary filesystem errors or CLI parsing errors outside the command body. The unconditional no-traceback/JSON-error promise is false. (evidence/pr/hallow/22/diff.patch:1176,1201,1238) Partly backed: Check/security and dupes have the claimed envelope. Health has no Backed: Finding serialization replaces Not backed as stated: Next steps omit the originating root/config and interpolate a baseline path without quoting. (evidence/pr/hallow/22/diff.patch:2641,2647,2656,2664) Partly backed: Manifest commands/options come from Typer, and rule defaults/MCP names come from live registries. Format values are manually listed, Backed: Schema generation and the committed-file freshness assertion exist. That assertion verifies synchronization with the models, not compliance with the promised envelope. (evidence/pr/hallow/22/diff.patch:2696,2967) Backed: The redundant cycle and clone-group emission loops are removed. The cycle fixture now includes both the finding and auxiliary cycle data; reverting SARIF deduplication would produce four results and fail its three-result assertion. (evidence/pr/hallow/22/diff.patch:2349,2733,2748,2771,2983,2998) Backed as intentional compatibility changes: Default exit status, CI warning enforcement/output, and finding JSON structure change. README documents them. (evidence/pr/hallow/22/diff.patch:18,1575,1644,2427) Verification: The twelve tests exist and generally invoke the CLI on temporary projects. Current staged checks pass lint, self-check, and Python 3.11–3.14 tests. The workflow installs from the checkout and runs coverage, both ruff checks, and self-analysis; the configured coverage floor is 80%. The unpublished-PyPI failure is obsolete for this head. The exact historical count of 165 and the warning-enforced self-analysis result remain author claims: current self-check does not use Not independently established: Equivalence to fallow and historical problem statements are not proved by this diff. The staged change does establish the removed implementation and its replacement, not the external contract’s full requirements. (evidence/pr/hallow/22/metadata.json:1; evidence/pr/hallow/22/diff.patch:1616,1646,2722) Attacks
Earlier reviewsThere are no staged reviews, review comments, or issue comments. There are no recorded change requests to mark closed. (evidence/pr/hallow/22/reviews.json:1; evidence/pr/hallow/22/review-comments.json:1; evidence/pr/hallow/22/issue-comments.json:1) VerdictNOT READY.
The manifest omissions and test gaps also need correction or narrower claims; current passing CI does not test away these counterexamples. (evidence/pr/hallow/22/diff.patch:2208,2879,2945; evidence/pr/hallow/22/checks.txt:3) I could not write review.md with the available read-only tools; I had no execution facility, external fallow sources, detailed CI logs, or rebuilt post-merge #23 tree to verify the remaining points. |
…O errors Review findings on #22: - Error contract: OSError (an --output that is a directory, permission denied) escaped as a traceback. It is now the JSON error document, exit 2, code HALLOW_IO_ERROR. A refused or missing baseline in `baselines prune` is an error document, not a prune document. Usage errors caught by the argument parser stay text on stderr with exit 2; the README now says so instead of promising JSON for them. - next_steps: commands are built with shlex.join and carry the run's --root/--config, so a baseline path with a space or `$(...)` is one quoted argument and the command analyses the same project from any directory. - Envelope: all documents share one base model (kind, schema_version, version, elapsed_ms, gate_outcomes, next_steps). health and baselines-prune now carry every field; prune reports a `prune-check` gate (enforced with --check). MCP `analyze` and the JSON formatter report the default findings gate instead of an empty one. - `dupes` applies `# hallow-ignore` suppressions like the full analysis, so a suppressed clone cannot fail `dupes --fail-on-issues`. - `hallow schema` publishes possible values for --format, --mode and --boundaries per command. Tests: next steps executed from another directory with a spaced config and baseline path; warnings-only --ci exits 1; envelope fields on every document kind; I/O error and refused-prune error documents; MCP gate; dupes suppression; manifest values. Seven fail on the previous head.
|
Response to the round-1 review (code changes are in f7ee5b1):
Seven of the new tests fail on the previous head (3e9c5ae). |
f7ee5b1 had cli/manifest.py import FLAG_VALUES from cli/main.py while main.py imports manifest.py, so hallow's own self-check reported a circular-dependencies error (exit 1 under the new exit-code contract). The option value tuples now live in hallow.cli.options, imported by both.
Once security exits 1 on error-severity findings (#22), CliRunner reports SystemExit(1) as result.exception. The test now rejects any exception other than SystemExit and checks the exit code is 0 or 1, alongside the control/suppressed assertions.
Adversarial review — baseline-orchestrator-codex (Codex, gpt-6-astra), round 2 at 590674ePosted verbatim. Citations of the form Round-1 findingsReviewed head:
The remaining unsupported claims are runnable next steps, complete MCP equivalence, universal JSON errors, and a complete drift-proof manifest. Uniform successful-document fields and default MCP gates are now supported by code. (evidence/pr/hallow/22/diff.patch:1729–1737,2282–2287,2446,2572–2648,2801–2827) What the PR claimsThe following body claims remain in the staged metadata. (evidence/pr/hallow/22/metadata.json:1)
The response additionally claims that all four blockers are fixed, commands work after changing directory, manifest values and test gaps are corrected, seven new tests fail on the previous head, and #23 is being rebuilt. The revised README strengthens the command promise to “from any directory” and narrows JSON errors to command execution rather than parser errors. (evidence/pr/hallow/22/issue-comments.json:1; evidence/pr/hallow/22/round2-delta.patch:10–14) What the diff showsBacked by the round-2 changes: ordinary I/O error translation; shared successful-document fields; prune gate reporting; default MCP/formatter findings gates; dupes source suppression before gating; shared format/mode values; warning-only CI and per-kind envelope tests. (evidence/pr/hallow/22/round2-delta.patch:233–236,329–337,363–388,450–461,494–557,619–637,717–781) Partly backed: runnable commands. POSIX shell quoting and explicit argument propagation are implemented; absolute execution context and MCP context are not. The “from any directory” claim exceeds the implementation. (evidence/pr/hallow/22/round2-delta.patch:257–263,471–472,565–598) Partly backed: the error and manifest claims. README now acknowledges parser text errors, but the PR body and exported exit-code description remain broader. Allowed values are supplied, but effective format defaults and a guarantee against drift are not. (evidence/pr/hallow/22/round2-delta.patch:13–14,413–432,450–461; evidence/pr/hallow/22/diff.patch:2282–2287,2348–2358,2414–2415; evidence/pr/hallow/22/metadata.json:1) Backed and unchanged from round 1: exit/CI enforcement, quiet handling, opt-in stale gating, finding actions, schema generation/freshness and formatter deduplication. These are retained whole-PR findings, not new round-2 changes. (evidence/pr/hallow/22/diff.patch:1606–1609,1669–1698,2711–2750,2836–2853,2884–2938,3104–3109) Verification backed at this head: lint, self-check and Python 3.11–3.14 tests pass. The workflow installs from the checkout and runs pytest coverage, ruff check and ruff format. The former unpublished-PyPI failure does not apply. Historical test counts, the previous-head mutation result and external fallow equivalence remain unverified. The body’s one-commit description is stale; head.json lists three own commits. (evidence/pr/hallow/22/checks.txt:3–8; evidence/pr/hallow/22/context/ci.yml:22–33,44–49,60–63; evidence/pr/hallow/22/head.json:1; evidence/pr/hallow/22/metadata.json:1; evidence/pr/hallow/22/issue-comments.json:1) Attacks
Earlier reviewsFormal reviews and inline review comments are empty. Issue comments contain the round-1 review and the author’s response. The response closes the ordinary I/O, shared-envelope/gate and dupes-suppression blockers. Generated-command context remains partly closed; manifest/default and documentation claims still need correction. The claimed seven-test previous-head result is not independently demonstrated by staged execution logs. (evidence/pr/hallow/22/reviews.json:1; evidence/pr/hallow/22/review-comments.json:1; evidence/pr/hallow/22/issue-comments.json:1; evidence/pr/hallow/22/diff.patch:1217–1220,1734–1737,1913–1916,2210–2214,2282–2287,2446,2753–2767) VerdictNOT READY at The remaining blocker is generated next-step context. Relative and implicit CLI roots/configs are not anchored to the originating run, and MCP next steps receive no context. The newly added health recommendation uses the same defective context builder. Preserve the effective execution context and add behavioral assertions for relative paths, omitted roots and MCP recommendations after changing directory. (evidence/pr/hallow/22/diff.patch:1729–1737,1825–1828,2446,2753–2786,3119–3139) Also correct the remaining universal-error and drift-proof/default-manifest claims, and preserve the stack integration condition. Passing CI does not settle these counterexamples. (evidence/pr/hallow/22/diff.patch:2282–2287,2348–2358,2386–2387,2414–2415; evidence/pr/hallow/22/metadata.json:1; evidence/pr/hallow/22/head.json:1; evidence/pr/hallow/22/checks.txt:3–8) I could not write |
Round-2 review: next-step commands repeated --root/--config as typed, so `check --root ./project -c ./custom.toml` produced commands that address other paths when run elsewhere; a run without --root emitted no root; MCP emitted no context. The command now carries the run's effective project root (also when it was found by walking up from the cwd) and config as absolute paths, the prune suggestion's baseline path is absolute, and MCP `analyze` passes its resolved root. Tests: relative --root/--config and an implicit root, each command run from another directory and shown to analyse the original project; MCP next step carries the absolute root. Both fail on the previous head.
|
Response to the round-2 review (code changes are in dcda5be):
|
Audit's next step now uses the effective-root context from #22 (absolute --root); test asserts the suggested audit command targets the repo.
Adversarial review — baseline-orchestrator-codex (Codex, gpt-6-astra), round 3 at d1fea85Posted verbatim. Citations of the form Round-2 findingsReviewed head:
What the PR claimsThe current body makes the following claims. Each is recorded in evidence/pr/hallow/22/metadata.json:1.
The round-2 response additionally claims absolute implicit/relative CLI context, absolute baseline paths, resolved MCP context, two regression tests that fail on the previous head, corrected error/manifest wording and a downstream #23 rebuild. (evidence/pr/hallow/22/issue-comments.json:1) What the diff showsBacked by the round-3 delta: effective absolute CLI roots; absolute explicit config paths; absolute prune baseline paths; MCP configuration reuse and root propagation. The existing command builder still quotes arguments with Backed and retained: exit/gate enforcement, CI format precedence, quiet status handling, opt-in stale gating, ordinary operational-error translation, successful-document fields, action serialization, schema generation/freshness and formatter deduplication. These are whole-PR confirmations used to assess prior findings, not new round-3 changes. (evidence/pr/hallow/22/diff.patch:1200–1244,1276–1282,1606–1609,1669–1698,2591–2649,2677–2741,2857–2874,2894–2959,3125–3129) Partly backed: the manifest description. Live definitions supply command metadata and declared defaults; shared constants supply format and mode values. Effective format defaults are represented in help text rather than a structured default field. Boundary validation does not use the claimed shared tuple. These are the DOC qualifications described above. (evidence/pr/hallow/22/diff.patch:2272–2321,2379–2390; evidence/pr/hallow/22/context/head-files/src__hallow__cli__main.py.01.txt:71–84) Partly backed: complete MCP equivalence and verification scope. MCP uses the same check model and gate computation but still reports zero elapsed time. The new CLI test demonstrates a preview in the original project; the new MCP test demonstrates the emitted root. Neither test file proves the author actually ran a previous-head comparison. (evidence/pr/hallow/22/diff.patch:2457–2464,2829–2848; evidence/pr/hallow/22/round3-delta.patch:115–144) Backed by current CI evidence: lint, self-check and tests on Python 3.11–3.14 pass at the staged head. The workflow installs the checkout, runs pytest coverage and both ruff checks; coverage configuration sets an 80% floor. The former unpublished-PyPI failure does not apply. The self-check does not enforce warnings. (evidence/pr/hallow/22/checks.txt:3–8; evidence/pr/hallow/22/context/ci.yml:22–33,44–49,60–63; evidence/pr/hallow/22/context/pyproject.toml:70–72) Not established by this diff: exact fallow equivalence, historical local results and a verified downstream #23 tree. The one-commit and twelve-test descriptions are stale DOC items. (evidence/pr/hallow/22/metadata.json:1; evidence/pr/hallow/22/head.json:1; evidence/pr/hallow/22/issue-comments.json:1; evidence/pr/hallow/22/diff.patch:3140–3259) Attacks
Earlier reviewsFormal reviews and inline review comments are empty. Issue comments contain the earlier reviews and author responses. The latest response’s central repair claim is supported by the new context code. Its previous-head test result and downstream rebuild remain author statements. Its assertion that the new test executes each recommendation exceeds the actual first-recommendation test. The individual prior dispositions are listed under Round-2 findings. (evidence/pr/hallow/22/reviews.json:1; evidence/pr/hallow/22/review-comments.json:1; evidence/pr/hallow/22/issue-comments.json:1; evidence/pr/hallow/22/round3-delta.patch:18–101,129–144) VerdictREADY at BLOCKER
LIMITATION
DOC
These DOC items are nonblocking under the supplied rule: the corrected body no longer promises universal parser JSON or a drift-proof manifest, and the remaining inaccuracies do not establish false ordinary analysis behavior. (evidence/BRIEF.md:174; evidence/pr/hallow/22/metadata.json:1) I could not write |
Round-3 review DOC: the exported exit-code description (hallow schema) still said every exit-2 error is a JSON document under --format json. It now matches the README: run-time errors are JSON; argument-parser usage errors are text on stderr.
|
Round 3 READY. Both DOC items are addressed. The exit-code-2 text that |
Stacked on #21 (and through it on #11–#20). The base branch is
feat/finding-ids-and-baselines, so this diff is one commit (a54a6fb). Merge #21 first, with a merge commit; GitHub then retargets this PR tomain.Problem
hallow checkexited 0 on error-severity findings unless--ciwas given;--formatproduced a Python traceback, with exit 1 (the same code as "findings");--configfile was a traceback too.--format jsonwrote whatevermodel_dumpproduced:kindor version field;fixobject instead of the actions a script can take;healthanddupesJSON were bare lists and dicts with no shared shape.circular-dependenciesfinding and again fromresults.cycles. GitHub annotations also repeated every clone group, and compact output repeated cycles.Change
Ports fallow's exit-code contract (
crates/cli/src/exit_codes.rs,schema.rs) and JSON output (docs/output-schema.json).Exit codes (
hallow.exit_codes)--configfile does not exist)check,securityand barehallowexit 1 on error-severity findings without any flag.--fail-on-issuescounts warn-severity findings too.--cimeans--format sarif --fail-on-issues --quiet; an explicit--formatwins.--quietsilences status messages on stderr, never the output.--fail-on-stale-baselineturns on enforcement of the stale-baseline gate.--format json, an error raised while a command runs (bad config, unwritable--output, unknown format, refused baseline) is a document on stdout:{"error": true, "message", "exit_code", "code"}. Otherwise it printserror: …on stderr. Command-line usage errors caught by the argument parser (unknown option, invalid--boundaries, options before the command) stay text on stderr, with exit 2.JSON documents (
hallow.output.envelope, pydantic models)kind(check,security,health,dupesorbaselines-prune),schema_version,version,elapsed_ms,gate_outcomesandnext_steps.gate_outcomes:{status: pass|warn|fail|skipped, enforced, observed}. The run exits 1 when an enforced gate has statusfail.next_steps: a list of{id, command, reason}. Each is a read-only, shell-quoted command carrying the run's effective project root and config as absolute paths, so it runs as written from any directory.finding_id,symbolandactions[]({type, auto_fixable, description, comment | config_key | value}) replace the internalfixobject.analyzetool returns the samecheckdocument.hallow schema--formatdefaults tohuman), the effective default is given in the flag's help text, not indefault. The allowed values for--formatand--modecome fromhallow.cli.options, which the commands validate against. The--boundariesvalues listed there are a separate tuple, checked against the preset table byget_preset; tests pin the manifest's--formatand--modevalues, and nothing checks the--boundariestuple against the preset table.hallow schema --output-schemaprints the JSON Schema of every document. It is committed asdocs/output-schema.json, and a test fails when the file is stale.Behaviour changes
--ci.hallow checkwith no flags now exits 1 on error-severity findings.--cifails on warnings. It now also fails on warn-severity findings, and its default output is SARIF. Both the bundled workflow (hallow check --format github --ci) and the pre-commit hook (hallow check --ci) will fail on warnings.checkJSON is now a document: findings sit underfindingswithactions, and counts undersummary. The top-levelfix/total_issueslayout is gone.Verification
tests/test_cli_contract.py(12 tests) covers:--fail-on-issues;actionsandnext_steps;--ciproducing SARIF, with an explicit format winning;--quietsilencing status but not output;healthanddupesdocuments;--fail-on-issues(warnings only).