Fix workflow resume disabled-check bypass after project move/rename

RunState.installed_registry_root previously persisted the creation-time
absolute project path unconditionally whenever a run belonged to an
installed workflow. After the whole project directory was renamed or
moved, workflow_resume would open a WorkflowRegistry at that now
nonexistent path, get back an empty/default registry, and silently skip
the disabled-workflow check -- a paused run for a disabled workflow could
be resumed successfully from the new location.

Fix persists installed_registry_root only when the owning root genuinely
differs from the current project_root (true cross-project direct-file-
source invocations). The common same-project case now persists None and
is re-derived from the live project_root at resume time via a new
_resolve_run_owner_root() helper, which also falls back to project_root
if a stored root no longer exists on disk -- covering both the common
case transparently surviving project moves and the cross-project case
degrading safely if its owner project vanishes, rather than silently
skipping the disabled check.

Backward compatible: state files missing the new fields, and states with
a still-existing distinct cross-project root, behave unchanged.

Added regression tests:
- resume blocked after project moved then disabled at new location
- resume still works after project moved while workflow stays enabled
- cross-project registry root is still correctly honored when it exists
- resume falls back to current project's registry when a stored
  cross-project root no longer exists

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
marcelsafin
2026-07-11 15:25:18 +02:00
parent 616f727425
commit 4f247356d0
2 changed files with 193 additions and 5 deletions

View File

@@ -77,6 +77,33 @@ def _open_workflow_registry(project_root: Path, out=None):
raise typer.Exit(1)
def _resolve_run_owner_root(
installed_registry_root: str | None, project_root: Path
) -> Path:
"""Determine which project's registry gates resuming a run.
``installed_registry_root`` is only ever persisted when the run's
installed workflow genuinely belongs to a *different* project than the
one whose ``runs/`` directory holds this run's own state (a direct
external workflow-file invocation) -- see ``workflow_run``. The common
case (an installed workflow run from its own project) stores ``None``,
so a later project rename/move is transparently picked up here by
falling back to the *current* ``project_root`` instead of a stale
absolute path baked in at run start.
A persisted cross-project root that itself no longer exists (that
other project moved/was deleted) is not trusted either: silently
skipping the disabled check because a stored path merely happens not
to resolve would defeat the guard's purpose, so this also falls back
to ``project_root`` rather than risk that.
"""
if installed_registry_root:
candidate = Path(installed_registry_root)
if candidate.is_dir():
return candidate
return project_root
def _parse_input_values(
input_values: list[str] | None, *, json_output: bool = False
) -> dict[str, Any]:
@@ -730,7 +757,20 @@ def workflow_run(
definition,
inputs,
installed_workflow_id=registered_id,
installed_registry_root=registry_root if registered_id else None,
# Only persist an explicit root when the installed workflow
# genuinely belongs to a *different* project than the one
# whose runs/ directory holds this run's own state (a
# direct external workflow-file invocation) -- the common
# case (an installed workflow run from its own project)
# leaves this None so resume re-derives the owning root
# from wherever the project currently is, transparently
# surviving a project rename/move instead of baking in a
# stale absolute path at run start.
installed_registry_root=(
registry_root
if registered_id and registry_root != project_root
else None
),
)
except ValueError as exc:
err.print(f"[red]Error:[/red] {exc}")
@@ -801,10 +841,8 @@ def workflow_resume(
raise typer.Exit(1)
if pre_state.installed_workflow_id is not None:
owner_root = (
Path(pre_state.installed_registry_root)
if pre_state.installed_registry_root
else project_root
owner_root = _resolve_run_owner_root(
pre_state.installed_registry_root, project_root
)
installed_meta = _open_workflow_registry(owner_root, err).get(
pre_state.installed_workflow_id

View File

@@ -10038,6 +10038,156 @@ steps:
result = runner.invoke(app, ["workflow", "resume", run_id, "--json"])
assert result.exit_code == 0, result.output
def test_resume_blocks_after_project_moved_following_disable(
self, temp_dir, monkeypatch
):
"""Renaming/moving the entire project after starting a run must not
let a subsequent disable-then-resume bypass the guard. Persisting
the run's *creation-time absolute* project path would make resume
open a now-nonexistent old root (WorkflowRegistry falls back to an
empty default there), missing the disabled entry that actually
lives in the *current* (moved) project's registry. The common,
same-project case must instead re-derive the owning root from the
project's current location on every resume."""
from typer.testing import CliRunner
from specify_cli import app
import shutil
project_v1 = temp_dir / "project-v1"
(project_v1 / ".specify" / "workflows").mkdir(parents=True)
monkeypatch.chdir(project_v1)
runner = CliRunner()
run_id = self._install_and_run_gated(runner, app, project_v1)
project_v2 = temp_dir / "project-v2"
shutil.move(str(project_v1), str(project_v2))
monkeypatch.chdir(project_v2)
result = runner.invoke(app, ["workflow", "disable", "gated-wf"])
assert result.exit_code == 0, result.output
result = runner.invoke(app, ["workflow", "resume", run_id])
assert result.exit_code != 0
assert "disabled" in result.output
def test_resume_after_project_moved_still_works_when_enabled(
self, temp_dir, monkeypatch
):
"""The inverse of the move regression: an enabled workflow's run
must still resume normally after the project is moved -- the
current-project fallback must not itself block legitimate
resumes."""
from typer.testing import CliRunner
from specify_cli import app
import shutil
project_v1 = temp_dir / "project-v1-ok"
(project_v1 / ".specify" / "workflows").mkdir(parents=True)
monkeypatch.chdir(project_v1)
runner = CliRunner()
run_id = self._install_and_run_gated(runner, app, project_v1)
project_v2 = temp_dir / "project-v2-ok"
shutil.move(str(project_v1), str(project_v2))
monkeypatch.chdir(project_v2)
result = runner.invoke(app, ["workflow", "resume", run_id, "--json"])
assert result.exit_code == 0, result.output
def test_resume_respects_cross_project_registry_root(
self, temp_dir, monkeypatch
):
"""A run started via a direct workflow.yml path belonging to a
different project than the cwd used for `workflow run`/`workflow
resume` must still gate resuming on *that* owning project's
registry, not the cwd project's (which has no entry for this ID
at all). This is the genuine cross-project case that must remain
unaffected by only special-casing the common same-project one."""
from typer.testing import CliRunner
from specify_cli import app
owner_project = temp_dir / "owner-project"
(owner_project / ".specify" / "workflows").mkdir(parents=True)
monkeypatch.chdir(owner_project)
runner = CliRunner()
src = owner_project / "gated-src"
src.mkdir()
(src / "workflow.yml").write_text(self._GATED_WORKFLOW_YAML, encoding="utf-8")
result = runner.invoke(app, ["workflow", "add", str(src), "--dev"])
assert result.exit_code == 0, result.output
unrelated_cwd = temp_dir / "unrelated-cwd"
unrelated_cwd.mkdir()
monkeypatch.chdir(unrelated_cwd)
target = owner_project / ".specify" / "workflows" / "gated-wf" / "workflow.yml"
result = runner.invoke(app, ["workflow", "run", str(target), "--json"])
assert result.exit_code == 0, result.output
run_id = json.loads(result.stdout)["run_id"]
monkeypatch.chdir(owner_project)
result = runner.invoke(app, ["workflow", "disable", "gated-wf"])
assert result.exit_code == 0, result.output
# Resume must run from unrelated_cwd (where this run's own
# state.json actually lives) yet still be blocked by the owner
# project's disabled entry.
monkeypatch.chdir(unrelated_cwd)
result = runner.invoke(app, ["workflow", "resume", run_id])
assert result.exit_code != 0
assert "disabled" in result.output
def test_resume_falls_back_to_current_project_when_cross_project_root_vanishes(
self, temp_dir, monkeypatch
):
"""A persisted cross-project owning root that no longer exists (that
other project was itself deleted/moved away) must not be trusted as
a safety signal -- silently skipping the disabled check just
because the stored path happens not to resolve would defeat the
guard. Falls back to the current project's own registry instead."""
from typer.testing import CliRunner
from specify_cli import app
import shutil
owner_project = temp_dir / "owner-project-2"
(owner_project / ".specify" / "workflows").mkdir(parents=True)
monkeypatch.chdir(owner_project)
runner = CliRunner()
src = owner_project / "gated-src"
src.mkdir()
(src / "workflow.yml").write_text(self._GATED_WORKFLOW_YAML, encoding="utf-8")
result = runner.invoke(app, ["workflow", "add", str(src), "--dev"])
assert result.exit_code == 0, result.output
unrelated_cwd = temp_dir / "unrelated-cwd-2"
unrelated_cwd.mkdir()
monkeypatch.chdir(unrelated_cwd)
target = owner_project / ".specify" / "workflows" / "gated-wf" / "workflow.yml"
result = runner.invoke(app, ["workflow", "run", str(target), "--json"])
assert result.exit_code == 0, result.output
run_id = json.loads(result.stdout)["run_id"]
# owner_project vanishes entirely -- its persisted absolute root
# is now dangling.
shutil.rmtree(owner_project)
# unrelated_cwd (where this run's own state.json lives) has its own
# separately-installed, disabled workflow under the same ID.
src2 = unrelated_cwd / "gated-src"
src2.mkdir()
(src2 / "workflow.yml").write_text(
self._GATED_WORKFLOW_YAML, encoding="utf-8"
)
result = runner.invoke(app, ["workflow", "add", str(src2), "--dev"])
assert result.exit_code == 0, result.output
result = runner.invoke(app, ["workflow", "disable", "gated-wf"])
assert result.exit_code == 0, result.output
result = runner.invoke(app, ["workflow", "resume", run_id])
assert result.exit_code != 0
assert "disabled" in result.output
def test_disable_shows_marker_in_list(self, project_dir, monkeypatch):
from typer.testing import CliRunner
from specify_cli import app