Unify CLI error handling on CliError and the three-line format#134
Merged
Conversation
main() catches CliError centrally and renders it via format_error, so
every command shares one failure surface. Domain validation errors in
session, memory, pilot, doctor, skills, and context now raise CliError
with a recovery hint instead of bare ValueError/RuntimeError; the seven
per-handler 'Error: {exc}' catch blocks in cli.py are gone, and
run_pilot no longer launders normalize_tools' CliError (with its wiki
link) into a plain ValueError.
CliError also drops frozen=True: contextlib's re-raise assigns
__traceback__, which a frozen dataclass rejects with
FrozenInstanceError.
Exit codes are unchanged; team.py and knowledge.py keep their existing
warning-based results (they already handle their own raises at the
public-function boundary).
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.
Summary
Completes the error-handling unification flagged in the codebase assessment (and designed but under-adopted since issue #92):
CliError+ the three-lineerror / next / seeformat is now the single failure surface for the CLI.main()catchesCliErrorcentrally and rendersformat_error(...)to stderr, returningexc.exit_code. The special-casedSystemExitdance around_normalize_args_tools_in_placeis gone —normalize_tools'CliErrorjust propagates.CliErrorwith recovery hints:session(git missing, not a repo, no active session, no start_commit, git command failures),memory(forbidden/restricted/unknown classes, secret-looking content, promote errors),pilot/doctor(unknown persona),skills(empty name),context(_validate_choice). Previously these were bareValueError/RuntimeErrorprinted asError: ...by 7 copy-pasted handler catches — or, forskills/context, raw tracebacks.run_pilotno longer laundersCliErrorintoValueError— the unknown-tool error now keeps itssee:wiki link that the conversion was discarding.select-modelmissing-prompt now uses the same shape (exit code still 2).CliErrordropsfrozen=True: Python's re-raise machinery (contextlib.__exit__) assigns__traceback__, which a frozen dataclass rejects withFrozenInstanceError. This was a latent landmine — anyCliErrorcrossing a@contextmanagercrashed.team.pyandknowledge.pyalready catch their own raises at the public-function boundary and return warning-based results — converting their internal raises would have broken those catches.Exit codes are unchanged throughout. Tests asserting
RuntimeError/ValueErroron the converted paths now assertCliError(same match strings); CHANGELOG documents the library-facing exception-type change under Unreleased.Example (before → after)
Verification
uv run pytest -q: 638 passed in 3.12s.uv run ruff check src tests: clean.🤖 Generated with Claude Code