Add BYOID experiment flag and skeleton for BYOID auth flow.#27545
Add BYOID experiment flag and skeleton for BYOID auth flow.#27545DavidAPierce wants to merge 1 commit into
Conversation
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 implements the foundational infrastructure for a 'Bring Your Own IDentifier' (BYOID) authentication flow. By gating this feature behind an experimental flag, the changes ensure that the new authentication method remains opt-in and configurable via both CLI arguments and settings files. The implementation spans core configuration, CLI argument parsing, UI state management, and robust validation logic to prevent unauthorized or misconfigured use of the experimental feature. 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 the 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 counterproductive. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces experimental support for BYOID (Bring Your Own IDentifier) authentication across the CLI and core packages, including CLI argument parsing, settings schema updates, and validation logic. However, there are two critical issues in packages/core/src/config/config.ts that need to be addressed: the isExperimentalByoidEnabled() method is defined twice, which will cause a TypeScript compilation error, and the byoidConfigPath property is declared but never assigned in the constructor, rendering it always undefined at runtime.
Note: Security Review did not run due to the size of the PR.
| isExperimentalByoidEnabled(): boolean { | ||
| return this.experimentalByoid; | ||
| } | ||
|
|
||
| getByoidConfigPath(): string | undefined { | ||
| return this.byoidConfigPath; | ||
| } |
There was a problem hiding this comment.
The method isExperimentalByoidEnabled() is defined twice in the Config class (once at line 2661 and again here at line 4150). This duplicate method definition will cause a TypeScript compilation error. Please remove this duplicate definition.
| isExperimentalByoidEnabled(): boolean { | |
| return this.experimentalByoid; | |
| } | |
| getByoidConfigPath(): string | undefined { | |
| return this.byoidConfigPath; | |
| } | |
| getByoidConfigPath(): string | undefined { | |
| return this.byoidConfigPath; | |
| } |
|
|
||
| this.experimentalAutoMemory = params.experimentalAutoMemory ?? false; | ||
| this.experimentalGemma = params.experimentalGemma ?? true; | ||
| this.experimentalByoid = params.experimentalByoid ?? false; |
There was a problem hiding this comment.
The byoidConfigPath property is declared as a private readonly field on the Config class, but it is never assigned in the constructor. As a result, it will always be undefined at runtime, breaking the BYOID configuration path propagation. Please assign it in the constructor. Avoid using redundant nullish coalescing operators for configuration defaults, relying on the schema instead.
| this.experimentalByoid = params.experimentalByoid ?? false; | |
| this.byoidConfigPath = params.byoidConfigPath; |
References
- Rely on the schema as the single source of truth for configuration defaults, avoiding redundant nullish coalescing operators.
Summary
Gates the new BYOID (Bring Your Own IDentifier) authentication features behind an experimental flag and updates related tests and UI components to support the gated flow. Focuses on ensuring that experimental authentication method is not used unless explicitly enabled.
Details
experimentalByoidtoConfigParametersand theConfigclass inpackages/coreto provide a central gating mechanism.createContentGeneratorinpackages/coreto enforce the experiment flag whenAuthType.BYOIDis requested.validateAuthMethodinpackages/clito require the flag for BYOID validation to succeed.--experimental-byoidCLI argument.byoidunder experimental features.AppContainer.tsx,gemini.tsx, anduseAuth.tsto correctly handle and pass theexperimentalByoidstate.config.tsand missing dependencies inAppContainerhooks.auth.test.tsto cover all gating permutations, including verifying that explicit CLI flags can override settings state for the experiment.Related Issues
Part of the BYOID authentication feature set.
How to Validate
security.auth.selectedTypeset tobyoidwithout the experimental flag to confirm it is blocked with a descriptive error.--experimental-byoid(or enable it insettings.json) to confirm the gating is bypassed.Pre-Merge Checklist