Skip to content

Commit 01e40cb

Browse files
Fix vendored planning ignoring PyPI refusals (#1908) (#1930)
* Start fix for #1908 Assisted-by: Claude Code:claude-opus-5-5 * Fix vendored planning ignoring PyPI refusals scan --mode vendored chose which new patches may take a --max-new-patches slot through its own copy of the preflight checks, and that copy never ran the PyPI wiring guards. A PyPI patch the wet run refuses before downloading (pypi_no_requirements, pypi_uv_no_sources) still took a rollout slot and pushed an installable patch to the next run, while --dry-run showed the same patch as would_refuse. The planning pass now reads the dry-run preview's would-refuse rows, so the two can no longer disagree. preview_vendor takes the run's GlobalArgs and resolves the patch-server origins and gem takeover refusals itself, and scan computes it once per run, reusing it for the dry-run rendering instead of running every preflight twice. Fixes #1908 Assisted-by: Claude Code:claude-opus-5-5 * Tidy the shared vendored preview The dry-run rendering now reuses the planning pass's preview directly instead of keeping an unreachable recompute path, the preview tests call the inner helper by its own name, and get's layering allowlist drops its import of vendor, which it no longer needs. Refs #1908 Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3dcb53b commit 01e40cb

4 files changed

Lines changed: 103 additions & 91 deletions

File tree

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

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2284,18 +2284,7 @@ async fn run_get_vendored(
22842284
// Dry run: ledger-classification preview only (scan's posture) — no
22852285
// download, no vendor step, no writes.
22862286
if args.common.dry_run {
2287-
let takeover = super::vendor::gem_takeover_preview_refusals(
2288-
&args.common,
2289-
selected.iter().map(|p| p.purl.as_str()),
2290-
)
2291-
.await;
2292-
let preview = super::scan::preview_vendor(
2293-
&args.common.cwd,
2294-
selected,
2295-
&super::hosted_unwind::patch_server_origins(&args.common),
2296-
&takeover,
2297-
)
2298-
.await;
2287+
let preview = super::scan::preview_vendor(&args.common, selected).await;
22992288
if args.common.json {
23002289
preview.record_into(&mut env);
23012290
emit(&env);

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

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3297,13 +3297,17 @@ async fn run_scan(
32973297
};
32983298

32993299
// The rollout plan (§5.2): deferred NEW rows leave the selection here.
3300-
// Vendored eligibility is the wet run's Bun / vlt / npm-lock / gem
3301-
// takeover preflight; the agent partition above already removed what
3302-
// cannot land in place.
3300+
// Vendored eligibility is the dry-run preview's would-refuse verdicts
3301+
// (the wet run's preflights and takeover gates), computed once and
3302+
// reused by the dry-run rendering below; the agent partition above
3303+
// already removed what cannot land in place.
3304+
let mut vendor_preview = None;
33033305
let selected = if report_only {
33043306
selected
33053307
} else if vendor {
3306-
let refused = vendor_flow::preflight_refused_purls(&args.common, &selected).await;
3308+
let preview = preview_vendor(&args.common, &selected).await;
3309+
let refused = preview.refused_purls();
3310+
vendor_preview = Some(preview);
33073311
stage.plan(&rows, |r| !refused.contains(&r.writer.purl));
33083312
let deferred = stage.deferred_keys();
33093313
selected
@@ -3422,20 +3426,13 @@ async fn run_scan(
34223426
// rendered as `[would-refuse]` lines so a preview never
34233427
// advertises vendoring the wet run would refuse.
34243428
let preview = if vendor {
3425-
let takeover = crate::commands::vendor::gem_takeover_preview_refusals(
3426-
&args.common,
3427-
selected.iter().map(|p| p.purl.as_str()),
3428-
)
3429-
.await;
3430-
Some(
3431-
preview_vendor(
3432-
&args.common.cwd,
3433-
&selected,
3434-
&crate::commands::hosted_unwind::patch_server_origins(&args.common),
3435-
&takeover,
3436-
)
3437-
.await,
3438-
)
3429+
// `vendor` is never report-only, so the planning pass
3430+
// above always computed it.
3431+
let mut preview = vendor_preview
3432+
.take()
3433+
.expect("the vendored planning pass computed the preview");
3434+
preview.retain_selected(&selected);
3435+
Some(preview)
34393436
} else {
34403437
None
34413438
};

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

Lines changed: 87 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,30 @@ impl VendorPreview {
110110
.count()
111111
}
112112

113+
/// The purls the wet run would refuse: the rollout planning pass, so a
114+
/// refused NEW patch holds no `--max-new-patches` slot (it still
115+
/// reaches the engine, which reports the refusal).
116+
pub(crate) fn refused_purls(&self) -> HashSet<String> {
117+
self.rows
118+
.iter()
119+
.filter(|r| matches!(r.verdict, PreviewVerdict::WouldRefuse { .. }))
120+
.map(|r| r.purl.clone())
121+
.collect()
122+
}
123+
124+
/// Keep only the rows of `selected` (the patches the rollout plan left
125+
/// in the run), so a preview computed before planning renders exactly
126+
/// what this run would vendor. Every verdict is per patch, so a row
127+
/// does not depend on which other patches were previewed with it.
128+
pub(crate) fn retain_selected(&mut self, selected: &[PatchSearchResult]) {
129+
let keep: HashSet<(&str, &str)> = selected
130+
.iter()
131+
.map(|p| (p.purl.as_str(), p.uuid.as_str()))
132+
.collect();
133+
self.rows
134+
.retain(|r| keep.contains(&(r.purl.as_str(), r.uuid.as_str())));
135+
}
136+
113137
/// Record the preview into a dry-run envelope, every event carrying
114138
/// `details.mode: "vendored"`: `verified` for a patch the wet run would
115139
/// vendor (`oldUuid` on a re-vendor), `skipped` / `already_vendored`
@@ -178,19 +202,38 @@ impl VendorPreview {
178202
/// before any download, or whose hosted pnpm pin the takeover would fail
179203
/// to replace ([`hosted_pnpm_refusals`]: the pnpm backend's lock-text
180204
/// refusal, #853), and would-refuse for PyPI purls the PyPI backend's
181-
/// wiring guards refuse ([`pypi_refusals`], #891). `origins` are the
182-
/// run's `--patch-server-url` origins. The preview stays a ledger
205+
/// wiring guards refuse ([`pypi_refusals`], #891), plus the gem
206+
/// hosted→vendored takeover refusals
207+
/// ([`crate::commands::vendor::gem_takeover_preview_refusals`]). The preview stays a ledger
183208
/// classification otherwise (engine refusals outside the preflights are
184209
/// not predicted), and a would-refuse never flips the run's status or exit
185210
/// code. The preflights (the only disk access besides the ledger; the PyPI
186211
/// one may also probe `pipenv --version` and list Hatch environments, as
187212
/// the wet run's guards do) run only when the selection holds an npm or
188213
/// PyPI purl.
189-
/// `takeover_refusals` adds the hosted→vendored takeover refusals the
190-
/// caller resolved (the gem gates of
191-
/// [`crate::commands::vendor::gem_takeover_preview_refusals`]), keyed by
192-
/// the selected purl, as would-refuse rows too.
214+
///
215+
/// The one answer to "which selected patches would the wet run refuse?":
216+
/// the dry-run rows and the rollout planning pass
217+
/// ([`VendorPreview::refused_purls`]) both read it, so a patch the dry run
218+
/// shows as would-refuse never takes a `--max-new-patches` slot (#1908). Resolves the run's `--patch-server-url` origins and the
219+
/// gem takeover refusals itself.
193220
pub(crate) async fn preview_vendor(
221+
common: &GlobalArgs,
222+
selected: &[PatchSearchResult],
223+
) -> VendorPreview {
224+
let origins = crate::commands::hosted_unwind::patch_server_origins(common);
225+
let takeover = crate::commands::vendor::gem_takeover_preview_refusals(
226+
common,
227+
selected.iter().map(|p| p.purl.as_str()),
228+
)
229+
.await;
230+
preview_vendor_with(&common.cwd, selected, &origins, &takeover).await
231+
}
232+
233+
/// [`preview_vendor`] over caller-resolved `origins` (the run's
234+
/// `--patch-server-url` origins) and `takeover_refusals` (keyed by the
235+
/// selected purl, each a would-refuse row); the unit tests seed both.
236+
async fn preview_vendor_with(
194237
cwd: &Path,
195238
selected: &[PatchSearchResult],
196239
origins: &[String],
@@ -341,43 +384,6 @@ pub(crate) fn merge_vendor_envelope(env: &mut Envelope, venv: Envelope) {
341384
}
342385
}
343386

344-
/// The purls of `selected` the wet run's Bun, vlt or npm package-lock
345-
/// preflight, the hosted pnpm takeover's lock-text gates, or the gem
346-
/// hosted→vendored takeover gate, would refuse before any download (the
347-
/// `would_refuse` rows of [`preview_vendor_json`]): the vendored planning
348-
/// pass, so a refused NEW patch holds no rollout slot.
349-
pub(super) async fn preflight_refused_purls(
350-
common: &GlobalArgs,
351-
selected: &[PatchSearchResult],
352-
) -> HashSet<String> {
353-
let cwd = common.cwd.as_path();
354-
let state = load_state(cwd).await;
355-
let refusal =
356-
bun_vendor_preflight_with_ledger(cwd, selected, state.as_ref().map(|s| &s.entries)).await;
357-
let vlt_refusals =
358-
vlt_vendor_preflight_selected(cwd, selected, state.as_ref().map(|s| &s.entries)).await;
359-
let npm_lock_refusal = npm_lock_refusal(cwd, selected).await;
360-
let origins = crate::commands::hosted_unwind::patch_server_origins(common);
361-
let pnpm_refusals =
362-
hosted_pnpm_refusals(cwd, selected, &origins, &state.unwrap_or_default()).await;
363-
let gem_refusals = crate::commands::vendor::gem_takeover_preview_refusals(
364-
common,
365-
selected.iter().map(|p| p.purl.as_str()),
366-
)
367-
.await;
368-
selected
369-
.iter()
370-
.filter(|p| {
371-
refusal.as_ref().is_some_and(|r| r.applies_to(&p.purl))
372-
|| vlt_refusal_for(&vlt_refusals, &p.purl).is_some()
373-
|| (p.purl.starts_with("pkg:npm/") && npm_lock_refusal.is_some())
374-
|| pnpm_refusals.contains_key(&p.purl)
375-
|| gem_refusals.contains_key(&p.purl)
376-
})
377-
.map(|p| p.purl.clone())
378-
.collect()
379-
}
380-
381387
/// The npm package-lock backend's project-level refusal (a lock that is
382388
/// not v2/v3, see [`socket_patch_core::vendor::npm_lock_vendor_preflight`]),
383389
/// which refuses every npm purl of the project before any download or
@@ -768,8 +774,8 @@ async fn run_vendor_json_path(
768774
// The planning pass: a patch the preflight refuses holds no slot (it
769775
// still reaches the engine, which reports the refusal).
770776
let writers = writers_of(&rows);
771-
let origins = crate::commands::hosted_unwind::patch_server_origins(&args.common);
772-
let refused = preflight_refused_purls(&args.common, &writers).await;
777+
let mut preview = preview_vendor(&args.common, &writers).await;
778+
let refused = preview.refused_purls();
773779
stage.plan(&rows, |r| !refused.contains(&r.writer.purl));
774780
let deferred = stage.deferred_keys();
775781
let selected: Vec<PatchSearchResult> = writers
@@ -782,14 +788,8 @@ async fn run_vendor_json_path(
782788
// No downloads, no backends: classify against the ledger
783789
// and preview the GC, exactly like agent mode's dry run (and the
784790
// human arm, which previews the same GC through `finish_human`).
785-
let takeover = crate::commands::vendor::gem_takeover_preview_refusals(
786-
&args.common,
787-
selected.iter().map(|p| p.purl.as_str()),
788-
)
789-
.await;
790-
preview_vendor(&args.common.cwd, &selected, &origins, &takeover)
791-
.await
792-
.record_into(env);
791+
preview.retain_selected(&selected);
792+
preview.record_into(env);
793793
if prune {
794794
gc_into(
795795
&args.common,
@@ -1394,7 +1394,7 @@ mod migration_tests {
13941394

13951395
#[cfg(test)]
13961396
mod preview_tests {
1397-
use super::{preview_vendor, PreviewVerdict};
1397+
use super::{preview_vendor_with, PreviewVerdict};
13981398
use crate::json_envelope::{Command, Envelope};
13991399
use socket_patch_core::api::types::PatchSearchResult;
14001400
use std::collections::HashMap;
@@ -1412,7 +1412,7 @@ mod preview_tests {
14121412
origins: &[String],
14131413
takeover: &HashMap<String, (&'static str, String)>,
14141414
) -> serde_json::Value {
1415-
let preview = preview_vendor(cwd, selected, origins, takeover).await;
1415+
let preview = preview_vendor_with(cwd, selected, origins, takeover).await;
14161416
let patches: Vec<serde_json::Value> = preview
14171417
.rows
14181418
.iter()
@@ -1450,7 +1450,7 @@ mod preview_tests {
14501450
async fn preview_records_verdicts_as_vendored_events() {
14511451
let tmp = tempfile::tempdir().unwrap();
14521452
seed_entry(tmp.path(), NPM, UUID);
1453-
let preview = preview_vendor(
1453+
let preview = preview_vendor_with(
14541454
tmp.path(),
14551455
&[sel(UUID, NPM), sel(UUID, PYPI)],
14561456
&[],
@@ -1918,6 +1918,37 @@ wheels = [
19181918
);
19191919
}
19201920

1921+
/// #1908: the rollout planning pass reads the same verdicts as the
1922+
/// preview, so a PyPI patch the backend refuses before any download
1923+
/// (here `pypi_uv_no_sources`) holds no `--max-new-patches` slot, while
1924+
/// the same project without the setting stays eligible.
1925+
#[tokio::test]
1926+
async fn planning_pass_excludes_a_pypi_refusal() {
1927+
let tmp = tempfile::tempdir().unwrap();
1928+
std::fs::write(tmp.path().join("uv.lock"), UV_LOCK).unwrap();
1929+
std::fs::write(
1930+
tmp.path().join("pyproject.toml"),
1931+
format!("{UV_PYPROJECT}\n[tool.uv]\nno-sources = true\n"),
1932+
)
1933+
.unwrap();
1934+
let common = crate::args::GlobalArgs {
1935+
cwd: tmp.path().to_path_buf(),
1936+
silent: true,
1937+
..Default::default()
1938+
};
1939+
let refused = super::preview_vendor(&common, &[sel(UUID, SIX)])
1940+
.await
1941+
.refused_purls();
1942+
assert!(refused.contains(SIX), "{refused:?}");
1943+
1944+
// Control: without the setting the patch keeps its slot.
1945+
std::fs::write(tmp.path().join("pyproject.toml"), UV_PYPROJECT).unwrap();
1946+
let refused = super::preview_vendor(&common, &[sel(UUID, SIX)])
1947+
.await
1948+
.refused_purls();
1949+
assert!(refused.is_empty(), "{refused:?}");
1950+
}
1951+
19211952
/// #891: a PyPI purl with a live hosted pin is not judged by the PyPI
19221953
/// guards on the still-hosted text: the wet run restores the upstream
19231954
/// pin first, so the preview keeps it `would_vendor`.

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,6 @@ const ALLOWED_COMMAND_IMPORTS: &[(&str, &str, &str)] = &[
5151
"vex",
5252
"get's vendored leg reads VexEmbedArgs defaults",
5353
),
54-
(
55-
"get",
56-
"vendor",
57-
"get's vendored dry run previews vendor's gem takeover refusals",
58-
),
5954
// The agent download engine's nested apply still builds ApplyArgs.
6055
(
6156
"agent_download",

0 commit comments

Comments
 (0)