fix: lit/divi-design-detach nennt Snapshot und diff auch, wenn die erste Seite nach dem Preset-Schritt scheitert; Tests für Sperre, Markup und fehlende design_rev

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/Integrations/Divi/Design/DiviDesignDetachAbility.php

@@ -10,7 +10,10 @@ use LIT\MCP\Abilities\Ability;
 use LIT\MCP\Abilities\Annotations;
 use LIT\MCP\Abilities\Errors;
 use LIT\MCP\Abilities\ToolResult;
+use LIT\MCP\Abilities\WriteFailed;
 use LIT\MCP\Integrations\Divi\Library\GlobalUsages;
+use LIT\MCP\Integrations\Divi\Release\DiviLocked;
+use LIT\MCP\Log;
 use WP_Error;
 
 defined('ABSPATH') || exit;
@@ -293,6 +296,8 @@ final class DiviDesignDetachAbility implements Ability
      * @param array<string, mixed> $input
      * @param array<string, mixed> $target
      * @return array<string, mixed>|WP_Error
+     * @throws WriteFailed When the preset step fails, or the first page fails without a preset step.
+     * @throws DiviLocked  Likewise.
      */
     private function confirmed(array $input, array $target): array|WP_Error
     {
@@ -310,13 +315,24 @@ final class DiviDesignDetachAbility implements Ability
         }
         $wrotePresets = $presets !== null && $presets['diff'] !== [];
 
-        $pages = $this->pages->apply(
+        try {
-            $this->name(),
+            $pages = $this->pages->apply(
-            $target['query'],
+                $this->name(),
-            $target['rewrite'],
+                $target['query'],
-            $target['post_ids'],
+                $target['rewrite'],
-            $target['change']
+                $target['post_ids'],
-        );
+                $target['change']
+            );
+        } catch (WriteFailed | DiviLocked $e) {
+            // Thrown when the first page fails: the registrar would answer "nothing was saved" – untrue after the
+            // preset step. Without one, its mapping stays as for every Divi write.
+            if (! $wrotePresets) {
+                throw $e;
+            }
+            Log::failure($this->name(), $e);
+
+            return self::afterPresets(self::firstPageError($e), $presets);
+        }
         if ($pages instanceof WP_Error) {
             return $wrotePresets ? self::afterPresets($pages, $presets) : $pages;
         }
@@ -341,6 +357,23 @@ final class DiviDesignDetachAbility implements Ability
             ->toArray();
     }
 
+    /**
+     * The client error for a failure on the first page, mapped like AbilityRegistrar::errorFor(), but a clean rollback
+     * no longer claims that nothing was saved (afterPresets() adds what the preset step changed).
+     */
+    private static function firstPageError(WriteFailed|DiviLocked $e): WP_Error
+    {
+        if ($e instanceof WriteFailed && ! $e->rolledBack) {
+            return $e->toError();
+        }
+        $locked = DiviLocked::in($e);
+        if ($locked !== null) {
+            return $locked->toError();
+        }
+
+        return $e->withDetails([], state: 'Keine Seite wurde gespeichert.')->toError();
+    }
+
     /**
      * A page error after the preset step wrote: the client learns that the nested references are gone already.
      *

tests/Unit/Integrations/Divi/Design/DiviDesignDetachAbilityTest.php

@@ -7,6 +7,7 @@ namespace LIT\MCP\Tests\Unit\Integrations\Divi\Design;
 use Brain\Monkey\Functions;
 use Closure;
 use LIT\MCP\Abilities\Errors;
+use LIT\MCP\Abilities\WriteFailed;
 use LIT\MCP\Integrations\Divi\Design\DesignChange;
 use LIT\MCP\Integrations\Divi\Design\DesignSystemWriter;
 use LIT\MCP\Integrations\Divi\Design\DetachRewrite;
@@ -17,11 +18,15 @@ use LIT\MCP\Integrations\Divi\Design\PresetUsageQuery;
 use LIT\MCP\Integrations\Divi\Design\SnapshotStore;
 use LIT\MCP\Integrations\Divi\Library\GlobalUsages;
 use LIT\MCP\Integrations\Divi\Library\UsageReport;
+use LIT\MCP\Integrations\Divi\Release\DiviLocked;
+use LIT\MCP\Integrations\Divi\Release\ReleaseState;
+use LIT\MCP\Integrations\Divi\Release\ReleaseStatus;
 use LIT\MCP\Tests\Support\FakeChangeLog;
 use LIT\MCP\Tests\Unit\TestCase;
 use Mockery;
 use Mockery\MockInterface;
 use ReflectionProperty;
+use RuntimeException;
 
 final class DiviDesignDetachAbilityTest extends TestCase
 {
@@ -40,25 +45,31 @@ final class DiviDesignDetachAbilityTest extends TestCase
         PageRewriter $pages,
         ?DesignSystemWriter $writer = null,
         ?SnapshotStore $snapshots = null,
-        ?FakeChangeLog $log = null
+        ?FakeChangeLog $log = null,
+        ?UsageReport $report = null,
+        bool $unfiltered = true
     ): DiviDesignDetachAbility {
         $usages = Mockery::mock(GlobalUsages::class);
-        $usages->allows('reportFor')->andReturn(new UsageReport(0, [], 0, 0, [], 0, false));
+        $usages->allows('reportFor')->andReturn($report ?? new UsageReport(0, [], 0, 0, [], 0, false));
 
         return new DiviDesignDetachAbility(
             $writer ?? self::readOnlyWriter(),
             new DesignChange($snapshots ?? Mockery::mock(SnapshotStore::class), $log ?? FakeChangeLog::forbidden()),
             $usages,
             $pages,
-            static fn (): bool => true
+            static fn (): bool => $unfiltered
         );
     }
 
-    /** Reads PresetBookTest::stored(), never saves. */
+    /**
-    private static function readOnlyWriter(): DesignSystemWriter&MockInterface
+     * Reads $stored (default PresetBookTest::stored()), never saves.
+     *
+     * @param array<string, mixed>|null $stored
+     */
+    private static function readOnlyWriter(?array $stored = null): DesignSystemWriter&MockInterface
     {
         $writer = Mockery::mock(DesignSystemWriter::class);
-        $writer->allows('presets')->andReturn(PresetBookTest::stored());
+        $writer->allows('presets')->andReturn($stored ?? PresetBookTest::stored());
         $writer->allows('canUseVisualBuilder')->andReturn(true);
         $writer->expects('savePresets')->never();
 
@@ -307,6 +318,136 @@ final class DiviDesignDetachAbilityTest extends TestCase
         self::assertStringNotContainsString('lit/divi-presets-delete', $result['summary']);
     }
 
+    /** Saves the preset step once (snapshot presets-4, log 7); for page failures after it. */
+    private function abilityAfterThePresetStep(PageRewriter $pages): DiviDesignDetachAbility
+    {
+        $writer = Mockery::mock(DesignSystemWriter::class);
+        $writer->allows('presets')->andReturn(PresetBookTest::stored());
+        $writer->allows('canUseVisualBuilder')->andReturn(true);
+        $writer->expects('savePresets')
+            ->andReturn(PresetBook::fromDivi(PresetBookTest::stored())->withoutNestedRef('gggggggggg')->data());
+        $snapshots = Mockery::mock(SnapshotStore::class);
+        $snapshots->expects('save')->andReturn('presets-4');
+        $snapshots->expects('attachLog');
+
+        return $this->ability($pages, $writer, $snapshots, new FakeChangeLog(7));
+    }
+
+    /** @return array<string, mixed> The confirmed nested call with the current design_rev. */
+    private static function confirmedLesetext(?array $stored = null): array
+    {
+        return [
+            'preset' => self::LESETEXT,
+            'confirm' => true,
+            'design_rev' => PresetBook::fromDivi($stored ?? PresetBookTest::stored())->rev(),
+        ];
+    }
+
+    /** @return array<string, array{RuntimeException, string}> */
+    public static function firstPageFailures(): array
+    {
+        return [
+            'rolled back' => [WriteFailed::rolledBack('Seite 12 nicht gespeichert.'), 'write_failed'],
+            'Divi locked' => [new DiviLocked(new ReleaseStatus(ReleaseState::Untested, '5.16.0')), 'divi_untested'],
+            'Divi locked inside a rollback' => [
+                WriteFailed::rolledBack(
+                    'Seite 12 nicht gespeichert.',
+                    new DiviLocked(new ReleaseStatus(ReleaseState::Untested, '5.16.0'))
+                ),
+                'divi_untested',
+            ],
+            'partial' => [WriteFailed::partial('Seite 12 nicht gespeichert.', postId: 12), 'partial_write'],
+        ];
+    }
+
+    /** @dataProvider firstPageFailures */
+    public function testAFirstPageFailureAfterThePresetStepSaysThePresetsAreChanged(
+        RuntimeException $failure,
+        string $code
+    ): void {
+        $pages = Mockery::mock(PageRewriter::class);
+        $pages->expects('apply')->andThrow($failure);
+        // 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');
+        $previous = ini_set('error_log', $log);
+        try {
+            $result = $this->abilityAfterThePresetStep($pages)->execute(self::confirmedLesetext());
+            $logged = (string) file_get_contents($log);
+        } finally {
+            ini_set('error_log', (string) $previous);
+            unlink($log);
+        }
+
+        $error = self::assertToolError($result, $code);
+        self::assertStringContainsString('lit/divi-design-detach failed', $logged);
+
+        self::assertStringContainsString('bereits entfernt (Snapshot presets-4)', $error['message']);
+        self::assertStringNotContainsString('Nichts wurde gespeichert', $error['message']);
+        self::assertSame('presets-4', $error['snapshot_id']);
+        self::assertSame('dddddddddd', $error['diff'][0]['id']);
+    }
+
+    /** @dataProvider firstPageFailures */
+    public function testAFirstPageFailureWithoutAPresetStepIsThrownAsBefore(RuntimeException $failure): void
+    {
+        $pages = Mockery::mock(PageRewriter::class);
+        $pages->expects('apply')->andThrow($failure);
+
+        $this->expectExceptionObject($failure);
+
+        $this->ability($pages)->execute([
+            'preset' => ['module' => 'button', 'preset' => 'CTA primär'],
+            'confirm' => true,
+        ]);
+    }
+
+    /** @return array<string, array{UsageReport}> */
+    public static function blockingUsages(): array
+    {
+        return [
+            'live page without edit_post' => [new UsageReport(1, [], 0, 1, [12], 0, false)],
+            'too many live pages' => [new UsageReport(600, [], 0, 600, [], 0, true)],
+        ];
+    }
+
+    /**
+     * Ruling P8a: the preset plan carries the page usages, so a blocked live page refuses before the presets.
+     *
+     * @dataProvider blockingUsages
+     */
+    public function testABlockedLivePageRefusesBeforeThePresetsAreWritten(UsageReport $report): void
+    {
+        $ability = $this->ability(self::untouched(), null, null, null, $report);
+
+        $error = self::assertToolError($ability->execute(self::confirmedLesetext()), 'forbidden');
+
+        self::assertStringContainsString('nichts wurde gespeichert', $error['message']);
+    }
+
+    /** Final review I-2: the preset step saves the whole inventory, so HTML/CSS elsewhere needs unfiltered_html. */
+    public function testMarkupInOtherPresetsNeedsUnfilteredHtml(): void
+    {
+        $stored = PresetBookTest::storedWithMarkup();
+        $pages = Mockery::mock(PageRewriter::class);
+        $pages->allows('preview')->andReturn(PageRewriteReportTest::preview(1));
+        $pages->expects('apply')->never();
+        $ability = $this->ability($pages, self::readOnlyWriter($stored), null, null, null, false);
+
+        $dry = self::assertToolError($ability->execute(['preset' => self::LESETEXT]), 'forbidden');
+        $confirmed = self::assertToolError($ability->execute(self::confirmedLesetext($stored)), 'forbidden');
+
+        self::assertStringContainsString('unfiltered_html', $dry['message']);
+        self::assertStringContainsString('unfiltered_html', $confirmed['message']);
+    }
+
+    public function testNestingWithoutADesignRevWritesNeitherPresetsNorPages(): void
+    {
+        $error = $this->ability(self::untouched())->execute(['preset' => self::LESETEXT, 'confirm' => true]);
+
+        self::assertToolError($error, 'validation_failed');
+        self::assertArrayHasKey('design_rev', self::errorFields($error));
+    }
+
     public function testSchemaAndMeta(): void
     {
         $ability = $this->ability(self::untouched());