Skip to content

Commit d9b63b2

Browse files
committed
fix(cli): fail the default relay when another desktop's check fails
A paired desktop that was throttled, timed out, or whose owner couldn't be confirmed might accept a relay of its own, so one accepted relay elsewhere is not yet the only one. Signed-off-by: Will Pfleger <[email protected]>
1 parent d18f448 commit d9b63b2

2 files changed

Lines changed: 40 additions & 6 deletions

File tree

‎crates/buzz-cli/src/desktop_provider.rs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,10 +1133,10 @@ async fn accepted_relay(
11331133
}
11341134
}
11351135
match (accepted.len(), transient) {
1136-
(1, _) => Ok(accepted.pop()),
1137-
// Nothing usable, but a check that might pass on retry: report it
1138-
// rather than fall back to a relay this person is not paired for.
1139-
(0, Some(e)) => Err(e),
1136+
// A desktop whose check failed may accept a relay of its own: report
1137+
// it rather than pick, or fall back to, a relay it might contradict.
1138+
(0 | 1, Some(e)) => Err(e),
1139+
(1, None) => Ok(accepted.pop()),
11401140
_ => Ok(None),
11411141
}
11421142
}
@@ -2489,14 +2489,16 @@ pub(crate) mod tests {
24892489
.unwrap_err();
24902490
assert_eq!(crate::error::exit_code(&err), 2, "{err:?}");
24912491

2492-
// A usable relay wins over a transient failure elsewhere.
2492+
// A still-throttled desktop might add a second relay, so one
2493+
// accepted relay elsewhere does not settle the choice.
24932494
let (throttled, _) = provider(
24942495
|_, _| serde_json::json!({"result": "", "error": "rate_limited", "retry_after": 1}),
24952496
)
24962497
.await;
24972498
let store = MemoryStore::with(&[("a", "ta", &[RELAY]), ("b", "tb", &[RELAY])]);
24982499
let ads = vec![("a".into(), good), ("b".into(), throttled)];
2499-
assert!(accepted_relay(&store, ads).await.unwrap().is_some());
2500+
let err = accepted_relay(&store, ads).await.unwrap_err();
2501+
assert_eq!(crate::error::exit_code(&err), 2, "{err:?}");
25002502
}
25012503

25022504
/// With no usable relay, a retryable failure is reported over an

‎crates/buzz-cli/src/lib.rs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2748,6 +2748,38 @@ mod tests {
27482748
Cli::command().debug_assert();
27492749
}
27502750

2751+
/// Relay selection asks only what a command needs: an explicit key never
2752+
/// asks for a pairing, unpairing asks nothing, pairing asks for no other
2753+
/// pairing, and an agent's inherited relay comes first. `None` for
2754+
/// `inherited` means it must not be asked; the paired lookup never is.
2755+
#[cfg(unix)]
2756+
#[tokio::test]
2757+
async fn relay_selection_asks_only_what_a_command_needs() {
2758+
use clap::Parser;
2759+
const INHERITED: &str = "wss://inherited.example/";
2760+
const DEFAULT: &str = "http://localhost:3000";
2761+
let cases = [
2762+
("users get", true, Some(None), DEFAULT),
2763+
("auth desktop unpair", false, None, DEFAULT),
2764+
("auth desktop pair", false, Some(None), DEFAULT),
2765+
("users get", false, Some(Some(INHERITED)), INHERITED),
2766+
];
2767+
for (args, explicit_key, inherited, want) in cases {
2768+
let argv = std::iter::once("buzz").chain(args.split(' '));
2769+
let cli = Cli::try_parse_from(argv).unwrap();
2770+
let inherited = async move {
2771+
Ok(inherited
2772+
.unwrap_or_else(|| panic!("`{args}` asked for an inherited relay"))
2773+
.map(str::to_owned))
2774+
};
2775+
let paired = async { panic!("`{args}` asked for a paired relay") };
2776+
let relay = default_relay_with(&cli.command, explicit_key, inherited, paired)
2777+
.await
2778+
.unwrap();
2779+
assert_eq!(relay, want, "`{args}`");
2780+
}
2781+
}
2782+
27512783
#[test]
27522784
fn messages_send_from_thread_conflicts_with_reply_and_broadcast() {
27532785
let base = [

0 commit comments

Comments
 (0)