ci(executable): verify the packaged binary per PR and make the release verify actually fail#2307
Merged
Merged
Conversation
…ctually fail Two related fixes to the executable pipeline: 1. The release "Verify executable" step ran a bare `./percy --version`. The executables are built on Node 14, where an unhandled promise rejection is a non-fatal warning (exit 0), so a binary that crashes on startup still exits 0 — the check passed and the broken binary was uploaded anyway. Replace it with scripts/verify-executable.sh, which fails unless `--version` exits 0, prints a real semver, and emits no runtime-error markers (used by both the macOS and Windows verify steps). 2. The build only ran on `release: published`, so a binary-breaking change was only caught after merge, at release time. Add a `pull_request` workflow (executable-check.yml) that builds the executables and runs the same verify, with no signing and no upload, so such PRs fail CI before merge. scripts/executable.sh now skips the macOS signing/notarization/upload block when APPLE_DEV_CERT is unset (PR builds, incl. forks), leaving ./percy as the macOS binary so the verify step runs natively. The signed release path is unchanged. Note: making this block merges requires marking "Build & verify executable" as a required status check in branch protection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
`pkg ... -d` enables pkg's debug mode, which logs every bundled file
("included as DISCLOSED code (with sources)" / "asset content") — thousands of
lines that drown the verify output. The flag only affects logging, not the
produced binaries, so remove it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
pranavz28
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Separate PR for the two CI gaps surfaced by the
_require is not a functionbinary crash (the code fix itself is #2306).Issue 1 — a broken binary still got uploaded
The release Build Executables workflow runs
./percy --versionas its "Verify executable" gate, then uploads. But the executables are built on Node 14, where an unhandled promise rejection is only a warning and the process still exits 0. So when the binary crashed on startup,--versionprinted the stack trace yet returned exit code 0 — the gate passed and the broken binary was uploaded anyway.Fix:
scripts/verify-executable.sh— a real smoke test that fails unless--version:UnhandledPromiseRejection,is not a function,TypeError,Cannot find module,Error:, …).It ignores benign Node deprecation warnings. Both the macOS and Windows verify steps in
executable.ymlnow use it.Issue 2 — nothing built the binary per PR
executable.ymlonly triggers onrelease: published, so a binary-breaking change was only caught after merge, at release time.Fix: new
executable-check.ymlruns onpull_request→ builds the executables and runs the same verify, without signing or uploading. A PR that breaks the binary now fails CI.To make
executable.shusable without secrets, its macOS signing/notarization/upload block is now guarded byAPPLE_DEV_CERT: when unset (PR builds, including forks) it's skipped and./percyis left as the macOS binary so the verify step runs natively on the runner. The signed release path is unchanged (secret is present → identical behavior).A workflow alone doesn't block merges. To get the "any PR causing such issues will not even get merged" behavior, mark
Build & verify executableas a required status check in branch protection formaster.Verification
scripts/verify-executable.shself-tested locally: fails the exact Node-14UnhandledPromiseRejectionWarning: ... _require is not a functionoutput (exit 1), passes a healthy1.32.2(exit 0), and does not false-positive on aDeprecationWarning.bash -nclean on both scripts; both workflow YAMLs parse.🤖 Generated with Claude Code