Skip to content

Fix agent apply writing into shared package stores (#332, #361) - #486

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-agent-apply-shared-store-dir
Oct 2, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-agent-apply-shared-store-dir

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #332
Fixes #361

Summary

In agent mode, apply and rollback no longer write through a package directory that links into a store shared by other projects on the machine. Two package managers install that way: PDM's symlink install cache and pnpm's global virtual store. A package that resolves into one of those stores is now refused, and the error names the store and how to get a private copy.

Root cause

Agent-mode apply and rollback commit each file with a stage + rename(2) in the file's parent directory. That isolates a hardlinked or symlinked file, but not a package directory that is itself a symlink into a store shared by every project on the machine:

The rename lands inside the shared directory, so another project gets patched, and a rollback in one project unpatches the others.

Fix

  • A new patch::shared_store module classifies a package path by its real (canonical) location. Detection is positive and marker-based:

    • PDM cache entry: packages/<stem>/ with a referrers file, and the path under its lib/. Checked against the PDM 2.0.3 and 2.12.4 sources (CachedPackage, install_wheel_with_cache).
    • pnpm global virtual store: v<N>/links with the store's files/ beside it. Checked against a real pnpm 10.28 GVS install.

    Ordinary pnpm node_modules/.pnpm links, workspace links and relocated per-project virtual stores carry neither marker, so they are patched as before.

  • The check covers every directory the patch writes into (shared_store_of_patch_dirs): the package root plus the parent of each file key. That matters for PyPI, where the root is site-packages and the PDM link is site-packages/<pkg> below it (Bugbot finding on b3a739f, fixed in 0f84373).

  • apply_package_patch_at refuses such a package in every state, dry run and already-patched included, so this project never records the shared copy as its own patch. The pnpm/vlt peer copies run through the same engine and get the same check.

  • rollback_package_patch_at refuses whenever it would write, dry run included. A shared copy that is already original needs no write and passes.

  • The refusal comes out as failed / apply_failed with the message: "Refusing to patch : it is in pnpm's global virtual store (enableGlobalVirtualStore), which is shared by other projects on this machine … set enableGlobalVirtualStore to false and reinstall, or use scan --mode hosted / --mode vendored". The PDM message suggests pdm config install.cache_method hardlink instead.

  • CHANGELOG (Unreleased › Fixed) and docs/ecosystems.md are updated.

Test evidence

Issue Regression test Before fix After fix
#361 (pnpm GVS) patch::apply::tests::test_apply_refuses_shared_store_package_dir (npm leg), test_apply_refuses_already_patched_shared_store_package_dir, patch::rollback::tests::test_rollback_refuses_shared_store_package_dir (npm leg) FAILED ok
#332 (PDM cache) test_apply_refuses_shared_store_package_dir (pypi leg), test_rollback_refuses_shared_store_package_dir (pypi leg) FAILED ok
#332 (PyPI root = site-packages) shared_store::tests::patch_dirs_find_a_link_below_the_package_root, plus the PyPI legs above, which use the real site-packages + urllib3/index.js shape FAILED with a root-only check ok
no regression test_apply_patches_through_per_project_pnpm_link, shared_store::tests::* (detection tests incl. negatives) ok ok

Red run on this branch before the guard: 3 failed; 5 passed. With the first, root-only guard, the PyPI-shaped legs still fail (pkg:pypi/urllib3@1.26.18 dry_run=true: must refuse). With 0f84373, everything passes.

Real pnpm 10.28 end-to-end (enableGlobalVirtualStore: true, projects a/b/c sharing one store, hand-staged .socket/ for pkg:npm/is-odd@3.0.1):

  • apply in b: failed / apply_failed ("Refusing to patch …/store/v10/links/@/is-odd/3.0.1/…/node_modules/is-odd …"), exit 1. c's index.js is untouched.
  • With the shared copy already patched, rollback in a is refused (exit 1) and the shared bytes stay patched, so b and c aren't silently unpatched.
  • Control: a plain pnpm project (node_modules/is-odd -> .pnpm/…) still applies and rolls back normally.

Checks

  • CI on 0f84373: all 355 check runs finished (349 success, 6 skipped). Cursor Bugbot: "no new issues". Its one finding on b3a739f is fixed and the thread is resolved.
  • Local cargo clippy --workspace --all-features -- -D warnings: clean.
  • Local cargo test --workspace --all-features --no-fail-fast: everything passes except 13 tests in 4 targets. All 13 are permission tests that make a directory read-only and expect a write to fail, and that can't happen when the sandbox runs as root (e.g. copy_tree::relax_loop_must_not_traverse_symlinked_root, the *_state_write_failure_* / *unremovable* tests). All of them are green in CI.
  • cargo fmt --all -- --check: main itself isn't rustfmt-clean under the pinned 1.93.1 rustfmt (it reformats 120+ unrelated files), so I ran rustfmt only on the files this PR changes, and they are clean.

Follow-ups

  • PDM's pth cache method already fails closed (File not found), and symlink_individual / hardlink are isolated by the file-level rename. Neither needs a change.

🤖 Generated with Claude Code


Note

Medium Risk
Changes agent-mode patch/rollback behavior for PDM symlink-cache and pnpm GVS installs (now errors instead of cross-project writes); detection logic must not false-positive on normal per-project layouts.

Overview
Agent-mode apply and rollback now fail closed when a target package directory (or any parent dir touched by the patch keys) resolves into a cross-project shared store, instead of writing through a symlinked package root and affecting every linked project.

A new patch::shared_store module detects PDM’s symlink install cache (packages/…/referrers + lib/) and pnpm’s global virtual store (v<N>/links beside files/). Per-project pnpm layouts are unchanged. Refusal messages name the store and suggest remediation (e.g. disable GVS, pdm config install.cache_method hardlink) or hosted/vendored mode. Rollback skips the check when everything is already original (no write). CHANGELOG and docs/ecosystems.md document the behavior; regression tests cover apply/rollback for both ecosystems and PyPI site-packages roots.

Reviewed by Cursor Bugbot for commit 3628931. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
PDM's symlink install cache and pnpm's global virtual store link a
package directory into a store every project on the machine uses.
Agent apply renamed the patched file inside that directory, so other
projects were patched too, and a rollback in one project silently
unpatched the rest.

Apply now refuses a package whose real location is such a store, and
rollback refuses whenever it would write there. The error names the
store and how to get a private copy. Per-project stores reached
through a symlink (node_modules/.pnpm, workspace links) are patched
as before.

Fixes #332, #361.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 1, 2026 17:58
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/patch/apply.rs
A PyPI patch is rooted at site-packages with file keys like
urllib3/response.py, so the PDM cache link sits below the root and
the first guard, which only looked at the root, never saw it. Apply
and rollback now classify the directory of every patched file, so a
PDM symlink-cache package is refused as intended.

Refs #332.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 1, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: ready for review at 0f84373 (0f84373c4c0d4c33ead5bfdd372955546f637446).

  • CI: all 355 check runs success or skipped on the head SHA, no failures or pending runs
  • Bugbot: reviewed 0f84373, no findings; 0 unresolved review threads
  • Mergeable, no conflicts
  • Reviewer note: New shared-store detection for PyPI and pnpm package dirs; review the store-path heuristics.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Reviewed 0f84373c4c0d4c33ead5bfdd372955546f637446. Recommendation: ready to merge from a code-review perspective.

No actionable correctness or security regressions found. Shared-store detection covers the package root and nested file-parent directories, including PyPI site-packages layouts. Apply refuses even already-patched shared copies; rollback permits an already-original no-op and refuses writes. Ordinary per-project store paths retain their existing behavior.

Validation: cargo test -p socket-patch-core --lib shared_store: 9 passed. cargo test -p socket-patch-core --lib patch::apply: 85 passed, including apply-lock tests matched by the filter. cargo test -p socket-patch-core --lib patch::rollback: 41 passed. Full workspace and real PDM/pnpm global-store installs not rerun.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final merge check for 0f84373c4c0d4c33ead5bfdd372955546f637446: the reviewed code remains clear, but this PR currently conflicts with main (d63ae5f2) in CHANGELOG.md. Confirmed with git merge-tree; no source-code conflicts were reported. Resolve that conflict and let the required checks finish before merging. The focused validation in my original comment still applies to this unchanged head.

Resolve the CHANGELOG conflict by keeping both Fixed entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L5H2ijb6wThkigkiW7vRr7
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Merged main (d63ae5f2) in 3628931 to resolve the CHANGELOG.md conflict. Both Fixed entries are kept: this PR's shared-store entry and #498's vendor -g entry. No source files conflicted. Locally, clippy -D warnings is clean and shared_store / patch::apply / patch::rollback pass (128 tests). CI on 3628931: all 354 check runs success or skipped. The PR is mergeable and is waiting on an approving review.


Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 8a50d5a into main Oct 2, 2026
354 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-agent-apply-shared-store-dir branch October 2, 2026 16:21
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3628931. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Bugbot reviewed the merge head 3628931 and found no new issues. All CI checks are green or skipped, there are no unresolved review threads, and the PR is approved and mergeable. It is ready to merge.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Re-reviewed 362893133f97404af0823a8359fc3df3956bd421. Ready from code review; no further fix needed.

The update merges main and retains both changelog entries. The shared-store detector and core apply/rollback implementation are unchanged from the reviewed fix. The branch also merges cleanly with current main (73b17db5), and its Ubuntu, Windows, and macOS test jobs pass. No new actionable defect found.

Fresh validation: shared_store 9 passed; patch::apply 85 passed (including apply-lock tests matched by the filter); patch::rollback 41 passed. These cover nested PyPI cache links, refusal before shared-store writes, already-patched apply, and no-op rollback. Real PDM/pnpm shared-store installs and the full platform matrix were not rerun locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

3 participants