Skip to content

BaseTmuxBackend.list_window_ids folds every nonzero exit to [], so a failed listing reads as an empty session #525

Description

@dracic

Summary

BaseTmuxBackend.list_window_ids raises only on TimeoutExpired/OSError and turns every nonzero list-windows exit into []:

try:
    probe = self._run(["list-windows", "-t", f"={session}", "-F", "#{window_id}"], check=False)
except (subprocess.TimeoutExpired, OSError) as exc:
    raise TmuxError(f"{self._BINARY} list-windows failed: {exc}") from exc
if probe.returncode != 0:
    return []

The seam contract says [] means "no windows" and that a transport failure raises instead — the two must not be conflated. That holds for the timeout/missing-binary path, but nonzero exits cover more than a vanished session: a server that errors while its windows are alive, a rejected -F format, a target-resolution or CLI-quoting failure on psmux. All of those answer [], i.e. "this session has no windows", with no signal anywhere.

Two live modules already document the sentinel in contradictory terms. psmux_backend._sweep_orphan_keys treats an empty listing as a failed probe, not an empty session, and warns rather than acting on it:

a session being swept just minted a window, so an empty live list is a failed probe, not an empty session — treating it as truth would sweep every key, live windows included

while window_alive (a membership test over this same list) reads it as truth.

Why it matters now

#435 (PR #524) made the ctl-window prune verify its kills: it kills every candidate, takes one list_window_ids(CTL_SESSION), and partitions into removed / survived / unverifiable. An [] from a failed listing classifies every candidate as verifiably removed — the optimistic report that change exists to eliminate.

To be clear about the direction of travel: this is not a regression from #435. Before it, the prune reported every attempted kill as removed unconditionally, so the rc≠0 path is now the only remaining route to an over-optimistic answer where it used to be the guaranteed one. PR #524 documents the ceiling at the call site rather than widening it. Raised by review on that PR (CodeRabbit and two independent reviewers), and held back deliberately — see the scope note below.

Why it is not a one-line fix

list_window_ids backs window_alive, which is the engine's liveness probe. Today a vanished session answers [] → "dead", and the crash/recovery paths are built on that reading. Making nonzero exits raise flips those to "unknowable" repo-wide: this is a change to run supervision, not to cleanup reporting, and it needs its own blast-radius pass over the engine.

It also needs a decision that has no obvious answer:

Which nonzero exits count as "proved gone"? tmux's can't find session and no server running are both nonzero and both genuinely mean gone. Treating all nonzero as unknowable would make an ordinary vanished session unverifiable — which, in the prune, invents a phantom survivor that every subsequent cleanup re-reports and never clears. That is the same dishonest report from the other side. So the fix is a discrimination problem, not a "raise instead of return" problem, and stderr matching is backend-specific (psmux's wording is its own).

Suggested scope

  • Decide the discrimination rule at the seam: which conditions are proved gone (→ []) versus unknowable (→ MultiplexerError), and whether that is stderr matching, a has_session confirmation round trip, or a new tri-state return.
  • Audit window_alive's callers for what an "unknowable" answer must do — the engine must not tear down a possibly-working session on it (the seam already says so; the point is that it currently never gets the chance).
  • Apply the same lens to list_windows and session_options, which have the same rc != 0 → sentinel shape for metadata rather than liveness.
  • Regression test: a nonzero listing while the session is demonstrably still live must not read as an empty session.

Related: #311 (the qualified-id obligation — the other undocumented seam pairing the prune's verdict rests on), #442 (verify._git merging stderr into stdout — the same class of transport sentinel quietly corrupting a probe, in the git chokepoint), #33 / #38 (bias liveness toward alive/unknown rather than dead, same doctrine on the engine-pid side).

Refs #435, #524

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions