fix: only warn about ignored flags when explicitly passed via CLI (#373)
The warning about launch-time options being ignored (when daemon is already running) was incorrectly shown when options were set via environment variables like AGENT_BROWSER_EXECUTABLE_PATH, even when no CLI flag was passed. Now the warning only appears when flags are explicitly passed on the command line, not when values come solely from environment variables. Fixes #372
This commit is contained in:
@@ -1425,6 +1425,14 @@ mod tests {
|
||||
provider: None,
|
||||
ignore_https_errors: false,
|
||||
device: None,
|
||||
cli_executable_path: false,
|
||||
cli_extensions: false,
|
||||
cli_profile: false,
|
||||
cli_state: false,
|
||||
cli_args: false,
|
||||
cli_user_agent: false,
|
||||
cli_proxy: false,
|
||||
cli_proxy_bypass: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,6 +19,17 @@ pub struct Flags {
|
||||
pub provider: Option<String>,
|
||||
pub ignore_https_errors: bool,
|
||||
pub device: Option<String>,
|
||||
|
||||
// Track which launch-time options were explicitly passed via CLI
|
||||
// (as opposed to being set only via environment variables)
|
||||
pub cli_executable_path: bool,
|
||||
pub cli_extensions: bool,
|
||||
pub cli_profile: bool,
|
||||
pub cli_state: bool,
|
||||
pub cli_args: bool,
|
||||
pub cli_user_agent: bool,
|
||||
pub cli_proxy: bool,
|
||||
pub cli_proxy_bypass: bool,
|
||||
}
|
||||
|
||||
pub fn parse_flags(args: &[String]) -> Flags {
|
||||
@@ -51,6 +62,15 @@ pub fn parse_flags(args: &[String]) -> Flags {
|
||||
provider: env::var("AGENT_BROWSER_PROVIDER").ok(),
|
||||
ignore_https_errors: false,
|
||||
device: env::var("AGENT_BROWSER_IOS_DEVICE").ok(),
|
||||
// Track CLI-passed flags (default false, set to true when flag is passed)
|
||||
cli_executable_path: false,
|
||||
cli_extensions: false,
|
||||
cli_profile: false,
|
||||
cli_state: false,
|
||||
cli_args: false,
|
||||
cli_user_agent: false,
|
||||
cli_proxy: false,
|
||||
cli_proxy_bypass: false,
|
||||
};
|
||||
|
||||
let mut i = 0;
|
||||
@@ -75,12 +95,14 @@ pub fn parse_flags(args: &[String]) -> Flags {
|
||||
"--executable-path" => {
|
||||
if let Some(s) = args.get(i + 1) {
|
||||
flags.executable_path = Some(s.clone());
|
||||
flags.cli_executable_path = true;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
"--extension" => {
|
||||
if let Some(s) = args.get(i + 1) {
|
||||
flags.extensions.push(s.clone());
|
||||
flags.cli_extensions = true;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
@@ -93,36 +115,42 @@ pub fn parse_flags(args: &[String]) -> Flags {
|
||||
"--profile" => {
|
||||
if let Some(s) = args.get(i + 1) {
|
||||
flags.profile = Some(s.clone());
|
||||
flags.cli_profile = true;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
"--state" => {
|
||||
if let Some(s) = args.get(i + 1) {
|
||||
flags.state = Some(s.clone());
|
||||
flags.cli_state = true;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
"--proxy" => {
|
||||
if let Some(p) = args.get(i + 1) {
|
||||
flags.proxy = Some(p.clone());
|
||||
flags.cli_proxy = true;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
"--proxy-bypass" => {
|
||||
if let Some(s) = args.get(i + 1) {
|
||||
flags.proxy_bypass = Some(s.clone());
|
||||
flags.cli_proxy_bypass = true;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
"--args" => {
|
||||
if let Some(s) = args.get(i + 1) {
|
||||
flags.args = Some(s.clone());
|
||||
flags.cli_args = true;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
"--user-agent" => {
|
||||
if let Some(s) = args.get(i + 1) {
|
||||
flags.user_agent = Some(s.clone());
|
||||
flags.cli_user_agent = true;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
@@ -311,4 +339,44 @@ mod tests {
|
||||
assert_eq!(flags.session, "test");
|
||||
assert_eq!(flags.executable_path, Some("/custom/chrome".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_cli_executable_path_tracking() {
|
||||
// When --executable-path is passed via CLI, cli_executable_path should be true
|
||||
let flags = parse_flags(&args("--executable-path /path/to/chrome snapshot"));
|
||||
assert!(flags.cli_executable_path);
|
||||
assert_eq!(flags.executable_path, Some("/path/to/chrome".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_cli_executable_path_not_set_without_flag() {
|
||||
// When no --executable-path is passed, cli_executable_path should be false
|
||||
// (even if env var sets executable_path to Some value, which we can't test here)
|
||||
let flags = parse_flags(&args("snapshot"));
|
||||
assert!(!flags.cli_executable_path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_cli_extension_tracking() {
|
||||
let flags = parse_flags(&args("--extension /path/to/ext snapshot"));
|
||||
assert!(flags.cli_extensions);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_cli_profile_tracking() {
|
||||
let flags = parse_flags(&args("--profile /path/to/profile snapshot"));
|
||||
assert!(flags.cli_profile);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_cli_multiple_flags_tracking() {
|
||||
let flags = parse_flags(&args(
|
||||
"--executable-path /chrome --profile /profile --proxy http://proxy snapshot",
|
||||
));
|
||||
assert!(flags.cli_executable_path);
|
||||
assert!(flags.cli_profile);
|
||||
assert!(flags.cli_proxy);
|
||||
assert!(!flags.cli_extensions);
|
||||
assert!(!flags.cli_state);
|
||||
}
|
||||
}
|
||||
|
||||
+39
-14
@@ -221,22 +221,51 @@ fn main() {
|
||||
}
|
||||
};
|
||||
|
||||
// Warn if launch-time options were specified but daemon was already running
|
||||
// Warn if launch-time options were explicitly passed via CLI but daemon was already running
|
||||
// Only warn about flags that were passed on the command line, not those set via environment
|
||||
// variables (since the daemon already uses the env vars when it starts).
|
||||
if daemon_result.already_running {
|
||||
let has_extensions = !flags.extensions.is_empty();
|
||||
let ignored_flags: Vec<&str> = [
|
||||
flags.executable_path.as_ref().map(|_| "--executable-path"),
|
||||
if has_extensions {
|
||||
if flags.cli_executable_path {
|
||||
Some("--executable-path")
|
||||
} else {
|
||||
None
|
||||
},
|
||||
if flags.cli_extensions {
|
||||
Some("--extension")
|
||||
} else {
|
||||
None
|
||||
},
|
||||
flags.profile.as_ref().map(|_| "--profile"),
|
||||
flags.state.as_ref().map(|_| "--state"),
|
||||
flags.args.as_ref().map(|_| "--args"),
|
||||
flags.user_agent.as_ref().map(|_| "--user-agent"),
|
||||
flags.proxy.as_ref().map(|_| "--proxy"),
|
||||
flags.proxy_bypass.as_ref().map(|_| "--proxy-bypass"),
|
||||
if flags.cli_profile {
|
||||
Some("--profile")
|
||||
} else {
|
||||
None
|
||||
},
|
||||
if flags.cli_state {
|
||||
Some("--state")
|
||||
} else {
|
||||
None
|
||||
},
|
||||
if flags.cli_args {
|
||||
Some("--args")
|
||||
} else {
|
||||
None
|
||||
},
|
||||
if flags.cli_user_agent {
|
||||
Some("--user-agent")
|
||||
} else {
|
||||
None
|
||||
},
|
||||
if flags.cli_proxy {
|
||||
Some("--proxy")
|
||||
} else {
|
||||
None
|
||||
},
|
||||
if flags.cli_proxy_bypass {
|
||||
Some("--proxy-bypass")
|
||||
} else {
|
||||
None
|
||||
},
|
||||
flags.ignore_https_errors.then(|| "--ignore-https-errors"),
|
||||
]
|
||||
.into_iter()
|
||||
@@ -250,10 +279,6 @@ fn main() {
|
||||
ignored_flags.join(", ")
|
||||
);
|
||||
}
|
||||
|
||||
if flags.ignore_https_errors {
|
||||
eprintln!("{} --ignore-https-errors ignored: daemon already running. Use 'agent-browser close' first to restart with this option.", color::warning_indicator());
|
||||
}
|
||||
}
|
||||
|
||||
// Validate mutually exclusive options
|
||||
|
||||
Reference in New Issue
Block a user