ci(nix): fix the bump workflow's PR creation, and catch npmDepsHash drift - #230
Merged
Conversation
…he PR that causes it Two failures, one visible and one not. `gh pr create` ran under GITHUB_TOKEN while the repo has "Allow GitHub Actions to create and approve pull requests" turned off, so the v1.7.0 bump pushed its branch and then died on the API call. #136 was opened by hand to work around it. Every other release workflow already authenticates with OPENSCREEN_RELEASE_TOKEN, which is not subject to that restriction and whose PRs trigger CI — so the note telling reviewers to push an empty commit goes away too. The quiet one: `src` is this repo's tree rather than a fetched tarball, so npmDepsHash has to agree with whatever package-lock.json sits beside it, and the only thing that refreshed it fired on stable releases. main's hash last matched on 2026-07-05 while the lockfile moved ten more times, leaving `nix run github:getopenscreen/openscreen` broken for four weeks with nothing reporting it. A release-time bump structurally cannot cover that gap, so the check now runs on lockfile and nix/ changes and prints the hash to paste. Version goes to 1.7.0 here, which is what #136 was for.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe Nix package version and dependency hash are updated. A workflow validates hash synchronization on relevant pull requests and pushes. Release bump pull requests use ChangesNix release automation
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant Nix
participant PackageFiles
GitHubActions->>Nix: Compute npm dependency hash
Nix->>PackageFiles: Read package-lock.json
GitHubActions->>PackageFiles: Read npmDepsHash from nix/package.nix
GitHubActions->>GitHubActions: Compare the hashes
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
The value dated to v1.6.0 on 2026-07-05; package-lock.json moved ten times after that, so `nix build` had been refusing the mismatch ever since. The replacement is what the new check computed from the current lockfile: recorded sha256-IZypOLWlDShIjCKWxlJcrdtIkMu0P/DuXaq4c0HW3FY= expected sha256-SggSPoDnKzmvgXpIGP11y6h390SkoZszeMjFTaokRjQ= #136 carried sha256-cb8loUzQs4fz3um0xbYgtjQdcRZh/Ptk2YPgv3FHP/s=, computed from main as it stood on 2026-07-19 — stale for the same reason, which is why merging it would not have fixed anything.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/nix-check.yml (1)
33-33: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable checkout credential persistence.
actions/checkout@v4persists its token in local Git configuration by default. Setpersist-credentials: false. Set workflow permissions tocontents: readbecause this validation job does not write repository data.Proposed hardening
name: Nix + +permissions: + contents: read # nix/package.nix records `npmDepsHash`, a hash of the npm dependency set that @@ - - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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/nix-check.yml at line 33, Update the checkout step using actions/checkout@v4 to set persist-credentials to false, and configure the workflow permissions to contents: read for this validation job. Preserve the existing checkout behavior otherwise.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In @.github/workflows/nix-check.yml:
- Line 33: Update the checkout step using actions/checkout@v4 to set
persist-credentials to false, and configure the workflow permissions to
contents: read for this validation job. Preserve the existing checkout behavior
otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f86bfb60-e847-45e2-ac6f-b88a969b0a01
📒 Files selected for processing (3)
.github/workflows/bump-nix-package.yml.github/workflows/nix-check.ymlnix/package.nix
The push trigger listed main alone, which is the same gap ci.yml carries a comment about after PRs #167-#169 merged into a release branch with every job skipped. Pull requests were already covered everywhere — that trigger has no branch filter — but a rebase force-push onto release/** is not a pull request, and a release branch is the last place to let a stale hash through unnoticed.
ci(nix): least-privilege the two nix workflows, symmetric path filters Review follow-ups, all three mechanical: - nix-check.yml declares `permissions: contents: read` instead of inheriting it. It was the only workflow in .github/workflows/ without a permissions block, and inheriting a repo-level setting is precisely what broke the bump workflow it ships alongside. - The push trigger now watches .github/workflows/nix-check.yml like the pull_request trigger already did, so an edit to the check itself is checked on both legs. - bump-nix-package.yml drops `pull-requests: write`. The PR is opened with the PAT now, so that grant on GITHUB_TOKEN buys nothing. @
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #136.
The visible break
gh pr createinbump-nix-package.ymlran underGITHUB_TOKEN, but the repo has Allow GitHub Actions to create and approve pull requests off:So the v1.7.0 run pushed its branch and then died on the API call — run 29707554857:
That is why #136 was opened by hand, and v1.8.0 would have hit it again. Switched to
OPENSCREEN_RELEASE_TOKEN, which every other release workflow already uses — it isn't subject to that restriction, and its PRs trigger CI, so the "push an empty commit if you want CI" note in the generated body is gone.Preferred over flipping the repo setting, which would loosen PR creation repo-wide for one workflow.
The quiet one
srcis this repo's own tree (afileset, not a fetched tarball), sonpmDepsHashhas to agree with thepackage-lock.jsonsitting beside it. The only thing that ever refreshed that hash was this release-time workflow.nix/package.nixlast changed 2026-07-05 (bump nix package to v1.6.0)package-lock.jsonlast changed 2026-07-29, after 10 commits touching itSo
nix run github:getopenscreen/openscreenhad been failing on main for about four weeks, and nothing reported it — no workflow rannix buildor otherwise looked at the hash.A release-time bump structurally cannot cover this: the drift starts the moment a lockfile PR merges, not at the next release.
nix-check.ymlruns on PRs touchingpackage-lock.jsonornix/**, compares the recorded hash againstprefetch-npm-deps package-lock.json, and fails with the value to paste. Same command the bump workflow already uses, so no new tooling.The guard proved itself on this PR
First push kept the stale hash deliberately. Run 30789853182 — failed, as it should have:
Pasted that value, run 30789963366 — green,
In sync.Worth noting: #136 carried
sha256-cb8loUzQs4fz3um0xbYgtjQdcRZh/Ptk2YPgv3FHP/s=, computed from main as it stood on 2026-07-19. That is neither of the two values above — it was stale for the same reason, so merging #136 would not have fixed the build either.versionalso goes to1.7.0, which is all #136 carried.Not covered here
No
required_status_checksrule exists in themain-protectionruleset, so this check reports but does not block a merge. Making it blocking is a separate call — there are currently no required checks at all on this repo.Summary by CodeRabbit