Fix relaunch.ps1 RID detection for unpackaged Windows builds#650
Fix relaunch.ps1 RID detection for unpackaged Windows builds#650
Conversation
With WindowsPackageType=None (the current setting), dotnet build places PolyPilot.exe directly in bin/Debug/net10.0-windows10.0.19041.0/ without a RID subdirectory (e.g. win-x64/). After git clean, the script failed with 'Could not detect runtime identifier' because it only searched subdirectories for the exe. Now checks the framework directory first, falling back to RID subdirectory scanning for packaged or cross-compiled builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📋 Design-level finding (outside diff)🟡 MODERATE — "Old app instance remains running" message is misleading after kill (Flagged by: 3/3 reviewers, pre-existing)
This is pre-existing (not introduced by this PR), but worth noting for a follow-up fix. Suggestion: track whether instances were killed and print the appropriate message: if ($OldPids.Count -gt 0) {
Write-Host "Old instances were stopped before build — app is not running."
} else {
Write-Host "Old app instance remains running."
}
|
There was a problem hiding this comment.
Expert Review — PR #650: Fix relaunch.ps1 RID detection
Methodology: 3 independent reviewers with adversarial consensus. Findings validated through follow-up cross-review when only 1/3 initially flagged.
Summary
The fix is correct and well-scoped — it resolves the reported issue (unpackaged Windows builds failing RID detection) while preserving backward compatibility for packaged/cross-compiled builds. No regressions, security issues, or data loss risks introduced.
Findings (ranked by severity)
| # | Severity | Finding | Consensus | Location |
|---|---|---|---|---|
| 1 | MODERATE | Stale root exe may be preferred over fresh RID-subfolder exe when switching WindowsPackageType | 3/3 (after follow-up) | L96-97 (inline) |
| 2 | MODERATE | Old app instance remains running message misleading after process termination | 3/3 (after follow-up) | L85 (pre-existing, outside diff) |
| 3 | MINOR | Get-ChildItem throws raw stack trace if FrameworkDir missing | 2/3 | L99 (inline, pre-existing behavior) |
Discarded (single reviewer only):
- logDir creation inside retry loop (pre-existing, trivial)
Assessment
- CI status: No CI checks configured for this PowerShell script
- Test coverage: Script-level — no automated tests exist for relaunch.ps1
- Finding 1 is the only net-new concern — the stale-exe scenario is an edge case (requires switching WindowsPackageType without cleaning build output) but could cause confusion. Consider a LastWriteTimeUtc comparison or preferring the RID subfolder when both exist.
- Findings 2 and 3 are pre-existing and not blocking.
Verdict: Good to merge. Finding 1 is low-risk (edge case) but worth considering for a follow-up improvement.
Generated by Expert Code Review (auto) for issue #650
| if (Test-Path (Join-Path $FrameworkDir $ExeName)) { | ||
| $BuildDir = $FrameworkDir |
There was a problem hiding this comment.
🟡 MODERATE — Stale root exe may be preferred over fresh RID-subfolder exe (Flagged by: 3/3 reviewers)
dotnet build does not clean previous output. If a stale PolyPilot.exe exists in $FrameworkDir from an earlier unpackaged build, and the current build outputs the real artifact in a RID subfolder (e.g. after switching to a packaged build), the Test-Path check here will find the stale root exe and skip the RID subfolder scan — silently launching old code.
Scenario: Developer does an unpackaged build (exe in framework dir), then switches WindowsPackageType to packaged (exe now in win-x64/ subfolder). The stale root exe remains → script launches it instead of the just-built one.
Suggestion: Compare LastWriteTimeUtc of both candidates when the root exe exists, or prefer the RID subfolder when both are present.
| if (Test-Path (Join-Path $FrameworkDir $ExeName)) { | ||
| $BuildDir = $FrameworkDir | ||
| } else { | ||
| $RidDir = Get-ChildItem -Path $FrameworkDir -Directory | |
There was a problem hiding this comment.
🟢 MINOR — Get-ChildItem throws terminating error if $FrameworkDir doesn't exist (Flagged by: 2/3 reviewers)
If $FrameworkDir doesn't exist (e.g. custom OutputPath in Directory.Build.props), Test-Path on L96 returns $false (safe), but Get-ChildItem -Path $FrameworkDir -Directory here throws a terminating error because $ErrorActionPreference = 'Stop' is set (L14). The user sees a raw PowerShell stack trace instead of the friendly [X] message.
Not a regression — the old code had identical behavior. Consider adding -ErrorAction SilentlyContinue or a Test-Path $FrameworkDir guard in a follow-up:
} else {
if (-not (Test-Path $FrameworkDir)) {
Write-Host '[X] Build output directory not found:' $FrameworkDir
exit 1
}
$RidDir = Get-ChildItem ...
🔍 Multi-Model Code Review — PR #650Fix relaunch.ps1 RID detection for unpackaged Windows builds Methodology: 3 independent reviewers with adversarial consensus for disputed findings. SummaryThe fix is correct, minimal, and well-scoped. With WindowsPackageType=None, dotnet build places PolyPilot.exe directly in the framework directory (no RID subfolder). The old code only searched subdirectories and always failed. The fix adds a direct-path check first (Test-Path), falling through to the RID-subdirectory scan as a fallback. Both branches correctly set BuildDir before downstream usage. New FindingsNone. All 3 reviewers independently confirmed no new CRITICAL, MODERATE, or MINOR issues introduced by this change. One reviewer initially flagged a Write-Host format inconsistency in the error message, but adversarial cross-check by the other 2 reviewers determined it is actually consistent with the dominant pattern in the file (multi-argument positional args). Finding discarded. Edge Cases Verified (3/3 consensus)
Existing Review StatusThe automated Expert Code Review already identified 3 findings — all pre-existing or edge-case design choices, none introduced by this PR:
All remain valid observations for follow-up but are not blocking for this PR. CI Status✅ All 5 checks passing Test CoverageNo automated tests exist for relaunch.ps1 (PowerShell launch script). Manual validation recommended for both unpackaged and packaged build output layouts. This is acceptable for a build tooling script. Verdict: ✅ ApproveClean, correct fix. No regressions, no new issues. The updated error message (Could not find PolyPilot.exe in build output) is also an improvement over the old one (Could not detect runtime identifier). Good to merge. |
Problem
After \git clean, running
elaunch.ps1\ fails with:
\
[X] Could not detect runtime identifier in build output
\\
Root Cause
With \WindowsPackageType=None\ (the current csproj setting), \dotnet build\ places \PolyPilot.exe\ directly in \�in/Debug/net10.0-windows10.0.19041.0/\ — there is no RID subdirectory (e.g. \win-x64/). The script only searched subdirectories for the exe and never found it.
Fix
Check the framework directory itself first for the exe. Fall back to scanning RID subdirectories for packaged or cross-compiled builds.