Skip to content

Add VS Code resource debugging for .NET and Go - #18602

Draft
Adam Ratzman (adamint) wants to merge 102 commits into
microsoft:mainfrom
adamint:adamint/issue18538-vscode-debug-shortcut
Draft

Add VS Code resource debugging for .NET and Go#18602
Adam Ratzman (adamint) wants to merge 102 commits into
microsoft:mainfrom
adamint:adamint/issue18538-vscode-debug-shortcut

Conversation

@adamint

@adamint Adam Ratzman (adamint) commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

Adds resource debugger attachment for running .NET and Go resources from the Aspire view and the aspire_resource_debug Copilot tool.

Fixes #18538.
Fixes #18937.

What changed

  • Adds a shared resource-debug service, provider registry, session registry, and safe result contract for tree and language-model callers.
  • Adds .NET and Go attach providers.
  • Resolves the launched application instead of attaching to dotnet run, dotnet watch, or go run. Process discovery supports Windows, macOS, and Linux.
  • Uses kernel-backed executable identity on macOS and exact executable identity on every platform.
  • Integrates Expose a project resource child PID or evaluated AssemblyName for debugger attach #18937's launcher-child behavior: dotnet run requires the direct application child, while dotnet watch and go run support verified descendants.
  • Preserves the selected AppHost PID through fresh resolution, serialization, CLI capability checks, and session tracking so concurrent runs from the same AppHost path are not conflated.
  • Detects resource processes already owned by an Aspire-launched VS Code debug session and returns alreadyDebugging.
  • Publishes non-sensitive project.launchCommand, project.configuration, and project.targetFramework snapshot metadata. Current snapshots use exact TargetPath identity; the TargetName compatibility fallback is limited to older snapshots that do not contain the new metadata.
  • Adds classified resource-debug telemetry without resource names, paths, PIDs, arguments, environment values, or raw failures.
  • Adds tree enablement, progress, cancellation, debugger-extension guidance, and localized outcomes.
  • Adds aspire_resource_debug with input validation, explicit confirmation, invocation-time re-resolution, linked cancellation, and bounded output.
  • Adds packaged VSIX E2E coverage for package contributions, resource-debug tool behavior, privacy boundaries, edge cases, and real .NET and Go attach sessions.

Evidence

Real process-tree validation covers:

  • dotnet run: the application is a direct child.
  • dotnet watch: the application is a transitive descendant.
  • go run: the application is a descendant.
  • Exact .NET target selection with same-name decoys and non-default assembly names.
  • Kernel-backed macOS executable lookup for long names.
  • Go/Delve attach without trusting flattened command text.

The packaged Linux VSIX E2E shard installs pinned C# and Go debugger extensions plus Delve. It invokes the registered resource-debug tool for both languages, binds a source breakpoint, observes the matching stack frame, detaches, verifies the resource still responds, and tears down the AppHost.

Validation

extension compile, lint, and unit tests
  2943 passing
  5 pending

focused debugger unit tests
  866 passing

Aspire.Cli.Tests
  146 total
  141 passed
  5 skipped (platform-specific)

ResourceSnapshotBuilderTests
  25 passed

ExtensionE2eWorkflowTests + TestTriggerMapTests
  38 passed

extension production package
  succeeded

resource-debug-tools packaged VSIX E2E
  passed on Linux and Windows CI

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
    • No

The language-model boundary intentionally excludes process IDs, absolute paths, arguments, environment values, and raw failures. Debug attachment also requires VS Code confirmation before invocation.

Copilot AI balanced review requested due to automatic review settings July 1, 2026 20:16
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18602

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18602"

Copilot AI 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.

Pull request overview

This PR adds the first VS Code extension lane for #18538: a right-click Attach debugger action on running .NET project resources in the Aspire panel. It builds a CoreCLR attach debug configuration that targets the project's process by processName (best-effort resolving <AssemblyName> from the project file), rather than executable.pid, because DCP reports the dotnet run launcher PID rather than the child app process. The action is gated on a running top-level Project resource with a dotnet executable and a .csproj/.fsproj/.vbproj path, and only when the C# extension is installed; it re-resolves the latest resource before attaching so stale/ambiguous/missing resources fail closed with a warning.

Bundled alongside the feature is a broad telemetry overhaul: every event is renamed to its final wire name (aspire/vscode/... and aspire/dashboard/...) and routed through sendDangerousTelemetryEvent/sendDangerousTelemetryErrorEvent to bypass VS Code's automatic <extensionId>/ prefix, with manual opt-in gating (telemetryLevel), manual common properties, and explicit value sanitization.

Changes:

  • Add aspire-vscode.attachDebuggerToResource command, menu contribution (canAttachDebugger context value), attachability helpers, and the attach execution flow with latest-resource re-resolution.
  • Rename all telemetry events to final wire names and route them through the "dangerous" send path with explicit gating/sanitization and manually-added common properties.
  • Add localized strings and extensive unit tests for both the attach flow and the telemetry changes.
Show a summary per file
File Description
extension/src/views/AspireAppHostTreeProvider.ts Core attach logic: attachability helpers, process-name derivation, attachDebuggerToResource, latest-resource resolution; propagates appHostPath through tree items.
extension/src/extension.ts Registers the new attach command; renames the extension/activated telemetry event.
extension/package.json Contributes the command, hidden palette entry, and view/item/context menu item; reorders action groups.
extension/package.nls.json / extension/src/loc/strings.ts / extension/loc/xlf/aspire-vscode.xlf Adds/regenerates localized strings for the attach action.
extension/src/utils/telemetry.ts Switches to dangerous-send path, adds manual gating, common properties, and value sanitization; hard-codes telemetry client version.
extension/src/utils/telemetryRegistry.ts Renames event schema keys to final aspire/vscode/... and aspire/dashboard/... wire names; documents the convention.
extension/telemetry.json Updates classification inventory keys to the new wire names.
extension/src/dcp/DashboardTelemetryPassthrough.ts / AspireDcpServer.ts / AspireDebugSession.ts / AppHostLaunchService.ts / AppHostsViewTelemetry.ts / meaningfulEngagement.ts / appHostDiscovery.ts / workspace.ts / interactionService.ts Update event-name call sites and passthrough comments to the new namespaces.
extension/src/test/*.test.ts, extension/src/test-e2e/packageSurface.e2e.test.ts Extensive tests for the attach flow, telemetry gating/sanitization/naming, and the manifest/surface contributions.

Review details

  • Files reviewed: 29/29 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread extension/src/utils/telemetryRegistry.ts Outdated
Comment thread extension/src/test/appHostTreeView.test.ts Outdated
Comment thread extension/src/utils/telemetry.ts Outdated
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

Copilot AI review requested due to automatic review settings July 1, 2026 23:36

Copilot AI 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 details

  • Files reviewed: 30/30 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread extension/src/utils/telemetry.ts Outdated
Copilot AI review requested due to automatic review settings July 1, 2026 23:45

Copilot AI 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 details

  • Files reviewed: 30/30 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread extension/src/dcp/AspireDcpServer.ts Outdated
@adamint

Adam Ratzman (adamint) commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

I tried to place these as inline tracking comments, but GitHub rejected new inline comments because there is already a pending review draft for this user on the PR. I kept the tracking here instead while the branch was fixed:

  1. The handled attach guard failures in attachDebuggerToResource should return a failure result for command telemetry instead of plain void, otherwise withCommandTelemetry records user-visible attach failures as successful invocations.
  2. The home-directory sanitizer needed coverage and a fix for usernames with spaces; paths like /Users/Alice Smith/project or C:\Users\Alice Smith\project could leave part of the username in telemetry after the dangerous-send path.

These were fixed before the later Windows punctuation redaction follow-up. This is a top-level tracking comment, so there is no review thread to resolve; final tracking is in #18602 (comment).

@adamint

Adam Ratzman (adamint) commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Pushed 8f4fe85 with the Windows telemetry redaction fix. Windows CI caught that the home-directory sanitizer could cross punctuation/text between two C:\Users\... paths; the parser now keeps punctuation boundaries intact and the regression is covered.

Validation on the live head passed locally: compile-tests, lint, compile, and full telemetry.test (31 passing). gh pr checks is green on 8f4fe85 (328 success / 0 pending / 3 skipped / 0 failed), and the review/test fleet found no blocking agent-owned follow-up.

Remaining non-agent gates: live CoreCLR attach/manual readiness pass, plus telemetry/reviewer sign-off for the common.* properties and dangerous-send sanitizer parity.

Copilot AI review requested due to automatic review settings July 2, 2026 20:14

Copilot AI 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 details

  • Files reviewed: 30/30 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 3, 2026 02:09

Copilot AI 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 details

  • Files reviewed: 30/30 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 3, 2026 02:46

Copilot AI 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 details

  • Files reviewed: 30/30 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 4, 2026 15:42

Copilot AI 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 details

  • Files reviewed: 30/30 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 4, 2026 16:36

Copilot AI 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 details

  • Files reviewed: 30/30 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7c01c030-226e-41c5-81ca-a6483a1856df

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Cross-platform process attachment and security-sensitive metadata handling warrant final human review despite extensive coverage.

Review tier: Balanced
Findings: 1 High severity · 4 Medium severity

Pre-existing issues (5)
Severity Finding
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment
Issues resolved since last review (2)
Severity Finding
Low severity extension/​scripts/​run-e2e.js — If this new resource-debug dependency is missing, resolveRequiredVsixPath reports that it is… View resolved comment
High severity src/​Aspire.Hosting/​Dcp/​ResourceSnapshotBuilder.cs — These values are copied from EffectiveArgs without consulting the corresponding… View resolved comment

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7c01c030-226e-41c5-81ca-a6483a1856df

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Tree attachment crosses Restricted Mode’s trust boundary and disabled debugger extensions lack actionable enablement guidance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2 High severity · 4 Medium severity

New issues introduced by this change (1)
Severity Finding
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The…
Pre-existing issues (5)
Severity Finding
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment
Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

extension/src/views/AspireAppHostTreeProvider.ts:1565

  • This does not handle the disabled-extension case required by the linked completion criteria. A disabled C# or Go extension is reported as missing, but the message only says to install it; reinstalling a disabled extension is a no-op, so the user remains blocked with no Enable/Open Extensions action. Route this outcome through the existing DebuggerInstallHintService behavior (including its disabled-extension fallback) or provide equivalent install/enable actions here.

Comment thread extension/src/views/AspireAppHostTreeProvider.ts
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
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: 7c01c030-226e-41c5-81ca-a6483a1856df

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Target-affecting .NET launch options can break process resolution, and disabled debugger extensions receive incomplete setup guidance.

Review tier: Balanced
Findings: 2 High severity · 4 Medium severity

Pre-existing issues (6)
Severity Finding
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The… View comment
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment
Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

extension/src/views/AspireAppHostTreeProvider.ts:1564

  • A debugger extension absent from the VS Code registry may be installed but disabled, yet this path always tells the user to install it and offers no setup action. That does not provide the install-or-enable guidance required by the linked completion criteria. Route this outcome through DebuggerInstallHintService (which already installs missing extensions and opens Extensions for disabled ones) instead of showing this inert install-only warning.
    src/Aspire.Hosting/Dcp/ResourceSnapshotBuilder.cs:301
  • This only preserves configuration/framework, but other valid dotnet run switches also change the evaluated output identity. For example, --runtime linux-x64 (and the --arch/--os aliases or -p: properties) can change TargetPath and UseAppHost; the resource then launches the RID-specific child, while getDotNetAttachTargetInfo reevaluates MSBuild without that state and searches for a different executable. Attaching therefore fails for customized launch recipes. Preserve the complete target-affecting identity—preferably the actual launched child PID/path—or publish and pass all target-affecting options during reevaluation.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
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: 7c01c030-226e-41c5-81ca-a6483a1856df
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7c01c030-226e-41c5-81ca-a6483a1856df

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Cross-platform process identity and debugger lifecycle behavior require final human validation of the pending packaged Linux E2E shard.

Review tier: Balanced
Findings: 2 High severity · 4 Medium severity

Pre-existing issues (6)
Severity Finding
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The… View comment
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7c01c030-226e-41c5-81ca-a6483a1856df

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Its 82-file, security-sensitive cross-platform process-attachment surface and still-running CI require final human validation.

Review tier: Balanced
Findings: 2 High severity · 4 Medium severity

Pre-existing issues (6)
Severity Finding
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The… View comment
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Tree attachment can evaluate a .NET project while the workspace is in Restricted Mode.

Review tier: Balanced
Findings: 2 High severity · 4 Medium severity

Pre-existing issues (6)
Severity Finding
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The… View comment
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment
Suppressed comments (1)

extension/src/views/AspireAppHostTreeProvider.ts:1532

  • Tree-initiated attach is still available in Restricted Mode, and the shared service has no trust guard. For .NET resources this reaches dotnet msbuild during attach-target resolution before VS Code's startDebugging boundary, so clicking the action can evaluate an untrusted project. Block this command before invoking the resource-debug service (with a localized Restricted Mode outcome/message), or enforce trust in the shared service while preserving the language-model tool's safe result contract.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
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: 7c01c030-226e-41c5-81ca-a6483a1856df
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7c01c030-226e-41c5-81ca-a6483a1856df

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Debugger overrides can retarget a confirmed local attachment through settings such as CoreCLR pipeTransport.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 3 High severity · 4 Medium severity

New issues introduced by this change (1)
Severity Finding
High severity extension/​src/​debugger/​resourceDebugService.ts — The override merge can still retarget a confirmed local attach. For example, the CoreCLR provider…
Pre-existing issues (6)
Severity Finding
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The… View comment
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment

Comment thread extension/src/debugger/resourceDebugService.ts Outdated

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Target identity can be reconstructed incorrectly for valid .NET launch options, and PID-based CLI selection lacks direct regression coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 3 High severity · 5 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity src/​Aspire.Cli/​Backchannel/​AppHostConnectionResolver.cs — The new PID discriminator is not exercised by a resolver test. The added command tests only…
Pre-existing issues (7)
Severity Finding
High severity extension/​src/​debugger/​resourceDebugService.ts — The override merge can still retarget a confirmed local attach. For example, the CoreCLR provider… View comment
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The… View comment
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment
Suppressed comments (1)

src/Aspire.Hosting/Dcp/ResourceSnapshotBuilder.cs:321

  • This marks every recognized run/watch invocation attachable after preserving only configuration and framework. Valid options such as --runtime/--arch/--os, --artifacts-path, and MSBuild property overrides can change TargetPath; redacted configuration/framework values are also silently treated as absent. The extension then reevaluates the default target identity and cannot resolve the actual child. Please fail closed (project.launchCommand = null) whenever target identity depends on an unmodeled or sensitive launcher argument, or serialize and replay every non-sensitive identity input.

Comment thread src/Aspire.Cli/Backchannel/AppHostConnectionResolver.cs
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
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: 7c01c030-226e-41c5-81ca-a6483a1856df

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

The security-sensitive, cross-platform debugger change spans 80 files and still awaits its required packaged Linux attach shard.

Review tier: Balanced
Findings: 3 High severity · 5 Medium severity

Pre-existing issues (8)
Severity Finding
High severity extension/​src/​debugger/​resourceDebugService.ts — The override merge can still retarget a confirmed local attach. For example, the CoreCLR provider… View comment
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The… View comment
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity src/​Aspire.Cli/​Backchannel/​AppHostConnectionResolver.cs — The new PID discriminator is not exercised by a resolver test. The added command tests only… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment

@adamint

Copy link
Copy Markdown
Member Author

Three current gaps still need cleanup: tree attach must stop at the Restricted Mode boundary, debugger overrides must not retarget a confirmed local attach through remote/transport settings, and same-path AppHost PID selection needs a real resolver regression. I’m fixing all three before the final CI pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 7c01c030-226e-41c5-81ca-a6483a1856df

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

.NET path matching mishandles Windows UNC paths, preventing reliable attachment from network shares.

Review tier: Balanced
Findings: 1 High severity · 5 Medium severity

Pre-existing issues (6)
Severity Finding
High severity extension/​src/​debugger/​resourceDebugService.ts — The selected AppHost PID is lost here: fetchAppHostResourcesOnce accepts only a path and runs… View comment
Medium severity src/​Aspire.Cli/​Backchannel/​AppHostConnectionResolver.cs — The new PID discriminator is not exercised by a resolver test. The added command tests only… View comment
Medium severity extension/​src/​debugger/​languages/​go.ts — Issue #18538 requires supported non-.NET resources to retain the same debugger user overrides as… View comment
Medium severity extension/​src/​test-e2e/​resourceDebugTools.e2e.test.ts — This packaged E2E path stops at debuggerExtensionMissing; it never starts a .NET or Go debug… View comment
Medium severity extension/​src/​lm/​appHostTargetResolverService.tsstartsWith(&#39;..&#39;) rejects valid in-workspace paths whose first component merely begins with two… View comment
Medium severity extension/​src/​debugger/​languages/​dotnet.ts — This apphost match relies only on ps -o comm=, even though the new Go implementation explicitly… View comment
Issues resolved since last review (2)
Severity Finding
High severity extension/​src/​debugger/​resourceDebugService.ts — The override merge can still retarget a confirmed local attach. For example, the CoreCLR provider… View resolved comment
High severity extension/​src/​views/​AspireAppHostTreeProvider.ts — Block tree-initiated attach in Restricted Mode before invoking the shared service. The… View resolved comment
Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

extension/src/debugger/languages/dotnet.ts:1172

  • UNC executable paths are not classified as Windows paths here. For a target such as \\server\share\Api.dll, the comparison remains case-sensitive, so a casing difference between the evaluated TargetPath and the CIM executable path prevents self-contained resource attach even though drive-letter paths already compare case-insensitively. Detect UNC form from the original operands before normalizing.

This issue also appears on line 1237 of the same file.

extension/src/debugger/languages/dotnet.ts:1237

  • The framework-dependent matcher has the same UNC gap: after normalization, \\server\share\Api.dll starts with //, not a drive letter, so matching is case-sensitive and can reject the exact Windows target when path casing differs. Include raw UNC paths in the Windows-path check.
    const isWindowsPath = /^[a-z]:\//i.test(normalizedCommand) || /^[a-z]:\//i.test(normalizedTargetPath);

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

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.

Expose a project resource child PID or evaluated AssemblyName for debugger attach [VS Code] Attach a debugger to resources in a running AppHost

3 participants