a5ed558d
refactor: quality fixes for the small-items batch — constraint-stating docs, sendPtyModeTo restored
a73x 2026-08-12 14:46
Commit message
build.zig
| Old | New | ||
|---|---|---|---|
| @@ -31,7 +31,7 @@ fn quicDeps(b: *std.Build, target: std.Build.ResolvedTarget) struct { | |||
| 31 | // `make clean-deps`, wan.sh's musl cross-build and linkQuic below. Under | 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, | 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 | 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 | 34 | // of leaving it uncached is a fork+exec that stats the marker and |
| 35 | // exits: ~1ms per build. | 35 | // exits: ~1ms per build. |
| 36 | run.has_side_effects = true; | 36 | run.has_side_effects = true; |
| 37 | return .{ | 37 | return .{ |
| @@ -187,6 +187,18 @@ pub fn build(b: *std.Build) void { | |||
| 187 | }); | 187 | }); |
| 188 | quic_client_mod.addImport("quic", quic_mod); | 188 | quic_client_mod.addImport("quic", quic_mod); |
| 189 | 189 | ||
| 190 | // No imports that teach it anything, deliberately: the proxy is a byte | ||
| 191 | // pump that knows nothing about the protocol it carries. `testtmp` is | ||
| 192 | // the one exception and does not weaken that — it hands its tests a | ||
| 193 | // short directory to put a socket in and knows nothing about the bytes. | ||
| 194 | const proxy_mod = b.createModule(.{ | ||
| 195 | .root_source_file = b.path("src/proxy.zig"), | ||
| 196 | .target = target, | ||
| 197 | .optimize = optimize, | ||
| 198 | .link_libc = true, | ||
| 199 | }); | ||
| 200 | proxy_mod.addImport("testtmp", testtmp_mod); | ||
| 201 | |||
| 190 | const client_mod = b.createModule(.{ | 202 | const client_mod = b.createModule(.{ |
| 191 | .root_source_file = b.path("src/client.zig"), | 203 | .root_source_file = b.path("src/client.zig"), |
| 192 | .target = target, | 204 | .target = target, |
| @@ -201,6 +213,9 @@ pub fn build(b: *std.Build) void { | |||
| 201 | // display decision and never becomes state anybody else can see. | 213 | // display decision and never becomes state anybody else can see. |
| 202 | client_mod.addImport("predict", predict_mod); | 214 | client_mod.addImport("predict", predict_mod); |
| 203 | client_mod.addImport("handoff", handoff_mod); | 215 | client_mod.addImport("handoff", handoff_mod); |
| 216 | // The client borrows ignoreSigpipe, which proxy owns. proxy is a leaf, | ||
| 217 | // so this adds no cycle and teaches the proxy nothing. | ||
| 218 | client_mod.addImport("proxy", proxy_mod); | ||
| 204 | 219 | ||
| 205 | const mux_mod = b.createModule(.{ | 220 | const mux_mod = b.createModule(.{ |
| 206 | .root_source_file = b.path("src/mux_main.zig"), | 221 | .root_source_file = b.path("src/mux_main.zig"), |
| @@ -214,24 +229,6 @@ pub fn build(b: *std.Build) void { | |||
| 214 | mux_mod.addImport("spawn", spawn_mod); | 229 | mux_mod.addImport("spawn", spawn_mod); |
| 215 | mux_mod.addImport("handoff", handoff_mod); | 230 | mux_mod.addImport("handoff", handoff_mod); |
| 216 | 231 | ||
| 217 | // No imports that teach it anything, deliberately: the proxy is a byte | ||
| 218 | // pump that knows nothing about the protocol it carries. `testtmp` is | ||
| 219 | // the one exception and does not weaken that — it hands its tests a | ||
| 220 | // short directory to put a socket in and knows nothing about the bytes. | ||
| 221 | const proxy_mod = b.createModule(.{ | ||
| 222 | .root_source_file = b.path("src/proxy.zig"), | ||
| 223 | .target = target, | ||
| 224 | .optimize = optimize, | ||
| 225 | .link_libc = true, | ||
| 226 | }); | ||
| 227 | proxy_mod.addImport("testtmp", testtmp_mod); | ||
| 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 | |||
| 235 | // Test helpers, built as real binaries because that is how the suite | 232 | // Test helpers, built as real binaries because that is how the suite |
| 236 | // uses them: rawmode is a deterministic stand-in for an editor (nvim's | 233 | // uses them: rawmode is a deterministic stand-in for an editor (nvim's |
| 237 | // redraw timing is its own business and it is not installed everywhere), | 234 | // redraw timing is its own business and it is not installed everywhere), |
src/client.zig
| Old | New | ||
|---|---|---|---|
| @@ -12,11 +12,10 @@ 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 | 15 | // For ignoreSigpipe only, which proxy.zig owns. |
| 16 | // the client is one of its callers and why the call site's order matters. | ||
| 17 | const proxy = @import("proxy"); | 16 | const proxy = @import("proxy"); |
| 18 | 17 | ||
| 19 | const detach_key: u8 = 0x1c; // Ctrl-\, the detach chord (see module doc). | 18 | const detach_key: u8 = 0x1c; // Ctrl-\, the detach chord. |
| 20 | 19 | ||
| 21 | var winch_flag = std.atomic.Value(bool).init(false); | 20 | var winch_flag = std.atomic.Value(bool).init(false); |
| 22 | 21 | ||
src/protocol.zig
| Old | New | ||
|---|---|---|---|
| @@ -368,8 +368,9 @@ test "appendFrame encodes the same bytes writeFrame sends" { | |||
| 368 | try std.testing.expectEqualSlices(u8, &golden, list.items); | 368 | try std.testing.expectEqualSlices(u8, &golden, list.items); |
| 369 | 369 | ||
| 370 | // writeFrame is driven for real rather than round-tripped: a round trip | 370 | // writeFrame is driven for real rather than round-tripped: a round trip |
| 371 | // through readFrame passes even when writer and reader drift together, | 371 | // through readFrame passes without pinning a single byte, so it can say |
| 372 | // which is exactly the drift between the two encoders this test names. | 372 | // nothing about whether writeFrame and appendFrame agree — which is the |
| 373 | // drift this test names. | ||
| 373 | const p = try std.posix.pipe(); | 374 | const p = try std.posix.pipe(); |
| 374 | defer std.posix.close(p[0]); | 375 | defer std.posix.close(p[0]); |
| 375 | try writeFrame(p[1], .input, "abc"); | 376 | try writeFrame(p[1], .input, "abc"); |
| @@ -378,6 +379,12 @@ test "appendFrame encodes the same bytes writeFrame sends" { | |||
| 378 | try readExact(p[0], &sent); | 379 | try readExact(p[0], &sent); |
| 379 | try std.testing.expectEqualSlices(u8, &golden, &sent); | 380 | try std.testing.expectEqualSlices(u8, &golden, &sent); |
| 380 | try std.testing.expectEqualSlices(u8, list.items, &sent); | 381 | try std.testing.expectEqualSlices(u8, list.items, &sent); |
| 382 | |||
| 383 | // The comparisons above read a fixed count, so a writeFrame that emitted | ||
| 384 | // a ninth byte would still pass them. The write end is already closed, | ||
| 385 | // so anything left unread surfaces here as a successful read instead. | ||
| 386 | var extra: [1]u8 = undefined; | ||
| 387 | try std.testing.expectError(error.UnexpectedEof, readExact(p[0], &extra)); | ||
| 381 | } | 388 | } |
| 382 | 389 | ||
| 383 | test "appendFrame concatenates frames the way a queue would" { | 390 | test "appendFrame concatenates frames the way a queue would" { |
src/proxy.zig
| Old | New | ||
|---|---|---|---|
| @@ -15,12 +15,10 @@ 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 the other two callers need the identical install: | 18 | /// Exported so that every process which writes to a pipe it does not own |
| 19 | /// `muxd endpoint` writes its announce to the same stdout this pump is about | 19 | /// installs the identical ignore rather than its own copy. No protocol |
| 20 | /// to use, before calling `run`, and the client's session loop needs a dying | 20 | /// knowledge crosses the boundary, which is the only thing this file's |
| 21 | /// daemon to surface as EPIPE. One installer rather than three copies, so | 21 | /// import list forbids. |
| 22 | /// they cannot drift. No protocol knowledge crosses the boundary, which is | ||
| 23 | /// the only thing this file's import list forbids. | ||
| 24 | /// | 22 | /// |
| 25 | /// The one real difference from the std default: SIG_IGN survives exec, a | 23 | /// The one real difference from the std default: SIG_IGN survives exec, a |
| 26 | /// handler does not. So a caller that SPAWNS must install this after the | 24 | /// handler does not. So a caller that SPAWNS must install this after the |
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 ensurePtyModeSent(self: *Server, i: usize) void { | 1111 | fn sendPtyModeTo(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.ensurePtyModeSent(i); | 1260 | self.sendPtyModeTo(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.ensurePtyModeSent(slot); | 1379 | self.sendPtyModeTo(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 |