installable: drop state passing via environment variables by faukah · Pull Request #550 · nix-community/nh

faukah · 2026-02-01T23:29:21Z

Sanity Checking

Add a 👍 reaction to pull requests you find important.

Summary by CodeRabbit

Refactor
- Centralized configuration resolution for all commands, producing consistent defaults and simpler flows.
- Removed scattered environment-variable parsing and in-process env mutations, reducing surprising side effects and making behavior more predictable.
- Adjusted update ordering so updates run after target resolution for more reliable operations.
Bug Fixes
- Eliminated context-specific env parsing that could cause incorrect target selection.

coderabbitai · 2026-02-01T23:29:29Z

Walkthrough

Introduce a public CommandContext enum and Installable::resolve(CommandContext) to centralize context-aware NH_FLAKE resolution; replace scattered NH_FLAKE and NH_CURRENT_COMMAND handling with resolve calls across darwin.rs, home.rs, nixos.rs, and remove env-mutation side effects in interface.rs.

Changes

Cohort / File(s)	Summary
Core Resolution Logic `src/installable.rs`	Add `pub enum CommandContext { Os, Home, Darwin }` and `pub fn resolve(self, context: CommandContext) -> color_eyre::Result<Self>` to resolve `Unspecified` using prioritized env vars per context (context-specific NH_*_FLAKE then NH_FLAKE). Remove NH_CURRENT_COMMAND lookup and add tests for resolve behavior.
OS command integrations `src/nixos.rs`	Replace NH_OS_FLAKE parsing with `Installable::resolve(CommandContext::Os)` across rebuild/activate, build-image, repl, and related flows; remove `get_nh_os_flake_env`. Update control flow to use resolved Installable before toplevel derivation/update steps.
Home command integrations `src/home.rs`	Replace NH_HOME_FLAKE parsing with `Installable::resolve(CommandContext::Home)` in rebuild and repl flows; perform update logic after resolution; preserve Unspecified defaulting behavior via existing defaults.
Darwin command integrations `src/darwin.rs`	Resolve installable via `Installable::resolve(CommandContext::Darwin)` in rebuild and repl flows; remove NH_DARWIN_FLAKE parsing and branches; move update() calls to occur after installable resolution.
CLI / interface cleanup `src/interface.rs`	Remove unsafe `std::env::set_var("NH_CURRENT_COMMAND", ...)` mutations for Os/Home/Darwin dispatch; delegate directly to command-specific `run` methods without mutating environment.
Misc imports / error handling multiple files	Add `installable::CommandContext` imports where needed; remove now-unused NH_*_FLAKE parsing error paths and related eyre usages; small refactors to reflect the new resolution flow.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as CLI (nh)
  participant Interface as Interface (dispatch)
  participant Installable as Installable::resolve
  participant Env as Environment (NH_*_FLAKE)
  participant Command as Command.run

  CLI->>Interface: invoke command
  Interface->>Installable: resolve(context)
  Installable->>Env: read prioritized env vars (context-specific)
  Env-->>Installable: env value or none
  Installable-->>Interface: resolved Installable (or Unspecified)
  Interface->>Command: run with resolved Installable

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

installable: handle permission and I/O errors more gracefully in fallback logic #491: Modifies installable resolution fallback/canonicalization and error handling used by the new context-aware resolve logic.
nixos: cleanup #436: Refactors NixOS installable resolution and removes scattered NH_OS_FLAKE handling; overlaps with OS-path changes here.
installable: handle invalid installable resolution more gracefully #486: Adjusts early resolution of Unspecified and default-finding logic that interacts with the new resolve behavior.

Suggested reviewers

NotAShelf

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: replacing environment variable-based state passing with centralized CommandContext-based resolution across all modules.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch faukah/push-kmyuvunxousq

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

NotAShelf

LGTM overall, leaving a quick comment a few typos that I'd like for you to fix before I'm on my desktop to review properly.

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/installable.rs`:
- Around line 308-368: The code currently consumes NH_*_FLAKE in
Installable::from_arg_matches_mut before Installable::resolve runs, allowing
global env vars to bypass CommandContext-specific precedence; change
from_arg_matches_mut to not read or consume any NH_*_FLAKE env vars (leave the
Installable as Unspecified when no explicit CLI/arg provided) and let
Installable::resolve(context: CommandContext) perform all NH_*_FLAKE
parsing/selection (or, if you prefer, re-run resolve when an env-derived value
is detected) so that NH_OS_FLAKE / NH_HOME_FLAKE / NH_DARWIN_FLAKE take
precedence per CommandContext; also update the resolve() docstring to document
that Ok(Unspecified) is returned when no env/default is present.

🧹 Nitpick comments (1)

src/installable.rs (1)

759-1119: Consider restoring pre‑existing env vars in tests.
These tests mutate NH_*_FLAKE but don’t restore prior values, so a CI/dev environment with those vars set can leak state across tests. A small snapshot/restore guard would make this suite more robust.

coderabbitai

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@src/darwin.rs`:
- Around line 103-109: The call to update() is passing the unresolved CLI
installable (&self.common.installable) which can be Unspecified when
NH_DARWIN_FLAKE/NH_FLAKE is used; change the call to pass the resolved
installable (&installable) instead so update() receives the concrete value
(match the fix used in home.rs and the pattern in nixos.rs), i.e., locate the
block where update(&self.common.installable, ...) is invoked in the darwin
update flow and replace the first argument with &installable.

In `@src/home.rs`:
- Around line 84-90: The call to update(...) is passing the unresolved CLI value
(&self.common.installable) which can be Unspecified; change it to pass the
resolved installable (the toplevel variable computed earlier) so update() sees a
Flake variant — i.e., replace &self.common.installable with &toplevel (or
whatever the resolved installable binding is) in the update(...) invocation
while keeping the same update_args and commit_lock_file arguments.

In `@src/nixos.rs`:
- Around line 650-656: rebuild_and_activate() currently ignores OsRebuildArgs
update flags; mirror the build_only() behavior by invoking update(&toplevel,
self.update_args.update_input.clone(), self.common.passthrough.commit_lock_file)
when self.update_args.update_all || self.update_args.update_input.is_some().
Locate rebuild_and_activate(), add the same conditional update() call (using the
same toplevel, update_args, and commit_lock_file symbols as in build_only()),
and propagate any returned Result/handle the ? operator consistent with the
surrounding function.

faukah force-pushed the faukah/push-kmyuvunxousq branch from 91d190e to e004d20 Compare February 1, 2026 23:30

faukah marked this pull request as ready for review February 1, 2026 23:32

faukah requested a review from NotAShelf February 1, 2026 23:35

NotAShelf requested changes Feb 2, 2026

View reviewed changes

Comment thread src/installable.rs Outdated

Comment thread src/installable.rs Outdated

Comment thread src/installable.rs Outdated

faukah added 2 commits February 2, 2026 10:19

installable: drop state passing via environment variables

335c25e

installable: add tests to resolve installable

c82629a

faukah force-pushed the faukah/push-kmyuvunxousq branch from e004d20 to c82629a Compare February 2, 2026 09:19

coderabbitai Bot reviewed Feb 2, 2026

View reviewed changes

Comment thread src/installable.rs

faukah mentioned this pull request Feb 6, 2026

NH_DARWIN_FLAKE is not used when running nh darwin switch -u #554

Closed

2 tasks

faukah commented Feb 6, 2026

View reviewed changes

Comment thread src/nixos.rs

coderabbitai Bot reviewed Feb 6, 2026

View reviewed changes

Comment thread src/darwin.rs

Comment thread src/home.rs

Comment thread src/nixos.rs

{nixos,home,darwin}: resolve installable before updating flake inputs

7d675da

faukah force-pushed the faukah/push-kmyuvunxousq branch from 88ba3db to 7d675da Compare February 6, 2026 15:07

NotAShelf merged commit 2077983 into master Feb 7, 2026
13 checks passed

NotAShelf deleted the faukah/push-kmyuvunxousq branch February 7, 2026 10:03

faukah mentioned this pull request Feb 15, 2026

home switch --update-input not using NH_HOME_FLAKE for updating specific flake inputs. #564

Closed

2 tasks

coderabbitai Bot mentioned this pull request Feb 15, 2026

installable: resolve Installable in a single location #565

Merged

13 tasks

coderabbitai Bot mentioned this pull request Jun 10, 2026

nh-installable: init #684

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

installable: drop state passing via environment variables#550

installable: drop state passing via environment variables#550
NotAShelf merged 3 commits into
masterfrom
faukah/push-kmyuvunxousq

faukah commented Feb 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Feb 1, 2026 •

edited

Loading

Uh oh!

NotAShelf left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

faukah commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sanity Checking

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

NotAShelf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

faukah commented Feb 1, 2026 •

edited

Loading

coderabbitai Bot commented Feb 1, 2026 •

edited

Loading