diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index 98005a790b..81479ac9f4 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -207,9 +207,18 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]: # 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". + # a raw ``TypeError`` at run time. + # + # Distinguish an *omitted* ``enum`` (no restriction — valid) from a + # *present* one via ``in``, not ``.get()``: ``.get("enum")`` returns + # ``None`` for both an absent key and an explicit ``enum:``/``enum: + # null``, so a ``None``-based check would silently accept a declared + # null enum even though it is a non-list. Mirrors the present-but-null + # ``requires:`` handling below. + enum_present = "enum" in input_def enum_values = input_def.get("enum") - if enum_values is not None and not isinstance(enum_values, list): + enum_is_valid = not enum_present or isinstance(enum_values, list) + if not enum_is_valid: errors.append( f"Input {input_name!r} has invalid 'enum': must be a list, " f"got {type(enum_values).__name__}." @@ -223,7 +232,6 @@ 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 = ( @@ -1484,6 +1492,7 @@ def _coerce_input( ) -> Any: """Coerce a provided input value to the declared type.""" input_type = input_def.get("type", "string") + enum_present = "enum" in input_def enum_values = input_def.get("enum") # ``enum`` must be a list. A scalar (``enum: 5``, ``enum: true``) makes @@ -1493,9 +1502,13 @@ def _coerce_input( # "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): + # list so both forms fail fast with a clear message. + # + # An *omitted* ``enum`` means "no restriction" and is left alone; a + # *present* ``enum:``/``enum: null`` is a declared non-list and must fail + # like any other. Distinguish the two via ``in`` — ``.get()`` collapses + # both to ``None`` and would let a null enum resolve silently. + if enum_present and not isinstance(enum_values, list): msg = ( f"Input {name!r} has invalid 'enum': must be a list, got " f"{type(enum_values).__name__}." diff --git a/tests/test_workflows.py b/tests/test_workflows.py index cd1b235411..2c84ebcf31 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -4503,6 +4503,242 @@ 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_validate_workflow_rejects_null_enum(self): + """An explicitly declared ``enum:`` / ``enum: null`` is a non-list too and + must be rejected. + + ``.get("enum")`` returns ``None`` for both an omitted key and a present + null value, so a ``None``-based guard would silently accept a declared + null enum. validate_workflow distinguishes the two by key presence, so a + present null is reported like any other non-list.""" + from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "null-enum" + name: "Null Enum" + version: "1.0.0" +inputs: + scope: + type: string + enum: +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_omitted_enum_is_valid(self): + """An *omitted* ``enum`` means "no restriction" and must stay valid — the + null-enum guard must not over-reach and flag inputs that declare no enum + at all.""" + from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "no-enum" + name: "No Enum" + version: "1.0.0" +inputs: + scope: + type: string + default: "full" +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_resolve_inputs_rejects_null_enum_at_runtime(self, project_dir): + """A present ``enum: null`` must raise a clean ``ValueError`` at run time + too — ``execute`` does not auto-validate, and ``.get("enum")`` collapsing + null to ``None`` would otherwise let the input resolve as if unrestricted.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "runtime-null-enum" + name: "Runtime Null Enum" + version: "1.0.0" +inputs: + scope: + type: string + enum: +""") + engine = WorkflowEngine(project_dir) + with pytest.raises(ValueError, match="invalid 'enum'"): + engine._resolve_inputs(definition, {"scope": "bar"}) + 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.