From b00d7c88ad8d7c9f14eba4a66f04c91c85bf5ffd Mon Sep 17 00:00:00 2001 From: Aashir Javed <150792417+Aaxhirrr@users.noreply.github.com> Date: Tue, 10 Mar 2026 12:09:07 -0700 Subject: [PATCH 01/15] fix(ui): prevent empty tool-group border stubs after filtering (#21852) Co-authored-by: jacob314 --- .../messages/ToolGroupMessage.test.tsx | 244 ++++++++++++++++++ .../components/messages/ToolGroupMessage.tsx | 14 +- 2 files changed, 253 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index 8971d488d3..d5cbdabe60 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -69,6 +69,11 @@ describe('', () => { ui: { errorVerbosity: 'full' }, }, }); + const lowVerbositySettings = createMockSettings({ + merged: { + ui: { errorVerbosity: 'low' }, + }, + }); describe('Golden Snapshots', () => { it('renders single successful tool call', async () => { @@ -721,6 +726,245 @@ describe('', () => { expect(lastFrame({ allowEmpty: true })).toBe(''); unmount(); }); + + it('does not render a bottom-border fragment when all tools are filtered out', async () => { + const toolCalls = [ + createToolCall({ + callId: 'hidden-error-tool', + name: 'error-tool', + status: CoreToolCallStatus.Error, + resultDisplay: 'Hidden in low verbosity', + isClientInitiated: false, + }), + ]; + const item = createItem(toolCalls); + + const { lastFrame, unmount, waitUntilReady } = renderWithProviders( + , + { + config: baseMockConfig, + settings: lowVerbositySettings, + }, + ); + + await waitUntilReady(); + expect(lastFrame({ allowEmpty: true })).toBe(''); + unmount(); + }); + + it('still renders explicit closing slices for split static/pending groups', async () => { + const toolCalls: IndividualToolCallDisplay[] = []; + const item = createItem(toolCalls); + + const { lastFrame, unmount, waitUntilReady } = renderWithProviders( + , + { + config: baseMockConfig, + settings: fullVerbositySettings, + }, + ); + + await waitUntilReady(); + expect(lastFrame({ allowEmpty: true })).not.toBe(''); + unmount(); + }); + + it('does not render a border fragment when plan-mode tools are filtered out', async () => { + const toolCalls = [ + createToolCall({ + callId: 'plan-write', + name: WRITE_FILE_DISPLAY_NAME, + approvalMode: ApprovalMode.PLAN, + status: CoreToolCallStatus.Success, + resultDisplay: 'Plan file written', + }), + ]; + const item = createItem(toolCalls); + + const { lastFrame, unmount, waitUntilReady } = renderWithProviders( + , + { + config: baseMockConfig, + settings: fullVerbositySettings, + }, + ); + + await waitUntilReady(); + expect(lastFrame({ allowEmpty: true })).toBe(''); + unmount(); + }); + + it('does not render a border fragment when only confirming tools are present', async () => { + const toolCalls = [ + createToolCall({ + callId: 'confirm-only', + status: CoreToolCallStatus.AwaitingApproval, + confirmationDetails: { + type: 'info', + title: 'Confirm', + prompt: 'Proceed?', + }, + }), + ]; + const item = createItem(toolCalls); + + const { lastFrame, unmount, waitUntilReady } = renderWithProviders( + , + { + config: baseMockConfig, + settings: fullVerbositySettings, + }, + ); + + await waitUntilReady(); + expect(lastFrame({ allowEmpty: true })).toBe(''); + unmount(); + }); + + it('does not leave a border stub when transitioning from visible to fully filtered tools', async () => { + const visibleTools = [ + createToolCall({ + callId: 'visible-success', + name: 'visible-tool', + status: CoreToolCallStatus.Success, + resultDisplay: 'visible output', + }), + ]; + const hiddenTools = [ + createToolCall({ + callId: 'hidden-error', + name: 'hidden-error-tool', + status: CoreToolCallStatus.Error, + resultDisplay: 'hidden output', + isClientInitiated: false, + }), + ]; + + const initialItem = createItem(visibleTools); + const hiddenItem = createItem(hiddenTools); + + const firstRender = renderWithProviders( + , + { + config: baseMockConfig, + settings: lowVerbositySettings, + }, + ); + await firstRender.waitUntilReady(); + expect(firstRender.lastFrame()).toContain('visible-tool'); + firstRender.unmount(); + + const secondRender = renderWithProviders( + , + { + config: baseMockConfig, + settings: lowVerbositySettings, + }, + ); + await secondRender.waitUntilReady(); + expect(secondRender.lastFrame({ allowEmpty: true })).toBe(''); + secondRender.unmount(); + }); + + it('keeps visible tools rendered with many filtered tools (stress case)', async () => { + const visibleTool = createToolCall({ + callId: 'visible-tool', + name: 'visible-tool', + status: CoreToolCallStatus.Success, + resultDisplay: 'visible output', + }); + const hiddenTools = Array.from({ length: 50 }, (_, index) => + createToolCall({ + callId: `hidden-${index}`, + name: `hidden-error-${index}`, + status: CoreToolCallStatus.Error, + resultDisplay: `hidden output ${index}`, + isClientInitiated: false, + }), + ); + const toolCalls = [visibleTool, ...hiddenTools]; + const item = createItem(toolCalls); + + const { lastFrame, unmount, waitUntilReady } = renderWithProviders( + , + { + config: baseMockConfig, + settings: lowVerbositySettings, + }, + ); + + await waitUntilReady(); + const output = lastFrame(); + expect(output).toContain('visible-tool'); + expect(output).not.toContain('hidden-error-0'); + expect(output).not.toContain('hidden-error-49'); + unmount(); + }); + + it('renders explicit closing slice even at very narrow terminal width', async () => { + const toolCalls: IndividualToolCallDisplay[] = []; + const item = createItem(toolCalls); + + const { lastFrame, unmount, waitUntilReady } = renderWithProviders( + , + { + config: baseMockConfig, + settings: fullVerbositySettings, + }, + ); + + await waitUntilReady(); + expect(lastFrame({ allowEmpty: true })).not.toBe(''); + unmount(); + }); }); describe('Plan Mode Filtering', () => { diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 05f9984d69..01cec31727 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -141,11 +141,15 @@ export const ToolGroupMessage: React.FC = ({ const contentWidth = terminalWidth - TOOL_MESSAGE_HORIZONTAL_MARGIN; - // If all tools are filtered out (e.g., in-progress AskUser tools, confirming tools), - // only render if we need to close a border from previous - // tool groups. borderBottomOverride=true means we must render the closing border; - // undefined or false means there's nothing to display. - if (visibleToolCalls.length === 0 && borderBottomOverride !== true) { + // If all tools are filtered out (e.g., in-progress AskUser tools, low-verbosity + // internal errors, plan-mode hidden write/edit), we should not emit standalone + // border fragments. The only case where an empty group should render is the + // explicit "closing slice" (tools: []) used to bridge static/pending sections. + const isExplicitClosingSlice = allToolCalls.length === 0; + if ( + visibleToolCalls.length === 0 && + (!isExplicitClosingSlice || borderBottomOverride !== true) + ) { return null; } From b404fc02e755910f47e2efc6a752578d44232cee Mon Sep 17 00:00:00 2001 From: Mark McLaughlin Date: Tue, 10 Mar 2026 12:10:26 -0700 Subject: [PATCH 02/15] fix(auth): update terminology to 'sign in' and 'sign out' (#20892) Co-authored-by: Jacob Richman --- README.md | 4 ++-- docs/cli/settings.md | 2 +- docs/core/subagents.md | 2 +- docs/get-started/authentication.md | 14 ++++++------ docs/get-started/index.md | 2 +- docs/reference/configuration.md | 2 +- docs/release-confidence.md | 4 ++-- docs/resources/tos-privacy.md | 6 ++--- docs/resources/troubleshooting.md | 4 ++-- packages/cli/src/config/settingsSchema.ts | 2 +- packages/cli/src/core/auth.test.ts | 4 ++-- packages/cli/src/core/auth.ts | 2 +- packages/cli/src/ui/auth/AuthDialog.test.tsx | 4 ++-- packages/cli/src/ui/auth/AuthDialog.tsx | 2 +- .../cli/src/ui/auth/AuthInProgress.test.tsx | 4 ++-- packages/cli/src/ui/auth/AuthInProgress.tsx | 4 ++-- .../ui/auth/LoginWithGoogleRestartDialog.tsx | 6 ++--- .../__snapshots__/AuthDialog.test.tsx.snap | 4 ++-- ...LoginWithGoogleRestartDialog.test.tsx.snap | 4 ++-- packages/cli/src/ui/auth/useAuth.test.tsx | 2 +- packages/cli/src/ui/auth/useAuth.ts | 2 +- .../cli/src/ui/commands/authCommand.test.ts | 16 ++++++++------ packages/cli/src/ui/commands/authCommand.ts | 10 +++++---- .../cli/src/ui/components/AboutBox.test.tsx | 4 ++-- packages/cli/src/ui/components/AboutBox.tsx | 4 ++-- .../LogoutConfirmationDialog.test.tsx | 6 ++--- .../components/LogoutConfirmationDialog.tsx | 6 ++--- .../ui/components/ModelStatsDisplay.test.tsx | 2 +- .../src/ui/components/ModelStatsDisplay.tsx | 4 ++-- .../src/ui/components/StatsDisplay.test.tsx | 2 +- .../cli/src/ui/components/StatsDisplay.tsx | 4 ++-- .../src/ui/components/UserIdentity.test.tsx | 22 ++++++++++++++----- .../cli/src/ui/components/UserIdentity.tsx | 7 ++++-- .../src/ui/components/ValidationDialog.tsx | 2 +- schemas/settings.schema.json | 4 ++-- 35 files changed, 95 insertions(+), 78 deletions(-) diff --git a/README.md b/README.md index 2b25865179..93485498ed 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,7 @@ Integrate Gemini CLI directly into your GitHub workflows with Choose the authentication method that best fits your needs: -### Option 1: Login with Google (OAuth login using your Google Account) +### Option 1: Sign in with Google (OAuth login using your Google Account) **✨ Best for:** Individual developers as well as anyone who has a Gemini Code Assist License. (see @@ -161,7 +161,7 @@ for details) - **No API key management** - just sign in with your Google account - **Automatic updates** to latest models -#### Start Gemini CLI, then choose _Login with Google_ and follow the browser authentication flow when prompted +#### Start Gemini CLI, then choose _Sign in with Google_ and follow the browser authentication flow when prompted ```bash gemini diff --git a/docs/cli/settings.md b/docs/cli/settings.md index 0f4b44f159..33f585ca2a 100644 --- a/docs/cli/settings.md +++ b/docs/cli/settings.md @@ -66,7 +66,7 @@ they appear in the UI. | Show Line Numbers | `ui.showLineNumbers` | Show line numbers in the chat. | `true` | | Show Citations | `ui.showCitations` | Show citations for generated text in the chat. | `false` | | Show Model Info In Chat | `ui.showModelInfoInChat` | Show the model name in the chat for each model turn. | `false` | -| Show User Identity | `ui.showUserIdentity` | Show the logged-in user's identity (e.g. email) in the UI. | `true` | +| Show User Identity | `ui.showUserIdentity` | Show the signed-in user's identity (e.g. email) in the UI. | `true` | | Use Alternate Screen Buffer | `ui.useAlternateBuffer` | Use an alternate screen buffer for the UI, preserving shell history. | `false` | | Use Background Color | `ui.useBackgroundColor` | Whether to use background colors in the UI. | `true` | | Incremental Rendering | `ui.incrementalRendering` | Enable incremental rendering for the UI. This option will reduce flickering but may cause rendering artifacts. Only supported when useAlternateBuffer is enabled. | `true` | diff --git a/docs/core/subagents.md b/docs/core/subagents.md index 37085569af..e464566c01 100644 --- a/docs/core/subagents.md +++ b/docs/core/subagents.md @@ -194,7 +194,7 @@ returns coordinates and element descriptions that the browser agent uses with the `click_at` tool for precise, coordinate-based interactions. > **Note:** The visual agent requires API key or Vertex AI authentication. It is -> not available when using Google Login. +> not available when using "Sign in with Google". ## Creating custom subagents diff --git a/docs/get-started/authentication.md b/docs/get-started/authentication.md index bc603bbdf3..964e776567 100644 --- a/docs/get-started/authentication.md +++ b/docs/get-started/authentication.md @@ -17,8 +17,8 @@ Select the authentication method that matches your situation in the table below: | User Type / Scenario | Recommended Authentication Method | Google Cloud Project Required | | :--------------------------------------------------------------------- | :--------------------------------------------------------------- | :---------------------------------------------------------- | -| Individual Google accounts | [Login with Google](#login-google) | No, with exceptions | -| Organization users with a company, school, or Google Workspace account | [Login with Google](#login-google) | [Yes](#set-gcp) | +| Individual Google accounts | [Sign in with Google](#login-google) | No, with exceptions | +| Organization users with a company, school, or Google Workspace account | [Sign in with Google](#login-google) | [Yes](#set-gcp) | | AI Studio user with a Gemini API key | [Use Gemini API Key](#gemini-api) | No | | Google Cloud Vertex AI user | [Vertex AI](#vertex-ai) | [Yes](#set-gcp) | | [Headless mode](#headless) | [Use Gemini API Key](#gemini-api) or
[Vertex AI](#vertex-ai) | No (for Gemini API Key)
[Yes](#set-gcp) (for Vertex AI) | @@ -36,7 +36,7 @@ Select the authentication method that matches your situation in the table below: [Google AI Ultra for Business](https://support.google.com/a/answer/16345165) subscriptions. -## (Recommended) Login with Google +## (Recommended) Sign in with Google If you run Gemini CLI on your local machine, the simplest authentication method is logging in with your Google account. This method requires a web browser on a @@ -54,9 +54,9 @@ To authenticate and use Gemini CLI: gemini ``` -2. Select **Login with Google**. Gemini CLI opens a login prompt using your web - browser. Follow the on-screen instructions. Your credentials will be cached - locally for future sessions. +2. Select **Sign in with Google**. Gemini CLI opens a sign in prompt using your + web browser. Follow the on-screen instructions. Your credentials will be + cached locally for future sessions. ### Do I need to set my Google Cloud project? @@ -391,7 +391,7 @@ on this page. [Headless mode](../cli/headless) will use your existing authentication method, if an existing authentication credential is cached. -If you have not already logged in with an authentication credential, you must +If you have not already signed in with an authentication credential, you must configure authentication using environment variables: - [Use Gemini API Key](#gemini-api) diff --git a/docs/get-started/index.md b/docs/get-started/index.md index c516f90ac4..566ac6e9df 100644 --- a/docs/get-started/index.md +++ b/docs/get-started/index.md @@ -38,7 +38,7 @@ cases, you can log in with your existing Google account: ``` 2. When asked "How would you like to authenticate for this project?" select **1. - Login with Google**. + Sign in with Google**. 3. Select your Google account. diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 39870262c9..4b50b99280 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -297,7 +297,7 @@ their corresponding top-level category object in your `settings.json` file. - **Default:** `false` - **`ui.showUserIdentity`** (boolean): - - **Description:** Show the logged-in user's identity (e.g. email) in the UI. + - **Description:** Show the signed-in user's identity (e.g. email) in the UI. - **Default:** `true` - **`ui.useAlternateBuffer`** (boolean): diff --git a/docs/release-confidence.md b/docs/release-confidence.md index f2dcccff4f..536e49772c 100644 --- a/docs/release-confidence.md +++ b/docs/release-confidence.md @@ -79,8 +79,8 @@ manually run through this checklist. - [ ] Verify version: `gemini --version` - **Authentication:** - - [ ] In interactive mode run `/auth` and verify all login flows work: - - [ ] Login With Google + - [ ] In interactive mode run `/auth` and verify all sign in flows work: + - [ ] Sign in with Google - [ ] API Key - [ ] Vertex AI diff --git a/docs/resources/tos-privacy.md b/docs/resources/tos-privacy.md index 98d4a58b98..00de950e74 100644 --- a/docs/resources/tos-privacy.md +++ b/docs/resources/tos-privacy.md @@ -46,7 +46,7 @@ for further information. | Gemini Developer API Key | Gemini API - Paid Services | [Gemini API Terms of Service - Paid Services](https://ai.google.dev/gemini-api/terms#paid-services) | [Google Privacy Policy](https://policies.google.com/privacy) | | Vertex AI GenAI API Key | Vertex AI GenAI API | [Google Cloud Platform Terms of Service](https://cloud.google.com/terms/service-terms/) | [Google Cloud Privacy Notice](https://cloud.google.com/terms/cloud-privacy-notice) | -## 1. If you have logged in with your Google account to Gemini Code Assist +## 1. If you have signed in with your Google account to Gemini Code Assist For users who use their Google account to access [Gemini Code Assist](https://codeassist.google), these Terms of Service and @@ -68,7 +68,7 @@ Code Assist Standard or Enterprise edition, the terms and privacy policy of Gemini Code Assist Standard or Enterprise edition will apply to all your use of Gemini Code Assist._ -## 2. If you have logged in with a Gemini API key to the Gemini Developer API +## 2. If you have signed in with a Gemini API key to the Gemini Developer API If you are using a Gemini API key for authentication with the [Gemini Developer API](https://ai.google.dev/gemini-api/docs), these Terms of @@ -84,7 +84,7 @@ Service and Privacy Notice documents apply: - Privacy Notice: The collection and use of your data is described in the [Google Privacy Policy](https://policies.google.com/privacy). -## 3. If you have logged in with a Gemini API key to the Vertex AI GenAI API +## 3. If you have signed in with a Gemini API key to the Vertex AI GenAI API If you are using a Gemini API key for authentication with a [Vertex AI GenAI API](https://cloud.google.com/vertex-ai/generative-ai/docs/reference/rest) diff --git a/docs/resources/troubleshooting.md b/docs/resources/troubleshooting.md index ea6341a0d6..3a7cd35b19 100644 --- a/docs/resources/troubleshooting.md +++ b/docs/resources/troubleshooting.md @@ -29,13 +29,13 @@ topics on: added to your organization's Gemini Code Assist subscription. - **Error: - `Failed to login. Message: Your current account is not eligible... because it is not currently available in your location.`** + `Failed to sign in. Message: Your current account is not eligible... because it is not currently available in your location.`** - **Cause:** Gemini CLI does not currently support your location. For a full list of supported locations, see the following pages: - Gemini Code Assist for individuals: [Available locations](https://developers.google.com/gemini-code-assist/resources/available-locations#americas) -- **Error: `Failed to login. Message: Request contains an invalid argument`** +- **Error: `Failed to sign in. Message: Request contains an invalid argument`** - **Cause:** Users with Google Workspace accounts or Google Cloud accounts associated with their Gmail accounts may not be able to activate the free tier of the Google Code Assist plan. diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 0e96c88b24..2b4685cf81 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -676,7 +676,7 @@ const SETTINGS_SCHEMA = { requiresRestart: false, default: true, description: - "Show the logged-in user's identity (e.g. email) in the UI.", + "Show the signed-in user's identity (e.g. email) in the UI.", showInDialog: true, }, useAlternateBuffer: { diff --git a/packages/cli/src/core/auth.test.ts b/packages/cli/src/core/auth.test.ts index 5db9cd5449..639ed20a89 100644 --- a/packages/cli/src/core/auth.test.ts +++ b/packages/cli/src/core/auth.test.ts @@ -48,14 +48,14 @@ describe('auth', () => { }); it('should return error message on failed auth', async () => { - const error = new Error('Auth failed'); + const error = new Error('Authentication failed'); vi.mocked(mockConfig.refreshAuth).mockRejectedValue(error); const result = await performInitialAuth( mockConfig, AuthType.LOGIN_WITH_GOOGLE, ); expect(result).toEqual({ - authError: 'Failed to login. Message: Auth failed', + authError: 'Failed to sign in. Message: Authentication failed', accountSuspensionInfo: null, }); expect(mockConfig.refreshAuth).toHaveBeenCalledWith( diff --git a/packages/cli/src/core/auth.ts b/packages/cli/src/core/auth.ts index f0b8015013..0bc89f5bda 100644 --- a/packages/cli/src/core/auth.ts +++ b/packages/cli/src/core/auth.ts @@ -64,7 +64,7 @@ export async function performInitialAuth( }; } return { - authError: `Failed to login. Message: ${getErrorMessage(e)}`, + authError: `Failed to sign in. Message: ${getErrorMessage(e)}`, accountSuspensionInfo: null, }; } diff --git a/packages/cli/src/ui/auth/AuthDialog.test.tsx b/packages/cli/src/ui/auth/AuthDialog.test.tsx index c157a6a40d..7ab5fc0be2 100644 --- a/packages/cli/src/ui/auth/AuthDialog.test.tsx +++ b/packages/cli/src/ui/auth/AuthDialog.test.tsx @@ -209,7 +209,7 @@ describe('AuthDialog', () => { { setup: () => {}, expected: AuthType.LOGIN_WITH_GOOGLE, - desc: 'defaults to Login with Google', + desc: 'defaults to Sign in with Google', }, ])('selects initial auth type $desc', async ({ setup, expected }) => { setup(); @@ -351,7 +351,7 @@ describe('AuthDialog', () => { unmount(); }); - it('exits process for Login with Google when browser is suppressed', async () => { + it('exits process for Sign in with Google when browser is suppressed', async () => { vi.useFakeTimers(); const exitSpy = vi .spyOn(process, 'exit') diff --git a/packages/cli/src/ui/auth/AuthDialog.tsx b/packages/cli/src/ui/auth/AuthDialog.tsx index 58956e5f86..4e523d6b11 100644 --- a/packages/cli/src/ui/auth/AuthDialog.tsx +++ b/packages/cli/src/ui/auth/AuthDialog.tsx @@ -44,7 +44,7 @@ export function AuthDialog({ const [exiting, setExiting] = useState(false); let items = [ { - label: 'Login with Google', + label: 'Sign in with Google', value: AuthType.LOGIN_WITH_GOOGLE, key: AuthType.LOGIN_WITH_GOOGLE, }, diff --git a/packages/cli/src/ui/auth/AuthInProgress.test.tsx b/packages/cli/src/ui/auth/AuthInProgress.test.tsx index 7f279a1067..bd6a3cb126 100644 --- a/packages/cli/src/ui/auth/AuthInProgress.test.tsx +++ b/packages/cli/src/ui/auth/AuthInProgress.test.tsx @@ -59,8 +59,8 @@ describe('AuthInProgress', () => { , ); await waitUntilReady(); - expect(lastFrame()).toContain('[Spinner] Waiting for auth...'); - expect(lastFrame()).toContain('Press ESC or CTRL+C to cancel'); + expect(lastFrame()).toContain('[Spinner] Waiting for authentication...'); + expect(lastFrame()).toContain('Press Esc or Ctrl+C to cancel'); unmount(); }); diff --git a/packages/cli/src/ui/auth/AuthInProgress.tsx b/packages/cli/src/ui/auth/AuthInProgress.tsx index f5c5d7db6e..03d609c444 100644 --- a/packages/cli/src/ui/auth/AuthInProgress.tsx +++ b/packages/cli/src/ui/auth/AuthInProgress.tsx @@ -53,8 +53,8 @@ export function AuthInProgress({ ) : ( - Waiting for auth... (Press ESC or CTRL+C - to cancel) + Waiting for authentication... (Press Esc + or Ctrl+C to cancel) )} diff --git a/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.tsx b/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.tsx index 94ca359b59..a781828d09 100644 --- a/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.tsx +++ b/packages/cli/src/ui/auth/LoginWithGoogleRestartDialog.tsx @@ -45,13 +45,13 @@ export const LoginWithGoogleRestartDialog = ({ ); const message = - 'You have successfully logged in with Google. Gemini CLI needs to be restarted.'; + "You've successfully signed in with Google. Gemini CLI needs to be restarted."; return ( - {message} Press 'r' to restart, or 'escape' to - choose a different auth method. + {message} Press R to restart, or Esc to choose a different + authentication method. ); diff --git a/packages/cli/src/ui/auth/__snapshots__/AuthDialog.test.tsx.snap b/packages/cli/src/ui/auth/__snapshots__/AuthDialog.test.tsx.snap index 2d341c405e..05bc9f422e 100644 --- a/packages/cli/src/ui/auth/__snapshots__/AuthDialog.test.tsx.snap +++ b/packages/cli/src/ui/auth/__snapshots__/AuthDialog.test.tsx.snap @@ -7,7 +7,7 @@ exports[`AuthDialog > Snapshots > renders correctly with auth error 1`] = ` │ │ │ How would you like to authenticate for this project? │ │ │ -│ (selected) Login with Google(not selected) Use Gemini API Key(not selected) Vertex AI │ +│ (selected) Sign in with Google(not selected) Use Gemini API Key(not selected) Vertex AI │ │ │ │ Something went wrong │ │ │ @@ -28,7 +28,7 @@ exports[`AuthDialog > Snapshots > renders correctly with default props 1`] = ` │ │ │ How would you like to authenticate for this project? │ │ │ -│ (selected) Login with Google(not selected) Use Gemini API Key(not selected) Vertex AI │ +│ (selected) Sign in with Google(not selected) Use Gemini API Key(not selected) Vertex AI │ │ │ │ (Use Enter to select) │ │ │ diff --git a/packages/cli/src/ui/auth/__snapshots__/LoginWithGoogleRestartDialog.test.tsx.snap b/packages/cli/src/ui/auth/__snapshots__/LoginWithGoogleRestartDialog.test.tsx.snap index 20fad6d488..7c7a95e24f 100644 --- a/packages/cli/src/ui/auth/__snapshots__/LoginWithGoogleRestartDialog.test.tsx.snap +++ b/packages/cli/src/ui/auth/__snapshots__/LoginWithGoogleRestartDialog.test.tsx.snap @@ -2,8 +2,8 @@ exports[`LoginWithGoogleRestartDialog > renders correctly 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ You have successfully logged in with Google. Gemini CLI needs to be restarted. Press 'r' to │ -│ restart, or 'escape' to choose a different auth method. │ +│ You've successfully signed in with Google. Gemini CLI needs to be restarted. Press R to restart, │ +│ or Esc to choose a different authentication method. │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ " `; diff --git a/packages/cli/src/ui/auth/useAuth.test.tsx b/packages/cli/src/ui/auth/useAuth.test.tsx index 20a02ffb21..f236428ff1 100644 --- a/packages/cli/src/ui/auth/useAuth.test.tsx +++ b/packages/cli/src/ui/auth/useAuth.test.tsx @@ -288,7 +288,7 @@ describe('useAuth', () => { ); await waitFor(() => { - expect(result.current.authError).toContain('Failed to login'); + expect(result.current.authError).toContain('Failed to sign in'); expect(result.current.authState).toBe(AuthState.Updating); }); }); diff --git a/packages/cli/src/ui/auth/useAuth.ts b/packages/cli/src/ui/auth/useAuth.ts index afd438bb00..809a3b34b8 100644 --- a/packages/cli/src/ui/auth/useAuth.ts +++ b/packages/cli/src/ui/auth/useAuth.ts @@ -149,7 +149,7 @@ export const useAuthCommand = ( // Show the error message directly without "Failed to login" prefix onAuthError(getErrorMessage(e)); } else { - onAuthError(`Failed to login. Message: ${getErrorMessage(e)}`); + onAuthError(`Failed to sign in. Message: ${getErrorMessage(e)}`); } } })(); diff --git a/packages/cli/src/ui/commands/authCommand.test.ts b/packages/cli/src/ui/commands/authCommand.test.ts index ba1e369b14..88e3273c8d 100644 --- a/packages/cli/src/ui/commands/authCommand.test.ts +++ b/packages/cli/src/ui/commands/authCommand.test.ts @@ -34,11 +34,13 @@ describe('authCommand', () => { vi.clearAllMocks(); }); - it('should have subcommands: login and logout', () => { + it('should have subcommands: signin and signout', () => { expect(authCommand.subCommands).toBeDefined(); expect(authCommand.subCommands).toHaveLength(2); - expect(authCommand.subCommands?.[0]?.name).toBe('login'); - expect(authCommand.subCommands?.[1]?.name).toBe('logout'); + expect(authCommand.subCommands?.[0]?.name).toBe('signin'); + expect(authCommand.subCommands?.[0]?.altNames).toContain('login'); + expect(authCommand.subCommands?.[1]?.name).toBe('signout'); + expect(authCommand.subCommands?.[1]?.altNames).toContain('logout'); }); it('should return a dialog action to open the auth dialog when called with no args', () => { @@ -59,19 +61,19 @@ describe('authCommand', () => { expect(authCommand.description).toBe('Manage authentication'); }); - describe('auth login subcommand', () => { + describe('auth signin subcommand', () => { it('should return auth dialog action', () => { const loginCommand = authCommand.subCommands?.[0]; - expect(loginCommand?.name).toBe('login'); + expect(loginCommand?.name).toBe('signin'); const result = loginCommand!.action!(mockContext, ''); expect(result).toEqual({ type: 'dialog', dialog: 'auth' }); }); }); - describe('auth logout subcommand', () => { + describe('auth signout subcommand', () => { it('should clear cached credentials', async () => { const logoutCommand = authCommand.subCommands?.[1]; - expect(logoutCommand?.name).toBe('logout'); + expect(logoutCommand?.name).toBe('signout'); const { clearCachedCredentialFile } = await import( '@google/gemini-cli-core' diff --git a/packages/cli/src/ui/commands/authCommand.ts b/packages/cli/src/ui/commands/authCommand.ts index 0314555baf..80c432894c 100644 --- a/packages/cli/src/ui/commands/authCommand.ts +++ b/packages/cli/src/ui/commands/authCommand.ts @@ -14,8 +14,9 @@ import { clearCachedCredentialFile } from '@google/gemini-cli-core'; import { SettingScope } from '../../config/settings.js'; const authLoginCommand: SlashCommand = { - name: 'login', - description: 'Login or change the auth method', + name: 'signin', + altNames: ['login'], + description: 'Sign in or change the authentication method', kind: CommandKind.BUILT_IN, autoExecute: true, action: (_context, _args): OpenDialogActionReturn => ({ @@ -25,8 +26,9 @@ const authLoginCommand: SlashCommand = { }; const authLogoutCommand: SlashCommand = { - name: 'logout', - description: 'Log out and clear all cached credentials', + name: 'signout', + altNames: ['logout'], + description: 'Sign out and clear all cached credentials', kind: CommandKind.BUILT_IN, action: async (context, _args): Promise => { await clearCachedCredentialFile(); diff --git a/packages/cli/src/ui/components/AboutBox.test.tsx b/packages/cli/src/ui/components/AboutBox.test.tsx index b7a615a18f..3f1226b651 100644 --- a/packages/cli/src/ui/components/AboutBox.test.tsx +++ b/packages/cli/src/ui/components/AboutBox.test.tsx @@ -36,7 +36,7 @@ describe('AboutBox', () => { expect(output).toContain('gemini-pro'); expect(output).toContain('default'); expect(output).toContain('macOS'); - expect(output).toContain('Logged in with Google'); + expect(output).toContain('Signed in with Google'); unmount(); }); @@ -63,7 +63,7 @@ describe('AboutBox', () => { ); await waitUntilReady(); const output = lastFrame(); - expect(output).toContain('Logged in with Google (test@example.com)'); + expect(output).toContain('Signed in with Google (test@example.com)'); unmount(); }); diff --git a/packages/cli/src/ui/components/AboutBox.tsx b/packages/cli/src/ui/components/AboutBox.tsx index 7ea744b0fe..aa5fd44c57 100644 --- a/packages/cli/src/ui/components/AboutBox.tsx +++ b/packages/cli/src/ui/components/AboutBox.tsx @@ -116,8 +116,8 @@ export const AboutBox: React.FC = ({ {selectedAuthType.startsWith('oauth') ? userEmail - ? `Logged in with Google (${userEmail})` - : 'Logged in with Google' + ? `Signed in with Google (${userEmail})` + : 'Signed in with Google' : selectedAuthType} diff --git a/packages/cli/src/ui/components/LogoutConfirmationDialog.test.tsx b/packages/cli/src/ui/components/LogoutConfirmationDialog.test.tsx index e080806678..ae17922c9d 100644 --- a/packages/cli/src/ui/components/LogoutConfirmationDialog.test.tsx +++ b/packages/cli/src/ui/components/LogoutConfirmationDialog.test.tsx @@ -28,9 +28,9 @@ describe('LogoutConfirmationDialog', () => { ); await waitUntilReady(); - expect(lastFrame()).toContain('You are now logged out.'); + expect(lastFrame()).toContain('You are now signed out'); expect(lastFrame()).toContain( - 'Login again to continue using Gemini CLI, or exit the application.', + 'Sign in again to continue using Gemini CLI, or exit the application.', ); expect(lastFrame()).toContain('(Use Enter to select, Esc to close)'); unmount(); @@ -45,7 +45,7 @@ describe('LogoutConfirmationDialog', () => { expect(RadioButtonSelect).toHaveBeenCalled(); const mockCall = vi.mocked(RadioButtonSelect).mock.calls[0][0]; expect(mockCall.items).toEqual([ - { label: 'Login', value: LogoutChoice.LOGIN, key: 'login' }, + { label: 'Sign in', value: LogoutChoice.LOGIN, key: 'login' }, { label: 'Exit', value: LogoutChoice.EXIT, key: 'exit' }, ]); expect(mockCall.isFocused).toBe(true); diff --git a/packages/cli/src/ui/components/LogoutConfirmationDialog.tsx b/packages/cli/src/ui/components/LogoutConfirmationDialog.tsx index 3bcb4a9f35..fbe4c30bd0 100644 --- a/packages/cli/src/ui/components/LogoutConfirmationDialog.tsx +++ b/packages/cli/src/ui/components/LogoutConfirmationDialog.tsx @@ -37,7 +37,7 @@ export const LogoutConfirmationDialog: React.FC< const options: Array> = [ { - label: 'Login', + label: 'Sign in', value: LogoutChoice.LOGIN, key: 'login', }, @@ -61,10 +61,10 @@ export const LogoutConfirmationDialog: React.FC< > - You are now logged out. + You are now signed out - Login again to continue using Gemini CLI, or exit the application. + Sign in again to continue using Gemini CLI, or exit the application. diff --git a/packages/cli/src/ui/components/ModelStatsDisplay.test.tsx b/packages/cli/src/ui/components/ModelStatsDisplay.test.tsx index d47c2cca96..cd6961b742 100644 --- a/packages/cli/src/ui/components/ModelStatsDisplay.test.tsx +++ b/packages/cli/src/ui/components/ModelStatsDisplay.test.tsx @@ -539,7 +539,7 @@ describe('', () => { const output = lastFrame(); expect(output).toContain('Auth Method:'); - expect(output).toContain('Logged in with Google'); + expect(output).toContain('Signed in with Google'); expect(output).toContain('(test@example.com)'); expect(output).toContain('Tier:'); expect(output).toContain('Pro'); diff --git a/packages/cli/src/ui/components/ModelStatsDisplay.tsx b/packages/cli/src/ui/components/ModelStatsDisplay.tsx index eec58e9968..0c6ae45e8c 100644 --- a/packages/cli/src/ui/components/ModelStatsDisplay.tsx +++ b/packages/cli/src/ui/components/ModelStatsDisplay.tsx @@ -340,8 +340,8 @@ export const ModelStatsDisplay: React.FC = ({ {selectedAuthType.startsWith('oauth') ? userEmail - ? `Logged in with Google (${userEmail})` - : 'Logged in with Google' + ? `Signed in with Google (${userEmail})` + : 'Signed in with Google' : selectedAuthType} diff --git a/packages/cli/src/ui/components/StatsDisplay.test.tsx b/packages/cli/src/ui/components/StatsDisplay.test.tsx index 0a3c5eca21..2b4422e69c 100644 --- a/packages/cli/src/ui/components/StatsDisplay.test.tsx +++ b/packages/cli/src/ui/components/StatsDisplay.test.tsx @@ -616,7 +616,7 @@ describe('', () => { const output = lastFrame(); expect(output).toContain('Auth Method:'); - expect(output).toContain('Logged in with Google (test@example.com)'); + expect(output).toContain('Signed in with Google (test@example.com)'); expect(output).toContain('Tier:'); expect(output).toContain('Pro'); }); diff --git a/packages/cli/src/ui/components/StatsDisplay.tsx b/packages/cli/src/ui/components/StatsDisplay.tsx index f26c9a3ea5..d369374d95 100644 --- a/packages/cli/src/ui/components/StatsDisplay.tsx +++ b/packages/cli/src/ui/components/StatsDisplay.tsx @@ -589,8 +589,8 @@ export const StatsDisplay: React.FC = ({ {selectedAuthType.startsWith('oauth') ? userEmail - ? `Logged in with Google (${userEmail})` - : 'Logged in with Google' + ? `Signed in with Google (${userEmail})` + : 'Signed in with Google' : selectedAuthType} diff --git a/packages/cli/src/ui/components/UserIdentity.test.tsx b/packages/cli/src/ui/components/UserIdentity.test.tsx index aa7f4d3da2..2aade5675b 100644 --- a/packages/cli/src/ui/components/UserIdentity.test.tsx +++ b/packages/cli/src/ui/components/UserIdentity.test.tsx @@ -1,6 +1,6 @@ /** * @license - * Copyright 2025 Google LLC + * Copyright 2026 Google LLC * SPDX-License-Identifier: Apache-2.0 */ @@ -45,7 +45,7 @@ describe('', () => { await waitUntilReady(); const output = lastFrame(); - expect(output).toContain('test@example.com'); + expect(output).toContain('Signed in with Google: test@example.com'); expect(output).toContain('/auth'); expect(output).not.toContain('/upgrade'); unmount(); @@ -91,7 +91,8 @@ describe('', () => { await waitUntilReady(); const output = lastFrame(); - expect(output).toContain('Logged in with Google'); + expect(output).toContain('Signed in with Google'); + expect(output).not.toContain('Signed in with Google:'); expect(output).toContain('/auth'); expect(output).not.toContain('/upgrade'); unmount(); @@ -111,11 +112,20 @@ describe('', () => { await waitUntilReady(); const output = lastFrame(); - expect(output).toContain('test@example.com'); + expect(output).toContain('Signed in with Google: test@example.com'); expect(output).toContain('/auth'); - expect(output).toContain('Premium Plan'); + expect(output).toContain('Plan: Premium Plan'); expect(output).toContain('/upgrade'); + // Check for two lines (or more if wrapped, but here it should be separate) + const lines = output?.split('\n').filter((line) => line.trim().length > 0); + expect(lines?.some((line) => line.includes('Signed in with Google'))).toBe( + true, + ); + expect(lines?.some((line) => line.includes('Plan: Premium Plan'))).toBe( + true, + ); + unmount(); }); @@ -168,7 +178,7 @@ describe('', () => { await waitUntilReady(); const output = lastFrame(); - expect(output).toContain('Enterprise Tier'); + expect(output).toContain('Plan: Enterprise Tier'); expect(output).toContain('/upgrade'); unmount(); }); diff --git a/packages/cli/src/ui/components/UserIdentity.tsx b/packages/cli/src/ui/components/UserIdentity.tsx index 7b07a4f91c..fa2f5c5afa 100644 --- a/packages/cli/src/ui/components/UserIdentity.tsx +++ b/packages/cli/src/ui/components/UserIdentity.tsx @@ -43,7 +43,10 @@ export const UserIdentity: React.FC = ({ config }) => { {authType === AuthType.LOGIN_WITH_GOOGLE ? ( - {email ?? 'Logged in with Google'} + + Signed in with Google{email ? ':' : ''} + {email ? ` ${email}` : ''} + ) : ( `Authenticated with ${authType}` )} @@ -55,7 +58,7 @@ export const UserIdentity: React.FC = ({ config }) => { {tierName && ( - {tierName} + Plan: {tierName} /upgrade diff --git a/packages/cli/src/ui/components/ValidationDialog.tsx b/packages/cli/src/ui/components/ValidationDialog.tsx index f03e09c963..b6c9ab213e 100644 --- a/packages/cli/src/ui/components/ValidationDialog.tsx +++ b/packages/cli/src/ui/components/ValidationDialog.tsx @@ -136,7 +136,7 @@ export function ValidationDialog({ {' '} - Waiting for verification... (Press ESC or CTRL+C to cancel) + Waiting for verification... (Press Esc or Ctrl+C to cancel) {errorMessage && ( diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index adfb1044b6..d505a05838 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -405,8 +405,8 @@ }, "showUserIdentity": { "title": "Show User Identity", - "description": "Show the logged-in user's identity (e.g. email) in the UI.", - "markdownDescription": "Show the logged-in user's identity (e.g. email) in the UI.\n\n- Category: `UI`\n- Requires restart: `no`\n- Default: `true`", + "description": "Show the signed-in user's identity (e.g. email) in the UI.", + "markdownDescription": "Show the signed-in user's identity (e.g. email) in the UI.\n\n- Category: `UI`\n- Requires restart: `no`\n- Default: `true`", "default": true, "type": "boolean" }, From 0d60d68cf99cce5b14829cb974b4c31f0af697ab Mon Sep 17 00:00:00 2001 From: Abhi <43648792+abhipatel12@users.noreply.github.com> Date: Tue, 10 Mar 2026 15:13:00 -0400 Subject: [PATCH 03/15] docs(mcp): standardize mcp tool fqn documentation (#21664) --- docs/cli/tutorials/mcp-setup.md | 2 +- docs/extensions/reference.md | 6 ++-- docs/hooks/reference.md | 2 +- docs/reference/configuration.md | 21 +++++++++----- docs/reference/policy-engine.md | 27 +++++++++++++----- docs/tools/mcp-server.md | 37 +++++++++++++++++-------- packages/core/src/policy/toml-loader.ts | 5 ++++ 7 files changed, 70 insertions(+), 30 deletions(-) diff --git a/docs/cli/tutorials/mcp-setup.md b/docs/cli/tutorials/mcp-setup.md index 8723a65892..76c2806f9d 100644 --- a/docs/cli/tutorials/mcp-setup.md +++ b/docs/cli/tutorials/mcp-setup.md @@ -89,7 +89,7 @@ don't need to learn special commands; just ask in natural language. The agent will: 1. Recognize the request matches a GitHub tool. -2. Call `github_list_pull_requests`. +2. Call `mcp_github_list_pull_requests`. 3. Present the data to you. ### Scenario: Creating an issue diff --git a/docs/extensions/reference.md b/docs/extensions/reference.md index dbba51fa40..e6012f4d33 100644 --- a/docs/extensions/reference.md +++ b/docs/extensions/reference.md @@ -262,12 +262,14 @@ but lower priority than user or admin policies. ```toml [[rule]] -toolName = "my_server__dangerous_tool" +mcpName = "my_server" +toolName = "dangerous_tool" decision = "ask_user" priority = 100 [[safety_checker]] -toolName = "my_server__write_data" +mcpName = "my_server" +toolName = "write_data" priority = 200 [safety_checker.checker] type = "in-process" diff --git a/docs/hooks/reference.md b/docs/hooks/reference.md index a750bc94b3..5242c3a13d 100644 --- a/docs/hooks/reference.md +++ b/docs/hooks/reference.md @@ -85,7 +85,7 @@ compared against the name of the tool being executed. `run_shell_command`). See the [Tools Reference](../reference/tools) for a full list of available tool names. - **MCP Tools**: Tools from MCP servers follow the naming pattern - `mcp____`. + `mcp__`. - **Regex Support**: Matchers support regular expressions (e.g., `matcher: "read_.*"` matches all file reading tools). diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 4b50b99280..c93d0c2e66 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -1176,13 +1176,20 @@ their corresponding top-level category object in your `settings.json` file. Configures connections to one or more Model-Context Protocol (MCP) servers for discovering and using custom tools. Gemini CLI attempts to connect to each -configured MCP server to discover available tools. If multiple MCP servers -expose a tool with the same name, the tool names will be prefixed with the -server alias you defined in the configuration (e.g., -`serverAlias__actualToolName`) to avoid conflicts. Note that the system might -strip certain schema properties from MCP tool definitions for compatibility. At -least one of `command`, `url`, or `httpUrl` must be provided. If multiple are -specified, the order of precedence is `httpUrl`, then `url`, then `command`. +configured MCP server to discover available tools. Every discovered tool is +prepended with the `mcp_` prefix and its server alias to form a fully qualified +name (FQN) (e.g., `mcp_serverAlias_actualToolName`) to avoid conflicts. Note +that the system might strip certain schema properties from MCP tool definitions +for compatibility. At least one of `command`, `url`, or `httpUrl` must be +provided. If multiple are specified, the order of precedence is `httpUrl`, then +`url`, then `command`. + +> **Warning:** Avoid using underscores (`_`) in your server aliases (e.g., use +> `my-server` instead of `my_server`). The underlying policy engine parses Fully +> Qualified Names (`mcp_server_tool`) using the first underscore after the +> `mcp_` prefix. An underscore in your server alias will cause the parser to +> misidentify the server name, which can cause security policies to fail +> silently. - **`mcpServers.`** (object): The server parameters for the named server. diff --git a/docs/reference/policy-engine.md b/docs/reference/policy-engine.md index c0a331d99d..2ea23d4be4 100644 --- a/docs/reference/policy-engine.md +++ b/docs/reference/policy-engine.md @@ -76,9 +76,13 @@ The `toolName` in the rule must match the name of the tool being called. - **Wildcards**: You can use wildcards to match multiple tools. - `*`: Matches **any tool** (built-in or MCP). - - `server__*`: Matches any tool from a specific MCP server. - - `*__toolName`: Matches a specific tool name across **all** MCP servers. - - `*__*`: Matches **any tool from any MCP server**. + - `mcp_server_*`: Matches any tool from a specific MCP server. + - `mcp_*_toolName`: Matches a specific tool name across **all** MCP servers. + - `mcp_*`: Matches **any tool from any MCP server**. + +> **Recommendation:** While FQN wildcards are supported, the recommended +> approach for MCP tools is to use the `mcpName` field in your TOML rules. See +> [Special syntax for MCP tools](#special-syntax-for-mcp-tools). #### Arguments pattern @@ -164,8 +168,8 @@ A rule matches a tool call if all of its conditions are met: 1. **Tool name**: The `toolName` in the rule must match the name of the tool being called. - - **Wildcards**: You can use wildcards like `*`, `server__*`, or - `*__toolName` to match multiple tools. See [Tool Name](#tool-name) for + - **Wildcards**: You can use wildcards like `*`, `mcp_server_*`, or + `mcp_*_toolName` to match multiple tools. See [Tool Name](#tool-name) for details. 2. **Arguments pattern**: If `argsPattern` is specified, the tool's arguments are converted to a stable JSON string, which is then tested against the @@ -224,7 +228,7 @@ toolName = "run_shell_command" subagent = "generalist" # (Optional) The name of an MCP server. Can be combined with toolName -# to form a composite name like "mcpName__toolName". +# to form a composite FQN internally like "mcp_mcpName_toolName". mcpName = "my-custom-server" # (Optional) Metadata hints provided by the tool. A rule matches if all @@ -301,7 +305,16 @@ priority = 100 ### Special syntax for MCP tools You can create rules that target tools from Model Context Protocol (MCP) servers -using the `mcpName` field or composite wildcard patterns. +using the `mcpName` field. **This is the recommended approach** for defining MCP +policies, as it is much more robust than manually writing Fully Qualified Names +(FQNs) or string wildcards. + +> **Warning:** Do not use underscores (`_`) in your MCP server names (e.g., use +> `my-server` rather than `my_server`). The policy parser splits Fully Qualified +> Names (`mcp_server_tool`) on the _first_ underscore following the `mcp_` +> prefix. If your server name contains an underscore, the parser will +> misinterpret the server identity, which can cause wildcard rules and security +> policies to fail silently. **1. Targeting a specific tool on a server** diff --git a/docs/tools/mcp-server.md b/docs/tools/mcp-server.md index bbb5c62aba..6b8cd22ac0 100644 --- a/docs/tools/mcp-server.md +++ b/docs/tools/mcp-server.md @@ -555,21 +555,34 @@ Upon successful connection: `excludeTools` configuration 4. **Name sanitization:** Tool names are cleaned to meet Gemini API requirements: - - Invalid characters (non-alphanumeric, underscore, dot, hyphen) are replaced - with underscores + - Characters other than letters, numbers, underscore (`_`), hyphen (`-`), dot + (`.`), and colon (`:`) are replaced with underscores - Names longer than 63 characters are truncated with middle replacement - (`___`) + (`...`) -### 3. Conflict resolution +### 3. Tool naming and namespaces -When multiple servers expose tools with the same name: +To prevent collisions across multiple servers or conflicting built-in tools, +every discovered MCP tool is assigned a strict namespace. -1. **First registration wins:** The first server to register a tool name gets - the unprefixed name -2. **Automatic prefixing:** Subsequent servers get prefixed names: - `serverName__toolName` -3. **Registry tracking:** The tool registry maintains mappings between server - names and their tools +1. **Automatic FQN:** All MCP tools are unconditionally assigned a fully + qualified name (FQN) using the format `mcp_{serverName}_{toolName}`. +2. **Registry tracking:** The tool registry maintains metadata mappings between + these FQNs and their original server identities. +3. **Overwrites:** If two servers share the exact same alias in your + configuration and provide tools with the exact same name, the last registered + tool overwrites the previous one. +4. **Policies:** To configure permissions (like auto-approval or denial) for MCP + tools, see + [Special syntax for MCP tools](../reference/policy-engine.md#special-syntax-for-mcp-tools) + in the Policy Engine documentation. + +> **Warning:** Do not use underscores (`_`) in your MCP server names (e.g., use +> `my-server` rather than `my_server`). The policy parser splits Fully Qualified +> Names (`mcp_server_tool`) on the _first_ underscore following the `mcp_` +> prefix. If your server name contains an underscore, the parser will +> misinterpret the server identity, which can cause wildcard rules and security +> policies to fail silently. ### 4. Schema processing @@ -695,7 +708,7 @@ MCP Servers Status: 🐳 dockerizedServer (CONNECTED) Command: docker run -i --rm -e API_KEY my-mcp-server:latest - Tools: docker__deploy, docker__status + Tools: mcp_dockerizedServer_docker_deploy, mcp_dockerizedServer_docker_status Discovery State: COMPLETED ``` diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index 83dda26e9e..f5c176dc25 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -457,6 +457,11 @@ export async function loadPoliciesFromToml( const mcpName = rule.mcpName; if (mcpName) { + // TODO(mcp): Decouple mcpName rules from FQN string parsing + // to support underscores in server aliases natively. Leaving + // mcpName and toolName separate here and relying on metadata + // during policy evaluation will avoid underscore splitting bugs. + // See: https://github.com/google-gemini/gemini-cli/issues/21727 effectiveToolName = formatMcpToolName( mcpName, effectiveToolName, From 077c1a1e2d08f749dc2737b599961d45f1d7fcca Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Tue, 10 Mar 2026 19:15:45 +0000 Subject: [PATCH 04/15] make command names consistent (#21907) --- docs/reference/keyboard-shortcuts.md | 23 +++--- packages/cli/src/ui/key/keyBindings.ts | 104 ++++++++++++------------- 2 files changed, 64 insertions(+), 63 deletions(-) diff --git a/docs/reference/keyboard-shortcuts.md b/docs/reference/keyboard-shortcuts.md index 097b380268..3529ead3ec 100644 --- a/docs/reference/keyboard-shortcuts.md +++ b/docs/reference/keyboard-shortcuts.md @@ -106,20 +106,25 @@ available combinations. | Cycle through approval modes: default (prompt), auto_edit (auto-approve edits), and plan (read-only). Plan mode is skipped when the agent is busy. | `Shift+Tab` | | Expand and collapse blocks of content when not in alternate buffer mode. | `Ctrl+O` | | Expand or collapse a paste placeholder when cursor is over placeholder. | `Ctrl+O` | -| Toggle current background shell visibility. | `Ctrl+B` | -| Toggle background shell list. | `Ctrl+L` | -| Kill the active background shell. | `Ctrl+K` | -| Confirm selection in background shell list. | `Enter` | -| Dismiss background shell list. | `Esc` | -| Move focus from background shell to Gemini. | `Shift+Tab` | -| Move focus from background shell list to Gemini. | `Tab` | -| Show warning when trying to move focus away from background shell. | `Tab` | -| Show warning when trying to move focus away from shell input. | `Tab` | | Move focus from Gemini to the active shell. | `Tab` | | Move focus from the shell back to Gemini. | `Shift+Tab` | | Clear the terminal screen and redraw the UI. | `Ctrl+L` | | Restart the application. | `R`
`Shift+R` | | Suspend the CLI and move it to the background. | `Ctrl+Z` | +| Show warning when trying to move focus away from shell input. | `Tab` | + +#### Background Shell Controls + +| Action | Keys | +| ------------------------------------------------------------------ | ----------- | +| Dismiss background shell list. | `Esc` | +| Confirm selection in background shell list. | `Enter` | +| Toggle current background shell visibility. | `Ctrl+B` | +| Toggle background shell list. | `Ctrl+L` | +| Kill the active background shell. | `Ctrl+K` | +| Move focus from background shell to Gemini. | `Shift+Tab` | +| Move focus from background shell list to Gemini. | `Tab` | +| Show warning when trying to move focus away from background shell. | `Tab` | diff --git a/packages/cli/src/ui/key/keyBindings.ts b/packages/cli/src/ui/key/keyBindings.ts index b375d991c8..5f1e833a53 100644 --- a/packages/cli/src/ui/key/keyBindings.ts +++ b/packages/cli/src/ui/key/keyBindings.ts @@ -73,16 +73,6 @@ export enum Command { OPEN_EXTERNAL_EDITOR = 'input.openExternalEditor', PASTE_CLIPBOARD = 'input.paste', - BACKGROUND_SHELL_ESCAPE = 'backgroundShellEscape', - BACKGROUND_SHELL_SELECT = 'backgroundShellSelect', - TOGGLE_BACKGROUND_SHELL = 'toggleBackgroundShell', - TOGGLE_BACKGROUND_SHELL_LIST = 'toggleBackgroundShellList', - KILL_BACKGROUND_SHELL = 'backgroundShell.kill', - UNFOCUS_BACKGROUND_SHELL = 'backgroundShell.unfocus', - UNFOCUS_BACKGROUND_SHELL_LIST = 'backgroundShell.listUnfocus', - SHOW_BACKGROUND_SHELL_UNFOCUS_WARNING = 'backgroundShell.unfocusWarning', - SHOW_SHELL_INPUT_UNFOCUS_WARNING = 'shellInput.unfocusWarning', - // App Controls SHOW_ERROR_DETAILS = 'app.showErrorDetails', SHOW_FULL_TODOS = 'app.showFullTodos', @@ -98,6 +88,17 @@ export enum Command { CLEAR_SCREEN = 'app.clearScreen', RESTART_APP = 'app.restart', SUSPEND_APP = 'app.suspend', + SHOW_SHELL_INPUT_UNFOCUS_WARNING = 'app.showShellUnfocusWarning', + + // Background Shell Controls + BACKGROUND_SHELL_ESCAPE = 'background.escape', + BACKGROUND_SHELL_SELECT = 'background.select', + TOGGLE_BACKGROUND_SHELL = 'background.toggle', + TOGGLE_BACKGROUND_SHELL_LIST = 'background.toggleList', + KILL_BACKGROUND_SHELL = 'background.kill', + UNFOCUS_BACKGROUND_SHELL = 'background.unfocus', + UNFOCUS_BACKGROUND_SHELL_LIST = 'background.unfocusList', + SHOW_BACKGROUND_SHELL_UNFOCUS_WARNING = 'background.unfocusWarning', } /** @@ -105,20 +106,10 @@ export enum Command { */ export class KeyBinding { private static readonly VALID_KEYS = new Set([ - // Letters & Numbers - ...'abcdefghijklmnopqrstuvwxyz0123456789', - // Punctuation - '`', - '-', - '=', - '[', - ']', - '\\', - ';', - "'", - ',', - '.', - '/', + ...'abcdefghijklmnopqrstuvwxyz0123456789', // Letters & Numbers + ..."`-=[]\\;',./", // Punctuation + ...Array.from({ length: 19 }, (_, i) => `f${i + 1}`), // Function Keys + ...Array.from({ length: 10 }, (_, i) => `numpad${i}`), // Numpad Numbers // Navigation & Actions 'left', 'up', @@ -139,10 +130,6 @@ export class KeyBinding { 'insert', 'numlock', 'scrolllock', - // Function Keys - ...Array.from({ length: 19 }, (_, i) => `f${i + 1}`), - // Numpad - ...Array.from({ length: 10 }, (_, i) => `numpad${i}`), 'numpad_multiply', 'numpad_add', 'numpad_separator', @@ -354,15 +341,6 @@ export const defaultKeyBindings: KeyBindingConfig = { [Command.TOGGLE_COPY_MODE]: [new KeyBinding('ctrl+s')], [Command.TOGGLE_YOLO]: [new KeyBinding('ctrl+y')], [Command.CYCLE_APPROVAL_MODE]: [new KeyBinding('shift+tab')], - [Command.TOGGLE_BACKGROUND_SHELL]: [new KeyBinding('ctrl+b')], - [Command.TOGGLE_BACKGROUND_SHELL_LIST]: [new KeyBinding('ctrl+l')], - [Command.KILL_BACKGROUND_SHELL]: [new KeyBinding('ctrl+k')], - [Command.UNFOCUS_BACKGROUND_SHELL]: [new KeyBinding('shift+tab')], - [Command.UNFOCUS_BACKGROUND_SHELL_LIST]: [new KeyBinding('tab')], - [Command.SHOW_BACKGROUND_SHELL_UNFOCUS_WARNING]: [new KeyBinding('tab')], - [Command.SHOW_SHELL_INPUT_UNFOCUS_WARNING]: [new KeyBinding('tab')], - [Command.BACKGROUND_SHELL_SELECT]: [new KeyBinding('enter')], - [Command.BACKGROUND_SHELL_ESCAPE]: [new KeyBinding('escape')], [Command.SHOW_MORE_LINES]: [new KeyBinding('ctrl+o')], [Command.EXPAND_PASTE]: [new KeyBinding('ctrl+o')], [Command.FOCUS_SHELL_INPUT]: [new KeyBinding('tab')], @@ -370,6 +348,17 @@ export const defaultKeyBindings: KeyBindingConfig = { [Command.CLEAR_SCREEN]: [new KeyBinding('ctrl+l')], [Command.RESTART_APP]: [new KeyBinding('r'), new KeyBinding('shift+r')], [Command.SUSPEND_APP]: [new KeyBinding('ctrl+z')], + [Command.SHOW_SHELL_INPUT_UNFOCUS_WARNING]: [new KeyBinding('tab')], + + // Background Shell Controls + [Command.BACKGROUND_SHELL_ESCAPE]: [new KeyBinding('escape')], + [Command.BACKGROUND_SHELL_SELECT]: [new KeyBinding('enter')], + [Command.TOGGLE_BACKGROUND_SHELL]: [new KeyBinding('ctrl+b')], + [Command.TOGGLE_BACKGROUND_SHELL_LIST]: [new KeyBinding('ctrl+l')], + [Command.KILL_BACKGROUND_SHELL]: [new KeyBinding('ctrl+k')], + [Command.UNFOCUS_BACKGROUND_SHELL]: [new KeyBinding('shift+tab')], + [Command.UNFOCUS_BACKGROUND_SHELL_LIST]: [new KeyBinding('tab')], + [Command.SHOW_BACKGROUND_SHELL_UNFOCUS_WARNING]: [new KeyBinding('tab')], }; interface CommandCategory { @@ -475,20 +464,25 @@ export const commandCategories: readonly CommandCategory[] = [ Command.CYCLE_APPROVAL_MODE, Command.SHOW_MORE_LINES, Command.EXPAND_PASTE, - Command.TOGGLE_BACKGROUND_SHELL, - Command.TOGGLE_BACKGROUND_SHELL_LIST, - Command.KILL_BACKGROUND_SHELL, - Command.BACKGROUND_SHELL_SELECT, - Command.BACKGROUND_SHELL_ESCAPE, - Command.UNFOCUS_BACKGROUND_SHELL, - Command.UNFOCUS_BACKGROUND_SHELL_LIST, - Command.SHOW_BACKGROUND_SHELL_UNFOCUS_WARNING, - Command.SHOW_SHELL_INPUT_UNFOCUS_WARNING, Command.FOCUS_SHELL_INPUT, Command.UNFOCUS_SHELL_INPUT, Command.CLEAR_SCREEN, Command.RESTART_APP, Command.SUSPEND_APP, + Command.SHOW_SHELL_INPUT_UNFOCUS_WARNING, + ], + }, + { + title: 'Background Shell Controls', + commands: [ + Command.BACKGROUND_SHELL_ESCAPE, + Command.BACKGROUND_SHELL_SELECT, + Command.TOGGLE_BACKGROUND_SHELL, + Command.TOGGLE_BACKGROUND_SHELL_LIST, + Command.KILL_BACKGROUND_SHELL, + Command.UNFOCUS_BACKGROUND_SHELL, + Command.UNFOCUS_BACKGROUND_SHELL_LIST, + Command.SHOW_BACKGROUND_SHELL_UNFOCUS_WARNING, ], }, ]; @@ -576,9 +570,18 @@ export const commandDescriptions: Readonly> = { 'Expand and collapse blocks of content when not in alternate buffer mode.', [Command.EXPAND_PASTE]: 'Expand or collapse a paste placeholder when cursor is over placeholder.', + [Command.FOCUS_SHELL_INPUT]: 'Move focus from Gemini to the active shell.', + [Command.UNFOCUS_SHELL_INPUT]: 'Move focus from the shell back to Gemini.', + [Command.CLEAR_SCREEN]: 'Clear the terminal screen and redraw the UI.', + [Command.RESTART_APP]: 'Restart the application.', + [Command.SUSPEND_APP]: 'Suspend the CLI and move it to the background.', + [Command.SHOW_SHELL_INPUT_UNFOCUS_WARNING]: + 'Show warning when trying to move focus away from shell input.', + + // Background Shell Controls + [Command.BACKGROUND_SHELL_ESCAPE]: 'Dismiss background shell list.', [Command.BACKGROUND_SHELL_SELECT]: 'Confirm selection in background shell list.', - [Command.BACKGROUND_SHELL_ESCAPE]: 'Dismiss background shell list.', [Command.TOGGLE_BACKGROUND_SHELL]: 'Toggle current background shell visibility.', [Command.TOGGLE_BACKGROUND_SHELL_LIST]: 'Toggle background shell list.', @@ -589,11 +592,4 @@ export const commandDescriptions: Readonly> = { 'Move focus from background shell list to Gemini.', [Command.SHOW_BACKGROUND_SHELL_UNFOCUS_WARNING]: 'Show warning when trying to move focus away from background shell.', - [Command.SHOW_SHELL_INPUT_UNFOCUS_WARNING]: - 'Show warning when trying to move focus away from shell input.', - [Command.FOCUS_SHELL_INPUT]: 'Move focus from Gemini to the active shell.', - [Command.UNFOCUS_SHELL_INPUT]: 'Move focus from the shell back to Gemini.', - [Command.CLEAR_SCREEN]: 'Clear the terminal screen and redraw the UI.', - [Command.RESTART_APP]: 'Restart the application.', - [Command.SUSPEND_APP]: 'Suspend the CLI and move it to the background.', }; From 00a39b3da90dc083549d61ba14a15893ece35d9d Mon Sep 17 00:00:00 2001 From: Adam Weidman <65992621+adamfweidman@users.noreply.github.com> Date: Tue, 10 Mar 2026 15:16:46 -0400 Subject: [PATCH 05/15] refactor: remove agent_card_requires_auth config flag (#21914) --- packages/core/src/agents/agentLoader.test.ts | 20 ------------------- packages/core/src/agents/agentLoader.ts | 9 ++------- .../core/src/agents/auth-provider/types.ts | 5 ++--- 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/packages/core/src/agents/agentLoader.test.ts b/packages/core/src/agents/agentLoader.test.ts index 9c03094b3f..a526382553 100644 --- a/packages/core/src/agents/agentLoader.test.ts +++ b/packages/core/src/agents/agentLoader.test.ts @@ -557,26 +557,6 @@ auth: }); }); - it('should parse auth with agent_card_requires_auth flag', async () => { - const filePath = await writeAgentMarkdown(`--- -kind: remote -name: protected-card-agent -agent_card_url: https://example.com/card -auth: - type: apiKey - key: $MY_API_KEY - agent_card_requires_auth: true ---- -`); - const result = await parseAgentMarkdown(filePath); - expect(result[0]).toMatchObject({ - auth: { - type: 'apiKey', - agent_card_requires_auth: true, - }, - }); - }); - it('should parse remote agent with oauth2 auth', async () => { const filePath = await writeAgentMarkdown(`--- kind: remote diff --git a/packages/core/src/agents/agentLoader.ts b/packages/core/src/agents/agentLoader.ts index b91187204e..12337c6248 100644 --- a/packages/core/src/agents/agentLoader.ts +++ b/packages/core/src/agents/agentLoader.ts @@ -45,7 +45,6 @@ interface FrontmatterLocalAgentDefinition */ interface FrontmatterAuthConfig { type: 'apiKey' | 'http' | 'oauth2'; - agent_card_requires_auth?: boolean; // API Key key?: string; name?: string; @@ -123,9 +122,7 @@ const localAgentSchema = z /** * Base fields shared by all auth configs. */ -const baseAuthFields = { - agent_card_requires_auth: z.boolean().optional(), -}; +const baseAuthFields = {}; /** * API Key auth schema. @@ -356,9 +353,7 @@ export async function parseAgentMarkdown( function convertFrontmatterAuthToConfig( frontmatter: FrontmatterAuthConfig, ): A2AAuthConfig { - const base = { - agent_card_requires_auth: frontmatter.agent_card_requires_auth, - }; + const base = {}; switch (frontmatter.type) { case 'apiKey': diff --git a/packages/core/src/agents/auth-provider/types.ts b/packages/core/src/agents/auth-provider/types.ts index f4e2e48b13..0808f54ae2 100644 --- a/packages/core/src/agents/auth-provider/types.ts +++ b/packages/core/src/agents/auth-provider/types.ts @@ -24,9 +24,8 @@ export interface A2AAuthProvider extends AuthenticationHandler { initialize?(): Promise; } -export interface BaseAuthConfig { - agent_card_requires_auth?: boolean; -} +// eslint-disable-next-line @typescript-eslint/no-empty-object-type +export interface BaseAuthConfig {} /** Client config for google-credentials (not in A2A spec, Gemini-specific). */ export interface GoogleCredentialsAuthConfig extends BaseAuthConfig { From be6747043293384800cfc8caf90927fe8df945ad Mon Sep 17 00:00:00 2001 From: Alisa <62909685+alisa-alisa@users.noreply.github.com> Date: Tue, 10 Mar 2026 12:19:48 -0700 Subject: [PATCH 06/15] feat(a2a): implement standardized normalization and streaming reassembly (#21402) Co-authored-by: matt korwel --- packages/core/src/agents/a2aUtils.test.ts | 283 ++++++++++++++++++- packages/core/src/agents/a2aUtils.ts | 326 +++++++++++++++++++--- 2 files changed, 574 insertions(+), 35 deletions(-) diff --git a/packages/core/src/agents/a2aUtils.test.ts b/packages/core/src/agents/a2aUtils.test.ts index 2bcdad2c40..c3fe170aa5 100644 --- a/packages/core/src/agents/a2aUtils.test.ts +++ b/packages/core/src/agents/a2aUtils.test.ts @@ -4,13 +4,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { extractMessageText, extractIdsFromResponse, isTerminalState, A2AResultReassembler, AUTH_REQUIRED_MSG, + normalizeAgentCard, + getGrpcCredentials, + pinUrlToIp, + splitAgentCardUrl, } from './a2aUtils.js'; import type { SendMessageResult } from './a2a-client-manager.js'; import type { @@ -22,8 +26,105 @@ import type { TaskStatusUpdateEvent, TaskArtifactUpdateEvent, } from '@a2a-js/sdk'; +import * as dnsPromises from 'node:dns/promises'; +import type { LookupAddress } from 'node:dns'; + +vi.mock('node:dns/promises', () => ({ + lookup: vi.fn(), +})); describe('a2aUtils', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('getGrpcCredentials', () => { + it('should return secure credentials for https', () => { + const credentials = getGrpcCredentials('https://test.agent'); + expect(credentials).toBeDefined(); + }); + + it('should return insecure credentials for http', () => { + const credentials = getGrpcCredentials('http://test.agent'); + expect(credentials).toBeDefined(); + }); + }); + + describe('pinUrlToIp', () => { + it('should resolve and pin hostname to IP', async () => { + vi.mocked( + dnsPromises.lookup as unknown as ( + hostname: string, + options: { all: true }, + ) => Promise, + ).mockResolvedValue([{ address: '93.184.216.34', family: 4 }]); + + const { pinnedUrl, hostname } = await pinUrlToIp( + 'http://example.com:9000', + 'test-agent', + ); + expect(hostname).toBe('example.com'); + expect(pinnedUrl).toBe('http://93.184.216.34:9000/'); + }); + + it('should handle raw host:port strings (standard for gRPC)', async () => { + vi.mocked( + dnsPromises.lookup as unknown as ( + hostname: string, + options: { all: true }, + ) => Promise, + ).mockResolvedValue([{ address: '93.184.216.34', family: 4 }]); + + const { pinnedUrl, hostname } = await pinUrlToIp( + 'example.com:9000', + 'test-agent', + ); + expect(hostname).toBe('example.com'); + expect(pinnedUrl).toBe('93.184.216.34:9000'); + }); + + it('should throw error if resolution fails (fail closed)', async () => { + vi.mocked(dnsPromises.lookup).mockRejectedValue(new Error('DNS Error')); + + await expect( + pinUrlToIp('http://unreachable.com', 'test-agent'), + ).rejects.toThrow("Failed to resolve host for agent 'test-agent'"); + }); + + it('should throw error if resolved to private IP', async () => { + vi.mocked( + dnsPromises.lookup as unknown as ( + hostname: string, + options: { all: true }, + ) => Promise, + ).mockResolvedValue([{ address: '10.0.0.1', family: 4 }]); + + await expect( + pinUrlToIp('http://malicious.com', 'test-agent'), + ).rejects.toThrow('resolves to private IP range'); + }); + + it('should allow localhost/127.0.0.1/::1 exceptions', async () => { + vi.mocked( + dnsPromises.lookup as unknown as ( + hostname: string, + options: { all: true }, + ) => Promise, + ).mockResolvedValue([{ address: '127.0.0.1', family: 4 }]); + + const { pinnedUrl, hostname } = await pinUrlToIp( + 'http://localhost:9000', + 'test-agent', + ); + expect(hostname).toBe('localhost'); + expect(pinnedUrl).toBe('http://127.0.0.1:9000/'); + }); + }); + describe('isTerminalState', () => { it('should return true for completed, failed, canceled, and rejected', () => { expect(isTerminalState('completed')).toBe(true); @@ -223,6 +324,173 @@ describe('a2aUtils', () => { } as Message), ).toBe(''); }); + + it('should handle file parts with neither name nor uri', () => { + const message: Message = { + kind: 'message', + role: 'user', + messageId: '1', + parts: [ + { + kind: 'file', + file: { + mimeType: 'text/plain', + }, + } as FilePart, + ], + }; + expect(extractMessageText(message)).toBe('File: [binary/unnamed]'); + }); + }); + + describe('normalizeAgentCard', () => { + it('should throw if input is not an object', () => { + expect(() => normalizeAgentCard(null)).toThrow('Agent card is missing.'); + expect(() => normalizeAgentCard(undefined)).toThrow( + 'Agent card is missing.', + ); + expect(() => normalizeAgentCard('not an object')).toThrow( + 'Agent card is missing.', + ); + }); + + it('should preserve unknown fields while providing defaults for mandatory ones', () => { + const raw = { + name: 'my-agent', + customField: 'keep-me', + }; + + const normalized = normalizeAgentCard(raw); + + expect(normalized.name).toBe('my-agent'); + // @ts-expect-error - testing dynamic preservation + expect(normalized.customField).toBe('keep-me'); + expect(normalized.description).toBe(''); + expect(normalized.skills).toEqual([]); + expect(normalized.defaultInputModes).toEqual([]); + }); + + it('should normalize and synchronize interfaces while preserving other fields', () => { + const raw = { + name: 'test', + supportedInterfaces: [ + { + url: 'grpc://test', + protocolBinding: 'GRPC', + protocolVersion: '1.0', + }, + ], + }; + + const normalized = normalizeAgentCard(raw); + + // Should exist in both fields + expect(normalized.additionalInterfaces).toHaveLength(1); + expect( + (normalized as unknown as Record)[ + 'supportedInterfaces' + ], + ).toHaveLength(1); + + const intf = normalized.additionalInterfaces?.[0] as unknown as Record< + string, + unknown + >; + + expect(intf['transport']).toBe('GRPC'); + expect(intf['url']).toBe('grpc://test'); + + // Should fallback top-level url + expect(normalized.url).toBe('grpc://test'); + }); + + it('should preserve existing top-level url if present', () => { + const raw = { + name: 'test', + url: 'http://existing', + supportedInterfaces: [{ url: 'http://other', transport: 'REST' }], + }; + + const normalized = normalizeAgentCard(raw); + expect(normalized.url).toBe('http://existing'); + }); + + it('should NOT prepend http:// scheme to raw IP:port strings for gRPC interfaces', () => { + const raw = { + name: 'raw-ip-grpc', + supportedInterfaces: [{ url: '127.0.0.1:9000', transport: 'GRPC' }], + }; + + const normalized = normalizeAgentCard(raw); + expect(normalized.additionalInterfaces?.[0].url).toBe('127.0.0.1:9000'); + expect(normalized.url).toBe('127.0.0.1:9000'); + }); + + it('should prepend http:// scheme to raw IP:port strings for REST interfaces', () => { + const raw = { + name: 'raw-ip-rest', + supportedInterfaces: [{ url: '127.0.0.1:8080', transport: 'REST' }], + }; + + const normalized = normalizeAgentCard(raw); + expect(normalized.additionalInterfaces?.[0].url).toBe( + 'http://127.0.0.1:8080', + ); + }); + + it('should NOT override existing transport if protocolBinding is also present', () => { + const raw = { + name: 'priority-test', + supportedInterfaces: [ + { url: 'foo', transport: 'GRPC', protocolBinding: 'REST' }, + ], + }; + const normalized = normalizeAgentCard(raw); + expect(normalized.additionalInterfaces?.[0].transport).toBe('GRPC'); + }); + }); + + describe('splitAgentCardUrl', () => { + const standard = '.well-known/agent-card.json'; + + it('should return baseUrl as-is if it does not end with standard path', () => { + const url = 'http://localhost:9001/custom/path'; + expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url }); + }); + + it('should split correctly if URL ends with standard path', () => { + const url = `http://localhost:9001/${standard}`; + expect(splitAgentCardUrl(url)).toEqual({ + baseUrl: 'http://localhost:9001/', + path: undefined, + }); + }); + + it('should handle trailing slash in baseUrl when splitting', () => { + const url = `http://example.com/api/${standard}`; + expect(splitAgentCardUrl(url)).toEqual({ + baseUrl: 'http://example.com/api/', + path: undefined, + }); + }); + + it('should ignore hashes and query params when splitting', () => { + const url = `http://localhost:9001/${standard}?foo=bar#baz`; + expect(splitAgentCardUrl(url)).toEqual({ + baseUrl: 'http://localhost:9001/', + path: undefined, + }); + }); + + it('should return original URL if parsing fails', () => { + const url = 'not-a-url'; + expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url }); + }); + + it('should handle standard path appearing earlier in the path', () => { + const url = `http://localhost:9001/${standard}/something-else`; + expect(splitAgentCardUrl(url)).toEqual({ baseUrl: url }); + }); }); describe('A2AResultReassembler', () => { @@ -233,6 +501,7 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'status-update', taskId: 't1', + contextId: 'ctx1', status: { state: 'working', message: { @@ -247,6 +516,7 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'artifact-update', taskId: 't1', + contextId: 'ctx1', append: false, artifact: { artifactId: 'a1', @@ -259,6 +529,7 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'status-update', taskId: 't1', + contextId: 'ctx1', status: { state: 'working', message: { @@ -273,6 +544,7 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'artifact-update', taskId: 't1', + contextId: 'ctx1', append: true, artifact: { artifactId: 'a1', @@ -291,6 +563,7 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'status-update', + contextId: 'ctx1', status: { state: 'auth-required', message: { @@ -310,6 +583,7 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'status-update', + contextId: 'ctx1', status: { state: 'auth-required', }, @@ -323,6 +597,7 @@ describe('a2aUtils', () => { const chunk = { kind: 'status-update', + contextId: 'ctx1', status: { state: 'auth-required', message: { @@ -351,6 +626,8 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'task', + id: 'task-1', + contextId: 'ctx1', status: { state: 'completed' }, history: [ { @@ -369,6 +646,8 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'task', + id: 'task-1', + contextId: 'ctx1', status: { state: 'working' }, history: [ { @@ -387,6 +666,8 @@ describe('a2aUtils', () => { reassembler.update({ kind: 'task', + id: 'task-1', + contextId: 'ctx1', status: { state: 'completed' }, artifacts: [ { diff --git a/packages/core/src/agents/a2aUtils.ts b/packages/core/src/agents/a2aUtils.ts index dc39f4e660..ec8b36bba1 100644 --- a/packages/core/src/agents/a2aUtils.ts +++ b/packages/core/src/agents/a2aUtils.ts @@ -4,6 +4,9 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as grpc from '@grpc/grpc-js'; +import { lookup } from 'node:dns/promises'; +import { z } from 'zod'; import type { Message, Part, @@ -12,12 +15,40 @@ import type { FilePart, Artifact, TaskState, - TaskStatusUpdateEvent, + AgentCard, + AgentInterface, } from '@a2a-js/sdk'; +import { isAddressPrivate } from '../utils/fetch.js'; import type { SendMessageResult } from './a2a-client-manager.js'; export const AUTH_REQUIRED_MSG = `[Authorization Required] The agent has indicated it requires authorization to proceed. Please follow the agent's instructions.`; +const AgentInterfaceSchema = z + .object({ + url: z.string().default(''), + transport: z.string().optional(), + protocolBinding: z.string().optional(), + }) + .passthrough(); + +const AgentCardSchema = z + .object({ + name: z.string().default('unknown'), + description: z.string().default(''), + url: z.string().default(''), + version: z.string().default(''), + protocolVersion: z.string().default(''), + capabilities: z.record(z.unknown()).default({}), + skills: z.array(z.union([z.string(), z.record(z.unknown())])).default([]), + defaultInputModes: z.array(z.string()).default([]), + defaultOutputModes: z.array(z.string()).default([]), + + additionalInterfaces: z.array(AgentInterfaceSchema).optional(), + supportedInterfaces: z.array(AgentInterfaceSchema).optional(), + preferredTransport: z.string().optional(), + }) + .passthrough(); + /** * Reassembles incremental A2A streaming updates into a coherent result. * Shows sequential status/messages followed by all reassembled artifacts. @@ -100,12 +131,11 @@ export class A2AResultReassembler { } break; - case 'message': { + case 'message': this.pushMessage(chunk); break; - } - default: + // Handle unknown kinds gracefully break; } } @@ -210,36 +240,165 @@ function extractPartText(part: Part): string { return ''; } -// Type Guards +/** + * Normalizes an agent card by ensuring it has the required properties + * and resolving any inconsistencies between protocol versions. + */ +export function normalizeAgentCard(card: unknown): AgentCard { + if (!isObject(card)) { + throw new Error('Agent card is missing.'); + } -function isTextPart(part: Part): part is TextPart { - return part.kind === 'text'; -} + // Use Zod to validate and parse the card, ensuring safe defaults and narrowing types. + const parsed = AgentCardSchema.parse(card); + // Narrowing to AgentCard interface after runtime validation. + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const result = parsed as unknown as AgentCard; -function isDataPart(part: Part): part is DataPart { - return part.kind === 'data'; -} + // Normalize interfaces and synchronize both interface fields. + const normalizedInterfaces = extractNormalizedInterfaces(parsed); + result.additionalInterfaces = normalizedInterfaces; -function isFilePart(part: Part): part is FilePart { - return part.kind === 'file'; -} + // Sync supportedInterfaces for backward compatibility. + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const legacyResult = result as unknown as Record; + legacyResult['supportedInterfaces'] = normalizedInterfaces; -function isStatusUpdateEvent( - result: SendMessageResult, -): result is TaskStatusUpdateEvent { - return result.kind === 'status-update'; + // Fallback preferredTransport: If not specified, default to GRPC if available. + if ( + !result.preferredTransport && + normalizedInterfaces.some((i) => i.transport === 'GRPC') + ) { + result.preferredTransport = 'GRPC'; + } + + // Fallback: If top-level URL is missing, use the first interface's URL. + if (result.url === '' && normalizedInterfaces.length > 0) { + result.url = normalizedInterfaces[0].url; + } + + return result; } /** - * Returns true if the given state is a terminal state for a task. + * Returns gRPC channel credentials based on the URL scheme. */ -export function isTerminalState(state: TaskState | undefined): boolean { - return ( - state === 'completed' || - state === 'failed' || - state === 'canceled' || - state === 'rejected' - ); +export function getGrpcCredentials(url: string): grpc.ChannelCredentials { + return url.startsWith('https://') + ? grpc.credentials.createSsl() + : grpc.credentials.createInsecure(); +} + +/** + * Returns gRPC channel options to ensure SSL/authority matches the original hostname + * when connecting via a pinned IP address. + */ +export function getGrpcChannelOptions( + hostname: string, +): Record { + return { + 'grpc.default_authority': hostname, + 'grpc.ssl_target_name_override': hostname, + }; +} + +/** + * Resolves a hostname to its IP address and validates it against SSRF. + * Returns the pinned IP-based URL and the original hostname. + */ +export async function pinUrlToIp( + url: string, + agentName: string, +): Promise<{ pinnedUrl: string; hostname: string }> { + if (!url) return { pinnedUrl: url, hostname: '' }; + + // gRPC URLs in A2A can be 'host:port' or 'dns:///host:port' or have schemes. + // We normalize to host:port for resolution. + const hasScheme = url.includes('://'); + const normalizedUrl = hasScheme ? url : `http://${url}`; + + try { + const parsed = new URL(normalizedUrl); + const hostname = parsed.hostname; + + const sanitizedHost = + hostname.startsWith('[') && hostname.endsWith(']') + ? hostname.slice(1, -1) + : hostname; + + // Resolve DNS to check the actual target IP and pin it + const addresses = await lookup(hostname, { all: true }); + const publicAddresses = addresses.filter( + (addr) => + !isAddressPrivate(addr.address) || + sanitizedHost === 'localhost' || + sanitizedHost === '127.0.0.1' || + sanitizedHost === '::1', + ); + + if (publicAddresses.length === 0) { + if (addresses.length > 0) { + throw new Error( + `Refusing to load agent '${agentName}': transport URL '${url}' resolves to private IP range.`, + ); + } + throw new Error( + `Failed to resolve any public IP addresses for host: ${hostname}`, + ); + } + + const pinnedIp = publicAddresses[0].address; + const pinnedHostname = pinnedIp.includes(':') ? `[${pinnedIp}]` : pinnedIp; + + // Reconstruct URL with IP + parsed.hostname = pinnedHostname; + let pinnedUrl = parsed.toString(); + + // If original didn't have scheme, remove it (standard for gRPC targets) + if (!hasScheme) { + pinnedUrl = pinnedUrl.replace(/^http:\/\//, ''); + // URL.toString() might append a trailing slash + if (pinnedUrl.endsWith('/') && !url.endsWith('/')) { + pinnedUrl = pinnedUrl.slice(0, -1); + } + } + + return { pinnedUrl, hostname }; + } catch (e) { + if (e instanceof Error && e.message.includes('Refusing')) throw e; + throw new Error(`Failed to resolve host for agent '${agentName}': ${url}`, { + cause: e, + }); + } +} + +/** + * Splts an agent card URL into a baseUrl and a standard path if it already + * contains '.well-known/agent-card.json'. + */ +export function splitAgentCardUrl(url: string): { + baseUrl: string; + path?: string; +} { + const standardPath = '.well-known/agent-card.json'; + try { + const parsedUrl = new URL(url); + if (parsedUrl.pathname.endsWith(standardPath)) { + // Reconstruct baseUrl from parsed components to avoid issues with hashes or query params. + parsedUrl.pathname = parsedUrl.pathname.substring( + 0, + parsedUrl.pathname.lastIndexOf(standardPath), + ); + parsedUrl.search = ''; + parsedUrl.hash = ''; + // We return undefined for path if it's the standard one, + // because the SDK's DefaultAgentCardResolver appends it automatically. + return { baseUrl: parsedUrl.toString(), path: undefined }; + } + } catch (_e) { + // Ignore URL parsing errors here, let the resolver handle them. + } + return { baseUrl: url }; } /** @@ -255,27 +414,126 @@ export function extractIdsFromResponse(result: SendMessageResult): { let taskId: string | undefined; let clearTaskId = false; - if ('kind' in result) { - const kind = result.kind; - if (kind === 'message' || kind === 'artifact-update') { + if (!('kind' in result)) return { contextId, taskId, clearTaskId }; + + switch (result.kind) { + case 'message': + case 'artifact-update': taskId = result.taskId; contextId = result.contextId; - } else if (kind === 'task') { + break; + + case 'task': taskId = result.id; contextId = result.contextId; if (isTerminalState(result.status?.state)) { clearTaskId = true; } - } else if (isStatusUpdateEvent(result)) { + break; + + case 'status-update': taskId = result.taskId; contextId = result.contextId; - // Note: We ignore the 'final' flag here per A2A protocol best practices, - // as a stream can close while a task is still in a 'working' state. if (isTerminalState(result.status?.state)) { clearTaskId = true; } - } + break; + default: + // Handle other kind values if any + break; } return { contextId, taskId, clearTaskId }; } + +/** + * Extracts and normalizes interfaces from the card, handling protocol version fallbacks. + * Preserves all original fields to maintain SDK compatibility. + */ +function extractNormalizedInterfaces( + card: Record, +): AgentInterface[] { + const rawInterfaces = + getArray(card, 'additionalInterfaces') || + getArray(card, 'supportedInterfaces'); + + if (!rawInterfaces) { + return []; + } + + const mapped: AgentInterface[] = []; + for (const i of rawInterfaces) { + if (isObject(i)) { + // Use schema to validate interface object. + const parsed = AgentInterfaceSchema.parse(i); + // Narrowing to AgentInterface after runtime validation. + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const normalized = parsed as unknown as AgentInterface & { + protocolBinding?: string; + }; + + // Normalize 'transport' from 'protocolBinding' if missing. + if (!normalized.transport && normalized.protocolBinding) { + normalized.transport = normalized.protocolBinding; + } + + // Robust URL: Ensure the URL has a scheme (except for gRPC). + if ( + normalized.url && + !normalized.url.includes('://') && + !normalized.url.startsWith('/') && + normalized.transport !== 'GRPC' + ) { + // Default to http:// for insecure REST/JSON-RPC if scheme is missing. + normalized.url = `http://${normalized.url}`; + } + + mapped.push(normalized as AgentInterface); + } + } + return mapped; +} + +/** + * Safely extracts an array property from an object. + */ +function getArray( + obj: Record, + key: string, +): unknown[] | undefined { + const val = obj[key]; + return Array.isArray(val) ? val : undefined; +} + +// Type Guards + +function isTextPart(part: Part): part is TextPart { + return part.kind === 'text'; +} + +function isDataPart(part: Part): part is DataPart { + return part.kind === 'data'; +} + +function isFilePart(part: Part): part is FilePart { + return part.kind === 'file'; +} + +/** + * Returns true if the given state is a terminal state for a task. + */ +export function isTerminalState(state: TaskState | undefined): boolean { + return ( + state === 'completed' || + state === 'failed' || + state === 'canceled' || + state === 'rejected' + ); +} + +/** + * Type guard to check if a value is a non-array object. + */ +function isObject(val: unknown): val is Record { + return typeof val === 'object' && val !== null && !Array.isArray(val); +} From 5d213764fb33f6a725f93efeda97718462854b9a Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Tue, 10 Mar 2026 12:24:54 -0700 Subject: [PATCH 07/15] feat(cli): enable skill activation via slash commands (#21758) Co-authored-by: matt korwel --- .../src/services/SkillCommandLoader.test.ts | 125 ++++++++++++++++++ .../cli/src/services/SkillCommandLoader.ts | 53 ++++++++ packages/cli/src/ui/commands/types.ts | 1 + .../cli/src/ui/hooks/slashCommandProcessor.ts | 3 + packages/cli/src/ui/hooks/useGeminiStream.ts | 12 +- packages/cli/src/ui/types.ts | 1 + packages/core/src/commands/types.ts | 5 + packages/core/src/scheduler/policy.test.ts | 37 ++++++ packages/core/src/scheduler/policy.ts | 13 ++ 9 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/services/SkillCommandLoader.test.ts create mode 100644 packages/cli/src/services/SkillCommandLoader.ts diff --git a/packages/cli/src/services/SkillCommandLoader.test.ts b/packages/cli/src/services/SkillCommandLoader.test.ts new file mode 100644 index 0000000000..15a2ebec18 --- /dev/null +++ b/packages/cli/src/services/SkillCommandLoader.test.ts @@ -0,0 +1,125 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { vi, describe, it, expect, beforeEach } from 'vitest'; +import { SkillCommandLoader } from './SkillCommandLoader.js'; +import { CommandKind } from '../ui/commands/types.js'; +import { ACTIVATE_SKILL_TOOL_NAME } from '@google/gemini-cli-core'; + +describe('SkillCommandLoader', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let mockConfig: any; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let mockSkillManager: any; + + beforeEach(() => { + mockSkillManager = { + getDisplayableSkills: vi.fn(), + isAdminEnabled: vi.fn().mockReturnValue(true), + }; + + mockConfig = { + isSkillsSupportEnabled: vi.fn().mockReturnValue(true), + getSkillManager: vi.fn().mockReturnValue(mockSkillManager), + }; + }); + + it('should return an empty array if skills support is disabled', async () => { + mockConfig.isSkillsSupportEnabled.mockReturnValue(false); + const loader = new SkillCommandLoader(mockConfig); + const commands = await loader.loadCommands(new AbortController().signal); + expect(commands).toEqual([]); + }); + + it('should return an empty array if SkillManager is missing', async () => { + mockConfig.getSkillManager.mockReturnValue(null); + const loader = new SkillCommandLoader(mockConfig); + const commands = await loader.loadCommands(new AbortController().signal); + expect(commands).toEqual([]); + }); + + it('should return an empty array if skills are admin-disabled', async () => { + mockSkillManager.isAdminEnabled.mockReturnValue(false); + const loader = new SkillCommandLoader(mockConfig); + const commands = await loader.loadCommands(new AbortController().signal); + expect(commands).toEqual([]); + }); + + it('should load skills as slash commands', async () => { + const mockSkills = [ + { name: 'skill1', description: 'Description 1' }, + { name: 'skill2', description: '' }, + ]; + mockSkillManager.getDisplayableSkills.mockReturnValue(mockSkills); + + const loader = new SkillCommandLoader(mockConfig); + const commands = await loader.loadCommands(new AbortController().signal); + + expect(commands).toHaveLength(2); + + expect(commands[0]).toMatchObject({ + name: 'skill1', + description: 'Description 1', + kind: CommandKind.SKILL, + autoExecute: true, + }); + + expect(commands[1]).toMatchObject({ + name: 'skill2', + description: 'Activate the skill2 skill', + kind: CommandKind.SKILL, + autoExecute: true, + }); + }); + + it('should return a tool action when a skill command is executed', async () => { + const mockSkills = [{ name: 'test-skill', description: 'Test skill' }]; + mockSkillManager.getDisplayableSkills.mockReturnValue(mockSkills); + + const loader = new SkillCommandLoader(mockConfig); + const commands = await loader.loadCommands(new AbortController().signal); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const actionResult = await commands[0].action!({} as any, ''); + expect(actionResult).toEqual({ + type: 'tool', + toolName: ACTIVATE_SKILL_TOOL_NAME, + toolArgs: { name: 'test-skill' }, + postSubmitPrompt: undefined, + }); + }); + + it('should return a tool action with postSubmitPrompt when args are provided', async () => { + const mockSkills = [{ name: 'test-skill', description: 'Test skill' }]; + mockSkillManager.getDisplayableSkills.mockReturnValue(mockSkills); + + const loader = new SkillCommandLoader(mockConfig); + const commands = await loader.loadCommands(new AbortController().signal); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const actionResult = await commands[0].action!({} as any, 'hello world'); + expect(actionResult).toEqual({ + type: 'tool', + toolName: ACTIVATE_SKILL_TOOL_NAME, + toolArgs: { name: 'test-skill' }, + postSubmitPrompt: 'hello world', + }); + }); + + it('should sanitize skill names with spaces', async () => { + const mockSkills = [{ name: 'my awesome skill', description: 'Desc' }]; + mockSkillManager.getDisplayableSkills.mockReturnValue(mockSkills); + + const loader = new SkillCommandLoader(mockConfig); + const commands = await loader.loadCommands(new AbortController().signal); + + expect(commands[0].name).toBe('my-awesome-skill'); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const actionResult = (await commands[0].action!({} as any, '')) as any; + expect(actionResult.toolArgs).toEqual({ name: 'my awesome skill' }); + }); +}); diff --git a/packages/cli/src/services/SkillCommandLoader.ts b/packages/cli/src/services/SkillCommandLoader.ts new file mode 100644 index 0000000000..85f1884299 --- /dev/null +++ b/packages/cli/src/services/SkillCommandLoader.ts @@ -0,0 +1,53 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { type Config, ACTIVATE_SKILL_TOOL_NAME } from '@google/gemini-cli-core'; +import { CommandKind, type SlashCommand } from '../ui/commands/types.js'; +import { type ICommandLoader } from './types.js'; + +/** + * Loads Agent Skills as slash commands. + */ +export class SkillCommandLoader implements ICommandLoader { + constructor(private config: Config | null) {} + + /** + * Discovers all available skills from the SkillManager and converts + * them into executable slash commands. + * + * @param _signal An AbortSignal (unused for this synchronous loader). + * @returns A promise that resolves to an array of `SlashCommand` objects. + */ + async loadCommands(_signal: AbortSignal): Promise { + if (!this.config || !this.config.isSkillsSupportEnabled()) { + return []; + } + + const skillManager = this.config.getSkillManager(); + if (!skillManager || !skillManager.isAdminEnabled()) { + return []; + } + + // Convert all displayable skills into slash commands. + const skills = skillManager.getDisplayableSkills(); + + return skills.map((skill) => { + const commandName = skill.name.trim().replace(/\s+/g, '-'); + return { + name: commandName, + description: skill.description || `Activate the ${skill.name} skill`, + kind: CommandKind.SKILL, + autoExecute: true, + action: async (_context, args) => ({ + type: 'tool', + toolName: ACTIVATE_SKILL_TOOL_NAME, + toolArgs: { name: skill.name }, + postSubmitPrompt: args.trim().length > 0 ? args.trim() : undefined, + }), + }; + }); + } +} diff --git a/packages/cli/src/ui/commands/types.ts b/packages/cli/src/ui/commands/types.ts index e4f0d0ad52..28f52461e4 100644 --- a/packages/cli/src/ui/commands/types.ts +++ b/packages/cli/src/ui/commands/types.ts @@ -182,6 +182,7 @@ export enum CommandKind { EXTENSION_FILE = 'extension-file', MCP_PROMPT = 'mcp-prompt', AGENT = 'agent', + SKILL = 'skill', } // The standardized contract for any command in the system. diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 20a76dcf43..6f3ecd7b96 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -52,6 +52,7 @@ import { CommandService } from '../../services/CommandService.js'; import { BuiltinCommandLoader } from '../../services/BuiltinCommandLoader.js'; import { FileCommandLoader } from '../../services/FileCommandLoader.js'; import { McpPromptLoader } from '../../services/McpPromptLoader.js'; +import { SkillCommandLoader } from '../../services/SkillCommandLoader.js'; import { parseSlashCommand } from '../../utils/commands.js'; import { type ExtensionUpdateAction, @@ -324,6 +325,7 @@ export const useSlashCommandProcessor = ( (async () => { const commandService = await CommandService.create( [ + new SkillCommandLoader(config), new McpPromptLoader(config), new BuiltinCommandLoader(config), new FileCommandLoader(config), @@ -445,6 +447,7 @@ export const useSlashCommandProcessor = ( type: 'schedule_tool', toolName: result.toolName, toolArgs: result.toolArgs, + postSubmitPrompt: result.postSubmitPrompt, }; case 'message': addItem( diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index d2e485db1f..6b6c4554f2 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -759,7 +759,8 @@ export const useGeminiStream = ( if (slashCommandResult) { switch (slashCommandResult.type) { case 'schedule_tool': { - const { toolName, toolArgs } = slashCommandResult; + const { toolName, toolArgs, postSubmitPrompt } = + slashCommandResult; const toolCallRequest: ToolCallRequestInfo = { callId: `${toolName}-${Date.now()}-${Math.random().toString(16).slice(2)}`, name: toolName, @@ -768,6 +769,15 @@ export const useGeminiStream = ( prompt_id, }; await scheduleToolCalls([toolCallRequest], abortSignal); + + if (postSubmitPrompt) { + localQueryToSendToGemini = postSubmitPrompt; + return { + queryToSend: localQueryToSendToGemini, + shouldProceed: true, + }; + } + return { queryToSend: null, shouldProceed: false }; } case 'submit_prompt': { diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index 3898461fb0..2f8e414a83 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -483,6 +483,7 @@ export type SlashCommandProcessorResult = type: 'schedule_tool'; toolName: string; toolArgs: Record; + postSubmitPrompt?: PartListUnion; } | { type: 'handled'; // Indicates the command was processed and no further action is needed. diff --git a/packages/core/src/commands/types.ts b/packages/core/src/commands/types.ts index d9cc7a24e9..62bda279af 100644 --- a/packages/core/src/commands/types.ts +++ b/packages/core/src/commands/types.ts @@ -12,6 +12,11 @@ export interface ToolActionReturn { type: 'tool'; toolName: string; toolArgs: Record; + /** + * Optional content to be submitted as a prompt to the Gemini model + * after the tool call completes. + */ + postSubmitPrompt?: PartListUnion; } /** diff --git a/packages/core/src/scheduler/policy.test.ts b/packages/core/src/scheduler/policy.test.ts index 4bf2b32a46..fc81d2dc69 100644 --- a/packages/core/src/scheduler/policy.test.ts +++ b/packages/core/src/scheduler/policy.test.ts @@ -164,6 +164,43 @@ describe('policy.ts', () => { const result = await checkPolicy(toolCall, mockConfig); expect(result.decision).toBe(PolicyDecision.ASK_USER); }); + + it('should return ALLOW if decision is ASK_USER and request is client-initiated', async () => { + const mockPolicyEngine = { + check: vi.fn().mockResolvedValue({ decision: PolicyDecision.ASK_USER }), + } as unknown as Mocked; + + const mockConfig = { + getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), + isInteractive: vi.fn().mockReturnValue(true), + } as unknown as Mocked; + + const toolCall = { + request: { name: 'test-tool', args: {}, isClientInitiated: true }, + tool: { name: 'test-tool' }, + } as ValidatingToolCall; + + const result = await checkPolicy(toolCall, mockConfig); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + + it('should still return DENY if request is client-initiated but policy says DENY', async () => { + const mockPolicyEngine = { + check: vi.fn().mockResolvedValue({ decision: PolicyDecision.DENY }), + } as unknown as Mocked; + + const mockConfig = { + getPolicyEngine: vi.fn().mockReturnValue(mockPolicyEngine), + } as unknown as Mocked; + + const toolCall = { + request: { name: 'test-tool', args: {}, isClientInitiated: true }, + tool: { name: 'test-tool' }, + } as ValidatingToolCall; + + const result = await checkPolicy(toolCall, mockConfig); + expect(result.decision).toBe(PolicyDecision.DENY); + }); }); describe('updatePolicy', () => { diff --git a/packages/core/src/scheduler/policy.ts b/packages/core/src/scheduler/policy.ts index 1ac70a108b..c0ea06f59b 100644 --- a/packages/core/src/scheduler/policy.ts +++ b/packages/core/src/scheduler/policy.ts @@ -69,6 +69,19 @@ export async function checkPolicy( const { decision } = result; + // If the tool call was initiated by the client (e.g. via a slash command), + // we treat it as implicitly confirmed by the user and bypass the + // confirmation prompt if the policy engine's decision is 'ASK_USER'. + if ( + decision === PolicyDecision.ASK_USER && + toolCall.request.isClientInitiated + ) { + return { + decision: PolicyDecision.ALLOW, + rule: result.rule, + }; + } + /* * Return the full check result including the rule that matched. * This is necessary to access metadata like custom deny messages. From bc75a6198298560d2ab533c8b3f5404c40536bcc Mon Sep 17 00:00:00 2001 From: Yongrui Lin Date: Tue, 10 Mar 2026 12:29:18 -0700 Subject: [PATCH 08/15] docs(cli): mention per-model token usage in stream-json result event (#21908) --- docs/cli/headless.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/cli/headless.md b/docs/cli/headless.md index dd9a385313..c83ce70d0e 100644 --- a/docs/cli/headless.md +++ b/docs/cli/headless.md @@ -31,7 +31,8 @@ Returns a stream of newline-delimited JSON (JSONL) events. - `tool_use`: Tool call requests with arguments. - `tool_result`: Output from executed tools. - `error`: Non-fatal warnings and system errors. - - `result`: Final outcome with aggregated statistics. + - `result`: Final outcome with aggregated statistics and per-model token usage + breakdowns. ## Exit codes From e5615f47c45730839daec95ca3ca264ef1db4541 Mon Sep 17 00:00:00 2001 From: Adib234 <30782825+Adib234@users.noreply.github.com> Date: Tue, 10 Mar 2026 15:34:10 -0400 Subject: [PATCH 09/15] fix(plan): prevent plan truncation in approval dialog by supporting unconstrained heights (#21037) Co-authored-by: jacob314 --- packages/cli/src/ui/AppContainer.tsx | 6 +----- packages/cli/src/ui/components/AskUserDialog.tsx | 15 ++++++++++----- .../cli/src/ui/components/ExitPlanModeDialog.tsx | 1 + .../ui/components/ToolConfirmationQueue.test.tsx | 5 ++++- packages/core/src/confirmation-bus/types.ts | 2 ++ 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 42d40ec73a..c3288ee728 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1389,11 +1389,7 @@ Logging in with Google... Restarting Gemini CLI to continue. // Compute available terminal height based on controls measurement const availableTerminalHeight = Math.max( 0, - terminalHeight - - controlsHeight - - staticExtraHeight - - 2 - - backgroundShellHeight, + terminalHeight - controlsHeight - backgroundShellHeight - 1, ); config.setShellExecutionConfig({ diff --git a/packages/cli/src/ui/components/AskUserDialog.tsx b/packages/cli/src/ui/components/AskUserDialog.tsx index 3c8ccbfb34..4233616144 100644 --- a/packages/cli/src/ui/components/AskUserDialog.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.tsx @@ -807,16 +807,21 @@ const ChoiceQuestionView: React.FC = ({ const TITLE_MARGIN = 1; const FOOTER_HEIGHT = 2; // DialogFooter + margin const overhead = HEADER_HEIGHT + TITLE_MARGIN + FOOTER_HEIGHT; + const listHeight = availableHeight ? Math.max(1, availableHeight - overhead) : undefined; - const questionHeight = + + const questionHeightLimit = listHeight && !isAlternateBuffer - ? Math.min(15, Math.max(1, listHeight - DIALOG_PADDING)) + ? question.unconstrainedHeight + ? Math.max(1, listHeight - selectionItems.length * 2) + : Math.min(15, Math.max(1, listHeight - DIALOG_PADDING)) : undefined; + const maxItemsToShow = - listHeight && questionHeight - ? Math.max(1, Math.floor((listHeight - questionHeight) / 2)) + listHeight && questionHeightLimit + ? Math.max(1, Math.floor((listHeight - questionHeightLimit) / 2)) : selectionItems.length; return ( @@ -824,7 +829,7 @@ const ChoiceQuestionView: React.FC = ({ {progressHeader} diff --git a/packages/cli/src/ui/components/ExitPlanModeDialog.tsx b/packages/cli/src/ui/components/ExitPlanModeDialog.tsx index ec5a4c2a9b..4124a7c6d7 100644 --- a/packages/cli/src/ui/components/ExitPlanModeDialog.tsx +++ b/packages/cli/src/ui/components/ExitPlanModeDialog.tsx @@ -249,6 +249,7 @@ export const ExitPlanModeDialog: React.FC = ({ ], placeholder: 'Type your feedback...', multiSelect: false, + unconstrainedHeight: false, }, ]} onSubmit={(answers) => { diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx index 7b45bd0458..ab12ae496f 100644 --- a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx @@ -282,7 +282,10 @@ describe('ToolConfirmationQueue', () => { // hideToolIdentity is true for ask_user -> subtracts 4 instead of 6 // availableContentHeight = 19 - 4 = 15 // ToolConfirmationMessage handlesOwnUI=true -> returns full 15 - // AskUserDialog uses 15 lines to render its multi-line question and options. + // AskUserDialog allocates questionHeight = availableHeight - overhead - DIALOG_PADDING. + // listHeight = 15 - overhead (Header:0, Margin:1, Footer:2) = 12. + // maxQuestionHeight = listHeight - 4 = 8. + // 8 lines is enough for the 6-line question. await waitFor(() => { expect(lastFrame()).toContain('Line 6'); expect(lastFrame()).not.toContain('lines hidden'); diff --git a/packages/core/src/confirmation-bus/types.ts b/packages/core/src/confirmation-bus/types.ts index 99df9da616..91aeab8308 100644 --- a/packages/core/src/confirmation-bus/types.ts +++ b/packages/core/src/confirmation-bus/types.ts @@ -167,6 +167,8 @@ export interface Question { multiSelect?: boolean; /** Placeholder hint text. For type='text', shown in the input field. For type='choice', shown in the "Other" custom input. */ placeholder?: string; + /** Allow the question to consume more vertical space instead of being strictly capped. */ + unconstrainedHeight?: boolean; } export interface AskUserRequest { From 1b6963703291c2503245933c9a34d36c296a31c1 Mon Sep 17 00:00:00 2001 From: Coco Sheng Date: Tue, 10 Mar 2026 15:36:17 -0400 Subject: [PATCH 10/15] feat(a2a): switch from callback-based to event-driven tool scheduler (#21467) Co-authored-by: Abhi Co-authored-by: Adam Weidman --- .../a2a-server/src/agent/executor.test.ts | 248 +++++++ packages/a2a-server/src/agent/executor.ts | 71 +- .../src/agent/task-event-driven.test.ts | 655 ++++++++++++++++++ packages/a2a-server/src/agent/task.test.ts | 30 +- packages/a2a-server/src/agent/task.ts | 318 +++++++-- packages/a2a-server/src/config/config.ts | 2 + packages/a2a-server/src/config/settings.ts | 6 + .../a2a-server/src/utils/testing_utils.ts | 1 + .../core/src/policy/policy-engine.test.ts | 42 ++ packages/core/src/policy/policy-engine.ts | 9 + 10 files changed, 1323 insertions(+), 59 deletions(-) create mode 100644 packages/a2a-server/src/agent/executor.test.ts create mode 100644 packages/a2a-server/src/agent/task-event-driven.test.ts diff --git a/packages/a2a-server/src/agent/executor.test.ts b/packages/a2a-server/src/agent/executor.test.ts new file mode 100644 index 0000000000..2b77f3006c --- /dev/null +++ b/packages/a2a-server/src/agent/executor.test.ts @@ -0,0 +1,248 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { CoderAgentExecutor } from './executor.js'; +import type { + ExecutionEventBus, + RequestContext, + TaskStore, +} from '@a2a-js/sdk/server'; +import { EventEmitter } from 'node:events'; +import { requestStorage } from '../http/requestStorage.js'; + +// Mocks for constructor dependencies +vi.mock('../config/config.js', () => ({ + loadConfig: vi.fn().mockReturnValue({ + getSessionId: () => 'test-session', + getTargetDir: () => '/tmp', + getCheckpointingEnabled: () => false, + }), + loadEnvironment: vi.fn(), + setTargetDir: vi.fn().mockReturnValue('/tmp'), +})); + +vi.mock('../config/settings.js', () => ({ + loadSettings: vi.fn().mockReturnValue({}), +})); + +vi.mock('../config/extension.js', () => ({ + loadExtensions: vi.fn().mockReturnValue([]), +})); + +vi.mock('../http/requestStorage.js', () => ({ + requestStorage: { + getStore: vi.fn(), + }, +})); + +vi.mock('./task.js', () => { + const mockTaskInstance = (taskId: string, contextId: string) => ({ + id: taskId, + contextId, + taskState: 'working', + acceptUserMessage: vi + .fn() + .mockImplementation(async function* (context, aborted) { + const isConfirmation = ( + context.userMessage.parts as Array<{ kind: string }> + ).some((p) => p.kind === 'confirmation'); + // Hang only for main user messages (text), allow confirmations to finish quickly + if (!isConfirmation && aborted) { + await new Promise((resolve) => { + aborted.addEventListener('abort', resolve, { once: true }); + }); + } + yield { type: 'content', value: 'hello' }; + }), + acceptAgentMessage: vi.fn().mockResolvedValue(undefined), + scheduleToolCalls: vi.fn().mockResolvedValue(undefined), + waitForPendingTools: vi.fn().mockResolvedValue(undefined), + getAndClearCompletedTools: vi.fn().mockReturnValue([]), + addToolResponsesToHistory: vi.fn(), + sendCompletedToolsToLlm: vi.fn().mockImplementation(async function* () {}), + cancelPendingTools: vi.fn(), + setTaskStateAndPublishUpdate: vi.fn(), + dispose: vi.fn(), + getMetadata: vi.fn().mockResolvedValue({}), + geminiClient: { + initialize: vi.fn().mockResolvedValue(undefined), + }, + toSDKTask: () => ({ + id: taskId, + contextId, + kind: 'task', + status: { state: 'working', timestamp: new Date().toISOString() }, + metadata: {}, + history: [], + artifacts: [], + }), + }); + + const MockTask = vi.fn().mockImplementation(mockTaskInstance); + (MockTask as unknown as { create: Mock }).create = vi + .fn() + .mockImplementation(async (taskId: string, contextId: string) => + mockTaskInstance(taskId, contextId), + ); + + return { Task: MockTask }; +}); + +describe('CoderAgentExecutor', () => { + let executor: CoderAgentExecutor; + let mockTaskStore: TaskStore; + let mockEventBus: ExecutionEventBus; + + beforeEach(() => { + vi.clearAllMocks(); + mockTaskStore = { + save: vi.fn().mockResolvedValue(undefined), + load: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + list: vi.fn().mockResolvedValue([]), + } as unknown as TaskStore; + + mockEventBus = new EventEmitter() as unknown as ExecutionEventBus; + mockEventBus.publish = vi.fn(); + mockEventBus.finished = vi.fn(); + + executor = new CoderAgentExecutor(mockTaskStore); + }); + + it('should distinguish between primary and secondary execution', async () => { + const taskId = 'test-task'; + const contextId = 'test-context'; + + const mockSocket = new EventEmitter(); + const requestContext = { + userMessage: { + messageId: 'msg-1', + taskId, + contextId, + parts: [{ kind: 'text', text: 'hi' }], + metadata: { + coderAgent: { kind: 'agent-settings', workspacePath: '/tmp' }, + }, + }, + } as unknown as RequestContext; + + // Mock requestStorage for primary + (requestStorage.getStore as Mock).mockReturnValue({ + req: { socket: mockSocket }, + }); + + // First execution (Primary) + const primaryPromise = executor.execute(requestContext, mockEventBus); + + // Give it enough time to reach line 490 in executor.ts + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect( + ( + executor as unknown as { executingTasks: Set } + ).executingTasks.has(taskId), + ).toBe(true); + const wrapper = executor.getTask(taskId); + expect(wrapper).toBeDefined(); + + // Mock requestStorage for secondary + const secondarySocket = new EventEmitter(); + (requestStorage.getStore as Mock).mockReturnValue({ + req: { socket: secondarySocket }, + }); + + const secondaryRequestContext = { + userMessage: { + messageId: 'msg-2', + taskId, + contextId, + parts: [{ kind: 'confirmation', callId: '1', outcome: 'proceed' }], + metadata: { + coderAgent: { kind: 'agent-settings', workspacePath: '/tmp' }, + }, + }, + } as unknown as RequestContext; + + const secondaryPromise = executor.execute( + secondaryRequestContext, + mockEventBus, + ); + + // Secondary execution should NOT add to executingTasks (already there) + // and should return early after its loop + await secondaryPromise; + + // Task should still be in executingTasks and NOT disposed + expect( + ( + executor as unknown as { executingTasks: Set } + ).executingTasks.has(taskId), + ).toBe(true); + expect(wrapper?.task.dispose).not.toHaveBeenCalled(); + + // Now simulate secondary socket closure - it should NOT affect primary + secondarySocket.emit('end'); + expect( + ( + executor as unknown as { executingTasks: Set } + ).executingTasks.has(taskId), + ).toBe(true); + expect(wrapper?.task.dispose).not.toHaveBeenCalled(); + + // Set to terminal state to verify disposal on finish + wrapper!.task.taskState = 'completed'; + + // Now close primary socket + mockSocket.emit('end'); + + await primaryPromise; + + expect( + ( + executor as unknown as { executingTasks: Set } + ).executingTasks.has(taskId), + ).toBe(false); + expect(wrapper?.task.dispose).toHaveBeenCalled(); + }); + + it('should evict task from cache when it reaches terminal state', async () => { + const taskId = 'test-task-terminal'; + const contextId = 'test-context'; + + const mockSocket = new EventEmitter(); + (requestStorage.getStore as Mock).mockReturnValue({ + req: { socket: mockSocket }, + }); + + const requestContext = { + userMessage: { + messageId: 'msg-1', + taskId, + contextId, + parts: [{ kind: 'text', text: 'hi' }], + metadata: { + coderAgent: { kind: 'agent-settings', workspacePath: '/tmp' }, + }, + }, + } as unknown as RequestContext; + + const primaryPromise = executor.execute(requestContext, mockEventBus); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const wrapper = executor.getTask(taskId)!; + expect(wrapper).toBeDefined(); + // Simulate terminal state + wrapper.task.taskState = 'completed'; + + // Finish primary execution + mockSocket.emit('end'); + await primaryPromise; + + expect(executor.getTask(taskId)).toBeUndefined(); + expect(wrapper.task.dispose).toHaveBeenCalled(); + }); +}); diff --git a/packages/a2a-server/src/agent/executor.ts b/packages/a2a-server/src/agent/executor.ts index 7fc35657fb..dbb8269376 100644 --- a/packages/a2a-server/src/agent/executor.ts +++ b/packages/a2a-server/src/agent/executor.ts @@ -252,6 +252,10 @@ export class CoderAgentExecutor implements AgentExecutor { ); await this.taskStore?.save(wrapper.toSDKTask()); logger.info(`[CoderAgentExecutor] Task ${taskId} state CANCELED saved.`); + + // Cleanup listener subscriptions to avoid memory leaks. + wrapper.task.dispose(); + this.tasks.delete(taskId); } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Unknown error'; @@ -320,23 +324,26 @@ export class CoderAgentExecutor implements AgentExecutor { if (store) { // Grab the raw socket from the request object const socket = store.req.socket; - const onClientEnd = () => { + const onSocketEnd = () => { logger.info( - `[CoderAgentExecutor] Client socket closed for task ${taskId}. Cancelling execution.`, + `[CoderAgentExecutor] Socket ended for message ${userMessage.messageId} (task ${taskId}). Aborting execution loop.`, ); if (!abortController.signal.aborted) { abortController.abort(); } // Clean up the listener to prevent memory leaks - socket.removeListener('close', onClientEnd); + socket.removeListener('end', onSocketEnd); }; // Listen on the socket's 'end' event (remote closed the connection) - socket.on('end', onClientEnd); + socket.on('end', onSocketEnd); + socket.once('close', () => { + socket.removeListener('end', onSocketEnd); + }); // It's also good practice to remove the listener if the task completes successfully abortSignal.addEventListener('abort', () => { - socket.removeListener('end', onClientEnd); + socket.removeListener('end', onSocketEnd); }); logger.info( `[CoderAgentExecutor] Socket close handler set up for task ${taskId}.`, @@ -457,6 +464,26 @@ export class CoderAgentExecutor implements AgentExecutor { return; } + // Check if this is the primary/initial execution for this task + const isPrimaryExecution = !this.executingTasks.has(taskId); + + if (!isPrimaryExecution) { + logger.info( + `[CoderAgentExecutor] Primary execution already active for task ${taskId}. Starting secondary loop for message ${userMessage.messageId}.`, + ); + currentTask.eventBus = eventBus; + for await (const _ of currentTask.acceptUserMessage( + requestContext, + abortController.signal, + )) { + logger.info( + `[CoderAgentExecutor] Processing user message ${userMessage.messageId} in secondary execution loop for task ${taskId}.`, + ); + } + // End this execution-- the original/source will be resumed. + return; + } + logger.info( `[CoderAgentExecutor] Starting main execution for message ${userMessage.messageId} for task ${taskId}.`, ); @@ -598,18 +625,30 @@ export class CoderAgentExecutor implements AgentExecutor { } } } finally { - this.executingTasks.delete(taskId); - logger.info( - `[CoderAgentExecutor] Saving final state for task ${taskId}.`, - ); - try { - await this.taskStore?.save(wrapper.toSDKTask()); - logger.info(`[CoderAgentExecutor] Task ${taskId} state saved.`); - } catch (saveError) { - logger.error( - `[CoderAgentExecutor] Failed to save task ${taskId} state in finally block:`, - saveError, + if (isPrimaryExecution) { + this.executingTasks.delete(taskId); + logger.info( + `[CoderAgentExecutor] Saving final state for task ${taskId}.`, ); + try { + await this.taskStore?.save(wrapper.toSDKTask()); + logger.info(`[CoderAgentExecutor] Task ${taskId} state saved.`); + } catch (saveError) { + logger.error( + `[CoderAgentExecutor] Failed to save task ${taskId} state in finally block:`, + saveError, + ); + } + + if ( + ['canceled', 'failed', 'completed'].includes(currentTask.taskState) + ) { + logger.info( + `[CoderAgentExecutor] Task ${taskId} reached terminal state ${currentTask.taskState}. Evicting and disposing.`, + ); + wrapper.task.dispose(); + this.tasks.delete(taskId); + } } } } diff --git a/packages/a2a-server/src/agent/task-event-driven.test.ts b/packages/a2a-server/src/agent/task-event-driven.test.ts new file mode 100644 index 0000000000..f9dda8a752 --- /dev/null +++ b/packages/a2a-server/src/agent/task-event-driven.test.ts @@ -0,0 +1,655 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { Task } from './task.js'; +import { + type Config, + MessageBusType, + ToolConfirmationOutcome, + ApprovalMode, + Scheduler, + type MessageBus, +} from '@google/gemini-cli-core'; +import { createMockConfig } from '../utils/testing_utils.js'; +import type { ExecutionEventBus } from '@a2a-js/sdk/server'; + +describe('Task Event-Driven Scheduler', () => { + let mockConfig: Config; + let mockEventBus: ExecutionEventBus; + let messageBus: MessageBus; + + beforeEach(() => { + vi.clearAllMocks(); + mockConfig = createMockConfig({ + isEventDrivenSchedulerEnabled: () => true, + }) as Config; + messageBus = mockConfig.getMessageBus(); + mockEventBus = { + publish: vi.fn(), + on: vi.fn(), + off: vi.fn(), + once: vi.fn(), + removeAllListeners: vi.fn(), + finished: vi.fn(), + }; + }); + + it('should instantiate Scheduler when enabled', () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + expect(task.scheduler).toBeInstanceOf(Scheduler); + }); + + it('should subscribe to TOOL_CALLS_UPDATE and map status changes', async () => { + // @ts-expect-error - Calling private constructor + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall = { + request: { callId: '1', name: 'ls', args: {} }, + status: 'executing', + }; + + // Simulate MessageBus event + // Simulate MessageBus event + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + + if (!handler) { + throw new Error('TOOL_CALLS_UPDATE handler not found'); + } + + handler({ + type: MessageBusType.TOOL_CALLS_UPDATE, + toolCalls: [toolCall], + }); + + expect(mockEventBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + status: expect.objectContaining({ + state: 'submitted', // initial task state + }), + metadata: expect.objectContaining({ + coderAgent: expect.objectContaining({ + kind: 'tool-call-update', + }), + }), + }), + ); + }); + + it('should handle tool confirmations by publishing to MessageBus', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall = { + request: { callId: '1', name: 'ls', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-1', + confirmationDetails: { type: 'info', title: 'test', prompt: 'test' }, + }; + + // Simulate MessageBus event to stash the correlationId + // Simulate MessageBus event + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + + if (!handler) { + throw new Error('TOOL_CALLS_UPDATE handler not found'); + } + + handler({ + type: MessageBusType.TOOL_CALLS_UPDATE, + toolCalls: [toolCall], + }); + + // Simulate A2A client confirmation + const part = { + kind: 'data', + data: { + callId: '1', + outcome: 'proceed_once', + }, + }; + + const handled = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart(part); + expect(handled).toBe(true); + + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-1', + confirmed: true, + outcome: ToolConfirmationOutcome.ProceedOnce, + }), + ); + }); + + it('should handle Rejection (Cancel) and Modification (ModifyWithEditor)', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall = { + request: { callId: '1', name: 'ls', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-1', + confirmationDetails: { type: 'info', title: 'test', prompt: 'test' }, + }; + + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] }); + + // Simulate Rejection (Cancel) + const handled = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: '1', outcome: 'cancel' }, + }); + expect(handled).toBe(true); + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-1', + confirmed: false, + }), + ); + + const toolCall2 = { + request: { callId: '2', name: 'ls', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-2', + confirmationDetails: { type: 'info', title: 'test', prompt: 'test' }, + }; + handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall2] }); + + // Simulate ModifyWithEditor + const handled2 = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: '2', outcome: 'modify_with_editor' }, + }); + expect(handled2).toBe(true); + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-2', + confirmed: false, + outcome: ToolConfirmationOutcome.ModifyWithEditor, + payload: undefined, + }), + ); + }); + + it('should handle MCP Server tool operations correctly', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall = { + request: { callId: '1', name: 'call_mcp_tool', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-mcp-1', + confirmationDetails: { + type: 'mcp', + title: 'MCP Server Operation', + prompt: 'test_mcp', + }, + }; + + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] }); + + // Simulate ProceedOnce for MCP + const handled = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: '1', outcome: 'proceed_once' }, + }); + expect(handled).toBe(true); + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-mcp-1', + confirmed: true, + outcome: ToolConfirmationOutcome.ProceedOnce, + }), + ); + }); + + it('should handle MCP Server tool ProceedAlwaysServer outcome', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall = { + request: { callId: '1', name: 'call_mcp_tool', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-mcp-2', + confirmationDetails: { + type: 'mcp', + title: 'MCP Server Operation', + prompt: 'test_mcp', + }, + }; + + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] }); + + const handled = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: '1', outcome: 'proceed_always_server' }, + }); + expect(handled).toBe(true); + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-mcp-2', + confirmed: true, + outcome: ToolConfirmationOutcome.ProceedAlwaysServer, + }), + ); + }); + + it('should handle MCP Server tool ProceedAlwaysTool outcome', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall = { + request: { callId: '1', name: 'call_mcp_tool', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-mcp-3', + confirmationDetails: { + type: 'mcp', + title: 'MCP Server Operation', + prompt: 'test_mcp', + }, + }; + + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] }); + + const handled = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: '1', outcome: 'proceed_always_tool' }, + }); + expect(handled).toBe(true); + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-mcp-3', + confirmed: true, + outcome: ToolConfirmationOutcome.ProceedAlwaysTool, + }), + ); + }); + + it('should handle MCP Server tool ProceedAlwaysAndSave outcome', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall = { + request: { callId: '1', name: 'call_mcp_tool', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-mcp-4', + confirmationDetails: { + type: 'mcp', + title: 'MCP Server Operation', + prompt: 'test_mcp', + }, + }; + + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] }); + + const handled = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: '1', outcome: 'proceed_always_and_save' }, + }); + expect(handled).toBe(true); + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-mcp-4', + confirmed: true, + outcome: ToolConfirmationOutcome.ProceedAlwaysAndSave, + }), + ); + }); + + it('should execute without confirmation in YOLO mode and not transition to input-required', async () => { + // Enable YOLO mode + const yoloConfig = createMockConfig({ + isEventDrivenSchedulerEnabled: () => true, + getApprovalMode: () => ApprovalMode.YOLO, + }) as Config; + const yoloMessageBus = yoloConfig.getMessageBus(); + + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', yoloConfig, mockEventBus); + task.setTaskStateAndPublishUpdate = vi.fn(); + + const toolCall = { + request: { callId: '1', name: 'ls', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-1', + confirmationDetails: { type: 'info', title: 'test', prompt: 'test' }, + }; + + const handler = (yoloMessageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] }); + + // Should NOT auto-publish ProceedOnce anymore, because PolicyEngine handles it directly + expect(yoloMessageBus.publish).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + }), + ); + + // Should NOT transition to input-required since it was auto-approved + expect(task.setTaskStateAndPublishUpdate).not.toHaveBeenCalledWith( + 'input-required', + expect.anything(), + undefined, + undefined, + true, + ); + }); + + it('should handle output updates via the message bus', async () => { + // @ts-expect-error - Calling private constructor + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall = { + request: { callId: '1', name: 'ls', args: {} }, + status: 'executing', + liveOutput: 'chunk1', + }; + + // Simulate MessageBus event + // Simulate MessageBus event + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + + if (!handler) { + throw new Error('TOOL_CALLS_UPDATE handler not found'); + } + + handler({ + type: MessageBusType.TOOL_CALLS_UPDATE, + toolCalls: [toolCall], + }); + + // Should publish artifact update for output + expect(mockEventBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + kind: 'artifact-update', + artifact: expect.objectContaining({ + artifactId: 'tool-1-output', + parts: [{ kind: 'text', text: 'chunk1' }], + }), + }), + ); + }); + + it('should complete artifact creation without hanging', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCallId = 'create-file-123'; + task['_registerToolCall'](toolCallId, 'executing'); + + const toolCall = { + request: { + callId: toolCallId, + name: 'writeFile', + args: { path: 'test.sh' }, + }, + status: 'success', + result: { ok: true }, + }; + + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + handler({ type: MessageBusType.TOOL_CALLS_UPDATE, toolCalls: [toolCall] }); + + // The tool should be complete and registered appropriately, eventually + // triggering the toolCompletionPromise resolution when all clear. + const internalTask = task as unknown as { + completedToolCalls: unknown[]; + pendingToolCalls: Map; + }; + expect(internalTask.completedToolCalls.length).toBe(1); + expect(internalTask.pendingToolCalls.size).toBe(0); + }); + + it('should preserve messageId across multiple text chunks to prevent UI duplication', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + // Initialize the ID for the first turn (happens internally upon LLM stream) + task.currentAgentMessageId = 'test-id-123'; + + // Simulate sending multiple text chunks + task._sendTextContent('chunk 1'); + task._sendTextContent('chunk 2'); + + // Both text contents should have been published with the same messageId + const textCalls = (mockEventBus.publish as Mock).mock.calls.filter( + (call) => call[0].status?.message?.kind === 'message', + ); + expect(textCalls.length).toBe(2); + expect(textCalls[0][0].status.message.messageId).toBe('test-id-123'); + expect(textCalls[1][0].status.message.messageId).toBe('test-id-123'); + + // Simulate starting a new turn by calling getAndClearCompletedTools + // (which precedes sendCompletedToolsToLlm where a new ID is minted) + task.getAndClearCompletedTools(); + + // sendCompletedToolsToLlm internally rolls the ID forward. + // Simulate what sendCompletedToolsToLlm does: + const internalTask = task as unknown as { + setTaskStateAndPublishUpdate: (state: string, change: unknown) => void; + }; + internalTask.setTaskStateAndPublishUpdate('working', {}); + + // Simulate what sendCompletedToolsToLlm does: generate a new UUID for the next turn + task.currentAgentMessageId = 'test-id-456'; + + task._sendTextContent('chunk 3'); + + const secondTurnCalls = (mockEventBus.publish as Mock).mock.calls.filter( + (call) => call[0].status?.message?.messageId === 'test-id-456', + ); + expect(secondTurnCalls.length).toBe(1); + expect(secondTurnCalls[0][0].status.message.parts[0].text).toBe('chunk 3'); + }); + + it('should handle parallel tool calls correctly', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const toolCall1 = { + request: { callId: '1', name: 'ls', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-1', + confirmationDetails: { type: 'info', title: 'test 1', prompt: 'test 1' }, + }; + + const toolCall2 = { + request: { callId: '2', name: 'pwd', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-2', + confirmationDetails: { type: 'info', title: 'test 2', prompt: 'test 2' }, + }; + + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + + // Publish update for both tool calls simultaneously + handler({ + type: MessageBusType.TOOL_CALLS_UPDATE, + toolCalls: [toolCall1, toolCall2], + }); + + // Confirm first tool call + const handled1 = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: '1', outcome: 'proceed_once' }, + }); + expect(handled1).toBe(true); + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-1', + confirmed: true, + }), + ); + + // Confirm second tool call + const handled2 = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: '2', outcome: 'cancel' }, + }); + expect(handled2).toBe(true); + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-2', + confirmed: false, + }), + ); + }); + + it('should wait for executing tools before transitioning to input-required state', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + task.setTaskStateAndPublishUpdate = vi.fn(); + + // Register tool 1 as executing + task['_registerToolCall']('1', 'executing'); + + const toolCall1 = { + request: { callId: '1', name: 'ls', args: {} }, + status: 'executing', + }; + + const toolCall2 = { + request: { callId: '2', name: 'pwd', args: {} }, + status: 'awaiting_approval', + correlationId: 'corr-2', + confirmationDetails: { type: 'info', title: 'test 2', prompt: 'test 2' }, + }; + + const handler = (messageBus.subscribe as Mock).mock.calls.find( + (call: unknown[]) => call[0] === MessageBusType.TOOL_CALLS_UPDATE, + )?.[1]; + + handler({ + type: MessageBusType.TOOL_CALLS_UPDATE, + toolCalls: [toolCall1, toolCall2], + }); + + // Should NOT transition to input-required yet + expect(task.setTaskStateAndPublishUpdate).not.toHaveBeenCalledWith( + 'input-required', + expect.anything(), + undefined, + undefined, + true, + ); + + // Complete tool 1 + const toolCall1Complete = { + ...toolCall1, + status: 'success', + result: { ok: true }, + }; + + handler({ + type: MessageBusType.TOOL_CALLS_UPDATE, + toolCalls: [toolCall1Complete, toolCall2], + }); + + // Now it should transition + expect(task.setTaskStateAndPublishUpdate).toHaveBeenCalledWith( + 'input-required', + expect.anything(), + undefined, + undefined, + true, + ); + }); + + it('should ignore confirmations for unknown tool calls', async () => { + // @ts-expect-error - Calling private constructor + const task = new Task('task-id', 'context-id', mockConfig, mockEventBus); + + const handled = await ( + task as unknown as { + _handleToolConfirmationPart: (part: unknown) => Promise; + } + )._handleToolConfirmationPart({ + kind: 'data', + data: { callId: 'unknown-id', outcome: 'proceed_once' }, + }); + + // Should return false for unhandled tool call + expect(handled).toBe(false); + + // Should not publish anything to the message bus + expect(messageBus.publish).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/a2a-server/src/agent/task.test.ts b/packages/a2a-server/src/agent/task.test.ts index e29f669333..bf15d7fc49 100644 --- a/packages/a2a-server/src/agent/task.test.ts +++ b/packages/a2a-server/src/agent/task.test.ts @@ -504,13 +504,14 @@ describe('Task', () => { }); describe('auto-approval', () => { - it('should auto-approve tool calls when autoExecute is true', () => { + it('should NOT publish ToolCallConfirmationEvent when autoExecute is true', () => { task.autoExecute = true; const onConfirmSpy = vi.fn(); const toolCalls = [ { request: { callId: '1' }, status: 'awaiting_approval', + correlationId: 'test-corr-id', confirmationDetails: { type: 'edit', onConfirm: onConfirmSpy, @@ -524,9 +525,17 @@ describe('Task', () => { expect(onConfirmSpy).toHaveBeenCalledWith( ToolConfirmationOutcome.ProceedOnce, ); + const calls = (mockEventBus.publish as Mock).mock.calls; + // Search if ToolCallConfirmationEvent was published + const confEvent = calls.find( + (call) => + call[0].metadata?.coderAgent?.kind === + CoderAgentEvent.ToolCallConfirmationEvent, + ); + expect(confEvent).toBeUndefined(); }); - it('should auto-approve tool calls when approval mode is YOLO', () => { + it('should NOT publish ToolCallConfirmationEvent when approval mode is YOLO', () => { (mockConfig.getApprovalMode as Mock).mockReturnValue(ApprovalMode.YOLO); task.autoExecute = false; const onConfirmSpy = vi.fn(); @@ -534,6 +543,7 @@ describe('Task', () => { { request: { callId: '1' }, status: 'awaiting_approval', + correlationId: 'test-corr-id', confirmationDetails: { type: 'edit', onConfirm: onConfirmSpy, @@ -547,6 +557,14 @@ describe('Task', () => { expect(onConfirmSpy).toHaveBeenCalledWith( ToolConfirmationOutcome.ProceedOnce, ); + const calls = (mockEventBus.publish as Mock).mock.calls; + // Search if ToolCallConfirmationEvent was published + const confEvent = calls.find( + (call) => + call[0].metadata?.coderAgent?.kind === + CoderAgentEvent.ToolCallConfirmationEvent, + ); + expect(confEvent).toBeUndefined(); }); it('should NOT auto-approve when autoExecute is false and mode is not YOLO', () => { @@ -567,6 +585,14 @@ describe('Task', () => { task._schedulerToolCallsUpdate(toolCalls); expect(onConfirmSpy).not.toHaveBeenCalled(); + const calls = (mockEventBus.publish as Mock).mock.calls; + // Search if ToolCallConfirmationEvent was published + const confEvent = calls.find( + (call) => + call[0].metadata?.coderAgent?.kind === + CoderAgentEvent.ToolCallConfirmationEvent, + ); + expect(confEvent).toBeDefined(); }); }); }); diff --git a/packages/a2a-server/src/agent/task.ts b/packages/a2a-server/src/agent/task.ts index ef15a907e6..652635779b 100644 --- a/packages/a2a-server/src/agent/task.ts +++ b/packages/a2a-server/src/agent/task.ts @@ -5,6 +5,7 @@ */ import { + Scheduler, CoreToolScheduler, type GeminiClient, GeminiEventType, @@ -34,6 +35,8 @@ import { isSubagentProgress, EDIT_TOOL_NAMES, processRestorableToolCalls, + MessageBusType, + type ToolCallsUpdateMessage, } from '@google/gemini-cli-core'; import { type ExecutionEventBus, @@ -96,21 +99,30 @@ function isToolCallConfirmationDetails( export class Task { id: string; contextId: string; - scheduler: CoreToolScheduler; + scheduler: Scheduler | CoreToolScheduler; config: Config; geminiClient: GeminiClient; pendingToolConfirmationDetails: Map; + pendingCorrelationIds: Map = new Map(); taskState: TaskState; eventBus?: ExecutionEventBus; completedToolCalls: CompletedToolCall[]; + processedToolCallIds: Set = new Set(); skipFinalTrueAfterInlineEdit = false; modelInfo?: string; currentPromptId: string | undefined; + currentAgentMessageId = uuidv4(); promptCount = 0; autoExecute: boolean; + private get isYoloMatch(): boolean { + return ( + this.autoExecute || this.config.getApprovalMode() === ApprovalMode.YOLO + ); + } // For tool waiting logic private pendingToolCalls: Map = new Map(); //toolCallId --> status + private toolsAlreadyConfirmed: Set = new Set(); private toolCompletionPromise?: Promise; private toolCompletionNotifier?: { resolve: () => void; @@ -127,7 +139,13 @@ export class Task { this.id = id; this.contextId = contextId; this.config = config; - this.scheduler = this.createScheduler(); + + if (this.config.isEventDrivenSchedulerEnabled()) { + this.scheduler = this.setupEventDrivenScheduler(); + } else { + this.scheduler = this.createLegacyScheduler(); + } + this.geminiClient = this.config.getGeminiClient(); this.pendingToolConfirmationDetails = new Map(); this.taskState = 'submitted'; @@ -227,7 +245,7 @@ export class Task { logger.info( `[Task] Waiting for ${this.pendingToolCalls.size} pending tool(s)...`, ); - return this.toolCompletionPromise; + await this.toolCompletionPromise; } cancelPendingTools(reason: string): void { @@ -240,6 +258,13 @@ export class Task { this.toolCompletionNotifier.reject(new Error(reason)); } this.pendingToolCalls.clear(); + this.pendingCorrelationIds.clear(); + + if (this.scheduler instanceof Scheduler) { + this.scheduler.cancelAll(); + } else { + this.scheduler.cancelAll(new AbortController().signal); + } // Reset the promise for any future operations, ensuring it's in a clean state. this._resetToolCompletionPromise(); } @@ -252,7 +277,7 @@ export class Task { kind: 'message', role, parts: [{ kind: 'text', text }], - messageId: uuidv4(), + messageId: role === 'agent' ? this.currentAgentMessageId : uuidv4(), taskId: this.id, contextId: this.contextId, }; @@ -425,26 +450,34 @@ export class Task { // Only send an update if the status has actually changed. if (hasChanged) { - const coderAgentMessage: CoderAgentMessage = - tc.status === 'awaiting_approval' - ? { kind: CoderAgentEvent.ToolCallConfirmationEvent } - : { kind: CoderAgentEvent.ToolCallUpdateEvent }; - const message = this.toolStatusMessage(tc, this.id, this.contextId); + // Skip sending confirmation event if we are going to auto-approve it anyway + if ( + tc.status === 'awaiting_approval' && + tc.confirmationDetails && + this.isYoloMatch + ) { + logger.info( + `[Task] Skipping ToolCallConfirmationEvent for ${tc.request.callId} due to YOLO mode.`, + ); + } else { + const coderAgentMessage: CoderAgentMessage = + tc.status === 'awaiting_approval' + ? { kind: CoderAgentEvent.ToolCallConfirmationEvent } + : { kind: CoderAgentEvent.ToolCallUpdateEvent }; + const message = this.toolStatusMessage(tc, this.id, this.contextId); - const event = this._createStatusUpdateEvent( - this.taskState, - coderAgentMessage, - message, - false, // Always false for these continuous updates - ); - this.eventBus?.publish(event); + const event = this._createStatusUpdateEvent( + this.taskState, + coderAgentMessage, + message, + false, // Always false for these continuous updates + ); + this.eventBus?.publish(event); + } } }); - if ( - this.autoExecute || - this.config.getApprovalMode() === ApprovalMode.YOLO - ) { + if (this.isYoloMatch) { logger.info( '[Task] ' + (this.autoExecute ? '' : 'YOLO mode enabled. ') + @@ -492,7 +525,7 @@ export class Task { } } - private createScheduler(): CoreToolScheduler { + private createLegacyScheduler(): CoreToolScheduler { const scheduler = new CoreToolScheduler({ outputUpdateHandler: this._schedulerOutputUpdate.bind(this), onAllToolCallsComplete: this._schedulerAllToolCallsComplete.bind(this), @@ -503,6 +536,171 @@ export class Task { return scheduler; } + private messageBusListener?: (message: ToolCallsUpdateMessage) => void; + + private setupEventDrivenScheduler(): Scheduler { + const messageBus = this.config.getMessageBus(); + const scheduler = new Scheduler({ + schedulerId: this.id, + config: this.config, + messageBus, + getPreferredEditor: () => DEFAULT_GUI_EDITOR, + }); + + this.messageBusListener = this.handleEventDrivenToolCallsUpdate.bind(this); + messageBus.subscribe( + MessageBusType.TOOL_CALLS_UPDATE, + this.messageBusListener, + ); + + return scheduler; + } + + dispose(): void { + if (this.messageBusListener) { + this.config + .getMessageBus() + .unsubscribe(MessageBusType.TOOL_CALLS_UPDATE, this.messageBusListener); + this.messageBusListener = undefined; + } + + if (this.scheduler instanceof Scheduler) { + this.scheduler.dispose(); + } + } + + private handleEventDrivenToolCallsUpdate( + event: ToolCallsUpdateMessage, + ): void { + if (event.type !== MessageBusType.TOOL_CALLS_UPDATE) { + return; + } + + const toolCalls = event.toolCalls; + + toolCalls.forEach((tc) => { + this.handleEventDrivenToolCall(tc); + }); + + this.checkInputRequiredState(); + } + + private handleEventDrivenToolCall(tc: ToolCall): void { + const callId = tc.request.callId; + + // Do not process events for tools that have already been finalized. + // This prevents duplicate completions if the state manager emits a snapshot containing + // already resolved tools whose IDs were removed from pendingToolCalls. + if ( + this.processedToolCallIds.has(callId) || + this.completedToolCalls.some((c) => c.request.callId === callId) + ) { + return; + } + + const previousStatus = this.pendingToolCalls.get(callId); + const hasChanged = previousStatus !== tc.status; + + // 1. Handle Output + if (tc.status === 'executing' && tc.liveOutput) { + this._schedulerOutputUpdate(callId, tc.liveOutput); + } + + // 2. Handle terminal states + if ( + tc.status === 'success' || + tc.status === 'error' || + tc.status === 'cancelled' + ) { + this.toolsAlreadyConfirmed.delete(callId); + if (hasChanged) { + logger.info( + `[Task] Tool call ${callId} completed with status: ${tc.status}`, + ); + this.completedToolCalls.push(tc); + this._resolveToolCall(callId); + } + } else { + // Keep track of pending tools + this._registerToolCall(callId, tc.status); + } + + // 3. Handle Confirmation Stash + if (tc.status === 'awaiting_approval' && tc.confirmationDetails) { + const details = tc.confirmationDetails; + + if (tc.correlationId) { + this.pendingCorrelationIds.set(callId, tc.correlationId); + } + + this.pendingToolConfirmationDetails.set(callId, { + ...details, + onConfirm: async () => {}, + } as ToolCallConfirmationDetails); + } + + // 4. Publish Status Updates to A2A event bus + if (hasChanged) { + const coderAgentMessage: CoderAgentMessage = + tc.status === 'awaiting_approval' + ? { kind: CoderAgentEvent.ToolCallConfirmationEvent } + : { kind: CoderAgentEvent.ToolCallUpdateEvent }; + + const message = this.toolStatusMessage(tc, this.id, this.contextId); + const statusUpdate = this._createStatusUpdateEvent( + this.taskState, + coderAgentMessage, + message, + false, + ); + this.eventBus?.publish(statusUpdate); + } + } + + private checkInputRequiredState(): void { + if (this.isYoloMatch) { + return; + } + + // 6. Handle Input Required State + let isAwaitingApproval = false; + let isExecuting = false; + + for (const [callId, status] of this.pendingToolCalls.entries()) { + if (status === 'executing' || status === 'scheduled') { + isExecuting = true; + } else if ( + status === 'awaiting_approval' && + !this.toolsAlreadyConfirmed.has(callId) + ) { + isAwaitingApproval = true; + } + } + + if ( + isAwaitingApproval && + !isExecuting && + !this.skipFinalTrueAfterInlineEdit + ) { + this.skipFinalTrueAfterInlineEdit = false; + const wasAlreadyInputRequired = this.taskState === 'input-required'; + + this.setTaskStateAndPublishUpdate( + 'input-required', + { kind: CoderAgentEvent.StateChangeEvent }, + undefined, + undefined, + /*final*/ true, + ); + + // Unblock waitForPendingTools to correctly end the executor loop and release the HTTP response stream. + // The IDE client will open a new stream with the confirmation reply. + if (!wasAlreadyInputRequired && this.toolCompletionNotifier) { + this.toolCompletionNotifier.resolve(); + } + } + } + private _pickFields< T extends ToolCall | AnyDeclarativeTool, K extends UnionKeys, @@ -713,7 +911,16 @@ export class Task { }; this.setTaskStateAndPublishUpdate('working', stateChange); - await this.scheduler.schedule(updatedRequests, abortSignal); + // Pre-register tools to ensure waitForPendingTools sees them as pending + // before the async scheduler enqueues them and fires the event bus update. + for (const req of updatedRequests) { + if (!this.pendingToolCalls.has(req.callId)) { + this._registerToolCall(req.callId, 'scheduled'); + } + } + + // Fire and forget so we don't block the executor loop before waitForPendingTools can be called + void this.scheduler.schedule(updatedRequests, abortSignal); } async acceptAgentMessage(event: ServerGeminiStreamEvent): Promise { @@ -839,9 +1046,15 @@ export class Task { ) { return false; } + if (!part.data['outcome']) { + return false; + } const callId = part.data['callId']; const outcomeString = part.data['outcome']; + + this.toolsAlreadyConfirmed.add(callId); + let confirmationOutcome: ToolConfirmationOutcome | undefined; if (outcomeString === 'proceed_once') { @@ -854,6 +1067,8 @@ export class Task { confirmationOutcome = ToolConfirmationOutcome.ProceedAlwaysServer; } else if (outcomeString === 'proceed_always_tool') { confirmationOutcome = ToolConfirmationOutcome.ProceedAlwaysTool; + } else if (outcomeString === 'proceed_always_and_save') { + confirmationOutcome = ToolConfirmationOutcome.ProceedAlwaysAndSave; } else if (outcomeString === 'modify_with_editor') { confirmationOutcome = ToolConfirmationOutcome.ModifyWithEditor; } else { @@ -864,8 +1079,9 @@ export class Task { } const confirmationDetails = this.pendingToolConfirmationDetails.get(callId); + const correlationId = this.pendingCorrelationIds.get(callId); - if (!confirmationDetails) { + if (!confirmationDetails && !correlationId) { logger.warn( `[Task] Received tool confirmation for unknown or already processed callId: ${callId}`, ); @@ -887,24 +1103,35 @@ export class Task { // This will trigger the scheduler to continue or cancel the specific tool. // The scheduler's onToolCallsUpdate will then reflect the new state (e.g., executing or cancelled). - // If `edit` tool call, pass updated payload if presesent - if (confirmationDetails.type === 'edit') { - const newContent = part.data['newContent']; - const payload = - typeof newContent === 'string' - ? ({ newContent } as ToolConfirmationPayload) - : undefined; - this.skipFinalTrueAfterInlineEdit = !!payload; - try { + // If `edit` tool call, pass updated payload if present + const newContent = part.data['newContent']; + const payload = + confirmationDetails?.type === 'edit' && typeof newContent === 'string' + ? ({ newContent } as ToolConfirmationPayload) + : undefined; + this.skipFinalTrueAfterInlineEdit = !!payload; + + try { + if (correlationId) { + await this.config.getMessageBus().publish({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId, + confirmed: + confirmationOutcome !== ToolConfirmationOutcome.Cancel && + confirmationOutcome !== + ToolConfirmationOutcome.ModifyWithEditor, + outcome: confirmationOutcome, + payload, + }); + } else if (confirmationDetails?.onConfirm) { + // Fallback for legacy callback-based confirmation await confirmationDetails.onConfirm(confirmationOutcome, payload); - } finally { - // Once confirmationDetails.onConfirm finishes (or fails) with a payload, - // reset skipFinalTrueAfterInlineEdit so that external callers receive - // their call has been completed. - this.skipFinalTrueAfterInlineEdit = false; } - } else { - await confirmationDetails.onConfirm(confirmationOutcome); + } finally { + // Once confirmation payload is sent or callback finishes, + // reset skipFinalTrueAfterInlineEdit so that external callers receive + // their call has been completed. + this.skipFinalTrueAfterInlineEdit = false; } } finally { if (gcpProject) { @@ -920,6 +1147,7 @@ export class Task { // Note !== ToolConfirmationOutcome.ModifyWithEditor does not work! if (confirmationOutcome !== 'modify_with_editor') { this.pendingToolConfirmationDetails.delete(callId); + this.pendingCorrelationIds.delete(callId); } // If outcome is Cancel, scheduler should update status to 'cancelled', which then resolves the tool. @@ -953,6 +1181,9 @@ export class Task { getAndClearCompletedTools(): CompletedToolCall[] { const tools = [...this.completedToolCalls]; + for (const tool of tools) { + this.processedToolCallIds.add(tool.request.callId); + } this.completedToolCalls = []; return tools; } @@ -1013,6 +1244,7 @@ export class Task { }; // Set task state to working as we are about to call LLM this.setTaskStateAndPublishUpdate('working', stateChange); + this.currentAgentMessageId = uuidv4(); yield* this.geminiClient.sendMessageStream( llmParts, aborted, @@ -1034,6 +1266,10 @@ export class Task { if (confirmationHandled) { anyConfirmationHandled = true; // If a confirmation was handled, the scheduler will now run the tool (or cancel it). + // We resolve the toolCompletionPromise manually in checkInputRequiredState + // to break the original execution loop, so we must reset it here so the + // new loop correctly awaits the tool's final execution. + this._resetToolCompletionPromise(); // We don't send anything to the LLM for this part. // The subsequent tool execution will eventually lead to resolveToolCall. continue; @@ -1048,6 +1284,7 @@ export class Task { if (hasContentForLlm) { this.currentPromptId = this.config.getSessionId() + '########' + this.promptCount++; + this.currentAgentMessageId = uuidv4(); logger.info('[Task] Sending new parts to LLM.'); const stateChange: StateChange = { kind: CoderAgentEvent.StateChangeEvent, @@ -1093,7 +1330,6 @@ export class Task { if (content === '') { return; } - logger.info('[Task] Sending text content to event bus.'); const message = this._createTextMessage(content); const textContent: TextContent = { kind: CoderAgentEvent.TextContentEvent, @@ -1125,7 +1361,7 @@ export class Task { data: content, } as Part, ], - messageId: uuidv4(), + messageId: this.currentAgentMessageId, taskId: this.id, contextId: this.contextId, }; diff --git a/packages/a2a-server/src/config/config.ts b/packages/a2a-server/src/config/config.ts index 5b6757701d..229abc65c9 100644 --- a/packages/a2a-server/src/config/config.ts +++ b/packages/a2a-server/src/config/config.ts @@ -106,6 +106,8 @@ export async function loadConfig( trustedFolder: true, extensionLoader, checkpointing, + enableEventDrivenScheduler: + settings.experimental?.enableEventDrivenScheduler ?? true, interactive: !isHeadlessMode(), enableInteractiveShell: !isHeadlessMode(), ptyInfo: 'auto', diff --git a/packages/a2a-server/src/config/settings.ts b/packages/a2a-server/src/config/settings.ts index b3c44cc177..0c353b46aa 100644 --- a/packages/a2a-server/src/config/settings.ts +++ b/packages/a2a-server/src/config/settings.ts @@ -37,6 +37,12 @@ export interface Settings { showMemoryUsage?: boolean; checkpointing?: CheckpointingSettings; folderTrust?: boolean; + general?: { + previewFeatures?: boolean; + }; + experimental?: { + enableEventDrivenScheduler?: boolean; + }; // Git-aware file filtering settings fileFiltering?: { diff --git a/packages/a2a-server/src/utils/testing_utils.ts b/packages/a2a-server/src/utils/testing_utils.ts index 7d77d8dc9a..4981dbbd67 100644 --- a/packages/a2a-server/src/utils/testing_utils.ts +++ b/packages/a2a-server/src/utils/testing_utils.ts @@ -64,6 +64,7 @@ export function createMockConfig( getEmbeddingModel: vi.fn().mockReturnValue('text-embedding-004'), getSessionId: vi.fn().mockReturnValue('test-session-id'), getUserTier: vi.fn(), + isEventDrivenSchedulerEnabled: vi.fn().mockReturnValue(false), getMessageBus: vi.fn(), getPolicyEngine: vi.fn(), getEnableExtensionReloading: vi.fn().mockReturnValue(false), diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index baf475701c..a54da32376 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -333,6 +333,48 @@ describe('PolicyEngine', () => { PolicyDecision.ASK_USER, ); }); + + it('should return ALLOW by default in YOLO mode when no rules match', async () => { + engine = new PolicyEngine({ approvalMode: ApprovalMode.YOLO }); + + // No rules defined, should return ALLOW in YOLO mode + const { decision } = await engine.check({ name: 'any-tool' }, undefined); + expect(decision).toBe(PolicyDecision.ALLOW); + }); + + it('should NOT override explicit DENY rules in YOLO mode', async () => { + const rules: PolicyRule[] = [ + { toolName: 'dangerous-tool', decision: PolicyDecision.DENY }, + ]; + engine = new PolicyEngine({ rules, approvalMode: ApprovalMode.YOLO }); + + const { decision } = await engine.check( + { name: 'dangerous-tool' }, + undefined, + ); + expect(decision).toBe(PolicyDecision.DENY); + + // But other tools still allowed + expect( + (await engine.check({ name: 'safe-tool' }, undefined)).decision, + ).toBe(PolicyDecision.ALLOW); + }); + + it('should respect rule priority in YOLO mode when a match exists', async () => { + const rules: PolicyRule[] = [ + { + toolName: 'test-tool', + decision: PolicyDecision.ASK_USER, + priority: 10, + }, + { toolName: 'test-tool', decision: PolicyDecision.DENY, priority: 20 }, + ]; + engine = new PolicyEngine({ rules, approvalMode: ApprovalMode.YOLO }); + + // Priority 20 (DENY) should win over priority 10 (ASK_USER) + const { decision } = await engine.check({ name: 'test-tool' }, undefined); + expect(decision).toBe(PolicyDecision.DENY); + }); }); describe('addRule', () => { diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index a2f64bf356..b626666370 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -466,6 +466,15 @@ export class PolicyEngine { // Default if no rule matched if (decision === undefined) { + if (this.approvalMode === ApprovalMode.YOLO) { + debugLogger.debug( + `[PolicyEngine.check] NO MATCH in YOLO mode - using ALLOW`, + ); + return { + decision: PolicyDecision.ALLOW, + }; + } + debugLogger.debug( `[PolicyEngine.check] NO MATCH - using default decision: ${this.defaultDecision}`, ); From 9eae91a48917c03054a5c9f7fd61651d6cc49055 Mon Sep 17 00:00:00 2001 From: Ayush Debnath <139256624+Solventerritory@users.noreply.github.com> Date: Wed, 11 Mar 2026 01:27:23 +0530 Subject: [PATCH 11/15] feat(voice): implement speech-friendly response formatter (#20989) Co-authored-by: Spencer --- packages/core/src/index.ts | 3 + .../core/src/voice/responseFormatter.test.ts | 288 ++++++++++++++++++ packages/core/src/voice/responseFormatter.ts | 185 +++++++++++ 3 files changed, 476 insertions(+) create mode 100644 packages/core/src/voice/responseFormatter.test.ts create mode 100644 packages/core/src/voice/responseFormatter.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 47af5f76e1..e035dc4502 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -219,5 +219,8 @@ export * from './agents/types.js'; export * from './utils/stdio.js'; export * from './utils/terminal.js'; +// Export voice utilities +export * from './voice/responseFormatter.js'; + // Export types from @google/genai export type { Content, Part, FunctionCall } from '@google/genai'; diff --git a/packages/core/src/voice/responseFormatter.test.ts b/packages/core/src/voice/responseFormatter.test.ts new file mode 100644 index 0000000000..679ff1b89c --- /dev/null +++ b/packages/core/src/voice/responseFormatter.test.ts @@ -0,0 +1,288 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { formatForSpeech } from './responseFormatter.js'; + +describe('formatForSpeech', () => { + describe('edge cases', () => { + it('should return empty string for empty input', () => { + expect(formatForSpeech('')).toBe(''); + }); + + it('should return plain text unchanged', () => { + expect(formatForSpeech('Hello world')).toBe('Hello world'); + }); + }); + + describe('ANSI escape codes', () => { + it('should strip color codes', () => { + expect(formatForSpeech('\x1b[31mError\x1b[0m')).toBe('Error'); + }); + + it('should strip bold/dim codes', () => { + expect(formatForSpeech('\x1b[1mBold\x1b[22m text')).toBe('Bold text'); + }); + + it('should strip cursor movement codes', () => { + expect(formatForSpeech('line1\x1b[2Kline2')).toBe('line1line2'); + }); + }); + + describe('markdown stripping', () => { + it('should strip bold markers **text**', () => { + expect(formatForSpeech('**Error**: something went wrong')).toBe( + 'Error: something went wrong', + ); + }); + + it('should strip bold markers __text__', () => { + expect(formatForSpeech('__Error__: something')).toBe('Error: something'); + }); + + it('should strip italic markers *text*', () => { + expect(formatForSpeech('*note*: pay attention')).toBe( + 'note: pay attention', + ); + }); + + it('should strip inline code backticks', () => { + expect(formatForSpeech('Run `npm install` first')).toBe( + 'Run npm install first', + ); + }); + + it('should strip blockquote prefix', () => { + expect(formatForSpeech('> This is a quote')).toBe('This is a quote'); + }); + + it('should strip heading markers', () => { + expect(formatForSpeech('# Results\n## Details')).toBe('Results\nDetails'); + }); + + it('should replace markdown links with link text', () => { + expect(formatForSpeech('[Gemini API](https://ai.google.dev)')).toBe( + 'Gemini API', + ); + }); + + it('should strip unordered list markers', () => { + expect(formatForSpeech('- item one\n- item two')).toBe( + 'item one\nitem two', + ); + }); + + it('should strip ordered list markers', () => { + expect(formatForSpeech('1. first\n2. second')).toBe('first\nsecond'); + }); + }); + + describe('fenced code blocks', () => { + it('should unwrap a plain code block', () => { + expect(formatForSpeech('```\nconsole.log("hi")\n```')).toBe( + 'console.log("hi")', + ); + }); + + it('should unwrap a language-tagged code block', () => { + expect(formatForSpeech('```typescript\nconst x = 1;\n```')).toBe( + 'const x = 1;', + ); + }); + + it('should summarise a JSON object code block above threshold', () => { + const json = JSON.stringify({ status: 'ok', count: 42, items: [] }); + // Pass jsonThreshold lower than the json string length (38 chars) + const result = formatForSpeech(`\`\`\`json\n${json}\n\`\`\``, { + jsonThreshold: 10, + }); + expect(result).toBe('(JSON object with 3 keys)'); + }); + + it('should summarise a JSON array code block above threshold', () => { + const json = JSON.stringify([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + // Pass jsonThreshold lower than the json string length (23 chars) + const result = formatForSpeech(`\`\`\`\n${json}\n\`\`\``, { + jsonThreshold: 10, + }); + expect(result).toBe('(JSON array with 10 items)'); + }); + + it('should summarise a large JSON object using default threshold', () => { + // Build a JSON object whose stringified form exceeds the default 80-char threshold + const big = { + status: 'success', + count: 42, + items: ['alpha', 'beta', 'gamma'], + meta: { page: 1, totalPages: 10 }, + timestamp: '2026-03-03T00:00:00Z', + }; + const json = JSON.stringify(big); + expect(json.length).toBeGreaterThan(80); + const result = formatForSpeech(`\`\`\`json\n${json}\n\`\`\``); + expect(result).toBe('(JSON object with 5 keys)'); + }); + + it('should not summarise a tiny JSON value', () => { + // Below the default 80-char threshold → keep as-is + const result = formatForSpeech('```json\n{"a":1}\n```', { + jsonThreshold: 80, + }); + expect(result).toBe('{"a":1}'); + }); + }); + + describe('path abbreviation', () => { + it('should abbreviate a deep Unix path (default depth 3)', () => { + const result = formatForSpeech( + 'at /home/user/project/packages/core/src/tools/file.ts', + ); + expect(result).toContain('\u2026/src/tools/file.ts'); + expect(result).not.toContain('/home/user/project'); + }); + + it('should convert :line suffix to "line N"', () => { + const result = formatForSpeech( + 'Error at /home/user/project/src/tools/file.ts:142', + ); + expect(result).toContain('line 142'); + }); + + it('should drop column from :line:col suffix', () => { + const result = formatForSpeech( + 'Error at /home/user/project/src/tools/file.ts:142:7', + ); + expect(result).toContain('line 142'); + expect(result).not.toContain(':7'); + }); + + it('should respect custom pathDepth option', () => { + const result = formatForSpeech( + '/home/user/project/packages/core/src/file.ts', + { pathDepth: 2 }, + ); + expect(result).toContain('\u2026/src/file.ts'); + }); + + it('should not abbreviate a short path within depth', () => { + const result = formatForSpeech('/src/file.ts', { pathDepth: 3 }); + // Only 2 segments — no abbreviation needed + expect(result).toBe('/src/file.ts'); + }); + + it('should abbreviate a Windows path on a non-C drive', () => { + const result = formatForSpeech( + 'D:\\Users\\project\\packages\\core\\src\\file.ts', + { pathDepth: 3 }, + ); + expect(result).toContain('\u2026/core/src/file.ts'); + expect(result).not.toContain('D:\\Users\\project'); + }); + + it('should convert :line on a Windows path on a non-C drive', () => { + const result = formatForSpeech( + 'Error at D:\\Users\\project\\src\\tools\\file.ts:55', + ); + expect(result).toContain('line 55'); + expect(result).not.toContain('D:\\Users\\project'); + }); + + it('should abbreviate a Unix path containing a scoped npm package segment', () => { + const result = formatForSpeech( + 'at /home/user/project/node_modules/@google/gemini-cli-core/src/index.ts:12:3', + { pathDepth: 5 }, + ); + expect(result).toContain('line 12'); + expect(result).not.toContain(':3'); + expect(result).toContain('@google'); + }); + }); + + describe('stack trace collapsing', () => { + it('should collapse a multi-frame stack trace', () => { + const trace = [ + 'Error: ENOENT', + ' at Object.open (/project/src/file.ts:10:5)', + ' at Module._load (/project/node_modules/loader.js:20:3)', + ' at Function.Module._load (/project/node_modules/loader.js:30:3)', + ].join('\n'); + + const result = formatForSpeech(trace); + expect(result).toContain('and 2 more frames'); + expect(result).not.toContain('Module._load'); + }); + + it('should not collapse a single stack frame', () => { + const trace = + 'Error: ENOENT\n at Object.open (/project/src/file.ts:10:5)'; + const result = formatForSpeech(trace); + expect(result).not.toContain('more frames'); + }); + + it('should preserve surrounding text when collapsing a stack trace', () => { + const input = [ + 'Operation failed.', + ' at Object.open (/project/src/file.ts:10:5)', + ' at Module._load (/project/node_modules/loader.js:20:3)', + ' at Function.load (/project/node_modules/loader.js:30:3)', + 'Please try again.', + ].join('\n'); + + const result = formatForSpeech(input); + expect(result).toContain('Operation failed.'); + expect(result).toContain('Please try again.'); + expect(result).toContain('and 2 more frames'); + }); + }); + + describe('truncation', () => { + it('should truncate output longer than maxLength', () => { + const long = 'word '.repeat(200); + const result = formatForSpeech(long, { maxLength: 50 }); + expect(result.length).toBeLessThanOrEqual( + 50 + '\u2026 (1000 chars total)'.length, + ); + expect(result).toContain('\u2026'); + expect(result).toContain('chars total'); + }); + + it('should not truncate output within maxLength', () => { + const short = 'Hello world'; + expect(formatForSpeech(short, { maxLength: 500 })).toBe('Hello world'); + }); + }); + + describe('whitespace normalisation', () => { + it('should collapse more than two consecutive blank lines', () => { + const result = formatForSpeech('para1\n\n\n\n\npara2'); + expect(result).toBe('para1\n\npara2'); + }); + + it('should trim leading and trailing whitespace', () => { + expect(formatForSpeech(' hello ')).toBe('hello'); + }); + }); + + describe('real-world examples', () => { + it('should clean an ENOENT error with markdown and path', () => { + const input = + '**Error**: `ENOENT: no such file or directory`\n> at /home/user/project/packages/core/src/tools/file-utils.ts:142:7'; + const result = formatForSpeech(input); + expect(result).not.toContain('**'); + expect(result).not.toContain('`'); + expect(result).not.toContain('>'); + expect(result).toContain('Error'); + expect(result).toContain('ENOENT'); + expect(result).toContain('line 142'); + }); + + it('should clean a heading + list response', () => { + const input = '# Results\n- item one\n- item two\n- item three'; + const result = formatForSpeech(input); + expect(result).toBe('Results\nitem one\nitem two\nitem three'); + }); + }); +}); diff --git a/packages/core/src/voice/responseFormatter.ts b/packages/core/src/voice/responseFormatter.ts new file mode 100644 index 0000000000..dc1cbac4c4 --- /dev/null +++ b/packages/core/src/voice/responseFormatter.ts @@ -0,0 +1,185 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Options for formatForSpeech(). + */ +export interface FormatForSpeechOptions { + /** + * Maximum output length in characters before truncating. + * @default 500 + */ + maxLength?: number; + /** + * Number of trailing path segments to keep when abbreviating absolute paths. + * @default 3 + */ + pathDepth?: number; + /** + * Maximum number of characters in a JSON value before summarising it. + * @default 80 + */ + jsonThreshold?: number; +} + +// ANSI escape sequences (CSI, OSC, etc.) +// eslint-disable-next-line no-control-regex +const ANSI_RE = /\x1b(?:\[[0-9;]*[mGKHF]|\][^\x07\x1b]*\x07|[()][AB012])/g; + +// Fenced code blocks ```lang\n...\n``` +const CODE_FENCE_RE = /```[^\n]*\n([\s\S]*?)```/g; + +// Inline code `...` +const INLINE_CODE_RE = /`([^`]+)`/g; + +// Bold/italic markers **text**, *text*, __text__, _text_ +// Exclude newlines so the pattern cannot span multiple lines and accidentally +// consume list markers that haven't been stripped yet. +const BOLD_ITALIC_RE = /\*{1,2}([^*\n]+)\*{1,2}|_{1,2}([^_\n]+)_{1,2}/g; + +// Blockquote prefix "> " +const BLOCKQUOTE_RE = /^>\s?/gm; + +// ATX headings # heading +const HEADING_RE = /^#{1,6}\s+/gm; + +// Markdown links [text](url) +const LINK_RE = /\[([^\]]+)\]\([^)]+\)/g; + +// Markdown list markers "- " or "* " or "N. " at line start +const LIST_MARKER_RE = /^[ \t]*(?:[-*]|\d+\.)\s+/gm; + +// Two or more consecutive stack-trace frames (Node.js style " at …" lines). +// Matching blocks of ≥2 lets us replace each group in-place, preserving any +// text that follows the trace rather than appending it to the end. +const STACK_BLOCK_RE = /(?:^[ \t]+at [^\n]+(?:\n|$)){2,}/gm; + +// Absolute Unix paths optionally ending with :line or :line:col +// Hyphen placed at start of char class to avoid useless-escape lint error +const UNIX_PATH_RE = + /(?:^|(?<=\s|[(`"']))(\/[-\w.@]+(?:\/[-\w.@]+)*)(:\d+(?::\d+)?)?/g; + +// Absolute Windows paths C:\... or C:/... (any drive letter) +const WIN_PATH_RE = + /(?:^|(?<=\s|[(`"']))([A-Za-z]:[/\\][-\w. ]+(?:[/\\][-\w. ]+)*)(:\d+(?::\d+)?)?/g; + +/** + * Abbreviates an absolute path to at most `depth` trailing segments, + * prefixed with "…". Optionally converts `:line` suffix to `line N`. + */ +function abbreviatePath( + full: string, + suffix: string | undefined, + depth: number, +): string { + const segments = full.split(/[/\\]/).filter(Boolean); + const kept = segments.length > depth ? segments.slice(-depth) : segments; + const abbreviated = + segments.length > depth ? `\u2026/${kept.join('/')}` : full; + + if (!suffix) return abbreviated; + // Convert ":142" → " line 142", ":142:7" → " line 142" + const lineNum = suffix.split(':').filter(Boolean)[0]; + return `${abbreviated} line ${lineNum}`; +} + +/** + * Summarises a JSON string as "(JSON object with N keys)" or + * "(JSON array with N items)", falling back to the original if parsing fails. + */ +function summariseJson(jsonStr: string): string { + try { + const parsed: unknown = JSON.parse(jsonStr); + if (Array.isArray(parsed)) { + return `(JSON array with ${parsed.length} item${parsed.length === 1 ? '' : 's'})`; + } + if (parsed !== null && typeof parsed === 'object') { + const keys = Object.keys(parsed).length; + return `(JSON object with ${keys} key${keys === 1 ? '' : 's'})`; + } + } catch { + // not valid JSON — leave as-is + } + return jsonStr; +} + +/** + * Transforms a markdown/ANSI-formatted string into speech-ready plain text. + * + * Transformations applied (in order): + * 1. Strip ANSI escape codes + * 2. Collapse fenced code blocks to their content (or a JSON summary) + * 3. Collapse stack traces to first frame + count + * 4. Strip markdown syntax (bold, italic, blockquotes, headings, links, lists, inline code) + * 5. Abbreviate deep absolute paths + * 6. Normalise whitespace + * 7. Truncate to maxLength + */ +export function formatForSpeech( + text: string, + options?: FormatForSpeechOptions, +): string { + const maxLength = options?.maxLength ?? 500; + const pathDepth = options?.pathDepth ?? 3; + const jsonThreshold = options?.jsonThreshold ?? 80; + + if (!text) return ''; + + let out = text; + + // 1. Strip ANSI escape codes + out = out.replace(ANSI_RE, ''); + + // 2. Fenced code blocks — try to summarise JSON content, else keep text + out = out.replace(CODE_FENCE_RE, (_match, body: string) => { + const trimmed = body.trim(); + if (trimmed.length > jsonThreshold) { + const summary = summariseJson(trimmed); + if (summary !== trimmed) return summary; + } + return trimmed; + }); + + // 3. Collapse stack traces: replace each contiguous block of ≥2 frames + // in-place so that any text after the trace is preserved in order. + out = out.replace(STACK_BLOCK_RE, (block) => { + const lines = block + .trim() + .split('\n') + .map((l) => l.trim()); + const rest = lines.length - 1; + return `${lines[0]} (and ${rest} more frame${rest === 1 ? '' : 's'})\n`; + }); + + // 4. Strip markdown syntax + out = out + .replace(INLINE_CODE_RE, '$1') + .replace(BOLD_ITALIC_RE, (_m, g1?: string, g2?: string) => g1 ?? g2 ?? '') + .replace(BLOCKQUOTE_RE, '') + .replace(HEADING_RE, '') + .replace(LINK_RE, '$1') + .replace(LIST_MARKER_RE, ''); + + // 5. Abbreviate absolute paths + // Windows paths first to avoid the leading letter being caught by Unix RE + out = out.replace(WIN_PATH_RE, (_m, full: string, suffix?: string) => + abbreviatePath(full, suffix, pathDepth), + ); + out = out.replace(UNIX_PATH_RE, (_m, full: string, suffix?: string) => + abbreviatePath(full, suffix, pathDepth), + ); + + // 6. Normalise whitespace: collapse multiple blank lines, trim + out = out.replace(/\n{3,}/g, '\n\n').trim(); + + // 7. Truncate + if (out.length > maxLength) { + const total = out.length; + out = out.slice(0, maxLength).trimEnd() + `\u2026 (${total} chars total)`; + } + + return out; +} From 5caa192cfc0700b70c1cb3537f603a8ba04b732a Mon Sep 17 00:00:00 2001 From: Aditya Bijalwan Date: Wed, 11 Mar 2026 01:45:03 +0530 Subject: [PATCH 12/15] feat: add pulsating blue border automation overlay to browser agent (#21173) Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Gaurav <39389231+gsquared94@users.noreply.github.com> --- .../src/agents/browser/automationOverlay.ts | 133 ++++++++++++++++++ .../browser/browserAgentFactory.test.ts | 29 ++++ .../src/agents/browser/browserAgentFactory.ts | 10 ++ .../src/agents/browser/browserManager.test.ts | 83 +++++++++++ .../core/src/agents/browser/browserManager.ts | 90 +++++++++--- .../core/src/agents/browser/mcpToolWrapper.ts | 6 +- .../mcpToolWrapperConfirmation.test.ts | 2 + 7 files changed, 331 insertions(+), 22 deletions(-) create mode 100644 packages/core/src/agents/browser/automationOverlay.ts diff --git a/packages/core/src/agents/browser/automationOverlay.ts b/packages/core/src/agents/browser/automationOverlay.ts new file mode 100644 index 0000000000..a1aa40d58b --- /dev/null +++ b/packages/core/src/agents/browser/automationOverlay.ts @@ -0,0 +1,133 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @fileoverview Automation overlay utilities for visual indication during browser automation. + * + * Provides functions to inject and remove a pulsating blue border overlay + * that indicates when the browser is under AI agent control. + * + * Uses the Web Animations API instead of injected