Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-multi-account-selection.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

Fix multi-account selection: MCP server now respects `GOOGLE_WORKSPACE_CLI_ACCOUNT` env var (#221), and `--account` flag before service name no longer causes parse errors (#181)
57 changes: 45 additions & 12 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ async fn run() -> Result<(), GwsError> {
));
}

// Find the first non-flag arg (skip --account and its value)
// Find the first non-flag arg (skip --account/--api-version and their values)
let mut first_arg: Option<String> = None;
{
let mut skip_next = false;
Expand All @@ -80,11 +80,11 @@ async fn run() -> Result<(), GwsError> {
skip_next = false;
continue;
}
if a == "--account" {
if a == "--account" || a == "--api-version" {
skip_next = true;
continue;
}
if a.starts_with("--account=") {
if a.starts_with("--account=") || a.starts_with("--api-version=") {
continue;
}
if !a.starts_with("--") || a.as_str() == "--help" || a.as_str() == "--version" {
Expand Down Expand Up @@ -165,7 +165,7 @@ async fn run() -> Result<(), GwsError> {
// Re-parse args (skip argv[0] which is the binary, and argv[1] which is the service name)
// Filter out --api-version and its value
// Prepend "gws" as the program name since try_get_matches_from expects argv[0]
let sub_args = filter_args_for_subcommand(&args);
let sub_args = filter_args_for_subcommand(&args, &first_arg);

let matches = cli.try_get_matches_from(&sub_args).map_err(|e| {
// If it's a help or version display, print it and exit cleanly
Expand Down Expand Up @@ -321,10 +321,11 @@ pub fn parse_service_and_version(
Ok((api_name, version))
}

pub fn filter_args_for_subcommand(args: &[String]) -> Vec<String> {
pub fn filter_args_for_subcommand(args: &[String], service_name: &str) -> Vec<String> {
let mut sub_args: Vec<String> = vec!["gws".to_string()];
let mut skip_next = false;
for arg in args.iter().skip(2) {
let mut service_skipped = false;
for arg in args.iter().skip(1) {
if skip_next {
skip_next = false;
continue;
Expand All @@ -336,6 +337,10 @@ pub fn filter_args_for_subcommand(args: &[String]) -> Vec<String> {
if arg.starts_with("--account=") || arg.starts_with("--api-version=") {
continue;
}
if !service_skipped && arg == service_name {
service_skipped = true;
continue;
}
sub_args.push(arg.clone());
}
sub_args
Expand Down Expand Up @@ -698,7 +703,7 @@ mod tests {
"files".into(),
"list".into(),
];
let filtered = filter_args_for_subcommand(&args);
let filtered = filter_args_for_subcommand(&args, "drive");
assert_eq!(filtered, vec!["gws", "files", "list"]);
assert!(!filtered.contains(&"--account".to_string()));
assert!(!filtered.contains(&"user@corp.com".to_string()));
Expand All @@ -714,7 +719,7 @@ mod tests {
"files".into(),
"list".into(),
];
let filtered = filter_args_for_subcommand(&args);
let filtered = filter_args_for_subcommand(&args, "drive");
assert_eq!(filtered, vec!["gws", "files", "list"]);
}

Expand All @@ -730,7 +735,7 @@ mod tests {
"files".into(),
"list".into(),
];
let filtered = filter_args_for_subcommand(&args);
let filtered = filter_args_for_subcommand(&args, "drive");
assert_eq!(filtered, vec!["gws", "files", "list"]);
}

Expand All @@ -744,7 +749,7 @@ mod tests {
"--format".into(),
"table".into(),
];
let filtered = filter_args_for_subcommand(&args);
let filtered = filter_args_for_subcommand(&args, "drive");
assert_eq!(filtered, vec!["gws", "files", "list", "--format", "table"]);
}

Expand Down Expand Up @@ -777,7 +782,7 @@ mod tests {
"files".into(),
"list".into(),
];
let filtered = filter_args_for_subcommand(&args);
let filtered = filter_args_for_subcommand(&args, "drive");
assert!(!filtered.contains(&"--account".to_string()));
assert!(!filtered.contains(&"work@corp.com".to_string()));
assert!(filtered.contains(&"files".to_string()));
Expand All @@ -796,6 +801,34 @@ mod tests {
);
}

#[test]
fn test_filter_args_account_before_service() {
// --account appears BEFORE the service name (issue #181)
let args: Vec<String> = vec![
"gws".into(),
"--account".into(),
"work@corp.com".into(),
"drive".into(),
"files".into(),
"list".into(),
];
let filtered = filter_args_for_subcommand(&args, "drive");
assert_eq!(filtered, vec!["gws", "files", "list"]);
}

#[test]
fn test_filter_args_account_equals_before_service() {
let args: Vec<String> = vec![
"gws".into(),
"--account=work@corp.com".into(),
"drive".into(),
"files".into(),
"list".into(),
];
let filtered = filter_args_for_subcommand(&args, "drive");
assert_eq!(filtered, vec!["gws", "files", "list"]);
}

#[test]
fn test_filter_args_strips_account_equals() {
let args: Vec<String> = vec![
Expand All @@ -805,7 +838,7 @@ mod tests {
"files".into(),
"list".into(),
];
let filtered = filter_args_for_subcommand(&args);
let filtered = filter_args_for_subcommand(&args, "drive");
assert!(!filtered.iter().any(|a| a.contains("account")));
assert_eq!(filtered, vec!["gws", "files", "list"]);
}
Expand Down
3 changes: 2 additions & 1 deletion src/mcp_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,8 @@ async fn execute_mcp_method(
};

let scopes: Vec<&str> = crate::select_scope(&method.scopes).into_iter().collect();
let (token, auth_method) = match crate::auth::get_token(&scopes, None).await {
let account = std::env::var("GOOGLE_WORKSPACE_CLI_ACCOUNT").ok();
let (token, auth_method) = match crate::auth::get_token(&scopes, account.as_deref()).await {
Comment on lines +817 to +818
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This change correctly makes the MCP server respect the GOOGLE_WORKSPACE_CLI_ACCOUNT environment variable. However, it overlooks the global --account flag, leading to inconsistent behavior.

Currently, if a user runs gws --account user@corp.com mcp, the command will fail with an unrecognized subcommand error because the mcp command's argument parser does not know about the --account flag.

For --account to work as a truly global flag, the mcp subcommand should also handle it. The account value is already extracted in main.rs, but it's not made available to the mcp_server logic when invoked via the CLI flag.

A potential solution would involve:

  1. In main.rs, setting the GOOGLE_WORKSPACE_CLI_ACCOUNT environment variable from the account variable that's already determined. This would make the --account flag's value available to any part of the code that reads this environment variable.
  2. Updating the argument parser for the mcp command (in build_mcp_cli) to accept and ignore the --account flag to prevent parsing errors.

Without this, the behavior of the --account flag is inconsistent across the CLI, which can be very confusing for users.

Ok(t) => (Some(t), crate::executor::AuthMethod::OAuth),
Err(e) => {
eprintln!(
Expand Down
Loading