Skip to content

fix: resolve multi-site bugs from DXT v3.1.12 testing#172

Merged
docdyhr merged 2 commits intomainfrom
fix/multi-site-bug-fixes
Mar 16, 2026
Merged

fix: resolve multi-site bugs from DXT v3.1.12 testing#172
docdyhr merged 2 commits intomainfrom
fix/multi-site-bug-fixes

Conversation

@docdyhr
Copy link
Owner

@docdyhr docdyhr commented Mar 16, 2026

Summary

Fixes 5 bugs identified during comprehensive functional testing of the mcp-wordpress.dxt v3.1.12 Claude Desktop extension across 3 WordPress sites (dyhr.com, neigong.net, qgg.dk).

  • BUG-1 (High): wp_cache_clear completely broken in multi-site — handler signature mismatch caused site parameter to be ignored. Migrated cache tools to modern (client, params) pattern.
  • BUG-2 (High): wp_seo_site_audit returned identical results for all sites — hardcoded "https://example.com" placeholder replaced with this.client.getSiteUrl().
  • BUG-3 (Medium): wp_performance_stats metrics misleadingly identical across sites — clarified as session-wide aggregates with explicit scope labeling.
  • BUG-4 (Low): wp_list_users showed "No roles"/"No email" — improved to "Restricted (requires admin)" with permission note in tool description.
  • BUG-5 (Low): wp_delete_page silent response on some sites — now validates result.deleted and uses page title when available.
  • Docs: Added admin role requirement to wp_get_site_settings and wp_update_site_settings descriptions.

Test plan

  • All 68 test files pass (2092 tests, 0 failures)
  • Build succeeds cleanly
  • Pre-commit hooks pass (lint, typecheck, security tests)
  • Pre-push hooks pass (full build + security audit)
  • Re-test with Claude Desktop DXT extension on all 3 sites
  • Verify wp_cache_clear --site=site1 works in multi-site mode
  • Verify wp_seo_site_audit returns distinct URLs per site

🤖 Generated with Claude Code

Summary by Sourcery

Align WordPress cache, SEO audit, performance, user listing, page deletion, and site settings tools with multi-site behavior and permission expectations uncovered during DXT v3.1.12 testing.

Bug Fixes:

  • Fix cache tools to use the modern client+params handler signature so that multi-site routing works correctly, including for wp_cache_clear.
  • Ensure wp_seo_site_audit uses the actual site URL from the WordPress client instead of a hardcoded placeholder, and correctly distinguishes external links per site.
  • Make wp_delete_page return clear, non-silent responses based on the deletion result, preferring the page title when available.

Enhancements:

  • Clarify wp_performance_stats and wp_performance_benchmark descriptions to explain that top-level metrics are session-wide aggregates rather than per-site values, and expose scope in the overview output.
  • Improve wp_list_users output and description to reflect restricted fields for non-admin users instead of showing misleading placeholders.
  • Document administrator role requirements in wp_get_site_settings and wp_update_site_settings tool descriptions.

Documentation:

  • Document administrator capability requirements for site settings tools and user metadata visibility in tool descriptions.

Tests:

  • Update cache, pages, users, and SEO auditor tests to match the new handler signatures, behaviors, and permission-aware outputs.

- BUG-1: Fix wp_cache_clear ignoring site parameter in multi-site mode
  Migrate cache tools from old (params) handler signature to modern
  (client, params) pattern matching ToolRegistry expectations. Remove
  obsolete resolveClient() — site routing handled by ToolRegistry.

- BUG-2: Fix wp_seo_site_audit returning identical results for all sites
  Replace hardcoded "https://example.com" placeholder with
  this.client.getSiteUrl(). Fix external link regex to use actual site host.

- BUG-3: Clarify wp_performance_stats metrics scope
  Add "session-wide" scope label to overview metrics. Update tool
  descriptions to document that top-level metrics are aggregated across
  all sites, with per-site data in siteSpecific section.

- BUG-4: Fix wp_list_users showing "No roles"/"No email" for all users
  Update fallback text to "Restricted (requires admin)" to indicate
  WordPress REST API field restrictions for non-admin users. Add
  permission note to tool description.

- BUG-5: Improve wp_delete_page response handling
  Validate result.deleted and use page title from result.previous when
  available. Gracefully handle null/empty responses from legacy WordPress.

- Add admin role requirement to wp_get/update_site_settings descriptions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 16, 2026 10:06
@sourcery-ai
Copy link

sourcery-ai bot commented Mar 16, 2026

Reviewer's Guide

Migrates cache tools to the modern (client, params) handler pattern for multi-site correctness, fixes site-specific behavior in SEO and performance tools, improves user-facing messaging and permissions clarity for user and page operations, and updates tests accordingly.

Sequence diagram for multi-site wp_cache_clear with (client, params) pattern

sequenceDiagram
  actor User
  participant ClaudeDesktop
  participant ToolRegistry
  participant WordPressClient
  participant CacheTools

  User->>ClaudeDesktop: Run wp_cache_clear --site=neigong --pattern=posts
  ClaudeDesktop->>ToolRegistry: Resolve tool wp_cache_clear
  ToolRegistry->>ToolRegistry: Select client for site neigong
  ToolRegistry->>CacheTools: handleClearCache(WordPressClient, params)
  activate CacheTools
  CacheTools->>WordPressClient: instance type check (CachedWordPressClient)
  alt client is CachedWordPressClient
    CacheTools->>WordPressClient: clearCachePattern(pattern)
    WordPressClient-->>CacheTools: clearedCount
    CacheTools-->>ToolRegistry: { success: true, message, cleared_entries, pattern }
  else client not CachedWordPressClient
    CacheTools-->>ToolRegistry: { success: false, message: caching disabled }
  end
  deactivate CacheTools
  ToolRegistry-->>ClaudeDesktop: Tool result
  ClaudeDesktop-->>User: Display human-readable cache clear summary
Loading

Class diagram for updated CacheTools multi-site handler pattern

classDiagram
  class CacheTools {
    - clients: Map<string, WordPressClient>
    + getTools(): ToolDefinition[]
    + handleGetCacheStats(client: WordPressClient, params: Record<string, unknown>): Promise<unknown>
    + handleClearCache(client: WordPressClient, params: Record<string, unknown>): Promise<unknown>
    + handleWarmCache(client: WordPressClient, params: Record<string, unknown>): Promise<unknown>
    + handleGetCacheInfo(client: WordPressClient, params: Record<string, unknown>): Promise<unknown>
  }

  class WordPressClient {
    + getSiteUrl(): string
    + clearCache(): number
    + clearCachePattern(pattern: string): number
  }

  class CachedWordPressClient {
    + getSiteUrl(): string
    + clearCache(): number
    + clearCachePattern(pattern: string): number
  }

  class ToolDefinition {
    + name: string
    + description: string
    + inputSchema: object
    + handler: function
  }

  CacheTools --> ToolDefinition : returns
  CachedWordPressClient --|> WordPressClient
  CacheTools ..> WordPressClient : uses
  CacheTools ..> CachedWordPressClient : runtime type check
Loading

Class diagram for updated PageTools.handleDeletePage behavior

classDiagram
  class PageTools {
    + getTools(): ToolDefinition[]
    + handleDeletePage(client: WordPressClient, params: Record<string, unknown>): Promise<unknown>
  }

  class WordPressClient {
    + deletePage(id: number, force: boolean): DeletePageResult
  }

  class DeletePageResult {
    + deleted: boolean
    + previous: DeletedPage
  }

  class DeletedPage {
    + id: number
    + title: PageTitle
  }

  class PageTitle {
    + rendered: string
  }

  PageTools ..> WordPressClient : calls deletePage
  WordPressClient --> DeletePageResult : returns
  DeletePageResult --> DeletedPage : previous
  DeletedPage --> PageTitle : title
Loading

Class diagram for updated SiteAuditor site-specific SEO audit

classDiagram
  class SiteAuditorConfig {
    + maxPagesForContentAudit: number
  }

  class SiteAuditor {
    - client: WordPressClient
    - config: SiteAuditorConfig
    + auditSite() Promise<SiteAuditResult>
    + collectSiteData() Promise<SiteData>
  }

  class WordPressClient {
    + getSiteUrl() string
    + getPosts(options: object) Post[]
    + getPages(options: object) Page[]
  }

  class SiteData {
    + siteUrl: string
    + posts: Post[]
    + pages: Page[]
    + externalDependencies: number
  }

  class Post {
    + content: Content
  }

  class Page {
    + content: Content
  }

  class Content {
    + rendered: string
  }

  SiteAuditor ..> WordPressClient : uses
  SiteAuditor --> SiteData : builds
  SiteData --> Post : posts
  SiteData --> Page : pages
  Post --> Content : content
  Page --> Content : content
Loading

Class diagram for updated PerformanceTools metrics scope

classDiagram
  class PerformanceTools {
    + getTools(): ToolDefinition[]
    + gatherMetrics(site: string): Promise<PerformanceMetrics>
  }

  class PerformanceMetrics {
    + requests: RequestMetrics
    + errors: ErrorMetrics
    + cache: CacheMetrics
  }

  class RequestMetrics {
    + total: number
    + averageResponseTime: number
  }

  class ErrorMetrics {
    + errorRate: number
  }

  class CacheMetrics {
    + hits: number
    + misses: number
  }

  class PerformanceOverviewResult {
    + scope: string
    + overallHealth: string
    + performanceScore: number
    + totalRequests: number
    + averageResponseTime: number
    + errorRate: number
  }

  PerformanceTools --> PerformanceMetrics : builds from session data
  PerformanceTools --> PerformanceOverviewResult : sets scope based on site
Loading

Class diagram for updated UserTools.wp_list_users permissions-aware output

classDiagram
  class UserTools {
    + getTools(): ToolDefinition[]
    + handleListUsers(client: WordPressClient, params: Record<string, unknown>): Promise<string>
  }

  class WordPressClient {
    + listUsers(options: object): User[]
  }

  class User {
    + id: number
    + slug: string
    + name: string
    + email: string
    + roles: string[]
    + description: string
    + url: string
    + registeredDate: string
  }

  UserTools ..> WordPressClient : uses
  WordPressClient --> User : returns list
Loading

Class diagram for updated SiteTools admin-restricted settings tools

classDiagram
  class SiteTools {
    + getTools(): ToolDefinition[]
    + handleGetSiteSettings(client: WordPressClient, params: Record<string, unknown>): Promise<object>
    + handleUpdateSiteSettings(client: WordPressClient, params: Record<string, unknown>): Promise<object>
  }

  class WordPressClient {
    + getSettings(): object
    + updateSettings(settings: object): object
  }

  SiteTools ..> WordPressClient : uses
Loading

File-Level Changes

Change Details Files
Migrate cache tool handlers to (client, params) pattern and remove internal site resolution logic for proper multi-site behavior.
  • Replace legacy parameters arrays with inputSchema objects for cache tools definitions.
  • Update cache handlers to accept a WordPressClient instance directly instead of resolving it from a site parameter.
  • Use pattern from params in wp_cache_clear while keeping behavior when no pattern is provided.
  • Remove internal resolveClient helper and associated error handling for site resolution.
  • Adjust cache tool tests to call handlers with (client, params) and to validate inputSchema instead of parameters, removing tests around site resolution and parameter arrays.
src/tools/cache.ts
tests/tools/cache.test.js
Improve deletion feedback for pages to surface title and handle varying WordPress delete responses.
  • Capture the result of client.deletePage and inspect the deleted flag to determine success.
  • Prefer using the previous.title.rendered field when available to include the page title in the success message.
  • Fallback to ID-based messaging for installations that return empty or null responses on successful deletion.
  • Update page deletion tests to assert the new, more descriptive success messages without relying on strict ID-only text.
src/tools/pages.ts
tests/tools/pages.test.js
Clarify performance metrics scope as session-wide aggregates and label overview output accordingly.
  • Extend wp_performance_stats description to explain that top-level metrics are session-wide aggregates and per-site data lives under siteSpecific when a site is provided.
  • Extend wp_performance_benchmark description to note that benchmarks use session-wide aggregated metrics.
  • Include a scope field in the overview section, distinguishing between session-wide and session-wide (all sites combined) when a site parameter is used.
src/tools/performance/PerformanceTools.ts
Make wp_list_users output and description accurately reflect admin-only visibility of sensitive fields.
  • Augment wp_list_users tool description to document that role, email, and registration date require administrator privileges and that non-admins see limited metadata.
  • Change email, roles, and registration date fallbacks from generic placeholders like No email/No roles/Unknown to Restricted (requires admin).
  • Keep other non-sensitive fields as-is while still providing some basic information for non-admin users.
  • Update user tools tests to expect Restricted (requires admin) messaging instead of the older placeholders.
src/tools/users.ts
tests/tools/users.test.js
Make SEO site audit truly site-specific by using the client’s configured site URL and more robust external link detection.
  • Replace the hardcoded https://example.com site URL in SiteAuditor with this.client.getSiteUrl() from the WordPress client.
  • Compute the site host from the resolved siteUrl and treat any links not containing that host as external when counting external dependencies.
  • Update SEO SiteAuditor tests to mock getSiteUrl on the client so audits run against the configured URL.
src/tools/seo/auditors/SiteAuditor.ts
tests/tools/seo/SiteAuditor.test.js
Document administrator requirements for site settings tools.
  • Update wp_get_site_settings description to call out manage_options administrator capability requirement.
  • Update wp_update_site_settings description similarly to make admin-only behavior explicit.
src/tools/site.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • In SiteAuditor, the new external link detection filters by !url.includes(siteHost), which can misclassify domains that merely contain the site host as a substring (e.g., notexample.com vs example.com); consider parsing the URL host and comparing exact hosts instead.
  • Several handlers now accept params: Record<string, unknown> and then cast (e.g., handleClearCache, handleDeletePage); tightening these to typed parameter objects (or validating shapes before cast) would improve type safety and reduce potential runtime errors from malformed tool input.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `SiteAuditor`, the new external link detection filters by `!url.includes(siteHost)`, which can misclassify domains that merely contain the site host as a substring (e.g., `notexample.com` vs `example.com`); consider parsing the URL host and comparing exact hosts instead.
- Several handlers now accept `params: Record<string, unknown>` and then cast (e.g., `handleClearCache`, `handleDeletePage`); tightening these to typed parameter objects (or validating shapes before cast) would improve type safety and reduce potential runtime errors from malformed tool input.

## Individual Comments

### Comment 1
<location path="tests/tools/pages.test.js" line_range="456-458" />
<code_context>

       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");
     });

</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for deletePage when the API returns an empty/null response but deletion succeeds

The new `handleDeletePage` logic branches on `result?.deleted` and `result.previous`, with a fallback message for installations that return empty or null responses. Current tests cover the `result.deleted === true` + `result.previous.title.rendered` path and the error path, but not the fallback when `deletePage` resolves to `null` or `{}`.

Please add a test that mocks `mockClient.deletePage.mockResolvedValueOnce(null)` (or `{}`), calls `handleDeletePage` with `force` true/false, and asserts the generic success message (`✅ Page 1 has been moved to trash` / `permanently deleted`) is returned. This will lock in the intended behavior for empty/null responses mentioned in the code comment.

Suggested implementation:

```javascript
      expect(mockClient.deletePage).toHaveBeenCalledWith(1, undefined);
      expect(typeof result).toBe("string");
      expect(result).toContain("✅ Page");
      expect(result).toContain("Deleted Page");
      expect(result).toContain("moved to trash");
    });

    it("should handle deletion with empty response", async () => {
      mockClient.deletePage.mockResolvedValueOnce(null);

      const result = await handleDeletePage(1, mockClient);

      expect(mockClient.deletePage).toHaveBeenCalledWith(1, undefined);
      expect(typeof result).toBe("string");
      expect(result).toContain("✅ Page 1 has been moved to trash");
    });

    it("should handle forced deletion", async () => {

```

If the actual `handleDeletePage` signature in your test file differs (for example, if it’s `handleDeletePage(mockClient, 1)` or `handleDeletePage({ client: mockClient, id: 1 })`), update the call in the new test accordingly so that:
- It uses the same argument pattern as the non-forced deletion test above it.
- It still causes `mockClient.deletePage` to be invoked with `(1, undefined)` when `force` is not set.

If you also have a specific generic message for *forced* deletion on empty responses (e.g. “permanently deleted”), you may want to add a second test similar to `should handle deletion with empty response` but calling the forced variant of `handleDeletePage` and asserting the permanent-delete message instead.
</issue_to_address>

### Comment 2
<location path="tests/tools/seo/SiteAuditor.test.js" line_range="18" />
<code_context>
 const createMockClient = () => ({
   getPosts: vi.fn(),
   getPages: vi.fn(),
+  getSiteUrl: vi.fn().mockReturnValue("https://example.com"),
   authenticate: vi.fn().mockResolvedValue(true),
 });
</code_context>
<issue_to_address>
**suggestion (testing):** Extend SiteAuditor tests to cover the new external link detection logic that depends on siteUrl

The new logic derives the host from `siteData.siteUrl` and filters external links by excluding URLs containing that host. While `getSiteUrl` is mocked, there’s no assertion that internal links are ignored and external ones are counted.

Please add/extend a test that:
- Uses `getSiteUrl` to return `https://example.com`.
- Includes content with both `https://example.com/...` and external links like `https://other.com/...`.
- Asserts that `externalDependencies` counts only the truly external domains.

This will specifically exercise the multi-site bug fix and host-based filtering behavior.

Suggested implementation:

```javascript
const createMockClient = () => ({
  getPosts: vi.fn(),
  getPages: vi.fn(),
  getSiteUrl: vi.fn().mockReturnValue("https://example.com"),
  authenticate: vi.fn().mockResolvedValue(true),
});

it("counts only external links based on siteUrl host", async () => {
  const client = createMockClient();

  client.getPages.mockResolvedValue([
    {
      id: 1,
      content: `
        <a href="https://example.com/about">Internal link</a>
        <a href="https://example.com/blog/post">Another internal link</a>
        <a href="https://other.com/page">External link</a>
        <a href="https://another.com/page">Another external</a>
      `,
    },
  ]);

  client.getPosts.mockResolvedValue([]);

  // NOTE: Adjust the invocation below to match the existing SiteAuditor API.
  // The intention is to run the auditor and inspect the externalDependencies
  // in its report/summary.
  const auditor = new SiteAuditor(client);
  const report = await auditor.run();

  // Assert that only truly external domains are counted
  expect(report.externalDependencies).toEqual(
    expect.arrayContaining(["https://other.com", "https://another.com"]),
  );
  expect(report.externalDependencies).not.toEqual(
    expect.arrayContaining([
      "https://example.com",
      "https://example.com/about",
      "https://example.com/blog/post",
    ]),
  );
});

```

The new test assumes:
1. A `SiteAuditor` class (or similar) is available in this test file and can be constructed with `new SiteAuditor(client)` and executed via `await auditor.run()`.
2. The result exposes an `externalDependencies` collection that:
   - Contains external URLs or domains as strings.
   - Excludes links whose host matches `siteUrl` (`https://example.com`).

To integrate with your existing codebase:
- Update `new SiteAuditor(client)` and `auditor.run()` to match the actual API (e.g., `auditSite(client)`, `runSiteAudit({ client })`, or similar).
- If `externalDependencies` is structured differently (e.g., an object keyed by domain, or nested inside a `summary` object), adjust the assertions accordingly, ensuring:
  - Internal URLs containing `example.com` are **not** counted.
  - External URLs like `https://other.com/...` and `https://another.com/...` are counted.
- If deduplication is done at the domain level (e.g., only `other.com` and `another.com`), change the expectations to look for those domains instead of full URLs.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines -456 to 458
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");
});

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add a test for deletePage when the API returns an empty/null response but deletion succeeds

The new handleDeletePage logic branches on result?.deleted and result.previous, with a fallback message for installations that return empty or null responses. Current tests cover the result.deleted === true + result.previous.title.rendered path and the error path, but not the fallback when deletePage resolves to null or {}.

Please add a test that mocks mockClient.deletePage.mockResolvedValueOnce(null) (or {}), calls handleDeletePage with force true/false, and asserts the generic success message (✅ Page 1 has been moved to trash / permanently deleted) is returned. This will lock in the intended behavior for empty/null responses mentioned in the code comment.

Suggested implementation:

      expect(mockClient.deletePage).toHaveBeenCalledWith(1, undefined);
      expect(typeof result).toBe("string");
      expect(result).toContain("✅ Page");
      expect(result).toContain("Deleted Page");
      expect(result).toContain("moved to trash");
    });

    it("should handle deletion with empty response", async () => {
      mockClient.deletePage.mockResolvedValueOnce(null);

      const result = await handleDeletePage(1, mockClient);

      expect(mockClient.deletePage).toHaveBeenCalledWith(1, undefined);
      expect(typeof result).toBe("string");
      expect(result).toContain("✅ Page 1 has been moved to trash");
    });

    it("should handle forced deletion", async () => {

If the actual handleDeletePage signature in your test file differs (for example, if it’s handleDeletePage(mockClient, 1) or handleDeletePage({ client: mockClient, id: 1 })), update the call in the new test accordingly so that:

  • It uses the same argument pattern as the non-forced deletion test above it.
  • It still causes mockClient.deletePage to be invoked with (1, undefined) when force is not set.

If you also have a specific generic message for forced deletion on empty responses (e.g. “permanently deleted”), you may want to add a second test similar to should handle deletion with empty response but calling the forced variant of handleDeletePage and asserting the permanent-delete message instead.

const createMockClient = () => ({
getPosts: vi.fn(),
getPages: vi.fn(),
getSiteUrl: vi.fn().mockReturnValue("https://example.com"),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Extend SiteAuditor tests to cover the new external link detection logic that depends on siteUrl

The new logic derives the host from siteData.siteUrl and filters external links by excluding URLs containing that host. While getSiteUrl is mocked, there’s no assertion that internal links are ignored and external ones are counted.

Please add/extend a test that:

  • Uses getSiteUrl to return https://example.com.
  • Includes content with both https://example.com/... and external links like https://other.com/....
  • Asserts that externalDependencies counts only the truly external domains.

This will specifically exercise the multi-site bug fix and host-based filtering behavior.

Suggested implementation:

const createMockClient = () => ({
  getPosts: vi.fn(),
  getPages: vi.fn(),
  getSiteUrl: vi.fn().mockReturnValue("https://example.com"),
  authenticate: vi.fn().mockResolvedValue(true),
});

it("counts only external links based on siteUrl host", async () => {
  const client = createMockClient();

  client.getPages.mockResolvedValue([
    {
      id: 1,
      content: `
        <a href="https://example.com/about">Internal link</a>
        <a href="https://example.com/blog/post">Another internal link</a>
        <a href="https://other.com/page">External link</a>
        <a href="https://another.com/page">Another external</a>
      `,
    },
  ]);

  client.getPosts.mockResolvedValue([]);

  // NOTE: Adjust the invocation below to match the existing SiteAuditor API.
  // The intention is to run the auditor and inspect the externalDependencies
  // in its report/summary.
  const auditor = new SiteAuditor(client);
  const report = await auditor.run();

  // Assert that only truly external domains are counted
  expect(report.externalDependencies).toEqual(
    expect.arrayContaining(["https://other.com", "https://another.com"]),
  );
  expect(report.externalDependencies).not.toEqual(
    expect.arrayContaining([
      "https://example.com",
      "https://example.com/about",
      "https://example.com/blog/post",
    ]),
  );
});

The new test assumes:

  1. A SiteAuditor class (or similar) is available in this test file and can be constructed with new SiteAuditor(client) and executed via await auditor.run().
  2. The result exposes an externalDependencies collection that:
    • Contains external URLs or domains as strings.
    • Excludes links whose host matches siteUrl (https://example.com).

To integrate with your existing codebase:

  • Update new SiteAuditor(client) and auditor.run() to match the actual API (e.g., auditSite(client), runSiteAudit({ client }), or similar).
  • If externalDependencies is structured differently (e.g., an object keyed by domain, or nested inside a summary object), adjust the assertions accordingly, ensuring:
    • Internal URLs containing example.com are not counted.
    • External URLs like https://other.com/... and https://another.com/... are counted.
  • If deduplication is done at the domain level (e.g., only other.com and another.com), change the expectations to look for those domains instead of full URLs.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 077b040c44

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const content = item.content?.rendered || "";
const externalLinks = content.match(/https?:\/\/(?!example\.com)[^"'\s>]*/gi) || [];
const siteHost = siteData.siteUrl.replace(/^https?:\/\//, "").replace(/\/.*$/, "");
const externalLinks = content.match(/https?:\/\/[^"'\s>]*/gi)?.filter((url) => !url.includes(siteHost)) || [];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Compare external link hostnames instead of substrings

Using !url.includes(siteHost) misclassifies some external URLs as internal whenever the audited site's host appears anywhere in the URL string (for example, auditing example.com will treat https://notexample.com/... or redirect URLs containing example.com in query params as internal). That undercounts externalDependencies and can prevent the external-dependencies issue from being raised when it should be, so audit results become inaccurate for those sites.

Useful? React with 👍 / 👎.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several multi-site correctness issues found during DXT v3.1.12 testing by aligning tool handler signatures with the server’s (client, args) execution model, removing hardcoded site placeholders, and clarifying/strengthening user-facing outputs for restricted fields and deletions.

Changes:

  • Migrates cache tools to inputSchema + (client, params) handlers and updates cache tool tests accordingly.
  • Fixes SEO site audit to use the active site URL and updates external dependency detection logic.
  • Improves multi-site clarity and robustness in performance stats descriptions, user listing output, and page deletion responses; updates tool descriptions to note admin requirements.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/tools/cache.ts Switches cache tools to inputSchema and (client, params) handlers (multi-site-safe execution).
tests/tools/cache.test.js Updates cache tests to call handlers with (client, params) and assert inputSchema.
src/tools/seo/auditors/SiteAuditor.ts Replaces hardcoded site URL with client.getSiteUrl() and adjusts external dependency scanning.
tests/tools/seo/SiteAuditor.test.js Extends mock client with getSiteUrl() to support new auditor behavior.
src/tools/performance/PerformanceTools.ts Clarifies that top-level metrics are session-wide aggregates and adds scope labeling.
src/tools/users.ts Updates wp_list_users output/description to reflect admin-only metadata restrictions.
tests/tools/users.test.js Updates expectations to match the new restricted-field messaging.
src/tools/pages.ts Enhances delete-page messaging using returned previous.title and attempts to validate deletion results.
tests/tools/pages.test.js Updates deletion assertions to accommodate title-based delete responses.
src/tools/site.ts Notes admin role requirement in site settings tool descriptions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +604 to +605
const siteHost = siteData.siteUrl.replace(/^https?:\/\//, "").replace(/\/.*$/, "");
const externalLinks = content.match(/https?:\/\/[^"'\s>]*/gi)?.filter((url) => !url.includes(siteHost)) || [];
Comment on lines +218 to +226
if (result?.deleted) {
let response = `✅ Page ${id} has been ${action}.`;
if (result.previous) {
response = `✅ Page "${result.previous.title?.rendered || id}" has been ${action}.`;
}
return response;
}

// Some WordPress installations return empty/null responses on successful deletion
Comment on lines +604 to +605
const siteHost = siteData.siteUrl.replace(/^https?:\/\//, "").replace(/\/.*$/, "");
const externalLinks = content.match(/https?:\/\/[^"'\s>]*/gi)?.filter((url) => !url.includes(siteHost)) || [];
- SiteAuditor: use URL hostname parsing instead of substring matching
  for external link detection (prevents notexample.com false negatives)
- SiteAuditor: hoist siteHost computation out of reduce loop
- Pages: handle explicit deleted:false response from WordPress
- Tests: add null/empty response and deleted:false deletion tests
- Tests: add external link hostname detection test for SiteAuditor

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@docdyhr docdyhr merged commit 805a5e4 into main Mar 16, 2026
19 checks passed
@docdyhr docdyhr deleted the fix/multi-site-bug-fixes branch March 16, 2026 14:10
docdyhr pushed a commit that referenced this pull request Mar 16, 2026
## [3.1.13](v3.1.12...v3.1.13) (2026-03-16)

### 🐛 Bug Fixes

* resolve multi-site bugs from DXT v3.1.12 testing ([#172](#172)) ([805a5e4](805a5e4))
@docdyhr
Copy link
Owner Author

docdyhr commented Mar 16, 2026

🎉 This PR is included in version 3.1.13 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants