Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs real behavior proof before merge. Reviewed September 15, 2026, 6:46 PM ET / 22:46 UTC (Revision 4). ClawSweeper reviewWhat this changesBuilds x64 and ARM64 Windows application packages in CI, provides signed development downloads, and attaches unsigned Store submission assets to alpha releases. Merge readiness⛔ Blocked before merge - 4 items remain This remains useful work beyond the merged packaging implementation. No concrete code defect was found, and both current-head native packaging jobs passed; installation and upgrade proof remains incomplete. Priority: P2 Review scores
Verification
How this fits togetherThe packaging pipeline turns Companion source into Windows packages for testers and Store submission. CI validates the artifacts, while the release pipeline attaches unsigned Store inputs only to canonical alpha releases. flowchart TD
A[Source and release version] --> B[Native x64 and ARM64 builds]
B --> C[Signed development packages]
B --> D[Unsigned Store packages]
C --> E[Actions tester downloads]
D --> F[Validate provenance and hashes]
F --> G{Canonical alpha tag}
G --> H[GitHub submission assets]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the existing packaging owners and alpha-only submission boundary, with demonstrated signed installation and settings-preserving Dev upgrades before making tester downloads merge-ready. Do we have a high-confidence way to reproduce the issue? Not applicable: this adds CI downloads and release assets rather than repairing a reproduced product failure. Is this the best way to solve the issue? Yes, the implementation appropriately reuses existing builders and release gates; its remaining readiness gap is real installation and upgrade coverage. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 3c43751b2bac. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles) |
Build verified x64 and ARM64 workflow downloads through the existing packaging scripts. Bound Dev CI revisions, stage only public signing material, and gate selected MSIX jobs without enabling MSIX release publishing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb0e5a80-d2cf-41b0-9fb0-21eb32623526
Keep Dev-signed packages as workflow artifacts and stable assets unchanged. Add manual default-branch alpha dispatch with existing tag gates, validate Store staging provenance, and document submission/version constraints. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb0e5a80-d2cf-41b0-9fb0-21eb32623526
74b6a8b to
e348ef7
Compare
karkarl
left a comment
There was a problem hiding this comment.
Adversarial review verdict: no actionable findings.
Independent reviews by Claude Opus 4.7 and GPT Codex 5.3 agreed, with no disputed findings. Reviewed commit: e348ef7.
Validation: all four focused CI/MSIX contract suites passed locally (test-ci-gate-results.ps1, test-ci-workflow-contract.ps1, test-msix-ci-artifacts.ps1, and test-msix-alpha-release.ps1). The alpha suite initially failed to locate Git Bash and passed after prioritizing the full Git installation in the process PATH. GitHub's CI Gate and both x64 and ARM64 MSIX artifact jobs also passed.
Limitation: actual alpha release publication remains unverified because the PR's release job was skipped.
Summary
Build x64 and ARM64 MSIX packages in CI, and attach the unsigned Store submission packages and metadata to canonical alpha GitHub pre-releases for manual upload to Partner Center.
Dev-signed tester packages remain Actions artifacts only. Stable, stable-correction, and non-alpha releases keep the existing EXE/ZIP asset set.
Rebased onto
mainat3c43751b. #1328 (feat: add Microsoft Store MSIX packaging) has merged, so its packaging changes are no longer part of this PR's diff. Current head:e348ef7d9ffdef4b76f438d60484ffd2d6529bcc.Partially addresses #1375. Store distribution, automatic submission, signed-package retrieval, and coexistence/migration in #1374 remain separate work.
Behavior
vX.Y.Z-alpha.NreleaseAlpha releases are public and visible on the Releases page, but use
prerelease: trueandmake_latest: false. Existing 30-day alpha retention applies. Manual dispatch bypasses only the clock check; published-head, pending non-alpha tag, GitVersion, tag ownership, CI Gate, and signing checks remain. A previously published head is skipped.The Store assets are submission inputs, not installers. Upload the MSIX files manually to Partner Center. This workflow does not submit, retrieve, or publish Store-signed packages.
Store version caveat:
X.Y.Z-alpha.Nstill produces Windows package versionX.Y.Z.0. Multiple alpha tags sharing a base version can collide with earlier Store submissions; check Partner Center before uploading. Dev versions remainX.Y.Z.<github.run_number>, with explicit bounds and no wrapping.Artifact and signing safeguards
The existing builders remain authoritative:
Build-StoreMsix.ps1for validated unsigned Store inputs, andbuild.ps1 -Msix DevplusExport-DevMsixArtifact.ps1for Dev downloads.Each selected MSIX matrix leg provisions a disposable non-exportable Dev certificate and removes its signer/trust in an
always()cleanup step. Only the public certificate is exported. Dev uploads use an explicit four-file allowlist, never the whole packaging directory or a PFX.Alpha release staging requires both architectures, a clean expected source commit, Store identity/publisher, Release configuration, expected architecture/version/filename, unsigned boolean metadata, and matching package hashes. Both inputs are validated before any release output is created. Package and metadata bytes are preserved.
Alpha-only attachments:
Installing a Dev artifact remains an explicit trust decision. It uses the existing Dev identity and may upgrade an existing Dev installation rather than create an isolated app. No product permissions, node/MCP commands, user settings migration, or new production signing secret are introduced.
Required proof pools
windows-11-arm64: Native packaging passed on the historical hosted run below. Current-head native ARM64 build/install/launch proof remains pending; the full pool is not satisfied.windows-clean-installer-upgrade: Historical existing-host x64 installation/launch is recorded below. Clean installation and controlled retained-settings upgrade across different runner certificates remain unverified.These declarations do not establish Store readiness or waive the broader rollout gates.
Validation
The following passed against the source tree committed as
e348ef7d; no code changed between these runs and the commit..\build.ps1dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restoredotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore.\scripts\test-msix-alpha-release.ps1.\scripts\test-msix-ci-artifacts.ps1.\scripts\test-ci-workflow-contract.ps1.\scripts\test-ci-gate-results.ps1.\scripts\test-ci-change-classifier.ps1.\scripts\test-stable-correction-release-validator.ps1.\scripts\validate-docs.ps1andgit diff --checkTest restores completed before
--no-restoreruns. Tests used this worktree'sOPENCLAW_REPO_ROOTand isolatedOPENCLAW_TRAY_DATA_DIR.Local environment: prerequisite diagnostics passed. The host's inherited NuGet source was disabled, and the v3 endpoint failed TLS negotiation. Final build/tests used a session-only
RestoreConfigFilepointing to the officialhttps://www.nuget.org/api/v2/feed. No tracked dependency, warning policy, audit setting, or CI configuration was changed for this workaround; the final runs did not use the older runtime-patch override.Hosted CI for
e348ef7d: Build and Test 35031354398 is running. Fast validation, classification, and release metadata passed; both native MSIX jobs are in progress. CodeQL 35031354557 passed. The successful old run below is historical, not proof of this rebased head.Review: a read-only review identified that manual feature-branch dispatch could affect GitVersion normalization despite default-branch checkout. Fixed with
disableNormalization: true, covered by a workflow regression and the actual GitVersion proof below. Structured autoreview was retried after the fix: local mode hit its diff-size guard; the committed branch bundle succeeded, but the Codex executable is unavailable. No guards were bypassed and no clean structured-review result is claimed.Real behavior proof
Current alpha stager, genuine historical inputs: ran
Stage-StoreMsixReleaseAssets.ps1against both Store artifacts from run34644112448, with expected clean source17fce3fa321ed157c4ef0134d9261549f403cc30and test alpha version2026.7.2-alpha.4. It produced exactly the four release filenames above, preserving package and metadata bytes. Package hashes still matched the historical table below. This proves current staging behavior, not a current-head package rebuild or a published release.Actual GitVersion 6.8.2 proof: in an isolated clone checked out to default-branch commit
3c43751b,/nonormalize /nofetch /nocacheproduced2026.9.4-alpha.5both withGITHUB_REF=refs/heads/mainand with a different feature-branch ref/SHA. The reported branch and source remainedmainand3c43751b. The real working branch and remote tags were not altered.No alpha tag/release was created and no Partner Center submission was made for validation. The public release attachment path and manual Actions dispatch still need an eligible, approved post-merge release run.
Historical hosted packages and developer x64 launch, before this rebase
Build and Test run 34644112448, attempt 2 passed for original PR head
74b6a8bb. It built clean test-merge commit17fce3fa321ed157c4ef0134d9261549f403cc30, not the current head.windows-11-arm.Both signing/cleanup paths passed, with logs reporting
Development MSIX certificate and local trust removed.All four downloads were inspected; package hashes, metadata, manifest identity/version/architecture, and public-only certificate exports matched. Both exported certificates hadHasPrivateKey: false. The artifact IDs below were rechecked while updating this description and remain unexpired.2026.7.2.0c1e455d6f507c4bca4c652be4e8dc190d1f49cd8b7abdb95c9d9428343ddf1c02026.7.2.06d5fba319a97f67dde6477eb22f9be759c1e385b5e02a950730d488fe3c6d75a2026.7.2.3209d933b8abd6d3fb5086eeb2a99d08a4064b15407679712b88a239242e75f202de2026.7.2.3209eab791386df222a19d0cb68686609c59d3d85f859709e2100936b01d780b4573The developer installed the hosted x64 Dev package and launched the welcome screen on the existing Windows host. Read-only inspection confirmed package
OpenClawFoundation.OpenClaw.Dev, x64 version2026.7.2.3209, and a running executable inside that installed package. The downloaded signature was valid, with signer90EA74F2039332D8A72CEBAC40B8B58226BD0B44.The developer's existing screenshot is preserved here:
The attachment was verified by a successful image download while updating this description.
The installation capture used
Add-AppxPackage -AllowUnsignedafter certificate import. Although the package signature independently validated, installation without that flag was not demonstrated. This is existing-host launch proof, not a clean-machine install, completed onboarding, controlled upgrade, or native ARM64 launch.Earlier local package generation at
74b6a8bbalso passed for Store x64, Store ARM64 cross-build, and Dev x64. Those packages and their earlier test results do not establish current-head acceptance.Not verified / blocked
-AllowUnsigned: not demonstrated.This PR remains a draft.