Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 54 additions & 2 deletions src/specify_cli/workflows/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,20 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
f"Must be 'string', 'number', or 'boolean'."
)

# ``enum`` must be a list. Checked here — not only via the
# ``_coerce_input`` call below — because that call is reached only
# when a ``default`` is present, and the ``integration: auto`` case
# strips ``enum`` before coercing; a scalar/string ``enum`` on an
# input with no default (or the auto-integration default) would
# otherwise slip through here and then crash ``_resolve_inputs`` with
# a raw ``TypeError`` at run time. ``None`` means "no enum".
enum_values = input_def.get("enum")
if enum_values is not None and not isinstance(enum_values, list):
errors.append(
f"Input {input_name!r} has invalid 'enum': must be a list, "
f"got {type(enum_values).__name__}."
)

# Validate the default eagerly so authoring mistakes (e.g. a
# default not in the declared enum, or a non-numeric default for
# a number input) surface at install/validation time instead of
Expand All @@ -201,13 +215,28 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
# enum-membership check is exempted for that exact case — the
# declared type is still enforced (e.g. ``type: number`` paired
# with ``default: "auto"`` is still rejected).
enum_is_valid = enum_values is None or isinstance(enum_values, list)
if "default" in input_def:
default_value = input_def["default"]
is_auto_integration = (
input_name == "integration" and default_value == "auto"
)
# Strip ``enum`` from the definition handed to ``_coerce_input``
# when either:
# * this is the auto-integration sentinel (enum-membership is
# a runtime concern, exempted for ``"auto"``), or
# * the ``enum`` is malformed (non-list) and already reported
# above — leaving it in would make ``_coerce_input`` re-raise
# the same enum-shape error re-framed as an "invalid default"
# (a confusing duplicate).
# Removing *only* ``enum`` (rather than skipping the check
# entirely) preserves the default's type validation: a
# ``type: string`` input with ``default: 5, enum: 5`` still
# reports the wrong-typed default alongside the enum error,
# instead of hiding it.
strip_enum = (is_auto_integration or not enum_is_valid)
validation_input_def: dict[str, Any] = input_def
if is_auto_integration and "enum" in input_def:
if strip_enum and "enum" in input_def:
validation_input_def = {
key: value
for key, value in input_def.items()
Expand Down Expand Up @@ -1368,11 +1397,18 @@ def _resolve_inputs(
# definition (``string`` rejects non-strings, ``number`` rejects
# bools and uncoercible values, ``boolean`` rejects non-bools),
# so ill-typed values still fail fast here.
#
# ``execute()`` accepts unvalidated definitions, so a malformed
# (non-list) ``enum`` can reach here. Only strip a *list* ``enum``:
# a scalar/string ``enum`` must stay in the definition so
# ``_coerce_input`` raises the clean shape ``ValueError`` instead of
# being silently exempted by the ``auto`` membership skip (which
# would otherwise let ``enum: 5`` resolve successfully).
coerce_input_def = input_def
if (
name == "integration"
and value == "auto"
and "enum" in input_def
and isinstance(input_def.get("enum"), list)
):
coerce_input_def = {
key: val
Expand Down Expand Up @@ -1418,6 +1454,22 @@ def _coerce_input(
input_type = input_def.get("type", "string")
enum_values = input_def.get("enum")

# ``enum`` must be a list. A scalar (``enum: 5``, ``enum: true``) makes
# the ``value not in enum_values`` membership test below raise a raw
# ``TypeError`` ("argument of type 'int' is not ... iterable"), which
# escapes ``validate_workflow``'s ``except ValueError`` and breaks its
# "return errors, never raise" contract — and crashes ``_resolve_inputs``
# outright at run time. A bare string is just as wrong: ``value in "abc"``
# is a silent substring/character test, not enum membership. Require a
# list so both forms fail fast with a clear message. ``None`` means "no
# enum" and is left alone.
if enum_values is not None and not isinstance(enum_values, list):
msg = (
f"Input {name!r} has invalid 'enum': must be a list, got "
f"{type(enum_values).__name__}."
)
raise ValueError(msg)
Comment on lines +1466 to +1471

if input_type == "number":
# Reject bools explicitly: ``bool`` is a subclass of ``int`` so
# ``float(True)`` succeeds and would silently coerce a YAML
Expand Down
161 changes: 161 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -3950,6 +3950,167 @@ def test_validate_workflow_rejects_non_string_default_for_string_type(self):
errors = validate_workflow(definition)
assert any("invalid default" in e for e in errors), errors

def test_validate_workflow_rejects_scalar_enum(self):
"""A non-list ``enum`` (``enum: 5``) must be reported as a validation
error, not crash ``validate_workflow`` with a raw ``TypeError`` from the
``value not in enum_values`` membership test."""
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "scalar-enum"
name: "Scalar Enum"
version: "1.0.0"
inputs:
scope:
type: string
enum: 5
steps:
- id: noop
type: gate
message: "noop"
options: [approve]
""")
errors = validate_workflow(definition)
assert any("invalid 'enum'" in e and "must be a list" in e for e in errors), errors

def test_validate_workflow_rejects_string_enum(self):
"""A bare-string ``enum`` (``enum: abc``) must be rejected too — otherwise
``value in "abc"`` is a silent substring/character test, not enum
membership."""
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "string-enum"
name: "String Enum"
version: "1.0.0"
inputs:
scope:
type: string
default: "a"
enum: "abc"
steps:
- id: noop
type: gate
message: "noop"
options: [approve]
""")
errors = validate_workflow(definition)
assert any("invalid 'enum'" in e and "must be a list" in e for e in errors), errors
# The malformed enum is reported once — not re-framed a second time as an
# "invalid default" by the default-coercion path.
assert sum("invalid 'enum'" in e for e in errors) == 1, errors

def test_resolve_inputs_rejects_scalar_enum_at_runtime(self, project_dir):
"""A non-list ``enum`` must raise a clean ``ValueError`` (not a raw
``TypeError``) when a provided value is coerced at run time, since
``execute`` does not auto-validate the definition first."""
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "runtime-scalar-enum"
name: "Runtime Scalar Enum"
version: "1.0.0"
inputs:
scope:
type: string
enum: 5
""")
engine = WorkflowEngine(project_dir)
with pytest.raises(ValueError, match="invalid 'enum'"):
engine._resolve_inputs(definition, {"scope": "bar"})

def test_validate_workflow_accepts_list_enum(self):
"""A well-formed list ``enum`` must still validate cleanly and continue to
check the default against enum membership."""
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "list-enum"
name: "List Enum"
version: "1.0.0"
inputs:
scope:
type: string
default: "full"
enum: ["full", "backend-only"]
steps:
- id: noop
type: gate
message: "noop"
options: [approve]
""")
errors = validate_workflow(definition)
assert not any("enum" in e for e in errors), errors

def test_validate_workflow_scalar_enum_still_reports_bad_default_type(self):
"""A malformed ``enum`` must not mask an independent default type error.

With ``type: string`` and ``default: 5`` alongside ``enum: 5``, the
malformed enum is reported once *and* the wrong-typed default is still
surfaced — stripping only ``enum`` from the coercion input preserves the
default's type validation instead of skipping it entirely."""
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "scalar-enum-bad-default"
name: "Scalar Enum Bad Default"
version: "1.0.0"
inputs:
scope:
type: string
default: 5
enum: 5
steps:
- id: noop
type: gate
message: "noop"
options: [approve]
""")
errors = validate_workflow(definition)
# The enum shape is reported exactly once...
assert sum("invalid 'enum'" in e for e in errors) == 1, errors
# ...and the independent wrong-typed default is still surfaced.
assert any(
"invalid default" in e and "expected a string" in e for e in errors
), errors

def test_resolve_inputs_auto_integration_rejects_scalar_enum(self, project_dir):
"""The ``integration: auto`` runtime exemption must not swallow a
malformed ``enum``.

``_resolve_inputs`` strips ``enum`` before coercing the ``auto``
sentinel so a workflow listing specific integrations doesn't crash on
``"auto"``. But that strip must apply only to a *list* ``enum`` — a
scalar ``enum: 5`` must still reach ``_coerce_input`` and raise a clean
``ValueError``, not be silently exempted and resolve successfully."""
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition

definition = WorkflowDefinition.from_string("""
schema_version: "1.0"
workflow:
id: "auto-integration-scalar-enum"
name: "Auto Integration Scalar Enum"
version: "1.0.0"
inputs:
integration:
type: string
default: "auto"
enum: 5
""")
engine = WorkflowEngine(project_dir)
with pytest.raises(ValueError, match="invalid 'enum'"):
engine._resolve_inputs(definition, {})

def test_while_loop_condition_reads_latest_iteration(self, project_dir):
"""Regression: while-loop condition must see updated step output
from the most recent iteration, not stale iteration-0 data.
Expand Down