Skip to content

Detect reserved PSGallery module versions#215

Merged
PrzemyslawKlys merged 1 commit intomainfrom
codex/psgallery-hidden-version-detection
Mar 13, 2026
Merged

Detect reserved PSGallery module versions#215
PrzemyslawKlys merged 1 commit intomainfrom
codex/psgallery-hidden-version-detection

Conversation

@PrzemyslawKlys
Copy link
Member

Summary

  • detect reserved PowerShell Gallery module versions through the exact package metadata endpoint when the normal feed omits hidden or deleted versions
  • keep the existing feed-based path for normal listed and unlisted entries, but step past reserved hidden versions inside the requested X-pattern range
  • add focused tests for both feed-visible unlisted versions and feed-hidden reserved versions

Validation

  • dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj -c Release -f net10.0 --filter "FullyQualifiedName~ModuleVersionStepperTests" --no-restore
  • dotnet build .\PowerForge\PowerForge.csproj -c Release -f net472 --nologo
  • pwsh -NoProfile -Command "Import-Module .\PSPublishModule\bin\Release\net8.0\PSPublishModule.dll -Force; Step-Version -ExpectedVersion '3.0.X' -Module 'PSPublishModule' -Advanced | ConvertTo-Json -Compress -Depth 5"
  • powershell.exe -NoProfile -Command "Import-Module .\PSPublishModule\bin\Release\net472\PSPublishModule.dll -Force; Step-Version -ExpectedVersion '3.0.X' -Module 'PSPublishModule' -Advanced | ConvertTo-Json -Compress -Depth 5"

Notes

  • live PSGallery currently returns 200 for https://www.powershellgallery.com/api/v2/Packages(Id='PSPublishModule',Version='3.0.0') and for the api/v2/package download URL, while FindPackagesById() omits 3.0.0
  • the working tree still has your local publish toggle in Module/Build/Build-Module.ps1, but it is intentionally not part of this commit

@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@claude
Copy link

claude bot commented Mar 13, 2026

Code Review

Overall, this is a well-structured solution to a real PSGallery limitation. The logic is sound and the implementation is consistent with the existing codebase patterns. Here are my observations:


Correctness / Logic

Probe starting point when feed returns a cross-range version
In TryResolveReservedPowerShellGalleryVersion, when the feed's latest is 2.0.27 and the pattern is 3.0.X, ComputeNextVersion("3.0.X", cursor=2.0.27) correctly produces 3.0.0 as the first probe candidate. The test in FakeReservedPowerShellGalleryHandler validates exactly this scenario. ✓

Guard in FakePowerShellGalleryFeedHandler
The added 404 guard for non-FindPackagesById requests was a necessary fix — without it, the existing Step_UsesUnlistedPowerShellGalleryVersionWhenStepping test would have the reserved probe accidentally hitting the first-page XML as a Packages(...) response and incorrectly finding a "reserved" version. ✓


Performance Concern

Minimum 4 HTTP round-trips added to every non-prerelease PSGallery version step

The probe loop's early-exit condition when no hits have been found yet is missCount >= maxConsecutiveMissesBeforeHit (= 4). This means every call to Step(...) for a PSGallery module now makes at least 4 additional sequential HTTP requests via GetAwaiter().GetResult(), each with a 20-second timeout. In the worst-case (all timeouts), that's 4 × 20s = 80 seconds of added latency just from the cold-start guard.

For the very common scenario where no reserved versions exist, these 4 probes are wasted. Consider whether the threshold could be lowered (e.g., maxConsecutiveMissesBeforeHit = 2) or whether the probe could be skipped when the feed already returned a version in the same major.minor as the pattern (since a reserved version would have to be above the feed's result to matter).

// ModuleVersionStepper.cs:180-181
const int maxConsecutiveMissesAfterHit = 3;
const int maxConsecutiveMissesBeforeHit = 4;  // 4 HTTP requests minimum for every step

Missing Logging on Timeout / No-Hit

VersionExists silently returns false on TaskCanceledException. If this fires during normal operation the caller has no indication that a probe timed out rather than returned a genuine 404. A verbose-level log entry in the catch (TaskCanceledException) path (similar to the unlisted-version log) would make diagnosing slow CI runs much easier:

// PowerShellGalleryVersionFeedClient.cs:102-104
catch (TaskCanceledException)
{
    // Consider: _logger.Verbose($"Timeout checking reserved version {version} for '{packageId}'.");
    return false;
}

Similarly, when the probe loop completes and latestReserved is null, there is no log. A single verbose line confirming "no reserved versions found in range" would help when debugging version-stepping issues.


Test Coverage Gaps

  1. prerelease = true early-return — The if (prerelease) return null; guard at the top of TryResolveReservedPowerShellGalleryVersion has no test. A one-liner fact would suffice.

  2. Reserved version lower than feed version — The condition reservedVersion.CompareTo(galleryVersion) > 0 guards against using a reserved version that's already superseded by the feed. There's no test where the reserved probe finds a version but the feed already has a higher one.

  3. Timeout scenario — Not critical since it degrades gracefully, but a test asserting that a timeout on VersionExists still produces a sensible result would add confidence.


Minor

  • The whitespace-only reformatting of the Step(...) method signature (trailing spaces) is harmless but adds noise to the diff.
  • using System.Globalization; reordering is fine — alphabetical is tidier.
  • The OperationCanceledException base class rather than TaskCanceledException may be worth considering, since HttpClient cancellation can surface as either depending on the runtime version, though TaskCanceledException inherits from OperationCanceledException so the existing catch still works.

Summary

The core feature is correct and the test for the primary scenario is solid. The main concern is the minimum 4 extra HTTP requests per version-step even when no reserved versions exist — worth evaluating against the expected frequency of this code path and whether the maxConsecutiveMissesBeforeHit threshold can be reduced without sacrificing reliability.

@PrzemyslawKlys PrzemyslawKlys merged commit c7e77e8 into main Mar 13, 2026
6 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the codex/psgallery-hidden-version-detection branch March 13, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant