feat: Add function to cleanup video artifacts

backend/src/__tests__/controllers/cleanupController.test.ts

@@ -0,0 +1,79 @@
+import { Request, Response } from 'express';
+import * as fs from 'fs-extra';
+import * as path from 'path';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { cleanupTempFiles } from '../../controllers/cleanupController';
+
+// Mock config/paths to use a temp directory
+vi.mock('../../config/paths', async () => {
+  const path = await import('path');
+  return {
+    VIDEOS_DIR: path.default.join(process.cwd(), 'src', '__tests__', 'temp_cleanup_test_videos_dir')
+  };
+});
+
+import { VIDEOS_DIR } from '../../config/paths';
+
+// Mock storageService to simulate no active downloads
+vi.mock('../../services/storageService', () => ({
+  getDownloadStatus: vi.fn(() => ({ activeDownloads: [] }))
+}));
+
+describe('cleanupController', () => {
+  const req = {} as Request;
+  const res = {
+    status: vi.fn().mockReturnThis(),
+    json: vi.fn()
+  } as unknown as Response;
+
+  beforeEach(async () => {
+    // Ensure test directory exists
+    await fs.ensureDir(VIDEOS_DIR);
+    vi.clearAllMocks();
+  });
+
+  afterEach(async () => {
+    // Clean up test directory
+    if (await fs.pathExists(VIDEOS_DIR)) {
+      await fs.remove(VIDEOS_DIR);
+    }
+  });
+
+  it('should delete directories starting with temp_ recursively', async () => {
+    // Create structure:
+    // videos/
+    //   temp_folder1/ (should be deleted)
+    //     file.txt
+    //   normal_folder/ (should stay)
+    //     temp_nested/ (should be deleted per current recursive logic)
+    //     normal_nested/ (should stay)
+    //   video.mp4 (should stay)
+    //   video.mp4.part (should be deleted)
+
+    const tempFolder1 = path.join(VIDEOS_DIR, 'temp_folder1');
+    const normalFolder = path.join(VIDEOS_DIR, 'normal_folder');
+    const nestedTemp = path.join(normalFolder, 'temp_nested');
+    const nestedNormal = path.join(normalFolder, 'normal_nested');
+    const partFile = path.join(VIDEOS_DIR, 'video.mp4.part');
+    const normalFile = path.join(VIDEOS_DIR, 'video.mp4');
+
+    await fs.ensureDir(tempFolder1);
+    await fs.writeFile(path.join(tempFolder1, 'file.txt'), 'content');
+    
+    await fs.ensureDir(normalFolder);
+    await fs.ensureDir(nestedTemp);
+    await fs.ensureDir(nestedNormal);
+    
+    await fs.ensureFile(partFile);
+    await fs.ensureFile(normalFile);
+
+    await cleanupTempFiles(req, res);
+
+    expect(await fs.pathExists(tempFolder1)).toBe(false);
+    expect(await fs.pathExists(normalFolder)).toBe(true);
+    expect(await fs.pathExists(nestedTemp)).toBe(false);
+    expect(await fs.pathExists(nestedNormal)).toBe(true);
+    expect(await fs.pathExists(partFile)).toBe(false);
+    expect(await fs.pathExists(normalFile)).toBe(true);
+  });
+});

backend/src/__tests__/utils/cleanupVideoArtifacts.test.ts

@@ -0,0 +1,83 @@
+import * as fs from 'fs-extra';
+import * as path from 'path';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { cleanupVideoArtifacts } from '../../utils/downloadUtils';
+
+// Mock path for testing
+const TEST_DIR = path.join(__dirname, 'temp_cleanup_artifacts_test');
+
+vi.mock('../config/paths', () => ({
+  VIDEOS_DIR: TEST_DIR
+}));
+
+describe('cleanupVideoArtifacts', () => {
+  beforeEach(async () => {
+    await fs.ensureDir(TEST_DIR);
+  });
+
+  afterEach(async () => {
+    if (await fs.pathExists(TEST_DIR)) {
+      await fs.remove(TEST_DIR);
+    }
+  });
+
+  it('should remove .part files', async () => {
+    const baseName = 'video_123';
+    const filePath = path.join(TEST_DIR, `${baseName}.mp4.part`);
+    await fs.ensureFile(filePath);
+    
+    await cleanupVideoArtifacts(baseName, TEST_DIR);
+    
+    expect(await fs.pathExists(filePath)).toBe(false);
+  });
+
+  it('should remove .ytdl files', async () => {
+    const baseName = 'video_123';
+    const filePath = path.join(TEST_DIR, `${baseName}.mp4.ytdl`);
+    await fs.ensureFile(filePath);
+    
+    await cleanupVideoArtifacts(baseName, TEST_DIR);
+    
+    expect(await fs.pathExists(filePath)).toBe(false);
+  });
+
+  it('should remove intermediate format files (.f137.mp4)', async () => {
+    const baseName = 'video_123';
+    const filePath = path.join(TEST_DIR, `${baseName}.f137.mp4`);
+    await fs.ensureFile(filePath);
+    
+    await cleanupVideoArtifacts(baseName, TEST_DIR);
+    
+    expect(await fs.pathExists(filePath)).toBe(false);
+  });
+
+  it('should remove partial files with intermediate formats (.f137.mp4.part)', async () => {
+    const baseName = 'video_123';
+    const filePath = path.join(TEST_DIR, `${baseName}.f137.mp4.part`);
+    await fs.ensureFile(filePath);
+    
+    await cleanupVideoArtifacts(baseName, TEST_DIR);
+    
+    expect(await fs.pathExists(filePath)).toBe(false);
+  });
+  
+  it('should remove temp files (.temp.mp4)', async () => {
+      const baseName = 'video_123';
+      const filePath = path.join(TEST_DIR, `${baseName}.temp.mp4`);
+      await fs.ensureFile(filePath);
+      
+      await cleanupVideoArtifacts(baseName, TEST_DIR);
+      
+      expect(await fs.pathExists(filePath)).toBe(false);
+  });
+
+  it('should NOT remove unrelated files', async () => {
+    const baseName = 'video_123';
+    const unrelatedFile = path.join(TEST_DIR, 'video_456.mp4.part');
+    await fs.ensureFile(unrelatedFile);
+    
+    await cleanupVideoArtifacts(baseName, TEST_DIR);
+    
+    expect(await fs.pathExists(unrelatedFile)).toBe(true);
+  });
+});

backend/src/controllers/cleanupController.ts

@@ -35,8 +35,23 @@ export const cleanupTempFiles = async (
         const fullPath = path.join(dir, entry.name);
 
         if (entry.isDirectory()) {
-          // Recursively clean subdirectories
-          await cleanupDirectory(fullPath);
+          // Check for temp_ folder
+          if (entry.name.startsWith("temp_")) {
+            try {
+              await fs.remove(fullPath);
+              deletedCount++;
+              logger.debug(`Deleted temp directory: ${fullPath}`);
+            } catch (error) {
+              const errorMsg = `Failed to delete directory ${fullPath}: ${
+                error instanceof Error ? error.message : String(error)
+              }`;
+              logger.warn(errorMsg);
+              errors.push(errorMsg);
+            }
+          } else {
+            // Recursively clean subdirectories
+            await cleanupDirectory(fullPath);
+          }
         } else if (entry.isFile()) {
           // Check if file has .ytdl or .part extension
           if (entry.name.endsWith(".ytdl") || entry.name.endsWith(".part")) {

backend/src/services/downloaders/BaseDownloader.ts

@@ -3,8 +3,8 @@ import fs from "fs-extra";
 import path from "path";
 import { DownloadCancelledError } from "../../errors/DownloadErrors";
 import {
-  isCancellationError,
-  isDownloadActive,
+    isCancellationError,
+    isDownloadActive,
 } from "../../utils/downloadUtils";
 import { formatVideoFilename } from "../../utils/helpers";
 import { logger } from "../../utils/logger";
@@ -110,14 +110,14 @@ export abstract class BaseDownloader implements IDownloader {
    * @param cleanupFn - Optional cleanup function to call before throwing
    * @throws DownloadCancelledError if error is a cancellation error
    */
-  protected handleCancellationError(
+  protected async handleCancellationError(
     error: unknown,
-    cleanupFn?: () => void
-  ): void {
+    cleanupFn?: () => void | Promise<void>
+  ): Promise<void> {
     if (isCancellationError(error)) {
       logger.info("Download was cancelled");
       if (cleanupFn) {
-        cleanupFn();
+        await cleanupFn();
       }
       throw DownloadCancelledError.create();
     }

backend/src/services/downloaders/BilibiliDownloader.ts

@@ -4,18 +4,18 @@ import path from "path";
 import { IMAGES_DIR, SUBTITLES_DIR, VIDEOS_DIR } from "../../config/paths";
 import { DownloadCancelledError } from "../../errors/DownloadErrors";
 import { bccToVtt } from "../../utils/bccToVtt";
-import { formatBytes } from "../../utils/downloadUtils";
+import { formatBytes, safeRemove } from "../../utils/downloadUtils";
 import {
-  extractBilibiliVideoId,
-  formatVideoFilename,
+    extractBilibiliVideoId,
+    formatVideoFilename,
 } from "../../utils/helpers";
 import { logger } from "../../utils/logger";
 import { ProgressTracker } from "../../utils/progressTracker";
 import {
-  executeYtDlpJson,
-  executeYtDlpSpawn,
-  getNetworkConfigFromUserConfig,
-  getUserYtDlpConfig,
+    executeYtDlpJson,
+    executeYtDlpSpawn,
+    getNetworkConfigFromUserConfig,
+    getUserYtDlpConfig,
 } from "../../utils/ytDlpUtils";
 import * as storageService from "../storageService";
 import { Collection, Video } from "../storageService";
@@ -402,29 +402,25 @@ export class BilibiliDownloader extends BaseDownloader {
       // Use spawn to capture stdout for progress
       const subprocess = executeYtDlpSpawn(url, flags);
 
-      // Register cancel function if provided
       if (onStart) {
-        onStart(() => {
+        onStart(async () => {
           logger.info("Killing subprocess for download:", downloadId);
           subprocess.kill();
 
           // Clean up partial files
           logger.info("Cleaning up partial files...");
-          try {
-            if (fs.existsSync(tempDir)) {
-              fs.removeSync(tempDir);
-              logger.info("Deleted temp directory:", tempDir);
-            }
-            if (fs.existsSync(videoPath)) {
-              fs.unlinkSync(videoPath);
-              logger.info("Deleted partial video file:", videoPath);
-            }
-            if (fs.existsSync(thumbnailPath)) {
-              fs.unlinkSync(thumbnailPath);
-              logger.info("Deleted partial thumbnail file:", thumbnailPath);
-            }
-          } catch (cleanupError) {
-            logger.error("Error cleaning up partial files:", cleanupError);
+          
+          if (fs.existsSync(tempDir)) {
+            await safeRemove(tempDir);
+            logger.info("Deleted temp directory:", tempDir);
+          }
+          if (fs.existsSync(videoPath)) {
+            await safeRemove(videoPath);
+            logger.info("Deleted partial video file:", videoPath);
+          }
+          if (fs.existsSync(thumbnailPath)) {
+            await safeRemove(thumbnailPath);
+            logger.info("Deleted partial thumbnail file:", thumbnailPath);
           }
         });
       }
@@ -469,9 +465,9 @@ export class BilibiliDownloader extends BaseDownloader {
         downloadError = error;
         // Use base class helper for cancellation handling
         const downloader = new BilibiliDownloader();
-        downloader.handleCancellationError(error, () => {
+        downloader.handleCancellationError(error, async () => {
           if (fs.existsSync(tempDir)) {
-            fs.removeSync(tempDir);
+            await safeRemove(tempDir);
           }
         });
         // Only log as error if it's not an expected subtitle-related issue

backend/src/services/downloaders/MissAVDownloader.ts

@@ -4,7 +4,7 @@ import fs from "fs-extra";
 import path from "path";
 import puppeteer from "puppeteer";
 import { DATA_DIR, IMAGES_DIR, VIDEOS_DIR } from "../../config/paths";
-import { cleanupTemporaryFiles } from "../../utils/downloadUtils";
+import { cleanupTemporaryFiles, safeRemove } from "../../utils/downloadUtils";
 import { formatVideoFilename } from "../../utils/helpers";
 import { logger } from "../../utils/logger";
 import { ProgressTracker } from "../../utils/progressTracker";
@@ -326,13 +326,13 @@ export class MissAVDownloader extends BaseDownloader {
           });
 
           if (onStart) {
-            onStart(() => {
+            onStart(async () => {
               logger.info("Killing subprocess for download:", downloadId);
               child.kill();
 
               // Clean up temporary files created by yt-dlp (*.part, *.ytdl, etc.)
               logger.info("Cleaning up temporary files...");
-              cleanupTemporaryFiles(newVideoPath);
+              await cleanupTemporaryFiles(newVideoPath);
             });
           }
         });
@@ -341,8 +341,8 @@ export class MissAVDownloader extends BaseDownloader {
       } catch (err: any) {
         // Use base class helper for cancellation handling
         const downloader = new MissAVDownloader();
-        downloader.handleCancellationError(err, () => {
-          cleanupTemporaryFiles(newVideoPath);
+        await downloader.handleCancellationError(err, async () => {
+          await cleanupTemporaryFiles(newVideoPath);
         });
         logger.error("yt-dlp execution failed:", err);
         throw err;
@@ -353,7 +353,7 @@ export class MissAVDownloader extends BaseDownloader {
       try {
         downloader.throwIfCancelled(downloadId);
       } catch (error) {
-        cleanupTemporaryFiles(newVideoPath);
+        await cleanupTemporaryFiles(newVideoPath);
         throw error;
       }
 
@@ -430,8 +430,8 @@ export class MissAVDownloader extends BaseDownloader {
         IMAGES_DIR,
         `${newSafeBaseFilename}.jpg`
       );
-      if (fs.existsSync(newVideoPath)) fs.removeSync(newVideoPath);
-      if (fs.existsSync(newThumbnailPath)) fs.removeSync(newThumbnailPath);
+      if (fs.existsSync(newVideoPath)) await safeRemove(newVideoPath);
+      if (fs.existsSync(newThumbnailPath)) await safeRemove(newThumbnailPath);
       throw error;
     }
   }

backend/src/services/downloaders/YtDlpDownloader.ts

@@ -3,8 +3,8 @@ import fs from "fs-extra";
 import path from "path";
 import { IMAGES_DIR, SUBTITLES_DIR, VIDEOS_DIR } from "../../config/paths";
 import {
-  cleanupPartialVideoFiles,
   cleanupSubtitleFiles,
+  cleanupVideoArtifacts
 } from "../../utils/downloadUtils";
 import { formatVideoFilename } from "../../utils/helpers";
 import { logger } from "../../utils/logger";
@@ -471,15 +471,19 @@ export class YtDlpDownloader extends BaseDownloader {
       const subprocess = executeYtDlpSpawn(videoUrl, flags);
 
       if (onStart) {
-        onStart(() => {
+        onStart(async () => {
           logger.info("Killing subprocess for download:", downloadId);
           subprocess.kill();
 
           // Clean up partial files
           logger.info("Cleaning up partial files...");
-          cleanupPartialVideoFiles(newVideoPathWithFormat);
-          cleanupPartialVideoFiles(newThumbnailPath);
-          cleanupSubtitleFiles(newSafeBaseFilename);
+          await cleanupVideoArtifacts(newSafeBaseFilename);
+          await cleanupVideoArtifacts(newSafeBaseFilename, IMAGES_DIR); // For thumbnail? Or just use safeRemove for thumbnailPath
+          // Actually cleanupVideoArtifacts defaults to VIDEOS_DIR. Thumbnail is in IMAGES_DIR.
+          if (fs.existsSync(newThumbnailPath)) {
+             await fs.remove(newThumbnailPath);
+          }
+          await cleanupSubtitleFiles(newSafeBaseFilename);
         });
       }
 
@@ -495,9 +499,10 @@ export class YtDlpDownloader extends BaseDownloader {
       } catch (error: any) {
         // Use base class helper for cancellation handling
         const downloader = new YtDlpDownloader();
-        downloader.handleCancellationError(error, () => {
-          cleanupPartialVideoFiles(newVideoPathWithFormat);
-          cleanupSubtitleFiles(newSafeBaseFilename);
+
+        await downloader.handleCancellationError(error, async () => {
+          await cleanupVideoArtifacts(newSafeBaseFilename);
+          await cleanupSubtitleFiles(newSafeBaseFilename);
         });
 
         // Check if error is subtitle-related and video file exists
@@ -534,8 +539,8 @@ export class YtDlpDownloader extends BaseDownloader {
       try {
         downloader.throwIfCancelled(downloadId);
       } catch (error) {
-        cleanupPartialVideoFiles(newVideoPathWithFormat);
-        cleanupSubtitleFiles(newSafeBaseFilename);
+        await cleanupVideoArtifacts(newSafeBaseFilename);
+        await cleanupSubtitleFiles(newSafeBaseFilename);
         throw error;
       }
 
@@ -545,7 +550,7 @@ export class YtDlpDownloader extends BaseDownloader {
       try {
         downloader.throwIfCancelled(downloadId);
       } catch (error) {
-        cleanupSubtitleFiles(newSafeBaseFilename);
+        await cleanupSubtitleFiles(newSafeBaseFilename);
         throw error;
       }
 
@@ -563,7 +568,7 @@ export class YtDlpDownloader extends BaseDownloader {
       try {
         downloader.throwIfCancelled(downloadId);
       } catch (error) {
-        cleanupSubtitleFiles(newSafeBaseFilename);
+        await cleanupSubtitleFiles(newSafeBaseFilename);
         throw error;
       }
 
@@ -584,7 +589,7 @@ export class YtDlpDownloader extends BaseDownloader {
           try {
             downloader.throwIfCancelled(downloadId);
           } catch (error) {
-            cleanupSubtitleFiles(baseFilename);
+            await cleanupSubtitleFiles(baseFilename);
             throw error;
           }
 
@@ -624,7 +629,7 @@ export class YtDlpDownloader extends BaseDownloader {
         }
       } catch (subtitleError) {
         // If it's a cancellation error, re-throw it
-        downloader.handleCancellationError(subtitleError);
+        await downloader.handleCancellationError(subtitleError);
         logger.error("Error processing subtitle files:", subtitleError);
       }
     } catch (error) {

backend/src/utils/downloadUtils.ts

@@ -50,10 +50,10 @@ export function isCancellationError(error: unknown): boolean {
  * @param directory - Optional directory to search in (defaults to VIDEOS_DIR)
  * @returns Array of deleted file paths
  */
-export function cleanupSubtitleFiles(
+export async function cleanupSubtitleFiles(
   baseFilename: string,
   directory: string = VIDEOS_DIR
-): string[] {
+): Promise<string[]> {
   const deletedFiles: string[] = [];
 
   try {
@@ -69,7 +69,7 @@ export function cleanupSubtitleFiles(
     for (const subtitleFile of subtitleFiles) {
       const subtitlePath = path.join(directory, subtitleFile);
       if (fs.existsSync(subtitlePath)) {
-        fs.unlinkSync(subtitlePath);
+        await safeRemove(subtitlePath);
         deletedFiles.push(subtitlePath);
         console.log("Deleted subtitle file:", subtitlePath);
       }
@@ -86,7 +86,7 @@ export function cleanupSubtitleFiles(
  * @param videoPath - The full path to the video file
  * @returns Array of deleted file paths
  */
-export function cleanupTemporaryFiles(videoPath: string): string[] {
+export async function cleanupTemporaryFiles(videoPath: string): Promise<string[]> {
   const deletedFiles: string[] = [];
 
   try {
@@ -114,7 +114,7 @@ export function cleanupTemporaryFiles(videoPath: string): string[] {
     for (const tempFile of tempFiles) {
       const tempFilePath = path.join(videoDir, tempFile);
       if (fs.existsSync(tempFilePath)) {
-        fs.unlinkSync(tempFilePath);
+        await safeRemove(tempFilePath);
         deletedFiles.push(tempFilePath);
         console.log("Deleted temporary file:", tempFilePath);
       }
@@ -122,7 +122,7 @@ export function cleanupTemporaryFiles(videoPath: string): string[] {
 
     // Also check for the main video file if it exists (partial download)
     if (fs.existsSync(videoPath)) {
-      fs.unlinkSync(videoPath);
+      await safeRemove(videoPath);
       deletedFiles.push(videoPath);
       console.log("Deleted partial video file:", videoPath);
     }
@@ -138,20 +138,20 @@ export function cleanupTemporaryFiles(videoPath: string): string[] {
  * @param videoPath - The full path to the video file
  * @returns Array of deleted file paths
  */
-export function cleanupPartialVideoFiles(videoPath: string): string[] {
+export async function cleanupPartialVideoFiles(videoPath: string): Promise<string[]> {
   const deletedFiles: string[] = [];
 
   try {
     const partVideoPath = `${videoPath}.part`;
 
     if (fs.existsSync(partVideoPath)) {
-      fs.unlinkSync(partVideoPath);
+      await safeRemove(partVideoPath);
       deletedFiles.push(partVideoPath);
       console.log("Deleted partial video file:", partVideoPath);
     }
 
     if (fs.existsSync(videoPath)) {
-      fs.unlinkSync(videoPath);
+      await safeRemove(videoPath);
       deletedFiles.push(videoPath);
       console.log("Deleted video file:", videoPath);
     }
@@ -220,3 +220,91 @@ export function calculateDownloadedSize(
   const downloadedBytes = (percentage / 100) * totalBytes;
   return formatBytes(downloadedBytes);
 }
+
+/**
+ * Clean up all artifacts related to a video download (temp files, partials, format streams)
+ * @param baseFilename - The base filename to match (without extension)
+ * @param directory - Optional directory to search in (defaults to VIDEOS_DIR)
+ * @returns Array of deleted file paths
+ */
+export async function cleanupVideoArtifacts(
+  baseFilename: string,
+  directory: string = VIDEOS_DIR
+): Promise<string[]> {
+  const deletedFiles: string[] = [];
+
+  try {
+    if (!fs.existsSync(directory)) {
+      return deletedFiles;
+    }
+
+    const files = fs.readdirSync(directory);
+    
+    // Filter files that start with the base filename and are likely download artifacts
+    const artifactFiles = files.filter((file: string) => {
+      if (!file.startsWith(baseFilename)) return false;
+      
+      // Always cleanup .part and .ytdl files
+      if (file.endsWith(".part") || file.endsWith(".ytdl")) return true;
+      
+      // Cleanup intermediate format files (e.g., .f137.mp4, .f140.m4a, .temp.mp4)
+      // yt-dlp often uses .f[format_id]. in filenames for intermediate streams
+      if (/\.f[0-9]+/.test(file) || /\.temp\./.test(file)) return true;
+      
+      // Cleanup the main video file variants (mp4, mkv, webm, etc) if this is called during cleanup
+      // This matches strictly files that share the base filename
+      const ext = path.extname(file);
+      const fileWithoutExt = path.basename(file, ext);
+      if (fileWithoutExt === baseFilename) return true;
+      
+      return false;
+    });
+
+    for (const artifact of artifactFiles) {
+      const artifactPath = path.join(directory, artifact);
+      if (fs.existsSync(artifactPath)) {
+        await safeRemove(artifactPath);
+        deletedFiles.push(artifactPath);
+        console.log("Deleted artifact file:", artifactPath);
+      }
+    }
+  } catch (error) {
+    console.error("Error cleaning up video artifacts:", error);
+  }
+
+  return deletedFiles;
+}
+
+/**
+ * Safely remove a file or directory with retries
+ * Useful when processes might still be releasing locks
+ */
+export async function safeRemove(
+  pathToRemove: string,
+  retryCount: number = 3,
+  initialDelay: number = 500
+): Promise<void> {
+  // Initial delay to allow processes to release locks
+  if (initialDelay > 0) {
+    await new Promise((resolve) => setTimeout(resolve, initialDelay));
+  }
+
+  for (let i = 0; i < retryCount; i++) {
+    try {
+      if (fs.existsSync(pathToRemove)) {
+        await fs.remove(pathToRemove);
+      }
+      return;
+    } catch (err) {
+      if (i === retryCount - 1) {
+        console.error(
+          `Failed to remove ${pathToRemove} after ${retryCount} attempts:`,
+          err
+        );
+      } else {
+        // Linear backoff
+        await new Promise((resolve) => setTimeout(resolve, 500));
+      }
+    }
+  }
+}