From 5a89a14d5f90adacc1377c389153f28036bdf568 Mon Sep 17 00:00:00 2001 From: ashish-kumar-dash Date: Tue, 15 Sep 2026 21:50:03 +0530 Subject: [PATCH 1/2] refactor(smitebot): deduplicate campaign load into CampaignState::load_campaign stop, status, and reproduce each repeated the same 18-line block: resolve runs_dir, build the state path, load state, and log a not-found hint on failure. Extract it into CampaignState::load_campaign(id) -> Option<(Self, PathBuf)>, collapsing each call site to one line. --- smitebot/src/commands/corpus.rs | 40 ++++++++++-------------------- smitebot/src/commands/reproduce.rs | 15 ++--------- smitebot/src/commands/status.rs | 15 ++--------- smitebot/src/commands/stop.rs | 15 ++--------- smitebot/src/state.rs | 18 ++++++++++++++ 5 files changed, 37 insertions(+), 66 deletions(-) diff --git a/smitebot/src/commands/corpus.rs b/smitebot/src/commands/corpus.rs index 93fdcdd7..1da0fb6d 100644 --- a/smitebot/src/commands/corpus.rs +++ b/smitebot/src/commands/corpus.rs @@ -70,38 +70,20 @@ impl CorpusCommand { } } -/// Loads the campaign state for `campaign_id`, logging a not-found hint on error. -fn load_campaign(runs_dir: &Path, campaign_id: &str) -> Option { - let state_path = runs_dir.join(campaign_id).join("state.json"); - match CampaignState::load(&state_path) { - Ok(state) => Some(state), - Err(e) => { - log::error!("{e}"); - log::error!( - "campaign '{campaign_id}' not found; list campaigns with: ls {}", - runs_dir.display() - ); - None - } - } -} - /// Loads every campaign's state, then merges their runner queues into `output`. /// /// All states are loaded before any file is written, so a bad campaign ID fails /// before the output directory is touched rather than leaving a partial merge. fn execute_merge(args: &MergeArgs) -> bool { - let Some(runs_dir) = CampaignState::runs_dir() else { - log::error!("unable to determine home directory"); - return false; - }; - let mut states = Vec::with_capacity(args.campaign_ids.len()); for campaign_id in &args.campaign_ids { - let Some(state) = load_campaign(&runs_dir, campaign_id) else { - return false; - }; - states.push(state); + match CampaignState::load_campaign(campaign_id) { + Ok((state, _)) => states.push(state), + Err(e) => { + log::error!("{e}"); + return false; + } + } } if output_dir_occupied(&args.output) { @@ -222,8 +204,12 @@ fn execute_minimize(args: &MinimizeArgs) -> bool { return false; }; - let Some(state) = load_campaign(&runs_dir, &args.campaign_id) else { - return false; + let state = match CampaignState::load_campaign(&args.campaign_id) { + Ok((state, _)) => state, + Err(e) => { + log::error!("{e}"); + return false; + } }; // The --aflpp-path flag overrides the path recorded at campaign start, for when diff --git a/smitebot/src/commands/reproduce.rs b/smitebot/src/commands/reproduce.rs index 15adb78e..cc0a1c50 100644 --- a/smitebot/src/commands/reproduce.rs +++ b/smitebot/src/commands/reproduce.rs @@ -36,21 +36,10 @@ impl ReproduceCommand { /// `false` only on an operational failure: unknown campaign, missing input, /// missing image, or a Docker spawn error. pub fn execute(args: &ReproduceArgs) -> bool { - let Some(runs_dir) = CampaignState::runs_dir() else { - log::error!("unable to determine home directory"); - return false; - }; - - let state_path = runs_dir.join(&args.campaign_id).join("state.json"); - let state = match CampaignState::load(&state_path) { - Ok(s) => s, + let (state, _) = match CampaignState::load_campaign(&args.campaign_id) { + Ok(x) => x, Err(e) => { log::error!("{e}"); - log::error!( - "campaign '{}' not found; list campaigns with: ls {}", - args.campaign_id, - runs_dir.display() - ); return false; } }; diff --git a/smitebot/src/commands/status.rs b/smitebot/src/commands/status.rs index 000630c3..1c19ba41 100644 --- a/smitebot/src/commands/status.rs +++ b/smitebot/src/commands/status.rs @@ -44,21 +44,10 @@ impl StatusCommand { /// Reports the status of a campaign, either as a one-shot summary or by /// attaching to its live tmux dashboard. pub fn execute(args: &StatusArgs) -> bool { - let Some(runs_dir) = CampaignState::runs_dir() else { - log::error!("unable to determine home directory"); - return false; - }; - let state_path = runs_dir.join(&args.campaign_id).join("state.json"); - - let state = match CampaignState::load(&state_path) { - Ok(state) => state, + let (state, _) = match CampaignState::load_campaign(&args.campaign_id) { + Ok(x) => x, Err(e) => { log::error!("{e}"); - log::error!( - "campaign '{}' not found; list campaigns with: ls {}", - args.campaign_id, - runs_dir.display() - ); return false; } }; diff --git a/smitebot/src/commands/stop.rs b/smitebot/src/commands/stop.rs index 0039c788..b4306e73 100644 --- a/smitebot/src/commands/stop.rs +++ b/smitebot/src/commands/stop.rs @@ -41,21 +41,10 @@ impl StopCommand { /// Stops a campaign: reaps its runner process groups, tears down the tmux /// session, and records the stop in state.json. pub fn execute(args: &StopArgs) -> bool { - let Some(runs_dir) = CampaignState::runs_dir() else { - log::error!("unable to determine home directory"); - return false; - }; - let state_path = runs_dir.join(&args.campaign_id).join("state.json"); - - let mut state = match CampaignState::load(&state_path) { - Ok(state) => state, + let (mut state, state_path) = match CampaignState::load_campaign(&args.campaign_id) { + Ok(x) => x, Err(e) => { log::error!("{e}"); - log::error!( - "campaign '{}' not found; list campaigns with: ls {}", - args.campaign_id, - runs_dir.display() - ); return false; } }; diff --git a/smitebot/src/state.rs b/smitebot/src/state.rs index 85b36cf1..f3adda6f 100644 --- a/smitebot/src/state.rs +++ b/smitebot/src/state.rs @@ -102,6 +102,8 @@ pub enum StateError { path: PathBuf, source: serde_json::Error, }, + #[error("unable to determine home directory")] + HomeDir, } impl CampaignState { @@ -174,6 +176,17 @@ impl CampaignState { source, }) } + + /// Loads state for campaign `id` from `~/.smitebot/runs//state.json`. + /// + /// Returns the state and its path (the path is needed by callers that + /// mutate and re-save state, e.g. `stop`). + pub fn load_campaign(id: &str) -> Result<(Self, PathBuf), StateError> { + let runs_dir = Self::runs_dir().ok_or(StateError::HomeDir)?; + let state_path = runs_dir.join(id).join("state.json"); + let state = Self::load(&state_path)?; + Ok((state, state_path)) + } } #[cfg(test)] @@ -325,6 +338,11 @@ sharedir = "/tmp/nyx" assert_eq!(loaded.stop_time, Some(1_749_469_200)); } + #[test] + fn load_campaign_returns_err_for_missing_campaign() { + assert!(CampaignState::load_campaign("nonexistent-campaign-xkcd-abc123").is_err()); + } + #[test] fn load_reports_missing_file() { let err = CampaignState::load(Path::new("/no/such/state.json")).unwrap_err(); From 693081bacb62b39f32451af3031db658d4b44032 Mon Sep 17 00:00:00 2001 From: ashish-kumar-dash Date: Wed, 23 Sep 2026 15:11:35 +0530 Subject: [PATCH 2/2] changes --- smitebot/src/commands/corpus.rs | 4 ++-- smitebot/src/commands/reproduce.rs | 4 ++-- smitebot/src/commands/start.rs | 32 ++++++++++-------------------- smitebot/src/commands/status.rs | 4 ++-- smitebot/src/commands/stop.rs | 6 +++--- smitebot/src/state.rs | 19 +++++++++--------- 6 files changed, 29 insertions(+), 40 deletions(-) diff --git a/smitebot/src/commands/corpus.rs b/smitebot/src/commands/corpus.rs index 1da0fb6d..bab3a0b5 100644 --- a/smitebot/src/commands/corpus.rs +++ b/smitebot/src/commands/corpus.rs @@ -78,7 +78,7 @@ fn execute_merge(args: &MergeArgs) -> bool { let mut states = Vec::with_capacity(args.campaign_ids.len()); for campaign_id in &args.campaign_ids { match CampaignState::load_campaign(campaign_id) { - Ok((state, _)) => states.push(state), + Ok(state) => states.push(state), Err(e) => { log::error!("{e}"); return false; @@ -205,7 +205,7 @@ fn execute_minimize(args: &MinimizeArgs) -> bool { }; let state = match CampaignState::load_campaign(&args.campaign_id) { - Ok((state, _)) => state, + Ok(s) => s, Err(e) => { log::error!("{e}"); return false; diff --git a/smitebot/src/commands/reproduce.rs b/smitebot/src/commands/reproduce.rs index cc0a1c50..8fa8745b 100644 --- a/smitebot/src/commands/reproduce.rs +++ b/smitebot/src/commands/reproduce.rs @@ -36,8 +36,8 @@ impl ReproduceCommand { /// `false` only on an operational failure: unknown campaign, missing input, /// missing image, or a Docker spawn error. pub fn execute(args: &ReproduceArgs) -> bool { - let (state, _) = match CampaignState::load_campaign(&args.campaign_id) { - Ok(x) => x, + let state = match CampaignState::load_campaign(&args.campaign_id) { + Ok(s) => s, Err(e) => { log::error!("{e}"); return false; diff --git a/smitebot/src/commands/start.rs b/smitebot/src/commands/start.rs index d6e21731..3216ab31 100644 --- a/smitebot/src/commands/start.rs +++ b/smitebot/src/commands/start.rs @@ -118,13 +118,6 @@ impl StartCommand { } }; - let Some(runs_dir) = CampaignState::runs_dir() else { - log::error!("unable to determine home directory"); - return false; - }; - - let state_path = runs_dir.join(&campaign_id).join("state.json"); - let Some(git_hash) = smite_git_hash(&config.smite_dir) else { log::error!("could not determine smite git hash"); return false; @@ -143,21 +136,21 @@ impl StartCommand { tmux_session, ); - if let Err(e) = state.save(&state_path) { + if let Err(e) = state.save_campaign() { log::error!("{e}"); return false; } - if !launch_runners(&config, &seed_dir, &mut state, &state_path) { + if !launch_runners(&config, &seed_dir, &mut state) { return false; } - if let Err(e) = state.save(&state_path) { + if let Err(e) = state.save_campaign() { log::error!("{e}"); return false; } log::info!("campaign {} is running", state.id); - log::info!("state saved to {}", state_path.display()); + log::info!("state saved to ~/.smitebot/runs/{}/state.json", state.id); log::info!("attaching to tmux session '{}'", state.tmux_session); if let Err(e) = tmux::attach(&state.tmux_session) { @@ -170,12 +163,7 @@ impl StartCommand { /// Spawns all runners inside a tmux session, verifies they produce /// `fuzzer_stats`, and updates campaign state with PIDs. -fn launch_runners( - config: &CampaignConfig, - seed_dir: &Path, - state: &mut CampaignState, - state_path: &Path, -) -> bool { +fn launch_runners(config: &CampaignConfig, seed_dir: &Path, state: &mut CampaignState) -> bool { let session = &state.tmux_session; log::info!( "starting {} runners in tmux session '{session}'", @@ -198,7 +186,7 @@ fn launch_runners( if let Err(e) = result { log::error!("failed to create tmux window for runner {id}: {e}"); state.runners = runners; - fail_campaign(state, state_path); + fail_campaign(state); return false; } @@ -207,7 +195,7 @@ fn launch_runners( state.runners = runners; - if let Err(e) = state.save(state_path) { + if let Err(e) = state.save_campaign() { log::warn!("failed to save state: {e}"); } @@ -221,7 +209,7 @@ fn launch_runners( // verify_startup has already logged the specific reason per runner // (window died, or ceiling reached). log::error!("one or more runners failed to start"); - fail_campaign(state, state_path); + fail_campaign(state); return false; } @@ -231,7 +219,7 @@ fn launch_runners( /// Marks the campaign as failed, logs instructions to inspect the tmux session, /// and persists the updated state. -fn fail_campaign(state: &mut CampaignState, state_path: &Path) { +fn fail_campaign(state: &mut CampaignState) { if !state.runners.is_empty() { log::info!( "inspect tmux session '{}' for error output, \ @@ -241,7 +229,7 @@ fn fail_campaign(state: &mut CampaignState, state_path: &Path) { ); } state.status = Status::Failed; - if let Err(e) = state.save(state_path) { + if let Err(e) = state.save_campaign() { log::warn!("failed to save state: {e}"); } } diff --git a/smitebot/src/commands/status.rs b/smitebot/src/commands/status.rs index 1c19ba41..a46b7050 100644 --- a/smitebot/src/commands/status.rs +++ b/smitebot/src/commands/status.rs @@ -44,8 +44,8 @@ impl StatusCommand { /// Reports the status of a campaign, either as a one-shot summary or by /// attaching to its live tmux dashboard. pub fn execute(args: &StatusArgs) -> bool { - let (state, _) = match CampaignState::load_campaign(&args.campaign_id) { - Ok(x) => x, + let state = match CampaignState::load_campaign(&args.campaign_id) { + Ok(s) => s, Err(e) => { log::error!("{e}"); return false; diff --git a/smitebot/src/commands/stop.rs b/smitebot/src/commands/stop.rs index b4306e73..ef6a72b3 100644 --- a/smitebot/src/commands/stop.rs +++ b/smitebot/src/commands/stop.rs @@ -41,8 +41,8 @@ impl StopCommand { /// Stops a campaign: reaps its runner process groups, tears down the tmux /// session, and records the stop in state.json. pub fn execute(args: &StopArgs) -> bool { - let (mut state, state_path) = match CampaignState::load_campaign(&args.campaign_id) { - Ok(x) => x, + let mut state = match CampaignState::load_campaign(&args.campaign_id) { + Ok(s) => s, Err(e) => { log::error!("{e}"); return false; @@ -63,7 +63,7 @@ impl StopCommand { state.status = Status::Stopped; state.stop_time = Some(utils::epoch_secs()); - if let Err(e) = state.save(&state_path) { + if let Err(e) = state.save_campaign() { log::error!( "runners were reaped but recording the stop failed: {e}; \ campaign {} will still show as running in state.json", diff --git a/smitebot/src/state.rs b/smitebot/src/state.rs index f3adda6f..599f9515 100644 --- a/smitebot/src/state.rs +++ b/smitebot/src/state.rs @@ -143,7 +143,7 @@ impl CampaignState { /// Saves the campaign state as JSON, using an atomic write to prevent /// corruption if the process is interrupted. - pub fn save(&self, path: &Path) -> Result<(), StateError> { + fn save(&self, path: &Path) -> Result<(), StateError> { if let Some(parent) = path.parent() { fs::create_dir_all(parent).map_err(|source| StateError::CreateDir { path: parent.to_path_buf(), @@ -165,8 +165,14 @@ impl CampaignState { Ok(()) } + /// Saves the campaign state to `~/.smitebot/runs//state.json`. + pub fn save_campaign(&self) -> Result<(), StateError> { + let runs_dir = Self::runs_dir().ok_or(StateError::HomeDir)?; + self.save(&runs_dir.join(&self.id).join("state.json")) + } + /// Loads campaign state from a JSON file written by `save`. - pub fn load(path: &Path) -> Result { + fn load(path: &Path) -> Result { let contents = fs::read_to_string(path).map_err(|source| StateError::Read { path: path.to_path_buf(), source, @@ -178,14 +184,9 @@ impl CampaignState { } /// Loads state for campaign `id` from `~/.smitebot/runs//state.json`. - /// - /// Returns the state and its path (the path is needed by callers that - /// mutate and re-save state, e.g. `stop`). - pub fn load_campaign(id: &str) -> Result<(Self, PathBuf), StateError> { + pub fn load_campaign(id: &str) -> Result { let runs_dir = Self::runs_dir().ok_or(StateError::HomeDir)?; - let state_path = runs_dir.join(id).join("state.json"); - let state = Self::load(&state_path)?; - Ok((state, state_path)) + Self::load(&runs_dir.join(id).join("state.json")) } }