a73x

df389073

refactor: small-items batch — detach_key, one SIGPIPE owner, dead Pty.read, ensurePtyModeSent, deps caching, writeFrame golden pin

a73x   2026-08-12 14:20

Commit message
refactor: small-items batch — detach_key, one SIGPIPE owner, dead Pty.read, ensurePtyModeSent, deps caching, writeFrame golden pin

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
build.zig
Old New
@@ -23,6 +23,16 @@ fn quicDeps(b: *std.Build, target: std.Build.ResolvedTarget) struct {
23 run.setName(b.fmt("build QUIC deps ({s})", .{name})); 23 run.setName(b.fmt("build QUIC deps ({s})", .{name}));
24 // Never cached by the build graph: the script's own marker file is the 24 // Never cached by the build graph: the script's own marker file is the
25 // cache, and it is the only thing that knows whether the libs are there. 25 // cache, and it is the only thing that knows whether the libs are there.
26 //
27 // Not an oversight, and the alternative was measured. Step.Run only
28 // treats a command as cacheable once it has an output arg, and
29 // addOutputFileArg insists on choosing the path — while this script's
30 // fixed `deps/quic/out/<target>` is a contract shared with `make deps`,
31 // `make clean-deps`, wan.sh's musl cross-build and linkQuic below. Under
32 // a build-chosen path the marker would live in a hash-named directory,
33 // so any argv change would re-download ~30MB rather than skip. The cost
34 // of leaving it uncached is one fork+exec that stats the marker and
35 // exits: ~1ms per build.
26 run.has_side_effects = true; 36 run.has_side_effects = true;
27 return .{ 37 return .{
28 .step = &run.step, 38 .step = &run.step,
@@ -216,6 +226,12 @@ pub fn build(b: *std.Build) void {
216 }); 226 });
217 proxy_mod.addImport("testtmp", testtmp_mod); 227 proxy_mod.addImport("testtmp", testtmp_mod);
218 228
229 // The client borrows ignoreSigpipe, which proxy owns. Declared here
230 // rather than beside the client's other imports because proxy_mod does
231 // not exist yet up there; proxy is a leaf, so this adds no cycle and
232 // teaches the proxy nothing.
233 client_mod.addImport("proxy", proxy_mod);
234
219 // Test helpers, built as real binaries because that is how the suite 235 // Test helpers, built as real binaries because that is how the suite
220 // uses them: rawmode is a deterministic stand-in for an editor (nvim's 236 // uses them: rawmode is a deterministic stand-in for an editor (nvim's
221 // redraw timing is its own business and it is not installed everywhere), 237 // redraw timing is its own business and it is not installed everywhere),
src/client.zig
Old New
@@ -12,6 +12,11 @@ const TmpDir = @import("testtmp").TmpDir;
12 const quic_client = @import("quic_client"); 12 const quic_client = @import("quic_client");
13 const predict = @import("predict"); 13 const predict = @import("predict");
14 const handoff = @import("handoff"); 14 const handoff = @import("handoff");
15 // For ignoreSigpipe only, which proxy.zig owns; see its docstring for why
16 // the client is one of its callers and why the call site's order matters.
17 const proxy = @import("proxy");
18
19 const detach_key: u8 = 0x1c; // Ctrl-\, the detach chord (see module doc).
15 20
16 var winch_flag = std.atomic.Value(bool).init(false); 21 var winch_flag = std.atomic.Value(bool).init(false);
17 22
@@ -306,13 +311,6 @@ const Transport = struct {
306 }; 311 };
307 } 312 }
308 313
309 /// Idempotent, and it has to be: reconnect() releases the dead transport
310 /// on entry, and if the user then aborts, attach()'s `defer
311 /// transport.close()` closes the very same value again. A second
312 /// close(2) on a stale fd is EBADF, which std.posix maps to
313 /// `unreachable` — a panic, not an error. `--via` hides that (killing a
314 /// reaped child is harmless), `--sock` does not, and `--sock` is exactly
315 /// the locally-killed-daemon case.
316 /// The descriptor to wait on for readability. 314 /// The descriptor to wait on for readability.
317 /// 315 ///
318 /// For a socket and for `--via` this is also where frames are read 316 /// For a socket and for `--via` this is also where frames are read
@@ -407,6 +405,13 @@ const Transport = struct {
407 return .{ .frame = frame }; 405 return .{ .frame = frame };
408 } 406 }
409 407
408 /// Idempotent, and it has to be: reconnect() releases the dead transport
409 /// on entry, and if the user then aborts, attach()'s `defer
410 /// transport.close()` closes the very same value again. A second
411 /// close(2) on a stale fd is EBADF, which std.posix maps to
412 /// `unreachable` — a panic, not an error. `--via` hides that (killing a
413 /// reaped child is harmless), `--sock` does not, and `--sock` is exactly
414 /// the locally-killed-daemon case.
410 fn close(self: *Transport) void { 415 fn close(self: *Transport) void {
411 if (self.conn.r == -1) return; // already released 416 if (self.conn.r == -1) return; // already released
412 defer self.conn = .{ .r = -1, .w = -1 }; 417 defer self.conn = .{ .r = -1, .w = -1 };
@@ -483,7 +488,7 @@ fn waitReady(
483 const n = std.posix.read(std.posix.STDIN_FILENO, &buf) catch 0; 488 const n = std.posix.read(std.posix.STDIN_FILENO, &buf) catch 0;
484 if (n == 0) watch_stdin = false; 489 if (n == 0) watch_stdin = false;
485 if (n > 0) { 490 if (n > 0) {
486 if (std.mem.indexOfScalar(u8, buf[0..n], 0x1c) != null) return error.UserAbort; 491 if (std.mem.indexOfScalar(u8, buf[0..n], detach_key) != null) return error.UserAbort;
487 // Not the abort key. Whether these bytes are kept or dropped 492 // Not the abort key. Whether these bytes are kept or dropped
488 // is the caller's policy, not this function's: on a first 493 // is the caller's policy, not this function's: on a first
489 // attach they are the user's first keystrokes and are owed to 494 // attach they are the user's first keystrokes and are owed to
@@ -565,7 +570,7 @@ fn readAnnounceAbortable(
565 const got = std.posix.read(std.posix.STDIN_FILENO, &in) catch 0; 570 const got = std.posix.read(std.posix.STDIN_FILENO, &in) catch 0;
566 if (got == 0) watch_stdin = false; 571 if (got == 0) watch_stdin = false;
567 if (got > 0) { 572 if (got > 0) {
568 if (std.mem.indexOfScalar(u8, in[0..got], 0x1c) != null) 573 if (std.mem.indexOfScalar(u8, in[0..got], detach_key) != null)
569 return error.UserAbort; 574 return error.UserAbort;
570 // waitReady's contract, and for waitReady's reason: on a 575 // waitReady's contract, and for waitReady's reason: on a
571 // first attach these are the user's first keystrokes and are 576 // first attach these are the user's first keystrokes and are
@@ -776,18 +781,11 @@ fn session(
776 carry: *std.ArrayList(u8), 781 carry: *std.ArrayList(u8),
777 ) !u8 { 782 ) !u8 {
778 // A daemon that dies mid-write must surface as an error return from 783 // A daemon that dies mid-write must surface as an error return from
779 // write(), not a fatal SIGPIPE. Zig's start.zig already installs a noop 784 // write(), not a fatal SIGPIPE. SIG_IGN survives exec while a handler
780 // SIGPIPE handler, so this is defence in depth rather than the thing that 785 // does not, so this must stay *after* the `--via` child is spawned (it
781 // makes EPIPE reachable. The one difference that matters: SIG_IGN survives 786 // is — attach() spawns, then calls us), or ssh and the remote proxy
782 // exec while a handler does not, so this must stay *after* the `--via` 787 // would inherit an ignored SIGPIPE they never asked for.
783 // child is spawned (it is — attach() spawns, then calls us), or ssh and 788 proxy.ignoreSigpipe();
784 // the remote proxy would inherit an ignored SIGPIPE they never asked for.
785 var ign: std.posix.Sigaction = .{
786 .handler = .{ .handler = std.posix.SIG.IGN },
787 .mask = std.posix.sigemptyset(),
788 .flags = 0,
789 };
790 std.posix.sigaction(std.posix.SIG.PIPE, &ign, null);
791 789
792 const stdin_fd = std.posix.STDIN_FILENO; 790 const stdin_fd = std.posix.STDIN_FILENO;
793 const stdout_fd = std.posix.STDOUT_FILENO; 791 const stdout_fd = std.posix.STDOUT_FILENO;
@@ -1190,7 +1188,7 @@ fn session(
1190 if (n == 0) { 1188 if (n == 0) {
1191 stdin_open = false; 1189 stdin_open = false;
1192 } else { 1190 } else {
1193 if (std.mem.indexOfScalar(u8, buf[0..n], 0x1c) != null) { 1191 if (std.mem.indexOfScalar(u8, buf[0..n], detach_key) != null) {
1194 // Ctrl-\: detach and leave the session running. 1192 // Ctrl-\: detach and leave the session running.
1195 transport.writeFrame(.detach, "") catch {}; 1193 transport.writeFrame(.detach, "") catch {};
1196 exit_msg = "mux: detached (session still running; run mux to reattach)"; 1194 exit_msg = "mux: detached (session still running; run mux to reattach)";
@@ -1526,7 +1524,7 @@ fn drainStdinForQuit(stdin_fd: std.posix.fd_t, timeout_ms: u64) bool {
1526 std.Thread.sleep((timeout_ms - elapsed) * std.time.ns_per_ms); 1524 std.Thread.sleep((timeout_ms - elapsed) * std.time.ns_per_ms);
1527 return false; 1525 return false;
1528 } 1526 }
1529 if (std.mem.indexOfScalar(u8, buf[0..n], 0x1c) != null) return true; 1527 if (std.mem.indexOfScalar(u8, buf[0..n], detach_key) != null) return true;
1530 // Anything else is dropped, and we keep waiting out the backoff — 1528 // Anything else is dropped, and we keep waiting out the backoff —
1531 // returning early here would collapse the pacing the moment the 1529 // returning early here would collapse the pacing the moment the
1532 // user touched a key. 1530 // user touched a key.
src/protocol.zig
Old New
@@ -360,10 +360,24 @@ pub fn composeDelta(alloc: std.mem.Allocator, payload: []const u8) !ComposedDelt
360 360
361 test "appendFrame encodes the same bytes writeFrame sends" { 361 test "appendFrame encodes the same bytes writeFrame sends" {
362 const alloc = std.testing.allocator; 362 const alloc = std.testing.allocator;
363 const golden = [_]u8{ 0x02, 3, 0, 0, 0, 'a', 'b', 'c' };
364
363 var list: std.ArrayList(u8) = .empty; 365 var list: std.ArrayList(u8) = .empty;
364 defer list.deinit(alloc); 366 defer list.deinit(alloc);
365 try appendFrame(&list, alloc, .input, "abc"); 367 try appendFrame(&list, alloc, .input, "abc");
366 try std.testing.expectEqualSlices(u8, &[_]u8{ 0x02, 3, 0, 0, 0, 'a', 'b', 'c' }, list.items); 368 try std.testing.expectEqualSlices(u8, &golden, list.items);
369
370 // writeFrame is driven for real rather than round-tripped: a round trip
371 // through readFrame passes even when writer and reader drift together,
372 // which is exactly the drift between the two encoders this test names.
373 const p = try std.posix.pipe();
374 defer std.posix.close(p[0]);
375 try writeFrame(p[1], .input, "abc");
376 std.posix.close(p[1]);
377 var sent: [golden.len]u8 = undefined;
378 try readExact(p[0], &sent);
379 try std.testing.expectEqualSlices(u8, &golden, &sent);
380 try std.testing.expectEqualSlices(u8, list.items, &sent);
367 } 381 }
368 382
369 test "appendFrame concatenates frames the way a queue would" { 383 test "appendFrame concatenates frames the way a queue would" {
src/proxy.zig
Old New
@@ -15,10 +15,12 @@ const TmpDir = @import("testtmp").TmpDir;
15 /// of to a std default (`std.options.keep_sigpipe`) another module could 15 /// of to a std default (`std.options.keep_sigpipe`) another module could
16 /// flip. 16 /// flip.
17 /// 17 ///
18 /// Exported because `muxd endpoint` writes its announce to the same stdout 18 /// Exported because the other two callers need the identical install:
19 /// this pump is about to use, before calling `run` — one installer rather 19 /// `muxd endpoint` writes its announce to the same stdout this pump is about
20 /// than a copy, so the two cannot drift. No protocol knowledge crosses the 20 /// to use, before calling `run`, and the client's session loop needs a dying
21 /// boundary, which is the only thing this file's import list forbids. 21 /// daemon to surface as EPIPE. One installer rather than three copies, so
22 /// they cannot drift. No protocol knowledge crosses the boundary, which is
23 /// the only thing this file's import list forbids.
22 /// 24 ///
23 /// The one real difference from the std default: SIG_IGN survives exec, a 25 /// The one real difference from the std default: SIG_IGN survives exec, a
24 /// handler does not. So a caller that SPAWNS must install this after the 26 /// handler does not. So a caller that SPAWNS must install this after the
src/pty.zig
Old New
@@ -105,10 +105,6 @@ pub const Pty = struct {
105 return .{ .master = master, .child = pid }; 105 return .{ .master = master, .child = pid };
106 } 106 }
107 107
108 pub fn read(self: *Pty, buf: []u8) !usize {
109 return std.posix.read(self.master, buf);
110 }
111
112 pub fn write(self: *Pty, data: []const u8) !usize { 108 pub fn write(self: *Pty, data: []const u8) !usize {
113 return std.posix.write(self.master, data); 109 return std.posix.write(self.master, data);
114 } 110 }
src/server.zig
Old New
@@ -1108,7 +1108,7 @@ pub const Server = struct {
1108 /// fires on a change — so without this, a client joining a session that 1108 /// fires on a change — so without this, a client joining a session that
1109 /// is sitting quietly at a prompt would wait for the mode to move before 1109 /// is sitting quietly at a prompt would wait for the mode to move before
1110 /// it learned anything about it, which is to say forever. 1110 /// it learned anything about it, which is to say forever.
1111 fn sendPtyModeTo(self: *Server, i: usize) void { 1111 fn ensurePtyModeSent(self: *Server, i: usize) void {
1112 const flags = self.mode_sent orelse blk: { 1112 const flags = self.mode_sent orelse blk: {
1113 // Reached whenever an attach lands before the mode has ever 1113 // Reached whenever an attach lands before the mode has ever
1114 // been polled — which is not only the pre-first-pump case a 1114 // been polled — which is not only the pre-first-pump case a
@@ -1257,7 +1257,7 @@ pub const Server = struct {
1257 if (applied) self.recordSize(i); 1257 if (applied) self.recordSize(i);
1258 // Ahead of the state it describes, so a client can never be 1258 // Ahead of the state it describes, so a client can never be
1259 // holding grid content it has no mode for. 1259 // holding grid content it has no mode for.
1260 self.sendPtyModeTo(i); 1260 self.ensurePtyModeSent(i);
1261 self.sendResync(i, req.have_seq, req.have_epoch, size_changed and applied); 1261 self.sendResync(i, req.have_seq, req.have_epoch, size_changed and applied);
1262 }, 1262 },
1263 .resize => { 1263 .resize => {
@@ -1376,7 +1376,7 @@ pub const Server = struct {
1376 // .resize/.attach arms — but only when the grid really went 1376 // .resize/.attach arms — but only when the grid really went
1377 // there. A refused attach must leave the slot at 0x0. 1377 // there. A refused attach must leave the slot at 0x0.
1378 if (applied) self.recordSize(slot); 1378 if (applied) self.recordSize(slot);
1379 self.sendPtyModeTo(slot); 1379 self.ensurePtyModeSent(slot);
1380 // Latest wins: a size change broadcasts, repainting every 1380 // Latest wins: a size change broadcasts, repainting every
1381 // client at the new attacher's size. A same-size join is 1381 // client at the new attacher's size. A same-size join is
1382 // the joiner's business alone — see sendResync. So is a 1382 // the joiner's business alone — see sendResync. So is a