From e354ed1cc427cde5702ce3a30d72192d70fbfde0 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:23:07 +0100 Subject: [PATCH 01/13] feat(preset): add update command for replacing installed presets Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) --- docs/reference/presets.md | 27 +++++++ src/specify_cli/presets/_commands.py | 72 +++++++++++++++++ tests/test_presets.py | 112 ++++++++++++++++++++++++++- 3 files changed, 210 insertions(+), 1 deletion(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 0723200d67..3fd6a094bf 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -31,6 +31,33 @@ Installs a preset from the catalog, a URL, or a local directory. Preset commands > **Note:** All preset commands require a project already initialized with `specify init`. +## Update a Preset + +```bash +specify preset update [--from ] [--dev ] [--priority ] +``` + +Replaces an already-installed preset by running the normal `preset remove` +operation first and then the normal `preset add` operation. `--from`, `--dev`, +and `--priority` are forwarded to `preset add`; `--from` and `--dev` cannot be +combined. Without an explicit source, add resolves the preset through its usual +bundled and catalogue lookup. + +Update is deliberately destructive. If removal succeeds but replacement +installation fails, the previous preset has already been removed. The command +reports a copy-pastable `specify preset add` retry command, including the +replacement source and priority. There is no pre-flight validation, version +comparison, staging, rollback, or automatic recovery. A missing or invalid +replacement source can therefore leave the preset removed. + +A successful update follows normal remove and add behaviour: it re-enables the +preset, recreates `installed_at`, removes local modifications tracked by the +preset, and treats an explicit `--from` or `--dev` as an intentional source +change. If constitution synchronisation is enabled, both normal reconciliation +passes run. The generated-file guard still protects a hand-edited +`.specify/memory/constitution.md`; no update-specific constitution optimisation +is applied. + ## Remove a Preset ```bash diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index c5dbf0ddca..ee1d3c212c 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -10,6 +10,7 @@ import os import re +import shlex from pathlib import Path import typer @@ -454,6 +455,77 @@ def preset_remove( raise typer.Exit(1) +@preset_app.command("update") +def preset_update( + preset_id: str = typer.Argument(..., help="Installed preset ID to replace"), + from_url: str = typer.Option( + None, + "--from", + help="Install the replacement from a .zip, .tar.gz, or .tgz URL", + ), + dev: str = typer.Option( + None, + "--dev", + help="Install the replacement from a local directory (development mode)", + ), + priority: int = typer.Option( + 10, + "--priority", + help="Resolution priority for the replacement (default 10)", + ), +): + """Replace an installed preset using the normal remove and add flows.""" + from .. import _require_specify_project + from . import PresetManager + + if from_url and dev: + console.print("[red]Error:[/red] --from and --dev are mutually exclusive") + raise typer.Exit(1) + + project_root = _require_specify_project() + manager = PresetManager(project_root) + if not manager.registry.is_installed(preset_id): + console.print(f"[red]Error:[/red] Preset '{preset_id}' is not installed") + raise typer.Exit(1) + + # Keep update deliberately destructive: remove performs its complete normal + # reconciliation before add resolves and installs the replacement. + preset_remove(preset_id) + + retry_args = ["specify", "preset", "add", preset_id] + if from_url: + retry_args.extend(["--from", from_url]) + if dev: + retry_args.extend(["--dev", dev]) + retry_args.extend(["--priority", str(priority)]) + + def report_add_failure() -> None: + console.print( + "[red]Error:[/red] Preset update failed; the previous preset was removed." + ) + console.print( + "Retry with: [cyan]" + f"{_escape_markup(shlex.join(retry_args))}" + "[/cyan]", + soft_wrap=True, + ) + + try: + preset_add( + preset_id=preset_id, + from_url=from_url, + dev=dev, + priority=priority, + ) + except typer.Exit as error: + report_add_failure() + raise typer.Exit(error.exit_code or 1) + except Exception as error: + console.print(f"[red]Error:[/red] {_escape_markup(str(error))}") + report_add_failure() + raise typer.Exit(1) + + @preset_app.command("search") def preset_search( query: str = typer.Argument(None, help="Search query"), diff --git a/tests/test_presets.py b/tests/test_presets.py index 17aef20dce..43ab87ea44 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -25,6 +25,7 @@ from unittest.mock import MagicMock import yaml +import typer from tests.conftest import strip_ansi from specify_cli.presets import ( @@ -41,7 +42,10 @@ ) from specify_cli.extensions import ExtensionRegistry from specify_cli._console import console -from specify_cli.presets._commands import _warn_unmet_extension_dependencies +from specify_cli.presets._commands import ( + _warn_unmet_extension_dependencies, + preset_update, +) # ===== Fixtures ===== @@ -11484,6 +11488,112 @@ def test_lean_overrides_commands(self, project_dir): assert result is not None, f"Lean override for {name} not resolved" +# ===== Preset Update Command Tests ===== + + +class TestPresetUpdateCommand: + """Test the destructive remove-then-add update contract.""" + + @staticmethod + def _manager(monkeypatch, project_dir, installed=True): + from specify_cli.presets import _commands as commands + + registry = SimpleNamespace(is_installed=lambda _preset_id: installed) + manager = SimpleNamespace(registry=registry) + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + monkeypatch.setattr("specify_cli.presets.PresetManager", lambda _root: manager) + return commands + + def test_unknown_preset_fails_without_remove_or_add(self, project_dir, monkeypatch): + commands = self._manager(monkeypatch, project_dir, installed=False) + calls = [] + monkeypatch.setattr(commands, "preset_remove", lambda *_args: calls.append("remove")) + monkeypatch.setattr(commands, "preset_add", lambda **_kwargs: calls.append("add")) + + with pytest.raises(typer.Exit) as exc_info: + preset_update("missing") + + assert exc_info.value.exit_code == 1 + assert calls == [] + + def test_mutually_exclusive_sources_are_rejected(self, project_dir, monkeypatch): + self._manager(monkeypatch, project_dir) + with pytest.raises(typer.Exit) as exc_info: + preset_update("test-pack", from_url="https://example.com/preset.zip", dev="./preset") + assert exc_info.value.exit_code == 1 + + def test_remove_failure_prevents_add(self, project_dir, monkeypatch): + commands = self._manager(monkeypatch, project_dir) + calls = [] + + def fail_remove(_preset_id): + calls.append("remove") + raise typer.Exit(1) + + monkeypatch.setattr(commands, "preset_remove", fail_remove) + monkeypatch.setattr(commands, "preset_add", lambda **_kwargs: calls.append("add")) + + with pytest.raises(typer.Exit) as exc_info: + preset_update("test-pack", from_url=None, dev=None) + + assert exc_info.value.exit_code == 1 + assert calls == ["remove"] + + def test_update_forwards_id_sources_and_priority_to_add(self, project_dir, monkeypatch): + commands = self._manager(monkeypatch, project_dir) + calls = [] + monkeypatch.setattr(commands, "preset_remove", lambda preset_id: calls.append(("remove", preset_id))) + monkeypatch.setattr( + commands, + "preset_add", + lambda **kwargs: calls.append(("add", kwargs)), + ) + + preset_update( + "test-pack", + from_url="https://example.com/replacement.zip", + dev=None, + priority=4, + ) + + assert calls == [ + ("remove", "test-pack"), + ( + "add", + { + "preset_id": "test-pack", + "from_url": "https://example.com/replacement.zip", + "dev": None, + "priority": 4, + }, + ), + ] + + def test_add_failure_states_removed_and_prints_retry_command( + self, project_dir, monkeypatch, capsys + ): + commands = self._manager(monkeypatch, project_dir) + monkeypatch.setattr(commands, "preset_remove", lambda _preset_id: None) + + def fail_add(**_kwargs): + raise typer.Exit(1) + + monkeypatch.setattr(commands, "preset_add", fail_add) + + with pytest.raises(typer.Exit) as exc_info: + preset_update( + "test-pack", + from_url=None, + dev="/tmp/replacement preset", + priority=6, + ) + + assert exc_info.value.exit_code == 1 + output = strip_ansi(capsys.readouterr().out) + assert "previous preset was removed" in output + assert "specify preset add test-pack --dev '/tmp/replacement preset' --priority 6" in output + + # ===== Bundled Preset Locator Tests ===== From 424bb5c0545e98ea762f3392eacc2acf2877c359 Mon Sep 17 00:00:00 2001 From: Damien Hardy <49808393+digimangos@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:01:09 +0100 Subject: [PATCH 02/13] Correct 'catalogue' to 'catalog' in presets.md Assisted-by: GitHub Copilot Autofix (autonomous) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- docs/reference/presets.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 3fd6a094bf..0e729ada9b 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -41,7 +41,7 @@ Replaces an already-installed preset by running the normal `preset remove` operation first and then the normal `preset add` operation. `--from`, `--dev`, and `--priority` are forwarded to `preset add`; `--from` and `--dev` cannot be combined. Without an explicit source, add resolves the preset through its usual -bundled and catalogue lookup. +bundled and catalog lookup. Update is deliberately destructive. If removal succeeds but replacement installation fails, the previous preset has already been removed. The command From 502c32e8be66263fa40fa87ec087b9c632a07808 Mon Sep 17 00:00:00 2001 From: Damien Hardy <49808393+digimangos@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:02:14 +0100 Subject: [PATCH 03/13] Refactor error reporting to use subprocess for args Assisted-by: GitHub Copilot Autofix (autonomous) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/specify_cli/presets/_commands.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index ee1d3c212c..71f96c364b 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -500,12 +500,19 @@ def preset_update( retry_args.extend(["--priority", str(priority)]) def report_add_failure() -> None: + import subprocess + + rendered_args = ( + subprocess.list2cmdline(retry_args) + if os.name == "nt" + else shlex.join(retry_args) + ) console.print( "[red]Error:[/red] Preset update failed; the previous preset was removed." ) console.print( "Retry with: [cyan]" - f"{_escape_markup(shlex.join(retry_args))}" + f"{_escape_markup(rendered_args)}" "[/cyan]", soft_wrap=True, ) From 8b54c8ea10e9e5f4f19b8c8c5fb23db64b62332c Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:19:23 +0100 Subject: [PATCH 04/13] docs(preset): document update identity limitation Document that update does not preflight-check replacement manifest IDs for --from and --dev sources. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 29b77f74-30d5-4969-981b-07a05607e971 --- docs/reference/presets.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 0e729ada9b..1b1cc78289 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -48,7 +48,10 @@ installation fails, the previous preset has already been removed. The command reports a copy-pastable `specify preset add` retry command, including the replacement source and priority. There is no pre-flight validation, version comparison, staging, rollback, or automatic recovery. A missing or invalid -replacement source can therefore leave the preset removed. +replacement source can therefore leave the preset removed. With `--from` or +`--dev`, the replacement manifest's `preset.id` is not checked against the +requested ID before removal; a source declaring a different ID may therefore +install a different preset after the requested one has been removed. A successful update follows normal remove and add behaviour: it re-enables the preset, recreates `installed_at`, removes local modifications tracked by the From ede9c1b351aa997950397172d4e5f073ac9d36e7 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:29:50 +0100 Subject: [PATCH 05/13] test(preset): make retry assertion platform-aware Use the same command-line rendering rules as preset update so the retry-command test passes on Windows and POSIX systems. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 29b77f74-30d5-4969-981b-07a05607e971 --- tests/test_presets.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/test_presets.py b/tests/test_presets.py index 43ab87ea44..305d872e03 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -13,6 +13,9 @@ import pytest import io import json +import os +import shlex +import subprocess import tempfile import tarfile import shutil @@ -11591,7 +11594,22 @@ def fail_add(**_kwargs): assert exc_info.value.exit_code == 1 output = strip_ansi(capsys.readouterr().out) assert "previous preset was removed" in output - assert "specify preset add test-pack --dev '/tmp/replacement preset' --priority 6" in output + retry_args = [ + "specify", + "preset", + "add", + "test-pack", + "--dev", + "/tmp/replacement preset", + "--priority", + "6", + ] + expected = ( + subprocess.list2cmdline(retry_args) + if os.name == "nt" + else shlex.join(retry_args) + ) + assert expected in output # ===== Bundled Preset Locator Tests ===== From fa70afbeebfe3845f0dcdea478ed656b6b6b9901 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:32:53 +0100 Subject: [PATCH 06/13] fix(preset): validate priority before update removes the preset specify preset update foo --priority 0 removed foo before add rejected the priority, leaving the preset gone and printing a retry command that carried the same rejected value, so the retry could never succeed. Validate priority before preset_remove runs. Extract the check into _validate_priority so add, set-priority, and update cannot drift apart on the accepted range or the message they print. The substitution in add and set-priority is behaviour-preserving: same condition, same message, same exit code. Also pass explicit option values in the direct-call update tests. Two of them previously omitted from_url/dev, leaving Typer OptionInfo defaults in place; because those objects are truthy, the tests exercised the mutually-exclusive branch rather than the path they described. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 29b77f74-30d5-4969-981b-07a05607e971 --- docs/reference/presets.md | 21 ++++++++------- src/specify_cli/presets/_commands.py | 36 +++++++++++++++++++++----- tests/test_presets.py | 38 +++++++++++++++++++++++++--- 3 files changed, 77 insertions(+), 18 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 1b1cc78289..144dd1053f 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -43,15 +43,18 @@ and `--priority` are forwarded to `preset add`; `--from` and `--dev` cannot be combined. Without an explicit source, add resolves the preset through its usual bundled and catalog lookup. -Update is deliberately destructive. If removal succeeds but replacement -installation fails, the previous preset has already been removed. The command -reports a copy-pastable `specify preset add` retry command, including the -replacement source and priority. There is no pre-flight validation, version -comparison, staging, rollback, or automatic recovery. A missing or invalid -replacement source can therefore leave the preset removed. With `--from` or -`--dev`, the replacement manifest's `preset.id` is not checked against the -requested ID before removal; a source declaring a different ID may therefore -install a different preset after the requested one has been removed. +Update is deliberately destructive. Arguments are checked before anything is +removed: `--from` and `--dev` cannot be combined, `--priority` must be 1 or +higher, and the preset must already be installed. Beyond those checks the +replacement source itself is not inspected in advance. If removal succeeds but +replacement installation fails, the previous preset has already been removed. +The command reports a copy-pastable `specify preset add` retry command, +including the replacement source and priority. There is no source pre-flight, +version comparison, staging, rollback, or automatic recovery. A missing or +invalid replacement source can therefore leave the preset removed. With +`--from` or `--dev`, the replacement manifest's `preset.id` is not checked +against the requested ID before removal; a source declaring a different ID may +therefore install a different preset after the requested one has been removed. A successful update follows normal remove and add behaviour: it re-enables the preset, recreates `installed_at`, removes local modifications tracked by the diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 71f96c364b..30fd73b1ec 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -48,6 +48,29 @@ preset_app.add_typer(preset_catalog_app, name="catalog") +#: Lowest priority a user may request. Lower numbers win resolution, so the +#: stack is anchored at 1 rather than 0 to leave no unreachable slot above the +#: highest-precedence preset. +MINIMUM_PRESET_PRIORITY = 1 + + +def _validate_priority(priority: int) -> None: + """Reject a non-positive priority before any destructive work begins. + + Shared by add, set-priority, and update so the three commands cannot drift + apart on the accepted range or the message they print. update in particular + must call this *before* removing the installed preset: validating only + inside add would leave the preset removed and print a retry command + carrying the same rejected priority. + """ + if priority < MINIMUM_PRESET_PRIORITY: + console.print( + "[red]Error:[/red] Priority must be a positive integer " + f"({MINIMUM_PRESET_PRIORITY} or higher)" + ) + raise typer.Exit(1) + + def _warn_unmet_extension_dependencies(manager, manifest) -> None: """Warn when a preset's declared extension dependencies are unsatisfied. @@ -241,9 +264,7 @@ def preset_add( project_root = _require_specify_project() # Validate priority - if priority < 1: - console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") - raise typer.Exit(1) + _validate_priority(priority) manager = PresetManager(project_root) speckit_version = get_speckit_version() @@ -482,6 +503,11 @@ def preset_update( console.print("[red]Error:[/red] --from and --dev are mutually exclusive") raise typer.Exit(1) + # Validate priority before removal. add rejects the same range, but only + # after remove has already run, which would leave the preset removed and + # the printed retry command carrying the rejected priority. + _validate_priority(priority) + project_root = _require_specify_project() manager = PresetManager(project_root) if not manager.registry.is_installed(preset_id): @@ -777,9 +803,7 @@ def preset_set_priority( project_root = _require_specify_project() # Validate priority - if priority < 1: - console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") - raise typer.Exit(1) + _validate_priority(priority) manager = PresetManager(project_root) diff --git a/tests/test_presets.py b/tests/test_presets.py index 305d872e03..5f01381735 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -11514,7 +11514,7 @@ def test_unknown_preset_fails_without_remove_or_add(self, project_dir, monkeypat monkeypatch.setattr(commands, "preset_add", lambda **_kwargs: calls.append("add")) with pytest.raises(typer.Exit) as exc_info: - preset_update("missing") + preset_update("missing", from_url=None, dev=None, priority=10) assert exc_info.value.exit_code == 1 assert calls == [] @@ -11522,7 +11522,12 @@ def test_unknown_preset_fails_without_remove_or_add(self, project_dir, monkeypat def test_mutually_exclusive_sources_are_rejected(self, project_dir, monkeypatch): self._manager(monkeypatch, project_dir) with pytest.raises(typer.Exit) as exc_info: - preset_update("test-pack", from_url="https://example.com/preset.zip", dev="./preset") + preset_update( + "test-pack", + from_url="https://example.com/preset.zip", + dev="./preset", + priority=10, + ) assert exc_info.value.exit_code == 1 def test_remove_failure_prevents_add(self, project_dir, monkeypatch): @@ -11537,7 +11542,7 @@ def fail_remove(_preset_id): monkeypatch.setattr(commands, "preset_add", lambda **_kwargs: calls.append("add")) with pytest.raises(typer.Exit) as exc_info: - preset_update("test-pack", from_url=None, dev=None) + preset_update("test-pack", from_url=None, dev=None, priority=10) assert exc_info.value.exit_code == 1 assert calls == ["remove"] @@ -11611,6 +11616,33 @@ def fail_add(**_kwargs): ) assert expected in output + def test_invalid_priority_rejected_before_removal( + self, project_dir, monkeypatch, capsys + ): + """--priority 0 must fail without removing the installed preset. + + add rejects the same range, but only after remove has run. Validating + late would delete the preset and print a retry command carrying the + rejected priority, so the retry could never succeed. + """ + commands = self._manager(monkeypatch, project_dir) + calls = [] + monkeypatch.setattr( + commands, "preset_remove", lambda preset_id: calls.append("remove") + ) + monkeypatch.setattr( + commands, "preset_add", lambda **_kwargs: calls.append("add") + ) + + with pytest.raises(typer.Exit) as exc_info: + preset_update("test-pack", from_url=None, dev=None, priority=0) + + assert exc_info.value.exit_code == 1 + assert calls == [] + output = strip_ansi(capsys.readouterr().out) + assert "Priority must be a positive integer" in output + assert "previous preset was removed" not in output + # ===== Bundled Preset Locator Tests ===== From 819c9b2a15b546e022e3bd272c8b66e6dec8718c Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:53:16 +0100 Subject: [PATCH 07/13] docs(preset): document constitution limitations for failed updates State that a failed add after removal can leave a generated constitution reconciled against the stack without the removed preset, and that the normal remove and add passes may rewrite it even when the final resolved content is unchanged. Assisted-by: GitHub Copilot (model: claude-opus-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/reference/presets.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 144dd1053f..10a4f2ffa5 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -60,9 +60,12 @@ A successful update follows normal remove and add behaviour: it re-enables the preset, recreates `installed_at`, removes local modifications tracked by the preset, and treats an explicit `--from` or `--dev` as an intentional source change. If constitution synchronisation is enabled, both normal reconciliation -passes run. The generated-file guard still protects a hand-edited -`.specify/memory/constitution.md`; no update-specific constitution optimisation -is applied. +passes run. If add fails after removal, a generated constitution may remain +reconciled against the stack without the removed preset. The generated-file +guard still protects a hand-edited `.specify/memory/constitution.md`. No +update-specific constitution optimisation is applied, so the normal remove and +add passes may rewrite the generated constitution even when the final resolved +content is unchanged. ## Remove a Preset From edafd3734ce73507c8f515a129810e6547f842b8 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:56:19 +0100 Subject: [PATCH 08/13] docs(preset): use American English terminology Correct documentation spelling to behavior, synchronization, and optimization. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/reference/presets.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 10a4f2ffa5..0bd77443e9 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -56,14 +56,14 @@ invalid replacement source can therefore leave the preset removed. With against the requested ID before removal; a source declaring a different ID may therefore install a different preset after the requested one has been removed. -A successful update follows normal remove and add behaviour: it re-enables the +A successful update follows normal remove and add behavior: it re-enables the preset, recreates `installed_at`, removes local modifications tracked by the preset, and treats an explicit `--from` or `--dev` as an intentional source -change. If constitution synchronisation is enabled, both normal reconciliation +change. If constitution synchronization is enabled, both normal reconciliation passes run. If add fails after removal, a generated constitution may remain reconciled against the stack without the removed preset. The generated-file guard still protects a hand-edited `.specify/memory/constitution.md`. No -update-specific constitution optimisation is applied, so the normal remove and +update-specific constitution optimization is applied, so the normal remove and add passes may rewrite the generated constitution even when the final resolved content is unchanged. From 6f6964a72028ae01efeaeec7cbe613d6be652f93 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:04:37 +0100 Subject: [PATCH 09/13] test(preset): cover update replacement workflow Add end-to-end coverage for successful preset replacement and failed replacement retry recovery. Update the preset documentation to describe manifest-diff limitations. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/reference/presets.md | 5 +- .../test_preset_update_workflow.py | 217 ++++++++++++++++++ 2 files changed, 220 insertions(+), 2 deletions(-) create mode 100644 tests/integration/test_preset_update_workflow.py diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 0bd77443e9..8b06ea59e2 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -50,8 +50,9 @@ replacement source itself is not inspected in advance. If removal succeeds but replacement installation fails, the previous preset has already been removed. The command reports a copy-pastable `specify preset add` retry command, including the replacement source and priority. There is no source pre-flight, -version comparison, staging, rollback, or automatic recovery. A missing or -invalid replacement source can therefore leave the preset removed. With +version comparison, manifest diff, staging, rollback, automatic repair, or +recovery transaction. A missing or invalid replacement source can therefore +leave the preset removed. With `--from` or `--dev`, the replacement manifest's `preset.id` is not checked against the requested ID before removal; a source declaring a different ID may therefore install a different preset after the requested one has been removed. diff --git a/tests/integration/test_preset_update_workflow.py b/tests/integration/test_preset_update_workflow.py new file mode 100644 index 0000000000..5766dec617 --- /dev/null +++ b/tests/integration/test_preset_update_workflow.py @@ -0,0 +1,217 @@ +"""Workflow-level integration tests for ``specify preset update``. + +``tests/test_presets.py`` covers the orchestration contract with mocked +``preset_remove``/``preset_add`` calls, which proves *what* the wrapper calls +but not that the calls are wired to the real install/remove machinery. These +tests drive the CLI through ``CliRunner`` against a real project and a real +``PresetManager``, asserting on registry and on-disk state rather than call +counts, so the destructive remove-then-add contract is verified end to end. +""" +from __future__ import annotations + +import os +import shlex +import subprocess +from pathlib import Path + +import pytest +import yaml +from typer.testing import CliRunner + +from specify_cli import app +from specify_cli.presets import PresetManager +from tests.conftest import strip_ansi + +PRESET_ID = "update-workflow-demo" + + +def make_project(root: Path) -> Path: + """Create a minimal Spec Kit project with the core templates presets need.""" + templates = root / ".specify" / "templates" + (templates / "commands").mkdir(parents=True, exist_ok=True) + (templates / "spec-template.md").write_text( + "# Core Spec Template\n", encoding="utf-8" + ) + return root + + +def write_preset( + directory: Path, + *, + version: str, + body: str, + extra_file: str | None = None, +) -> Path: + """Write a minimal single-template preset under *directory*. + + ``extra_file`` adds a version-specific file so a later install can be + distinguished from a stale copy of the previous one that was never removed. + """ + templates = directory / "templates" + templates.mkdir(parents=True, exist_ok=True) + (templates / "spec-template.md").write_text(body, encoding="utf-8") + if extra_file is not None: + (directory / extra_file).write_text("marker\n", encoding="utf-8") + + (directory / "preset.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "preset": { + "id": PRESET_ID, + "name": "Update Workflow Demo", + "version": version, + "description": "Fixture preset for update workflow tests", + "author": "Spec Kit tests", + "license": "MIT", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "description": "Replacement spec template", + "replaces": "spec-template", + } + ] + }, + } + ), + encoding="utf-8", + ) + return directory + + +def render_command(args: list[str]) -> str: + """Render *args* the way ``preset update`` renders its retry command.""" + if os.name == "nt": + return subprocess.list2cmdline(args) + return shlex.join(args) + + +def parse_command(rendered: str) -> list[str]: + """Inverse of :func:`render_command` for the simple args used here.""" + if os.name == "nt": + return [token.strip('"') for token in shlex.split(rendered, posix=False)] + return shlex.split(rendered) + + +def retry_command_from(output: str) -> str: + """Extract the retry command printed after a failed update.""" + for line in strip_ansi(output).splitlines(): + stripped = line.strip() + if stripped.startswith("Retry with: "): + return stripped[len("Retry with: ") :].strip() + raise AssertionError(f"No retry command found in output:\n{output}") + + +@pytest.fixture +def project(tmp_path: Path, monkeypatch) -> Path: + root = make_project(tmp_path / "proj") + monkeypatch.chdir(root) + return root + + +def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path): + """A successful update really swaps the installed preset on disk.""" + runner = CliRunner() + + original = write_preset( + tmp_path / "v1", + version="1.0.0", + body="# Version One Template\n", + extra_file="only-in-v1.md", + ) + replacement = write_preset( + tmp_path / "v2", + version="2.0.0", + body="# Version Two Template\n", + extra_file="only-in-v2.md", + ) + + install = runner.invoke( + app, ["preset", "add", "--dev", str(original), "--priority", "20"] + ) + assert install.exit_code == 0, install.output + + installed_dir = project / ".specify" / "presets" / PRESET_ID + assert (installed_dir / "only-in-v1.md").exists() + + update = runner.invoke( + app, + ["preset", "update", PRESET_ID, "--dev", str(replacement), "--priority", "5"], + ) + assert update.exit_code == 0, update.output + + metadata = PresetManager(project).registry.get(PRESET_ID) + assert metadata is not None, "update must leave the preset installed" + assert metadata["version"] == "2.0.0" + assert metadata["priority"] == 5 + + template = installed_dir / "templates" / "spec-template.md" + assert template.read_text(encoding="utf-8") == "# Version Two Template\n" + assert (installed_dir / "only-in-v2.md").exists() + assert not (installed_dir / "only-in-v1.md").exists(), ( + "the previous preset's files must be removed, not merged with the " + "replacement" + ) + + +def test_preset_update_failed_replacement_prints_working_retry_command( + project: Path, tmp_path: Path +): + """A failed replacement leaves the preset removed and the retry usable.""" + runner = CliRunner() + + original = write_preset( + tmp_path / "v1", version="1.0.0", body="# Version One Template\n" + ) + install = runner.invoke(app, ["preset", "add", "--dev", str(original)]) + assert install.exit_code == 0, install.output + assert PresetManager(project).registry.is_installed(PRESET_ID) + + # The source does not exist yet, so add fails after remove has run. + replacement = tmp_path / "replacement" + update = runner.invoke( + app, + ["preset", "update", PRESET_ID, "--dev", str(replacement), "--priority", "7"], + ) + + assert update.exit_code == 1, update.output + output = strip_ansi(update.output) + assert "Directory not found" in output, "add's own error must be preserved" + assert "previous preset was removed" in output + assert not PresetManager(project).registry.is_installed(PRESET_ID), ( + "update is destructive: a failed replacement must leave the preset " + "removed rather than silently restored" + ) + assert not (project / ".specify" / "presets" / PRESET_ID).exists() + + rendered = retry_command_from(update.output) + assert rendered == render_command( + [ + "specify", + "preset", + "add", + PRESET_ID, + "--dev", + str(replacement), + "--priority", + "7", + ] + ) + + # Supply a valid source at the advertised path and run the printed command + # verbatim: the retry must actually recover the project, not merely appear. + write_preset(replacement, version="2.0.0", body="# Recovered Template\n") + retry_args = parse_command(rendered) + assert retry_args[0] == "specify" + retry = runner.invoke(app, retry_args[1:]) + + assert retry.exit_code == 0, retry.output + metadata = PresetManager(project).registry.get(PRESET_ID) + assert metadata is not None + assert metadata["version"] == "2.0.0" + assert metadata["priority"] == 7 From 678e8d94c80684de083fab68520f9c8a5cc0cbc1 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:48:41 +0100 Subject: [PATCH 10/13] test(preset): cover update CLI contract Exercise Typer parsing for the required preset ID, unsupported options, and help output. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test_preset_update_workflow.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/integration/test_preset_update_workflow.py b/tests/integration/test_preset_update_workflow.py index 5766dec617..a257136343 100644 --- a/tests/integration/test_preset_update_workflow.py +++ b/tests/integration/test_preset_update_workflow.py @@ -114,6 +114,36 @@ def project(tmp_path: Path, monkeypatch) -> Path: return root +def test_preset_update_cli_contract(): + """Typer exposes the required ID and only the supported update options.""" + runner = CliRunner() + + missing_id = runner.invoke(app, ["preset", "update"]) + assert missing_id.exit_code == 2 + assert "Missing argument 'preset_id'" in strip_ansi(missing_id.output) + + for unsupported_option in ("--all", "--dry-run"): + rejected = runner.invoke( + app, ["preset", "update", PRESET_ID, unsupported_option] + ) + assert rejected.exit_code == 2 + assert f"No such option: {unsupported_option}" in strip_ansi(rejected.output) + + help_result = runner.invoke(app, ["preset", "update", "--help"]) + assert help_result.exit_code == 0 + help_output = strip_ansi(help_result.output) + assert "Usage: specify preset update [OPTIONS] {preset_id}" in help_output + assert ( + "Replace an installed preset using the normal remove and add flows." + in help_output + ) + assert "Installed preset ID to replace" in help_output + for supported_option in ("--from", "--dev", "--priority"): + assert supported_option in help_output + assert "--all" not in help_output + assert "--dry-run" not in help_output + + def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path): """A successful update really swaps the installed preset on disk.""" runner = CliRunner() From fc2f31f38443ebe1e45c52765747becc05b5f086 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:10:47 +0100 Subject: [PATCH 11/13] fix(preset): reject empty update sources Reject explicitly supplied empty --from and --dev values before removing the installed preset, and treat supplied options as mutually exclusive regardless of their contents. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/_commands.py | 8 +++- .../test_preset_update_workflow.py | 16 +++++++ tests/test_presets.py | 48 +++++++++++++++++-- 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 30fd73b1ec..bdeb6f5a35 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -499,9 +499,15 @@ def preset_update( from .. import _require_specify_project from . import PresetManager - if from_url and dev: + if from_url is not None and dev is not None: console.print("[red]Error:[/red] --from and --dev are mutually exclusive") raise typer.Exit(1) + if from_url == "": + console.print("[red]Error:[/red] --from must not be empty") + raise typer.Exit(1) + if dev == "": + console.print("[red]Error:[/red] --dev must not be empty") + raise typer.Exit(1) # Validate priority before removal. add rejects the same range, but only # after remove has already run, which would leave the preset removed and diff --git a/tests/integration/test_preset_update_workflow.py b/tests/integration/test_preset_update_workflow.py index a257136343..1fd88a9659 100644 --- a/tests/integration/test_preset_update_workflow.py +++ b/tests/integration/test_preset_update_workflow.py @@ -129,6 +129,22 @@ def test_preset_update_cli_contract(): assert rejected.exit_code == 2 assert f"No such option: {unsupported_option}" in strip_ansi(rejected.output) + for source_option in ("--from", "--dev"): + empty_source = runner.invoke( + app, ["preset", "update", PRESET_ID, source_option, ""] + ) + assert empty_source.exit_code == 1 + assert f"{source_option} must not be empty" in strip_ansi(empty_source.output) + + mutually_exclusive = runner.invoke( + app, + ["preset", "update", PRESET_ID, "--from", "", "--dev", "replacement"], + ) + assert mutually_exclusive.exit_code == 1 + assert "--from and --dev are mutually exclusive" in strip_ansi( + mutually_exclusive.output + ) + help_result = runner.invoke(app, ["preset", "update", "--help"]) assert help_result.exit_code == 0 help_output = strip_ansi(help_result.output) diff --git a/tests/test_presets.py b/tests/test_presets.py index 5f01381735..a62d069515 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -11519,16 +11519,56 @@ def test_unknown_preset_fails_without_remove_or_add(self, project_dir, monkeypat assert exc_info.value.exit_code == 1 assert calls == [] - def test_mutually_exclusive_sources_are_rejected(self, project_dir, monkeypatch): - self._manager(monkeypatch, project_dir) + @pytest.mark.parametrize( + ("from_url", "dev"), + [ + ("https://example.com/preset.zip", "./preset"), + ("", "./preset"), + ("https://example.com/preset.zip", ""), + ], + ) + def test_mutually_exclusive_sources_are_rejected( + self, project_dir, monkeypatch, from_url, dev + ): + commands = self._manager(monkeypatch, project_dir) + calls = [] + monkeypatch.setattr(commands, "preset_remove", lambda *_args: calls.append("remove")) + monkeypatch.setattr(commands, "preset_add", lambda **_kwargs: calls.append("add")) + with pytest.raises(typer.Exit) as exc_info: preset_update( "test-pack", - from_url="https://example.com/preset.zip", - dev="./preset", + from_url=from_url, + dev=dev, + priority=10, + ) + + assert exc_info.value.exit_code == 1 + assert calls == [] + + @pytest.mark.parametrize( + ("from_url", "dev", "option"), + [("", None, "--from"), (None, "", "--dev")], + ) + def test_empty_source_is_rejected_before_removal( + self, project_dir, monkeypatch, capsys, from_url, dev, option + ): + commands = self._manager(monkeypatch, project_dir) + calls = [] + monkeypatch.setattr(commands, "preset_remove", lambda *_args: calls.append("remove")) + monkeypatch.setattr(commands, "preset_add", lambda **_kwargs: calls.append("add")) + + with pytest.raises(typer.Exit) as exc_info: + preset_update( + "test-pack", + from_url=from_url, + dev=dev, priority=10, ) + assert exc_info.value.exit_code == 1 + assert calls == [] + assert f"{option} must not be empty" in strip_ansi(capsys.readouterr().out) def test_remove_failure_prevents_add(self, project_dir, monkeypatch): commands = self._manager(monkeypatch, project_dir) From 42a60d35179a2ebbc7b385d6a69bf50f04e384c8 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:43:00 +0100 Subject: [PATCH 12/13] fix(preset): keep update retry usable for option-like IDs A preset ID starting with a hyphen satisfies manifest validation but Typer parses it as an option, so the printed retry command could not be copy-pasted. Place the source and priority options before a -- separator for such IDs, preserving the existing order for normal ones. Assisted-by: GitHub Copilot (model: claude-opus-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/_commands.py | 17 +++-- .../test_preset_update_workflow.py | 66 ++++++++++++++++++- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index bdeb6f5a35..370a7cc1c0 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -524,12 +524,17 @@ def preset_update( # reconciliation before add resolves and installs the replacement. preset_remove(preset_id) - retry_args = ["specify", "preset", "add", preset_id] - if from_url: - retry_args.extend(["--from", from_url]) - if dev: - retry_args.extend(["--dev", dev]) - retry_args.extend(["--priority", str(priority)]) + retry_args = ["specify", "preset", "add"] + retry_options = [] + if from_url is not None: + retry_options.extend(["--from", from_url]) + if dev is not None: + retry_options.extend(["--dev", dev]) + retry_options.extend(["--priority", str(priority)]) + if preset_id.startswith("-"): + retry_args.extend([*retry_options, "--", preset_id]) + else: + retry_args.extend([preset_id, *retry_options]) def report_add_failure() -> None: import subprocess diff --git a/tests/integration/test_preset_update_workflow.py b/tests/integration/test_preset_update_workflow.py index 1fd88a9659..529c8757ae 100644 --- a/tests/integration/test_preset_update_workflow.py +++ b/tests/integration/test_preset_update_workflow.py @@ -41,6 +41,7 @@ def write_preset( version: str, body: str, extra_file: str | None = None, + preset_id: str = PRESET_ID, ) -> Path: """Write a minimal single-template preset under *directory*. @@ -58,7 +59,7 @@ def write_preset( { "schema_version": "1.0", "preset": { - "id": PRESET_ID, + "id": preset_id, "name": "Update Workflow Demo", "version": version, "description": "Fixture preset for update workflow tests", @@ -261,3 +262,66 @@ def test_preset_update_failed_replacement_prints_working_retry_command( assert metadata is not None assert metadata["version"] == "2.0.0" assert metadata["priority"] == 7 + + +def test_preset_update_retry_handles_option_like_id(project: Path, tmp_path: Path): + """The printed retry places a leading-hyphen ID after ``--``.""" + runner = CliRunner() + preset_id = "--option-like-preset" + + original = write_preset( + tmp_path / "option-v1", + version="1.0.0", + body="# Option-like Version One\n", + preset_id=preset_id, + ) + install = runner.invoke(app, ["preset", "add", "--dev", str(original)]) + assert install.exit_code == 0, install.output + assert PresetManager(project).registry.is_installed(preset_id) + + replacement = tmp_path / "option-replacement" + update = runner.invoke( + app, + [ + "preset", + "update", + "--dev", + str(replacement), + "--priority", + "9", + "--", + preset_id, + ], + ) + assert update.exit_code == 1, update.output + assert not PresetManager(project).registry.is_installed(preset_id) + + rendered = retry_command_from(update.output) + assert rendered == render_command( + [ + "specify", + "preset", + "add", + "--dev", + str(replacement), + "--priority", + "9", + "--", + preset_id, + ] + ) + + write_preset( + replacement, + version="2.0.0", + body="# Option-like Version Two\n", + preset_id=preset_id, + ) + retry_args = parse_command(rendered) + retry = runner.invoke(app, retry_args[1:]) + + assert retry.exit_code == 0, retry.output + metadata = PresetManager(project).registry.get(preset_id) + assert metadata is not None + assert metadata["version"] == "2.0.0" + assert metadata["priority"] == 9 From 8b5f5163b2ca4bacede5500bebe5239c20b482fc Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:22:51 +0100 Subject: [PATCH 13/13] fix(preset): quote Windows retry commands Render Windows recovery commands explicitly for PowerShell, using literal argument quoting so shell metacharacters remain part of URLs and paths. Add regression coverage against a real PowerShell parser when available. Assisted-by: GitHub Copilot (model: HydraFusion, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/reference/presets.md | 3 +- src/specify_cli/presets/_commands.py | 28 ++++++--- .../test_preset_update_workflow.py | 11 ++-- tests/test_presets.py | 61 ++++++++++++++++++- 4 files changed, 88 insertions(+), 15 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 8b06ea59e2..7b4752ae7a 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -49,7 +49,8 @@ higher, and the preset must already be installed. Beyond those checks the replacement source itself is not inspected in advance. If removal succeeds but replacement installation fails, the previous preset has already been removed. The command reports a copy-pastable `specify preset add` retry command, -including the replacement source and priority. There is no source pre-flight, +including the replacement source and priority. On Windows, the reported command +is explicitly formatted for PowerShell. There is no source pre-flight, version comparison, manifest diff, staging, rollback, automatic repair, or recovery transaction. A missing or invalid replacement source can therefore leave the preset removed. With diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 370a7cc1c0..fd895e1486 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -54,6 +54,19 @@ MINIMUM_PRESET_PRIORITY = 1 +def _render_powershell_argv(argv: list[str]) -> str: + """Render argv as a copy-pastable PowerShell command. + + PowerShell single-quoted strings are literal except that an embedded single + quote is escaped by doubling it. The call operator is required because the + executable name is quoted too. + """ + def quote_arg(arg: str) -> str: + return "'" + arg.replace("'", "''") + "'" + + return "& " + " ".join(quote_arg(arg) for arg in argv) + + def _validate_priority(priority: int) -> None: """Reject a non-positive priority before any destructive work begins. @@ -537,18 +550,17 @@ def preset_update( retry_args.extend([preset_id, *retry_options]) def report_add_failure() -> None: - import subprocess - - rendered_args = ( - subprocess.list2cmdline(retry_args) - if os.name == "nt" - else shlex.join(retry_args) - ) + if os.name == "nt": + retry_label = "Retry in PowerShell: " + rendered_args = _render_powershell_argv(retry_args) + else: + retry_label = "Retry with: " + rendered_args = shlex.join(retry_args) console.print( "[red]Error:[/red] Preset update failed; the previous preset was removed." ) console.print( - "Retry with: [cyan]" + f"{retry_label}[cyan]" f"{_escape_markup(rendered_args)}" "[/cyan]", soft_wrap=True, diff --git a/tests/integration/test_preset_update_workflow.py b/tests/integration/test_preset_update_workflow.py index 529c8757ae..559882282d 100644 --- a/tests/integration/test_preset_update_workflow.py +++ b/tests/integration/test_preset_update_workflow.py @@ -11,7 +11,6 @@ import os import shlex -import subprocess from pathlib import Path import pytest @@ -20,6 +19,7 @@ from specify_cli import app from specify_cli.presets import PresetManager +from specify_cli.presets._commands import _render_powershell_argv from tests.conftest import strip_ansi PRESET_ID = "update-workflow-demo" @@ -88,23 +88,24 @@ def write_preset( def render_command(args: list[str]) -> str: """Render *args* the way ``preset update`` renders its retry command.""" if os.name == "nt": - return subprocess.list2cmdline(args) + return _render_powershell_argv(args) return shlex.join(args) def parse_command(rendered: str) -> list[str]: """Inverse of :func:`render_command` for the simple args used here.""" if os.name == "nt": - return [token.strip('"') for token in shlex.split(rendered, posix=False)] + return shlex.split(rendered)[1:] return shlex.split(rendered) def retry_command_from(output: str) -> str: """Extract the retry command printed after a failed update.""" + prefix = "Retry in PowerShell: " if os.name == "nt" else "Retry with: " for line in strip_ansi(output).splitlines(): stripped = line.strip() - if stripped.startswith("Retry with: "): - return stripped[len("Retry with: ") :].strip() + if stripped.startswith(prefix): + return stripped[len(prefix) :].strip() raise AssertionError(f"No retry command found in output:\n{output}") diff --git a/tests/test_presets.py b/tests/test_presets.py index a62d069515..93959f877b 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -16,6 +16,7 @@ import os import shlex import subprocess +import sys import tempfile import tarfile import shutil @@ -46,6 +47,7 @@ from specify_cli.extensions import ExtensionRegistry from specify_cli._console import console from specify_cli.presets._commands import ( + _render_powershell_argv, _warn_unmet_extension_dependencies, preset_update, ) @@ -11650,12 +11652,69 @@ def fail_add(**_kwargs): "6", ] expected = ( - subprocess.list2cmdline(retry_args) + _render_powershell_argv(retry_args) if os.name == "nt" else shlex.join(retry_args) ) assert expected in output + def test_retry_command_quotes_powershell_metacharacters( + self, project_dir, monkeypatch, capsys + ): + """Windows retry commands keep PowerShell metacharacters literal.""" + commands = self._manager(monkeypatch, project_dir) + monkeypatch.setattr(commands, "preset_remove", lambda _preset_id: None) + + def fail_add(**_kwargs): + raise typer.Exit(1) + + monkeypatch.setattr(commands, "preset_add", fail_add) + monkeypatch.setattr(os, "name", "nt") + + with pytest.raises(typer.Exit) as exc_info: + preset_update( + "test-pack", + from_url=None, + dev=r"C:\replacement&$backup's presets", + priority=6, + ) + + assert exc_info.value.exit_code == 1 + output = strip_ansi(capsys.readouterr().out) + expected = ( + "& 'specify' 'preset' 'add' 'test-pack' '--dev' " + "'C:\\replacement&$backup''s presets' '--priority' '6'" + ) + assert "Retry in PowerShell: " in output + assert expected in output + + def test_powershell_retry_renderer_preserves_literal_arguments(self): + """The rendered command survives parsing by a real PowerShell.""" + powershell = shutil.which("pwsh") or shutil.which("powershell") + if powershell is None: + pytest.skip("PowerShell is not available") + + arguments = [ + "https://example.com/archive.zip?one=1&two=$value", + r"C:\owner's presets", + ] + rendered = _render_powershell_argv( + [ + sys.executable, + "-c", + "import json,sys; print(json.dumps(sys.argv[1:]))", + *arguments, + ] + ) + result = subprocess.run( + [powershell, "-NoProfile", "-Command", rendered], + check=True, + capture_output=True, + text=True, + ) + + assert json.loads(result.stdout) == arguments + def test_invalid_priority_rejected_before_removal( self, project_dir, monkeypatch, capsys ):