diff --git a/src/tools/accessibility.ts b/src/tools/accessibility.ts index 2d6497b1..f79373fc 100644 --- a/src/tools/accessibility.ts +++ b/src/tools/accessibility.ts @@ -1,7 +1,10 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; -import { AccessibilityScanner } from "./accessiblity-utils/scanner.js"; +import { + AccessibilityScanner, + type ScanSettings, +} from "./accessiblity-utils/scanner.js"; import { AccessibilityReportFetcher, type IssueSource, @@ -416,13 +419,61 @@ async function fetchAccessibilityIssues( return createSuccessResponse(messages); } +/** The WCAG versions and levels Website Scanner checks against, and RGAA 4.1, by name. */ +const WCAG_VERSIONS = { + wcag2a: "WCAG 2.0 A", + wcag2aa: "WCAG 2.0 AA", + wcag2aaa: "WCAG 2.0 AAA", + wcag21a: "WCAG 2.1 A", + wcag21aa: "WCAG 2.1 AA", + wcag21aaa: "WCAG 2.1 AAA", + wcag22a: "WCAG 2.2 A", + wcag22aa: "WCAG 2.2 AA", + wcag22aaa: "WCAG 2.2 AAA", + rgaa41: "RGAA 4.1", +} as const; + +/** + * What the scan ran with, read back from the run, since an unset setting takes the API's + * default and the plan can turn advanced rules off. When it cannot be read, only what was + * asked for is said: a guessed default would be stated as fact. + */ +function scanSettingsNote( + asked: ScanSettings, + applied: ScanSettings | undefined, +): string { + const wcagName = (value: string) => + WCAG_VERSIONS[value as keyof typeof WCAG_VERSIONS] ?? value; + const onOff = (value: boolean) => (value ? "on" : "off"); + const describe = (settings: ScanSettings) => + [ + settings.wcagVersion !== undefined ? wcagName(settings.wcagVersion) : "", + settings.bestPractices !== undefined + ? `best practices ${onOff(settings.bestPractices)}` + : "", + settings.needsReview !== undefined + ? `needs review ${onOff(settings.needsReview)}` + : "", + settings.advancedRules !== undefined + ? `advanced rules ${onOff(settings.advancedRules)}` + : "", + ] + .filter(Boolean) + .join(", "); + if (applied && describe(applied)) { + return `Scan settings, as the scan ran: ${describe(applied)}.`; + } + return describe(asked) + ? `Scan settings asked for: ${describe(asked)}; anything not asked for takes the scan's default.` + : "No scan settings were asked for, so the scan's defaults apply."; +} + async function executeAccessibilityScan( args: { name: string; pageURL: string; authConfigId?: number; - advancedRules?: boolean; - }, + } & ScanSettings, context: ScanProgressContext, server: McpServer, config: BrowserStackConfig, @@ -440,7 +491,12 @@ async function executeAccessibilityScan( context, config, args.authConfigId, - args.advancedRules, + { + wcagVersion: args.wcagVersion, + bestPractices: args.bestPractices, + needsReview: args.needsReview, + advancedRules: args.advancedRules, + }, ); } catch (error) { return handleMCPError("startAccessibilityScan", server, config, error); @@ -583,10 +639,13 @@ function createScanSuccessResponse( reportUrl: string, cursor: number | null, duplicates: string[] = [], + settings: ScanSettings = {}, + applied?: ScanSettings, ): CallToolResult { const messages = [ `Accessibility scan "${name}" completed. check the BrowserStack dashboard for more details [https://scanner.browserstack.com/site-scanner/scan-details/${name}].`, `Scan ID: ${scanId} and Scan Run ID: ${scanRunId}`, + scanSettingsNote(settings, applied), `You can also download the full report from the following link: ${reportUrl}`, `We found ${totalIssues} issues. Below are the details of the ${pageLength} most critical issues.`, ...duplicates, @@ -608,7 +667,7 @@ async function runAccessibilityScan( context: ScanProgressContext, config: BrowserStackConfig, authConfigId?: number, - advancedRules?: boolean, + settings: ScanSettings = {}, ): Promise { const scanner = await initializeScanner(config); @@ -616,7 +675,7 @@ async function runAccessibilityScan( name, [pageURL], authConfigId, - advancedRules, + settings, ); const scanId = startResp.data!.id; const scanRunId = startResp.data!.scanRunId; @@ -628,6 +687,7 @@ async function runAccessibilityScan( return createScanFailureResponse(name, status); } + const applied = await scanner.runSettings(scanId, scanRunId); const reportFetcher = await initializeReportFetcher(config); const reportLink = await reportFetcher.getReportLink({ kind: "scan", @@ -655,6 +715,8 @@ async function runAccessibilityScan( reportLink, next_page, excludedIssuesNote(duplicate_issues, unlinked_duplicates, hidden_issues), + settings, + applied, ); } @@ -699,7 +761,23 @@ export default function addAccessibilityTools( advancedRules: z .boolean() .optional() - .describe("Enable advanced accessibility rules in the scan settings"), + .describe("Run the advanced accessibility rules (default true)."), + wcagVersion: z + .enum(Object.keys(WCAG_VERSIONS) as [keyof typeof WCAG_VERSIONS]) + .optional() + .describe( + 'WCAG version and level to check against, e.g. "wcag22aa", or "rgaa41" for RGAA 4.1 (default "wcag21aa").', + ), + bestPractices: z + .boolean() + .optional() + .describe("Also report best-practice issues (default false)."), + needsReview: z + .boolean() + .optional() + .describe( + 'Also report "needs review" issues to check by hand (default true).', + ), }, { title: "Start Accessibility Scan", diff --git a/src/tools/accessiblity-utils/scanner.ts b/src/tools/accessiblity-utils/scanner.ts index 953fdfec..67457d9e 100644 --- a/src/tools/accessiblity-utils/scanner.ts +++ b/src/tools/accessiblity-utils/scanner.ts @@ -20,6 +20,21 @@ export interface AccessibilityScanStatus { errors?: string[]; } +/** + * What a Website Scanner scan checks for, as the create-scan API takes it under + * scanSettings. A setting left out takes the API's default. + */ +export interface ScanSettings { + /** The WCAG version and level, such as "wcag22aa", or "rgaa41" (API default "wcag21aa"). */ + wcagVersion?: string; + /** Report best-practice issues too (API default false). */ + bestPractices?: boolean; + /** Report issues that need a manual review (API default true). */ + needsReview?: boolean; + /** Run the advanced rules (API default true). */ + advancedRules?: boolean; +} + export class AccessibilityScanner { private auth: { username: string; password: string } | undefined; @@ -31,7 +46,7 @@ export class AccessibilityScanner { name: string, urlList: string[], authConfigId?: number, - advancedRules?: boolean, + settings: ScanSettings = {}, ): Promise { if (!this.auth?.username || !this.auth?.password) { throw new Error( @@ -80,14 +95,15 @@ export class AccessibilityScanner { } }); + const scanSettings = Object.fromEntries( + Object.entries(settings).filter(([, value]) => value !== undefined), + ); const baseRequestBody = { name, urlList: transformedUrlList, recurring: false, ...(authConfigId && { authConfigId }), - ...(advancedRules !== undefined && { - scanSettings: { advancedRules }, - }), + ...(Object.keys(scanSettings).length > 0 && { scanSettings }), }; let requestBody = baseRequestBody; @@ -121,21 +137,62 @@ export class AccessibilityScanner { ); return data; } catch (err: any) { - // apiClient throws generic errors, try to extract message - if (err?.response?.status === 422) { + // apiClient throws generic errors, try to extract message. A 422 also refuses a + // setting (a WCAG version the API does not take), so its own reason comes first. + const data = err?.response?.data; + const reason = + data?.error || + data?.message || + (Array.isArray(data?.errors) ? data.errors.join(", ") : undefined); + if (err?.response?.status === 422 && !reason) { throw new Error( "A scan with this name already exists. please update the name and run again.", ); } - const msg = - err?.response?.data?.error || - err?.response?.data?.message || - err?.message || - String(err); - throw new Error(`Failed to start scan: ${msg}`); + throw new Error( + `Failed to start scan: ${reason || err?.message || String(err)}`, + ); } } + /** + * The settings a run actually ran with, from its summary, or undefined when it cannot be + * read. A setting left out takes the API's default, and the plan can turn advanced rules + * off, so what was asked for is not always what ran. + */ + async runSettings( + scanId: string, + scanRunId: string, + ): Promise { + const resp = await apiClient + .get({ + url: `https://api-accessibility.browserstack.com/api/website-scanner/v1/scans/${scanId}/scan_runs/${scanRunId}`, + headers: { + Authorization: + "Basic " + + Buffer.from( + `${this.auth?.username}:${this.auth?.password}`, + ).toString("base64"), + }, + raise_error: false, + }) + .catch(() => undefined); + const run = resp?.ok ? resp.data?.data : undefined; + if (!run) return undefined; + return { + ...(run.wcagVersion ? { wcagVersion: run.wcagVersion } : {}), + ...(typeof run.bestPractices === "boolean" + ? { bestPractices: run.bestPractices } + : {}), + ...(typeof run.needsReview === "boolean" + ? { needsReview: run.needsReview } + : {}), + ...(typeof run.advanced === "boolean" + ? { advancedRules: run.advanced } + : {}), + }; + } + async pollStatus( scanId: string, scanRunId: string, diff --git a/tests/tools/accessibilityScanOutput.test.ts b/tests/tools/accessibilityScanOutput.test.ts index a440d123..4898994a 100644 --- a/tests/tools/accessibilityScanOutput.test.ts +++ b/tests/tools/accessibilityScanOutput.test.ts @@ -17,6 +17,7 @@ vi.mock("../../src/tools/accessiblity-utils/scanner.js", () => ({ setAuth = vi.fn(); startScan = m.startScan; waitUntilComplete = m.waitUntilComplete; + runSettings = vi.fn().mockResolvedValue(undefined); }, })); vi.mock("../../src/tools/accessiblity-utils/report-fetcher.js", () => ({ diff --git a/tests/tools/accessibilityScanSettings.test.ts b/tests/tools/accessibilityScanSettings.test.ts new file mode 100644 index 00000000..32a62885 --- /dev/null +++ b/tests/tools/accessibilityScanSettings.test.ts @@ -0,0 +1,204 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// A Website Scanner scan can be asked for a WCAG version and level, best practices, "needs +// review" issues and advanced rules: the create-scan API takes them under scanSettings, +// and anything left out takes the API's default. + +const m = vi.hoisted(() => ({ + post: vi.fn(), + startScan: vi.fn(), + waitUntilComplete: vi.fn(), + runSettings: vi.fn(), + getReportLink: vi.fn(), + parse: vi.fn(), +})); + +vi.mock("../../src/lib/apiClient.js", () => ({ apiClient: { post: m.post } })); +vi.mock("../../src/lib/local.js", () => ({ + isLocalURL: () => false, + ensureLocalBinarySetup: vi.fn(), + killExistingBrowserStackLocalProcesses: vi.fn(), +})); +vi.mock("../../src/logger.js", () => ({ + default: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, +})); +vi.mock("../../src/lib/instrumentation.js", () => ({ trackMCP: vi.fn() })); +vi.mock("../../src/index.js", () => ({ trackMCP: vi.fn(), PackageJsonVersion: "0.0.0" })); + +describe("AccessibilityScanner.startScan", () => { + beforeEach(() => { + vi.resetModules(); + m.post.mockReset(); + m.post.mockResolvedValue({ data: { success: true, data: { id: "1", scanRunId: "2" } } }); + }); + + async function scanner() { + const { AccessibilityScanner } = await vi.importActual< + typeof import("../../src/tools/accessiblity-utils/scanner.js") + >("../../src/tools/accessiblity-utils/scanner.js"); + const s = new AccessibilityScanner(); + s.setAuth({ username: "u", password: "p" }); + return s; + } + + it("sends the WCAG version, best practices, needs review and advanced rules it is given", async () => { + await (await scanner()).startScan("checkout", ["https://shop.example/"], undefined, { + wcagVersion: "wcag22aaa", + bestPractices: true, + needsReview: false, + advancedRules: false, + }); + + expect(m.post.mock.calls[0][0].body.scanSettings).toEqual({ + wcagVersion: "wcag22aaa", + bestPractices: true, + needsReview: false, + advancedRules: false, + }); + }); + + it("sends only the settings it is given, so the rest take the API's defaults", async () => { + await (await scanner()).startScan("checkout", ["https://shop.example/"], undefined, { + bestPractices: true, + }); + + expect(m.post.mock.calls[0][0].body.scanSettings).toEqual({ bestPractices: true }); + }); + + it("passes on the API's own reason for a 422, rather than assuming a duplicate name", async () => { + m.post.mockRejectedValue({ + response: { status: 422, data: { message: "wcagVersion field must be one of: wcag21aa, wcag22aa" } }, + }); + + const start = (await scanner()).startScan("checkout", ["https://shop.example/"], undefined, { + wcagVersion: "rgaa41", + }); + + await expect(start).rejects.toThrow(/wcagVersion field must be one of/); + await expect(start).rejects.not.toThrow(/already exists/); + }); + + it("still reads a 422 with no reason as a name already in use", async () => { + m.post.mockRejectedValue({ response: { status: 422, data: {} } }); + + await expect( + (await scanner()).startScan("checkout", ["https://shop.example/"]), + ).rejects.toThrow(/A scan with this name already exists/); + }); + + it("sends no scanSettings when none are given", async () => { + await (await scanner()).startScan("checkout", ["https://shop.example/"]); + + expect(m.post.mock.calls[0][0].body).not.toHaveProperty("scanSettings"); + }); +}); + +describe("startAccessibilityScan scan settings", () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + vi.doMock("../../src/tools/accessiblity-utils/scanner.js", () => ({ + AccessibilityScanner: class { + setAuth = vi.fn(); + startScan = m.startScan; + waitUntilComplete = m.waitUntilComplete; + runSettings = m.runSettings; + }, + })); + vi.doMock("../../src/tools/accessiblity-utils/report-fetcher.js", () => ({ + AccessibilityReportFetcher: class { + setAuth = vi.fn(); + getReportLink = m.getReportLink; + }, + })); + vi.doMock("../../src/tools/accessiblity-utils/report-parser.js", () => ({ + parseAccessibilityReportFromCSV: m.parse, + })); + m.startScan.mockResolvedValue({ data: { id: "7310042", scanRunId: "5520318" } }); + m.waitUntilComplete.mockResolvedValue("completed"); + m.getReportLink.mockResolvedValue("https://csv"); + m.parse.mockResolvedValue({ + records: [], + page_length: 0, + total_issues: 0, + next_page: null, + duplicate_issues: null, + unlinked_duplicates: 0, + hidden_issues: null, + }); + }); + + async function tool() { + const { default: addAccessibilityTools } = await import("../../src/tools/accessibility.js"); + let handler: (...args: any[]) => any = () => undefined; + let schema: Record = {}; + const server: any = { + tool: vi.fn((...args: any[]) => { + if (args[0] === "startAccessibilityScan") { + schema = args[2]; + handler = args[args.length - 1]; + } + }), + server: { getClientVersion: vi.fn().mockReturnValue({ version: "1.0" }) }, + }; + addAccessibilityTools(server, { "browserstack-username": "u", "browserstack-access-key": "k" } as any); + return { schema, run: (args: Record) => handler(args, { sendNotification: vi.fn(), _meta: {} }) }; + } + + const text = (result: any) => result.content.map((c: { text: string }) => c.text).join("\n"); + + it("passes the requested settings to the scan and reports the settings it ran with", async () => { + m.runSettings.mockResolvedValue({ + wcagVersion: "wcag22aaa", + bestPractices: true, + needsReview: true, + advancedRules: false, + }); + const { run } = await tool(); + + const result = await run({ + name: "checkout", + pageURL: "https://shop.example/", + wcagVersion: "wcag22aaa", + bestPractices: true, + }); + + expect(m.startScan).toHaveBeenCalledWith("checkout", ["https://shop.example/"], undefined, { + wcagVersion: "wcag22aaa", + bestPractices: true, + }); + expect(m.runSettings).toHaveBeenCalledWith("7310042", "5520318"); + // Advanced rules read back off, as a plan without them turns them off. + expect(text(result)).toContain( + "Scan settings, as the scan ran: WCAG 2.2 AAA, best practices on, needs review on, advanced rules off.", + ); + }); + + it("says only what was asked for when the run's settings cannot be read", async () => { + m.runSettings.mockResolvedValue(undefined); + const { run } = await tool(); + + const result = await run({ + name: "checkout", + pageURL: "https://shop.example/", + wcagVersion: "wcag22aaa", + bestPractices: true, + }); + + expect(text(result)).toContain( + "Scan settings asked for: WCAG 2.2 AAA, best practices on; anything not asked for takes the scan's default.", + ); + expect(text(result)).not.toContain("(default)"); + }); + + it("takes only the WCAG versions the API knows, RGAA 4.1 among them", async () => { + const { schema } = await tool(); + + for (const ok of ["wcag2a", "wcag21aa", "wcag22aaa", "rgaa41"]) { + expect(schema.wcagVersion.safeParse(ok).success).toBe(true); + } + for (const bad of ["wcag22", "WCAG 2.2 AA", "wcag23aa"]) { + expect(schema.wcagVersion.safeParse(bad).success).toBe(false); + } + }); +});