Conversation
…er lint [ci skip] Claude-Session: https://claude.ai/code/session_01QXSpeNWR9tb5BaVhNjrSaV
…onstraints, supports (#53) * feat(block-mcp): expose style variations + nesting constraints in /block-types format_block_type() now merges block.json `styles` with anything registered via register_block_style(), dedupes by name, and reports `parent`/`ancestor`/`allowed_blocks` nesting constraints. The full `supports` object is opt-in via a new `include_supports` request arg (default false) threaded through Block_Registry::get_block_types() and the /block-types REST route. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(block-mcp): PHPUnit coverage for /block-types styles + nesting fields Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * feat(block-mcp): thread include_supports through list_block_types tool client.getBlockTypes() and the list_block_types tool now accept include_supports; BlockType grows styles/parent/ancestor/allowed_blocks/ supports fields; agent-guide notes to check styles before is-style-* classNames and respect nesting constraints. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(block-mcp): TS coverage for include_supports forwarding + new field passthrough Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * chore(block-mcp): regenerate abilities manifest + bundle; README row Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…gory vocabulary (#54) * feat(block-mcp): /patterns response includes registered pattern categories GET /patterns now returns a top-level `categories` array (from WP_Block_Pattern_Categories_Registry), the vocabulary the existing `category` filter arg matches against. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(block-mcp): PHPUnit coverage for /patterns category filter + categories vocabulary Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * feat(block-mcp): list_patterns exposes category filter + categories vocabulary client.getPatterns() return type gains `categories`; the list_patterns tool schema gains `category`, forwards it, and passes the response's `categories` array through. Description explains the semantic split: registered patterns match declared pattern categories, synced patterns match the block categories used in their content. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(block-mcp): TS coverage for list_patterns category forwarding + categories passthrough Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * chore(block-mcp): regenerate abilities manifest + bundle; README row Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…emplate (#52) * feat(templates): add Template_Manager and read-only FSE template REST routes New Template_Manager wraps get_block_templates()/get_block_template() to list/read a block theme's templates and template parts (BLOCK-29). Adds Block_Reader::format_content_blocks() (delegated through Block_CRUD) to format raw template markup the same way get_page_blocks() formats a post, without persisting gk_refs (there's no post to write them into). New GET /templates and GET /template routes on REST_Controller, wired through build_block_services(). REST_Controller now takes a Template_Manager constructor argument; updated the three test call sites accordingly. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(templates): cover Template_Manager and the template REST routes Unit coverage for get_templates()/get_template() (theme-file listing, DB-override shadowing, area filtering, classic-theme fallback, title-as-array defense, not-found/missing-id/invalid-type errors) plus REST dispatch tests for permission checks and the id-as-query-arg contract. Both files work around a wp-phpunit quirk where switch_theme() to any fixture theme other than WP_DEFAULT_THEME resolves against the wrong theme root when only one theme directory is registered. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * feat(templates): add list_templates/get_template MCP tools Wires the new REST routes into the npm MCP server: WordPressBlockClient gains getTemplates()/getTemplate(), src/types.ts gains the Template* interfaces, and src/tools/templates.ts exposes list_templates + get_template (read-only annotations, matching discovery.ts). Registered in src/index.ts's tool aggregation + dispatch table. Documents the new tools in README.md and adds a "Templates" section to the agent guide. Rebuilds dist/index.cjs and its copy into the WordPress plugin (assets/mcp-server/index.cjs). Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…ate (#56) * feat(templates): add gated update_template/reset_template writes New Template_Manager::update_template()/reset_template() (BLOCK-33), gated by option gk_block_api_template_edits (default off, filter gk/block-mcp/templates/allow-edits), modeled line-for-line on Post_Manager::ALLOW_TRASH_OPTION/trashing_enabled(). The gate is enforced both in the new REST permission callback (check_template_edit_permissions: toggle ON and edit_posts or edit_theme_options) and again inside the service methods, so a direct caller gets the same 403 a disabled route would. update_template() resolves the template via get_block_template(); a theme-file-only id gets a database override created the way the Site Editor does (wp_insert_post + the mandatory wp_theme term, plus wp_template_part_area for parts), then the new content is applied through the standard Block_Writer pipeline (replace_all_blocks for `blocks` -- full registry/tier/dual-storage validation -- or save_post_content for `content`, wp_kses_post-sanitized). A content-apply failure on a freshly-created override rolls it back so a rejected write never leaves an empty shell. reset_template() deletes the override outright. New POST /template + POST /template/reset routes. Settings page gets a "Template editing" checkbox next to the trash/uploads/abilities toggles. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(templates): cover the gated template-write surface New TemplateManagerWriteTest.php: the toggle gate (default off, both directions via the filter), override creation (wp_theme + wp_template_part_area terms), content vs. blocks input, idempotent reuse of an existing override on a second write, rollback of a freshly-created override when a legacy block is rejected, reset (deletes the override, get_template reverts to the theme file), classic-theme and content/blocks-mutual-exclusivity guards, and a probe test confirming update_block works against a template override's wp_id like any other post. TemplatesRestTest.php gains REST-dispatch coverage for POST /template and POST /template/reset: 403 when the toggle is off, success for an editor (edit_posts) once it's on, 403 for a subscriber even with the toggle on, success via edit_theme_options alone (the "self" connection path), and reset deleting the override through the full route. The legacy-block-rejection REST test calls the controller handler directly rather than through rest_get_server()->dispatch(): the plugin's own rest_api_init wires a second, production REST_Controller onto the global REST server, and that instance's Preferences lazily caches namespace scores on first read, so a per-test option seed after that first read never reaches it. Permission/sanitization aren't under test in that one case (both are covered elsewhere in this file via real dispatch), so a direct call is safe -- matches AGENTS.md's documented "direct handler call is not dispatch" caveat. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * feat(templates): add update_template/reset_template MCP tools Wires the new gated write routes into the npm MCP server: WordPressBlockClient gains updateTemplate()/resetTemplate(), src/types.ts gains UpdateTemplateRequest/Response + ResetTemplateResponse, and src/tools/templates.ts exposes update_template + reset_template (destructiveHint: true, content/blocks mutual-exclusivity validated client-side before the request goes out). Documents the gate and both tools in README.md ("Editing templates") and the agent guide. Rebuilds dist/index.cjs and its copy into the WordPress plugin. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* feat(block-mcp): add GET /binding-sources — list registered block bindings sources
Block_Registry::get_binding_sources() wraps
get_all_registered_block_bindings_sources() behind a function_exists
guard (Block Bindings requires WP 6.5+); below that it returns an
empty sources array with an explanatory note. Each source reports
{name, label, uses_context?}.
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* test(block-mcp): PHPUnit coverage for /binding-sources; bump tool-count fixtures to 28
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* feat(block-mcp): add list_binding_sources MCP tool
client.getBindingSources() → GET /binding-sources; new no-arg
list_binding_sources discovery tool; agent-guide notes to check
registered sources before wiring metadata.bindings.
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* test(block-mcp): TS coverage for list_binding_sources dispatch + passthrough
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* chore(block-mcp): regenerate abilities manifest + bundle; README row
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* chore(block-mcp): retrigger CI (prior run dropped during GitHub Actions runner incident)
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* fix(block-mcp): abilities-manifest generator was missing TEMPLATE_TOOLS
Discovered while rebasing BLOCK-32 onto develop: the read-only
list_templates/get_template tools (merged in #52) are wired into
src/index.ts but scripts/export-abilities-manifest.mjs never imported
TEMPLATE_TOOLS, so they were silently absent from the WordPress
Abilities API surface (register_abilities() only reads the manifest).
Both tools are readOnlyHint:true so they classify correctly as `read`
permission with no further generator changes.
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* test(block-mcp): bump tool-count fixtures to 30 after rebase onto templates work
develop now includes #52 (list_templates/get_template). Combined with
this branch's list_binding_sources and the manifest-generator fix, the
full tool count is 30 (27 baseline + 2 template tools + 1 new).
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* chore(block-mcp): regenerate abilities manifest + bundle post-rebase
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* test(block-mcp): bump tool-count fixtures to 32 after rebase onto #56
develop now also includes update_template/reset_template (#56, same
TEMPLATE_TOOLS array my manifest-generator fix already covers).
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* chore(block-mcp): regenerate abilities manifest + bundle post-rebase (32 tools)
Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…ies API (#58) * test(abilities): templates group registration + gate parity [red] Writes the failing tests for BLOCK-34 first, before any production code change. Confirmed red against current origin/develop, where TEMPLATE_TOOLS is already wired into scripts/export-abilities-manifest.mjs's buildManifest() (the manifest already carries all 4 templates tools at 32 total / 29 non-yoast -- registration is manifest-driven and doesn't depend on Tool_Executor actually knowing how to run a tool), but two real gaps remain: 1. update_template/reset_template map to the manifest's default 'edit_post' permission instead of a gated one -- Abilities_Registry's permission check never consults Template_Manager::edits_enabled(), so the write gate the REST routes enforce (POST /template, POST /template/reset) doesn't apply on the Abilities/MCP-Adapter path at all. 2. Tool_Executor::execute() has no case for any of the 4 template tool names, so every one of them 400s with "Unknown Block MCP tool" the moment an agent actually tries to call it, gate aside. New tests/Abilities/TemplateAbilitiesTest.php covers both: ability registration presence, list_templates/get_template execution, and the write pair's gate parity with their REST twins -- toggle off denies (editor, subscriber), the gk/block-mcp/templates/allow-edits filter forcing it off despite the stored option, toggle on + edit_posts succeeds and persists (round-tripped via get-template), toggle on + no capability still denies, edit_theme_options alone (the "self" connection path) succeeds, and reset-template's full round trip. Extends AbilitiesRegistryTest's existing readonly/destructive annotation tests to cover the templates group (these already pass -- the annotations came through correctly with the manifest wiring) and adds the four template tool/ability names to the existing manifest-count/ability-ids assertions without changing the counts (both already account for the templates group). tests/abilities-manifest.test.ts gains three assertions: the templates group is present (already true), list_templates/get_template map to 'read' (already true), and update_template/reset_template map to a new 'template_edit' permission distinct from 'edit_post' (not yet true -- genuinely red). Confirmed red: $ vendor/bin/phpunit -c tests/phpunit.xml tests/Abilities/ Tests: 153, Assertions: 360, Failures: 8. (list_templates/get_template/update_template: "Unknown Block MCP tool"; update_template/reset_template gate-off + filter-off tests: got 'unknown_tool' instead of 'ability_invalid_permissions' -- the wrong 'edit_post' permission branch let an editor through, then execution 404'd instead of the permission callback denying it; edit_theme_options-alone test: "does not have necessary permission" -- edit_post's per-post branch doesn't recognize edit_theme_options at all) $ npm test -- tests/abilities-manifest.test.ts Tests 1 failed | 4 passed (5) (update_template/reset_template permission: expected 'template_edit', received 'edit_post') Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * feat(abilities): wire templates group execution + gate parity [green] Makes the BLOCK-34 red tests pass. scripts/export-abilities-manifest.mjs: new TEMPLATE_EDIT permission bucket (update_template, reset_template) mapped to a 'template_edit' permission key, checked before the read/edit_post fallbacks in permissionFor() so these two tools stop resolving to the ungated default 'edit_post' branch. Regenerated tools.manifest.json (still 32 tools -- TEMPLATE_TOOLS was already wired into buildManifest()'s tool list; only the two permission values change). Abilities_Registry::check_tool_permission() gets a 'template_edit' case that delegates to REST_Controller::check_template_edit_permissions() -- the exact same method POST /template and POST /template/reset use as their permission_callback -- so the toggle (Template_Manager::edits_enabled()) and its capability half (edit_posts or edit_theme_options) are enforced identically on both surfaces. No gate logic is re-implemented here. Tool_Executor::execute() gains dispatch cases + execute_*() methods for list_templates, get_template, update_template, and reset_template, each delegating to the matching REST_Controller handler via call_controller() (the existing hand-built WP_REST_Request + set_url_params()/set_body_params() pattern every other ability already uses). Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * docs(agents): add the Abilities-wiring step to the Add-an-MCP-tool recipe A new tool group file needs an explicit import + spread in scripts/export-abilities-manifest.mjs's buildManifest() (it only exports groups it's told about), a permission mapping in permissionFor() (reusing an existing bucket or adding a new gated one, mirroring the REST route's own permission callback -- never re-implementing the gate), and a Tool_Executor::execute() dispatch case -- three steps BLOCK-29/33 skipped for the templates group and BLOCK-34 had to backfill. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…atus control (#57) * test(patterns): create_pattern sync-status + XOR + cap checks [red] Failing-first PHPUnit coverage for POST /patterns (create_pattern), written against the not-yet-implemented REST_Controller::create_pattern() and check_create_pattern_permissions(). All 7 assertions currently error with "Call to undefined method" — confirmed red before implementation lands in the next commit(s). Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(patterns): create_pattern XOR validation + dispatch + response passthrough [red] Failing-first Vitest coverage for the not-yet-implemented create_pattern tool (handlePatternTool has no case for it yet). All 8 assertions currently fail with "Unknown pattern tool: create_pattern" — confirmed red before implementation lands in the next commit(s). Adds a createPattern mock stub to the shared mock client as test scaffolding. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * feat(patterns): implement create_pattern [green] Pattern_Manager::create_pattern() creates a wp_block post: structured blocks go through Block_CRUD::build_block_from_def() (the same registry/tier/dual-storage validation create_post uses), raw content through wp_kses_post(). sync_status defaults to synced (meta absent); 'unsynced' sets core's own wp_pattern_sync_status meta key. format_synced_pattern() now reports sync_status too. POST /patterns registers CREATABLE alongside the existing GET, gated by check_create_pattern_permissions() (edit_posts + the wp_block post type's create_posts cap, which maps to publish_posts). Pattern_Manager's constructor now takes a Block_CRUD; gk-block-mcp.php reorders construction so block_crud is built before pattern_manager. CreatePatternTest and the create_pattern Vitest suite (committed red in prior commits) now pass. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(patterns): update Pattern_Manager call sites for the new Block_CRUD param Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * feat(patterns): create_pattern MCP tool [green] client.createPattern() → POST /patterns; new create_pattern tool in PATTERN_TOOLS validates title + the content/blocks XOR client-side (mirroring create_post), defaults sync_status to synced and status to publish, and forwards structured blocks via the shared BLOCK_INPUT_SCHEMA. New CreatePatternRequest/CreatePatternResponse types; Pattern gains an optional sync_status field. The create_pattern Vitest suite (committed red in a prior commit) now passes. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * test(patterns): bump tool-count fixtures to 33 for create_pattern Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * docs(patterns): README row + agent-guide sentence for create_pattern; regenerate manifest + bundle Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…_Executor (#59) * test(abilities): list_binding_sources + create_pattern execution/gate parity [red] Writes the failing tests for BLOCK-35 first, before any production code change. Same class of gap Agent A flagged after #57/#55 merged and I fixed for the templates group in #58: both tools are already in the manifest and register as abilities (registration is manifest-driven, independent of Tool_Executor), but neither has a Tool_Executor::execute() case, so both 400 "Unknown Block MCP tool" over the Abilities/MCP-Adapter transport regardless of the caller's permissions. New tests/Abilities/BindingSourcesAbilityTest.php: registration presence, execution returns the {sources:[...]} shape, subscriber denial (read permission parity with the rest of the discovery group), readonly annotation. New tests/Abilities/CreatePatternAbilityTest.php: registration presence, execution creates a real wp_block post for an editor, subscriber denial (base edit_posts check), and the actual gate-parity bug this issue exists to fix — a Contributor (has edit_posts, lacks publish_posts) must be denied by the ability exactly as REST_Controller::check_create_pattern_permissions() denies the identical request over REST, because that permission callback checks wp_block's create_posts capability (-> publish_posts) on top of the base edit_posts check. A manifest permission of plain 'edit_post' would let this actor through the ability while REST denies them. Also pins check_create_pattern_permissions() denying the same Contributor directly, and the create-pattern annotation (not readonly, not destructive). tests/abilities-manifest.test.ts gains two assertions: list_binding_sources already maps to 'read' (confirms no TS-side change needed there — the gap is PHP-execution-only for this tool) and create_pattern must map to a new 'create_pattern' permission distinct from 'edit_post' (genuinely red). Confirmed red: $ vendor/bin/phpunit -c tests/phpunit.xml tests/Abilities/ Tests: 163, Assertions: 403, Failures: 3. (list_binding_sources execution: "Unknown Block MCP tool"; create_pattern execution: "Unknown Block MCP tool"; create_pattern Contributor-denial: got 'unknown_tool' instead of 'ability_invalid_permissions' -- the wrong 'edit_post' permission branch let the Contributor through, then execution 400'd instead of the permission callback denying it) $ npm test -- tests/abilities-manifest.test.ts Tests 1 failed | 6 passed (7) (create_pattern permission: expected 'create_pattern', received 'edit_post'; list_binding_sources permission already correctly 'read') Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * feat(abilities): wire list_binding_sources + create_pattern execution/gate parity [green] Makes the BLOCK-35 red tests pass. scripts/export-abilities-manifest.mjs: new CREATE_PATTERN permission bucket mapping create_pattern to a 'create_pattern' permission key (checked before the edit_post fallback). list_binding_sources needed no change -- its readOnlyHint annotation already resolved to 'read'. Regenerated tools.manifest.json (still 33 tools; only create_pattern's permission value changes). Abilities_Registry::check_tool_permission() gets a 'create_pattern' case that delegates to REST_Controller::check_create_pattern_permissions() -- the same dedicated callback POST /patterns uses, which checks edit_posts AND wp_block's create_posts capability (publish_posts). No gate logic is re-implemented. Tool_Executor::execute() gains dispatch cases + execute_list_binding_sources() (delegates to REST_Controller::get_binding_sources()) and execute_create_pattern() (delegates to REST_Controller::create_pattern(), passing input through as the JSON body -- Pattern_Manager::create_pattern() already validates title/content-blocks-XOR/sync_status/status, so no duplicate validation belongs here), both via the existing call_controller() pattern. Also fixes a bug in the red commit's own test: create_pattern's response key is `pattern_id`, not `id` (Pattern_Manager::create_pattern()'s actual return shape) -- caught by an "Undefined array key" error on the first green run, not a silent false pass. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* test(codex-review): pin all 5 confirmed P1 findings [red] Verified each of Codex's adversarial-review findings against source before writing anything (per the review request); all 5 confirmed real, no false positives: 1. Template_Manager::update_template() checks wp_insert_post()'s error but silently discards both wp_set_object_terms() calls that follow it (class-template-manager.php:316, :320). A pre_insert_term failure (or any other wp_set_object_terms() failure) leaves a published wp_template/ wp_template_part row with no wp_theme term -- orphaned: get_block_templates() can never find it again, yet it lingers in the database with whatever content the call went on to write, and the caller is told success:true. 2. Pattern_Manager::create_pattern() checks `'' === $title` on the RAW value before sanitize_text_field() runs (class-pattern-manager.php:747-748, applied only later at :817). A whitespace-only or pure-markup title (e.g. "<script></script>") passes the raw check and collapses to an empty post_title at insert time. 3. create_pattern() always returns a `core/block` synced-reference snippet (class-pattern-manager.php:858), even for sync_status:"unsynced" -- following it would re-link content the caller explicitly asked to keep non-propagating. 4. Tool_Executor::execute_list_block_types() never reads `include_supports` from $input, so ability callers always get stripped block types regardless of what they pass (class-tool-executor.php:150-159). 5. Tool_Executor::execute_list_patterns() never reads `category` and never forwards the REST response's `categories` vocabulary, so ability results diverge from REST/npm-MCP (class-tool-executor.php:191-225). New tests/Abilities/ToolExecutorParityAuditTest.php: a structural audit -- every property a manifest tool declares in its input_schema must appear, verbatim, in that tool's Tool_Executor::execute_<name>() method body. Running it against the unmodified codebase caught findings #4 and #5 exactly, plus a third divergence it correctly surfaced and I investigated separately: update_block's `block_name` is declared but never referenced. That one is NOT a wiring gap -- block_name selects which npm-MCP-server-side enricher (src/enrichers.ts) to run entirely client-side before only the already-enriched attributes/innerHTML are sent over the wire; the REST endpoint never accepts a block_name parameter, so Tool_Executor has nothing to forward it to. Documented as a real, separate, pre-existing gap (the Abilities path gets no automatic computed-field derivation the way npm-MCP callers do) via an explicit, commented exemption rather than silently dropped or expanded into a PHP-side enrichment port. New tests/Abilities/ListToolsAbilityParityTest.php: behavioral proof for findings #4/#5 specifically (the structural audit proves the argument name is *referenced*; these prove it changes the actual response). tests/REST/CreatePatternTest.php gains title-sanitization tests (#2, plus a control case: real text with incidental whitespace must still succeed) and reference/insert_hint tests (#3, plus a control case pinning the synced response shape is unchanged). tests/Templates/TemplateManagerWriteTest.php gains two rollback tests (#1): a pre_insert_term filter forces wp_theme term assignment to fail for a wp_template write, and a second test isolates the wp_template_part_area assignment specifically (by letting wp_theme succeed and only failing the area term) so both checks this issue asks for are independently proven. Confirmed red (full suite, not just the new files): $ composer test Tests: 1425, Assertions: 18456, Failures: 8. 1) CreatePatternTest::test_whitespace_only_title_is_rejected 2) CreatePatternTest::test_markup_only_title_that_sanitizes_to_empty_is_rejected 3) CreatePatternTest::test_unsynced_pattern_does_not_return_synced_reference 4) ListToolsAbilityParityTest::test_list_block_types_ability_forwards_include_supports 5) ListToolsAbilityParityTest::test_list_patterns_ability_forwards_category_filter_and_includes_categories 6) ToolExecutorParityAuditTest::test_every_manifest_input_property_is_referenced_by_its_tool_executor_method 7) TemplateManagerWriteTest::test_update_template_rolls_back_new_override_when_wp_theme_term_assignment_fails 8) TemplateManagerWriteTest::test_update_template_rolls_back_new_override_when_area_term_assignment_fails $ npm test Test Files 55 passed (55) -- no TS-side change needed for red yet; types/tool description updates for finding #3 land in the green commit. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * fix(codex-review): resolve all 5 confirmed P1 findings [green] Codex review of the merged range 602ae77...b089815 flagged five findings; all five verified genuine against source and fixed here: 1. Template_Manager::update_template() now checks the WP_Error return of both wp_set_object_terms() calls when creating a new override, deleting the freshly-inserted post and returning the error on failure instead of leaving an untraceable orphan post behind. 2. Pattern_Manager::create_pattern() now sanitizes `title` before the emptiness check, so a whitespace- or markup-only title that collapses to '' under sanitize_text_field() is rejected instead of silently creating a nameless pattern. 3. create_pattern()'s response now omits `reference` for an unsynced pattern (core/block is a live, propagating reference — wrong for a one-off) and instead returns `insert_hint` pointing callers at insert_pattern's synced:false path. TS types (CreatePatternResponse) and the create_pattern tool description are updated to match; the synced case is unchanged (backward compatible). 4. Tool_Executor::execute_list_block_types() now forwards `include_supports` on the Abilities/MCP-Adapter path, matching REST and the npm MCP server. 5. Tool_Executor::execute_list_patterns() now forwards `category` and includes `categories` in its response, matching REST and the npm MCP server. Findings 4 and 5 are the same REST-vs-Abilities parity bug class as prior fixes; ToolExecutorParityAuditTest (added in the red commit) statically compares every manifest-declared input property against each tool's execute_*() method body via Reflection, so a newly added argument that isn't forwarded on the Abilities path now fails CI instead of shipping silently. It documents two narrow, deliberate exemptions: tools that forward $input wholesale, and update_block's `block_name` (a client-side-only enrichment selector with no REST equivalent — a real, separate gap, not this bug class). All five findings are pinned by tests (see the red commit); none required a manual-only verification note. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…edit real templates — BLOCK-37 (#61) * test(templates): pin BLOCK-37 hybrid-theme template regression [red] staging.gravitykit.com's "gravitykit" theme is a hybrid: wp_is_block_theme() is false (no templates/index.html), but get_block_templates() finds real templates and parts via the templates/ and parts/ folders directly (1 template, 25 parts, verified live). Every template tool method gates on `! wp_is_block_theme()` instead of whether the id/type actually resolves, so it hides those templates from list_templates and blocks writes to parts that genuinely render on the site. Adds a "hybrid-theme" fixture (real templates/ and parts/ files, no templates/index.html or block-templates/index.html) plus: - get_templates() must list a hybrid theme's real templates/parts, no misleading "not a block theme" note - the note must stay absent for a real block theme's genuinely-empty result (regression pin — the note isn't "any empty result") - update_template()/reset_template() must succeed against a hybrid theme's resolvable part - a genuinely classic theme (nothing resolves) must keep the specific, actionable classic_theme 400 rather than regressing to a generic not_found — pins that the fix doesn't just delete the guard All 3 hybrid-theme behavioral assertions fail against current code; the classic-theme regression pin already passes (asserting current behavior we must not break). Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * fix(templates): gate on template resolution, not wp_is_block_theme() [green] get_templates() / update_template() / reset_template() all short-circuited on `! wp_is_block_theme()`, which is only true when a theme ships templates/index.html (or block-templates/index.html). A "hybrid" theme can have real, renderable templates and template parts without that specific file — staging.gravitykit.com's "gravitykit" theme is exactly this shape (1 template, 25 parts, wp_is_block_theme() false) — and the guard hid all of them from list_templates while blocking gated writes to parts that genuinely render on the site. - get_templates(): always queries get_block_templates(); the "not a block theme" note is now attached only when the result is empty AND wp_is_block_theme() is false, reworded so it no longer asserts nonexistence when the truth is "nothing matched this query." - update_template() / reset_template(): the wp_is_block_theme() 400 now fires only as a fallback when get_block_template( $id, $type ) fails to resolve at all AND the theme isn't a block theme — the primary gate is resolution, matching get_template()'s existing behavior. A genuinely classic theme (nothing resolves) keeps the same specific, actionable classic_theme 400 it always returned; a hybrid theme's resolvable part now succeeds instead of being rejected up front. Test infra: fixes a wp-phpunit gotcha the red commit's fixture tripped — search_theme_directories() memoizes its scan in a function-local static, so register_theme_directory() alone doesn't make a newly-registered root visible once anything else has already forced a scan; wp_clean_themes_cache() does. TS: list_templates' description no longer says "block theme" as if that were the qualifying condition. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
* test(coderabbit): pin CodeRabbit follow-up findings across PRs #52-61 [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 * fix(coderabbit): resolve CodeRabbit follow-up findings across PRs #52-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
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
…blocks Supersedes the closed PR #51 with a smaller, correct change. Extract the wp-cli runner into scripts/wp-exec.mjs and: - Local wp-env branch now invokes argv directly with no shell, so nothing in the command reaches a shell for interpretation. - SSH branch stays shell-based, since WP_CLI_SSH is a trusted operator prefix that may carry quoting, `&&`, or an sshpass wrapper. - Restore fail-loud behavior: spawnSync does not throw on failure the way execSync did, so non-zero exits and spawn errors are surfaced explicitly instead of returning empty output (which read as "0 posts"). Add tests/wp-exec.test.ts (5 tests) covering both branch shapes, the exit-status throw, the spawn-error rethrow, and the noise filter. Teeth verified: reverting the exit-status check turns the two error-handling tests red. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
Full Site Editing template tools (read + gated write), create_pattern with sync-status control, list_binding_sources, and richer block-type/pattern discovery, all also exposed as WordPress Abilities for the MCP Adapter. Bumps package.json, package-lock.json (both root entries), the plugin header and GK_BLOCK_MCP_VERSION, and readme.txt Stable tag, adds the 2.2.0 changelog entry, and rebuilds the tracked server bundle (baked version 2.2.0). Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
WalkthroughVersion 2.2.0 adds FSE template listing and editing, pattern creation, binding-source discovery, richer block metadata, HTTP method fallback, Abilities API permission routing, centralized WP-CLI execution, and related REST, MCP, and integration tests. ChangesMCP and REST feature surface
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPTools
participant REST
participant TemplateManager
participant Database
Client->>MCPTools: update_template(id, content or blocks)
MCPTools->>REST: POST /template
REST->>TemplateManager: update_template()
TemplateManager->>Database: create or update template override
Database-->>TemplateManager: override result
TemplateManager-->>REST: update response
REST-->>Client: template result
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
5 tasks
…t edit_posts (#65) * test(security): pin BLOCK-38 template-write cap gate [red] check_template_edit_permissions() currently passes on toggle-on AND (edit_posts OR edit_theme_options). Because the plugin performs the wp_insert_post()/wp_update_post() on wp_template/wp_template_part itself regardless of the caller's own caps, with the toggle on ANY contributor-or-above account (or a leaked low-privilege Application Password) can rewrite sitewide template chrome — header, footer, 404, archive, search. Primary red pin: test_update_template_route_403_for_contributor_even_with_toggle_on in TemplatesRestTest.php. Confirmed genuinely red against current shipped code via `git stash` of the (not-yet-written) implementation: 1) TemplatesRestTest::test_update_template_route_403_for_contributor_even_with_toggle_on Failed asserting that 200 is identical to 403. Companion pins added alongside it: - TemplatesRestTest.php: an editor (same edit_posts-only shape as a contributor) is equally denied; the dedicated agent role, after register_role() runs with the toggle on, holds the new Agent_Provisioner::TEMPLATE_EDIT_CAP and POST /template succeeds; toggling back off and re-running register_role() revokes the cap (asserted at the capability level on the role, not just behaviorally) and the route 403s again. The existing edit_theme_options-alone success test and the reset-template round-trip test are updated to use an edit_theme_options actor instead of editor/contributor, since those roles no longer have template-write permission — this is intentional, not incidental breakage. - TemplateAbilitiesTest.php: the same editor-denied case is pinned via the Abilities path (gk-block-mcp/update-template), proving the shared check_template_edit_permissions() callback can't be bypassed there. The persist/round-trip tests switch their actor to edit_theme_options for the same reason as above. - AgentProvisionerTest.php: register_role() grants TEMPLATE_EDIT_CAP on a fresh role when the toggle is on, and revokes it from an existing role when the toggle is later switched off — the one exception to the existing "additive only" re-assert loop, proven not to weaken test_register_role_strips_forbidden_caps_from_existing_role() (still green, unmodified). Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3 * fix(security): gate template writes on gk_block_mcp_edit_templates cap [green] Fixes BLOCK-38, a security-review finding that blocks the develop→main 2.2.0 release (#64). Fable's design: a plugin-owned primitive cap managed by the existing toggle, not a raw grant of edit_theme_options. - Agent_Provisioner::TEMPLATE_EDIT_CAP ('gk_block_mcp_edit_templates') is a new constant. Deliberately NOT added to forbidden_capabilities() — that denylist strips caps this class never grants; this one it grants and revokes on purpose. - register_role() computes the cap's value from Template_Manager::edits_enabled() before the existing gk/block-mcp/agent/caps filter runs, so operators can still override it. On an existing role, it's the one cap both ADDED (already covered by the additive re-assert loop) and — new — REMOVED when the toggle is off, scoped to only this single cap; the loop otherwise stays additive- only, and the forbidden-cap strip loop is untouched. Self-heals on every `init` (priority 99); a new `update_option_gk_block_api_template_edits` hook re-asserts immediately on settings save so grant/revoke doesn't wait for the next request. - check_template_edit_permissions() now passes on current_user_can('gk_block_mcp_edit_templates') OR current_user_can('edit_theme_options'), replacing the edit_posts clause. The toggle check still runs first (403 template_edits_disabled before any capability check). Docblock rewritten to document the actual vulnerability this closes, not the old (incorrect) edit_posts rationale. - Consent copy (class-settings-page.php) reworded to disclose the mechanism: granting the toggle changes what the Block MCP agent account itself can do to the theme layer, not just "let the assistant edit templates." Never grants raw edit_theme_options to the agent: that cap also opens core's /wp/v2/templates, /wp/v2/template-parts, /wp/v2/navigation, /wp/v2/global-styles, the Customizer, menus, and widgets, none of which the toggle is meant to reach — and it would be stripped by the forbidden-capabilities re-assert loop anyway. Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
wordpress-plugin/gk-block-mcp/includes/class-agent-provisioner.php (1)
178-184: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompound
has_cap()call inlined in theifcondition.As per coding guidelines, function-call results and compound expressions used in
if/while/ternary conditions should be assigned to a named variable first (short-circuit null/isset dereference guards are exempted; this isn't one).$existing->has_cap( self::TEMPLATE_EDIT_CAP )is called directly inside the&&. Note the identical pattern already exists in the unchanged loop just above this block, so this is consistent with existing style in the file rather than a new regression.🧹 Proposed fix
- if ( ! $caps[ self::TEMPLATE_EDIT_CAP ] && $existing->has_cap( self::TEMPLATE_EDIT_CAP ) ) { + $has_template_edit_cap = $existing->has_cap( self::TEMPLATE_EDIT_CAP ); + if ( ! $caps[ self::TEMPLATE_EDIT_CAP ] && $has_template_edit_cap ) { $existing->remove_cap( self::TEMPLATE_EDIT_CAP ); }🤖 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/class-agent-provisioner.php` around lines 178 - 184, Assign the result of $existing->has_cap( self::TEMPLATE_EDIT_CAP ) to a named boolean variable before the conditional, then use that variable with the existing !$caps[...] check in the if statement while preserving the current remove_cap behavior.Source: Coding guidelines
src/tools/templates.ts (1)
80-108: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd schema-level
oneOftoupdate_template, matchingcreate_pattern.
create_pattern's inputSchema enforces "exactly one of blocks/content" viaoneOffor schema-validating clients (with an explicit comment on why).update_templatehas the identical mutual-exclusivity contract but only enforces it at runtime inhandleTemplateTool— schema-only validators won't catch bad input before dispatch.♻️ Proposed fix
blocks: { type: 'array', description: 'Structured blocks to replace the template with. Mutually exclusive with content.', items: BLOCK_INPUT_SCHEMA, }, }, required: ['id'], + oneOf: [{ required: ['id', 'content'] }, { required: ['id', 'blocks'] }], }, },🤖 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 `@src/tools/templates.ts` around lines 80 - 108, Update the update_template inputSchema to add the same schema-level oneOf blocks/content constraint used by create_pattern, enforcing exactly one field while preserving the existing id and type properties and runtime validation.
🤖 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 `@AGENTS.md`:
- Line 147: Update the MCP tool inventory description in AGENTS.md so the
patterns group includes both insert_pattern and create_pattern, keeping the
existing grouping and wording unchanged otherwise.
In `@README.md`:
- Around line 340-349: Clarify the template editing documentation near the
per-block tool limitation: explicitly state that theme-file-only templates with
wp_id: null cannot be modified by per-block tools, while templates with numeric
database override IDs can be handled like normal posts by those tools. Preserve
the existing distinction between theme files and database overrides.
In `@wordpress-plugin/AGENTS.md`:
- Line 148: Update the template-edit permission documentation to reflect
authorization through gk_block_mcp_edit_templates while retaining the
edit_theme_options path: change wordpress-plugin/AGENTS.md lines 148-148,
AGENTS.md lines 143-143, and README.md lines 344-344, replacing references to
edit_posts or the “same edit_posts capability” wording with the shipped
capability behavior.
In `@wordpress-plugin/gk-block-mcp/includes/class-rest-controller.php`:
- Around line 356-359: Update the comment describing the template/template-part
POST route to state that access requires the toggle plus
Agent_Provisioner::TEMPLATE_EDIT_CAP or edit_theme_options, matching
check_template_edit_permissions(). Remove the outdated reference to edit_posts
while preserving the route and method context.
In `@wordpress-plugin/gk-block-mcp/includes/class-settings-page.php`:
- Around line 759-760: Update the settings checkbox rendering to pass the
persisted template-edits option value to checked(), rather than the filtered
result from Template_Manager::edits_enabled(). Retain $templates_enabled for the
warning display, and reuse that variable throughout the corresponding settings
section instead of invoking edits_enabled() again.
In `@wordpress-plugin/gk-block-mcp/includes/class-template-manager.php`:
- Around line 127-129: Assign the compound condition in the `wp_template_part`
query-building branch to a descriptive boolean variable, then use that variable
in the `if` statement. Preserve the existing `type` and non-empty `area` checks
and the `sanitize_key` assignment unchanged.
---
Nitpick comments:
In `@src/tools/templates.ts`:
- Around line 80-108: Update the update_template inputSchema to add the same
schema-level oneOf blocks/content constraint used by create_pattern, enforcing
exactly one field while preserving the existing id and type properties and
runtime validation.
In `@wordpress-plugin/gk-block-mcp/includes/class-agent-provisioner.php`:
- Around line 178-184: Assign the result of $existing->has_cap(
self::TEMPLATE_EDIT_CAP ) to a named boolean variable before the conditional,
then use that variable with the existing !$caps[...] check in the if statement
while preserving the current remove_cap behavior.
🪄 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: 96204985-090e-4bf8-830b-ded81d3330d4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (63)
AGENTS.mdREADME.mdpackage.jsonscripts/export-abilities-manifest.mjsscripts/highlight-code-blocks.mjsscripts/wp-exec.mjssrc/__tests__/helpers/mock-client.tssrc/__tests__/tools/discovery/list_binding_sources.test.tssrc/__tests__/tools/discovery/list_block_types.test.tssrc/__tests__/tools/discovery/list_patterns.test.tssrc/__tests__/tools/patterns/create_pattern.test.tssrc/__tests__/tools/templates/templates.test.tssrc/__tests__/unit/preferences/enrich-pattern-list.test.tssrc/agent-guide.tssrc/client.tssrc/coerce.tssrc/index.tssrc/tools/discovery.tssrc/tools/patterns.tssrc/tools/templates.tssrc/types.tstests/abilities-manifest.test.tstests/wp-exec.test.tswordpress-plugin/AGENTS.mdwordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjswordpress-plugin/gk-block-mcp/gk-block-mcp.phpwordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.jsonwordpress-plugin/gk-block-mcp/includes/class-abilities-registry.phpwordpress-plugin/gk-block-mcp/includes/class-agent-provisioner.phpwordpress-plugin/gk-block-mcp/includes/class-block-crud.phpwordpress-plugin/gk-block-mcp/includes/class-block-reader.phpwordpress-plugin/gk-block-mcp/includes/class-block-registry.phpwordpress-plugin/gk-block-mcp/includes/class-pattern-manager.phpwordpress-plugin/gk-block-mcp/includes/class-rest-controller.phpwordpress-plugin/gk-block-mcp/includes/class-settings-page.phpwordpress-plugin/gk-block-mcp/includes/class-template-manager.phpwordpress-plugin/gk-block-mcp/includes/class-tool-executor.phpwordpress-plugin/gk-block-mcp/phpstan-bootstrap.phpwordpress-plugin/gk-block-mcp/readme.txtwordpress-plugin/gk-block-mcp/tests/AGENTS.mdwordpress-plugin/gk-block-mcp/tests/Abilities/AbilitiesRegistryTest.phpwordpress-plugin/gk-block-mcp/tests/Abilities/BindingSourcesAbilityTest.phpwordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.phpwordpress-plugin/gk-block-mcp/tests/Abilities/ListToolsAbilityParityTest.phpwordpress-plugin/gk-block-mcp/tests/Abilities/TemplateAbilitiesTest.phpwordpress-plugin/gk-block-mcp/tests/Abilities/ToolExecutorParityAuditTest.phpwordpress-plugin/gk-block-mcp/tests/Block/PatternReferenceCountsTest.phpwordpress-plugin/gk-block-mcp/tests/Connect/AgentProvisionerTest.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/REST/RestSummaryTest.phpwordpress-plugin/gk-block-mcp/tests/REST/WriteHandlerErrorEnvelopeTest.phpwordpress-plugin/gk-block-mcp/tests/RestControllerTestCase.phpwordpress-plugin/gk-block-mcp/tests/Stress/PatternRecursionStressTest.phpwordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.phpwordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.phpwordpress-plugin/gk-block-mcp/tests/Templates/TemplatesRestTest.phpwordpress-plugin/gk-block-mcp/tests/fixtures/themes/hybrid-theme/parts/footer.htmlwordpress-plugin/gk-block-mcp/tests/fixtures/themes/hybrid-theme/style.csswordpress-plugin/gk-block-mcp/tests/fixtures/themes/hybrid-theme/templates/single.htmlwordpress-plugin/gk-block-mcp/tests/phpunit.xml
…/PATCH/DELETE (#67) * fix(client): fall back to X-HTTP-Method-Override when a host 405s PUT/PATCH/DELETE Some managed hosts (Convesio among them) front WordPress with a WAF that answers PUT, PATCH, and DELETE with a bare nginx 405 before the request reaches PHP. GET and POST pass, so reads and create_post kept working while every editing tool failed: update_block, update_blocks, delete_block, rewrite_post_blocks, update_post, and the two Yoast writers — eight call sites in all. The `?rest_route=` form is what the WAF singles out: the same PUT against the pretty `/wp-json/` path reaches WordPress fine. Switching to pretty permalinks was rejected because `?rest_route=` exists precisely so tool calls don't 404 on plain-permalink sites (see src/rest-url.ts). WordPress core honours `X-HTTP-Method-Override` on a POST, so a rejected request is replayed in that shape and the host is remembered for the life of the client, leaving later writes a single round-trip. The fallback is adaptive rather than blanket: hosts that accept the real verbs never see an override header, so the change is inert everywhere it isn't needed. The editing routes are registered as literal PUT / PATCH / DELETE rather than the EDITABLE alias, so a plain POST would not match them; the override header is what carries the intended verb through. Verified against production: update_post on a docs post returned 405 before and succeeds after. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq * test(client): cover method-override edge cases; document 405 in the README Broadens the fallback coverage from 6 cases to 16. New cases pin the parts a naive replay gets wrong: the request body, query parameters, and Basic auth all have to survive the replay, or a write silently lands empty or unauthenticated. Also pins the boundaries. The fallback must not engage for a 405 on POST (not an override verb) or for a non-405 failure on PATCH, since masking a real 403 behind a replay would hide a permission problem. When a host rejects the replay too, the error surfaces after exactly two attempts rather than looping, and the override survives a 429 backoff retry so the two interceptors compose. Reverting src/client.ts fails 11 of the 16; the 5 that still pass are the ones asserting the fallback stays dormant (three permissive-host cases, two scope guards), which is the signature we want. README: adds a 405 section to Error Codes explaining that the status comes from the host's firewall rather than the plugin, that the client replays such requests itself, and what to ask the host for when even the replay is rejected. Test counts were stale (257/335); verified counts are 885 Vitest and 1,440 PHPUnit. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq * fix(client): replay every rejected write, not only the first Concurrent writes all go out as real verbs, because none of them has seen a 405 yet. Gating the replay on `!useMethodOverride` meant the first rejection set the flag and replayed while the rest fell through, surfacing a 405 the caller could do nothing about. The flag was never what prevented recursion: a replay carries method `post`, which is not an override verb, so it cannot re-enter the branch. Requests issued after the flag is set are converted by the request interceptor and likewise arrive as `post`. Dropping the guard therefore only affects the race window. Caught by CodeRabbit on #67. The added test fails against the guarded client and passes here. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/assets/mcp-server/index.cjs`:
- Around line 40106-40116: Update the docblock above useMethodOverride to
replace the claim that only the first write pays for the rejected round-trip;
clarify that concurrent writes may each replay once before the sticky flag is
observed, while only later writes avoid the probe.
- Around line 40163-40167: Refine the fallback in the request handling around
edgeRejectedVerb so it only activates for an identifiable edge-generated 405 or
explicit configuration opt-in, not arbitrary PUT/PATCH/DELETE responses. Ensure
each request is replayed at most once, and set useMethodOverride only after the
method-override replay succeeds; leave it disabled when the replay fails.
🪄 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: 0a819c22-28b2-44e8-8ea5-429a8080f0e3
📒 Files selected for processing (5)
README.mdsrc/client.tstests/client-method-override.test.tswordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjswordpress-plugin/gk-block-mcp/readme.txt
🚧 Files skipped from review as they are similar to previous changes (2)
- wordpress-plugin/gk-block-mcp/readme.txt
- README.md
…38) (#66) * fix(security): BLOCK-38 CodeRabbit follow-ups — option lifecycle, cap-map safety, coverage Three follow-ups from PR #65's review: - Reassert the agent role on add/delete of gk_block_api_template_edits, not just update. delete_option() fires the generic `deleted_option` action (no `delete_option_{$option}` hook exists), and a fresh site's first enable routes through add_option() rather than update_option() — both previously left TEMPLATE_EDIT_CAP stale until the next `init`, so a settings reset didn't immediately revoke the write capability. - Guard the TEMPLATE_EDIT_CAP read in Agent_Provisioner::register_role() against the public gk/block-mcp/agent/caps filter unsetting the key entirely (rather than setting it false), which previously raised an undefined-array-key warning. Unset now revokes the cap, the safe default. - Add ability-layer coverage proving the dedicated block_mcp_agent role (holding TEMPLATE_EDIT_CAP, not edit_theme_options) can write templates through the Abilities API — the actual allow branch BLOCK-38 added, previously exercised only via edit_theme_options in this test file. Each of the first two fixes ships with a test that failed pre-fix (deleting the option left the cap granted; the filter unset triggered a PHP warning) and passes post-fix. composer test: 1488 tests green across all four suite configs (main, yoast, adapter, multisite). Claude-Session: https://claude.ai/code/session_01EddiNE6Q2mqXo5koeiVTpT * fix(settings): Release 2.2.0 CodeRabbit follow-ups — toggles persist stored value; template-cap docs Address the CodeRabbit review on PR #64 (Release 2.2.0). Behavioral fix (security): the Media uploads, Move-to-trash, template-editing, and MCP Adapter toggles rendered their checkbox from the post-filter effective value (*_enabled()), not the stored option. Because the checkbox is the form control, a gk/block-mcp/* filter overriding the stored value meant saving the settings page silently wrote the filtered value back into the option, flipping the admin's persisted security setting. Each checkbox now renders from the stored option; the divergence is surfaced by the existing "Heads up" override notice, which now reuses the already-computed *_enabled value instead of re-applying the filter. CodeRabbit flagged only template editing; the identical bug existed in all four toggles (uploads/trash shipped since 2.0.0/2.1.0), so all four are fixed. Regression test drives the real render_page() across every toggle (fails pre-fix, has teeth), and confirmed e2e against a live site. Docs/comments: - Template-write authorization is the dedicated gk_block_mcp_edit_templates capability or edit_theme_options (not edit_posts): corrected the stale rest-controller.php comment and the AGENTS.md/README.md permission docs. - README: reconcile per-block-tool applicability on templates (theme-file-only wp_id:null not writable; a numeric override is an ordinary post). - AGENTS.md: list create_pattern in the patterns MCP-tool inventory. - template-manager.php: assign the compound area condition to a named variable. Claude-Session: https://claude.ai/code/session_01EddiNE6Q2mqXo5koeiVTpT
2.2.0 has not shipped: there is no v2.2.0 tag, the newest release is 2.1.0, and the release PR is still open. So the method-override fix belongs in the 2.2.0 entry rather than a separate heading, and no version bump is needed. Also corrects the release date, which had gone stale while the release PR sat open. It needs another correction if the release slips past July 24. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq
Member
Author
|
The two findings against the method-override fallback are addressed in #68 (into Structured 405s no longer trigger the fallback. The route-collision risk is the sharp part of that finding: The flag no longer sticks on a failed replay. It now flips only after a replay succeeds, with a per-request marker carrying the override through. One replay per request was already guaranteed structurally rather than by a counter: a replay carries method The docblock claim about "only the first write" was stale after the concurrency fix and is reworded. |
…rm it works (#68) Two tightenings to the PUT/PATCH/DELETE fallback. Only an unstructured 405 now counts as an edge rejection. WordPress answers an unroutable method with 404 `rest_no_route`, so a 405 carrying a REST error body is a real answer from a route or a plugin, and replaying it as POST could reach a different route than the caller asked for: POST on the blocks collection inserts rather than replaces. Such a response is now surfaced untouched. The sticky flag is also set only after a replay succeeds. It previously flipped before the replay was attempted, so a host that rejects the override too was remembered as override-capable and every later write skipped the real-verb probe for a fallback already known to fail. A per-request marker carries the override through the replay so the flag no longer has to. Corrects the field docblock, which claimed only the first write pays the rejected round-trip. Concurrent writes have each already gone out as a real verb, so each pays its own; only later writes skip the probe. Verified against the WAF that prompted the fallback: PATCH, PUT, and DELETE all still complete through it. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq
…[ci skip] Drops the method-override bullet from Developer Updates. `WordPressBlockClient` is internal to the MCP server, so no integrator can reference it, and the bullet described the override header and its caching rather than a contract anyone can write code against. The user-visible outcome already lives in Fixed, which is also where the readme style guide puts a firewall-blocked-connection change. Narrows the settings-toggle fix to the toggles that shipped with the bug. Media uploads, Move to trash, and the MCP Adapter all existed in 2.1.0 or earlier; template editing arrives in this release, so its instance of the bug never reached a customer and is not a change worth reporting. Trims the firewall entry to the symptom and the resolution, and extends the summary to cover the release's two fixes rather than only its additions. Claude-Session: https://claude.ai/code/session_01Njh4D63XnZhsJHMbEU7vYq
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
wordpress-plugin/gk-block-mcp/tests/Connect/AgentProvisionerTest.php (1)
171-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep test docblocks focused on current contracts.
Remove “Before the fix,” regression-history, and implementation-history narration; retain only the behavior each test guarantees.
wordpress-plugin/gk-block-mcp/tests/Connect/AgentProvisionerTest.php#L171-L180: state that deleting the option immediately revokes the capability.wordpress-plugin/gk-block-mcp/tests/Connect/AgentProvisionerTest.php#L199-L209: state that first-time enabling immediately grants the capability.wordpress-plugin/gk-block-mcp/tests/Connect/AgentRoleTest.php#L172-L177: state that an unset capability-map entry revokes the capability safely.wordpress-plugin/gk-block-mcp/tests/Connect/SettingsPagePreferencesTest.php#L534-L545: state that checkbox state follows storage and divergence displays an override notice.As per coding guidelines, comments and docblocks must document present-day 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/Connect/AgentProvisionerTest.php` around lines 171 - 180, Rewrite the test docblocks to describe only current behavioral contracts, removing regression and implementation-history narration. In wordpress-plugin/gk-block-mcp/tests/Connect/AgentProvisionerTest.php lines 171-180, state that deleting the option immediately revokes the capability; lines 199-209, state that first-time enabling immediately grants it. In wordpress-plugin/gk-block-mcp/tests/Connect/AgentRoleTest.php lines 172-177, state that an unset capability-map entry safely revokes the capability. In wordpress-plugin/gk-block-mcp/tests/Connect/SettingsPagePreferencesTest.php lines 534-545, state that checkbox state follows storage and divergence displays an override notice.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/tests/Abilities/TemplateAbilitiesTest.php`:
- Around line 331-351: Before executing the update-template ability in this
test, explicitly inspect the registered Agent_Provisioner role and assert it has
TEMPLATE_EDIT_CAP while lacking edit_theme_options, manage_options,
unfiltered_html, and all delete capabilities. Keep these least-privilege
assertions before the ability execution so the test cannot pass if the role is
overprivileged.
- Around line 329-351: Update
test_update_template_ability_succeeds_via_dedicated_capability to preserve and
restore the preexisting Agent_Provisioner::ROLE state instead of unconditionally
calling remove_role(). Also reset the wp_template override created by the
successful update, using a finally/teardown path that runs on both success and
failure so later tests start with the original role and template state.
In `@wordpress-plugin/gk-block-mcp/tests/Connect/SettingsPagePreferencesTest.php`:
- Around line 558-571: Update the test using the temporary filter registered by
add_filter in the render_policy_html assertion flow: wrap the assertions in
try/finally and remove the same filter with remove_filter in the finally block,
ensuring cleanup occurs even when an assertion fails.
---
Nitpick comments:
In `@wordpress-plugin/gk-block-mcp/tests/Connect/AgentProvisionerTest.php`:
- Around line 171-180: Rewrite the test docblocks to describe only current
behavioral contracts, removing regression and implementation-history narration.
In wordpress-plugin/gk-block-mcp/tests/Connect/AgentProvisionerTest.php lines
171-180, state that deleting the option immediately revokes the capability;
lines 199-209, state that first-time enabling immediately grants it. In
wordpress-plugin/gk-block-mcp/tests/Connect/AgentRoleTest.php lines 172-177,
state that an unset capability-map entry safely revokes the capability. In
wordpress-plugin/gk-block-mcp/tests/Connect/SettingsPagePreferencesTest.php
lines 534-545, state that checkbox state follows storage and divergence displays
an override notice.
🪄 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: 31046250-e9cc-45b6-ab40-9729371bf745
📒 Files selected for processing (16)
AGENTS.mdREADME.mdsrc/client.tstests/client-method-override.test.tswordpress-plugin/AGENTS.mdwordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjswordpress-plugin/gk-block-mcp/gk-block-mcp.phpwordpress-plugin/gk-block-mcp/includes/class-agent-provisioner.phpwordpress-plugin/gk-block-mcp/includes/class-rest-controller.phpwordpress-plugin/gk-block-mcp/includes/class-settings-page.phpwordpress-plugin/gk-block-mcp/includes/class-template-manager.phpwordpress-plugin/gk-block-mcp/readme.txtwordpress-plugin/gk-block-mcp/tests/Abilities/TemplateAbilitiesTest.phpwordpress-plugin/gk-block-mcp/tests/Connect/AgentProvisionerTest.phpwordpress-plugin/gk-block-mcp/tests/Connect/AgentRoleTest.phpwordpress-plugin/gk-block-mcp/tests/Connect/SettingsPagePreferencesTest.php
🚧 Files skipped from review as they are similar to previous changes (11)
- wordpress-plugin/gk-block-mcp/includes/class-agent-provisioner.php
- wordpress-plugin/gk-block-mcp/gk-block-mcp.php
- wordpress-plugin/AGENTS.md
- AGENTS.md
- wordpress-plugin/gk-block-mcp/includes/class-settings-page.php
- README.md
- tests/client-method-override.test.ts
- src/client.ts
- wordpress-plugin/gk-block-mcp/includes/class-template-manager.php
- wordpress-plugin/gk-block-mcp/includes/class-rest-controller.php
- wordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjs
Comment on lines
+329
to
+351
| public function test_update_template_ability_succeeds_via_dedicated_capability() { | ||
| update_option( Template_Manager::ALLOW_TEMPLATE_EDITS_OPTION, '1' ); | ||
| Agent_Provisioner::register_role(); | ||
|
|
||
| wp_set_current_user( self::factory()->user->create( array( 'role' => Agent_Provisioner::ROLE ) ) ); | ||
|
|
||
| $marker = 'AGENT-CAP-MARKER-' . wp_rand(); | ||
| $result = wp_get_ability( 'gk-block-mcp/update-template' )->execute( | ||
| array( | ||
| 'id' => $this->theme . '//index', | ||
| 'content' => '<!-- wp:paragraph --><p>' . $marker . '</p><!-- /wp:paragraph -->', | ||
| ) | ||
| ); | ||
|
|
||
| $this->assertNotWPError( $result ); | ||
| $this->assertTrue( $result['success'] ); | ||
|
|
||
| $read = wp_get_ability( 'gk-block-mcp/get-template' )->execute( array( 'id' => $this->theme . '//index' ) ); | ||
|
|
||
| remove_role( Agent_Provisioner::ROLE ); | ||
|
|
||
| $this->assertNotWPError( $read ); | ||
| $this->assertStringContainsString( $marker, $read['content'] ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Restore shared role and template state after this test.
register_role() can update the preexisting canonical role, but Line 348 unconditionally deletes it. The successful write also leaves a wp_template override behind. Restore the prior role state and reset the created override in finally/teardown so later tests do not depend on execution order.
Based on supplied role-registration context, register_role() manages an existing canonical role in place.
🧰 Tools
🪛 PHPStan (2.2.5)
[error] 333-333: Call to an undefined static method TemplateAbilitiesTest::factory().
(staticMethod.notFound)
🤖 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/Abilities/TemplateAbilitiesTest.php`
around lines 329 - 351, Update
test_update_template_ability_succeeds_via_dedicated_capability to preserve and
restore the preexisting Agent_Provisioner::ROLE state instead of unconditionally
calling remove_role(). Also reset the wp_template override created by the
successful update, using a finally/teardown path that runs on both success and
failure so later tests start with the original role and template state.
Comment on lines
+331
to
+351
| Agent_Provisioner::register_role(); | ||
|
|
||
| wp_set_current_user( self::factory()->user->create( array( 'role' => Agent_Provisioner::ROLE ) ) ); | ||
|
|
||
| $marker = 'AGENT-CAP-MARKER-' . wp_rand(); | ||
| $result = wp_get_ability( 'gk-block-mcp/update-template' )->execute( | ||
| array( | ||
| 'id' => $this->theme . '//index', | ||
| 'content' => '<!-- wp:paragraph --><p>' . $marker . '</p><!-- /wp:paragraph -->', | ||
| ) | ||
| ); | ||
|
|
||
| $this->assertNotWPError( $result ); | ||
| $this->assertTrue( $result['success'] ); | ||
|
|
||
| $read = wp_get_ability( 'gk-block-mcp/get-template' )->execute( array( 'id' => $this->theme . '//index' ) ); | ||
|
|
||
| remove_role( Agent_Provisioner::ROLE ); | ||
|
|
||
| $this->assertNotWPError( $read ); | ||
| $this->assertStringContainsString( $marker, $read['content'] ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Assert the least-privilege precondition explicitly.
This passes if the agent role gains edit_theme_options even when TEMPLATE_EDIT_CAP is missing. Assert that the role has TEMPLATE_EDIT_CAP and lacks edit_theme_options, manage_options, unfiltered_html, and delete capabilities before executing the ability.
As per coding guidelines, “The dedicated agent role must remain least-privilege: do not grant delete capabilities, unfiltered_html, or manage_options by default.”
🧰 Tools
🪛 PHPStan (2.2.5)
[error] 333-333: Call to an undefined static method TemplateAbilitiesTest::factory().
(staticMethod.notFound)
🤖 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/Abilities/TemplateAbilitiesTest.php`
around lines 331 - 351, Before executing the update-template ability in this
test, explicitly inspect the registered Agent_Provisioner role and assert it has
TEMPLATE_EDIT_CAP while lacking edit_theme_options, manage_options,
unfiltered_html, and all delete capabilities. Keep these least-privilege
assertions before the ability execution so the test cannot pass if the role is
overprivileged.
Source: Coding guidelines
Comment on lines
+558
to
+571
| add_filter( $filter, $filter_forces ? '__return_true' : '__return_false' ); | ||
|
|
||
| $html = $this->render_policy_html(); | ||
|
|
||
| $this->assertSame( | ||
| $expect_checked, | ||
| $this->checkbox_is_checked( $html, $option ), | ||
| sprintf( 'the %s checkbox must reflect the stored option (%s), not the filter override', $option, $stored ) | ||
| ); | ||
| $this->assertStringContainsString( | ||
| '<code>' . $filter . '</code>', | ||
| $html, | ||
| 'a filter diverging from the stored value must surface the Heads-up override notice naming the filter' | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove each temporary override filter after the assertion.
The data-provider runs leave global filters registered, affecting later tests’ effective security settings. Use try/finally so cleanup also runs on assertion failure.
Proposed fix
- add_filter( $filter, $filter_forces ? '__return_true' : '__return_false' );
+ $callback = $filter_forces ? '__return_true' : '__return_false';
+ add_filter( $filter, $callback );
- $html = $this->render_policy_html();
+ try {
+ $html = $this->render_policy_html();
- $this->assertSame(
+ $this->assertSame(
$expect_checked,
$this->checkbox_is_checked( $html, $option ),
sprintf( 'the %s checkbox must reflect the stored option (%s), not the filter override', $option, $stored )
- );
- $this->assertStringContainsString(
+ );
+ $this->assertStringContainsString(
'<code>' . $filter . '</code>',
$html,
'a filter diverging from the stored value must surface the Heads-up override notice naming the filter'
- );
+ );
+ } finally {
+ remove_filter( $filter, $callback );
+ }📝 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
| add_filter( $filter, $filter_forces ? '__return_true' : '__return_false' ); | |
| $html = $this->render_policy_html(); | |
| $this->assertSame( | |
| $expect_checked, | |
| $this->checkbox_is_checked( $html, $option ), | |
| sprintf( 'the %s checkbox must reflect the stored option (%s), not the filter override', $option, $stored ) | |
| ); | |
| $this->assertStringContainsString( | |
| '<code>' . $filter . '</code>', | |
| $html, | |
| 'a filter diverging from the stored value must surface the Heads-up override notice naming the filter' | |
| ); | |
| $callback = $filter_forces ? '__return_true' : '__return_false'; | |
| add_filter( $filter, $callback ); | |
| try { | |
| $html = $this->render_policy_html(); | |
| $this->assertSame( | |
| $expect_checked, | |
| $this->checkbox_is_checked( $html, $option ), | |
| sprintf( 'the %s checkbox must reflect the stored option (%s), not the filter override', $option, $stored ) | |
| ); | |
| $this->assertStringContainsString( | |
| '<code>' . $filter . '</code>', | |
| $html, | |
| 'a filter diverging from the stored value must surface the Heads-up override notice naming the filter' | |
| ); | |
| } finally { | |
| remove_filter( $filter, $callback ); | |
| } |
🤖 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/Connect/SettingsPagePreferencesTest.php`
around lines 558 - 571, Update the test using the temporary filter registered by
add_filter in the render_policy_html assertion flow: wrap the assertions in
try/finally and remove the same filter with remove_filter in the finally block,
ensuring cleanup occurs even when an assertion fails.
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.
Release 2.2.0. Merging to
maintriggers CircleCI'sbuild_package_release(tagsv2.2.0, creates the GitHub release from the 2.2.0readme.txtchangelog, announces) andnpm-publish(publishes@gravitykit/block-mcp@2.2.0).What ships (net vs 2.1.0)
Added
list_templates,get_template) — see a block theme's templates/parts and whether a DB override shadows a theme file.update_template,reset_template) — off by default (optiongk_block_api_template_edits+ filtergk/block-mcp/templates/allow-edits); edits create a revertable DB override.create_pattern— synced or unsynced reusable patterns.list_binding_sources— registered block binding sources.Improved
list_block_typesreturnsstyles,parent,ancestor,allowed_blocks, and optionalinclude_supports.list_patternsaccepts acategoryfilter and returns the registeredcategories.New REST routes (
gk-block-api/v1)GET /templates,GET /template,POST /template,POST /template/reset,GET /binding-sources,POST /patterns.Provenance
15 commits (PRs #52–#63 plus the release bump). Reviewed by Codex (5 P1s fixed) and CodeRabbit (2 rounds); validated live on staging.gravitykit.com. All new functionality is TDD'd; the version bump is verified green (869 vitest, 4 PHP suites, byte-identical bundle baked at 2.2.0).
https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
💾 Build file (4241d5d).
Summary by CodeRabbit
create_pattern(synced/unsynced), including pattern-category filtering and returned category metadata.include_supports.