fix(tmux): detect a gone tmux window instead of reporting every one alive - #108
Conversation
…live
`tmux display-message -p -t <session>:<window>` is not an existence check.
When the window component does not resolve, tmux silently falls back to the
session's current window and still exits 0, so a deliberately bogus window
name produces the same successful output as a real-but-gone one. Verified on
tmux 3.7b against a window that was created and then killed:
$ tmux display-message -p -t 'S:fm-doomed' '#{pane_id}'
%0 # exit 0, the OTHER window's pane
$ tmux has-session -t 'S:fm-doomed'
can't find window: fm-doomed # exit 1, correct
Every probe built on it therefore reported every closed tmux window as still
alive for as long as its session survived. fm_backend_target_state always
answered `present`, so fm-teardown.sh could never release a completed
tmux-backed task ("endpoint is still alive; retaining metadata") and leaked
its worktree lease and metadata permanently. --force does not help, because
the ordinary safety proof itself is what false-positives.
The identity guard failed open the same way, and worse: it short-circuited to
success for every target that was not a @window-id - that is, for the
session:window shape fm-spawn.sh actually records - so it never guarded the
fleet's real targets, and it returned PASS even when the expected label
plainly disagreed with the live window name. Under it, fm_backend_tmux_kill
would kill a live window belonging to another task.
Existence now goes through has-session, which runs tmux's own target parser
over the full target and fails on a missing session, window, or pane. A bare
selector is matched against the live window list instead, because tmux would
read a bare target as a session name. Identity for a session:window target is
settled by comparing the target against what the task recorded and then
resolving it with tmux's exact-match syntax, so no display-message is
involved and `fm-foo` can never resolve to `fm-foobar`; a @window-id carries
no label, so that shape keeps the identity read, now gated on existence.
capture-pane and send-keys were checked the same way and do not share the
quirk, so only display-message uses needed replacing. current_path and
current_command are gated too: they used to answer with the session's current
window, which let agent liveness report `dead` - a confident verdict
bin/fm-bootstrap.sh gates a respawn on - for a window that was simply gone.
fm_backend_tmux_kill treats an absent target as a no-op success, preserving
fm_backend_kill's already-gone contract so a correctly-rejecting guard cannot
turn every closed window into a fresh teardown refusal.
tests/fm-backend-tmux-target-exists.test.sh pins both directions against a
real tmux server on a private socket: a live target is detected as alive, and
a deliberately bogus or already-closed one as gone, across session:window,
session:window.pane, @window-id, %pane-id and bare selectors. The existing
fake-tmux fixtures modelled existence only through display-message, so they
now model has-session, and a few that recorded a window label inconsistent
with their own task id are made production-shaped.
bin/fm-behavior-shards.sh's coverage guard requires the duration inventory to match tests/*.test.sh exactly, so a new behavior test has to be weighted here or the shard plan refuses and every shard is skipped.
…4.4+ A bare `local x` leaves the variable genuinely unset on bash 4.4+, so reading it under `set -u` aborts the calling script outright; bash 3.2 (macOS) reads it as empty and hides the fault. The identity guard's new session:window path reads expected_session even when no recorded scoped target was supplied, which killed fm-fleet-snapshot, fm-send, fm-daemon and the account-routing lanes on Linux while every macOS run passed. Verified against bash 5.3: the bare-local form dies on the first call, the initialised form completes every entry point.
…fakes has-session
Two portability faults surfaced on Linux/CI that macOS could not show.
A tab inside a tmux format string is not portable: tmux 3.4 renders
'#{session_name}<TAB>#{window_name}' as "S_fm-live", so the joined value can
never be split back into its two fields. That silently made the identity of
every @window-id unverifiable there - a pre-existing fault the guard inherited.
The two fields are now read separately, which is version-independent.
The fake tmux stubs across the suite encode endpoint existence in their
display-message arm, because that is the probe the adapter used to use. Now
that existence is proven with has-session, that arm is shared
(`has-session|display-message)`) so each stub keeps exactly one existence model
instead of two that can disagree - which is what made a killed window still
read as alive in the teardown, watcher, daemon and session-start fixtures.
The two stubs that actually address @window-ids (fm-backend and fm-send-strict)
answer #{session_name} and #{window_name} independently, matching real tmux;
fm-backend's is scoped to display-message so `list-windows -F '#{window_name}'`
is not mistaken for an identity read.
…dout Three follow-ups from running the suite on Linux. fm_backend_tmux_operation_target no longer probes existence itself. It resolves targets for capture-pane and send-keys, and both already fail correctly on a window that is gone, so the probe only cost a second round-trip on every send and wrongly rejected a bare ad hoc selector that names a pane the fixture serves. The display-message-based readers keep their own gate, which is where the gate is actually load-bearing. fm_backend_tmux_container_ensure now discards has-session's stdout as well as its stderr. This function's stdout IS the resolved session name its caller captures, so a probe must not be able to contribute a line to it. The shared fake tmux in tests/wake-helpers.sh models a live pane through capture-pane but answered every other subcommand with failure, so existence and readability disagreed once existence stopped going through display-message; tests/secondmate-helpers.sh answered has-session from an unconditional always-succeeds arm, which reported a killed window as alive forever, and its identity arm now answers one field at a time.
Follow-up fixture corrections found by running the suite on Linux.
The account-routing stub answered has-session from an unconditional
always-succeeds arm while modelling real existence through its '#{pane_id}'
probe, so a released endpoint still read as alive and spawn retained a worktree
it should have returned. has-session now follows that same oracle, with a bare
session target (the container-ensure probe) still always resolving.
Several fixtures recorded a window whose label did not match their own task id,
or copied a herdr-shaped `default:wN:pN` window onto a tmux lane. fm-spawn.sh
records `<session>:fm-<id>`, and every live task in the reference fleet matches
that, so these are corrected rather than accommodated - they only passed while
the identity guard was failing open. The session-start stub also accepts tmux's
exact-match spelling ("=session:=window") now that the adapter resolves windows
without fnmatch.
The adapter no longer asks for a tab-joined '#{session_name}<TAB>#{window_name}'
pair, because tmux 3.4 renders that TAB as "_". The stubs that still answered in
that shape were left unreachable by the change - none of their fixtures address
a @window-id, which is the only shape that still triggers an identity read - but
an unreachable wrong answer is a trap for the next fixture that does. They now
answer #{session_name} and #{window_name} independently.
One stub in fm-backend.test.sh already carried its own has-session arm; the
shared arm added earlier made that arm dead. Its dedicated arm is restored as
the live one, which also keeps has-session from printing to stdout there.
tools/lavish/test/lavish.test.mjs builds a fake tmux that answers
display-message and capture-pane for a live supervisor pane and fails every
other subcommand. Once endpoint existence stopped going through display-message
- whose exit status carries no existence information - that catch-all reported
the live supervisor as not live, so fm-lavish-queue refused to queue the prompt
("supervisor target is not live") and the board never delivered.
Reproduced locally in both directions before and after: pristine origin/main
passes tests/lavish.test.sh, this branch failed it, and it passes again with the
fixture modelling has-session the same way it already models the pane.
The new suite's recorded duration is also corrected to its measured CI time
(540ms, was an estimated 1500ms) so shard packing reflects reality.
|
SymptomReached via It is pre-existingIsolated with the suite's own partition mechanism, which runs exactly this one case: FM_TEST_PART_INDEX=57 FM_TEST_PART_TOTAL=143 \
bash -c '. "$0/tests/fm-teardown-suite.sh"' <tree>
An unmodified pre-#108 checkout fails the identical assertion on the same host, so #108 is not the cause. Both trees were run back to back on the same machine rather than compared against a measurement taken under different conditions. Why it is host-sensitive rather than always-redCI passes this suite consistently ( The case is a TOCTOU race: it starts wait_seconds=$(fm_test_load_scaled_timeout_seconds 30 150)
... fm_test_wait_for_file "$marker" "$teardown_pid" 0.05 ...
fail "object-storage graph proof did not expose its retained-identity boundary"Observed while reproducing: load average 21–38 on 14 cores with ~950–980 processes, several agents running teardown suites concurrently. A full That is a hypothesis about the mechanism, not a claim the test is "just flaky" — the authorship question is settled by the pre-#108 failure, but why it times out is not, and it should not be dismissed on load alone. Suggested directions
Reproductiongit worktree add --detach /tmp/base-wt b781f8e5
FM_TEST_PART_INDEX=57 FM_TEST_PART_TOTAL=143 \
bash -c '. "$0/tests/fm-teardown-suite.sh"' /tmp/base-wtRoughly one minute per run, versus 23+ minutes for the whole wrapper — useful for anyone iterating on this. |
Follow-up: full matrix, and a calibration leadThree rounds × three trees of the isolated case, same host, sequential:
Pre-#108 fails 3/3, so authorship is settled: neither #108 nor #109 causes this. The calibration lead: the one pass happened at the highest load observed (91.01), not the lowest. That is backwards for a naive "too much load" explanation, and it is consistent with the timeout being load-scaled: wait_seconds=$(fm_test_load_scaled_timeout_seconds 30 150)At very high load the scaling grants close to the 150s ceiling and the marker arrives in time; in the mid band (~17–27) it grants something nearer the 30s floor, which is not enough on a machine where the full wrapper takes 23+ minutes against CI's ~7.5. So the likely defect is not "the host is too slow" but that the scaling curve under-allocates in the mid-load band — the floor, or the load→seconds mapping, rather than the ceiling. Worth checking |
The defect
tmux display-message -p -t <session>:<window>is not an existence check. When the window component does not resolve, tmux silently falls back to the session's current window and still exits 0 — so a deliberately bogus window name returns the same successful output as a real-but-gone one.Reproduced against a window created and then killed (tmux 3.7b), which is the exact teardown scenario:
capture-pane -tandsend-keys -twere checked the same way and correctly exit 1.display-messageis uniquely broken, so only its uses needed replacing.Measured before/after, same fixture
fm_backend_target_existson a bogus windowfm_backend_target_existson a bogussession:window.panefm_backend_target_state, window closedfm_backend_target_state, closed, by@window-idfm_backend_tmux_killwith a mismatched identityCross-home data-loss hazard (the reason this is urgent, not just leaky)
The identity guard short-circuited to success for every target that was not a
@window-id— that is, for thesession:windowshapefm-spawn.shactually records. It never guarded the fleet's real targets, and it returned PASS even when the expected label plainly disagreed with the live window name.The consequence is not confined to one task, and not confined to one home. The tmux session is shared across firstmate homes — lanes belonging to different homes live in the same session — so a mismatched-identity
fm_backend_tmux_killcould close a window belonging to another home's in-flight work, not merely another task in the same fleet. That is destruction of work that may exist nowhere else, and the guard that was supposed to prevent it was returning PASS unconditionally for the only target shape the fleet uses.The bottom row of the table above is a live reproduction of that:
fm_backend_tmux_kill "$SESSION:$LIVE" "" "fm-__wrong_label__" "$SESSION:fm-__wrong_label__"killed the live window before this change and is refused after it.Two further consequences found while reproducing:
current_commandanswered with the session's current window, sofm_backend_tmux_agent_alivereturneddeadfor a window that was merely gone.deadis a confident verdict thatbin/fm-bootstrap.sh's secondmate-liveness sweep gates a respawn on.fm_backend_target_statealways answeredpresent, sobin/fm-teardown.shcould never release a completed tmux-backed task (endpoint is still alive; retaining metadata) and leaked its worktree lease and metadata permanently.--forcedoes not help, because the ordinary safety proof itself is what false-positives.Why the leak compounds rather than being a fixed number
backend=is written intostate/<id>.metaat spawn time, so switching the fleet'sconfig/backendto tmux only affects newly spawned lanes. The exposed population therefore grows with every spawn rather than being a one-time set. Measured on the reference fleet during a single session, tmux-backed tasks went 7 → 9 → 12 and already-leaked leases went 2 → 3. The fleet moved to this backend one night before this fix, so the affected population is still small but increasing continuously.Leaked leases are not cleaned up by this change, deliberately: leaking a lease costs a workspace, whereas releasing one wrongly can discard committed-but-unpushed or uncommitted work that exists nowhere else. Their owning homes should adjudicate them individually.
The fix
has-session, which runs tmux's own target parser over the full target and fails on a missing session, window, or pane. Verified correct forsession:window,session:window.pane,@window-id,%pane-idand session-only. A bare selector is matched against the live window list instead, because tmux would read a bare target as a session name — the one shapehas-sessiongets wrong, and the one that would have made this "the same bug moved".session:windowtarget is settled by comparing the target against what the task recorded, then resolving it with tmux's exact-match syntax (=session:=window), so nodisplay-messageis involved andfm-foocan never resolve tofm-foobar. A@window-idcarries no label, so that shape keeps the identity read — now gated on existence first.current_path/current_commandare existence-gated, so they return nothing rather than another window's values.fm_backend_tmux_operation_targetdeliberately does not probe:capture-paneandsend-keysalready fail correctly on a gone target, so a probe there would only add a round-trip per send.fm_backend_tmux_killtreats an absent target as a no-op success, preservingfm_backend_kill's already-gone contract so a correctly-rejecting guard cannot turn every closed window into a fresh teardown refusal.fm_backend_tmux_container_ensurediscardshas-session's stdout as well as its stderr: that function's stdout is the resolved session name its caller captures, so a probe must not be able to contribute a line to it.Two portability faults, both structurally invisible on macOS
Found only by running the suite on Linux and then proving each against a real
bash:5/ubuntu:24.04container:local xwith no value. Bash 3.2 (macOS) reads it as empty; bash 4.4+ leaves it genuinely unset, so reading it underset -uaborts the calling script. Every consumer here runsset -u. Proven: the bare-local form dies on the first call under bash 5.3, the initialised form completes every entry point.'#{session_name}<TAB>#{window_name}'asS_fm-live, so the joined value can never be split back into its two fields — which silently made the identity of every@window-idunverifiable there. This was pre-existing; the fields are now read one at a time.Both patterns were then audited across the two files this PR touches. Neither remains: no bare
localis read before assignment (the two flagged by a first-pass matcher,while IFS= read -r line || [ -n "$line" ]andfor meta in ..., are both assigned by the construct itself), and no tmux format string inbin/embeds a TAB. The surviving'#{session_name}:#{window_name}'joins on a colon, which tmux renders literally — confirmed bytests/fm-backend-tmux-smoke.test.sh'sresolve_bare_selectorcase passing on tmux 3.4.Tests
tests/fm-backend-tmux-target-exists.test.shis new and runs against a real tmux server on a private socket. Every assertion is written in both directions, and the negative direction always uses a deliberately bogus name alongside a realistic already-closed window — the assertion that would have caught this, since the old probe returned the same success for both. It also covers the closed-window lifecycle end to end, exact-vs-prefix resolution, agent liveness, and kill idempotency, and pins that the existence primitive never usesdisplay-message. It passes on macOS/tmux 3.7b and on Linux/tmux 3.4.The existing fake tmux stubs encoded endpoint existence in their
display-messagearm, because that is the probe the adapter used to use. That arm is now shared (has-session|display-message)) so each stub keeps exactly one existence model instead of two that can disagree. Several fixtures also recorded a window whose label did not match their own task id, or copied a herdr-shapeddefault:wN:pNwindow onto a tmux lane;fm-spawn.shrecords<session>:fm-<id>and every live task in the reference fleet matches that, so those are corrected rather than accommodated — they only passed while the identity guard was failing open.bin/fm-lint.shpasses (207 scripts, ShellCheck 0.11.0, clean).Pre-existing failures, with evidence
These were failing before this branch. Each was checked by running the identical suite against an unmodified
origin/mainworktree on the same host, not by inspection:origin/mainfm-watch-triage.test.shEXIT=1 passes=29not ok - watcher did not re-surface a paused secondmateEXIT=1 passes=29not ok - watcher did not re-surface a paused secondmatefm-backend.test.shEXIT=1 passes=27not ok - fm-spawn.sh should succeed for a project reached through a symlinked prefix…(
error: checkout-refresh LaunchAgent namespaces cannot be safely enumerated)EXIT=1 passes=27same assertion, same error
For
fm-backend.test.shthe passing sets were also diffed (comm -23over the sortedok -lines): zero tests pass on pristine that do not pass here, and both stop at the same assertion. Thefm-backendfailure is a host LaunchAgent-plist condition, not a code path this PR touches.fm-account-routing-a/fm-account-routing-bwere also checked this way and are now at exact parity (passes=63 EXIT=0on both trees).Every further failure was put through the same check rather than pattern-matched to these. That is how
tests/lavish.test.shwas caught: it looked pre-existing — it contains zero tmux references and this PR touches none of its files — but running pristineorigin/mainand this branch on the same host showedEXIT=0againstEXIT=1, so it was mine after all. The cause was the same fixture class as the rest:tools/lavish/test/lavish.test.mjsbuilds a fake tmux that answersdisplay-messageandcapture-panefor a live supervisor pane and fails every other subcommand, so once existence stopped going throughdisplay-messagethe catch-all reported the live supervisor as not live andfm-lavish-queuerefused to queue (supervisor target is not live). It is fixed and verified in both directions. Had the "it doesn't touch tmux" reasoning been trusted, a real regression would have shipped.Not a gate here
PR must be raised via no-mistakesfails by construction and is not a real gate for this repo's own work per the repo owner. Behaviour tests and lint are the checks that matter.