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

[danielmeppiel]

fix(install): make dependency resolution transactional

TL;DR

A failed cyclic dependency resolution now rolls back only the apm_modules/ paths staged or replaced by that attempt. After the user fixes the source graph, a normal apm install succeeds without manually deleting apm_modules/, while valid pre-existing cache entries remain protected.

Note

Fixes #2140. This deliberately does not use a blanket delete-all recovery path.

Problem (WHY)

• A two-node local cycle correctly exited 1, but left both package snapshots in apm_modules/.
• After the source cycle was fixed, the stale snapshots remained authoritative and the next install still exited 1.
• Recovery required rm -rf apm_modules, discarding unrelated valid cache entries.
• [!] This violated P6: "Behavior must be predictable, auditable, and explainable in plain English."
• [!] A delete-all fix would violate P4: "No bug fix, hardening, security patch, or refactor lands if it makes those commands harder, slower, more verbose, or more confusing for a new user."

Approach (WHAT)

#	Fix	Principle
1	Register every package path before resolution mutates it.	P6 - Reliability over magic
2	Move replaced content to a session-unique backup and record newly created paths.	Non-destructive, rollback-scoped staging
3	Commit only after graph validation; on any resolution exception, remove only session-created content and restore backups.	P4 - UX is the floor, not a trade
4	Keep the staging layer below target adapters and independent of package source type.	P3 - Vendor neutral by construction

Implementation (HOW)

File	Change
src/apm_cli/install/resolution_staging.py	Adds the thread-safe ResolutionStagingSession and transaction decorator. Existing paths are atomically moved to session backups; rollback restores them in reverse order.
src/apm_cli/install/phases/resolve.py	Wraps dependency resolution in the staging transaction and registers a path only after cache reuse has been ruled out.
tests/unit/install/test_resolution_transactionality.py	Reproduces #2140 through the real CLI, proves failed resolution is clean, then fixes the source and proves retry succeeds. Also verifies replaced cache content is restored.
docs/src/content/docs/troubleshooting/install-failures.md	Documents that cyclic-graph retries no longer require manual apm_modules/ deletion.

Diagram

Legend: dashed nodes are the new transaction boundary around existing resolution and validation.

flowchart LR
    S[Source manifests] --> P[Prepare touched path]
    P --> M[Materialize package snapshot]
    M --> V{Graph valid}
    V -->|yes| C[Discard session backups]
    V -->|no| R[Remove new snapshots and restore backups]
    R --> X[Correct source and retry]
    classDef new stroke-dasharray: 5 5;
    class P,C,R new;

Loading

Trade-offs

• Chose per-path backup and rollback over deleting all of apm_modules/; unrelated valid cache entries are preserved.
• Chose same-filesystem sibling staging under apm_modules/ over copying the full cache; path replacement stays atomic without cache-sized copy cost.
• Kept package materialization unchanged; authenticated-source resolution, content-hash checks, and valid cache short-circuits retain their existing behavior.
• The transaction covers resolution writes, not later deployment phases; this is the narrow owner locus for [BUG] Cyclic-dependency rejection leaves un-resumable apm_modules snapshot; corrected source still fails until manual rm -rf #2140.

Benefits

1. A rejected two-node cycle leaves neither newly staged package snapshot behind.
2. Fixing the source graph allows the next plain install to exit 0 and write the lockfile.
3. Copilot instructions deploy on the retry without manual cache deletion.
4. A pre-existing path replaced during a failed attempt is restored in the regression test.

Validation

Scenario evidence

#	Scenario (user promise)	Principle(s)	Test proving it	Type
1	Fix a rejected local dependency cycle and rerun install without deleting apm_modules/; install succeeds.	DevX, Governed by policy	tests/unit/install/test_resolution_transactionality.py::test_corrected_local_cycle_resumes_without_manual_cache_deletion (regression-trap for #2140)	integration
2	A failed resolution that replaces a valid cache path restores the original content.	Secure by default, Governed by policy	tests/unit/install/test_resolution_transactionality.py::test_resolution_rollback_restores_replaced_cache_path	unit

Test and lint evidence

Base regression run before production changes:

1 failed in 5.33s

Regression after the fix:

2 passed

Targeted install suite:

1787 passed in 28.21s

CI lint mirror:

All checks passed!
1424 files already formatted
Your code has been rated at 10.00/10
[+] auth-signal lint clean
YAML, file-length, and relative_to guards passed

How to test

• Create local packages A and B where A depends on B and B depends on A; run apm install --target copilot --verbose and expect exit 1 with no A/B snapshots retained.
• Remove B's dependency on A without deleting apm_modules/.
• Run the same install command and expect exit 0.
• Confirm apm.lock.yaml and .github/instructions/package-a.instructions.md exist.

Fixes #2140

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

This PR makes dependency resolution transactional by tracking and rolling back only the apm_modules/ paths that a failed resolution attempt staged or replaced (not a blanket cache deletion). This specifically addresses cyclic-graph failures leaving behind stale snapshots that poison subsequent installs.

Changes:

• Adds ResolutionStagingSession + transactional_resolution decorator to back up/restore per-path mutations under apm_modules/.
• Wraps the resolve phase so resolution writes are rolled back on resolution-time exceptions (including circular dependency rejection).
• Adds regression tests and updates troubleshooting docs to reflect resumable retries after fixing a cycle.

Show a summary per file

File	Description
src/apm_cli/install/resolution_staging.py	Introduces a per-install staging session that backs up replaced paths and can rollback/commit atomically.
src/apm_cli/install/phases/resolve.py	Wraps dependency resolution in the transactional staging boundary and prepares install paths before mutation.
tests/unit/install/test_resolution_transactionality.py	Adds regression coverage for rollback correctness and for “fix cycle then rerun” without manual cache deletion.
docs/src/content/docs/troubleshooting/install-failures.md	Documents that cyclic-graph retries no longer require manual apm_modules/ deletion.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Low

[danielmeppiel]

Superseded by #2155. The reliability campaign consolidated all 13 point fixes into a single architectural cure PR that re-homes each fix under its canonical owner module and adds owner-invariant guards. The fix for #2140 is folded into #2155 and re-verified there (full CI green). Closing this point PR in favor of the consolidated cure; the branch is preserved and this can be reopened if #2155 is not merged.