From db9015b26c875db7eef822757b1e95c0972d9204 Mon Sep 17 00:00:00 2001 From: AB Date: Tue, 22 Sep 2026 19:00:08 +0300 Subject: [PATCH] Allow network commands to select stored names Leaving a configured network required copying its opaque id even when its name identified exactly one saved network. Resolve exact, case-sensitive names from the existing local state alongside full ids and unique id prefixes. Use the shared resolver for leave, stop, start, secret and broadcast so the same selector has the same meaning throughout the CLI. IPC and storage still operate on the resolved id; release and offline-removal behavior are unchanged. Refuse ambiguous names and name/id-prefix collisions before any mutation, listing matching names and full ids without secrets. Full ids take precedence to provide a definitive escape from ambiguity. Preserve copied id ellipses without stripping literal ellipses from network names. Update help and examples. Make administrator-friendly, predictable CLI behavior a core requirement in AGENTS.md, including actionable errors and no guessing which resource a destructive command should affect. Validation on Windows: cargo fmt --all -- --check and workspace/all-target Clippy with -D warnings passed. Release workspace/all-target tests: 318 passed, including real-process online/offline leave-by-name, preservation of other networks, ambiguous/unknown selectors, and consistent name selection across commands. Two existing Windows SQLite wipe failures were explicitly skipped (a_wipe_removes_everything_and_the_next_start_is_a_stranger and wiping_twice_is_as_ordinary_as_wiping_once). The public-DHT smoke test and forwarding benchmark remain ignored by default. Tests use isolated state; the user's running agent and configured networks were not changed. --- AGENTS.md | 12 ++ README.md | 23 ++-- crates/tsunagi-cli/src/main.rs | 156 ++++++++++++++++-------- crates/tsunagi-cli/tests/network_cli.rs | 130 ++++++++++++++++++-- docs/broadcast.md | 10 +- docs/testing.md | 5 + 6 files changed, 261 insertions(+), 75 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 394ebb4..fe224ee 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,18 @@ messages. Scope and non-scope are in [README.md](README.md). There is no central server, no owner, no registration and no majority vote. Design accordingly: a majority is not a root of trust. +## User experience + +User convenience is a core requirement. The CLI should feel logical and +predictable to an experienced administrator: use consistent selectors and +flags, sensible defaults, and actionable errors. Let users refer to configured +networks by their exact name as well as by id or a unique id prefix; do not +make them copy an opaque identifier when the stored name identifies one network. +When a selector is ambiguous, list the matching names and ids and change +nothing. Never guess which resource a destructive command should affect. +Keep help text and examples in sync with behavior, and preserve existing +scripts when improving the interactive workflow. + ## Architectural boundaries Keep these separate. Crossing them is the main thing to review for. diff --git a/README.md b/README.md index ccf7efa..a997ddf 100644 --- a/README.md +++ b/README.md @@ -362,11 +362,11 @@ tsunagi id key rotate replace that key tsunagi network the networks this device belongs to tsunagi network join -n lab -s tsn1… join one; adds it to a running agent tsunagi network join -n lab resume one this device has, or make it on the spot -tsunagi network stop stop serving it, keeping everything -tsunagi network start serve it again, from where it left off -tsunagi network leave give up the address and name, then forget it +tsunagi network stop lab stop serving it, keeping everything +tsunagi network start lab serve it again, from where it left off +tsunagi network leave lab give up the address and name, then forget it tsunagi network secret the secret of each joined network -tsunagi network secret just that one, for copying +tsunagi network secret lab just that one, for copying tsunagi network secret generate a fresh secret for a network that does not exist yet tsunagi dns whether the local resolver is serving, and what @@ -599,8 +599,8 @@ received broadcasts never trigger another fanout. ```sh tsunagi join -n games --no-broadcast tsunagi join -n games --broadcast -tsunagi network broadcast off -tsunagi network broadcast on +tsunagi network broadcast games off +tsunagi network broadcast games on ``` The choice persists across restarts and a plain `join`. It can be changed while @@ -674,7 +674,7 @@ Membership outlives a session, so it also has to be possible to end it. ```bash tsunagi network # what this device belongs to -tsunagi network leave # give up the address and the name, then forget it +tsunagi network leave lab # give up the address and the name, then forget it tsunagi wipe --yes # remove everything and be a stranger again ``` @@ -703,9 +703,12 @@ nothing can sign or send it: the command says so and refuses, and `--offline` drops the network locally while leaving the others holding the old claim. The protocol key for that network goes too — rejoining is joining, not resuming. -A network is named by its id, never by its name: two networks can share a -name, and choosing between them for the user is how the wrong one gets left. A -unique prefix is enough. +`leave`, `stop`, `start`, `secret` and `broadcast` accept an exact stored network +name, its full id, or a unique id prefix (including a copied report ellipsis). +Names are case-sensitive. If several networks share a name, or a name matches +another network's id prefix, the command lists matching names and full ids and +changes nothing. Use a full id to choose explicitly; a full id always takes +precedence over a name. No secret is needed to select a configured network. `wipe` removes both directories' contents: the device identity, every network, every signed record and everything a protocol kept beside them. It refuses diff --git a/crates/tsunagi-cli/src/main.rs b/crates/tsunagi-cli/src/main.rs index 455f3fa..6f74cb7 100644 --- a/crates/tsunagi-cli/src/main.rs +++ b/crates/tsunagi-cli/src/main.rs @@ -147,7 +147,7 @@ struct JoinArgs { enum NetworkAction { /// Shows or changes this network's local LAN broadcast participation. Broadcast { - /// Network id or unique id prefix. + /// Exact network name, full id or unique id prefix. network: String, /// New choice; omit to show the stored choice. #[arg(value_enum)] @@ -168,12 +168,12 @@ enum NetworkAction { /// simply away, as if it had been switched off. It stays stopped /// across restarts until `tsunagi network start`. Stop { - /// Which network, by id. A unique prefix is enough. + /// Exact network name, full id or unique id prefix. network: String, }, /// Serves a stopped network again, from where it left off. Start { - /// Which network, by id. A unique prefix is enough. + /// Exact network name, full id or unique id prefix. network: String, }, /// Gives up this device's address and name in a network, and forgets it. @@ -186,8 +186,8 @@ enum NetworkAction { /// network's signed records, its cached hints and the protocol key it /// used. `stop` is the one that keeps them. Leave { - /// Which network, by id. A unique prefix is enough; the name is not, - /// because two networks may share one. + /// Exact network name, full id or unique id prefix. + /// If the name is shared, use the id to select one network. network: String, /// Remove it without telling anybody. @@ -202,8 +202,7 @@ enum NetworkAction { /// Printed only when asked for, never as part of an overview: these /// reports get pasted into chats and issue trackers. Secret { - /// Which network, by id; a unique prefix is enough. All of them if - /// omitted. + /// Exact network name, full id or unique id prefix. All of them if omitted. network: Option, #[command(subcommand)] @@ -1498,44 +1497,51 @@ fn stored_networks(paths: &StoragePaths) -> Vec .unwrap_or_default() } -/// Finds the one configured network whose id starts with `wanted`. +/// Resolves a stored network by exact name, full id or unique id prefix. /// -/// A prefix, because the ids are 52 characters and `status` prints them -/// shortened; the name is deliberately not accepted, since two networks can -/// share one and choosing for the user is how the wrong network gets left. +/// Full ids are definitive. Otherwise name and prefix matches must identify +/// exactly one network: a shared name or a name colliding with another id's +/// prefix must never select an arbitrary network for a destructive command. fn resolve_network<'a>( networks: &'a [tsunagi::storage::StoredNetwork], wanted: &str, ) -> Result<&'a tsunagi::storage::StoredNetwork, String> { - let wanted = wanted.trim().trim_end_matches('…'); + let wanted = wanted.trim_matches(|c: char| c.is_ascii_whitespace()); if wanted.is_empty() { - return Err("name a network by its id; `tsunagi network` lists them".to_string()); + return Err("select a network by name or id; `tsunagi network` lists them".to_string()); } + if let Some(network) = networks + .iter() + .find(|network| network.network_id.to_string() == wanted) + { + return Ok(network); + } + // Only id prefixes lose a copied report ellipsis; names are used verbatim. + let prefix = wanted.trim_end_matches('…'); let matched: Vec<&tsunagi::storage::StoredNetwork> = networks .iter() - .filter(|network| network.network_id.to_string().starts_with(wanted)) + .filter(|network| { + network.name.as_str() == wanted + || (!prefix.is_empty() && network.network_id.to_string().starts_with(prefix)) + }) .collect(); match matched.as_slice() { [one] => Ok(one), - [] => { - if networks + [] => Err(format!( + "no configured network matches name or id `{wanted}`; \ + `tsunagi network` lists them" + )), + several => { + let candidates = several .iter() - .any(|network| network.name.as_str() == wanted) - { - return Err(format!( - "`{wanted}` is a network name, not an id. Two networks can share a name, \ - so this takes the id; `tsunagi network` lists them." - )); - } + .map(|network| format!(" `{}` {}", network.name, network.network_id)) + .collect::>() + .join("\n"); Err(format!( - "no configured network has an id starting `{wanted}`; \ - `tsunagi network` lists them" + "`{wanted}` matches {} networks; use the full id or a unique id prefix:\n{candidates}", + several.len() )) } - several => Err(format!( - "`{wanted}` matches {} networks; use more of the id", - several.len() - )), } } @@ -4320,21 +4326,68 @@ mod network_tests { assert_eq!(picked.network_id, networks[0].network_id); let err = resolve_network(&networks, "").unwrap_err(); - assert!(err.contains("by its id"), "{err}"); + assert!(err.contains("by name or id"), "{err}"); } #[test] - fn a_name_is_refused_because_two_networks_can_share_one() { + fn a_unique_exact_name_selects_the_stored_network_without_normalizing_it() { + let networks = vec![ + configured("lab", "one"), + configured("Lab", "two"), + configured("Игры дома…", "three"), + ]; + for network in &networks { + assert_eq!( + resolve_network(&networks, network.name.as_str()) + .unwrap() + .network_id, + network.network_id + ); + } + assert!(resolve_network(&networks, "LAB").is_err()); + assert!(resolve_network(&networks, "Игры дома").is_err()); + assert!(resolve_network(&networks, "Игры").is_err()); + assert!(resolve_network(&networks, "…").is_err()); + } + + #[test] + fn a_shared_name_lists_the_ids_without_choosing_or_disclosing_secrets() { // Exactly the situation this command exists for: two networks called // `lab`, one of them joined with a mistyped secret. Choosing for the // user here is how the wrong one gets left. let networks = vec![configured("lab", "one"), configured("lab", "two")]; let err = resolve_network(&networks, "lab").unwrap_err(); - assert!(err.contains("not an id"), "{err}"); - assert!( - err.contains("tsunagi network"), - "it says where to look: {err}" + assert!(err.contains("matches 2 networks"), "{err}"); + for network in &networks { + assert!(err.contains(&network.network_id.to_string()), "{err}"); + assert!(!err.contains(network.secret.encode().as_str()), "{err}"); + } + } + + #[test] + fn a_name_colliding_with_an_id_prefix_requires_a_full_id() { + let first = configured("lab", "one"); + let id = first.network_id.to_string(); + let networks = vec![ + first, + configured(&id[..10], "two"), + configured(&id, "three"), + ]; + let err = resolve_network(&networks, &id[..10]).unwrap_err(); + assert!(err.contains("matches 2 networks"), "{err}"); + // An explicit full id remains definitive, even if another name is an id. + assert_eq!( + resolve_network(&networks, &id).unwrap().network_id, + networks[0].network_id ); + for network in &networks[1..] { + assert_eq!( + resolve_network(&networks, &network.network_id.to_string()) + .unwrap() + .network_id, + network.network_id + ); + } } #[test] @@ -4346,21 +4399,26 @@ mod network_tests { #[test] fn a_prefix_shared_by_two_networks_is_refused_rather_than_guessed() { - let networks = vec![configured("lab", "one"), configured("other", "two")]; - let shared = &networks[0].network_id.to_string()[..1]; - let both = networks + // 33 distinct ids must share at least one initial base32 character. + let networks: Vec<_> = (0..33) + .map(|i| configured(&format!("network-{i}"), "secret")) + .collect(); + let shared = networks .iter() - .filter(|network| network.network_id.to_string().starts_with(shared)) - .count(); - if both < 2 { - // The two derived ids happen not to share a first character; - // the empty prefix is the same question with a certain answer. - let err = resolve_network(&networks, "").unwrap_err(); - assert!(err.contains("by its id"), "{err}"); - return; - } - let err = resolve_network(&networks, shared).unwrap_err(); - assert!(err.contains("use more of the id"), "{err}"); + .map(|network| network.network_id.to_string()[..1].to_string()) + .find(|prefix| { + networks + .iter() + .filter(|network| network.network_id.to_string().starts_with(prefix)) + .count() + > 1 + }) + .unwrap(); + let err = resolve_network(&networks, &shared).unwrap_err(); + assert!( + err.contains("use the full id or a unique id prefix"), + "{err}" + ); } } diff --git a/crates/tsunagi-cli/tests/network_cli.rs b/crates/tsunagi-cli/tests/network_cli.rs index 87e9897..b583248 100644 --- a/crates/tsunagi-cli/tests/network_cli.rs +++ b/crates/tsunagi-cli/tests/network_cli.rs @@ -25,17 +25,28 @@ impl Drop for Running { impl Running { fn run(&self, args: &[&str]) -> std::process::Output { - std::process::Command::new(env!("CARGO_BIN_EXE_tsunagi")) - .args(args) - .arg("--state-dir") - .arg(self.dir.path().join("state")) - .arg("--cache-dir") - .arg(self.dir.path().join("cache")) - .output() - .expect("the agent binary runs") + run_in(&self.dir, args) } } +fn run_in(dir: &TempDir, args: &[&str]) -> std::process::Output { + std::process::Command::new(env!("CARGO_BIN_EXE_tsunagi")) + .args(args) + .arg("--state-dir") + .arg(dir.path().join("state")) + .arg("--cache-dir") + .arg(dir.path().join("cache")) + .output() + .expect("the agent binary runs") +} + +fn read_networks(dir: &TempDir) -> Vec { + tsunagi::storage::StateStore::open(dir.path().join("state/state.sqlite")) + .unwrap() + .list_networks() + .unwrap() +} + /// An agent with no network at all, the way a daemon is started before /// anything has been decided. fn start_bare(port: u16) -> Running { @@ -155,7 +166,7 @@ fn a_network_can_be_stopped_and_resumed_without_losing_anything() { .to_string(); assert!(secret_before.starts_with("tsn1")); - let stopped = agent.run(&["network", "stop", &id[..10]]); + let stopped = agent.run(&["network", "stop", "resident"]); assert!( stopped.status.success(), "{}", @@ -181,7 +192,7 @@ fn a_network_can_be_stopped_and_resumed_without_losing_anything() { String::from_utf8_lossy(&again.stdout) ); - let started = agent.run(&["network", "start", &id[..10]]); + let started = agent.run(&["network", "start", "resident"]); assert!(started.status.success()); assert!( String::from_utf8_lossy(&started.stdout).contains("started `resident`"), @@ -192,7 +203,7 @@ fn a_network_can_be_stopped_and_resumed_without_losing_anything() { assert!(listed.contains("running"), "{listed}"); // And nothing was given up on the way: the same network, same secret. - let secret_after = agent.run(&["network", "secret", &id[..10]]); + let secret_after = agent.run(&["network", "secret", "resident"]); assert_eq!( String::from_utf8_lossy(&secret_after.stdout).trim(), secret_before, @@ -290,7 +301,7 @@ fn broadcast_choice_is_per_network_live_persistent_and_preserved_by_rejoin() { .broadcast ); let id = disabled.network_id.to_string(); - let change = agent.run(&["network", "broadcast", &id, "on"]); + let change = agent.run(&["network", "broadcast", "broadcast-off", "on"]); assert!( change.status.success(), "{}", @@ -312,3 +323,98 @@ fn broadcast_choice_is_per_network_live_persistent_and_preserved_by_rejoin() { .success() ); } + +#[test] +fn leaving_by_name_removes_only_that_network_from_a_running_agent() { + let agent = start("resident", 0); + assert!(agent.run(&["join", "-n", "games"]).status.success()); + let resident = read_networks(&agent.dir) + .into_iter() + .find(|network| network.name.as_str() == "resident") + .unwrap(); + + let left = agent.run(&["network", "leave", "games"]); + assert!( + left.status.success(), + "{}", + String::from_utf8_lossy(&left.stderr) + ); + assert!(String::from_utf8_lossy(&left.stdout).contains("left `games`")); + let remaining = read_networks(&agent.dir); + assert_eq!(remaining.len(), 1); + assert_eq!(remaining[0].network_id, resident.network_id); + assert_eq!( + remaining[0].secret.encode().as_str(), + resident.secret.encode().as_str() + ); + let status = agent.run(&["network"]); + assert!(status.status.success()); + let out = String::from_utf8_lossy(&status.stdout); + assert!(out.contains("resident") && out.contains("running"), "{out}"); + assert!(!out.contains("games"), "{out}"); +} + +#[test] +fn leaving_by_name_offline_keeps_the_explicit_offline_requirement() { + let dir = TempDir::new().unwrap(); + for name in ["resident", "games"] { + assert!(run_in(&dir, &["join", "-n", name]).status.success()); + } + let refused = run_in(&dir, &["network", "leave", "games"]); + assert!(!refused.status.success()); + assert!(String::from_utf8_lossy(&refused.stderr).contains("--offline")); + assert_eq!(read_networks(&dir).len(), 2); + + let left = run_in(&dir, &["network", "leave", "games", "--offline"]); + assert!( + left.status.success(), + "{}", + String::from_utf8_lossy(&left.stderr) + ); + let remaining = read_networks(&dir); + assert_eq!(remaining.len(), 1); + assert_eq!(remaining[0].name.as_str(), "resident"); +} + +#[test] +fn ambiguous_and_unknown_names_leave_every_network_untouched() { + let agent = start_bare(0); + for secret in [ + "first-secret-for-the-cli-test", + "second-secret-for-the-cli-test", + ] { + assert!( + agent + .run(&["join", "-n", "games", "-s", secret]) + .status + .success() + ); + } + let before = read_networks(&agent.dir); + assert_eq!(before.len(), 2); + for name in ["games", "missing-network"] { + let result = agent.run(&["network", "leave", name]); + assert!(!result.status.success()); + let err = String::from_utf8_lossy(&result.stderr); + if name == "games" { + assert!(err.contains("matches 2 networks"), "{err}"); + for network in &before { + assert!(err.contains(&network.network_id.to_string()), "{err}"); + assert!(!err.contains(network.secret.encode().as_str()), "{err}"); + } + } else { + assert!(err.contains("no configured network"), "{err}"); + } + assert_eq!(read_networks(&agent.dir).len(), 2); + } + // The suggested full id still selects exactly one of the same-name networks. + let left = agent.run(&["network", "leave", &before[0].network_id.to_string()]); + assert!( + left.status.success(), + "{}", + String::from_utf8_lossy(&left.stderr) + ); + let remaining = read_networks(&agent.dir); + assert_eq!(remaining.len(), 1); + assert_eq!(remaining[0].network_id, before[1].network_id); +} diff --git a/docs/broadcast.md b/docs/broadcast.md index 2cbb4b2..eeca189 100644 --- a/docs/broadcast.md +++ b/docs/broadcast.md @@ -8,14 +8,16 @@ control session. It is not a network-wide vote or signed shared configuration. ```sh tsunagi join -n games --no-broadcast tsunagi join -n games --broadcast -tsunagi network broadcast off -tsunagi network broadcast on -tsunagi network broadcast +tsunagi network broadcast games off +tsunagi network broadcast games on +tsunagi network broadcast games ``` Joining without either flag preserves the saved choice. The `network broadcast` command can change a running network immediately or configure it while the -agent is stopped. `status` reports the local setting. There is no global `up` +agent is stopped. It accepts the exact network name, full id or a unique id +prefix; ambiguous selectors list candidates without changing anything. +`status` reports the local setting. There is no global `up` override: starting the agent preserves every network's independent policy. ## Delivery diff --git a/docs/testing.md b/docs/testing.md index 5f0c73c..4291987 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -108,6 +108,11 @@ with no secret invents one, prints it in full and prints a line the other side can paste unchanged; a bare name this device already knows resumes that network instead of inventing another of the same name; and a network can be stopped and started again with its secret and its place intact. +Network selection accepts exact names consistently across commands. Leave-by-name +is tested with a live agent and with offline state, preserving other networks; +duplicate and unknown names change nothing and full ids resolve ambiguity. +Selector unit tests cover case-sensitive names, Unicode/ellipsis names, copied +id prefixes and collisions between names and ids without revealing secrets. `crates/tsunagi-cli/tests/dns_service.rs` runs the real binary: the resolver comes up with no interface to attach it to, the listener is not rebuilt on