mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-13 21:07:00 -07:00
fix stale state in /rewind
This commit is contained in:
@@ -577,7 +577,7 @@ describe('useSlashCommandProcessor', () => {
|
||||
|
||||
it('should handle "load_history" action', async () => {
|
||||
const mockClient = {
|
||||
setHistory: vi.fn(),
|
||||
resumeChat: vi.fn().mockResolvedValue(undefined),
|
||||
stripThoughtsFromHistory: vi.fn(),
|
||||
} as unknown as GeminiClient;
|
||||
vi.spyOn(mockConfig, 'getGeminiClient').mockReturnValue(mockClient);
|
||||
|
||||
@@ -549,7 +549,9 @@ export const useSlashCommandProcessor = (
|
||||
}
|
||||
}
|
||||
case 'load_history': {
|
||||
config?.getGeminiClient()?.setHistory(result.clientHistory);
|
||||
await config
|
||||
?.getGeminiClient()
|
||||
?.resumeChat(result.clientHistory);
|
||||
fullCommandContext.ui.clear();
|
||||
result.history.forEach((item, index) => {
|
||||
fullCommandContext.ui.addItem(item, index);
|
||||
|
||||
@@ -196,7 +196,9 @@ describe('ChatCompressionService', () => {
|
||||
} as unknown as Config;
|
||||
|
||||
vi.mocked(getInitialChatHistory).mockImplementation(
|
||||
async (_config, extraHistory) => extraHistory || [],
|
||||
async (_config, extraHistory?: readonly Content[]) => [
|
||||
...(extraHistory || []),
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -324,7 +324,7 @@ export class GeminiClient {
|
||||
}
|
||||
|
||||
async resumeChat(
|
||||
history: Content[],
|
||||
history: readonly Content[],
|
||||
resumedSessionData?: ResumedSessionData,
|
||||
): Promise<void> {
|
||||
this.chat = await this.startChat(history, resumedSessionData);
|
||||
@@ -365,7 +365,7 @@ export class GeminiClient {
|
||||
}
|
||||
|
||||
async startChat(
|
||||
extraHistory?: Content[],
|
||||
extraHistory?: readonly Content[],
|
||||
resumedSessionData?: ResumedSessionData,
|
||||
): Promise<GeminiChat> {
|
||||
this.forceFullIdeContext = true;
|
||||
|
||||
@@ -273,6 +273,14 @@ export class GeminiChat {
|
||||
kind: 'main' | 'subagent' = 'main',
|
||||
) {
|
||||
await this.chatRecordingService.initialize(resumedSessionData, kind);
|
||||
|
||||
// If we have history but didn't resume a session record, sync it to the recording service.
|
||||
// This handles initial history passed to startChat.
|
||||
if (!resumedSessionData && this.agentHistory.get().length > 0) {
|
||||
this.chatRecordingService.updateMessagesFromHistory(
|
||||
this.agentHistory.get(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
setSystemInstruction(sysInstr: string) {
|
||||
@@ -775,7 +783,7 @@ export class GeminiChat {
|
||||
this.lastPromptTokenCount = estimateTokenCountSync(
|
||||
this.agentHistory.flatMap((c) => c.parts || []),
|
||||
);
|
||||
this.chatRecordingService.updateMessagesFromHistory(history);
|
||||
this.chatRecordingService.updateMessagesFromHistory(history, true);
|
||||
}
|
||||
|
||||
stripThoughtsFromHistory(): void {
|
||||
|
||||
@@ -1237,6 +1237,64 @@ describe('ChatRecordingService', () => {
|
||||
// No tool calls matched, so writeFileSync should NOT have been called
|
||||
expect(appendFileSyncSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should repopulate cachedConversation.messages when updating from history if cache is empty (regression)', async () => {
|
||||
// This simulates the state after /chat resume where history is loaded into GeminiChat
|
||||
// but ChatRecordingService's cache is still empty.
|
||||
const history: Content[] = [
|
||||
{
|
||||
role: 'user',
|
||||
parts: [{ text: 'Hello' }],
|
||||
},
|
||||
{
|
||||
role: 'model',
|
||||
parts: [{ text: 'Hi there!' }],
|
||||
},
|
||||
{
|
||||
role: 'user',
|
||||
parts: [{ text: 'How are you?' }],
|
||||
},
|
||||
];
|
||||
|
||||
// Initially empty (except for metadata)
|
||||
expect(chatRecordingService.getConversation()?.messages).toHaveLength(0);
|
||||
|
||||
chatRecordingService.updateMessagesFromHistory(history);
|
||||
|
||||
const messages = chatRecordingService.getConversation()?.messages;
|
||||
// CURRENTLY FAILS: it only updates tool results, doesn't reconstruct messages.
|
||||
expect(messages).toHaveLength(3);
|
||||
expect(messages![0].content).toEqual([{ text: 'Hello' }]);
|
||||
expect(messages![1].content).toEqual([{ text: 'Hi there!' }]);
|
||||
expect(messages![2].content).toEqual([{ text: 'How are you?' }]);
|
||||
});
|
||||
|
||||
it('should force reconstruction when reconstruct flag is true, even if cache is not empty', async () => {
|
||||
// 1. Initial state with some messages
|
||||
chatRecordingService.recordMessage({
|
||||
type: 'user',
|
||||
content: 'Old user message',
|
||||
model: 'gemini-pro',
|
||||
});
|
||||
|
||||
expect(chatRecordingService.getConversation()?.messages).toHaveLength(1);
|
||||
|
||||
// 2. New history to replace the old one
|
||||
const newHistory: Content[] = [
|
||||
{
|
||||
role: 'user',
|
||||
parts: [{ text: 'New user message' }],
|
||||
},
|
||||
];
|
||||
|
||||
// 3. Update with reconstruct = true
|
||||
chatRecordingService.updateMessagesFromHistory(newHistory, true);
|
||||
|
||||
const messages = chatRecordingService.getConversation()?.messages;
|
||||
expect(messages).toHaveLength(1);
|
||||
expect(messages![0].content).toEqual([{ text: 'New user message' }]);
|
||||
expect(messages![0].type).toBe('user');
|
||||
});
|
||||
});
|
||||
|
||||
describe('ENOENT (missing directory) handling', () => {
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { type ThoughtSummary } from '../utils/thoughtUtils.js';
|
||||
import { type ThoughtSummary, parseThought } from '../utils/thoughtUtils.js';
|
||||
import { getProjectHash } from '../utils/paths.js';
|
||||
import path from 'node:path';
|
||||
import * as fs from 'node:fs';
|
||||
@@ -23,6 +23,7 @@ import type {
|
||||
GenerateContentResponseUsageMetadata,
|
||||
} from '@google/genai';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
import { CoreToolCallStatus } from '../scheduler/types.js';
|
||||
import type { AgentLoopContext } from '../config/agent-loop-context.js';
|
||||
import {
|
||||
SESSION_FILE_PREFIX,
|
||||
@@ -844,10 +845,126 @@ export class ChatRecordingService {
|
||||
return this.cachedConversation;
|
||||
}
|
||||
|
||||
updateMessagesFromHistory(history: readonly Content[]): void {
|
||||
private reconstructMessagesFromHistory(
|
||||
history: readonly Content[],
|
||||
): MessageRecord[] {
|
||||
const messages: MessageRecord[] = [];
|
||||
let i = 0;
|
||||
|
||||
while (i < history.length) {
|
||||
const content = history[i];
|
||||
const parts = content.parts || [];
|
||||
|
||||
if (content.role === 'user') {
|
||||
// Simple user message
|
||||
messages.push({
|
||||
id: randomUUID(),
|
||||
timestamp: new Date().toISOString(),
|
||||
type: 'user',
|
||||
content: parts,
|
||||
});
|
||||
i++;
|
||||
} else if (content.role === 'model') {
|
||||
const geminiMsg: MessageRecord & { type: 'gemini' } = {
|
||||
id: randomUUID(),
|
||||
timestamp: new Date().toISOString(),
|
||||
type: 'gemini',
|
||||
content: parts.filter(
|
||||
(p) => !p.functionCall && !p.thought && !p.functionResponse,
|
||||
),
|
||||
toolCalls: [],
|
||||
thoughts: [],
|
||||
};
|
||||
|
||||
// Add thoughts
|
||||
for (const part of parts) {
|
||||
if (part.thought) {
|
||||
const thoughtObj = parseThought(part.text || '');
|
||||
geminiMsg.thoughts!.push({
|
||||
...thoughtObj,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Add tool calls
|
||||
for (const part of parts) {
|
||||
if (part.functionCall) {
|
||||
geminiMsg.toolCalls!.push({
|
||||
id: part.functionCall.id || `reconstructed-${randomUUID()}`,
|
||||
name: part.functionCall.name || 'unknown_tool',
|
||||
args: part.functionCall.args || {},
|
||||
status: CoreToolCallStatus.Success, // Assume success for reconstructed history
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Look ahead for responses
|
||||
if (
|
||||
geminiMsg.toolCalls!.length > 0 &&
|
||||
i + 1 < history.length &&
|
||||
history[i + 1].role === 'user'
|
||||
) {
|
||||
const nextTurn = history[i + 1];
|
||||
const nextParts = nextTurn.parts || [];
|
||||
const hasResponses = nextParts.some((p) => p.functionResponse);
|
||||
|
||||
if (hasResponses) {
|
||||
const respMap = new Map<string, Part[]>();
|
||||
for (const p of nextParts) {
|
||||
if (p.functionResponse?.id) {
|
||||
if (!respMap.has(p.functionResponse.id)) {
|
||||
respMap.set(p.functionResponse.id, []);
|
||||
}
|
||||
respMap.get(p.functionResponse.id)!.push(p);
|
||||
}
|
||||
}
|
||||
|
||||
for (const tc of geminiMsg.toolCalls!) {
|
||||
const respParts = respMap.get(tc.id);
|
||||
if (respParts) {
|
||||
tc.result = respParts;
|
||||
}
|
||||
}
|
||||
// Consume the response turn
|
||||
i++;
|
||||
}
|
||||
}
|
||||
|
||||
messages.push(geminiMsg);
|
||||
i++;
|
||||
} else {
|
||||
i++; // Skip unknown roles
|
||||
}
|
||||
}
|
||||
|
||||
return messages;
|
||||
}
|
||||
|
||||
updateMessagesFromHistory(
|
||||
history: readonly Content[],
|
||||
reconstruct = false,
|
||||
): void {
|
||||
if (!this.conversationFile || !this.cachedConversation) return;
|
||||
|
||||
try {
|
||||
// If the cache is empty (e.g. after /resume load_history), or reconstruction is forced,
|
||||
// reconstruct from history.
|
||||
if (
|
||||
(this.cachedConversation.messages.length === 0 && history.length > 0) ||
|
||||
reconstruct
|
||||
) {
|
||||
this.cachedConversation.messages =
|
||||
this.reconstructMessagesFromHistory(history);
|
||||
this.updateMetadata({ lastUpdated: new Date().toISOString() });
|
||||
// Snapshot the reconstruction to ensure persistence
|
||||
this.appendRecord({
|
||||
$set: { messages: this.cachedConversation.messages },
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const partsMap = new Map<string, Part[]>();
|
||||
for (const content of history) {
|
||||
if (content.role === 'user' && content.parts) {
|
||||
|
||||
@@ -86,7 +86,7 @@ ${environmentMemory}
|
||||
|
||||
export async function getInitialChatHistory(
|
||||
config: Config,
|
||||
extraHistory?: Content[],
|
||||
extraHistory?: readonly Content[],
|
||||
): Promise<Content[]> {
|
||||
const envParts = await getEnvironmentContext(config);
|
||||
const envContextString = envParts.map((part) => part.text || '').join('\n\n');
|
||||
|
||||
Reference in New Issue
Block a user