Skip to content

perf(tmux): prove endpoint existence in one tmux round-trip, not two - #109

Open
ruby-dlee wants to merge 2 commits into
mainfrom
fm/tmux-probe-single-roundtrip
Open

perf(tmux): prove endpoint existence in one tmux round-trip, not two#109
ruby-dlee wants to merge 2 commits into
mainfrom
fm/tmux-probe-single-roundtrip

Conversation

@ruby-dlee

Copy link
Copy Markdown
Owner

Single-purpose follow-up to #108. One function, one behaviour-neutral change.

What

fm_backend_target_exists ran the identity guard and then probed existence again, so a labelled check cost two tmux invocations where the old display-message probe cost one. Measured with a counting tmux shim against a live server, for a labelled session:window target:

tree tmux invocations per fm_backend_target_exists
before #108 1display-message -p -t S:fm-task-x1 '#{pane_id}'
#108 as merged 2has-session -t '=S:=fm-task-x1' and has-session -t S:fm-task-x1
this PR 1has-session -t '=S:=fm-task-x1'

The second probe re-asks a question the first already answered. When the caller supplies an expectation, the guard resolves the target in order to prove identity — exactly for a session:window, and by reading the id back for a @window-id — so its success is the existence proof. With no expectation there is nothing to verify and the probe is the whole check. Either way it is now one round-trip.

Why this is worth a PR rather than a footnote

This is a hot path. bin/fm-teardown.sh's managed_endpoint_is_gone calls it in a bounded retry loop of up to ten iterations, so the redundant probe was a doubled process spawn per iteration, per task, on every teardown.

Process count on this path is not a theoretical cost right now. A separate fm_run_bounded defect had been leaking its perl supervisor — 345 orphaned processes were cleared tonight, the oldest over thirty hours — and that leak was breaking gh-axi and fm-crosscheck outright by pushing a process-argv inventory past its bound. Spawn pressure on the existence path is the same resource that took three tools down. Halving it in a retry loop is worth more than the diff size suggests.

Behaviour is unchanged

The point of #108 was that a gone window must be detected as gone. That contract is re-verified here, not assumed:

  • The before/after control (live vs deliberately bogus vs created-then-killed window, across session:window, session:window.pane, @window-id, %pane-id and bare selectors) passes with 0 failures.
  • tests/fm-backend-tmux-target-exists.test.sh stays 14/14, including the closed-window lifecycle, exact-vs-prefix resolution, agent liveness, kill idempotency, and the assertion that the existence primitive never uses display-message.
  • fm-crew-state 63, fm-secondmate-liveness 16, fm-session-start 17, fm-send-strict 23, fm-fleet-snapshot-view 13 — all green locally.
  • bin/fm-lint.sh clean.

tests/fm-backend.test.sh stops locally at fm-spawn.sh should succeed for a project reached through a symlinked prefix… with checkout-refresh LaunchAgent namespaces cannot be safely enumerated. That reproduces identically on unmodified origin/main on this host and is a host LaunchAgent-plist condition, not this change.

How the redundancy was found

Not by reading the diff — it is invisible there. It surfaced while investigating a timing-sensitive teardown test, by counting actual tmux invocations at runtime with a shim. Worth noting for anyone reviewing similar guard/probe pairs.

PR must be raised via no-mistakes fails by construction and is not a gate for this repo's own work per the repo owner.

fm_backend_target_exists ran the identity guard and then probed existence
again, so a labelled check cost two tmux invocations where the previous
display-message probe cost one:

  before  display-message -p -t S:fm-task-x1 '#{pane_id}'
  after   has-session -t '=S:=fm-task-x1'   AND   has-session -t S:fm-task-x1

The second probe re-asks a question the first already answered. When the caller
supplies an expectation, the guard RESOLVES the target to prove identity -
exactly for a session:window, and by reading the id back for a @window-id - so
its success is itself the existence proof. With no expectation there is nothing
to verify and the probe is the whole check. Either way it is now one round-trip.

This is a hot path, not an incidental one: fm-teardown.sh's
managed_endpoint_is_gone calls it in a bounded retry loop of up to ten
iterations, so the redundant probe was a doubled process spawn per iteration.

Measured with a counting tmux shim on a live server, labelled session:window
target: 1 invocation before this branch's parent, 2 on it, 1 again with this
change. Behaviour is unchanged - the closed-window/bogus-target control passes
with no failures and tests/fm-backend-tmux-target-exists.test.sh stays 14/14.
Dropping the redundant second probe means a labelled existence check is now
proven ONLY by the identity guard's exact-match resolve ("=session:=window").
Real tmux treats that spelling and the plain one identically - both fail once
the window is killed - but two fixtures were distinguishing them, so they
reported a killed endpoint as still alive and secondmate teardown refused with
"endpoint is still alive; refusing child cleanup".

tests/secondmate-helpers.sh matches the probe target literally against its
kill-window log, so it now normalises the "=" markers before comparing.

fm-teardown-suite.sh's dotted-id registry-lock case modelled a LIVE endpoint
while asserting that teardown succeeds. It only passed because the stub's
list-windows answers with a hardcoded fm-task-x1, which never matched that
task's fm-foo.bar window, so the bare-name lookup missed it whatever the live
marker said. The case is about registry locking and a secondmate teardown
legitimately requires a quiesced endpoint, so it now models one with the same
`rm -f fakebin/.tmux-live` idiom the rest of the suite uses.

Verified: fm-secondmate-lifecycle-e2e 6, fm-secondmate-safety 70,
fm-backlog-handoff 9, and the isolated dotted-id case all pass.
@ruby-dlee
ruby-dlee force-pushed the fm/tmux-probe-single-roundtrip branch from 3c1f4a0 to aa71d59 Compare August 8, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant