a73x

b2bf74db

refactor: openHandoff performs the table, and decides nothing

a73x   2026-08-29 15:55

Commit message
refactor: openHandoff performs the table, and decides nothing

The loop is now: take a step, do it, say what came of it. Every ordering
comment left the function with the row it explains; what stays is the
effects — the argv choice at the spawn, the carry rule at the announce
read, the mutex at the cache write, the fallback line's text — each on the
thing it describes.

Behaviour is unchanged, which is the point: the six handoff e2e legs and
every openHandoff unit test are untouched and green.

The child, its errdefer kill, and the error `fail` returns are the driver's
because they are effects. `last_err` is optional so a `fail` the table
reached without a recorded error panics, rather than inventing one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017wi2HnuF1EK8HgViU11YLV

src/client/client.zig
Old New
@@ -470,104 +470,131 @@ pub const Transport = struct {
470 carry: ?*std.ArrayList(u8), 470 carry: ?*std.ArrayList(u8),
471 abort_fd: std.posix.fd_t, 471 abort_fd: std.posix.fd_t,
472 ) !Transport { 472 ) !Transport {
473 if (h.cache_path) |cp| { 473 // The ORDER is `handoff.next`'s; this loop performs the step it is
474 if (handoff.readCache(cp)) |ep| { 474 // handed and reports what came of it.
475 if (openQuicEndpoint(alloc, h, ep, carry, abort_fd)) |t| { 475 var st: handoff.State = .{
476 return t; 476 .cached = if (h.cache_path) |cp| (handoff.readCache(cp) catch null) else null,
477 } else |err| switch (err) { 477 .asked = h.asked,
478 // The user pressed Ctrl-\ while we were dialling. They 478 .has_cache = h.cache_path != null,
479 // asked to stop, not to try the next thing.
480 error.UserAbort => return err,
481 // Every other cached-path failure falls through to ssh,
482 // which is authoritative. What that costs depends on
483 // how the coordinates are dead: a resolve failure and
484 // a REFUSED port are both instant — the ICMP comes
485 // back and the quic client acts on it — while anything
486 // SILENT (blackholed UDP, a listener holding another
487 // key) spends the whole deadline first. See
488 // handoff.deadline_ms for both numbers.
489 else => {},
490 }
491 } else |_| {} // no cache yet, or one we cannot use: cold path
492 }
493
494 // ONE run, and `asked` is the whole of what picks it. The asking
495 // word ensures a daemon on the far side and announces on the same
496 // stdout, so there is no refusal for this side to read, no exit
497 // code to tell from ssh's own 255, and no second dial. A read
498 // spells the bare word, which starts nothing — the rule now holds
499 // by argv rather than by a branch here.
500 // `and len > 0` is what keeps `asked_argv`'s doc true: empty means
501 // "nothing to start", and an empty argv is not a no-op at the exec
502 // — the forked child null-unwraps argv[0] and dies, which arrives
503 // here as an announce that never came. Falling back to the reading
504 // word makes such a target a dial that starts nothing, which is
505 // what the field says it is.
506 const argv = if (h.asked and h.asked_argv.len > 0) h.asked_argv else h.ssh_argv;
507 var child = try spawnPipe(alloc, argv, h.quiet);
508 errdefer {
509 _ = child.kill() catch {};
510 }
511
512 // Bounded by nothing but ssh itself — the same trust `--via` already
513 // extends to the command it spawns.
514 //
515 // ssh owns the terminal while it runs: password and host-key
516 // prompts read /dev/tty, and a competing stdin reader here steals
517 // whole cooked lines from them — auth fails on an empty password,
518 // and the stolen line replays INTO THE SESSION via carry. On a
519 // first attach (carry != null) the tty is still cooked, so Ctrl-C's
520 // SIGINT already aborts this whole foreground group; the byte-read
521 // abort is only real on reconnect, where raw mode is live and
522 // carry is null by policy. Unconsumed type-ahead is not lost: it
523 // waits in the kernel's tty buffer for the session's first read.
524 const announce_abort_fd: std.posix.fd_t = if (carry != null) -1 else abort_fd;
525 const ep = (try readAnnounceAbortable(child.stdout.?.handle, alloc, null, announce_abort_fd)) orelse {
526 // `endpoint none`: the remote said, explicitly, that ssh is the
527 // session. Silent by design — no coordinates were ever in play,
528 // so there is nothing here to report as having failed.
529 return pipeTransport(child);
530 }; 479 };
531 // A cache write that fails costs a cold attach next time and nothing 480 // Optional because the warm path can finish without ever spawning
532 // else, so it is not worth a line of the user's attention. 481 // one. The errdefer owns it on EVERY error return, `fail` included,
533 // 482 // so no step spells a kill of its own.
534 // Serialized because openHandoff is no longer single-threaded: the 483 var child: ?std.process.Child = null;
535 // web hub runs one of these per tile, and two tiles naming the same 484 errdefer if (child) |*c| {
536 // host share a cache path. writeCache truncates in place — there is 485 _ = c.kill() catch {};
537 // no rename — so unserialized writers can tear the line. A torn one 486 };
538 // costs an attach, not a host: readCache refuses it and the cold 487 // What the terminal steps need from the steps before them; `dialed`
539 // path below rewrites it. The likelier miss is the loser writing the 488 // is the port the fallback line names.
540 // STALER endpoint, a cold attach that looks like a bug — and a mutex 489 var dialed: ?handoff.Endpoint = null;
541 // is cheaper than the afternoon. 490 var quic_t: ?Transport = null;
542 if (h.cache_path) |cp| { 491 // `fail` returns the error the failing step actually got, so the
543 cache_write_mu.lock(); 492 // caller's message names the real cause. Optional rather than a
544 defer cache_write_mu.unlock(); 493 // placeholder value: a `fail` that nothing recorded is a hole in the
545 handoff.writeCache(cp, ep) catch {}; 494 // table, and a panic says so where an invented error would not.
546 } 495 var last_err: ?anyerror = null;
547 496
548 if (openQuicEndpoint(alloc, h, ep, carry, abort_fd)) |t| { 497 var step = handoff.next(&st, null);
549 // QUIC carries the session now; the coordination ssh is done. 498 while (true) {
550 // kill() waitpid()s internally and closes the pipes with it. 499 const outcome: handoff.Outcome = switch (step) {
551 _ = child.kill() catch {}; 500 .dial_quic => |ep| blk: {
552 return t; 501 dialed = ep;
553 } else |err| switch (err) { 502 if (openQuicEndpoint(alloc, h, ep, carry, abort_fd)) |t| {
554 // The errdefer above owns the child on every error return, this 503 quic_t = t;
555 // one included; spelling the kill again here would be a second 504 break :blk .ok;
556 // place to keep in step with it. 505 } else |err| {
557 error.UserAbort => return err, 506 last_err = err;
558 else => { 507 break :blk if (err == error.UserAbort) .user_abort else .failed;
559 // The one line the fallback is allowed, and only on the 508 }
560 // attach that made the choice — see `asked`. It 509 },
561 // names what was tried and what is happening instead, and 510 .spawn_ssh => blk: {
562 // no cause: the client cannot tell a blocked port from a 511 // ONE run, and `asked` is the whole of what picks it. The
563 // wrong key (both are silence), and guessing would be worse 512 // asking word ensures a daemon on the far side and
564 // than the deadline it just spent. 513 // announces on the same stdout, so there is no refusal
565 if (h.asked) std.debug.print( 514 // for this side to read, no exit code to tell from ssh's
566 "mux: quic://{s}:{d} unreachable, attaching over ssh\n", 515 // own 255, and no second run. A read spells the bare
567 .{ handoff.dialHost(h.host), ep.port }, 516 // word, which starts nothing — the rule holds by argv
568 ); 517 // rather than by a branch.
569 return pipeTransport(child); 518 //
570 }, 519 // `and len > 0` is what keeps `asked_argv`'s doc true:
520 // empty means "nothing to start", and an empty argv is
521 // not a no-op at the exec — the forked child null-unwraps
522 // argv[0] and dies, which arrives here as an announce
523 // that never came. Falling back to the reading word makes
524 // such a target a dial that starts nothing, which is what
525 // the field says it is.
526 const argv = if (h.asked and h.asked_argv.len > 0) h.asked_argv else h.ssh_argv;
527 child = spawnPipe(alloc, argv, h.quiet) catch |err| {
528 last_err = err;
529 break :blk .failed;
530 };
531 break :blk .ok;
532 },
533 .read_announce => blk: {
534 // Bounded by nothing but ssh itself — the same trust
535 // `--via` already extends to the command it spawns.
536 //
537 // ssh owns the terminal while it runs: password and
538 // host-key prompts read /dev/tty, and a competing stdin
539 // reader here steals whole cooked lines from them — auth
540 // fails on an empty password, and the stolen line replays
541 // INTO THE SESSION via carry. On a first attach
542 // (carry != null) the tty is still cooked, so Ctrl-C's
543 // SIGINT already aborts this whole foreground group; the
544 // byte-read abort is only real on reconnect, where raw
545 // mode is live and carry is null by policy. Unconsumed
546 // type-ahead is not lost: it waits in the kernel's tty
547 // buffer for the session's first read.
548 const announce_abort_fd: std.posix.fd_t = if (carry != null) -1 else abort_fd;
549 const got = readAnnounceAbortable(child.?.stdout.?.handle, alloc, null, announce_abort_fd) catch |err| {
550 last_err = err;
551 break :blk .announce_failed;
552 };
553 break :blk if (got) |ep| .{ .announced = ep } else .none;
554 },
555 .write_cache => |ep| blk: {
556 // A cache write that fails costs a cold attach next time
557 // and nothing else, so it is not worth a line of the
558 // user's attention.
559 //
560 // Serialized because openHandoff is no longer
561 // single-threaded: the web hub runs one of these per
562 // tile, and two tiles naming the same host share a cache
563 // path. writeCache truncates in place — there is no
564 // rename — so unserialized writers can tear the line. A
565 // torn one costs an attach, not a host: readCache refuses
566 // it and the next cold path rewrites it. The likelier
567 // miss is the loser writing the STALER endpoint, a cold
568 // attach that looks like a bug — and a mutex is cheaper
569 // than the afternoon.
570 cache_write_mu.lock();
571 defer cache_write_mu.unlock();
572 handoff.writeCache(h.cache_path.?, ep) catch {};
573 break :blk .done;
574 },
575 .use_quic => {
576 // QUIC carries the session now, so the coordination ssh —
577 // if this handoff ran one at all — is done. kill()
578 // waitpid()s internally and closes the pipes with it.
579 if (child) |*c| {
580 _ = c.kill() catch {};
581 }
582 return quic_t.?;
583 },
584 .use_pipe => |say| {
585 // The line names what was tried and what is happening
586 // instead, and no cause: the client cannot tell a blocked
587 // port from a wrong key (both are silence), and guessing
588 // would be worse than the deadline it just spent.
589 if (say) std.debug.print(
590 "mux: quic://{s}:{d} unreachable, attaching over ssh\n",
591 .{ handoff.dialHost(h.host), dialed.?.port },
592 );
593 return pipeTransport(child.?);
594 },
595 .fail => return last_err.?,
596 };
597 step = handoff.next(&st, outcome);
571 } 598 }
572 } 599 }
573 600