diff --git a/README.md b/README.md index 7af9a3d..97b1635 100644 --- a/README.md +++ b/README.md @@ -167,10 +167,11 @@ daemon. Everything that touches the network goes through it. `config`, `skills`, `completions` and `portmap` stay local, because they are about this machine -rather than a session. `shares add`/`remove` do both: they write the config -file *and* update a running daemon, so a folder is served straight away rather -than at its next start. Files land on the *daemon's* filesystem, and `daemon -status` reports where. +rather than a session. `shares add`/`remove` and `config set download_dir` do +both: they write the config file *and* update a running daemon, so a folder is +served — or downloaded into — straight away rather than at the daemon's next +start. Files land on the *daemon's* filesystem, and `daemon status` reports +where. #### A download box you drive from your laptop diff --git a/docs/openrpc.json b/docs/openrpc.json index 0cd8934..144c76a 100644 --- a/docs/openrpc.json +++ b/docs/openrpc.json @@ -189,6 +189,18 @@ ], "type": "object" }, + "DirectoryParams": { + "description": "Where the daemon should land transfers from now on, on *its* filesystem.\nThe same trust as `shares.set`: an authenticated caller is the owner.", + "properties": { + "directory": { + "type": "string" + } + }, + "required": [ + "directory" + ], + "type": "object" + }, "DownloadDto": { "properties": { "download_directory": { @@ -1560,6 +1572,26 @@ }, "summary": "Drop a transfer that has not started." }, + { + "name": "download.set_dir", + "paramStructure": "by-name", + "params": [ + { + "name": "directory", + "required": true, + "schema": { + "type": "string" + } + } + ], + "result": { + "name": "result", + "schema": { + "$ref": "#/components/schemas/Ack" + } + }, + "summary": "Land future transfers in a different folder on the daemon's host." + }, { "name": "upload.list", "paramStructure": "by-name", diff --git a/soulseek-rs/src/SKILL.md b/soulseek-rs/src/SKILL.md index b1010d5..22345e0 100644 --- a/soulseek-rs/src/SKILL.md +++ b/soulseek-rs/src/SKILL.md @@ -58,8 +58,9 @@ So: **one daemon, then as many commands as you like.** Reach for `--no-daemon` only when you deliberately want an isolated session, and expect to justify it. Two things change while a daemon is running: files land in the *daemon's* -download directory (`daemon status` reports which), and `shares add` updates it -straight away rather than at its next start. +download directory (`daemon status` reports which), and `shares add` or +`config set download_dir` update it straight away rather than at its next +start. ## The contract diff --git a/soulseek-rs/src/api.rs b/soulseek-rs/src/api.rs index b660b62..31cdac0 100644 --- a/soulseek-rs/src/api.rs +++ b/soulseek-rs/src/api.rs @@ -152,6 +152,13 @@ pub trait SessionApi: Send + Sync { fn shared_counts(&self) -> (u32, u32); fn shared_directories(&self) -> Vec; fn set_shared_directories(&self, directories: Vec) -> Result<()>; + + /// Point future transfers at `directory`. + /// + /// Only a daemon holds a download folder of its own — locally every + /// download call names its directory, so the local session has nothing to + /// store and accepts silently. + fn set_download_directory(&self, directory: String) -> Result<()>; } /// The local session: the library client itself, so `Arc` is already an @@ -353,4 +360,8 @@ impl SessionApi for Client { fn set_shared_directories(&self, directories: Vec) -> Result<()> { Self::set_shared_directories(self, directories) } + + fn set_download_directory(&self, _directory: String) -> Result<()> { + Ok(()) + } } diff --git a/soulseek-rs/src/commands/settings.rs b/soulseek-rs/src/commands/settings.rs index 2b37669..01057d2 100644 --- a/soulseek-rs/src/commands/settings.rs +++ b/soulseek-rs/src/commands/settings.rs @@ -55,29 +55,45 @@ pub struct LiveDaemon { pub token: Option, } -/// Tell a running daemon what this machine now shares. +/// Tell a running daemon about a change it should apply now. /// -/// Locally, `shares add` means "shared from the next run on", because there is -/// no session yet. A daemon *is* that next run and it is already going, so -/// leaving it on its startup list would make the command look like it had -/// done nothing — the folder configured, and still invisible to the network. -fn push_to_daemon(out: &Out, daemon: &LiveDaemon, directories: Vec) { +/// Locally a config change means "from the next run on", because there is no +/// session yet. A daemon *is* that next run and it is already going, so +/// leaving it on its startup settings would make the command look like it +/// had done nothing — the folder configured, and the network none the wiser. +fn push_to_daemon( + out: &Out, + daemon: &LiveDaemon, + applied: &str, + kept: &str, + apply: impl FnOnce(&RemoteSession) -> soulseek_rs::Result<()>, +) { let Some(endpoint) = daemon.endpoint.as_ref() else { return; }; let token = daemon.token.clone().or_else(crate::daemon::stored_token); let outcome = RemoteSession::connect(endpoint, token) - .and_then(|session| session.set_shared_directories(directories)); + .and_then(|session| apply(&session)); match outcome { - Ok(()) => out.status("the running daemon is re-indexing"), + Ok(()) => out.status(applied), // Not fatal: the config file is written either way, so the change // still takes effect when the daemon next starts. Err(e) => out.warn(&format!( - "the daemon kept its own shares ({e}); restart it to pick this up" + "the daemon kept {kept} ({e}); restart it to pick this up" )), } } +fn push_shares(out: &Out, daemon: &LiveDaemon, directories: Vec) { + push_to_daemon( + out, + daemon, + "the running daemon is re-indexing", + "its own shares", + move |session| session.set_shared_directories(directories), + ); +} + pub fn shares_add( out: &Out, store: &Store, @@ -104,7 +120,7 @@ pub fn shares_add( updated.shared_dir = None; store.save(&updated)?; out.status(&format!("sharing {directory}")); - push_to_daemon(out, daemon, dirs); + push_shares(out, daemon, dirs); Ok(()) } @@ -128,7 +144,7 @@ pub fn shares_remove( updated.shared_dir = None; store.save(&updated)?; out.status(&format!("no longer sharing {directory}")); - push_to_daemon(out, daemon, dirs); + push_shares(out, daemon, dirs); Ok(()) } @@ -228,14 +244,36 @@ pub fn config_set( store: &Store, key: &str, value: &str, + daemon: impl FnOnce() -> LiveDaemon, ) -> CliResult { let mut updated = store.config.clone(); updated.set(key, value).map_err(CliError::usage)?; store.save(&updated)?; + let current = updated.get(key); out.emit(&ConfigRecord { key: key.to_string(), - value: updated.get(key), + value: current.clone(), }); + // A running daemon holds the live settings, so for the keys it owns a + // config write alone would look like it did nothing until its next + // restart — the same trap `shares add` closes by pushing to the session. + // `daemon` is resolved lazily: finding one means connecting to a socket, + // which the other keys should not pay for. + match key { + "download_dir" => { + if let Some(directory) = current { + push_to_daemon( + out, + &daemon(), + "the running daemon will download there now", + "its own download folder", + move |session| session.set_download_directory(directory), + ); + } + } + "shared_dirs" => push_shares(out, &daemon(), share_list(&updated)), + _ => {} + } Ok(()) } diff --git a/soulseek-rs/src/daemon/mod.rs b/soulseek-rs/src/daemon/mod.rs index 7287ef1..ebd487d 100644 --- a/soulseek-rs/src/daemon/mod.rs +++ b/soulseek-rs/src/daemon/mod.rs @@ -221,7 +221,7 @@ pub fn run( hub: Arc::clone(&hub), browses: Arc::clone(&browses), downloads: Arc::clone(&downloads), - download_dir: ctx.download_dir.clone(), + download_dir: Mutex::new(ctx.download_dir.clone()), server: ctx.settings.server_address.to_string(), token: token.clone(), started: Instant::now(), diff --git a/soulseek-rs/src/daemon/openrpc.rs b/soulseek-rs/src/daemon/openrpc.rs index 80f05ae..30e8d56 100644 --- a/soulseek-rs/src/daemon/openrpc.rs +++ b/soulseek-rs/src/daemon/openrpc.rs @@ -10,11 +10,11 @@ use super::proto::{ Ack, AuthParams, AuthResult, BrowseEvent, DaemonStatus, DirectoriesParams, - DownloadStartParams, DownloadStarted, DownloadStatusEvent, Downloads, - Event, IntervalSeconds, Members, MessageParams, Messages, Method, - QueryParams, RoomEventDto, RoomRef, SayParams, SearchResults, Searches, - Seconds, SessionLossEvent, SharesStatus, SlotsParams, TransferRef, - UploadInfoDto, Uploads, UserMessageDto, UserRef, UserResult, + DirectoryParams, DownloadStartParams, DownloadStarted, DownloadStatusEvent, + Downloads, Event, IntervalSeconds, Members, MessageParams, Messages, + Method, QueryParams, RoomEventDto, RoomRef, SayParams, SearchResults, + Searches, Seconds, SessionLossEvent, SharesStatus, SlotsParams, + TransferRef, UploadInfoDto, Uploads, UserMessageDto, UserRef, UserResult, }; use schemars::{JsonSchema, SchemaGenerator, generate::SchemaSettings}; use serde_json::{Value, json}; @@ -49,6 +49,7 @@ fn params_schema(method: Method, generator: &mut SchemaGenerator) -> Value { | Method::DownloadRemove | Method::DownloadRemoveQueued | Method::UploadCancel => schema_of::(generator), + Method::DownloadSetDir => schema_of::(generator), Method::UploadSlots => schema_of::(generator), Method::RoomJoin | Method::RoomLeave | Method::RoomMembers => { schema_of::(generator) @@ -104,6 +105,7 @@ fn result_schema(method: Method, generator: &mut SchemaGenerator) -> Value { | Method::DownloadResume | Method::DownloadRemove | Method::DownloadRemoveQueued + | Method::DownloadSetDir | Method::UploadCancel | Method::UploadSlots | Method::PrivilegesCheck @@ -155,6 +157,9 @@ fn summary(method: Method) -> &'static str { Method::DownloadResume => "Resume a paused transfer.", Method::DownloadRemove => "Forget a transfer entirely.", Method::DownloadRemoveQueued => "Drop a transfer that has not started.", + Method::DownloadSetDir => { + "Land future transfers in a different folder on the daemon's host." + } Method::UploadList => "Uploads being served, active first.", Method::UploadCancel => "Stop serving an upload in progress.", Method::UploadSlots => "How many uploads may run at once.", diff --git a/soulseek-rs/src/daemon/proto/method.rs b/soulseek-rs/src/daemon/proto/method.rs index 56a708e..96a69c2 100644 --- a/soulseek-rs/src/daemon/proto/method.rs +++ b/soulseek-rs/src/daemon/proto/method.rs @@ -81,6 +81,14 @@ pub struct DirectoriesParams { pub directories: Vec, } +/// Where the daemon should land transfers from now on, on *its* filesystem. +/// The same trust as `shares.set`: an authenticated caller is the owner. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +#[cfg_attr(test, derive(schemars::JsonSchema))] +pub struct DirectoryParams { + pub directory: String, +} + /// Every search this session has run, newest last. Shared by all clients, so /// a second one attaching sees what the first has been looking for. #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] diff --git a/soulseek-rs/src/daemon/proto/mod.rs b/soulseek-rs/src/daemon/proto/mod.rs index bd464db..37ccbd2 100644 --- a/soulseek-rs/src/daemon/proto/mod.rs +++ b/soulseek-rs/src/daemon/proto/mod.rs @@ -90,6 +90,7 @@ wire_vocabulary! { DownloadResume => "download.resume", DownloadRemove => "download.remove", DownloadRemoveQueued => "download.remove_queued", + DownloadSetDir => "download.set_dir", UploadList => "upload.list", UploadCancel => "upload.cancel", UploadSlots => "upload.slots", diff --git a/soulseek-rs/src/daemon/server/mod.rs b/soulseek-rs/src/daemon/server/mod.rs index 19674e3..9e3e292 100644 --- a/soulseek-rs/src/daemon/server/mod.rs +++ b/soulseek-rs/src/daemon/server/mod.rs @@ -8,12 +8,12 @@ use super::hub::{Hub, PendingBrowses, PendingDownload}; use super::proto::{ Ack, AuthResult, CODE_APPLICATION, CODE_INVALID_PARAMS, ChatMessageDto, - DaemonStatus, DirectoriesParams, DownloadDto, DownloadStartParams, - DownloadStarted, Downloads, IntervalSeconds, Members, MessageParams, - Messages, Method, OPENRPC, PROTOCOL_VERSION, QueryParams, RoomRef, - RpcError, SayParams, SearchResultDto, SearchResults, SearchSummary, - Searches, Seconds, SharesStatus, SlotsParams, TransferRef, UploadInfoDto, - Uploads, UserInfoDto, UserRef, UserResult, + DaemonStatus, DirectoriesParams, DirectoryParams, DownloadDto, + DownloadStartParams, DownloadStarted, Downloads, IntervalSeconds, Members, + MessageParams, Messages, Method, OPENRPC, PROTOCOL_VERSION, QueryParams, + RoomRef, RpcError, SayParams, SearchResultDto, SearchResults, + SearchSummary, Searches, Seconds, SharesStatus, SlotsParams, TransferRef, + UploadInfoDto, Uploads, UserInfoDto, UserRef, UserResult, }; use crate::api::SessionApi; use crate::output::Exit; @@ -29,9 +29,9 @@ pub struct Daemon { pub hub: Arc, pub browses: Arc, pub downloads: Arc>>, - /// Where transfers land, on *this* machine. A remote caller cannot see - /// this filesystem, so it does not get to choose. - pub download_dir: String, + /// Where transfers land, on *this* machine. A caller may point it at a + /// different folder with `download.set_dir`, but never per download. + pub download_dir: Mutex, pub server: String, pub token: String, pub started: Instant, @@ -126,6 +126,9 @@ impl Daemon { self.session.start_wishlist_search(&query.query).map_err( |e| RpcError::application(Exit::Connection, e.to_string()), )?; + // Without a start time every other window would render this + // search as finished the moment it goes out. + self.record_search_start(&query.query); ok(Ack::OK) } Method::SearchWishlistInterval => ok(IntervalSeconds { @@ -133,6 +136,11 @@ impl Daemon { }), Method::DownloadStart => self.start_download(parse(params)?), + Method::DownloadSetDir => { + let dir: DirectoryParams = parse(params)?; + self.set_download_dir(dir.directory)?; + ok(Ack::OK) + } Method::DownloadList => ok(Downloads { downloads: self .session @@ -344,10 +352,10 @@ impl Daemon { params: DownloadStartParams, ) -> Result { // The daemon's own directory, always. Letting a caller name a path - // would hand any authenticated client an arbitrary write on this - // host's filesystem, and a remote one cannot see that filesystem to - // choose sensibly anyway. - let directory = self.download_dir.clone(); + // per download would scatter transfers in ways `daemon.status` cannot + // report; changing the folder is a deliberate session-wide act, + // `download.set_dir`. + let directory = self.download_dir(); let (download, updates) = self .session .download_with_metadata( @@ -376,6 +384,35 @@ impl Daemon { }) } + fn download_dir(&self) -> String { + self.download_dir + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + } + + /// Point future transfers at `directory`, creating it on *this* host. + /// The same trust boundary as `shares.set`: authenticated means owner. + fn set_download_dir(&self, directory: String) -> Result<(), RpcError> { + if directory.trim().is_empty() { + return Err(RpcError::new( + CODE_INVALID_PARAMS, + "the download directory cannot be empty".to_string(), + )); + } + std::fs::create_dir_all(&directory).map_err(|e| { + RpcError::application( + Exit::Usage, + format!("cannot create {directory}: {e}"), + ) + })?; + *self + .download_dir + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = directory; + Ok(()) + } + fn shares(&self) -> SharesStatus { let (folders, files) = self.session.shared_counts(); SharesStatus { @@ -427,7 +464,7 @@ impl Daemon { listen_port: self.session.listen_port(), shared_folders: folders, shared_files: files, - download_dir: self.download_dir.clone(), + download_dir: self.download_dir(), session_loss: self.session.session_loss().map(Into::into), clients: self.hub.subscribers(), uptime_secs: self.started.elapsed().as_secs(), @@ -577,7 +614,7 @@ mod tests { hub: Arc::new(Hub::new()), browses: Arc::new(PendingBrowses::default()), downloads: Arc::new(Mutex::new(Vec::new())), - download_dir: "/tmp".to_string(), + download_dir: Mutex::new("/tmp".to_string()), server: "server.invalid:2416".to_string(), token: token.to_string(), started: Instant::now(), @@ -771,5 +808,11 @@ mod tests { ) -> soulseek_rs::Result<()> { Ok(()) } + fn set_download_directory( + &self, + _directory: String, + ) -> soulseek_rs::Result<()> { + Ok(()) + } } } diff --git a/soulseek-rs/src/remote/mod.rs b/soulseek-rs/src/remote/mod.rs index 656f517..970812b 100644 --- a/soulseek-rs/src/remote/mod.rs +++ b/soulseek-rs/src/remote/mod.rs @@ -10,11 +10,12 @@ mod mirror; use crate::api::{SessionApi, SessionSearch}; use crate::daemon::proto::{ Ack, AuthParams, AuthResult, CODE_APPLICATION, ChatMessageDto, - DaemonStatus, DirectoriesParams, DownloadDto, DownloadStartParams, - DownloadStarted, Downloads, Event, IntervalSeconds, Members, MessageParams, - Messages, Method, PROTOCOL_VERSION, QueryParams, Request, Response, - RoomRef, RpcError, SayParams, SearchResults, Searches, Seconds, - SharesStatus, SlotsParams, TransferRef, Uploads, UserRef, UserResult, + DaemonStatus, DirectoriesParams, DirectoryParams, DownloadDto, + DownloadStartParams, DownloadStarted, Downloads, Event, IntervalSeconds, + Members, MessageParams, Messages, Method, PROTOCOL_VERSION, QueryParams, + Request, Response, RoomRef, RpcError, SayParams, SearchResults, Searches, + Seconds, SharesStatus, SlotsParams, TransferRef, Uploads, UserRef, + UserResult, }; use mirror::Mirror; use serde::de::DeserializeOwned; @@ -781,6 +782,10 @@ impl SessionApi for RemoteSession { fn set_shared_directories(&self, directories: Vec) -> Result<()> { self.tell(Method::SharesSet, DirectoriesParams { directories }) } + + fn set_download_directory(&self, directory: String) -> Result<()> { + self.tell(Method::DownloadSetDir, DirectoryParams { directory }) + } } #[cfg(test)] diff --git a/soulseek-rs/src/run.rs b/soulseek-rs/src/run.rs index 05fec99..bb162ee 100644 --- a/soulseek-rs/src/run.rs +++ b/soulseek-rs/src/run.rs @@ -10,7 +10,7 @@ use crate::output::{CliError, CliResult, Exit, Out}; use crate::ui::launch_main_tui; use soulseek_rs::{ClientSettings, ClientVersion, PeerAddress}; use std::io::{BufRead, IsTerminal}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; use std::{env, io}; @@ -38,7 +38,7 @@ pub fn run(mut cli: Cli, out: &Out) -> CliResult { // rather than logging in a second time, which the server would answer // by cutting the daemon off. if let Some(endpoint) = daemon_endpoint(&cli, &resolved) { - return run_attached_tui(&cli, &resolved, &endpoint); + return run_attached_tui(&resolved, &endpoint, config_path); } return run_default_tui(&cli, &resolved, config_path, &file_config); }; @@ -72,7 +72,11 @@ pub fn run(mut cli: Cli, out: &Out) -> CliResult { } Commands::Config(ConfigCommand::Set { ref key, ref value }) => { return crate::commands::settings::config_set( - out, &store, key, value, + out, + &store, + key, + value, + || live_daemon(&cli, &resolved), ); } Commands::Shares(SharesCommand::List) => { @@ -442,7 +446,12 @@ fn run_default_tui( } }; - persist_credentials(&outcome, config_path, file_config, &secret_store); + persist_credentials( + &outcome, + config_path.as_deref(), + file_config, + &secret_store, + ); // Best-effort: make ourselves reachable behind a home router so firewalled // peers can connect back. Mapped only once the listener is up, and for the @@ -462,6 +471,7 @@ fn run_default_tui( resolved.max_concurrent_downloads, Duration::from_secs(resolved.search_timeout), store, + config_path, ) .map_err(|e| CliError::new(Exit::Failure, e.to_string())) } @@ -472,9 +482,9 @@ fn run_default_tui( /// ask for and nothing to store. Everything the UI shows comes back over the /// socket, and what it persists stays daemon-side. fn run_attached_tui( - cli: &Cli, resolved: &crate::persist::config::Resolved, endpoint: &crate::remote::Endpoint, + config_path: Option, ) -> CliResult { use ratatui::crossterm::{ event::EnableMouseCapture, @@ -482,7 +492,9 @@ fn run_attached_tui( terminal::{Clear, ClearType}, }; - let token = cli + // The resolved token, not the flag alone: `daemon_token` in config.toml + // is the documented remote setup, and the flag/env are layered into it. + let token = resolved .daemon_token .clone() .or_else(crate::daemon::stored_token); @@ -493,6 +505,21 @@ fn run_attached_tui( )) })?; + // The settings form shows and edits the *daemon's* download folder — the + // one transfers actually land in — not this machine's configured default. + // The connect above just succeeded, so a daemon that cannot answer this + // is a real problem to report, not one to paper over with a local path. + let download_dir = session + .ask_for::( + crate::daemon::proto::Method::DaemonStatus, + ) + .map(|status| status.download_dir) + .map_err(|e| { + CliError::connection(format!( + "the daemon at {endpoint} did not report its status: {e}" + )) + })?; + soulseek_rs::utils::logger::enable_buffering(); let _ = execute!(std::io::stdout(), Clear(ClearType::All), EnableMouseCapture); @@ -503,10 +530,11 @@ fn run_attached_tui( launch_main_tui( terminal, Arc::new(session), - resolved.download_dir.clone(), + download_dir, resolved.max_concurrent_downloads, Duration::from_secs(resolved.search_timeout), None, + config_path, ) .map_err(|e| CliError::new(Exit::Failure, e.to_string())) } @@ -516,7 +544,7 @@ fn run_attached_tui( /// best-effort: failing to persist must not break a working session. fn persist_credentials( outcome: &crate::ui::login::LoginOutcome, - config_path: Option, + config_path: Option<&Path>, file_config: &crate::persist::config::FileConfig, secret_store: &dyn crate::persist::secret::SecretStore, ) { @@ -525,7 +553,7 @@ fn persist_credentials( { let mut updated = file_config.clone(); updated.username = Some(outcome.username.clone()); - if let Err(e) = updated.save(&path) { + if let Err(e) = updated.save(path) { eprintln!("⚠️ Could not save config: {e}"); } } diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index 01e8aa2..7894b4b 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -28,6 +28,9 @@ pub struct MainTui { search_timeout: Duration, spinner_state: usize, store: Option, + /// Where settings changes are persisted — the config file this run + /// resolved, so `--config` is honoured. + config_path: Option, /// Last snapshot written to disk, to skip no-op saves. saved_snapshot: Snapshot, } @@ -39,6 +42,7 @@ impl MainTui { max_concurrent_downloads: usize, search_timeout: Duration, store: Option, + config_path: Option, ) -> Self { let mut tui = Self { client, @@ -48,6 +52,7 @@ impl MainTui { search_timeout, spinner_state: 0, store, + config_path, saved_snapshot: Snapshot::default(), }; tui.restore_persisted_state(); @@ -254,6 +259,7 @@ pub fn launch_main_tui( max_concurrent_downloads: usize, search_timeout: Duration, store: Option, + config_path: Option, ) -> Result<()> { let tui = MainTui::new( client, @@ -261,6 +267,7 @@ pub fn launch_main_tui( max_concurrent_downloads, search_timeout, store, + config_path, ); tui.run(terminal) } @@ -280,6 +287,10 @@ mod tests { searches: std::sync::Mutex>, /// Whether this session is a daemon's. Local sessions do not sync. shared: bool, + /// What `set_download_directory` was last asked for, if anything. + download_dir_set: std::sync::Mutex>, + /// What `set_shared_directories` was last asked for, if anything. + shares_set: std::sync::Mutex>>, } fn queued(username: &str, filename: &str) -> soulseek_rs::types::Download { @@ -468,8 +479,17 @@ mod tests { } fn set_shared_directories( &self, - _directories: Vec, + directories: Vec, ) -> soulseek_rs::Result<()> { + *self.shares_set.lock().expect("not poisoned") = Some(directories); + Ok(()) + } + fn set_download_directory( + &self, + directory: String, + ) -> soulseek_rs::Result<()> { + *self.download_dir_set.lock().expect("not poisoned") = + Some(directory); Ok(()) } } @@ -509,6 +529,7 @@ mod tests { 1, Duration::from_secs(1), None, + None, ) } @@ -710,4 +731,109 @@ mod tests { // record, so nothing is invented. assert!(tui(Vec::new()).state.messages.is_empty()); } + + /// Attached, both folders in the settings form name paths on the + /// *daemon's* machine: they go over the socket exactly as typed, and + /// nothing is created or validated against this machine's filesystem. + #[test] + fn applying_settings_attached_pushes_both_folders_to_the_daemon() { + let session = Arc::new(TalkativeSession { + shared: true, + ..TalkativeSession::default() + }); + let scratch = tempfile::tempdir().expect("a temp dir"); + let config_path = scratch.path().join("config.toml"); + let mut tui = MainTui::new( + session.clone(), + "/daemon/old".to_string(), + 1, + Duration::from_secs(1), + None, + Some(config_path.clone()), + ); + + let daemon_only = scratch.path().join("only-on-the-daemon"); + let daemon_dir = daemon_only.to_str().expect("utf-8").to_string(); + tui.open_settings(); + { + let settings = tui.state.settings.as_mut().expect("open"); + settings.download_dir.clone_from(&daemon_dir); + settings.share_dirs = vec!["/not-here-either".to_string()]; + } + tui.apply_settings(); + + assert_eq!( + *session.download_dir_set.lock().expect("not poisoned"), + Some(daemon_dir.clone()), + "the folder is pushed to the daemon" + ); + assert_eq!( + *session.shares_set.lock().expect("not poisoned"), + Some(vec!["/not-here-either".to_string()]), + "shares go over as typed — the daemon's filesystem judges them" + ); + assert!( + !daemon_only.exists(), + "no directory is created on this machine for a daemon's path" + ); + assert_eq!(tui.download_dir, daemon_dir); + + let saved = crate::persist::config::FileConfig::load(&config_path) + .expect("config readable"); + assert_eq!( + saved.download_dir.as_deref(), + Some(daemon_dir.as_str()), + "the change still lands in config.toml for the next restart" + ); + } + + /// Locally the same form works on this machine: the folder is created + /// here and share paths that do not exist here are dropped before the + /// session hears about them. + #[test] + fn applying_settings_locally_stays_on_this_machine() { + let session = Arc::new(TalkativeSession { + shared: false, + ..TalkativeSession::default() + }); + let scratch = tempfile::tempdir().expect("a temp dir"); + let config_path = scratch.path().join("config.toml"); + let share = scratch.path().join("music"); + std::fs::create_dir_all(&share).expect("share dir"); + let mut tui = MainTui::new( + session.clone(), + "/tmp".to_string(), + 1, + Duration::from_secs(1), + None, + Some(config_path), + ); + + let downloads = scratch.path().join("downloads"); + let downloads_dir = downloads.to_str().expect("utf-8").to_string(); + tui.open_settings(); + { + let settings = tui.state.settings.as_mut().expect("open"); + settings.download_dir.clone_from(&downloads_dir); + settings.share_dirs = vec![ + share.to_str().expect("utf-8").to_string(), + "/definitely-not-a-real-share".to_string(), + ]; + } + tui.apply_settings(); + + // The seam is told in both modes; a local session accepts and takes + // the directory per download instead of storing it. + assert_eq!( + *session.download_dir_set.lock().expect("not poisoned"), + Some(downloads_dir.clone()) + ); + assert_eq!( + *session.shares_set.lock().expect("not poisoned"), + Some(vec![share.to_str().expect("utf-8").to_string()]), + "paths missing on this machine are dropped before applying" + ); + assert!(downloads.is_dir(), "the folder is created here"); + assert_eq!(tui.download_dir, downloads_dir); + } } diff --git a/soulseek-rs/src/ui/main_tui/settings.rs b/soulseek-rs/src/ui/main_tui/settings.rs index e50152c..42c60a3 100644 --- a/soulseek-rs/src/ui/main_tui/settings.rs +++ b/soulseek-rs/src/ui/main_tui/settings.rs @@ -1,5 +1,6 @@ -//! Settings popup behavior: open, key handling, and applying changes -//! (persist to config.toml + live share rescan on the client). +//! Settings popup behavior: open, key handling, and applying changes — +//! persisted to config.toml, applied live to the session (pushed over the +//! socket when the session is a daemon's). use super::MainTui; use crate::models::{SettingsAction, SettingsState}; @@ -25,38 +26,68 @@ impl MainTui { } } - /// Persist the edited settings and apply the share set live. - fn apply_settings(&mut self) { + /// Persist the edited settings and apply them live. + /// + /// Attached to a daemon, both folders name paths on the *daemon's* + /// machine: they are pushed over the socket and validated there, and no + /// directory is created on this one. Locally they are this machine's and + /// are validated here. + pub(super) fn apply_settings(&mut self) { let Some(settings) = self.state.settings.as_ref() else { return; }; let download_dir = settings.download_dir.clone(); let share_dirs = settings.share_dirs.clone(); - if let Err(e) = std::fs::create_dir_all(&download_dir) { - self.set_settings_status(format!( - "Cannot create {download_dir}: {e}" - )); - return; - } - self.download_dir.clone_from(&download_dir); - - // Validate the share paths (tilde-expand, must exist) and apply. - let valid = crate::directories::resolve_shared_directories(&share_dirs); - let dropped = share_dirs.len() - valid.len(); - let mut status = match self.client.set_shared_directories(valid) { - Ok(()) if dropped > 0 => { - format!("Applied ({dropped} invalid path(s) ignored)") + let shares = if self.client.daemon_endpoint().is_some() { + share_dirs.clone() + } else { + if let Err(e) = std::fs::create_dir_all(&download_dir) { + self.set_settings_status(format!( + "Cannot create {download_dir}: {e}" + )); + return; } - Ok(()) => format!("Applied · sharing {}", self.share_counts()), - Err(e) => format!("Could not apply shares: {e}"), + // Validate the share paths (tilde-expand, must exist). + crate::directories::resolve_shared_directories(&share_dirs) }; + let dropped = share_dirs.len() - shares.len(); - // Persist to config.toml so the change survives a restart. - if let Some(path) = crate::persist::paths::config_file() { + // Two independent applies: a failure of one must not misreport or + // silently discard the other, so both run and both are told. + let mut refused = Vec::new(); + let dir_applied = + match self.client.set_download_directory(download_dir.clone()) { + Ok(()) => true, + Err(e) => { + refused.push(format!("download folder: {e}")); + false + } + }; + if let Err(e) = self.client.set_shared_directories(shares) { + refused.push(format!("shares: {e}")); + } + + let mut status = if !refused.is_empty() { + format!("Could not apply {}", refused.join("; ")) + } else if dropped > 0 { + format!("Applied ({dropped} invalid path(s) ignored)") + } else { + format!("Applied · sharing {}", self.share_counts()) + }; + if dir_applied { + self.download_dir.clone_from(&download_dir); + } + + // Persist to config.toml so the change survives a restart. What was + // refused live is not written either: the file must not promise a + // folder the session never accepted. + if let Some(path) = self.config_path.clone() { let result = crate::persist::config::FileConfig::load(&path) .and_then(|mut config| { - config.download_dir = Some(download_dir); + if dir_applied { + config.download_dir = Some(download_dir); + } // The singular `shared_dir` is merged with `shared_dirs` on // load, so leaving it would resurrect a removed folder. config.shared_dir = None; diff --git a/soulseek-rs/tests/cli_e2e.rs b/soulseek-rs/tests/cli_e2e.rs index f1b9d27..5c8fd4e 100644 --- a/soulseek-rs/tests/cli_e2e.rs +++ b/soulseek-rs/tests/cli_e2e.rs @@ -126,6 +126,12 @@ fn records(output: &Output) -> Vec { .collect() } +/// The first record of a `--json` run, parsed. +fn json_record(output: &Output) -> serde_json::Value { + serde_json::from_str(records(output).first().expect("one record")) + .expect("valid JSON") +} + // --------------------------------------------------------------------------- // No server required // --------------------------------------------------------------------------- @@ -3462,3 +3468,335 @@ fn adding_a_share_takes_effect_on_a_running_daemon() { "and should have indexed what is in it: {after}" ); } + +/// Setting the download folder reaches a daemon that is already running. +/// +/// Locally `config set download_dir` means "from the next run on". A daemon +/// *is* that next run and it is already going, so without pushing the change +/// every transfer would keep landing in the old folder while the config file +/// claims otherwise — the trap `shares add` already closes. +#[test] +fn setting_the_download_folder_takes_effect_on_a_running_daemon() { + if mode() != Mode::Daemon { + println!( + "skipped: there is no running session to update in local mode" + ); + return; + } + let server = server_or_skip!(); + let user = "cli_e2e_live_dl_dir"; + let socket = server.daemon_state(user).join("daemon.sock"); + let at = socket.display().to_string(); + let config = Scratch::new("live-dl-config"); + let config_file = config.path().join("config.toml"); + let moved = Scratch::new("live-dl-target"); + + let set = run(&[ + "--config", + config_file.to_str().expect("utf-8 path"), + "--daemon", + &at, + "config", + "set", + "download_dir", + &moved.display(), + ]); + assert_eq!(code(&set), EXIT_OK, "stderr: {}", stderr(&set)); + + // The daemon reports the new folder as its own... + let status = + run(&["--no-config", "--json", "--daemon", &at, "daemon", "status"]); + assert_eq!(code(&status), EXIT_OK, "stderr: {}", stderr(&status)); + assert_eq!( + json_record(&status)["download_dir"], + moved.display(), + "status must name the folder transfers will now land in" + ); + + // `config set shared_dirs` reaches the same running daemon: the folder + // is indexed now, not at the next restart. + let offered = Scratch::new("live-dl-offered"); + std::fs::write(offered.path().join("cli_probe_offered.bin"), probe_bytes()) + .expect("share file"); + let set_shares = run(&[ + "--config", + config_file.to_str().expect("utf-8 path"), + "--daemon", + &at, + "config", + "set", + "shared_dirs", + &offered.display(), + ]); + assert_eq!( + code(&set_shares), + EXIT_OK, + "stderr: {}", + stderr(&set_shares) + ); + let listing = + run(&["--no-config", "--json", "--daemon", &at, "shares", "status"]); + assert_eq!(code(&listing), EXIT_OK, "stderr: {}", stderr(&listing)); + let indexed = json_record(&listing); + assert!( + indexed["directories"] + .as_array() + .expect("directories is an array") + .iter() + .any(|d| d == &offered.display()), + "the daemon should serve the folder the config now names: {indexed}" + ); + + // ...and the next transfer actually lands there. + let share = Scratch::new("live-dl-share"); + std::fs::write(share.path().join("cli_probe_moved.bin"), probe_bytes()) + .expect("share file"); + let _sharer = server.client("cli_e2e_sharer_moved", vec![share.display()]); + settle(); + + let fetched = run(&[ + "--no-config", + "--quiet", + "--daemon", + &at, + "get", + "cli_probe_moved", + "--timeout", + "40", + ]); + assert_eq!(code(&fetched), EXIT_OK, "stderr: {}", stderr(&fetched)); + let reported = records(&fetched); + assert_eq!(reported.len(), 1, "one file, one record"); + assert!( + reported[0].trim().starts_with(&moved.display()), + "the bytes must land in the folder that was just set, got {}", + reported[0] + ); + assert_eq!( + std::fs::read(reported[0].trim()).expect("downloaded file"), + probe_bytes() + ); +} + +/// The configured `download_dir` is where a download lands when no flag names +/// one — the local half of the promise the daemon test above makes. +#[test] +fn a_download_lands_in_the_folder_the_config_names() { + if mode() != Mode::Local { + println!("skipped: the daemon's folder wins in daemon mode"); + return; + } + let server = server_or_skip!(); + let share = Scratch::new("share"); + let target = Scratch::new("configured-downloads"); + std::fs::write(share.path().join("cli_probe_cfgdir.bin"), probe_bytes()) + .expect("share file"); + let _sharer = server.client("cli_e2e_sharer_cfg", vec![share.display()]); + settle(); + + let config = Scratch::new("dl-dir-config"); + let config_file = config.path().join("config.toml"); + let set = run(&[ + "--config", + config_file.to_str().expect("utf-8 path"), + "config", + "set", + "download_dir", + &target.display(), + ]); + assert_eq!(code(&set), EXIT_OK, "stderr: {}", stderr(&set)); + + let fetched = cli_with_config( + &server, + "cli_e2e_cfg_fetcher", + &config_file, + &["get", "cli_probe_cfgdir", "--timeout", "40"], + ); + assert_eq!(code(&fetched), EXIT_OK, "stderr: {}", stderr(&fetched)); + let reported = records(&fetched); + assert_eq!(reported.len(), 1, "one file, one record"); + assert!( + reported[0].trim().starts_with(&target.display()), + "the config file names where downloads go, got {}", + reported[0] + ); +} + +/// With nothing running, the daemon control commands say so and exit with the +/// connection code — a script must be able to branch on "is it up?". +#[test] +fn daemon_status_and_stop_without_a_daemon_are_connection_errors() { + for command in ["status", "stop"] { + let output = run(&["--no-config", "daemon", command]); + assert_eq!( + code(&output), + EXIT_CONNECTION, + "daemon {command}: {}", + stderr(&output) + ); + assert!( + stderr(&output).contains("no daemon is running"), + "daemon {command} should say what is wrong, got: {}", + stderr(&output) + ); + assert!(stdout(&output).is_empty(), "stdout must stay data-only"); + } +} + +/// `--sort size` leads with the largest file, which is what a script that +/// wants "the full album rip, not the sample" branches on. +#[test] +fn search_sorted_by_size_leads_with_the_largest_file() { + let server = server_or_skip!(); + let share = Scratch::new("share"); + std::fs::write(share.path().join("cli_probe_sort_small.bin"), [7u8; 64]) + .expect("share file"); + std::fs::write( + share.path().join("cli_probe_sort_large.bin"), + vec![7u8; 4096], + ) + .expect("share file"); + let _sharer = server.client("cli_e2e_sharer_sort", vec![share.display()]); + settle(); + + let output = cli( + &server, + "cli_e2e_sorter", + &[ + "--search-timeout", + "5", + "search", + "cli_probe_sort", + "--sort", + "size", + ], + ); + assert_eq!(code(&output), EXIT_OK, "stderr: {}", stderr(&output)); + let reported = records(&output); + assert!(reported.len() >= 2, "both files answer, got {reported:?}"); + assert!( + reported[0].contains("cli_probe_sort_large"), + "largest first under --sort size, got {reported:?}" + ); + + // `--limit 1` keeps only the leader. + let capped = cli( + &server, + "cli_e2e_sorter", + &[ + "--search-timeout", + "5", + "search", + "cli_probe_sort", + "--sort", + "size", + "--limit", + "1", + ], + ); + assert_eq!(code(&capped), EXIT_OK, "stderr: {}", stderr(&capped)); + let capped = records(&capped); + assert_eq!(capped.len(), 1, "--limit 1 keeps one record"); + assert!( + capped[0].contains("cli_probe_sort_large"), + "and it is the ranked leader, got {capped:?}" + ); +} + +/// `--pick first` takes the first arrival without waiting out the window. +#[test] +fn get_pick_first_downloads_the_first_arrival() { + let server = server_or_skip!(); + let share = Scratch::new("share"); + let target = Scratch::new("downloads"); + std::fs::write(share.path().join("cli_probe_first.bin"), probe_bytes()) + .expect("share file"); + let _sharer = server.client("cli_e2e_sharer_first", vec![share.display()]); + settle(); + + let output = cli( + &server, + "cli_e2e_first_picker", + &[ + "--download-dir", + &target.display(), + "get", + "cli_probe_first", + "--pick", + "first", + "--timeout", + "40", + ], + ); + assert_eq!(code(&output), EXIT_OK, "stderr: {}", stderr(&output)); + let reported = records(&output); + assert_eq!(reported.len(), 1, "--pick first downloads one file"); + assert_eq!( + std::fs::read(reported[0].trim()).expect("downloaded file"), + probe_bytes() + ); +} + +/// The password can come from a command, `pass`-manager style: nothing +/// sensitive in argv, nothing in the environment. +#[test] +fn the_password_can_come_from_a_command() { + let server = server_or_skip!(); + let _owner = server.client("cli_e2e_cmd_pw", Vec::new()); + + let output = run(&[ + "--no-config", + "--quiet", + "--server", + &server.address(), + "--username", + "cli_e2e_cmd_pw", + "--password-cmd", + "echo pw", + "--no-listener", + "whoami", + ]); + assert_eq!(code(&output), EXIT_OK, "stderr: {}", stderr(&output)); + assert!( + records(&output)[0].starts_with("cli_e2e_cmd_pw\t"), + "got {:?}", + records(&output) + ); +} + +/// `--log-file` keeps diagnostics out of stderr and in a file a bug report +/// can attach. +#[test] +fn a_log_file_captures_what_verbose_would_print() { + let server = server_or_skip!(); + let _owner = server.client("cli_e2e_logged", Vec::new()); + let logs = Scratch::new("logs"); + let log_file = logs.path().join("run.log"); + let output = run(&[ + "--no-config", + "--quiet", + "-vvv", + "--log-file", + log_file.to_str().expect("utf-8 path"), + "--username", + "cli_e2e_logged", + "--password", + "pw", + "--server", + &server.address(), + "--no-listener", + "whoami", + ]); + assert_eq!(code(&output), EXIT_OK, "stderr: {}", stderr(&output)); + let written = + std::fs::read_to_string(&log_file).expect("the log file is created"); + assert!( + !written.trim().is_empty(), + "-vvv must leave diagnostics in the file" + ); + assert!( + !stderr(&output).contains("DEBUG"), + "with a log file, diagnostics stay out of stderr: {}", + stderr(&output) + ); +} diff --git a/soulseek-rs/tests/daemon_protocol_e2e.rs b/soulseek-rs/tests/daemon_protocol_e2e.rs index eb586aa..dd23385 100644 --- a/soulseek-rs/tests/daemon_protocol_e2e.rs +++ b/soulseek-rs/tests/daemon_protocol_e2e.rs @@ -48,18 +48,25 @@ struct Daemon { _server: Soulfind, } +/// The binary with a scrubbed environment: no test may see the developer's +/// real daemon, and `state` decides which one it does see. +fn cli_command(state: &Path) -> Command { + let mut command = Command::new(BIN); + for name in CLI_ENV_VARS { + command.env_remove(name); + } + command.env("SOULSEEK_STATE_DIR", state); + command.current_dir(std::env::temp_dir()); + command +} + impl Daemon { fn start() -> Option { let server = Soulfind::start()?; let state = scratch("daemon-proto"); let port = free_port()?; - let mut command = Command::new(BIN); - for name in CLI_ENV_VARS { - command.env_remove(name); - } - command.env("SOULSEEK_STATE_DIR", &state); - command.current_dir(std::env::temp_dir()); + let mut command = cli_command(&state); command.args([ "--no-config", "--server", @@ -75,6 +82,10 @@ impl Daemon { "daemon", "--bind", &format!("127.0.0.1:{port}"), + // Exercised on every boot: the limit must parse and apply + // without disturbing anything else the suite observes. + "--upload-slots", + "8", ]); command.stdout(Stdio::null()).stderr(Stdio::null()); let child = command.spawn().ok()?; @@ -521,13 +532,8 @@ fn a_config_driven_remote_client_reports_the_daemons_world() { ) .expect("config file"); - let mut command = Command::new(BIN); - for name in CLI_ENV_VARS { - command.env_remove(name); - } + let mut command = cli_command(&state); command.env("SOULSEEK_CONFIG_DIR", &config); - command.env("SOULSEEK_STATE_DIR", &state); - command.current_dir(std::env::temp_dir()); command.args(["--json", "whoami"]); let output = command.output().expect("the binary should run"); @@ -793,3 +799,157 @@ fn a_client_that_attaches_later_sees_what_is_already_there() { ); } } + +/// The download folder is a session setting, settable over the wire. +/// +/// `download.start` deliberately refuses a per-transfer destination, so +/// `download.set_dir` is the only way a remote owner can move where bytes +/// land — and `daemon.status` has to report the move, because that status is +/// what every attached client trusts for "where did my file go". +#[test] +#[cfg(unix)] +fn the_download_folder_moves_when_asked_and_status_says_so() { + let daemon = daemon_or_skip!(); + let mut connection = daemon.connect_socket().expect("socket"); + connection.authenticate(None).expect("an answer"); + + let moved = scratch("moved-downloads").join("deeper"); + let reply = connection + .call(&format!( + r#"{{"jsonrpc":"2.0","id":2,"method":"download.set_dir","params":{{"directory":{}}}}}"#, + serde_json::json!(moved.to_str().expect("utf-8 path")) + )) + .expect("an answer"); + assert!(reply.contains(r#""ok":true"#), "got {reply}"); + assert!( + moved.is_dir(), + "the daemon creates the folder it will download into" + ); + + let status = connection + .call(r#"{"jsonrpc":"2.0","id":3,"method":"daemon.status"}"#) + .expect("an answer"); + let parsed: serde_json::Value = + serde_json::from_str(&status).expect("valid JSON"); + assert_eq!( + parsed["result"]["download_dir"], + moved.to_str().expect("utf-8 path"), + "status must name the folder transfers now land in: {status}" + ); + + // An empty path is refused as bad parameters, not applied as "here". + let refused = connection + .call( + r#"{"jsonrpc":"2.0","id":4,"method":"download.set_dir","params":{"directory":" "}}"#, + ) + .expect("an answer"); + assert!(refused.contains("-32602"), "got {refused}"); + let _ = std::fs::remove_dir_all(moved.parent().expect("parent")); +} + +/// A wishlist search must read as *running* to the other windows. +/// +/// The daemon can only tell a running search from a finished one by the start +/// time it recorded; a wishlist search that never records one renders as +/// completed everywhere the moment it goes out. +#[test] +#[cfg(unix)] +fn a_wishlist_search_is_reported_as_just_started() { + let daemon = daemon_or_skip!(); + let mut connection = daemon.connect_socket().expect("socket"); + connection.authenticate(None).expect("an answer"); + + let sent = connection + .call( + r#"{"jsonrpc":"2.0","id":2,"method":"search.wishlist","params":{"query":"wishlist_probe"}}"#, + ) + .expect("an answer"); + assert!(sent.contains(r#""ok":true"#), "got {sent}"); + + let listed = connection + .call(r#"{"jsonrpc":"2.0","id":3,"method":"search.list"}"#) + .expect("an answer"); + let parsed: serde_json::Value = + serde_json::from_str(&listed).expect("valid JSON"); + let age = parsed["result"]["searches"] + .as_array() + .expect("searches is an array") + .iter() + .find(|search| search["query"] == "wishlist_probe") + .and_then(|search| search["started_secs_ago"].as_u64()) + .expect("the wishlist search is listed with an age"); + assert!( + age < 60, + "a search sent moments ago must read as just started, got {age}s" + ); +} + +/// The upload-slot limit is retunable on a live session. +#[test] +fn the_upload_slot_limit_can_be_retuned_live() { + let daemon = daemon_or_skip!(); + let mut connection = daemon.connect_tcp().expect("tcp"); + connection + .authenticate(Some(&daemon.token())) + .expect("an answer"); + let reply = connection + .call( + r#"{"jsonrpc":"2.0","id":2,"method":"upload.slots","params":{"slots":3}}"#, + ) + .expect("an answer"); + assert!(reply.contains(r#""ok":true"#), "got {reply}"); +} + +/// A wrong or missing token is the CLI's problem to explain, not just the +/// wire's: the user typed `--daemon-token`, so the failure has to come back +/// as a connection error rather than a hang or a fallback to local defaults. +#[test] +fn the_cli_reports_a_bad_token_as_a_connection_error() { + let daemon = daemon_or_skip!(); + let state = scratch("cli-bad-token"); + let run = |token: Option<&str>| { + let mut command = cli_command(&state); + command.args([ + "--no-config", + "--daemon", + &format!("127.0.0.1:{}", daemon.port), + ]); + if let Some(token) = token { + command.args(["--daemon-token", token]); + } + command.arg("whoami"); + command.output().expect("the binary should run") + }; + + let wrong = run(Some(&"0".repeat(64))); + assert_eq!( + wrong.status.code(), + Some(3), + "a rejected token is a connection error: {}", + String::from_utf8_lossy(&wrong.stderr) + ); + assert!( + String::from_utf8_lossy(&wrong.stdout).trim().is_empty(), + "a failing command must write nothing to stdout" + ); + + // No token at all: this state dir never ran a daemon, so there is no + // cookie file to fall back on, and TCP without a token is a hard no. + let missing = run(None); + assert_eq!( + missing.status.code(), + Some(3), + "TCP with no token to offer is a connection error: {}", + String::from_utf8_lossy(&missing.stderr) + ); + + // The right token still works from the same machine state. + let right = run(Some(&daemon.token())); + assert_eq!( + right.status.code(), + Some(0), + "the real token must keep working: {}", + String::from_utf8_lossy(&right.stderr) + ); + let _ = std::fs::remove_dir_all(&state); +}