fix: fail loudly when test MSIX signing has no inputs

[paulcam206]

Summary

• validate that -ArtifactsDirectory contains at least one supported architecture directory before doing any work
• fail with a message naming the inspected root and the expected layout, instead of exiting 0 after signing nothing
• move that validation ahead of SDK discovery and certificate creation, so a run that cannot sign never mints key material
• add scripts/Test-Sign-TestMSIX.Tests.ps1 covering the no-input path

This addresses a silent-success failure mode only. It intentionally does not change the signing, metadata, or certificate-cleanup behavior, and it does not broaden the accepted input layout.

Why

Sign-TestMSIX.ps1 iterates x64 and arm64, skipping each architecture whose directory is absent. When the artifact root is not CI-shaped, both iterations are skipped, the loop body never runs, and control falls through to an unconditional success message and a zero exit code.

Found by a real local signing run. Pointing the script at a directory that directly contained OpenClawGateway-x64.msix printed:

Created test-signed MSIX artifacts under: artifacts\local-msix\x64\0.1.2445.55030\signed

exited 0, and produced no files whatsoever. Passing the wrong shape was operator error; reporting success after signing nothing is the defect. In a release flow this can present a missing or unsigned artifact as a green step, and the only symptom is an empty output directory that nothing forces you to look at.

The validation is deliberately placed before New-SelfSignedCertificate. The previous ordering created a signing certificate, skipped every architecture, and discarded the certificate in finally — doing key-management work for a job it had already established it could not perform.

Scope decision

The CI-shaped input contract is retained rather than extended to also accept a directory containing an MSIX directly. Output directories, the exported .cer, and msix-metadata.json are all written per architecture, so accepting a flat layout would require inventing an architecture for artifacts that do not declare one. A clear error is better than a guess.

Validation

The regression test is verified to fail without the fix. Running the new test against the unmodified script from main:

Exception: Test-Sign-TestMSIX.Tests.ps1:36
  36 |          throw 'Signing unexpectedly succeeded without an architecture …
     | Signing unexpectedly succeeded without an architecture directory.
exit: 1

With the fix applied:

pwsh -NoProfile -File .\scripts\Test-Sign-TestMSIX.Tests.ps1
Test MSIX no-op signing regression test passed.

pwsh -NoProfile -File .\scripts\Test-SigningInputs.Tests.ps1
Gateway MSIX signing policy tests passed.

The test drives the real script in a child process against an isolated temporary artifact root and asserts the observable outcome: a non-zero exit, an error naming the missing architecture directories, no output directory created, and no new certificate for the policy publisher left in Cert:\CurrentUser\My. It removes its temporary directory afterwards and matches publisher subjects exactly, so unrelated development certificates are neither inspected nor removed.

[clawsweeper]

🦞👀
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]

Codex review: needs maintainer review before merge. Reviewed September 11, 2026, 6:40 PM ET / 22:40 UTC.

ClawSweeper review

What this changes

The PR makes Windows test signing reject artifact roots without x64 or arm64 directories before creating signing material, and adds a regression test.

Merge readiness

✅ Ready for maintainer review

The fix remains necessary on current main and has no blocking correctness findings. The supplied Windows transcript exercises the reported failure through the real signing script.

Priority: P2
Reviewed head: fc9812b4af3b464d0c506393a0c0c14339cedc43

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with production-path regression evidence and no blocking findings.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The supplied Windows before/after transcript runs Sign-TestMSIX through a real child process with an empty artifact root and reports the expected failure diagnostic, nonzero exit, absent output, and no retained new certificate after the fix.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The supplied Windows before/after transcript runs Sign-TestMSIX through a real child process with an empty artifact root and reports the expected failure diagnostic, nonzero exit, absent output, and no retained new certificate after the fix.
Evidence reviewed	6 items	Introduced guard: The new directory check precedes SDK discovery, output creation, and certificate generation. Existing per-architecture package and metadata validation remains intact. Current main still contains the defect: Main skips absent architecture directories and subsequently prints unconditional success. The live main branch still resolves to the supplied base SHA; the latest-release endpoint returned no release. Existing workflow contract: The test-signing workflow downloads packages into artifacts\x64 and artifacts\arm64 before invoking this script. Valid workflow inputs retain their existing behavior; the changed outcome concerns an invocation that produced no signed packages.
Findings	None	None.
Security	None	None.

How this fits together

The Windows packaging pipeline passes architecture-specific MSIX packages to a test-signing script. That script produces signed packages, public certificates, and updated metadata for local installation.

flowchart TD
    A[Unsigned artifact root] --> B{Supported architecture folder exists?}
    B -->|No| C[Actionable error and nonzero exit]
    B -->|Yes| D[Discover signing tool]
    D --> E[Create temporary certificate]
    E --> F[Validate and sign packages]
    F --> G[Signed packages and metadata]
    F --> H[Clean up signing material]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +19/-1, tests +63/-0	The production growth implements the early guard, with a focused regression harness exercising observable outcomes.

Technical review

Best possible solution:

Preserve the architecture-specific input contract and reject empty signing jobs before creating output or key material.

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

Yes: on Windows with the signing prerequisites available, passing an existing root without x64 or arm64 directories reaches main's unconditional success. Source inspection and the contributor's before/after transcript support this; the reviewer did not execute it.

Is this the best way to solve the issue?

Yes: checking the accepted layout before side effects directly fixes the silent no-op without introducing another input format or changing successful signing.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 52f2a53fb62b.

Labels

Label changes:

• add P2: This repairs misleading success in developer test-signing tooling with a limited, clearly defined scope.
• add proof: sufficient: Contributor real behavior proof is sufficient. The supplied Windows before/after transcript runs Sign-TestMSIX through a real child process with an empty artifact root and reports the expected failure diagnostic, nonzero exit, absent output, and no retained new certificate after the fix.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The supplied Windows before/after transcript runs Sign-TestMSIX through a real child process with an empty artifact root and reports the expected failure diagnostic, nonzero exit, absent output, and no retained new certificate after the fix.

Label justifications:

• P2: This repairs misleading success in developer test-signing tooling with a limited, clearly defined scope.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The supplied Windows before/after transcript runs Sign-TestMSIX through a real child process with an empty artifact root and reports the expected failure diagnostic, nonzero exit, absent output, and no retained new certificate after the fix.
• proof: sufficient: Contributor real behavior proof is sufficient. The supplied Windows before/after transcript runs Sign-TestMSIX through a real child process with an empty artifact root and reports the expected failure diagnostic, nonzero exit, absent output, and no retained new certificate after the fix.

Evidence

What I checked:

• Introduced guard: The new directory check precedes SDK discovery, output creation, and certificate generation. Existing per-architecture package and metadata validation remains intact. (scripts/Sign-TestMSIX.ps1:32, fc9812b4af3b)
• Current main still contains the defect: Main skips absent architecture directories and subsequently prints unconditional success. The live main branch still resolves to the supplied base SHA; the latest-release endpoint returned no release. (scripts/Sign-TestMSIX.ps1:78, 52f2a53fb62b)
• Existing workflow contract: The test-signing workflow downloads packages into artifacts\x64 and artifacts\arm64 before invoking this script. Valid workflow inputs retain their existing behavior; the changed outcome concerns an invocation that produced no signed packages. (.github/workflows/gateway-msix.yml:280, 52f2a53fb62b)
• Real script regression evidence: The complete supplied PR body records failure against the unmodified script and success after the fix. The inspected harness launches the production script in a child PowerShell process and checks exit status, diagnostic text, absent output, and no newly retained publisher certificate. The captured context sourceRevision is 8717be0dac2f5920aa577963aceabf79f7b8bd308b7c3469fd7ed2e486529c83. Reviewer execution was not performed under the read-only review constraint. (scripts/Test-Sign-TestMSIX.Tests.ps1:27, fc9812b4af3b)
• Signing history and routing: File history identifies the original packaging addition and subsequent restoration. Blame ties the retained architecture checks to the restoration commit; its raw parent record was inspected, and the parent tree has no signing script. A broader historical content search could not complete because a required older object was unavailable. (scripts/Sign-TestMSIX.ps1:98, a32dffb95159)
• Related signing work is distinct: The open chore(ci): finalize Artifact Signing setup #24 addresses Azure OIDC configuration for official signing, rather than the local test-signing no-input path. No merged replacement was identified in the repository PR listing.

Likely related people:

• Linus Huang: Raw commit a32dffb adds scripts/Sign-TestMSIX.ps1:80 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: a32dffb95159; files: scripts/Sign-TestMSIX.ps1)

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.