feat(vertex): add vertexLocation config setting for Vertex AI region override#25362
feat(vertex): add vertexLocation config setting for Vertex AI region override#25362Famous077 wants to merge 33 commits intogoogle-gemini:mainfrom
Conversation
…en reader test snapshots
…on and normalizeCommand
…leRenderer and integrity
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 introduces a persistent configuration option for Vertex AI region overrides, improving usability for users accessing experimental models. Additionally, it significantly enhances the security of the shell tool by implementing robust detection for command injection vulnerabilities, alongside necessary maintenance fixes for test suites. Highlights
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. Footnotes
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
|
/gemini-review |
|
Howdy @Famous077 , it looks to me like this PR has some of the changes from #24170 that might not belong in this change set?
These should probably be reverted if they're not the main focus of this PR, and are being addressed in another one. Addressing that should appease some of the automatic reviewer's comments. Once the CLI comments are addressed, it also makes it easier to review manually if the comment threads have a response and/or resolution. I'm not sure if you have access to the same style "resolve conversation" button on the reply thread for comments & suggestions as maintainers do: For this example, after reverting changes to files unneeded for this PR, responding to the CLI's comment about command substitution with something along the lines of "Command substitution is being handled in #24170 as a separate feature. I've reverted the changes related to it in [filenames]. Marking this thread as resolved." Would be a perfect way to close out the comment so that anyone at a glance has immediate context. I'll proceed with a more in-depth review of the PR itself, but wanted to give actionable feedback for this or other PRs. Thanks for your patience and contributions! |
|
Hi @DavidAPierce , I have reverted all those files which you suggested me. Right now, all the tests are green. Before any extra changes, I want your suggestions. Can you review and let me know what are the changes I have to do more. |
|
Howdy again, thanks for your patience. The work in contentGenerator.ts and config.ts LGTM. The diff view is reporting around 300+ lines of delta in the shell related files and the seemingly unrelated changes in ExitPlanModeDialog.test.tsx.snap. I don't think these changes are needed for the fixing of #20761 which this PR is addressing. My recommendation from here would be to completely revert the changes to:
So that the diff only shows changes to files absolutely needed for the resolution of the attached issue. It looks like the initial changes to the shell files were made to fix some failing typescript checks, so if those are still failing after these files are reverted, then those failures can be looked into further. At the moment, I suspect they're a bit out of sorts from repeated merging and pulling. |
…nally into GoogleGenAI
|
Hi @DavidAPierce . Thank you for time. As per your suggestions, I have reverted all those files and some remaining build errors are pre-existing on main and unrelated to this PR. please let me know if I need to update or change anything. |
|
The diff view for this PR on github still shows 500+ line changes for:
I would recommend rebasing from main for these files so that the total files changed for this PR goes from 7 -> 4 for the main focus of this PR. I suspect that the tests and errors for the three shell files will start functioning once they're synced to what's at head in origin/main, but if they are failing because of other changes in this PR, they can be re-evalued with higher scrutiny from a position where they're what's already merged rather than having to parse the 500+ line diffs. |
d77b21f to
c9fd1a8
Compare
|
Hi @DavidAPierce , Can you review whenever you get time. Thank you |

Summary
When using Gemini CLI with Vertex AI, requests are routed to
us-central1bydefault. The problem is that preview/experimental models like
gemini-3.1-pro-previeware only released to theglobalregion first, soanyone trying to use them gets an immediate 404 error with no clear explanation
of why.
This PR adds a
vertexLocationsetting tosettings.jsonso users canoverride the region without having to set environment variables every session.
Details
The root cause was twofold:
googleCloudLocationwas being read from theGOOGLE_CLOUD_LOCATIONenvvar but never actually stored into
ContentGeneratorConfig— so even ifusers set the env var correctly, it wasn't always being honored in the Vertex
AI auth path.
There was no persistent way to set the location. Users had to remember to
export
GOOGLE_CLOUD_LOCATIONin every terminal session.The fix wires
vertexLocationfromsettings.jsonthrough the full configpipeline —
ConfigParameters→Configclass getter →createContentGeneratorConfig→GoogleGenAIclient. Settings take priorityover the env var, but the env var still works as a fallback so nothing is
broken for existing users.
Also fixed 15 pre-existing TypeScript errors in
shell.test.tswhereinvocation.execute()was being called with a rawAbortSignalinstead ofthe expected
ExecuteOptionsobject — this was breaking the build on a cleancheckout.
Related Issues
Fixes #20761
How to Validate
Option 1 — via settings.json (new behavior):
Then run with a preview model:
Expected: Request succeeds instead of returning a 404.
Option 2 — env var (existing behavior, should still work):
Option 3 — verify priority (settings wins over env var):
Set
GOOGLE_CLOUD_LOCATION=us-central1in env and"vertexLocation": "global"in settings. The request should go to
global.Run the tests:
All 27 tests should pass including the new one:
should prefer vertexLocation from config over GOOGLE_CLOUD_LOCATION env varPre-Merge Checklist