Skip to content

fix(templates): hybrid themes (wp_is_block_theme() false) still list/edit real templates — BLOCK-37 - #61

Merged
zackkatz merged 2 commits into
developfrom
feature/block-37-template-tools-return-empty-on-hybrid-themes
Jul 23, 2026
Merged

fix(templates): hybrid themes (wp_is_block_theme() false) still list/edit real templates — BLOCK-37#61
zackkatz merged 2 commits into
developfrom
feature/block-37-template-tools-return-empty-on-hybrid-themes

Conversation

@zackkatz

@zackkatz zackkatz commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

Found by live validation on staging.gravitykit.com (BLOCK-37): the gravitykit theme (0.4.0) is a hybrid theme — wp_is_block_theme() returns false (no templates/index.html on disk) but get_block_templates() finds 1 real template and 25 real template parts via theme files directly. Every template-tool method gated on ! wp_is_block_theme() instead of whether the id/type actually resolves:

  • get_templates() short-circuited on the classic-theme guard and returned an empty list with a note ("Active theme is not a block theme; no block templates exist") that was factually wrong on this site — it hid all 25 real parts.
  • update_template() / reset_template() returned a classic_theme 400 before ever attempting resolution, blocking gated edits to parts that genuinely render on the site.

Verified live (from the issue):

$ wp eval '…'  → classic|templates:1|parts:25|theme:gravitykit
GET /gk-block-api/v1/templates → {"templates":[],"count":0,"note":"Active theme is not a block theme; no block templates exist."}

Fix

  • get_templates(): always queries get_block_templates(). The note is attached only when the result is empty AND wp_is_block_theme() is false — reworded so it no longer asserts nonexistence when the real situation is "nothing matched this query" (Active theme is not a full block theme; only registered block templates/parts are listed.).
  • update_template() / reset_template(): the id-required and content/blocks-mutual-exclusivity checks now run first (unchanged behavior); resolution via get_block_template( $id, $type ) is the primary gate, matching get_template()'s existing (already-correct) behavior. The wp_is_block_theme() check moves to a fallback inside the not-found branch — it only fires when nothing resolves at all AND the theme isn't a block theme, so a genuinely classic theme still gets the same specific, actionable classic_theme 400 it always did, while a hybrid theme's resolvable part now succeeds.
  • TS: list_templates' tool description no longer implies "block theme" is the qualifying condition for listing anything.

Why this design (not just deleting the guard)

Deleting wp_is_block_theme() outright and relying purely on the existing not_found 404 would have been simpler, but it would silently change the error code/status for a genuinely classic theme (no templates/parts at all) from classic_theme/400 to a generic not_found/404 — a real behavior regression the issue explicitly asked to avoid ("Full block theme and truly-classic behaviors unchanged"). Keeping classic_theme as a fallback inside the not-found branch gets both: hybrid themes are gated on resolution (the actual fix), and a truly classic theme keeps its more specific, actionable message.

Strict TDD — visible red/green

  • 107a3c2 test(templates): pin BLOCK-37 hybrid-theme template regression [red]
  • 3d15bc1 fix(templates): gate on template resolution, not wp_is_block_theme() [green]

Fixture: tests/fixtures/themes/hybrid-theme/ — real templates/single.html and parts/footer.html, deliberately no templates/index.html or block-templates/index.html, reproducing staging's exact shape. The red tests assert wp_is_block_theme() === false against this fixture before asserting anything else, so a broken fixture fails loudly as a fixture problem, not a false green.

New/changed tests:

  • test_get_templates_hybrid_theme_lists_real_templates_and_parts — lists the hybrid theme's real template + part, no note
  • test_get_templates_full_block_theme_empty_result_has_no_note — pins that "empty" alone (on a real block theme, e.g. an area filter matching nothing) never gets the note — it's not "any empty result"
  • test_update_template_creates_override_for_hybrid_theme_template_part / test_reset_template_reverts_hybrid_theme_override — gated write/reset succeed against a hybrid theme's resolvable part
  • test_update_template_classic_theme_still_returns_400_when_nothing_resolves — regression pin: a genuinely classic theme (nothing resolves) still gets classic_theme/400, not a generic not_found

All 3 hybrid-theme behavioral assertions failed against pre-fix code for the expected reason (verified before writing the fix); the classic-theme regression pin already passed pre-fix (asserting existing behavior that must not change) and stays green post-fix.

One test-infra fix rides along in the green commit: search_theme_directories() memoizes its scan in a function-local static, so register_theme_directory() alone doesn't make a newly-registered fixture root visible once anything earlier in the run has already forced a scan (which, in this suite, is always true by the time these tests run). wp_clean_themes_cache() forces the re-scan — without it the red tests were red for the wrong reason (fixture not found) rather than the real bug.

Verification

composer test    → 1430 + 28 + 11 + 5 = 1474 tests, 0 failures (was 1469 before BLOCK-37, +5 new)
composer lint     → 0 errors, 0 warnings
composer analyze  → [OK] No errors (PHPStan)
npm test          → 55 files / 859 tests passed
npx tsc --noEmit  → clean
manifest drift    → npx tsx scripts/export-abilities-manifest.mjs; git diff → tool count unchanged (33), only list_templates' description line changed
npm run build     → bundle regenerated + copied into wordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjs

No Siteminter smoke in this PR — not required per the assignment; the coordinator will redeploy to staging.gravitykit.com and re-run the live template validation after merge.

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

Summary by CodeRabbit

  • New Features

    • Template browsing now works with hybrid themes, including themes that are not fully classified as block themes.
    • Templates and template parts can be updated and reset when they can be resolved from the active theme.
    • Empty results now include explanatory notes only when no templates are available.
  • Bug Fixes

    • Improved handling of unresolved templates with clearer distinction between unavailable themes and missing template IDs.

💾 Build file (3d15bc1).

zackkatz added 2 commits July 22, 2026 21:00
staging.gravitykit.com's "gravitykit" theme is a hybrid: wp_is_block_theme()
is false (no templates/index.html), but get_block_templates() finds real
templates and parts via the templates/ and parts/ folders directly (1
template, 25 parts, verified live). Every template tool method gates on
`! wp_is_block_theme()` instead of whether the id/type actually resolves,
so it hides those templates from list_templates and blocks writes to parts
that genuinely render on the site.

Adds a "hybrid-theme" fixture (real templates/ and parts/ files, no
templates/index.html or block-templates/index.html) plus:
- get_templates() must list a hybrid theme's real templates/parts, no
  misleading "not a block theme" note
- the note must stay absent for a real block theme's genuinely-empty
  result (regression pin — the note isn't "any empty result")
- update_template()/reset_template() must succeed against a hybrid
  theme's resolvable part
- a genuinely classic theme (nothing resolves) must keep the specific,
  actionable classic_theme 400 rather than regressing to a generic
  not_found — pins that the fix doesn't just delete the guard

All 3 hybrid-theme behavioral assertions fail against current code;
the classic-theme regression pin already passes (asserting current
behavior we must not break).

Claude-Session: https://claude.ai/code/session_013YcSbKroBJjPanX3okQrT3
…[green]

get_templates() / update_template() / reset_template() all short-circuited
on `! wp_is_block_theme()`, which is only true when a theme ships
templates/index.html (or block-templates/index.html). A "hybrid" theme
can have real, renderable templates and template parts without that
specific file — staging.gravitykit.com's "gravitykit" theme is exactly
this shape (1 template, 25 parts, wp_is_block_theme() false) — and the
guard hid all of them from list_templates while blocking gated writes to
parts that genuinely render on the site.

- get_templates(): always queries get_block_templates(); the "not a
  block theme" note is now attached only when the result is empty AND
  wp_is_block_theme() is false, reworded so it no longer asserts
  nonexistence when the truth is "nothing matched this query."
- update_template() / reset_template(): the wp_is_block_theme() 400 now
  fires only as a fallback when get_block_template( $id, $type ) fails
  to resolve at all AND the theme isn't a block theme — the primary gate
  is resolution, matching get_template()'s existing behavior. A
  genuinely classic theme (nothing resolves) keeps the same specific,
  actionable classic_theme 400 it always returned; a hybrid theme's
  resolvable part now succeeds instead of being rejected up front.

Test infra: fixes a wp-phpunit gotcha the red commit's fixture tripped —
search_theme_directories() memoizes its scan in a function-local static,
so register_theme_directory() alone doesn't make a newly-registered root
visible once anything else has already forced a scan; wp_clean_themes_cache()
does.

TS: list_templates' description no longer says "block theme" as if that
were the qualifying condition.

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

linear-code Bot commented Jul 23, 2026

Copy link
Copy Markdown

BLOCK-37

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Template listing, updating, and resetting now support resolvable templates and parts in hybrid themes. Tool metadata, tests, and fixture themes were updated to reflect resolution-based behavior and conditional empty-result notes.

Changes

Hybrid Theme Template Support

Layer / File(s) Summary
Hybrid template listing
src/tools/templates.ts, wordpress-plugin/gk-block-mcp/assets/..., wordpress-plugin/gk-block-mcp/includes/abilities/..., wordpress-plugin/gk-block-mcp/includes/class-template-manager.php, wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php, wordpress-plugin/gk-block-mcp/tests/fixtures/themes/hybrid-theme/*
get_templates() lists resolvable templates and parts without requiring a full block theme, adds a note only for empty hybrid-theme results, and updates related metadata and coverage.
Hybrid template write resolution
wordpress-plugin/gk-block-mcp/includes/class-template-manager.php, wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php
update_template() and reset_template() resolve templates before distinguishing classic_theme from not_found, with hybrid template-part override and reset tests.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: hybrid themes can still list and edit real templates even when wp_is_block_theme() is false.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/block-37-template-tools-return-empty-on-hybrid-themes

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 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 `@src/tools/templates.ts`:
- Line 28: Update the template-listing description in the relevant tool
definition to state that wp_id identifies a database-backed template or override
whenever present. Remove the claim that it is non-null only when a database
override shadows a theme file, since Template_Manager::format_template_summary()
treats wp_id independently of has_theme_file.

In `@wordpress-plugin/gk-block-mcp/includes/class-template-manager.php`:
- Around line 167-168: Assign the wp_is_block_theme() result to a named
$is_block_theme variable before conditionals, and extract the compound
empty($formatted) check into a descriptive predicate before the branch in the
relevant template-listing flow. Apply the same named-variable pattern in
update_template() and reset_template(), covering the corresponding checks near
the referenced sections.
- Around line 281-289: Update the unresolved-template handling in the template
lookup flow around get_block_template and the corresponding template-part path:
return the existing not_found WP_Error for missing IDs when the active theme is
hybrid, while reserving classic_theme for genuine classic themes. Add regression
coverage for unresolved hybrid template and template-part IDs, including both
affected branches.

In `@wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php`:
- Around line 98-115: Revise comments at
wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php:98-115 and
:319-325, and
wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php:104-115,
:492-497, and :542-547 to describe only current behavior and contracts:
fixture-root registration/cache invalidation, hybrid listing behavior, local
cache invalidation, hybrid writes, and classic-theme errors respectively. Remove
run-history, old implementation, regression, and off-tree references; update
comments/docblocks around the relevant test methods without changing test
behavior.

In `@wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php`:
- Around line 554-557: In the test around the result assertions, explicitly
narrow or guard $result as a WP_Error after assertInstanceOf before calling
get_error_code() and get_error_data(). Preserve the existing assertions and 400
status validation while making the type refinement visible to PHPStan.
🪄 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: d9533b73-2f85-429e-91a2-7585e9f23af0

📥 Commits

Reviewing files that changed from the base of the PR and between 69c993c and 3d15bc1.

📒 Files selected for processing (9)
  • src/tools/templates.ts
  • wordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjs
  • wordpress-plugin/gk-block-mcp/includes/abilities/tools.manifest.json
  • wordpress-plugin/gk-block-mcp/includes/class-template-manager.php
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php
  • wordpress-plugin/gk-block-mcp/tests/fixtures/themes/hybrid-theme/parts/footer.html
  • wordpress-plugin/gk-block-mcp/tests/fixtures/themes/hybrid-theme/style.css
  • wordpress-plugin/gk-block-mcp/tests/fixtures/themes/hybrid-theme/templates/single.html

Comment thread src/tools/templates.ts
name: 'list_templates',
description:
'List a block theme\'s templates (page layouts like "single", "archive") or template parts (reusable regions like "header", "footer"). Each row includes `wp_id` — non-null only when a database override shadows the theme file, which is what makes a template editable via update_template. On a classic (non-block) theme, returns an empty list with a `note` explaining why.',
'List the active theme\'s templates (page layouts like "single", "archive") or template parts (reusable regions like "header", "footer") — works on a theme without a full block-theme structure too, as long as it has real templates/parts. Each row includes `wp_id` — non-null only when a database override shadows the theme file, which is what makes a template editable via update_template. Returns an empty list with a `note` only when there is truly nothing to list.',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Clarify the wp_id contract.

The PHP formatter exposes wp_id whenever the resolved template has that property; it does not require has_theme_file. Describing it as non-null only when an override shadows a theme file overstates the guarantee. Say that it identifies a database-backed template or override.

Based on Template_Manager::format_template_summary(), wp_id is independent of the has_theme_file field.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tools/templates.ts` at line 28, Update the template-listing description
in the relevant tool definition to state that wp_id identifies a database-backed
template or override whenever present. Remove the claim that it is non-null only
when a database override shadows a theme file, since
Template_Manager::format_template_summary() treats wp_id independently of
has_theme_file.

Comment on lines +167 to +168
if ( empty( $formatted ) && ! wp_is_block_theme() ) {
$result['note'] = __( 'Active theme is not a full block theme; only registered block templates/parts are listed.', 'gk-block-mcp' );

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Assign conditional checks to named variables.

These conditions inline wp_is_block_theme() and, at Line 167, a compound check. Assign the predicates first, then branch, as required by the repository PHP guidelines.

Proposed pattern
-		if ( empty( $formatted ) && ! wp_is_block_theme() ) {
+		$has_templates  = ! empty( $formatted );
+		$is_block_theme = wp_is_block_theme();
+		if ( ! $has_templates && ! $is_block_theme ) {

Apply the same $is_block_theme pattern in update_template() and reset_template().

As per coding guidelines, checks must be assigned to named variables before conditionals.

Also applies to: 283-284, 394-395

🤖 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-template-manager.php` around
lines 167 - 168, Assign the wp_is_block_theme() result to a named
$is_block_theme variable before conditionals, and extract the compound
empty($formatted) check into a descriptive predicate before the branch in the
relevant template-listing flow. Apply the same named-variable pattern in
update_template() and reset_template(), covering the corresponding checks near
the referenced sections.

Source: Coding guidelines

Comment on lines 281 to +289
$template = get_block_template( $id, $type );
if ( ! $template ) {
if ( ! wp_is_block_theme() ) {
return new \WP_Error(
'classic_theme',
__( 'Active theme is not a block theme; there are no block templates to edit.', 'gk-block-mcp' ),
array( 'status' => 400 )
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the file structure first
git ls-files wordpress-plugin/gk-block-mcp/includes/class-template-manager.php

# Show outline if available, then relevant line ranges
if command -v ast-grep >/dev/null 2>&1; then
  ast-grep outline wordpress-plugin/gk-block-mcp/includes/class-template-manager.php --view expanded || true
fi

nl -ba wordpress-plugin/gk-block-mcp/includes/class-template-manager.php | sed -n '240,430p'

Repository: GravityKit/block-mcp

Length of output: 973


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find all occurrences of the error codes and template lookup helpers.
rg -n "classic_theme|not_found|get_block_template|wp_is_block_theme|template-part|footer" wordpress-plugin/gk-block-mcp/includes/class-template-manager.php wordpress-plugin/gk-block-mcp/includes -g '*.php'

Repository: GravityKit/block-mcp

Length of output: 8951


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the public methods that return the template-not-found errors.
python3 - <<'PY'
from pathlib import Path
p = Path('wordpress-plugin/gk-block-mcp/includes/class-template-manager.php')
text = p.read_text()
for needle in ['function', 'classic_theme', 'not_found']:
    pass
# Print the surrounding blocks for methods containing get_block_template
lines = text.splitlines()
for i, line in enumerate(lines, start=1):
    if 'get_block_template' in line or 'classic_theme' in line or 'not_found' in line:
        start = max(1, i-25)
        end = min(len(lines), i+35)
        print(f"\n--- lines {start}-{end} around line {i} ---")
        for n in range(start, end+1):
            print(f"{n:4d}: {lines[n-1]}")
PY

Repository: GravityKit/block-mcp

Length of output: 33623


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read only the relevant section around the two reported ranges.
nl -ba wordpress-plugin/gk-block-mcp/includes/class-template-manager.php | sed -n '260,420p'

Repository: GravityKit/block-mcp

Length of output: 198


🌐 Web query:

WordPress wp_is_block_theme hybrid theme block templates get_block_template missing ID classic_theme not_found

💡 Result:

In WordPress, the behavior you are observing regarding wp_is_block_theme, get_block_template, and template discovery is rooted in how WordPress distinguishes between "full" block themes and hybrid themes [1][2]. wp_is_block_theme Functionality The function wp_is_block_theme specifically checks whether a theme is a block-based theme [3]. Internally, it wraps wp_get_theme->is_block_theme, which determines this status by checking for the presence of an index.html file in either the /templates/ or /block-templates/ directory of the theme [4][5]. If your hybrid theme relies on traditional PHP templates (e.g., index.php) and does not have an index.html block template at the root, wp_is_block_theme will return false, correctly identifying it as a classic or hybrid theme rather than a pure block theme [4][2]. get_block_template and Template Discovery The function get_block_template retrieves a block template object (WP_Block_Template) by its ID (e.g., 'theme_slug//template_slug') [6]. When you call this function for a template that does not exist in the database (customized via the Site Editor), WordPress falls back to get_block_file_template to look for the template file on the file system [7]. If you are encountering a "not found" or null result when calling get_block_template in a hybrid theme: 1. File Location: Ensure your block template files are correctly placed in the /templates/ or /block-templates/ directories of your theme [4][2]. 2. Theme Support: For hybrid themes to utilize block templates, ensure that the theme has explicitly declared support for them. While block template parts (via the /parts directory) can be used with broad theme support, full block templates may require the theme to be recognized as having block-template capabilities [1][8]. 3. Hybrid vs. Full Block: In a hybrid theme, the PHP template hierarchy (index.php, single.php) still takes precedence [2]. If WordPress locates a PHP template for a given request, it may bypass the block template discovery logic unless you are specifically using functions or hooks that force the rendering of a block template [9]. If you are developing a hybrid theme and need to access a block template file, verify that the ID passed to get_block_template matches the expected 'theme_slug//template_slug' format and that the file resides in the expected directory [6][7]. If you are seeing unexpected nulls, it is likely because the internal discovery mechanism is either failing to find the file due to pathing or because the current page request is resolving to a PHP template file, leading WordPress to skip the block template search [9].

Citations:


🌐 Web query:

WordPress wp_is_block_theme hybrid theme classic theme block templates semantics

💡 Result:

In WordPress, themes are generally categorized based on their architecture and compatibility with Full Site Editing (FSE). While WordPress recognizes two official types—Classic and Block—the term Hybrid is widely used by the developer community to describe themes that bridge these two approaches [1][2]. wp_is_block_theme The function wp_is_block_theme is a boolean helper introduced in WordPress 5.9.0 [3][4]. It returns true if the active theme is a Block theme and false otherwise [3]. Under the hood, it acts as a wrapper for wp_get_theme->is_block_theme [5][4]. Specifically, it determines if a theme is a block-based theme by checking for the presence of an index.html block template (usually located in /templates/ or /block-templates/) [3][5]. Theme Semantics 1. Block Themes: These are designed for Full Site Editing (FSE) [6]. They use HTML-based block templates (stored in a /templates/ directory) and a theme.json file to manage global styles, settings, and layout [7]. In these themes, the Site Editor is the primary tool for customization, replacing traditional Customizer menus and widgets [7][8]. 2. Classic Themes: These rely on the traditional WordPress structure, utilizing PHP template files (e.g., index.php, single.php) and the WordPress Customizer for site-wide settings [6][7]. While they support the block editor for post/page content, they do not use block-based site templates [7]. 3. Hybrid Themes: This is a non-official, community-recognized category [1][9]. A hybrid theme is essentially a classic theme that has adopted selected block features [2]. These themes maintain a classic PHP-based structure but incorporate modern elements such as theme.json, block patterns, or block-based template parts [1][2]. This allows developers to offer modern design flexibility while retaining the stability and specific workflows (like the Customizer) associated with classic themes [10][6]. Summary Table Theme Type | Core Architecture | Primary Customization | FSE Support --- | --- | --- | --- Block | HTML Block Templates | Site Editor [7] | Full Classic | PHP Templates [6] | Customizer [7] | None Hybrid | PHP Templates [6] | Customizer + Blocks [6] | Partial [6] Because wp_is_block_theme checks specifically for the presence of block-based template files [3][5], it will return false for both classic and hybrid themes, as neither relies on the full block-template architecture required for a theme to be classified as a Block theme [1][9].

Citations:


Return not_found for unresolved hybrid templates
! wp_is_block_theme() still routes a missing template/template-part ID to classic_theme even when the active theme already resolves other block templates/parts. Reserve that error for genuine classic themes and return not_found for unresolved IDs here; add regression coverage for missing hybrid template and template-part IDs. Also applies to: 392-400.

🤖 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-template-manager.php` around
lines 281 - 289, Update the unresolved-template handling in the template lookup
flow around get_block_template and the corresponding template-part path: return
the existing not_found WP_Error for missing IDs when the active theme is hybrid,
while reserving classic_theme for genuine classic themes. Add regression
coverage for unresolved hybrid template and template-part IDs, including both
affected branches.

Comment on lines +98 to +115
/**
* Register the fixture theme directory containing "hybrid-theme" (a
* theme with templates/ and parts/ files but no templates/index.html,
* so wp_is_block_theme() is false). A separate root from
* ensure_theme_root_resolvable()'s dummy one; by the time it's
* registered the "more than one root" workaround already applies, so
* this one just needs to contain the fixture.
*
* @return void
*/
private function register_hybrid_theme_root() {
register_theme_directory( dirname( __DIR__ ) . '/fixtures/themes' );
// search_theme_directories() memoizes its scan in a function-local
// static for the rest of the process; by this point in the run
// something has always already forced that memoization without
// this root, so appending it here is invisible until forced.
wp_clean_themes_cache();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Keep changed PHP comments limited to current behavior and contracts.

  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php#L98-L115: describe fixture-root registration and cache invalidation without run-history rationale.
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php#L319-L325: state the hybrid listing contract without referring to the old short-circuit.
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L104-L115: state the local cache-invalidation contract instead of referring to another test file.
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L492-L497: state the hybrid write contract without implementation history.
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L542-L547: state the classic-theme error contract without “unchanged regression” framing.

As per coding guidelines, “Comments and docblocks must document current behavior and hard contracts” and must omit historical journals and off-tree references.

📍 Affects 2 files
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php#L98-L115 (this comment)
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php#L319-L325
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L104-L115
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L492-L497
  • wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php#L542-L547
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php` around
lines 98 - 115, Revise comments at
wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerTest.php:98-115 and
:319-325, and
wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php:104-115,
:492-497, and :542-547 to describe only current behavior and contracts:
fixture-root registration/cache invalidation, hybrid listing behavior, local
cache invalidation, hybrid writes, and classic-theme errors respectively. Remove
run-history, old implementation, regression, and off-tree references; update
comments/docblocks around the relevant test methods without changing test
behavior.

Source: Coding guidelines

Comment on lines +554 to +557
$this->assertInstanceOf( \WP_Error::class, $result );
$this->assertSame( 'classic_theme', $result->get_error_code() );
$data = $result->get_error_data();
$this->assertSame( 400, $data['status'] );

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Narrow $result before calling WP_Error methods.

PHPStan still sees array|WP_Error after assertInstanceOf(), so Lines 555-556 fail static analysis.

Proposed fix
-		$this->assertInstanceOf( \WP_Error::class, $result );
+		if ( ! $result instanceof \WP_Error ) {
+			$this->fail( 'Expected a classic_theme error.' );
+			return;
+		}
 		$this->assertSame( 'classic_theme', $result->get_error_code() );
📝 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->assertInstanceOf( \WP_Error::class, $result );
$this->assertSame( 'classic_theme', $result->get_error_code() );
$data = $result->get_error_data();
$this->assertSame( 400, $data['status'] );
if ( ! $result instanceof \WP_Error ) {
$this->fail( 'Expected a classic_theme error.' );
return;
}
$this->assertSame( 'classic_theme', $result->get_error_code() );
$data = $result->get_error_data();
$this->assertSame( 400, $data['status'] );
🧰 Tools
🪛 PHPStan (2.2.5)

[error] 555-555: Cannot call method get_error_code() on array|WP_Error.

(method.nonObject)


[error] 556-556: Cannot call method get_error_data() on array|WP_Error.

(method.nonObject)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wordpress-plugin/gk-block-mcp/tests/Templates/TemplateManagerWriteTest.php`
around lines 554 - 557, In the test around the result assertions, explicitly
narrow or guard $result as a WP_Error after assertInstanceOf before calling
get_error_code() and get_error_data(). Preserve the existing assertions and 400
status validation while making the type refinement visible to PHPStan.

Source: Linters/SAST tools

@zackkatz
zackkatz merged commit 4554dbb into develop Jul 23, 2026
9 checks passed
@zackkatz
zackkatz deleted the feature/block-37-template-tools-return-empty-on-hybrid-themes branch July 23, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant