Conversation
Add Maven multi-module workspace discovery to the Java client. Uses `mvn help:evaluate -Dexpression=project.modules` to list declared modules, with recursive traversal for nested aggregators and wrapper support via `selectMvnRuntime()`. Adds `**/target/**` to default workspace discovery ignore patterns. Maven detection added between Cargo and JavaScript in ecosystem order. Implements TC-4260 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Assisted-by: Claude Code
Contributor
Reviewer's GuideImplements Maven multi-module workspace discovery in ExhortApi by resolving a Maven (or mvnw) binary, invoking Sequence diagram for Maven multi-module workspace discoverysequenceDiagram
participant Caller
participant ExhortApi
participant Operations
participant JavaMavenProvider
Caller->>ExhortApi: discoverWorkspaceManifests(workspaceDir, ignorePatterns)
ExhortApi->>ExhortApi: Files.exists(workspaceDir/Cargo.toml)
ExhortApi-->>Caller: discoverCargoManifests(...) (if Cargo workspace)
ExhortApi->>ExhortApi: Files.isRegularFile(workspaceDir/pom.xml)
ExhortApi->>ExhortApi: discoverMavenModules(workspaceDir, ignorePatterns)
rect rgb(235, 245, 255)
ExhortApi->>ExhortApi: resolveMavenBinary(workspaceDir)
ExhortApi->>Operations: getWrapperPreference(mvn)
Operations-->>ExhortApi: preferWrapper
ExhortApi->>Operations: isWindows()
Operations-->>ExhortApi: isWindowsFlag
ExhortApi->>JavaMavenProvider: traverseForMvnw(wrapperName, pomPath)
JavaMavenProvider-->>ExhortApi: mvnwPath or null
ExhortApi->>Operations: getCustomPathOrElse(mvn)
Operations-->>ExhortApi: mvnBin or null
ExhortApi-->>ExhortApi: mvnBin == null ?
end
ExhortApi-->>Caller: [root pom.xml only] (if mvnBin == null)
Note over ExhortApi: Maven binary available
ExhortApi->>ExhortApi: collectMavenModules(workspaceDir, mvnBin, visited, manifestPaths)
loop for each directory dir
ExhortApi->>ExhortApi: listMavenModules(dir, mvnBin)
ExhortApi->>Operations: runProcessGetFullOutput(dir, [mvn, help:evaluate, ...])
Operations-->>ExhortApi: ProcessExecOutput
ExhortApi-->>ExhortApi: check exitCode, output
ExhortApi->>ExhortApi: parseMavenModuleList(rawOutput)
ExhortApi-->>ExhortApi: List moduleNames
loop for each module
ExhortApi->>ExhortApi: resolve moduleDir and modulePom
ExhortApi-->>ExhortApi: Files.isRegularFile(modulePom)
ExhortApi->>ExhortApi: add modulePom to manifestPaths
ExhortApi->>ExhortApi: collectMavenModules(moduleDir, mvnBin, visited, manifestPaths)
end
end
ExhortApi->>Operations: WorkspaceUtils.filterByIgnorePatterns(workspaceDir, manifestPaths, ignorePatterns)
Operations-->>ExhortApi: filteredManifestPaths
ExhortApi-->>Caller: filteredManifestPaths
Updated class diagram for ExhortApi Maven workspace discoveryclassDiagram
class ExhortApi {
- static Set DEFAULT_WORKSPACE_DISCOVERY_IGNORE
+ List discoverWorkspaceManifests(Path workspaceDir, Set ignorePatterns) throws IOException
- List discoverCargoManifests(Path workspaceDir, Set ignorePatterns) throws IOException
- List discoverMavenModules(Path workspaceDir, Set ignorePatterns)
- void collectMavenModules(Path dir, String mvnBin, java.util.HashSet visited, List manifestPaths)
- List listMavenModules(Path dir, String mvnBin)
- static List parseMavenModuleList(String raw)
- String resolveMavenBinary(Path startDir)
}
class Operations {
+ static boolean isWindows()
+ static boolean getWrapperPreference(String tool)
+ static String getCustomPathOrElse(String binary)
+ static Operations.ProcessExecOutput runProcessGetFullOutput(Path dir, String[] command, Object env)
}
class JavaMavenProvider {
+ static String traverseForMvnw(String wrapperName, String pomPath)
}
class WorkspaceUtils {
+ static List filterByIgnorePatterns(Path workspaceDir, List manifestPaths, Set ignorePatterns)
}
class ProcessExecOutput {
+ int getExitCode()
+ String getOutput()
}
ExhortApi ..> Operations : uses
ExhortApi ..> JavaMavenProvider : uses
ExhortApi ..> WorkspaceUtils : uses
Operations ..> ProcessExecOutput : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit b32dc96. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The Javadoc for discoverMavenModules() says it returns an empty list when Maven is unavailable, but the implementation returns a singleton list with the root pom.xml; update either the Javadoc or the behavior so they are consistent.
- parseMavenModuleList() and listMavenModules() currently split the responsibility for handling the "null" sentinel value from mvn; consider centralizing that handling inside parseMavenModuleList() so it fully reflects the real mvn output format and callers don’t need to special-case it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Javadoc for discoverMavenModules() says it returns an empty list when Maven is unavailable, but the implementation returns a singleton list with the root pom.xml; update either the Javadoc or the behavior so they are consistent.
- parseMavenModuleList() and listMavenModules() currently split the responsibility for handling the "null" sentinel value from mvn; consider centralizing that handling inside parseMavenModuleList() so it fully reflects the real mvn output format and callers don’t need to special-case it.
## Individual Comments
### Comment 1
<location path="src/main/java/io/github/guacsec/trustifyda/impl/ExhortApi.java" line_range="954-957" />
<code_context>
+ * @param ignorePatterns glob patterns for paths to exclude from results
+ * @return list of discovered pom.xml paths, or empty list if Maven is unavailable
+ */
+ private List<Path> discoverMavenModules(Path workspaceDir, Set<String> ignorePatterns) {
+ Path rootPom = workspaceDir.resolve("pom.xml");
+ String mvnBin = resolveMavenBinary(workspaceDir);
+ if (mvnBin == null) {
+ LOG.warning("Maven binary not available; returning root pom.xml only");
+ return List.of(rootPom);
</code_context>
<issue_to_address>
**issue (bug_risk):** Method behavior disagrees with its JavaDoc when Maven is unavailable.
JavaDoc promises an empty list when Maven is unavailable, but this branch returns `List.of(rootPom)` when `resolveMavenBinary` is `null`. Either update the JavaDoc to document that the root POM is always returned, or change this branch to return an empty list to match the documented contract.
</issue_to_address>
### Comment 2
<location path="src/test/java/io/github/guacsec/trustifyda/impl/MavenWorkspaceDiscoveryTest.java" line_range="292" />
<code_context>
+ }
+ }
+
+ /** Verifies that when mvn command fails, the root pom.xml is still returned. */
+ @Test
+ void discoverWorkspaceManifests_mvnCommandFails() throws IOException {
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test for the case where Maven cannot be resolved at all (resolveMavenBinary returns null).
There’s good coverage for mvn command failures, but nothing that hits the `resolveMavenBinary()` path where it returns `null`. Since `discoverMavenModules()` then logs a warning and falls back to only the root `pom.xml`, please add a test that stubs `resolveMavenBinary` to return `null`, calls `discoverWorkspaceManifests` on a directory with a `pom.xml`, and verifies that only the root `pom.xml` is returned. This ensures correct behavior when Maven isn’t installed or resolvable.
Suggested implementation:
```java
assertThat(manifests).noneMatch(p -> p.toString().contains("module-missing"));
}
}
/** Verifies behavior when Maven cannot be resolved at all. */
@Test
void discoverWorkspaceManifests_mavenBinaryNotResolved() throws IOException {
// Given a Maven workspace where Maven cannot be resolved (resolveMavenBinary returns null)
Path workspaceDir = MAVEN_FIXTURES.resolve("maven_multi_module").toAbsolutePath().normalize();
WorkspaceDiscoveryApi api = new MavenWorkspaceDiscovery();
try (MockedStatic<Operations> mockOps = Mockito.mockStatic(Operations.class)) {
// Simulate that no wrapper is preferred and Maven cannot be resolved from PATH/custom location
mockOps.when(() -> Operations.getWrapperPreference("mvn")).thenReturn(false);
mockOps.when(Operations::isWindows).thenReturn(false);
mockOps.when(() -> Operations.getCustomPathOrElse("mvn")).thenReturn(null);
// When discovering workspace manifests
List<Path> manifests = api.discoverWorkspaceManifests(workspaceDir, Set.of());
// Then only the root pom.xml is returned
assertThat(manifests).containsExactly(workspaceDir.resolve("pom.xml"));
}
}
/** Verifies that when mvn command fails, the root pom.xml is still returned. */
```
If the actual resolution logic uses a different method or class than `Operations.getCustomPathOrElse("mvn")` to implement `resolveMavenBinary()`, you should adjust the stubbing in this test to directly mock the method that returns `null` (for example, by mocking a `resolveMavenBinary()` method on `MavenWorkspaceDiscovery` instead of or in addition to `Operations`). Also ensure that `WorkspaceDiscoveryApi`, `MavenWorkspaceDiscovery`, `Operations`, and `MockedStatic` are already imported; if not, add the appropriate imports at the top of the test file.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+954
to
+957
| private List<Path> discoverMavenModules(Path workspaceDir, Set<String> ignorePatterns) { | ||
| Path rootPom = workspaceDir.resolve("pom.xml"); | ||
| String mvnBin = resolveMavenBinary(workspaceDir); | ||
| if (mvnBin == null) { |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Method behavior disagrees with its JavaDoc when Maven is unavailable.
JavaDoc promises an empty list when Maven is unavailable, but this branch returns List.of(rootPom) when resolveMavenBinary is null. Either update the JavaDoc to document that the root POM is always returned, or change this branch to return an empty list to match the documented contract.
| } | ||
| } | ||
|
|
||
| /** Verifies that when mvn command fails, the root pom.xml is still returned. */ |
Contributor
There was a problem hiding this comment.
suggestion (testing): Missing test for the case where Maven cannot be resolved at all (resolveMavenBinary returns null).
There’s good coverage for mvn command failures, but nothing that hits the resolveMavenBinary() path where it returns null. Since discoverMavenModules() then logs a warning and falls back to only the root pom.xml, please add a test that stubs resolveMavenBinary to return null, calls discoverWorkspaceManifests on a directory with a pom.xml, and verifies that only the root pom.xml is returned. This ensures correct behavior when Maven isn’t installed or resolvable.
Suggested implementation:
assertThat(manifests).noneMatch(p -> p.toString().contains("module-missing"));
}
}
/** Verifies behavior when Maven cannot be resolved at all. */
@Test
void discoverWorkspaceManifests_mavenBinaryNotResolved() throws IOException {
// Given a Maven workspace where Maven cannot be resolved (resolveMavenBinary returns null)
Path workspaceDir = MAVEN_FIXTURES.resolve("maven_multi_module").toAbsolutePath().normalize();
WorkspaceDiscoveryApi api = new MavenWorkspaceDiscovery();
try (MockedStatic<Operations> mockOps = Mockito.mockStatic(Operations.class)) {
// Simulate that no wrapper is preferred and Maven cannot be resolved from PATH/custom location
mockOps.when(() -> Operations.getWrapperPreference("mvn")).thenReturn(false);
mockOps.when(Operations::isWindows).thenReturn(false);
mockOps.when(() -> Operations.getCustomPathOrElse("mvn")).thenReturn(null);
// When discovering workspace manifests
List<Path> manifests = api.discoverWorkspaceManifests(workspaceDir, Set.of());
// Then only the root pom.xml is returned
assertThat(manifests).containsExactly(workspaceDir.resolve("pom.xml"));
}
}
/** Verifies that when mvn command fails, the root pom.xml is still returned. */If the actual resolution logic uses a different method or class than Operations.getCustomPathOrElse("mvn") to implement resolveMavenBinary(), you should adjust the stubbing in this test to directly mock the method that returns null (for example, by mocking a resolveMavenBinary() method on MavenWorkspaceDiscovery instead of or in addition to Operations). Also ensure that WorkspaceDiscoveryApi, MavenWorkspaceDiscovery, Operations, and MockedStatic are already imported; if not, add the appropriate imports at the top of the test file.
a-oren
reviewed
Apr 28, 2026
| if (preferWrapper) { | ||
| String wrapperName = Operations.isWindows() ? "mvnw.cmd" : "mvnw"; | ||
| String mvnw = | ||
| JavaMavenProvider.traverseForMvnw(wrapperName, startDir.resolve("pom.xml").toString()); |
Contributor
There was a problem hiding this comment.
traverseForMvnw with 2 args is a private instance method (line 462 of JavaMavenProvider.java). The public static overload requires 3 args (line 466). This call from ExhortApi should not compile, it's calling a private method from outside the class, and Java won't auto-fill a null third argument.
a-oren
reviewed
Apr 28, 2026
| * @param startDir the directory from which to start searching for mvnw | ||
| * @return the resolved Maven binary path, or null if Maven is not available | ||
| */ | ||
| private String resolveMavenBinary(Path startDir) { |
Contributor
There was a problem hiding this comment.
resolveMavenBinary duplicates the logic from selectMvnRuntime but skips the binary verification step (mvnw -v). Consider reusing selectMvnRuntime (or extracting a shared helper) so both paths stay consistent. Not a bug, failures are caught downstream, but a broken binary would produce a misleading "mvn help:evaluate failed" warning instead of a clear "maven not available" message.
a-oren
requested changes
Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
discoverMavenModules()to discover all pom.xml manifest paths in Maven multi-module projects usingmvn help:evaluateresolveMavenBinary()reusingJavaMavenProvider.traverseForMvnw()discoverWorkspaceManifests()between Cargo and JavaScript**/target/**to default workspace discovery ignore patternsTest plan
parseMavenModuleList()unit tests (standard, single, null, empty, malformed, whitespace)**/target/**Implements TC-4260
🤖 Generated with Claude Code
Summary by Sourcery
Add Maven multi-module workspace discovery to Exhort workspace scanning and extend default ignore patterns.
New Features:
Enhancements:
Tests: