From 08d18e764dd5225350542a4edc540f8ccbbfd5d1 Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 20 May 2026 12:39:57 -0400 Subject: [PATCH 1/9] make ability namespace filter configurable replaces hardcoded mainwp/ filter with an abilityNamespaces allowlist (default ['mainwp']); non-primary namespaces get a {ns}__ tool prefix. reverse tool->ability lookup now goes through a cache index. --- .gitignore | 3 + README.md | 5 +- docs/configuration.md | 26 ++++ package.json | 2 +- settings.example.json | 1 + settings.schema.json | 11 ++ src/abilities.test.ts | 192 ++++++++++++++++++++++-- src/abilities.ts | 151 +++++++++++++++++-- src/config.test.ts | 80 ++++++++++ src/config.ts | 72 ++++++++- src/help.ts | 8 +- src/index.ts | 12 +- src/naming.test.ts | 109 +++++++------- src/naming.ts | 58 ++++--- src/tool-schema.ts | 12 +- src/tools.test.ts | 10 +- src/tools.ts | 21 +-- tests/evals/description-quality.test.ts | 1 + tests/evals/safety-coverage.test.ts | 1 + tests/evals/schema-quality.test.ts | 1 + tests/evals/token-budget.test.ts | 1 + tests/integration/abilities.test.ts | 1 + tests/integration/tools.test.ts | 1 + 23 files changed, 635 insertions(+), 144 deletions(-) diff --git a/.gitignore b/.gitignore index de643a4..90e8e5d 100644 --- a/.gitignore +++ b/.gitignore @@ -25,5 +25,8 @@ coverage/ # Manual test results (generated) test-results/ +# Local agent/review artifacts (generated) +.context/ + # Override global gitignore for CI/CD !.github/ diff --git a/README.md b/README.md index 5df16fb..7deb8e2 100644 --- a/README.md +++ b/README.md @@ -421,6 +421,7 @@ For Windsurf and other hosts, use the same JSON configuration pattern shown abov | `MAINWP_MAX_RETRIES` | No | `2` | Total retry attempts including initial request | | `MAINWP_RETRY_BASE_DELAY` | No | `1000` | Base delay between retries in milliseconds | | `MAINWP_RETRY_MAX_DELAY` | No | `2000` | Maximum delay between retries in milliseconds | +| `MAINWP_ABILITY_NAMESPACES` | No | `mainwp` | Comma-separated ability namespace allowlist | > **⚠️ Security Warning: SSL Verification** > @@ -476,7 +477,7 @@ Compact mode truncates descriptions to 60 characters and removes examples while ### Limiting Exposed Tools -You can expose only the tools you need. These configurations cover common scenarios: +You can expose only the tools you need. These configurations cover common scenarios. Tool names from non-primary namespaces (added via `abilityNamespaces`) use the `{namespace}__{tool}` form — e.g. `acme__do_thing_v1` — so reference them that way in `allowedTools` / `blockedTools`. **Read-only monitoring** (17 tools, ~73% reduction): @@ -669,7 +670,7 @@ See the [Security Guide](docs/security.md#confirmation-guardrails) for more deta Over 60 tools organized by category. Each tool shows parameters with type, requirement, and description. -> **Note:** Tool names omit the `mainwp` namespace. The ability `mainwp/list-sites-v1` becomes `list_sites_v1`. +> **Note:** Tool names omit the primary namespace (default `mainwp`), so `mainwp/list-sites-v1` becomes `list_sites_v1`. If you add other namespaces via `abilityNamespaces`, abilities in those namespaces are exposed as `{namespace}__{tool}` (e.g. `acme/do-thing-v1` → `acme__do_thing_v1`).
Sites diff --git a/docs/configuration.md b/docs/configuration.md index 7d7e1b3..4a24942 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -26,6 +26,7 @@ The server accepts configuration through environment variables and a configurati | `MAINWP_MAX_RETRIES` | No | `2` | Total retry attempts including initial request | | `MAINWP_RETRY_BASE_DELAY` | No | `1000` | Base delay between retries in milliseconds | | `MAINWP_RETRY_MAX_DELAY` | No | `2000` | Maximum delay between retries in milliseconds | +| `MAINWP_ABILITY_NAMESPACES` | No | `mainwp` | Comma-separated namespace allowlist (see below) | ## Configuration File @@ -69,9 +70,34 @@ Create a `settings.json` file in one of these locations (checked in order): | `maxRetries` | `MAINWP_MAX_RETRIES` | number | | `retryBaseDelay` | `MAINWP_RETRY_BASE_DELAY` | number | | `retryMaxDelay` | `MAINWP_RETRY_MAX_DELAY` | number | +| `abilityNamespaces` | `MAINWP_ABILITY_NAMESPACES` | string[] | A JSON schema is available at `settings.schema.json` for IDE autocompletion. +### Ability Namespaces + +By default the server only surfaces abilities in the `mainwp/` namespace. Set `abilityNamespaces` to expose abilities from third-party MainWP extensions that register their own abilities (via the WordPress Abilities API). The first entry in the list is the **primary** namespace — abilities in it get unprefixed tool names. Other namespaces get a `{namespace}__{tool}` prefix so MCP tool names stay collision-free. + +```json +{ + "abilityNamespaces": ["mainwp", "acme"] +} +``` + +```bash +MAINWP_ABILITY_NAMESPACES="mainwp,acme" +``` + +Examples with `abilityNamespaces: ["mainwp", "acme"]`: + +| Ability name | MCP tool name | +| ----------------------- | ------------------------ | +| `mainwp/list-sites-v1` | `list_sites_v1` | +| `acme/do-thing-v1` | `acme__do_thing_v1` | +| `acme-corp/do-thing-v1` | `acme_corp__do_thing_v1` | + +When combining with `allowedTools` / `blockedTools`, use the prefixed form for non-primary namespaces (e.g. `"allowedTools": ["list_sites_v1", "acme__do_thing_v1"]`). + --- ## Tool Filtering diff --git a/package.json b/package.json index 653e212..b883516 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mainwp/mcp", - "version": "1.0.0-beta.2", + "version": "1.0.0-beta.3", "description": "MCP Server for MainWP Dashboard - Exposes MainWP Abilities API as MCP tools", "type": "module", "main": "dist/index.js", diff --git a/settings.example.json b/settings.example.json index 1339a9e..154c101 100644 --- a/settings.example.json +++ b/settings.example.json @@ -22,6 +22,7 @@ "blockedTools": ["delete_site_v1"], "schemaVerbosity": "standard", "responseFormat": "compact", + "abilityNamespaces": ["mainwp"], "retryEnabled": true, "maxRetries": 2, diff --git a/settings.schema.json b/settings.schema.json index c9db1a6..4fa4cf3 100644 --- a/settings.schema.json +++ b/settings.schema.json @@ -99,6 +99,17 @@ "default": "compact", "description": "Response JSON format. 'compact' omits whitespace to reduce token usage. 'pretty' uses 2-space indentation for human-readable output. Default is 'compact'." }, + "abilityNamespaces": { + "type": "array", + "items": { + "type": "string", + "pattern": "^[a-z0-9-]+$" + }, + "minItems": 1, + "uniqueItems": true, + "default": ["mainwp"], + "description": "Allowed ability namespaces. Abilities whose name's namespace prefix is in this list are surfaced as MCP tools. The first entry is the primary namespace — its abilities get unprefixed tool names (e.g. mainwp/list-sites-v1 -> list_sites_v1). Abilities in other namespaces get a {ns}__ prefix (e.g. acme/do-thing-v1 -> acme__do_thing_v1; hyphens in the namespace are converted to underscores so the tool name stays within [a-z0-9_]). Category slugs are matched by `{namespace}-` prefix, so an extension's categories are included when its namespace is listed here. Defaults to [\"mainwp\"]; add third-party MainWP extension namespaces here to expose their abilities." + }, "retryEnabled": { "type": "boolean", "default": true, diff --git a/src/abilities.test.ts b/src/abilities.test.ts index ae67c55..fdbda4b 100644 --- a/src/abilities.test.ts +++ b/src/abilities.test.ts @@ -7,6 +7,7 @@ import { fetchAbilities, fetchCategories, getAbility, + getAbilityByToolName, executeAbility, clearCache, onCacheRefresh, @@ -209,6 +210,7 @@ const baseConfig: Config = { maxRetries: 2, retryBaseDelay: 1000, retryMaxDelay: 2000, + abilityNamespaces: ['mainwp'], configSource: 'environment', }; @@ -266,7 +268,7 @@ describe('fetchAbilities', () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); - it('should filter by namespace', async () => { + it('should filter by namespace (default: mainwp only)', async () => { const mixedAbilities = [ ...sampleAbilities, { name: 'other/some-ability', label: 'Other', description: 'Other', category: 'other' }, @@ -284,6 +286,73 @@ describe('fetchAbilities', () => { expect(abilities.every(a => a.name.startsWith('mainwp/'))).toBe(true); }); + it('keeps abilities from any configured namespace', async () => { + const mixedAbilities = [ + ...sampleAbilities, + { name: 'acme/do-thing-v1', label: 'Acme Do', description: 'Acme', category: 'acme-misc' }, + { name: 'other/skip-me-v1', label: 'Other', description: 'Skip', category: 'other-misc' }, + ]; + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => mixedAbilities, + headers: new Headers(), + }); + + const multiNsConfig: Config = { ...baseConfig, abilityNamespaces: ['mainwp', 'acme'] }; + const abilities = await fetchAbilities(multiNsConfig); + + expect(abilities.map(a => a.name)).toContain('acme/do-thing-v1'); + expect(abilities.map(a => a.name)).not.toContain('other/skip-me-v1'); + }); + + it('refreshes cache when abilityNamespaces changes between calls', async () => { + mockFetch.mockResolvedValue({ + ok: true, + json: async () => [ + ...sampleAbilities, + { name: 'acme/do-thing-v1', label: 'Acme', description: 'Acme', category: 'acme-misc' }, + ], + headers: new Headers(), + }); + + await fetchAbilities(baseConfig); + expect(mockFetch).toHaveBeenCalledTimes(1); + + // Different namespace allowlist must invalidate the cache despite fresh TTL. + await fetchAbilities({ ...baseConfig, abilityNamespaces: ['mainwp', 'acme'] }); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('discards cache and re-throws when signature mismatches and refresh fails', async () => { + // Populate cache for ['mainwp'] successfully. + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + await fetchAbilities(baseConfig); + expect(mockFetch).toHaveBeenCalledTimes(1); + + // Now request ['mainwp','acme'] but the refresh fails. The catch block must + // NOT serve the cache built for the wrong namespace; it must surface the error. + mockFetch.mockRejectedValueOnce(new Error('Network blip')); + await expect( + fetchAbilities({ ...baseConfig, abilityNamespaces: ['mainwp', 'acme'] }) + ).rejects.toThrow(/Network blip/); + + // The downstream toolNameIndex was nulled along with cachedAbilities, so a + // tool-name lookup must trigger a fresh fetch rather than returning stale data. + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + const ability = await getAbilityByToolName(baseConfig, 'list_sites_v1'); + expect(ability?.name).toBe('mainwp/list-sites-v1'); + expect(mockFetch).toHaveBeenCalledTimes(3); + }); + it('should handle fetch errors with cached fallback', async () => { // First successful fetch mockFetch.mockResolvedValueOnce({ @@ -440,6 +509,25 @@ describe('fetchCategories', () => { expect(categories[0].slug).toBe('mainwp-sites'); }); + it('includes categories from any configured namespace', async () => { + const mixedCategories = [ + ...sampleCategories, + { slug: 'acme-things', label: 'Acme', description: 'Acme stuff' }, + { slug: 'other-skip', label: 'Other', description: 'Should be skipped' }, + ]; + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => mixedCategories, + headers: new Headers(), + }); + + const config: Config = { ...baseConfig, abilityNamespaces: ['mainwp', 'acme'] }; + const categories = await fetchCategories(config); + + expect(categories.map(c => c.slug).sort()).toEqual(['acme-things', 'mainwp-sites']); + }); + it('should paginate when X-WP-TotalPages > 1', async () => { vi.resetAllMocks(); @@ -543,6 +631,62 @@ describe('getAbility', () => { }); }); +describe('getAbilityByToolName', () => { + beforeEach(() => { + vi.resetAllMocks(); + clearCache(); + initRateLimiter(0); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('resolves primary-namespace tool name (unprefixed)', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + + const ability = await getAbilityByToolName(baseConfig, 'list_sites_v1'); + expect(ability?.name).toBe('mainwp/list-sites-v1'); + }); + + it('resolves non-primary namespace tool name ({ns}__ prefix)', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => [ + ...sampleAbilities, + { + name: 'acme/do-thing-v1', + label: 'Acme Do', + description: 'Acme', + category: 'acme-misc', + meta: { annotations: { readonly: true, destructive: false, idempotent: true } }, + }, + ], + headers: new Headers(), + }); + + const config: Config = { ...baseConfig, abilityNamespaces: ['mainwp', 'acme'] }; + const ability = await getAbilityByToolName(config, 'acme__do_thing_v1'); + expect(ability?.name).toBe('acme/do-thing-v1'); + }); + + it('returns undefined for unknown tool name', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + + const ability = await getAbilityByToolName(baseConfig, 'totally_unknown_tool'); + expect(ability).toBeUndefined(); + }); +}); + describe('executeAbility', () => { beforeEach(() => { vi.resetAllMocks(); @@ -872,6 +1016,34 @@ describe('clearCache', () => { await getAbility(baseConfig, 'mainwp/list-sites-v1'); expect(mockFetch).toHaveBeenCalledTimes(2); }); + + it('should clear the toolName index', async () => { + // Warm cache and toolNameIndex + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + + const beforeClear = await getAbilityByToolName(baseConfig, 'list_sites_v1'); + expect(beforeClear?.name).toBe('mainwp/list-sites-v1'); + expect(mockFetch).toHaveBeenCalledTimes(1); + + // Clear cache (which must also clear toolNameIndex) + clearCache(); + + // Next getAbilityByToolName should trigger a new fetch — if clearCache forgot + // toolNameIndex, the lookup would silently hit the stale Map and skip the fetch. + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + + const afterClear = await getAbilityByToolName(baseConfig, 'list_sites_v1'); + expect(afterClear?.name).toBe('mainwp/list-sites-v1'); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); }); describe('onCacheRefresh', () => { @@ -907,7 +1079,7 @@ describe('onCacheRefresh', () => { describe('generateToolHelp', () => { it('should generate help for a simple ability', () => { const ability = sampleAbilities[0]; - const help = generateToolHelp(ability); + const help = generateToolHelp(ability, 'mainwp'); expect(help.toolName).toBe('list_sites_v1'); expect(help.abilityName).toBe('mainwp/list-sites-v1'); @@ -918,7 +1090,7 @@ describe('generateToolHelp', () => { it('should detect safety features', () => { const ability = sampleAbilities[1]; - const help = generateToolHelp(ability); + const help = generateToolHelp(ability, 'mainwp'); expect(help.safetyFeatures.supportsDryRun).toBe(true); expect(help.safetyFeatures.requiresConfirm).toBe(true); @@ -927,7 +1099,7 @@ describe('generateToolHelp', () => { it('should include parameters', () => { const ability = sampleAbilities[1]; - const help = generateToolHelp(ability); + const help = generateToolHelp(ability, 'mainwp'); expect(help.parameters).toContainEqual( expect.objectContaining({ name: 'site_id', required: true }) @@ -940,7 +1112,7 @@ describe('generateToolHelp', () => { describe('generateHelpDocument', () => { it('should generate complete help document', () => { - const helpDoc = generateHelpDocument(sampleAbilities); + const helpDoc = generateHelpDocument(sampleAbilities, 'mainwp'); expect(helpDoc.version).toBe('1.0'); expect(helpDoc.overview.totalTools).toBe(7); @@ -948,33 +1120,33 @@ describe('generateHelpDocument', () => { }); it('should list destructive tools', () => { - const helpDoc = generateHelpDocument(sampleAbilities); + const helpDoc = generateHelpDocument(sampleAbilities, 'mainwp'); expect(helpDoc.destructiveTools).toContain('delete_site_v1'); expect(helpDoc.destructiveTools).not.toContain('list_sites_v1'); }); it('should list tools with dry_run', () => { - const helpDoc = generateHelpDocument(sampleAbilities); + const helpDoc = generateHelpDocument(sampleAbilities, 'mainwp'); expect(helpDoc.toolsWithDryRun).toContain('delete_site_v1'); }); it('should list tools requiring confirm', () => { - const helpDoc = generateHelpDocument(sampleAbilities); + const helpDoc = generateHelpDocument(sampleAbilities, 'mainwp'); expect(helpDoc.toolsRequiringConfirm).toContain('delete_site_v1'); }); it('should group tools by category', () => { - const helpDoc = generateHelpDocument(sampleAbilities); + const helpDoc = generateHelpDocument(sampleAbilities, 'mainwp'); // mainwp-sites has: list-sites, delete-site, delete-site-plugins, delete-site-themes, update-site expect(helpDoc.toolsByCategory['mainwp-sites']).toHaveLength(5); }); it('should include safety conventions', () => { - const helpDoc = generateHelpDocument(sampleAbilities); + const helpDoc = generateHelpDocument(sampleAbilities, 'mainwp'); expect(helpDoc.overview.safetyConventions).toHaveProperty('dryRun'); expect(helpDoc.overview.safetyConventions).toHaveProperty('confirm'); diff --git a/src/abilities.ts b/src/abilities.ts index 2edc8fc..05d792d 100644 --- a/src/abilities.ts +++ b/src/abilities.ts @@ -17,6 +17,7 @@ import { MAX_URL_LENGTH, } from './http-client.js'; import type { Logger } from './logging.js'; +import { abilityNameToToolName } from './naming.js'; /** Maximum age of stale cache before hard-failing (30 minutes) */ const MAX_STALE_AGE_MS = 30 * 60 * 1000; @@ -77,17 +78,40 @@ export interface Category { */ let cachedAbilities: Ability[] | null = null; let abilitiesIndex: Map | null = null; +let toolNameIndex: Map | null = null; let cachedCategories: Category[] | null = null; let abilitiesCacheTimestamp: number = 0; let categoriesCacheTimestamp: number = 0; +let abilitiesNamespaceSignature: string = ''; +let categoriesNamespaceSignature: string = ''; const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes /** - * Hardcoded namespace filter for MainWP abilities. - * This server only supports MainWP abilities (mainwp/* namespace). + * Build a stable signature of the configured namespaces so a config change + * (e.g. adding a third-party namespace) forces a cache refresh instead of + * serving stale, namespace-filtered data. */ -const NAMESPACE_FILTER = 'mainwp/'; -const CATEGORY_FILTER = 'mainwp-'; +function namespaceSignature(namespaces: string[]): string { + return namespaces.join('|'); +} + +/** + * Returns true if the ability's namespace is in the allowlist. + */ +function isAllowedNamespace(abilityName: string, namespaces: string[]): boolean { + const slashIndex = abilityName.indexOf('/'); + if (slashIndex === -1) return false; + const namespace = abilityName.slice(0, slashIndex); + return namespaces.includes(namespace); +} + +/** + * Returns true if the category slug starts with any configured namespace's + * `{ns}-` prefix. + */ +function isAllowedCategory(slug: string, namespaces: string[]): boolean { + return namespaces.some(ns => slug.startsWith(`${ns}-`)); +} /** * Cache refresh callbacks @@ -123,8 +147,17 @@ export async function fetchAbilities( forceRefresh = false, logger?: Logger ): Promise { - // Return cached data if still valid - if (!forceRefresh && cachedAbilities && Date.now() - abilitiesCacheTimestamp < CACHE_TTL_MS) { + const namespaces = config.abilityNamespaces; + const signature = namespaceSignature(namespaces); + + // Return cached data only if still fresh AND the namespace allowlist hasn't + // changed since the cache was populated. + if ( + !forceRefresh && + cachedAbilities && + abilitiesNamespaceSignature === signature && + Date.now() - abilitiesCacheTimestamp < CACHE_TTL_MS + ) { return cachedAbilities; } @@ -140,7 +173,7 @@ export async function fetchAbilities( logger ); - const newAbilities = allAbilities.filter(a => a.name.startsWith(NAMESPACE_FILTER)); + const newAbilities = allAbilities.filter(a => isAllowedNamespace(a.name, namespaces)); // Check if abilities have changed (compare names) const oldNames = @@ -156,10 +189,28 @@ export async function fetchAbilities( cachedAbilities = newAbilities; abilitiesIndex = new Map(); + toolNameIndex = new Map(); + const primary = namespaces[0]; for (const ability of newAbilities) { abilitiesIndex.set(ability.name, ability); + + const toolName = abilityNameToToolName(ability.name, primary); + // Tool name collisions are structurally impossible: ABILITY_NAME_RE + // forbids `_` in ability slugs, `__` is used only as the namespace/slug + // separator for non-primary namespaces, and ability names are unique + // upstream. Fail loud if the invariant ever breaks — a silent override + // would shadow a real ability under the wrong name. + const existing = toolNameIndex.get(toolName); + if (existing) { + throw new Error( + `Tool name collision: "${toolName}" produced by both "${existing.name}" and "${ability.name}". ` + + `This indicates a violation of the namespace/slug invariants in abilities.ts.` + ); + } + toolNameIndex.set(toolName, ability); } abilitiesCacheTimestamp = Date.now(); + abilitiesNamespaceSignature = signature; // Notify callbacks if abilities changed if (hasChanged && oldNames !== '') { @@ -168,9 +219,16 @@ export async function fetchAbilities( return cachedAbilities; } catch (error) { - // Serve stale cache on transient failures, but only up to MAX_STALE_AGE_MS. - // Beyond that, the cache may be dangerously outdated (e.g., poisoned abilities). - if (cachedAbilities) { + // Snapshot the cache's current signature so a concurrent fetch that + // succeeds and writes new cache state can't be silently discarded by + // this catch block when we re-check below. + const cachedSignature = abilitiesNamespaceSignature; + + // Serve stale cache on transient failures, but only up to MAX_STALE_AGE_MS, + // and only when the cached data was built for the same namespace allowlist. + // A signature mismatch means the user changed config — falling back to the + // old cache would silently return the wrong set of abilities. + if (cachedAbilities && cachedSignature === signature) { const cacheAgeMs = Date.now() - abilitiesCacheTimestamp; const cacheAgeMinutes = Math.round(cacheAgeMs / 60000); if (cacheAgeMs > MAX_STALE_AGE_MS) { @@ -180,6 +238,7 @@ export async function fetchAbilities( }); cachedAbilities = null; abilitiesIndex = null; + toolNameIndex = null; throw error; } logger?.warning('Failed to refresh abilities, using cached data', { @@ -188,6 +247,23 @@ export async function fetchAbilities( }); return cachedAbilities; } + + // Cache is missing or its signature no longer matches the requested + // signature. If the signature has CHANGED since we entered the catch (a + // concurrent fetch succeeded with a different config), leave that cache + // intact — only null when the signature still matches the stale data we + // were about to serve. Either way, surface the error: we can't return + // wrong-namespace data, and we won't lie about a fetch that failed. + if (cachedAbilities && abilitiesNamespaceSignature === cachedSignature) { + logger?.warning('Discarding cache: namespace allowlist changed and refresh failed', { + cachedSignature, + requestedSignature: signature, + error: sanitizeError(String(error)), + }); + cachedAbilities = null; + abilitiesIndex = null; + toolNameIndex = null; + } throw error; } } @@ -200,8 +276,19 @@ export async function fetchCategories( forceRefresh = false, logger?: Logger ): Promise { - // Return cached data if still valid - if (!forceRefresh && cachedCategories && Date.now() - categoriesCacheTimestamp < CACHE_TTL_MS) { + const namespaces = config.abilityNamespaces; + const signature = namespaceSignature(namespaces); + + // Return cached data only if still fresh AND the namespace allowlist hasn't + // changed since the cache was populated. Without this, a config change + // would leave the category list out of sync with the ability list for up + // to one TTL window. + if ( + !forceRefresh && + cachedCategories && + categoriesNamespaceSignature === signature && + Date.now() - categoriesCacheTimestamp < CACHE_TTL_MS + ) { return cachedCategories; } @@ -217,13 +304,16 @@ export async function fetchCategories( logger ); - // Filter categories to only MainWP namespace - cachedCategories = allCategories.filter(c => c.slug.startsWith(CATEGORY_FILTER)); + // Filter categories to configured namespaces (allowlist of `{ns}-` prefixes) + cachedCategories = allCategories.filter(c => isAllowedCategory(c.slug, namespaces)); categoriesCacheTimestamp = Date.now(); + categoriesNamespaceSignature = signature; return cachedCategories; } catch (error) { - if (cachedCategories) { + const cachedSignature = categoriesNamespaceSignature; + + if (cachedCategories && cachedSignature === signature) { const cacheAgeMs = Date.now() - categoriesCacheTimestamp; const cacheAgeMinutes = Math.round(cacheAgeMs / 60000); if (cacheAgeMs > MAX_STALE_AGE_MS) { @@ -240,6 +330,19 @@ export async function fetchCategories( }); return cachedCategories; } + + // Signature mismatch or no cache — race-safe discard (see fetchAbilities). + if (cachedCategories && categoriesNamespaceSignature === cachedSignature) { + logger?.warning( + 'Discarding categories cache: namespace allowlist changed and refresh failed', + { + cachedSignature, + requestedSignature: signature, + error: sanitizeError(String(error)), + } + ); + cachedCategories = null; + } throw error; } } @@ -256,6 +359,21 @@ export async function getAbility( return abilitiesIndex?.get(name); } +/** + * Resolve an MCP tool name to its underlying ability via the cache index. + * Tool names are not uniquely decodable back to ability names (multi-namespace + * collisions get numeric suffixes), so reverse lookup must go through the map + * built during `fetchAbilities`. + */ +export async function getAbilityByToolName( + config: Config, + toolName: string, + logger?: Logger +): Promise { + await fetchAbilities(config, false, logger); + return toolNameIndex?.get(toolName); +} + /** * Serialize input to PHP-style query string for GET requests. * WordPress REST API parses PHP array notation: input[key][]=value @@ -453,7 +571,10 @@ export async function executeAbility( export function clearCache(): void { cachedAbilities = null; abilitiesIndex = null; + toolNameIndex = null; cachedCategories = null; abilitiesCacheTimestamp = 0; categoriesCacheTimestamp = 0; + abilitiesNamespaceSignature = ''; + categoriesNamespaceSignature = ''; } diff --git a/src/config.test.ts b/src/config.test.ts index c82a54a..1d07dd5 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -361,6 +361,81 @@ describe('loadConfig', () => { expect(config.dashboardUrl).toBe('https://from-env.com'); }); + + describe('abilityNamespaces', () => { + function envOnlyConfig() { + process.env.MAINWP_URL = 'https://test.com'; + process.env.MAINWP_USER = 'admin'; + process.env.MAINWP_APP_PASSWORD = 'xxxx'; + } + + it('defaults to ["mainwp"] when nothing is set', () => { + envOnlyConfig(); + const config = loadConfig(); + expect(config.abilityNamespaces).toEqual(['mainwp']); + }); + + it('parses comma-separated env var', () => { + envOnlyConfig(); + process.env.MAINWP_ABILITY_NAMESPACES = 'mainwp,acme,beta-test'; + + const config = loadConfig(); + expect(config.abilityNamespaces).toEqual(['mainwp', 'acme', 'beta-test']); + }); + + it('uses settings.json value when env var is unset', () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue( + JSON.stringify({ + dashboardUrl: 'https://test.com', + username: 'admin', + appPassword: 'xxxx', + abilityNamespaces: ['mainwp', 'acme'], + }) + ); + + const config = loadConfig(); + expect(config.abilityNamespaces).toEqual(['mainwp', 'acme']); + }); + + it('env var beats settings file', () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue( + JSON.stringify({ + dashboardUrl: 'https://from-file.com', + username: 'admin', + appPassword: 'xxxx', + abilityNamespaces: ['from-file'], + }) + ); + process.env.MAINWP_URL = 'https://test.com'; + process.env.MAINWP_ABILITY_NAMESPACES = 'from-env'; + + const config = loadConfig(); + expect(config.abilityNamespaces).toEqual(['from-env']); + }); + + it('rejects invalid namespace charset', () => { + envOnlyConfig(); + process.env.MAINWP_ABILITY_NAMESPACES = 'mainwp,BadCase'; + + expect(() => loadConfig()).toThrow(/abilityNamespaces.*BadCase/); + }); + + it('rejects empty array in settings.json', () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue( + JSON.stringify({ + dashboardUrl: 'https://test.com', + username: 'admin', + appPassword: 'xxxx', + abilityNamespaces: [], + }) + ); + + expect(() => loadConfig()).toThrow(/abilityNamespaces.*not be empty/); + }); + }); }); describe('getAbilitiesApiUrl', () => { @@ -382,6 +457,7 @@ describe('getAbilitiesApiUrl', () => { maxRetries: 2, retryBaseDelay: 1000, retryMaxDelay: 2000, + abilityNamespaces: ['mainwp'], configSource: 'environment', }; @@ -412,6 +488,7 @@ describe('getAuthHeaders', () => { maxRetries: 2, retryBaseDelay: 1000, retryMaxDelay: 2000, + abilityNamespaces: ['mainwp'], configSource: 'environment', }; @@ -440,6 +517,7 @@ describe('getAuthHeaders', () => { maxRetries: 2, retryBaseDelay: 1000, retryMaxDelay: 2000, + abilityNamespaces: ['mainwp'], configSource: 'environment', }; @@ -466,6 +544,7 @@ describe('getAuthHeaders', () => { maxRetries: 2, retryBaseDelay: 1000, retryMaxDelay: 2000, + abilityNamespaces: ['mainwp'], configSource: 'environment', }; @@ -494,6 +573,7 @@ describe('getAuthHeaders', () => { maxRetries: 2, retryBaseDelay: 1000, retryMaxDelay: 2000, + abilityNamespaces: ['mainwp'], configSource: 'environment', }; diff --git a/src/config.ts b/src/config.ts index aaa4412..f758196 100644 --- a/src/config.ts +++ b/src/config.ts @@ -60,6 +60,17 @@ export interface Config { schemaVerbosity: SchemaVerbosity; /** Response format: 'compact' omits whitespace to reduce token usage, 'pretty' uses 2-space indentation */ responseFormat: ResponseFormat; + /** + * Allowed ability namespaces (default: ['mainwp']). Abilities whose name's + * namespace prefix is in this list are surfaced as MCP tools. The first + * entry is the *primary* namespace — its abilities get unprefixed tool + * names; other namespaces produce `{ns}__{tool}` to avoid collisions. + * + * Typed as a non-empty tuple so `abilityNamespaces[0]` is statically known + * to be a string. `loadConfig` enforces this at runtime; the type makes + * direct Config construction (e.g. in tests) honor the same invariant. + */ + abilityNamespaces: [string, ...string[]]; /** Enable retry logic for transient errors (default: true) */ retryEnabled: boolean; /** Maximum retry attempts including initial request (default: 2) */ @@ -108,6 +119,8 @@ export interface SettingsFile { schemaVerbosity?: SchemaVerbosity; /** Response format: 'compact' omits whitespace to reduce token usage, 'pretty' uses 2-space indentation */ responseFormat?: ResponseFormat; + /** Allowed ability namespaces (default: ['mainwp']). First entry is the primary namespace. */ + abilityNamespaces?: string[]; /** Enable retry logic for transient errors (default: true) */ retryEnabled?: boolean; /** Maximum retry attempts including initial request (default: 2) */ @@ -120,6 +133,15 @@ export interface SettingsFile { const SETTINGS_FILENAME = 'settings.json'; +/** + * Valid WP Abilities namespace charset. Matches the WordPress core + * registry regex for the namespace portion of an ability name. + */ +const ABILITY_NAMESPACE_RE = /^[a-z0-9-]+$/; + +/** Default namespace allowlist. */ +const DEFAULT_ABILITY_NAMESPACES: readonly string[] = ['mainwp'] as const; + /** * Validate settings file structure and types * Throws on validation errors with descriptive messages @@ -157,7 +179,7 @@ function validateSettingsFile(settings: any, filePath: string): void { 'retryBaseDelay', 'retryMaxDelay', ]; - const arrayFields = ['allowedTools', 'blockedTools']; + const arrayFields = ['allowedTools', 'blockedTools', 'abilityNamespaces']; // Validate string fields for (const field of stringFields) { @@ -208,6 +230,23 @@ function validateSettingsFile(settings: any, filePath: string): void { } } + // Validate abilityNamespaces entries (charset + non-empty list). The + // generic arrayFields check above already verified every entry is a string, + // so only the charset check runs here. + if (Array.isArray(settings.abilityNamespaces)) { + if (settings.abilityNamespaces.length === 0) { + errors.push('"abilityNamespaces" must not be empty'); + } + for (const ns of settings.abilityNamespaces) { + if (typeof ns !== 'string') continue; // arrayFields check reported this already + if (!ABILITY_NAMESPACE_RE.test(ns)) { + errors.push( + `"abilityNamespaces" entry "${ns}" must match ${ABILITY_NAMESPACE_RE} (lowercase alphanumeric and hyphens)` + ); + } + } + } + // Detect unknown fields // Valid fields: all SettingsFile properties plus _comment (for inline documentation per schema) const validFields = new Set([ @@ -386,7 +425,8 @@ export function loadConfig(): Config { process.env.MAINWP_RETRY_ENABLED || process.env.MAINWP_MAX_RETRIES || process.env.MAINWP_RETRY_BASE_DELAY || - process.env.MAINWP_RETRY_MAX_DELAY + process.env.MAINWP_RETRY_MAX_DELAY || + process.env.MAINWP_ABILITY_NAMESPACES ); const configSource: 'environment' | 'settings file' | 'mixed' = @@ -488,6 +528,33 @@ export function loadConfig(): Config { ); } + // Parse ability namespace allowlist (default: ['mainwp']). Deduplicate so + // the env-var path (no schema-level uniqueItems) and the settings.json path + // produce identical cache signatures for equivalent input. + const rawAbilityNamespaces = getStringArray( + process.env.MAINWP_ABILITY_NAMESPACES, + settings?.abilityNamespaces, + [...DEFAULT_ABILITY_NAMESPACES] + ); + const abilityNamespaces = Array.from(new Set(rawAbilityNamespaces)); + if (abilityNamespaces.length === 0) { + const source = process.env.MAINWP_ABILITY_NAMESPACES + ? `MAINWP_ABILITY_NAMESPACES="${process.env.MAINWP_ABILITY_NAMESPACES}"` + : 'settings.json'; + throw new Error( + `abilityNamespaces must contain at least one non-empty entry (got from ${source})` + ); + } + for (const ns of abilityNamespaces) { + if (!ABILITY_NAMESPACE_RE.test(ns)) { + throw new Error( + `Invalid abilityNamespaces entry "${ns}": must match ${ABILITY_NAMESPACE_RE} (lowercase alphanumeric and hyphens)` + ); + } + } + // Re-assert the non-empty-tuple shape for the Config interface. + const abilityNamespacesTuple = abilityNamespaces as [string, ...string[]]; + // Parse retry configuration const retryEnabled = getBoolean(process.env.MAINWP_RETRY_ENABLED, settings?.retryEnabled, true); @@ -579,6 +646,7 @@ export function loadConfig(): Config { maxSessionData, schemaVerbosity, responseFormat, + abilityNamespaces: abilityNamespacesTuple, retryEnabled, maxRetries, retryBaseDelay, diff --git a/src/help.ts b/src/help.ts index 979cf7c..02abd55 100644 --- a/src/help.ts +++ b/src/help.ts @@ -55,8 +55,8 @@ export interface HelpDocument { /** * Generate help documentation for a single ability */ -export function generateToolHelp(ability: Ability): ToolHelp { - const toolName = abilityNameToToolName(ability.name); +export function generateToolHelp(ability: Ability, primaryNamespace: string): ToolHelp { + const toolName = abilityNameToToolName(ability.name, primaryNamespace); const props = (ability.input_schema?.properties || {}) as Record>; const required = (ability.input_schema?.required as string[]) || []; @@ -90,8 +90,8 @@ export function generateToolHelp(ability: Ability): ToolHelp { /** * Generate complete help document from all abilities */ -export function generateHelpDocument(abilities: Ability[]): HelpDocument { - const toolHelps = abilities.map(generateToolHelp); +export function generateHelpDocument(abilities: Ability[], primaryNamespace: string): HelpDocument { + const toolHelps = abilities.map(a => generateToolHelp(a, primaryNamespace)); const normalizeCategory = (c: string | undefined) => c?.trim() || 'uncategorized'; const categories = [...new Set(toolHelps.map(h => normalizeCategory(h.category)))].sort(); diff --git a/src/index.ts b/src/index.ts index 44e7d96..09c015f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -28,7 +28,6 @@ import { } from '@modelcontextprotocol/sdk/types.js'; import { loadConfig, Config, formatJson } from './config.js'; import { getTools, executeTool } from './tools.js'; -import { toolNameToAbilityName } from './naming.js'; import { getSessionDataUsage } from './session.js'; import { fetchAbilities, @@ -37,7 +36,7 @@ import { onCacheRefresh, executeAbility, initRateLimiter, - getAbility, + getAbilityByToolName, type Ability, } from './abilities.js'; import { generateHelpDocument, generateToolHelp } from './help.js'; @@ -48,7 +47,7 @@ import { formatErrorResponse, getErrorMessage, McpErrorFactory, McpError } from // Server metadata const SERVER_NAME = 'mainwp-mcp'; -const SERVER_VERSION = '1.0.0-beta.2'; +const SERVER_VERSION = '1.0.0-beta.3'; // Completion limits const MAX_COMPLETION_SUGGESTIONS = 20; @@ -254,7 +253,7 @@ async function createServer(config: Config): Promise<{ server: Server; logger: L if (uri === 'mainwp://help') { const abilities = await fetchAbilities(config, false, logger); - return jsonResource(generateHelpDocument(abilities)); + return jsonResource(generateHelpDocument(abilities, config.abilityNamespaces[0])); } // Validate and parse the resource URI (throws on invalid URIs) @@ -272,14 +271,13 @@ async function createServer(config: Config): Promise<{ server: Server; logger: L if (parsed.type === 'tool-help' && parsed.params?.tool_name) { const toolName = parsed.params.tool_name as string; - const abilityName = toolNameToAbilityName(toolName, 'mainwp'); - const ability = await getAbility(config, abilityName, logger); + const ability = await getAbilityByToolName(config, toolName, logger); if (!ability) { throw McpErrorFactory.resourceNotFound(uri); } - return jsonResource(generateToolHelp(ability)); + return jsonResource(generateToolHelp(ability, config.abilityNamespaces[0])); } throw new Error(`Unhandled resource type: ${parsed.type}`); diff --git a/src/naming.test.ts b/src/naming.test.ts index 7cb62b8..985d52b 100644 --- a/src/naming.test.ts +++ b/src/naming.test.ts @@ -3,66 +3,59 @@ */ import { describe, it, expect } from 'vitest'; -import { abilityNameToToolName, toolNameToAbilityName } from './naming.js'; +import { abilityNameToToolName } from './naming.js'; describe('abilityNameToToolName', () => { - it('should convert mainwp/list-sites-v1 to list_sites_v1', () => { - expect(abilityNameToToolName('mainwp/list-sites-v1')).toBe('list_sites_v1'); - }); - - it('should strip namespace prefix', () => { - expect(abilityNameToToolName('custom/my-ability')).toBe('my_ability'); - }); - - it('should convert hyphens to underscores', () => { - expect(abilityNameToToolName('ns/a-b-c-d')).toBe('a_b_c_d'); - }); - - it('should handle multiple hyphens', () => { - expect(abilityNameToToolName('mainwp/get-site-updates-count-v1')).toBe( - 'get_site_updates_count_v1' + describe('primary namespace (prefix stripped)', () => { + it('strips the primary namespace and converts hyphens to underscores', () => { + expect(abilityNameToToolName('mainwp/list-sites-v1', 'mainwp')).toBe('list_sites_v1'); + }); + + it('strips a custom primary namespace', () => { + expect(abilityNameToToolName('custom/my-ability', 'custom')).toBe('my_ability'); + }); + + it('handles abilities with no hyphens', () => { + expect(abilityNameToToolName('mainwp/simple', 'mainwp')).toBe('simple'); + }); + + it('handles multiple hyphens', () => { + expect(abilityNameToToolName('mainwp/get-site-updates-count-v1', 'mainwp')).toBe( + 'get_site_updates_count_v1' + ); + }); + + it('treats only the first slash as the namespace separator', () => { + expect(abilityNameToToolName('mainwp/sub/path-name', 'mainwp')).toBe('sub/path_name'); + }); + }); + + describe('non-primary namespace (prefix preserved with __)', () => { + it('preserves the namespace with a double-underscore separator', () => { + expect(abilityNameToToolName('acme/do-thing-v1', 'mainwp')).toBe('acme__do_thing_v1'); + }); + + it('handles short ability names', () => { + expect(abilityNameToToolName('acme/ping', 'mainwp')).toBe('acme__ping'); + }); + + it('distinguishes namespaces that share a trailing name', () => { + expect(abilityNameToToolName('one/ping-v1', 'mainwp')).toBe('one__ping_v1'); + expect(abilityNameToToolName('two/ping-v1', 'mainwp')).toBe('two__ping_v1'); + }); + + it('converts hyphens in hyphenated namespaces to underscores', () => { + // Tool names must stay within [a-z0-9_] for MCP client compatibility; + // a namespace like `acme-corp` must not leak its hyphen into the tool name. + expect(abilityNameToToolName('acme-corp/do-thing-v1', 'mainwp')).toBe( + 'acme_corp__do_thing_v1' + ); + }); + }); + + it('throws on missing namespace separator', () => { + expect(() => abilityNameToToolName('invalid-ability-name', 'mainwp')).toThrow( + /missing namespace/ ); }); - - it('should throw on missing namespace separator', () => { - expect(() => abilityNameToToolName('invalid-ability-name')).toThrow(/missing namespace/); - }); - - it('should handle ability with no hyphens after namespace', () => { - expect(abilityNameToToolName('mainwp/simple')).toBe('simple'); - }); - - it('should handle deeply nested namespace', () => { - // Only first slash is the namespace separator - expect(abilityNameToToolName('mainwp/sub/path-name')).toBe('sub/path_name'); - }); -}); - -describe('toolNameToAbilityName', () => { - it('should convert list_sites_v1 to mainwp/list-sites-v1', () => { - expect(toolNameToAbilityName('list_sites_v1', 'mainwp')).toBe('mainwp/list-sites-v1'); - }); - - it('should prepend custom namespace', () => { - expect(toolNameToAbilityName('my_tool', 'custom')).toBe('custom/my-tool'); - }); - - it('should convert underscores to hyphens', () => { - expect(toolNameToAbilityName('a_b_c_d', 'ns')).toBe('ns/a-b-c-d'); - }); - - it('should handle tool name with no underscores', () => { - expect(toolNameToAbilityName('simple', 'mainwp')).toBe('mainwp/simple'); - }); - - it('should handle empty namespace', () => { - expect(toolNameToAbilityName('test_tool', '')).toBe('/test-tool'); - }); - - it('should be inverse of abilityNameToToolName for standard names', () => { - const original = 'mainwp/list-sites-v1'; - const toolName = abilityNameToToolName(original); - const restored = toolNameToAbilityName(toolName, 'mainwp'); - expect(restored).toBe(original); - }); }); diff --git a/src/naming.ts b/src/naming.ts index 8db0773..e20d95d 100644 --- a/src/naming.ts +++ b/src/naming.ts @@ -1,38 +1,46 @@ /** * Name Conversion Utilities * - * Shared functions for converting between MainWP ability names and MCP tool names. - * Extracted to avoid circular imports between abilities.ts and tools.ts. + * Forward-only: ability name → MCP tool name. Reverse lookup + * (tool name → ability) is handled via the cache index in abilities.ts + * (see getAbilityByToolName) — tool names are not uniquely decodable. */ /** - * Convert ability name to MCP tool name - * Strips namespace prefix since MCP server name provides context. - * e.g., "mainwp/list-sites-v1" -> "list_sites_v1" + * Convert an ability name to its MCP tool name. + * + * The function has two output shapes selected by `primaryNamespace`: + * + * 1. **Ability is in the primary namespace** — namespace prefix is stripped + * (the MCP server name already provides that context): + * abilityNameToToolName('mainwp/list-sites-v1', 'mainwp') + * → 'list_sites_v1' + * + * 2. **Ability is in any other configured namespace** — namespace is kept + * with a double-underscore separator so it can't be confused with the + * single underscores inside tool names. Hyphens in the namespace itself + * are converted to underscores to keep MCP tool names within the + * `[a-z0-9_]+` charset that all MCP clients accept: + * abilityNameToToolName('acme/do-thing-v1', 'mainwp') + * → 'acme__do_thing_v1' + * abilityNameToToolName('acme-corp/do-thing-v1', 'mainwp') + * → 'acme_corp__do_thing_v1' + * + * The `__` separator stays unambiguous because ability slugs cannot contain + * underscores (enforced by ABILITY_NAME_RE in abilities.ts). */ -export function abilityNameToToolName(abilityName: string): string { - // Strip namespace prefix: "mainwp/list-sites-v1" → "list-sites-v1" +export function abilityNameToToolName(abilityName: string, primaryNamespace: string): string { const slashIndex = abilityName.indexOf('/'); if (slashIndex === -1) { throw new Error(`Invalid ability name format (missing namespace): ${abilityName}`); } - const withoutNamespace = abilityName.slice(slashIndex + 1); - // Convert hyphens to underscores: "list-sites-v1" → "list_sites_v1" - return withoutNamespace.replace(/-/g, '_'); -} + const namespace = abilityName.slice(0, slashIndex); + const rest = abilityName.slice(slashIndex + 1).replace(/-/g, '_'); -/** - * Map MCP tool name back to ability name. - * Prepends the namespace since tool names don't include it. - * - * Note: This server only uses the 'mainwp' namespace. The namespace parameter - * is kept for test flexibility but is always called with 'mainwp' in production. - * - * @example toolNameToAbilityName("list_sites_v1", "mainwp") -> "mainwp/list-sites-v1" - */ -export function toolNameToAbilityName(toolName: string, namespace: string): string { - // Convert underscores back to hyphens: "list_sites_v1" → "list-sites-v1" - const withHyphens = toolName.replace(/_/g, '-'); - // Prepend namespace: "mainwp/list-sites-v1" - return `${namespace}/${withHyphens}`; + if (namespace === primaryNamespace) { + return rest; + } + // Convert hyphens in the namespace too so the tool name stays within + // [a-z0-9_]+ for MCP client compatibility. + return `${namespace.replace(/-/g, '_')}__${rest}`; } diff --git a/src/tool-schema.ts b/src/tool-schema.ts index 0923b1f..e2991a2 100644 --- a/src/tool-schema.ts +++ b/src/tool-schema.ts @@ -257,10 +257,14 @@ export function buildSafetyTags( * @param ability - The MainWP ability to convert * @param verbosity - Schema verbosity level ('compact' or 'standard') */ -export function abilityToTool(ability: Ability, verbosity: SchemaVerbosity = 'standard'): Tool { - // Create a tool name from the ability name - // e.g., "mainwp/list-sites-v1" -> "mainwp_list_sites_v1" - const toolName = abilityNameToToolName(ability.name); +export function abilityToTool( + ability: Ability, + primaryNamespace: string, + verbosity: SchemaVerbosity = 'standard' +): Tool { + // e.g., 'mainwp/list-sites-v1' + primary='mainwp' -> 'list_sites_v1' + // 'acme/do-thing-v1' + primary='mainwp' -> 'acme__do_thing_v1' + const toolName = abilityNameToToolName(ability.name, primaryNamespace); const meta = ability.meta?.annotations; let inputSchema = convertInputSchema(ability); diff --git a/src/tools.test.ts b/src/tools.test.ts index 1bde184..911d939 100644 --- a/src/tools.test.ts +++ b/src/tools.test.ts @@ -4,7 +4,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { getTools, executeTool, clearToolsCache } from './tools.js'; -import { abilityNameToToolName, toolNameToAbilityName } from './naming.js'; +import { abilityNameToToolName } from './naming.js'; import { getSessionDataUsage, resetSessionData, isNoOpError } from './session.js'; import { clearPendingPreviews } from './confirmation.js'; import { generateInstructions, buildSafetyTags } from './tool-schema.js'; @@ -146,6 +146,7 @@ const baseConfig: Config = { maxRetries: 2, retryBaseDelay: 1000, retryMaxDelay: 2000, + abilityNamespaces: ['mainwp'], configSource: 'environment', }; @@ -1422,12 +1423,7 @@ describe('isNoOpError', () => { describe('name conversion re-exports', () => { it('should export abilityNameToToolName', () => { expect(typeof abilityNameToToolName).toBe('function'); - expect(abilityNameToToolName('mainwp/test-v1')).toBe('test_v1'); - }); - - it('should export toolNameToAbilityName', () => { - expect(typeof toolNameToAbilityName).toBe('function'); - expect(toolNameToAbilityName('test_v1', 'mainwp')).toBe('mainwp/test-v1'); + expect(abilityNameToToolName('mainwp/test-v1', 'mainwp')).toBe('test_v1'); }); }); diff --git a/src/tools.ts b/src/tools.ts index aed402e..5c47bde 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -12,7 +12,7 @@ import { AbilityAnnotations, fetchAbilities, executeAbility, - getAbility, + getAbilityByToolName, } from './abilities.js'; import { Config, formatJson } from './config.js'; import { validateInput, sanitizeError } from './security.js'; @@ -24,7 +24,6 @@ import { isNoOpError, NOOP_DESCRIPTIONS, } from './session.js'; -import { toolNameToAbilityName } from './naming.js'; import { abilityToTool } from './tool-schema.js'; import { handleConfirmationFlow } from './confirmation.js'; import { buildSafeModeBlockedResponse, buildNoChangeResponse } from './confirmation-responses.js'; @@ -71,7 +70,7 @@ export function clearToolsCache(): void { */ export async function getTools(config: Config, logger?: Logger): Promise { const abilities = await fetchAbilities(config, false, logger); - const fingerprint = `${config.schemaVerbosity}|${config.allowedTools?.join(',') ?? ''}|${config.blockedTools?.join(',') ?? ''}`; + const fingerprint = `${config.schemaVerbosity}|${config.allowedTools?.join(',') ?? ''}|${config.blockedTools?.join(',') ?? ''}|${config.abilityNamespaces.join(',')}`; if ( cachedTools && @@ -81,7 +80,10 @@ export async function getTools(config: Config, logger?: Logger): Promise return cachedTools; } - let tools = abilities.map(ability => abilityToTool(ability, config.schemaVerbosity)); + const primaryNamespace = config.abilityNamespaces[0]; + let tools = abilities.map(ability => + abilityToTool(ability, primaryNamespace, config.schemaVerbosity) + ); const originalCount = tools.length; // Apply allowlist filter (whitelist) @@ -156,13 +158,14 @@ export async function executeTool( // Validate input before forwarding to API validateInput(args); - abilityName = toolNameToAbilityName(toolName, 'mainwp'); - - // Fetch ability metadata to check if destructive - const ability = await getAbility(config, abilityName, reqLogger); + // Resolve tool name → ability via the cache reverse index built during + // fetchAbilities. This handles both primary-namespace (unprefixed) tools + // and prefixed `{ns}__tool` names from non-primary namespaces. + const ability = await getAbilityByToolName(config, toolName, reqLogger); if (!ability) { - throw new Error(`Ability not found: ${abilityName}`); + throw new Error(`Ability not found for tool: ${toolName}`); } + abilityName = ability.name; const ctx = { tool: toolName, ability: abilityName }; diff --git a/tests/evals/description-quality.test.ts b/tests/evals/description-quality.test.ts index dcb6ff4..559de4d 100644 --- a/tests/evals/description-quality.test.ts +++ b/tests/evals/description-quality.test.ts @@ -35,6 +35,7 @@ const baseConfig: Config = { maxSessionData: 52428800, schemaVerbosity: 'standard', responseFormat: 'compact', + abilityNamespaces: ['mainwp'], configSource: 'environment', retryEnabled: false, maxRetries: 2, diff --git a/tests/evals/safety-coverage.test.ts b/tests/evals/safety-coverage.test.ts index b35411d..80bd96e 100644 --- a/tests/evals/safety-coverage.test.ts +++ b/tests/evals/safety-coverage.test.ts @@ -38,6 +38,7 @@ const baseConfig: Config = { maxSessionData: 52428800, schemaVerbosity: 'standard', responseFormat: 'compact', + abilityNamespaces: ['mainwp'], configSource: 'environment', retryEnabled: false, maxRetries: 2, diff --git a/tests/evals/schema-quality.test.ts b/tests/evals/schema-quality.test.ts index d6781dc..bbe0fe8 100644 --- a/tests/evals/schema-quality.test.ts +++ b/tests/evals/schema-quality.test.ts @@ -34,6 +34,7 @@ const baseConfig: Config = { maxSessionData: 52428800, schemaVerbosity: 'standard', responseFormat: 'compact', + abilityNamespaces: ['mainwp'], configSource: 'environment', retryEnabled: false, maxRetries: 2, diff --git a/tests/evals/token-budget.test.ts b/tests/evals/token-budget.test.ts index 22cef7f..5baba8c 100644 --- a/tests/evals/token-budget.test.ts +++ b/tests/evals/token-budget.test.ts @@ -36,6 +36,7 @@ const baseConfig: Config = { maxSessionData: 52428800, schemaVerbosity: 'standard', responseFormat: 'compact', + abilityNamespaces: ['mainwp'], configSource: 'environment', retryEnabled: false, maxRetries: 2, diff --git a/tests/integration/abilities.test.ts b/tests/integration/abilities.test.ts index febba55..341f151 100644 --- a/tests/integration/abilities.test.ts +++ b/tests/integration/abilities.test.ts @@ -38,6 +38,7 @@ const baseConfig: Config = { maxSessionData: 52428800, schemaVerbosity: 'standard', responseFormat: 'compact', + abilityNamespaces: ['mainwp'], configSource: 'environment', retryEnabled: false, maxRetries: 2, diff --git a/tests/integration/tools.test.ts b/tests/integration/tools.test.ts index d81a693..5743f68 100644 --- a/tests/integration/tools.test.ts +++ b/tests/integration/tools.test.ts @@ -34,6 +34,7 @@ const baseConfig: Config = { maxSessionData: 52428800, schemaVerbosity: 'standard', responseFormat: 'compact', + abilityNamespaces: ['mainwp'], configSource: 'environment', retryEnabled: false, maxRetries: 2, From 67354ee3a77c5363ea512cacfb531ab28e116b76 Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 10 Jun 2026 10:08:48 -0400 Subject: [PATCH 2/9] Harden configurable namespace filtering Build the ability and tool name indices into locals and assign all cache state together, so a collision throw during a refresh leaves the existing index intact instead of serving a partially built one. Allow hyphenated namespaces in ability names so configs like acme-corp round-trip through execute, while forbidding leading and trailing hyphens in both namespace regexes and the settings schema. Drop abilities whose names fail the strict format check at fetch time with a logged warning, so a malformed upstream name never surfaces as an invalid MCP tool name; the execute-time check remains as a second layer. Correct the docs on what depends on the mainwp namespace staying configured: the site resource returns an error payload and site ID completions come back empty when it is filtered out. New tests cover collision-safe refresh, hyphenated namespace round-trips, multi-namespace tool surfacing, malformed-name filtering, tools and categories cache invalidation on namespace changes, env var dedup and error attribution, and the prefix-related category listing behavior. Sync package-lock.json to beta.3. --- README.md | 2 +- docs/configuration.md | 2 + package-lock.json | 4 +- settings.schema.json | 2 +- src/abilities.test.ts | 100 ++++++++++++++++++++++++++++++++ src/abilities.ts | 72 +++++++++++++++++------ src/config.test.ts | 24 ++++++++ src/config.ts | 12 ++-- src/naming.test.ts | 3 + src/tools.test.ts | 57 ++++++++++++++++++ tests/integration/tools.test.ts | 71 +++++++++++++++++++++++ 11 files changed, 323 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 7deb8e2..f1ad808 100644 --- a/README.md +++ b/README.md @@ -670,7 +670,7 @@ See the [Security Guide](docs/security.md#confirmation-guardrails) for more deta Over 60 tools organized by category. Each tool shows parameters with type, requirement, and description. -> **Note:** Tool names omit the primary namespace (default `mainwp`), so `mainwp/list-sites-v1` becomes `list_sites_v1`. If you add other namespaces via `abilityNamespaces`, abilities in those namespaces are exposed as `{namespace}__{tool}` (e.g. `acme/do-thing-v1` → `acme__do_thing_v1`). +> **Note:** Tool names omit the primary namespace (default `mainwp`), so `mainwp/list-sites-v1` becomes `list_sites_v1`. If you add other namespaces via `abilityNamespaces`, abilities in those namespaces are exposed as `{namespace}__{tool}` (e.g. `acme/do-thing-v1` → `acme__do_thing_v1`). Keep `mainwp` in `abilityNamespaces` — the `mainwp://site/{id}` resource and the site ID prompt completions call `mainwp/get-site-v1` and `mainwp/list-sites-v1` directly and start returning errors or empty results if those abilities are filtered out.
Sites diff --git a/docs/configuration.md b/docs/configuration.md index 4a24942..b7e791a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -98,6 +98,8 @@ Examples with `abilityNamespaces: ["mainwp", "acme"]`: When combining with `allowedTools` / `blockedTools`, use the prefixed form for non-primary namespaces (e.g. `"allowedTools": ["list_sites_v1", "acme__do_thing_v1"]`). +**Keep `mainwp` in the list.** The `mainwp://site/{id}` resource calls the `mainwp/get-site-v1` ability directly, and prompt argument completions for site IDs call `mainwp/list-sites-v1`. Removing `mainwp` from `abilityNamespaces` filters those abilities out: `mainwp://site/{id}` returns an error payload, site ID completions come back empty, and `mainwp://help` lists no MainWP tools. If you want to expose third-party namespaces, add them alongside `mainwp` rather than replacing it. + --- ## Tool Filtering diff --git a/package-lock.json b/package-lock.json index ac08aa2..771c24e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@mainwp/mcp", - "version": "1.0.0-beta.2", + "version": "1.0.0-beta.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@mainwp/mcp", - "version": "1.0.0-beta.2", + "version": "1.0.0-beta.3", "license": "GPL-3.0", "dependencies": { "@modelcontextprotocol/sdk": "^1.27.1", diff --git a/settings.schema.json b/settings.schema.json index 4fa4cf3..74eb1f0 100644 --- a/settings.schema.json +++ b/settings.schema.json @@ -103,7 +103,7 @@ "type": "array", "items": { "type": "string", - "pattern": "^[a-z0-9-]+$" + "pattern": "^[a-z0-9]([a-z0-9-]*[a-z0-9])?$" }, "minItems": 1, "uniqueItems": true, diff --git a/src/abilities.test.ts b/src/abilities.test.ts index fdbda4b..3219cdc 100644 --- a/src/abilities.test.ts +++ b/src/abilities.test.ts @@ -324,6 +324,60 @@ describe('fetchAbilities', () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); + it('drops abilities with malformed names before they reach the tool index', async () => { + const payload = [ + ...sampleAbilities, + { + name: 'mainwp/sub/path-name', + label: 'Malformed', + description: 'Extra slash should be filtered out', + category: 'mainwp-misc', + meta: { annotations: { readonly: true, destructive: false, idempotent: true } }, + }, + ]; + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => payload, + headers: new Headers(), + }); + + const abilities = await fetchAbilities(baseConfig, true, mockLogger); + + expect(abilities.map(a => a.name)).not.toContain('mainwp/sub/path-name'); + expect(abilities.length).toBe(sampleAbilities.length); + expect(await getAbilityByToolName(baseConfig, 'sub/path_name')).toBeUndefined(); + expect(mockLogger.warning).toHaveBeenCalledWith( + 'Skipping ability with malformed name', + expect.objectContaining({ name: expect.stringContaining('sub/path-name') }) + ); + }); + + it('keeps the existing index intact when a refresh hits the collision throw', async () => { + // Warm cache with a clean ability set. + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + await fetchAbilities(baseConfig); + + // Force a refresh that returns malformed data — two abilities with the + // same name produce the same tool name and trip the collision check. + // The new atomic-assignment code must leave the existing toolNameIndex + // intact, so a tool-name lookup for any ability from the first fetch + // still resolves. + const dupedPayload = [sampleAbilities[0], sampleAbilities[0]]; + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => dupedPayload, + headers: new Headers(), + }); + await fetchAbilities(baseConfig, true, mockLogger); + + const delAbility = await getAbilityByToolName(baseConfig, 'delete_site_v1'); + expect(delAbility?.name).toBe('mainwp/delete-site-v1'); + }); + it('discards cache and re-throws when signature mismatches and refresh fails', async () => { // Populate cache for ['mainwp'] successfully. mockFetch.mockResolvedValueOnce({ @@ -528,6 +582,52 @@ describe('fetchCategories', () => { expect(categories.map(c => c.slug).sort()).toEqual(['acme-things', 'mainwp-sites']); }); + it('surfaces categories for prefix-related namespaces (acme vs acme-corp)', async () => { + const mixedCategories = [ + { slug: 'acme-foo', label: 'Acme Foo', description: 'Acme category' }, + { slug: 'acme-corp-bar', label: 'Acme Corp Bar', description: 'Acme Corp category' }, + { slug: 'other-skip', label: 'Other', description: 'Should be skipped' }, + ]; + + mockFetch.mockResolvedValue({ + ok: true, + json: async () => mixedCategories, + headers: new Headers(), + }); + + // Both prefix-related namespaces configured: both categories surface. + const both: Config = { ...baseConfig, abilityNamespaces: ['acme', 'acme-corp'] }; + const categories = await fetchCategories(both); + expect(categories.map(c => c.slug).sort()).toEqual(['acme-corp-bar', 'acme-foo']); + + // Known limitation (see isAllowedCategory): with only 'acme' configured, + // 'acme-corp-bar' still passes the prefix filter because category slugs + // carry no explicit namespace field. Pinned here so a future change to + // this behavior is a conscious one. + const acmeOnly: Config = { ...baseConfig, abilityNamespaces: ['acme'] }; + const acmeCategories = await fetchCategories(acmeOnly); + expect(acmeCategories.map(c => c.slug).sort()).toEqual(['acme-corp-bar', 'acme-foo']); + }); + + it('refreshes cache when abilityNamespaces changes between calls', async () => { + mockFetch.mockResolvedValue({ + ok: true, + json: async () => [ + ...sampleCategories, + { slug: 'acme-things', label: 'Acme', description: 'Acme stuff' }, + ], + headers: new Headers(), + }); + + await fetchCategories(baseConfig); + expect(mockFetch).toHaveBeenCalledTimes(1); + + // Different namespace allowlist must invalidate the cache despite fresh + // TTL, matching the equivalent fetchAbilities behavior. + await fetchCategories({ ...baseConfig, abilityNamespaces: ['mainwp', 'acme'] }); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + it('should paginate when X-WP-TotalPages > 1', async () => { vi.resetAllMocks(); diff --git a/src/abilities.ts b/src/abilities.ts index 05d792d..78111e4 100644 --- a/src/abilities.ts +++ b/src/abilities.ts @@ -22,8 +22,15 @@ import { abilityNameToToolName } from './naming.js'; /** Maximum age of stale cache before hard-failing (30 minutes) */ const MAX_STALE_AGE_MS = 30 * 60 * 1000; -/** Strict format for ability names — prevents path traversal in URL construction */ -const ABILITY_NAME_RE = /^[a-z0-9]+\/[a-z0-9]([a-z0-9-]*[a-z0-9])?$/; +/** + * Strict format for ability names — prevents path traversal in URL construction. + * The namespace portion allows internal hyphens so hyphenated namespaces like + * `acme-corp` round-trip cleanly between config and execute, but forbids + * leading/trailing hyphens (same shape as the slug portion). Both portions + * forbid `_` so the `__` namespace/slug separator stays unambiguous in tool + * names. Must stay in sync with ABILITY_NAMESPACE_RE in config.ts. + */ +const ABILITY_NAME_RE = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?\/[a-z0-9]([a-z0-9-]*[a-z0-9])?$/; /** * Rate limiter instance (initialized via initRateLimiter) @@ -108,6 +115,15 @@ function isAllowedNamespace(abilityName: string, namespaces: string[]): boolean /** * Returns true if the category slug starts with any configured namespace's * `{ns}-` prefix. + * + * Known limitation: category metadata from the WP Abilities API does not + * carry an explicit namespace field, so when the upstream server registers + * both `acme` and `acme-corp` and the user configures only `['acme']`, a + * slug like `acme-corp-things` (which belongs to `acme-corp`) still passes + * this filter and shows up in the `mainwp://categories` resource as an + * empty category — none of its abilities pass `isAllowedNamespace`, so no + * ability misrouting occurs. The effect is cosmetic. If both prefix-related + * namespaces are listed in the allowlist, all categories surface correctly. */ function isAllowedCategory(slug: string, namespaces: string[]): boolean { return namespaces.some(ns => slug.startsWith(`${ns}-`)); @@ -173,7 +189,18 @@ export async function fetchAbilities( logger ); - const newAbilities = allAbilities.filter(a => isAllowedNamespace(a.name, namespaces)); + const newAbilities = allAbilities.filter(a => { + if (!isAllowedNamespace(a.name, namespaces)) return false; + // Defense in depth: drop abilities whose names fail the strict format + // check before they reach the tool index. A malformed name (extra + // slash, bad charset) would otherwise surface as an invalid MCP tool + // name in ListTools and only fail at execute time. + if (!ABILITY_NAME_RE.test(a.name)) { + logger?.warning('Skipping ability with malformed name', { name: sanitizeError(a.name) }); + return false; + } + return true; + }); // Check if abilities have changed (compare names) const oldNames = @@ -187,28 +214,37 @@ export async function fetchAbilities( .join(','); const hasChanged = oldNames !== newNames; - cachedAbilities = newAbilities; - abilitiesIndex = new Map(); - toolNameIndex = new Map(); + // Build indices into local variables first, then assign all module-level + // cache state together once the loop has completed cleanly. If the + // collision throw fires mid-loop we leave the existing cache untouched + // rather than serving a partially built tool-name index that would make + // some tools silently unresolvable. + const newAbilitiesIndex = new Map(); + const newToolNameIndex = new Map(); const primary = namespaces[0]; for (const ability of newAbilities) { - abilitiesIndex.set(ability.name, ability); + newAbilitiesIndex.set(ability.name, ability); const toolName = abilityNameToToolName(ability.name, primary); - // Tool name collisions are structurally impossible: ABILITY_NAME_RE - // forbids `_` in ability slugs, `__` is used only as the namespace/slug - // separator for non-primary namespaces, and ability names are unique - // upstream. Fail loud if the invariant ever breaks — a silent override - // would shadow a real ability under the wrong name. - const existing = toolNameIndex.get(toolName); + // Tool name collisions are rare but possible: ABILITY_NAME_RE forbids + // `_` in names and `__` is the namespace/slug separator for non-primary + // namespaces, but a double hyphen in a slug also maps to `__` (e.g. + // primary `mainwp/foo--bar` and non-primary `foo/bar` both produce + // `foo__bar`), and upstream could return duplicate names. Fail loud + // rather than silently shadowing one ability under the other's tool + // name. + const existing = newToolNameIndex.get(toolName); if (existing) { throw new Error( `Tool name collision: "${toolName}" produced by both "${existing.name}" and "${ability.name}". ` + `This indicates a violation of the namespace/slug invariants in abilities.ts.` ); } - toolNameIndex.set(toolName, ability); + newToolNameIndex.set(toolName, ability); } + cachedAbilities = newAbilities; + abilitiesIndex = newAbilitiesIndex; + toolNameIndex = newToolNameIndex; abilitiesCacheTimestamp = Date.now(); abilitiesNamespaceSignature = signature; @@ -361,9 +397,11 @@ export async function getAbility( /** * Resolve an MCP tool name to its underlying ability via the cache index. - * Tool names are not uniquely decodable back to ability names (multi-namespace - * collisions get numeric suffixes), so reverse lookup must go through the map - * built during `fetchAbilities`. + * Tool names are not uniquely decodable back to ability names — hyphens in + * the ability slug map to underscores, and the same shape can in principle + * collide across namespaces — so reverse lookup goes through the map built + * during `fetchAbilities`. The build loop throws on any collision rather + * than guessing; see `fetchAbilities` for the invariant rationale. */ export async function getAbilityByToolName( config: Config, diff --git a/src/config.test.ts b/src/config.test.ts index 1d07dd5..1eb300b 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -422,6 +422,15 @@ describe('loadConfig', () => { expect(() => loadConfig()).toThrow(/abilityNamespaces.*BadCase/); }); + it('rejects namespaces with leading or trailing hyphens', () => { + envOnlyConfig(); + process.env.MAINWP_ABILITY_NAMESPACES = 'mainwp,-acme'; + expect(() => loadConfig()).toThrow(/abilityNamespaces.*-acme/); + + process.env.MAINWP_ABILITY_NAMESPACES = 'mainwp,acme-'; + expect(() => loadConfig()).toThrow(/abilityNamespaces.*acme-/); + }); + it('rejects empty array in settings.json', () => { vi.mocked(fs.existsSync).mockReturnValue(true); vi.mocked(fs.readFileSync).mockReturnValue( @@ -435,6 +444,21 @@ describe('loadConfig', () => { expect(() => loadConfig()).toThrow(/abilityNamespaces.*not be empty/); }); + + it('deduplicates repeated env-var entries while preserving order', () => { + envOnlyConfig(); + process.env.MAINWP_ABILITY_NAMESPACES = 'mainwp,mainwp,acme,mainwp'; + + const config = loadConfig(); + expect(config.abilityNamespaces).toEqual(['mainwp', 'acme']); + }); + + it('attributes the empty-list error to the env var when the env var was all-blank', () => { + envOnlyConfig(); + process.env.MAINWP_ABILITY_NAMESPACES = ',,'; + + expect(() => loadConfig()).toThrow(/MAINWP_ABILITY_NAMESPACES/); + }); }); }); diff --git a/src/config.ts b/src/config.ts index f758196..ccd5bf7 100644 --- a/src/config.ts +++ b/src/config.ts @@ -134,10 +134,12 @@ export interface SettingsFile { const SETTINGS_FILENAME = 'settings.json'; /** - * Valid WP Abilities namespace charset. Matches the WordPress core - * registry regex for the namespace portion of an ability name. + * Valid WP Abilities namespace: lowercase alphanumeric with internal hyphens, + * no leading/trailing hyphen. Must stay in sync with the namespace portion of + * ABILITY_NAME_RE in abilities.ts — a namespace accepted here but rejected + * there would silently filter out every ability it matches. */ -const ABILITY_NAMESPACE_RE = /^[a-z0-9-]+$/; +const ABILITY_NAMESPACE_RE = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/; /** Default namespace allowlist. */ const DEFAULT_ABILITY_NAMESPACES: readonly string[] = ['mainwp'] as const; @@ -241,7 +243,7 @@ function validateSettingsFile(settings: any, filePath: string): void { if (typeof ns !== 'string') continue; // arrayFields check reported this already if (!ABILITY_NAMESPACE_RE.test(ns)) { errors.push( - `"abilityNamespaces" entry "${ns}" must match ${ABILITY_NAMESPACE_RE} (lowercase alphanumeric and hyphens)` + `"abilityNamespaces" entry "${ns}" must match ${ABILITY_NAMESPACE_RE} (lowercase alphanumeric and internal hyphens)` ); } } @@ -548,7 +550,7 @@ export function loadConfig(): Config { for (const ns of abilityNamespaces) { if (!ABILITY_NAMESPACE_RE.test(ns)) { throw new Error( - `Invalid abilityNamespaces entry "${ns}": must match ${ABILITY_NAMESPACE_RE} (lowercase alphanumeric and hyphens)` + `Invalid abilityNamespaces entry "${ns}": must match ${ABILITY_NAMESPACE_RE} (lowercase alphanumeric and internal hyphens)` ); } } diff --git a/src/naming.test.ts b/src/naming.test.ts index 985d52b..f4cef98 100644 --- a/src/naming.test.ts +++ b/src/naming.test.ts @@ -26,6 +26,9 @@ describe('abilityNameToToolName', () => { }); it('treats only the first slash as the namespace separator', () => { + // Raw function contract only: multi-slash names fail ABILITY_NAME_RE + // and are filtered out during fetchAbilities, so they never reach this + // function in practice. expect(abilityNameToToolName('mainwp/sub/path-name', 'mainwp')).toBe('sub/path_name'); }); }); diff --git a/src/tools.test.ts b/src/tools.test.ts index 911d939..b6f2a6e 100644 --- a/src/tools.test.ts +++ b/src/tools.test.ts @@ -423,6 +423,63 @@ describe('getTools', () => { // Short descriptions should pass through unchanged (no category prefix in compact) expect(listTool?.description).toBe('Get all managed sites'); }); + + it('surfaces tools from every configured namespace at once', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => [ + ...sampleAbilities, + { + name: 'acme/do-thing-v1', + label: 'Acme Do Thing', + description: 'Third-party ability', + category: 'acme-misc', + meta: { annotations: { readonly: true, destructive: false, idempotent: true } }, + }, + ], + headers: new Headers(), + }); + + const config = { + ...baseConfig, + abilityNamespaces: ['mainwp', 'acme'] as [string, ...string[]], + }; + const tools = await getTools(config); + + const names = tools.map(t => t.name); + expect(names).toContain('list_sites_v1'); + expect(names).toContain('acme__do_thing_v1'); + }); + + it('invalidates the tools cache when abilityNamespaces changes', async () => { + const payload = [ + ...sampleAbilities, + { + name: 'acme/do-thing-v1', + label: 'Acme Do Thing', + description: 'Third-party ability', + category: 'acme-misc', + meta: { annotations: { readonly: true, destructive: false, idempotent: true } }, + }, + ]; + mockFetch.mockResolvedValue({ + ok: true, + json: async () => payload, + headers: new Headers(), + }); + + const firstTools = await getTools(baseConfig); + expect(firstTools.map(t => t.name)).not.toContain('acme__do_thing_v1'); + + // Same diff-config-shape except for the namespace allowlist — the tools + // cache fingerprint must include abilityNamespaces, or this call would + // return the stale first result. + const secondTools = await getTools({ + ...baseConfig, + abilityNamespaces: ['mainwp', 'acme'] as [string, ...string[]], + }); + expect(secondTools.map(t => t.name)).toContain('acme__do_thing_v1'); + }); }); describe('executeTool', () => { diff --git a/tests/integration/tools.test.ts b/tests/integration/tools.test.ts index 5743f68..a3a17d6 100644 --- a/tests/integration/tools.test.ts +++ b/tests/integration/tools.test.ts @@ -446,4 +446,75 @@ describe('Tools Integration', () => { ); }); }); + + describe('non-primary namespace tools', () => { + it('resolves an {ns}__ tool name to the right ability URL', async () => { + const abilitiesPayload = [ + { + name: 'acme/do-thing-v1', + label: 'Acme Do Thing', + description: 'Third-party readonly ability', + category: 'acme-misc', + meta: { annotations: { readonly: true, destructive: false, idempotent: true } }, + }, + ]; + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => abilitiesPayload, + headers: new Headers(), + }); + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ result: 'pong' }), + headers: new Headers(), + }); + + const config: Config = { + ...baseConfig, + abilityNamespaces: ['mainwp', 'acme'] as [string, ...string[]], + }; + + const result = await executeTool(config, 'acme__do_thing_v1', { input: 'ping' }, mockLogger); + + const executeCall = mockFetch.mock.calls[1]; + const url = executeCall[0] as string; + expect(url).toContain('/abilities/acme/do-thing-v1/run'); + expect(JSON.parse(result[0].text)).toEqual({ result: 'pong' }); + }); + + it('round-trips a hyphenated namespace through execute', async () => { + const abilitiesPayload = [ + { + name: 'acme-corp/do-thing-v1', + label: 'Acme Corp Do Thing', + description: 'Hyphenated-namespace ability', + category: 'acme-corp-misc', + meta: { annotations: { readonly: true, destructive: false, idempotent: true } }, + }, + ]; + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => abilitiesPayload, + headers: new Headers(), + }); + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ ok: true }), + headers: new Headers(), + }); + + const config: Config = { + ...baseConfig, + abilityNamespaces: ['mainwp', 'acme-corp'] as [string, ...string[]], + }; + + await executeTool(config, 'acme_corp__do_thing_v1', {}, mockLogger); + + const executeCall = mockFetch.mock.calls[1]; + const url = executeCall[0] as string; + expect(url).toContain('/abilities/acme-corp/do-thing-v1/run'); + }); + }); }); From ced885c608b799d9e333d26677de0bf24d286518 Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 10 Jun 2026 11:49:08 -0400 Subject: [PATCH 3/9] Add changelog entry for 1.0.0-beta.3 The changelog covers the configurable namespace work: the new abilityNamespaces setting, the removed toolNameToAbilityName export, and the cache and malformed-name hardening. The version stays at beta.3, the bump the feature commit already made over the published beta.2; the lockfile is synced to match. --- CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a964f35..b612a8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,20 @@ All notable changes to mainwp-mcp are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/). +## [1.0.0-beta.3] - 2026-06-10 + +### Added + +Ability namespaces are now configurable. The server used to surface only `mainwp/` abilities; the new `abilityNamespaces` setting (or the `MAINWP_ABILITY_NAMESPACES` environment variable) lets you expose abilities that third-party MainWP extensions register through the WordPress Abilities API. The first namespace in the list is the primary one and its tools keep their plain names, so `mainwp/list-sites-v1` still appears as `list_sites_v1`. Abilities from other namespaces carry a prefix: `acme/do-thing-v1` becomes `acme__do_thing_v1`. Hyphenated namespaces such as `acme-corp` work end to end, including execution. Built-in resources and prompt completions depend on `mainwp/get-site-v1` and `mainwp/list-sites-v1`, so keep `mainwp` in the list when adding others. + +### Removed + +The `toolNameToAbilityName` export is gone. Tool names stopped being uniquely decodable once multiple namespaces came into play, so reverse lookup now goes through an index built when abilities are fetched. Anything importing that function from this package needs to switch to `getAbilityByToolName`. + +### Fixed + +A failed ability refresh can no longer leave the tool index half built. The cache swaps in atomically, and if the fetch hits duplicate tool names the previous index keeps serving while the error is reported. Abilities with malformed names, an extra slash for example, are dropped at fetch time with a logged warning rather than surfacing as invalid MCP tool names. + ## [1.0.0-beta.2] - 2026-03-26 ### Changed From ca434c1fca28358542319b16e2ec48a69dee8416 Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 10 Jun 2026 12:43:12 -0400 Subject: [PATCH 4/9] Set isError on failed tool call results Failed tool calls returned a normal MCP result with the error JSON in content but no isError flag, so MCP clients and agent frameworks that branch on isError treated failures as successes. executeTool now returns the full CallToolResult shape ({ content, isError? }) instead of bare content. isError: true is set on every failed-call path: unknown tool, input validation failure, ability execution failure, cancellation, safe-mode block, and confirmation rejections (INVALID_PARAMETER, CONFLICTING_PARAMETERS, PREVIEW_REQUIRED, PREVIEW_EXPIRED). CONFIRMATION_REQUIRED previews and idempotent NO_CHANGE responses stay non-error since the call did what was asked. The JSON error body in content is unchanged, so existing consumers still parse it. Verified live over stdio against the testbed dashboard: unknown tool and invalid input both return isError: true; successful calls carry no flag. --- src/confirmation.ts | 12 +++- src/index.ts | 14 ++-- src/tools.test.ts | 119 ++++++++++++++++++++------------ src/tools.ts | 67 ++++++++++++------ tests/integration/tools.test.ts | 49 +++++++------ 5 files changed, 165 insertions(+), 96 deletions(-) diff --git a/src/confirmation.ts b/src/confirmation.ts index 6705cc8..064d648 100644 --- a/src/confirmation.ts +++ b/src/confirmation.ts @@ -52,11 +52,14 @@ export function clearPendingPreviews(): void { * Result of the confirmation flow evaluation. * * - `respond`: return the response directly to the client (preview, error, etc.) + * `isError: true` marks rejections (invalid/conflicting parameters, missing + * or expired preview) so the tool result carries the MCP `isError` flag; + * preview responses (CONFIRMATION_REQUIRED) are successful workflow steps. * - `execute`: proceed with execution using the (possibly modified) effectiveArgs * - `skip`: confirmation flow does not apply — proceed with original args */ export type ConfirmationResult = - | { action: 'respond'; response: TextContent[] } + | { action: 'respond'; response: TextContent[]; isError?: boolean } | { action: 'execute'; effectiveArgs: Record } | { action: 'skip' }; @@ -149,6 +152,7 @@ export async function handleConfirmationFlow( return { action: 'respond', response: [{ type: 'text', text: formatJson(config, buildInvalidParameterResponse(ctx)) }], + isError: true, }; } @@ -169,6 +173,7 @@ export async function handleConfirmationFlow( response: [ { type: 'text', text: formatJson(config, buildConflictingParametersResponse(ctx)) }, ], + isError: true, }; } @@ -244,6 +249,7 @@ export async function handleConfirmationFlow( return { action: 'respond', response: [{ type: 'text', text: formatJson(config, buildPreviewRequiredResponse(ctx)) }], + isError: true, }; } // Verify token belongs to this tool (prevent cross-tool reuse) @@ -253,6 +259,7 @@ export async function handleConfirmationFlow( return { action: 'respond', response: [{ type: 'text', text: formatJson(config, buildPreviewRequiredResponse(ctx)) }], + isError: true, }; } // Verify token matches current arguments (prevent arg-swap) @@ -263,6 +270,7 @@ export async function handleConfirmationFlow( return { action: 'respond', response: [{ type: 'text', text: formatJson(config, buildPreviewRequiredResponse(ctx)) }], + isError: true, }; } previewKey = tokenPreviewKey; @@ -278,6 +286,7 @@ export async function handleConfirmationFlow( return { action: 'respond', response: [{ type: 'text', text: formatJson(config, buildPreviewRequiredResponse(ctx)) }], + isError: true, }; } @@ -288,6 +297,7 @@ export async function handleConfirmationFlow( return { action: 'respond', response: [{ type: 'text', text: formatJson(config, buildPreviewExpiredResponse(ctx)) }], + isError: true, }; } diff --git a/src/index.ts b/src/index.ts index 09c015f..fe8e83f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -146,15 +146,11 @@ async function createServer(config: Config): Promise<{ server: Server; logger: L const { name, arguments: args } = request.params; try { - // Pass abort signal for cancellation support - const content = await executeTool( - config, - name, - (args as Record) ?? {}, - logger, - { signal: extra.signal } - ); - return { content }; + // Pass abort signal for cancellation support; executeTool returns the + // full CallToolResult shape including isError on failed calls + return await executeTool(config, name, (args as Record) ?? {}, logger, { + signal: extra.signal, + }); } catch (error) { return { content: [ diff --git a/src/tools.test.ts b/src/tools.test.ts index b6f2a6e..7492fff 100644 --- a/src/tools.test.ts +++ b/src/tools.test.ts @@ -511,10 +511,11 @@ describe('executeTool', () => { const result = await executeTool(baseConfig, 'list_sites_v1', {}, mockLogger); - expect(result).toHaveLength(1); - expect(result[0].type).toBe('text'); - const parsed = JSON.parse(result[0].text); + expect(result.content).toHaveLength(1); + expect(result.content[0].type).toBe('text'); + const parsed = JSON.parse(result.content[0].text); expect(parsed).toContainEqual({ id: 1, name: 'Site 1' }); + expect(result.isError).toBeUndefined(); }); it('should validate input before execution', async () => { @@ -527,7 +528,22 @@ describe('executeTool', () => { // Invalid ID should be caught by validation const result = await executeTool(baseConfig, 'list_sites_v1', { site_id: -1 }, mockLogger); - expect(result[0].text).toContain('error'); + expect(result.content[0].text).toContain('error'); + expect(result.isError).toBe(true); + }); + + it('should return isError for unknown tool', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + + const result = await executeTool(baseConfig, 'no_such_tool_v1', {}, mockLogger); + + expect(result.isError).toBe(true); + const parsed = JSON.parse(result.content[0].text); + expect(parsed.error.message).toContain('Ability not found for tool: no_such_tool_v1'); }); it('should block destructive operations in safe mode', async () => { @@ -545,7 +561,8 @@ describe('executeTool', () => { mockLogger ); - expect(result[0].text).toContain('SAFE_MODE_BLOCKED'); + expect(result.content[0].text).toContain('SAFE_MODE_BLOCKED'); + expect(result.isError).toBe(true); }); it('should strip confirm parameter in safe mode', async () => { @@ -586,14 +603,17 @@ describe('executeTool', () => { mockLogger ); - expect(result[0].text).toContain('CONFIRMATION_REQUIRED'); - expect(result[0].text).toContain('preview'); + expect(result.content[0].text).toContain('CONFIRMATION_REQUIRED'); + expect(result.content[0].text).toContain('preview'); // Should include a confirmation token at top level - const parsed = JSON.parse(result[0].text); + const parsed = JSON.parse(result.content[0].text); expect(parsed.confirmation_token).toBeDefined(); expect(typeof parsed.confirmation_token).toBe('string'); expect(parsed.confirmation_token.length).toBeGreaterThan(0); + + // Preview is a successful workflow step, not a failed call + expect(result.isError).toBeUndefined(); }); it('should reject user_confirmed without prior preview', async () => { @@ -610,7 +630,8 @@ describe('executeTool', () => { mockLogger ); - expect(result[0].text).toContain('PREVIEW_REQUIRED'); + expect(result.content[0].text).toContain('PREVIEW_REQUIRED'); + expect(result.isError).toBe(true); }); it('should reject conflicting dry_run and user_confirmed', async () => { @@ -627,7 +648,8 @@ describe('executeTool', () => { mockLogger ); - expect(result[0].text).toContain('CONFLICTING_PARAMETERS'); + expect(result.content[0].text).toContain('CONFLICTING_PARAMETERS'); + expect(result.isError).toBe(true); }); it('should accept confirmation_token to resolve preview', async () => { @@ -649,7 +671,7 @@ describe('executeTool', () => { { site_id: 1, confirm: true }, mockLogger ); - const parsed = JSON.parse(previewResult[0].text); + const parsed = JSON.parse(previewResult.content[0].text); const token = parsed.confirmation_token; // Step 2: Confirm with token @@ -666,7 +688,7 @@ describe('executeTool', () => { mockLogger ); - expect(confirmResult[0].text).toContain('success'); + expect(confirmResult.content[0].text).toContain('success'); }); it('should reject invalid confirmation_token', async () => { @@ -683,7 +705,8 @@ describe('executeTool', () => { mockLogger ); - expect(result[0].text).toContain('PREVIEW_REQUIRED'); + expect(result.content[0].text).toContain('PREVIEW_REQUIRED'); + expect(result.isError).toBe(true); }); it('should not include confirmation_token in preview key', async () => { @@ -716,7 +739,7 @@ describe('executeTool', () => { ); // Second call should still succeed (overwrites same key) - expect(result2[0].text).toContain('CONFIRMATION_REQUIRED'); + expect(result2.content[0].text).toContain('CONFIRMATION_REQUIRED'); }); it('should handle user_confirmed on tool without confirm parameter', async () => { @@ -741,9 +764,9 @@ describe('executeTool', () => { ); // Non-destructive tool should execute normally — no error, no rejection - expect(result).toHaveLength(1); - expect(result[0].type).toBe('text'); - expect(result[0].text).not.toContain('error'); + expect(result.content).toHaveLength(1); + expect(result.content[0].type).toBe('text'); + expect(result.content[0].text).not.toContain('error'); expect(mockFetch).toHaveBeenCalledTimes(2); // Abilities fetch + execution }); @@ -761,7 +784,8 @@ describe('executeTool', () => { signal: controller.signal, }); - expect(result[0].text).toContain('cancelled'); + expect(result.content[0].text).toContain('cancelled'); + expect(result.isError).toBe(true); }); it('should log tool execution with timing', async () => { @@ -796,7 +820,8 @@ describe('executeTool', () => { const result = await executeTool(baseConfig, 'list_sites_v1', {}, mockLogger); - expect(result[0].text).toContain('error'); + expect(result.content[0].text).toContain('error'); + expect(result.isError).toBe(true); expect(mockLogger.error).toHaveBeenCalled(); }); @@ -814,10 +839,10 @@ describe('executeTool', () => { }); const result = await executeTool(baseConfig, 'list_sites_v1', {}, mockLogger); - const parsed = JSON.parse(result[0].text); + const parsed = JSON.parse(result.content[0].text); // Compact format should equal JSON.stringify without indentation - expect(result[0].text).toBe(JSON.stringify(parsed)); + expect(result.content[0].text).toBe(JSON.stringify(parsed)); }); it('should return pretty JSON when responseFormat is pretty', async () => { @@ -837,9 +862,9 @@ describe('executeTool', () => { const result = await executeTool(prettyConfig, 'list_sites_v1', {}, mockLogger); // Pretty format should contain newlines (indented) - expect(result[0].text).toContain('\n'); - const parsed = JSON.parse(result[0].text); - expect(result[0].text).toBe(JSON.stringify(parsed, null, 2)); + expect(result.content[0].text).toContain('\n'); + const parsed = JSON.parse(result.content[0].text); + expect(result.content[0].text).toBe(JSON.stringify(parsed, null, 2)); }); }); @@ -882,7 +907,7 @@ describe('confirmation flow - full cycle', () => { mockLogger ); - expect(previewResult[0].text).toContain('CONFIRMATION_REQUIRED'); + expect(previewResult.content[0].text).toContain('CONFIRMATION_REQUIRED'); // Step 2: Advance time beyond PREVIEW_EXPIRY_MS (5 minutes + 1ms) vi.setSystemTime(startTime + 5 * 60 * 1000 + 1); @@ -895,7 +920,8 @@ describe('confirmation flow - full cycle', () => { mockLogger ); - expect(expiredResult[0].text).toContain('PREVIEW_EXPIRED'); + expect(expiredResult.content[0].text).toContain('PREVIEW_EXPIRED'); + expect(expiredResult.isError).toBe(true); expect(mockLogger.warning).toHaveBeenCalledWith( 'Confirmation failed - preview expired', expect.objectContaining({ toolName: 'delete_site_v1' }) @@ -909,7 +935,7 @@ describe('confirmation flow - full cycle', () => { mockLogger ); - expect(subsequentResult[0].text).toContain('PREVIEW_REQUIRED'); + expect(subsequentResult.content[0].text).toContain('PREVIEW_REQUIRED'); }); it('should reject reuse of consumed confirmation_token', async () => { @@ -931,7 +957,7 @@ describe('confirmation flow - full cycle', () => { { site_id: 1, confirm: true }, mockLogger ); - const parsed = JSON.parse(previewResult[0].text); + const parsed = JSON.parse(previewResult.content[0].text); const token = parsed.confirmation_token; // Step 2: Confirm with token (consumes it) @@ -947,7 +973,7 @@ describe('confirmation flow - full cycle', () => { { site_id: 1, user_confirmed: true, confirmation_token: token }, mockLogger ); - expect(confirmResult[0].text).toContain('success'); + expect(confirmResult.content[0].text).toContain('success'); // Step 3: Attempt to reuse the same token const reuseResult = await executeTool( @@ -956,7 +982,7 @@ describe('confirmation flow - full cycle', () => { { site_id: 1, user_confirmed: true, confirmation_token: token }, mockLogger ); - expect(reuseResult[0].text).toContain('PREVIEW_REQUIRED'); + expect(reuseResult.content[0].text).toContain('PREVIEW_REQUIRED'); }); it('should reject cross-tool confirmation token reuse', async () => { @@ -978,7 +1004,7 @@ describe('confirmation flow - full cycle', () => { { site_id: 1, confirm: true }, mockLogger ); - const parsed = JSON.parse(previewResult[0].text); + const parsed = JSON.parse(previewResult.content[0].text); const token = parsed.confirmation_token; // Step 2: Attempt to use that token on a different destructive tool @@ -990,7 +1016,8 @@ describe('confirmation flow - full cycle', () => { ); // Should be rejected — token was scoped to delete_site_v1 - expect(crossToolResult[0].text).toContain('PREVIEW_REQUIRED'); + expect(crossToolResult.content[0].text).toContain('PREVIEW_REQUIRED'); + expect(crossToolResult.isError).toBe(true); expect(mockLogger.warning).toHaveBeenCalledWith( 'Confirmation failed - token belongs to different tool', expect.objectContaining({ toolName: 'delete_plugins_v1' }) @@ -1003,7 +1030,7 @@ describe('confirmation flow - full cycle', () => { { site_id: 1, user_confirmed: true, confirmation_token: token }, mockLogger ); - expect(reuseResult[0].text).toContain('PREVIEW_REQUIRED'); + expect(reuseResult.content[0].text).toContain('PREVIEW_REQUIRED'); }); it('should reject confirmation when arguments differ from preview (arg-swap)', async () => { @@ -1025,7 +1052,7 @@ describe('confirmation flow - full cycle', () => { { site_id: 1, confirm: true }, mockLogger ); - const parsed = JSON.parse(previewResult[0].text); + const parsed = JSON.parse(previewResult.content[0].text); const token = parsed.confirmation_token; expect(token).toBeDefined(); @@ -1038,7 +1065,8 @@ describe('confirmation flow - full cycle', () => { ); // Should be rejected — args don't match the preview - expect(swapResult[0].text).toContain('PREVIEW_REQUIRED'); + expect(swapResult.content[0].text).toContain('PREVIEW_REQUIRED'); + expect(swapResult.isError).toBe(true); expect(mockLogger.warning).toHaveBeenCalledWith( 'Confirmation failed - arguments do not match preview', expect.objectContaining({ toolName: 'delete_site_v1' }) @@ -1066,7 +1094,7 @@ describe('confirmation flow - full cycle', () => { mockLogger ); - expect(previewResult[0].text).toContain('CONFIRMATION_REQUIRED'); + expect(previewResult.content[0].text).toContain('CONFIRMATION_REQUIRED'); // Step 2: Confirm execution // Note: abilities are already cached from step 1, so no need to mock abilities fetch again @@ -1083,7 +1111,7 @@ describe('confirmation flow - full cycle', () => { mockLogger ); - expect(confirmResult[0].text).toContain('success'); + expect(confirmResult.content[0].text).toContain('success'); }); }); @@ -1147,8 +1175,10 @@ describe('no-op error handling for idempotent tools', () => { mockLogger ); - expect(result).toHaveLength(1); - const parsed = JSON.parse(result[0].text); + expect(result.content).toHaveLength(1); + // No-op is a successful outcome, not a failed call + expect(result.isError).toBeUndefined(); + const parsed = JSON.parse(result.content[0].text); expect(parsed.status).toBe('NO_CHANGE'); expect(parsed.message).toContain('activate_site_plugins_v1'); expect(parsed.details.code).toBe('already_active'); @@ -1237,7 +1267,7 @@ describe('no-op error handling for idempotent tools', () => { const result = await executeTool(baseConfig, 'simple_action_v1', { id: 1 }, mockLogger); // Should surface as a normal error, not NO_CHANGE - const parsed = JSON.parse(result[0].text); + const parsed = JSON.parse(result.content[0].text); expect(parsed.status).toBeUndefined(); expect(parsed.error).toBeDefined(); }); @@ -1266,7 +1296,7 @@ describe('no-op error handling for idempotent tools', () => { ); // Should surface as a normal error, not NO_CHANGE - const parsed = JSON.parse(result[0].text); + const parsed = JSON.parse(result.content[0].text); expect(parsed.status).toBeUndefined(); expect(parsed.error).toBeDefined(); expect(mockLogger.error).toHaveBeenCalledWith('Tool execution failed', expect.anything()); @@ -1295,7 +1325,7 @@ describe('no-op error handling for idempotent tools', () => { mockLogger ); - const parsed = JSON.parse(result[0].text); + const parsed = JSON.parse(result.content[0].text); expect(parsed.status).toBeUndefined(); expect(parsed.error).toBeDefined(); }); @@ -1324,7 +1354,7 @@ describe('no-op error handling for idempotent tools', () => { mockLogger ); - const responseBytes = Buffer.byteLength(result[0].text, 'utf8'); + const responseBytes = Buffer.byteLength(result.content[0].text, 'utf8'); const usage = getSessionDataUsage(baseConfig); expect(usage.used).toBeGreaterThanOrEqual(responseBytes); }); @@ -1521,7 +1551,8 @@ describe('default-deny annotations', () => { const config = { ...baseConfig, safeMode: true }; const result = await executeTool(config, 'mystery_action_v1', { target: 'test' }, mockLogger); - expect(result[0].text).toContain('SAFE_MODE_BLOCKED'); + expect(result.content[0].text).toContain('SAFE_MODE_BLOCKED'); + expect(result.isError).toBe(true); }); it('should log warning about missing annotations', async () => { diff --git a/src/tools.ts b/src/tools.ts index 5c47bde..eb621c5 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -36,6 +36,18 @@ export interface ExecuteToolOptions { signal?: AbortSignal; } +/** + * Result of a tool call, matching the MCP CallToolResult shape. + * `isError: true` marks failed calls (unknown tool, validation failure, + * execution failure, confirmation rejection) so clients can branch on it. + * A type alias (not interface) so it gets an implicit index signature and + * stays assignable to the SDK's CallToolResult handler return type. + */ +export type ToolCallResult = { + content: TextContent[]; + isError?: boolean; +}; + /** * Cached tool list to avoid re-converting abilities on every ListTools call. * Invalidated by abilities array reference change or config fingerprint change. @@ -133,7 +145,7 @@ export async function executeTool( args: Record, logger: Logger, options?: ExecuteToolOptions -): Promise { +): Promise { const startTime = performance.now(); const hasArguments = Object.keys(args).length > 0; const requestId = crypto.randomUUID(); @@ -211,12 +223,15 @@ export async function executeTool( // runaway API responses, not small fixed-size local error messages. if (isDestructive) { reqLogger.warning('Destructive operation blocked by safe mode', { toolName, abilityName }); - return [ - { - type: 'text', - text: formatJson(config, buildSafeModeBlockedResponse(ctx)), - }, - ]; + return { + content: [ + { + type: 'text', + text: formatJson(config, buildSafeModeBlockedResponse(ctx)), + }, + ], + isError: true, + }; } } @@ -235,7 +250,9 @@ export async function executeTool( }); if (confirmResult.action === 'respond') { - return confirmResult.response; + return confirmResult.isError + ? { content: confirmResult.response, isError: true } + : { content: confirmResult.response }; } if (confirmResult.action === 'execute') { effectiveArgs = confirmResult.effectiveArgs; @@ -271,12 +288,14 @@ export async function executeTool( sessionDataBytes: getSessionDataUsage(config).used, }); - return [ - { - type: 'text', - text: formattedResult, - }, - ]; + return { + content: [ + { + type: 'text', + text: formattedResult, + }, + ], + }; } catch (error) { // Idempotent no-op: tool already achieved the desired state (e.g. already_active) if (annotations?.idempotent && isNoOpError(error)) { @@ -297,7 +316,7 @@ export async function executeTool( responseBytes, sessionDataBytes: getSessionDataUsage(config).used, }); - return [{ type: 'text', text: noChangeText }]; + return { content: [{ type: 'text', text: noChangeText }] }; } const durationMs = Math.round(performance.now() - startTime); @@ -309,12 +328,16 @@ export async function executeTool( error: errorMessage, }); - // Use standardized error format with code - return [ - { - type: 'text', - text: formatErrorResponse(error, sanitizeError), - }, - ]; + // Use standardized error format with code; isError marks the call as + // failed per the MCP spec while keeping the JSON error body in content + return { + content: [ + { + type: 'text', + text: formatErrorResponse(error, sanitizeError), + }, + ], + isError: true, + }; } } diff --git a/tests/integration/tools.test.ts b/tests/integration/tools.test.ts index a3a17d6..ae7b255 100644 --- a/tests/integration/tools.test.ts +++ b/tests/integration/tools.test.ts @@ -131,10 +131,11 @@ describe('Tools Integration', () => { mockLogger ); - expect(result).toHaveLength(1); - expect(result[0].type).toBe('text'); + expect(result.content).toHaveLength(1); + expect(result.content[0].type).toBe('text'); + expect(result.isError).toBeUndefined(); - const parsed = JSON.parse(result[0].text); + const parsed = JSON.parse(result.content[0].text); expect(parsed.sites).toHaveLength(1); }); @@ -176,8 +177,9 @@ describe('Tools Integration', () => { mockLogger ); - expect(result[0].text).toContain('SAFE_MODE_BLOCKED'); - expect(result[0].text).toContain('delete_site_v1'); + expect(result.content[0].text).toContain('SAFE_MODE_BLOCKED'); + expect(result.content[0].text).toContain('delete_site_v1'); + expect(result.isError).toBe(true); }); it('should allow read-only tool in safe mode', async () => { @@ -196,7 +198,7 @@ describe('Tools Integration', () => { const safeConfig = { ...baseConfig, safeMode: true }; const result = await executeTool(safeConfig, 'list_sites_v1', {}, mockLogger); - const parsed = JSON.parse(result[0].text); + const parsed = JSON.parse(result.content[0].text); expect(parsed).toHaveProperty('sites'); }); }); @@ -226,9 +228,11 @@ describe('Tools Integration', () => { mockLogger ); - expect(result[0].text).toContain('CONFIRMATION_REQUIRED'); - expect(result[0].text).toContain('preview'); - expect(result[0].text).toContain('user_confirmed'); + expect(result.content[0].text).toContain('CONFIRMATION_REQUIRED'); + expect(result.content[0].text).toContain('preview'); + expect(result.content[0].text).toContain('user_confirmed'); + // Preview is a successful workflow step, not a failed call + expect(result.isError).toBeUndefined(); }); it('should complete two-phase confirmation flow', async () => { @@ -252,7 +256,7 @@ describe('Tools Integration', () => { mockLogger ); - expect(previewResult[0].text).toContain('CONFIRMATION_REQUIRED'); + expect(previewResult.content[0].text).toContain('CONFIRMATION_REQUIRED'); // Phase 2: Confirmed execution (need to clear logger mocks for fresh tracking) vi.mocked(mockLogger.info).mockClear(); @@ -271,7 +275,7 @@ describe('Tools Integration', () => { mockLogger ); - const parsed = JSON.parse(confirmResult[0].text); + const parsed = JSON.parse(confirmResult.content[0].text); expect(parsed.success).toBe(true); }); @@ -290,7 +294,8 @@ describe('Tools Integration', () => { mockLogger ); - expect(result[0].text).toContain('PREVIEW_REQUIRED'); + expect(result.content[0].text).toContain('PREVIEW_REQUIRED'); + expect(result.isError).toBe(true); }); it('should reject conflicting parameters', async () => { @@ -307,7 +312,8 @@ describe('Tools Integration', () => { mockLogger ); - expect(result[0].text).toContain('CONFLICTING_PARAMETERS'); + expect(result.content[0].text).toContain('CONFLICTING_PARAMETERS'); + expect(result.isError).toBe(true); }); it('should allow explicit dry_run to bypass confirmation', async () => { @@ -331,7 +337,7 @@ describe('Tools Integration', () => { ); // Should return the dry_run result directly, not CONFIRMATION_REQUIRED - const parsed = JSON.parse(result[0].text); + const parsed = JSON.parse(result.content[0].text); expect(parsed.dry_run).toBe(true); }); }); @@ -362,7 +368,8 @@ describe('Tools Integration', () => { mockLogger ); - expect(result[0].text).toContain('error'); + expect(result.content[0].text).toContain('error'); + expect(result.isError).toBe(true); expect(mockLogger.error).toHaveBeenCalled(); }); @@ -375,8 +382,9 @@ describe('Tools Integration', () => { const result = await executeTool(baseConfig, 'get_site_v1', { site_id: -1 }, mockLogger); - expect(result[0].text).toContain('error'); - expect(result[0].text).toContain('positive integer'); + expect(result.content[0].text).toContain('error'); + expect(result.content[0].text).toContain('positive integer'); + expect(result.isError).toBe(true); }); it('should handle tool not found', async () => { @@ -388,8 +396,9 @@ describe('Tools Integration', () => { const result = await executeTool(baseConfig, 'nonexistent_tool_v1', {}, mockLogger); - expect(result[0].text).toContain('error'); - expect(result[0].text).toContain('not found'); + expect(result.content[0].text).toContain('error'); + expect(result.content[0].text).toContain('not found'); + expect(result.isError).toBe(true); }); }); @@ -480,7 +489,7 @@ describe('Tools Integration', () => { const executeCall = mockFetch.mock.calls[1]; const url = executeCall[0] as string; expect(url).toContain('/abilities/acme/do-thing-v1/run'); - expect(JSON.parse(result[0].text)).toEqual({ result: 'pong' }); + expect(JSON.parse(result.content[0].text)).toEqual({ result: 'pong' }); }); it('round-trips a hyphenated namespace through execute', async () => { From 058f89ed80648d148de7eeaab9019e969ce30204 Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 10 Jun 2026 12:48:41 -0400 Subject: [PATCH 5/9] Warn at startup on namespace misconfiguration A server configured with a namespace allowlist that excludes 'mainwp' (e.g. MAINWP_ABILITY_NAMESPACES="acme") booted clean, advertised zero tools, and broke the built-in resources with nothing in the logs explaining why. Two warnings now cover this: - Startup warns when 'mainwp' is not in abilityNamespaces, since the mainwp://site/{id} resource calls mainwp/get-site-v1 and site ID prompt completions call mainwp/list-sites-v1, both of which fail when the namespace is filtered out (matches the 'Keep mainwp in the list' section in docs/configuration.md). - fetchAbilities warns when the namespace filter leaves zero abilities, naming the configured namespaces and the pre-filter count. Verified live: booting with MAINWP_ABILITY_NAMESPACES="acme" against the testbed dashboard logs both warnings on stderr. --- src/abilities.test.ts | 36 ++++++++++++++++++++++++++++++++++++ src/abilities.ts | 10 ++++++++++ src/index.ts | 14 ++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/src/abilities.test.ts b/src/abilities.test.ts index 3219cdc..8918a6f 100644 --- a/src/abilities.test.ts +++ b/src/abilities.test.ts @@ -306,6 +306,42 @@ describe('fetchAbilities', () => { expect(abilities.map(a => a.name)).not.toContain('other/skip-me-v1'); }); + it('warns when the namespace filter leaves zero abilities', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + + const acmeOnlyConfig: Config = { ...baseConfig, abilityNamespaces: ['acme'] }; + const abilities = await fetchAbilities(acmeOnlyConfig, false, mockLogger); + + expect(abilities).toHaveLength(0); + expect(mockLogger.warning).toHaveBeenCalledWith( + 'No abilities matched the configured namespaces', + expect.objectContaining({ + namespaces: ['acme'], + fetchedCount: sampleAbilities.length, + }) + ); + }); + + it('does not warn about empty namespace match when abilities are found', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => sampleAbilities, + headers: new Headers(), + }); + + const abilities = await fetchAbilities(baseConfig, false, mockLogger); + + expect(abilities.length).toBeGreaterThan(0); + expect(mockLogger.warning).not.toHaveBeenCalledWith( + 'No abilities matched the configured namespaces', + expect.anything() + ); + }); + it('refreshes cache when abilityNamespaces changes between calls', async () => { mockFetch.mockResolvedValue({ ok: true, diff --git a/src/abilities.ts b/src/abilities.ts index 78111e4..f870a59 100644 --- a/src/abilities.ts +++ b/src/abilities.ts @@ -202,6 +202,16 @@ export async function fetchAbilities( return true; }); + // A misconfigured namespace allowlist boots a server that advertises + // zero tools with no other symptom — warn loudly so the cause is in the + // logs instead of leaving a silently empty server. + if (newAbilities.length === 0) { + logger?.warning('No abilities matched the configured namespaces', { + namespaces, + fetchedCount: allAbilities.length, + }); + } + // Check if abilities have changed (compare names) const oldNames = cachedAbilities diff --git a/src/index.ts b/src/index.ts index 09c015f..8cfc13d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -512,6 +512,20 @@ async function main(): Promise { startupLogger.error('╚══════════════════════════════════════════════════════════════╝'); } + // The built-in mainwp://site/{id} resource calls mainwp/get-site-v1 and + // site ID prompt completions call mainwp/list-sites-v1. Without 'mainwp' + // in the allowlist those abilities are filtered out, so warn up front + // (see docs/configuration.md, "Keep mainwp in the list"). + if (!config.abilityNamespaces.includes('mainwp')) { + startupLogger.warning( + "Namespace allowlist does not include 'mainwp'. The mainwp://site/{id} resource calls " + + 'mainwp/get-site-v1 and site ID prompt completions call mainwp/list-sites-v1; with ' + + "'mainwp' filtered out, the resource returns an error payload and completions come " + + "back empty. Add 'mainwp' alongside other namespaces rather than replacing it.", + { abilityNamespaces: config.abilityNamespaces } + ); + } + // Validate credentials with fail-fast behavior startupLogger.info('Validating credentials...'); const abilities = await validateCredentials(config, startupLogger); From 7e26c98506869d8a763022b180218806d61b1223 Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 10 Jun 2026 12:51:18 -0400 Subject: [PATCH 6/9] Distinguish empty upstream from namespace filter mismatch CodeRabbit pointed out the zero-abilities warning claimed a namespace mismatch even when the dashboard itself returned no abilities. An empty upstream now logs 'Dashboard returned no abilities' instead. --- src/abilities.test.ts | 20 ++++++++++++++++++++ src/abilities.ts | 7 +++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/abilities.test.ts b/src/abilities.test.ts index 8918a6f..711ce7a 100644 --- a/src/abilities.test.ts +++ b/src/abilities.test.ts @@ -326,6 +326,26 @@ describe('fetchAbilities', () => { ); }); + it('warns with a distinct message when the upstream returns no abilities', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => [], + headers: new Headers(), + }); + + const abilities = await fetchAbilities(baseConfig, false, mockLogger); + + expect(abilities).toHaveLength(0); + expect(mockLogger.warning).toHaveBeenCalledWith( + 'Dashboard returned no abilities', + expect.objectContaining({ namespaces: ['mainwp'] }) + ); + expect(mockLogger.warning).not.toHaveBeenCalledWith( + 'No abilities matched the configured namespaces', + expect.anything() + ); + }); + it('does not warn about empty namespace match when abilities are found', async () => { mockFetch.mockResolvedValueOnce({ ok: true, diff --git a/src/abilities.ts b/src/abilities.ts index f870a59..b7aa408 100644 --- a/src/abilities.ts +++ b/src/abilities.ts @@ -204,8 +204,11 @@ export async function fetchAbilities( // A misconfigured namespace allowlist boots a server that advertises // zero tools with no other symptom — warn loudly so the cause is in the - // logs instead of leaving a silently empty server. - if (newAbilities.length === 0) { + // logs instead of leaving a silently empty server. An empty upstream is + // a different problem, so it gets its own message. + if (allAbilities.length === 0) { + logger?.warning('Dashboard returned no abilities', { namespaces }); + } else if (newAbilities.length === 0) { logger?.warning('No abilities matched the configured namespaces', { namespaces, fetchedCount: allAbilities.length, From 738dd81ab82fdbfce26d3d3a1efb1bf8941cee72 Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 10 Jun 2026 12:53:56 -0400 Subject: [PATCH 7/9] Use approximate tool counts in docs Tool counts had drifted: README said both 'Over 60 tools' and '64 tools', docs/configuration.md said 64, and the live count varies by dashboard version (the testbed currently exposes 62). Both docs now use the same 'around 60 tools' phrasing with a note that the exact count varies by Dashboard version, so the docs stop going stale on every dashboard release. --- README.md | 4 ++-- docs/configuration.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 5df16fb..b75d4b8 100644 --- a/README.md +++ b/README.md @@ -454,7 +454,7 @@ Configuration loads from `./settings.json` or `~/.config/mainwp-mcp/settings.jso ## Optimizing Token Usage -You have access to 64 tools, which consume approximately 28,000 tokens in your AI's context window. Two settings help reduce this footprint. +You have access to around 60 tools (the exact count varies by Dashboard version), which consume approximately 28,000 tokens in your AI's context window. Two settings help reduce this footprint. ### Compact Schema Mode @@ -667,7 +667,7 @@ See the [Security Guide](docs/security.md#confirmation-guardrails) for more deta ## Tools -Over 60 tools organized by category. Each tool shows parameters with type, requirement, and description. +Around 60 tools organized by category (the exact count varies by Dashboard version). Each tool shows parameters with type, requirement, and description. > **Note:** Tool names omit the `mainwp` namespace. The ability `mainwp/list-sites-v1` becomes `list_sites_v1`. diff --git a/docs/configuration.md b/docs/configuration.md index 7d7e1b3..85883c7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -78,7 +78,7 @@ A JSON schema is available at `settings.schema.json` for IDE autocompletion. Control which tools are exposed to AI assistants. Useful for limiting access to read-only operations, hiding destructive tools in production, or reducing context size for the AI. -The server exposes 64 tools by default, consuming approximately 28,000 tokens. Tool filtering can reduce this significantly while limiting the AI to specific capabilities. +The server exposes around 60 tools by default (the exact count varies by Dashboard version), consuming approximately 28,000 tokens. Tool filtering can reduce this significantly while limiting the AI to specific capabilities. ### Whitelist Mode From 268d3ef7ab3a5d8afedbfb78bc12ce9e58ded71c Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 10 Jun 2026 20:43:36 -0400 Subject: [PATCH 8/9] Extend beta.3 changelog with isError fix and misconfiguration warnings The entry covered only the namespace work. The combined PR ships the isError flag on failed tool calls and the startup warnings in the same release, so the changelog now mentions both. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b612a8e..8d7bcd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ Format follows [Keep a Changelog](https://keepachangelog.com/). Ability namespaces are now configurable. The server used to surface only `mainwp/` abilities; the new `abilityNamespaces` setting (or the `MAINWP_ABILITY_NAMESPACES` environment variable) lets you expose abilities that third-party MainWP extensions register through the WordPress Abilities API. The first namespace in the list is the primary one and its tools keep their plain names, so `mainwp/list-sites-v1` still appears as `list_sites_v1`. Abilities from other namespaces carry a prefix: `acme/do-thing-v1` becomes `acme__do_thing_v1`. Hyphenated namespaces such as `acme-corp` work end to end, including execution. Built-in resources and prompt completions depend on `mainwp/get-site-v1` and `mainwp/list-sites-v1`, so keep `mainwp` in the list when adding others. +The server now warns at startup when `mainwp` is missing from `abilityNamespaces`, and after fetching abilities when the namespace filter matches none of them. A misconfigured allowlist used to boot a server that advertised zero tools with nothing in the logs explaining why. An empty upstream gets its own message so a dead API is distinguishable from a filter mismatch. + ### Removed The `toolNameToAbilityName` export is gone. Tool names stopped being uniquely decodable once multiple namespaces came into play, so reverse lookup now goes through an index built when abilities are fetched. Anything importing that function from this package needs to switch to `getAbilityByToolName`. @@ -17,6 +19,8 @@ The `toolNameToAbilityName` export is gone. Tool names stopped being uniquely de A failed ability refresh can no longer leave the tool index half built. The cache swaps in atomically, and if the fetch hits duplicate tool names the previous index keeps serving while the error is reported. Abilities with malformed names, an extra slash for example, are dropped at fetch time with a logged warning rather than surfacing as invalid MCP tool names. +Failed tool calls now set `isError: true` on the MCP result. The error JSON was already in the response content but the flag was missing, so clients that branch on it treated failures as successes. Unknown tools, input validation failures, ability execution failures, cancellations, safe mode blocks, and confirmation rejections all carry the flag; confirmation previews and idempotent no-change responses stay ordinary results. The JSON error bodies are unchanged, so anything parsing them keeps working. + ## [1.0.0-beta.2] - 2026-03-26 ### Changed From 7c8f63c2029d8879913bd2ecfc62f49a362abfb1 Mon Sep 17 00:00:00 2001 From: Dennis Dornon Date: Wed, 10 Jun 2026 20:48:48 -0400 Subject: [PATCH 9/9] Assert success results in namespace round-trip tests CodeRabbit flagged that both namespace integration tests dropped the execution result. They now assert the parsed body and the absence of isError. The suggested input schemas were skipped: the server does not validate arguments against ability schemas at execute time, and schema-less abilities are a supported case. --- tests/integration/tools.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/integration/tools.test.ts b/tests/integration/tools.test.ts index ae7b255..f979df1 100644 --- a/tests/integration/tools.test.ts +++ b/tests/integration/tools.test.ts @@ -490,6 +490,7 @@ describe('Tools Integration', () => { const url = executeCall[0] as string; expect(url).toContain('/abilities/acme/do-thing-v1/run'); expect(JSON.parse(result.content[0].text)).toEqual({ result: 'pong' }); + expect(result.isError).toBeUndefined(); }); it('round-trips a hyphenated namespace through execute', async () => { @@ -519,11 +520,13 @@ describe('Tools Integration', () => { abilityNamespaces: ['mainwp', 'acme-corp'] as [string, ...string[]], }; - await executeTool(config, 'acme_corp__do_thing_v1', {}, mockLogger); + const result = await executeTool(config, 'acme_corp__do_thing_v1', {}, mockLogger); const executeCall = mockFetch.mock.calls[1]; const url = executeCall[0] as string; expect(url).toContain('/abilities/acme-corp/do-thing-v1/run'); + expect(JSON.parse(result.content[0].text)).toEqual({ ok: true }); + expect(result.isError).toBeUndefined(); }); }); });