Conversation
phpstan-bootstrap.php (placeholder constants for static analysis) was shipping in the build: the .distignore *.dist glob catches phpstan.neon.dist and phpcs.xml.dist, but this file ends in .php so it slipped through. It is dev-only (referenced solely by phpstan.neon.dist's bootstrapFiles, never loaded at runtime), so list it explicitly. .distignore is the single source of truth for both the CircleCI and GitHub Actions packaging paths, so this fixes both.
CircleCI's build_package_release owns releases (build + sign + announce to the
Release Manager, which updates EDD), as it did through 2.0.0. The GitHub Actions
'Create tag + release for the new version' step (added after 2.0.1) created the
v{version} tag/release on the main merge, racing CircleCI's create_release —
the collision aborted CircleCI before its announce, so EDD never updated (2.0.1
and 2.0.2 both stuck until pushed manually).
Restore build-plugin-zip.yml to its v2.0.1 shape: it still builds the zip,
uploads the artifact, updates the rolling 'latest' prerelease, and keeps the
'Tagged release' backstop that fires AFTER CircleCI pushes the v* tag (no race).
Only the v{version}-on-merge creation is removed, so CircleCI is the sole
creator of the version tag + release and its announce runs every release.
… null] The yoast_update_seo / yoast_bulk_update_seo input schema advertised noindex as type: ['boolean', 'null']. Anthropic's tool-schema validator rejects a 'null' member in a JSON Schema type array and returns a 400 for the ENTIRE tools/list request, which takes every tool on the server offline for Claude / Anthropic API clients, not just this field. Schema now advertises type: 'boolean'. The runtime handler is unchanged and still accepts an explicit null (tri-state true/false/null is preserved; covered by the existing 'noindex tri-state: null is preserved' test). Added a regression test asserting no field in the shared Yoast property set uses a type array containing 'null'. dist/ is a build artifact (npm run build / prepare) and is intentionally left out of this PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…undle; credit contributor Builds on @dallinchase's noindex scalar-type change. Verified the type-array rejection reproduces on Google Gemini's functionDeclarations (HTTP 400 "Proto field is not repeating, cannot start list"), NOT the Anthropic API (accepts ["boolean","null"] on stable 2023-06-01 + tool betas) or OpenAI (strict and non-strict). Re-attributes the field description to Gemini and corrects the tri-state wording: null resets to the post-type default (delete_post_meta), omit leaves the current value unchanged. Also: - Regenerate the git-tracked embedded bundle (CI verifies it byte-for-byte via .github/workflows/tests.yml + tests/packaging.test.ts). - Fix a TS2352 in the regression test (cast through unknown; tsconfig.build.json excludes tests so CI missed it, but tsc -p tsconfig.json flags it). - Credit Dallin Chase in the changelog for the original fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
npm audit: 0 vulnerabilities (clears the open Dependabot alerts).
Runtime (bundled into the shipped MCP server):
- axios ^1.7.9 -> ^1.16.0 (resolves several high/medium advisories)
- form-data and qs/hono refresh to patched versions via the lockfile
Dev:
- esbuild ^0.27.3 -> ^0.28.1; vitest ^3.2.4 -> ^3.2.6 (critical)
- vite refreshes to a patched 7.3.5 via the lockfile
- overrides: { esbuild: "$esbuild" } forces vite's nested esbuild to the patched
0.28.1, clearing the dev-server advisories without a major vitest bump
Bumps the MCP server (package.json) 2.0.1 -> 2.0.2 — 2.0.1 is already published,
and npm-publish.yml only ships when the version is new, so without the bump the
patched bundle would never reach npx users. The server versions independently of
the plugin.
Rebuilt the bundle (handshake version + patched axios/form-data) on top of the
current develop tree, so the committed embedded copy keeps the Yoast/Gemini fix
and gains the dependency patches. Verified: npm run build, tsc, the full vitest
suite (724 tests), and npm audit (0) all pass. Changelog entry added under
= develop =.
The Settings → Block MCP block-score and replacement tables stored a lossy
subset of preferences, and the first-ever save lost everything:
- Render and sanitize read the raw stored option instead of layering over
Preferences::get_defaults(), so the shipped defaults vanished from the tables
once any custom entry was stored (the runtime kept merging them, so the UI
contradicted enforcement).
- The sanitize callback only understood the form's indexed-row shape. Core
double-sanitizes an option on its first write — update_option() delegates to
add_option(), which sanitizes again — so the second pass received the
callback's own {namespace_scores, replacement_map} output, found no rows, and
collapsed the option to []. The first save silently wiped everything.
Render now uses the merged Preferences::get_preferences() view; sanitize layers
posted rows over the defaults and is idempotent (it accepts its own canonical
output, so a second pass is a no-op). The racy type-to-grow auto-row is replaced
with an explicit "Add row" button, and a score entered without a block-family
name now warns on save instead of being dropped silently.
# Conflicts: # wordpress-plugin/gk-block-mcp/readme.txt
…Add row PHPUnit (tests/Connect/SettingsPagePreferencesTest.php): render and sanitize layer over the shipped defaults; sanitize is idempotent; the first save of a brand-new option survives core's update_option()->add_option() double-sanitize via the real mechanism; a scored row with no name surfaces a settings error while a fully blank trailing row does not. Adds a committed admin-UI regression e2e harness (tests/e2e/, npm run test:e2e) that drives the live settings page: a namespace added via the Add row button persists across save, and a score with no name warns instead of vanishing.
- e2e: replace waitForLoadState('networkidle') (discouraged by Playwright) with
deterministic waits — wait for the settings-updated redirect before reloading,
and let the visible-warning assertion auto-wait across the form-submit
navigation.
- readme: move the "Add row" entry from Fixed to a new Improved section (a UI
enhancement, not a bug fix).
- tests: rewrite docblocks to present-tense contract language, dropping the
"before the fix" history per the comment guideline.
Skipped CodeRabbit's "@SInCE on new members": no PHPUnit test method in the
suite carries @SInCE — it's a production-API convention, not a test one.
chore(deps): patch npm security advisories (clears all 20 Dependabot alerts)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR updates settings/preferences behavior, adds compare-and-swap block writes, introduces WordPress Abilities integration, narrows the Yoast noindex schema, and revises release metadata, workflow, and docs. ChangesSettings Page and Preference Model
Block Write Safety and Cache Coherence
WordPress Abilities API
Yoast Schema
Release, Dependencies, and Documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…ent-discard fix (Fixed)
fix(settings): keep block-preference defaults and persist the first save
Stamps the queued develop work as 2.0.3: turns the = develop = changelog into = 2.0.3 on June 17, 2026 = with a release summary, and bumps the plugin Version header, GK_BLOCK_MCP_VERSION, and readme Stable tag from 2.0.2 to 2.0.3. The MCP server (package.json) stays 2.0.2 — it versions independently and has no new change this release; the 2.0.2 server publishes when this reaches main.
…nt attribution Addresses CodeRabbit on PR #34: - AGENTS.md: drop "== Upgrade Notice ==" from the version-bump list (four places, not five). Releases now source notes from the Changelog entry (per the earlier ci(release) change), and the plugin updates through Foundation, so the section is never surfaced. Scope @SInCE to shipped public members, with test files explicitly exempt — the whole suite leaves them un-annotated. - Correct the yoast_update_seo test comment to attribute the null-in-type-array rejection to AI clients like Gemini, matching the implementation comment. Skips CodeRabbit's "add Upgrade Notice" and "add @SInCE to the test class" — both contradict the established practice the guidance now documents.
The shipped preference table hardcoded a GravityKit-flavored opinion: a fixed namespace score list (filter/gravityforms preferred, stackable/ugb/jetpack branded legacy) plus a built-in Stackable/UGB replacement map. That contradicted sites not running those plugins and quietly imposed migrations no admin chose. Preferences are now derived from the site itself. - Ship opinion-free defaults: only `core` is preferred (90); every other namespace resolves to a neutral 50, and the replacement map ships empty. - Stop deep-merging `replacement_map`; it is the admin's authoritative list, taken verbatim, so a removed mapping stays removed. - Build the score table from a site-aware row universe: families registered by active plugins/themes, families the admin scored, and families found in published content. Store `namespace_scores` overrides-only so Reset reverts to default and storage never accumulates the neutral defaults. - Flag orphaned blocks in read responses (`preference.orphaned`) and in the score table when a namespace appears in content but has no registered provider. - Migrate existing sites on activation and lazily on `plugins_loaded`: bump the schema to 1.5.0, preserve saved preferences verbatim, and raise a one-time, dismissible notice explaining the new model for sites that had saved prefs. - Drop em-dashes from the settings copy.
…ocks - Add PreferencesMigrationTest for the 1.5.0 migration: schema stamping, idempotency, and the one-time notice flag for sites with saved preferences. - Rework SettingsPagePreferencesTest and PreferencesTest for the overrides-only store, verbatim replacement map, and site-aware row universe (Reset control, default marker, orphaned flag); drop the old "layer over shipped defaults" contracts. - Cover `preference.orphaned` for unregistered namespaces in BlockCrudTest. - Seed legacy/avoid namespaces explicitly in the shared bases now that the shipped defaults brand nothing legacy (BlockApiTestCase, PostManagerTest, RestSummaryTest).
Add the = develop = entry describing the site-aware, opinion-free preference model, preserved upgrade settings, the orphaned-block flag, and the new `preference.orphaned` developer field.
…st updates Every block write resolves refs and parses blocks from get_post(), which is served from the object cache. When a prior request or another worker has already changed post_content but this request still holds the old post in cache, the read-modify-write serialized the stale tree back to the database, silently undoing the concurrent change — a deleted block reappearing after the next edit, in the case that surfaced this. Freshen the post cache at the single write chokepoint (with_post_edit_context) so every write reads the database's current state. wp_cache_delete forces the operation's get_post() calls to re-read post_content; get_post re-primes the cache on the miss. Regression test reproduces the lost update through the real REST handlers for both update-by-ref and delete-by-ref: red without the freshen (the concurrently-deleted block returns), green with it. phpcs 0/0, PHPStan [OK], full suite green except 4 pre-existing Connect/Agent errors unrelated to this change. Claude-Session: https://claude.ai/code/session_01UEPJMRyhspoiwybaZZgREh
…d, never clobber The cache-freshen prevents object-cache stale reads on a single DB, but does not serialize two concurrent writers nor protect a read served by a lagging replica — in both, a write's in-memory tree predates the committed DB state and the unconditional wp_update_post overwrites the unseen change. Thread the exact pre-mutation post_content snapshot each writer parsed into save_post_content(); before wp_update_post, verify the database still holds it (a no-op touch pins HyperDB/LudicrousDB reads to the primary, then a byte-exact compare). On mismatch, fail closed with 409 edit_conflict and never write — so revisions + every save_post hook stay intact and no unseen change is clobbered. The lazy gk_ref persist (a raw $wpdb->update that bypasses wp_update_post) gets the same compare-and-swap folded into its WHERE. The architect-proposed `SET post_content = post_content` CAS is wrong on MySQL (affected-rows counts changed, not matched, so a no-op self-assign returns 0 on a match and would 409 every write); the byte-exact verify avoids that and is collation-exact too. GET_LOCK / FOR UPDATE / BINARY are deliberately excluded (MySQL-only, untestable on the SQLite harness) and a test pins they never emit. TDD: a stale-snapshot save and a stale ref-persist both return 409 / no-op and don't clobber or resurrect (red without the CAS); a non-conflicting write still fires save_post + creates a revision (pins wp_update_post preserved); no MySQL-only SQL reaches the DB. phpcs 0/0, PHPStan [OK], full suite green except 4 pre-existing Connect/Agent errors. Claude-Session: https://claude.ai/code/session_01UEPJMRyhspoiwybaZZgREh
…te auth, no test hook leaks - persist_ref_assignments: add a byte-exact compare-and-swap ahead of the collation WHERE, so a concurrent edit differing only by case/accent (which the default DB collation treats as equal) is detected, not clobbered. The WHERE snapshot stays as an atomic backstop for the verify-to-update window. - with_post_edit_context: freshen the post cache BEFORE the per-post permission check, so authorization is evaluated against the same current post (author/status) the mutation writes to, not a stale cached copy. - EditConflictCasTest: remove the save_post / query hooks after dispatch so they don't leak across tests in the same process; add a case-only conflict regression test (red against the collation-only compare, green with the byte-exact verify). phpcs 0/0, PHPStan [OK], full suite green except 4 pre-existing Connect/Agent errors. Claude-Session: https://claude.ai/code/session_01UEPJMRyhspoiwybaZZgREh
…ties (BLOCK-22)
Expose the block-tree operations as native WordPress Abilities so the official
MCP Adapter -- and any Abilities consumer (REST, JS, WP-CLI) -- can discover and
invoke them. A thin adapter: every ability delegates to the same service graph
the REST routes use, so behavior is identical whichever front door a caller uses.
Block_Abilities registers a `gk-block-mcp` ability category and five abilities:
get-page-blocks (readonly), update-block, insert-blocks, create-post, and
list-block-types (readonly), each with a JSON input schema, a capability-checked
permission callback (edit_post / edit_posts), readonly annotations, and an
execute callback delegating to Block_CRUD / Post_Manager / Block_Registry.
Feature-detected on function_exists('wp_register_ability') (WP 6.9+); a no-op on
the plugin's still-supported 6.0-6.8 cores, where the REST API + npm server
remain the universal path. Wired on `init` so the hooks attach before the
registry lazily initializes; registration is idempotent (guards on
wp_has_ability) so re-entrant firing is safe.
TDD RED->GREEN. 8 tests: registration + category, readonly annotations,
permission denial without caps, and end-to-end execute for all five abilities
(read returns the tree, update/insert persist, create emits canonical markup,
list returns types). Full suite green (1024 single-site + 24 yoast + 4
multisite), phpcs 0/0, phpstan OK.
Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
… write path Adversarial testing of the new abilities surfaced a pre-existing, engine-wide block-structure injection gap: wp_kses_post strips scripts but NOT block-comment delimiters, so innerHTML containing `<!-- /wp:x --><!-- wp:x -->` could break out of its block and materialize a phantom sibling block on the next parse. The insert path was already guarded; the update and mutate paths were not. Low severity (no XSS — scripts are still stripped), but a real content-integrity issue. Add Block_Writer::sanitize_inner_html() (wp_kses_post + strip `<!-- /?wp:… -->` delimiters) and route the three innerHTML sanitization sites through it: build_block_from_def (insert), apply_block_update_in_place (update), and Block_Mutator update-html (mutate). A block's structure lives in innerBlocks/innerContent, never in its raw innerHTML, so stripping delimiters is always safe — displayed block markup is entity-encoded, not a live comment. Regression tests in BlockCommentInjectionTest pin the update and mutate paths. Full suite green (1045 single-site + 24 yoast + 4 multisite), phpcs 0/0, phpstan OK. Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
…th adversarial hardening (BLOCK-22) Extend the Abilities bridge with two more abilities: - delete-block — remove one or more top-level blocks (Block_CRUD::delete_blocks), annotated destructive; the delete count is clamped to >= 1. - site-editor-context (readonly) — return the theme name plus flattened theme.json presets (colors, gradients, font sizes, spacing) via wp_get_global_settings, so an agent composes theme-aligned markup with preset slugs (has-primary-color) instead of hard-coded values. This is the WordPress-recommended prevention move (mirrors wpcom-mcp-site-editor-context). Plus a battery of adversarial tests proving the ability layer is robust: non-numeric/missing/negative/out-of-range inputs are rejected by schema validation or returned as graceful WP_Errors (never fatal), post_id 0 is denied by the capability check, script tags are stripped, content+blocks is rejected, and block-comment injection in innerHTML is neutralized (now backed by the write-path delimiter stripping in the preceding commit). 24 ability tests total. Full suite green (1045 single-site + 24 yoast + 4 multisite), phpcs 0/0, phpstan OK. Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
…ult on, opt-out) Registering Block MCP's operations as WordPress Abilities exposes them to the official MCP Adapter and the Abilities REST API -- a capability-gated but network-reachable surface. Give site owners explicit control, consistent with the plugin's other exposure switches (uploads kill-switch, trash toggle, post-type allow-list). Block_Abilities::is_enabled() gates register(): a '0'/'1' option (gk_block_api_abilities_enabled, default '1' = on) overridable via the gk/block-mcp/abilities/enabled filter (mirrors allow-trash). A checkbox on Settings -> Block MCP turns it off; the option is registered in the settings group, cleared on Reset, and shows a "filter is overriding" notice like the other toggles. Default ON keeps the MCP-Adapter integration working out of the box; off removes registration entirely (no Abilities/REST/Adapter surface). TDD: 4 tests (default-on, option-off, filter-off, register no-op when disabled). Full suite green (1049 single-site + 24 yoast + 4 multisite), phpcs 0/0, phpstan OK. Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
…sert-blocks CodeRabbit flagged that update-block and insert-blocks left the `destructive` annotation at the core null default while create-post and delete-block declare it explicitly. Make all four explicit — but with the spec-correct value, not the "both false" CodeRabbit proposed: per the MCP destructiveHint spec, false means "additive only", so insert-blocks (adds blocks) is false but update-block (overwrites a block's content) is true (it already resolved to true via the null default). delete-block stays true, create-post stays false. Regression test pins the four write abilities' destructive values. Full suite green (1050 single-site), phpcs 0/0, phpstan OK. Skipped CodeRabbit's @SInCE findings: the block-engine subsystem these files belong to does not annotate methods with @SInCE (class-block-writer.php and class-block-mutator.php carry zero; block-crud/reader/post-manager/term-manager one each, none per-method). The new filter already carries @SInCE 2.1.0, which is the established pattern. Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
…al REST server StaleCacheLostUpdateTest called controller methods directly, bypassing the auth/dispatch chain. Route the by-ref update/delete through rest_get_server()->dispatch() (registering routes via rest_api_init in set_up), matching AgentRestCapabilityTest and the project convention of exercising the real mechanism, not a bare method call. Dispatch enforces the route's ref regex (blk_[a-f0-9]+), so the fixtures now use production-shaped hex refs. No production change. Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
…a portable CAS save_post_content verified the snapshot (SELECT + compare) then wrote unconditionally via wp_update_post, leaving a window where a writer that committed between the verify and the write was silently clobbered — the simultaneous-writer case a bare verify cannot cover. persist_ref_assignments already closed this with an atomic conditional UPDATE; extend the same portable pattern to save_post_content: UPDATE wp_posts SET post_content = %s WHERE ID = %d AND post_content = %s A simultaneous writer that already moved the row matches 0 rows -> 409. The object cache is primed with the pre-write post so wp_update_post still runs the revision + save_post/clean_post_cache hooks with the correct $before. No MySQL-only locking (FOR UPDATE / GET_LOCK / BINARY) -- identical on the SQLite harness and MySQL, and it satisfies the existing portability guard (test_write_path_emits_no_mysql_only_sql). TDD: test_save_fails_closed_on_commit_between_verify_and_persist injects a concurrent commit between the verify and the swap (red without the CAS, the write clobbered the winner; green with it). Also converts EditConflictCasTest to real REST dispatch -- the test-fidelity half of the same review finding. Full suite green (1024 single-site + 24 yoast + 4 multisite), phpcs 0/0, phpstan OK. Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
…st-update fix(write): bulletproof block writes against lost updates (cache-freshen + optimistic-concurrency CAS)
…egister-block-mcp-operations-as-wordpress-69-abilities # Conflicts: # wordpress-plugin/gk-block-mcp/readme.txt
…k-mcp-operations-as-wordpress-69-abilities Register Block MCP operations as WordPress 6.9 Abilities (BLOCK-22)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wordpress-plugin/gk-block-mcp/includes/class-block-reader.php (1)
232-244: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRe-read disk state when ref persistence loses the CAS race.
With
$fresh_contentpassed as the expected snapshot,persist_ref_assignments()can now returnfalseon an edit conflict. The current branch only invalidates the cache;format_blocks()still returns the in-memory$blockscontaining refs that were not persisted, so follow-up by-ref writes can target IDs that do not exist on disk.Proposed fix
if ( $persisted ) { // Persist succeeded — re-warm cache from authoritative DB content. $this->invalidate( $post_id ); $new_content = (string) get_post_field( 'post_content', $post_id ); $this->parse_cache[ $post_id . ':' . md5( $new_content ) ] = $blocks; } else { - // Persist failed (read-only DB, replica lag, broken column). Refs - // exist only in $blocks, not on disk. Caching them would surface - // stable-looking refs in the response that the next write-by-ref - // would reject as ref_stale. Drop the cache for this post so the - // next read re-parses disk truth and tries again. + // Persistence does not commit; return current disk truth. $this->invalidate( $post_id ); + $current_content = (string) get_post_field( 'post_content', $post_id ); + $current_blocks = parse_blocks( $current_content ); + $current_is_array = is_array( $current_blocks ); + $blocks = $current_is_array ? $current_blocks : array(); + $current_key = (int) $post_id . ':' . md5( $current_content ); + $this->parse_cache[ $current_key ] = $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 `@wordpress-plugin/gk-block-mcp/includes/class-block-reader.php` around lines 232 - 244, When persist_ref_assignments() returns false due to a conflict, the else branch only invalidates the cache but the function still returns the in-memory $blocks containing unpersisted refs, causing subsequent write-by-ref operations to target non-existent ref IDs. In the else branch where persistence fails, after calling invalidate(), re-read the actual post content from disk using get_post_field(), parse it to get the blocks that actually exist on disk, and cache those parsed blocks instead of the failed $blocks to ensure follow-up operations target refs that actually exist on disk.
🧹 Nitpick comments (1)
wordpress-plugin/gk-block-mcp/tests/REST/StaleCacheLostUpdateTest.php (1)
36-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider consolidating duplicated test helpers.
The helpers
block(),with_ref(),commit_behind_cache(), andpersisted_content()are duplicated fromEditConflictCasTest.php. Extracting these to a shared trait or base test case would reduce duplication.♻️ Possible approach: extract to a shared trait
Create
tests/REST/ConcurrencyTestHelpers.php:<?php trait ConcurrencyTestHelpers { private function block( string $name, array $attrs, string $html ): array { /* ... */ } private function with_ref( array $block, string $ref ): array { /* ... */ } private function commit_behind_cache( int $post_id, array $blocks ): void { /* ... */ } private function persisted_content( int $post_id ): string { /* ... */ } }Then use in both test classes:
class EditConflictCasTest extends RestControllerTestCase { use ConcurrencyTestHelpers; // ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-mcp/tests/REST/StaleCacheLostUpdateTest.php` around lines 36 - 101, Extract the duplicated test helper methods block(), with_ref(), commit_behind_cache(), and persisted_content() from StaleCacheLostUpdateTest.php into a shared trait file. Create a new trait called ConcurrencyTestHelpers in the tests/REST directory, move these four private methods into that trait, and then add a use statement to both StaleCacheLostUpdateTest and EditConflictCasTest to use the trait instead of maintaining duplicate implementations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wordpress-plugin/gk-block-mcp/includes/class-block-writer.php`:
- Around line 345-354: The issue is in the query result handling within the CAS
operation around the wpdb->query() call and the subsequent error check. Before
returning the edit_conflict error on lines 353-354, you need to check whether
$new_content equals $expected. If they are equal, the UPDATE returning 0
affected rows is idempotent (the desired state is already in place) and should
not be treated as a conflict. Only treat this as an edit_conflict error when the
actual content has genuinely changed between the SELECT verification and this
UPDATE attempt. Additionally, ensure you distinguish between a database error
(false return from wpdb->query) and a legitimate conflict where affected rows is
0 but the values differ.
---
Outside diff comments:
In `@wordpress-plugin/gk-block-mcp/includes/class-block-reader.php`:
- Around line 232-244: When persist_ref_assignments() returns false due to a
conflict, the else branch only invalidates the cache but the function still
returns the in-memory $blocks containing unpersisted refs, causing subsequent
write-by-ref operations to target non-existent ref IDs. In the else branch where
persistence fails, after calling invalidate(), re-read the actual post content
from disk using get_post_field(), parse it to get the blocks that actually exist
on disk, and cache those parsed blocks instead of the failed $blocks to ensure
follow-up operations target refs that actually exist on disk.
---
Nitpick comments:
In `@wordpress-plugin/gk-block-mcp/tests/REST/StaleCacheLostUpdateTest.php`:
- Around line 36-101: Extract the duplicated test helper methods block(),
with_ref(), commit_behind_cache(), and persisted_content() from
StaleCacheLostUpdateTest.php into a shared trait file. Create a new trait called
ConcurrencyTestHelpers in the tests/REST directory, move these four private
methods into that trait, and then add a use statement to both
StaleCacheLostUpdateTest and EditConflictCasTest to use the trait instead of
maintaining duplicate implementations.
🪄 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: e09e54b6-a991-498e-bd0a-c8db9fad1932
📒 Files selected for processing (8)
wordpress-plugin/gk-block-mcp/includes/class-block-crud.phpwordpress-plugin/gk-block-mcp/includes/class-block-mutator.phpwordpress-plugin/gk-block-mcp/includes/class-block-reader.phpwordpress-plugin/gk-block-mcp/includes/class-block-writer.phpwordpress-plugin/gk-block-mcp/includes/class-rest-controller.phpwordpress-plugin/gk-block-mcp/readme.txtwordpress-plugin/gk-block-mcp/tests/REST/EditConflictCasTest.phpwordpress-plugin/gk-block-mcp/tests/REST/StaleCacheLostUpdateTest.php
✅ Files skipped from review due to trivial changes (1)
- wordpress-plugin/gk-block-mcp/readme.txt
Brings the lost-update CAS (#36) and WordPress 6.9 Abilities + settings toggle (#37) into the site-aware preferences branch. Conflicts resolved: - class-settings-page.php render() vars: keep the site-aware preferences view (stored overrides, not merged defaults) AND the abilities toggle vars. - readme.txt: combined changelog (Added: abilities; Improved: site-aware preferences; Security: innerHTML hardening; Fixed: lost-update + CAS; Developer Updates: preference.orphaned). Settings regression guards added in SettingsPageTabsTest, the reset guard proven to have teeth: handle_reset() clears every UI-managed option including the abilities toggle and the prefs-upgrade notice; register_settings() registers all checkbox toggles; the policy tab renders the abilities toggle. Full suite green: 1086 single-site + 24 yoast + 5 multisite, phpcs 0/0, phpstan OK. Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wordpress-plugin/gk-block-mcp/includes/class-settings-page.php (1)
319-330: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign the Remove controls with default re-layering.
Defaults are always merged back into
namespace_scoresandreplacement_map, and the page now renders that merged view. As a result, clicking Remove on a shipped default row appears to work until save, then the row reappears. Hide/disable Remove for default-backed rows, or add explicit tombstones if defaults are meant to be removable.Proposed UI-only fix when defaults are non-removable
- $merged_prefs = ( new Preferences() )->get_preferences(); + $default_prefs = Preferences::get_defaults(); + $merged_prefs = ( new Preferences() )->get_preferences(); $namespace_scores = $merged_prefs['namespace_scores']; $replacement_map = $merged_prefs['replacement_map'];$ns = $ns_keys[ $ns_index ]; $score = $namespace_scores[ $ns ]; + $ns_is_default = array_key_exists( $ns, $default_prefs['namespace_scores'] ); ?> @@ <td> - <button type="button" class="components-button is-link is-destructive gk-block-mcp-remove-row" aria-label="<?php echo esc_attr( sprintf( /* translators: %s: block family name */ __( 'Remove %s', 'gk-block-mcp' ), (string) $ns ) ); ?>"><?php esc_html_e( 'Remove', 'gk-block-mcp' ); ?></button> + <?php if ( ! $ns_is_default ) : ?> + <button type="button" class="components-button is-link is-destructive gk-block-mcp-remove-row" aria-label="<?php echo esc_attr( sprintf( /* translators: %s: block family name */ __( 'Remove %s', 'gk-block-mcp' ), (string) $ns ) ); ?>"><?php esc_html_e( 'Remove', 'gk-block-mcp' ); ?></button> + <?php else : ?> + <span class="description"><?php esc_html_e( 'Default', 'gk-block-mcp' ); ?></span> + <?php endif; ?> </td>$from = $rm_keys[ $rm_index ]; $to = $replacement_map[ $from ]; + $rm_is_default = array_key_exists( $from, $default_prefs['replacement_map'] ); ?> @@ <td> - <button type="button" class="components-button is-link is-destructive gk-block-mcp-remove-row" aria-label="<?php echo esc_attr( sprintf( /* translators: %s: legacy block name */ __( 'Remove %s', 'gk-block-mcp' ), (string) $from ) ); ?>"><?php esc_html_e( 'Remove', 'gk-block-mcp' ); ?></button> + <?php if ( ! $rm_is_default ) : ?> + <button type="button" class="components-button is-link is-destructive gk-block-mcp-remove-row" aria-label="<?php echo esc_attr( sprintf( /* translators: %s: legacy block name */ __( 'Remove %s', 'gk-block-mcp' ), (string) $from ) ); ?>"><?php esc_html_e( 'Remove', 'gk-block-mcp' ); ?></button> + <?php else : ?> + <span class="description"><?php esc_html_e( 'Default', 'gk-block-mcp' ); ?></span> + <?php endif; ?> </td>Also applies to: 513-515, 1148-1148, 1201-1201
🤖 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-settings-page.php` around lines 319 - 330, The Remove controls on default-backed rows in namespace_scores and replacement_map are not disabled or hidden, even though defaults are always re-merged during save via Preferences::get_defaults(), causing removed defaults to reappear. You need to identify which rows come from the defaults by comparing the current data with Preferences::get_defaults(), then disable or hide the Remove button for those default-backed rows in the UI rendering. Apply this logic consistently across all affected locations mentioned in the comment (lines 513-515, 1148-1148, and 1201-1201) to prevent users from attempting to remove rows that will be restored after save.
🧹 Nitpick comments (1)
wordpress-plugin/gk-block-mcp/includes/class-settings-page.php (1)
49-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRefresh comments to describe the current preferences flow.
Line 513 now reads merged preferences through
Preferences::get_preferences(), so the constructor note is stale. The sanitizer comment should also avoid bug-history wording like “a row a JS race swallowed” and state the current contract directly.Proposed wording cleanup
- * Defaults are read directly via `Preferences::get_defaults()` - * in the renderer — no Preferences instance needed here. + * The renderer reads merged preferences through + * `Preferences::get_preferences()`.- // Layer the posted rows over the shipped defaults so a partial submission - // — hidden defaults, or a row a JS race swallowed — can never erase them - // from storage. Posted values win; missing defaults are restored. This - // keeps the option consistent with the runtime merge in - // Preferences::get_preferences() rather than persisting a lossy subset. + // Layer posted rows over the shipped defaults so partial submissions + // restore shipped keys while preserving explicit posted overrides. This + // keeps the option consistent with the runtime merge in + // Preferences::get_preferences().As per coding guidelines, “Comments and docblocks must document what the code does today as standalone documentation; never use them as journals of how code was written.” Based on learnings, “Comments are public artifacts: present-tense behavior + hard contracts only; no history/journal.”
Also applies to: 319-323, 513-515
🤖 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-settings-page.php` around lines 49 - 51, Update the docblock comments in the class-settings-page.php file to document the current preferences flow as it exists today rather than historical implementation details. Remove stale information from the comment block at lines 49-51 that references defaults being read directly via Preferences::get_defaults(), since line 513 now reads merged preferences through Preferences::get_preferences(). Similarly, update the sanitizer comment section at lines 319-323 and 513-515 to remove bug-history wording (such as references to race conditions like "a row a JS race swallowed") and instead state the current contract and behavior in clear, present-tense language. Ensure all comments describe what the code does now as standalone documentation without journal-style explanations of past implementation changes.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wordpress-plugin/gk-block-mcp/includes/class-block-abilities.php`:
- Around line 375-407: Add the `@since 2.0.3` annotation to the PHPDoc comments
for the three permission callback methods: can_edit_post_input, can_create, and
can_read. Each method's documentation block should include a line with `@since
2.0.3` to document when these methods were introduced.
- Around line 480-504: The execute_site_editor_context() method is missing a
`@since` annotation in its DocBlock. Add the `@since` 2.0.3 annotation to the
method's DocBlock comment block, placing it after the `@return` annotation to
follow project conventions. Note that private helper methods like
global_setting() and flatten_presets() correctly omit `@since` tags per project
standards, but public execute callbacks require this annotation.
- Around line 69-82: The public methods __construct() and is_available() in the
Block_Abilities class are missing required `@since` annotations. Add a docblock
with `@since` 2.0.3 annotation to the __construct() method (which currently has no
docblock), and add the `@since` 2.0.3 tag to the existing docblock of the
is_available() method to comply with coding guidelines.
- Around line 95-108: The is_enabled() method has incorrect documentation
version information. Update the `@since` version in the filter hook documentation
from 2.1.0 to 2.0.3 for the gk/block-mcp/abilities/enabled filter, and add a
`@since` 2.0.3 annotation to the is_enabled() method itself in its docblock to
properly document when this method was introduced.
- Around line 118-166: The four public methods register(), register_category(),
register_abilities(), and ability_names() in the class-block-abilities.php file
are missing `@since` annotations in their docblock comments. Add `@since` 2.0.3 to
the docblock of each of these four methods to indicate when they were introduced
in the codebase.
- Around line 411-478: All six public execute callback methods
(execute_get_page_blocks, execute_update_block, execute_insert_blocks,
execute_create_post, execute_list_block_types, and execute_delete_block) are
missing `@since` version annotations in their PHPDoc documentation blocks. Add
`@since` 2.0.3 annotation to each method's PHPDoc comment block, placing it after
the `@return` annotation to document when these methods were introduced.
- Around line 22-40: The Block_Abilities class and its public constants are
missing required `@since` version annotations in their docblocks. Add `@since
2.0.3` tags to the class docblock at the top of the Block_Abilities class
definition, and add individual `@since 2.0.3` docblock annotations to each of
the three public constants: NAMESPACE_PREFIX, CATEGORY, and ENABLED_OPTION. Each
constant should have its own docblock with the `@since` tag included alongside its
description.
- Around line 427-443: The execute_insert_blocks method lacks validation for
negative position values, whereas execute_update_block and execute_delete_blocks
properly reject negative indices with a 400 error. Add explicit negative
position validation to execute_insert_blocks to check if the position parameter
(after extracting it from the input array) is negative, and return a WP_Error
with a 400 status code if it is, ensuring consistency with how the other block
mutation methods handle invalid indices.
In `@wordpress-plugin/gk-block-mcp/tests/Security/BlockCommentInjectionTest.php`:
- Around line 210-225: Both test methods lack explicit success assertions after
their write operations. In test_update_block_strips_injected_delimiters, add an
assertion after the update_block call to verify the operation succeeded without
errors. Similarly, in test_mutate_update_html_strips_injected_delimiters, add an
assertion after the mutate call to verify it succeeded. This ensures the tests
fail if the write operations fail, rather than silently passing due to unchanged
content still containing the expected delimiter count.
---
Outside diff comments:
In `@wordpress-plugin/gk-block-mcp/includes/class-settings-page.php`:
- Around line 319-330: The Remove controls on default-backed rows in
namespace_scores and replacement_map are not disabled or hidden, even though
defaults are always re-merged during save via Preferences::get_defaults(),
causing removed defaults to reappear. You need to identify which rows come from
the defaults by comparing the current data with Preferences::get_defaults(),
then disable or hide the Remove button for those default-backed rows in the UI
rendering. Apply this logic consistently across all affected locations mentioned
in the comment (lines 513-515, 1148-1148, and 1201-1201) to prevent users from
attempting to remove rows that will be restored after save.
---
Nitpick comments:
In `@wordpress-plugin/gk-block-mcp/includes/class-settings-page.php`:
- Around line 49-51: Update the docblock comments in the class-settings-page.php
file to document the current preferences flow as it exists today rather than
historical implementation details. Remove stale information from the comment
block at lines 49-51 that references defaults being read directly via
Preferences::get_defaults(), since line 513 now reads merged preferences through
Preferences::get_preferences(). Similarly, update the sanitizer comment section
at lines 319-323 and 513-515 to remove bug-history wording (such as references
to race conditions like "a row a JS race swallowed") and instead state the
current contract and behavior in clear, present-tense language. Ensure all
comments describe what the code does now as standalone documentation without
journal-style explanations of past implementation changes.
🪄 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: 707fde20-ee32-45bc-8f83-3ede04c46df3
📒 Files selected for processing (8)
wordpress-plugin/gk-block-mcp/gk-block-mcp.phpwordpress-plugin/gk-block-mcp/includes/class-block-abilities.phpwordpress-plugin/gk-block-mcp/includes/class-block-mutator.phpwordpress-plugin/gk-block-mcp/includes/class-block-writer.phpwordpress-plugin/gk-block-mcp/includes/class-settings-page.phpwordpress-plugin/gk-block-mcp/readme.txtwordpress-plugin/gk-block-mcp/tests/Integrations/BlockAbilitiesTest.phpwordpress-plugin/gk-block-mcp/tests/Security/BlockCommentInjectionTest.php
✅ Files skipped from review due to trivial changes (1)
- wordpress-plugin/gk-block-mcp/readme.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- wordpress-plugin/gk-block-mcp/includes/class-block-writer.php
Comment on lines
+22
to
+40
| class Block_Abilities { | ||
|
|
||
| /** | ||
| * Ability namespace prefix (per-product, matching the text domain). | ||
| */ | ||
| const NAMESPACE_PREFIX = 'gk-block-mcp/'; | ||
|
|
||
| /** | ||
| * Ability category slug. | ||
| */ | ||
| const CATEGORY = 'gk-block-mcp'; | ||
|
|
||
| /** | ||
| * Option key for the "expose operations as WordPress Abilities" toggle. | ||
| * | ||
| * Stored as the string '0'/'1'; defaults to enabled (opt-out) when unset. | ||
| */ | ||
| const ENABLED_OPTION = 'gk_block_api_abilities_enabled'; | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add @since annotations to class and public constants.
Per coding guidelines, shipped public members (production classes, methods, hooks, REST routes) must carry @since {version} docblock tags. The class docblock and all three public constants are missing @since 2.0.3 annotations.
📋 Suggested annotations
/**
* Registers Block MCP abilities with the WordPress Abilities API.
+ *
+ * `@since` 2.0.3
*/
class Block_Abilities {
/**
* Ability namespace prefix (per-product, matching the text domain).
+ *
+ * `@since` 2.0.3
*/
const NAMESPACE_PREFIX = 'gk-block-mcp/';
/**
* Ability category slug.
+ *
+ * `@since` 2.0.3
*/
const CATEGORY = 'gk-block-mcp';
/**
* Option key for the "expose operations as WordPress Abilities" toggle.
*
* Stored as the string '0'/'1'; defaults to enabled (opt-out) when unset.
+ *
+ * `@since` 2.0.3
*/
const ENABLED_OPTION = 'gk_block_api_abilities_enabled';Based on coding guidelines: "Annotate shipped public members (production classes, methods, hooks, REST routes) with @since {version} docblock tags."
📝 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
| class Block_Abilities { | |
| /** | |
| * Ability namespace prefix (per-product, matching the text domain). | |
| */ | |
| const NAMESPACE_PREFIX = 'gk-block-mcp/'; | |
| /** | |
| * Ability category slug. | |
| */ | |
| const CATEGORY = 'gk-block-mcp'; | |
| /** | |
| * Option key for the "expose operations as WordPress Abilities" toggle. | |
| * | |
| * Stored as the string '0'/'1'; defaults to enabled (opt-out) when unset. | |
| */ | |
| const ENABLED_OPTION = 'gk_block_api_abilities_enabled'; | |
| /** | |
| * Registers Block MCP abilities with the WordPress Abilities API. | |
| * | |
| * `@since` 2.0.3 | |
| */ | |
| class Block_Abilities { | |
| /** | |
| * Ability namespace prefix (per-product, matching the text domain). | |
| * | |
| * `@since` 2.0.3 | |
| */ | |
| const NAMESPACE_PREFIX = 'gk-block-mcp/'; | |
| /** | |
| * Ability category slug. | |
| * | |
| * `@since` 2.0.3 | |
| */ | |
| const CATEGORY = 'gk-block-mcp'; | |
| /** | |
| * Option key for the "expose operations as WordPress Abilities" toggle. | |
| * | |
| * Stored as the string '0'/'1'; defaults to enabled (opt-out) when unset. | |
| * | |
| * `@since` 2.0.3 | |
| */ | |
| const ENABLED_OPTION = 'gk_block_api_abilities_enabled'; |
🤖 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-block-abilities.php` around
lines 22 - 40, The Block_Abilities class and its public constants are missing
required `@since` version annotations in their docblocks. Add `@since 2.0.3`
tags to the class docblock at the top of the Block_Abilities class definition,
and add individual `@since 2.0.3` docblock annotations to each of the three
public constants: NAMESPACE_PREFIX, CATEGORY, and ENABLED_OPTION. Each constant
should have its own docblock with the `@since` tag included alongside its
description.
Source: Coding guidelines
Comment on lines
+69
to
+82
| public function __construct( Block_CRUD $crud, Post_Manager $post_manager, Block_Registry $registry ) { | ||
| $this->crud = $crud; | ||
| $this->post_manager = $post_manager; | ||
| $this->registry = $registry; | ||
| } | ||
|
|
||
| /** | ||
| * Whether the WordPress Abilities API is available on this site. | ||
| * | ||
| * @return bool | ||
| */ | ||
| public static function is_available() { | ||
| return function_exists( 'wp_register_ability' ) && function_exists( 'wp_register_ability_category' ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add @since annotations to public methods.
Both __construct() and is_available() are public methods and must carry @since 2.0.3 annotations per coding guidelines.
📋 Suggested annotations
/**
* Constructor.
*
+ * `@since` 2.0.3
+ *
* `@param` Block_CRUD $crud Block CRUD service.
* `@param` Post_Manager $post_manager Post manager service.
* `@param` Block_Registry $registry Block registry service.
*/
/**
* Whether the WordPress Abilities API is available on this site.
*
+ * `@since` 2.0.3
+ *
* `@return` bool
*/📝 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
| public function __construct( Block_CRUD $crud, Post_Manager $post_manager, Block_Registry $registry ) { | |
| $this->crud = $crud; | |
| $this->post_manager = $post_manager; | |
| $this->registry = $registry; | |
| } | |
| /** | |
| * Whether the WordPress Abilities API is available on this site. | |
| * | |
| * @return bool | |
| */ | |
| public static function is_available() { | |
| return function_exists( 'wp_register_ability' ) && function_exists( 'wp_register_ability_category' ); | |
| } | |
| /** | |
| * Constructor. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@param` Block_CRUD $crud Block CRUD service. | |
| * `@param` Post_Manager $post_manager Post manager service. | |
| * `@param` Block_Registry $registry Block registry service. | |
| */ | |
| public function __construct( Block_CRUD $crud, Post_Manager $post_manager, Block_Registry $registry ) { | |
| $this->crud = $crud; | |
| $this->post_manager = $post_manager; | |
| $this->registry = $registry; | |
| } | |
| /** | |
| * Whether the WordPress Abilities API is available on this site. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@return` bool | |
| */ | |
| public static function is_available() { | |
| return function_exists( 'wp_register_ability' ) && function_exists( 'wp_register_ability_category' ); | |
| } |
🤖 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-block-abilities.php` around
lines 69 - 82, The public methods __construct() and is_available() in the
Block_Abilities class are missing required `@since` annotations. Add a docblock
with `@since` 2.0.3 annotation to the __construct() method (which currently has no
docblock), and add the `@since` 2.0.3 tag to the existing docblock of the
is_available() method to comply with coding guidelines.
Source: Coding guidelines
Comment on lines
+95
to
+108
| public static function is_enabled() { | ||
| $enabled = '0' !== (string) get_option( self::ENABLED_OPTION, '1' ); | ||
|
|
||
| /** | ||
| * Filters whether Block MCP registers its operations as WordPress | ||
| * Abilities (exposing them to the official MCP Adapter and the Abilities | ||
| * REST API). Defaults to the Settings → Block MCP toggle. | ||
| * | ||
| * @since 2.1.0 | ||
| * | ||
| * @param bool $enabled Whether registration is enabled. | ||
| */ | ||
| return (bool) apply_filters( 'gk/block-mcp/abilities/enabled', $enabled ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Correct filter hook @since version and add method annotation.
The filter hook gk/block-mcp/abilities/enabled is documented with @since 2.1.0 (line 103), but this release is 2.0.3. The is_enabled() method itself is also missing its @since 2.0.3 annotation.
🔧 Version fix
/**
* Whether registering Block MCP abilities is enabled for this site.
*
* Default on (opt-out): registration exposes the operations to the official
* MCP Adapter and the Abilities REST API — a capability-gated but
* network-reachable surface — so a site owner can turn it off. Stored as the
* `gk_block_api_abilities_enabled` option (Settings → Block MCP) and
* filterable via `gk/block-mcp/abilities/enabled` for programmatic control.
*
+ * `@since` 2.0.3
+ *
* `@return` bool
*/
public static function is_enabled() {
$enabled = '0' !== (string) get_option( self::ENABLED_OPTION, '1' );
/**
* Filters whether Block MCP registers its operations as WordPress
* Abilities (exposing them to the official MCP Adapter and the Abilities
* REST API). Defaults to the Settings → Block MCP toggle.
*
- * `@since` 2.1.0
+ * `@since` 2.0.3
*
* `@param` bool $enabled Whether registration is enabled.
*/📝 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
| public static function is_enabled() { | |
| $enabled = '0' !== (string) get_option( self::ENABLED_OPTION, '1' ); | |
| /** | |
| * Filters whether Block MCP registers its operations as WordPress | |
| * Abilities (exposing them to the official MCP Adapter and the Abilities | |
| * REST API). Defaults to the Settings → Block MCP toggle. | |
| * | |
| * @since 2.1.0 | |
| * | |
| * @param bool $enabled Whether registration is enabled. | |
| */ | |
| return (bool) apply_filters( 'gk/block-mcp/abilities/enabled', $enabled ); | |
| } | |
| /** | |
| * Whether registering Block MCP abilities is enabled for this site. | |
| * | |
| * Default on (opt-out): registration exposes the operations to the official | |
| * MCP Adapter and the Abilities REST API — a capability-gated but | |
| * network-reachable surface — so a site owner can turn it off. Stored as the | |
| * `gk_block_api_abilities_enabled` option (Settings → Block MCP) and | |
| * filterable via `gk/block-mcp/abilities/enabled` for programmatic control. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@return` bool | |
| */ | |
| public static function is_enabled() { | |
| $enabled = '0' !== (string) get_option( self::ENABLED_OPTION, '1' ); | |
| /** | |
| * Filters whether Block MCP registers its operations as WordPress | |
| * Abilities (exposing them to the official MCP Adapter and the Abilities | |
| * REST API). Defaults to the Settings → Block MCP toggle. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@param` bool $enabled Whether registration is enabled. | |
| */ | |
| return (bool) apply_filters( 'gk/block-mcp/abilities/enabled', $enabled ); | |
| } |
🤖 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-block-abilities.php` around
lines 95 - 108, The is_enabled() method has incorrect documentation version
information. Update the `@since` version in the filter hook documentation from
2.1.0 to 2.0.3 for the gk/block-mcp/abilities/enabled filter, and add a `@since`
2.0.3 annotation to the is_enabled() method itself in its docblock to properly
document when this method was introduced.
Source: Coding guidelines
Comment on lines
+118
to
+166
| public function register() { | ||
| if ( ! self::is_available() || ! self::is_enabled() ) { | ||
| return; | ||
| } | ||
| add_action( 'wp_abilities_api_categories_init', array( $this, 'register_category' ) ); | ||
| add_action( 'wp_abilities_api_init', array( $this, 'register_abilities' ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Register the ability category. Idempotent. | ||
| * | ||
| * @return void | ||
| */ | ||
| public function register_category() { | ||
| if ( wp_has_ability_category( self::CATEGORY ) ) { | ||
| return; | ||
| } | ||
| wp_register_ability_category( | ||
| self::CATEGORY, | ||
| array( | ||
| 'label' => __( 'Block MCP', 'gk-block-mcp' ), | ||
| 'description' => __( 'Block-tree reading and editing for AI agents — surgical, ref-stable, revision-backed.', 'gk-block-mcp' ), | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Register all abilities. Idempotent — skips any already registered, so | ||
| * re-entrant or repeated firing of `wp_abilities_api_init` is safe. | ||
| * | ||
| * @return void | ||
| */ | ||
| public function register_abilities() { | ||
| foreach ( $this->definitions() as $name => $args ) { | ||
| if ( wp_has_ability( $name ) ) { | ||
| continue; | ||
| } | ||
| wp_register_ability( $name, $args ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * The fully-qualified names of every ability this bridge registers. | ||
| * | ||
| * @return string[] | ||
| */ | ||
| public function ability_names() { | ||
| return array_keys( $this->definitions() ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add @since annotations to registration and helper methods.
Four public methods (register(), register_category(), register_abilities(), ability_names()) are missing @since 2.0.3 annotations.
📋 Suggested annotations
/**
* Wire registration onto the Abilities init hooks.
*
* No-op when the Abilities API is absent (older cores) or when the site
* owner has disabled the toggle, so callers can invoke this unconditionally.
*
+ * `@since` 2.0.3
+ *
* `@return` void
*/
/**
* Register the ability category. Idempotent.
*
+ * `@since` 2.0.3
+ *
* `@return` void
*/
/**
* Register all abilities. Idempotent — skips any already registered, so
* re-entrant or repeated firing of `wp_abilities_api_init` is safe.
*
+ * `@since` 2.0.3
+ *
* `@return` void
*/
/**
* The fully-qualified names of every ability this bridge registers.
*
+ * `@since` 2.0.3
+ *
* `@return` string[]
*/📝 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
| public function register() { | |
| if ( ! self::is_available() || ! self::is_enabled() ) { | |
| return; | |
| } | |
| add_action( 'wp_abilities_api_categories_init', array( $this, 'register_category' ) ); | |
| add_action( 'wp_abilities_api_init', array( $this, 'register_abilities' ) ); | |
| } | |
| /** | |
| * Register the ability category. Idempotent. | |
| * | |
| * @return void | |
| */ | |
| public function register_category() { | |
| if ( wp_has_ability_category( self::CATEGORY ) ) { | |
| return; | |
| } | |
| wp_register_ability_category( | |
| self::CATEGORY, | |
| array( | |
| 'label' => __( 'Block MCP', 'gk-block-mcp' ), | |
| 'description' => __( 'Block-tree reading and editing for AI agents — surgical, ref-stable, revision-backed.', 'gk-block-mcp' ), | |
| ) | |
| ); | |
| } | |
| /** | |
| * Register all abilities. Idempotent — skips any already registered, so | |
| * re-entrant or repeated firing of `wp_abilities_api_init` is safe. | |
| * | |
| * @return void | |
| */ | |
| public function register_abilities() { | |
| foreach ( $this->definitions() as $name => $args ) { | |
| if ( wp_has_ability( $name ) ) { | |
| continue; | |
| } | |
| wp_register_ability( $name, $args ); | |
| } | |
| } | |
| /** | |
| * The fully-qualified names of every ability this bridge registers. | |
| * | |
| * @return string[] | |
| */ | |
| public function ability_names() { | |
| return array_keys( $this->definitions() ); | |
| } | |
| public function register() { | |
| if ( ! self::is_available() || ! self::is_enabled() ) { | |
| return; | |
| } | |
| add_action( 'wp_abilities_api_categories_init', array( $this, 'register_category' ) ); | |
| add_action( 'wp_abilities_api_init', array( $this, 'register_abilities' ) ); | |
| } | |
| /** | |
| * Wire registration onto the Abilities init hooks. | |
| * | |
| * No-op when the Abilities API is absent (older cores) or when the site | |
| * owner has disabled the toggle, so callers can invoke this unconditionally. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@return` void | |
| */ | |
| public function register() { | |
| if ( ! self::is_available() || ! self::is_enabled() ) { | |
| return; | |
| } | |
| add_action( 'wp_abilities_api_categories_init', array( $this, 'register_category' ) ); | |
| add_action( 'wp_abilities_api_init', array( $this, 'register_abilities' ) ); | |
| } | |
| /** | |
| * Register the ability category. Idempotent. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@return` void | |
| */ | |
| public function register_category() { | |
| if ( wp_has_ability_category( self::CATEGORY ) ) { | |
| return; | |
| } | |
| wp_register_ability_category( | |
| self::CATEGORY, | |
| array( | |
| 'label' => __( 'Block MCP', 'gk-block-mcp' ), | |
| 'description' => __( 'Block-tree reading and editing for AI agents — surgical, ref-stable, revision-backed.', 'gk-block-mcp' ), | |
| ) | |
| ); | |
| } | |
| /** | |
| * Register all abilities. Idempotent — skips any already registered, so | |
| * re-entrant or repeated firing of `wp_abilities_api_init` is safe. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@return` void | |
| */ | |
| public function register_abilities() { | |
| foreach ( $this->definitions() as $name => $args ) { | |
| if ( wp_has_ability( $name ) ) { | |
| continue; | |
| } | |
| wp_register_ability( $name, $args ); | |
| } | |
| } | |
| /** | |
| * The fully-qualified names of every ability this bridge registers. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@return` string[] | |
| */ | |
| public function ability_names() { | |
| return array_keys( $this->definitions() ); | |
| } |
🤖 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-block-abilities.php` around
lines 118 - 166, The four public methods register(), register_category(),
register_abilities(), and ability_names() in the class-block-abilities.php file
are missing `@since` annotations in their docblock comments. Add `@since` 2.0.3 to
the docblock of each of these four methods to indicate when they were introduced
in the codebase.
Source: Coding guidelines
Comment on lines
+375
to
+407
| /** | ||
| * Permit when the user can edit the post named in the input. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return bool | ||
| */ | ||
| public function can_edit_post_input( $input ) { | ||
| $post_id = isset( $input['post_id'] ) ? (int) $input['post_id'] : 0; | ||
| return current_user_can( 'edit_post', $post_id ); | ||
| } | ||
|
|
||
| /** | ||
| * Permit creation for users who can edit posts. The precise create/publish | ||
| * capability is enforced inside Post_Manager::create_post(). | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return bool | ||
| */ | ||
| public function can_create( $input ) { | ||
| unset( $input ); | ||
| return current_user_can( 'edit_posts' ); | ||
| } | ||
|
|
||
| /** | ||
| * Permit read/discovery for users who can edit posts. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return bool | ||
| */ | ||
| public function can_read( $input ) { | ||
| unset( $input ); | ||
| return current_user_can( 'edit_posts' ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add @since annotations to permission callbacks.
Three public permission callback methods are missing @since 2.0.3 annotations.
📋 Suggested annotations
/**
* Permit when the user can edit the post named in the input.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` bool
*/
/**
* Permit creation for users who can edit posts. The precise create/publish
* capability is enforced inside Post_Manager::create_post().
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` bool
*/
/**
* Permit read/discovery for users who can edit posts.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` bool
*/📝 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
| /** | |
| * Permit when the user can edit the post named in the input. | |
| * | |
| * @param array<string, mixed> $input Ability input. | |
| * @return bool | |
| */ | |
| public function can_edit_post_input( $input ) { | |
| $post_id = isset( $input['post_id'] ) ? (int) $input['post_id'] : 0; | |
| return current_user_can( 'edit_post', $post_id ); | |
| } | |
| /** | |
| * Permit creation for users who can edit posts. The precise create/publish | |
| * capability is enforced inside Post_Manager::create_post(). | |
| * | |
| * @param array<string, mixed> $input Ability input. | |
| * @return bool | |
| */ | |
| public function can_create( $input ) { | |
| unset( $input ); | |
| return current_user_can( 'edit_posts' ); | |
| } | |
| /** | |
| * Permit read/discovery for users who can edit posts. | |
| * | |
| * @param array<string, mixed> $input Ability input. | |
| * @return bool | |
| */ | |
| public function can_read( $input ) { | |
| unset( $input ); | |
| return current_user_can( 'edit_posts' ); | |
| } | |
| /** | |
| * Permit when the user can edit the post named in the input. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@param` array<string, mixed> $input Ability input. | |
| * `@return` bool | |
| */ | |
| public function can_edit_post_input( $input ) { | |
| $post_id = isset( $input['post_id'] ) ? (int) $input['post_id'] : 0; | |
| return current_user_can( 'edit_post', $post_id ); | |
| } | |
| /** | |
| * Permit creation for users who can edit posts. The precise create/publish | |
| * capability is enforced inside Post_Manager::create_post(). | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@param` array<string, mixed> $input Ability input. | |
| * `@return` bool | |
| */ | |
| public function can_create( $input ) { | |
| unset( $input ); | |
| return current_user_can( 'edit_posts' ); | |
| } | |
| /** | |
| * Permit read/discovery for users who can edit posts. | |
| * | |
| * `@since` 2.0.3 | |
| * | |
| * `@param` array<string, mixed> $input Ability input. | |
| * `@return` bool | |
| */ | |
| public function can_read( $input ) { | |
| unset( $input ); | |
| return current_user_can( 'edit_posts' ); | |
| } |
🤖 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-block-abilities.php` around
lines 375 - 407, Add the `@since 2.0.3` annotation to the PHPDoc comments for
the three permission callback methods: can_edit_post_input, can_create, and
can_read. Each method's documentation block should include a line with `@since
2.0.3` to document when these methods were introduced.
Source: Coding guidelines
Comment on lines
+411
to
+478
| /** | ||
| * Read a post's block tree via Block_CRUD. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return array|\WP_Error | ||
| */ | ||
| public function execute_get_page_blocks( $input ) { | ||
| return $this->crud->get_blocks( (int) $input['post_id'] ); | ||
| } | ||
|
|
||
| /** | ||
| * Update one block by flat index via Block_CRUD. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return array|\WP_Error | ||
| */ | ||
| public function execute_update_block( $input ) { | ||
| $attributes = isset( $input['attributes'] ) && is_array( $input['attributes'] ) ? $input['attributes'] : array(); | ||
| $inner_html = isset( $input['inner_html'] ) ? (string) $input['inner_html'] : null; | ||
| return $this->crud->update_block( (int) $input['post_id'], (int) $input['index'], $attributes, $inner_html ); | ||
| } | ||
|
|
||
| /** | ||
| * Insert blocks at a position via Block_CRUD. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return array|\WP_Error | ||
| */ | ||
| public function execute_insert_blocks( $input ) { | ||
| $position = isset( $input['position'] ) ? (int) $input['position'] : null; | ||
| $blocks = isset( $input['blocks'] ) && is_array( $input['blocks'] ) ? $input['blocks'] : array(); | ||
| return $this->crud->insert_blocks( (int) $input['post_id'], $position, $blocks ); | ||
| } | ||
|
|
||
| /** | ||
| * Create a post via Post_Manager. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return array|\WP_Error | ||
| */ | ||
| public function execute_create_post( $input ) { | ||
| return $this->post_manager->create_post( is_array( $input ) ? $input : array() ); | ||
| } | ||
|
|
||
| /** | ||
| * List registered block types via Block_Registry. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return array|\WP_Error | ||
| */ | ||
| public function execute_list_block_types( $input ) { | ||
| $args = array(); | ||
| if ( is_array( $input ) && ! empty( $input['namespace'] ) ) { | ||
| $args['namespace'] = (string) $input['namespace']; | ||
| } | ||
| return $this->registry->get_block_types( $args ); | ||
| } | ||
|
|
||
| /** | ||
| * Remove one or more blocks via Block_CRUD. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return array|\WP_Error | ||
| */ | ||
| public function execute_delete_block( $input ) { | ||
| $count = isset( $input['count'] ) ? max( 1, (int) $input['count'] ) : 1; | ||
| return $this->crud->delete_blocks( (int) $input['post_id'], (int) $input['index'], $count ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add @since annotations to execute callbacks.
All six public execute callback methods are missing @since 2.0.3 annotations.
📋 Suggested annotations
/**
* Read a post's block tree via Block_CRUD.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` array|\WP_Error
*/
/**
* Update one block by flat index via Block_CRUD.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` array|\WP_Error
*/
/**
* Insert blocks at a position via Block_CRUD.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` array|\WP_Error
*/
/**
* Create a post via Post_Manager.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` array|\WP_Error
*/
/**
* List registered block types via Block_Registry.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` array|\WP_Error
*/
/**
* Remove one or more blocks via Block_CRUD.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input.
* `@return` array|\WP_Error
*/🤖 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-block-abilities.php` around
lines 411 - 478, All six public execute callback methods
(execute_get_page_blocks, execute_update_block, execute_insert_blocks,
execute_create_post, execute_list_block_types, and execute_delete_block) are
missing `@since` version annotations in their PHPDoc documentation blocks. Add
`@since` 2.0.3 annotation to each method's PHPDoc comment block, placing it after
the `@return` annotation to document when these methods were introduced.
Source: Coding guidelines
Comment on lines
+427
to
+443
| public function execute_update_block( $input ) { | ||
| $attributes = isset( $input['attributes'] ) && is_array( $input['attributes'] ) ? $input['attributes'] : array(); | ||
| $inner_html = isset( $input['inner_html'] ) ? (string) $input['inner_html'] : null; | ||
| return $this->crud->update_block( (int) $input['post_id'], (int) $input['index'], $attributes, $inner_html ); | ||
| } | ||
|
|
||
| /** | ||
| * Insert blocks at a position via Block_CRUD. | ||
| * | ||
| * @param array<string, mixed> $input Ability input. | ||
| * @return array|\WP_Error | ||
| */ | ||
| public function execute_insert_blocks( $input ) { | ||
| $position = isset( $input['position'] ) ? (int) $input['position'] : null; | ||
| $blocks = isset( $input['blocks'] ) && is_array( $input['blocks'] ) ? $input['blocks'] : array(); | ||
| return $this->crud->insert_blocks( (int) $input['post_id'], $position, $blocks ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Block_CRUD methods reject negative index/position values
# Check if update_block, insert_blocks, delete_blocks validate index/position
rg -nP -A10 'function (update_block|insert_blocks|delete_blocks)\(' wordpress-plugin/gk-block-mcp/includes/class-block-crud.php | \
rg -P '(index|position)\s*(<|<=)\s*0|is_negative|absint|max\(0'
# Also check BlockAbilitiesTest for negative-index test assertions
rg -nP 'negative.*(index|position)' wordpress-plugin/gk-block-mcp/tests/Integrations/BlockAbilitiesTest.phpRepository: GravityKit/block-mcp
Length of output: 535
🏁 Script executed:
# Get the Block_CRUD methods implementation to see if they validate negative indices
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-crud.php | sed -n '1,50p'Repository: GravityKit/block-mcp
Length of output: 1603
🏁 Script executed:
# Find and show the update_block, insert_blocks, delete_blocks methods in Block_CRUD
rg -n 'public function (update_block|insert_blocks|delete_blocks)' wordpress-plugin/gk-block-mcp/includes/class-block-crud.phpRepository: GravityKit/block-mcp
Length of output: 498
🏁 Script executed:
# Get the complete execute methods (427-443 and 475-478)
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-abilities.php | sed -n '427,480p'Repository: GravityKit/block-mcp
Length of output: 2405
🏁 Script executed:
# Get the full update_block method implementation
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-crud.php | sed -n '169,183p'Repository: GravityKit/block-mcp
Length of output: 979
🏁 Script executed:
# Get the full insert_blocks method implementation
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-crud.php | sed -n '199,213p'Repository: GravityKit/block-mcp
Length of output: 782
🏁 Script executed:
# Get the full delete_blocks method implementation
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-crud.php | sed -n '214,230p'Repository: GravityKit/block-mcp
Length of output: 1002
🏁 Script executed:
# Find Block_Writer file
fd -t f 'block-writer' wordpress-plugin/gk-block-mcp/includes/Repository: GravityKit/block-mcp
Length of output: 220
🏁 Script executed:
# Get Block_Writer update_block, insert_blocks, delete_blocks implementations
rg -n 'public function (update_block|insert_blocks|delete_blocks)' wordpress-plugin/gk-block-mcp/includes/class-block-writer.phpRepository: GravityKit/block-mcp
Length of output: 500
🏁 Script executed:
# Get update_block implementation - check for negative index validation
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-writer.php | sed -n '826,860p'Repository: GravityKit/block-mcp
Length of output: 1483
🏁 Script executed:
# Get insert_blocks implementation - check for negative position validation
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-writer.php | sed -n '1218,1250p'Repository: GravityKit/block-mcp
Length of output: 1551
🏁 Script executed:
# Get delete_blocks implementation - check for negative index validation
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-writer.php | sed -n '1371,1410p'Repository: GravityKit/block-mcp
Length of output: 1486
🏁 Script executed:
# Check if insert_blocks has position validation further down
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-writer.php | sed -n '1250,1290p'Repository: GravityKit/block-mcp
Length of output: 1679
🏁 Script executed:
# Check the test cases to see what validation is expected
cat -n wordpress-plugin/gk-block-mcp/tests/Integrations/BlockAbilitiesTest.php | sed -n '495,530p'Repository: GravityKit/block-mcp
Length of output: 1666
🏁 Script executed:
# Check if there's a test for insert_blocks with negative position
rg -n 'insert.*negative|negative.*position' wordpress-plugin/gk-block-mcp/tests/Integrations/BlockAbilitiesTest.phpRepository: GravityKit/block-mcp
Length of output: 158
🏁 Script executed:
# Check if there's a test for insert_blocks that should validate negative position
rg -n 'test_insert.*negative\|negative.*insert' wordpress-plugin/gk-block-mcp/tests/Integrations/BlockAbilitiesTest.phpRepository: GravityKit/block-mcp
Length of output: 158
🏁 Script executed:
# Look for any insert_blocks test cases
rg -n 'execute_insert_blocks\|insert-blocks' wordpress-plugin/gk-block-mcp/tests/Integrations/BlockAbilitiesTest.php -A5Repository: GravityKit/block-mcp
Length of output: 158
🏁 Script executed:
# Get more context around insert_blocks position handling
cat -n wordpress-plugin/gk-block-mcp/includes/class-block-writer.php | sed -n '1275,1310p'Repository: GravityKit/block-mcp
Length of output: 1621
Negative index/position validation exists for update and delete, but not insert.
update_block and delete_blocks properly validate negative indices in Block_Writer (lines 847 and 1399 respectively), rejecting them with a 400 error. However, insert_blocks lacks validation—it gracefully handles negative positions via clamping (line 1279–1283) rather than rejecting them. For consistency with the other two methods (which tests confirm should reject negatives per lines 495–497), consider adding explicit negative position validation to execute_insert_blocks or Block_Writer::insert_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 `@wordpress-plugin/gk-block-mcp/includes/class-block-abilities.php` around
lines 427 - 443, The execute_insert_blocks method lacks validation for negative
position values, whereas execute_update_block and execute_delete_blocks properly
reject negative indices with a 400 error. Add explicit negative position
validation to execute_insert_blocks to check if the position parameter (after
extracting it from the input array) is negative, and return a WP_Error with a
400 status code if it is, ensuring consistency with how the other block mutation
methods handle invalid indices.
Comment on lines
+480
to
+504
| /** | ||
| * Build the site design context (theme + flattened theme.json presets) so an | ||
| * agent composes theme-aligned, valid block markup. | ||
| * | ||
| * @param array<string, mixed> $input Ability input (unused). | ||
| * @return array<string, mixed> | ||
| */ | ||
| public function execute_site_editor_context( $input ) { | ||
| unset( $input ); | ||
|
|
||
| $theme = wp_get_theme(); | ||
|
|
||
| return array( | ||
| 'theme' => array( | ||
| 'name' => $theme->get( 'Name' ), | ||
| 'stylesheet' => get_stylesheet(), | ||
| ), | ||
| 'presets' => array( | ||
| 'colors' => $this->flatten_presets( $this->global_setting( array( 'color', 'palette' ) ) ), | ||
| 'gradients' => $this->flatten_presets( $this->global_setting( array( 'color', 'gradients' ) ) ), | ||
| 'font_sizes' => $this->flatten_presets( $this->global_setting( array( 'typography', 'fontSizes' ) ) ), | ||
| 'spacing_sizes' => $this->flatten_presets( $this->global_setting( array( 'spacing', 'spacingSizes' ) ) ), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add @since annotation to execute_site_editor_context().
This public execute callback is missing its @since 2.0.3 annotation. (The private helper methods global_setting() and flatten_presets() correctly omit @since per project conventions.)
📋 Suggested annotation
/**
* Build the site design context (theme + flattened theme.json presets) so an
* agent composes theme-aligned, valid block markup.
*
+ * `@since` 2.0.3
+ *
* `@param` array<string, mixed> $input Ability input (unused).
* `@return` array<string, mixed>
*/Based on learnings: Private helper methods (global_setting(), flatten_presets()) correctly omit @since tags per project convention.
🤖 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-block-abilities.php` around
lines 480 - 504, The execute_site_editor_context() method is missing a `@since`
annotation in its DocBlock. Add the `@since` 2.0.3 annotation to the method's
DocBlock comment block, placing it after the `@return` annotation to follow
project conventions. Note that private helper methods like global_setting() and
flatten_presets() correctly omit `@since` tags per project standards, but public
execute callbacks require this annotation.
Source: Coding guidelines
Comment on lines
+210
to
+225
| $this->crud->update_block( $post_id, 0, array(), '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' ); | ||
|
|
||
| $content = (string) get_post_field( 'post_content', $post_id ); | ||
| $this->assertSame( 1, substr_count( $content, '<!-- wp:paragraph' ), 'update must not inject a phantom block delimiter' ); | ||
| } | ||
|
|
||
| /** | ||
| * The same guard covers edit_block_tree's `update-html` mutate op. | ||
| */ | ||
| public function test_mutate_update_html_strips_injected_delimiters() { | ||
| $post_id = $this->make_block_post( array( $this->paragraph_block( '<p>safe</p>' ) ) ); | ||
|
|
||
| $this->mutator->mutate( $post_id, 'update-html', array( 0 ), array( 'innerHTML' => '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' ) ); | ||
|
|
||
| $content = (string) get_post_field( 'post_content', $post_id ); | ||
| $this->assertSame( 1, substr_count( $content, '<!-- wp:paragraph' ) ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Assert write operations succeed before checking serialized output.
Both new regression tests can pass even if the write call fails, because unchanged content still has one paragraph delimiter. Add explicit non-error assertions to keep the tests failure-sensitive.
Suggested patch
- $this->crud->update_block( $post_id, 0, array(), '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' );
+ $result = $this->crud->update_block( $post_id, 0, array(), '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' );
+ $this->assertNotInstanceOf( \WP_Error::class, $result, 'update path should succeed before delimiter assertion' );
@@
- $this->mutator->mutate( $post_id, 'update-html', array( 0 ), array( 'innerHTML' => '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' ) );
+ $result = $this->mutator->mutate( $post_id, 'update-html', array( 0 ), array( 'innerHTML' => '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' ) );
+ $this->assertNotInstanceOf( \WP_Error::class, $result, 'mutate update-html path should succeed before delimiter assertion' );📝 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
| $this->crud->update_block( $post_id, 0, array(), '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' ); | |
| $content = (string) get_post_field( 'post_content', $post_id ); | |
| $this->assertSame( 1, substr_count( $content, '<!-- wp:paragraph' ), 'update must not inject a phantom block delimiter' ); | |
| } | |
| /** | |
| * The same guard covers edit_block_tree's `update-html` mutate op. | |
| */ | |
| public function test_mutate_update_html_strips_injected_delimiters() { | |
| $post_id = $this->make_block_post( array( $this->paragraph_block( '<p>safe</p>' ) ) ); | |
| $this->mutator->mutate( $post_id, 'update-html', array( 0 ), array( 'innerHTML' => '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' ) ); | |
| $content = (string) get_post_field( 'post_content', $post_id ); | |
| $this->assertSame( 1, substr_count( $content, '<!-- wp:paragraph' ) ); | |
| $result = $this->crud->update_block( $post_id, 0, array(), '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' ); | |
| $this->assertNotInstanceOf( \WP_Error::class, $result, 'update path should succeed before delimiter assertion' ); | |
| $content = (string) get_post_field( 'post_content', $post_id ); | |
| $this->assertSame( 1, substr_count( $content, '<!-- wp:paragraph' ), 'update must not inject a phantom block delimiter' ); | |
| } | |
| /** | |
| * The same guard covers edit_block_tree's `update-html` mutate op. | |
| */ | |
| public function test_mutate_update_html_strips_injected_delimiters() { | |
| $post_id = $this->make_block_post( array( $this->paragraph_block( '<p>safe</p>' ) ) ); | |
| $result = $this->mutator->mutate( $post_id, 'update-html', array( 0 ), array( 'innerHTML' => '<p>x</p><!-- /wp:paragraph --><!-- wp:paragraph --><p>injected</p>' ) ); | |
| $this->assertNotInstanceOf( \WP_Error::class, $result, 'mutate update-html path should succeed before delimiter assertion' ); | |
| $content = (string) get_post_field( 'post_content', $post_id ); | |
| $this->assertSame( 1, substr_count( $content, '<!-- wp:paragraph' ) ); |
🤖 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/Security/BlockCommentInjectionTest.php`
around lines 210 - 225, Both test methods lack explicit success assertions after
their write operations. In test_update_block_strips_injected_delimiters, add an
assertion after the update_block call to verify the operation succeeded without
errors. Similarly, in test_mutate_update_html_strips_injected_delimiters, add an
assertion after the mutate call to verify it succeeded. This ensures the tests
fail if the write operations fail, rather than silently passing due to unchanged
content still containing the expected delimiter count.
Source: Coding guidelines
…nd read disk truth on ref-persist conflict CodeRabbit review of the lost-update CAS: 1. save_post_content treated any 0-affected-rows swap as edit_conflict. A no-op save (new content equal to the snapshot) changes nothing and reports 0 affected on engines that count changed rows (MySQL) — a false 409 on an idempotent save. And (int) false === 0, so a DB error was mislabeled a conflict too. Classify explicitly: false -> db_write_failed (500); 0 affected is a conflict only when the written content differs (extracted to Block_Writer::swap_is_conflict, unit-tested harness-independently since the SQLite test engine counts matched rows, not changed rows, and so cannot reproduce the MySQL behaviour behaviourally). 2. When the lazy ref-persist loses its CAS (a concurrent edit moved the row), Block_Reader returned the in-memory tree carrying refs that never reached disk, so a follow-up write-by-ref would target a non-existent ref. It now re-reads disk truth. Pinned by a read-after-persist-conflict test. Skipped CodeRabbit's nitpick to extract the duplicated test helpers into a shared trait: the per-file fixture helpers are tiny and the suite's convention is per-file fixtures; a shared trait adds cross-file coupling for marginal gain. Full suite green (1089 single-site + 24 yoast + 5 multisite), phpcs 0/0, phpstan OK. Claude-Session: https://claude.ai/code/session_01QQaEitncpMyD4mkrxaMjQG
feat(preferences): site-aware, opinion-free block preferences
…page
resolve_url() and post_info() reduced the URL to PHP_URL_PATH, dropping the
query string, so ?p= / ?page_id= / ?post_type=&p= forms collapsed to
home_url('/') and resolved to the front page. A shared url_to_resolvable_path()
helper now preserves the query so url_to_postid() returns the intended post;
pretty permalinks are unaffected. Adds a regression test and a changelog entry.
Claude-Session: https://claude.ai/code/session_01G65Y2oopoWXv6ySVmTs7Vn
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wordpress-plugin/gk-block-mcp/includes/class-settings-page.php (3)
330-338: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExplicit
0scores are dropped without warning when the name is blank.Here
0is a valid override, but$score > 0treats it the same as an untouched row. A submitted{ name: '', score: 0 }therefore disappears silently instead of surfacing the warning this sanitizer is meant to emit. Track whether the raw score field was present/non-empty before casting so explicit0still increments$dropped_scored.Suggested fix
- $ns = isset( $row['name'] ) ? sanitize_key( $row['name'] ) : ''; - $score = isset( $row['score'] ) ? (int) $row['score'] : 0; + $ns = isset( $row['name'] ) ? sanitize_key( $row['name'] ) : ''; + $raw_score = isset( $row['score'] ) ? trim( (string) $row['score'] ) : ''; + $has_score = '' !== $raw_score; + $score = $has_score ? (int) $raw_score : 0; if ( '' === $ns ) { - if ( $score > 0 ) { + if ( $has_score ) { ++$dropped_scored; } continue; }🤖 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-settings-page.php` around lines 330 - 338, The score sanitizer in the settings page is treating an explicit 0 the same as an empty row, so blank-name rows with a submitted score of 0 are dropped without warning. Update the row-handling logic in the settings-page sanitizer to detect whether the raw score field was actually present/non-empty before casting, and use that signal to increment the dropped-scored counter even when the numeric value becomes 0. Keep the fix near the existing $ns/$score handling so the warning behavior for malformed rows remains consistent.
513-518: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset leaves the trash gate enabled.
handle_reset()clears the media and abilities toggles, but it never deletesPost_Manager::ALLOW_TRASH_OPTION. If trashing was enabled before reset, assistants still keep the non-default “move posts to trash” permission afterward, which contradicts the “Clears everything on this page” contract.Suggested fix
delete_option( 'gk_block_api_post_types_allowlist' ); delete_option( self::DUAL_MANUAL_OPTION ); delete_option( Media_Manager::UPLOADS_OPTION ); + delete_option( \GravityKit\BlockMCP\Post_Manager::ALLOW_TRASH_OPTION ); delete_option( \GravityKit\BlockMCP\Block_Abilities::ENABLED_OPTION ); delete_option( Block_Inventory::STORAGE_MODES_OPTION );Based on learnings, use the app-level trash gate (
gk_block_api_allow_trash/gk/block-mcp/post/allow-trash) instead of relying on the agent account's capabilities for trash behavior.🤖 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-settings-page.php` around lines 513 - 518, handle_reset() in class-settings-page.php is missing the app-level trash gate cleanup, so the reset still leaves trash behavior enabled. Update the reset flow to also delete Post_Manager::ALLOW_TRASH_OPTION (gk_block_api_allow_trash) alongside the other options, and make sure any trash-related permission checks use the app-level gate gk/block-mcp/post/allow-trash rather than the agent account’s capabilities.Source: Learnings
1448-1473: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdded replacement rows never get a Remove control.
addRow()clones the last row verbatim, and in this table the last row is the blank template whose action cell is empty. Every row created through “Add row” therefore also has an empty third cell, so once the user types into it there is no UI way to remove that row until after a save/reload.🤖 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-settings-page.php` around lines 1448 - 1473, The addRow() cloning logic in class-settings-page.php copies the blank template row exactly, so newly added rows inherit an empty action cell and never get a Remove control. Update addRow() to inject or restore the row’s Remove button/link after cloning (or clone a row that already includes the action cell markup), and ensure the control remains wired correctly for every appended row.
🧹 Nitpick comments (4)
wordpress-plugin/gk-block-mcp/tests/REST/UrlResolutionTest.php (1)
50-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the regression docblock generic.
“sent internal docs to the homepage” depends on off-tree context. Prefer a self-contained failure mode like “previously resolved to the front page instead of the page post.”
As per coding guidelines, test docblocks should state the contract/regression/failure mode without narrative, and comments should not require outside artifacts to understand.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-mcp/tests/REST/UrlResolutionTest.php` around lines 50 - 54, The docblock above the URL resolution test is too specific to an external incident and should be made self-contained. Update the comment near UrlResolutionTest and the regression test description to state the generic failure mode for the ?post_type=&p={id} URL form, such as resolving to the front page instead of the intended page post, without mentioning internal docs or outside context.Source: Coding guidelines
wordpress-plugin/gk-block-mcp/includes/class-rest-controller.php (1)
1414-1422: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssign the scheme check before branching.
Line 1415 inlines
strpos()in the conditional; this file’s guideline asks production conditionals to use named checks first.Proposed adjustment
private function url_to_resolvable_path( $url ) { - if ( false === strpos( $url, '://' ) ) { + $has_scheme = false !== strpos( $url, '://' ); + if ( ! $has_scheme ) { return $url; } $parts = wp_parse_url( $url ); $path = isset( $parts['path'] ) ? $parts['path'] : '/'; - if ( ! empty( $parts['query'] ) ) { + $has_query = ! empty( $parts['query'] ); + if ( $has_query ) { $path .= '?' . $parts['query']; }As per coding guidelines, “Before any conditional, assign checks to named variables instead of inlining function calls or compound expressions.”
🤖 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-rest-controller.php` around lines 1414 - 1422, The scheme check in url_to_resolvable_path() is currently inlined inside the conditional, which violates the named-check guideline. Update the function to assign the result of strpos() to a clearly named variable before the branching, then use that variable in the early return check. Keep the change localized to url_to_resolvable_path() and preserve the existing behavior for URLs without a scheme.Source: Coding guidelines
wordpress-plugin/gk-block-mcp/gk-block-mcp.php (1)
346-354: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep this migration docblock current instead of version-anchored.
“The 2.1 read layer” will go stale immediately and already clashes with this function’s 1.5.0 migration context. Please describe the present behavior without hard-coding a future release number.
As per coding guidelines, "Comment and docblock text must describe the code’s current behavior only."
🤖 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/gk-block-mcp.php` around lines 346 - 354, Update the migration docblock in the site-aware preferences migration block to describe only the current behavior, not a future release. In the comment near the migration logic, remove the hard-coded “2.1 read layer” reference and rephrase it to explain that saved preferences are preserved verbatim, nothing is rewritten or dropped, and the one-time review notice is only flagged for sites with existing saved preferences. Keep the wording aligned with the migration context handled by this code and avoid any version-anchored language.Source: Coding guidelines
wordpress-plugin/gk-block-mcp/tests/Preferences/PreferencesTest.php (1)
20-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required contract docblocks to these new tests.
Several changed test methods still rely on method names or inline comments to explain intent. This repo requires each test method to carry a docblock with the asserted contract, why the test exists, and the failure mode it pins; the rationale in the pattern-score cases should move out of inline comments and into those docblocks.
As per coding guidelines, "Every test method must have a docblock stating the contract being asserted, why the test exists, what regression it pins, and what the failure mode was" and "Docblocks (not inline comments) should contain the rationale for why a test exists."
Also applies to: 61-66, 95-106, 135-148, 152-191
🤖 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/Preferences/PreferencesTest.php` around lines 20 - 25, Add the required docblocks to each newly changed test method in PreferencesTest so the contract, rationale, regression being pinned, and failure mode are documented directly above the method. Update the affected test methods such as test_core_namespace_is_preferred and the other pattern-score cases so any intent currently implied by the method name or inline comments is captured in a method docblock instead. Keep the assertions unchanged; just move the explanation into the test-level docblocks to satisfy the repository’s test documentation contract.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wordpress-plugin/gk-block-mcp/includes/class-block-reader.php`:
- Around line 278-289: The fallback in class-block-reader.php only replaces
$blocks when parse_blocks( $disk_content ) returns a non-empty array, so empty
authoritative content leaves stale in-memory refs behind. Update the reread path
in the block read/persist flow so get_blocks() always returns the disk truth
after a failed persist, including when get_post_field( 'post_content', $post_id
) is empty. Add a regression test that drives the real concurrent-write failure
path and verifies the returned blocks become empty instead of preserving stale
refs.
In `@wordpress-plugin/gk-block-mcp/tests/REST/EditConflictCasTest.php`:
- Around line 385-387: The temporary query filter around
EditConflictCasTest::get_blocks() is not guaranteed to be removed if an
exception is thrown, which can leak into later tests. Update the test to wrap
the get_blocks() call in a try/finally so the remove_filter('query', $inject)
cleanup always runs, and keep the existing add_filter/remove_filter pairing
localized to this block.
---
Outside diff comments:
In `@wordpress-plugin/gk-block-mcp/includes/class-settings-page.php`:
- Around line 330-338: The score sanitizer in the settings page is treating an
explicit 0 the same as an empty row, so blank-name rows with a submitted score
of 0 are dropped without warning. Update the row-handling logic in the
settings-page sanitizer to detect whether the raw score field was actually
present/non-empty before casting, and use that signal to increment the
dropped-scored counter even when the numeric value becomes 0. Keep the fix near
the existing $ns/$score handling so the warning behavior for malformed rows
remains consistent.
- Around line 513-518: handle_reset() in class-settings-page.php is missing the
app-level trash gate cleanup, so the reset still leaves trash behavior enabled.
Update the reset flow to also delete Post_Manager::ALLOW_TRASH_OPTION
(gk_block_api_allow_trash) alongside the other options, and make sure any
trash-related permission checks use the app-level gate
gk/block-mcp/post/allow-trash rather than the agent account’s capabilities.
- Around line 1448-1473: The addRow() cloning logic in class-settings-page.php
copies the blank template row exactly, so newly added rows inherit an empty
action cell and never get a Remove control. Update addRow() to inject or restore
the row’s Remove button/link after cloning (or clone a row that already includes
the action cell markup), and ensure the control remains wired correctly for
every appended row.
---
Nitpick comments:
In `@wordpress-plugin/gk-block-mcp/gk-block-mcp.php`:
- Around line 346-354: Update the migration docblock in the site-aware
preferences migration block to describe only the current behavior, not a future
release. In the comment near the migration logic, remove the hard-coded “2.1
read layer” reference and rephrase it to explain that saved preferences are
preserved verbatim, nothing is rewritten or dropped, and the one-time review
notice is only flagged for sites with existing saved preferences. Keep the
wording aligned with the migration context handled by this code and avoid any
version-anchored language.
In `@wordpress-plugin/gk-block-mcp/includes/class-rest-controller.php`:
- Around line 1414-1422: The scheme check in url_to_resolvable_path() is
currently inlined inside the conditional, which violates the named-check
guideline. Update the function to assign the result of strpos() to a clearly
named variable before the branching, then use that variable in the early return
check. Keep the change localized to url_to_resolvable_path() and preserve the
existing behavior for URLs without a scheme.
In `@wordpress-plugin/gk-block-mcp/tests/Preferences/PreferencesTest.php`:
- Around line 20-25: Add the required docblocks to each newly changed test
method in PreferencesTest so the contract, rationale, regression being pinned,
and failure mode are documented directly above the method. Update the affected
test methods such as test_core_namespace_is_preferred and the other
pattern-score cases so any intent currently implied by the method name or inline
comments is captured in a method docblock instead. Keep the assertions
unchanged; just move the explanation into the test-level docblocks to satisfy
the repository’s test documentation contract.
In `@wordpress-plugin/gk-block-mcp/tests/REST/UrlResolutionTest.php`:
- Around line 50-54: The docblock above the URL resolution test is too specific
to an external incident and should be made self-contained. Update the comment
near UrlResolutionTest and the regression test description to state the generic
failure mode for the ?post_type=&p={id} URL form, such as resolving to the front
page instead of the intended page post, without mentioning internal docs or
outside context.
🪄 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: ee042d11-b730-488a-ad24-b577bca030ce
📒 Files selected for processing (18)
wordpress-plugin/gk-block-mcp/gk-block-mcp.phpwordpress-plugin/gk-block-mcp/includes/class-block-reader.phpwordpress-plugin/gk-block-mcp/includes/class-block-writer.phpwordpress-plugin/gk-block-mcp/includes/class-preferences.phpwordpress-plugin/gk-block-mcp/includes/class-rest-controller.phpwordpress-plugin/gk-block-mcp/includes/class-settings-page.phpwordpress-plugin/gk-block-mcp/readme.txtwordpress-plugin/gk-block-mcp/tests/Block/BlockCrudTest.phpwordpress-plugin/gk-block-mcp/tests/BlockApiTestCase.phpwordpress-plugin/gk-block-mcp/tests/Connect/SettingsPagePreferencesTest.phpwordpress-plugin/gk-block-mcp/tests/Connect/SettingsPageTabsTest.phpwordpress-plugin/gk-block-mcp/tests/Post/PostManagerTest.phpwordpress-plugin/gk-block-mcp/tests/Preferences/PreferencesMigrationTest.phpwordpress-plugin/gk-block-mcp/tests/Preferences/PreferencesTest.phpwordpress-plugin/gk-block-mcp/tests/REST/EditConflictCasTest.phpwordpress-plugin/gk-block-mcp/tests/REST/RestSummaryTest.phpwordpress-plugin/gk-block-mcp/tests/REST/UrlResolutionTest.phpwordpress-plugin/gk-block-mcp/uninstall.php
✅ Files skipped from review due to trivial changes (1)
- wordpress-plugin/gk-block-mcp/readme.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- wordpress-plugin/gk-block-mcp/includes/class-block-writer.php
Comment on lines
+278
to
+289
| // Persist failed: a concurrent edit moved the row, or the DB is | ||
| // read-only / replica-lagged. The refs we assigned exist only in | ||
| // $blocks, not on disk, so the response must surface disk truth | ||
| // instead — otherwise a follow-up write-by-ref targets a ref that | ||
| // was never persisted. Re-read the authoritative content. | ||
| $this->invalidate( $post_id ); | ||
| $disk_content = (string) get_post_field( 'post_content', $post_id ); | ||
| $disk_blocks = parse_blocks( $disk_content ); | ||
| if ( is_array( $disk_blocks ) && ! empty( $disk_blocks ) ) { | ||
| $blocks = $disk_blocks; | ||
| $this->parse_cache[ $post_id . ':' . md5( $disk_content ) ] = $blocks; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return disk truth even when the concurrent winner emptied the post.
This fallback only swaps in the authoritative reread when parse_blocks( $disk_content ) is non-empty. If the competing writer cleared post_content, $blocks stays on the stale in-memory tree with refs that were never persisted, so get_blocks() can still return blocks that no longer exist.
Proposed fix
- $disk_content = (string) get_post_field( 'post_content', $post_id );
- $disk_blocks = parse_blocks( $disk_content );
- if ( is_array( $disk_blocks ) && ! empty( $disk_blocks ) ) {
- $blocks = $disk_blocks;
- $this->parse_cache[ $post_id . ':' . md5( $disk_content ) ] = $blocks;
- }
+ $disk_content = (string) get_post_field( 'post_content', $post_id );
+ $disk_blocks = parse_blocks( $disk_content );
+ $blocks = is_array( $disk_blocks ) ? $disk_blocks : array();
+ $this->parse_cache[ $post_id . ':' . md5( $disk_content ) ] = $blocks;Based on learnings, every bug fix must include a regression test that fails before the fix, passes after it, and exercises the real system path rather than a bare method call.
📝 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
| // Persist failed: a concurrent edit moved the row, or the DB is | |
| // read-only / replica-lagged. The refs we assigned exist only in | |
| // $blocks, not on disk, so the response must surface disk truth | |
| // instead — otherwise a follow-up write-by-ref targets a ref that | |
| // was never persisted. Re-read the authoritative content. | |
| $this->invalidate( $post_id ); | |
| $disk_content = (string) get_post_field( 'post_content', $post_id ); | |
| $disk_blocks = parse_blocks( $disk_content ); | |
| if ( is_array( $disk_blocks ) && ! empty( $disk_blocks ) ) { | |
| $blocks = $disk_blocks; | |
| $this->parse_cache[ $post_id . ':' . md5( $disk_content ) ] = $blocks; | |
| } | |
| // Persist failed: a concurrent edit moved the row, or the DB is | |
| // read-only / replica-lagged. The refs we assigned exist only in | |
| // $blocks, not on disk, so the response must surface disk truth | |
| // instead — otherwise a follow-up write-by-ref targets a ref that | |
| // was never persisted. Re-read the authoritative content. | |
| $this->invalidate( $post_id ); | |
| $disk_content = (string) get_post_field( 'post_content', $post_id ); | |
| $disk_blocks = parse_blocks( $disk_content ); | |
| $blocks = is_array( $disk_blocks ) ? $disk_blocks : array(); | |
| $this->parse_cache[ $post_id . ':' . md5( $disk_content ) ] = $blocks; |
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 287-287: Do not use a weak hash algorithm
Context: md5( $disk_content )
Note: [CWE-328] Use of Weak Hash.
(weak-hash-algorithm)
🤖 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-block-reader.php` around lines
278 - 289, The fallback in class-block-reader.php only replaces $blocks when
parse_blocks( $disk_content ) returns a non-empty array, so empty authoritative
content leaves stale in-memory refs behind. Update the reread path in the block
read/persist flow so get_blocks() always returns the disk truth after a failed
persist, including when get_post_field( 'post_content', $post_id ) is empty. Add
a regression test that drives the real concurrent-write failure path and
verifies the returned blocks become empty instead of preserving stale refs.
Source: Learnings
Comment on lines
+385
to
+387
| add_filter( 'query', $inject ); | ||
| $blocks = $this->crud->get_blocks( $post_id ); | ||
| remove_filter( 'query', $inject ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Remove the query filter in a finally block.
If get_blocks() throws, the query filter remains installed and can contaminate later tests. Wrap the call so cleanup is guaranteed.
Proposed adjustment
add_filter( 'query', $inject );
-$blocks = $this->crud->get_blocks( $post_id );
-remove_filter( 'query', $inject );
+try {
+ $blocks = $this->crud->get_blocks( $post_id );
+} finally {
+ remove_filter( 'query', $inject );
+}📝 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( 'query', $inject ); | |
| $blocks = $this->crud->get_blocks( $post_id ); | |
| remove_filter( 'query', $inject ); | |
| add_filter( 'query', $inject ); | |
| try { | |
| $blocks = $this->crud->get_blocks( $post_id ); | |
| } finally { | |
| remove_filter( 'query', $inject ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wordpress-plugin/gk-block-mcp/tests/REST/EditConflictCasTest.php` around
lines 385 - 387, The temporary query filter around
EditConflictCasTest::get_blocks() is not guaranteed to be removed if an
exception is thrown, which can leak into later tests. Update the test to wrap
the get_blocks() call in a try/finally so the remove_filter('query', $inject)
cleanup always runs, and keep the existing add_filter/remove_filter pairing
localized to this block.
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/includes/class-connect-page.php`:
- Around line 709-721: The HTTPS/remediation state in connection_state() is
being chosen using wp_is_application_passwords_supported() alone, which can
misclassify local/dev sites as app_passwords_disabled even when HTTPS is not
actually active. Update the branching in connection_state() to gate the “HTTPS
is on” copy on is_ssl(), and only return app_passwords_disabled when SSL is
present but Application Passwords are still unavailable; otherwise keep
returning needs_https.
In `@wordpress-plugin/gk-block-mcp/tests/Connect/ConnectPageTest.php`:
- Around line 1633-1679: The new
`test_connection_state_app_passwords_disabled_when_https_on()` can be
nondeterministic if `WP_ENVIRONMENT_TYPE` is `local`, because
`connection_state()` may short-circuit through
`wp_is_application_passwords_supported()` and ignore the HTTPS branch. Make the
test independent of runner environment by explicitly controlling or mocking the
environment-type/application-password support path, or by asserting against a
setup that cannot be treated as `local`. Keep the assertions for
`Connect_Page::connection_state()` focused on the intended
`app_passwords_disabled` versus `needs_https` split.
🪄 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: cd9f39e4-c208-428a-8f86-c88cadd9576d
📒 Files selected for processing (4)
wordpress-plugin/gk-block-mcp/includes/class-app-password-issuer.phpwordpress-plugin/gk-block-mcp/includes/class-connect-page.phpwordpress-plugin/gk-block-mcp/readme.txtwordpress-plugin/gk-block-mcp/tests/Connect/ConnectPageTest.php
✅ Files skipped from review due to trivial changes (2)
- wordpress-plugin/gk-block-mcp/includes/class-app-password-issuer.php
- wordpress-plugin/gk-block-mcp/readme.txt
Comment on lines
709
to
721
| public function connection_state() { | ||
| if ( ! wp_is_application_passwords_available() ) { | ||
| // Core reports Application Passwords unavailable for two unrelated | ||
| // reasons: the site is not on HTTPS, or HTTPS is fine but a plugin, | ||
| // constant, or the wp_is_application_passwords_available filter has | ||
| // switched the feature off. Only the first is solved by enabling | ||
| // HTTPS, so the two states carry different remediation copy. | ||
| if ( wp_is_application_passwords_supported() ) { | ||
| return 'app_passwords_disabled'; | ||
| } | ||
|
|
||
| return 'needs_https'; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the WordPress test suite (WP_UnitTestCase) set WP_ENVIRONMENT_TYPE to 'local' by default?
💡 Result:
No, the WordPress test suite (WP_UnitTestCase) does not automatically set WP_ENVIRONMENT_TYPE to 'local' by default. [1][2] WP_UnitTestCase focuses on bootstrapping the WordPress environment, resetting global state, and providing testing utilities [1][2][3]. It does not natively define or enforce the WP_ENVIRONMENT_TYPE constant [1][2]. If you are seeing WP_ENVIRONMENT_TYPE set to 'local', it is likely being defined elsewhere in your testing infrastructure: 1. Environment Configuration: In many development setups, such as the WordPress core development repository, a.env file or local environment configuration explicitly sets LOCAL_WP_ENVIRONMENT_TYPE=local [4][5]. 2. Testing Bootstrap: You or your testing framework may be setting this constant in your tests/bootstrap.php file or via your phpunit.xml configuration to ensure consistency during test execution [6][7][8]. 3. WP-ENV: If you use the @wordpress/env package, it manages its own environment variables for the containers it spins up, which may default to specific settings depending on the configuration [9][10]. Because wp_get_environment_type caches the environment type in a static variable, it can be difficult to change during a single PHP process's lifecycle [11]. If your tests depend on a specific environment type, it is recommended to explicitly set the WP_ENVIRONMENT_TYPE constant in your test bootstrap file or phpunit.xml before the WordPress environment is fully loaded [6][7].
Citations:
- 1: https://github.com/WordPress/wordpress-develop/blob/trunk/tests/phpunit/includes/abstract-testcase.php
- 2: https://github.com/wp-phpunit/wp-phpunit/blob/master/includes/abstract-testcase.php
- 3: https://taylorlovett.com/wp_unittestcase-the-hidden-api/
- 4: https://github.com/WordPress/wordpress-develop/blob/86dc3ffe0bdcde726ac967efa3e6df508fc68617/.env
- 5: https://github.com/WordPress/wordpress-develop/blob/6.6/.env
- 6: https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/
- 7: https://github.com/wp-phpunit/docs
- 8: https://developer.wordpress.org/news/2025/12/how-to-add-automated-unit-tests-to-your-wordpress-plugin/
- 9: https://developer.wordpress.org/block-editor/reference-guides/packages/packages-env/
- 10: https://learn.rtcamp.com/courses/testing/l/setting-up-unit-tests/
- 11: https://core.trac.wordpress.org/ticket/51752
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant implementation and tests.
git ls-files 'wordpress-plugin/gk-block-mcp/includes/class-connect-page.php' \
'wordpress-plugin/gk-block-mcp/tests/**' | sed 's#^`#FILE` #'
echo
echo "=== Outline: class-connect-page.php ==="
ast-grep outline wordpress-plugin/gk-block-mcp/includes/class-connect-page.php --view expanded
echo
echo "=== Relevant snippets ==="
sed -n '690,770p' wordpress-plugin/gk-block-mcp/includes/class-connect-page.php
echo
echo "=== Search for connection_state/render_section tests ==="
rg -n "connection_state|app_passwords_disabled|needs_https|HTTPS is on|secure \(HTTPS is on\)" wordpress-plugin/gk-block-mcp/tests wordpress-plugin/gk-block-mcp/includes/class-connect-page.phpRepository: GravityKit/block-mcp
Length of output: 24222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect WordPress environment-type and SSL logic to determine whether the
# copy can diverge from the actual site state.
python3 - <<'PY'
print("placeholder")
PYRepository: GravityKit/block-mcp
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact implementation and any unit tests touching the state/copy.
sed -n '709,770p' wordpress-plugin/gk-block-mcp/includes/class-connect-page.php
echo
echo "=== Tests mentioning application passwords / environment type ==="
rg -n "wp_is_application_passwords_supported|wp_is_application_passwords_available|WP_ENVIRONMENT_TYPE|is_ssl\(|app_passwords_disabled|needs_https" wordpress-plugin/gk-block-mcp/testsRepository: GravityKit/block-mcp
Length of output: 8389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Test case around the asserted branch ==="
sed -n '1630,1670p' wordpress-plugin/gk-block-mcp/tests/Connect/ConnectPageTest.php
echo
echo "=== Search for WP_ENVIRONMENT_TYPE / environment bootstrap ==="
rg -n "WP_ENVIRONMENT_TYPE|wp_get_environment_type|is_ssl\(|local" wordpress-plugin/gk-block-mcp/tests/bootstrap-wp.php wordpress-plugin/gk-block-mcp/tests/phpunit.xml wordpress-plugin/gk-block-mcp/tests/phpunit/*.xml wordpress-plugin/gk-block-mcp/tests/Connect/ConnectPageTest.phpRepository: GravityKit/block-mcp
Length of output: 4074
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1688,1718p' wordpress-plugin/gk-block-mcp/includes/class-connect-page.phpRepository: GravityKit/block-mcp
Length of output: 1449
Gate the HTTPS copy on is_ssl() here. wp_is_application_passwords_supported() can be true on WP_ENVIRONMENT_TYPE=local even without HTTPS, so app_passwords_disabled can still show “HTTPS is on” on a local/dev site when Application Passwords are filtered off.
🤖 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-connect-page.php` around lines
709 - 721, The HTTPS/remediation state in connection_state() is being chosen
using wp_is_application_passwords_supported() alone, which can misclassify
local/dev sites as app_passwords_disabled even when HTTPS is not actually
active. Update the branching in connection_state() to gate the “HTTPS is on”
copy on is_ssl(), and only return app_passwords_disabled when SSL is present but
Application Passwords are still unavailable; otherwise keep returning
needs_https.
Comment on lines
+1633
to
+1679
| /** | ||
| * connection_state() must not report 'needs_https' when HTTPS is on. | ||
| * | ||
| * Regression: connection_state() collapsed every reason core reports | ||
| * Application Passwords unavailable into 'needs_https', which renders an | ||
| * "HTTPS required" notice. On a site that already serves HTTPS but has | ||
| * Application Passwords switched off (a security plugin, a hardening | ||
| * constant, or the wp_is_application_passwords_available filter), that | ||
| * message is a misdiagnosis — it sent users to chase an HTTPS problem that | ||
| * did not exist. The state must now distinguish the two: 'needs_https' | ||
| * only when the site genuinely lacks HTTPS, 'app_passwords_disabled' when | ||
| * HTTPS is fine but the feature is off. | ||
| */ | ||
| public function test_connection_state_app_passwords_disabled_when_https_on() { | ||
| $page = new Connect_Page(); | ||
| $original_https = isset( $_SERVER['HTTPS'] ) ? $_SERVER['HTTPS'] : null; | ||
| $original_server_port = isset( $_SERVER['SERVER_PORT'] ) ? $_SERVER['SERVER_PORT'] : null; | ||
|
|
||
| remove_filter( 'wp_is_application_passwords_available', '__return_true' ); | ||
| add_filter( 'wp_is_application_passwords_available', '__return_false' ); | ||
|
|
||
| try { | ||
| // HTTPS on (Apache native SSL termination) but feature disabled. | ||
| $_SERVER['HTTPS'] = 'on'; | ||
| $this->assertTrue( is_ssl(), 'guard: is_ssl() must be true with HTTPS on' ); | ||
| $this->assertSame( 'app_passwords_disabled', $page->connection_state() ); | ||
|
|
||
| // HTTPS genuinely absent → the real HTTPS-required state. | ||
| unset( $_SERVER['HTTPS'] ); | ||
| $_SERVER['SERVER_PORT'] = '80'; | ||
| $this->assertFalse( is_ssl(), 'guard: is_ssl() must be false without HTTPS' ); | ||
| $this->assertSame( 'needs_https', $page->connection_state() ); | ||
| } finally { | ||
| if ( null === $original_https ) { | ||
| unset( $_SERVER['HTTPS'] ); | ||
| } else { | ||
| $_SERVER['HTTPS'] = $original_https; | ||
| } | ||
|
|
||
| if ( null === $original_server_port ) { | ||
| unset( $_SERVER['SERVER_PORT'] ); | ||
| } else { | ||
| $_SERVER['SERVER_PORT'] = $original_server_port; | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Good regression coverage, but see the linked environment-type caveat.
Docblock clearly states the regression, symptom, and fix contract per guidelines. The one risk: as noted on connection_state(), this test's needs_https assertion could go green/red depending on the test runner's WP_ENVIRONMENT_TYPE, since wp_is_application_passwords_supported() short-circuits true for 'local' regardless of is_ssl(). Worth confirming CI does not set that constant, or the underlying connection_state() fix (checking is_ssl() directly) removes the dependency entirely.
🤖 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/ConnectPageTest.php` around lines
1633 - 1679, The new
`test_connection_state_app_passwords_disabled_when_https_on()` can be
nondeterministic if `WP_ENVIRONMENT_TYPE` is `local`, because
`connection_state()` may short-circuit through
`wp_is_application_passwords_supported()` and ignore the HTTPS branch. Make the
test independent of runner environment by explicitly controlling or mocking the
environment-type/application-password support path, or by asserting against a
setup that cannot be treated as `local`. Keep the assertions for
`Connect_Page::connection_state()` focused on the intended
`app_passwords_disabled` versus `needs_https` split.
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
3 participants
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.
Promotes the queued
developwork tomainfor the next release. Opened per thebranch → develop → mainflow.Included now
noindexschema type no longer takes the assistant's tool list offline on Gemini clients. (Thanks @dallinchase.)phpstan-bootstrap.phpexcluded from the plugin zip.mainso the delete-by-ref hotfix and its changelog entry rejoin thedevelopline (no longer diverged).Rolling in before merge
Two PRs are in review against
developand will appear here automatically once merged:npm audit0).Before merging
= develop =into= X.Y.Z on <date> =inreadme.txt, and update the pluginVersion/Stable tag(and the serverpackage.jsonif releasing it together).Hold until #32 and #33 land in
develop.💾 Build file (e0157b9).
Summary by CodeRabbit
Release Notes
noindexinput schema to a single boolean type (documenting null behavior).