Avoid persisting empty resume sessions (#27770)

This commit is contained in:
Sandy Tao
2026-06-09 15:41:47 -07:00
committed by GitHub
parent 4523560278
commit 3a13b8eeb6
9 changed files with 318 additions and 199 deletions
+3 -2
View File
@@ -1757,8 +1757,9 @@ describe('startInteractiveUI', () => {
// Verify all startup tasks were called
expect(getVersion).toHaveBeenCalledTimes(1);
// 5 cleanups: mouseEvents, consolePatcher, lineWrapping, instance.unmount, and TTY check
expect(registerCleanup).toHaveBeenCalledTimes(5);
// 6 cleanups: mouseEvents, lineWrapping, non-resumable session cleanup,
// instance.unmount, TTY check, and consolePatcher
expect(registerCleanup).toHaveBeenCalledTimes(6);
// Verify cleanup handler is registered with unmount function
const cleanupFn = vi.mocked(registerCleanup).mock.calls[0][0];
+18
View File
@@ -194,6 +194,17 @@ export async function startInteractiveUI(
});
const cleanupUnmount = () => instance.unmount();
const cleanupNonResumableCurrentSession = async () => {
try {
await config
.getGeminiClient()
?.getChatRecordingService()
?.deleteCurrentSessionIfNotResumableAsync();
} catch (e: unknown) {
debugLogger.error('Error cleaning up non-resumable session:', e);
}
};
registerCleanup(cleanupNonResumableCurrentSession);
registerCleanup(cleanupUnmount);
const cleanupTtyCheck = setupTtyCheck();
@@ -212,6 +223,13 @@ export async function startInteractiveUI(
debugLogger.error('Error cleaning up console patcher:', e);
}
try {
removeCleanup(cleanupNonResumableCurrentSession);
await cleanupNonResumableCurrentSession();
} catch (e: unknown) {
debugLogger.error('Error removing non-resumable session cleanup:', e);
}
try {
removeCleanup(cleanupUnmount);
instance.unmount();
@@ -3673,9 +3673,12 @@ describe('InputPrompt', () => {
});
it('should toggle paste expansion on double-click', async () => {
vi.spyOn(Date, 'now').mockReturnValue(1000);
const id = '[Pasted Text: 10 lines]';
const largeText =
'line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10';
const togglePasteExpansion = vi.fn();
const baseProps = props;
const TestWrapper = () => {
@@ -3714,8 +3717,9 @@ describe('InputPrompt', () => {
row: 0,
col: 2,
}),
togglePasteExpansion: vi.fn().mockImplementation(() => {
setIsExpanded(!isExpanded);
togglePasteExpansion: vi.fn().mockImplementation((...args) => {
togglePasteExpansion(...args);
setIsExpanded((expanded) => !expanded);
}),
getExpandedPasteAtLine: vi
.fn()
@@ -3746,7 +3750,8 @@ describe('InputPrompt', () => {
// 2. Verify expanded content is visible
await waitFor(() => {
expect(stdout.lastFrame()).toMatchSnapshot();
expect(togglePasteExpansion).toHaveBeenCalledWith(id, 0, 2);
expect(stdout.lastFrame()).toContain('line10');
});
// Simulate double-click to collapse
@@ -3755,6 +3760,8 @@ describe('InputPrompt', () => {
// 3. Verify placeholder is restored
await waitFor(() => {
expect(togglePasteExpansion).toHaveBeenCalledTimes(2);
expect(stdout.lastFrame()).toContain(id);
expect(stdout.lastFrame()).toMatchSnapshot();
});
@@ -161,13 +161,6 @@ exports[`InputPrompt > mouse interaction > should toggle paste expansion on doub
"
`;
exports[`InputPrompt > mouse interaction > should toggle paste expansion on double-click 3`] = `
"▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
> [Pasted Text: 10 lines]
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
"
`;
exports[`InputPrompt > multiline rendering > should correctly render multiline input including blank lines 1`] = `
"────────────────────────────────────────────────────────────────────────────────────────────────────
> hello
+74 -142
View File
@@ -9,7 +9,6 @@ import {
SessionSelector,
extractFirstUserMessage,
formatRelativeTime,
hasUserOrAssistantMessage,
SessionError,
convertSessionToHistoryFormats,
} from './sessionUtils.js';
@@ -512,6 +511,80 @@ describe('SessionSelector', () => {
expect(sessions[0].id).toBe(sessionIdWithUser);
});
it('should not list command-only sessions', async () => {
const commandOnlySessionId = randomUUID();
const chatsDir = path.join(tmpDir, 'chats');
await fs.mkdir(chatsDir, { recursive: true });
const metadata = {
sessionId: commandOnlySessionId,
projectHash: 'test-hash',
startTime: '2024-01-01T10:00:00.000Z',
lastUpdated: '2024-01-01T10:01:00.000Z',
};
const commandMessage = {
type: 'user',
content: '/resume',
id: 'msg1',
timestamp: '2024-01-01T10:00:30.000Z',
};
await fs.writeFile(
path.join(
chatsDir,
`${SESSION_FILE_PREFIX}2024-01-01T10-00-${commandOnlySessionId.slice(0, 8)}.jsonl`,
),
`${JSON.stringify(metadata)}\n${JSON.stringify(commandMessage)}\n`,
);
const sessionSelector = new SessionSelector(storage);
const sessions = await sessionSelector.listSessions();
expect(sessions).toEqual([]);
});
it('should use the first non-command user message for display', async () => {
const sessionId = randomUUID();
const chatsDir = path.join(tmpDir, 'chats');
await fs.mkdir(chatsDir, { recursive: true });
const metadata = {
sessionId,
projectHash: 'test-hash',
startTime: '2024-01-01T10:00:00.000Z',
lastUpdated: '2024-01-01T10:02:00.000Z',
};
const commandMessage = {
type: 'user',
content: '/resume',
id: 'msg1',
timestamp: '2024-01-01T10:00:30.000Z',
};
const realMessage = {
type: 'user',
content: 'Help me fix resume history',
id: 'msg2',
timestamp: '2024-01-01T10:01:00.000Z',
};
await fs.writeFile(
path.join(
chatsDir,
`${SESSION_FILE_PREFIX}2024-01-01T10-00-${sessionId.slice(0, 8)}.jsonl`,
),
`${JSON.stringify(metadata)}\n${JSON.stringify(commandMessage)}\n${JSON.stringify(realMessage)}\n`,
);
const sessionSelector = new SessionSelector(storage);
const sessions = await sessionSelector.listSessions();
expect(sessions).toHaveLength(1);
expect(sessions[0].firstUserMessage).toBe('Help me fix resume history');
expect(sessions[0].displayName).toBe('Help me fix resume history');
});
it('should list session with gemini message even without user message', async () => {
const sessionIdGeminiOnly = randomUUID();
@@ -781,147 +854,6 @@ describe('extractFirstUserMessage', () => {
});
});
describe('hasUserOrAssistantMessage', () => {
it('should return true when session has user message', () => {
const messages = [
{
type: 'user',
content: 'Hello',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
] as MessageRecord[];
expect(hasUserOrAssistantMessage(messages)).toBe(true);
});
it('should return true when session has gemini message', () => {
const messages = [
{
type: 'gemini',
content: 'Hello, how can I help?',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
] as MessageRecord[];
expect(hasUserOrAssistantMessage(messages)).toBe(true);
});
it('should return true when session has both user and gemini messages', () => {
const messages = [
{
type: 'user',
content: 'Hello',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
{
type: 'gemini',
content: 'Hi there!',
id: 'msg2',
timestamp: '2024-01-01T10:01:00.000Z',
},
] as MessageRecord[];
expect(hasUserOrAssistantMessage(messages)).toBe(true);
});
it('should return false when session only has info messages', () => {
const messages = [
{
type: 'info',
content: 'Session started',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
] as MessageRecord[];
expect(hasUserOrAssistantMessage(messages)).toBe(false);
});
it('should return false when session only has error messages', () => {
const messages = [
{
type: 'error',
content: 'An error occurred',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
] as MessageRecord[];
expect(hasUserOrAssistantMessage(messages)).toBe(false);
});
it('should return false when session only has warning messages', () => {
const messages = [
{
type: 'warning',
content: 'Warning message',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
] as MessageRecord[];
expect(hasUserOrAssistantMessage(messages)).toBe(false);
});
it('should return false when session only has system messages (mixed)', () => {
const messages = [
{
type: 'info',
content: 'Session started',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
{
type: 'error',
content: 'An error occurred',
id: 'msg2',
timestamp: '2024-01-01T10:01:00.000Z',
},
{
type: 'warning',
content: 'Warning message',
id: 'msg3',
timestamp: '2024-01-01T10:02:00.000Z',
},
] as MessageRecord[];
expect(hasUserOrAssistantMessage(messages)).toBe(false);
});
it('should return true when session has user message among system messages', () => {
const messages = [
{
type: 'info',
content: 'Session started',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
{
type: 'user',
content: 'Hello',
id: 'msg2',
timestamp: '2024-01-01T10:01:00.000Z',
},
{
type: 'error',
content: 'An error occurred',
id: 'msg3',
timestamp: '2024-01-01T10:02:00.000Z',
},
] as MessageRecord[];
expect(hasUserOrAssistantMessage(messages)).toBe(true);
});
it('should return false for empty messages array', () => {
const messages: MessageRecord[] = [];
expect(hasUserOrAssistantMessage(messages)).toBe(false);
});
});
describe('formatRelativeTime', () => {
it('should format time correctly', () => {
const now = new Date();
+4 -11
View File
@@ -139,15 +139,6 @@ export interface SessionSelectionResult {
displayInfo: string;
}
/**
* Checks if a session has at least one user or assistant (gemini) message.
* Sessions with only system messages (info, error, warning) are considered empty.
* @param messages - The array of message records to check
* @returns true if the session has meaningful content
*/
export const hasUserOrAssistantMessage = (messages: MessageRecord[]): boolean =>
messages.some((msg) => msg.type === 'user' || msg.type === 'gemini');
/**
* Cleans and sanitizes message content for display by:
* - Converting newlines to spaces
@@ -287,8 +278,10 @@ export const getAllSessionFiles = async (
const lastUpdated =
content.lastUpdated || content.startTime || fallbackTimestamp;
// Skip sessions that only contain system messages (info, error, warning)
if (!content.hasUserOrAssistantMessage) {
// Skip sessions with no resumable conversation content, including
// startup-only, system-only, command-only, and internal-context-only
// sessions.
if (!content.hasResumableContent) {
return { fileName: file, sessionInfo: null };
}
@@ -40,6 +40,8 @@ vi.mock('node:fs', async (importOriginal) => {
import {
ChatRecordingService,
hasResumableConversationContent,
isResumableMessageRecord,
loadConversationRecord,
type ConversationRecord,
type ToolCallRecord,
@@ -125,6 +127,76 @@ describe('ChatRecordingService', () => {
}
});
describe('isResumableMessageRecord', () => {
it('should treat malformed messages without content as non-resumable', () => {
const message = {
id: 'malformed-message',
timestamp: '2024-01-01T00:00:00.000Z',
type: 'user',
} as MessageRecord;
expect(() => isResumableMessageRecord(message)).not.toThrow();
expect(isResumableMessageRecord(message)).toBe(false);
});
it('should return false for command-only messages', () => {
const messages = [
{
type: 'user',
content: '/resume',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
{
type: 'user',
content: '?help',
id: 'msg2',
timestamp: '2024-01-01T10:01:00.000Z',
},
] as MessageRecord[];
expect(hasResumableConversationContent(messages)).toBe(false);
});
it('should return false for internal context-only messages', () => {
const messages = [
{
type: 'user',
content: '<session_context>previous state</session_context>',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
{
type: 'user',
content: '<hook_context>hook data</hook_context>',
id: 'msg2',
timestamp: '2024-01-01T10:01:00.000Z',
},
] as MessageRecord[];
expect(hasResumableConversationContent(messages)).toBe(false);
});
it('should return true for real user or assistant content', () => {
const messages = [
{
type: 'user',
content: '/resume',
id: 'msg1',
timestamp: '2024-01-01T10:00:00.000Z',
},
{
type: 'gemini',
content: 'I can help with that.',
id: 'msg2',
timestamp: '2024-01-01T10:01:00.000Z',
},
] as MessageRecord[];
expect(hasResumableConversationContent(messages)).toBe(true);
});
});
describe('initialize', () => {
it('should create a new session if none is provided', async () => {
await chatRecordingService.initialize();
@@ -838,6 +910,49 @@ describe('ChatRecordingService', () => {
});
});
describe('deleteCurrentSessionIfNotResumableAsync', () => {
it('should delete a startup-only session', async () => {
await chatRecordingService.initialize();
const conversationFile = chatRecordingService.getConversationFilePath();
expect(conversationFile).not.toBeNull();
expect(fs.existsSync(conversationFile!)).toBe(true);
await chatRecordingService.deleteCurrentSessionIfNotResumableAsync();
expect(fs.existsSync(conversationFile!)).toBe(false);
});
it('should delete a command-only session', async () => {
await chatRecordingService.initialize();
chatRecordingService.recordMessage({
type: 'user',
content: '/resume',
model: 'gemini-pro',
});
const conversationFile = chatRecordingService.getConversationFilePath();
expect(conversationFile).not.toBeNull();
await chatRecordingService.deleteCurrentSessionIfNotResumableAsync();
expect(fs.existsSync(conversationFile!)).toBe(false);
});
it('should keep a session with a real user message', async () => {
await chatRecordingService.initialize();
chatRecordingService.recordMessage({
type: 'user',
content: 'Help me debug this test',
model: 'gemini-pro',
});
const conversationFile = chatRecordingService.getConversationFilePath();
expect(conversationFile).not.toBeNull();
await chatRecordingService.deleteCurrentSessionIfNotResumableAsync();
expect(fs.existsSync(conversationFile!)).toBe(true);
});
});
describe('recordDirectories', () => {
beforeEach(async () => {
await chatRecordingService.initialize();
@@ -23,6 +23,8 @@ import type {
import { debugLogger } from '../utils/debugLogger.js';
import type { AgentLoopContext } from '../config/agent-loop-context.js';
import type { HistoryTurn } from '../core/agentChatHistory.js';
import { partListUnionToString } from '../core/geminiRequest.js';
import { isIgnoredUserContent } from '../utils/sessionUtils.js';
import {
SESSION_FILE_PREFIX,
type TokensSummary,
@@ -98,6 +100,36 @@ function isTextPart(part: unknown): part is { text: string } {
return isStringProperty(part, 'text');
}
/**
* Returns true when a stored message represents conversation content worth
* surfacing in resume flows.
*/
export function isResumableMessageRecord(message: MessageRecord): boolean {
const contentString = message.content
? partListUnionToString(message.content)
: '';
if (message.type === 'user') {
return !isIgnoredUserContent(contentString.trim());
}
if (message.type === 'gemini') {
return (
contentString.trim().length > 0 ||
(message.toolCalls?.length ?? 0) > 0 ||
(message.thoughts?.length ?? 0) > 0
);
}
return false;
}
export function hasResumableConversationContent(
messages: readonly MessageRecord[],
): boolean {
return messages.some((message) => isResumableMessageRecord(message));
}
export async function loadConversationRecord(
filePath: string,
options?: LoadConversationOptions,
@@ -106,7 +138,7 @@ export async function loadConversationRecord(
messageCount?: number;
userMessageCount?: number;
firstUserMessage?: string;
hasUserOrAssistantMessage?: boolean;
hasResumableContent?: boolean;
memoryScratchpadIsStale?: boolean;
})
| null
@@ -127,7 +159,7 @@ export async function loadConversationRecord(
const messageIds: string[] = [];
const messageKinds = new Map<
string,
{ isUser: boolean; isUserOrAssistant: boolean }
{ isUser: boolean; isResumable: boolean }
>();
let isTrackingMemoryScratchpadFreshness = false;
let memoryScratchpadIsStale = false;
@@ -174,19 +206,18 @@ export async function loadConversationRecord(
}
const id = record.id;
const isUser = hasProperty(record, 'type') && record.type === 'user';
const isUserOrAssistant =
hasProperty(record, 'type') &&
(record.type === 'user' || record.type === 'gemini');
const isResumable = isResumableMessageRecord(record);
// Track message count and first user message
if (options?.metadataOnly) {
messageIds.push(id);
messageKinds.set(id, { isUser, isUserOrAssistant });
messageKinds.set(id, { isUser, isResumable });
}
if (
!firstUserMessageStr &&
isUser &&
hasProperty(record, 'content') &&
record['content']
record['content'] &&
isResumable
) {
// Basic extraction of first user message for display
const rawContent = record['content'];
@@ -230,12 +261,14 @@ export async function loadConversationRecord(
if (isMessageRecord(msg)) {
const id = msg.id;
const isUser = msg.type === 'user';
const isUserOrAssistant =
msg.type === 'user' || msg.type === 'gemini';
const isResumable = isResumableMessageRecord(msg);
if (options?.metadataOnly) {
messageIds.push(id);
messageKinds.set(id, { isUser, isUserOrAssistant });
messageKinds.set(id, {
isUser,
isResumable,
});
} else {
messagesMap.set(id, msg);
}
@@ -243,6 +276,7 @@ export async function loadConversationRecord(
if (
!firstUserMessageStr &&
isUser &&
isResumable &&
msg.content &&
(Array.isArray(msg.content) ||
typeof msg.content === 'string')
@@ -274,12 +308,14 @@ export async function loadConversationRecord(
if (isMessageRecord(msg)) {
const id = msg.id;
const isUser = msg.type === 'user';
const isUserOrAssistant =
msg.type === 'user' || msg.type === 'gemini';
const isResumable = isResumableMessageRecord(msg);
if (options?.metadataOnly) {
messageIds.push(id);
messageKinds.set(id, { isUser, isUserOrAssistant });
messageKinds.set(id, {
isUser,
isResumable,
});
} else {
messagesMap.set(id, msg);
}
@@ -287,6 +323,7 @@ export async function loadConversationRecord(
if (
!firstUserMessageStr &&
isUser &&
isResumable &&
msg.content &&
(Array.isArray(msg.content) ||
typeof msg.content === 'string')
@@ -314,7 +351,10 @@ export async function loadConversationRecord(
const loadedMessages = Array.from(messagesMap.values());
const metadataFirstUserMessage =
loadedMessages.find((message) => message.type === 'user') ?? null;
loadedMessages.find(
(message) =>
message.type === 'user' && isResumableMessageRecord(message),
) ?? null;
let fallbackFirstUserMessage = firstUserMessageStr;
if (!fallbackFirstUserMessage && metadataFirstUserMessage) {
const rawContent = metadataFirstUserMessage.content;
@@ -329,9 +369,9 @@ export async function loadConversationRecord(
const userMessageCount = options?.metadataOnly
? Array.from(messageKinds.values()).filter((m) => m.isUser).length
: loadedMessages.filter((m) => m.type === 'user').length;
const hasUserOrAssistant = options?.metadataOnly
? Array.from(messageKinds.values()).some((m) => m.isUserOrAssistant)
: loadedMessages.some((m) => m.type === 'user' || m.type === 'gemini');
const hasResumableContent = options?.metadataOnly
? Array.from(messageKinds.values()).some((m) => m.isResumable)
: hasResumableConversationContent(loadedMessages);
return {
sessionId: metadata.sessionId,
@@ -351,7 +391,7 @@ export async function loadConversationRecord(
? memoryScratchpadIsStale
: undefined,
firstUserMessage: fallbackFirstUserMessage,
hasUserOrAssistantMessage: hasUserOrAssistant,
hasResumableContent,
};
} catch (error) {
debugLogger.error('Error loading conversation record from JSONL:', error);
@@ -791,6 +831,23 @@ export class ChatRecordingService {
}
}
/**
* Deletes the current session only if it has no resumable conversation
* content. This removes abandoned startup-only sessions while preserving any
* session with a real user prompt, model response, or tool activity.
*/
async deleteCurrentSessionIfNotResumableAsync(): Promise<void> {
if (!this.conversationFile || !this.cachedConversation) {
return;
}
if (hasResumableConversationContent(this.cachedConversation.messages)) {
return;
}
await this.deleteCurrentSessionAsync();
}
/**
* Rewinds the conversation to the state just before the specified message ID.
* All messages from (and including) the specified ID onwards are removed.
@@ -913,7 +970,7 @@ async function parseLegacyRecordFallback(
messageCount?: number;
userMessageCount?: number;
firstUserMessage?: string;
hasUserOrAssistantMessage?: boolean;
hasResumableContent?: boolean;
})
| null
> {
@@ -929,7 +986,7 @@ async function parseLegacyRecordFallback(
if (options?.metadataOnly) {
let fallbackFirstUserMessageStr: string | undefined;
const firstUserMessage = legacyRecord.messages?.find(
(m) => m.type === 'user',
(m) => m.type === 'user' && isResumableMessageRecord(m),
);
if (firstUserMessage) {
const rawContent = firstUserMessage.content;
@@ -948,20 +1005,18 @@ async function parseLegacyRecordFallback(
userMessageCount:
legacyRecord.messages?.filter((m) => m.type === 'user').length || 0,
firstUserMessage: fallbackFirstUserMessageStr,
hasUserOrAssistantMessage:
legacyRecord.messages?.some(
(m) => m.type === 'user' || m.type === 'gemini',
) || false,
hasResumableContent:
legacyRecord.messages?.some((m) => isResumableMessageRecord(m)) ||
false,
};
}
return {
...legacyRecord,
userMessageCount:
legacyRecord.messages?.filter((m) => m.type === 'user').length || 0,
hasUserOrAssistantMessage:
legacyRecord.messages?.some(
(m) => m.type === 'user' || m.type === 'gemini',
) || false,
hasResumableContent:
legacyRecord.messages?.some((m) => isResumableMessageRecord(m)) ||
false,
};
}
} catch {
+11 -6
View File
@@ -94,6 +94,16 @@ function ensurePartArray(content: PartListUnion): Part[] {
return [content];
}
export function isIgnoredUserContent(trimmedContent: string): boolean {
return (
trimmedContent.length === 0 ||
trimmedContent.startsWith('/') ||
trimmedContent.startsWith('?') ||
trimmedContent.startsWith('<session_context>') ||
trimmedContent.startsWith('<hook_context>')
);
}
/**
* Converts session/conversation data into Gemini client history formats.
*/
@@ -110,12 +120,7 @@ export function convertSessionToClientHistory(
if (msg.type === 'user') {
const contentString = partListUnionToString(msg.content);
const trimmedContent = contentString.trim();
if (
trimmedContent.startsWith('/') ||
trimmedContent.startsWith('?') ||
trimmedContent.startsWith('<session_context>') ||
trimmedContent.startsWith('<hook_context>')
) {
if (isIgnoredUserContent(trimmedContent)) {
continue;
}