From ffa9c262831b68679d8a9d9e196235ef50ad122a Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Fri, 17 Jul 2026 17:43:11 +0500 Subject: [PATCH] fix(workflows): reject a non-string prompt in prompt-step validate() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PromptStep.execute` str()-coerces `config['prompt']` and dispatches the result to the integration CLI as the model's instructions. But its `validate` only checked that `prompt` was *present*, not that it was a string — the exact parity gap the sibling `ShellStep` closes for `run`. So a YAML authoring slip like `prompt: [review, this]` or `prompt:` (null) passed validation, then `execute` sent the Python repr (`"['review', 'this']"`, `"None"`) to the LLM verbatim — silently wrong instructions with no error and a COMPLETED status. The engine does not auto-validate step config (`load_workflow` explicitly defers validation), so validation is the only place this surfaces before dispatch. Extend `validate` to reject any non-string `prompt` with the shell-step's phrasing ("'prompt' must be a string, got "), mirroring the shell `run` and command `input`/`options` type checks. A `{{ ... }}` expression is still a str, so it stays valid. Adds regression coverage for non-string prompts (null/list/int/dict) and confirms an expression prompt still validates. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../workflows/steps/prompt/__init__.py | 12 +++++++++ tests/test_workflows.py | 25 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/specify_cli/workflows/steps/prompt/__init__.py b/src/specify_cli/workflows/steps/prompt/__init__.py index 5ec99b794d..c8f8a32fd6 100644 --- a/src/specify_cli/workflows/steps/prompt/__init__.py +++ b/src/specify_cli/workflows/steps/prompt/__init__.py @@ -160,4 +160,16 @@ def validate(self, config: dict[str, Any]) -> list[str]: errors.append( f"Prompt step {config.get('id', '?')!r} is missing 'prompt' field." ) + elif not isinstance(config["prompt"], str): + # execute() str()-coerces prompt and dispatches it to the + # integration CLI, so a null or list 'prompt' would send the Python + # repr ('None', "['review', 'this']") to the model as instructions — + # silently wrong, with no error. Reject non-strings at validation, + # mirroring the shell-step 'run' and command-step input/options type + # checks. An expression like "{{ ... }}" is still a str, so it stays + # valid. + errors.append( + f"Prompt step {config.get('id', '?')!r}: 'prompt' must be a " + f"string, got {type(config['prompt']).__name__}." + ) return errors diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 8fefad740e..cf95fa5b7a 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1336,6 +1336,21 @@ def test_validate_missing_prompt(self): errors = step.validate({"id": "test"}) assert any("missing 'prompt'" in e for e in errors) + @pytest.mark.parametrize("bad_prompt", [None, ["review", "this"], 42, {"a": 1}]) + def test_validate_rejects_non_string_prompt(self, bad_prompt): + """A non-string 'prompt' must be rejected at validation. + + execute() str()-coerces prompt and dispatches it to the integration + CLI, so a null or list prompt would otherwise send the Python repr to + the model as instructions — silently wrong. Mirrors the shell-step + 'run' type check. + """ + from specify_cli.workflows.steps.prompt import PromptStep + + step = PromptStep() + errors = step.validate({"id": "p", "prompt": bad_prompt}) + assert any("'prompt' must be a string" in e for e in errors) + def test_validate_valid(self): from specify_cli.workflows.steps.prompt import PromptStep @@ -1343,6 +1358,16 @@ def test_validate_valid(self): errors = step.validate({"id": "test", "prompt": "do something"}) assert errors == [] + def test_validate_accepts_expression_prompt(self): + """A '{{ ... }}' expression prompt is a str, so it stays valid.""" + from specify_cli.workflows.steps.prompt import PromptStep + + step = PromptStep() + errors = step.validate( + {"id": "p", "prompt": "Review {{ inputs.file }}"} + ) + assert errors == [] + class TestShellStep: """Test the shell step type."""