Conversation
924c26c to
df033f9
Compare
There was a problem hiding this comment.
Pull request overview
This PR centralizes Playground website site-management logic into a single PlaygroundSitesAPI, exposes it as window.playgroundSites, and refactors MCP/WebMCP + UI to call the API instead of duplicating Redux dispatch sequences.
Changes:
- Added
site-management-middleware.tswithcreateSitesAPI()+useSitesAPI()and a DevTools global (window.playgroundSites). - Refactored MCP bridge/WebMCP + multiple UI components to use the centralized API methods.
- Renamed MCP tools (
playground_open_site*,playground_save_*) and updated docs/tests accordingly; added NotAllowedError handling during boot.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/playground/website/src/lib/state/redux/store.ts | Registers the new site management middleware and renamed MCP bridge middleware in the Redux chain. |
| packages/playground/website/src/lib/state/redux/site-management-middleware.ts | Implements the centralized Sites API, exposes it on window, and provides a React hook. |
| packages/playground/website/src/lib/state/redux/init-mcp-bridge.ts | Simplifies MCP bridge setup to delegate site operations to createSitesAPI(). |
| packages/playground/website/src/lib/state/redux/boot-site-client.ts | Adds explicit handling for directory-handle permission denial (NotAllowedError). |
| packages/playground/website/src/components/site-manager/site-settings-form/unconnected-site-settings-form.tsx | Adjusts react-hook-form defaults to re-render when active site changes. |
| packages/playground/website/src/components/site-manager/site-settings-form/temporary-site-settings-form.tsx | Switches “update temporary site settings” flow to use Sites API new temp-site creation. |
| packages/playground/website/src/components/site-manager/site-settings-form/stored-site-settings-form.tsx | Uses Sites API to update persisted site runtime configuration. |
| packages/playground/website/src/components/site-manager/site-info-panel/index.tsx | Uses Sites API for delete instead of direct Redux action. |
| packages/playground/website/src/components/site-error-modal/site-error-modal.tsx | Uses Sites API for delete in error recovery flow. |
| packages/playground/website/src/components/saved-playgrounds-overlay/index.tsx | Uses Sites API for activation and deletion of sites. |
| packages/playground/website/src/components/save-site-modal/index.tsx | Uses Sites API for save-to-OPFS / save-to-local-fs workflows. |
| packages/playground/website/src/components/rename-site-modal/index.tsx | Uses Sites API for rename workflow. |
| packages/playground/website/src/components/ensure-playground-site/ensure-playground-site-is-selected.tsx | Uses Sites API for creating/selecting sites instead of local helper + Redux actions. |
| packages/playground/website/playwright/e2e/sites-api.spec.ts | Adds e2e coverage for window.playgroundSites exposure and core methods. |
| packages/playground/mcp/tests/e2e/webmcp.spec.ts | Updates WebMCP e2e tests for renamed save tool + rename precondition. |
| packages/playground/mcp/tests/e2e/mcp-tools.spec.ts | Updates MCP tools e2e tests for renamed tools and workflows. |
| packages/playground/mcp/src/webmcp.ts | Refactors WebMCP tool registration to use new bridge config + renamed helpers/tool ids. |
| packages/playground/mcp/src/tools/tool-definitions.ts | Renames tools + storage-label formatter and updates user-facing descriptions. |
| packages/playground/mcp/src/tools/register-mcp-server-tools.ts | Updates server tool registration names and the open-site command id. |
| packages/playground/mcp/src/mcp-server.ts | Updates the server instructions text to match renamed tools. |
| packages/playground/mcp/src/client.ts | Updates exported types to the new PlaygroundBridgeConfig. |
| packages/playground/mcp/src/bridge-server.ts | Updates storage label formatter usage + clarifies open-site command naming. |
| packages/playground/mcp/src/bridge-client.ts | Renames config interface and switches to new tool ids; routes through the new config methods. |
| packages/playground/mcp/README.md | Updates documentation to reference renamed site-management tools. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
packages/playground/website/src/lib/state/redux/site-management-api-middleware.ts
Show resolved
Hide resolved
...yground/website/src/components/ensure-playground-site/ensure-playground-site-is-selected.tsx
Outdated
Show resolved
Hide resolved
packages/playground/website/src/lib/state/redux/site-management-middleware.ts
Outdated
Show resolved
Hide resolved
packages/playground/website/src/lib/state/redux/site-management-api-middleware.ts
Show resolved
Hide resolved
packages/playground/website/src/components/site-error-modal/site-error-modal.tsx
Outdated
Show resolved
Hide resolved
packages/playground/website/src/components/saved-playgrounds-overlay/index.tsx
Show resolved
Hide resolved
brandonpayton
left a comment
There was a problem hiding this comment.
@bgrgicak thanks for working on this! I'm marking this PR as approved, but there are a couple of items that would be good to address first. Once you think those are in good shape, please feel free to merge.
| logger.error('Failed to delete site', error); | ||
| } finally { | ||
| dispatch(removeClientInfo(siteSlug)); | ||
| dispatch(clearActiveSiteError()); |
There was a problem hiding this comment.
Do we want to remove the client info and clear the error if deleting the site failed?
There was a problem hiding this comment.
No, we should only do it if successful. I made the change.
| } = useForm<SiteFormData>({ | ||
| defaultValues, | ||
| defaultValues: mergedDefaults, | ||
| values: mergedDefaults, |
There was a problem hiding this comment.
It seems strange to override the way values are treated in this form as part of this site management API PR. Maybe we should skip this change or move it to another PR.
There was a problem hiding this comment.
I would like to keep the change in this PR because it ensures the UI is updated when the site API makes changes. Without this change, if you have the site manager open and run setPhpVersion or setNetworking, the value will update and reload Playground, but the UI won't update.
My goal for the PlaygroundSitesAPI is to be the central place for managing sites, and no matter if the change is coming from the UI, MCP, or a function call, it should have the same result.
packages/playground/website/src/lib/state/redux/site-management-api-middleware.ts
Show resolved
Hide resolved
- Add site-management-api-middleware with sites API methods - Refactor MCP bridge to use PlaygroundBridgeConfig interface - Add WebMCP site management tools (list, save, rename) - Add sites-api e2e tests - Expose window.playgroundSites for external integrations
7c730a2 to
6312ada
Compare
| import { selectClientBySiteSlug } from './slice-clients'; | ||
| import { setOPFSSitesLoadingState } from './slice-sites'; | ||
| import { createSitesAPI } from './site-management-api-middleware'; | ||
| import type { McpBridgeHandle } from '@wp-playground/mcp/client'; |
There was a problem hiding this comment.
Are we loading @modelcontextprotocol/sdk in the Playground website now? What's the bundle size impact? That sdk is ~4MB on npm, or 2MB for the ESM version (with source maps). Are we pruning that to something much smaller?
There was a problem hiding this comment.
No, @modelcontextprotocol/sdk runs in the MCP server on the host machine, it doesn't run on the Playground website.
| async rename(newName: string) { | ||
| const site = getActiveSiteOrThrow(); | ||
| if (site.metadata.storage === 'none') { | ||
| throw new Error( | ||
| 'Cannot rename a temporary site. Save it first.' | ||
| ); | ||
| } | ||
| await dispatch( | ||
| updateSiteMetadata({ | ||
| slug: site.slug, | ||
| changes: { name: newName }, | ||
| }) | ||
| ); | ||
| }, |
There was a problem hiding this comment.
It's interesting that we're adding a new rename() function without deleting any prior rename() function. Is this logic now duplicated? Or was this the only way to rename before?
updateSiteMetadata({
slug: site.slug,
changes: { name: trimmed },
}) as any
There was a problem hiding this comment.
This was the only way to rename before. We didn't have a rename function before this PR.
| }, | ||
|
|
||
| getClient() { | ||
| const site = getActiveSiteOrThrow(); |
There was a problem hiding this comment.
We have getActiveSiteOrThrow, but then we have the full explicit construction for non-active sites:
const site = selectSiteBySlug(getState(), siteSlug);
if (!site) {
throw new Error(`Site not found: ${siteSlug}`);
}Let's stick to this full explicit style, it's one less click when reading the code.
There was a problem hiding this comment.
| const storage = updatedSite?.metadata.storage ?? 'none'; | ||
| if (storage === 'none') { | ||
| throw new Error( | ||
| 'Failed to save the site — the storage is still temporary after persist.' |
There was a problem hiding this comment.
I don't understand this error message. We've failed to save the site, but we're "after persist", sooo is the site persisted now?
If it's internal and only for logs, then let's be very specific about what went wrong so the developer can read it without context and have some idea. If it's for the user, then let's remove all technical details and tell them what next step would help them do what they wanted to do. Ditto for all error strings in this PR.
There was a problem hiding this comment.
The site management API is used by the UI, MCP, and as a window variable by MCPs like Playwright and devtools. The MCPs use the error messages to understand why function/tool calls failed.
The UI didn't display these errors at all (except for some save errors), so I added support for displaying errors in the UI in #3454.
The PR also updates the error messages so they are understandable for humans and agents.
| } | ||
| if (site.metadata.storage === 'none') { | ||
| throw new Error( | ||
| 'Cannot delete a temporary site. It will be removed automatically when you close the tab.' |
There was a problem hiding this comment.
Where are all these strings displayed? Let's document that. Will it echo "when you close the tab" in the terminal for the MCP users? That could be confusing, they're not looking at the browser.
There was a problem hiding this comment.
I added @throws JSDoc comments to explain in which cases errors are thrown. Does that cover your concern or did you have something else in mind?
| return { slug: site.slug, storage }; | ||
| }, | ||
|
|
||
| async saveToLocalFileSystem( |
There was a problem hiding this comment.
We need documentation. What purpose does each of these functions fulfill? Do they cause a full page reload? Redirect to a different URL? Change any persisted metadata? It's unclear at the moment.
There was a problem hiding this comment.
I added JSDoc comments to all functions in https://github.com/WordPress/wordpress-playground/pull/3454/changes#diff-0712f62c2a861481e9e0aa7bf56de2fc0a894aa7410bbf91f95635858f14a22bL40
| const updatedSite = selectSiteBySlug(getState(), site.slug); | ||
| const storage = updatedSite?.metadata.storage ?? 'none'; | ||
| if (storage === 'none') { | ||
| throw new Error( |
There was a problem hiding this comment.
When would that happen? It seems like persistTemporarySite() should throw in these scenarios, otherwise it gives the consumer a wrong impression that the operation succeeded when it actually failed.
There was a problem hiding this comment.
This shouldn't be possible. I updated persistTemporarySite to throw errors and ensured the UI catches. https://github.com/WordPress/wordpress-playground/pull/3454/changes#diff-77504650ae8afcd3b899ef2e25114c3cd6ef80fc032fba479de6dfc9d9cc38ddR137
|
@adamziel I'm working on addressing your feedback in a new PR. |

Motivation for the change, related issues
This PR extracts a centralized site management API (
PlaygroundSitesAPI) from scattered Redux dispatch calls across the Playground website, and exposes it aswindow.playgroundSitesfor programmatic access.Site management logic (listing, renaming, saving, deleting, switching sites) was duplicated across many UI components, the MCP bridge, and WebMCP — each reimplementing the same Redux dispatch sequences. This made it hard to add new consumers (like browser DevTools or external tooling) and easy to introduce inconsistencies.
Implementation details
New
site-management-middleware.tsA single
createSitesAPI(getState, dispatch)factory that produces aPlaygroundSitesAPIobject with explicit methods:list()— all sites with active statusgetClient()— PlaygroundClient for the active siterename(newName)— rename the active site (rejects temporary sites)saveInBrowser(name?)— persist to OPFSsaveToLocalFileSystem(name?, handle?)— persist to local filesystemsetPhpVersion(version)/setNetworking(enabled)— update runtime configdelete(siteSlug)— remove a saved sitesetActiveSite(siteSlug)— switch site and wait for boot to completecreateNewTemporarySite(slug?, settings?)— create and activate a temporary siteA
useSitesAPI()React hook gives components access.window.playgroundSitesglobalExposed once the site finishes loading. Enables browser DevTools and AI agents to manage sites programmatically.
MCP bridge simplification
PlaygroundConfig→PlaygroundBridgeConfigwith a narrower interface thatPlaygroundSitesAPIsatisfies structurallyinit-mcp-bridge.tsno longer reimplements site management — just passessitesAPImethods throughif (config.saveSite),if (config.renameSite)) removed — all methods are always availableTool renames for clarity for consistency
playground_open_site→playground_open_site_in_new_tabplayground_save_site→playground_save_in_browserpresentStorage()→formatStorageLabel()UI component cleanup
Components (
RenameSiteModal,SaveSiteModal,SiteInfoPanel,SavedPlaygroundsOverlay, etc.) now callsitesAPI.rename(),sitesAPI.delete(), etc. instead of dispatching Redux actions directly.Other fixes
boot-site-client.ts: added handling forNotAllowedError(directory handle permission denied)unconnected-site-settings-form.tsx: wrapped default values inuseMemoand passedvaluesprop touseFormso the form re-renders when the active site changesTesting Instructions
Manual testing
window.playgroundSitesmethods likewindow.playgroundSites.list()