Skip to content

Commit 7056924

Browse files
committed
Address seventeenth review: project checks, catalog robustness
- Add .specify/ project check to workflow run/resume/status/search/info - remove_catalog validates config shape (dict + list) before indexing - _fetch_single_catalog validates response is a dict - _get_merged_workflows raises when all catalogs fail to fetch - add_catalog guards against non-dict catalog entries in config
1 parent c0eb5a5 commit 7056924

3 files changed

Lines changed: 44 additions & 5 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4221,6 +4221,9 @@ def workflow_run(
42214221
from .workflows.engine import WorkflowEngine
42224222

42234223
project_root = Path.cwd()
4224+
if not (project_root / ".specify").exists():
4225+
console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)")
4226+
raise typer.Exit(1)
42244227
engine = WorkflowEngine(project_root)
42254228
engine.on_step_start = lambda sid, label: console.print(f" \u25b8 [{sid}] {label} \u2026")
42264229

@@ -4285,6 +4288,9 @@ def workflow_resume(
42854288
from .workflows.engine import WorkflowEngine
42864289

42874290
project_root = Path.cwd()
4291+
if not (project_root / ".specify").exists():
4292+
console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)")
4293+
raise typer.Exit(1)
42884294
engine = WorkflowEngine(project_root)
42894295
engine.on_step_start = lambda sid, label: console.print(f" \u25b8 [{sid}] {label} \u2026")
42904296

@@ -4318,6 +4324,9 @@ def workflow_status(
43184324
from .workflows.engine import WorkflowEngine
43194325

43204326
project_root = Path.cwd()
4327+
if not (project_root / ".specify").exists():
4328+
console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)")
4329+
raise typer.Exit(1)
43214330
engine = WorkflowEngine(project_root)
43224331

43234332
if run_id:
@@ -4669,6 +4678,9 @@ def workflow_search(
46694678
from .workflows.catalog import WorkflowCatalog, WorkflowCatalogError
46704679

46714680
project_root = Path.cwd()
4681+
if not (project_root / ".specify").exists():
4682+
console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)")
4683+
raise typer.Exit(1)
46724684
catalog = WorkflowCatalog(project_root)
46734685

46744686
try:
@@ -4702,6 +4714,9 @@ def workflow_info(
47024714
from .workflows.engine import WorkflowEngine
47034715

47044716
project_root = Path.cwd()
4717+
if not (project_root / ".specify").exists():
4718+
console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)")
4719+
raise typer.Exit(1)
47054720

47064721
# Check installed first
47074722
registry = WorkflowRegistry(project_root)

src/specify_cli/workflows/catalog.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,11 @@ def _validate_catalog_url(url: str) -> None:
350350
f"Failed to fetch catalog from {entry.url}: {exc}"
351351
) from exc
352352

353+
if not isinstance(data, dict):
354+
raise WorkflowCatalogError(
355+
f"Catalog from {entry.url} is not a valid JSON object."
356+
)
357+
353358
# Write cache
354359
self.cache_dir.mkdir(parents=True, exist_ok=True)
355360
with open(cache_file, "w", encoding="utf-8") as f:
@@ -365,13 +370,15 @@ def _get_merged_workflows(
365370
"""Merge workflows from all active catalogs (lower priority number wins)."""
366371
catalogs = self.get_active_catalogs()
367372
merged: dict[str, dict[str, Any]] = {}
373+
fetch_errors = 0
368374

369375
# Process later/higher-numbered entries first so earlier/lower-numbered
370376
# entries overwrite them on workflow ID conflicts.
371377
for entry in reversed(catalogs):
372378
try:
373379
data = self._fetch_single_catalog(entry, force_refresh)
374380
except WorkflowCatalogError:
381+
fetch_errors += 1
375382
continue
376383
workflows = data.get("workflows", {})
377384
# Handle both dict and list formats
@@ -391,6 +398,10 @@ def _get_merged_workflows(
391398
wf_data["_catalog_name"] = entry.name
392399
wf_data["_install_allowed"] = entry.install_allowed
393400
merged[wf_id] = wf_data
401+
if fetch_errors == len(catalogs) and catalogs:
402+
raise WorkflowCatalogError(
403+
"All configured catalogs failed to fetch."
404+
)
394405
return merged
395406

396407
# -- Public API -------------------------------------------------------
@@ -467,15 +478,18 @@ def add_catalog(self, url: str, name: str | None = None) -> None:
467478
raise WorkflowValidationError(
468479
"Catalog config 'catalogs' must be a list."
469480
)
470-
# Check for duplicate URL
481+
# Check for duplicate URL (guard against non-dict entries)
471482
for cat in catalogs:
472-
if cat.get("url") == url:
483+
if isinstance(cat, dict) and cat.get("url") == url:
473484
raise WorkflowValidationError(
474485
f"Catalog URL already configured: {url}"
475486
)
476487

477488
# Derive priority from the highest existing priority + 1
478-
max_priority = max((cat.get("priority", 0) for cat in catalogs), default=0)
489+
max_priority = max(
490+
(cat.get("priority", 0) for cat in catalogs if isinstance(cat, dict)),
491+
default=0,
492+
)
479493
catalogs.append(
480494
{
481495
"name": name or f"catalog-{len(catalogs) + 1}",
@@ -498,7 +512,15 @@ def remove_catalog(self, index: int) -> str:
498512
raise WorkflowValidationError("No catalog config file found.")
499513

500514
data = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {}
515+
if not isinstance(data, dict):
516+
raise WorkflowValidationError(
517+
"Catalog config file is corrupted (expected a mapping)."
518+
)
501519
catalogs = data.get("catalogs", [])
520+
if not isinstance(catalogs, list):
521+
raise WorkflowValidationError(
522+
"Catalog config 'catalogs' must be a list."
523+
)
502524

503525
if index < 0 or index >= len(catalogs):
504526
raise WorkflowValidationError(
@@ -511,4 +533,6 @@ def remove_catalog(self, index: int) -> str:
511533
with open(config_path, "w", encoding="utf-8") as f:
512534
yaml.dump(data, f, default_flow_style=False, sort_keys=False, allow_unicode=True)
513535

514-
return removed.get("name", f"catalog-{index + 1}")
536+
if isinstance(removed, dict):
537+
return removed.get("name", f"catalog-{index + 1}")
538+
return f"catalog-{index + 1}"

workflows/speckit/workflow.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ inputs:
1818
prompt: "Feature name"
1919
integration:
2020
type: string
21-
default: "claude"
21+
default: "copilot"
2222
prompt: "Integration to use (e.g. claude, copilot, gemini)"
2323
scope:
2424
type: string

0 commit comments

Comments
 (0)