fix: respect account selection in MCP server and CLI --account flag#223
Conversation
…oogleworkspace#221, googleworkspace#181) MCP server now reads GOOGLE_WORKSPACE_CLI_ACCOUNT env var and passes it to get_token instead of always using None (default account). CLI filter_args_for_subcommand now dynamically locates the service name instead of hardcoding skip(2), fixing --account before service name causing unrecognized subcommand errors.
🦋 Changeset detectedLatest commit: b53c6d7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses critical issues related to multi-account handling within the Google Workspace CLI. It enhances the MCP server's ability to respect user-defined accounts via environment variables and refines the CLI's argument parsing to correctly interpret account flags regardless of their position relative to the service name. These changes significantly improve the flexibility and reliability of account selection across the tool. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces two important fixes for argument parsing. The MCP server now correctly respects the GOOGLE_WORKSPACE_CLI_ACCOUNT environment variable for multi-account support. Additionally, the CLI is now more robust in handling the --account flag when it's placed before the service name, preventing parsing errors.
The changes are well-implemented and include corresponding test updates. I've identified one high-severity issue where the argument parsing logic for finding the service name is still incomplete, as it doesn't handle the --api-version flag appearing before the service name. Addressing this would make the global flag handling fully consistent and robust.
The first_arg loop only skipped --account but not --api-version, so `gws --api-version v3 drive ...` would misidentify --api-version as the service name. Now both global flags are consistently skipped.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces two important fixes for account selection. First, it allows the --account and --api-version global flags to be placed before the service name, which improves the CLI's usability. The argument filtering logic in src/main.rs has been correctly updated to support this. Second, the MCP server now respects the GOOGLE_WORKSPACE_CLI_ACCOUNT environment variable, which fixes an issue where it would always use the default account.
However, I've found a critical issue with the implementation. While the MCP server now respects the environment variable, it does not handle the global --account flag. This leads to an inconsistent user experience, as the flag does not work for the mcp subcommand. I've left a detailed comment on how this could be addressed.
| let account = std::env::var("GOOGLE_WORKSPACE_CLI_ACCOUNT").ok(); | ||
| let (token, auth_method) = match crate::auth::get_token(&scopes, account.as_deref()).await { |
There was a problem hiding this comment.
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:
- In
main.rs, setting theGOOGLE_WORKSPACE_CLI_ACCOUNTenvironment variable from theaccountvariable that's already determined. This would make the--accountflag's value available to any part of the code that reads this environment variable. - Updating the argument parser for the
mcpcommand (inbuild_mcp_cli) to accept and ignore the--accountflag to prevent parsing errors.
Without this, the behavior of the --account flag is inconsistent across the CLI, which can be very confusing for users.
Summary
gws mcp) now readsGOOGLE_WORKSPACE_CLI_ACCOUNTenv var and passes it toget_token, instead of always using the default account. Fixes MCP server ignores GOOGLE_WORKSPACE_CLI_ACCOUNT env var #221.--accountflag placed before the service name (e.g.gws --account user@corp.com drive files list) no longer causesunrecognized subcommanderrors.filter_args_for_subcommandnow dynamically locates the service name instead of hardcodingskip(2). Fixes Bug: multiple accounts don't work as described #181.Test plan
filter_args_for_subcommandtests updated and passingtest_filter_args_account_before_service,test_filter_args_account_equals_before_servicecargo clippy -- -D warningscleancargo testpasses (1 pre-existing failure intest_get_quota_project_reads_adcunrelated to this change)