Fix internal build break: remove failing TS-starter / bundle-layout verifier checks, add GitHub-CI coverage, fix latent sidecar-scope bug#17346
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17346Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17346" |
|
Triggered a validation build of this revert on the internal Build: |
There was a problem hiding this comment.
Pull request overview
This PR reverts the verifier-side additions from #17274 and the follow-up fix from #17341 to unblock the internal signed build pipeline while environmental compatibility issues are investigated (tracked in #17345).
Changes:
- Removes the Acquisition E2E test coverage that validated
verify-cli-archive.ps1against a synthetic bundle-shaped archive. - Simplifies
eng/scripts/verify-cli-archive.ps1back to validating sidecar absence,aspire --version, andaspire new aspire-starter. - Removes the synthetic “bundle extraction + TS starter” fake archive generation helper and its script path constant.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/Aspire.Acquisition.Tests/Scripts/VerifyCliArchivePowerShellTests.cs | Deletes the synthetic end-to-end test for verify-cli-archive.ps1. |
| tests/Aspire.Acquisition.Tests/Scripts/Common/ScriptPaths.cs | Removes the VerifyCliArchivePowerShell script path constant. |
| tests/Aspire.Acquisition.Tests/Scripts/Common/FakeArchiveHelper.cs | Removes the fake “bundle-shaped” archive generator and mock CLI script. |
| eng/scripts/verify-cli-archive.ps1 | Reverts bundle-layout and TypeScript-starter verification; keeps only basic archive validation + C# starter creation. |
…rchive verifier The signed build+sign pipeline (microsoft-aspire, definition 1602) is broken on main since microsoft#17274 merged. Two checks added by that PR fail in the 1ES signed-build agent -- the only environment that actually runs verify-cli-archive.ps1 against the real signed archive: 1. Test-TypeScriptStarterProject runs `aspire new aspire-ts-starter`, whose packager-managed AppHost bootstrap performs a NuGet restore that resolves transitive deps (Microsoft.Extensions.*, Grpc.*, OpenTelemetry.*, etc.) to https://api.nuget.org/v3/index.json. The signed-build agent has no public-internet egress and the restore fails with `Permission denied (api.nuget.org:443)`. 2. Test-ExtractedBundleLayout's file assertions only succeed when the prior TypeScript starter step has triggered the AppHost server bootstrap and extracted ~/.aspire/bundle. The C# starter doesn't exercise that path. With the TS starter removed there's no trigger for the bundle layout to be populated, and its embedded `aspire-managed --help` smoke call returns exit 1 by design (the binary's top-level dispatch is dashboard|server|nuget; anything else falls through to ShowUsage()). This is the same class of regression as the earlier `aspire-managed --help` failure in the same script: the verifier doesn't run on GitHub PRs and only fires in the post-merge build+sign pipeline, so the incompatibility wasn't caught pre-merge. Surgical removal of the failing pieces: - Drop Test-TypeScriptStarterProject (function + call site). - Drop Test-ExtractedBundleLayout (function + call site). - Drop tests/Aspire.Acquisition.Tests/Scripts/VerifyCliArchivePowerShellTests.cs, which only covered the removed functions. - Drop CreateFakeBundleArchiveAsync and CreateFakeCliScript additions from FakeArchiveHelper.cs, which had no other consumers. - Drop the unused ScriptPaths.VerifyCliArchivePowerShell entry. Keep all the orthogonal infrastructure microsoft#17274 added so it can be reused when the TS check returns: - Helpers Get-ExecutableFileName, Get-ArchiveRoot, Get-ArchiveRidFamily. - Refactored Test-CSharpStarterProject function (functionally equivalent to the pre-microsoft#17274 inline C# starter check). - Test-ArchiveSidecar's use of Get-ArchiveRidFamily. - aspireBin discovery via Get-ArchiveRoot. The script header's step list is updated to match what now runs. Re-adding the TS coverage will require either an internal NuGet mirror config the agent can reach (e.g. dropping a NuGet.config that maps * -> dotnet-public into the temp project root before `aspire new`), or a way to skip the auto-restore on `aspire new aspire-ts-starter`. Tracked in microsoft#17345. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c4cbb22 to
c5f6774
Compare
|
Switched to a surgical removal of just the failing parts of #17274 (kept the helpers and the Build: |
verify-cli-archive.ps1 only runs in the post-merge AzDO build+sign pipeline (never on GitHub PRs), so regressions in its contract surface days late. The recent microsoft#17274 -> microsoft#17341 -> microsoft#17345 chain is the case in point: the verifier was broken on main for hours before anyone noticed. Add a synthetic-archive E2E test that exercises the verifier's current contract in GitHub CI against a fake `aspire` binary, with no real CLI, no bundle, and no network access: - VerifyCliArchive_AcceptsCleanPerRidArchive: per-RID archive with the fake `aspire` mock handles `aspire --version` and `aspire new aspire-starter`; asserts the verifier exits 0 and emits the expected per-step success lines. - VerifyCliArchive_RejectsArchiveWithStraySidecar: per-RID archive containing a stray `.aspire-install.json`; asserts the verifier exits non-zero with the user-facing error text from Test-ArchiveSidecar. This is the contract from microsoft#16817 that the verifier was originally written to enforce. Adds FakeArchiveHelper.CreateFakeVerifyArchiveAsync to produce these archives. Pre-existing CreateFakeArchiveAsync stays as-is so the install-script tests are unaffected. Unix-only because the fake aspire mock is a bash script. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pushed a follow-up commit ( New The AzDO validation build |
Test-ArchiveSidecar was called with $archiveRoot (the directory
Get-ArchiveRoot picks out as the binary's home), not $extractDir (the
true extraction directory). When a per-RID archive nests its binary
under a single sub-directory (e.g. {.aspire-install.json, payload/aspire}),
Get-ArchiveRoot returns the sub-directory, and the sidecar scan only
recurses into that sub-directory -- missing a stray .aspire-install.json
at the actual archive root.
The sidecar contract from microsoft#16817 ("per-RID archives are shared across
install routes; each route authors its own sidecar after extraction")
is about the archive, not whichever directory happens to hold the
binary. Scan the full extraction directory instead.
Latent bug only -- today's signed archives put `aspire` at the
extraction root, so Get-ArchiveRoot returns $extractDir unchanged and
the scope-narrowing didn't matter in practice. A producer-side
refactor that introduced a top-level directory would have silently
hidden a contract violation.
Add VerifyCliArchive_RejectsStraySidecarWhenBinaryIsNestedUnderSubdirectory
to pin the fix: it builds an archive with the binary nested under
`payload/` and a stray sidecar at the archive root, and asserts the
verifier rejects it. Without the script fix the verifier accepts the
archive; with it the test passes. CreateFakeVerifyArchiveAsync gains
a nestAspireUnderSubdir parameter so the same helper covers both the
binary-at-root and binary-in-subdir shapes Get-ArchiveRoot has to
handle.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pushed
Fix is one line in the script (scan Local run: 3/3 tests pass (1.8s). Cancelled the prior AzDO validation build (2980648) — it was validating the version of the script without the latent-bug fix. Triggered a fresh build |
The signed build+sign pipeline (
microsoft-aspire, definition 1602) has been broken onmainsince #17274 merged. Two checks added by that PR fail in the 1ES signed-build agent — the only environment that actually runseng/scripts/verify-cli-archive.ps1against the real signed archive:Test-ExtractedBundleLayoutinvokesaspire-managed --help, which returns exit 1 by design —Aspire.Managed's entry point is a hardargs switchondashboard|server|nuget, and anything else falls through toShowUsage()returning 1 (src/Aspire.Managed/Program.cs:22-28,58-62). PR Fix internal build break: 'aspire-managed --help' in CLI archive verifier returns exit 1 #17341 patched this toaspire-managed nuget --help, which only fixed half the breakage.Test-TypeScriptStarterProjectinvokesaspire new aspire-ts-starter, whose packager-managed AppHost bootstrap performs a NuGet restore that resolves transitive deps (Microsoft.Extensions.*,Grpc.*,OpenTelemetry.*, …) tohttps://api.nuget.org/v3/index.json. The signed-build agent has no public-internet egress and the restore fails withPermission denied (api.nuget.org:443).Same class of regression both times: the verifier doesn't run on GitHub PRs and only fires in the post-merge build+sign pipeline, so the incompatibility wasn't caught pre-merge.
Surgical removal of the failing checks
Only the failing pieces are removed; all orthogonal infrastructure from #17274 is kept so it can be reused when the TS check returns.
Removed:
Test-TypeScriptStarterProject(function + call site)Test-ExtractedBundleLayout(function + call site)CreateFakeBundleArchiveAsync+CreateFakeCliScriptfromFakeArchiveHelper.cs(no other consumers)Kept:
Get-ExecutableFileName,Get-ArchiveRoot,Get-ArchiveRidFamilyTest-CSharpStarterProjectfunction (functionally equivalent to the pre-Validate TypeScript CLI archive layout #17274 inline C# starter check)Test-ArchiveSidecar's use ofGet-ArchiveRidFamilyaspireBindiscovery viaGet-ArchiveRootThe script header's step list is updated to match what now runs.
Added: slim GitHub-CI coverage for the verifier
The verifier still only runs in the post-merge AzDO pipeline, so regressions in its contract surface days late. A slim synthetic-archive E2E test exercises the verifier in GitHub CI with no real CLI, no bundle, and no network:
VerifyCliArchive_AcceptsCleanPerRidArchive— clean per-RID archive whose fakeaspiremock handles--versionandnew aspire-starter; asserts exit 0 + expected per-step success lines.VerifyCliArchive_RejectsArchiveWithStraySidecar— archive with a stray.aspire-install.jsonat the root; asserts non-zero exit with the user-facing error text fromTest-ArchiveSidecar. This is the per-RID archive contract from feat(cli): co-locate bundle with CLI binary for packager-managed installs #16817 that the verifier was originally written to enforce.VerifyCliArchive_RejectsStraySidecarWhenBinaryIsNestedUnderSubdirectory— same contract, with the binary nested under a single subdirectory. Catches a latent scope-narrowing bug in the sidecar check (see below).New
FakeArchiveHelper.CreateFakeVerifyArchiveAsyncproduces these archives. Pre-existingCreateFakeArchiveAsyncis unchanged, so install-script tests are unaffected. Unix-only because the fake aspire mock is a bash script.Fixed: latent
Test-ArchiveSidecarscope-narrowingTest-ArchiveSidecarwas being called with$archiveRoot(the directoryGet-ArchiveRootpicks out as the binary's home), not$extractDir(the true extraction directory). For an archive shaped like{.aspire-install.json, payload/aspire},Get-ArchiveRootreturnspayload/, and the sidecar scan recurses only intopayload/— silently missing the stray sidecar at the actual archive root.Latent only — today's signed archives put
aspireat the extraction root, soGet-ArchiveRootreturns$extractDirunchanged and the scope-narrowing didn't matter in practice. A producer-side refactor that introduced a top-level directory would have silently hidden a #16817 contract violation. Fixed by scanning$extractDirinstead; pinned by the third test above.Follow-up
Re-introducing the TS-starter and bundle-layout coverage is tracked in #17349.
Fixes #17345