From f811df08cec9e8aede4009a8a05eb674045c460d Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 19:33:31 +0800 Subject: [PATCH] test(remote): give each machine in the route-origin tests its own key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `a_routed_auth_prompt_carries_the_machine_that_raised_it` has been waved through as a known flake all day. It is not one. It fails because the test helper that builds its `NativeSshSpec` never spells the host. `SshProfile::new` takes a *name*; the address is a separate `host` field, and every production caller assigns both — `spec_for`, `open_quick_connect` and `parse_ssh_connect` all follow `SshProfile::new(host)` with `profile.host = host`. `native_spec` in this module's tests dropped that line, so the spec it returns addresses nobody and `ConnectionKey::from_spec` spells it `me@:22` — the same string for every machine in the module that talks to port 22. `ORIGINS` is keyed by exactly that string. So `a_routed_auth_prompt_carries_the_machine_that_raised_it` (build-box) and `a_pane_and_its_workspace_resolve_to_the_same_machine` (twin-box) were noting their origin in one shared slot and then reading it back. libtest runs them on different threads; whichever noted last won, and the other was handed the other machine's `HostId`. The routed-auth test usually lost, because it spawns a responder thread and waits on a mailbox before it reads, while the pane test reads immediately. The proof is the origin table at the moment of failure, dumped from a temporary instrumented build: assertion `left == right` failed: origins=[("me@:2222", 12106367783589518659), ("me@:22", 6831549409032087863), ("local-stdio:/opt/tty7-server", 11414125097972604568)] left: HostId(6831549409032087863) # ssh-direct:me@twin-box:22 right: HostId(7267374002226354343) # ssh-direct:me@build-box:22 An empty host in every key, and the routed-auth test reading twin-box's id out of build-box's slot. Measured on 893172f5, Windows 11, one binary per configuration: | configuration | before | after | | --------------------------------- | ------ | ----- | | full suite (`tty7-app`) | 6/20 | 0/40 | | this module only (`a_` filter) | 17/20 | 0/30 | | the test alone (`--exact`) | 0/20 | 0/20 | The isolated column is the tell: nothing outside this module was ever involved, and no amount of rerunning the test by itself could have shown the defect. No product path is affected. Only the test helper forgot the field; the three callers that build a spec for real assign it, and a route in the app has always addressed the machine it names. `two_machines_do_not_share_one_route_origin_key` is the guard: it asserts the key spells the host and that two machines do not collide. Reverting the one-line fix fails it in 0.00s with `left: "me@:22"`. --- src/ui/remote_connect.rs | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/ui/remote_connect.rs b/src/ui/remote_connect.rs index b04ea116..42d101b4 100644 --- a/src/ui/remote_connect.rs +++ b/src/ui/remote_connect.rs @@ -960,6 +960,7 @@ mod tests { fn native_spec(user: &str, host: &str, port: u16) -> NativeSshSpec { let mut profile = crate::core::ssh_profile::SshProfile::new(host.to_string()); + profile.host = host.to_string(); profile.user = user.to_string(); profile.port = port; crate::ui::ssh_connect::build_native_ssh_spec( @@ -970,6 +971,36 @@ mod tests { ) } + /// `SshProfile::new` takes a *name*, and the address lives in a separate + /// `host` field; every production caller assigns both. `native_spec` used + /// to assign only the name, so the spec it handed back addressed nobody + /// and `ConnectionKey::from_spec` spelled it `me@:22` — one key for every + /// machine in this module that talks to port 22. + /// + /// `ORIGINS` is keyed by exactly that string, so the two tests below that + /// note an origin and read it back were writing to and reading from the + /// same slot. Whichever noted last won, and the other was handed the wrong + /// machine: 17 failures in 20 runs of this module alone, 6 in 20 of the + /// whole binary, and none when either test ran by itself. + #[test] + fn two_machines_do_not_share_one_route_origin_key() { + use crate::daemon::router::RouteTarget; + + let build = RouteTarget::Ssh(Box::new(native_spec("me", "build-box", 22))); + let twin = RouteTarget::Ssh(Box::new(native_spec("me", "twin-box", 22))); + + assert_eq!( + build.origin_key(), + "me@build-box:22", + "a route origin key names the machine it dials" + ); + assert_ne!( + build.origin_key(), + twin.origin_key(), + "two machines sharing one key make `note_origin` overwrite the other's" + ); + } + #[test] fn a_routed_auth_prompt_carries_the_machine_that_raised_it() { let _turn = claim_mailbox(); @@ -978,6 +1009,7 @@ mod tests { let route = crate::daemon::router::RouteTarget::Ssh(Box::new(native_spec("me", "build-box", 22))); note_origin(&route, &target); + let origin_key = route.origin_key(); let handle = std::thread::spawn(move || { use crate::daemon::router::RouteAuthResponder as _; @@ -1005,7 +1037,12 @@ mod tests { ); std::thread::sleep(Duration::from_millis(5)); }; - assert_eq!(pending.host, target.host_id()); + assert_eq!( + pending.host, + target.host_id(), + "the prompt names the machine noted under {:?}", + origin_key + ); pending.answer(AuthResponse::Secret("hunter2".into())); assert_eq!( handle.join().unwrap(),