Add NB2123 host and reorganize home modules into shared/darwin layers - #1
Merged
Conversation
- Add NB2123 (work MacBook) - corp gateway env, cert bundle, npm/yarn registries, dev-shell sidecar for the NPA project, and codified macOS defaults (dock, finder, screensaver, symbolic hotkeys, .DS_Store off on network/USB) - Introduce ~/.config/dotfiles/private.nix for per-host identity and secrets (JWT, PAT, base URL, CA PEM). Flake reads it via --impure and passes as specialArg; template at hosts/NB2123/private.example.nix - Reorganize modules/home/ into a platform-generic default + darwin/ bundle. Hosts on macOS import a single path (modules/home/darwin). Darwin-only modules carry an isDarwin guard for defense in depth - Guard the corp-npa dev-shell sidecar on private.npa presence so enabling direnv on non-NPA hosts doesn't blow up - Add home modules: claude-code (settings.json jq-merge activation), git (global ignores, GCM, includeIf identities), rtk, zed, keepass, helium - Add pkgs: Ice, Emdash, Raycast, Zed (brew casks on macOS; nixpkgs paths for Linux where relevant) - Split git config layers: brutcha identity + GCM shared; corp CA path, ADO useHttpPath, and Creditas identity host/project scoped - Rewrite README + add hosts/NB2123/README.md - Fix sketchybar typos and clean up an obsolete keepass TODO
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThis PR adds the NB2123 host, host-private certificate and registry wiring, opt-in Darwin/Home Manager modules, new development and package wiring, and updated documentation plus cache-check tooling. ChangesNB2123 host and Darwin wiring
Development modules and package wiring
Docs, scripts, and cosmetic updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant flake.nix
participant NB2123
participant HomeManager
participant private.nix
flake.nix->>private.nix: import host private values
flake.nix->>NB2123: define darwinConfigurations.NB2123
NB2123->>HomeManager: pass private via specialArgs
HomeManager->>HomeManager: build host-scoped config
sequenceDiagram
participant HomeManager
participant HeliumModule
participant KeepassModule
participant FileSystem
HomeManager->>HeliumModule: enable Helium
HeliumModule->>FileSystem: merge prefsOverrides and flagOverrides
HomeManager->>KeepassModule: enable KeePassXC integration
KeepassModule->>FileSystem: write browser settings and native messaging manifest
HomeManager->>FileSystem: register .app bundles with LaunchServices
sequenceDiagram
participant DevShellModule
participant CorpNpaModule
participant ProjectDir
participant GitConfig
DevShellModule->>CorpNpaModule: import when private.npa exists
CorpNpaModule->>ProjectDir: write flake.nix, .envrc, .emdash.json
CorpNpaModule->>ProjectDir: symlink sidecar files into ~/git/<projectId>
CorpNpaModule->>GitConfig: apply per-project identity include
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
modules/home/development/rtk.nix (1)
18-22: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOrdering guarantee only holds via the shared default; not enforced.
rtk.enabledefaults toclaude-code.enable, andrtkInitrunsentryAfter [ "claudeCodeSettings" ... ]. If a user ever setsrtk.enable = truewhileclaude-code.enable = false(default no longer applies), theclaudeCodeSettingsactivation entry won't exist, silently dropping the ordering guarantee this comment block relies on —rtk init -g --auto-patchcould then run against a nonexistent/still-symlinked~/.claude/settings.json.An
assertionsentry (e.g.cfg.enable -> config.home.apps.development.claude-code.enable) would make this dependency explicit instead of implicit-via-default.♻️ Proposed assertion
config = lib.mkIf cfg.enable { + assertions = [{ + assertion = config.home.apps.development.claude-code.enable; + message = "home.apps.development.rtk requires claude-code.enable (claudeCodeSettings must run first)."; + }]; home.packages = [ pkgs.rtk ];Also applies to: 31-35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/home/development/rtk.nix` around lines 18 - 22, The ordering dependency between rtk and claude-code is only implied by the shared default, so make it explicit in the rtk module. Add an assertion in the rtk configuration logic that rejects cfg.enable when config.home.apps.development.claude-code.enable is false, and keep the existing entryAfter dependency in rtkInit tied to claudeCodeSettings. Use the existing symbols rtk.enable, rtkInit, and claudeCodeSettings so the dependency remains enforced even when defaults are overridden.modules/home/darwin/window-manager/sketchybar.nix (1)
17-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIndentation is inconsistent within the
programs.sketchybarblock.
sketchybar = {,enable = true;, andconfig = {are not indented relative toprograms = {the way the rest of the file's style suggests (compare toxdg.configFileblock below). Minor readability nit only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/home/darwin/window-manager/sketchybar.nix` around lines 17 - 22, The `programs.sketchybar` block has inconsistent indentation relative to `programs = {`, so adjust the alignment of `sketchybar = {`, `enable = true;`, and `config = {` to match the surrounding Nix style used elsewhere in the file, especially the `xdg.configFile` block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flake.nix`:
- Around line 120-124: The Home Manager wiring in flake.nix is forwarding
private.nix through home-manager.extraSpecialArgs, which can expose long-lived
secrets during Nix evaluation. Update the private argument passed via
extraSpecialArgs so it contains only non-secret metadata, and move secret access
out of evaluation in the downstream NB2123 config that reads
private.secrets.anthropicJwt. Load the token at runtime from a secure source
such as Keychain, sops/agenix, or a 0600 file outside the store, and apply the
same change wherever private is passed through the affected call sites.
In `@hosts/NB2123/home.nix`:
- Around line 26-38: Move the secret-backed env values out of the Nix-evaluated
cfg.env assignment in the claude-code block so they are not embedded by
builtins.toJSON into settingsOverridesJson. Replace direct uses of
private.secrets.anthropicBaseUrl and private.secrets.anthropicJwt with runtime
secret injection or activation-time sourcing, and apply the same pattern to the
private.secrets.azureDevopsPat usage in registries.nix. Keep the claude-code and
registries settings structure intact while ensuring no secret values are
serialized into the store-backed activation script.
In `@hosts/NB2123/registries.nix`:
- Around line 17-21: The Yarn registry config in registries.nix is embedding
private.secrets.azureDevopsPat into the generated .yarnrc.yml text, which places
the Azure DevOps PAT in the Nix store. Update the logic around the npmRegistries
/ home.file.".yarnrc.yml".text generation to source the token from a secrets
manager or other runtime secret mechanism instead of interpolating it directly;
if keeping it in plaintext is intentional, add an explicit comment/documentation
note near the private.secrets.azureDevopsPat usage and the registries config to
make that tradeoff clear.
In `@modules/home/development/claude-code.nix`:
- Around line 12-20: `settingsOverridesJson` is embedding all of `cfg.env` into
a store-backed JSON blob, which can expose secrets like auth tokens. Update
`claudeCodeSettings` so `cfg.env` only carries non-sensitive values, and load
any secret at activation/runtime from a private source instead of via
`builtins.toJSON`; use the existing
`settingsOverridesJson`/`home.activation.claudeCodeSettings` path to locate the
affected flow.
In `@modules/home/development/dev-shells/corp-npa.nix`:
- Around line 18-23: The shellHook in corp-npa.nix is disabling Node TLS
verification for the entire dev shell, which is too broad. Update the shellHook
so NODE_TLS_REJECT_UNAUTHORIZED=0 is not exported globally; instead, scope the
bypass to only the specific corp NPA commands or hosts that still need it, and
keep caExport/NODE_EXTRA_CA_CERTS as the default for all other Node tooling.
- Around line 70-76: The symlink check in the dev-shell setup loop is using a
test that follows links, so a dangling symlink is treated as missing and the
subsequent ln -s fails on re-activation. Update the existence check in the
corp-npa.nix shell snippet to treat any existing path, including broken
symlinks, as present before creating links for .emdash.json and .envrc, so
rerunning home-manager switch skips stale links instead of trying to recreate
them.
In `@README.md`:
- Around line 27-48: The README tree block is an unlabeled fenced code block
that markdownlint flags; update the fence around the directory listing to use a
neutral language such as text. Locate the documentation snippet in README.md and
change only the opening fence for that tree diagram so the markdown lint passes
while keeping the content unchanged.
In `@scripts/check-cache.sh`:
- Around line 45-48: The package lookup in the nix eval expression is using an
unquoted attribute access via .$pkg, which fails for hyphenated package names.
Update the lookup in the cache probe to use a quoted/dynamic attribute access
form in the expression, such as builtins.getAttr or pkgs."$pkg", so the script
handles watched packages like git-credential-manager, zed-editor, and dotnet-sdk
correctly.
---
Nitpick comments:
In `@modules/home/darwin/window-manager/sketchybar.nix`:
- Around line 17-22: The `programs.sketchybar` block has inconsistent
indentation relative to `programs = {`, so adjust the alignment of `sketchybar =
{`, `enable = true;`, and `config = {` to match the surrounding Nix style used
elsewhere in the file, especially the `xdg.configFile` block.
In `@modules/home/development/rtk.nix`:
- Around line 18-22: The ordering dependency between rtk and claude-code is only
implied by the shared default, so make it explicit in the rtk module. Add an
assertion in the rtk configuration logic that rejects cfg.enable when
config.home.apps.development.claude-code.enable is false, and keep the existing
entryAfter dependency in rtkInit tied to claudeCodeSettings. Use the existing
symbols rtk.enable, rtkInit, and claudeCodeSettings so the dependency remains
enforced even when defaults are overridden.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcfd1b50-311e-4927-ac1e-60ceaf2eb6b2
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
.gitignoreREADME.mdconfig/sketchybar/items/network.luaconfig/sketchybar/items/ram.luaflake.nixhosts/NB2123/README.mdhosts/NB2123/cert-bundle.nixhosts/NB2123/default.nixhosts/NB2123/home.nixhosts/NB2123/private.example.nixhosts/NB2123/registries.nixhosts/makima/home.nixmodules/darwin/apps/system.nixmodules/home/darwin/default.nixmodules/home/darwin/development.nixmodules/home/darwin/internet/default.nixmodules/home/darwin/internet/helium.nixmodules/home/darwin/security/default.nixmodules/home/darwin/security/keepass.nixmodules/home/darwin/window-manager/aerospace.nixmodules/home/darwin/window-manager/default.nixmodules/home/darwin/window-manager/jankyborders.nixmodules/home/darwin/window-manager/sketchybar.nixmodules/home/default.nixmodules/home/development/claude-code.nixmodules/home/development/darwin.nixmodules/home/development/default.nixmodules/home/development/dev-shells/corp-npa.nixmodules/home/development/dev-shells/default.nixmodules/home/development/dev-shells/templates/node-project.flake.nixmodules/home/development/git.nixmodules/home/development/nvim.nixmodules/home/development/rtk.nixmodules/home/development/zed.nixmodules/home/internet/default.nixmodules/home/internet/zen.nixmodules/home/window-manager/jankyborders.nixmodules/shared/apps/development.nixpkgs/default.nixpkgs/emdash/default.nixpkgs/raycast/default.nixpkgs/zed-editor/default.nixscripts/check-cache.sh
💤 Files with no reviewable changes (4)
- modules/home/development/darwin.nix
- modules/home/window-manager/jankyborders.nix
- modules/home/internet/zen.nix
- modules/home/internet/default.nix
Secrets previously imported at eval time from private.nix (anthropicJwt, anthropicBaseUrl, azureDevopsPat, corpCaBundlePem) leaked into the world-readable /nix/store via activation scripts and pkgs.writeText. Now they live only in the KeePassXC vault under corp/*, extracted at activation time using the master password cached in macOS Keychain (service kdbx-master, account du234).
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hosts/NB2123/cert-bundle.nix`:
- Around line 33-43: The cert-bundle update logic currently only logs warnings
when corp CA extraction is skipped, which can leave /etc/nix/cert-bundle.pem
absent. Update the cert-bundle.nix activation flow around the vault-missing and
extractCa failure branches to always seed a fallback bundle at
/etc/nix/cert-bundle.pem, using the system cert bundle already referenced in the
successful path, so nix.settings.ssl-cert-file always points to an existing
file.
In `@hosts/NB2123/home.nix`:
- Around line 69-78: The activation snippet currently exits early in the
missing-vault and missing-Keychain branches, which can abort the rest of the
Home Manager activation sequence. Refactor the secret-extraction logic in the
guarded block so it uses if/else handling instead of calling exit 0, keeping
only the corp-secrets-specific work conditional while allowing the surrounding
activation script to continue normally.
- Around line 64-66: The KDBX master password bootstrap examples currently use
the broad Keychain access flag, which should be avoided. Update the bootstrap
guidance in the related Keychain helper examples to remove the permissive access
and either leave the item prompt-protected or restrict access to only the
specific helper binaries that need it. Make sure both example lines in the
affected comment block are updated consistently so users do not copy the unsafe
option.
In `@hosts/NB2123/private.example.nix`:
- Around line 2-4: The host metadata comment points to the wrong activation
hook; update the reference so it matches the actual KeePass extraction hook used
by this host. In the private.example.nix header, replace the mention of
home.apps.security.keepass.secrets with the real symbol
home.activation.keepassSecretsExtract so the template and implementation stay
aligned.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ddd1c5b8-9aa2-4b00-ad1c-5bf2d3f8ed22
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
hosts/NB2123/README.mdhosts/NB2123/cert-bundle.nixhosts/NB2123/home.nixhosts/NB2123/private.example.nixhosts/NB2123/registries.nixmodules/home/development/rtk.nix
✅ Files skipped from review due to trivial changes (1)
- hosts/NB2123/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/home/development/rtk.nix
added 2 commits
July 2, 2026 22:12
- corp-npa.nix: drop NODE_TLS_REJECT_UNAUTHORIZED=0 — verified with `NODE_TLS_REJECT_UNAUTHORIZED=1 yarn install --frozen-lockfile` in the NPA checkout, which completed without cert errors. The corp CA from the KDBX-extracted /etc/nix/cert-bundle.pem via NODE_EXTRA_CA_CERTS is now sufficient on its own. - corp-npa.nix: guard sidecar symlink creation against dangling links by also testing -L, not just -e (which follows into non-existence). - README.md: label the directory-tree fenced block as `text` for MD040. - scripts/check-cache.sh: quote the dynamic attribute access (`."$pkg"`) so hyphenated watchlist entries (git-credential-manager, zed-editor, dotnet-sdk) resolve instead of silently failing eval.
- corp-npa.nix: drop NODE_TLS_REJECT_UNAUTHORIZED=0 - corp-npa.nix: guard sidecar symlink creation against dangling links - home.nix: replace `exit 0` in home.activation blocks with if/else - home.nix + README + memory: tighten Keychain bootstrap - private.example.nix: update stale hook reference - README.md: label the directory-tree fenced block - scripts/check-cache.sh: quote the dynamic attribute access
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This was referenced Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit