Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion src/specify_cli/workflows/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,22 @@ def load(cls, run_id: str, project_root: Path) -> RunState:
installed_origin_tracked=has_installed_workflow_id,
)
state.status = RunStatus(state_data["status"])
state.current_step_index = state_data.get("current_step_index", 0)

# Validate the index shape before restoring it. The upper bound cannot
# be checked until resume() loads the workflow definition and is handled
# there. Reject bool explicitly because it subclasses int; otherwise a
# malformed value could fail during slicing or resume from the wrong step.
current_step_index = state_data.get("current_step_index", 0)
if (
isinstance(current_step_index, bool)
or not isinstance(current_step_index, int)
or current_step_index < 0
):
Comment thread
Copilot marked this conversation as resolved.
raise ValueError(
"Invalid run state: 'current_step_index' must be a "
f"non-negative integer, got {current_step_index!r}"
)
state.current_step_index = current_step_index
state.current_step_id = state_data.get("current_step_id")
state.step_results = state_data.get("step_results", {})
state.workflow_dir = state_data.get("workflow_dir")
Expand Down Expand Up @@ -1095,6 +1110,21 @@ def resume(
else:
definition = self.load_workflow(state.workflow_id)

# RunState.load() rejects a non-int/negative current_step_index but
# can't check the upper bound — the step count isn't known until the
# workflow definition is loaded, above. An out-of-range positive
# index (e.g. a hand-edited state.json) would otherwise slice
# definition.steps[state.current_step_index:] into an empty list
# below, silently completing the run without executing any step.
if state.current_step_index >= len(definition.steps):
msg = (
"Invalid run state: 'current_step_index' "
f"({state.current_step_index}) is out of range for "
f"workflow {state.workflow_id!r} with {len(definition.steps)} "
"step(s)."
)
raise ValueError(msg)

dispatch_default_errors = _dispatch_default_errors(definition)
if dispatch_default_errors:
raise ValueError(" ".join(dispatch_default_errors))
Expand Down
68 changes: 68 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -7376,6 +7376,40 @@ def test_load_rejects_stored_run_id_mismatch(self, project_dir):
):
RunState.load("requested-run", project_dir)

@pytest.mark.parametrize(
"bad_current_step_index",
["not-a-number", 1.5, -1, [0], {"index": 0}, True],
)
def test_load_rejects_invalid_current_step_index(
self, project_dir, bad_current_step_index
):
"""Reject non-integer and negative resume indices at load time.

``bool`` is covered explicitly because it subclasses ``int``.
"""
from specify_cli.workflows.engine import RunState

run_dir = (
project_dir / ".specify" / "workflows" / "runs" / "bad-index-run"
)
run_dir.mkdir(parents=True)
(run_dir / "state.json").write_text(
json.dumps(
{
"run_id": "bad-index-run",
"workflow_id": "test-workflow",
"status": "paused",
"current_step_index": bad_current_step_index,
}
),
encoding="utf-8",
)

with pytest.raises(
ValueError, match="'current_step_index' must be a non-negative integer"
):
RunState.load("bad-index-run", project_dir)

@pytest.mark.parametrize(
("installed_workflow_id", "installed_registry_root"),
[
Expand Down Expand Up @@ -16678,6 +16712,40 @@ def test_resume_preload_rejects_malformed_state_cleanly(
assert result.exception is None or isinstance(result.exception, SystemExit)
assert "Invalid run state" in result.output

def test_resume_rejects_out_of_range_current_step_index(
self, project_dir, monkeypatch
):
"""An out-of-range positive index must fail cleanly, not silently
complete the run with no steps executed.

``resume()`` slices ``definition.steps[state.current_step_index:]``;
for any index >= len(steps) that slice is an empty list, so the run
would otherwise finish with status "completed" having executed
nothing.
"""
from typer.testing import CliRunner
from specify_cli import app

monkeypatch.chdir(project_dir)
runner = CliRunner()
run_id = self._install_and_run_gated(runner, app, project_dir)
state_path = (
project_dir / ".specify" / "workflows" / "runs" / run_id / "state.json"
)
data = json.loads(state_path.read_text(encoding="utf-8"))
data["current_step_index"] = 5
state_path.write_text(json.dumps(data), encoding="utf-8")

result = runner.invoke(app, ["workflow", "resume", run_id])

assert result.exit_code != 0
assert result.exception is None or isinstance(result.exception, SystemExit)
assert "Invalid run state" in result.output
assert "out of range" in result.output

reloaded = json.loads(state_path.read_text(encoding="utf-8"))
assert reloaded["status"] == "paused"

def test_resume_legacy_run_respects_current_disabled_state(
self, project_dir, monkeypatch
):
Expand Down