test: wait for conditions instead of fixed sleeps in the watcher suites

[fujibee]

CI speedup, part 2 of 2 (part 1 was #501, sharding). Replaces fixed settling
sleeps in the watcher suites with bounded condition waits, and hoists the
polling helpers that already existed ad hoc in three files into
tests/test_helper.bash.

Measured, not estimated

sleep was replaced with a logging shim on PATH and the suite re-run, so the
targets came from what actually blocks rather than from what the source says.

The literal totals are misleading, in two separate ways.

First, the suite contains ~3400s of sleep literals, and 96% of that never
blocks anything: it is sleep 30 3>&- & background stand-ins used as
long-lived fake processes. Deleting them would save nothing.

Second, the shim attributes a sleep by BATS_TEST_FILENAME, and that variable
is inherited by the scripts under test. So part of the remaining ~140s is the
watcher's own poll interval (scripts/watch.sh:488, sleep "$INTERVAL"), not
a test-side wait at all. test_watch.bats has zero bare sleep 1 in its
source yet logged 27 of them — every one from watch.sh. No amount of
test-side work removes those.

What was actually addressable is therefore a good deal smaller than the raw
number suggests, and the A/B below is the honest measure of it.

A/B, two rounds, same machine, git stash between runs

	before	after
test_delivery.bats alone, r1 / r2	86s / 86s	75s / 76s
test_watch.bats + test_delivery.bats, r1 / r2	151s / 151s	135s / 143s

Roughly -12s mean on the pair (~8%). Note the "after" column varies (135 vs
143) where "before" does not: fixed sleeps are deterministic, condition waits
finish when the machine lets them. That is the intended trade.

A single non-A/B run early on read 106s against an 86s baseline and looked like
a regression. It was load from a concurrent job. Nothing was reported until the
stash-based A/B was in hand.

Two negative assertions got stronger, not weaker

Both subscription tests asserted that a message does not arrive, after a
fixed sleep 3 presumed to be "enough poll iterations". That is not an
assertion, it is a guess about timing.

They now wait for a later row to be delivered, which proves the watcher has
already scanned past the row the assertion denies. In the static-subscription
test the existing rows would not carry that proof — the alice row precedes the
bob row, so seeing alice says nothing about bob — so a sentinel row is inserted
after bob's and waited on instead.

A trap worth naming

Tightening a poll from sleep 0.5 to sleep 0.1 while leaving the loop at 10
iterations silently cuts the timeout budget from 5s to 1s. That passes locally
and invents a new flake on a loaded runner. Iteration counts were raised to
keep every ceiling exactly where it was. Faster and less tolerant are not the
same change.

Deliberately left alone

Two fixed sleeps stay, each with a comment saying why, so a future sweep
does not "fix" them:

• test_delivery.bats — separating two mtimes. The thing being waited on is
  the clock; filesystem timestamp granularity is a whole second on some CI
  filesystems. There is no condition to poll.
• test_watch.bats — asserting that nothing further is emitted after one poll
  interval. When the expected outcome is the absence of an event, there is no
  event to wait for.

test_codex_bridge_launcher.bats is not touched here. It is the most
expensive file per test (8.2s), and none of that is settling waits:

sleep 10 3>&- & local parent_a=$!    # stand-in parent
export MOCK_BRIDGE_SLEEP=8           # mock bridge lifetime
...assertions complete in well under 1s...
wait "$parent_a"                     # blocks for the remaining ~10s

Per-test timings confirm it: the five tests with sleep 10/sleep 14 parents
run 10.3-11.3s, the five with sleep 6 run 7.2-7.8s. Duration tracks fixture
lifetime, not work.

Shortening those fixtures is the tempting fix and the dangerous one. The mock
has to outlive the window in which a duplicate dispatcher could appear; if it
exits first, the singleton assertion passes because nothing was left to race —
a falsely green test, not a flaky one. Killing the stand-ins after the
assertions instead of waiting them out looks safe, but it changes the teardown
ordering the lock protocol is being exercised under.

Either way it is a scope call rather than a mechanical conversion, so it is
reported rather than taken unilaterally.

Verification

bats tests/test_watch.bats tests/test_delivery.bats — exit 0, 27/27 and
140/140 locally.

---

What CI caught after the first round (added at head 95da913)

Two failures, only one of them mine.

The conversion introduced a real race (test_watch.bats)

The restart test kills the first watcher, restarts it, and asserts the second
does not re-deliver what the first already streamed. I made it wait for the
message to appear on stdout — but watch.sh writes the line first and
persists the watermark after. Killing on the line can drop the mark, so the
restart re-delivers, which is precisely what the test denies. It failed on
macOS.

It now waits for the watermark file to hold the delivered row's id. Same
mistake in kind as the global-stop finding: waiting for the observable event
is not the same as waiting for the durable one. The sleep 2 it replaced did
not guarantee the write either — it just usually won the race.

A test on main was asserting nothing (test_delivery.bats)

session-end.sh only kills a pid whose command line still looks like
watch.sh — deliberate protection against pid recycling. The fixture was
sleep 30, which can never satisfy that guard, so the kill could never fire.

Measured against main's own code:

BEFORE assertion: ps=[sleep 30]        <- alive
! kill -0 "$target_pid" 2>/dev/null    -> passes
AFTER  assertion: ps=[sleep 30]        <- still alive

The process is alive on both sides of an assertion that claims it is gone. The
test has never verified the behaviour in its own name.

Converting the wait to a poll is what surfaced it: a single check after a fixed
sleep missed it, repeated checking did not. The fixture is now a real watcher, so the guard is
satisfied and the kill actually happens.

To be exact about what was and was not established: the behaviour above is
measured. The mechanism by which kill -0 reports failure for a live process
in that position is not explained here — a reduced bash repro of the same
shape returns 0 three times in a row. Fixing the fixture removes the dependence
on it either way.

Carried in its own commit, per review request.

Retracted: the helper was not evidence of anything yet

An earlier revision of this description said wait_for_pid_exit returning
promptly was the evidence that the process had really died. That was wrong,
and review caught it: the helper read any failed kill -0 as "exited". A
failed kill -0 is ESRCH (dead) or EPERM (alive, but not signalable by us —
sandboxes do this, and a live instance was found in delivery.sh status the
same day). So the helper could report "gone" for a running process, which is
the defect the session-end fixture was just fixed for, reintroduced one level
down inside the helper written to prevent it.

Saying "gone" now requires kill(2) and the process table to agree, mirroring
_agmsg_pid_alive. The loop also reaps first: an unreaped zombie still answers
kill -0, so a process that had exited could look alive for the whole
timeout — a separate path to the same class of lie.

tests/test_wait_helpers.bats pins this. The EPERM branch cannot be produced
portably in-suite, so the decision rule itself is pinned: with kill stubbed to
report "Operation not permitted" and ps giving no evidence either way, the
answer must still be ALIVE.

The mechanism behind the original observation is not explained. Three
controlled probes — consecutive calls in the test shell, calls in a subshell,
and stderr captured to files — all had kill -0 return 0 correctly. That it
does not reproduce on demand is itself the argument for not depending on it.