mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(workflows): guard non-mapping 'workflow:' block in WorkflowDefinition (#3694)
* fix(workflows): guard non-mapping 'workflow:' block in WorkflowDefinition
A present-but-non-mapping top-level `workflow:` block (bare `workflow:` ->
YAML null, or `workflow: <str>` / `workflow: [..]`) crashed
WorkflowDefinition.__init__ with AttributeError: the `{}` default of
`data.get("workflow", {})` only applies when the key is ABSENT, so a non-dict
value reached `workflow.get("id", ...)`. This fires inside from_yaml/
from_string — before validate_workflow can report the malformed shape — and
in the CLI escapes as a raw traceback (load_workflow is wrapped to catch only
FileNotFoundError/ValueError).
Normalize the local `workflow` to {} when it is not a mapping (self.data keeps
the raw value so validate_workflow still reports it), mirroring the adjacent
default_options guard.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(workflows): assert self.data preserves the raw non-mapping workflow value
Address review: the previous assertion only proved the key stayed present; it
would pass even if construction replaced the malformed value with {}. Assert
definition.data["workflow"] equals the original parsed value and is still a
non-mapping, proving the guard normalizes only the local variable.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -42,6 +42,17 @@ class WorkflowDefinition:
|
||||
self.source_path = source_path
|
||||
|
||||
workflow = data.get("workflow", {})
|
||||
# A present-but-non-mapping ``workflow:`` block (bare ``workflow:`` ->
|
||||
# None, or ``workflow: <str/list>``) would crash the following
|
||||
# ``workflow.get(...)`` calls with AttributeError, so construction fails
|
||||
# before any validation can run. Normalize the local to {} instead: the
|
||||
# header fields fall back to their defaults and ``validate_workflow``
|
||||
# (which reads those parsed attributes) reports the missing
|
||||
# ``workflow.id``/``workflow.name``. ``self.data`` is deliberately left
|
||||
# holding the raw value, since it is what gets written back out when a
|
||||
# definition is serialized. Mirrors the default_options guard below.
|
||||
if not isinstance(workflow, dict):
|
||||
workflow = {}
|
||||
self.id: str = workflow.get("id", "")
|
||||
self.name: str = workflow.get("name", "")
|
||||
self.version: str = workflow.get("version", "0.0.0")
|
||||
|
||||
@@ -3629,6 +3629,31 @@ class TestWorkflowDefinition:
|
||||
resolved = WorkflowEngine()._resolve_inputs(definition, {}) # must not raise
|
||||
assert resolved == {}
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"block", ["workflow:\nsteps: []\n", "workflow: hi\nsteps: []\n", "workflow: [a]\nsteps: []\n"]
|
||||
)
|
||||
def test_non_mapping_workflow_block_parses_then_validates(self, block):
|
||||
# A present-but-non-mapping `workflow:` block must not crash construction
|
||||
# with AttributeError; it should parse to an empty header so
|
||||
# validate_workflow reports the missing id/name (it reads the parsed
|
||||
# attributes, not the raw block).
|
||||
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow
|
||||
|
||||
definition = WorkflowDefinition.from_string(block) # must not raise
|
||||
assert definition.id == ""
|
||||
errors = validate_workflow(definition)
|
||||
assert any("workflow.id" in e for e in errors)
|
||||
# The RAW malformed value is preserved on .data (the guard only
|
||||
# normalizes the local var, not self.data) — .data is what gets written
|
||||
# back out when a definition is serialized. Assert it was NOT replaced
|
||||
# with {} by comparing against the original parse and confirming it is
|
||||
# still a non-mapping.
|
||||
import yaml
|
||||
|
||||
raw_workflow = yaml.safe_load(block).get("workflow")
|
||||
assert definition.data["workflow"] == raw_workflow
|
||||
assert not isinstance(definition.data["workflow"], dict)
|
||||
|
||||
def test_from_string_invalid(self):
|
||||
from specify_cli.workflows.engine import WorkflowDefinition
|
||||
|
||||
|
||||
Reference in New Issue
Block a user