From 20d0d7385b8737762db92a075bc82ff1ca9f0681 Mon Sep 17 00:00:00 2001 From: Jeppe Fredsgaard Blaabjerg Date: Fri, 28 Aug 2026 12:04:44 +0200 Subject: [PATCH 1/3] fix(manifest): resolve Maven dependencies through Maven's own resolver The Maven facts extension collected the dependency graph with maven-dependency-tree, then re-resolved each artifact itself, passing the root module's repository list for every node in the tree. Maven resolves each node against the repositories that node's own descriptor lineage contributes, so a dependency served only by a repository declared in one module of a reactor could not be materialized for any other module that reached it. Aether's local repository also records which repository each cached file came from, so not even an already-downloaded copy counted as available, and --with-files aborted the scan on a dependency the build itself resolves without trouble. Resolution now goes through ProjectDependenciesResolver, the component Maven's own lifecycle uses to build a project's classpath. Per-node repositories, dependency management, scope derivation and reactor substitution are Maven's rather than a re-implementation of them, and failures are reported from Maven's own per-dependency errors. Which artifacts get fetched is expressed as a DependencyFilter, so a plain --facts run collects without downloading anything and a reactor sibling's jar is never requested at the validate phase the CLI runs, where nothing has been packaged. A filtered-out node yields no ArtifactResult, so it can never be mistaken for a resolution failure. Coordinate ids keep Maven's `type` rather than Aether's file extension, and versions use the base version so a resolved remote snapshot cannot leak a timestamped coordinate no manifest names. Conflict-losing nodes, which a verbose collect leaves in the graph, are skipped. Records are byte-identical to the previous output on the projects exercised here. Drops the bundled maven-dependency-tree; the extension jar goes 67K -> 26K. Adds two compat fixtures. repo-inheritance covers a dependency reachable only through a repository a sibling module declares, plus the fail-closed half: an unresolvable dependency must still be reported, since a silently missing jar leaves reachability blind to what it contains. duplicate-failure covers several modules failing on the same dependency, whose identical failures collapse in the value-equality accumulator shared across the reactor. The Maven matrix now spans 3.2.5 through 4.0.0-rc-6, the range the extension claims to support. --- .../manifest/scripts/maven-extension/pom.xml | 21 +- .../ext/CoanaFactsLifecycleParticipant.java | 15 +- .../socket/SocketFactsRecordsEngine.java | 228 +++++++++++------- src/commands/manifest/scripts/test/README.md | 13 +- .../scripts/test/maven-compat/.gitignore | 8 + .../test/maven-compat/assert-fail-closed.py | 31 +++ .../assert-no-invented-failures.py | 26 ++ .../maven-compat/duplicate-failure/m1/pom.xml | 10 + .../maven-compat/duplicate-failure/m2/pom.xml | 10 + .../maven-compat/duplicate-failure/pom.xml | 26 ++ .../repo-inheritance/liba/pom.xml | 32 +++ .../repo-inheritance/libb/pom.xml | 18 ++ .../maven-compat/repo-inheritance/pom.xml | 21 ++ .../smoke-test-duplicate-failure.sh | 37 +++ .../smoke-test-repo-inheritance.sh | 118 +++++++++ .../manifest/scripts/test/run-compat.sh | 10 +- 16 files changed, 508 insertions(+), 116 deletions(-) create mode 100644 src/commands/manifest/scripts/test/maven-compat/assert-fail-closed.py create mode 100644 src/commands/manifest/scripts/test/maven-compat/assert-no-invented-failures.py create mode 100644 src/commands/manifest/scripts/test/maven-compat/duplicate-failure/m1/pom.xml create mode 100644 src/commands/manifest/scripts/test/maven-compat/duplicate-failure/m2/pom.xml create mode 100644 src/commands/manifest/scripts/test/maven-compat/duplicate-failure/pom.xml create mode 100644 src/commands/manifest/scripts/test/maven-compat/repo-inheritance/liba/pom.xml create mode 100644 src/commands/manifest/scripts/test/maven-compat/repo-inheritance/libb/pom.xml create mode 100644 src/commands/manifest/scripts/test/maven-compat/repo-inheritance/pom.xml create mode 100755 src/commands/manifest/scripts/test/maven-compat/smoke-test-duplicate-failure.sh create mode 100755 src/commands/manifest/scripts/test/maven-compat/smoke-test-repo-inheritance.sh diff --git a/src/commands/manifest/scripts/maven-extension/pom.xml b/src/commands/manifest/scripts/maven-extension/pom.xml index 42b4e2c7fe..8a867a7d9e 100644 --- a/src/commands/manifest/scripts/maven-extension/pom.xml +++ b/src/commands/manifest/scripts/maven-extension/pom.xml @@ -43,8 +43,9 @@ - + org.apache.maven.plugins maven-shade-plugin @@ -97,32 +98,18 @@ ${maven.version} provided + org.apache.maven.resolver maven-resolver-api 1.9.18 provided - - - org.apache.maven.resolver - maven-resolver-util - 1.9.18 - provided - org.slf4j slf4j-api 1.7.36 provided - - - - org.apache.maven.shared - maven-dependency-tree - 3.3.0 - diff --git a/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/ext/CoanaFactsLifecycleParticipant.java b/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/ext/CoanaFactsLifecycleParticipant.java index 4f76a13321..d2fbf92172 100644 --- a/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/ext/CoanaFactsLifecycleParticipant.java +++ b/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/ext/CoanaFactsLifecycleParticipant.java @@ -3,9 +3,8 @@ import org.apache.maven.AbstractMavenLifecycleParticipant; import org.apache.maven.MavenExecutionException; import org.apache.maven.execution.MavenSession; +import org.apache.maven.project.ProjectDependenciesResolver; import org.apache.maven.rtinfo.RuntimeInformation; -import org.apache.maven.shared.dependency.graph.DependencyGraphBuilder; -import org.eclipse.aether.RepositorySystem; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import tech.coana.socket.SocketFactsRecordsEngine; @@ -28,17 +27,13 @@ public class CoanaFactsLifecycleParticipant extends AbstractMavenLifecyclePartic private static final Logger LOG = LoggerFactory.getLogger("coana"); - private final RepositorySystem repoSystem; - private final DependencyGraphBuilder dependencyGraphBuilder; + private final ProjectDependenciesResolver dependenciesResolver; private final RuntimeInformation runtimeInformation; @Inject public CoanaFactsLifecycleParticipant( - RepositorySystem repoSystem, - DependencyGraphBuilder dependencyGraphBuilder, - RuntimeInformation runtimeInformation) { - this.repoSystem = repoSystem; - this.dependencyGraphBuilder = dependencyGraphBuilder; + ProjectDependenciesResolver dependenciesResolver, RuntimeInformation runtimeInformation) { + this.dependenciesResolver = dependenciesResolver; this.runtimeInformation = runtimeInformation; } @@ -60,7 +55,7 @@ public void afterSessionEnd(MavenSession session) throws MavenExecutionException opts.excludePaths = opt(session, "socket.excludePaths"); File rootDir = new File(session.getExecutionRootDirectory()); try { - new SocketFactsRecordsEngine(repoSystem, dependencyGraphBuilder, runtimeInformation.getMavenVersion(), LOG) + new SocketFactsRecordsEngine(dependenciesResolver, runtimeInformation.getMavenVersion(), LOG) .run(session, session.getProjects(), rootDir, opts); } catch (IOException exception) { throw new MavenExecutionException("Cannot write socket facts records", exception); diff --git a/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java b/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java index 2c34673588..6272d6fecd 100644 --- a/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java +++ b/src/commands/manifest/scripts/maven-extension/src/main/java/tech/coana/socket/SocketFactsRecordsEngine.java @@ -1,22 +1,17 @@ package tech.coana.socket; -import org.apache.maven.artifact.Artifact; -import org.apache.maven.artifact.handler.ArtifactHandler; import org.apache.maven.execution.MavenSession; import org.apache.maven.model.Resource; -import org.apache.maven.project.DefaultProjectBuildingRequest; +import org.apache.maven.project.DefaultDependencyResolutionRequest; +import org.apache.maven.project.DependencyResolutionException; +import org.apache.maven.project.DependencyResolutionRequest; +import org.apache.maven.project.DependencyResolutionResult; import org.apache.maven.project.MavenProject; -import org.apache.maven.project.ProjectBuildingRequest; -import org.apache.maven.shared.dependency.graph.DependencyGraphBuilder; -import org.apache.maven.shared.dependency.graph.DependencyGraphBuilderException; -import org.apache.maven.shared.dependency.graph.DependencyNode; -import org.eclipse.aether.RepositorySystem; -import org.eclipse.aether.RepositorySystemSession; -import org.eclipse.aether.artifact.DefaultArtifact; -import org.eclipse.aether.repository.RemoteRepository; -import org.eclipse.aether.resolution.ArtifactRequest; -import org.eclipse.aether.resolution.ArtifactResolutionException; -import org.eclipse.aether.resolution.ArtifactResult; +import org.apache.maven.project.ProjectDependenciesResolver; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.graph.Dependency; +import org.eclipse.aether.graph.DependencyFilter; +import org.eclipse.aether.graph.DependencyNode; import org.slf4j.Logger; import java.io.File; @@ -40,6 +35,13 @@ * (same contract as the Gradle/SBT scripts; no JSON/hashing here). Per module: a prod root * (compile/runtime/system) and a dev root (test/provided). A reactor module becomes a component only * where another depends on it, by its bare {@code groupId:artifactId:version} id. + * + *

Resolution goes through {@link ProjectDependenciesResolver} — the same component Maven's own + * lifecycle uses to build a project's classpath — so the graph, the scope and management semantics, + * the per-node repository lists and the reactor substitution are Maven's, not an approximation of + * them. Resolving a dependency against only its root module's repositories, as a hand-rolled walk + * does, loses the repositories a dependency's own POM contributes and then cannot see artifacts + * cached from them (Aether's local repository tracks each file's origin repository). */ public final class SocketFactsRecordsEngine { @@ -56,25 +58,25 @@ public static final class Options { private static final List ALL_SCOPES = Arrays.asList("compile", "provided", "runtime", "system", "test"); - private final RepositorySystem repoSystem; - private final DependencyGraphBuilder dependencyGraphBuilder; + // Aether's ArtifactProperties.TYPE: Maven's `type` (jar, test-jar, ...) as opposed to the file + // extension, carried as an artifact property once a Maven dependency becomes an Aether one. + private static final String ARTIFACT_PROPERTY_TYPE = "type"; + // ConflictResolver.NODE_DATA_WINNER, inlined so nothing here needs maven-resolver-util. + private static final String NODE_DATA_CONFLICT_WINNER = "conflict.winner"; + + private final ProjectDependenciesResolver dependenciesResolver; private final String mavenVersion; private final Logger log; public SocketFactsRecordsEngine( - RepositorySystem repoSystem, - DependencyGraphBuilder dependencyGraphBuilder, - String mavenVersion, - Logger log) { - this.repoSystem = repoSystem; - this.dependencyGraphBuilder = dependencyGraphBuilder; + ProjectDependenciesResolver dependenciesResolver, String mavenVersion, Logger log) { + this.dependenciesResolver = dependenciesResolver; this.mavenVersion = mavenVersion; this.log = log; } public void run(MavenSession session, List reactor, File rootDir, Options opts) throws IOException { - RepositorySystemSession repoSession = session.getRepositorySession(); Set passingScopes = computePassingScopes(opts.includeConfigs, opts.excludeConfigs); // GAVs to materialize under --with-files (null = all). Scopes artifact downloads so reachability // doesn't fetch the whole dependency universe. Module src/tgt dirs are emitted regardless (no download). @@ -110,7 +112,7 @@ public void run(MavenSession session, List reactor, File rootDir, if (SocketSupport.isExcludedPath(ws, excludes)) continue; Map nodes = new LinkedHashMap<>(); Set directIds = new HashSet<>(); - collectModule(session, repoSession, module, passingScopes, reactorGavs, populateGavs, opts, nodes, directIds, failures); + collectModule(session, module, passingScopes, reactorGavs, populateGavs, opts, nodes, directIds, failures); rootIdx = emitModuleRoots(lines, rootIdx, ws, nodes, directIds); } @@ -119,11 +121,10 @@ public void run(MavenSession session, List reactor, File rootDir, write(opts.recordsFile, lines); } - // ---- resolution (mirrors the reference engine's visit, minus JSON shaping) ---- + // ---- resolution ---- private void collectModule( MavenSession session, - RepositorySystemSession repoSession, MavenProject module, Set passingScopes, Set reactorGavs, @@ -132,72 +133,159 @@ private void collectModule( Map nodes, Set directIds, Set failures) { - DependencyNode root; + String moduleCoord = module.getGroupId() + ":" + module.getArtifactId() + ":" + module.getVersion(); + DependencyResolutionResult result; + DependencyResolutionException thrown = null; try { - ProjectBuildingRequest req = new DefaultProjectBuildingRequest(session.getProjectBuildingRequest()); - req.setProject(module); - root = dependencyGraphBuilder.buildDependencyGraph(req, null); - } catch (DependencyGraphBuilderException e) { - String coord = module.getGroupId() + ":" + module.getArtifactId() + ":" + module.getVersion(); - failures.add(new Failure(coord, rootMessage(e), "graph")); - log.warn("[socket-facts] could not build dependency graph for " + coord + ": " + rootMessage(e)); - return; + DependencyResolutionRequest request = + new DefaultDependencyResolutionRequest(module, session.getRepositorySession()); + request.setResolutionFilter(materializationFilter(opts, passingScopes, reactorGavs, populateGavs)); + result = dependenciesResolver.resolve(request); + } catch (DependencyResolutionException e) { + // Maven attaches the partial result — graph plus per-dependency errors — to the exception, so + // one unresolvable artifact still yields a complete graph and a precise failure record. + thrown = e; + result = e.getResult(); + if (result == null) { + failures.add(new Failure(moduleCoord, rootMessage(e), "graph")); + log.warn("[socket-facts] could not resolve dependencies for " + moduleCoord + ": " + rootMessage(e)); + return; + } + } + // Tracked per call, not by `failures.size()`: the set is shared across modules and Failure has + // value equality, so a sibling module failing on the same dependency absorbs this module's add. + boolean reported = false; + for (Exception e : result.getCollectionErrors()) { + failures.add(new Failure(moduleCoord, rootMessage(e), "graph")); + log.warn("[socket-facts] could not build dependency graph for " + moduleCoord + ": " + rootMessage(e)); + reported = true; } - List repos = module.getRemoteProjectRepositories(); + for (Dependency dep : result.getUnresolvedDependencies()) { + Artifact artifact = dep.getArtifact(); + if (artifact == null) continue; + List errors = result.getResolutionErrors(dep); + failures.add(new Failure( + gav(artifact), rootMessage(errors.isEmpty() ? null : errors.get(0)), scopeOf(dep))); + log.debug("[socket-facts] could not materialize " + artifact + " (" + scopeOf(dep) + ")"); + reported = true; + } + // Fail closed: a throw whose result named nothing must still surface, or an unresolved dependency + // would silently leave the reachability analysis blind to whatever that artifact contains. + if (thrown != null && !reported) { + failures.add(new Failure(moduleCoord, rootMessage(thrown), "graph")); + log.warn("[socket-facts] could not resolve dependencies for " + moduleCoord + ": " + rootMessage(thrown)); + } + DependencyNode root = result.getDependencyGraph(); + if (root == null) return; Set visited = new HashSet<>(); for (DependencyNode child : root.getChildren()) { - String id = visit(repoSession, child, passingScopes, reactorGavs, populateGavs, opts, repos, nodes, visited, failures); + String id = visit(child, passingScopes, reactorGavs, opts, nodes, visited); if (id != null) directIds.add(id); } } + /** + * Which nodes Maven should MATERIALIZE (fetch the artifact for). Everything else is still collected + * — the graph stays complete — but never resolved, which is what keeps a plain {@code --facts} run + * download-free and keeps us from requesting a reactor sibling's jar: at the {@code validate} phase + * the CLI runs, no sibling has been packaged and none need be installed. + * + *

A node the filter rejects produces no {@code ArtifactResult}, so it lands in neither + * {@code getResolvedDependencies()} nor {@code getUnresolvedDependencies()} and can never be + * mistaken for a resolution failure. + */ + private static DependencyFilter materializationFilter( + final Options opts, + final Set passingScopes, + final Set reactorGavs, + final Set populateGavs) { + if (!opts.withFiles) { + return new DependencyFilter() { + @Override + public boolean accept(DependencyNode node, List parents) { + return false; + } + }; + } + return new DependencyFilter() { + @Override + public boolean accept(DependencyNode node, List parents) { + Dependency dep = node == null ? null : node.getDependency(); + Artifact artifact = dep == null ? null : dep.getArtifact(); + if (artifact == null) return false; + String scope = scopeOf(dep); + // A system-scope artifact carries its systemPath on the model and has no repository to be + // fetched from; visit() reads the file straight off the node instead. + if ("system".equals(scope) || !passingScopes.contains(scope)) return false; + String gav = gav(artifact); + if (reactorGavs.contains(gav)) return false; + return populateGavs == null || populateGavs.contains(gav); + } + }; + } + private String visit( - RepositorySystemSession repoSession, DependencyNode dn, Set passingScopes, Set reactorGavs, - Set populateGavs, Options opts, - List repos, Map nodes, - Set visited, - Set failures) { + Set visited) { + Dependency dep = dn.getDependency(); Artifact artifact = dn.getArtifact(); - String scope = artifact.getScope(); - if (scope == null || scope.isEmpty()) scope = "compile"; + if (dep == null || artifact == null) return null; + // A verbose collect — Maven's -X turns one on — keeps conflict-losing nodes in the graph, tagged + // with the winner they lost to. Only the winner is on the classpath Maven would build. + if (dn.getData().get(NODE_DATA_CONFLICT_WINNER) != null) return null; + String scope = scopeOf(dep); if (!passingScopes.contains(scope)) return null; - String gav = artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getVersion(); + String gav = gav(artifact); boolean internal = reactorGavs.contains(gav); - String type = artifact.getType(); + // Maven's `type` rather than aether's file extension, so a test-jar keeps the coordId the + // assembler and the Gradle/SBT scripts already emit. + String type = artifact.getProperty(ARTIFACT_PROPERTY_TYPE, artifact.getExtension()); String classifier = artifact.getClassifier(); + // Base version: a resolved remote snapshot's `version` is the timestamped build, which would + // put a coordinate in the records that no manifest ever names. + String version = artifact.getBaseVersion(); String id = internal - ? SocketSupport.bareId(artifact.getGroupId(), artifact.getArtifactId(), artifact.getVersion()) - : SocketSupport.coordId(artifact.getGroupId(), artifact.getArtifactId(), type, classifier, artifact.getVersion()); + ? SocketSupport.bareId(artifact.getGroupId(), artifact.getArtifactId(), version) + : SocketSupport.coordId(artifact.getGroupId(), artifact.getArtifactId(), type, classifier, version); // One walk per node per module traversal: a shared subtree reached via another edge is already - // fully recorded (node, children, resolved file), and the resolve below is a download-on-miss — - // so hand back the id without re-resolving or re-descending. Keeps reconverging graphs linear. + // fully recorded (node, children, resolved file), so hand back the id without re-descending. + // Keeps reconverging graphs linear. if (!visited.add(id)) return id; Node node = internal - ? upsert(nodes, id, artifact.getGroupId(), artifact.getArtifactId(), "", "", artifact.getVersion()) + ? upsert(nodes, id, artifact.getGroupId(), artifact.getArtifactId(), "", "", version) : upsert(nodes, id, artifact.getGroupId(), artifact.getArtifactId(), - type == null ? "" : type, classifier == null ? "" : classifier, artifact.getVersion()); - // `a.file` downloads if uncached, so scope to the requested GAVs (null = all). - if (!internal && opts.withFiles && (populateGavs == null || populateGavs.contains(gav))) { - String file = resolveArtifactFile(repoSession, artifact, scope, repos, failures); + type == null ? "" : type, classifier == null ? "" : classifier, version); + // Maven wrote each accepted node's resolved file back onto the node; a reactor module reports its + // own dirs through its `project` record instead of a `file` record. + if (!internal && opts.withFiles) { + String file = SocketSupport.existingAbsolutePath(artifact.getFile()); if (file != null) node.files.add(file); } if (isProd(scope)) node.prod = true; for (DependencyNode child : dn.getChildren()) { - String childId = visit(repoSession, child, passingScopes, reactorGavs, populateGavs, opts, repos, nodes, visited, failures); + String childId = visit(child, passingScopes, reactorGavs, opts, nodes, visited); if (childId != null) node.children.add(childId); } return id; } + private static String gav(Artifact artifact) { + return artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getBaseVersion(); + } + + private static String scopeOf(Dependency dep) { + String scope = dep == null ? null : dep.getScope(); + return scope == null || scope.isEmpty() ? "compile" : scope; + } + private static boolean isProd(String scope) { return scope.equals("compile") || scope.equals("runtime") || scope.equals("system"); } @@ -212,34 +300,6 @@ private static Node upsert( return node; } - private String resolveArtifactFile( - RepositorySystemSession repoSession, Artifact artifact, String scope, List repos, Set failures) { - if ("system".equals(scope)) { - return SocketSupport.existingAbsolutePath(artifact.getFile()); - } - ArtifactHandler handler = artifact.getArtifactHandler(); - String extension = handler != null ? handler.getExtension() : artifact.getType(); - String classifier = artifact.getClassifier(); - try { - ArtifactRequest request = new ArtifactRequest() - .setArtifact(new DefaultArtifact( - artifact.getGroupId(), - artifact.getArtifactId(), - classifier == null ? "" : classifier, - extension, - artifact.getVersion())) - .setRepositories(repos); - ArtifactResult result = repoSystem.resolveArtifact(repoSession, request); - File file = result.getArtifact() != null ? result.getArtifact().getFile() : null; - return SocketSupport.existingAbsolutePath(file); - } catch (ArtifactResolutionException e) { - String coord = artifact.getGroupId() + ":" + artifact.getArtifactId() + ":" + artifact.getVersion(); - failures.add(new Failure(coord, rootMessage(e), scope)); - log.debug("[socket-facts] could not materialize " + artifact + ": " + rootMessage(e)); - return null; - } - } - // ---- emission ---- // Split a module's resolved nodes into a prod root and a dev root (each artifact has one effective diff --git a/src/commands/manifest/scripts/test/README.md b/src/commands/manifest/scripts/test/README.md index 3515e747f9..0038bdf299 100644 --- a/src/commands/manifest/scripts/test/README.md +++ b/src/commands/manifest/scripts/test/README.md @@ -23,9 +23,12 @@ The matrix needs several JDKs. Point `JDK8` / `JDK11` / `JDK17` / `JDK21` at JDK homes to use the right one per row; otherwise the current `java` is used. The sbt rows also need the `sbt` launcher on `PATH`. -The runner downloads the build-tool distributions and invokes the per-ecosystem -`smoke-test.sh`. The unit-level assembler/sidecar behavior is covered separately -by the `*.test.mts` unit tests. +The runner does not fetch the build-tool distributions: SocketDev's CDN allowlist +keeps downloads out of a committed script. It expects each Gradle and Maven +distribution already unzipped under the cache root below, and prints the archive +URL for any that is missing; sbt comes from the `sbt` launcher on `PATH`. It then +invokes the per-ecosystem `smoke-test.sh`. The unit-level assembler/sidecar +behavior is covered separately by the `*.test.mts` unit tests. ## Stub dependencies @@ -36,6 +39,10 @@ test dep, a transitive), never the code, so a stub is behaviourally identical he and can never age into a CVE alert or a version bump. The generated repos are gitignored; nothing binary is committed. +The exception is `maven-compat/duplicate-failure`, whose dependency is deliberately +absent from every repository: it asserts how an unresolvable dependency is +reported, so it needs no stub and runs offline. + Each build tool still fetches its own closure — Maven's plugins, sbt's scala-library, the Gradle distribution — from the network, so these fixtures are not "fully offline"; they simply declare no third-party dependencies of their own. diff --git a/src/commands/manifest/scripts/test/maven-compat/.gitignore b/src/commands/manifest/scripts/test/maven-compat/.gitignore index 186d745866..e229e05c8b 100644 --- a/src/commands/manifest/scripts/test/maven-compat/.gitignore +++ b/src/commands/manifest/scripts/test/maven-compat/.gitignore @@ -3,3 +3,11 @@ project/records.tsv project/workspaces-records.tsv project/target/ project/*/target/ +repo-inheritance/localrepo/ +repo-inheritance/records.tsv +repo-inheritance/target/ +repo-inheritance/*/target/ +repo-inheritance/.mirror-settings.xml +duplicate-failure/records.tsv +duplicate-failure/target/ +duplicate-failure/*/target/ diff --git a/src/commands/manifest/scripts/test/maven-compat/assert-fail-closed.py b/src/commands/manifest/scripts/test/maven-compat/assert-fail-closed.py new file mode 100644 index 0000000000..738de9a1ac --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/assert-fail-closed.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 +# Assert the extension failed CLOSED: a dependency it could not materialize must appear as a +# `failure` record, which the CLI turns into an aborted scan. Silently dropping the jar would leave +# the reachability analysis unable to see through that artifact and under-report reachability. +# +# Both fixture modules reach demo.scoped:widget, so both fail on it, each naming the repository it +# attempted — so the count of records is not pinned here. What is pinned: the failure is attributed +# to the artifact's own scope, never to a module-level `graph` failure, which would claim the +# dependency graph could not be built when it collected fine. +# smoke-test-duplicate-failure.sh covers the identical-message case, where those records collapse. +import sys + +rows = [l.rstrip('\n').split('\t') for l in open(sys.argv[1]) if l.strip()] +failures = [r[1:] for r in rows if r[0] == 'failure'] +widget = [f for f in failures if f[0].startswith('demo.scoped:widget:')] +graph = [f for f in failures if f[2] == 'graph'] +errors = [] + +if not widget: + errors.append(f"an unresolvable dependency emitted no failure record; failures={failures}") +elif any(f[2] != 'compile' for f in widget): + errors.append(f"demo.scoped:widget should fail in config 'compile', got {[f[2] for f in widget]}") +if graph: + errors.append(f"graph collected fine, so no module-level 'graph' failure should exist: {graph}") + +if errors: + print("FAIL:") + for e in errors: + print(" -", e) + sys.exit(1) +print(f"PASS (fail closed): unresolvable demo.scoped:widget reported in config 'compile' ({len(widget)} record(s)); no module-level 'graph' failure") diff --git a/src/commands/manifest/scripts/test/maven-compat/assert-no-invented-failures.py b/src/commands/manifest/scripts/test/maven-compat/assert-no-invented-failures.py new file mode 100644 index 0000000000..80577d4fc8 --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/assert-no-invented-failures.py @@ -0,0 +1,26 @@ +#!/usr/bin/env python3 +# Assert the extension reported the unresolvable dependency, and only that: no module-level `graph` +# failure may be invented for a reactor whose graph collected fine. Counting the artifact's own +# failures is deliberately loose — identical messages collapse in the shared accumulator, so the +# count depends on how each Maven version words them, while the `graph` invariant does not. +import sys + +rows = [l.rstrip('\n').split('\t') for l in open(sys.argv[1]) if l.strip()] +failures = [r[1:] for r in rows if r[0] == 'failure'] +ghost = [f for f in failures if f[0].startswith('demo.missing:ghost:')] +graph = [f for f in failures if f[2] == 'graph'] +errors = [] + +if not ghost: + errors.append(f"unresolvable demo.missing:ghost emitted no failure record; failures={failures}") +elif any(f[2] != 'compile' for f in ghost): + errors.append(f"demo.missing:ghost should fail in config 'compile', got {[f[2] for f in ghost]}") +if graph: + errors.append(f"every module's graph collected fine, so no 'graph' failure should exist: {graph}") + +if errors: + print("FAIL:") + for e in errors: + print(" -", e) + sys.exit(1) +print("PASS (no invented failures): demo.missing:ghost reported in config 'compile'; no module-level 'graph' failure") diff --git a/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/m1/pom.xml b/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/m1/pom.xml new file mode 100644 index 0000000000..7a2a7ca62c --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/m1/pom.xml @@ -0,0 +1,10 @@ + + + 4.0.0 + + demo + dup-root + 1.0 + + m1 + diff --git a/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/m2/pom.xml b/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/m2/pom.xml new file mode 100644 index 0000000000..f053c10061 --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/m2/pom.xml @@ -0,0 +1,10 @@ + + + 4.0.0 + + demo + dup-root + 1.0 + + m2 + diff --git a/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/pom.xml b/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/pom.xml new file mode 100644 index 0000000000..f8edf851a2 --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/duplicate-failure/pom.xml @@ -0,0 +1,26 @@ + + + 4.0.0 + demo + dup-root + 1.0 + pom + + m1 + m2 + + + 8 + 8 + UTF-8 + + + + + demo.missing + ghost + 9.9.9 + + + diff --git a/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/liba/pom.xml b/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/liba/pom.xml new file mode 100644 index 0000000000..7fcf3743be --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/liba/pom.xml @@ -0,0 +1,32 @@ + + + 4.0.0 + + demo + inherit-root + 1.0 + + liba + + + + socket-scoped-stubs + ${stub.repo.url} + + true + + ignore + + + false + + + + + + demo.scoped + widget + 1.0 + + + diff --git a/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/libb/pom.xml b/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/libb/pom.xml new file mode 100644 index 0000000000..d9612220d5 --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/libb/pom.xml @@ -0,0 +1,18 @@ + + + 4.0.0 + + demo + inherit-root + 1.0 + + libb + + + + demo + liba + 1.0 + + + diff --git a/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/pom.xml b/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/pom.xml new file mode 100644 index 0000000000..4add413fc8 --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/repo-inheritance/pom.xml @@ -0,0 +1,21 @@ + + + 4.0.0 + demo + inherit-root + 1.0 + pom + + liba + libb + + + + 8 + 8 + UTF-8 + file://${maven.multiModuleProjectDirectory}/localrepo + + diff --git a/src/commands/manifest/scripts/test/maven-compat/smoke-test-duplicate-failure.sh b/src/commands/manifest/scripts/test/maven-compat/smoke-test-duplicate-failure.sh new file mode 100755 index 0000000000..cf5a4ecf5c --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/smoke-test-duplicate-failure.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# Regression guard: an unresolvable dependency shared by several reactor modules must be reported as +# what it is — a per-artifact resolution failure — and must never be reported as a module-level +# `graph` failure, which would claim the dependency graph itself could not be built. +# +# Every module inherits the same missing dependency and the same repository list, so every module's +# failure carries an identical message. The accumulator is a value-equality set shared across the +# whole reactor, so those identical failures collapse into one entry — and a fail-closed check that +# asks "did this module add anything?" by comparing set sizes concludes, wrongly, that the module +# reported nothing and invents a `graph` failure for it. +# +# Runs offline: the missing artifact is never looked for in a remote repository, so this needs no +# stub repo and never touches the network. +# +# Usage: smoke-test-duplicate-failure.sh +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +MVN="${1:?usage: smoke-test-duplicate-failure.sh }" +JAR="${2:?usage: smoke-test-duplicate-failure.sh }" +PROJECT="$HERE/duplicate-failure" +RECORDS="$PROJECT/records.tsv" +# shellcheck source=SCRIPTDIR/../compat-cache.sh +. "$HERE/../compat-cache.sh" +M2="$SOCKET_COMPAT_CACHE/m2" + +rm -rf "$M2/demo/missing" "$RECORDS" + +echo "+ $("$MVN" -v 2>/dev/null | head -1) (duplicate failure)" +( cd "$PROJECT" && "$MVN" --batch-mode -q -o \ + "-Dmaven.ext.class.path=$JAR" \ + -Dcoana.task=socket-facts \ + -Dsocket.withFiles=true \ + "-Dmaven.repo.local=$M2" \ + "-Dsocket.recordsFile=$RECORDS" \ + validate ) + +python3 "$HERE/assert-no-invented-failures.py" "$RECORDS" diff --git a/src/commands/manifest/scripts/test/maven-compat/smoke-test-repo-inheritance.sh b/src/commands/manifest/scripts/test/maven-compat/smoke-test-repo-inheritance.sh new file mode 100755 index 0000000000..447756674e --- /dev/null +++ b/src/commands/manifest/scripts/test/maven-compat/smoke-test-repo-inheritance.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# Regression guard: a dependency reachable only through a repository declared by a SIBLING reactor +# module must still be materialized under -Dsocket.withFiles. +# +# The fixture mirrors the shape that broke in the field: `liba` declares the only repository serving +# demo.scoped:widget, and `libb` reaches that artifact transitively through its reactor dependency on +# liba. Maven resolves each dependency node against the repositories that node's own descriptor +# lineage contributes, so widget resolves for libb too. Resolving instead against libb's own +# repository list — the module the walk started from — finds nothing, and (because Aether's local +# repository records which repository each cached file came from) not even an already-downloaded copy +# in ~/.m2 is considered available. +# +# Runs at `validate`, the phase the CLI's runner uses: no reactor module is packaged and none is +# installed, so this also guards that a reactor sibling's own jar is never requested. +# +# Usage: smoke-test-repo-inheritance.sh +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +MVN="${1:?usage: smoke-test-repo-inheritance.sh }" +JAR="${2:?usage: smoke-test-repo-inheritance.sh }" +PROJECT="$HERE/repo-inheritance" +RECORDS="$PROJECT/records.tsv" +STUBS="$PROJECT/localrepo" +# shellcheck source=SCRIPTDIR/../compat-cache.sh +. "$HERE/../compat-cache.sh" +M2="$SOCKET_COMPAT_CACHE/m2" + +bash "$HERE/../make-stub-repo.sh" "$STUBS" 'demo.scoped:widget:1.0' +# The stub never stays cached: every run has to resolve it through the repository liba declares. +rm -rf "$M2/demo/scoped" "$RECORDS" + +echo "+ $("$MVN" -v 2>/dev/null | head -1) (repo inheritance)" +( cd "$PROJECT" && "$MVN" --batch-mode -q \ + "-Dmaven.ext.class.path=$JAR" \ + -Dcoana.task=socket-facts \ + -Dsocket.withFiles=true \ + "-Dmaven.repo.local=$M2" \ + "-Dstub.repo.url=file://$STUBS" \ + "-Dsocket.recordsFile=$RECORDS" \ + validate ) + +python3 - "$RECORDS" <<'PY' +import sys +rows = [l.rstrip('\n').split('\t') for l in open(sys.argv[1]) if l.strip()] +roots, nodes, files, edges, failures = {}, {}, {}, set(), [] +for r in rows: + if r[0] == 'root': roots[r[1]] = r[2] # rootId -> projectKey + elif r[0] == 'node': nodes.setdefault(r[2], set()).add(r[1]) # coordId -> {rootId} + elif r[0] == 'file': files.setdefault(r[2], set()).add(r[3]) # coordId -> {path} + elif r[0] == 'edge': edges.add((r[1], r[2], r[3])) # (rootId, parent, child) + elif r[0] == 'failure': failures.append(r[1:]) + +WIDGET = 'demo.scoped:widget:jar:1.0' +LIBA = 'demo:liba:1.0' +errors = [] + +if failures: + errors.append(f"expected no failure records, got {failures}") + +def roots_for(project_key): + return {rid for rid, key in roots.items() if key == project_key} + +for project_key in ('liba', 'libb'): + rids = roots_for(project_key) + if not rids: + errors.append(f"no root record for module {project_key!r}") + continue + if not (nodes.get(WIDGET, set()) & rids): + errors.append(f"{WIDGET} missing from {project_key}'s graph") + +# The point of the fixture: libb reaches widget only through the reactor module liba. +if not any(rid in roots_for('libb') and p == LIBA and c == WIDGET for rid, p, c in edges): + errors.append(f"no {LIBA} -> {WIDGET} edge in libb's root") + +jars = [p for p in files.get(WIDGET, ()) if p.endswith('.jar')] +if not jars: + errors.append(f"{WIDGET} jar not materialized: {files.get(WIDGET)}") + +# A reactor sibling reports its dirs through its `project` record, never a `file` record. +if files.get(LIBA): + errors.append(f"reactor module {LIBA} should have no file records, got {files[LIBA]}") + +if errors: + print("FAIL:") + for e in errors: print(" -", e) + sys.exit(1) +print(f"PASS (repo inheritance): {WIDGET} resolved for liba and libb via liba's repository; jar materialized") +PY + +# Second run, the other half of the contract: fail CLOSED. With the artifact gone from every +# repository — central mirrored to the same, now widget-less, stub repo so nothing crosses a network +# — a dependency that cannot be materialized MUST surface as a failure record. A silently missing jar +# would leave the reachability analysis blind to whatever it contains and under-report reachability. +SETTINGS="$PROJECT/.mirror-settings.xml" +cat >"$SETTINGS" < + + + socket-no-network + central + file://$STUBS + + + +XML +rm -rf "$STUBS/demo/scoped" "$M2/demo/scoped" "$RECORDS" +( cd "$PROJECT" && "$MVN" --batch-mode -q \ + "-Dmaven.ext.class.path=$JAR" \ + -Dcoana.task=socket-facts \ + -Dsocket.withFiles=true \ + "-Dmaven.repo.local=$M2" \ + "-Dstub.repo.url=file://$STUBS" \ + -s "$SETTINGS" \ + "-Dsocket.recordsFile=$RECORDS" \ + validate ) +rm -f "$SETTINGS" + +python3 "$HERE/assert-fail-closed.py" "$RECORDS" diff --git a/src/commands/manifest/scripts/test/run-compat.sh b/src/commands/manifest/scripts/test/run-compat.sh index 7c5fd7dc58..4ae053bd24 100755 --- a/src/commands/manifest/scripts/test/run-compat.sh +++ b/src/commands/manifest/scripts/test/run-compat.sh @@ -24,7 +24,10 @@ CACHE="$SOCKET_COMPAT_CACHE" # Same matrix as the former CI workflow. Rows: " [scala]". GRADLE_MATRIX=("1.12 8" "2.14.1 8" "3.3 8" "8.10.2 17" "9.2.1 21") -MAVEN_MATRIX=("3.6.3 11" "3.8.8 17" "3.9.9 17") +# The Maven rows span the range the extension claims to support (see maven-extension/pom.xml): +# 3.2.5 is the oldest, 4.x the newest. Resolution goes through Maven's own +# ProjectDependenciesResolver, so both ends are load-bearing, not decorative. +MAVEN_MATRIX=("3.2.5 8" "3.6.3 11" "3.8.8 17" "3.9.9 17" "4.0.0-rc-6 21") SBT_MATRIX=("0.13.18 8 2.10.7" "1.4.9 11 2.12.20" "1.6.2 17 2.12.20" "1.9.9 17 2.12.20") # Select a JDK for the given Java major: use $JDK if set, else current java. @@ -74,12 +77,15 @@ run_maven() { if [ ! -x "$dir/bin/mvn" ]; then # Distribution downloads stay out of this committed script (fleet CDN # allowlist); the operator fetches the archive once into the cache. + local line="maven-${ver%%.*}" echo "maven $ver not found at $dir/bin/mvn." >&2 - echo "Fix: download https://archive.apache.org/dist/maven/maven-3/$ver/binaries/apache-maven-$ver-bin.zip and unzip it into $CACHE" >&2 + echo "Fix: download https://archive.apache.org/dist/maven/$line/$ver/binaries/apache-maven-$ver-bin.zip and unzip it into $CACHE" >&2 exit 1 fi bash "$HERE/maven-compat/smoke-test.sh" "$dir/bin/mvn" "$jar" bash "$HERE/maven-compat/smoke-test-workspaces.sh" "$dir/bin/mvn" "$jar" + bash "$HERE/maven-compat/smoke-test-repo-inheritance.sh" "$dir/bin/mvn" "$jar" + bash "$HERE/maven-compat/smoke-test-duplicate-failure.sh" "$dir/bin/mvn" "$jar" done } From 0b443513cdce3862f1f6bfbb59d3a1fb0681dad4 Mon Sep 17 00:00:00 2001 From: Jeppe Fredsgaard Blaabjerg Date: Fri, 28 Aug 2026 12:05:03 +0200 Subject: [PATCH 2/3] docs(changelog): restore [Unreleased] and drop the burned 1.1.161 section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1.1.161 never reached npm — 1.1.160 is still latest there — so no user can install what that section describes. Its tag and GitHub release are immutable and stay; the changelog documents the published package, and GitHub generates its own release notes from the PR list, so the two need not agree. The next bump derives 1.1.162 from the reachable tag either way, so nothing here affects the release tooling. How the section got there: #1516 wrote its heading as `## [Unreleased] - 2026-08-27`, and unreleasedRange() in scripts/release/changelog.mts locates the block by comparing the trimmed, lowercased heading for equality with `## [unreleased]`. The trailing date made it miss, so the release found nothing accrued, fell back to the commit-derived section, and inserted its own heading above the block it could not see — stranding `[Unreleased]` below a released version. - Drops the 1.1.161 section and returns the Coana 15.10.25 note to `## [Unreleased]`, to be promoted by the next release that ships. - Puts `## [Unreleased]` back at the top, without a date. - Leaves out "stop the coana bump from hand-writing versions": a release-workflow change with nothing for a user of the package to act on, which only appeared because the commit-derived fallback ran. - Files the Maven resolver fix under `[Unreleased]`. The locator's intolerance of a trailing date is left alone here; it wants its own change. --- CHANGELOG.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e9ac06c6e..c6b207276f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,16 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). -## [1.1.161](https://github.com/SocketDev/socket-cli/releases/tag/v1.1.161) - 2026-08-27 - -### Fixed -- **`release`** — stop the coana bump from hand-writing versions (#1515) - -## [Unreleased] - 2026-08-27 +## [Unreleased] ### Changed - Updated the Coana CLI to v `15.10.25`. +### Fixed +- Maven reachability scans now resolve dependencies through Maven itself, so they see the same artifacts a Maven build does — including one served only by a repository that a single module declares. + ## [1.1.160](https://github.com/SocketDev/socket-cli/releases/tag/v1.1.160) - 2026-08-26 ### Changed From e7e8f09e122353fcd5b580f60992574e0fefee96 Mon Sep 17 00:00:00 2001 From: Jeppe Fredsgaard Blaabjerg Date: Fri, 28 Aug 2026 12:11:36 +0200 Subject: [PATCH 3/3] docs(bump-coana): forbid a date on the [Unreleased] heading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule against writing a `## []` heading did not say what the heading may look like when it is recreated, and #1516 shows why that matters. The bump wrote `## [1.1.161](...) - 2026-08-27`; the follow-up correction changed the version to `Unreleased` but kept the date, leaving `## [Unreleased] - 2026-08-27`. unreleasedRange() in scripts/release/changelog.mts matches that heading for equality — case-insensitively, but otherwise exactly — so the dated form is invisible to it. The release promoted nothing, fell back to the section derived from the commits in range, and inserted its own heading above the block it could not see. The note sat below a released version where no release would pick it up, and the version it named never reached npm. Pins the recreated heading to exactly `## [Unreleased]` and says why a date breaks promotion, so the next correction of a malformed heading lands on the form the release can actually find. --- .claude/skills/bump-coana/SKILL.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.claude/skills/bump-coana/SKILL.md b/.claude/skills/bump-coana/SKILL.md index 8f8f6e918e..6bd6a029a7 100644 --- a/.claude/skills/bump-coana/SKILL.md +++ b/.claude/skills/bump-coana/SKILL.md @@ -54,7 +54,8 @@ advance. 1. Read `CHANGELOG.md` in the repository root. 2. Find the `## [Unreleased]` heading. If it is absent — the previous release consumes it — recreate it directly after the header section (which ends with - "The format is based on..."). + "The format is based on..."), spelled exactly `## [Unreleased]` and nothing + else. 3. Add the entry under `## [Unreleased]`, in its `### Changed` subsection, creating that subsection if it is missing. If a Coana line is already there from an earlier unreleased bump, update it in place rather than adding a @@ -65,6 +66,15 @@ release workflow, which promotes the whole `## [Unreleased]` block under the version it derives. Writing one here both names a version that may never exist and consumes the block, leaving the real release with empty notes. +🚨 **Never put a date on the `## [Unreleased]` heading.** `unreleasedRange()` in +`scripts/release/changelog.mts` finds the block by matching that heading for +equality (case-insensitively, but otherwise exactly), so a heading like +`## [Unreleased] - 2026-08-27` is invisible to it. The release then promotes +nothing, falls back to a section derived from the commits in range, and inserts +its own heading *above* the block it could not see — stranding the entry below a +released version, where no release will ever pick it up. Dates belong only on +release headings, which the workflow writes. + **Resulting shape**: ```markdown ## [Unreleased]