Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 58 additions & 6 deletions packages/cli/src/utils/sessionCleanup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import {
debugLogger,
sanitizeFilenamePart,
Storage,
TOOL_OUTPUT_DIR,
TOOL_OUTPUTS_DIR,
type Config,
} from '@google/gemini-cli-core';
import type { Settings, SessionRetentionSettings } from '../config/settings.js';
Expand Down Expand Up @@ -101,6 +102,19 @@ export async function cleanupExpiredSessions(
} catch {
/* ignore if log doesn't exist */
}

// ALSO cleanup tool outputs for this session
const safeSessionId = sanitizeFilenamePart(sessionId);
const toolOutputDir = path.join(
config.storage.getProjectTempDir(),
TOOL_OUTPUTS_DIR,
`session-${safeSessionId}`,
);
try {
await fs.rm(toolOutputDir, { recursive: true, force: true });
} catch {
/* ignore if doesn't exist */
}
}

if (config.getDebugMode()) {
Expand Down Expand Up @@ -350,7 +364,7 @@ export async function cleanupToolOutputFiles(
const retentionConfig = settings.general.sessionRetention;
const tempDir =
projectTempDir ?? new Storage(process.cwd()).getProjectTempDir();
const toolOutputDir = path.join(tempDir, TOOL_OUTPUT_DIR);
const toolOutputDir = path.join(tempDir, TOOL_OUTPUTS_DIR);

// Check if directory exists
try {
Expand All @@ -360,15 +374,16 @@ export async function cleanupToolOutputFiles(
return result;
}

// Get all files in the tool_output directory
// Get all entries in the tool-outputs directory
const entries = await fs.readdir(toolOutputDir, { withFileTypes: true });
const files = entries.filter((e) => e.isFile());
result.scanned = files.length;
result.scanned = entries.length;

if (files.length === 0) {
if (entries.length === 0) {
return result;
}

const files = entries.filter((e) => e.isFile());

// Get file stats for age-based cleanup (parallel for better performance)
const fileStatsResults = await Promise.all(
files.map(async (file) => {
Expand Down Expand Up @@ -430,6 +445,43 @@ export async function cleanupToolOutputFiles(
}
}

// For now, continue to cleanup individual files in the root tool-outputs dir
// but also scan and cleanup expired session subdirectories.
const subdirs = entries.filter(
(e) => e.isDirectory() && e.name.startsWith('session-'),
);
for (const subdir of subdirs) {
try {
// Security: Validate that the subdirectory name is a safe filename part
// and doesn't attempt path traversal.
if (subdir.name !== sanitizeFilenamePart(subdir.name)) {
debugLogger.debug(
`Skipping unsafe tool-output subdirectory: ${subdir.name}`,
);
continue;
}

const subdirPath = path.join(toolOutputDir, subdir.name);
const stat = await fs.stat(subdirPath);

let shouldDelete = false;
if (retentionConfig.maxAge) {
const maxAgeMs = parseRetentionPeriod(retentionConfig.maxAge);
const cutoffDate = new Date(now.getTime() - maxAgeMs);
if (stat.mtime < cutoffDate) {
shouldDelete = true;
}
}

if (shouldDelete) {
await fs.rm(subdirPath, { recursive: true, force: true });
result.deleted++; // Count as one "unit" of deletion for stats
}
} catch (error) {
debugLogger.debug(`Failed to cleanup subdir ${subdir.name}: ${error}`);
}
}

// Delete the files
for (const fileName of filesToDelete) {
try {
Expand Down
95 changes: 82 additions & 13 deletions packages/cli/src/utils/toolOutputCleanup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import * as os from 'node:os';
import { debugLogger, TOOL_OUTPUT_DIR } from '@google/gemini-cli-core';
import { debugLogger, TOOL_OUTPUTS_DIR } from '@google/gemini-cli-core';
import type { Settings } from '../config/settings.js';
import { cleanupToolOutputFiles } from './sessionCleanup.js';

Expand Down Expand Up @@ -57,7 +57,7 @@ describe('Tool Output Cleanup', () => {
expect(result.deleted).toBe(0);
});

it('should return early when tool_output directory does not exist', async () => {
it('should return early when tool-outputs directory does not exist', async () => {
const settings: Settings = {
general: {
sessionRetention: {
Expand All @@ -67,7 +67,7 @@ describe('Tool Output Cleanup', () => {
},
};

// Don't create the tool_output directory
// Don't create the tool-outputs directory
const result = await cleanupToolOutputFiles(settings, false, testTempDir);

expect(result.disabled).toBe(false);
Expand All @@ -86,8 +86,8 @@ describe('Tool Output Cleanup', () => {
},
};

// Create tool_output directory and files
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUT_DIR);
// Create tool-outputs directory and files
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUTS_DIR);
await fs.mkdir(toolOutputDir, { recursive: true });

const now = Date.now();
Expand Down Expand Up @@ -128,8 +128,8 @@ describe('Tool Output Cleanup', () => {
},
};

// Create tool_output directory and files
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUT_DIR);
// Create tool-outputs directory and files
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUTS_DIR);
await fs.mkdir(toolOutputDir, { recursive: true });

const now = Date.now();
Expand Down Expand Up @@ -174,8 +174,8 @@ describe('Tool Output Cleanup', () => {
},
};

// Create empty tool_output directory
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUT_DIR);
// Create empty tool-outputs directory
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUTS_DIR);
await fs.mkdir(toolOutputDir, { recursive: true });

const result = await cleanupToolOutputFiles(settings, false, testTempDir);
Expand All @@ -197,8 +197,8 @@ describe('Tool Output Cleanup', () => {
},
};

// Create tool_output directory and files
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUT_DIR);
// Create tool-outputs directory and files
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUTS_DIR);
await fs.mkdir(toolOutputDir, { recursive: true });

const now = Date.now();
Expand Down Expand Up @@ -260,8 +260,8 @@ describe('Tool Output Cleanup', () => {
},
};

// Create tool_output directory and an old file
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUT_DIR);
// Create tool-outputs directory and an old file
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUTS_DIR);
await fs.mkdir(toolOutputDir, { recursive: true });

const tenDaysAgo = Date.now() - 10 * 24 * 60 * 60 * 1000;
Expand All @@ -281,5 +281,74 @@ describe('Tool Output Cleanup', () => {

debugSpy.mockRestore();
});

it('should delete expired session subdirectories', async () => {
const settings: Settings = {
general: {
sessionRetention: {
enabled: true,
maxAge: '1d',
},
},
};

const toolOutputDir = path.join(testTempDir, TOOL_OUTPUTS_DIR);
await fs.mkdir(toolOutputDir, { recursive: true });

const now = Date.now();
const tenDaysAgo = now - 10 * 24 * 60 * 60 * 1000;
const oneHourAgo = now - 1 * 60 * 60 * 1000;

const oldSessionDir = path.join(toolOutputDir, 'session-old');
const recentSessionDir = path.join(toolOutputDir, 'session-recent');

await fs.mkdir(oldSessionDir);
await fs.mkdir(recentSessionDir);

// Set modification times
await fs.utimes(oldSessionDir, tenDaysAgo / 1000, tenDaysAgo / 1000);
await fs.utimes(recentSessionDir, oneHourAgo / 1000, oneHourAgo / 1000);

const result = await cleanupToolOutputFiles(settings, false, testTempDir);

expect(result.deleted).toBe(1);
const remainingDirs = await fs.readdir(toolOutputDir);
expect(remainingDirs).toContain('session-recent');
expect(remainingDirs).not.toContain('session-old');
});

it('should skip subdirectories with path traversal characters', async () => {
const settings: Settings = {
general: {
sessionRetention: {
enabled: true,
maxAge: '1d',
},
},
};

const toolOutputDir = path.join(testTempDir, TOOL_OUTPUTS_DIR);
await fs.mkdir(toolOutputDir, { recursive: true });

// Create an unsafe directory name
const unsafeDir = path.join(toolOutputDir, 'session-.._.._danger');
await fs.mkdir(unsafeDir, { recursive: true });

const debugSpy = vi
.spyOn(debugLogger, 'debug')
.mockImplementation(() => {});

await cleanupToolOutputFiles(settings, false, testTempDir);

expect(debugSpy).toHaveBeenCalledWith(
expect.stringContaining('Skipping unsafe tool-output subdirectory'),
);

// Directory should still exist (it was skipped, not deleted)
const entries = await fs.readdir(toolOutputDir);
expect(entries).toContain('session-.._.._danger');

debugSpy.mockRestore();
});
});
});
1 change: 1 addition & 0 deletions packages/core/src/scheduler/tool-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ describe('ToolExecutor', () => {
SHELL_TOOL_NAME,
'call-trunc',
expect.any(String), // temp dir
'test-session-id', // session id from makeFakeConfig
);

expect(fileUtils.formatTruncatedToolOutput).toHaveBeenCalledWith(
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/scheduler/tool-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export class ToolExecutor {
toolName,
callId,
this.config.storage.getProjectTempDir(),
this.config.getSessionId(),
);
outputFile = savedPath;
content = formatTruncatedToolOutput(content, outputFile, lines);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ Line
Line


Output too large. Full output available at: /mock/history/tool-outputs/run_shell_command_deterministic.txt
Output too large. Full output available at: /mock/temp/tool-outputs/session-mock-session/run_shell_command_deterministic.txt
</tool_output_masked>"
`;
4 changes: 2 additions & 2 deletions packages/core/src/services/chatCompressionService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type { BaseLlmClient } from '../core/baseLlmClient.js';
import type { GeminiChat } from '../core/geminiChat.js';
import type { Config } from '../config/config.js';
import * as fileUtils from '../utils/fileUtils.js';
import { TOOL_OUTPUT_DIR } from '../utils/fileUtils.js';
import { TOOL_OUTPUTS_DIR } from '../utils/fileUtils.js';
import { getInitialChatHistory } from '../utils/environmentContext.js';
import * as tokenCalculation from '../utils/tokenCalculation.js';
import { tokenLimit } from '../core/tokenLimits.js';
Expand Down Expand Up @@ -512,7 +512,7 @@ describe('ChatCompressionService', () => {
);

// Verify a file was actually created in the tool_output subdirectory
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUT_DIR);
const toolOutputDir = path.join(testTempDir, TOOL_OUTPUTS_DIR);
const files = fs.readdirSync(toolOutputDir);
expect(files.length).toBeGreaterThan(0);
expect(files[0]).toMatch(/grep_.*\.txt/);
Expand Down
Loading
Loading