a73x

c0d8a460

fix: review round 1 — comments that name what fails when they are wrong

a73x   2026-08-28 19:53

Commit message
fix: review round 1 — comments that name what fails when they are wrong

I1: `fn run`'s doc block said "every code this function picks is 1" on a
function that returns 2 twelve lines down (the no-key refusal). The
arithmetic goes; the claim that survives is the enforceable one — every
code it picks is a boot failure caught before anything bound.

M1: the lifetime test hung its shell up with `die 0`, so the retired
contract and the new one both answered 0 and the exit-code pin proved
nothing. `die 7` makes it a pin: under the retired contract `code` is 7.

M2: `waitStats` gave each request 200 ms and shared one socket, so a reply
that arrived late was read as the answer to the NEXT question — and every
reply queued during the `sessions=0` loop predates the hangup, i.e. says
`sessions=1`. It now opens a connection per call. A drain would have left
the other half open: a request the first loop gave up on can still be
answered after the drain. A socket with no history cannot answer the wrong
question at all.

M3: the e2e `sleep 2` claimed to be the window the old exit-on-empty arm
spent its grace on. That arm had no grace — `reap` handed the code to `run`
in the same pass. It is a chosen settle window, and the comment now says so
and says what it buys.

And, carried from Task 1's review: xversion.sh's hermetic-home rationale
still counted throwaway sockets in stripes, which went with Task 1. The
reason is unchanged; the mechanism is host lines and their tiles.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
src/cli/main.zig
Old New
@@ -544,12 +544,11 @@ fn resumeRun(alloc: std.mem.Allocator, o: Opts, resume_fd: std.posix.fd_t) !u8 {
544 } 544 }
545 545
546 /// `muxd run`: the daemon, in the foreground. Every code this function picks 546 /// `muxd run`: the daemon, in the foreground. Every code this function picks
547 /// is 1, and every one of them is a boot failure — an operator mistake caught 547 /// is a boot failure — an operator mistake caught before anything bound.
548 /// before anything bound. Reaching `srv.run()` means the daemon served, and 548 /// Reaching `srv.run()` means the daemon served, and it answers 0 whenever
549 /// it answers 0 whenever something asks it to stop. No session's exit is ever 549 /// something asks it to stop. No session's exit is ever reported here: a
550 /// reported here: a shell's code goes to that shell's own clients 550 /// shell's code goes to that shell's own clients (`exit_status`), and an
551 /// (`exit_status`), and an emptied daemon is one with nothing on it rather 551 /// emptied daemon is one with nothing on it rather than one that is leaving.
552 /// than one that is leaving.
553 fn run(alloc: std.mem.Allocator, o: Opts, sock_path: []const u8) !u8 { 552 fn run(alloc: std.mem.Allocator, o: Opts, sock_path: []const u8) !u8 {
554 // Address and key are settled before anything binds: a mistyped address 553 // Address and key are settled before anything binds: a mistyped address
555 // or an unreadable key must not first leave a session socket and a live 554 // or an unreadable key must not first leave a session socket and a live
src/server_test_session.zig
Old New
@@ -1091,14 +1091,21 @@ fn awaitFrameThreaded(
1091 return null; 1091 return null;
1092 } 1092 }
1093 1093
1094 /// Ask the daemon for its stats until `want` appears in the text. The 1094 /// Ask the daemon for its stats until `want` appears in the text — what
1095 /// counter is the daemon's own account of its table — what `muxd stats` 1095 /// `muxd stats` shows an operator looking at a box with nothing on it.
1096 /// shows an operator looking at a box with nothing running on it. 1096 ///
1097 fn waitStats(alloc: std.mem.Allocator, fd: std.posix.fd_t, want: []const u8) !void { 1097 /// A connection of its OWN, per call, and that is the assertion's teeth: a
1098 /// reply that misses its 200 ms stays buffered, so a shared socket would let
1099 /// a later question be answered by an earlier question's frame — one
1100 /// generated before the very event being waited on, which is how
1101 /// `sessions=1` after a rebirth passes without the rebirth.
1102 fn waitStats(alloc: std.mem.Allocator, sock_path: []const u8, want: []const u8) !void {
1103 const obs = try std.net.connectUnixSocket(sock_path);
1104 defer obs.close();
1098 var tries: usize = 0; 1105 var tries: usize = 0;
1099 while (tries < 40) : (tries += 1) { 1106 while (tries < 40) : (tries += 1) {
1100 try proto.writeFrame(fd, .stats_req, ""); 1107 try proto.writeFrame(obs.handle, .stats_req, "");
1101 const frame = (try awaitFrameThreaded(alloc, fd, .stats_reply, 200)) orelse continue; 1108 const frame = (try awaitFrameThreaded(alloc, obs.handle, .stats_reply, 200)) orelse continue;
1102 defer frame.deinit(alloc); 1109 defer frame.deinit(alloc);
1103 if (std.mem.indexOf(u8, frame.payload, want) != null) return; 1110 if (std.mem.indexOf(u8, frame.payload, want) != null) return;
1104 } 1111 }
@@ -1110,20 +1117,19 @@ fn waitStats(alloc: std.mem.Allocator, fd: std.posix.fd_t, want: []const u8) !vo
1110 /// its own thread, so a daemon that RETURNED from `run` answers none of it 1117 /// its own thread, so a daemon that RETURNED from `run` answers none of it
1111 /// and each one times out. 1118 /// and each one times out.
1112 fn probeEmptiedDaemon(alloc: std.mem.Allocator, sock_path: []const u8) !void { 1119 fn probeEmptiedDaemon(alloc: std.mem.Allocator, sock_path: []const u8) !void {
1113 const obs = try std.net.connectUnixSocket(sock_path);
1114 defer obs.close();
1115
1116 // Attach the default session, then hang up its shell — the daemon's 1120 // Attach the default session, then hang up its shell — the daemon's
1117 // only one, so the table empties behind it. 1121 // only one, so the table empties behind it. 7 rather than 0 so the
1122 // caller's exit-code pin can tell the two contracts apart: under the
1123 // retired one this shell's code WAS the daemon's.
1118 const c1 = try std.net.connectUnixSocket(sock_path); 1124 const c1 = try std.net.connectUnixSocket(sock_path);
1119 defer c1.close(); 1125 defer c1.close();
1120 try attachNamed(c1.handle, 80, 24, proto.default_session); 1126 try attachNamed(c1.handle, 80, 24, proto.default_session);
1121 const snap = (try awaitFrameThreaded(alloc, c1.handle, .snapshot, 4000)) orelse 1127 const snap = (try awaitFrameThreaded(alloc, c1.handle, .snapshot, 4000)) orelse
1122 return error.NoFirstSnapshot; 1128 return error.NoFirstSnapshot;
1123 snap.deinit(alloc); 1129 snap.deinit(alloc);
1124 try proto.writeFrame(c1.handle, .input, "die 0\n"); 1130 try proto.writeFrame(c1.handle, .input, "die 7\n");
1125 1131
1126 try waitStats(alloc, obs.handle, "sessions=0"); 1132 try waitStats(alloc, sock_path, "sessions=0");
1127 1133
1128 // Still serving with nothing to serve: a birth on the emptied daemon 1134 // Still serving with nothing to serve: a birth on the emptied daemon
1129 // takes the default name back, which is the attach `mux --sock PATH` 1135 // takes the default name back, which is the attach `mux --sock PATH`
@@ -1134,7 +1140,7 @@ fn probeEmptiedDaemon(alloc: std.mem.Allocator, sock_path: []const u8) !void {
1134 const reborn = (try awaitFrameThreaded(alloc, c2.handle, .snapshot, 4000)) orelse 1140 const reborn = (try awaitFrameThreaded(alloc, c2.handle, .snapshot, 4000)) orelse
1135 return error.EmptyDaemonRefusedABirth; 1141 return error.EmptyDaemonRefusedABirth;
1136 reborn.deinit(alloc); 1142 reborn.deinit(alloc);
1137 try waitStats(alloc, obs.handle, "sessions=1"); 1143 try waitStats(alloc, sock_path, "sessions=1");
1138 } 1144 }
1139 1145
1140 test "Server: a daemon outlives its last session and ends only on stop" { 1146 test "Server: a daemon outlives its last session and ends only on stop" {
@@ -1168,9 +1174,10 @@ test "Server: a daemon outlives its last session and ends only on stop" {
1168 th.join(); 1174 th.join();
1169 try probed; 1175 try probed;
1170 1176
1171 // Not the code of any shell: the one this daemon was born with was hung 1177 // 0 and not 7. The shell this daemon was born with exited 7, and under
1172 // up with `die 0` two stats replies ago, and the one born after it is 1178 // the retired contract that WAS the daemon's exit code — so this line
1173 // still alive. This is the shutdown's own code. 1179 // fails on a reap that still answers, not merely on a shutdown that
1180 // returns the wrong thing.
1174 try std.testing.expectEqual(@as(?u8, 0), code); 1181 try std.testing.expectEqual(@as(?u8, 0), code);
1175 } 1182 }
1176 1183
test/e2e_05_session.sh
Old New
@@ -173,10 +173,12 @@ wait_sessions "$SOCK21" 0 "M18: every one of the three shells has exited"
173 173
174 # The daemon is asked of the OS, not of itself: `stats` answering above 174 # The daemon is asked of the OS, not of itself: `stats` answering above
175 # already needed a live daemon, but only a pid can say the PROCESS is still 175 # already needed a live daemon, but only a pid can say the PROCESS is still
176 # here. The two seconds are not a wait for anything — the table emptied 176 # here. The two seconds are not a wait for anything and are not derived from
177 # before `stats` said so — they are the window the old exit-on-empty arm 177 # anything — the retired arm had no grace, it handed the code straight from
178 # used its grace on, so a daemon that still has one is caught here and not 178 # `reap` to `run` in the same pass. They are a settle window, chosen, so
179 # by the next scenario finding a dead socket. 179 # that a daemon leaving a BEAT after its table empties — through `deinit`,
180 # or through some later arm nobody has written yet — is caught here rather
181 # than by the next scenario finding a dead socket.
180 sleep 2 182 sleep 2
181 kill -0 "$D18PID" 2>/dev/null || { 183 kill -0 "$D18PID" 2>/dev/null || {
182 echo "e2e FAIL: M18: the daemon left with its last session — only muxd stop ends one" 184 echo "e2e FAIL: M18: the daemon left with its last session — only muxd stop ends one"
test/xversion.sh
Old New
@@ -84,8 +84,10 @@ done
84 TMP="${TMPDIR:-/tmp}/mux-xver-$$" 84 TMP="${TMPDIR:-/tmp}/mux-xver-$$"
85 # Every named attach records a DAEMON in $XDG_STATE_HOME/mux/hosts; without 85 # Every named attach records a DAEMON in $XDG_STATE_HOME/mux/hosts; without
86 # a hermetic home this suite's throwaway sockets pile up on the developer's 86 # a hermetic home this suite's throwaway sockets pile up on the developer's
87 # own wall, one dead stripe each. Legs that take a terminal narrow it 87 # own wall as host lines — tiles for every session while a container is
88 # further, one dir per leg — see `side_channel_session`. 88 # still up, and after that a line nothing times out and only `mux hosts rm`
89 # removes. Legs that take a terminal narrow it further, one dir per leg —
90 # see `side_channel_session`.
89 XDG_STATE_HOME="$TMP/state"; XDG_CONFIG_HOME="$TMP/cfg"; XDG_CACHE_HOME="$TMP/cache" 91 XDG_STATE_HOME="$TMP/state"; XDG_CONFIG_HOME="$TMP/cfg"; XDG_CACHE_HOME="$TMP/cache"
90 export XDG_STATE_HOME XDG_CONFIG_HOME XDG_CACHE_HOME 92 export XDG_STATE_HOME XDG_CONFIG_HOME XDG_CACHE_HOME
91 RUN_OLD="$TMP/o" 93 RUN_OLD="$TMP/o"