Skip to content

flake.nix: pnpmDeps, bump fetcherVersion = 4 - #1247

Open
manuelbb-luh wants to merge 3 commits into
Nano-Collective:mainfrom
manuelbb-luh:patch-1
Open

manuelbb-luh wants to merge 3 commits into
Nano-Collective:mainfrom
manuelbb-luh:patch-1

Conversation

@manuelbb-luh

Copy link
Copy Markdown

Description

Fix build failure with recent nixpkgs.
Support for fetcherVersion = 3 has been dropped: NixOS/nixpkgs#538919
I have updated the version number and put in the hash I got on my local machine.

Have done no testing or anything, just noticed the issue when trying to upgrade my flake-based NixOS.
This is my current local workaround which builds successfully:

# home.nix
let
  nanocoder-base = inputs.nanocoder.packages."${pkgs.stdenv.hostPlatform.system}".default;
  fetchPnpmDeps = pkgs.fetchPnpmDeps.override { pnpm = pkgs.pnpm_11; };
  nanocoder = nanocoder-base.overrideAttrs (fin: {
    pnpmDeps = (fetchPnpmDeps {
      inherit (fin) pname version src;
      hash = "sha256-J9DxZAW9627pa+gUEhGqcr/Fd4s4jqDlIcLE2iI5vXE="; 
      fetcherVersion = 4;
    });
  });
in

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Changeset

  • Added a changeset (pnpm changeset) describing this change for the changelog

Docs-only or internal chores need no changeset (or run pnpm changeset --empty to note that intentionally).

Testing

Automated Tests

  • New features include passing tests in .spec.ts/tsx files
  • All existing tests pass (pnpm test:all completes successfully)
  • Tests cover both success and error scenarios

Manual Testing

  • Tested with Ollama
  • Tested with OpenRouter
  • Tested with OpenAI-compatible API
  • Tested MCP integration (if applicable)

Checklist

  • If this was for an open issue, I was assigned to it
  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated (if needed)
  • No breaking changes (or clearly documented)
  • Appropriate logging added using structured logging (see CONTRIBUTING.md)

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

nc-review: comments — 2 important, 1 nit

@manuelbb-luh — a few things worth a look, none blocking.

The PR bumps pnpmDeps's fetcherVersion from 3 to 4 (matching nixpkgs PR #538919, which dropped v3) and drops the now-redundant overrideAttrs workaround for the long-fixed pnpm_config_* env-var shell-syntax bug. The new hash is consistent with the new fetcher and the change is narrowly scoped to flake.nix. Two follow-ups worth flagging: a changeset is required for this user-facing build fix, and the stale comment at the top of .github/workflows/update-nix.yml (which still tells maintainers about the now-removed override) should be updated in the same PR.

🟠 important · changeset · .changeset

The PR touches a user-facing surface (the nix flake build) — end users on recent nixpkgs currently cannot build nanocoder from the flake, and this fixes it. The project rubric says a user-facing change needs a changeset (.changeset/*.md), and the maintainer workflow release-prepare.yml consumes those. The author's PR template leaves the changesets box unchecked and no changeset file is present. Add a patch-level changeset such as .changeset/flake-fetcherversion-4.md describing the fix.

🟠 important · completeness · .github/workflows/update-nix.yml

The header comment at the top of update-nix.yml still describes the pnpm_config_side_effects_cache / pnpm_config_update_notifier workaround and points readers at the pnpmDeps block in flake.nix for the verify-and-remove recipe. The PR removes both the workaround and the long comment from flake.nix, but leaves this CI-side comment stale, so the next maintainer who runs the workflow will follow a recipe for code that no longer exists. Either delete the header comment in this PR or rewrite it to reflect the new fetcherVersion = 4 reality.

⚪ nit · completeness · flake.nix

The new pnpmDeps hash (sha256-J9DxZAW9627pa+gUEhGqcr/Fd4s4jqDlIcLE2iI5vXE=) was generated on the contributor's local machine, not through the maintainer workflow's nix run nixpkgs#nix-update -- --flake default --build. The author self-disclosed "Have done no testing or anything." fetcherVersion = 4 is supposed to be deterministic, but a fresh pnpmDeps derivation whose hash was never cross-checked is worth re-verifying (e.g. by re-running nix-update --build or nix-build on a second machine) before merging. If the maintainer workflow regenerates it, the commit hash will simply change — not a correctness bug, just a provenance nit.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:comments nc-review left non-blocking findings label Sep 9, 2026
@will-lamerton

Copy link
Copy Markdown
Member

Please can you address nc-review? Thanks :) @manuelbb-luh

@github-actions

Copy link
Copy Markdown
Contributor

Hi @manuelbb-luh, thanks for this PR! It looks like a maintainer has left feedback
or review activity and there are still some outstanding items to wrap up.

Whenever you get a chance, could you take a look at the open comments?
If anything is unclear or you'd like a hand, just reply here and we'll help you get it across the line.

@github-actions github-actions Bot added the area:ci GitHub Actions and CI label Sep 18, 2026
@will-lamerton

Copy link
Copy Markdown
Member

Thanks for picking up the nc-review points @manuelbb-luh, the changeset and the workflow comment are both sorted.

I verified the substance of the change locally and it's correct: nixpkgs master does now have export pnpm_config_side_effects_cache=false (with the =), so dropping the overrideAttrs workaround is right, and your new hash reproduces. I built pnpmDeps against nixpkgs 79b35bf on aarch64-darwin and the fixed-output hash matched sha256-J9DxZAW9627pa+gUEhGqcr/Fd4s4jqDlIcLE2iI5vXE=, then built the full package successfully. So that's independently reproduced on a different machine and platform, which clears the provenance nit.

One blocker though: flake.lock needs bumping in this PR.

The lock pins nixpkgs at d233902 (2026-05-15), and that rev's supportedFetcherVersions is [1 2 3]. fetcherVersion = 4 only lands later. So on this branch the flake doesn't evaluate at all:

$ nix eval .#packages.aarch64-darwin.default.pnpmDeps.drvPath
error: fetchPnpmDeps `fetcherVersion` is not set to a supported value (1, 2, 3)

Your local setup works because you override fetchPnpmDeps with your own newer pkgs, so our pinned lock never comes into play. Anyone running nix build github:nano-collective/nanocoder hits the error above. It also breaks release, since update-nix.yml runs nix-update --flake default --build against this lock.

nix flake update nixpkgs is all that's needed, that's exactly what I did locally to get the green builds above.

Two stale comments left over as well:

  • .github/workflows/update-nix.yml line 3 still opens with "pnpm 11 reproducibility relies on two env-var derivation attrs in flake.nix (pnpm_config_side_effects_cache, pnpm_config_update_notifier)", but this PR deletes those attrs, so the rewritten comment contradicts its own first sentence.
  • flake.nix lines 34-36 in the let block (untouched here) still says "The remaining pnpmDeps reproducibility fix lives below: see the comment on pnpmDeps for the upstream shell-syntax bug we work around with env-var derivation attrs."

Tiny nits: trailing whitespace on flake.nix:61 and update-nix.yml:6.

Once the lock bump is in I'll approve and get the checks approved to run.

@github-actions

Copy link
Copy Markdown
Contributor

Hi @manuelbb-luh, thanks for this PR! It looks like a maintainer has left feedback
or review activity and there are still some outstanding items to wrap up.

Whenever you get a chance, could you take a look at the open comments?
If anything is unclear or you'd like a hand, just reply here and we'll help you get it across the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:comments nc-review left non-blocking findings area:ci GitHub Actions and CI stale:nudged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants