Skip to content

feat(abilities): wire list_binding_sources + create_pattern into Tool_Executor - #59

Merged
zackkatz merged 2 commits into
developfrom
feature/block-35-wire-list_binding_sources-create_pattern-into-tool_executor
Jul 23, 2026
Merged

feat(abilities): wire list_binding_sources + create_pattern into Tool_Executor#59
zackkatz merged 2 commits into
developfrom
feature/block-35-wire-list_binding_sources-create_pattern-into-tool_executor

Conversation

@zackkatz

@zackkatz zackkatz commented Jul 23, 2026

Copy link
Copy Markdown
Member

What changed

Wires list_binding_sources (BLOCK-32) and create_pattern (BLOCK-31, PR #57) into Tool_Executor — the exact class of gap Agent A flagged and I fixed for the templates group in #58, applied here to the two other tools that fell through the same crack: registered as abilities (manifest is data-driven, doesn't depend on execution support) but with no Tool_Executor::execute() case, so both 400'd "Unknown Block MCP tool" over the Abilities/MCP-Adapter transport regardless of the caller's permissions.

create_pattern had a second, more serious gap on top of that: its REST twin (POST /patterns) uses a dedicated permission callback, check_create_pattern_permissions()check_permissions() (base edit_posts) plus a check against the wp_block post type's create_posts capability, which WordPress core maps to publish_posts, not edit_posts. The manifest had create_pattern on the default edit_post permission bucket, which checks only the former. A Contributor (has edit_posts, lacks publish_posts) would have been let through the Abilities path even though the identical request 403s over REST.

Fix

  • scripts/export-abilities-manifest.mjs: new CREATE_PATTERN permission bucket → 'create_pattern'. list_binding_sources needed no manifest change — its readOnlyHint annotation already resolved to 'read', matching the other discovery reads.
  • Abilities_Registry::check_tool_permission(): new 'create_pattern' case delegating to REST_Controller::check_create_pattern_permissions() directly — the same method the REST route uses, no gate logic duplicated.
  • Tool_Executor::execute(): dispatch cases + execute_list_binding_sources() (delegates to Block_Registry::get_binding_sources() via the REST handler) and execute_create_pattern() (delegates to Pattern_Manager::create_pattern() via the REST handler, passing input straight through as the JSON body — the REST handler already validates title/content-blocks-XOR/sync_status/status, so nothing is duplicated).

Audit: anything else unfinished? (per the coordinator's ask before the Codex review)

Cross-checked every manifest tool name against Tool_Executor's dispatch cases after this fix: 33 manifest tools, 33 executor cases, zero missing, zero dead code. Also cross-checked every distinct permission_callback shape used across REST_Controller::register_routes() (check_permissions, check_create_pattern_permissions, check_template_edit_permissions, check_edit_permissions, check_upload_permissions, the inline manage_options closure) against Abilities_Registry::check_tool_permission()'s switch — every REST-side permission shape now has a matching Abilities-side branch. Nothing else in this class of bug (registered-but-unexecutable, or executable-but-under-permissioned) is currently outstanding. I did not attempt a full per-tool re-audit of every existing (already-wired) ability's permission correctness beyond this structural check — that's a larger undertaking better suited to the upcoming Codex review of the whole range.

TDD: red before green (commit history is the proof, not squashed)

  1. 2daf529 test(abilities): list_binding_sources + create_pattern execution/gate parity [red] — new tests/Abilities/BindingSourcesAbilityTest.php (registration, execution, subscriber denial, readonly annotation) + new tests/Abilities/CreatePatternAbilityTest.php (registration, execution creates a real wp_block post, subscriber denial, the Contributor gate-parity test, a direct pin of check_create_pattern_permissions() denying the same Contributor, annotation checks) + 2 new assertions in tests/abilities-manifest.test.ts. Confirmed genuinely red:
    $ vendor/bin/phpunit -c tests/phpunit.xml tests/Abilities/
    Tests: 163, Assertions: 403, Failures: 3.
    
    $ npm test -- tests/abilities-manifest.test.ts
    Tests  1 failed | 6 passed (7)
    
  2. bb66212 feat(abilities): wire list_binding_sources + create_pattern execution/gate parity [green] — the fix above. All 3 PHP failures + the 1 TS failure go green (also fixed a bug in my own red-commit test: create_pattern's response key is pattern_id, not id — caught by a PHP "Undefined array key" error on the first green run, not a silent pass).

Acceptance criteria

  • Tool_Executor::execute() cases for both tools, delegating to the same managers the REST routes use (Block_Registry::get_binding_sources(), Pattern_Manager::create_pattern()).
  • list_binding_sources = read parity with the other discovery reads (already true; confirmed, not assumed).
  • create_pattern = same cap as the REST route (check_permissions + wp_block create_posts) — proven live (see smoke below), not just in the SQLite test harness.
  • Strict TDD, visible red-green, no local squashing.

Gate output (real numbers, run in the worktree)

$ npm test
 Test Files  55 passed (55)
      Tests  859 passed (859)

$ npm run build
  dist/index.cjs  3.6mb
  Copied dist/index.cjs -> wordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjs

$ npx tsx scripts/export-abilities-manifest.mjs && git diff --exit-code wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json
Wrote 33 tools to .../tools.manifest.json
(no diff — exit 0)

$ composer test          (from wordpress-plugin/gk-block-mcp)
OK (1415 tests, 18636 assertions)
OK (28 tests, 82 assertions)     [yoast group]
OK (11 tests, 25 assertions)     [mcp-adapter group]
OK (5 tests, 37 assertions)      [multisite]

$ composer lint
FOUND 0 ERRORS AND 0 WARNINGS

$ composer analyze
[OK] No errors

Siteminter smoke — real MCP Adapter JSON-RPC transport (per the #58 method)

Fresh site blockmcp-d (WordPress 7.0.2; installed the WordPress MCP Adapter plugin from its GitHub Release v0.5.0, same as #58, to exercise the real transport rather than a wp-cli fallback). Plugin symlinked to the worktree; gk_block_api_abilities_enabled enabled.

tools/list — both tools present:

total tools: 30
binding-sources present: True
create-pattern present: True

tools/call list-binding-sources (admin) — real registered bindings sources from this WordPress install:

{"structuredContent":{"sources":[
  {"name":"core/pattern-overrides","label":"Pattern Overrides","uses_context":["pattern/overrides"]},
  {"name":"core/post-data","label":"Post Data","uses_context":["postId","postType"]},
  {"name":"core/post-meta","label":"Post Meta","uses_context":["postId","postType"]},
  {"name":"core/term-data","label":"Term Data","uses_context":["termId","taxonomy"]},
  {"name":"twentytwentyfive/format","label":"Post format name"}
]},"isError":false}

tools/call create-pattern (admin) — creates a real synced wp_block post:

{"structuredContent":{"pattern_id":4,"title":"MCP-ADAPTER-PATTERN-SMOKE","slug":"mcp-adapter-pattern-smoke","sync_status":"synced","edit_url":"http://localhost:8936/wp-admin/post.php?post=4&action=edit","reference":{"blockName":"core/block","attrs":{"ref":4}},"warnings":[]},"isError":false}

Verified independently via wp-cli: wp post get 4 --field=post_typewp_block.

Gate parity, live — a Contributor (created via wp user create ... --role=contributor) denied create-pattern with the exact REST-route message:

{"content":[{"type":"text","text":"Sorry, you are not allowed to create patterns."}],"isError":true}

...but the same Contributor CAN still call the read-only tool (proves the gate is scoped to the write, not an over-broad denial):

tools/call list-binding-sources as contributor: isError: False, source count: 5

Test fixtures (the pattern post, the contributor user) deleted before destroying the site; site destroyed after; confirmed no leftover containers.

Fixes BLOCK-35
https://linear.app/gravitykit/issue/BLOCK-35/wire-list-binding-sources-create-pattern-into-tool-executor-abilities

https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3

Summary by CodeRabbit

  • New Features

    • Added an ability to list available binding sources.
    • Added an ability to create synced patterns.
    • Synced pattern creation now returns the created pattern details.
  • Bug Fixes

    • Improved permission handling so pattern creation follows its dedicated authorization checks.
    • Restricted pattern creation for users without the required publishing capabilities.
    • Enforced read-only access for binding-source listings.
  • Tests

    • Added coverage for registration, successful execution, metadata, and permission enforcement for both abilities.

zackkatz added 2 commits July 22, 2026 19:54
… 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
…/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
@linear-code

linear-code Bot commented Jul 23, 2026

Copy link
Copy Markdown

BLOCK-35

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Adds MCP execution for listing binding sources and creating patterns. Manifest permissions now distinguish create_pattern, and the abilities registry delegates that check to the REST controller. New PHPUnit coverage validates registration, execution, permissions, and metadata.

Changes

Ability routing and authorization

Layer / File(s) Summary
Manifest permission mappings
scripts/export-abilities-manifest.mjs, tests/abilities-manifest.test.ts, wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json
Adds the create_pattern permission mapping and verifies manifest permissions for binding-source listing and pattern creation.
Tool execution and authorization
wordpress-plugin/gk-block-mcp/includes/class-tool-executor.php, wordpress-plugin/gk-block-mcp/includes/class-abilities-registry.php
Dispatches list_binding_sources and create_pattern to REST handlers and applies the dedicated create-pattern permission callback.
Ability behavior validation
wordpress-plugin/gk-block-mcp/tests/Abilities/*AbilityTest.php
Tests registration, successful execution, permission failures, capability parity, and readonly/destructive metadata for both abilities.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MCP Client
  participant Abilities Registry
  participant Tool Executor
  participant REST Controller
  participant WordPress

  MCP Client->>Abilities Registry: Execute ability
  Abilities Registry->>REST Controller: Check create_pattern permissions
  REST Controller-->>Abilities Registry: Return permission result
  Abilities Registry->>Tool Executor: Dispatch authorized tool
  Tool Executor->>REST Controller: Invoke create_pattern or get_binding_sources
  REST Controller->>WordPress: Create wp_block or retrieve sources
  WordPress-->>REST Controller: Return operation result
  REST Controller-->>Tool Executor: Return normalized response
  Tool Executor-->>MCP Client: Return ability result
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: wiring list_binding_sources and create_pattern into Tool_Executor.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/block-35-wire-list_binding_sources-create_pattern-into-tool_executor

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Checkov (3.3.8)
wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php (1)

85-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep test docblocks to current test contracts.

Remove issue/history references, off-tree pointers, implementation-proof narratives, and future-drift speculation.

  • wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php#L85-L94: describe the contributor-denial contract without referring to the issue.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/BindingSourcesAbilityTest.php#L17-L21: remove the reference to another test class and its docblock.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/BindingSourcesAbilityTest.php#L39-L44: state the expected response contract only.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/BindingSourcesAbilityTest.php#L55-L61: remove cross-test comparisons and gate rationale.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/BindingSourcesAbilityTest.php#L72-L76: state the readonly metadata assertion only.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php#L19-L23: remove the reference to another test class and its docblock.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php#L41-L48: state the successful creation contract only.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php#L66-L69: describe the permission denial without calling it the read branch.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php#L112-L118: state parity with the permission callback without speculative drift language.
  • wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php#L128-L131: state the annotation values without source-file references.
🤖 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/CreatePatternAbilityTest.php`
around lines 85 - 94, Update the test docblocks to describe only their current
contracts, removing issue history, cross-test or source-file references,
implementation rationale, and speculative drift language. In
wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php at
lines 19-23, 41-48, 66-69, 85-94, 112-118, and 128-131, document the relevant
test class contracts: successful creation, permission denial, contributor
denial, permission-callback parity, and annotation values. In
wordpress-plugin/gk-block-mcp/tests/Abilities/BindingSourcesAbilityTest.php at
lines 17-21, 39-44, 55-61, and 72-76, remove the extra docblock or
cross-test/gate rationale and state only the expected response and readonly
metadata contracts.

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.

Nitpick comments:
In `@wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php`:
- Around line 85-94: Update the test docblocks to describe only their current
contracts, removing issue history, cross-test or source-file references,
implementation rationale, and speculative drift language. In
wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php at
lines 19-23, 41-48, 66-69, 85-94, 112-118, and 128-131, document the relevant
test class contracts: successful creation, permission denial, contributor
denial, permission-callback parity, and annotation values. In
wordpress-plugin/gk-block-mcp/tests/Abilities/BindingSourcesAbilityTest.php at
lines 17-21, 39-44, 55-61, and 72-76, remove the extra docblock or
cross-test/gate rationale and state only the expected response and readonly
metadata contracts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 83cc3349-5aac-46dd-84ee-7139df1f266b

📥 Commits

Reviewing files that changed from the base of the PR and between 8f1a33f and bb66212.

📒 Files selected for processing (7)
  • scripts/export-abilities-manifest.mjs
  • tests/abilities-manifest.test.ts
  • wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json
  • wordpress-plugin/gk-block-mcp/includes/class-abilities-registry.php
  • wordpress-plugin/gk-block-mcp/includes/class-tool-executor.php
  • wordpress-plugin/gk-block-mcp/tests/Abilities/BindingSourcesAbilityTest.php
  • wordpress-plugin/gk-block-mcp/tests/Abilities/CreatePatternAbilityTest.php

@zackkatz
zackkatz merged commit b089815 into develop Jul 23, 2026
9 checks passed
@zackkatz
zackkatz deleted the feature/block-35-wire-list_binding_sources-create_pattern-into-tool_executor branch July 23, 2026 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant