Skip to content

fix(home-manager): take the stack from the user's pkgs first - #25

Merged
kleinbem merged 1 commit into
mainfrom
hm-host-pkgs
Nov 11, 2026
Merged

kleinbem merged 1 commit into
mainfrom
hm-host-pkgs

Conversation

@kleinbem

@kleinbem kleinbem commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

The home-manager module took every stack package from cadre's own flake outputs. Those are built on plain nixpkgs without the host's overlays, so the user got a buzz-app that misses the host's fixes and holds. On nixos-nvme that buzz-app depends on nixpkgs' buzz-cli 0.5.28, which fails under the new AWS_LC_SYS_USE_SYSTEM=1, and just dev::apply can't build the system.

The module now prefers pkgs.<name> and falls back to the flake's package when pkgs doesn't have it. The NixOS module already uses pkgs.

Tested: nixos-nvme evaluated with this branch now only contains nix-packages' buzz-cli (0.1.0-unstable-2026-11-07, which links aws-lc) and builds it; the 0.5.28 derivation is gone from the closure. nix fmt -- --fail-on-change and nix flake check --no-build pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Packages now use the host’s available versions when present, with the supplied package retained as a fallback.

The flake's own packages are built on plain nixpkgs, without the host's
overlays, so services.cadre installed a buzz-app that misses the fleet's
fixes and holds. Its nixpkgs buzz-cli 0.5.28 fails on the new
AWS_LC_SYS_USE_SYSTEM=1 and took down the nixos-nvme system build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kleinbem
kleinbem enabled auto-merge (squash) November 11, 2026 10:44
@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Explain Change →

Walkthrough

The Home Manager stack now uses packages from host pkgs when available. It falls back to the supplied package set otherwise.

Changes: Home Manager package selection

Layer / File(s) Summary
Package source selection
modules/home-manager/cadre.nix
For each package name, stack now selects the host’s pkgs attribute when present. Otherwise, it uses the supplied package for the host system.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Possibly related PRs

Merge Risk: Minimal · up to 94ae3

The Home Manager module now takes packages from the user's pkgs first, which is the intended fix for the host-overlay mismatch. The fallback cadre wrapper may still bundle flake-built dependencies, but that behavior predates this change. No new merge-blocking risk is established.

Architecture risk: Low · up to 94ae3

The change affects 1 system.

Changed systems: modules

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — modules (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in modules/home-manager/cadre.nix: stack now resolves each package name from host pkgs when available, and falls back to the corresponding package supplied for the host system; previously, it used the supplied package set directly.
Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: Home Manager now takes the package stack from the user's pkgs first. It is concise and specific.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional comments: 1

modules/home-manager/cadre.nix-16-16: 1

16-16: Major · Functional Correctness · Heavy lift

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Build the fallback cadre wrapper with host packages.

If pkgs.cadre is absent, this selects the flake’s cadre wrapper even when host overrides exist for goose-cli or mesh-llm. The wrapper embeds those dependencies and puts them before the inherited PATH, so cadre still runs the flake versions. Host fixes can therefore remain unused or the old dependencies can still block the build. Construct the fallback wrapper with the selected host dependencies. (raw.githubusercontent.com)

Review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Central YAML; Path: .coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5f12185d-8cbf-4cdc-bd8a-526e9a1201cc
Commits

Reviewing files that changed from the base of the PR and between a31e772 and 94ae3ae.

Files selected for processing: 1

  • modules/home-manager/cadre.nix

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@kleinbem
kleinbem merged commit 68ce719 into main Nov 11, 2026
4 checks passed
@kleinbem
kleinbem deleted the hm-host-pkgs branch November 11, 2026 12:07
kleinbem added a commit to kleinbem/nix-config that referenced this pull request Nov 11, 2026
Bumps `cadre` to kleinbem-dev/cadre@68ce719 (kleinbem-dev/cadre#25).
services.cadre's home-manager module now takes the stack from the host's
pkgs, so nixos-nvme's home-manager `buzz-app` uses nix-packages'
`buzz-cli` (links aws-lc) instead of nixpkgs' 0.5.28, which fails under
`AWS_LC_SYS_USE_SYSTEM=1` and stopped `just dev::apply`.

Tested: nixos-nvme evaluates with this lock and its closure contains
only `buzz-cli-0.1.0-unstable-2026-11-07`; the 0.5.28 derivation is
gone.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
kleinbem added a commit to kleinbem-dev/nix-detour that referenced this pull request Nov 11, 2026
A hold only reaches packages from the package set its overlay is in.
When a target gets a package from another set (another flake's
`packages` output, an `import nixpkgs` in a module), the hold changes
nothing there and nobody notices until the build fails again.

That happened on 2026-11-11: kleinbem/nix-detour-fleet#20 held
`buzz-cli` with the system aws-lc, but nixos-nvme still failed on the
unheld `buzz-cli-0.5.28`, because cadre's home-manager module installed
`buzz-app` from cadre's own flake `packages` (fixed in
kleinbem-dev/cadre#25).

- `lib.watch.unreached { pkgs, names, closure }`: per held name, the
closure derivations with the held package's name (or plain nixpkgs'
name, for a hold on another version) that aren't the held derivation.
Fetch and vendor derivations have other names and don't match.
- `scripts/reach.sh`: runs it on every `nixDetourTargets` target that
evaluates (its `.drv` closure via `nix-store -qR`).
- `hold.sh` / update action: a report section and an `unreached` output.
Reporting only; it doesn't hold or fail anything.

Tested:
- `reach.sh` on kleinbem/nix-config with the old cadre lock: `unreached
buzz-cli nixos-nvme /nix/store/iw2q8i20…-buzz-cli-0.5.28.drv`, the
derivation that failed. With the cadre fix (kleinbem/nix-config#751): no
output. About 2 minutes for nixos-nvme on x86_64-linux.
- New unit tests in `mk-holds` (unheld copy, held only plus its src,
hold on another version); `nix flake check` passes; shellcheck clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **New Features**
* Update reports now identify targets that still include an unheld
version of a held package, and provide these results through the
action’s outputs.
* Reports also identify targets that could not be checked. The “Nothing
fails” message appears only when all reported issue categories are
empty.

* **Documentation**
* Clarified that holds apply only to packages from the package set where
the overlay is applied, and explained why a target may still include
another version of a held package.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant