[codex] Fix PR workflow MSBuild 17.14 mismatch

[HavenDV]

Summary

• stop using the reusable workflow's plain msbuild path for PR validation
• pin the PR workflow to .NET 10 explicitly
• build the repo through H.NotifyIcon.slnx with dotnet build instead of MSBuild-only /target: arguments

Why

The reusable PR workflow currently invokes msbuild when use-msbuild: true. On GitHub's Windows runner that resolves to MSBuild 17.14, while this repo now resolves .NET SDK 10.0.201, which requires MSBuild 18.0+. That makes every PR fail before it gets to meaningful validation.

Notes

This change is only for the PR validation workflow. The push workflow in this repo already uses dotnet build directly.

[claude]

PR Review: Fix PR workflow MSBuild 17.14 mismatch

Overall: The fix is correct and well-motivated. The MSBuild/SDK version mismatch is a real blocker and this cleanly resolves it. A few things worth noting before merging:

---

What looks good

• Dropping use-msbuild: true and explicitly setting dotnet-version: '10.0.x' is the right fix. This aligns the PR workflow with the push workflow (dotnet.yml already pins 10.0.x there).
• Using dotnet build via the reusable workflow is consistent with the project's existing approach in dotnet.yml.
• Removing the verbose /target: list reduces maintenance burden; those MSBuild-style target paths were fragile and tied to the old msbuild invocation.

---

Concerns to consider

1. Scope change: PR now builds more than push does

The push workflow (dotnet.yml) builds only src/libs/**/*.csproj (library projects only). This PR workflow will build the full H.NotifyIcon.slnx, which includes sample apps (H.NotifyIcon.Apps.Wpf, H.NotifyIcon.Apps.WinUI, H.NotifyIcon.Apps.Maui, H.NotifyIcon.Apps.Console, TrimmingHelper) and integration tests.

This is not necessarily bad — wider coverage is good — but it creates an asymmetry: a PR could fail CI due to an app project issue that the push workflow would never catch. Worth deciding intentionally whether the full-solution scope is desired, or if the solution file should be scoped to libs only (e.g., a separate H.NotifyIcon.libs.slnx).

2. Build configuration

The old /target: approach used whatever configuration MSBuild defaulted to. The push workflow uses --configuration Release explicitly. It is worth confirming whether the reusable workflow (dotnet_build-test-publish.yml) passes --configuration Release or defaults to Debug when given a project-path. Release builds can expose additional warnings and trimming/AOT-compatibility issues that Debug builds miss.

3. .slnx support in the reusable workflow

The .slnx format (XML solution files) requires .NET 10 SDK / MSBuild 18+. Since dotnet-version: '10.0.x' is now explicitly set, SDK support should be fine — just make sure dotnet_build-test-publish.yml passes the .slnx path directly to dotnet build rather than assuming a .sln or .csproj extension.

---

Minor notes

• The draft status is appropriate — validating a CI workflow change before merging makes sense.
• No test coverage concerns: this is a CI configuration change, not library code.
• No security concerns.

---

Summary: The root cause diagnosis is correct and the fix is sound. Main things to verify before merging: (a) that the reusable workflow handles .slnx correctly, and (b) whether the broader build scope (apps + libs vs. libs only) is intentional. If so, this is ready to go.

[claude]

test

[claude]

PR Review — Fix PR workflow MSBuild 17.14 mismatch

Overall: Clean, well-scoped fix for a genuine CI problem. MSBuild 17.14 on the Windows runner does not support .NET SDK 10 projects; switching to dotnet build (already used in dotnet.yml) is the right call.

---

What is good

• Correct root cause — dotnet build is the proper fix for the MSBuild/SDK version mismatch.
• Aligns the PR workflow with the push workflow (dotnet.yml already uses dotnet build + dotnet-version: 10.0.x).
• Omitting sample app projects from H.NotifyIcon.PR.slnx is correct: WinUI packaging etc. are not needed for library validation.

---

Concern 1 — Maintenance burden of a second solution file

H.NotifyIcon.PR.slnx is a libs+tests subset of H.NotifyIcon.slnx. Whenever a new library is added, both files need updating, which is easy to miss. A comment at the top would help. If the reusable workflow supports it, a project glob passed to dotnet build (like the PowerShell Get-ChildItem loop in dotnet.yml) would avoid the second file entirely.

Concern 2 — H.NotifyIcon.Maui is a new addition to PR CI

The old MSBuild /target: list did NOT include H.NotifyIcon.Maui. The new slnx does — likely a coverage improvement, but:

• The PR summary does not mention this scope change.
• MAUI targets Android, iOS, macCatalyst, and Windows. Confirm the runner MAUI workload (workloads: maui) covers all TFMs under dotnet build. A build failure here would be a regression vs. the old approach that silently skipped Maui in PR CI.

Concern 3 — Integration tests now in the PR build

H.NotifyIcon.IntegrationTests was also absent from the old /target: list. Depending on what the reusable workflow does with a .slnx path, tests may execute on every PR. These tests are documented as Windows-only and may require a real system tray. Confirm whether they are only compiled (fine) or also executed (needs runner validation).

Concern 4 — Reusable workflow pinned to @main

Pre-existing, not introduced here: HavenDV/workflows/.github/workflows/dotnet_build-test-publish.yml@main means any upstream change takes effect immediately. A SHA pin would improve stability.

---

Minor nit: Windows-style backslashes in H.NotifyIcon.PR.slnx match H.NotifyIcon.slnx and are fine since the workflow runs only on windows-latest.

---

Summary: The fix is correct and the approach is sound. Two things to confirm before merging: (1) MAUI builds cleanly with the MAUI workload on the PR runner, and (2) integration tests do not require interactive/UI infrastructure to run in CI. Once verified, this looks ready to go.