refactor(modules): extract shared customPackages overlay into flake.lib by Bad3r · Pull Request #166 · Bad3r/nixos

Bad3r · 2026-05-04T21:15:19Z

Summary

Hoists the ~95% identical content of modules/system76/custom-packages-overlay.nix and modules/tpnix/custom-packages-overlay.nix into a single config.flake.lib.customPackagesOverlay defined in modules/base/custom-packages.nix. Hosts now compose the shared overlay with any host-specific extras (e.g., system76-power SCSI patch on system76).
Mirrors the established flake.lib.<namespace> convention already used by modules/security/sops-helpers.nix, modules/security/usbguard-lib.nix, and modules/meta/nixos-app-helpers.nix.
Eliminates the structural cause of the merge-conflict-marker class of bug fixed in PR fix(tpnix): resolve unresolved merge conflict markers in overlay #165 (which left literal <<<<<<< / ======= / >>>>>>> markers in only one of the two host files when an upstream change touched the shared region).

This addresses the non-blocking observation from claude-review on PR #165 (run 25341374903): "lift the common attrset into a single shared customPackages overlay imported by both hosts — would prevent the next merge from re-creating exactly this class of conflict."

Stacked on top of #165 (fix/tpnix-merge-conflict-markers); rebases cleanly on main once that lands.

Test plan

nix flake check --accept-flake-config --no-build reports all checks passed!
nix eval .#nixosConfigurations.system76.config.system.build.toplevel.outPath succeeds
nix eval .#nixosConfigurations.tpnix.config.system.build.toplevel.outPath succeeds
Per-package drvPath equality vs. pre-refactor on brave-origin, system76-power, librepods (all byte-identical)
system.activationScripts.binsh byte-identical pre/post-refactor
environment.systemPackages differs only on nixos-version, which embeds system.configurationRevision (dirty-tree suffix accounts for the hash diff during local validation)
Pre-commit hook chain green (deadnix, statix, nix-parse, treefmt, gitleaks, managed-files-drift)

Diff overview

 modules/base/custom-packages.nix             | +80
 modules/system76/custom-packages-overlay.nix | -68 / +9   (now 16 lines)
 modules/tpnix/custom-packages-overlay.nix    | -73 / +1   (now 8 lines)

claude · 2026-05-04T21:15:34Z

Claude finished @Bad3r's task in 2m 4s —— View job

Review

The refactor is sound and achieves its stated goal. Pre/post are byte-identical for both hosts (independently verified): the shared overlay defines exactly the union of the pre-refactor attributes minus system76-power, which the system76 host adds via a second overlay. Because the shared overlay doesn't define system76-power, the layering order between the two overlays is irrelevant — prev.system76-power resolves to the same un-overridden derivation it did pre-refactor. Same reasoning holds for librepods, marktext, and wfuzz: none of their prev.* references are redefined upstream in the shared overlay, so their drvPaths are preserved.

All 22 final.callPackage ../../packages/<name> references resolve to existing directories under packages/. Both system76 and tpnix enable i3wm (modules/<host>/window-manager.nix), so hoisting i3-focus-or-launch, i3-scratchpad-show-or-create, and monitor-query into the shared overlay is correct.

The structural eliminations are exactly what the PR #165 review called out — there's no longer a duplicated overlay block where an upstream change can silently land in only one host file. ✓

Suggestions

1. Namespace convention (minor) — modules/base/custom-packages.nix:10

Every other flake.lib value in this repo is grouped by an area namespace:

flake.lib.security.* (modules/security/sops-helpers.nix:9, modules/security/usbguard-lib.nix:3)
flake.lib.nixos.* (modules/meta/nixos-app-helpers.nix:146, modules/lib/r2-runtime.nix:3)
flake.lib.agents.* (modules/agents/mcp.nix:466, modules/agents/skills.nix:276)
flake.lib.meta.inputBranches (modules/meta/input-branches.nix:8)
flake.lib.xdg.desktopFiles (modules/meta/ci.nix:8)

This PR introduces flake.lib.customPackagesOverlay at the root, which is the first un-namespaced entry. The PR description claims it "Mirrors the established flake.lib.<namespace> convention", but that convention actually has a namespace component. Consider flake.lib.overlays.customPackages (best fit if more overlays land later) or flake.lib.nixpkgs.customPackagesOverlay. Cheap to adjust now while there's only one consumer pair.

2. File-name asymmetry (very minor)

The shared module is modules/base/custom-packages.nix while hosts use custom-packages-overlay.nix. Aligning the names (e.g. modules/base/custom-packages-overlay.nix) makes the relationship more discoverable via filename. Optional.

3. Worth noting in the file header

modules/base/custom-packages.nix:1-3 says "used by every host", which is accurate today (only system76/tpnix) but the value is defined, not auto-applied — hosts still have to import it into nixpkgs.overlays. A line like # Hosts opt in by composing this into nixpkgs.overlays. would prevent a future reader from assuming it's globally applied just by being in modules/base/.

Nits

modules/system76/custom-packages-overlay.nix:9 uses _final: for the host-extras overlay; idiomatic and consistent with the prefix convention.
The shared overlay correctly uses _: (no module args needed) — matches modules/security/sops-helpers.nix style.

Test coverage

The PR description lists thorough manual validation (drvPath equality on three representative packages, system.activationScripts.binsh equality, full nix flake check, host eval). For a refactor of this kind that's the right bar; no automated drift guard exists in the repo today and adding one for this single attribute would be overkill.

✅ LGTM aside from the namespace placement nit — the refactor is correct, the equivalence reasoning checks out, and it eliminates a real recurring failure mode.

Read the PR diff and changed files
Review modules/base/custom-packages.nix (new shared overlay)
Review modules/system76/custom-packages-overlay.nix (host composition)
Review modules/tpnix/custom-packages-overlay.nix (host composition)
Verify equivalence with pre-refactor behavior
Check naming/conventions vs. existing flake.lib.<namespace> modules
Post final review
• Branch: refactor/dedup-host-overlays

gemini-code-assist

Code Review

This pull request centralizes custom package definitions into a shared Nixpkgs overlay, reducing duplication across host-specific modules and resolving merge conflict markers in the tpnix configuration. Feedback suggests using the final fixpoint instead of prev for dependencies in package overrides to ensure consistency and allow other overlays to influence the package set.

…rlay Address claude-review nits on PR #166: 1. Namespace: move `flake.lib.customPackagesOverlay` -> `flake.lib.overlays.customPackages`. Aligns with the existing `flake.lib.<area>.<name>` convention used by `flake.lib.security.*`, `flake.lib.nixos.*`, `flake.lib.agents.*`, `flake.lib.meta.inputBranches`, `flake.lib.xdg.desktopFiles`. Leaves room for additional overlays under the same namespace. 2. Filename: rename `modules/base/custom-packages.nix` ->`modules/base/custom-packages-overlay.nix`. Mirrors the host-side filenames so the relationship is discoverable by name alone. 3. Header: clarify that the overlay is *defined* (not auto-applied) and that hosts opt in by composing it into `nixpkgs.overlays`. Prevents future readers from assuming `modules/base/` content is globally applied. Verified equivalent: `nix flake check --accept-flake-config --no-build` -> all checks passed; both host toplevel evals succeed.

Bad3r · 2026-05-04T21:21:15Z

Reply to automated review (run 25343941782)

All three suggestions adopted in `36655dc5`:

Adopted

#	Suggestion	Action
1	Namespace `flake.lib.customPackagesOverlay` is the only un-namespaced entry; group under an area	Moved to `flake.lib.overlays.customPackages`. Matches `flake.lib.security.`, `flake.lib.nixos.`, `flake.lib.agents.`, `flake.lib.meta.`, `flake.lib.xdg.*`. Leaves the `overlays` namespace open for any future shared overlays.
2	File-name asymmetry between shared and host files	Renamed `modules/base/custom-packages.nix` -> `modules/base/custom-packages-overlay.nix`. The shared and host files now share a base name, making the producer/consumer relationship discoverable by filename.
3	Header comment should clarify the overlay is defined, not auto-applied	Rewrote the file header: it now explicitly states the module only defines the overlay, points readers at the host-side composition pattern, and documents the in-tree-vs-override resolution rules.

Verification

`nix flake check --accept-flake-config --no-build`: `all checks passed!`
`nix eval .#nixosConfigurations.system76.config.system.build.toplevel.outPath`: succeeds
`nix eval .#nixosConfigurations.tpnix.config.system.build.toplevel.outPath`: succeeds
Pre-commit hook chain: green

The "Nits" section (use of `final:` and `:` patterns) and the "Test coverage" assessment required no action -- those are observations of existing correct style. No items rejected.

claude · 2026-05-04T21:21:26Z

Claude finished @Bad3r's task in 2m 16s —— View job

Review

The latest commit (36655dc) cleanly applies all three suggestions from the prior review. Independent verification:

Suggestion	Status	Evidence
Move under an area namespace	✓	`modules/base/custom-packages-overlay.nix:12` declares `flake.lib.overlays.customPackages`, matching the pattern of `flake.lib.security`, `flake.lib.agents`, `flake.lib.nixos`, `flake.lib.meta`, `flake.lib.xdg`
Filename alignment	✓	Renamed to `modules/base/custom-packages-overlay.nix`; producer/consumer share base name
Header clarifies "defined, not auto-applied"	✓	`custom-packages-overlay.nix:1-10` now spells out the opt-in composition contract and documents the `final.callPackage` vs. `prev.<pkg>.overrideAttrs` resolution split

Both host call-sites (modules/system76/custom-packages-overlay.nix:5, modules/tpnix/custom-packages-overlay.nix:5) reference the new attribute path correctly, and the system76 host-extras overlay still layers on top with _final: prev: as before.

On the gemini-code-assist suggestion (`final` vs `prev` in `librepods` / `marktext`)

The suggestion is technically correct as a Nixpkgs idiom: dependencies in an overrideAttrs that participate in the fixpoint should generally come from final so that later overlays can still influence them.

Practical impact in this repo, today: zero. I checked every other module that sets nixpkgs.overlays for the system76 and tpnix hosts:

modules/system76/custom-packages-overlay.nix:6-17 — only overrides system76-power
modules/system76/firmware-manager-fix.nix, modules/tpnix/firmware-manager-fix.nix — only override firmware-manager
modules/{system76,tpnix}/cachyos-kernel.nix — kernel-only
modules/base/envfs.nix, modules/apps/{claude-desktop,doom-emacs,burpsuitepro,google-chrome,cewl,sss-pass-gpg-bootstrap}.nix — none of them rebind openssl, qt6.*, libpulseaudio, node-gyp, or fetchFromGitHub

So final.openssl and prev.openssl resolve to the same derivation in the current closure — no drvPath movement either way. That matches the PR's "byte-identical pre/post" claim.

That said, adopting the suggestion is still worthwhile because:

It's defensive — if someone later adds e.g. an openssl patch overlay for a CVE, librepods would automatically pick it up instead of silently linking against an un-patched copy.
It's consistent with the rest of the same file: wfuzz (line 41-43) and the in-tree callPackage lines all use final. The only prev.* references that aren't prev.<pkg>.overrideAttrs (the legitimate uses) are the dependency lookups inside the override bodies — those are exactly what gemini flagged.
The pre-refactor host files used prev here too, so this is a pre-existing inconsistency, not regression of this PR — but it's cheap to fix while the file is being touched.

The change preserves byte-identical drvPaths today and only differs from the current behavior in the (currently hypothetical) case where another overlay rebinds those deps — which is the exact scenario the idiom is designed for.

Suggested follow-up (small, in-scope for this PR)

In modules/base/custom-packages-overlay.nix:

librepods = prev.librepods.overrideAttrs (_old: rec {
  version = "0.2.5";
  src = final.fetchFromGitHub {           # was: prev.fetchFromGitHub
    owner = "kavishdevar";
    repo = "librepods";
    tag = "v${version}";
    hash = "sha256-6l1WjwjDbv5e3tDaWo9+XSEjr9ge/hKysIkeUqyiO4U=";
  };
  buildInputs = [
    final.libpulseaudio                   # was: prev.*
    final.openssl
    final.qt6.qtbase
    final.qt6.qtconnectivity
    final.qt6.qtdeclarative
    final.qt6.qttools
  ];
});

marktext = prev.marktext.overrideAttrs (old: {
  nativeBuildInputs = (old.nativeBuildInputs or [ ]) ++ [
    final."node-gyp"                      # was: prev."node-gyp"
  ];
  npm_config_node_gyp = "${final."node-gyp"}/bin/node-gyp";
  NODE_GYP = "${final."node-gyp"}/bin/node-gyp";
});

Leave the prev.<pkg>.overrideAttrs calls themselves on prev — that part is correct (you want to extend the unmodified upstream attrs, not recurse through the fixpoint).

Fix this →

Nits / non-blocking

modules/base/custom-packages-overlay.nix:78 — monitor-query = import ../../lib/shell/monitor-query.nix { inherit (final) writeText; }; is fine; uses final correctly.
The trailing comment on line 79 (# toggle-logseq is created in modules/apps/i3wm/config.nix) is helpful — it explains why a sibling i3 helper is not here.

Verdict

LGTM. The structural refactor is correct, the new commit applies the prior namespace/filename/header guidance faithfully, and the gemini suggestion is a small, optional polish that wouldn't change current outputs but would make the overlay more robust to future overlay composition. Whether to fold that into this PR or punt to a follow-up is the author's call.

Read the PR diff and changed files
Verify 36655dc correctly addresses prior namespace / filename / header suggestions
Cross-check the final-vs-prev claim against every other overlay in the host closures
Post review
• Branch: refactor/dedup-host-overlays

Adopt the gemini-code-assist suggestion on PR #166 (lines 63 / 73 of the shared overlay): switch dependency lookups inside the `librepods` and `marktext` `overrideAttrs` bodies from `prev.*` to `final.*`. The `prev.<pkg>.overrideAttrs` calls themselves stay on `prev` (they need to extend the unmodified upstream attrs), but the deps consumed inside the override bodies (`fetchFromGitHub`, `libpulseaudio`, `openssl`, `qt6.*`, `node-gyp`) now go through `final`. This is the standard Nixpkgs idiom: it lets a later overlay -- e.g. an `openssl` CVE patch overlay -- propagate into the librepods build instead of being silently bypassed. No `nixpkgs.overlays` consumer in either host closure currently rebinds any of these dependencies, so drvPaths are byte-identical pre and post: - `librepods.drvPath`: `/nix/store/4sjfa72q2kvpvf5ywm0168g1p370bynf-librepods-0.2.5.drv` - `marktext.drvPath`: `/nix/store/sw5869iqqarh41bwjlqhja5yl1yj6vnh-marktext-0.17.0-unstable-2025-11-19.drv` The change is purely defensive against future overlay composition, and brings these two overrides into alignment with the rest of the file (`wfuzz`, `brave-origin`, all in-tree `callPackage` lines, `monitor-query`) which already use `final`. Comments updated to record the rationale. Validated with `nix flake check --accept-flake-config --no-build` (all checks passed) and pre-commit hook chain green.

claude · 2026-05-04T21:26:44Z

Claude finished @Bad3r's task in 1m 33s —— View job

Review

Read latest commit (b4e24fc) and verify final vs prev adjustments
Re-verify shared overlay attribute set vs. host extras
Cross-check that no other overlay in either host closure rebinds the deps now sourced from final
Confirm both host call-sites still resolve to flake.lib.overlays.customPackages
Post final review

Latest push (b4e24fc) cleanly applies the gemini-code-assist final-vs-prev suggestion. Independent verification:

Change	Status	Evidence
`librepods.src` deps via `final`	✓	`modules/base/custom-packages-overlay.nix:51` `final.fetchFromGitHub`
`librepods.buildInputs` deps via `final`	✓	`custom-packages-overlay.nix:57-64` (`libpulseaudio`, `openssl`, `qt6.{qtbase,qtconnectivity,qtdeclarative,qttools}`)
`marktext` `node-gyp` deps via `final`	✓	`custom-packages-overlay.nix:71-75` (nativeBuildInputs + both env vars)
`prev.<pkg>.overrideAttrs` host-side call kept on `prev`	✓	Lines 49 / 70 — correct, you want to extend the unmodified upstream attrs, not recurse through the fixpoint
Rationale comment added	✓	Lines 47-48 (librepods) and 67-69 (marktext) explain the "later overlay can patch deps" reasoning
No breakage in host call-sites	✓	`modules/system76/custom-packages-overlay.nix:5` and `modules/tpnix/custom-packages-overlay.nix:5` still reference `config.flake.lib.overlays.customPackages`

drvPath equivalence sanity check

The commit message lists byte-identical drvPaths pre/post for both librepods and marktext. I cross-checked every other nixpkgs.overlays consumer in both host closures — none rebind openssl, libpulseaudio, qt6.*, node-gyp, fetchFromGitHub, or rustPlatform.*:

modules/{system76,tpnix}/firmware-manager-fix.nix — firmware-manager only
modules/{system76,tpnix}/cachyos-kernel.nix — kernel only
modules/system76/custom-packages-overlay.nix — system76-power only
modules/base/envfs.nix — envfs only
modules/apps/{doom-emacs,burpsuitepro,google-chrome,cewl,sss-pass-gpg-bootstrap,claude-desktop}.nix — none rebind the listed deps (claude-desktop's openssl reference is a buildFHSEnv consumer, not an overlay rebind)

So the "byte-identical pre/post" claim holds and the change is purely defensive against future overlay composition. ✓

Style consistency

The override pattern in this file now matches itself end-to-end: wfuzz, brave-origin, all final.callPackage lines, monitor-query, librepods, marktext all use final for dep lookups. The only prev.* references that remain are the legitimate prev.<pkg>.overrideAttrs entry points, which is exactly the right shape.

Out-of-scope follow-up (do NOT fold into this PR)

While verifying, I noticed modules/base/envfs.nix:13-34 uses the same prev.fetchFromGitHub / prev.rustPlatform.fetchCargoVendor pattern that was just fixed here. It's loaded into both host closures via flake.nixosModules.base, so the same defensive reasoning applies. Worth a separate small PR (refactor(base): use final for dep lookups in envfs overlay override) — but it's outside the stated scope of this PR (which targets the shared customPackages overlay only), and folding it in would muddy the diff. Leaving it as a noted follow-up.

Nits (non-blocking)

custom-packages-overlay.nix:1-10 — header is excellent now: explicitly states "defines, not auto-applies", points readers at the host composition example, and documents the final.callPackage vs prev.<pkg>.overrideAttrs resolution split. Matches the level of clarity used elsewhere in modules/base/.
custom-packages-overlay.nix:82 — the # toggle-logseq is created in modules/apps/i3wm/config.nix (needs config.gui.scratchpad) comment is helpful negative-space documentation; preserves the prior context for why a sibling i3 helper isn't here.

Verdict

LGTM. The structural refactor is correct, naming follows the repo's flake.lib.<area>.* convention, the shared/host file naming is symmetric, and the final-vs-prev polish brings the override patterns in line with the rest of the file while staying byte-identical against the current overlay set. Ready to merge.

Optional follow-up: extend the same final-for-deps treatment to modules/base/envfs.nix in a separate PR.
• Branch: refactor/dedup-host-overlays

Bad3r · 2026-05-04T21:26:46Z

Reply to second-round review (run 25344192340)

`b4e24fc8` adopts the gemini-code-assist suggestions on `modules/base/custom-packages-overlay.nix` lines 63 (librepods) and 73 (marktext). Both review threads now resolved.

Adopted (gemini-code-assist, also seconded by claude-review)

File	Change	Why
`modules/base/custom-packages-overlay.nix` (librepods)	`prev.fetchFromGitHub`, `prev.libpulseaudio`, `prev.openssl`, `prev.qt6.` -> `final.`	Standard Nixpkgs idiom. Lets a later overlay (e.g. an `openssl` CVE patch overlay) propagate into this build instead of being silently bypassed.
`modules/base/custom-packages-overlay.nix` (marktext)	`prev."node-gyp"` -> `final."node-gyp"` (in `nativeBuildInputs`, `npm_config_node_gyp`, `NODE_GYP`)	Same reasoning. Brings the override in line with `wfuzz`, `brave-origin`, and the in-tree `callPackage` lines that already use `final`.
Comments	Updated above each override to record the rationale	So future maintainers know the fixpoint convention is intentional, not coincidental.

The `prev..overrideAttrs` calls themselves stay on `prev` -- that part was already correct (the override extends the unmodified upstream attrs).

Acknowledged (no action needed)

Item	Why no change
`monitor-query` already uses `final` (`{ inherit (final) writeText; }`)	Already correct.
`# toggle-logseq is created in modules/apps/i3wm/config.nix` comment	Already present and intentional -- explains why the i3 sibling is not in this overlay.
Idiomatic `final:` / `:` patterns	Already correct, flagged only as a positive observation.

Verification

Check	Result
`nix flake check --accept-flake-config --no-build`	`all checks passed!`
`librepods.drvPath` pre vs. post	Byte-identical: `/nix/store/4sjfa72q2kvpvf5ywm0168g1p370bynf-librepods-0.2.5.drv`
`marktext.drvPath` pre vs. post	Byte-identical: `/nix/store/sw5869iqqarh41bwjlqhja5yl1yj6vnh-marktext-0.17.0-unstable-2025-11-19.drv`
Pre-commit hook chain	Green (deadnix, statix, nix-parse, treefmt, gitleaks, managed-files-drift)

The byte-identical drvPaths match claude-review's prediction: no overlay in either host closure currently rebinds the affected dependencies, so the change is purely defensive against future overlay composition.

Bad3r · 2026-05-04T21:29:21Z

Reply to third-round review (run 25344427322)

Thanks for the LGTM. Closing the loop on the noted out-of-scope follow-up.

Acknowledged (no change in this PR)

The observation about `modules/base/envfs.nix:13-34` reusing the same `prev.fetchFromGitHub` / `prev.rustPlatform.fetchCargoVendor` pattern is correct, and the same defensive reasoning applies. Per the reviewer's own framing ("do NOT fold into this PR" / "outside the stated scope") and to keep this diff minimal, that change is being handled in a separate PR rather than extending the scope here.

Tracking PR will be opened immediately and linked back.

Verified non-actionable

Item	Why no action
Header clarity (`custom-packages-overlay.nix:1-10`)	Already at the level of the rest of `modules/base/`.
Negative-space comment about `toggle-logseq`	Helpful documentation; intentionally preserved.
End-to-end `final`-for-deps consistency in this file	Confirmed: every dep lookup goes through `final`; only the legitimate `prev..overrideAttrs` entry points remain on `prev`.

Net status

All review threads on this PR are now resolved. Verdict from the third review iteration: ✅ LGTM, ready to merge.

…167) * fix(tpnix): resolve unresolved merge conflict markers in overlay PR #160 squash-merged a state where `Merge branch 'main' into feat/csec-tools` (22973ff) left literal conflict markers in `modules/tpnix/custom-packages-overlay.nix:28,37,57`. The result is not valid Nix syntax: `nix-instantiate --parse` rejects the file, the `tpnix` host build is broken, and both the `wfuzz` (feat/csec-tools) and `librepods` (main) overrides are mutually exclusive in that state. Resolution mirrors the system76 sibling at `modules/system76/custom-packages-overlay.nix`, which was resolved correctly during the same merge: keep both blocks, since the wfuzz and librepods overrides are independent. Verified with `nix-instantiate --parse` and `nix flake check --accept-flake-config --no-build`. Reported by claude-code review on PR #160 (run 25340703466). * refactor(base): use final for dep lookups in envfs overlay override `modules/base/envfs.nix:13-34` pinned `envfs` to 1.2.0 with dependency lookups (`prev.fetchFromGitHub`, `prev.rustPlatform.fetchCargoVendor`) sourced from `prev`. Switch those to `final.*` so any later overlay -- e.g. a future `rustPlatform`/`fetchurl` patch overlay -- propagates into the override instead of silently linking against the un-modified upstream copy. The `prev.envfs.overrideAttrs` entry point itself stays on `prev` (it must extend the unmodified upstream attrs, not recurse through the fixpoint). This is the same shape applied to `librepods` and `marktext` in #166. Comment updated above the overlay to record the rationale. `envfs.drvPath` is byte-identical pre/post: `/nix/store/f6npid91mdh5dpyxxbrn5wyzcn9dxf11-envfs-1.2.0.drv` -- no overlay in either host closure currently rebinds `fetchFromGitHub` or `rustPlatform.fetchCargoVendor`, so the change is purely defensive against future overlay composition. Validated with `nix flake check --accept-flake-config --no-build` (all checks passed). Suggested as an out-of-scope follow-up by claude-review on PR #166 (run 25344427322). * docs(base): correct envfs overlay comment to not imply overlay order The comment above the `envfs` overlay said "any later overlay that rebinds them feeds into this build". That implied registration order mattered. Nixpkgs overlays are evaluated as a single fixed point, so any overlay (regardless of registration order) that rebinds the referenced deps is observed via `final`. Reword to "any other overlay" and add a parenthetical noting that ordering is irrelevant here. Suggested by claude-review on PR #167 (run 25344637086). * refactor(base): dedupe cargoDeps src in envfs overlay override The `cargoDeps.src` argument inside the `envfs` override was an exact duplicate of the outer `src` (same `owner` / `repo` / `rev` / `hash`). Bind once via a `let` block inside the `overrideAttrs` body and reuse via `inherit src;` at both call sites. The four-line drift risk -- a future bump touching only one of the two `src` literals -- is now structurally impossible. `envfs.drvPath` is byte-identical pre/post: `/nix/store/f6npid91mdh5dpyxxbrn5wyzcn9dxf11-envfs-1.2.0.drv`. Suggested by gemini-code-assist on PR #167 (line 30 of `envfs.nix`) and seconded by claude-review (run 25344831509).

Bad3r · 2026-05-05T03:49:06Z

Reply: round 4 review feedback

Pulled and re-validated every reviewer suggestion across the three claude-bot reviews and the two gemini-code-assist inline threads. The only unaddressed item remaining was the out-of-scope follow-up flagged by the third claude review (modules/base/envfs.nix:13-34 reusing the prev.fetchFromGitHub / prev.rustPlatform.fetchCargoVendor pattern). That has now been folded in rather than deferred to a separate PR, since the change is small, structurally identical to the customPackages overlay polish in b4e24fc8, and shares the same defensive rationale.

Adopted

File	Change	Why
`modules/base/envfs.nix`	Overlay arg renamed `_final:` -> `final:`; `prev.fetchFromGitHub`, `prev.rustPlatform.fetchCargoVendor` -> `final.*` (3 sites). `prev.envfs.overrideAttrs` stays on `prev`.	Standard Nixpkgs idiom: dependency lookups inside an override body should resolve through `final` so a later overlay (e.g. a `rustPlatform` toolchain swap or upstream patch) feeds into this build instead of being silently bypassed. Matches the discipline now in place across `modules/base/custom-packages-overlay.nix`.
`modules/base/envfs.nix` (header)	Added a four-line comment block above the overlay documenting the `final`/`prev` resolution split.	Future-maintainer documentation: states that the fixpoint discipline is intentional and explains why `prev.envfs.overrideAttrs` is the one place still on `prev`.

Rationale for folding into this PR (rather than a tracking PR)

The third review explicitly recommended punting this to a separate PR to keep the diff focused. Reconsidered because:

The change is mechanical and tiny (9 insertions, 4 deletions in one file).
It applies the exact same idiom in the exact same way as the customPackages overlay change in b4e24fc8 -- splitting them across two PRs would fragment the same conceptual cleanup.
drvPath is byte-identical pre/post, so the merge surface remains zero-risk.

If the reviewer prefers the split, this commit can be cherry-picked back out and reissued as a follow-up PR -- but the unified shape reads more cleanly as one logical "use final for dep lookups in shared overlay overrides" pass across modules/base/.

Validation

Check	Result
`nix flake check --accept-flake-config --no-build`	`all checks passed!`
`nixosConfigurations.tpnix.pkgs.envfs.drvPath` pre vs. post	Byte-identical: `/nix/store/f6npid91mdh5dpyxxbrn5wyzcn9dxf11-envfs-1.2.0.drv`
Pre-commit hook chain (deadnix, statix, nix-parse, treefmt, typos)	Green

Status of prior review feedback (recap)

Reviewer	Item	Status
claude-bot (round 1)	Namespace under `flake.lib.overlays.*`	Adopted in `36655dc5`
claude-bot (round 1)	File-name symmetry (`custom-packages-overlay.nix`)	Adopted in `36655dc5`
claude-bot (round 1)	Header clarifies "defines, not auto-applies"	Adopted in `36655dc5`
gemini-code-assist (inline, line 65)	`librepods` deps via `final`	Adopted in `b4e24fc8`; thread resolved
gemini-code-assist (inline, line 76)	`marktext` deps via `final`	Adopted in `b4e24fc8`; thread resolved
claude-bot (round 3)	Out-of-scope envfs follow-up	Adopted in this commit

No items rejected. No outstanding suggestions remain across any reviewer thread.

Bad3r · 2026-05-05T03:54:03Z

Correction to the previous reply

The prior reply (#issuecomment-4376405223) is partially incorrect. PR #167 (refactor(base): use final for dep lookups in envfs overlay override) was opened, reviewed, and merged into main at f6bc417f (2026-05-05T03:46:12Z) before this PR's branch was checked. That PR already applied the same final.* for deps change to modules/base/envfs.nix -- and went one step further by deduping cargoDeps.src via a let binding (suggested by gemini-code-assist on PR #167, line 30).

8857e567 on this branch therefore:

Duplicates work that is already in main.
Misses the additional let src = ... dedup that landed as the second commit in refactor(base): use final for dep lookups in envfs overlay override #167.
Will conflict with main at merge time.

Net status of PR 166 review feedback

All reviewer suggestions across both gemini-code-assist threads and all three claude-bot reviews are now addressed -- the customPackages overlay polish in this PR plus the envfs polish in #167.

Next step

Rebasing the branch on main to:

Drop d0bca282 (already merged independently as fix(tpnix): resolve unresolved merge conflict markers in overlay #165 -> 7c40ce19).
Drop 8857e567 (superseded by refactor(base): use final for dep lookups in envfs overlay override #167's merged commits).
Leave only the customPackages overlay refactor on this branch, on top of a clean base.

Will follow up once the rebase completes.

`modules/system76/custom-packages-overlay.nix` and `modules/tpnix/custom-packages-overlay.nix` shared ~95% identical content (the entire `final: prev:` body apart from the `system76-power` SCSI patch). That duplication is what caused the merge in PR #160 to leave conflict markers in only one of the two files when an upstream change touched the shared region. Extract the common overlay into `modules/base/custom-packages.nix` as `config.flake.lib.customPackagesOverlay`, mirroring the established pattern used by `flake.lib.security.*` (see `modules/security/`). Hosts now compose the shared overlay with any host-specific extras: - `system76`: prepends the shared overlay, then layers a small overlay that adds the `system76-power` SCSI patch (hardware-specific). - `tpnix`: just consumes the shared overlay. Verified semantically equivalent to pre-refactor by comparing derivation paths for `brave-origin`, `system76-power`, `librepods` (all byte-identical), `system.activationScripts.binsh` (identical), and `environment.systemPackages` (only difference is `nixos-version`, which embeds `system.configurationRevision`; the dirty-tree suffix during validation accounts for the hash diff). Validated with: - `nix flake check --accept-flake-config --no-build`: all checks passed - `nix eval .#nixosConfigurations.{system76,tpnix}.config.system.build.toplevel.outPath`: both succeed - pre-commit hooks (deadnix, statix, nix-parse, treefmt, gitleaks, managed-files-drift): all pass Addresses the non-blocking observation from claude-review on PR #165 (run 25341374903): "lift the common attrset into a single shared `customPackages` overlay imported by both hosts -- would prevent the next merge from re-creating exactly this class of conflict."

…rlay Address claude-review nits on PR #166: 1. Namespace: move `flake.lib.customPackagesOverlay` -> `flake.lib.overlays.customPackages`. Aligns with the existing `flake.lib.<area>.<name>` convention used by `flake.lib.security.*`, `flake.lib.nixos.*`, `flake.lib.agents.*`, `flake.lib.meta.inputBranches`, `flake.lib.xdg.desktopFiles`. Leaves room for additional overlays under the same namespace. 2. Filename: rename `modules/base/custom-packages.nix` ->`modules/base/custom-packages-overlay.nix`. Mirrors the host-side filenames so the relationship is discoverable by name alone. 3. Header: clarify that the overlay is *defined* (not auto-applied) and that hosts opt in by composing it into `nixpkgs.overlays`. Prevents future readers from assuming `modules/base/` content is globally applied. Verified equivalent: `nix flake check --accept-flake-config --no-build` -> all checks passed; both host toplevel evals succeed.

Adopt the gemini-code-assist suggestion on PR #166 (lines 63 / 73 of the shared overlay): switch dependency lookups inside the `librepods` and `marktext` `overrideAttrs` bodies from `prev.*` to `final.*`. The `prev.<pkg>.overrideAttrs` calls themselves stay on `prev` (they need to extend the unmodified upstream attrs), but the deps consumed inside the override bodies (`fetchFromGitHub`, `libpulseaudio`, `openssl`, `qt6.*`, `node-gyp`) now go through `final`. This is the standard Nixpkgs idiom: it lets a later overlay -- e.g. an `openssl` CVE patch overlay -- propagate into the librepods build instead of being silently bypassed. No `nixpkgs.overlays` consumer in either host closure currently rebinds any of these dependencies, so drvPaths are byte-identical pre and post: - `librepods.drvPath`: `/nix/store/4sjfa72q2kvpvf5ywm0168g1p370bynf-librepods-0.2.5.drv` - `marktext.drvPath`: `/nix/store/sw5869iqqarh41bwjlqhja5yl1yj6vnh-marktext-0.17.0-unstable-2025-11-19.drv` The change is purely defensive against future overlay composition, and brings these two overrides into alignment with the rest of the file (`wfuzz`, `brave-origin`, all in-tree `callPackage` lines, `monitor-query`) which already use `final`. Comments updated to record the rationale. Validated with `nix flake check --accept-flake-config --no-build` (all checks passed) and pre-commit hook chain green.

claude · 2026-05-05T03:57:49Z

Claude finished @Bad3r's task in 1m 57s —— View job

Status: PR already merged

This PR was squash-merged into main as 4d7cb2f at 2026-05-05T03:57:53Z, ~4 seconds before this synchronize event fired and triggered me. The 3 commits on this branch (51833e4, 7ddd940, 6d1a8d1) are exactly what landed; git diff 6d1a8d1 4d7cb2f -- modules/base/custom-packages-overlay.nix modules/{system76,tpnix}/custom-packages-overlay.nix is empty.

The rebase concern raised in the prior comment (envfs duplication via 8857e567 / d0bca282) didn't actually affect the merge — neither commit was present on the branch tip when it was squash-merged. git log origin/main..HEAD showed only the three intended commits, and squash-merging collapsed them into a single clean commit on main. No corrective action needed.

Retrospective review of merged content

For posterity, since the review was requested.

What landed (`4d7cb2f`)

File	Δ	Role
`modules/base/custom-packages-overlay.nix`	+84 (new)	Defines `flake.lib.overlays.customPackages`
`modules/system76/custom-packages-overlay.nix`	+4 / -70	Composes shared overlay + `system76-power` SCSI patch
`modules/tpnix/custom-packages-overlay.nix`	+3 / -70	Just consumes the shared overlay

Net: +91 / -140 across 3 files. Eliminates ~95% identical duplication that was the structural cause of the PR #165 conflict-marker class of bug.

Verdict: ✅ Correctly merged

Namespacing — flake.lib.overlays.customPackages (custom-packages-overlay.nix:12) follows the repo convention (flake.lib.security.*, flake.lib.agents.*, flake.lib.nixos.*, flake.lib.meta.*, flake.lib.xdg.*) and reserves the overlays namespace for future shared overlays.
Filename symmetry — shared and host files all named custom-packages-overlay.nix; producer/consumer relationship is discoverable.
Header documentation (custom-packages-overlay.nix:1-10) — explicitly states the module defines the overlay (not auto-applied), points readers at a host composition example, and documents the final.callPackage (in-tree) vs. prev.<pkg>.overrideAttrs (upstream-patch) split. Good level of clarity for a modules/base/ shared library.
Fixpoint discipline — every dep lookup inside an override body uses final (lines 51, 57–63, 72, 74, 75); only the legitimate prev.<pkg>.overrideAttrs entry points (lines 49, 70) use prev. Matches Nixpkgs idiom and stays robust against future overlay composition.
Composition order — system76 correctly prepends the shared overlay then layers the system76-power patch (modules/system76/custom-packages-overlay.nix:4-17); since the shared overlay doesn't define system76-power, layering order is irrelevant for that attribute, so drvPaths are preserved.
Negative-space comment preserved (line 82): documents why toggle-logseq lives in modules/apps/i3wm/config.nix rather than here.

Items worth keeping in mind for future overlay work

These aren't issues with the merged code — they're observations from this review thread that may apply elsewhere:

The final vs prev discipline applied here is now also in place for modules/base/envfs.nix (via refactor(base): use final for dep lookups in envfs overlay override #167). Any new overlay that overrides prev.<pkg> should follow the same shape: keep prev.<pkg>.overrideAttrs on prev, but resolve dep lookups (fetchFromGitHub, buildInputs, nativeBuildInputs, toolchain refs) through final.
The flake.lib.overlays.* namespace is now established. If a future shared overlay emerges (e.g. a flake.lib.overlays.csec for cybersec tooling), this PR's pattern is the template.

Todos

Sync with origin/main and inspect rebase state
Read the current PR diff vs origin/main
Review modules/base/custom-packages-overlay.nix (shared overlay)
Review host call-sites (system76, tpnix)
Cross-check final vs prev discipline and overlay composition
Verify the rebase concern from the latest comment (8857e56 / d0bca28 — not on branch tip; no issue)
Post review

No file changes were made (PR is already merged; nothing to push).
• Merged commit: 4d7cb2f • Branch: refactor/dedup-host-overlays

github-project-automation Bot added this to ❄️ NixOS May 4, 2026

github-project-automation Bot moved this to Backlog in ❄️ NixOS May 4, 2026

gemini-code-assist Bot reviewed May 4, 2026

View reviewed changes

Comment thread modules/base/custom-packages-overlay.nix

Comment thread modules/base/custom-packages-overlay.nix

Bad3r mentioned this pull request May 4, 2026

refactor(base): use final for dep lookups in envfs overlay override #167

Merged

3 tasks

Bad3r added 3 commits May 5, 2026 06:56

Bad3r force-pushed the refactor/dedup-host-overlays branch from 8857e56 to 6d1a8d1 Compare May 5, 2026 03:57

Bad3r merged commit 4d7cb2f into main May 5, 2026
1 check passed

Bad3r deleted the refactor/dedup-host-overlays branch May 5, 2026 03:57

github-project-automation Bot moved this from Backlog to Done in ❄️ NixOS May 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(modules): extract shared customPackages overlay into flake.lib#166

refactor(modules): extract shared customPackages overlay into flake.lib#166
Bad3r merged 3 commits intomainfrom
refactor/dedup-host-overlays

Bad3r commented May 4, 2026

Uh oh!

claude Bot commented May 4, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Bad3r commented May 4, 2026

Uh oh!

claude Bot commented May 4, 2026 •

edited

Loading

Uh oh!

claude Bot commented May 4, 2026 •

edited

Loading

Uh oh!

Bad3r commented May 4, 2026

Uh oh!

Bad3r commented May 4, 2026

Uh oh!

Bad3r commented May 5, 2026

Uh oh!

Bad3r commented May 5, 2026

Uh oh!

claude Bot commented May 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Bad3r commented May 4, 2026

Summary

Test plan

Diff overview

Uh oh!

claude Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Suggestions

Nits

Test coverage

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Bad3r commented May 4, 2026

Reply to automated review (run 25343941782)

Adopted

Verification

Uh oh!

claude Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

On the gemini-code-assist suggestion (final vs prev in librepods / marktext)

Suggested follow-up (small, in-scope for this PR)

Nits / non-blocking

Verdict

Uh oh!

claude Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

drvPath equivalence sanity check

Style consistency

Out-of-scope follow-up (do NOT fold into this PR)

Nits (non-blocking)

Verdict

Uh oh!

Bad3r commented May 4, 2026

Reply to second-round review (run 25344192340)

Adopted (gemini-code-assist, also seconded by claude-review)

Acknowledged (no action needed)

Verification

Uh oh!

Bad3r commented May 4, 2026

Reply to third-round review (run 25344427322)

Acknowledged (no change in this PR)

Verified non-actionable

Net status

Uh oh!

Bad3r commented May 5, 2026

Reply: round 4 review feedback

Adopted

Rationale for folding into this PR (rather than a tracking PR)

Validation

Status of prior review feedback (recap)

Uh oh!

Bad3r commented May 5, 2026

Correction to the previous reply

Net status of PR 166 review feedback

Next step

Uh oh!

claude Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status: PR already merged

Retrospective review of merged content

What landed (4d7cb2f)

Verdict: ✅ Correctly merged

Items worth keeping in mind for future overlay work

Todos

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

claude Bot commented May 4, 2026 •

edited

Loading

claude Bot commented May 4, 2026 •

edited

Loading

On the gemini-code-assist suggestion (`final` vs `prev` in `librepods` / `marktext`)

claude Bot commented May 4, 2026 •

edited

Loading

claude Bot commented May 5, 2026 •

edited

Loading

What landed (`4d7cb2f`)