Win AOT Release#193
Conversation
Code Review -- PR #193: Win AOT ReleaseOverviewThis PR extends the release matrix with two Windows Native AOT entries ( 🔴 Critical: Missing
|
| Step | Bash-only construct |
|---|---|
| Extract version from tag (line 71) | ${GITHUB_REF#refs/tags/} -- bash parameter expansion; PowerShell treats this as a literal string, breaking version extraction entirely |
| Set deployment flags (lines 78-89) | set -euo pipefail and if [ ... ] -- invalid PowerShell |
| Publish Console App (lines 92-119) | set -euo pipefail and if [ ... ] -- invalid PowerShell |
Fix: add shell: bash to each of those three steps, or add a job-level default:
defaults:
run:
shell: bashThe steps added by this PR (runner guard, create archive, verify archive contents, smoke test) already carry shell: bash correctly. The gap is in the pre-existing steps that now run on a new runner type for the first time.
🔴 Critical: CIR links into the specs/ tree (violates feedback_no_spec_references)
docs/change-intent-records/2026-05-02-linux-native-aot-release.md was amended to add a hyperlink to ../../specs/002-win-aot-release/. Project memory feedback_no_spec_references is explicit: specs/NNN-... paths are ephemeral (deleted post-merge); source/tests/docs/ must never link to them. This link will rot the moment specs/002-win-aot-release/ is deleted post-merge. Remove the hyperlink; the surrounding prose can retain the plain-text feature number and issue reference.
🟡 Minor: Stray BOM character in CLAUDE.md
The diff shows a UTF-8 BOM (U+FEFF) prepended to the first line. Strip it.
🟡 Minor: Verify unzip is available on windows-11-arm
The "Verify archive contents" and "Smoke test" steps call unzip via Git Bash. windows-latest ships unzip as part of Git for Windows; windows-11-arm is a newer image whose pre-installed software may differ. The rehearsal-tag run (T15) will confirm this. If unzip is absent, 7z (guaranteed on all GitHub Windows runners) is a straightforward fallback for both listing (7z l) and extraction (7z x).
🟡 Minor: DebugType=embedded is unconditional
Both .csproj files place <DebugType>embedded</DebugType> in the base PropertyGroup, applying it to Debug builds as well as Release. The intent is to suppress side .pdb files in release publish output -- a Condition="'$(Configuration)' == 'Release'" guard would make that intent more precise. Not a correctness issue for the pipeline, but worth considering for NetPace.Core where debug NuGet packs will embed symbols rather than produce a separate .pdb.
What is good
- Runner OS guard is a genuinely useful fail-fast step with a clear, actionable error message. It directly catches the silent
ubuntu-latestfallback footgun. - PDB scrub is surgical:
rm -f ./*.pdbonly in thewin-* && aotbranch; all other variants are untouched. - Archive contents verification is the right new enforcement point. The
grep -c . || truepattern correctly handles the empty-archive edge case (0 != 1 = fails). - Size-assertion widening is correct: adding
&& "$runtime" != win-*to the negation properly extends AOT coverage to the new Windows RIDs. - Research docs (R1-R11) thoroughly justify every non-obvious decision (runner choice, PDB exclusion, shell selection, globalization mode).
- T15-T19 correctly left open -- the rehearsal tag is the real integration gate and the PR does not claim it done.
DebugType=embeddedrationale is well explained in both csproj comments and the newdocs/RELEASING.mdArchive-contents contract section.
Summary
| Severity | Issue | Location |
|---|---|---|
| Critical | Missing shell: bash on 3 pre-existing steps now running on Windows runners |
release-binaries.yml lines 71, 78, 92 |
| Critical | CIR links into ephemeral specs/ directory |
docs/change-intent-records/2026-05-02-linux-native-aot-release.md |
| Minor | Stray BOM character | CLAUDE.md |
| Minor | unzip availability on windows-11-arm unverified |
release-binaries.yml smoke/verify steps |
| Minor | DebugType=embedded unconditional (affects Debug builds and NuGet debug packs) |
Both .csproj files |
The shell issue is the one item that will definitely break CI. Fix the three missing shell: bash declarations and remove the spec-link from the CIR before pushing the rehearsal tag.
|
Please fix the following items from your earlier code review (see comment #193 (comment)): Missing shell: bash on 3 pre-existing steps now running on Windows runners DebugType=embedded unconditional (affects Debug builds and NuGet debug packs)
|
|
Claude finished @FrankRay78's task in 2m 30s —— View job Fixing Code Review Items
GitHub App permissions do not allow modifying
The quickest fix is a job-level default at the top of the defaults:
run:
shell: bash· Branch |
- Strip UTF-8 BOM prepended to CLAUDE.md line 1 - Replace hyperlink to specs/002-win-aot-release/ in Linux AOT CIR with plain-text reference (specs/ is ephemeral and deleted post-merge) Co-authored-by: Frank Ray <FrankRay78@users.noreply.github.com>
|
@claude Failed to generate assets on tag: https://github.com/FrankRay78/NetPace/releases/tag/0.23.0 |
|
Claude finished @FrankRay78's task in 4m 21s —— View job • Release Asset Generation Failure — Root Cause & Fix
Root CauseThe "Verify archive contents" step added in PR #193 enforces that every release archive contains exactly one entry. Every job failed because the archive contained two entries:
Fix AppliedAdded Next Steps
|
|
@claude Still failing - https://github.com/FrankRay78/NetPace/actions/runs/25788763634 Perform a proper RCA before making changes, then test thoroughly to be sure. |
|
Claude finished @FrankRay78's task in 3m 57s —— View job • RCA: Release Pipeline Still Failing
Root Cause AnalysisTwo separate problems, both contributing: Problem 1: Previous fix was never merged — The last CI response committed fix Problem 2: The previous The chain of events (visible in the CI log, lines 279–283):
Why Fix Applied
<Target Name="ExcludeDocXmlFromPublish" AfterTargets="ComputeFilesToPublish">
<ItemGroup>
<ResolvedFileToPublish Remove="@(ResolvedFileToPublish)"
Condition="'%(Extension)' == '.xml'" />
</ItemGroup>
</Target>· Branch |
|
@claude Still borken: https://github.com/FrankRay78/NetPace/actions/runs/25789481599 Don't make any more fixes. Instead, perform detailed RCA and report back here. Make decent recommendations to fix, once and for all. I also want a way to verify the next fix will work, before I make any more release/tags. Do nothing else until I say. |
|
Claude finished @FrankRay78's task in 2m 23s —— View job RCA: Release Pipeline Failing —
|
| What | Where | Status |
|---|---|---|
NetPace.Core.xml in non-AOT archives |
Fixed by PR #197 (ExcludeDocXmlFromPublish) |
✅ Working |
NetPace.dbg in Linux AOT archives |
No .dbg scrub in "Create archive" |
❌ Root cause of current failure |
NetPace.pdb in Windows AOT archives |
rm -f ./*.pdb already in place |
✅ Working (not yet exercised on real tag) |
One-line fix: add rm -f ./*.dbg inside an if [ "${{ matrix.deployment }}" = "aot" ] block in the Linux branch of the "Create archive" step.
· Branch
1 participant
Summary
Adds Windows Native AOT to the release matrix as two new archives —
netpace-{ver}-win-x64-aot.zipandnetpace-{ver}-win-arm64-aot.zip— single native.exes with no .NET runtime dependency, bringing the per-release total to 16. Mirrors feature 001's Linux AOT pattern (matrix.include:entries + native runner per RID,windows-latestfor x64 andwindows-11-armfor arm64, since cross-OS AOT cross-compilation is fragile). Also tightens the archive-contents contract end-to-end: every release archive (all 16) must now contain exactly one entry, enforced by a new "Verify archive contents" workflow step; managed PDBs are embedded via<DebugType>embedded</DebugType>in bothNetPace.Console.csprojandNetPace.Core.csprojso no side.pdbships in any variant, and the Windows AOT native PDB is scrubbed before zipping. Includes a runner-OS guard step that fails fast on amatrix.runs_ontypo (would otherwise silently fall back toubuntu-latest).Spec
specs/002-win-aot-release
Changed Files
New Artifacts