Fix Java starter JDK and transport reliability - #19635
Open
Adam Ratzman (adamint) wants to merge 8 commits into
Open
Fix Java starter JDK and transport reliability#19635Adam Ratzman (adamint) wants to merge 8 commits into
Adam Ratzman (adamint) wants to merge 8 commits into
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19635Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19635" |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Java 25 toolchain selection, Java AppHost project modeling, and generated Java JSON-RPC transport reliability.
Changes:
- Resolves Java commands from the effective child environment and improves Maven/JDK handling.
- Adds Java transport concurrency, cancellation, array, and callback write-back support.
- Improves VS Code Java debugging, parsing, starter project modeling, and E2E coverage.
Show a summary per file
| File | Description |
|---|---|
tests/Aspire.Hosting.Java.Tests/AddJavaAppTests.cs |
Tests Java executable debug serialization. |
tests/Aspire.Hosting.Java.Tests/AddJavaAppPublishTests.cs |
Tests Maven profile and Gradle version detection. |
tests/Aspire.Hosting.CodeGeneration.Java.Tests/Snapshots/AtsGeneratedAspire.verified.java |
Updates generated Java snapshot. |
tests/Aspire.Hosting.CodeGeneration.Java.Tests/JavaLanguageSupportTests.cs |
Tests release and build outputs. |
tests/Aspire.Cli.Tests/Templating/TemplateGitIgnoreTests.cs |
Verifies frontend ignore rules. |
tests/Aspire.Cli.Tests/Templating/JavaStarterScaffoldTests.cs |
Verifies starter JDT metadata. |
tests/Aspire.Cli.Tests/Projects/ProcessGuestLauncherTests.cs |
Updates launcher construction. |
tests/Aspire.Cli.Tests/Projects/JavaAppHostToolchainResolverTests.cs |
Tests reactor and wrapper handling. |
tests/Aspire.Cli.Tests/Projects/GuestRuntimeTests.cs |
Tests effective PATH and build outputs. |
tests/Aspire.Cli.Tests/Projects/ExtensionGuestLauncherTests.cs |
Tests extension command resolution. |
src/Aspire.TypeSystem/RuntimeSpec.cs |
Adds required build outputs. |
src/Aspire.Hosting.Java/JavaVersionDetector.cs |
Revises Maven and Gradle detection. |
src/Aspire.Hosting.Java/JavaLaunchConfiguration.cs |
Adds selected Java executable. |
src/Aspire.Hosting.Java/JavaHostingExtensions.cs |
Resolves IDE Java executable. |
src/Aspire.Hosting.CodeGeneration.Java/Resources/Transport.java |
Reworks Java JSON-RPC transport. |
src/Aspire.Hosting.CodeGeneration.Java/Resources/Base.java |
Exposes generated union APIs. |
src/Aspire.Hosting.CodeGeneration.Java/JavaLanguageSupport.cs |
Models compiler outputs compatibly. |
src/Aspire.Hosting.CodeGeneration.Java/AtsJavaCodeGenerator.cs |
Adds arrays, nullability, and write-back. |
src/Aspire.Cli/Templating/Templates/java-starter/frontend/package.json |
Pins corrected nanoid. |
src/Aspire.Cli/Templating/Templates/java-starter/frontend/package-lock.json |
Updates locked nanoid. |
src/Aspire.Cli/Templating/Templates/java-starter/api/pom.xml |
Targets Java 25. |
src/Aspire.Cli/Templating/Templates/java-starter/.settings/org.eclipse.jdt.core.prefs |
Configures JDT for Java 25. |
src/Aspire.Cli/Templating/Templates/java-starter/.project |
Defines the root JDT project. |
src/Aspire.Cli/Templating/Templates/java-starter/.gitignore |
Ignores frontend artifacts. |
src/Aspire.Cli/Templating/Templates/java-starter/.classpath |
Models AppHost and generated SDK sources. |
src/Aspire.Cli/Projects/ProcessGuestLauncher.cs |
Resolves commands from child environment. |
src/Aspire.Cli/Projects/JavaAppHostToolchainResolver.cs |
Handles Maven reactors and wrappers. |
src/Aspire.Cli/Projects/GuestRuntime.cs |
Checks outputs and removes injected resolution. |
src/Aspire.Cli/Projects/GuestAppHostProject.cs |
Uses the revised runtime constructor. |
src/Aspire.Cli/Projects/ExtensionGuestLauncher.cs |
Passes resolved commands to VS Code. |
extension/src/types/extensionApi.ts |
Adds Java E2E bridge commands. |
extension/src/testing/e2eStateFileBridge.ts |
Implements definition/project queries. |
extension/src/test/packageManifest.test.ts |
Covers Java Explorer actions. |
extension/src/test/javaDebugger.test.ts |
Tests executable routing and parsing. |
extension/src/test/javaAppHostParser.test.ts |
Covers nested and invalid main methods. |
extension/src/test-e2e/javaStarterProjectModel.e2e.test.ts |
Validates starter JDT project structure. |
extension/src/test-e2e/helpers/java.ts |
Adds starter workspace helpers. |
extension/src/editor/parsers/javaAppHostParser.ts |
Implements Java 25 main selection. |
extension/src/debugger/languages/java.ts |
Routes selected JVM to debugger. |
extension/src/debugger/AspireDebugSession.ts |
Carries and redacts Java executable paths. |
extension/src/dcp/types.ts |
Extends Java launch contract. |
extension/scripts/run-e2e.js |
Generates starter-specific E2E workspaces. |
extension/package.json |
Enables Java AppHost context actions. |
.github/workflows/extension-e2e-tests.yml |
Adds Java starter E2E shard. |
Review details
Files not reviewed (1)
- src/Aspire.Cli/Templating/Templates/java-starter/frontend/package-lock.json: Generated file
- Files reviewed: 44/46 changed files
- Comments generated: 5
- Review effort level: Balanced
…arter-jdk-25 # Conflicts: # extension/src/testing/e2eStateFileBridge.ts # extension/src/types/extensionApi.ts
Contributor
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Cli/Templating/Templates/java-starter/frontend/package-lock.json: Generated file
Suppressed comments (3)
src/Aspire.Hosting.CodeGeneration.Java/Resources/Transport.java:933
- Cancellation can be sent before the capability request has registered this ID on the host. This happens deterministically for an already-cancelled token and can also race with an immediate
cancel():onCancelqueuescancelTokenwhile argument marshalling is still in progress, and the host currently drops unknown IDs. The later capability invocation then creates an uncancelled token and may run indefinitely. Defer cancellation transmission until the invocation is registered, or make the host retain early cancellations.
token.onCancel(listener);
cancellationRegistrations.put(id, () -> token.removeCancelListener(listener));
src/Aspire.Hosting.CodeGeneration.Java/Resources/Transport.java:169
onCancelcan invoke the same listener twice when registration races withcancel(): the cancellation thread can observe the newly added listener and invoke it, while this thread then observescancelled == trueand invokes it again.CopyOnWriteArrayListdoes not make this add/check sequence atomic. Synchronize listener registration with the cancellation transition or wrap each registration with an exactly-once guard.
public void onCancel(Runnable listener) {
listeners.add(listener);
if (cancelled.get()) {
listener.run();
src/Aspire.Hosting.Java/JavaVersionDetector.cs:232
- Selecting the textually last match conflates independent Gradle settings.
sourceCompatibilityandtargetCompatibilityare not reassignments of one value; for example,targetCompatibility = 21followed bysourceCompatibility = 17emits Java 21 bytecode, but this code detects 17 and publishes a runtime that fails withUnsupportedClassVersionError. Track the final assignment per setting and give the emitted target/toolchain safe precedence.
- Files reviewed: 44/46 changed files
- Comments generated: 2
- Review effort level: Balanced
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Provision Java 21 for test projects that declare the requirement, make the profiling assertion match the resolved executable name, and document the post-push CI and Copilot review convergence loop. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b812ecf-6528-4af7-91d9-4bd8410efe30
Contributor
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Cli/Templating/Templates/java-starter/frontend/package-lock.json: Generated file
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/Aspire.Hosting.CodeGeneration.Java/AtsJavaCodeGenerator.cs:1818
- This migration only updates generated capability methods. The hand-authored
ReferenceExpression.getValue(CancellationToken)still callsclient.registerCancellation(...)beforeinvokeCapability, so an already-cancelled token sendscancelTokenbefore the remote invocation exists and its registration is never removed afterward. Pass the token object throughreqArgsthere as well so this transport-owned ordering and cleanup applies consistently.
src/Aspire.Hosting.CodeGeneration.Java/Resources/Transport.java:375 - Marshalling happens before the
try/finally. If one argument registers a cancellation token and serialization of a later argument throws, the request is never sent but the registration and token listener remain live; cancelling it later can emitcancelTokenfor a nonexistent invocation. Include marshalling in the cleanup boundary so every ID accumulated before a marshalling failure is unregistered.
- Files reviewed: 62/65 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b812ecf-6528-4af7-91d9-4bd8410efe30
Contributor
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Cli/Templating/Templates/java-starter/frontend/package-lock.json: Generated file
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/Aspire.Cli/Projects/ProcessGuestLauncher.cs:51
- This copy restores a case-sensitive comparer. If the merged environment contains both
PATHandPath(for example, inheritedPATHplus a launch-profilePathoverride),TryResolveExecutablePathprefers the exactPATHentry while the Windows child environment applies the later case-insensitive override. The executable can therefore be resolved from a different PATH than the process receives. Normalize with the repository'sProcessEnvironment.Comparerwhile copying so resolution and launch use the same effective value.
var effectiveEnvironmentVariables = environmentVariables.ToDictionary();
ProfilingTelemetry.AddActivityContextToEnvironment(activity, effectiveEnvironmentVariables);
src/Aspire.Cli/Projects/ExtensionGuestLauncher.cs:44
- This copy restores a case-sensitive comparer. If the merged environment contains both
PATHandPath,TryResolveExecutablePathprefers the exactPATHentry while the extension/Windows process environment applies names case-insensitively, so the debugger can receive a launcher resolved from the stale path. Normalize with the repository'sProcessEnvironment.Comparerwhile copying.
var effectiveEnvironmentVariables = environmentVariables.ToDictionary();
ProfilingTelemetry.AddActivityContextToEnvironment(Activity.Current, effectiveEnvironmentVariables);
- Files reviewed: 62/65 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b812ecf-6528-4af7-91d9-4bd8410efe30
Comment on lines
+149
to
+150
| var environmentVariables = command.EnvironmentVariables ?? new Dictionary<string, string>(); | ||
|
|
Comment on lines
+333
to
+335
| const normalizedExecutable = executable.toLowerCase().replace(/\.(exe|com|bat|cmd)$/, ''); | ||
| const isBareJava = args[0].toLowerCase() === 'java'; | ||
| if (normalizedExecutable !== 'java' || (!isBareJava && !isAbsolutePath(args[0]))) { |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
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
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.
Fixes the Java starter using Java 21 from the ambient process when the modeled toolchain is Java 25, and tightens the surrounding Java AppHost path.
This makes the child process environment authoritative, resolves Maven wrappers and declared reactor modules correctly, and models the starter root plus generated SDK as a JDT project. It also fixes Java JSON-RPC response routing, callback reentrancy, cancellation/disconnect races, exact response IDs, typed/primitive arrays, nullable primitives, and callback write-back.
The VS Code parser now keeps the enclosing AppHost entry point when nested classes have their own
main, and the starter E2E verifies Red Hat Java sees the root AppHost/generated SDK and nested Maven API as the intended projects. The frontend lockfile also pins the fixednanoidversion.Verified with the generated Java suite (62), changed CLI suites (110), Java hosting suite (309), extension unit suite (2540), TypeScript compile/bundle/E2E compile/lint, the focused Java starter Extension Host E2E, Maven package, and a clean frontend install/audit/lint/build.