mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
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>
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user