[Mikola Lysenko (mikolalysenko)]

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

Fixes #450

Root cause

The agent manifest (.socket/manifest.json) records no scope. rollback crawls only its own scope: the project, or -g / --global-prefix. Any record it restored, found already original, or found not installed there landed in the removal set. The record was deleted and GC swept its before-blobs, even when the copy the patch was applied to sits in the other scope. That copy stayed patched, the run exited 0, and nothing on the machine could find or revert it again.

Fix

commands/rollback.rs, manifest cleanup:

• Before a record leaves the manifest because it was restored, already original, or not installed in this scope, rollback checks the other scope. That is the project at --cwd for a global run, or the default global install (CrawlerOptions { global: true }) for a project run. It looks for copies of the purl whose files still hash to the patch's afterHash. Vendored records are skipped, since vendored state is project-only.
• If such a copy exists, the record stays with its whole variant group, so its blobs stay referenced and GC keeps them. A new patched_in_other_scope warning names the purl, the copy, and the command that restores it (socket-patch rollback or socket-patch rollback -g, plus --cwd when the run used one).
• The exit code is unchanged: this scope's end state is unpatched, as before. If no patched copy exists anywhere (for example, the user reinstalled), the record is still dropped.
• CLI_CONTRACT.md documents the new warning code.

Tests: red on main, green here (tests/global_scope_project_state.rs)

Case from #450	Test
Project apply, then rollback --global-prefix with an original global copy (wet and dry run)	global_rollback_keeps_a_record_patched_in_the_project
Project apply, then rollback --global-prefix with no global copy	global_rollback_keeps_a_record_not_installed_globally
-g apply, then project rollback (direction 2), then rollback -g restores it and drops the record	project_rollback_keeps_a_record_patched_globally
Both scopes patched, project rollback restores its own copy; JSON and human warning text	project_rollback_restoring_its_copy_keeps_a_record_patched_globally
Control: original in both scopes, so the record is still dropped	global_rollback_drops_a_record_original_in_both_scopes (green before and after)

All four regression tests fail when rollback.rs is reverted to origin/main and pass with the fix. The direction-2 tests put a stub npm on PATH that answers npm root -g with a temp root, so they are #[cfg(unix)].

Local verification

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test --workspace --all-features --no-fail-fast: 14,912 passed, 11 failed, all environmental. None touch rollback:
  • e2e_bun_lockb (2): blob fetch to patches-api.socket.dev blocked by the sandbox proxy, plus a bun output difference. The same 2 fail with main's rollback.rs.
  • in_process_redirect (3), repair (2), and socket-patch-core --lib (4, which this PR doesn't touch): read-only-dir and write-failure tests that can't fail when the sandbox runs as root.
• No wrapper changes are needed: the npm, pypi and gem wrappers only dispatch the binary.

Review notes and follow-ups

• /code-review high ran before this left draft. Its findings were addressed in 558df01: probe not_installed and restored-here records too, keep the variant group, add --cwd to the remedy, and reuse crawler_options().
• Not fixed here: remove -g delegates to the same rollback engine and still drops the record of a copy patched in the other scope. remove explicitly asks to drop the record, so whether it should refuse or warn is a separate decision; follow-up.
• Known limits, not fixed: a project run only checks the default global install, so it can't see a custom --global-prefix used at apply time. A global copy patched with the same uuid by a different project's apply -g also keeps this project's record; the warning makes that visible. The extra crawl runs only when a record is about to leave the manifest, so --preserve-state runs and runs where cleanup is skipped pay nothing.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes rollback manifest cleanup and blob retention for cross-scope agent patches; behavior is conservative (keep records on doubt) and exit codes stay the same, but incorrect probing could leave stale records or miss warnings.

Overview
Fixes #450: rollback no longer drops agent manifest entries (and their blobs) when this scope is restored, already original, or not installed but a project vs global copy still matches the patch afterHash.

Before manifest cleanup, rollback crawls the other scope (project for -g / --global-prefix, default global install for a project run), skips vendored keys and paths already handled in this run, and retains the full variant group when patched or unverifiable bytes are found. It emits a new informational patched_in_other_scope warning with the copy path and a scoped socket-patch rollback remedy (including --cwd when used). Exit behavior is unchanged when this scope is clean.

CLI_CONTRACT.md documents the warning. Integration tests in global_scope_project_state.rs cover both directions, dry run, unverifiable files, and the case where both scopes are original so the record still drops.

^{Reviewed by Cursor Bugbot for commit 71d22fa. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ 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 71d22fa. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: 71d22fab4be28f176e6cde4de06c15b19ce975a2
• CI: 37/37 non-skipped checks green (19 skipped, ci-ok green), no main-wide failures. No merge conflict.
• Bugbot: reviewed this head with no new findings. Its two findings on 558df01 (remedy recommended a full-scope rollback; unreadable copies dropped kept records) were fixed in 71d22fa and both threads are resolved.
• For the reviewer: remove -g still drops a record whose copy is patched in the other scope (left as a follow-up in the description), and a project run only probes the default global install, not a custom --global-prefix.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. The agent manifest records no scope, so rollback (project, or -g/--global-prefix) used to drop a record and GC its blobs whenever its own scope was restored, already original, or not installed, even while the copy in the other scope stayed patched and was then unrecoverable (#450). Before a record leaves the manifest, rollback now crawls the other scope. If a copy there still holds the afterHash bytes, or can't be hashed, it keeps the record and its variant group and emits a new patched_in_other_scope warning naming the purl-scoped command that restores it. The exit code doesn't change.

Risk: low to medium. The change only affects which records survive cleanup, and on doubt it keeps the record. Worst case is a stale record plus a warning, never lost state. The extra crawl runs only when a record is about to be dropped, and a project run now crawls the default global install at that point.

Look here

• rollback.rs:301: the other-scope probe. It skips paths this run already verified (--cwd inside the global root).
• rollback.rs:334: fail-closed on unverifiable files.
• rollback.rs:2044-2066:`` which records get probed (vendored ones are excluded) and how the variant group is kept.
• rollback.rs:350: the remedy text.
• global_scope_project_state.rs:778:``` the new tests, both directions plus a control.

Verified

• Read the full diff (3 files: rollback.rs, CLI_CONTRACT.md, the integration tests). No unrelated edits, no debug leftovers, CHANGELOG.md untouched.
• cargo test -p socket-patch-cli --test global_scope_project_state: 42/42 pass. With rollback.rs reverted to main, the 5 *_keeps_* tests fail and the drops control still passes, so the tests do exercise the fix.
• CI on this head: ci-ok and clippy green, 37/37 non-skipped checks. Bugbot reviewed 71d22fa with no new findings, and both earlier threads are resolved.
• The direction-2 tests stub npm root -g and are #[cfg(unix)]. The production code is path-agnostic.

Changes I made: none.

Open questions (non-blocking, both in the description): remove -g still drops such records, and a project run probes only the default global install, not a custom --global-prefix used at apply time. Both are follow-ups.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code