mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(workflows): registry rollback on save failure, consistent disabled check, honest update summary
- WorkflowRegistry.add now rolls back its in-memory mutation when save() raises, so a later successful save cannot persist metadata for a failed update alongside the restored YAML backup. - workflow run uses the same truthiness check for 'enabled' as list and disable, so malformed values like 0 or null refuse to run. - workflow update reports 'No workflows were eligible for update' when every target was skipped instead of claiming all are up to date. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -374,7 +374,7 @@ def workflow_run(
|
||||
|
||||
if registered_id is not None:
|
||||
installed_meta = WorkflowRegistry(project_root).get(registered_id)
|
||||
if isinstance(installed_meta, dict) and installed_meta.get("enabled", True) is False:
|
||||
if isinstance(installed_meta, dict) and not installed_meta.get("enabled", True):
|
||||
err.print(
|
||||
f"[red]Error:[/red] Workflow '{_escape_markup(registered_id)}' is disabled. "
|
||||
f"Enable with: specify workflow enable {_escape_markup(registered_id)}"
|
||||
@@ -983,7 +983,7 @@ def _install_workflow_from_catalog(
|
||||
}
|
||||
# Preserve a prior disabled state across updates/reinstalls.
|
||||
existing = registry.get(workflow_id)
|
||||
if isinstance(existing, dict) and existing.get("enabled", True) is False:
|
||||
if isinstance(existing, dict) and not existing.get("enabled", True):
|
||||
entry["enabled"] = False
|
||||
registry.add(workflow_id, entry)
|
||||
console.print(
|
||||
@@ -1085,6 +1085,7 @@ def workflow_update(
|
||||
console.print("🔄 Checking for updates...\n")
|
||||
|
||||
updates_available: list[dict[str, str]] = []
|
||||
checked = 0
|
||||
for wf_id in targets:
|
||||
safe_id = _escape_markup(str(wf_id))
|
||||
metadata = installed.get(wf_id)
|
||||
@@ -1122,14 +1123,19 @@ def workflow_update(
|
||||
)
|
||||
continue
|
||||
if catalog_version > installed_version:
|
||||
checked += 1
|
||||
updates_available.append(
|
||||
{"id": wf_id, "installed": str(installed_version), "available": str(catalog_version)}
|
||||
)
|
||||
else:
|
||||
checked += 1
|
||||
console.print(f"✓ {safe_id}: Up to date (v{installed_version})")
|
||||
|
||||
if not updates_available:
|
||||
console.print("\n[green]All workflows are up to date![/green]")
|
||||
if checked:
|
||||
console.print("\n[green]All workflows are up to date![/green]")
|
||||
else:
|
||||
console.print("\n[yellow]No workflows were eligible for update[/yellow]")
|
||||
raise typer.Exit(0)
|
||||
|
||||
console.print("\n[bold]Updates available:[/bold]\n")
|
||||
|
||||
@@ -93,12 +93,22 @@ class WorkflowRegistry:
|
||||
from datetime import datetime, timezone
|
||||
|
||||
existing = self.data["workflows"].get(workflow_id, {})
|
||||
had_entry = workflow_id in self.data["workflows"]
|
||||
metadata["installed_at"] = existing.get(
|
||||
"installed_at", datetime.now(timezone.utc).isoformat()
|
||||
)
|
||||
metadata["updated_at"] = datetime.now(timezone.utc).isoformat()
|
||||
self.data["workflows"][workflow_id] = metadata
|
||||
self.save()
|
||||
try:
|
||||
self.save()
|
||||
except OSError:
|
||||
# Roll back the in-memory mutation so a later successful save
|
||||
# cannot persist metadata for a write that failed.
|
||||
if had_entry:
|
||||
self.data["workflows"][workflow_id] = existing
|
||||
else:
|
||||
del self.data["workflows"][workflow_id]
|
||||
raise
|
||||
|
||||
def remove(self, workflow_id: str) -> bool:
|
||||
"""Remove an installed workflow entry. Returns True if found."""
|
||||
|
||||
@@ -7533,6 +7533,47 @@ steps:
|
||||
result = runner.invoke(app, ["workflow", "update"])
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "re-add to update" in result.output
|
||||
# Every target was skipped — must not claim everything is up to date.
|
||||
assert "No workflows were eligible for update" in result.output
|
||||
assert "up to date!" not in result.output
|
||||
|
||||
def test_registry_add_rolls_back_memory_on_save_failure(self, project_dir, monkeypatch):
|
||||
from specify_cli.workflows.catalog import WorkflowRegistry
|
||||
|
||||
registry = WorkflowRegistry(project_dir)
|
||||
registry.add("align-wf", {"version": "1.0.0", "source": "catalog"})
|
||||
|
||||
def boom():
|
||||
raise OSError("disk full")
|
||||
|
||||
monkeypatch.setattr(registry, "save", boom)
|
||||
with pytest.raises(OSError):
|
||||
registry.add("align-wf", {"version": "2.0.0", "source": "catalog"})
|
||||
assert registry.get("align-wf")["version"] == "1.0.0"
|
||||
|
||||
with pytest.raises(OSError):
|
||||
registry.add("other-wf", {"version": "1.0.0", "source": "catalog"})
|
||||
assert registry.get("other-wf") is None
|
||||
|
||||
def test_run_refuses_falsy_non_bool_enabled(self, project_dir, monkeypatch):
|
||||
""""enabled": 0 shows as disabled in list — run must agree."""
|
||||
import json as json_mod
|
||||
|
||||
from typer.testing import CliRunner
|
||||
from specify_cli import app
|
||||
from specify_cli.workflows.catalog import WorkflowRegistry
|
||||
|
||||
monkeypatch.chdir(project_dir)
|
||||
runner = CliRunner()
|
||||
self._install_dev(runner, app, project_dir)
|
||||
|
||||
registry = WorkflowRegistry(project_dir)
|
||||
registry.data["workflows"]["align-wf"]["enabled"] = 0
|
||||
registry.registry_path.write_text(json_mod.dumps(registry.data), encoding="utf-8")
|
||||
|
||||
result = runner.invoke(app, ["workflow", "run", "align-wf"])
|
||||
assert result.exit_code != 0
|
||||
assert "disabled" in result.output
|
||||
|
||||
def test_update_installs_newer_catalog_version(self, project_dir, monkeypatch):
|
||||
from unittest.mock import patch
|
||||
|
||||
Reference in New Issue
Block a user