fix(prompt): stop a stale render sticking on the prompt

functions/fish_prompt.fish

@@ -16,12 +16,17 @@ set -g $prompt_var
 set -q _tide_prompt_tmpdir || set -g _tide_prompt_tmpdir (mktemp -d)
 set -g _tide_prompt_tmpfile $_tide_prompt_tmpdir/prompt
 
-# Bumped on every dispatch and stamped into each job's output, so a job that
-# finishes after a newer one has already been dispatched -- e.g. a slow
-# in-repo render outliving a fast render for the directory `cd`ed into next
-# -- can be recognized as superseded and dropped instead of overwriting
-# fresher content.
-set -q _tide_render_gen || set -g _tide_render_gen 0
+# Bumped once per prompt cycle and stamped into $_tide_repaint every time a
+# repaint is requested. Without it, a repaint requested while a command is
+# still running -- a render landing, or a terminal resize, during a long
+# command -- would be mistaken for a repaint of the prompt drawn next, whose
+# render would then be skipped, leaving pre-command content on screen until
+# something else redrew it.
+set -q _tide_cycle || set -g _tide_cycle 0
+
+function _tide_next_prompt_cycle --on-event fish_postexec
+    set -g _tide_cycle (math $_tide_cycle + 1)
+end
 
 set_color normal | read -l color_normal
 status fish-path | read -l fish_path
@@ -32,24 +37,25 @@ end
 
 # _tide_repaint prevents us from creating a second background job
 function _tide_refresh_prompt --on-signal SIGUSR1 --inherit-variable prompt_var
-    set -l rendered (cat $_tide_prompt_tmpfile 2>/dev/null)
-    # First line is the generation this job was dispatched at (see
-    # _tide_dispatch_render) -- a mismatch means a newer job has since been
-    # dispatched, so this result is stale and superseded; drop it rather
-    # than clobbering $prompt_var with old content. No repaint here is
-    # correct: the newer job has either already applied its own result or
-    # will signal on its own once it finishes.
-    test "$rendered[1]" = "$_tide_render_gen" || return
-    set -g $prompt_var $rendered[2..]
-    set -g _tide_repaint
+    # Every job publishes under its own pid (see _tide_dispatch_render), so
+    # only the newest job's file is ever read: a superseded job that finishes
+    # late writes somewhere else entirely, where it can neither be read here
+    # nor overwrite fresher content. Nothing there yet means this signal came
+    # from such a straggler while the current job is still rendering -- drop
+    # it and wait, the current job signals for itself once it finishes.
+    set -q _tide_last_pid || return
+    set -l rendered (cat $_tide_prompt_tmpfile.$_tide_last_pid 2>/dev/null)
+    set -q rendered[1] || return
+    set -g $prompt_var $rendered
+    set -g _tide_repaint $_tide_cycle
     commandline -f repaint
 end
 
 # On resize, repaint with the last completed render but leave $prompt_var
 # alone -- in synchronous-fallback mode the tmpfile is never written, so
 # reading it here would blank the prompt.
 function _tide_repaint_on_resize --on-variable COLUMNS
-    set -g _tide_repaint
+    set -g _tide_repaint $_tide_cycle
     commandline -f repaint
 end
 
@@ -75,33 +81,40 @@ function _tide_dispatch_render --inherit-variable prompt_var --inherit-variable
         return
     end
 
-    set -g _tide_render_gen (math $_tide_render_gen + 1)
-
-    # The job renders into a private scratch file (suffixed with its own
-    # pid) and atomically renames it over the shared tmpfile, so the
-    # SIGUSR1 handler can never read a partial write from a newer job. Its
-    # first line is stamped with the generation dispatched here (baked in
-    # as a literal below, not read back from the variable, since the job
-    # is a separate process) so _tide_refresh_prompt can recognize and
-    # drop a result superseded by a since-dispatched job. The tmpfile path
-    # crosses into the job string-escaped and is expanded as a variable
-    # there, never re-parsed as syntax, so any TMPDIR is safe.
+    # Removing the previous job's files is left to the job below, so that fork
+    # lands in the background instead of on the interactive path.
+    set -l rm_stale
+    if set -q _tide_last_pid
+        set -l prev (string escape -- $_tide_prompt_tmpfile.$_tide_last_pid)
+        set rm_stale "command rm -f $prev 2>/dev/null"
+
+        # `.part` is renamed away the moment a job publishes, so one still
+        # sitting there means the previous job never finished -- only then is
+        # there anything to kill, and only then do we pay for the fork. Its
+        # scratch file is only safe to delete once that kill has landed:
+        # deleting it under a job that is still rendering would leave that
+        # job's rename with nothing to rename.
+        if test -e $_tide_prompt_tmpfile.$_tide_last_pid.part && command kill $_tide_last_pid 2>/dev/null
+            set -l prev_part (string escape -- $_tide_prompt_tmpfile.$_tide_last_pid.part)
+            set rm_stale "command rm -f $prev $prev_part 2>/dev/null"
+        end
+    end
+
+    # The job renders into a private scratch file and atomically renames it to
+    # a path named after its own pid, so the SIGUSR1 handler can never read a
+    # partial write, and a superseded job can never overwrite the result of a
+    # job dispatched after it. Paths cross into the job string-escaped -- and
+    # the tmpfile is expanded there as a variable, never re-parsed as syntax
+    # -- so any TMPDIR is safe.
     $fish_path -c "set _tide_pipestatus $_tide_pipestatus
 set _tide_parent_dirs $_tide_parent_dirs
 set _tide_prompt_tmpfile "(string escape -- $_tide_prompt_tmpfile)"
-echo $_tide_render_gen >\$_tide_prompt_tmpfile.\$fish_pid
-PATH="(string escape "$PATH")" CMD_DURATION=$CMD_DURATION fish_key_bindings=$fish_key_bindings fish_bind_mode=$fish_bind_mode $argv[1] >>\$_tide_prompt_tmpfile.\$fish_pid
-command mv -f \$_tide_prompt_tmpfile.\$fish_pid \$_tide_prompt_tmpfile
-command kill -s USR1 $fish_pid 2>/dev/null" &
+PATH="(string escape "$PATH")" CMD_DURATION=$CMD_DURATION fish_key_bindings=$fish_key_bindings fish_bind_mode=$fish_bind_mode $argv[1] >\$_tide_prompt_tmpfile.\$fish_pid.part
+command mv -f \$_tide_prompt_tmpfile.\$fish_pid.part \$_tide_prompt_tmpfile.\$fish_pid 2>/dev/null
+command kill -s USR1 $fish_pid 2>/dev/null
+$rm_stale" &
     builtin disown
 
-    # A completed job has already mv'd its scratch file away, so its
-    # existence means the previous job is still running or died mid-render --
-    # only then is there anything to kill/clean up, and only then do we pay
-    # for the two external forks.
-    test -e $_tide_prompt_tmpfile.$_tide_last_pid &&
-        command kill $_tide_last_pid 2>/dev/null &&
-        command rm -f $_tide_prompt_tmpfile.$_tide_last_pid
     set -g _tide_last_pid $last_pid
 
     if not set -q _tide_signal_confirmed
@@ -131,7 +144,9 @@ if contains newline $_tide_left_items # two line prompt initialization
     eval "
 function fish_prompt
     set -lx _tide_status \$status
-    _tide_pipestatus=\$pipestatus if not set -e _tide_repaint
+    _tide_pipestatus=\$pipestatus if test \"\$_tide_repaint\" = \"\$_tide_cycle\"
+        set -e _tide_repaint
+    else if not contains -- --final-rendering \$argv
         _tide_dispatch_render _tide_2_line_prompt
     end
 
@@ -168,7 +183,9 @@ else # one line prompt initialization
     eval "
 function fish_prompt
     set -lx _tide_status \$status
-    _tide_pipestatus=\$pipestatus if not set -e _tide_repaint
+    _tide_pipestatus=\$pipestatus if test \"\$_tide_repaint\" = \"\$_tide_cycle\"
+        set -e _tide_repaint
+    else if not contains -- --final-rendering \$argv
         _tide_dispatch_render _tide_1_line_prompt
     end
 

tests/fish_prompt.test.fish

@@ -114,4 +114,71 @@ echo stderr-lines (count <$stderr_log)
 # CHECK: stderr-lines 0
 command rm -f $stderr_log
 
+# A superseded job that finishes late must not be able to hide or destroy the
+# newest job's result. Every job publishes under its own pid, so this can be
+# staged directly: write what the current job published, then write what a
+# slower job dispatched *earlier* publishes afterwards, then deliver the
+# signal. The late write lands somewhere the handler never reads, so the
+# order of the two writes cannot matter -- which is the whole point, since
+# with one shared file the later write won.
+fish -i -c '
+    false
+    fish_prompt >/dev/null
+    for i in (seq 1 50)
+        set -q _tide_repaint && break
+        sleep 0.01
+    end
+    echo fresh-content >$_tide_prompt_tmpfile.$_tide_last_pid
+    echo stale-junk >$_tide_prompt_tmpfile.999999
+    command kill -s USR1 $fish_pid
+    sleep 0.1
+    echo applied: (_tide_decolor (fish_prompt))
+' </dev/null
+# CHECK: applied: fresh-content
+
+# Superseding a render has to stay silent. Each job cleans up after the one
+# before it, and a job still rendering is killed first -- take its scratch
+# file away without killing it and its rename has nothing to rename, so it
+# complains to the terminal from a background process the prompt no longer
+# controls. Dispatching renders back to back makes jobs overlap.
+set -l stderr_log (mktemp)
+fish -i -c '
+    for i in (seq 20)
+        set -e _tide_repaint
+        fish_prompt >/dev/null
+    end
+    sleep 0.5
+' </dev/null 2>$stderr_log
+echo overlap-stderr-lines (count <$stderr_log)
+# CHECK: overlap-stderr-lines 0
+command rm -f $stderr_log
+
+# A repaint requested in one prompt cycle must not suppress the render of the
+# next one. Both handlers can fire while a command is still running -- an
+# async render landing, or a terminal resize -- and the prompt drawn after
+# that command has to show the command's own status, not the previous one's.
+# `fish_postexec` doesn't fire for statements inside `fish -c`, so the cycle
+# boundary is emitted by hand.
+for source in render resize
+    fish -i -c "
+    false
+    fish_prompt >/dev/null
+    for i in (seq 1 50)
+        set -q _tide_repaint && break
+        sleep 0.01
+    end
+    test $source = resize && _tide_repaint_on_resize
+
+    emit fish_postexec
+    true; fish_prompt >/dev/null
+    for i in (seq 1 50)
+        test \"\$_tide_repaint\" = \"\$_tide_cycle\" && break
+        sleep 0.01
+    end
+    echo $source: (_tide_decolor (fish_prompt))
+" </dev/null
+end
+# CHECK: render: {{.*}}✔{{.*}}
+# CHECK: resize: {{.*}}✔{{.*}}
+
 set -e tide_left_prompt_items tide_right_prompt_items tide_prompt_add_newline_before tide_left_prompt_frame_enabled tide_right_prompt_frame_enabled tide_prompt_min_cols tide_status_icon tide_status_icon_failure tide_jobs_icon tide_jobs_number_threshold

tests/fish_prompt_transient.test.fish

@@ -36,6 +36,17 @@ fish -i -c '
     echo transient-right:
     _tide_decolor (fish_right_prompt --final-rendering)
     echo transient-right-end
+
+    # The transient render prints the character and nothing else, so it never
+    # reads the rendered prompt -- dispatching a background job for it would
+    # be a fork spent on output that gets thrown away, and one more job in
+    # flight while the command runs. Clear the repaint flag first, so a skip
+    # can only come from the --final-rendering check.
+    set -e _tide_repaint
+    set -l pid_before $_tide_last_pid
+    fish_prompt --final-rendering >/dev/null
+    test "$_tide_last_pid" = "$pid_before" &&
+        echo transient-dispatched-no || echo transient-dispatched-yes
 ' </dev/null 2>$stderr_log
 # CHECK: normal:
 # CHECK: {{.*}}╭─{{.*}}─╮{{.*}}
@@ -44,6 +55,7 @@ fish -i -c '
 # CHECK: {{\x1b\[0J}}❯
 # CHECK: transient-right:
 # CHECK: transient-right-end
+# CHECK: transient-dispatched-no
 
 echo stderr-lines (count <$stderr_log)
 # CHECK: stderr-lines 0