diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac8bb4e3..51f3109b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ +## [0.12.9] - 2026-07-09 + +### Changed + +- fix(integrations): skip Windows Store python3 alias stub in resolve_python_interpreter (#3385) +- fix(integrations): escape control characters in SKILL.md frontmatter (#3399) +- fix(workflows): apply chained expression filters left-to-right (#3339) +- fix(scripts): resolve invoke_separator by parse success, not python3 availability (#3304) (#3320) +- fix(shared-infra): refresh_shared_templates preserves recovered user files (#3378) +- fix(agents): resolve skill placeholders in Goose (yaml) command output (#3374) +- fix(bundler): enforce version pin on bundled preset/extension installs (#3377) +- Update Golden Demo extension to v0.3.0 (#3394) +- test: isolate integration test home (#3144) +- chore: release 0.12.8, begin 0.12.9.dev0 development (#3410) + +## [0.12.8] - 2026-07-08 + +### Changed + +- [extension] Add LLM Wiki extension to community catalog (#3361) +- Docs: Document missing CLI flags and integrations (#3182) +- Docs: Remove Cursor from CLI check list in README (#3184) +- feat(extensions): port update-agent-context to Python (#3387) +- fix(scripts): fall through to grep/sed when python3 is a broken stub in feature.json parser (#3312) +- fix(toml): escape control characters so generated command files parse (#3341) +- fix(cli): exit cleanly on malformed IPv6 URLs in `extension`/`preset`/`workflow add` (#3369) +- fix(github-http): return None on malformed GHES port instead of raising (#3379) +- fix(integrations): guard _sha256 against unreadable managed files (#3376) +- chore: release 0.12.7, begin 0.12.8.dev0 development (#3398) + ## [0.12.7] - 2026-07-07 ### Changed diff --git a/docs/community/extensions.md b/docs/community/extensions.md index 7ccb2b5c8..ae953b3ce 100644 --- a/docs/community/extensions.md +++ b/docs/community/extensions.md @@ -58,7 +58,7 @@ The following community-contributed extensions are available in [`catalog.commun | Fleet Orchestrator | Orchestrate a full feature lifecycle with human-in-the-loop gates across all SpecKit phases | `process` | Read+Write | [spec-kit-fleet](https://github.com/sharathsatish/spec-kit-fleet) | | GitHub Issues Integration 1 | Generate spec artifacts from GitHub Issues - import issues, sync updates, and maintain bidirectional traceability | `integration` | Read+Write | [spec-kit-github-issues](https://github.com/Fatima367/spec-kit-github-issues) | | GitHub Issues Integration 2 | Creates and syncs local specs from an existing GitHub issue | `integration` | Read+Write | [spec-kit-issue](https://github.com/aaronrsun/spec-kit-issue) | -| Golden Demo | Extracts acceptance criteria from specs, builds test vectors, and produces a behavioral drift report — complementary to Architecture Guard and CDD | `docs` | Read+Write | [spec-kit-golden-demo](https://github.com/jasstt/spec-kit-golden-demo) | +| Golden Demo | Deterministic behavioral drift oracle. Extracts acceptance criteria, generates fuzz test vectors (seed=42), compares golden Python implementations against real code in any language. CI/CD gatekeeper with warn/strict modes. | `docs` | Read+Write | [spec-kit-golden-demo](https://github.com/jasstt/spec-kit-golden-demo) | | Improve Extension | Audits any codebase as a senior advisor and writes prioritized, self-contained spec prompts under specs/ that the spec-kit lifecycle can process | `process` | Read+Write | [spec-kit-improve](https://github.com/d0whc3r/spec-kit-improve) | | Intake | Normalize PRD, design, HTML SSOT, and test-case evidence into SDD-ready intake artifacts. | `docs` | Read+Write | [spec-kit-intake](https://github.com/bigsmartben/spec-kit-intake) | | Intelligent Agent Orchestrator | Cross-catalog agent discovery and intelligent prompt-to-command routing | `process` | Read+Write | [spec-kit-orchestrator](https://github.com/pragya247/spec-kit-orchestrator) | diff --git a/extensions/catalog.community.json b/extensions/catalog.community.json index 55b226c41..d0ba50e41 100644 --- a/extensions/catalog.community.json +++ b/extensions/catalog.community.json @@ -1401,10 +1401,10 @@ "golden-demo": { "name": "Golden Demo", "id": "golden-demo", - "description": "Extracts acceptance criteria from specs, builds test vectors, and produces a behavioral drift report — complementary to Architecture Guard and CDD.", + "description": "Deterministic behavioral drift oracle. Extracts acceptance criteria, generates fuzz test vectors (seed=42), compares golden Python implementations against real code in any language. CI/CD gatekeeper with warn/strict modes.", "author": "jasstt", - "version": "0.1.1", - "download_url": "https://github.com/jasstt/spec-kit-golden-demo/archive/refs/tags/v0.1.1.zip", + "version": "0.3.0", + "download_url": "https://github.com/jasstt/spec-kit-golden-demo/archive/refs/tags/v0.3.0.zip", "repository": "https://github.com/jasstt/spec-kit-golden-demo", "homepage": "https://github.com/jasstt/spec-kit-golden-demo", "documentation": "https://github.com/jasstt/spec-kit-golden-demo", @@ -1415,13 +1415,16 @@ "speckit_version": ">=0.1.0" }, "provides": { - "commands": 2, + "commands": 3, "hooks": 2 }, "tags": [ "testing", "drift-detection", "behavioral-oracle", + "fuzzing", + "ci-cd", + "cross-language", "tdd", "quality" ], @@ -1429,7 +1432,7 @@ "downloads": 0, "stars": 0, "created_at": "2026-06-24T00:00:00Z", - "updated_at": "2026-06-24T00:00:00Z" + "updated_at": "2026-07-07T00:00:00Z" }, "harness": { "name": "Research Harness", diff --git a/pyproject.toml b/pyproject.toml index bff15afa3..7b1fd7790 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "specify-cli" -version = "0.12.8.dev0" +version = "0.12.10.dev0" 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" diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index fd2f05f82..dc60f9ff5 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -244,21 +244,29 @@ get_invoke_separator() { local integration_json="$repo_root/.specify/integration.json" local separator="." - local parsed_with_jq=0 + local parsed=0 if [[ -f "$integration_json" ]]; then + # Try parsers in order (jq -> python3 -> awk), falling through on + # failure. Selection is by *parse success*, not mere availability: on + # Windows `python3` commonly resolves to the Microsoft Store App + # Execution Alias stub, which passes `command -v` but fails at runtime + # (exit 49). An availability-gated branch would pick python3, swallow + # its failure, and — because this function historically had no text + # fallback — silently return "." even for `-`-separator integrations + # (e.g. forge, cline), yielding wrong command hints (issue #3304). if command -v jq >/dev/null 2>&1; then local jq_separator if jq_separator=$(jq -r '(.default_integration // .integration // "") as $k | if $k == "" then "." else (.integration_settings[$k].invoke_separator // ".") end' "$integration_json" 2>/dev/null); then - parsed_with_jq=1 case "$jq_separator" in - "."|"-") separator="$jq_separator" ;; + "."|"-") separator="$jq_separator"; parsed=1 ;; esac fi fi - if [[ "$parsed_with_jq" -eq 0 ]] && command -v python3 >/dev/null 2>&1; then - if separator=$(python3 - "$integration_json" <<'PY' 2>/dev/null + if [[ "$parsed" -eq 0 ]] && command -v python3 >/dev/null 2>&1; then + local py_separator + if py_separator=$(python3 - "$integration_json" <<'PY' 2>/dev/null import json import sys @@ -274,17 +282,64 @@ try: separator = entry["invoke_separator"] print(separator) except Exception: - print(".") + sys.exit(1) PY ); then - case "$separator" in - "."|"-") ;; - *) separator="." ;; + case "$py_separator" in + "."|"-") separator="$py_separator"; parsed=1 ;; esac - else - separator="." fi fi + + if [[ "$parsed" -eq 0 ]]; then + # Last-resort text fallback for environments with neither jq nor a + # working python3 (e.g. stock Windows + Git Bash). Reads the active + # integration key (default_integration, else integration) and its + # invoke_separator from within the integration_settings object. + # Handles both pretty-printed (the written form) and compact JSON. + # Accumulate all lines into one buffer in END rather than using + # gawk-only whole-file slurp (RS="^$"), so this stays portable to + # the BSD awk on macOS. + local awk_separator + awk_separator=$(awk ' + function keyval(d, name, v) { + if (match(d, "\"" name "\"[ \t\r\n]*:[ \t\r\n]*\"[^\"]*\"")) { + v=substr(d,RSTART,RLENGTH); sub(/^.*:[ \t\r\n]*"/,"",v); sub(/"$/,"",v); return v + } + return "" + } + { doc = doc $0 "\n" } + END { + key=keyval(doc,"default_integration"); if (key=="") key=keyval(doc,"integration") + sep="." + if (key!="") { + settings=doc + if (match(doc, /"integration_settings"[ \t\r\n]*:[ \t\r\n]*[{]/)) { + settings=substr(doc, RSTART+RLENGTH-1) + } + if (match(settings, "\"" key "\"[ \t\r\n]*:[ \t\r\n]*[{]")) { + start=RSTART+RLENGTH-1 + depth=0 + obj="" + for (i=start; i<=length(settings); i++) { + c=substr(settings,i,1) + obj=obj c + if (c=="{") depth++ + else if (c=="}") { depth--; if (depth==0) break } + } + if (match(obj, /"invoke_separator"[ \t\r\n]*:[ \t\r\n]*"[-.]"/)) { + tok=substr(obj,RSTART,RLENGTH); s=substr(tok,length(tok)-1,1) + if (s=="." || s=="-") sep=s + } + } + } + print sep + } + ' "$integration_json" 2>/dev/null) + case "$awk_separator" in + "."|"-") separator="$awk_separator" ;; + esac + fi fi _SPECIFY_INVOKE_SEPARATOR_CACHE_REPO_ROOT="$repo_root" diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index f2ed932c4..110234a03 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -141,8 +141,9 @@ def _install_shared_infra( Copies ``.specify/scripts//`` and ``.specify/templates/`` from the bundled core_pack or source checkout, where ```` is - ``bash`` when *script_type* is ``"sh"`` and ``powershell`` when it is - ``"ps"``. Tracks all installed files in ``speckit.manifest.json``. + ``bash`` when *script_type* is ``"sh"``, ``python`` when it is ``"py"``, + and ``powershell`` when it is ``"ps"``. Tracks all installed files in + ``speckit.manifest.json``. Shared scripts and page templates are processed to resolve ``__SPECKIT_COMMAND___`` placeholders using *invoke_separator* diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 391015394..c53586912 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -701,6 +701,12 @@ class CommandRegistrar: ) output = self.render_toml_command(frontmatter, body, source_id) elif agent_config["format"] == "yaml": + body = self.resolve_skill_placeholders( + agent_name, frontmatter, body, project_root + ) + body = self._convert_argument_placeholder( + body, "$ARGUMENTS", agent_config["args"] + ) output = self.render_yaml_command( frontmatter, body, source_id, cmd_name ) diff --git a/src/specify_cli/bundler/services/primitives.py b/src/specify_cli/bundler/services/primitives.py index bd0d8ddee..229fb6137 100644 --- a/src/specify_cli/bundler/services/primitives.py +++ b/src/specify_cli/bundler/services/primitives.py @@ -33,12 +33,13 @@ DEFAULT_PRIORITY = 10 def _assert_pinned_version( kind: str, component_id: str, pinned: str | None, advertised: object ) -> None: - """Refuse to install when the catalog version differs from the manifest pin. + """Refuse to install when the resolved version differs from the manifest pin. Bundle manifests pin component versions for reproducibility; installing - whatever the active catalog currently serves would silently violate the - pin. When the catalog advertises no version we cannot enforce the pin, so - installation proceeds (the catalog, not the bundler, owns that gap). + whatever the resolved source (catalog *or* bundled asset) provides would + silently violate the pin. When the source advertises no version we cannot + enforce the pin, so installation proceeds (the source, not the bundler, + owns that gap). """ if not pinned or advertised is None: return @@ -54,11 +55,35 @@ def _assert_pinned_version( if not matches: raise BundlerError( f"{kind} '{component_id}' is pinned to version {pinned} in the bundle " - f"manifest, but the active catalog serves {actual}. Update the bundle's " - "pinned version or the catalog before installing." + f"manifest, but the resolved version is {actual}. Update the bundle's " + "pinned version or the source before installing." ) +def _bundled_manifest_version(manifest_path: Path, root_key: str) -> str | None: + """Best-effort read of a bundled asset's declared version from its manifest. + + Returns ``None`` when the manifest is missing/unreadable/invalid, which + ``_assert_pinned_version`` treats as "cannot enforce" (proceed) — matching + the catalog "advertises no version" escape hatch. + """ + try: + import yaml + + data = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + if isinstance(data, dict): + section = data.get(root_key) + if isinstance(section, dict): + version = section.get("version") + # Only a non-empty string is a usable version; anything else + # (missing / non-string / whitespace) means "cannot enforce". + if isinstance(version, str) and version.strip(): + return version + except Exception: # noqa: BLE001 - unreadable/invalid manifest: skip pin + return None + return None + + class _KindManager(Protocol): def is_installed(self, component: ComponentRef) -> bool: ... @@ -134,6 +159,15 @@ class _PresetKindManager: bundled = _locate_bundled_preset(component.id) if bundled is not None: + # Enforce the manifest pin against the bundled asset's own version, + # mirroring the catalog path below (the bundled path previously + # skipped the pin entirely). + _assert_pinned_version( + "Preset", + component.id, + component.version, + _bundled_manifest_version(bundled / "preset.yml", "preset"), + ) self._manager.install_from_directory(bundled, speckit_version, priority) return @@ -198,6 +232,15 @@ class _ExtensionKindManager: bundled = _locate_bundled_extension(component.id) if bundled is not None: + # Enforce the manifest pin against the bundled asset's own version, + # mirroring the catalog path below (the bundled path previously + # skipped the pin entirely). + _assert_pinned_version( + "Extension", + component.id, + component.version, + _bundled_manifest_version(bundled / "extension.yml", "extension"), + ) self._manager.install_from_directory( bundled, speckit_version, priority=priority ) diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index 4eb9bb19a..bfbb81b85 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -17,6 +17,7 @@ import os import re import shlex import shutil +import subprocess import sys from abc import ABC from dataclasses import dataclass @@ -54,6 +55,18 @@ _CORE_COMMAND_TEMPLATE_RANK = { } +def yaml_quote(value: str) -> str: + """Emit *value* as a double-quoted YAML scalar on a single line. + + A hand-rolled quote cannot carry raw newlines (YAML folds them to + spaces) or control characters (the reader rejects them), so let the + YAML emitter produce the escapes. + """ + return yaml.safe_dump( + str(value), default_style='"', allow_unicode=True, width=sys.maxsize + ).strip() + + # --------------------------------------------------------------------------- # IntegrationOption # --------------------------------------------------------------------------- @@ -592,10 +605,42 @@ class IntegrationBase(ABC): if candidate.exists(): return relative for name in ("python3", "python"): - if shutil.which(name): - return name + found = shutil.which(name) + if not found: + continue + # On Windows, python3/python on PATH may be the Microsoft + # Store App Execution Alias stub: it exists but only prints + # an installer hint and exits non-zero, so existence is not + # enough (see #3304 for the same defect in the sh scripts). + if sys.platform == "win32" and not IntegrationBase._interpreter_runs( + found + ): + continue + return name return sys.executable or "python3" + @staticmethod + def _interpreter_runs(path: str) -> bool: + """Return True when *path* executes as a Python interpreter. + + Runs isolated (``-I``) without ``site`` (``-S``) and discards + I/O so the probe is a fast liveness check that cannot trigger + ``sitecustomize``/user startup hooks. + """ + try: + return ( + subprocess.run( + [path, "-I", "-S", "-c", ""], + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + timeout=15, + ).returncode + == 0 + ) + except (OSError, subprocess.SubprocessError): + return False + @staticmethod def process_template( content: str, @@ -1077,7 +1122,6 @@ class TomlIntegration(IntegrationBase): # YamlIntegration — YAML-format agents (Goose) # --------------------------------------------------------------------------- - class YamlIntegration(IntegrationBase): """Concrete base for integrations that use YAML recipe format. @@ -1480,21 +1524,17 @@ class SkillsIntegration(IntegrationBase): if not description: description = f"Spec Kit: {command_name} workflow" - # Build SKILL.md with manually formatted frontmatter to match - # the release packaging script output exactly (double-quoted - # values, no yaml.safe_dump quoting differences). - def _quote(v: str) -> str: - escaped = v.replace("\\", "\\\\").replace('"', '\\"') - return f'"{escaped}"' - + # Build SKILL.md with manually formatted frontmatter (stable + # double-quoted values). yaml_quote escapes newlines and control + # characters that a plain quoted f-string cannot carry. skill_content = ( f"---\n" - f"name: {_quote(skill_name)}\n" - f"description: {_quote(description)}\n" - f"compatibility: {_quote('Requires spec-kit project structure with .specify/ directory')}\n" + f"name: {yaml_quote(skill_name)}\n" + f"description: {yaml_quote(description)}\n" + f"compatibility: {yaml_quote('Requires spec-kit project structure with .specify/ directory')}\n" f"metadata:\n" - f" author: {_quote('github-spec-kit')}\n" - f" source: {_quote('templates/commands/' + src_file.name)}\n" + f" author: {yaml_quote('github-spec-kit')}\n" + f" source: {yaml_quote('templates/commands/' + src_file.name)}\n" f"---\n" f"{processed_body}" ) diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index 5d1f3a261..63ea5f998 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -18,7 +18,7 @@ from typing import Any import yaml -from ..base import IntegrationOption, SkillsIntegration +from ..base import IntegrationOption, SkillsIntegration, yaml_quote from ..manifest import IntegrationManifest @@ -153,20 +153,18 @@ class HermesIntegration(SkillsIntegration): if not description: description = f"Spec Kit: {command_name} workflow" - # Build SKILL.md with manually formatted frontmatter - def _quote(v: str) -> str: - escaped = v.replace("\\", "\\\\").replace('"', '\\"') - return f'"{escaped}"' - + # Build SKILL.md with manually formatted frontmatter. yaml_quote + # escapes newlines and control characters that a plain quoted + # f-string cannot carry. skill_content = ( f"---\n" - f"name: {_quote(skill_name)}\n" - f"description: {_quote(description)}\n" + f"name: {yaml_quote(skill_name)}\n" + f"description: {yaml_quote(description)}\n" f"compatibility: " - f"{_quote('Requires spec-kit project structure with .specify/ directory')}\n" + f"{yaml_quote('Requires spec-kit project structure with .specify/ directory')}\n" f"metadata:\n" - f" author: {_quote('github-spec-kit')}\n" - f" source: {_quote('templates/commands/' + src_file.name)}\n" + f" author: {yaml_quote('github-spec-kit')}\n" + f" source: {yaml_quote('templates/commands/' + src_file.name)}\n" f"---\n" f"{processed_body}" ) diff --git a/src/specify_cli/shared_infra.py b/src/specify_cli/shared_infra.py index 0685b6c9b..1b07cc771 100644 --- a/src/specify_cli/shared_infra.py +++ b/src/specify_cli/shared_infra.py @@ -328,7 +328,10 @@ def refresh_shared_templates( _ensure_safe_shared_destination(project_path, dst) rel = dst.relative_to(project_path).as_posix() if dst.exists() and not force: - if rel not in tracked_files or rel in modified: + if rel not in tracked_files or rel in modified or manifest.is_recovered(rel): + # Never overwrite a recovered (pre-existing user) file without + # --force, matching install_shared_infra's is_recovered gate + # (#2918). Without this, refresh clobbers user content. skipped_files.append(rel) continue @@ -344,7 +347,7 @@ def refresh_shared_templates( if skipped_files: console.print( - f"[yellow]⚠[/yellow] {len(skipped_files)} modified or untracked shared template file(s) were not updated:" + f"[yellow]⚠[/yellow] {len(skipped_files)} modified, untracked, or preserved (recovered) shared template file(s) were not updated:" ) for rel in skipped_files: console.print(f" {rel}") @@ -400,7 +403,7 @@ def install_shared_infra( # manifest entries the core no longer ships (stale-script cleanup, #3076). seen_rels: set[str] = set() scripts_scanned = False - variant_dir = "bash" if script_type == "sh" else "powershell" + variant_dir = {"sh": "bash", "py": "python"}.get(script_type, "powershell") def _decide_overwrite(rel: str, dst: Path) -> tuple[bool, str | None]: """Return (write, bucket) where bucket is 'skip', 'preserved', or None.""" @@ -462,6 +465,10 @@ def install_shared_infra( for src_path in variant_src.rglob("*"): if not src_path.is_file(): continue + # Python bytecode caches are local artifacts, not + # workflow scripts — never install them. + if "__pycache__" in src_path.parts: + continue # Mark scanned only once a real source file is seen. An # empty (or symlink-skipped) variant keeps this False, so # stale-cleanup is skipped — otherwise it would treat every diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index 2b7ceca15..d6736bc32 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -242,6 +242,26 @@ def _interpolate_expressions(template: str, namespace: dict[str, Any]) -> str: return "".join(out) +def _split_top_level(text: str, sep: str) -> list[str]: + """Split *text* on each occurrence of *sep* that lies outside any quoted + string or nested brackets. + + Used to break a filter chain (``a | map('x') | join(',')``) into its + individual filter segments without splitting on a ``|`` that appears inside + a quoted argument. Each returned segment is a slice at a top-level + boundary, so the quote/bracket scan restarts cleanly on the remainder. + """ + parts: list[str] = [] + start = 0 + while True: + idx = _find_top_level(text[start:], sep) + if idx == -1: + parts.append(text[start:]) + return parts + parts.append(text[start:start + idx]) + start += idx + len(sep) + + def _split_top_level_commas(text: str) -> list[str]: """Split *text* on commas that are not inside quotes or nested brackets. @@ -305,6 +325,68 @@ def _find_top_level(text: str, token: str) -> int: return -1 +def _apply_filter(value: Any, filter_expr: str, namespace: dict[str, Any]) -> Any: + """Apply a single pipe filter segment to *value*. + + *filter_expr* is one link of a filter chain — the text between two + top-level ``|`` separators, already stripped (e.g. ``map('name')``, + ``default('x')``, ``from_json``). Returns the filtered value so the caller + can feed it into the next link. + + Raises ``ValueError`` on any mis-wired or unknown filter rather than + silently returning *value* unchanged: a passthrough would turn a mistyped + or unsupported filter into a wrong result with no signal. + """ + # `from_json` is strict: it takes no arguments and tolerates no trailing + # tokens. Match on the leading filter name and require the whole filter to + # be exactly `from_json`, so every mis-wired form (`from_json()`, + # `from_json('x')`, `from_json)`, `from_json extra`) fails loudly instead of + # silently falling through to the unknown-filter path. + leading = re.match(r"\w+", filter_expr) + if leading and leading.group(0) == "from_json": + if filter_expr != "from_json": + raise ValueError( + "from_json: expected '| from_json' with no arguments or " + f"trailing tokens, got '| {filter_expr}'" + ) + return _filter_from_json(value) + + # Parse filter name and argument + filter_match = re.match(r"(\w+)\((.+)\)", filter_expr) + if filter_match: + fname = filter_match.group(1) + farg = _evaluate_simple_expression(filter_match.group(2).strip(), namespace) + if fname == "default": + return _filter_default(value, farg) + if fname == "join": + return _filter_join(value, farg) + if fname == "map": + return _filter_map(value, farg) + if fname == "contains": + return _filter_contains(value, farg) + # Filter without args + if filter_expr == "default": + return _filter_default(value) + # No recognized filter matched. Fail loudly rather than silently returning + # the unfiltered value. Distinguish a *registered* filter used in an + # unsupported form (e.g. `| join` or `| map` with no argument) from a + # genuinely unknown filter name, so the message names the real problem + # instead of calling a known filter "unknown". + name = leading.group(0) if leading else filter_expr + expected = ( + "expected one of default or default('x'), join('sep'), " + "map('attr'), contains('s'), or from_json" + ) + if name in _REGISTERED_FILTERS: + raise ValueError( + f"filter '{name}' used in an unsupported form (got " + f"'| {filter_expr}'): {expected}" + ) + raise ValueError( + f"unknown filter '{name}': {expected} (got '| {filter_expr}')" + ) + + def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: """Evaluate a simple expression against the namespace. @@ -329,65 +411,17 @@ def _evaluate_simple_expression(expr: str, namespace: dict[str, Any]) -> Any: # Handle pipe filters. Detect the pipe at the top level only, so a literal # '|' inside a quoted operand (e.g. `inputs.x == 'a|b'`) or nested brackets is # not mistaken for a filter separator — mirroring the operator parsing below. + # Filters chain left-to-right: `list | map('name') | join(', ')` feeds each + # filter's result into the next, so `map` (which yields a list) can be + # rendered by `join`. Splitting only at the first pipe would hand the whole + # tail to one filter and mangle any later `|`. pipe_idx = _find_top_level(expr, "|") if pipe_idx != -1: - value = _evaluate_simple_expression(expr[:pipe_idx].strip(), namespace) - filter_expr = expr[pipe_idx + 1:].strip() - - # `from_json` is strict: it takes no arguments and tolerates no - # trailing tokens. Match on the leading filter name and require the - # whole filter to be exactly `from_json`, so every mis-wired form - # (`from_json()`, `from_json('x')`, `from_json)`, `from_json extra`) - # fails loudly instead of silently falling through to the - # unknown-filter path and returning the unparsed value. (filter_expr - # is already stripped above.) - leading = re.match(r"\w+", filter_expr) - if leading and leading.group(0) == "from_json": - if filter_expr != "from_json": - raise ValueError( - "from_json: expected '| from_json' with no arguments or " - f"trailing tokens, got '| {filter_expr}'" - ) - return _filter_from_json(value) - - # Parse filter name and argument - filter_match = re.match(r"(\w+)\((.+)\)", filter_expr) - if filter_match: - fname = filter_match.group(1) - farg = _evaluate_simple_expression(filter_match.group(2).strip(), namespace) - if fname == "default": - return _filter_default(value, farg) - if fname == "join": - return _filter_join(value, farg) - if fname == "map": - return _filter_map(value, farg) - if fname == "contains": - return _filter_contains(value, farg) - # Filter without args - filter_name = filter_expr.strip() - if filter_name == "default": - return _filter_default(value) - # No recognized filter matched. Fail loudly rather than silently - # returning the unfiltered value: a passthrough turns a mis-typed or - # unsupported filter into a wrong result with no signal. Mirrors the - # strict `from_json` handling above. Distinguish a *registered* filter - # used in an unsupported form (e.g. `| join` or `| map` with no - # argument) from a genuinely unknown filter name, so the message names - # the real problem instead of calling a known filter "unknown". - leading_name = re.match(r"\w+", filter_expr) - name = leading_name.group(0) if leading_name else filter_expr - expected = ( - "expected one of default or default('x'), join('sep'), " - "map('attr'), contains('s'), or from_json" - ) - if name in _REGISTERED_FILTERS: - raise ValueError( - f"filter '{name}' used in an unsupported form (got " - f"'| {filter_expr}'): {expected}" - ) - raise ValueError( - f"unknown filter '{name}': {expected} (got '| {filter_expr}')" - ) + segments = _split_top_level(expr, "|") + value = _evaluate_simple_expression(segments[0].strip(), namespace) + for segment in segments[1:]: + value = _apply_filter(value, segment.strip(), namespace) + return value # Boolean operators — parse 'or' first (lower precedence) so that # 'a or b and c' is evaluated as 'a or (b and c)'. Splits are quote/bracket diff --git a/templates/commands/analyze.md b/templates/commands/analyze.md index e4ba8f7d8..2cd83bd7c 100644 --- a/templates/commands/analyze.md +++ b/templates/commands/analyze.md @@ -3,6 +3,7 @@ description: Perform a non-destructive cross-artifact consistency and quality an scripts: sh: scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks ps: scripts/powershell/check-prerequisites.ps1 -Json -RequireTasks -IncludeTasks + py: scripts/python/check_prerequisites.py --json --require-tasks --include-tasks --- ## User Input diff --git a/templates/commands/checklist.md b/templates/commands/checklist.md index e202ebb66..6a5c6d874 100644 --- a/templates/commands/checklist.md +++ b/templates/commands/checklist.md @@ -3,6 +3,7 @@ description: Generate a custom checklist for the current feature based on user r scripts: sh: scripts/bash/check-prerequisites.sh --json ps: scripts/powershell/check-prerequisites.ps1 -Json + py: scripts/python/check_prerequisites.py --json --- ## Checklist Purpose: "Unit Tests for English" diff --git a/templates/commands/clarify.md b/templates/commands/clarify.md index 4948fdcfa..a52ef3942 100644 --- a/templates/commands/clarify.md +++ b/templates/commands/clarify.md @@ -7,6 +7,7 @@ handoffs: scripts: sh: scripts/bash/check-prerequisites.sh --json --paths-only ps: scripts/powershell/check-prerequisites.ps1 -Json -PathsOnly + py: scripts/python/check_prerequisites.py --json --paths-only --- ## User Input diff --git a/templates/commands/converge.md b/templates/commands/converge.md index 35cf3736c..eadb96ee5 100644 --- a/templates/commands/converge.md +++ b/templates/commands/converge.md @@ -3,6 +3,7 @@ description: Assess the current codebase against the feature's spec, plan, and t scripts: sh: scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks ps: scripts/powershell/check-prerequisites.ps1 -Json -RequireTasks -IncludeTasks + py: scripts/python/check_prerequisites.py --json --require-tasks --include-tasks --- ## User Input diff --git a/templates/commands/implement.md b/templates/commands/implement.md index eda580d56..5efbcf724 100644 --- a/templates/commands/implement.md +++ b/templates/commands/implement.md @@ -3,6 +3,7 @@ description: Execute the implementation plan by processing and executing all tas scripts: sh: scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks ps: scripts/powershell/check-prerequisites.ps1 -Json -RequireTasks -IncludeTasks + py: scripts/python/check_prerequisites.py --json --require-tasks --include-tasks --- ## User Input diff --git a/templates/commands/taskstoissues.md b/templates/commands/taskstoissues.md index f1df10001..6b60e6f6a 100644 --- a/templates/commands/taskstoissues.md +++ b/templates/commands/taskstoissues.md @@ -4,6 +4,7 @@ tools: ['github/github-mcp-server/list_issues', 'github/github-mcp-server/issue_ scripts: sh: scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks ps: scripts/powershell/check-prerequisites.ps1 -Json -RequireTasks -IncludeTasks + py: scripts/python/check_prerequisites.py --json --require-tasks --include-tasks --- ## User Input diff --git a/tests/integrations/conftest.py b/tests/integrations/conftest.py index 467187235..4b7806c4a 100644 --- a/tests/integrations/conftest.py +++ b/tests/integrations/conftest.py @@ -5,10 +5,8 @@ import pytest from specify_cli.integrations.base import MarkdownIntegration -@pytest.fixture(autouse=True) -def _isolate_integration_home(monkeypatch: pytest.MonkeyPatch, tmp_path): - """Keep integration tests from reading or writing the real user home.""" - home = tmp_path / "home" +def _redirect_home(monkeypatch: pytest.MonkeyPatch, home) -> None: + """Point HOME/USERPROFILE/XDG env vars at an isolated *home* directory.""" for path in (home, home / ".cache", home / ".config", home / ".local" / "share"): path.mkdir(parents=True, exist_ok=True) @@ -19,6 +17,28 @@ def _isolate_integration_home(monkeypatch: pytest.MonkeyPatch, tmp_path): monkeypatch.setenv("XDG_DATA_HOME", str(home / ".local" / "share")) +@pytest.fixture(scope="session", autouse=True) +def _isolate_integration_home_session(tmp_path_factory): + """Isolate the user home for setup that runs outside a test function. + + The per-test fixture below re-points HOME for each test, but function-scoped + fixtures do not apply to module-/session-scoped fixtures. Some of those (e.g. + the ``status_*_template`` fixtures in ``test_integration_subcommand.py``) run + ``specify init`` during setup, before any per-test isolation takes effect. + A standalone ``MonkeyPatch`` gives them an isolated home too. + """ + monkeypatch = pytest.MonkeyPatch() + _redirect_home(monkeypatch, tmp_path_factory.mktemp("session-home")) + yield + monkeypatch.undo() + + +@pytest.fixture(autouse=True) +def _isolate_integration_home(monkeypatch: pytest.MonkeyPatch, tmp_path): + """Keep integration tests from reading or writing the real user home.""" + _redirect_home(monkeypatch, tmp_path / "home") + + class StubIntegration(MarkdownIntegration): """Minimal concrete integration for testing.""" diff --git a/tests/integrations/test_base.py b/tests/integrations/test_base.py index fe531e624..d03ea0cb2 100644 --- a/tests/integrations/test_base.py +++ b/tests/integrations/test_base.py @@ -306,9 +306,12 @@ class TestResolveCommandRefs: class TestResolvePythonInterpreter: def test_returns_python_on_path(self, monkeypatch): # Positive: when python3 is on PATH it is preferred over python. + # Pin a POSIX platform so the Windows stub probe (tested separately + # below) does not reject the fake PATH entries on Windows CI. def fake_which(name): return f"/usr/bin/{name}" if name in ("python3", "python") else None + monkeypatch.setattr("specify_cli.integrations.base.sys.platform", "linux") monkeypatch.setattr( "specify_cli.integrations.base.shutil.which", fake_which ) @@ -318,6 +321,7 @@ class TestResolvePythonInterpreter: def fake_which(name): return "/usr/bin/python" if name == "python" else None + monkeypatch.setattr("specify_cli.integrations.base.sys.platform", "linux") monkeypatch.setattr( "specify_cli.integrations.base.shutil.which", fake_which ) @@ -369,12 +373,79 @@ class TestResolvePythonInterpreter: def test_ignores_missing_venv(self, monkeypatch, tmp_path): # Negative: no venv directory -> PATH resolution is used instead. + monkeypatch.setattr("specify_cli.integrations.base.sys.platform", "linux") monkeypatch.setattr( "specify_cli.integrations.base.shutil.which", lambda name: "/usr/bin/python3" if name == "python3" else None, ) assert IntegrationBase.resolve_python_interpreter(tmp_path) == "python3" + def test_windows_skips_store_alias_stub(self, monkeypatch): + # On Windows, python3 on PATH may be the Microsoft Store App + # Execution Alias stub: it exists but only prints an installer + # hint and exits non-zero. Existence is not enough; the + # interpreter must actually run (mirrors #3304 for the CLI). + monkeypatch.setattr("specify_cli.integrations.base.sys.platform", "win32") + monkeypatch.setattr( + "specify_cli.integrations.base.shutil.which", + lambda name: f"C:\\WindowsApps\\{name}.exe" + if name in ("python3", "python") + else None, + ) + monkeypatch.setattr( + IntegrationBase, "_interpreter_runs", staticmethod(lambda path: False) + ) + monkeypatch.setattr( + "specify_cli.integrations.base.sys.executable", "C:\\Python\\python.exe" + ) + result = IntegrationBase.resolve_python_interpreter() + assert result == "C:\\Python\\python.exe" + + def test_windows_keeps_working_interpreter(self, monkeypatch): + # Positive: a real python3 on Windows PATH passes the run check. + monkeypatch.setattr("specify_cli.integrations.base.sys.platform", "win32") + monkeypatch.setattr( + "specify_cli.integrations.base.shutil.which", + lambda name: f"C:\\Python\\{name}.exe" if name == "python3" else None, + ) + monkeypatch.setattr( + IntegrationBase, "_interpreter_runs", staticmethod(lambda path: True) + ) + assert IntegrationBase.resolve_python_interpreter() == "python3" + + def test_windows_stub_python3_falls_through_to_working_python(self, monkeypatch): + # python3 is the stub but python is a real install: pick python. + monkeypatch.setattr("specify_cli.integrations.base.sys.platform", "win32") + monkeypatch.setattr( + "specify_cli.integrations.base.shutil.which", + lambda name: f"C:\\somewhere\\{name}.exe" + if name in ("python3", "python") + else None, + ) + monkeypatch.setattr( + IntegrationBase, + "_interpreter_runs", + staticmethod(lambda path: path.endswith("python.exe")), + ) + assert IntegrationBase.resolve_python_interpreter() == "python" + + def test_posix_does_not_spawn_run_check(self, monkeypatch): + # Non-Windows platforms have no App Execution Alias; existence + # on PATH stays sufficient and no subprocess is spawned. + monkeypatch.setattr("specify_cli.integrations.base.sys.platform", "linux") + monkeypatch.setattr( + "specify_cli.integrations.base.shutil.which", + lambda name: "/usr/bin/python3" if name == "python3" else None, + ) + + def boom(path): + raise AssertionError("run check must not execute on POSIX") + + monkeypatch.setattr( + IntegrationBase, "_interpreter_runs", staticmethod(boom) + ) + assert IntegrationBase.resolve_python_interpreter() == "python3" + class TestProcessTemplatePyScriptType: CONTENT = ( @@ -390,6 +461,7 @@ class TestProcessTemplatePyScriptType: def test_py_prefixes_interpreter(self, monkeypatch): # Positive: py script type prefixes a resolved interpreter and the # script path is rewritten to the .specify location. + monkeypatch.setattr("specify_cli.integrations.base.sys.platform", "linux") monkeypatch.setattr( "specify_cli.integrations.base.shutil.which", lambda name: "/usr/bin/python3" if name == "python3" else None, diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index ed978cbb5..cb2d136e6 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -2073,3 +2073,44 @@ class TestIntegrationCatalogDiscoveryCLI: assert listing.exit_code == 0, listing.output assert "default" in listing.output assert "community" in listing.output + + +def test_refresh_shared_templates_preserves_recovered_user_file(tmp_path): + """refresh_shared_templates must not overwrite a recovered (pre-existing + user) template without --force, matching install_shared_infra's gate (#2918). + """ + from specify_cli.shared_infra import ( + load_speckit_manifest, + refresh_shared_templates, + ) + + project = tmp_path / "proj" + templates_dir = project / ".specify" / "templates" + templates_dir.mkdir(parents=True) + user_file = templates_dir / "spec-template.md" + user_file.write_text("# USER CUSTOM CONTENT\n", encoding="utf-8") + + # Record the pre-existing file as recovered (its hash was adopted, not written). + manifest = load_speckit_manifest(project, version="test", console=_NoopConsole()) + rel = ".specify/templates/spec-template.md" + manifest.record_existing(rel, recovered=True) + manifest.save() + + # Bundled source ships a different body for the same template. + core_pack = tmp_path / "core-pack" + src = core_pack / "templates" + src.mkdir(parents=True) + (src / "spec-template.md").write_text("# BUNDLED CONTENT v2\n", encoding="utf-8") + + refresh_shared_templates( + project, + version="test", + core_pack=core_pack, + repo_root=tmp_path / "unused", + console=_NoopConsole(), + invoke_separator=".", + force=False, + ) + + # Recovered user content must survive (fail-before: replaced by bundled body). + assert user_file.read_text(encoding="utf-8") == "# USER CUSTOM CONTENT\n" diff --git a/tests/integrations/test_home_isolation.py b/tests/integrations/test_home_isolation.py index e79ff5dff..c862b1686 100644 --- a/tests/integrations/test_home_isolation.py +++ b/tests/integrations/test_home_isolation.py @@ -15,6 +15,10 @@ def test_integration_tests_use_tmp_home(tmp_path: Path) -> None: assert Path(os.environ["XDG_CONFIG_HOME"]) == home / ".config" assert Path(os.environ["XDG_DATA_HOME"]) == home / ".local" / "share" + # Most integrations resolve the user home via Path.home() (e.g. Hermes, + # catalog), so the isolation has to reach that API, not just the env vars. + assert Path.home() == home + assert home.is_dir() assert (home / ".cache").is_dir() assert (home / ".config").is_dir() diff --git a/tests/integrations/test_integration_goose.py b/tests/integrations/test_integration_goose.py index 104b7188d..300b056c4 100644 --- a/tests/integrations/test_integration_goose.py +++ b/tests/integrations/test_integration_goose.py @@ -36,3 +36,50 @@ class TestGooseIntegration(YamlIntegrationTests): param.get("key") == "args" for param in data.get("parameters", []) ), f"{recipe_file} uses {{{{args}}}} but does not declare args" + + +class TestGooseCommandPlaceholderResolution: + """register_commands must resolve skill placeholders for the yaml branch. + + The yaml (Goose recipe) branch previously skipped + resolve_skill_placeholders / _convert_argument_placeholder that the + markdown and toml branches apply, so extension/preset command bodies + kept literal {SCRIPT} / __AGENT__ / repo-relative paths. + """ + + def test_register_commands_resolves_placeholders_in_recipe(self, tmp_path): + from specify_cli.agents import CommandRegistrar + + ext_dir = tmp_path / "extension" + cmd_dir = ext_dir / "commands" + cmd_dir.mkdir(parents=True) + cmd_file = cmd_dir / "example.md" + cmd_file.write_text( + "---\n" + "description: Placeholder command\n" + "scripts:\n" + " sh: scripts/bash/do.sh\n" + " ps: scripts/powershell/do.ps1\n" + "---\n\n" + "Run {SCRIPT} for agent __AGENT__ with $ARGUMENTS.\n", + encoding="utf-8", + ) + + registrar = CommandRegistrar() + commands = [{"name": "speckit.example", "file": "commands/example.md"}] + registrar.register_commands("goose", commands, "test-ext", ext_dir, tmp_path) + + recipe = tmp_path / ".goose" / "recipes" / "speckit.example.yaml" + assert recipe.exists(), "goose recipe should be generated" + # Parse the recipe and assert the prompt actually got the correct + # replacements — not merely that the literal tokens are absent (which + # a wrong-but-token-free output could also satisfy). + data = yaml.safe_load(recipe.read_text(encoding="utf-8")) + prompt = data["prompt"] + assert ".specify/scripts/" in prompt # {SCRIPT} -> resolved script path + assert "agent goose" in prompt # __AGENT__ -> agent name + assert "{{args}}" in prompt # $ARGUMENTS -> goose args token + # And the raw placeholders must not survive. + assert "{SCRIPT}" not in prompt + assert "__AGENT__" not in prompt + assert "$ARGUMENTS" not in prompt diff --git a/tests/integrations/test_skill_frontmatter_quoting.py b/tests/integrations/test_skill_frontmatter_quoting.py new file mode 100644 index 000000000..739039ea3 --- /dev/null +++ b/tests/integrations/test_skill_frontmatter_quoting.py @@ -0,0 +1,111 @@ +"""Regression tests for SKILL.md frontmatter quoting (#3391). + +The skills setup path builds SKILL.md frontmatter by hand with +double-quoted values. A double-quoted YAML scalar cannot carry a raw +newline (the parser folds it to a space) or a control character (the +reader rejects the document), so descriptions taken from template +frontmatter must be escaped by the YAML emitter. +""" + +from pathlib import Path + +import yaml + +from specify_cli.integrations import get_integration +from specify_cli.integrations.base import yaml_quote +from specify_cli.integrations.manifest import IntegrationManifest + +MULTILINE = "first line\nsecond line\n" +CONTROL = "ding\aling" + +HOSTILE_TEMPLATE = """--- +description: | + first line + second line +--- + +Body of the command. +""" + +CONTROL_TEMPLATE = """--- +description: "ding\\aling" +--- + +Body of the command. +""" + + +def _parse_frontmatter(skill_file: Path) -> dict: + content = skill_file.read_text(encoding="utf-8") + assert content.startswith("---\n") + return yaml.safe_load(content.split("---", 2)[1]) + + +def _fake_templates(tmp_path: Path, body: str) -> Path: + templates = tmp_path / "templates" + templates.mkdir(exist_ok=True) + (templates / "plan.md").write_text(body, encoding="utf-8") + return templates + + +class TestYamlQuote: + def test_simple_value_keeps_plain_double_quoted_form(self): + assert yaml_quote("speckit-plan") == '"speckit-plan"' + assert yaml_quote('say "hi"') == '"say \\"hi\\""' + assert yaml_quote("back\\slash") == '"back\\\\slash"' + + def test_multiline_value_round_trips(self): + quoted = yaml_quote(MULTILINE) + assert "\n" not in quoted + assert yaml.safe_load(quoted) == MULTILINE + + def test_control_character_round_trips(self): + quoted = yaml_quote(CONTROL) + assert "\a" not in quoted + assert yaml.safe_load(quoted) == CONTROL + + +class TestSkillFrontmatterQuoting: + def _generate(self, tmp_path, monkeypatch, template: str) -> Path: + integration = get_integration("agy") + monkeypatch.setattr( + integration, + "shared_commands_dir", + lambda: _fake_templates(tmp_path, template), + ) + manifest = IntegrationManifest("agy", tmp_path) + created = integration.setup(tmp_path, manifest) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) == 1 + return skill_files[0] + + def test_multiline_description_survives(self, tmp_path, monkeypatch): + skill_file = self._generate(tmp_path, monkeypatch, HOSTILE_TEMPLATE) + fm = _parse_frontmatter(skill_file) + assert fm["description"] == MULTILINE + + def test_control_character_description_parses(self, tmp_path, monkeypatch): + skill_file = self._generate(tmp_path, monkeypatch, CONTROL_TEMPLATE) + fm = _parse_frontmatter(skill_file) + assert fm["description"] == CONTROL + + +class TestHermesSkillFrontmatterQuoting: + def test_multiline_description_survives(self, tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir(exist_ok=True) + monkeypatch.setattr(Path, "home", lambda: home) + + integration = get_integration("hermes") + monkeypatch.setattr( + integration, + "shared_commands_dir", + lambda: _fake_templates(tmp_path, HOSTILE_TEMPLATE), + ) + manifest = IntegrationManifest("hermes", tmp_path) + created = integration.setup(tmp_path, manifest) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) == 1 + + fm = _parse_frontmatter(skill_files[0]) + assert fm["description"] == MULTILINE diff --git a/tests/test_command_template_py_scripts.py b/tests/test_command_template_py_scripts.py new file mode 100644 index 000000000..6ad687964 --- /dev/null +++ b/tests/test_command_template_py_scripts.py @@ -0,0 +1,96 @@ +"""Command templates with a py: script line must render for --script py. + +Covers #3283: ``py:`` lines in the ``scripts:`` frontmatter of +``templates/commands/*.md`` reference Python scripts that exist in the repo, +and ``process_template`` turns them into a valid Python invocation +(interpreter-prefixed, path rewritten to the ``.specify`` tree). + +``plan.md`` and ``tasks.md`` gain their ``py:`` lines together with +``setup_plan.py``/``setup_tasks.py`` in the core-scripts port (#3280); the +existence check below enforces that ordering. +""" + +import re +from pathlib import Path + +import pytest + +from specify_cli.integrations.base import IntegrationBase + +REPO_ROOT = Path(__file__).parent.parent +TEMPLATES_DIR = REPO_ROOT / "templates" / "commands" + +_PY_LINE = re.compile(r"^\s*py: (scripts/python/\S+\.py)", re.MULTILINE) + + +def _py_script(name: str) -> str | None: + match = _PY_LINE.search((TEMPLATES_DIR / name).read_text(encoding="utf-8")) + return match.group(1) if match else None + + +PY_TEMPLATES = sorted( + p.name for p in TEMPLATES_DIR.glob("*.md") if _py_script(p.name) +) + + +@pytest.fixture(autouse=True) +def _pin_interpreter(monkeypatch): + monkeypatch.setattr( + "specify_cli.integrations.base.shutil.which", + lambda name: "/usr/bin/python3" if name == "python3" else None, + ) + + +def test_py_templates_discovered(): + # Guard: the glob must find the known py-scripted templates, otherwise + # the parametrized tests below would silently pass on an empty set. + assert "implement.md" in PY_TEMPLATES + assert "clarify.md" in PY_TEMPLATES + + +@pytest.mark.parametrize("name", PY_TEMPLATES) +def test_referenced_python_script_exists(name: str): + # A py: line must never point at a script the repo does not ship — + # rendering would produce a broken invocation at runtime. + script = _py_script(name) + assert (REPO_ROOT / script).is_file(), f"{name} references missing {script}" + + +@pytest.mark.parametrize("name", PY_TEMPLATES) +def test_template_renders_python_invocation(name: str): + content = (TEMPLATES_DIR / name).read_text(encoding="utf-8") + result = IntegrationBase.process_template(content, "agent", "py") + assert "{SCRIPT}" not in result + assert re.search( + r"python3 \.specify/scripts/python/\w+\.py(?: --[\w-]+)*", result + ), f"{name} did not render a Python invocation" + + +@pytest.mark.parametrize("name", PY_TEMPLATES) +def test_sh_rendering_unchanged(name: str): + # Negative: adding py: lines must not leak into sh rendering. + content = (TEMPLATES_DIR / name).read_text(encoding="utf-8") + result = IntegrationBase.process_template(content, "agent", "sh") + assert "{SCRIPT}" not in result + assert "scripts/python" not in result + + +def test_install_shared_infra_copies_python_scripts(tmp_path): + # --script py must install scripts/python/ into .specify/scripts/python/ + # so the rendered invocations point at files that exist. + from rich.console import Console + + from specify_cli.shared_infra import install_shared_infra + + install_shared_infra( + tmp_path, + "py", + version="0.0.0", + core_pack=None, + repo_root=REPO_ROOT, + console=Console(quiet=True), + force=False, + ) + dest = tmp_path / ".specify" / "scripts" / "python" + assert (dest / "check_prerequisites.py").is_file() + assert not (tmp_path / ".specify" / "scripts" / "powershell").exists() diff --git a/tests/test_setup_tasks.py b/tests/test_setup_tasks.py index 47a284f8a..4551b5c29 100644 --- a/tests/test_setup_tasks.py +++ b/tests/test_setup_tasks.py @@ -466,6 +466,68 @@ def test_bash_command_hint_preserves_hyphens_inside_segments(tasks_repo: Path) - assert result.stdout.strip() == "/speckit.jira.sync-status" +def _install_broken_json_tool_stubs(repo: Path) -> Path: + """Create a bin dir with `jq` and `python3` stubs that exist but fail. + + Mimics stock Windows + Git Bash, where a JSON tool may be missing or broken + and `python3` resolves to the Microsoft Store App Execution Alias stub: both + satisfy `command -v` yet fail at runtime (the alias exits 49). Prepending + this to PATH forces the invoke-separator parser past jq and python3 to its + awk text fallback (#3304). + """ + stub_dir = repo / "_broken_bin" + stub_dir.mkdir(exist_ok=True) + for name in ("jq", "python3"): + stub = stub_dir / name + stub.write_text( + "#!/bin/sh\n" + 'echo "simulated broken interpreter/tool" >&2\n' + "exit 49\n", + encoding="utf-8", + newline="\n", + ) + stub.chmod(0o755) + return stub_dir + + +@requires_bash +def test_bash_command_hint_falls_back_to_awk_when_jq_and_python3_broken( + tasks_repo: Path, +) -> None: + """Separator resolution survives broken jq and python3 stubs (#3304). + + `get_invoke_separator` historically selected python3 by availability and + had no text fallback, so a Windows Store python3 stub made it silently + return "." even for `-`-separator integrations (e.g. forge), yielding a + wrong hint like `/speckit.plan`. The awk fallback must recover `-`. + """ + _write_integration_state(tasks_repo, "forge", "-") + stub_dir = _install_broken_json_tool_stubs(tasks_repo) + + script = tasks_repo / ".specify" / "scripts" / "bash" / "common.sh" + env = _clean_env() + env["PATH"] = f"{stub_dir}{os.pathsep}{env.get('PATH', '')}" + + result = subprocess.run( + [ + "bash", + "-c", + 'source "$1"; format_speckit_command "$2" "$PWD"', + "bash", + str(script), + "plan", + ], + cwd=tasks_repo, + capture_output=True, + text=True, + check=False, + env=env, + ) + + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == "/speckit-plan" + + @requires_bash def test_bash_command_hint_caches_invoke_separator_per_process(tasks_repo: Path) -> None: _write_integration_state(tasks_repo, "claude", "-") diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 41522d3e8..9341bdec4 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -601,6 +601,73 @@ class TestExpressions: ): evaluate_expression("{{ inputs.tags | map }}", ctx) + def test_chained_filters_apply_left_to_right(self): + # Filters chain: each filter's result feeds the next. `map` yields a + # list and `join` is the only filter that renders a list to a string, + # so `map('name') | join(', ')` is the canonical pairing — it must not + # raise. Previously the pipe parser split only at the first `|` and + # handed the whole tail (`map('name') | join(', ')`) to one filter, + # which the `name(arg)` regex mangled into a ValueError. + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext( + inputs={ + "rows": [{"name": "a"}, {"name": "b"}], + "tags": ["x", "y"], + "missing": None, + } + ) + assert ( + evaluate_expression( + "{{ inputs.rows | map('name') | join(', ') }}", ctx + ) + == "a, b" + ) + # A three-link chain: map -> join -> contains. + assert ( + evaluate_expression( + "{{ inputs.rows | map('name') | join(', ') | contains('a') }}", + ctx, + ) + is True + ) + # default's fallback then flows into the next filter. + assert ( + evaluate_expression( + "{{ inputs.missing | default('x') | contains('x') }}", ctx + ) + is True + ) + + def test_chained_filter_error_in_later_link_raises(self): + # A mis-wired filter anywhere in the chain must fail loudly, not just + # the first link. + import pytest + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"rows": [{"name": "a"}]}) + with pytest.raises(ValueError, match="unknown filter 'bogus'"): + evaluate_expression( + "{{ inputs.rows | map('name') | bogus }}", ctx + ) + + def test_pipe_in_quoted_arg_is_not_a_filter_separator(self): + # A literal `|` inside a quoted operand or filter argument must not be + # mistaken for a filter-chain separator — the top-level split has to + # respect quotes. + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"mode": "a|b", "tags": ["a|b", "c"]}) + assert evaluate_expression("{{ inputs.mode == 'a|b' }}", ctx) is True + # `|` inside a filter argument stays part of the argument. + assert ( + evaluate_expression("{{ inputs.tags | join(' | ') }}", ctx) + == "a|b | c" + ) + def test_condition_evaluation(self): from specify_cli.workflows.expressions import evaluate_condition from specify_cli.workflows.base import StepContext diff --git a/tests/unit/test_bundler_primitives.py b/tests/unit/test_bundler_primitives.py index f662d22fc..9891e6f77 100644 --- a/tests/unit/test_bundler_primitives.py +++ b/tests/unit/test_bundler_primitives.py @@ -131,3 +131,87 @@ def test_preset_install_preserves_explicit_zero_priority(tmp_path: Path, monkeyp # An explicit priority of 0 must be passed through, not replaced by default. assert calls["priority"] == 0 + + +def _write_manifest(path: Path, root_key: str, version: str) -> Path: + path.mkdir(parents=True, exist_ok=True) + (path / f"{root_key}.yml").write_text( + f"{root_key}:\n id: x\n version: {version}\n", encoding="utf-8" + ) + return path + + +def test_bundled_extension_pin_mismatch_refuses(tmp_path: Path, monkeypatch): + """A bundled extension whose version != the manifest pin must be refused + (the bundled path previously skipped the pin the catalog path enforces).""" + import specify_cli._assets as assets + from specify_cli.extensions import ExtensionManager + + bundled = _write_manifest(tmp_path / "ext", "extension", "1.0.0") + monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: bundled) + called: list = [] + monkeypatch.setattr( + ExtensionManager, "install_from_directory", + lambda self, *a, **k: called.append(a), + ) + + manager = primitive_manager("extensions", tmp_path, allow_network=False) + with pytest.raises(BundlerError, match="pinned to version 2.0.0"): + manager.install(ComponentRef(kind="extensions", id="my-ext", version="2.0.0")) + assert called == [] # install must not proceed + + +def test_bundled_extension_pin_match_installs(tmp_path: Path, monkeypatch): + import specify_cli._assets as assets + from specify_cli.extensions import ExtensionManager + + bundled = _write_manifest(tmp_path / "ext", "extension", "1.0.0") + monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: bundled) + called: list = [] + monkeypatch.setattr( + ExtensionManager, "install_from_directory", + lambda self, *a, **k: called.append(a), + ) + + manager = primitive_manager("extensions", tmp_path, allow_network=False) + # matching pin, and unpinned, both install cleanly + manager.install(ComponentRef(kind="extensions", id="my-ext", version="1.0.0")) + manager.install(ComponentRef(kind="extensions", id="my-ext", version=None)) + assert len(called) == 2 + + +def test_bundled_preset_pin_mismatch_refuses(tmp_path: Path, monkeypatch): + import specify_cli._assets as assets + from specify_cli.presets import PresetManager + + bundled = _write_manifest(tmp_path / "preset", "preset", "1.0.0") + monkeypatch.setattr(assets, "_locate_bundled_preset", lambda cid: bundled) + called: list = [] + monkeypatch.setattr( + PresetManager, "install_from_directory", + lambda self, *a, **k: called.append(a), + ) + + manager = primitive_manager("presets", tmp_path, allow_network=False) + with pytest.raises(BundlerError, match="pinned to version 2.0.0"): + manager.install(ComponentRef(kind="presets", id="my-preset", version="2.0.0")) + assert called == [] + + +def test_bundled_preset_pin_match_installs(tmp_path: Path, monkeypatch): + import specify_cli._assets as assets + from specify_cli.presets import PresetManager + + bundled = _write_manifest(tmp_path / "preset", "preset", "1.0.0") + monkeypatch.setattr(assets, "_locate_bundled_preset", lambda cid: bundled) + called: list = [] + monkeypatch.setattr( + PresetManager, "install_from_directory", + lambda self, *a, **k: called.append(a), + ) + + manager = primitive_manager("presets", tmp_path, allow_network=False) + # matching pin, and unpinned, both proceed to install + manager.install(ComponentRef(kind="presets", id="my-preset", version="1.0.0")) + manager.install(ComponentRef(kind="presets", id="my-preset", version=None)) + assert len(called) == 2