Fix duplicate inherited ATS method exports - #20438
David Pine (IEvangelist) wants to merge 2 commits into
Conversation
Reuse inherited method exports only when the same capability ID is owned by a base context included in the scan. Preserve namespace and ExposeMethods aliases, unscanned bases, overrides, and genuine duplicate diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 20438Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 20438" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The scanner change is narrowly scoped and comprehensively covers the identified inheritance cases without unresolved issues.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes false duplicate ATS capability diagnostics from inherited methods, unblocking fail-closed 13.6 API ingestion.
Changes:
- Detects when a scanned exported base type already owns an inherited capability.
- Preserves aliases, overrides, unexported ancestors, and cross-assembly behavior.
- Adds 11 focused inheritance regression cases.
| File | Description |
|---|---|
src/Aspire.Hosting.RemoteHost/AtsCapabilityScanner.cs |
Deduplicates inherited method projections. |
tests/Aspire.Hosting.RemoteHost.Tests/AtsCapabilityScannerTests.cs |
Makes the test class partial. |
tests/Aspire.Hosting.RemoteHost.Tests/AtsCapabilityScannerTests.Inheritance.cs |
Adds inheritance regression coverage. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Extend existing concrete-target precedence through CLR inheritance. This prevents Radius CSharpAppResource collisions from globally dropping the generic IDotnetProgramResource export while preserving both wire IDs and guest method names. Cover derived targets, unrelated implementations, dispatch, and genuinely ambiguous interfaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 435fd4e)
Tests selector8 / 99 PR test projects · 4 PR jobs, from 3 changed files. Selected PR test projects (8 / 99)
Selected PR jobs (4)
How these were chosen — grouped by what changed🔧 📦 affected project 🧪 🧪 Job reasons
Selection computed for commit |
| // generic capability even from unrelated implementations of that interface. | ||
| if (exactTargetCapabilities.Count == 0) | ||
| { | ||
| exactTargetCapabilities = collidingCapabilities |
There was a problem hiding this comment.
This can leave a derived target with no configure method. For B : I, D : B, and C : B, J, let I, J, and B each export a different capability named configure (IDs a, m, and z). Here the new specificity rule trims a from D in favor of z, but the ambiguous collision on C later removes z globally. D then has neither capability; before this change it retained a. I reproduced the final collision state against the built scanner (D had no capability). Please defer per-target pruning until global removals are known, or resolve collisions per target so each surviving target retains a fallback.
Follow-up to #1780, which merged with two open checkboxes. Both are now validated against the latest staging build. This is a single commit on top of the current `release/13.6` tip (`5589ce6d`), so it merges cleanly. - [x] Generated API catalog refreshed from genuine 13.6 packages, including the late `WithRepl` additions and removed attributes. - [x] REPL samples and all other new samples checked against the real 13.6 SDK, not stale Twoslash types. ## Provenance - **Feed:** `darc-pub-microsoft-aspire-f4c27f2d` (`https://pkgs.dev.azure.com/dnceng/public/_packaging/darc-pub-microsoft-aspire-f4c27f2d/nuget/v3/index.json`). - **Source:** microsoft/aspire `release/13.6` at `f4c27f2d43ddc1cacd0dd083b30d1fea1cee7a62`, the newest staging build. Every regenerated official TS module records `sourceCommit: f4c27f2d…`, and the restored nuspecs (for example Redis, Blazor and CodeGeneration.TypeScript) report that commit. - **Versions:** stable packages are `13.6.0`; prerelease packages are `13.6.0-preview.1.26478.8`. The codegen package is `Aspire.Hosting.CodeGeneration.TypeScript 13.6.0`, mapped exclusively to the staging feed. nuget.org has no `13.6.0`, so resolution is unambiguous. - **Scanner:** a locally fixed CLI/ATS scanner, built from `a11eca96` plus `320eed42` (the microsoft/aspire#20438 content) and `435fd4eb` (the microsoft/aspire#20443 content). Both upstream PRs are still unmerged. - Nothing in `a11eca96..f4c27f2d` touches `AtsCapabilityScanner`, `Aspire.TypeSystem` or `Aspire.Hosting.CodeGeneration.TypeScript`. The last two commits (microsoft/aspire#20566 and microsoft/aspire#20562) only change dashboard layout code and Dockerfiles. - The layout's `dashboard` and `dcp` folders are copies from the `8230626c` staging CLI bundle. Layout discovery requires them, but they aren't used for scanning or code generation. - **Isolation:** the stable version number didn't change between builds, so generation used fresh process-local `NUGET_PACKAGES`, HTTP cache, `TEMP` and `ASPIRE_HOME` folders. That rules out reusing `e8fd6fbb` bits. `ASPIRE_REPO_ROOT` and `ASPIRE_REPO_PATH` were unset, and there was no source-project substitution. - **Pipeline:** the repo's own pipeline, in this order: `update:integrations` → `generate-package-json.ps1` → `normalize:api-data -- --pkgs` → `update:ts-api` (with Twoslash `.d.ts`) → `validate:api-data`. - Generated JSON and `d.ts` were not hand-edited, and nothing from 14.x was imported. ### Deviation: no `ASPIRE_RELEASE_VERSION` pin Pinning `13.6.0` left the 50 prerelease packages stale. The feed is commit-specific, so the unpinned run resolves every package from the same build. ### Packages absent from the feed These seven official packages aren't in this build, so they are carried forward unchanged: - `Aspire.Elastic.Clients.Elasticsearch` 13.3.0 - `Aspire.Hosting.AgentFramework.DevUI` 1.22.0-preview.260918.1 - `Aspire.Hosting.AWS` 13.7.2 - `Aspire.Hosting.ClickHouse` 13.5.3 - `Aspire.Hosting.DocumentDB` 0.116.0 - `Aspire.Hosting.Elasticsearch` 13.3.0 - `Aspire.Hosting.GitHub.Models` 13.5.4 ## Changes - **Generated data:** regenerated `aspire-integrations.json` (still 217 packages; 82 at `13.6.0` and 50 at `13.6.0-preview.1.26478.8`), `pkgs/` (210 succeeded, 0 failed), `ts-modules/` (146 succeeded, 0 failed) and `twoslash/aspire.d.ts`. `integration-docs.json` needed no change. - **Generator fix** (`generate-package-json.ps1`): the 24 Provisioning overlays restored `Aspire.Hosting` at the overlay's own prerelease version, which doesn't exist now that `Aspire.Hosting` is stable `13.6.0`. The script now uses the resolved `Aspire.Hosting` version, via a new `-HostingVersion` parameter with a feed-lookup fallback for selective runs. - **Blazor docs:** `AddDotnetProjectBlazorGateway` and `WithBlazorClientApp` report their own `ASPIREDOTNETPROJECT001` diagnostic. `ASPIREBLAZOR001` applies to other experimental Blazor hosting types, such as `BlazorWasmAppResource`. - A compile check with the pragma removed reports only `ASPIREDOTNETPROJECT001`, on exactly those two methods. - The previous wording, from #1564, said the gateway methods carry both diagnostics. - **Dashboard docs (microsoft/aspire#20562):** the dashboard no longer has a terminal button in the header or a terminal entry in the mobile menu. - `dashboard/explore.mdx` and the What's new bullet now describe the backtick key as the way to open and hide the terminal dock. ## Validation - **`WithRepl` coverage:** present in the C# and TS API data for all six REPL packages (PostgreSQL, MySql, MongoDB, SqlServer, Redis, Valkey) and in `aspire.d.ts`. - **C# compile:** every new sample compiles with 0 warnings and 0 errors against exact packages from the `f4c27f2d` feed (16 at `13.6.0`, 9 at `26478.8`). This covers REPL×6, Rust, ConnectorNamespace, Foundry Toolbox, Radius, CSI, Helm, Blazor (now matching #1564's `WithExternalHttpEndpoints` sample), Provisioning and Dotnet. The Dotnet samples need no `ASPIREDOTNETPROJECT001` suppression. - **TS compile:** the SDK was generated by a real `aspire restore` (codegen `13.6.0`, `Aspire.Hosting.Redis/13.6.0` and `Aspire.Hosting.Blazor/13.6.0-preview.1.26478.8`, all at `f4c27f2d`). Strict `tsc` (NodeNext) passes for all 18 `.mts` samples, including the new Blazor gateway TS sample. A negative control (`withReplz`) fails with TS2551. - **Scanner warnings:** - Radius reports no collisions; the earlier `withContainerImage` collision on `CSharpAppResource` is gone. - The `createRoleAssignment` overload collisions remain in 16 Provisioning overlays. They are recorded here, not suppressed; no doc sample calls this method. - **Tests:** `pnpm validate:api-data` passes (217 identities, 146 modules matched to C# provenance). These suites pass: `test:unit:structured-data` (82), `api-reference` (55), `api-markdown` (30), `ts-api` (31), `twoslash-types` (12), `twoslash-blocks` (2), `llms-txt` (12) and `docs` (2). - No local `pnpm build` was run; CI covers it. ## Not done - **Contributors:** `update:release-contributors` needs the `v13.6.0` tag, which doesn't exist yet. This is left for after the release is tagged. Co-authored-by: David Pine <7679720+IEvangelist@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
[-512BD4?style=flat&logoColor=white&logo=...)](https://aka.ms/aspire/vnext) ## Summary Prepare the Aspire 13.6 documentation release. The release article is reconciled against [`microsoft/aspire` release/13.6 at `a11eca9611073f7cf66fa87faac63c2119e87713`](https://github.com/microsoft/aspire/tree/a11eca9611073f7cf66fa87faac63c2119e87713), compared with `v13.5.0`. The inventory contains 402 first-parent history entries, including maintenance, merges, and reversions—not 402 distinct shipped features. Highlights cover dashboard persistence/run history and terminals; Java, Rust, Connector Namespace and Sandboxes; coordinated .NET builds; CLI and VS Code workflows; deployment and integration improvements; emulator images; and migration guidance for Cosmos DB, Front Door, connection-string aliases, and terminal namespaces. ### Published documentation and ingestion work - `94b5f0c19`: migrate Java guides from Toolkit to first-party hosting, document typed Azure provisioning customization and VS Code agent lifecycle tools, and refresh 34 container-image records from the pinned release source. Default tag changes: App Configuration `1.0.2 → 1.2.0`, Cosmos DB `stable → vnext-latest`. - `27e2db77a`: exact release-version selection, ATS-only package metadata, union and canonical SDK enum handling, empty C# navigation filtering, and fail-closed handling of SDK dump errors. - `0b64558c1`: reconcile all 29 contributors against the pinned release snapshot. - `2696e2207`: recognize const-object enum declarations, retain colliding enum-name literals in the shared Twoslash bundle while preserving exact package-specific JSON, and increase the full example-audit time budget for the larger dataset. - `7ab5f74bd`: include the exact Dotnet package as supporting scan context for Radius's generic .NET program export; subtract core/supporting modules without dropping Radius's real IDs or receiver targets. - **`44b9819bb`: publish the complete, validated 13.6 catalog, mappings, C#/TypeScript API data, and Twoslash bundle together.** - Concurrent release updates are preserved, including the environment badge and microsoft#1745's agent lifecycle options. Isolation/launch-profile inputs were confirmed in the pinned 13.6 source. - Related product changes: microsoft/aspire#18033, microsoft/aspire#19675, and microsoft/aspire#19134. The maintainer-set planned release date remains September 29, 2026 (microsoft#1690). Published package constants remain separate from prerelease ingestion. Release membership comes from product source, not merely a docs PR's target; microsoft#1740 documents microsoft/aspire#20231 (14.0), not a 13.6 feature. ### Generated data and provenance The published snapshot contains **217 catalog entries**, **161 documentation mappings** (including the 26 new mappings), **210 C# records**, **146 TypeScript modules**, and the rebuilt Twoslash bundle. There are 132 official packages at exact version `13.6.0-preview.1.26473.12`, with **no 14.x imports**. Independently versioned packages retain their selected catalog versions. Generation used genuine pinned packages from the public dotnet9 feed and an isolated, locally built **13.6** CLI/RemoteHost. Package provenance remains `a11eca9611073f7cf66fa87faac63c2119e87713`; the scanner fixes are separate development-build provenance, not a claim that an official corrected CLI has shipped. - Full TypeScript ingestion with the inherited-method fix (`320eed42f85a7d4b090a9429b98c7a6a8547a4b8`) produced 146 modules, 0 failures, and 7 explicit skips. - Core/Radius regeneration with the additional target-specificity fix (`435fd4eb7d06ef99702ae5106cf01c10f5c6a780`) succeeded. The real compound Radius + exact Dotnet SDK dump has no diagnostics and retains both `withContainerImage` and `withDotnetProgramContainerImage` capability IDs. - ProjectResource/CSharpAppResource use the concrete Radius export; DotnetProjectResource uses the generic export. Dotnet's APIs are not attributed to Radius. No source-package attributes, capability identities, or missing-export checks were altered to force validation through. ## Third-party links and affiliations - Radius documentation — no material affiliation. - Source/specification links remain within `microsoft/aspire`; Java debugger links point to the corresponding Visual Studio Marketplace extensions. No new material affiliation claims. ## Validation - All **25 authored TypeScript guide/release examples** pass against the final declaration bundle; guide samples were also compiled with the genuine generated SDK. - The **complete annotated-site Twoslash audit passes**, with no diagnostic suppression. - Final semantic validation passes: 217 package identities, 146 module-provenance matches, 74 DTO shapes, and 2,873 handle inheritance chains. - **82 structured-data tests**, **75 API-reference/declaration-generator/API-route tests**, and **17 ATS transformer tests** pass. The final API-reference gate resolves the Radius exports without exceptions or aliases. - Earlier compile-only C# validation covered 24 guide methods and one type declaration without warnings or errors. Browser checks covered Java, Azure customization, VS Code and AI-agent routes and language tabs. - Source scanner/dispatcher/context-filter/API-export regressions: **215 pass on each product branch**, including exact-package dispatch and generated-SDK compilation checks. - No local production site build, cloud deployment, destructive cleanup scenario, CLI self-update, or release workflow was run. Current-head CI must still complete after the latest pushes. ## Remaining release gates - **Main scanner fix: microsoft/aspire#20438**, ready for review, latest commit `5482164dc1225f3c2a1a93bf5055e11b720117e9`. **13.6 backport: microsoft/aspire#20443**, draft, latest commit `435fd4eb7d06ef99702ae5106cf01c10f5c6a780`. Both require human review/merge; neither has been merged or released by this session. Latest-commit CI is still being monitored. - Official shipping requires the corrected RemoteHost-containing CLI/bundle. Rebuilding Network or Radius alone is insufficient. Local documentation generation does not replace that packaging gate. - The Express diagnostic short link requires owner action: microsoft#1599 (comment). - Complete current-head docs CI and final release/runtime acceptance. Keep this PR draft until the release gates are satisfied. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: aspire-repo-bot[bot] <268009190+aspire-repo-bot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: James Newton-King <james@newtonking.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JamesNK <303201+JamesNK@users.noreply.github.com> Co-authored-by: David Pine <7679720+IEvangelist@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Maddy Montaquila <maddy@pi.hole> Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> Co-authored-by: Ella Hathaway <ellahathaway@microsoft.com> Co-authored-by: aspire-repo-bot[bot] <aspire-repo-bot[bot]@users.noreply.github.com> Co-authored-by: David Aniebo <aniebovictor001@gmail.com> Co-authored-by: Alistair Matthews <alistairwebdojo@live.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Maddy Montaquila <maddy@MadBook-Pro-20.local> Co-authored-by: Ella Hathaway <67609881+ellahathaway@users.noreply.github.com> Co-authored-by: David Pine <dapine@microsoft.com> Co-authored-by: Nell Shamrell-Harrington <nellshamrell@gmail.com> Co-authored-by: David Negstad <50252651+danegsta@users.noreply.github.com> Co-authored-by: Sébastien Ros <sebastienros@gmail.com> Co-authored-by: David Negstad <David.Negstad@microsoft.com> Co-authored-by: Sébastien Ros <1165805+sebastienros@users.noreply.github.com> Co-authored-by: karolz-ms <15271049+karolz-ms@users.noreply.github.com> Co-authored-by: Karol Zadora-Przylecki <karolz@microsoft.com> Co-authored-by: Mitch Denny <midenn@microsoft.com> Co-authored-by: Mitch Denny <midenn@orangecake.local> Co-authored-by: Mitch Denny <midenn@Mac.localdomain> Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com> Co-authored-by: Maddy Montaquila <maddyleger1@gmail.com> Co-authored-by: Maddy Montaquila <maleger@microsoft.com> Copilot-Session: b0007635-1ab8-4ffc-9c7b-8c09439c79f6 Copilot-Session: 9ecc352e-ddd2-4c3e-9c4a-eafcc2a74157 Copilot-Session: b8ebe88c-337e-4d4a-aa80-e14c2c76289b Copilot-Session: 5b0c81a3-d822-462e-8cf5-8eb6debfe968 Copilot-Session: b156fe61-0b1c-460c-9735-1eaeaa356b0a Copilot-Session: 41298945-9592-4284-be9e-be0c58f31fad Copilot-Session: 5eeee6c8-e0b2-4836-96ff-a9726e92b2ee Copilot-Session: 829e510d-2b06-4301-9759-3bd760c45e5c
Description
Fix false duplicate-capability diagnostics when the shared RemoteHost ATS scanner encounters the same inherited method through exported derived proxy types. These errors block fail-closed API ingestion for microsoft/aspire.dev#1599 even though the generated
AddTocapability IDs are already unique. Companion release backport: #20443.Skip an inherited method projection only when an exported base included in the scan owns the same capability ID. Preserve intentional aliases, methods inherited from unexported ancestors, omitted-base-assembly cases, assembly-level exports, overrides, and diagnostics for genuinely conflicting declarations.
The change is limited to
AtsCapabilityScanner.cs, its existing test class, andAtsCapabilityScannerTests.Inheritance.cs(11 inheritance regression cases). It does not change generated IDs, public APIs, package versions, property behavior, or the separateAtsContextFilterdiagnostic-ownership behavior.Observable behavior
Against the exact
13.6.0-preview.1.26473.12package fixture, using the corrected scanner source:All three actual
AddToMethodInfoinvocations preserve the expected in-memory SDK resources. The base and both derived proxy interfaces retain theirAddToand inheritedClearNamefunctionality.Radius follow-up: distinct target-specificity defect
Commit
5482164dc1225f3c2a1a93bf5055e11b720117e9fixes a separate overload-selection defect. Radius's ProjectResource-specificwithContainerImagealready shadows its genericIDotnetProgramResourceoverload on ProjectResource. But CSharpAppResource inherits ProjectResource: neither declared target exactly matched the subclass, so collision handling removed the genericwithDotnetProgramContainerImagecapability globally, including from the unrelated DotnetProjectResource implementation.When there is no exact target, recognize a unique declared target that is strictly more specific than every competitor and reuse the existing per-target shadowing path. Unrelated/equally specific targets remain ambiguous. This uses CLR assignability, not a Radius allowlist or suppressed warning.
Wire compatibility: no Radius attribute or guest method is renamed. ProjectResource/CSharpAppResource retain
Aspire.Hosting.Radius/withContainerImage; DotnetProjectResource retainsAspire.Hosting.Radius/withDotnetProgramContainerImage. All keep.withContainerImage(image). Ignoring the legacy export instead would break its existing wire ID.Validation
Initial commit
cbaabf400506c0b36d3fbc8748b6076dfe0673cdpassed 212 targeted tests, exact-version Network/Kusto scans and in-memory dispatch checks, strict generated-SDK TypeScript compilation, and a complete patch apply-check against release/13.6.Latest follow-up
5482164dc1225f3c2a1a93bf5055e11b720117e9:13.6.0-preview.1.26473.12fixture confirms both wire IDs and annotation replacement/same-builder semantics. Complete generated SDK and.withContainerImage()calls on Project/CSharp/Dotnet compile under strict NodeNext/ES2022/noEmit settings.13.6.0-preview.1.26473.12+a11eca9611073f7cf66fa87faac63c2119e87713. Single-package canonical output remains scoped to its loaded receiver context; the compound raw dump supplies Dotnet supporting receivers.git diff --checkclean. No package metadata/output JSON editing or identity spoofing.The full product suite, full AppHost/cloud startup, official release packaging, cloud provisioning, and release publishing were not run. Local metadata harness success is not published-release validation.
Shipping dependencies
This PR needs review and latest-commit CI. #20443 provides the release/13.6 backport; maintainer-approved merge ordering and an officially rebuilt RemoteHost-containing CLI/bundle remain required. Repeat exact-version full/scoped Network/Kusto and compound Radius+Dotnet checks on distributed binaries. Rebuilding an integration package alone is insufficient. Keep full-scan uniqueness coverage because the separate scoped diagnostic-ownership behavior is unchanged. No merge or release action was performed.
Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?