mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
e802a7dd52 |
30
CHANGELOG.md
30
CHANGELOG.md
@@ -2,36 +2,6 @@
|
||||
|
||||
<!-- insert new changelog below this comment -->
|
||||
|
||||
## [0.12.13] - 2026-07-13
|
||||
|
||||
### Changed
|
||||
|
||||
- fix(workflows): fail switch step on non-mapping cases instead of crashing (#3481)
|
||||
- Cleanup agent-file-template.md (#2579)
|
||||
- fix: mark Kiro integration as multi-install safe (#3472)
|
||||
- fix: rewrite extension-relative subdir paths in generated command bodies (#3444)
|
||||
- fix(templates): point constitution sync checklist at installed command files (#3418)
|
||||
- feat(workflows): make shell step timeout configurable (#3327) (#3328)
|
||||
- docs: clarify that release tags keep the leading v prefix (#3463)
|
||||
- fix(workflows): don't crash on membership test against a non-iterable (#3448)
|
||||
- fix(workflows): if-step validate accepts falsy non-list else (#3264)
|
||||
- chore: release 0.12.12, begin 0.12.13.dev0 development (#3490)
|
||||
|
||||
## [0.12.12] - 2026-07-13
|
||||
|
||||
### Changed
|
||||
|
||||
- fix(extensions): set-priority repairs corrupted boolean priority (#3268)
|
||||
- fix(presets): set-priority repairs corrupted boolean priority (#3269)
|
||||
- fix(workflows): engine loop cap ignores bool max_iterations (#3270)
|
||||
- docs(bundles): document --integration on 'bundle update' (#3271)
|
||||
- fix(workflows): harden catalog.py against mis-shaped registry & non-string fields (#3375)
|
||||
- Add Verify Review Ship extension to community catalog (#3450)
|
||||
- fix(bundle): reject file:// / local download_url — catalog URLs are HTTPS-only (#3344)
|
||||
- fix(extensions): handle prefix-colliding env vars in _get_env_config (#3350)
|
||||
- docs: document copilot skills mode (--skills) and markdown deprecation (#3313)
|
||||
- chore: release 0.12.11, begin 0.12.12.dev0 development (#3460)
|
||||
|
||||
## [0.12.11] - 2026-07-10
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -45,7 +45,7 @@ Spec-Driven Development **flips the script** on traditional software development
|
||||
|
||||
### 1. Install Specify CLI
|
||||
|
||||
Requires **[uv](https://docs.astral.sh/uv/)** ([install uv](./docs/install/uv.md)). Replace `vX.Y.Z` with the latest release tag from [Releases](https://github.com/github/spec-kit/releases) — keep the leading `v` (for example, `v0.12.11`, not `0.12.11`):
|
||||
Requires **[uv](https://docs.astral.sh/uv/)** ([install uv](./docs/install/uv.md)). Replace `vX.Y.Z` with the latest tag from [Releases](https://github.com/github/spec-kit/releases):
|
||||
|
||||
```bash
|
||||
uv tool install specify-cli --from git+https://github.com/github/spec-kit.git@vX.Y.Z
|
||||
|
||||
@@ -148,7 +148,6 @@ The following community-contributed extensions are available in [`catalog.commun
|
||||
| Token Economy | Token routing, measured savings, and context audit workflows | `process` | Read+Write | [spec-kit-token-economy](https://github.com/formin/spec-kit-token-economy) |
|
||||
| V-Model Extension Pack | Enforces V-Model paired generation of development specs and test specs with full traceability | `docs` | Read+Write | [spec-kit-v-model](https://github.com/leocamello/spec-kit-v-model) |
|
||||
| Verify Extension | Post-implementation quality gate that validates implemented code against specification artifacts | `code` | Read-only | [spec-kit-verify](https://github.com/ismaelJimenez/spec-kit-verify) |
|
||||
| Verify Review Ship | Adds post-implementation verify, review, and ship readiness gates to Spec Kit workflows | `process` | Read-only | [spec-kit-verify-review-ship](https://github.com/cadugevaerd/spec-kit-verify-review-ship) |
|
||||
| Verify Tasks Extension | Detect phantom completions: tasks marked [X] in tasks.md with no real implementation | `code` | Read-only | [spec-kit-verify-tasks](https://github.com/datastone-inc/spec-kit-verify-tasks) |
|
||||
| Version Guard | Verify tech stack versions against live npm registries before planning and implementation | `process` | Read-only | [spec-kit-version-guard](https://github.com/KevinBrown5280/spec-kit-version-guard) |
|
||||
| What-if Analysis | Preview the downstream impact (complexity, effort, tasks, risks) of requirement changes before committing to them | `visibility` | Read-only | [spec-kit-whatif](https://github.com/DevAbdullah90/spec-kit-whatif) |
|
||||
|
||||
@@ -11,8 +11,7 @@ If you want to try Spec Kit without installing it permanently, use `uvx` to run
|
||||
# Create a new project (latest from main)
|
||||
uvx --from git+https://github.com/github/spec-kit.git specify init <PROJECT_NAME>
|
||||
|
||||
# Or target a specific release (replace vX.Y.Z with a tag from Releases;
|
||||
# keep the leading v, e.g. v0.12.11 not 0.12.11)
|
||||
# Or target a specific release (replace vX.Y.Z with a tag from Releases)
|
||||
uvx --from git+https://github.com/github/spec-kit.git@vX.Y.Z specify init <PROJECT_NAME>
|
||||
|
||||
# Initialize in the current directory
|
||||
|
||||
@@ -7,8 +7,7 @@
|
||||
Pin a specific release tag for stability (check [Releases](https://github.com/github/spec-kit/releases) for the latest):
|
||||
|
||||
```bash
|
||||
# Install a specific stable release (recommended — replace vX.Y.Z with the
|
||||
# latest tag, keeping the leading v, e.g. v0.12.11 not 0.12.11)
|
||||
# Install a specific stable release (recommended — replace vX.Y.Z with the latest tag)
|
||||
pipx install git+https://github.com/github/spec-kit.git@vX.Y.Z
|
||||
|
||||
# Or install latest from main (may include unreleased changes)
|
||||
|
||||
@@ -15,7 +15,7 @@
|
||||
|
||||
### Persistent Installation (Recommended)
|
||||
|
||||
Install once and use everywhere. Replace `vX.Y.Z` with a release tag from [Releases](https://github.com/github/spec-kit/releases) — keep the leading `v` (for example, `v0.12.11`, not `0.12.11`):
|
||||
Install once and use everywhere. Replace `vX.Y.Z` with a tag from [Releases](https://github.com/github/spec-kit/releases):
|
||||
|
||||
> [!NOTE]
|
||||
> The command below requires **[uv](https://docs.astral.sh/uv/)**. If you see `command not found: uv`, [install uv first](./install/uv.md).
|
||||
|
||||
@@ -51,11 +51,10 @@ If the current directory is not yet a Spec Kit project, `install` initializes on
|
||||
specify bundle update [<bundle_id>]
|
||||
```
|
||||
|
||||
| Option | Description |
|
||||
| ---------------- | --------------------------------------------------------------------------------------------------------------------- |
|
||||
| `--all` | Update every installed bundle |
|
||||
| `--integration` | Override the integration used when refreshing components; applied only when the project's active integration can't be determined |
|
||||
| `--offline` | Do not access the network |
|
||||
| Option | Description |
|
||||
| ------------ | ------------------------------------ |
|
||||
| `--all` | Update every installed bundle |
|
||||
| `--offline` | Do not access the network |
|
||||
|
||||
Re-resolves a bundle and **refreshes** its components through each primitive's update path, bringing already-installed components up to the bundle's newly pinned versions while preserving primitive-level overrides (such as preset priority). Provide a bundle id, or use `--all` to update everything installed.
|
||||
|
||||
|
||||
@@ -18,7 +18,7 @@ The Specify CLI supports a wide range of AI coding agents. When you run `specify
|
||||
| [Firebender](https://firebender.com/) | `firebender` | IDE-based agent for Android Studio / IntelliJ |
|
||||
| [Forge](https://forgecode.dev/) | `forge` | |
|
||||
| [Gemini CLI](https://github.com/google-gemini/gemini-cli) | `gemini` | |
|
||||
| [GitHub Copilot](https://code.visualstudio.com/) | `copilot` | Defaults to legacy markdown mode: `.agent.md` command files under `.github/agents/`, companion `.prompt.md` files under `.github/prompts/`, and a `.vscode/settings.json` merge. Pass `--integration-options="--skills"` to scaffold skills as `speckit-<command>/SKILL.md` under `.github/skills/` instead. Legacy markdown mode is deprecated and will stop being the default in a future release. |
|
||||
| [GitHub Copilot](https://code.visualstudio.com/) | `copilot` | |
|
||||
| [Goose](https://goose-docs.ai/) | `goose` | Uses YAML recipe format in `.goose/recipes/` |
|
||||
| [Hermes](https://github.com/NousResearch/hermes-agent) | `hermes` | Skills-based integration; installs skills globally into `~/.hermes/skills/` |
|
||||
| [IBM Bob](https://www.ibm.com/products/bob) | `bob` | IDE-based agent |
|
||||
@@ -219,7 +219,6 @@ Some integrations accept additional options via `--integration-options`:
|
||||
| ----------- | ------------------- | -------------------------------------------------------------- |
|
||||
| `generic` | `--commands-dir` | Required. Directory for command files |
|
||||
| `kimi` | `--migrate-legacy` | Migrate legacy `.kimi/skills/` installs to `.kimi-code/skills/` (including dotted→hyphenated skill naming, e.g. `speckit.xxx` → `speckit-xxx`) |
|
||||
| `copilot` | `--skills` | Scaffold commands as agent skills (`speckit-<command>/SKILL.md` under `.github/skills/`, invoked as `/speckit-<command>`) instead of the default legacy markdown mode (`.github/agents/*.agent.md` plus `.github/prompts/*.prompt.md` and a `.vscode/settings.json` merge). Without this flag, install warns that legacy markdown mode is deprecated. |
|
||||
|
||||
Example:
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"schema_version": "1.0",
|
||||
"updated_at": "2026-07-10T00:00:00Z",
|
||||
"updated_at": "2026-07-08T00:00:00Z",
|
||||
"catalog_url": "https://raw.githubusercontent.com/github/spec-kit/main/extensions/catalog.community.json",
|
||||
"extensions": {
|
||||
"aide": {
|
||||
@@ -4356,40 +4356,6 @@
|
||||
"created_at": "2026-03-03T00:00:00Z",
|
||||
"updated_at": "2026-04-09T00:00:00Z"
|
||||
},
|
||||
"verify-review-ship": {
|
||||
"name": "Verify Review Ship",
|
||||
"id": "verify-review-ship",
|
||||
"description": "Adds post-implementation verify, review, and ship readiness gates to Spec Kit workflows.",
|
||||
"author": "Carlos Eduardo Gevaerd Araujo",
|
||||
"version": "0.1.0",
|
||||
"download_url": "https://github.com/cadugevaerd/spec-kit-verify-review-ship/archive/refs/tags/v0.1.0.zip",
|
||||
"repository": "https://github.com/cadugevaerd/spec-kit-verify-review-ship",
|
||||
"homepage": "https://github.com/cadugevaerd/spec-kit-verify-review-ship",
|
||||
"documentation": "https://github.com/cadugevaerd/spec-kit-verify-review-ship/blob/main/README.md",
|
||||
"changelog": "https://github.com/cadugevaerd/spec-kit-verify-review-ship/blob/main/CHANGELOG.md",
|
||||
"license": "MIT",
|
||||
"category": "process",
|
||||
"effect": "read-only",
|
||||
"requires": {
|
||||
"speckit_version": ">=0.1.0"
|
||||
},
|
||||
"provides": {
|
||||
"commands": 3,
|
||||
"hooks": 1
|
||||
},
|
||||
"tags": [
|
||||
"quality",
|
||||
"review",
|
||||
"shipping",
|
||||
"workflow",
|
||||
"testing"
|
||||
],
|
||||
"verified": false,
|
||||
"downloads": 0,
|
||||
"stars": 0,
|
||||
"created_at": "2026-07-10T00:00:00Z",
|
||||
"updated_at": "2026-07-10T00:00:00Z"
|
||||
},
|
||||
"verify-tasks": {
|
||||
"name": "Verify Tasks Extension",
|
||||
"id": "verify-tasks",
|
||||
|
||||
@@ -158,7 +158,8 @@ presets/
|
||||
├── plan-template.md
|
||||
├── tasks-template.md
|
||||
├── checklist-template.md
|
||||
└── constitution-template.md
|
||||
├── constitution-template.md
|
||||
└── agent-file-template.md
|
||||
```
|
||||
|
||||
## Module Structure
|
||||
|
||||
@@ -44,6 +44,12 @@ provides:
|
||||
description: "Self-test constitution template"
|
||||
replaces: "constitution-template"
|
||||
|
||||
- type: "template"
|
||||
name: "agent-file-template"
|
||||
file: "templates/agent-file-template.md"
|
||||
description: "Self-test agent file template"
|
||||
replaces: "agent-file-template"
|
||||
|
||||
- type: "command"
|
||||
name: "speckit.specify"
|
||||
file: "commands/speckit.specify.md"
|
||||
|
||||
9
presets/self-test/templates/agent-file-template.md
Normal file
9
presets/self-test/templates/agent-file-template.md
Normal file
@@ -0,0 +1,9 @@
|
||||
# Agent File (Self-Test Preset)
|
||||
|
||||
<!-- preset:self-test -->
|
||||
|
||||
> This template is provided by the self-test preset.
|
||||
|
||||
## Agent Instructions
|
||||
|
||||
Follow these guidelines when working on this project.
|
||||
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "specify-cli"
|
||||
version = "0.12.13"
|
||||
version = "0.12.11"
|
||||
description = "Specify CLI, part of GitHub Spec Kit. A tool to bootstrap your projects for Spec-Driven Development (SDD)."
|
||||
readme = "README.md"
|
||||
requires-python = ">=3.11"
|
||||
|
||||
@@ -213,52 +213,6 @@ class CommandRegistrar:
|
||||
".specify.specify/", ".specify/"
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def rewrite_extension_paths(
|
||||
text: str, extension_id: str, extension_dir: Path
|
||||
) -> str:
|
||||
"""Rewrite extension-relative paths to their installed locations.
|
||||
|
||||
Extension command bodies reference bundled files relative to the
|
||||
extension root (e.g. ``agents/control/commander.md``). After install
|
||||
those files live under ``.specify/extensions/<id>/``, so bare
|
||||
references would resolve against the workspace root and never be
|
||||
found (#2101).
|
||||
|
||||
Only directories that actually exist inside *extension_dir* are
|
||||
rewritten, keeping the behaviour conservative and avoiding false
|
||||
positives on prose. ``commands`` (slash-command sources), ``specs``
|
||||
(user project artifacts) and dot-directories are never rewritten.
|
||||
"""
|
||||
if not isinstance(text, str) or not text:
|
||||
return text
|
||||
|
||||
skip = {"commands", ".git", "specs"}
|
||||
try:
|
||||
subdirs = [
|
||||
entry.name
|
||||
for entry in extension_dir.iterdir()
|
||||
if entry.is_dir()
|
||||
and entry.name not in skip
|
||||
and not entry.name.startswith(".")
|
||||
]
|
||||
except OSError:
|
||||
return text
|
||||
|
||||
for subdir in subdirs:
|
||||
# Only rewrite relative references (subdir/... or ./subdir/...);
|
||||
# absolute paths like /subdir/... keep their meaning. Use a
|
||||
# callable replacement: subdir/extension_id come from the
|
||||
# filesystem and could contain backslashes or "\1"-like
|
||||
# sequences, which would corrupt a string replacement template.
|
||||
replacement = f".specify/extensions/{extension_id}/{subdir}/"
|
||||
text = re.sub(
|
||||
r'(^|[\s`"\'(])(?:\./)?' + re.escape(subdir) + "/",
|
||||
lambda m: m.group(1) + replacement,
|
||||
text,
|
||||
)
|
||||
return text
|
||||
|
||||
def render_markdown_command(
|
||||
self, frontmatter: dict, body: str, source_id: str, context_note: str = None
|
||||
) -> str:
|
||||
@@ -685,9 +639,6 @@ class CommandRegistrar:
|
||||
frontmatter[key] = core_frontmatter[key]
|
||||
frontmatter.pop("strategy", None)
|
||||
|
||||
if extension_id:
|
||||
body = self.rewrite_extension_paths(body, extension_id, source_root)
|
||||
|
||||
frontmatter = self._adjust_script_paths(
|
||||
frontmatter, extension_id=extension_id
|
||||
)
|
||||
|
||||
@@ -746,16 +746,11 @@ def _resolve_manifest_path(path: Path | None) -> Path:
|
||||
def _download_manifest(resolved, *, offline: bool):
|
||||
"""Resolve a bundle's manifest from its catalog ``download_url``.
|
||||
|
||||
Catalog ``download_url``s are HTTPS-only (``http`` allowed for localhost),
|
||||
matching the extensions/presets/workflows catalog systems. Remote URLs are
|
||||
fetched with the shared authenticated, redirect-validated HTTP client, and
|
||||
only when not ``--offline``.
|
||||
|
||||
Local and ``file://`` sources are intentionally not resolved here: to
|
||||
install a bundle from disk, pass the path positionally
|
||||
(``specify bundle install ./path/to/bundle.yml`` — a bundle directory or a
|
||||
``.zip`` artifact also works), which :func:`_local_manifest_source` handles
|
||||
before catalog resolution and which never touches ``download_url``.
|
||||
Local/``file://`` URLs always work offline and may point at a ``.zip``
|
||||
artifact, a bundle directory, or a ``bundle.yml`` (handled by
|
||||
:func:`_local_manifest_source`). Remote ``https://`` URLs are fetched with
|
||||
the shared authenticated, redirect-validated HTTP client, and only when not
|
||||
``--offline``.
|
||||
"""
|
||||
from urllib.parse import urlparse
|
||||
|
||||
@@ -768,35 +763,26 @@ def _download_manifest(resolved, *, offline: bool):
|
||||
parsed = urlparse(url)
|
||||
scheme = parsed.scheme.lower()
|
||||
|
||||
# ``file://`` URLs and bare filesystem paths (including Windows drive paths
|
||||
# like ``C:\bundle.yml``, which urlparse reads as a single-letter scheme)
|
||||
# are not valid catalog download URLs. Catalog URLs are HTTPS-only across
|
||||
# every catalog system; installing from disk is done by passing the path
|
||||
# positionally, which never reaches URL resolution. Give an actionable
|
||||
# error rather than accepting a scheme the rest of the codebase rejects.
|
||||
# On Windows an absolute path like ``C:\bundle.yml`` parses with a
|
||||
# single-letter ``scheme``; treat it as a local file, not a URL scheme.
|
||||
if scheme in ("", "file") or re.match(r"^[A-Za-z]:[\\/]", url):
|
||||
raise BundlerError(
|
||||
f"Catalog entry '{resolved.entry.id}' has a non-HTTP(S) download_url "
|
||||
f"({url}); catalog download URLs must be HTTPS (http for localhost) — "
|
||||
"a file:// URL, a local filesystem path, or a scheme-less value "
|
||||
"(e.g. 'example.com/bundle.zip') is not accepted. "
|
||||
"To install a bundle from disk, pass the path directly: "
|
||||
"'specify bundle install <path-to-bundle.yml | bundle-dir | .zip>'."
|
||||
)
|
||||
local = Path(parsed.path if scheme == "file" else url)
|
||||
manifest = _local_manifest_source(str(local))
|
||||
if manifest is None:
|
||||
raise BundlerError(f"Bundle manifest not found: {local}")
|
||||
return manifest
|
||||
|
||||
# Validate the scheme/host *before* the offline gate so an invalid or
|
||||
# non-HTTPS download_url reports the real problem in every mode, rather
|
||||
# than a misleading "Network access disabled" under --offline.
|
||||
# (_download_remote_manifest re-checks this, but only once network access
|
||||
# is permitted.) HTTPS-only, http allowed for localhost.
|
||||
_require_https(f"bundle '{resolved.entry.id}'", url)
|
||||
if scheme in ("http", "https"):
|
||||
if offline:
|
||||
raise BundlerError(
|
||||
f"Network access disabled; cannot download bundle '{resolved.entry.id}' "
|
||||
f"from {url}."
|
||||
)
|
||||
return _download_remote_manifest(resolved.entry.id, url)
|
||||
|
||||
if offline:
|
||||
raise BundlerError(
|
||||
f"Network access disabled; cannot download bundle '{resolved.entry.id}' "
|
||||
f"from {url}."
|
||||
)
|
||||
return _download_remote_manifest(resolved.entry.id, url)
|
||||
raise BundlerError(
|
||||
f"Unsupported download_url scheme for bundle '{resolved.entry.id}': {url}"
|
||||
)
|
||||
|
||||
|
||||
def _require_https(label: str, url: str) -> None:
|
||||
|
||||
@@ -1078,11 +1078,6 @@ class ExtensionManager:
|
||||
frontmatter = registrar._adjust_script_paths(
|
||||
frontmatter, extension_id=manifest.id
|
||||
)
|
||||
# Mirror the register_commands() rewrite (#2101): resolve
|
||||
# extension-relative subdir references (agents/, knowledge-base/,
|
||||
# etc.) to their installed .specify/extensions/<id>/ location
|
||||
# before the generic placeholder/path resolution below.
|
||||
body = registrar.rewrite_extension_paths(body, manifest.id, extension_dir)
|
||||
body = registrar.resolve_skill_placeholders(
|
||||
selected_ai, frontmatter, body, self.project_root, extension_id=manifest.id
|
||||
)
|
||||
@@ -2760,32 +2755,18 @@ class ConfigManager:
|
||||
if not key.startswith(prefix):
|
||||
continue
|
||||
|
||||
# Remove prefix and split into parts. Drop empty components from a
|
||||
# malformed name (e.g. ``SPECKIT_<EXT>_`` with no key, or
|
||||
# consecutive underscores ``SPECKIT_X__Y``) so we never create an
|
||||
# entry under an empty key.
|
||||
config_path = [p for p in key[len(prefix) :].lower().split("_") if p]
|
||||
if not config_path:
|
||||
continue
|
||||
# Remove prefix and split into parts
|
||||
config_path = key[len(prefix) :].lower().split("_")
|
||||
|
||||
# Build nested dict. Two env vars can collide on a prefix, e.g.
|
||||
# SPECKIT_X_CONNECTION=a and SPECKIT_X_CONNECTION_URL=b. Guard the
|
||||
# walk so a colliding scalar is replaced by a dict (deeper/more
|
||||
# specific vars win) instead of being indexed into — which raised
|
||||
# TypeError ('str' object does not support item assignment) — and
|
||||
# guard the leaf so a scalar processed after the nested var does
|
||||
# not clobber the nested dict. Order-independent: both insertion
|
||||
# orders yield {'connection': {'url': ...}}. Nested-wins mirrors
|
||||
# _merge_configs' dict-preserving semantics.
|
||||
# Build nested dict
|
||||
current = env_config
|
||||
for part in config_path[:-1]:
|
||||
if not isinstance(current.get(part), dict):
|
||||
if part not in current:
|
||||
current[part] = {}
|
||||
current = current[part]
|
||||
|
||||
# Set the final value, unless a nested dict already occupies it.
|
||||
if not isinstance(current.get(config_path[-1]), dict):
|
||||
current[config_path[-1]] = value
|
||||
# Set the final value
|
||||
current[config_path[-1]] = value
|
||||
|
||||
return env_config
|
||||
|
||||
|
||||
@@ -1566,14 +1566,7 @@ def extension_set_priority(
|
||||
raw_priority = metadata.get("priority")
|
||||
# Only skip if the stored value is already a valid int equal to requested priority
|
||||
# This ensures corrupted values (e.g., "high") get repaired even when setting to default (10)
|
||||
# A bool is an int in Python (isinstance(True, int) is True), so exclude it explicitly —
|
||||
# mirroring normalize_priority's bool guard — otherwise a corrupted True/False priority
|
||||
# equals 1/0 here and is never repaired.
|
||||
if (
|
||||
isinstance(raw_priority, int)
|
||||
and not isinstance(raw_priority, bool)
|
||||
and raw_priority == priority
|
||||
):
|
||||
if isinstance(raw_priority, int) and raw_priority == priority:
|
||||
console.print(f"[yellow]Extension '{_escape_markup(str(display_name))}' already has priority {priority}[/yellow]")
|
||||
raise typer.Exit(0)
|
||||
|
||||
|
||||
@@ -13,7 +13,6 @@ _KIRO_ARG_FALLBACK = "(the user will provide the argument in this conversation)"
|
||||
|
||||
class KiroCliIntegration(MarkdownIntegration):
|
||||
key = "kiro-cli"
|
||||
multi_install_safe = True
|
||||
config = {
|
||||
"name": "Kiro CLI",
|
||||
"folder": ".kiro/",
|
||||
|
||||
@@ -778,7 +778,6 @@ class PresetManager:
|
||||
matching_cmds, ext_id, ext_dir,
|
||||
self.project_root,
|
||||
context_note=f"\n<!-- Extension: {ext_id} -->\n<!-- Config: .specify/extensions/{ext_id}/ -->\n",
|
||||
extension_id=ext_id,
|
||||
)
|
||||
registered = True
|
||||
except Exception:
|
||||
@@ -1200,8 +1199,6 @@ class PresetManager:
|
||||
"command_name": cmd_name,
|
||||
"source_file": source_file,
|
||||
"source": f"extension:{manifest.id}",
|
||||
"extension_id": manifest.id,
|
||||
"extension_dir": ext_root,
|
||||
}
|
||||
modern_skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name)
|
||||
restore_index.setdefault(modern_skill_name, restore_info)
|
||||
@@ -1466,17 +1463,6 @@ class PresetManager:
|
||||
if extension_restore:
|
||||
content = extension_restore["source_file"].read_text(encoding="utf-8")
|
||||
frontmatter, body = registrar.parse_frontmatter(content)
|
||||
# Mirror the register-time rewrite (#2101): resolve
|
||||
# extension-relative subdir references (agents/,
|
||||
# knowledge-base/, etc.) to their installed location before
|
||||
# the generic placeholder resolution below, otherwise
|
||||
# restoring after a preset override removal would leave
|
||||
# bare, unresolvable paths in the skill body.
|
||||
body = registrar.rewrite_extension_paths(
|
||||
body,
|
||||
extension_restore["extension_id"],
|
||||
extension_restore["extension_dir"],
|
||||
)
|
||||
if isinstance(selected_ai, str):
|
||||
body = registrar.resolve_skill_placeholders(
|
||||
selected_ai, frontmatter, body, self.project_root
|
||||
@@ -3052,8 +3038,6 @@ class PresetResolver:
|
||||
"path": candidate,
|
||||
"source": source,
|
||||
"strategy": "replace",
|
||||
"extension_id": ext_id,
|
||||
"extension_dir": ext_dir,
|
||||
})
|
||||
|
||||
# Priority 4: Core templates (always "replace")
|
||||
@@ -3173,32 +3157,10 @@ class PresetResolver:
|
||||
if not layers:
|
||||
return None
|
||||
|
||||
def _read_layer_content(layer: Dict[str, Any]) -> str:
|
||||
"""Read a layer's raw text, rewriting extension-relative subdir
|
||||
references (agents/, knowledge-base/, etc.) to their installed
|
||||
location when the layer is extension-provided (#2101).
|
||||
|
||||
Extension layers are always inserted with strategy "replace"
|
||||
(see collect_all_layers), so a layer only ever needs this
|
||||
rewrite when it wins outright above or serves as the
|
||||
composition base below — never as a mid-stack composing
|
||||
(append/prepend/wrap) layer.
|
||||
"""
|
||||
text = layer["path"].read_text(encoding="utf-8")
|
||||
extension_id = layer.get("extension_id")
|
||||
extension_dir = layer.get("extension_dir")
|
||||
if extension_id and extension_dir:
|
||||
from ..agents import CommandRegistrar
|
||||
|
||||
text = CommandRegistrar.rewrite_extension_paths(
|
||||
text, extension_id, extension_dir
|
||||
)
|
||||
return text
|
||||
|
||||
# If the top (highest-priority) layer is replace, it wins entirely —
|
||||
# lower layers are irrelevant regardless of their strategies.
|
||||
if layers[0]["strategy"] == "replace":
|
||||
return _read_layer_content(layers[0])
|
||||
return layers[0]["path"].read_text(encoding="utf-8")
|
||||
|
||||
# Composition: build content bottom-up from the effective base.
|
||||
# The base is the nearest replace layer scanning from highest priority
|
||||
@@ -3221,7 +3183,7 @@ class PresetResolver:
|
||||
|
||||
# Convert to reversed_layers index
|
||||
base_reversed_idx = len(layers) - 1 - base_layer_idx
|
||||
content = _read_layer_content(layers[base_layer_idx])
|
||||
content = layers[base_layer_idx]["path"].read_text(encoding="utf-8")
|
||||
# Compose only the layers above the base (higher priority = lower index in layers,
|
||||
# higher index in reversed_layers). Process bottom-up from base+1.
|
||||
start_idx = base_reversed_idx + 1
|
||||
|
||||
@@ -469,14 +469,7 @@ def preset_set_priority(
|
||||
raw_priority = metadata.get("priority")
|
||||
# Only skip if the stored value is already a valid int equal to requested priority
|
||||
# This ensures corrupted values (e.g., "high") get repaired even when setting to default (10)
|
||||
# A bool is an int in Python (isinstance(True, int) is True), so exclude it explicitly —
|
||||
# mirroring normalize_priority's bool guard — otherwise a corrupted True/False priority
|
||||
# equals 1/0 here and is never repaired.
|
||||
if (
|
||||
isinstance(raw_priority, int)
|
||||
and not isinstance(raw_priority, bool)
|
||||
and raw_priority == priority
|
||||
):
|
||||
if isinstance(raw_priority, int) and raw_priority == priority:
|
||||
console.print(f"[yellow]Preset '{preset_id}' already has priority {priority}[/yellow]")
|
||||
raise typer.Exit(0)
|
||||
|
||||
|
||||
@@ -76,16 +76,8 @@ class WorkflowRegistry:
|
||||
if self.registry_path.exists():
|
||||
try:
|
||||
with open(self.registry_path, encoding="utf-8") as f:
|
||||
data = json.load(f)
|
||||
# Validate shape: must be a dict with a dict "workflows" field,
|
||||
# otherwise every method that indexes data["workflows"] crashes.
|
||||
# Mirrors StepRegistry._load.
|
||||
if not isinstance(data, dict):
|
||||
return {"schema_version": self.SCHEMA_VERSION, "workflows": {}}
|
||||
if not isinstance(data.get("workflows"), dict):
|
||||
data["workflows"] = {}
|
||||
return data
|
||||
except (json.JSONDecodeError, ValueError, OSError, UnicodeError):
|
||||
return json.load(f)
|
||||
except (json.JSONDecodeError, ValueError):
|
||||
# Corrupted registry file — reset to default
|
||||
return {"schema_version": self.SCHEMA_VERSION, "workflows": {}}
|
||||
return {"schema_version": self.SCHEMA_VERSION, "workflows": {}}
|
||||
@@ -446,9 +438,9 @@ class WorkflowCatalog:
|
||||
q = query.lower()
|
||||
searchable = " ".join(
|
||||
[
|
||||
str(wf_data.get("name") or ""),
|
||||
str(wf_data.get("description") or ""),
|
||||
str(wf_data.get("id") or ""),
|
||||
wf_data.get("name", ""),
|
||||
wf_data.get("description", ""),
|
||||
wf_data.get("id", ""),
|
||||
]
|
||||
).lower()
|
||||
if q not in searchable:
|
||||
|
||||
@@ -982,16 +982,7 @@ class WorkflowEngine:
|
||||
from .expressions import evaluate_condition
|
||||
|
||||
max_iters = step_config.get("max_iterations")
|
||||
# A bool is an int in Python (isinstance(True, int) is True
|
||||
# and True == 1), so a bool max_iterations would slip past
|
||||
# the int check and cap the loop at range(0)==1 iteration
|
||||
# instead of the default. Exclude bools, mirroring the
|
||||
# while/do-while validators and the continue_on_error guard.
|
||||
if (
|
||||
isinstance(max_iters, bool)
|
||||
or not isinstance(max_iters, int)
|
||||
or max_iters < 1
|
||||
):
|
||||
if not isinstance(max_iters, int) or max_iters < 1:
|
||||
max_iters = 10
|
||||
condition = step_config.get("condition", False)
|
||||
for _loop_iter in range(max_iters - 1):
|
||||
|
||||
@@ -464,9 +464,9 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any:
|
||||
if op == "<=":
|
||||
return _safe_compare(left, right, "<=")
|
||||
if op == " in ":
|
||||
return _safe_membership(left, right, negate=False)
|
||||
return left in right if right is not None else False
|
||||
if op == " not in ":
|
||||
return _safe_membership(left, right, negate=True)
|
||||
return left not in right if right is not None else True
|
||||
|
||||
# Numeric literal
|
||||
try:
|
||||
@@ -511,26 +511,6 @@ def _coerce_number(value: Any) -> Any:
|
||||
return value
|
||||
|
||||
|
||||
def _safe_membership(left: Any, right: Any, *, negate: bool) -> bool:
|
||||
"""Safely evaluate ``left in right`` (or ``not in``) without crashing.
|
||||
|
||||
``left in right`` raises ``TypeError`` whenever the operands don't support
|
||||
membership testing — most commonly a non-iterable right operand (``None``,
|
||||
an int, a bool), but also cases like an unhashable ``left`` against a set.
|
||||
In every such case the membership relation is undefined, so treat it as
|
||||
``False`` (``not in`` as ``True``) rather than leaking the error out of the
|
||||
evaluator and crashing the whole workflow. Mirrors the graceful
|
||||
``TypeError`` handling in ``_safe_compare`` for the ordering operators, and
|
||||
generalizes the previous ``right is not None`` guard to any operand pair
|
||||
that can't be membership-tested.
|
||||
"""
|
||||
try:
|
||||
contained = left in right
|
||||
except TypeError:
|
||||
contained = False
|
||||
return not contained if negate else contained
|
||||
|
||||
|
||||
def _safe_compare(left: Any, right: Any, op: str) -> bool:
|
||||
"""Compare two values for ordering, coercing numeric strings when possible.
|
||||
|
||||
|
||||
@@ -47,8 +47,8 @@ class IfThenStep(StepBase):
|
||||
errors.append(
|
||||
f"If step {config.get('id', '?')!r}: 'then' must be a list of steps."
|
||||
)
|
||||
else_branch = config.get("else")
|
||||
if else_branch is not None and not isinstance(else_branch, list):
|
||||
else_branch = config.get("else", [])
|
||||
if else_branch and not isinstance(else_branch, list):
|
||||
errors.append(
|
||||
f"If step {config.get('id', '?')!r}: 'else' must be a list of steps."
|
||||
)
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import math
|
||||
import subprocess
|
||||
from typing import Any
|
||||
|
||||
@@ -26,20 +25,15 @@ class ShellStep(StepBase):
|
||||
run_cmd = str(run_cmd)
|
||||
|
||||
cwd = context.project_root or "."
|
||||
# Per-step execution timeout in seconds; defaults to 300 for backward
|
||||
# compatibility. The engine does not auto-validate step config, so
|
||||
# validate here as well — a caller that skips WorkflowEngine.validate()
|
||||
# must fail the step cleanly rather than crash subprocess.run() with a
|
||||
# TypeError (or silently coerce ``timeout: true`` to a 1s duration,
|
||||
# since bool is an int subclass).
|
||||
# Defensive: the engine does not auto-validate step config, so an
|
||||
# invalid ``timeout`` (string, None, ...) would otherwise raise a
|
||||
# TypeError from subprocess.run() and crash the whole run. Mirror
|
||||
# the engine's handling of unvalidated ``continue_on_error`` by
|
||||
# only honoring well-formed values and falling back to the default.
|
||||
timeout = config.get("timeout", 300)
|
||||
timeout_error = self._timeout_error(config)
|
||||
if timeout_error is not None:
|
||||
return StepResult(
|
||||
status=StepStatus.FAILED,
|
||||
error=timeout_error,
|
||||
output={"exit_code": -1, "stdout": "", "stderr": "invalid timeout"},
|
||||
)
|
||||
if isinstance(timeout, bool) or not isinstance(timeout, int) or timeout <= 0:
|
||||
timeout = 300
|
||||
|
||||
# NOTE: shell=True is required to support pipes, redirects, and
|
||||
# multi-command expressions in workflow YAML. Workflow authors
|
||||
# control commands; catalog-installed workflows should be reviewed
|
||||
@@ -98,32 +92,6 @@ class ShellStep(StepBase):
|
||||
output={"exit_code": -1, "stdout": "", "stderr": str(exc)},
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _timeout_error(config: dict[str, Any]) -> str | None:
|
||||
"""Return an error message if ``config['timeout']`` is invalid, else None.
|
||||
|
||||
Shared by execute() and validate() so both paths reject the same
|
||||
values with the same message. An absent ``timeout`` is valid (the
|
||||
default is used). bool is a subclass of int, but ``timeout: true`` is a
|
||||
config error rather than a duration, so it is rejected explicitly.
|
||||
Non-finite floats (YAML ``.inf``/``.nan``) pass a plain ``> 0`` check
|
||||
but would raise in subprocess.run(), so they are rejected too.
|
||||
"""
|
||||
if "timeout" not in config:
|
||||
return None
|
||||
timeout = config["timeout"]
|
||||
if (
|
||||
isinstance(timeout, bool)
|
||||
or not isinstance(timeout, (int, float))
|
||||
or not math.isfinite(timeout)
|
||||
or timeout <= 0
|
||||
):
|
||||
return (
|
||||
f"Shell step {config.get('id', '?')!r}: 'timeout' must be a "
|
||||
f"positive number of seconds, got {timeout!r}."
|
||||
)
|
||||
return None
|
||||
|
||||
def validate(self, config: dict[str, Any]) -> list[str]:
|
||||
errors = super().validate(config)
|
||||
if "run" not in config:
|
||||
@@ -146,7 +114,16 @@ class ShellStep(StepBase):
|
||||
f"Shell step {config.get('id', '?')!r}: 'output_format' must "
|
||||
f"be 'json' when present, got {output_format!r}."
|
||||
)
|
||||
timeout_error = self._timeout_error(config)
|
||||
if timeout_error is not None:
|
||||
errors.append(timeout_error)
|
||||
if "timeout" in config:
|
||||
timeout = config["timeout"]
|
||||
# bool is an int subclass, so reject it explicitly.
|
||||
if (
|
||||
isinstance(timeout, bool)
|
||||
or not isinstance(timeout, int)
|
||||
or timeout <= 0
|
||||
):
|
||||
errors.append(
|
||||
f"Shell step {config.get('id', '?')!r}: 'timeout' must be a "
|
||||
f"positive integer (seconds) when present, got {timeout!r}."
|
||||
)
|
||||
return errors
|
||||
|
||||
@@ -26,20 +26,6 @@ class SwitchStep(StepBase):
|
||||
str_value = str(value) if value is not None else ""
|
||||
|
||||
cases = config.get("cases", {})
|
||||
if not isinstance(cases, dict):
|
||||
# The engine does not auto-validate step config, so an unvalidated
|
||||
# run with a non-mapping ``cases`` (a list/scalar authoring mistake)
|
||||
# would otherwise raise AttributeError from ``.items()`` below and
|
||||
# crash the whole run. Fail this step loudly instead, mirroring the
|
||||
# fan-out step's non-list ``items`` handling.
|
||||
return StepResult(
|
||||
status=StepStatus.FAILED,
|
||||
error=(
|
||||
f"Switch step {config.get('id', '?')!r}: 'cases' must be a "
|
||||
f"mapping, got {type(cases).__name__}."
|
||||
),
|
||||
output={"matched_case": None, "expression_value": value},
|
||||
)
|
||||
for case_key, case_steps in cases.items():
|
||||
if str(case_key) == str_value:
|
||||
return StepResult(
|
||||
|
||||
@@ -81,7 +81,7 @@ Follow this execution flow:
|
||||
- Read `.specify/templates/plan-template.md` and ensure any "Constitution Check" or rules align with updated principles.
|
||||
- Read `.specify/templates/spec-template.md` for scope/requirements alignment—update if constitution adds/removes mandatory sections or constraints.
|
||||
- Read `.specify/templates/tasks-template.md` and ensure task categorization reflects new or removed principle-driven task types (e.g., observability, versioning, testing discipline).
|
||||
- Read each installed Spec Kit command file for your agent (including this one) — named `speckit.*` or `speckit-*` (dot or hyphen depending on the agent), or laid out as `speckit-<name>/SKILL.md` for skills-based integrations, e.g. in `.github/agents/`, `.github/skills/`, `.claude/skills/`, or your agent's equivalent commands directory — to verify no outdated references (CLAUDE-only or other agent-specific names) remain when generic guidance is required.
|
||||
- Read each command file in `.specify/templates/commands/*.md` (including this one) to verify no outdated references (agent-specific names like CLAUDE only) remain when generic guidance is required.
|
||||
- Read any runtime guidance docs (e.g., `README.md`, `docs/quickstart.md`, or agent-specific guidance files if present). Update references to principles changed.
|
||||
|
||||
5. Produce a Sync Impact Report (prepend as an HTML comment at top of the constitution file after update):
|
||||
|
||||
@@ -175,23 +175,7 @@ def test_build_produces_artifact(project: Path):
|
||||
assert len(artifacts) == 1
|
||||
|
||||
|
||||
def _mock_manifest_download(monkeypatch, source_path: Path) -> None:
|
||||
"""Mock the HTTPS manifest fetch to return a locally-authored manifest.
|
||||
|
||||
Catalog ``download_url``s are HTTPS-only, so ``info`` tests can no longer
|
||||
point one at a local file. Patch ``_download_manifest`` to return the
|
||||
manifest parsed from *source_path* (a bundle.yml or a .zip artifact),
|
||||
exercising ``info``'s expansion without a network call.
|
||||
"""
|
||||
from specify_cli.commands.bundle import _local_manifest_source
|
||||
|
||||
monkeypatch.setattr(
|
||||
"specify_cli.commands.bundle._download_manifest",
|
||||
lambda resolved, *, offline: _local_manifest_source(str(source_path)),
|
||||
)
|
||||
|
||||
|
||||
def test_info_expands_full_component_set(project: Path, monkeypatch):
|
||||
def test_info_expands_full_component_set(project: Path):
|
||||
bundle_dir = project / "src-bundle"
|
||||
bundle_dir.mkdir()
|
||||
(bundle_dir / "bundle.yml").write_text(
|
||||
@@ -199,14 +183,13 @@ def test_info_expands_full_component_set(project: Path, monkeypatch):
|
||||
)
|
||||
catalog = project / "local-catalog.json"
|
||||
entry = catalog_entry_dict(
|
||||
"demo-bundle", download_url="https://example.com/demo-bundle.zip"
|
||||
"demo-bundle", download_url=str(bundle_dir / "bundle.yml")
|
||||
)
|
||||
write_catalog_file(catalog, {"demo-bundle": entry})
|
||||
added = runner.invoke(
|
||||
app, ["bundle", "catalog", "add", str(catalog), "--id", "local"]
|
||||
)
|
||||
assert added.exit_code == 0, added.output
|
||||
_mock_manifest_download(monkeypatch, bundle_dir / "bundle.yml")
|
||||
|
||||
result = runner.invoke(app, ["bundle", "info", "demo-bundle", "--json", "--offline"])
|
||||
assert result.exit_code == 0, result.output
|
||||
@@ -224,7 +207,7 @@ def test_info_expands_full_component_set(project: Path, monkeypatch):
|
||||
assert "Trust" in text.output
|
||||
|
||||
|
||||
def test_info_expands_discovery_only_bundle(project: Path, monkeypatch):
|
||||
def test_info_expands_discovery_only_bundle(project: Path):
|
||||
# Discovery-only bundles must still be fully inspectable via `info`;
|
||||
# only `install` is refused for them.
|
||||
bundle_dir = project / "disc-bundle"
|
||||
@@ -234,7 +217,7 @@ def test_info_expands_discovery_only_bundle(project: Path, monkeypatch):
|
||||
)
|
||||
catalog = project / "disc-catalog.json"
|
||||
entry = catalog_entry_dict(
|
||||
"demo-bundle", download_url="https://example.com/demo-bundle.zip"
|
||||
"demo-bundle", download_url=str(bundle_dir / "bundle.yml")
|
||||
)
|
||||
write_catalog_file(catalog, {"demo-bundle": entry})
|
||||
config = {
|
||||
@@ -247,7 +230,6 @@ def test_info_expands_discovery_only_bundle(project: Path, monkeypatch):
|
||||
(project / ".specify" / "bundle-catalogs.yml").write_text(
|
||||
yaml.safe_dump(config), encoding="utf-8"
|
||||
)
|
||||
_mock_manifest_download(monkeypatch, bundle_dir / "bundle.yml")
|
||||
result = runner.invoke(app, ["bundle", "info", "demo-bundle", "--json", "--offline"])
|
||||
assert result.exit_code == 0, result.output
|
||||
payload = json.loads(result.output)
|
||||
@@ -255,9 +237,8 @@ def test_info_expands_discovery_only_bundle(project: Path, monkeypatch):
|
||||
assert ("extensions", "ext-a") in components
|
||||
|
||||
|
||||
def test_info_expands_zip_sourced_bundle(project: Path, monkeypatch):
|
||||
# A .zip artifact is extracted to read bundle.yml; info expands it. (The
|
||||
# download itself is HTTPS-only now and mocked here — see contract note.)
|
||||
def test_info_resolves_local_zip_download_url(project: Path):
|
||||
# A local .zip artifact as download_url is extracted to read bundle.yml.
|
||||
bundle_dir = project / "zip-src"
|
||||
bundle_dir.mkdir()
|
||||
(bundle_dir / "bundle.yml").write_text(
|
||||
@@ -268,15 +249,12 @@ def test_info_expands_zip_sourced_bundle(project: Path, monkeypatch):
|
||||
catalog = project / "zip-catalog.json"
|
||||
write_catalog_file(
|
||||
catalog,
|
||||
{"demo-bundle": catalog_entry_dict(
|
||||
"demo-bundle", download_url="https://example.com/demo-bundle.zip"
|
||||
)},
|
||||
{"demo-bundle": catalog_entry_dict("demo-bundle", download_url=str(artifact))},
|
||||
)
|
||||
added = runner.invoke(
|
||||
app, ["bundle", "catalog", "add", str(catalog), "--id", "local"]
|
||||
)
|
||||
assert added.exit_code == 0, added.output
|
||||
_mock_manifest_download(monkeypatch, artifact)
|
||||
result = runner.invoke(app, ["bundle", "info", "demo-bundle", "--json", "--offline"])
|
||||
assert result.exit_code == 0, result.output
|
||||
payload = json.loads(result.output)
|
||||
|
||||
@@ -112,62 +112,3 @@ def test_install_bundled_extension_from_zip_offline(tmp_path: Path):
|
||||
assert not ExtensionManager(project).registry.is_installed("agent-context")
|
||||
finally:
|
||||
os.chdir(previous)
|
||||
|
||||
|
||||
def test_download_manifest_rejects_file_url(tmp_path: Path):
|
||||
"""A catalog ``file://`` download_url is rejected — catalog URLs are
|
||||
HTTPS-only, matching extensions/presets/workflows. Disk installs go through
|
||||
the positional path (see the local-source tests above), not download_url.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
|
||||
from specify_cli.commands.bundle import _download_manifest
|
||||
|
||||
manifest_path = write_manifest(tmp_path / "my bundles")
|
||||
resolved = SimpleNamespace(
|
||||
entry=SimpleNamespace(id="demo-bundle", download_url=manifest_path.as_uri())
|
||||
)
|
||||
|
||||
with pytest.raises(BundlerError, match="bundle install"):
|
||||
_download_manifest(resolved, offline=True)
|
||||
|
||||
|
||||
def test_download_manifest_rejects_bare_path(tmp_path: Path):
|
||||
"""A bare filesystem path download_url is likewise rejected."""
|
||||
from types import SimpleNamespace
|
||||
|
||||
from specify_cli.commands.bundle import _download_manifest
|
||||
|
||||
manifest_path = write_manifest(tmp_path / "plain")
|
||||
resolved = SimpleNamespace(
|
||||
entry=SimpleNamespace(id="demo-bundle", download_url=str(manifest_path))
|
||||
)
|
||||
|
||||
with pytest.raises(BundlerError, match="bundle install"):
|
||||
_download_manifest(resolved, offline=True)
|
||||
|
||||
|
||||
def test_local_install_still_resolves_via_positional_path(tmp_path: Path):
|
||||
"""The supported local route — a positional path, not a download_url —
|
||||
still resolves the manifest via _local_manifest_source."""
|
||||
manifest_path = write_manifest(tmp_path / "my bundles")
|
||||
manifest = _local_manifest_source(str(manifest_path))
|
||||
assert manifest is not None
|
||||
assert manifest.bundle.id == "demo-bundle"
|
||||
|
||||
|
||||
def test_download_manifest_rejects_non_https_url_even_offline(tmp_path: Path):
|
||||
"""A non-HTTPS download_url must report the HTTPS problem, not a misleading
|
||||
'Network access disabled', even under --offline (scheme is validated before
|
||||
the offline gate)."""
|
||||
from types import SimpleNamespace
|
||||
|
||||
from specify_cli.commands.bundle import _download_manifest
|
||||
|
||||
resolved = SimpleNamespace(
|
||||
entry=SimpleNamespace(
|
||||
id="demo-bundle", download_url="http://example.com/bundle.zip"
|
||||
)
|
||||
)
|
||||
with pytest.raises(BundlerError, match="HTTPS"):
|
||||
_download_manifest(resolved, offline=True)
|
||||
|
||||
@@ -42,9 +42,6 @@ class TestKiroCliIntegration(MarkdownIntegrationTests):
|
||||
COMMANDS_SUBDIR = "prompts"
|
||||
REGISTRAR_DIR = ".kiro/prompts"
|
||||
|
||||
def test_declares_multi_install_safe(self):
|
||||
assert get_integration(self.KEY).multi_install_safe is True
|
||||
|
||||
def test_registrar_config(self):
|
||||
"""Override base assertion: kiro-cli uses a prose fallback for args
|
||||
because Kiro CLI file-based prompts do not natively substitute
|
||||
|
||||
@@ -911,65 +911,6 @@ class TestExtensionSkillRegistration:
|
||||
assert ".specify/scripts/bash/resolve-skill.sh" not in content
|
||||
assert ".specify/scripts/bash/ensure-skills.sh" not in content
|
||||
|
||||
def test_skill_registration_rewrites_extension_subdir_paths(self, project_dir, temp_dir):
|
||||
"""Auto-registered skills should resolve extension-relative subdir
|
||||
references (agents/, knowledge-base/) to their installed location,
|
||||
matching the rewrite already applied by register_commands() (#2101)."""
|
||||
_create_init_options(project_dir, ai="claude", ai_skills=True)
|
||||
skills_dir = _create_skills_dir(project_dir, ai="claude")
|
||||
|
||||
ext_dir = temp_dir / "path-ext"
|
||||
ext_dir.mkdir()
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"extension": {
|
||||
"id": "path-ext",
|
||||
"name": "Path Extension",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"commands": [
|
||||
{
|
||||
"name": "speckit.path-ext.run",
|
||||
"file": "commands/run.md",
|
||||
"description": "Run command",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(ext_dir / "extension.yml", "w") as f:
|
||||
yaml.safe_dump(manifest_data, f)
|
||||
|
||||
(ext_dir / "commands").mkdir()
|
||||
(ext_dir / "agents" / "control").mkdir(parents=True)
|
||||
(ext_dir / "agents" / "control" / "commander.md").write_text("# Commander\n")
|
||||
(ext_dir / "knowledge-base").mkdir()
|
||||
(ext_dir / "knowledge-base" / "agent-scores.yaml").write_text("scores: {}\n")
|
||||
(ext_dir / "templates").mkdir()
|
||||
(ext_dir / "templates" / "kill-report.md").write_text("# Kill Report\n")
|
||||
|
||||
(ext_dir / "commands" / "run.md").write_text(
|
||||
"---\n"
|
||||
"description: Run command\n"
|
||||
"---\n\n"
|
||||
"Read agents/control/commander.md and knowledge-base/agent-scores.yaml.\n"
|
||||
"Use templates/kill-report.md as the report template.\n"
|
||||
)
|
||||
|
||||
manager = ExtensionManager(project_dir)
|
||||
manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
|
||||
|
||||
content = (skills_dir / "speckit-path-ext-run" / "SKILL.md").read_text()
|
||||
assert ".specify/extensions/path-ext/agents/control/commander.md" in content
|
||||
assert ".specify/extensions/path-ext/knowledge-base/agent-scores.yaml" in content
|
||||
# extension's own templates/ dir must resolve under the extension,
|
||||
# not the project-level .specify/templates/
|
||||
assert ".specify/extensions/path-ext/templates/kill-report.md" in content
|
||||
assert "Read agents/control" not in content
|
||||
assert "and knowledge-base/" not in content
|
||||
|
||||
def test_missing_command_file_skipped(self, skills_project, temp_dir):
|
||||
"""Commands with missing source files should be skipped gracefully."""
|
||||
project_dir, skills_dir = skills_project
|
||||
|
||||
@@ -2376,185 +2376,6 @@ Run {SCRIPT}
|
||||
assert ".specify/scripts/powershell/setup-plan.ps1 -Json" in content
|
||||
assert ".specify/scripts/bash/setup-plan.sh" not in content
|
||||
|
||||
@staticmethod
|
||||
def _make_subdir_extension(temp_dir, ext_id="echelon", aliases=None):
|
||||
"""Create an extension whose command body references bundled subdirs."""
|
||||
import yaml
|
||||
|
||||
ext_dir = temp_dir / ext_id
|
||||
ext_dir.mkdir()
|
||||
(ext_dir / "commands").mkdir()
|
||||
(ext_dir / "agents" / "control").mkdir(parents=True)
|
||||
(ext_dir / "knowledge-base").mkdir()
|
||||
(ext_dir / "templates").mkdir()
|
||||
(ext_dir / "specs" / "001-internal").mkdir(parents=True)
|
||||
|
||||
command = {
|
||||
"name": f"speckit.{ext_id}.run",
|
||||
"file": "commands/run.md",
|
||||
"description": "Run",
|
||||
}
|
||||
if aliases:
|
||||
command["aliases"] = aliases
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"extension": {
|
||||
"id": ext_id,
|
||||
"name": "Echelon",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {"commands": [command]},
|
||||
}
|
||||
with open(ext_dir / "extension.yml", "w") as f:
|
||||
yaml.dump(manifest_data, f)
|
||||
|
||||
(ext_dir / "commands" / "run.md").write_text(
|
||||
"---\ndescription: Run\n---\n\n"
|
||||
"Read agents/control/commander.md for instructions.\n"
|
||||
"Load knowledge-base/agent-scores.yaml for calibration.\n"
|
||||
"Use templates/kill-report.md as output format.\n"
|
||||
"Artifacts go to specs/001-internal/plan.md.\n"
|
||||
"See commands/run.md for the source.\n"
|
||||
)
|
||||
return ext_dir
|
||||
|
||||
def test_codex_skill_registration_rewrites_extension_subdir_paths(
|
||||
self, project_dir, temp_dir
|
||||
):
|
||||
"""Extension-relative subdir refs must point at the installed location."""
|
||||
ext_dir = self._make_subdir_extension(temp_dir)
|
||||
|
||||
skills_dir = project_dir / ".agents" / "skills"
|
||||
skills_dir.mkdir(parents=True)
|
||||
|
||||
manifest = ExtensionManifest(ext_dir / "extension.yml")
|
||||
registrar = CommandRegistrar()
|
||||
registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir)
|
||||
|
||||
content = (skills_dir / "speckit-echelon-run" / "SKILL.md").read_text()
|
||||
assert ".specify/extensions/echelon/agents/control/commander.md" in content
|
||||
assert ".specify/extensions/echelon/knowledge-base/agent-scores.yaml" in content
|
||||
assert ".specify/extensions/echelon/templates/kill-report.md" in content
|
||||
assert "Read agents/" not in content
|
||||
# specs/ refs point at the user's project artifacts, never the extension
|
||||
assert "to specs/001-internal/plan.md" in content
|
||||
assert ".specify/extensions/echelon/specs/" not in content
|
||||
# commands/ refs are slash-command sources, not runtime reads
|
||||
assert "See commands/run.md" in content
|
||||
|
||||
def test_skill_registration_rewrites_extension_subdir_paths_in_aliases(
|
||||
self, project_dir, temp_dir
|
||||
):
|
||||
"""Alias skills reuse the rewritten body."""
|
||||
ext_dir = self._make_subdir_extension(
|
||||
temp_dir, ext_id="ext-alias-paths", aliases=["speckit.ext-alias-paths.go"]
|
||||
)
|
||||
|
||||
skills_dir = project_dir / ".agents" / "skills"
|
||||
skills_dir.mkdir(parents=True)
|
||||
|
||||
manifest = ExtensionManifest(ext_dir / "extension.yml")
|
||||
registrar = CommandRegistrar()
|
||||
registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir)
|
||||
|
||||
alias_content = (
|
||||
skills_dir / "speckit-ext-alias-paths-go" / "SKILL.md"
|
||||
).read_text()
|
||||
assert (
|
||||
".specify/extensions/ext-alias-paths/agents/control/commander.md"
|
||||
in alias_content
|
||||
)
|
||||
assert "Read agents/" not in alias_content
|
||||
|
||||
def test_markdown_registration_rewrites_extension_subdir_paths(
|
||||
self, project_dir, temp_dir
|
||||
):
|
||||
"""Markdown-format agents get the same rewrite via the shared path."""
|
||||
ext_dir = self._make_subdir_extension(temp_dir, ext_id="ext-md-paths")
|
||||
|
||||
amp_dir = project_dir / ".agents" / "commands"
|
||||
amp_dir.mkdir(parents=True)
|
||||
|
||||
manifest = ExtensionManifest(ext_dir / "extension.yml")
|
||||
registrar = CommandRegistrar()
|
||||
registrar.register_commands_for_agent("amp", manifest, ext_dir, project_dir)
|
||||
|
||||
content = (amp_dir / "speckit.ext-md-paths.run.md").read_text()
|
||||
assert ".specify/extensions/ext-md-paths/agents/control/commander.md" in content
|
||||
assert "Read agents/" not in content
|
||||
|
||||
def test_rewrite_extension_paths_only_rewrites_existing_subdirs(self, temp_dir):
|
||||
"""Only directories present in the extension are rewritten."""
|
||||
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
|
||||
|
||||
ext_dir = temp_dir / "ext-existing"
|
||||
(ext_dir / "agents").mkdir(parents=True)
|
||||
(ext_dir / ".hidden").mkdir()
|
||||
|
||||
text = (
|
||||
"Read agents/one.md then knowledge-base/two.md.\n"
|
||||
"Also ./agents/three.md but not /agents/abs.md.\n"
|
||||
"Keep .hidden/secret.md alone.\n"
|
||||
)
|
||||
rewritten = AgentCommandRegistrar.rewrite_extension_paths(
|
||||
text, "ext-existing", ext_dir
|
||||
)
|
||||
|
||||
assert ".specify/extensions/ext-existing/agents/one.md" in rewritten
|
||||
assert "Also .specify/extensions/ext-existing/agents/three.md" in rewritten
|
||||
# absolute paths keep their meaning
|
||||
assert "not /agents/abs.md" in rewritten
|
||||
# knowledge-base/ does not exist in this extension: left untouched
|
||||
assert "then knowledge-base/two.md" in rewritten
|
||||
assert ".hidden/secret.md" in rewritten
|
||||
assert ".specify/extensions/ext-existing/.hidden/" not in rewritten
|
||||
|
||||
def test_rewrite_extension_paths_handles_regex_special_replacement_text(
|
||||
self, temp_dir
|
||||
):
|
||||
"""subdir/extension_id containing regex-replacement-special characters
|
||||
(e.g. backslash / group references) must not raise or be misinterpreted
|
||||
by re.sub's replacement template (#2101).
|
||||
|
||||
The subdir name uses brackets rather than a backslash: on Windows,
|
||||
"\\" is a path separator, so a subdir literally named "assets\\q"
|
||||
would create nested directories "assets/q" instead of a single
|
||||
directory, and iterdir() would then only discover "assets" - never
|
||||
exercising the intended replacement text. extension_id isn't used to
|
||||
create a directory, so it's free to contain a real backslash/"\\1"
|
||||
to verify the callable replacement treats it literally.
|
||||
"""
|
||||
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
|
||||
|
||||
ext_dir = temp_dir / "ext-backslash"
|
||||
weird_subdir = "assets[q]"
|
||||
(ext_dir / weird_subdir).mkdir(parents=True)
|
||||
# sanity-check the cross-platform assumption above
|
||||
assert [p.name for p in ext_dir.iterdir()] == [weird_subdir]
|
||||
|
||||
text = f"Read {weird_subdir}/file.md but not /{weird_subdir}/abs.md.\n"
|
||||
rewritten = AgentCommandRegistrar.rewrite_extension_paths(
|
||||
text, "ext\\1", ext_dir
|
||||
)
|
||||
|
||||
assert f".specify/extensions/ext\\1/{weird_subdir}/file.md" in rewritten
|
||||
# absolute paths are still left untouched
|
||||
assert f"/{weird_subdir}/abs.md" in rewritten
|
||||
|
||||
def test_rewrite_extension_paths_missing_dir_returns_text(self, temp_dir):
|
||||
"""A missing extension directory leaves the text unchanged."""
|
||||
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
|
||||
|
||||
text = "Read agents/one.md."
|
||||
assert (
|
||||
AgentCommandRegistrar.rewrite_extension_paths(
|
||||
text, "gone", temp_dir / "does-not-exist"
|
||||
)
|
||||
== text
|
||||
)
|
||||
|
||||
def test_register_commands_for_copilot(self, extension_dir, project_dir):
|
||||
"""Test registering commands for Copilot agent with .agent.md extension."""
|
||||
# Create .github/agents directory (Copilot project)
|
||||
@@ -6603,42 +6424,6 @@ class TestExtensionPriorityCLI:
|
||||
plain = strip_ansi(result.output)
|
||||
assert "already has priority 5" in plain
|
||||
|
||||
def test_set_priority_repairs_corrupted_bool(self, extension_dir, project_dir):
|
||||
"""A corrupted boolean priority must be repaired, not skipped.
|
||||
|
||||
``isinstance(True, int)`` is True and ``True == 1`` in Python, so a
|
||||
stored ``True`` priority would short-circuit the ``already has
|
||||
priority 1`` skip path and never get rewritten to a real int —
|
||||
contradicting the comment that promises corrupted values are
|
||||
repaired. The guard must exclude bools (like normalize_priority).
|
||||
"""
|
||||
from typer.testing import CliRunner
|
||||
from unittest.mock import patch
|
||||
from specify_cli import app
|
||||
|
||||
runner = CliRunner()
|
||||
|
||||
manager = ExtensionManager(project_dir)
|
||||
manager.install_from_directory(
|
||||
extension_dir, "0.1.0", register_commands=False, priority=5
|
||||
)
|
||||
# Inject a corrupted boolean priority (True == 1).
|
||||
manager.registry.update("test-ext", {"priority": True})
|
||||
|
||||
with patch.object(Path, "cwd", return_value=project_dir):
|
||||
result = runner.invoke(app, ["extension", "set-priority", "test-ext", "1"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
plain = strip_ansi(result.output)
|
||||
# The corrupted bool must be repaired, not reported as already-set.
|
||||
assert "already has priority" not in plain
|
||||
assert "priority changed" in plain
|
||||
|
||||
# The stored value is now a real int, not a bool.
|
||||
reloaded = ExtensionManager(project_dir).registry.get("test-ext")
|
||||
assert reloaded["priority"] == 1
|
||||
assert not isinstance(reloaded["priority"], bool)
|
||||
|
||||
def test_set_priority_invalid_value(self, extension_dir, project_dir):
|
||||
"""Test set-priority rejects invalid priority values."""
|
||||
from typer.testing import CliRunner
|
||||
@@ -7843,56 +7628,3 @@ class TestConfigManagerNonMappingYaml:
|
||||
(ext_dir / "jira-config.yml").write_text("just a string\n", encoding="utf-8")
|
||||
executor = HookExecutor(tmp_path)
|
||||
assert executor._evaluate_condition("config.x is set", "jira") is False
|
||||
|
||||
|
||||
class TestConfigManagerEnvPrefixCollision:
|
||||
"""Prefix-colliding env vars must not crash or clobber nested config."""
|
||||
|
||||
def test_scalar_then_nested_yields_nested(self, tmp_path, monkeypatch):
|
||||
"""SPECKIT_X_CONNECTION=x then SPECKIT_X_CONNECTION_URL=y.
|
||||
|
||||
The scalar-first order previously raised TypeError ('str' object
|
||||
does not support item assignment) when the walk indexed into 'x'.
|
||||
"""
|
||||
monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION", "x")
|
||||
monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION_URL", "y")
|
||||
cm = ConfigManager(tmp_path, "testext")
|
||||
assert cm._get_env_config() == {"connection": {"url": "y"}}
|
||||
|
||||
def test_nested_then_scalar_does_not_clobber(self, tmp_path, monkeypatch):
|
||||
"""Reverse order previously returned {'connection': 'x'}, losing url."""
|
||||
monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION_URL", "y")
|
||||
monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION", "x")
|
||||
cm = ConfigManager(tmp_path, "testext")
|
||||
assert cm._get_env_config() == {"connection": {"url": "y"}}
|
||||
|
||||
def test_colliding_env_does_not_disable_hook_condition(self, tmp_path, monkeypatch):
|
||||
"""`config.connection.url is set` must stay True under colliding env.
|
||||
|
||||
Before the fix the TypeError propagated into should_execute_hook's
|
||||
blanket `except Exception: return False`, silently disabling the hook.
|
||||
"""
|
||||
ext_dir = tmp_path / ".specify" / "extensions" / "testext"
|
||||
ext_dir.mkdir(parents=True)
|
||||
(ext_dir / "testext-config.yml").write_text(
|
||||
"connection:\n url: https://example.com\n", encoding="utf-8"
|
||||
)
|
||||
monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION", "x")
|
||||
monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION_URL", "y")
|
||||
executor = HookExecutor(tmp_path)
|
||||
# Exercise the public API: before the fix the TypeError was swallowed
|
||||
# by should_execute_hook's `except Exception: return False`, so the
|
||||
# hook was silently disabled (False); after the fix it returns True.
|
||||
assert executor.should_execute_hook(
|
||||
{"condition": "config.connection.url is set", "extension": "testext"}
|
||||
) is True
|
||||
|
||||
def test_malformed_env_names_ignored(self, tmp_path, monkeypatch):
|
||||
"""A name with no key (SPECKIT_X_) or empty parts (consecutive
|
||||
underscores) must not create an entry under an empty key."""
|
||||
monkeypatch.setenv("SPECKIT_TESTEXT_", "orphan") # no key at all
|
||||
monkeypatch.setenv("SPECKIT_TESTEXT_A__B", "z") # empty middle part
|
||||
cm = ConfigManager(tmp_path, "testext")
|
||||
cfg = cm._get_env_config()
|
||||
assert "" not in cfg
|
||||
assert cfg == {"a": {"b": "z"}}
|
||||
|
||||
@@ -2620,7 +2620,7 @@ class TestSelfTestPreset:
|
||||
assert manifest.id == "self-test"
|
||||
assert manifest.name == "Self-Test Preset"
|
||||
assert manifest.version == "1.0.0"
|
||||
assert len(manifest.templates) == 7 # 5 templates + 2 commands
|
||||
assert len(manifest.templates) == 8 # 6 templates + 2 commands
|
||||
|
||||
def test_self_test_provides_all_core_templates(self):
|
||||
"""Verify the self-test preset provides an override for every core template."""
|
||||
@@ -3663,8 +3663,6 @@ class TestPresetSkills:
|
||||
|
||||
extension_dir = project_dir / ".specify" / "extensions" / "fakeext"
|
||||
(extension_dir / "commands").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control" / "commander.md").write_text("# Commander\n")
|
||||
(extension_dir / "commands" / "cmd.md").write_text(
|
||||
"---\n"
|
||||
"description: Extension fakeext cmd\n"
|
||||
@@ -3673,7 +3671,6 @@ class TestPresetSkills:
|
||||
"---\n\n"
|
||||
"extension:fakeext\n"
|
||||
"Run {SCRIPT}\n"
|
||||
"Read agents/control/commander.md for context.\n"
|
||||
)
|
||||
extension_manifest = {
|
||||
"schema_version": "1.0",
|
||||
@@ -3739,92 +3736,8 @@ class TestPresetSkills:
|
||||
assert "source: extension:fakeext" in content
|
||||
assert "extension:fakeext" in content
|
||||
assert '.specify/scripts/bash/setup-plan.sh --json "$ARGUMENTS"' in content
|
||||
# Extension-relative subdir references must resolve to their
|
||||
# installed location on restore too (#2101), not just on first
|
||||
# registration.
|
||||
assert ".specify/extensions/fakeext/agents/control/commander.md" in content
|
||||
assert "Read agents/control" not in content
|
||||
assert "# Fakeext Cmd Skill" in content
|
||||
|
||||
def test_skill_composed_over_extension_base_rewrites_subdir_paths(
|
||||
self, project_dir, temp_dir
|
||||
):
|
||||
"""When a preset composes (append) over an extension-provided base
|
||||
command, the resulting skill (read from the .composed output) must
|
||||
still resolve the extension's own subdir references (#2101), not
|
||||
just when the extension wins outright (replace)."""
|
||||
self._write_init_options(project_dir, ai="codex")
|
||||
skills_dir = project_dir / ".agents" / "skills"
|
||||
self._create_skill(skills_dir, "speckit-fakeext-cmd", body="original extension skill")
|
||||
|
||||
extension_dir = project_dir / ".specify" / "extensions" / "fakeext"
|
||||
(extension_dir / "commands").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control" / "commander.md").write_text("# Commander\n")
|
||||
(extension_dir / "commands" / "cmd.md").write_text(
|
||||
"---\ndescription: Extension fakeext cmd\n---\n\n"
|
||||
"Read agents/control/commander.md for context.\n"
|
||||
)
|
||||
extension_manifest = {
|
||||
"schema_version": "1.0",
|
||||
"extension": {
|
||||
"id": "fakeext",
|
||||
"name": "Fake Extension",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"commands": [
|
||||
{
|
||||
"name": "speckit.fakeext.cmd",
|
||||
"file": "commands/cmd.md",
|
||||
"description": "Fake extension command",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(extension_dir / "extension.yml", "w") as f:
|
||||
yaml.dump(extension_manifest, f)
|
||||
|
||||
preset_dir = temp_dir / "ext-base-append-skill"
|
||||
preset_dir.mkdir()
|
||||
(preset_dir / "commands").mkdir()
|
||||
(preset_dir / "commands" / "speckit.fakeext.cmd.md").write_text(
|
||||
"---\ndescription: Preset overlay\n---\n\n## Extra\n"
|
||||
)
|
||||
preset_manifest = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "ext-base-append-skill",
|
||||
"name": "Ext Base Append Skill",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.fakeext.cmd",
|
||||
"file": "commands/speckit.fakeext.cmd.md",
|
||||
"strategy": "append",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(preset_dir / "preset.yml", "w") as f:
|
||||
yaml.dump(preset_manifest, f)
|
||||
|
||||
manager = PresetManager(project_dir)
|
||||
manager.install_from_directory(preset_dir, "0.1.5")
|
||||
|
||||
skill_file = skills_dir / "speckit-fakeext-cmd" / "SKILL.md"
|
||||
content = skill_file.read_text()
|
||||
assert ".specify/extensions/fakeext/agents/control/commander.md" in content
|
||||
assert "Read agents/control" not in content
|
||||
assert "## Extra" in content
|
||||
|
||||
def test_preset_remove_skips_skill_dir_without_skill_file(self, project_dir, temp_dir):
|
||||
"""Preset removal should not delete arbitrary directories missing SKILL.md."""
|
||||
self._write_init_options(project_dir, ai="codex")
|
||||
@@ -4147,40 +4060,6 @@ class TestPresetSetPriority:
|
||||
plain = strip_ansi(result.output)
|
||||
assert "already has priority 5" in plain
|
||||
|
||||
def test_set_priority_repairs_corrupted_bool(self, project_dir, pack_dir):
|
||||
"""A corrupted boolean priority must be repaired, not skipped.
|
||||
|
||||
``isinstance(True, int)`` is True and ``True == 1`` in Python, so a
|
||||
stored ``True`` priority would short-circuit the ``already has
|
||||
priority 1`` skip path and never get rewritten to a real int —
|
||||
contradicting the comment that promises corrupted values are
|
||||
repaired. The guard must exclude bools (like normalize_priority).
|
||||
"""
|
||||
from typer.testing import CliRunner
|
||||
from unittest.mock import patch
|
||||
from specify_cli import app
|
||||
|
||||
runner = CliRunner()
|
||||
|
||||
manager = PresetManager(project_dir)
|
||||
manager.install_from_directory(pack_dir, "0.1.5", priority=5)
|
||||
# Inject a corrupted boolean priority (True == 1).
|
||||
manager.registry.update("test-pack", {"priority": True})
|
||||
|
||||
with patch.object(Path, "cwd", return_value=project_dir):
|
||||
result = runner.invoke(app, ["preset", "set-priority", "test-pack", "1"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
plain = strip_ansi(result.output)
|
||||
# The corrupted bool must be repaired, not reported as already-set.
|
||||
assert "already has priority" not in plain
|
||||
assert "priority changed" in plain
|
||||
|
||||
# The stored value is now a real int, not a bool.
|
||||
reloaded = PresetManager(project_dir).registry.get("test-pack")
|
||||
assert reloaded["priority"] == 1
|
||||
assert not isinstance(reloaded["priority"], bool)
|
||||
|
||||
def test_set_priority_invalid_value(self, project_dir, pack_dir):
|
||||
"""Test set-priority rejects invalid priority values."""
|
||||
from typer.testing import CliRunner
|
||||
@@ -6059,86 +5938,6 @@ class TestResolveContent:
|
||||
content = resolver.resolve_content("spec-template")
|
||||
assert content == "# Replaced content\n"
|
||||
|
||||
@pytest.mark.parametrize("strategy", ["append", "prepend", "wrap"])
|
||||
def test_resolve_content_rewrites_extension_base_subdir_paths(
|
||||
self, project_dir, temp_dir, strategy
|
||||
):
|
||||
"""Composing over an extension-provided base command must resolve the
|
||||
extension's own subdir references (agents/, knowledge-base/) to their
|
||||
installed location (#2101), not just when the extension wins outright.
|
||||
"""
|
||||
extension_dir = project_dir / ".specify" / "extensions" / "fakeext"
|
||||
(extension_dir / "commands").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control" / "commander.md").write_text("# Commander\n")
|
||||
(extension_dir / "commands" / "cmd.md").write_text(
|
||||
"---\ndescription: Extension fakeext cmd\n---\n\n"
|
||||
"Read agents/control/commander.md for context.\n"
|
||||
)
|
||||
extension_manifest = {
|
||||
"schema_version": "1.0",
|
||||
"extension": {
|
||||
"id": "fakeext",
|
||||
"name": "Fake Extension",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"commands": [
|
||||
{
|
||||
"name": "speckit.fakeext.cmd",
|
||||
"file": "commands/cmd.md",
|
||||
"description": "Fake extension command",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(extension_dir / "extension.yml", "w") as f:
|
||||
yaml.dump(extension_manifest, f)
|
||||
|
||||
preset_dir = temp_dir / f"ext-base-{strategy}"
|
||||
preset_dir.mkdir()
|
||||
(preset_dir / "commands").mkdir()
|
||||
overlay_body = (
|
||||
"{CORE_TEMPLATE}\n## Extra\n" if strategy == "wrap" else "## Extra\n"
|
||||
)
|
||||
(preset_dir / "commands" / "speckit.fakeext.cmd.md").write_text(
|
||||
f"---\ndescription: Preset overlay\n---\n\n{overlay_body}"
|
||||
)
|
||||
preset_manifest = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": f"ext-base-{strategy}",
|
||||
"name": "Ext Base",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.fakeext.cmd",
|
||||
"file": "commands/speckit.fakeext.cmd.md",
|
||||
"strategy": strategy,
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(preset_dir / "preset.yml", "w") as f:
|
||||
yaml.dump(preset_manifest, f)
|
||||
|
||||
manager = PresetManager(project_dir)
|
||||
manager.install_from_directory(preset_dir, "0.1.5")
|
||||
|
||||
resolver = PresetResolver(project_dir)
|
||||
content = resolver.resolve_content("speckit.fakeext.cmd", "command")
|
||||
assert content is not None
|
||||
assert ".specify/extensions/fakeext/agents/control/commander.md" in content
|
||||
assert "Read agents/control" not in content
|
||||
assert "## Extra" in content
|
||||
|
||||
|
||||
class TestCollectAllLayers:
|
||||
"""Test PresetResolver.collect_all_layers() method."""
|
||||
@@ -6219,172 +6018,6 @@ class TestCollectAllLayers:
|
||||
class TestRemoveReconciliation:
|
||||
"""Test that removing a preset re-registers the next layer's command."""
|
||||
|
||||
def test_remove_restores_extension_command_subdir_paths_for_non_skill_agent(
|
||||
self, project_dir, temp_dir
|
||||
):
|
||||
"""When a preset override of an extension command is removed, the
|
||||
reconciled non-skill-agent command file should have the extension's
|
||||
own subdir references rewritten to their installed location (#2101),
|
||||
not left as bare, unresolvable paths."""
|
||||
gemini_dir = project_dir / ".gemini" / "commands"
|
||||
gemini_dir.mkdir(parents=True)
|
||||
|
||||
extension_dir = project_dir / ".specify" / "extensions" / "fakeext"
|
||||
(extension_dir / "commands").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control" / "commander.md").write_text("# Commander\n")
|
||||
(extension_dir / "commands" / "cmd.md").write_text(
|
||||
"---\ndescription: Extension fakeext cmd\n---\n\n"
|
||||
"Read agents/control/commander.md for context.\n"
|
||||
)
|
||||
extension_manifest = {
|
||||
"schema_version": "1.0",
|
||||
"extension": {
|
||||
"id": "fakeext",
|
||||
"name": "Fake Extension",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"commands": [
|
||||
{
|
||||
"name": "speckit.fakeext.cmd",
|
||||
"file": "commands/cmd.md",
|
||||
"description": "Fake extension command",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(extension_dir / "extension.yml", "w") as f:
|
||||
yaml.dump(extension_manifest, f)
|
||||
|
||||
manager = PresetManager(project_dir)
|
||||
|
||||
preset_dir = temp_dir / "ext-cmd-override"
|
||||
preset_dir.mkdir()
|
||||
(preset_dir / "commands").mkdir()
|
||||
(preset_dir / "commands" / "speckit.fakeext.cmd.md").write_text(
|
||||
"---\ndescription: Override fakeext cmd\n---\n\npreset override content\n"
|
||||
)
|
||||
preset_manifest = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "ext-cmd-override",
|
||||
"name": "Ext Cmd Override",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.fakeext.cmd",
|
||||
"file": "commands/speckit.fakeext.cmd.md",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(preset_dir / "preset.yml", "w") as f:
|
||||
yaml.dump(preset_manifest, f)
|
||||
|
||||
manager.install_from_directory(preset_dir, "0.1.5")
|
||||
|
||||
cmd_files = list(gemini_dir.glob("*fakeext*"))
|
||||
assert cmd_files, "Command file should exist in gemini dir"
|
||||
assert "preset override content" in cmd_files[0].read_text()
|
||||
|
||||
manager.remove("ext-cmd-override")
|
||||
|
||||
cmd_files = list(gemini_dir.glob("*fakeext*"))
|
||||
assert cmd_files, "Command file should still exist after removal"
|
||||
content = cmd_files[0].read_text()
|
||||
assert "preset override content" not in content
|
||||
assert ".specify/extensions/fakeext/agents/control/commander.md" in content
|
||||
assert "Read agents/control" not in content
|
||||
|
||||
def test_install_composes_extension_command_and_rewrites_subdir_paths_for_non_skill_agent(
|
||||
self, project_dir, temp_dir
|
||||
):
|
||||
"""When a preset overlays (append) an extension-provided base command,
|
||||
the initial composed non-skill-agent command file must have the
|
||||
extension's own subdir references rewritten to their installed
|
||||
location (#2101), matching the live repro: extension body
|
||||
'Read agents/control/commander.md', preset appends to
|
||||
speckit.fakeext.cmd, generated Gemini content retains the bare path."""
|
||||
gemini_dir = project_dir / ".gemini" / "commands"
|
||||
gemini_dir.mkdir(parents=True)
|
||||
|
||||
extension_dir = project_dir / ".specify" / "extensions" / "fakeext"
|
||||
(extension_dir / "commands").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control").mkdir(parents=True, exist_ok=True)
|
||||
(extension_dir / "agents" / "control" / "commander.md").write_text("# Commander\n")
|
||||
(extension_dir / "commands" / "cmd.md").write_text(
|
||||
"---\ndescription: Extension fakeext cmd\n---\n\n"
|
||||
"Read agents/control/commander.md for context.\n"
|
||||
)
|
||||
extension_manifest = {
|
||||
"schema_version": "1.0",
|
||||
"extension": {
|
||||
"id": "fakeext",
|
||||
"name": "Fake Extension",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"commands": [
|
||||
{
|
||||
"name": "speckit.fakeext.cmd",
|
||||
"file": "commands/cmd.md",
|
||||
"description": "Fake extension command",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(extension_dir / "extension.yml", "w") as f:
|
||||
yaml.dump(extension_manifest, f)
|
||||
|
||||
preset_dir = temp_dir / "ext-cmd-append"
|
||||
preset_dir.mkdir()
|
||||
(preset_dir / "commands").mkdir()
|
||||
(preset_dir / "commands" / "speckit.fakeext.cmd.md").write_text(
|
||||
"---\ndescription: Append fakeext cmd\n---\n\n## Extra\n"
|
||||
)
|
||||
preset_manifest = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "ext-cmd-append",
|
||||
"name": "Ext Cmd Append",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.fakeext.cmd",
|
||||
"file": "commands/speckit.fakeext.cmd.md",
|
||||
"strategy": "append",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(preset_dir / "preset.yml", "w") as f:
|
||||
yaml.dump(preset_manifest, f)
|
||||
|
||||
manager = PresetManager(project_dir)
|
||||
manager.install_from_directory(preset_dir, "0.1.5")
|
||||
|
||||
cmd_files = list(gemini_dir.glob("*fakeext*"))
|
||||
assert cmd_files, "Command file should exist in gemini dir"
|
||||
content = cmd_files[0].read_text()
|
||||
assert ".specify/extensions/fakeext/agents/control/commander.md" in content
|
||||
assert "Read agents/control" not in content
|
||||
assert "## Extra" in content
|
||||
|
||||
def test_remove_restores_lower_priority_command(
|
||||
self, project_dir, temp_dir, valid_pack_data
|
||||
):
|
||||
|
||||
@@ -460,34 +460,6 @@ class TestExpressions:
|
||||
assert evaluate_expression("{{ inputs.s | contains('ab') }}", ctx2) is True
|
||||
assert evaluate_expression("{{ inputs.missing | default('a|b') }}", ctx2) == "a|b"
|
||||
|
||||
def test_membership_against_non_iterable_is_false_not_error(self):
|
||||
from specify_cli.workflows.expressions import (
|
||||
evaluate_condition,
|
||||
evaluate_expression,
|
||||
)
|
||||
from specify_cli.workflows.base import StepContext
|
||||
|
||||
# A non-iterable right operand (int, bool, None, float) makes a raw
|
||||
# `x in y` raise TypeError in Python. The evaluator must treat it as
|
||||
# "not contained" (False, and `not in` as True) instead of leaking the
|
||||
# TypeError and crashing the whole workflow run. This generalizes the
|
||||
# previous `right is not None` guard and mirrors _safe_compare, which
|
||||
# already swallows TypeError for the ordering operators.
|
||||
ctx = StepContext(inputs={"tag": "x", "count": 5, "ratio": 1.5, "flag": True})
|
||||
assert evaluate_expression("{{ inputs.tag in inputs.count }}", ctx) is False
|
||||
assert evaluate_expression("{{ inputs.tag not in inputs.count }}", ctx) is True
|
||||
assert evaluate_expression("{{ 'a' in inputs.ratio }}", ctx) is False
|
||||
assert evaluate_expression("{{ 'a' in inputs.flag }}", ctx) is False
|
||||
assert evaluate_expression("{{ inputs.tag in inputs.missing }}", ctx) is False
|
||||
# A condition that would otherwise crash the run now evaluates cleanly.
|
||||
assert evaluate_condition("{{ inputs.tag in inputs.count }}", ctx) is False
|
||||
|
||||
# Regression: genuine membership over a real iterable still works.
|
||||
ok = StepContext(inputs={"items": ["x", "y"], "s": "xyz"})
|
||||
assert evaluate_expression("{{ 'x' in inputs.items }}", ok) is True
|
||||
assert evaluate_expression("{{ 'z' not in inputs.items }}", ok) is True
|
||||
assert evaluate_expression("{{ 'y' in inputs.s }}", ok) is True
|
||||
|
||||
def test_filter_default(self):
|
||||
from specify_cli.workflows.expressions import evaluate_expression
|
||||
from specify_cli.workflows.base import StepContext
|
||||
@@ -1381,6 +1353,107 @@ class TestShellStep:
|
||||
assert step.validate({"id": "s", "run": "echo hi"}) == []
|
||||
assert step.validate({"id": "s", "run": "{{ steps.x.output }}"}) == []
|
||||
|
||||
def test_timeout_is_configurable(self, monkeypatch):
|
||||
"""A 'timeout' field overrides the 300s default (#3327)."""
|
||||
import subprocess as sp
|
||||
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
seen = {}
|
||||
real_run = sp.run
|
||||
|
||||
def spy_run(*args, **kwargs):
|
||||
seen["timeout"] = kwargs.get("timeout")
|
||||
return real_run(*args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"specify_cli.workflows.steps.shell.subprocess.run", spy_run
|
||||
)
|
||||
step = ShellStep()
|
||||
result = step.execute(
|
||||
{"id": "t", "run": "echo hi", "timeout": 1800}, StepContext()
|
||||
)
|
||||
assert result.status == StepStatus.COMPLETED
|
||||
assert seen["timeout"] == 1800
|
||||
|
||||
def test_timeout_defaults_to_300(self, monkeypatch):
|
||||
import subprocess as sp
|
||||
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
seen = {}
|
||||
real_run = sp.run
|
||||
|
||||
def spy_run(*args, **kwargs):
|
||||
seen["timeout"] = kwargs.get("timeout")
|
||||
return real_run(*args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"specify_cli.workflows.steps.shell.subprocess.run", spy_run
|
||||
)
|
||||
result = ShellStep().execute({"id": "t", "run": "echo hi"}, StepContext())
|
||||
assert result.status == StepStatus.COMPLETED
|
||||
assert seen["timeout"] == 300
|
||||
|
||||
def test_timeout_error_reports_configured_value(self, monkeypatch):
|
||||
import subprocess as sp
|
||||
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
def raise_timeout(*args, **kwargs):
|
||||
raise sp.TimeoutExpired(cmd="x", timeout=kwargs.get("timeout"))
|
||||
|
||||
monkeypatch.setattr(
|
||||
"specify_cli.workflows.steps.shell.subprocess.run", raise_timeout
|
||||
)
|
||||
result = ShellStep().execute(
|
||||
{"id": "t", "run": "sleep 999", "timeout": 7}, StepContext()
|
||||
)
|
||||
assert result.status == StepStatus.FAILED
|
||||
assert "7 seconds" in result.error
|
||||
|
||||
@pytest.mark.parametrize("bad", [0, -5, "600", 1.5, None, True])
|
||||
def test_execute_ignores_unvalidated_bad_timeout(self, bad, monkeypatch):
|
||||
"""execute() falls back to 300 when config skipped validation (#3327)."""
|
||||
import subprocess as sp
|
||||
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
seen = {}
|
||||
real_run = sp.run
|
||||
|
||||
def spy_run(*args, **kwargs):
|
||||
seen["timeout"] = kwargs.get("timeout")
|
||||
return real_run(*args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"specify_cli.workflows.steps.shell.subprocess.run", spy_run
|
||||
)
|
||||
result = ShellStep().execute(
|
||||
{"id": "t", "run": "echo hi", "timeout": bad}, StepContext()
|
||||
)
|
||||
assert result.status == StepStatus.COMPLETED
|
||||
assert seen["timeout"] == 300
|
||||
|
||||
@pytest.mark.parametrize("bad", [0, -5, "600", 1.5, None, True])
|
||||
def test_validate_rejects_bad_timeout(self, bad):
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
|
||||
errors = ShellStep().validate({"id": "s", "run": "echo hi", "timeout": bad})
|
||||
assert any("'timeout'" in e for e in errors)
|
||||
|
||||
def test_validate_accepts_positive_int_timeout(self):
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
|
||||
assert (
|
||||
ShellStep().validate({"id": "s", "run": "echo hi", "timeout": 1800}) == []
|
||||
)
|
||||
|
||||
|
||||
def test_output_format_json_exposes_data(self, tmp_path):
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
@@ -1437,130 +1510,6 @@ class TestShellStep:
|
||||
errors = step.validate({"id": "emit", "run": "exit 0", "output_format": "yaml"})
|
||||
assert any("'output_format' must be 'json'" in e for e in errors)
|
||||
|
||||
def test_configured_timeout_is_passed_to_subprocess(self, monkeypatch):
|
||||
"""A ``timeout:`` value on the step overrides the 300s default and is
|
||||
threaded through to ``subprocess.run`` (issue #3327)."""
|
||||
import subprocess
|
||||
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def fake_run(*args, **kwargs):
|
||||
captured["timeout"] = kwargs.get("timeout")
|
||||
return subprocess.CompletedProcess(
|
||||
args=args[0] if args else "", returncode=0, stdout="", stderr=""
|
||||
)
|
||||
|
||||
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||
step = ShellStep()
|
||||
result = step.execute(
|
||||
{"id": "qa", "run": "echo hi", "timeout": 1800}, StepContext()
|
||||
)
|
||||
assert result.status == StepStatus.COMPLETED
|
||||
assert captured["timeout"] == 1800
|
||||
|
||||
def test_default_timeout_preserved_when_omitted(self, monkeypatch):
|
||||
"""Omitting ``timeout:`` preserves the historical 300s default."""
|
||||
import subprocess
|
||||
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def fake_run(*args, **kwargs):
|
||||
captured["timeout"] = kwargs.get("timeout")
|
||||
return subprocess.CompletedProcess(
|
||||
args=args[0] if args else "", returncode=0, stdout="", stderr=""
|
||||
)
|
||||
|
||||
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||
step = ShellStep()
|
||||
step.execute({"id": "qa", "run": "echo hi"}, StepContext())
|
||||
assert captured["timeout"] == 300
|
||||
|
||||
def test_timeout_error_reports_configured_value(self, monkeypatch):
|
||||
"""The timeout failure message reflects the configured duration, not a
|
||||
hardcoded 300."""
|
||||
import subprocess
|
||||
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
def fake_run(*args, **kwargs):
|
||||
raise subprocess.TimeoutExpired(cmd="echo hi", timeout=5)
|
||||
|
||||
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||
step = ShellStep()
|
||||
result = step.execute(
|
||||
{"id": "qa", "run": "echo hi", "timeout": 5}, StepContext()
|
||||
)
|
||||
assert result.status == StepStatus.FAILED
|
||||
assert "5 seconds" in (result.error or "")
|
||||
|
||||
def test_execute_fails_cleanly_on_invalid_timeout(self, monkeypatch):
|
||||
"""execute() must fail the step (not raise) on an invalid timeout even
|
||||
when validate() was skipped — the engine does not auto-validate step
|
||||
config, so an unvalidated string/bool/non-finite timeout would
|
||||
otherwise crash subprocess.run() and take down the whole run."""
|
||||
import subprocess
|
||||
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
def fail_if_called(*args, **kwargs):
|
||||
raise AssertionError("subprocess.run should not run on invalid timeout")
|
||||
|
||||
monkeypatch.setattr(subprocess, "run", fail_if_called)
|
||||
step = ShellStep()
|
||||
# A string would raise TypeError; ``True`` would silently become a 1s
|
||||
# timeout (bool is an int subclass); ``.inf`` would raise at runtime.
|
||||
for bad in ("30", True, float("inf"), 0):
|
||||
result = step.execute(
|
||||
{"id": "qa", "run": "echo hi", "timeout": bad}, StepContext()
|
||||
)
|
||||
assert result.status == StepStatus.FAILED
|
||||
assert "'timeout' must be a positive number" in (result.error or "")
|
||||
|
||||
def test_validate_rejects_non_positive_timeout(self):
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
|
||||
step = ShellStep()
|
||||
for bad in (0, -30):
|
||||
errors = step.validate({"id": "qa", "run": "echo hi", "timeout": bad})
|
||||
assert any("'timeout' must be a positive number" in e for e in errors)
|
||||
|
||||
def test_validate_rejects_non_numeric_timeout(self):
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
|
||||
step = ShellStep()
|
||||
# A string and a bool are both invalid (bool is an int subclass but a
|
||||
# config error, not a duration).
|
||||
for bad in ("30", True):
|
||||
errors = step.validate({"id": "qa", "run": "echo hi", "timeout": bad})
|
||||
assert any("'timeout' must be a positive number" in e for e in errors)
|
||||
|
||||
def test_validate_rejects_non_finite_timeout(self):
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
|
||||
step = ShellStep()
|
||||
# inf/nan are floats and slip past a plain ``> 0`` check (``nan <= 0``
|
||||
# is False), but ``subprocess.run(timeout=...)`` would then fail at
|
||||
# runtime. YAML ``.inf``/``.nan`` scalars parse to these via safe_load.
|
||||
for bad in (float("inf"), float("-inf"), float("nan")):
|
||||
errors = step.validate({"id": "qa", "run": "echo hi", "timeout": bad})
|
||||
assert any("'timeout' must be a positive number" in e for e in errors)
|
||||
|
||||
def test_validate_accepts_positive_numeric_timeout(self):
|
||||
from specify_cli.workflows.steps.shell import ShellStep
|
||||
|
||||
step = ShellStep()
|
||||
for good in (1, 300, 1800, 12.5):
|
||||
errors = step.validate({"id": "qa", "run": "echo hi", "timeout": good})
|
||||
assert not any("'timeout'" in e for e in errors)
|
||||
|
||||
class _StubStdin:
|
||||
"""Stdin stub exposing only a fixed ``isatty`` result.
|
||||
|
||||
@@ -2039,46 +1988,6 @@ class TestIfThenStep:
|
||||
errors = step.validate({"id": "test", "then": []})
|
||||
assert any("missing 'condition'" in e for e in errors)
|
||||
|
||||
@pytest.mark.parametrize("bad_else", [False, 0, "", {}, 42])
|
||||
def test_validate_rejects_non_list_else(self, bad_else):
|
||||
"""A non-list 'else' must be rejected even when it is falsy.
|
||||
|
||||
The original guard used ``if else_branch and ...`` which
|
||||
short-circuits for falsy non-list values (False/0/''/{}), letting a
|
||||
malformed else-branch pass validation only to be silently skipped at
|
||||
runtime. ``then`` is already strictly validated; ``else`` must match.
|
||||
"""
|
||||
from specify_cli.workflows.steps.if_then import IfThenStep
|
||||
|
||||
step = IfThenStep()
|
||||
errors = step.validate(
|
||||
{"id": "i", "condition": "true", "then": [], "else": bad_else}
|
||||
)
|
||||
assert any("'else' must be a list of steps" in e for e in errors)
|
||||
|
||||
@pytest.mark.parametrize("ok_else", [None, [], [{"id": "x", "command": "/y"}]])
|
||||
def test_validate_accepts_valid_else(self, ok_else):
|
||||
"""An explicit 'else' of None or a list stays valid.
|
||||
|
||||
``else`` is set explicitly here (including ``else: None``) so the
|
||||
explicit-None case is exercised, not just the missing-key case.
|
||||
"""
|
||||
from specify_cli.workflows.steps.if_then import IfThenStep
|
||||
|
||||
step = IfThenStep()
|
||||
errors = step.validate(
|
||||
{"id": "i", "condition": "true", "then": [], "else": ok_else}
|
||||
)
|
||||
assert not any("'else'" in e for e in errors)
|
||||
|
||||
def test_validate_accepts_missing_else(self):
|
||||
"""A missing 'else' key stays valid (no else branch)."""
|
||||
from specify_cli.workflows.steps.if_then import IfThenStep
|
||||
|
||||
step = IfThenStep()
|
||||
errors = step.validate({"id": "i", "condition": "true", "then": []})
|
||||
assert not any("'else'" in e for e in errors)
|
||||
|
||||
|
||||
class TestSwitchStep:
|
||||
"""Test the switch step type."""
|
||||
@@ -2143,35 +2052,6 @@ class TestSwitchStep:
|
||||
assert result.output["matched_case"] == "__default__"
|
||||
assert result.next_steps == []
|
||||
|
||||
def test_execute_non_dict_cases_fails_loudly(self):
|
||||
"""A non-mapping ``cases`` must fail the step, not crash the run.
|
||||
|
||||
``validate`` rejects a non-dict ``cases``, but the engine's
|
||||
``execute()`` does not auto-validate (see ``WorkflowEngine.load_workflow``
|
||||
docstring). Before the guard, ``execute`` called ``cases.items()`` on the
|
||||
raw value, so an unvalidated run with a list/scalar ``cases`` raised
|
||||
AttributeError and took down the whole run instead of failing this step.
|
||||
Mirrors the fan-out step's non-list ``items`` handling.
|
||||
"""
|
||||
from specify_cli.workflows.steps.switch import SwitchStep
|
||||
from specify_cli.workflows.base import StepContext, StepStatus
|
||||
|
||||
step = SwitchStep()
|
||||
ctx = StepContext(steps={"review": {"output": {"choice": "approve"}}})
|
||||
for bad_cases in (["approve"], "approve", 5):
|
||||
result = step.execute(
|
||||
{
|
||||
"id": "route",
|
||||
"expression": "{{ steps.review.output.choice }}",
|
||||
"cases": bad_cases,
|
||||
},
|
||||
ctx,
|
||||
)
|
||||
assert result.status == StepStatus.FAILED
|
||||
assert "'cases' must be a mapping" in (result.error or "")
|
||||
# expression is still evaluated, so its value is surfaced for context.
|
||||
assert result.output["expression_value"] == "approve"
|
||||
|
||||
def test_validate_missing_expression(self):
|
||||
from specify_cli.workflows.steps.switch import SwitchStep
|
||||
|
||||
@@ -3992,56 +3872,6 @@ steps:
|
||||
assert "retry-loop:tick:1" in state.step_results
|
||||
assert "retry-loop:tick:2" in state.step_results
|
||||
|
||||
def test_loop_with_bool_max_iterations_uses_default_cap(self, project_dir):
|
||||
"""A boolean max_iterations must fall back to the default cap of 10,
|
||||
not be treated as the int 1 (bool-is-int trap).
|
||||
|
||||
``max_iterations: true`` would otherwise slip past the int check
|
||||
(``isinstance(True, int)`` is True and ``True < 1`` is False) and
|
||||
cap the loop at ``range(True - 1) == range(0)`` — a single
|
||||
iteration. ``execute()`` does not auto-validate, so the engine's own
|
||||
guard is the only line of defence here.
|
||||
"""
|
||||
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition
|
||||
from specify_cli.workflows.base import RunStatus
|
||||
|
||||
import sys
|
||||
|
||||
counter_file = project_dir / ".counter"
|
||||
counter_file.write_text("0", encoding="utf-8")
|
||||
py = sys.executable
|
||||
script_file = project_dir / "_tick.py"
|
||||
script_file.write_text(
|
||||
f"import pathlib; p = pathlib.Path(r'{counter_file}')\n"
|
||||
"n = int(p.read_text()) + 1; p.write_text(str(n))\n"
|
||||
"print('pending', end='')\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
yaml_str = f"""
|
||||
schema_version: "1.0"
|
||||
workflow:
|
||||
id: "while-bool-max-iterations"
|
||||
name: "While Bool Max Iterations"
|
||||
version: "1.0.0"
|
||||
steps:
|
||||
- id: retry-loop
|
||||
type: while
|
||||
condition: "{{{{ 'done' not in steps.tick.output.stdout }}}}"
|
||||
max_iterations: true
|
||||
steps:
|
||||
- id: tick
|
||||
type: shell
|
||||
run: '"{py}" "{script_file}"'
|
||||
"""
|
||||
definition = WorkflowDefinition.from_string(yaml_str)
|
||||
engine = WorkflowEngine(project_dir)
|
||||
state = engine.execute(definition)
|
||||
|
||||
assert state.status == RunStatus.COMPLETED
|
||||
# Falls back to the default cap of 10, not range(True - 1) == 1 run.
|
||||
assert counter_file.read_text(encoding="utf-8").strip() == "10"
|
||||
|
||||
def test_do_while_loop_runs_to_max_when_condition_stays_true(self, project_dir):
|
||||
"""Do-while loop must still run to max_iterations when the condition
|
||||
never becomes false.
|
||||
@@ -4916,59 +4746,12 @@ class TestWorkflowRegistry:
|
||||
registry2 = WorkflowRegistry(project_dir)
|
||||
assert registry2.is_installed("test-wf")
|
||||
|
||||
@pytest.mark.parametrize("bad_content", ["[]", '{"schema_version": "1.0"}'])
|
||||
def test_load_tolerates_misshaped_registry(self, project_dir, bad_content):
|
||||
"""A JSON-valid but mis-shaped registry file must not crash every method.
|
||||
|
||||
A list root, or a dict lacking a 'workflows' mapping, previously made
|
||||
is_installed/get/list/remove/add raise TypeError/KeyError. Mirrors the
|
||||
shape guard StepRegistry._load already has.
|
||||
"""
|
||||
from specify_cli.workflows.catalog import WorkflowRegistry
|
||||
|
||||
reg_path = project_dir / ".specify" / "workflows" / "workflow-registry.json"
|
||||
reg_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
reg_path.write_text(bad_content, encoding="utf-8")
|
||||
|
||||
registry = WorkflowRegistry(project_dir)
|
||||
assert registry.data == {
|
||||
"schema_version": WorkflowRegistry.SCHEMA_VERSION,
|
||||
"workflows": {},
|
||||
}
|
||||
# None of these should raise on the recovered-default shape.
|
||||
assert registry.is_installed("x") is False
|
||||
assert registry.get("x") is None
|
||||
assert registry.list() == {} # list() always returns a dict
|
||||
registry.remove("x")
|
||||
registry.add("x", {"name": "X"})
|
||||
assert registry.is_installed("x")
|
||||
|
||||
|
||||
# ===== Workflow Catalog Tests =====
|
||||
|
||||
class TestWorkflowCatalog:
|
||||
"""Test WorkflowCatalog catalog resolution."""
|
||||
|
||||
def test_search_with_non_string_fields(self, project_dir, monkeypatch):
|
||||
"""Non-string workflow fields (null/int name/description) must not
|
||||
raise TypeError in search — StepCatalog.search already coerces these."""
|
||||
from specify_cli.workflows.catalog import WorkflowCatalog
|
||||
|
||||
catalog = WorkflowCatalog(project_dir)
|
||||
monkeypatch.setattr(catalog, "_get_merged_workflows", lambda **kw: {
|
||||
"42": {
|
||||
"id": 42,
|
||||
"name": None,
|
||||
"description": 99,
|
||||
"_catalog_name": "test",
|
||||
"_install_allowed": True,
|
||||
},
|
||||
})
|
||||
|
||||
assert len(catalog.search()) == 1
|
||||
assert len(catalog.search(query="42")) == 1
|
||||
assert len(catalog.search(query="missing")) == 0
|
||||
|
||||
def test_default_catalogs(self, project_dir, monkeypatch):
|
||||
from specify_cli.workflows.catalog import WorkflowCatalog
|
||||
|
||||
|
||||
@@ -112,14 +112,8 @@ Run a shell command and capture output:
|
||||
- id: run-tests
|
||||
type: shell
|
||||
run: "cd {{ inputs.project_dir }} && npm test"
|
||||
timeout: 1800 # Optional: max seconds before the command is killed (default 300)
|
||||
```
|
||||
|
||||
`timeout` is the maximum time in seconds the command may run before it is
|
||||
killed and the step fails; it must be a positive number and defaults to
|
||||
`300` (five minutes) when omitted. Raise it for long-running gates such as
|
||||
full builds, linter aggregators, or integration-test targets.
|
||||
|
||||
### Init Steps
|
||||
|
||||
Bootstrap a project the same way `specify init` does — scaffolding
|
||||
|
||||
Reference in New Issue
Block a user