chore: update - #1760
Conversation
…on AC systemd-inhibit --what=idle only blocks logind idle tracking, but noctalia detects idle via the Wayland ext-idle-notify-v1 protocol directly from Hyprland. Replace with wlinhibit which holds a Wayland idle-inhibit surface that the compositor respects.
wlinhibit needs the Wayland socket to create an idle inhibitor surface.
Wayland idle-inhibit is all-or-nothing - wlinhibit blocked lock too. New approach: noctalia only handles lock (screenOffTimeout/suspendTimeout disabled). A swayidle instance manages screen-off and suspend on battery only. On AC, swayidle is stopped so only noctalia's lock fires.
Deploy ~/.npmrc, ~/.config/pnpm/rc, and ~/.bunfig.toml with hardened defaults: 7-day minimum release age, ignore install scripts, save exact versions, omit optional deps, enforce engine constraints.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Bun, NPM, and PNPM config files plus Nix modules enforcing install policies, registers those modules in config/default.nix, and refactors noctalia idle handling from systemd-inhibit to an AC-aware swayidle-driven script with service environment and idle-timeout updates. ChangesPackage Manager Setup and Noctalia Idle Behavior
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![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 configuration files for Bun, npm, and pnpm, implementing security-oriented defaults such as ignoring scripts and enforcing minimum release ages. It also refactors the ac-idle-inhibit.sh script and its Nix service to manage swayidle dynamically based on AC power status. A review comment identifies a potential failure point in the shell script where set -e could cause the service to crash if the AC status file is missing, suggesting a more robust check for the file's existence.
| while true; do | ||
| if [ "$(cat "$AC" 2>/dev/null)" = "1" ]; then | ||
| systemd-inhibit --what=idle --why="On AC power" --mode=block sleep 2 | ||
| ON_AC="$(cat "$AC" 2>/dev/null)" |
There was a problem hiding this comment.
The script uses set -e, which causes it to exit immediately if cat "$AC" fails (e.g., if the sysfs path does not exist on a desktop or different hardware). This will lead to the systemd service failing and potentially entering a restart loop. It's better to check for the file's existence and default to a safe value (like assuming AC power) if it's missing.
| ON_AC="$(cat "$AC" 2>/dev/null)" | |
| if [ -f "$AC" ]; then | |
| ON_AC="$(cat "$AC")" | |
| else | |
| ON_AC="1" | |
| fi |
References
- Avoid using
set -ein execution scripts that must handle failures gracefully to avoid blocking or service failure loops.
![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
🧹 Nitpick comments (2)
config/noctalia/ac-idle-inhibit.sh (2)
22-28: ⚖️ Poor tradeoffOptional: switch from a 2 s polling loop to udev/upower events.
The current design works, but it wakes the CPU every 2 seconds forever and reacts to AC changes with up to a 2 s lag.
upower --monitor-detailor audevrule onpower_supplyevents would be event-driven, zero-poll, and instant. Not a blocker — flagging as a nice-to-have for battery life on laptops.🤖 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 `@config/noctalia/ac-idle-inhibit.sh` around lines 22 - 28, The current script starts swayidle and relies on a 2s polling loop to detect AC changes; replace the poll with an event-driven watcher that reacts to power_supply/UPower events (so you stop waking the CPU every 2s). Implement a small background handler that listens for UPower DBus signals (e.g. via upower --monitor-detail or a DBus subscription) or a udev monitor on power_supply events and in its handler start/stop or restart the swayidle process (manage the SWAYIDLE_PID variable and invoke swayidle -w as currently done) instead of polling; ensure the watcher runs in background (systemd user service or nohup) and properly exports/updates SWAYIDLE_PID when starting/stopping swayidle.
6-6: ⚡ Quick winHardcoded AC sysfs path reduces portability.
/sys/class/power_supply/ACAD/onlineis device-specific (the name varies per machine:AC,ACAD,ADP1,AC0, etc.). If this config is reused on different hardware,catwill fail silently,ON_ACwill be empty, and the script will permanently behave as if on battery (spawningswayidleeven when plugged in).Consider discovering the AC adapter dynamically:
♻️ Suggested refactor
-AC=/sys/class/power_supply/ACAD/online +# Pick the first Mains-type supply (AC adapter) available on this machine. +AC="$(grep -l '^Mains$' /sys/class/power_supply/*/type 2>/dev/null | head -n1)" +AC="${AC%/type}/online"🤖 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 `@config/noctalia/ac-idle-inhibit.sh` at line 6, The script currently hardcodes AC=/sys/class/power_supply/ACAD/online which breaks portability; change the detection logic so the script dynamically finds the AC adapter under /sys/class/power_supply by scanning entries and selecting the one whose "type" file equals "Mains" (or whose name matches common AC prefixes) and then use its "online" file to set ON_AC instead of AC; update references to AC (and the ON_AC check that decides whether to spawn swayidle) to use this discovered path so the script works across machines with names like AC, ACAD, ADP1, AC0, etc.
🤖 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 `@config/noctalia/ac-idle-inhibit.sh`:
- Around line 16-20: The race between the liveness check (kill -0 on
SWAYIDLE_PID) and the subsequent kill can cause the standalone kill to fail and,
under set -e, abort the script; update the block handling SWAYIDLE_PID to
tolerate a failing kill by appending a no-op on failure (e.g. change the
dangerous call to kill "$SWAYIDLE_PID" 2>/dev/null || true) so the script won't
exit if the process disappeared after the check, ensure SWAYIDLE_PID is still
cleared (SWAYIDLE_PID="") and likewise make the hyprctl dispatch dpms on call
resilient by adding || true so transient compositor errors don't terminate the
poller.
---
Nitpick comments:
In `@config/noctalia/ac-idle-inhibit.sh`:
- Around line 22-28: The current script starts swayidle and relies on a 2s
polling loop to detect AC changes; replace the poll with an event-driven watcher
that reacts to power_supply/UPower events (so you stop waking the CPU every 2s).
Implement a small background handler that listens for UPower DBus signals (e.g.
via upower --monitor-detail or a DBus subscription) or a udev monitor on
power_supply events and in its handler start/stop or restart the swayidle
process (manage the SWAYIDLE_PID variable and invoke swayidle -w as currently
done) instead of polling; ensure the watcher runs in background (systemd user
service or nohup) and properly exports/updates SWAYIDLE_PID when
starting/stopping swayidle.
- Line 6: The script currently hardcodes AC=/sys/class/power_supply/ACAD/online
which breaks portability; change the detection logic so the script dynamically
finds the AC adapter under /sys/class/power_supply by scanning entries and
selecting the one whose "type" file equals "Mains" (or whose name matches common
AC prefixes) and then use its "online" file to set ON_AC instead of AC; update
references to AC (and the ON_AC check that decides whether to spawn swayidle) to
use this discovered path so the script works across machines with names like AC,
ACAD, ADP1, AC0, etc.
🪄 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: 3eea1399-a9ba-4a83-8848-772aef4ab2c2
📒 Files selected for processing (9)
config/bun/bunfig.tomlconfig/bun/default.nixconfig/default.nixconfig/noctalia/ac-idle-inhibit.shconfig/noctalia/default.nixconfig/npm/default.nixconfig/npm/npmrcconfig/pnpm/default.nixconfig/pnpm/rc
| if [ -n "$SWAYIDLE_PID" ] && kill -0 "$SWAYIDLE_PID" 2>/dev/null; then | ||
| kill "$SWAYIDLE_PID" 2>/dev/null | ||
| SWAYIDLE_PID="" | ||
| hyprctl dispatch dpms on | ||
| fi |
There was a problem hiding this comment.
kill under set -e can terminate the script on a tiny race.
Between the kill -0 liveness check on Line 16 and the kill on Line 17, swayidle may exit on its own (or be reaped elsewhere). With set -e, a non-zero exit from kill will abort the whole AC monitor service. The same applies in the cleanup function on Line 9, except there it is shielded by the && short-circuit — the standalone kill here is not.
🛡️ Suggested guard
- if [ -n "$SWAYIDLE_PID" ] && kill -0 "$SWAYIDLE_PID" 2>/dev/null; then
- kill "$SWAYIDLE_PID" 2>/dev/null
- SWAYIDLE_PID=""
- hyprctl dispatch dpms on
- fi
+ if [ -n "$SWAYIDLE_PID" ] && kill -0 "$SWAYIDLE_PID" 2>/dev/null; then
+ kill "$SWAYIDLE_PID" 2>/dev/null || true
+ SWAYIDLE_PID=""
+ hyprctl dispatch dpms on || true
+ fiI also added || true to the hyprctl call since a transient compositor hiccup shouldn't crash the long-running poller.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -n "$SWAYIDLE_PID" ] && kill -0 "$SWAYIDLE_PID" 2>/dev/null; then | |
| kill "$SWAYIDLE_PID" 2>/dev/null | |
| SWAYIDLE_PID="" | |
| hyprctl dispatch dpms on | |
| fi | |
| if [ -n "$SWAYIDLE_PID" ] && kill -0 "$SWAYIDLE_PID" 2>/dev/null; then | |
| kill "$SWAYIDLE_PID" 2>/dev/null || true | |
| SWAYIDLE_PID="" | |
| hyprctl dispatch dpms on || true | |
| fi |
🤖 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 `@config/noctalia/ac-idle-inhibit.sh` around lines 16 - 20, The race between
the liveness check (kill -0 on SWAYIDLE_PID) and the subsequent kill can cause
the standalone kill to fail and, under set -e, abort the script; update the
block handling SWAYIDLE_PID to tolerate a failing kill by appending a no-op on
failure (e.g. change the dangerous call to kill "$SWAYIDLE_PID" 2>/dev/null ||
true) so the script won't exit if the process disappeared after the check,
ensure SWAYIDLE_PID is still cleared (SWAYIDLE_PID="") and likewise make the
hyprctl dispatch dpms on call resilient by adding || true so transient
compositor errors don't terminate the poller.
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
| @@ -0,0 +1,5 @@ | |||
| minimum-release-age=10080 | |||
There was a problem hiding this comment.
npm ignores this setting — npm's canonical key is min-release-age (not minimum-release-age), and the unit is days, not minutes (npm v11 docs; npm/cli #8965). Even after a rename, 10080 would mean ~27 years and break every install. The 7-day cooldown the commit message describes does not currently apply to npm install.
| minimum-release-age=10080 | |
| min-release-age=7 |
| @@ -0,0 +1,5 @@ | |||
| minimum-release-age=10080 | |||
There was a problem hiding this comment.
Latent on pnpm v11+: pnpm v11 restricted the global rc file to auth/registry settings — non-auth settings like minimumReleaseAge must live in pnpm-workspace.yaml (project) or ~/.config/pnpm/config.yaml (global) as minimumReleaseAge: 10080 (camelCase, minutes). Works on pnpm ≤ v10, but on v11 this file is ignored for these settings and pnpm's own default of 1440 min wins. (Refs: https://pnpm.io/11.x/configuring, https://pnpm.io/11.x/cli/config)
| # On battery: run swayidle for screen-off (5 min) and suspend (10 min). | ||
| # On AC: stop swayidle so only noctalia's lock fires. | ||
| set -euo pipefail | ||
|
|
There was a problem hiding this comment.
Hardware-specific path: /sys/class/power_supply/ACAD/online only exists on certain laptops; common alternates are AC, AC0, ADP1, ACPI. Combined with cat ... 2>/dev/null inside $(...), a missing node yields ON_AC="", which falls into the else branch — so on any other machine the script will treat AC as battery and keep swayidle running with screen-off + suspend even when plugged in (the opposite of the PR's intent). Detect the Mains entry under /sys/class/power_supply/*/type instead.
| Description = "Screen-off and suspend on battery via swayidle"; | ||
| After = [ "graphical-session.target" ]; | ||
| PartOf = [ "graphical-session.target" ]; | ||
| }; |
There was a problem hiding this comment.
hyprctl needs more than WAYLAND_DISPLAY: without HYPRLAND_INSTANCE_SIGNATURE (and XDG_RUNTIME_DIR), hyprctl falls back to scanning $XDG_RUNTIME_DIR/hypr/ and silently picks an arbitrary socket if multiple Hyprland instances exist. Consider PassEnvironment = [ "WAYLAND_DISPLAY" "HYPRLAND_INSTANCE_SIGNATURE" "XDG_RUNTIME_DIR" ]; so the user manager forwards them when present.
| }; | |
| PassEnvironment = [ "WAYLAND_DISPLAY" "HYPRLAND_INSTANCE_SIGNATURE" "XDG_RUNTIME_DIR" ]; |
| @@ -0,0 +1,7 @@ | |||
| { ... }: | |||
| { | |||
| xdg.configFile."pnpm/rc" = { | |||
There was a problem hiding this comment.
Wrong path on macOS: pnpm reads its global rc from ~/Library/Preferences/pnpm/rc on Darwin (unless XDG_CONFIG_HOME is exported), but xdg.configFile always lands at ~/.config/pnpm/rc. Since config/default.nix imports ./pnpm unconditionally, this module silently does nothing on macOS hosts. Branch on pkgs.stdenv.isDarwin or gate the import to Linux.
| minimumReleaseAge = 604800 | ||
| exact = true | ||
| ignoreScripts = true | ||
| optional = false |
There was a problem hiding this comment.
optional = false skips required native binaries: rollup, esbuild, swc, lightningcss, sharp, and many other modern toolchain packages ship per-arch binaries via optionalDependencies. Disabling optionals globally lets bun install succeed but breaks the apps at runtime with Cannot find module @rollup/rollup-linux-x64-gnu (or equivalent). Same applies to the corresponding omit=optional lines in config/npm/npmrc and config/pnpm/rc.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 9 files
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/noctalia/ac-idle-inhibit.sh">
<violation number="1" location="config/noctalia/ac-idle-inhibit.sh:13">
P1: The AC-state read can terminate the whole script under `set -e` when the sysfs file read fails. Add a fallback so transient read errors don’t stop idle management.</violation>
</file>
<file name="config/pnpm/rc">
<violation number="1" location="config/pnpm/rc:1">
P1: In pnpm v11+, non-registry/auth settings in the global `rc` file are silently ignored. To ensure these security defaults are actually enforced upon upgrade, use `~/.config/pnpm/config.yaml` instead, formatted as YAML (e.g., `minimumReleaseAge: 10080`, `ignoreScripts: true`).</violation>
</file>
<file name="config/npm/npmrc">
<violation number="1" location="config/npm/npmrc:1">
P1: Wrong configuration key and unit. npm's setting is `min-release-age` (not `minimum-release-age`) and its unit is **days**, not minutes. The current value `10080` would be silently ignored since `minimum-release-age` isn't a recognized npm config key. For a 7-day cooldown, use `min-release-age=7`.</violation>
</file>
<file name="config/bun/bunfig.toml">
<violation number="1" location="config/bun/bunfig.toml:5">
P1: `optional = false` will break packages that ship platform-specific native binaries via `optionalDependencies`. Many critical toolchain packages (rollup, esbuild, swc, lightningcss, sharp) use this pattern — e.g. `@rollup/rollup-linux-x64-gnu` is an optional dep. Install will succeed but the packages will fail at runtime with missing module errors. The same concern applies to `omit=optional` in the npmrc and pnpm rc files.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| while true; do | ||
| if [ "$(cat "$AC" 2>/dev/null)" = "1" ]; then | ||
| systemd-inhibit --what=idle --why="On AC power" --mode=block sleep 2 | ||
| ON_AC="$(cat "$AC" 2>/dev/null)" |
There was a problem hiding this comment.
P1: The AC-state read can terminate the whole script under set -e when the sysfs file read fails. Add a fallback so transient read errors don’t stop idle management.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/noctalia/ac-idle-inhibit.sh, line 13:
<comment>The AC-state read can terminate the whole script under `set -e` when the sysfs file read fails. Add a fallback so transient read errors don’t stop idle management.</comment>
<file context>
@@ -1,14 +1,31 @@
while true; do
- if [ "$(cat "$AC" 2>/dev/null)" = "1" ]; then
- systemd-inhibit --what=idle --why="On AC power" --mode=block sleep 2
+ ON_AC="$(cat "$AC" 2>/dev/null)"
+
+ if [ "$ON_AC" = "1" ]; then
</file context>
| ON_AC="$(cat "$AC" 2>/dev/null)" | |
| ON_AC="$(cat "$AC" 2>/dev/null || echo 0)" |
| @@ -0,0 +1,5 @@ | |||
| minimum-release-age=10080 | |||
There was a problem hiding this comment.
P1: In pnpm v11+, non-registry/auth settings in the global rc file are silently ignored. To ensure these security defaults are actually enforced upon upgrade, use ~/.config/pnpm/config.yaml instead, formatted as YAML (e.g., minimumReleaseAge: 10080, ignoreScripts: true).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/pnpm/rc, line 1:
<comment>In pnpm v11+, non-registry/auth settings in the global `rc` file are silently ignored. To ensure these security defaults are actually enforced upon upgrade, use `~/.config/pnpm/config.yaml` instead, formatted as YAML (e.g., `minimumReleaseAge: 10080`, `ignoreScripts: true`).</comment>
<file context>
@@ -0,0 +1,5 @@
+minimum-release-age=10080
+ignore-scripts=true
+save-exact=true
</file context>
| @@ -0,0 +1,5 @@ | |||
| minimum-release-age=10080 | |||
There was a problem hiding this comment.
P1: Wrong configuration key and unit. npm's setting is min-release-age (not minimum-release-age) and its unit is days, not minutes. The current value 10080 would be silently ignored since minimum-release-age isn't a recognized npm config key. For a 7-day cooldown, use min-release-age=7.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/npm/npmrc, line 1:
<comment>Wrong configuration key and unit. npm's setting is `min-release-age` (not `minimum-release-age`) and its unit is **days**, not minutes. The current value `10080` would be silently ignored since `minimum-release-age` isn't a recognized npm config key. For a 7-day cooldown, use `min-release-age=7`.</comment>
<file context>
@@ -0,0 +1,5 @@
+minimum-release-age=10080
+ignore-scripts=true
+save-exact=true
</file context>
| minimum-release-age=10080 | |
| min-release-age=7 |
| minimumReleaseAge = 604800 | ||
| exact = true | ||
| ignoreScripts = true | ||
| optional = false |
There was a problem hiding this comment.
P1: optional = false will break packages that ship platform-specific native binaries via optionalDependencies. Many critical toolchain packages (rollup, esbuild, swc, lightningcss, sharp) use this pattern — e.g. @rollup/rollup-linux-x64-gnu is an optional dep. Install will succeed but the packages will fail at runtime with missing module errors. The same concern applies to omit=optional in the npmrc and pnpm rc files.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/bun/bunfig.toml, line 5:
<comment>`optional = false` will break packages that ship platform-specific native binaries via `optionalDependencies`. Many critical toolchain packages (rollup, esbuild, swc, lightningcss, sharp) use this pattern — e.g. `@rollup/rollup-linux-x64-gnu` is an optional dep. Install will succeed but the packages will fail at runtime with missing module errors. The same concern applies to `omit=optional` in the npmrc and pnpm rc files.</comment>
<file context>
@@ -0,0 +1,5 @@
+minimumReleaseAge = 604800
+exact = true
+ignoreScripts = true
+optional = false
</file context>
| optional = false | |
| # optional = false # WARNING: breaks packages using optionalDependencies for native binaries (rollup, esbuild, swc, etc.) |
1 participant
Summary
Test plan
make buildandmake switchsucceedac-idle-inhibitservice running on AC (no swayidle spawned)~/.npmrc,~/.config/pnpm/rc,~/.bunfig.tomldeployed with correct content🤖 Generated with Claude Code
Summary by cubic
Fixes Wayland idle behavior so the screen stays on when plugged in, while still locking. Adds hardened defaults for
npm,pnpm, andbunto reduce supply-chain risk.Bug Fixes
swayidlehandles screen-off (5m) and suspend (10m). AC: stopswayidleso only lock runs and screen stays on.WAYLAND_DISPLAY, set PATH forswayidle,hyprctl, and core utils; restart on failure.screenOffTimeout=0,suspendTimeout=0,lockTimeout=5m._; format Nix files.New Features
npm(~/.npmrc),pnpm(~/.config/pnpm/rc),bun(~/.bunfig.toml).Written for commit f0d2e62. Summary will update on new commits.