chore: CodeRabbit follow-ups across PRs #52-61 - #62
Merged
Conversation
… [red]
Adds/updates test coverage for the confirmed-genuine CodeRabbit findings
that require a behavior change, plus test-quality fixes (docblocks,
cleanup, portability guards) for the ones that don't. Verified red
against pre-fix code via targeted stash of the implementation changes:
- discovery.ts: list_block_types rejects a non-boolean include_supports
(e.g. the truthy string "false") instead of silently forwarding it.
- patterns.ts: create_pattern rejects an invalid sync_status/status
instead of silently defaulting; declares oneOf(blocks, content) in its
inputSchema so a schema-validating client rejects both-or-neither
before dispatch, not just at runtime.
- class-template-manager.php: update_template's term-assignment rollback
reports rollback_failed (distinct from the original cause) when its own
wp_delete_post() also fails, instead of silently returning the original
error as if cleanup succeeded.
- class-template-manager.php: a hybrid theme (real content elsewhere) with
a bad id returns not_found, not classic_theme — classic_theme is now
reserved for a genuinely classic theme (regression-pinned for both
update_template and reset_template).
Test-quality only (no corresponding production-code change):
- BlockTypesTest.php: contract docblocks on every test; allowed_blocks
assertion skipped on WordPress < 6.5 (the property was added in 6.5,
the plugin's floor is 6.0 — a real portability gap in the test, not
just a style nit).
- PatternsCategoryTest.php: contract docblocks; pattern-registry cleanup
moved to try/finally so an assertion failure doesn't leak fixtures into
later tests.
- BindingSourcesTest.php: contract docblocks; the two tests that assumed
WordPress 6.5+ now branch on the real function_exists() capability and
assert the 6.0-6.4 fallback shape too; a negative case added so
uses_context presence is proven conditional, not unconditional.
- CreatePatternTest.php: the "real text plus whitespace" success case now
wraps the text in markup too, so over-aggressive sanitization would be
caught, not just under-aggressive; review-tool attribution stripped
from section-header comments.
- TemplateManagerTest.php / TemplateManagerWriteTest.php: comments
trimmed to state current contracts rather than implementation history
("the old X" / "Codex review" / "regression"/"proves" framing).
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…61 [green] - discovery.ts: list_block_types validates include_supports is a boolean before forwarding, rejecting a non-boolean instead of letting a truthy non-boolean (e.g. the string "false") silently flip to true downstream. - patterns.ts: create_pattern validates sync_status/status against their declared enums before casting; declares oneOf(blocks, content) in its inputSchema so the "exactly one" contract is enforced structurally, not only at runtime. - coerce.ts: adds isNonEmptyArray()/isNonEmptyString(), the single shared definition of "present" for a list/string field. patterns.ts and client.ts's independent (if currently identical) copies of the same blocks/content emptiness check now both call it, so they can't silently drift apart. - README.md: documents include_supports as opt-in (default false). - class-rest-controller.php: adds the missing @SInCE 2.2.0 to get_binding_sources() and create_pattern() — both new in this range. - class-template-manager.php: - update_template()'s rollback helper (used by both term-assignment failures and a content-write failure) now checks its own wp_delete_post() return value and reports a distinct rollback_failed error — including the orphaned post's ID — when cleanup itself also fails, instead of returning the original error as if cleanup succeeded. - the classic_theme/not_found choice on an unresolved id now also checks whether the theme has ANY real template/part, not just wp_is_block_theme(): a hybrid theme with content elsewhere gets not_found (this id is simply wrong); a genuinely classic theme keeps classic_theme (there's nothing here at all). Applies to both update_template and reset_template. - is_wp_error() checks in the touched code assign a named boolean before branching, per the repo's coding standard. - src/tools/templates.ts: list_templates' description corrected — wp_id is present whenever a template is database-backed (an override OR a fully custom template with no theme file), not only when an override "shadows" a theme file. - Manifest/bundle regenerated (tool count unchanged at 33) for the create_pattern schema/description and list_templates description changes. ## Skipped findings (verified against current code) - PR52 class-template-manager.php:226 — stale. format_template_summary()'s wp_id comment already names update_template/reset_template as real, implemented tools, not speculative future work. - PR57 tools.manifest.json:1192 — stale. create_pattern's manifest permission already routes through check_create_pattern_permissions (BLOCK-35), matching the REST route's cap check exactly. - PR55 discovery.ts:214 (list_binding_sources enrichment) — the codebase's existing enrichment (enrichBlockTypes/enrichPatternList) groups/summarizes server-computed classification data (tier, score) it already has; binding sources carry no such classification, so "guidance" here would mean inventing and maintaining hardcoded prose about named third-party sources — a different, heavier kind of enrichment than the established pattern, not a small clean addition. - PR57 src/client.ts:404 (@SInCE on createPattern) — this repo's @SInCE convention is PHP-only; zero TS docblocks anywhere in the codebase carry one. - PR55 BindingSourcesTest.php:51 ("add a controllable capability-check seam") — would require adding a test-only filter/seam to production code (Block_Registry::get_binding_sources()) purely so a modern (6.9) test environment can force the pre-6.5 branch; CodeRabbit's own severity tag flags this as a "Heavy lift". The existing test already documents why it pins the fallback's literal shape instead of exercising it live. Its sibling asks (docblocks, branch-on-real- capability for the other two tests, negative uses_context case) are fixed. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
WalkthroughThe change tightens MCP tool validation, clarifies tool documentation, improves template error classification and rollback handling, and expands REST and template regression coverage. ChangesMCP tool contracts and validation
Template classification and rollback
REST contract test maintenance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TemplateManager
participant WordPressTaxonomy
participant WordPressPostAPI
participant OverridePost
TemplateManager->>WordPressTaxonomy: assign required override terms
WordPressTaxonomy-->>TemplateManager: return assignment result
TemplateManager->>WordPressPostAPI: write override content or blocks
WordPressPostAPI-->>TemplateManager: return write result
TemplateManager->>OverridePost: delete newly-created override on failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
wordpress-plugin/gk-block-mcp/tests/REST/BlockTypesTest.php (1)
43-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the historical regression narrative.
Keep the current
stylescontract, but remove “an earlier version” language from Lines 45-48.As per coding guidelines, comments and docblocks must document present behavior and hard contracts, not development history.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-mcp/tests/REST/BlockTypesTest.php` around lines 43 - 49, Update the docblock near the styles contract to describe only the current behavior: styles must include every block.json-declared variation and correctly flag the default with is_default. Remove the historical regression and failure-mode narrative referencing earlier versions and missing styles.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json`:
- Line 2119: The `description` for the template-listing tool incorrectly
presents non-null `wp_id` as required for editing. Update that wording to
describe `wp_id` only as indicating existing database backing, while preserving
that templates with null `wp_id` can still be edited through `update_template`
by creating an override and aligning with the `get_template` contract.
In `@wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php`:
- Around line 516-520: Add a `@var` \WP_Error $result narrowing annotation
immediately after each assertInstanceOf(\WP_Error::class, $result) in
TemplateManagerWriteTest.php at lines 516-520, 602-605, 619-622, and 653-656,
before invoking get_error_code(), get_error_message(), or get_error_data().
- Around line 494-514: Wrap the update_template() invocation and both filter
removals in a try/finally block so pre_insert_term and pre_delete_post are
always removed, including when update_template() throws. Keep the existing
filter setup and result assignment unchanged, and perform cleanup in the finally
block.
---
Nitpick comments:
In `@wordpress-plugin/gk-block-mcp/tests/REST/BlockTypesTest.php`:
- Around line 43-49: Update the docblock near the styles contract to describe
only the current behavior: styles must include every block.json-declared
variation and correctly flag the default with is_default. Remove the historical
regression and failure-mode narrative referencing earlier versions and missing
styles.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72c1f0bf-eb88-4ff2-ba48-c0582dba192a
📒 Files selected for processing (18)
README.mdsrc/__tests__/tools/discovery/list_block_types.test.tssrc/__tests__/tools/patterns/create_pattern.test.tssrc/client.tssrc/coerce.tssrc/tools/discovery.tssrc/tools/patterns.tssrc/tools/templates.tswordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjswordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.jsonwordpress-plugin/gk-block-mcp/includes/class-rest-controller.phpwordpress-plugin/gk-block-mcp/includes/class-template-manager.phpwordpress-plugin/gk-block-mcp/tests/REST/BindingSourcesTest.phpwordpress-plugin/gk-block-mcp/tests/REST/BlockTypesTest.phpwordpress-plugin/gk-block-mcp/tests/REST/CreatePatternTest.phpwordpress-plugin/gk-block-mcp/tests/REST/PatternsCategoryTest.phpwordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.phpwordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php
| "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` — non-null only when a database override shadows the theme file, which is what makes a template 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) 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.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not imply that wp_id is required for editability.
Line 2119 says a non-null wp_id is what makes a template editable, but Line 2203 documents that update_template can create an override when wp_id is null. Align this wording with the get_template contract at Line 2168 and describe wp_id as existing database backing, not an editability prerequisite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json` at line
2119, The `description` for the template-listing tool incorrectly presents
non-null `wp_id` as required for editing. Update that wording to describe
`wp_id` only as indicating existing database backing, while preserving that
templates with null `wp_id` can still be edited through `update_template` by
creating an override and aligning with the `get_template` contract.
Comment on lines
+494
to
+514
| $term_filter = static function ( $term, $taxonomy ) { | ||
| if ( 'wp_theme' === $taxonomy ) { | ||
| return new \WP_Error( 'term_insert_failed', 'Simulated taxonomy failure.' ); | ||
| } | ||
| return $term; | ||
| }; | ||
| add_filter( 'pre_insert_term', $term_filter, 10, 2 ); | ||
|
|
||
| $delete_filter = static function () { | ||
| return false; // Force wp_delete_post() to short-circuit and fail. | ||
| }; | ||
| 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 ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Always remove test filters.
An unexpected exception from update_template() skips both removals and contaminates later tests. Use try/finally.
Proposed fix
add_filter( 'pre_insert_term', $term_filter, 10, 2 );
-
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 {
+ remove_filter( 'pre_insert_term', $term_filter, 10 );
+ remove_filter( 'pre_delete_post', $delete_filter );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $term_filter = static function ( $term, $taxonomy ) { | |
| if ( 'wp_theme' === $taxonomy ) { | |
| return new \WP_Error( 'term_insert_failed', 'Simulated taxonomy failure.' ); | |
| } | |
| return $term; | |
| }; | |
| add_filter( 'pre_insert_term', $term_filter, 10, 2 ); | |
| $delete_filter = static function () { | |
| return false; // Force wp_delete_post() to short-circuit and fail. | |
| }; | |
| 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 ); | |
| $term_filter = static function ( $term, $taxonomy ) { | |
| if ( 'wp_theme' === $taxonomy ) { | |
| return new \WP_Error( 'term_insert_failed', 'Simulated taxonomy failure.' ); | |
| } | |
| return $term; | |
| }; | |
| add_filter( 'pre_insert_term', $term_filter, 10, 2 ); | |
| $delete_filter = static function () { | |
| return false; // Force wp_delete_post() to short-circuit and fail. | |
| }; | |
| add_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 { | |
| remove_filter( 'pre_insert_term', $term_filter, 10 ); | |
| remove_filter( 'pre_delete_post', $delete_filter ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php`
around lines 494 - 514, Wrap the update_template() invocation and both filter
removals in a try/finally block so pre_insert_term and pre_delete_post are
always removed, including when update_template() throws. Keep the existing
filter setup and result assignment unchanged, and perform cleanup in the finally
block.
Comment on lines
+516
to
+520
| $this->assertInstanceOf( \WP_Error::class, $result ); | ||
| $this->assertSame( 'rollback_failed', $result->get_error_code() ); | ||
| $this->assertStringContainsString( 'Simulated taxonomy failure', $result->get_error_message() ); | ||
| $data = $result->get_error_data(); | ||
| $this->assertSame( 500, $data['status'] ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Narrow WP_Error before calling its methods.
PHPStan reports each result as array|WP_Error, so these method calls fail static analysis despite the preceding assertion.
wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L516-L520: add a@var \WP_Error $resultassertion afterassertInstanceOf().wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L602-L605: add the same narrowing annotation.wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L619-L622: add the same narrowing annotation.wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L653-L656: add the same narrowing annotation.
🧰 Tools
🪛 PHPStan (2.2.5)
[error] 517-517: Cannot call method get_error_code() on array|WP_Error.
(method.nonObject)
[error] 518-518: Cannot call method get_error_message() on array|WP_Error.
(method.nonObject)
[error] 519-519: Cannot call method get_error_data() on array|WP_Error.
(method.nonObject)
📍 Affects 1 file
wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L516-L520(this comment)wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L602-L605wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L619-L622wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L653-L656
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php`
around lines 516 - 520, Add a `@var` \WP_Error $result narrowing annotation
immediately after each assertInstanceOf(\WP_Error::class, $result) in
TemplateManagerWriteTest.php at lines 516-520, 602-605, 619-622, and 653-656,
before invoking get_error_code(), get_error_message(), or get_error_data().
Sources: Coding guidelines, Linters/SAST tools
zackkatz
added a commit
that referenced
this pull request
Jul 23, 2026
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CodeRabbit review feedback across the 8 merged PRs (#52-#61) — 31 findings, verified individually against current
develop(not each PR's original diff, since later PRs changed a lot of the flagged code). Every finding below is either fixed, or explicitly skipped with a one-line reason.26 fixed, 5 skipped (2 confirmed stale, 3 judgment-call skips with documented reasoning).
Fixed — genuine code-quality / correctness
README.mdinclude_supportsas opt-in (default false)src/tools/discovery.tslist_block_typesrejects a non-booleaninclude_supports(e.g. the truthy string"false") instead of silently forwarding itclass-rest-controller.php@since 2.2.0toget_binding_sources()class-rest-controller.php@since 2.2.0tocreate_pattern()src/tools/patterns.tscreate_pattern's inputSchema now declaresoneOf(blocks, content)— the XOR contract is enforced structurally, not just at runtimesrc/tools/patterns.tscreate_patternvalidatessync_status/statusagainst their enums before castingsrc/client.ts+src/coerce.tsisNonEmptyArray()/isNonEmptyString()— the single shared definition of "empty" forblocks/content, used by both the MCP dispatch layer and the client layer instead of two independent (if currently identical) copiesclass-template-manager.phpupdate_template()'s rollback path now checks its ownwp_delete_post()return and reports a distinctrollback_failederror (with the orphaned post ID) when cleanup itself also fails, instead of returning the original error as if cleanup succeededclass-template-manager.phpis_wp_error()predicates before branching in the touched rollback code, per the repo's coding standardclass-template-manager.phpget_templates()'s hybrid-theme note conditionclass-template-manager.phpupdate_template()/reset_template():classic_themevsnot_foundon an unresolved id now also checks whether the theme has any real template/part, not justwp_is_block_theme()— a hybrid theme with content elsewhere getsnot_found(this id is simply wrong); a genuinely classic theme keepsclassic_theme(nothing exists at all)src/tools/templates.tslist_templates' description corrected:wp_idis present whenever a template is database-backed (an override or a fully custom template with no theme file), not only when an override "shadows" a theme filetests/Templates/TemplateManagerWriteTest.php(test file, PHPStan)$resultbefore callingWP_Errormethods so PHPStan's own (broader) pass doesn't flagarray|WP_ErrorFixed — test quality
BlockTypesTest.php: contract docblocks on every test;allowed_blocksassertion skipped on WordPress < 6.5 —WP_Block_Type::$allowed_blockswas added in 6.5.0, the plugin's floor is 6.0, so this was a genuine portability gap in the test, not just a style nit (bundled test WP is 6.9.4, so the guard is a no-op in this repo's own CI).PatternsCategoryTest.php: contract docblocks; pattern-registry cleanup moved totry/finallyso an assertion failure doesn't leak fixtures into later tests.BindingSourcesTest.php: contract docblocks; the two tests that assumed WordPress 6.5+ now branch on the realfunction_exists()capability and assert the 6.0-6.4 fallback shape too; added a negative case souses_contextpresence is proven conditional (only for a source that declares it), not unconditional.CreatePatternTest.php: the "real text plus whitespace" success case now wraps the text in markup too (<em>Real Title</em>), so over-aggressive sanitization would be caught, not just under-aggressive; review-tool attribution stripped from section-header comments.TemplateManagerTest.php/TemplateManagerWriteTest.php: comments trimmed to state current contracts rather than implementation/review history ("the old X short-circuit" / "Codex review" / "Unchanged regression... proves" framing).Skipped, with reason
class-template-manager.php:226format_template_summary()'swp_idcomment already namesupdate_template/reset_templateas real, implemented tools — not speculative future work (BLOCK-33 implemented them after this finding was raised).tools.manifest.json:1192create_pattern's manifestpermission: "create_pattern"already routes throughcheck_create_pattern_permissionsinAbilities_Registry::check_tool_permission()(BLOCK-35), matching the REST route's cap check exactly.src/tools/discovery.ts:214(enrichlist_binding_sources)enrichBlockTypes/enrichPatternList) groups/summarizes server-computed classification data (tier, score) it already has. Binding sources carry no such classification — "guidance" here would mean inventing and maintaining hardcoded prose describing named third-party sources, a heavier and fundamentally different kind of enrichment than the established pattern, not a small clean addition.src/client.ts:404(@sinceoncreatePattern)@sinceconvention is PHP-only (per project standard); zero TS docblocks anywhere in the codebase carry one — adding it here would be the first and only instance.BindingSourcesTest.php:51("add a controllable capability-check seam")Block_Registry::get_binding_sources()) purely so a modern (6.9.4) test environment can force the pre-6.5 branch. CodeRabbit's own severity tag flags this as a "Heavy lift". The existing test already has a docblock explaining exactly why it pins the fallback's literal shape/message instead of exercising it live (the bundled WP version can't flipfunction_exists()false). Its sibling asks in the same finding — docblocks, branching the other two tests on the real capability, the negativeuses_contextcase — are fixed.Verification
Strict TDD: RED (
3912710) → GREEN (fad0d6d), unsquashed. Every behavioral change (schema XOR,include_supports/sync_status/statusvalidation, rollback-failure handling, hybrid-vs-classicnot_found) was proven RED against pre-fix code via a targetedgit stashof just the implementation files, tests re-run, restored, re-run GREEN — not merely "wrote a test that happens to pass."No Linear issue for this batch (CodeRabbit follow-ups, not a tracked feature/bug).
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
💾 Build file (fad0d6d).