Tame audit polling: SAM's status tool waits server-side; Lighthouse becomes opt-in for agents (#510)
This commit is contained in:
parent
a6f96c516e
commit
43265048df
@ -29,7 +29,7 @@ The project-context tools are free and shared with the app and other agents.
|
|||||||
|
|
||||||
- `whoami`: confirm connection and remaining credits before spending anything. If OpenSEO is not connected, stop and ask the user to connect it.
|
- `whoami`: confirm connection and remaining credits before spending anything. If OpenSEO is not connected, stop and ask the user to connect it.
|
||||||
- `list_projects` / `create_project`: resolve the `projectId`.
|
- `list_projects` / `create_project`: resolve the `projectId`.
|
||||||
- `run_site_audit`: start the crawl (default page budget), then poll `get_audit_status` and read `get_audit_issues`. Use `get_audit_pages` when per-page evidence helps.
|
- `run_site_audit`: start the crawl (default page budget). Leave Lighthouse off (its default) — it adds several minutes and this report doesn't need it; pass `runLighthouse: true` only when the user asks for performance/Core Web Vitals depth. Then check `get_audit_status` (the crawl takes a minute or two — wait between checks rather than polling in a loop) and read `get_audit_issues`. Use `get_audit_pages` when per-page evidence helps.
|
||||||
- `get_backlinks_overview`: backlink and referring-domain picture; usually the deciding data for the "one thing".
|
- `get_backlinks_overview`: backlink and referring-domain picture; usually the deciding data for the "one thing".
|
||||||
- `get_domain_overview`: estimated organic traffic and organic keyword count. Skip when the site is clearly dead.
|
- `get_domain_overview`: estimated organic traffic and organic keyword count. Skip when the site is clearly dead.
|
||||||
- `research_keywords`: keyword ideas with volume and difficulty, used to propose a starting focus area. One call with 1-3 seeds taken from what the site is actually about. Skip when the site is down.
|
- `research_keywords`: keyword ideas with volume and difficulty, used to propose a starting focus area. One call with 1-3 seeds taken from what the site is actually about. Skip when the site is down.
|
||||||
@ -39,7 +39,7 @@ Keep total spend modest: one audit, one backlinks overview, at most one domain o
|
|||||||
## Workflow
|
## Workflow
|
||||||
|
|
||||||
1. `whoami`, then resolve the `projectId`.
|
1. `whoami`, then resolve the `projectId`.
|
||||||
2. `run_site_audit` for the domain. While it crawls, fetch `get_backlinks_overview`.
|
2. `run_site_audit` for the domain (Lighthouse stays off unless the user asked for performance depth). While it crawls, fetch `get_backlinks_overview`.
|
||||||
3. When the crawl finishes, read `get_audit_issues` (and `get_domain_overview` if the site is alive).
|
3. When the crawl finishes, read `get_audit_issues` (and `get_domain_overview` if the site is alive).
|
||||||
4. If the audit comes back broken or nearly empty (certificate errors, 5xx, one page crawled): investigate before writing. Check the certificate and redirect variants yourself, and search the web for the business. A dead domain often has a live successor site, which flips the whole recommendation to "redirect the old domain".
|
4. If the audit comes back broken or nearly empty (certificate errors, 5xx, one page crawled): investigate before writing. Check the certificate and redirect variants yourself, and search the web for the business. A dead domain often has a live successor site, which flips the whole recommendation to "redirect the old domain".
|
||||||
5. Verify every finding you plan to report against the live page HTML by fetching pages yourself. Report nothing you have not seen evidence for.
|
5. Verify every finding you plan to report against the live page HTML by fetching pages yourself. Report nothing you have not seen evidence for.
|
||||||
|
|||||||
@ -29,7 +29,7 @@ The project-context tools are free and shared with the app and other agents.
|
|||||||
|
|
||||||
- `whoami`: confirm connection and remaining credits before spending anything. If OpenSEO is not connected, stop and ask the user to connect it.
|
- `whoami`: confirm connection and remaining credits before spending anything. If OpenSEO is not connected, stop and ask the user to connect it.
|
||||||
- `list_projects` / `create_project`: resolve the `projectId`.
|
- `list_projects` / `create_project`: resolve the `projectId`.
|
||||||
- `run_site_audit`: start the crawl (default page budget), then poll `get_audit_status` and read `get_audit_issues`. Use `get_audit_pages` when per-page evidence helps.
|
- `run_site_audit`: start the crawl (default page budget). Leave Lighthouse off (its default) — it adds several minutes and this report doesn't need it; pass `runLighthouse: true` only when the user asks for performance/Core Web Vitals depth. Then check `get_audit_status` (the crawl takes a minute or two — wait between checks rather than polling in a loop) and read `get_audit_issues`. Use `get_audit_pages` when per-page evidence helps.
|
||||||
- `get_backlinks_overview`: backlink and referring-domain picture; usually the deciding data for the "one thing".
|
- `get_backlinks_overview`: backlink and referring-domain picture; usually the deciding data for the "one thing".
|
||||||
- `get_domain_overview`: estimated organic traffic and organic keyword count. Skip when the site is clearly dead.
|
- `get_domain_overview`: estimated organic traffic and organic keyword count. Skip when the site is clearly dead.
|
||||||
- `research_keywords`: keyword ideas with volume and difficulty, used to propose a starting focus area. One call with 1-3 seeds taken from what the site is actually about. Skip when the site is down.
|
- `research_keywords`: keyword ideas with volume and difficulty, used to propose a starting focus area. One call with 1-3 seeds taken from what the site is actually about. Skip when the site is down.
|
||||||
@ -39,7 +39,7 @@ Keep total spend modest: one audit, one backlinks overview, at most one domain o
|
|||||||
## Workflow
|
## Workflow
|
||||||
|
|
||||||
1. `whoami`, then resolve the `projectId`.
|
1. `whoami`, then resolve the `projectId`.
|
||||||
2. `run_site_audit` for the domain. While it crawls, fetch `get_backlinks_overview`.
|
2. `run_site_audit` for the domain (Lighthouse stays off unless the user asked for performance depth). While it crawls, fetch `get_backlinks_overview`.
|
||||||
3. When the crawl finishes, read `get_audit_issues` (and `get_domain_overview` if the site is alive).
|
3. When the crawl finishes, read `get_audit_issues` (and `get_domain_overview` if the site is alive).
|
||||||
4. If the audit comes back broken or nearly empty (certificate errors, 5xx, one page crawled): investigate before writing. Check the certificate and redirect variants yourself, and search the web for the business. A dead domain often has a live successor site, which flips the whole recommendation to "redirect the old domain".
|
4. If the audit comes back broken or nearly empty (certificate errors, 5xx, one page crawled): investigate before writing. Check the certificate and redirect variants yourself, and search the web for the business. A dead domain often has a live successor site, which flips the whole recommendation to "redirect the old domain".
|
||||||
5. Verify every finding you plan to report against the live page HTML by fetching pages yourself. Report nothing you have not seen evidence for.
|
5. Verify every finding you plan to report against the live page HTML by fetching pages yourself. Report nothing you have not seen evidence for.
|
||||||
|
|||||||
60
src/server/features/sam/samChatTools.test.ts
Normal file
60
src/server/features/sam/samChatTools.test.ts
Normal file
@ -0,0 +1,60 @@
|
|||||||
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import type { Tool } from "ai";
|
||||||
|
import { waitingAuditStatusTool } from "./samChatTools";
|
||||||
|
|
||||||
|
vi.mock("cloudflare:workers", () => ({
|
||||||
|
env: {},
|
||||||
|
DurableObject: class {
|
||||||
|
kind = "mock";
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
// The server-side wait in SAM's get_audit_status: a completed audit must
|
||||||
|
// return without waiting, and a running one must return as soon as the status
|
||||||
|
// line changes — a regression in either turns every status check into the
|
||||||
|
// full 50-second budget.
|
||||||
|
|
||||||
|
const running = (line: string) => ({
|
||||||
|
summary: line,
|
||||||
|
data: { status: { status: "running" } },
|
||||||
|
});
|
||||||
|
const completed = {
|
||||||
|
summary: "done",
|
||||||
|
data: { status: { status: "completed" } },
|
||||||
|
};
|
||||||
|
|
||||||
|
function buildTool(outputs: unknown[]) {
|
||||||
|
const execute = vi.fn(() => Promise.resolve(outputs.shift()));
|
||||||
|
// oxlint-disable-next-line typescript/no-unsafe-type-assertion -- the wrapper only touches execute
|
||||||
|
const tool = waitingAuditStatusTool(() => ({ execute }) as unknown as Tool);
|
||||||
|
return { tool, execute };
|
||||||
|
}
|
||||||
|
|
||||||
|
const callOptions = { toolCallId: "t", messages: [] };
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("waitingAuditStatusTool", () => {
|
||||||
|
it("returns a finished audit without waiting", async () => {
|
||||||
|
const { tool, execute } = buildTool([completed]);
|
||||||
|
await expect(tool.execute?.({}, callOptions)).resolves.toBe(completed);
|
||||||
|
expect(execute).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("waits while running and returns as soon as progress changes", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
const { tool, execute } = buildTool([
|
||||||
|
running("phase crawl, 3/56 pages"),
|
||||||
|
running("phase crawl, 3/56 pages"),
|
||||||
|
running("phase lighthouse, 56/56 pages"),
|
||||||
|
]);
|
||||||
|
const call: unknown = tool.execute?.({}, callOptions);
|
||||||
|
await vi.advanceTimersByTimeAsync(10_000);
|
||||||
|
await expect(call).resolves.toMatchObject({
|
||||||
|
summary: "phase lighthouse, 56/56 pages",
|
||||||
|
});
|
||||||
|
expect(execute).toHaveBeenCalledTimes(3);
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -35,7 +35,6 @@ import {
|
|||||||
getGoogleAnalyticsTrafficAcquisitionTool,
|
getGoogleAnalyticsTrafficAcquisitionTool,
|
||||||
getSearchOpportunitiesTool,
|
getSearchOpportunitiesTool,
|
||||||
} from "@/server/mcp/tools/google-analytics-tools";
|
} from "@/server/mcp/tools/google-analytics-tools";
|
||||||
import { GA4_OAUTH_APP_PENDING, isGa4ConnectAvailable } from "@/shared/ga4";
|
|
||||||
import {
|
import {
|
||||||
findSerpCompetitorsTool,
|
findSerpCompetitorsTool,
|
||||||
getGoogleBusinessQuestionsTool,
|
getGoogleBusinessQuestionsTool,
|
||||||
@ -139,6 +138,81 @@ function adaptMcpTool<Shape extends ZodRawShape>(
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Audits run for minutes, and a chat model cannot sleep — given an instant
|
||||||
|
// status tool it spin-polls, and every call plus its result is persisted into
|
||||||
|
// the session history. SAM's get_audit_status therefore waits server-side:
|
||||||
|
// while the audit is running, it re-reads every few seconds and returns as
|
||||||
|
// soon as the status line changes (or when the budget runs out), so one tool
|
||||||
|
// call buys ~a minute of quiet waiting. The sleep sits BETWEEN adapted calls,
|
||||||
|
// so each re-read scopes its own short-lived DB client rather than holding one
|
||||||
|
// through the wait; each re-read also emits its own mcp:tool_call event, so
|
||||||
|
// telemetry counts server polls, not model calls.
|
||||||
|
const AUDIT_STATUS_POLL_MS = 2_000;
|
||||||
|
const AUDIT_STATUS_WAIT_BUDGET_MS = 50_000;
|
||||||
|
|
||||||
|
// The adapted tool returns toModelOutput's { summary, data } flattening; the
|
||||||
|
// summary line carries phase + page counts, so a changed line IS progress.
|
||||||
|
const auditProgressLine = (result: unknown): string | null =>
|
||||||
|
typeof result === "object" &&
|
||||||
|
result !== null &&
|
||||||
|
"summary" in result &&
|
||||||
|
typeof result.summary === "string"
|
||||||
|
? result.summary
|
||||||
|
: null;
|
||||||
|
|
||||||
|
const auditIsRunning = (result: unknown): boolean => {
|
||||||
|
if (typeof result !== "object" || result === null || !("data" in result)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const data = result.data;
|
||||||
|
if (typeof data !== "object" || data === null || !("status" in data)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const status = data.status;
|
||||||
|
return (
|
||||||
|
typeof status === "object" &&
|
||||||
|
status !== null &&
|
||||||
|
"status" in status &&
|
||||||
|
status.status === "running"
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
export function waitingAuditStatusTool(
|
||||||
|
adapt: (
|
||||||
|
definition: McpToolDefinition<typeof getAuditStatusTool.config.inputSchema>,
|
||||||
|
) => Tool,
|
||||||
|
): Tool {
|
||||||
|
const base = adapt({
|
||||||
|
...getAuditStatusTool,
|
||||||
|
config: {
|
||||||
|
...getAuditStatusTool.config,
|
||||||
|
description: `${getAuditStatusTool.config.description} While the audit is running this call waits up to ~1 minute server-side and returns as soon as progress changes, so never call it in a tight loop: check a few times, narrating progress to the user in between, and if it is still running after that, say so and let the user come back for the results.`,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const baseExecute = base.execute;
|
||||||
|
if (!baseExecute) return base;
|
||||||
|
|
||||||
|
return {
|
||||||
|
...base,
|
||||||
|
execute: async (args, options) => {
|
||||||
|
const deadline = Date.now() + AUDIT_STATUS_WAIT_BUDGET_MS;
|
||||||
|
let result: unknown = await baseExecute(args, options);
|
||||||
|
const initial = auditProgressLine(result);
|
||||||
|
while (
|
||||||
|
auditIsRunning(result) &&
|
||||||
|
auditProgressLine(result) === initial &&
|
||||||
|
Date.now() < deadline
|
||||||
|
) {
|
||||||
|
await new Promise((resolve) =>
|
||||||
|
setTimeout(resolve, AUDIT_STATUS_POLL_MS),
|
||||||
|
);
|
||||||
|
result = await baseExecute(args, options);
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
// Free (credit-less) site-reading tools, mirroring the onboarding agent's
|
// Free (credit-less) site-reading tools, mirroring the onboarding agent's
|
||||||
// read_website but split into discovery + reading so the model can pick which
|
// read_website but split into discovery + reading so the model can pick which
|
||||||
// pages to read instead of blindly taking the first N sitemap entries.
|
// pages to read instead of blindly taking the first N sitemap entries.
|
||||||
@ -295,42 +369,38 @@ export function buildSamMcpTools(
|
|||||||
get_keyword_metrics: adaptTool(getKeywordMetricsTool),
|
get_keyword_metrics: adaptTool(getKeywordMetricsTool),
|
||||||
get_search_console_performance: adaptTool(getSearchConsolePerformanceTool),
|
get_search_console_performance: adaptTool(getSearchConsolePerformanceTool),
|
||||||
inspect_urls: adaptTool(inspectUrlsTool),
|
inspect_urls: adaptTool(inspectUrlsTool),
|
||||||
// Same rollout gate as the MCP server: GA4 tools are hidden until the
|
// Unconditional like the MCP server's registrations — the GA4 launch gate
|
||||||
// OAuth app clears verification, except for allowlisted users.
|
// was removed in #505.
|
||||||
...(!GA4_OAUTH_APP_PENDING || isGa4ConnectAvailable(authContext.userEmail)
|
get_google_analytics_organic_landing_pages: adaptObjectTool(
|
||||||
? {
|
getGoogleAnalyticsOrganicLandingPagesTool,
|
||||||
get_google_analytics_organic_landing_pages: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsOrganicLandingPagesTool,
|
get_google_analytics_page_performance: adaptObjectTool(
|
||||||
),
|
getGoogleAnalyticsPagePerformanceTool,
|
||||||
get_google_analytics_page_performance: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsPagePerformanceTool,
|
get_google_analytics_key_events: adaptObjectTool(
|
||||||
),
|
getGoogleAnalyticsKeyEventsTool,
|
||||||
get_google_analytics_key_events: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsKeyEventsTool,
|
get_search_opportunities: adaptObjectTool(getSearchOpportunitiesTool),
|
||||||
),
|
get_google_analytics_organic_overview: adaptObjectTool(
|
||||||
get_search_opportunities: adaptObjectTool(getSearchOpportunitiesTool),
|
getGoogleAnalyticsOrganicOverviewTool,
|
||||||
get_google_analytics_organic_overview: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsOrganicOverviewTool,
|
get_google_analytics_traffic_acquisition: adaptObjectTool(
|
||||||
),
|
getGoogleAnalyticsTrafficAcquisitionTool,
|
||||||
get_google_analytics_traffic_acquisition: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsTrafficAcquisitionTool,
|
get_google_analytics_measurement_health: adaptObjectTool(
|
||||||
),
|
getGoogleAnalyticsMeasurementHealthTool,
|
||||||
get_google_analytics_measurement_health: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsMeasurementHealthTool,
|
get_google_analytics_ecommerce_performance: adaptObjectTool(
|
||||||
),
|
getGoogleAnalyticsEcommercePerformanceTool,
|
||||||
get_google_analytics_ecommerce_performance: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsEcommercePerformanceTool,
|
get_google_analytics_site_search: adaptObjectTool(
|
||||||
),
|
getGoogleAnalyticsSiteSearchTool,
|
||||||
get_google_analytics_site_search: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsSiteSearchTool,
|
get_google_analytics_audience_breakdown: adaptObjectTool(
|
||||||
),
|
getGoogleAnalyticsAudienceBreakdownTool,
|
||||||
get_google_analytics_audience_breakdown: adaptObjectTool(
|
),
|
||||||
getGoogleAnalyticsAudienceBreakdownTool,
|
|
||||||
),
|
|
||||||
}
|
|
||||||
: {}),
|
|
||||||
run_site_audit: adaptTool(runSiteAuditTool),
|
run_site_audit: adaptTool(runSiteAuditTool),
|
||||||
get_audit_status: adaptTool(getAuditStatusTool),
|
get_audit_status: waitingAuditStatusTool(adaptTool),
|
||||||
get_audit_issues: adaptTool(getAuditIssuesTool),
|
get_audit_issues: adaptTool(getAuditIssuesTool),
|
||||||
get_audit_pages: adaptTool(getAuditPagesTool),
|
get_audit_pages: adaptTool(getAuditPagesTool),
|
||||||
};
|
};
|
||||||
|
|||||||
@ -57,7 +57,7 @@ const runInputSchema = {
|
|||||||
.boolean()
|
.boolean()
|
||||||
.optional()
|
.optional()
|
||||||
.describe(
|
.describe(
|
||||||
"Run Lighthouse on a sample of up to 10 representative pages (default true).",
|
"Run Lighthouse on a sample of up to 10 representative pages (default false — it adds several minutes of wall-clock time). Pass true only when the user wants performance/Core Web Vitals detail.",
|
||||||
),
|
),
|
||||||
} as const;
|
} as const;
|
||||||
|
|
||||||
@ -83,7 +83,10 @@ export const runSiteAuditTool = {
|
|||||||
},
|
},
|
||||||
},
|
},
|
||||||
handler: withMcpProjectAuth(async (args: RunArgs, context) => {
|
handler: withMcpProjectAuth(async (args: RunArgs, context) => {
|
||||||
const lighthouseStrategy = (args.runLighthouse ?? true) ? "auto" : "none";
|
// Default OFF for agent calls: Lighthouse turns a 1-2 minute crawl into a
|
||||||
|
// many-minute wait, which chat agents handle badly. The app UI passes its
|
||||||
|
// own explicit lighthouseStrategy, so this default only governs agents.
|
||||||
|
const lighthouseStrategy = (args.runLighthouse ?? false) ? "auto" : "none";
|
||||||
const limitTier = await AuditService.resolveAuditLimitTier(
|
const limitTier = await AuditService.resolveAuditLimitTier(
|
||||||
context.auth.organizationId,
|
context.auth.organizationId,
|
||||||
);
|
);
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user