fix(coderabbit): resolve PR #62 re-review findings (#63)

Three findings from CodeRabbit's re-review of PR #62:

1. TemplateManagerWriteTest.php: a real test-isolation bug —
   test_update_template_reports_rollback_failure_when_delete_also_fails()
   added pre_insert_term/pre_delete_post filters and only removed them
   after update_template() returned. An unexpected exception from that
   call would skip both remove_filter() calls, leaking the filters into
   every later test in the same PHP process (any subsequent
   wp_insert_term()/wp_delete_post() call would be affected). Wrapped the
   call in try/finally so cleanup always runs.

2. Same file: added `@var \WP_Error $result` narrowing docblocks after
   four assertInstanceOf(\WP_Error::class, $result) calls, immediately
   before each result's get_error_code()/get_error_data() calls.
   composer analyze is already clean on develop (tests/* is excluded
   from this repo's own PHPStan paths) — applied because it's a trivial,
   readability-neutral-to-positive one-liner per site, not because our
   gate required it.

3. src/tools/templates.ts: list_templates' description said a non-null
   wp_id "is what makes a template editable via update_template" —
   false. update_template()'s own description (and behavior) already
   documents that a wp_id:null template is still editable: calling it
   creates a new override. Reworded to describe wp_id as existing
   database backing, matching get_template's accurate framing, without
   implying editability requires it.

Manifest/bundle regenerated for the description change (tool count
unchanged at 33).

This is the last dispatch on the CodeRabbit-follow-up thread.

Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3

Author: zackkatz

src/tools/templates.ts

@@ -25,7 +25,7 @@ export const TEMPLATE_TOOLS = [
   {
     name: 'list_templates',
     description:
-      'List the active theme\'s templates (page layouts like "single", "archive") or template parts (reusable regions like "header", "footer") — works on a theme without a full block-theme structure too, as long as it has real templates/parts. Each row includes `wp_id` — present (non-null) whenever the template is backed by a database post, whether that\'s an override shadowing a theme file or a fully custom template with none; that\'s what makes it editable via update_template. Returns an empty list with a `note` only when there is truly nothing to list.',
+      'List the active theme\'s templates (page layouts like "single", "archive") or template parts (reusable regions like "header", "footer") — works on a theme without a full block-theme structure too, as long as it has real templates/parts. Each row includes `wp_id` — present (non-null) when a database post already backs the template (an override shadowing a theme file, or a fully custom template with none); null means it currently resolves purely to the theme file. Every template is editable via update_template regardless — a null `wp_id` just means calling it creates the override rather than updating an existing one. Returns an empty list with a `note` only when there is truly nothing to list.',
     annotations: { ...READ_ANNOT, title: 'List templates' },
     inputSchema: {
       type: 'object' as const,

wordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjs

@@ -53604,7 +53604,7 @@ var TEMPLATE_SOURCE_ENUM = ["theme", "plugin", "custom"];
 var TEMPLATE_TOOLS = [
   {
     name: "list_templates",
-    description: 'List the active theme\'s templates (page layouts like "single", "archive") or template parts (reusable regions like "header", "footer") \u2014 works on a theme without a full block-theme structure too, as long as it has real templates/parts. Each row includes `wp_id` \u2014 present (non-null) whenever the template is backed by a database post, whether that\'s an override shadowing a theme file or a fully custom template with none; that\'s what makes it editable via update_template. Returns an empty list with a `note` only when there is truly nothing to list.',
+    description: 'List the active theme\'s templates (page layouts like "single", "archive") or template parts (reusable regions like "header", "footer") \u2014 works on a theme without a full block-theme structure too, as long as it has real templates/parts. Each row includes `wp_id` \u2014 present (non-null) when a database post already backs the template (an override shadowing a theme file, or a fully custom template with none); null means it currently resolves purely to the theme file. Every template is editable via update_template regardless \u2014 a null `wp_id` just means calling it creates the override rather than updating an existing one. Returns an empty list with a `note` only when there is truly nothing to list.',
     annotations: { ...READ_ANNOT2, title: "List templates" },
     inputSchema: {
       type: "object",

wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json

@@ -2116,7 +2116,7 @@
       "name": "list_templates",
       "ability": "gk-block-mcp/list-templates",
       "label": "List templates",
-      "description": "List the active theme's templates (page layouts like \"single\", \"archive\") or template parts (reusable regions like \"header\", \"footer\") — works on a theme without a full block-theme structure too, as long as it has real templates/parts. Each row includes `wp_id` — present (non-null) whenever the template is backed by a database post, whether that's an override shadowing a theme file or a fully custom template with none; that's what makes it editable via update_template. Returns an empty list with a `note` only when there is truly nothing to list.",
+      "description": "List the active theme's templates (page layouts like \"single\", \"archive\") or template parts (reusable regions like \"header\", \"footer\") — works on a theme without a full block-theme structure too, as long as it has real templates/parts. Each row includes `wp_id` — present (non-null) when a database post already backs the template (an override shadowing a theme file, or a fully custom template with none); null means it currently resolves purely to the theme file. Every template is editable via update_template regardless — a null `wp_id` just means calling it creates the override rather than updating an existing one. Returns an empty list with a `note` only when there is truly nothing to list.",
       "input_schema": {
         "type": "object",
         "properties": {

wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php

@@ -504,16 +504,22 @@ public function test_update_template_reports_rollback_failure_when_delete_also_f
 		};
 		add_filter( 'pre_delete_post', $delete_filter );
 
-		$result = $this->tm->update_template(
-			$this->theme . '//index',
-			'wp_template',
-			array( 'content' => '<!-- wp:paragraph --><p>x</p><!-- /wp:paragraph -->' )
-		);
-
-		remove_filter( 'pre_insert_term', $term_filter, 10 );
-		remove_filter( 'pre_delete_post', $delete_filter );
+		try {
+			$result = $this->tm->update_template(
+				$this->theme . '//index',
+				'wp_template',
+				array( 'content' => '<!-- wp:paragraph --><p>x</p><!-- /wp:paragraph -->' )
+			);
+		} finally {
+			// Run on an unexpected exception too — a leftover filter here
+			// would otherwise contaminate every later term/delete call in
+			// the same PHP process, not just this test.
+			remove_filter( 'pre_insert_term', $term_filter, 10 );
+			remove_filter( 'pre_delete_post', $delete_filter );
+		}
 
 		$this->assertInstanceOf( \WP_Error::class, $result );
+		/** @var \WP_Error $result */
 		$this->assertSame( 'rollback_failed', $result->get_error_code() );
 		$this->assertStringContainsString( 'Simulated taxonomy failure', $result->get_error_message() );
 		$data = $result->get_error_data();
@@ -600,6 +606,7 @@ public function test_update_template_hybrid_theme_bad_id_returns_not_found_not_c
 		);
 
 		$this->assertInstanceOf( \WP_Error::class, $result );
+		/** @var \WP_Error $result */
 		$this->assertSame( 'not_found', $result->get_error_code() );
 		$data = $result->get_error_data();
 		$this->assertSame( 404, $data['status'] );
@@ -617,6 +624,7 @@ public function test_reset_template_hybrid_theme_bad_id_returns_not_found_not_cl
 		$result = $this->tm->reset_template( 'hybrid-theme//does-not-exist', 'wp_template_part' );
 
 		$this->assertInstanceOf( \WP_Error::class, $result );
+		/** @var \WP_Error $result */
 		$this->assertSame( 'not_found', $result->get_error_code() );
 		$data = $result->get_error_data();
 		$this->assertSame( 404, $data['status'] );
@@ -635,6 +643,7 @@ public function test_update_template_classic_theme_still_returns_400_when_nothin
 		$result = $this->tm->update_template( 'default//does-not-exist', 'wp_template', array( 'content' => '<!-- wp:paragraph --><p>x</p><!-- /wp:paragraph -->' ) );
 
 		$this->assertInstanceOf( \WP_Error::class, $result );
+		/** @var \WP_Error $result */
 		$this->assertSame( 'classic_theme', $result->get_error_code() );
 		$data = $result->get_error_data();
 		$this->assertSame( 400, $data['status'] );
@@ -651,6 +660,7 @@ public function test_reset_template_classic_theme_still_returns_400_when_nothing
 		$result = $this->tm->reset_template( 'default//does-not-exist', 'wp_template' );
 
 		$this->assertInstanceOf( \WP_Error::class, $result );
+		/** @var \WP_Error $result */
 		$this->assertSame( 'classic_theme', $result->get_error_code() );
 		$data = $result->get_error_data();
 		$this->assertSame( 400, $data['status'] );