fix: clear stale prerelease during manifest refresh

[PrzemyslawKlys]

Summary

• clear the in-memory prerelease value when a manifest segment omits Prerelease
• stop refresh-only builds from carrying forward a stale prerelease tag from the existing psd1
• add a regression test covering the RefreshPSD1Only rebuild path

Why

When a module manifest already contained a prerelease value such as Preview1, removing Prerelease from the build configuration did not actually clear it during a rebuild. ModulePipelineRunner.Plan seeded plan.PreRelease from the existing psd1 and only overwrote it when the new manifest configuration supplied a non-empty prerelease value.

That meant:

• the manifest refresh could remove the psd1 key
• but the in-memory plan still carried the old prerelease value
• downstream version formatting and build summaries could continue showing -Preview1

Validation

• dotnet test PowerForge.Tests/PowerForge.Tests.csproj --filter "FullyQualifiedName~Run_RefreshPSD1Only_ClearsStalePrereleaseWhenManifestSegmentOmitsIt|FullyQualifiedName~Run_WritesPrereleaseToPsDataAndRemovesTopLevelPrerelease|FullyQualifiedName~Run_RefreshesManifestMetadataAndClearsStaleValues"

Notes

A broader manifest-refresh test slice on current main still has one unrelated pre-existing failure in Run_RemovesInboxAndDuplicatedExternalDependenciesFromManifest, so I validated this change with the prerelease-focused regression slice above.

[chatgpt-codex-connector]

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]

Code Review

Summary

This is a focused, correct bug fix for a real problem: a stale prerelease value persisting in-memory when a manifest segment omits Prerelease. The change is minimal and well-reasoned.

---

Fix Analysis (ModulePipelineRunner.Plan.cs, line 205)

Before:

if (!string.IsNullOrWhiteSpace(m.Prerelease)) preRelease = m.Prerelease;

After:

preRelease = string.IsNullOrWhiteSpace(m.Prerelease) ? null : m.Prerelease.Trim();

The fix is correct: making the assignment unconditional means the config segment is now authoritative — an absent Prerelease explicitly clears whatever the baseline seeded. The added .Trim() is a small bonus hardening.

---

One design consideration worth discussing

The other fields in this same block still use the guard pattern (only update when non-empty):

if (!string.IsNullOrWhiteSpace(m.ModuleVersion)) expectedVersion = m.ModuleVersion;
if (!string.IsNullOrWhiteSpace(m.Author)) author = m.Author;
// ...

Prerelease is now the only field where an empty/omitted value explicitly clears a baseline value. This asymmetry is intentional and arguably correct (prerelease removal is a meaningful intent, not just an omission), but it is worth documenting. A future maintainer aligning other fields to match this pattern may inadvertently break the clear-semantics if they don't understand why it differs.

A brief inline comment would help:

// Unconditional: an absent Prerelease explicitly clears any baseline prerelease value.
preRelease = string.IsNullOrWhiteSpace(m.Prerelease) ? null : m.Prerelease.Trim();

---

Scope note

The fix addresses the case where a ConfigurationManifestSegment omits Prerelease. The related case — where no manifest segment is present at all — still seeds preRelease from the baseline (lines 156–157 of the same file), which is the correct behavior ("I didn't tell you to refresh the manifest, so leave it alone"). The test covers exactly the right scenario.

---

Test quality (ModulePipelineRefreshManifestOnlyTests.cs)

The new test is well-structured:

• Sets up a realistic starting state (existing psd1 with Preview1)
• Asserts plan.PreRelease is null before the build runs, confirming the fix at the planning layer
• Runs the full pipeline and asserts the key is absent from the written psd1
• Checks both TryGetTopLevelString and TryGetPsDataStringArray to cover both manifest storage formats
• Cleans up via temp directory in finally

No issues here.

---

Verdict

Approve with minor suggestion. The fix is correct, the test adds meaningful regression coverage, and the PR is appropriately scoped. The only suggestion is an optional inline comment to explain the intentional asymmetry between Prerelease and the other manifest fields.

[PrzemyslawKlys]

Addressed the CI failure and the review suggestion on the current head:

• fixed the net472 Windows build break by making the Prerelease clear assignment nullability-safe for the compiler
• kept the explanatory inline comment clarifying why Prerelease is intentionally handled differently from the other manifest fields

Local validation:

• dotnet build .\PowerForge\PowerForge.csproj -c Release -f net472 --nologo
• dotnet build .\PowerForge.PowerShell\PowerForge.PowerShell.csproj -c Release -f net472 --nologo
• dotnet build .\PSPublishModule\PSPublishModule.csproj -c Release -f net472 --nologo
• dotnet test PowerForge.Tests/PowerForge.Tests.csproj --filter "FullyQualifiedName~Run_RefreshPSD1Only_ClearsStalePrereleaseWhenManifestSegmentOmitsIt|FullyQualifiedName~Run_WritesPrereleaseToPsDataAndRemovesTopLevelPrerelease|FullyQualifiedName~Run_RefreshesManifestMetadataAndClearsStaleValues"

[claude]

PR Review: fix: clear stale prerelease during manifest refresh

The fix is correct and well-targeted. The one-line change in ModulePipelineRunner.Plan.cs closes the stale-prerelease bug cleanly, and the regression test covers the exact failure path described in the PR.

Code Quality

The new line is clear and idiomatic. The null-forgiving operator is safe because the IsNullOrWhiteSpace guard already rules out null/empty on the right-hand branch.

Consistency observation (non-blocking): The surrounding fields (Author, CompanyName, Description, etc.) still use the old guard pattern. That means those fields can never be cleared from the baseline either. Whether that is intentional or a future follow-up is outside the scope of this PR.

Test Coverage

The new test Run_RefreshPSD1Only_ClearsStalePrereleaseWhenManifestSegmentOmitsIt is well-structured. It seeds a real .psd1 with a stale prerelease value, runs the pipeline with a manifest segment that omits Prerelease, then asserts both that plan.PreRelease is null (in-memory) AND that the written .psd1 contains no Prerelease key at either the top-level or PSData level (on-disk). That two-layer assertion is exactly right for this class of bug.

Optional addition: A parallel RefreshPSD1Only variant of Run_WritesPrereleaseToPsDataAndRemovesTopLevelPrerelease (segment with Prerelease set + RefreshPSD1Only = true) would lock in the positive/non-regression side for the refresh-only path. Not a blocker - the full-build path is already covered in ModulePipelineManifestRefreshTests.cs.

Potential Issues

None identified. The change is narrow and does not affect any other fields. The .Trim() call is a minor bonus that normalises accidental whitespace in the prerelease string.

Verdict: The fix is correct, minimal, and well-tested. Good to merge.