[mokagio]

Why

Azure Artifact Signing bills per signature, and the October bill jumped (AINFRA-3645).
Each Windows build here makes about 94 signatures, and from 1 Aug to 11 Nov 73% of this repo's 66k signatures came from feature branches.

Given those builds are for reviewers to try, not to ship, I think it's okay to compromise and disable code signing, at least temporarily while we get to the bottom of the billing overage.

What changes

• Windows builds on PR branches are unsigned. A Sign the Windows build? block at the end of the build runs a signed Windows build on demand, in the Windows Build (signed) job.
• Trunk and release/* keep signing, because RELEASING.md ships the artifacts from the release/X.Y.Z pull request's build. build_windows.ps1 fails either if it would build unsigned.
• The docs that promised signed artifacts on every PR now say how to get one.

macOS and Linux don't change. Apple doesn't bill per signature.

How to test this

Platforms: Windows, because the change is to Windows signing in CI. There's no app change, so there's nothing to click through.

1. On this PR's Buildkite build: Windows Build (unsigned) logs SIGN_WINDOWS_BUILD is not set, building unsigned, has no Setup code signing section, and uploads the .exe.
2. Unblock Sign the Windows build?: Windows Build (signed) sets up signing, and its signature verification passes.
3. node --test tests/unit/windows-build-signing.test.cjs (needs pwsh; it skips without). It fails against the previous build_windows.ps1: unsigned trunk and release/* builds went green, and a signed build exited 0 straight after the signing setup.

What must not have happened: a trunk or release/* build producing an unsigned installer. The tests cover it, and the first trunk build after merging shows it for real.

Risks and limitations

• PR reviewers who install the Windows build see a SmartScreen warning unless they unblock the signed build.
• The block waits for every step before it, so you can't request a signed build until macOS and Linux finish, or at all if one of them failed.
• A release bump branch named anything other than release/X.Y.Z would build unsigned and the release checklist would catch it. RELEASING.md already requires that name.

Related

Part of AINFRA-3664. Studio made the same change in Automattic/studio#5879.

---

Design decisions and alternatives considered

The issue suggested gating every build behind a block. Trunk and release/* sign automatically instead, because the release ships the bump PR's build, and a forgotten unblock there would ship unsigned or cost a rebuild. Trunk was 27% of signatures, so it isn't worth the extra release step.

Signing only the installer after the build would leave the inner binaries unsigned, so the on-demand path rebuilds instead.

87 of the 94 signatures are dugite's bundled Git binaries. Skipping already-signed third-party files would cut the cost of signed builds too. That's a separate follow-up.

Review outcome (required — see AGENTS.md)

3 [fix here] · 2 [follow-up]: all 3 fixed, and 1 follow-up is addressed in the docs.

• Fixed: nothing in CI caught a shipping branch building unsigned. build_windows.ps1 now fails it, and tests/unit/windows-build-signing.test.cjs covers it.

• Fixed: if ($LASTEXITCODE -ne 0) let exit $null pass. It now uses if ($LASTEXITCODE).

• Fixed (no change needed): the worry was that a build left at the block would hold a PR as pending on GitHub. This pipeline doesn't publish commit statuses, and publish_blocked_as_pending is off.

• Follow-up: the block waits for macOS and Linux. Listed under Risks.

• Addressed in docs: the signed and unsigned installers have the same file name. The docs point to the Windows Build (signed) job.

• Review: completed. A separate Claude agent (Opus 5.5) followed .github/instructions/code-review.instructions.md on 15c51e9 against base bb15f32. npm run lint passes. In npm test, the 7 new tests and the signing tests pass. The other 309 failures are local only: dugite's bundled Git wasn't downloaded because install scripts are disabled on this machine.

• Since review: 15c51e9 → d704556 adds the fixes above, the test, and Windows-compatible stubs after unit (windows-latest) failed on the first push. Not re-reviewed; unit passes on macOS and Windows.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: WordPress/contributor-toolkit
• Branch: update/gate-windows-signing-on-prs
• File: .coderabbit.yaml
• Commit: ebbf1a4
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 87a93a0d-b192-4fe8-b1c9-a705daf75c24

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

Pre-merge checks: 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Description check		This assessment has not completed. The check will be retried on the next pre-merge check run.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[ryanwelcher]

@mokagio does this take into account beta releases? I assumed we'd need to sign those for testing?

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[mokagio]

@ryanwelcher hey Ryan, thank you for checking on beta releases! If my understanding of the release flow is correct, then yes this takes them into account, too. That's because:

Betas start like stable release, from a release/* branch

contributor-toolkit/RELEASING.md

Lines 5 to 11 in bb15f32

	Versions are `X.Y.Z` for a stable release and `X.Y.Z-beta.N` for a beta. Tags add a `v`: `v1.2.0`, `v1.2.0-beta.2`. Any other suffix, such as `-rc.1`, is not covered here: stop and decide how to release it before using these steps. In the commands below, `X.Y.Z` stands for the full version, `-beta.N` included.
	## 1. Open the version-bump pull request
	`electron-builder` names the artifacts after the version in `package.json`, so the version moves before anything is built.
	1. Branch `release/X.Y.Z` from an up-to-date trunk. Check first that trunk matches `origin`, that nothing tracked has changed (untracked files are fine), and that the tag does not exist yet. The two `rev-parse` lines print the same commit, and the last two commands print nothing:

The filter is on release/* indiscriminately:

https://github.com/WordPress/contributor-toolkit/pull/756/changes#diff-a3991ebf1475eb82acab13946ffc1eca02b43917f6d5bec3898f45c8b0b9bd53R9-R10