feat(workflows): install custom step types from local dirs and archives - #4757
markuswondrak wants to merge 2 commits into
Conversation
…es (github#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)
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Removal staging can reload uninstalled code, directory-copy safety has a symlink race, and unrelated composition documentation is included.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Adds local-directory and archive-URL installation for custom workflow steps, with shared validation, atomic registry persistence, runtime refresh, and removal handling.
Changes:
- Adds
--dev,--from, and--forceinstallation flows. - Centralizes package validation, staging, provenance, locking, and cleanup.
- Expands documentation and regression coverage.
| File | Description |
|---|---|
src/specify_cli/workflows/step/installer.py |
Implements shared installation and removal logic. |
src/specify_cli/workflows/step/command_add.py |
Adds local and archive CLI sources. |
src/specify_cli/workflows/step/command_remove.py |
Delegates transactional removal. |
src/specify_cli/workflows/step/command_info.py |
Displays installation provenance. |
src/specify_cli/workflows/step/_helpers.py |
Wraps shared installer validation. |
src/specify_cli/workflows/step/catalog/_domain.py |
Makes registry writes atomic. |
src/specify_cli/workflows/__init__.py |
Refreshes project-local step modules. |
docs/reference/workflows.md |
Documents custom packages and composition. |
docs/reference/bundles.md |
Documents bundle-local step limitations. |
tests/specify_cli/workflows/test_custom_steps.py |
Tests runtime step refresh. |
tests/specify_cli/workflows/step/test_installer.py |
Covers installer validation and failures. |
tests/specify_cli/workflows/step/test_command_add.py |
Covers new installation sources. |
tests/specify_cli/workflows/step/test_command_remove.py |
Covers removal behavior. |
tests/specify_cli/workflows/step/test_command_info.py |
Tests provenance output. |
tests/specify_cli/workflows/step/test_command_list.py |
Updates list-command tests. |
tests/specify_cli/workflows/step/test_command_search.py |
Updates search-command tests. |
tests/specify_cli/workflows/step/catalog/test_registry.py |
Tests atomic registry failure handling. |
tests/specify_cli/workflows/step/catalog/test_command_list.py |
Updates catalog-list tests. |
tests/specify_cli/bundles/test_primitives.py |
Tests bundle step delegation. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if stat.S_ISDIR(mode): | ||
| _copy(Path(entry.path), target) |
There was a problem hiding this comment.
@mnriem flagging this for you.
This is a real gap, but it is cross-cutting rather than specific to this PR, so I'd propose deferring it to a dedicated issue rather than resolving it here.
The repository has no shared safe primitive for installing from a live local directory. Archives are centralized and hardened (_download_security.safe_extract_archive: rejects symlinks/hardlinks/special files, blocks traversal, enforces bounds), but each local-directory installer rolls its own check-then-copy:
specify workflow add --devvalidates withos.walk(..., followlinks=False)+lstat, thenshutil.copytree(...)— andcopytreefollows symlinks.preset/extension --devuseshutil.copytreeessentially without traversal validation.- Overlays avoid recursive copy and read individual files.
So the same class of issue — and a weaker variant, since copytree dereferences symlinks — already exists in the sibling installers. Patching only step --dev here would add fd-level machinery to one caller while leaving the rest inconsistent. The coherent fix is one shared, fd-relative safe-copy primitive (O_DIRECTORY|O_NOFOLLOW + dir_fd traversal, symlink/special-file rejection, entry/size bounds) used by every local-directory install, together with the swap-at-boundary regression you describe.
Proposal: keep this PR scoped to #4695 and open a dedicated issue for that shared local-install primitive. I can file it if that direction sounds right.
AI-generated reply on behalf of @markuswondrak. Agent: opencode, model: deepseek-v4.1-flash, autonomous. Extent: review-response drafting.
| staged_dir = Path( | ||
| tempfile.mkdtemp( | ||
| prefix=f".{step_id}.removing-", dir=steps_base_dir | ||
| ) | ||
| ) |
| ### Workflow composition (`type: workflow`) | ||
|
|
||
| A `workflow` step runs an installed workflow as a **scoped subtree of the | ||
| current run** — there is one run, one run directory, and one process. The | ||
| included workflow behaves like a function call: values cross the boundary only |
mnriem
added
the
triage-can-wait
Summary
Implements #4695 only. This replacement for #4754 is based directly on current
main; it intentionally excludes the unrelated workflow-composition implementation from #4680.workflow step add --dev <directory>and--from <archive-url>with shared package validation, provenance, trust confirmation, and--forcehandling.Evidence
Regression coverage exercises YAML-native metadata rejection, atomic registry serialization failure, staged metadata changes, archive declaration mismatch after redirects, Rich markup package names, runtime refresh between projects, and bundle step delegation.
Validation
.venv/bin/python -m pytest tests/specify_cli/workflows/step tests/specify_cli/workflows/test_custom_steps.py tests/specify_cli/bundles/test_primitives.py tests/specify_cli/bundles/test_references.py -q185 passedAI Disclosure
Implementation was generated with OpenCode (models:
deepseek-v4.1-flashfor the original feature andgpt-5.6-terrafor remediation), autonomous mode. The AI authored code, tests, documentation, commits, and this PR description on behalf of @markuswondrak.