fix: make JSON capture preflight side-effect free - #369
codeforester wants to merge 2 commits into
Conversation
| finally: | ||
| if isinstance(output_capture, _DeferredJsonCapture): | ||
| if output_capture.pending: | ||
| output_capture.resolve_json_output(state.json_output) |
There was a problem hiding this comment.
Correctness (reproduced): when a Click parse/validation error occurs before the leaf command's wrapper runs, the deferred JSON-capture fallback uses the stale preflight guess instead of the value Click's real parse already computed, silently downgrading a JSON-configured invocation to plain-text output. Reproduced: a subcommand with a required argument, invoked without it and without an explicit --json flag, emits a plain-text usage error instead of the base-cli.error JSON envelope main correctly produces for the same scenario.
| parent_context = context | ||
| current_command = next_command | ||
| current_args = remaining[1:] | ||
| info_name = command_name |
There was a problem hiding this comment.
Correctness (reproduced): the subcommand-walk in _click_lifecycle_value sets the child Click Context's info_name to the literal, unnormalized command-line token instead of the name Click's own resolve_command would use after token_normalize_func. With a group using token_normalize_func=str.lower and a default_map keyed by canonical name, invoking with a differently-cased token makes the preflight miss the default_map entry and return a firm-but-wrong False, causing output_capture to be set to None (no capture) while the real parse later resolves json=True — real stdout leaks to the terminal outside the JSON envelope.
| args = list(sys.argv[1:] if argv is None else argv) | ||
| command = app.click_command | ||
| click = dialect_for_command(command) | ||
| preliminary_json = _json_requested( |
There was a problem hiding this comment.
Efficiency: run_app now unconditionally runs the full Click-parser-based preflight (_click_lifecycle_value, which builds a click.Context and calls get_params/make_parser().parse_args() per nested command level) twice per invocation with identical args/command/default_map — the two calls always produce the same result, so the first pass is pure waste. main only performed this resolution once, often skipped via a cheap fast path.
Summary
--positional data.Validation
Fixes #338
Fixes #339