Skip to content

fix(ci): authorize MakeAppx zero-version bundles - #34

Merged
RomneyDa merged 1 commit into
mainfrom
fix/zero-version-msixbundle
Sep 12, 2026
Merged

fix(ci): authorize MakeAppx zero-version bundles#34
RomneyDa merged 1 commit into
mainfrom
fix/zero-version-msixbundle

Conversation

@RomneyDa

Copy link
Copy Markdown
Member

Summary

  • omit /bv when the requested child package version is 0.0.0.0 because MakeAppx assigns an outer date/time bundle version for the all-zero case
  • validate the generated four-part outer bundle version independently while continuing to require both embedded MSIX packages to be exactly version 0.0.0.0 and byte-identical to authorized inputs
  • add regression coverage and document the one-off proof behavior

Failure evidence

Official run 34681944373 passed all package builds and bundle composition, then failed before Azure login because MakeAppx emitted outer bundle version 2026.912.815.0 while the embedded packages remained 0.0.0.0. No signing or release occurred.

Validation

Fresh Windows CI must pass before merge.

@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 12, 2026
@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 12, 2026, 4:20 AM ET / 08:20 UTC.

ClawSweeper review

What this changes

Adjusts Windows bundle creation and signing validation to accept MakeAppx-generated outer versions while preserving approved embedded packages, with regression coverage and documentation.

Merge readiness

Ready for maintainer review

This remains a useful fix: current main still rejects the reported MakeAppx bundle identity, and no merged replacement covers the correction. No blocking defect was found in the proposed patch.

Priority: P2
Reviewed head: 681887bf229959cabaa8be3d332402dad14e9354

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused correction with relevant regression coverage and no identified blocking defect; Windows execution was not independently verified.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Its captured body supplies before-fix failure context, while the added fixtures are supplemental tests rather than after-fix MakeAppx execution evidence.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Its captured body supplies before-fix failure context, while the added fixtures are supplemental tests rather than after-fix MakeAppx execution evidence.
Evidence reviewed 7 items Verified introduced change: The local diff matches the supplied complete introduction evidence: five files change, with no workflow, dependency, permission, or release-policy modifications.
Current main still contains the failing comparison: Main compares the outer bundle identity directly with the approved child-package version. The captured PR body reports that official run 34681944373 produced outer version 2026.912.815.0 with zero-version children and stopped at authorization; that run was not independently executed or inspected here.
Content authorization remains intact: The changed validator bounds the outer version while retaining publisher/name checks, exactly two distinct architectures, approved child versions, expected filenames, and SHA-256 equality with authorized standalone packages.
Findings None None.
Security None None.

How this fits together

The Windows packaging pipeline combines x64 and ARM64 packages into one installable bundle. Signing authorization checks its identity and embedded package contents before the workflow can request Azure signing credentials.

flowchart TD
  A[Approved release policy] --> D[Signing authorization]
  B[x64 and ARM64 packages] --> C[MakeAppx bundle creation]
  C --> D
  D --> E{Identity and contents valid?}
  E -->|Yes| F[Azure signing]
  E -->|No| G[Stop before credentials]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +27 net lines; tests +17 net lines; documentation +6 lines The production growth supports the reported version mismatch and adds bounded identity validation.

Technical review

Best possible solution:

Accept MakeAppx’s outer identity for the approved zero-version proof while keeping embedded package versions and bytes bound to the release policy.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: the reported timestamp outer identity necessarily fails main’s equality check against the approved 0.0.0.0 child version. A Windows MakeAppx run was not performed during this read-only review.

Is this the best way to solve the issue?

Yes, for the current zero-version signing proof: the patch separates container-version validation from authorization of the embedded packages without changing signing credentials or workflow permissions.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 54d4c43adfd8.

Labels

Label changes:

  • add P2: This repairs a bounded official-packaging failure without evidence of disruption to installed users.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Its captured body supplies before-fix failure context, while the added fixtures are supplemental tests rather than after-fix MakeAppx execution evidence.

Label justifications:

  • P2: This repairs a bounded official-packaging failure without evidence of disruption to installed users.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from the external-contributor proof gate. Its captured body supplies before-fix failure context, while the added fixtures are supplemental tests rather than after-fix MakeAppx execution evidence.

Evidence

What I checked:

  • Verified introduced change: The local diff matches the supplied complete introduction evidence: five files change, with no workflow, dependency, permission, or release-policy modifications. (scripts/Build-MSIXBundle.ps1:108, 681887bf2299)
  • Current main still contains the failing comparison: Main compares the outer bundle identity directly with the approved child-package version. The captured PR body reports that official run 34681944373 produced outer version 2026.912.815.0 with zero-version children and stopped at authorization; that run was not independently executed or inspected here. (scripts/Test-SigningInputs.ps1:364, 54d4c43adfd8)
  • Content authorization remains intact: The changed validator bounds the outer version while retaining publisher/name checks, exactly two distinct architectures, approved child versions, expected filenames, and SHA-256 equality with authorized standalone packages. (scripts/Test-SigningInputs.ps1:379, 681887bf2299)
  • Signing boundary: Official authorization runs only for manual official builds on main; the separate signing job depends on successful authorization and uses the protected release-signing environment. These boundaries are unchanged. (.github/workflows/gateway-msix.yml, 681887bf2299)
  • Focused supplemental tests: Builder tests retain explicit nonzero /bv coverage and add zero-version omission coverage using a fake MakeAppx executable. Signing fixtures now exercise timestamp outer identities, while existing rejection cases include substituted embedded packages. Tests were inspected, not executed. (scripts/Test-Build-MSIXBundle.Tests.ps1:78, 681887bf2299)
  • Merged feature context: GitHub confirms feat(ci): publish signed Gateway MSIX releases #33 merged as 54d4c43 and established the initial zero-version signing proof. It does not contain this correction. Commit metadata and its patch connect RomneyDa to prior merged work on both changed production scripts. Deeper local follow-history and blame attempts encountered unavailable objects; GitHub commit evidence supplied the immediate history. (scripts/Test-SigningInputs.ps1:422, 54d4c43adfd8)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@RomneyDa
RomneyDa merged commit 735bd4e into main Sep 12, 2026
16 checks passed
@RomneyDa
RomneyDa deleted the fix/zero-version-msixbundle branch September 12, 2026 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant