test(watch): say what the file held when a delivery wait times out (#1000) - #1008
Merged
Merged
Conversation
…1000) The watch liveness case (#67) failed once each on three PRs in one night, on three platforms, and never on main; none of the three logs could say whether the watcher had not delivered or had already exited, because the wait helper reported only "did not appear". On timeout it now prints the file's state -- missing, empty, or its lines -- and, when the caller hands it the watcher's pid, whether that watcher is still running. The #67 site passes its pid. Nothing changes on the green path; the helper still returns 1 on timeout.
…luded Review found the diagnostic committing the two mistakes it exists to catch. `kill -0` returning false was reported as "watcher exited" -- but kill -0 also fails on EPERM and on an observation error, so that wording claimed absence from a failed presence check (pid 1 is alive and answers false, measured). It now reports "kill -0 could not confirm it running (exited, or not observable)". And `wc -l` counts newlines, not lines, so a partial line mid-write read as nothing; the dump now prints the byte count and whether the last line is unterminated, which tells "wrote nothing" from "a write may be in progress", and closes the stream after an unterminated line so the next diagnostic does not run on.
…of a live file Review found the dump observing the live file four separate times -- wc -c, a first tail -c 1, sed, a second tail -c 1 -- with a live writer free to append between them, so the byte count, the terminator verdict and the printed content could describe a state that never existed. Everything is computed from a single snapshot copy now, the header says "snapshot at timeout", and a snapshot that cannot be taken is reported as exactly that rather than guessed around.
fujibee
added a commit
that referenced
this pull request
Aug 31, 2026
…me, and tmux/herdr/plain drivers (#terminal-driver v1, PR1)
New driver axis "terminals": the ONE terminal a member's CLI runs under (tmux,
herdr, or plain), behind a locked ABI, so the terminal operations currently
inlined as $TMUX / HERDR_* branches across spawn/despawn/watch can move behind a
contract and peek/poke can be added. This PR is new files only — nothing existing
is rewired yet (that is PR2/PR3), so no existing behavior can regress.
Contract (5 verbs + terminal_name, per docs/spec/driver-interface.md):
terminal_check / terminal_describe ABI-required (deps, metadata)
terminal_detect <session_id> RECORD op: ONE observation returns BOTH
which terminal we are under AND this
session's own pane id. Deliberately not two
ops: splitting them lets state change
between the two observations (the defect
shape corrected three times the same night
— _wait_role_count, #1008's four reads).
herdr resolves the pane from the session id
via `agent list` (inherited HERDR_PANE_ID
is NOT trusted); tmux uses $TMUX_PANE; plain
is the exit-0 fallback.
terminal_spawn / despawn / peek / poke / name
Registry (scripts/lib/terminal-registry.sh): a terminals facade over
driver-registry.sh. terminal.conf is DATA read by a clone of agmsg_type_get;
ops.sh is sourced into the caller. Resolution: --terminal / AGMSG_TERMINAL >
detection (order herdr > tmux > plain, so it never fails) — the single answer
that ends the historical $TMUX-vs-HERDR_* dual system; detection sources each
candidate in a subshell so terminal_* definitions do not clobber across
candidates. Record ref is <terminal>:<id>; reading tolerates the pre-axis forms
(a bare %N/@n reads as tmux, the old herdr:<id> still reads as herdr; first-colon
split so a herdr id's own ':' survives).
Drivers: tmux (faithful to the pre-axis argv), herdr, plain (peek/poke/despawn/
name -> status 13 unsupported, reason on stderr). plain's detect is the fallback.
Tests (tests/test_terminal_registry.bats, 18): resolution incl. precedence and
order, record scheme incl. legacy/inner-colon, conf reader, and every driver op
against fake tmux/herdr binaries that record argv. The fake binaries record that
a binary was CALLED, not that the RIGHT driver was selected — so the resolution
tests assert the returned terminal NAME, and a selection->op test loads the
resolved terminal and asserts the op reaches the herdr binary and never tmux, so
a wrong selection cannot pass as "argv as expected". Mutation-verified: a
single-burst text+Enter poke (dropping the #619 arrow), and a tmux-before-herdr
detection order (which reddens both the order test and the selection->op test),
each redden their test.
MEASURED vs ASSERTED (herdr): the pre-axis calls (pane split/rename/run, tab
create, pane close), `agent list` being JSON, name encoding team__name, and
`pane read --source visible` are grounded in seat-0 measurements and the existing
tree. The EXACT argv of `herdr agent prompt` (poke) and the `agent list` JSON
field names used to extract the pane (agent_session / pane_id) are asserted from
the assignment brief and verified only by the live matrix on the real CLI, not
here; the fixtures pin the control flow and the argv this driver emits, so a
real-CLI mismatch is a localized one-line fix the matrix catches.
Source of the scope (no committed scope/naming-map doc exists): the assignment
messages, the seat-0 measurements, and the existing code + driver ABI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1000 — the first item under "what settles it": what was in the file when the wait gave up.
The watch liveness case (#67) failed once each on three PRs in one night, on three platforms, and zero times on
main; none of the three logs could say whether the watcher had failed to deliver or had already exited, because_wait_for_file_containsreported only that the needle "did not appear". The distinction #1000 needs — "the watcher never wrote" vs "it wrote something else" vs "it was already gone" — was unobservable after the fact.On timeout the helper now prints what it saw: the file missing, present-but-empty, or its line count and lines; and, when the caller passes the watcher's pid (the #67 site now does), whether that watcher was still running or had exited. Nothing changes on the green path, and the helper still returns 1 on timeout — this adds observation, not behavior. The dump was exercised deliberately against a wrong needle, a one-line file and a dead pid before committing; the #67 case is green under bash 5 and
/bin/bash3.2, and the enforceable-assertions check sits at its baseline.The next time this test fails on any PR, its log will answer #1000's first question by itself. Scope:
tests/test_watch.batsonly; no overlap with #991's watch changes (it uses the helper but does not define it).