More maven parsing fix for experiment detector by zhenghao104 · Pull Request #1724 · microsoft/component-detection

zhenghao104 · 2026-03-16T20:37:49Z

No description provided.

Copilot

Pull request overview

This PR updates the experimental MavenWithFallbackDetector to improve static POM parsing behavior by introducing “smart” POM preloading intended to help cross-document property/version resolution during fallback parsing.

Changes:

Added per-scan state to support preloading all POM XmlDocuments for variable resolution.
Wired preloading into the fallback path and variable-replacement path to enable cross-document lookups.

src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs

github-actions · 2026-03-16T23:55:40Z

👋 Hi! It looks like you modified some files in the Detectors folder.
You may need to bump the detector versions if any of the following scenarios apply:

The detector detects more or fewer components than before
The detector generates different parent/child graph relationships than before
The detector generates different devDependencies values than before

If none of the above scenarios apply, feel free to ignore this comment 🙂

Copilot

Pull request overview

This PR updates the Maven detector flow to avoid .mvndeps file cleanup races by deferring cleanup until detector execution completes, and enhances the experimental Maven fallback detector’s static parsing to better handle version variables.

Changes:

Add orchestrator-level cleanup of Maven .mvndeps files after all detectors finish.
Refactor Maven detectors to remove per-file reader coordination and introduce a two-pass static POM parsing approach (collect variables, then resolve pending components).
Add orchestrator unit tests covering Maven cleanup behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs	Adds tests validating `.mvndeps` cleanup behavior depending on whether Maven detectors ran.
src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs	Adds post-processing cleanup routine for Maven `.mvndeps` files after detector execution completes.
src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs	Introduces a record type to queue unresolved Maven components for second-pass resolution.
src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs	Removes reader registration/unregistration logic and tweaks debug logging.
src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs	Implements two-pass variable collection + pending component resolution and updates telemetry/logging.
src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs	Removes file-reader coordination and deletion logic from the command service.
src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs	Removes file-reader coordination APIs from the service interface.

test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs

src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs

src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs

Copilot

Pull request overview

This PR updates Maven detector behavior to avoid Maven CLI dependency-file cleanup races, and improves MavenWithFallback static parsing by deferring variable-based version resolution to a second pass.

Changes:

Add orchestrator-level cleanup of Maven CLI *.mvndeps files after all detectors complete.
Rework MavenWithFallbackDetector static parsing to collect variables first and resolve variable-based versions in a second pass (via PendingComponent).
Remove Maven CLI file-reader registration/unregistration coordination APIs from IMavenCommandService/MavenCommandService, and update detectors accordingly.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs	Adds tests asserting Maven deps files are deleted only when Maven detectors run.
src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs	Adds post-scan Maven deps file cleanup in the orchestrator.
src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs	Introduces a record to hold unresolved Maven components for second-pass processing.
src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs	Removes per-file reader unregister logic; adds additional debug logging.
src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs	Implements two-pass variable collection + resolution for static parsing.
src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs	Removes file-reader reference counting/cleanup logic.
src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs	Removes file-reader coordination methods from the interface.

src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs

test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs

src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs

Copilot

Pull request overview

This PR tightens Maven detection behavior by (1) improving MavenWithFallback’s static POM parsing to resolve variables across parent/child hierarchies using multi-pass processing, and (2) moving Maven CLI .mvndeps cleanup to the orchestrator after all detectors complete to avoid cross-detector races.

Changes:

Add orchestrator-level cleanup of Maven CLI-generated dependency files after detector execution.
Implement multi-pass variable collection + hierarchy-aware resolution in MavenWithFallbackDetector and add a PendingComponent model for deferred registrations.
Expand test coverage for variable resolution edge cases and new cleanup behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/Microsoft.ComponentDetection.Orchestrator.Tests/Services/DetectorProcessingServiceTests.cs	Adds tests validating post-processing cleanup of `.mvndeps` files depending on which detectors ran.
test/Microsoft.ComponentDetection.Detectors.Tests/MavenWithFallbackDetectorTests.cs	Adds many tests covering parent/child/sibling variable resolution, deferred resolution, and traversal robustness.
src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs	Adds `CleanupMavenFilesAsync` and invokes it after all detectors finish.
src/Microsoft.ComponentDetection.Detectors/maven/PendingComponent.cs	Introduces a record to hold unresolved components for second/third-pass resolution.
src/Microsoft.ComponentDetection.Detectors/maven/MvnCliComponentDetector.cs	Removes per-file reader unregister logic and adds debug logging around contribution counts.
src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs	Introduces multi-pass parsing with concurrent state structures, hierarchy tracking, and deferred resolution.
src/Microsoft.ComponentDetection.Detectors/maven/MavenCommandService.cs	Removes file-reader tracking/cleanup coordination APIs (cleanup is now orchestrator-driven).
src/Microsoft.ComponentDetection.Detectors/maven/IMavenCommandService.cs	Removes file-reader tracking/cleanup coordination API from the interface.

Copilot · 2026-03-20T22:57:06Z

test/Microsoft.ComponentDetection.Detectors.Tests/MavenWithFallbackDetectorTests.cs

+        stopwatch.Stop();
+
+        // Assert - Should complete quickly (well under 1 second for this simple test)
+        stopwatch.ElapsedMilliseconds.Should().BeLessThan(1000, "Smart loop prevention should not significantly impact performance");
+        scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);


The performance assertion based on a hard 1-second wall-clock threshold is likely to be flaky on slower/contended CI agents. Prefer an assertion that the code completes (no hang) or validate algorithmic behavior without relying on timing, or at least use a much looser threshold and isolate the test as non-gating if timing is required.

Copilot · 2026-03-20T22:57:06Z

src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs

+        // Clean up Maven CLI temporary files after all detectors have finished
+        await this.CleanupMavenFilesAsync(settings.SourceDirectory, detectors);


CleanupMavenFilesAsync is invoked unconditionally after detector execution. This will delete *.mvndeps even when the scan request has CleanupCreatedFiles=false, which breaks the contract implied by ScanSettings/ScanRequest and could remove user files unexpectedly. Gate this call (or the cleanup method) on settings.CleanupCreatedFiles being true.

Suggested change

// Clean up Maven CLI temporary files after all detectors have finished

await this.CleanupMavenFilesAsync(settings.SourceDirectory, detectors);

// Clean up Maven CLI temporary files after all detectors have finished,

// but only if cleanup of created files is enabled in settings.

if (settings.CleanupCreatedFiles)

{

await this.CleanupMavenFilesAsync(settings.SourceDirectory, detectors);

}

Copilot · 2026-03-20T22:57:06Z

src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs

+            var searchPattern = "*.mvndeps"; // Simple file pattern for Directory.GetFiles
+
+            this.logger.LogDebug("Starting Maven CLI cleanup in directory: {SourceDirectory}", sourceDirectory.FullName);
+
+            await Task.Run(() =>
+            {
+                // Search recursively for Maven CLI dependency files
+                var filesToClean = Directory.GetFiles(sourceDirectory.FullName, searchPattern, SearchOption.AllDirectories);
+


CleanupMavenFilesAsync deletes all files matching "*.mvndeps" using Directory.GetFiles(..., AllDirectories). In this repo MavenCommandService generates a specific filename ("bcde.mvndeps"), so using a broad glob risks deleting unrelated user files; additionally GetFiles materializes the full list and can be memory-heavy on large repos. Consider restricting to the known generated filename and using EnumerateFiles (optionally with cancellation) to stream deletions.

Copilot · 2026-03-20T22:57:06Z

src/Microsoft.ComponentDetection.Detectors/maven/MavenWithFallbackDetector.cs

@@ -118,6 +130,12 @@ public class MavenWithFallbackDetector : FileComponentDetector, IExperimentalDet
    private readonly ConcurrentQueue<string> mavenCliErrors = [];
    private readonly ConcurrentQueue<string> failedEndpoints = [];

+    /// <summary>
+    /// Cache for parent POM lookups to avoid repeated file system operations.
+    /// Key: current file path, Value: parent POM path or empty string if not found.
+    /// </summary>
+    private readonly ConcurrentDictionary<string, string> parentPomCache = new();
+


MavenWithFallbackDetector is registered as a singleton, but the newly added ConcurrentDictionary/ConcurrentQueue fields (collectedVariables, processedMavenProjects, mavenParentChildRelationships, parentPomCache, etc.) are never cleared between scans. This can cause memory growth and incorrect variable resolution due to stale state leaking across scans. Reset these collections (and related counters/flags) at the start of each scan (e.g., in OnPrepareDetectionAsync) and/or ensure they are per-scan locals.

grvillic · 2026-03-21T01:40:09Z

src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs

+
+            this.logger.LogDebug("Starting Maven CLI cleanup in directory: {SourceDirectory}", sourceDirectory.FullName);
+
+            await Task.Run(() =>


Unless I am missing something, we don't have anything async here. Why is it being wrapped with Task.Run? We will still be blocking a thread while waiting for the output, so it isn't scaling the actual work IIRC.

grvillic · 2026-03-21T01:54:52Z

src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs

+            await Task.Run(() =>
+            {
+                // Search recursively for Maven CLI dependency files
+                var filesToClean = Directory.GetFiles(sourceDirectory.FullName, searchPattern, SearchOption.AllDirectories);


Why not leveraging FileSystemEnumerable and Parallel.ForEach which can handle streaming and fewer allocations, see FastDirectoryWalkerFactory.cs. The longer the list of files to explore the slower this will be. It would be good to have a telemetry record to capture how long did we take doing the cleanup.

grvillic · 2026-03-21T01:56:45Z

src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs

+
+        try
+        {
+            var searchPattern = "*.mvndeps"; // Simple file pattern for Directory.GetFiles


Let's make this variable a private file-level constant, also we don't expect other file names, only bcde.mvndeps right? Why not only sticking to that pattern and create a public constant variable that can be shared between the detectors that use it, and this place so they are always in sync.

grvillic · 2026-03-21T02:00:16Z

src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs

+    private async Task CleanupMavenFilesAsync(DirectoryInfo sourceDirectory, IEnumerable<IComponentDetector> detectors)
+    {
+        // Only clean up if Maven detectors are running
+        var mavenDetectorIds = new[] { "MvnCli", "MavenWithFallback" };


Without reflection I think we can make another constant string shared between the detector IDs and here, so we are never out of sync.

zhenghao104 requested a review from a team as a code owner March 16, 2026 20:37

zhenghao104 requested review from Copilot and melotic March 16, 2026 20:37

Copilot started reviewing on behalf of zhenghao104 March 16, 2026 20:38 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

Process pom.xml in order and backfill any variable version

6f27926

zhenghao104 force-pushed the users/zhentan/more-maven-fix-v2 branch from 981dd8d to 6f27926 Compare March 16, 2026 23:52

FernandoRojo approved these changes Mar 17, 2026

View reviewed changes

Update cleanup method

2770c58

Copilot AI review requested due to automatic review settings March 17, 2026 23:48

Copilot started reviewing on behalf of zhenghao104 March 17, 2026 23:49 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

zhenghao104 added 2 commits March 17, 2026 18:27

Merge branch 'main' into users/zhentan/more-maven-fix-v2

4f9a665

Cleanup unused method

9bec9b4

Copilot AI review requested due to automatic review settings March 18, 2026 01:37

Copilot started reviewing on behalf of zhenghao104 March 18, 2026 01:38 View session

Copilot AI reviewed Mar 18, 2026

View reviewed changes

zhenghao104 added 2 commits March 17, 2026 19:55

Another edge case

b31cb46

Cleanup parent pom.xml logic

69e3307

Copilot AI review requested due to automatic review settings March 20, 2026 22:52

Copilot started reviewing on behalf of zhenghao104 March 20, 2026 22:53 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

grvillic reviewed Mar 21, 2026

View reviewed changes

		// Clean up Maven CLI temporary files after all detectors have finished
		await this.CleanupMavenFilesAsync(settings.SourceDirectory, detectors);


		this.logger.LogDebug("Starting Maven CLI cleanup in directory: {SourceDirectory}", sourceDirectory.FullName);

		await Task.Run(() =>

Conversation

zhenghao104 commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

grvillic Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

grvillic Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

grvillic Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

grvillic Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Mar 16, 2026 •

edited

Loading