mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-13 05:12:55 -07:00
fix(cli): filter subagent sessions from resume history (#19698)
This commit is contained in:
@@ -445,9 +445,76 @@ describe('SessionSelector', () => {
|
|||||||
const sessionSelector = new SessionSelector(config);
|
const sessionSelector = new SessionSelector(config);
|
||||||
const sessions = await sessionSelector.listSessions();
|
const sessions = await sessionSelector.listSessions();
|
||||||
|
|
||||||
|
// Should list the session with gemini message
|
||||||
expect(sessions.length).toBe(1);
|
expect(sessions.length).toBe(1);
|
||||||
expect(sessions[0].id).toBe(sessionIdGeminiOnly);
|
expect(sessions[0].id).toBe(sessionIdGeminiOnly);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should not list sessions marked as subagent', async () => {
|
||||||
|
const mainSessionId = randomUUID();
|
||||||
|
const subagentSessionId = randomUUID();
|
||||||
|
|
||||||
|
// Create test session files
|
||||||
|
const chatsDir = path.join(tmpDir, 'chats');
|
||||||
|
await fs.mkdir(chatsDir, { recursive: true });
|
||||||
|
|
||||||
|
// Main session - should be listed
|
||||||
|
const mainSession = {
|
||||||
|
sessionId: mainSessionId,
|
||||||
|
projectHash: 'test-hash',
|
||||||
|
startTime: '2024-01-01T10:00:00.000Z',
|
||||||
|
lastUpdated: '2024-01-01T10:30:00.000Z',
|
||||||
|
messages: [
|
||||||
|
{
|
||||||
|
type: 'user',
|
||||||
|
content: 'Hello world',
|
||||||
|
id: 'msg1',
|
||||||
|
timestamp: '2024-01-01T10:00:00.000Z',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
kind: 'main',
|
||||||
|
};
|
||||||
|
|
||||||
|
// Subagent session - should NOT be listed
|
||||||
|
const subagentSession = {
|
||||||
|
sessionId: subagentSessionId,
|
||||||
|
projectHash: 'test-hash',
|
||||||
|
startTime: '2024-01-01T11:00:00.000Z',
|
||||||
|
lastUpdated: '2024-01-01T11:30:00.000Z',
|
||||||
|
messages: [
|
||||||
|
{
|
||||||
|
type: 'user',
|
||||||
|
content: 'Internal subagent task',
|
||||||
|
id: 'msg1',
|
||||||
|
timestamp: '2024-01-01T11:00:00.000Z',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
kind: 'subagent',
|
||||||
|
};
|
||||||
|
|
||||||
|
await fs.writeFile(
|
||||||
|
path.join(
|
||||||
|
chatsDir,
|
||||||
|
`${SESSION_FILE_PREFIX}2024-01-01T10-00-${mainSessionId.slice(0, 8)}.json`,
|
||||||
|
),
|
||||||
|
JSON.stringify(mainSession, null, 2),
|
||||||
|
);
|
||||||
|
|
||||||
|
await fs.writeFile(
|
||||||
|
path.join(
|
||||||
|
chatsDir,
|
||||||
|
`${SESSION_FILE_PREFIX}2024-01-01T11-00-${subagentSessionId.slice(0, 8)}.json`,
|
||||||
|
),
|
||||||
|
JSON.stringify(subagentSession, null, 2),
|
||||||
|
);
|
||||||
|
|
||||||
|
const sessionSelector = new SessionSelector(config);
|
||||||
|
const sessions = await sessionSelector.listSessions();
|
||||||
|
|
||||||
|
// Should only list the main session
|
||||||
|
expect(sessions.length).toBe(1);
|
||||||
|
expect(sessions[0].id).toBe(mainSessionId);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('extractFirstUserMessage', () => {
|
describe('extractFirstUserMessage', () => {
|
||||||
|
|||||||
@@ -276,6 +276,12 @@ export const getAllSessionFiles = async (
|
|||||||
return { fileName: file, sessionInfo: null };
|
return { fileName: file, sessionInfo: null };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Skip subagent sessions - these are implementation details of a tool call
|
||||||
|
// and shouldn't be surfaced for resumption in the main agent history.
|
||||||
|
if (content.kind === 'subagent') {
|
||||||
|
return { fileName: file, sessionInfo: null };
|
||||||
|
}
|
||||||
|
|
||||||
const firstUserMessage = extractFirstUserMessage(content.messages);
|
const firstUserMessage = extractFirstUserMessage(content.messages);
|
||||||
const isCurrentSession = currentSessionId
|
const isCurrentSession = currentSessionId
|
||||||
? file.includes(currentSessionId.slice(0, 8))
|
? file.includes(currentSessionId.slice(0, 8))
|
||||||
|
|||||||
@@ -41,14 +41,15 @@ import type { Config } from '../config/config.js';
|
|||||||
import { MockTool } from '../test-utils/mock-tool.js';
|
import { MockTool } from '../test-utils/mock-tool.js';
|
||||||
import { getDirectoryContextString } from '../utils/environmentContext.js';
|
import { getDirectoryContextString } from '../utils/environmentContext.js';
|
||||||
import { z } from 'zod';
|
import { z } from 'zod';
|
||||||
|
import { getErrorMessage } from '../utils/errors.js';
|
||||||
import { promptIdContext } from '../utils/promptIdContext.js';
|
import { promptIdContext } from '../utils/promptIdContext.js';
|
||||||
import {
|
import {
|
||||||
logAgentStart,
|
logAgentStart,
|
||||||
logAgentFinish,
|
logAgentFinish,
|
||||||
logRecoveryAttempt,
|
logRecoveryAttempt,
|
||||||
} from '../telemetry/loggers.js';
|
} from '../telemetry/loggers.js';
|
||||||
import { LlmRole } from '../telemetry/types.js';
|
|
||||||
import {
|
import {
|
||||||
|
LlmRole,
|
||||||
AgentStartEvent,
|
AgentStartEvent,
|
||||||
AgentFinishEvent,
|
AgentFinishEvent,
|
||||||
RecoveryAttemptEvent,
|
RecoveryAttemptEvent,
|
||||||
@@ -1250,7 +1251,7 @@ describe('LocalAgentExecutor', () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
await expect(executor.run({ goal: 'test' }, signal)).rejects.toThrow(
|
await expect(executor.run({ goal: 'test' }, signal)).rejects.toThrow(
|
||||||
`Failed to create chat object: ${initError}`,
|
`Failed to create chat object: ${getErrorMessage(initError)}`,
|
||||||
);
|
);
|
||||||
|
|
||||||
// Ensure the error was reported via the activity callback
|
// Ensure the error was reported via the activity callback
|
||||||
@@ -1258,7 +1259,7 @@ describe('LocalAgentExecutor', () => {
|
|||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
type: 'ERROR',
|
type: 'ERROR',
|
||||||
data: expect.objectContaining({
|
data: expect.objectContaining({
|
||||||
error: `Error: Failed to create chat object: ${initError}`,
|
error: `Error: Failed to create chat object: ${getErrorMessage(initError)}`,
|
||||||
}),
|
}),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -33,8 +33,8 @@ import {
|
|||||||
import {
|
import {
|
||||||
AgentStartEvent,
|
AgentStartEvent,
|
||||||
AgentFinishEvent,
|
AgentFinishEvent,
|
||||||
RecoveryAttemptEvent,
|
|
||||||
LlmRole,
|
LlmRole,
|
||||||
|
RecoveryAttemptEvent,
|
||||||
} from '../telemetry/types.js';
|
} from '../telemetry/types.js';
|
||||||
import type {
|
import type {
|
||||||
LocalAgentDefinition,
|
LocalAgentDefinition,
|
||||||
@@ -48,6 +48,7 @@ import {
|
|||||||
DEFAULT_MAX_TURNS,
|
DEFAULT_MAX_TURNS,
|
||||||
DEFAULT_MAX_TIME_MINUTES,
|
DEFAULT_MAX_TIME_MINUTES,
|
||||||
} from './types.js';
|
} from './types.js';
|
||||||
|
import { getErrorMessage } from '../utils/errors.js';
|
||||||
import { templateString } from './utils.js';
|
import { templateString } from './utils.js';
|
||||||
import { DEFAULT_GEMINI_MODEL, isAutoModel } from '../config/models.js';
|
import { DEFAULT_GEMINI_MODEL, isAutoModel } from '../config/models.js';
|
||||||
import type { RoutingContext } from '../routing/routingStrategy.js';
|
import type { RoutingContext } from '../routing/routingStrategy.js';
|
||||||
@@ -826,16 +827,19 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
|
|||||||
systemInstruction,
|
systemInstruction,
|
||||||
[{ functionDeclarations: tools }],
|
[{ functionDeclarations: tools }],
|
||||||
startHistory,
|
startHistory,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
'subagent',
|
||||||
);
|
);
|
||||||
} catch (error) {
|
} catch (e: unknown) {
|
||||||
await reportError(
|
await reportError(
|
||||||
error,
|
e,
|
||||||
`Error initializing Gemini chat for agent ${this.definition.name}.`,
|
`Error initializing Gemini chat for agent ${this.definition.name}.`,
|
||||||
startHistory,
|
startHistory,
|
||||||
'startChat',
|
'startChat',
|
||||||
);
|
);
|
||||||
// Re-throw as a more specific error after reporting.
|
// Re-throw as a more specific error after reporting.
|
||||||
throw new Error(`Failed to create chat object: ${error}`);
|
throw new Error(`Failed to create chat object: ${getErrorMessage(e)}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -249,10 +249,11 @@ export class GeminiChat {
|
|||||||
private history: Content[] = [],
|
private history: Content[] = [],
|
||||||
resumedSessionData?: ResumedSessionData,
|
resumedSessionData?: ResumedSessionData,
|
||||||
private readonly onModelChanged?: (modelId: string) => Promise<Tool[]>,
|
private readonly onModelChanged?: (modelId: string) => Promise<Tool[]>,
|
||||||
|
kind: 'main' | 'subagent' = 'main',
|
||||||
) {
|
) {
|
||||||
validateHistory(history);
|
validateHistory(history);
|
||||||
this.chatRecordingService = new ChatRecordingService(config);
|
this.chatRecordingService = new ChatRecordingService(config);
|
||||||
this.chatRecordingService.initialize(resumedSessionData);
|
this.chatRecordingService.initialize(resumedSessionData, kind);
|
||||||
this.lastPromptTokenCount = estimateTokenCountSync(
|
this.lastPromptTokenCount = estimateTokenCountSync(
|
||||||
this.history.flatMap((c) => c.parts || []),
|
this.history.flatMap((c) => c.parts || []),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -86,6 +86,21 @@ describe('ChatRecordingService', () => {
|
|||||||
expect(files[0]).toMatch(/^session-.*-test-ses\.json$/);
|
expect(files[0]).toMatch(/^session-.*-test-ses\.json$/);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should include the conversation kind when specified', () => {
|
||||||
|
chatRecordingService.initialize(undefined, 'subagent');
|
||||||
|
chatRecordingService.recordMessage({
|
||||||
|
type: 'user',
|
||||||
|
content: 'ping',
|
||||||
|
model: 'm',
|
||||||
|
});
|
||||||
|
|
||||||
|
const sessionFile = chatRecordingService.getConversationFilePath()!;
|
||||||
|
const conversation = JSON.parse(
|
||||||
|
fs.readFileSync(sessionFile, 'utf8'),
|
||||||
|
) as ConversationRecord;
|
||||||
|
expect(conversation.kind).toBe('subagent');
|
||||||
|
});
|
||||||
|
|
||||||
it('should resume from an existing session if provided', () => {
|
it('should resume from an existing session if provided', () => {
|
||||||
const chatsDir = path.join(testTempDir, 'chats');
|
const chatsDir = path.join(testTempDir, 'chats');
|
||||||
fs.mkdirSync(chatsDir, { recursive: true });
|
fs.mkdirSync(chatsDir, { recursive: true });
|
||||||
|
|||||||
@@ -102,6 +102,8 @@ export interface ConversationRecord {
|
|||||||
summary?: string;
|
summary?: string;
|
||||||
/** Workspace directories added during the session via /dir add */
|
/** Workspace directories added during the session via /dir add */
|
||||||
directories?: string[];
|
directories?: string[];
|
||||||
|
/** The kind of conversation (main agent or subagent) */
|
||||||
|
kind?: 'main' | 'subagent';
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -128,6 +130,7 @@ export class ChatRecordingService {
|
|||||||
private cachedLastConvData: string | null = null;
|
private cachedLastConvData: string | null = null;
|
||||||
private sessionId: string;
|
private sessionId: string;
|
||||||
private projectHash: string;
|
private projectHash: string;
|
||||||
|
private kind?: 'main' | 'subagent';
|
||||||
private queuedThoughts: Array<ThoughtSummary & { timestamp: string }> = [];
|
private queuedThoughts: Array<ThoughtSummary & { timestamp: string }> = [];
|
||||||
private queuedTokens: TokensSummary | null = null;
|
private queuedTokens: TokensSummary | null = null;
|
||||||
private config: Config;
|
private config: Config;
|
||||||
@@ -141,13 +144,21 @@ export class ChatRecordingService {
|
|||||||
/**
|
/**
|
||||||
* Initializes the chat recording service: creates a new conversation file and associates it with
|
* Initializes the chat recording service: creates a new conversation file and associates it with
|
||||||
* this service instance, or resumes from an existing session if resumedSessionData is provided.
|
* this service instance, or resumes from an existing session if resumedSessionData is provided.
|
||||||
|
*
|
||||||
|
* @param resumedSessionData Data from a previous session to resume from.
|
||||||
|
* @param kind The kind of conversation (main or subagent).
|
||||||
*/
|
*/
|
||||||
initialize(resumedSessionData?: ResumedSessionData): void {
|
initialize(
|
||||||
|
resumedSessionData?: ResumedSessionData,
|
||||||
|
kind?: 'main' | 'subagent',
|
||||||
|
): void {
|
||||||
try {
|
try {
|
||||||
|
this.kind = kind;
|
||||||
if (resumedSessionData) {
|
if (resumedSessionData) {
|
||||||
// Resume from existing session
|
// Resume from existing session
|
||||||
this.conversationFile = resumedSessionData.filePath;
|
this.conversationFile = resumedSessionData.filePath;
|
||||||
this.sessionId = resumedSessionData.conversation.sessionId;
|
this.sessionId = resumedSessionData.conversation.sessionId;
|
||||||
|
this.kind = resumedSessionData.conversation.kind;
|
||||||
|
|
||||||
// Update the session ID in the existing file
|
// Update the session ID in the existing file
|
||||||
this.updateConversation((conversation) => {
|
this.updateConversation((conversation) => {
|
||||||
@@ -180,6 +191,7 @@ export class ChatRecordingService {
|
|||||||
startTime: new Date().toISOString(),
|
startTime: new Date().toISOString(),
|
||||||
lastUpdated: new Date().toISOString(),
|
lastUpdated: new Date().toISOString(),
|
||||||
messages: [],
|
messages: [],
|
||||||
|
kind: this.kind,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -435,6 +447,7 @@ export class ChatRecordingService {
|
|||||||
startTime: new Date().toISOString(),
|
startTime: new Date().toISOString(),
|
||||||
lastUpdated: new Date().toISOString(),
|
lastUpdated: new Date().toISOString(),
|
||||||
messages: [],
|
messages: [],
|
||||||
|
kind: this.kind,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user