From 4aa7eaae910877c720a6da4f4984d2339b7d6fec Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 21:37:06 +0000 Subject: [PATCH 1/2] ENG-11018: make reflex_cli usable by an agent (--json, non-interactive, --follow) An agent delegating to `reflex cloud` had three ways to get stuck: it got human-formatted tables to regex, a prompt could block it forever, and `apps logs` followed by default and never returned. Every command in `reflex cloud` now takes `--json`. The document is written straight to stdout rather than through the console, and passing the flag reserves stdout for it: log records, tables, rules, spinners and progress bars render to stderr for the duration, so a warning from a helper deep in the call stack cannot land in the middle of the output. The reservation is also taken from argv in the group callback, because that callback speaks (the reflex-version warning) before click has parsed the subcommand's options. `--interactive` is now one shared option defaulting to whether stdout is a terminal, instead of thirty copies defaulting to True. A pipe, a CI job or an agent gets the non-interactive path it would have had to ask for; a person at a terminal sees no change. `apps logs --follow` defaults to off. Following means prompting between pages, and a prompt nobody answers is a command that never exits, so it is opt-in and is refused outright in JSON mode, where paging belongs to the caller. Commands that only logged their outcome now report it: a refusal comes back as `"deleted": false` with the server's message rather than as a success, and logs that could not be read stay distinguishable from an app with none. `generate_config` returns the path it wrote, so `config` no longer claims to have generated a file it declined to overwrite. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UqTVZN9bsCxt7hz5U3qJok --- .../news/+eng-11018-reserve-stdout.feature.md | 1 + .../src/reflex_base/utils/console.py | 26 +- .../reflex-base/src/reflex_base/utils/log.py | 44 +- .../+eng-11018-agent-friendly-cli.feature.md | 1 + .../src/reflex_cli/utils/hosting.py | 16 +- .../src/reflex_cli/utils/output.py | 138 ++++++ .../src/reflex_cli/v2/apps.py | 283 ++++++----- .../src/reflex_cli/v2/deployments.py | 6 + .../src/reflex_cli/v2/gcp.py | 32 +- .../src/reflex_cli/v2/project.py | 128 ++--- .../src/reflex_cli/v2/providers.py | 38 +- .../src/reflex_cli/v2/scan.py | 20 +- .../src/reflex_cli/v2/secrets.py | 55 +-- .../src/reflex_cli/v2/vmtypes_regions.py | 56 +-- reflex/reflex.py | 8 +- tests/units/reflex_base/utils/test_log.py | 49 ++ tests/units/reflex_cli/utils/test_output.py | 193 ++++++++ tests/units/reflex_cli/v2/test_apps.py | 443 +++++++++++++++++- tests/units/reflex_cli/v2/test_gcp.py | 100 +++- tests/units/reflex_cli/v2/test_project.py | 115 +++-- tests/units/reflex_cli/v2/test_scan.py | 4 +- tests/units/reflex_cli/v2/test_secrets.py | 106 ++++- .../reflex_cli/v2/test_vmtypes_regions.py | 72 ++- 23 files changed, 1513 insertions(+), 421 deletions(-) create mode 100644 packages/reflex-base/news/+eng-11018-reserve-stdout.feature.md create mode 100644 packages/reflex-hosting-cli/news/+eng-11018-agent-friendly-cli.feature.md create mode 100644 packages/reflex-hosting-cli/src/reflex_cli/utils/output.py create mode 100644 tests/units/reflex_cli/utils/test_output.py diff --git a/packages/reflex-base/news/+eng-11018-reserve-stdout.feature.md b/packages/reflex-base/news/+eng-11018-reserve-stdout.feature.md new file mode 100644 index 00000000000..996f22c8e5e --- /dev/null +++ b/packages/reflex-base/news/+eng-11018-reserve-stdout.feature.md @@ -0,0 +1 @@ +`reflex_base.utils.log.reserve_stdout()` reserves stdout for a machine-readable document, rendering log records, tables, rules, spinners and progress bars to stderr for as long as it is set. diff --git a/packages/reflex-base/src/reflex_base/utils/console.py b/packages/reflex-base/src/reflex_base/utils/console.py index b8a5985322f..c68474bf8b9 100644 --- a/packages/reflex-base/src/reflex_base/utils/console.py +++ b/packages/reflex-base/src/reflex_base/utils/console.py @@ -34,6 +34,17 @@ _console = Console(highlight=False) _console_stderr = Console(stderr=True, highlight=False) + +def _human_console() -> Console: + """Get the console human-readable output renders to. + + Returns: + The stderr console while stdout is reserved for a machine-readable + document, the stdout one otherwise. + """ + return _console_stderr if _log.is_stdout_reserved() else _console + + # Deprecated features who's warning has been printed. _EMITTED_DEPRECATION_WARNINGS = set() @@ -108,7 +119,7 @@ def print(msg: str, *, dedupe: bool = False, level: str = "info", **kwargs): if msg in _EMITTED_PRINTS: return _EMITTED_PRINTS.add(msg) - _console.print(msg, **kwargs) + _human_console().print(msg, **kwargs) def _print_stderr(msg: str, *, dedupe: bool = False, level: str = "error", **kwargs): @@ -249,7 +260,7 @@ def log(msg: str, *, dedupe: bool = False, **kwargs): if _log.is_json_mode(): _log.emit_json_print(msg) else: - _console.log(msg, **kwargs) + _human_console().log(msg, **kwargs) if should_use_log_file_console(): print_to_log_file(msg, **kwargs) @@ -263,7 +274,7 @@ def rule(title: str, **kwargs): """ if _log.is_json_mode(): return - _console.rule(title, **kwargs) + _human_console().rule(title, **kwargs) def warn(msg: str, *, dedupe: bool = False, **kwargs): @@ -493,7 +504,7 @@ def print_table( for row in tabular_data: table.add_row(*row) - _console.print(table) + _human_console().print(table) def progress(): @@ -506,7 +517,10 @@ def progress(): *Progress.get_default_columns()[:-1], MofNCompleteColumn(), TimeElapsedColumn(), - disable=_log.is_json_mode(), + # A bar is decoration, and it redraws in place: there is nowhere to + # put it in a machine-readable stream, and nothing to draw it over + # once stdout belongs to a document. + disable=_log.is_json_mode() or _log.is_stdout_reserved(), ) @@ -522,7 +536,7 @@ def status(*args, **kwargs): """ if _log.is_json_mode(): return _log._quiet_console.status(*args, **kwargs) - return _console.status(*args, **kwargs) + return _human_console().status(*args, **kwargs) @contextlib.contextmanager diff --git a/packages/reflex-base/src/reflex_base/utils/log.py b/packages/reflex-base/src/reflex_base/utils/log.py index c7c17e10bf7..f9987d2bd10 100644 --- a/packages/reflex-base/src/reflex_base/utils/log.py +++ b/packages/reflex-base/src/reflex_base/utils/log.py @@ -77,6 +77,9 @@ # Console that renders nowhere, backing interactive rich features in JSON mode. _quiet_console = Console(quiet=True) +# Whether stdout carries a machine-readable document rather than human output. +_stdout_reserved = False + # The current log level. _log_level = LogLevel.INFO @@ -197,7 +200,9 @@ def emit(self, record: logging.LogRecord): """ try: style, prefix = _style_for(record) - console = _console_stderr if record.levelno >= logging.ERROR else _console + console = ( + _console_stderr if record.levelno >= logging.ERROR else human_console() + ) # Records may carry a rich Progress to print through, so the # message lands above an active progress bar. progress = getattr(record, "progress", None) @@ -241,7 +246,7 @@ def _write_json(payload: dict, *, stderr: bool): payload: The record fields. stderr: Whether the record targets stderr. """ - stream = sys.stderr if stderr else sys.stdout + stream = sys.stderr if stderr or _stdout_reserved else sys.stdout stream.write(json.dumps(payload, default=str) + "\n") stream.flush() @@ -402,6 +407,38 @@ def is_json_mode() -> bool: return environment.REFLEX_LOG_JSON.get() +def reserve_stdout(reserved: bool = True): + """Reserve stdout for a machine-readable document. + + A command that writes structured output (``--json``) owns stdout for the + duration, so every human-readable message -- log records, tables, spinners + -- renders to stderr instead and cannot land in the middle of the document. + + Args: + reserved: Whether stdout carries data rather than human output. + """ + global _stdout_reserved + _stdout_reserved = reserved + + +def is_stdout_reserved() -> bool: + """Check whether stdout is reserved for a machine-readable document. + + Returns: + True if human-readable output has to go to stderr. + """ + return _stdout_reserved + + +def human_console() -> Console: + """Get the console human-readable output renders to. + + Returns: + The stderr console while stdout is reserved, the stdout one otherwise. + """ + return _console_stderr if _stdout_reserved else _console + + def set_json_mode(enabled: bool): """Enable or disable machine-readable JSON log output. @@ -563,7 +600,8 @@ def ensure_configured(): def _reset(): """Detach the sinks and restore propagation (test teardown helper).""" - global _configured + global _configured, _stdout_reserved + _stdout_reserved = False for handler in (_console_handler(), _json_handler(), _active_file_handler): if handler is not None: _REFLEX_LOGGER.removeHandler(handler) diff --git a/packages/reflex-hosting-cli/news/+eng-11018-agent-friendly-cli.feature.md b/packages/reflex-hosting-cli/news/+eng-11018-agent-friendly-cli.feature.md new file mode 100644 index 00000000000..84d746eac2d --- /dev/null +++ b/packages/reflex-hosting-cli/news/+eng-11018-agent-friendly-cli.feature.md @@ -0,0 +1 @@ +Every `reflex cloud` command now takes `--json`, writing one JSON document to stdout while human-readable messages move to stderr, so the output is parseable without reading a Rich table. `--interactive` defaults to whether stdout is a terminal, so a pipe, a CI job or an agent is never left waiting at a prompt instead of exiting. `reflex cloud apps logs --follow` now defaults to off: following prompts between pages and never returns on its own, so it is opt-in. diff --git a/packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py b/packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py index 49ba13f263c..f882d584c39 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py @@ -2762,13 +2762,16 @@ def read_config( return Config.from_yaml_or_toml_or_none() -def generate_config(interactive: bool = True, token: str | None = None): +def generate_config(interactive: bool = True, token: str | None = None) -> Path | None: """Generate the config file with app-based prefilling. Args: interactive: Whether to use interactive mode for authentication and app selection. token: An existing authentication token to use instead of interactive auth. + Returns: + The path of the config file written, or None if none was. + Raises: click.exceptions.Exit: If authentication fails or user cancels operation. """ @@ -2776,11 +2779,12 @@ def generate_config(interactive: bool = True, token: str | None = None): import yaml except ImportError: logger.error("Please install PyYAML to use this command: pip install pyyaml") - return + return None - if Path("cloud.yml").exists(): + config_path = Path("cloud.yml") + if config_path.exists(): logger.error("cloud.yml already exists.") - return + return None try: authenticated_client = get_authenticated_client( @@ -2821,13 +2825,13 @@ def generate_config(interactive: bool = True, token: str | None = None): ) default = {"name": current_dir_name} - with Path("cloud.yml").open("w") as config_file: + with config_path.open("w") as config_file: yaml.dump(default, config_file, default_flow_style=False, sort_keys=False) logger.log(log.SUCCESS, "cloud.yml created successfully.") logger.info( "For more configuration options, see: https://reflex.dev/docs/hosting/config-file/" ) - return + return config_path def log_out_on_browser(): diff --git a/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py b/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py new file mode 100644 index 00000000000..b15acd5c423 --- /dev/null +++ b/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py @@ -0,0 +1,138 @@ +"""Machine-readable output, and the shared options that turn it on. + +An agent driving the cloud CLI needs two things a person at a terminal does +not: output it can parse without regexing a Rich table, and the certainty that +nothing will stop and wait for a keystroke. ``--json`` answers the first and +reserves stdout for the document while it does; ``--interactive`` answers the +second by defaulting to whether stdout is a terminal. +""" + +from __future__ import annotations + +import json +import sys +from collections.abc import Sequence +from typing import Any + +import click +from reflex_base.utils import log + +# The spellings that ask for JSON on the command line, and the one that +# refuses it. Read straight off argv so the group callback can reserve stdout +# before click has parsed the subcommand's options -- anything it says would +# otherwise land on stdout ahead of the document. +_JSON_FLAGS = frozenset({"--json", "-j"}) +_NO_JSON_FLAG = "--no-json" + + +def stdout_is_tty() -> bool: + """Check whether stdout is attached to a terminal. + + Returns: + True if somebody is plausibly watching, False under a pipe, a CI job + or an agent. + """ + isatty = getattr(sys.stdout, "isatty", None) + if isatty is None: + return False + try: + return bool(isatty()) + except ValueError: + # A closed stream. Nobody is answering a prompt on it either way. + return False + + +def _resolve_interactive( + ctx: click.Context, param: click.Parameter, value: bool | None +) -> bool: + """Resolve an unset ``--interactive`` against the terminal. + + Args: + ctx: The click context. + param: The click parameter. + value: The flag's value, or None when neither spelling was passed. + + Returns: + Whether the command may prompt. + """ + return stdout_is_tty() if value is None else value + + +interactive_option = click.option( + "--interactive/--no-interactive", + "-i/", + "interactive", + default=None, + callback=_resolve_interactive, + help="Whether to prompt for confirmations and choices. Defaults to on when " + "stdout is a terminal and off otherwise, so a pipe, a CI job or an agent is " + "never left waiting at a prompt.", +) + + +def json_requested(argv: Sequence[str] | None = None) -> bool: + """Check whether a command line asks for JSON output. + + Args: + argv: The arguments to inspect; defaults to this process's own. + + Returns: + True if a JSON flag is present and not overridden by ``--no-json``. + """ + args = set(sys.argv[1:] if argv is None else argv) + return bool(_JSON_FLAGS & args) and _NO_JSON_FLAG not in args + + +def reserve_stdout_for_argv(argv: Sequence[str] | None = None) -> None: + """Reserve stdout up front when the command line asks for JSON. + + Always writes the reservation rather than only setting it, so a long-lived + process (tests, an embedded runner) cannot inherit the previous + invocation's answer. + + Args: + argv: The arguments to inspect; defaults to this process's own. + """ + log.reserve_stdout(json_requested(argv)) + + +def _reserve_stdout(ctx: click.Context, param: click.Parameter, value: bool) -> bool: + """Reserve stdout for the document once ``--json`` is parsed. + + Args: + ctx: The click context. + param: The click parameter. + value: Whether JSON output was asked for. + + Returns: + The flag's value, unchanged. + """ + if value: + log.reserve_stdout() + return value + + +json_option = click.option( + "--json/--no-json", + "-j", + "as_json", + is_flag=True, + is_eager=True, + callback=_reserve_stdout, + help="Output the result as a single JSON document on stdout. Human-readable " + "messages go to stderr instead, so stdout stays parseable.", +) + + +def print_json(payload: Any) -> None: + """Write one JSON document to stdout. + + Deliberately not routed through :mod:`reflex_cli.utils.console`: this is the + output the command was asked for, not a message about it, so it goes to + stdout even while the console renders to stderr, and is never wrapped in a + log record. + + Args: + payload: The value to serialize. + """ + click.echo(json.dumps(payload, default=str)) diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py index 369b9a41ca7..3efdc46118a 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/apps.py @@ -2,7 +2,6 @@ from __future__ import annotations -import json import logging from typing import Any @@ -21,6 +20,7 @@ ScaleParamError, ScaleTypeError, ) +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -91,20 +91,8 @@ def _resolve_app_id( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def app_history( app_id: str | None, app_name: str | None, @@ -148,7 +136,7 @@ def app_history( history = hosting.get_app_history(app_id=app_id, client=authenticated_client) if as_json: - console.print(json.dumps(history)) + print_json(history) return if history: headers = list(history[0].keys()) @@ -174,19 +162,15 @@ def app_history( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def app_rollback( deployment_id: str, app_id: str | None, app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Roll an app back to a previous deployment. @@ -216,6 +200,13 @@ def app_rollback( != "y" ): logger.info("Rollback cancelled.") + if as_json: + print_json({ + "app_id": app_id, + "deployment_id": deployment_id, + "rolled_back": False, + "cancelled": True, + }) return result = hosting.rollback_deployment( @@ -224,6 +215,14 @@ def app_rollback( if result: logger.error(result) raise click.exceptions.Exit(1) + if as_json: + print_json({ + "app_id": app_id, + "deployment_id": deployment_id, + "rolled_back": True, + "cancelled": False, + }) + return logger.log(log.SUCCESS, f"Rollback to deployment {deployment_id} started.") console.print( f"Track progress with `reflex cloud apps status {deployment_id} " @@ -250,13 +249,8 @@ def app_rollback( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def app_describe( deployment_id: str, description: str, @@ -264,6 +258,7 @@ def app_describe( app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Set or clear the changelog note on a past deployment. @@ -289,6 +284,13 @@ def app_describe( if result: logger.error(result) raise click.exceptions.Exit(1) + if as_json: + print_json({ + "app_id": app_id, + "deployment_id": deployment_id, + "description": description, + }) + return if description.strip(): logger.log( log.SUCCESS, f"Updated description for deployment {deployment_id}." @@ -305,16 +307,12 @@ def app_describe( @apps_cli.command("build-logs") @click.argument("deployment_id", required=True) @click.option("--token", help="The authentication token.") -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def deployment_build_logs( deployment_id: str, token: str | None, + as_json: bool, interactive: bool, ): """Retrieve the build logs for a specific deployment.""" @@ -327,6 +325,9 @@ def deployment_build_logs( logs = hosting.get_deployment_build_logs( deployment_id=deployment_id, client=authenticated_client ) + if as_json: + print_json({"deployment_id": deployment_id, "logs": logs}) + return console.print(logs) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") @@ -345,18 +346,14 @@ def deployment_build_logs( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def deployment_status( deployment_id: str, watch: bool, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Retrieve the status of a specific deployment.""" @@ -369,15 +366,33 @@ def deployment_status( token=token, interactive=interactive ) if watch: - status = hosting.watch_deployment_status( + succeeded = hosting.watch_deployment_status( deployment_id=deployment_id, client=authenticated_client ) - if status is False: + if as_json: + # Re-read once the watch ends: the watch itself reports + # progress through the log stream and returns only whether it + # got there, which is not a status a caller can act on. + print_json({ + "deployment_id": deployment_id, + "status": hosting.get_deployment_status( + deployment_id=deployment_id, client=authenticated_client + ), + "success": succeeded, + }) + if succeeded is False: raise click.exceptions.Exit(1) else: status = hosting.get_deployment_status( deployment_id=deployment_id, client=authenticated_client ) + if as_json: + print_json({ + "deployment_id": deployment_id, + "status": status, + "success": "failed" not in status, + }) + return logger.error(status) if "failed" in status else console.print(status) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") @@ -394,18 +409,14 @@ def deployment_status( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def stop_app( app_id: str | None, app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Stop a running application.""" @@ -442,10 +453,12 @@ def stop_app( raise click.exceptions.Exit(1) result = hosting.stop_app(app_id=app_id, client=authenticated_client) + failed = bool(result) and "failed" in result + if as_json: + print_json({"app_id": app_id, "stopped": not failed, "message": result}) + return if result: - logger.error(result) if "failed" in result else logger.log( - log.SUCCESS, result - ) + logger.error(result) if failed else logger.log(log.SUCCESS, result) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") raise click.exceptions.Exit(1) from err @@ -461,18 +474,14 @@ def stop_app( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def start_app( app_id: str | None, app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Start a stopped application.""" @@ -508,10 +517,12 @@ def start_app( raise click.exceptions.Exit(1) result = hosting.start_app(app_id=app_id, client=authenticated_client) + failed = bool(result) and "failed" in result + if as_json: + print_json({"app_id": app_id, "started": not failed, "message": result}) + return if result: - logger.error(result) if "failed" in result else logger.log( - log.SUCCESS, result - ) + logger.error(result) if failed else logger.log(log.SUCCESS, result) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") raise click.exceptions.Exit(1) from err @@ -527,18 +538,14 @@ def start_app( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def delete_app( app_id: str | None, app_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Delete an application.""" @@ -582,6 +589,12 @@ def delete_app( ) except GetAppError: logger.warning(f"No application found with ID '{app_id}'") + if as_json: + print_json({ + "app_id": app_id, + "deleted": False, + "message": f"No application found with ID '{app_id}'", + }) return if not app_result: logger.warning(f"App with ID '{app_id}' not found.") @@ -618,9 +631,26 @@ def delete_app( != "y" ): logger.info("Deletion cancelled.") + if as_json: + print_json({ + "app_id": app_id, + "deleted": False, + "cancelled": True, + }) return result = hosting.delete_app(app_id=app_id, client=authenticated_client) + if as_json: + # A refusal comes back as a message rather than as an exception, so + # the document has to read it too: reporting the call as a deletion + # is how a caller ends up believing an app is gone. + failed = result is None or (isinstance(result, str) and "failed" in result) + print_json({ + "app_id": app_id, + "deleted": not failed, + "message": result, + }) + return if result: logger.warning(result) except NotAuthenticatedError as err: @@ -641,17 +671,17 @@ def delete_app( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option @click.option("--cursor", type=str, help="The cursor for pagination.") @click.option("--pretty", type=bool, help="Use pretty printing for logs.") @click.option( - "--follow", type=bool, default=True, help="Asks to continue to query logs." + "--follow", + type=bool, + default=False, + help="After printing a page, prompt to fetch the next one. Off by default: " + "the prompt never returns on its own, so a script or an agent that asked " + "for logs would hang instead of exiting.", ) def app_logs( app_id: str | None, @@ -661,10 +691,11 @@ def app_logs( start: int | None, end: int | None, loglevel: str, + as_json: bool, interactive: bool, cursor: str | None = None, pretty: bool = False, - follow: bool = True, + follow: bool = False, ): """Retrieve logs for a given application.""" import pprint @@ -707,6 +738,11 @@ def app_logs( logger.error("must provide both start and end") raise click.exceptions.Exit(1) + # Following means prompting between pages, which never returns on its + # own, so it needs somebody at the terminal and a stream that is not + # carrying a JSON document. + following = follow and interactive and not as_json + while True: logger.debug(f"fetching logs with cursor: {cursor}") result = hosting.get_app_logs( @@ -719,6 +755,15 @@ def app_logs( ) if not isinstance(result, list): logger.warning("Unable to retrieve logs.") + if as_json: + # Kept apart from an empty page: "we could not read them" + # and "there are none" call for different next steps. + print_json({ + "app_id": app_id, + "entries": [], + "cursor": None, + "error": "Unable to retrieve logs.", + }) return if len(result) == 2 and isinstance(result[1], str): cursor = result[1] @@ -727,13 +772,31 @@ def app_logs( cursor = None if not result: logger.warning("No logs found for the specified criteria.") + if as_json: + print_json({ + "app_id": app_id, + "entries": [], + "cursor": cursor, + "error": None, + }) return result.reverse() + if as_json: + # One page per invocation, with the cursor to ask for the next: + # a document is only a document once it is complete, so paging + # is the caller's loop rather than ours. + print_json({ + "app_id": app_id, + "entries": result, + "cursor": cursor, + "error": None, + }) + return for log in result: if pretty: log = pprint.pformat(log, indent=2) logger.info(log) - if not (interactive and follow): + if not following: return from rich.prompt import Prompt @@ -763,19 +826,8 @@ def app_logs( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in JSON format.", -) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def list_apps( project_id: str | None, project_name: str | None, @@ -821,7 +873,7 @@ def list_apps( raise click.exceptions.Exit(1) from ex if as_json: - console.print(json.dumps(deployments)) + print_json(deployments) return if deployments: headers = list(deployments[0].keys()) @@ -846,13 +898,8 @@ def list_apps( help="The log level to use.", ) @click.option("--scale-type", help="The type of scaling.") -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def scale_app( app_id: str | None, app_name: str | None, @@ -861,6 +908,7 @@ def scale_app( token: str | None, loglevel: str, scale_type: str | None, + as_json: bool, interactive: bool, ): """Scale an application by changing the VM type or adding/removing regions.""" @@ -921,6 +969,15 @@ def scale_app( hosting.scale_app( app_id=app_id, scale_params=scale_params, client=authenticated_client ) + if as_json: + print_json({ + "app_id": app_id, + "scaled": True, + "vmtype": scale_params.vm_type, + "regions": list(scale_params.regions), + "scale_type": scale_params.type, + }) + return logger.log(log.SUCCESS, "Successfully scaled the app.") except NotAuthenticatedError as err: @@ -946,20 +1003,8 @@ def scale_app( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in JSON format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def inspect_app( app_id: str | None, token: str | None, @@ -995,7 +1040,7 @@ def inspect_app( app_info = hosting.get_app(app_id=app_id, client=authenticated_client) if as_json: - console.print(json.dumps(app_info)) + print_json(app_info) return if app_info: diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py index acc11f15462..e876fd39904 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py @@ -12,6 +12,7 @@ from packaging import version from reflex_cli import constants +from reflex_cli.utils.output import reserve_stdout_for_argv from reflex_cli.v2.apps import apps_cli from reflex_cli.v2.gcp import deploy_command as gcp_deploy_command from reflex_cli.v2.project import project_cli @@ -35,6 +36,11 @@ def hosting_cli(ctx: click.Context) -> None: It provides commands for managing apps, projects, secrets, and VM types/regions. """ + # Before anything below can speak: this callback runs ahead of the + # subcommand's own option parsing, so its --json is not known yet and a + # warning from here would land on stdout in front of the document. + reserve_stdout_for_argv() + if _reflex_version is None: ctx.fail("Reflex is not installed. Install it with `pip install reflex`.") if _reflex_version < constants.ReflexHostingCli.MINIMUM_REFLEX_VERSION: diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py index a0c87ad5025..7fad9e625fc 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py @@ -38,6 +38,7 @@ from reflex_cli import constants from reflex_cli.utils import console +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -222,12 +223,8 @@ help="The directory containing the Reflex app. Uploaded to Cloud Build as the build context; the source tree itself is not modified.", ) @click.option("--token", help="The Reflex authentication token.") -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to prompt before running the deploy script.", -) +@json_option +@interactive_option @click.option( "--dry-run", is_flag=True, @@ -257,6 +254,7 @@ def deploy_command( envs: tuple[str, ...], source_dir: str, token: str | None, + as_json: bool, interactive: bool, dry_run: bool, loglevel: str, @@ -425,6 +423,16 @@ def deploy_command( console.print(env_vars_yaml) console.print("─" * 60) logger.info("Dry run — nothing staged or executed.") + if as_json: + print_json({ + "dry_run": True, + "source_dir": str(source_path), + "deploy_env": deploy_env, + "cloudbuild_yaml": cloudbuild_yaml, + "dockerfile": dockerfile, + "deploy_script": deploy_script, + "env_vars_yaml": env_vars_yaml, + }) return if interactive: @@ -451,6 +459,16 @@ def deploy_command( cwd=source_path, env_overrides=env_overrides, ) + if as_json: + print_json({ + "dry_run": False, + "deployed": exit_code == 0, + "exit_code": exit_code, + "gcp_project": gcp_project, + "region": region, + "service_name": service_name, + "version": version_value, + }) if exit_code != 0: logger.error(f"Deploy script exited with status {exit_code}.") raise click.exceptions.Exit(exit_code) @@ -791,7 +809,7 @@ def _run_deploy_script( cwd=cwd, env=env, check=False, - stdout=sys.stdout, + stdout=sys.stderr if log.is_stdout_reserved() else sys.stdout, stderr=sys.stderr, ) except OSError as ex: diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/project.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/project.py index cebeedba700..59b17944eda 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/project.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/project.py @@ -9,6 +9,7 @@ from reflex_cli import constants from reflex_cli.utils import console from reflex_cli.utils.exceptions import NotAuthenticatedError +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -27,20 +28,8 @@ def project_cli(): default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def create_project( name: str, token: str | None, @@ -65,7 +54,7 @@ def create_project( raise click.exceptions.Exit(1) from err if as_json: - console.print(json.dumps(project)) + print_json(project) return if project: project = [project] @@ -89,18 +78,14 @@ def create_project( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def invite_user_to_project( role: str, user: str, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Invite a user to a project.""" @@ -121,6 +106,9 @@ def invite_user_to_project( if "failed" in result: logger.error(f"Unable to invite user to project: {result}") raise click.exceptions.Exit(1) + if as_json: + print_json({"role_id": role, "user_id": user, "invited": True}) + return logger.log(log.SUCCESS, "Successfully invited user to project.") @@ -134,17 +122,14 @@ def invite_user_to_project( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def select_project( project_id: str | None, project_name: str | None, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Select a project.""" @@ -184,6 +169,9 @@ def select_project( if "failed" in result: logger.error(result) raise click.exceptions.Exit(1) + if as_json: + print_json({"project_id": project_id, "selected": True, "message": result}) + return logger.log(log.SUCCESS, result) @@ -195,16 +183,12 @@ def select_project( help="The log level to use.", ) @click.option("--token", help="The authentication token.") -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def get_select_project( loglevel: str, token: str | None, + as_json: bool, interactive: bool, ): """Get the currently selected project.""" @@ -220,6 +204,9 @@ def get_select_project( project_details = hosting.get_project( project_id=project, client=authenticated_client ) + if as_json: + print_json({"project_id": project, "name": project_details["name"]}) + return console.print_table( [[project, project_details["name"]]], headers=["Selected Project ID", "Project Name"], @@ -231,6 +218,8 @@ def get_select_project( raise click.exceptions.Exit(1) from None except Exception as e: logger.error(f"Unable to get the currently selected project: {e}") + elif as_json: + print_json({"project_id": None, "name": None}) else: logger.warning( "no selected project. run `reflex cloud project select` to set one." @@ -245,20 +234,8 @@ def get_select_project( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def get_projects( token: str | None, loglevel: str, @@ -276,7 +253,7 @@ def get_projects( ) projects = hosting.get_projects(client=authenticated_client) if as_json: - console.print(json.dumps(projects)) + print_json(projects) return if projects: headers = list(projects[0].keys()) @@ -314,19 +291,8 @@ def get_projects( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def get_project_roles( project_id: str | None, project_name: str | None, @@ -362,7 +328,7 @@ def get_project_roles( ) if as_json: - console.print(json.dumps(roles)) + print_json(roles) return if roles: headers = list(roles[0].keys()) @@ -393,19 +359,8 @@ def get_project_roles( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def get_project_role_permissions( role_id: str, project_id: str | None, @@ -441,7 +396,7 @@ def get_project_role_permissions( ) if as_json: - console.print(json.dumps(permissions)) + print_json(permissions) return if permissions: headers = list(permissions[0].keys()) @@ -474,19 +429,8 @@ def get_project_role_permissions( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@json_option +@interactive_option def get_project_role_users( project_id: str | None, project_name: str | None, @@ -522,7 +466,7 @@ def get_project_role_users( ) if as_json: - console.print(json.dumps(users)) + print_json(users) return if users: headers = list(users[0].keys()) diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/providers.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/providers.py index 5f621a30ac3..62fde3ee319 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/providers.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/providers.py @@ -9,7 +9,6 @@ from __future__ import annotations -import json import logging from typing import Any @@ -19,6 +18,7 @@ from reflex_cli import constants from reflex_cli.utils import console from reflex_cli.utils.exceptions import NotAuthenticatedError +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -201,20 +201,8 @@ def _connection_row( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def providers_status( org_id: str | None, token: str | None, @@ -246,7 +234,7 @@ def providers_status( raise click.exceptions.Exit(1) from ex if as_json: - console.print(json.dumps(status)) + print_json(status) return configured = status.get("configured") @@ -307,20 +295,8 @@ def providers_status( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def providers_list( org_id: str | None, token: str | None, @@ -382,7 +358,7 @@ def providers_list( runtime_service_accounts = None if as_json: - console.print(json.dumps(connections)) + print_json(connections) return if not connections: console.print( diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/scan.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/scan.py index fa4809da2be..84435fb5b6a 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/scan.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/scan.py @@ -3,7 +3,6 @@ from __future__ import annotations import io -import json import logging import os import time @@ -17,6 +16,7 @@ from reflex_cli import constants from reflex_cli.utils import console from reflex_cli.utils.exceptions import NotAuthenticatedError +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -182,20 +182,8 @@ def _print_violations(result: dict[str, Any]) -> None: default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in JSON format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def scan_command( directory: Path, token: str | None, @@ -252,7 +240,7 @@ def scan_command( result = payload.get("result") or {} if as_json: - console.print(json.dumps(result)) + print_json(result) else: _print_violations(result) diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/secrets.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/secrets.py index 79923fd8842..d8870148783 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/secrets.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/secrets.py @@ -10,6 +10,7 @@ from reflex_cli import constants from reflex_cli.utils import console from reflex_cli.utils.exceptions import NotAuthenticatedError +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -28,20 +29,8 @@ def secrets_cli(): default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in JSON format.", -) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def get_secrets( app_id: str | None, token: str | None, @@ -78,7 +67,7 @@ def get_secrets( logger.error(secrets) raise click.exceptions.Exit(1) if as_json: - console.print(secrets) + print_json(secrets) return if secrets: headers = ["Keys"] @@ -115,13 +104,8 @@ def get_secrets( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def update_secrets( app_id: str | None, envfile: str | None, @@ -129,6 +113,7 @@ def update_secrets( reboot: bool, token: str | None, loglevel: str, + as_json: bool, interactive: bool, ): """Update secrets for a given application.""" @@ -177,6 +162,14 @@ def update_secrets( hosting.update_secrets( app_id=app_id, secrets=secrets, reboot=reboot, client=authenticated_client ) + if as_json: + # Names only: a value the caller just sent back to them is a secret + # written into a log or a transcript. + print_json({ + "app_id": app_id, + "updated": sorted(secrets), + "rebooted": reboot, + }) except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") raise click.exceptions.Exit(1) from err @@ -197,19 +190,15 @@ def update_secrets( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def delete_secret( app_id: str | None, key: str, token: str | None, reboot: bool, loglevel: str, + as_json: bool, interactive: bool, ): """Delete a secret for a given application.""" @@ -241,6 +230,14 @@ def delete_secret( if "failed" in result: logger.error(result) raise click.exceptions.Exit(1) + if as_json: + print_json({ + "app_id": app_id, + "key": key, + "deleted": True, + "rebooted": reboot, + }) + return logger.log(log.SUCCESS, "Successfully deleted secret.") except NotAuthenticatedError as err: logger.error("You are not authenticated. Run `reflex login` to authenticate.") diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/vmtypes_regions.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/vmtypes_regions.py index 6e97908ecdc..5b62c9a1e5d 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/vmtypes_regions.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/vmtypes_regions.py @@ -1,6 +1,5 @@ """VMTypes and Regions commands for the Reflex Cloud CLI.""" -import json import logging import click @@ -8,6 +7,7 @@ from reflex_cli import constants from reflex_cli.utils import console +from reflex_cli.utils.output import interactive_option, json_option, print_json logger = logging.getLogger(__name__) @@ -32,16 +32,12 @@ def vm_types_regions_cli(): default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def create_token( name: str, token: str | None, + as_json: bool, interactive: bool, duration: int, loglevel: constants.LogLevel = constants.LogLevel.INFO, @@ -61,6 +57,9 @@ def create_token( token = hosting.create_token( name=name, expiration=duration, client=authenticated_client ) + if as_json: + print_json({"name": name, "token": token, "expires_in_days": duration}) + return logger.log(log.SUCCESS, f"Token: {token}") @@ -72,13 +71,7 @@ def create_token( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) +@json_option def get_vm_types( token: str | None, loglevel: str, @@ -91,7 +84,7 @@ def get_vm_types( vmtypes = hosting.get_vm_types() if as_json: - console.print(json.dumps(vmtypes)) + print_json(vmtypes) return if vmtypes: ordered_vmtpes: list[list[str | float]] = [ @@ -116,13 +109,7 @@ def get_vm_types( default=constants.LogLevel.INFO.value, help="The log level to use.", ) -@click.option( - "--json/--no-json", - "-j", - "as_json", - is_flag=True, - help="Whether to output the result in json format.", -) +@json_option def get_deployment_regions( loglevel: str, as_json: bool, @@ -171,7 +158,7 @@ def get_deployment_regions( list_regions_info = hosting.get_regions() if as_json: - console.print(json.dumps(list_regions_info)) + print_json(list_regions_info) return if list_regions_info: headers = list(list_regions_info[0].keys()) @@ -184,19 +171,22 @@ def get_deployment_regions( @vm_types_regions_cli.command(name="config") @click.option("--token", help="An existing authentication token.") -@click.option( - "--interactive/--no-interactive", - "-i", - is_flag=True, - default=True, - help="Whether to use interactive mode.", -) +@json_option +@interactive_option def generate_cloud_config( token: str | None = None, + as_json: bool = False, interactive: bool = True, ): """Generate a configuration file for the cloud deployment.""" from reflex_cli.utils import hosting - hosting.generate_config(interactive=interactive, token=token) - console.print("Configuration file generated.") + config_path = hosting.generate_config(interactive=interactive, token=token) + if as_json: + print_json({ + "generated": config_path is not None, + "path": str(config_path.resolve()) if config_path else None, + }) + return + if config_path is not None: + console.print("Configuration file generated.") diff --git a/reflex/reflex.py b/reflex/reflex.py index 3d44a60a34d..d8ccabca2d2 100644 --- a/reflex/reflex.py +++ b/reflex/reflex.py @@ -12,6 +12,7 @@ from reflex_base.config import get_config, reload_config from reflex_base.environment import environment from reflex_base.utils import console, log +from reflex_cli.utils.output import interactive_option from reflex_cli.v2.deployments import hosting_cli from reflex.custom_components.custom_components import custom_components_cli @@ -888,12 +889,7 @@ def makemigrations(message: str | None): help="An optional note recorded on this deployment and shown in " "`reflex cloud apps history`.", ) -@click.option( - "--interactive/--no-interactive", - is_flag=True, - default=True, - help="Whether to list configuration options and ask for confirmation.", -) +@interactive_option @click.option( "--envfile", help="The path to an env file to use. Will override any envs set manually.", diff --git a/tests/units/reflex_base/utils/test_log.py b/tests/units/reflex_base/utils/test_log.py index f204943012b..974e7946707 100644 --- a/tests/units/reflex_base/utils/test_log.py +++ b/tests/units/reflex_base/utils/test_log.py @@ -644,3 +644,52 @@ def test_deprecate_json_location_is_user_frame(monkeypatch, capsys): path, _, lineno = location.rpartition(":") assert Path(path).name == Path(__file__).name assert lineno.isdigit() + + +def test_reserve_stdout_moves_log_records_to_stderr(capsys): + """While stdout is reserved, log records render to stderr instead.""" + log.reserve_stdout() + logger.info("progress") + out, err = capsys.readouterr() + assert out == "" + assert "progress" in err + + +def test_reserve_stdout_moves_console_output_to_stderr(capsys): + """Console prints, tables and rules follow the log records.""" + log.reserve_stdout() + console.print("a message") + console.print_table([["one"]], headers=["col"]) + console.rule("a rule") + out, err = capsys.readouterr() + assert out == "" + assert "a message" in err + assert "one" in err + assert "a rule" in err + + +def test_reserve_stdout_moves_json_records_to_stderr(monkeypatch, capsys): + """JSON log records move too: they are still messages, not the document.""" + monkeypatch.setenv("REFLEX_LOG_JSON", "true") + log.configure() + log.reserve_stdout() + logger.info("progress") + out, err = capsys.readouterr() + assert out == "" + assert json.loads(err)["message"] == "progress" + + +def test_releasing_the_reservation_restores_stdout(capsys): + """Human-readable output goes back to stdout once the document is done.""" + log.reserve_stdout() + log.reserve_stdout(False) + logger.info("progress") + out, _ = capsys.readouterr() + assert "progress" in out + + +def test_reset_releases_the_reservation(): + """Teardown cannot leave a later command writing to the wrong stream.""" + log.reserve_stdout() + log._reset() + assert log.is_stdout_reserved() is False diff --git a/tests/units/reflex_cli/utils/test_output.py b/tests/units/reflex_cli/utils/test_output.py new file mode 100644 index 00000000000..c80e7224068 --- /dev/null +++ b/tests/units/reflex_cli/utils/test_output.py @@ -0,0 +1,193 @@ +"""Tests for the shared machine-readable output options in reflex_cli.utils.output.""" + +import io +import json + +import click +import pytest +from click.testing import CliRunner +from pytest_mock import MockFixture +from reflex_base.utils import log +from reflex_cli.utils import output + +runner = CliRunner() + + +@pytest.fixture(autouse=True) +def _release_stdout(): + """Release any stdout reservation a test left behind. + + Yields: + None. + """ + yield + log.reserve_stdout(False) + + +@click.command() +@output.json_option +@output.interactive_option +def _probe(as_json: bool, interactive: bool): + """Report the resolved flags and whether stdout was reserved.""" + output.print_json({ + "as_json": as_json, + "interactive": interactive, + "stdout_reserved": log.is_stdout_reserved(), + }) + + +def test_stdout_is_tty_false_for_a_plain_stream(monkeypatch): + """A stream nobody is watching is not a terminal.""" + monkeypatch.setattr("sys.stdout", io.StringIO()) + assert output.stdout_is_tty() is False + + +def test_stdout_is_tty_true_for_a_terminal(monkeypatch): + """A stream that claims to be a terminal is one.""" + stream = io.StringIO() + monkeypatch.setattr(stream, "isatty", lambda: True) + monkeypatch.setattr("sys.stdout", stream) + assert output.stdout_is_tty() is True + + +def test_stdout_is_tty_false_for_a_closed_stream(monkeypatch): + """A closed stream answers False rather than raising.""" + stream = io.StringIO() + stream.close() + monkeypatch.setattr("sys.stdout", stream) + assert output.stdout_is_tty() is False + + +def test_stdout_is_tty_false_when_the_stream_has_no_isatty(monkeypatch): + """A replacement stdout without isatty is not a terminal.""" + monkeypatch.setattr("sys.stdout", object()) + assert output.stdout_is_tty() is False + + +@pytest.mark.parametrize( + ("argv", "expected"), + [ + ([], False), + (["apps", "list"], False), + (["apps", "list", "--json"], True), + (["apps", "list", "-j"], True), + (["apps", "list", "--no-json"], False), + # An explicit refusal wins over the flag that asked for it, matching + # click's own last-word-wins parsing closely enough for a stream + # decision made before parsing. + (["apps", "list", "--json", "--no-json"], False), + ], +) +def test_json_requested(argv: list[str], expected: bool): + """A JSON flag on the command line is recognized before click parses it. + + Args: + argv: The arguments to inspect. + expected: Whether they ask for JSON. + """ + assert output.json_requested(argv) is expected + + +def test_reserve_stdout_for_argv_clears_a_stale_reservation(): + """A command line without --json releases a previous reservation.""" + log.reserve_stdout(True) + output.reserve_stdout_for_argv(["apps", "list"]) + assert log.is_stdout_reserved() is False + + +def test_interactive_defaults_off_without_a_terminal(mocker: MockFixture): + """Nothing prompts when stdout is a pipe, a CI job or an agent.""" + mocker.patch("reflex_cli.utils.output.stdout_is_tty", return_value=False) + + result = runner.invoke(_probe, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["interactive"] is False + + +def test_interactive_defaults_on_with_a_terminal(mocker: MockFixture): + """A person at a terminal still gets the prompts.""" + mocker.patch("reflex_cli.utils.output.stdout_is_tty", return_value=True) + + result = runner.invoke(_probe, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["interactive"] is True + + +@pytest.mark.parametrize( + ("flag", "expected"), + [("--interactive", True), ("-i", True), ("--no-interactive", False)], +) +def test_explicit_interactive_flag_beats_the_terminal( + mocker: MockFixture, flag: str, expected: bool +): + """An explicit flag decides regardless of what stdout is. + + Args: + mocker: The pytest-mock fixture. + flag: The spelling passed on the command line. + expected: The value it resolves to. + """ + mocker.patch("reflex_cli.utils.output.stdout_is_tty", return_value=not expected) + + result = runner.invoke(_probe, [flag]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["interactive"] is expected + + +def test_json_flag_reserves_stdout_before_the_command_runs(): + """The reservation is in place by the time the body can log anything.""" + result = runner.invoke(_probe, ["--json"]) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + assert payload["as_json"] is True + assert payload["stdout_reserved"] is True + + +def test_no_json_leaves_stdout_unreserved(): + """Without --json, human-readable output keeps stdout.""" + result = runner.invoke(_probe, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["stdout_reserved"] is False + + +def test_print_json_writes_one_document_while_the_console_writes_to_stderr(): + """The document owns stdout even while messages are being printed. + + This is what makes ``--json`` parseable: a warning from a helper deep in + the call stack would otherwise land in the middle of the document. + """ + from reflex_cli.utils import console + + @click.command() + @output.json_option + def noisy(as_json: bool): + """Print a message and then the document.""" + console.print("a message for a person") + output.print_json({"ok": True}) + + result = runner.invoke(noisy, ["--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"ok": True} + assert "a message for a person" in result.stderr + + +def test_print_json_serializes_values_json_cannot(): + """A value without a JSON form is rendered as its string, not an error.""" + + @click.command() + def pathy(): + """Print a payload holding a non-serializable value.""" + from pathlib import Path + + output.print_json({"path": Path("cloud.yml")}) + + result = runner.invoke(pathy, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"path": "cloud.yml"} diff --git a/tests/units/reflex_cli/v2/test_apps.py b/tests/units/reflex_cli/v2/test_apps.py index 96c574cd9ea..b26e75966fb 100644 --- a/tests/units/reflex_cli/v2/test_apps.py +++ b/tests/units/reflex_cli/v2/test_apps.py @@ -90,8 +90,6 @@ def test_app_history_as_json(mocker: MockFixture): } ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, ["apps", "history", "test_app_id", "--json"], @@ -104,19 +102,17 @@ def test_app_history_as_json(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with( - json.dumps([ - { - "id": "deployment1", - "status": "success", - "hostname": "example.com", - "python version": "3.10", - "reflex version": "1.2.3", - "vm type": "small", - "timestamp": "2024-11-29T12:00:00Z", - } - ]) - ) + assert json.loads(result.stdout) == [ + { + "id": "deployment1", + "status": "success", + "hostname": "example.com", + "python version": "3.10", + "reflex version": "1.2.3", + "vm type": "small", + "timestamp": "2024-11-29T12:00:00Z", + } + ] def test_app_history_no_deployments(mocker: MockFixture): @@ -588,7 +584,7 @@ def test_delete_app_success(mocker: MockFixture, caplog: pytest.LogCaptureFixtur ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="y") - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count == 2 @@ -649,7 +645,7 @@ def test_delete_app_failure(mocker: MockFixture, caplog: pytest.LogCaptureFixtur ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="y") - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count == 2 @@ -740,7 +736,7 @@ def test_delete_app_http_error(mocker: MockFixture, caplog: pytest.LogCaptureFix return_value={"X-API-TOKEN": "fake_token"}, ) - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count >= 1 @@ -775,7 +771,7 @@ def test_delete_app_confirmation_cancelled( ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="n") - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count == 2 @@ -875,7 +871,7 @@ def test_delete_app_get_app_fails_fallback_to_unknown( ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="y") - result = runner.invoke(hosting_cli, ["apps", "delete", "app123"]) + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--interactive"]) assert result.exit_code == 0, result.output assert mock_get_app.call_count == 1 @@ -910,7 +906,9 @@ def test_delete_app_with_app_name_confirmation( ) mock_ask = mocker.patch("reflex_cli.utils.console.ask", return_value="y") - result = runner.invoke(hosting_cli, ["apps", "delete", "--app-name", "my-test-app"]) + result = runner.invoke( + hosting_cli, ["apps", "delete", "--app-name", "my-test-app", "--interactive"] + ) assert result.exit_code == 0, result.output mock_search_app.assert_called_once() @@ -1197,8 +1195,6 @@ def test_list_apps_json_output(mocker: MockFixture): "reflex_cli.utils.hosting.list_apps", return_value=[{"id": "1", "name": "App1"}], ) - mock_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke(hosting_cli, ["apps", "list", "--json"]) assert result.exit_code == 0, result.output @@ -1208,7 +1204,7 @@ def test_list_apps_json_output(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_print.assert_called_once_with(json.dumps([{"id": "1", "name": "App1"}])) + assert json.loads(result.stdout) == [{"id": "1", "name": "App1"}] def test_list_apps_error(mocker: MockFixture, caplog: pytest.LogCaptureFixture): @@ -1712,7 +1708,7 @@ def test_app_rollback_defaults_to_cancel(mocker: MockFixture): result = runner.invoke( apps_cli, - ["rollback", "dep-1", "--app-id", "app-1"], + ["rollback", "dep-1", "--app-id", "app-1", "--interactive"], input="\n", ) @@ -1840,3 +1836,400 @@ def test_resolve_app_id_explicit_id_wins(mocker: MockFixture): ) search.assert_not_called() read_config.assert_not_called() + + +def _authed(mocker: MockFixture) -> hosting.AuthenticatedClient: + """Patch the client lookup and return the client it hands back. + + Args: + mocker: The pytest-mock fixture. + + Returns: + The authenticated client every command under test will receive. + """ + client = hosting.AuthenticatedClient(token="fake-token", validated_data={}) + mocker.patch( + "reflex_cli.utils.hosting.get_authenticated_client", return_value=client + ) + return client + + +def test_app_logs_does_not_follow_by_default(mocker: MockFixture): + """One page is fetched and the command returns, with nothing to answer. + + Following prompts between pages, and a prompt nobody answers is a command + that never exits -- which is why it is opt-in. + """ + _authed(mocker) + mock_get_app_logs = mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1"], "next-cursor"], + ) + prompt = mocker.patch("rich.prompt.Prompt.ask", return_value="") + + result = runner.invoke(hosting_cli, ["apps", "logs", "app123", "--interactive"]) + + assert result.exit_code == 0, result.output + mock_get_app_logs.assert_called_once() + prompt.assert_not_called() + + +def test_app_logs_follow_needs_a_person_to_answer_the_prompt(mocker: MockFixture): + """--follow is ignored without interactive mode rather than hanging.""" + _authed(mocker) + mock_get_app_logs = mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1"], "next-cursor"], + ) + prompt = mocker.patch("rich.prompt.Prompt.ask", return_value="") + + result = runner.invoke( + hosting_cli, + ["apps", "logs", "app123", "--follow", "true", "--no-interactive"], + ) + + assert result.exit_code == 0, result.output + mock_get_app_logs.assert_called_once() + prompt.assert_not_called() + + +def test_app_logs_follow_pages_when_asked_interactively(mocker: MockFixture): + """Passing --follow at a terminal still walks the pages.""" + _authed(mocker) + mock_get_app_logs = mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1"], "next-cursor"], + ) + prompt = mocker.patch("rich.prompt.Prompt.ask", return_value="exit") + + result = runner.invoke( + hosting_cli, + ["apps", "logs", "app123", "--follow", "true", "--interactive"], + ) + + assert result.exit_code == 0, result.output + mock_get_app_logs.assert_called_once() + prompt.assert_called_once() + + +def test_app_logs_json_output(mocker: MockFixture): + """The page and its next cursor come back as one document.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1", "log2"], "next-cursor"], + ) + + result = runner.invoke(hosting_cli, ["apps", "logs", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + # Reversed into chronological order, the same as the rendered form. + "entries": ["log2", "log1"], + "cursor": "next-cursor", + "error": None, + } + + +def test_app_logs_json_output_never_follows(mocker: MockFixture): + """--follow cannot page a document that is only complete once.""" + _authed(mocker) + mock_get_app_logs = mocker.patch( + "reflex_cli.utils.hosting.get_app_logs", + return_value=[["log1"], "next-cursor"], + ) + prompt = mocker.patch("rich.prompt.Prompt.ask", return_value="") + + result = runner.invoke( + hosting_cli, + ["apps", "logs", "app123", "--json", "--follow", "true", "--interactive"], + ) + + assert result.exit_code == 0, result.output + mock_get_app_logs.assert_called_once() + prompt.assert_not_called() + assert json.loads(result.stdout)["cursor"] == "next-cursor" + + +def test_app_logs_json_output_when_empty(mocker: MockFixture): + """No logs is an empty document rather than a warning to parse.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_app_logs", return_value=[]) + + result = runner.invoke(hosting_cli, ["apps", "logs", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "entries": [], + "cursor": None, + "error": None, + } + + +def test_stop_app_json_output(mocker: MockFixture): + """Stopping an app reports the outcome as a document.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.stop_app", return_value="app stopped") + + result = runner.invoke(hosting_cli, ["apps", "stop", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "stopped": True, + "message": "app stopped", + } + + +def test_stop_app_json_output_on_failure(mocker: MockFixture): + """A refusal is reported in the document, not only in the log.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.stop_app", return_value="stop failed") + + result = runner.invoke(hosting_cli, ["apps", "stop", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["stopped"] is False + + +def test_start_app_json_output(mocker: MockFixture): + """Starting an app reports the outcome as a document.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.start_app", return_value="app started") + + result = runner.invoke(hosting_cli, ["apps", "start", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "started": True, + "message": "app started", + } + + +def test_delete_app_json_output(mocker: MockFixture): + """Deleting an app reports the outcome as a document.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_app", + return_value={"id": "app123", "name": "test-app"}, + ) + mocker.patch("reflex_cli.utils.hosting.delete_app", return_value="") + + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "deleted": True, + "message": "", + } + + +def test_delete_app_json_output_on_failure(mocker: MockFixture): + """A refusal is reported as a failed deletion, not as a deleted app.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_app", + return_value={"id": "app123", "name": "test-app"}, + ) + mocker.patch( + "reflex_cli.utils.hosting.delete_app", + return_value="delete app failed: app is deploying", + ) + + result = runner.invoke(hosting_cli, ["apps", "delete", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "deleted": False, + "message": "delete app failed: app is deploying", + } + + +def test_app_logs_json_output_when_unreadable(mocker: MockFixture): + """Logs that could not be read are distinguishable from none existing.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_app_logs", return_value=None) + + result = runner.invoke(hosting_cli, ["apps", "logs", "app123", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "entries": [], + "cursor": None, + "error": "Unable to retrieve logs.", + } + + +def test_delete_app_json_output_when_cancelled(mocker: MockFixture): + """Declining the confirmation is reported rather than left silent.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_app", + return_value={"id": "app123", "name": "test-app"}, + ) + delete = mocker.patch("reflex_cli.utils.hosting.delete_app") + mocker.patch("reflex_cli.utils.console.ask", return_value="n") + + result = runner.invoke( + hosting_cli, ["apps", "delete", "app123", "--json", "--interactive"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "deleted": False, + "cancelled": True, + } + delete.assert_not_called() + + +def test_app_rollback_json_output(mocker: MockFixture): + """A rollback reports what it rolled back to.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.rollback_deployment", return_value="") + + result = runner.invoke( + hosting_cli, + ["apps", "rollback", "dep-1", "--app-id", "app-1", "--json"], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app-1", + "deployment_id": "dep-1", + "rolled_back": True, + "cancelled": False, + } + + +def test_app_describe_json_output(mocker: MockFixture): + """Setting a changelog note reports the note it set.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.update_deployment_description", return_value="" + ) + + result = runner.invoke( + hosting_cli, + [ + "apps", + "describe", + "dep-1", + "--app-id", + "app-1", + "--description", + "hotfix", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app-1", + "deployment_id": "dep-1", + "description": "hotfix", + } + + +def test_deployment_build_logs_json_output(mocker: MockFixture): + """Build logs come back as a field rather than as raw console text.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_deployment_build_logs", + return_value="step 1\nstep 2", + ) + + result = runner.invoke(hosting_cli, ["apps", "build-logs", "dep-1", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "deployment_id": "dep-1", + "logs": "step 1\nstep 2", + } + + +def test_deployment_status_json_output(mocker: MockFixture): + """A status read reports the status and whether it is a failure.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.get_deployment_status", return_value="deploying" + ) + + result = runner.invoke(hosting_cli, ["apps", "status", "dep-1", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "deployment_id": "dep-1", + "status": "deploying", + "success": True, + } + + +def test_deployment_status_json_output_while_watching(mocker: MockFixture): + """Watching re-reads the status once it ends, since the watch returns a bool.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.watch_deployment_status", return_value=True) + mocker.patch( + "reflex_cli.utils.hosting.get_deployment_status", + return_value="completed successfully", + ) + + result = runner.invoke( + hosting_cli, ["apps", "status", "dep-1", "--watch", "--json"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "deployment_id": "dep-1", + "status": "completed successfully", + "success": True, + } + + +def test_scale_app_json_output(mocker: MockFixture): + """Scaling reports the parameters it applied.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.scale_app") + mocker.patch( + "reflex_cli.core.config.Config.from_yaml_or_toml_or_default", + return_value=Config(), + ) + + result = runner.invoke( + hosting_cli, ["apps", "scale", "app123", "--vmtype", "c1m1", "--json"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "scaled": True, + "vmtype": "c1m1", + "regions": [], + "scale_type": "size", + } + + +def test_json_output_keeps_human_messages_off_stdout(mocker: MockFixture): + """A log line from the command body never lands inside the document.""" + _authed(mocker) + mocker.patch( + "reflex_cli.utils.hosting.list_apps", return_value=[{"id": "1", "name": "App1"}] + ) + mocker.patch( + "reflex_cli.utils.hosting.get_selected_project", return_value="project-1" + ) + mocker.patch( + "reflex_cli.utils.hosting.get_project", + return_value={"id": "project-1", "name": "My Project"}, + ) + + result = runner.invoke(hosting_cli, ["apps", "list", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == [{"id": "1", "name": "App1"}] diff --git a/tests/units/reflex_cli/v2/test_gcp.py b/tests/units/reflex_cli/v2/test_gcp.py index ac7d6aec193..41d1ccf344e 100644 --- a/tests/units/reflex_cli/v2/test_gcp.py +++ b/tests/units/reflex_cli/v2/test_gcp.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import os from pathlib import Path from unittest import mock @@ -674,7 +675,15 @@ def test_gcp_deploy_aborts_on_no(mocker: MockFixture, tmp_path: Path): result = runner.invoke( hosting_cli, - ["gcp-standalone", "--gcp", "--gcp-project", "p", "--source", str(tmp_path)], + [ + "gcp-standalone", + "--gcp", + "--gcp-project", + "p", + "--source", + str(tmp_path), + "--interactive", + ], input="n\n", ) @@ -1057,3 +1066,92 @@ def test_deploy_gcp_requires_gcp_project(mocker: MockFixture, tmp_path: Path): @pytest.fixture(autouse=True) def _no_log_level_side_effects(mocker: MockFixture): mocker.patch("reflex_cli.utils.console.set_log_level") + + +def test_gcp_deploy_json_output(mocker: MockFixture, tmp_path: Path): + """A standalone deploy reports where it deployed and whether it worked.""" + _patch_environment(mocker) + _mock_manifest_response(mocker) + + result = runner.invoke( + hosting_cli, + [ + "gcp-standalone", + "--gcp", + "--gcp-project", + "p", + "--region", + "us-central1", + "--service-name", + "svc", + "--version", + "v1", + "--source", + str(tmp_path), + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "dry_run": False, + "deployed": True, + "exit_code": 0, + "gcp_project": "p", + "region": "us-central1", + "service_name": "svc", + "version": "v1", + } + + +def test_gcp_deploy_json_output_on_dry_run(mocker: MockFixture, tmp_path: Path): + """A dry run hands back what it would have staged, unrendered.""" + run_mock = _patch_environment(mocker) + _mock_manifest_response(mocker) + + result = runner.invoke( + hosting_cli, + [ + "gcp-standalone", + "--gcp", + "--gcp-project", + "p", + "--source", + str(tmp_path), + "--dry-run", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + assert payload["dry_run"] is True + assert payload["dockerfile"] == DOCKERFILE + assert "gcloud builds submit" in payload["deploy_script"] + assert payload["deploy_env"]["GCP_PROJECT"] == "p" + run_mock.assert_not_called() + + +def test_gcp_deploy_json_output_on_script_failure(mocker: MockFixture, tmp_path: Path): + """A failing script still produces a document, alongside the non-zero exit.""" + run_mock = _patch_environment(mocker) + run_mock.return_value = 7 + _mock_manifest_response(mocker) + + result = runner.invoke( + hosting_cli, + [ + "gcp-standalone", + "--gcp", + "--gcp-project", + "p", + "--source", + str(tmp_path), + "--json", + ], + ) + + assert result.exit_code == 7 + payload = json.loads(result.stdout) + assert payload["deployed"] is False + assert payload["exit_code"] == 7 diff --git a/tests/units/reflex_cli/v2/test_project.py b/tests/units/reflex_cli/v2/test_project.py index 24bb75cd182..40649cce35e 100644 --- a/tests/units/reflex_cli/v2/test_project.py +++ b/tests/units/reflex_cli/v2/test_project.py @@ -83,8 +83,6 @@ def test_create_project_with_json_output(mocker: MockFixture): token="valid_token", validated_data={"foo": "bar"} ), ) - mock_print = mocker.patch("reflex_cli.utils.console.print") - project_name = "test_project" token = "valid_token" @@ -99,7 +97,7 @@ def test_create_project_with_json_output(mocker: MockFixture): ), ) - mock_print.assert_called_once_with(json.dumps({"name": "test_project", "id": 1})) + assert json.loads(result.stdout) == {"name": "test_project", "id": 1} assert result.exit_code == 0, result.output @@ -511,8 +509,6 @@ def test_get_project_roles_as_json(mocker: MockFixture): {"role": "viewer", "user": "user2@example.com"}, ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, ["project", "roles", "--project-id", "test_project_id", "--json"], @@ -525,12 +521,10 @@ def test_get_project_roles_as_json(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with( - json.dumps([ - {"role": "admin", "user": "user1@example.com"}, - {"role": "viewer", "user": "user2@example.com"}, - ]) - ) + assert json.loads(result.stdout) == [ + {"role": "admin", "user": "user1@example.com"}, + {"role": "viewer", "user": "user2@example.com"}, + ] def test_get_project_roles_empty_roles(mocker: MockFixture): @@ -684,8 +678,6 @@ def test_get_project_role_permissions_as_json(mocker: MockFixture): {"permission": "write", "resource": "resource2"}, ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, [ @@ -706,12 +698,10 @@ def test_get_project_role_permissions_as_json(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with( - json.dumps([ - {"permission": "read", "resource": "resource1"}, - {"permission": "write", "resource": "resource2"}, - ]) - ) + assert json.loads(result.stdout) == [ + {"permission": "read", "resource": "resource1"}, + {"permission": "write", "resource": "resource2"}, + ] def test_get_project_role_permissions_empty_permissions(mocker: MockFixture): @@ -825,8 +815,6 @@ def test_get_project_role_users_as_json(mocker: MockFixture): {"user_id": "user2", "role": "developer"}, ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, [ @@ -845,12 +833,10 @@ def test_get_project_role_users_as_json(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with( - json.dumps([ - {"user_id": "user1", "role": "admin"}, - {"user_id": "user2", "role": "developer"}, - ]) - ) + assert json.loads(result.stdout) == [ + {"user_id": "user1", "role": "admin"}, + {"user_id": "user2", "role": "developer"}, + ] def test_get_project_role_users_empty_users(mocker: MockFixture): @@ -880,3 +866,78 @@ def test_get_project_role_users_empty_users(mocker: MockFixture): ), ) mock_console_print.assert_called_once_with("[]") + + +def _authed(mocker: MockFixture) -> hosting.AuthenticatedClient: + """Patch the client lookup and return the client it hands back. + + Args: + mocker: The pytest-mock fixture. + + Returns: + The authenticated client every command under test will receive. + """ + client = hosting.AuthenticatedClient(token="fake-token", validated_data={}) + mocker.patch( + "reflex_cli.utils.hosting.get_authenticated_client", return_value=client + ) + return client + + +def test_invite_user_to_project_json_output(mocker: MockFixture): + """An invite reports who was invited to what role.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.invite_user_to_project", return_value="ok") + + result = runner.invoke( + hosting_cli, ["project", "invite", "role-1", "user-1", "--json"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "role_id": "role-1", + "user_id": "user-1", + "invited": True, + } + + +def test_select_project_json_output(mocker: MockFixture): + """Selecting a project reports the project it selected.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_project", return_value={"id": "p1"}) + mocker.patch("reflex_cli.utils.hosting.select_project", return_value="selected p1") + + result = runner.invoke(hosting_cli, ["project", "select", "p1", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "project_id": "p1", + "selected": True, + "message": "selected p1", + } + + +def test_get_selected_project_json_output(mocker: MockFixture): + """The selected project comes back as a document rather than a table.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_selected_project", return_value="p1") + mocker.patch( + "reflex_cli.utils.hosting.get_project", + return_value={"id": "p1", "name": "My Project"}, + ) + + result = runner.invoke(hosting_cli, ["project", "selected", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"project_id": "p1", "name": "My Project"} + + +def test_get_selected_project_json_output_when_none(mocker: MockFixture): + """No selection is a document saying so, not a warning to parse.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.get_selected_project", return_value=None) + + result = runner.invoke(hosting_cli, ["project", "selected", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"project_id": None, "name": None} diff --git a/tests/units/reflex_cli/v2/test_scan.py b/tests/units/reflex_cli/v2/test_scan.py index ce54962f9b9..feb82fe4bee 100644 --- a/tests/units/reflex_cli/v2/test_scan.py +++ b/tests/units/reflex_cli/v2/test_scan.py @@ -216,14 +216,12 @@ def test_scan_json_output(mocker: MockFixture, tmp_path: Path): "reflex_cli.utils.hosting.get_security_review", return_value={"job_id": "job123", "status": "complete", "result": _RESULT}, ) - mock_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke( hosting_cli, ["scan", str(tmp_path), "--json", "--fail-on", "none"] ) assert result.exit_code == 0, result.output - mock_print.assert_called_once_with(json.dumps(_RESULT)) + assert json.loads(result.stdout) == _RESULT def test_scan_polls_until_complete(mocker: MockFixture, tmp_path: Path): diff --git a/tests/units/reflex_cli/v2/test_secrets.py b/tests/units/reflex_cli/v2/test_secrets.py index cd2060e5bf5..45a9c3baddd 100644 --- a/tests/units/reflex_cli/v2/test_secrets.py +++ b/tests/units/reflex_cli/v2/test_secrets.py @@ -1,3 +1,4 @@ +import json import logging import tempfile from pathlib import Path @@ -104,8 +105,6 @@ def test_get_secrets_json_output(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - app_id = "app_id" args = ["secrets", "list", app_id, "--json"] @@ -118,10 +117,10 @@ def test_get_secrets_json_output(mocker: MockFixture): token="fake-token", validated_data={"foo": "bar"} ), ) - mock_console_print.assert_called_once_with({ + assert json.loads(result.stdout) == { "secret_key_1": "value1", "secret_key_2": "value2", - }) + } assert result.exit_code == 0, result.output @@ -297,3 +296,102 @@ def test_update_secrets_invalid_env_format(mocker: MockFixture): assert result.exit_code == 1 assert "Invalid env format: should be =." in result.stdout + + +def _authed(mocker: MockFixture) -> hosting.AuthenticatedClient: + """Patch the client lookup and return the client it hands back. + + Args: + mocker: The pytest-mock fixture. + + Returns: + The authenticated client every command under test will receive. + """ + client = hosting.AuthenticatedClient(token="fake-token", validated_data={}) + mocker.patch( + "reflex_cli.utils.hosting.get_authenticated_client", return_value=client + ) + return client + + +def test_update_secrets_json_output(mocker: MockFixture): + """An update reports the names it wrote, never the values.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.update_secrets") + + result = runner.invoke( + hosting_cli, + [ + "secrets", + "update", + "app123", + "--env", + "B=2", + "--env", + "A=1", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "updated": ["A", "B"], + "rebooted": False, + } + assert "1" not in json.dumps(json.loads(result.stdout)["updated"]) + + +def test_update_secrets_json_output_keeps_warnings_off_stdout( + mocker: MockFixture, tmp_path: Path +): + """A warning raised on the way through does not break the document. + + Args: + mocker: The pytest-mock fixture. + tmp_path: A temporary directory to hold the env file. + """ + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.update_secrets") + envfile = tmp_path / ".env" + envfile.write_text("A=1\n") + + result = runner.invoke( + hosting_cli, + [ + "secrets", + "update", + "app123", + "--envfile", + str(envfile), + "--env", + "B=2", + "--json", + ], + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "updated": ["A"], + "rebooted": False, + } + assert "--envfile is set; ignoring --env" in result.stderr + + +def test_delete_secret_json_output(mocker: MockFixture): + """Deleting a secret reports the key it removed.""" + _authed(mocker) + mocker.patch("reflex_cli.utils.hosting.delete_secret", return_value="deleted") + + result = runner.invoke( + hosting_cli, ["secrets", "delete", "app123", "MY_KEY", "--json"] + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "app_id": "app123", + "key": "MY_KEY", + "deleted": True, + "rebooted": False, + } diff --git a/tests/units/reflex_cli/v2/test_vmtypes_regions.py b/tests/units/reflex_cli/v2/test_vmtypes_regions.py index 56134d3e31c..32830a6cea8 100644 --- a/tests/units/reflex_cli/v2/test_vmtypes_regions.py +++ b/tests/units/reflex_cli/v2/test_vmtypes_regions.py @@ -58,15 +58,14 @@ def test_get_vm_types_as_json(mocker: MockFixture): {"id": "2", "name": "Medium", "cpu": 4, "ram": 8}, ], ) - mock_console_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke(hosting_cli, ["vmtypes", "--json"]) assert result.exit_code == 0, result.output mock_get_vm_types.assert_called_once() - mock_console_print.assert_called_once_with( - '[{"id": "1", "name": "Small", "cpu": 2, "ram": 4}, {"id": "2", "name": "Medium", "cpu": 4, "ram": 8}]' - ) + assert json.loads(result.stdout) == [ + {"id": "1", "name": "Small", "cpu": 2, "ram": 4}, + {"id": "2", "name": "Medium", "cpu": 4, "ram": 8}, + ] def test_get_vm_types_empty(mocker: MockFixture): @@ -164,18 +163,14 @@ def test_get_deployment_regions_as_json(mocker: MockFixture): {"name": "Stockholm, Sweden", "code": "arn"}, ], ) - mock_print = mocker.patch("reflex_cli.utils.console.print") - result = runner.invoke(hosting_cli, ["regions", "--json"]) assert result.exit_code == 0, result.output mock_get_regions.assert_called_once() - mock_print.assert_called_once_with( - json.dumps([ - {"name": "Amsterdam, Netherlands", "code": "ams"}, - {"name": "Stockholm, Sweden", "code": "arn"}, - ]) - ) + assert json.loads(result.stdout) == [ + {"name": "Amsterdam, Netherlands", "code": "ams"}, + {"name": "Stockholm, Sweden", "code": "arn"}, + ] def test_get_deployment_regions_empty(mocker: MockFixture): @@ -223,3 +218,54 @@ def test_get_deployment_regions_http_error( assert result.exit_code == 0, result.output errors = [r.getMessage() for r in caplog.records if r.levelno == logging.ERROR] assert errors == ["Unable to get regions due to HTTP Error."] + + +def test_create_token_json_output(mocker: MockFixture): + """Minting a token reports it as a field rather than in a log line.""" + mocker.patch( + "reflex_cli.utils.hosting.get_authenticated_client", + return_value=mocker.MagicMock(), + ) + mocker.patch("reflex_cli.utils.hosting.create_token", return_value="tok-1") + + result = runner.invoke(hosting_cli, ["create-token", "ci", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "name": "ci", + "token": "tok-1", + "expires_in_days": 90, + } + + +def test_generate_cloud_config_json_output(mocker: MockFixture, tmp_path): + """Generating a config reports the file it wrote. + + Args: + mocker: The pytest-mock fixture. + tmp_path: A temporary directory standing in for the app root. + """ + written = tmp_path / "cloud.yml" + mocker.patch("reflex_cli.utils.hosting.generate_config", return_value=written) + + result = runner.invoke(hosting_cli, ["config", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == { + "generated": True, + "path": str(written.resolve()), + } + + +def test_generate_cloud_config_json_output_when_nothing_written(mocker: MockFixture): + """A config that already exists is reported as not generated. + + Args: + mocker: The pytest-mock fixture. + """ + mocker.patch("reflex_cli.utils.hosting.generate_config", return_value=None) + + result = runner.invoke(hosting_cli, ["config", "--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout) == {"generated": False, "path": None} From 8f690bb0684f733c6c4d913137bb611cba4c4af6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 22:37:42 +0000 Subject: [PATCH 2/2] Fix flag ordering and reservation lifetime in the JSON stdout reservation Both from review on #6917. `json_requested` read argv as a set, so it disagreed with click wherever order mattered: click parses a boolean flag pair last-wins, and `--no-json --json` enables JSON while the set test answered "a refusal is present". Scanning back to front gives click's answer. The same disagreement came from combined short flags -- `-ij` is `-i -j` to click and was nothing to a membership test -- so the scan reads those too. Reading them means it can also fire on a `-j` click would take as another option's value, which is the direction to be wrong in: a message on stderr costs some context, one inside the document costs the whole parse. The reservation is process-global and nothing released it, so a `--json` command left every later log line in the process writing to stderr. A CLI process exits and never notices; an embedding one, or a second run in the same interpreter, does. It now follows the click context's lifetime and restores the previous value on close. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UqTVZN9bsCxt7hz5U3qJok --- .../src/reflex_cli/utils/output.py | 72 ++++++++++++++++-- .../src/reflex_cli/v2/deployments.py | 2 +- tests/units/reflex_cli/utils/test_output.py | 75 +++++++++++++++---- 3 files changed, 128 insertions(+), 21 deletions(-) diff --git a/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py b/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py index b15acd5c423..7606c4cb010 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/utils/output.py @@ -22,9 +22,38 @@ # before click has parsed the subcommand's options -- anything it says would # otherwise land on stdout ahead of the document. _JSON_FLAGS = frozenset({"--json", "-j"}) +_JSON_SHORT = "j" _NO_JSON_FLAG = "--no-json" +def _json_flag_state(arg: str) -> bool | None: + """Read what one command-line argument says about JSON output. + + Args: + arg: A single command-line argument. + + Returns: + True if it asks for JSON, False if it refuses it, None if it says + nothing either way. + """ + if arg == _NO_JSON_FLAG: + return False + if arg in _JSON_FLAGS: + return True + # Short flags combine, so `-ij` is `-i -j`. Reading them means this scan + # can also fire on a `-j` that click would take as some other option's + # value, which is the direction to be wrong in: a message on stderr costs + # a little context, one inside the document costs the whole parse. + return ( + True + if len(arg) > 1 + and arg.startswith("-") + and not arg.startswith("--") + and _JSON_SHORT in arg[1:] + else None + ) + + def stdout_is_tty() -> bool: """Check whether stdout is attached to a terminal. @@ -73,17 +102,43 @@ def _resolve_interactive( def json_requested(argv: Sequence[str] | None = None) -> bool: """Check whether a command line asks for JSON output. + Scanned back to front, so the last flag decides -- the same answer click + reaches for a boolean flag pair, which a set membership test cannot give: + ``--no-json --json`` enables JSON and ``--json --no-json`` does not. + Args: argv: The arguments to inspect; defaults to this process's own. Returns: - True if a JSON flag is present and not overridden by ``--no-json``. + True if the command line asks for JSON output. + """ + args = sys.argv[1:] if argv is None else argv + for arg in reversed(list(args)): + if (state := _json_flag_state(arg)) is not None: + return state + return False + + +def _hold_reservation(ctx: click.Context, reserved: bool) -> None: + """Reserve stdout for this context, releasing it again when it closes. + + The reservation is process-global, so without an explicit release a + ``--json`` command leaves every later log line in the process writing to + stderr -- which a CLI process never notices, and an embedding one or a + second run in the same interpreter does. + + Args: + ctx: The click context whose lifetime the reservation follows. + reserved: Whether stdout carries data rather than human output. """ - args = set(sys.argv[1:] if argv is None else argv) - return bool(_JSON_FLAGS & args) and _NO_JSON_FLAG not in args + previous = log.is_stdout_reserved() + log.reserve_stdout(reserved) + ctx.call_on_close(lambda: log.reserve_stdout(previous)) -def reserve_stdout_for_argv(argv: Sequence[str] | None = None) -> None: +def reserve_stdout_for_argv( + argv: Sequence[str] | None = None, *, ctx: click.Context | None = None +) -> None: """Reserve stdout up front when the command line asks for JSON. Always writes the reservation rather than only setting it, so a long-lived @@ -92,8 +147,13 @@ def reserve_stdout_for_argv(argv: Sequence[str] | None = None) -> None: Args: argv: The arguments to inspect; defaults to this process's own. + ctx: The click context to release the reservation with, if there is one. """ - log.reserve_stdout(json_requested(argv)) + reserved = json_requested(argv) + if ctx is None: + log.reserve_stdout(reserved) + return + _hold_reservation(ctx, reserved) def _reserve_stdout(ctx: click.Context, param: click.Parameter, value: bool) -> bool: @@ -108,7 +168,7 @@ def _reserve_stdout(ctx: click.Context, param: click.Parameter, value: bool) -> The flag's value, unchanged. """ if value: - log.reserve_stdout() + _hold_reservation(ctx, True) return value diff --git a/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py b/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py index e876fd39904..419a44a06a9 100644 --- a/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py +++ b/packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py @@ -39,7 +39,7 @@ def hosting_cli(ctx: click.Context) -> None: # Before anything below can speak: this callback runs ahead of the # subcommand's own option parsing, so its --json is not known yet and a # warning from here would land on stdout in front of the document. - reserve_stdout_for_argv() + reserve_stdout_for_argv(ctx=ctx) if _reflex_version is None: ctx.fail("Reflex is not installed. Install it with `pip install reflex`.") diff --git a/tests/units/reflex_cli/utils/test_output.py b/tests/units/reflex_cli/utils/test_output.py index c80e7224068..603e1b4d287 100644 --- a/tests/units/reflex_cli/utils/test_output.py +++ b/tests/units/reflex_cli/utils/test_output.py @@ -64,20 +64,26 @@ def test_stdout_is_tty_false_when_the_stream_has_no_isatty(monkeypatch): assert output.stdout_is_tty() is False -@pytest.mark.parametrize( - ("argv", "expected"), - [ - ([], False), - (["apps", "list"], False), - (["apps", "list", "--json"], True), - (["apps", "list", "-j"], True), - (["apps", "list", "--no-json"], False), - # An explicit refusal wins over the flag that asked for it, matching - # click's own last-word-wins parsing closely enough for a stream - # decision made before parsing. - (["apps", "list", "--json", "--no-json"], False), - ], -) +# Every spelling that has to agree with click, since the scan runs before +# click parses and decides where the group callback's own output goes. +_JSON_ARGV_CASES = [ + ([], False), + (["apps", "list"], False), + (["apps", "list", "--json"], True), + (["apps", "list", "-j"], True), + (["apps", "list", "--no-json"], False), + # Last flag wins, in both directions: click parses these as a pair, so a + # membership test that answers "--no-json is present" is wrong about the + # second one. + (["apps", "list", "--json", "--no-json"], False), + (["apps", "list", "--no-json", "--json"], True), + # Short flags combine. + (["apps", "list", "-ij"], True), + (["apps", "list", "-ji"], True), +] + + +@pytest.mark.parametrize(("argv", "expected"), _JSON_ARGV_CASES) def test_json_requested(argv: list[str], expected: bool): """A JSON flag on the command line is recognized before click parses it. @@ -88,6 +94,23 @@ def test_json_requested(argv: list[str], expected: bool): assert output.json_requested(argv) is expected +@pytest.mark.parametrize(("argv", "expected"), _JSON_ARGV_CASES) +def test_json_requested_agrees_with_click(argv: list[str], expected: bool): + """The pre-parse scan reaches the same answer click's parser does. + + The scan only exists to route output emitted before parsing, so a + disagreement puts a log line on the stdout a document is about to own. + + Args: + argv: The arguments to inspect. + expected: Whether they ask for JSON. + """ + result = runner.invoke(_probe, argv[2:]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["as_json"] is expected + + def test_reserve_stdout_for_argv_clears_a_stale_reservation(): """A command line without --json releases a previous reservation.""" log.reserve_stdout(True) @@ -155,6 +178,30 @@ def test_no_json_leaves_stdout_unreserved(): assert json.loads(result.stdout)["stdout_reserved"] is False +def test_reservation_is_released_when_the_command_ends(): + """The reservation lasts the command, not the process. + + A CLI process exits and never notices, but an embedding one -- or a second + run in the same interpreter -- would have every later log line writing to + stderr on behalf of a command that finished. + """ + result = runner.invoke(_probe, ["--json"]) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["stdout_reserved"] is True + assert log.is_stdout_reserved() is False + + +def test_a_later_command_is_not_reserved_by_an_earlier_one(): + """A plain run after a --json run still writes to stdout.""" + runner.invoke(_probe, ["--json"]) + + result = runner.invoke(_probe, []) + + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["stdout_reserved"] is False + + def test_print_json_writes_one_document_while_the_console_writes_to_stderr(): """The document owns stdout even while messages are being printed.