Skip to content

Commit 060e1c2

Browse files
mikolalysenkoclaude
andcommitted
Stop calling a residual-reference keep drift
When a vendored PyPI revert restored its recorded wiring but kept the wheel because another project file (a `pipenv requirements` export) still installs from it, `remove` and `rollback` reported the keep as "lockfile wiring drifted" and told the user to re-run `scan --mode vendored` to normalize, then remove. That remedy loops: the re-vendor re-wires Pipfile.lock and the next remove keeps the entry again. The keep now carries its cause. `vendor_artifact_kept`, the remove warning, skip reason and top-level error, rollback's vendoredKept reason and `vendor --revert` / reconcile skips say a project file still installs from the artifact, and the remedy is to point that file back at the registry release (or re-export it from the restored lock) and run the unwind again. A hosted takeover refused for the same reason names the file instead of a wiring edit. Drift keeps keep their existing wording. Fixes #1184 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 2d4d430 commit 060e1c2

9 files changed

Lines changed: 364 additions & 55 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1266,7 +1266,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified
12661266
| `vendor_state_retained` | `skipped` | remove `--skip-rollback`: vendor wiring + artifact deliberately left in place (the next `vendor` run reconciles the dropped entry). Also the top-level error code when `--skip-rollback` targets a vendored patch with no manifest record (every `scan`/`get --mode vendored` entry — and, v5.0, the ledger-only leftover of an earlier `remove --skip-rollback` of a manifest-tracked vendored patch). |
12671267
| `hosted_state_retained` | (top-level error) | remove `--skip-rollback` targeting a hosted-only patch (no manifest entry): restoring the pin's upstream registry entry is the only possible removal, so the combination is refused (exit 1), mirroring the manifest-less vendored refusal above. |
12681268
| `vendor_state_preserved` | `skipped` | remove `--preserve-state` (v5.0): lockfile unwired; artifact, ledger entry, and manifest entry all kept for a later re-apply. Rollback's counterpart is the `vendoredPreserved: []` envelope array. |
1269-
| `vendor_revert_kept` | `skipped` + top-level error | remove (v5.0): the vendored revert drift-kept (`kept_artifact`), so the ledger entry AND the manifest entry were both kept. ANY drift-keep makes the run a `partialFailure` (exit 1) — part of the requested removal did not happen; when EVERY matching entry drift-kept, the top-level error carries this code (`summary.removed` stays 0; the identifier DID match, so never `not_found`). Remedy: re-run `scan --mode vendored` to normalize, then remove. Rollback's counterpart is the `vendoredKept: []` envelope array (also exit 1). |
1269+
| `vendor_revert_kept` | `skipped` + top-level error | remove (v5.0): the vendored revert drift-kept (`kept_artifact`), so the ledger entry AND the manifest entry were both kept. ANY drift-keep makes the run a `partialFailure` (exit 1) — part of the requested removal did not happen; when EVERY matching entry drift-kept, the top-level error carries this code (`summary.removed` stays 0; the identifier DID match, so never `not_found`). Remedy: re-run `scan --mode vendored` to normalize, then remove. A PyPI revert that restored its recorded wiring but kept the wheel because another project file still installs from it (`vendor_revert_residual_reference`, e.g. a `pipenv requirements` / `uv export` / `poetry export` requirements file) is a keep too, but not a drift (#1184): its skip reason, the per-entry warning, the top-level message and rollback's `vendoredKept[].reason` say that a project file still installs from the vendored artifact, and the remedy is to point the file named by `vendor_revert_residual_reference` back at the registry release (or re-export it from the restored lock), then remove (or roll back) again. Its `vendor_artifact_kept` advisory says the same, and a hosted takeover refused for it (`redirect_vendored_revert_failed`) names the file instead of a wiring edit. Rollback's counterpart is the `vendoredKept: []` envelope array (also exit 1). |
12701270
| `hosted_reverted` | `removed` | remove (v5.0): a hosted lockfile pin was restored to its upstream registry entry as part of removing the patch (`verified` on dry-run). Beside a manifest entry it bypasses `summary.removed` like `vendor_reverted`. |
12711271
| `hosted_revert_failed` | top-level error | remove (v5.0): a matched hosted pin could not be restored to its upstream registry entry (`--offline`, a registry that does not answer, `bun.lockb`, a lock shape the restore refuses — see "Hosted unwind coverage"), or writing the restored files failed; the message names the `git checkout -- <files>` remedy. The manifest was not modified, exit 1. Rollback's counterpart is a `hosted.failed[]` entry (also `partial_failure` exit 1). v4's `hosted_revert_unsupported` is no longer emitted (every ecosystem has a restore). |
12721272
| `reinstall_required` | rollback `warnings[]` | rollback (v5.0): vendored/hosted wiring was unwound, but installed trees keep their patched bytes until the next package-manager install — the stale-install advisory. When the run also emits a Bun advisory (`vendor_bun_reinstall_required` / `redirect_bun_reinstall_required`) the detail and the human note add " (Bun: a plain `bun install` keeps them; run `bun install --force`)". |

‎crates/socket-patch-cli/src/commands/remove.rs‎

Lines changed: 71 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ use super::rollback::{rollback_patches_inner, InnerSelection};
1818
use crate::args::{apply_env_toggles, GlobalArgs};
1919
use crate::commands::hosted_unwind::{run_hosted_leg, HostedLegOutcome};
2020
use crate::commands::lock_cli::acquire_or_emit;
21-
use crate::commands::vendored_backend::{RevertedEntry, VendorRevertStep, VendoredBackend};
21+
use crate::commands::vendored_backend::{
22+
KeepCause, RevertedEntry, VendorRevertStep, VendoredBackend,
23+
};
2224
use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent, Status};
2325
use crate::ui::short_uuid;
2426
use crate::ui::{plural, sweep_failure};
@@ -863,11 +865,7 @@ pub async fn run(args: RemoveArgs) -> i32 {
863865
// this is the only way the removal can be empty): the remove did
864866
// not happen. NOT not_found; partialFailure keeps `summary.removed`
865867
// honest at 0.
866-
let msg = format!(
867-
"{}: every matching entry's vendored state drift-kept; nothing was \
868-
removed (re-run `scan --mode vendored` to normalize, then remove)",
869-
args.identifier
870-
);
868+
let msg = all_kept_message(&args.identifier, &vendor_leg.kept_causes);
871869
track_patch_remove_failed(&msg, &telemetry).await;
872870
if args.common.json {
873871
let mut env = Envelope::new(Command::Remove);
@@ -1125,21 +1123,62 @@ pub async fn run(args: RemoveArgs) -> i32 {
11251123
// above are gated, so name the outcome once here.
11261124
if !args.common.json {
11271125
eprintln!(
1128-
"Error: {} matching entr{} drift-kept (vendored state and manifest \
1129-
record retained); re-run `scan --mode vendored` to normalize, then \
1130-
remove again",
1131-
vendor_leg.kept.len(),
1132-
if vendor_leg.kept.len() == 1 {
1133-
"y was"
1134-
} else {
1135-
"ies were"
1136-
}
1126+
"Error: {}",
1127+
kept_error_line(&vendor_leg.kept_causes, "manifest record")
11371128
);
11381129
}
11391130
1
11401131
}
11411132
}
11421133

1134+
/// The top-level error when every matching vendored entry was kept: the
1135+
/// drift wording and its normalize remedy only for drift-keeps (#1184).
1136+
fn all_kept_message(identifier: &str, causes: &[KeepCause]) -> String {
1137+
if causes.iter().all(|c| *c == KeepCause::Drift) {
1138+
return format!(
1139+
"{identifier}: every matching entry's vendored state drift-kept; nothing was \
1140+
removed (re-run `scan --mode vendored` to normalize, then remove)"
1141+
);
1142+
}
1143+
format!(
1144+
"{identifier}: every matching entry's vendored state was kept; nothing was removed \
1145+
({})",
1146+
kept_remedies(causes)
1147+
)
1148+
}
1149+
1150+
/// The human error line for kept entries (`retained` names the record kept
1151+
/// beside the vendored state).
1152+
fn kept_error_line(causes: &[KeepCause], retained: &str) -> String {
1153+
let n = causes.len();
1154+
let entries = if n == 1 { "y was" } else { "ies were" };
1155+
if causes.iter().all(|c| *c == KeepCause::Drift) {
1156+
return format!(
1157+
"{n} matching entr{entries} drift-kept (vendored state and {retained} \
1158+
retained); re-run `scan --mode vendored` to normalize, then remove again"
1159+
);
1160+
}
1161+
format!(
1162+
"{n} matching entr{entries} kept (vendored state and {retained} retained); {}",
1163+
kept_remedies(causes)
1164+
)
1165+
}
1166+
1167+
/// One remedy per distinct keep cause.
1168+
fn kept_remedies(causes: &[KeepCause]) -> String {
1169+
let mut out: Vec<String> = Vec::new();
1170+
for cause in [KeepCause::Reference, KeepCause::Drift] {
1171+
if causes.contains(&cause) {
1172+
let remedy = cause.remedy("remove again");
1173+
out.push(match cause {
1174+
KeepCause::Reference => remedy,
1175+
KeepCause::Drift => format!("for a drift-kept entry, {remedy}"),
1176+
});
1177+
}
1178+
}
1179+
out.join("; ")
1180+
}
1181+
11431182
/// The vendored leg's envelope material, collected by
11441183
/// [`revert_vendored_matches`].
11451184
#[derive(Default)]
@@ -1150,9 +1189,11 @@ struct RemoveVendorLeg {
11501189
/// Backend warnings, drift-keeps and preserved entries — `Skipped`
11511190
/// events.
11521191
skipped: Vec<PatchEvent>,
1153-
/// Ledger keys whose revert drift-kept: entry, artifact and any
1192+
/// Ledger keys whose revert kept the artifact: entry, artifact and any
11541193
/// manifest record stay.
11551194
kept: Vec<String>,
1195+
/// Why each [`Self::kept`] entry was kept (same order).
1196+
kept_causes: Vec<KeepCause>,
11561197
/// Entries actually reverted and dropped from the ledger (wet runs).
11571198
reverted_count: usize,
11581199
/// Entries unwired with their artifact and ledger entry kept
@@ -1223,29 +1264,29 @@ async fn revert_vendored_matches(
12231264
);
12241265
return Err(1);
12251266
}
1226-
VendorRevertStep::Kept => {
1227-
// Drift-keep: the lock changed under us and the backend
1228-
// left both the wiring and the artifact alone. Per the
1229-
// RevertOutcome contract the ledger entry stays — and so
1230-
// must any manifest entry, or `vendor`'s reconcile would
1231-
// re-revert an entry whose backing record is gone.
1267+
VendorRevertStep::Kept(cause) => {
1268+
// The backend kept the artifact (a drift-keep, or a file
1269+
// that still installs from it). Per the RevertOutcome
1270+
// contract the ledger entry stays — and so must any
1271+
// manifest entry, or `vendor`'s reconcile would re-revert
1272+
// an entry whose backing record is gone.
1273+
let reason = cause.reason();
12321274
let (note, detail) = if manifest_backed {
12331275
(
12341276
"; its manifest entry was kept too",
1235-
"lockfile wiring drifted; vendored state and manifest entry kept",
1277+
format!("{reason}; vendored state and manifest entry kept"),
12361278
)
12371279
} else {
12381280
(
12391281
"",
1240-
"lockfile wiring drifted; vendored state and ledger entry kept",
1282+
format!("{reason}; vendored state and ledger entry kept"),
12411283
)
12421284
};
12431285
if loud {
1244-
eprintln!(
1245-
"Warning: Kept vendored state for {key}: lockfile wiring drifted{note}"
1246-
);
1286+
eprintln!("Warning: Kept vendored state for {key}: {reason}{note}");
12471287
}
12481288
leg.kept.push(key.clone());
1289+
leg.kept_causes.push(cause);
12491290
leg.skipped.push(
12501291
PatchEvent::new(PatchAction::Skipped, key.clone())
12511292
.with_reason("vendor_revert_kept", detail),
@@ -1571,11 +1612,7 @@ async fn remove_ledger_only(
15711612
// top-level error names the outcome — nothing was removed.
15721613
env.mark_partial_failure();
15731614
if leg.kept.len() == keys.len() {
1574-
let msg = format!(
1575-
"{}: every matching entry's vendored state drift-kept; nothing was \
1576-
removed (re-run `scan --mode vendored` to normalize, then remove)",
1577-
args.identifier
1578-
);
1615+
let msg = all_kept_message(&args.identifier, &leg.kept_causes);
15791616
track_patch_remove_failed(&msg, telemetry).await;
15801617
env.error = Some(EnvelopeError::new("vendor_revert_kept", msg));
15811618
}
@@ -1593,14 +1630,8 @@ async fn remove_ledger_only(
15931630
// are gated, so name the outcome once here.
15941631
if !args.common.json {
15951632
eprintln!(
1596-
"Error: {} matching entr{} drift-kept (vendored state and ledger record \
1597-
retained); re-run `scan --mode vendored` to normalize, then remove again",
1598-
leg.kept.len(),
1599-
if leg.kept.len() == 1 {
1600-
"y was"
1601-
} else {
1602-
"ies were"
1603-
}
1633+
"Error: {}",
1634+
kept_error_line(&leg.kept_causes, "ledger record")
16041635
);
16051636
}
16061637
1

‎crates/socket-patch-cli/src/commands/rollback.rs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ use std::time::Duration;
2727
use crate::args::{apply_env_toggles, is_local_go, parse_bool_flag, GlobalArgs};
2828
use crate::commands::hosted_unwind::run_hosted_leg;
2929
use crate::commands::lock_cli::acquire_or_emit;
30-
use crate::commands::vendored_backend::{RevertedEntry, VendorRevertStep, VendoredBackend};
30+
use crate::commands::vendored_backend::{
31+
KeepCause, RevertedEntry, VendorRevertStep, VendoredBackend,
32+
};
3133
use crate::ecosystem_dispatch::{
3234
distinct_npm_copies, find_all_packages_for_rollback, partition_purls, JvmScope,
3335
};
@@ -794,10 +796,18 @@ async fn run_vendored_leg(
794796
}
795797
out.failed.push((key, why));
796798
}
797-
VendorRevertStep::Kept => out.kept.push((
799+
VendorRevertStep::Kept(KeepCause::Drift) => out.kept.push((
798800
key,
799801
"lockfile wiring drifted; vendored state left untouched".to_string(),
800802
)),
803+
VendorRevertStep::Kept(cause @ KeepCause::Reference) => out.kept.push((
804+
key,
805+
format!(
806+
"{}; vendored state kept — {}",
807+
cause.reason(),
808+
cause.remedy("roll back again")
809+
),
810+
)),
801811
VendorRevertStep::WouldRevert if preserve => {
802812
if loud {
803813
println!("Would unwire vendoring for {key} (artifact preserved)");

‎crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,11 @@ impl Takeover {
253253
outcome.error.as_deref().unwrap_or("unknown error")
254254
),
255255
}))
256+
} else if let Some(residual) = residual_only_keep(&outcome) {
257+
// Nothing drifted: another project file (an exported
258+
// requirements file) still installs from the vendored
259+
// wheel (#1184). Name it, not a drift.
260+
Some(residual_takeover_warning(&purl, &residual))
256261
} else if revert_keeps_wiring(&outcome) {
257262
// A wiring record drifted and was left in place, so the
258263
// project may still resolve through the vendored artifact
@@ -552,6 +557,45 @@ fn revert_keeps_wiring(outcome: &RevertOutcome) -> bool {
552557
.any(|w| w.code == "vendor_revert_residual_reference")
553558
}
554559

560+
/// The `vendor_revert_residual_reference` detail of a revert whose ONLY
561+
/// keep signal is a file that still references the artifact.
562+
fn residual_only_keep(outcome: &RevertOutcome) -> Option<String> {
563+
if outcome.drift_skipped() {
564+
return None;
565+
}
566+
outcome
567+
.warnings
568+
.iter()
569+
.find(|w| w.code == socket_patch_core::vendor::RESIDUAL_REFERENCE_CODE)
570+
.map(|w| {
571+
// `kept <dir>: <file> still resolves through it, and deleting …`:
572+
// only the clause naming the file; the revert's own remedy is
573+
// not this command's.
574+
let detail = w.detail.as_str();
575+
detail
576+
.split_once(": ")
577+
.map_or(detail, |(_, rest)| rest)
578+
.split(", and deleting")
579+
.next()
580+
.unwrap_or(detail)
581+
.to_string()
582+
})
583+
}
584+
585+
/// The refusal for a takeover whose vendored wheel another project file
586+
/// still installs from.
587+
fn residual_takeover_warning(purl: &str, residual: &str) -> serde_json::Value {
588+
serde_json::json!({
589+
"code": "redirect_vendored_revert_failed",
590+
"detail": format!(
591+
"{purl} is vendored and another project file still installs from its \
592+
vendored wheel ({residual}), so it is left in place; NOT switched to hosted — \
593+
point that file back at the registry release (re-export it once the hosted \
594+
scan has rewired the lock), then re-run `scan --mode hosted`"
595+
),
596+
})
597+
}
598+
555599
/// The refusal for a takeover whose vendored wiring drifted since vendoring.
556600
fn drifted_takeover_warning(purl: &str) -> serde_json::Value {
557601
serde_json::json!({

‎crates/socket-patch-cli/src/commands/vendor.rs‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ use crate::commands::apply::{representative_file, result_to_event, variant_match
5151
use crate::commands::bun_preflight::bun_vendor_preflight_pairs;
5252
use crate::commands::lock_cli::acquire_or_emit;
5353
use crate::commands::vendored_backend::{
54-
ApplyRequest, RevertedEntry, VendorRevertStep, VendoredBackend,
54+
ApplyRequest, KeepCause, RevertedEntry, VendorRevertStep, VendoredBackend,
5555
};
5656
use crate::commands::vex::{
5757
generate_vex_from_manifest_path, generate_vex_without_manifest, ManifestlessVex, VexEmbedArgs,
@@ -4105,13 +4105,24 @@ pub(crate) async fn reconcile_dropped(
41054105
// Drift-skip keep: the backend left the drifted lock alone and
41064106
// kept the artifacts, so the ledger entry must survive too — and
41074107
// the genuine outcome is a COUNTED skip, not a removal.
4108-
VendorRevertStep::Kept => env.record(
4108+
VendorRevertStep::Kept(KeepCause::Drift) => env.record(
41094109
PatchEvent::new(PatchAction::Skipped, purl.clone()).with_reason(
41104110
"vendor_revert_kept",
41114111
"patch no longer in manifest, but its lock entries drifted since \
41124112
vendoring; artifacts and ledger entry kept",
41134113
),
41144114
),
4115+
VendorRevertStep::Kept(cause @ KeepCause::Reference) => env.record(
4116+
PatchEvent::new(PatchAction::Skipped, purl.clone()).with_reason(
4117+
"vendor_revert_kept",
4118+
format!(
4119+
"patch no longer in manifest, but {}; artifacts and ledger entry \
4120+
kept — {}",
4121+
cause.reason(),
4122+
cause.remedy("re-run `vendor`")
4123+
),
4124+
),
4125+
),
41154126
VendorRevertStep::WouldRevert | VendorRevertStep::Reverted => {
41164127
if !common.json && !common.silent {
41174128
println!("{}", format_reconciled(&purl, common.dry_run));
@@ -4208,13 +4219,23 @@ async fn run_revert(args: &VendorArgs, env: &mut Envelope) -> i32 {
42084219
// the genuine outcome is a COUNTED skip, not a removal.
42094220
// (`record_warning` above already surfaced the per-record
42104221
// details as uncounted advisory events.)
4211-
VendorRevertStep::Kept => env.record(
4222+
VendorRevertStep::Kept(KeepCause::Drift) => env.record(
42124223
PatchEvent::new(PatchAction::Skipped, purl.clone()).with_reason(
42134224
"vendor_revert_kept",
42144225
"lock entries drifted since vendoring; artifacts and ledger entry kept \
42154226
— undo the drift and re-run `vendor --revert` to finish",
42164227
),
42174228
),
4229+
VendorRevertStep::Kept(cause @ KeepCause::Reference) => env.record(
4230+
PatchEvent::new(PatchAction::Skipped, purl.clone()).with_reason(
4231+
"vendor_revert_kept",
4232+
format!(
4233+
"{}; artifacts and ledger entry kept — {}",
4234+
cause.reason(),
4235+
cause.remedy("re-run `vendor --revert` to finish")
4236+
),
4237+
),
4238+
),
42184239
VendorRevertStep::WouldRevert | VendorRevertStep::Reverted => {
42194240
env.record(PatchEvent::new(PatchAction::Removed, purl.clone()));
42204241
reverted_flavors.extend(flavor);

0 commit comments

Comments
 (0)