diff --git a/src/specify_cli/__main__.py b/src/specify_cli/__main__.py new file mode 100644 index 0000000000..0ae7916d64 --- /dev/null +++ b/src/specify_cli/__main__.py @@ -0,0 +1,6 @@ +"""Allow ``python -m specify_cli`` to run the CLI.""" + +from specify_cli import main + +if __name__ == "__main__": + main() diff --git a/src/specify_cli/presets/_manager.py b/src/specify_cli/presets/_manager.py index 33274dfdfc..48c27afd2f 100644 --- a/src/specify_cli/presets/_manager.py +++ b/src/specify_cli/presets/_manager.py @@ -2,6 +2,7 @@ import hashlib import json +import os import shutil import tempfile from pathlib import Path @@ -150,6 +151,96 @@ def _materialize_constitution_template( return result +# Generated files for script composition (#4551). Both carry no +# stack-specific data — only the script's own name — so they never need +# rewriting when priorities/enablement change; the dispatcher resolves the +# live chain on every invocation via `specify preset script-chain`. They are +# generated (not shipped under scripts/bash) so projects initialized before +# this feature get the runner too, and shared-infra inventories are unchanged. +_SCRIPT_DISPATCHER_MARKER = "# speckit-generated: script continuation dispatcher" +_SCRIPT_RUNNER_MARKER = "# speckit-generated: script continuation runner" +_SCRIPT_RUNNER_NAME = "continuation-runner.sh" +# Names the generated runtime relies on: a dispatcher for ``common`` would +# overwrite the library every dispatcher sources, and one for +# ``continuation-runner`` would overwrite the runner itself. +_RESERVED_SCRIPT_NAMES = frozenset({"common", "continuation-runner"}) + +_SCRIPT_CONTINUATION_DISPATCHER_TEMPLATE = """#!/usr/bin/env bash +# speckit-generated: script continuation dispatcher +# Generated by specify for the "{script_name}" script. Do not edit directly; +# customize via presets/overrides instead (`specify preset add`). +set -e +set -o pipefail + +SCRIPT_DIR="$(CDPATH="" cd -- "$(dirname -- "${{BASH_SOURCE[0]}}")" && pwd)" +source "$SCRIPT_DIR/common.sh" + +REPO_ROOT=$(get_repo_root) +if command -v specify >/dev/null 2>&1; then + SPECIFY_CMD=(specify) +elif command -v python3 >/dev/null 2>&1; then + SPECIFY_CMD=(python3 -m specify_cli) +else + echo "ERROR: 'specify' is required to resolve the '{script_name}' script chain" >&2 + exit 1 +fi +# Strip carriage returns: on Windows the CLI emits CRLF, and $(...) only +# strips trailing newlines, so a stray CR would corrupt the exec path. +CHAIN=$( (cd "$REPO_ROOT" && "${{SPECIFY_CMD[@]}}" preset script-chain "{script_name}") | tr -d '\\r' ) || {{ + echo "ERROR: could not resolve the script chain for '{script_name}'" >&2 + exit 1 +}} + +TOP_LAYER=$(printf '%s\\n' "$CHAIN" | head -n 1) +REMAINING=$(printf '%s\\n' "$CHAIN" | tail -n +2) + +if [[ -n "$REMAINING" ]]; then + export SPECKIT_SCRIPT_CONTINUATION="$REMAINING" + export CORE_SCRIPT="$SCRIPT_DIR/{runner_name}" +fi + +# Run via bash so layers need no execute bit (preset copies keep source modes). +exec bash "$TOP_LAYER" "$@" +""" + +_SCRIPT_CONTINUATION_RUNNER = """#!/usr/bin/env bash +# speckit-generated: script continuation runner +# Advances the script continuation one hop: consumes the head of +# SPECKIT_SCRIPT_CONTINUATION (newline-delimited remaining layers, highest +# priority first) and runs it, re-exporting the remainder so a further +# "wrap" layer's own $CORE_SCRIPT call continues correctly. +# Portable to bash 3.2: no mapfile or associative arrays. +set -e + +if [[ -z "${SPECKIT_SCRIPT_CONTINUATION:-}" ]]; then + echo "ERROR: continuation runner invoked with no remaining script layers" >&2 + exit 1 +fi + +__speckit_remaining=() +while IFS= read -r __speckit_line; do + __speckit_line="${__speckit_line%$'\\r'}" + [[ -n "$__speckit_line" ]] && __speckit_remaining+=("$__speckit_line") +done <<< "$SPECKIT_SCRIPT_CONTINUATION" + +if [[ ${#__speckit_remaining[@]} -eq 0 ]]; then + echo "ERROR: continuation runner found no remaining script layers" >&2 + exit 1 +fi + +__speckit_next="${__speckit_remaining[0]}" + +if [[ ${#__speckit_remaining[@]} -gt 1 ]]; then + SPECKIT_SCRIPT_CONTINUATION=$(printf '%s\\n' "${__speckit_remaining[@]:1}") + export SPECKIT_SCRIPT_CONTINUATION +else + unset SPECKIT_SCRIPT_CONTINUATION +fi + +exec bash "$__speckit_next" "$@" +""" + + class PresetManager(_PresetCommandMethods, _PresetSkillMethods): """Manages preset lifecycle: installation, removal, updates.""" @@ -481,6 +572,29 @@ def install_from_directory( stacklevel=2, ) + # Scripts don't need the same repeated reconciliation as commands: + # once a script name's canonical file is a continuation dispatcher, + # it re-resolves the live stack on every invocation (#4551), so + # later priority/enablement changes take effect without touching + # this file again. Only install (here) and remove() ever need to + # write it. + script_names = [ + t["name"] + for t in manifest.templates + if t.get("type") == "script" + ] + for script_name in script_names: + try: + self._reconcile_script_chain(script_name) + except Exception as exc: + import warnings + warnings.warn( + f"Post-install script reconciliation failed for " + f"{manifest.id} script '{script_name}': {exc}. " + f"Run 'specify preset script-chain {script_name}' to diagnose.", + stacklevel=2, + ) + # TODO: constitution-sync is a named preset with core-owned side effects. # Give synchronization an explicit owner without changing its opt-in # behavior or overwriting authored constitutions. @@ -492,6 +606,78 @@ def install_from_directory( return manifest + def _reconcile_script_chain(self, script_name: str) -> None: + """Materialize the project's canonical bash script for ``script_name``. + + Scripts are executed rather than read, so composition is resolved at + *invocation* time (#4551): while any preset provides this script, the + canonical ``.specify/scripts/bash/.sh`` is a fixed dispatcher + that resolves the live chain via ``specify preset script-chain``. + Because it is used for every chain length (including one layer), later + enable/disable/set-priority changes never require rewriting it. When + no preset provides the script any more, the bundled core script is + restored (or the generated dispatcher removed if there is no core). + """ + if script_name in _RESERVED_SCRIPT_NAMES: + raise PresetValidationError( + f"Script name '{script_name}' is reserved for the generated " + f"script continuation runtime and cannot be provided by a preset." + ) + resolver = PresetResolver(self.project_root) + chain = resolver.resolve_script_chain(script_name) + scripts_dir = self.project_root / ".specify" / "scripts" / "bash" + canonical = scripts_dir / f"{script_name}.sh" + + # Validate every ancestor (not just the leaf): a symlinked ``.specify`` + # would otherwise redirect the writes and the cleanup unlink outside + # the project. + _ensure_safe_shared_directory(self.project_root, scripts_dir) + _ensure_safe_shared_destination(self.project_root, canonical) + runner = scripts_dir / _SCRIPT_RUNNER_NAME + _ensure_safe_shared_destination(self.project_root, runner) + + # Judge "provided by a preset" from every active declaration, not the + # truncated chain: an extension's replace layer can end the chain + # above a lower-priority preset, and a later set-priority or enable + # change must still find the dispatcher already in place. + preset_roots = ( + self.project_root / ".specify" / "presets", + self.project_root / ".specify" / "templates" / "overrides", + ) + provided_by_preset = bool(chain) and any( + root in layer["path"].parents + for layer in resolver.collect_all_layers(script_name, "script") + for root in preset_roots + ) + + if provided_by_preset: + _write_shared_text(self.project_root, runner, _SCRIPT_CONTINUATION_RUNNER) + _write_shared_text( + self.project_root, + canonical, + _SCRIPT_CONTINUATION_DISPATCHER_TEMPLATE.format( + script_name=script_name, runner_name=_SCRIPT_RUNNER_NAME + ), + ) + if os.name != "nt": + for generated in (runner, canonical): + generated.chmod(generated.stat().st_mode | 0o111) + return + + # No preset layer left: restore core, or drop a stale generated stub. + if chain: + _write_shared_text( + self.project_root, canonical, chain[-1].read_text(encoding="utf-8") + ) + # Command frontmatter executes this path directly, and + # _write_shared_text writes 0644. + if os.name != "nt": + canonical.chmod(canonical.stat().st_mode | 0o111) + elif canonical.is_file(): + first_line = canonical.read_text(encoding="utf-8").splitlines()[:2] + if any(_SCRIPT_DISPATCHER_MARKER in line for line in first_line): + canonical.unlink() + def _seed_constitution_from_preset( self, manifest: PresetManifest, preset_dir: Path ) -> None: @@ -702,6 +888,7 @@ def remove(self, pack_id: str) -> bool: # entirely and _unregister_skills would restore core/extension # content instead of a surviving lower-priority preset's override. removed_cmd_names = set() + removed_script_names = set() removed_constitution = any( path.exists() for path in ( @@ -740,6 +927,10 @@ def remove(self, pack_id: str) -> bool: for alias in tmpl.get("aliases", []): if isinstance(alias, str): removed_cmd_names.add(alias) + if tmpl.get("type") == "script": + name = tmpl.get("name") + if isinstance(name, str): + removed_script_names.add(name) except PresetValidationError: # Invalid manifest — skip alias extraction; primary command # names from registered_commands are still unregistered. @@ -881,6 +1072,19 @@ def remove(self, pack_id: str) -> bool: stacklevel=2, ) + if removed_script_names: + for script_name in removed_script_names: + try: + self._reconcile_script_chain(script_name) + except Exception as exc: + import warnings + warnings.warn( + f"Post-removal script reconciliation failed for " + f"{pack_id} script '{script_name}': {exc}. " + f"Run 'specify preset script-chain {script_name}' to diagnose.", + stacklevel=2, + ) + if removed_constitution: try: self._reconcile_constitution() diff --git a/src/specify_cli/presets/_resolver.py b/src/specify_cli/presets/_resolver.py index ea1c4e03f9..8630fe4b2a 100644 --- a/src/specify_cli/presets/_resolver.py +++ b/src/specify_cli/presets/_resolver.py @@ -682,6 +682,77 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: return layers + def resolve_script_chain(self, script_name: str) -> List[Path]: + """Return the ordered chain of files backing a script's continuation. + + Scripts are executed rather than merely read, so — unlike + templates and commands — their composition doesn't need to be + spliced into a single file ahead of time. A ``"wrap"`` script + contains a literal ``$CORE_SCRIPT`` reference that a runtime + continuation runner resolves hop by hop, so this returns the + stack of files that reference forms, in priority order, rather + than composed content. + + This walks the same priority stack as ``resolve_content()`` for + ``template_type="script"``: the highest-priority layer down + through the nearest layer with strategy ``"replace"`` + (inclusive), which terminates the chain — only ``"replace"`` and + ``"wrap"`` are valid script strategies, so a chain longer than + one entry always has a ``"wrap"`` top. Layers below the + terminating ``"replace"`` layer are never reachable and are + omitted, matching ``resolve_content()``. + + Returns an empty list when the script name has no layers, or + when none of them has strategy ``"replace"`` (composition has no + base to terminate on — the same condition under which + ``resolve_content()`` returns ``None``). + """ + layers = list(self.collect_all_layers(script_name, "script")) + if not any(layer["strategy"] == "replace" for layer in layers): + # collect_all_layers() only knows scripts/.sh; the real + # built-in Bash assets live under scripts/bash/. + bundled = self._find_bundled_bash_script(script_name) + if bundled is not None: + layers.append( + {"path": bundled, "source": "core (bundled)", "strategy": "replace"} + ) + if not layers: + return [] + base_idx = next( + (i for i, layer in enumerate(layers) if layer["strategy"] == "replace"), + None, + ) + if base_idx is None: + return [] + chain_layers = layers[: base_idx + 1] + for layer in chain_layers: + if layer["strategy"] != "wrap": + continue + try: + body = layer["path"].read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError) as exc: + raise PresetValidationError( + f"Cannot read wrap script '{layer['source']}': {exc}" + ) from exc + if "$CORE_SCRIPT" not in body: + raise PresetValidationError( + f"Wrap strategy in '{layer['source']}' is missing the " + f"$CORE_SCRIPT placeholder; executing it would silently " + f"drop every lower layer." + ) + return [layer["path"] for layer in chain_layers] + + def _find_bundled_bash_script(self, script_name: str) -> Optional[Path]: + """Locate the built-in Bash script from the core pack or source tree.""" + try: + from specify_cli import _locate_core_pack, _repo_root + except ImportError: + return None + core_pack = _locate_core_pack() + base = core_pack if core_pack is not None else _repo_root() + candidate = base / "scripts" / "bash" / f"{script_name}.sh" + return candidate if candidate.is_file() else None + def _find_bundled_core( self, template_name: str, diff --git a/src/specify_cli/presets/command_resolve.py b/src/specify_cli/presets/command_resolve.py index 7256307160..083cae7214 100644 --- a/src/specify_cli/presets/command_resolve.py +++ b/src/specify_cli/presets/command_resolve.py @@ -11,6 +11,49 @@ from ._commands import preset_app +@preset_app.command("script-chain", hidden=True) +def preset_script_chain( + script_name: str = typer.Argument( + ..., help="Script name to resolve (e.g., setup-plan)" + ), +): + """Print the resolved script continuation chain, one path per line. + + Internal command consumed by the generated script continuation + dispatcher and runner (see ``PresetResolver.resolve_script_chain``); + not intended for interactive use. Output is the ordered chain of + file paths from highest priority to the terminating "replace" layer, + one absolute path per line, with no other output. + """ + from .. import _require_specify_project + from . import PresetResolver, PresetValidationError + + if re.fullmatch(r"[a-z0-9-]+", script_name) is None: + typer.echo( + f"Error: invalid script name '{script_name}'; " + "use lowercase letters, digits, and hyphens", + err=True, + ) + raise typer.Exit(1) + + project_root = _require_specify_project() + resolver = PresetResolver(project_root) + try: + chain = resolver.resolve_script_chain(script_name) + except PresetValidationError as exc: + typer.echo(f"Error: {exc}", err=True) + raise typer.Exit(1) + if not chain: + typer.echo( + f"Error: could not resolve a script chain for '{script_name}'", + err=True, + ) + raise typer.Exit(1) + + for path in chain: + typer.echo(str(path)) + + @preset_app.command("resolve") def preset_resolve( template_name: str = typer.Argument( diff --git a/tests/specify_cli/presets/_helpers.py b/tests/specify_cli/presets/_helpers.py index 32cb132adb..365e31826a 100644 --- a/tests/specify_cli/presets/_helpers.py +++ b/tests/specify_cli/presets/_helpers.py @@ -252,3 +252,45 @@ def _create_multi_command_preset_with_aliases(self, temp_dir, preset_id, command with open(preset_dir / "preset.yml", "w") as f: yaml.dump(manifest_data, f) return preset_dir + + +def create_pack(temp_dir, valid_pack_data, pack_id, content, + strategy="replace", template_type="template", + template_name="spec-template"): + """Helper to create a preset pack directory.""" + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": pack_id, "name": pack_id} + + tmpl_entry = { + "type": template_type, + "name": template_name, + } + if template_type == "script": + tmpl_entry["file"] = f"scripts/{template_name}.sh" + elif template_type == "command": + tmpl_entry["file"] = f"commands/{template_name}.md" + else: + tmpl_entry["file"] = f"templates/{template_name}.md" + if strategy != "replace": + tmpl_entry["strategy"] = strategy + pack_data["provides"] = {"templates": [tmpl_entry]} + + pack_dir = temp_dir / pack_id + pack_dir.mkdir(exist_ok=True) + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + + if template_type == "script": + subdir = pack_dir / "scripts" + subdir.mkdir(exist_ok=True) + (subdir / f"{template_name}.sh").write_text(content) + elif template_type == "command": + subdir = pack_dir / "commands" + subdir.mkdir(exist_ok=True) + (subdir / f"{template_name}.md").write_text(content) + else: + subdir = pack_dir / "templates" + subdir.mkdir(exist_ok=True) + (subdir / f"{template_name}.md").write_text(content) + + return pack_dir diff --git a/tests/specify_cli/presets/test_manager.py b/tests/specify_cli/presets/test_manager.py index 91e0805593..57dc493c02 100644 --- a/tests/specify_cli/presets/test_manager.py +++ b/tests/specify_cli/presets/test_manager.py @@ -1,6 +1,9 @@ """Tests for preset installation and removal in specify_cli.presets._manager.""" import json +import os +import subprocess +import sys import tarfile import zipfile from pathlib import Path @@ -25,6 +28,7 @@ from tests.specify_cli.presets._helpers import ( make_convention_constitution_preset as _make_convention_constitution_preset, ) +from tests.specify_cli.presets._helpers import create_pack as _create_pack class TestPresetManifest: @@ -1607,3 +1611,234 @@ def test_remove_restores_lower_priority_command( cmd_files = list(gemini_dir.glob("*specify*")) assert cmd_files, "Command file should still exist after removal" assert "Lo content" in cmd_files[0].read_text() + + +class TestScriptChainReconciliation: + """Test PresetManager._reconcile_script_chain() (#4551). + + Verifies the canonical ``.specify/scripts/bash/.sh`` file that + agents actually invoke: a plain copy when there's nothing to + compose, and a fixed continuation dispatcher stub when there is. + """ + + def _canonical(self, project_dir, name): + return project_dir / ".specify" / "scripts" / "bash" / f"{name}.sh" + + def test_install_replace_script_uses_dispatcher_and_resolves_override( + self, project_dir, temp_dir, valid_pack_data + ): + """Even a single-layer override gets the dispatcher, so a later + priority/enable change that makes it a multi-layer chain needs no + rewrite of the canonical file.""" + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, + valid_pack_data, + "override-only", + "echo overridden\n", + strategy="replace", + template_type="script", + template_name="plain-override", + ) + manager.install_from_directory(pack_dir, "0.1.5") + + canonical = self._canonical(project_dir, "plain-override") + assert "speckit-generated: script continuation dispatcher" in canonical.read_text() + chain = PresetResolver(project_dir).resolve_script_chain("plain-override") + assert [p.read_text() for p in chain] == ["echo overridden\n"] + + def test_install_writes_runner_next_to_dispatcher( + self, project_dir, temp_dir, valid_pack_data + ): + """The runner is generated, not shipped, so pre-existing projects get it.""" + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, valid_pack_data, "runner-pack", "echo x\n", + template_type="script", template_name="needs-runner", + ) + manager.install_from_directory(pack_dir, "0.1.5") + runner = project_dir / ".specify" / "scripts" / "bash" / "continuation-runner.sh" + assert "script continuation runner" in runner.read_text() + + def test_remove_only_provider_removes_generated_dispatcher( + self, project_dir, temp_dir, valid_pack_data + ): + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, valid_pack_data, "solo-pack", "echo x\n", + template_type="script", template_name="no-core-script", + ) + manager.install_from_directory(pack_dir, "0.1.5") + canonical = self._canonical(project_dir, "no-core-script") + assert canonical.is_file() + manager.remove("solo-pack") + assert not canonical.exists() + + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX execute bits") + def test_remove_last_provider_restores_executable_core_script( + self, project_dir, temp_dir, valid_pack_data + ): + """Frontmatter runs the canonical path directly, so the restored core + script must stay executable after the dispatcher is replaced.""" + core_script = ( + project_dir / ".specify" / "templates" / "scripts" / "exec-restore.sh" + ) + core_script.parent.mkdir(parents=True, exist_ok=True) + core_script.write_text("echo core\n") + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, valid_pack_data, "exec-pack", "echo x\n", + strategy="wrap", template_type="script", template_name="exec-restore", + ) + manager.install_from_directory(pack_dir, "0.1.5") + manager.remove("exec-pack") + + canonical = self._canonical(project_dir, "exec-restore") + assert canonical.read_text() == "echo core\n" + assert canonical.stat().st_mode & 0o111 + + def test_reconcile_refuses_symlinked_destination( + self, project_dir, temp_dir, valid_pack_data + ): + import os + outside = temp_dir / "outside" + outside.mkdir() + scripts = project_dir / ".specify" / "scripts" + scripts.mkdir(parents=True) + try: + os.symlink(outside, scripts / "bash", target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("symlinks unavailable") + pack_dir = _create_pack( + temp_dir, valid_pack_data, "sym-pack", "echo x\n", + template_type="script", template_name="sym-script", + ) + with pytest.warns(UserWarning, match="symlink"): + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + assert list(outside.iterdir()) == [] + + def test_install_wrap_script_writes_dispatcher_stub( + self, project_dir, temp_dir, valid_pack_data + ): + core_script = ( + project_dir / ".specify" / "templates" / "scripts" / "stub-target.sh" + ) + core_script.parent.mkdir(parents=True, exist_ok=True) + core_script.write_text("echo core\n") + + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, + valid_pack_data, + "stub-pack", + "echo before\n$CORE_SCRIPT\necho after\n", + strategy="wrap", + template_type="script", + template_name="stub-target", + ) + manager.install_from_directory(pack_dir, "0.1.5") + + canonical = self._canonical(project_dir, "stub-target") + content = canonical.read_text() + assert 'preset script-chain "stub-target"' in content + assert "SPECKIT_SCRIPT_CONTINUATION" in content + assert "CORE_SCRIPT" in content + # The dispatcher carries no stack-specific data (no reference to + # "stub-pack" or the resolved core path) — it resolves fresh at + # every invocation instead. + assert "stub-pack" not in content + + @pytest.mark.parametrize("reserved", ["common", "continuation-runner"]) + def test_reserved_helper_names_are_refused(self, project_dir, reserved): + """A dispatcher named after a runtime helper would overwrite it.""" + manager = PresetManager(project_dir) + with pytest.raises(PresetValidationError, match="reserved"): + manager._reconcile_script_chain(reserved) + + def test_symlinked_specify_dir_is_not_written_through( + self, project_dir, temp_dir, valid_pack_data + ): + """A symlinked ancestor must not redirect the dispatcher writes.""" + outside = temp_dir / "outside" + (outside / "scripts" / "bash").mkdir(parents=True) + real_specify = project_dir / ".specify" + moved = temp_dir / "moved-specify" + real_specify.rename(moved) + try: + real_specify.symlink_to(outside, target_is_directory=True) + except (OSError, NotImplementedError): + real_specify.mkdir() + pytest.skip("symlinks unavailable on this platform") + manager = PresetManager(project_dir) + with pytest.raises(ValueError, match="symlink"): + manager._reconcile_script_chain("linked") + assert list((outside / "scripts" / "bash").iterdir()) == [] + + def test_dispatcher_written_when_extension_layer_ends_chain( + self, project_dir, temp_dir, valid_pack_data, monkeypatch + ): + """An extension replace layer above a preset truncates the chain, but + the preset is still an active declaration and needs the dispatcher so + a later priority change is not inert.""" + preset_file = ( + project_dir / ".specify" / "presets" / "p1" / "scripts" / "shadowed.sh" + ) + preset_file.parent.mkdir(parents=True) + preset_file.write_text("echo preset\n") + ext_file = temp_dir / "ext-shadowed.sh" + ext_file.write_text("echo ext\n") + monkeypatch.setattr( + PresetResolver, "resolve_script_chain", lambda self, name: [ext_file] + ) + monkeypatch.setattr( + PresetResolver, + "collect_all_layers", + lambda self, name, kind: [ + {"path": ext_file, "source": "extension", "strategy": "replace"}, + {"path": preset_file, "source": "preset", "strategy": "replace"}, + ], + ) + PresetManager(project_dir)._reconcile_script_chain("shadowed") + canonical = self._canonical(project_dir, "shadowed") + assert "script continuation dispatcher" in canonical.read_text() + + def test_python_module_entry_point_runs_cli(self): + """The dispatcher's ``python3 -m specify_cli`` fallback needs __main__.""" + import subprocess + import sys + + result = subprocess.run( + [sys.executable, "-m", "specify_cli", "preset", "script-chain", "--help"], + capture_output=True, + text=True, + encoding="utf-8", + ) + assert result.returncode == 0, result.stderr + + def test_remove_last_composing_preset_reverts_to_core_copy( + self, project_dir, temp_dir, valid_pack_data + ): + core_script = ( + project_dir / ".specify" / "templates" / "scripts" / "revert-me.sh" + ) + core_script.parent.mkdir(parents=True, exist_ok=True) + core_script.write_text("echo core\n") + + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, + valid_pack_data, + "revert-pack", + "echo before\n$CORE_SCRIPT\n", + strategy="wrap", + template_type="script", + template_name="revert-me", + ) + manager.install_from_directory(pack_dir, "0.1.5") + + canonical = self._canonical(project_dir, "revert-me") + assert "SPECKIT_SCRIPT_CONTINUATION" in canonical.read_text() + + manager.remove("revert-pack") + + assert canonical.read_text() == "echo core\n" diff --git a/tests/specify_cli/presets/test_registration.py b/tests/specify_cli/presets/test_registration.py index 0aa4d74b34..dc5110089a 100644 --- a/tests/specify_cli/presets/test_registration.py +++ b/tests/specify_cli/presets/test_registration.py @@ -13,6 +13,7 @@ def test_preset_commands_registered_once_in_stable_order(): "remove", "update", "search", + "script-chain", "resolve", "info", "set-priority", diff --git a/tests/specify_cli/presets/test_resolver.py b/tests/specify_cli/presets/test_resolver.py index d1e416f7a1..50d4b32e2c 100644 --- a/tests/specify_cli/presets/test_resolver.py +++ b/tests/specify_cli/presets/test_resolver.py @@ -17,6 +17,7 @@ CORE_TEMPLATE_NAMES, install_self_test_preset, ) +from tests.specify_cli.presets._helpers import create_pack as _create_pack class TestPresetResolver: @@ -1760,43 +1761,164 @@ def test_layers_read_strategy_from_manifest(self, project_dir, temp_dir, valid_p assert layers[1]["strategy"] == "replace" -def _create_pack(temp_dir, valid_pack_data, pack_id, content, - strategy="replace", template_type="template", - template_name="spec-template"): - """Helper to create a preset pack directory.""" - pack_data = {**valid_pack_data} - pack_data["preset"] = {**valid_pack_data["preset"], "id": pack_id, "name": pack_id} - - tmpl_entry = { - "type": template_type, - "name": template_name, - } - if template_type == "script": - tmpl_entry["file"] = f"scripts/{template_name}.sh" - elif template_type == "command": - tmpl_entry["file"] = f"commands/{template_name}.md" - else: - tmpl_entry["file"] = f"templates/{template_name}.md" - if strategy != "replace": - tmpl_entry["strategy"] = strategy - pack_data["provides"] = {"templates": [tmpl_entry]} - - pack_dir = temp_dir / pack_id - pack_dir.mkdir(exist_ok=True) - with open(pack_dir / "preset.yml", 'w') as f: - yaml.dump(pack_data, f) - - if template_type == "script": - subdir = pack_dir / "scripts" - subdir.mkdir(exist_ok=True) - (subdir / f"{template_name}.sh").write_text(content) - elif template_type == "command": - subdir = pack_dir / "commands" - subdir.mkdir(exist_ok=True) - (subdir / f"{template_name}.md").write_text(content) - else: - subdir = pack_dir / "templates" - subdir.mkdir(exist_ok=True) - (subdir / f"{template_name}.md").write_text(content) - - return pack_dir +class TestResolveScriptChain: + """Test PresetResolver.resolve_script_chain() (#4551). + + Unlike resolve_content(), which splices script content together + ahead of time, this returns the ordered *files* a runtime + continuation dispatcher walks hop by hop, so priority/enablement + changes take effect without re-splicing. + """ + + def test_missing_script_returns_empty(self, project_dir): + resolver = PresetResolver(project_dir) + assert resolver.resolve_script_chain("does-not-exist") == [] + + def test_single_core_layer(self, project_dir): + """A script with no overrides resolves to a one-entry chain.""" + core_script = ( + project_dir / ".specify" / "templates" / "scripts" / "solo-script.sh" + ) + core_script.parent.mkdir(parents=True, exist_ok=True) + core_script.write_text("echo core\n") + + resolver = PresetResolver(project_dir) + chain = resolver.resolve_script_chain("solo-script") + assert chain == [core_script] + + def test_wrap_over_core_orders_top_first( + self, project_dir, temp_dir, valid_pack_data + ): + core_script = ( + project_dir / ".specify" / "templates" / "scripts" / "wrapped.sh" + ) + core_script.parent.mkdir(parents=True, exist_ok=True) + core_script.write_text("echo core\n") + + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, + valid_pack_data, + "wrap-pack", + "echo before\n$CORE_SCRIPT\necho after\n", + strategy="wrap", + template_type="script", + template_name="wrapped", + ) + manager.install_from_directory(pack_dir, "0.1.5") + + resolver = PresetResolver(project_dir) + chain = resolver.resolve_script_chain("wrapped") + assert len(chain) == 2 + assert chain[0].read_text().startswith("echo before") + assert chain[1] == core_script + + def test_replace_layer_terminates_chain( + self, project_dir, temp_dir, valid_pack_data + ): + """A "replace" layer wins outright; nothing below it is reachable.""" + core_script = ( + project_dir / ".specify" / "templates" / "scripts" / "overridden.sh" + ) + core_script.parent.mkdir(parents=True, exist_ok=True) + core_script.write_text("echo core\n") + + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, + valid_pack_data, + "replace-pack", + "echo replaced\n", + strategy="replace", + template_type="script", + template_name="overridden", + ) + manager.install_from_directory(pack_dir, "0.1.5") + + resolver = PresetResolver(project_dir) + chain = resolver.resolve_script_chain("overridden") + assert len(chain) == 1 + assert chain[0].read_text() == "echo replaced\n" + + def test_no_replace_base_returns_empty(self, project_dir, temp_dir, valid_pack_data): + """A wrap-only stack with no core/replace layer can't terminate.""" + manager = PresetManager(project_dir) + pack_dir = _create_pack( + temp_dir, + valid_pack_data, + "dangling-wrap", + "echo before\n$CORE_SCRIPT\n", + strategy="wrap", + template_type="script", + template_name="dangling", + ) + manager.install_from_directory(pack_dir, "0.1.5") + + resolver = PresetResolver(project_dir) + assert resolver.resolve_script_chain("dangling") == [] + + def test_priority_change_reorders_chain_without_reinstall( + self, project_dir, temp_dir, valid_pack_data + ): + """Changing priority alone (no reinstall) must reorder the next + resolve_script_chain() call — this is the property the runtime + dispatcher relies on to avoid re-materializing on every + enable/disable/set-priority change.""" + core_script = ( + project_dir / ".specify" / "templates" / "scripts" / "reorder-me.sh" + ) + core_script.parent.mkdir(parents=True, exist_ok=True) + core_script.write_text("echo core\n") + + manager = PresetManager(project_dir) + for pid, prio in [("layer-a", 5), ("layer-b", 10)]: + pack_dir = _create_pack( + temp_dir, + valid_pack_data, + pid, + f"echo {pid} before\n$CORE_SCRIPT\necho {pid} after\n", + strategy="wrap", + template_type="script", + template_name="reorder-me", + ) + manager.install_from_directory(pack_dir, "0.1.5", priority=prio) + + resolver = PresetResolver(project_dir) + chain_before = resolver.resolve_script_chain("reorder-me") + assert "layer-a" in str(chain_before[0]) + + manager.registry.update("layer-a", {"priority": 20}) + + chain_after = resolver.resolve_script_chain("reorder-me") + assert "layer-b" in str(chain_after[0]) + assert "layer-a" in str(chain_after[1]) + + + def test_wrap_missing_core_script_placeholder_is_rejected( + self, project_dir, temp_dir, valid_pack_data + ): + core = project_dir / ".specify" / "templates" / "scripts" / "bad-wrap.sh" + core.parent.mkdir(parents=True, exist_ok=True) + core.write_text("echo core\n") + pack_dir = _create_pack( + temp_dir, valid_pack_data, "bad-wrap-pack", "echo no placeholder\n", + strategy="wrap", template_type="script", template_name="bad-wrap", + ) + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + with pytest.raises(PresetValidationError, match="CORE_SCRIPT"): + PresetResolver(project_dir).resolve_script_chain("bad-wrap") + + def test_builtin_bash_script_is_found_as_core_base( + self, project_dir, temp_dir, valid_pack_data + ): + """A wrap over a real built-in (scripts/bash/setup-plan.sh) must + resolve without a fabricated .specify/templates/scripts core.""" + pack_dir = _create_pack( + temp_dir, valid_pack_data, "real-wrap", "echo a\n$CORE_SCRIPT\n", + strategy="wrap", template_type="script", template_name="setup-plan", + ) + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + chain = PresetResolver(project_dir).resolve_script_chain("setup-plan") + assert len(chain) == 2 + assert chain[1].name == "setup-plan.sh" + assert chain[1].parent.name == "bash" diff --git a/tests/test_script_continuation_bash.py b/tests/test_script_continuation_bash.py new file mode 100644 index 0000000000..6471a8557e --- /dev/null +++ b/tests/test_script_continuation_bash.py @@ -0,0 +1,194 @@ +"""End-to-end bash test for the script continuation dispatcher (#4551). + +Proves the runtime chain actually executes correctly, not just that the +resolver computes the right file list: installs two "wrap" script +presets over a core script, invokes the *canonical* materialized script +exactly as a coding agent would (via its fixed frontmatter path), and +checks the process actually ran outer-before -> inner-before -> core -> +inner-after -> outer-after, with args and exit status propagated. +""" + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +from specify_cli.presets import PresetManager + +from tests.conftest import requires_bash + +PROJECT_ROOT = Path(__file__).resolve().parent.parent +COMMON_SH = PROJECT_ROOT / "scripts" / "bash" / "common.sh" + +# The `specify` console script must be resolvable from the bash +# subprocess's PATH, since the dispatcher shells out to `specify preset +# script-chain`. Resolve it relative to the running interpreter rather +# than assuming a particular PATH setup, so this works the same way in a +# venv, in CI, or via `pip install -e .` locally. +_BIN_DIR = str(Path(sys.executable).parent) + + +def _clean_env() -> dict: + env = os.environ.copy() + for key in list(env): + if key.startswith("SPECIFY_"): + env.pop(key) + env["PATH"] = _BIN_DIR + os.pathsep + env.get("PATH", "") + return env + + +@pytest.fixture +def project_dir(tmp_path: Path) -> Path: + project = tmp_path / "project" + (project / ".specify" / "templates" / "scripts").mkdir(parents=True) + (project / ".specify" / "scripts" / "bash").mkdir(parents=True) + shutil.copy(COMMON_SH, project / ".specify" / "scripts" / "bash" / "common.sh") + return project + + +def _install_wrap_layer( + project_dir: Path, + temp_dir: Path, + pack_id: str, + priority: int, + script_name: str, + label: str, +) -> None: + pack_dir = temp_dir / pack_id + (pack_dir / "scripts").mkdir(parents=True) + (pack_dir / "scripts" / f"{script_name}.sh").write_text( + "#!/usr/bin/env bash\n" + "set -e\n" + f'echo "{label}-before $*"\n' + '"$CORE_SCRIPT" "$@"\n' + f'echo "{label}-after"\n' + ) + manifest = { + "schema_version": "1.0", + "preset": { + "id": pack_id, + "name": pack_id, + "version": "0.1.0", + "description": "test", + "author": "Test Author", + "repository": "https://github.com/test/test-pack", + "license": "MIT", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "script", + "name": script_name, + "file": f"scripts/{script_name}.sh", + "strategy": "wrap", + } + ] + }, + } + (pack_dir / "preset.yml").write_text(yaml.safe_dump(manifest), encoding="utf-8") + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0", priority=priority) + + +@requires_bash +def test_two_layer_continuation_runs_in_priority_order( + project_dir: Path, tmp_path: Path +) -> None: + core_script = project_dir / ".specify" / "templates" / "scripts" / "chained.sh" + core_script.write_text( + '#!/usr/bin/env bash\necho "core $*"\n' + ) + + temp_dir = tmp_path / "packs" + temp_dir.mkdir() + # priority 1 (outer, checked first) and priority 5 (inner) + _install_wrap_layer(project_dir, temp_dir, "outer-pack", 1, "chained", "outer") + _install_wrap_layer(project_dir, temp_dir, "inner-pack", 5, "chained", "inner") + + canonical = project_dir / ".specify" / "scripts" / "bash" / "chained.sh" + assert canonical.is_file(), "install should have written the dispatcher stub" + + result = subprocess.run( + ["bash", str(canonical), "arg1", "arg2"], + cwd=project_dir, + capture_output=True, + text=True, + check=False, + env=_clean_env(), + ) + assert result.returncode == 0, result.stderr + result.stdout + lines = [line for line in result.stdout.splitlines() if line.strip()] + assert lines == [ + "outer-before arg1 arg2", + "inner-before arg1 arg2", + "core arg1 arg2", + "inner-after", + "outer-after", + ] + + +@requires_bash +def test_priority_change_takes_effect_without_reinstall( + project_dir: Path, tmp_path: Path +) -> None: + """The whole point of the continuation model (#4551): swapping + priority must change execution order on the *next invocation*, + without touching the canonical file at all.""" + core_script = project_dir / ".specify" / "templates" / "scripts" / "reorder.sh" + core_script.write_text('#!/usr/bin/env bash\necho "core"\n') + + temp_dir = tmp_path / "packs" + temp_dir.mkdir() + _install_wrap_layer(project_dir, temp_dir, "layer-a", 1, "reorder", "a") + _install_wrap_layer(project_dir, temp_dir, "layer-b", 5, "reorder", "b") + + canonical = project_dir / ".specify" / "scripts" / "bash" / "reorder.sh" + before_bytes = canonical.read_bytes() + + manager = PresetManager(project_dir) + manager.registry.update("layer-a", {"priority": 20}) + + # The dispatcher file itself must be byte-for-byte unchanged — + # reordering must not require rewriting it. + assert canonical.read_bytes() == before_bytes + + result = subprocess.run( + ["bash", str(canonical)], + cwd=project_dir, + capture_output=True, + text=True, + check=False, + env=_clean_env(), + ) + assert result.returncode == 0, result.stderr + result.stdout + lines = [line for line in result.stdout.splitlines() if line.strip()] + assert lines == ["b-before ", "a-before ", "core", "a-after", "b-after"] + + +@requires_bash +def test_nonzero_exit_status_propagates_through_the_chain( + project_dir: Path, tmp_path: Path +) -> None: + core_script = project_dir / ".specify" / "templates" / "scripts" / "failing.sh" + core_script.write_text('#!/usr/bin/env bash\nexit 7\n') + + temp_dir = tmp_path / "packs" + temp_dir.mkdir() + _install_wrap_layer(project_dir, temp_dir, "wrap-pack", 1, "failing", "w") + + canonical = project_dir / ".specify" / "scripts" / "bash" / "failing.sh" + result = subprocess.run( + ["bash", str(canonical)], + cwd=project_dir, + capture_output=True, + text=True, + check=False, + env=_clean_env(), + ) + assert result.returncode == 7