fix(dotfiles-updater): skip nix-daemon restart in automated updates - #425
Conversation
The dotfiles-updater service was failing because it tried to restart nix-daemon with sudo during automated updates. This is unnecessary since the daemon should already be running. Changes: - Add AUTOMATED_UPDATE=true environment variable to the service - Skip nix-daemon restart when AUTOMATED_UPDATE is set - Similar to existing CI and Docker environment handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe changes introduce an AUTOMATED_UPDATE environment variable to identify automated update scenarios. The Makefile's nix-connect target adds a conditional check for this variable alongside CI and IN_DOCKER, with corresponding messaging updates. The home-manager dotfiles-updater service converts its Environment specification from a single string to a list format and includes the new AUTOMATED_UPDATE=true variable. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello @shunkakinoki, 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 provides a crucial fix for the automated dotfiles update mechanism. By introducing a specific environment variable for automated runs, the system can now intelligently bypass an unnecessary and privilege-escalation-prone attempt to restart the Nix daemon. This ensures that automated updates can complete successfully without encountering permission errors, streamlining the maintenance process. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Mesa DescriptionTL;DRFixes the 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 addresses a failure in the dotfiles-updater service during automated updates on Linux by introducing an AUTOMATED_UPDATE environment variable to skip restarting the nix-daemon. The changes in the Makefile and the systemd service definition in home-manager/services/dotfiles-updater/default.nix are logical and correctly implement the fix for Linux. The change to use a list for the Environment attribute in the Nix configuration is also a good improvement for correctness.
However, the fix is incomplete as it doesn't cover the Darwin (macOS) case, where a similar sudo call is made and will likely fail in an automated context. I've added a high-severity comment with a suggestion to extend the fix to Darwin to ensure cross-platform consistency and prevent failures.
| elif [ "$(OS)" = "Linux" ]; then \ | ||
| if [ "$$CI" = "true" ] || [ "$$IN_DOCKER" = "true" ]; then \ | ||
| echo "🏃♂️ Nix daemon management (e.g., systemctl) is skipped in CI/Docker environments."; \ | ||
| if [ "$$CI" = "true" ] || [ "$$IN_DOCKER" = "true" ] || [ "$$AUTOMATED_UPDATE" = "true" ]; then \ |
There was a problem hiding this comment.
This condition correctly adds the check for automated updates on Linux, but the same problem of using sudo in an automated context exists for Darwin on lines 173-174. The sudo launchctl commands will likely fail without a password.
To ensure automated updates work on both platforms, you should also handle the Darwin case. This would involve:
- Adding a similar check for
$$AUTOMATED_UPDATEaround thesudo launchctlcommands for Darwin. - Updating
home-manager/services/dotfiles-updater/default.nixto setAUTOMATED_UPDATE = "true";in theEnvironmentfor thelaunchd.agents.dotfiles-updateras well.
For example, you could modify the nix-connect target for Darwin like this:
@if [ "$(OS)" = "Darwin" ]; then \
if [ "$$AUTOMATED_UPDATE" = "true" ]; then \
echo "🏃♂️ Nix daemon management is skipped in automated environments."; \
else \
sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist 2>/dev/null || true; \
sudo launchctl load -w /Library/LaunchDaemons/org.nixos.nix-daemon.plist; \
fi; \
...![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
home-manager/services/dotfiles-updater/default.nix (1)
6-31: AddAUTOMATED_UPDATE=trueto Darwin launchd environment to skip nix-daemon restart during automated updates.Darwin's launchd agent configuration lacks the
AUTOMATED_UPDATE=trueenvironment variable that the Linux systemd service now uses. When the automated update runsmake install, it invokes the Makefile'snix-connecttarget. The Darwin variant unconditionally executessudo launchctlcommands to restart the nix-daemon, whereas the Linux variant skips this step in automated contexts. Since the launchd agent runs as a user (not root), this sudo call will fail without an interactive password prompt. AddAUTOMATED_UPDATE = "true";to the Environment attribute set to prevent the nix-daemon restart, matching the behavior of the Linux systemd service.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Makefile(1 hunks)home-manager/services/dotfiles-updater/default.nix(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.nix
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.nix: Use nixfmt for formatting all Nix files
Document complex configurations with comments in Nix files
**/*.nix: Use 2 spaces for indentation in Nix files
Keep line length under 100 characters in Nix files
Sort attribute sets alphabetically in Nix files
Use consistent spacing around operators in Nix files
Format lists and sets consistently in Nix filesUse treefmt.toml for formatting Nix files
**/*.nix: UsemkOptionfor configurable options in Nix modules
Implement proper typing for all options in Nix modules
Follow the Nix expression language style guide
Files:
home-manager/services/dotfiles-updater/default.nix
**/default.nix
📄 CodeRabbit inference engine (CLAUDE.md)
Use
default.nixfiles for module exports
Files:
home-manager/services/dotfiles-updater/default.nix
home-manager/services/*/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Service configurations should be located in
home-manager/services/<name>/with proper service definitions and correct dependency handling
Files:
home-manager/services/dotfiles-updater/default.nix
home-manager/services/**/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Service configurations must include proper service definitions, handle dependencies correctly, and document service parameters
Files:
home-manager/services/dotfiles-updater/default.nix
home-manager/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
home-manager/**/*.nix: Use typed options whenever possible in Nix configurations
Document all configuration options in Nix modules and programs
Follow home-manager's module structure and keep configurations modular
Use proper indentation and formatting in Nix configuration files
Files:
home-manager/services/dotfiles-updater/default.nix
home-manager/services/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/nix.mdc)
Service configurations in
home-manager/services/should follow systemd service conventions, include proper service dependencies, and have clear documentation for service parameters
Files:
home-manager/services/dotfiles-updater/default.nix
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Document all major changes in Nix configurations
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/programs/**/*.nix : Follow program-specific best practices in program configuration files
📚 Learning: 2025-11-25T09:35:01.066Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/nix.mdc:0-0
Timestamp: 2025-11-25T09:35:01.066Z
Learning: Applies to home-manager/services/**/*.nix : Service configurations in `home-manager/services/` should follow systemd service conventions, include proper service dependencies, and have clear documentation for service parameters
Applied to files:
home-manager/services/dotfiles-updater/default.nix
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/services/**/default.nix : Service configurations must include proper service definitions, handle dependencies correctly, and document service parameters
Applied to files:
home-manager/services/dotfiles-updater/default.nix
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/services/*/default.nix : Service configurations should be located in `home-manager/services/<name>/` with proper service definitions and correct dependency handling
Applied to files:
home-manager/services/dotfiles-updater/default.nix
📚 Learning: 2025-11-25T09:34:40.062Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Document all major changes in Nix configurations
Applied to files:
home-manager/services/dotfiles-updater/default.nixMakefile
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/programs/**/*.nix : Follow program-specific best practices in program configuration files
Applied to files:
home-manager/services/dotfiles-updater/default.nix
⏰ 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). (15)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Agent
- GitHub Check: nix-nixos
- GitHub Check: nix-linux
- GitHub Check: nix-darwin
- GitHub Check: lua-hammerspoon
- GitHub Check: lua-neovim
- GitHub Check: lua-neovim-test
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: shellcheck
- GitHub Check: shellspec
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
🔇 Additional comments (2)
Makefile (1)
176-183: LGTM! Clean implementation of automated update skip logic.The conditional logic correctly adds
AUTOMATED_UPDATEalongside the existingCIandIN_DOCKERchecks, following the established pattern. The informational message at lines 181-183 provides helpful context for debugging automated update scenarios.home-manager/services/dotfiles-updater/default.nix (1)
39-55: Environment list format is correct.The conversion from attribute set to list format is the proper systemd convention for setting multiple environment variables. The addition of
AUTOMATED_UPDATE=truecorrectly enables the Makefile's conditional skip logic for nix-daemon restart.
| pkgs.which | ||
| ] | ||
| }" | ||
| "AUTOMATED_UPDATE=true" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add documentation for the AUTOMATED_UPDATE variable.
As per coding guidelines for Nix service configurations, document what this environment variable controls and why it's necessary.
];
ExecStart = "${./update.sh}";
+ # AUTOMATED_UPDATE=true signals make nix-connect to skip daemon restart
+ # Prevents sudo permission errors in automated systemd user service context
};
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In home-manager/services/dotfiles-updater/default.nix around line 54, add an
inline Nix comment immediately above or beside the "AUTOMATED_UPDATE=true" entry
that documents what AUTOMATED_UPDATE controls (toggles the service's automatic
periodic update behavior for dotfiles), the expected values (e.g., "true" or
"false"), and why it's required for automated updates to run (enables the
updater timer/activation). Keep the comment short (1–2 lines), use Nix comment
syntax (#), and include a brief example of turning it off if needed.
There was a problem hiding this comment.
Pull request overview
This PR fixes a permission issue in the dotfiles-updater systemd service by skipping nix-daemon restart operations during automated updates. The service was failing because it attempted to run sudo systemctl restart nix-daemon.service without passwordless sudo access.
Key changes:
- Added
AUTOMATED_UPDATE=trueenvironment variable to the Linux systemd service configuration - Modified the Makefile's
nix-connecttarget to check forAUTOMATED_UPDATEand skip daemon restart when set - Converted the systemd service's Environment field from string to array format to accommodate multiple environment variables
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| home-manager/services/dotfiles-updater/default.nix | Changed Environment to array format and added AUTOMATED_UPDATE=true to skip sudo operations |
| Makefile | Added AUTOMATED_UPDATE check to skip nix-daemon restart, following the same pattern as CI and Docker environments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Environment = [ | ||
| "PATH=${ | ||
| lib.makeBinPath [ | ||
| pkgs.bash | ||
| pkgs.coreutils | ||
| pkgs.curl | ||
| pkgs.gawk | ||
| pkgs.git | ||
| pkgs.gnumake | ||
| pkgs.gnused | ||
| pkgs.nix | ||
| pkgs.sudo | ||
| pkgs.which | ||
| ] | ||
| }" | ||
| "AUTOMATED_UPDATE=true" | ||
| ]; |
There was a problem hiding this comment.
The Environment field is using an array format here, which is inconsistent with other systemd services in the codebase. All other systemd.user.services in this repository use a single string format for Environment, even when setting just one variable (e.g., Environment = "PATH=..." in cliproxyapi, code-syncer, neverssl-keepalive, and ollama services).
While both formats are valid in NixOS/home-manager, consider maintaining consistency with the existing codebase pattern. For multiple environment variables, the string format can concatenate them with spaces, like: Environment = "PATH=... AUTOMATED_UPDATE=true".
| Environment = [ | |
| "PATH=${ | |
| lib.makeBinPath [ | |
| pkgs.bash | |
| pkgs.coreutils | |
| pkgs.curl | |
| pkgs.gawk | |
| pkgs.git | |
| pkgs.gnumake | |
| pkgs.gnused | |
| pkgs.nix | |
| pkgs.sudo | |
| pkgs.which | |
| ] | |
| }" | |
| "AUTOMATED_UPDATE=true" | |
| ]; | |
| Environment = "PATH=${ | |
| lib.makeBinPath [ | |
| pkgs.bash | |
| pkgs.coreutils | |
| pkgs.curl | |
| pkgs.gawk | |
| pkgs.git | |
| pkgs.gnumake | |
| pkgs.gnused | |
| pkgs.nix | |
| pkgs.sudo | |
| pkgs.which | |
| ] | |
| }: AUTOMATED_UPDATE=true"; |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Summary
Fixes the dotfiles-updater service failing due to attempting to restart nix-daemon with sudo during automated updates.
Problem
The dotfiles-updater systemd service was failing with exit code 2 because:
make installduring automated updatesmake installincludesmake nix-connectnix-connecttries to runsudo systemctl restart nix-daemon.serviceSolution
AUTOMATED_UPDATE=trueenvironment variable to the dotfiles-updater serviceAUTOMATED_UPDATEis setTest Plan
🤖 Generated with Claude Code
Summary by cubic
Skip nix-daemon restarts during automated updates to prevent dotfiles-updater failures. Adds an AUTOMATED_UPDATE env flag and treats automated runs like CI/Docker.
Written for commit dd87c85. Summary will update automatically on new commits.