From e0cae3130174e6def2781b1a336ccb618ce75d13 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Tue, 14 Jul 2026 22:07:37 +0500 Subject: [PATCH 1/2] fix(workflows): route 'workflow status --json' errors to stderr The workflow_status run_id error paths (FileNotFoundError -> 'Run not found', ValueError -> invalid run) used the stdout console and fired before the json_output branch, so 'specify workflow status --json' wrote a Rich-rendered error to stdout and corrupted the JSON stream a consumer would json.loads(). Route both through _error_console(json_output) so they go to stderr under --json, matching the sibling 'workflow run'/'workflow resume' commands (which use the identical RunState.load try/except) and the documented stdout-purity contract. Test asserts the not-found error appears on stderr and stdout stays empty under --json (fails before: the error was on stdout). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/workflows/_commands.py | 8 ++++++-- tests/test_workflows.py | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index 9ab199c023..c01903322b 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -1262,14 +1262,18 @@ def workflow_status( engine = WorkflowEngine(project_root) if run_id: + # Route errors to stderr under --json so the stdout JSON stream stays + # parseable (mirrors `workflow run`/`workflow resume`); both handlers + # fire before the json_output branch below. + err = _error_console(json_output) try: from .engine import RunState state = RunState.load(run_id, project_root) except FileNotFoundError: - console.print(f"[red]Error:[/red] Run not found: {run_id}") + err.print(f"[red]Error:[/red] Run not found: {run_id}") raise typer.Exit(1) except ValueError as exc: - console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") + err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") raise typer.Exit(1) if json_output: diff --git a/tests/test_workflows.py b/tests/test_workflows.py index a766283119..1178eb20b5 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -12520,6 +12520,29 @@ def test_status_run_not_found_unchanged(self, project_dir, monkeypatch): assert result.exit_code != 0 assert "Run not found: nonexistent-run" in result.output + def test_status_json_not_found_error_goes_to_stderr( + self, project_dir, monkeypatch, capsys + ): + """Under --json, the not-found/invalid-run error must go to stderr so the + stdout JSON stream stays parseable (empty on the error path) — mirroring + `workflow run`/`workflow resume`. Before this fix both handlers used the + stdout console, corrupting a consumer's json.loads(stdout).""" + import typer + from specify_cli.workflows import _commands + + (project_dir / ".specify" / "workflows").mkdir(parents=True, exist_ok=True) + monkeypatch.setattr( + _commands, "_require_specify_project", lambda: project_dir + ) + with pytest.raises(typer.Exit) as exc: + _commands.workflow_status("does-not-exist", json_output=True) + assert exc.value.exit_code == 1 + captured = capsys.readouterr() + assert "Run not found" in captured.err + assert "Run not found" not in captured.out + # stdout carries no partial/corrupt JSON on the error path. + assert captured.out.strip() == "" + def test_status_no_run_id_list_path_unaffected(self, project_dir, monkeypatch): """The no-run-id list-all-runs path must remain unaffected by the new single-run ValueError boundary.""" From de544bc25e2b48fa74167a7a8372bc2009629376 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Wed, 15 Jul 2026 10:34:16 +0500 Subject: [PATCH 2/2] test(workflows): cover the ValueError handler in workflow status --json purity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stderr-routing fix reroutes both the FileNotFoundError and ValueError run_id handlers, but the test only exercised FileNotFoundError — a regression of the ValueError path back to stdout would have gone uncaught. Add a ValueError case (RunState.load raising) asserting the same stderr-only / empty-stdout behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_workflows.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 1178eb20b5..b98a6c4f71 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -12543,6 +12543,35 @@ def test_status_json_not_found_error_goes_to_stderr( # stdout carries no partial/corrupt JSON on the error path. assert captured.out.strip() == "" + def test_status_json_invalid_run_error_goes_to_stderr( + self, project_dir, monkeypatch, capsys + ): + """The ValueError handler (a malformed/invalid run state) must ALSO route + to stderr under --json, not just the FileNotFoundError one — otherwise a + regression there would silently corrupt the JSON stream and this suite + wouldn't catch it.""" + import typer + from specify_cli.workflows import _commands + from specify_cli.workflows.engine import RunState + + (project_dir / ".specify" / "workflows").mkdir(parents=True, exist_ok=True) + monkeypatch.setattr( + _commands, "_require_specify_project", lambda: project_dir + ) + + def _raise_value_error(*args, **kwargs): + raise ValueError("corrupt run state: bad status") + + monkeypatch.setattr(RunState, "load", _raise_value_error) + + with pytest.raises(typer.Exit) as exc: + _commands.workflow_status("some-run", json_output=True) + assert exc.value.exit_code == 1 + captured = capsys.readouterr() + assert "corrupt run state" in captured.err + assert "corrupt run state" not in captured.out + assert captured.out.strip() == "" + def test_status_no_run_id_list_path_unaffected(self, project_dir, monkeypatch): """The no-run-id list-all-runs path must remain unaffected by the new single-run ValueError boundary."""