Skip to content

Share the AUR notifier and the Nix hash classifier between their callers - #609

Merged
jeremy merged 1 commit into
mainfrom
ci/shared-notify-and-nix-check
Aug 4, 2026
Merged

Share the AUR notifier and the Nix hash classifier between their callers#609
jeremy merged 1 commit into
mainfrom
ci/shared-notify-and-nix-check

Conversation

@jeremy

@jeremy jeremy commented Aug 4, 2026

Copy link
Copy Markdown
Member

Two pieces of logic each existed twice, and in both cases only one copy would
have been fixed. Both are consequences of my own earlier changes.

Label-based dedup, in both AUR paths

#607 was opened by release.yml, not the manual recovery workflow — its body
("Publishing [v0.8.1]…") is release.yml's wording, not aur-publish.yml's
("Recovery publish of…"). The canonical #602 was already open; I had retitled it,
and both files dedupe with --search "in:title $TITLE", so the lookup missed
and filed a duplicate. Fixing only the manual workflow leaves the automatic
release path opening another one on the next failure.

scripts/notify-issue.sh now searches open issues by the aur-publish
label
— which survives retitling, reassignment and rewording — comments on the
match, or creates with the label. Both workflows call it.

It fails closed

The old lookup was ... 2>/dev/null || true. That makes a rate limit, a
permissions gap and a GitHub outage indistinguishable from "no open issue", and
the very next line files one. Four branches, each pinned by a test:

Lookup result Behaviour
errors file nothing, exit 2, annotation retained
zero matches create one issue, carrying the label
exactly one comment on it
more than one refuse to guess, exit 3, file nothing

Both workflows emit their ::error:: annotation regardless of what the notifier
did, so when it fails closed the failure is still visible on the run.

Extract the Nix classifier so tests can reach it

The nix-build job from #606 triggers on go.mod/go.sum, so every weekly
dependabot Go bump will fail it
— a dependency change invalidates vendorHash
and dependabot will not update it. The job should fail. The defect is that it
failed as a raw nix dump with no remedy, and being an inline run: block, no
test could reach it. Its path filter also omitted .github/workflows/test.yml,
so a PR editing the job would skip the job it edited.

scripts/nix-build-check.sh runs the build, replays nix's diagnostics either
way, propagates the build's real exit status (no || true), and emits the
corrected sha256-… plus make update-nix-hash — but only when both the
fixed-output diagnostic and an SRI-shaped value are present.

That guard is scripts/extract-nix-vendor-hash.sh, shared with
update-nix-flake.sh
rather than copied into it. Two production classifiers
that can drift is one too many when the one that drifts is the one that writes
into nix/package.nix. Fixture tests pin the helper; separate wrapper tests pin
exit propagation, diagnostic replay, annotation rendering, and that a successful
build is not trusted until the binary it produced actually runs.

Considered and rejected: narrowing the filter to drop go.sum. It would still
have caught v0.8.0's Go-toolchain drift (go.mod's go directive plus
flake.lock) but would stop catching a stale vendorHash — the second v0.8.0
defect.

Verification

  • bin/ci exit 0; make lint-actions clean (actionlint + zizmor).
  • 31 BATS tests green across the three new suites and the existing
    update_nix_flake.bats, which now routes through the shared classifier.
  • Mutation-checked, not just green: reverting the lookup to || true fails
    exactly the fail-closed test, and loosening the SRI pattern to [^[:space:]]+
    fails exactly the two shape tests. Nothing else moved.
  • The live notify path was dry-run against real GitHub with writes stubbed — it
    resolves to labelled Publish v0.8.1 to the AUR once Arch re-enables pushes #602 and takes the comment branch, so it will not open
    a third issue.
  • This PR changes test.yml, so the nix-build job running here is itself the
    proof that the widened path filter works.
  • A temporary wrong-vendorHash commit will demonstrate the ::error::
    rendering once, then be removed before review.

The aur-publish label has been created and applied to #602, so the label path
is live before this merges.

Refs #602, #607


Summary by cubic

Share the AUR failure notifier and the Nix vendorHash classifier across workflows to remove duplicate logic, prevent duplicate issues, and make CI failures actionable with the correct hash and remedy.

  • Bug Fixes

    • AUR issue dedup now keys on the aur-publish label via scripts/notify-issue.sh.
    • Notifier fails closed on lookup errors or multiple matches (files nothing; exits non‑zero). Both workflows still emit their ::error:: annotation and propagate the notifier’s status.
  • Refactors

    • Extracted vendorHash classifier to scripts/extract-nix-vendor-hash.sh and wired it into scripts/nix-build-check.sh and scripts/update-nix-flake.sh.
    • nix-build job now runs scripts/nix-build-check.sh: replays nix logs, preserves real exit status, annotates with the corrected sha256-… and make update-nix-hash only on a fixed‑output mismatch with an SRI hash, verifies the built binary runs, and errors if nix prints no store path.
    • Widened test.yml path filter to include Nix and script paths (incl. this file). Added BATS tests for the notifier, classifier, and build wrapper.

Written for commit 7a5f278. Summary will update on new commits.

Review in cubic

Two workflow paths each carried their own copy of logic that had to
agree, and in both cases only one copy would have been fixed.

Dedup by label, not title. #607 was opened by release.yml — its body is
release.yml's wording — while the canonical #602 sat open under a title
I had edited. Both files searched `in:title`, so both would have missed;
fixing only the recovery workflow would have left the automatic release
path opening a duplicate on the next failure. notify-issue.sh now looks
up open issues by the `aur-publish` label, which survives retitling, and
both workflows call it.

It also fails closed. The old lookup wrapped `gh issue list` in
`2>/dev/null || true`, which makes a rate limit, a permissions gap and a
GitHub outage indistinguishable from "no open issue" — and the next line
files one. A lookup that errors now files nothing and exits nonzero;
more than one labelled issue is also refused rather than resolved by
guessing. Both workflows still emit their `::error::` annotation
whatever the notifier does, so the underlying failure stays visible.

Extract the Nix classifier so it can be tested. The nix-build job from
#606 triggers on go.mod/go.sum, so every weekly dependabot Go bump fails
it: a dependency change invalidates vendorHash and dependabot does not
update it. The job *should* fail there. What was wrong is that it failed
as a raw nix dump with no remedy, and being inline in the workflow, no
test could reach it. nix-build-check.sh now runs the build, replays
nix's diagnostics either way, propagates the real exit status, and emits
the corrected `sha256-…` with `make update-nix-hash` — but only when the
classifier confirms both a fixed-output mismatch diagnostic and an
SRI-shaped value.

That classifier is extract-nix-vendor-hash.sh, shared with
update-nix-flake.sh rather than copied into it. Two production parsers
that can drift is one too many when the one that drifts is the one that
writes to nix/package.nix.

The nix path filter picks up test.yml, the two scripts, and keeps
go.sum. Narrowing it to drop go.sum would still have caught v0.8.0's
Go-toolchain drift but would stop catching a stale vendorHash — the
second v0.8.0 defect.

Refs #602, #607
Copilot AI review requested due to automatic review settings August 4, 2026 00:00
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

  • .github/workflows/aur-publish.yml
  • .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.

@github-actions github-actions Bot added tests Tests (unit and e2e) ci CI/CD workflows labels Aug 4, 2026
@jeremy
jeremy force-pushed the ci/shared-notify-and-nix-check branch from 2de10d1 to 7a5f278 Compare August 4, 2026 00:02
@jeremy

jeremy commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Annotation rendering, demonstrated and removed

The temporary wrong-vendorHash commit has served its purpose and is gone
(branch is back to a single commit, 7a5f2782). What it produced in
the run:

building '/nix/store/…-basecamp-0.8.1-go-modules.drv'...
error: hash mismatch in fixed-output derivation '/nix/store/…-go-modules.drv':
         specified: sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
            got:    sha256-DmCW1Ms9TmcvEzvNIcqutf1bxtOpN+MjpPO0XP1VaZQ=
error: Cannot build '/nix/store/…-basecamp-0.8.1.drv'.
##[error]Stale Nix vendorHash. Expected sha256-DmCW1Ms9TmcvEzvNIcqutf1bxtOpN+MjpPO0XP1VaZQ=. \
A go.mod or go.sum change invalidates it and dependabot does not update it — \
run 'make update-nix-hash' and commit nix/package.nix.

The hash named in the annotation is exactly the one that was removed, nix's own
diagnostics are replayed above it rather than replaced by it, and the job exits 1.

Path filter, demonstrated

The first push (7a5f2782) touches test.yml, scripts/** and e2e/** — and
no nix/**, go.mod, go.sum or flake.*. On that run the Nix flake builds job's Install Nix and Build the flake steps both ran and passed,
rather than being skipped:

https://github.com/basecamp/basecamp-cli/actions/runs/30864176279

Under the old filter a PR that changed only this job would have skipped the job
it changed.

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 ec979fc into main Aug 4, 2026
44 of 47 checks passed
@jeremy
jeremy deleted the ci/shared-notify-and-nix-check branch August 4, 2026 00:05
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