a73x

92bb8917

test: the stubborn shell's fixture owns its own readiness wait

a73x   2026-09-04 10:16

Commit message
test: the stubborn shell's fixture owns its own readiness wait

Five tests each spelling the wait was the defect, not the spelling. The
first attempt had none of them waiting; the second had two waiting on the
right session and three on the default while they ended a named one. A test
that names a session or a slot can name the wrong one, and twice did.

The harness has two doors now, one per way a stubborn shell comes into
existence. startStubborn writes the script, starts the daemon on it and
waits for the default session's shell to arm. attachStubborn births a named
session, waits for the snapshot that says the daemon seated it, then waits
for THAT session's own shell. Both are bounded and both fail with
StubbornShellNeverArmed. writeStubbornShell and awaitStubbornArmed are
private behind them, so no test can reach the pieces or pick a name: the
five call sites became one line each and none of them mentions a session.

The pid-named marker the script touches right after its trap line is
unchanged, and so is every assertion.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SakwJEwD9dXBoRP5kWbemW
src/server/server_test_attach.zig
Old New
@@ -1494,15 +1494,7 @@ test "Server: the two bounded deadlines read a monotonic clock, not the calendar
1494 1494
1495 var td = try h.TestDaemon.open(alloc, "mono"); 1495 var td = try h.TestDaemon.open(alloc, "mono");
1496 defer td.deinit(); 1496 defer td.deinit();
1497 const script = try h.writeStubbornShell(alloc, &td.tmp); 1497 try td.startStubborn(alloc, .{});
1498 defer alloc.free(script);
1499
1500 try td.start(.{ .shell = script });
1501 // The shell has to be STUBBORN before anything ends it: until its `trap`
1502 // line runs, the SIGTERM an accepted end sends is fatal, and a session
1503 // that died there is a session no assertion below is about. Named for the
1504 // session this test ENDS, which is the shell whose traps matter.
1505 try h.awaitStubbornArmed(&td.srv, &td.tmp, "", 5000);
1506 1498
1507 // CLOCK_MONOTONIC counts from boot and CLOCK_REALTIME from 1970, so any 1499 // CLOCK_MONOTONIC counts from boot and CLOCK_REALTIME from 1970, so any
1508 // machine that has been up less than a decade separates them by a 1500 // machine that has been up less than a decade separates them by a
src/server/server_test_harness.zig
Old New
@@ -163,6 +163,53 @@ pub const TestDaemon = struct {
163 return self; 163 return self;
164 } 164 }
165 165
166 /// `Opts` minus `shell`, which the stubborn doors supply: naming a shell
167 /// there is the one thing a stubborn-shell test must not do. One field
168 /// today rather than a copy of the whole list — a test that needs a grid
169 /// size or an env pair on a stubborn shell adds it here, in the one place
170 /// both doors read.
171 pub const StubbornOpts = struct { version: []const u8 = "" };
172
173 /// The DEFAULT session's door. Writes the stubborn shell, starts the
174 /// daemon on it, and does not return until that shell's traps are armed.
175 ///
176 /// The wait belongs to the door and not to the tests, because five of them
177 /// needed it and the two that spelled it by hand both got it wrong: first
178 /// by not waiting at all, then by waiting on the wrong session. A test
179 /// that comes through here cannot make either mistake, because it never
180 /// names a session or a slot.
181 pub fn startStubborn(self: *TestDaemon, alloc: std.mem.Allocator, opts: StubbornOpts) !void {
182 const script = try writeStubbornShell(alloc, &self.tmp);
183 // Freed here on purpose: `start` dupes it into `self.shell`, which is
184 // what the Server keeps.
185 defer alloc.free(script);
186 try self.start(.{ .shell = script, .version = opts.version });
187 try awaitStubbornArmed(&self.srv, &self.tmp, "", stubborn_arm_ms);
188 }
189
190 /// A NAMED session's door: birth it by attaching, wait for the snapshot
191 /// that says the daemon seated the client, then wait for THAT session's
192 /// own shell to arm. A different shell with a different pid from the
193 /// default session's, which is exactly what a hand-written wait got
194 /// wrong.
195 ///
196 /// Pumps, through `awaitFrame`, so it is for the tests that drive
197 /// `pumpOnce` themselves — which is every caller of the stubborn shell.
198 pub fn attachStubborn(
199 self: *TestDaemon,
200 alloc: std.mem.Allocator,
201 name: []const u8,
202 cols: u16,
203 rows: u16,
204 ) !std.net.Stream {
205 const c = try dial.dialAttachNamed(self.sock_path, cols, rows, name);
206 errdefer c.close();
207 (try awaitFrame(alloc, &self.srv, c.handle, .snapshot, 400) orelse
208 return error.NoState).deinit(alloc);
209 try awaitStubbornArmed(&self.srv, &self.tmp, name, stubborn_arm_ms);
210 return c;
211 }
212
166 /// Run the daemon's pump on a thread, for the tests that talk to it over a 213 /// Run the daemon's pump on a thread, for the tests that talk to it over a
167 /// real socket instead of driving `pumpOnce` themselves. 214 /// real socket instead of driving `pumpOnce` themselves.
168 pub fn threaded(self: *TestDaemon) !void { 215 pub fn threaded(self: *TestDaemon) !void {
@@ -579,7 +626,7 @@ pub fn writeDyingGapShell(alloc: std.mem.Allocator, tmp: *TmpDir) ![:0]u8 {
579 /// A shell that IGNORES both signals `Pty.requestExit` has and never reads 626 /// A shell that IGNORES both signals `Pty.requestExit` has and never reads
580 /// stdin again, so closing the master cannot end it either — SIGKILL is the 627 /// stdin again, so closing the master cannot end it either — SIGKILL is the
581 /// only thing left that can. What a bounded end has to survive. 628 /// only thing left that can. What a bounded end has to survive.
582 pub fn writeStubbornShell(alloc: std.mem.Allocator, tmp: *TmpDir) ![:0]u8 { 629 fn writeStubbornShell(alloc: std.mem.Allocator, tmp: *TmpDir) ![:0]u8 {
583 // Blocked in `open(2)` on a fifo nobody writes: no child to orphan, no spin. 630 // Blocked in `open(2)` on a fifo nobody writes: no child to orphan, no spin.
584 // A `sleep` loop leaves a `sleep` behind for up to its full second (measured: 631 // A `sleep` loop leaves a `sleep` behind for up to its full second (measured:
585 // 791 ms past the shell's SIGKILL), and looping on `read` off the closed 632 // 791 ms past the shell's SIGKILL), and looping on `read` off the closed
@@ -618,18 +665,18 @@ pub fn writeStubbornShell(alloc: std.mem.Allocator, tmp: *TmpDir) ![:0]u8 {
618 /// Wait until the stubborn shell behind the session `wire_name` spells has 665 /// Wait until the stubborn shell behind the session `wire_name` spells has
619 /// installed the traps that make it stubborn, or fail saying it never did. 666 /// installed the traps that make it stubborn, or fail saying it never did.
620 /// 667 ///
621 /// By NAME, and the name must be the one the caller is about to END. Three of 668 /// Private, and reached only through `TestDaemon.startStubborn` and
622 /// these tests end a session their own `dialAttachNamed` created, which is a 669 /// `TestDaemon.attachStubborn`, which is the whole point: each door knows
623 /// different shell with a different pid from the default session `Server.init` 670 /// which session it just created, so no test picks the name and none can pick
624 /// spawned; a wait that named the default proved nothing about the shell being 671 /// the wrong one. Both spellings of getting it wrong by hand have already
625 /// ended and left the race exactly where it was. Worse in the repeated-end 672 /// happened — no wait at all, then a wait on the default session while the
626 /// test, where a "nag" that died to the first pre-trap SIGTERM passes 673 /// test ended a named one. The second was the more dangerous, because a "nag"
627 /// `expect(!alive(pid))` without the SIGKILL deadline it is named for ever 674 /// that dies to the first pre-trap SIGTERM satisfies `expect(!alive(pid))`
628 /// being reached. `""` resolves to the default session, so one spelling 675 /// without the SIGKILL deadline that test is named for ever being reached.
629 /// covers every site. 676 /// `""` is the default session here, since `sessions.find` resolves it.
630 /// 677 ///
631 /// Every caller of `writeStubbornShell` needs this before it asks for an end, 678 /// The wait has to happen at all because `Pty.requestExit` closes the master
632 /// and none of them had it. `Pty.requestExit` closes the master (SIGHUP) and 679 /// (SIGHUP) and
633 /// sends SIGTERM; the script ignores both ONCE its `trap` line has run, and 680 /// sends SIGTERM; the script ignores both ONCE its `trap` line has run, and
634 /// dies to either before that. So a shell spawned and ended within the same 681 /// dies to either before that. So a shell spawned and ended within the same
635 /// millisecond was a coin toss, and the two systems do not toss the same 682 /// millisecond was a coin toss, and the two systems do not toss the same
@@ -646,7 +693,12 @@ pub fn writeStubbornShell(alloc: std.mem.Allocator, tmp: *TmpDir) ![:0]u8 {
646 /// 693 ///
647 /// It polls a file rather than the pump, because the shell writes that file 694 /// It polls a file rather than the pump, because the shell writes that file
648 /// on its own and a daemon on a thread would make pumping here a race. 695 /// on its own and a daemon on a thread would make pumping here a race.
649 pub fn awaitStubbornArmed(srv: *Server, tmp: *TmpDir, wire_name: []const u8, budget_ms: i64) !void { 696 /// Long enough that no loaded machine can trip it, short enough to be an
697 /// answer: the arming itself took 21 ms at worst across the measurements
698 /// above.
699 const stubborn_arm_ms: i64 = 5000;
700
701 fn awaitStubbornArmed(srv: *Server, tmp: *TmpDir, wire_name: []const u8, budget_ms: i64) !void {
650 const si = srv.sessions.find(wire_name) orelse return error.NoSuchSessionToArm; 702 const si = srv.sessions.find(wire_name) orelse return error.NoSuchSessionToArm;
651 const s = srv.sessions.table[si] orelse return error.NoSuchSessionToArm; 703 const s = srv.sessions.table[si] orelse return error.NoSuchSessionToArm;
652 var name_buf: [64]u8 = undefined; 704 var name_buf: [64]u8 = undefined;
src/server/server_test_session.zig
Old New
@@ -1630,19 +1630,10 @@ test "Server: a keystroke into an ending session does not cost that client its e
1630 1630
1631 var td = try h.TestDaemon.open(alloc, "endtype"); 1631 var td = try h.TestDaemon.open(alloc, "endtype");
1632 defer td.deinit(); 1632 defer td.deinit();
1633 const script = try h.writeStubbornShell(alloc, &td.tmp); 1633 try td.startStubborn(alloc, .{});
1634 defer alloc.free(script);
1635
1636 try td.start(.{ .shell = script });
1637 1634
1638 const c = try dial.dialAttachNamed(td.sock_path, 80, 24, "typing"); 1635 const c = try td.attachStubborn(alloc, "typing", 80, 24);
1639 defer c.close(); 1636 defer c.close();
1640 (try awaitFrame(alloc, &td.srv, c.handle, .snapshot, 400) orelse return error.NoState).deinit(alloc);
1641 // The shell has to be STUBBORN before anything ends it: until its `trap`
1642 // line runs, the SIGTERM an accepted end sends is fatal, and a session
1643 // that died there is a session no assertion below is about. Named for the
1644 // session this test ENDS, which is the shell whose traps matter.
1645 try h.awaitStubbornArmed(&td.srv, &td.tmp, "typing", 5000);
1646 1637
1647 var rq: [proto.end_req_max_len]u8 = undefined; 1638 var rq: [proto.end_req_max_len]u8 = undefined;
1648 try proto.writeFrame(c.handle, .end_req, proto.encodeEndReq(&rq, false, "typing")); 1639 try proto.writeFrame(c.handle, .end_req, proto.encodeEndReq(&rq, false, "typing"));
@@ -1663,19 +1654,10 @@ test "Server: a repeated end_req does not push the SIGKILL deadline out" {
1663 1654
1664 var td = try h.TestDaemon.open(alloc, "endagain"); 1655 var td = try h.TestDaemon.open(alloc, "endagain");
1665 defer td.deinit(); 1656 defer td.deinit();
1666 const script = try h.writeStubbornShell(alloc, &td.tmp); 1657 try td.startStubborn(alloc, .{});
1667 defer alloc.free(script);
1668
1669 try td.start(.{ .shell = script });
1670 1658
1671 const c = try dial.dialAttachNamed(td.sock_path, 80, 24, "nag"); 1659 const c = try td.attachStubborn(alloc, "nag", 80, 24);
1672 defer c.close(); 1660 defer c.close();
1673 (try awaitFrame(alloc, &td.srv, c.handle, .snapshot, 400) orelse return error.NoState).deinit(alloc);
1674 // The shell has to be STUBBORN before anything ends it: until its `trap`
1675 // line runs, the SIGTERM an accepted end sends is fatal, and a session
1676 // that died there is a session no assertion below is about. Named for the
1677 // session this test ENDS, which is the shell whose traps matter.
1678 try h.awaitStubbornArmed(&td.srv, &td.tmp, "nag", 5000);
1679 1661
1680 const pid = shellPidOf(&td.srv, "nag"); 1662 const pid = shellPidOf(&td.srv, "nag");
1681 var rq: [proto.end_req_max_len]u8 = undefined; 1663 var rq: [proto.end_req_max_len]u8 = undefined;
src/server/server_test_upgrade.zig
Old New
@@ -444,17 +444,9 @@ test "Server: an upgrade asked for during a session's hangup is refused, not att
444 444
445 var td = try h.TestDaemon.open(alloc, "upending"); 445 var td = try h.TestDaemon.open(alloc, "upending");
446 defer td.deinit(); 446 defer td.deinit();
447 const script = try h.writeStubbornShell(alloc, &td.tmp);
448 defer alloc.free(script);
449
450 // A real version, so every other check would PASS: without the ending 447 // A real version, so every other check would PASS: without the ending
451 // session this upgrade is one the daemon would go through with. 448 // session this upgrade is one the daemon would go through with.
452 try td.start(.{ .shell = script, .version = "0.0.1-1" }); 449 try td.startStubborn(alloc, .{ .version = "0.0.1-1" });
453 // The shell has to be STUBBORN before anything ends it: until its `trap`
454 // line runs, the SIGTERM an accepted end sends is fatal, and a session
455 // that died there is a session no assertion below is about. Named for the
456 // session this test ENDS, which is the shell whose traps matter.
457 try h.awaitStubbornArmed(&td.srv, &td.tmp, "", 5000);
458 450
459 // The shell outlives the hangup by the whole grace, so the exec's 451 // The shell outlives the hangup by the whole grace, so the exec's
460 // clearCloexec and the manifest's pty_fd would both see -1. 452 // clearCloexec and the manifest's pty_fd would both see -1.
@@ -603,22 +595,13 @@ test "Server: an accepted end cancels a pending upgrade — the exec never sees
603 595
604 var td = try h.TestDaemon.open(alloc, "upcancel"); 596 var td = try h.TestDaemon.open(alloc, "upcancel");
605 defer td.deinit(); 597 defer td.deinit();
606 const script = try h.writeStubbornShell(alloc, &td.tmp); 598 try td.startStubborn(alloc, .{ .version = "0.0.1-1" });
607 defer alloc.free(script);
608
609 try td.start(.{ .shell = script, .version = "0.0.1-1" });
610 599
611 // Plural: the end takes ONE session and the upgrade would have carried 600 // Plural: the end takes ONE session and the upgrade would have carried
612 // the others, so a guard that only looked at the ending slot's own 601 // the others, so a guard that only looked at the ending slot's own
613 // clients would still pass here. 602 // clients would still pass here.
614 const ca = try dial.dialAttachNamed(td.sock_path, 80, 24, "a"); 603 const ca = try td.attachStubborn(alloc, "a", 80, 24);
615 defer ca.close(); 604 defer ca.close();
616 (try awaitFrame(alloc, &td.srv, ca.handle, .snapshot, 400) orelse return error.NoState).deinit(alloc);
617 // The shell has to be STUBBORN before anything ends it: until its `trap`
618 // line runs, the SIGTERM an accepted end sends is fatal, and a session
619 // that died there is a session no assertion below is about. Named for the
620 // session this test ENDS, which is the shell whose traps matter.
621 try h.awaitStubbornArmed(&td.srv, &td.tmp, "a", 5000);
622 605
623 try td.tmp.dir.writeFile(.{ 606 try td.tmp.dir.writeFile(.{
624 .sub_path = "fakemux.sh", 607 .sub_path = "fakemux.sh",