✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

@claude can you review this?

Claude finished @AllTerrainDeveloper's task in 3m 9s —— View job

PR Review: Allow deactivating plugins in CMO desktop & dock

• Gather context and read changed files
• Review PHP changes (includes/core/payload.php)
• Review TypeScript changes (src/dock.ts, src/item-visibility-menu.ts)
• Review tests (tests/phpunit/tests/desktopModeBuildDockItems.php)
• Review docs (docs/javascript-reference.md)
• Post final review

This is a well-structured feature. The separation of concerns is clean (PHP resolves ownership, JS acts on it), the documentation is thorough, and the code correctly integrates with the existing trackedFetch, wpdConfirm, and refreshMenu() plumbing. A few things worth addressing:

🔴 Missing positive test for desktop_mode_resolve_menu_plugin_file()

tests/phpunit/tests/desktopModeBuildDockItems.php:697–746 — all three added tests verify null returns (core slugs, unknown slugs, Desktop Mode itself). There is no test that exercises the successful resolution path — i.e., a menu page whose hook has a callback inside WP_PLUGIN_DIR that matches an installed plugin.

If the $wp_filter walk or the folder-matching logic silently regresses (wrong offset math, path normalization edge case, get_plugins() key shape assumption), no test would catch it. The feature would silently show no "Deactivate" action without any visible error.

A minimal positive test would need a synthetic callback registered on a fake hookname whose getFileName() returns a path under WP_PLUGIN_DIR, plus a matching entry in the plugins list. Fix this →%20register%20a%20closure%20callback%20on%20a%20fake%20hookname%20from%20get_plugin_page_hookname%2C%202)%20mock%20or%20ensure%20the%20callback%27s%20declaring%20file%20path%20falls%20under%20WP_PLUGIN_DIR%2C%20and%203)%20assert%20the%20function%20returns%20the%20expected%20plugin%20basename%20string%20(not%20null).&repo=WordPress/desktop-mode)

🟡 Separator color won't adapt to light themes

src/item-visibility-menu.ts:229–231:

hr.style.cssText =
    'border: 0; border-top: 1px solid rgba(255,255,255,0.12); margin: 4px 6px;';

rgba(255,255,255,0.12) is baked in. If the shell gets a light-mode dock or a user-overridden theme, this renders a near-invisible hairline. The rest of the component system uses CSS custom properties (design tokens). A CSS variable like var(--wpd-context-menu-separator-color, rgba(255,255,255,0.12)) would let themes override this without touching JS. Fix this →

🟡 Redundant require_once guards

includes/core/payload.php:462–467:

if ( ! function_exists( 'get_plugin_page_hookname' ) ) {
    require_once ABSPATH . 'wp-admin/includes/plugin.php';
}
if ( ! function_exists( 'get_plugins' ) ) {
    require_once ABSPATH . 'wp-admin/includes/plugin.php';
}

Both functions live in the same file. If the first block fires it loads both; if the first block doesn't fire, get_plugin_page_hookname already exists (which means plugin.php is already loaded, so get_plugins is also available). The second check can never independently trigger the require_once. These can be merged into one guard:

if ( ! function_exists( 'get_plugin_page_hookname' ) || ! function_exists( 'get_plugins' ) ) {
    require_once ABSPATH . 'wp-admin/includes/plugin.php';
}

or just a single unconditional require_once (it's idempotent). Fix this →

✅ Things that look good

• Correct ARIA semantics: <wpd-context-menu> carries role="menu" (wpd-context-menu.ts:81); <hr> has an implicit HTML role="separator" inside a role="menu" — correct per ARIA menu pattern.
• refreshMenu() is the right call: It's a Stable API (javascript-reference.md:3272) that spawns a hidden chromeless iframe to probe the real admin context — exactly what's needed after a REST deactivation that didn't go through plugins.php.
• Plugin file encoding is correct: .php strip + per-segment encodeURIComponent + literal / matches Core's (?P<plugin>[^.\/]+(?:\/[^.\/]+)?) regex expectation, and mirrors the existing encodePluginPath() in the plugins-window. The server-side get_plugins() cross-check prevents arbitrary file paths from escaping.
• Desktop Mode self-exclusion is correct: The DESKTOP_MODE_FILE guard at payload.php:507–509 ensures the shell can never accidentally offer to deactivate itself.
• trackedFetch / wpdConfirm usage is correct: Follows the project's conventions from AGENTS.md.
• pluginFile defaults to null for core menus / mu-plugins / drop-ins — safe default for the deactivate path, even if desktop_mode_resolve_menu_plugin_file() somehow changes behaviour.

The two 🟡 items are low risk but easy to fix. The 🔴 missing test is the one worth addressing before merge — the algorithm is non-trivial enough that a regression path (file path comparison off-by-one, normalization edge case on Windows-style paths) would silently fall back to "no deactivate option" with no signal.

@claude can you review?

Claude finished @AllTerrainDeveloper's task in 3m 18s —— View job

PR Review: Allow deactivating plugins in CMO desktop & dock

• Read includes/core/payload.php
• Read src/dock.ts
• Read src/item-visibility-menu.ts
• Read tests/phpunit/tests/desktopModeBuildDockItems.php
• Read docs/javascript-reference.md
• Post final review

All four issues from the prior review are resolved in the latest commits: parse_url → wp_parse_url, separator now uses a CSS variable fallback, redundant require_once guards merged, and a positive resolver test added. Three new findings below.

🔴 Orphaned docblock on desktop_mode_plugin_file_for_path()

includes/core/payload.php — two consecutive docblocks appear in sequence, with no function between them:

/**
 * Map an arbitrary filesystem path inside `WP_PLUGIN_DIR` to the
 * corresponding plugin file in `get_plugins()`…
 * @param string $file Absolute filesystem path.
 * @return string|null Plugin file (`<folder>/<file>.php`) or null.
 */
/**                              ← second docblock immediately follows
 * Look up the human-readable display name for a plugin file…
 */
function desktop_mode_plugin_display_name( $plugin_file ) {

PHP's docblock convention (and every doc generator / IDE tooling) attaches a docblock to the immediately following declaration. The first block — intended for desktop_mode_plugin_file_for_path() — is being swallowed by desktop_mode_plugin_display_name(). The actual desktop_mode_plugin_file_for_path() function then has no docblock at all. This is a copy-paste artifact: the @param/@return for the path-mapper is silently lost, and IDEs will show the path-mapper as undocumented while showing display_name with the wrong signature. Fix this →

🟡 Self-exclusion test doesn't exercise the self-exclusion guard

tests/phpunit/tests/desktopModeBuildDockItems.php — test_resolve_plugin_file_excludes_desktop_mode_itself registers a closure declared inside the test file and asserts null. The comment at the bottom admits why:

"The closure is declared in this test file, which lives under tests/phpunit/tests/ — outside WP_PLUGIN_DIR — so the resolver must return null."

The null return here comes from "no frame in WP_PLUGIN_DIR", not from the $self_basename guard (payload.php:472–476). The self-exclusion branch is never reached. A real test would need a callback whose declaring file is under WP_PLUGIN_DIR and whose plugin basename equals DESKTOP_MODE_FILE (or mock the constant). As written, the test validates the right result but for the wrong reason — a regression in the self-exclusion guard would go undetected. Fix this →

🟡 Positive test cleanup can leak the fixture on failure

tests/phpunit/tests/desktopModeBuildDockItems.php:738–780 — test_resolve_plugin_file_returns_plugin_basename… writes a real file to WP_PLUGIN_DIR, then cleans up at the bottom of the test. The cleanup runs only when the method exits normally. If $this->assertSame( $fake_basename, $resolved ) throws (i.e., the test fails), the @unlink and @rmdir calls are skipped, leaving the fixture directory on disk. Subsequent test runs then find an existing dm-positive-resolver-fixture/ and skip mkdir, which is fine — but get_plugins() will pick up the fixture file as an installed plugin in every future run on that environment. A try/finally block around the body (or moving cleanup into tearDown) would guarantee it always runs. Fix this →

✅ Things that look good

• Three-strategy resolution is sound: attribution tracker → CPT/taxonomy backtrace → page-hook reflection. Each layer has documented failure modes and the fallbacks are safe (false negatives only — null means no deactivate option, never a false positive).
• $wp_filter callback mutation is safe: the tracker preserves the same $id key (which WP derives from the original callable), so remove_action($orig) still correctly targets the wrapper.
• REST URL encoding is correct: .php strip + per-segment encodeURIComponent + literal / matches Core's (?P<plugin>[^.\/]+(?:\/[^.\/]+)?) regex constraint. Same pattern as the plugins-window's encodePluginPath().
• closeWindowsForPlugin owns four independent ownership checks (by id, by config.baseId, by config.url-derived id, by live iframe.src-derived id). The cross-origin iframe try/catch correctly handles SecurityError.
• wp.desktop.refreshMenu() call after deactivation is correct: the REST deactivation doesn't go through plugins.php, so the desktop-mode-plugins-changed postMessage won't fire automatically — refreshMenu() is the right trigger for the hidden-iframe probe.
• wpdConfirm, trackedFetch, showToast usage all follows project conventions (AGENTS.md).
• Desktop Mode self-exclusion in PHP is correct (DESKTOP_MODE_FILE guard at payload.php:472–476).
• pluginFile defaults to null for core menus, mu-plugins, and drop-ins — the dock right-click can never produce a false "Deactivate" action for non-deactivatable items.