Skip to content

Commit 1ee2b62

Browse files
doquanghuyclaude
andauthored
fix: non-zero exit code when a workflow run ends failed or aborted (#2959)
* 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 * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 811a3aa commit 1ee2b62

3 files changed

Lines changed: 111 additions & 3 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2100,6 +2100,16 @@ def _workflow_run_payload(state: Any) -> dict[str, Any]:
21002100
}
21012101

21022102

2103+
def _run_outcome_exit_code(status_value: str) -> int:
2104+
"""Exit code for a finished run/resume: non-zero on terminal failure.
2105+
2106+
``failed`` and ``aborted`` map to 1 so scripts and orchestrators can
2107+
rely on the process exit code; ``completed`` and ``paused`` map to 0
2108+
(paused is a legitimate waiting state, not a failure).
2109+
"""
2110+
return 1 if status_value in ("failed", "aborted") else 0
2111+
2112+
21032113
def _emit_workflow_json(payload: dict[str, Any]) -> None:
21042114
"""Write a workflow payload as machine-readable JSON to stdout.
21052115
@@ -2214,7 +2224,7 @@ def workflow_run(
22142224

22152225
if json_output:
22162226
_emit_workflow_json(_workflow_run_payload(state))
2217-
return
2227+
raise typer.Exit(_run_outcome_exit_code(state.status.value))
22182228

22192229
status_colors = {
22202230
"completed": "green",
@@ -2229,6 +2239,8 @@ def workflow_run(
22292239
if state.status.value == "paused":
22302240
console.print(f"\nResume with: [cyan]specify workflow resume {state.run_id}[/cyan]")
22312241

2242+
raise typer.Exit(_run_outcome_exit_code(state.status.value))
2243+
22322244

22332245
@workflow_app.command("resume")
22342246
def workflow_resume(
@@ -2269,7 +2281,7 @@ def workflow_resume(
22692281

22702282
if json_output:
22712283
_emit_workflow_json(_workflow_run_payload(state))
2272-
return
2284+
raise typer.Exit(_run_outcome_exit_code(state.status.value))
22732285

22742286
status_colors = {
22752287
"completed": "green",
@@ -2280,6 +2292,8 @@ def workflow_resume(
22802292
color = status_colors.get(state.status.value, "white")
22812293
console.print(f"\n[{color}]Status: {state.status.value}[/{color}]")
22822294

2295+
raise typer.Exit(_run_outcome_exit_code(state.status.value))
2296+
22832297

22842298
@workflow_app.command("status")
22852299
def workflow_status(

tests/test_workflow_run_without_project.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ def test_workflow_run_failing_yaml_without_project(self, tmp_path):
162162
], catch_exceptions=False)
163163
finally:
164164
os.chdir(old_cwd)
165-
assert result.exit_code == 0, f"workflow run failed unexpectedly: {result.output}"
165+
# A failed workflow now maps to a non-zero process exit code so
166+
# scripts and CI can rely on $? (the CLI itself still ran fine).
167+
assert result.exit_code == 1, f"expected exit 1 on failed run: {result.output}"
166168
assert "Status: failed" in result.output
167169

168170
def test_workflow_run_yaml_rejects_symlinked_specify_dir(self, tmp_path):

tests/test_workflows.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4964,3 +4964,95 @@ def fake_open_url(url, timeout=None, extra_headers=None):
49644964
asset_calls = [(url, h) for url, h in captured_urls if "releases/assets/" in url]
49654965
assert len(asset_calls) >= 1
49664966
assert asset_calls[0][1] == {"Accept": "application/octet-stream"}
4967+
4968+
4969+
class TestWorkflowRunExitCodes:
4970+
"""CLI-level tests for the run/resume process exit codes."""
4971+
4972+
_WF_OK = """
4973+
schema_version: "1.0"
4974+
workflow:
4975+
id: "exit-ok"
4976+
name: "Exit OK"
4977+
version: "1.0.0"
4978+
steps:
4979+
- id: fine
4980+
type: shell
4981+
run: "exit 0"
4982+
"""
4983+
4984+
_WF_FAIL = """
4985+
schema_version: "1.0"
4986+
workflow:
4987+
id: "exit-fail"
4988+
name: "Exit Fail"
4989+
version: "1.0.0"
4990+
steps:
4991+
- id: boom
4992+
type: shell
4993+
run: "exit 1"
4994+
"""
4995+
4996+
def _write(self, tmp_path, content):
4997+
path = tmp_path / "wf.yml"
4998+
path.write_text(content, encoding="utf-8")
4999+
return path
5000+
5001+
def test_run_completed_exits_zero(self, tmp_path, monkeypatch):
5002+
from typer.testing import CliRunner
5003+
from specify_cli import app
5004+
5005+
monkeypatch.chdir(tmp_path)
5006+
runner = CliRunner()
5007+
result = runner.invoke(app, ["workflow", "run", str(self._write(tmp_path, self._WF_OK))])
5008+
assert result.exit_code == 0
5009+
assert "Status: completed" in result.stdout
5010+
5011+
def test_run_failed_exits_nonzero(self, tmp_path, monkeypatch):
5012+
from typer.testing import CliRunner
5013+
from specify_cli import app
5014+
5015+
monkeypatch.chdir(tmp_path)
5016+
runner = CliRunner()
5017+
result = runner.invoke(app, ["workflow", "run", str(self._write(tmp_path, self._WF_FAIL))])
5018+
assert "Status: failed" in result.stdout
5019+
assert result.exit_code == 1
5020+
5021+
def test_run_failed_exits_nonzero_with_json(self, tmp_path, monkeypatch):
5022+
import json as _json
5023+
from typer.testing import CliRunner
5024+
from specify_cli import app
5025+
5026+
monkeypatch.chdir(tmp_path)
5027+
runner = CliRunner()
5028+
result = runner.invoke(
5029+
app,
5030+
["workflow", "run", str(self._write(tmp_path, self._WF_FAIL)), "--json"],
5031+
)
5032+
assert result.exit_code == 1, result.stdout
5033+
payload = _json.loads(result.stdout)
5034+
assert payload["status"] == "failed"
5035+
5036+
def test_resume_failed_run_exits_nonzero(self, tmp_path, monkeypatch):
5037+
# End-to-end coverage for the `workflow resume` exit-code mapping:
5038+
# resuming a run whose outcome is still `failed` must exit non-zero,
5039+
# mirroring `workflow run`. Resume re-executes the failed step, which
5040+
# fails again, so the resumed outcome stays `failed`.
5041+
import json as _json
5042+
from typer.testing import CliRunner
5043+
from specify_cli import app
5044+
5045+
monkeypatch.chdir(tmp_path)
5046+
(tmp_path / ".specify").mkdir() # `workflow resume` requires a project
5047+
runner = CliRunner()
5048+
run = runner.invoke(
5049+
app,
5050+
["workflow", "run", str(self._write(tmp_path, self._WF_FAIL)), "--json"],
5051+
)
5052+
assert run.exit_code == 1, run.stdout
5053+
run_id = _json.loads(run.stdout)["run_id"]
5054+
5055+
resumed = runner.invoke(app, ["workflow", "resume", run_id, "--json"])
5056+
assert resumed.exit_code == 1, resumed.stdout
5057+
payload = _json.loads(resumed.stdout)
5058+
assert payload["status"] == "failed"

0 commit comments

Comments
 (0)