feat(hm): add syncthing service and enhance nix config - #1259
Conversation
- Add syncthing as a home-manager service (Linux-only via services.syncthing) - Upgrade nix config: lix package, registry pinning, full substituter set, trusted-public-keys, use-xdg-base-directories, and gc.automatic - Flatten home-manager/misc/nix → home-manager/nix (remove misc wrapper) - Move NixOS-only nix settings to matic (channel.enable, nix-path, trusted-users, environment.etc nix/inputs/nixpkgs) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughWalkthroughReplaced the old misc/nix module with a dedicated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Syncthing service for Home Manager on Linux and significantly refines the Nix configuration across Home Manager and NixOS. The changes aim to improve the development environment by leveraging Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Mesa DescriptionTL;DRAdds What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the syncthing service for Linux hosts and significantly enhances the Nix configuration within home-manager. The changes include switching to the lix package, pinning the nixpkgs registry to the flake input, and adding a comprehensive set of binary caches, which are all great improvements for consistency and performance. The configuration is also nicely refactored by removing an unnecessary misc directory.
My review includes a suggestion to refine the syncthing service activation to be more targeted, enabling it only on desktop environments to avoid unintentionally running it on headless servers.
| sshAgent = import ./ssh-agent { | ||
| inherit config lib pkgs; | ||
| }; | ||
| syncthing = import ./syncthing { inherit lib pkgs; }; |
There was a problem hiding this comment.
To allow the syncthing module to be conditionally enabled based on host properties, please pass the config attribute here. This change is related to the suggestion on home-manager/services/syncthing/default.nix.
syncthing = import ./syncthing { inherit config lib pkgs; };
| { pkgs, lib, ... }: | ||
| { | ||
| services.syncthing = lib.mkIf pkgs.stdenv.isLinux { | ||
| enable = true; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Enabling syncthing for all Linux hosts might be too broad, as it could be unintentionally enabled on headless servers. It would be more robust to enable it only on desktop systems by checking config.isDesktop.
To implement this, you'll also need to pass config to this module from home-manager/services/default.nix.
{ pkgs, lib, config, ... }:
{
services.syncthing = lib.mkIf (pkgs.stdenv.isLinux && config.isDesktop) {
enable = true;
};
}
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@home-manager/nix/default.nix`:
- Line 9: The nix expression sets package = lib.mkDefault
pkgs.lixPackageSets.latest.lix but lixPackageSets isn't provided; either add the
lix-module input and apply its overlay in flake.nix so pkgs.lixPackageSets
exists, or change the reference to use a standard package (e.g., replace
pkgs.lixPackageSets.latest.lix with pkgs.nix) so package resolves; update the
flake inputs and overlays if choosing the lix-module approach and ensure the
overlay exposes lixPackageSets, or simply point package at pkgs.nix via
lib.mkDefault to fix the attribute error.
In `@named-hosts/matic/default.nix`:
- Line 339: Update the stale inline comment that currently reads "NixOS-level
nix settings (complements home-manager misc/nix)" to reference the new module
path "home-manager/nix/" (e.g., change "home-manager misc/nix" to
"home-manager/nix/") so the comment correctly reflects the relocated module;
ensure the updated comment remains concise and in the same comment block near
the NixOS-level nix settings declaration.
🪄 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: 44c9a073-6483-4966-b695-48a51e580e7d
📒 Files selected for processing (7)
home-manager/default.nixhome-manager/misc/default.nixhome-manager/misc/nix/default.nixhome-manager/nix/default.nixhome-manager/services/default.nixhome-manager/services/syncthing/default.nixnamed-hosts/matic/default.nix
💤 Files with no reviewable changes (2)
- home-manager/misc/default.nix
- home-manager/misc/nix/default.nix
| }: | ||
| { | ||
| nix = { | ||
| package = lib.mkDefault pkgs.lixPackageSets.latest.lix; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify lix-related inputs in flake.nix
echo "=== Checking for lix input in flake.nix ==="
rg -n "lix" flake.nix || echo "No lix references found in flake.nix"
echo ""
echo "=== Checking for lixPackageSets usage in codebase ==="
rg -n "lixPackageSets" --type nixRepository: shunkakinoki/dotfiles
Length of output: 291
🏁 Script executed:
#!/bin/bash
# Examine home-manager/nix/default.nix to understand pkgs instantiation
echo "=== Content of home-manager/nix/default.nix ==="
cat home-manager/nix/default.nix
echo ""
echo "=== Checking flake.nix for any overlays ==="
head -100 flake.nix
echo ""
echo "=== Searching for any overlay or lixPackageSets definition ==="
rg -n "overlay|lixPackageSets" --type nix | head -20Repository: shunkakinoki/dotfiles
Length of output: 4979
🏁 Script executed:
#!/bin/bash
# Examine overlays/default.nix to see if lixPackageSets is provided
echo "=== Content of overlays/default.nix ==="
cat overlays/default.nix
echo ""
echo "=== Search for lixPackageSets definition or provision anywhere ==="
rg -n "lixPackageSets" . --type nixRepository: shunkakinoki/dotfiles
Length of output: 2606
Add lix input to flake.nix or use standard pkgs.nix.
Line 9 references pkgs.lixPackageSets.latest.lix, but lixPackageSets is not available. The flake.nix inputs do not include any lix-related input, and the overlays do not provide this package set. This will fail evaluation with an attribute error.
To fix:
- Add the lix-module input to flake.nix and apply its overlay, OR
- Use
pkgs.nixinstead:
- package = lib.mkDefault pkgs.lixPackageSets.latest.lix;
+ package = lib.mkDefault pkgs.nix;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@home-manager/nix/default.nix` at line 9, The nix expression sets package =
lib.mkDefault pkgs.lixPackageSets.latest.lix but lixPackageSets isn't provided;
either add the lix-module input and apply its overlay in flake.nix so
pkgs.lixPackageSets exists, or change the reference to use a standard package
(e.g., replace pkgs.lixPackageSets.latest.lix with pkgs.nix) so package
resolves; update the flake inputs and overlays if choosing the lix-module
approach and ensure the overlay exposes lixPackageSets, or simply point package
at pkgs.nix via lib.mkDefault to fix the attribute error.
There was a problem hiding this comment.
Pull request overview
Adds a Home Manager Syncthing module and refactors/extends Nix configuration, including moving Nix-related Home Manager config from home-manager/misc/nix to a new top-level home-manager/nix module and pinning nixpkgs via the registry.
Changes:
- Introduce a Home Manager Syncthing service module and wire it into
home-manager/services. - Add a new
home-manager/nixmodule (Lix package, pinnedregistry.nixpkgs, substituters/keys, XDG paths, automatic GC) and remove the oldhome-manager/misc/nix. - Update
named-hosts/maticwith additional NixOS-level Nix settings and an/etc/nix/inputs/nixpkgspin.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| named-hosts/matic/default.nix | Adjusts host-level NixOS Nix settings and pins nixpkgs under /etc/nix/inputs. |
| home-manager/services/syncthing/default.nix | Adds a Home Manager module enabling Syncthing on Linux. |
| home-manager/services/default.nix | Registers the new Syncthing module in the services import list. |
| home-manager/nix/default.nix | New consolidated Home Manager Nix configuration (registry pin, caches, GC, XDG). |
| home-manager/misc/nix/default.nix | Removes the previous Home Manager Nix configuration module. |
| home-manager/misc/default.nix | Removes the misc wrapper module list. |
| home-manager/default.nix | Switches imports from misc to the new ./nix module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "nix-command" | ||
| "flakes" | ||
| ]; | ||
| nix-path = lib.mkForce "nixpkgs=/etc/nix/inputs/nixpkgs"; |
There was a problem hiding this comment.
experimental-features (flakes + nix-command) was removed from this host’s NixOS nix.settings. Other NixOS configs in this repo still explicitly set these (e.g. hosts/nixos/default.nix), and without them nixos-rebuild --flake / flake-based nix commands can fail depending on the Nix version. Consider restoring nix.settings.experimental-features = [ "nix-command" "flakes" ]; here (or explicitly switching the system nix.package to one where flakes are enabled by default).
| nix-path = lib.mkForce "nixpkgs=/etc/nix/inputs/nixpkgs"; | |
| nix-path = lib.mkForce "nixpkgs=/etc/nix/inputs/nixpkgs"; | |
| experimental-features = [ "nix-command" "flakes" ]; |
| ]; | ||
|
|
||
| # Nix settings | ||
| # NixOS-level nix settings (complements home-manager nix/) |
There was a problem hiding this comment.
Comment still refers to home-manager misc/nix, but this PR flattens that to home-manager/nix. Updating the comment will avoid confusion when tracking where nix.conf settings come from.
| # NixOS-level nix settings (complements home-manager nix/) | |
| # NixOS-level nix settings (complements home-manager/nix) |
| services.syncthing = lib.mkIf pkgs.stdenv.isLinux { | ||
| enable = true; | ||
| }; |
There was a problem hiding this comment.
This module enables Syncthing for every Linux Home Manager config by default. That doesn’t match the PR description/test plan which are scoped to matic, and it can have unintended operational/security impact on other Linux hosts (background service, ports, state/config dirs). Consider gating it behind a host flag (e.g. inputs.host.isDesktop / inputs.host.isMatic) or making it opt-in (leave default services.syncthing.enable = false and enable it in the specific host config).
Match macOS screensaver idleTime (600s) for consistency across devices. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="config/hyprland/hypridle.conf">
<violation number="1" location="config/hyprland/hypridle.conf:25">
P2: Battery detection checks for “any offline AC input” instead of “no AC input online,” which can trigger battery-only lock behavior while plugged in on multi-input systems.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| listener { | ||
| timeout = 300 | ||
| on-timeout = loginctl lock-session | ||
| on-timeout = bash -c 'cat /sys/class/power_supply/AC*/online | grep -q 0 && loginctl lock-session' |
There was a problem hiding this comment.
P2: Battery detection checks for “any offline AC input” instead of “no AC input online,” which can trigger battery-only lock behavior while plugged in on multi-input systems.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/hyprland/hypridle.conf, line 25:
<comment>Battery detection checks for “any offline AC input” instead of “no AC input online,” which can trigger battery-only lock behavior while plugged in on multi-input systems.</comment>
<file context>
@@ -19,16 +19,29 @@ listener {
listener {
timeout = 300
- on-timeout = loginctl lock-session
+ on-timeout = bash -c 'cat /sys/class/power_supply/AC*/online | grep -q 0 && loginctl lock-session'
}
</file context>
| on-timeout = bash -c 'cat /sys/class/power_supply/AC*/online | grep -q 0 && loginctl lock-session' | |
| on-timeout = bash -c '! grep -q 1 /sys/class/power_supply/AC*/online && loginctl lock-session' |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/hyprland/hypridle.conf (1)
15-20: Minor: AC dim timeout (30 min) occurs after AC lock timeout (10 min).On AC power, the screen locks at 10 minutes but dims at 30 minutes. This means users won't get the visual dimming cue before the lock. If this is intentional (e.g., dimming is only for the lock screen itself), feel free to disregard. Otherwise, consider swapping the timeouts so dimming acts as a warning before locking.
Also applies to: 28-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/hyprland/hypridle.conf` around lines 15 - 20, The AC dim listener currently uses timeout = 1800 (30m) which fires after the 10-minute lock; change the listener timeout so dimming happens before locking (e.g., set timeout to a value less than the lock timeout such as 540 seconds) in the same listener block (identify by "listener", "timeout", "on-timeout", "on-resume") so on-timeout provides a visual warning prior to the lock while keeping the AC-only check and the on-resume behavior intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/hyprland/hypridle.conf`:
- Around line 15-20: The AC dim listener currently uses timeout = 1800 (30m)
which fires after the 10-minute lock; change the listener timeout so dimming
happens before locking (e.g., set timeout to a value less than the lock timeout
such as 540 seconds) in the same listener block (identify by "listener",
"timeout", "on-timeout", "on-resume") so on-timeout provides a visual warning
prior to the lock while keeping the AC-only check and the on-resume behavior
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85dc0579-c238-430a-8f52-902244bb4924
📒 Files selected for processing (2)
config/hyprland/hypridle.confnix-darwin/config/system.nix
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
syncthingas a home-manager service (Linux-only,services.syncthing.enable)home-manager/nix: lix package,registry.nixpkgspinned to flake input, full substituter + trusted-public-keys set,use-xdg-base-directories,gc.automatichome-manager/misc/nix/→home-manager/nix/(remove the pointlessmiscwrapper)channel.enable = false,nix-pathoverride,trusted-users,environment.etc."nix/inputs/nixpkgs"Test plan
nixos-rebuild switchon matic builds without errorshome-manager switchon all hosts builds without errors127.0.0.1:8384on maticnix registry listshowsnixpkgspinned to flake input🤖 Generated with Claude Code
Summary by cubic
Enhances
home-managerNix config for reproducible, faster builds and aligns idle/lock behavior across Linux (Hyprland) and macOS.syncthingwas removed from this PR.New Features
home-manager/nix: uselix, pinregistry.nixpkgsto flake input, XDG dirs, auto GC, full substituters/keys.idleTimeset to 600s.Refactors
home-manager/nix: move frommisc, split index/config, let-boundniximport, alphabetize let bindings.matic: disable channels, forcenix-path, settrusted-users(incl.@wheel), add/etc/nix/inputs/nixpkgs.Written for commit 1953f99. Summary will update on new commits.