Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions home-manager/programs/fish/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
v = "nvim";

# Function-based abbreviations
caf = "_caf_function";
cawxe = "_cawxe_function";
cawxeh = "_cawxeh_function";
caxe = "_caxe_function";
Expand Down Expand Up @@ -226,6 +227,7 @@
})
[
"__tmux_bootstrap_default_session"
"_caf_function"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The repository has a test coverage spec (spec/fish_functions_coverage_spec.sh) that dynamically checks if every fish function has a corresponding test file under spec/fish/. Since you added _caf_function.fish, you must also add a corresponding test file spec/fish/_caf_function_test.fish to prevent the test suite from failing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The repository has a test coverage spec (spec/fish_functions_coverage_spec.sh) that checks every fish function has a corresponding test file. Adding _caf_function without a corresponding spec/fish/_caf_function_test.fish will cause the test suite to fail.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/fish/default.nix, line 230:

<comment>The repository has a test coverage spec (`spec/fish_functions_coverage_spec.sh`) that checks every fish function has a corresponding test file. Adding `_caf_function` without a corresponding `spec/fish/_caf_function_test.fish` will cause the test suite to fail.</comment>

<file context>
@@ -226,6 +227,7 @@
       })
       [
         "__tmux_bootstrap_default_session"
+        "_caf_function"
         "_cawxe_function"
         "_cawxeh_function"
</file context>

"_cawxe_function"
"_cawxeh_function"
"_caxe_function"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ function __tmux_bootstrap_default_session --description "Create built-in tmux se
set -l session_name $argv[1]
set -l dotfiles_root ~/dotfiles

# Warm the server first so config/plugins (continuum, resurrect) finish
# initializing before we build and attach. On a cold start, otherwise the
# first new-session boots the server concurrently with the attach, freezing
# the freshly-attached client (standalone `tpo` works only on a 2nd call).
tmux start-server 2>/dev/null
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Comment thread
indent-zero[bot] marked this conversation as resolved.
Resolved
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved

switch $session_name
case primary mobile desktop
tmux new-session -d -s $session_name -n btop
Expand Down
48 changes: 48 additions & 0 deletions home-manager/programs/fish/functions/_caf_function.fish
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
function _caf_function --description "Keep the machine awake lid-closed (caffeinate / systemd-inhibit)"
set -l pid_file "$HOME/.local/state/caf.pid"

switch "$argv[1]"
case on
mkdir -p (dirname "$pid_file")
if test (uname) = Darwin
sudo pmset -a disablesleep 1
caffeinate -dimsu &

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: caf on is not idempotent; repeated calls can leak extra inhibitor processes that caf off will not stop.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/fish/functions/_caf_function.fish, line 9:

<comment>`caf on` is not idempotent; repeated calls can leak extra inhibitor processes that `caf off` will not stop.</comment>

<file context>
@@ -0,0 +1,48 @@
+            mkdir -p (dirname "$pid_file")
+            if test (uname) = Darwin
+                sudo pmset -a disablesleep 1
+                caffeinate -dimsu &
+            else
+                systemd-inhibit --what=handle-lid-switch:sleep:idle \
</file context>

else
systemd-inhibit --what=handle-lid-switch:sleep:idle \
--who=caf --why=caf --mode=block sleep infinity &
end
echo $last_pid >"$pid_file"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-running caf on leaks the prior keeper: This unconditionally overwrites $pid_file with the new $last_pid. If the user runs caf on twice (or caf on while a keeper is already alive), the previous caffeinate/systemd-inhibit process becomes an orphan that caf off will never kill (it only knows the most-recent PID). On Linux that orphan keeps holding a handle-lid-switch:sleep:idle inhibitor — visible in systemd-inhibit --list but no longer tracked here — until reboot or manual kill.

Consider short-circuiting case on with the same liveness check used in the toggle/status branches, e.g.:

if test -f "$pid_file"; and kill -0 (cat "$pid_file") 2>/dev/null
    echo "caf already on (pid "(cat "$pid_file")")"
    return 0
end

disown
echo "caf on: awake lid-closed (pid "(cat "$pid_file")")"
Comment on lines +5 to +16

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

caf on is not idempotent and can leave orphan inhibit processes.

On Line 5 and Line 42, repeated starts create additional background inhibitors while Line 14 overwrites the PID file, so off only stops the latest one. This can leave the machine permanently inhibited unexpectedly.

Suggested fix
 function _caf_function --description "Keep the machine awake lid-closed (caffeinate / systemd-inhibit)"
     set -l pid_file "$HOME/.local/state/caf.pid"

     switch "$argv[1]"
         case on
             mkdir -p (dirname "$pid_file")
+            if test -f "$pid_file"
+                set -l existing_pid (cat "$pid_file")
+                if test -n "$existing_pid"; and kill -0 "$existing_pid" 2>/dev/null
+                    echo "caf already active (pid $existing_pid)"
+                    return 0
+                end
+                rm -f "$pid_file"
+            end
             if test (uname) = Darwin
                 sudo pmset -a disablesleep 1
                 caffeinate -dimsu &
             else
                 systemd-inhibit --what=handle-lid-switch:sleep:idle \
                     --who=caf --why=caf --mode=block sleep infinity &
             end
             echo $last_pid >"$pid_file"
             disown
             echo "caf on: awake lid-closed (pid "(cat "$pid_file")")"

Also applies to: 37-43

🤖 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 `@home-manager/programs/fish/functions/_caf_function.fish` around lines 5 - 16,
The "caf on" case is not idempotent: repeated runs spawn multiple background
inhibit processes while the PID file (pid_file) is overwritten so "off" only
stops the last one; fix by making the startup idempotent—before starting, check
pid_file for an existing PID(s), verify whether that PID is still a running
inhibitor process (or belongs to caffeinate/systemd-inhibit), and if valid, do
nothing; if stale, remove it and then start the inhibitor and write its PID
atomically; also ensure the code that writes the PID uses the correct variable
(last_pid) and that disown/echo handle failures so orphan processes are not left
behind (apply same logic to the "caf on" branch for both Darwin caffeinate and
systemd-inhibit paths and mirror in the "caf off" handler to clean all recorded
PIDs).

case off
if test (uname) = Darwin
sudo pmset -a disablesleep 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unconditional sudo pmset on Darwin off: This runs every time caf off (or the toggle path when nothing is active) is invoked on macOS, prompting for sudo even when there is no keeper to tear down. Mirror the pid-file guard used just below so the privileged call only happens when caf was actually on:

if test -f "$pid_file"
    if test (uname) = Darwin
        sudo pmset -a disablesleep 0
    end
    kill (cat "$pid_file") 2>/dev/null
    rm -f "$pid_file"
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: sudo pmset -a disablesleep 0 runs unconditionally on Darwin whenever caf off is invoked, prompting for sudo even when caf was never turned on (no PID file exists). Guard it with the same test -f "$pid_file" check used for the kill below, so the privileged call only fires when caf was actually active.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/fish/functions/_caf_function.fish, line 19:

<comment>`sudo pmset -a disablesleep 0` runs unconditionally on Darwin whenever `caf off` is invoked, prompting for sudo even when caf was never turned on (no PID file exists). Guard it with the same `test -f "$pid_file"` check used for the kill below, so the privileged call only fires when caf was actually active.</comment>

<file context>
@@ -0,0 +1,48 @@
+            echo "caf on: awake lid-closed (pid "(cat "$pid_file")")"
+        case off
+            if test (uname) = Darwin
+                sudo pmset -a disablesleep 0
+            end
+            if test -f "$pid_file"
</file context>

end
if test -f "$pid_file"
kill (cat "$pid_file") 2>/dev/null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The PID file is trusted without validation, so caf off can kill the wrong process.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/fish/functions/_caf_function.fish, line 22:

<comment>The PID file is trusted without validation, so `caf off` can kill the wrong process.</comment>

<file context>
@@ -0,0 +1,48 @@
+                sudo pmset -a disablesleep 0
+            end
+            if test -f "$pid_file"
+                kill (cat "$pid_file") 2>/dev/null
+                rm -f "$pid_file"
+            end
</file context>

rm -f "$pid_file"
Comment on lines +21 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

PID-file kill is unsafe without process identity validation.

On Line 22, killing an unvalidated PID from disk can terminate an unrelated process after PID reuse. Validate that the PID still belongs to the expected keep-awake command before sending kill.

Suggested fix
         case off
             if test (uname) = Darwin
                 sudo pmset -a disablesleep 0
             end
             if test -f "$pid_file"
-                kill (cat "$pid_file") 2>/dev/null
+                set -l pid (cat "$pid_file")
+                set -l cmd (ps -p "$pid" -o command= 2>/dev/null)
+                if string match -rq 'caffeinate|systemd-inhibit|sleep infinity' -- "$cmd"
+                    kill "$pid" 2>/dev/null
+                else
+                    echo "caf off: refusing to kill unrecognized pid $pid"
+                end
                 rm -f "$pid_file"
             end
             echo "caf off: normal sleep restored"
🤖 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 `@home-manager/programs/fish/functions/_caf_function.fish` around lines 21 -
23, Read the PID into a variable (e.g., pid="$(cat "$pid_file")"), verify the
process exists and its command line matches the expected keep-awake command
(e.g., using ps -p "$pid" -o args= or checking /proc/"$pid"/cmdline) before
calling kill, and only remove "$pid_file" after a successful matched kill; if
the PID is absent or the command does not match, treat the pid file as stale and
remove it without killing. Reference: pid_file and the keep-awake command check
around the block that currently runs kill (cat "$pid_file") and rm -f
"$pid_file".

end
echo "caf off: normal sleep restored"
case status
if test (uname) = Darwin
pmset -g | string match -e disablesleep
else
systemd-inhibit --list 2>/dev/null | string match -e caf
end
if test -f "$pid_file"; and kill -0 (cat "$pid_file") 2>/dev/null
echo "caf active (pid "(cat "$pid_file")")"
else
echo "caf inactive"
end
case ''
# Toggle: active if the keeper pid is alive
if test -f "$pid_file"; and kill -0 (cat "$pid_file") 2>/dev/null
_caf_function off
else
_caf_function on
end
case '*'
echo "Usage: caf [on|off|status]"
return 1
end
end
Comment on lines +1 to +48

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using (cat "$pid_file") repeatedly inside command substitutions is inefficient and can lead to syntax errors if the PID file is empty or missing (e.g., kill -0 called with no arguments).

We can optimize this by reading the PID into a local variable using read -l pid < "$pid_file" and validating that it is non-empty before running kill. Additionally, we can reuse $last_pid directly in the on case instead of reading the file we just wrote.

function _caf_function --description "Keep the machine awake lid-closed (caffeinate / systemd-inhibit)"
    set -l pid_file "$HOME/.local/state/caf.pid"

    switch "$argv[1]"
        case on
            mkdir -p (dirname "$pid_file")
            if test (uname) = Darwin
                sudo pmset -a disablesleep 1
                caffeinate -dimsu &
            else
                systemd-inhibit --what=handle-lid-switch:sleep:idle \\
                    --who=caf --why=caf --mode=block sleep infinity &
            end
            echo $last_pid >"$pid_file"
            disown
            echo "caf on: awake lid-closed (pid $last_pid)"
        case off
            if test (uname) = Darwin
                sudo pmset -a disablesleep 0
            end
            if test -f "$pid_file"
                read -l pid < "$pid_file"
                if test -n "$pid"
                    kill $pid 2>/dev/null
                end
                rm -f "$pid_file"
            end
            echo "caf off: normal sleep restored"
        case status
            if test (uname) = Darwin
                pmset -g | string match -e disablesleep
            else
                systemd-inhibit --list 2>/dev/null | string match -e caf
            end
            if test -f "$pid_file"
                read -l pid < "$pid_file"
                if test -n "$pid"; and kill -0 $pid 2>/dev/null
                    echo "caf active (pid $pid)"
                else
                    echo "caf inactive"
                end
            else
                echo "caf inactive"
            end
        case ''
            # Toggle: active if the keeper pid is alive
            set -l active no
            if test -f "$pid_file"
                read -l pid < "$pid_file"
                if test -n "$pid"; and kill -0 $pid 2>/dev/null
                    set active yes
                end
            end
            if test $active = yes
                _caf_function off
            else
                _caf_function on
            end
        case '*'
            echo "Usage: caf [on|off|status]"
            return 1
    end
end

Loading