fix: media usage check fails closed on database errors; refused deletes checked for partial state

MediaUsageFinder treats a failed query (null result or last_error) as a possible usage and an incomplete scan, so media-delete refuses without force. A refused wp_delete_attachment() is write_failed only when the attachment is intact, otherwise partial_write. media-update: re-read hint on write_failed, normalized ids in failures, documented lock resolution and kses on restore. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
2026-09-26 02:54:37 +02:00
parent 931ddeb137
commit 7f136f6f4c
10 changed files with 238 additions and 20 deletions
@@ -102,14 +102,15 @@ final class Errors
     * A write failed and was rolled back completely.
     *
     * @param array<string, mixed> $details Extra error details (e.g. a backup that stays).
+     * @param string|null          $hint    Replaces the default hint (e.g. "re-read before retrying").
     */
-    public static function writeFailed(string $reason, array $details = []): WP_Error
+    public static function writeFailed(string $reason, array $details = [], ?string $hint = null): WP_Error
    {
        return self::make(
            'write_failed',
            trim('Speichern fehlgeschlagen: ' . $reason) . ' Nichts wurde gespeichert.',
-            'Eingaben prüfen, ggf. korrigieren und erneut versuchen; tritt der Fehler wieder auf, den Administrator '
-                . 'informieren (Details im PHP-Error-Log der Website).',
+            $hint ?? 'Eingaben prüfen, ggf. korrigieren und erneut versuchen; tritt der Fehler wieder auf, den '
+                . 'Administrator informieren (Details im PHP-Error-Log der Website).',
            500,
            $details
        );
@@ -14,7 +14,8 @@ use WP_Error;
 * client-safe German text. The AbilityRegistrar logs the detail and returns toError():
 * lit_write_failed when everything was rolled back, lit_partial_write otherwise.
 * $state/$hint customise the partial-write error for writes that are not about a post (see
- * Errors::partialWrite()); $details are added to either error.
+ * Errors::partialWrite()); $details are added to either error. $rolledBackHint replaces the
+ * default hint of the write_failed error.
 */
 final class WriteFailed extends RuntimeException
 {
@@ -28,7 +29,8 @@ final class WriteFailed extends RuntimeException
        string $detail = '',
        public readonly ?string $state = null,
        public readonly ?string $hint = null,
-        public readonly array $details = []
+        public readonly array $details = [],
+        public readonly ?string $rolledBackHint = null
    ) {
        $internal = $detail !== '' ? $detail : ($previous !== null ? $previous->getMessage() : $reason);
        parent::__construct(
@@ -41,7 +43,7 @@ final class WriteFailed extends RuntimeException
    public function toError(): WP_Error
    {
        return $this->rolledBack
-            ? Errors::writeFailed($this->reason, $this->details)
+            ? Errors::writeFailed($this->reason, $this->details, $this->rolledBackHint)
            : Errors::partialWrite(
                $this->reason,
                $this->postId,
@@ -10,6 +10,7 @@ use LIT\MCP\Abilities\FollowUp;
 use LIT\MCP\Abilities\ToolResult;
 use LIT\MCP\ChangeLog\ChangeLog;
 use WP_Error;
+use WP_Post;

 /**
 * lit/media-delete: deletes one attachment after confirmation. WordPress has no media trash by
@@ -123,8 +124,9 @@ final class MediaDeleteAbility implements Ability
            );
        }

+        $file = (string) get_post_meta($id, '_wp_attached_file', true);
        if (! wp_delete_attachment($id, false)) {
-            return Errors::writeFailed(sprintf('WordPress hat das Löschen von Medium %d abgelehnt.', $id));
+            return $this->refused($id, $title, $file);
        }

        $warnings = [];
@@ -145,6 +147,40 @@ final class MediaDeleteAbility implements Ability
            ->toArray();
    }

+    /**
+     * wp_delete_attachment() returned false/null. It deletes meta, terms and the row before the
+     * files, so a failure can leave half a deletion: only an attachment that is still there with
+     * its file reference intact counts as "nothing deleted".
+     */
+    private function refused(int $id, string $title, string $file): WP_Error
+    {
+        $reason = sprintf('WordPress hat das Löschen von Medium %d abgelehnt.', $id);
+        $current = get_post($id);
+        $intact = $current instanceof WP_Post
+            && $current->post_type === 'attachment'
+            && (string) get_post_meta($id, '_wp_attached_file', true) === $file;
+        if ($intact) {
+            return Errors::writeFailed($reason);
+        }
+
+        $this->log->recordPartial(
+            $this->name(),
+            'attachment',
+            (string) $id,
+            sprintf('Medium "%s": Löschen nur teilweise ausgeführt', $title)
+        );
+
+        return Errors::partialWrite(
+            $reason,
+            null,
+            null,
+            sprintf('Medium %d ist möglicherweise teilweise gelöscht (Daten oder Dateien fehlen).', $id),
+            'Den Aufruf nicht einfach wiederholen. Das Medium mit lit/media-get bzw. in der WordPress-Mediathek '
+                . 'prüfen und dem Nutzer Bescheid geben.',
+            ['attachment_id' => $id]
+        );
+    }
+
    /** @param list<array<string, mixed>> $usages */
    private function question(int $id, string $title, bool $trash, array $usages): string
    {
@@ -12,6 +12,10 @@ use WP_Post;
 * Writes the editable metadata of an attachment (alt text, title, caption, description) and
 * restores it from a snapshot taken before (lit/media-update rolls a batch back with it).
 * Every failure is a RuntimeException with an internal message for the PHP error log.
+ *
+ * Both apply() and restore() save through wp_update_post(), which runs the kses filters again
+ * for users without unfiltered_html: a caption or description with markup the current user may
+ * not save comes back filtered on restore (like any save of that user in the media library).
 */
 final class MediaMetaWriter
 {
@@ -10,6 +10,7 @@ use LIT\MCP\Abilities\FollowUp;
 use LIT\MCP\Abilities\ToolResult;
 use LIT\MCP\Abilities\WriteFailed;
 use LIT\MCP\ChangeLog\ChangeLog;
+use LIT\MCP\Integrations\WordPress\PostGuard;
 use Throwable;
 use WP_Error;
 use WP_Post;
@@ -21,7 +22,8 @@ use WP_Post;
 * changed are restored (write_failed) – or, if that fails too, partial_write names them.
 *
 * The `modified` lock compares post_modified_gmt, which WordPress bumps on every write of the
- * posts row (title, caption, description) but not on a pure alt-text (post meta) change.
+ * posts row (title, caption, description) but not on a pure alt-text (post meta) change. Its
+ * resolution is one second: a parallel write within the same second as the read goes unnoticed.
 */
 final class MediaUpdateAbility implements Ability
 {
@@ -274,7 +276,17 @@ final class MediaUpdateAbility implements Ability

        $detail = 'Media update failed: ' . $cause->getMessage();
        if ($unrestored === []) {
-            throw new WriteFailed(self::REASON, true, null, null, $cause, $detail);
+            throw new WriteFailed(
+                self::REASON,
+                true,
+                null,
+                null,
+                $cause,
+                $detail,
+                rolledBackHint: 'Das Zurücksetzen hat modified der Medien geändert: vor einem neuen Versuch die Medien '
+                    . 'neu lesen (lit/media-get oder lit-media-search) und den Stapel mit dem aktuellen modified '
+                    . 'senden; tritt der Fehler wieder auf, den Administrator informieren (Details im PHP-Error-Log).'
+            );
        }

        $ids = [];
@@ -362,11 +374,9 @@ final class MediaUpdateAbility implements Ability
    private static function failure(int $index, array $item, WP_Error $error): array
    {
        $payload = json_decode($error->get_error_message(), true);
-        $id = $item['id'] ?? null;
-
        return [
            'index' => $index,
-            'id' => is_int($id) ? $id : null,
+            'id' => PostGuard::id($item['id'] ?? null),
            'code' => (string) preg_replace('/^lit_/', '', (string) $error->get_error_code()),
            'error' => is_array($payload) ? (string) ($payload['message'] ?? '') : $error->get_error_message(),
        ];
@@ -34,9 +34,10 @@ class MediaUsageFinder
    public const POSSIBLE = 'möglicherweise';
    public const UNREADABLE_TITLE = '(nicht lesbar)';
    public const INCOMPLETE_TITLE = 'Inhaltssuche unvollständig – weitere Verwendungen möglich';
+    public const FAILED_TITLE = 'Verwendungsprüfung fehlgeschlagen – Verwendungen unbekannt';
    public const WHERE = [
        'featured', 'field', 'content', 'site_icon', 'custom_logo', 'divi_logo', 'header_image',
-        'background_image', 'widget',
+        'background_image', 'widget', 'unknown',
    ];

    /** Max. entries whose content is inspected per call. */
@@ -62,6 +63,9 @@ class MediaUsageFinder
    /**
     * @param Closure(): list<string> $mediaMetaKeys Keys of file/image fields (ContentSchema::mediaFieldKeys()).
     */
+    /** Set by results() when a query of the current collect() run failed. */
+    private bool $queryFailed = false;
+
    public function __construct(private Closure $mediaMetaKeys, private ?\wpdb $db = null)
    {
    }
@@ -109,16 +113,21 @@ class MediaUsageFinder

        $found = [];
        $complete = true;
+        $this->queryFailed = false;
        $this->addRows($found, $this->featured($wpdb, $attachmentId, $limit), 'featured');
+        $this->addFailure($found);
        if (count($found) < $limit) {
            $this->addRows($found, $this->fields($wpdb, $attachmentId, $limit), 'field');
+            $this->addFailure($found);
        }
        if (count($found) < $limit) {
            $this->addSiteUsages($found, $attachmentId, $fileRegex);
        }
        if (count($found) < $limit) {
            $complete = $this->addContentUsages($found, $wpdb, $attachmentId, $stem, $fileRegex, $limit);
+            $this->addFailure($found);
        }
+        $complete = $complete && ! $this->queryFailed;

        return [
            'usages' => array_slice(array_values($found), 0, $max),
@@ -127,6 +136,41 @@ class MediaUsageFinder
        ];
    }

+    /**
+     * A failed query may have hidden usages: add one "possible" entry so the attachment never
+     * looks unused (media-delete then needs force, the unused filter skips it).
+     *
+     * @param array<string, array<string, mixed>> $found
+     */
+    private function addFailure(array &$found): void
+    {
+        if ($this->queryFailed && ! isset($found['check:failed'])) {
+            $found['check:failed'] = self::siteUsage(self::FAILED_TITLE, 'check', 'unknown', self::POSSIBLE);
+        }
+    }
+
+    /**
+     * get_results() that notices failures: a null result or a database error marks the
+     * current scan as failed (see addFailure()) instead of passing for "no rows".
+     *
+     * @return list<object>
+     */
+    private function results(\wpdb $wpdb, ?string $sql): array
+    {
+        $wpdb->last_error = '';
+        // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- every caller passes $wpdb->prepare() output.
+        $rows = is_string($sql) && $sql !== '' ? $wpdb->get_results($sql) : null;
+        if (is_array($rows) && (string) $wpdb->last_error === '') {
+            return array_values($rows);
+        }
+
+        $this->queryFailed = true;
+        $message = sprintf('[wp-lit-mcp] media usage query failed: %s', (string) $wpdb->last_error);
+        error_log($message); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
+
+        return [];
+    }
+
    /** @return array<string, mixed> JSON Schema of one entry of usages(). */
    public static function usageSchema(): array
    {
@@ -149,7 +193,7 @@ class MediaUsageFinder
        $statuses = self::EXCLUDED_STATUSES;

        // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- only table names and a constant.
-        return (array) $wpdb->get_results($wpdb->prepare(
+        return $this->results($wpdb, $wpdb->prepare(
            "SELECT p.ID, p.post_title, p.post_type FROM {$wpdb->postmeta} pm
             INNER JOIN {$wpdb->posts} p ON p.ID = pm.post_id
             WHERE pm.meta_key = '_thumbnail_id' AND pm.meta_value = %s
@@ -172,7 +216,7 @@ class MediaUsageFinder
        $statuses = self::EXCLUDED_STATUSES;

        // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber -- $placeholders holds one %s per key.
-        return (array) $wpdb->get_results($wpdb->prepare(
+        return $this->results($wpdb, $wpdb->prepare(
            "SELECT DISTINCT p.ID, p.post_title, p.post_type FROM {$wpdb->postmeta} pm
             INNER JOIN {$wpdb->posts} p ON p.ID = pm.post_id
             WHERE pm.meta_key IN ({$placeholders}) AND pm.meta_value = %s
@@ -340,7 +384,7 @@ class MediaUsageFinder
        $statuses = self::EXCLUDED_STATUSES;

        // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- only placeholders, table names, a constant.
-        return (array) $wpdb->get_results($wpdb->prepare(
+        return $this->results($wpdb, $wpdb->prepare(
            "SELECT p.ID, p.post_title, p.post_type FROM {$wpdb->posts} p
             WHERE p.post_type NOT IN ('revision', 'attachment', 'nav_menu_item')
             AND p.post_status NOT IN ({$statuses})
@@ -359,7 +403,7 @@ class MediaUsageFinder
        $placeholders = implode(', ', array_fill(0, count($ids), '%d'));

        // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare -- $placeholders holds one %d per ID.
-        $rows = (array) $wpdb->get_results($wpdb->prepare(
+        $rows = $this->results($wpdb, $wpdb->prepare(
            "SELECT p.ID, p.post_content FROM {$wpdb->posts} p WHERE p.ID IN ({$placeholders})",
            ...$ids
        ));
@@ -21,6 +21,15 @@ final class WriteFailedTest extends TestCase
        self::assertSame('lit_write_failed', $failure->toError()->get_error_code());
    }

+    public function testRolledBackHintReplacesTheDefaultHintOfWriteFailedOnly(): void
+    {
+        $clean = new WriteFailed('x', true, rolledBackHint: 'Erst neu lesen.');
+        $partial = new WriteFailed('x', false, rolledBackHint: 'Erst neu lesen.');
+
+        self::assertSame('Erst neu lesen.', $clean->toError()->get_error_data()['hint']);
+        self::assertNotSame('Erst neu lesen.', $partial->toError()->get_error_data()['hint']);
+    }
+
    public function testIncompleteRollbackMapsToPartialWriteWithTheIds(): void
    {
        $error = (new WriteFailed('Felder konnten nicht gespeichert werden.', false, 5, 811))->toError();
@@ -23,6 +23,7 @@ final class MediaDeleteAbilityTest extends TestCase
        parent::setUp();
        Functions\when('get_post')->justReturn(new WP_Post(['ID' => 33, 'post_type' => 'attachment', 'post_status' => 'inherit', 'post_title' => 'Foto']));
        Functions\when('current_user_can')->justReturn(true);
+        Functions\when('get_post_meta')->justReturn('2026/09/foto.jpg');
    }

    private function ability(array $usages, ChangeLog $log, bool $trash = false): MediaDeleteAbility
@@ -96,8 +97,9 @@ final class MediaDeleteAbilityTest extends TestCase
        self::assertSame('trashed', $result['changed'][0]['action']);
    }

-    public function testRefusedDeleteIsAWriteFailure(): void
+    public function testRefusedDeleteWithTheAttachmentIntactIsAWriteFailure(): void
    {
+        Functions\when('get_post_meta')->justReturn('2026/09/foto.jpg');
        Functions\expect('wp_delete_attachment')->once()->andReturn(false);

        $result = $this->ability([], Mockery::mock(ChangeLog::class))->execute(['id' => 33, 'confirm' => true]);
@@ -105,6 +107,54 @@ final class MediaDeleteAbilityTest extends TestCase
        self::assertSame('lit_write_failed', $result->get_error_code());
    }

+    public function testRefusedDeleteThatRemovedDataIsAPartialWrite(): void
+    {
+        $meta = '2026/09/foto.jpg';
+        Functions\when('get_post_meta')->alias(static function () use (&$meta): string {
+            return $meta;
+        });
+        Functions\expect('wp_delete_attachment')->once()->andReturnUsing(static function () use (&$meta) {
+            $meta = '';
+
+            return null;
+        });
+        $log = Mockery::mock(ChangeLog::class);
+        $log->expects('recordPartial')->once()->with('lit/media-delete', 'attachment', '33', Mockery::type('string'));
+
+        $result = $this->ability([], $log)->execute(['id' => 33, 'confirm' => true]);
+
+        self::assertSame('lit_partial_write', $result->get_error_code());
+        self::assertSame(33, $result->get_error_data()['attachment_id']);
+    }
+
+    public function testADatabaseErrorInTheUsageCheckRefusesWithoutForce(): void
+    {
+        Functions\when('get_post_meta')->justReturn('2026/09/foto.jpg');
+        Functions\when('get_option')->justReturn(false);
+        Functions\when('get_theme_mod')->justReturn(false);
+        Functions\expect('wp_delete_attachment')->never();
+        /** @var \Mockery\MockInterface&\wpdb $db */
+        $db = Mockery::mock(\wpdb::class)->makePartial();
+        $db->allows('get_results')->andReturn(null);
+        $ability = new MediaDeleteAbility(
+            new MediaUsageFinder(static fn (): array => [], $db),
+            Mockery::mock(ChangeLog::class),
+            static fn (): bool => false
+        );
+
+        $file = (string) tempnam(sys_get_temp_dir(), 'lit-mcp-log');
+        $previous = ini_set('error_log', $file);
+        try {
+            $result = $ability->execute(['id' => 33, 'confirm' => true]);
+        } finally {
+            ini_set('error_log', (string) $previous);
+            unlink($file);
+        }
+
+        self::assertSame('lit_in_use', $result->get_error_code());
+        self::assertSame(MediaUsageFinder::FAILED_TITLE, $result->get_error_data()['usages'][0]['title']);
+    }
+
    public function testChangeLogFailureIsOnlyAWarning(): void
    {
        Functions\expect('wp_delete_attachment')->once()->andReturn(new WP_Post(['ID' => 33]));
@@ -102,17 +102,19 @@ final class MediaUpdateAbilityTest extends TestCase
        $failures = $result->get_error_data()['failures'];
        self::assertSame([1, 2, 3], array_column($failures, 'index'));
        self::assertSame(['not_found', 'not_found', 'validation_failed'], array_column($failures, 'code'));
+        self::assertSame([99, 5, 35], array_column($failures, 'id'));
        self::assertStringContainsString('nicht gefunden', $failures[0]['error']);
        self::assertStringContainsString('Papierkorb', $failures[2]['error']);
        self::assertSame([], $this->postUpdates);
        self::assertSame([], $this->altUpdates);
    }

-    public function testFailuresOfOneKindKeepTheirCode(): void
+    public function testFailuresOfOneKindKeepTheirCodeAndReportTheNormalizedId(): void
    {
-        $result = (new MediaUpdateAbility($this->log()))->execute(['items' => [['id' => 99, 'alt' => 'x']]]);
+        $result = (new MediaUpdateAbility($this->log()))->execute(['items' => [['id' => '99', 'alt' => 'x']]]);

        self::assertSame('lit_not_found', $result->get_error_code());
+        self::assertSame(99, $result->get_error_data()['failures'][0]['id']);
    }

    public function testRejectsEmptyDuplicateAndOverlongItems(): void
@@ -165,6 +167,7 @@ final class MediaUpdateAbilityTest extends TestCase
        } catch (WriteFailed $e) {
            self::assertTrue($e->rolledBack);
            self::assertSame('lit_write_failed', $e->toError()->get_error_code());
+            self::assertStringContainsString('neu lesen', $e->toError()->get_error_data()['hint']);
            self::assertStringContainsString('Datenbankfehler', $e->getMessage());
        }

@@ -316,4 +316,63 @@ final class MediaUsageFinderTest extends TestCase
        self::assertStringNotContainsString('foto', $this->queries[1]);
        self::assertStringNotContainsString("'%/%'", $this->queries[1]);
    }
+
+    /**
+     * A finder whose query $failing (SQL fragment) fails – returns null or sets last_error.
+     */
+    private function failingFinder(string $failing, bool $viaLastError): MediaUsageFinder
+    {
+        Functions\when('get_post_meta')->justReturn('2026/09/foto.jpg');
+        Functions\when('get_option')->justReturn(false);
+        Functions\when('get_theme_mod')->justReturn(false);
+
+        /** @var MockInterface&\wpdb $db */
+        $db = Mockery::mock(\wpdb::class)->makePartial();
+        $db->allows('get_results')->andReturnUsing(static function (string $sql) use ($db, $failing, $viaLastError) {
+            if (! str_contains($sql, $failing)) {
+                return [];
+            }
+            if ($viaLastError) {
+                $db->last_error = 'Lost connection to MySQL server';
+
+                return [];
+            }
+
+            return null;
+        });
+
+        return new MediaUsageFinder(static fn (): array => ['photo'], $db);
+    }
+
+    /** @return mixed Result of $run with error_log() redirected (a PHP internal, not stubbable). */
+    private static function quietly(callable $run): mixed
+    {
+        $file = (string) tempnam(sys_get_temp_dir(), 'lit-mcp-log');
+        $previous = ini_set('error_log', $file);
+        try {
+            return $run();
+        } finally {
+            ini_set('error_log', (string) $previous);
+            unlink($file);
+        }
+    }
+
+    public function testAFailedQueryNeverReportsUnused(): void
+    {
+        foreach (['_thumbnail_id', 'pm.meta_key IN', 'post_content LIKE'] as $failing) {
+            foreach ([true, false] as $viaLastError) {
+                $label = $failing . ($viaLastError ? ' (last_error)' : ' (null)');
+
+                self::assertTrue(self::quietly(fn () => $this->failingFinder($failing, $viaLastError)->isUsed(33)), $label);
+
+                $scan = self::quietly(fn () => $this->failingFinder($failing, $viaLastError)->scan(33));
+                self::assertFalse($scan['complete'], $label);
+                self::assertSame(
+                    [['post_id' => 0, 'title' => MediaUsageFinder::FAILED_TITLE, 'post_type' => 'check', 'where' => 'unknown', 'certainty' => 'möglicherweise']],
+                    $scan['usages'],
+                    $label
+                );
+            }
+        }
+    }
 }