mirror of
https://github.com/github/spec-kit.git
synced 2026-07-14 05:32:07 +08:00
fix(workflows): report validation errors instead of crashing on non-string workflow.yml scalars (#3421)
* fix(workflows): validate scalar types before string operations in workflow validation YAML parses unquoted scalars like version: 1.0 and id: 123 as float/int, which crashed validate_workflow and workflow add with raw tracebacks. Type-check id, name, version and step ids before regex and string operations so these surface as validation errors. Accept an unquoted schema_version: 1.0 instead of printing a self-identical rejection message. Fixes #3420 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): treat falsey non-strings as type errors, not missing fields Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): only accept schema_version 1.0 so the error message is accurate The check also accepted "1" while the error said Expected '1.0'. Unquoted YAML 1.0 still works via str(); plain 1 is now rejected with the message that matches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -601,7 +601,15 @@ def workflow_add(
|
||||
except (ValueError, yaml.YAMLError) as exc:
|
||||
console.print(f"[red]Error:[/red] Invalid workflow YAML: {exc}")
|
||||
raise typer.Exit(1)
|
||||
if not definition.id or not definition.id.strip():
|
||||
# Non-string ids (e.g. unquoted ``id: 123`` or ``id: 0``) fall through
|
||||
# to validate_workflow below, which reports a typed error instead of
|
||||
# crashing on ``.strip()`` here. Only None/empty/whitespace-only ids
|
||||
# are rejected as missing.
|
||||
if (
|
||||
definition.id is None
|
||||
or definition.id == ""
|
||||
or (isinstance(definition.id, str) and not definition.id.strip())
|
||||
):
|
||||
console.print("[red]Error:[/red] Workflow definition has an empty or missing 'id'")
|
||||
raise typer.Exit(1)
|
||||
|
||||
|
||||
@@ -129,26 +129,49 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
|
||||
errors: list[str] = []
|
||||
|
||||
# -- Schema version ---------------------------------------------------
|
||||
if definition.schema_version not in ("1.0", "1"):
|
||||
# str() so an unquoted ``schema_version: 1.0`` (YAML float) is accepted —
|
||||
# rejecting it would print "Unsupported schema_version 1.0. Expected '1.0'."
|
||||
if str(definition.schema_version) != "1.0":
|
||||
errors.append(
|
||||
f"Unsupported schema_version {definition.schema_version!r}. "
|
||||
f"Expected '1.0'."
|
||||
)
|
||||
|
||||
# -- Top-level fields -------------------------------------------------
|
||||
if not definition.id:
|
||||
# YAML parses unquoted scalars like ``id: 123`` or ``version: 1.0`` as
|
||||
# int/float; check types before regex/string operations so authoring
|
||||
# mistakes surface as validation errors instead of tracebacks. Only
|
||||
# ``None``/empty-string count as missing so falsey non-strings
|
||||
# (``id: 0``, ``name: false``) still get the typed error.
|
||||
if definition.id is None or definition.id == "":
|
||||
errors.append("Workflow is missing 'workflow.id'.")
|
||||
elif not isinstance(definition.id, str):
|
||||
errors.append(
|
||||
f"'workflow.id' must be a string, got "
|
||||
f"{type(definition.id).__name__} ({definition.id!r})."
|
||||
)
|
||||
elif not _ID_PATTERN.match(definition.id):
|
||||
errors.append(
|
||||
f"Workflow ID {definition.id!r} must be lowercase alphanumeric "
|
||||
f"with hyphens."
|
||||
)
|
||||
|
||||
if not definition.name:
|
||||
if definition.name is None or definition.name == "":
|
||||
errors.append("Workflow is missing 'workflow.name'.")
|
||||
elif not isinstance(definition.name, str):
|
||||
errors.append(
|
||||
f"'workflow.name' must be a string, got "
|
||||
f"{type(definition.name).__name__} ({definition.name!r})."
|
||||
)
|
||||
|
||||
if not definition.version:
|
||||
if definition.version is None or definition.version == "":
|
||||
errors.append("Workflow is missing 'workflow.version'.")
|
||||
elif not isinstance(definition.version, str):
|
||||
errors.append(
|
||||
f"'workflow.version' must be a string, got "
|
||||
f"{type(definition.version).__name__} ({definition.version!r}) — "
|
||||
f'quote it in YAML (version: "1.0.0").'
|
||||
)
|
||||
elif not re.match(r"^\d+\.\d+\.\d+$", definition.version):
|
||||
errors.append(
|
||||
f"Workflow version {definition.version!r} is not valid "
|
||||
@@ -256,9 +279,15 @@ def _validate_steps(
|
||||
continue
|
||||
|
||||
step_id = step_config.get("id")
|
||||
if not step_id:
|
||||
if step_id is None or step_id == "":
|
||||
errors.append("Step is missing 'id' field.")
|
||||
continue
|
||||
if not isinstance(step_id, str):
|
||||
errors.append(
|
||||
f"Step ID must be a string, got "
|
||||
f"{type(step_id).__name__} ({step_id!r})."
|
||||
)
|
||||
continue
|
||||
|
||||
if ":" in step_id:
|
||||
errors.append(
|
||||
|
||||
Reference in New Issue
Block a user