refactor(cli): group subagent trajectory deletion and use native filesystem testing (#22890)

packages/cli/src/utils/sessionCleanup.integration.test.ts

@@ -252,4 +252,154 @@ describe('Session Cleanup Integration', () => {
       await fs.rm(tempDir, { recursive: true, force: true });
     }
   });
+
+  it('should delete subagent files and their artifacts when parent expires', async () => {
+    // Create a temporary directory with test sessions
+    const fs = await import('node:fs/promises');
+    const path = await import('node:path');
+    const os = await import('node:os');
+
+    const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-test-'));
+    const chatsDir = path.join(tempDir, 'chats');
+    const logsDir = path.join(tempDir, 'logs');
+    const toolOutputsDir = path.join(tempDir, 'tool-outputs');
+
+    await fs.mkdir(chatsDir, { recursive: true });
+    await fs.mkdir(logsDir, { recursive: true });
+    await fs.mkdir(toolOutputsDir, { recursive: true });
+
+    const now = new Date();
+    const oldDate = new Date(now.getTime() - 5 * 24 * 60 * 60 * 1000); // 5 days ago
+
+    // The shortId that ties them together
+    const sharedShortId = 'abcdef12';
+
+    const parentSessionId = 'parent-uuid-123';
+    const parentFile = path.join(
+      chatsDir,
+      `${SESSION_FILE_PREFIX}2024-01-01T10-00-00-${sharedShortId}.json`,
+    );
+    await fs.writeFile(
+      parentFile,
+      JSON.stringify({
+        sessionId: parentSessionId,
+        messages: [],
+        startTime: oldDate.toISOString(),
+        lastUpdated: oldDate.toISOString(),
+      }),
+    );
+
+    const subagentSessionId = 'subagent-uuid-456';
+    const subagentFile = path.join(
+      chatsDir,
+      `${SESSION_FILE_PREFIX}2024-01-01T10-05-00-${sharedShortId}.json`,
+    );
+    await fs.writeFile(
+      subagentFile,
+      JSON.stringify({
+        sessionId: subagentSessionId,
+        messages: [],
+        startTime: oldDate.toISOString(),
+        lastUpdated: oldDate.toISOString(),
+      }),
+    );
+
+    const parentLogFile = path.join(
+      logsDir,
+      `session-${parentSessionId}.jsonl`,
+    );
+    await fs.writeFile(parentLogFile, '{"log": "parent"}');
+
+    const parentToolOutputsDir = path.join(
+      toolOutputsDir,
+      `session-${parentSessionId}`,
+    );
+    await fs.mkdir(parentToolOutputsDir, { recursive: true });
+    await fs.writeFile(
+      path.join(parentToolOutputsDir, 'some-output.txt'),
+      'data',
+    );
+
+    const subagentLogFile = path.join(
+      logsDir,
+      `session-${subagentSessionId}.jsonl`,
+    );
+    await fs.writeFile(subagentLogFile, '{"log": "subagent"}');
+
+    const subagentToolOutputsDir = path.join(
+      toolOutputsDir,
+      `session-${subagentSessionId}`,
+    );
+    await fs.mkdir(subagentToolOutputsDir, { recursive: true });
+    await fs.writeFile(
+      path.join(subagentToolOutputsDir, 'some-output.txt'),
+      'data',
+    );
+
+    const currentShortId = 'current1';
+    const currentFile = path.join(
+      chatsDir,
+      `${SESSION_FILE_PREFIX}2025-01-20T10-00-00-${currentShortId}.json`,
+    );
+    await fs.writeFile(
+      currentFile,
+      JSON.stringify({
+        sessionId: 'current-session',
+        messages: [
+          {
+            type: 'user',
+            content: [{ type: 'text', text: 'hello' }],
+            timestamp: now.toISOString(),
+          },
+        ],
+        startTime: now.toISOString(),
+        lastUpdated: now.toISOString(),
+      }),
+    );
+
+    // Configure test
+    const config: Config = {
+      storage: {
+        getProjectTempDir: () => tempDir,
+      },
+      getSessionId: () => 'current-session', // Mock CLI instance ID
+      getDebugMode: () => false,
+      initialize: async () => undefined,
+    } as unknown as Config;
+
+    const settings: Settings = {
+      general: {
+        sessionRetention: {
+          enabled: true,
+          maxAge: '1d', // Expire things older than 1 day
+        },
+      },
+    };
+
+    try {
+      const result = await cleanupExpiredSessions(config, settings);
+
+      // Verify the cleanup result object
+      // It scanned 3 files. It should delete 2 (parent + subagent), and keep 1 (current)
+      expect(result.disabled).toBe(false);
+      expect(result.scanned).toBe(3);
+      expect(result.deleted).toBe(2);
+      expect(result.skipped).toBe(1);
+
+      // Verify on-disk file states
+      const chats = await fs.readdir(chatsDir);
+      expect(chats).toHaveLength(1);
+      expect(chats).toContain(
+        `${SESSION_FILE_PREFIX}2025-01-20T10-00-00-${currentShortId}.json`,
+      ); // Only current is left
+
+      const logs = await fs.readdir(logsDir);
+      expect(logs).toHaveLength(0); // Both parent and subagent logs were deleted
+
+      const tools = await fs.readdir(toolOutputsDir);
+      expect(tools).toHaveLength(0); // Both parent and subagent tool output dirs were deleted
+    } finally {
+      await fs.rm(tempDir, { recursive: true, force: true });
+    }
+  });
 });

packages/cli/src/utils/sessionCleanup.ts

@@ -9,6 +9,7 @@ import * as path from 'node:path';
 import {
   debugLogger,
   sanitizeFilenamePart,
+  SESSION_FILE_PREFIX,
   Storage,
   TOOL_OUTPUTS_DIR,
   type Config,
@@ -26,6 +27,12 @@ const MULTIPLIERS = {
   m: 30 * 24 * 60 * 60 * 1000, // months (30 days) to ms
 };
 
+/**
+ * Matches a trailing hyphen followed by exactly 8 alphanumeric characters before the .json extension.
+ * Example: session-20250110-abcdef12.json -> captures "abcdef12"
+ */
+const SHORT_ID_REGEX = /-([a-zA-Z0-9]{8})\.json$/;
+
 /**
  * Result of session cleanup operation
  */
@@ -37,6 +44,65 @@ export interface CleanupResult {
   failed: number;
 }
 
+/**
+ * Helpers for session cleanup.
+ */
+
+/**
+ * Derives an 8-character shortId from a session filename.
+ */
+function deriveShortIdFromFileName(fileName: string): string | null {
+  if (fileName.startsWith(SESSION_FILE_PREFIX) && fileName.endsWith('.json')) {
+    const match = fileName.match(SHORT_ID_REGEX);
+    return match ? match[1] : null;
+  }
+  return null;
+}
+
+/**
+ * Gets the log path for a session ID.
+ */
+function getSessionLogPath(tempDir: string, safeSessionId: string): string {
+  return path.join(tempDir, 'logs', `session-${safeSessionId}.jsonl`);
+}
+
+/**
+ * Cleans up associated artifacts (logs, tool-outputs, directory) for a session.
+ */
+async function deleteSessionArtifactsAsync(
+  sessionId: string,
+  config: Config,
+): Promise<void> {
+  const tempDir = config.storage.getProjectTempDir();
+
+  // Cleanup logs
+  const logsDir = path.join(tempDir, 'logs');
+  const safeSessionId = sanitizeFilenamePart(sessionId);
+  const logPath = getSessionLogPath(tempDir, safeSessionId);
+  if (logPath.startsWith(logsDir)) {
+    await fs.unlink(logPath).catch(() => {});
+  }
+
+  // Cleanup tool outputs
+  const toolOutputDir = path.join(
+    tempDir,
+    TOOL_OUTPUTS_DIR,
+    `session-${safeSessionId}`,
+  );
+  const toolOutputsBase = path.join(tempDir, TOOL_OUTPUTS_DIR);
+  if (toolOutputDir.startsWith(toolOutputsBase)) {
+    await fs
+      .rm(toolOutputDir, { recursive: true, force: true })
+      .catch(() => {});
+  }
+
+  // Cleanup session directory
+  const sessionDir = path.join(tempDir, safeSessionId);
+  if (safeSessionId && sessionDir.startsWith(tempDir + path.sep)) {
+    await fs.rm(sessionDir, { recursive: true, force: true }).catch(() => {});
+  }
+}
+
 /**
  * Main entry point for session cleanup during CLI startup
  */
@@ -72,7 +138,6 @@ export async function cleanupExpiredSessions(
       return { ...result, disabled: true };
     }
 
-    // Get all session files (including corrupted ones) for this project
     const allFiles = await getAllSessionFiles(chatsDir, config.getSessionId());
     result.scanned = allFiles.length;
 
@@ -86,62 +151,64 @@ export async function cleanupExpiredSessions(
       retentionConfig,
     );
 
+    const processedShortIds = new Set<string>();
+
     // Delete all sessions that need to be deleted
     for (const sessionToDelete of sessionsToDelete) {
       try {
-        const sessionPath = path.join(chatsDir, sessionToDelete.fileName);
+        const shortId = deriveShortIdFromFileName(sessionToDelete.fileName);
-        await fs.unlink(sessionPath);
+
+        if (shortId) {
+          if (processedShortIds.has(shortId)) {
+            continue;
+          }
+          processedShortIds.add(shortId);
+
+          const matchingFiles = allFiles
+            .map((f) => f.fileName)
+            .filter(
+              (f) =>
+                f.startsWith(SESSION_FILE_PREFIX) &&
+                f.endsWith(`-${shortId}.json`),
+            );
+
+          for (const file of matchingFiles) {
+            const filePath = path.join(chatsDir, file);
+            let fullSessionId: string | undefined;
 
-        // ALSO cleanup Activity logs in the project logs directory
-        const sessionId = sessionToDelete.sessionInfo?.id;
-        if (sessionId) {
-          const logsDir = path.join(config.storage.getProjectTempDir(), 'logs');
-          const logPath = path.join(logsDir, `session-${sessionId}.jsonl`);
             try {
-            await fs.unlink(logPath);
+              // Try to read file to get full sessionId
-          } 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 });
+                const fileContent = await fs.readFile(filePath, 'utf8');
+                const content: unknown = JSON.parse(fileContent);
+                if (
+                  content &&
+                  typeof content === 'object' &&
+                  'sessionId' in content
+                ) {
+                  const record = content as Record<string, unknown>;
+                  const id = record['sessionId'];
+                  if (typeof id === 'string') {
+                    fullSessionId = id;
+                  }
+                }
               } catch {
-            /* ignore if doesn't exist */
+                // If read/parse fails, skip getting sessionId, just delete the file
               }
 
-          // ALSO cleanup the session-specific directory (contains plans, tasks, etc.)
+              // Delete the session file
-          const sessionDir = path.join(
+              if (!fullSessionId || fullSessionId !== config.getSessionId()) {
-            config.storage.getProjectTempDir(),
+                await fs.unlink(filePath);
-            sessionId,
-          );
-          try {
-            await fs.rm(sessionDir, { recursive: true, force: true });
-          } catch {
-            /* ignore if doesn't exist */
-          }
-        }
 
-        if (config.getDebugMode()) {
+                if (fullSessionId) {
-          if (sessionToDelete.sessionInfo === null) {
+                  await deleteSessionArtifactsAsync(fullSessionId, config);
-            debugLogger.debug(
-              `Deleted corrupted session file: ${sessionToDelete.fileName}`,
-            );
-          } else {
-            debugLogger.debug(
-              `Deleted expired session: ${sessionToDelete.sessionInfo.id} (${sessionToDelete.sessionInfo.lastUpdated})`,
-            );
-          }
                 }
                 result.deleted++;
+              } else {
+                result.skipped++;
+              }
             } catch (error) {
-        // Ignore ENOENT errors (file already deleted)
+              // Ignore ENOENT (file already deleted)
               if (
                 error instanceof Error &&
                 'code' in error &&
@@ -149,15 +216,45 @@ export async function cleanupExpiredSessions(
               ) {
                 // File already deleted, do nothing.
               } else {
-          // Log error directly to console
+                debugLogger.warn(
+                  `Failed to delete matching file ${file}: ${error instanceof Error ? error.message : 'Unknown error'}`,
+                );
+                result.failed++;
+              }
+            }
+          }
+        } else {
+          // Fallback to old logic
+          const sessionPath = path.join(chatsDir, sessionToDelete.fileName);
+          await fs.unlink(sessionPath);
+
+          const sessionId = sessionToDelete.sessionInfo?.id;
+          if (sessionId) {
+            await deleteSessionArtifactsAsync(sessionId, config);
+          }
+
+          if (config.getDebugMode()) {
+            debugLogger.debug(
+              `Deleted fallback session: ${sessionToDelete.fileName}`,
+            );
+          }
+          result.deleted++;
+        }
+      } catch (error) {
+        // Ignore ENOENT (file already deleted)
+        if (
+          error instanceof Error &&
+          'code' in error &&
+          error.code === 'ENOENT'
+        ) {
+          // File already deleted
+        } else {
           const sessionId =
             sessionToDelete.sessionInfo === null
               ? sessionToDelete.fileName
               : sessionToDelete.sessionInfo.id;
-          const errorMessage =
-            error instanceof Error ? error.message : 'Unknown error';
           debugLogger.warn(
-            `Failed to delete session ${sessionId}: ${errorMessage}`,
+            `Failed to delete session ${sessionId}: ${error instanceof Error ? error.message : 'Unknown error'}`,
           );
           result.failed++;
         }
@@ -182,9 +279,6 @@ export async function cleanupExpiredSessions(
   return result;
 }
 
-/**
- * Identifies sessions that should be deleted (corrupted or expired based on retention policy)
- */
 /**
  * Identifies sessions that should be deleted (corrupted or expired based on retention policy)
  */
@@ -248,14 +342,20 @@ export async function identifySessionsToDelete(
     let shouldDelete = false;
 
     // Age-based retention check
-    if (cutoffDate && new Date(session.lastUpdated) < cutoffDate) {
+    if (cutoffDate) {
+      const lastUpdatedDate = new Date(session.lastUpdated);
+      const isExpired = lastUpdatedDate < cutoffDate;
+      if (isExpired) {
         shouldDelete = true;
       }
+    }
 
     // Count-based retention check (keep only N most recent deletable sessions)
-    if (maxDeletableSessions !== undefined && i >= maxDeletableSessions) {
+    if (maxDeletableSessions !== undefined) {
+      if (i >= maxDeletableSessions) {
         shouldDelete = true;
       }
+    }
 
     if (shouldDelete) {
       sessionsToDelete.push(entry);