Skip to content

Fail plan/model/parse on parse errors and surface their diagnostics - #81

Merged
henrygarner merged 1 commit into
mainfrom
fix/single-file-parse-diagnostics
Aug 6, 2026
Merged

Fail plan/model/parse on parse errors and surface their diagnostics#81
henrygarner merged 1 commit into
mainfrom
fix/single-file-parse-diagnostics

Conversation

@henrygarner

Copy link
Copy Markdown
Contributor

Fixes #80.

Problem

allium plan returned {"obligations": [], "version": 3} and exit 0 for a spec that does not parse. A consumer could not tell "valid spec with no obligations" from "unparseable garbage".

Root cause: plan, model and parse share run_single_file, which serialised a view of the parsed module and hard-coded ExitCode::SUCCESS, never touching result.diagnostics. Because the parser is error-tolerant (a failed parse still yields a partial module plus diagnostics rather than failing outright), a broken spec produced a well-formed-but-empty result that read as success. The information existed; the single-file path just never wired it to the output or the exit code.

Fix

Every single-file command now routes through a shared single_file_exit:

  • Exit non-zero on an error-severity diagnostic (issue resolution 1). Missing-file (exit 1) and wrong-arity (exit 2) are unchanged.
  • Merge a diagnostics array into the JSON output, mirroring check (issue resolution 2).
  • Warnings and info do not fail the run, so a valid spec with, say, a missing version header still exits 0. This is the distinction the issue drew: check fails on warnings, plan should not.

Help text for all three commands documents the behaviour (issue resolution 3).

Verification (issue matrix)

input plan exit diagnostics
unterminated entity 1 error surfaced
total garbage 1 error surfaced
malformed field 1 error surfaced
valid spec 0 []
valid, warning only (no version header) 0 warning surfaced
missing file 1 — (stderr, no JSON)

Prevention

  • The exit-code + diagnostics contract lives in one shared helper, so a new single-file command inherits it by construction.
  • cli_smoke regression tests iterate over plan/model/parse, covering the whole family rather than the single reported command.
  • docs/project/specs/allium-single-file-tool-behaviour.allium states the contract in Allium, alongside the existing check/analyse specs.

Version bump follows separately on main after merge.

plan/model/parse shared run_single_file, which serialised a view of the
parsed module and hard-coded exit 0, discarding parse diagnostics. As the
parser is error-tolerant, a spec that does not parse yielded a partial
module and, for plan, an empty obligation set indistinguishable from a
valid spec with none.

Route every single-file command through single_file_exit: exit non-zero on
an error-severity diagnostic and merge a `diagnostics` array (mirroring
check) into the JSON output. Warnings and info do not fail the run, so a
valid spec still exits 0. Add cli_smoke regression tests across the family
and a behaviour spec for the shared contract.

Fixes #80
@henrygarner
henrygarner merged commit ed7ee36 into main Aug 6, 2026
5 of 10 checks passed
@henrygarner
henrygarner deleted the fix/single-file-parse-diagnostics branch August 6, 2026 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plan exits 0 on a spec that does not parse, returning an empty obligation set indistinguishable from success

1 participant