[release-pipeline] Ship the failproofaid binaries and make releases ref-aware - #634
Conversation
The daemon split (#632) added every packaging input — the platform manifests, the pinned optional dependencies, and a 4-way cross-compile matrix — but never touched publish.yml, so every release built four binaries as Actions artifacts and threw them away with the runner. CI was green throughout, because nothing checks that what gets built also gets shipped. Rework publish.yml into four jobs: preflight (version/dist-tag resolution, npm credential check, daemon detection), daemon (calls build-daemon.yml, now a reusable workflow), release-assets (downloads the artifacts, assembles SHA256SUMS, attaches both to the release), and publish (npm + aliases). The assets land BEFORE the npm publish because the installed CLI downloads its daemon from that release tag — publish the package first and its binary does not exist yet. Two guards make a branch dispatch safe. The version bump checks main out and pushes to it, so it now runs only for a release or a dispatch from main; a dispatch from a feature branch would otherwise rewrite main's version line to whatever that branch carries. And `latest` is refused from a non-main dispatch, with `auto` resolving to `next` there, so a branch build cannot move a dist-tag that a later release from main would then move backwards. Everything binary-related is gated on the ref actually carrying a Rust workspace, so this is a no-op on main until #632 lands: both daemon jobs skip and the npm publish behaves exactly as before. build-daemon.yml comes along inert for the same reason — it gates its matrix on a detect job, which is also what stops its own path filter from running `cargo build` against a checkout with no crates. Also adds a dry-run input (builds, checksums and `npm publish --dry-run` while writing nothing), fixes publish.yml's bun cache key, which hashed a `bun.lockb` this repo does not track, and adds a tripwire test over both workflows, since a hand-maintained pipeline that silently drops its own build artifacts is exactly what just happened. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbXKFnpRXBwsrvUJDdySky
📝 WalkthroughWalkthroughThe release pipeline now builds four native daemon targets, assembles GitHub Release assets, controls npm dist-tags and version updates, supports dry runs, and validates workflow behavior with Vitest drift checks. ChangesRelease pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbXKFnpRXBwsrvUJDdySky
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/publish.yml (1)
34-39: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNeither release workflow declares a least-privilege token default. Both workflows omit a
permissions:block, so their jobs receive the repository default token scopes, and thedaemonjob propagates those scopes into the reusable workflow.
.github/workflows/publish.yml#L34-L39: add a workflow-levelpermissions: contents: readabovejobs:; therelease-assetsandpublishjobs already declare the write scopes they need..github/workflows/build-daemon.yml#L14-L25: add a workflow-levelpermissions: contents: readabovejobs:, since this workflow only reads the repository, uses caches, and uploads artifacts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yml around lines 34 - 39, Add workflow-level least-privilege permissions by setting contents: read above jobs in .github/workflows/publish.yml (lines 34-39) and .github/workflows/build-daemon.yml (lines 14-25); preserve the existing job-specific write permissions in publish.yml.Source: Linters/SAST tools
.github/workflows/build-daemon.yml (1)
69-79: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUpdate the incorrect runner-label comment
macos-15-intelis a supported GitHub-hosted runner label. Replace the text “an unknown runner label doesn't degrade” with wording that accurately describes this macOS 13 image replacement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-daemon.yml around lines 69 - 79, Update the comment above the macOS matrix entries to remove the inaccurate statement that an unknown runner label would not degrade, and replace it with wording that accurately explains macos-15-intel as the supported replacement for the retired macOS 13 image. Leave the runner configuration unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 87-102: Replace direct GitHub expression interpolation in the
publish workflow with environment variables: pass github.ref_name and
github.event_name through the step env and reference those variables in the
shell conditions and error message. Apply the same env-based pattern to the
later publish steps, including the github.event_name use around the referenced
step and all needs.* expansions, while preserving the existing branching and
validation behavior.
- Around line 308-314: Update the “Bump version for next development cycle” step
to discard the package.json modification created by “Set publish version in
package.json” before its git checkout main operation. Ensure the working tree
reset occurs after npm version completes and before switching branches, while
preserving the existing guard conditions.
- Around line 260-264: Update the cache key definitions in the workflow files’
actions/cache steps to hash the tracked bun.lock file instead of bun.lockb,
while preserving the existing runner-specific key and restore-keys patterns.
---
Nitpick comments:
In @.github/workflows/build-daemon.yml:
- Around line 69-79: Update the comment above the macOS matrix entries to remove
the inaccurate statement that an unknown runner label would not degrade, and
replace it with wording that accurately explains macos-15-intel as the supported
replacement for the retired macOS 13 image. Leave the runner configuration
unchanged.
In @.github/workflows/publish.yml:
- Around line 34-39: Add workflow-level least-privilege permissions by setting
contents: read above jobs in .github/workflows/publish.yml (lines 34-39) and
.github/workflows/build-daemon.yml (lines 14-25); preserve the existing
job-specific write permissions in publish.yml.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 87973d41-4bba-487a-8792-872be8b644b7
📒 Files selected for processing (4)
.github/workflows/build-daemon.yml.github/workflows/publish.ymlCHANGELOG.md__tests__/ci/release-pipeline.test.ts
… packages The four `@failproofai/failproofaid-<os>-<arch>` packages were the plan of record — declared as optionalDependencies, pinned to the root version, built by a 4-way cross-compile — but nothing ever published them. The workflow only uploaded the binaries as Actions artifacts and publish.yml was never touched at all, so all four names 404 on npm today and a released CLI would have resolved a daemon that does not exist. #634 fixes the pipeline either way; this removes the channel it would have had to publish, because the release assets have to exist regardless for anyone installing failproofaid on its own, and a second channel is a second thing to keep in step with the first. daemon-download.ts fetches `failproofaid-<os>-<arch>.gz` from the release tagged with this CLI's own version, verifies it against the published SHA256SUMS *before* decompressing, and installs it to `~/.failproofai/bin/failproofaid-<version>` by atomic rename, mode 0755. The URL is constructed from package.json's version rather than discovered through the API: no rate limit, no `releases/latest` redirect, and no way to run a daemon built from different source than the CLI talking to it. The versioned filename is what keeps an upgrade from overwriting a running binary (ETXTBSY) or silently repointing a live service unit. A bad checksum, a missing manifest entry and a failed fetch are refusals rather than warnings — what this writes is an executable a service manager runs at login. Only the install path downloads; resolveFailproofaidBinaryPath() stays a pure disk check, so the hook path can never block on the network. FAILPROOFAI_NO_DOWNLOAD=1 opts an air-gapped machine out while leaving an already-installed binary working, and FAILPROOFAI_DAEMON_BASE_URL points at an internal mirror (and at a local server in the tests). build-daemon.yml matches #634's copy so the rebase is a no-op: it gzips each binary and uploads that, which is also what makes the artifact's lost executable bit a non-issue. Verified: 13 new download tests against a real local HTTP server covering checksum mismatch, a manifest with no entry, a 404, the disabled-downloads opt-out and the atomic install; the daemon-service resolution tests now run against a scratch HOME so a developer machine with a real daemon cannot flake them; and a clean `npm pack` + container run confirms the shim reports the config hint with nothing installed and execs an overridden binary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbXKFnpRXBwsrvUJDdySky
… packages The four `@failproofai/failproofaid-<os>-<arch>` packages were the plan of record — declared as optionalDependencies, pinned to the root version, built by a 4-way cross-compile — but nothing ever published them. The workflow only uploaded the binaries as Actions artifacts and publish.yml was never touched at all, so all four names 404 on npm today and a released CLI would have resolved a daemon that does not exist. #634 fixes the pipeline either way; this removes the channel it would have had to publish, because the release assets have to exist regardless for anyone installing failproofaid on its own, and a second channel is a second thing to keep in step with the first. daemon-download.ts fetches `failproofaid-<os>-<arch>.gz` from the release tagged with this CLI's own version, verifies it against the published SHA256SUMS *before* decompressing, and installs it to `~/.failproofai/bin/failproofaid-<version>` by atomic rename, mode 0755. The URL is constructed from package.json's version rather than discovered through the API: no rate limit, no `releases/latest` redirect, and no way to run a daemon built from different source than the CLI talking to it. The versioned filename is what keeps an upgrade from overwriting a running binary (ETXTBSY) or silently repointing a live service unit. A bad checksum, a missing manifest entry and a failed fetch are refusals rather than warnings — what this writes is an executable a service manager runs at login. Only the install path downloads; resolveFailproofaidBinaryPath() stays a pure disk check, so the hook path can never block on the network. FAILPROOFAI_NO_DOWNLOAD=1 opts an air-gapped machine out while leaving an already-installed binary working, and FAILPROOFAI_DAEMON_BASE_URL points at an internal mirror (and at a local server in the tests). build-daemon.yml matches #634's copy so the rebase is a no-op: it gzips each binary and uploads that, which is also what makes the artifact's lost executable bit a non-issue. Verified: 13 new download tests against a real local HTTP server covering checksum mismatch, a manifest with no entry, a 404, the disabled-downloads opt-out and the atomic install; the daemon-service resolution tests now run against a scratch HOME so a developer machine with a real daemon cannot flake them; and a clean `npm pack` + container run confirms the shim reports the config hint with nothing installed and execs an overridden binary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbXKFnpRXBwsrvUJDdySky
Why
PR #632 (the daemon split) added every packaging input — the four platform manifests, the pinned
optionalDependencies, andbuild-daemon.yml's 4-way cross-compile — butgit log main..failproofaid -- .github/workflows/publish.ymlis empty. It never touched the publish workflow. So every release would build four binaries, upload them as Actions artifacts, and throw them away with the runner. CI stayed green throughout, because nothing checks that what gets built also gets shipped.Second, unrelated problem in the same file: the version-bump step checks
mainout and pushes to it regardless of which ref was dispatched. Aworkflow_dispatchfrom a feature branch would rewrite main's version line to whatever that branch happens to carry.What changed
publish.ymlbecomes four jobs:preflightdaemonbuild-daemon.yml, now a reusable workflowrelease-assetsSHA256SUMS, attaches both to the GitHub ReleasepublishAssets are attached before the npm publish, deliberately: the installed CLI downloads its daemon from that release tag, so publishing the package first ships a version whose binary does not exist yet.
Two guards make a dispatch from a branch safe:
releaseevent or a dispatch frommain;latestis refused from a non-main dispatch, andautoresolves tonextthere — so a branch build cannot move a dist-tag that a later release from main would then move backwards.Also: a
dry_runinput (builds, checksums,npm publish --dry-run, writes nothing), and a fix for the bun cache key, which hashed abun.lockbthis repo does not track.This is a no-op on main today
Every binary-related job is gated on the ref carrying a Rust workspace.
mainhas none until #632 lands, sodaemonandrelease-assetsskip and the npm publish behaves exactly as it does now.build-daemon.ymlcomes along inert for the same reason — it gates its matrix on adetectjob, which is also what stops its own path filter from runningcargo buildagainst a checkout with no crates.Tests
__tests__/ci/release-pipeline.test.ts— a tripwire over both workflows, in the spirit ofdogfood-configs.test.ts. It pins the publish-after-assets ordering, the bump guard, theworkflow_call/no-standalone-releasetriggers, the 4-platform matrix, and that a failed daemon build blocks the publish (a failed dependency leaves dependentsskipped, which would otherwise read as "nothing to do"). The platform list is cross-checked against the CLI'sPlatformKeyunion on refs that have it.🤖 Generated with Claude Code
https://claude.ai/code/session_01EbXKFnpRXBwsrvUJDdySky
Summary by CodeRabbit
Review follow-up
bun.lockin every Bun cache key.