fix(release): preserve merge approvals and exclude native binaries from npm - #2259
Conversation
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice hardening pass on the release pipeline. The switch from pull_request: closed to a push-triggered flow with a check-release gate that re-verifies the merged PR against the pushed SHA (same repo head/base, release/v* head ref, merged, correct target branch) is a solid tightening, and the shell-level tests exercising each rejection case (fork, unmerged, wrong branch, wrong commit, deleted fork with null head.repo, empty PR list, API failure) give me confidence the gate fails closed.
A few things I checked and think are fine:
set -euo pipefailplus command substitution around agh api | jqpipeline correctly propagatesghfailures — thecheck-releasestep fails,verify/publishare skipped.- The narrowed
filesallowlist (dist/index.js,dist/main.js,dist/assets) keepsdist/bin/*and source maps out of the npm tarball while still shipping README/LICENSE (which npm always includes). Thepackage.test.tsfixture directly validates this againstbun pm pack, which is a much better guarantee than eyeballing globs. github.event.deletedguard oncheck-releaseprevents branch-delete events from running the flow.- Using
github.shathroughoutverifyandpublishmeans later pushes can't retarget the release, which was the point.
No blocking issues found; ship it.
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2259 +/- ##
=========================================
Coverage 97.04% 97.04%
=========================================
Files 566 566
Lines 39353 39353
=========================================
Hits 38190 38190
Misses 1163 1163 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
| publish: | ||
| needs: verify | ||
| # npm provenance is only issued from GitHub-hosted runners. | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
so one we get allowlisted we can swap this to codebuild as well?
Description
Preserve the existing release flow: dispatch prepare, review and merge the release PR, then verify and publish the exact merged commit.
release-prepare.ymlto the approvedaws-release-4-corerunner.release-publish.ymlon pushes torefactor, and proceed only when the pushed SHA matches a mergedrelease/v*PR opened byagentcore-devx-automation[bot](account ID282717993) from the same repository into that branch. Skip manually authored release PRs, unrelated pushes, ordinary PRs, and fork PRs; fail closed if the GitHub API request fails. Pin the author's account ID so a login rename does not change the identity check.ubuntu-latest. Moving this workflow to the dedicated pool remains a separate change after allowlisting. Verification runners are unchanged."files": ["dist", "!dist/bin"]. Keep the generated bundle, runtime assets, and any additional build chunks, while excluding standalone native binaries. Do not add main's legacy scripts, shrinkwrap, or MCP-harness entries.The packaging regression reproduced with one compiled Linux binary: 40.37 MB before, 1.44 MB after. The current build's package contains 89 files, preserving all 87 non-binary
distfiles. A fresh workflow already packed before compilation; the fix also excludes native binaries during manual packing and in reused build directories. Otherdistoutputs remain included.No companion
mainchange is needed: itsrelease-prepare.ymlis only the dispatch placeholder, and its existing release workflows already use the dedicated runner.Related Issue
Closes #2258
Follow-up to #2206 and #2052.
Documentation PR
Not applicable. Release documentation is updated in
.github/workflows/README.md.Type of Change
Testing
The
refactorbranch uses Bun rather than the default branch's separate npm unit/integration scripts.bun test(existing repository test suite).bun run typecheckbun run lint:checkbun run format:checkbun run buildbun run compile:linux-x64, plus Node-bundle and native-binary--helpsmoke tests.npm pack --dry-run --ignore-scripts --json, with a native binary present.dist/binfiles are excluded, while additional JS chunks and worker files remain included.The release-gate and packaging checks were exercised locally; no new test scripts are included in this PR.
No release workflow was dispatched, no npm package was published, and no release artifacts were uploaded for validation.
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.