[BUG] Make dependency resolution transactional so a corrected cyclic graph resumes without manual apm_modules deletion

docs/src/content/docs/troubleshooting/install-failures.md

@@ -220,6 +220,8 @@ apm install
 
 The cache short-circuits already-downloaded packages and the integrate phase overwrites partially-deployed files.
 
+If resolution rejects a cyclic dependency graph, fix the package manifests and run `apm install` again. APM rolls back only the package snapshots staged by the rejected resolution, so no manual `apm_modules/` deletion is required.
+
 If files in `apm_modules/` or under target harness directories look corrupt, force a fresh deploy by combining cache bypass with overwrite:
 
 ```bash

src/apm_cli/install/phases/resolve.py

@@ -24,11 +24,13 @@
 from typing import TYPE_CHECKING
 
 from apm_cli.install.helpers.ref_seed import seed_ref_resolver_from_lockfile
+from apm_cli.install.resolution_staging import transactional_resolution
 from apm_cli.models.apm_package import GitReferenceType, ResolvedReference
 from apm_cli.utils.short_sha import format_short_sha
 
 if TYPE_CHECKING:
     from apm_cli.install.context import InstallContext
+    from apm_cli.install.resolution_staging import ResolutionStagingSession
     from apm_cli.models.dependency.reference import DependencyReference
 
 _logger = logging.getLogger(__name__)
@@ -374,13 +376,9 @@ def _fail_on_resolution_errors(ctx: InstallContext, dependency_graph) -> None:
     raise RuntimeError(f"Dependency resolution failed: {joined_errors}")
 
 
-def _resolve_dependencies(ctx: InstallContext) -> None:
-    """Run ``APMDependencyResolver``, handle errors; populate ``ctx.deps_to_install`` and ``ctx.dependency_graph``.
-
-    Also wires the download callback (which handles transitive package fetching),
-    builds ``ctx.dep_base_dirs``, writes ancillary state to ``ctx``, and cleans up
-    the shared clone cache.
-    """
+@transactional_resolution
+def _resolve_dependencies(ctx: InstallContext, staging_session: ResolutionStagingSession) -> None:
+    """Resolve dependencies and populate the resolution fields on ``ctx``."""
     import threading as _threading
 
     from apm_cli.core.scope import InstallScope
@@ -499,6 +497,7 @@ def download_callback(dep_ref, modules_dir, parent_chain="", parent_pkg=None):
             )
             if not _force_semver_resolve:
                 return install_path
+        staging_session.prepare_path(install_path)
         # F1 (#1116): surface a heartbeat BEFORE the network/copy work so
         # users see the install advancing past silent transitive lookups.
         # Under F7's parallel BFS this callback may run on a worker

src/apm_cli/install/resolution_staging.py

@@ -0,0 +1,81 @@
+"""Rollback-scoped staging for dependency resolution writes."""
+
+from __future__ import annotations
+
+import threading
+import uuid
+from collections.abc import Callable
+from functools import wraps
+from pathlib import Path
+from typing import Any
+
+from apm_cli.utils.path_security import ensure_path_within, safe_rmtree
+
+
+class ResolutionStagingSession:
+    """Track paths mutated during resolution and restore them on failure."""
+
+    def __init__(self, apm_modules_dir: Path) -> None:
+        """Create an empty staging session rooted below ``apm_modules``."""
+        self._modules_dir = apm_modules_dir
+        self._staging_root = apm_modules_dir / ".apm-resolution-staging" / uuid.uuid4().hex
+        self._backups: dict[Path, Path | None] = {}
+        self._lock = threading.Lock()
+
+    def prepare_path(self, path: Path) -> None:
+        """Record *path* and preserve its pre-resolution contents if present."""
+        resolved = ensure_path_within(path, self._modules_dir)
+        with self._lock:
+            if resolved in self._backups:
+                return
+            backup: Path | None = None
+            if resolved.exists():
+                resolved_base = ensure_path_within(self._modules_dir, self._modules_dir)
+                relative = resolved.relative_to(resolved_base)
+                backup = self._staging_root / relative
+                backup.parent.mkdir(parents=True, exist_ok=True)
+                resolved.replace(backup)
+            self._backups[resolved] = backup
+
+    def commit(self) -> None:
+        """Discard preserved pre-resolution contents after successful validation."""
+        self._remove_staging_root()
+        self._backups.clear()
+
+    def rollback(self) -> None:
+        """Remove session-created paths and restore every replaced path."""
+        with self._lock:
+            for path, backup in reversed(self._backups.items()):
+                if path.exists():
+                    safe_rmtree(path, self._modules_dir)
+                if backup is not None and backup.exists():
+                    path.parent.mkdir(parents=True, exist_ok=True)
+                    backup.replace(path)
+            self._remove_staging_root()
+            self._backups.clear()
+
+    def _remove_staging_root(self) -> None:
+        if self._staging_root.exists():
+            safe_rmtree(self._staging_root, self._modules_dir)
+        staging_parent = self._staging_root.parent
+        if staging_parent.exists() and not any(staging_parent.iterdir()):
+            staging_parent.rmdir()
+
+
+ResolveFunction = Callable[[Any, ResolutionStagingSession], None]
+
+
+def transactional_resolution(resolve: ResolveFunction) -> Callable[[Any], None]:
+    """Wrap a resolve operation in a rollback-scoped staging session."""
+
+    @wraps(resolve)
+    def wrapped(ctx: Any) -> None:
+        session = ResolutionStagingSession(ctx.apm_modules_dir)
+        try:
+            resolve(ctx, session)
+        except BaseException:
+            session.rollback()
+            raise
+        session.commit()
+
+    return wrapped

tests/unit/install/test_resolution_transactionality.py

@@ -0,0 +1,97 @@
+"""Regression coverage for transactional dependency resolution."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from click.testing import CliRunner
+
+from apm_cli.cli import cli
+from apm_cli.install.resolution_staging import ResolutionStagingSession
+from apm_cli.models.apm_package import clear_apm_yml_cache
+
+
+def _write_package(path: Path, name: str, dependency: str | None = None) -> None:
+    """Write a local package and one Copilot instruction."""
+    path.mkdir(parents=True, exist_ok=True)
+    dependency_block = ""
+    if dependency is not None:
+        dependency_block = f"dependencies:\n  apm:\n    - path: {dependency}\n"
+    (path / "apm.yml").write_text(
+        f"name: {name}\nversion: 1.0.0\n{dependency_block}",
+        encoding="ascii",
+    )
+    instructions = path / ".apm" / "instructions"
+    instructions.mkdir(parents=True, exist_ok=True)
+    (instructions / f"{name}.instructions.md").write_text(
+        f"# {name}\n",
+        encoding="ascii",
+    )
+
+
+def test_resolution_rollback_restores_replaced_cache_path(tmp_path: Path) -> None:
+    """Rollback must restore a valid cache entry replaced by this attempt."""
+    modules = tmp_path / "apm_modules"
+    package = modules / "_local" / "package-a"
+    package.mkdir(parents=True)
+    marker = package / "marker.txt"
+    marker.write_text("original", encoding="ascii")
+    session = ResolutionStagingSession(modules)
+
+    session.prepare_path(package)
+    package.mkdir(parents=True)
+    marker.write_text("replacement", encoding="ascii")
+    session.rollback()
+
+    assert marker.read_text(encoding="ascii") == "original"
+    assert not (modules / ".apm-resolution-staging").exists()
+
+
+def test_corrected_local_cycle_resumes_without_manual_cache_deletion(
+    tmp_path: Path,
+    monkeypatch,
+) -> None:
+    """A rejected local cycle must not leave snapshots that poison the retry."""
+    workspace = tmp_path / "workspace"
+    project = workspace / "project"
+    package_a = workspace / "package-a"
+    package_b = workspace / "package-b"
+    home = workspace / "home"
+    project.mkdir(parents=True)
+    home.mkdir()
+    (project / ".github").mkdir()
+    (project / "apm.yml").write_text(
+        "name: consumer\nversion: 1.0.0\ndependencies:\n  apm:\n    - path: ../package-a\n",
+        encoding="ascii",
+    )
+    _write_package(package_a, "package-a", "../package-b")
+    _write_package(package_b, "package-b", "../package-a")
+
+    monkeypatch.chdir(project)
+    monkeypatch.setenv("HOME", str(home))
+    runner = CliRunner()
+
+    rejected = runner.invoke(
+        cli,
+        ["install", "--target", "copilot", "--verbose"],
+        catch_exceptions=True,
+    )
+
+    assert rejected.exit_code != 0, rejected.output
+    assert "Circular dependencies detected" in rejected.output
+    modules = project / "apm_modules"
+    assert not (modules / "_local" / "package-a").exists()
+    assert not (modules / "_local" / "package-b").exists()
+    assert not (project / "apm.lock.yaml").exists()
+
+    _write_package(package_b, "package-b")
+    clear_apm_yml_cache()
+    resumed = runner.invoke(
+        cli,
+        ["install", "--target", "copilot", "--verbose"],
+        catch_exceptions=True,
+    )
+
+    assert resumed.exit_code == 0, resumed.output
+    assert (project / "apm.lock.yaml").is_file()
+    assert (project / ".github" / "instructions" / "package-a.instructions.md").is_file()