feat!: harmonise exit codes — usage 3, retrieval 4 - #42
Merged
Merged
Conversation
BREAKING: exit 3 changes meaning. It was retrieval/validation/internal
failure; it is now usage error. Retrieval failure moves to 4.
FAMILY.md v2.2 harmonises 3 = usage error across every family member. A
malformed call is the one non-verdict outcome every tool has, so an agent that
mis-invokes any of them should get the same answer.
charactercheck was the only member where 3 was already spent, which is why
this was a decision rather than a patch. The cheaper option -- keep retrieval
on 3 and put usage errors on 4 -- was rejected: it would have left the family
with two spellings for the outcome an agent hits most often. Harmonising costs
a breaking change in one repo; not harmonising costs every future caller.
0 unchanged no lint, nothing unhandled
1 unchanged lint findings
2 unchanged unhandled content (the honest lane)
3 WAS fetch NOW usage error: malformed call, incl. argparse and the
structured bad_flag guards, which previously exited 2 and so
shared the honest lane's code
4 NEW retrieval/validation/internal failure (was 3)
Named constants carried most of the weight: EXIT_FETCH moved 3 -> 4 and
EXIT_USAGE = 3 was added, so tests referring to errors.EXIT_FETCH followed
automatically. argparse hardcodes 2 in error(), so main() now builds a parser
subclass that exits EXIT_USAGE.
tool.json's exit_codes and command_exit_contracts are regenerated from the
CLI's own SCHEMA rather than hand-edited -- test_schema_documents_* compares
the two surfaces and catches drift.
Boundary held deliberately: a valid call whose subject has unsupported content
is still 2. One test assertion was moved to 3 in error during this change and
reverted; over-applying was the real risk here, not under-applying.
322 tests pass. Version 0.8.0 -- the contract changed, so consumers need to be
able to pin across it.
Closes #18
A malformed ref is a typed errors.py failure, which moved 3 -> 4 with the rest of that lane. The step asserted 3. Deliberate scope note: bad_ref means "the reference is not a supported input shape", which is arguably a malformed CALL and so arguably belongs on 3 with argparse and the flag guards. It stays on 4 here because errors.py is built as ONE typed-failure lane carrying an action field, and splitting that lane by usage-vs-retrieval is a larger classification exercise across ~25 error kinds. Filed separately rather than decided in passing. The defect this PR fixes is closed either way: no usage error shares the honest lane's exit 2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Closes #18. BREAKING: exit 3 changes meaning.
Implements the decision recorded on #18 — harmonise with the family even where it costs a refactor. FAMILY.md v2.2 (chaoz23/srdcheck#89) now states
3= usage error across every member.Why this rather than the cheaper option
charactercheck was the only member where 3 was already spent, which is what made #18 a decision rather than a patch. Putting usage errors on 4 and leaving retrieval on 3 would have been non-breaking here — and would have left the family with two spellings for the outcome an agent hits most often. Harmonising costs one breaking change; not harmonising costs every future caller.
It also fixes the original defect:
bad_flagand argparse errors used to exit 2, the honest lane's code, so an agent that mis-invoked charactercheck was told by SKILL.md to route its own mistake to a human.What made it tractable
Named constants carried most of it —
EXIT_FETCH3→4 plus a newEXIT_USAGE = 3, so tests referencingerrors.EXIT_FETCHfollowed automatically. argparse hardcodes 2 inerror(), somain()now builds a parser subclass.tool.json'sexit_codesandcommand_exit_contractsare regenerated from the CLI's own SCHEMA, not hand-edited —test_schema_documents_*compares the two surfaces and caught my first pass at exactly that drift.The boundary held
A valid call whose subject has unsupported content is still 2. I moved one such assertion to 3 by over-applying and reverted it — over-application was the real risk here, and the suite caught it.
Verified: bad flag → 3 · bad_flag combo → 3 · valid derive with unhandled → 2 · missing file → 4 ·
--schema→ 0. 322 tests pass.Pin bump, required
Bundled. The old gate caps parsed exit codes at 3, so it reports
UNDOCUMENTED_EXIT_CODEagainst this branch — verified. chaoz23/srdcheck#89 raised the cap and dropped this repo's waiver; against that gate charactercheck is PASS withdocumented [0,1,2,3,4].Version 0.8.0 — the contract changed, so consumers need to pin across it.
🤖 Generated with Claude Code