From d2599c82d3d28e159d3cef0b9a636ca496950ce1 Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Mon, 27 Jul 2026 12:46:25 +0200 Subject: [PATCH] fix(shares): let add and remove reach a running daemon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shares add wrote the config file and stopped there, so with a daemon running the folder stayed invisible to the network until the daemon restarted — the command reported success and nothing had happened. Locally that is the right meaning, because there is no session yet; a daemon is already the next run. Both commands now push the new list over shares.set. The e2e test was checked against a disabled push to confirm it detects the bug. --- docs/daemon-protocol.md | 5 +++ soulseek-rs/src/SKILL.md | 3 +- soulseek-rs/src/commands/settings.rs | 52 +++++++++++++++++++-- soulseek-rs/src/run.rs | 18 +++++++- soulseek-rs/tests/cli_e2e.rs | 67 ++++++++++++++++++++++++++++ 5 files changed, 138 insertions(+), 7 deletions(-) diff --git a/docs/daemon-protocol.md b/docs/daemon-protocol.md index 9875741..6b68b35 100644 --- a/docs/daemon-protocol.md +++ b/docs/daemon-protocol.md @@ -143,6 +143,11 @@ configuration and a client cannot name one — that would be an arbitrary write on the daemon's host. `daemon.status` reports the directory in use. There is no method to fetch the bytes over the control socket. +**Configuration is per machine, shares are per session.** A client's config +file is its own; changing it does not reach a daemon. The exception is +`shares.set`, which the CLI's `shares add`/`remove` call so a folder is served +without waiting for a restart. + **Several clients, no locking.** Last write wins. Two clients cancelling the same transfer is two hands on one keyboard, not an error. diff --git a/soulseek-rs/src/SKILL.md b/soulseek-rs/src/SKILL.md index cf3c6a6..5114215 100644 --- a/soulseek-rs/src/SKILL.md +++ b/soulseek-rs/src/SKILL.md @@ -120,7 +120,8 @@ clients never need it. **`shares list`** — one object per configured folder: `directory`, `usable`. **`shares add `** and **`shares remove `** — change the -config file; they print nothing. +config file; they print nothing. With a daemon running they also update it in +place, so the folder is on the network at once rather than after a restart. **`shares status`** and **`shares reindex`** — `folders`, `files`, `directories`, counted by logging in and scanning. diff --git a/soulseek-rs/src/commands/settings.rs b/soulseek-rs/src/commands/settings.rs index 7781b43..2b37669 100644 --- a/soulseek-rs/src/commands/settings.rs +++ b/soulseek-rs/src/commands/settings.rs @@ -5,11 +5,13 @@ //! what the next run — interactive or scripted — starts from. use super::{Ctx, Session}; +use crate::api::SessionApi; use crate::directories; use crate::output::{ CliError, CliResult, ConfigRecord, Out, ShareRecord, ShareStatusRecord, }; use crate::persist::config::FileConfig; +use crate::remote::{Endpoint, RemoteSession}; use std::path::PathBuf; /// Where a `shares`/`config` change is written, and what it starts from. @@ -46,7 +48,42 @@ pub fn shares_list(out: &Out, store: &Store, resolved: &[String]) -> CliResult { Ok(()) } -pub fn shares_add(out: &Out, store: &Store, directory: &str) -> CliResult { +/// Where a running daemon can be reached, so a share change takes effect now +/// rather than at its next restart. +pub struct LiveDaemon { + pub endpoint: Option, + pub token: Option, +} + +/// Tell a running daemon what this machine now shares. +/// +/// 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) { + 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)); + match outcome { + Ok(()) => out.status("the running daemon is re-indexing"), + // 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" + )), + } +} + +pub fn shares_add( + out: &Out, + store: &Store, + directory: &str, + daemon: &LiveDaemon, +) -> CliResult { // Refuse a folder that is not there: sharing it would silently index // nothing, and the mistake would only surface as "no one can find my // files" much later. @@ -63,14 +100,20 @@ pub fn shares_add(out: &Out, store: &Store, directory: &str) -> CliResult { return Ok(()); } dirs.push(directory.to_string()); - updated.shared_dirs = Some(dirs); + updated.shared_dirs = Some(dirs.clone()); updated.shared_dir = None; store.save(&updated)?; out.status(&format!("sharing {directory}")); + push_to_daemon(out, daemon, dirs); Ok(()) } -pub fn shares_remove(out: &Out, store: &Store, directory: &str) -> CliResult { +pub fn shares_remove( + out: &Out, + store: &Store, + directory: &str, + daemon: &LiveDaemon, +) -> CliResult { let mut dirs = share_list(&store.config); let before = dirs.len(); dirs.retain(|existing| existing != directory); @@ -81,10 +124,11 @@ pub fn shares_remove(out: &Out, store: &Store, directory: &str) -> CliResult { } let mut updated = store.config.clone(); - updated.shared_dirs = Some(dirs); + updated.shared_dirs = Some(dirs.clone()); updated.shared_dir = None; store.save(&updated)?; out.status(&format!("no longer sharing {directory}")); + push_to_daemon(out, daemon, dirs); Ok(()) } diff --git a/soulseek-rs/src/run.rs b/soulseek-rs/src/run.rs index 500971a..7a47c13 100644 --- a/soulseek-rs/src/run.rs +++ b/soulseek-rs/src/run.rs @@ -84,12 +84,18 @@ pub fn run(mut cli: Cli, out: &Out) -> CliResult { } Commands::Shares(SharesCommand::Add { ref directory }) => { return crate::commands::settings::shares_add( - out, &store, directory, + out, + &store, + directory, + &live_daemon(&cli), ); } Commands::Shares(SharesCommand::Remove { ref directory }) => { return crate::commands::settings::shares_remove( - out, &store, directory, + out, + &store, + directory, + &live_daemon(&cli), ); } Commands::Wish(WishCommand::Add { ref query }) => { @@ -243,6 +249,14 @@ fn context( }) } +/// The daemon a config change should also reach, if one is running. +fn live_daemon(cli: &Cli) -> crate::commands::settings::LiveDaemon { + crate::commands::settings::LiveDaemon { + endpoint: daemon_endpoint(cli), + token: cli.daemon_token.clone(), + } +} + /// Ask a daemon what it is, so this run describes the session it will actually /// use. A daemon that cannot be reached is left to `Session::open` to report, /// which produces a better message than anything this early probe could. diff --git a/soulseek-rs/tests/cli_e2e.rs b/soulseek-rs/tests/cli_e2e.rs index 904285d..f1b9d27 100644 --- a/soulseek-rs/tests/cli_e2e.rs +++ b/soulseek-rs/tests/cli_e2e.rs @@ -3395,3 +3395,70 @@ fn the_daemon_token_is_stable_across_reads() { "a restart must not lock existing clients out" ); } + +/// Adding a folder reaches a daemon that is already running. +/// +/// 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 +/// without pushing the change the command would look like it had worked while +/// the network still saw nothing. +#[test] +fn adding_a_share_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_shares"; + let socket = server.daemon_state(user).join("daemon.sock"); + let at = socket.display().to_string(); + let config = Scratch::new("live-shares-config"); + let config_file = config.path().join("config.toml"); + let share = Scratch::new("live-shares"); + std::fs::write(share.path().join("cli_probe_live.bin"), probe_bytes()) + .expect("share file"); + + let before = + run(&["--no-config", "--json", "--daemon", &at, "shares", "status"]); + assert_eq!(code(&before), EXIT_OK, "stderr: {}", stderr(&before)); + + let added = run(&[ + "--config", + config_file.to_str().expect("utf-8 path"), + "--daemon", + &at, + "shares", + "add", + &share.display(), + ]); + assert_eq!(code(&added), EXIT_OK, "stderr: {}", stderr(&added)); + + // The daemon re-indexes on the way through, so the folder is visible to + // the network now rather than after a restart. + let after: serde_json::Value = serde_json::from_str( + records(&run(&[ + "--no-config", + "--json", + "--daemon", + &at, + "shares", + "status", + ])) + .first() + .expect("one record"), + ) + .expect("valid JSON"); + let listed = after["directories"] + .as_array() + .expect("directories is an array"); + assert!( + listed.iter().any(|d| d == &share.display()), + "the daemon should be sharing the folder that was just added: {after}" + ); + assert!( + after["files"].as_u64().is_some_and(|files| files >= 1), + "and should have indexed what is in it: {after}" + ); +}