mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
Compare commits
11 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c78993d74b | ||
|
|
6d77b4a099 | ||
|
|
57cc518d63 | ||
|
|
eb2252a1cb | ||
|
|
2df0394cb2 | ||
|
|
3d2901eb75 | ||
|
|
c1e5cfa0aa | ||
|
|
b139bd0393 | ||
|
|
f75f5f836b | ||
|
|
c864fc7447 | ||
|
|
848e41bc92 |
30
CHANGELOG.md
30
CHANGELOG.md
@@ -2,6 +2,36 @@
|
||||
|
||||
<!-- insert new changelog below this comment -->
|
||||
|
||||
## [0.13.1] - 2026-07-21
|
||||
|
||||
### Changed
|
||||
|
||||
- fix(integrations): catch OverflowError on a `priority: .inf` in add/remove (#3589)
|
||||
- fix(workflows): reject bool / .inf catalog priority in workflow & step catalog loaders (#3526)
|
||||
- fix(catalogs): 'priority: .inf' yields a clean validation error instead of crashing (#3525)
|
||||
- docs(integrations): document the 'integration list --catalog' flag (#3530)
|
||||
- fix(workflows): fail fan-in loudly on a non-string wait_for entry (#3579)
|
||||
- fix(workflows): fail fan-out loudly on a truthy non-mapping step template (#3537)
|
||||
- fix(workflows): reject a non-string prompt in prompt-step validate() (#3582)
|
||||
- fix(workflows): route 'workflow status --json' errors to stderr (#3520)
|
||||
- fix(integrations): Forge dispatches hyphenated /speckit-<cmd> invocations (#3529)
|
||||
- chore: release 0.13.0, begin 0.13.1.dev0 development (#3588)
|
||||
|
||||
## [0.13.0] - 2026-07-17
|
||||
|
||||
### Changed
|
||||
|
||||
- fix(auth): Azure DevOps az-CLI token acquisition returns None on undecodable output (#3527)
|
||||
- feat(extensions): add assess idea assessment pipeline extension (#3568)
|
||||
- fix(bundle): surface a clean BundlerError on a malformed bundle download URL (#3586)
|
||||
- Add OKF Knowledge Bundle Generator extension to community catalog (#3585)
|
||||
- Update Autonomous Run Governance preset to v0.2.2 (#3584)
|
||||
- docs: update extension guide PyPI upgrade guidance (#3578)
|
||||
- fix(presets): raise PresetValidationError, not raw ValueError, on malformed catalog URL (#3576)
|
||||
- chore(deps): bump github/codeql-action/init from 4.36.2 to 4.37.1 (#3571)
|
||||
- docs: align README hero tagline and subtitle with docs/index.md (#3581)
|
||||
- chore: release 0.12.18, begin 0.12.19.dev0 development (#3583)
|
||||
|
||||
## [0.12.18] - 2026-07-17
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -48,7 +48,11 @@ The Specify CLI supports a wide range of AI coding agents. When you run `specify
|
||||
specify integration list
|
||||
```
|
||||
|
||||
Shows all available integrations, which one is currently installed, and whether each requires a CLI tool or is IDE-based.
|
||||
| Option | Description |
|
||||
| ----------- | ----------------------------------------------------------------------------------------------------------------------- |
|
||||
| `--catalog` | Also browse the catalog (built-in **and** community). Community integrations that are not built in are only shown here. |
|
||||
|
||||
Shows the built-in integrations, which one is currently installed, and whether each requires a CLI tool or is IDE-based.
|
||||
When multiple integrations are installed, the list marks the default integration separately from the other installed integrations.
|
||||
The list also shows whether each built-in integration is declared multi-install safe.
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "specify-cli"
|
||||
version = "0.12.19.dev0"
|
||||
version = "0.13.1"
|
||||
description = "Specify CLI, part of GitHub Spec Kit. A tool to bootstrap your projects for Spec-Driven Development (SDD)."
|
||||
readme = "README.md"
|
||||
requires-python = ">=3.11"
|
||||
|
||||
@@ -149,7 +149,10 @@ class CatalogStackBase:
|
||||
)
|
||||
try:
|
||||
priority = int(raw_priority)
|
||||
except (TypeError, ValueError):
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — a YAML ``priority: .inf``
|
||||
# would otherwise escape as an uncaught traceback instead of the
|
||||
# clean validation error.
|
||||
raise self._validation_error(
|
||||
f"Invalid catalog config {config_path}: "
|
||||
f"Invalid priority for catalog '{item.get('name', idx + 1)}': "
|
||||
|
||||
@@ -429,7 +429,8 @@ class IntegrationCatalog(CatalogStackBase):
|
||||
)
|
||||
try:
|
||||
normalized_priority = int(raw_priority)
|
||||
except (TypeError, ValueError):
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — a ``priority: .inf``.
|
||||
raise IntegrationValidationError(
|
||||
f"Invalid catalog entry at index {idx} in {config_path}: "
|
||||
f"'priority' must be an integer, got "
|
||||
@@ -537,7 +538,8 @@ class IntegrationCatalog(CatalogStackBase):
|
||||
else:
|
||||
try:
|
||||
priority = int(raw_priority)
|
||||
except (TypeError, ValueError):
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — a ``priority: .inf``.
|
||||
priority = yaml_idx + 1
|
||||
priority_pairs.append((priority, yaml_idx))
|
||||
if not priority_pairs:
|
||||
|
||||
@@ -91,6 +91,18 @@ class ForgeIntegration(MarkdownIntegration):
|
||||
}
|
||||
invoke_separator = "-"
|
||||
|
||||
def build_command_invocation(self, command_name: str, args: str = "") -> str:
|
||||
"""Forge installs hyphenated slash-commands (``/speckit-<name>``), so the
|
||||
dispatch invocation must match. The inherited MarkdownIntegration default
|
||||
builds the dotted ``/speckit.<name>``, which references a command Forge
|
||||
never registered. Reuse the same hyphenation as the installed frontmatter
|
||||
``name`` (see ``format_forge_command_name``), mirroring the skills agents.
|
||||
"""
|
||||
invocation = "/" + format_forge_command_name(command_name)
|
||||
if args:
|
||||
invocation = f"{invocation} {args}"
|
||||
return invocation
|
||||
|
||||
def setup(
|
||||
self,
|
||||
project_root: Path,
|
||||
|
||||
@@ -2235,7 +2235,10 @@ class PresetCatalog:
|
||||
)
|
||||
try:
|
||||
priority = int(raw_priority)
|
||||
except (TypeError, ValueError):
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — a YAML ``priority: .inf``
|
||||
# would otherwise escape as an uncaught traceback instead of the
|
||||
# clean validation error (mirrors catalogs.py).
|
||||
raise PresetValidationError(
|
||||
f"Invalid priority for catalog '{item.get('name', idx + 1)}': "
|
||||
f"expected integer, got {raw_priority!r}"
|
||||
|
||||
@@ -1262,14 +1262,18 @@ def workflow_status(
|
||||
engine = WorkflowEngine(project_root)
|
||||
|
||||
if run_id:
|
||||
# Route errors to stderr under --json so the stdout JSON stream stays
|
||||
# parseable (mirrors `workflow run`/`workflow resume`); both handlers
|
||||
# fire before the json_output branch below.
|
||||
err = _error_console(json_output)
|
||||
try:
|
||||
from .engine import RunState
|
||||
state = RunState.load(run_id, project_root)
|
||||
except FileNotFoundError:
|
||||
console.print(f"[red]Error:[/red] Run not found: {run_id}")
|
||||
err.print(f"[red]Error:[/red] Run not found: {run_id}")
|
||||
raise typer.Exit(1)
|
||||
except ValueError as exc:
|
||||
console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}")
|
||||
err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}")
|
||||
raise typer.Exit(1)
|
||||
|
||||
if json_output:
|
||||
|
||||
@@ -364,13 +364,24 @@ class WorkflowCatalog:
|
||||
if not url:
|
||||
continue
|
||||
self._validate_catalog_url(url)
|
||||
try:
|
||||
priority = int(item.get("priority", idx + 1))
|
||||
except (TypeError, ValueError):
|
||||
raw_priority = item.get("priority", idx + 1)
|
||||
# bool is an int subclass: int(True) == 1 would silently accept a
|
||||
# ``priority: true`` as priority 1. Reject it explicitly, mirroring
|
||||
# the base CatalogStackBase loader.
|
||||
if isinstance(raw_priority, bool):
|
||||
raise WorkflowValidationError(
|
||||
f"Invalid priority for catalog "
|
||||
f"'{item.get('name', idx + 1)}': "
|
||||
f"expected integer, got {item.get('priority')!r}"
|
||||
f"expected integer, got {raw_priority!r}"
|
||||
)
|
||||
try:
|
||||
priority = int(raw_priority)
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — a ``priority: .inf``.
|
||||
raise WorkflowValidationError(
|
||||
f"Invalid priority for catalog "
|
||||
f"'{item.get('name', idx + 1)}': "
|
||||
f"expected integer, got {raw_priority!r}"
|
||||
)
|
||||
raw_install = item.get("install_allowed", False)
|
||||
if isinstance(raw_install, str):
|
||||
@@ -685,7 +696,9 @@ class WorkflowCatalog:
|
||||
def _coerce_priority(value: Any) -> int:
|
||||
try:
|
||||
return int(value)
|
||||
except (TypeError, ValueError):
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — treat an uncoercible
|
||||
# existing priority as 0 rather than crashing 'catalog add'.
|
||||
return 0
|
||||
|
||||
max_priority = max(
|
||||
@@ -1007,13 +1020,23 @@ class StepCatalog:
|
||||
if not url:
|
||||
continue
|
||||
self._validate_catalog_url(url)
|
||||
try:
|
||||
priority = int(item.get("priority", idx + 1))
|
||||
except (TypeError, ValueError):
|
||||
raw_priority = item.get("priority", idx + 1)
|
||||
# bool is an int subclass: reject ``priority: true`` explicitly rather
|
||||
# than silently coercing it to 1 (mirrors CatalogStackBase).
|
||||
if isinstance(raw_priority, bool):
|
||||
raise StepValidationError(
|
||||
f"Invalid priority for catalog "
|
||||
f"'{item.get('name', idx + 1)}': "
|
||||
f"expected integer, got {item.get('priority')!r}"
|
||||
f"expected integer, got {raw_priority!r}"
|
||||
)
|
||||
try:
|
||||
priority = int(raw_priority)
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — a ``priority: .inf``.
|
||||
raise StepValidationError(
|
||||
f"Invalid priority for catalog "
|
||||
f"'{item.get('name', idx + 1)}': "
|
||||
f"expected integer, got {raw_priority!r}"
|
||||
)
|
||||
raw_install = item.get("install_allowed", False)
|
||||
if isinstance(raw_install, str):
|
||||
@@ -1314,7 +1337,9 @@ class StepCatalog:
|
||||
def _coerce_priority(value: Any) -> int:
|
||||
try:
|
||||
return int(value)
|
||||
except (TypeError, ValueError):
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — treat an uncoercible
|
||||
# existing priority as 0 rather than crashing 'catalog add'.
|
||||
return 0
|
||||
|
||||
max_priority = max(
|
||||
|
||||
@@ -42,6 +42,28 @@ class FanInStep(StepBase):
|
||||
output={"results": []},
|
||||
)
|
||||
|
||||
# A non-string entry can never match a real step id. An unhashable one
|
||||
# (a list/dict from a YAML indentation slip like ``wait_for: [[a, b]]``)
|
||||
# crashes the whole run at ``context.steps.get(step_id, ...)`` below with
|
||||
# a raw TypeError; a hashable-but-non-string one (``wait_for: [123]``)
|
||||
# silently joins an empty ``{}`` and still reports COMPLETED — the exact
|
||||
# "silent empty result + COMPLETED" wiring bug the whole-list guard above
|
||||
# and the engine's fan-in validation (engine.py) both reject. The engine
|
||||
# does not auto-validate step config, so fail this step loudly on an
|
||||
# unvalidated run too, using the engine's phrasing.
|
||||
bad_entries = [w for w in wait_for if not isinstance(w, str)]
|
||||
if bad_entries:
|
||||
first = bad_entries[0]
|
||||
return StepResult(
|
||||
status=StepStatus.FAILED,
|
||||
error=(
|
||||
f"Fan-in step {config.get('id', '?')!r}: 'wait_for' entries "
|
||||
f"must be step-id strings, got {type(first).__name__} "
|
||||
f"({first!r})."
|
||||
),
|
||||
output={"results": []},
|
||||
)
|
||||
|
||||
# Collect results from referenced steps
|
||||
results = []
|
||||
for step_id in wait_for:
|
||||
|
||||
@@ -25,6 +25,33 @@ class FanOutStep(StepBase):
|
||||
max_concurrency = config.get("max_concurrency", 1)
|
||||
step_template = config.get("step", {})
|
||||
|
||||
# The engine does not auto-validate step config (see
|
||||
# ``WorkflowEngine.load_workflow``). On a COMPLETED fan-out it reads the
|
||||
# ``step_template`` back out and, when it is truthy, calls
|
||||
# ``template.get("id", ...)`` in ``_run_fan_out``. A truthy non-mapping
|
||||
# ``step`` (a scalar or list authoring mistake) would crash the whole
|
||||
# run with AttributeError there — the engine invokes ``execute`` and
|
||||
# ``_run_fan_out`` with no surrounding try/except. ``validate`` already
|
||||
# rejects a non-mapping ``step``; fail this step loudly on an
|
||||
# unvalidated run instead, mirroring the ``items`` guard below. An empty
|
||||
# or absent ``step`` defaults to ``{}`` (falsy) and the engine's
|
||||
# ``if template and items`` skips fan-out, so it stays valid here.
|
||||
if not isinstance(step_template, dict):
|
||||
return StepResult(
|
||||
status=StepStatus.FAILED,
|
||||
error=(
|
||||
f"Fan-out step {config.get('id', '?')!r}: 'step' must be a "
|
||||
f"mapping (nested step template), got "
|
||||
f"{type(step_template).__name__}."
|
||||
),
|
||||
output={
|
||||
"items": [],
|
||||
"max_concurrency": max_concurrency,
|
||||
"step_template": {},
|
||||
"item_count": 0,
|
||||
},
|
||||
)
|
||||
|
||||
if not isinstance(items, list):
|
||||
# A non-list here is a wiring error (the expression did not
|
||||
# resolve to a collection); silently fanning out over zero
|
||||
@@ -66,8 +93,13 @@ class FanOutStep(StepBase):
|
||||
f"Fan-out step {config.get('id', '?')!r} is missing "
|
||||
f"'step' field (nested step template)."
|
||||
)
|
||||
step = config.get("step")
|
||||
if step is not None and not isinstance(step, dict):
|
||||
elif not isinstance(config["step"], dict):
|
||||
# A present-but-non-mapping ``step`` (including an explicit
|
||||
# ``step: null``) is an authoring mistake. ``config.get("step", {})``
|
||||
# in ``execute`` only substitutes the ``{}`` default for an *absent*
|
||||
# key, so an explicit ``None`` reaches the runtime guard and FAILS
|
||||
# the step. Reject it here too so a workflow cannot pass validation
|
||||
# and then fail during execution.
|
||||
errors.append(
|
||||
f"Fan-out step {config.get('id', '?')!r}: 'step' must be a mapping."
|
||||
)
|
||||
|
||||
@@ -160,4 +160,16 @@ class PromptStep(StepBase):
|
||||
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
|
||||
|
||||
@@ -216,6 +216,24 @@ class TestBuildCommandInvocation:
|
||||
i = get_integration("codex")
|
||||
assert i.build_command_invocation("speckit.git.commit", "fix typo") == "/speckit-git-commit fix typo"
|
||||
|
||||
def test_forge_core_command_hyphenated(self):
|
||||
"""Forge installs hyphenated slash-commands (/speckit-<name>), so the
|
||||
dispatch invocation must be hyphenated too — not the dotted default it
|
||||
would inherit from MarkdownIntegration."""
|
||||
from specify_cli.integrations import get_integration
|
||||
i = get_integration("forge")
|
||||
assert i.build_command_invocation("speckit.plan") == "/speckit-plan"
|
||||
assert i.build_command_invocation("plan") == "/speckit-plan"
|
||||
|
||||
def test_forge_extension_command_hyphenated(self):
|
||||
from specify_cli.integrations import get_integration
|
||||
i = get_integration("forge")
|
||||
assert i.build_command_invocation("speckit.git.commit") == "/speckit-git-commit"
|
||||
assert (
|
||||
i.build_command_invocation("speckit.git.commit", "fix typo")
|
||||
== "/speckit-git-commit fix typo"
|
||||
)
|
||||
|
||||
|
||||
class TestResolveCommandRefs:
|
||||
"""Tests for __SPECKIT_COMMAND_<NAME>__ placeholder resolution."""
|
||||
|
||||
@@ -922,6 +922,57 @@ class TestCatalogSourceManagement:
|
||||
assert str(cfg_path) in message
|
||||
assert "expected a mapping" in message
|
||||
|
||||
def test_add_catalog_rejects_inf_priority_in_existing_entry(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
# ``priority: .inf`` loads as float('inf'); int() on it raises
|
||||
# OverflowError, which used to escape the IntegrationValidationError
|
||||
# contract as a raw traceback (github/spec-kit#3526 fixed the sibling
|
||||
# workflow/step loaders the same way).
|
||||
self._isolate(tmp_path, monkeypatch)
|
||||
cfg_path = tmp_path / ".specify" / "integration-catalogs.yml"
|
||||
cfg_path.write_text(
|
||||
yaml.dump(
|
||||
{
|
||||
"catalogs": [
|
||||
{
|
||||
"url": "https://a.example.com/catalog.json",
|
||||
"priority": float("inf"),
|
||||
}
|
||||
]
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
cat = IntegrationCatalog(tmp_path)
|
||||
with pytest.raises(
|
||||
IntegrationValidationError, match="must be an integer"
|
||||
):
|
||||
cat.add_catalog("https://new.example.com/catalog.json")
|
||||
|
||||
def test_remove_catalog_tolerates_inf_priority(self, tmp_path, monkeypatch):
|
||||
# Building the remove display order must not crash on a ``priority:
|
||||
# .inf`` entry; it falls back to positional order like the other
|
||||
# non-integer priorities do.
|
||||
self._isolate(tmp_path, monkeypatch)
|
||||
cfg_path = tmp_path / ".specify" / "integration-catalogs.yml"
|
||||
cfg_path.write_text(
|
||||
yaml.dump(
|
||||
{
|
||||
"catalogs": [
|
||||
{
|
||||
"url": "https://a.example.com/catalog.json",
|
||||
"priority": float("inf"),
|
||||
},
|
||||
{"url": "https://b.example.com/catalog.json", "priority": 2},
|
||||
]
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
cat = IntegrationCatalog(tmp_path)
|
||||
cat.remove_catalog(0) # must not raise OverflowError
|
||||
|
||||
def test_add_catalog_skips_blank_url_entries(self, tmp_path, monkeypatch):
|
||||
self._isolate(tmp_path, monkeypatch)
|
||||
cfg_path = tmp_path / ".specify" / "integration-catalogs.yml"
|
||||
|
||||
@@ -4542,6 +4542,36 @@ class TestCatalogStack:
|
||||
catalog.get_active_catalogs()
|
||||
assert str(config_path) in str(exc_info.value)
|
||||
|
||||
def test_load_catalog_config_rejects_infinite_priority(self, temp_dir):
|
||||
"""A ``priority: .inf`` yields a clean validation error, not an uncaught
|
||||
OverflowError from int(float('inf'))."""
|
||||
import yaml as yaml_module
|
||||
|
||||
project_dir = self._make_project(temp_dir)
|
||||
config_path = project_dir / ".specify" / "extension-catalogs.yml"
|
||||
config_path.write_text(
|
||||
yaml_module.dump(
|
||||
{
|
||||
"catalogs": [
|
||||
{
|
||||
"name": "inf-priority",
|
||||
"url": "https://example.com/catalog.json",
|
||||
"priority": float("inf"),
|
||||
}
|
||||
]
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
catalog = ExtensionCatalog(project_dir)
|
||||
|
||||
with pytest.raises(
|
||||
ValidationError, match="Invalid priority|expected integer"
|
||||
) as exc_info:
|
||||
catalog.get_active_catalogs()
|
||||
assert str(config_path) in str(exc_info.value)
|
||||
|
||||
def test_load_catalog_config_defaults_blank_names(self, temp_dir):
|
||||
"""Blank and null names normalize by valid catalog order."""
|
||||
import yaml as yaml_module
|
||||
|
||||
@@ -2671,6 +2671,24 @@ class TestPresetCatalogMultiCatalog:
|
||||
with pytest.raises(PresetValidationError, match="Invalid priority|expected integer"):
|
||||
catalog._load_catalog_config(config_path)
|
||||
|
||||
def test_load_catalog_config_rejects_infinite_priority(self, project_dir):
|
||||
"""A ``priority: .inf`` yields a clean validation error, not an uncaught
|
||||
OverflowError from int(float('inf'))."""
|
||||
config_path = project_dir / ".specify" / "preset-catalogs.yml"
|
||||
config_path.write_text(yaml.dump({
|
||||
"catalogs": [
|
||||
{
|
||||
"name": "inf-priority",
|
||||
"url": "https://example.com/catalog.json",
|
||||
"priority": float("inf"),
|
||||
}
|
||||
]
|
||||
}))
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
with pytest.raises(PresetValidationError, match="Invalid priority|expected integer"):
|
||||
catalog._load_catalog_config(config_path)
|
||||
|
||||
def test_load_catalog_config_install_allowed_string(self, project_dir):
|
||||
"""Test that install_allowed accepts string values."""
|
||||
config_path = project_dir / ".specify" / "preset-catalogs.yml"
|
||||
|
||||
@@ -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."""
|
||||
@@ -2684,6 +2709,39 @@ class TestFanOutStep:
|
||||
assert result.status == StepStatus.COMPLETED
|
||||
assert result.output["item_count"] == 0
|
||||
|
||||
def test_execute_non_dict_step_fails_loudly(self):
|
||||
"""A truthy non-mapping ``step`` must fail the step, not crash the run.
|
||||
|
||||
``validate`` rejects a non-dict ``step``, but the engine's ``execute()``
|
||||
does not auto-validate (see ``WorkflowEngine.load_workflow``). On a
|
||||
COMPLETED fan-out the engine reads ``step_template`` back out and, when
|
||||
it is truthy, calls ``template.get("id", ...)`` in ``_run_fan_out``. A
|
||||
truthy non-mapping ``step`` (a scalar or list authoring mistake) raised
|
||||
AttributeError there and took down the whole run. Mirrors the fan-out
|
||||
non-list ``items`` guard and the switch non-dict ``cases`` guard.
|
||||
"""
|
||||
from specify_cli.workflows.steps.fan_out import FanOutStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
step = FanOutStep()
|
||||
ctx = StepContext(steps={"tasks": {"output": {"task_list": [1, 2]}}})
|
||||
# ``None`` is an explicit ``step: null``: ``config.get("step", {})`` only
|
||||
# substitutes the default for an *absent* key, so it reaches the guard
|
||||
# and must fail here too — matching ``validate``.
|
||||
for bad_step in (["impl"], "impl", 5, None):
|
||||
result = step.execute(
|
||||
{
|
||||
"id": "parallel",
|
||||
"items": "{{ steps.tasks.output.task_list }}",
|
||||
"step": bad_step,
|
||||
},
|
||||
ctx,
|
||||
)
|
||||
assert result.status == StepStatus.FAILED
|
||||
assert "'step' must be a" in (result.error or "")
|
||||
assert result.output["item_count"] == 0
|
||||
assert result.output["step_template"] == {}
|
||||
|
||||
def test_validate_missing_fields(self):
|
||||
from specify_cli.workflows.steps.fan_out import FanOutStep
|
||||
|
||||
@@ -2696,12 +2754,13 @@ class TestFanOutStep:
|
||||
from specify_cli.workflows.steps.fan_out import FanOutStep
|
||||
|
||||
step = FanOutStep()
|
||||
errors = step.validate({
|
||||
"id": "test",
|
||||
"items": "{{ x }}",
|
||||
"step": "not-a-dict",
|
||||
})
|
||||
assert any("'step' must be a mapping" in e for e in errors)
|
||||
for bad_step in ("not-a-dict", ["impl"], 5, None):
|
||||
errors = step.validate({
|
||||
"id": "test",
|
||||
"items": "{{ x }}",
|
||||
"step": bad_step,
|
||||
})
|
||||
assert any("'step' must be a mapping" in e for e in errors), bad_step
|
||||
|
||||
|
||||
class TestFanInStep:
|
||||
@@ -2784,6 +2843,34 @@ class TestFanInStep:
|
||||
assert "'wait_for' must be a list" in (result.error or "")
|
||||
assert result.output["results"] == []
|
||||
|
||||
@pytest.mark.parametrize("bad_entry", [["a", "b"], {"a": 1}, 123, None])
|
||||
def test_execute_non_string_wait_for_entry_fails_loudly(self, bad_entry):
|
||||
"""A ``wait_for`` list with a non-string entry must fail the step, not
|
||||
crash the run or silently produce a bogus join.
|
||||
|
||||
The whole-list guard (``test_execute_non_list_wait_for_fails_loudly``)
|
||||
and the engine's fan-in validation both already reject the list *shape*,
|
||||
but neither the step's ``execute`` nor the engine's runtime path guarded
|
||||
the list's *elements*. On an unvalidated run an unhashable entry
|
||||
(a list/dict from a YAML indentation slip like ``wait_for: [[a, b]]``)
|
||||
crashed ``context.steps.get(entry, ...)`` with a raw TypeError, while a
|
||||
hashable-but-non-string entry (``wait_for: [123]``) silently joined an
|
||||
empty ``{}`` and still reported COMPLETED — the same wiring bug the
|
||||
list-shape guard exists to prevent. Mirrors the engine's
|
||||
``test_non_string_wait_for_entry_is_rejected`` load-time check.
|
||||
"""
|
||||
from specify_cli.workflows.steps.fan_in import FanInStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
step = FanInStep()
|
||||
ctx = StepContext(steps={"a": {"output": {"x": 1}}})
|
||||
# A valid entry alongside the bad one proves it is the entry, not the
|
||||
# list, that is rejected.
|
||||
result = step.execute({"id": "collect", "wait_for": ["a", bad_entry]}, ctx)
|
||||
assert result.status == StepStatus.FAILED
|
||||
assert "'wait_for' entries must be step-id strings" in (result.error or "")
|
||||
assert result.output["results"] == []
|
||||
|
||||
def test_validate_empty_wait_for(self):
|
||||
from specify_cli.workflows.steps.fan_in import FanInStep
|
||||
|
||||
@@ -5775,6 +5862,26 @@ class TestWorkflowCatalog:
|
||||
assert len(entries) == 1
|
||||
assert entries[0].name == "custom"
|
||||
|
||||
@pytest.mark.parametrize("bad_priority", [True, False, float("inf")])
|
||||
def test_config_priority_bool_or_inf_rejected(self, project_dir, bad_priority):
|
||||
"""`priority: true` must not be silently coerced to 1, and `priority: .inf`
|
||||
must not crash with an uncaught OverflowError — both raise a clean
|
||||
validation error (parity with the base CatalogStackBase loader)."""
|
||||
from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError
|
||||
|
||||
config_path = project_dir / ".specify" / "workflow-catalogs.yml"
|
||||
config_path.write_text(yaml.dump({
|
||||
"catalogs": [{
|
||||
"name": "bad",
|
||||
"url": "https://example.com/wf-catalog.json",
|
||||
"priority": bad_priority,
|
||||
"install_allowed": True,
|
||||
}]
|
||||
}))
|
||||
catalog = WorkflowCatalog(project_dir)
|
||||
with pytest.raises(WorkflowValidationError, match="Invalid priority|expected integer"):
|
||||
catalog.get_active_catalogs()
|
||||
|
||||
def test_validate_url_http_rejected(self, project_dir):
|
||||
from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError
|
||||
|
||||
@@ -5878,6 +5985,31 @@ class TestWorkflowCatalog:
|
||||
assert len(data["catalogs"]) == 1
|
||||
assert data["catalogs"][0]["url"] == "https://example.com/new-catalog.json"
|
||||
|
||||
def test_add_catalog_with_existing_inf_priority(self, project_dir):
|
||||
"""add_catalog() derives the new priority from existing ones via
|
||||
_coerce_priority; an existing `priority: .inf` must not crash it
|
||||
(int(float('inf')) is an OverflowError) — it is treated as 0 and the add
|
||||
succeeds."""
|
||||
from specify_cli.workflows.catalog import WorkflowCatalog
|
||||
|
||||
config_path = project_dir / ".specify" / "workflow-catalogs.yml"
|
||||
config_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
config_path.write_text(yaml.dump({
|
||||
"catalogs": [{
|
||||
"name": "existing",
|
||||
"url": "https://a.example.com/c.json",
|
||||
"priority": float("inf"),
|
||||
"install_allowed": True,
|
||||
}]
|
||||
}))
|
||||
|
||||
catalog = WorkflowCatalog(project_dir)
|
||||
catalog.add_catalog("https://b.example.com/c.json", "new")
|
||||
|
||||
data = yaml.safe_load(config_path.read_text())
|
||||
new = next(c for c in data["catalogs"] if c["url"] == "https://b.example.com/c.json")
|
||||
assert new["priority"] == 1 # max(inf coerced to 0) + 1
|
||||
|
||||
def test_add_catalog_duplicate_rejected(self, project_dir):
|
||||
from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError
|
||||
|
||||
@@ -6298,6 +6430,25 @@ class TestStepCatalog:
|
||||
assert len(entries) == 1
|
||||
assert entries[0].name == "custom"
|
||||
|
||||
@pytest.mark.parametrize("bad_priority", [True, False, float("inf")])
|
||||
def test_config_priority_bool_or_inf_rejected(self, project_dir, bad_priority):
|
||||
"""`priority: true`/`.inf` in a step-catalog config raise a clean
|
||||
validation error instead of coercing to 1 / crashing with OverflowError."""
|
||||
from specify_cli.workflows.catalog import StepCatalog, StepValidationError
|
||||
|
||||
config_path = project_dir / ".specify" / "step-catalogs.yml"
|
||||
config_path.write_text(yaml.dump({
|
||||
"catalogs": [{
|
||||
"name": "bad",
|
||||
"url": "https://example.com/step-catalog.json",
|
||||
"priority": bad_priority,
|
||||
"install_allowed": True,
|
||||
}]
|
||||
}))
|
||||
catalog = StepCatalog(project_dir)
|
||||
with pytest.raises(StepValidationError, match="Invalid priority|expected integer"):
|
||||
catalog.get_active_catalogs()
|
||||
|
||||
def test_validate_url_http_rejected(self, project_dir):
|
||||
from specify_cli.workflows.catalog import StepCatalog, StepValidationError
|
||||
|
||||
@@ -6393,6 +6544,30 @@ class TestStepCatalog:
|
||||
assert len(data["catalogs"]) == 1
|
||||
assert data["catalogs"][0]["url"] == "https://example.com/new-steps.json"
|
||||
|
||||
def test_add_catalog_with_existing_inf_priority(self, project_dir):
|
||||
"""Step-catalog add_catalog() must not crash when an existing entry has a
|
||||
`priority: .inf` (int(float('inf')) is an OverflowError) — _coerce_priority
|
||||
treats it as 0 and the add succeeds."""
|
||||
from specify_cli.workflows.catalog import StepCatalog
|
||||
|
||||
config_path = project_dir / ".specify" / "step-catalogs.yml"
|
||||
config_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
config_path.write_text(yaml.dump({
|
||||
"catalogs": [{
|
||||
"name": "existing",
|
||||
"url": "https://a.example.com/s.json",
|
||||
"priority": float("inf"),
|
||||
"install_allowed": True,
|
||||
}]
|
||||
}))
|
||||
|
||||
catalog = StepCatalog(project_dir)
|
||||
catalog.add_catalog("https://b.example.com/s.json", "new")
|
||||
|
||||
data = yaml.safe_load(config_path.read_text())
|
||||
new = next(c for c in data["catalogs"] if c["url"] == "https://b.example.com/s.json")
|
||||
assert new["priority"] == 1 # max(inf coerced to 0) + 1
|
||||
|
||||
def test_add_catalog_empty_yaml_file(self, project_dir):
|
||||
"""An empty YAML config file should be treated as empty, not corrupted."""
|
||||
from specify_cli.workflows.catalog import StepCatalog
|
||||
@@ -13000,6 +13175,58 @@ steps:
|
||||
assert result.exit_code != 0
|
||||
assert "Run not found: nonexistent-run" in result.output
|
||||
|
||||
def test_status_json_not_found_error_goes_to_stderr(
|
||||
self, project_dir, monkeypatch, capsys
|
||||
):
|
||||
"""Under --json, the not-found/invalid-run error must go to stderr so the
|
||||
stdout JSON stream stays parseable (empty on the error path) — mirroring
|
||||
`workflow run`/`workflow resume`. Before this fix both handlers used the
|
||||
stdout console, corrupting a consumer's json.loads(stdout)."""
|
||||
import typer
|
||||
from specify_cli.workflows import _commands
|
||||
|
||||
(project_dir / ".specify" / "workflows").mkdir(parents=True, exist_ok=True)
|
||||
monkeypatch.setattr(
|
||||
_commands, "_require_specify_project", lambda: project_dir
|
||||
)
|
||||
with pytest.raises(typer.Exit) as exc:
|
||||
_commands.workflow_status("does-not-exist", json_output=True)
|
||||
assert exc.value.exit_code == 1
|
||||
captured = capsys.readouterr()
|
||||
assert "Run not found" in captured.err
|
||||
assert "Run not found" not in captured.out
|
||||
# stdout carries no partial/corrupt JSON on the error path.
|
||||
assert captured.out.strip() == ""
|
||||
|
||||
def test_status_json_invalid_run_error_goes_to_stderr(
|
||||
self, project_dir, monkeypatch, capsys
|
||||
):
|
||||
"""The ValueError handler (a malformed/invalid run state) must ALSO route
|
||||
to stderr under --json, not just the FileNotFoundError one — otherwise a
|
||||
regression there would silently corrupt the JSON stream and this suite
|
||||
wouldn't catch it."""
|
||||
import typer
|
||||
from specify_cli.workflows import _commands
|
||||
from specify_cli.workflows.engine import RunState
|
||||
|
||||
(project_dir / ".specify" / "workflows").mkdir(parents=True, exist_ok=True)
|
||||
monkeypatch.setattr(
|
||||
_commands, "_require_specify_project", lambda: project_dir
|
||||
)
|
||||
|
||||
def _raise_value_error(*args, **kwargs):
|
||||
raise ValueError("corrupt run state: bad status")
|
||||
|
||||
monkeypatch.setattr(RunState, "load", _raise_value_error)
|
||||
|
||||
with pytest.raises(typer.Exit) as exc:
|
||||
_commands.workflow_status("some-run", json_output=True)
|
||||
assert exc.value.exit_code == 1
|
||||
captured = capsys.readouterr()
|
||||
assert "corrupt run state" in captured.err
|
||||
assert "corrupt run state" not in captured.out
|
||||
assert captured.out.strip() == ""
|
||||
|
||||
def test_status_no_run_id_list_path_unaffected(self, project_dir, monkeypatch):
|
||||
"""The no-run-id list-all-runs path must remain unaffected by the
|
||||
new single-run ValueError boundary."""
|
||||
|
||||
Reference in New Issue
Block a user