Skip to content

Commit 551dd1e

Browse files
doquanghuyclaude
andcommitted
fix(workflow): only surface gate detail in --json when the run is paused
Address review (github#2965): _gate_outcome() emitted a gate block whenever current_step_id pointed at a gate step. Since RunState.current_step_id is never cleared on completion, a completed/failed run whose last step was a gate leaked stale gate detail in run/resume/status --json. Guard on status == paused. Also assert CLI success in the _run_json test helper before JSON-parsing, and add direct coverage for the suppression guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 304e336 commit 551dd1e

2 files changed

Lines changed: 33 additions & 0 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2768,6 +2768,12 @@ def _gate_outcome(state: Any) -> dict[str, Any] | None:
27682768
and (after an interactive choice) the decision lets orchestrators
27692769
drive review gates without parsing the human-facing stream.
27702770
"""
2771+
# Only a run that is actually *paused* sits at a gate awaiting a
2772+
# decision. RunState.current_step_id is not cleared on completion, so
2773+
# without this guard a completed/failed run whose last executed step was
2774+
# a gate would surface stale gate details (in run/resume/status --json).
2775+
if getattr(state.status, "value", state.status) != "paused":
2776+
return None
27712777
step = (getattr(state, "step_results", None) or {}).get(state.current_step_id)
27722778
if not isinstance(step, dict) or step.get("type") != "gate":
27732779
return None

tests/test_workflows.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3984,6 +3984,9 @@ def _run_json(self, tmp_path, monkeypatch, content):
39843984
monkeypatch.chdir(tmp_path)
39853985
runner = CliRunner()
39863986
result = runner.invoke(app, ["workflow", "run", str(path), "--json"])
3987+
# Assert the CLI succeeded before parsing so a real failure surfaces
3988+
# the actual output instead of an opaque JSON decode error.
3989+
assert result.exit_code == 0, result.stdout
39873990
return _json.loads(result.stdout)
39883991

39893992
def test_gate_pause_carries_gate_block(self, tmp_path, monkeypatch):
@@ -4001,3 +4004,27 @@ def test_completed_run_has_no_gate_block(self, tmp_path, monkeypatch):
40014004
payload = self._run_json(tmp_path, monkeypatch, self._WF_PLAIN)
40024005
assert payload["status"] == "completed"
40034006
assert "gate" not in payload
4007+
4008+
def test_gate_block_suppressed_when_run_not_paused(self):
4009+
# RunState.current_step_id is not cleared on completion, so a
4010+
# completed/failed run whose last executed step was a gate still
4011+
# points current_step_id at that gate. The gate block must only be
4012+
# emitted while the run is actually paused at it.
4013+
from types import SimpleNamespace
4014+
from specify_cli import _gate_outcome
4015+
4016+
gate_step = {
4017+
"type": "gate",
4018+
"output": {"message": "m", "options": ["approve"], "choice": "approve"},
4019+
}
4020+
4021+
def _state(status):
4022+
return SimpleNamespace(
4023+
status=SimpleNamespace(value=status),
4024+
current_step_id="review",
4025+
step_results={"review": gate_step},
4026+
)
4027+
4028+
assert _gate_outcome(_state("completed")) is None
4029+
assert _gate_outcome(_state("failed")) is None
4030+
assert _gate_outcome(_state("paused")) is not None

0 commit comments

Comments
 (0)