Skip to content

make shell config customizable in base module#10

Open
0xgleb wants to merge 9 commits into
masterfrom
claude/customizable-shell-config-ff5yd
Open

make shell config customizable in base module#10
0xgleb wants to merge 9 commits into
masterfrom
claude/customizable-shell-config-ff5yd

Conversation

@0xgleb

@0xgleb 0xgleb commented Mar 29, 2026

Copy link
Copy Markdown
Member

Add omnix.base.shell options so consumers can control:

  • defaultShell: choose bash/zsh/nushell with auto program enablement
  • bash.viMode: opt out of hardcoded vi keybindings
  • nushell.manageConfig: choose whether omnix or home-manager owns config
  • shellPackage: read-only derivation for use in users.users..shell

https://claude.ai/code/session_01QYnXiuNMQ9aCbqbu7j7tKH

Summary by CodeRabbit

  • New Features
    • Choose default interactive shell: bash, zsh, or nushell.
    • Zsh and nushell are enabled automatically when selected; nushell added to system shells and packages only when chosen.
    • Optional management of nushell configuration files.
    • Toggleable bash vi editing mode.
    • System exposes which shell package is active.
  • Tests
    • Added CI workflow to run flake checks and validate per-module evaluations.

Add omnix.base.shell options so consumers can control:
- defaultShell: choose bash/zsh/nushell with auto program enablement
- bash.viMode: opt out of hardcoded vi keybindings
- nushell.manageConfig: choose whether omnix or home-manager owns config
- shellPackage: read-only derivation for use in users.users.<name>.shell

https://claude.ai/code/session_01QYnXiuNMQ9aCbqbu7j7tKH
@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@0xgleb has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 15 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e4e568fb-1899-4d00-87c2-bb310ddefb36

📥 Commits

Reviewing files that changed from the base of the PR and between 1b79a6a and 2a4b749.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • flake.nix
  • modules/base.nix
📝 Walkthrough

Walkthrough

Adds configurable shell selection under omnix.base.shell with derived booleans and a read-only shellPackage; conditions shell-specific package, program, and config enabling for bash, zsh, and nushell. Adds a GitHub Actions CI workflow and exposes per-module evaluation checks in flake.nix outputs.

Changes

Cohort / File(s) Summary
Shell Configuration Options
modules/base.nix
Introduced omnix.base.shell options: defaultShell (`"bash"
Continuous Integration workflow
.github/workflows/ci.yml
Added a new CI workflow that runs on pull_request and push to master, performing checkout, Nix install, Nix cache setup, and nix flake check as a flake-check job on ubuntu-latest.
Flake per-system checks
flake.nix
Added an evalModule helper and a checks output exposing per-module isolation evaluations: eval-base, eval-firewall, eval-storage, eval-digitalocean, eval-services, and eval-disko (the latter composes the flake’s nixosModules.disko with disko.nixosModules.disko).
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'make shell config customizable in base module' directly and accurately describes the main change: adding configurable shell selection options to the base module.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/customizable-shell-config-ff5yd

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.

@0xgleb 0xgleb enabled auto-merge March 29, 2026 09:44

@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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@modules/base.nix`:
- Line 97: The current line sets programs.zsh.enable with lib.mkIf which
prevents downstream overrides; change it to use lib.mkDefault so consumers can
override—specifically replace the existing expression using lib.mkIf (and the
isZsh check) with a lib.mkDefault-wrapped value (e.g., programs.zsh.enable =
lib.mkDefault (lib.mkIf isZsh true)) so the default remains conditional on isZsh
but becomes overridable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2f1b2f97-a1df-4a4d-8a31-5a751aae4e5c

📥 Commits

Reviewing files that changed from the base of the PR and between 822144e and 3e74651.

📒 Files selected for processing (1)
  • modules/base.nix
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.nix

📄 CodeRabbit inference engine (AGENTS.md)

**/*.nix: Use lib.mkOption with proper types for all module options
Every module must be independently usable (no implicit dependencies on other omnix modules)
Options must live under the omnix.* namespace
Use lib.mkIf cfg.enable for conditional configuration
Use lib.mkDefault for overridable defaults
Every configurable value must be a module option with a type and description
Prefer lib.types.str over lib.types.path for values that may come from terraform or other dynamic sources
Group related options under a single omnix.<module> prefix
Functions take an attrset of required parameters
Return an attrset of derivations or values from library functions
Use pkgs.writeShellApplication for shell scripts (not raw writeScript)
Include runtimeInputs in shell scripts and never assume tools are on PATH
All shell scripts must use set -eo pipefail
When changing module option interfaces, preserve the old option with lib.mkRenamedOptionModule or lib.mkRemovedOptionModule with a clear migration message

Files:

  • modules/base.nix
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: omnix must never contain dev shell configuration
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Group related options under a single `omnix.<module>` prefix

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Options must live under the `omnix.*` namespace

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `pkgs.writeShellApplication` for shell scripts (not raw `writeScript`)

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Include `runtimeInputs` in shell scripts and never assume tools are on PATH

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkOption` with proper types for all module options

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : When changing module option interfaces, preserve the old option with `lib.mkRenamedOptionModule` or `lib.mkRemovedOptionModule` with a clear migration message

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Every module must be independently usable (no implicit dependencies on other omnix modules)

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : All shell scripts must use `set -eo pipefail`

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkIf cfg.enable` for conditional configuration

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: omnix must never contain dev shell configuration

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkDefault` for overridable defaults

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Every configurable value must be a module option with a type and description

Applied to files:

  • modules/base.nix
🔇 Additional comments (5)
modules/base.nix (5)

3-7: LGTM!

Clean let bindings with derived shell type flags. The pattern of extracting cfg and shellCfg for readability is idiomatic.


28-61: LGTM!

Well-structured shell options with proper types and descriptions. The enum constraint on defaultShell, the read-only shellPackage, and the nested bash/nushell sub-options follow the coding guidelines for grouping related options under omnix.<module> prefix.


89-92: LGTM!

Correct mapping from shell enum to package. Since shellPackage is declared readOnly, the direct assignment is appropriate.


99-115: LGTM!

Good use of lib.optional for conditional nushell package inclusion and lib.mkIf for the shells/etc configuration. The nushell config files are appropriately minimal.


117-118: LGTM!

The activation script for creating the per-service profiles directory is straightforward and idempotent.

Comment thread modules/base.nix Outdated
lib.mkIf prevents downstream overrides; mkDefault lets consumers
set programs.zsh.enable independently of the defaultShell choice.

https://claude.ai/code/session_01QYnXiuNMQ9aCbqbu7j7tKH

@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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@modules/base.nix`:
- Around line 89-93: The shellPackage selection uses pkgs.bash
(omnix.base.shell.shellPackage), which lacks readline for login shells; change
the fallback to use pkgs.bashInteractive when the configured login shell is bash
(check omnix.base.shell.defaultShell == "bash" or add an isBash guard), leaving
pkgs.nushell and pkgs.zsh branches unchanged; update the conditional so the
branch that returns pkgs.bash returns pkgs.bashInteractive when defaultShell is
"bash" (or replace pkgs.bash with pkgs.bashInteractive if this option only
applies to login shells).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2c28d3e3-cc26-4ad5-a393-a4b6e591f6e8

📥 Commits

Reviewing files that changed from the base of the PR and between 3e74651 and f1c808e.

📒 Files selected for processing (1)
  • modules/base.nix
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.nix

📄 CodeRabbit inference engine (AGENTS.md)

**/*.nix: Use lib.mkOption with proper types for all module options
Every module must be independently usable (no implicit dependencies on other omnix modules)
Options must live under the omnix.* namespace
Use lib.mkIf cfg.enable for conditional configuration
Use lib.mkDefault for overridable defaults
Every configurable value must be a module option with a type and description
Prefer lib.types.str over lib.types.path for values that may come from terraform or other dynamic sources
Group related options under a single omnix.<module> prefix
Functions take an attrset of required parameters
Return an attrset of derivations or values from library functions
Use pkgs.writeShellApplication for shell scripts (not raw writeScript)
Include runtimeInputs in shell scripts and never assume tools are on PATH
All shell scripts must use set -eo pipefail
When changing module option interfaces, preserve the old option with lib.mkRenamedOptionModule or lib.mkRemovedOptionModule with a clear migration message

Files:

  • modules/base.nix
🧠 Learnings (13)
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Group related options under a single `omnix.<module>` prefix

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Options must live under the `omnix.*` namespace

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : When changing module option interfaces, preserve the old option with `lib.mkRenamedOptionModule` or `lib.mkRemovedOptionModule` with a clear migration message

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkOption` with proper types for all module options

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `pkgs.writeShellApplication` for shell scripts (not raw `writeScript`)

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Every module must be independently usable (no implicit dependencies on other omnix modules)

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : All shell scripts must use `set -eo pipefail`

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkDefault` for overridable defaults

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkIf cfg.enable` for conditional configuration

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Prefer `lib.types.str` over `lib.types.path` for values that may come from terraform or other dynamic sources

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: omnix must never contain dev shell configuration

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Every configurable value must be a module option with a type and description

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Include `runtimeInputs` in shell scripts and never assume tools are on PATH

Applied to files:

  • modules/base.nix
🔇 Additional comments (2)
modules/base.nix (2)

29-61: Well-structured shell option surface under omnix.base.shell.

The new options are typed, documented, and grouped coherently, which keeps the module interface clean and discoverable.

As per coding guidelines: "Use lib.mkOption with proper types for all module options" and "Group related options under a single omnix.<module> prefix".


97-115: Conditional zsh/nushell wiring looks correct and overridable.

programs.zsh.enable = lib.mkDefault isZsh (Line 97) and conditional nushell config ownership (Line 106 onward) match the intended consumer override model.

As per coding guidelines: "Use lib.mkDefault for overridable defaults".

Comment thread modules/base.nix
claude added 5 commits March 29, 2026 10:00
pkgs.bash lacks readline support; pkgs.bashInteractive includes it,
which is needed for proper line editing in login shells.

https://claude.ai/code/session_01QYnXiuNMQ9aCbqbu7j7tKH
Add checks output to flake.nix that evaluates each NixOS module in
isolation, catching evaluation errors before merge. Add GitHub Actions
workflow that runs nix flake check on PRs and pushes to master.

https://claude.ai/code/session_01QYnXiuNMQ9aCbqbu7j7tKH
Switch from cachix/install-nix-action to DeterminateSystems installer
with magic-nix-cache, and drop --no-build so checks fully build.

https://claude.ai/code/session_01QYnXiuNMQ9aCbqbu7j7tKH
Each check now builds system.build.toplevel, catching real build
failures rather than just evaluation errors.

https://claude.ai/code/session_01QYnXiuNMQ9aCbqbu7j7tKH
The omnix disko module sets disko.devices which requires the upstream
disko NixOS module to define that option. Add extraModules parameter
to evalModule so checks can include external dependencies.

https://claude.ai/code/session_01QYnXiuNMQ9aCbqbu7j7tKH

@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.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 8-10: Add a job timeout to prevent indefinite CI consumption by
updating the flake-check job definition (job name: flake-check) to include a
timeout-minutes setting (e.g., timeout-minutes: 60 or another
project-appropriate value) at the job level alongside runs-on; this ensures the
job is automatically cancelled after the configured time and protects CI
minutes.
- Around line 12-15: The workflow uses
DeterminateSystems/nix-installer-action@main and
DeterminateSystems/magic-nix-cache-action@main which should be pinned to
immutable releases; update those two usages (leave actions/checkout@v4 as-is) to
specific tags or commit SHAs (e.g., a released version tag or a commit SHA) for
reproducible CI and security, and update any related documentation/comments to
note the chosen pinned versions.

In `@flake.nix`:
- Around line 61-72: The evalModule helper currently evaluates modules without
providing required option values, causing `option ... is used but not defined`
errors; update evalModule to either (A) supply stub values for the required
options by merging a small module into the modules list that sets
omnix.base.sshKeys = []; omnix.storage.volumeName = "stub";
omnix.services.project = "stub" (so eval-base, eval-storage, eval-services
pass), or (B) avoid enabling those modules during evaluation by not including
the modules that require those options; modify the modules array built in
evalModule to include the stub module (or skip the specific modules) so nix
flake check no longer fails.

In `@modules/base.nix`:
- Around line 106-115: The current environment.etc block creates
/etc/nushell/config.nu and env.nu but Nushell won't auto-load /etc/nushell
unless NU_VENDOR_AUTOLOAD_DIR is set; update the module to either export
NU_VENDOR_AUTOLOAD_DIR=/etc/nushell when shellCfg.nushell.manageConfig is true
(add the env var to the environment variables set by the module) or change the
placement to the user's XDG config path (write the files into the user's
$XDG_CONFIG_HOME/nushell/ location instead); adjust the logic around
shellCfg.nushell.manageConfig and the entries creating "nushell/config.nu" and
"nushell/env.nu" accordingly so the files are actually discovered at runtime.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f490d28a-ebea-4c93-ac76-ca60072b3394

📥 Commits

Reviewing files that changed from the base of the PR and between f1c808e and 1b79a6a.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • flake.nix
  • modules/base.nix
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: flake-check
🧰 Additional context used
📓 Path-based instructions (2)
**/*.nix

📄 CodeRabbit inference engine (AGENTS.md)

**/*.nix: Use lib.mkOption with proper types for all module options
Every module must be independently usable (no implicit dependencies on other omnix modules)
Options must live under the omnix.* namespace
Use lib.mkIf cfg.enable for conditional configuration
Use lib.mkDefault for overridable defaults
Every configurable value must be a module option with a type and description
Prefer lib.types.str over lib.types.path for values that may come from terraform or other dynamic sources
Group related options under a single omnix.<module> prefix
Functions take an attrset of required parameters
Return an attrset of derivations or values from library functions
Use pkgs.writeShellApplication for shell scripts (not raw writeScript)
Include runtimeInputs in shell scripts and never assume tools are on PATH
All shell scripts must use set -eo pipefail
When changing module option interfaces, preserve the old option with lib.mkRenamedOptionModule or lib.mkRemovedOptionModule with a clear migration message

Files:

  • flake.nix
  • modules/base.nix
*

⚙️ CodeRabbit configuration file

Focus on providing constructive criticism. Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s). Flag potential footguns. Suggest FP alternatives to mutable/imperative code. Point out architectural flaws like leaky abstractions, tight coupling, wrong level of abstraction, poor type modeling, over-abstraction, unclear domain boundaries. Code should generally be organized based on business concerns rather than technical aspects - suggest improvements if you find violations. Point out gaps in test coverage but suggest tests that are not too coupled to the implementation and actually test domain invariants and business logic

Files:

  • flake.nix
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Group related options under a single `omnix.<module>` prefix
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `pkgs.writeShellApplication` for shell scripts (not raw `writeScript`)
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : All shell scripts must use `set -eo pipefail`
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Include `runtimeInputs` in shell scripts and never assume tools are on PATH
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Consumer repos using devenv dev shells require `nix flake check --impure` (devenv needs impure evaluation to resolve the working directory)
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Template projects should pass `nix flake check`

Applied to files:

  • .github/workflows/ci.yml
  • flake.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Consumer repos using devenv dev shells require `nix flake check --impure` (devenv needs impure evaluation to resolve the working directory)

Applied to files:

  • .github/workflows/ci.yml
  • flake.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkDefault` for overridable defaults

Applied to files:

  • flake.nix
  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Every module must be independently usable (no implicit dependencies on other omnix modules)

Applied to files:

  • flake.nix
  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Group related options under a single `omnix.<module>` prefix

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Options must live under the `omnix.*` namespace

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `pkgs.writeShellApplication` for shell scripts (not raw `writeScript`)

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkOption` with proper types for all module options

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Include `runtimeInputs` in shell scripts and never assume tools are on PATH

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : When changing module option interfaces, preserve the old option with `lib.mkRenamedOptionModule` or `lib.mkRemovedOptionModule` with a clear migration message

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : All shell scripts must use `set -eo pipefail`

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Use `lib.mkIf cfg.enable` for conditional configuration

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Prefer `lib.types.str` over `lib.types.path` for values that may come from terraform or other dynamic sources

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: omnix must never contain dev shell configuration

Applied to files:

  • modules/base.nix
📚 Learning: 2026-03-21T15:47:14.585Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.585Z
Learning: Applies to **/*.nix : Every configurable value must be a module option with a type and description

Applied to files:

  • modules/base.nix
🔇 Additional comments (6)
modules/base.nix (4)

3-7: Well-structured local bindings for shell configuration.

Clean decomposition of configuration accessors and derived booleans. This improves readability throughout the module.


29-61: Well-designed shell configuration options.

Options are properly typed with lib.mkOption, include descriptions, and are correctly grouped under omnix.base.shell. The readOnly = true on shellPackage appropriately prevents consumers from setting it directly. Based on learnings: "Group related options under a single omnix.<module> prefix".


89-92: Correct shell package selection with pkgs.bashInteractive.

The use of pkgs.bashInteractive for bash ensures readline support in login shells, as required for proper interactive features.


94-97: Conditional shell configuration looks correct.

The lib.mkIf for bash.viMode and lib.mkDefault for zsh.enable follow the coding guidelines appropriately.

flake.nix (2)

79-89: Correct pattern for external module dependencies in eval-disko.

Including disko.nixosModules.disko as an extra module for the disko check is the right approach for modules with external dependencies. As per coding guidelines: "Every module must be independently usable (no implicit dependencies on other omnix modules)".


56-58: Clean pkgs binding.

Standard pattern for importing nixpkgs with system.

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml
Comment thread flake.nix Outdated
Comment thread modules/base.nix
@0xgleb 0xgleb self-assigned this May 9, 2026
@0xgleb

0xgleb commented May 9, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

1 similar comment
@0xgleb

0xgleb commented May 9, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 9, 2026

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

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.

2 participants