diff --git a/src/tools/cache.ts b/src/tools/cache.ts index 50fa162..4314ba9 100644 --- a/src/tools/cache.ts +++ b/src/tools/cache.ts @@ -21,55 +21,42 @@ export class CacheTools { { name: "wp_cache_stats", description: "Get cache statistics for a WordPress site.", - parameters: [ - { - name: "site", - type: "string", - description: - "Site ID to get cache stats for. If not provided, uses default site or fails if multiple sites configured.", - }, - ], + inputSchema: { + type: "object" as const, + properties: {}, + }, handler: this.handleGetCacheStats.bind(this), }, { name: "wp_cache_clear", description: "Clear cache for a WordPress site.", - parameters: [ - { - name: "site", - type: "string", - description: "Site ID to clear cache for.", + inputSchema: { + type: "object" as const, + properties: { + pattern: { + type: "string", + description: 'Optional pattern to clear specific cache entries (e.g., "posts", "categories").', + }, }, - { - name: "pattern", - type: "string", - description: 'Optional pattern to clear specific cache entries (e.g., "posts", "categories").', - }, - ], + }, handler: this.handleClearCache.bind(this), }, { name: "wp_cache_warm", description: "Pre-warm cache with essential WordPress data.", - parameters: [ - { - name: "site", - type: "string", - description: "Site ID to warm cache for.", - }, - ], + inputSchema: { + type: "object" as const, + properties: {}, + }, handler: this.handleWarmCache.bind(this), }, { name: "wp_cache_info", description: "Get detailed cache configuration and status information.", - parameters: [ - { - name: "site", - type: "string", - description: "Site ID to get cache info for.", - }, - ], + inputSchema: { + type: "object" as const, + properties: {}, + }, handler: this.handleGetCacheInfo.bind(this), }, ]; @@ -78,10 +65,8 @@ export class CacheTools { /** * Get cache statistics */ - async handleGetCacheStats(params: { site?: string }) { + async handleGetCacheStats(client: WordPressClient, _params: Record) { return toolWrapper(async () => { - const client = this.resolveClient(params.site); - if (!(client instanceof CachedWordPressClient)) { return { caching_enabled: false, @@ -112,10 +97,8 @@ export class CacheTools { /** * Clear cache */ - async handleClearCache(params: { site?: string; pattern?: string }) { + async handleClearCache(client: WordPressClient, params: Record) { return toolWrapper(async () => { - const client = this.resolveClient(params.site); - if (!(client instanceof CachedWordPressClient)) { return { success: false, @@ -124,14 +107,15 @@ export class CacheTools { } let cleared: number; + const pattern = params.pattern as string | undefined; - if (params.pattern) { - cleared = client.clearCachePattern(params.pattern); + if (pattern) { + cleared = client.clearCachePattern(pattern); return { success: true, - message: `Cleared ${cleared} cache entries matching pattern "${params.pattern}".`, + message: `Cleared ${cleared} cache entries matching pattern "${pattern}".`, cleared_entries: cleared, - pattern: params.pattern, + pattern, }; } else { cleared = client.clearCache(); @@ -147,10 +131,8 @@ export class CacheTools { /** * Warm cache with essential data */ - async handleWarmCache(params: { site?: string }) { + async handleWarmCache(client: WordPressClient, _params: Record) { return toolWrapper(async () => { - const client = this.resolveClient(params.site); - if (!(client instanceof CachedWordPressClient)) { return { success: false, @@ -174,10 +156,8 @@ export class CacheTools { /** * Get detailed cache information */ - async handleGetCacheInfo(params: { site?: string }) { + async handleGetCacheInfo(client: WordPressClient, _params: Record) { return toolWrapper(async () => { - const client = this.resolveClient(params.site); - if (!(client instanceof CachedWordPressClient)) { return { caching_enabled: false, @@ -224,32 +204,6 @@ export class CacheTools { }; }); } - - /** - * Resolve client from site parameter - */ - private resolveClient(siteId?: string): WordPressClient { - if (!siteId) { - if (this.clients.size === 1) { - return Array.from(this.clients.values())[0]; - } else if (this.clients.size === 0) { - throw new Error("No WordPress sites configured."); - } else { - throw new Error( - `Multiple sites configured. Please specify --site parameter. Available sites: ${Array.from( - this.clients.keys(), - ).join(", ")}`, - ); - } - } - - const client = this.clients.get(siteId); - if (!client) { - throw new Error(`Site "${siteId}" not found. Available sites: ${Array.from(this.clients.keys()).join(", ")}`); - } - - return client; - } } export default CacheTools; diff --git a/src/tools/pages.ts b/src/tools/pages.ts index 2f0cb23..bc44f54 100644 --- a/src/tools/pages.ts +++ b/src/tools/pages.ts @@ -212,8 +212,21 @@ export class PageTools { public async handleDeletePage(client: WordPressClient, params: Record): Promise { const { id, force } = params as { id: number; force?: boolean }; try { - await client.deletePage(id, force); - const action = params.force ? "permanently deleted" : "moved to trash"; + const result = await client.deletePage(id, force); + const action = force ? "permanently deleted" : "moved to trash"; + + if (result?.deleted === false) { + throw new Error( + `WordPress refused to delete page ${id}. The page may be protected or the operation was rejected.`, + ); + } + + if (result?.deleted) { + const title = result.previous?.title?.rendered; + return title ? `✅ Page "${title}" has been ${action}.` : `✅ Page ${id} has been ${action}.`; + } + + // Some WordPress installations return empty/null responses on successful deletion return `✅ Page ${id} has been ${action}.`; } catch (_error) { throw new Error(`Failed to delete page: ${getErrorMessage(_error)}`); diff --git a/src/tools/performance/PerformanceTools.ts b/src/tools/performance/PerformanceTools.ts index 86e131b..3bc8315 100644 --- a/src/tools/performance/PerformanceTools.ts +++ b/src/tools/performance/PerformanceTools.ts @@ -99,7 +99,10 @@ export default class PerformanceTools { return [ { name: "wp_performance_stats", - description: "Get real-time performance statistics and metrics", + description: + "Get real-time performance statistics and metrics. " + + "Note: Top-level metrics (totalRequests, averageResponseTime, errorRate) are session-wide aggregates across all sites. " + + "Per-site cache and client stats are shown in the siteSpecific section when a site parameter is provided.", parameters: [ { name: "site", @@ -156,7 +159,9 @@ export default class PerformanceTools { }, { name: "wp_performance_benchmark", - description: "Compare current performance against industry benchmarks", + description: + "Compare current performance against industry benchmarks. " + + "Note: Benchmarks are based on session-wide aggregated metrics across all sites, not per-site metrics.", parameters: [ { name: "site", @@ -318,6 +323,7 @@ export default class PerformanceTools { if (category === "overview" || category === "all") { result.overview = { + scope: site ? "session-wide (all sites combined)" : "session-wide", overallHealth: calculateHealthStatus(metrics), performanceScore: calculatePerformanceScore(metrics), totalRequests: metrics.requests.total, diff --git a/src/tools/seo/auditors/SiteAuditor.ts b/src/tools/seo/auditors/SiteAuditor.ts index 0696ab3..26b0738 100644 --- a/src/tools/seo/auditors/SiteAuditor.ts +++ b/src/tools/seo/auditors/SiteAuditor.ts @@ -237,8 +237,8 @@ export class SiteAuditor { const posts = await this.client.getPosts({ per_page: this.config.maxPagesForContentAudit, status: ["publish"] }); const pages = await this.client.getPages({ per_page: this.config.maxPagesForContentAudit, status: ["publish"] }); - // Get site info (mock for now) - const siteUrl = "https://example.com"; // Would come from WordPress REST API + // Get site URL from the WordPress client configuration + const siteUrl = this.client.getSiteUrl(); return { siteUrl, @@ -599,9 +599,22 @@ export class SiteAuditor { } // Check for external dependencies (basic analysis) + let siteHostname: string; + try { + siteHostname = new URL(siteData.siteUrl).hostname; + } catch { + siteHostname = siteData.siteUrl.replace(/^https?:\/\//, "").replace(/[/:].*/g, ""); + } const externalDependencies = [...siteData.posts, ...siteData.pages].reduce((count, item) => { const content = item.content?.rendered || ""; - const externalLinks = content.match(/https?:\/\/(?!example\.com)[^"'\s>]*/gi) || []; + const externalLinks = + content.match(/https?:\/\/[^"'\s>]*/gi)?.filter((url) => { + try { + return new URL(url).hostname !== siteHostname; + } catch { + return true; + } + }) || []; return count + externalLinks.length; }, 0); diff --git a/src/tools/site.ts b/src/tools/site.ts index b5cc3ac..6001818 100644 --- a/src/tools/site.ts +++ b/src/tools/site.ts @@ -21,7 +21,8 @@ export class SiteTools { return [ { name: "wp_get_site_settings", - description: "Retrieves the general settings for a WordPress site.", + description: + "Retrieves the general settings for a WordPress site. Requires administrator role (manage_options capability).", inputSchema: { type: "object", properties: {}, @@ -30,7 +31,8 @@ export class SiteTools { }, { name: "wp_update_site_settings", - description: "Updates one or more general settings for a WordPress site.", + description: + "Updates one or more general settings for a WordPress site. Requires administrator role (manage_options capability).", inputSchema: { type: "object", properties: { diff --git a/src/tools/users.ts b/src/tools/users.ts index 12b0787..1cbe741 100644 --- a/src/tools/users.ts +++ b/src/tools/users.ts @@ -25,6 +25,8 @@ export class UserTools { name: "wp_list_users", description: "Lists users from a WordPress site with comprehensive filtering and detailed user information including roles, registration dates, and activity status.\n\n" + + "**Note:** Role, email, and registration date fields require **administrator** privileges. " + + "Non-admin users will see limited metadata due to WordPress REST API restrictions.\n\n" + "**Usage Examples:**\n" + "• List all users: `wp_list_users`\n" + '• Search users: `wp_list_users --search="john"`\n' + @@ -206,16 +208,16 @@ export class UserTools { month: "short", day: "numeric", }) - : "Unknown"; + : "Restricted (requires admin)"; - const roles = u.roles?.join(", ") || "No roles"; + const roles = u.roles?.length ? u.roles.join(", ") : "Restricted (requires admin)"; const description = u.description || "No description"; const displayName = u.name || "No display name"; const userUrl = u.url || "No URL"; return ( `- **ID ${u.id}**: ${displayName} (@${u.slug})\n` + - ` 📧 Email: ${u.email || "No email"}\n` + + ` 📧 Email: ${u.email || "Restricted (requires admin)"}\n` + ` 🎭 Roles: ${roles}\n` + ` 📅 Registered: ${registrationDate}\n` + ` 🔗 URL: ${userUrl}\n` + diff --git a/tests/tools/cache.test.js b/tests/tools/cache.test.js index 0a0f3b8..3119f86 100644 --- a/tests/tools/cache.test.js +++ b/tests/tools/cache.test.js @@ -10,7 +10,7 @@ describe("CacheTools", () => { beforeEach(() => { vi.clearAllMocks(); - // Mock regular client + // Mock regular client (no caching) mockClient = { cacheManager: { getStats: vi.fn(), @@ -56,7 +56,7 @@ describe("CacheTools", () => { tools.forEach((tool) => { expect(tool).toHaveProperty("name"); expect(tool).toHaveProperty("description"); - expect(tool).toHaveProperty("parameters"); + expect(tool).toHaveProperty("inputSchema"); expect(tool).toHaveProperty("handler"); expect(typeof tool.handler).toBe("function"); }); @@ -84,7 +84,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const statsTool = tools.find((t) => t.name === "wp_cache_stats"); - const result = await statsTool.handler({ site: "cached" }); + const result = await statsTool.handler(mockCachedClient, {}); expect(result.caching_enabled).toBe(true); expect(result.cache_stats).toEqual({ @@ -104,7 +104,7 @@ describe("CacheTools", () => { it("should return disabled message when caching is disabled", async () => { const tools = cacheTools.getTools(); const statsTool = tools.find((t) => t.name === "wp_cache_stats"); - const result = await statsTool.handler({ site: "default" }); + const result = await statsTool.handler(mockClient, {}); expect(result.caching_enabled).toBe(false); expect(result.message).toContain("Caching is disabled for this site"); @@ -118,7 +118,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const statsTool = tools.find((t) => t.name === "wp_cache_stats"); - await expect(statsTool.handler({ site: "cached" })).rejects.toThrow("Failed to get stats"); + await expect(statsTool.handler(mockCachedClient, {})).rejects.toThrow("Failed to get stats"); }); }); @@ -128,7 +128,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const clearTool = tools.find((t) => t.name === "wp_cache_clear"); - const result = await clearTool.handler({ site: "cached" }); + const result = await clearTool.handler(mockCachedClient, {}); expect(result.success).toBe(true); expect(result.message).toContain("Cleared all cache entries (75 total)"); @@ -141,7 +141,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const clearTool = tools.find((t) => t.name === "wp_cache_clear"); - const result = await clearTool.handler({ site: "cached", pattern: "posts" }); + const result = await clearTool.handler(mockCachedClient, { pattern: "posts" }); expect(result.success).toBe(true); expect(result.message).toContain('Cleared 25 cache entries matching pattern "posts"'); @@ -153,7 +153,7 @@ describe("CacheTools", () => { it("should return disabled message when caching is disabled", async () => { const tools = cacheTools.getTools(); const clearTool = tools.find((t) => t.name === "wp_cache_clear"); - const result = await clearTool.handler({ site: "default" }); + const result = await clearTool.handler(mockClient, {}); expect(result.success).toBe(false); expect(result.message).toContain("Caching is not enabled for this site"); @@ -167,7 +167,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const clearTool = tools.find((t) => t.name === "wp_cache_clear"); - await expect(clearTool.handler({ site: "cached" })).rejects.toThrow("Clear failed"); + await expect(clearTool.handler(mockCachedClient, {})).rejects.toThrow("Clear failed"); }); }); @@ -184,7 +184,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const warmTool = tools.find((t) => t.name === "wp_cache_warm"); - const result = await warmTool.handler({ site: "cached" }); + const result = await warmTool.handler(mockCachedClient, {}); expect(result.success).toBe(true); expect(result.message).toContain("Cache warmed with essential WordPress data"); @@ -196,7 +196,7 @@ describe("CacheTools", () => { it("should return disabled message when caching is disabled", async () => { const tools = cacheTools.getTools(); const warmTool = tools.find((t) => t.name === "wp_cache_warm"); - const result = await warmTool.handler({ site: "default" }); + const result = await warmTool.handler(mockClient, {}); expect(result.success).toBe(false); expect(result.message).toContain("Caching is not enabled for this site"); @@ -208,7 +208,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const warmTool = tools.find((t) => t.name === "wp_cache_warm"); - await expect(warmTool.handler({ site: "cached" })).rejects.toThrow("Warm failed"); + await expect(warmTool.handler(mockCachedClient, {})).rejects.toThrow("Warm failed"); }); }); @@ -233,7 +233,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const infoTool = tools.find((t) => t.name === "wp_cache_info"); - const result = await infoTool.handler({ site: "cached" }); + const result = await infoTool.handler(mockCachedClient, {}); expect(result.caching_enabled).toBe(true); expect(result.cache_configuration).toBeDefined(); @@ -256,7 +256,7 @@ describe("CacheTools", () => { it("should return disabled message when caching is disabled", async () => { const tools = cacheTools.getTools(); const infoTool = tools.find((t) => t.name === "wp_cache_info"); - const result = await infoTool.handler({ site: "default" }); + const result = await infoTool.handler(mockClient, {}); expect(result.caching_enabled).toBe(false); expect(result.message).toContain("Caching is disabled for this site"); @@ -271,44 +271,7 @@ describe("CacheTools", () => { const tools = cacheTools.getTools(); const infoTool = tools.find((t) => t.name === "wp_cache_info"); - await expect(infoTool.handler({ site: "cached" })).rejects.toThrow("Info failed"); - }); - }); - - describe("parameter validation", () => { - it("should have proper parameter definitions", () => { - const tools = cacheTools.getTools(); - - tools.forEach((tool) => { - expect(tool.parameters).toBeDefined(); - expect(Array.isArray(tool.parameters)).toBe(true); - - tool.parameters.forEach((param) => { - expect(param).toHaveProperty("name"); - expect(param).toHaveProperty("type"); - expect(param).toHaveProperty("description"); - expect(typeof param.name).toBe("string"); - expect(typeof param.type).toBe("string"); - expect(typeof param.description).toBe("string"); - }); - }); - }); - }); - - describe("site resolution", () => { - it("should handle invalid site gracefully", async () => { - const tools = cacheTools.getTools(); - const statsTool = tools.find((t) => t.name === "wp_cache_stats"); - - await expect(statsTool.handler({ site: "nonexistent" })).rejects.toThrow('Site "nonexistent" not found'); - }); - - it("should handle multiple sites configuration", async () => { - const tools = cacheTools.getTools(); - const statsTool = tools.find((t) => t.name === "wp_cache_stats"); - - // Should fail when no site is specified with multiple sites - await expect(statsTool.handler({})).rejects.toThrow("Multiple sites configured. Please specify --site parameter"); + await expect(infoTool.handler(mockCachedClient, {})).rejects.toThrow("Info failed"); }); }); }); diff --git a/tests/tools/pages.test.js b/tests/tools/pages.test.js index ae698b3..cf62c27 100644 --- a/tests/tools/pages.test.js +++ b/tests/tools/pages.test.js @@ -453,7 +453,9 @@ describe("PageTools", () => { expect(mockClient.deletePage).toHaveBeenCalledWith(1, undefined); expect(typeof result).toBe("string"); - expect(result).toContain("✅ Page 1 has been moved to trash"); + expect(result).toContain("✅ Page"); + expect(result).toContain("Deleted Page"); + expect(result).toContain("moved to trash"); }); it("should handle forced deletion", async () => { @@ -473,10 +475,39 @@ describe("PageTools", () => { }); expect(mockClient.deletePage).toHaveBeenCalledWith(1, true); + expect(typeof result).toBe("string"); + expect(result).toContain("✅ Page"); + expect(result).toContain("Permanently Deleted Page"); + expect(result).toContain("permanently deleted"); + }); + + it("should handle deletion with null response (legacy WordPress)", async () => { + mockClient.deletePage.mockResolvedValueOnce(null); + + const result = await pageTools.handleDeletePage(mockClient, { id: 1 }); + + expect(mockClient.deletePage).toHaveBeenCalledWith(1, undefined); + expect(typeof result).toBe("string"); + expect(result).toContain("✅ Page 1 has been moved to trash"); + }); + + it("should handle deletion with empty object response", async () => { + mockClient.deletePage.mockResolvedValueOnce({}); + + const result = await pageTools.handleDeletePage(mockClient, { id: 1, force: true }); + expect(typeof result).toBe("string"); expect(result).toContain("✅ Page 1 has been permanently deleted"); }); + it("should throw when WordPress refuses deletion (deleted: false)", async () => { + mockClient.deletePage.mockResolvedValueOnce({ deleted: false }); + + await expect(pageTools.handleDeletePage(mockClient, { id: 1 })).rejects.toThrow( + "WordPress refused to delete page 1", + ); + }); + it("should handle deletion errors", async () => { mockClient.deletePage.mockRejectedValueOnce(new Error("Delete failed")); diff --git a/tests/tools/seo/SiteAuditor.test.js b/tests/tools/seo/SiteAuditor.test.js index 74aab92..e3914e4 100644 --- a/tests/tools/seo/SiteAuditor.test.js +++ b/tests/tools/seo/SiteAuditor.test.js @@ -15,6 +15,7 @@ import { SiteAuditor } from "../../../dist/tools/seo/auditors/SiteAuditor.js"; const createMockClient = () => ({ getPosts: vi.fn(), getPages: vi.fn(), + getSiteUrl: vi.fn().mockReturnValue("https://example.com"), authenticate: vi.fn().mockResolvedValue(true), }); @@ -462,6 +463,46 @@ describe("SiteAuditor", () => { }); }); + describe("External Link Detection", () => { + it("should correctly distinguish internal from external links using hostname comparison", async () => { + // Set up content with internal links, external links, and tricky subdomain cases + const postsWithLinks = [ + { + id: 1, + title: { rendered: "Link Test Post" }, + content: { + rendered: ` + Internal + Internal path + External (substring match trap) + External + External subdomain + `, + }, + excerpt: { rendered: "Test" }, + slug: "link-test", + status: "publish", + date: "2024-01-01T00:00:00", + modified: "2024-01-01T00:00:00", + link: "https://example.com/link-test", + }, + ]; + + mockClient.getPosts.mockResolvedValue(postsWithLinks); + mockClient.getPages.mockResolvedValue([]); + + const params = { site: "test" }; + const auditResult = await siteAuditor.performSiteAudit(params); + + // The audit should complete without errors and include performance section + expect(auditResult).toBeDefined(); + expect(Array.isArray(auditResult.sections)).toBe(true); + + const perfSection = auditResult.sections.find((s) => s.name.toLowerCase().includes("performance")); + expect(perfSection).toBeDefined(); + }); + }); + describe("Content Analysis", () => { it("should analyze content readability correctly", async () => { const params = { site: "test" }; diff --git a/tests/tools/users.test.js b/tests/tools/users.test.js index aa7e884..9c727ba 100644 --- a/tests/tools/users.test.js +++ b/tests/tools/users.test.js @@ -213,9 +213,7 @@ describe("UserTools", () => { const result = await userTools.handleListUsers(mockClient, {}); expect(result).toContain("No display name"); - expect(result).toContain("No email"); - expect(result).toContain("No roles"); - expect(result).toContain("Unknown"); + expect(result).toContain("Restricted (requires admin)"); expect(result).toContain("No description"); expect(result).toContain("No URL"); });