fix: CodeRabbit re-review of PR #62 (final pass)

[zackkatz]

Summary

CodeRabbit re-reviewed PR #62 and left 3 comments. All 3 verified genuine and fixed — no skips this round.

#	File:line	Finding	Fix
1	tests/Templates/TemplateManagerWriteTest.php:514	Real test-isolation bug. test_update_template_reports_rollback_failure_when_delete_also_fails() registered 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 anywhere in the suite would be silently affected.	Wrapped the call in try/finally so cleanup always runs, matching the pattern already used elsewhere in this file.
2	tests/Templates/TemplateManagerWriteTest.php:520	PHPStan (CodeRabbit's own pass) reports $result as array|WP_Error at 4 sites, so the get_error_code()/get_error_data() calls after assertInstanceOf() fail static analysis. composer analyze is already clean on develop — tests/* is excluded from this repo's own PHPStan paths, so this was never a real gate failure.	Applied anyway: added @var \WP_Error $result narrowing docblocks at all 4 flagged sites — a trivial, readability-neutral-to-positive one-liner each, not a real fix to a real problem.
3	includes/abilities/tools.manifest.json:2119	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 src/tools/templates.ts's list_templates description to describe wp_id as existing database backing (matching get_template's already-accurate framing), without implying editability requires it. Manifest + bundle regenerated.

Verification

composer test    → 1434 + 28 + 11 + 5 tests, 0 failures
composer lint     → 0 errors, 0 warnings
composer analyze  → [OK] No errors
npm test          → 55 files / 864 tests passed
npx tsc --noEmit  → clean
manifest drift    → tool count unchanged (33); only list_templates' description changed
npm run build     → bundle regenerated + copied

Per the coordinator: this is the last dispatch on the CodeRabbit-follow-up thread — no further re-review chasing after this merges.

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

Summary by CodeRabbit

• Documentation

  • Clarified template ID behavior, including the difference between database-backed templates and templates resolved from theme files.
  • Documented that all templates can be updated, with updates to theme-only templates creating an override.

• Tests

  • Improved cleanup during template update rollback tests.
  • Added type clarification to template resolution error tests.

💾 Build file (a555fd9).

[coderabbitai]

Walkthrough

The change clarifies wp_id semantics for list_templates and documents override creation through update_template. Template write tests now guarantee filter cleanup after failures and add WP_Error type annotations.

Changes

Template tooling

Layer / File(s)	Summary
Clarify template identifier semantics src/tools/templates.ts, wordpress-plugin/gk-block-mcp/assets/..., wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json	Updates list_templates descriptions to distinguish database-backed templates from theme-file-only templates and explain override creation when wp_id is null.
Harden template write tests wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php	Ensures failure-path filters are removed in a finally block and adds WP_Error type annotations to unresolved-template tests.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• GravityKit/block-mcp#52: Introduces the related templates tooling and behaviors.
• GravityKit/block-mcp#60: Covers related template override rollback behavior.
• GravityKit/block-mcp#61: Updates the same list_templates descriptions and template write tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title describes the re-review process, not the actual code changes, so it doesn't clearly communicate the PR's purpose.	Rename it to summarize the concrete changes, e.g. "Clarify template wp_id semantics and harden template update tests".

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/coderabbit-followups-2

---

_{Comment @coderabbitai help to get the list of available commands.}