Skip to content

Commit 61717de

Browse files
committed
Audit only the Go modules an undo unwired
Review follow-up. The audit no longer runs inside the shared revert step, so a forward `vendor` run that reconciles a dropped entry does not warn mid-run and again at the end. The undo commands (`vendor --revert`, `rollback`, `remove`) now call it explicitly, and it reports only issues for the Go modules they unwired, never unrelated issues under the wrong package. This also drops the rollback dedup that was only needed for that case. `vendor --check` runs the audit once. The docs now name `vendor --revert` and `remove` among the commands that warn. Assisted-by: Claude Code:claude-opus-5-5
1 parent 959fe16 commit 61717de

6 files changed

Lines changed: 69 additions & 62 deletions

File tree

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,9 +1295,15 @@ async fn revert_vendored_matches(
12951295
let mut leg = RemoveVendorLeg::default();
12961296
// Stops at the first hard failure: remove aborts there, leaving the
12971297
// remaining matches (and the manifest) untouched.
1298-
let reverted = VendoredBackend::new(&args.common, None)
1299-
.revert(keys, state, opts, true)
1300-
.await;
1298+
let backend = VendoredBackend::new(&args.common, None);
1299+
let reverted = backend.revert(keys, state, opts, true).await;
1300+
for (key, w) in backend.go_undo_warnings(&reverted).await {
1301+
if loud {
1302+
eprintln!("Warning: {}", w.detail);
1303+
}
1304+
leg.skipped
1305+
.push(PatchEvent::new(PatchAction::Skipped, key).with_reason(w.code, w.detail));
1306+
}
13011307
for RevertedEntry {
13021308
key,
13031309
warnings,

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -989,9 +989,14 @@ async fn run_vendored_leg(
989989
keep_artifact: preserve,
990990
};
991991
let loud = !common.json && !common.silent;
992-
let reverted = VendoredBackend::new(common, None)
993-
.revert(keys, state, opts, false)
994-
.await;
992+
let backend = VendoredBackend::new(common, None);
993+
let reverted = backend.revert(keys, state, opts, false).await;
994+
for (_, w) in backend.go_undo_warnings(&reverted).await {
995+
if loud {
996+
eprintln!("Warning: {}", w.detail);
997+
}
998+
out.warnings.push((w.code.to_string(), w.detail));
999+
}
9951000
for RevertedEntry {
9961001
key,
9971002
warnings,
@@ -2253,18 +2258,11 @@ pub async fn run(args: RollbackArgs) -> i32 {
22532258
.chain(hosted_leg.warnings.iter())
22542259
.cloned()
22552260
.collect();
2256-
// Both Go legs audit the same go.mod and vendor/modules.txt
2257-
// (#1777): an issue the agent leg already reported is not
2258-
// repeated for the vendored one.
22592261
vendored_leg
22602262
.warnings
22612263
.iter()
22622264
.chain(hosted_leg.warnings.iter())
2263-
.for_each(|w| {
2264-
if !run_warnings.contains(w) {
2265-
run_warnings.push(w.clone());
2266-
}
2267-
});
2265+
.for_each(|(code, detail)| run_warnings.push((code.clone(), detail.clone())));
22682266
// A restored package whose ownership could not be put back, or
22692267
// whose stale bytecode could not be removed (the engine reports
22702268
// it on `error` with `success: true`), is restored but worth a

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

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,7 +1241,8 @@ async fn run_check(args: &VendorArgs) -> i32 {
12411241
} else {
12421242
None
12431243
};
1244-
let go_sync_issues = tokio::sync::OnceCell::new();
1244+
let go_sync_issues =
1245+
socket_patch_core::vendor::go_consumer_sync::audit(root, &HashMap::new()).await;
12451246
for (key, entry) in entries {
12461247
let record = entry.record.as_ref().or_else(|| manifest.patches.get(key));
12471248
let mut failure = match record {
@@ -1280,11 +1281,6 @@ async fn run_check(args: &VendorArgs) -> i32 {
12801281
if failure.is_none() && entry.ecosystem == "golang" {
12811282
if let Some((module, _)) = socket_patch_core::utils::purl::parse_golang_purl(key) {
12821283
let issues: Vec<String> = go_sync_issues
1283-
.get_or_init(|| async {
1284-
socket_patch_core::vendor::go_consumer_sync::audit(root, &HashMap::new())
1285-
.await
1286-
})
1287-
.await
12881284
.iter()
12891285
.filter(|issue| issue.module() == module)
12901286
.map(ToString::to_string)
@@ -1374,10 +1370,6 @@ async fn run_check(args: &VendorArgs) -> i32 {
13741370
.map(|(module, _)| module.into_owned())
13751371
.collect();
13761372
let stale_records: Vec<(String, String)> = go_sync_issues
1377-
.get_or_init(|| async {
1378-
socket_patch_core::vendor::go_consumer_sync::audit(root, &HashMap::new()).await
1379-
})
1380-
.await
13811373
.iter()
13821374
.filter(|issue| {
13831375
matches!(
@@ -4480,14 +4472,18 @@ async fn run_revert(args: &VendorArgs, env: &mut Envelope) -> i32 {
44804472
// (rollback's vendored leg, both of remove's paths, the manifest
44814473
// reconcile): dispatch → drift-keep → per-entry ledger save. Only the
44824474
// event vocabulary and the human lines are this command's.
4483-
let reverted = VendoredBackend::new(common, None)
4475+
let backend = VendoredBackend::new(common, None);
4476+
let reverted = backend
44844477
.revert(
44854478
&recorded,
44864479
&mut state,
44874480
RevertOpts::new(common.dry_run),
44884481
false,
44894482
)
44904483
.await;
4484+
for (purl, w) in backend.go_undo_warnings(&reverted).await {
4485+
record_warning(env, &purl, &w, common);
4486+
}
44914487
for RevertedEntry {
44924488
key: purl,
44934489
flavor,

‎crates/socket-patch-cli/src/commands/vendored_backend/mod.rs‎

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -142,18 +142,13 @@ impl<'a> VendoredBackend<'a> {
142142
stop_on_failure: bool,
143143
) -> Vec<RevertedEntry> {
144144
let mut out = Vec::with_capacity(keys.len());
145-
let mut unwired_go = Vec::new();
146145
for key in keys {
147146
// Captured before a clean revert drops the entry.
148147
let flavor = state.entries.get(key).and_then(|e| e.flavor.clone());
149148
let gem = state
150149
.entries
151150
.get(key)
152151
.map(|e| (e.ecosystem.clone(), e.base_purl.clone()));
153-
let golang = state
154-
.entries
155-
.get(key)
156-
.is_some_and(|e| e.ecosystem == "golang");
157152
let mut result = revert_vendor_entry(&self.common.cwd, key, state, opts).await;
158153
let hard_failure = matches!(
159154
result.step,
@@ -192,9 +187,6 @@ impl<'a> VendoredBackend<'a> {
192187
.await;
193188
}
194189
}
195-
if golang && unwired && !matches!(result.step, VendorRevertStep::WouldRevert) {
196-
unwired_go.push(out.len());
197-
}
198190
out.push(RevertedEntry {
199191
key: key.clone(),
200192
flavor,
@@ -205,36 +197,50 @@ impl<'a> VendoredBackend<'a> {
205197
break;
206198
}
207199
}
208-
self.push_go_consumer_warnings(&unwired_go, &mut out).await;
209200
out
210201
}
211202

212-
/// Dropping a vendored `replace` leaves a committed vendor/modules.txt
213-
/// (or go.mod requirements) still describing it, which breaks every
214-
/// default build until `go mod vendor` runs (#343, #1777). Audit once
215-
/// after the loop and attach each issue to the unwired Go entry of its
216-
/// module (the first one when none matches), so every reverting
217-
/// command reports it.
218-
async fn push_go_consumer_warnings(&self, unwired_go: &[usize], out: &mut [RevertedEntry]) {
219-
let Some(&first) = unwired_go.first() else {
220-
return;
221-
};
222-
let issues =
223-
socket_patch_core::vendor::go_consumer_sync::audit(&self.common.cwd, &HashMap::new())
224-
.await;
225-
for issue in issues {
226-
let owner = unwired_go
227-
.iter()
228-
.copied()
229-
.find(|&i| {
230-
socket_patch_core::utils::purl::parse_golang_purl(&out[i].key)
231-
.is_some_and(|(module, _)| module == issue.module())
232-
})
233-
.unwrap_or(first);
234-
out[owner]
235-
.warnings
236-
.push(VendorWarning::new(issue.code(), issue.to_string()));
203+
/// Dropping a vendored Go `replace` leaves a committed
204+
/// vendor/modules.txt still recording it, which breaks every default
205+
/// build until `go mod vendor` runs (#343, #1777). The undo commands
206+
/// call this after [`Self::revert`]: it audits the Go consumer state
207+
/// once and returns the issues of the Go modules `reverted` unwired,
208+
/// each keyed by the ledger key of its entry. Not part of `revert`
209+
/// itself, whose forward-run caller (the manifest reconcile) audits
210+
/// once at the end of the run instead.
211+
pub(crate) async fn go_undo_warnings(
212+
&self,
213+
reverted: &[RevertedEntry],
214+
) -> Vec<(String, VendorWarning)> {
215+
let unwired: Vec<(&str, String)> = reverted
216+
.iter()
217+
.filter(|e| {
218+
matches!(
219+
e.step,
220+
VendorRevertStep::Reverted
221+
| VendorRevertStep::Preserved
222+
| VendorRevertStep::LedgerWriteFailed(_)
223+
)
224+
})
225+
.filter_map(|e| {
226+
socket_patch_core::utils::purl::parse_golang_purl(&e.key)
227+
.map(|(module, _)| (e.key.as_str(), module.into_owned()))
228+
})
229+
.collect();
230+
if unwired.is_empty() {
231+
return Vec::new();
237232
}
233+
socket_patch_core::vendor::go_consumer_sync::audit(&self.common.cwd, &HashMap::new())
234+
.await
235+
.into_iter()
236+
.filter_map(|issue| {
237+
let (key, _) = unwired.iter().find(|(_, m)| m == issue.module())?;
238+
Some((
239+
key.to_string(),
240+
VendorWarning::new(issue.code(), issue.to_string()),
241+
))
242+
})
243+
.collect()
238244
}
239245
}
240246

‎crates/socket-patch-cli/tests/e2e_vendor_golang_build.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1167,7 +1167,7 @@ fn vendored_undo_needs_go_mod_vendor(label: &str, undo: &[&str], workspace: bool
11671167
// still reports as unledgered; only the modules.txt drift is gone.)
11681168
let (_code, so, se) = run_socket(&consumer, &["vendor", "--check", "--cwd", cs], &modcache);
11691169
assert!(
1170-
!se.contains("go_vendor_modules_txt_out_of_sync"),
1170+
!se.contains("go_vendor_modules_txt_out_of_sync") && !so.contains("modules.txt"),
11711171
"[{label}] in sync after {named}:\n{so}\n{se}"
11721172
);
11731173
}

‎docs/ecosystems.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,8 +1475,9 @@ for it until `rollback -g`.
14751475

14761476
Two files the go command derives from `go.mod` can disagree with a new or removed
14771477
socket `replace`, and go then refuses to build. socket-patch does not regenerate
1478-
them (that needs the go toolchain and the whole module graph); `apply`, `vendor`,
1479-
`rollback` and `scan --mode hosted` warn with the command that does, and
1478+
them (that needs the go toolchain and the whole module graph); `apply`, `vendor`
1479+
(including `vendor --revert`), `rollback`, `remove` and `scan --mode hosted` warn
1480+
with the command that does, and
14801481
`apply --check` / `vendor --check` report the disagreement as drift until it is run:
14811482

14821483
| Warning code | When | Fix |

0 commit comments

Comments
 (0)