From 1930e67305614782b72ccbb77504a0582a43802e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 18:25:22 +0000 Subject: [PATCH] Report every unresolvable SDK package, not just the first $ErrorActionPreference is Stop in update-sdks.ps1, which makes a bare Write-Error terminating. The two Write-Error calls in the resolution loop were not wrapped, so the first unresolvable package threw straight out of the script and the `continue` beneath each one was dead code. A maintainer saw one failure per run: fix it, rerun, discover the next. Failures are now named on the error stream as they happen and collected, and the run fails once after every package has been looked at. The throw stays ahead of the write phase, so a run that could not resolve part of the family does not converge the rest and leave the repository on two versions of the same SDK -- the partially applied state this script exists to repair. Two test cases cover the path, which had none: every case in the suite passed -Version explicitly, which skips Get-LatestReleasedVersion entirely. Neither touches the network -- one points at a closed port, the other at an in-process HttpListener stub that provokes both failure branches at once. Fixes #27 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012betVHk3gFcj5RYkEe4vrm --- docs/sdk-pinning.md | 6 ++ scripts/tests/update-sdks.tests.ps1 | 152 +++++++++++++++++++++++++++- scripts/update-sdks.ps1 | 22 +++- 3 files changed, 176 insertions(+), 4 deletions(-) diff --git a/docs/sdk-pinning.md b/docs/sdk-pinning.md index d25811c..83dfeb7 100644 --- a/docs/sdk-pinning.md +++ b/docs/sdk-pinning.md @@ -30,6 +30,12 @@ target for the rest of the repository rather than being rolled backwards. Edits are textual and scoped to the version that follows the package name, so key order, formatting, comments, unrelated entries and the trailing newline all survive. +A package whose version cannot be resolved — a feed hiccup, a package that was never published, +a name that has since been retired — fails the run. Every package is still looked at first, so +one run names every unresolvable package rather than one per run, and the run fails before +anything is written. Reporting the failures and converging the rest would leave the repository +on two versions of the family, which is the state this exists to repair. + ## What it does not do It does not open a pull request. It builds and tests the repository with the new pins and diff --git a/scripts/tests/update-sdks.tests.ps1 b/scripts/tests/update-sdks.tests.ps1 index c851048..4bd75bb 100644 --- a/scripts/tests/update-sdks.tests.ps1 +++ b/scripts/tests/update-sdks.tests.ps1 @@ -6,8 +6,10 @@ Every case here is a defect observed in the previous workflow against the real feed, not a hypothetical. Run with pwsh: ./scripts/tests/update-sdks.tests.ps1 - -Version is used throughout so the cases do not depend on the network or on which - version happens to be current the day they run. + -Version is used for the rewriting cases so they do not depend on the network or on which + version happens to be current the day they run. The resolution cases deliberately omit it, + because that is the only path that calls the feed at all, and they point the script at a + closed port or at an in-process stub feed rather than at nuget.org. #> [CmdletBinding()] param() @@ -60,6 +62,123 @@ function Test-Case { } } +# A flat container index, served in process. The feed is the only thing the resolution path +# touches that the rewriting cases never reach, so it is the only thing that has to be faked. +# Versions maps a lower-case package id -- the flat container addresses packages in lower case -- +# to the versions it publishes, and an id that is absent is answered with 404 so a lookup +# failure and a package with no release can both be provoked in one run. +function Start-StubFeed { + param([hashtable]$Versions) + + $listener = [System.Net.HttpListener]::new() + $port = 0 + foreach ($candidate in 18080..18179) { + try { + $listener.Prefixes.Clear() + $listener.Prefixes.Add("http://127.0.0.1:$candidate/") + $listener.Start() + $port = $candidate + break + } + catch { + # The port is taken. Try the next one. + } + } + + if ($port -eq 0) { + throw 'No free port in 18080-18179 for the stub feed.' + } + + $job = Start-ThreadJob -ScriptBlock { + param($Listener, $Map) + + try { + while ($Listener.IsListening) { + $context = $Listener.GetContext() + # //index.json + $id = $context.Request.Url.Segments[1].TrimEnd('/') + + if ($Map.ContainsKey($id)) { + $body = '{"versions":["' + ($Map[$id] -join '","') + '"]}' + $bytes = [System.Text.Encoding]::UTF8.GetBytes($body) + $context.Response.ContentType = 'application/json' + $context.Response.OutputStream.Write($bytes, 0, $bytes.Length) + } + else { + $context.Response.StatusCode = 404 + } + + $context.Response.Close() + } + } + catch { + # Stopping the listener is how this loop ends. + } + } -ArgumentList $listener, $Versions + + return [pscustomobject]@{ Listener = $listener; Job = $job; Url = "http://127.0.0.1:$port" } +} + +function Stop-StubFeed { + param($Feed) + + $Feed.Listener.Stop() + $Feed.Listener.Close() + Remove-Job -Job $Feed.Job -Force +} + +# The resolution path fails the run rather than rewriting anything, so these cases assert on the +# message the run fails with and on the fixture being left alone, not on a rewritten file. +function Test-ResolutionFailure { + param([string]$Name, [hashtable]$Files, [string]$FeedUrl, [string[]]$Reports, [hashtable]$Unchanged) + + $script:Count++ + $root = New-Fixture -Files $Files + try { + $message = $null + try { + & $script:Script -Path $root -FeedUrl $FeedUrl 6>$null 2>$null | Out-Null + } + catch { + $message = $_.Exception.Message + } + + if ($null -eq $message) { + $script:Failures++ + Write-Host "FAIL $Name" -ForegroundColor Red + Write-Host ' the run was expected to fail, and did not.' -ForegroundColor Red + return + } + + $missing = @($Reports | Where-Object { $message -notlike "*$_*" }) + if ($missing.Count -gt 0) { + $script:Failures++ + Write-Host "FAIL $Name" -ForegroundColor Red + Write-Host " the failure never named: $($missing -join ', ')" -ForegroundColor Red + Write-Host " it said: $message" -ForegroundColor Red + return + } + + if ($Unchanged) { + foreach ($file in $Unchanged.Keys) { + $actual = Get-Content -LiteralPath (Join-Path $root $file) -Raw + if ($actual -ne $Unchanged[$file]) { + $script:Failures++ + Write-Host "FAIL $Name" -ForegroundColor Red + Write-Host " $file was written despite the failure:" -ForegroundColor Red + Write-Host " $actual" -ForegroundColor Red + return + } + } + } + + Write-Host "ok $Name" -ForegroundColor Green + } + finally { + Remove-Item -LiteralPath $root -Recurse -Force + } +} + # The bare ktsu.Sdk entry. "ktsu.Sdk" -like "ktsu.Sdk.*" is False and the old csproj pattern # required a suffix, so the package nearly every repository pins was never tracked. VST, which # pins only ktsu.Sdk, reported "No ktsu SDKs found" and sat on 2.8.0 while the org moved on. @@ -110,6 +229,35 @@ Test-Case -Name 'an already-current repository is untouched' -Version '2.29.0' ` -Files @{ 'global.json' = "{`n `"msbuild-sdks`": {`n `"ktsu.Sdk`": `"2.29.0`"`n }`n}`n" } ` -Expected @{ 'global.json' = "{`n `"msbuild-sdks`": {`n `"ktsu.Sdk`": `"2.29.0`"`n }`n}`n" } +# $ErrorActionPreference is Stop inside the script, which makes a bare Write-Error terminating. +# The first unresolvable package therefore threw out of the script and the `continue` beneath it +# never ran, so a maintainer saw one failure per run: fix it, rerun, discover the next. The +# packages are grouped alphabetically, so ktsu.Sdk failing used to hide the other two entirely. +$threePackages = '{"msbuild-sdks":{"ktsu.Sdk":"1.0.0","ktsu.Sdk.App":"1.0.0","ktsu.Sdk.Tool":"1.0.0"}}' +Test-ResolutionFailure -Name 'every unresolvable package is reported, not just the first' ` + -Files @{ 'global.json' = $threePackages } ` + -FeedUrl 'http://127.0.0.1:9' ` + -Reports @('ktsu.Sdk.App', 'ktsu.Sdk.Tool') ` + -Unchanged @{ 'global.json' = $threePackages } + +# The same defect on the other branch, and the reason the run must fail before it writes. Here +# ktsu.Sdk resolves to a version the fixture is behind, so a fix that reported the failures and +# carried on would converge that one reference and leave the other two pinned to 1.0.0 -- the +# partially applied state this script exists to repair. ktsu.Sdk.App 404s (the catch branch) and +# ktsu.Sdk.Tool publishes only a prerelease (the "no released version" branch). +$mixed = '{"msbuild-sdks":{"ktsu.Sdk":"1.0.0","ktsu.Sdk.App":"1.0.0","ktsu.Sdk.Tool":"1.0.0"}}' +$feed = Start-StubFeed -Versions @{ 'ktsu.sdk' = @('2.29.0'); 'ktsu.sdk.tool' = @('1.0.0-pre.1') } +try { + Test-ResolutionFailure -Name 'a resolvable package neither hides a failure nor is written past one' ` + -Files @{ 'global.json' = $mixed } ` + -FeedUrl $feed.Url ` + -Reports @('ktsu.Sdk.App', 'ktsu.Sdk.Tool') ` + -Unchanged @{ 'global.json' = $mixed } +} +finally { + Stop-StubFeed -Feed $feed +} + Write-Host '' if ($script:Failures -gt 0) { Write-Host "$script:Failures of $script:Count case(s) failed." -ForegroundColor Red diff --git a/scripts/update-sdks.ps1 b/scripts/update-sdks.ps1 index ef18a98..832d9fa 100644 --- a/scripts/update-sdks.ps1 +++ b/scripts/update-sdks.ps1 @@ -162,6 +162,14 @@ foreach ($group in $references | Group-Object Name | Sort-Object Name) { Write-Host " $($group.Name): $pinned" } +# A package that cannot be resolved is a failure, but it is not a reason to stop looking at the +# rest of them. $ErrorActionPreference is Stop in this scope, which makes a bare Write-Error +# terminating, so the first unresolvable package used to throw straight out of the script and +# the `continue` beneath it was dead code. A maintainer saw one failure per run and had to fix +# and rerun to discover the next. Failures are named on the error stream as they happen, and +# the run fails once, after every package has been looked at. +$failed = [System.Collections.Generic.List[string]]::new() + $targets = @{} foreach ($group in $references | Group-Object Name | Sort-Object Name) { $name = $group.Name @@ -179,12 +187,14 @@ foreach ($group in $references | Group-Object Name | Sort-Object Name) { # A lookup failure must not be reported as "up to date". That conflation is what let # this run green and do nothing every week: a parse error inside the lookup was caught # and turned into a null, and a null read as "no update available". - Write-Error "Could not resolve the latest version of $name from $FeedUrl : $($_.Exception.Message)" + Write-Error "Could not resolve the latest version of $name from $FeedUrl : $($_.Exception.Message)" -ErrorAction Continue + $failed.Add($name) continue } if (-not $latest) { - Write-Error "$name has no released version on $FeedUrl." + Write-Error "$name has no released version on $FeedUrl." -ErrorAction Continue + $failed.Add($name) continue } @@ -201,6 +211,14 @@ foreach ($group in $references | Group-Object Name | Sort-Object Name) { $targets[$name] = $latest } +# Failing here rather than earlier is what makes one run report every unresolvable package. It +# is still before anything is written, so a run that could not resolve part of the family does +# not leave the repository half-converged -- which is the very state this script exists to +# repair. The messages above carry the reasons; this carries the verdict. +if ($failed.Count -gt 0) { + throw "Could not resolve $($failed.Count) of $($targets.Count + $failed.Count) $Prefix package(s): $($failed -join ', ')" +} + $stale = @($references | Where-Object { $targets.ContainsKey($_.Name) -and $_.Version -ne $targets[$_.Name] }) if ($stale.Count -eq 0) {