fix(workflows): gate validate() must not crash on non-string options (#3233)

GateStep.validate() reports non-string options as an error, but then -- when on_reject is 'abort'/'retry' -- still runs the reject-choice check 'any(o.lower() in ... for o in options)'. For a non-string option (e.g. options: [123]) o.lower() raised AttributeError, which escaped validate() and broke validate_workflow's documented 'return a list of errors, never raise' contract. Guard the check so it only runs when every option is a string (the non-string case is already reported above).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ali jawwad
2026-06-30 01:07:50 +05:00
committed by GitHub
parent 9ece347a77
commit 876dca8659
2 changed files with 25 additions and 1 deletions

View File

@@ -194,7 +194,14 @@ class GateStep(StepBase):
f"Gate step {config.get('id', '?')!r}: 'on_reject' must be "
f"'abort', 'skip', or 'retry'."
)
if on_reject in ("abort", "retry") and isinstance(options, list):
# Only inspect option text when every option is a string; otherwise the
# `o.lower()` below would raise AttributeError on a non-string option
# (already reported above) and break validate_workflow's never-raise contract.
if (
on_reject in ("abort", "retry")
and isinstance(options, list)
and all(isinstance(o, str) for o in options)
):
reject_choices = {"reject", "abort"}
if not any(o.lower() in reject_choices for o in options):
errors.append(

View File

@@ -1455,6 +1455,23 @@ class TestGateStep:
})
assert any("on_reject" in e for e in errors)
def test_validate_non_string_options_does_not_raise(self):
"""Non-string options with on_reject=abort/retry must be REPORTED as an
error, not crash: the reject-choice check calls o.lower() on each option,
which previously raised AttributeError on a non-string option and broke
validate_workflow's 'return errors, never raise' contract."""
from specify_cli.workflows.steps.gate import GateStep
step = GateStep()
# on_reject defaults to "abort", which triggers the option-text check.
errors = step.validate({"id": "test", "message": "Review", "options": [123]})
assert any("must be strings" in e for e in errors)
# also with an explicit retry on_reject
errors = step.validate(
{"id": "test", "message": "Review", "options": [True], "on_reject": "retry"}
)
assert any("must be strings" in e for e in errors)
def test_interactive_prompt_renders_show_file(self, tmp_path, monkeypatch, capsys):
from specify_cli.workflows.steps.gate import GateStep
from specify_cli.workflows.base import StepContext, StepStatus