Skip to content

ci(release): add exact-head candidate artifacts#4532

Merged
Hmbown merged 6 commits into
mainfrom
agent/v091-release-candidate-ci-20260718
Jul 18, 2026
Merged

ci(release): add exact-head candidate artifacts#4532
Hmbown merged 6 commits into
mainfrom
agent/v091-release-candidate-ci-20260718

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 18, 2026

Copy link
Copy Markdown
Owner

What changed

  • add an exact-SHA workflow_dispatch path that forces the complete CI profile
  • add a non-publishing release-candidate workflow over the authoritative seven-target matrix
  • share build, bundle, installer, inventory, checksum, and npm dogfood logic with the public release workflow
  • preserve executable bits after Actions artifact transport and make archive construction deterministic for identical inputs
  • refuse public release reruns that would replace any existing asset bytes

Safety boundary

The candidate workflow has read-only repository permissions and uploads short-lived Actions artifacts only. It cannot create a tag or GitHub Release, publish crates or npm, push containers or Homebrew changes, or deploy the website. Public v0.9.1 tagging and publication remain explicitly approval-gated.

Validation

  • 53 Node workflow/assembly/immutability/npm tests
  • actionlint
  • shellcheck
  • cargo fmt --all -- --check
  • git diff --check
  • ./scripts/release/check-versions.sh
  • ./scripts/release/check-ohos-deps.sh
  • exact-range and worktree gitleaks

The workflow has not been dispatched. Its first exact-head run belongs after this implementation and every other v0.9.1 source change reach final main.

Fixes #4531

Hmbown added 3 commits July 18, 2026 08:16
Factor the seven-target release build and 34-file asset assembly into a read-only reusable workflow shared by public releases and a manual non-publishing candidate path. Manual CI now validates the requested SHA and runs the full Linux, macOS, Windows, npm, mobile, workflow, Actions, and docs gates instead of light placeholders.

Add checksum/inventory contracts, archive fixtures, packaged-wrapper smoke against assembled assets, and an explicit publication stop line in the release runbook.

Signed-off-by: Hunter B <hmbown@gmail.com>
Restore the Unix executable contract after GitHub artifact transport normalizes files to 0644. Normalize archive metadata as well so identical inputs do not create packaging-only checksum drift, while retaining the separate public-asset immutability guard.

Signed-off-by: Hunter B <hmbown@gmail.com>
Fail before build work and again immediately before upload whenever a tag already owns GitHub Release assets. Disable the release action overwrite path and document that recovery requires a deliberate maintainer decision instead of silently replacing public bytes.

Signed-off-by: Hunter B <hmbown@gmail.com>
Copilot AI review requested due to automatic review settings July 18, 2026 15:28
@cursor

cursor Bot commented Jul 18, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@Hmbown Hmbown added release-blocker Must be fixed before the next release reliability Reliability, flaky behavior, retries, fallbacks, and robustness documentation Improvements or additions to documentation v0.9.1 Targeting v0.9.1 labels Jul 18, 2026
@Hmbown Hmbown added this to the v0.9.1 milestone Jul 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 3m 16s —— View job


Review complete

  • Gather context (diff, workflow files, scripts, tests)
  • Review correctness/logic in new scripts and workflows
  • Check provider/model routing safety (N/A — this PR only touches release/CI infra, no model/provider routing code)
  • Check reuse/simplification and dead code
  • Check test coverage for new behavior
  • Check security (secrets, shell/exec, input validation)
  • Post final review summary

Summary

This is a careful, well-scoped CI/release change. The safety boundary (read-only permissions: contents: read on both release-candidate.yml and release-artifacts.yml, no tag/publish/registry steps) is enforced by the workflow YAML itself and cross-checked by .github/scripts/release-workflows.test.js, which greps for forbidden patterns (contents: write, softprops/action-gh-release, docker/login-action, npm publish, etc.) in both workflows. That's a good belt-and-suspenders design.

Correctness

  • ci.yml / release-candidate.yml exact-SHA guards (git rev-parse HEAD vs normalized expected_sha) are sound: both jobs checkout with fetch-depth: 0 before comparing, so HEAD resolves correctly, and the [^0-9a-fA-F] + length-40 validation rejects short/invalid SHAs before use.
  • scripts/release/assemble-release-assets.js: assemble() uses fs.lstat + isFile() before copying intermediate artifacts, which correctly rejects symlinked/non-regular downloaded artifacts rather than silently following them.
  • create-release-bundles.sh: mtime/uid/gid normalization (--mtime, --owner=0, --group=0, touch -t) plus explicit chmod 0755 restoration after the known actions/upload-artifact 0644-normalization behavior — confirmed by the round-trip reproducibility test and the tar-mode-after-extract assertion in assemble-release-assets.test.js.
  • ensure-release-assets-absent.js: fails closed on non-404 errors and on a malformed release payload (no assets array), and is invoked twice in release.yml (before work starts, and immediately before action-gh-release) plus overwrite_files: false on the upload step itself — reasonable defense in depth against the TOCTOU window between the two checks.

Minor/nit — dead code
scripts/release/assemble-release-assets.js:111-119, the CodeWhaleSetup.exe special case in intermediateArtifactPath is functionally identical to the generic fallback (path.join(inputDirectory, name, name) already produces .../CodeWhaleSetup.exe/CodeWhaleSetup.exe). It's harmless but redundant and can be dropped:

function intermediateArtifactPath(inputDirectory, name) {
  if (name === BUNDLE_CHECKSUM_MANIFEST || BUNDLE_ASSET_NAMES.includes(name)) {
    return path.join(inputDirectory, "codewhale-bundles", name);
  }
  return path.join(inputDirectory, name, name);
}

Fix this →

Test coverage — good. assemble-release-assets.test.js and ensure-release-assets-absent.test.js cover the assembly/verify round-trip, tampering detection (checksum mismatch, unexpected file), bundle reproducibility, executable-mode restoration, and the immutability guard's 404/403/malformed-JSON edge cases. release-workflows.test.js statically pins the 7-target/34-asset inventory and the permission/publish-surface boundary so a future edit can't silently reintroduce a write scope.

Security — no command injection found: execFileSync calls use argument arrays (no shell string interpolation of untrusted data), repo/tag inputs are validated against strict regexes in validateTarget, and the local HTTP server in npm-wrapper-smoke.js still path-resolves and rejects traversal outside its root.

Provider/model routing (EPIC #2608 invariant) — not applicable; nothing in this diff touches model/provider selection.

No blocking issues found.

Checkout the resolved candidate SHA in a dedicated read-only web job and require public facts, docs, tests, lint, type checking, and the production build before constructing any release artifacts.\n\nKeep the candidate workflow non-publishing and bind artifact creation to the web gate result.

Signed-off-by: Hunter B <hmbown@gmail.com>
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 4m 4s —— View job


Review complete

  • Gather context (diff since last review, workflow files, scripts, tests)
  • Review correctness/logic in new/changed code
  • Check provider/model routing safety (N/A — CI/release infra only)
  • Check reuse/simplification and dead code
  • Check test coverage for new behavior
  • Check security (secrets, shell/exec, input validation)
  • Post final review summary

Summary

This push (commit ed43029) adds a new web gate job to release-candidate.yml that checks out the resolved candidate SHA and requires public-facts drift check, docs parity, tests, lint, type check, and a production build to pass in web/ before the artifacts job runs. The earlier review of this PR (posted 15:28 UTC) already covered the rest of the diff and found no blocking issues; this pass focuses on what changed since then.

Correctness — sound. artifacts now correctly depends on [resolve, web] and gates on needs.web.result == 'success', matching the existing !cancelled() guard style used elsewhere in this workflow. All referenced web/package.json scripts (check:facts, prebuild, check:docs, lint, test) and web/package-lock.json exist. Good catch preserving the step order (check:facts before prebuild) — this matches the anti-regression invariant documented in .github/workflows/web.yml:36-42 (issue #3771: running prebuild first would self-heal a stale committed facts.generated.ts and mask drift). Getting that ordering right here, without carrying the explanatory comment, is a small thing but easy to get backwards.

Minor bug — broken regex escape helper
.github/scripts/release-workflows.test.js:58:

assert.match(candidate, new RegExp(`run: ${command.replace(/[.*+?^${}()|[\\]\\]/g, "\\$&")}`));

The character class is malformed: [.*+?^${}()|[\\] closes one character early (the \\ escape is consumed inside the class, then the very next ] terminates it), leaving the trailing \\] as a stray literal-\ + literal-] requirement outside the class. The canonical/correct pattern escapes ] first, then \, then closes: /[.*+?^${}()|[\]\\]/g. As written, .replace() here effectively never matches anything, so it silently escapes nothing. This is harmless today only because none of the 8 asserted commands (npm ci, npx tsc --noEmit, etc.) contain regex metacharacters — but the helper is broken and will produce an incorrect (likely non-matching) regex the moment a future command in that list contains ., (, ), or similar. Fix this →

Reuse/simplificationrelease-candidate.yml's new web job duplicates six steps verbatim from web.yml's existing lint job (checkout → setup-node@22 → npm cicheck:factsprebuildcheck:docslinttsc --noEmit), then adds npm test and npm run build on top. This PR already reuses workflow logic elsewhere via uses: ./.github/workflows/release-artifacts.yml for the artifacts job — worth considering the same pattern (or a composite action) for the shared web-verification steps so web.yml and release-candidate.yml can't silently drift apart if one is edited without the other. Not blocking, since the two jobs aren't currently identical (test/build differ) and correctness is fine as-is.

Tests — the new assertions in release-workflows.test.js appropriately pin the new job's shape (job name, ref: needs.resolve.outputs.sha, working-directory: web, the needs: [resolve, web] graph, and the success gate), aside from the regex nit above.

Security — no change to the read-only permissions boundary; the new job only reads and builds, no secrets or write scopes introduced.

No blocking issues.

Resolve each remote Action used by candidate artifact construction or public publication to the current audited commit, retaining the release version as an update hint.\n\nAdd a contract test that rejects mutable remote refs in all three release workflows and clean up the affected shell blocks so configured actionlint remains green.

Signed-off-by: Hunter B <hmbown@gmail.com>
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 2m 40s —— View job


Review complete — commit bdcc746 (pin every publication-path action)

  • Gather context (diff since last review)
  • Review action-pinning changes in the three release workflows
  • Check new contract test for mutable-ref rejection
  • Check shellcheck/actionlint cleanup claims
  • Post final review summary

Summary

This commit pins every remote uses: in release-candidate.yml, release-artifacts.yml, and release.yml to a full 40-character commit SHA, keeps the version/date as a trailing # vX comment (so Dependabot's SHA-pinning convention still applies for future bumps), and adds a contract test enforcing it.

Correctness

  • The same action@ref pair uses the identical SHA everywhere it recurs (checked across all three files) — e.g. actions/checkout@9c091bb2… appears 10+ times and actions/upload-artifact@043fb46d… appears 7 times, always matching. No copy-paste drift between occurrences.
  • The new contract test's capture regex /^\s+(?:-\s+)?uses:\s+([^@\s]+)@([^#\s]+)/gm correctly stops the ref group at the space before the trailing # vX comment, so it validates the SHA itself, not the comment. Confirmed against the actual file content.
  • Scope is intentionally limited to the three publication-path workflows; ci.yml still uses mutable tags (@v7, @stable, @master) and is correctly excluded from both the pinning and the new test — consistent with the commit message's stated scope ("candidate artifact construction or public publication"), not an oversight.
  • The echo ... >> "${GITHUB_ENV}" / >> "${GITHUB_OUTPUT}" blocks were correctly converted to grouped { ...; } >> file form everywhere they previously repeated the redirect (SC2129) — verified both occurrences in release-artifacts.yml and the one in release.yml were fixed, not just some of them.

Gap I couldn't verify — please double check

  • I don't have network access in this sandbox, so I could not confirm that each pinned SHA actually resolves to the tag named in its trailing comment (e.g. that actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 really is v7 upstream, rather than a typo'd or stale commit). The new test only validates that the ref looks like a 40-hex-char SHA — it can't catch a SHA that's well-formed but wrong. Since the entire point of this commit is supply-chain integrity, it's worth a gh api repos/<owner>/<repo>/commits/<tag> spot-check (or equivalent) against upstream before merge, if that hasn't already been done as part of authoring this.

Prior nits (non-blocking, still open)
The two minor items from earlier reviews of this PR are untouched by this commit and remain open (neither blocks merge): the malformed regex character class in release-workflows.test.js:58 (command.replace(/[.*+?^${}()|[\\]\\]/g, ...), harmless today only because no asserted command string contains regex metacharacters), and the redundant CodeWhaleSetup.exe special case in scripts/release/assemble-release-assets.js:112-113.

Note on tooling: I couldn't execute actionlint, shellcheck, or node --test in this sandbox (commands required approval not available in this run), so the lint/test claims in the PR description are taken on the author's word and cross-checked only via static reading of the diff — the static read didn't turn up anything contradicting them.

No blocking issues found in this push.

Repair the command escaping helper so future metacharacters cannot weaken the workflow contract, and remove the redundant installer path branch.

Signed-off-by: Hunter B <hmbown@gmail.com>
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 43s —— View job


Review complete — commit 9729924 (close review nits)

  • Gather context (diff since last review)
  • Verify the regex character-class fix
  • Verify the CodeWhaleSetup.exe branch removal is behavior-preserving
  • Post final review summary

Summary

This commit closes both non-blocking nits from the earlier reviews on this PR. Both fixes are correct.

1. Regex escape helper — fixed correctly
.github/scripts/release-workflows.test.js:58 now reads:

new RegExp(`run: ${command.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`)

Tokenizing the character class: . * + ? ^ $ { } ( ) | [ as literals, then \] (escaped ]), then \\ (escaped \), then the class closes. That's the canonical ordering (escape ] first, then \, then close) — the previous version closed the class one character early and silently escaped nothing. This now correctly escapes regex metacharacters in any future command string added to that list.

2. Redundant CodeWhaleSetup.exe branch — removed correctly
scripts/release/assemble-release-assets.js's intermediateArtifactPath no longer special-cases CodeWhaleSetup.exe. Confirmed the fallback path.join(inputDirectory, name, name) produces the identical path (.../CodeWhaleSetup.exe/CodeWhaleSetup.exe) that the removed branch produced, so this is a pure dead-code deletion with no behavior change.

Tooling note: as in the prior review pass, I could not execute node --test, actionlint, or shellcheck in this sandbox (commands require approval not available in this run) — verification here is by static reading only. The two changed files are small and isolated enough that I'm confident in the read, but a CI run remains the authoritative check.

No blocking issues. This closes out the previously open nits — I have no further findings on top of the three earlier review passes on this PR.

@Hmbown

Hmbown commented Jul 18, 2026

Copy link
Copy Markdown
Owner Author

Supply-chain receipt for 9729924: I re-resolved all 14 unique action/ref pairs live against their upstream Git repositories after the final push. Every pinned 40-character SHA matched the documented tag or branch, including peeled annotated tags for sccache v0.0.10, rust-cache v2, and action-gh-release v3. The release workflow contract, asset assembly/reproducibility suite, and release immutability suite also passed locally (11/11). No publication action was taken.

@Hmbown
Hmbown merged commit 616bcb9 into main Jul 18, 2026
19 checks passed
@Hmbown
Hmbown deleted the agent/v091-release-candidate-ci-20260718 branch July 18, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation release-blocker Must be fixed before the next release reliability Reliability, flaky behavior, retries, fallbacks, and robustness v0.9.1 Targeting v0.9.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

release: add non-publishing exact-head candidate verification

2 participants