Compare commits

..

6 Commits

Author SHA1 Message Date
jacob314 d78db4f514 fix: address PR feedback for slash commands and background task tracking 2026-05-18 14:27:13 -07:00
jacob314 3e1be515e5 fix: address PR feedback for slash commands and test cleanup 2026-05-18 11:54:08 -07:00
jacob314 469f5b7a68 fix: address PR feedback and background task lifecycle bugs
Addresses high-priority feedback from the automated code review tool regarding the logic error and inconsistency in `useAgentStream.ts` and `useExecutionLifecycle.ts`.

- `useAgentStream.ts`: Fixed `schedule_tool` fall-through by returning early if the command type is not `submit_prompt`. Prevented history duplication by only adding items not already handled by the slash command processor. Ensured the original raw string is logged.
- `useExecutionLifecycle.ts`: Removed `isActive` guards from the unmount cleanup effect and the `ExecutionLifecycleService.onBackground` listener to prevent background tasks from being incorrectly unsubscribed or missed when switching between Gemini and Agent streams.
- Updated `useAgentStream.test.tsx` to reflect the fixed history logic.
2026-05-18 09:48:40 -07:00
jacob314 b2946b1052 Checkpoint in support /tool commands. 2026-05-18 08:48:53 -07:00
Adam Weidman 84423e6ea1 fix(core): enforce compile-time exhaustiveness in content-utils (#27207) 2026-05-18 15:16:49 +00:00
Adam Weidman 5611ff40e7 feat(core): add adk.agentSessionSubagentEnabled flag (#26947) 2026-05-17 17:38:34 +00:00
12 changed files with 206 additions and 47 deletions
+6
View File
@@ -1854,6 +1854,12 @@ their corresponding top-level category object in your `settings.json` file.
- **Default:** `false`
- **Requires restart:** Yes
- **`experimental.adk.agentSessionSubagentEnabled`** (boolean):
- **Description:** Route subagent invocations through the AgentSession
protocol instead of legacy executors.
- **Default:** `false`
- **Requires restart:** Yes
- **`experimental.enableAgents`** (boolean):
- **Description:** Enable local and remote subagents.
- **Default:** `true`
@@ -571,6 +571,18 @@ describe('SettingsSchema', () => {
expect(agentSessionNoninteractiveEnabled.description).toBe(
'Enable non-interactive agent sessions.',
);
const agentSessionSubagentEnabled =
adk.properties.agentSessionSubagentEnabled;
expect(agentSessionSubagentEnabled).toBeDefined();
expect(agentSessionSubagentEnabled.type).toBe('boolean');
expect(agentSessionSubagentEnabled.category).toBe('Experimental');
expect(agentSessionSubagentEnabled.default).toBe(false);
expect(agentSessionSubagentEnabled.requiresRestart).toBe(true);
expect(agentSessionSubagentEnabled.showInDialog).toBe(false);
expect(agentSessionSubagentEnabled.description).toBe(
'Route subagent invocations through the AgentSession protocol instead of legacy executors.',
);
});
});
+10
View File
@@ -2194,6 +2194,16 @@ const SETTINGS_SCHEMA = {
'Enable the agent session implementation for the interactive CLI.',
showInDialog: false,
},
agentSessionSubagentEnabled: {
type: 'boolean',
label: 'Agent Session Subagent Enabled',
category: 'Experimental',
requiresRestart: true,
default: false,
description:
'Route subagent invocations through the AgentSession protocol instead of legacy executors.',
showInDialog: false,
},
},
},
enableAgents: {
+1
View File
@@ -1185,6 +1185,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
useAgentStream({
agent: streamAgent,
addItem: historyManager.addItem,
handleSlashCommand,
onCancelSubmit,
isShellFocused: embeddedShellFocused,
logger,
@@ -4,14 +4,14 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { act } from 'react';
import type { LegacyAgentProtocol } from '@google/gemini-cli-core';
import type { AgentProtocol } from '@google/gemini-cli-core';
import { renderHookWithProviders } from '../../test-utils/render.js';
// --- MOCKS ---
const mockLegacyAgentProtocol = vi.hoisted(() => ({
const mockAgentProtocol = vi.hoisted(() => ({
send: vi.fn().mockResolvedValue({ streamId: 'test-stream-id' }),
subscribe: vi.fn().mockReturnValue(() => {}),
abort: vi.fn().mockResolvedValue(undefined),
@@ -40,24 +40,31 @@ describe('useAgentStream', () => {
vi.clearAllMocks();
});
afterEach(() => {
vi.restoreAllMocks();
});
it('should initialize on mount', async () => {
await renderHookWithProviders(() =>
useAgentStream({
agent: mockLegacyAgentProtocol as unknown as LegacyAgentProtocol,
agent: mockAgentProtocol as unknown as AgentProtocol,
addItem: mockAddItem,
handleSlashCommand: vi.fn().mockResolvedValue(false),
onCancelSubmit: mockOnCancelSubmit,
isShellFocused: false,
}),
);
expect(mockLegacyAgentProtocol.subscribe).toHaveBeenCalled();
expect(mockAgentProtocol.subscribe).toHaveBeenCalled();
});
it('should call agent.send when submitQuery is called', async () => {
const mockHandleSlashCommand = vi.fn().mockResolvedValue(false);
const { result } = await renderHookWithProviders(() =>
useAgentStream({
agent: mockLegacyAgentProtocol as unknown as LegacyAgentProtocol,
agent: mockAgentProtocol as unknown as AgentProtocol,
addItem: mockAddItem,
handleSlashCommand: mockHandleSlashCommand,
onCancelSubmit: mockOnCancelSubmit,
isShellFocused: false,
}),
@@ -67,7 +74,7 @@ describe('useAgentStream', () => {
await result.current.submitQuery('hello');
});
expect(mockLegacyAgentProtocol.send).toHaveBeenCalledWith({
expect(mockAgentProtocol.send).toHaveBeenCalledWith({
message: { content: [{ type: 'text', text: 'hello' }] },
});
expect(mockAddItem).toHaveBeenCalledWith(
@@ -76,17 +83,70 @@ describe('useAgentStream', () => {
);
});
it('should update streamingState based on agent_start and agent_end events', async () => {
it('should intercept slash commands and not call agent.send if handled', async () => {
const mockHandleSlashCommand = vi
.fn()
.mockResolvedValue({ type: 'handled' });
const { result } = await renderHookWithProviders(() =>
useAgentStream({
agent: mockLegacyAgentProtocol as unknown as LegacyAgentProtocol,
agent: mockAgentProtocol as unknown as AgentProtocol,
addItem: mockAddItem,
handleSlashCommand: mockHandleSlashCommand,
onCancelSubmit: mockOnCancelSubmit,
isShellFocused: false,
}),
);
const eventHandler = vi.mocked(mockLegacyAgentProtocol.subscribe).mock
await act(async () => {
await result.current.submitQuery('/about');
});
expect(mockHandleSlashCommand).toHaveBeenCalledWith('/about');
expect(mockAgentProtocol.send).not.toHaveBeenCalled();
});
it('should intercept slash commands and call agent.send with new content if submit_prompt', async () => {
const mockHandleSlashCommand = vi.fn().mockResolvedValue({
type: 'submit_prompt',
content: 'modified prompt',
});
const { result } = await renderHookWithProviders(() =>
useAgentStream({
agent: mockAgentProtocol as unknown as AgentProtocol,
addItem: mockAddItem,
handleSlashCommand: mockHandleSlashCommand,
onCancelSubmit: mockOnCancelSubmit,
isShellFocused: false,
}),
);
await act(async () => {
await result.current.submitQuery('/mcp-prompt');
});
expect(mockHandleSlashCommand).toHaveBeenCalledWith('/mcp-prompt');
expect(mockAgentProtocol.send).toHaveBeenCalledWith({
message: { content: [{ type: 'text', text: 'modified prompt' }] },
});
expect(mockAddItem).toHaveBeenCalledWith(
expect.objectContaining({ type: MessageType.USER, text: '/mcp-prompt' }),
expect.any(Number),
);
});
it('should update streamingState based on agent_start and agent_end events', async () => {
const mockHandleSlashCommand = vi.fn().mockResolvedValue(false);
const { result } = await renderHookWithProviders(() =>
useAgentStream({
agent: mockAgentProtocol as unknown as AgentProtocol,
addItem: mockAddItem,
handleSlashCommand: mockHandleSlashCommand,
onCancelSubmit: mockOnCancelSubmit,
isShellFocused: false,
}),
);
const eventHandler = vi.mocked(mockAgentProtocol.subscribe).mock
.calls[0][0];
expect(result.current.streamingState).toBe(StreamingState.Idle);
@@ -116,14 +176,15 @@ describe('useAgentStream', () => {
it('should accumulate text content and update pendingHistoryItems', async () => {
const { result } = await renderHookWithProviders(() =>
useAgentStream({
agent: mockLegacyAgentProtocol as unknown as LegacyAgentProtocol,
agent: mockAgentProtocol as unknown as AgentProtocol,
addItem: mockAddItem,
handleSlashCommand: vi.fn().mockResolvedValue(false),
onCancelSubmit: mockOnCancelSubmit,
isShellFocused: false,
}),
);
const eventHandler = vi.mocked(mockLegacyAgentProtocol.subscribe).mock
const eventHandler = vi.mocked(mockAgentProtocol.subscribe).mock
.calls[0][0];
act(() => {
@@ -160,14 +221,15 @@ describe('useAgentStream', () => {
it('should process thought events and update thought state', async () => {
const { result } = await renderHookWithProviders(() =>
useAgentStream({
agent: mockLegacyAgentProtocol as unknown as LegacyAgentProtocol,
agent: mockAgentProtocol as unknown as AgentProtocol,
addItem: mockAddItem,
handleSlashCommand: vi.fn().mockResolvedValue(false),
onCancelSubmit: mockOnCancelSubmit,
isShellFocused: false,
}),
);
const eventHandler = vi.mocked(mockLegacyAgentProtocol.subscribe).mock
const eventHandler = vi.mocked(mockAgentProtocol.subscribe).mock
.calls[0][0];
act(() => {
@@ -190,8 +252,9 @@ describe('useAgentStream', () => {
it('should call agent.abort when cancelOngoingRequest is called', async () => {
const { result } = await renderHookWithProviders(() =>
useAgentStream({
agent: mockLegacyAgentProtocol as unknown as LegacyAgentProtocol,
agent: mockAgentProtocol as unknown as AgentProtocol,
addItem: mockAddItem,
handleSlashCommand: vi.fn().mockResolvedValue(false),
onCancelSubmit: mockOnCancelSubmit,
isShellFocused: false,
}),
@@ -201,7 +264,7 @@ describe('useAgentStream', () => {
await result.current.cancelOngoingRequest();
});
expect(mockLegacyAgentProtocol.abort).toHaveBeenCalled();
expect(mockAgentProtocol.abort).toHaveBeenCalled();
expect(mockOnCancelSubmit).toHaveBeenCalledWith(false, true);
});
});
+60 -10
View File
@@ -5,12 +5,14 @@
*/
import { useState, useRef, useCallback, useEffect, useMemo } from 'react';
import { type PartListUnion } from '@google/genai';
import {
getErrorMessage,
MessageSenderType,
debugLogger,
geminiPartsToContentParts,
displayContentToString,
partToString,
parseThought,
CoreToolCallStatus,
type ApprovalMode,
@@ -20,15 +22,16 @@ import {
type AgentEvent,
type AgentProtocol,
type Logger,
type Part,
} from '@google/gemini-cli-core';
import type {
HistoryItemWithoutId,
LoopDetectionConfirmationRequest,
IndividualToolCallDisplay,
HistoryItemToolDisplayGroup,
SlashCommandProcessorResult,
} from '../types.js';
import { StreamingState, MessageType } from '../types.js';
import { isSlashCommand } from '../utils/commandUtils.js';
import { findLastSafeSplitPoint } from '../utils/markdownUtilities.js';
import { getToolGroupBorderAppearance } from '../utils/borderStyles.js';
import { type BackgroundTask } from './useExecutionLifecycle.js';
@@ -41,6 +44,9 @@ import { useKeypress } from './useKeypress.js';
export interface UseAgentStreamOptions {
agent?: AgentProtocol;
addItem: UseHistoryManagerReturn['addItem'];
handleSlashCommand: (
cmd: string,
) => Promise<SlashCommandProcessorResult | false>;
onCancelSubmit: (
shouldRestorePrompt?: boolean,
clearBuffer?: boolean,
@@ -56,6 +62,7 @@ export interface UseAgentStreamOptions {
export const useAgentStream = ({
agent,
addItem,
handleSlashCommand,
onCancelSubmit,
isShellFocused,
logger,
@@ -333,8 +340,8 @@ export const useAgentStream = ({
useKeypress(
(key) => {
if (key.name === 'escape' && !isShellFocused) {
void cancelOngoingRequest(false);
if (key.ctrl && key.name === 'c') {
void cancelOngoingRequest();
return true;
}
return false;
@@ -348,7 +355,7 @@ export const useAgentStream = ({
const submitQuery = useCallback(
async (
query: Part[] | string,
query: PartListUnion,
options?: { isContinuation: boolean },
_prompt_id?: string,
) => {
@@ -360,16 +367,59 @@ export const useAgentStream = ({
geminiMessageBufferRef.current = '';
let localQuery: PartListUnion = query;
if (!options?.isContinuation) {
if (typeof query === 'string') {
addItem({ type: MessageType.USER, text: query }, timestamp);
void logger?.logMessage(MessageSenderType.USER, query);
let shouldAddToHistory = true;
if (typeof localQuery === 'string') {
const trimmedQuery = localQuery.trim();
void logger?.logMessage(MessageSenderType.USER, trimmedQuery);
if (isSlashCommand(trimmedQuery)) {
const slashResult = await handleSlashCommand(trimmedQuery);
if (slashResult) {
if (slashResult.type === 'submit_prompt') {
localQuery = slashResult.content;
} else if (slashResult.type === 'schedule_tool') {
addItem(
{
type: MessageType.ERROR,
text: `The /${slashResult.toolName} command is not yet supported in Agent mode.`,
},
timestamp,
);
return;
} else {
// 'handled' or other types that don't need LLM submission
shouldAddToHistory = false;
return;
}
}
}
}
if (shouldAddToHistory) {
const originalQueryText =
typeof query === 'string' ? query : partToString(query);
addItem(
{ type: MessageType.USER, text: originalQueryText },
timestamp,
);
if (typeof localQuery !== 'string') {
void logger?.logMessage(
MessageSenderType.USER,
partToString(localQuery),
);
}
}
startNewPrompt();
}
const parts = geminiPartsToContentParts(
typeof query === 'string' ? [{ text: query }] : query,
(Array.isArray(localQuery) ? localQuery : [localQuery]).map((p) =>
typeof p === 'string' ? { text: p } : p,
),
);
try {
@@ -384,9 +434,8 @@ export const useAgentStream = ({
);
}
},
[agent, addItem, logger, startNewPrompt],
[agent, addItem, logger, startNewPrompt, handleSlashCommand],
);
useEffect(() => {
if (trackedTools.length > 0) {
const isNewBatch = !trackedTools.some((tc) =>
@@ -397,6 +446,7 @@ export const useAgentStream = ({
setIsFirstToolInGroup(true);
}
} else if (streamingState === StreamingState.Idle) {
// Clear when idle to be ready for next turn
setPushedToolCallIds(new Set());
setIsFirstToolInGroup(true);
}
+19 -5
View File
@@ -387,6 +387,11 @@ export const useGeminiStream = (
[setIsResponding],
);
const streamingState = useMemo(
() => calculateStreamingState(isResponding, toolCalls),
[isResponding, toolCalls],
);
const {
handleShellCommand,
activeShellPtyId,
@@ -409,11 +414,7 @@ export const useGeminiStream = (
terminalWidth,
terminalHeight,
activeBackgroundExecutionId,
);
const streamingState = useMemo(
() => calculateStreamingState(isResponding, toolCalls),
[isResponding, toolCalls],
streamingState === StreamingState.WaitingForConfirmation,
);
// Reset tracking when a new batch of tools starts
@@ -599,6 +600,7 @@ export const useGeminiStream = (
backgroundTasks,
settings.merged.ui?.compactToolOutput,
]);
const pendingToolGroupItems = useMemo((): HistoryItemWithoutId[] => {
const remainingTools = toolCalls.filter(
(tc) => !pushedToolCallIds.has(tc.request.callId),
@@ -983,16 +985,28 @@ export const useGeminiStream = (
if (postSubmitPrompt) {
localQueryToSendToGemini = postSubmitPrompt;
addItem(
{ type: MessageType.USER, text: trimmedQuery },
userMessageTimestamp,
);
return {
queryToSend: localQueryToSendToGemini,
shouldProceed: true,
};
}
addItem(
{ type: MessageType.USER, text: trimmedQuery },
userMessageTimestamp,
);
return { queryToSend: null, shouldProceed: false };
}
case 'submit_prompt': {
localQueryToSendToGemini = slashCommandResult.content;
addItem(
{ type: MessageType.USER, text: trimmedQuery },
userMessageTimestamp,
);
return {
queryToSend: localQueryToSendToGemini,
+4 -1
View File
@@ -93,13 +93,16 @@ export function contentPartsToGeminiParts(content: ContentPart[]): Part[] {
// References are converted to text for the model
result.push({ text: part.text });
break;
default:
default: {
const _exhaustiveCheck: never = part;
void _exhaustiveCheck;
debugLogger.warn(
`Unhandled ContentPart type: ${JSON.stringify(part)} fallback to serialization`,
);
// Serialize unknown ContentPart variants instead of dropping them
result.push({ text: JSON.stringify(part) });
break;
}
}
}
return result;
+8
View File
@@ -237,6 +237,7 @@ export interface GemmaModelRouterSettings {
export interface ADKSettings {
agentSessionNoninteractiveEnabled?: boolean;
agentSessionInteractiveEnabled?: boolean;
agentSessionSubagentEnabled?: boolean;
}
export interface ExtensionSetting {
@@ -913,6 +914,7 @@ export class Config implements McpContext, AgentLoopContext {
private readonly gemmaModelRouter: GemmaModelRouterSettings;
private readonly agentSessionNoninteractiveEnabled: boolean;
private readonly agentSessionInteractiveEnabled: boolean;
private readonly agentSessionSubagentEnabled: boolean;
private readonly retryFetchErrors: boolean;
private readonly maxAttempts: number;
@@ -1359,6 +1361,8 @@ export class Config implements McpContext, AgentLoopContext {
params.adk?.agentSessionNoninteractiveEnabled ?? false;
this.agentSessionInteractiveEnabled =
params.adk?.agentSessionInteractiveEnabled ?? false;
this.agentSessionSubagentEnabled =
params.adk?.agentSessionSubagentEnabled ?? false;
this.retryFetchErrors = params.retryFetchErrors ?? true;
this.maxAttempts = Math.min(
params.maxAttempts ?? DEFAULT_MAX_ATTEMPTS,
@@ -2573,6 +2577,10 @@ export class Config implements McpContext, AgentLoopContext {
return this.contextManagement.enabled;
}
isAgentSessionSubagentEnabled(): boolean {
return this.agentSessionSubagentEnabled;
}
getMemoryBoundaryMarkers(): readonly string[] {
return this.memoryBoundaryMarkers;
}
-9
View File
@@ -439,15 +439,6 @@ describe('resolveModel', () => {
expect(model).toBe(customModel);
});
it('should fallback for known legacy models like gemini-pro-latest', () => {
expect(resolveModel('gemini-pro-latest')).toBe(DEFAULT_GEMINI_MODEL);
});
it('should NOT fallback for unknown gemini models that might be future versions', () => {
const futureModel = 'gemini-4-pro';
expect(resolveModel(futureModel)).toBe(futureModel);
});
it('should handle non-string inputs gracefully', () => {
// @ts-expect-error - testing invalid runtime input
expect(resolveModel(['a', 'b'])).toBe('b');
-6
View File
@@ -201,12 +201,6 @@ export function resolveModel(
}
}
// Fallback for known legacy Gemini model aliases that are no longer supported
// by the API but may still be present in user settings.
if (resolved === 'gemini-pro-latest') {
return DEFAULT_GEMINI_MODEL;
}
if (!hasAccessToPreview && isPreviewModel(resolved)) {
// Downgrade to stable models if user lacks preview access.
switch (resolved) {
+7
View File
@@ -3223,6 +3223,13 @@
"markdownDescription": "Enable the agent session implementation for the interactive CLI.\n\n- Category: `Experimental`\n- Requires restart: `yes`\n- Default: `false`",
"default": false,
"type": "boolean"
},
"agentSessionSubagentEnabled": {
"title": "Agent Session Subagent Enabled",
"description": "Route subagent invocations through the AgentSession protocol instead of legacy executors.",
"markdownDescription": "Route subagent invocations through the AgentSession protocol instead of legacy executors.\n\n- Category: `Experimental`\n- Requires restart: `yes`\n- Default: `false`",
"default": false,
"type": "boolean"
}
},
"additionalProperties": false