diff --git a/docs/references/agent-troubleshooting.md b/docs/references/agent-troubleshooting.md index af2b5aa..5dd4708 100644 --- a/docs/references/agent-troubleshooting.md +++ b/docs/references/agent-troubleshooting.md @@ -74,8 +74,9 @@ steel browser start --local --session "$SESSION_NAME" authenticate (`steel login`) or export `STEEL_API_KEY`. - `Failed to reach Steel session API ...`: check endpoint flags/env and runtime availability. -- Existing session not reused: - verify exact `--session` spelling and same mode (cloud/local). +- `Session "" is not running...`: + the session expired or the name is misspelled. Check `steel browser sessions`, + then run `steel browser start --session `. - Stale state: `steel browser stop --all`, then start a new named session. diff --git a/src/commands/browser/action.rs b/src/commands/browser/action.rs index 2d42ad8..ad8d50d 100644 --- a/src/commands/browser/action.rs +++ b/src/commands/browser/action.rs @@ -11,7 +11,6 @@ use crate::browser::daemon::process; use crate::browser::daemon::protocol::{ DEFAULT_INACTIVITY_TIMEOUT_MS, DaemonCommand, DaemonCreateParams, }; -use crate::status; use crate::util::{api, output}; // ── Shared arg types ──────────────────────────────────────────────── @@ -393,7 +392,7 @@ pub struct FillArgs { /// Element selector or ref pub selector: String, /// Value to fill - #[arg(trailing_var_arg = true, num_args = 1..)] + #[arg(num_args = 1..)] pub value: Vec, } @@ -402,7 +401,7 @@ pub struct TypeArgs { /// Element selector or ref pub selector: String, /// Text to type - #[arg(trailing_var_arg = true, num_args = 1..)] + #[arg(num_args = 1..)] pub text: Vec, /// Clear the field before typing #[arg(long)] @@ -443,7 +442,7 @@ pub struct SetValueArgs { /// Element selector or ref pub selector: String, /// Value to set - #[arg(trailing_var_arg = true, num_args = 1..)] + #[arg(num_args = 1..)] pub value: Vec, } @@ -641,7 +640,7 @@ pub struct SetHeadersArgs { #[derive(Parser)] pub struct SetUserAgentArgs { /// User agent string - #[arg(trailing_var_arg = true, num_args = 1..)] + #[arg(num_args = 1..)] pub user_agent: Vec, } @@ -1240,7 +1239,13 @@ async fn ensure_daemon(session_name: Option<&str>) -> Result { return Ok(client); } - status!("Starting browser session..."); + if let Some(name) = session_name { + anyhow::bail!( + "Session \"{name}\" is not running. It may have expired. Run `steel browser start --session {name}` to start a new one." + ); + } + + eprintln!("warning: session \"default\" is not running; starting a new one"); let (mode, base_url, auth) = api::resolve_with_auth(); let params = DaemonCreateParams { diff --git a/src/commands/browser/mod.rs b/src/commands/browser/mod.rs index 8c2666e..994fc21 100644 --- a/src/commands/browser/mod.rs +++ b/src/commands/browser/mod.rs @@ -77,3 +77,91 @@ pub async fn run(args: BrowserArgs) -> anyhow::Result<()> { Command::Action(action) => action::run(action, session.as_deref()).await, } } + +#[cfg(test)] +mod tests { + use clap::Parser; + + use super::action::{ActionCommand, SetCommand}; + use super::{BrowserArgs, Command}; + use crate::commands::{self, Cli}; + + fn parse(args: &[&str]) -> BrowserArgs { + let argv = ["steel", "browser"].into_iter().chain(args.iter().copied()); + match Cli::try_parse_from(argv).unwrap().command { + commands::Command::Browser(browser) => browser, + _ => panic!("expected browser command"), + } + } + + #[test] + fn session_after_fill_value_is_a_flag() { + let args = parse(&["fill", "@e1", "herman", "miller", "--session", "repro"]); + assert_eq!(args.session.as_deref(), Some("repro")); + let Command::Action(ActionCommand::Fill(fill)) = args.command else { + panic!("expected fill"); + }; + assert_eq!(fill.value, ["herman", "miller"]); + } + + #[test] + fn flags_after_type_text_are_flags() { + let args = parse(&[ + "type", + "@e1", + "hello", + "--clear", + "--delay", + "50", + "--session", + "repro", + ]); + assert_eq!(args.session.as_deref(), Some("repro")); + let Command::Action(ActionCommand::Type(typed)) = args.command else { + panic!("expected type"); + }; + assert_eq!(typed.text, ["hello"]); + assert!(typed.clear); + assert_eq!(typed.delay, Some(50)); + } + + #[test] + fn session_after_setvalue_value_is_a_flag() { + let args = parse(&["setvalue", "@e1", "42", "--session", "repro"]); + assert_eq!(args.session.as_deref(), Some("repro")); + let Command::Action(ActionCommand::SetValue(set)) = args.command else { + panic!("expected setvalue"); + }; + assert_eq!(set.value, ["42"]); + } + + #[test] + fn session_after_user_agent_is_a_flag() { + let args = parse(&[ + "set", + "useragent", + "Mozilla/5.0", + "Test", + "--session", + "repro", + ]); + assert_eq!(args.session.as_deref(), Some("repro")); + let Command::Action(ActionCommand::Set { + command: SetCommand::UserAgent(ua), + }) = args.command + else { + panic!("expected set useragent"); + }; + assert_eq!(ua.user_agent, ["Mozilla/5.0", "Test"]); + } + + #[test] + fn double_dash_keeps_flag_like_text() { + let args = parse(&["fill", "@e1", "--", "--session", "repro"]); + assert_eq!(args.session, None); + let Command::Action(ActionCommand::Fill(fill)) = args.command else { + panic!("expected fill"); + }; + assert_eq!(fill.value, ["--session", "repro"]); + } +} diff --git a/tests/cli_blackbox.rs b/tests/cli_blackbox.rs index b048a1b..8abfbcf 100644 --- a/tests/cli_blackbox.rs +++ b/tests/cli_blackbox.rs @@ -403,6 +403,46 @@ fn browser_start_without_auth_json_mode_surfaces_error() { ); } +#[test] +fn browser_action_with_missing_named_session_fails_without_starting_one() { + let (mut cmd, tmp) = steel_cmd(); + cmd.args([ + "--json", + "browser", + "navigate", + "https://example.com", + "--session", + "missing", + ]); + let output = cmd.output().expect("failed to execute steel binary"); + assert!(!output.status.success()); + + let out = stdout(&output); + let parsed: serde_json::Value = serde_json::from_str(out.trim()) + .unwrap_or_else(|_| panic!("expected valid JSON, got: {out}")); + let error_msg = parsed["error"].as_str().unwrap_or(""); + assert!( + error_msg.contains("steel browser start --session missing"), + "error should say how to start the session, got: {error_msg}" + ); + assert!( + !tmp.path().join("daemon-missing.log").exists(), + "no daemon should be spawned for a missing named session" + ); +} + +#[test] +fn browser_action_without_session_warns_before_starting_default() { + let output = run(&["--json", "browser", "navigate", "https://example.com"]); + assert!(!output.status.success()); + + let err = stderr(&output); + assert!( + err.contains("warning: session \"default\" is not running"), + "stderr should warn that a default session is being started, got: {err}" + ); +} + // ─── Environment variable contracts ───────────────────────────────── #[test]