[release/13.2] Backport: Fix 13.2 IDE execution regressions for Azure Functions and class library projects (#15714)#15763
Conversation
… Functions and class library projects (microsoft#15714)
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15763Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15763" |
There was a problem hiding this comment.
Pull request overview
Backport to release/13.2 that restores IDE execution/debugging behavior for project resources (including custom debug types like Azure Functions and class-library-based AWS Lambda) that regressed in Aspire 13.2, across both Aspire.Hosting and the VS Code extension.
Changes:
- Aspire.Hosting: treat
"project"launch configuration support as implicit and add a debug-session fallback that emitsProjectLaunchConfigurationfor IDE execution when annotations/debug session info are missing. - VS Code extension: add support for
commandName: "Executable"launch profiles, expand$(VAR)/%VAR%in relevant paths/args, and make commandName comparisons case-insensitive. - Add/expand unit tests covering VS and VS Code debug session scenarios and launch profile behaviors.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs | Adds coverage for implicit "project" support and debug-session fallback behaviors (VS + VS Code scenarios). |
| src/Aspire.Hosting/Utils/ExtensionUtils.cs | Makes "project" support implicit per spec; custom launch types still require explicit advertisement. |
| src/Aspire.Hosting/Dcp/DcpExecutor.cs | Adds debug-session fallback to IDE execution and refactors creation of ProjectLaunchConfiguration. |
| extension/src/test/launchProfiles.test.ts | Adds tests for env-var expansion and working directory resolution. |
| extension/src/test/dotnetDebugger.test.ts | Adds tests for Executable launch profile handling and env-var expansion in executable/args. |
| extension/src/debugger/launchProfiles.ts | Implements env-var expansion, case-insensitive commandName handling, and working directory expansion. |
| extension/src/debugger/languages/dotnet.ts | Uses executable launch profiles by launching executablePath + expanded args and building the project. |
| extension/src/dcp/AspireDcpServer.ts | Preserves “args absent vs empty” semantics by passing payload.args through unchanged. |
| /// <see cref="SupportsDebuggingAnnotation"/> (e.g. AddResource-based subclasses) or the | ||
| /// IDE did not send <c>DEBUG_SESSION_INFO</c> (Visual Studio scenario). | ||
| /// </summary> | ||
| private bool ShouldFallBackToIdeExecution(bool isInDebugSession, SupportsDebuggingAnnotation? supportsDebuggingAnnotation) |
There was a problem hiding this comment.
Method name ShouldFallBackToIdeExecution uses the uncommon FallBack casing, which is inconsistent with the rest of the codebase’s usage of “Fallback” (e.g., FallbackExecutionTypes). Consider renaming to ShouldFallbackToIdeExecution (and updating the call site) for consistency/searchability.
| private bool ShouldFallBackToIdeExecution(bool isInDebugSession, SupportsDebuggingAnnotation? supportsDebuggingAnnotation) | |
| private bool ShouldFallbackToIdeExecution(bool isInDebugSession, SupportsDebuggingAnnotation? supportsDebuggingAnnotation) |
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
|
We discussed this one offline. Marking as servicing approved as Adam confirmed all scenarios were passing and there was manual validation done to confirm it. I'll wait on @adamint to let me know when this is ready to merge as he mentioned he wanted to get additional reviews here. |
With Norm’s confirmation, I’m confident about merging |
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
LGTM. Two minor nits left as comments — neither is blocking for this backport. Consider addressing them when porting to main.
| return result; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Nit: The two regex replacements run sequentially on the same string, so if an env var's value contains %SOMETHING% syntax, the second pass will inadvertently expand it (or collapse it to empty). For example, if MY_VAR = %USERPROFILE%\tools and input is $(MY_VAR)/bin, the first pass yields %USERPROFILE%\tools/bin, then the second pass expands %USERPROFILE% again. Unlikely in practice, but differs from VS's native single-pass expansion.
Not a blocker for this backport.
| if (debugConfiguration.args) { | ||
| debugConfiguration.args = expandEnvironmentVariables(debugConfiguration.args); | ||
| } else if (baseProfile.commandLineArgs) { | ||
| // Fall back to launch profile args if run session args were empty |
There was a problem hiding this comment.
Minor: debugConfiguration.args was set by determineArguments() above. If the run session sends an explicit empty args array [], determineArguments returns "" (joined empty array). Since "" is falsy, this else if branch would override the explicit empty args with baseProfile.commandLineArgs, breaking the contract that explicit run session args (even empty) take precedence over profile args.
In practice this is mitigated by the payload.args change in AspireDcpServer.ts (absent args are now undefined, not []), so the edge case is unlikely to trigger. Consider using debugConfiguration.args !== undefined && debugConfiguration.args !== null instead of truthiness if porting to main.
Not a blocker for this backport.
joperezr
left a comment
joperezr
left a comment
There was a problem hiding this comment.
LGTM — well-scoped regression fix for 4 real user-facing bugs on a servicing branch. The behavior changes are intentional, well-tested, and manually validated across VS, VS Code, and CLI.
Inline comments are all non-blocking suggestions to consider for the main branch. This PR can be merged as-is.
| // Expand $(VAR) syntax (used by VS and MSBuild-style launch profiles) | ||
| let result = value.replace(/\$\(([^)]+)\)/g, (_, varName) => process.env[varName] ?? ''); | ||
| // Expand %VAR% syntax (Windows) |
There was a problem hiding this comment.
NIT (non-blocking, consider for main): The regex ([^)]+) is fairly permissive — it matches any characters except ). A stricter pattern like ([A-Za-z_][A-Za-z0-9_]*) would only match valid env var identifiers, reducing risk of unexpected matching on edge cases. Additionally, when a variable is not found, it's silently replaced with an empty string, which could mask typos in launch profiles (e.g., $(TYPO_VARNAME) → empty). A debug-level log warning for unresolved variables would help troubleshooting.
| [KnownConfigNames.DebugSessionInfo] = JsonSerializer.Serialize(runSessionInfo), | ||
| [KnownConfigNames.ExtensionEndpoint] = "http://localhost:1234" | ||
| }; | ||
|
|
There was a problem hiding this comment.
NIT (non-blocking, consider for main): Several of the VS fallback tests (e.g., StandardAndCustomProjects_VSScenario_BothRunInIde) verify ExecutionType.IDE is set but don't assert that the custom-typed project (e.g., "azure-functions") actually falls back to a ProjectLaunchConfiguration with Type = "project". Without this assertion, a future regression could accidentally use the annotation's custom ExecutableLaunchConfiguration in the fallback path and the tests wouldn't catch it. Consider adding Assert.Equal("project", launchConfigs[0].Type) for the custom project in these tests.
| /// pass <see cref="ExtensionUtils.SupportsDebugging"/>. Returns <see langword="true"/> when | ||
| /// the app host is running inside a debug session and either the resource has no | ||
| /// <see cref="SupportsDebuggingAnnotation"/> (e.g. AddResource-based subclasses) or the | ||
| /// IDE did not send <c>DEBUG_SESSION_INFO</c> (Visual Studio scenario). | ||
| /// </summary> | ||
| private bool ShouldFallBackToIdeExecution(bool isInDebugSession, SupportsDebuggingAnnotation? supportsDebuggingAnnotation) | ||
| { | ||
| if (!isInDebugSession) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| if (supportsDebuggingAnnotation is not null && !string.IsNullOrEmpty(_configuration[KnownConfigNames.DebugSessionInfo])) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
NIT (non-blocking, consider for main): ShouldFallBackToIdeExecution merges two logically distinct scenarios: (1) no annotation exists (AddResource case), and (2) annotation exists but no DebugSessionInfo (VS case). The method name leans toward the VS fallback scenario. Additionally, if DebugSessionInfo JSON parsing fails (exception caught in GetSupportedLaunchConfigurations), it returns null and this method treats that the same as the VS case — correct behavior, but worth a brief inline comment for future readers.
| env | ||
| )); |
There was a problem hiding this comment.
NIT (non-blocking, consider for main): The }\nelse if brace placement here differs from the typical TypeScript convention of } else if on the same line. Minor style inconsistency.
| }); | ||
|
|
||
| test('expands HOME variable like AWS Lambda launch profiles use', () => { | ||
| const home = process.env['HOME'] ?? ''; | ||
| const input = '$(HOME)/.dotnet/tools/.store/amazon.lambda.testtool/0.13.0/content/RuntimeSupport.dll'; |
There was a problem hiding this comment.
NIT (non-blocking, consider for main): This test relies on process.env['HOME'], which may not be set on Windows CI (where USERPROFILE is used instead). On Windows, HOME could be undefined, causing the assertion to compare against empty string — the test would still pass but wouldn't be exercising real expansion. Consider using a guaranteed-present env var like PATH, or setting a custom test var with try/finally cleanup.
Description
Backport of #15714 to
release/13.2.Fixes regressions introduced in Aspire 13.2 where project resources using custom debug types (Azure Functions, AWS Lambda class libraries) or added via
AddResourcewithoutWithDebugSupportno longer received IDE execution in Visual Studio or VS Code, breaking debugging and symbol loading.Changes backported
Aspire.Hosting (C#):
DcpExecutor.PrepareProjectExecutables(): Adds a fallback branch that provides IDE execution with a standardProjectLaunchConfigurationwhen in a debug session but either noSupportsDebuggingAnnotationexists (e.g.AddResource-basedProjectResourcesubclasses) or the IDE did not sendDEBUG_SESSION_INFO(Visual Studio scenario where VS handles all project types natively). Restores pre-13.2 behavior.ExtensionUtils.SupportsDebugging(): Makes"project"launch configuration support implicit per DCP spec, so standard projects always get IDE execution even when the extension only advertises custom launch types.VS Code Extension (TypeScript):
Executablecommand launch profiles used by AWS Lambda. When the launch profile specifiescommandName: "Executable"with anexecutablePath, the extension builds the project and uses the profile's executable path as the debug program instead of the project output DLL.expandEnvironmentVariables()to handle$(VAR)and%VAR%syntax in launch profile paths (e.g.,$(HOME)used by AWS Lambda tool paths).commandNamecomparison case-insensitive.determineWorkingDirectory.Only files necessary for the fix are included — playground apps from the original PR are excluded.
Merge conflicts resolved
No merge conflicts — the changes applied cleanly to
release/13.2.Validation
Fixes #15606
Fixes #15647
Fixes #15378
Fixes #15699
Checklist