Skip to content

Commit c159f08

Browse files
markuswondrakMarkusCopilotCopilot
authored
feat(workflows): install custom step types from local dirs and archives (#4769)
* feat(workflows): install custom step types from local dirs and archives (#4695) `specify workflow step add` gains `--dev <directory>` and `--from <archive-url>` alongside the existing catalog source. All three converge on a new `step/installer.py` domain module that owns package validation (shape, symlink/special-file rejection, 512-file/50 MiB limits), same-filesystem staging with revalidation, atomic commit, `--force` replacement, and source-kind-only registry provenance. Direct URLs require a default-deny trust prompt before any request. Docs document the local-authoring flow and the deferred bundle-local limitation. Assisted-by: opencode (model: deepseek-v4.1-flash, autonomous) * fix(workflows): harden local step installation Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous) Assisted-by: OpenCode (model: deepseek-v4.1-flash, autonomous) * fix(workflows): address step install review findings Assisted-by: OpenCode (model: deepseek-v4.1-flash, autonomous) * fix(workflows): address remaining step install findings Allow archive byte-format detection without transport hints, escape user-controlled installer errors, reject case-folded orphan directories, and exercise install locking on Windows. Assisted-by: OpenCode (model: github-copilot/gpt-6-luna, autonomous) * fix(workflows): bound step package traversal depth and entries Replace recursive package validation and staging walks with iterative traversal so deeply nested --dev trees fail with StepInstallError instead of RecursionError. Enforce a 32-level directory depth limit and count directories toward the 512-entry package budget in both validation and copy, covering trees that change after validation. Assisted-by: GitHub Copilot CLI (model: unknown, Auto mode, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(workflows): compare resolved paths in force-reinstall failure mocks The installer operates on resolved step paths, so on macOS (where the temp directory lives under the /var -> /private/var symlink) the rmtree and os.replace mocks never matched the unresolved target and the expected StepInstallError was not raised. Compare resolved paths instead. Assisted-by: GitHub Copilot CLI (model: unknown, Auto mode, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update documentation on installed-package policy limits Clarify the impact of excluded entries on budget limits. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Refactor step ID collision handling in installer.py Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(workflows): serialize step remove with installs via shared project lock `workflow step remove` mutated the step directory and registry without the install lock, so a concurrent install could resurrect a removed entry or leave a newly installed package unregistered through a stale registry snapshot. - Extract the duplicated workflow/step lock logic into `shared_infra._exclusive_project_lock`; both install transactions now use it. - Run step removal under the step lock and resolve the registry and step directory only after the lock is acquired. - Report only lock-acquisition failures as lock errors; exceptions raised inside the critical section now propagate unchanged. - Add helper tests and deterministic remove-vs-install race tests (both orderings), plus lock-failure and rollback coverage. Assisted-by: GitHub Copilot CLI (model: Claude Opus 5.5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): bound step package traversal by the entry ceiling The 512-entry ceiling did not bound validation work: each directory listing was fully materialized and sorted before any entry was counted, and excluded subtrees (`.git`, `__pycache__`) were walked in full without consuming the budget, so a `--dev` checkout with a large `.git` cost unbounded time and memory. - Stream `os.scandir` through a shared helper that drops excluded names unread and raises the entry-limit error once the remaining budget is exceeded, before sorting. Validation and staging copy both use it. - Prune excluded directories instead of descending into them, matching `bundles/packager.py`. Their contents were already never staged, so a symlink or deep nesting inside `.git` no longer fails validation. - Update the workflow reference to describe the exclusion behavior. - Replace the symlink-inside-`.git` rejection test with pruning coverage and add read-count regressions for validation, a cross-directory budget, and the staging copy. Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous) * fix(workflows): escape step IDs in step remove and add messages Step IDs may legally contain `[`/`]`, so interpolating them unescaped into Rich markup rendered an ID such as `[red]step` as `step`. `step remove` printed the ID, step directory, and exception text raw on every path, and the discovery-only warning in `step add` did the same. - Escape the step ID, directory path, and exception text in all `step remove` output, matching `step info`. - Escape the step ID in the `step add` discovery-only catalog warning. - Add CLI regressions for the not-installed, lock-failure, orphan-removal, directory-delete-failure, and discovery-only paths. Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous) * fix(workflows): call the step package budget an entry limit The package budget counts files and directories together, but the validation, staging, and catalog preflight errors reported it as a "file limit". Report it as an "N-entry limit (files and directories combined)" and update the matching test assertions. Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous) * fix(workflows): report step lock failures once `step remove` shares the step lock with `step add`, but the shared helper labelled every acquisition failure as "Failed to lock step installation". `step remove` then added its own prefix, printing "Failed to lock step removal '<id>': Failed to lock step installation: <error>". - Make the helper's message operation-neutral ("Failed to acquire the step lock"), since install and remove both use it. - In `step remove`, report the underlying acquisition error after the removal prefix instead of the helper's message. - Assert a single lock message on the remove path, and add a `step add --dev` lock-failure regression. Both fail against the previous wording. Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous) * Update context string in project lock Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(bundles): restore failed step refreshes under the step lock When a bundle step refresh removes a step and the reinstall fails, the rollback reloaded `step-registry.json`, put the old entry back and saved, all without the step lock that `step add` and `step remove` hold. A concurrent step operation could commit between that reload and save, or save a stale snapshot over the restored entry, and lose a registry entry. - Run the rollback's directory and registry restore inside `_step_install_transaction`, reloading the registry after the lock is taken. - Skip the restore if the step was registered again after the removal, so a newer package and entry are not overwritten by the stale backup. - If the lock can't be acquired, leave the project untouched and add a note to the reinstall error instead of restoring unlocked. - Add regressions for a concurrent writer holding the lock during the rollback, a concurrent reinstall of the same step, and lock failure; all three fail against the previous rollback. Also cover `step remove` with a symlinked lock file, which fails against the old `step install` lock context. Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous) * revert(bundles): keep the step refresh rollback out of this PR Reverts the `bundles/primitives.py` part of c7f8a51 and its three rollback tests. The step refresh rollback is pre-existing code that this PR did not otherwise touch; locking it pulled the bundle refresh flow into review scope, and further findings there (snapshot and removal before the lock, restore failures masking the reinstall error) are also pre-existing on `main`. It belongs in a separate change for bundle refresh atomicity. The `step remove` symlinked-lock test from c7f8a51 is kept: it covers the step lock and removal locking that this PR adds. Assisted-by: OpenCode (model: Claude Opus 5.5, autonomous) --------- Co-authored-by: Markus <markus@example.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent dd5d50f commit c159f08

23 files changed

Lines changed: 4615 additions & 458 deletions

‎docs/reference/bundles.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ The source may also be a bundle directory or `.zip` artifact. Refresh uses the s
8181

8282
A local bundle source supplies the manifest, not its component payloads. Components resolved through catalogs still require network access to refresh, even when already installed. Add `--offline` only when the components being installed or refreshed ship with Spec Kit; otherwise the command reports which component needs network access. Re-run without `--offline` to fetch that component through its catalog.
8383

84+
> **Step payloads resolve through the step catalog only.** A bundle's `provides.steps` entries still resolve exclusively through the active step catalogs. Bundle-local `steps/<id>/` payloads and relative `provides.steps[].source` overrides are **not** resolved in this release, so a step declared that way cannot be installed offline. To ship a step with a bundle today, publish it to a step catalog the bundle's users can reach.
85+
8486
## Update Bundles
8587

8688
```bash

‎docs/reference/workflows.md‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,154 @@ specify workflow run speckit -i spec="Build a kanban board with drag-and-drop ta
582582

583583
> **Security note:** a `shell` step runs a local command with **your** privileges. There is no capability sandbox — `requires` is an advisory pre-condition block (spec-kit version, integrations), not a runtime gate, so it does **not** restrict what a step can do. In particular there is no `requires.permissions` capability gate: it is rejected by validation precisely because it would imply a sandbox that does not exist. Review any catalog or downloaded workflow before running it, and use a `gate` step to require explicit approval before sensitive or destructive shell commands.
584584

585+
### Custom step packages
586+
587+
Custom step types are installed with `specify workflow step`. A step is a
588+
directory package containing metadata and executable Python:
589+
590+
```text
591+
my-step/
592+
├── step.yml # required, at the package root
593+
├── __init__.py # required, at the package root
594+
└── helpers.py # optional nested modules and data files
595+
```
596+
597+
`step.yml` declares the step's identity. `step.type_key` must exactly match the
598+
`<step_id>` passed on the command line — the ID is never inferred from package
599+
content:
600+
601+
```yaml
602+
step:
603+
type_key: my-step
604+
name: My Step
605+
version: 0.1.0
606+
author: you
607+
description: What this step does
608+
```
609+
610+
`__init__.py` must define a `StepBase` subclass whose `type_key` matches:
611+
612+
```python
613+
from specify_cli.workflows.base import StepBase, StepResult
614+
615+
616+
class MyStep(StepBase):
617+
type_key = "my-step"
618+
619+
def execute(self, config, context):
620+
return StepResult(output={"ok": True})
621+
```
622+
623+
#### Install from a local directory
624+
625+
```bash
626+
specify workflow step add my-step --dev /path/to/my-step
627+
```
628+
629+
`--dev` takes a **directory** (not an archive, not a bare `step.yml`) that is a
630+
complete package. This needs no catalog, server, or network, which makes it the
631+
supported local-authoring loop:
632+
633+
```bash
634+
specify workflow step add my-step --dev ./my-step
635+
specify workflow step list
636+
specify workflow step info my-step
637+
# edit ./my-step, then replace the installed copy:
638+
specify workflow step add my-step --dev ./my-step --force
639+
specify workflow step remove my-step
640+
```
641+
642+
#### Install from an archive URL
643+
644+
```bash
645+
specify workflow step add my-step --from https://example.com/my-step.zip
646+
```
647+
648+
`--from` accepts a `.zip`, `.tar.gz`, or `.tgz` archive (a bare `step.yml`
649+
URL is **not** a package). The archive may place `step.yml` and `__init__.py`
650+
at its root or under exactly one top-level directory; unrelated top-level
651+
siblings are rejected. Because a step package contains executable Python, a
652+
direct URL install shows a default-deny trust confirmation before any network
653+
request; declining cancels with no request and no error. HTTPS is required
654+
(HTTP is permitted only for loopback hosts), redirects must remain secure, and
655+
downloads are size-bounded.
656+
657+
#### Install from the catalog
658+
659+
```bash
660+
specify workflow step add my-step
661+
```
662+
663+
Catalog installs resolve individual file URLs from the active step catalogs and
664+
then go through the same validation and commit path as `--dev` and `--from`.
665+
Discovery-only catalogs cannot be installed from.
666+
667+
#### Replacement and force
668+
669+
```bash
670+
specify workflow step add my-step --dev ./my-step --force
671+
specify workflow step add my-step --from https://example.com/my-step.zip --force
672+
```
673+
674+
`--force` first stages and validates the replacement before touching the
675+
existing installation, and can replace both a registered install and a leftover
676+
unregistered directory. Validation and staging failures leave the previous
677+
package untouched. If removing the old directory fails, the replacement is not
678+
published. If publishing the replacement or updating the registry fails after
679+
the old directory has been removed, the installation may be left incomplete:
680+
rerun the command with the original source and `--force` to reinstall. No
681+
automatic rollback is attempted.
682+
683+
#### Package validation
684+
685+
Every source is validated identically before anything is committed:
686+
687+
- `step.yml` and `__init__.py` must be regular, non-symlink files at the package
688+
root.
689+
- The package tree is copied recursively (relative imports, nested helper
690+
modules, and data files are supported). A symlinked package root, any
691+
descendant symlink, and any filesystem object that is not a regular file or
692+
directory are rejected.
693+
- `.git`, `__pycache__`, and `.DS_Store` entries are skipped without being
694+
inspected: they are not copied, excluded directories are not entered, and
695+
they do not count toward any limit.
696+
- The installed-package policy permits at most **512 retained entries** (files
697+
and directories combined), at most **32 levels** of directory nesting, and
698+
**50 MiB** of retained content.
699+
- Archive URLs also pass transport/extraction safety limits before package
700+
validation: at most 512 archive entries, 50 MiB downloaded or extracted, and
701+
10 MiB per archive member. Catalog files have a 50 MiB per-response bound.
702+
- Installation validates and copies the package but does **not** import or
703+
execute `__init__.py`. Installed custom step modules are loaded during startup
704+
of `workflow add`, `workflow run`, and `workflow resume`, before any particular
705+
custom step necessarily executes.
706+
707+
> **Security note:** Loading a custom step runs its Python with **your**
708+
> privileges. Only install and retain step packages from sources you trust.
709+
710+
#### Listing, running, and removing
711+
712+
Installed custom steps appear in `specify workflow step list` and are loaded
713+
automatically by `workflow add`, `workflow run`, and `workflow resume`. Remove
714+
one with:
715+
716+
```bash
717+
specify workflow step remove my-step
718+
```
719+
720+
#### Registry provenance
721+
722+
Each installed step records only the *kind* of its source — `catalog`
723+
(optionally with the catalog name), `local`, or `url`. Local paths and source
724+
URLs are never persisted. `specify workflow step info <id>` shows the source.
725+
726+
#### Bundle-local limitation
727+
728+
A bundle's `provides.steps` still resolves only through the active step
729+
catalogs. Bundle-local `steps/<id>/` payloads and relative
730+
`provides.steps[].source` overrides are **not** resolved in this release, so
731+
such steps are not installable offline. See the [Bundles reference](bundles.md).
732+
585733
### Per-Step Integration Configuration
586734

587735
Command steps may pass structured runtime configuration to integrations that

‎src/specify_cli/shared_infra.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import contextlib
56
import hashlib
67
import hmac
78
import logging
@@ -233,6 +234,60 @@ def _validate_safe_shared_directory(project_path: Path, directory: Path) -> None
233234
raise ValueError(f"Shared infrastructure directory escapes project root: {label}") from None
234235

235236

237+
@contextlib.contextmanager
238+
def _exclusive_project_lock(project_root: Path, lock_name: str, *, context: str):
239+
"""Hold an exclusive inter-process lock on ``.specify/<lock_name>``.
240+
241+
Callers use one lock file per mutable project resource so that its
242+
directory and registry changes are serialized across processes. Every
243+
failure to acquire the lock is raised as ``OSError``.
244+
"""
245+
project_root = Path(project_root)
246+
lock_dir = project_root / ".specify"
247+
try:
248+
_ensure_safe_shared_directory(
249+
project_root, lock_dir, context=f"{context} lock directory"
250+
)
251+
except ValueError as exc:
252+
raise OSError(str(exc)) from exc
253+
lock_file = lock_dir / lock_name
254+
if lock_file.is_symlink():
255+
raise OSError(f"Refusing to use symlinked {context} lock: {lock_file}")
256+
257+
flags = os.O_RDWR | os.O_CREAT
258+
flags |= getattr(os, "O_NOFOLLOW", 0)
259+
flags |= getattr(os, "O_CLOEXEC", 0)
260+
fd = os.open(lock_file, flags, 0o600)
261+
try:
262+
if lock_file.is_symlink():
263+
raise OSError(f"Refusing to use symlinked {context} lock: {lock_file}")
264+
# Call the lock primitives through their modules so tests can observe
265+
# contention by patching ``msvcrt.locking`` / ``fcntl.flock``.
266+
if os.name == "nt":
267+
import errno
268+
import msvcrt
269+
import time
270+
271+
if os.fstat(fd).st_size == 0:
272+
os.write(fd, b"\0")
273+
while True:
274+
os.lseek(fd, 0, os.SEEK_SET)
275+
try:
276+
msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)
277+
break
278+
except OSError as exc:
279+
if exc.errno not in (errno.EACCES, errno.EDEADLK):
280+
raise
281+
time.sleep(0.05)
282+
else:
283+
import fcntl
284+
285+
fcntl.flock(fd, fcntl.LOCK_EX)
286+
yield
287+
finally:
288+
os.close(fd)
289+
290+
236291
def _ensure_safe_shared_destination(
237292
project_path: Path,
238293
dest: Path,

‎src/specify_cli/workflows/__init__.py‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,27 @@ def _register_builtin_steps() -> None:
7575

7676
# The step types Spec Kit ships, snapshotted before any community step can be
7777
# loaded. ``load_custom_steps`` adds project-installed ids to the process-global
78-
# ``STEP_REGISTRY`` and never removes them, so ``STEP_REGISTRY`` cannot answer
78+
# ``STEP_REGISTRY`` and refreshes them for each project, so it cannot answer
7979
# "is this bundled with Spec Kit?" in a long-lived process: a step loaded for one
8080
# project would look built-in for the next. Callers that need the immutable set
8181
# (e.g. the bundler's reference checker) must use this instead.
8282
BUILTIN_STEP_TYPES: frozenset[str] = frozenset(STEP_REGISTRY)
83+
_CUSTOM_STEP_MODULES: set[str] = set()
84+
85+
86+
def _unload_custom_steps() -> None:
87+
"""Clear custom registrations and synthetic imports from a prior project."""
88+
import sys
89+
90+
for type_key in tuple(STEP_REGISTRY):
91+
if type_key not in BUILTIN_STEP_TYPES:
92+
del STEP_REGISTRY[type_key]
93+
for module_name in _CUSTOM_STEP_MODULES:
94+
sys.modules.pop(module_name, None)
95+
prefix = module_name + "."
96+
for loaded_name in [name for name in sys.modules if name.startswith(prefix)]:
97+
sys.modules.pop(loaded_name, None)
98+
_CUSTOM_STEP_MODULES.clear()
8399

84100

85101
def load_custom_steps(project_root: Path) -> list[str]:
@@ -97,6 +113,7 @@ def load_custom_steps(project_root: Path) -> list[str]:
97113
import re as _re
98114
import sys as _sys
99115

116+
_unload_custom_steps()
100117
steps_dir = Path(project_root) / ".specify" / "workflows" / "steps"
101118

102119
# Defense-in-depth: refuse to execute step code from a symlinked
@@ -192,6 +209,7 @@ def load_custom_steps(project_root: Path) -> list[str]:
192209
_register_step(step_class())
193210
loaded.append(type_key)
194211
registered = True
212+
_CUSTOM_STEP_MODULES.add(module_name)
195213
finally:
196214
# If the step wasn't successfully registered (failed import,
197215
# no matching StepBase subclass, or registration error), remove
@@ -206,7 +224,7 @@ def load_custom_steps(project_root: Path) -> list[str]:
206224
k for k in _sys.modules if k.startswith(submodule_prefix)
207225
]:
208226
_sys.modules.pop(_mod_key, None)
209-
except Exception: # noqa: BLE001
227+
except Exception: # noqa: BLE001, S112
210228
# Silently skip broken step packages at load time
211229
continue
212230

‎src/specify_cli/workflows/_commands.py‎

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -434,51 +434,12 @@ def _stage_workflow_file(
434434
@contextlib.contextmanager
435435
def _workflow_install_transaction(project_root: Path):
436436
"""Serialize workflow file swaps with their registry updates."""
437-
from ..shared_infra import _ensure_safe_shared_directory
437+
from ..shared_infra import _exclusive_project_lock
438438

439-
lock_dir = project_root / ".specify"
440-
try:
441-
_ensure_safe_shared_directory(
442-
project_root, lock_dir, context="workflow install lock directory"
443-
)
444-
except ValueError as exc:
445-
raise OSError(str(exc)) from exc
446-
lock_file = lock_dir / ".workflow-install.lock"
447-
if lock_file.is_symlink():
448-
raise OSError(f"Refusing to use symlinked workflow install lock: {lock_file}")
449-
450-
flags = os.O_RDWR | os.O_CREAT
451-
flags |= getattr(os, "O_NOFOLLOW", 0)
452-
flags |= getattr(os, "O_CLOEXEC", 0)
453-
fd = os.open(lock_file, flags, 0o600)
454-
try:
455-
if lock_file.is_symlink():
456-
raise OSError(
457-
f"Refusing to use symlinked workflow install lock: {lock_file}"
458-
)
459-
if os.name == "nt":
460-
import errno
461-
import msvcrt
462-
import time
463-
464-
if os.fstat(fd).st_size == 0:
465-
os.write(fd, b"\0")
466-
while True:
467-
os.lseek(fd, 0, os.SEEK_SET)
468-
try:
469-
msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)
470-
break
471-
except OSError as exc:
472-
if exc.errno not in (errno.EACCES, errno.EDEADLK):
473-
raise
474-
time.sleep(0.05)
475-
else:
476-
import fcntl
477-
478-
fcntl.flock(fd, fcntl.LOCK_EX)
439+
with _exclusive_project_lock(
440+
project_root, ".workflow-install.lock", context="workflow install"
441+
):
479442
yield
480-
finally:
481-
os.close(fd)
482443

483444

484445
def _commit_workflow_file(

0 commit comments

Comments
 (0)