Fix Go check, drop and vendor findings from code review

- apply --check: skip the native vendor check for a module whose
  modules.txt drift the audit already reports, so one cause gives one
  drift (matches go_sync_warnings).
- drop_replace_entry: plan the go.sum restore, remove the directive, and
  only then write go.sum, so a failed go.mod/go.work edit leaves both
  unchanged.
- Native vendor: treat a file with two //go:build lines or an
  unparseable expression as omitted, as imports.ShouldBuild does.
- reconcile: report a failed drop of a version-less directive under a
  PURL built from its copy path, not the bare module path.
- Only probe selection when replace_refusal did not already refuse.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: mikolalysenko

crates/socket-patch-cli/src/commands/apply.rs

@@ -407,38 +407,52 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 {
                     drifts.push((id, "go_redirect_drift".to_string(), d.to_string()));
                 }
             }
-            // The native vendor tree a default build compiles must carry
-            // every in-scope Go patch, vendor-owned ones included. Its
-            // modules.txt drift is the audit's below: one report per cause.
-            let no_texts = HashMap::new();
-            for (purl, record) in &manifest.patches {
-                if in_scope.contains(purl) {
-                    if let Err(detail) =
-                        socket_patch_core::vendor::go_consumer_sync::verify_native_vendor_tree(
-                            &args.common.cwd,
-                            purl,
-                            record,
-                            &no_texts,
-                        )
-                        .await
-                    {
-                        drifts.push((
-                            purl.clone(),
-                            socket_patch_core::vendor::go_consumer_sync::NATIVE_VENDOR_CODE.into(),
-                            detail,
-                        ));
-                    }
-                }
-            }
             // The redirects can be intact while the project no longer
             // builds: a committed vendor/modules.txt or the go.mod
             // requirements out of step with them (#343, #618).
-            for issue in socket_patch_core::vendor::go_consumer_sync::audit(
+            let issues = socket_patch_core::vendor::go_consumer_sync::audit(
                 &args.common.cwd,
                 &HashMap::new(),
             )
-            .await
-            {
+            .await;
+            // The native vendor tree a default build compiles must carry
+            // every in-scope Go patch, vendor-owned ones included. A module
+            // whose modules.txt drift the audit reports is skipped: one
+            // report per cause.
+            let stale: HashSet<&str> = issues
+                .iter()
+                .filter(|issue| {
+                    matches!(
+                        issue,
+                        socket_patch_core::vendor::go_consumer_sync::GoSyncIssue::VendorModulesTxt { .. }
+                    )
+                })
+                .map(|issue| issue.module())
+                .collect();
+            let no_texts = HashMap::new();
+            for (purl, record) in &manifest.patches {
+                if !in_scope.contains(purl)
+                    || parse_golang_purl(purl).is_some_and(|(m, _)| stale.contains(m.as_ref()))
+                {
+                    continue;
+                }
+                if let Err(detail) =
+                    socket_patch_core::vendor::go_consumer_sync::verify_native_vendor_tree(
+                        &args.common.cwd,
+                        purl,
+                        record,
+                        &no_texts,
+                    )
+                    .await
+                {
+                    drifts.push((
+                        purl.clone(),
+                        socket_patch_core::vendor::go_consumer_sync::NATIVE_VENDOR_CODE.into(),
+                        detail,
+                    ));
+                }
+            }
+            for issue in &issues {
                 drifts.push((
                     issue.module().to_string(),
                     issue.code().to_string(),

crates/socket-patch-core/src/patch/redirect/golang_local.rs

@@ -199,14 +199,16 @@ pub async fn reuse_go_redirect(
     }) {
         return None;
     }
-    let refusal = crate::vendor::go_consumer_sync::replace_refusal(
+    let mut refusal = crate::vendor::go_consumer_sync::replace_refusal(
         project_root,
         &module,
         &version,
         GO_PATCHES_DIR,
     )
-    .await
-    .or(selection_refusal(project_root, &module, &version).await);
+    .await;
+    if refusal.is_none() {
+        refusal = selection_refusal(project_root, &module, &version).await;
+    }
     if let Some(refusal) = refusal {
         return Some(synthesized_result(
             purl,
@@ -531,7 +533,14 @@ pub async fn reconcile_go_redirects(
             )
             .await
             {
-                let purl = purl.unwrap_or_else(|| entry.module.clone());
+                // A version-less directive names its version in the copy
+                // path; a failure still needs a PURL to report.
+                let purl = purl
+                    .or_else(|| {
+                        let (_, v) = entry.path.as_deref()?.rsplit_once('@')?;
+                        Some(build_golang_purl(&entry.module, v))
+                    })
+                    .unwrap_or_else(|| entry.module.clone());
                 out.failed.push((
                     purl.clone(),
                     format!("kept the stale Go patch redirect for {purl}: {e}"),

crates/socket-patch-core/src/vendor/go_mod_edit.rs

@@ -282,12 +282,20 @@ pub async fn drop_replace_entry(
         .into_iter()
         .filter(|entry| entry.module == module && entry.owner == Some(owner))
         .collect();
-    if !entries.is_empty() {
-        // A dry run makes the same checks (a conflicting or invalid saved
-        // checksum refuses) without writing go.sum, so it predicts the run.
-        super::go_sum_backup::restore(project_root, &entries, dry_run).await?;
+    // A dry run makes the same checks (a conflicting or invalid saved
+    // checksum refuses) without writing go.sum, so it predicts the run.
+    let sums = if entries.is_empty() {
+        None
+    } else {
+        super::go_sum_backup::restore(project_root, &entries).await?
+    };
+    // go.sum changes only once the directive is gone, so a failed edit
+    // leaves both as they were.
+    let changed = remove_from_replace_files(project_root, module, owner, dry_run).await?;
+    if let (Some(sums), false) = (sums, dry_run) {
+        super::go_sum_backup::write(project_root, &sums).await?;
     }
-    remove_from_replace_files(project_root, module, owner, dry_run).await
+    Ok(changed)
 }
 
 /// Remove `module`'s `owner` replace from the root `go.work` (when present)

crates/socket-patch-core/src/vendor/go_native_vendor.rs

@@ -698,8 +698,10 @@ fn omitted_by_build_constraint(text: &str) -> bool {
         }
         if !block {
             if let Some(expr) = go_build_expr(line) {
+                // imports.ShouldBuild rejects a second `//go:build` line,
+                // so `go mod vendor` leaves the file out.
                 if modern.replace(expr).is_some() {
-                    return false;
+                    return true;
                 }
             } else if legacy_allowed {
                 if let Some(expr) = plus_build_expr(line) {
@@ -730,7 +732,8 @@ fn omitted_by_build_constraint(text: &str) -> bool {
         }
     }
     if let Some(expr) = modern {
-        return constraint(expr) == Some(false);
+        // An expression constraint.Parse rejects also leaves the file out.
+        return constraint(expr) != Some(true);
     }
     legacy[..legacy_end]
         .iter()
@@ -912,6 +915,8 @@ mod tests {
             "// +build !",
             "// +build integration-test",
             "// +build linux,!!amd64 darwin-x",
+            "//go:build linux &&",
+            "//go:build linux\n//go:build darwin",
         ] {
             assert!(
                 omitted_by_build_constraint(&format!("{header}\n\npackage main\n")),

crates/socket-patch-core/src/vendor/go_sum_backup.rs

@@ -111,12 +111,12 @@ fn backup(root: &Path, entry: &ReplaceEntry) -> Option<PathBuf> {
 /// A version the build no longer selects (go.mod now requires another one,
 /// or a pruned graph no longer has the module) needs no checksums: restoring
 /// them would leave go.sum untidy and make discovery see the old version
-/// again. `dry_run` makes every check without writing go.sum.
+/// again. Returns the restored go.sum without writing it (`None`: no
+/// change), so the caller writes it only once the directive is gone.
 pub(super) async fn restore(
     root: &Path,
     entries: &[ReplaceEntry],
-    dry_run: bool,
-) -> Result<(), String> {
+) -> Result<Option<String>, String> {
     use super::go_build_list::{project_selection, Selection};
     let path = root.join("go.sum");
     let mut sums = read(&path).await?;
@@ -157,12 +157,14 @@ pub(super) async fn restore(
             sums = new;
         }
     }
-    if !dry_run && sums != original {
-        atomic_write_bytes_preserving_mode(&path, sums.as_bytes())
-            .await
-            .map_err(|e| e.to_string())?;
-    }
-    Ok(())
+    Ok((sums != original).then_some(sums))
+}
+
+/// Write the go.sum [`restore`] planned.
+pub(super) async fn write(root: &Path, sums: &str) -> Result<(), String> {
+    atomic_write_bytes_preserving_mode(&root.join("go.sum"), sums.as_bytes())
+        .await
+        .map_err(|e| e.to_string())
 }
 
 pub(crate) async fn forget(root: &Path, entries: &[ReplaceEntry]) {
@@ -319,6 +321,35 @@ mod tests {
         assert_eq!(read(&root.join("go.sum")).await.unwrap(), "");
     }
 
+    /// go.sum changes only after the directive is removed: a refused go.mod
+    /// edit leaves both as they were.
+    #[cfg(unix)]
+    #[tokio::test]
+    async fn failed_directive_edit_leaves_go_sum_unchanged() {
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path();
+        tokio::fs::write(
+            root.join("go.mod"),
+            "module consumer\ngo 1.21\nrequire example.com/upstream v1.0.0\n",
+        )
+        .await
+        .unwrap();
+        tokio::fs::write(root.join("go.sum"), SUMS).await.unwrap();
+        ensure_replace_entry(root, MODULE, VERSION, GO_PATCHES_DIR, false)
+            .await
+            .unwrap();
+        tokio::fs::write(root.join("go.sum"), "").await.unwrap();
+        let real = tmp.path().join("real.mod");
+        tokio::fs::rename(root.join("go.mod"), &real).await.unwrap();
+        std::os::unix::fs::symlink(&real, root.join("go.mod")).unwrap();
+        assert!(
+            drop_replace_entry(root, MODULE, ReplaceOwner::GoPatches, false)
+                .await
+                .is_err()
+        );
+        assert_eq!(read(&root.join("go.sum")).await.unwrap(), "");
+    }
+
     #[tokio::test]
     async fn dry_run_drop_restores_nothing() {
         let tmp = tempfile::tempdir().unwrap();