microsoft / microsoft/aspire

TypeScript projector: canonical capability inherits alias options interface due to alphabetical registration order

Open
#19,732 1 comment 0 reactions 0 assignees View on GitHub
area-app-model area-codegen area-polyglot triage:bot-seen
Dominant language
C#
Stars
6.3k
Forks
991
Avg merge
2d 15h
Merged PRs (30d)
196

Description

### Is there an existing issue for this?

- [X] I have searched the existing issues

### Describe the bug

`TypeScriptApiProjector.RegisterOptionsInterface` has a guard whose stated intent is to stop capabilities that share a projected method name from also sharing an options interface when their optional parameters differ:

```csharp
// src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiProjector.cs:2167
var capabilityName = GetCapabilityName(capabilityId);
if (!string.Equals(capabilityName, methodName, StringComparison.Ordinal)
&& !AreOptionsExactMatch(existingParams, optionalParams))
{
// Capabilities can share a projected method name while accepting different options.
// Reusing the method-name interface would let callers pass options that the selected
// capability implementation never reads, so fall back to the capability ID.
RegisterDisambiguatedOptionsInterface(...);
return;
}
```

The `capabilityName == methodName` escape hatch encodes "the canonical capability is entitled to keep the method-named interface". That is reasonable, but it only holds if the canonical capability registers **first**.

Capabilities are registered sorted ordinal by capability ID (`TypeScriptApiProjector.cs:277`):

```csharp
.OrderBy(capability => capability.CapabilityId, StringComparer.Ordinal)
```

So whenever an alias ID sorts before the canonical ID, the alias creates the method-named interface, and the canonical capability then falls through the guard (because its name *does* equal the method name) and merges its own extra options into the alias's interface — the exact inversion of the intended behaviour.

### Expected Behavior

The canonical capability (capability ID == projected method name) should own the method-named options interface regardless of registration order. Aliases whose optional parameters differ should be pushed to disambiguated, capability-ID-named interfaces.

Concretely, for the `withVolume` family the generated output should be roughly:

```ts
export interface WithVolumeOptions { name?: string; isReadOnly?: boolean; } // container
export interface WithProjectVolumeOptions { isReadOnly?: boolean; } // project
export interface WithExecutableVolumeOptions { isReadOnly?: boolean; } // executable
```

### Steps To Reproduce

Reproduces in-repo against the `withVolume` capability family, no external project needed.

Three capabilities project to the method name `withVolume`:

| Capability ID | Declared at | Required params | Optional params |
|---|---|---|---|
| `withExecutableVolume` | `src/Aspire.Hosting/Ats/CoreExports.cs:130` | `target, name, env` | `isReadOnly` |
| `withProjectVolume` | `src/Aspire.Hosting/Ats/CoreExports.cs:110` | `target, name, env` | `isReadOnly` |
| `withVolume` (canonical, container) | `src/Aspire.Hosting/Ats/CoreExports.cs:91` | `target` | `name`, `isReadOnly` |

Ordinal order is `withExecutableVolume` < `withProjectVolume` < `withVolume`, so registration proceeds:

1. `withExecutableVolume` — no existing interface → creates `WithVolumeOptions` as `{ isReadOnly }`.
2. `withProjectVolume` — `AreOptionsExactMatch` is true → shares. (Correct.)
3. `withVolume` — interface exists, but `capabilityName == methodName`, so the guard is skipped entirely → `AreOptionsCompatible` merges `name` into the shared interface.

Observable in `tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts:1935`:

```ts
export interface WithVolumeOptions {
/** Whether the published volume is read-only. */
isReadOnly?: boolean;
/** The volume name. If null, an anonymous volume is created. */
name?: string;
}
```

The member ordering is the fingerprint of the inversion — `isReadOnly` (contributed by the alias, which registered first) precedes `name` (merged in later by the canonical capability).

Project and executable then advertise an options property they cannot honour, because `name` is already one of their required positional parameters (`:22021`, `:30162`, `:38540`):

```ts
withVolume(target: string, options?: WithVolumeOptions): ContainerResourcePromise;
withVolume(target: string, name: string, env: string, options?: WithVolumeOptions): ProjectResourcePromise;
```

A polyglot caller can write `withVolume("/data", "cache", "DATA_PATH", { name: "other" })`; the `name` in options is silently ignored.

### Anything else?

**Severity is low, which is why this is being filed rather than fixed inline.** The defect is a widening: the shared interface gains an extra optional property that some callers cannot honour. No existing code breaks and TypeScript does not error.

Deferring the fix is also close to non-breaking, because TypeScript is structurally typed. After a fix, an object literal such as `withVolume(t, n, e, { isReadOnly: true })` continues to compile against a renamed interface. Only code that explicitly imports or annotates the `WithVolumeOptions` type name would need updating.

**Possible approaches**

1. Make the canonical capability win regardless of arrival order — when a capability whose ID equals the method name registers and its options are not an exact match, evict previously assigned aliases to disambiguated interfaces and let the canonical own the base name. This is the direct fix but requires `AssignOptionsInterface` to be reversible, or a pre-pass that identifies the canonical capability before any interface is created.
2. Add a secondary sort key at `TypeScriptApiProjector.cs:277` so a capability whose ID equals its projected method name always registers before its aliases. Smaller change, but the blast radius across the roughly 60 `MethodName` aliases in `src/` is unknown until the snapshots are regenerated.

Whichever approach is taken, a regression test should cover an alias that sorts **before** its canonical capability, since that ordering is what defeats the current guard.

**Context:** found by automated review on #19404, which introduced the `withProjectVolume` and `withExecutableVolume` aliases. Before that PR, `WithVolumeOptions` was created by the canonical container capability as `{ name, isReadOnly }`; the PR did not change the interface members, only which capability creates it — and therefore which capabilities end up sharing it.

Contributor guide

Open the contributing guide

Research direction

Start in src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiProjector.cs at RegisterOptionsInterface and the capability ordering near line 277. Compare the withVolume declarations in src/Aspire.Hosting/Ats/CoreExports.cs with the generated snapshot at tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts. Add regression coverage for an alias that sorts before its canonical capability, and verify the canonical interface and disambiguated aliases in the regenerated output.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, typescript
Domain
tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.