fix(web-integration): prevent temp file leakage in Playwright tests

[quanru]

Summary

Fixed a bug where midscene-dump-*.json temporary files were not cleaned up after Playwright tests were interrupted or crashed, leading to disk space waste over time.

Problem

The cleanup logic was only implemented in the reporter's onTestEnd callback, which doesn't execute in these scenarios:

• Tests interrupted with Ctrl+C or timeout
• Process crashes or unexpected exits
• Tests fail before completion
• Reporter not configured in playwright.config

This led to temp files accumulating in the system temp directory.

Solution

Implemented a three-layer cleanup mechanism:

Layer 1: Page Close Event

• Clean up temp files when page closes
• Uses pageTempFiles Map to track each page's temp file

Layer 2: Dump Update Cleanup

• When updating dump data, delete the old temp file
• Prevents multiple files per page from accumulating

Layer 3: Process Exit Handlers

• Register cleanup handlers for SIGINT, SIGTERM, exit, beforeExit
• Uses globalTempFiles Set to track all temp files
• Ensures cleanup even on unexpected process termination

Changes

• Modified packages/web-integration/src/playwright/ai-fixture.ts:
  • Added global temp file tracking system
  • Implemented registerCleanupHandlers() for process exit cleanup
  • Enhanced updateDumpAnnotation() to track and clean old files
  • Added cleanup logic in page.on('close') event handler
• Added packages/web-integration/tests/unit-test/playwright-ai-fixture-cleanup.test.ts:
  • Unit tests for cleanup functionality
  • Tests cover various scenarios (page close, multiple updates, etc.)

Verification

Before Fix:

$ ls /tmp/midscene-dump-*.json
midscene-dump-xxx-1.json (416KB)
midscene-dump-xxx-2.json (1.2MB)

After Fix:

$ ls /tmp/midscene-dump-*.json
(no matches found)

Tested with interrupted tests (Ctrl+C and timeout) - all temp files are properly cleaned up.

Test Plan

• Verified cleanup on normal test completion
• Verified cleanup on test interruption (Ctrl+C)
• Verified cleanup on test timeout
• Verified no temp files remain after multiple test runs
• Created unit tests for cleanup logic

🤖 Generated with Claude Code

[quanru]

Test Strategy Update

Removed the unit test file in the second commit because:

1. Unit tests require complex mocking of Playwright and Agent internals, making them brittle and hard to maintain
2. Integration tests are more appropriate for this feature - we need to verify actual file system cleanup
3. Thoroughly verified through manual testing:
   • ✅ Tested with Ctrl+C interruption → 0 remaining temp files
   • ✅ Tested with timeout → 0 remaining temp files
   • ✅ Multiple test cycles → consistent cleanup
   • ✅ Related fixture tests passing (8/8)

The fix has been validated to work correctly in real-world scenarios. The three-layer cleanup mechanism ensures temp files are cleaned up regardless of how tests terminate.

[Copilot]

Pull Request Overview

This PR fixes a resource leak in Playwright tests where temporary dump files were not being cleaned up after test interruptions or crashes. The implementation adds a three-layer cleanup mechanism to ensure temp files are removed under various termination scenarios.

Key Changes:

• Added global temp file tracking with process exit handlers for cleanup on SIGINT, SIGTERM, exit, and beforeExit events
• Implemented page-level cleanup that removes temp files when pages close
• Enhanced dump update logic to delete old temp files when creating new ones

---

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

[Copilot]

Using process.once() for multiple event handlers creates a race condition. If cleanup runs on 'SIGINT', the 'exit' handler will still fire and attempt to clean already-deleted files. Use a flag to track if cleanup has already run, or consolidate to a single exit handler that covers all scenarios.

[Copilot]

Deleting from the Set while iterating over it can cause iteration issues. Collect files to delete first, then clear the Set after the loop completes. Replace the loop body with collection logic and add globalTempFiles.clear() after the loop.

Suggested change

	for (const filePath of globalTempFiles) {
	try {
	rmSync(filePath, { force: true });
	globalTempFiles.delete(filePath);
	} catch (error) {
	// Silently ignore errors during cleanup
	debugPage(`Failed to clean up temp file: ${filePath}`, error);
	}
	}
	const filesToDelete = Array.from(globalTempFiles);
	for (const filePath of filesToDelete) {
	try {
	rmSync(filePath, { force: true });
	} catch (error) {
	// Silently ignore errors during cleanup
	debugPage(`Failed to clean up temp file: ${filePath}`, error);
	}
	}
	globalTempFiles.clear();

[Copilot]

[nitpick] The timestamp in the filename is unnecessary since the old file is immediately deleted. This makes the filename unpredictable and harder to debug. Consider using a consistent filename pattern like midscene-dump-${test.testId || uuid()}-${pageId}.json to make file tracking more straightforward.

Suggested change

	const tempFileName = `midscene-dump-${test.testId || uuid()}-${Date.now()}.json`;
	const tempFileName = `midscene-dump-${test.testId || uuid()}-${pageId}.json`;