From cf8ed5ba6785dbd20a8c0aec5edcdaaca6a1839d Mon Sep 17 00:00:00 2001 From: Marco Allegretti Date: Wed, 25 Feb 2026 18:51:14 +0100 Subject: [PATCH] fix(cli): improve enter/stop error context - Stop now takes the store lock like other mutating commands\n- Wrap enter/exec/stop errors with env input and resolved id context\n- Enter/exec now return a clearer error when the env is already running or not built --- crates/karapace-cli/src/commands/enter.rs | 9 +++++++-- crates/karapace-cli/src/commands/exec.rs | 5 ++++- crates/karapace-cli/src/commands/stop.rs | 15 ++++++++++---- crates/karapace-core/src/engine.rs | 24 +++++++++++++++++++++-- 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/crates/karapace-cli/src/commands/enter.rs b/crates/karapace-cli/src/commands/enter.rs index ffcfa25..f21f071 100644 --- a/crates/karapace-cli/src/commands/enter.rs +++ b/crates/karapace-cli/src/commands/enter.rs @@ -13,10 +13,15 @@ pub fn run( let _lock = StoreLock::acquire(&layout.lock_file()).map_err(|e| format!("store lock: {e}"))?; let resolved = resolve_env_id_pretty(engine, env_id)?; + let short = resolved.get(..12).unwrap_or(&resolved); if command.is_empty() { - engine.enter(&resolved).map_err(|e| e.to_string())?; + engine + .enter(&resolved) + .map_err(|e| format!("{e} (env '{env_id}' -> {short})"))?; } else { - engine.exec(&resolved, command).map_err(|e| e.to_string())?; + engine + .exec(&resolved, command) + .map_err(|e| format!("{e} (env '{env_id}' -> {short})"))?; } Ok(EXIT_SUCCESS) } diff --git a/crates/karapace-cli/src/commands/exec.rs b/crates/karapace-cli/src/commands/exec.rs index 35257b2..3564558 100644 --- a/crates/karapace-cli/src/commands/exec.rs +++ b/crates/karapace-cli/src/commands/exec.rs @@ -14,6 +14,9 @@ pub fn run( let _lock = StoreLock::acquire(&layout.lock_file()).map_err(|e| format!("store lock: {e}"))?; let resolved = resolve_env_id_pretty(engine, env_id)?; - engine.exec(&resolved, command).map_err(|e| e.to_string())?; + let short = resolved.get(..12).unwrap_or(&resolved); + engine + .exec(&resolved, command) + .map_err(|e| format!("{e} (env '{env_id}' -> {short})"))?; Ok(EXIT_SUCCESS) } diff --git a/crates/karapace-cli/src/commands/stop.rs b/crates/karapace-cli/src/commands/stop.rs index ffa6ad7..dba4ee0 100644 --- a/crates/karapace-cli/src/commands/stop.rs +++ b/crates/karapace-cli/src/commands/stop.rs @@ -1,10 +1,17 @@ use super::{resolve_env_id_pretty, EXIT_SUCCESS}; -use karapace_core::Engine; +use karapace_core::{Engine, StoreLock}; +use karapace_store::StoreLayout; use std::path::Path; -pub fn run(engine: &Engine, _store_path: &Path, env_id: &str) -> Result { +pub fn run(engine: &Engine, store_path: &Path, env_id: &str) -> Result { + let layout = StoreLayout::new(store_path); + let _lock = StoreLock::acquire(&layout.lock_file()).map_err(|e| format!("store lock: {e}"))?; + let resolved = resolve_env_id_pretty(engine, env_id)?; - engine.stop(&resolved).map_err(|e| e.to_string())?; - println!("stopped environment {env_id}"); + let short = resolved.get(..12).unwrap_or(&resolved); + engine + .stop(&resolved) + .map_err(|e| format!("{e} (env '{env_id}' -> {short})"))?; + println!("stopped environment {env_id} ({short})"); Ok(EXIT_SUCCESS) } diff --git a/crates/karapace-core/src/engine.rs b/crates/karapace-core/src/engine.rs index 309fdc9..4f6964f 100644 --- a/crates/karapace-core/src/engine.rs +++ b/crates/karapace-core/src/engine.rs @@ -378,7 +378,17 @@ impl Engine { .get(env_id) .map_err(|_| CoreError::EnvNotFound(env_id.to_owned()))?; - validate_transition(meta.state, EnvState::Running)?; + if meta.state == EnvState::Running { + return Err(CoreError::Runtime( + karapace_runtime::RuntimeError::AlreadyRunning(env_id.to_owned()), + )); + } + if meta.state != EnvState::Built { + return Err(CoreError::InvalidTransition { + from: meta.state.to_string(), + to: "enter requires built state".to_owned(), + }); + } let normalized = self.load_manifest(&meta.manifest_hash)?; let store_str = self.store_root_str.clone(); @@ -415,7 +425,17 @@ impl Engine { .get(env_id) .map_err(|_| CoreError::EnvNotFound(env_id.to_owned()))?; - validate_transition(meta.state, EnvState::Running)?; + if meta.state == EnvState::Running { + return Err(CoreError::Runtime( + karapace_runtime::RuntimeError::AlreadyRunning(env_id.to_owned()), + )); + } + if meta.state != EnvState::Built { + return Err(CoreError::InvalidTransition { + from: meta.state.to_string(), + to: "exec requires built state".to_owned(), + }); + } let normalized = self.load_manifest(&meta.manifest_hash)?; let store_str = self.store_root_str.clone();