diff --git a/codeframe/cli/app.py b/codeframe/cli/app.py index 45c6c4a9..7156181c 100644 --- a/codeframe/cli/app.py +++ b/codeframe/cli/app.py @@ -2128,6 +2128,11 @@ def tasks_generate( "[bold]cf tasks generate --no-llm[/bold] to extract bullets directly." ) raise typer.Exit(1) + except typer.Exit: + # A deliberate exit is not an error. typer.Exit subclasses + # RuntimeError, so without this the catch-all below prints + # its exit code as a second message: "Error: 1" (#1113). + raise except Exception as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) @@ -5823,6 +5828,11 @@ def schedule_show( except FileNotFoundError as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) + except typer.Exit: + # A deliberate exit is not an error. typer.Exit subclasses + # RuntimeError, so without this the catch-all below prints + # its exit code as a second message: "Error: 1" (#1113). + raise except Exception as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) @@ -5916,6 +5926,11 @@ def schedule_predict( except FileNotFoundError as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) + except typer.Exit: + # A deliberate exit is not an error. typer.Exit subclasses + # RuntimeError, so without this the catch-all below prints + # its exit code as a second message: "Error: 1" (#1113). + raise except Exception as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) @@ -5993,6 +6008,11 @@ def schedule_bottlenecks( except FileNotFoundError as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) + except typer.Exit: + # A deliberate exit is not an error. typer.Exit subclasses + # RuntimeError, so without this the catch-all below prints + # its exit code as a second message: "Error: 1" (#1113). + raise except Exception as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) @@ -6152,6 +6172,11 @@ def templates_apply( except ValueError as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) + except typer.Exit: + # A deliberate exit is not an error. typer.Exit subclasses + # RuntimeError, so without this the catch-all below prints + # its exit code as a second message: "Error: 1" (#1113). + raise except Exception as e: console.print(f"[red]Error:[/red] {e}") raise typer.Exit(1) diff --git a/tests/cli/test_typer_exit_not_swallowed_1113.py b/tests/cli/test_typer_exit_not_swallowed_1113.py new file mode 100644 index 00000000..93bcdd5e --- /dev/null +++ b/tests/cli/test_typer_exit_not_swallowed_1113.py @@ -0,0 +1,155 @@ +"""#1113 — a deliberate `typer.Exit` was being re-printed as "Error: 1". + +`typer.Exit` subclasses `RuntimeError`, so a command's own catch-all caught its +own intentional exit and stringified the exit *code* as a message: + + Error: No PRD found. + Add one first: codeframe prd add + Error: 1 + +To a new user that reads as a second, unexplained failure right after a message +that was otherwise clear. + +The per-command fix is one `except typer.Exit: raise`. The scanner below is the +part that matters long-term: it fails for any *new* command that reintroduces +the pattern, which is what the issue asked for in preference to spot fixes. +""" + +import ast +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from codeframe.cli.app import app + +pytestmark = pytest.mark.v2 + +runner = CliRunner() +APP_SOURCE = Path(__file__).resolve().parents[2] / "codeframe" / "cli" / "app.py" + +_BROAD = {"Exception", "BaseException"} + + +def _raises_typer_exit(node: ast.AST) -> bool: + for n in ast.walk(node): + if isinstance(n, ast.Raise) and n.exc is not None: + call = n.exc + if isinstance(call, ast.Call): + call = call.func + if isinstance(call, ast.Attribute) and call.attr == "Exit": + return True + return False + + +def _handler_names(handler: ast.ExceptHandler) -> list[ast.expr]: + if handler.type is None: + return [] + if isinstance(handler.type, ast.Tuple): + return list(handler.type.elts) + return [handler.type] + + +def find_unguarded_exits() -> list[tuple[str, int]]: + """Every `try` that raises typer.Exit and then catches it with a broad handler.""" + tree = ast.parse(APP_SOURCE.read_text()) + offenders: list[tuple[str, int]] = [] + + for fn in ast.walk(tree): + if not isinstance(fn, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + for node in ast.walk(fn): + if not isinstance(node, ast.Try): + continue + # Only the try *body* matters: a raise inside a handler propagates + # out of the statement rather than into a sibling handler. + if not any(_raises_typer_exit(stmt) for stmt in node.body): + continue + + guarded = any( + isinstance(name, ast.Attribute) and name.attr == "Exit" + for handler in node.handlers + for name in _handler_names(handler) + ) + if guarded: + continue + + broad = [ + h + for h in node.handlers + if h.type is None + or (isinstance(h.type, ast.Name) and h.type.id in _BROAD) + ] + if broad: + offenders.append((fn.name, broad[0].lineno)) + + return offenders + + +class TestTheScannerIsTheRule: + def test_no_command_swallows_its_own_exit(self): + offenders = find_unguarded_exits() + assert offenders == [], ( + "these raise typer.Exit inside a try whose broad handler will catch " + "it and print the exit code as 'Error: '. Add " + "`except typer.Exit: raise` above the catch-all:\n" + + "\n".join(f" {name} (app.py:{line})" for name, line in offenders) + ) + + def test_the_scanner_actually_detects_the_pattern(self): + """A scanner that cannot fail is worse than no scanner.""" + source = ( + "import typer\n" + "def cmd():\n" + " try:\n" + " raise typer.Exit(1)\n" + " except Exception as e:\n" + " print(e)\n" + ) + tree = ast.parse(source) + fn = tree.body[1] + try_node = fn.body[0] + assert _raises_typer_exit(try_node.body[0]) + assert not any( + isinstance(name, ast.Attribute) and name.attr == "Exit" + for handler in try_node.handlers + for name in _handler_names(handler) + ) + + +class TestTheUserVisibleOutput: + """AC: `cf tasks generate` with no PRD prints its message and nothing else.""" + + def _init(self, tmp_path): + result = runner.invoke(app, ["init", str(tmp_path)]) + assert result.exit_code == 0, result.output + + def test_tasks_generate_with_no_prd(self, tmp_path): + self._init(tmp_path) + result = runner.invoke(app, ["tasks", "generate", "-w", str(tmp_path)]) + + assert result.exit_code == 1, "the exit code must still signal failure" + assert "Error: 1" not in result.output + assert "No PRD found" in result.output + + def test_templates_apply_with_no_prd(self, tmp_path): + self._init(tmp_path) + result = runner.invoke(app, ["templates", "apply", "standard", "-w", str(tmp_path)]) + + assert result.exit_code == 1 + assert "Error: 1" not in result.output + + def test_no_command_prints_a_bare_numeric_error(self, tmp_path): + """A stringified exit code has no business in any message.""" + self._init(tmp_path) + for argv in ( + ["tasks", "generate", "-w", str(tmp_path)], + ["templates", "apply", "standard", "-w", str(tmp_path)], + ["prd", "show", "-w", str(tmp_path)], + ["tasks", "show", "deadbeef", "-w", str(tmp_path)], + ): + result = runner.invoke(app, argv) + for code in range(1, 5): + assert f"Error: {code}" not in result.output, ( + f"`cf {' '.join(argv)}` printed its exit code as a message" + )