diff --git a/src/specify_cli/bundler/services/primitives.py b/src/specify_cli/bundler/services/primitives.py index 31b1126a34..0309aa6fad 100644 --- a/src/specify_cli/bundler/services/primitives.py +++ b/src/specify_cli/bundler/services/primitives.py @@ -428,8 +428,21 @@ def refresh(self, component: ComponentRef) -> None: except BundlerError: if backup_dir.exists(): shutil.copytree(backup_dir, step_dir, dirs_exist_ok=True) - if metadata is not None and not self._registry.is_installed(component.id): - self._registry.add(component.id, metadata) + # Re-read the registry: ``StepRegistry`` snapshots the file once + # in ``__init__`` (``self.data = self._load()``) and + # ``is_installed`` only consults that snapshot. ``self.remove()`` + # above has already deleted the entry from disk, but + # ``self._registry``'s snapshot still contains it -- so the + # guard was always False here and the restore never ran, in + # exactly the failure case it was written for. The step package + # came back but stayed unregistered: ``workflow step list`` + # stopped showing it and ``workflow step add`` then refused with + # "Step directory already exists". + from ...workflows.catalog import StepRegistry + + current = StepRegistry(self._root) + if metadata is not None and not current.is_installed(component.id): + current.add(component.id, metadata) raise finally: shutil.rmtree(backup_dir.parent, ignore_errors=True) diff --git a/tests/unit/test_bundler_primitives.py b/tests/unit/test_bundler_primitives.py index dc39106b50..272b63bb7f 100644 --- a/tests/unit/test_bundler_primitives.py +++ b/tests/unit/test_bundler_primitives.py @@ -334,3 +334,65 @@ def _plan(manifest): effective_integration=None, components=components, ) + + +def test_step_refresh_restores_registry_entry_when_reinstall_fails( + tmp_path: Path, monkeypatch +): + """A failed step refresh must leave the registry entry restored. + + ``refresh`` keeps a backup and restores it "if the remove+reinstall path + fails", but the registry half of that rollback was unreachable: + ``StepRegistry`` snapshots the file once in ``__init__`` and + ``is_installed`` reads only that snapshot, so after ``self.remove()`` + deleted the entry from disk the stale snapshot still reported it as + installed and ``not ...is_installed(...)`` was always False. + + The step package came back but stayed unregistered — ``workflow step + list`` stopped showing it, and ``workflow step add`` then refused with + "Step directory already exists". + """ + import json + + import specify_cli + from specify_cli.workflows.catalog import StepRegistry + + steps_dir = tmp_path / ".specify" / "workflows" / "steps" + (steps_dir / "my-step").mkdir(parents=True) + (steps_dir / "my-step" / "step.yml").write_text( + "step:\n type_key: my-step\n", encoding="utf-8" + ) + (steps_dir / "my-step" / "__init__.py").write_text("", encoding="utf-8") + (steps_dir / StepRegistry.REGISTRY_FILE).write_text( + json.dumps( + { + "schema_version": "1.0", + "steps": { + "my-step": { + "name": "My Step", + "version": "1.0.0", + "type_key": "my-step", + } + }, + } + ), + encoding="utf-8", + ) + + assert StepRegistry(tmp_path).is_installed("my-step") + + # Removal succeeds (real code path); only the re-install fails, which is + # what a catalog 404 / size-limit / type_key mismatch produces. + def _boom(step_id, *args, **kwargs): + raise BundlerError(f"Failed to install step '{step_id}'.") + + monkeypatch.setattr(specify_cli, "workflow_step_add", _boom) + + manager = primitive_manager("steps", tmp_path, allow_network=True) + with pytest.raises(BundlerError): + manager.refresh(_component("steps", "my-step")) + + # Read the registry fresh from disk — the point of the fix. + assert StepRegistry(tmp_path).is_installed("my-step"), ( + steps_dir / StepRegistry.REGISTRY_FILE + ).read_text(encoding="utf-8")