Export canonical TypeScript API data from the CLI - #19032
Export canonical TypeScript API data from the CLI#19032Adam Ratzman (adamint) wants to merge 31 commits into
Conversation
Extract every TypeScript-specific resolution decision out of AtsTypeScriptCodeGenerator into TypeScriptApiProjector: type mapping, options flattening, callback shaping, promise wrapping, and fluent return selection. The generator consumes the resolved model instead of recomputing those decisions inline, and TypeScriptApiExportWriter serializes the same model into a schema version 1 canonical export. Documentation that reconstructs signatures from raw ATS drifts from the SDK that actually ships (microsoft#17608). Sharing one projection makes that drift impossible: the generated aspire.mts snapshots are byte-identical, and a new test asserts every exported declaration appears verbatim in the corresponding generated public interface. Ownership is resolved per capability rather than per type, mirroring AtsContextFilter, so a package that extends another package's resource documents its own members without republishing the referenced type. Referenced types contribute opaque declaration fragments keyed by their real owner, which lets a manifest concatenate packages and type-check without site-authored shims. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
Adds IApiReferenceExporter as an optional companion to ICodeGenerator, so a language provider can describe the surface it generates without every provider being forced to. AtsTypeScriptCodeGenerator implements it by building the same TypeScriptApiProjector code generation uses, which is what keeps documentation from drifting from emitted source. RemoteHost exposes it as an authenticated exportApi RPC that resolves the existing code generator and then requires the optional interface, rather than introducing a second discovery mechanism. The provider's JSON document is returned verbatim; the payload schema belongs to the language provider. Also fixes a reference-closure bug this uncovered. Types owned by the selected assembly were seeded into AtsContextFilter's included sets directly, so the "was this newly added?" guard refused to walk their own members. An owned DTO exposing an enum from a non-Aspire dependency kept the DTO and dropped the enum, and code generation then failed on the dangling reference -- generateCode for Aspire.Hosting hit this too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
Adds a hidden `aspire sdk export` that asks a scanner AppHost for the canonical API reference of one package in one language and writes it to stdout. Because documentation pipelines consume it, stdout carries exactly one JSON document and every status message goes to stderr, so `aspire sdk export ... > api.json` produces a usable file. The package version must be exact. A document published under a range would describe a different SDK after the next restore, so floating versions are rejected before restore rather than resolved. With no --package, the command defaults to Aspire.Hosting at this CLI's own identity version, which is the whole point: the docs describe the SDK this CLI generates against. SdkCommandPreparation holds only what dump and export genuinely share -- argument parsing, scanner AppHost setup, exporting-assembly discovery. Dump keeps its own serialization and is deliberately not routed through the canonical exporter; a test asserts its payload still has the capabilities shape and no schemaVersion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
The export contract promises that concatenating a manifest's declaration fragments type-checks without site-authored shims. Running TypeScript over real Aspire.Hosting and Aspire.Hosting.Redis exports showed it did not. Handle types with no generated wrapper class surface in signatures under their raw handle alias name, but the fragment pass derived a class name instead, so it declared a symbol nothing referenced and left the referenced alias undefined. Emit the same alias the generator emits. The runtime fragment was also missing Handle, InputType and AbortSignal, and MarshalledHandle was missing $type. Keep sdk export's own --help reachable: Hidden on a subcommand suppresses its help output, and the parent sdk command is already hidden. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
The scanner AppHost does not reference the language's code generation package by default, so the server loaded no generators and every export failed with "No code generator found for language: typescript". sdk generate already adds the package for exactly this reason. Verified end to end against the repo-local AppHost server: exporting Aspire.Hosting and Aspire.Hosting.Redis now writes schema version 1 documents to stdout with nothing on stderr, and TypeScript type-checks their combined declaration fragments with no errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
A package that extends a type another package owns emitted its
contribution as a normal interface item: same "interface:{name}" stable
ID as the owning package's item, and owningAssembly pointing at the
extending package. Across a manifest that collides with the real page and
misattributes the type, which the export contract explicitly forbids.
Give these items an augmentation kind, an "augmentation:{name}" ID, and
the type's real owning assembly. The declaration fragments already used
this split; the items now match.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19032Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19032" |
There was a problem hiding this comment.
Pull request overview
Adds canonical TypeScript API export through aspire sdk export, sharing projection logic with generated SDK output.
Changes:
- Adds the export model, serializer, RPC, and CLI command.
- Fixes ATS reference-closure handling.
- Adds schema, CLI, RPC, and regression tests.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Hosting.CodeGeneration.TypeScript/AtsTypeScriptCodeGenerator.cs |
Uses shared projection and implements export. |
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiProjector.cs |
Centralizes TypeScript API projection. |
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiModel.cs |
Defines the export model. |
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiExportWriter.cs |
Serializes schema v1. |
src/Aspire.TypeSystem/IApiReferenceExporter.cs |
Adds the exporter contract. |
src/Aspire.TypeSystem/ApiReferenceExportOptions.cs |
Adds package export options. |
src/Aspire.TypeSystem/api/Aspire.TypeSystem.cs |
Updates the generated API baseline. |
src/Aspire.Hosting.RemoteHost/CodeGeneration/CodeGenerationService.cs |
Exposes the export RPC. |
src/Aspire.Hosting.RemoteHost/CodeGeneration/CodeGeneratorResolver.cs |
Resolves exporters. |
src/Aspire.Hosting.RemoteHost/Diagnostics/RemoteHostProfilingTelemetry.cs |
Adds export telemetry. |
src/Aspire.Hosting.RemoteHost/AtsContextFilter.cs |
Expands referenced type closure. |
src/Aspire.Hosting.RemoteHost/Aspire.Hosting.RemoteHost.csproj |
Grants test internals access. |
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs |
Implements sdk export. |
src/Aspire.Cli/Commands/Sdk/SdkCommandPreparation.cs |
Shares scanner preparation. |
src/Aspire.Cli/Commands/Sdk/SdkDumpCommand.cs |
Uses shared preparation. |
src/Aspire.Cli/Commands/Sdk/SdkCommand.cs |
Registers the subcommand. |
src/Aspire.Cli/Projects/IAppHostRpcClient.cs |
Adds the client contract. |
src/Aspire.Cli/Projects/AppHostRpcClient.cs |
Implements export RPC invocation. |
src/Aspire.Cli/Program.cs |
Registers command services. |
tests/Aspire.Hosting.RemoteHost.Tests/CodeGeneration/ApiReferenceExportTests.cs |
Tests RPC behavior. |
tests/Aspire.Hosting.RemoteHost.Tests/AtsContextFilterTests.cs |
Covers closure expansion. |
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/AtsTypeScriptCodeGeneratorTests.cs |
Tests export parity and declarations. |
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/AtsTypeScriptCodeGeneratorTests.ApiExport.verified.json |
Captures schema output. |
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/AtsTypeScriptCodeGeneratorTests.ApiDeclarations.verified.txt |
Captures declaration fragments. |
tests/Aspire.Cli.Tests/Commands/Sdk/SdkExportCommandTests.cs |
Tests CLI behavior. |
tests/Aspire.Cli.Tests/TestServices/FakeAppHostServerSession.cs |
Extends the fake RPC client. |
tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs |
Registers the command in tests. |
Review details
Suppressed comments (3)
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:128
- The code generator is restored at the running CLI version rather than the requested package version. Exporting an older/newer package can therefore apply a different release's projection rules (or fail when
--sourcecontains only the requested release), so the result is not canonical for the requestedName@Version. Restore the generator atpackageVersion, as normal AppHost generation does for its effective SDK version.
integrations.Add(IntegrationReference.FromPackage(codeGenPackage, ExecutionContext.IdentityVersion));
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:76
- This command promises that every human-readable message goes to stderr, but it never switches
InteractionService.Consolefrom its default stdout route. As a result, preparation diagnostics and the--outputsuccess message can be written to stdout. Route the command's human output to stderr before any discovery or preparation work.
var language = parseResult.GetValue(s_languageOption)!;
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:119
- An explicit
Aspire.Hosting@Versionis never restored here. Both scanner implementations ignore thesdkVersionargument, so a CLI running a different version scans its bundled/repositoryAspire.Hostingassembly and then labels that surface with the requested version. The command therefore cannot honor its exactName@Versioncontract for the core package; the scanner must load the requested core package version rather than skipping it.
if (!string.Equals(packageName, CorePackageName, StringComparison.OrdinalIgnoreCase))
{
integrations.Add(reference);
}
- Files reviewed: 25/27 changed files
- Comments generated: 4
- Review effort level: Balanced
The generated capability scanner writes a Directory.Packages.props that turns central package management on, which rejects an inline Version attribute with NU1008. Any integration outside the repo failed to build, so sdk export could not be pointed at a third-party package. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
Ownership only transfers to PreparedSdkSession once one is returned. A throw from StartAsync or GetRpcClientAsync left the spawned scanner running and holding the temp directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
Fragments built from raw string literals carry whatever line endings the source was checked out with, so a Windows build disagreed with a Linux build about identical declarations. Consumers deduplicate a manifest by comparing content for the same ID, so the text has to be stable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
Every integration that extends DistributedApplicationBuilder produced the same augmentation item ID, which recreated the cross-package collision the augmentation kind exists to avoid, so the contributing package is now part of the ID. CreateBuilderOptions also gained its client-only throwOnPendingRejections property from the module emitter alone, so the export described a smaller interface than the one we ship. That list moved to the projector and both paths read it from there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
There was a problem hiding this comment.
Review details
Suppressed comments (3)
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiProjector.cs:276
- This hardcodes the generated options-bag parameter name to
options, while the source emitter callsGetPublicOptionsParameterNameto avoid collisions. A capability that already has a parameter namedoptionsis therefore exported with a different—or even duplicate and invalid—signature from the generated.mts, defeating the canonical-source contract. Resolve and pass the collision-safe name here too.
var parameterList = BuildPublicParameterList(
requiredParams,
hasOptionals,
optionsTypeName,
trailingCancellationToken: GetTrailingCancellationTokenParameter(optionalParams));
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiProjector.cs:744
- The structured
parametersarray is still built from raw ATS parameters instead of the resolved public signature. For example, the checked-in export declareswithOptionalString(options?: WithOptionalStringOptions)but reportsvalueandenabledas its parameters. Any consumer rendering the structured fields will reconstruct the same stale positional API this PR is intended to eliminate. Build this list from the required parameters plus the resolved options-bag and trailing cancellation-token parameters.
var parameters = capability.Parameters
.Where(p => builderModel is null || p.Name != targetParamName)
.Select(p => new TypeScriptApiParameter
{
Name = p.Name,
DeclaredType = MapParameterToTypeScript(p),
IsOptional = p.IsOptional || p.IsNullable,
Summary = p.Documentation?.Summary
})
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:194
- The new CLI command has only fake project/session/RPC tests; there is no CLI end-to-end test that starts the real scanner and validates the machine-output contract. That leaves package/codegen restoration, RPC registration, and stdout/stderr separation untested—the exact integration points this command introduces. Add a Hex1b CLI E2E test using a local-hive package that invokes
sdk export, redirects stdout, parses the JSON, and verifies stderr remains separate.
await using var session = await SdkCommandPreparation.PrepareSessionAsync(
_appHostServerProjectFactory,
_serverSessionFactory,
InteractionService,
_logger,
"aspire-sdk-export-",
sdkVersion,
integrations,
packageSource,
cancellationToken);
- Files reviewed: 27/29 changed files
- Comments generated: 1
- Review effort level: Balanced
The scanner never honored a requested core version. Both PrepareAsync implementations accept sdkVersion and ignore it, so `sdk export --package Aspire.Hosting@13.0.0` returned byte-identical JSON to a 13.5.0-dev export, relabelled 13.0.0. That is the stale-signature problem this command exists to fix. Honoring the request is not possible: the prebuilt server bundles core and the RPC host compiled together, so loading a foreign core would break the RPC contract. Reject the skew instead, with a message pointing at the CLI that can produce the requested export. Integration packages are unaffected: they become real PackageReferences and already resolve to the requested version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
There was a problem hiding this comment.
Review details
Suppressed comments (3)
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:135
- A non-core Aspire package can request any version here, but the scanner and TypeScript code-generation package are still pinned to the running CLI's
IdentityVersion. Thus a 13.6 CLI exportingAspire.Hosting.Redis@13.5.0applies the 13.6 signature-shaping generator and labels the result as 13.5.0, recreating the version drift this command is intended to prevent. Either restore/run the matching SDK and generator or reject officialAspire.Hosting.*versions that differ fromIdentitySdkVersion, as is already done for core.
else
{
integrations.Add(reference);
}
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiProjector.cs:276
ResolveMethodSignaturealways names the synthesized optional-parameter bagoptions, but the source emitter callsGetPublicOptionsParameterNameto avoid collisions. Real generated APIs such asaddCSharpAppandpromptProgresstherefore useoptionsBag, while this export reportsoptions, reintroducing the docs/generated-signature drift this PR is intended to remove. Compute and pass the same collision-safe name here.
var parameterList = BuildPublicParameterList(
requiredParams,
hasOptionals,
optionsTypeName,
trailingCancellationToken: GetTrailingCancellationTokenParameter(optionalParams));
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiProjector.cs:740
- The structured
parametersarray is built from the original ATS parameters rather than the public signature. For example, the snapshot declaresaddTestRedis(name, options?: AddTestRedisOptions)but serializesnameandport, with nooptionsparameter. Consumers using these schema fields will reconstruct the same stale positional API this export is meant to eliminate. Build this list from the required public parameters plus the synthesized options bag (and any separately emitted cancellation token).
var parameters = capability.Parameters
.Where(p => builderModel is null || p.Name != targetParamName)
.Select(p => new TypeScriptApiParameter
{
Name = p.Name,
- Files reviewed: 27/29 changed files
- Comments generated: 1
- Review effort level: Balanced
The command always emits JSON, so it has no --format option for BaseCommand's json redirect to key off. Preparation diagnostics and the --output success message therefore went to stdout and corrupted the document when a caller piped it. DisplaySuccess takes no per-call override, so it could not opt out. Route the interaction service to stderr at command entry. The JSON write already overrides back to stdout explicitly, and an explicit override wins over the service setting. SdkExportSendsProgressToStderrOnly asserted on the per-call override, which is null for these calls, so it passed vacuously. It now resolves the effective destination and covers the --output path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 819baaf4-91c3-44ce-8004-3dbc9110f810
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiExportWriter.cs:130
- A parameterless
[Obsolete]is represented by the projector asDeprecationMessage = string.Empty, butAddIfPresentdrops empty strings here. The generated TypeScript still emits@deprecated, while the canonical JSON reports no deprecation at all. Serializedeprecatedwhenever the value is non-null; an empty string still carries the deprecation state.
AddIfPresent(json, "deprecated", member.DeprecationMessage);
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:27
- The new user-visible command is covered only with fake
IAppHostServerProject/RPC clients. That cannot catch failures in the packaged CLI's command registration, code-generator package restore, scanner startup, RPC transport, or the stdout/stderr piping contract—the integration path this command exists to provide. Add a CLI end-to-end test that runs the packaged/local-hiveaspire sdk export, parses redirected stdout as JSON, and verifies stderr remains separate.
internal sealed class SdkExportCommand : BaseCommand
- Files reviewed: 38/41 changed files
- Comments generated: 2
- Review effort level: Balanced
| foreach (var (id, assemblyName) in result.CapabilityExportingAssemblyNames) | ||
| { | ||
| allCapabilityExportingAssemblyNames.TryAdd(id, assemblyName); | ||
| } |
There was a problem hiding this comment.
Fixed in 2886db7 — AtsCapabilityScanner.PruneRegistriesToSurvivingCapabilities now runs after both filters in both scan paths. The ownership map alone was not enough: GetKnownAssemblyNames also reaches the assembly through context.Methods[].DeclaringType, so the method and property registries are pruned with it. Covered by ScanAssemblies_CapabilityLostToACollision_DropsItsAssemblyFromExportingAssemblyNames and TryResolveCanonicalAssemblyName_RejectsAnAssemblyWhoseCapabilitiesWereAllFiltered, both of which fail without the prune.
| public sealed partial class ApiReferenceExportOptions | ||
| { | ||
| public ApiReferenceExportOptions(string packageName, string packageVersion, System.Collections.Generic.IReadOnlyCollection<string> exportingAssemblyNames) { } | ||
|
|
||
| public System.Collections.Generic.IReadOnlyCollection<string> ExportingAssemblyNames { get { throw null; } } |
There was a problem hiding this comment.
The previous commit had the exporter reuse options interface names recorded while resolving "the manifest", on the theory that a per-package export could then name RunAsEmulatorOptions the way full generation does. The manifest it was handed cannot contain the collision. `sdk export` builds a scanner app host referencing exactly the requested package plus the code generation package, so the scanned context is that package, core, and codegen -- never a sibling integration. Azure.EventHubs and Azure.ServiceBus are siblings; neither is in the other's closure, and each export runs in its own app host. Both resolves see one RunAsEmulator, find no collision, and assign the base name, which is exactly the state the previous commit claimed to fix. The test passed only because it fabricated a two-package manifest and handed it to the projector directly, proving the mechanism works while assuming an input the pipeline never produces. Worse, seeding did change one thing: with core in every manifest, an integration colliding with a core method now gets its name decided by scan order, and the core export -- resolved from a core-only manifest -- would not agree. That trades an unreachable fix for a reachable regression. The real constraint is that an options interface name is not a function of the package that owns it. It depends on what else was loaded, so `sdk generate` in two different app hosts already names the same package's interface differently. There is no single "generated SDK" for an export to match. Making names a function of package identity would fix that, but it renames types in emitted TypeScript and belongs in its own change. Keeping the two pieces that stand on their own: the unmatched package id now fails instead of publishing an empty document, and the three copies of the create-or-merge logic in RegisterOptionsInterface are one helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 93b90ae0-2187-486e-9bd2-a8ce41c09897
There was a problem hiding this comment.
Review details
Suppressed comments (3)
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:166
- The requested integration is pinned, but the TypeScript generator is still selected at the running CLI's version. An installed 13.5 CLI can therefore export
Aspire.Hosting.Redis@13.4.0using the bundled 13.5 core surface and 13.5 projection rules; running the same request with a 13.4 CLI can produce different declarations even though both documents are keyed only as Redis 13.4.0. The checkout substitution guard does not run for the installed/prebuilt path. Either reject first-party package/CLI version skew, or make the generator/core version part of the restored and exported identity soName@Versiondetermines one canonical document.
var codeGenPackage = await GetCodeGenerationPackageAsync(language, cancellationToken);
if (codeGenPackage is not null)
{
integrations.Add(IntegrationReference.FromPackage(codeGenPackage, ExecutionContext.IdentityVersion));
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:400
- Writing directly to the destination truncates an existing export before the asynchronous write completes. Cancellation, disk-full, or another I/O failure can therefore leave a partial JSON file (and destroy a previously valid one), even though this command treats partial documents as unsafe to publish. Write to a uniquely created sibling file and atomically replace the destination only after the complete JSON has been flushed; clean up the temporary file on failure.
await File.WriteAllTextAsync(outputFile.FullName, json, cancellationToken);
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:75
- This new user-visible CLI workflow has only mocked command tests and in-process RemoteHost tests. Those cannot catch failures in the installed CLI's DI registration, real scanner restore/startup, JSON-RPC transport, or actual stdout/stderr redirection—the end-to-end contract this command exposes. Add a CLI E2E test under
tests/Aspire.Cli.EndToEnd.Teststhat runs the packaged PR CLI against a known package/source, redirects stdout, parses the exported JSON, and verifies the exit code and clean output streams.
protected override async Task<CommandResult> ExecuteAsync(ParseResult parseResult, CancellationToken cancellationToken)
- Files reviewed: 39/41 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Options interface names were a function of the scan: the first capability to claim a base name kept it, and the next incompatible one walked a counter to RunAsEmulator1Options. Two things fell out of that. Adding an unrelated integration to an app host could rename an interface the user's hand-written TypeScript refers to, because the name a capability got depended on which other packages were present and in what order they were scanned. And `sdk export` runs one app host per package, so Azure.EventHubs and Azure.ServiceBus -- whose runAsEmulator overloads take incompatible callback types -- each projected alone and each emitted RunAsEmulatorOptions. Concatenating those fragments redeclares one interface with conflicting members. Names are now derived from the assembly that exports the capability, so they are a function of the capability alone and a per-package projection agrees with a whole-app-host projection by construction. Aspire.Hosting keeps unqualified names; every other assembly contributes a qualifier (Aspire.Hosting.Azure.EventHubs -> AzureEventHubsRunAsEmulatorOptions). The suffix loop stays as an intra-assembly last resort, where iteration order is the same subsequence in both views. Options interfaces also now go through the same ownership gate as builders, entry points, enums and DTOs, and their declaration fragments are keyed by the owning assembly rather than by the requesting package. Keying by the requester gave one interface a different fragment id in every export that reached it, so concatenation redeclared it instead of deduplicating it. Snapshot churn: 10 of 101 options interfaces in TwoPassScanningGeneratedAspire.verified.ts are renamed, all of them owned by the test fixture assembly. The 91 owned by Aspire.Hosting are unchanged. No app host under playground/ or tests/PolyglotAppHosts/ names an options interface -- they all pass object literals -- so the rename does not reach checked-in TypeScript. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| private TypeScriptApiItem ProjectEntryPoint(TypeScriptApiPackageIdentity package, AtsCapabilityInfo capability) | ||
| { | ||
| var member = ProjectMethod(package.Name, builderModel: null, capability); |
There was a problem hiding this comment.
Confirmed and fixed in e30db3c. Both paths now go through a single ResolveEntryPointSignature, so the declaration carries client: AspireClientRpc first and keeps ATS optionals positional; ApiExportDeclaresEntryPointsWithTheSignatureTheGeneratorEmits asserts the exported declaration against the emitted export async function text. Reverting to ResolveMethodSignature reproduces function startThing(name: string, options?: ...).
|
Heads up on the four red Azure jobs — they're inherited from main, not from anything here. Main tip #19148 is already open to fix it. These jobs should clear on the next run once it lands. The earlier E2E reds were separate and transient — four jobs failed in |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Here are all the things I checked to ensure this is the correct design. Everything looks good, good job! Q: What existing command exports the ATS API surface? A: Q: Can the existing command output JSON? A: Yes: Q: Should this PR use A: Probably not. The PR exports the final language-specific generated API, not raw ATS. A separate Q: Does the PR support other polyglot languages? A: Not currently. Only TypeScript implements Q: Why are custom exporter implementations required? A: Each generator applies different naming, type mapping, nullability, options, async, fluent-wrapper, module, and declaration rules. Raw ATS does not describe the final generated API. Q: Is language an argument to the new CLI command? A: Yes, and it is required: aspire sdk export --language typescriptThe short form is Q: What happens when another language is requested? A: A known generator without an exporter fails with an Q: What is the generated JSON shape? A: It is a versioned document containing Q: Is the JSON schema different for each language? A: The current contract allows each language provider to define its own schema. Only TypeScript defines one today. Q: Could Aspire use a common schema? A: Yes, and that is likely preferable for a unified polyglot documentation pipeline. Exporters would remain language-specific but map into shared DTOs, with language-specific syntax stored in declaration or extension fields. Defining this contract now would avoid incompatible schemas later. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
The determinism tests added with the owning-assembly naming change build projectors over raw hand-made contexts, but `sdk export` never hands the projector a raw scan: `FilterForApiExport` narrows it to the requested package first. That narrowing is the whole bug, so the property was proven about the projector rather than about the context the CLI produces. This goes through the filter and compares the export against the source the generator actually emits. Both packages are covered because only one of them fails under the old scheme, and it is not the obvious one: Event Hubs is scanned first and kept the unsuffixed base name in both views, so it agreed by luck. Comparing interface bodies rather than names is what makes the failure visible. Asserting only that the name appears in the generated source passes under the old scheme too, because the name did appear there -- it just belonged to the other package. Service Bus exported `RunAsEmulatorOptions` as `configureContainer?: boolean` while the SDK gave that same name to Event Hubs as `configureContainer?: string`, so a consumer concatenating the export got the wrong shape instead of a redeclaration error. Verified by reproducing the old naming locally: the Service Bus case fails with `Expected: "configureContainer?: boolean" / Actual: "configureContainer?: string"`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 93b90ae0-2187-486e-9bd2-a8ce41c09897
The remark described the intermediate version of the test rather than the committed one. It said only Service Bus fails under the old scheme and that Event Hubs "agreed by luck", which was true of the name-containment check I started with but not of the test as it stands: with the literal name assertions in place both rows fail, just at different assertions, Event Hubs on the name and Service Bus on the body. That mattered because a future reader reproducing the old-scheme experiment against the committed test would have seen Event Hubs fail and concluded the comment was wrong. The asymmetry is still the reason the body comparison exists, so the remark now explains it that way instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 93b90ae0-2187-486e-9bd2-a8ce41c09897
There was a problem hiding this comment.
Review details
Suppressed comments (4)
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:166
- This code-generation package also determines the exported TypeScript surface, but a bare version is a NuGet minimum and may restore a newer generator when this exact CLI version is unavailable. The result would then be produced by a different generator while retaining the requested package label. Pin the code-generation package exactly on the export path as well.
var codeGenPackage = await GetCodeGenerationPackageAsync(language, cancellationToken);
if (codeGenPackage is not null)
{
integrations.Add(IntegrationReference.FromPackage(codeGenPackage, ExecutionContext.IdentityVersion));
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:156
- An installed 13.5 CLI accepts a first-party package such as
Aspire.Hosting.Redis@13.4.0, then projects that exact assembly with the 13.5 core and TypeScript generator. Generator behavior is versioned too (this PR itself renames generated options interfaces), so the JSON does not represent the TypeScript surface emitted by the 13.4 SDK even though it is labeled 13.4. Reject first-party package/CLI version skew as the core-package path does, or load the matching core and generator versions.
// Pin the requested version: a bare NuGet version is a minimum, so an unavailable
// version would restore as a later one and be published under the wrong number.
// Use packageName rather than reference.Name so the restored reference and the
// exported label can never name the package differently.
integrations.Add(IntegrationReference.FromExactPackage(packageName, reference.Version));
src/Aspire.Cli/Configuration/IntegrationReference.cs:99
- When exactness is requested, an explicit non-singleton range such as
[13.2.0,13.3.0)is returned unchanged, soFromExactPackagecan still float despite its contract. Passing through[13.2.0]is fine, but range/floating expressions should be rejected wheneverforceExactorRequireExactVersionis true.
// An explicit range the caller already wrote (`[1.2.3]`, `(1.0,2.0)`) is left alone: wrapping
// it again would produce a syntactically invalid range.
if (!(forceExact || RequireExactVersion) || Version.Length == 0 || Version[0] is '[' or '(')
{
return Version;
src/Aspire.Cli/Commands/Sdk/SdkCommandPreparation.cs:163
- If validation rejects this project, ownership never reaches
AppHostServerSession, so anIDisposableimplementation such asPrebuiltAppHostServeris not disposed and its bundle layout lease remains held. The same applies when preparation returns failure before a session is created. Track project ownership and dispose it on every pre-session exit/exception, transferring ownership only after the session is created.
var appHostServerProject = await appHostServerProjectFactory.CreateAsync(tempDir, cancellationToken);
if (validateProject?.Invoke(appHostServerProject) is string rejection)
{
interactionService.DisplayError(rejection);
return null;
- Files reviewed: 40/44 changed files
- Comments generated: 1
- Review effort level: Balanced
| if (char.IsLetterOrDigit(character)) | ||
| { | ||
| qualifier.Append(character); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fixed in e30db3c: the qualifier now encodes separators instead of dropping them (. -> _, literal _ doubled, anything else _x<hex>), which is reversible and therefore injective. OptionsInterfaceQualifiersDistinguishAssembliesThatDifferOnlyBySeparatorPlacement covers your Contoso.Foo.Bar / Contoso.FooBar case and reproduces ContosoFooBar when the fix is reverted.
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:166
- The requested package is pinned, but its TypeScript projector is always restored at the running CLI's version. Therefore the same
Package@Versionproduces different JSON depending on which CLI runs the command. This is already observable in this branch: the new projector qualifies non-core options names (for exampleRedisWithDataVolumeOptions), whereas an older CLI generated the unqualified name. A 13.5 CLI exportingAspire.Hosting.Redis@13.4.0consequently labels 13.5 projection rules as 13.4. Either require a compatible CLI/code-generator version, restore the generator that belongs to the exported SDK, or include the generator version in the exported identity.
integrations.Add(IntegrationReference.FromPackage(codeGenPackage, ExecutionContext.IdentityVersion));
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiExportWriter.cs:130
- An obsolete capability without a message is serialized as if it were not obsolete. The projector deliberately uses
nullfor active members andstring.Emptyfor[Obsolete]without a message, butAddIfPresentdrops both null and empty strings. Preserve the emptydeprecatedfield so consumers can distinguish those states.
AddIfPresent(json, "deprecated", member.DeprecationMessage);
- Files reviewed: 40/44 changed files
- Comments generated: 1
- Review effort level: Balanced
| var qualifier = new StringBuilder(remainder.Length); | ||
| foreach (var character in remainder) | ||
| { | ||
| if (char.IsLetterOrDigit(character)) | ||
| { | ||
| qualifier.Append(character); | ||
| } | ||
| } | ||
|
|
||
| return qualifier.Length == 0 ? string.Empty : ToPascalCase(qualifier.ToString()); |
There was a problem hiding this comment.
Fixed in e30db3c: a qualifier starting with a digit is now prefixed with _, which cannot alias a name that already begins with an underscore because a literal _ encodes as a doubled one. Covered by OptionsInterfaceQualifiersEscapeAssemblyNamesThatStartWithADigit.
The capability ownership map, method registry, and property registry are all populated while assemblies are scanned, before FilterInvalidCapabilities and FilterMethodNameCollisions run. Those filters drop capabilities but cannot reach the registries, so an assembly whose every capability was filtered out stayed named by them. That is a silent failure rather than a leak. TryResolveCanonicalAssemblyName resolves a requested package against the assembly names these registries carry, so such a package resolved, filtered to nothing, and let `sdk export` publish an empty API document under a successful exit code. Pruning the ownership map alone is not sufficient: the method and property registries reach the same assembly through their declaring types, so canonicalization still succeeded. All three are pruned together. Removing the entries is safe because every consumer reaches them by the capability id of a capability it already holds. Both scan paths prune. Only the multi-assembly path -- the one `sdk export` uses -- can actually go stale today, since a capability id is `package/methodName` and two capabilities in one assembly cannot share a method name without sharing an id. The single-assembly path prunes for symmetry because it runs the same shared filters, and is covered by an invariant test rather than a reproduction. Also reverts the hand-edit to src/Aspire.TypeSystem/api/Aspire.TypeSystem.cs. Files under */api/*.cs are generated and regenerated by the release process, not edited in individual PRs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:166
- This leaves the TypeScript code-generator package as a NuGet minimum even though that package defines every exported name and signature. If the matching version is absent but a later version is available, NuGet silently upgrades the generator and the command publishes that later projection under the requested package's unchanged
Name@Version; the document does not record a generator version. Pin the generator package (and update the test that currently asserts it is floating), or include and validate the generator identity as part of the export contract.
var codeGenPackage = await GetCodeGenerationPackageAsync(language, cancellationToken);
if (codeGenPackage is not null)
{
integrations.Add(IntegrationReference.FromPackage(codeGenPackage, ExecutionContext.IdentityVersion));
tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/WithDataVolumeOptionsMerged.verified.ts:1
- This snapshot is not byte-identical: it renames an exported TypeScript interface from
WithDataVolumeOptionstoCodeGenerationTypeScriptTestsWithDataVolumeOptions, contrary to the PR description's “no behavior change” claim. Because consumers can name these exported interfaces directly, either preserve the existing generated names where there is no collision or update the PR contract and migration expectations to acknowledge the source-breaking generator change.
- Files reviewed: 40/44 changed files
- Comments generated: 1
- Review effort level: Balanced
| { | ||
| var isCorePackage = string.Equals(packageName, CorePackageName, StringComparison.OrdinalIgnoreCase); | ||
|
|
||
| if (isCorePackage && ExecutionContext.IdentityOverridden) |
There was a problem hiding this comment.
Confirmed and fixed in e30db3c. IdentityVersionForged now tracks ASPIRE_CLI_VERSION specifically, so a sidecar-supplied version no longer trips the core guard; SdkExportOfTheCorePackageAcceptsASidecarSuppliedIdentity fails with exit code 1 when the check is reverted to the aggregate.
Options interface qualifiers were not injective. The qualifier kept only letters and digits, so Contoso.Foo.Bar and Contoso.FooBar both produced ContosoFooBar. A per-package export cannot see that another package lands on the same qualifier, so it has no chance to disambiguate the way full generation would; both packages emit a ContosoFooBarRunAsEmulatorOptions with different members and concatenating their fragments fails to compile. The separator is now encoded rather than dropped, which is reversible and therefore injective. An assembly name beginning with a digit also produced an unparseable TypeScript identifier, so that case is escaped. Entry points were exported with the wrong signature. ProjectEntryPoint routed through the member signature resolver, which drops the client parameter and folds optionals into an options bag, while GenerateEntryPointFunction emits a free function taking the client first and keeping optionals positional. Consumers type-check against the exported declarations, so the two disagreeing published declarations that describe no callable function. Both paths now resolve through ResolveEntryPointSignature. The core export was gated on IdentityOverridden, an aggregate that is true whenever any identity field came from an environment variable or the install sidecar. Every install route writes a sidecar carrying channel and version, so the aggregate is set on ordinary installs and the guard rejected exactly the CLIs its own error message told callers to use. Only a version supplied by ASPIRE_CLI_VERSION makes the label unverifiable, so IdentityVersionForged tracks that specifically. Each fix has a test that fails without it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Suppressed comments (3)
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:166
- The exported package is pinned to
packageVersion, but its TypeScript surface is projected by the code-generation package at the running CLI'sIdentityVersion. Generator behavior is version-dependent (this PR itself changes options-interface naming), so a newer CLI can exportAspire.Hosting.Redis@13.4.0using 13.5 generator semantics and publish that result under the 13.4 package key. Either constrain exports to the matching SDK/generator version or include and validate the generator identity in the export contract; pinning only the integration does not make the documented TypeScript API canonical forName@Version.
// The code generator lives in a separate package that the scanner AppHost does not reference
// by default, so without this the server loads no generators and every export fails with
// "No code generator found". `sdk generate` adds the same package for the same reason.
var codeGenPackage = await GetCodeGenerationPackageAsync(language, cancellationToken);
if (codeGenPackage is not null)
{
integrations.Add(IntegrationReference.FromPackage(codeGenPackage, ExecutionContext.IdentityVersion));
src/Aspire.Hosting.CodeGeneration.TypeScript/TypeScriptApiExportWriter.cs:130
- An obsolete capability with no message is projected as
DeprecationMessage = string.Empty, butAddIfPresentdrops empty strings. The generated TypeScript emits a bare@deprecatedtag for this case, while the canonical JSON omitsdeprecatedentirely and documentation consumers treat the API as current. Preserve any non-null deprecation value, including the empty string.
AddIfPresent(json, "deprecated", member.DeprecationMessage);
src/Aspire.Cli/Commands/Sdk/SdkExportCommand.cs:81
- Routing
InteractionServicehere happens afterProgram.DisplayFirstTimeUseNoticeIfNeededAsync. That startup path only recognizes--format json/--jsonas machine-readable, so a first interactivesdk exportcan write the welcome banner to stdout before this method runs (and--outputcan unexpectedly write banner text to stdout too). Register thesdk exportcommand shape inHasMachineReadableOutputand cover it inRootCommandTests, so the pure-JSON stdout contract holds before command execution begins.
// This command always emits machine-readable JSON, so it has no --format option for
// BaseCommand's json redirect to key off. Without this, preparation diagnostics and the
// --output success message land on stdout and corrupt the document a caller is piping.
// The JSON write overrides back to stdout explicitly, and an explicit override wins.
InteractionService.Console = ConsoleOutput.Error;
- Files reviewed: 42/46 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Adds
aspire sdk export, which emits the TypeScript API surface for an exactName@Versionas JSON on stdout. This is the producer half of fixing #17608 — aspire.dev currently reconstructs TypeScript signatures itself inAtsJsonGenerator, so what the docs show drifts from what the generator actually emits.The idea is that the generator becomes the only thing that decides what TypeScript looks like, and the docs site just renders what it's handed.
$ aspire sdk export --package Aspire.Hosting.Redis@13.5.0 --language typescript > redis.api.jsonPure JSON on stdout, logs on stderr, so it pipes cleanly.
What's here
TypeScriptApiProjector— pulled the projection logic out of the code generator so the export and the generated.tscome from one place. 73 members moved, no behavior change (the.verified.tssnapshots are byte-identical across the whole branch).IApiReferenceExporter/ApiReferenceExportOptionsinAspire.TypeSystem, plus anexportApiRPC on RemoteHost.sdk exportin the CLI.owningAssembly, and every declaration fragment is self-contained, so a consumer can concatenate the fragments for a package plus its closure and get something that type-checks with no site-authored shims.Bugs this turned up
Writing the tests first shook out four real ones, all with regression coverage:
AtsContextFilterreference closure — owned types were seeded into the included sets before the walk, soCollectReferencedType's "did I just add this?" guard short-circuited and never walked their members. Any filteredgenerateCodethrew onHealthStatus. This one predates the branch.FooHandleviaGetWrapperOrHandleName, but the stub pass derived a class name instead. So we declared a symbol nobody referenced and left the referenced one dangling. 7 unresolved names when you runtscover the real output.sdk exportcouldn't find a code generator — the scanner AppHost doesn't referenceAspire.Hosting.CodeGeneration.TypeScript;sdk generateadds it viaILanguageDiscoveryand export didn't. Every invocation failed until I wired it up.addRedisshipped asinterface:DistributedApplicationBuilderowned byAspire.Hosting.Redis, which collides with core's item ID across a manifest and misattributes a core-owned type. Nowaugmentation:{name}with the real owner.Making the version label true
The version on an export document is what consumers key published documentation on, and review surfaced that nothing was making it true.
sdk export Package@Versioncould publish a surface that was not that version's, two ways:DotNetBasedAppHostServerProject, which replaces every first-partyAspire.Hosting.*package reference with the matching project undersrc/and throws the requested version away. Asking a 13.5.0 checkout for 13.4.0 exported the checkout under the older number.Both are now narrow contracts on the export path.
IntegrationReferencecarriesRequireExactVersion, andsdk exportsets it on the requested package so an unavailable version fails with NU1102 instead of resolving upward.IAppHostServerProject.GetLocalProjectSubstitutionreports the substitution a checkout would make, andsdk exportrefuses a version the checkout would not actually produce. Same-version local development, third-party exports,--sourcepinning, the core-package guard, and code generator restoration are unchanged, and run/dump keep the minimum-version form that lets transitive dependencies unify.Three more holes turned up while writing that:
src/<name>/<name>.csprojfor everyAspire.Hosting*reference and, when that project was absent, dropped the reference outright.sdk export --package Aspire.Hosting.DoesNotExist@13.5.0-devrestored cleanly and published an empty module under a package id that has never existed. It now falls back to an exact package reference, which fails with NU1101 as it should.IdentitySdkVersion, whichASPIRE_CLI_VERSIONand the install sidecar exist to override, soASPIRE_CLI_VERSION=99.0.0published the current Redis source as 99.0.0. The checkout now states its own version line fromeng/Versions.props, and an identity override, an unreadable version line, or a mismatch on either half rejects. Overrides keep working everywhere no substitution happens.[13.4.0]explains on sight.PackageVersiononApiReferenceExportOptionstook the documentation half rather than the validation half. An exporter sees loaded assemblies, not the package resolution that produced them, so the DTO cannot tell an exact-but-wrong version from a right one. Exactness belongs where the restore is decided; the docs say the caller owns accuracy and point atsdk export, which rejects a floating or range version before the scanner is built.sdk dumpkeeps requested-version semantics deliberately: it restores with a minimum-version reference, and--format ci— the format the checked-in*.ats.txtbaselines use — carries no package versions at all, so nothing version-keyed is published from it. That distinction is now stated in code, in the output-format spec, and in a test.Testing
Aspire.Hosting.CodeGeneration.TypeScript.Tests103/103,Aspire.Hosting.RemoteHost.Tests467/467,Aspire.Cli.Tests4801/4801 (33 skipped)Aspire.Hosting, 27 KB forAspire.Hosting.Redis, empty stderr, exit 0tsc --noEmit --strictwithskipLibCheckoff — 0 errorsproject.assets.json), and the exact reference fails with NU1102. Covered on both the repository scanner and the prebuilt closure paths, since the package-only install path never touches the scanner.The generated
Aspire.TypeSystemAPI baseline is regenerated via GenAPI, additive only.Consumer side is microsoft/aspire.dev — it validates this exact output against fixtures produced by this branch.
Contributes to #17608