Gate set_last_save_timestamp on save script exit code

`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)
This commit is contained in:
pleasedodisturb 2026-05-21 21:06:20 +02:00
parent 0698e8f4b1
commit 851234b21e
No known key found for this signature in database

View file

@ -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
}