fix: typed write failures, template rollback, parent and term cleanup

WriteFailed maps to lit_write_failed / lit_partial_write (partial writes are logged with result partial); old template read before the update; row marker set before wp_update_post; created terms deleted on rollback; parent must be a readable non-descendant of a hierarchical type; featured image must be readable; snapshot keeps empty values; meta comparison after sanitize_meta; follow-up failures (listener, change log) only add warnings. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
2026-09-25 23:11:18 +02:00
parent 4f9a47fe32
commit a9b2f2db71
18 changed files with 934 additions and 130 deletions
@@ -6,12 +6,13 @@ namespace LIT\MCP\Integrations\WordPress\Content;

 use LIT\MCP\Abilities\Ability;
 use LIT\MCP\Abilities\Errors;
+use LIT\MCP\Abilities\FollowUp;
 use LIT\MCP\Abilities\Rev;
 use LIT\MCP\Abilities\ToolResult;
+use LIT\MCP\Abilities\WriteFailed;
 use LIT\MCP\ChangeLog\ChangeLog;
 use LIT\MCP\Integrations\WordPress\PostGuard;
 use LIT\MCP\Integrations\WordPress\Schema\ContentSchema;
-use RuntimeException;
 use Throwable;
 use WP_Error;
 use WP_Post;
@@ -19,11 +20,15 @@ use WP_Post;
 /**
 * lit/content-create: new entry, always a draft. All or nothing (spec §8.2): input is
 * validated before the insert; if fields, terms or the featured image cannot be written
- * afterwards, the brand-new draft is deleted again and the error is rethrown (the
- * AbilityRegistrar logs it and reports lit_internal_error).
+ * afterwards, created terms and the brand-new draft are deleted again and a WriteFailed
+ * is thrown (AbilityRegistrar → lit_write_failed, or lit_partial_write if that cleanup
+ * failed or the insert itself threw; partial writes are logged with result "partial").
 */
 final class ContentCreateAbility implements Ability
 {
+    private const REASON_INSERT = 'WordPress hat beim Anlegen des Eintrags einen Fehler gemeldet.';
+    private const REASON_EXTRAS = 'Felder, Begriffe oder Beitragsbild konnten nicht gespeichert werden.';
+
    public function __construct(
        private ContentSchema $schema,
        private PostWriter $writer,
@@ -101,7 +106,14 @@ final class ContentCreateAbility implements Ability
            'post_status' => 'draft',
            'post_author' => get_current_user_id(),
        ]);
-        $postId = wp_insert_post(wp_slash($data), true);
+        try {
+            $postId = wp_insert_post(wp_slash($data), true);
+        } catch (Throwable $e) {
+            // A hook threw – the row may or may not exist and its ID is unknown; nothing is
+            // searched or deleted, the client is told to check (lit_partial_write without id).
+            $this->logPartial($postType, '', 'Anlegen abgebrochen; Entwurf möglicherweise vorhanden');
+            throw new WriteFailed(self::REASON_INSERT, false, null, null, $e);
+        }
        if ($postId instanceof WP_Error) {
            return Errors::invalidInput('WordPress hat den Eintrag abgelehnt: ' . $postId->get_error_message());
        }
@@ -110,42 +122,94 @@ final class ContentCreateAbility implements Ability
        try {
            $this->writer->writeExtras($postId, $postType, $prepared, $input);
        } catch (Throwable $e) {
-            $this->discard($postId, $e);
+            $this->discard($postId, $postType, $e);
        }
-        do_action('lit_mcp_after_save', $postId, $this->name(), $input);
+
+        return $this->result($postId, $postType, $input, $prepared['warnings']);
+    }
+
+    /**
+     * @param array<string, mixed> $input
+     * @param list<string>         $warnings
+     * @return array<string, mixed>
+     */
+    private function result(int $postId, string $postType, array $input, array $warnings): array
+    {
+        // The draft is saved – failures from here on are logged and reported as warnings only.
+        FollowUp::run(
+            fn () => do_action('lit_mcp_after_save', $postId, $this->name(), $input),
+            $this->name() . ' lit_mcp_after_save',
+            'Eine Erweiterung ist nach dem Speichern auf einen Fehler gestoßen; der Entwurf ist gespeichert.',
+            $warnings
+        );

        $post = get_post($postId);
        $title = $post instanceof WP_Post ? (string) $post->post_title : (string) $input['title'];
        $summary = sprintf('Entwurf "%s" angelegt', $title);
-        $logId = $this->log->record($this->name(), $postType, (string) $postId, $summary);
+        $logId = FollowUp::run(
+            fn () => $this->log->record($this->name(), $postType, (string) $postId, $summary),
+            $this->name() . ' change log',
+            'Änderungsprotokoll konnte nicht geschrieben werden; der Entwurf ist gespeichert.',
+            $warnings
+        );

        return ToolResult::ok(sprintf('"%s" als Entwurf angelegt (ID %d).', $title, $postId))
            ->changed((string) $postId, $postType, 'created', $title)
            ->rev($post instanceof WP_Post ? Rev::ofPost($post) : null)
-            ->logId($logId)
+            ->logId(is_int($logId) ? $logId : null)
            ->urls((string) get_preview_post_link($postId), (string) get_edit_post_link($postId, 'raw'))
-            ->warnings($prepared['warnings'])
+            ->warnings($warnings)
            ->toArray();
    }

    /**
-     * Deletes the draft this call just created (it cannot have been used anywhere yet).
+     * Rolls the brand-new draft back: terms it created, then the draft itself (force –
+     * it cannot have been used anywhere yet).
     *
-     * @throws RuntimeException Always – the original failure, noting whether the draft is gone.
+     * @throws WriteFailed Always.
     */
-    private function discard(int $postId, Throwable $cause): never
+    private function discard(int $postId, string $postType, Throwable $cause): never
    {
-        $deleted = wp_delete_post($postId, true);
+        $problems = [];
+        try {
+            $this->writer->deleteCreatedTerms();
+        } catch (Throwable $e) {
+            $problems[] = $e->getMessage();
+        }

-        throw new RuntimeException(
-            sprintf(
-                'Extras of new post %d could not be written (%s); draft %s.',
-                $postId,
-                $cause->getMessage(),
-                $deleted ? 'deleted again' : 'could NOT be deleted'
-            ),
-            0,
-            $cause
+        try {
+            if (! wp_delete_post($postId, true)) {
+                $problems[] = sprintf('draft %d could not be deleted', $postId);
+            }
+        } catch (Throwable $e) {
+            $problems[] = sprintf('draft %d could not be deleted: %s', $postId, $e->getMessage());
+        }
+
+        $detail = sprintf('Extras of new post %d could not be written: %s', $postId, $cause->getMessage());
+        if ($problems === []) {
+            throw new WriteFailed(self::REASON_EXTRAS, true, null, null, $cause, $detail);
+        }
+
+        $summary = sprintf('Entwurf %d: Anlegen nur teilweise zurückgenommen', $postId);
+        $this->logPartial($postType, (string) $postId, $summary);
+        throw new WriteFailed(
+            self::REASON_EXTRAS,
+            false,
+            $postId,
+            null,
+            $cause,
+            $detail . '; rollback: ' . implode('; ', $problems)
+        );
+    }
+
+    private function logPartial(string $postType, string $postId, string $summary): void
+    {
+        $ignored = [];
+        FollowUp::run(
+            fn () => $this->log->record($this->name(), $postType, $postId, $summary, null, null, 'partial'),
+            $this->name() . ' change log',
+            '',
+            $ignored
        );
    }
 }
@@ -6,8 +6,10 @@ namespace LIT\MCP\Integrations\WordPress\Content;

 use LIT\MCP\Abilities\Ability;
 use LIT\MCP\Abilities\Errors;
+use LIT\MCP\Abilities\FollowUp;
 use LIT\MCP\Abilities\Rev;
 use LIT\MCP\Abilities\ToolResult;
+use LIT\MCP\Abilities\WriteFailed;
 use LIT\MCP\ChangeLog\ChangeLog;
 use LIT\MCP\Integrations\WordPress\PostGuard;
 use LIT\MCP\Integrations\WordPress\RevisionPoint;
@@ -23,8 +25,9 @@ use WP_Post;
 * principle 7). All or nothing (spec §8.2): input is validated first; if fields,
 * terms or the featured image fail after the post row was updated, the row is
 * restored from the restore-point revision (plus old values for columns revisions
- * do not cover), the extras get their old values back, and the error is rethrown
- * (the AbilityRegistrar logs it and reports lit_internal_error).
+ * do not cover), the extras get their old values back, created terms are deleted, and
+ * a WriteFailed is thrown (AbilityRegistrar → lit_write_failed, or lit_partial_write
+ * when the rollback was incomplete; partial writes are logged with result "partial").
 */
 final class ContentUpdateAbility implements Ability
 {
@@ -32,6 +35,8 @@ final class ContentUpdateAbility implements Ability
        'title', 'content', 'excerpt', 'slug', 'parent', 'menu_order', 'template', 'terms', 'fields', 'featured_image',
    ];

+    private const REASON = 'Felder, Begriffe, Beitragsbild oder Kerndaten konnten nicht gespeichert werden.';
+
    /** Post columns a revision stores (and wp_restore_post_revision() restores). */
    private const REVISIONED = ['post_title', 'post_content', 'post_excerpt'];

@@ -133,71 +138,137 @@ final class ContentUpdateAbility implements Ability

        $postId = (int) $post->ID;
        $snapshot = $this->writer->snapshot($postId, $postType, $prepared, $input);
+        $before = $this->columnsBefore($post, $data);
        $revisionId = RevisionPoint::capture($postId);

-        $rowUpdated = false;
+        $rowTouched = false;
        try {
            if ($data !== []) {
+                // Set before the call: a throwing save_post hook runs after the row was written.
+                $rowTouched = true;
                $result = wp_update_post(wp_slash(array_merge($data, ['ID' => $postId])), true);
                if ($result instanceof WP_Error) {
                    return Errors::invalidInput(
                        'WordPress hat die Änderung abgelehnt: ' . $result->get_error_message()
                    );
                }
-                $rowUpdated = true;
            }

            $this->writer->writeExtras($postId, $postType, $prepared, $input);
        } catch (Throwable $e) {
-            $this->rollBack($post, $rowUpdated ? $data : [], $revisionId, $snapshot, $e);
+            $this->rollBack($post, $rowTouched ? $before : [], $revisionId, $snapshot, $e);
        }
-        do_action('lit_mcp_after_save', $postId, $this->name(), $input);
+
+        return $this->result($post, $input, $prepared['warnings'], $revisionId);
+    }
+
+    /**
+     * @param array<string, mixed> $input
+     * @param list<string>         $warnings
+     * @return array<string, mixed>
+     */
+    private function result(WP_Post $post, array $input, array $warnings, ?int $revisionId): array
+    {
+        $postId = (int) $post->ID;
+        $postType = (string) $post->post_type;
+
+        // The change is saved – failures from here on are logged and reported as warnings only.
+        FollowUp::run(
+            fn () => do_action('lit_mcp_after_save', $postId, $this->name(), $input),
+            $this->name() . ' lit_mcp_after_save',
+            'Eine Erweiterung ist nach dem Speichern auf einen Fehler gestoßen; die Änderung ist gespeichert.',
+            $warnings
+        );

        $updated = get_post($postId);
        $updated = $updated instanceof WP_Post ? $updated : $post;
        $changedKeys = array_keys(array_intersect_key($input, array_flip(self::CHANGE_KEYS)));
        $summary = sprintf('"%s" geändert: %s', $updated->post_title, implode(', ', $changedKeys));
-        $logId = $this->log->record($this->name(), $postType, (string) $postId, $summary, $revisionId);
+        $logId = FollowUp::run(
+            fn () => $this->log->record($this->name(), $postType, (string) $postId, $summary, $revisionId),
+            $this->name() . ' change log',
+            'Änderungsprotokoll konnte nicht geschrieben werden; die Änderung ist gespeichert.',
+            $warnings
+        );

        return ToolResult::ok(sprintf('%s (Status unverändert: %s).', $summary, $updated->post_status))
            ->changed((string) $postId, $postType, 'updated', (string) $updated->post_title)
            ->rev(Rev::ofPost($updated))
            ->revisionId($revisionId)
-            ->logId($logId)
+            ->logId(is_int($logId) ? $logId : null)
            ->urls((string) get_preview_post_link($updated), (string) get_edit_post_link($postId, 'raw'))
-            ->warnings($prepared['warnings'])
+            ->warnings($warnings)
            ->toArray();
    }

    /**
-     * @param array<string, mixed> $data     Columns that were written to the post row.
-     * @param array<string, mixed> $snapshot Output of PostWriter::snapshot().
-     * @throws RuntimeException Always – the original failure plus the rollback outcome.
+     * Values of the columns about to be written, read before the update (the template
+     * lives in post meta, so it must be read now – afterwards it is the new one).
+     *
+     * @param array<string, mixed> $data
+     * @return array<string, mixed>
     */
-    private function rollBack(WP_Post $post, array $data, ?int $revisionId, array $snapshot, Throwable $cause): never
+    private function columnsBefore(WP_Post $post, array $data): array
    {
+        $before = [];
+        foreach (array_keys($data) as $column) {
+            $before[$column] = $column === 'page_template'
+                ? ((string) get_page_template_slug($post) ?: PostWriter::DEFAULT_TEMPLATE)
+                : $post->{$column};
+        }
+
+        return $before;
+    }
+
+    /**
+     * @param array<string, mixed> $before   Pre-update values of the columns that were (possibly) written.
+     * @param array<string, mixed> $snapshot Output of PostWriter::snapshot().
+     * @throws WriteFailed Always – rolledBack tells whether everything was restored.
+     */
+    private function rollBack(WP_Post $post, array $before, ?int $revisionId, array $snapshot, Throwable $cause): never
+    {
+        $postId = (int) $post->ID;
+        $steps = [
+            fn () => $this->writer->deleteCreatedTerms(),
+            fn () => $this->writer->restore($postId, (string) $post->post_type, $snapshot),
+            fn () => $this->restoreRow($post, $before, $revisionId),
+        ];
        $failures = [];
-        try {
-            $this->writer->restore((int) $post->ID, (string) $post->post_type, $snapshot);
-        } catch (Throwable $e) {
-            $failures[] = $e->getMessage();
+        foreach ($steps as $step) {
+            try {
+                $step();
+            } catch (Throwable $e) {
+                $failures[] = $e->getMessage();
+            }
        }

-        try {
-            $this->restoreRow($post, $data, $revisionId);
-        } catch (Throwable $e) {
-            $failures[] = $e->getMessage();
+        $detail = sprintf('Update of post %d failed: %s', $postId, $cause->getMessage());
+        if ($failures === []) {
+            throw new WriteFailed(self::REASON, true, $postId, $revisionId, $cause, $detail);
        }

-        throw new RuntimeException(
-            sprintf(
-                'Update of post %d failed (%s); %s.',
-                (int) $post->ID,
-                $cause->getMessage(),
-                $failures === [] ? 'changes rolled back' : 'rollback incomplete: ' . implode('; ', $failures)
+        $ignored = [];
+        FollowUp::run(
+            fn () => $this->log->record(
+                $this->name(),
+                (string) $post->post_type,
+                (string) $postId,
+                sprintf('"%s": Änderung nur teilweise zurückgenommen', $post->post_title),
+                $revisionId,
+                null,
+                'partial'
            ),
-            0,
-            $cause
+            $this->name() . ' change log',
+            '',
+            $ignored
+        );
+        throw new WriteFailed(
+            self::REASON,
+            false,
+            $postId,
+            $revisionId,
+            $cause,
+            $detail . '; rollback incomplete: ' . implode('; ', $failures)
        );
    }

@@ -207,35 +278,28 @@ final class ContentUpdateAbility implements Ability
     * saved or confirmed it). Everything else – and everything when revisions are off,
     * where the newest revision may be stale – gets the value read before the update.
     *
-     * @param array<string, mixed> $data
+     * @param array<string, mixed> $before
     * @throws RuntimeException
     */
-    private function restoreRow(WP_Post $post, array $data, ?int $revisionId): void
+    private function restoreRow(WP_Post $post, array $before, ?int $revisionId): void
    {
-        if ($data === []) {
+        if ($before === []) {
            return;
        }

-        $old = [];
-        foreach (array_keys($data) as $column) {
-            $old[$column] = $column === 'page_template'
-                ? ((string) get_page_template_slug($post) ?: PostWriter::DEFAULT_TEMPLATE)
-                : $post->{$column};
-        }
-
-        $fromRevision = array_values(array_intersect(array_keys($old), self::REVISIONED));
+        $fromRevision = array_values(array_intersect(array_keys($before), self::REVISIONED));
        if ($revisionId !== null && $fromRevision !== [] && wp_revisions_enabled($post)) {
            $restored = wp_restore_post_revision($revisionId, $fromRevision);
            if ($restored && ! is_wp_error($restored)) {
-                $old = array_diff_key($old, array_flip($fromRevision));
+                $before = array_diff_key($before, array_flip($fromRevision));
            }
        }

-        if ($old === []) {
+        if ($before === []) {
            return;
        }

-        $result = wp_update_post(wp_slash(array_merge($old, ['ID' => (int) $post->ID])), true);
+        $result = wp_update_post(wp_slash(array_merge($before, ['ID' => (int) $post->ID])), true);
        if ($result instanceof WP_Error || ! $result) {
            $message = $result instanceof WP_Error ? $result->get_error_message() : 'unknown error';
            throw new RuntimeException('Post row could not be restored: ' . $message);
@@ -7,6 +7,7 @@ namespace LIT\MCP\Integrations\WordPress\Content;
 use LIT\MCP\Abilities\Errors;
 use LIT\MCP\Integrations\WordPress\Schema\ContentSchema;
 use LIT\MCP\Integrations\WordPress\Schema\FieldValidator;
+use LIT\MCP\Integrations\WordPress\Schema\MetaFieldSource;
 use LIT\MCP\Integrations\WordPress\Schema\TermAssigner;
 use RuntimeException;
 use Throwable;
@@ -16,7 +17,8 @@ use WP_Post;
 /**
 * Shared validation and write logic of lit/content-create and lit/content-update.
 * prepare() validates everything before the first write (all or nothing, spec §8.2);
- * snapshot()/restore() let lit/content-update undo the extras when a later write fails.
+ * snapshot()/restore() let lit/content-update undo the extras when a later write fails;
+ * deleteCreatedTerms() removes terms the last writeExtras() created by name.
 * Not final so the abilities can be unit-tested with a mock.
 */
 class PostWriter
@@ -34,6 +36,9 @@ class PostWriter
    /** wp_insert_post() ignores an empty page_template; this value selects the default template. */
    public const DEFAULT_TEMPLATE = 'default';

+    /** @var array<int, string> Terms created by the last writeExtras(): term ID => taxonomy. */
+    private array $createdTerms = [];
+
    public function __construct(
        private ContentSchema $schema,
        private FieldValidator $validator,
@@ -138,7 +143,8 @@ class PostWriter
     */
    public function writeExtras(int $postId, string $postType, array $prepared, array $input): void
    {
-        $this->terms->assign($postId, $prepared['terms']);
+        $this->createdTerms = [];
+        $this->createdTerms = $this->terms->assign($postId, $prepared['terms']);
        $this->writeFields($postId, $postType, $prepared['fields']);

        if (array_key_exists('featured_image', $input)) {
@@ -146,6 +152,18 @@ class PostWriter
        }
    }

+    /**
+     * Rollback helper: deletes the terms the last writeExtras() created by name.
+     *
+     * @throws RuntimeException
+     */
+    public function deleteCreatedTerms(): void
+    {
+        $created = $this->createdTerms;
+        $this->createdTerms = [];
+        $this->terms->deleteTerms($created);
+    }
+
    /**
     * Current values of everything writeExtras() would change – call before the first write.
     *
@@ -159,8 +177,10 @@ class PostWriter
        $fields = [];
        foreach (array_keys($prepared['fields']) as $key) {
            $definition = $definitions[$key];
-            $old = $this->schema->sourceOf($definition)->read($postId, $definition);
-            $fields[$key] = in_array($old, ['', [], null, false], true) ? null : $old;
+            $source = $this->schema->sourceOf($definition);
+            // null = "was not set" → restore() clears it; '', false, [] are real values and are rewritten.
+            $absent = $source instanceof MetaFieldSource && ! metadata_exists('post', $postId, (string) $key);
+            $fields[$key] = $absent ? null : $source->read($postId, $definition);
        }

        $terms = [];
@@ -251,6 +271,10 @@ class PostWriter
            return 'Inhaltstyp unterstützt kein Beitragsbild.';
        }

+        if (! current_user_can('read_post', $imageId)) {
+            return 'Keine Berechtigung für dieses Medium.';
+        }
+
        return wp_attachment_is_image($imageId) ? null : 'Kein Bild aus der Mediathek.';
    }

@@ -262,11 +286,25 @@ class PostWriter
            return null;
        }

-        $parent = get_post($parentId);
+        if (! is_post_type_hierarchical($postType)) {
+            return 'Inhaltstyp ist nicht hierarchisch; parent ist nicht möglich.';
+        }

-        return $parent instanceof WP_Post && $parent->post_type === $postType
-            ? null
-            : 'Übergeordneter Eintrag muss existieren und denselben Inhaltstyp haben.';
+        $postId = (int) ($input['post_id'] ?? 0);
+        if ($postId > 0 && $parentId === $postId) {
+            return 'Ein Eintrag kann nicht sich selbst übergeordnet sein.';
+        }
+
+        $parent = get_post($parentId);
+        if (! $parent instanceof WP_Post || $parent->post_type !== $postType) {
+            return 'Übergeordneter Eintrag muss existieren und denselben Inhaltstyp haben.';
+        }
+
+        if ($postId > 0 && in_array($postId, array_map('intval', get_post_ancestors($parent)), true)) {
+            return 'Übergeordneter Eintrag ist diesem Eintrag untergeordnet (Zirkelbezug).';
+        }
+
+        return current_user_can('read_post', $parentId) ? null : 'Keine Berechtigung für den übergeordneten Eintrag.';
    }

    /**