43b9ed13

review: pin what the blind path moved, and stop assuming leg 3's premise

a73x 2026-08-21 18:19

Commit message

review: pin what the blind path moved, and stop assuming leg 3's premise

Review round on the two commits before this one. Nothing was a MUST-FIX;
the two that mattered were both invisible.

**The blind path moved side-channel replay and nobody noticed.** noteBlind
has no `.none` case, so a gap advances seq on every pty chunk. Events are
stamped at tracker.seq and replayed strictly above the reattaching client's
watermark, so a bare BEL or an OSC 52 with no redraw behind it — previously
stamped at the seq the gap began on and dropped — now survives a delta
reattach. Better behaviour, arrived at sideways, under two comments that
said the opposite: replayPending's doc called the drop "the price", and the
gap fixture's `gap-open` bullet justified itself on "only a changed cell, a
moved cursor or history growth advances tracker.seq". Both corrected, both
now saying which path they still describe.

Pinned in delta.zig rather than through the gap fixture, which prints a
visible marker first precisely so it never depended on the seq rule — there
is no deterministic way to pin it from that side, because whether the
escapes share a pty chunk with the marker is not ours to decide.

**noteBlind's cursor and history_rows were unpinned.** They go straight
into the delta header, so dropping them repaints the right text with the
cursor parked where the gap began — and deleting either line left the whole
unit suite green. Now asserted against the engine in the reattach test.
Watched to fire: deleting `self.cursor = eng.cursorPos()` gives
"expected 1, found 0". The client that would have worn it is a browser
reconnect; the CLI opens at seq 0 and takes the snapshot arm.

**Leg 3 assumed its own premise.** "Nobody attached" is the leg's entire
meaning and it was resting on leg 2's waitexit, which proves the mux
process is gone, not that the daemon reaped the slot. Those are different
instants. Now polled out of `muxd stats`, and watched to fire by mutating
the expected count.

Also: every ceiling is env-overridable, because `make ci` gates delivery on
bounds calibrated against one idle 16-core box and the only other way for a
slow machine to pass is deleting the target. Verified with SOLO_MAX_MS=1.
DPID is assigned before the trap that dereferences it under `set -u`, and
guarded with `if` rather than `&&` so the guard cannot itself trip `set -e`.
Leg 3's two output guards now match leg 2's, and the client leg's timeout
message says 90s, which is what the expect actually bounds.

docs/decisions.md gets the entry this work owed it: the measurements with
the box they came from, the three obligations of the blind path, the
user/system split that a user-space profile cannot show, the gate's
calibration, and the one warning worth leaving — hasClientsIn counts a 0x0
attach and must, because the webhub stand-in reads real content back
through one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDnJ6v15RvwZ2Y3zvfrWpR

docs/decisions.md

+offered an agent to, a nested `mux -A` must exit 2 AND say so with the
+preflight's message, because the self-attach refusal standing behind it also
+exits 2 and the code alone cannot tell which one spoke.
+## 2026-08-21 (a detached session stops rendering, and what that moved)
+Every number below is one box: idle 16-core x86_64, ReleaseSafe, `make
+throughput`'s own harness. They are quoted so a later reading has something
+to disagree with, not because they transfer.
+**`make install` shipped Debug for the whole prototype's life.** `install`
+depended on `build`, which is `zig build` with no `-Doptimize`, so
+`~/.local/bin` got a Debug tree — and ghostty gates its page-integrity check
+on ITS OWN optimize mode, rebuilding a hash map over every cell and standing
+up a fresh DebugAllocator per page mutation. 20k lines of `yes` at 80×24, no
+client: **3713ms installed, 6ms ReleaseSafe, 6ms tmux, 24ms a bare pty.**
+perf put 96% of cycles in `verifyIntegrity`. The dev tree stays Debug on
+purpose — the assertions are worth having while working on the engine — so
+`install` and `throughput` each build ReleaseSafe into their own prefix
+rather than flipping the default. The rule that follows: **no speed number
+measured in the dev tree means anything**, and `make test`/`e2e`/`soak` all
+inherit Debug.
+**A user-space profile misranked the thing it was used to rank.** The
+detached daemon's cost looked like the formatter (57% in
+`PageFormatter.writeCodepointWithReplacement`) with the allocator a ~5%
+footnote. The real split was `utime 24.1s / stime 106.5s` — **81% in the
+kernel**, from `dumpVtRow` taking a fresh GPA buffer per row, fifty per pty
+read, churning mmap/munmap and page faults. `perf record -p` cannot see it:
+those frames come back as unresolved `[unknown]` addresses. Removing the
+loop cut instructions 5.9× and wall time 27× (64784ms → 2361ms on 374MB of
+full-width repaint at 200×50); the missing 4.7× was system time. **Check the
+user/system split before ranking work off a profile.**
+**The blind path owes three things `update()` used to provide.** `seq` must
+advance — it is stamped into `last_return.seq` and two commands returning
+during one blind stretch must not share one, or an await cannot tell which
+it was told about. Every `row_seq` must be stamped, so a reattach gets a
+delta carrying the whole grid rather than one omitting what moved unseen.
+And the hashes must be marked stale: blind output moves a row A→B, the
+reattach delta carries B, but the stored hash still says A — so a later B→A
+reads as "unchanged", nothing is sent, and the client shows B for the rest
+of the session. Only a change-and-change-back triggers it and nothing else
+in the tracker would catch it.
+**It moved side-channel replay, and that was not intended.** `noteBlind` has
+no `.none` case, so a gap advances seq on every pty chunk. Events are
+stamped at `tracker.seq` and replayed strictly above the reattaching
+client's watermark, so a bare BEL or an OSC 52 with no redraw behind it —
+previously stamped at the seq the gap began on and therefore dropped — now
+survives a delta reattach. Better behaviour, arrived at sideways. Pinned in
+delta.zig rather than in the gap fixture, which prints a visible marker
+first precisely so it never depended on the seq rule.
+**`hasClientsIn` counts a 0×0 attach, and must.** `muxa run`/`await` hold a
+client slot for the whole verb, so the win does not reach the agent
+workload — those sessions take the rendering path. The tempting narrowing,
+treating `cols == 0` as "not watching", is **unsafe**: the webhub stand-in
+attaches at 0×0 and reads real content back over its WebSocket — the e2e
+scenario whose comment reads "A POSTed tile is a REAL tile". 0×0 means
+"claims no grid", never "wants no bytes". Anchored on that sentence rather
+than a line number, which drifts.
+**The throughput gate's bounds are calibrated, not principled.** Best-of-5
+per leg, because noise only ever adds time and a regression moves the
+minimum as much as the mean. Solo 6ms/ceiling 10; client 45-60ms with a 60ms
+comfort target that only reports and a 75ms ceiling — a hard 60 flaked 1 run
+in 10 with nothing wrong, and a gate pinned to the noise floor gets disabled
+by its third false alarm. Repaint 87ms/ceiling 200, set near the good number
+rather than under the bad one: at 400 a machine twice as fast runs the
+BROKEN build in 240ms and the gate says nothing. All bounds are env
+overridable so a slower box can relax them without deleting the target.
+`yes` cannot see any of this — one-character rows are nearly free to render,
+which is why the regression this branch fixed was invisible to the first two
+legs (5ms vs 6ms) and obvious to the third (87ms vs 479ms).

src/delta.zig

+    try std.testing.expect(
+        std.mem.indexOf(u8, composed.bytes, "printed with nobody watching") != null,
+    );
+    // The header, not only the rows. buildDeltaSince serialises the
+    // TRACKER's cursor and history_rows, so a blind path that stamped every
+    // row but forgot to refresh those two would repaint the right text with
+    // the cursor parked where the gap began. Deleting either line in
+    // noteBlind left the entire unit suite green until this assertion
+    // existed, and the client that would wear it is a browser reconnect —
+    // the CLI opens at seq 0 and takes the snapshot arm instead.
+    const hdr = try proto.readDeltaHeader(payload);
+    const cur = eng.cursorPos();
+    try std.testing.expectEqual(cur.x, hdr.cursor_x);
+    try std.testing.expectEqual(cur.y, hdr.cursor_y);
+    try std.testing.expectEqual(eng.historyRows(), hdr.history_rows);
+}
+test "DeltaTracker: a blind chunk that changed nothing still advances seq" {
+    // Reads like a triviality; it is the premise two comments in server.zig
+    // now rest on. Side-channel events (OSC 52, a bell) are stamped at
+    // tracker.seq and replayed only when strictly ABOVE the reattaching
+    // client's watermark. update() answers .none for a chunk that moved no
+    // cell, which left a bare BEL during a gap stamped at the seq the gap
+    // began on and therefore never replayed — replayPending calls that out
+    // as the price it pays. noteBlind has no .none case, so during a gap
+    // those events now land above the watermark and survive the reattach.
+    //
+    // Deleting the unconditional `self.seq += 1` is what silently reverts
+    // that, and no server test would notice: the gap fixture prints a
+    // visible marker first precisely so it never depends on this.
+    const alloc = std.testing.allocator;
+    const eng = try Engine.init(alloc, .{ .cols = 80, .rows = 24 });
+    defer eng.deinit();
+    var tracker: DeltaTracker = .{};
+    defer tracker.deinit(alloc);
+    try tracker.rebuild(alloc, eng, 24, 80);
+    // No feed at all: the strongest form of "this chunk changed nothing".
+    const before = tracker.seq;
+    switch (tracker.noteBlind(eng)) {
+        .advanced => {},
+        else => return error.ExpectedAdvance,
+    }
+    try std.testing.expect(tracker.seq > before);
+test "DeltaTracker: two blind stretches never share a seq" {

src/server.zig

Old		New
@@ -3363,12 +3363,21 @@ pub const Server = struct {
3363	///	3363	///
3364	/// `> have_seq`, not `>=`: an event stamped at the seq the client already	3364	/// `> have_seq`, not `>=`: an event stamped at the seq the client already
3365	/// quotes is one it was there for, and replaying it would set the	3365	/// quotes is one it was there for, and replaying it would set the
3366	/// clipboard twice for a client that never missed anything. The price is	3366	/// clipboard twice for a client that never missed anything.
3367	/// that an event whose pty chunk changed no cell — a bare BEL, an OSC 52	3367	///
3368	/// with no redraw behind it — is stamped at the seq the gap began on and	3368	/// That used to cost an invisible chunk its replay: update() answers
3369	/// so is not replayed. Live delivery is unaffected; only the replay of an	3369	/// `.none` when no cell moved, so a bare BEL or an OSC 52 with no redraw
3370	/// invisible chunk is lost, and buying it back would mean handing out	3370	/// behind it stayed stamped at the seq the gap began on, and this loop
3371	/// duplicates to every client that was watching.	3371	/// refused it. It no longer does — and not by design. A gap is by
		3372	/// definition unattached, `noteBlind` has no `.none` case, so every pty
		3373	/// chunk during one advances seq and every event drained after it lands
		3374	/// strictly above the departed client's watermark. Pinned in delta.zig
		3375	/// ("a blind chunk that changed nothing still advances seq"), because
		3376	/// nothing on this side would notice it going away again.
		3377	///
		3378	/// The `>` still binds for an ATTACHED client, which is what it was
		3379	/// written for: the alternative is handing duplicates to everyone who
		3380	/// was watching, and that is still the wrong trade.
3372	fn replayPending(self: *Server, si: usize, i: usize, have_seq: u64) void {	3381	fn replayPending(self: *Server, si: usize, i: usize, have_seq: u64) void {
3373	const s = self.ses(si);	3382	const s = self.ses(si);
3374	// Enum declaration order, so clipboard precedes bell. Fixed rather	3383	// Enum declaration order, so clipboard precedes bell. Fixed rather
@@ -9115,12 +9124,19 @@ test "Server: a bell in a later chunk is its own frame, not folded into the firs
9115	/// - `gap-open`: removing it leaves the suite green and no mutation	9124	/// - `gap-open`: removing it leaves the suite green and no mutation
9116	/// uncovered. It removes a dependency instead; see its bullet.	9125	/// uncovered. It removes a dependency instead; see its bullet.
9117	///	9126	///
9118	/// - `gap-open` FIRST, and visible. Only a changed cell, a moved cursor or	9127	/// - `gap-open` FIRST, and visible. This was load-bearing back when only a
9119	/// history growth advances tracker.seq, so a burst of pure escapes can be	9128	/// changed cell, a moved cursor or history growth advanced tracker.seq: a
9120	/// drained at the same seq the departing client already held — and the	9129	/// burst of pure escapes could then drain at the same seq the departing
9121	/// replay is `> have_seq`, which would then refuse them. Printing	9130	/// client already held, and the replay is `> have_seq`, which would
9122	/// something visible before any escape guarantees every stamp below is	9131	/// refuse them. Printing something visible first put every stamp below it
9123	/// strictly above the watermark, whatever the pty happens to chunk.	9132	/// strictly above the watermark, whatever the pty happened to chunk.
		9133	///
		9134	/// A gap is unattached, and the unattached path now advances seq on every
		9135	/// chunk (`noteBlind`; the consequence is spelled out at replayPending),
		9136	/// so the escapes would clear the watermark unaided. Kept regardless: it
		9137	/// costs one printf, it keeps this fixture honest if the blind path ever
		9138	/// regains a `.none`, and the dependency argument below never rested on
		9139	/// the seq rule in the first place.
9124	///	9140	///
9125	/// Removing it leaves the suite GREEN, and that is not an argument for	9141	/// Removing it leaves the suite GREEN, and that is not an argument for
9126	/// removing it. What it removes is a DEPENDENCY: without it these tests	9142	/// removing it. What it removes is a DEPENDENCY: without it these tests

test/throughput.sh

Old		New
@@ -38,11 +38,19 @@ MUXA="$3"
38	PTYCLIENT="$4"	38	PTYCLIENT="$4"
39		39
40	# Lines of `yes`, and the wall-clock ceiling each leg must come in under.	40	# Lines of `yes`, and the wall-clock ceiling each leg must come in under.
		41	#
		42	# Every bound is overridable from the environment. All of them were
		43	# calibrated on one idle 16-core box, and `make ci` now gates delivery on
		44	# them, so a slower machine has to be able to relax the gate without
		45	# editing this file — otherwise the only move left is deleting the target,
		46	# and a deleted gate catches nothing. SOLO_MAX_MS carries the thinnest
		47	# margin of the three: 10ms against an observed 6ms, on a bracket that
		48	# includes `yes \| head` fork and pipe cost.
41	SOLO_LINES=20000	49	SOLO_LINES=20000
42	SOLO_MAX_MS=10	50	SOLO_MAX_MS=${SOLO_MAX_MS:-10}
43	CLIENT_LINES=200000	51	CLIENT_LINES=200000
44	CLIENT_WARN_MS=60	52	CLIENT_WARN_MS=${CLIENT_WARN_MS:-60}
45	CLIENT_MAX_MS=75	53	CLIENT_MAX_MS=${CLIENT_MAX_MS:-75}
46	# Leg 3 repaints a full screen over and over with NOBODY attached. Its	54	# Leg 3 repaints a full screen over and over with NOBODY attached. Its
47	# own leg because `yes` cannot see what it sees: a one-character row is	55	# own leg because `yes` cannot see what it sees: a one-character row is
48	# nearly free to render, so the solo leg above stays flat through a	56	# nearly free to render, so the solo leg above stays flat through a
@@ -50,7 +58,7 @@ CLIENT_MAX_MS=75
50	# vim, logs, a build — is full-width, and the daemon renders every row	58	# vim, logs, a build — is full-width, and the daemon renders every row
51	# of it to hash it.	59	# of it to hash it.
52	# 2^REPAINT_DOUBLINGS frames of ~1.8KB; 13 is ~15MB.	60	# 2^REPAINT_DOUBLINGS frames of ~1.8KB; 13 is ~15MB.
53	REPAINT_DOUBLINGS=13	61	REPAINT_DOUBLINGS=${REPAINT_DOUBLINGS:-13}
54	# Measured either side of the fix, same box, best-of-5: 87ms with the	62	# Measured either side of the fix, same box, best-of-5: 87ms with the
55	# detached check in place, 479ms without it. The ceiling sits near the	63	# detached check in place, 479ms without it. The ceiling sits near the
56	# good number rather than under the bad one — put it at 400 and a machine	64	# good number rather than under the bad one — put it at 400 and a machine
@@ -59,13 +67,22 @@ REPAINT_DOUBLINGS=13
59	#	67	#
60	# 5.5x here against 27x at 200x50: the penalty is per rendered row, and	68	# 5.5x here against 27x at 200x50: the penalty is per rendered row, and
61	# this leg renders 23 of 80 columns rather than 49 of 200.	69	# this leg renders 23 of 80 columns rather than 49 of 200.
62	REPAINT_MAX_MS=200	70	REPAINT_MAX_MS=${REPAINT_MAX_MS:-200}
63	# Repeats per leg; the fastest one is the reading. See the header.	71	# Repeats per leg; the fastest one is the reading. See the header.
64	REPS=5	72	REPS=${REPS:-5}
65		73
66	TMPD="$(mktemp -d "${TMPDIR:-/tmp}/muxd-throughput-XXXXXX")"	74	TMPD="$(mktemp -d "${TMPDIR:-/tmp}/muxd-throughput-XXXXXX")"
67	SOCK="$TMPD/t.sock"	75	SOCK="$TMPD/t.sock"
68	cleanup() { kill "$DPID" 2>/dev/null \|\| true; rm -rf "$TMPD"; }	76	# Assigned BEFORE the trap is installed, not after the daemon launches:
		77	# cleanup dereferences it under `set -u`, so a trap firing in between would
		78	# die on an unbound variable and never reach the rm. Guarded with `if`
		79	# rather than `&&` because a failing AND-OR list as a function's first
		80	# statement is exactly the shape `set -e` is entitled to act on.
		81	DPID=""
		82	cleanup() {
		83	if [ -n "$DPID" ]; then kill "$DPID" 2>/dev/null \|\| true; fi
		84	rm -rf "$TMPD"
		85	}
69	trap cleanup EXIT INT TERM	86	trap cleanup EXIT INT TERM
70		87
71	"$MUXD" run --sock "$SOCK" --shell /bin/sh --cols 80 --rows 24 &	88	"$MUXD" run --sock "$SOCK" --shell /bin/sh --cols 80 --rows 24 &
@@ -137,7 +154,7 @@ RC=0
137	-- "$MUX" --sock "$SOCK" < "$TMPD/script" > "$TMPD/log" 2>&1 \|\| RC=$?	154	-- "$MUX" --sock "$SOCK" < "$TMPD/script" > "$TMPD/log" 2>&1 \|\| RC=$?
138	[ "$RC" -eq 0 ] \|\| {	155	[ "$RC" -eq 0 ] \|\| {
139	echo "throughput FAIL: client leg exited $RC"	156	echo "throughput FAIL: client leg exited $RC"
140	echo " (3 = the floods never completed inside 60s)"	157	echo " (3 = the floods never completed inside the expect's 90s)"
141	cat "$TMPD/log"	158	cat "$TMPD/log"
142	exit 1	159	exit 1
143	}	160	}
@@ -166,7 +183,27 @@ echo "client: $CLIENT_LINES lines, attached: ${CLIENT_MS}ms (target ${CLIENT_WA
166	}	183	}
167		184
168	# ---- leg 3: full-width repaint, nobody attached ------------------------	185	# ---- leg 3: full-width repaint, nobody attached ------------------------
169	# The client from leg 2 has quit, so this is a detached session again.	186	# "Nobody attached" is this leg's entire meaning, so it is asserted rather
		187	# than assumed. Leg 2's `waitexit` proved the mux PROCESS is gone; it did
		188	# not prove the daemon has reaped the client slot, and those are different
		189	# instants. Measured with a slot still open, this leg would be timing the
		190	# attached path against a detached ceiling and failing for the wrong
		191	# reason. The greedy `.*` takes the LAST clients= in the line, which is the
		192	# per-session count rather than the daemon-wide one.
		193	i=0
		194	while [ "$i" -lt 50 ]; do
		195	if [ "$("$MUXD" stats --sock "$SOCK" \| sed -n 's/.clients=$[0-9]$.*/\1/p')" = "0" ]; then
		196	break
		197	fi
		198	sleep 0.1
		199	i=$((i+1))
		200	done
		201	[ "$i" -lt 50 ] \|\| {
		202	echo "throughput FAIL: leg 2's client is still attached, so leg 3 would not"
		203	echo " be measuring a detached session at all"
		204	"$MUXD" stats --sock "$SOCK"
		205	exit 1
		206	}
170	#	207	#
171	# One frame: cursor home, then 23 rows of 79 columns. 23 and not 24 so the	208	# One frame: cursor home, then 23 rows of 79 columns. 23 and not 24 so the
172	# screen is REPAINTED rather than scrolled — scrolling is a different cost	209	# screen is REPAINTED rather than scrolled — scrolling is a different cost
@@ -200,8 +237,14 @@ while [ ! -s "$TMPD/repaint.done" ] && [ "$i" -lt 120 ]; do sleep 0.5; i=$((i+1)
200	echo "throughput FAIL: $REPS repaints of $REPAINT_BYTES bytes unfinished after 60s"	237	echo "throughput FAIL: $REPS repaints of $REPAINT_BYTES bytes unfinished after 60s"
201	exit 1	238	exit 1
202	}	239	}
203	REPAINT_ALL="$(cat "$TMPD/repaint")"	240	REPAINT_ALL="$(cat "$TMPD/repaint" 2>/dev/null \|\| true)"
204	REPAINT_MS="$(echo "$REPAINT_ALL" \| sort -n \| head -1)"	241	REPAINT_MS="$(echo "$REPAINT_ALL" \| sort -n \| head -1)"
		242	# Same two guards as leg 2's, in the same order. Unreachable given the
		243	# `-s repaint.done` poll above, but an asymmetry between twins is an
		244	# invitation to "fix" whichever one someone reads second.
		245	[ -n "$REPAINT_MS" ] \|\| {
		246	echo "throughput FAIL: repaint leg recorded no timing"; exit 1;
		247	}
205	[ "$(echo "$REPAINT_ALL" \| wc -l)" -eq "$REPS" ] \|\| {	248	[ "$(echo "$REPAINT_ALL" \| wc -l)" -eq "$REPS" ] \|\| {
206	echo "throughput FAIL: wanted $REPS repaint readings, got: $REPAINT_ALL"	249	echo "throughput FAIL: wanted $REPS repaint readings, got: $REPAINT_ALL"
207	exit 1	250	exit 1