mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(workflows): reject a non-string prompt in prompt-step validate() (#3582)
`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 <type>"), 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) <noreply@anthropic.com>
This commit is contained in:
@@ -1381,6 +1381,21 @@ class TestPromptStep:
|
||||
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
|
||||
|
||||
@@ -1388,6 +1403,16 @@ class TestPromptStep:
|
||||
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."""
|
||||
|
||||
Reference in New Issue
Block a user