Skip to content

MCP-684 Bound parallel organization entitlement checks - #728

Merged
nquinquenel merged 2 commits into
masterfrom
fix/nquinquenel/MCP-684-parallelEntitlementChecks
Nov 12, 2026
Merged

nquinquenel merged 2 commits into
masterfrom
fix/nquinquenel/MCP-684-parallelEntitlementChecks

Conversation

@nquinquenel

Copy link
Copy Markdown
Member

MCP-684 https://sonarsource.atlassian.net/browse/MCP-684

Human description

Cloud startup can wait for each accessible organization in sequence when no organization is configured. Check up to four organizations concurrently and select Vortex as soon as one qualifies. Continue through every organization when none qualifies. Active checks finish and populate the session cache.

Agent description

Approach: A dedicated executor bounds concurrent combined entitlement checks at four. Completion-order processing avoids waiting for a slow earlier organization. No count cap or startup deadline changes tool availability. Existing fixed-organization, force-local, IDE, Server, HTTP/HTTPS, and toolset gates remain unchanged. Active requests can continue after selection; failed checks remain retryable.

Acceptance coverage: Tests cover the concurrency bound, early success without waiting for active checks, a late entitled organization, all-negative coverage, cache reuse, and failed-check retries. A controlled test compares twelve delayed 100 ms responses and requires the parallel scan to take less than 75% of sequential scan time.

Vortex: Project binding and context/agentic entitlement are enabled. Vortex supplied resource and interrupt-handling guidance. DEEP analysis raised one new boolean-expression finding, which was fixed. No changed-line findings remain. Existing SonarQubeMcpServer.java:219 java:S106 remains: "Replace this use of System.out by a logger."

Independent review: Clean, round 1, gpt-6.1-sol, zero blocking findings. Runtime validation in the restricted reviewer environment was unavailable because its Gradle distribution could not be downloaded.

Validation gaps: Docker integration tests were not run because the Docker daemon is unavailable; CI must cover them. Independent reviewer host preparation was not validated because repository guidance supplies no preparation plan. Its Gradle test and Spotless experiment could not run because services.gradle.org was unavailable in the restricted environment. Host ./gradlew build spotlessCheck passed in 12m 39s, including the full unit suite.

Repository guidance warning: The guidance helper reported a missing ADR directory. Adding repository guidance or seeding records is outside this change. Decision record: none changed.

Attachments used: none.

Usage: gpt-6.1-sol, 13,586,487 tokens at description preparation; review usage 749,909 tokens in one session, included in accounting.

This PR was opened as a draft by the autonomous fabrik. It may become ready only after its mechanical gates pass. Human approval and merge remain required.


🤖 Co-authored by Fabrik

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

MCP-684

Co-Authored-By: Fabrik <fabrik@sonar.ai>
@nquinquenel
nquinquenel force-pushed the fix/nquinquenel/MCP-684-parallelEntitlementChecks branch from 561d309 to ce824a8 Compare November 11, 2026 14:36
@nquinquenel
nquinquenel marked this pull request as ready for review November 11, 2026 14:57
@nquinquenel
nquinquenel requested a review from a team as a code owner November 11, 2026 14:57

@vnaskos-sonar vnaskos-sonar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: MCP-684

It meets all of MCP-684's acceptance criteria, and I found nothing blocking. The FutureTask change, which stops network calls running under map locks, is a good fix.

Optional: OrgFeatureEntitlements.java:81 creates non-daemon threads (I checked). If the process exits while leftover checks are running, the JVM waits for them, bounded by the HTTP timeouts. Adding .daemon(true) is a one-line fix.

@sonarqubecloud

Copy link
Copy Markdown

@gitar-bot

gitar-bot Bot commented Nov 12, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 🟡 Medium risk 2 closed / 2 findings

Parallelizing organization entitlement checks changes startup feature selection; a concurrent check failure could abort scanning before an eligible organization is discovered.

Bounds the no-organization cloud startup entitlement scan to four concurrent checks, returning as soon as one organization qualifies. Resolved the ConcurrentHashMap computeIfAbsent serialization of parallel checks and the failure to cancel in-flight worker checks on interrupt during shutdown. No open issues found.

✅ 2 closed
✅ Performance: Parallel checks still serialize on ConcurrentHashMap.computeIfAbsent locks

📄 src/main/java/org/sonarsource/sonarqube/mcp/serverapi/OrgFeatureEntitlements.java:59 📄 src/main/java/org/sonarsource/sonarqube/mcp/serverapi/OrgFeatureEntitlements.java:62-76
isVortexEnabledForAnyOrganization runs isVortexEnabledForOrg on four threads. Each of those calls runs checkCloudVortexEntitlement, which does the CAG and A3S HTTP round-trips, inside vortexEntitlements.computeIfAbsent (line 59). ConcurrentHashMap keeps the bin locked for as long as the mapping function runs, and its Javadoc says other updates may block during that time, so the computation should be short. With the default 16-bin table, four random org keys share a bin about a third of the time, and those checks then run one after another. Once more than 12 results are cached (negative results are stored), a resize also has to wait on every bin with a request in flight. Before this PR the scan was sequential, so this lock cost nothing. Now it eats into the speedup this PR is meant to deliver, and the test's 75% threshold won't catch it. Fix: do the network check outside computeIfAbsent, for example by caching a CompletableFuture/FutureTask per key and removing it on a null or failed result, or by checking and then calling putIfAbsent.

✅ Edge Case: Interrupt during shutdown does not cancel in-flight worker checks

📄 src/main/java/org/sonarsource/sonarqube/mcp/serverapi/OrgFeatureEntitlements.java:63 📄 src/main/java/org/sonarsource/sonarqube/mcp/serverapi/OrgFeatureEntitlements.java:85-93
stopBackgroundInitialization stops startup by interrupting the mcp-background-init thread. Before this PR, that interrupt reached the thread that was making the entitlement HTTP calls. Now the init thread is only waiting in completions.take(). Its InterruptedException handler rethrows, and the finally block calls executor.shutdown(), which does not interrupt running tasks. The worker threads inherit the non-daemon status of mcp-background-init, so up to four of them keep making HTTP requests while shutdownHttpClient is closing the client, and they can keep the JVM alive until those requests finish or time out. Fix: call executor.shutdownNow() on the interrupt and failure paths, and keep shutdown() only for the normal return where in-flight checks are meant to fill the cache.

Verification Rules 2/2 met

Functional validation No results

Rules 2 passed 2 not applicable

✅ Internal artifact registries
✅ Repository license compliance

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@nquinquenel
nquinquenel merged commit 8dc1b6f into master Nov 12, 2026
13 checks passed
@nquinquenel
nquinquenel deleted the fix/nquinquenel/MCP-684-parallelEntitlementChecks branch November 12, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants