remove and rollback each render VendorRevertStep in their own loop, and the loops drifted (remove --preserve-state --dry-run previews a full revert)

Offen
#1,861 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Maintainer antworten meist innerhalb von 1 Tag

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Anfängerfreundlichkeit
55/100
Issue-Typ
Refactoring
Klarheit
Klar beschrieben
Aktivitätsstatus
Aktiv
Tech-Stack
rust
Bereich
cli, testing

Rechercherichtung

Start with crates/socket-patch-cli/src/commands/vendored_backend/mod.rs, then compare the revert handling loops in commands/remove.rs and commands/rollback.rs. Read the vendored rollback and remove tests named in the acceptance criteria, then add coverage for remove --preserve-state --dry-run --json. Done means dry-run and wet-run emit the same event code and preserved count, with the listed vendored tests passing.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

agent:triaged arch-audit priority:p3 refactor

[agent] Filed by the scheduled architecture audit (audit-cli-tests). Register: register comment.

Kind: refactor (with one suspected behavior drift). Register ID: T16.

Problem

Both commands call VendoredBackend's revert for each ledger key and then turn the VendorRevertStep into events, warnings and counters, each in its own loop:

vendor.rs classifies the same steps two more times (L4537-L4610, L4637-L4710).

The loops have drifted in four ways:

1. Dry run with --preserve-state. Core returns WouldRevert before it checks keep_artifact (vendored_backend/mod.rs L380-L384).``

  • rollback handles this with a WouldRevert if preserve arm, which counts the key as preserved.
  • remove has no such arm. It prints "(artifact preserved)" but emits Verified / vendor_would_revert / "vendoring would be reverted on remove" and never bumps leg.preserved.
  • So a dry run reports a revert, while the wet run reports skipped / vendor_state_preserved.
  • I found this by reading the code; I did not run it, and no remove test covers it.

2. Go-undo warnings.

  • remove turns them into per-key Skipped events.
  • rollback turns them into run warnings and drops the key.

3. Drift-keep text.

  • remove uses cause.reason().
  • rollback writes its own "lockfile wiring drifted…" text.

4. Failures.

  • remove stops at the first failed key.
  • rollback collects every failure.
Proposed change
  1. Add a WouldPreserve step to core revert_one (opts.dry_run && opts.keep_artifact).
  2. Give VendoredBackend a typed per-run tally: reverted, preserved, kept-with-cause, failed and warnings.
  3. Leave remove and rollback with rendering only, in their own verb ("remove" / "roll back again").
  4. Decide in the PR whether remove keeps stop-at-first-failure. If it does, keep it as an explicit option on the tally driver rather than a second loop.
Size

3 files (vendored_backend/mod.rs, remove.rs, rollback.rs), about −150 lines.

Acceptance criteria
  • rollback's vendored tests (in_process_rollback_vendored.rs, rollback_duality_invariants.rs) and remove's vendored e2e tests pass unchanged.
  • A new test runs remove --preserve-state --dry-run --json over a vendored entry and asserts the same event code and the same preserved count as the wet run.
Vorherrschende Sprache
Rust
Sterne
8
Forks
2
Ø Merge
18 Std. 47 Min.
Gemergte PRs (30 T.)
637

Entwicklungsumgebung

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus SocketDev/socket-patch

Alle Issues in SocketDev/socket-patch

Ähnliche Issues

Weitere Issues zu Rust

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.