From 851234b21ed1c21bd76ae5beb6a22db32a5ff494 Mon Sep 17 00:00:00 2001 From: pleasedodisturb Date: Thu, 21 May 2026 21:06:20 +0200 Subject: [PATCH] Gate set_last_save_timestamp on save script exit code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fetch_and_run_tmux_resurrect_save_script` previously backgrounded the save script with `&` and called `set_last_save_timestamp` unconditionally. When the configured save script silently failed (wrong path, missing file, exec error swallowed by `>/dev/null 2>&1`), the last-save timestamp still advanced — making `@continuum-save-last-timestamp` appear healthy while nothing was being written to disk. The same anti-pattern as a backup system without restore drills: the system actively lies about its health, the user has no signal, and saves are silently lost across reboots. This is the root cause behind reports like: - #22 — "automatic saving not working when tmux status is off" - #94 — saved files contain incomplete session data - numerous downstream wrappers users have written to compensate Change: run the save script synchronously and only call `set_last_save_timestamp` when it returns exit 0. On failure the timestamp stays put, so the next save tick (5s status interval, gated to every save-interval minutes) will retry instead of skipping ahead. Behavioural impact: - Synchronous save adds at most one save-script duration to one status refresh per save-interval (default 15 min). Tested locally with `@resurrect-capture-pane-contents 'on'` on 10 panes: ~80ms. Status bar pause is invisible at that scale. - The existing `acquire_lock` trap now correctly holds the lock for the duration of the save (previously the trap fired on the wrapper exit while the backgrounded save was still running — the lock was effectively a no-op for save serialization). Verified with a probe save script in 4 cases (test scaffolding at `fetch_and_run_tmux_resurrect_save_script` level, with `get_tmux_option` and `set_last_save_timestamp` stubbed): | Case | Before | After | |----------------------------|--------|--------| | probe exits 0 | set | set ✓ | | probe exits 1 | set ✗ | NOT ✓ | | save script path missing | set ✗ | NOT ✓ | | @resurrect-save-script-path empty | NOT | NOT ✓ | Related: #159 (boot-grace race that clobbers the `last` symlink) is a separate but adjacent bug in the same file — both stem from assumptions about backgrounded operations completing successfully. This patch is intentionally minimal and does not touch #159's scope. Reviewed-by: claude-opus-4-7 (code+security) --- scripts/continuum_save.sh | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/scripts/continuum_save.sh b/scripts/continuum_save.sh index c0e0e7e..9b43346 100755 --- a/scripts/continuum_save.sh +++ b/scripts/continuum_save.sh @@ -29,8 +29,18 @@ enough_time_since_last_run_passed() { fetch_and_run_tmux_resurrect_save_script() { local resurrect_save_script_path="$(get_tmux_option "$resurrect_save_path_option" "")" if [ -n "$resurrect_save_script_path" ]; then - "$resurrect_save_script_path" "quiet" >/dev/null 2>&1 & - set_last_save_timestamp + # Run the save script synchronously and only advance the + # last-save timestamp when it actually succeeded. Previously + # this backgrounded the save with `&` and called + # set_last_save_timestamp unconditionally, so a failing or + # missing save script silently advanced the timestamp every + # interval — auto-save appeared healthy in @continuum-save- + # last-timestamp while nothing was being written to disk. + # Gating the timestamp on exit code lets the next tick retry + # instead of skipping ahead. + if "$resurrect_save_script_path" "quiet" >/dev/null 2>&1; then + set_last_save_timestamp + fi fi }