From 6b25c75a05ed1c717fc171a6b0ea42bcc21b46c3 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Sat, 11 Jul 2026 00:23:49 +0200 Subject: [PATCH] fix(workflows): atomic registry save and accurate mixed-target update summary - save() wrote the registry with open('w'), so a failed dump truncated the file and the next load reset every entry. Write to a sibling temp file and os.replace into place. - workflow update no longer claims all workflows are up to date when some targets were skipped; it reports checked-only status with a skipped count. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/workflows/_commands.py | 9 ++++- src/specify_cli/workflows/catalog.py | 16 ++++++-- tests/test_workflows.py | 53 ++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index 6ce5ceb27..973e69fe0 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -1138,10 +1138,15 @@ def workflow_update( console.print(f"✓ {safe_id}: Up to date (v{installed_version})") if not updates_available: - if checked: + if not checked: + console.print("\n[yellow]No workflows were eligible for update[/yellow]") + elif checked == len(targets): console.print("\n[green]All workflows are up to date![/green]") else: - console.print("\n[yellow]No workflows were eligible for update[/yellow]") + console.print( + f"\n[green]All checked workflows are up to date[/green] " + f"[yellow]({len(targets) - checked} skipped)[/yellow]" + ) raise typer.Exit(0) console.print("\n[bold]Updates available:[/bold]\n") diff --git a/src/specify_cli/workflows/catalog.py b/src/specify_cli/workflows/catalog.py index 0ab5d81c5..942a6cfd0 100644 --- a/src/specify_cli/workflows/catalog.py +++ b/src/specify_cli/workflows/catalog.py @@ -83,10 +83,20 @@ class WorkflowRegistry: return {"schema_version": self.SCHEMA_VERSION, "workflows": {}} def save(self) -> None: - """Persist registry to disk.""" + """Persist registry to disk atomically.""" self.workflows_dir.mkdir(parents=True, exist_ok=True) - with open(self.registry_path, "w", encoding="utf-8") as f: - json.dump(self.data, f, indent=2) + # Write-then-replace so a failed dump cannot truncate the registry. + tmp_path = self.registry_path.with_name(self.registry_path.name + ".tmp") + try: + with open(tmp_path, "w", encoding="utf-8") as f: + json.dump(self.data, f, indent=2) + os.replace(tmp_path, self.registry_path) + except OSError: + try: + tmp_path.unlink() + except OSError: + pass + raise def add(self, workflow_id: str, metadata: dict[str, Any]) -> None: """Add or update an installed workflow entry.""" diff --git a/tests/test_workflows.py b/tests/test_workflows.py index e773f3b60..77589fb61 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -7563,6 +7563,59 @@ steps: registry.add("align-wf", {"version": "1.0.0", "source": "catalog"}) assert registry.get("align-wf")["version"] == "1.0.0" + def test_registry_save_failure_preserves_file_on_disk(self, project_dir, monkeypatch): + """A failed dump must not truncate the persisted registry.""" + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.add("align-wf", {"version": "1.0.0", "source": "catalog"}) + + import specify_cli.workflows.catalog as catalog_mod + + def boom(*args, **kwargs): + raise OSError("disk full") + + monkeypatch.setattr(catalog_mod.json, "dump", boom) + with pytest.raises(OSError): + registry.add("align-wf", {"version": "2.0.0", "source": "catalog"}) + monkeypatch.undo() + + fresh = WorkflowRegistry(project_dir) + assert fresh.get("align-wf")["version"] == "1.0.0" + assert not list(registry.workflows_dir.glob("*.tmp")) + + def test_update_mixed_targets_does_not_claim_all_up_to_date(self, project_dir, monkeypatch): + """Skipped targets must not be presented as verified up to date.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) # local source → skipped + WorkflowRegistry(project_dir).add("catalog-wf", { + "name": "Catalog Workflow", + "version": "1.0.0", + "description": "", + "source": "catalog", + "url": "https://example.com/workflow.yml", + }) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + }, + ) + result = runner.invoke(app, ["workflow", "update"]) + assert result.exit_code == 0, result.output + assert "All workflows are up to date!" not in result.output + assert "All checked workflows are up to date" in result.output + assert "skipped" in result.output + def test_run_refuses_falsy_non_bool_enabled(self, project_dir, monkeypatch): """A falsy non-bool "enabled" (0) shows as disabled in list — run must agree.""" import json as json_mod