From df51dea8302051ca68802e3b65d781fbc230d3db Mon Sep 17 00:00:00 2001 From: doquanghuy Date: Sat, 13 Jun 2026 00:22:59 +0700 Subject: [PATCH 1/2] fix: non-zero exit code when a workflow run ends failed or aborted workflow run and workflow resume printed Status: failed (or emitted the --json payload) and exited 0, so scripts and CI could not rely on the process exit code. Map terminal outcomes: failed|aborted -> 1, completed|paused -> 0, on both the text and --json paths. The previous exit-0-on-failed behavior was pinned by test_workflow_run_failing_yaml_without_project; the pin is updated to the new contract. Fixes #2958 --- src/specify_cli/__init__.py | 18 +++++- tests/test_workflow_run_without_project.py | 4 +- tests/test_workflows.py | 68 ++++++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 186593000c..ef9b51b898 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2756,6 +2756,16 @@ def _workflow_run_payload(state: Any) -> dict[str, Any]: } +def _run_outcome_exit_code(status_value: str) -> int: + """Exit code for a finished run/resume: non-zero on terminal failure. + + ``failed`` and ``aborted`` map to 1 so scripts and orchestrators can + rely on the process exit code; ``completed`` and ``paused`` map to 0 + (paused is a legitimate waiting state, not a failure). + """ + return 1 if status_value in ("failed", "aborted") else 0 + + def _emit_workflow_json(payload: dict[str, Any]) -> None: """Write a workflow payload as machine-readable JSON to stdout. @@ -2868,7 +2878,7 @@ def workflow_run( if json_output: _emit_workflow_json(_workflow_run_payload(state)) - return + raise typer.Exit(_run_outcome_exit_code(state.status.value)) status_colors = { "completed": "green", @@ -2883,6 +2893,8 @@ def workflow_run( if state.status.value == "paused": console.print(f"\nResume with: [cyan]specify workflow resume {state.run_id}[/cyan]") + raise typer.Exit(_run_outcome_exit_code(state.status.value)) + @workflow_app.command("resume") def workflow_resume( @@ -2921,7 +2933,7 @@ def workflow_resume( if json_output: _emit_workflow_json(_workflow_run_payload(state)) - return + raise typer.Exit(_run_outcome_exit_code(state.status.value)) status_colors = { "completed": "green", @@ -2932,6 +2944,8 @@ def workflow_resume( color = status_colors.get(state.status.value, "white") console.print(f"\n[{color}]Status: {state.status.value}[/{color}]") + raise typer.Exit(_run_outcome_exit_code(state.status.value)) + @workflow_app.command("status") def workflow_status( diff --git a/tests/test_workflow_run_without_project.py b/tests/test_workflow_run_without_project.py index 05235493fa..8ba0c7eaa9 100644 --- a/tests/test_workflow_run_without_project.py +++ b/tests/test_workflow_run_without_project.py @@ -162,7 +162,9 @@ def test_workflow_run_failing_yaml_without_project(self, tmp_path): ], catch_exceptions=False) finally: os.chdir(old_cwd) - assert result.exit_code == 0, f"workflow run failed unexpectedly: {result.output}" + # A failed workflow now maps to a non-zero process exit code so + # scripts and CI can rely on $? (the CLI itself still ran fine). + assert result.exit_code == 1, f"expected exit 1 on failed run: {result.output}" assert "Status: failed" in result.output def test_workflow_run_yaml_rejects_symlinked_specify_dir(self, tmp_path): diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 51da5cc86b..22a24d1b64 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3944,3 +3944,71 @@ def fake_open_url(url, timeout=None, extra_headers=None): asset_calls = [(url, h) for url, h in captured_urls if "releases/assets/" in url] assert len(asset_calls) >= 1 assert asset_calls[0][1] == {"Accept": "application/octet-stream"} + + +class TestWorkflowRunExitCodes: + """CLI-level tests for the run/resume process exit codes.""" + + _WF_OK = """ +schema_version: "1.0" +workflow: + id: "exit-ok" + name: "Exit OK" + version: "1.0.0" +steps: + - id: fine + type: shell + run: "true" +""" + + _WF_FAIL = """ +schema_version: "1.0" +workflow: + id: "exit-fail" + name: "Exit Fail" + version: "1.0.0" +steps: + - id: boom + type: shell + run: "false" +""" + + def _write(self, tmp_path, content): + path = tmp_path / "wf.yml" + path.write_text(content, encoding="utf-8") + return path + + def test_run_completed_exits_zero(self, tmp_path, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(tmp_path) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "run", str(self._write(tmp_path, self._WF_OK))]) + assert result.exit_code == 0 + assert "Status: completed" in result.stdout + + def test_run_failed_exits_nonzero(self, tmp_path, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(tmp_path) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "run", str(self._write(tmp_path, self._WF_FAIL))]) + assert "Status: failed" in result.stdout + assert result.exit_code == 1 + + def test_run_failed_exits_nonzero_with_json(self, tmp_path, monkeypatch): + import json as _json + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(tmp_path) + runner = CliRunner() + result = runner.invoke( + app, + ["workflow", "run", str(self._write(tmp_path, self._WF_FAIL)), "--json"], + ) + payload = _json.loads(result.stdout) + assert payload["status"] == "failed" + assert result.exit_code == 1 From 40e9bb77fb159a4eecce999e1c4e29a7131ccf89 Mon Sep 17 00:00:00 2001 From: doquanghuy Date: Wed, 17 Jun 2026 09:02:48 +0700 Subject: [PATCH 2/2] test: portable exit-code step commands + cover resume failed->exit-1 Address review (#2959): replace non-portable run: 'true'/'false' with 'exit 0'/'exit 1' (Windows cmd.exe has no true/false builtins under shell=True), and add an end-to-end 'workflow resume' test asserting a resumed failed run exits non-zero. Co-Authored-By: Claude Fable 5 --- tests/test_workflows.py | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 22a24d1b64..4bb1e2bd32 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -3958,7 +3958,7 @@ class TestWorkflowRunExitCodes: steps: - id: fine type: shell - run: "true" + run: "exit 0" """ _WF_FAIL = """ @@ -3970,7 +3970,7 @@ class TestWorkflowRunExitCodes: steps: - id: boom type: shell - run: "false" + run: "exit 1" """ def _write(self, tmp_path, content): @@ -4009,6 +4009,30 @@ def test_run_failed_exits_nonzero_with_json(self, tmp_path, monkeypatch): app, ["workflow", "run", str(self._write(tmp_path, self._WF_FAIL)), "--json"], ) + assert result.exit_code == 1, result.stdout payload = _json.loads(result.stdout) assert payload["status"] == "failed" - assert result.exit_code == 1 + + def test_resume_failed_run_exits_nonzero(self, tmp_path, monkeypatch): + # End-to-end coverage for the `workflow resume` exit-code mapping: + # resuming a run whose outcome is still `failed` must exit non-zero, + # mirroring `workflow run`. Resume re-executes the failed step, which + # fails again, so the resumed outcome stays `failed`. + import json as _json + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(tmp_path) + (tmp_path / ".specify").mkdir() # `workflow resume` requires a project + runner = CliRunner() + run = runner.invoke( + app, + ["workflow", "run", str(self._write(tmp_path, self._WF_FAIL)), "--json"], + ) + assert run.exit_code == 1, run.stdout + run_id = _json.loads(run.stdout)["run_id"] + + resumed = runner.invoke(app, ["workflow", "resume", run_id, "--json"]) + assert resumed.exit_code == 1, resumed.stdout + payload = _json.loads(resumed.stdout) + assert payload["status"] == "failed"