mirror of
https://github.com/github/spec-kit.git
synced 2026-07-04 04:45:43 +08:00
* Initial plan * Add workflow step catalog: StepRegistry, StepCatalog, CLI commands, and 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> * Potential fix for pull request finding 'An assert statement has a side-effect' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Address PR review: path traversal, cache robustness, collision check, 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 * Fix StepRegistry shape validation and StepCatalog empty-YAML handling 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> * Polish: rename _default to default_registry, strengthen unreadable-file 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> * Address PR review: atomic install, hostname validation, cache resilience, 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> * Fix shutil.move with existing step_dir: remove before move to avoid subdirectory 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> * Call load_custom_steps at execution time; enforce hostname in _safe_fetch and _validate_url Agent-Logs-Url: https://github.com/github/spec-kit/sessions/73865880-fb25-4061-a43e-4e4b4d1c4de6 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * Wrap YAML parsing in try/except; atomic step install via os.rename() under same fs Agent-Logs-Url: https://github.com/github/spec-kit/sessions/ff915bc5-ec7e-4e6a-b505-35f5795250df Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * Validate YAML root is a dict in _load_catalog_config and workflow_step_add; fix WorkflowCatalog hostname validation Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * Fix load_custom_steps() package imports and add reserved step ID validation * Move _re/_sys imports out of loop and _RESERVED_STEP_IDS to module level * Address review: collision-resistant module names, extra_files support, remove orphan dir * Harden extra_files: warn on non-dict, resolve symlinks in path traversal check * Switch _safe_fetch and StepCatalog._fetch_single_catalog to use open_url for auth consistency * Harden step_id validation against path-segment tricks; raise on StepRegistry.save() OSError * Clean up sys.modules on broken step packages; handle StepValidationError in step add/remove * Address review thread: int-coerce priorities, sys.modules cleanup, _require_specify_project, registry-first remove * fix: normalize workflow step catalog metadata fallbacks * fix: address latest workflow step and catalog review findings * Handle non-string extra_files keys in workflow step add * Harden StepRegistry symlink reads and extra_files path/URL validation * Harden custom step loader and step remove against symlinks and OSError * Fix StepCatalog.search() to coerce non-string fields before joining * Fix WorkflowCatalog YAML parsing error handling and isinstance checks * Harden step registry save and custom step/catalog ID handling * Harden cache validation and staging OSError handling * Address review: reorder symlink guard and split mixed test - Move symlink-parent check before is_dir() in load_custom_steps() so we never stat an external target through a symlink - Split test_get_merged_steps_normalizes_list_ids_to_strings into two focused tests: one for list-id normalization, one for get_step_info return values Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: symlink-before-stat in loader, restore registry on rmtree failure - load_custom_steps(): check is_symlink() before is_dir() on step directories so symlinked entries are skipped without statting external targets - workflow_step_remove: restore the registry entry when shutil.rmtree() fails so filesystem and registry state stay consistent and a future 'step add' isn't blocked Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden step_id validation and file-write error handling - _validate_step_id_or_exit: reject whitespace-only/padded IDs, Windows-invalid characters (<>:"|?*), control characters, trailing dots/spaces, and Windows reserved device names (con, nul, etc.) - Wrap step.yml/__init__.py staging writes in OSError handler - Wrap extra_files disk writes (mkdir + write_bytes) in OSError handler that names the failing relative path - Registry rollback on rmtree failure: restore verbatim metadata and emit a warning if the restore itself fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: cache symlink guard, verbatim registry rollback, Windows test fix - StepCatalog: add _is_cache_path_safe() guard that checks for symlinks in .specify/workflows/steps/.cache path; skip cache reads and writes when any component is symlinked to prevent writes outside project root - Registry rollback: write metadata directly to registry.data['steps'] and call save() instead of using add() which overwrites timestamps - temp_dir fixture: use ignore_errors=True on Windows to avoid flaky teardown from locked file handles (WinError 32) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Simplify exec_module call by removing redundant nested try/except Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix empty YAML tolerance in WorkflowCatalog.add_catalog, scope ignore_errors to Windows - WorkflowCatalog.add_catalog(): treat None from yaml.safe_load() (empty file) as an empty mapping instead of raising 'corrupted' - temp_dir fixture: limit ignore_errors to sys.platform == 'win32' so real cleanup issues surface on non-Windows platforms Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Chain exceptions in _load_catalog_config for both catalog classes Add 'from exc' to preserve root cause in tracebacks while keeping clean user-facing messages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make default catalog tests hermetic by isolating HOME Monkeypatch Path.home() to project_dir and clear catalog env vars so tests don't break on machines with a real ~/.specify/step-catalogs.yml or ~/.specify/workflow-catalogs.yml. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix falsy ID handling in _get_merged_steps for list-based catalogs Check for None explicitly instead of using 'or' which drops valid falsy IDs like 0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Compare reserved step IDs case-insensitively for filesystem safety On case-insensitive filesystems (Windows, common macOS), variants like STEP-REGISTRY.JSON would collide with the actual registry file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add explanatory comments to intentional empty except blocks Document why cache-read failures are silently ignored in both WorkflowCatalog and StepCatalog _fetch_single_catalog methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Co-authored-by: Manfred Riem <mnriem@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
202 lines
7.6 KiB
Python
202 lines
7.6 KiB
Python
"""Workflow engine for multi-step, resumable automation workflows.
|
|
|
|
Provides:
|
|
- ``StepBase`` — abstract base every step type must implement.
|
|
- ``StepContext`` — execution context passed to each step.
|
|
- ``StepResult`` — return value from step execution.
|
|
- ``STEP_REGISTRY`` — maps ``type_key`` to ``StepBase`` subclass instances.
|
|
- ``WorkflowEngine`` — orchestrator that loads, validates, and executes
|
|
workflow YAML definitions.
|
|
- ``load_custom_steps`` — loads community-installed step types into STEP_REGISTRY.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
from typing import TYPE_CHECKING
|
|
|
|
if TYPE_CHECKING:
|
|
from .base import StepBase
|
|
|
|
# Maps step type_key → StepBase instance.
|
|
STEP_REGISTRY: dict[str, StepBase] = {}
|
|
|
|
|
|
def _register_step(step: StepBase) -> None:
|
|
"""Register a step type instance in the global registry.
|
|
|
|
Raises ``ValueError`` for falsy keys and ``KeyError`` for duplicates.
|
|
"""
|
|
key = step.type_key
|
|
if not key:
|
|
raise ValueError("Cannot register step type with an empty type_key.")
|
|
if key in STEP_REGISTRY:
|
|
raise KeyError(f"Step type with key {key!r} is already registered.")
|
|
STEP_REGISTRY[key] = step
|
|
|
|
|
|
def get_step_type(type_key: str) -> StepBase | None:
|
|
"""Return the step type for *type_key*, or ``None`` if not registered."""
|
|
return STEP_REGISTRY.get(type_key)
|
|
|
|
|
|
# -- Register built-in step types ----------------------------------------
|
|
|
|
def _register_builtin_steps() -> None:
|
|
"""Register all built-in step types."""
|
|
from .steps.command import CommandStep
|
|
from .steps.do_while import DoWhileStep
|
|
from .steps.fan_in import FanInStep
|
|
from .steps.fan_out import FanOutStep
|
|
from .steps.gate import GateStep
|
|
from .steps.if_then import IfThenStep
|
|
from .steps.prompt import PromptStep
|
|
from .steps.shell import ShellStep
|
|
from .steps.switch import SwitchStep
|
|
from .steps.while_loop import WhileStep
|
|
|
|
_register_step(CommandStep())
|
|
_register_step(DoWhileStep())
|
|
_register_step(FanInStep())
|
|
_register_step(FanOutStep())
|
|
_register_step(GateStep())
|
|
_register_step(IfThenStep())
|
|
_register_step(PromptStep())
|
|
_register_step(ShellStep())
|
|
_register_step(SwitchStep())
|
|
_register_step(WhileStep())
|
|
|
|
|
|
_register_builtin_steps()
|
|
|
|
|
|
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.
|
|
"""
|
|
import hashlib as _hashlib
|
|
import importlib.util as _importlib_util
|
|
import re as _re
|
|
import sys as _sys
|
|
|
|
steps_dir = Path(project_root) / ".specify" / "workflows" / "steps"
|
|
|
|
# Defense-in-depth: refuse to execute step code from a symlinked
|
|
# parent directory under .specify/workflows/steps, which could redirect
|
|
# the import outside the project root and bypass the install-time
|
|
# symlink guard. Check symlinks *before* is_dir() since the latter
|
|
# follows symlinks and would stat an external target.
|
|
_current = Path(project_root)
|
|
for _part in (".specify", "workflows", "steps"):
|
|
_current = _current / _part
|
|
if _current.is_symlink():
|
|
return []
|
|
|
|
if not steps_dir.is_dir():
|
|
return []
|
|
|
|
loaded: list[str] = []
|
|
for step_dir in steps_dir.iterdir():
|
|
# Check symlinks before is_dir() since the latter follows symlinks
|
|
# and would stat an external target through a symlinked directory.
|
|
if step_dir.is_symlink():
|
|
continue
|
|
if not step_dir.is_dir():
|
|
continue
|
|
step_yml = step_dir / "step.yml"
|
|
init_py = step_dir / "__init__.py"
|
|
if step_yml.is_symlink() or init_py.is_symlink():
|
|
continue
|
|
if not step_yml.is_file() or not init_py.is_file():
|
|
continue
|
|
|
|
try:
|
|
import yaml as _yaml
|
|
|
|
meta = _yaml.safe_load(step_yml.read_text(encoding="utf-8")) or {}
|
|
step_meta = meta.get("step", {})
|
|
type_key = step_meta.get("type_key", "")
|
|
if not type_key:
|
|
continue
|
|
|
|
# Skip if already registered (e.g. built-in or previously loaded)
|
|
if type_key in STEP_REGISTRY:
|
|
continue
|
|
|
|
# Sanitize type_key so the synthetic module name is a valid identifier
|
|
# (e.g. "test-custom" → "_speckit_custom_step_test_custom_<hash>").
|
|
# The 8-char SHA-256 hash of the original type_key makes the name
|
|
# collision-resistant when different type_keys produce the same
|
|
# sanitized form (e.g. "a-b" and "a_b" both sanitize to "a_b" but
|
|
# have different hashes).
|
|
safe_key = _re.sub(r"[^A-Za-z0-9_]", "_", type_key)
|
|
key_hash = _hashlib.sha256(type_key.encode()).hexdigest()[:8]
|
|
module_name = f"_speckit_custom_step_{safe_key}_{key_hash}"
|
|
|
|
# Treat the step directory as a proper package so that relative
|
|
# imports inside the step (e.g. ``from .helpers import …``) work.
|
|
spec = _importlib_util.spec_from_file_location(
|
|
module_name,
|
|
init_py,
|
|
submodule_search_locations=[str(step_dir)],
|
|
)
|
|
if spec is None or spec.loader is None:
|
|
continue
|
|
module = _importlib_util.module_from_spec(spec)
|
|
module.__package__ = module_name
|
|
# Register before exec so relative imports resolve correctly.
|
|
_sys.modules[module_name] = module
|
|
registered = False
|
|
try:
|
|
spec.loader.exec_module(module) # type: ignore[union-attr]
|
|
|
|
# Find the StepBase subclass in the module
|
|
from .base import StepBase as _StepBase
|
|
|
|
step_class = None
|
|
for attr_name in dir(module):
|
|
attr = getattr(module, attr_name)
|
|
try:
|
|
if (
|
|
isinstance(attr, type)
|
|
and issubclass(attr, _StepBase)
|
|
and attr is not _StepBase
|
|
and getattr(attr, "type_key", "") == type_key
|
|
):
|
|
step_class = attr
|
|
break
|
|
except TypeError:
|
|
continue
|
|
|
|
if step_class is None:
|
|
continue
|
|
|
|
_register_step(step_class())
|
|
loaded.append(type_key)
|
|
registered = True
|
|
finally:
|
|
# If the step wasn't successfully registered (failed import,
|
|
# no matching StepBase subclass, or registration error), remove
|
|
# the synthetic module — and any submodules loaded via relative
|
|
# imports (e.g. ``from .helpers import …``) — from sys.modules so
|
|
# a broken/skipped step package leaves no lingering import state
|
|
# behind.
|
|
if not registered:
|
|
_sys.modules.pop(module_name, None)
|
|
submodule_prefix = module_name + "."
|
|
for _mod_key in [
|
|
k for k in _sys.modules if k.startswith(submodule_prefix)
|
|
]:
|
|
_sys.modules.pop(_mod_key, None)
|
|
except Exception: # noqa: BLE001
|
|
# Silently skip broken step packages at load time
|
|
continue
|
|
|
|
return loaded
|