github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-08-03 06:26:30 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat(workflows): WorkflowResolver standalone (PR 1) (#3557)

Browse Source 
* feat(workflows): add standalone WorkflowResolver and overlay subsystem

Implement PR 1 of the workflow-overlays plan: a concrete, standalone
WorkflowResolver for downstream workflow extensibility without touching the
Preset subsystem.

- Add overlay manifest schema (Overlay, OverlayEdit, validate_overlay_yaml)
- Add pure-function merge engine (find_step, apply_edit, merge_steps,
  validate_edits) with recursive anchor search and higher-wins semantics
- Add StepListComposer and tiered layer sources (project, installed, base)
- Add WorkflowResolver facade with inline HIGHER_WINS priority sorting
- Add CLI verbs: workflow overlay add/set-priority/enable/disable/remove/list
  and workflow resolve <id>
- Wire WorkflowEngine.load_workflow through WorkflowResolver
- Extend workflow add to copy optional overlays/ subdirectory from local
  workflow directories
- Add comprehensive unit, integration, and security tests

Refs: discussion #3473 (https://github.com/github/spec-kit/discussions/3473)

Assisted-by: Kimi (model: opencode-go/kimi-k2.7-code, autonomous)

* fix(workflows): reject symlinked overlay directories in layer sources

Address PR #3557 review comments r3594064534 and r3594064563:

- ProjectOverlaySource.collect now rejects symlinked per-workflow overlay
  directories (.specify/workflows/overlays/<id>) before iterating
- InstalledOverlaySource.collect now rejects symlinked installed overlay
  directories (.specify/workflows/<id>/overlays) before iterating
- workflow_overlay_list catches ValueError from resolver and exits with
  code 1 instead of crashing on unhandled exceptions
- Added .specify/workflows/overlays to _reject_unsafe_workflow_storage
  chokepoint for defense-in-depth

These guards prevent symlinked overlay directories from redirecting
auto-loaded overlay YAML to attacker-controlled content outside the
project, which could inject executable shell steps into trusted workflows.

Refs: PR #3557 review comments r3594064534, r3594064563

Assisted-by: opencode-go/qwen3.7-max (autonomous)

* fix(workflows): address Copilot review findings in merge engine

- Apply inserts before winning replace to prevent anchor-not-found errors
  when replace changes step ID (r3594064604)
- Track attribution recursively for nested steps in composite inserts/replaces
  so workflow resolve attributes all child steps correctly (r3594064638)
- Add regression tests for both fixes

Refs: PR #3557 review discussion

Assisted-by: GitHub Copilot (model: qwen3.7-plus, autonomous)

* refactor(workflows): simplify overlay architecture to 2-tier

Remove installed overlays tier to enforce clean separation of concerns:
- workflow add installs workflows only (no overlay copying)
- workflow overlay add installs overlays only (project-local)

Changes:
- Remove InstalledOverlaySource class and all references
- Remove overlay-copying logic from _validate_and_install_local()
- Update WorkflowResolver to 2-tier: project overlays + base workflow
- Fix --priority override timing: apply before validation, not after
- Remove tests for installed overlays (no longer applicable)

Rationale: If upstream controls both base workflow and shipped overlays,
and both get overwritten on bundle update, there's no reason to ship
overlays separately. Overlays only make sense when someone other than
the base author adds them.

Resolves all three review findings from PR #3557:
- r3594064677: workflow add no longer copies overlays from all call sites
- r3594064705: --priority override now applied before validation
- r3594064726: no stale installed overlays (tier removed entirely)

Assisted-by: Claude (model: claude-opus-4-7, autonomous)

* fix(workflows): harden overlay symlink handling

Assisted-by: GitHub Copilot (model: GPT-5.4, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(workflows): remove stale installed-overlay references from workflows.md

The 2-tier refactor (cc28185) removed the installed-overlay tier entirely,
but docs/reference/workflows.md was not updated. This commit addresses all
four Cluster 2 findings from the PR review:

- workflow add: remove sentence about copying overlays/ subdirectory
- How Overlays Work: drop installed-overlay table row and precedence prose;
  rewrite to 2-tier model (project overlays only, source-order tie-break)
- overlay remove: drop trailing sentence about installed overlays
- Interaction with Bundles: rewrite to say workflow add installs only
  workflow.yml; remove installed-overlay discovery language

Fixes: r3596368791, r3596368831, r3596368873, r3596368919

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(overlays): detect ancestor-conflict anchors in merge_steps

When two overlay edits target anchors that share a parent/descendant
relationship (e.g. remove an if-step + insert_after a nested child),
merge_steps processed them independently and in dict-insertion order,
making the outcome non-deterministic.

Add two private helpers to merge.py:
- _descendant_ids(step): returns all step IDs nested inside a step dict
  by delegating to the existing _all_base_step_ids helper on children.
- _check_anchor_conflicts(anchors, base_steps): for each targeted anchor
  finds its descendants and checks whether any other targeted anchor is
  among them; returns human-readable error strings.

Wire _check_anchor_conflicts into merge_steps immediately after
edits_by_anchor is built, before any tree mutation occurs. Raises
ValueError listing the conflicting anchor pair(s) so overlay authors
know exactly what to fix.

Add TestMergeStepsAncestorConflicts (6 cases):
- remove parent + insert_after child raises ValueError
- replace parent + remove child raises ValueError
- conflict across multiple overlays raises ValueError
- sibling anchors (not ancestor/descendant) pass
- single anchor passes
- parent targeted but child not targeted passes

Closes review comment r3596368746 (PR #3557, round 2, cluster 3).

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(overlays): fix over-broad conflict detection and non-deterministic ID collision

Finding 1.1 — _check_anchor_conflicts was rejecting any ancestor/descendant
anchor pair, including insert-only edits that are perfectly safe. Only
replace/remove on an ancestor can destroy its subtree and make a descendant
anchor unresolvable. Change the signature to accept a dict[str, str]
(anchor → winning operation) and skip the check for insert_after/insert_before.

Finding 1.2 — merge_steps was calling find_step on the already-mutated tree,
so a replacement step that reused a base step ID could be accidentally targeted
by a later edit group (non-deterministic result depending on dict iteration
order). Replace the anchor-group loop with a single-pass _traverse_and_apply
that walks the original tree structure and applies edits as each step is
encountered. Anchors are never re-looked up in a mutated tree.

Design invariant enforced: overlays always apply to the original base tree and
cannot target steps introduced by other overlays. Non-remove edits on non-base
anchors now raise ValueError early.

Also removes apply_edit (no production callers, only tested in isolation) and
its test class — the new traversal inlines the same mechanics without the
find_step round-trip.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(overlays): reject ID trailing newlines and reuse existing .yaml path

Fix two input validation bugs in the overlay layer (Group 2 of copilot
review PR #3557):

1. _validate_safe_id in schema.py used re.match() which anchors only at
   the start of the string, so IDs like 'overlay\n' passed validation
   and could produce newline-containing file paths. Changed to fullmatch()
   so the entire string must satisfy the pattern.

2. workflow_overlay_add always wrote <id>.yml without checking whether
   <id>.yaml already existed. Since the resolver loads both extensions,
   this created two active layers whose edits applied twice. Now uses
   the existing _find_overlay_file() to detect a pre-existing file and
   reuse its path, falling back to .yml only for new overlays.

Tests added for both fixes.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(overlays): fix display order inversion and wrap file-read errors

Finding group 3 from copilot-review-v2.md:

3.1 — Precedence display inverted (overlays/__init__.py)
collect_all_layers used a single-pass sort by (-priority, source_asc),
which placed the *losing* equal-priority source first in the display
while claiming "highest first". Fix: two-pass stable sort — source
descending then priority descending — so the actual winner (last applied
by the composer) rises to the top of the display.

3.2 — Unwrapped file-read errors (overlays/layer_sources.py)
Only yaml.YAMLError was caught around path.read_text(), so an
unreadable or non-UTF-8 overlay produced a raw traceback. Fix: widen
the except clause to (yaml.YAMLError, OSError, UnicodeDecodeError),
matching the pattern used throughout catalog.py.

Tests:
- test_workflow_resolve_equal_priority_winner_shown_first: verifies
  project:zzz (the winner) appears before project:aaa in workflow resolve
  output when both overlays share the same priority.
- tests/workflows/test_overlay_layer_sources.py (new): OSError and
  non-UTF-8 bytes both produce OverlayLoadError, not raw tracebacks.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: rename misleading overlay test

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: remove EOF blank line in overlay resolver

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: handle overlay read and enumeration errors

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(overlays): validate resolver workflow IDs

Reject unsafe and reserved workflow IDs before overlay or base sources construct paths, preventing traversal through resolver and engine fallback paths.

Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(overlays): drop _remove_sources_recursively from remove branch

In _traverse_and_apply, the remove branch called _remove_sources_recursively
to clean up attribution entries for the deleted step.  This was inherited from
the old apply_edit loop (c70a5d6) where it was needed because the sources dict
was queried exhaustively.

In the current single-pass design, _build_attribution only traverses the result
list, so stale sources entries for removed steps are never read.  The cleanup
call is therefore unnecessary — and actively harmful when another overlay has
replaced a different step with a new step that reuses the same ID: the pop
clobbers the replacement's attribution entry, causing workflow resolve to report
the surviving step as 'unknown'.

Fix: simply remove the _remove_sources_recursively call from the remove branch.
Add an attribution assertion to the existing reused-ID regression test to catch
this case.

Fixes: r3604242050 (Copilot review finding)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

* Fix CLI overlay ID validation anchoring

Use fullmatch for CLI workflow/overlay ID validation so trailing newlines are rejected consistently with manifest validation.

Add regression coverage for newline-suffixed workflow and overlay IDs in overlay set-priority.

Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(workflows): validate workflow_id in layer sources before path construction

ProjectOverlaySource.collect() and BaseWorkflowSource.collect() joined
workflow_id directly onto storage paths without validation, enabling
path traversal (e.g. '../../outside') when called outside the
WorkflowResolver.

Add _validate_workflow_id() to layer_sources.py — mirrors the same
_SAFE_ID_PATTERN / _RESERVED_WORKFLOW_IDS check used by WorkflowResolver
in overlays/__init__.py — and call it at the top of both collect()
methods before any path is constructed.

Adds parametrised tests covering unsafe IDs and verifying no filesystem
access occurs for an invalid ID.

Closes review finding r3604772700.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(workflows): align layer source validation with _safe_workflow_id_dir

Plan §4.1 requires that Workflow-ID-Validierung, Symlink-/Containment-
Prüfungen and Fehlerübersetzung must not diverge between workflow
management and the overlay resolver.

My previous fix added ID pattern + reserved-name validation to both
collect() methods but was missing the containment step and the
BaseWorkflowSource directory/file checks that _safe_workflow_id_dir
performs.

Changes:
- Add _ensure_contained_dir(path, root) to layer_sources.py — pure
  domain mirror of overlays/_commands.py::_ensure_contained_dir that
  raises OverlayLoadError instead of typer.Exit
- ProjectOverlaySource.collect(): replace two inline symlink/dir checks
  with _ensure_contained_dir(workflow_overlay_dir, self.overlays_dir),
  adding the missing resolve().relative_to() containment step
- BaseWorkflowSource.collect(): add _ensure_contained_dir on the
  workflow directory, and add workflow.yml symlink check before is_file()

The same logic now lives in three places (workflow CLI, overlay CLI,
layer sources). The DRY extraction to workflows/_validation.py is
deferred to PR 3 per plan §4.1.

Tests: add containment and symlink tests for both sources.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(overlays): resolve identity from manifest field, not filename

Align overlay identity resolution with the project-wide convention:
presets use preset.id, extensions use extension.id, workflows use
workflow.id, and workflow steps use step.type_key. Overlays must
derive identity from the manifest id field, not the filename.

Rewrite _find_overlay_file() to scan all YAML files in the overlay
directory and match on the manifest id field, fixing the bug where
enable/disable/remove/set-priority failed when filename != manifest id.

Closes: PR #3557 discussion r3605010197

Assisted-by: opencode-go/qwen3.7-max (autonomous)

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(workflows): make validation behavior consistent across YAML loading paths

Address PR #3557 review finding r3607632921:

- Wrap yaml.YAMLError → ValueError in from_yaml() and from_string() so
  malformed YAML matches the documented exception contract
- Add except ValueError to workflow_info to handle composition errors
  cleanly instead of crashing with a raw traceback
- Remove validate_workflow() from compose() so the resolver path is
  parse-only like all other YAML loading mechanisms; callers validate
  explicitly via engine.validate()
- Update test to reflect new behavior: resolve() returns composed
  definition, caller validates separately

Assisted-by: opencode-go/qwen3.7-max (autonomous)

* fix(overlays): list disabled overlays in management view

Keep disabled overlays visible in workflow overlay list while leaving resolution behavior unchanged.

- add an include_disabled opt-in to overlay source/resolver collection
- use include_disabled=True for workflow overlay list
- add regression tests for list visibility and default filtering

Assisted-by: GitHub Copilot (model: GPT-5.4, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* docs: align overlay extends and resolver contract

Assisted-by: GitHub Copilot (model: GPT-5.3-Codex, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: use atomic write for overlay file updates to prevent hard-link attack

Replace in-place write_text() calls in workflow_overlay_add() and
_update_overlay_field() with the same mkstemp → write → os.replace()
pattern used by the workflow installer (_stage_workflow_file /
_commit_workflow_file / _discard_staged_workflow_file).

The prior code rejected symlinks and validated path containment, but a
hard-linked destination file passes both checks while sharing an inode
with an external file. write_text() would then truncate and overwrite
that external inode. The atomic staging approach never opens the
existing destination for writing, eliminating the hard-link vector.

Fixes findings r3608669512 and r3608669517 on PR #3557.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, supervised)

* fix(composer): preserve invalid base definition instead of coercing steps to []

When 'steps' is not a list, returning early with the unmodified
WorkflowDefinition lets validate_workflow surface the proper error
("'steps' must be a list.") to the caller. The previous silent
coercion to [] masked the validation error entirely.

Fixes: https://github.com/github/spec-kit/pull/3557#discussion_r3608669506

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, supervised)

* fix: align workflow overlay priority semantics

Assisted-by: GitHub Copilot (model: GPT-5.6 Terra, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: validate overlay priority presentation

Assisted-by: GitHub Copilot (model: GPT-5.6 Terra, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: catch OverflowError in normalize_priority for float infinity values

YAML values like `priority: .inf` parse to float('inf'), causing
int() to raise OverflowError. This broke validate_overlay_yaml()'s
'validation never raises' contract. Adding OverflowError to the
except clause makes it fall back to the default priority (10),
consistent with other invalid value handling.

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

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>
This commit is contained in:
Markus Wondrak
2026-07-21 15:35:25 +02:00
committed by GitHub
parent 8a5bcc21a5
commit d6fa0460ed
19 changed files with 4938 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

186
docs/reference/workflows.md
View File

@@ -91,8 +91,192 @@ specify workflow add <source>
| `--dev` | Install from a local workflow YAML file or directory |
| `--from <url>` | Install from a custom URL (`<source>` names the expected workflow ID) |
Installs a workflow from the catalog, a URL (HTTPS required), or a local file path.
Installs a workflow from the catalog, a URL (HTTPS required), a local YAML file, or a local directory containing `workflow.yml`.
## Workflow Overlays
Workflow overlays let a project extend or override an installed workflow without editing the installed `workflow.yml`. This keeps local customizations safe across `specify bundle update` or `specify workflow add` upgrades.
When `specify workflow run <workflow-id>` loads a workflow, the engine composes the base workflow with all enabled overlays for that workflow id. The result is validated like any other workflow definition.
### How Overlays Work
An overlay is a YAML file that declares a set of edit operations against the step list of a base workflow. Overlays use lower-wins precedence: higher priority numbers are applied first and lower numbers last. Equal-priority overlays are applied alphabetically by ID, with the last ID winning conflicts.
Project overlay files live at:
| Location | Purpose |
| --- | --- |
| `.specify/workflows/overlays/<id>/*.yml` | Project-local customizations |
### Overlay File Format
The recommended edit format uses the operation name as the key and the anchor step id as the value:
```yaml
id: "my-overlay"
extends: "speckit"
priority: 10
enabled: true
edits:
- insert_after: implement
step:
id: run-lint
type: shell
run: "ruff check src/"
- replace: review-spec
step:
id: review-spec
type: gate
message: "Review the generated spec (overlay override)."
options: [approve, reject]
on_reject: abort
```
The explicit form is also supported:
```yaml
edits:
- operation: insert_after
anchor: implement
step:
id: run-lint
type: shell
run: "ruff check src/"
```
#### Fields
| Field | Required | Description |
| --- | --- | --- |
| `id` | yes | Identifier for this overlay. Used in `specify workflow overlay *` commands. Must be lowercase letters, digits, and hyphens only; no dots, underscores, path separators, or `overlays`. |
| `extends` | yes | The workflow id this overlay applies to. Uses the same safe-id format as `id`; `overlays`, `runs`, and `steps` are reserved. |
| `priority` | no | Integer; defaults to `10`. Lower values have higher precedence and win conflicts. Missing or invalid values fall back to `10`. |
| `enabled` | no | Boolean. Defaults to `true`. Disabled overlays are ignored. |
| `edits` | yes | Non-empty list of edit operations. |
#### Edit Operations
| Operation | `step` required | Effect |
| --- | --- | --- |
| `insert_after` | yes | Insert `step` immediately after the anchor step. |
| `insert_before` | yes | Insert `step` immediately before the anchor step. |
| `replace` | yes | Replace the anchor step with `step`. |
| `remove` | no | Remove the anchor step from the list. |
The `anchor` is the `id` of a step in the base workflow. Anchors are resolved recursively inside `then`, `else`, `steps`, `cases.*`, and `default` blocks, so nested base steps can also be targeted. Fan-out templates (`step` inside a `fan-out` step) are **not** valid anchors.
Step ids must not contain `:` — that character is reserved for engine-generated nested ids.
### Overlay CLI Commands
#### Add a Project Overlay
```bash
specify workflow overlay add <path-to-overlay.yml> --priority <n>
```
Validates the overlay file and copies it to `.specify/workflows/overlays/<extends>/<id>.yml`. `--priority` defaults to `10` and overrides the `priority` field in the file.
#### List Overlays
```bash
specify workflow overlay list <workflow-id>
```
Shows all overlays for the workflow, ordered by resolver precedence. Disabled overlays are marked as disabled in the listing and are ignored during workflow resolution.
#### Change Priority
```bash
specify workflow overlay set-priority <workflow-id> <overlay-id> <n>
```
#### Enable or Disable
```bash
specify workflow overlay disable <workflow-id> <overlay-id>
specify workflow overlay enable <workflow-id> <overlay-id>
```
#### Remove
```bash
specify workflow overlay remove <workflow-id> <overlay-id>
```
Removes the project overlay file.
#### Inspect the Composed Workflow
```bash
specify workflow resolve <workflow-id>
```
Prints the layer stack (base + overlays) and the source attribution for each step after composition. Useful for debugging which overlay contributed or overrode a step.
### Example: Adding Automated Linting after Implementation
Given the built-in `speckit` workflow, create `project-overlay.yml`:
```yaml
id: "add-lint"
extends: "speckit"
priority: 10
edits:
- insert_after: implement
step:
id: run-lint
type: shell
run: "ruff check src/"
```
Install it:
```bash
specify workflow overlay add project-overlay.yml --priority 10
```
Run the workflow:
```bash
specify workflow run speckit -i spec="Build a kanban board"
```
The composed workflow will now run the full SDD cycle and execute `ruff check src/` automatically after the `implement` step.
### Example: Replacing a Gate
```yaml
id: "skip-plan-review"
extends: "speckit"
priority: 5
edits:
- replace: review-plan
step:
id: review-plan
type: command
command: speckit.plan
input:
args: "{{ inputs.spec }}"
```
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.
### Interaction with Bundles and Updates
`specify workflow add <local-directory>` installs `workflow.yml` from the local directory into `.specify/workflows/<id>/`.
When an installed workflow is refreshed or reinstalled, project overlays in `.specify/workflows/overlays/<id>/` are preserved because they live outside the installed workflow directory.
### Limitations
- Overlays operate on the step list only. They cannot change workflow metadata (name, description, inputs, `requires`) or expression logic.
- Fan-out templates cannot be used as anchors.
- An overlay that targets a step id that does not exist in the base workflow will raise a validation error when the workflow is resolved.
- Overlays cannot target steps added by other overlays.
- Overlays cannot add new inputs or change the input schema of the base workflow.
## Update Workflows
```bash

2
src/specify_cli/extensions/__init__.py
View File

@@ -136,7 +136,7 @@ def normalize_priority(value: Any, default: int = DEFAULT_HOOK_PRIORITY) -> int:
return default
try:
priority = int(value)
except (TypeError, ValueError):
except (TypeError, ValueError, OverflowError):
return default
return priority if priority >= 1 else default

112
src/specify_cli/workflows/_commands.py
View File

@@ -49,6 +49,13 @@ workflow_step_catalog_app = typer.Typer(
)
workflow_step_app.add_typer(workflow_step_catalog_app, name="catalog")
workflow_overlay_app = typer.Typer(
name="overlay",
help="Manage workflow overlays",
add_completion=False,
)
workflow_app.add_typer(workflow_overlay_app, name="overlay")
def _error_console(json_output: bool):
"""Console for error text: stderr under ``--json`` so the JSON stdout
@@ -192,6 +199,10 @@ def _reject_unsafe_workflow_storage(project_root: Path) -> None:
project_root / ".specify" / "workflows" / "runs",
".specify/workflows/runs",
)
_reject_unsafe_dir(
project_root / ".specify" / "workflows" / "overlays",
".specify/workflows/overlays",
)
def _scan_for_workflow_owner(parts: tuple[str, ...]) -> int | None:
@@ -366,7 +377,7 @@ def _resolve_installed_workflow_ownership(
_WORKFLOW_ID_PATTERN = re.compile(r"^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$")
_RESERVED_WORKFLOW_IDS: frozenset[str] = frozenset({"runs", "steps"})
_RESERVED_WORKFLOW_IDS: frozenset[str] = frozenset({"overlays", "runs", "steps"})
def _reject_insecure_download_redirect(old_url: str, new_url: str) -> None:
@@ -2386,6 +2397,9 @@ def workflow_info(
# Local workflow definition not found on disk; fall back to
# catalog/registry lookup below.
pass
except ValueError as exc:
console.print(f"[red]Error:[/red] Invalid workflow: {_escape_markup(str(exc))}")
raise typer.Exit(1)
if definition:
console.print(f"\n[bold cyan]{definition.name}[/bold cyan] ({definition.id})")
@@ -3152,6 +3166,102 @@ def workflow_step_catalog_remove(
console.print(f"[green]✓[/green] Step catalog source '{removed_name}' removed")
@workflow_overlay_app.command("add")
def workflow_overlay_add_cmd(
source: Path = typer.Argument(..., help="Path to overlay YAML file"),
priority: int = typer.Option(
10,
"--priority",
help="Resolution priority (lower = higher precedence, default 10)",
),
):
"""Add a project-local overlay for a workflow."""
from .overlays._commands import workflow_overlay_add
project_root = _require_specify_project()
if workflow_overlay_add(project_root, source, priority) is None:
raise typer.Exit(1)
@workflow_overlay_app.command("set-priority")
def workflow_overlay_set_priority_cmd(
workflow_id: str = typer.Argument(..., help="Workflow ID the overlay extends"),
overlay_id: str = typer.Argument(..., help="Overlay ID"),
priority: int = typer.Argument(
..., help="New priority (lower = higher precedence)"
),
):
"""Set the priority of a project-local overlay."""
from .overlays._commands import workflow_overlay_set_priority
project_root = _require_specify_project()
if not workflow_overlay_set_priority(project_root, workflow_id, overlay_id, priority):
raise typer.Exit(1)
@workflow_overlay_app.command("enable")
def workflow_overlay_enable_cmd(
workflow_id: str = typer.Argument(..., help="Workflow ID the overlay extends"),
overlay_id: str = typer.Argument(..., help="Overlay ID"),
):
"""Enable a project-local overlay."""
from .overlays._commands import workflow_overlay_enable
project_root = _require_specify_project()
if not workflow_overlay_enable(project_root, workflow_id, overlay_id):
raise typer.Exit(1)
@workflow_overlay_app.command("disable")
def workflow_overlay_disable_cmd(
workflow_id: str = typer.Argument(..., help="Workflow ID the overlay extends"),
overlay_id: str = typer.Argument(..., help="Overlay ID"),
):
"""Disable a project-local overlay."""
from .overlays._commands import workflow_overlay_disable
project_root = _require_specify_project()
if not workflow_overlay_disable(project_root, workflow_id, overlay_id):
raise typer.Exit(1)
@workflow_overlay_app.command("remove")
def workflow_overlay_remove_cmd(
workflow_id: str = typer.Argument(..., help="Workflow ID the overlay extends"),
overlay_id: str = typer.Argument(..., help="Overlay ID"),
):
"""Remove a project-local overlay."""
from .overlays._commands import workflow_overlay_remove
project_root = _require_specify_project()
if not workflow_overlay_remove(project_root, workflow_id, overlay_id):
raise typer.Exit(1)
@workflow_overlay_app.command("list")
def workflow_overlay_list_cmd(
workflow_id: str = typer.Argument(..., help="Workflow ID"),
):
"""List overlays for a workflow."""
from .overlays._commands import workflow_overlay_list
project_root = _require_specify_project()
if workflow_overlay_list(project_root, workflow_id) is None:
raise typer.Exit(1)
@workflow_app.command("resolve")
def workflow_resolve_cmd(
workflow_id: str = typer.Argument(..., help="Workflow ID to resolve"),
):
"""Show layer attribution for a resolved workflow."""
from .overlays._commands import workflow_resolve
project_root = _require_specify_project()
if workflow_resolve(project_root, workflow_id) is None:
raise typer.Exit(1)
def register(app: typer.Typer) -> None:
"""Attach the workflow command group to the root Typer app."""
app.add_typer(workflow_app, name="workflow")

25
src/specify_cli/workflows/engine.py
View File

@@ -79,7 +79,11 @@ class WorkflowDefinition:
def from_yaml(cls, path: Path) -> WorkflowDefinition:
"""Load a workflow definition from a YAML file."""
with open(path, encoding="utf-8") as f:
data = yaml.safe_load(f)
try:
data = yaml.safe_load(f)
except yaml.YAMLError as exc:
msg = f"Invalid YAML in {path}: {exc}"
raise ValueError(msg) from exc
if not isinstance(data, dict):
msg = f"Workflow YAML must be a mapping, got {type(data).__name__}."
raise ValueError(msg)
@@ -88,7 +92,11 @@ class WorkflowDefinition:
@classmethod
def from_string(cls, content: str) -> WorkflowDefinition:
"""Load a workflow definition from a YAML string."""
data = yaml.safe_load(content)
try:
data = yaml.safe_load(content)
except yaml.YAMLError as exc:
msg = f"Invalid YAML: {exc}"
raise ValueError(msg) from exc
if not isinstance(data, dict):
msg = f"Workflow YAML must be a mapping, got {type(data).__name__}."
raise ValueError(msg)
@@ -727,13 +735,24 @@ class WorkflowEngine:
ValueError:
If the workflow YAML is invalid.
"""
from .overlays import WorkflowResolver
path = Path(source).expanduser()
# Try as a direct file path first
if path.suffix.lower() in (".yml", ".yaml") and path.is_file():
return WorkflowDefinition.from_yaml(path)
# Try as an installed workflow ID
# Try as an installed workflow ID, resolving any overlays.
resolver = WorkflowResolver(self.project_root)
try:
return resolver.resolve(str(source))
except FileNotFoundError:
# Fall back to the direct workflow.yml path so callers still get
# the original error when the workflow id is not installed.
pass
# Legacy direct path check for workflows installed without registry entries.
installed_path = (
self.project_root
/ ".specify"

95
src/specify_cli/workflows/overlays/__init__.py Normal file
View File

@@ -0,0 +1,95 @@
"""Workflow overlay resolver — composes installed workflows from layers."""
from __future__ import annotations
from pathlib import Path
from ..engine import WorkflowDefinition
from .composer import StepListComposer
from .layer_sources import (
BaseWorkflowSource,
Layer,
ProjectOverlaySource,
)
from .merge import ComposedStep
from .schema import _RESERVED_WORKFLOW_IDS, _SAFE_ID_PATTERN
def _validate_workflow_id(workflow_id: str) -> None:
"""Reject workflow IDs that are unsafe as installed-storage path segments."""
if (
not isinstance(workflow_id, str)
or not _SAFE_ID_PATTERN.fullmatch(workflow_id)
or workflow_id in _RESERVED_WORKFLOW_IDS
):
raise ValueError(f"Invalid workflow ID: {workflow_id!r}")
class WorkflowResolver:
"""Resolves a workflow ID to its composed ``WorkflowDefinition``.
Collects layers from two tiers:
- project-local overlays (``.specify/workflows/overlays/<id>/*.yml``)
- the base workflow itself (``.specify/workflows/<id>/workflow.yml``)
Resolution is lower-wins: overlays with lower priority numbers are applied
later and override earlier edits on the same anchors.
"""
def __init__(self, project_root: Path) -> None:
self.project_root = project_root
self._sources = [
ProjectOverlaySource(project_root),
BaseWorkflowSource(project_root),
]
self._composer = StepListComposer()
def collect_all_layers(
self, workflow_id: str, *, include_disabled: bool = False
) -> list[Layer]:
"""Collect overlays sorted by precedence, followed by the base layer.
Lower priority numbers win. Ties are sorted alphabetically by source,
matching ``PresetRegistry.list_by_priority()``. The base workflow is a
foundation rather than a precedence candidate, so it is kept separate.
"""
_validate_workflow_id(workflow_id)
all_layers: list[Layer] = []
for source in self._sources:
all_layers.extend(
source.collect(workflow_id, include_disabled=include_disabled)
)
overlays = [layer for layer in all_layers if layer.tier != "base"]
base_layers = [layer for layer in all_layers if layer.tier == "base"]
return (
sorted(overlays, key=lambda layer: (layer.priority, layer.source))
+ base_layers
)
def resolve(self, workflow_id: str) -> WorkflowDefinition:
"""Resolve a workflow ID to its composed definition.
This method composes layers but does not validate workflow semantics;
callers should validate the returned definition when needed.
Raises:
FileNotFoundError: if the workflow cannot be found.
ValueError: if layer collection/composition fails.
"""
layers = self.collect_all_layers(workflow_id)
definition, _ = self._composer.compose(layers)
if definition is None:
raise FileNotFoundError(f"Workflow not found: {workflow_id}")
return definition
def resolve_with_layers(
self, workflow_id: str
) -> tuple[WorkflowDefinition, list[Layer], list[ComposedStep]]:
"""Resolve a workflow and return its definition plus layer attribution."""
layers = self.collect_all_layers(workflow_id)
definition, attribution = self._composer.compose(layers)
if definition is None:
raise FileNotFoundError(f"Workflow not found: {workflow_id}")
return definition, layers, attribution

442
src/specify_cli/workflows/overlays/_commands.py Normal file
View File

@@ -0,0 +1,442 @@
"""CLI handlers for ``specify workflow overlay *`` and ``specify workflow resolve``."""
from __future__ import annotations
from pathlib import Path
from typing import Any
import typer
import yaml
from ..._console import console, err_console
from ...extensions import normalize_priority
from .._commands import (
_commit_workflow_file,
_discard_committed_backup_file,
_reject_unsafe_dir,
_reject_unsafe_workflow_storage,
_safe_discard_staged_workflow_file,
_stage_workflow_file,
)
from . import WorkflowResolver
from .schema import _RESERVED_WORKFLOW_IDS, _SAFE_ID_PATTERN, validate_overlay_yaml
def _validate_overlay_id_or_exit(id_value: str, label: str) -> None:
"""Validate a single-segment overlay/workflow id from CLI arguments."""
if not isinstance(id_value, str) or not id_value:
err_console.print(f"[red]Error:[/red] {label} is required and must be a non-empty string.")
raise typer.Exit(1)
if not _SAFE_ID_PATTERN.fullmatch(id_value):
err_console.print(
f"[red]Error:[/red] Invalid {label} {id_value!r}: "
"only lowercase letters, digits, and hyphens are allowed."
)
raise typer.Exit(1)
def _validate_workflow_id_or_exit(workflow_id: str) -> None:
"""Validate a workflow id, treating the overlay root as reserved."""
_validate_overlay_id_or_exit(workflow_id, "workflow ID")
if workflow_id in _RESERVED_WORKFLOW_IDS:
err_console.print(
f"[red]Error:[/red] Invalid workflow ID {workflow_id!r}: "
"reserved name."
)
raise typer.Exit(1)
def _overlay_root(project_root: Path) -> Path:
"""Return the project-local overlay root after rejecting unsafe ancestors."""
_reject_unsafe_workflow_storage(project_root)
root = project_root / ".specify" / "workflows" / "overlays"
_reject_unsafe_dir(root, ".specify/workflows/overlays")
return root
def _project_overlay_dir(project_root: Path, workflow_id: str) -> Path:
"""Return the project-local overlay directory for a workflow id.
Raises typer.Exit if the resolved path escapes the overlay root.
"""
_validate_workflow_id_or_exit(workflow_id)
root = _overlay_root(project_root)
target = root / workflow_id
return _ensure_contained_dir(target, root)
def _ensure_contained_dir(path: Path, root: Path) -> Path:
"""Ensure *path* resolves inside *root* and is not a symlink.
Returns *path* if safe. Raises typer.Exit on traversal or symlink.
"""
_reject_unsafe_dir(root, ".specify/workflows/overlays")
if path.is_symlink():
err_console.print(
f"[red]Error:[/red] Refusing to use symlinked path {path}."
)
raise typer.Exit(1)
if path.exists() and not path.is_dir():
err_console.print(
f"[red]Error:[/red] Overlay directory path is not a directory: {path}."
)
raise typer.Exit(1)
try:
resolved = path.resolve()
root_resolved = root.resolve()
resolved.relative_to(root_resolved)
except ValueError:
err_console.print(
f"[red]Error:[/red] Path traversal detected: {path} is outside the allowed directory."
)
raise typer.Exit(1)
return path
def _find_overlay_file(project_root: Path, workflow_id: str, overlay_id: str) -> Path | None:
"""Locate a project-local overlay file by its manifest ID, not filename.
Scans all YAML files in the overlay directory and matches on the ``id``
field inside each manifest. This aligns with ``ProjectOverlaySource.collect()``
which also derives identity from the manifest, not the filename.
"""
_validate_workflow_id_or_exit(workflow_id)
_validate_overlay_id_or_exit(overlay_id, "overlay ID")
overlay_dir = _project_overlay_dir(project_root, workflow_id)
if not overlay_dir.is_dir():
return None
try:
entries = sorted(overlay_dir.iterdir())
except OSError:
return None
matches: list[Path] = []
for path in entries:
if not path.is_file() or path.suffix not in (".yml", ".yaml"):
continue
if path.is_symlink():
continue
data, _ = _read_overlay(path)
if data is None:
continue
if data.get("id") == overlay_id:
matches.append(path)
if len(matches) > 1:
paths = ", ".join(str(path) for path in matches)
err_console.print(
f"[red]Error:[/red] Duplicate overlay ID '{overlay_id}' in {paths}. "
"Resolve the duplicate manifest IDs before continuing."
)
raise typer.Exit(1)
return matches[0] if matches else None
def _ensure_contained_path(path: Path, root: Path) -> Path:
"""Return *path* only if it resolves inside *root*; otherwise raise typer.Exit."""
_reject_unsafe_dir(root, ".specify/workflows/overlays")
if path.is_symlink():
err_console.print(
f"[red]Error:[/red] Refusing to use symlinked path {path}."
)
raise typer.Exit(1)
try:
resolved = path.resolve()
root_resolved = root.resolve()
resolved.relative_to(root_resolved)
except ValueError:
err_console.print(
f"[red]Error:[/red] Path traversal detected: {path} is outside the allowed directory."
)
raise typer.Exit(1)
return path
def _read_overlay(path: Path) -> tuple[dict[str, Any] | None, list[str]]:
"""Read and parse an overlay YAML file, returning (data, errors)."""
try:
content = path.read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError) as exc:
return None, [f"Failed to read {path}: {exc}"]
try:
data = yaml.safe_load(content)
except yaml.YAMLError as exc:
return None, [f"Invalid YAML in {path}: {exc}"]
if not isinstance(data, dict):
return None, [f"Overlay {path} must be a YAML mapping."]
return data, []
def workflow_overlay_add(
project_root: Path,
source: Path,
priority: int | None = None,
) -> Path | None:
"""Add a project-local overlay from a YAML file.
Returns the path of the installed overlay file, or None on failure.
"""
_reject_unsafe_workflow_storage(project_root)
data, errors = _read_overlay(source)
if data is None:
for err in errors:
err_console.print(f"[red]Error:[/red] {err}")
return None
# Apply --priority override before validation so a valid CLI priority
# can fix a missing or invalid priority in the file.
if priority is not None:
if isinstance(priority, bool) or not isinstance(priority, int) or priority < 1:
err_console.print("[red]Error:[/red] Priority must be >= 1.")
return None
data["priority"] = normalize_priority(priority)
overlay, validation_errors = validate_overlay_yaml(data)
if overlay is None:
err_console.print("[red]Error:[/red] Overlay validation failed:")
for err in validation_errors:
err_console.print(f" \u2022 {err}")
return None
data["priority"] = overlay.priority
target_dir = _project_overlay_dir(project_root, overlay.extends)
# Reuse an existing .yaml file so we don't create a duplicate .yml layer.
existing = _find_overlay_file(project_root, overlay.extends, overlay.id)
if existing is not None:
target_path = existing
else:
target_path = _ensure_contained_path(
target_dir / f"{overlay.id}.yml", _overlay_root(project_root)
)
backup: Path | None = None
try:
target_dir.mkdir(parents=True, exist_ok=True)
existed_before = target_path.exists()
staged = _stage_workflow_file(target_path.parent)
try:
staged.write_bytes(yaml.safe_dump(data, sort_keys=False).encode("utf-8"))
backup = _commit_workflow_file(staged, target_path, existed_before)
except BaseException:
_safe_discard_staged_workflow_file(
staged, target_path.parent, existed_before
)
raise
except OSError as exc:
err_console.print(f"[red]Error:[/red] Failed to write overlay: {exc}")
return None
_discard_committed_backup_file(backup)
console.print(
f"[green]\u2713[/green] Overlay '{overlay.id}' added for workflow '{overlay.extends}'"
)
return target_path
def _update_overlay_field(
project_root: Path,
workflow_id: str,
overlay_id: str,
field: str,
value: Any,
) -> bool:
"""Update a single field in a project-local overlay file."""
_reject_unsafe_workflow_storage(project_root)
path = _find_overlay_file(project_root, workflow_id, overlay_id)
if path is None:
err_console.print(
f"[red]Error:[/red] Overlay '{overlay_id}' not found for workflow '{workflow_id}'"
)
return False
data, errors = _read_overlay(path)
if data is None:
for err in errors:
err_console.print(f"[red]Error:[/red] {err}")
return False
data[field] = value
overlay, validation_errors = validate_overlay_yaml(data)
if overlay is None:
err_console.print("[red]Error:[/red] Overlay validation failed:")
for err in validation_errors:
err_console.print(f" \u2022 {err}")
return False
backup: Path | None = None
try:
existed_before = path.exists()
staged = _stage_workflow_file(path.parent)
try:
staged.write_bytes(yaml.safe_dump(data, sort_keys=False).encode("utf-8"))
backup = _commit_workflow_file(staged, path, existed_before)
except BaseException:
_safe_discard_staged_workflow_file(staged, path.parent, existed_before)
raise
except OSError as exc:
err_console.print(f"[red]Error:[/red] Failed to write overlay: {exc}")
return False
_discard_committed_backup_file(backup)
return True
def workflow_overlay_set_priority(
project_root: Path,
workflow_id: str,
overlay_id: str,
priority: int,
) -> bool:
"""Set the priority of a project-local overlay."""
if isinstance(priority, bool) or not isinstance(priority, int) or priority < 1:
err_console.print("[red]Error:[/red] Priority must be >= 1.")
raise typer.Exit(1)
normalized_priority = normalize_priority(priority)
if _update_overlay_field(
project_root, workflow_id, overlay_id, "priority", normalized_priority
):
console.print(
f"[green]\u2713[/green] Priority of overlay '{overlay_id}' set to {normalized_priority}"
)
return True
return False
def workflow_overlay_enable(
project_root: Path,
workflow_id: str,
overlay_id: str,
) -> bool:
"""Enable a project-local overlay."""
if _update_overlay_field(project_root, workflow_id, overlay_id, "enabled", True):
console.print(f"[green]\u2713[/green] Overlay '{overlay_id}' enabled")
return True
return False
def workflow_overlay_disable(
project_root: Path,
workflow_id: str,
overlay_id: str,
) -> bool:
"""Disable a project-local overlay."""
if _update_overlay_field(project_root, workflow_id, overlay_id, "enabled", False):
console.print(f"[green]\u2713[/green] Overlay '{overlay_id}' disabled")
return True
return False
def workflow_overlay_remove(
project_root: Path,
workflow_id: str,
overlay_id: str,
) -> bool:
"""Remove a project-local overlay file."""
_reject_unsafe_workflow_storage(project_root)
path = _find_overlay_file(project_root, workflow_id, overlay_id)
if path is None:
err_console.print(
f"[red]Error:[/red] Overlay '{overlay_id}' not found for workflow '{workflow_id}'"
)
return False
try:
path.unlink()
except OSError as exc:
err_console.print(f"[red]Error:[/red] Failed to remove overlay: {exc}")
return False
console.print(f"[green]\u2713[/green] Overlay '{overlay_id}' removed")
return True
def workflow_overlay_list(project_root: Path, workflow_id: str) -> list[dict[str, Any]] | None:
"""List all overlays for a workflow and print a summary table.
Returns the raw list data for machine-readable callers, or None on error.
"""
_reject_unsafe_workflow_storage(project_root)
_validate_workflow_id_or_exit(workflow_id)
resolver = WorkflowResolver(project_root)
try:
layers = resolver.collect_all_layers(workflow_id, include_disabled=True)
except ValueError as exc:
err_console.print(f"[red]Error:[/red] {exc}")
return None
overlays = [layer for layer in layers if layer.tier != "base"]
if not overlays:
console.print(f"[yellow]No overlays found for workflow '{workflow_id}'.[/yellow]")
return []
console.print(f"Overlays for workflow '{workflow_id}':")
rows: list[dict[str, Any]] = []
for layer in overlays:
overlay = layer.content
rows.append({
"id": overlay.id,
"source": layer.source,
"tier": layer.tier,
"priority": normalize_priority(overlay.priority),
"enabled": overlay.enabled,
"path": str(layer.path) if layer.path else None,
})
enabled_marker = "enabled" if overlay.enabled else "disabled"
console.print(
f" \u2022 {overlay.id} (priority={normalize_priority(overlay.priority)}, "
f"source={layer.source}, {enabled_marker})"
)
return rows
def workflow_resolve(project_root: Path, workflow_id: str) -> dict[str, Any] | None:
"""Print layer attribution for a resolved workflow.
Returns a serializable attribution payload.
"""
_reject_unsafe_workflow_storage(project_root)
_validate_workflow_id_or_exit(workflow_id)
resolver = WorkflowResolver(project_root)
try:
definition, layers, attribution = resolver.resolve_with_layers(workflow_id)
except FileNotFoundError:
err_console.print(
f"[red]Error:[/red] Workflow '{workflow_id}' not found"
)
return None
except ValueError as exc:
err_console.print(f"[red]Error:[/red] {exc}")
return None
console.print(f"Resolved workflow '{workflow_id}':")
console.print("Layers (highest precedence first):")
for layer in layers:
priority = (
"n/a" if layer.tier == "base" else str(normalize_priority(layer.priority))
)
console.print(
f" \u2022 [{layer.tier}] {layer.source} "
f"(priority={priority})"
)
console.print("Step attribution:")
for composed in attribution:
console.print(f" \u2022 {composed.step_id}: {composed.source}")
return {
"workflow_id": workflow_id,
"layers": [
{
"source": layer.source,
"tier": layer.tier,
"priority": (
None
if layer.tier == "base"
else normalize_priority(layer.priority)
),
}
for layer in layers
],
"attribution": [
{"step_id": composed.step_id, "source": composed.source}
for composed in attribution
],
}

97
src/specify_cli/workflows/overlays/composer.py Normal file
View File

@@ -0,0 +1,97 @@
"""Workflow overlay composer — builds a WorkflowDefinition from layers."""
from __future__ import annotations
from typing import Any
from ..engine import WorkflowDefinition
from .layer_sources import Layer
from .merge import OverlayLayer, merge_steps, validate_edits
class StepListComposer:
"""Compose a workflow from a base layer and overlay layers.
- The base layer (tier="base") provides the full step list.
- Overlay layers provide edit operations.
- Overlays are applied in merge order: highest priority number first,
lowest last, so lower priority numbers win. Ties are applied by overlay
ID, with the alphabetically last ID winning.
- Returns a parsed WorkflowDefinition; callers must validate separately.
"""
def compose(
self, layers: list[Layer]
) -> tuple[WorkflowDefinition | None, list]:
"""Compose a ``WorkflowDefinition`` from the given layers.
Returns ``(None, [])`` when no base layer is present.
"""
base_layer: Layer | None = None
overlay_layers: list[Layer] = []
for layer in layers:
if layer.tier == "base":
base_layer = layer
else:
overlay_layers.append(layer)
if base_layer is None or base_layer.path is None:
return None, []
# Read the base workflow definition from disk.
base_definition = WorkflowDefinition.from_yaml(base_layer.path)
base_steps = base_definition.data.get("steps", [])
if not isinstance(base_steps, list):
# Preserve the invalid definition intact so validate_workflow can
# report "'steps' must be a list." to the caller; coercing to []
# here would mask that error.
return base_definition, []
# Last applied wins, so apply lower priority numbers last.
merge_order = sorted(
overlay_layers,
key=lambda layer: (-layer.priority, layer.content.id),
)
# Validate edits against base anchors before mutation.
base_step_ids = self._collect_base_step_ids(base_steps)
for layer in merge_order:
edit_errors = validate_edits(layer.content.edits, base_step_ids)
if edit_errors:
raise ValueError(
f"Overlay '{layer.content.id}' has invalid edits:\n - "
+ "\n - ".join(edit_errors)
)
composed_steps, attribution = merge_steps(
base_steps,
[OverlayLayer(layer.content, layer.source) for layer in merge_order],
)
# Build composed data while preserving all non-step fields from base.
composed_data: dict[str, Any] = dict(base_definition.data)
composed_data["steps"] = composed_steps
composed_definition = WorkflowDefinition(composed_data, source_path=base_layer.path)
return composed_definition, attribution
def _collect_base_step_ids(self, steps: list[dict[str, Any]]) -> set[str]:
"""Collect all base step IDs reachable in the step tree."""
ids: set[str] = set()
for step in steps:
if not isinstance(step, dict):
continue
step_id = step.get("id")
if isinstance(step_id, str):
ids.add(step_id)
for key in ("then", "else", "steps", "default"):
nested = step.get(key)
if isinstance(nested, list):
ids.update(self._collect_base_step_ids(nested))
cases = step.get("cases")
if isinstance(cases, dict):
for case_steps in cases.values():
if isinstance(case_steps, list):
ids.update(self._collect_base_step_ids(case_steps))
return ids

234
src/specify_cli/workflows/overlays/layer_sources.py Normal file
View File

@@ -0,0 +1,234 @@
"""Workflow overlay layer sources."""
from __future__ import annotations
from dataclasses import dataclass
from pathlib import Path
import yaml
from .schema import Overlay, _RESERVED_WORKFLOW_IDS, _SAFE_ID_PATTERN, validate_overlay_yaml
@dataclass
class Layer:
"""A single layer in the workflow overlay stack."""
content: Overlay
source: str
tier: str
priority: int
path: Path | None = None
class OverlayLoadError(ValueError):
"""Raised when an overlay file cannot be loaded or validated."""
def __init__(self, path: Path, errors: list[str]) -> None:
self.path = path
self.errors = errors
super().__init__(f"Invalid overlay {path}:\n - " + "\n - ".join(errors))
def _validate_workflow_id(workflow_id: str, context_path: Path) -> None:
"""Raise OverlayLoadError if workflow_id is not a safe path-segment identifier.
Mirrors the same check performed by WorkflowResolver so layer sources are
safe to call directly, without going through the resolver.
"""
if (
not isinstance(workflow_id, str)
or not _SAFE_ID_PATTERN.fullmatch(workflow_id)
or workflow_id in _RESERVED_WORKFLOW_IDS
):
raise OverlayLoadError(
context_path,
[f"Invalid workflow ID: {workflow_id!r}"],
)
def _ensure_contained_dir(path: Path, root: Path) -> None:
"""Raise OverlayLoadError if *path* is a symlink, a non-directory, or escapes *root*.
Mirrors the logic of ``_ensure_contained_dir`` in ``overlays/_commands.py``
but raises ``OverlayLoadError`` instead of ``typer.Exit`` so layer sources
can enforce the same invariants without a CLI dependency.
The caller is responsible for ensuring *root* itself is already validated
(e.g. via ``_resolve_project_overlay_root``).
"""
if path.is_symlink():
raise OverlayLoadError(path, ["Symlinked overlay directories are not allowed"])
if path.exists() and not path.is_dir():
raise OverlayLoadError(path, ["Overlay directory path is not a directory"])
try:
path.resolve().relative_to(root.resolve())
except ValueError:
raise OverlayLoadError(
path, ["Path traversal detected: directory escapes allowed root"]
) from None
def _resolve_workflows_root(project_root: Path) -> Path:
"""Return the workflow storage root after rejecting unsafe ancestors."""
project_root_resolved = project_root.resolve()
workflows_root = project_root / ".specify" / "workflows"
current = project_root
for part in (".specify", "workflows"):
current = current / part
if current.is_symlink():
raise OverlayLoadError(
current,
[f"Symlinked workflow directories are not allowed ({current})"],
)
if current.exists() and not current.is_dir():
raise OverlayLoadError(
current,
[f"Workflow directory path is not a directory ({current})"],
)
try:
workflows_root.resolve().relative_to(project_root_resolved)
except ValueError:
raise OverlayLoadError(
workflows_root,
["Workflow directory escapes the project root"],
) from None
return workflows_root
def _resolve_project_overlay_root(project_root: Path) -> Path:
"""Return the unresolved overlay root after rejecting unsafe ancestors."""
workflows_root = _resolve_workflows_root(project_root)
overlays_root = workflows_root / "overlays"
if overlays_root.is_symlink():
raise OverlayLoadError(
overlays_root,
[f"Symlinked overlay directories are not allowed ({overlays_root})"],
)
if overlays_root.exists() and not overlays_root.is_dir():
raise OverlayLoadError(
overlays_root,
[f"Overlay directory path is not a directory ({overlays_root})"],
)
return overlays_root
class ProjectOverlaySource:
"""Project-local overlays: ``.specify/workflows/overlays/<id>/*.yml``."""
tier = "project-overlay"
def __init__(self, project_root: Path) -> None:
self.project_root = project_root
self.overlays_dir = project_root / ".specify" / "workflows" / "overlays"
def collect(self, workflow_id: str, *, include_disabled: bool = False) -> list[Layer]:
"""Collect project-local overlays for the given workflow id.
Args:
workflow_id: Workflow identifier whose overlay directory to scan.
include_disabled: When True, return disabled overlays for
management/list views. Resolution paths keep the default False.
"""
self.overlays_dir = _resolve_project_overlay_root(self.project_root)
_validate_workflow_id(workflow_id, self.overlays_dir)
workflow_overlay_dir = self.overlays_dir / workflow_id
_ensure_contained_dir(workflow_overlay_dir, self.overlays_dir)
if not workflow_overlay_dir.is_dir():
return []
layers: list[Layer] = []
overlay_paths_by_id: dict[str, Path] = {}
try:
entries = sorted(workflow_overlay_dir.iterdir())
except OSError as exc:
raise OverlayLoadError(
workflow_overlay_dir, [f"Cannot enumerate overlays: {exc}"]
) from exc
for path in entries:
if not path.is_file() or path.suffix not in (".yml", ".yaml"):
continue
if path.is_symlink():
raise OverlayLoadError(path, ["Symlinked overlay files are not allowed"])
try:
data = yaml.safe_load(path.read_text(encoding="utf-8")) or {}
except yaml.YAMLError as exc:
raise OverlayLoadError(path, [f"Invalid YAML: {exc}"]) from exc
except (OSError, UnicodeDecodeError) as exc:
raise OverlayLoadError(path, [f"Cannot load overlay: {exc}"]) from exc
if (
not include_disabled
and isinstance(data, dict)
and data.get("enabled", True) is False
):
continue
overlay, errors = validate_overlay_yaml(data)
if overlay is None or errors:
raise OverlayLoadError(path, errors)
if overlay.extends != workflow_id:
raise OverlayLoadError(
path,
[
f"Overlay extends {overlay.extends!r}, but is stored under "
f"workflow {workflow_id!r}."
],
)
first_path = overlay_paths_by_id.get(overlay.id)
if first_path is not None:
raise OverlayLoadError(
path,
[
f"Duplicate overlay id {overlay.id!r}; also declared in "
f"{first_path}."
],
)
overlay_paths_by_id[overlay.id] = path
layers.append(
Layer(
content=overlay,
source=f"project:{overlay.id}",
tier=self.tier,
priority=overlay.priority,
path=path,
)
)
return layers
class BaseWorkflowSource:
"""Base workflow layer: ``.specify/workflows/<id>/workflow.yml``."""
tier = "base"
def __init__(self, project_root: Path) -> None:
self.project_root = project_root
self.workflows_dir = project_root / ".specify" / "workflows"
def collect(self, workflow_id: str, *, include_disabled: bool = False) -> list[Layer]:
"""Return the base workflow as a single layer if it exists."""
self.workflows_dir = _resolve_workflows_root(self.project_root)
_validate_workflow_id(workflow_id, self.workflows_dir)
workflow_dir = self.workflows_dir / workflow_id
_ensure_contained_dir(workflow_dir, self.workflows_dir)
path = workflow_dir / "workflow.yml"
if path.is_symlink():
raise OverlayLoadError(path, ["Symlinked workflow files are not allowed"])
if not path.is_file():
return []
# The base layer is represented by an Overlay with empty edits.
overlay = Overlay(
id=workflow_id,
extends=workflow_id,
priority=0,
edits=[],
)
return [
Layer(
content=overlay,
source="base",
tier=self.tier,
priority=0,
path=path,
)
]

395
src/specify_cli/workflows/overlays/merge.py Normal file
View File

@@ -0,0 +1,395 @@
"""Pure-function merge engine for workflow step lists."""
from __future__ import annotations
import copy
from dataclasses import dataclass
from typing import Any
from .schema import VALID_OPERATIONS, Overlay, OverlayEdit
@dataclass(frozen=True)
class ComposedStep:
"""Attribution tracking for a single composed step."""
step_id: str
source: str
@dataclass(frozen=True)
class OverlayLayer:
"""An overlay together with its layer source for attribution."""
overlay: Overlay
source: str
# Nested step keys that may contain a list of steps.
_NESTED_LIST_KEYS = ("then", "else", "steps", "default")
def find_step(
steps: list[dict[str, Any]], step_id: str
) -> tuple[list[dict[str, Any]], int] | None:
"""Recursively locate a step by ID and return its (parent_list, index).
Searches flat lists and nested lists inside ``then``, ``else``, ``steps``,
``default``, and ``cases.*``. Does *not* descend into ``fan-out`` template
steps because those are runtime-multiplied stamps, not uniquely-addressable
nodes.
"""
for i, step in enumerate(steps):
if not isinstance(step, dict):
continue
if step.get("id") == step_id:
return (steps, i)
for key in _NESTED_LIST_KEYS:
nested = step.get(key)
if isinstance(nested, list):
result = find_step(nested, step_id)
if result is not None:
return result
cases = step.get("cases")
if isinstance(cases, dict):
for case_steps in cases.values():
if isinstance(case_steps, list):
result = find_step(case_steps, step_id)
if result is not None:
return result
return None
def _all_base_step_ids(steps: list[dict[str, Any]]) -> set[str]:
"""Collect all step IDs reachable in a step tree (excluding fan-out templates)."""
ids: set[str] = set()
for step in steps:
if not isinstance(step, dict):
continue
step_id = step.get("id")
if isinstance(step_id, str):
ids.add(step_id)
for key in _NESTED_LIST_KEYS:
nested = step.get(key)
if isinstance(nested, list):
ids.update(_all_base_step_ids(nested))
cases = step.get("cases")
if isinstance(cases, dict):
for case_steps in cases.values():
if isinstance(case_steps, list):
ids.update(_all_base_step_ids(case_steps))
return ids
def _descendant_ids(step: dict[str, Any]) -> set[str]:
"""Return all step IDs nested inside *step* (not including *step* itself)."""
ids: set[str] = set()
for key in _NESTED_LIST_KEYS:
nested = step.get(key)
if isinstance(nested, list):
ids.update(_all_base_step_ids(nested))
cases = step.get("cases")
if isinstance(cases, dict):
for case_steps in cases.values():
if isinstance(case_steps, list):
ids.update(_all_base_step_ids(case_steps))
return ids
def _check_anchor_conflicts(
anchor_operations: dict[str, str],
base_steps: list[dict[str, Any]],
) -> list[str]:
"""Return error messages for anchor pairs where one is an ancestor of the other.
Only flags conflicts where the ancestor's winning edit is ``replace`` or
``remove`` — operations that destroy the subtree and make any descendant
anchor unresolvable. Pure insert operations on an ancestor leave it intact,
so its descendants remain reachable regardless of processing order.
Callers should raise on any returned errors before mutating the step tree.
"""
errors: list[str] = []
for anchor, operation in sorted(anchor_operations.items()):
if operation in ("insert_after", "insert_before"):
# Inserts leave the ancestor step intact; descendants are unaffected.
continue
location = find_step(base_steps, anchor)
if location is None:
continue # missing anchors are reported by validate_edits
parent_list, idx = location
step = parent_list[idx]
conflicting = set(anchor_operations.keys()) & _descendant_ids(step)
for child_anchor in sorted(conflicting):
errors.append(
f"Anchor conflict: '{anchor}' is an ancestor of '{child_anchor}'. "
"Targeting both anchors in the same overlay set produces "
"order-dependent results; restructure edits to avoid nesting."
)
return errors
def _init_sources_recursively(
steps: list[dict[str, Any]], sources: dict[str, str]
) -> None:
"""Initialize attribution sources for all base steps, recursively."""
for step in steps:
if not isinstance(step, dict):
continue
step_id = step.get("id")
if isinstance(step_id, str):
sources[step_id] = "base"
for key in _NESTED_LIST_KEYS:
nested = step.get(key)
if isinstance(nested, list):
_init_sources_recursively(nested, sources)
cases = step.get("cases")
if isinstance(cases, dict):
for case_steps in cases.values():
if isinstance(case_steps, list):
_init_sources_recursively(case_steps, sources)
def _record_sources_recursively(
step: dict[str, Any],
source: str,
sources: dict[str, str],
) -> None:
"""Record *source* for a step and all its nested child steps.
Traverses ``then``, ``else``, ``steps``, ``default``, and ``cases.*``
so that ``workflow resolve`` attributes every step inside a composite
insert or replacement to the correct overlay layer.
"""
step_id = step.get("id")
if isinstance(step_id, str):
sources[step_id] = source
for key in _NESTED_LIST_KEYS:
nested = step.get(key)
if isinstance(nested, list):
for child in nested:
if isinstance(child, dict):
_record_sources_recursively(child, source, sources)
cases = step.get("cases")
if isinstance(cases, dict):
for case_steps in cases.values():
if isinstance(case_steps, list):
for child in case_steps:
if isinstance(child, dict):
_record_sources_recursively(child, source, sources)
def _remove_sources_recursively(
step: dict[str, Any],
sources: dict[str, str],
) -> None:
"""Remove source entries for a step and all its nested child steps.
Traverses the same nesting keys as ``_record_sources_recursively``.
"""
step_id = step.get("id")
if isinstance(step_id, str) and sources.get(step_id) == "base":
sources.pop(step_id, None)
for key in _NESTED_LIST_KEYS:
nested = step.get(key)
if isinstance(nested, list):
for child in nested:
if isinstance(child, dict):
_remove_sources_recursively(child, sources)
cases = step.get("cases")
if isinstance(cases, dict):
for case_steps in cases.values():
if isinstance(case_steps, list):
for child in case_steps:
if isinstance(child, dict):
_remove_sources_recursively(child, sources)
def _build_attribution(
steps: list[dict[str, Any]],
sources: dict[str, str],
) -> list[ComposedStep]:
"""Build an ordered attribution list from the composed step tree."""
result: list[ComposedStep] = []
for step in steps:
if not isinstance(step, dict):
continue
step_id = step.get("id")
if isinstance(step_id, str):
result.append(ComposedStep(step_id, sources.get(step_id, "unknown")))
for key in _NESTED_LIST_KEYS:
nested = step.get(key)
if isinstance(nested, list):
result.extend(_build_attribution(nested, sources))
cases = step.get("cases")
if isinstance(cases, dict):
for case_steps in cases.values():
if isinstance(case_steps, list):
result.extend(_build_attribution(case_steps, sources))
return result
def _traverse_and_apply(
steps: list[dict[str, Any]],
edits_by_anchor: dict[str, list[tuple[OverlayLayer, OverlayEdit]]],
sources: dict[str, str],
) -> list[dict[str, Any]]:
"""Walk the original step tree and apply overlay edits as each step is encountered.
Edits are always resolved against the *original* structure — this function
traverses the unmodified list passed in, so a replacement step's new ID can
never be mistaken for a base anchor. Nested lists (``then``, ``else``, etc.)
are recursed into only for steps that survive the edit (not for replaced
steps).
*edits* are expected to be in merge order (lowest priority first, highest
priority last); the winning edit for each anchor is ``edits[-1]``.
"""
result: list[dict[str, Any]] = []
for step in steps:
if not isinstance(step, dict):
result.append(step)
continue
step_id = step.get("id")
edits = edits_by_anchor.get(step_id, []) if isinstance(step_id, str) else []
winning_edit = edits[-1][1] if edits else None
if winning_edit is not None and winning_edit.operation == "remove":
# Winning edit removes this step; ignore all other edits on this anchor.
# Do NOT call _remove_sources_recursively here: _build_attribution only
# traverses the result list, so stale sources entries for removed steps
# are never read. Calling it would incorrectly pop the attribution of a
# *surviving* step that reuses the same ID (e.g. a replacement step
# introduced by a higher-priority overlay targeting a different anchor).
continue
# Insert before (in merge order).
for layer, edit in edits:
if edit.operation == "insert_before":
new_step = copy.deepcopy(edit.step)
_record_sources_recursively(new_step, layer.source, sources)
result.append(new_step)
if winning_edit is not None and winning_edit.operation == "replace":
winning_layer = edits[-1][0]
new_step = copy.deepcopy(winning_edit.step)
_remove_sources_recursively(step, sources)
_record_sources_recursively(new_step, winning_layer.source, sources)
result.append(new_step)
else:
# No replacement: keep this step and recurse into its nested lists.
for key in _NESTED_LIST_KEYS:
nested = step.get(key)
if isinstance(nested, list):
step[key] = _traverse_and_apply(nested, edits_by_anchor, sources)
cases = step.get("cases")
if isinstance(cases, dict):
for case_key, case_steps in cases.items():
if isinstance(case_steps, list):
cases[case_key] = _traverse_and_apply(case_steps, edits_by_anchor, sources)
result.append(step)
# Insert after (highest priority closest to anchor — reversed merge order).
for layer, edit in reversed(edits):
if edit.operation == "insert_after":
new_step = copy.deepcopy(edit.step)
_record_sources_recursively(new_step, layer.source, sources)
result.append(new_step)
return result
def merge_steps(
base_steps: list[dict[str, Any]],
overlays: list[OverlayLayer],
) -> tuple[list[dict[str, Any]], list[ComposedStep]]:
"""Apply overlays to base steps in merge order and return composed steps.
*overlays* is expected to be sorted by merge order (lowest priority first,
highest priority last). The returned step list is a deep copy of the base;
base_steps is never mutated.
Higher-wins semantics are enforced for edits that target the same base
anchor: the highest-priority edit (last in *overlays*) decides the fate of
the anchor. A lower-priority ``remove`` cannot prevent a higher-priority
``replace`` or ``insert_*`` on the same anchor.
"""
steps = copy.deepcopy(base_steps)
sources: dict[str, str] = {}
_init_sources_recursively(steps, sources)
# Group edits by anchor, preserving merge order.
edits_by_anchor: dict[str, list[tuple[OverlayLayer, OverlayEdit]]] = {}
for layer in overlays:
for edit in layer.overlay.edits:
edits_by_anchor.setdefault(edit.anchor, []).append((layer, edit))
# Raise early for non-remove edits that target anchors not present in the base.
# Overlays always apply to the original tree; they cannot target steps introduced
# by other overlays.
base_ids = _all_base_step_ids(base_steps)
for anchor, anchor_edits in edits_by_anchor.items():
winning_op = anchor_edits[-1][1].operation
if winning_op != "remove" and anchor not in base_ids:
raise ValueError(f"Anchor '{anchor}' not found in workflow steps.")
# Reject edits that target anchors with a parent/descendant relationship when
# the ancestor edit replaces or removes its subtree — those produce
# order-dependent results. Pure insert edits on an ancestor are safe because
# the ancestor step (and its descendants) remain intact.
anchor_winning_ops = {
anchor: anchor_edits[-1][1].operation
for anchor, anchor_edits in edits_by_anchor.items()
}
anchor_conflicts = _check_anchor_conflicts(anchor_winning_ops, base_steps)
if anchor_conflicts:
raise ValueError(
"Overlay anchor conflict(s) detected:\n - " + "\n - ".join(anchor_conflicts)
)
# Apply all overlay edits via a single-pass traversal of the original tree.
# Each edit is resolved against the original step structure, so a replacement
# step's new ID can never be mistaken for a base anchor in a later edit group.
result = _traverse_and_apply(steps, edits_by_anchor, sources)
attribution = _build_attribution(result, sources)
return result, attribution
def validate_edits(
edits: list[OverlayEdit],
base_step_ids: set[str],
) -> list[str]:
"""Validate overlay edits against a set of known base step IDs.
Returns a list of human-readable error messages. Does not raise.
"""
errors: list[str] = []
for idx, edit in enumerate(edits):
if edit.operation not in VALID_OPERATIONS:
errors.append(f"Edit {idx}: invalid operation {edit.operation!r}.")
continue
if edit.anchor not in base_step_ids:
errors.append(
f"Edit {idx}: anchor '{edit.anchor}' does not match any base step id."
)
if edit.operation == "remove":
if edit.step is not None:
errors.append(f"Edit {idx}: 'remove' must not include a step.")
continue
if not isinstance(edit.step, dict):
errors.append(f"Edit {idx}: '{edit.operation}' requires a step mapping.")
continue
step_id = edit.step.get("id")
if not isinstance(step_id, str) or not step_id:
errors.append(f"Edit {idx}: step is missing required 'id'.")
continue
if ":" in step_id:
errors.append(
f"Edit {idx}: step id {step_id!r} contains ':' which is reserved "
"for engine-generated nested IDs."
)
return errors

176
src/specify_cli/workflows/overlays/schema.py Normal file
View File

@@ -0,0 +1,176 @@
"""Workflow overlay schema — dataclasses and validation for overlay manifests."""
from __future__ import annotations
import re
from dataclasses import dataclass
from typing import Any, Literal
from ...extensions import normalize_priority
# Safe single-segment identifiers: no path separators, no traversal, no dots.
_SAFE_ID_PATTERN = re.compile(r"^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$")
_RESERVED_OVERLAY_WORKFLOW_IDS: frozenset[str] = frozenset({"overlays"})
_RESERVED_WORKFLOW_IDS: frozenset[str] = frozenset({"overlays", "runs", "steps"})
VALID_OPERATIONS = frozenset({"insert_after", "insert_before", "replace", "remove"})
# Map shorthand keys to operation names.
_SHORTHAND_OPERATION_KEYS: frozenset[str] = VALID_OPERATIONS
@dataclass(frozen=True)
class OverlayEdit:
"""A single edit operation on a workflow step list."""
operation: Literal["insert_after", "insert_before", "replace", "remove"]
anchor: str
step: dict[str, Any] | None = None
@dataclass
class Overlay:
"""A declared overlay (one YAML file)."""
id: str
extends: str
edits: list[OverlayEdit]
priority: int = 10
enabled: bool = True
def _validate_safe_id(
value: str,
field_name: str,
allow_reserved: bool = False,
reserved_ids: frozenset[str] = _RESERVED_OVERLAY_WORKFLOW_IDS,
) -> str | None:
"""Return an error message if *value* is not a safe path segment ID."""
if not isinstance(value, str) or not value:
return f"Overlay '{field_name}' is required and must be a non-empty string."
if not _SAFE_ID_PATTERN.fullmatch(value):
return (
f"Overlay '{field_name}' {value!r} contains invalid characters; "
"only lowercase letters, digits, and hyphens are allowed."
)
if not allow_reserved and value in reserved_ids:
return f"Overlay '{field_name}' {value!r} is reserved."
return None
def _parse_edit(edit_raw: dict[str, Any], idx: int) -> tuple[OverlayEdit | None, str | None]:
"""Parse a single edit dict into an OverlayEdit or an error string."""
shorthand_keys = [key for key in _SHORTHAND_OPERATION_KEYS if key in edit_raw]
has_operation = "operation" in edit_raw
operation: str | None = None
anchor: Any = None
if shorthand_keys and has_operation:
return None, (
f"Edit at index {idx} mixes shorthand operation key "
f"({shorthand_keys[0]!r}) with explicit 'operation' field."
)
if len(shorthand_keys) > 1:
return None, (
f"Edit at index {idx} has multiple operation keys: "
f"{', '.join(repr(k) for k in shorthand_keys)}."
)
if shorthand_keys:
operation = shorthand_keys[0]
anchor = edit_raw[operation]
elif has_operation:
operation = edit_raw.get("operation")
anchor = edit_raw.get("anchor")
else:
return None, f"Edit at index {idx} has no operation; expected one of {sorted(VALID_OPERATIONS)}."
if operation not in VALID_OPERATIONS:
return None, f"Edit at index {idx} has invalid operation {operation!r}."
if not isinstance(anchor, str) or not anchor:
return None, f"Edit at index {idx} has invalid 'anchor'."
step = edit_raw.get("step")
if operation == "remove":
if step is not None:
return None, f"Edit at index {idx} ('remove') must not include 'step'."
return OverlayEdit(operation=operation, anchor=anchor), None
if not isinstance(step, dict):
return None, f"Edit at index {idx} ('{operation}') requires 'step' mapping."
step_id = step.get("id")
if not isinstance(step_id, str) or not step_id:
return None, f"Edit at index {idx} step is missing required 'id'."
if ":" in step_id:
return None, (
f"Edit at index {idx} step id {step_id!r} contains ':' "
"which is reserved for engine-generated nested IDs."
)
return OverlayEdit(operation=operation, anchor=anchor, step=step), None
def validate_overlay_yaml(data: dict[str, Any]) -> tuple[Overlay | None, list[str]]:
"""Validate an overlay manifest dict and return (Overlay, errors).
Errors are returned as a list of strings; validation never raises.
"""
errors: list[str] = []
if not isinstance(data, dict):
return None, ["Overlay manifest must be a mapping."]
overlay_id = data.get("id")
if err := _validate_safe_id(overlay_id, "id"):
errors.append(err)
overlay_id = ""
extends = data.get("extends")
if err := _validate_safe_id(
extends,
"extends",
reserved_ids=_RESERVED_WORKFLOW_IDS,
):
errors.append(err)
extends = ""
priority = normalize_priority(data.get("priority", 10))
edits_raw = data.get("edits")
edits: list[OverlayEdit] = []
if not isinstance(edits_raw, list):
errors.append("Overlay 'edits' is required and must be a list.")
elif not edits_raw:
errors.append("Overlay 'edits' must be a non-empty list.")
else:
for idx, edit_raw in enumerate(edits_raw):
if not isinstance(edit_raw, dict):
errors.append(f"Edit at index {idx} must be a mapping.")
continue
edit, err = _parse_edit(edit_raw, idx)
if err:
errors.append(err)
continue
if edit is not None:
edits.append(edit)
enabled = data.get("enabled", True)
if not isinstance(enabled, bool):
errors.append("Overlay 'enabled' must be a boolean.")
enabled = bool(enabled)
if errors:
return None, errors
return (
Overlay(
id=overlay_id,
extends=extends,
priority=priority,
edits=edits,
enabled=enabled,
),
[],
)

32
tests/test_workflows.py
View File

@@ -7091,7 +7091,7 @@ class TestWorkflowRemoveGuard:
assert "Invalid workflow ID" in result.output
assert sentinel.read_text(encoding="utf-8") == "keep"
@pytest.mark.parametrize("workflow_id", ["runs", "steps"])
@pytest.mark.parametrize("workflow_id", ["overlays", "runs", "steps"])
def test_remove_rejects_reserved_storage_ids(
self, project_dir, monkeypatch, workflow_id
):
@@ -7477,9 +7477,39 @@ steps:
# Literal bracketed text survives; Rich did not consume it as a tag.
assert "[red]evil[/red]" in out
def test_add_rejects_reserved_overlay_storage_id(self, temp_dir, monkeypatch):
"""workflow add must not install into the overlay storage directory."""
from typer.testing import CliRunner
from specify_cli import app
(temp_dir / ".specify" / "workflows").mkdir(parents=True)
overlay_file = temp_dir / "incoming.yml"
overlay_file.write_text(
"""
schema_version: "1.0"
workflow:
id: "overlays"
name: "Bad Workflow"
version: "1.0.0"
steps:
- id: step-one
command: speckit.specify
""".strip()
+ "\n",
encoding="utf-8",
)
monkeypatch.chdir(temp_dir)
result = CliRunner().invoke(app, ["workflow", "add", str(overlay_file)])
assert result.exit_code != 0
assert "Invalid workflow ID" in result.output
assert not (temp_dir / ".specify" / "workflows" / "overlays" / "workflow.yml").exists()
@pytest.mark.parametrize(
"workflow_id",
[
"overlays",
"runs",
"steps",
"nested/workflow",

26
tests/workflows/conftest.py Normal file
View File

@@ -0,0 +1,26 @@
"""Shared fixtures for workflow tests."""
from __future__ import annotations
import shutil
import sys
import tempfile
from pathlib import Path
import pytest
@pytest.fixture
def temp_dir():
"""Create a temporary directory for tests."""
tmpdir = tempfile.mkdtemp()
yield Path(tmpdir)
shutil.rmtree(tmpdir, ignore_errors=(sys.platform == "win32"))
@pytest.fixture
def project_dir(temp_dir):
"""Create a mock spec-kit project with ``.specify/workflows/`` directory."""
workflows_dir = temp_dir / ".specify" / "workflows"
workflows_dir.mkdir(parents=True, exist_ok=True)
return temp_dir

815
tests/workflows/test_overlay_commands.py Normal file
View File

@@ -0,0 +1,815 @@
"""Tests for workflow overlay CLI commands."""
from __future__ import annotations
from pathlib import Path
import pytest
import typer
import yaml
from typer.testing import CliRunner
from specify_cli import app
runner = CliRunner()
@pytest.fixture
def project_dir(tmp_path):
"""Create a mock spec-kit project with ``.specify/workflows/`` directory."""
workflows_dir = tmp_path / ".specify" / "workflows"
workflows_dir.mkdir(parents=True, exist_ok=True)
return tmp_path
def _write_workflow(project_root: Path, workflow_id: str, data: dict) -> Path:
wf_dir = project_root / ".specify" / "workflows" / workflow_id
wf_dir.mkdir(parents=True, exist_ok=True)
wf_path = wf_dir / "workflow.yml"
wf_path.write_text(yaml.safe_dump(data), encoding="utf-8")
return wf_path
def _write_overlay(project_root: Path, workflow_id: str, overlay_id: str, data: dict) -> Path:
ov_dir = project_root / ".specify" / "workflows" / "overlays" / workflow_id
ov_dir.mkdir(parents=True, exist_ok=True)
ov_path = ov_dir / f"{overlay_id}.yml"
ov_path.write_text(yaml.safe_dump(data), encoding="utf-8")
return ov_path
class TestOverlayCli:
"""CLI-level tests for ``specify workflow overlay *``."""
def test_overlay_add(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
}
),
encoding="utf-8",
)
result = runner.invoke(
app, ["workflow", "overlay", "add", str(overlay_file), "--priority", "5"]
)
assert result.exit_code == 0, result.output
assert "Overlay 'ov1' added" in result.output
installed = project_dir / ".specify" / "workflows" / "overlays" / "wf" / "ov1.yml"
assert installed.is_file()
data = yaml.safe_load(installed.read_text(encoding="utf-8"))
assert data["priority"] == 5
def test_overlay_add_reuses_yaml_extension(self, project_dir, monkeypatch):
"""If <id>.yaml already exists, overlay add must write to it instead of creating <id>.yml."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
# Pre-create the overlay using the .yaml extension.
existing_yaml = project_dir / ".specify" / "workflows" / "overlays" / "wf" / "ov1.yaml"
existing_yaml.parent.mkdir(parents=True, exist_ok=True)
existing_yaml.write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "wf",
"priority": 1,
"edits": [{"remove": "a"}],
}
),
encoding="utf-8",
)
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "wf",
"priority": 20,
"edits": [{"remove": "a"}],
}
),
encoding="utf-8",
)
result = runner.invoke(app, ["workflow", "overlay", "add", str(overlay_file)])
assert result.exit_code == 0, result.output
# Should have written to the pre-existing .yaml file.
assert existing_yaml.is_file()
data = yaml.safe_load(existing_yaml.read_text(encoding="utf-8"))
assert data["priority"] == 10
# Must NOT have created a duplicate .yml alongside the .yaml.
duplicate_yml = existing_yaml.with_suffix(".yml")
assert not duplicate_yml.exists(), "duplicate .yml was created alongside existing .yaml"
assert list(existing_yaml.parent.glob(f".{existing_yaml.name}.*.bak")) == []
def test_overlay_add_with_priority_override_missing_in_file(self, project_dir, monkeypatch):
"""--priority must fix a missing priority in the overlay file."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
# Overlay file has NO priority field
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "wf",
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
}
),
encoding="utf-8",
)
result = runner.invoke(
app, ["workflow", "overlay", "add", str(overlay_file), "--priority", "5"]
)
assert result.exit_code == 0, result.output
assert "Overlay 'ov1' added" in result.output
installed = project_dir / ".specify" / "workflows" / "overlays" / "wf" / "ov1.yml"
assert installed.is_file()
data = yaml.safe_load(installed.read_text(encoding="utf-8"))
assert data["priority"] == 5
def test_overlay_add_defaults_priority_to_ten(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "wf",
"edits": [{"remove": "a"}],
}
),
encoding="utf-8",
)
result = runner.invoke(app, ["workflow", "overlay", "add", str(overlay_file)])
assert result.exit_code == 0, result.output
installed = project_dir / ".specify" / "workflows" / "overlays" / "wf" / "ov1.yml"
assert yaml.safe_load(installed.read_text(encoding="utf-8"))["priority"] == 10
def test_overlay_add_rejects_non_positive_priority(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "wf",
"edits": [{"remove": "a"}],
}
),
encoding="utf-8",
)
result = runner.invoke(
app,
["workflow", "overlay", "add", str(overlay_file), "--priority", "0"],
)
assert result.exit_code == 1
assert "must be >= 1" in result.output
def test_overlay_set_priority(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
result = runner.invoke(
app, ["workflow", "overlay", "set-priority", "wf", "ov1", "20"]
)
assert result.exit_code == 0, result.output
data = yaml.safe_load(
(
project_dir / ".specify" / "workflows" / "overlays" / "wf" / "ov1.yml"
).read_text(encoding="utf-8")
)
assert data["priority"] == 20
assert list(
(project_dir / ".specify" / "workflows" / "overlays" / "wf").glob(
".ov1.yml.*.bak"
)
) == []
def test_overlay_set_priority_rejects_zero(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
result = runner.invoke(
app, ["workflow", "overlay", "set-priority", "wf", "ov1", "0"]
)
assert result.exit_code == 1
assert "must be >= 1" in result.output
def test_overlay_set_priority_rejects_ids_with_trailing_newline(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
result = runner.invoke(
app, ["workflow", "overlay", "set-priority", "wf", "ov1\n", "20"]
)
assert result.exit_code == 1
assert "Invalid overlay ID" in result.output
result = runner.invoke(
app, ["workflow", "overlay", "set-priority", "wf\n", "ov1", "20"]
)
assert result.exit_code == 1
assert "Invalid workflow ID" in result.output
def test_overlay_disable_and_enable(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
result = runner.invoke(app, ["workflow", "overlay", "disable", "wf", "ov1"])
assert result.exit_code == 0, result.output
data = yaml.safe_load(
(
project_dir / ".specify" / "workflows" / "overlays" / "wf" / "ov1.yml"
).read_text(encoding="utf-8")
)
assert data["enabled"] is False
result = runner.invoke(app, ["workflow", "overlay", "enable", "wf", "ov1"])
assert result.exit_code == 0, result.output
data = yaml.safe_load(
(
project_dir / ".specify" / "workflows" / "overlays" / "wf" / "ov1.yml"
).read_text(encoding="utf-8")
)
assert data["enabled"] is True
def test_overlay_remove(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
result = runner.invoke(app, ["workflow", "overlay", "remove", "wf", "ov1"])
assert result.exit_code == 0, result.output
assert not (
project_dir / ".specify" / "workflows" / "overlays" / "wf" / "ov1.yml"
).exists()
def test_overlay_list(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
result = runner.invoke(app, ["workflow", "overlay", "list", "wf"])
assert result.exit_code == 0, result.output
assert "ov1" in result.output
def test_overlay_list_shows_disabled_overlay(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"enabled": False,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
result = runner.invoke(app, ["workflow", "overlay", "list", "wf"])
assert result.exit_code == 0, result.output
assert "ov1" in result.output
assert "disabled" in result.output
def test_workflow_resolve(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
result = runner.invoke(app, ["workflow", "resolve", "wf"])
assert result.exit_code == 0, result.output
assert "base" in result.output
assert "project:ov1" in result.output
assert "new" in result.output
assert "priority=n/a" in result.output
from specify_cli.workflows.overlays._commands import workflow_resolve
payload = workflow_resolve(project_dir, "wf")
assert payload is not None
assert payload["layers"][-1]["tier"] == "base"
assert payload["layers"][-1]["priority"] is None
def test_workflow_resolve_equal_priority_layers_sort_by_source(self, project_dir, monkeypatch):
"""Equal-priority overlays are listed alphabetically by source."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
# "zzz" sorts last alphabetically, so the composer applies it last and wins.
# Resolver layer output follows the common priority/source sort order.
_write_overlay(
project_dir,
"wf",
"aaa",
{
"id": "aaa",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "aaa-step", "type": "command", "command": "echo"},
}
],
},
)
_write_overlay(
project_dir,
"wf",
"zzz",
{
"id": "zzz",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "zzz-step", "type": "command", "command": "echo"},
}
],
},
)
result = runner.invoke(app, ["workflow", "resolve", "wf"])
assert result.exit_code == 0, result.output
zzz_pos = result.output.index("project:zzz")
aaa_pos = result.output.index("project:aaa")
assert aaa_pos < zzz_pos
def test_workflow_add_does_not_copy_overlays(self, project_dir, monkeypatch, tmp_path):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
source_dir = tmp_path / "source-wf"
source_dir.mkdir()
(source_dir / "workflow.yml").write_text(
yaml.safe_dump(
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
}
),
encoding="utf-8",
)
overlays_dir = source_dir / "overlays"
overlays_dir.mkdir()
(overlays_dir / "ov1.yml").write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
}
),
encoding="utf-8",
)
result = runner.invoke(app, ["workflow", "add", str(source_dir)])
assert result.exit_code == 0, result.output
# Overlays in the source directory should NOT be copied — workflow add
# only installs the workflow.yml, not sibling overlays.
installed_overlay = (
project_dir / ".specify" / "workflows" / "wf" / "overlays" / "ov1.yml"
)
assert not installed_overlay.exists()
class TestOverlayFilenameVsManifestId:
"""Overlay identity must come from the manifest ``id`` field, not the filename.
This matches the project-wide convention: presets use ``preset.id``,
extensions use ``extension.id``, workflows use ``workflow.id``, and
workflow steps use ``step.type_key``. Overlays must follow the same pattern.
"""
def _write_mismatched_overlay(
self, project_root: Path, workflow_id: str, filename: str, manifest_id: str, data: dict
) -> Path:
"""Write an overlay file where filename != manifest id."""
ov_dir = project_root / ".specify" / "workflows" / "overlays" / workflow_id
ov_dir.mkdir(parents=True, exist_ok=True)
ov_path = ov_dir / filename
ov_path.write_text(yaml.safe_dump(data), encoding="utf-8")
return ov_path
def test_find_overlay_by_manifest_id_not_filename(self, project_dir, monkeypatch):
"""_find_overlay_file must locate overlays by manifest id, not filename."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
# File is named "custom.yml" but manifest declares id: "lint"
self._write_mismatched_overlay(
project_dir,
"wf",
"custom.yml",
"lint",
{
"id": "lint",
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
},
)
from specify_cli.workflows.overlays._commands import _find_overlay_file
# Must find by manifest id "lint", not by filename "custom"
found = _find_overlay_file(project_dir, "wf", "lint")
assert found is not None
assert found.name == "custom.yml"
# Must NOT find by filename stem "custom"
not_found = _find_overlay_file(project_dir, "wf", "custom")
assert not_found is None
def test_enable_disable_with_mismatched_filename(self, project_dir, monkeypatch):
"""enable/disable must work when filename != manifest id."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
self._write_mismatched_overlay(
project_dir,
"wf",
"custom.yml",
"lint",
{
"id": "lint",
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
},
)
result = runner.invoke(app, ["workflow", "overlay", "disable", "wf", "lint"])
assert result.exit_code == 0, result.output
data = yaml.safe_load(
(project_dir / ".specify" / "workflows" / "overlays" / "wf" / "custom.yml").read_text(
encoding="utf-8"
)
)
assert data["enabled"] is False
result = runner.invoke(app, ["workflow", "overlay", "enable", "wf", "lint"])
assert result.exit_code == 0, result.output
data = yaml.safe_load(
(project_dir / ".specify" / "workflows" / "overlays" / "wf" / "custom.yml").read_text(
encoding="utf-8"
)
)
assert data["enabled"] is True
def test_set_priority_with_mismatched_filename(self, project_dir, monkeypatch):
"""set-priority must work when filename != manifest id."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
self._write_mismatched_overlay(
project_dir,
"wf",
"custom.yml",
"lint",
{
"id": "lint",
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
},
)
result = runner.invoke(app, ["workflow", "overlay", "set-priority", "wf", "lint", "25"])
assert result.exit_code == 0, result.output
data = yaml.safe_load(
(project_dir / ".specify" / "workflows" / "overlays" / "wf" / "custom.yml").read_text(
encoding="utf-8"
)
)
assert data["priority"] == 25
def test_remove_with_mismatched_filename(self, project_dir, monkeypatch):
"""remove must work when filename != manifest id."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
self._write_mismatched_overlay(
project_dir,
"wf",
"custom.yml",
"lint",
{
"id": "lint",
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
},
)
result = runner.invoke(app, ["workflow", "overlay", "remove", "wf", "lint"])
assert result.exit_code == 0, result.output
assert not (
project_dir / ".specify" / "workflows" / "overlays" / "wf" / "custom.yml"
).exists()
def test_duplicate_manifest_id_is_rejected(self, project_dir, monkeypatch):
"""Two files with the same manifest ID are ambiguous."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
# Two files, both declare id: "lint"
self._write_mismatched_overlay(
project_dir,
"wf",
"aaa.yml",
"lint",
{
"id": "lint",
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
},
)
self._write_mismatched_overlay(
project_dir,
"wf",
"zzz.yml",
"lint",
{
"id": "lint",
"extends": "wf",
"priority": 20,
"edits": [{"remove": "a"}],
},
)
from specify_cli.workflows.overlays._commands import _find_overlay_file
with pytest.raises(typer.Exit):
_find_overlay_file(project_dir, "wf", "lint")

153
tests/workflows/test_overlay_composer.py Normal file
View File

@@ -0,0 +1,153 @@
"""Tests for StepListComposer validation and error handling."""
from __future__ import annotations
from pathlib import Path
import pytest
import yaml
from specify_cli.workflows.overlays import WorkflowResolver
from specify_cli.workflows.overlays.composer import StepListComposer
from specify_cli.workflows.overlays.layer_sources import BaseWorkflowSource, Layer
from specify_cli.workflows.overlays.schema import Overlay, OverlayEdit
def _write_workflow(project_root: Path, workflow_id: str, data: dict) -> Path:
wf_dir = project_root / ".specify" / "workflows" / workflow_id
wf_dir.mkdir(parents=True, exist_ok=True)
wf_path = wf_dir / "workflow.yml"
wf_path.write_text(yaml.safe_dump(data), encoding="utf-8")
return wf_path
class TestStepListComposerValidation:
"""Composer validates edits before applying them."""
def test_composer_reports_invalid_anchors(self, project_dir):
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
base_layer = BaseWorkflowSource(project_dir).collect("wf")[0]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[OverlayEdit("insert_after", "missing", {"id": "new", "type": "command", "command": "echo"})],
)
layer = Layer(content=overlay, source="project:ov", tier="project-overlay", priority=10)
composer = StepListComposer()
with pytest.raises(ValueError, match="does not match any base step id"):
composer.compose([base_layer, layer])
def test_composer_validates_edits_before_merge(self, project_dir):
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "a", {"id": "bad:id", "type": "command", "command": "echo"})],
)
layer = Layer(content=overlay, source="project:ov", tier="project-overlay", priority=10)
composer = StepListComposer()
with pytest.raises(ValueError, match="bad:id"):
composer.compose([BaseWorkflowSource(project_dir).collect("wf")[0], layer])
def test_resolver_reports_invalid_anchor_as_validation_error(self, project_dir):
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "missing",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
}
),
encoding="utf-8",
)
# Manually inject the overlay by writing it to disk in the correct location.
overlay_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
overlay_dir.mkdir(parents=True, exist_ok=True)
(overlay_dir / "ov.yml").write_text(overlay_file.read_text(encoding="utf-8"), encoding="utf-8")
resolver = WorkflowResolver(project_dir)
with pytest.raises(ValueError, match="missing"):
resolver.resolve("wf")
def test_composer_applies_lower_priority_last(self, project_dir):
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "base"}],
},
)
high_number = Overlay(
id="high-number",
extends="wf",
priority=20,
edits=[
OverlayEdit(
"replace",
"a",
{"id": "a", "type": "command", "command": "priority-20"},
)
],
)
low_number = Overlay(
id="low-number",
extends="wf",
priority=5,
edits=[
OverlayEdit(
"replace",
"a",
{"id": "a", "type": "command", "command": "priority-5"},
)
],
)
definition, attribution = StepListComposer().compose(
[
BaseWorkflowSource(project_dir).collect("wf")[0],
Layer(high_number, "project:high-number", "project-overlay", 20),
Layer(low_number, "project:low-number", "project-overlay", 5),
]
)
assert definition is not None
assert definition.data["steps"][0]["command"] == "priority-5"
assert attribution[0].source == "project:low-number"

272
tests/workflows/test_overlay_layer_sources.py Normal file
View File

@@ -0,0 +1,272 @@
"""Tests for ProjectOverlaySource and BaseWorkflowSource."""
from __future__ import annotations
from pathlib import Path
from unittest.mock import patch
import pytest
import yaml
from specify_cli.workflows.overlays.layer_sources import (
BaseWorkflowSource,
OverlayLoadError,
ProjectOverlaySource,
)
@pytest.fixture
def project_dir(tmp_path: Path) -> Path:
workflows_dir = tmp_path / ".specify" / "workflows"
workflows_dir.mkdir(parents=True, exist_ok=True)
return tmp_path
def _write_overlay_file(project_dir: Path, workflow_id: str, overlay_id: str, data: dict) -> Path:
ov_dir = project_dir / ".specify" / "workflows" / "overlays" / workflow_id
ov_dir.mkdir(parents=True, exist_ok=True)
path = ov_dir / f"{overlay_id}.yml"
path.write_text(yaml.safe_dump(data), encoding="utf-8")
return path
class TestProjectOverlaySourceFileReadErrors:
"""File-read errors must be wrapped in OverlayLoadError, not leaked as raw tracebacks."""
def test_oserror_raises_overlay_load_error(self, project_dir: Path) -> None:
"""An OSError from read_text (e.g. permission denied) is wrapped in OverlayLoadError."""
_write_overlay_file(
project_dir,
"wf",
"ov1",
{"id": "ov1", "extends": "wf", "priority": 5, "edits": []},
)
source = ProjectOverlaySource(project_dir)
with patch.object(Path, "read_text", side_effect=OSError("Permission denied")):
with pytest.raises(OverlayLoadError) as exc_info:
source.collect("wf")
assert exc_info.value.errors, "OverlayLoadError must carry a non-empty errors list"
def test_unicode_error_raises_overlay_load_error(self, project_dir: Path) -> None:
"""A file containing non-UTF-8 bytes raises OverlayLoadError, not UnicodeDecodeError."""
ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
ov_dir.mkdir(parents=True, exist_ok=True)
# Write raw invalid UTF-8 bytes directly so read_text(encoding="utf-8") fails.
bad_file = ov_dir / "bad.yml"
bad_file.write_bytes(b"\xff\xfe invalid utf-8")
source = ProjectOverlaySource(project_dir)
with pytest.raises(OverlayLoadError) as exc_info:
source.collect("wf")
assert exc_info.value.errors, "OverlayLoadError must carry a non-empty errors list"
_UNSAFE_IDS = [
"../outside",
"../../escape",
"nested/workflow",
"wf\n",
"overlays",
"runs",
"steps",
"",
"/absolute",
"UPPER",
"has space",
]
class TestProjectOverlaySourceIdValidation:
"""ProjectOverlaySource.collect() must reject unsafe IDs before path construction."""
@pytest.mark.parametrize("workflow_id", _UNSAFE_IDS)
def test_rejects_unsafe_id(self, project_dir: Path, workflow_id: str) -> None:
source = ProjectOverlaySource(project_dir)
with pytest.raises(OverlayLoadError, match="Invalid workflow ID"):
source.collect(workflow_id)
@pytest.mark.parametrize("workflow_id", _UNSAFE_IDS)
def test_does_not_access_filesystem_for_unsafe_id(
self, project_dir: Path, workflow_id: str
) -> None:
"""No directory walk or file read should happen for an invalid ID."""
source = ProjectOverlaySource(project_dir)
with patch.object(Path, "iterdir", side_effect=AssertionError("iterdir called")):
with pytest.raises(OverlayLoadError, match="Invalid workflow ID"):
source.collect(workflow_id)
class TestProjectOverlaySourceContainment:
"""ProjectOverlaySource.collect() must enforce containment of the workflow overlay dir."""
def test_rejects_symlinked_workflow_overlay_dir(self, project_dir: Path, tmp_path: Path) -> None:
"""A symlinked per-workflow overlay directory must be rejected."""
real_dir = tmp_path / "real-overlay"
real_dir.mkdir()
overlay_root = project_dir / ".specify" / "workflows" / "overlays"
overlay_root.mkdir(parents=True, exist_ok=True)
link = overlay_root / "wf"
link.symlink_to(real_dir)
source = ProjectOverlaySource(project_dir)
with pytest.raises(OverlayLoadError, match="Symlinked overlay directories are not allowed"):
source.collect("wf")
def test_rejects_workflow_overlay_dir_escaping_root(
self, project_dir: Path, tmp_path: Path
) -> None:
"""A workflow overlay dir that resolves outside the overlay root must be rejected.
This requires the ID itself to pass validation but the resolved path to escape —
which is possible if the overlay root itself is a junction/mount that resolves
outside the project root; or in edge cases on case-insensitive file systems.
We simulate it by patching Path.resolve to return an outside path.
"""
overlay_root = project_dir / ".specify" / "workflows" / "overlays"
overlay_root.mkdir(parents=True, exist_ok=True)
workflow_overlay_dir = overlay_root / "wf"
workflow_overlay_dir.mkdir()
outside = tmp_path / "outside" / "wf"
outside.mkdir(parents=True)
original_resolve = Path.resolve
def fake_resolve(self: Path, **kwargs: object) -> Path:
if self == workflow_overlay_dir:
return outside
return original_resolve(self, **kwargs)
source = ProjectOverlaySource(project_dir)
with patch.object(Path, "resolve", fake_resolve):
with pytest.raises(OverlayLoadError, match="Path traversal detected"):
source.collect("wf")
class TestProjectOverlaySourceDisabledFiltering:
"""ProjectOverlaySource.collect() should expose disabled entries only on opt-in."""
def test_skips_disabled_by_default(self, project_dir: Path) -> None:
_write_overlay_file(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 5,
"enabled": False,
"edits": [{"remove": "a"}],
},
)
source = ProjectOverlaySource(project_dir)
assert source.collect("wf") == []
def test_can_include_disabled_for_management_views(self, project_dir: Path) -> None:
_write_overlay_file(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 5,
"enabled": False,
"edits": [{"remove": "a"}],
},
)
source = ProjectOverlaySource(project_dir)
layers = source.collect("wf", include_disabled=True)
assert [layer.content.id for layer in layers] == ["ov1"]
assert layers[0].content.enabled is False
def test_skips_invalid_disabled_overlay_during_resolution(self, project_dir: Path) -> None:
_write_overlay_file(
project_dir,
"wf",
"disabled",
{
"id": "disabled",
"extends": "wf",
"enabled": False,
"edits": "not-a-list",
},
)
source = ProjectOverlaySource(project_dir)
assert source.collect("wf") == []
with pytest.raises(OverlayLoadError, match="edits"):
source.collect("wf", include_disabled=True)
def test_rejects_duplicate_manifest_ids(self, project_dir: Path) -> None:
data = {
"id": "duplicate",
"extends": "wf",
"edits": [{"remove": "a"}],
}
_write_overlay_file(project_dir, "wf", "first", data)
_write_overlay_file(project_dir, "wf", "second", data)
with pytest.raises(OverlayLoadError, match="Duplicate overlay id"):
ProjectOverlaySource(project_dir).collect("wf")
class TestBaseWorkflowSourceIdValidation:
"""BaseWorkflowSource.collect() must reject unsafe IDs before path construction."""
@pytest.mark.parametrize("workflow_id", _UNSAFE_IDS)
def test_rejects_unsafe_id(self, project_dir: Path, workflow_id: str) -> None:
source = BaseWorkflowSource(project_dir)
with pytest.raises(OverlayLoadError, match="Invalid workflow ID"):
source.collect(workflow_id)
class TestBaseWorkflowSourceContainment:
"""BaseWorkflowSource.collect() must enforce the same checks as _safe_workflow_id_dir."""
def test_rejects_symlinked_workflow_dir(self, project_dir: Path, tmp_path: Path) -> None:
"""A symlinked workflow directory must be rejected."""
real_dir = tmp_path / "real-wf"
real_dir.mkdir()
(real_dir / "workflow.yml").write_text("schema_version: '1.0'\n", encoding="utf-8")
workflows_dir = project_dir / ".specify" / "workflows"
workflows_dir.mkdir(parents=True, exist_ok=True)
link = workflows_dir / "wf"
link.symlink_to(real_dir)
source = BaseWorkflowSource(project_dir)
with pytest.raises(OverlayLoadError, match="Symlinked overlay directories are not allowed"):
source.collect("wf")
def test_rejects_symlinked_workflow_yml(self, project_dir: Path, tmp_path: Path) -> None:
"""A symlinked workflow.yml must be rejected even if the directory is real."""
real_yml = tmp_path / "workflow.yml"
real_yml.write_text("schema_version: '1.0'\n", encoding="utf-8")
workflows_dir = project_dir / ".specify" / "workflows"
wf_dir = workflows_dir / "wf"
wf_dir.mkdir(parents=True, exist_ok=True)
link = wf_dir / "workflow.yml"
link.symlink_to(real_yml)
source = BaseWorkflowSource(project_dir)
with pytest.raises(OverlayLoadError, match="Symlinked workflow files are not allowed"):
source.collect("wf")
def test_missing_workflow_returns_empty(self, project_dir: Path) -> None:
"""A workflow directory that does not exist returns an empty layer list."""
source = BaseWorkflowSource(project_dir)
assert source.collect("no-such-wf") == []
def test_rejects_symlinked_workflows_root(self, project_dir: Path, tmp_path: Path) -> None:
outside = tmp_path / "outside"
outside.mkdir()
workflows_dir = project_dir / ".specify" / "workflows"
workflows_dir.rmdir()
workflows_dir.symlink_to(outside)
with pytest.raises(OverlayLoadError, match="Symlinked workflow directories"):
BaseWorkflowSource(project_dir).collect("wf")

718
tests/workflows/test_overlay_merge.py Normal file
View File

@@ -0,0 +1,718 @@
"""Tests for the workflow overlay merge engine."""
from __future__ import annotations
import copy
from typing import Any
import pytest
from specify_cli.workflows.overlays.merge import (
ComposedStep,
OverlayLayer,
find_step,
merge_steps,
validate_edits,
)
from specify_cli.workflows.overlays.schema import Overlay, OverlayEdit
def _step(id: str, **kwargs: Any) -> dict[str, Any]: # noqa: A002
"""Build a minimal step dict with the given id."""
return {"id": id, "type": "command", "command": "speckit.specify", **kwargs}
def _layer(overlay: Overlay, source: str) -> OverlayLayer:
"""Build an OverlayLayer for merge_steps."""
return OverlayLayer(overlay, source)
class TestFindStep:
"""Recursive anchor lookup across nested step lists."""
def test_find_step_flat(self):
steps = [_step("a"), _step("b"), _step("c")]
result = find_step(steps, "b")
assert result is not None
assert result[0] is steps
assert result[1] == 1
def test_find_step_missing(self):
steps = [_step("a"), _step("b")]
assert find_step(steps, "missing") is None
def test_find_step_in_then(self):
steps = [
{
"id": "if-1",
"type": "if",
"condition": "true",
"then": [_step("then-a")],
"else": [_step("else-b")],
},
]
result = find_step(steps, "then-a")
assert result is not None
assert result[0] is steps[0]["then"]
assert result[1] == 0
def test_find_step_in_else(self):
steps = [
{
"id": "if-1",
"type": "if",
"condition": "true",
"then": [_step("then-a")],
"else": [_step("else-b")],
},
]
result = find_step(steps, "else-b")
assert result is not None
assert result[0] is steps[0]["else"]
assert result[1] == 0
def test_find_step_in_nested_steps(self):
steps = [
{
"id": "while-1",
"type": "while",
"condition": "true",
"steps": [_step("inner-a"), _step("inner-b")],
},
]
result = find_step(steps, "inner-b")
assert result is not None
assert result[0] is steps[0]["steps"]
assert result[1] == 1
def test_find_step_in_switch_cases(self):
steps = [
{
"id": "switch-1",
"type": "switch",
"expression": "{{ inputs.x }}",
"cases": {
"one": [_step("case-a")],
"two": [_step("case-b")],
},
"default": [_step("default-c")],
},
]
assert find_step(steps, "case-b")[1] == 0
assert find_step(steps, "default-c")[1] == 0
def test_find_step_not_in_fan_out_template(self):
steps = [
{
"id": "fan-1",
"type": "fan-out",
"items": "{{ inputs.items }}",
"step": {"id": "template-x", "type": "command", "command": "echo"},
},
]
assert find_step(steps, "template-x") is None
class TestMergeSteps:
"""Composition of multiple overlays in merge order."""
def test_merge_steps_no_overlays(self):
base = [_step("a"), _step("b")]
steps, attribution = merge_steps(base, [])
assert [s["id"] for s in steps] == ["a", "b"]
assert attribution == [ComposedStep("a", "base"), ComposedStep("b", "base")]
def test_merge_steps_single_overlay(self):
base = [_step("a"), _step("b")]
overlay = Overlay(
id="ov1",
extends="wf",
priority=10,
edits=[OverlayEdit("insert_after", "a", _step("new"))],
)
steps, attribution = merge_steps(base, [_layer(overlay, "project:ov1")])
assert [s["id"] for s in steps] == ["a", "new", "b"]
assert attribution == [
ComposedStep("a", "base"),
ComposedStep("new", "project:ov1"),
ComposedStep("b", "base"),
]
def test_merge_steps_higher_priority_wins(self):
base = [_step("a")]
low = Overlay(
id="low",
extends="wf",
priority=5,
edits=[OverlayEdit("insert_after", "a", _step("low-step"))],
)
high = Overlay(
id="high",
extends="wf",
priority=10,
edits=[OverlayEdit("insert_after", "a", _step("high-step"))],
)
steps, attribution = merge_steps(base, [_layer(low, "project:low"), _layer(high, "project:high")])
# low applied first, then high; both insert after 'a', so high-step ends
# closer to the anchor (higher priority wins the conflict).
assert [s["id"] for s in steps] == ["a", "high-step", "low-step"]
assert attribution == [
ComposedStep("a", "base"),
ComposedStep("high-step", "project:high"),
ComposedStep("low-step", "project:low"),
]
def test_merge_steps_replace_wins_over_insert(self):
"""Overlays apply to the original tree only; targeting an overlay-introduced step raises."""
base = [_step("a")]
insert = Overlay(
id="insert",
extends="wf",
priority=5,
edits=[OverlayEdit("insert_after", "a", _step("inserted"))],
)
replace = Overlay(
id="replace",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "inserted", _step("replaced"))],
)
# "inserted" is not a base step — overlays cannot target each other's steps.
with pytest.raises(ValueError, match="Anchor 'inserted' not found"):
merge_steps(base, [_layer(insert, "project:insert"), _layer(replace, "project:replace")])
def test_merge_steps_does_not_mutate_base(self):
base = [_step("a")]
overlay = Overlay(
id="ov1",
extends="wf",
priority=10,
edits=[OverlayEdit("insert_after", "a", _step("new"))],
)
original = copy.deepcopy(base)
merge_steps(base, [_layer(overlay, "project:ov1")])
assert base == original
def test_merge_steps_attribution_uses_source_not_overlay_id(self):
base = [_step("a")]
overlay = Overlay(
id="same-id",
extends="wf",
priority=10,
edits=[OverlayEdit("insert_after", "a", _step("new"))],
)
steps, attribution = merge_steps(base, [_layer(overlay, "installed:same-id")])
assert [s["id"] for s in steps] == ["a", "new"]
assert attribution == [
ComposedStep("a", "base"),
ComposedStep("new", "installed:same-id"),
]
def test_merge_steps_nested_base_attribution(self):
base = [
{
"id": "if-1",
"type": "if",
"condition": "true",
"then": [_step("then-a")],
"else": [_step("else-b")],
},
]
steps, attribution = merge_steps(base, [])
assert attribution == [
ComposedStep("if-1", "base"),
ComposedStep("then-a", "base"),
ComposedStep("else-b", "base"),
]
def test_merge_steps_higher_replace_wins_lower_replace_same_anchor(self):
base = [_step("implement")]
low = Overlay(
id="low",
extends="wf",
priority=5,
edits=[OverlayEdit("replace", "implement", _step("low-implement"))],
)
high = Overlay(
id="high",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "implement", _step("high-implement"))],
)
steps, attribution = merge_steps(base, [_layer(low, "project:low"), _layer(high, "project:high")])
assert [s["id"] for s in steps] == ["high-implement"]
assert any(
composed.step_id == "high-implement" and composed.source == "project:high"
for composed in attribution
)
def test_merge_steps_higher_replace_wins_after_lower_remove_same_anchor(self):
base = [_step("implement")]
low = Overlay(
id="low",
extends="wf",
priority=5,
edits=[OverlayEdit("remove", "implement")],
)
high = Overlay(
id="high",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "implement", _step("high-implement"))],
)
steps, attribution = merge_steps(base, [_layer(low, "project:low"), _layer(high, "project:high")])
assert [s["id"] for s in steps] == ["high-implement"]
def test_merge_steps_higher_insert_wins_after_lower_remove_same_anchor(self):
base = [_step("implement")]
low = Overlay(
id="low",
extends="wf",
priority=5,
edits=[OverlayEdit("remove", "implement")],
)
high = Overlay(
id="high",
extends="wf",
priority=10,
edits=[OverlayEdit("insert_after", "implement", _step("high-after"))],
)
steps, attribution = merge_steps(base, [_layer(low, "project:low"), _layer(high, "project:high")])
assert [s["id"] for s in steps] == ["implement", "high-after"]
assert attribution == [
ComposedStep("implement", "base"),
ComposedStep("high-after", "project:high"),
]
def test_merge_steps_later_overlay_wins_tie_same_anchor(self):
"""When two overlays have the same priority, the one applied later wins."""
base = [_step("a")]
first = Overlay(
id="first",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "a", _step("first-replace"))],
)
second = Overlay(
id="second",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "a", _step("second-replace"))],
)
# Merge order: first applied, then second wins tie.
steps, attribution = merge_steps(
base,
[
_layer(first, "overlay:first"),
_layer(second, "overlay:second"),
],
)
assert [s["id"] for s in steps] == ["second-replace"]
assert any(
composed.step_id == "second-replace" and composed.source == "overlay:second"
for composed in attribution
)
def test_merge_steps_insert_after_then_replace_same_anchor_id_change(self):
"""Inserts must be applied before the winning replace so the anchor still exists.
Regression: when a replace changes the step ID, applying it before inserts
causes ``find_step`` to fail on the now-gone original anchor.
"""
base = [_step("build")]
low = Overlay(
id="low",
extends="wf",
priority=5,
edits=[OverlayEdit("insert_after", "build", _step("test"))],
)
high = Overlay(
id="high",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "build", _step("compile"))],
)
steps, attribution = merge_steps(
base, [_layer(low, "project:low"), _layer(high, "project:high")]
)
# The insert should land after the original anchor position, then the
# anchor is replaced. Final order: ["compile", "test"].
assert [s["id"] for s in steps] == ["compile", "test"]
assert attribution == [
ComposedStep("compile", "project:high"),
ComposedStep("test", "project:low"),
]
def test_merge_steps_insert_before_then_replace_same_anchor_id_change(self):
"""Same as above but with insert_before — anchor must still be findable."""
base = [_step("build")]
low = Overlay(
id="low",
extends="wf",
priority=5,
edits=[OverlayEdit("insert_before", "build", _step("lint"))],
)
high = Overlay(
id="high",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "build", _step("compile"))],
)
steps, attribution = merge_steps(
base, [_layer(low, "project:low"), _layer(high, "project:high")]
)
assert [s["id"] for s in steps] == ["lint", "compile"]
assert attribution == [
ComposedStep("lint", "project:low"),
ComposedStep("compile", "project:high"),
]
def test_merge_steps_unknown_anchor_still_raises(self):
base = [_step("a")]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[OverlayEdit("replace", "missing", _step("new"))],
)
with pytest.raises(ValueError, match="Anchor 'missing' not found"):
merge_steps(base, [_layer(overlay, "project:ov")])
# ── composite step attribution ───────────────────────────────────────
def test_merge_insert_composite_if_attribution(self):
"""Nested then/else children of an inserted 'if' step get the overlay source."""
base = [_step("a")]
composite = {
"id": "if-1",
"type": "if",
"condition": "true",
"then": [_step("then-a")],
"else": [_step("else-b")],
}
overlay = Overlay(
id="ov", extends="wf", priority=10,
edits=[OverlayEdit("insert_after", "a", composite)],
)
_steps, attribution = merge_steps(
base, [_layer(overlay, "project:ov")]
)
assert attribution == [
ComposedStep("a", "base"),
ComposedStep("if-1", "project:ov"),
ComposedStep("then-a", "project:ov"),
ComposedStep("else-b", "project:ov"),
]
def test_merge_insert_composite_switch_attribution(self):
"""Nested cases/default children of an inserted 'switch' step get the overlay source."""
base = [_step("a")]
composite = {
"id": "switch-1",
"type": "switch",
"expression": "{{inputs.x}}",
"cases": {"one": [_step("case-one")], "two": [_step("case-two")]},
"default": [_step("default-z")],
}
overlay = Overlay(
id="ov", extends="wf", priority=10,
edits=[OverlayEdit("insert_before", "a", composite)],
)
_steps, attribution = merge_steps(
base, [_layer(overlay, "project:ov")]
)
assert attribution == [
ComposedStep("switch-1", "project:ov"),
ComposedStep("default-z", "project:ov"),
ComposedStep("case-one", "project:ov"),
ComposedStep("case-two", "project:ov"),
ComposedStep("a", "base"),
]
def test_merge_replace_flat_with_composite_attribution(self):
"""Replacing a flat step with a composite step attributes all nested children."""
base = [_step("a")]
composite = {
"id": "if-1",
"type": "if",
"condition": "true",
"then": [_step("inner-x"), _step("inner-y")],
}
overlay = Overlay(
id="ov", extends="wf", priority=10,
edits=[OverlayEdit("replace", "a", composite)],
)
_steps, attribution = merge_steps(
base, [_layer(overlay, "project:ov")]
)
assert attribution == [
ComposedStep("if-1", "project:ov"),
ComposedStep("inner-x", "project:ov"),
ComposedStep("inner-y", "project:ov"),
]
def test_merge_remove_composite_step_cleans_nested_sources(self):
"""Removing a composite step also cleans its nested children from sources."""
base = [
{
"id": "if-1",
"type": "if",
"condition": "true",
"then": [_step("then-a")],
"else": [_step("else-b")],
},
_step("a"),
]
overlay = Overlay(
id="ov", extends="wf", priority=10,
edits=[OverlayEdit("remove", "if-1")],
)
steps, attribution = merge_steps(
base, [_layer(overlay, "project:ov")]
)
assert [s["id"] for s in steps] == ["a"]
assert attribution == [ComposedStep("a", "base")]
def test_merge_insert_deeply_nested_composite_attribution(self):
"""Deep nesting (if inside while) gets the overlay source at every level."""
base = [_step("a")]
inner_if = {
"id": "inner-if",
"type": "if",
"condition": "true",
"then": [_step("deep-x")],
}
composite = {
"id": "while-1",
"type": "while",
"condition": "true",
"steps": [inner_if],
}
overlay = Overlay(
id="ov", extends="wf", priority=10,
edits=[OverlayEdit("insert_after", "a", composite)],
)
_steps, attribution = merge_steps(
base, [_layer(overlay, "project:ov")]
)
assert attribution == [
ComposedStep("a", "base"),
ComposedStep("while-1", "project:ov"),
ComposedStep("inner-if", "project:ov"),
ComposedStep("deep-x", "project:ov"),
]
class TestValidateEdits:
"""Edit validation against known base step IDs."""
def test_valid_edits(self):
edits = [
OverlayEdit("insert_after", "a", _step("new")),
OverlayEdit("remove", "b"),
]
assert validate_edits(edits, {"a", "b"}) == []
def test_invalid_anchor(self):
edits = [OverlayEdit("insert_after", "missing", _step("new"))]
errors = validate_edits(edits, {"a"})
assert any("missing" in e for e in errors)
def test_step_id_contains_colon(self):
edits = [OverlayEdit("insert_after", "a", _step("bad:id"))]
errors = validate_edits(edits, {"a"})
assert any("':'" in e for e in errors)
def test_remove_requires_no_step(self):
edits = [OverlayEdit("remove", "a", _step("extra"))]
errors = validate_edits(edits, {"a"})
assert len(errors) > 0
class TestMergeStepsAncestorConflicts:
"""merge_steps raises when two targeted anchors are in a parent/descendant relationship."""
def _if_step(self, parent_id: str, child_id: str) -> dict[str, Any]:
return {
"id": parent_id,
"type": "if",
"condition": "true",
"then": [_step(child_id)],
}
def test_remove_parent_and_insert_after_child_raises(self):
"""Removing a parent while inserting after its nested child is an anchor conflict."""
parent_id = "if-step"
child_id = "then-child"
base = [self._if_step(parent_id, child_id)]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[
OverlayEdit("remove", parent_id),
OverlayEdit("insert_after", child_id, _step("new-step")),
],
)
with pytest.raises(ValueError, match="ancestor"):
merge_steps(base, [_layer(overlay, "project:ov")])
def test_replace_parent_and_remove_child_raises(self):
"""Replacing a parent while also removing a nested child is an anchor conflict."""
parent_id = "if-step"
child_id = "then-child"
base = [self._if_step(parent_id, child_id)]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[
OverlayEdit("replace", parent_id, _step("new-parent")),
OverlayEdit("remove", child_id),
],
)
with pytest.raises(ValueError, match="ancestor"):
merge_steps(base, [_layer(overlay, "project:ov")])
def test_conflict_across_multiple_overlays_raises(self):
"""Conflict is detected even when conflicting anchors come from different overlays."""
parent_id = "if-step"
child_id = "then-child"
base = [self._if_step(parent_id, child_id)]
overlay_a = Overlay(
id="ov-a",
extends="wf",
priority=5,
edits=[OverlayEdit("remove", parent_id)],
)
overlay_b = Overlay(
id="ov-b",
extends="wf",
priority=10,
edits=[OverlayEdit("insert_after", child_id, _step("new-step"))],
)
with pytest.raises(ValueError, match="ancestor"):
merge_steps(
base,
[_layer(overlay_a, "project:ov-a"), _layer(overlay_b, "project:ov-b")],
)
def test_sibling_anchors_not_conflicting(self):
"""Anchors in sibling branches (not ancestor/descendant) are allowed."""
base = [
{
"id": "if-step",
"type": "if",
"condition": "true",
"then": [_step("then-child")],
"else": [_step("else-child")],
}
]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[
OverlayEdit("insert_after", "then-child", _step("after-then")),
OverlayEdit("insert_after", "else-child", _step("after-else")),
],
)
# Should not raise — the two anchors are siblings, not ancestor/descendant.
steps, _ = merge_steps(base, [_layer(overlay, "project:ov")])
step_ids = [s.get("id") for s in steps[0]["then"]] + [s.get("id") for s in steps[0]["else"]]
assert "after-then" in step_ids
assert "after-else" in step_ids
def test_single_anchor_not_conflicting(self):
"""A single anchor is never in conflict with itself."""
parent_id = "if-step"
child_id = "then-child"
base = [self._if_step(parent_id, child_id)]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[OverlayEdit("remove", parent_id)],
)
steps, _ = merge_steps(base, [_layer(overlay, "project:ov")])
assert steps == []
def test_child_not_targeted_no_conflict(self):
"""Targeting a parent alone (child not in any edit) is allowed."""
parent_id = "if-step"
child_id = "then-child"
base = [self._if_step(parent_id, child_id), _step("other")]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[
OverlayEdit("remove", parent_id),
OverlayEdit("insert_after", "other", _step("new-step")),
],
)
# "other" is not inside "if-step", so no ancestor conflict.
steps, _ = merge_steps(base, [_layer(overlay, "project:ov")])
assert [s["id"] for s in steps] == ["other", "new-step"]
def test_insert_only_on_ancestor_and_descendant_not_conflicting(self):
"""insert_after on both a parent and its nested child is valid and order-independent."""
parent_id = "if-step"
child_id = "then-child"
base = [self._if_step(parent_id, child_id)]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[
OverlayEdit("insert_after", parent_id, _step("after-parent")),
OverlayEdit("insert_after", child_id, _step("after-child")),
],
)
# Should not raise — inserts leave the ancestor intact.
steps, attribution = merge_steps(base, [_layer(overlay, "project:ov")])
# "after-parent" is inserted at the top level after the if-step.
assert [s["id"] for s in steps] == [parent_id, "after-parent"]
# "after-child" is inserted inside the then list.
then_ids = [s["id"] for s in steps[0]["then"]]
assert then_ids == [child_id, "after-child"]
class TestMergeStepsIdCollision:
"""merge_steps is deterministic when a replacement reuses a base step ID."""
def test_replace_with_reused_id_does_not_affect_original(self):
"""Replacing A with new_step(id=B) must not interfere with editing original B.
Before the fix, the remove-B anchor group would find the replacement step
(which now has id='b') instead of the original 'b' step, producing a
different result depending on dict iteration order.
"""
base = [_step("a"), _step("b"), _step("c")]
overlay = Overlay(
id="ov",
extends="wf",
priority=10,
edits=[
# Replace "a" with a new step that reuses id "b".
OverlayEdit("replace", "a", {**_step("b"), "command": "speckit.replaced"}),
# Remove the original "b".
OverlayEdit("remove", "b"),
],
)
steps, attribution = merge_steps(base, [_layer(overlay, "project:ov")])
# The original "b" is removed; the replacement (also id="b") survives.
# "c" is untouched.
assert len(steps) == 2
remaining_ids = [s["id"] for s in steps]
assert remaining_ids == ["b", "c"]
# The surviving "b" step is the replacement (has the custom command).
assert steps[0]["command"] == "speckit.replaced"
# Attribution for the surviving replacement "b" must not be "unknown".
# Previously, removing original "b" would pop sources["b"], erasing the
# attribution recorded for the replacement step (regression guard for the
# _remove_sources_recursively-in-remove-branch bug).
sources = {cs.step_id: cs.source for cs in attribution}
assert sources.get("b") == "project:ov", (
f"expected 'project:ov' but got {sources.get('b')!r}"
)

277
tests/workflows/test_overlay_schema.py Normal file
View File

@@ -0,0 +1,277 @@
"""Tests for overlay YAML schema normalization, especially shorthand edits."""
from __future__ import annotations
import pytest
from specify_cli.workflows.overlays.schema import (
OverlayEdit,
validate_overlay_yaml,
)
class TestShorthandEdits:
"""Requirements-compliant shorthand edit format."""
def test_shorthand_insert_after(self):
overlay, errors = validate_overlay_yaml(
{
"id": "lint",
"extends": "wf",
"priority": 10,
"edits": [
{
"insert_after": "implement",
"step": {"id": "lint", "type": "shell", "command": "npm run lint"},
}
],
}
)
assert not errors, errors
assert overlay is not None
assert overlay.edits == [
OverlayEdit("insert_after", "implement", {"id": "lint", "type": "shell", "command": "npm run lint"})
]
def test_shorthand_insert_before(self):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [
{
"insert_before": "a",
"step": {"id": "b", "type": "command", "command": "echo"},
}
],
}
)
assert not errors, errors
assert overlay is not None
assert overlay.edits == [
OverlayEdit("insert_before", "a", {"id": "b", "type": "command", "command": "echo"})
]
def test_shorthand_replace(self):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [
{
"replace": "a",
"step": {"id": "a", "type": "command", "command": "echo"},
}
],
}
)
assert not errors, errors
assert overlay is not None
assert overlay.edits == [
OverlayEdit("replace", "a", {"id": "a", "type": "command", "command": "echo"})
]
def test_shorthand_remove(self):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
}
)
assert not errors, errors
assert overlay is not None
assert overlay.edits == [OverlayEdit("remove", "a")]
def test_explicit_operation_format_still_valid(self):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "b", "type": "command", "command": "echo"},
}
],
}
)
assert not errors, errors
assert overlay is not None
assert overlay.edits == [
OverlayEdit("insert_after", "a", {"id": "b", "type": "command", "command": "echo"})
]
def test_multiple_operation_fields_rejected(self):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [
{
"insert_after": "a",
"remove": "a",
}
],
}
)
assert overlay is None
assert any("multiple" in e.lower() for e in errors), errors
def test_invalid_operation_field_rejected(self):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [{"destroy": "a"}],
}
)
assert overlay is None
assert any("operation" in e.lower() for e in errors), errors
def test_shorthand_and_explicit_mixed_list(self):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [
{"insert_after": "a", "step": {"id": "b", "type": "command", "command": "echo"}},
{
"operation": "remove",
"anchor": "c",
},
],
}
)
assert not errors, errors
assert overlay is not None
assert overlay.edits == [
OverlayEdit("insert_after", "a", {"id": "b", "type": "command", "command": "echo"}),
OverlayEdit("remove", "c"),
]
def test_shorthand_remove_must_not_include_step(self):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": "wf",
"priority": 10,
"edits": [
{
"remove": "a",
"step": {"id": "b", "type": "command", "command": "echo"},
}
],
}
)
assert overlay is None
assert any("remove" in e.lower() and "step" in e.lower() for e in errors), errors
class TestOverlayIdValidation:
"""Overlay and workflow IDs must be safe path segments."""
@pytest.mark.parametrize("overlay_id", ["../ov", "a/b", "a\\\\b", ".", "..", ""])
def test_invalid_overlay_id_rejected(self, overlay_id):
overlay, errors = validate_overlay_yaml(
{
"id": overlay_id,
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
}
)
assert overlay is None
assert any("id" in e.lower() for e in errors), errors
class TestOverlayPriorityNormalization:
"""Stored overlay priorities match preset normalization semantics."""
@pytest.mark.parametrize("priority", [None, True, "invalid", 0])
def test_invalid_or_missing_priority_defaults_to_ten(self, priority):
data = {
"id": "ov",
"extends": "wf",
"edits": [{"remove": "a"}],
}
if priority is not None:
data["priority"] = priority
overlay, errors = validate_overlay_yaml(data)
assert errors == []
assert overlay is not None
assert overlay.priority == 10
@pytest.mark.parametrize("extends", ["../wf", "a/b", "a\\\\b", ".", "..", ""])
def test_invalid_extends_rejected(self, extends):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": extends,
"priority": 10,
"edits": [{"remove": "a"}],
}
)
assert overlay is None
assert any("extends" in e.lower() for e in errors), errors
@pytest.mark.parametrize("extends", ["overlays", "runs", "steps"])
def test_reserved_workflow_id_rejected(self, extends):
overlay, errors = validate_overlay_yaml(
{
"id": "ov",
"extends": extends,
"priority": 10,
"edits": [{"remove": "a"}],
}
)
assert overlay is None
assert any("reserved" in error.lower() for error in errors), errors
def test_valid_dashed_id_accepted(self):
overlay, errors = validate_overlay_yaml(
{
"id": "my-overlay",
"extends": "my-workflow",
"priority": 10,
"edits": [{"remove": "a"}],
}
)
assert not errors, errors
assert overlay is not None
def test_validate_safe_id_rejects_trailing_newline(self):
"""A trailing newline must not pass ID validation (fullmatch guard)."""
overlay, errors = validate_overlay_yaml(
{
"id": "overlay\n",
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
}
)
assert overlay is None
assert any("id" in e.lower() for e in errors), errors
def test_validate_safe_id_rejects_embedded_newline(self):
"""An embedded newline must not pass ID validation."""
overlay, errors = validate_overlay_yaml(
{
"id": "ov\nerlay",
"extends": "wf",
"priority": 10,
"edits": [{"remove": "a"}],
}
)
assert overlay is None
assert any("id" in e.lower() for e in errors), errors

321
tests/workflows/test_overlay_security.py Normal file
View File

@@ -0,0 +1,321 @@
"""Security tests for workflow overlay path handling."""
from __future__ import annotations
from pathlib import Path
import pytest
import yaml
from typer.testing import CliRunner
from specify_cli import app
runner = CliRunner()
@pytest.fixture
def project_dir(tmp_path):
"""Create a mock spec-kit project with ``.specify/workflows/`` directory."""
workflows_dir = tmp_path / ".specify" / "workflows"
workflows_dir.mkdir(parents=True, exist_ok=True)
return tmp_path
def _write_workflow(project_root: Path, workflow_id: str, data: dict) -> Path:
wf_dir = project_root / ".specify" / "workflows" / workflow_id
wf_dir.mkdir(parents=True, exist_ok=True)
wf_path = wf_dir / "workflow.yml"
wf_path.write_text(yaml.safe_dump(data), encoding="utf-8")
return wf_path
def _write_overlay(project_root: Path, workflow_id: str, overlay_id: str, data: dict) -> Path:
ov_dir = project_root / ".specify" / "workflows" / "overlays" / workflow_id
ov_dir.mkdir(parents=True, exist_ok=True)
ov_path = ov_dir / f"{overlay_id}.yml"
ov_path.write_text(yaml.safe_dump(data), encoding="utf-8")
return ov_path
class TestOverlayPathTraversal:
"""Overlay CLI must stay inside the overlay directory."""
def test_overlay_add_rejects_traversal_in_workflow_id(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "../wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
}
),
encoding="utf-8",
)
result = runner.invoke(
app, ["workflow", "overlay", "add", str(overlay_file), "--priority", "5"]
)
assert result.exit_code != 0, result.output
assert "invalid" in result.output.lower() or "traversal" in result.output.lower()
def test_overlay_add_rejects_traversal_in_overlay_id(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "../../ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
}
),
encoding="utf-8",
)
result = runner.invoke(
app, ["workflow", "overlay", "add", str(overlay_file), "--priority", "5"]
)
assert result.exit_code != 0, result.output
assert "invalid" in result.output.lower() or "traversal" in result.output.lower()
def test_overlay_remove_cannot_escape_overlays_dir(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
# Create a base workflow file that would be the traversal target.
target = project_dir / ".specify" / "workflows" / "wf" / "workflow.yml"
assert target.is_file()
result = runner.invoke(
app, ["workflow", "overlay", "remove", "wf", "../wf/workflow"]
)
assert result.exit_code != 0, result.output
assert target.is_file()
assert "Invalid" in result.output or "traversal" in result.output.lower()
def test_overlay_remove_rejects_symlink(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
overlay_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
real_file = overlay_dir / "ov1.yml"
symlink_file = overlay_dir / "symlink.yml"
symlink_file.symlink_to(real_file)
result = runner.invoke(app, ["workflow", "overlay", "remove", "wf", "symlink"])
assert result.exit_code != 0, result.output
assert real_file.is_file()
assert "symlink" in result.output.lower() or "Invalid" in result.output
def test_overlay_add_rejects_symlinked_target_file(self, project_dir, monkeypatch):
"""overlay add must not overwrite through a symlinked overlay file target."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
overlay_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
overlay_dir.mkdir(parents=True, exist_ok=True)
real_file = overlay_dir / "other.yml"
real_file.write_text("sentinel\n", encoding="utf-8")
(overlay_dir / "ov1.yml").symlink_to(real_file)
overlay_file = project_dir / "overlay.yml"
overlay_file.write_text(
yaml.safe_dump(
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
}
),
encoding="utf-8",
)
result = runner.invoke(app, ["workflow", "overlay", "add", str(overlay_file)])
assert result.exit_code != 0, result.output
assert "symlinked path" in result.output.lower()
assert real_file.read_text(encoding="utf-8") == "sentinel\n"
@pytest.mark.parametrize("workflow_id", ["overlays", "runs", "steps"])
def test_overlay_operations_reject_reserved_workflow_id(
self, project_dir, monkeypatch, workflow_id
):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
result = runner.invoke(app, ["workflow", "overlay", "list", workflow_id])
assert result.exit_code != 0, result.output
assert "Invalid" in result.output or "reserved" in result.output.lower()
def test_overlay_set_priority_rejects_traversal(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
result = runner.invoke(
app, ["workflow", "overlay", "set-priority", "wf", "../other", "10"]
)
assert result.exit_code != 0, result.output
assert "invalid" in result.output.lower() or "traversal" in result.output.lower()
def test_overlay_enable_rejects_traversal(self, project_dir, monkeypatch):
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
result = runner.invoke(app, ["workflow", "overlay", "enable", "wf", "../other"])
assert result.exit_code != 0, result.output
assert "invalid" in result.output.lower() or "traversal" in result.output.lower()
def test_overlay_rejects_symlinked_overlays_dir(self, project_dir, monkeypatch, tmp_path):
"""Overlay commands must reject a symlinked .specify/workflows/overlays directory."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
# Create a symlinked overlays directory pointing outside the project
outside_dir = tmp_path / "outside"
outside_dir.mkdir()
overlays_dir = project_dir / ".specify" / "workflows" / "overlays"
overlays_dir.symlink_to(outside_dir)
result = runner.invoke(app, ["workflow", "overlay", "list", "wf"])
assert result.exit_code != 0, result.output
assert "symlink" in result.output.lower()
def test_overlay_list_rejects_symlinked_per_workflow_dir(self, project_dir, monkeypatch, tmp_path):
"""Overlay list must reject a symlinked per-workflow overlay directory."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
# Create a real overlay directory outside the project.
outside_dir = tmp_path / "outside_wf"
outside_dir.mkdir()
outside_dir.joinpath("evil.yml").write_text(
yaml.safe_dump(
{
"id": "evil",
"extends": "wf",
"priority": 100,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "evil-step", "type": "command", "command": "echo"},
}
],
}
),
encoding="utf-8",
)
# Symlink the per-workflow overlay directory to the outside location.
overlays_root = project_dir / ".specify" / "workflows" / "overlays"
overlays_root.mkdir(parents=True, exist_ok=True)
symlink_dir = overlays_root / "wf"
symlink_dir.symlink_to(outside_dir)
result = runner.invoke(app, ["workflow", "overlay", "list", "wf"])
assert result.exit_code != 0, result.output
assert "symlink" in result.output.lower()
def test_overlay_list_reports_invalid_yaml_cleanly(self, project_dir, monkeypatch):
"""Overlay list should surface malformed overlay YAML as a clean user error."""
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
_write_workflow(
project_dir,
"wf",
{
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "echo"}],
},
)
overlay_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
overlay_dir.mkdir(parents=True, exist_ok=True)
(overlay_dir / "broken.yml").write_text("id: broken\nextends: wf\npriority: [\n", encoding="utf-8")
result = runner.invoke(app, ["workflow", "overlay", "list", "wf"])
assert result.exit_code != 0, result.output
assert "Invalid YAML" in result.output

567
tests/workflows/test_resolver_integration.py Normal file
View File

@@ -0,0 +1,567 @@
"""Integration tests for the WorkflowResolver."""
from __future__ import annotations
from pathlib import Path
import pytest
import yaml
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow
from specify_cli.workflows.overlays import WorkflowResolver
from specify_cli.workflows.overlays.merge import ComposedStep
def _write_workflow(project_root: Path, workflow_id: str, data: dict) -> Path:
wf_dir = project_root / ".specify" / "workflows" / workflow_id
wf_dir.mkdir(parents=True, exist_ok=True)
wf_path = wf_dir / "workflow.yml"
wf_path.write_text(yaml.safe_dump(data), encoding="utf-8")
return wf_path
def _write_overlay(project_root: Path, workflow_id: str, overlay_id: str, data: dict) -> Path:
ov_dir = project_root / ".specify" / "workflows" / "overlays" / workflow_id
ov_dir.mkdir(parents=True, exist_ok=True)
ov_path = ov_dir / f"{overlay_id}.yml"
ov_path.write_text(yaml.safe_dump(data), encoding="utf-8")
return ov_path
class TestWorkflowResolver:
"""End-to-end resolution of base workflows plus overlays."""
@pytest.mark.parametrize(
"workflow_id",
[
"../outside",
"nested/workflow",
"wf\n",
"overlays",
"runs",
"steps",
],
)
def test_rejects_unsafe_id_before_collecting_sources(
self, project_dir, workflow_id
):
resolver = WorkflowResolver(project_dir)
class UnexpectedSource:
def collect(self, _workflow_id):
pytest.fail("source collection must not run for an unsafe workflow ID")
resolver._sources = [UnexpectedSource()]
with pytest.raises(ValueError, match="Invalid workflow ID"):
resolver.resolve(workflow_id)
def test_rejects_absolute_id_before_collecting_sources(
self, project_dir, tmp_path
):
resolver = WorkflowResolver(project_dir)
outside = tmp_path / "outside"
class UnexpectedSource:
def collect(self, _workflow_id):
pytest.fail("source collection must not run for an absolute workflow ID")
resolver._sources = [UnexpectedSource()]
with pytest.raises(ValueError, match="Invalid workflow ID"):
resolver.resolve(str(outside))
def test_resolve_without_overlays(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
resolver = WorkflowResolver(project_dir)
definition = resolver.resolve("wf")
assert isinstance(definition, WorkflowDefinition)
assert definition.id == "wf"
assert [s["id"] for s in definition.steps] == ["a"]
def test_resolve_with_project_overlay_insert(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [
{"id": "a", "type": "command", "command": "speckit.specify"},
{"id": "b", "type": "command", "command": "speckit.specify"},
],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "speckit.plan"},
}
],
},
)
resolver = WorkflowResolver(project_dir)
definition = resolver.resolve("wf")
assert [s["id"] for s in definition.steps] == ["a", "new", "b"]
def test_resolve_lower_priority_wins(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"low",
{
"id": "low",
"extends": "wf",
"priority": 5,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "low-step", "type": "command", "command": "echo"},
}
],
},
)
_write_overlay(
project_dir,
"wf",
"high",
{
"id": "high",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "high-step", "type": "command", "command": "echo"},
}
],
},
)
resolver = WorkflowResolver(project_dir)
definition = resolver.resolve("wf")
# Lower priority is applied later; both insert_after 'a', so low-step
# ends up closer to the anchor and wins the conflict.
assert [s["id"] for s in definition.steps] == ["a", "low-step", "high-step"]
def test_resolve_with_layers_returns_attribution(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
resolver = WorkflowResolver(project_dir)
definition, layers, attribution = resolver.resolve_with_layers("wf")
assert [s["id"] for s in definition.steps] == ["a", "new"]
assert any(layer.tier == "base" for layer in layers)
assert attribution == [ComposedStep("a", "base"), ComposedStep("new", "project:ov1")]
def test_resolve_attribution_for_nested_base_steps(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [
{
"id": "if-1",
"type": "if",
"condition": "true",
"then": [{"id": "then-a", "type": "command", "command": "echo"}],
"else": [{"id": "else-b", "type": "command", "command": "echo"}],
}
],
}
_write_workflow(project_dir, "wf", data)
resolver = WorkflowResolver(project_dir)
definition, _layers, attribution = resolver.resolve_with_layers("wf")
assert [s["id"] for s in definition.steps] == ["if-1"]
sources = {c.step_id: c.source for c in attribution}
assert sources["if-1"] == "base"
assert sources["then-a"] == "base"
assert sources["else-b"] == "base"
def test_resolve_invalid_project_overlay_fails(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"broken",
{
"id": "broken",
"extends": "wf",
"priority": 10,
"edits": "not-a-list",
},
)
resolver = WorkflowResolver(project_dir)
with pytest.raises(ValueError):
resolver.resolve("wf")
def test_resolve_disabled_overlay_is_skipped(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"disabled",
{
"id": "disabled",
"extends": "wf",
"priority": 10,
"enabled": False,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
resolver = WorkflowResolver(project_dir)
definition = resolver.resolve("wf")
assert [s["id"] for s in definition.steps] == ["a"]
def test_collect_all_layers_can_include_disabled_overlay_for_listing(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"disabled",
{
"id": "disabled",
"extends": "wf",
"priority": 10,
"enabled": False,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
resolver = WorkflowResolver(project_dir)
default_layers = resolver.collect_all_layers("wf")
listed_layers = resolver.collect_all_layers("wf", include_disabled=True)
assert [layer.source for layer in default_layers] == ["base"]
assert [layer.source for layer in listed_layers] == ["project:disabled", "base"]
def test_resolve_invalid_anchor_raises(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "missing",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
resolver = WorkflowResolver(project_dir)
with pytest.raises(ValueError, match="anchor 'missing' does not match any base step id"):
resolver.resolve("wf")
def test_resolve_missing_workflow(self, project_dir):
resolver = WorkflowResolver(project_dir)
with pytest.raises(FileNotFoundError, match="Workflow not found"):
resolver.resolve("missing")
def test_resolve_returns_composed_result_for_caller_validation(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "replace",
"anchor": "a",
"step": {"id": "a", "type": "invalid-type", "command": "echo"},
}
],
},
)
resolver = WorkflowResolver(project_dir)
definition = resolver.resolve("wf")
errors = validate_workflow(definition)
assert any("invalid-type" in err for err in errors)
def test_resolve_rejects_symlinked_project_overlay_dir(self, project_dir, tmp_path):
"""ProjectOverlaySource must reject a symlinked per-workflow overlay directory."""
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
# Create a real overlay directory outside the project with a malicious overlay.
outside_dir = tmp_path / "outside_overlays" / "wf"
outside_dir.mkdir(parents=True, exist_ok=True)
outside_dir.joinpath("evil.yml").write_text(
yaml.safe_dump(
{
"id": "evil",
"extends": "wf",
"priority": 100,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "evil-step", "type": "command", "command": "rm -rf /"},
}
],
}
),
encoding="utf-8",
)
# Symlink the per-workflow overlay directory to the outside location.
overlays_root = project_dir / ".specify" / "workflows" / "overlays"
overlays_root.mkdir(parents=True, exist_ok=True)
symlink_dir = overlays_root / "wf"
symlink_dir.symlink_to(outside_dir)
resolver = WorkflowResolver(project_dir)
with pytest.raises(ValueError, match="Symlinked overlay directories are not allowed"):
resolver.resolve("wf")
def test_resolve_rejects_symlinked_overlay_root(self, project_dir, tmp_path):
"""ProjectOverlaySource must reject a symlinked overlay root too."""
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
outside_root = tmp_path / "outside-overlays-root"
outside_root.mkdir(parents=True, exist_ok=True)
workflow_dir = outside_root / "wf"
workflow_dir.mkdir()
workflow_dir.joinpath("evil.yml").write_text(
yaml.safe_dump(
{
"id": "evil",
"extends": "wf",
"priority": 100,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "evil-step", "type": "command", "command": "rm -rf /"},
}
],
}
),
encoding="utf-8",
)
overlays_root = project_dir / ".specify" / "workflows" / "overlays"
overlays_root.symlink_to(outside_root, target_is_directory=True)
resolver = WorkflowResolver(project_dir)
with pytest.raises(ValueError, match="Symlinked overlay directories are not allowed"):
resolver.resolve("wf")
def test_resolve_reports_invalid_overlay_yaml_cleanly(self, project_dir):
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
overlay_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
overlay_dir.mkdir(parents=True, exist_ok=True)
(overlay_dir / "broken.yml").write_text("id: broken\nextends: wf\npriority: [\n", encoding="utf-8")
resolver = WorkflowResolver(project_dir)
with pytest.raises(ValueError, match="Invalid YAML"):
resolver.resolve("wf")
def test_resolve_attribution_for_inserted_composite_step(self, project_dir):
"""Inserted composite steps must attribute nested children to the overlay source."""
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {
"id": "if-1",
"type": "if",
"condition": "true",
"then": [{"id": "then-x", "type": "command", "command": "echo"}],
"else": [{"id": "else-y", "type": "command", "command": "echo"}],
},
}
],
},
)
resolver = WorkflowResolver(project_dir)
_definition, _layers, attribution = resolver.resolve_with_layers("wf")
sources = {c.step_id: c.source for c in attribution}
assert sources["a"] == "base"
assert sources["if-1"] == "project:ov1"
assert sources["then-x"] == "project:ov1"
assert sources["else-y"] == "project:ov1"
def test_engine_load_workflow_uses_resolver(self, project_dir):
from specify_cli.workflows.engine import WorkflowEngine
data = {
"schema_version": "1.0",
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
"steps": [{"id": "a", "type": "command", "command": "speckit.specify"}],
}
_write_workflow(project_dir, "wf", data)
_write_overlay(
project_dir,
"wf",
"ov1",
{
"id": "ov1",
"extends": "wf",
"priority": 10,
"edits": [
{
"operation": "insert_after",
"anchor": "a",
"step": {"id": "new", "type": "command", "command": "echo"},
}
],
},
)
engine = WorkflowEngine(project_dir)
definition = engine.load_workflow("wf")
assert [s["id"] for s in definition.steps] == ["a", "new"]
def test_engine_rejects_traversal_without_legacy_path_fallback(
self, project_dir
):
from specify_cli.workflows.engine import WorkflowEngine
outside = project_dir / ".specify" / "outside"
outside.mkdir(parents=True)
(outside / "workflow.yml").write_text(
yaml.safe_dump(
{
"schema_version": "1.0",
"workflow": {
"id": "outside",
"name": "Outside",
"version": "1.0.0",
},
"steps": [
{
"id": "external",
"type": "command",
"command": "echo",
}
],
}
),
encoding="utf-8",
)
engine = WorkflowEngine(project_dir)
with pytest.raises(ValueError, match="Invalid workflow ID"):
engine.load_workflow("../outside")