Skip to content

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:

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_reference keeps 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, which CLI_CONTRACT.md doesn't document.
  • cargo writes a cargo_lock_entry wiring record that its revert never reads (cargo.rs#L1219-L1234), because it restores from the typed entry.lock (cargo.rs#L1978).

Symptoms

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 in vendor/revert.rs that 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) or Unknown;
    • 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)

Dependencies


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

Preserved scope and acceptance criteria from #990

Proposed change

  • Add vendor::revert::finish(outcome, project_root, uuid_dir_rel, opts, KeepPolicy) -> RevertOutcome next to RevertOutcome in vendor/mod.rs or a new vendor/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

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}.rs finds no call left in a revert function.
  • One unit test per KeepPolicy variant in the new module covers dry run, drift, --preserve-state and removal.
  • The existing per-backend revert tests stay green unchanged. That includes every drift_skipped / kept_artifact assertion, the yarn vendor_lock_still_wired_revert_blocked tests and the pypi vendor_artifact_remove_failed test.
  • cargo test -p socket-patch-core vendor:: and the CLI in_process_vendor suites 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

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Oct 7, 2026
  2. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    [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

  3. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    [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

  4. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Draft PR for the item 1 slice: #1245.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:claimedagent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions