feat: provide more functionality#3
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 40 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughUpdated project to migrate embedded Bash scripts to Nushell with new mkNuScript wrappers, restructured the flake using flake-parts, added a staticSites NixOS module, introduced example configurations and validation tooling, and expanded documentation and testing for Nushell conventions and workflows. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
AGENTS.md (1)
59-137:⚠️ Potential issue | 🟠 MajorAdd explicit agent implementations + responsibilities section.
This update adds style/process rules, but it still doesn’t document which agents exist and what each one is responsible for, which makes AGENTS.md incomplete for onboarding and governance.
As per coding guidelines,
AGENTS.md: "Document agent implementations and their responsibilities in AGENTS.md".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 59 - 137, Add a new "Agent implementations & responsibilities" section to AGENTS.md that enumerates each agent (name), its primary responsibilities, public interface/commands (e.g., CLI entrypoint like main, exported functions), ownership/team, inputs/outputs (expected flags/env vars and output format), failure modes and recommended error handling (how to surface errors, retries), testing requirements (test file naming and key assertions), and communication/permission boundaries (what other agents or services it may call and required credentials). Use concise bullet entries per agent so readers can quickly find the agent by name and see its responsibilities, interfaces, testing and ownership details.lib/deploy.nix (1)
62-84:⚠️ Potential issue | 🟠 MajorPrevent service/site profile name collisions.
Both
profilesOrderandprofilesuse the bare profile name for services and static sites. If both attrsets contain the same key, the later//merge silently overwrites the service profile with the site profile, so one deploy target disappears without an evaluation error.🚫 Possible guard
enabledServices = builtins.filter (name: services.${name}.enabled) (builtins.attrNames services); enabledSites = builtins.filter (name: staticSites.${name}.enabled) (builtins.attrNames staticSites); + overlappingProfiles = + builtins.filter (name: builtins.elem name enabledSites) enabledServices; -in +in +if overlappingProfiles != [ ] then + throw "mkDeploy: service and static site names must be distinct: ${builtins.concatStringsSep ", " overlappingProfiles}" +else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/deploy.nix` around lines 62 - 84, profilesOrder and profiles currently use raw names from enabledServices and enabledSites which can collide when the same key exists in both; change both to namespace the keys (e.g. prefix service names with "service:" and site names with "site:") so the attr names are unique, update the profiles map construction that uses mkProfile and mkSiteProfile to emit those names (and adjust profilePath for site profiles to include the new names or keep original base but use the namespaced attr key), and update any downstream consumers that iterate profilesOrder or look up profiles to use the new namespaced keys (references: profilesOrder, profiles, enabledServices, enabledSites, mkProfile, mkSiteProfile, staticSites, profileBase).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 93: Remove the trailing space inside the inline code span on the line
"Test functions prefixed with `test ` for auto-discovery" (change `test ` to
`test`) to fix MD038, and ensure all fenced code blocks (the snippet shown
around lines 103–106) are surrounded by blank lines before and after the ```
fence to fix MD031; locate the code block in AGENTS.md, add a blank line above
the opening ``` and below the closing ```, and verify inline code spans nearby
contain no leading/trailing spaces.
In `@CHANGELOG.md`:
- Around line 27-42: The changelog currently lists major removals and
replacements but lacks an explicit "Breaking changes / Migration" subsection;
add a new subsection titled "Breaking changes / Migration" that clearly lists
concrete migration steps for users affected by removals such as `lib.mkRemote`
(note replacement via `mkTerraform`'s `remote` output), the removal of bash/age
helper exports (`parseIdentity`, `resolveIp`, `decryptState`) and the
`lib/shell.nix` rewrite exposing `mkNuScript`, and the switch from `flake-utils`
to `flake-parts`; for each item include the impact (what stopped working), the
replacement or workaround (e.g., use `mkTerraform.remote`, update scripts to use
`mkNuScript` or port helpers), and any commands or config lines users must
change to migrate.
In `@examples/full/os.nix`:
- Around line 52-67: The staging service is bound to port 8080 but
omnix.firewall only enables HTTP/HTTPS, so explicitly open 8080 by adding it to
omnix.firewall.allowedTCPPorts (or else move the staging site behind the 80/443
reverse-proxy); update the omnix.firewall config to include port 8080 so the
staging block (staging.port = 8080) is reachable without relying on other omnix
modules.
In `@flake.nix`:
- Around line 81-97: The flake currently exposes nixosConfigurations (mkExample,
example-minimal, example-single-service, example-full) but checks.examples only
runs the port-consistency script; update checks to also evaluate/build those
example systems so `nix flake check` fails if examples/os.nix are broken.
Concretely, modify the checks.examples entry to include the example NixOS
systems (referencing nixosConfigurations.example-minimal,
nixosConfigurations.example-single-service, nixosConfigurations.example-full or
call mkExample on the example files) so the check performs a build/evaluation of
each example (and apply the same change for the other identical checks block
noted around the later 124-128 region).
In `@lib/terraform.nix`:
- Around line 21-27: The mkTfTask function currently takes positional
parameters; change it to accept a single attrset of required parameters (e.g. {
name, subcommand, runtimeInputs, keysFile, ... }) so the function signature
becomes mkTfTask = args: with args; or mkTfTask = { name, subcommand,
runtimeInputs, keysFile }: and update the body to reference those names (leave
the call to mkNuScript and fields script and extraArgs unchanged, passing
extraArgs = [ (toString keysFile) ]); ensure any callers are updated to call
mkTfTask with an attrset.
- Around line 62-76: The template currently inherits rekey directly into the
package set which allows a null value from the conditional block (the rekey = if
... then mkNuScript { ... } else null) to leak into flake outputs; update the
template inheritance so rekey is filtered out when null (e.g. use conditional
inheritance or merge with // { rekey = null; }) when adding rekey into packages,
ensuring the mkNuScript-based rekey (references: rekey, mkNuScript,
runtimeInputs, keysFile, secretsRules) is only exported when non-null and
otherwise replaced with null to prevent null propagation into packages and flake
check failures.
In `@modules/static-sites.nix`:
- Around line 50-63: Add a guard that asserts there is at most one site with
isDefault = true to avoid multiple conflicting nginx default virtual hosts:
compute the set/length of enabledSites entries where siteCfg.isDefault is true
(using enabledSites, siteCfg, and the services.nginx.virtualHosts mapping
context) and assert that the count is <= 1, failing the build with a clear
message if more than one default is configured; place this assertion near where
services.nginx.virtualHosts is defined so the configuration fails early.
- Around line 15-47: The module lacks a top-level enable option and gates its
config on enabledSites instead of cfg.enable; add an
options.omnix.staticSites.enable = lib.mkOption { type = lib.types.bool; default
= true; description = "Enable the staticSites module"; } and change the config
guard from lib.mkIf (enabledSites != { }) to lib.mkIf cfg.enable so all
conditional configuration (inside the existing config block that references
enabledSites, port, isDefault, extraLocations) is applied only when cfg.enable
is true; keep enabledSites (or definitions) as the place for per-site
definitions but ensure every configurable value remains declared under
options.omnix.staticSites (e.g., definitions) and use cfg when referencing them
in the config block.
In `@scripts/bootstrap.nu`:
- Around line 37-39: The current check uses a generic substring test ($updated |
str contains $new_key) which can pass if new_key appears anywhere; change the
verification to assert the host field was specifically updated by testing for
the exact host assignment (e.g. match host\s*=\s*"<new_key>" or build the
literal string host = "<$new_key>" and use str contains on that) so the
condition verifies the host = "..." entry was rewritten rather than any
occurrence of new_key.
- Around line 25-30: In extract-host-key, validate the SSH output before
indexing parts: check that raw is non-empty (after trimming) and that splitting
it into parts yields at least two tokens (parts.0 and parts.1) before
constructing the return string; if validation fails, surface a clear bootstrap
error (fail/return an explicit error) mentioning the host_ip and that the host
key was empty/malformed instead of letting the code crash when accessing
parts.0/parts.1.
In `@scripts/common.nu`:
- Around line 73-76: The current host_ip assignment uses open $TF_STATE | get
$output_key which leaves Terraform state as raw text so get fails; change the
read to explicitly parse JSON by using open $TF_STATE --raw | from json | get
$output_key (keep the surrounding try block and the rm -f $TF_STATE removal) so
TF_STATE is parsed before extracting the output key; update the expression
inside the try where host_ip is assigned to use this pipeline.
In `@scripts/terraform.nu`:
- Around line 77-92: The catch block and final cleanup unconditionally remove
the edited plaintext ($vars_file), losing changes when encrypt-vars fails;
update the error handling so that on failure you do not unlink $vars_file but
instead preserve it as a 0600 recovery (e.g., move/rename to
"${vars_file}.recover" or chmod 600 and leave), and ensure the final
unconditional rm -f $vars_file is only executed when encrypt-vars succeeded (or
remove that unconditional removal and make cleanup conditional), referencing the
decrypt-vars/encrypt-vars calls, the catch block, and the rm -f $vars_file
cleanup.
- Around line 42-52: The final unconditional call to cleanup-with-plan in the
"main plan" function removes the TF_PLAN artifact (TF_PLAN = "infra/tfplan")
after a successful plan, preventing "main apply" from reading it; remove that
trailing cleanup-with-plan call (or guard it to run only on error) so
cleanup-with-plan is invoked only in the catch branch, leaving the created
tfplan file intact for main apply to consume. Ensure references to decrypt-vars,
decrypt-state, and the terraform plan invocation (^terraform -chdir=infra plan
-out=tfplan ...$parsed.rest) remain unchanged.
---
Outside diff comments:
In `@AGENTS.md`:
- Around line 59-137: Add a new "Agent implementations & responsibilities"
section to AGENTS.md that enumerates each agent (name), its primary
responsibilities, public interface/commands (e.g., CLI entrypoint like main,
exported functions), ownership/team, inputs/outputs (expected flags/env vars and
output format), failure modes and recommended error handling (how to surface
errors, retries), testing requirements (test file naming and key assertions),
and communication/permission boundaries (what other agents or services it may
call and required credentials). Use concise bullet entries per agent so readers
can quickly find the agent by name and see its responsibilities, interfaces,
testing and ownership details.
In `@lib/deploy.nix`:
- Around line 62-84: profilesOrder and profiles currently use raw names from
enabledServices and enabledSites which can collide when the same key exists in
both; change both to namespace the keys (e.g. prefix service names with
"service:" and site names with "site:") so the attr names are unique, update the
profiles map construction that uses mkProfile and mkSiteProfile to emit those
names (and adjust profilePath for site profiles to include the new names or keep
original base but use the namespaced attr key), and update any downstream
consumers that iterate profilesOrder or look up profiles to use the new
namespaced keys (references: profilesOrder, profiles, enabledServices,
enabledSites, mkProfile, mkSiteProfile, staticSites, profileBase).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2672f215-2380-47ff-9d4d-f93fc36b6fd1
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
AGENTS.mdCHANGELOG.mdREADME.mdROADMAP.mdexamples/full/config/api.tomlexamples/full/config/staging-api.tomlexamples/full/config/worker.tomlexamples/full/os.nixexamples/minimal/os.nixexamples/single-service/config/api.tomlexamples/single-service/os.nixflake.nixlib/bootstrap.nixlib/deploy.nixlib/shell.nixlib/terraform.nixmodules/base.nixmodules/static-sites.nixmodules/storage.nixscripts/bootstrap.nuscripts/bootstrap.test.nuscripts/common.nuscripts/common.test.nuscripts/deploy.nuscripts/terraform.nuscripts/validate-examples.nutemplates/do-service/os.nix
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.nix
📄 CodeRabbit inference engine (AGENTS.md)
**/*.nix: Uselib.mkOptionwith proper types for all module options
Every module must be independently usable (no implicit dependencies on other omnix modules)
Options must live under theomnix.*namespace
Uselib.mkIf cfg.enablefor conditional configuration
Uselib.mkDefaultfor overridable defaults
Every configurable value must be a module option with a type and description
Preferlib.types.stroverlib.types.pathfor values that may come from terraform or other dynamic sources
Group related options under a singleomnix.<module>prefix
Functions take an attrset of required parameters
Return an attrset of derivations or values from library functions
Usepkgs.writeShellApplicationfor shell scripts (not rawwriteScript)
IncluderuntimeInputsin shell scripts and never assume tools are on PATH
All shell scripts must useset -eo pipefail
When changing module option interfaces, preserve the old option withlib.mkRenamedOptionModuleorlib.mkRemovedOptionModulewith a clear migration message
Files:
templates/do-service/os.nixexamples/minimal/os.nixmodules/base.nixmodules/storage.nixlib/shell.nixmodules/static-sites.nixexamples/full/os.nixlib/deploy.nixlib/bootstrap.nixexamples/single-service/os.nixflake.nixlib/terraform.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:
CHANGELOG.mdAGENTS.mdROADMAP.mdREADME.mdflake.nix
**/*.md
⚙️ CodeRabbit configuration file
Focus on the contents of the docs and not on cosmetic things like markdown formatting. We use markdown files for various docs including but not limited to the north star system specification, SPEC.md, the plan for how to get there, ROADMAP.md, guidelines for AI contributors, AGENTS.md, project overview and instructions for human contributors, README.md. Think about the target audience of a document when deciding what comment to leave. For specifications and designs, suggest potential product, architectural, and UI/UX improvements. For plans, suggest changes that would make things more parallelizable and deliverable-focused. For instructions, suggest better rules and guidelines and point out missing instructions. In all cases, flag needless bloat, prefer clear concise writing, and consider the structure of the document and order of the sections
Files:
CHANGELOG.mdAGENTS.mdROADMAP.mdREADME.md
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document agent implementations and their responsibilities in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Template projects should pass `nix flake check`
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
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.571Z
Learning: Applies to **/*.nix : All shell scripts must use `set -eo pipefail`
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Options must live under the `omnix.*` namespace
Applied to files:
templates/do-service/os.nixAGENTS.mdROADMAP.mdmodules/static-sites.nixexamples/full/os.nixREADME.mdexamples/single-service/os.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Use `pkgs.writeShellApplication` for shell scripts (not raw `writeScript`)
Applied to files:
modules/base.nixAGENTS.mdlib/shell.nixROADMAP.mdlib/deploy.nixlib/bootstrap.nixscripts/deploy.nulib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
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:
CHANGELOG.mdlib/shell.nixROADMAP.mdmodules/static-sites.nixREADME.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Every module must be independently usable (no implicit dependencies on other omnix modules)
Applied to files:
CHANGELOG.mdAGENTS.mdROADMAP.mdmodules/static-sites.nixexamples/full/os.nixREADME.mdexamples/single-service/os.nixflake.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Group related options under a single `omnix.<module>` prefix
Applied to files:
CHANGELOG.mdAGENTS.mdROADMAP.mdmodules/static-sites.nixexamples/full/os.nixREADME.mdexamples/single-service/os.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Template projects should pass `nix flake check`
Applied to files:
scripts/validate-examples.nuROADMAP.mdflake.nix
📚 Learning: 2026-03-21T15:46:59.282Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T15:46:59.282Z
Learning: Applies to AGENTS.md : Document agent implementations and their responsibilities in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : All shell scripts must use `set -eo pipefail`
Applied to files:
AGENTS.mdlib/shell.nixROADMAP.mdlib/deploy.nixlib/bootstrap.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Include `runtimeInputs` in shell scripts and never assume tools are on PATH
Applied to files:
AGENTS.mdlib/shell.nixlib/deploy.nixlib/bootstrap.nixflake.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Functions take an attrset of required parameters
Applied to files:
AGENTS.mdlib/shell.nixREADME.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Return an attrset of derivations or values from library functions
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Never suppress Nix evaluation errors or warnings
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Every configurable value must be a module option with a type and description
Applied to files:
AGENTS.mdmodules/static-sites.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: omnix must never contain frontend build logic
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: omnix must never contain CI pipeline definitions
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
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:
lib/shell.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Use `lib.mkOption` with proper types for all module options
Applied to files:
modules/static-sites.nixREADME.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Use `lib.mkDefault` for overridable defaults
Applied to files:
README.mdflake.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Consumer repos using devenv dev shells require `nix flake check --impure` (devenv needs impure evaluation to resolve the working directory)
Applied to files:
flake.nix
🪛 markdownlint-cli2 (0.21.0)
AGENTS.md
[warning] 93-93: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 103-103: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 106-106: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🔇 Additional comments (13)
modules/storage.nix (1)
23-28: Good validation guard forvolumeName.This assertion tightens input safety before using the value in the device path and gives a clear failure message.
modules/base.nix (1)
62-62: Tmpfiles migration looks correct.Using
systemd.tmpfiles.ruleshere is a cleaner declarative replacement for imperative directory creation.lib/shell.nix (1)
9-29:mkNuScriptwrapper is well-structured.The helper interface is clear, arguments are shell-escaped, and runtime tool dependencies are explicit.
examples/single-service/config/api.toml (1)
1-3: Config sample aligns with validator expectations.
server.port = 8000matches the example proxy target and keeps the example-consistency check meaningful.examples/full/config/api.toml (1)
1-3: Looks good for full-example API placeholder config.Simple and consistent with the documented full example port mapping.
examples/full/config/worker.toml (1)
1-3: Worker placeholder config is clean and usable.This gives the full example a concrete worker config baseline without overfitting implementation details.
ROADMAP.md (1)
5-7:⚠️ Potential issue | 🟡 MinorUpdate stale status wording in migration intro.
Line 5 says scripts are “currently bash embedded,” but this same section now marks the migration tasks as done. Please switch this paragraph to past tense (or explicitly describe only remaining gaps) to avoid contradictory roadmap status.
⛔ Skipped due to learnings
Learnt from: CR Repo: data-cartel/omnix PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-03-21T15:47:14.571Z 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.571Z 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.571Z Learning: Applies to **/*.nix : Include `runtimeInputs` in shell scripts and never assume tools are on PATHLearnt from: CR Repo: data-cartel/omnix PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-03-21T15:47:14.571Z Learning: Applies to **/*.nix : Options must live under the `omnix.*` namespaceLearnt from: CR Repo: data-cartel/omnix PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-03-21T15:47:14.571Z Learning: omnix must never contain dev shell configurationLearnt from: CR Repo: data-cartel/omnix PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-03-21T15:47:14.571Z Learning: Applies to **/*.nix : Every module must be independently usable (no implicit dependencies on other omnix modules)examples/full/config/staging-api.toml (1)
1-2: Looks good.The config is minimal and unambiguous for the staging API port.
templates/do-service/os.nix (1)
11-11: State version update is clean.Line 11 is a straightforward template baseline bump with no local integration issues in this file.
examples/minimal/os.nix (1)
6-14: Good minimal example shape.The module composition is coherent for a barebones droplet profile and stays focused on the intended minimal surface.
scripts/bootstrap.test.nu (1)
6-24: Nice targeted coverage forupdate-keys-nix.Both single-line and multiline fixtures validate the replacement behavior on realistic key layouts.
scripts/common.test.nu (1)
6-28: Good domain-focused test matrix.This set checks the key invariants for identity parsing, including defaults and argument passthrough.
scripts/deploy.nu (1)
39-41: No change required;appendcorrectly unpacks lists.The code at lines 39–41 correctly merges argument lists. In Nushell,
appendunpacks the list elements into the input list (not nested), so$args | append $ctx.rest | append $targetproduces a flat list of arguments suitable for passing to the external^deploycommand. This pattern is idiomatic and equivalent to using++.> Likely an incorrect or invalid review comment.
| localSystem, | ||
| }: | ||
| let | ||
| shell = import ./shell.nix { inherit pkgs scriptsDir; }; |
There was a problem hiding this comment.
Deploy module importing from the shell module smells weird. Thoughts @CodeRabbit?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the outer function signatures and shell.nix import patterns across lib files
rg -n "import ./shell.nix\|scriptsDir\|mkNuScript\|{ deploy-rs\|{ pkgs, scriptsDir" --type=nix -A2 -B2
Length of output: 203
🏁 Script executed:
#!/bin/bash
# Check shell.nix import patterns across lib files
echo "=== shell.nix imports ==="
rg -n "import ./shell.nix" --type=nix -A2 -B3
echo ""
echo "=== scriptsDir usage ==="
rg -n "scriptsDir" --type=nix -B2 -A2
echo ""
echo "=== outer function signatures ==="
rg -n "deploy-rs\|mkNuScript" --type=nix -B1 -A1
Length of output: 2140
@0xgleb, it's a reasonable smell to flag. There are actually two distinct issues layered here:
1. scriptsDir is misplaced in the outer let
scriptsDir = ../scripts lives at line 52 in the outer let block, but is only used inside wrappers. That's why the import looks structurally odd compared to terraform.nix and bootstrap.nix — those two have pkgs as a top-level argument, so they can import shell.nix at the module's top level. deploy.nix's pkgs only appears inside wrappers, forcing a split. Moving scriptsDir into wrappers resolves that:
wrappers =
{
pkgs,
infraPkgs,
localSystem,
}:
let
scriptsDir = ../scripts; # ← moved here, where it's actually used
shell = import ./shell.nix { inherit pkgs scriptsDir; };
inherit (shell) mkNuScript;
...
in2. Deeper: direct import vs. dependency injection
More fundamentally, deploy-rs is injected as an argument (dependency injection), but shell.nix is directly imported (tight coupling). For consistency, mkNuScript — or a pre-configured shell lib — could be injected the same way:
# Option A: inject mkNuScript into wrappers
wrappers = { pkgs, infraPkgs, localSystem, mkNuScript }:
# Option B: inject at outer function level alongside deploy-rs
{ deploy-rs, mkNuScript }:The direct import is pragmatic and fine for a sibling lib/ utility, but if shell.nix ever grows or gets tested independently, injection would make the dependency boundary explicit and consistent with the rest of the architecture.
The immediate fix (moving scriptsDir into wrappers) is low-risk and worth doing now. The injection refactor could be a follow-up.
🧠 Learnings used
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
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.571Z
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.571Z
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.571Z
Learning: Consumer repos using devenv dev shells require `nix flake check --impure` (devenv needs impure evaluation to resolve the working directory)
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Use `lib.mkDefault` for overridable defaults
There was a problem hiding this comment.
@0xgleb, solid structure overall — the test discovery pattern (scope commands | where name starts-with "test "), subprocess isolation, use std/assert, and the parallel bootstrap.test.nu naming all match the AGENTS.md conventions nicely. The two fixture variants (single-line vs. multi-line indented) give good coverage of the regex-replacement logic in update-keys-nix.
Two issues worth fixing before this is considered production-quality:
1. Temp files leak on assertion failure
rm -f $fixture is a bare final statement — if any assert throws, the cleanup line is never reached. Wrap each test body in try/catch:
def "test update-keys-nix single-line" [] {
let fixture = (mktemp)
try {
'{ host = "ssh-ed25519 AAAA_old_key"; }' | save -f $fixture
update-keys-nix $fixture "ssh-ed25519 AAAA_new_key"
let content = (open $fixture --raw)
assert ($content | str contains "ssh-ed25519 AAAA_new_key")
assert not ($content | str contains "AAAA_old_key")
} catch { |err|
rm -f $fixture
error make $err.value
}
rm -f $fixture
}Same pattern for the multi-line test.
2. Test failures are silently swallowed
nu -c exits non-zero when an assertion fails, but the for loop never inspects the exit code — so "all N tests passed" prints even when tests failed. Use | complete to capture the result and fail explicitly:
for test_name in $tests {
print $" running: ($test_name)"
let result = (
nu -c $"use std/assert; use ($bootstrap_path) *; source ($test_path); ($test_name)"
| complete
)
if $result.exit_code != 0 {
error make { msg: $"FAILED: ($test_name)\n($result.stderr)" }
}
}Minor (optional)
Consider adding a negative test — e.g. confirm update-keys-nix errors (or is a no-op) when the fixture contains no ssh-ed25519 key, to guard against silent no-replacement bugs. The bootstrap script validates the replacement and calls error make, so this code path is exercised in production but not yet in the test suite.
🧠 Learnings used
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Each module should evaluate without errors in isolation
There was a problem hiding this comment.
Gotta get rid of catch all modules ideally
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 114-120: The AGENTS.md entries for the bootstrap and common agents
still state testing is integration-only; update the bootstrap
responsibilities/public interface/testing text to reference the new direct test
entrypoints scripts/bootstrap.test.nu and scripts/common.test.nu (and note that
tests now exist for direct unit-style invocation), and adjust the "Testing" and
"Failure modes" lines for both the bootstrap and common sections to indicate
they have direct test coverage via these test scripts in addition to integration
tests so contributors know to extend coverage via those .test.nu entrypoints.
In `@flake.nix`:
- Around line 124-132: Add a smoke check that evaluates the exported template
(templates/do-service) under nix by extending the checks set: create a new entry
(e.g., checks.template-do-service or checks.do-service-template) that runs a
minimal build/evaluation of the template output (for example by invoking nix
flake check or evaluating the flake output for the template with pkgs.runCommand
or nix eval) to ensure the template scaffolding is valid; update the checks
block alongside existing checks.examples, checks.example-minimal,
checks.example-single-service, and checks.example-full so the template is
exercised by nix flake check and will fail CI on regressions.
- Around line 37-42: The example system checks build for non-Linux hosts because
mkExample hardcodes system = "x86_64-linux" while perSystem iterates all
systems; wrap the example/checks definitions inside a Linux-only guard or move
them out of perSystem: locate the perSystem block and the mkExample function and
wrap the checks.example-* (and similar blocks at the other noted locations) with
lib.mkIf pkgs.stdenv.isLinux (or relocate mkExample and its checks to a
top-level Linux-only section) so Darwin systems no longer attempt to evaluate
Linux NixOS closures.
In `@lib/deploy.nix`:
- Around line 79-85: The site profilePath currently reuses the same on-disk path
as services (profilePath = "${profileBase}/${name}") causing deployment
clobbering; update the code that builds site entries (the map producing name =
"site:${name}", value = { path = mkSiteProfile name staticSites.${name}.package;
profilePath = ... }) to use a distinct path (e.g. prefix or namespace the
directory like "${profileBase}/site-${name}" or "${profileBase}/site/${name}")
so "site:${name}" profiles do not share the same profilePath as services.
In `@lib/terraform.nix`:
- Around line 14-19: The shared runtimeInputs used by tfEditVars is missing
essential CLI tools; update the runtimeInputs list (used by tfEditVars) to
include pkgs.coreutils (for chmod) and a guaranteed editor such as pkgs.vim or
pkgs.ed so the edit-vars path in scripts/terraform.nu can run in a pure wrapper
PATH; ensure both pkgs.coreutils and your chosen editor are added to the
runtimeInputs array so chmod and vi fallback work reliably.
In `@modules/static-sites.nix`:
- Around line 81-83: The tmpfiles rule is pre-creating each site path so
lib/deploy.nix cannot atomically ln -sfn the package into ${siteBase}/${name};
remove the per-site "d ${siteBase}/${name} ..." entries in
systemd.tmpfiles.rules and instead ensure only the parent ${siteBase} directory
is created (e.g., a single tmpfiles rule for ${siteBase} with 0755 root root -),
leaving individual ${siteBase}/${name} paths absent so lib/deploy.nix can
create/swap symlinks as expected.
In `@scripts/bootstrap.nu`:
- Around line 41-48: The update-keys-nix function currently replaces any host
assignment globally which can overwrite the wrong node; modify update-keys-nix
to accept a config_name (node/attr name) parameter and scope the replacement to
only the host line inside that node's stanza. Locate the stanza for config_name
(using a regex or parser that matches the node/attribute block containing
config_name) and perform the host value replace only within that block, then
update the $expected check to verify the scoped replacement succeeded for that
config_name; keep the function name update-keys-nix and adjust its parameters
and replacement regex/logic accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4238383b-96de-4f7c-bd29-d54375516b76
📒 Files selected for processing (11)
AGENTS.mdCHANGELOG.mdexamples/full/os.nixflake.nixlib/deploy.nixlib/terraform.nixmodules/static-sites.nixscripts/bootstrap.nuscripts/common.nuscripts/terraform.nutemplates/do-service/flake.nix
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.nix
📄 CodeRabbit inference engine (AGENTS.md)
**/*.nix: Uselib.mkOptionwith proper types for all module options
Every module must be independently usable (no implicit dependencies on other omnix modules)
Options must live under theomnix.*namespace
Uselib.mkIf cfg.enablefor conditional configuration
Uselib.mkDefaultfor overridable defaults
Every configurable value must be a module option with a type and description
Preferlib.types.stroverlib.types.pathfor values that may come from terraform or other dynamic sources
Group related options under a singleomnix.<module>prefix
Functions take an attrset of required parameters
Return an attrset of derivations or values from library functions
Usepkgs.writeShellApplicationfor shell scripts (not rawwriteScript)
IncluderuntimeInputsin shell scripts and never assume tools are on PATH
All shell scripts must useset -eo pipefail
When changing module option interfaces, preserve the old option withlib.mkRenamedOptionModuleorlib.mkRemovedOptionModulewith a clear migration message
Files:
templates/do-service/flake.nixmodules/static-sites.nixflake.nixexamples/full/os.nixlib/deploy.nixlib/terraform.nix
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document agent implementations and their responsibilities in AGENTS.md
Files:
AGENTS.md
*
⚙️ 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:
AGENTS.mdCHANGELOG.mdflake.nix
**/*.md
⚙️ CodeRabbit configuration file
Focus on the contents of the docs and not on cosmetic things like markdown formatting. We use markdown files for various docs including but not limited to the north star system specification, SPEC.md, the plan for how to get there, ROADMAP.md, guidelines for AI contributors, AGENTS.md, project overview and instructions for human contributors, README.md. Think about the target audience of a document when deciding what comment to leave. For specifications and designs, suggest potential product, architectural, and UI/UX improvements. For plans, suggest changes that would make things more parallelizable and deliverable-focused. For instructions, suggest better rules and guidelines and point out missing instructions. In all cases, flag needless bloat, prefer clear concise writing, and consider the structure of the document and order of the sections
Files:
AGENTS.mdCHANGELOG.md
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Template projects should pass `nix flake check`
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
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.571Z
Learning: Consumer repos using devenv dev shells require `nix flake check --impure` (devenv needs impure evaluation to resolve the working directory)
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : All shell scripts must use `set -eo pipefail`
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Template projects should pass `nix flake check`
Applied to files:
templates/do-service/flake.nixflake.nixlib/deploy.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:46:59.282Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T15:46:59.282Z
Learning: Applies to AGENTS.md : Document agent implementations and their responsibilities in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Use `pkgs.writeShellApplication` for shell scripts (not raw `writeScript`)
Applied to files:
AGENTS.mdscripts/bootstrap.nulib/deploy.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:46:59.282Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T15:46:59.282Z
Learning: Maintain clear agent architecture documentation
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : All shell scripts must use `set -eo pipefail`
Applied to files:
AGENTS.mdlib/deploy.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Include `runtimeInputs` in shell scripts and never assume tools are on PATH
Applied to files:
AGENTS.mdflake.nixlib/deploy.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Every module must be independently usable (no implicit dependencies on other omnix modules)
Applied to files:
AGENTS.mdmodules/static-sites.nixCHANGELOG.mdflake.nixexamples/full/os.nixlib/deploy.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Options must live under the `omnix.*` namespace
Applied to files:
AGENTS.mdmodules/static-sites.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Functions take an attrset of required parameters
Applied to files:
AGENTS.mdlib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Never suppress Nix evaluation errors or warnings
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Every configurable value must be a module option with a type and description
Applied to files:
AGENTS.mdmodules/static-sites.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: omnix must never contain frontend build logic
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: omnix must never contain CI pipeline definitions
Applied to files:
AGENTS.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Group related options under a single `omnix.<module>` prefix
Applied to files:
modules/static-sites.nixCHANGELOG.mdexamples/full/os.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
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/static-sites.nixCHANGELOG.md
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Use `lib.mkOption` with proper types for all module options
Applied to files:
modules/static-sites.nixlib/deploy.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Use `lib.mkIf cfg.enable` for conditional configuration
Applied to files:
modules/static-sites.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Consumer repos using devenv dev shells require `nix flake check --impure` (devenv needs impure evaluation to resolve the working directory)
Applied to files:
flake.nixlib/deploy.nixlib/terraform.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Applies to **/*.nix : Use `lib.mkDefault` for overridable defaults
Applied to files:
flake.nix
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Keep modules focused on a single concern
Applied to files:
scripts/common.nu
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
Learning: Each module should evaluate without errors in isolation
Applied to files:
scripts/common.nu
📚 Learning: 2026-03-21T15:47:14.571Z
Learnt from: CR
Repo: data-cartel/omnix PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-21T15:47:14.571Z
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:
lib/deploy.nixlib/terraform.nix
🔇 Additional comments (4)
lib/terraform.nix (2)
62-76: Verify thatrekey = nullnever leaks into flake packages.Returning
nullhere is only safe if every consumer filters it out. Anypackagesoutput that directly inheritsrekeywill makenix flake checkfail because flake packages must be derivations, notnull. Based on learnings, "Template projects should passnix flake check".#!/bin/bash set -euo pipefail echo "Current rekey export in lib/terraform.nix:" sed -n '62,76p' lib/terraform.nix echo echo "Flake outputs that mention rekey or packages:" fd '^flake\.nix$' -x bash -lc ' file="$1" echo "== $file ==" rg -n -C3 "\brekey\b|packages\s*=" "$file" ' bash {}Expected result: every flake
packagesblock that mentionsrekeyguards the disabled case instead of exportingnull.
21-27: Nice refactor:mkTfTasknow takes an attrset.This makes the helper easier to extend without touching every call site. As per coding guidelines "Functions take an attrset of required parameters".
examples/full/os.nix (1)
59-64: Nice: staging is reachable without hidden firewall coupling.
allowedTCPPorts = [ 8080 ]keeps the example self-contained for the non-default site. Based on learnings, "Every module must be independently usable (no implicit dependencies on other omnix modules)".scripts/terraform.nu (1)
42-53: Good: the saved plan now survives a successfulplan.Using
cleanupon Line 52 removes decrypted inputs without deletingtfplan, somain applycan still consume it.
| systems = [ | ||
| "x86_64-linux" | ||
| "aarch64-linux" | ||
| "x86_64-darwin" | ||
| "aarch64-darwin" | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the flake.nix file to see the context
head -150 flake.nix | tail -120Repository: data-cartel/omnix
Length of output: 3670
Gate the example system builds to Linux-only checks.
perSystem runs for all systems (including Darwin), but mkExample hardcodes system = "x86_64-linux". This causes checks.example-* on Darwin systems to evaluate Linux NixOS closures, which is incorrect. Guard these checks with lib.mkIf pkgs.stdenv.isLinux within the perSystem block, or move them outside of perSystem entirely.
Also applies to: 83-86, 130-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` around lines 37 - 42, The example system checks build for
non-Linux hosts because mkExample hardcodes system = "x86_64-linux" while
perSystem iterates all systems; wrap the example/checks definitions inside a
Linux-only guard or move them out of perSystem: locate the perSystem block and
the mkExample function and wrap the checks.example-* (and similar blocks at the
other noted locations) with lib.mkIf pkgs.stdenv.isLinux (or relocate mkExample
and its checks to a top-level Linux-only section) so Darwin systems no longer
attempt to evaluate Linux NixOS closures.
1 participant
Summary by CodeRabbit
New Features
Documentation
Infrastructure
Tests