Repository navigation
[release/17.0] Propagate Go child resource errors through Build - #22057
Closed
David Pine (IEvangelist) wants to merge 2 commits into
Closed
David Pine (IEvangelist) wants to merge 2 commits into
David Pine (IEvangelist) wants to merge 2 commits into
Conversation
github-actions
Bot
added
the
needs-area-label
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 22057Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 22057" |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The error formatter drops all but the first capability failure when a joined Build error is wrapped with context, requiring traversal through single-error wrappers and a regression test for wrapped aggregates.
1 open issue
What changed in this PR
Backports Go child-resource error propagation to release/17.0, so generated AppHosts report recorded resource failures through the builder’s Err() and Build(). The change excludes unrelated runtime hardening from the source PR.
Changes:
- Synchronizes wrapper error state and aggregates failures without duplicating propagated errors.
- Updates generated error checks and formatting for joined failures.
- Adds runtime regression tests and regenerates release-specific snapshots.
| File | Description |
|---|---|
tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/EscapedMetadataGeneratedAspire.verified.go |
Updates escaped-metadata error checks. |
tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/AtsGeneratedAspire.verified.go |
Captures generated aggregation and runtime changes. |
tests/Aspire.Hosting.CodeGeneration.Go.Tests/AtsGoCodeGeneratorTests.cs |
Adds propagation, formatting, concurrency, and successful-build tests. |
src/Aspire.Hosting.CodeGeneration.Go/Resources/transport.go |
Collects client errors and formats joined failures. |
src/Aspire.Hosting.CodeGeneration.Go/Resources/base.go |
Synchronizes wrapper errors and distinguishes owned from propagated failures. |
src/Aspire.Hosting.CodeGeneration.Go/AtsGoCodeGenerator.cs |
Emits aggregate builder checks and propagation-aware resource methods. |
Review effort: Balanced
| if !errors.As(err, &capErr) { | ||
| return err.Error() | ||
| } | ||
| if joined, ok := err.(interface{ Unwrap() []error }); ok { |
Member
Author
There was a problem hiding this comment.
Addressed in #22201: FormatError now walks single-error wrappers to find the joined errors.
This comment has been minimized.
This comment has been minimized.
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
David Pine (IEvangelist)
force-pushed
the
dapine/backport-21969-release-17.0
branch
from
November 10, 2026 13:13
16611d5 to
487913f
Compare
David Pine (IEvangelist)
requested review from
James Newton-King (JamesNK),
Adam Ratzman (adamint),
David Negstad (danegsta),
David Fowler (davidfowl),
Eric Erhardt (eerhardt),
Ella Hathaway (ellahathaway),
Karol Zadora-Przylecki (karolz-ms),
Mitch Denny (mitchdenny),
Ankit Jain (radical) and
Sébastien Ros (sebastienros)
as code owners
November 10, 2026 13:13
David Pine (IEvangelist)
changed the base branch from
release/17.0
to
mitchdenny-backport-17-0-55-pr21890
November 10, 2026 13:14
Contributor
Tests selector4 / 101 PR test projects · 4 PR jobs, from 8 changed files. Selected PR test projects (4 / 101)
Selected PR jobs (4)
How these were chosen — grouped by what changed🔧 📦 affected project 🔧 🔧 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
4 of 14 tasks
James Newton-King (JamesNK)
approved these changes
Nov 10, 2026
## Description Generated Go AppHosts can run callbacks concurrently with the main builder chain without racing the sticky error or lazy collection accessors. Invalid request arguments and unencodable callback results now fail their individual calls instead of disconnecting every pending operation. Callback decoding errors and `runtime.Goexit` produce explicit JSON-RPC errors; message headers and cancellation requests are bounded. The Go API export also includes structured `type` and `optional` metadata for every field and constant. DTO optionality comes from ATS metadata, not the generated `omitempty` tag or pointer spelling. This is additive to schema version 1 and includes options, runtime fields, enum constants, and exported value leaves. Export snapshots and the output-format documentation are updated. Related to #21204, #21212, and #21115. This does not close either follow-up issue. The remaining structured Kubernetes manifest work is separate in #21824. ### User-facing usage Collection serialization preserves its existing signature; failures are now observable through the additive `Err` methods: ```go payload := resource.GetTags().ToJSON() if err := resource.GetTags().Err(); err != nil { return err } _ = payload ``` `aspire sdk export --language go --package Name@Version` now emits fields such as: ```json { "name": "Mode", "type": "*TestPersistenceMode", "optional": true } ``` Cancellation watchers are armed only after the invocation carrying their IDs has entered the FIFO write queue. The cancellation RPC has a five-second timeout; a canceled call still returns through its own context without waiting for that RPC. ### Validation Commands ran on Windows with the restored repository SDK and an isolated Go 1.27.3 toolchain added to `PATH`. Linux race checks used Go 1.26.10 and GCC from the existing local `aspire-go-e2e-6972:latest` image. | Command | Result | | --- | --- | | `.\restore.cmd` | Passed; 17 dependency audit warnings, no errors. | | `dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` | Passed: 130 tests, no failures or skips. Reviewed and accepted generated Go/JSON snapshot changes. | | `dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-method "*.GeneratedRuntime_PassesGoRuntimeHardeningTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` | Passed after adding the final actual-getter-failure assertions. | | `go test -race -timeout 60s -v .` | Passed in a generated module extracted from the Go snapshot, with `runtime_test.go`, `transport_hardening_test.go`, and `decode_test.go` from the C# tests. Includes a callback and main chaining on the same captured generated builder, encoding failures, callback errors/Goexit, header bounds, and cancellation ordering/timeout cleanup. | | `go test -race -timeout 60s -run TestCallbackAndMainCanChainOnCapturedBuilder .` | Failed as expected against the original main snapshot with data races; the same regression passes with this branch. | | `dotnet test --project tests\Aspire.Hosting.RemoteHost.Tests\Aspire.Hosting.RemoteHost.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` | Passed: 698 tests, 1 skipped. Includes integration scanning and Go export/codegen coverage. | | `dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.SdkExportCommandTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` | Passed: 31 tests. | | `dotnet build src\Aspire.Cli\Aspire.Cli.csproj --configuration Release /p:SkipNativeBuild=true` | Passed, with the existing ILCompiler audit warning. Managed Release build, not native AOT. | | `.\.dotnet\dotnet.exe artifacts\bin\Aspire.Cli\Release\net11.0\aspire.dll sdk export --help` | Passed; help lists Go as an export language. | | `git diff --check` | Passed. | ### Remaining policy and execution gaps - Callback registry release still needs a host release signal and a cross-language ownership contract. This PR does not unregister callbacks when a registration capability returns, because the host can invoke them later. - Disconnect-driven `os.Exit(1)` is unchanged. Choosing an observable shutdown contract requires a cross-language decision; no GA lifecycle guarantee is introduced. - The reported required-token path is currently unreachable because `SplitParameters` places all cancellation tokens in Go options. Both token-emission paths select the supplied token's context. - A bundled current-main CLI export measuring Redis/Azure Storage XML-summary coverage was not run. No documentation-copy policy was changed based solely on project-built behavior. - No new grouped export view was introduced; consumers still have `capabilityId` for grouping. The aspire.dev consumer and its grouping decision are outside this product-only change. - Native AOT publishing, the full CLI suite, a many-integration live AppHost, polyglot E2E, macOS, and the complete CI matrix were not run. Go feature-flag-off validation is obsolete because #21183 removed that flag. ### Security considerations This changes handling of AppHost JSON-RPC messages and callback payloads. Headers are limited to 16 entries and 8 KiB before body allocation, in addition to the existing 64 MiB body cap. Invalid typed callback arguments are rejected before user code is called; decoding no longer silently substitutes invalid DTO fields. No authentication, listener, or permission policy changes are made. A formal threat model/security review has not been performed. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [x] Yes - If yes, did you have an API Review for it? - [ ] Yes - [x] No - Did you add `<remarks />` and `<code />` elements on your triple slash comments? - [ ] Yes - [x] No - [ ] No - Does the change make any security assumptions or guarantees? - [x] Yes - If yes, have you done a threat model and had a security review? - [ ] Yes - [x] No - [ ] No The additive APIs are Go `List.Err`/`Dict.Err` and JSON export metadata; no public C# API was added. Go documentation comments describe the error contract. Triple-slash XML elements do not apply to these Go members. The fixes in this PR are complete; the explicitly deferred cross-language policy and bundle-validation items remain tracked in the related issues. --------- Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit e7ff435)
## Description
Generated Go AppHosts use fluent resource methods that return resource
handles and record failures on those handles. Previously, unchecked
child-resource failures could be missed, allowing `Build()` to proceed
with an invalid application model.
This change records one owned error per failed wrapper on the shared
generated client and combines all resource failures with `errors.Join`.
`DistributedApplicationBuilder.Err()` and `Build()` now return the
complete error summary, while independent resource additions continue
after an earlier child fails so every resource error can be collected.
Errors propagated through an already-failed chain are not recorded
again, preserving wrapper-local first-error semantics without
duplicating failures in the builder aggregate.
`FormatError` renders every joined `CapabilityError`, includes ordinary
errors in mixed aggregates, and preserves the original contextual
message for ordinary multi-wrapped errors. Error collection remains
synchronized for callbacks and concurrent callers, and the existing
cancellation behavior is unchanged.
### User-facing usage
All failures from fluent resource declarations are returned by the
existing `Build()` boundary:
```go
builder.AddRedis("")
builder.AddPostgres("")
app, err := builder.Build()
if err != nil {
log.Fatal(aspire.FormatError(err))
}
```
The formatted error includes both resource failures, and the build
capability is not invoked after errors have been collected.
### Validation
```powershell
dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"
```
Passed: 130/130 tests, including the embedded Go runtime tests with race
detection, multiple independent resource failures, concurrent error
recording, duplicate-propagation prevention, and joined-error
formatting.
```powershell
dotnet test --project tests\Aspire.Hosting.RemoteHost.Tests\Aspire.Hosting.RemoteHost.Tests.csproj --no-launch-profile -- --filter-method "*.WithField_GeneratedSdkAcceptsStructuredValues" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"
```
Passed: 5/5 tests.
Fixes # (issue)
## Checklist
- Is this feature complete?
- [x] Yes. Ready to ship.
- [ ] No. Follow-up changes expected.
- Are you including unit tests for the changes and scenario tests if
relevant?
- [x] Yes
- [ ] No
- Did you add public API?
- [ ] Yes
- If yes, did you have an API Review for it?
- [ ] Yes
- [ ] No
- Did you add `<remarks />` and `<code />` elements on your triple slash
comments?
- [ ] Yes
- [ ] No
- [x] No
- Does the change make any security assumptions or guarantees?
- [ ] Yes
- If yes, have you done a threat model and had a security review?
- [ ] Yes
- [ ] No
- [x] No
---------
Co-authored-by: David Pine <7679720+IEvangelist@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c8717a87-0ee5-4296-a6fd-ea6e2d91a57e
(cherry picked from commit 031536b)
This was referenced Nov 11, 2026
David Pine (IEvangelist)
force-pushed
the
dapine/backport-21969-release-17.0
branch
from
November 11, 2026 02:38
487913f to
957aca6
Compare
David Pine (IEvangelist)
changed the base branch from
mitchdenny-backport-17-0-55-pr21890
to
release/17.0
November 11, 2026 02:38
7 of 14 tasks
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Forwarded callback tokens lose host cancellation linkage, and callback-valued DTO properties reject valid cancellation-token arguments before user code runs; both need fixes and regression tests.
3 open issues
Review effort: Balanced
Comment on lines
+2054
to
+2056
| if (clientExpression is not null && IsCancellationTokenTypeId(type.TypeId)) | ||
| { | ||
| return $"callbackCancellationTokenArg({clientExpression}, args, {index})"; |
Member
Author
There was a problem hiding this comment.
Addressed in #22201: DTO callbacks now bind the registering client, so token/handle args decode correctly.
Comment on lines
+172
to
+175
| token := newCancellationTokenFrom(context.Background()) | ||
| token.notify = func() { | ||
| go c.cancelToken(id) | ||
| } |
Member
Author
There was a problem hiding this comment.
Addressed in #22201: a forwarded callback token reuses the host-issued ID, so cancelling it reaches the nested operation.
Adam Ratzman (adamint)
left a comment
Adam Ratzman (adamint)
left a comment
Member
There was a problem hiding this comment.
I found three issues that should be fixed before this backport is merged.
| WriteLine("\tif s.err != nil { return s }"); | ||
| EmitHandleParamErrorChecks("\t", capability, "s.setErr(err); return s"); | ||
| WriteLine($"\tif {receiverErr} != nil {{ return s }}"); | ||
| EmitHandleParamErrorChecks("\t", capability, "s.setPropagatedErr(err); return s"); |
There was a problem hiding this comment.
Could we run the same Err() guard for handle-valued unions too? WithUnionDependency currently only type-checks the union, then serializeValue turns an errored wrapper with no handle into nil and still sends the capability call. I reproduced this with an errored testRedisResource: the target's Err() stayed nil and the fake host received withUnionDependency. The union path should propagate the child error and return before serialization.
Member
Author
There was a problem hiding this comment.
Addressed in #22201: unions with handle members now propagate an errored handle's Err() before sending.
| // invokeCapability has unmarshalled and registered this guest-provided ID. | ||
| if (!_pendingCancellations.ContainsKey(tokenId)) | ||
| { | ||
| var node = _pendingCancellationOrder.AddLast(tokenId); |
There was a problem hiding this comment.
This caps the number of pending entries, but not their size. An authenticated guest can send 1,024 unique 1 MB IDs and keep roughly a GB of ID data alive for the connection. Generated IDs are tiny (ct_<counter>_<timestamp>); could we reject IDs over a small protocol limit before adding them?
Member
Author
There was a problem hiding this comment.
Not planned. Exploiting this needs an authenticated guest, which already controls the connection and can do far worse, so a cap adds little.
| // Err overrides the promoted handle Err(): a failed AddX records its error on | ||
| // the child it returns, which a caller may never check, so the builder | ||
| // reports every error owned by a handle of its client. | ||
| WriteDocLines(RenderGoDoc("Err", BuilderErrSummary)); |
There was a problem hiding this comment.
Can we update the API projection for this override too? ProjectSynthesizedMembers still publishes scope.ErrMethod scanned from handleWrapperBase, so the Go API metadata says builder Err() returns only the first wrapper error even though this method returns client.combinedError(). The verified API snapshot currently preserves that stale summary.
Member
Author
There was a problem hiding this comment.
Fixed in #22201: the builder's Err summary now describes the aggregated errors.
Jose Perez Rodriguez (joperezr)
pushed a commit
that referenced
this pull request
Nov 11, 2026
…tadata (#22123) ## Description Backport of #21827 to `release/17.0`, cherry-picked from e7ff435 without source modifications. Fixes Go callback concurrency, per-call transport error handling, cancellation ordering, and missing typed API-export metadata. This PR targets `release/17.0` directly with a single commit and no abandoned-stack commits. It replaces #22122, whose head was `main`. Merge this first, then #22057, then #22064, then #22121. Those PRs also target `release/17.0` directly and temporarily include their prerequisite commits; they are not an abandoned stack. Related issues: #21204, #21212, #21115. This does not close those broader follow-ups. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [x] Yes - If yes, did you have an API Review for it? - [ ] Yes - [x] No - Did you add `<remarks />` and `<code />` elements on your triple slash comments? - [ ] Yes - [x] No - [ ] No - Does the change make any security assumptions or guarantees? - [x] Yes - If yes, have you done a threat model and had a security review? - [ ] Yes - [x] No - [ ] No Additive APIs are Go collection `Err` methods and API-export metadata, not public C# APIs. JSON-RPC payload/header handling changes; no authentication or permission-policy changes. Review statuses above follow the source PR. ## Customer Impact Go AppHosts can race when callbacks and builder chains share wrappers. Invalid call payloads can disconnect unrelated pending operations. This backport isolates those failures, synchronizes shared state, and exposes structured field and constant metadata in Go API exports. ## Testing Validated on this release-based branch: - `.\restore.cmd` — passed. - `dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` — 128 passed; two scaffold newline checks initially failed because three unchanged starter files retained CRLF from an earlier checkout despite `eol=lf`. Confirmed these inputs and the scaffold tests are identical to the release baseline; normalized only working-tree line endings, with no committed file changes. - `dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-method "*.GoStarterFiles_UseLfLineEndings" --filter-method "*.ScaffoldedAndStarterSources_AreGofmtClean" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` — both previously failing checks passed after normalization. All 130 tests have passed across these runs. - `dotnet test --project tests\Aspire.Hosting.RemoteHost.Tests\Aspire.Hosting.RemoteHost.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` — 713 passed, 1 skipped, 0 failed. - `dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.SdkExportCommandTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` — 31 passed, 0 failed. - `git diff --check origin/release/17.0..HEAD` — passed. Linux race checks, native AOT, and full cross-platform CI were not rerun for this backport; source-PR validation is not claimed as release-branch validation. ## Risk Medium: changes Go runtime concurrency, transport/cancellation behavior, and API-export metadata. Successful-call signatures and the existing export schema version are retained; focused runtime and export tests cover the changed behavior. ## Regression? Unknown: the source PR does not identify a regressing release or commit. Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
4 of 14 tasks
Member
|
Superseded by #22171, which consolidates all four Go source changes in the requested dependency order into one PR against release/17.0. The source patches are unchanged and retain their cherry-pick references. Closing this duplicate backport; existing review discussions remain available here and should be considered when reviewing #22171. |
Jose Perez Rodriguez (joperezr)
added a commit
that referenced
this pull request
Nov 12, 2026
## Description Consolidate the Go AppHost release backports into one PR against `release/17.0`, so shipping the complete Go changes requires one merge rather than three remaining dependent backport merges. David Pine's [November 11 release-order message](https://teams.microsoft.com/l/message/19:86b689da3ce444f8952d0b79157f7562@thread.tacv2/1794418391504?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=f2e0b0b6-f130-46fa-8ec6-2ef4a7168ed2&parentMessageId=1794418391504&teamName=Aspire&channelName=Tactics%20And%20Shiproom&createdTime=1794418391504) opens with "These backports, in this order to get Go AppHost release ready:" and lists #22123, #22057, #22064, and #22121 in that order. The existing backport descriptions specify the same sequence. At preparation time, `release/17.0` points to `9a97ab889bfb731de74ead0280be279ffdd000bf`, the already-merged #22123. This branch first reverts that commit, then reapplies the four original source commits with `git cherry-pick -x` in Pine's order: 1. #21827 / #22123: `e7ff435302acb88f9cfe7c9a61e3633e0f553f08` - harden the Go SDK runtime and complete typed API export metadata. 2. #21969 / #22057: `031536bc9eea22ee9ce2cc1d31bfd30d0c14cfd2` - propagate child resource errors through Build. 3. #21976 / #22064: `cd9e0e8d159d20f3cf7e64eda2415dcc3d40c168` - make generated Go SDK APIs idiomatic. 4. #21874 / #22121: `209d9226e0463b638a5d68a034505e874cae9091` - add the React frontend to the Go starter. The revert exists only on this consolidation branch; the shared release branch was not changed. All four source patches are unchanged (verified with matching patch IDs), with their original source references retained in commit messages. Earlier release prerequisites remain untouched. This PR supersedes #22057, #22064, and #22121, which have been closed with comments linking here. Existing review discussions remain available on those PRs. ### User-facing usage The starter provides a Go API, React frontend, and optional Redis: ```console aspire new aspire-go-starter --non-interactive --use-redis-cache false ``` Use `--use-redis-cache true` to include Redis. Generated projects contain `apphost.go`, `api/`, and `frontend/`. ### Breaking changes The #21976 source change updates generated Go SDK signatures and names. Existing Go AppHosts need to regenerate their SDK and update affected calls. Shared ATS metadata changes are also included. ### Screenshots / Recordings > **This PR includes UI changes.** Please add screenshots or screen recordings so reviewers can evaluate the visual changes without running locally. > > - For before/after comparisons, place them side-by-side or label them clearly. > - For interactive changes (animations, transitions, new flows), prefer a short screen recording (GIF or video). > - If you cannot capture visuals now, note what scenario to test and mark this section as TODO. <!-- Add screenshots/recordings here --> TODO: Visual evidence was not captured. Run the generated Go starter and inspect its React frontend, weather data through the Vite proxy, and optional Redis scenario. ### Security considerations These are unchanged source backports, not new security fixes. They include Go transport/callback/cancellation handling and starter process, proxy, and deployment behavior. Existing review comments on #22057 still need disposition; consolidation does not resolve or invalidate those comments. No new threat-model or security-review result is claimed here. Fixes # (issue) ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [x] Yes - If yes, did you have an API Review for it? - [ ] Yes - [ ] No - Did you add `<remarks />` and `<code />` elements on your triple slash comments? - [ ] Yes - [ ] No - [ ] No - Does the change make any security assumptions or guarantees? - [x] Yes - If yes, have you done a threat model and had a security review? - [ ] Yes - [ ] No - [ ] No ## Customer Impact Go AppHost users receive the compatible generated SDK, child-resource error propagation, runtime hardening, and a complete Go/React starter together, without having to assemble dependent backports. ## Testing Validation was performed on the consolidated release-based head `065867bf777dd1e755e2718e8ac979a83fd37613`, not inherited from the source PRs. Test processes used the repo-local `artifacts\tools\go\bin` on PATH (`go version go1.27.4 windows/amd64`). - `.\restore.cmd` - passed; pre-existing NuGet vulnerability warnings were reported. - `dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - passed, **137 tests, 0 skipped**, including Go build/vet/runtime and gofmt-dependent tests. An intermediate rerun found generated frontend `node_modules` copied into the scaffold test fixture; removing only generated `node_modules`/`dist` artifacts restored the clean run without source changes. - `dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-method "*.GoStarterTemplate*" --filter-method "*.GoStarterHonorsRedisChoice*" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - passed, **10 tests**. - `dotnet test --project tests\Aspire.Hosting.Go.Tests\Aspire.Hosting.Go.Tests.csproj --no-launch-profile -- --filter-method "*.WithModTidyIsIdempotent" --filter-method "*.WithModTidyCreatesChildResource" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - passed, **2 tests**. - `dotnet test --project tests\Aspire.Hosting.RemoteHost.Tests\Aspire.Hosting.RemoteHost.Tests.csproj --no-launch-profile -- --filter-class "*.CancellationTokenRegistryTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - passed, **14 tests**. An earlier attempted filter named `RemoteCancellationRegistryTests` selected zero tests; the corrected run above is the validation result. - `dotnet build tests\Aspire.Cli.EndToEnd.Tests\Aspire.Cli.EndToEnd.Tests.csproj --no-restore` - passed, **0 warnings/errors**. E2E execution was not run on this Windows host. - `npm ci` followed by `npm run build` in `src\Aspire.Cli\Templating\Templates\go-starter\frontend` - passed; TypeScript and Vite production build succeeded. Generated build artifacts were removed afterward. - `dotnet run --project tools\SelectTests -- --from origin/release/17.0 --to HEAD --explain` - passed; **0 unattributed changed files**. `.gitattributes` selects all PR test projects/jobs, including starter validation and polyglot. - `dotnet test --project tests\Infrastructure.Tests\Infrastructure.Tests.csproj --no-launch-profile -- --filter-namespace "Infrastructure.Tests.TestTriggerMap" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - **364 passed, 22 failed**. Failures are Windows test-harness limitations: Bash subprocesses cannot resolve backslash-delimited Windows script paths, and five git-fixture cleanup cases throw `UnauthorizedAccessException` deleting git objects. This suite is not claimed green; Linux/CI validation is still required. No test or script source was changed. - `git diff --check origin/release/17.0...HEAD` - passed. Starter runtime, Docker deployment, screenshots, full polyglot compatibility suites, and GitHub CI execution were not validated locally. ## Risk Medium. The source changes include shared ATS metadata, breaking generated Go SDK changes, transport/error handling, and starter/frontend deployment behavior. Consolidation preserves the source patches but does not replace review or CI validation. ## Regression? Unknown: the source material does not identify a regressed release. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
4 of 8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Backport of #21969 to
release/17.0. Targetsrelease/17.0directly, based on release commitfea795884e, with only these unmodified source cherry-picks:e7ff435302acb88f9cfe7c9a61e3633e0f553f08(prerequisite, backport [release/17.0] Harden Go SDK runtime and complete typed API export metadata #22123).031536bc9e(this fix).Merge order: #22123 → #22057 → #22064 → #22121. All four target the release branch directly, not one another. Later PRs temporarily include their prerequisite commits; refresh them against the release branch as prerequisites land. No unrelated main/abandoned-stack commits are included.
The original automated attempt conflicted because #21827 was missing from release: https://github.com/microsoft/aspire/actions/runs/42756228525.
Checklist
<remarks />and<code />elements on your triple slash comments?Checklist describes #21969 itself; the prerequisite's additive Go APIs and JSON-RPC changes are documented in #22123.
Customer Impact
Generated Go AppHosts can miss failed child-resource declarations and invoke Build() with an invalid model. This fix reports all recorded resource failures through the builder's Err() and Build().
Testing
Validated the release-based tree, not the former main-based stack. The tested tree
b61aa7bf2f412afaf0a3ebe3a14ab7c65b783ebdexactly matches live head957aca6b66445d567d1f3f28161b0c3f4c6be354..\restore.cmd— passed.dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"— 130 passed, 0 failed.dotnet test --project tests\Aspire.Hosting.RemoteHost.Tests\Aspire.Hosting.RemoteHost.Tests.csproj --no-launch-profile -- --filter-class "*.KubernetesManifestTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"— 13 passed, 0 failed.git diff --check origin/release/17.0..HEAD— passed.Broader prerequisite runtime/export validation is recorded in #22123. Linux race checks and full cross-platform CI were not rerun locally.
Risk
Low for #21969: limited to generated Go error propagation; successful builds retain their existing behavior. The temporarily included runtime-hardening prerequisite has Medium risk, documented in #22123.
Regression?
Unknown: the source PR does not identify a regressing version or commit.