mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
fix(workflows): handle an unreadable run state in workflow status (#3999)
`workflow status <run_id>` and `workflow resume <run_id>` both call `RunState.load()`, and a prior fix aligned them on the FileNotFoundError and ValueError boundaries. `resume` also handles OSError; `status` never gained that handler. So an unreadable `state.json` -- wrong permissions, an I/O error, or a directory sitting where the file belongs -- escapes as a raw traceback with no output at all, while `resume` on the same run prints a clean `Error:` line and exits 1. `state_path.exists()` is True for a directory, so the existing guard passes and `open()` raises. Add the missing `except OSError` next to its siblings, using the same `_escape_markup` + `typer.Exit(1)` shape, and routing through `err` so the message lands on stderr under `--json` and the stdout JSON stream stays parseable. Two regression tests: the end-to-end CLI path (a directory in place of state.json) and the `--json` stderr-routing path. Both fail without the source change. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
40037b1aca
commit
204d94fdb1
@@ -1594,6 +1594,12 @@ def workflow_status(
|
||||
except ValueError as exc:
|
||||
err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}")
|
||||
raise typer.Exit(1)
|
||||
except OSError as exc:
|
||||
# An unreadable state.json (bad permissions, a directory in its
|
||||
# place, I/O error) must fail as cleanly as the malformed-JSON
|
||||
# case above -- `workflow resume` already handles OSError here.
|
||||
err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}")
|
||||
raise typer.Exit(1)
|
||||
|
||||
if json_output:
|
||||
# Build on the shared run/resume payload so the common fields
|
||||
|
||||
@@ -16705,6 +16705,57 @@ steps:
|
||||
assert "corrupt run state" not in captured.out
|
||||
assert captured.out.strip() == ""
|
||||
|
||||
def test_status_unreadable_run_state_exits_cleanly(
|
||||
self, project_dir, monkeypatch
|
||||
):
|
||||
"""`workflow status <run_id>` gained a ValueError boundary to match
|
||||
`workflow resume`, but not resume's OSError one -- so an unreadable
|
||||
state.json (bad permissions, a directory in its place, an I/O error)
|
||||
still leaked a raw traceback. exists() is True for a directory, so
|
||||
the guard passes and open() raises OSError."""
|
||||
from typer.testing import CliRunner
|
||||
from specify_cli import app
|
||||
|
||||
monkeypatch.chdir(project_dir)
|
||||
runs_dir = project_dir / ".specify" / "workflows" / "runs" / "abc123"
|
||||
runs_dir.mkdir(parents=True, exist_ok=True)
|
||||
# A directory where state.json should be: exists() passes, open() fails.
|
||||
(runs_dir / "state.json").mkdir(exist_ok=True)
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(app, ["workflow", "status", "abc123"])
|
||||
assert result.exit_code != 0
|
||||
assert result.exception is None or isinstance(result.exception, SystemExit)
|
||||
assert "Error" in result.output
|
||||
|
||||
def test_status_json_unreadable_run_state_error_goes_to_stderr(
|
||||
self, project_dir, monkeypatch, capsys
|
||||
):
|
||||
"""The OSError handler must route to stderr under --json too, so the
|
||||
stdout JSON stream stays parseable -- mirroring the sibling
|
||||
FileNotFoundError/ValueError handlers."""
|
||||
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_os_error(*args, **kwargs):
|
||||
raise PermissionError(13, "Permission denied")
|
||||
|
||||
monkeypatch.setattr(RunState, "load", _raise_os_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 "Permission denied" in captured.err
|
||||
assert "Permission denied" 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