Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
30 changes: 22 additions & 8 deletions home-manager/programs/fish/functions/_two_function.fish
Original file line number Diff line number Diff line change
@@ -1,17 +1,31 @@
function _two_function --description "Attach to tmux work session"
set -l restore (tmux list-keys 2>/dev/null | string match -rg '(/\S+/resurrect/scripts/restore\.sh)')
set -l restore $restore[1]
if test -n "$restore"
tmux run-shell "$restore"
end

if tmux has-session -t work 2>/dev/null
if test -n "$TMUX"
tmux switch-client -t work
else
tmux attach-session -t work
end
else
tmuxinator start work
return
end

# No work session — try resurrect restore
if test -f ~/.tmux/resurrect/last
# Start a detached session so the server is running
tmux new-session -d -s _restore 2>/dev/null
set -l restore (tmux list-keys 2>/dev/null | string match -rg '(/\S+/resurrect/scripts/restore\.sh)')
set -l restore $restore[1]
Comment on lines +13 to +16

Copilot AI Mar 27, 2026

Copy link

Choose a reason for hiding this comment

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

Using a fixed temp session name (_restore) is risky: if a user already has a session with that name, this function will unconditionally kill it. Consider generating a unique session name (e.g., include PID/random) and only killing the session if it was actually created by this function.

Copilot uses AI. Check for mistakes.
if test -n "$restore"
tmux run-shell "$restore"
end
# Clean up temp session if restore created work
if tmux has-session -t work 2>/dev/null
tmux kill-session -t _restore 2>/dev/null
tmux attach-session -t work

Copilot AI Mar 27, 2026

Copy link

Choose a reason for hiding this comment

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

In the resurrect-restore path, this always runs tmux attach-session -t work when work appears, even if $TMUX is set (already inside tmux). Attaching from inside tmux will fail; this should mirror the earlier logic and switch-client -t work when inside tmux.

Suggested change
tmux attach-session -t work
if test -n "$TMUX"
tmux switch-client -t work
else
tmux attach-session -t work
end

Copilot uses AI. Check for mistakes.
return
end
tmux kill-session -t _restore 2>/dev/null
Comment on lines +20 to +26

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

The cleanup logic for the temporary _restore session can be simplified to avoid repeating the tmux kill-session command in the source code. This makes the code more DRY (Don't Repeat Yourself) and easier to maintain.

    # Clean up temp session and attach if restore created work
    tmux has-session -t work 2>/dev/null
    set -l work_restored $status
    tmux kill-session -t _restore 2>/dev/null
    if test $work_restored -eq 0
      tmux attach-session -t work
      return
    end

end
Comment on lines +11 to 27

Copilot AI Mar 27, 2026

Copy link

Choose a reason for hiding this comment

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

The new resurrect-restore behavior (creating a detached bootstrap session, running restore, then switching/attaching) isn’t covered by the fish specs yet. Adding a test that simulates ~/.tmux/resurrect/last existing and asserts that tmux run-shell is invoked and the function attaches/switches to work would prevent regressions in this main new code path.

Copilot uses AI. Check for mistakes.

# Fallback to tmuxinator
tmuxinator start work
end
18 changes: 15 additions & 3 deletions spec/fish/_two_function_test.fish
Original file line number Diff line number Diff line change
@@ -1,23 +1,35 @@
set fn (status dirname)/../../home-manager/programs/fish/functions
source $fn/_two_function.fish

# ── no resurrect, no work session → tmuxinator start work ─
# ── no resurrect file, no work session → tmuxinator start work ─
set log (mktemp)
function tmux
if test "$argv[1]" = list-keys; echo ""; return; end
if test "$argv[1]" = has-session; return 1; end
if test "$argv[1]" = new-session; return 0; end
if test "$argv[1]" = list-keys; echo ""; return; end
if test "$argv[1]" = kill-session; return 0; end
end
function tmuxinator; echo $argv >> $log; end

# Ensure no resurrect file interferes
set -l _bak ""
if test -f ~/.tmux/resurrect/last
set _bak (mktemp)
mv ~/.tmux/resurrect/last $_bak

@cubic-dev-ai cubic-dev-ai Bot Mar 27, 2026

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: This test mutates the real ~/.tmux/resurrect/last file and only restores it if the script reaches the cleanup block. If the test aborts early, the user’s resurrect data is lost. Use a guaranteed cleanup (fish_exit handler) or avoid moving the real file.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spec/fish/_two_function_test.fish, line 18:

<comment>This test mutates the real ~/.tmux/resurrect/last file and only restores it if the script reaches the cleanup block. If the test aborts early, the user’s resurrect data is lost. Use a guaranteed cleanup (fish_exit handler) or avoid moving the real file.</comment>

<file context>
@@ -1,23 +1,35 @@
+set -l _bak ""
+if test -f ~/.tmux/resurrect/last
+    set _bak (mktemp)
+    mv ~/.tmux/resurrect/last $_bak
+end
+
</file context>
Fix with Cubic

end

_two_function

if test -n "$_bak"
mv $_bak ~/.tmux/resurrect/last
end
Comment on lines +15 to +25

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 manual backup and restore of ~/.tmux/resurrect/last is not robust. If the test script fails or is interrupted after moving the file (line 18) but before restoring it (line 24), the user's file will be lost. This could be made safer by using an exit handler to guarantee the file is restored.

Consider using functions --on-process-exit to define a cleanup function that will run regardless of how the script terminates.

Example:

function _cleanup_resurrect_file --on-process-exit
    if set -q --global __resurrect_last_bak
        # Logic to move file back
        if test -f "$__resurrect_last_bak"
            mv "$__resurrect_last_bak" ~/.tmux/resurrect/last
        end
        set -e __resurrect_last_bak
    end
end

if test -f ~/.tmux/resurrect/last
    set -g __resurrect_last_bak (mktemp)
    mv ~/.tmux/resurrect/last $__resurrect_last_bak
end

# ... test logic ...
# No manual restore needed here

Comment on lines +14 to +25

Copilot AI Mar 27, 2026

Copy link

Choose a reason for hiding this comment

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

This test manipulates the real ~/.tmux/resurrect/last on the machine running the suite (moves it out of the way and back). That makes the test non-hermetic and can clobber a developer’s resurrect state if the test aborts before the restore step. Prefer isolating by setting HOME to a temp dir (as other fish specs do) and creating/removing a resurrect file under that temp HOME instead of touching the real ~ path.

Copilot uses AI. Check for mistakes.

@test "missing work session starts via tmuxinator" (grep -c "start work" $log) -ge 1

# ── session exists, outside tmux → attach ────────────────
set log2 (mktemp)
set -e TMUX
function tmux
if test "$argv[1]" = list-keys; echo ""; return; end
if test "$argv[1]" = has-session; return 0; end
echo $argv >> $log2
end
Expand Down
Loading