Add workflow step catalog — community-installable step types#2394
Add workflow step catalog — community-installable step types#2394
Conversation
…nd tests Agent-Logs-Url: https://github.com/github/spec-kit/sessions/2885e646-477d-4df8-b9a3-06d8cb29e748 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
…e-effect' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a catalog/registry system for community-installable workflow step types, plus CLI commands to discover, install, and manage them alongside built-in steps.
Changes:
- Introduces
StepRegistryandStepCatalog(multi-source resolution + SHA256 cache) for step type distribution/management. - Adds dynamic filesystem-based loading of installed custom step packages into
STEP_REGISTRY. - Expands CLI with
specify workflow step …and adds tests and initial (empty) catalog JSON files.
Show a summary per file
| File | Description |
|---|---|
| workflows/step-catalog.json | Adds the built-in “official” step catalog scaffold (currently empty). |
| workflows/step-catalog.community.json | Adds the built-in “community” step catalog scaffold (currently empty). |
| src/specify_cli/workflows/catalog.py | Implements StepRegistry + StepCatalog with config resolution and caching. |
| src/specify_cli/workflows/init.py | Adds load_custom_steps(project_root) dynamic import/registration for installed step packages. |
| src/specify_cli/init.py | Adds Typer subcommands for listing/searching/installing/removing steps and managing step catalogs. |
| tests/test_workflows.py | Adds unit tests covering registry CRUD, catalog resolution/validation, search/info, and dynamic loading behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
src/specify_cli/init.py:5555
workflow_step_remove()buildsstep_dirfrom unvalidatedstep_idand thenshutil.rmtree(step_dir). A malicious value like../...could delete arbitrary directories outside.specify/workflows/steps. Add the same resolved-pathrelative_to()guard used byworkflow_remove/workflow_addbefore performing deletions.
step_dir = project_root / ".specify" / "workflows" / "steps" / step_id
if step_dir.exists():
import shutil
shutil.rmtree(step_dir)
- Files reviewed: 6/6 changed files
- Comments generated: 4
… failed-to-load display - Add resolve()+relative_to() path traversal guards in workflow_step_add and workflow_step_remove to prevent directory escape via step_id - Harden _is_url_cache_valid in both StepCatalog and WorkflowCatalog to coerce fetched_at to float and catch TypeError/ValueError - Check STEP_REGISTRY and StepRegistry before installing to prevent collisions with built-in step types or already-installed steps - Show 'Custom (installed, failed to load)' section in workflow step list for steps in the registry that failed to load into STEP_REGISTRY
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
src/specify_cli/workflows/catalog.py:1021
- StepCatalog.add_catalog()/remove_catalog() read YAML without handling yaml.YAMLError/OSError; those exceptions will bubble past the CLI handlers (which only catch StepValidationError) and produce a stack trace. Wrap YAML reads in try/except and re-raise as StepValidationError so the CLI consistently reports a user-facing error.
def remove_catalog(self, index: int) -> str:
"""Remove a catalog source by index (0-based). Returns the removed name."""
config_path = self.project_root / ".specify" / "step-catalogs.yml"
if not config_path.exists():
raise StepValidationError("No step catalog config file found.")
data = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {}
if not isinstance(data, dict):
raise StepValidationError(
"Catalog config file is corrupted (expected a mapping)."
)
- Files reviewed: 6/6 changed files
- Comments generated: 2
| def _load(self) -> dict[str, Any]: | ||
| """Load registry from disk or create default.""" | ||
| if self.registry_path.exists(): | ||
| try: | ||
| with open(self.registry_path, encoding="utf-8") as f: | ||
| return json.load(f) | ||
| except (json.JSONDecodeError, ValueError): | ||
| return {"schema_version": self.SCHEMA_VERSION, "steps": {}} | ||
| return {"schema_version": self.SCHEMA_VERSION, "steps": {}} | ||
|
|
| data: dict[str, Any] = {"catalogs": []} | ||
| if config_path.exists(): | ||
| raw = yaml.safe_load(config_path.read_text(encoding="utf-8")) | ||
| if not isinstance(raw, dict): | ||
| raise StepValidationError( | ||
| "Catalog config file is corrupted (expected a mapping)." | ||
| ) | ||
| data = raw | ||
|
|
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/github/spec-kit/sessions/0dca6393-f5a9-40de-bb5c-77ba6af033d2 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
…le test Agent-Logs-Url: https://github.com/github/spec-kit/sessions/0dca6393-f5a9-40de-bb5c-77ba6af033d2 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
Applied all changes from the review thread in commits
Three new tests cover these cases: malformed |
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (2)
src/specify_cli/workflows/init.py:113
- The module name passed to
spec_from_file_location()is derived fromtype_key. Iftype_keycontains characters like.it will create package-like names insys.modulesand can cause confusing import behavior/collisions. Safer approach: derive the module name from a hash of the path ortype_key(and/or sanitize to[a-zA-Z0-9_]+).
spec = _importlib_util.spec_from_file_location(
f"_speckit_custom_step_{type_key}", init_py
)
src/specify_cli/init.py:5664
workflow step infoalso callsload_custom_steps()to populateSTEP_REGISTRY, which executes installed step code as a side effect of a read-only command. Consider avoiding imports here (useStepRegistry+step.ymlmetadata), or require an explicit opt-in before executing third-party step code.
# Load custom steps so registry is up to date
from .workflows import load_custom_steps
load_custom_steps(project_root)
- Files reviewed: 6/6 changed files
- Comments generated: 6
| def load_custom_steps(project_root: Path) -> list[str]: | ||
| """Load community-installed custom step types into STEP_REGISTRY. | ||
|
|
||
| Scans ``.specify/workflows/steps/`` for installed step packages. | ||
| Each valid package must contain ``step.yml`` (with a ``step.type_key`` | ||
| field) and ``__init__.py`` (a ``StepBase`` subclass). | ||
|
|
||
| Returns a list of type_keys that were successfully loaded. | ||
| Silently skips packages that fail to import or validate. | ||
| """ |
| # Load custom steps if in a spec-kit project | ||
| custom_keys: set[str] = set() | ||
| if specify_dir.exists(): | ||
| from .workflows import load_custom_steps | ||
| loaded = load_custom_steps(project_root) | ||
| custom_keys.update(loaded) | ||
| # Also read registry for metadata |
| step_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| try: | ||
| step_yml_content = _safe_fetch(step_yml_url) | ||
| init_py_content = _safe_fetch(init_url) | ||
| except Exception as exc: | ||
| import shutil | ||
| shutil.rmtree(step_dir, ignore_errors=True) | ||
| console.print(f"[red]Error:[/red] Failed to download step files: {exc}") |
| parsed = urlparse(url) | ||
| is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1") | ||
| if parsed.scheme != "https" and not ( | ||
| parsed.scheme == "http" and is_localhost | ||
| ): |
| self.cache_dir.mkdir(parents=True, exist_ok=True) | ||
| with open(cache_file, "w", encoding="utf-8") as f: | ||
| json.dump(data, f, indent=2) | ||
| with open(meta_file, "w", encoding="utf-8") as f: | ||
| json.dump({"url": entry.url, "fetched_at": time.time()}, f) |
| # Make it unreadable | ||
| registry.registry_path.chmod(0o000) | ||
| try: | ||
| registry2 = StepRegistry(project_dir) | ||
| assert registry2.list() == {} |
|
@copilot apply changes based on the comments in this thread |
…nce, no dynamic imports in list/info Agent-Logs-Url: https://github.com/github/spec-kit/sessions/3e18fef0-e2e6-4b3e-9e8d-9adb1e5e464e Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
…ubdirectory nesting Agent-Logs-Url: https://github.com/github/spec-kit/sessions/3e18fef0-e2e6-4b3e-9e8d-9adb1e5e464e Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
Applied all changes from the review thread in commits
|
The workflow engine shipped with a dynamic
STEP_REGISTRYbut no distribution mechanism for community-authored step types. This adds a full catalog system for discovering, installing, and managing custom step types, following the same patterns as the workflow/extension catalogs.New classes (
workflows/catalog.py)StepRegistry— persists installed custom steps in.specify/workflows/steps/step-registry.json; validates JSON shape on load and falls back to a clean default on corruption or unreadable filesStepCatalog— multi-source catalog stack with SHA256-based caching; resolvesSPECKIT_STEP_CATALOG_URLenv var →.specify/step-catalogs.yml→~/.specify/step-catalogs.yml→ built-in defaults (step-catalog.json+step-catalog.community.json)StepCatalogError/StepValidationError/StepCatalogEntrysupporting typesDynamic step loading (
workflows/__init__.py)load_custom_steps(project_root)scans.specify/workflows/steps/, dynamically imports each package's__init__.py, finds theStepBasesubclass matching the declaredtype_key, and registers it intoSTEP_REGISTRY. Broken packages are silently skipped. Dynamic imports are intentionally deferred to workflow execution time; CLI metadata commands (list,info) read fromStepRegistryandstep.ymlonly and never execute installed step code.CLI surface (
specify workflow step …)addvalidates that the downloadedstep.yml'stype_keymatches the catalog ID and that all fetches use HTTPS before writing anything to disk. Downloads happen in a temporary directory and are moved to the final location atomically only after all validation passes — a transient failure never corrupts or deletes a pre-existing step directory.Catalog files
workflows/step-catalog.json— official catalog (empty, ready for entries)workflows/step-catalog.community.json— community catalog (empty, ready for entries)Robustness & security hardening
step_idis validated against the resolved base directory in bothaddandremovebefore any filesystem operation.step addrejects IDs that collide with built-in or already-installed step types.step adddownloads and validates in a temp directory, then moves to the final location only on success; pre-existing directories are never deleted on failure._validate_catalog_url()checksparsed.hostname(not justparsed.netloc) to correctly reject hostless URLs such ashttps://:80/path.try/except OSErrorso a read-only filesystem never aborts a successful HTTP fetch.StepRegistry._load()validates the on-disk JSON shape and catchesOSError/UnicodeError, falling back to a clean default on any read failure.StepCatalog.add_catalog()treats an empty.specify/step-catalogs.ymlas an empty mapping rather than a corruption error.Tests
30+ tests across
TestStepRegistryCustom,TestStepCatalog, andTestLoadCustomStepscovering CRUD, catalog resolution (env var / project / user / default), URL validation, search, dynamic loading edge cases, registry shape validation, unreadable-file fallback, and empty YAML initialization. Thechmod-based unreadability test is skipped on Windows.