Flatten single optional DTO 'options' parameter in Go polyglot generator#18698
Conversation
When an exported C# API's only optional parameter is a DTO named 'options', the Go generator now threads the DTO directly (options ...*HostedAgentOptions) instead of wrapping it in a generated method-options struct, matching the TypeScript generator's flattened shape (issue #17664). The guard is stricter than TypeScript because Go permits only one trailing variadic: flatten only when the optional set is exactly one non-callback, non-cancellation-token DTO named 'options'. The RPC payload is unchanged -- both paths emit reqArgs["options"] = serializeValue(merged) for the same DTO. - Add TryGetDirectOptionsParameter helper used at all four decision points - Skip wrapper name reservation, wrapper struct emission, and use the DTO type directly in the parameter list and args construction for direct-options - Regenerate TwoPassScanning Go snapshot - Add focused positive/negative unit tests - Update Aspire.Hosting and Foundry Go polyglot validation fixtures to the flattened call shape 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 -- 18698Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18698" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR teaches the Go polyglot code generator to "flatten" a capability whose only optional parameter is a DTO named options. Instead of wrapping the DTO in a generated method-options struct (options ...*AsHostedAgentOptions{ Options: ... }), the generator now threads the DTO type directly (options ...*HostedAgentOptions), matching the shape already produced by the TypeScript generator (issue #17664). The guard is intentionally stricter than TypeScript's because Go models all optionals as a single trailing variadic, so flattening applies only when there is exactly one optional DTO named options with no coexisting cancellation token or callback. The emitted RPC payload is unchanged — the merged DTO is still sent under the original options argument name.
Changes:
- Added
TryGetDirectOptionsParameterguard and wired it into name reservation, parameter-list rendering, args construction, and options-struct generation inAtsGoCodeGenerator.cs. - Regenerated the
TwoPassScanningsnapshot (wrapper structs removed, call sites now serialize the merged DTO directly underoptions). - Added two unit tests (flatten happy-path + negative case where
optionscoexists with other optionals) and updated theAspire.HostingandAspire.Hosting.FoundryGo fixtures to the flattened call shape.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting.CodeGeneration.Go/AtsGoCodeGenerator.cs | Core change: adds the flattening guard and applies it consistently across name reservation, signature/args emission, and struct generation. |
| tests/Aspire.Hosting.CodeGeneration.Go.Tests/AtsGoCodeGeneratorTests.cs | Adds positive (flatten) and negative (no-flatten) unit tests. |
| tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.go | Regenerated snapshot reflecting flattened signatures and removed wrapper structs. |
| tests/PolyglotAppHosts/Aspire.Hosting/Go/apphost.go | Updated consumer calls to the flattened DTO shape. |
| tests/PolyglotAppHosts/Aspire.Hosting.Foundry/Go/apphost.go | Updated AsHostedAgent call to pass HostedAgentOptions directly. |
Extract the `TryGetDirectOptionsParameter` decision into a shared internal helper in Aspire.TypeSystem, parameterized by a cancellation-token predicate and a CoexistingCancellationTokenPolicy so each generator keeps its exact semantics. Go and TypeScript now delegate to it via thin wrappers; emission stays per-language. Behavior-preserving: Go (25) and TypeScript (99) generator tests pass with unchanged snapshots. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Relocate AtsOptionsFlattening from Aspire.TypeSystem to src/Shared/CodeGeneration and consume it via <Compile Include> in both generators, instead of exposing it through InternalsVisibleTo on the version-frozen contract assembly. Replace the CoexistingCancellationTokenPolicy enum with a cancellationTokenIsSeparateParameter bool so both values drive the single behavioral fork. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Match the wrapper path's conditional ToMap() emission so a lone WithFoo(nil)
call sends no "options" key instead of an empty "options: {}", keeping the
RPC payload identical to the pre-flattening wrapper output.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
joperezr
left a comment
There was a problem hiding this comment.
Left super small nit, but otherwise this looks good to go. The new flattened usage looks much better :). I'm sure that this change will trigger our docs workflow to add a breaking change on the what's new for 13.5, but if it doesn't, make sure we track and add it somewhere as this is obviously a breaking change for folks that are using the Go apphost (which is experimental, so it's fine to make this change.)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 3 / 99 test projects · 4 jobs, from 9 changed files. Selected test projects (3 / 99)
Selected jobs (4)
How these were chosen — grouped by what changed🔧 🔧 🔧 🔧 🧪 🧪 Job reasons
Selection computed for commit |
|
✅ No documentation update needed. docs_optional → internal_refactor No signals triggered (signal_count = 0). This PR refactors the Go polyglot code generator ( |
|
The CI build failed due to test failure(s) that appear unrelated to the PR changes. These may be flaky tests. Suspected flaky test(s):
Suggested actions:
You can re-run the failed jobs from the workflow run page. |
Adds a Group A path signal (polyglot_code_generator_changed) so changes under src/Aspire.Hosting.CodeGeneration.* and src/Shared/CodeGeneration/ gate docs review. These generate the polyglot SDK source consumers write against, so a change there can be breaking even when the C# API and RPC payload are unchanged. Closes the blind spot from #18698, which was misclassified as internal_refactor (signal_count == 0) because no signal covered generated polyglot output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds a Group A path signal (polyglot_code_generator_changed) so changes under src/Aspire.Hosting.CodeGeneration.* and src/Shared/CodeGeneration/ gate docs review. These generate the polyglot SDK source consumers write against, so a change there can be breaking even when the C# API and RPC payload are unchanged. Closes the blind spot from #18698, which was misclassified as internal_refactor (signal_count == 0) because no signal covered generated polyglot output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Add polyglot code-generator signal to pr-docs-check catalog Adds a Group A path signal (polyglot_code_generator_changed) so changes under src/Aspire.Hosting.CodeGeneration.* and src/Shared/CodeGeneration/ gate docs review. These generate the polyglot SDK source consumers write against, so a change there can be breaking even when the C# API and RPC payload are unchanged. Closes the blind spot from #18698, which was misclassified as internal_refactor (signal_count == 0) because no signal covered generated polyglot output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Match previous_filename for renamed files in Group A path triggers GitHub reports a rename with the destination in `filename` and the source in `previous_filename`. The Group A path-trigger loop matched only `filename`, so relocating a watched file to an untracked path (an outbound rename) slipped through as signal_count == 0 even though the `any` status filter is meant to include renames. The loop now also matches `previous_filename` for renamed entries and records the path that actually matched. Adds an outbound-rename regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ci, hono, protobufjs, and more) (#18735) * [auto-sec] Consolidate npm security remediations across all frontend manifests Addresses 100+ Dependabot security alerts across 29 npm/yarn/pnpm manifests in playground/, src/Aspire.Cli/Templating/Templates/, src/Aspire.ProjectTemplates/, tests/, and extension/. ## Packages fixed (via overrides/resolutions/pnpm.overrides or direct version bumps): ### CVE remediations - vite: bumped to ^8.0.16 (was ^8.0.0) — CVE-2026-53571 (high), CVE-2026-53632 (medium) - vite 7.x projects: override to 7.3.5 - vite 6.x projects: bumped to ^6.4.3 - @angular/core + @angular/common + @angular/compiler + Angular ecosystem: - playground/AspireJavaScript.Angular: pinned all @angular/* to 21.2.17 via overrides — CVE-2026-54266/54267/54268 (high), CVE-2026-54265 (medium) - hono: 4.12.25 — CVE-2026-54286/54287/54288/54289/54290 - http-proxy-middleware: 3.0.7 — CVE-2026-55602 (medium), CVE-2026-55603 (high) - piscina: 5.2.0 — CVE-2026-55388 (high) - webpack-dev-server: bumped to ^5.2.5 — CVE-2026-9595 (medium) - undici: 7.28.0 (npm/yarn), 6.27.0 (yarn test) — multiple CVEs (high/medium/low) - js-yaml: 4.2.0 — CVE-2026-53550 (medium) - @babel/core: bumped to ^7.29.6 — CVE-2026-49356 (low) - protobufjs: 8.6.0 — CVE-2026-48712/54269/54270 (high/medium) - @opentelemetry/core: 2.8.0 — CVE-2026-54285 (medium) - launch-editor: 2.14.1 — CVE-2026-53632 (medium) - markdown-it: 14.2.0 — CVE-2026-48988 (medium) - form-data: 4.0.6 — CVE-2026-12143 (high) - tmp: 0.2.7 — CVE-2026-49982 (high) - tar: 7.5.16 — CVE-2026-53655 (medium) - esbuild: 0.28.1 — GHSA-g7r4-m6w7-qqqr (low) - brace-expansion: 5.0.7 — moderate vuln in py-starter/ts-starter templates ### Lockfile regeneration - All package-lock.json files regenerated with 'npm install --package-lock-only' - extension/yarn.lock regenerated with yarn v1 (resolutions applied) - NodeFrontend pnpm-lock.yaml regenerated with pnpm - tests/PolyglotAppHosts pnpm-lock.yaml regenerated Note: tests/PolyglotAppHosts/TypeScript.PackageManagers.Yarn yarn.lock has resolutions added to package.json; lockfile will be regenerated on next 'yarn install' run (requires Yarn v4). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * [auto-sec] Align E2E test with undici 7.28.0 lock Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Regenerate Yarn fixture lockfile Update the Yarn 4 lockfile to reflect the security resolutions for esbuild, tar, and undici so immutable installs succeed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ba8c860a-2cf2-4af4-8297-79de367a57a9 * Add polyglot code-generator signal to pr-docs-check catalog (#18766) * Add polyglot code-generator signal to pr-docs-check catalog Adds a Group A path signal (polyglot_code_generator_changed) so changes under src/Aspire.Hosting.CodeGeneration.* and src/Shared/CodeGeneration/ gate docs review. These generate the polyglot SDK source consumers write against, so a change there can be breaking even when the C# API and RPC payload are unchanged. Closes the blind spot from #18698, which was misclassified as internal_refactor (signal_count == 0) because no signal covered generated polyglot output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Match previous_filename for renamed files in Group A path triggers GitHub reports a rename with the destination in `filename` and the source in `previous_filename`. The Group A path-trigger loop matched only `filename`, so relocating a watched file to an untracked path (an outbound rename) slipped through as signal_count == 0 even though the `any` status filter is meant to include renames. The loop now also matches `previous_filename` for renamed entries and records the path that actually matched. Adds an outbound-rename regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Address npm remediation review feedback Align tsx and esbuild compatibility, scope Angular transitive overrides, and refresh affected npm, Yarn, pnpm, and Bun locks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c9776bc3-78cf-43d0-89c1-8727485c2dc3 --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Ella Hathaway <67609881+ellahathaway@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description
When a C# exported API's only optional parameter is a DTO named
options, the Go polyglot generator now threads the DTO type directly (options ...*HostedAgentOptions) instead of wrapping it in a generated method-options struct (options ...*AsHostedAgentOptions{ Options: ... }). This mirrors the already-flattened shape emitted by the TypeScript generator.The guard (
TryGetDirectOptionsParameter) is intentionally stricter than TypeScript's: Go models all optionals as a single trailing variadic and permits only one, so flattening applies only when the optional set is exactly one DTO namedoptionswith no coexisting cancellation token or callback. The RPC payload is unchanged — the merged DTO is still sent under the originaloptionsargument name, byte-identical to the previous wrapped form.Changes:
AtsGoCodeGenerator.cs: added theTryGetDirectOptionsParameterguard, wired into name reservation,RenderParameterList,EmitArgsConstruction, andGenerateOptionsStructs.TwoPassScanningsnapshot with flattened output.Aspire.HostingandAspire.Hosting.FoundryGo polyglot fixtures to the flattened call shape.Validated with a local
go build ./...of the fullAspire.Hostingfixture (EXIT=0), confirming all flattened call sites are valid Go.Fixes #17664
Checklist
<remarks />and<code />elements on your triple slash comments?