Skip to content

Refactor electron headers handling in desktop.nix - #61824

Open
fkz wants to merge 1 commit into
NousResearch:mainfrom
fkz:main
Open

Refactor electron headers handling in desktop.nix#61824
fkz wants to merge 1 commit into
NousResearch:mainfrom
fkz:main

Conversation

@fkz

@fkz fkz commented Jul 10, 2026

Copy link
Copy Markdown

What does this PR do?

Remove hardcoded hash and use electron.headers from nixpkgs input directly instead.
This improves maintainability of the nix build, as the hash does not need to be changed when nixpkgs is updated.
Additionally, it can also be used with different nixpkgs versions (e.g. by using nix run github:NousResearch/hermes-agent --override-input nixpkgs nixpkgs).

Related Issue

Fixes #61443

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Remove depedency hash in nix/desktop.nix and instead use headers directly from nixpkgs

How to Test

To reproduce, build the desktop variant using nix, by doing:

  1. nix build github:NousResearch/hermes-agent#desktop

After this PR, you can also use it with (somewhat) different nixpkgs versions with a different electron. For example do the following:

  1. nix build github:NousResearch/hermes-agent#desktop --override-input nixpkgs github:NixOS/nixpkgs/a0374025a863d007d98e3297f6aa46cc3141c2f0

This fails in main, but succeeds after this PR, as we use a slightly different electron version.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform:

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

…op.nix

Remove hardcoded hash and use electron.headers directly instead.

Fixes NousResearch#61443
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change comp/desktop Electron desktop app (apps/desktop/*) area/nix Nix flake, NixOS module, container packaging P2 Medium — degraded but workaround exists labels Jul 10, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Competing fixes for #61443: this PR derives the Electron node-headers from the electron.headers nixpkgs input (durable — no manual hash bump per electron update), while #61530 is a one-off stopgap that just bumps the hardcoded sha256. Related, not duplicate — a maintainer should pick the durable-derive approach here vs the stopgap bump in #61530.

@fkz
fkz marked this pull request as ready for review July 10, 2026 03:32
@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 11, 2026
@ak2k

ak2k commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Prior art: #53202 (opened 2026-06-26) makes the same change in nix/desktop.nix — dropping the hand-pinned fetchurl in favor of nixpkgs' electron.headers, which is version-locked to the electron package (same pattern as signal-desktop, github-desktop, session-desktop in nixpkgs). Verified there with nix build .#desktop on aarch64-darwin at electron 41.9.1.

#53202 also adds an offline nix build .#web .#tui .#desktop CI lane. That matters for this bug class specifically: the break only surfaces in the sandboxed Nix build (the networked Docker/npm ci paths download headers at build time and never notice), and no existing workflow built the flake offline — which is how the stale hash merged green in the first place. Without that guard, a reintroduced hand-pinned fetch would desync again on the next nixpkgs electron bump.

Flagging so a maintainer evaluating this vs #61530 sees all three options: #53202 = this same durable derive + the CI guard that prevents recurrence.

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/*) P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/refactor Code restructuring, no behavior change

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