* fix: prevent state commands from starting daemon without session_name (#677) State management commands (state_list, state_show, state_clear, state_clean, state_rename) are pure file operations that don't need a running daemon. Previously, these commands would trigger daemon startup via ensure_daemon(), and if AGENT_BROWSER_SESSION_NAME was exported after the first command (e.g. `state clear --all`), the daemon would start without session_name. Subsequent open/close commands would reuse that daemon, causing close to skip state persistence entirely. Fix: execute state management commands locally in the CLI process before ensure_daemon() is called. This is done via a new dispatch_state_command() function in state.rs that centralizes the command routing, used by both the CLI (local path) and the daemon (batch/IPC path). Also: - Add OutputOptions::from_flags() helper to deduplicate construction - Add unit tests for dispatch_state_command routing and error handling * style: fix fmt and clippy warnings - Remove redundant closure in dispatch_state_command (clippy::redundant_closure) - Remove needless borrow in run_batch (clippy::needless_borrow) - Fix trailing blank lines (rustfmt) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: hyunjinee <leehj0110@kakao.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
hyunjinee
parent
d374e413be
commit
9c0955ca99
+27
-10
@@ -303,6 +303,31 @@ fn main() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Handle state management commands locally — these are pure file operations
|
||||||
|
// that don't need a daemon, avoiding an unnecessary daemon startup that
|
||||||
|
// would lack runtime config like session_name.
|
||||||
|
if let Some(result) = native::state::dispatch_state_command(&cmd) {
|
||||||
|
let action = cmd.get("action").and_then(|v| v.as_str());
|
||||||
|
let resp = match result {
|
||||||
|
Ok(data) => connection::Response {
|
||||||
|
success: true,
|
||||||
|
data: Some(data),
|
||||||
|
error: None,
|
||||||
|
},
|
||||||
|
Err(e) => connection::Response {
|
||||||
|
success: false,
|
||||||
|
data: None,
|
||||||
|
error: Some(e),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
let output_opts = OutputOptions::from_flags(&flags);
|
||||||
|
output::print_response_with_opts(&resp, action, &output_opts);
|
||||||
|
if !resp.success {
|
||||||
|
exit(1);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
let daemon_opts = DaemonOptions {
|
let daemon_opts = DaemonOptions {
|
||||||
headed: flags.headed,
|
headed: flags.headed,
|
||||||
debug: flags.debug,
|
debug: flags.debug,
|
||||||
@@ -744,11 +769,7 @@ fn main() {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let output_opts = OutputOptions {
|
let output_opts = OutputOptions::from_flags(&flags);
|
||||||
json: flags.json,
|
|
||||||
content_boundaries: flags.content_boundaries,
|
|
||||||
max_output: flags.max_output,
|
|
||||||
};
|
|
||||||
|
|
||||||
match send_command(cmd.clone(), &flags.session) {
|
match send_command(cmd.clone(), &flags.session) {
|
||||||
Ok(resp) => {
|
Ok(resp) => {
|
||||||
@@ -863,11 +884,7 @@ fn run_batch(flags: &Flags, bail: bool) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let output_opts = OutputOptions {
|
let output_opts = OutputOptions::from_flags(flags);
|
||||||
json: flags.json,
|
|
||||||
content_boundaries: flags.content_boundaries,
|
|
||||||
max_output: flags.max_output,
|
|
||||||
};
|
|
||||||
|
|
||||||
let mut results: Vec<serde_json::Value> = Vec::new();
|
let mut results: Vec<serde_json::Value> = Vec::new();
|
||||||
let mut had_error = false;
|
let mut had_error = false;
|
||||||
|
|||||||
@@ -947,11 +947,10 @@ pub async fn execute_command(cmd: &Value, state: &mut DaemonState) -> Value {
|
|||||||
"errors" => handle_errors(state).await,
|
"errors" => handle_errors(state).await,
|
||||||
"state_save" => handle_state_save(cmd, state).await,
|
"state_save" => handle_state_save(cmd, state).await,
|
||||||
"state_load" => handle_state_load(cmd, state).await,
|
"state_load" => handle_state_load(cmd, state).await,
|
||||||
"state_list" => handle_state_list().await,
|
"state_list" | "state_show" | "state_clear" | "state_clean" | "state_rename" => {
|
||||||
"state_show" => handle_state_show(cmd).await,
|
state::dispatch_state_command(cmd)
|
||||||
"state_clear" => handle_state_clear(cmd).await,
|
.expect("dispatch_state_command must handle all state_* actions matched here")
|
||||||
"state_clean" => handle_state_clean(cmd).await,
|
}
|
||||||
"state_rename" => handle_state_rename(cmd).await,
|
|
||||||
"trace_start" => handle_trace_start(state).await,
|
"trace_start" => handle_trace_start(state).await,
|
||||||
"trace_stop" => handle_trace_stop(cmd, state).await,
|
"trace_stop" => handle_trace_stop(cmd, state).await,
|
||||||
"profiler_start" => handle_profiler_start(cmd, state).await,
|
"profiler_start" => handle_profiler_start(cmd, state).await,
|
||||||
@@ -2671,40 +2670,6 @@ async fn handle_state_load(cmd: &Value, state: &DaemonState) -> Result<Value, St
|
|||||||
Ok(json!({ "loaded": true, "path": path }))
|
Ok(json!({ "loaded": true, "path": path }))
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn handle_state_list() -> Result<Value, String> {
|
|
||||||
state::state_list()
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn handle_state_show(cmd: &Value) -> Result<Value, String> {
|
|
||||||
let path = cmd
|
|
||||||
.get("path")
|
|
||||||
.and_then(|v| v.as_str())
|
|
||||||
.ok_or("Missing 'path' parameter")?;
|
|
||||||
state::state_show(path)
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn handle_state_clear(cmd: &Value) -> Result<Value, String> {
|
|
||||||
let path = cmd.get("path").and_then(|v| v.as_str());
|
|
||||||
state::state_clear(path)
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn handle_state_clean(cmd: &Value) -> Result<Value, String> {
|
|
||||||
let days = cmd.get("days").and_then(|v| v.as_u64()).unwrap_or(30);
|
|
||||||
state::state_clean(days)
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn handle_state_rename(cmd: &Value) -> Result<Value, String> {
|
|
||||||
let path = cmd
|
|
||||||
.get("path")
|
|
||||||
.and_then(|v| v.as_str())
|
|
||||||
.ok_or("Missing 'path' parameter")?;
|
|
||||||
let name = cmd
|
|
||||||
.get("name")
|
|
||||||
.and_then(|v| v.as_str())
|
|
||||||
.ok_or("Missing 'name' parameter")?;
|
|
||||||
state::state_rename(path, name)
|
|
||||||
}
|
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Phase 6 handlers
|
// Phase 6 handlers
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -475,6 +475,41 @@ pub fn find_auto_state_file(session_name: &str) -> Option<String> {
|
|||||||
best_path.map(|(p, _)| p)
|
best_path.map(|(p, _)| p)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Dispatch a state management command from its JSON payload.
|
||||||
|
/// Returns `Some(result)` for recognised state_* actions, `None` otherwise.
|
||||||
|
pub fn dispatch_state_command(cmd: &Value) -> Option<Result<Value, String>> {
|
||||||
|
let action = cmd.get("action").and_then(|v| v.as_str())?;
|
||||||
|
match action {
|
||||||
|
"state_list" => Some(state_list()),
|
||||||
|
"state_show" => Some(
|
||||||
|
cmd.get("path")
|
||||||
|
.and_then(|v| v.as_str())
|
||||||
|
.ok_or_else(|| "Missing 'path' parameter".to_string())
|
||||||
|
.and_then(state_show),
|
||||||
|
),
|
||||||
|
"state_clear" => {
|
||||||
|
let path = cmd.get("path").and_then(|v| v.as_str());
|
||||||
|
Some(state_clear(path))
|
||||||
|
}
|
||||||
|
"state_clean" => {
|
||||||
|
let days = cmd.get("days").and_then(|v| v.as_u64()).unwrap_or(30);
|
||||||
|
Some(state_clean(days))
|
||||||
|
}
|
||||||
|
"state_rename" => Some(
|
||||||
|
cmd.get("path")
|
||||||
|
.and_then(|v| v.as_str())
|
||||||
|
.ok_or_else(|| "Missing 'path' parameter".to_string())
|
||||||
|
.and_then(|path| {
|
||||||
|
cmd.get("name")
|
||||||
|
.and_then(|v| v.as_str())
|
||||||
|
.ok_or_else(|| "Missing 'name' parameter".to_string())
|
||||||
|
.and_then(|name| state_rename(path, name))
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
_ => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub fn get_sessions_dir() -> PathBuf {
|
pub fn get_sessions_dir() -> PathBuf {
|
||||||
if let Some(home) = dirs::home_dir() {
|
if let Some(home) = dirs::home_dir() {
|
||||||
home.join(".agent-browser").join("sessions")
|
home.join(".agent-browser").join("sessions")
|
||||||
@@ -604,4 +639,45 @@ mod tests {
|
|||||||
assert_eq!(json["secure"], true);
|
assert_eq!(json["secure"], true);
|
||||||
assert_eq!(json["sameSite"], "Strict");
|
assert_eq!(json["sameSite"], "Strict");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_dispatch_state_command_routes_state_list() {
|
||||||
|
let cmd = serde_json::json!({ "action": "state_list" });
|
||||||
|
let result = dispatch_state_command(&cmd);
|
||||||
|
assert!(result.is_some());
|
||||||
|
assert!(result.unwrap().is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_dispatch_state_command_returns_none_for_unknown() {
|
||||||
|
let cmd = serde_json::json!({ "action": "navigate" });
|
||||||
|
assert!(dispatch_state_command(&cmd).is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_dispatch_state_command_returns_none_for_missing_action() {
|
||||||
|
let cmd = serde_json::json!({});
|
||||||
|
assert!(dispatch_state_command(&cmd).is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_dispatch_state_show_missing_path() {
|
||||||
|
let cmd = serde_json::json!({ "action": "state_show" });
|
||||||
|
let result = dispatch_state_command(&cmd).unwrap();
|
||||||
|
assert!(result.is_err());
|
||||||
|
assert_eq!(result.unwrap_err(), "Missing 'path' parameter");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_dispatch_state_rename_missing_params() {
|
||||||
|
let cmd = serde_json::json!({ "action": "state_rename" });
|
||||||
|
let result = dispatch_state_command(&cmd).unwrap();
|
||||||
|
assert!(result.is_err());
|
||||||
|
assert_eq!(result.unwrap_err(), "Missing 'path' parameter");
|
||||||
|
|
||||||
|
let cmd = serde_json::json!({ "action": "state_rename", "path": "/tmp/test.json" });
|
||||||
|
let result = dispatch_state_command(&cmd).unwrap();
|
||||||
|
assert!(result.is_err());
|
||||||
|
assert_eq!(result.unwrap_err(), "Missing 'name' parameter");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -23,6 +23,16 @@ pub struct OutputOptions {
|
|||||||
pub max_output: Option<usize>,
|
pub max_output: Option<usize>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl OutputOptions {
|
||||||
|
pub fn from_flags(flags: &crate::flags::Flags) -> Self {
|
||||||
|
Self {
|
||||||
|
json: flags.json,
|
||||||
|
content_boundaries: flags.content_boundaries,
|
||||||
|
max_output: flags.max_output,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn truncate_if_needed(content: &str, max: Option<usize>) -> String {
|
fn truncate_if_needed(content: &str, max: Option<usize>) -> String {
|
||||||
let Some(limit) = max else {
|
let Some(limit) = max else {
|
||||||
return content.to_string();
|
return content.to_string();
|
||||||
|
|||||||
Reference in New Issue
Block a user