fix(workflows): fail a gate whose on_reject is not abort/skip/retry (#3888)

execute() reads `on_reject = config.get("on_reject", "abort")` and, in the
reject branch, handles only "abort" and "retry" before falling through to
its `# on_reject == "skip"` case. So any other value makes a REJECTED gate
report COMPLETED and the run walks straight past the review the gate
exists to enforce:

  on_reject='abort'  -> failed     "Gate rejected by user at step 'g'"
  on_reject='retry'  -> paused
  on_reject='skip'   -> completed  (by design)
  on_reject='Abort'  -> completed  <-- rejection silently discarded
  on_reject='fail'   -> completed  <-- same
  on_reject='stop'   -> completed  <-- same
  on_reject=None     -> completed  <-- same
  on_reject=5        -> completed  <-- same

Reachable by a capitalisation slip, a guessed verb, a non-string, or a
bare `on_reject:` — note `config.get(k, default)` does NOT substitute the
default for an explicit YAML null.

`validate` already rejects anything outside abort/skip/retry, but the
engine does not auto-validate before execute(). Fail loudly instead,
mirroring the `options` and `verdict_input` guards in the same method, and
placed before the non-TTY short-circuit so it surfaces in CI too.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/specify_cli/workflows/steps/gate/__init__.py

@@ -75,6 +75,31 @@ class GateStep(StepBase):
                 },
             )
 
+        # ``validate`` rejects an ``on_reject`` outside abort/skip/retry, but the
+        # engine does not auto-validate before ``execute``. The reject branch
+        # below handles only "abort" and "retry" and then falls through to its
+        # ``on_reject == "skip"`` case, so on an unvalidated run any other value
+        # makes a REJECTED gate report COMPLETED and the run walks straight past
+        # the review the gate exists to enforce. Reachable by a capitalisation
+        # slip ("Abort"), a guessed verb ("fail", "stop"), a non-string, or the
+        # ``None`` that a bare ``on_reject:`` yields -- note ``config.get(k,
+        # default)`` does NOT substitute the default for an explicit null. Fail
+        # loudly instead, mirroring the ``options``/``verdict_input`` guards here.
+        if on_reject not in ("abort", "skip", "retry"):
+            return StepResult(
+                status=StepStatus.FAILED,
+                error=(
+                    f"Gate step {config.get('id', '?')!r}: 'on_reject' must be "
+                    f"'abort', 'skip', or 'retry', got {on_reject!r}."
+                ),
+                output={
+                    "message": message,
+                    "options": options,
+                    "on_reject": on_reject,
+                    "choice": None,
+                },
+            )
+
         if has_verdict_input and (
             not isinstance(verdict_input, str) or not verdict_input
         ):

tests/test_workflows.py

@@ -2552,6 +2552,36 @@ steps:
         })
         assert any("on_reject" in e for e in errors)
 
+    @pytest.mark.parametrize(
+        "bad_on_reject", ["Abort", "fail", "stop", "SKIP", None, 5, ["abort"]]
+    )
+    def test_execute_invalid_on_reject_fails_loudly(self, bad_on_reject):
+        """An unrecognised ``on_reject`` must not silently complete a rejection.
+
+        ``validate`` rejects anything outside abort/skip/retry, but the engine
+        does not auto-validate before ``execute``. The reject branch handles only
+        "abort" and "retry", then falls through to its ``"skip"`` case — so a
+        REJECTED gate reported COMPLETED and the run continued past the review
+        the gate exists to enforce. Reachable by a capitalisation slip, a guessed
+        verb, a non-string, or a bare ``on_reject:`` (which yields None, since
+        ``config.get(k, default)`` does not replace an explicit null).
+        """
+        from specify_cli.workflows.steps.gate import GateStep
+        from specify_cli.workflows.base import StepContext, StepStatus
+
+        result = GateStep().execute(
+            {
+                "id": "review",
+                "message": "Review the spec.",
+                "options": ["approve", "reject"],
+                "on_reject": bad_on_reject,
+                "verdict_input": "spec_verdict",
+            },
+            StepContext(inputs={"spec_verdict": "reject"}),
+        )
+        assert result.status == StepStatus.FAILED
+        assert "'on_reject' must be" in (result.error or "")
+
     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,