Repository navigation
Tracking: revert every vendored backend through one record-revert engine instead of nine hand-written mechanisms #989
Description
[agent] Filed by the scheduled architecture audit routine (ecosystems and formats). Register: register comment.
Kind: tracking. Source: review §2.1, Part 5.3 and 5.8; register E24.
Problem
Every vendored backend's revert_*_opts re-implements the same envelope by hand, and the record restore under it is done about nine ways (Part 5.3). On 9c43dfc:
1. The finish step is copied 12 times. It runs: dry-run return → drift-keep → --preserve-state return → remove_tree_and_prune(uuid_dir, .socket). The copies:
bun_binary.rs#L528-L561bun_lock.rs#L920-L950composer_lock.rs#L586-L612gem.rs#L1242-L1271maven_repo.rs#L658-L691npm_lock.rs#L800-L876nuget_feed.rs#L766-L808pnpm_lock.rs#L1004-L1034pypi.rs#L1625-L1690vlt_lock.rs#L1908-L2015yarn_berry_lock.rs#L885-L917yarn_classic_lock.rs#L540-L571
The copies use four keep policies, which CLI_CONTRACT.md documents per backend:
- Unconditional drift-keep: gem, npm, pnpm, bun, bun binary.
- Keep only while a live file names the uuid dir (
any_live_file_references): composer, maven legacy, nuget. - A post-revert "lock still mentions the uuid" refusal,
vendor_lock_still_wired_revert_blocked: yarn classic and berry. - Drift or
vendor_revert_residual_referencekeeps the artifact, and a removal failure is only a warning (vendor_artifact_remove_failed): pypi. Every other backend makes a removal failure an error.
2. The record loop is copied 4 times. The uuid gate, a reverse for w in entry.wiring.iter().rev(), match w.kind, an "unrecognized wiring kind {:?}; fragment left alone" warning, and Ok(true) / Ok(false) → vendor_lock_entry_drifted / Err → fail. The copies:
3. Only poetry and pdm use the one shared splice helper:
Only that helper has an all-or-nothing mode for coupled records (revert_lock_fragment_splice_atomic).
4. Other differences:
- requirements.txt speaks its own drift code,
vendor_revert_line_drifted, whichCLI_CONTRACT.mddoesn't document. - cargo writes a
cargo_lock_entrywiring record that its revert never reads (cargo.rs#L1219-L1234), because it restores from the typedentry.lock(cargo.rs#L1978).
Symptoms
- Vendored gem revert restores Gemfile.lock but leaves the Gemfile path: line when that line carries a trailing comment, and no re-run can finish it #988: the gem forward recognizer is a substring and the revert recognizer an exact line, and the coupled records are reverted non-atomically, so the revert is half-applied and drift-keeps forever.
- Vendored requirements.txt can't be reverted once the user touches the
# socket-patch vendor:comment:vendor --revertdrift-keeps forever (exit 0), and the suggested "re-vendor" remedy is a no-op or adds a duplicate line #977: requirements.txt has the same forward/revert recognizer split. - Vendored yarn classic rollback can't undo a block yarn has merged with another range (
left-pad@^1.1.0, left-pad@^1.3.0:): it reports the block as gone, exits 1 forever and leaves the lock wired #692: yarn classic can't revert a merged block. - Vendored NuGet on a core.autocrlf checkout: vendor --revert / remove / rollback revert packages.lock.json but leave nuget.config wired, so every restore fails NU1403 (vendor --revert exits 0) #537: NuGet revert on a
core.autocrlfcheckout. - Fix uv vendored revert half-reverting after a relock (#806, #821) #822 (fixed) was the same half-revert class in uv.
Impact: every backend re-decides the drift-keep, atomicity and "is this still our wiring" rules, so a fix lands in one backend at a time. The revert code in the non-npm backends alone is about 3.5K lines.
Target design
- One
revert_entry(entry, opts, backend)engine invendor/revert.rsthat owns the envelope:- the uuid gate;
- the pre-revert refusals each backend supplies;
- a plan pass over records in reverse order, where each record yields
Restore(file, new_text),Converged,Drifted(detail)orUnknown; - an all-or-nothing write for records that a backend marks as coupled;
- one finish step with one documented
KeepPolicy.
- Each backend supplies only
plan_record(&WiringRecord, &ReadFn), plus the forward-path recognizer it already has, so forward and revert share one "is ours" predicate. - Legacy ledger kinds are adapted to current records at load, never by a second revert path.
Checklist (in order)
- 1. Finish every vendored revert through one shared helper with an explicit keep policy, deleting the 12 copies (child 1: Finish every vendored revert through one shared helper with an explicit keep policy instead of 12 copied finish blocks #990; no behavior change).
- 2. Share the record loop and the unknown-kind warning (composer, gem, maven legacy, nuget) through a
RecordRevert { Done, Drifted, FileMissing }driver with an atomic option. Blocked by 1. - 3. Move poetry, pdm and pipenv onto the driver (pipenv's canonical rewrite becomes a JSON-value record). Overlaps Share the single-lock wire and revert envelope across the Poetry, PDM and Pipenv vendored backends #937.
- 4. Move requirements.txt and the
pypi_lockthree-way merge onto the driver, with the forward-path recognizer (fixes Vendored requirements.txt can't be reverted once the user touches the# socket-patch vendor:comment:vendor --revertdrift-keeps forever (exit 0), and the suggested "re-vendor" remedy is a no-op or adds a duplicate line #977). Thevendor_revert_line_drifteddecision is whether to keep it as an alias or fold it intovendor_lock_entry_drifted. - 5. npm family: express the seven lock reverts as record plans after Tracking: drive the seven npm-family vendor backends through one generic driver #920's generic driver.
- 6. Decide whether to unify the four keep policies into one, a contract change filed as a decision when this step is reached.
- 7. Delete
ledger_snapshotsonce no backend records whole-file snapshots (with E27).
Dependencies
- Related: Tracking: dispatch vendored backends through one per-ecosystem table instead of string matches in core and the CLI #959 / Route vendored revert and in-use dispatch through one core VendorBackend enum instead of CLI string matches #960 (backend dispatch), Tracking: drive the seven npm-family vendor backends through one generic driver #920 (npm driver), Share the single-lock wire and revert envelope across the Poetry, PDM and Pipenv vendored backends #937 (PyPI envelope).
- Vendored gem revert restores Gemfile.lock but leaves the Gemfile path: line when that line carries a trailing comment, and no re-run can finish it #988 can land independently first.
Consolidated work — backlog review, 2026-10-08
The following standalone issues are now tracked here. Their closure consolidates scheduling; it does not mean their implementation is complete. Original reports and discussion remain linked below.
#990: Finish every vendored revert through one shared helper with an explicit keep policy instead of 12 copied finish blocks
- Track the remaining scope from Finish every vendored revert through one shared helper with an explicit keep policy instead of 12 copied finish blocks #990 in this issue. Explicit shared revert-finish child; retain its per-backend keep-policy table under the parent rather than another issue.
Preserved scope and acceptance criteria from #990
Proposed change
- Add
vendor::revert::finish(outcome, project_root, uuid_dir_rel, opts, KeepPolicy) -> RevertOutcomenext toRevertOutcomeinvendor/mod.rsor a newvendor/revert.rs. - Replace the 12 finish blocks with one call each, passing the backend's current policy.
- Keep each backend's error text byte-identical, or pick one text and update the tests that assert it in the same PR.
- Leave cargo, golang and uv alone, because their removal paths differ. List them in the PR as follow-ups.
Size and scope
- About −300 / +90 production lines across the 12 backend files plus the new helper.
- This is mechanical: no policy change, and no change to warning codes or the JSON shape.
- Out of scope: the record loop (checklist step 2 of Tracking: revert every vendored backend through one record-revert engine instead of nine hand-written mechanisms #989), unifying the policies (step 6), and Vendored gem revert restores Gemfile.lock but leaves the Gemfile path: line when that line carries a trailing comment, and no re-run can finish it #988's gem fix.
Acceptance criteria
-
grep -n "remove_tree_and_prune(" crates/socket-patch-core/src/vendor/{bun_binary,bun_lock,composer_lock,gem,maven_repo,npm_lock,nuget_feed,pnpm_lock,pypi,vlt_lock,yarn_berry_lock,yarn_classic_lock}.rsfinds no call left in a revert function. - One unit test per
KeepPolicyvariant in the new module covers dry run, drift,--preserve-stateand removal. - The existing per-backend revert tests stay green unchanged. That includes every
drift_skipped/kept_artifactassertion, the yarnvendor_lock_still_wired_revert_blockedtests and the pypivendor_artifact_remove_failedtest. -
cargo test -p socket-patch-core vendor::and the CLIin_process_vendorsuites pass.
Preserve the existing backend keep policies
| Policy | Backends |
|---|---|
DriftKeeps: keep on any drift |
gem, npm, pnpm, bun, bun binary |
KeepWhileReferenced(files): keep on drift only while any_live_file_references |
composer (composer.lock), maven legacy (pom.xml), nuget (wired files) |
RefuseIfStillWired(files): lock_text_mentions_uuid makes it an error, vendor_lock_still_wired_revert_blocked |
yarn classic (yarn.lock), berry (yarn.lock, package.json) |
DriftOrResidualKeeps, where a removal failure is a warning |
pypi |
Activity
audit-ecosystems: file E24 (#989, #990), add E68 (#988)
mikolalysenko commentedon Oct 7, 2026
[agent] Triaged as
priority:p3(cross-cutting refactor). This is a tracking issue, so the agent claims its child slices rather than the tracker itself. #990 is the first slice. The symptom issues it lists (#988, #977, #692, #537) are triaged on their own and can each be fixed locally before the engine exists.Generated by Claude Code
mikolalysenko commentedon Oct 9, 2026
[agent] Claiming this issue for the architecture refactor routine (highest leverage: checklist item 1, one shared vendored-revert finish step with an explicit keep policy; it unblocks item 2). Slice: gem, composer, Maven legacy, NuGet and pnpm, whose backend files no open fix or refactor PR touches. bun, bun binary, npm, vlt, pypi and both yarn backends follow once their files are free. Branch: arch-refactor/989-revert-finish. Claim-ID: 2026-10-09T07:58:45Z-4f66ea
Generated by Claude Code
mikolalysenko commentedon Oct 9, 2026
[agent] Draft PR for the item 1 slice: #1245.
Generated by Claude Code
Finish vendored reverts through one shared step
refactor: run 20261009T084958Z, #1245 (#989 item 1 slice)
mikolalysenko commentedon Oct 12, 2026
[agent] Janitor: releasing the stale claim
Claim-ID: 2026-10-09T07:58:45Z-4f66ea. slices merged, last #1188 on 2026-10-09 21:39Z; no open PR for the rest of this issue, and the claimer has been silent since 2026-10-09T08:02:28Z (more than 48h). Removingagent:claimedso another fixer can pick up what remains.Generated by Claude Code