fix(core): fix manual deletion of subagent histories (#22407)

This commit is contained in:
Abhi
2026-03-14 16:09:43 -04:00
committed by GitHub
parent 9f7691fd88
commit c5502b2dc5
2 changed files with 219 additions and 36 deletions

View File

@@ -439,6 +439,7 @@ describe('ChatRecordingService', () => {
describe('deleteSession', () => {
it('should delete the session file, tool outputs, session directory, and logs if they exist', () => {
const sessionId = 'test-session-id';
const shortId = '12345678';
const chatsDir = path.join(testTempDir, 'chats');
const logsDir = path.join(testTempDir, 'logs');
const toolOutputsDir = path.join(testTempDir, 'tool-outputs');
@@ -449,8 +450,12 @@ describe('ChatRecordingService', () => {
fs.mkdirSync(toolOutputsDir, { recursive: true });
fs.mkdirSync(sessionDir, { recursive: true });
const sessionFile = path.join(chatsDir, `${sessionId}.json`);
fs.writeFileSync(sessionFile, '{}');
// Create main session file with timestamp
const sessionFile = path.join(
chatsDir,
`session-2023-01-01T00-00-${shortId}.json`,
);
fs.writeFileSync(sessionFile, JSON.stringify({ sessionId }));
const logFile = path.join(logsDir, `session-${sessionId}.jsonl`);
fs.writeFileSync(logFile, '{}');
@@ -458,7 +463,8 @@ describe('ChatRecordingService', () => {
const toolOutputDir = path.join(toolOutputsDir, `session-${sessionId}`);
fs.mkdirSync(toolOutputDir, { recursive: true });
chatRecordingService.deleteSession(sessionId);
// Call with shortId
chatRecordingService.deleteSession(shortId);
expect(fs.existsSync(sessionFile)).toBe(false);
expect(fs.existsSync(logFile)).toBe(false);
@@ -466,6 +472,93 @@ describe('ChatRecordingService', () => {
expect(fs.existsSync(sessionDir)).toBe(false);
});
it('should delete subagent files and their logs when parent is deleted', () => {
const parentSessionId = '12345678-session-id';
const shortId = '12345678';
const subagentSessionId = 'subagent-session-id';
const chatsDir = path.join(testTempDir, 'chats');
const logsDir = path.join(testTempDir, 'logs');
const toolOutputsDir = path.join(testTempDir, 'tool-outputs');
fs.mkdirSync(chatsDir, { recursive: true });
fs.mkdirSync(logsDir, { recursive: true });
fs.mkdirSync(toolOutputsDir, { recursive: true });
// Create parent session file
const parentFile = path.join(
chatsDir,
`session-2023-01-01T00-00-${shortId}.json`,
);
fs.writeFileSync(
parentFile,
JSON.stringify({ sessionId: parentSessionId }),
);
// Create subagent session file
const subagentFile = path.join(
chatsDir,
`session-2023-01-01T00-01-${shortId}.json`,
);
fs.writeFileSync(
subagentFile,
JSON.stringify({ sessionId: subagentSessionId, kind: 'subagent' }),
);
// Create logs for both
const parentLog = path.join(logsDir, `session-${parentSessionId}.jsonl`);
fs.writeFileSync(parentLog, '{}');
const subagentLog = path.join(
logsDir,
`session-${subagentSessionId}.jsonl`,
);
fs.writeFileSync(subagentLog, '{}');
// Create tool outputs for both
const parentToolOutputDir = path.join(
toolOutputsDir,
`session-${parentSessionId}`,
);
fs.mkdirSync(parentToolOutputDir, { recursive: true });
const subagentToolOutputDir = path.join(
toolOutputsDir,
`session-${subagentSessionId}`,
);
fs.mkdirSync(subagentToolOutputDir, { recursive: true });
// Call with parent sessionId
chatRecordingService.deleteSession(parentSessionId);
expect(fs.existsSync(parentFile)).toBe(false);
expect(fs.existsSync(subagentFile)).toBe(false);
expect(fs.existsSync(parentLog)).toBe(false);
expect(fs.existsSync(subagentLog)).toBe(false);
expect(fs.existsSync(parentToolOutputDir)).toBe(false);
expect(fs.existsSync(subagentToolOutputDir)).toBe(false);
});
it('should delete by basename', () => {
const sessionId = 'test-session-id';
const shortId = '12345678';
const chatsDir = path.join(testTempDir, 'chats');
const logsDir = path.join(testTempDir, 'logs');
fs.mkdirSync(chatsDir, { recursive: true });
fs.mkdirSync(logsDir, { recursive: true });
const basename = `session-2023-01-01T00-00-${shortId}`;
const sessionFile = path.join(chatsDir, `${basename}.json`);
fs.writeFileSync(sessionFile, JSON.stringify({ sessionId }));
const logFile = path.join(logsDir, `session-${sessionId}.jsonl`);
fs.writeFileSync(logFile, '{}');
// Call with basename
chatRecordingService.deleteSession(basename);
expect(fs.existsSync(sessionFile)).toBe(false);
expect(fs.existsSync(logFile)).toBe(false);
});
it('should not throw if session file does not exist', () => {
expect(() =>
chatRecordingService.deleteSession('non-existent'),

View File

@@ -590,46 +590,27 @@ export class ChatRecordingService {
}
/**
* Deletes a session file by session ID.
* Deletes a session file by sessionId, filename, or basename.
* Derives an 8-character shortId to find and delete all associated files
* (parent and subagents).
*
* @throws {Error} If shortId validation fails.
*/
deleteSession(sessionId: string): void {
deleteSession(sessionIdOrBasename: string): void {
try {
const tempDir = this.context.config.storage.getProjectTempDir();
const chatsDir = path.join(tempDir, 'chats');
const sessionPath = path.join(chatsDir, `${sessionId}.json`);
if (fs.existsSync(sessionPath)) {
fs.unlinkSync(sessionPath);
const shortId = this.deriveShortId(sessionIdOrBasename);
if (!fs.existsSync(chatsDir)) {
return; // Nothing to delete
}
// Cleanup Activity logs in the project logs directory
const logsDir = path.join(tempDir, 'logs');
const logPath = path.join(logsDir, `session-${sessionId}.jsonl`);
if (fs.existsSync(logPath)) {
fs.unlinkSync(logPath);
}
const matchingFiles = this.getMatchingSessionFiles(chatsDir, shortId);
// Cleanup tool outputs for this session
const safeSessionId = sanitizeFilenamePart(sessionId);
const toolOutputDir = path.join(
tempDir,
'tool-outputs',
`session-${safeSessionId}`,
);
// Robustness: Ensure the path is strictly within the tool-outputs base
const toolOutputsBase = path.join(tempDir, 'tool-outputs');
if (
fs.existsSync(toolOutputDir) &&
toolOutputDir.startsWith(toolOutputsBase)
) {
fs.rmSync(toolOutputDir, { recursive: true, force: true });
}
// ALSO cleanup the session-specific directory (contains plans, tasks, etc.)
const sessionDir = path.join(tempDir, safeSessionId);
// Robustness: Ensure the path is strictly within the temp root
if (fs.existsSync(sessionDir) && sessionDir.startsWith(tempDir)) {
fs.rmSync(sessionDir, { recursive: true, force: true });
for (const file of matchingFiles) {
this.deleteSessionAndArtifacts(chatsDir, file, tempDir);
}
} catch (error) {
debugLogger.error('Error deleting session file.', error);
@@ -637,6 +618,115 @@ export class ChatRecordingService {
}
}
/**
* Derives an 8-character shortId from a sessionId, filename, or basename.
*/
private deriveShortId(sessionIdOrBasename: string): string {
let shortId = sessionIdOrBasename;
if (sessionIdOrBasename.startsWith(SESSION_FILE_PREFIX)) {
const withoutExt = sessionIdOrBasename.replace('.json', '');
const parts = withoutExt.split('-');
shortId = parts[parts.length - 1];
} else if (sessionIdOrBasename.length >= 8) {
shortId = sessionIdOrBasename.slice(0, 8);
} else {
throw new Error('Invalid sessionId or basename provided for deletion');
}
if (shortId.length !== 8) {
throw new Error('Derived shortId must be exactly 8 characters');
}
return shortId;
}
/**
* Finds all session files matching the pattern session-*-<shortId>.json
*/
private getMatchingSessionFiles(chatsDir: string, shortId: string): string[] {
const files = fs.readdirSync(chatsDir);
return files.filter(
(f) =>
f.startsWith(SESSION_FILE_PREFIX) && f.endsWith(`-${shortId}.json`),
);
}
/**
* Deletes a single session file and its associated logs, tool-outputs, and directory.
*/
private deleteSessionAndArtifacts(
chatsDir: string,
file: string,
tempDir: string,
): void {
const filePath = path.join(chatsDir, file);
try {
const fileContent = fs.readFileSync(filePath, 'utf8');
const content = JSON.parse(fileContent) as unknown;
let fullSessionId: string | undefined;
if (content && typeof content === 'object' && 'sessionId' in content) {
const id = (content as Record<string, unknown>)['sessionId'];
if (typeof id === 'string') {
fullSessionId = id;
}
}
// Delete the session file
fs.unlinkSync(filePath);
if (fullSessionId) {
this.deleteSessionLogs(fullSessionId, tempDir);
this.deleteSessionToolOutputs(fullSessionId, tempDir);
this.deleteSessionDirectory(fullSessionId, tempDir);
}
} catch (error) {
debugLogger.error(`Error deleting associated file ${file}:`, error);
}
}
/**
* Cleans up activity logs for a session.
*/
private deleteSessionLogs(sessionId: string, tempDir: string): void {
const logsDir = path.join(tempDir, 'logs');
const safeSessionId = sanitizeFilenamePart(sessionId);
const logPath = path.join(logsDir, `session-${safeSessionId}.jsonl`);
if (fs.existsSync(logPath) && logPath.startsWith(logsDir)) {
fs.unlinkSync(logPath);
}
}
/**
* Cleans up tool outputs for a session.
*/
private deleteSessionToolOutputs(sessionId: string, tempDir: string): void {
const safeSessionId = sanitizeFilenamePart(sessionId);
const toolOutputDir = path.join(
tempDir,
'tool-outputs',
`session-${safeSessionId}`,
);
const toolOutputsBase = path.join(tempDir, 'tool-outputs');
if (
fs.existsSync(toolOutputDir) &&
toolOutputDir.startsWith(toolOutputsBase)
) {
fs.rmSync(toolOutputDir, { recursive: true, force: true });
}
}
/**
* Cleans up the session-specific directory.
*/
private deleteSessionDirectory(sessionId: string, tempDir: string): void {
const safeSessionId = sanitizeFilenamePart(sessionId);
const sessionDir = path.join(tempDir, safeSessionId);
if (fs.existsSync(sessionDir) && sessionDir.startsWith(tempDir)) {
fs.rmSync(sessionDir, { recursive: true, force: true });
}
}
/**
* Rewinds the conversation to the state just before the specified message ID.
* All messages from (and including) the specified ID onwards are removed.