mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(workflows): route 'workflow status --json' errors to stderr (#3520)
* 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 <bad-id> --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) <noreply@anthropic.com> * test(workflows): cover the ValueError handler in workflow status --json purity 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -13000,6 +13000,58 @@ steps:
|
||||
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_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."""
|
||||
|
||||
Reference in New Issue
Block a user