fix(mcp): accept expected partial outputs (#449)
This commit is contained in:
parent
b631715112
commit
67d68281e3
@ -15,7 +15,7 @@ const STANDARD_MESSAGES: Record<ErrorCode, string> = {
|
||||
"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:
|
||||
|
||||
@ -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<AuditLimitTier> {
|
||||
@ -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");
|
||||
|
||||
@ -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,
|
||||
|
||||
178
src/server/mcp/tools/meta-only-response.test.ts
Normal file
178
src/server/mcp/tools/meta-only-response.test.ts
Normal file
@ -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<string, unknown> = {
|
||||
...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<typeof objectSchema>[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 <name>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<string>();
|
||||
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);
|
||||
}
|
||||
});
|
||||
});
|
||||
@ -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);
|
||||
|
||||
@ -33,7 +33,7 @@ type GscPerfRow = {
|
||||
clicks: number;
|
||||
impressions: number;
|
||||
ctr: number;
|
||||
position: number;
|
||||
position?: number;
|
||||
};
|
||||
|
||||
const GSC_PERF_COLUMNS: McpTableColumn<GscPerfRow>[] = [
|
||||
@ -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(),
|
||||
)
|
||||
|
||||
@ -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,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user