Skip to content

Commit be970e7

Browse files
doquanghuyclaude
andcommitted
fix(workflows): normalize gate choice to str; portable plain-gate test
Address Copilot review: - `_gate_outcome` normalized `message` and `options` but passed `choice` through as-is; an unvalidated gate can record a non-string `choice`, which contradicts the stable-schema rationale. Coerce `choice` to `str | None` (None still means "no decision yet"), consistent with the other two fields. Adds a focused choice-coercion test. - The plain (no-gate) test workflow used `run: "true"`, which fails under cmd.exe on Windows (ShellStep uses shell=True). Use the cross-platform `run: "exit 0"` (matching the exit-code suite's workflows). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 27a7869 commit be970e7

2 files changed

Lines changed: 31 additions & 5 deletions

File tree

‎src/specify_cli/__init__.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2143,15 +2143,17 @@ def _gate_outcome(state: Any) -> dict[str, Any] | None:
21432143
if not isinstance(step, dict) or not _is_gate_step(step):
21442144
return None
21452145
output = step.get("output") or {}
2146-
# `message` and `options` may be non-string YAML literals in an unvalidated
2147-
# workflow (GateStep coerces neither for the payload), so normalise both
2148-
# here for a stable JSON schema: message → str, options → list[str] | None.
2146+
# `message`, `options`, and `choice` may be non-string YAML literals in an
2147+
# unvalidated workflow (GateStep coerces none of them for the payload), so
2148+
# normalise all three for a stable JSON schema: message → str, options →
2149+
# list[str] | None, choice → str | None (None means no decision yet).
21492150
message = output.get("message")
2151+
choice = output.get("choice")
21502152
return {
21512153
"step_id": state.current_step_id,
21522154
"message": None if message is None else str(message),
21532155
"options": _normalize_gate_options(output.get("options")),
2154-
"choice": output.get("choice"),
2156+
"choice": None if choice is None else str(choice),
21552157
}
21562158

21572159

‎tests/test_workflows.py‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5368,7 +5368,7 @@ class TestWorkflowRunGateOutcomeJson:
53685368
steps:
53695369
- id: fine
53705370
type: shell
5371-
run: "true"
5371+
run: "exit 0"
53725372
"""
53735373

53745374
def _run_json(self, tmp_path, monkeypatch, content, *, expected_exit=0):
@@ -5507,6 +5507,30 @@ def _options_payload(options):
55075507
assert _options_payload(7) == ["7"] # numeric scalar
55085508
assert _options_payload(None) is None # absent stays absent
55095509

5510+
def test_gate_block_choice_coerced_to_string(self):
5511+
# An unvalidated gate can record a non-string choice; the JSON
5512+
# surface normalises it to str (and keeps None = no decision yet),
5513+
# consistent with the message/options normalization.
5514+
from types import SimpleNamespace
5515+
from specify_cli import _gate_outcome
5516+
5517+
def _choice_payload(choice):
5518+
state = SimpleNamespace(
5519+
status=SimpleNamespace(value="paused"),
5520+
current_step_id="review",
5521+
step_results={
5522+
"review": {
5523+
"type": "gate",
5524+
"output": {"message": "m", "options": ["ok"], "choice": choice},
5525+
}
5526+
},
5527+
)
5528+
return _gate_outcome(state)["choice"]
5529+
5530+
assert _choice_payload(None) is None # no decision yet
5531+
assert _choice_payload("reject") == "reject" # normal string passes through
5532+
assert _choice_payload(2) == "2" # non-string coerced
5533+
55105534
def test_gate_block_detected_without_type_field(self):
55115535
# A run paused by an older version has no persisted step `type`. The
55125536
# gate is still detected by its unique output signature (`on_reject`),

0 commit comments

Comments
 (0)