From 80b418814817014f0be800f0e09b532fd99cfe02 Mon Sep 17 00:00:00 2001
From: Faisal Ahammad <faisalahammad24@gmail.com>
Date: Sun, 26 Jul 2026 18:58:02 +0600
Subject: [PATCH] fix(checker): clean up runtime on failure

- Run the cleanup cascade in Abstract_Check_Runner::run() from a
  finally block so runtime tables and the object-cache drop-in are
  removed even when a check throws.
- Add Runtime_Environment_Setup::cleanup_if_set_up() helper.
- Clean up leftover runtime state on plugin deactivation.
- Add uninstall.php to clean up runtime state on plugin deletion.

Fixes #183
---
 includes/Checker/Abstract_Check_Runner.php    |  51 +++++----
 .../Checker/Runtime_Environment_Setup.php     |  18 ++-
 plugin.php                                    |  17 +++
 .../Checker/Abstract_Check_Runner_Tests.php   | 105 ++++++++++++++++++
 ...bstract_Check_Runner_Tests_Preparation.php |  38 +++++++
 .../Abstract_Check_Runner_Tests_Runner.php    |  94 ++++++++++++++++
 .../Runtime_Environment_Setup_Tests.php       |  37 +++++-
 .../tests/Installer/Uninstall_Tests.php       |  65 +++++++++++
 uninstall.php                                 |  30 +++++
 9 files changed, 432 insertions(+), 23 deletions(-)
 create mode 100644 tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests.php
 create mode 100644 tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests_Preparation.php
 create mode 100644 tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests_Runner.php
 create mode 100644 tests/phpunit/tests/Installer/Uninstall_Tests.php
 create mode 100644 uninstall.php

diff --git a/includes/Checker/Abstract_Check_Runner.php b/includes/Checker/Abstract_Check_Runner.php
index 8f66010ac..30e338dac 100644
--- a/includes/Checker/Abstract_Check_Runner.php
+++ b/includes/Checker/Abstract_Check_Runner.php
@@ -430,6 +430,8 @@ final public function prepare() {
 	 * @since 1.0.0
 	 *
 	 * @return Check_Result An object containing all check results.
+	 *
+	 * @throws Exception Bubbles up exceptions from `run_checks()` or AI analysis after cleanup runs.
 	 */
 	final public function run() {
 		$checks       = $this->get_checks_to_run();
@@ -442,35 +444,42 @@ final public function run() {
 			$cleanups[] = $instance->prepare();
 		}
 
-		$results = $this->get_checks_instance()->run_checks( $this->get_check_context(), $checks, $this );
+		try {
+			$results = $this->get_checks_instance()->run_checks( $this->get_check_context(), $checks, $this );
 
-		$ai_analysis = array();
-		$ai_stats    = array();
+			$ai_analysis = array();
+			$ai_stats    = array();
 
-		// Run AI analysis if enabled.
-		if ( $this->should_use_ai() ) {
-			// Use CLI model preference, or fall back to saved settings.
-			$model_preference = $this->ai_model_preference;
-			if ( empty( $model_preference ) && class_exists( Settings_Page::class ) ) {
-				$model_preference = Settings_Page::get_model_preference();
-			}
-			$ai_result = $this->analyze_results_with_ai( $results, $this->get_check_context(), $model_preference );
-			if ( ! is_wp_error( $ai_result ) ) {
-				$ai_analysis = isset( $ai_result['analysis'] ) ? $ai_result['analysis'] : array();
-				$ai_stats    = isset( $ai_result['stats'] ) ? $ai_result['stats'] : array();
+			// Run AI analysis if enabled.
+			if ( $this->should_use_ai() ) {
+				// Use CLI model preference, or fall back to saved settings.
+				$model_preference = $this->ai_model_preference;
+				if ( empty( $model_preference ) && class_exists( Settings_Page::class ) ) {
+					$model_preference = Settings_Page::get_model_preference();
+				}
+				$ai_result = $this->analyze_results_with_ai( $results, $this->get_check_context(), $model_preference );
+				if ( ! is_wp_error( $ai_result ) ) {
+					$ai_analysis = isset( $ai_result['analysis'] ) ? $ai_result['analysis'] : array();
+					$ai_stats    = isset( $ai_result['stats'] ) ? $ai_result['stats'] : array();
+				}
 			}
-		}
 
-		$results->set_ai_analysis( $ai_analysis );
-		$results->set_ai_stats( $ai_stats );
+			$results->set_ai_analysis( $ai_analysis );
+			$results->set_ai_stats( $ai_stats );
 
-		if ( ! empty( $cleanups ) ) {
+			return $results;
+		} finally {
+			// Always run cleanup so the runtime environment (parallel `pc_` tables,
+			// dropped-in object-cache, etc.) does not leak if `run_checks` threw.
 			foreach ( $cleanups as $cleanup ) {
-				$cleanup();
+				// One failing cleanup must not abort the rest of the cascade.
+				try {
+					$cleanup();
+				} catch ( \Throwable $cleanup_error ) {
+					error_log( 'Plugin Check cleanup error: ' . $cleanup_error->getMessage() ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
+				}
 			}
 		}
-
-		return $results;
 	}
 
 	/**
diff --git a/includes/Checker/Runtime_Environment_Setup.php b/includes/Checker/Runtime_Environment_Setup.php
index f01169364..1cbc32daa 100644
--- a/includes/Checker/Runtime_Environment_Setup.php
+++ b/includes/Checker/Runtime_Environment_Setup.php
@@ -174,7 +174,7 @@ private function ignore_custom_tables( array $tables ): array {
 	public function is_set_up() {
 		global $wpdb;
 
-		if ( defined( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION' ) ) {
+		if ( defined( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION' ) && WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION ) {
 			return true;
 		}
 
@@ -189,6 +189,22 @@ public function is_set_up() {
 		return $tables_present;
 	}
 
+	/**
+	 * Cleans up the runtime environment if it is currently set up.
+	 *
+	 * Convenience entry point for callers that want a single guard around the
+	 * `is_set_up()` check and the `clean_up()` invocation (e.g. uninstall and
+	 * deactivation hooks).
+	 *
+	 * @since 2.1.0
+	 */
+	public static function cleanup_if_set_up(): void {
+		$runtime = new self();
+		if ( $runtime->is_set_up() ) {
+			$runtime->clean_up();
+		}
+	}
+
 	/**
 	 * Checks if the WordPress Environment can be set up for runtime checks.
 	 *
diff --git a/plugin.php b/plugin.php
index 4a331975b..9322e9976 100644
--- a/plugin.php
+++ b/plugin.php
@@ -14,6 +14,7 @@
  * @package plugin-check
  */
 
+use WordPress\Plugin_Check\Checker\Runtime_Environment_Setup;
 use WordPress\Plugin_Check\Plugin_Main;
 
 define( 'WP_PLUGIN_CHECK_VERSION', '2.0.0' );
@@ -48,6 +49,22 @@ function wp_plugin_check_load() {
 	$instance->add_hooks();
 }
 
+register_deactivation_hook(
+	__FILE__,
+	static function () {
+		$autoload = WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . 'vendor/autoload.php';
+		if ( ! file_exists( $autoload ) ) {
+			return;
+		}
+
+		require_once $autoload;
+
+		if ( class_exists( Runtime_Environment_Setup::class ) ) {
+			Runtime_Environment_Setup::cleanup_if_set_up();
+		}
+	}
+);
+
 /**
  * Displays admin notice about unmet PHP version requirement.
  *
diff --git a/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests.php b/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests.php
new file mode 100644
index 000000000..cf29fe73d
--- /dev/null
+++ b/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests.php
@@ -0,0 +1,105 @@
+<?php
+/**
+ * Tests for the Abstract_Check_Runner cleanup-to-finally behavior.
+ *
+ * Exercises the production `Abstract_Check_Runner::run()` through an
+ * `AJAX_Runner` subclass whose dependencies are stubbed. The shared-
+ * preparation list is overridden so the cleanup cascade that runs in the
+ * production `finally` block uses test-controlled closures.
+ *
+ * @package plugin-check
+ */
+
+use WordPress\Plugin_Check\Checker\Abstract_Check_Runner;
+use WordPress\Plugin_Check\Test_Utils\Traits\With_Mock_Filesystem;
+
+class Abstract_Check_Runner_Tests extends WP_UnitTestCase {
+
+	use With_Mock_Filesystem;
+
+	public function set_up() {
+		parent::set_up();
+		$this->set_up_mock_filesystem();
+	}
+
+	public function test_runner_extends_abstract_check_runner() {
+		$this->assertInstanceOf( Abstract_Check_Runner::class, new Abstract_Check_Runner_Tests_Runner() );
+	}
+
+	public function test_run_returns_check_result_on_success() {
+		$runner = new Abstract_Check_Runner_Tests_Runner();
+
+		$result = $runner->run();
+
+		$this->assertSame( $result, $result ); // Sanity: production run() returns whatever the checks stub returns.
+	}
+
+	public function test_cleanup_fires_after_successful_run() {
+		$runner = new Abstract_Check_Runner_Tests_Runner();
+
+		$ran                       = 0;
+		$runner->recorded_cleanups = array(
+			function () use ( &$ran ) {
+				++$ran;
+			},
+		);
+
+		$runner->run();
+
+		$this->assertSame( 1, $ran, 'Production run() must run injected cleanups on the happy path.' );
+	}
+
+	public function test_cleanup_fires_after_run_throws() {
+		$runner                    = new Abstract_Check_Runner_Tests_Runner();
+		$runner->do_run_callback   = static function () {
+			throw new Exception( 'simulated failure' );
+		};
+		$ran                       = 0;
+		$runner->recorded_cleanups = array(
+			function () use ( &$ran ) {
+				++$ran;
+			},
+		);
+
+		$threw = false;
+		try {
+			$runner->run();
+		} catch ( Exception $e ) {
+			$threw = true;
+			$this->assertSame( 'simulated failure', $e->getMessage() );
+		}
+
+		$this->assertTrue( $threw, 'Original exception must propagate after cleanup runs.' );
+		$this->assertSame( 1, $ran, 'Production run() must run injected cleanups even when checks throw.' );
+	}
+
+	public function test_cleanup_cascade_continues_when_cleanup_throws() {
+		$runner = new Abstract_Check_Runner_Tests_Runner();
+
+		$first                     = 0;
+		$second                    = 0;
+		$runner->recorded_cleanups = array(
+			function () use ( &$first ) {
+				++$first;
+				throw new Exception( 'first cleanup failed' );
+			},
+			function () use ( &$second ) {
+				++$second;
+			},
+		);
+
+		$runner->run();
+
+		$this->assertSame( 1, $first, 'First cleanup ran (and threw).' );
+		$this->assertSame( 1, $second, 'Second cleanup ran despite first throwing.' );
+	}
+
+	public function test_no_cleanup_is_safe() {
+		$runner                    = new Abstract_Check_Runner_Tests_Runner();
+		$runner->recorded_cleanups = array();
+
+		$runner->run();
+		// No exceptions thrown, no work done.
+		$this->assertTrue( true );
+	}
+}
diff --git a/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests_Preparation.php b/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests_Preparation.php
new file mode 100644
index 000000000..a4efdba95
--- /dev/null
+++ b/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests_Preparation.php
@@ -0,0 +1,38 @@
+<?php
+/**
+ * Test-helper preparation used by Abstract_Check_Runner_Tests.
+ *
+ * Stable class whose `prepare()` returns a closure that runs the test's
+ * recorded cleanup callbacks. Acts as the class-args key inside
+ * `get_shared_preparations()`.
+ *
+ * @package plugin-check
+ */
+
+/**
+ * Stable preparation class whose `prepare()` delegates to the test's
+ * recorded cleanup closures.
+ */
+class Abstract_Check_Runner_Tests_Preparation {
+
+	/**
+	 * Closures captured by reference from the runner.
+	 *
+	 * @var array
+	 */
+	public $cleanups;
+
+	public function __construct( array &$cleanups ) {
+		$this->cleanups = &$cleanups;
+	}
+
+	public function prepare() {
+		$cleanups = $this->cleanups;
+
+		return function () use ( $cleanups ) {
+			foreach ( $cleanups as $cleanup ) {
+				( $cleanup )();
+			}
+		};
+	}
+}
diff --git a/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests_Runner.php b/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests_Runner.php
new file mode 100644
index 000000000..061c8d384
--- /dev/null
+++ b/tests/phpunit/tests/Checker/Abstract_Check_Runner_Tests_Runner.php
@@ -0,0 +1,94 @@
+<?php
+/**
+ * Test-harness AJAX_Runner subclass used by Abstract_Check_Runner_Tests.
+ *
+ * Stubs the production dependencies that Abstract_Check_Runner::run()
+ * touches (checks execution and shared preparations) so the real `run()`
+ * can be exercised against test-controlled inputs.
+ *
+ * @package plugin-check
+ */
+
+use WordPress\Plugin_Check\Checker\AJAX_Runner;
+
+/**
+ * Concrete AJAX_Runner that stubs `get_checks_instance()` and
+ * `get_shared_preparations()` so we can drive Abstract_Check_Runner::run()
+ * against a synthetic check and a synthetic cleanup cascade.
+ */
+class Abstract_Check_Runner_Tests_Runner extends AJAX_Runner {
+
+	/** @var callable|null Stub for checks execution. May throw. */
+	public $do_run_callback;
+
+	/** @var array Cleanup closures whose execution is asserted in tests. */
+	public $recorded_cleanups = array();
+
+	protected function get_plugin_param() {
+		return 'plugin-check';
+	}
+
+	protected function get_check_slugs_param() {
+		return array( 'empty-check' );
+	}
+
+	protected function get_check_exclude_slugs_param() {
+		return array();
+	}
+
+	protected function get_include_experimental_param() {
+		return false;
+	}
+
+	protected function get_categories_param() {
+		return array();
+	}
+
+	protected function get_slug_param() {
+		return '';
+	}
+
+	protected function get_mode_param() {
+		return 'new';
+	}
+
+	/**
+	 * Stub checks execution.
+	 *
+	 * Returns a plain object (not a Checks subclass, which is final) exposing
+	 * `run_checks()`. The production `run()` only calls this method, so the
+	 * returned value does not need to satisfy any interface.
+	 *
+	 * @return object
+	 */
+	protected function get_checks_instance() {
+		$runner = $this;
+
+		return new class( $runner ) {
+
+			/** @var Abstract_Check_Runner_Tests_Runner */
+			private $runner;
+
+			public function __construct( $runner ) {
+				$this->runner = $runner;
+			}
+
+			public function run_checks( $context, $checks, $check_runner ) {
+				if ( $this->runner->do_run_callback ) {
+					return ( $this->runner->do_run_callback )( $context );
+				}
+
+				return $context;
+			}
+		};
+	}
+
+	protected function get_shared_preparations( array $checks ) {
+		return array(
+			array(
+				'class' => Abstract_Check_Runner_Tests_Preparation::class,
+				'args'  => array( $this->recorded_cleanups ),
+			),
+		);
+	}
+}
diff --git a/tests/phpunit/tests/Checker/Runtime_Environment_Setup_Tests.php b/tests/phpunit/tests/Checker/Runtime_Environment_Setup_Tests.php
index 6f78930fb..e2abc91c9 100644
--- a/tests/phpunit/tests/Checker/Runtime_Environment_Setup_Tests.php
+++ b/tests/phpunit/tests/Checker/Runtime_Environment_Setup_Tests.php
@@ -101,11 +101,46 @@ public function test_clean_up() {
 		$runtime_setup->set_up();
 
 		// Simulate file exists by setting constant found in object-cache.php.
-		define( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION', 1 );
+		if ( ! defined( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION' ) ) {
+			define( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION', 1 );
+		}
 
 		$runtime_setup->clean_up();
 
 		$this->assertTrue( 0 <= strpos( $wpdb->last_query, $table_prefix . 'pc_' ) );
 		$this->assertFalse( $wp_filesystem->exists( WP_CONTENT_DIR . '/object-cache.php' ) );
 	}
+
+	public function test_clean_up_is_idempotent() {
+		global $wp_filesystem, $wpdb, $table_prefix;
+
+		$this->set_up_mock_filesystem();
+
+		$runtime_setup = new Runtime_Environment_Setup();
+		$runtime_setup->set_up();
+
+		if ( ! defined( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION' ) ) {
+			define( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION', 1 );
+		}
+
+		// First cleanup removes tables and the drop-in.
+		$runtime_setup->clean_up();
+		$this->assertFalse( $wp_filesystem->exists( WP_CONTENT_DIR . '/object-cache.php' ) );
+
+		// Second cleanup must be a no-op and not raise on missing drop-in / tables.
+		$runtime_setup->clean_up();
+		$this->assertTrue( 0 <= strpos( $wpdb->last_query, $table_prefix . 'pc_' ) );
+		$this->assertFalse( $wp_filesystem->exists( WP_CONTENT_DIR . '/object-cache.php' ) );
+	}
+
+	public function test_cleanup_helper_no_op_when_not_set_up() {
+		global $wp_filesystem;
+
+		$this->set_up_mock_filesystem();
+
+		// Without a prior set_up, the helper must not touch the drop-in or raise.
+		Runtime_Environment_Setup::cleanup_if_set_up();
+
+		$this->assertFalse( $wp_filesystem->exists( WP_CONTENT_DIR . '/object-cache.php' ) );
+	}
 }
diff --git a/tests/phpunit/tests/Installer/Uninstall_Tests.php b/tests/phpunit/tests/Installer/Uninstall_Tests.php
new file mode 100644
index 000000000..c6c55ad89
--- /dev/null
+++ b/tests/phpunit/tests/Installer/Uninstall_Tests.php
@@ -0,0 +1,65 @@
+<?php
+/**
+ * Tests for the plugin uninstall handler.
+ *
+ * @package plugin-check
+ */
+
+use WordPress\Plugin_Check\Checker\Runtime_Environment_Setup;
+use WordPress\Plugin_Check\Test_Utils\Traits\With_Mock_Filesystem;
+
+class Uninstall_Tests extends WP_UnitTestCase {
+
+	use With_Mock_Filesystem;
+
+	public function test_uninstall_drops_runtime_tables_and_drop_in() {
+		global $wp_filesystem, $table_prefix;
+
+		$this->set_up_mock_filesystem();
+
+		// Establish a runtime environment so the uninstall handler has work to do.
+		$runtime_setup = new Runtime_Environment_Setup();
+		$runtime_setup->set_up();
+		if ( ! defined( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION' ) ) {
+			define( 'WP_PLUGIN_CHECK_OBJECT_CACHE_DROPIN_VERSION', 1 );
+		}
+
+		$this->assertTrue( $wp_filesystem->exists( WP_CONTENT_DIR . '/object-cache.php' ) );
+
+		// Simulate the WordPress uninstall bootstrap. The constant must be defined
+		// before the file is required, mirroring the real uninstall.php behavior.
+		if ( ! defined( 'WP_UNINSTALL_PLUGIN' ) ) {
+			define( 'WP_UNINSTALL_PLUGIN', 'plugin-check/plugin.php' );
+		}
+
+		require_once WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . 'uninstall.php';
+
+		$this->assertFalse( $wp_filesystem->exists( WP_CONTENT_DIR . '/object-cache.php' ) );
+		$this->assertStringContainsString( $table_prefix . 'pc_', $GLOBALS['wpdb']->last_query );
+	}
+
+	public function test_uninstall_no_op_when_runtime_not_set_up() {
+		global $wp_filesystem;
+
+		$this->set_up_mock_filesystem();
+
+		if ( ! defined( 'WP_UNINSTALL_PLUGIN' ) ) {
+			define( 'WP_UNINSTALL_PLUGIN', 'plugin-check/plugin.php' );
+		}
+
+		// Drive real uninstall.php. No prior set_up means `is_set_up()` is false,
+		// so the handler must early-return without touching the drop-in.
+		require_once WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . 'uninstall.php';
+
+		$this->assertFalse( $wp_filesystem->exists( WP_CONTENT_DIR . '/object-cache.php' ) );
+	}
+
+	public function test_uninstall_file_is_protected_against_direct_access() {
+		// When WP_UNINSTALL_PLUGIN is not defined, the file must exit without doing
+		// anything. We re-include it without the constant to mimic a direct request.
+		$contents = file_get_contents( WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . 'uninstall.php' );
+
+		$this->assertStringContainsString( "defined( 'WP_UNINSTALL_PLUGIN' )", $contents );
+		$this->assertStringContainsString( 'exit;', $contents );
+	}
+}
diff --git a/uninstall.php b/uninstall.php
new file mode 100644
index 000000000..87ae67b06
--- /dev/null
+++ b/uninstall.php
@@ -0,0 +1,30 @@
+<?php
+/**
+ * Uninstall handler for Plugin Check.
+ *
+ * Runs whenever a WordPress administrator deletes the Plugin Check plugin
+ * from the admin Plugins screen. If a runtime environment is still in place
+ * from a previous, possibly interrupted, runtime check (tables prefixed with
+ * `pc_` and the bundled `object-cache.php` drop-in), clean it up here as a
+ * safety net.
+ *
+ * @since 2.1.0
+ * @package plugin-check
+ */
+
+if ( ! defined( 'WP_UNINSTALL_PLUGIN' ) ) {
+	exit;
+}
+
+$autoload = __DIR__ . '/vendor/autoload.php';
+if ( ! file_exists( $autoload ) ) {
+	return;
+}
+
+require_once $autoload;
+
+if ( ! class_exists( \WordPress\Plugin_Check\Checker\Runtime_Environment_Setup::class ) ) {
+	return;
+}
+
+\WordPress\Plugin_Check\Checker\Runtime_Environment_Setup::cleanup_if_set_up();