[Mikola Lysenko (mikolalysenko)]

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

Fixes #1782

Summary

An interrupted socket-patch vendor eject (hosted → vendored) could leave
an npm project unpatched with nothing to recover from. If Ctrl-C, a CI cancel
or a kill landed in a ~100 ms window, the lock was already back on the
registry tarball and the hosted .npmrc was gone, but no commit journal
existed yet. A re-run then printed "No manifest found, nothing to vendor"
and exited 0. With this change, an eject killed before its commit leaves the
project hosted, exactly as it was, and a re-run completes the eject. An eject
killed after the journal is written is finished by the next command.

Root cause

run_eject (crates/socket-patch-cli/src/commands/vendor.rs) ran the wet
restore_upstream straight to disk, and only afterwards called the vendored
backend, which opens its group commit. The only record of the hosted state
in that window was the in-memory EjectSnapshot. The scan --mode vendored
takeover was already crash-safe because it runs its restore inside the
backend's group commit.

Fix

• run_eject opens the group commit before the upstream restore and hands
  it to the vendored apply through EjectCapture.group.
  vendor_records_reusing takes that group in place of opening its own.
  The restore and the vendored wiring are captured in memory and land in a
  single journaled commit. The eject uses this journaled path only when the
  overlay captures every file the restore touches and the apply runs at the
  same root (see "Not covered" below).
• If the restore fails, or the apply returns before committing, the group is
  discarded before the existing EjectSnapshot rollback runs.
• When a failed commit kept its journal, the eject skips the snapshot
  rollback. Putting the hosted bytes back would match the journal's recorded
  old bytes, and the next command would replay over them anyway.
• vendor replays a pending commit journal (by taking the apply lock)
  before it reads the lockfiles. Without this, a re-run after a post-journal
  kill would try to eject the hosted pin the replay is about to replace.
• A group handed in during a dry run is ignored.
• The vendored apply drops a group handed in by the eject unless every pin
  is vendored in the captured ledger. A failed package's upstream restore
  therefore never reaches a commit (review finding: a kill or a pending
  commit could otherwise roll it forward, unpatched). Regression: a
  group_commit_journal failpoint run in
  rolled_back_eject_reports_nothing_vendored, red before and green after.

Tests

New tests in crates/socket-patch-cli/tests/vendor_eject.rs:

• eject_killed_before_its_commit_stays_hosted_and_reruns (An interrupted vendor eject (Ctrl-C, SIGTERM or SIGKILL) can leave the npm lock back on the registry, unpatched, with no journal, and a re-run then says "No manifest found, nothing to vendor" (exit 0) #1782). It kills
  the eject at the vendor_group_commit failpoint, then checks that the lock
  is byte-identical (still hosted), that no journal or ledger exists, and that
  a re-run ejects successfully.
  Red on main (nothing reached the lock before the commit assertion
  fails because the lock was already restored), green with the fix.
• eject_killed_after_its_journal_rolls_forward. It kills the eject at the
  group_commit_journal failpoint; the next vendor replays the journal,
  the lock ends up vendored and the ledger has the entry.

Local runs at fa35cf3:

• cargo fmt --all -- --check and cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test --workspace --all-features --no-fail-fast: 15k+ pass. Every
  eject, vendor, group-commit, crash-safety and mode-migration suite passes:
  vendor_eject, vendor_eject_bun_lockb, vendor_eject_fresh_checkout,
  vendor_group_commit_e2e, vendor_crash_safety_e2e,
  mode_migration_{npm,bun,cargo,composer,pypi}, in_process_vendor*.
• The remaining local failures also fail on origin/main in this sandbox,
  so they are environmental:
  • chmod-based write-failure tests, which can't fail when running as root
    (uid 0). This includes unchanged socket-patch-core lib tests.
  • e2e_bun_lockb: one test needs the real patches API, which the sandbox
    can't reach; the offline-takeover test is Nightly e2e_bun_lockb red: offline takeover test predates #1648 #1770 (fixed by CI: update bun.lockb offline takeover test (#1770) #1771); and
    workspace_text_migration_heals_on_rerun.

Checklist:

• An interrupted vendor eject (Ctrl-C, SIGTERM or SIGKILL) can leave the npm lock back on the registry, unpatched, with no journal, and a re-run then says "No manifest found, nothing to vendor" (exit 0) #1782: eject_killed_before_its_commit_stays_hosted_and_reruns (red → green), plus eject_killed_after_its_journal_rolls_forward

Not covered

• Gradle pins keep the old unjournaled restore. The restore removes the
  hosted index and settings script under .socket/gradle/, and the group
  commit doesn't capture those files. A half-captured restore killed early
  would leave settings.gradle applying a deleted script, which is worse
  than today. The EjectSnapshot rollback still covers failures that aren't
  crashes. Follow-up: add the hosted Gradle files to the captured set.
• --manifest-path rooting the project somewhere other than --cwd
  also keeps the old path. In that case the restore and the vendored apply
  write the same files under different path spellings.

/code-review high

I ran it before marking ready. Fixed: the Gradle partial-capture regression,
the pending-journal vs. snapshot-rollback conflict, the root-spelling
mismatch, and the dry-run gate on a handed group. I also added the
post-journal test. Not changed:

• The duplicated switched_off("group_commit") gate: the two call sites
  intentionally differ (the eject also requires a capture-safe restore).
• The "did not commit" branch: kept as an explicit guard, because the
  ledger backstop can't tell an uncommitted group apart from an earlier
  ledger entry.

https://claude.ai/code/session_011f9aeQTtfPaHkRAMwQAsZN

---

Note

Medium Risk
Changes crash-safety and commit/rollback ordering for vendor eject and journal replay on every vendor run when a journal exists, but behavior is gated and covered by targeted eject/group-commit tests.

Overview
Fixes #1782: a killed hosted→vendored eject could briefly restore the lock to the registry with no journal, so a re-run reported nothing to vendor while the project was effectively unpatched.

Eject now opens a group commit before restore_upstream (when every restored path is capture-safe and apply runs at project root) and passes it into the vendored apply via EjectCapture.group, so upstream restore and vendored wiring land in one journaled commit. Failed restore, an apply that never commits, or a partial eject drops that group and keeps the existing snapshot rollback; if a failed commit left a journal, rollback is skipped so the next replay is not fought. The vendored apply refuses to commit an eject group unless every pin is in the captured ledger, so a failed package’s plain-registry restore cannot be journaled.

vendor (non-revert) acquires the apply lock and replays a pending commit journal before reading lockfiles, so a post-journal kill finishes on the next run instead of ejecting again from stale hosted pins.

Adds regression tests for kill-before-commit (lock unchanged, re-run succeeds) and kill-after-journal (replay completes vendoring), plus a fresh-checkout case ensuring a failed multi-pin eject never leaves a journal.

^{Reviewed by Cursor Bugbot for commit 1ff7c0c. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Bugbot is paused — your team has an unpaid invoice

Bugbot is paused because your team has an unpaid invoice.

Pay it from the billing page in the Cursor dashboard and Bugbot resumes automatically.

(requestId: d73f9757-de90-4385-bbb9-9b7565b939cd)

[cursor]

Bugbot is paused — your team has an unpaid invoice

Bugbot is paused because your team has an unpaid invoice.

Pay it from the billing page in the Cursor dashboard and Bugbot resumes automatically.

(requestId: 06b98b07-17e2-4c13-8f2e-47304fe2a047)

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at fa35cf3 (burn-down agent).

• CI: 82 check runs: 32 success, 50 skipped, all green (Cursor Bugbot included).
• Bugbot reviewed fa35cf3: no issues found. No unresolved review threads.
• Slack announcement not sent this run (Slack posting unavailable); the next run retries.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review at fa35cf3 (burn-down agent).

• CI: 81/81 check runs on fa35cf3 green or skipped (31 success, 50 skipped); mergeable into main.
• Bugbot reviewed fa35cf3 (now that billing is restored): no issues found. No open review threads.
• Slack announcement not sent: the Slack send tool was unavailable this run.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: A hosted-to-vendored vendor eject used to write the upstream registry restore to disk before the vendored group commit's journal existed. If the eject was interrupted there, npm was left unpatched (#1782). Now the GroupCommit is opened before restore_upstream, so the restore and the vendored wiring land in one journaled commit. vendor also replays a pending commit journal before it reads the lockfiles.

Risk: low to medium. It changes the core eject transaction, but the journaled path only runs when the overlay captures every touched file and the project root equals --cwd. Every other case keeps the old path unchanged. The core commit, journal and replay code is reused, not modified. The diff is 2 files, +182/-10.

Look here

• vendor.rs:961: the journal replay before the inventory read
• vendor.rs:2057: the journaled_restore gate, and GroupCommit::begin before restore_upstream
• vendor.rs:2122: the guard for an apply that returned without committing
• vendor.rs:2165: commit_pending skips the snapshot rollback
• vendor.rs:3216: vendor_records_reusing takes the group it is handed

Verified

• Traced each crash point (before begin, during the restore or apply, after the journal is written) and the failure and rollback paths, plus Windows path comparison and the symlink refusals.
• cargo test -p socket-patch-cli --test vendor_eject: 12/12 pass. Both new tests fail on main, so they do exercise the fix.
• cargo test -p socket-patch-cli --lib commands::vendor: 74 pass.
• cargo clippy -p socket-patch-cli --lib --bins -D warnings and cargo fmt --check: clean.
• CI on the head: 32 success, 50 skipped, 0 failed. Bugbot: success. CHANGELOG.md is untouched. The description matches the code.

Changes I made: none.

Open questions (non-blocking, could be follow-ups)

• vendor.rs:2122/2165: if the commit succeeds but removing the journal fails, and the ledger backstop then fails, the snapshot rollback is skipped. The error message still says the eject is undone. This needs two failures at once.
• vendor.rs:2003-2061: the capture-safety check uses the dry-run plan's reverted_files, computed before the apply lock is taken. Re-checking the wet restore.reverted_files against captures() would close that small time-of-check/time-of-use window.
• vendor --dry-run will replay a pending journal, which finishes an already-committed transaction but is still a write.

Auto-merge (squash) is armed, so approving sends this PR straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final reviewer: I turned auto-merge off. The review brief covers fa35cf3, and 1ff7c0c ("Never commit a partly failed vendor eject") changes code after it. That commit was pushed less than 45 minutes ago, so I'll review it in a later run and post a new brief. Auto-merge goes back on then if nothing blocks.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

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 1ff7c0c. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 1ff7c0c (burn-down agent).

• CI: 49/49 check runs on 1ff7c0c green (30 success, 19 skipped); mergeable, no conflicts.
• Bugbot: reviewed 1ff7c0c, no new issues; no unresolved review threads.
• Reviewer focus: since the fa35cf3 brief, 1ff7c0c makes a partly failed eject never commit (from review feedback). Auto-merge is off; the final reviewer re-arms it after re-reviewing this head.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: Ready for review again at 1ff7c0c. The label was first applied at fa35cf3.

• CI: 49/49 check runs on 1ff7c0c green or skipped (30 success, 19 skipped); no main-wide failures.
• Bugbot: reviewed 1ff7c0c, no new issues.
• Human review: the 21:17 finding (a partly failed eject could roll forward with a package unpatched) is fixed in 1ff7c0c, and its thread is answered. No unresolved threads.
• Auto-merge is off (the final reviewer turned it off at 1ff7c0c).
• Reviewer focus: the change that never commits a partly failed eject, in vendor.rs around the shared restore group (~:2057/:2165).

---

Generated by Claude Code