fix(video): cleanup temp files on extraction failure

[sjhddh]

Description

This PR addresses a potential resource leak in src/memu/utils/video.py.

Currently, if the ffmpeg subprocess fails (e.g., timeout, corrupt file, or missing codec), the RuntimeError is raised, but the temporary directory and files created before the call are left on the disk.

Changes

• Wrapped the extraction logic in a try...except block.
• Added shutil.rmtree(temp_dir) and temp_file.unlink() in the exception handler to ensure artifacts are removed even when errors occur.
• Preserves the original exception re-raising behavior.

Why

To prevent disk clutter and ensure the library behaves like a good citizen, especially in long-running agentic workflows where these errors might accumulate over time.

[Copilot]

Pull request overview

This PR addresses a resource leak in the video frame extraction utilities by adding cleanup logic for temporary files and directories when ffmpeg subprocess operations fail. The implementation adds exception handlers for CalledProcessError and TimeoutExpired to clean up temp resources before re-raising the errors.

Changes:

• Added shutil import for recursive directory removal
• Introduced created_temp_file and created_temp_dir flags to track auto-created temporary resources
• Added cleanup logic in exception handlers to remove temp files/directories on CalledProcessError and TimeoutExpired

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The cleanup logic only handles CalledProcessError and TimeoutExpired exceptions, but there are other exceptions that can be raised within the try block that would leave temp directories uncleaned:

1. RuntimeError raised at line 202-203 when output file is not created
2. ValueError from float() conversion at line 172 (if duration is invalid)

These exceptions will bypass cleanup and leave the temp directory on disk. Consider adding a broader exception handler (e.g., a bare except clause or catching Exception) to ensure cleanup happens for all error paths, while still re-raising the original exception.

Suggested change

	raise RuntimeError(msg) from e
	raise RuntimeError(msg) from e
	except Exception:
	if created_temp_dir and output_dir_obj.exists():
	shutil.rmtree(output_dir_obj)
	logger.exception("Unexpected error during video processing")
	raise

[Copilot]

The created_temp_file flag is set after creating the temp file but before _resolve_output_path is called. If _resolve_output_path raises a ValueError (e.g., if the path starts with '-'), the temp file will be created but the flag won't help cleanup since output_path_obj is not yet defined. Consider moving the flag assignment after output_path_obj is successfully resolved, or add cleanup in a finally block to handle this edge case.

Suggested change

	if output_path is None:
	with tempfile.NamedTemporaryFile(suffix=".jpg", delete=False) as tmp_file:
	output_path = tmp_file.name
	created_temp_file = True
	output_path_obj = VideoFrameExtractor._resolve_output_path(output_path)
	output_path_obj: Path | None = None
	if output_path is None:
	with tempfile.NamedTemporaryFile(suffix=".jpg", delete=False) as tmp_file:
	output_path = tmp_file.name
	created_temp_file = True
	try:
	output_path_obj = VideoFrameExtractor._resolve_output_path(output_path)
	except ValueError:
	# If we created a temporary file and resolving the output path fails,
	# clean up the temporary file to avoid leaking it.
	if created_temp_file and output_path is not None:
	try:
	Path(output_path).unlink()
	except OSError:
	# Best-effort cleanup; ignore errors from unlinking.
	pass
	raise

[Copilot]

The created_temp_dir flag is set after creating the temp directory but before _ensure_safe_cli_path and mkdir are called. If either of these operations raises an exception (e.g., ValueError from _ensure_safe_cli_path if the path starts with '-', or OSError from mkdir), the temp directory will be created but won't be cleaned up since output_dir_obj might not be defined or the exception handling only exists within the try block. Consider moving the flag assignment after output_dir_obj is successfully created, or add cleanup in a finally block to handle this edge case.

Suggested change

	output_dir_obj = VideoFrameExtractor._ensure_safe_cli_path(Path(output_dir))
	output_dir_obj.mkdir(parents=True, exist_ok=True)
	try:
	output_dir_obj = VideoFrameExtractor._ensure_safe_cli_path(Path(output_dir))
	output_dir_obj.mkdir(parents=True, exist_ok=True)
	except Exception:
	if created_temp_dir and output_dir is not None:
	shutil.rmtree(output_dir, ignore_errors=True)
	raise

[Copilot]

The cleanup logic only handles CalledProcessError and TimeoutExpired exceptions, but there are other exceptions that can be raised within the try block that would leave temp files uncleaned:

1. RuntimeError raised at line 100-102 when output file is not created
2. ValueError from float() conversion at line 77 (if duration is invalid)

These exceptions will bypass cleanup and leave the temp file on disk. Consider adding a broader exception handler (e.g., a bare except clause or catching Exception) to ensure cleanup happens for all error paths, while still re-raising the original exception.

[sairin1202]

seems good! thx for contribution