fix: atomic Divi check lock, failed-check notice, test fixes

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/Admin/SettingsPage.php

@@ -165,6 +165,10 @@ final class SettingsPage
                 'warning',
                 __('Es läuft bereits eine Divi-Prüfung; bitte in ein paar Minuten erneut versuchen.', 'wp-lit-mcp'),
             ],
+            'divi_check_failed' => [
+                'error',
+                __('Die Divi-Prüfung ist abgebrochen; Details stehen im PHP-Error-Log.', 'wp-lit-mcp'),
+            ],
             'divi_released' => ['success', __('Die Divi-Version ist für LIT MCP freigegeben.', 'wp-lit-mcp')],
             'divi_release_refused' => [
                 'error',

src/Integrations/Divi/Release/DiviCheckRoute.php

@@ -7,6 +7,8 @@ namespace LIT\MCP\Integrations\Divi\Release;
 use Closure;
 use LIT\MCP\Admin\DiviTab;
 use LIT\MCP\Admin\SettingsPage;
+use LIT\MCP\Log;
+use Throwable;
 use WP_REST_Response;
 
 defined('ABSPATH') || exit;
@@ -15,8 +17,12 @@ defined('ABSPATH') || exit;
  * "Prüfung starten" of the Divi tab (spec §6.1): POST /wp-json/lit-mcp/v1/divi-check. A REST request on purpose –
  * not admin-post – so the check runs in the same context as MCP calls (Divi renders its blocks in REST requests,
  * lit-page-render-summary). The form sends the wp_rest nonce as _wpnonce (cookie authentication); only manage_options.
- * One check at a time: the option LOCK (add_option is atomic); a lock older than LOCK_TTL is a dead check's and taken
+ * One check at a time: an atomic "INSERT IGNORE" on the options table (self::LOCK) is the lock – add_option() reads
- * over. Answers 303 back to the tab.
+ * then writes (wp-includes/option.php) and is not atomic, so two simultaneous requests could both "acquire" it. A
+ * lock older than LOCK_TTL is a dead check's, taken over with a conditional UPDATE (compare-and-swap on the value
+ * just read) so only one of several concurrent takers wins. A check that throws still releases the lock (finally)
+ * and redirects with divi_check_failed (logged, never shown to the browser as an unhandled 500). Answers 303 back
+ * to the tab.
  */
 final class DiviCheckRoute
 {
@@ -59,6 +65,9 @@ final class DiviCheckRoute
                 ($this->reports ?? new CheckReportStore())->save($report);
                 ReleaseGate::forget();
                 $notice = 'divi_checked';
+            } catch (Throwable $e) {
+                Log::failure('divi check', $e);
+                $notice = 'divi_check_failed';
             } finally {
                 delete_option(self::LOCK);
             }
@@ -73,16 +82,44 @@ final class DiviCheckRoute
 
     private static function lock(): bool
     {
-        if (add_option(self::LOCK, time(), '', false)) {
+        global $wpdb;
+
+        if (self::insertLock($wpdb, time())) {
             return true;
         }
 
-        $since = (int) get_option(self::LOCK, 0);
+        // get_var(), not get_option(): the row was written with raw SQL, bypassing the options cache.
-        if ($since > 0 && time() - $since < self::LOCK_TTL) {
+        $since = $wpdb->get_var($wpdb->prepare(
+            "SELECT option_value FROM {$wpdb->options} WHERE option_name = %s",
+            self::LOCK
+        ));
+        if ($since === null) {
+            // The lock was released between our failed insert and this read; try once more.
+            return self::insertLock($wpdb, time());
+        }
+        if (time() - (int) $since < self::LOCK_TTL) {
             return false;
         }
-        delete_option(self::LOCK);
 
-        return (bool) add_option(self::LOCK, time(), '', false);
+        $taken = $wpdb->query($wpdb->prepare(
+            "UPDATE {$wpdb->options} SET option_value = %s WHERE option_name = %s AND option_value = %s",
+            (string) time(),
+            self::LOCK,
+            $since
+        ));
+
+        return (int) $taken === 1;
+    }
+
+    /** INSERT IGNORE: 1 row affected = the lock did not exist and is now ours; 0 = someone else holds it. */
+    private static function insertLock(\wpdb $wpdb, int $now): bool
+    {
+        $inserted = $wpdb->query($wpdb->prepare(
+            "INSERT IGNORE INTO {$wpdb->options} (option_name, option_value, autoload) VALUES (%s, %s, 'no')",
+            self::LOCK,
+            (string) $now
+        ));
+
+        return (int) $inserted === 1;
     }
 }

tests/Unit/Admin/DiviTabTest.php

@@ -73,6 +73,7 @@ final class DiviTabTest extends TestCase
     {
         $reports = [null, self::report(false), self::report(true, '5.2.9'), self::report(true, '5.3.0', 'old')];
         foreach ($reports as $report) {
+            $this->redirect = null;
             $releases = Mockery::mock(ReleaseStore::class);
             $releases->expects('add')->never();
 

tests/Unit/Admin/SettingsPageTest.php

@@ -179,6 +179,7 @@ final class SettingsPageTest extends TestCase
         return [
             ['divi_checked', 'success', 'Divi-Prüfung abgeschlossen'],
             ['divi_check_running', 'warning', 'bereits eine Divi-Prüfung'],
+            ['divi_check_failed', 'error', 'PHP-Error-Log'],
             ['divi_released', 'success', 'für LIT MCP freigegeben'],
             ['divi_release_refused', 'error', 'Prüfung erneut ausführen'],
             ['divi_withdrawn', 'success', 'Freigabe zurückgezogen'],

tests/Unit/Integrations/Divi/Release/DiviCheckRouteTest.php

@@ -11,6 +11,7 @@ use LIT\MCP\Integrations\Divi\Release\CompatibilityCheck;
 use LIT\MCP\Integrations\Divi\Release\DiviCheckRoute;
 use LIT\MCP\Tests\Unit\TestCase;
 use Mockery;
+use RuntimeException;
 
 final class DiviCheckRouteTest extends TestCase
 {
@@ -24,6 +25,28 @@ final class DiviCheckRouteTest extends TestCase
         Functions\when('get_current_user_id')->justReturn(1);
     }
 
+    protected function tearDown(): void
+    {
+        unset($GLOBALS['wpdb']);
+        parent::tearDown();
+    }
+
+    /** Mock of the global $wpdb, its INSERT IGNORE returning $inserted rows affected. */
+    private static function wpdb(int $inserted): \wpdb
+    {
+        $db = Mockery::mock(\wpdb::class)->makePartial();
+        $db->expects('query')->once()->with(Mockery::on(static function (string $sql): bool {
+            return str_starts_with(
+                $sql,
+                "INSERT IGNORE INTO wp_options (option_name, option_value, autoload) "
+                    . "VALUES ('lit_mcp_divi_check_lock', '"
+            ) && str_ends_with($sql, "', 'no')");
+        }))->andReturn($inserted);
+        $GLOBALS['wpdb'] = $db;
+
+        return $db;
+    }
+
     public function testRegistersAPostRouteForAdministratorsOnly(): void
     {
         Functions\expect('register_rest_route')->once()->with(
@@ -43,15 +66,12 @@ final class DiviCheckRouteTest extends TestCase
 
     public function testRunsTheCheckStoresTheReportAndRedirectsToTheTab(): void
     {
+        self::wpdb(1);
         $report = new CheckReport('r1', 'd', 1, '5.3.0', '0.6.0', 'k', []);
         $check = Mockery::mock(CompatibilityCheck::class);
         $check->expects('run')->with(1)->andReturn($report);
         $reports = Mockery::mock(CheckReportStore::class);
         $reports->expects('save')->with($report);
-        Functions\expect('add_option')
-            ->once()
-            ->with(DiviCheckRoute::LOCK, Mockery::type('int'), '', false)
-            ->andReturn(true);
         Functions\expect('delete_option')->once()->with(DiviCheckRoute::LOCK);
 
         $response = (new DiviCheckRoute(static fn () => $check, $reports))->handle();
@@ -63,8 +83,8 @@ final class DiviCheckRouteTest extends TestCase
 
     public function testASecondCheckWaitsForTheFirst(): void
     {
-        Functions\when('add_option')->justReturn(false);
+        $db = self::wpdb(0);
-        Functions\when('get_option')->alias(static fn (): int => time());
+        $db->expects('get_var')->once()->andReturn((string) time());
         Functions\expect('delete_option')->never();
         $reports = Mockery::mock(CheckReportStore::class);
         $reports->expects('save')->never();
@@ -76,18 +96,66 @@ final class DiviCheckRouteTest extends TestCase
 
     public function testAStaleLockIsTakenOver(): void
     {
-        Functions\when('add_option')->alias(static function (): bool {
+        $db = self::wpdb(0);
-            static $calls = 0;
+        $db->expects('get_var')->once()->andReturn('1');
-
+        $db->expects('query')->once()->with(Mockery::on(static function (string $sql): bool {
-            return ++$calls > 1;
+            return str_starts_with($sql, "UPDATE wp_options SET option_value = '")
-        });
+                && str_ends_with(
-        Functions\when('get_option')->justReturn(1);
+                    $sql,
-        Functions\expect('delete_option')->twice()->with(DiviCheckRoute::LOCK);
+                    "' WHERE option_name = 'lit_mcp_divi_check_lock' AND option_value = '1'"
+                );
+        }))->andReturn(1);
+        Functions\expect('delete_option')->once()->with(DiviCheckRoute::LOCK);
         $check = Mockery::mock(CompatibilityCheck::class);
         $check->expects('run')->andReturn(new CheckReport('r1', 'd', 1, '5.3.0', '0.6.0', 'k', []));
         $reports = Mockery::mock(CheckReportStore::class);
         $reports->expects('save');
 
-        (new DiviCheckRoute(static fn () => $check, $reports))->handle();
+        $response = (new DiviCheckRoute(static fn () => $check, $reports))->handle();
+
+        self::assertStringContainsString('lit_mcp_notice=divi_checked', $response->get_headers()['Location']);
+    }
+
+    public function testATakeoverLostToAnotherRequestWaitsForIt(): void
+    {
+        $db = self::wpdb(0);
+        $db->expects('get_var')->once()->andReturn('1');
+        $db->expects('query')->once()->with(Mockery::on(static fn (string $sql): bool => str_starts_with(
+            $sql,
+            "UPDATE wp_options SET option_value = '"
+        )))->andReturn(0);
+        Functions\expect('delete_option')->never();
+        $reports = Mockery::mock(CheckReportStore::class);
+        $reports->expects('save')->never();
+
+        $response = (new DiviCheckRoute(static fn () => self::fail('no second check'), $reports))->handle();
+
+        self::assertStringContainsString('lit_mcp_notice=divi_check_running', $response->get_headers()['Location']);
+    }
+
+    public function testTheLockIsReleasedAndTheCheckMarkedFailedWhenItThrows(): void
+    {
+        self::wpdb(1);
+        Functions\expect('delete_option')->once()->with(DiviCheckRoute::LOCK);
+        $check = Mockery::mock(CompatibilityCheck::class);
+        $check->expects('run')->andThrow(new RuntimeException('divi kaputt'));
+        $reports = Mockery::mock(CheckReportStore::class);
+        $reports->expects('save')->never();
+        // error_log() is a PHP internal and cannot be stubbed; redirect it to a temp file instead.
+        $log = (string) tempnam(sys_get_temp_dir(), 'lit-mcp-log');
+        $previous = ini_set('error_log', $log);
+
+        try {
+            $response = (new DiviCheckRoute(static fn () => $check, $reports))->handle();
+        } finally {
+            ini_set('error_log', (string) $previous);
+        }
+
+        self::assertStringContainsString('lit_mcp_notice=divi_check_failed', $response->get_headers()['Location']);
+        self::assertStringContainsString(
+            'divi check failed: RuntimeException: divi kaputt',
+            (string) file_get_contents($log)
+        );
+        unlink($log);
     }
 }