From 3247cebb175abc22b67aec9f1b85c9c500598115 Mon Sep 17 00:00:00 2001 From: Markus Date: Thu, 27 Aug 2026 10:23:35 +0200 Subject: [PATCH] feat(workflows): add plugin slots Assisted-by: GitHub Copilot (model: gpt-5.6-terra, autonomous) --- docs/reference/workflows.md | 35 +++ src/specify_cli/workflows/__init__.py | 2 + src/specify_cli/workflows/engine.py | 9 +- .../workflows/steps/plugin/__init__.py | 58 +++++ tests/test_workflows.py | 4 +- tests/unit/test_bundler_references.py | 4 +- tests/workflows/test_plugin_step.py | 229 ++++++++++++++++++ workflows/ARCHITECTURE.md | 8 +- workflows/PUBLISHING.md | 2 +- workflows/README.md | 20 +- 10 files changed, 362 insertions(+), 9 deletions(-) create mode 100644 src/specify_cli/workflows/steps/plugin/__init__.py create mode 100644 tests/workflows/test_plugin_step.py diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index 3b838b7227..9bce8936a1 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -286,6 +286,40 @@ edits: Lower priority values have higher precedence. Change this overlay to `priority: 5` if it must win a conflict with the `add-lint` overlay above. It replaces the `review-plan` gate with a non-interactive command. +### Plugin slots (upstream extension points) + +Workflow authors can declare a named, no-op extension point with `type: plugin`: + +```yaml +- id: post-implement + type: plugin + name: "Post-implementation checks" +``` + +The step `id` is the unique overlay anchor; `name` is a required non-blank, +human-readable label only. An unfilled slot completes as a `skipped` step with +`output: {slot: }`, so subsequent steps continue normally. + +Fill a slot with a schema-valid overlay `replace` edit anchored on the step +`id`, not its `name`: + +```yaml +id: fill-post-implement +extends: my-workflow +edits: + - replace: post-implement + step: + id: post-implement + type: shell + run: "echo Run project-specific checks" +``` + +Reuse the slot's `id` when later expressions or `fan-in.wait_for` refer to it. +The replacement must also preserve every output key those later steps consume: +an unfilled plugin slot supplies only `steps..output.slot`. Plugin slots are +not supported inside `fan-out.step` templates because runtime-multiplied +templates cannot be overlay anchors. + ### Interaction with Bundles and Updates `specify workflow add ` installs the complete local workflow @@ -494,6 +528,7 @@ specify workflow run speckit -i spec="Build a kanban board with drag-and-drop ta | `prompt` | Send an arbitrary prompt to the AI coding agent | | `shell` | Execute a shell command and capture output | | `init` | Bootstrap a project (like `specify init`) | +| `plugin` | Named extension point; skipped when unfilled | | `gate` | Pause for human approval before continuing | | `if` | Conditional branching (then/else) | | `switch` | Multi-branch dispatch on an expression | diff --git a/src/specify_cli/workflows/__init__.py b/src/specify_cli/workflows/__init__.py index 0d1e101a9e..dae7ce3882 100644 --- a/src/specify_cli/workflows/__init__.py +++ b/src/specify_cli/workflows/__init__.py @@ -51,6 +51,7 @@ def _register_builtin_steps() -> None: from .steps.gate import GateStep from .steps.if_then import IfThenStep from .steps.init import InitStep + from .steps.plugin import PluginStep from .steps.prompt import PromptStep from .steps.shell import ShellStep from .steps.switch import SwitchStep @@ -63,6 +64,7 @@ def _register_builtin_steps() -> None: _register_step(GateStep()) _register_step(IfThenStep()) _register_step(InitStep()) + _register_step(PluginStep()) _register_step(PromptStep()) _register_step(ShellStep()) _register_step(SwitchStep()) diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index d17513cc0b..4ea3f58be0 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -139,7 +139,7 @@ def _get_valid_step_types() -> set[str]: if STEP_REGISTRY: return set(STEP_REGISTRY.keys()) return { - "command", "shell", "prompt", "gate", "if", "init", + "command", "shell", "prompt", "gate", "if", "init", "plugin", "switch", "while", "do-while", "fan-out", "fan-in", } @@ -432,6 +432,13 @@ def _validate_steps( step_errors = step_impl.validate(step_config) errors.extend(step_errors) + if step_type == "plugin" and inside_fan_out: + errors.append( + f"Plugin step {step_id!r} is not supported inside fan-out " + "templates because overlays cannot address runtime-multiplied " + "templates." + ) + # Validate optional `continue_on_error` field. The engine honours # this on any step that returns StepStatus.FAILED so the pipeline can route # around the failure via a downstream `if` or `switch` (or a diff --git a/src/specify_cli/workflows/steps/plugin/__init__.py b/src/specify_cli/workflows/steps/plugin/__init__.py new file mode 100644 index 0000000000..a3bbda61ff --- /dev/null +++ b/src/specify_cli/workflows/steps/plugin/__init__.py @@ -0,0 +1,58 @@ +"""Plugin step — a named, no-op workflow extension point. + +An upstream workflow declares a slot at the position where a downstream +project may extend it. The step ``id`` is the overlay anchor; ``name`` is only +the human-readable slot label. A project overlay fills the slot with the +standard ``replace`` operation on the slot step's ``id``. Unfilled slots are +skipped when the workflow runs. + +Example YAML:: + + # Upstream workflow + - id: post-implement + type: plugin + name: post-implement + + # .specify/workflows/overlays/my-workflow/fill-post-implement.yml + id: fill-post-implement + extends: my-workflow + edits: + - replace: post-implement + step: + id: post-implement + type: shell + run: echo "Run project-specific checks" +""" + +from __future__ import annotations + +from typing import Any + +from specify_cli.workflows.base import StepBase, StepContext, StepResult, StepStatus + + +class PluginStep(StepBase): + """Provide a named workflow extension point that skips when unfilled.""" + + type_key = "plugin" + + def execute(self, config: dict[str, Any], context: StepContext) -> StepResult: + return StepResult( + status=StepStatus.SKIPPED, + output={"slot": config.get("name")}, + ) + + def validate(self, config: dict[str, Any]) -> list[str]: + errors = super().validate(config) + name = config.get("name") + if name is None: + errors.append( + f"Plugin step {config.get('id', '?')!r} requires a 'name' field " + "(the slot label)." + ) + elif not isinstance(name, str) or not name.strip(): + errors.append( + f"Plugin step {config.get('id', '?')!r}: 'name' must be a " + "non-blank string." + ) + return errors diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d599f3c6a4..05d58312b3 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -4,7 +4,7 @@ - Step registry & auto-discovery - Base classes (StepBase, StepContext, StepResult) - Expression engine -- All 10 built-in step types +- All 12 built-in step types - Workflow definition loading & validation - Workflow engine execution & state persistence - Workflow catalog & registry @@ -108,7 +108,7 @@ def test_all_step_types_registered(self): expected = { "command", "shell", "prompt", "gate", "if", "switch", - "while", "do-while", "fan-out", "fan-in", "init", + "while", "do-while", "fan-out", "fan-in", "init", "plugin", } assert expected.issubset(set(STEP_REGISTRY.keys())) diff --git a/tests/unit/test_bundler_references.py b/tests/unit/test_bundler_references.py index b9ad426660..edb4160e60 100644 --- a/tests/unit/test_bundler_references.py +++ b/tests/unit/test_bundler_references.py @@ -27,7 +27,7 @@ def test_bundled_extension_resolves(tmp_path: Path): def test_builtin_step_type_resolves(tmp_path: Path): """A built-in step type must resolve, like a bundled extension. - Spec Kit ships 11 step types as built-ins registered in ``STEP_REGISTRY`` + Spec Kit ships 12 step types as built-ins registered in ``STEP_REGISTRY`` rather than as on-disk asset directories, so there is no ``_locate_bundled_step``. The ``steps`` branch of ``_resolved_locally`` only asked ``StepRegistry(root).is_installed()``, which tracks *community* step @@ -40,7 +40,7 @@ def test_builtin_step_type_resolves(tmp_path: Path): warnings: list[str] = [] check = make_reference_checker(root, allow_network=True, warnings=warnings) - for step_id in ("shell", "gate", "command", "if"): + for step_id in ("shell", "gate", "command", "if", "plugin"): assert step_id in BUILTIN_STEP_TYPES, step_id assert check(_ref("steps", step_id)) is None, step_id assert warnings == [] diff --git a/tests/workflows/test_plugin_step.py b/tests/workflows/test_plugin_step.py new file mode 100644 index 0000000000..c2bec26810 --- /dev/null +++ b/tests/workflows/test_plugin_step.py @@ -0,0 +1,229 @@ +"""Tests for the plugin workflow extension-point step.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +import yaml + +from specify_cli.workflows import BUILTIN_STEP_TYPES, get_step_type +from specify_cli.workflows.base import RunStatus, StepContext, StepStatus +from specify_cli.workflows.engine import WorkflowDefinition, WorkflowEngine, validate_workflow +from specify_cli.workflows.overlays import WorkflowResolver +from specify_cli.workflows.steps.plugin import PluginStep + + +def _workflow_data(steps: list[dict[str, object]]) -> dict[str, object]: + return { + "schema_version": "1.0", + "workflow": {"id": "plugin-workflow", "name": "Plugin Workflow", "version": "1.0.0"}, + "steps": steps, + } + + +def _write_workflow(project_root: Path, data: dict[str, object]) -> None: + workflow_dir = project_root / ".specify" / "workflows" / "plugin-workflow" + workflow_dir.mkdir(parents=True, exist_ok=True) + (workflow_dir / "workflow.yml").write_text( + yaml.safe_dump(data), encoding="utf-8" + ) + + +def _write_overlay(project_root: Path, data: dict[str, object]) -> None: + overlay_dir = ( + project_root / ".specify" / "workflows" / "overlays" / "plugin-workflow" + ) + overlay_dir.mkdir(parents=True, exist_ok=True) + (overlay_dir / "fill-slot.yml").write_text(yaml.safe_dump(data), encoding="utf-8") + + +def test_plugin_step_is_registered_as_builtin(): + step = get_step_type("plugin") + + assert isinstance(step, PluginStep) + assert step.type_key == "plugin" + assert "plugin" in BUILTIN_STEP_TYPES + + +def test_plugin_step_validate_returns_errors_for_malformed_names(): + step = PluginStep() + + assert any("missing required 'id'" in error for error in step.validate({})) + assert "requires a 'name' field" in step.validate({"id": "slot"})[0] + assert "requires a 'name' field" in step.validate({"id": "slot", "name": None})[0] + for name in ("", " ", 123): + errors = step.validate({"id": "slot", "name": name}) + assert len(errors) == 1 + assert "non-blank string" in errors[0] + assert step.validate({"id": "slot", "name": "lint"}) == [] + + +@pytest.mark.parametrize( + ("name", "expected_error"), + [ + (None, "requires a 'name' field"), + ("", "non-blank string"), + (" ", "non-blank string"), + (123, "non-blank string"), + ], +) +def test_plugin_step_errors_are_reported_through_workflow_validation( + name: object, expected_error: str +): + definition = WorkflowDefinition( + _workflow_data([{"id": "slot", "type": "plugin", "name": name}]) + ) + + errors = validate_workflow(definition) + + assert any("Plugin step 'slot'" in error for error in errors) + assert any(expected_error in error for error in errors) + + +def test_addressable_nested_plugin_step_validates_cleanly(): + definition = WorkflowDefinition( + _workflow_data( + [ + { + "id": "conditional", + "type": "if", + "condition": "true", + "then": [{"id": "slot", "type": "plugin", "name": "lint"}], + } + ] + ) + ) + + assert validate_workflow(definition) == [] + + +def test_plugin_step_skips_without_mutating_the_shared_instance(): + step = PluginStep() + before = vars(step).copy() + + result = step.execute({"id": "slot", "name": "lint"}, StepContext()) + + assert result.status is StepStatus.SKIPPED + assert result.output == {"slot": "lint"} + assert vars(step) == before + + +def test_unfilled_plugin_slot_is_persisted_and_does_not_halt_workflow(project_dir): + _write_workflow( + project_dir, + _workflow_data( + [ + {"id": "slot", "type": "plugin", "name": "post-implement"}, + {"id": "marker", "type": "shell", "run": "echo marker"}, + ] + ), + ) + engine = WorkflowEngine(project_dir) + + definition = engine.load_workflow("plugin-workflow") + assert engine.validate(definition) == [] + state = engine.execute(definition, run_id="plugin-run") + + assert state.status is RunStatus.COMPLETED + state_data = json.loads((state.runs_dir / "state.json").read_text(encoding="utf-8")) + assert state_data["step_results"]["slot"]["status"] == "skipped" + assert state_data["step_results"]["slot"]["output"] == {"slot": "post-implement"} + assert state_data["step_results"]["marker"]["status"] == "completed" + + log_entries = [ + json.loads(line) + for line in (state.runs_dir / "log.jsonl").read_text(encoding="utf-8").splitlines() + ] + skipped_events = [ + entry + for entry in log_entries + if entry["event"] == "step_completed" and entry["step_id"] == "slot" + ] + assert len(skipped_events) == 1 + assert skipped_events[0]["status"] == "skipped" + + +def test_overlay_replaces_plugin_slot_and_attributes_it_to_the_overlay(project_dir): + _write_workflow( + project_dir, + _workflow_data( + [ + {"id": "before", "type": "shell", "run": "echo before"}, + {"id": "slot", "type": "plugin", "name": "post-implement"}, + {"id": "after", "type": "shell", "run": "echo after"}, + ] + ), + ) + _write_overlay( + project_dir, + { + "id": "fill-slot", + "extends": "plugin-workflow", + "edits": [ + { + "replace": "slot", + "step": {"id": "slot", "type": "shell", "run": "echo filled"}, + } + ], + }, + ) + engine = WorkflowEngine(project_dir) + + definition = engine.load_workflow("plugin-workflow") + assert [step["id"] for step in definition.steps] == ["before", "slot", "after"] + assert definition.steps[1]["type"] == "shell" + assert engine.validate(definition) == [] + state = engine.execute(definition, run_id="filled-slot-run") + assert state.status is RunStatus.COMPLETED + assert "filled" in state.step_results["slot"]["output"]["stdout"] + + _definition, _layers, attribution = WorkflowResolver(project_dir).resolve_with_layers( + "plugin-workflow" + ) + sources = {step.step_id: step.source for step in attribution} + assert sources == { + "before": "base", + "slot": "project:fill-slot", + "after": "base", + } + + +def test_plugin_steps_are_rejected_inside_fan_out_templates(): + definition = WorkflowDefinition( + _workflow_data( + [ + { + "id": "fan", + "type": "fan-out", + "items": [], + "step": {"id": "slot", "type": "plugin", "name": "per-item"}, + } + ] + ) + ) + + errors = validate_workflow(definition) + + assert any( + "Plugin step 'slot' is not supported inside fan-out templates" in error + for error in errors + ) + + +def test_non_plugin_fan_out_templates_remain_valid(): + definition = WorkflowDefinition( + _workflow_data( + [ + { + "id": "fan", + "type": "fan-out", + "items": [], + "step": {"id": "template", "type": "shell", "run": "echo item"}, + } + ] + ) + ) + + assert validate_workflow(definition) == [] diff --git a/workflows/ARCHITECTURE.md b/workflows/ARCHITECTURE.md index 477c0968ae..d98284ed3f 100644 --- a/workflows/ARCHITECTURE.md +++ b/workflows/ARCHITECTURE.md @@ -19,6 +19,7 @@ flowchart TD G --> H{Step type?} H -- command --> I["CommandStep.execute()"] H -- shell --> J["ShellStep.execute()"] + H -- plugin --> V["PluginStep.execute()"] H -- gate --> K["GateStep.execute()"] H -- "if" --> L["IfThenStep.execute()"] H -- switch --> M["SwitchStep.execute()"] @@ -27,12 +28,13 @@ flowchart TD I --> P{Result status?} J --> P + V --> P K --> P L --> P M --> P N --> P O --> P - P -- COMPLETED --> Q{Has next_steps?} + P -- "COMPLETED / SKIPPED" --> Q{Has next_steps?} P -- PAUSED --> R["Save state → exit"] P -- FAILED --> S["Log error → exit"] Q -- Yes --> G @@ -77,7 +79,7 @@ When a `gate` step pauses execution, the engine persists `current_step_index` an ## Step Types -The engine ships with 11 built-in step types, each in its own subpackage under `src/specify_cli/workflows/steps/`: +The engine ships with 12 built-in step types, each in its own subpackage under `src/specify_cli/workflows/steps/`: | Type Key | Class | Purpose | Returns `next_steps`? | |----------|-------|---------|-----------------------| @@ -85,6 +87,7 @@ The engine ships with 11 built-in step types, each in its own subpackage under ` | `prompt` | `PromptStep` | Send an arbitrary inline prompt to integration CLI | No | | `shell` | `ShellStep` | Run a shell command, capture output | No | | `init` | `InitStep` | Bootstrap a project (equivalent to `specify init`) | No | +| `plugin` | `PluginStep` | Named extension point; skipped when unfilled | No | | `gate` | `GateStep` | Interactive human review/approval | No (pauses in CI) | | `if` | `IfThenStep` | Conditional branching (then/else) | Yes | | `switch` | `SwitchStep` | Multi-branch dispatch on expression | Yes | @@ -200,6 +203,7 @@ src/specify_cli/ │ ├── command/ # Dispatch command to AI integration │ ├── shell/ # Run shell command │ ├── init/ # Bootstrap a project (specify init) +│ ├── plugin/ # Named, skipped-when-unfilled extension point │ ├── gate/ # Human review checkpoint │ ├── if_then/ # Conditional branching │ ├── prompt/ # Arbitrary inline prompts diff --git a/workflows/PUBLISHING.md b/workflows/PUBLISHING.md index 2caf55d810..662b2cd51d 100644 --- a/workflows/PUBLISHING.md +++ b/workflows/PUBLISHING.md @@ -90,7 +90,7 @@ steps: - ✅ `version` follows semantic versioning (X.Y.Z) - ✅ `description` is concise - ✅ All step IDs are unique -- ✅ Step types are valid: `command`, `prompt`, `shell`, `gate`, `if`, `switch`, `while`, `do-while`, `fan-out`, `fan-in` +- ✅ Step types are valid: `command`, `prompt`, `shell`, `init`, `plugin`, `gate`, `if`, `switch`, `while`, `do-while`, `fan-out`, `fan-in` - ✅ Required fields present per step type (e.g., `condition` for `if`, `expression` for `switch`) - ✅ Input types are valid: `string`, `number`, `boolean` - ✅ Step IDs do not contain `:` (reserved for engine-generated nested IDs like `parentId:childId`) diff --git a/workflows/README.md b/workflows/README.md index bd0b767938..d019a497d0 100644 --- a/workflows/README.md +++ b/workflows/README.md @@ -78,7 +78,7 @@ specify workflow run speckit \ ## Step Types -Workflows support 11 built-in step types: +Workflows support 12 built-in step types: ### Command Steps (default) @@ -150,6 +150,24 @@ and resolves the integration from the step config or the workflow default: preset: healthcare-compliance # Optional preset ID ``` +### Plugin Steps + +Declare a named extension point that downstream projects can fill with a +workflow overlay. The slot is skipped when unfilled; its `id` is the overlay +anchor and `name` is a required human-readable label: + +```yaml +- id: post-implement + type: plugin + name: "Post-implementation checks" +``` + +Use an overlay `replace` edit anchored on `post-implement` to fill the slot. +Keep the same `id` when downstream expressions or fan-in steps reference it, +and preserve any output keys they consume. Plugin steps are invalid inside +`fan-out.step` templates because those runtime-multiplied templates cannot be +targeted by overlays. + ### Gate Steps Pause for human review. The workflow resumes when `specify workflow resume` is called: