Handle duplicate static web asset identities gracefully

[ilonatommy]

Summary

Multiple StaticWebAssets MSBuild tasks crash with ArgumentException: An item with the same key has already been added when candidate assets contain duplicate Identity keys. This PR handles duplicates gracefully by keeping the first occurrence and skipping subsequent ones.

What causes duplicates

Two known scenarios produce duplicate asset identities:

1. Hosted Blazor WASM projects referencing multiple WASM client projects — dotnet.js.map from microsoft.netcore.app.runtime.mono.browser-wasm appears multiple times via different dependency paths. This blocks the dotnet/aspnetcore CI codeflow PR (aspnetcore#65673).

2. Blazor Web App referencing another Blazor Web App — blazor.web.js from Microsoft.AspNetCore.App.Internal.Assets appears twice (repro: DanielSundberg/DiscoverPrecompressedAssets).

What this PR fixes

All four code paths that build Identity-keyed dictionaries from static web asset items:

File	Change
DiscoverPrecompressedAssets.cs	ToDictionary → ContainsKey loop with diagnostic log
StaticWebAsset.ToAssetDictionary()	.Add → ContainsKey guard (used by 5 other tasks)
GenerateStaticWebAssetsManifest.cs	ToDictionary → ContainsKey loop
GenerateStaticWebAssetEndpointsManifest.cs	ToDictionary → ContainsKey loop

Verified locally

• Reproduced crash on dotnet/aspnetcore codeflow branch (darc-main-b867beb1-...) with SDK 11.0.100-preview.3.26128.104
• Verified fix — HostedInAspNet.Server builds successfully after patching
• Reproduced crash on dotnet/sdk#52089 minimal repro (BlazorApp referencing BlazorPlugin)
• Verified fix — the DiscoverPrecompressedAssets crash is eliminated

What this does NOT fix

For the Blazor-references-Blazor scenario (#52089), eliminating the ArgumentException crash surfaces a different, pre-existing error:

Conflicting assets with the same target path '_framework/blazor.server.js'

This conflict occurs because the same framework asset arrives from two projects with different SourceType (Discovered vs Project). The proper architectural fix for this is tracked in #53135 (Framework SourceType by @javiercn), which prevents framework asset duplicates from being generated in the first place.

This PR and #53135 are complementary:

• This PR: safety net — prevents ToDictionary crashes regardless of duplication source
• Add Framework SourceType for static web assets #53135: architectural fix — prevents framework asset duplication upstream

Fixes #52089
Fixes #52647

[dotnet-policy-service]

Thanks for your PR, @@ilonatommy.
To learn about the PR process and branching schedule of this repo, please take a look at the SDK PR Guide.

[Copilot]

Pull request overview

This PR prevents Static Web Assets MSBuild tasks from crashing when static web asset item groups contain duplicate Identity values, by changing Identity-keyed dictionary construction to skip subsequent duplicates.

Changes:

• Replaced ToDictionary(...) calls with Dictionary + duplicate-guard loops in manifest generation tasks.
• Updated StaticWebAsset.ToAssetDictionary() to use OSPath.PathComparer and skip duplicate identities instead of throwing.
• Updated DiscoverPrecompressedAssets to dedupe candidate assets by identity and emit a low-importance diagnostic when duplicates are skipped.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetsManifest.cs	Avoids ToDictionary duplicate-key crashes when filtering publish endpoints.
src/StaticWebAssetsSdk/Tasks/GenerateStaticWebAssetEndpointsManifest.cs	Avoids ToDictionary duplicate-key crashes when building the endpoints manifest asset map.
src/StaticWebAssetsSdk/Tasks/Data/StaticWebAsset.cs	Makes ToAssetDictionary() resilient to duplicate identities (and uses OS-specific path comparer).
src/StaticWebAssetsSdk/Tasks/Compression/DiscoverPrecompressedAssets.cs	Dedupes candidates by identity and logs when duplicates are skipped to prevent task failure.

[lewing]

Hey @ilonatommy, thanks for the thorough investigation here — the repro work and root cause analysis are really clear.

I wanted to share some context on this since I'm the one who introduced the regression (runtime#124125). Here's the chain of events:

1. sdk#52283 fixed esproj compression by flipping TryFindInputFilePath to prefer Identity over OriginalItemSpec
2. This exposed that WASM boot config Identity pointed to an OutputPath copy that could be stale on incremental builds, causing wrong SRI hashes (aspnetcore#65271)
3. runtime#124125 (my PR) fixed this by making Identity = real file path on disk via per-item ContentRoot="%(RootDir)%(Directory)", following @javiercn's suggestion in sdk#52847 to "define assets in their original location on disk"
4. This broke multi-WASM-client scenarios because NuGet cache pass-through files (dotnet.js, maps, ICU, native wasm) resolve to the same Identity across projects → ToDictionary crash

The root cause is specifically that ConvertDllsToWebcil converts .dll files to .wasm in $(IntermediateOutputPath)webcil/ (unique per project ✅) but passes through non-dll files at their original NuGet cache path (shared across projects ❌).

Why I think the ContainsKey approach is risky here: Javier's SWA pipeline deliberately uses .Add() in ToAssetDictionary — duplicate Identity is designed to signal an upstream pipeline bug. Silently skipping duplicates can mask real issues. For example, the Blazor-references-Blazor scenario you mentioned (sdk#52089) — the ContainsKey fix eliminates the crash but surfaces a different pre-existing "conflicting target path" error, which suggests the real fix needs to happen upstream.

The fix I'm working on is in runtime's Browser.targets: after ConvertDllsToWebcil, copy the NuGet cache pass-through files to a per-project location ($(IntermediateOutputPath)wasm/) before they enter DefineStaticWebAssets. This follows the same pattern as sdk#52816 (which solved a similar HotReload duplicate asset issue). With per-project copies:

• Identity = real file on disk (Javier's requirement) ✅
• Identity is unique per project (no duplicates) ✅
• SRI integrity on incremental builds stays correct ✅

I have the fix applied in a local VMR build that's running now and should have a runtime PR up soon. cc @javiercn

[ilonatommy]

I have the fix applied in a local VMR build that's running now and should have a runtime PR up soon. cc @javiercn

I agree, I don't like the part of dropping one of the entries either. As soon as your PR is out, feel free to close this one.

[ilonatommy]

Closing in favor of dotnet/runtime#125309.

[lewing]

Following up on my earlier comment — after deeper investigation, I've confirmed the SDK-only approach in this PR is the correct fix. The runtime copy approach I was originally pursuing (dotnet/runtime#125309) re-introduces the staleness problem that runtime#124125 was specifically fixing.

The duplicate Identity scenario is valid: Identity = FullPath, so same Identity = same physical file on disk (shared NuGet cache). Skip-duplicate is always semantically correct here.

I independently arrived at the same fix (same 4 files, same pattern) and validated it end-to-end: build ✅, publish ✅, multi-client WASM ✅. Your version with TryGetValue + diagnostic warnings is better than my bare ContainsKey. I've closed my duplicate PR #53328.

One note: TryAdd won't work on net472 (the task project multi-targets) — but your TryGetValue approach avoids that entirely. 👍

[javiercn]

Spoke offline with @lewing. This is not the right approach to solve this problem.

The issue stems from a target (in our case wasm, but blazor web does the same) introducing multiple definitions of the same asset, for example, dotnet.js or blazor.web.js. this typically happens through project references (multiple wasm apps, multiple "blazor web apps", although this is not a thing) and fails on the consuming project because unlike for packages which naturally only include a single version.

For assets introduced this way, they get defined in the context of the consuming project. The results is that the two duplicates have the same identity (which is not expected), but end up with different metadata properties (legitimately).

Choosing a winner is not the right approach as it will inevitably break one or another app. For example, if you pick a winner for dotnet.js, you could have /App1/dotnet.js and /App2/dotnet.js (Base path app1 and app2 respectively) both pointing to the path in the runtime pack, and by picking the first one, the second app would simply stop working (because the file wouldn't be served).

Assets being unique across the entire pipeline (that is their Identity being unique across the entire pipeline) has been a constraint / design invariant for years, and not something we should change lightly.

[lewing]

dotnet/runtime#125329 is where I'm prototyping things now

Stale