Skip to content

Commit d2bffd1

Browse files
akirkakirk
committed
Address review feedback
- Leave the login screen uncovered, and say so. `wp-login.php` fires neither `wp_head` nor `admin_head`, so it carries no registry. The docs claimed one exists in every WordPress document and told authors to hook `login_head`, which would have thrown there. Verified against the real form (`reauth=1` reaches it despite the auto-login): no registry, tools withdrawn, restored on leaving. Covered by a test so the hook is not added back by accident. - Guard the polyfill's `Object.defineProperty`. A non-configurable `modelContext` made it throw, which aborted the listener effect and took tool registration, the site proxy and the WebSocket bridge with it, none of which need WebMCP. - Move `reservedToolNames` below its caller, per the repo's caller-then- callee ordering. - Tag the call-flow fence as `text` (markdownlint MD040). Claude-Session: https://claude.ai/code/session_01SES9wgwFmrkkhwqfmfMuMU
1 parent 419033b commit d2bffd1

4 files changed

Lines changed: 62 additions & 19 deletions

File tree

‎packages/playground/mcp/README.md‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ add_action( 'wp_head', function () {
107107

108108
WordPress runs in a nested iframe an agent never sees, so Playground proxies these tools:
109109

110-
```
110+
```text
111111
agent → document.modelContext (Playground page)
112112
→ PlaygroundClient.callWebMCPTool() (Comlink)
113113
→ remote frame (postMessage)
@@ -125,14 +125,27 @@ That catches people out: hooking only `wp_head` means the tools are gone the mom
125125
```php
126126
add_action( 'wp_head', 'my_register_tools' );
127127
add_action( 'admin_head', 'my_register_tools' );
128-
add_action( 'login_head', 'my_register_tools' );
129128
```
130129

131130
Documents WordPress renders no head for — `admin-ajax.php`, REST routes, static files, a PDF — carry no tools, and the previous page's tools are withdrawn rather than left behind to fail when called.
132131

133132
A browser tab shows one site at a time, so the tools always belong to the active site. Switching sites restarts the proxy against the new one, and tool names need no per-site qualifier.
134133

135-
`document.modelContext` is provided by Playground's mu-plugin inside every WordPress document, so a plugin can rely on it being there.
134+
`document.modelContext` is provided by Playground's mu-plugin on `wp_head` and `admin_head`, which covers the front end and wp-admin.
135+
136+
**The login screen is deliberately not covered.** `wp-login.php` fires neither hook, so that document has no registry and proxies no tools; a site's tools come back when the user leaves it. `login_head` still fires there, so a plugin hooking it must feature-detect rather than assume the registry exists — worth doing anywhere, since most browsers do not implement WebMCP:
137+
138+
```php
139+
add_action( 'login_head', function () {
140+
?>
141+
<script>
142+
if ( document.modelContext ) {
143+
document.modelContext.registerTool( { /* … */ } );
144+
}
145+
</script>
146+
<?php
147+
} );
148+
```
136149

137150
### Reading the tools
138151

‎packages/playground/mcp/src/webmcp-polyfill.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
* @see packages/playground/remote/src/lib/playground-mu-plugin/0-playground.php
2222
*/
2323

24+
import { logger } from '@php-wasm/logger';
2425
import type {
2526
ModelContext,
2627
ModelContextClient,
@@ -85,10 +86,18 @@ export function installWebMCPPolyfill(): boolean {
8586
},
8687
};
8788

88-
Object.defineProperty(document, 'modelContext', {
89-
configurable: true,
90-
value: modelContext,
91-
});
89+
try {
90+
Object.defineProperty(document, 'modelContext', {
91+
configurable: true,
92+
value: modelContext,
93+
});
94+
} catch (error) {
95+
// A non-configurable `modelContext` cannot be replaced. Callers wire
96+
// up the MCP bridge right after this, so failing loudly here would
97+
// cost them the parts that do not need WebMCP at all.
98+
logger.warn('Failed to install the WebMCP polyfill:', error);
99+
return false;
100+
}
92101
return true;
93102
}
94103

‎packages/playground/mcp/src/webmcp-site-tools.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,6 @@ export interface WebMCPSiteToolProxy {
4747
stop(): void;
4848
}
4949

50-
/**
51-
* Names Playground registers itself. A site tool may not take one of them
52-
* over, or an agent asking to execute PHP would reach the site's plugin
53-
* instead.
54-
*/
55-
function reservedToolNames(): Set<string> {
56-
return new Set([
57-
...Object.keys(toolDefinitions),
58-
...Object.keys(getSiteToolDefinitions()),
59-
]);
60-
}
61-
6250
/**
6351
* Starts mirroring `client`'s in-site WebMCP tools onto this page.
6452
*
@@ -150,6 +138,18 @@ export function startWebMCPSiteToolProxy(
150138
};
151139
}
152140

141+
/**
142+
* Names Playground registers itself. A site tool may not take one of them
143+
* over, or an agent asking to execute PHP would reach the site's plugin
144+
* instead.
145+
*/
146+
function reservedToolNames(): Set<string> {
147+
return new Set([
148+
...Object.keys(toolDefinitions),
149+
...Object.keys(getSiteToolDefinitions()),
150+
]);
151+
}
152+
153153
function proxyTool(
154154
client: PlaygroundClient,
155155
tool: WebMCPToolDescriptor

‎packages/playground/personal-wp/playwright/e2e/webmcp-proxy.spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,27 @@ test.describe('WebMCP proxy', () => {
168168
.poll(toolNames, { timeout: 30_000, intervals: [500] })
169169
.toContain('site_greeting');
170170

171+
// The login screen is deliberately left out: it fires neither
172+
// `wp_head` nor `admin_head`, so it carries no registry and proxies
173+
// nothing. `reauth=1` reaches the form despite the auto-login.
174+
await page.evaluate(async () => {
175+
await (window as any).__webmcpExecutors['playground_navigate']({
176+
path: '/wp-login.php?reauth=1',
177+
});
178+
});
179+
await expect
180+
.poll(toolNames, { timeout: 30_000, intervals: [500] })
181+
.not.toContain('site_greeting');
182+
183+
await page.evaluate(async () => {
184+
await (window as any).__webmcpExecutors['playground_navigate']({
185+
path: '/',
186+
});
187+
});
188+
await expect
189+
.poll(toolNames, { timeout: 30_000, intervals: [500] })
190+
.toContain('site_greeting');
191+
171192
// Navigating to a page that registers nothing withdraws the tool.
172193
await page.evaluate(async () => {
173194
const executors = (window as any).__webmcpExecutors;

0 commit comments

Comments
 (0)