Skip to content

Fix Rust AppHost run-session notification routing - #19831

Merged
Adam Ratzman (adamint) merged 2 commits into
microsoft:mainfrom
adamint:adamint-investigate-issue-19750
Sep 1, 2026
Merged

Adam Ratzman (adamint) merged 2 commits into
microsoft:mainfrom
adamint:adamint-investigate-issue-19750

Conversation

@adamint

Copy link
Copy Markdown
Member

Description

Rust AppHosts launched through CodeLLDB or cppvsdbg could send an empty run-session ID to DCP before the real Rust resource started. DCP rejected that notification and recycled the shared connection, which could leave the resource at PID 0 with no logs or proxy while the adapter-owned process kept running.

This keeps AppHost adapter events on the AppHost lifecycle path instead of reporting them as resource events. It also rejects empty run-session IDs at the extension's DCP boundary so another producer cannot send the same invalid payload.

The Rust NoDebug launch path remains unchanged. We still use the configured adapter to launch the AppHost; we just no longer treat that AppHost child as a DCP resource run.

Validation:

  • Extension lint passed.
  • Full extension unit suite: 2,754 passing, 5 pending.
  • Focused AppHost lifecycle and DCP wire regressions: 2 passing after rebasing onto current main.

Fixes #19750

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
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

Adam Ratzman (adamint) and others added 2 commits August 31, 2026 18:19
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0d661ed-8b37-4959-b365-e29b3e8911e2
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c0d661ed-8b37-4959-b365-e29b3e8911e2
Copilot AI balanced review requested due to automatic review settings August 31, 2026 23:21
@github-actions

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 -- 19831

Or

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

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

Review tier: Balanced
Findings: None

What changed in this PR

Prevents Rust AppHost adapter lifecycle events from corrupting DCP run sessions.

Changes:

  • Separates AppHost lifecycle events from resource notifications.
  • Rejects empty run-session IDs at the DCP boundary.
  • Adds focused regression tests.
File Description
extension/​src/​debugger/​adapterTracker.ts Suppresses DCP lifecycle notifications for AppHosts.
extension/​src/​dcp/​AspireDcpServer.ts Drops and logs notifications with empty session IDs.
extension/​src/​test/​adapterTracker.test.ts Tests AppHost lifecycle routing.
extension/​src/​test/​aspireDcpServer.test.ts Tests empty-ID rejection before delivery.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

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.

I reviewed the AppHost-versus-resource adapter routing, DCP run-session lifecycle and wire-delivery paths, Rust adapter behavior, shutdown/restart/telemetry ownership, and focused regression coverage.

I found no correctness issues. I left one nonblocking maintainability note about removing a delivery method made unreachable by this change.

Comment thread extension/src/dcp/AspireDcpServer.ts
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.

Rust Debug Failing

3 participants