Skip to content

Fix the Nix flake, and stop the hash check reporting false success - #606

Merged
jeremy merged 3 commits into
mainfrom
fix-nix-toolchain
Aug 3, 2026
Merged

Fix the Nix flake, and stop the hash check reporting false success#606
jeremy merged 3 commits into
mainfrom
fix-nix-toolchain

Conversation

@jeremy

@jeremy jeremy commented Aug 3, 2026

Copy link
Copy Markdown
Member

v0.8.0 shipped a flake that cannot build

go: go.mod requires go >= 1.26.5 (running go 1.26.4; GOTOOLCHAIN=local)

#533 raised go.mod to 1.26.5 on 2026-07-13. flake.lock still pinned a
nixpkgs from 2026-06-28 carrying 1.26.4, and release prep never updates the
lock. Every nix build user has been broken since, across two releases.

Why nothing caught it

Both safeguards were blind, independently:

  • update-nix-flake.sh reported success on failure. It ran
    nix build ... || true, then treated the presence of
    building '...basecamp-0.8.0-go-modules.drv' as proof of success. Nix prints that
    when the build starts — so a hard failure printed
    vendorHash: verified (build succeeded). It said exactly that during the v0.8.0
    release, which is how a broken flake got through a pre-tag check that existed to
    prevent this.
  • nix-verify could not fail the run. continue-on-error: true, and it only runs
    on tags — weeks after the drift is introduced. For v0.8.0 it never ran at all,
    because the AUR outage reddened release and skipped every needs: [release] job.

The fix

Lock + hash. Updating nixpkgs to one carrying 1.26.5 surfaced a second defect the
Go error had been masking — the recorded vendorHash was stale as well:

specified: sha256-hG1eymlnBAhRDtOqi078uVwMwEFC9+8ilft1LOW7SzY=
got:       sha256-DmCW1Ms9TmcvEzvNIcqutf1bxtOpN+MjpPO0XP1VaZQ=

Both corrected. nix build now exits 0 and the resulting binary reports
basecamp version 0.8.0.

Fail closed. The release-time check now reads the real exit status. A failure that
is not a hash mismatch is fatal instead of shrugged off, and when it does update a
hash it rebuilds to prove the new one works — which the old code never did.

Catch it at the source. nix-verify is no longer continue-on-error, and a
path-filtered nix-build job runs on PRs touching go.mod, go.sum, flake.* or
nix/**, so a Go bump that outpaces the lock fails its own PR.

Verification

Run against the real Docker path, not reasoned about:

Scenario Before After
Stale lock (the v0.8.0 condition) vendorHash: verified (build succeeded), exit 0 exit 1, prints the Go version error
Fixed lock + hash vendorHash: verified (build succeeded), exit 2
nix build on this branch fails exit 0, binary runs

bin/ci exit 0, actionlint clean, zizmor no findings.

Consequence for the queue

0.8.1 cannot be a bare manifest bump — it has to carry this first, or it repeats the
broken Nix release. Let release.sh stamp both plugin manifests as part of that
release rather than bumping them standalone.


Summary by cubic

Fixes the broken Nix flake by updating flake.lock to a nixpkgs with Go 1.26.5 and correcting the Go vendorHash. Makes the hash check accurate and fail-closed so a flake that can’t build can’t ship again.

  • Bug Fixes

    • Update flake.lock to a revision with Go 1.26.5.
    • Update nix/package.nix vendorHash.
    • Only rewrite vendorHash when nix reports a fixed-output hash mismatch and the captured value is SRI-shaped; then rebuild to verify.
  • CI

    • Make release nix-verify blocking (remove continue-on-error).
    • Fail closed end-to-end: use the real nix build exit code, build unconditionally, update the hash only on mismatch, and make update-nix-hash now propagates real failures (RC 0=updated, 2=no-op).
    • Add a path-filtered nix-build job in PR CI for changes to go.mod, go.sum, flake.*, and nix/**.
    • Add regression tests for the hash-check script (stubbed Docker), including a case that rejects unrelated “got:” logs.

Written for commit e508d3d. Summary will update on new commits.

Review in cubic

v0.8.0 shipped a flake that cannot build at all:

    go: go.mod requires go >= 1.26.5 (running go 1.26.4; GOTOOLCHAIN=local)

#533 raised go.mod to 1.26.5 on 2026-07-13, but flake.lock still pinned a
nixpkgs from 2026-06-28 carrying 1.26.4, and release prep never updates the
lock. Every `nix build` user has been broken since, across two releases.

Nothing caught it because every check that could have was blind:

- update-nix-flake.sh ran `nix build ... || true`, then treated the presence of
  `building '...basecamp-0.8.0-go-modules.drv'` as proof of success. That string
  is printed when the build *starts*, so a hard failure reported "vendorHash:
  verified (build succeeded)".
- nix-verify was continue-on-error, so it could not fail a run — and it only
  runs on tags, weeks after the drift is introduced.

Update the lock to a nixpkgs with 1.26.5, which then surfaces a second defect
the Go error had been masking: the recorded vendorHash was stale too. Both
fixed; `nix build` now succeeds and the resulting binary reports its version.

Make the release-time check fail closed. It reads the real exit status, and a
failure that is not a hash mismatch is now fatal rather than shrugged off. When
it does update a hash it rebuilds to prove the new one works, which the old code
never did.

Make nix-verify blocking, and add a path-filtered flake build to PR CI so a Go
bump that outpaces the lock fails its own PR instead of surfacing three weeks
later at release time.
Copilot AI review requested due to automatic review settings August 3, 2026 17:26
@github-actions github-actions Bot added the ci CI/CD workflows label Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

  • .github/workflows/release.yml
  • .github/workflows/test.yml

Shadow mode — this check is informational only. When activated, changes to these paths will require approval from a maintainer.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e18571a4e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/test.yml

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the broken Nix flake that shipped in v0.8.0 and closes the two independent gaps that let it ship. When #533 raised go.mod to Go 1.26.5, flake.lock still pinned a nixpkgs carrying 1.26.4, and the recorded vendorHash was also stale, so nix build failed for every user. The release-time hash check masked this by treating the "build starting" log line as success, and the tag-only nix-verify job was continue-on-error and never ran. The fix updates the lock and hash, makes the hash script read the real exit status and rebuild to prove the corrected hash, makes nix-verify blocking, and adds a path-filtered PR job so a Go bump that outpaces the lock fails its own PR.

Changes:

  • Update flake.lock to a nixpkgs revision with Go 1.26.5 and correct nix/package.nix's vendorHash.
  • Rewrite scripts/update-nix-flake.sh to use the real nix build exit status (via a trailing NIX_BUILD_EXIT= marker), fail closed on non-hash failures, and rebuild to verify an updated hash.
  • Harden CI: remove continue-on-error from release nix-verify, and add a path-filtered nix-build job in test.yml for go.mod/go.sum/flake.*/nix/** changes.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scripts/update-nix-flake.sh Replaces the || true + log-heuristic with a real exit-status check, fail-closed error handling, and a rebuild-to-verify step after updating the hash.
nix/package.nix Updates the stale vendorHash to the value produced by the corrected build.
flake.lock Bumps the pinned nixpkgs revision to one carrying Go 1.26.5.
.github/workflows/test.yml Adds a path-filtered nix-build PR job that builds the flake and runs the binary when flake-relevant files change.
.github/workflows/release.yml Makes nix-verify blocking by removing continue-on-error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The script now fails closed, but three paths still bypassed that.

make update-nix-hash ran the script under `|| true`, converting the new exit 1
straight back into success — the manual entrypoint undid the fix for anyone not
going through release.sh. Preserve 0 (updated) and 2 (nothing to do); propagate
everything else. The script's Docker-unavailable message pointed at that same
target as a workaround, which was circular; it now says to install Docker,
because a release must not ship an unverified flake.

The Docker build sat entirely inside the deps-changed branch, so a stable
release that touched neither go.mod nor go.sum skipped it. That is exactly the
shape of the failure it exists to catch: a Go toolchain bump outpacing
flake.lock changes neither file. Build unconditionally; the deps comparison now
only chooses the wording.

Grant pull-requests: read for dorny/paths-filter, matching installer-bash32.
The filter did work without it on this repo — public repos can enumerate PR
files unauthenticated — but the action documents the permission as required, so
relying on that is relying on an accident.

Add exit-status regression tests with a stubbed docker. The PR-time flake build
proves the current flake compiles; it cannot prove the script distinguishes a
passing build from a failing one, which is the defect that actually shipped.
The stub reproduces the v0.8.0 log shape exactly: nix announcing it is building
basecamp, then dying on the Go version.
Copilot AI review requested due to automatic review settings August 3, 2026 19:03
@github-actions github-actions Bot added the tests Tests (unit and e2e) label Aug 3, 2026
@jeremy

jeremy commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Pushed 70d0260d closing three remaining bypasses. All three were real; verified each before fixing.

P1 — make update-nix-hash fails open. Makefile:216 ran the script under || true,
converting the new exit 1 straight back into success. The manual entrypoint undid the fix for
anyone not going through release.sh. Now preserves 0 (updated) and 2 (nothing to do) and
propagates everything else. Proved against real Docker with a stale lock:

make EXIT=2
error: Build failed due to failed dependency
NIX_BUILD_EXIT=1
make: *** [update-nix-hash] Error 1

The script's Docker-unavailable message also pointed at that same Make target as a workaround —
circular, since it is the weaker path. It now says to install Docker.

P2 — validation was conditional. The Docker build sat entirely inside the deps-changed
branch, so a stable release touching neither go.mod nor go.sum skipped it. That is precisely
the shape of the failure it exists to catch: a Go toolchain bump outpacing flake.lock changes
neither file. The build is now unconditional; the deps comparison only picks the wording. 0.8.1
would have run it either way (goldmark moved), but the gate should not depend on that.

P2 — pull-requests: read added, matching installer-bash32. See the thread for why the
stated failure mode did not actually occur here — the fix is still right, just for a different
reason.

Regression tests for exit-status classification (e2e/update_nix_flake.bats, 5 cases, stubbed
docker). The PR-time flake build proves the current flake compiles; it cannot prove the script
tells a passing build from a failing one, which is the defect that shipped. One case reproduces the
v0.8.0 log shape exactly — nix announcing building '...basecamp-0.8.0-go-modules.drv', then dying
on the Go version — and asserts the script exits 1 rather than reporting success.

On refreshing flake.lock during release prep: agreed, not doing it. That would land an
unreviewed dependency change on main after release-check has already run. Locks move through
PRs; release prep validates the dependency set and never mutates it. The new PR-CI job is what
keeps the lock from drifting in the first place.

bin/ci exit 0, actionlint clean, zizmor no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70d0260db6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/update-nix-flake.sh Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Matching a bare `got:` was far too loose. Any failing build whose log happens to
contain one — a Go test assertion printing `got: 42` — had its second field
written straight into vendorHash. The rebuild then failed, so nothing shipped,
but nix/package.nix was left corrupted and the run reported a hash problem when
the real failure was something else entirely.

Making the build unconditional widened the exposure: more failures now reach
this parse than when it only ran on dependency changes.

Require both signals before touching the file — nix must have reported a
fixed-output hash mismatch, and the captured value must be SRI-shaped.

The new BATS case fails against the old parse and passes against this one, so it
pins the behaviour rather than merely describing it.
Copilot AI review requested due to automatic review settings August 3, 2026 19:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

@jeremy
jeremy merged commit 9160c09 into main Aug 3, 2026
25 of 26 checks passed
@jeremy
jeremy deleted the fix-nix-toolchain branch August 3, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/CD workflows tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants