diff --git a/src/lib/feedback-hint.ts b/src/lib/feedback-hint.ts index 75f5290d..7b03fa7d 100644 --- a/src/lib/feedback-hint.ts +++ b/src/lib/feedback-hint.ts @@ -24,10 +24,18 @@ export const NO_HINT_TOOLS: ReadonlySet = new Set([ "getAccessibilityAuthConfig", ]); +/** + * Set (as a symbol-keyed property, so it is never serialised to the client) on an error + * result that the user resolves on their own account, such as a plan limit. Asking the + * agent to report those with submitFeedback only sends it the wrong way. + */ +export const NO_FEEDBACK_HINT = Symbol.for("browserstack.mcp.noFeedbackHint"); + export function withFeedbackHint(toolName: string, result: T): T { if (NO_HINT_TOOLS.has(toolName)) return result; const r = result as { isError?: unknown; content?: unknown } | null; if (typeof r !== "object" || r === null || r.isError !== true) return result; + if ((r as Record)[NO_FEEDBACK_HINT] === true) return result; if (!Array.isArray(r.content) || SELF_FIXABLE.test(textOf(r))) return result; return { ...r, diff --git a/src/tools/accessibility.ts b/src/tools/accessibility.ts index d32aacf4..9f7de761 100644 --- a/src/tools/accessibility.ts +++ b/src/tools/accessibility.ts @@ -24,6 +24,7 @@ import { AccessibilityAuthConfig, safeAuthConfigData, } from "./accessiblity-utils/auth-config.js"; +import { NO_FEEDBACK_HINT } from "../lib/feedback-hint.js"; import { trackMCP } from "../lib/instrumentation.js"; import { compareIssues, @@ -103,6 +104,26 @@ function createSuccessResponse(messages: string[]): CallToolResult { }; } +/** + * Refusals the user resolves on their own account, with what to tell them. The API's own + * words ("You do not have access to External APIs", "user info missing") read like faults, + * so agents retried, renamed the scan, or reported them with submitFeedback. Each pattern + * is the API's message as logged in telemetry. + */ +function accountRefusal(message: string): string | undefined { + const scanLimit = message.match(/make over (\d+) scans/i); + if (scanLimit) { + return `This account has reached its plan's limit of ${scanLimit[1]} saved Website Scanner scans, so no new scan can be started. To scan again, delete scans that are no longer needed under Website Scanner in the Accessibility dashboard (https://accessibility.browserstack.com/site-scanner), or upgrade the plan. This is a plan limit, not a fault: tell the user, and do not retry or start the scan under another name.`; + } + if (/access to External APIs/i.test(message)) { + return "This account's BrowserStack plan does not include the External APIs add-on, which the MCP's accessibility tools need: they work through the Accessibility API. Scans can still be run and read in the Accessibility dashboard (https://accessibility.browserstack.com). To use them from here, the account admin can add External APIs to the plan through BrowserStack sales. This is a plan limit, not a fault: tell the user, and do not retry."; + } + if (/user info missing/i.test(message)) { + return "Website Scanner has not set this user up yet: it does that the first time they open Website Scanner in the Accessibility dashboard, and refuses its API until then. Ask the user to open https://accessibility.browserstack.com/site-scanner once while signed in to BrowserStack, then start the scan again."; + } + return undefined; +} + function handleMCPError( toolName: string, server: McpServer, @@ -111,6 +132,13 @@ function handleMCPError( ): CallToolResult { trackMCP(toolName, server.server.getClientVersion()!, error, config); const errorMessage = error instanceof Error ? error.message : "Unknown error"; + const refusal = accountRefusal(errorMessage); + if (refusal) { + return Object.assign( + createErrorResponse(`${refusal}\nBrowserStack said: ${errorMessage}`), + { [NO_FEEDBACK_HINT]: true }, + ); + } return createErrorResponse( `Failed to ${toolName.replace(/([A-Z])/g, " $1").toLowerCase()}: ${errorMessage}. Please open an issue on GitHub if the problem persists`, ); @@ -150,6 +178,38 @@ async function initializeReportFetcher( return reportFetcher; } +/** The longest question the accessibility knowledge base takes; longer ones are refused. */ +export const EXPERT_QUERY_MAX_CHARS = 500; + +/** The opening of `text` within `max` characters, cut at a word break when one is near. */ +function cutAtWord(text: string, max: number): string { + if (text.length <= max) return text.trimEnd(); + const head = text.slice(0, max); + const lastBreak = head.search(/\s\S*$/); + return (lastBreak > max * 0.6 ? head.slice(0, lastBreak) : head).trimEnd(); +} + +/** + * A question the knowledge base will take. It refuses anything over 500 characters + * ("Query is too long"), and agents writing a detailed question hit that: two refusals on + * 4 Nov alone. Agents put the question itself last, after the context, so a longer one + * keeps its last sentence and as much of its opening as fits around it. + */ +function fitExpertQuery(query: string): { query: string; shortened: boolean } { + if (query.length <= EXPERT_QUERY_MAX_CHARS) + return { query, shortened: false }; + const text = query.trim(); + const sentences = text.match(/[^.?!]+(?:[.?!]+["')\]]*\s*|$)/g) ?? [text]; + const last = sentences[sentences.length - 1].trim(); + const gap = " … "; + if (sentences.length < 2 || last.length > EXPERT_QUERY_MAX_CHARS * 0.6) { + return { query: cutAtWord(text, EXPERT_QUERY_MAX_CHARS), shortened: true }; + } + const opening = text.slice(0, text.length - last.length); + const room = EXPERT_QUERY_MAX_CHARS - last.length - gap.length; + return { query: `${cutAtWord(opening, room)}${gap}${last}`, shortened: true }; +} + async function executeAccessibilityRAG( args: { query: string }, server: McpServer, @@ -162,7 +222,19 @@ async function executeAccessibilityRAG( undefined, config, ); - return await queryAccessibilityRAG(args.query, config); + const { query, shortened } = fitExpertQuery(args.query); + const answer = await queryAccessibilityRAG(query, config); + if (!shortened) return answer; + return { + ...answer, + content: [ + { + type: "text", + text: `The question was ${args.query.length} characters; the knowledge base takes at most ${EXPERT_QUERY_MAX_CHARS}, so ${query.length} were sent: its opening and its last sentence. If the answer misses the point, ask again more briefly.`, + }, + ...(answer.content ?? []), + ], + }; } catch (error) { return handleMCPError("accessibilityExpert", server, config, error); } @@ -302,10 +374,6 @@ function excludedIssuesNote( return notes; } -/** How often, and for how long, a first read waits on a report that is still processing. */ -export const REPORT_STATUS_POLL_MS = 10_000; -export const REPORT_STATUS_WAIT_MS = 5 * 60 * 1000; - /** Statuses during which a report's issues are still being added (`rule_processing`: an app build's). */ const PROCESSING_STATUSES = new Set([ "queued", @@ -317,54 +385,22 @@ const PROCESSING_STATUSES = new Set([ ]); /** - * Of those, the ones worth waiting on: the processing after the run (a web scan's advanced - * issues, an app build's rules), which ends within minutes. A report whose scan or tests are still running is read as it is, - * with a note, since that can last as long as the suite does. + * Of those, the processing after the run: a web report's advanced issues while "finalizing", + * an app build's rules while "rule_processing". It usually ends within a few minutes, but a + * first read no longer waits for it inside the call. It used to wait up to five minutes and + * then read the export, and a client cuts off a call that long: the claude.ai connector + * aborts a tool call silent for 305 s, progress notifications or not. Every such call left + * the agent with nothing, and one reported a fix as verified that had added 26 issues. So + * the read says the report is not ready, and to call again, which any client handles. */ -const WAIT_STATUSES = new Set(["finalizing", "rule_processing"]); +const FINALIZING_STATUSES = new Set(["finalizing", "rule_processing"]); + +/** Website Scanner run statuses that leave no results to read. */ +const FAILED_SCAN_STATUSES = new Set(["failed", "cancelled"]); interface ProcessingState { - /** The status when the issues were read; undefined when it could not be read. */ + /** The report's status when its issues were read; undefined when it could not be read. */ status?: string; - /** The status first seen, before any waiting. */ - firstStatus?: string; - waitedMs: number; -} - -/** - * Where a report is in its processing before its issues are read. Issues keep being added - * until it completes; an advanced scan adds its advanced-rule issues while "finalizing", for - * minutes after the tests end, so a first read waits for that, reporting progress, for up to - * REPORT_STATUS_WAIT_MS. A run still in progress is not waited on (it can take as long as - * the suite), nor is a later page, nor a status that cannot be read. - */ -async function waitForProcessing( - reportFetcher: AccessibilityReportFetcher, - source: IssueSource, - wait: boolean, - context: ScanProgressContext | undefined, -): Promise { - const started = Date.now(); - const firstStatus = await reportFetcher.reportStatus(source); - let status = firstStatus; - while ( - wait && - status && - WAIT_STATUSES.has(status) && - Date.now() - started < REPORT_STATUS_WAIT_MS - ) { - const waited = Date.now() - started; - if (context) { - await notifyScanProgress( - context, - `The report is still ${status}; waiting for it to complete (${formatDuration(waited)} of up to ${formatDuration(REPORT_STATUS_WAIT_MS)}).`, - Math.min(99, Math.round((waited / REPORT_STATUS_WAIT_MS) * 100)), - ); - } - await new Promise((resolve) => setTimeout(resolve, REPORT_STATUS_POLL_MS)); - status = await reportFetcher.reportStatus(source); - } - return { status, firstStatus, waitedMs: Date.now() - started }; } function formatDuration(ms: number): string { @@ -374,27 +410,38 @@ function formatDuration(ms: number): string { : `${Math.floor(seconds / 60)}m ${String(seconds % 60).padStart(2, "0")}s`; } -/** What the answer says about the report's processing, if anything. */ -function processingNote({ - status, - firstStatus, - waitedMs, -}: ProcessingState): string[] { - if (status && PROCESSING_STATUSES.has(status)) { - const meaning = - status === "finalizing" - ? ", which on an advanced scan means advanced issue processing" - : ""; - return [ - `⚠️ This report is still processing (status: ${status}${meaning}). The issues below are only those found so far: more will be added and the total will grow. Call fetchAccessibilityIssues again in a few minutes for the complete list.`, - ]; - } - if (firstStatus && PROCESSING_STATUSES.has(firstStatus)) { - return [ - `The report was still processing (status: ${firstStatus}); waited ${formatDuration(waitedMs)} for it to complete before reading its issues.`, - ]; - } - return []; +/** What the answer says about a report read while it is still processing, if anything. */ +function processingNote({ status }: ProcessingState): string[] { + if (!status || !PROCESSING_STATUSES.has(status)) return []; + const meaning = + status === "finalizing" + ? ", which on an advanced scan means advanced issue processing" + : ""; + return [ + `⚠️ This report is still processing (status: ${status}${meaning}). The issues below are only those found so far: more will be added and the total will grow. Call fetchAccessibilityIssues again in a few minutes for the complete list.`, + ]; +} + +/** + * The answer for a first read of a report still finalizing: not ready yet, and when to come + * back. Not an error, since nothing failed, but it says outright that no issues were read, + * so an empty answer cannot be taken for a clean page. + */ +function stillFinalizingResponse( + source: IssueSource, + status: string, + preface: string[], + extraArgs: string, +): CallToolResult { + const state = + status === "rule_processing" + ? "is still running its accessibility rules (status: rule_processing)" + : "is still finalizing (status: finalizing): BrowserStack is still adding its issues"; + return createSuccessResponse([ + ...preface, + `⏳ The report for ${describeSource(source)} ${state}. This usually takes a few minutes after the run ends. No issues have been read yet, so this is not a result: do not tell the user the page is clean or the fix is verified.`, + `Call fetchAccessibilityIssues again with ${sourceArgs(source)}${extraArgs} in a minute or two. If it still says this after about 10 minutes, tell the user the report is stuck instead of calling again.`, + ]); } /** The last line: how to get the next page, or that there is none. */ @@ -968,12 +1015,9 @@ async function fetchAccessibilityIssues( `compareWith compares Website Scanner runs and App Accessibility builds; ${describeSource(source)} is neither.`, ); } - const processing = await waitForProcessing( - reportFetcher, - source, - cursor === 0, - context, - ); + const processing: ProcessingState = { + status: await reportFetcher.reportStatus(source), + }; const stillProcessing = processing.status !== undefined && PROCESSING_STATUSES.has(processing.status); @@ -981,6 +1025,29 @@ async function fetchAccessibilityIssues( requested.kind === "link" ? [`Resolved the report link to ${describeSource(source)}.`] : []; + const pageArg = pageUrl ? `, pageUrl: "${pageUrl}"` : ""; + + if ( + source.kind === "scan" && + processing.status && + FAILED_SCAN_STATUSES.has(processing.status) + ) { + return failedScanRunResponse(source, processing.status, config, preface); + } + if ( + cursor === 0 && + processing.status && + FINALIZING_STATUSES.has(processing.status) + ) { + const compareArg = + compareWith !== undefined ? `, compareWith: "${compareWith}"` : ""; + return stillFinalizingResponse( + source, + processing.status, + preface, + `${pageArg}${compareArg}`, + ); + } if (source.kind === "appBuild" && compareWith !== undefined) { return compareAppBuilds( @@ -1017,7 +1084,6 @@ async function fetchAccessibilityIssues( ); } - const pageArg = pageUrl ? `, pageUrl: "${pageUrl}"` : ""; let reportLink: string; try { reportLink = await reportFetcher.getReportLink(source, { @@ -1520,21 +1586,10 @@ function renameReason( /** How many failed pages a failure result lists before summing up the rest. */ const FAILED_PAGES_LISTED = 10; -/** - * A failed run, with the reason the API gives for it: the run's own and each page's. It used - * to say only "The dashboard says why", which left an agent nothing to tell the user. - */ -function createScanFailureResponse( - name: string, - status: string, - scanId: string, - scanRunId: string, - dashboardLink: string | undefined, - details: ScanFailureDetails = { pages: [] }, -): CallToolResult { - const { reason, pages } = details; +/** Why a run failed, as the API gives it: the run's own reason and each page's, if any. */ +function failureReasons({ reason, pages }: ScanFailureDetails): string[] { const listed = pages.slice(0, FAILED_PAGES_LISTED); - const why = [ + return [ reason ? `Why: ${reason}${/[.!?]$/.test(reason) ? "" : "."}` : "", pages.length ? [ @@ -1546,6 +1601,57 @@ function createScanFailureResponse( ].join("\n") : "", ].filter(Boolean); +} + +/** + * A read of a scan run that failed or was cancelled. Its export is empty, and this used to + * read "Retrieved 0 accessibility issues … All issues retrieved", so an agent could call a + * page clean that was never scanned. It says the run failed and why, as startAccessibilityScan + * does when the scan fails while it waits. + */ +async function failedScanRunResponse( + source: Extract, + status: string, + config: BrowserStackConfig, + preface: string[], +): Promise { + const { scanId, scanRunId } = source; + const scanner = await initializeScanner(config); + const [dashboardLink, details] = await Promise.all([ + scanner.dashboardLink(scanId), + status === "failed" + ? scanner.failureDetails(scanId, scanRunId) + : Promise.resolve({ pages: [] }), + ]); + const why = failureReasons(details); + return createErrorResponse( + [ + ...preface, + `❌ ${describeSource(source)} ${status === "cancelled" ? "was cancelled" : "failed"} (status: ${status}), so it has no accessibility results. An empty issue list here does not mean the page is clean: do not tell the user it passed or that a fix is verified.`, + ...(why.length + ? why + : status === "failed" + ? ["The API gives no reason; the dashboard may say why."] + : []), + scanDashboardNote(dashboardLink, scanId), + "Fix the cause and start a new scan with startAccessibilityScan, then read that one.", + ].join("\n"), + ); +} + +/** + * A failed run, with the reason the API gives for it: the run's own and each page's. It used + * to say only "The dashboard says why", which left an agent nothing to tell the user. + */ +function createScanFailureResponse( + name: string, + status: string, + scanId: string, + scanRunId: string, + dashboardLink: string | undefined, + details: ScanFailureDetails = { pages: [] }, +): CallToolResult { + const why = failureReasons(details); return createErrorResponse( [ `❌ Accessibility scan "${name}" failed with status: ${status}. Scan ID: ${scanId} and Scan Run ID: ${scanRunId}.`, diff --git a/tests/tools/accessibilityAccountRefusals.test.ts b/tests/tools/accessibilityAccountRefusals.test.ts new file mode 100644 index 00000000..0f8bf17e --- /dev/null +++ b/tests/tools/accessibilityAccountRefusals.test.ts @@ -0,0 +1,273 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// Refusals the user resolves on their own account (plan limits, a Website Scanner user not +// set up yet) are explained rather than reported as faults, and accessibilityExpert keeps +// questions within the knowledge base's 500-character limit. API messages are as logged on +// 4 Nov 2026. + +const m = vi.hoisted(() => ({ + startScan: vi.fn(), + waitUntilComplete: vi.fn(), + getReportLink: vi.fn(), + sourceFromReportUrl: vi.fn(), + reportStatus: vi.fn(), + // A build id is probed on both APIs; by default it stays what it was named as. + resolveBuild: vi.fn(async (source: unknown) => source), + appBuildReport: vi.fn(), + appReportIssues: vi.fn(), + parse: vi.fn(), + dashboardLink: vi.fn(), + failureDetails: vi.fn(async () => ({ pages: [] })), + rag: vi.fn(), +})); + +vi.mock("../../src/tools/accessiblity-utils/accessibility-rag.js", () => ({ + queryAccessibilityRAG: m.rag, +})); + +vi.mock("../../src/tools/accessiblity-utils/scanner.js", () => ({ + AccessibilityScanner: class { + setAuth = vi.fn(); + startScan = m.startScan; + waitUntilComplete = m.waitUntilComplete; + runSettings = vi.fn().mockResolvedValue(undefined); + dashboardLink = m.dashboardLink; + failureDetails = m.failureDetails; + }, + ScanTimeoutError: class extends Error { + constructor(readonly waitedMs: number) { + super("still running"); + } + }, +})); +vi.mock("../../src/tools/accessiblity-utils/report-fetcher.js", () => ({ + AccessibilityReportFetcher: class { + setAuth = vi.fn(); + getReportLink = m.getReportLink; + sourceFromReportUrl = m.sourceFromReportUrl; + reportStatus = m.reportStatus; + resolveBuild = m.resolveBuild; + appBuildReport = m.appBuildReport; + appReportIssues = m.appReportIssues; + }, + EXPORT_WAIT_MS: 45_000, + ExportPendingError: class extends Error { + constructor( + readonly taskId: string, + readonly waitedMs: number, + ) { + super("still generating"); + } + }, +})); +vi.mock("../../src/tools/accessiblity-utils/report-parser.js", () => ({ + parseAccessibilityReportFromCSV: m.parse, +})); +vi.mock("../../src/lib/get-auth.js", () => ({ + getBrowserStackAuth: vi.fn().mockReturnValue("fake-user:fake-key"), +})); +vi.mock("../../src/logger.js", () => ({ + default: { error: vi.fn(), info: vi.fn(), debug: vi.fn(), warn: vi.fn() }, +})); +vi.mock("../../src/lib/instrumentation.js", () => ({ trackMCP: vi.fn() })); +// apiClient reaches the server entry through lib/utils; it must not start the server. +vi.mock("../../src/index.js", () => ({ + trackMCP: vi.fn(), + PackageJsonVersion: "0.0.0", +})); + +import addAccessibilityTools, { + EXPERT_QUERY_MAX_CHARS, +} from "../../src/tools/accessibility.js"; +import { + FEEDBACK_HINT, + withFeedbackHint, +} from "../../src/lib/feedback-hint.js"; + +const CONFIG = { + "browserstack-username": "fake-user", + "browserstack-access-key": "fake-key", +}; + +function register() { + const handlers: Record any> = {}; + const server: any = { + tool: vi.fn((...args: any[]) => { + handlers[args[0]] = args[args.length - 1]; + }), + server: { + getClientVersion: vi.fn().mockReturnValue({ version: "1.0" }), + getClientCapabilities: vi.fn().mockReturnValue({}), + }, + }; + addAccessibilityTools(server, CONFIG); + return handlers; +} + +const text = (result: any) => + result.content.map((c: { text: string }) => c.text).join("\n"); + +const CONTEXT = { sendNotification: vi.fn(), _meta: {} }; + +describe("refusals the user resolves on their own account", () => { + let handlers: Record any>; + + beforeEach(() => { + vi.clearAllMocks(); + handlers = register(); + }); + + const startWith = async (apiMessage: string) => { + m.startScan.mockRejectedValue(new Error(apiMessage)); + return handlers.startAccessibilityScan( + { name: "home", pageURL: "https://site.example" }, + CONTEXT, + ); + }; + + it("explains the free plan's saved-scan limit, and says not to retry under another name", async () => { + const result = await startWith( + "Failed to start scan: You don't have access to make over 50 scans", + ); + + expect(result.isError).toBe(true); + const body = text(result); + expect(body).toContain( + "This account has reached its plan's limit of 50 saved Website Scanner scans", + ); + expect(body).toContain("delete scans that are no longer needed"); + expect(body).toContain("do not retry or start the scan under another name"); + expect(body).toContain( + "BrowserStack said: Failed to start scan: You don't have access to make over 50 scans", + ); + expect(body).not.toContain("open an issue on GitHub"); + }); + + it("explains a plan without the External APIs add-on", async () => { + const body = text( + await startWith( + "Failed to start scan: You do not have access to External APIs.", + ), + ); + + expect(body).toContain( + "does not include the External APIs add-on, which the MCP's accessibility tools need", + ); + expect(body).toContain( + "Scans can still be run and read in the Accessibility dashboard", + ); + expect(body).not.toContain("open an issue on GitHub"); + }); + + it("explains the add-on refusal on any accessibility tool, not only the scan", async () => { + m.resolveBuild.mockRejectedValueOnce( + new Error("You do not have access to External APIs."), + ); + const result = await handlers.fetchAccessibilityIssues( + { buildId: "q7m2k9x4c1v8b3n6z0w5r2t9y4p1s8d3f6g0h2ja" }, + CONTEXT, + ); + + expect(result.isError).toBe(true); + expect(text(result)).toContain("does not include the External APIs add-on"); + }); + + it("tells a user Website Scanner has not set up yet to open it once", async () => { + const body = text( + await startWith("Failed to start scan: user info missing"), + ); + + expect(body).toContain("Website Scanner has not set this user up yet"); + expect(body).toContain( + "open https://accessibility.browserstack.com/site-scanner once while signed in", + ); + expect(body).not.toContain("open an issue on GitHub"); + }); + + it("does not ask the agent to report these with submitFeedback", async () => { + for (const apiMessage of [ + "Failed to start scan: You don't have access to make over 50 scans", + "Failed to start scan: You do not have access to External APIs.", + "Failed to start scan: user info missing", + ]) { + const result = await startWith(apiMessage); + expect( + text(withFeedbackHint("startAccessibilityScan", result)), + ).not.toContain(FEEDBACK_HINT); + } + }); + + it("leaves other failures as they were, hint included", async () => { + const result = await startWith("Failed to start scan: socket hang up"); + + const body = text(result); + expect(body).toContain( + "Failed to start accessibility scan: Failed to start scan: socket hang up. Please open an issue on GitHub", + ); + expect(text(withFeedbackHint("startAccessibilityScan", result))).toContain( + FEEDBACK_HINT, + ); + }); +}); + +describe("accessibilityExpert and the knowledge base's 500-character limit", () => { + let handlers: Record any>; + const ANSWER = { content: [{ type: "text", text: "Use 4.5:1." }] }; + + beforeEach(() => { + vi.clearAllMocks(); + m.rag.mockResolvedValue(ANSWER); + handlers = register(); + }); + + it("passes a question within the limit as it is, with no note", async () => { + const result = await handlers.accessibilityExpert({ + query: "What contrast ratio does body text need?", + }); + + expect(m.rag).toHaveBeenCalledWith( + "What contrast ratio does body text need?", + CONFIG, + ); + expect(result).toEqual(ANSWER); + }); + + it("keeps the question's opening and its last sentence, and says so above the answer", async () => { + const context = + "The chart renders bars as SVG rect elements with role=img and an aria-label. ".repeat( + 8, + ); + const question = "Do the individual bars also need accessible names?"; + const query = context + question; // about 670 characters + const result = await handlers.accessibilityExpert({ query }); + + const sent: string = m.rag.mock.calls[0][0]; + expect(sent.length).toBeLessThanOrEqual(EXPERT_QUERY_MAX_CHARS); + expect(sent.endsWith(` … ${question}`)).toBe(true); + const opening = sent.slice(0, sent.indexOf(" … ")); + expect(query.startsWith(opening)).toBe(true); + expect(query[opening.length]).toMatch(/\s/); + const body = text(result); + expect(body).toContain( + `The question was ${query.length} characters; the knowledge base takes at most 500, so ${sent.length} were sent: its opening and its last sentence.`, + ); + expect(body).toContain("Use 4.5:1."); + }); + + it("keeps only the opening when the last sentence is itself too long to keep whole", async () => { + const query = "word ".repeat(150).trim() + "?"; // one 750-character sentence + await handlers.accessibilityExpert({ query }); + + const sent: string = m.rag.mock.calls[0][0]; + expect(sent.length).toBeLessThanOrEqual(EXPERT_QUERY_MAX_CHARS); + expect(query.startsWith(sent)).toBe(true); + expect(sent).not.toContain(" … "); + }); + + it("cuts a question with no word break near the limit at the limit itself", async () => { + const query = "x".repeat(700); + await handlers.accessibilityExpert({ query }); + + expect(m.rag.mock.calls[0][0]).toBe("x".repeat(EXPERT_QUERY_MAX_CHARS)); + }); +}); diff --git a/tests/tools/accessibilityScanOutput.test.ts b/tests/tools/accessibilityScanOutput.test.ts index bc1b0f83..588eeefe 100644 --- a/tests/tools/accessibilityScanOutput.test.ts +++ b/tests/tools/accessibilityScanOutput.test.ts @@ -69,10 +69,7 @@ vi.mock("../../src/index.js", () => ({ PackageJsonVersion: "0.0.0", })); -import addAccessibilityTools, { - REPORT_STATUS_POLL_MS, - REPORT_STATUS_WAIT_MS, -} from "../../src/tools/accessibility.js"; +import addAccessibilityTools from "../../src/tools/accessibility.js"; import { ExportPendingError } from "../../src/tools/accessiblity-utils/report-fetcher.js"; const CONFIG = { @@ -384,55 +381,68 @@ describe("fetchAccessibilityIssues and a report still processing", () => { expect(body).not.toContain("still processing"); }); - it("waits while the report is finalizing, then reads the complete issues", async () => { - m.reportStatus - .mockResolvedValueOnce("finalizing") - .mockResolvedValueOnce("finalizing") - .mockResolvedValueOnce("completed"); - const pending = handlers.fetchAccessibilityIssues( + it("does not wait while the report is finalizing: says to call again, without reading any issues", async () => { + m.reportStatus.mockResolvedValue("finalizing"); + const result = await handlers.fetchAccessibilityIssues( { buildId: BUILD_ID }, context, ); - await vi.advanceTimersByTimeAsync(REPORT_STATUS_POLL_MS * 2); - const result = await pending; - expect(m.reportStatus).toHaveBeenCalledTimes(3); - expect(m.getReportLink).toHaveBeenCalledTimes(1); - expect(sendNotification).toHaveBeenCalledWith( - expect.objectContaining({ - method: "notifications/progress", - params: expect.objectContaining({ - message: expect.stringContaining("still finalizing"), - }), - }), - ); + expect(m.reportStatus).toHaveBeenCalledTimes(1); + expect(m.getReportLink).not.toHaveBeenCalled(); + expect(m.parse).not.toHaveBeenCalled(); + expect(sendNotification).not.toHaveBeenCalled(); + expect(result.isError).toBeFalsy(); const body = text(result); expect(body).toContain( - "The report was still processing (status: finalizing); waited 20s for it to complete", + `The report for Automated Tests build ${BUILD_ID} is still finalizing (status: finalizing)`, ); - expect(body).toContain("✅ All issues retrieved."); + expect(body).toContain("No issues have been read yet"); + expect(body).toContain( + `Call fetchAccessibilityIssues again with buildId: "${BUILD_ID}" in a minute or two.`, + ); + expect(body).not.toContain("Retrieved"); + expect(body).not.toContain("All issues retrieved"); }); - it("says the issues are only those found so far when processing outlasts the wait", async () => { + it("says which build a report link resolved to, and to call again with that build id", async () => { + m.sourceFromReportUrl.mockResolvedValue({ + kind: "build", + buildId: BUILD_ID, + }); m.reportStatus.mockResolvedValue("finalizing"); - const pending = handlers.fetchAccessibilityIssues( - { buildId: BUILD_ID }, - context, + const body = text( + await handlers.fetchAccessibilityIssues( + { + reportUrl: `https://accessibility.browserstack.com/automated-tests/projects/p/builds/b/1?thBuildId=${BUILD_ID}`, + }, + context, + ), ); - await vi.advanceTimersByTimeAsync( - REPORT_STATUS_WAIT_MS + REPORT_STATUS_POLL_MS, + + expect(body).toContain( + `Resolved the report link to Automated Tests build ${BUILD_ID}.`, + ); + expect(body).toContain( + `Call fetchAccessibilityIssues again with buildId: "${BUILD_ID}" in a minute or two.`, + ); + }); + + it("keeps the page asked for in the call to make again", async () => { + m.reportStatus.mockResolvedValue("finalizing"); + const body = text( + await handlers.fetchAccessibilityIssues( + { buildId: BUILD_ID, pageUrl: "https://example.com/usage" }, + context, + ), ); - const result = await pending; - const body = text(result); expect(body).toContain( - "This report is still processing (status: finalizing", + `buildId: "${BUILD_ID}", pageUrl: "https://example.com/usage" in a minute or two.`, ); - expect(body).toContain("only those found so far"); - expect(body).not.toContain("✅ All issues retrieved."); }); - it("does not wait again for a later page, but still says the report is processing", async () => { + it("reads a later page as it is, saying the report is still processing", async () => { m.reportStatus.mockResolvedValue("finalizing"); const result = await handlers.fetchAccessibilityIssues( { buildId: BUILD_ID, cursor: 4400 }, @@ -444,8 +454,7 @@ describe("fetchAccessibilityIssues and a report still processing", () => { }); it("does not wait while the tests are still running, and says the issues are those found so far", async () => { - // A long suite can run for an hour; blocking the first read for five minutes and then - // returning partial results anyway serves nobody. Only post-run processing is waited on. + // A long suite can run for an hour, so a running report is read as it is, with a note. m.reportStatus.mockResolvedValue("running"); const result = await handlers.fetchAccessibilityIssues( @@ -600,22 +609,39 @@ describe("fetchAccessibilityIssues and App Accessibility builds", () => { expect(body.indexOf(LINK)).toBeGreaterThan(block); }); - it("waits while an app build's rules are still processing", async () => { - m.reportStatus - .mockResolvedValueOnce("rule_processing") - .mockResolvedValueOnce("failed"); + it("does not wait while an app build's rules are still processing: says to call again", async () => { + m.reportStatus.mockResolvedValue("rule_processing"); - const pending = handlers.fetchAccessibilityIssues( + const result = await handlers.fetchAccessibilityIssues( { buildId: BUILD_ID }, context, ); - await vi.advanceTimersByTimeAsync(REPORT_STATUS_POLL_MS); - const result = await pending; - expect(m.reportStatus).toHaveBeenCalledTimes(2); - expect(m.appBuildReport).toHaveBeenCalledTimes(1); - expect(text(result)).toContain( - "The report was still processing (status: rule_processing); waited", + expect(m.reportStatus).toHaveBeenCalledTimes(1); + expect(m.appBuildReport).not.toHaveBeenCalled(); + expect(result.isError).toBeFalsy(); + const body = text(result); + expect(body).toContain( + "is still running its accessibility rules (status: rule_processing)", + ); + expect(body).toContain("No issues have been read yet"); + expect(body).toContain( + `Call fetchAccessibilityIssues again with buildId: "${BUILD_ID}" in a minute or two.`, + ); + }); + + it("keeps compareWith in the call to make again when the build is still processing", async () => { + m.reportStatus.mockResolvedValue("rule_processing"); + + const body = text( + await handlers.fetchAccessibilityIssues( + { buildId: BUILD_ID, compareWith: "base-build-id" }, + context, + ), + ); + + expect(body).toContain( + `buildId: "${BUILD_ID}", compareWith: "base-build-id" in a minute or two.`, ); }); @@ -1076,3 +1102,91 @@ describe("what a web report's answer says about its issues", () => { expect(m.getReportLink).not.toHaveBeenCalled(); }); }); + +describe("fetchAccessibilityIssues on a scan run that failed", () => { + const SCAN = { scanId: "4300500", scanRunId: "2420030" }; + const DASHBOARD = + "https://accessibility.browserstack.com/website-scanner/scans/4300500"; + let handlers: Record any>; + + beforeEach(() => { + vi.clearAllMocks(); + m.dashboardLink.mockResolvedValue(DASHBOARD); + m.getReportLink.mockResolvedValue("https://csv"); + m.parse.mockResolvedValue({ + ...GROUPED, + records: [], + page_length: 0, + total_issues: 0, + next_page: null, + remaining: 0, + duplicate_issues: 0, + }); + handlers = register(); + }); + + it("says the run failed and why, instead of reading its empty export as 0 issues", async () => { + m.reportStatus.mockResolvedValue("failed"); + m.failureDetails.mockResolvedValue({ + reason: "The requested page is not reachable", + pages: [ + { + pageUrl: "http://bs-local.com:8202/after/home.html", + status: "failed", + reason: "Failed to start the test", + }, + ], + }); + + const result = await handlers.fetchAccessibilityIssues(SCAN, CONTEXT); + + expect(result.isError).toBe(true); + expect(m.failureDetails).toHaveBeenCalledWith("4300500", "2420030"); + expect(m.getReportLink).not.toHaveBeenCalled(); + expect(m.parse).not.toHaveBeenCalled(); + const body = text(result); + expect(body).toContain( + "❌ Website Scanner scan 4300500, run 2420030 failed (status: failed), so it has no accessibility results.", + ); + expect(body).toContain("does not mean the page is clean"); + expect(body).toContain("Why: The requested page is not reachable."); + expect(body).toContain( + "- http://bs-local.com:8202/after/home.html: Failed to start the test", + ); + expect(body).toContain(DASHBOARD); + expect(body).not.toContain("Retrieved 0"); + expect(body).not.toContain("All issues retrieved"); + }); + + it("still says the run failed when the API gives no reason", async () => { + m.reportStatus.mockResolvedValue("failed"); + m.failureDetails.mockResolvedValue({ pages: [] }); + + const body = text(await handlers.fetchAccessibilityIssues(SCAN, CONTEXT)); + + expect(body).toContain("failed (status: failed)"); + expect(body).toContain("The API gives no reason"); + }); + + it("says a cancelled run was cancelled", async () => { + m.reportStatus.mockResolvedValue("cancelled"); + + const result = await handlers.fetchAccessibilityIssues(SCAN, CONTEXT); + + expect(result.isError).toBe(true); + expect(m.failureDetails).not.toHaveBeenCalled(); + expect(text(result)).toContain( + "run 2420030 was cancelled (status: cancelled), so it has no accessibility results.", + ); + }); + + it("reads a completed run's issues as before", async () => { + m.reportStatus.mockResolvedValue("completed"); + + const result = await handlers.fetchAccessibilityIssues(SCAN, CONTEXT); + + expect(m.failureDetails).not.toHaveBeenCalled(); + expect(m.getReportLink).toHaveBeenCalledTimes(1); + expect(text(result)).toContain("Retrieved 0 accessibility issues"); + }); +});