Fix MSIX bundle version generation

[Carlos Nihelton (CarlosNihelton)]

Description

This PR makes the MsixService.Bundle component propagate the version information it already knew to the BundleService.CreateBundleAsync() method which, in turn, passes it as the makeapp.exe's /bv CLI flag, forcing the Bundle.Identity@Version to be the same as the Package.Identity@Version read from the MSIX's Package.appxmanifest file(s) when generating an msix bundle.

Usage Example

This PR doesn't change the usage of winapp.

Related Issue

Closes: #677 .

Type of Change

• 🐛 Bug fix

Checklist

• New tests added for new functionality (if applicable)
• Tested locally on Windows
• Main README.md updated (if applicable)
• docs/usage.md updated (if CLI commands changed)
• Language-specific guides updated (if applicable)
• Sample projects updated to reflect changes (if applicable)
• Agent skill templates updated in docs/fragments/skills/ (if CLI commands/workflows changed)

Screenshots / Demo

AppxMetadata\AppxBundleManifest.xml before those changes:

AppxMetadata\AppxBundleManifest.xml after those changes:

Additional Notes

Without those changes, a multi architecture MSIX bundle is bound to have a non-predictable version, making it impossible to use embedded App installer files with packages created with winappCLI. After those changes, the MSIX bundle version becomes predictable, allowing for the MSIX packages to embed App installer files that point to the multi architecture MSIX bundle with no mistakes.

AI Description

This PR updates the MsixService.Bundle component to ensure that the bundle version matches the package version specified in the Package.appxmanifest file. This change allows for consistent versioning in MSIX bundles, making it easier for developers to manage embedded App Installer files. The usage of the CreateBundleAsync method remains unchanged with respect to the interface, but a new optional bundleVersion argument has been added for version control.

await _service.CreateBundleAsync([file1], output, _taskContext, bundleVersion: new MsixVersion(1, 2, 3, 4));

Breaking Change: None.

[Copilot]

Pull request overview

Fixes MSIX bundle version generation so bundle identity versions match source package manifests.

Changes:

• Validates and models four-part MSIX versions.
• Passes the manifest version to MakeAppx via /bv.
• Adds unit and orchestration coverage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
MsixService.Bundle.cs	Validates and forwards the manifest version.
IBundleService.cs	Adds the optional bundle-version parameter.
BundleService.cs	Emits the MakeAppx /bv argument.
MsixVersion.cs	Implements MSIX version parsing and formatting.
MsixVersionTests.cs	Tests version behavior.
MsixServiceBundleOrchestrationTests.cs	Tests validation and propagation.
BundleServiceTests.cs	Tests /bv command construction.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Nikola Metulev (nmetulev)]

🤖 AI-generated review — produced by the winappcli pr-review skill in GitHub Copilot CLI (orchestrator model: Claude Opus 4.8, with an independent GPT-5.5 cross-check). Findings were validated by building the branch, running the unit tests, and running makeappx directly. This is a non-blocking Comment review — please verify before acting.

Overall: 👍 low-risk, merge-ready — valid issue, correct fix

Issue #677 is real, and this PR fixes it correctly. No critical or high-severity issues were found across 8 review dimensions plus an independent multi-model cross-check. The branch builds clean (0 warnings / 0 errors) and the 50 affected unit tests pass.

The fix also incidentally closes a pre-existing version-string injection vector: before this PR the raw Identity/@Version string was interpolated into the output bundle filename unsanitized. MsixVersion.TryParse now guarantees only a canonical ushort.ushort.ushort.ushort value can reach the filesystem path and the makeappx command line.

Empirical validation

I ran the real Windows SDK makeappx (10.0.26100) against two single-arch packages (both Version=1.2.3.4, x64 + arm64) and read the actual Bundle.Identity/@Version out of each produced AppxBundleManifest.xml:

makeappx bundle scenario	Resulting Bundle.Identity/@Version
no /bv (pre-PR behavior)	2026.720.2212.0 — timestamp-derived
/bv 1.2.3.4 (this PR)	1.2.3.4 — matches the manifest ✅
/bv 0.0.0.0	2026.720.2212.0 — timestamp, silently ignored (see M1)

This reproduces both the bug (nondeterministic timestamp version) and the fix (deterministic, manifest-matching version) with live evidence rather than just documentation.

Findings — Critical: 0 · High: 0 · Medium: 1 · Low: 3

🟡 M1 (medium) — 0.0.0.0 silently defeats the fix

Services/BundleService.cs + Helpers/MsixVersion.cs — MsixVersion.TryParse accepts 0.0.0.0 (explicitly tested), and makeappx treats /bv 0.0.0.0 identically to omitting /bv — reproduced above (it produced the timestamp version, not 0.0.0.0). So for a 0.0.0.0 manifest version the bundle reverts to exactly the nondeterministic behavior this PR set out to eliminate. Real-world impact is low (0.0.0.0 is not a valid shippable MSIX version), so this is edge-completeness rather than a common failure.
Recommendation: reject 0.0.0.0 in the bundle path with a clear error (or document it as a known limitation), and add a 0.0.0.0 orchestration/bundle test.

🔵 L1 (low) — unused comparison API

Helpers/MsixVersion.cs — the struct implements IComparable<MsixVersion>, IComparable, CompareTo(object), and all six relational operators, but production code only uses TryParse/ToString; ordering is exercised solely by unit tests. A readonly record struct MsixVersion(ushort Major, ushort Minor, ushort Build, ushort Revision) would give equality/hashcode for free — add ordering when a real caller needs it. (The custom MSIX parser is justified: System.Version allows 2–4 int parts, while MSIX requires exactly four UInt16 components.)

🔵 L2 (low) — output filename normalization

Services/MsixService.Bundle.cs — the default bundle filename now derives from MsixVersion.ToString() instead of the raw manifest string, so a leading-zero version (e.g. 01.02.03.04) yields App_1.2.3.4_….msixbundle. Cosmetic, but could surprise a script globbing for the old name. Accept it (canonicalization is arguably more correct) or preserve the trimmed raw string for the filename while using MsixVersion only for /bv.

🔵 L3 (low) — partial value-type test coverage

WinApp.Cli.Tests/MsixVersionTests.cs — <=, >=, GetHashCode, boxed Equals/CompareTo(object), and ordering that differs on the major/minor/build components (only a revision-diff is tested) aren't pinned. Cheap to add.

Also verified clean

• Docs / samples: internal-only change (no CLI command or flag added/changed); the docs/usage.md "Multi-architecture bundles" section already reflects the new behavior; no cli-schema.json / winapp-commands.ts regeneration needed.
• Packaging / release: no NuGet MSBuild target parses the bundle version; no version.json bump expected for a fix; npm wrapper unaffected.
• Regression: the new throw on a non-four-part Identity/@Version is fail-fast, not a regression — a valid AppxManifest always carries a required four-part version, and makeappx pack would reject an invalid one anyway. Minor nit: MsixVersion? bundleVersion = null is always reassigned right after the throwing guard, so the later != null filename else-branch is unreachable — trivial, non-blocking.

---

Validation boundary: I validated the two halves of the chain — the built winapp binary emitting /bv "<version>" (unit tests) and makeappx honoring it (live run) — but did not drive the full winapp pack CLI command, which needs real per-arch PE binaries for architecture detection; the PR's own before/after screenshots cover that top-level path.

[Nikola Metulev (nmetulev)]

Thanks for the PR, great fix. I'm good to approve and merge after the issues above are addressed

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/winapp-CLI/WinApp.Cli/Helpers/MsixVersion.cs:48

• The $ anchor in .NET also matches immediately before a final newline, so TryParse("1.2.3.4\n", ...) succeeds; ushort.Parse then accepts that newline as trailing whitespace. This lets a non-canonical value through to the output filename and /bv argument despite this guard. Use absolute string anchors (\A and \z) so the entire original value must match.

        const string pattern = @"^(0|[1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])(\.(0|[1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])){3}$";

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[Zach Teutsch (zateutsch)]

Looks like everything from the reviews has been addressed - approving. Thanks Carlos Nihelton (@CarlosNihelton)

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

docs/usage.md:271

• This exception contradicts the implementation: CreateMsixBundleAsync passes a non-null /bv value for every valid manifest version, including 0.0.0.0. MakeAppx generates a timestamp-based bundle version only when /bv is omitted, so this sentence promises behavior the changed path does not provide. Also, “atributed” is misspelled.

The package version defined in the slices is atributed to the MSIX bundle version, except if it's `0.0.0.0`, in which case a timestamp-based version is automatically generated.