Skip to content

fix(nix/desktop): electron.headers + offline nix-build guard (.#web/.#tui/.#desktop) - #53202

Open
ak2k wants to merge 2 commits into
NousResearch:mainfrom
ak2k:fix/lockfile-restore-platform-optional-deps
Open

fix(nix/desktop): electron.headers + offline nix-build guard (.#web/.#tui/.#desktop)#53202
ak2k wants to merge 2 commits into
NousResearch:mainfrom
ak2k:fix/lockfile-restore-platform-optional-deps

Conversation

@ak2k

@ak2k ak2k commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Rebased onto current main and expanded from the original web/tui guard into a self-contained fix + guard. Two commits:

fix(nix/desktop): compile node-pty against electron.headers

The desktop renderer compiled node-pty against a hand-pinned fetchurl of Electron's node headers — URL templated off electron.version, but sha256 hardcoded. Every nixpkgs electron bump swaps the tarball out from under the pinned hash, failing the fixed-output derivation only at build time (the networked Docker / npm ci paths never see it). nixpkgs recently resolved electron41.9.1, breaking .#desktop for every Nix consumer:

error: hash mismatch in fixed-output derivation 'node-v41.9.1-headers.tar.gz.drv':
  specified: sha256-zi/QMwRZ0+FwE9XTE+DiSIeJXAwxmLKEaBWD5W3pMOI=
       got:  sha256-zOl8rx6woWh7aeRUOlkTMviKc/EAQQX6nr/MxAx1ZPI=

Fix: use nixpkgs' own electron.headers derivation (--nodedir=${electron.headers}). It's version-locked to the electron package, so it tracks every bump automatically — no hand-pinned hash to go stale, no network, and it's already the --nodedir layout (the manual fetchurl + tar both go away). Same pattern as signal-desktop, github-desktop, session-desktop, rstudio in nixpkgs. Closes #61443.

Verified: nix build .#desktop succeeds on aarch64-darwin with electron 41.9.1 (node-pty native binary compiles + stages).

ci(nix): offline nix-build guard for .#web/.#tui/.#desktop

No workflow built the flake offline before this (nix-setup existed but nothing called it), so two regression classes could merge green while breaking Nix consumers:

  1. Pruned lockfile.#web/.#tui build via importNpmLock (offline, no network), so a package-lock.json missing a platform-optional package (e.g. an incremental macOS npm install pruning @esbuild/<platform>) fails here even though Docker/npm ci succeed.
  2. Desktop native-compile breaks — like the electron-headers hash this PR fixes.

Adds a nix-build workflow running nix build .#web .#tui .#desktop offline, gated on a new nix lane (nix/, flake.nix, flake.lock) or the frontend lane, and required in the all-checks-pass gate. Classifier tests updated (23 pass).

@ak2k
ak2k requested a review from a team June 26, 2026 17:59
@alt-glitch alt-glitch added type/bug Something isn't working area/nix Nix flake, NixOS module, container packaging dependencies Pull requests that update a dependency file sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation P2 Medium — degraded but workaround exists labels Jun 26, 2026
@ak2k
ak2k force-pushed the fix/lockfile-restore-platform-optional-deps branch 2 times, most recently from 3b4ffee to b9493ba Compare June 26, 2026 19:01
@ak2k ak2k changed the title fix(npm): restore platform-optional packages pruned from package-lock.json chore(ci): build .#web and .#tui offline to guard against lockfile platform-pruning Jun 26, 2026
@ak2k
ak2k force-pushed the fix/lockfile-restore-platform-optional-deps branch from ab0116d to 5284e11 Compare July 11, 2026 16:15
@ak2k ak2k changed the title chore(ci): build .#web and .#tui offline to guard against lockfile platform-pruning fix(nix/desktop): electron.headers + offline nix-build guard (.#web/.#tui/.#desktop) Jul 11, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the durable Nix fix and regression guard. The reported defect is still present on current main: nix/desktop.nix:29-32 derives the header URL from electron.version but fixes one SHA-256, and the active rebuild path consumes that archive at nix/desktop.nix:80-93.

The proposed electron.headers substitution removes that independently pinned input while retaining the Electron ABI target. The guard also exercises the repository's real offline npm-install mechanism (nix/lib.nix:55-59), and the PR's nix-build / nix-build check passed.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
@ak2k

ak2k commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up that main HEAD (31c08a9) currently fails the offline Nix build this PR guards — in two independent ways, which I think is a good argument for landing the guard:

  1. .#desktop — the electron node-headers FOD this PR already fixes (hardcoded sha256 vs nixpkgs electron 41.9.1).
  2. .#tui — independent of (1): esbuild can't resolve @hermes/shared/charge-settlement even though apps/shared/src/charge-settlement.ts exists. Root cause looks like stale workspace paths in package-lock.json@hermes/shared is still referenced as file:../shared in places alongside the correct file:../apps/shared, left over from the shared/apps/shared/ relocation. Reproduces under the repo's own flake.lock nixpkgs (nix build .#tui), so it's not a downstream-pin artifact. A lockfile regen should clear it.

Both are exactly the "merges green under Docker/npm ci, breaks every Nix consumer" class the nix-build job here catches. Happy to open a separate issue for the tui/lockfile one if that's easier to track.

@ak2k

ak2k commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Correction on point (2) above: the tui break's root cause is the nix/tui.nix source filter omitting apps/shared (tracked in #67056, fixed by #67529), not package-lock drift as I first guessed. Point (1), the electron-headers .#desktop FOD, stands.

@berker-z

Copy link
Copy Markdown

God bless u

@ak2k

ak2k commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Additional data point: this breaks for consumers whose nixpkgs resolves a different electron

We hit this downstream. Worth adding a second framing, because the current one ("breaks on every nixpkgs electron bump") reads as latent — your CI is green, so nothing looks urgent.

The sharper version: nix/desktop.nix builds only against an electron matching the one hermes' own lock resolves.

Your flake.lock gives electron 41.0.2, and the hardcoded sha256-zi/QMw… is correct for it. A downstream flake setting hermes-agent.inputs.nixpkgs.follows — the standard way to dedupe a closure, and what consumers following your Nix docs will reach for — may land on a different one. We were on nixpkgs-nixos-unstable at 41.9.1:

error: hash mismatch in fixed-output derivation 'node-v41.9.1-headers.tar.gz.drv':
  specified: sha256-zi/QMwRZ0+FwE9XTE+DiSIeJXAwxmLKEaBWD5W3pMOI=
       got:  sha256-zOl8rx6woWh7aeRUOlkTMviKc/EAQQX6nr/MxAx1ZPI=
→ hermes-desktop-renderer-0.17.0 → hermes-desktop

The URL templates off electron.version; the hash doesn't. They desynchronize the moment a consumer's electron differs. This is structurally invisible to your CI, which only ever builds against the pinned lock where the hash is right by construction.

electron.headers fixes the class rather than the instance — it's version-locked to the electron package, so it tracks whatever electron the consumer's nixpkgs provides. That's the argument for it over #61530's hash bump, which restores green for 41.0.2 and re-breaks for anyone else.

ak2k added 2 commits July 29, 2026 09:17
hermes-desktop's renderer compiled node-pty against a hand-pinned fetchurl of
Electron's node headers, whose URL is templated off electron.version but whose
sha256 was hardcoded. Every nixpkgs electron bump swaps the tarball out from
under that pinned hash, failing the fixed-output derivation at build time — and
only there, never in the networked Docker/npm ci paths. nixpkgs recently
resolved electron to 41.9.1, breaking the build for every Nix consumer.

Use nixpkgs' own electron.headers derivation instead. It is version-locked to
the electron package, so it tracks every bump automatically with no hand-pinned
hash to go stale, needs no network, and is already the --nodedir layout (so the
manual fetch + tar extraction both go away). Same pattern as signal-desktop,
github-desktop, session-desktop, rstudio in nixpkgs.
No workflow built the flake offline before this (the nix-setup composite existed
but nothing called it), so two classes of regression could merge green while
breaking downstream Nix consumers: a package-lock.json that an incremental
npm install pruned platform-optional packages from (fails the offline
importNpmLock build of .#web/.#tui), and a desktop native-compile break like the
electron.headers hash this PR also fixes.

Add a nix-build workflow that runs `nix build .#web .#tui .#desktop` offline,
gated on a new `nix` lane (nix/, flake.nix, flake.lock) OR the frontend lane
(package-lock.json / web / ui-tui / apps), and require it in the all-checks gate.
@ak2k
ak2k force-pushed the fix/lockfile-restore-platform-optional-deps branch from 5284e11 to 08d4cb0 Compare July 29, 2026 13:27
@ak2k

ak2k commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (was 5284e11, now 08d4cb0) — the branch had gone dirty. Both commits preserved: the electron.headers fix and the offline nix-build guard.

Conflicts were confined to the CI scaffolding; nix/desktop.nix merged cleanly (it picked up your mkNpmPassthru folder/attrdirs change), and this PR's contribution to it is unchanged from the pre-rebase version. Resolutions were additive in every case: your npm_lock lane and this PR's nix lane now sit side by side in detect-changes/action.yml, ci.yml, and classify_changes.py, and nix-build joins the gate's needs: list. tests/ci/test_classify_changes.py needed a real merge in the root-lockfile case — kept your npm_lock=True assertion and added the flake cases alongside. 35/35 pass, actionlint clean on both workflows.

I also A/B'd the fix against the failure it prevents. Both arms pinned to the same nixpkgs (38a48874, electron 41.9.1) via .override { electron = …; }, so only the fix differs:

branch result
main (unfixed) hash mismatch … node-v41.9.1-headers.tar.gz
this PR builds

So electron.headers fixes the class, not just the currently-pinned version.

One honest note on the guard's scope, since it's the half this PR uniquely adds: it would not have caught the consumer breakage I reported above, because CI only ever builds against this repo's own lock, where the hardcoded hash is correct by construction. What it does catch is the next time nixpkgs moves electron underneath you — which is how the current stale hash got there. That's the regression worth gating on.

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

Labels

area/nix Nix flake, NixOS module, container packaging comp/desktop Electron desktop app (apps/desktop/*) dependencies Pull requests that update a dependency file P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nix: .#desktop build breaks on every nixpkgs electron bump (hardcoded node-headers hash)

4 participants