Fix Maven 4 implicit subprojects missed outside vendored (#1809) (#1886)

* Start fix for #1809

Assisted-by: Claude Code:claude-opus-5-5

* Treat Maven 4 implicit subprojects as a reactor

A Maven 4 root (model 4.1.0, pom packaging, no <subprojects> list)
builds every direct subdirectory holding a pom.xml. Only the vendored
planner knew this. Hosted scan pinned just the root pom while a
module kept its own version, VEX attested that pin as not_affected,
the sbt ambiguity guard let such a root through, and the project
scope of the Maven cache skipped the modules' dependencies.

Every caller now asks one maven_reactor predicate that follows the
planner's own discovery rule, including the discoverSubprojects
opt-out in .mvn/maven.config. The hosted engine reads each
subdirectory's pom.xml beside such a root so the rewriter can refuse
it as a reactor.

Fixes #1809

Assisted-by: Claude Code:claude-opus-5-5

* Share one subproject probe; skip refused pins

Review follow-ups for the Maven 4 reactor fix. The hosted engine,
VEX and the Maven cache scope now find discovered subprojects
through the planner's own probe, so a .git directory or a directory
named pom.xml never makes a root a reactor in one place but not
another.

A refused reactor root pins nothing, so the hosted confirmation
probe no longer counts a suffixed version an older release left in
it, or one a module pom names, as a redirected dependency. The CLI
contract now describes Maven 4 discovered reactors.

Refs #1809

Assisted-by: Claude Code:claude-opus-5-5

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: mikolalysenko

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -9,7 +9,8 @@
 //! (Maven caches the pom of every artifact, parent and BOM it resolves):
 //!
 //! - **Seeds**: every `<dependency>` of the reactor (the root `pom.xml`,
-//!   its `<modules>` / `<subprojects>` recursively, every profile, and the
+//!   its `<modules>` / `<subprojects>` recursively (and the subprojects
+//!   Maven 4 discovers under a root listing none), every profile, and the
 //!   `<dependencies>` each inherits from its parents), any scope.
 //! - **Edges**: an artifact's own non-optional `compile` / `runtime`
 //!   dependencies (and those its parents declare), which is what Maven
@@ -30,7 +31,9 @@ use std::collections::{HashMap, HashSet, VecDeque};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
-use crate::vendor::jvm::maven_reactor::{pom_model, PomDecl, PomModel};
+use crate::vendor::jvm::maven_reactor::{
+    implicit_subprojects, may_discover_subprojects, pom_model, PomDecl, PomModel, MAVEN_CONFIG,
+};
 
 /// `(groupId, artifactId, version)`.
 type Gav = (String, String, String);
@@ -225,12 +228,17 @@ impl Walk<'_> {
             local: true,
         }];
         let mut seen: HashSet<PathBuf> = HashSet::from([root]);
+        let config = crate::utils::fs::read_regular_to_bytes_sync(&self.cwd.join(MAVEN_CONFIG))
+            .ok()
+            .map(|b| String::from_utf8_lossy(&b).into_owned());
         while let Some(node) = pending.pop() {
             if seen.len() > MAX_REACTOR {
                 return None;
             }
             let dir = node.path.parent().unwrap_or(self.cwd).to_path_buf();
-            for module in node.model.modules.clone() {
+            let mut modules = node.model.modules.clone();
+            modules.extend(discovered_subprojects(&node.path, &dir, config.as_deref()));
+            for module in modules {
                 if module.is_empty() || module.contains("${") {
                     continue;
                 }
@@ -461,6 +469,40 @@ impl Walk<'_> {
     }
 }
 
+/// The subprojects Maven 4 discovers under the pom at `path` (#1809): when
+/// it lists none ([`may_discover_subprojects`]), the direct subdirectories
+/// of `dir` holding a `pom.xml` ([`implicit_subprojects`]), as `<module>`
+/// names.
+fn discovered_subprojects(path: &Path, dir: &Path, config: Option<&str>) -> Vec<String> {
+    let Ok(bytes) = crate::utils::fs::read_regular_to_bytes_sync(path) else {
+        return Vec::new();
+    };
+    if !may_discover_subprojects(&String::from_utf8_lossy(&bytes), config) {
+        return Vec::new();
+    }
+    // The planners' listing: bare names, directories (followed through a
+    // link) ending in `/`.
+    let list = |rel: &str| -> Vec<String> {
+        let Ok(entries) = std::fs::read_dir(dir.join(rel)) else {
+            return Vec::new();
+        };
+        entries
+            .flatten()
+            .filter_map(|e| {
+                let name = e.file_name().to_str()?.to_string();
+                Some(if e.path().is_dir() {
+                    format!("{name}/")
+                } else {
+                    name
+                })
+            })
+            .collect()
+    };
+    let mut out = implicit_subprojects(&list, "");
+    out.sort();
+    out
+}
+
 /// The pom at `path` parsed, when it is readable.
 fn read_model(path: &Path, include_profiles: bool) -> Option<Arc<PomModel>> {
     crate::utils::fs::read_regular_to_bytes_sync(path)
@@ -754,6 +796,43 @@ mod tests {
         assert!(!scope.admits("new.group", "lib", "3"));
     }
 
+    /// #1809: Maven 4 builds every direct subdirectory holding a `pom.xml`
+    /// of a 4.1.0 `pom`-packaged root that lists no subprojects, so their
+    /// dependencies seed the scope; with discovery turned off in
+    /// `.mvn/maven.config` they do not.
+    #[test]
+    fn maven_4_discovered_subprojects_seed_the_scope() {
+        let dir = tempfile::tempdir().unwrap();
+        let (cwd, repo) = (dir.path().join("p"), dir.path().join("m2"));
+        write(
+            &cwd,
+            "pom.xml",
+            "<project xmlns=\"http://maven.apache.org/POM/4.1.0\"><modelVersion>4.1.0</modelVersion>\
+             <groupId>com.example</groupId><artifactId>root</artifactId><version>1</version>\
+             <packaging>pom</packaging></project>",
+        );
+        write(
+            &cwd,
+            "a/pom.xml",
+            &format!(
+                "<project xmlns=\"http://maven.apache.org/POM/4.1.0\"><modelVersion>4.1.0</modelVersion>\
+                 <parent/><artifactId>a</artifactId>{}</project>",
+                deps(&[dep("org.example", "lib", Some("1.0"), "")])
+            ),
+        );
+        cache(&repo, "org.example", "lib", "1.0", "");
+        let scope = project_scope(&cwd, &repo).unwrap();
+        assert!(scope.admits("org.example", "lib", "1.0"));
+
+        write(
+            &cwd,
+            ".mvn/maven.config",
+            "-Dmaven.project.discoverSubprojects=false\n",
+        );
+        let scope = project_scope(&cwd, &repo).unwrap();
+        assert!(!scope.admits("org.example", "lib", "1.0"));
+    }
+
     #[test]
     fn modules_parents_boms_and_management_are_followed() {
         let dir = tempfile::tempdir().unwrap();

crates/socket-patch-core/src/crawlers/maven_scope.rs

@@ -762,6 +762,27 @@ pub async fn read_candidate_files(
         read_go_workspace(view, unreadable, &mut out).await;
     }
 
+    // A Maven 4 root that discovers its subprojects (#1809): each direct
+    // subdirectory holding a `pom.xml` is a module, which the pom rewriter
+    // must see to refuse the root as a reactor (it pins only the root pom).
+    // Advisory: keyed `<dir>/pom.xml`, never rewritten, and one that cannot
+    // be read still counts as present.
+    if candidates.iter().any(|c| c.dep.ecosystem == "maven") {
+        if let Some(pom) = out.files.get("pom.xml") {
+            let config = out.files.get(crate::patch::redirect::MVN_CONFIG);
+            if crate::vendor::jvm::maven_reactor::may_discover_subprojects(
+                pom,
+                config.map(String::as_str),
+            ) {
+                for dir in crate::vendor::jvm::maven_reactor::implicit_subprojects_in(view).await {
+                    let rel = format!("{dir}/pom.xml");
+                    let text = read_advisory(view, unreadable, &rel).await;
+                    out.files.insert(rel, text.unwrap_or_default());
+                }
+            }
+        }
+    }
+
     // NuGet merges the user config and every parent directory's config
     // under the project's own: a catch-all mapping the rewriter creates
     // must name their sources too (#354). Disk only (the in-memory engine
@@ -3283,10 +3304,23 @@ fn confirm(
     fn is_npm_manifest(name: &str) -> bool {
         name == "package.json" || name.ends_with("/package.json")
     }
+    // A Maven reactor root the pom rewriter refused pins nothing, so a
+    // suffixed version an earlier release left in it, or in `.mvn/`,
+    // proves nothing (#1809); nor do the module poms read beside a Maven 4
+    // root, advisory input only.
+    let maven_reactor_refused = rewrite
+        .warnings
+        .iter()
+        .any(|w| w.code == "redirect_maven_multimodule_unsupported");
+    let is_unpinned_maven = |name: &str| {
+        name.ends_with("/pom.xml")
+            || maven_reactor_refused && (name == "pom.xml" || name.starts_with(".mvn/"))
+    };
     let final_texts: Vec<(&str, &String)> = files
         .iter()
         .filter(|(name, _)| !(pdm_inactive && name.as_str() == "pdm.lock"))
         .filter(|(name, _)| !is_npm_manifest(name))
+        .filter(|(name, _)| !is_unpinned_maven(name))
         .filter(|(name, _)| !crate::patch::redirect::sbt::is_synthetic_key(name))
         .map(|(name, content)| (name.as_str(), rewrite.files.get(name).unwrap_or(content)))
         .chain(
@@ -5373,6 +5407,69 @@ snapshots:
         assert!(done.confirmed.is_empty(), "{:?}", done.confirmed);
     }
 
+    /// #1809: a Maven 4 root whose subprojects Maven discovers (model
+    /// 4.1.0, `pom` packaging, no list) is a reactor once a direct
+    /// subdirectory holds a `pom.xml`. The engine reads that pom beside
+    /// the root, on disk and in memory, so the pom rewriter refuses the
+    /// root instead of pinning only it while the module keeps its version.
+    #[tokio::test]
+    async fn a_maven_4_implicit_reactor_root_is_refused() {
+        let root = "<project xmlns=\"http://maven.apache.org/POM/4.1.0\">\n  <modelVersion>4.1.0</modelVersion>\n  <groupId>x</groupId>\n  <artifactId>y</artifactId>\n  <version>1</version>\n  <packaging>pom</packaging>\n  <dependencies>\n    <dependency>\n      <groupId>com.socketfixture</groupId>\n      <artifactId>victim</artifactId>\n      <version>1.10.0</version>\n    </dependency>\n  </dependencies>\n</project>\n";
+        let child = "<project xmlns=\"http://maven.apache.org/POM/4.1.0\">\n  <modelVersion>4.1.0</modelVersion>\n  <parent/>\n  <artifactId>a</artifactId>\n</project>\n";
+        let refused = |read: &CandidateFiles, done: &Rewritten| {
+            assert!(
+                read.files.contains_key("a/pom.xml"),
+                "{:?}",
+                read.files.keys()
+            );
+            assert!(!done.rewrite.files.contains_key("pom.xml"));
+            assert!(done.confirmed.is_empty(), "{:?}", done.confirmed);
+            assert!(done
+                .rewrite
+                .warnings
+                .iter()
+                .any(|w| w.code == "redirect_maven_multimodule_unsupported"));
+        };
+
+        let mut p = MemoryProject::new();
+        p.insert_text("pom.xml", root);
+        p.insert_text("a/pom.xml", child);
+        let (read, done) = gradle_rewrite(&p).await;
+        refused(&read, &done);
+
+        let tmp = tempfile::tempdir().unwrap();
+        std::fs::write(tmp.path().join("pom.xml"), root).unwrap();
+        std::fs::create_dir(tmp.path().join("a")).unwrap();
+        std::fs::write(tmp.path().join("a/pom.xml"), child).unwrap();
+        // A directory without a pom is no subproject.
+        std::fs::create_dir(tmp.path().join("src")).unwrap();
+        let (read, done) = gradle_rewrite_in(&ProjectView::Disk(tmp.path())).await;
+        refused(&read, &done);
+        assert!(!read.files.contains_key("src/pom.xml"));
+
+        // A suffixed version an earlier release pinned into the root, or one
+        // a module names, confirms nothing: the refused root pins nothing.
+        let stale = root.replace("1.10.0", "1.10.0-socket.4d5e6f70");
+        let named = child.replace(
+            "</artifactId>",
+            "</artifactId>\n  <properties><v>1.10.0-socket.4d5e6f70</v></properties>",
+        );
+        for (r, c) in [(stale.as_str(), child), (root, named.as_str())] {
+            let mut p = MemoryProject::new();
+            p.insert_text("pom.xml", r);
+            p.insert_text("a/pom.xml", c);
+            let (read, done) = gradle_rewrite(&p).await;
+            refused(&read, &done);
+        }
+
+        // Without a subproject the root is a single pom, pinned as before.
+        std::fs::remove_dir_all(tmp.path().join("a")).unwrap();
+        let (read, done) = gradle_rewrite_in(&ProjectView::Disk(tmp.path())).await;
+        assert!(!read.files.keys().any(|k| k.ends_with("/pom.xml")));
+        assert!(done.rewrite.files["pom.xml"].contains("1.10.0-socket.4d5e6f70"));
+        assert_eq!(done.confirmed.len(), 1);
+    }
+
     /// An sbt build beside a Gradle build: the Gradle planner pinning a
     /// uuid the sbt rewriter refused (here: no resolution evidence) does
     /// not confirm it — the sbt build still loads the upstream artifact.

crates/socket-patch-core/src/hosted/engine.rs

@@ -602,7 +602,7 @@ pub fn wiring_files(files: &BTreeMap<String, String>) -> Vec<String> {
 }
 
 /// A [`crate::gradle::ListFn`] over the keys of `files`.
-fn key_list(files: &BTreeMap<String, String>, dir: &str) -> Vec<String> {
+pub(crate) fn key_list(files: &BTreeMap<String, String>, dir: &str) -> Vec<String> {
     let prefix = if dir.is_empty() {
         String::new()
     } else {

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

@@ -8656,10 +8656,13 @@ fn rewrite_maven_pom(
     // would keep the unpatched upstream jar while the dep counts as
     // redirected and VEX attests it (#261). Refuse the whole root, untouched;
     // vendored mode's reactor planner rewrites each module's declaration.
-    if pom
-        .as_deref()
-        .is_some_and(crate::vendor::jvm::maven_reactor::declares_modules)
-    {
+    // Maven 4 subprojects found by discovery count too (#1809): the engine
+    // reads each direct subdirectory's `pom.xml` beside such a root.
+    if pom.is_some() && {
+        let read = |rel: &str| files.get(rel).map(|t| t.as_bytes().to_vec());
+        let list = |dir: &str| gradle::key_list(files, dir);
+        crate::vendor::jvm::maven_reactor::is_reactor_root(&read, &list)
+    } {
         let gas: Vec<String> = maven
             .iter()
             .map(|dep| match &dep.namespace {
@@ -8670,7 +8673,7 @@ fn rewrite_maven_pom(
         result.warnings.push(RewriteWarning {
             code: "redirect_maven_multimodule_unsupported".into(),
             detail: format!(
-                "pom.xml declares <modules>/<subprojects>; hosted mode reads only the root pom, so a module's own <version> of {} would stay unpatched. pom.xml and .mvn/ were left as they are and the dep is not counted as redirected; use `scan --mode vendored`, which pins each module's declaration",
+                "pom.xml heads a multi-module reactor (<modules>/<subprojects>, or Maven 4 subproject discovery); hosted mode reads only the root pom, so a module's own <version> of {} would stay unpatched. pom.xml and .mvn/ were left as they are and the dep is not counted as redirected; use `scan --mode vendored`, which pins each module's declaration",
                 gas.join(", ")
             ),
         });
@@ -10478,6 +10481,64 @@ mod tests {
         assert!(!warning_codes(&r).contains(&"redirect_maven_multimodule_unsupported"));
     }
 
+    /// #1809: a Maven 4 (model 4.1.0) `pom`-packaged root that lists no
+    /// subprojects builds every direct subdirectory holding a `pom.xml`, so
+    /// it is a reactor like a declared one and refused untouched. Without
+    /// such a subdirectory, under a 4.0.0 model, or with discovery turned
+    /// off in `.mvn/maven.config`, it is a single pom and still rewritten.
+    #[test]
+    fn maven_4_implicit_subprojects_root_is_refused() {
+        let dep = "<dependencies>\n    <dependency>\n      <groupId>org.slf4j</groupId>\n      <artifactId>slf4j-api</artifactId>\n      <version>1.7.36</version>\n    </dependency>\n  </dependencies>\n";
+        let root = |model: &str| {
+            format!(
+                "<project xmlns=\"http://maven.apache.org/POM/{model}\">\n  <modelVersion>{model}</modelVersion>\n  <groupId>com.example</groupId>\n  <artifactId>root</artifactId>\n  <version>1.0.0</version>\n  <packaging>pom</packaging>\n  {dep}</project>\n"
+            )
+        };
+        let child = "<project xmlns=\"http://maven.apache.org/POM/4.1.0\">\n  <modelVersion>4.1.0</modelVersion>\n  <parent/>\n  <artifactId>child</artifactId>\n  <dependencies>\n    <dependency>\n      <groupId>org.slf4j</groupId>\n      <artifactId>slf4j-api</artifactId>\n      <version>1.7.36</version>\n    </dependency>\n  </dependencies>\n</project>\n";
+        let tree = |model: &str, extra: &[(&str, &str)]| {
+            let mut files = BTreeMap::from([("pom.xml".to_string(), root(model))]);
+            files.extend(extra.iter().map(|(k, v)| (k.to_string(), v.to_string())));
+            rewrite_registry_redirect(&files, &[maven_override()])
+        };
+
+        let r = tree("4.1.0", &[("child/pom.xml", child)]);
+        assert!(
+            r.files.is_empty() && r.edits.is_empty(),
+            "an implicit reactor root must not be edited: files={:?} edits={:?}",
+            r.files.keys(),
+            r.edits
+        );
+        assert_eq!(
+            warning_codes(&r),
+            vec!["redirect_maven_multimodule_unsupported"]
+        );
+        assert!(r.warnings[0]
+            .detail
+            .contains("Maven 4 subproject discovery"));
+
+        let off = "-Dmaven.project.discoverSubprojects=false\n";
+        for (case, r) in [
+            ("no subproject", tree("4.1.0", &[])),
+            ("model 4.0.0", tree("4.0.0", &[("child/pom.xml", child)])),
+            (
+                "discovery off",
+                tree(
+                    "4.1.0",
+                    &[("child/pom.xml", child), (".mvn/maven.config", off)],
+                ),
+            ),
+        ] {
+            assert!(
+                r.files["pom.xml"].contains(MAVEN_SUFFIXED),
+                "{case}: a single pom is still rewritten"
+            );
+            assert!(
+                !warning_codes(&r).contains(&"redirect_maven_multimodule_unsupported"),
+                "{case}"
+            );
+        }
+    }
+
     /// A direct declaration whose groupId / artifactId is a property of
     /// the pom's own `<properties>` is the GA (#683): its literal is
     /// rewritten, not shadowed by an added `<dependencyManagement>` pin

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

@@ -3418,7 +3418,7 @@ mod tests {
     fn detect_routes_gradle_only_roots_here() {
         let files = fs(&[("build.gradle", "")]);
         let read = |rel: &str| files.get(rel).cloned();
-        assert_eq!(detect(&read), Shape::Gradle);
+        assert_eq!(detect(&read, &super::super::no_list), Shape::Gradle);
     }
 
     #[test]