diff --git a/src/client/lib/error-messages.ts b/src/client/lib/error-messages.ts index bc05e99..09973b3 100644 --- a/src/client/lib/error-messages.ts +++ b/src/client/lib/error-messages.ts @@ -15,7 +15,7 @@ const STANDARD_MESSAGES: Record = { "You've reached audit capacity for your account. Delete old audits from your projects to start a new one.", AUDIT_PAGE_LIMIT_EXCEEDED: `Free plan audits are limited to ${FREE_MAX_AUDIT_PAGES} pages. Upgrade to run larger audits.`, AUDIT_ALREADY_RUNNING: - "You already have an audit running. Wait for it to finish or delete it before starting another.", + "You've reached the limit of audits running at once. Wait for one to finish or delete it before starting another.", VALIDATION_ERROR: "Please check your input and try again.", CRAWL_TARGET_BLOCKED: "This crawl target is blocked by security policy.", BACKLINKS_BILLING_ISSUE: diff --git a/src/server/features/audit/services/AuditService.ts b/src/server/features/audit/services/AuditService.ts index 1610b38..f47d98f 100644 --- a/src/server/features/audit/services/AuditService.ts +++ b/src/server/features/audit/services/AuditService.ts @@ -26,9 +26,9 @@ import { import { reconcileRunningAudit } from "@/server/features/audit/services/auditReconciler"; import { isHostedServerAuthMode } from "@/server/lib/runtime-env"; -// Plan-tier limits are the abuse bound in hosted mode: free accounts get one -// small audit at a time, paid keeps the full limits, and customers with no -// Autumn product at all are turned away. Self-hosted isn't gated. +// Plan-tier limits are the abuse bound in hosted mode: free accounts get small +// audits with a bounded burst, paid keeps the full limits, and customers with +// no Autumn product at all are turned away. Self-hosted isn't gated. async function resolveAuditLimitTier( customer: BillingCustomerContext, ): Promise { @@ -91,10 +91,10 @@ async function startAudit(input: { try { // Concurrency and capacity are enforced after the insert, not before: a // pre-insert read is a check-then-act race, so parallel requests would all - // pass the free tier's one-running-audit gate. Post-insert, each request - // sees at least its own row, so at most one racer can pass; the losers - // roll back via the catch below. Two true racers may both abort — the - // user just retries. + // pass the free tier's running-audits gate. Post-insert, each request sees + // at least its own row, so racers can't all slip under the limit; the + // losers roll back via the catch below. Racers at the boundary may all + // abort — the user just retries. const usage = await AuditRepository.getAuditUsageForUser(input.actorUserId); if (usage.runningCount > limits.maxRunningAudits) { throw new AppError("AUDIT_ALREADY_RUNNING"); diff --git a/src/server/features/audit/services/audit-capacity.ts b/src/server/features/audit/services/audit-capacity.ts index 258e798..edf62cc 100644 --- a/src/server/features/audit/services/audit-capacity.ts +++ b/src/server/features/audit/services/audit-capacity.ts @@ -10,10 +10,12 @@ export type AuditLimitTier = "free" | "paid" | "self_hosted"; // The crawler runs on our Workers compute and isn't credit-metered, so these // per-tier bounds are the abuse control: free accounts cost nothing to create, -// so they get one small audit at a time and a modest total budget. Paid gets -// bounds sized for real sites rather than abuse (a payment method on file is -// the deterrent). The cumulative bound is a hosted commercial policy, while -// the per-audit page limit is also a technical Workflow/database ceiling. +// so they get small audits, a modest burst of concurrent runs, and a modest +// total budget — the cumulative cap bounds total work, concurrency only bounds +// the rate. Paid gets bounds sized for real sites rather than abuse (a payment +// method on file is the deterrent). The cumulative bound is a hosted commercial +// policy, while the per-audit page limit is also a technical Workflow/database +// ceiling. export const AUDIT_LIMITS: Record< AuditLimitTier, { @@ -25,7 +27,7 @@ export const AUDIT_LIMITS: Record< free: { maxPagesPerAudit: FREE_MAX_AUDIT_PAGES, maxCapacityUnits: 2_000, - maxRunningAudits: 1, + maxRunningAudits: 5, }, paid: { maxPagesPerAudit: PAID_MAX_AUDIT_PAGES, diff --git a/src/server/mcp/tools/meta-only-response.test.ts b/src/server/mcp/tools/meta-only-response.test.ts new file mode 100644 index 0000000..10a5487 --- /dev/null +++ b/src/server/mcp/tools/meta-only-response.test.ts @@ -0,0 +1,178 @@ +import { readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { objectSchema } from "@/server/mcp/output-schemas"; + +vi.mock("cloudflare:workers", () => ({ + env: {}, + DurableObject: class { + readonly ctx = null; + }, +})); + +import * as addRankTrackingKeywords from "./add-rank-tracking-keywords"; +import * as createProject from "./create-project"; +import * as createRankTracker from "./create-rank-tracker"; +import * as dataforseoResearchTools from "./dataforseo-research-tools"; +import * as estimateRankTrackerCost from "./estimate-rank-tracker-cost"; +import * as getBacklinksOverview from "./get-backlinks-overview"; +import * as getBacklinksProfile from "./get-backlinks-profile"; +import * as getDomainKeywordSuggestions from "./get-domain-keyword-suggestions"; +import * as getDomainOverview from "./get-domain-overview"; +import * as getRankTracker from "./get-rank-tracker"; +import * as getSerpResults from "./get-serp-results"; +import * as googleAnalyticsTools from "./google-analytics-tools"; +import * as listProjects from "./list-projects"; +import * as listSavedKeywords from "./list-saved-keywords"; +import * as localSeoTools from "./local-seo-tools"; +import * as projectContext from "./project-context"; +import * as removeRankTrackingKeywords from "./remove-rank-tracking-keywords"; +import * as researchKeywords from "./research-keywords"; +import * as runRankTracker from "./run-rank-tracker"; +import * as saveKeywords from "./save-keywords"; +import * as searchConsoleTools from "./search-console-tools"; +import * as siteAuditTools from "./site-audit-tools"; +import * as whoami from "./whoami"; + +const toolExports: Record = { + ...addRankTrackingKeywords, + ...createProject, + ...createRankTracker, + ...dataforseoResearchTools, + ...estimateRankTrackerCost, + ...getBacklinksOverview, + ...getBacklinksProfile, + ...getDomainKeywordSuggestions, + ...getDomainOverview, + ...getRankTracker, + ...getSerpResults, + ...googleAnalyticsTools, + ...listProjects, + ...listSavedKeywords, + ...localSeoTools, + ...projectContext, + ...removeRankTrackingKeywords, + ...researchKeywords, + ...runRankTracker, + ...saveKeywords, + ...searchConsoleTools, + ...siteAuditTools, + ...whoami, +}; + +type ToolDefinition = { + name: string; + config: { outputSchema?: Parameters[0] }; +}; + +function isToolDefinition(value: unknown): value is ToolDefinition { + return ( + typeof value === "object" && + value !== null && + "name" in value && + typeof value.name === "string" && + "config" in value + ); +} + +const TOOLS_DIR = join(import.meta.dirname, "."); + +/** Every `export const Tool = {` in a tools file, with the character + * offset where that tool's source begins. */ +function toolSpans(source: string): { exportName: string; start: number }[] { + const spans: { exportName: string; start: number }[] = []; + const pattern = /export const (\w+Tool)\s*=/g; + let match: RegExpExecArray | null; + while ((match = pattern.exec(source)) !== null) { + spans.push({ exportName: match[1], start: match.index }); + } + return spans; +} + +/** Offsets of `mcpResponse({...})` calls whose argument object has no + * top-level `structuredContent` key. Brace matching keeps a nested mention + * from masking a missing one. */ +function metaOnlyResponseOffsets(source: string): number[] { + const offsets: number[] = []; + let index = source.indexOf("mcpResponse("); + while (index !== -1) { + let depth = 0; + let cursor = index + "mcpResponse".length; + const argStart = cursor; + for (; cursor < source.length; cursor++) { + const char = source[cursor]; + if (char === "(" || char === "{" || char === "[") depth++; + else if (char === ")" || char === "}" || char === "]") { + depth--; + if (depth === 0) break; + } + } + const args = source.slice(argStart, cursor + 1); + let argDepth = 0; + let hasStructuredContent = false; + for (let i = 0; i < args.length; i++) { + const char = args[i]; + if (char === "(" || char === "{" || char === "[") argDepth++; + else if (char === ")" || char === "}" || char === "]") argDepth--; + // depth 2 == a key directly on the argument object literal + else if (argDepth === 2 && args.startsWith("structuredContent", i)) { + hasStructuredContent = true; + } + } + if (!hasStructuredContent) offsets.push(index); + index = source.indexOf("mcpResponse(", cursor + 1); + } + return offsets; +} + +/** Tool exports that answer with meta but no structuredContent somewhere in + * their handler. */ +function toolsReturningMetaOnly(): string[] { + const names = new Set(); + const files = readdirSync(TOOLS_DIR).filter( + (file) => file.endsWith(".ts") && !file.endsWith(".test.ts"), + ); + for (const file of files) { + const source = readFileSync(join(TOOLS_DIR, file), "utf8"); + const spans = toolSpans(source); + for (const offset of metaOnlyResponseOffsets(source)) { + const owner = spans.filter((span) => span.start < offset).at(-1); + if (owner) names.add(owner.exportName); + } + } + return [...names].toSorted(); +} + +/** + * mcpResponse turns a response carrying `meta` but no `structuredContent` into + * `{ meta }`, and the MCP SDK validates that against the tool's output schema + * and converts a mismatch into a client-visible -32602 — after the handler has + * already done the work. run_site_audit's capacity refusal hit exactly this in + * production. So any tool that can answer with meta alone must declare an + * output schema that accepts a meta-only payload. + * + * Asserting the reverse (that *every* tool tolerates a meta-only payload) would + * be wrong: 43 of the 46 tools legitimately require output fields they always + * populate, and loosening those would forfeit real validation. + */ +describe("tools that answer with meta but no structured content", () => { + it("declare an output schema that accepts a meta-only response", async () => { + const owners = toolsReturningMetaOnly(); + expect(owners.length).toBeGreaterThan(0); + + for (const exportName of owners) { + const tool = toolExports[exportName]; + if (!isToolDefinition(tool)) { + throw new Error(`${exportName} is not an exported tool definition`); + } + const { name, config } = tool; + if (!config.outputSchema) continue; + + const result = await objectSchema(config.outputSchema).safeParseAsync({ + meta: { organizationId: "org_123", projectId: "project_123" }, + }); + + expect(result.success, `${name} rejects a meta-only response`).toBe(true); + } + }); +}); diff --git a/src/server/mcp/tools/output-schema-validation.test.ts b/src/server/mcp/tools/output-schema-validation.test.ts index 5dc9260..a86668d 100644 --- a/src/server/mcp/tools/output-schema-validation.test.ts +++ b/src/server/mcp/tools/output-schema-validation.test.ts @@ -4,6 +4,8 @@ import { objectSchema } from "@/server/mcp/output-schemas"; import * as researchTools from "./dataforseo-research-tools"; import * as localSeoTools from "./local-seo-tools"; import { getBacklinksProfileTool } from "./get-backlinks-profile"; +import { getSearchConsolePerformanceTool } from "./search-console-tools"; +import { runSiteAuditTool } from "./site-audit-tools"; import { makeToolContext } from "./tool-test-support"; const mocks = vi.hoisted(() => ({ @@ -13,6 +15,9 @@ const mocks = vi.hoisted(() => ({ vi.mock("cloudflare:workers", () => ({ env: {}, + DurableObject: class { + readonly ctx = null; + }, })); vi.mock("@/server/features/projects/services/ProjectService", () => ({ @@ -145,6 +150,37 @@ describe("DataForSEO research tool output schemas", () => { }); }); +describe("MCP output schemas with expected missing fields", () => { + // Google omits position for the discover and googleNews search types. + it("accepts Search Console rows without a position", async () => { + const schema = objectSchema( + getSearchConsolePerformanceTool.config.outputSchema, + ); + + const result = await schema.safeParseAsync({ + ok: true, + rows: [{ clicks: 0, impressions: 1, ctr: 0 }], + }); + + expect(result.success).toBe(true); + }); + + // Refusals (for example, audit capacity) never start an audit, so they + // have no id to report. + it("accepts a site-audit refusal without an audit id", async () => { + const schema = objectSchema(runSiteAuditTool.config.outputSchema); + + const result = await schema.safeParseAsync({ + meta: { + organizationId: "org_123", + projectId: "project_123", + }, + }); + + expect(result.success).toBe(true); + }); +}); + describe("get_backlinks_profile MCP tool", () => { it("returns paginated backlink rows and honors filters, sorting, and mode", async () => { mocks.profileBacklinksPage.mockResolvedValue(backlinkPage); diff --git a/src/server/mcp/tools/search-console-tools.ts b/src/server/mcp/tools/search-console-tools.ts index 8dbfbdf..e3b15cd 100644 --- a/src/server/mcp/tools/search-console-tools.ts +++ b/src/server/mcp/tools/search-console-tools.ts @@ -33,7 +33,7 @@ type GscPerfRow = { clicks: number; impressions: number; ctr: number; - position: number; + position?: number; }; const GSC_PERF_COLUMNS: McpTableColumn[] = [ @@ -183,7 +183,7 @@ export const getSearchConsolePerformanceTool = { config: { title: "Get Google Search Console performance", description: - "Query the connected Search Console property's Search Analytics: clicks, impressions, CTR, and average position by query/page/country/device/date. First-party data — use it for what already ranks, near-ranking queries, and pages with real demand. ctr is a 0-1 fraction; position is a 1-based average; dates are Pacific Time; the last ~3 days may be incomplete. Read-only; uses no credits.", + "Query the connected Search Console property's Search Analytics: clicks, impressions, CTR, and average position by query/page/country/device/date. First-party data — use it for what already ranks, near-ranking queries, and pages with real demand. ctr is a 0-1 fraction; position is a 1-based average and is omitted from rows when type is 'discover' or 'googleNews' (Google does not report it there — treat it as unavailable, not a failure); dates are Pacific Time; the last ~3 days may be incomplete. Read-only; uses no credits.", inputSchema: perfInputSchema, outputSchema: { ok: z.boolean(), @@ -203,7 +203,10 @@ export const getSearchConsolePerformanceTool = { clicks: z.number(), impressions: z.number(), ctr: z.number(), - position: z.number(), + // Google omits position for the discover and googleNews search + // types even though the other metrics are present. The table + // already renders a missing position as an em dash. + position: z.number().optional(), }) .passthrough(), ) diff --git a/src/server/mcp/tools/site-audit-tools.ts b/src/server/mcp/tools/site-audit-tools.ts index 728551a..d8a5d3a 100644 --- a/src/server/mcp/tools/site-audit-tools.ts +++ b/src/server/mcp/tools/site-audit-tools.ts @@ -73,7 +73,9 @@ export const runSiteAuditTool = { inputSchema: runInputSchema, outputSchema: z .object({ - auditId: z.string(), + // Expected refusal responses (for example, account audit capacity) + // do not start an audit and therefore have no id. + auditId: z.string().optional(), ...optionalMetaOutputSchema, }) .passthrough(), @@ -101,12 +103,17 @@ export const runSiteAuditTool = { limitTier, })); } catch (error) { - if ( - error instanceof AppError && - error.code === "AUDIT_CAPACITY_REACHED" - ) { + // Expected refusals become readable answers instead of protocol errors: + // no audit started, so there is no auditId to report. + const refusalText = + error instanceof AppError && error.code === "AUDIT_CAPACITY_REACHED" + ? "Audit capacity reached for this account — delete old audits in the dashboard to free capacity, then try again." + : error instanceof AppError && error.code === "AUDIT_ALREADY_RUNNING" + ? "This account is at its limit of concurrently running audits. Poll get_audit_status until one finishes, then try again." + : null; + if (refusalText) { return mcpResponse({ - text: "Audit capacity reached for this account — delete old audits in the dashboard to free capacity, then try again.", + text: refusalText, meta: buildProjectMeta( context, args.projectId,