Skip to content

Commit 38b7b17

Browse files
committed
Address tenth review: max_iterations validation, catalog config guard, version alignment
- Validate max_iterations is int >= 1 in while and do-while steps - Guard add_catalog against corrupted config (non-dict/non-list) - Align speckit_version requirement to >=0.6.1 (current package version) - Fan-out template validation uses separate seen_ids set to avoid false duplication errors with user-defined step IDs
1 parent 3932af0 commit 38b7b17

5 files changed

Lines changed: 30 additions & 6 deletions

File tree

src/specify_cli/workflows/catalog.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -455,11 +455,18 @@ def add_catalog(self, url: str, name: str | None = None) -> None:
455455

456456
data: dict[str, Any] = {"catalogs": []}
457457
if config_path.exists():
458-
data = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {
459-
"catalogs": []
460-
}
458+
raw = yaml.safe_load(config_path.read_text(encoding="utf-8"))
459+
if not isinstance(raw, dict):
460+
raise WorkflowValidationError(
461+
"Catalog config file is corrupted (expected a mapping)."
462+
)
463+
data = raw
461464

462465
catalogs = data.get("catalogs", [])
466+
if not isinstance(catalogs, list):
467+
raise WorkflowValidationError(
468+
"Catalog config 'catalogs' must be a list."
469+
)
463470
# Check for duplicate URL
464471
for cat in catalogs:
465472
if cat.get("url") == url:

src/specify_cli/workflows/engine.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,13 @@ def _validate_steps(
189189
if isinstance(default, list):
190190
_validate_steps(default, seen_ids, errors)
191191

192-
# Validate fan-out nested step
192+
# Validate fan-out nested step (template — not added to seen_ids
193+
# since the engine renames items to _fanout_<step>_<base>_<idx>)
193194
fan_step = step_config.get("step")
194195
if isinstance(fan_step, dict):
195-
_validate_steps([fan_step], seen_ids, errors)
196+
fan_errors: list[str] = []
197+
_validate_steps([fan_step], set(), fan_errors)
198+
errors.extend(fan_errors)
196199

197200

198201
# -- Run State Persistence ------------------------------------------------

src/specify_cli/workflows/steps/do_while/__init__.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ def validate(self, config: dict[str, Any]) -> list[str]:
4949
f"Do-while step {config.get('id', '?')!r} is missing "
5050
f"'max_iterations' field."
5151
)
52+
else:
53+
max_iter = config.get("max_iterations")
54+
if not isinstance(max_iter, int) or max_iter < 1:
55+
errors.append(
56+
f"Do-while step {config.get('id', '?')!r}: "
57+
f"'max_iterations' must be an integer >= 1."
58+
)
5259
nested = config.get("steps", [])
5360
if not isinstance(nested, list):
5461
errors.append(

src/specify_cli/workflows/steps/while_loop/__init__.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ def validate(self, config: dict[str, Any]) -> list[str]:
5656
f"While step {config.get('id', '?')!r} is missing "
5757
f"'max_iterations' field."
5858
)
59+
else:
60+
max_iter = config.get("max_iterations")
61+
if not isinstance(max_iter, int) or max_iter < 1:
62+
errors.append(
63+
f"While step {config.get('id', '?')!r}: "
64+
f"'max_iterations' must be an integer >= 1."
65+
)
5966
nested = config.get("steps", [])
6067
if not isinstance(nested, list):
6168
errors.append(

workflows/speckit/workflow.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ workflow:
88
integration: copilot
99

1010
requires:
11-
speckit_version: ">=0.7.0"
11+
speckit_version: ">=0.6.1"
1212
integrations:
1313
any: ["copilot", "claude", "gemini"]
1414

0 commit comments

Comments
 (0)