fix: trim PR and move interactive flag to ADK settings

- Reverted unrelated changes to nonInteractiveCli.test.ts and snapshots.
- Reverted unrelated history truncation config additions.
- Renamed useAgentProtocol to interactiveAgentSessionEnabled and moved it to the ADKSettings group as requested.
This commit is contained in:
Michael Bleigh
2026-04-03 13:44:36 -07:00
parent 9b40e96f80
commit 72ec782fe0
7 changed files with 31 additions and 366 deletions
+22 -246
View File
@@ -77,8 +77,6 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
uiTelemetryService: {
getMetrics: vi.fn(),
},
LegacyAgentSession: original.LegacyAgentSession,
geminiPartsToContentParts: original.geminiPartsToContentParts,
coreEvents: mockCoreEvents,
createWorkingStdio: vi.fn(() => ({
stdout: process.stdout,
@@ -110,8 +108,6 @@ describe('runNonInteractive', () => {
sendMessageStream: Mock;
resumeChat: Mock;
getChatRecordingService: Mock;
getChat: Mock;
getCurrentSequenceModel: Mock;
};
const MOCK_SESSION_METRICS: SessionMetrics = {
models: {},
@@ -167,8 +163,6 @@ describe('runNonInteractive', () => {
recordMessageTokens: vi.fn(),
recordToolCalls: vi.fn(),
})),
getChat: vi.fn(() => ({ recordCompletedToolCalls: vi.fn() })),
getCurrentSequenceModel: vi.fn().mockReturnValue(null),
};
mockConfig = {
@@ -275,54 +269,6 @@ describe('runNonInteractive', () => {
// so we no longer expect shutdownTelemetry to be called directly here
});
it('should stream the specific stream started by send', async () => {
const { LegacyAgentSession } = await import('@google/gemini-cli-core');
const streamSpy = vi.spyOn(LegacyAgentSession.prototype, 'stream');
const events: ServerGeminiStreamEvent[] = [
{ type: GeminiEventType.Content, value: 'Hello again' },
{
type: GeminiEventType.Finished,
value: { reason: undefined, usageMetadata: { totalTokenCount: 10 } },
},
];
mockGeminiClient.sendMessageStream.mockReturnValue(
createStreamFromEvents(events),
);
await runNonInteractive({
config: mockConfig,
settings: mockSettings,
input: 'Test input',
prompt_id: 'prompt-id-stream',
});
expect(streamSpy).toHaveBeenCalledWith({ streamId: expect.any(String) });
});
it('fails fast if the session acknowledges a message send without a stream', async () => {
const { LegacyAgentSession } = await import('@google/gemini-cli-core');
const sendSpy = vi
.spyOn(LegacyAgentSession.prototype, 'send')
.mockResolvedValue({ streamId: null });
const streamSpy = vi.spyOn(LegacyAgentSession.prototype, 'stream');
await expect(
runNonInteractive({
config: mockConfig,
settings: mockSettings,
input: 'Test input',
prompt_id: 'prompt-id-null-stream',
}),
).rejects.toThrow(
'LegacyAgentSession.send() unexpectedly returned no stream for a message send.',
);
expect(streamSpy).not.toHaveBeenCalled();
sendSpy.mockRestore();
streamSpy.mockRestore();
});
it('should register activity logger when GEMINI_CLI_ACTIVITY_LOG_TARGET is set', async () => {
vi.stubEnv('GEMINI_CLI_ACTIVITY_LOG_TARGET', '/tmp/test.jsonl');
const events: ServerGeminiStreamEvent[] = [
@@ -613,7 +559,7 @@ describe('runNonInteractive', () => {
input: 'Initial fail',
prompt_id: 'prompt-id-4',
}),
).rejects.toThrow('API connection failed');
).rejects.toThrow(apiError);
});
it('should not exit if a tool is not found, and should send error back to model', async () => {
@@ -877,79 +823,6 @@ describe('runNonInteractive', () => {
);
});
it('should keep only the final post-tool assistant text in JSON output', async () => {
const toolCallEvent: ServerGeminiStreamEvent = {
type: GeminiEventType.ToolCallRequest,
value: {
callId: 'tool-1',
name: 'testTool',
args: { arg1: 'value1' },
isClientInitiated: false,
prompt_id: 'prompt-id-json-tool-text',
},
};
mockSchedulerSchedule.mockResolvedValue([
{
status: CoreToolCallStatus.Success,
request: toolCallEvent.value,
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
responseParts: [{ text: 'Tool executed successfully' }],
callId: 'tool-1',
error: undefined,
errorType: undefined,
contentLength: undefined,
},
},
]);
mockGeminiClient.sendMessageStream
.mockReturnValueOnce(
createStreamFromEvents([
{ type: GeminiEventType.Content, value: 'Let me check that...' },
toolCallEvent,
{
type: GeminiEventType.Finished,
value: { reason: undefined, usageMetadata: { totalTokenCount: 5 } },
},
]),
)
.mockReturnValueOnce(
createStreamFromEvents([
{ type: GeminiEventType.Content, value: 'Final answer' },
{
type: GeminiEventType.Finished,
value: { reason: undefined, usageMetadata: { totalTokenCount: 3 } },
},
]),
);
vi.mocked(mockConfig.getOutputFormat).mockReturnValue(OutputFormat.JSON);
vi.mocked(uiTelemetryService.getMetrics).mockReturnValue(
MOCK_SESSION_METRICS,
);
await runNonInteractive({
config: mockConfig,
settings: mockSettings,
input: 'Use a tool',
prompt_id: 'prompt-id-json-tool-text',
});
expect(processStdoutSpy).toHaveBeenCalledWith(
JSON.stringify(
{
session_id: 'test-session-id',
response: 'Final answer',
stats: MOCK_SESSION_METRICS,
},
null,
2,
),
);
});
it('should write JSON output with stats for empty response commands', async () => {
// Test the scenario where a command completes but produces no content at all
const events: ServerGeminiStreamEvent[] = [
@@ -1196,11 +1069,10 @@ describe('runNonInteractive', () => {
// Spy on handleCancellationError to verify it's called
const errors = await import('./utils/errors.js');
const cancellationSentinel = new Error('Cancelled');
const handleCancellationErrorSpy = vi
.spyOn(errors, 'handleCancellationError')
.mockImplementation(() => {
throw cancellationSentinel;
throw new Error('Cancelled');
});
const events: ServerGeminiStreamEvent[] = [
@@ -1216,7 +1088,7 @@ describe('runNonInteractive', () => {
signal.addEventListener('abort', () => {
clearTimeout(timeout);
setTimeout(() => {
reject(new Error('Aborted'));
reject(new Error('Aborted')); // This will be caught by nonInteractiveCli and passed to handleError
}, 300);
});
});
@@ -1249,10 +1121,20 @@ describe('runNonInteractive', () => {
keypressHandler('\u0003', { ctrl: true, name: 'c' });
}
// The Ctrl+C path should route through handleCancellationError rather than
// surfacing the raw stream abort.
await expect(runPromise).rejects.toBe(cancellationSentinel);
expect(handleCancellationErrorSpy).toHaveBeenCalledTimes(1);
// The promise should reject with 'Aborted' because our mock stream throws it,
// and nonInteractiveCli catches it and calls handleError, which doesn't necessarily throw.
// Wait, if handleError is called, we should check that.
// But here we want to check if Ctrl+C works.
// In our current setup, Ctrl+C aborts the signal. The stream throws 'Aborted'.
// nonInteractiveCli catches 'Aborted' and calls handleError.
// If we want to test that handleCancellationError is called, we need the loop to detect abortion.
// But our stream throws before the loop can detect it.
// Let's just check that the promise rejects with 'Aborted' for now,
// which proves the abortion signal reached the stream.
await expect(runPromise).rejects.toThrow('Aborted');
expect(
processStderrSpy.mock.calls.some(
@@ -1279,78 +1161,6 @@ describe('runNonInteractive', () => {
// but we can also do it manually if needed.
});
it('should honor cancellation that happens before session.send()', async () => {
const originalIsTTY = process.stdin.isTTY;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const originalSetRawMode = (process.stdin as any).setRawMode;
Object.defineProperty(process.stdin, 'isTTY', {
value: true,
configurable: true,
});
if (!originalSetRawMode) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(process.stdin as any).setRawMode = vi.fn();
}
const stdinOnSpy = vi
.spyOn(process.stdin, 'on')
.mockImplementation(
(event: string | symbol, listener: (...args: unknown[]) => void) => {
if (event === 'keypress') {
listener('\u0003', { ctrl: true, name: 'c' });
}
return process.stdin;
},
);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
vi.spyOn(process.stdin as any, 'setRawMode').mockImplementation(() => true);
vi.spyOn(process.stdin, 'resume').mockImplementation(() => process.stdin);
vi.spyOn(process.stdin, 'pause').mockImplementation(() => process.stdin);
vi.spyOn(process.stdin, 'removeAllListeners').mockImplementation(
() => process.stdin,
);
const errors = await import('./utils/errors.js');
const cancellationSentinel = new Error('Cancelled before send');
const handleCancellationErrorSpy = vi
.spyOn(errors, 'handleCancellationError')
.mockImplementation(() => {
throw cancellationSentinel;
});
const { LegacyAgentSession } = await import('@google/gemini-cli-core');
const sendSpy = vi.spyOn(LegacyAgentSession.prototype, 'send');
await expect(
runNonInteractive({
config: mockConfig,
settings: mockSettings,
input: 'Cancelled query',
prompt_id: 'prompt-id-pre-send-cancel',
}),
).rejects.toBe(cancellationSentinel);
expect(handleCancellationErrorSpy).toHaveBeenCalledTimes(1);
expect(sendSpy).not.toHaveBeenCalled();
expect(stdinOnSpy).toHaveBeenCalled();
handleCancellationErrorSpy.mockRestore();
sendSpy.mockRestore();
Object.defineProperty(process.stdin, 'isTTY', {
value: originalIsTTY,
configurable: true,
});
if (originalSetRawMode) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(process.stdin as any).setRawMode = originalSetRawMode;
} else {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
delete (process.stdin as any).setRawMode;
}
});
it('should throw FatalInputError if a command requires confirmation', async () => {
const mockCommand = {
name: 'confirm',
@@ -1520,9 +1330,6 @@ describe('runNonInteractive', () => {
name: 'ShellTool',
description: 'A shell tool',
run: vi.fn(),
build: vi.fn().mockReturnValue({
getDescription: () => 'A shell tool',
}),
}),
getFunctionDeclarations: vi.fn().mockReturnValue([{ name: 'ShellTool' }]),
} as unknown as ToolRegistry);
@@ -1971,7 +1778,9 @@ describe('runNonInteractive', () => {
throw new Error('Recording failed');
}),
};
// @ts-expect-error - Mocking internal structure
mockGeminiClient.getChat = vi.fn().mockReturnValue(mockChat);
// @ts-expect-error - Mocking internal structure
mockGeminiClient.getCurrentSequenceModel = vi
.fn()
.mockReturnValue('model-1');
@@ -2192,6 +2001,7 @@ describe('runNonInteractive', () => {
expect(processStderrSpy).toHaveBeenCalledWith(
'Agent execution stopped: Stopped by hook\n',
);
// Should exit without calling sendMessageStream again
expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(1);
});
@@ -2222,9 +2032,9 @@ describe('runNonInteractive', () => {
expect(processStderrSpy).toHaveBeenCalledWith(
'[WARNING] Agent execution blocked: Blocked by hook\n',
);
// Stream continues after blocked event — content should be output
expect(getWrittenOutput()).toBe('Final answer\n');
// sendMessageStream is called once, recursion is internal to it and transparent to the caller
expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(1);
expect(getWrittenOutput()).toBe('Final answer\n');
});
});
@@ -2365,40 +2175,6 @@ describe('runNonInteractive', () => {
);
});
it('should emit warning event for loop_detected in streaming JSON mode', async () => {
vi.mocked(mockConfig.getOutputFormat).mockReturnValue(
OutputFormat.STREAM_JSON,
);
vi.mocked(uiTelemetryService.getMetrics).mockReturnValue(
MOCK_SESSION_METRICS,
);
const streamEvents: ServerGeminiStreamEvent[] = [
{ type: GeminiEventType.LoopDetected } as ServerGeminiStreamEvent,
{ type: GeminiEventType.Content, value: 'Continuing after loop' },
{
type: GeminiEventType.Finished,
value: { reason: undefined, usageMetadata: { totalTokenCount: 5 } },
},
];
mockGeminiClient.sendMessageStream.mockReturnValue(
createStreamFromEvents(streamEvents),
);
await runNonInteractive({
config: mockConfig,
settings: mockSettings,
input: 'Loop test explicit',
prompt_id: 'prompt-id-loop-explicit',
});
const output = getWrittenOutput();
// The STREAM_JSON output should contain an error event with warning severity
expect(output).toContain('"type":"error"');
expect(output).toContain('"severity":"warning"');
expect(output).toContain('Loop detected');
});
it('should report cancelled tool calls as success in stream-json mode (legacy parity)', async () => {
const toolCallEvent: ServerGeminiStreamEvent = {
type: GeminiEventType.ToolCallRequest,
@@ -11,17 +11,6 @@ Enter to submit · Esc to cancel
"
`;
exports[`AskUserDialog > Choice question placeholder > uses default placeholder when not provided 2`] = `
"Select your preferred language:
● 1. TypeScript
2. JavaScript
3. Enter a custom value
Enter to select · ↑/↓ to navigate · Esc to cancel
"
`;
exports[`AskUserDialog > Choice question placeholder > uses placeholder for "Other" option when provided 1`] = `
"Select your preferred language:
@@ -33,17 +22,6 @@ Enter to submit · Esc to cancel
"
`;
exports[`AskUserDialog > Choice question placeholder > uses placeholder for "Other" option when provided 2`] = `
"Select your preferred language:
● 1. TypeScript
2. JavaScript
3. Type another language...
Enter to select · ↑/↓ to navigate · Esc to cancel
"
`;
exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: false) > shows scroll arrows correctly when useAlternateBuffer is false 1`] = `
"Choose an option
@@ -203,19 +181,6 @@ Enter to submit · Tab/Shift+Tab to edit answers · Esc to cancel
"
`;
exports[`AskUserDialog > shows warning for unanswered questions on Review tab 2`] = `
"← □ License │ □ README │ ≡ Review →
Which license?
● 1. MIT
Permissive license
2. Enter a custom value
Enter to select · ←/→ to switch questions · Esc to cancel
"
`;
exports[`AskUserDialog > verifies "All of the above" visual state with snapshot 1`] = `
"Which features?
(Select all that apply)
@@ -140,33 +140,6 @@ Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel
"
`;
exports[`ExitPlanModeDialog > useAlternateBuffer: true > bubbles up Ctrl+C when feedback is empty while editing 2`] = `
"Overview
Add user authentication to the CLI application.
Implementation Steps
1. Create src/auth/AuthService.ts with login/logout methods
2. Add session storage in src/storage/SessionStore.ts
3. Update src/commands/index.ts to check auth status
4. Add tests in src/auth/__tests__/
Files to Modify
- src/index.ts - Add auth middleware
- src/config.ts - Add auth configuration options
● 1. Yes, automatically accept edits
Approves plan and allows tools to run automatically
2. Yes, manually accept edits
Approves plan but requires confirmation for each tool
3. Type your feedback...
Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel
"
`;
exports[`ExitPlanModeDialog > useAlternateBuffer: true > calls onFeedback when feedback is typed and submitted 1`] = `
"Overview
@@ -24,11 +24,6 @@ exports[`DenseToolMessage > flattens newlines in string results 1`] = `
"
`;
exports[`DenseToolMessage > flattens newlines in string results 2`] = `
" ✓ test-tool Test description → Line 1 Line 2
"
`;
exports[`DenseToolMessage > renders correctly for Edit tool using confirmationDetails 1`] = `
" ? Edit styles.scss → Confirming
"
@@ -151,15 +151,6 @@ exports[`<OverflowProvider><DiffRenderer /></OverflowProvider> > with useAlterna
"
`;
exports[`<OverflowProvider><DiffRenderer /></OverflowProvider> > with useAlternateBuffer = true > should handle diff with only header and no changes 2`] = `
"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮
│ │
│ No changes detected. │
│ │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
"
`;
exports[`<OverflowProvider><DiffRenderer /></OverflowProvider> > with useAlternateBuffer = true > should handle empty diff content 1`] = `
"No diff content.
"
@@ -78,7 +78,7 @@ exports[`<ToolGroupMessage /> > Golden Snapshots > renders header when scrolled
exports[`<ToolGroupMessage /> > Golden Snapshots > renders mixed tool calls including update_topic 1`] = `
"
Testing Topic: This is the description
Testing Topic: This is the summary
╭──────────────────────────────────────────────────────────────────────────╮
│ ✓ read_file Read a file │
@@ -142,12 +142,6 @@ exports[`<ToolGroupMessage /> > Golden Snapshots > renders two tool groups where
"
`;
exports[`<ToolGroupMessage /> > Golden Snapshots > renders update_topic tool call using TopicMessage > update_topic_tool 1`] = `
"
Testing Topic: This is the description
"
`;
exports[`<ToolGroupMessage /> > Golden Snapshots > renders with limited terminal height 1`] = `
"╭──────────────────────────────────────────────────────────────────────────╮
│ ✓ tool-with-result Tool with output │
+8 -37
View File
@@ -225,6 +225,7 @@ export interface GemmaModelRouterSettings {
export interface ADKSettings {
agentSessionNoninteractiveEnabled?: boolean;
interactiveAgentSessionEnabled?: boolean;
}
export interface ExtensionSetting {
@@ -697,7 +698,6 @@ export interface ConfigParameters {
experimentalJitContext?: boolean;
autoDistillation?: boolean;
experimentalMemoryManager?: boolean;
useAgentProtocol?: boolean;
experimentalAgentHistoryTruncation?: boolean;
experimentalAgentHistoryTruncationThreshold?: number;
experimentalAgentHistoryRetainedMessages?: number;
@@ -893,6 +893,7 @@ export class Config implements McpContext, AgentLoopContext {
private readonly gemmaModelRouter: GemmaModelRouterSettings;
private readonly agentSessionNoninteractiveEnabled: boolean;
private readonly interactiveAgentSessionEnabled: boolean;
private readonly continueOnFailedApiCall: boolean;
private readonly retryFetchErrors: boolean;
@@ -939,11 +940,6 @@ export class Config implements McpContext, AgentLoopContext {
private readonly adminSkillsEnabled: boolean;
private readonly experimentalJitContext: boolean;
private readonly experimentalMemoryManager: boolean;
private readonly useAgentProtocol: boolean;
private readonly experimentalAgentHistoryTruncation: boolean;
private readonly experimentalAgentHistoryTruncationThreshold: number;
private readonly experimentalAgentHistoryRetainedMessages: number;
private readonly experimentalAgentHistorySummarization: boolean;
private readonly memoryBoundaryMarkers: readonly string[];
private readonly topicUpdateNarration: boolean;
private readonly disableLLMCorrection: boolean;
@@ -1146,15 +1142,6 @@ export class Config implements McpContext, AgentLoopContext {
this.experimentalJitContext = params.experimentalJitContext ?? false;
this.experimentalMemoryManager = params.experimentalMemoryManager ?? false;
this.useAgentProtocol = params.useAgentProtocol ?? false;
this.experimentalAgentHistoryTruncation =
params.experimentalAgentHistoryTruncation ?? false;
this.experimentalAgentHistoryTruncationThreshold =
params.experimentalAgentHistoryTruncationThreshold ?? 30;
this.experimentalAgentHistoryRetainedMessages =
params.experimentalAgentHistoryRetainedMessages ?? 15;
this.experimentalAgentHistorySummarization =
params.experimentalAgentHistorySummarization ?? false;
this.memoryBoundaryMarkers = params.memoryBoundaryMarkers ?? ['.git'];
this.contextManagement = {
enabled: params.contextManagement?.enabled ?? false,
@@ -1329,6 +1316,8 @@ export class Config implements McpContext, AgentLoopContext {
this.agentSessionNoninteractiveEnabled =
params.adk?.agentSessionNoninteractiveEnabled ?? false;
this.interactiveAgentSessionEnabled =
params.adk?.interactiveAgentSessionEnabled ?? false;
this.retryFetchErrors = params.retryFetchErrors ?? true;
this.maxAttempts = Math.min(
params.maxAttempts ?? DEFAULT_MAX_ATTEMPTS,
@@ -2409,28 +2398,6 @@ export class Config implements McpContext, AgentLoopContext {
return this.experimentalMemoryManager;
}
getExperimentalUseAgentProtocol(): boolean {
return (
this.useAgentProtocol ||
process.env['GEMINI_CLI_USE_AGENT_PROTOCOL'] === 'true'
);
}
isExperimentalAgentHistoryTruncationEnabled(): boolean {
return this.experimentalAgentHistoryTruncation;
}
getExperimentalAgentHistoryTruncationThreshold(): number {
return this.experimentalAgentHistoryTruncationThreshold;
}
getExperimentalAgentHistoryRetainedMessages(): number {
return this.experimentalAgentHistoryRetainedMessages;
}
isExperimentalAgentHistorySummarizationEnabled(): boolean {
return this.experimentalAgentHistorySummarization;
}
getContextManagementConfig(): ContextManagementConfig {
return this.contextManagement;
}
@@ -3414,6 +3381,10 @@ export class Config implements McpContext, AgentLoopContext {
return this.agentSessionNoninteractiveEnabled;
}
getInteractiveAgentSessionEnabled(): boolean {
return this.interactiveAgentSessionEnabled;
}
/**
* Get override settings for a specific agent.
* Reads from agents.overrides.<agentName>.