From aafcaf45c87c3033687db44b8478a5078f5bb1a8 Mon Sep 17 00:00:00 2001 From: Arjun Chikara Date: Thu, 5 Nov 2026 16:21:04 +0530 Subject: [PATCH 1/4] feat(accessibility): remember the false positives a user confirms An agent working through accessibility issues with a user sorts true from false positives by asking the user, and the answer is lost when the session ends: the next report brings the same false positives back and the user is asked again. A proof of concept showed an agent given one well-described false positive (a design system's custom elements, under role-required) removes all 88 issues of that pattern from the next report, and only 9 when the record says no more than "it works for me". - fetchAccessibilityIssues now brings the account's saved false positives for the rules of the issues it answers with, with how to apply them (same rule, same site unless global, match the pattern not the selector, never remove a counter-example, ask before widening) and how to save a new one (inspect the element; describe the pattern and its widest confirmed scope). A memory that cannot be read, or an account without it, leaves the answer as before. - Saving, reading and revoking are web_accessibility capabilities (submit_false_positive_feedback, get_false_positive_memory, revoke_false_positive_feedback) rather than tools: the catalog's description total is at its ceiling, and these need the guidance only when used. - The memory takes at most 24,000 characters of an answer; the fullest rule's oldest entries are left out first, counted, and readable through the capability. Backend: the Accessibility API's /api/fp-memory/v1 endpoints (ATA-1564). Co-Authored-By: Claude Opus 5.5 --- .../web_accessibility.capability-index.json | 428 +++++++++++++++++- src/tools/accessibility.ts | 11 + src/tools/accessiblity-utils/fp-memory.ts | 168 +++++++ tests/setup.ts | 10 + .../tools/capabilityWebAccessibility.test.ts | 21 +- tests/tools/fpMemory.test.ts | 213 +++++++++ 6 files changed, 843 insertions(+), 8 deletions(-) create mode 100644 src/tools/accessiblity-utils/fp-memory.ts create mode 100644 tests/tools/fpMemory.test.ts diff --git a/capability/web_accessibility.capability-index.json b/capability/web_accessibility.capability-index.json index 3b3aa5cf..78f15c92 100644 --- a/capability/web_accessibility.capability-index.json +++ b/capability/web_accessibility.capability-index.json @@ -3,7 +3,7 @@ "version": "1.0", "build_id": "web_accessibility-v1.0-lists", "web_accessibility": { - "summary": "BrowserStack Web Accessibility: WCAG accessibility reports for websites and web apps. Find the account's Workflow Analyzer reports from the browser extension, Website Scanner scans with their scan history, Automated Tests projects and builds from web tests executed with accessibility checks on, and Assisted Test reports, each with the id, creator, status and issue counts needed to pick one. Each list names the id the accessibility issue tool reads. These capabilities list reports; they do not read a report's individual issues, start scans, or change or delete anything. Mobile apps are covered by a separate product.", + "summary": "BrowserStack Web Accessibility: WCAG accessibility reports for websites and web apps. Find the account's Workflow Analyzer reports from the browser extension, Website Scanner scans with their scan history, Automated Tests projects and builds from web tests executed with accessibility checks on, and Assisted Test reports, each with the id, creator, status and issue counts needed to pick one. Each list names the id the accessibility issue tool reads. These capabilities list reports; they do not read a report's individual issues or start scans. They also keep the account's false-positive memory: issues users confirmed are not real, saved per rule so later reports filter them without asking again. Mobile apps are covered by a separate product.", "routing_terms": [ "web accessibility", "website accessibility", @@ -12,7 +12,10 @@ "site scanner", "workflow analyzer", "assisted test", - "automated tests accessibility" + "automated tests accessibility", + "false positive", + "not an issue", + "false positive memory" ], "auth": { "type": "http", @@ -411,6 +414,391 @@ "$response": "UpstreamFailure" } } + }, + { + "name": "submit_false_positive_feedback", + "method": "POST", + "path": "/api/fp-memory/v1/feedback", + "mode": "write", + "entity": "false_positive_feedback", + "body": [ + { + "name": "ruleCode", + "type": "string", + "required": true, + "description": "The issue's issue_type from fetchAccessibilityIssues, e.g. role-required.", + "example": "role-required" + }, + { + "name": "pageUrl", + "type": "string", + "required": true, + "description": "The issue's affected_page, an absolute URL; its host is the site the entry applies to." + }, + { + "name": "component", + "type": "string", + "description": "The issue's component, as fetchAccessibilityIssues gave it." + }, + { + "name": "htmlSnippet", + "type": "string", + "description": "The issue's HTML_snippet, as fetchAccessibilityIssues gave it." + }, + { + "name": "verdict", + "type": "string", + "required": true, + "values": [ + "false_positive", + "not_applicable", + "accepted_risk" + ], + "description": "false_positive unless the user says the issue is real but does not apply, or is a risk they accept." + }, + { + "name": "scope", + "type": "string", + "required": true, + "values": [ + "element", + "component", + "component_family", + "site_predicate" + ], + "description": "The widest set of elements the user confirmed the root cause holds for. element alone recalls almost nothing on the next report." + }, + { + "name": "rootCause", + "type": "string", + "required": true, + "minLength": 20, + "description": "Why the rule is wrong, in terms of how the element is built (e.g. its role is set inside its shadow root), not only that it works for the user." + }, + { + "name": "pattern", + "type": "object", + "required": true, + "description": "What identifies the element and its kind. predicate is required; add what you observed.", + "fields": [ + { + "name": "predicate", + "type": "string", + "required": true, + "minLength": 10 + }, + { + "name": "tag", + "type": "string" + }, + { + "name": "tagPrefix", + "type": "string" + }, + { + "name": "isCustomElement", + "type": "boolean" + }, + { + "name": "hasShadowRoot", + "type": "boolean" + }, + { + "name": "classes", + "type": "array" + }, + { + "name": "designSystem", + "type": "string" + } + ] + }, + { + "name": "evidence", + "type": "object", + "description": "What you checked, as {observations: [...]}: computed role, shadow DOM content, screen reader output." + }, + { + "name": "counterExamples", + "type": "array", + "maxItems": 50, + "description": "Similar-looking elements the user says ARE real issues, as short strings; they are never removed." + }, + { + "name": "userStatement", + "type": "string", + "required": true, + "description": "The user's own words, as they said them." + }, + { + "name": "global", + "type": "boolean", + "default": false, + "description": "True only when the user says it holds on all the account's sites; otherwise it applies to pageUrl's site." + }, + { + "name": "product", + "type": "string", + "values": [ + "website-scanner", + "automated-tests", + "workflow-analyzer" + ], + "description": "The product of the report the issue came from." + }, + { + "name": "reportReference", + "type": "string", + "description": "The report the issue came from: its scanRunId, buildId or workflowReportId." + } + ], + "intent": "Save an accessibility issue the user confirmed is a false positive, so later fetchAccessibilityIssues answers filter the same pattern without asking the user again.", + "guidance": [ + "Before saving, inspect the element yourself (open the page if you can, else read the issue's HTML) and write rootCause and pattern.predicate from what you saw: a record that only says the element works for the user matches almost nothing on the next report.", + "Propose the widest scope the root cause supports (e.g. component_family for every custom element of one design system) and have the user confirm it; ask them for any similar element that is a real issue and send it in counterExamples.", + "Send the issue's fields as fetchAccessibilityIssues gave them; selectors are not taken, as they differ per page.", + "warnings in the reply ask you to re-check the scope with the user; resend at the wider scope if they agree." + ], + "returns": [ + "data.id", + "data.ruleCode", + "data.docsUrl", + "data.site", + "data.global", + "data.activeEntriesForRule", + "data.warnings" + ], + "responses": { + "201": { + "description": "Saved.", + "schema": { + "type": "object", + "properties": { + "success": { + "type": "boolean" + }, + "data": { + "type": "object", + "properties": { + "id": { + "type": "integer" + }, + "ruleCode": { + "type": "string" + }, + "docsUrl": { + "type": "string" + }, + "site": { + "type": "string" + }, + "global": { + "type": "boolean" + }, + "activeEntriesForRule": { + "type": "integer" + }, + "warnings": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + } + }, + "401": { + "$response": "Unauthorized" + }, + "403": { + "$response": "FpMemoryDisabled" + }, + "422": { + "$response": "FpFeedbackRejected" + }, + "500": { + "$response": "InternalServerError" + } + } + }, + { + "name": "get_false_positive_memory", + "method": "GET", + "path": "/api/fp-memory/v1/memory", + "mode": "read", + "entity": "false_positive_feedback", + "query": [ + { + "name": "ruleCodes", + "type": "string", + "description": "Comma-separated rule codes (issue_type) to read, at most 200; omit for every rule.", + "example": "role-required,image-alt" + }, + { + "name": "site", + "type": "string", + "description": "A host or page URL: only that site's entries, plus those saved for all sites." + } + ], + "intent": "Read the account's saved false positives, grouped by rule with each rule's docs link, to filter accessibility issues with or to find an entry's id.", + "guidance": [ + "fetchAccessibilityIssues already brings the memory for its issues' rules; read here only when it says some entries did not fit, or to find an entry to revoke.", + "Apply an entry only to issues of the same rule, on its site unless global, matching its pattern and rootCause, never the selector." + ], + "returns": [ + "data.rules.ruleCode", + "data.rules.docsUrl", + "data.rules.entryCount", + "data.rules.entries", + "data.truncated" + ], + "responses": { + "200": { + "description": "The memory, grouped by rule; newest entries first, at most 5000.", + "schema": { + "type": "object", + "properties": { + "success": { + "type": "boolean" + }, + "data": { + "type": "object", + "properties": { + "truncated": { + "type": "boolean" + }, + "rules": { + "type": "array", + "items": { + "type": "object", + "properties": { + "ruleCode": { + "type": "string" + }, + "docsUrl": { + "type": "string" + }, + "entryCount": { + "type": "integer" + }, + "entries": { + "type": "array", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer" + }, + "site": { + "type": "string" + }, + "global": { + "type": "boolean" + }, + "verdict": { + "type": "string" + }, + "matchScope": { + "type": "string" + }, + "rootCause": { + "type": "string" + }, + "pattern": { + "type": "object" + }, + "counterExamples": { + "type": "array" + }, + "userStatement": { + "type": "string" + }, + "seedIssue": { + "type": "object" + } + } + } + } + } + } + } + } + } + } + } + }, + "401": { + "$response": "Unauthorized" + }, + "403": { + "$response": "FpMemoryDisabled" + }, + "500": { + "$response": "InternalServerError" + } + } + }, + { + "name": "revoke_false_positive_feedback", + "method": "POST", + "path": "/api/fp-memory/v1/feedback/{id}/revoke", + "mode": "write", + "entity": "false_positive_feedback", + "path_params": [ + { + "name": "id", + "type": "integer", + "required": true, + "minimum": 1, + "description": "The entry's id, from the memory or the save reply." + } + ], + "intent": "Remove a saved false positive the user no longer agrees with, so the issues it covered show again in later reports.", + "guidance": [ + "Revoked entries are kept on the account's side but no longer returned or applied; there is no undo, so save a new entry instead if the user changes their mind." + ], + "returns": [ + "data.id", + "data.status" + ], + "responses": { + "200": { + "description": "Revoked.", + "schema": { + "type": "object", + "properties": { + "success": { + "type": "boolean" + }, + "data": { + "type": "object", + "properties": { + "id": { + "type": "integer" + }, + "status": { + "type": "string" + } + } + } + } + } + }, + "401": { + "$response": "Unauthorized" + }, + "403": { + "$response": "FpMemoryDisabled" + }, + "404": { + "description": "No active entry with this id in the account." + }, + "500": { + "$response": "InternalServerError" + } + } } ], "entities": { @@ -569,6 +957,30 @@ "key_facts": [ "Its individual issues are read on the Accessibility dashboard; the list gives its counts." ] + }, + "false_positive_feedback": { + "entity": "false_positive_feedback", + "title": "False-positive feedback", + "description": "One issue pattern a user confirmed is not a real problem, saved per rule to filter it from later reports.", + "aliases": [ + "false positive", + "false positive memory", + "not an issue", + "fp feedback", + "ignored issue pattern" + ], + "id_convention": "A plain integer, returned when the feedback is saved and on each entry of the memory.", + "parents": [], + "relations": [], + "capabilities": [ + "submit_false_positive_feedback", + "get_false_positive_memory", + "revoke_false_positive_feedback" + ], + "key_facts": [ + "The memory belongs to the account: every user's agent reads it. An entry applies to the site it was saved on, or to all sites when global.", + "fetchAccessibilityIssues already brings the memory for the rules of the issues it answers with." + ] } }, "schemas": { @@ -1268,6 +1680,18 @@ "schema": { "$schema": "Failure" } + }, + "FpMemoryDisabled": { + "description": "The caller's role cannot act on issues, or false-positive memory is not enabled for this account (error says which).", + "schema": { + "$schema": "Failure" + } + }, + "FpFeedbackRejected": { + "description": "The feedback was not saved: unknown rule, a root cause or predicate too thin to match later issues by, a pageUrl without a host, or the rule already holds 1000 entries. error says which.", + "schema": { + "$schema": "Failure" + } } } } diff --git a/src/tools/accessibility.ts b/src/tools/accessibility.ts index 2936bc04..a50c277c 100644 --- a/src/tools/accessibility.ts +++ b/src/tools/accessibility.ts @@ -36,6 +36,10 @@ import { BrowserStackConfig } from "../lib/types.js"; import { elicitCredentialsIfSupported } from "../lib/elicit-credentials.js"; import logger from "../logger.js"; import { NEEDS_A11Y_CONFIG_ID, NEEDS_A11Y_SCAN_ID } from "./tool-handoff.js"; +import { + memoryForReport, + memoryMessages, +} from "./accessiblity-utils/fp-memory.js"; interface AuthCredentials { username: string; @@ -1048,6 +1052,12 @@ async function fetchAccessibilityIssues( nextPage: cursor, pageUrl, }); + // The memory for this page's rules comes with the issues, so it is applied whether or + // not the agent thinks to ask for it. + const memory = await memoryForReport( + setupAuth(config), + Object.keys(rules ?? {}), + ); return createSuccessResponse([ ...preface, @@ -1059,6 +1069,7 @@ async function fetchAccessibilityIssues( ...excludedIssuesNote(duplicate_issues, unlinked_duplicates, hidden_issues), fullReportLine(reportLink), WEB_ISSUES_NOTE, + ...memoryMessages(memory), `Issues: ${wrapUntrusted("accessibility scan results", JSON.stringify({ issues: records, rules }))}`, closingMessage(next_page, remaining, source, stillProcessing, pageArg), ]); diff --git a/src/tools/accessiblity-utils/fp-memory.ts b/src/tools/accessiblity-utils/fp-memory.ts new file mode 100644 index 00000000..a7c8f1cf --- /dev/null +++ b/src/tools/accessiblity-utils/fp-memory.ts @@ -0,0 +1,168 @@ +import { apiClient } from "../../lib/apiClient.js"; +import { getRegionalHost } from "../../lib/regional-host.js"; +import { wrapUntrusted } from "../../lib/untrusted-content.js"; +import logger from "../../logger.js"; + +/** + * The account's false-positive (FP) memory: what users and their agents said is not a real + * issue, saved per rule so later reports can be filtered without asking the user again. + * + * Saving, reading and revoking entries are web_accessibility capabilities (no tool of their + * own, so the catalog stays within its description budget). This module only reads the + * memory for the rules of the issues fetchAccessibilityIssues is answering with, so it + * arrives with them whether or not the agent thinks to ask for it. + */ + +/** The capabilities an answer names: keep in step with web_accessibility's index. */ +export const SUBMIT_CAPABILITY = "submit_false_positive_feedback"; +export const READ_CAPABILITY = "get_false_positive_memory"; + +/** A read made while answering with issues must not hold the issues back for long. */ +const MEMORY_READ_TIMEOUT_MS = 8_000; + +/** How much of an answer the memory may take; a rule's oldest entries go first. */ +export const MEMORY_CHARACTERS = 24_000; +const SNIPPET_CHARACTERS = 200; + +export interface FpEntry { + id: number; + site: string; + global: boolean; + verdict: string; + matchScope: string; + rootCause: string; + pattern: Record; + evidence?: Record | null; + counterExamples?: unknown[] | null; + userStatement: string; + seedIssue?: { pageUrl?: string; component?: string; htmlSnippet?: string }; +} + +export interface FpRuleMemory { + ruleCode: string; + docsUrl: string; + entryCount: number; + entries: FpEntry[]; +} + +export interface FpMemory { + rules: FpRuleMemory[]; + truncated: boolean; +} + +/** + * The memory for the rules of the issues being answered with. Undefined when the account + * does not have it (403: its release flag is off) or it cannot be read in time: the issues + * are then answered without it, never not at all. + */ +export async function memoryForReport( + auth: { username: string; password: string }, + ruleCodes: string[], +): Promise { + if (ruleCodes.length === 0) return undefined; + try { + const resp = await apiClient.get({ + url: `${getRegionalHost("accessibilityApi")}/api/fp-memory/v1/memory?ruleCodes=${encodeURIComponent(ruleCodes.join(","))}`, + headers: { + Authorization: + "Basic " + + Buffer.from(`${auth.username}:${auth.password}`).toString("base64"), + }, + timeout: MEMORY_READ_TIMEOUT_MS, + raise_error: false, + }); + if (resp.status === 403) return undefined; + if (!resp.ok || !resp.data?.success) { + logger.warn(`False-positive memory not read: HTTP ${resp.status}`); + return undefined; + } + return resp.data.data as FpMemory; + } catch (error) { + logger.warn( + `False-positive memory not read: ${error instanceof Error ? error.message : String(error)}`, + ); + return undefined; + } +} + +function compactEntry(entry: FpEntry) { + const snippet = entry.seedIssue?.htmlSnippet ?? ""; + return { + id: entry.id, + site: entry.global ? "all sites" : entry.site, + verdict: entry.verdict, + scope: entry.matchScope, + root_cause: entry.rootCause, + pattern: entry.pattern, + ...(entry.counterExamples?.length + ? { counter_examples: entry.counterExamples } + : {}), + ...(entry.evidence ? { evidence: entry.evidence } : {}), + user_statement: entry.userStatement, + seed_issue: { + page: entry.seedIssue?.pageUrl, + component: entry.seedIssue?.component, + ...(snippet + ? { + html: + snippet.length > SNIPPET_CHARACTERS + ? `${snippet.slice(0, SNIPPET_CHARACTERS)}…` + : snippet, + } + : {}), + }, + }; +} + +/** + * The memory as an answer carries it, within `budget` characters: each rule with its docs + * link and its entries, newest first. When it does not fit, the oldest entries of the + * fullest rule are left out first, and counted, so the agent knows to read the rest. + */ +export function memoryPayload( + memory: FpMemory, + budget = MEMORY_CHARACTERS, +): { json: string; omitted: { rule: string; count: number }[] } { + const rules = memory.rules.map((rule) => ({ + rule: rule.ruleCode, + docs: rule.docsUrl, + entries: rule.entries.map(compactEntry), + })); + const omitted = new Map(); + let json = JSON.stringify(rules); + while (json.length > budget) { + const fullest = rules.reduce((a, b) => + b.entries.length > a.entries.length ? b : a, + ); + if (fullest.entries.length <= 1) break; + fullest.entries.pop(); + omitted.set(fullest.rule, (omitted.get(fullest.rule) ?? 0) + 1); + json = JSON.stringify(rules); + } + return { + json, + omitted: [...omitted].map(([rule, count]) => ({ rule, count })), + }; +} + +export const FP_APPLY_NOTE = + "False-positive memory: below are issues this account's users already confirmed are not real, saved per rule. Before showing the issues, remove the ones the memory covers, and do not ask the user about them again. Apply an entry only to issues of the same rule (issue_type), on its site (the host of an issue's affected_page) unless it says all sites. Match on its pattern and root_cause, never on selector or exact HTML: ids, selectors and labels differ per page and language. Remove automatically only up to the entry's scope (element: that element; component: the same component; component_family: any component its pattern.predicate describes; site_predicate: anything on the site the predicate describes). Never remove a counter_example. List issues that look like the same pattern but fall outside the scope as likely false positives and ask the user once per group; if they agree, save that at the wider scope. Say how many issues you removed per rule, with the entry ids. Each rule's docs link explains what the rule checks."; + +export const FP_FILING_GUIDE = `When the user says an issue is not a real problem, save it with the web_accessibility capability ${SUBMIT_CAPABILITY} (invokeCapability), so later reports are filtered without asking again. First inspect the element yourself (open the page if you have a browser, else read the issue's HTML): what it is built from (tag, custom element, shadow root, classes, design system) and why the rule is wrong about it. Write rootCause about how the element is built, not only that it works for the user, and pattern.predicate as the widest set of elements that root cause covers (e.g. every custom element of the same design system). Propose that scope and ask the user to confirm it, and to name any similar element that is a real issue (counterExamples). Quote the user as they said it in userStatement. Mark global only when the user says it holds on all their sites.`; + +/** What an issues answer adds for the memory: how to apply it, the memory, how to add to it. */ +export function memoryMessages(memory: FpMemory | undefined): string[] { + if (!memory) return []; + if (memory.rules.length === 0) return [FP_FILING_GUIDE]; + const { json, omitted } = memoryPayload(memory); + return [ + FP_APPLY_NOTE, + `False-positive memory: ${wrapUntrusted("saved false positives (user-written)", json)}`, + ...(omitted.length || memory.truncated + ? [ + `Not all entries fit here${omitted.length ? ` (${omitted.map((o) => `${o.count} older entries of ${o.rule}`).join(", ")})` : ""}. When an issue of such a rule is not covered by the entries above, read that rule's entries with the web_accessibility capability ${READ_CAPABILITY}.`, + ] + : []), + FP_FILING_GUIDE, + ]; +} diff --git a/tests/setup.ts b/tests/setup.ts index bdafb54c..89ae42db 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -18,3 +18,13 @@ vi.mock("sharp", () => { vi.mock("../src/lib/account-region.js", () => ({ accountRegion: () => undefined, })); + +// No false-positive memory reads while fetching issues; tests/tools/fpMemory.test.ts +// unmocks the module to test it. +vi.mock( + "../src/tools/accessiblity-utils/fp-memory.js", + async (importOriginal) => ({ + ...(await importOriginal()), + memoryForReport: async () => undefined, + }), +); diff --git a/tests/tools/capabilityWebAccessibility.test.ts b/tests/tools/capabilityWebAccessibility.test.ts index ef712b4a..b634688d 100644 --- a/tests/tools/capabilityWebAccessibility.test.ts +++ b/tests/tools/capabilityWebAccessibility.test.ts @@ -40,8 +40,11 @@ const queryNames = (name: string) => describe("the Web Accessibility capability index", () => { it("ships the six lists, as paged reads, each on an entity that names it", () => { - const names = web.capabilities.map((c: any) => c.name).sort(); - expect(names).toEqual([ + // The false-positive memory capabilities are tested in fpMemory.test.ts. + const lists = web.capabilities.filter((c: any) => + c.name.startsWith("list_"), + ); + expect(lists.map((c: any) => c.name).sort()).toEqual([ "list_assisted_test_reports", "list_automated_tests_builds", "list_automated_tests_projects", @@ -49,7 +52,7 @@ describe("the Web Accessibility capability index", () => { "list_website_scanner_scans", "list_workflow_analyzer_reports", ]); - for (const c of web.capabilities) { + for (const c of lists) { expect(c.mode).toBe("read"); expect(c.paginated).toBe(true); expect(c.max_page_size).toBeGreaterThan(0); @@ -69,7 +72,7 @@ describe("the Web Accessibility capability index", () => { expect(limit("list_assisted_test_reports").maximum).toBe(99); // Above 25 the page holds 25 scans and the next one starts past the ones in between. expect(limit("list_website_scanner_scans").maximum).toBe(25); - for (const c of web.capabilities) { + for (const c of web.capabilities.filter((c: any) => c.paginated)) { expect(c.max_page_size).toBe( c.query.find((q: any) => q.name === "limit").maximum, ); @@ -154,9 +157,15 @@ describe("the Web Accessibility capability index", () => { it.each([ ["list workflow analyzer reports", "list_workflow_analyzer_reports"], ["list website scanner scans", "list_website_scanner_scans"], - ["list the runs of a website scanner scan", "list_website_scanner_scan_runs"], + [ + "list the runs of a website scanner scan", + "list_website_scanner_scan_runs", + ], ["list automated tests projects", "list_automated_tests_projects"], - ["list the builds of an automated tests project", "list_automated_tests_builds"], + [ + "list the builds of an automated tests project", + "list_automated_tests_builds", + ], ["list assisted test reports", "list_assisted_test_reports"], ])("finds %j as %s", (query, expected) => { const { hits } = searchCapabilities(products, query, { diff --git a/tests/tools/fpMemory.test.ts b/tests/tools/fpMemory.test.ts new file mode 100644 index 00000000..a4b9e157 --- /dev/null +++ b/tests/tools/fpMemory.test.ts @@ -0,0 +1,213 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +// The false-positive memory: what users confirmed is not a real issue, read for the rules of +// the issues fetchAccessibilityIssues answers with, so the agent filters them without asking +// again. Saving, reading and revoking are web_accessibility capabilities. What has to hold: +// the memory never holds the issues back, it fits its budget oldest-first, it is quoted as +// untrusted, and the capabilities an answer names exist and can be invoked. + +vi.unmock("../../src/tools/accessiblity-utils/fp-memory.js"); + +const get = vi.hoisted(() => vi.fn()); +vi.mock("../../src/lib/apiClient.js", () => ({ apiClient: { get } })); +vi.mock("../../src/logger.js", () => ({ + default: { error: vi.fn(), info: vi.fn(), debug: vi.fn(), warn: vi.fn() }, +})); + +import { + FP_APPLY_NOTE, + FP_FILING_GUIDE, + READ_CAPABILITY, + SUBMIT_CAPABILITY, + memoryForReport, + memoryMessages, + memoryPayload, + type FpEntry, + type FpMemory, +} from "../../src/tools/accessiblity-utils/fp-memory.js"; + +const AUTH = { username: "u", password: "k" }; + +function entry(id: number, extra: Partial = {}): FpEntry { + return { + id, + site: "www.shop.example", + global: false, + verdict: "false_positive", + matchScope: "component_family", + rootCause: + "The role is set inside the custom element's shadow root; the host has no role attribute.", + pattern: { + tagPrefix: "acme-", + predicate: "acme- custom elements rendering a native control", + }, + counterExamples: ["div.to_sign_in"], + userStatement: "These Acme components work with VoiceOver", + seedIssue: { + pageUrl: "https://www.shop.example/de/angebote/halbtax", + component: "acme-header-button#", + htmlSnippet: "", + }, + ...extra, + }; +} + +function memory(entries: FpEntry[], ruleCode = "role-required"): FpMemory { + return { + rules: [ + { + ruleCode, + docsUrl: `https://www.browserstack.com/docs/accessibility/rules/a11y-engine/${ruleCode}/`, + entryCount: entries.length, + entries, + }, + ], + truncated: false, + }; +} + +beforeEach(() => vi.clearAllMocks()); + +describe("memoryForReport", () => { + it("asks for the answer's rules only, with the user's credentials", async () => { + get.mockResolvedValue({ + ok: true, + status: 200, + data: { success: true, data: memory([entry(1)]) }, + }); + + const result = await memoryForReport(AUTH, ["role-required", "image-alt"]); + + expect(result?.rules[0].entries[0].id).toBe(1); + const call = get.mock.calls[0][0]; + expect(call.url).toContain( + "/api/fp-memory/v1/memory?ruleCodes=role-required%2Cimage-alt", + ); + expect(call.headers.Authorization).toBe( + "Basic " + Buffer.from("u:k").toString("base64"), + ); + expect(call.timeout).toBeGreaterThan(0); + }); + + it("does not ask when the answer names no rules", async () => { + expect(await memoryForReport(AUTH, [])).toBeUndefined(); + expect(get).not.toHaveBeenCalled(); + }); + + it("answers without memory when the account does not have it", async () => { + get.mockResolvedValue({ ok: false, status: 403, data: { success: false } }); + expect(await memoryForReport(AUTH, ["role-required"])).toBeUndefined(); + }); + + it("answers without memory when it cannot be read, rather than failing", async () => { + get.mockRejectedValue(new Error("timeout of 8000ms exceeded")); + expect(await memoryForReport(AUTH, ["role-required"])).toBeUndefined(); + get.mockResolvedValue({ ok: false, status: 500, data: {} }); + expect(await memoryForReport(AUTH, ["role-required"])).toBeUndefined(); + }); +}); + +describe("memoryMessages", () => { + it("says nothing when the account has no memory", () => { + expect(memoryMessages(undefined)).toEqual([]); + }); + + it("only says how to save one when no rule of the answer has any yet", () => { + expect(memoryMessages({ rules: [], truncated: false })).toEqual([ + FP_FILING_GUIDE, + ]); + }); + + it("says how to apply the memory, quotes it as untrusted, then how to add to it", () => { + const messages = memoryMessages(memory([entry(1)])); + + expect(messages[0]).toBe(FP_APPLY_NOTE); + expect(messages[1]).toMatch(/ { + expect(memoryMessages(memory([entry(1, { global: true })]))[1]).toContain( + "all sites", + ); + }); +}); + +describe("memoryPayload", () => { + it("leaves out the fullest rule's oldest entries first, and counts them", () => { + const many = Array.from({ length: 40 }, (_, i) => entry(100 - i)); + const { json, omitted } = memoryPayload(memory(many), 4_000); + + expect(json.length).toBeLessThanOrEqual(4_000); + const kept = JSON.parse(json)[0].entries.map((e: any) => e.id); + expect(kept[0]).toBe(100); + expect(omitted).toEqual([ + { rule: "role-required", count: 40 - kept.length }, + ]); + }); + + it("shortens long HTML snippets", () => { + const long = entry(1, { + seedIssue: { htmlSnippet: "
" + "x".repeat(1000) + "
" }, + }); + const html = JSON.parse(memoryPayload(memory([long])).json)[0].entries[0] + .seed_issue.html; + expect(html.length).toBeLessThanOrEqual(201); + }); +}); + +describe("the capabilities an answer names", () => { + const index = JSON.parse( + readFileSync( + fileURLToPath( + new URL( + "../../capability/web_accessibility.capability-index.json", + import.meta.url, + ), + ), + "utf8", + ), + ).web_accessibility; + const byName = (name: string) => + index.capabilities.find((c: any) => c.name === name); + + it("exist in the Web Accessibility index", () => { + expect(byName(SUBMIT_CAPABILITY)).toBeDefined(); + expect(byName(READ_CAPABILITY)).toBeDefined(); + expect(FP_FILING_GUIDE).toContain(SUBMIT_CAPABILITY); + }); + + it("are never destructive, which invokeCapability refuses before binding", () => { + const fp = index.capabilities.filter( + (c: any) => c.entity === "false_positive_feedback", + ); + expect(fp.map((c: any) => [c.name, c.method, c.mode])).toEqual([ + ["submit_false_positive_feedback", "POST", "write"], + ["get_false_positive_memory", "GET", "read"], + ["revoke_false_positive_feedback", "POST", "write"], + ]); + }); + + it("require on submit what the API refuses without", () => { + const required = byName(SUBMIT_CAPABILITY) + .body.filter((b: any) => b.required) + .map((b: any) => b.name) + .sort(); + expect(required).toEqual([ + "pageUrl", + "pattern", + "rootCause", + "ruleCode", + "scope", + "userStatement", + "verdict", + ]); + }); +}); From 8189fa25dc86d5b82cf920ea18699dd3111f36a0 Mon Sep 17 00:00:00 2001 From: Arjun Chikara Date: Fri, 6 Nov 2026 10:46:02 +0530 Subject: [PATCH 2/4] fix(accessibility): make the agent remove saved false positives before reporting Asked to "fetch the list of issues from this report", an agent handed the saved false positives as one note among the answer's others reported the report's own list and totals without removing them. Reproduced with a smaller model on an 80-issue answer: 3 of 4 replies listed all 80. - The answer now opens, before any count, by saying how many issues the saved entries may cover and that they are to be removed before any list, count or summary is given, without asking the user again and even when the full list is asked for. Its totals are labelled as before that removal, and it closes by saying to lead with how many issues are left and what was removed per rule. With the same answer, 6 of 6 replies removed exactly the covered issues, kept the counter-example and left another rule's issue alone. - Each issue the memory may cover is marked saved_fp_entries with its entry ids (same rule, same site or an entry for all sites), and only those entries are shown, so the instruction points at the issues themselves. - startAccessibilityScan's answer gets the same, as it lists issues too. - fetchAccessibilityIssues' description says to remove the saved false positives it marks, in place of the list of its parameters (shorter). Co-Authored-By: Claude Opus 5.5 --- src/tools/accessibility.ts | 45 +++- src/tools/accessiblity-utils/fp-memory.ts | 109 +++++++-- .../tools/accessibilityFpMemoryAnswer.test.ts | 230 ++++++++++++++++++ tests/tools/fpMemory.test.ts | 102 ++++++-- 4 files changed, 440 insertions(+), 46 deletions(-) create mode 100644 tests/tools/accessibilityFpMemoryAnswer.test.ts diff --git a/src/tools/accessibility.ts b/src/tools/accessibility.ts index a50c277c..26c698e1 100644 --- a/src/tools/accessibility.ts +++ b/src/tools/accessibility.ts @@ -37,8 +37,9 @@ import { elicitCredentialsIfSupported } from "../lib/elicit-credentials.js"; import logger from "../logger.js"; import { NEEDS_A11Y_CONFIG_ID, NEEDS_A11Y_SCAN_ID } from "./tool-handoff.js"; import { + applyMemory, memoryForReport, - memoryMessages, + type MemoryView, } from "./accessiblity-utils/fp-memory.js"; interface AuthCredentials { @@ -1054,23 +1055,29 @@ async function fetchAccessibilityIssues( }); // The memory for this page's rules comes with the issues, so it is applied whether or // not the agent thinks to ask for it. - const memory = await memoryForReport( - setupAuth(config), - Object.keys(rules ?? {}), + const fp = applyMemory( + await memoryForReport(setupAuth(config), Object.keys(rules ?? {})), + records, + pageUrl, ); + const beforeRemoval = fp.marked + ? ", before removing saved false positives" + : ""; return createSuccessResponse([ + ...fp.lead, ...preface, ...processingNote(processing), ...(page ? pageNote(page) : []), - `Retrieved ${page_length} accessibility issues (Total: ${total_issues})`, + `Retrieved ${page_length} accessibility issues (Total: ${total_issues}${beforeRemoval})`, // The whole report's counts, said on its first page. ...(cursor ? [] : webSummaryNote(summary, total_issues)), ...excludedIssuesNote(duplicate_issues, unlinked_duplicates, hidden_issues), fullReportLine(reportLink), WEB_ISSUES_NOTE, - ...memoryMessages(memory), - `Issues: ${wrapUntrusted("accessibility scan results", JSON.stringify({ issues: records, rules }))}`, + ...fp.notes, + `Issues: ${wrapUntrusted("accessibility scan results", JSON.stringify({ issues: fp.records, rules }))}`, + ...fp.closing, closingMessage(next_page, remaining, source, stillProcessing, pageArg), ]); } @@ -1485,14 +1492,22 @@ function createScanSuccessResponse( dashboardLink: string | undefined, settings: ScanSettings = {}, applied?: ScanSettings, + fp: MemoryView<(typeof report.records)[number]> = applyMemory( + undefined, + report.records, + ), ): CallToolResult { const cursor = report.next_page; + const beforeRemoval = fp.marked + ? " (before removing saved false positives)" + : ""; const messages = [ + ...fp.lead, `Accessibility scan "${name}" completed. ${scanDashboardNote(dashboardLink, scanId)}`, `Scan ID: ${scanId} and Scan Run ID: ${scanRunId}`, scanSettingsNote(settings, applied), fullReportLine(reportUrl), - `We found ${report.total_issues} issues. Below are the details of the ${report.page_length} most critical issues.`, + `We found ${report.total_issues} issues${beforeRemoval}. Below are the details of the ${report.page_length} most critical issues.`, ...webSummaryNote(report.summary, report.total_issues), ...excludedIssuesNote( report.duplicate_issues, @@ -1500,7 +1515,9 @@ function createScanSuccessResponse( report.hidden_issues, ), WEB_ISSUES_NOTE, - `Scan results: ${wrapUntrusted("accessibility scan results", JSON.stringify({ issues: report.records, rules: report.rules }))}`, + ...fp.notes, + `Scan results: ${wrapUntrusted("accessibility scan results", JSON.stringify({ issues: fp.records, rules: report.rules }))}`, + ...fp.closing, ]; if (cursor !== null) { @@ -1575,15 +1592,21 @@ async function runAccessibilityScan( throw error; } + const report = await parseAccessibilityReportFromCSV(reportLink); return createScanSuccessResponse( name, - await parseAccessibilityReportFromCSV(reportLink), + report, scanId, scanRunId, reportLink, dashboardLink, settings, applied, + applyMemory( + await memoryForReport(setupAuth(config), Object.keys(report.rules ?? {})), + report.records, + pageURL, + ), ); } @@ -1776,7 +1799,7 @@ export default function addAccessibilityTools( tools.fetchAccessibilityIssues = server.tool( "fetchAccessibilityIssues", - "Fetch accessibility issues from one source: scanId+scanRunId, buildId, workflowReportId, appReportId or reportUrl." + + "Fetch a web or app accessibility report's issues. Remove the saved false positives it marks before reporting any." + NEEDS_A11Y_SCAN_ID, { scanId: z diff --git a/src/tools/accessiblity-utils/fp-memory.ts b/src/tools/accessiblity-utils/fp-memory.ts index a7c8f1cf..4a951f8d 100644 --- a/src/tools/accessiblity-utils/fp-memory.ts +++ b/src/tools/accessiblity-utils/fp-memory.ts @@ -146,23 +146,100 @@ export function memoryPayload( } export const FP_APPLY_NOTE = - "False-positive memory: below are issues this account's users already confirmed are not real, saved per rule. Before showing the issues, remove the ones the memory covers, and do not ask the user about them again. Apply an entry only to issues of the same rule (issue_type), on its site (the host of an issue's affected_page) unless it says all sites. Match on its pattern and root_cause, never on selector or exact HTML: ids, selectors and labels differ per page and language. Remove automatically only up to the entry's scope (element: that element; component: the same component; component_family: any component its pattern.predicate describes; site_predicate: anything on the site the predicate describes). Never remove a counter_example. List issues that look like the same pattern but fall outside the scope as likely false positives and ask the user once per group; if they agree, save that at the wider scope. Say how many issues you removed per rule, with the entry ids. Each rule's docs link explains what the rule checks."; + "How to apply the false-positive memory: an issue marked saved_fp_entries has false positives saved for its rule (issue_type) on its site; compare it with those entries only. Match on an entry's pattern and root_cause, never on selector or exact HTML: ids, selectors and labels differ per page and language. Remove it when the entry covers it within the entry's scope (element: that element; component: the same component; component_family: any component its pattern.predicate describes; site_predicate: anything on the site the predicate describes), and do not ask the user about it again. Never remove a counter_example. A marked issue that looks like the same pattern but falls outside the scope is a likely false positive: ask the user once per group, and if they agree save it at the wider scope. An issue without saved_fp_entries is not covered by the memory. Each rule's docs link explains what the rule checks."; export const FP_FILING_GUIDE = `When the user says an issue is not a real problem, save it with the web_accessibility capability ${SUBMIT_CAPABILITY} (invokeCapability), so later reports are filtered without asking again. First inspect the element yourself (open the page if you have a browser, else read the issue's HTML): what it is built from (tag, custom element, shadow root, classes, design system) and why the rule is wrong about it. Write rootCause about how the element is built, not only that it works for the user, and pattern.predicate as the widest set of elements that root cause covers (e.g. every custom element of the same design system). Propose that scope and ask the user to confirm it, and to name any similar element that is a real issue (counterExamples). Quote the user as they said it in userStatement. Mark global only when the user says it holds on all their sites.`; -/** What an issues answer adds for the memory: how to apply it, the memory, how to add to it. */ -export function memoryMessages(memory: FpMemory | undefined): string[] { - if (!memory) return []; - if (memory.rules.length === 0) return [FP_FILING_GUIDE]; - const { json, omitted } = memoryPayload(memory); - return [ - FP_APPLY_NOTE, - `False-positive memory: ${wrapUntrusted("saved false positives (user-written)", json)}`, - ...(omitted.length || memory.truncated - ? [ - `Not all entries fit here${omitted.length ? ` (${omitted.map((o) => `${o.count} older entries of ${o.rule}`).join(", ")})` : ""}. When an issue of such a rule is not covered by the entries above, read that rule's entries with the web_accessibility capability ${READ_CAPABILITY}.`, - ] - : []), - FP_FILING_GUIDE, - ]; +/** An issue as an answer lists it: its rule, and the page it is on when the answer spans several. */ +interface IssueLike { + issue_type: string; + affected_page?: string; +} + +/** What an answer with issues says about the memory, and where. */ +export interface MemoryView { + /** The issues, those with saved entries for their rule and site marked saved_fp_entries. */ + records: (T & { saved_fp_entries?: number[] })[]; + /** How many of them are marked. */ + marked: number; + /** First in the answer: remove the false positives before giving any list or count. */ + lead: string[]; + /** Before the issues: how to apply the memory, the entries, how to save a new one. */ + notes: string[]; + /** After the issues: how to report what is left. */ + closing: string[]; +} + +function hostOf(page: string | undefined): string | undefined { + try { + return page ? new URL(page).hostname.toLowerCase() : undefined; + } catch { + return undefined; + } +} + +/** + * The memory applied to one answer's issues. An agent handed the memory as one note among + * many reported the report's own totals without filtering, so the issues the memory may + * cover are marked with their entry ids, the answer opens by saying they are to be removed + * before anything is reported, and it closes by saying how to report what is left. Only + * the entries of these issues' rules and sites (or saved for all sites) are shown. + * `page` is the page every issue is on, when the answer is about one. + */ +export function applyMemory( + memory: FpMemory | undefined, + records: T[], + page?: string, +): MemoryView { + if (!memory) return { records, marked: 0, lead: [], notes: [], closing: [] }; + const byRule = new Map(memory.rules.map((rule) => [rule.ruleCode, rule])); + const used = new Map>(); + let marked = 0; + const annotated = records.map((record) => { + const host = hostOf(record.affected_page ?? page); + const ids = (byRule.get(record.issue_type)?.entries ?? []) + .filter((entry) => entry.global || (host && entry.site === host)) + .map((entry) => entry.id); + if (ids.length === 0) return record; + marked++; + const seen = used.get(record.issue_type) ?? new Set(); + ids.forEach((id) => seen.add(id)); + used.set(record.issue_type, seen); + return { ...record, saved_fp_entries: ids }; + }); + if (marked === 0) { + return { records, marked, lead: [], notes: [FP_FILING_GUIDE], closing: [] }; + } + const shown: FpMemory = { + truncated: memory.truncated, + rules: memory.rules + .filter((rule) => used.has(rule.ruleCode)) + .map((rule) => { + const entries = rule.entries.filter((e) => + used.get(rule.ruleCode)!.has(e.id), + ); + return { ...rule, entries, entryCount: entries.length }; + }), + }; + const { json, omitted } = memoryPayload(shown); + return { + records: annotated, + marked, + lead: [ + `⚠️ Saved false positives apply to this report: ${marked} of the ${records.length} issues below are marked saved_fp_entries, as this account's users already confirmed false positives of their rule on their site. Before you give the user any list, count or summary of issues, check each marked issue against its entries (memory and how to apply it are below) and remove the ones they cover. The user already confirmed these, so do not ask them to confirm again, and leave them out even when the user asks for the full list or all issues: their report is the issues that are left. Every count in this answer is from before that removal.`, + ], + notes: [ + FP_APPLY_NOTE, + `False-positive memory: ${wrapUntrusted("saved false positives (user-written)", json)}`, + ...(omitted.length || memory.truncated + ? [ + `Not all entries fit here${omitted.length ? ` (${omitted.map((o) => `${o.count} older entries of ${o.rule}`).join(", ")})` : ""}. When a marked issue is not covered by the entries above, read its rule's entries with the web_accessibility capability ${READ_CAPABILITY}.`, + ] + : []), + FP_FILING_GUIDE, + ], + closing: [ + "When you answer the user: list only the issues left after removing the saved false positives, and lead with how many are left; say how many you removed for each rule, with the entry ids; and say that the report's own totals above count them, so the issues left are that many fewer.", + ], + }; } diff --git a/tests/tools/accessibilityFpMemoryAnswer.test.ts b/tests/tools/accessibilityFpMemoryAnswer.test.ts new file mode 100644 index 00000000..794fbf01 --- /dev/null +++ b/tests/tools/accessibilityFpMemoryAnswer.test.ts @@ -0,0 +1,230 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// An agent asked to "fetch the issues from this report" was handed the saved false +// positives as one note among many, and reported the report's own list and totals without +// removing them. What has to hold: when saved entries cover issues in the answer, the +// answer opens by saying to remove them before giving any list or count, marks each issue +// they may cover, labels its totals as before that removal, and closes by saying how to +// report what is left. Without saved entries, the answer is what it always was. + +const m = vi.hoisted(() => ({ + parse: vi.fn(), + memoryForReport: vi.fn(), +})); + +vi.mock("../../src/tools/accessiblity-utils/report-fetcher.js", () => ({ + AccessibilityReportFetcher: class { + setAuth = vi.fn(); + getReportLink = vi.fn(async () => "https://exports.example/report.csv"); + reportStatus = vi.fn(async () => "completed"); + resolveBuild = vi.fn(async (source: unknown) => source); + }, + EXPORT_WAIT_MS: 45_000, + ExportPendingError: class extends Error {}, +})); +vi.mock("../../src/tools/accessiblity-utils/report-parser.js", () => ({ + parseAccessibilityReportFromCSV: m.parse, + loadMainIssues: vi.fn(), + compareIssues: vi.fn(), +})); +vi.mock( + "../../src/tools/accessiblity-utils/fp-memory.js", + async (importOriginal) => ({ + ...(await importOriginal()), + memoryForReport: m.memoryForReport, + }), +); +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() })); +vi.mock("../../src/index.js", () => ({ + trackMCP: vi.fn(), + PackageJsonVersion: "0.0.0", +})); + +import addAccessibilityTools from "../../src/tools/accessibility.js"; + +function fetchIssues() { + let handler: (...args: any[]) => any = () => undefined; + const server: any = { + tool: vi.fn((...args: any[]) => { + if (args[0] === "fetchAccessibilityIssues") + handler = args[args.length - 1]; + }), + server: { getClientVersion: vi.fn().mockReturnValue({ version: "1.0" }) }, + }; + addAccessibilityTools(server, { + "browserstack-username": "fake-user", + "browserstack-access-key": "fake-key", + } as any); + return (args: Record) => + handler(args, { sendNotification: vi.fn(), _meta: {} }); +} + +const texts = (result: any): string[] => + result.content.map((c: { text: string }) => c.text); + +const PAGE = "https://www.shop.example/checkout"; +const issue = (issue_type: string, component: string) => ({ + issue_type, + component, + HTML_snippet: `<${component.replace("#", "")}>`, + severity: "serious", + affected_page: PAGE, +}); + +const REPORT = { + records: [ + issue("role-required", "acme-header-button#"), + issue("role-required", "acme-menu-trigger#"), + issue("image-alt", "img#"), + ], + rules: { + "role-required": { description: "Interactive elements need a role" }, + "image-alt": { description: "Images need alt text" }, + }, + summary: { + by_severity: { serious: 3 }, + by_rule: [ + { issue_type: "role-required", count: 2 }, + { issue_type: "image-alt", count: 1 }, + ], + needs_review: 0, + best_practice: 0, + }, + page_length: 3, + total_issues: 3, + next_page: null, + remaining: 0, + duplicate_issues: null, + unlinked_duplicates: 0, + hidden_issues: null, +}; + +const MEMORY = { + truncated: false, + rules: [ + { + ruleCode: "role-required", + docsUrl: + "https://www.browserstack.com/docs/accessibility/rules/a11y-engine/role-required/", + entryCount: 1, + entries: [ + { + id: 41, + site: "www.shop.example", + global: false, + verdict: "false_positive", + matchScope: "component_family", + rootCause: + "Role is set inside each acme- custom element's shadow root; the host has none.", + pattern: { + tagPrefix: "acme-", + predicate: "acme- custom elements rendering a native control", + }, + userStatement: "Those Acme buttons work fine with a screen reader", + }, + ], + }, + ], +}; + +const RUN = { scanId: "7310042", scanRunId: "5520318" }; + +beforeEach(() => { + vi.clearAllMocks(); + m.parse.mockResolvedValue(structuredClone(REPORT)); +}); + +describe("fetchAccessibilityIssues with saved false positives", () => { + it("opens by saying to remove them before giving any list or count", async () => { + m.memoryForReport.mockResolvedValue(MEMORY); + + const lines = texts(await fetchIssues()(RUN)); + + expect(lines[0]).toMatch(/Saved false positives apply to this report/); + expect(lines[0]).toContain("2 of the 3 issues below"); + expect(lines[0]).toMatch(/Before you give the user any list, count/); + const firstCount = lines.findIndex((l) => l.startsWith("Retrieved")); + expect(firstCount).toBeGreaterThan(0); + expect(lines[firstCount]).toContain( + "Total: 3, before removing saved false positives", + ); + }); + + it("asks only for the rules of the issues it answers with", async () => { + m.memoryForReport.mockResolvedValue(MEMORY); + + await fetchIssues()(RUN); + + expect(m.memoryForReport.mock.calls[0][1]).toEqual([ + "role-required", + "image-alt", + ]); + }); + + it("marks the issues the entries may cover, and only those", async () => { + m.memoryForReport.mockResolvedValue(MEMORY); + + const block = texts(await fetchIssues()(RUN)).find((l) => + l.startsWith("Issues:"), + )!; + const issues = JSON.parse(block.split("\n")[2]).issues; + + expect(issues.map((i: any) => i.saved_fp_entries)).toEqual([ + [41], + [41], + undefined, + ]); + }); + + it("gives the memory before the issues and how to report what is left after them", async () => { + m.memoryForReport.mockResolvedValue(MEMORY); + + const lines = texts(await fetchIssues()(RUN)); + const memoryAt = lines.findIndex((l) => + l.startsWith("False-positive memory:"), + ); + const issuesAt = lines.findIndex((l) => l.startsWith("Issues:")); + const closingAt = lines.findIndex((l) => + l.includes("list only the issues left"), + ); + + expect(memoryAt).toBeGreaterThan(-1); + expect(memoryAt).toBeLessThan(issuesAt); + expect(closingAt).toBeGreaterThan(issuesAt); + }); + + it("answers as it always did when no saved entry covers these issues", async () => { + m.memoryForReport.mockResolvedValue({ + truncated: false, + rules: [ + { + ...MEMORY.rules[0], + entries: [ + { ...MEMORY.rules[0].entries[0], site: "elsewhere.example" }, + ], + }, + ], + }); + + const lines = texts(await fetchIssues()(RUN)); + + expect(lines[0]).not.toMatch(/Saved false positives/); + expect(lines.join("\n")).not.toContain("saved_fp_entries"); + expect(lines.join("\n")).toContain("Total: 3)"); + }); + + it("answers as it always did when the account has no memory", async () => { + m.memoryForReport.mockResolvedValue(undefined); + + const lines = texts(await fetchIssues()(RUN)); + + expect(lines.join("\n")).not.toMatch(/false positive/i); + expect(lines.join("\n")).toContain("Total: 3)"); + }); +}); diff --git a/tests/tools/fpMemory.test.ts b/tests/tools/fpMemory.test.ts index a4b9e157..1808c79b 100644 --- a/tests/tools/fpMemory.test.ts +++ b/tests/tools/fpMemory.test.ts @@ -22,7 +22,7 @@ import { READ_CAPABILITY, SUBMIT_CAPABILITY, memoryForReport, - memoryMessages, + applyMemory, memoryPayload, type FpEntry, type FpMemory, @@ -109,34 +109,98 @@ describe("memoryForReport", () => { }); }); -describe("memoryMessages", () => { - it("says nothing when the account has no memory", () => { - expect(memoryMessages(undefined)).toEqual([]); +const ON_SITE = "https://www.shop.example/de/angebote"; +const issue = (issue_type: string, affected_page = ON_SITE) => ({ + issue_type, + affected_page, + component: "acme-header-button#", +}); + +describe("applyMemory", () => { + it("leaves an answer as it was when the account has no memory", () => { + const records = [issue("role-required")]; + const view = applyMemory(undefined, records); + expect(view).toEqual({ + records, + marked: 0, + lead: [], + notes: [], + closing: [], + }); }); - it("only says how to save one when no rule of the answer has any yet", () => { - expect(memoryMessages({ rules: [], truncated: false })).toEqual([ - FP_FILING_GUIDE, + it("only says how to save one when no saved entry covers these issues", () => { + const view = applyMemory({ rules: [], truncated: false }, [ + issue("role-required"), ]); + expect(view.marked).toBe(0); + expect(view.notes).toEqual([FP_FILING_GUIDE]); + expect(view.lead).toEqual([]); }); - it("says how to apply the memory, quotes it as untrusted, then how to add to it", () => { - const messages = memoryMessages(memory([entry(1)])); + it("marks each issue with the entries saved for its rule on its site", () => { + const view = applyMemory(memory([entry(1), entry(2)]), [ + issue("role-required"), + issue("image-alt"), + issue("role-required", "https://other.example/page"), + ]); - expect(messages[0]).toBe(FP_APPLY_NOTE); - expect(messages[1]).toMatch(/ { + const view = applyMemory(memory([entry(1, { global: true })]), [ + issue("role-required", "https://other.example/page"), + ]); + expect(view.records[0].saved_fp_entries).toEqual([1]); + expect(view.notes[1]).toContain("all sites"); + }); + + it("takes the page an answer is about for issues that do not name one", () => { + const view = applyMemory( + memory([entry(1)]), + [{ issue_type: "role-required" }], + ON_SITE, ); - expect(messages[1]).toContain("div.to_sign_in"); - expect(messages.at(-1)).toBe(FP_FILING_GUIDE); + expect(view.records[0].saved_fp_entries).toEqual([1]); }); - it("says an entry saved for all sites applies to all sites", () => { - expect(memoryMessages(memory([entry(1, { global: true })]))[1]).toContain( - "all sites", + it("opens by saying to remove the marked issues before giving any list or count", () => { + const view = applyMemory(memory([entry(1)]), [ + issue("role-required"), + issue("image-alt"), + ]); + + expect(view.lead).toHaveLength(1); + expect(view.lead[0]).toContain("1 of the 2 issues below"); + expect(view.lead[0]).toMatch(/Before you give the user any list, count/); + expect(view.lead[0]).toMatch(/from before that removal/); + expect(view.closing[0]).toMatch(/only the issues left/); + }); + + it("shows only the entries of the marked issues, quoted as untrusted, then how to add one", () => { + const view = applyMemory( + memory([entry(1), entry(2, { site: "other.example" })]), + [issue("role-required")], + ); + + expect(view.notes[0]).toBe(FP_APPLY_NOTE); + expect(view.notes[1]).toMatch(/untrusted/i); + expect(view.notes[1]).toContain( + "https://www.browserstack.com/docs/accessibility/rules/a11y-engine/role-required/", + ); + expect(view.notes[1]).toContain("div.to_sign_in"); + const shown = JSON.parse( + view.notes[1].slice( + view.notes[1].indexOf("[{"), + view.notes[1].lastIndexOf("}]") + 2, + ), ); + expect(shown[0].entries.map((e: any) => e.id)).toEqual([1]); + expect(view.notes.at(-1)).toBe(FP_FILING_GUIDE); }); }); From 7062aa2ccaa2bfef929aa8f5ed3e1c5d2e1601fd Mon Sep 17 00:00:00 2001 From: Arjun Chikara Date: Fri, 6 Nov 2026 16:00:14 +0530 Subject: [PATCH 3/4] fix(accessibility): follow the FP memory API's public ids and paging The API now returns string ids (fp_...), pages the memory with limit/cursor instead of a truncated flag, refuses duplicates with 409, and names entry fields as the POST takes them (scope, pageUrl, component, htmlSnippet). The fetch answer reads one page of 200, and says to page with nextCursor when entries did not fit. Co-Authored-By: Claude Opus 5.5 --- .../web_accessibility.capability-index.json | 109 ++++++++++-------- src/tools/accessiblity-utils/fp-memory.ts | 36 +++--- .../tools/accessibilityFpMemoryAnswer.test.ts | 12 +- tests/tools/fpMemory.test.ts | 44 ++++--- 4 files changed, 114 insertions(+), 87 deletions(-) diff --git a/capability/web_accessibility.capability-index.json b/capability/web_accessibility.capability-index.json index 78f15c92..4f3067fd 100644 --- a/capability/web_accessibility.capability-index.json +++ b/capability/web_accessibility.capability-index.json @@ -449,11 +449,7 @@ "name": "verdict", "type": "string", "required": true, - "values": [ - "false_positive", - "not_applicable", - "accepted_risk" - ], + "values": ["false_positive", "not_applicable", "accepted_risk"], "description": "false_positive unless the user says the issue is real but does not apply, or is a risk they accept." }, { @@ -557,7 +553,8 @@ "Before saving, inspect the element yourself (open the page if you can, else read the issue's HTML) and write rootCause and pattern.predicate from what you saw: a record that only says the element works for the user matches almost nothing on the next report.", "Propose the widest scope the root cause supports (e.g. component_family for every custom element of one design system) and have the user confirm it; ask them for any similar element that is a real issue and send it in counterExamples.", "Send the issue's fields as fetchAccessibilityIssues gave them; selectors are not taken, as they differ per page.", - "warnings in the reply ask you to re-check the scope with the user; resend at the wider scope if they agree." + "warnings in the reply ask you to re-check the scope with the user; resend at the wider scope if they agree.", + "A 409 reply means an active entry already covers this rule, site, scope and predicate: nothing was saved, and data.existingId names that entry; tell the user it is already saved." ], "returns": [ "data.id", @@ -566,7 +563,8 @@ "data.site", "data.global", "data.activeEntriesForRule", - "data.warnings" + "data.warnings", + "data.existingId" ], "responses": { "201": { @@ -581,7 +579,7 @@ "type": "object", "properties": { "id": { - "type": "integer" + "type": "string" }, "ruleCode": { "type": "string" @@ -615,6 +613,9 @@ "403": { "$response": "FpMemoryDisabled" }, + "409": { + "$response": "FpFeedbackDuplicate" + }, "422": { "$response": "FpFeedbackRejected" }, @@ -640,6 +641,19 @@ "name": "site", "type": "string", "description": "A host or page URL: only that site's entries, plus those saved for all sites." + }, + { + "name": "limit", + "type": "integer", + "minimum": 1, + "maximum": 200, + "default": 100, + "description": "Entries per page, newest first." + }, + { + "name": "cursor", + "type": "string", + "description": "The previous page's data.nextCursor, to read the next page." } ], "intent": "Read the account's saved false positives, grouped by rule with each rule's docs link, to filter accessibility issues with or to find an entry's id.", @@ -652,11 +666,11 @@ "data.rules.docsUrl", "data.rules.entryCount", "data.rules.entries", - "data.truncated" + "data.nextCursor" ], "responses": { "200": { - "description": "The memory, grouped by rule; newest entries first, at most 5000.", + "description": "The memory, grouped by rule; newest entries first, one page at a time.", "schema": { "type": "object", "properties": { @@ -666,8 +680,9 @@ "data": { "type": "object", "properties": { - "truncated": { - "type": "boolean" + "nextCursor": { + "type": ["string", "null"], + "description": "Set when more entries follow: pass it as cursor." }, "rules": { "type": "array", @@ -689,7 +704,7 @@ "type": "object", "properties": { "id": { - "type": "integer" + "type": "string" }, "site": { "type": "string" @@ -700,7 +715,7 @@ "verdict": { "type": "string" }, - "matchScope": { + "scope": { "type": "string" }, "rootCause": { @@ -715,8 +730,14 @@ "userStatement": { "type": "string" }, - "seedIssue": { - "type": "object" + "pageUrl": { + "type": "string" + }, + "component": { + "type": "string" + }, + "htmlSnippet": { + "type": "string" } } } @@ -735,6 +756,9 @@ "403": { "$response": "FpMemoryDisabled" }, + "422": { + "description": "Invalid limit, or a cursor that is not an entry of the account." + }, "500": { "$response": "InternalServerError" } @@ -749,20 +773,17 @@ "path_params": [ { "name": "id", - "type": "integer", + "type": "string", "required": true, - "minimum": 1, - "description": "The entry's id, from the memory or the save reply." + "description": "The entry's id (fp_…), from the memory or the save reply.", + "example": "fp_Ab12Cd34Ef56" } ], "intent": "Remove a saved false positive the user no longer agrees with, so the issues it covered show again in later reports.", "guidance": [ "Revoked entries are kept on the account's side but no longer returned or applied; there is no undo, so save a new entry instead if the user changes their mind." ], - "returns": [ - "data.id", - "data.status" - ], + "returns": ["data.id", "data.status"], "responses": { "200": { "description": "Revoked.", @@ -776,7 +797,7 @@ "type": "object", "properties": { "id": { - "type": "integer" + "type": "string" }, "status": { "type": "string" @@ -818,9 +839,7 @@ "id_convention": "A plain integer, discovered through the report list; it is the workflowReportId the accessibility issue tool reads.", "parents": [], "relations": [], - "capabilities": [ - "list_workflow_analyzer_reports" - ], + "capabilities": ["list_workflow_analyzer_reports"], "key_facts": [ "Reports are shared across the account, so the list holds everyone's; one person's are picked out by the creator on each row.", "A report is created by scanning in the browser extension; there is nothing to create or delete here." @@ -848,9 +867,7 @@ "via": "runs once, or again on each recurrence" } ], - "capabilities": [ - "list_website_scanner_scans" - ], + "capabilities": ["list_website_scanner_scans"], "key_facts": [ "A scan's own row carries only its latest run; the earlier runs are listed per scan.", "Scans are shared across the account, so the list holds everyone's." @@ -868,18 +885,14 @@ "previous scan" ], "id_convention": "A plain integer, read together with its scan's id; discovered through the scan's run list.", - "parents": [ - "website_scan" - ], + "parents": ["website_scan"], "relations": [ { "entity": "website_scan", "via": "belongs to one scan" } ], - "capabilities": [ - "list_website_scanner_scan_runs" - ], + "capabilities": ["list_website_scanner_scan_runs"], "key_facts": [ "A page that did not load in a run is still counted as scanned in some places; the run's failure count says how many did not load." ] @@ -902,9 +915,7 @@ "via": "groups the builds reported under its name" } ], - "capabilities": [ - "list_automated_tests_projects" - ], + "capabilities": ["list_automated_tests_projects"], "key_facts": [ "A project is created by the first test run reported under its name; there is nothing to create or delete here." ] @@ -921,18 +932,14 @@ "accessibility test run" ], "id_convention": "A 40-character string of lowercase letters and digits, the thBuildId in the build's dashboard link; discovered through the build list.", - "parents": [ - "automated_tests_project" - ], + "parents": ["automated_tests_project"], "relations": [ { "entity": "automated_tests_project", "via": "belongs to one project" } ], - "capabilities": [ - "list_automated_tests_builds" - ], + "capabilities": ["list_automated_tests_builds"], "key_facts": [ "A build appears only if its tests ran with accessibility checks on.", "status is the accessibility report's; a completed report whose build ran no tests reads failed." @@ -951,9 +958,7 @@ "id_convention": "A plain integer, discovered through the report list.", "parents": [], "relations": [], - "capabilities": [ - "list_assisted_test_reports" - ], + "capabilities": ["list_assisted_test_reports"], "key_facts": [ "Its individual issues are read on the Accessibility dashboard; the list gives its counts." ] @@ -969,7 +974,7 @@ "fp feedback", "ignored issue pattern" ], - "id_convention": "A plain integer, returned when the feedback is saved and on each entry of the memory.", + "id_convention": "A string starting with fp_, returned when the feedback is saved and on each entry of the memory.", "parents": [], "relations": [], "capabilities": [ @@ -1692,6 +1697,12 @@ "schema": { "$schema": "Failure" } + }, + "FpFeedbackDuplicate": { + "description": "Not saved: an active entry already covers the same rule, site (or all sites), scope and predicate. data.existingId names it.", + "schema": { + "$schema": "Failure" + } } } } diff --git a/src/tools/accessiblity-utils/fp-memory.ts b/src/tools/accessiblity-utils/fp-memory.ts index 4a951f8d..91464643 100644 --- a/src/tools/accessiblity-utils/fp-memory.ts +++ b/src/tools/accessiblity-utils/fp-memory.ts @@ -20,22 +20,27 @@ export const READ_CAPABILITY = "get_false_positive_memory"; /** A read made while answering with issues must not hold the issues back for long. */ const MEMORY_READ_TIMEOUT_MS = 8_000; +/** The most entries one read returns; more than the answer's budget below holds anyway. */ +const MEMORY_PAGE_SIZE = 200; + /** How much of an answer the memory may take; a rule's oldest entries go first. */ export const MEMORY_CHARACTERS = 24_000; const SNIPPET_CHARACTERS = 200; export interface FpEntry { - id: number; + id: string; site: string; global: boolean; verdict: string; - matchScope: string; + scope: string; rootCause: string; pattern: Record; evidence?: Record | null; counterExamples?: unknown[] | null; userStatement: string; - seedIssue?: { pageUrl?: string; component?: string; htmlSnippet?: string }; + pageUrl?: string; + component?: string; + htmlSnippet?: string; } export interface FpRuleMemory { @@ -47,7 +52,8 @@ export interface FpRuleMemory { export interface FpMemory { rules: FpRuleMemory[]; - truncated: boolean; + /** Set when the account holds more entries for these rules than one read returns. */ + nextCursor: string | null; } /** @@ -62,7 +68,7 @@ export async function memoryForReport( if (ruleCodes.length === 0) return undefined; try { const resp = await apiClient.get({ - url: `${getRegionalHost("accessibilityApi")}/api/fp-memory/v1/memory?ruleCodes=${encodeURIComponent(ruleCodes.join(","))}`, + url: `${getRegionalHost("accessibilityApi")}/api/fp-memory/v1/memory?ruleCodes=${encodeURIComponent(ruleCodes.join(","))}&limit=${MEMORY_PAGE_SIZE}`, headers: { Authorization: "Basic " + @@ -86,12 +92,12 @@ export async function memoryForReport( } function compactEntry(entry: FpEntry) { - const snippet = entry.seedIssue?.htmlSnippet ?? ""; + const snippet = entry.htmlSnippet ?? ""; return { id: entry.id, site: entry.global ? "all sites" : entry.site, verdict: entry.verdict, - scope: entry.matchScope, + scope: entry.scope, root_cause: entry.rootCause, pattern: entry.pattern, ...(entry.counterExamples?.length @@ -100,8 +106,8 @@ function compactEntry(entry: FpEntry) { ...(entry.evidence ? { evidence: entry.evidence } : {}), user_statement: entry.userStatement, seed_issue: { - page: entry.seedIssue?.pageUrl, - component: entry.seedIssue?.component, + page: entry.pageUrl, + component: entry.component, ...(snippet ? { html: @@ -159,7 +165,7 @@ interface IssueLike { /** What an answer with issues says about the memory, and where. */ export interface MemoryView { /** The issues, those with saved entries for their rule and site marked saved_fp_entries. */ - records: (T & { saved_fp_entries?: number[] })[]; + records: (T & { saved_fp_entries?: string[] })[]; /** How many of them are marked. */ marked: number; /** First in the answer: remove the false positives before giving any list or count. */ @@ -193,7 +199,7 @@ export function applyMemory( ): MemoryView { if (!memory) return { records, marked: 0, lead: [], notes: [], closing: [] }; const byRule = new Map(memory.rules.map((rule) => [rule.ruleCode, rule])); - const used = new Map>(); + const used = new Map>(); let marked = 0; const annotated = records.map((record) => { const host = hostOf(record.affected_page ?? page); @@ -202,7 +208,7 @@ export function applyMemory( .map((entry) => entry.id); if (ids.length === 0) return record; marked++; - const seen = used.get(record.issue_type) ?? new Set(); + const seen = used.get(record.issue_type) ?? new Set(); ids.forEach((id) => seen.add(id)); used.set(record.issue_type, seen); return { ...record, saved_fp_entries: ids }; @@ -211,7 +217,7 @@ export function applyMemory( return { records, marked, lead: [], notes: [FP_FILING_GUIDE], closing: [] }; } const shown: FpMemory = { - truncated: memory.truncated, + nextCursor: memory.nextCursor, rules: memory.rules .filter((rule) => used.has(rule.ruleCode)) .map((rule) => { @@ -231,9 +237,9 @@ export function applyMemory( notes: [ FP_APPLY_NOTE, `False-positive memory: ${wrapUntrusted("saved false positives (user-written)", json)}`, - ...(omitted.length || memory.truncated + ...(omitted.length || memory.nextCursor ? [ - `Not all entries fit here${omitted.length ? ` (${omitted.map((o) => `${o.count} older entries of ${o.rule}`).join(", ")})` : ""}. When a marked issue is not covered by the entries above, read its rule's entries with the web_accessibility capability ${READ_CAPABILITY}.`, + `Not all entries fit here${omitted.length ? ` (${omitted.map((o) => `${o.count} older entries of ${o.rule}`).join(", ")})` : ""}. When a marked issue is not covered by the entries above, read its rule's entries with the web_accessibility capability ${READ_CAPABILITY} (ruleCodes set to its rule; pass each page's nextCursor as cursor for the next).`, ] : []), FP_FILING_GUIDE, diff --git a/tests/tools/accessibilityFpMemoryAnswer.test.ts b/tests/tools/accessibilityFpMemoryAnswer.test.ts index 794fbf01..9ea077ce 100644 --- a/tests/tools/accessibilityFpMemoryAnswer.test.ts +++ b/tests/tools/accessibilityFpMemoryAnswer.test.ts @@ -106,7 +106,7 @@ const REPORT = { }; const MEMORY = { - truncated: false, + nextCursor: null, rules: [ { ruleCode: "role-required", @@ -115,11 +115,11 @@ const MEMORY = { entryCount: 1, entries: [ { - id: 41, + id: "fp_41", site: "www.shop.example", global: false, verdict: "false_positive", - matchScope: "component_family", + scope: "component_family", rootCause: "Role is set inside each acme- custom element's shadow root; the host has none.", pattern: { @@ -176,8 +176,8 @@ describe("fetchAccessibilityIssues with saved false positives", () => { const issues = JSON.parse(block.split("\n")[2]).issues; expect(issues.map((i: any) => i.saved_fp_entries)).toEqual([ - [41], - [41], + ["fp_41"], + ["fp_41"], undefined, ]); }); @@ -201,7 +201,7 @@ describe("fetchAccessibilityIssues with saved false positives", () => { it("answers as it always did when no saved entry covers these issues", async () => { m.memoryForReport.mockResolvedValue({ - truncated: false, + nextCursor: null, rules: [ { ...MEMORY.rules[0], diff --git a/tests/tools/fpMemory.test.ts b/tests/tools/fpMemory.test.ts index 1808c79b..5d548476 100644 --- a/tests/tools/fpMemory.test.ts +++ b/tests/tools/fpMemory.test.ts @@ -32,11 +32,11 @@ const AUTH = { username: "u", password: "k" }; function entry(id: number, extra: Partial = {}): FpEntry { return { - id, + id: `fp_${id}`, site: "www.shop.example", global: false, verdict: "false_positive", - matchScope: "component_family", + scope: "component_family", rootCause: "The role is set inside the custom element's shadow root; the host has no role attribute.", pattern: { @@ -45,11 +45,9 @@ function entry(id: number, extra: Partial = {}): FpEntry { }, counterExamples: ["div.to_sign_in"], userStatement: "These Acme components work with VoiceOver", - seedIssue: { - pageUrl: "https://www.shop.example/de/angebote/halbtax", - component: "acme-header-button#", - htmlSnippet: "", - }, + pageUrl: "https://www.shop.example/de/angebote/halbtax", + component: "acme-header-button#", + htmlSnippet: "", ...extra, }; } @@ -64,7 +62,7 @@ function memory(entries: FpEntry[], ruleCode = "role-required"): FpMemory { entries, }, ], - truncated: false, + nextCursor: null, }; } @@ -80,10 +78,10 @@ describe("memoryForReport", () => { const result = await memoryForReport(AUTH, ["role-required", "image-alt"]); - expect(result?.rules[0].entries[0].id).toBe(1); + expect(result?.rules[0].entries[0].id).toBe("fp_1"); const call = get.mock.calls[0][0]; expect(call.url).toContain( - "/api/fp-memory/v1/memory?ruleCodes=role-required%2Cimage-alt", + "/api/fp-memory/v1/memory?ruleCodes=role-required%2Cimage-alt&limit=200", ); expect(call.headers.Authorization).toBe( "Basic " + Buffer.from("u:k").toString("base64"), @@ -130,7 +128,7 @@ describe("applyMemory", () => { }); it("only says how to save one when no saved entry covers these issues", () => { - const view = applyMemory({ rules: [], truncated: false }, [ + const view = applyMemory({ rules: [], nextCursor: null }, [ issue("role-required"), ]); expect(view.marked).toBe(0); @@ -146,7 +144,7 @@ describe("applyMemory", () => { ]); expect(view.marked).toBe(1); - expect(view.records[0].saved_fp_entries).toEqual([1, 2]); + expect(view.records[0].saved_fp_entries).toEqual(["fp_1", "fp_2"]); expect(view.records[1].saved_fp_entries).toBeUndefined(); expect(view.records[2].saved_fp_entries).toBeUndefined(); }); @@ -155,7 +153,7 @@ describe("applyMemory", () => { const view = applyMemory(memory([entry(1, { global: true })]), [ issue("role-required", "https://other.example/page"), ]); - expect(view.records[0].saved_fp_entries).toEqual([1]); + expect(view.records[0].saved_fp_entries).toEqual(["fp_1"]); expect(view.notes[1]).toContain("all sites"); }); @@ -165,7 +163,7 @@ describe("applyMemory", () => { [{ issue_type: "role-required" }], ON_SITE, ); - expect(view.records[0].saved_fp_entries).toEqual([1]); + expect(view.records[0].saved_fp_entries).toEqual(["fp_1"]); }); it("opens by saying to remove the marked issues before giving any list or count", () => { @@ -193,17 +191,29 @@ describe("applyMemory", () => { "https://www.browserstack.com/docs/accessibility/rules/a11y-engine/role-required/", ); expect(view.notes[1]).toContain("div.to_sign_in"); + expect(view.notes.join("\n")).not.toContain("Not all entries fit"); const shown = JSON.parse( view.notes[1].slice( view.notes[1].indexOf("[{"), view.notes[1].lastIndexOf("}]") + 2, ), ); - expect(shown[0].entries.map((e: any) => e.id)).toEqual([1]); + expect(shown[0].entries.map((e: any) => e.id)).toEqual(["fp_1"]); expect(view.notes.at(-1)).toBe(FP_FILING_GUIDE); }); }); +describe("applyMemory with more entries than one read returns", () => { + it("says how to read the next page", () => { + const view = applyMemory({ ...memory([entry(1)]), nextCursor: "fp_1" }, [ + issue("role-required"), + ]); + const note = view.notes.find((n) => n.startsWith("Not all entries fit")); + expect(note).toContain(READ_CAPABILITY); + expect(note).toContain("nextCursor"); + }); +}); + describe("memoryPayload", () => { it("leaves out the fullest rule's oldest entries first, and counts them", () => { const many = Array.from({ length: 40 }, (_, i) => entry(100 - i)); @@ -211,7 +221,7 @@ describe("memoryPayload", () => { expect(json.length).toBeLessThanOrEqual(4_000); const kept = JSON.parse(json)[0].entries.map((e: any) => e.id); - expect(kept[0]).toBe(100); + expect(kept[0]).toBe("fp_100"); expect(omitted).toEqual([ { rule: "role-required", count: 40 - kept.length }, ]); @@ -219,7 +229,7 @@ describe("memoryPayload", () => { it("shortens long HTML snippets", () => { const long = entry(1, { - seedIssue: { htmlSnippet: "
" + "x".repeat(1000) + "
" }, + htmlSnippet: "
" + "x".repeat(1000) + "
", }); const html = JSON.parse(memoryPayload(memory([long])).json)[0].entries[0] .seed_issue.html; From b80afa368e52926fca25283890f25ba009b62e81 Mon Sep 17 00:00:00 2001 From: Arjun Chikara Date: Fri, 6 Nov 2026 18:22:39 +0530 Subject: [PATCH 4/4] fix(accessibility): read FP memory for the page's site and flag unread pages Co-Authored-By: Claude Opus 5.5 --- src/.DS_Store | Bin 0 -> 6148 bytes src/tools/accessibility.ts | 8 +++- src/tools/accessiblity-utils/fp-memory.ts | 31 ++++++++++++-- tests/.DS_Store | Bin 0 -> 6148 bytes tests/tools/fpMemory.test.ts | 47 +++++++++++++++++++++- 5 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 src/.DS_Store create mode 100644 tests/.DS_Store diff --git a/src/.DS_Store b/src/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..907b7914c2ad30c6ced80f187d5ad2f898e97557 GIT binary patch literal 6148 zcmeHK%}T>S5T0o(1iTcy=*fKq`vk_4di2r_KRekEvYR0yo*@#VX5CwLbcQfGphxQPH1v?=JgQ!D zYd`Ph>&NFp^Y-G~Zkv19Bg=mx>0O)wXTTY72KJl*yQ7Q4J?FuE{!4s@vm08XGzg0a*R5)%wd!>9-ggf$eXp=>1vYdG}5{7S>9sNuv`e6VHyE?zig z$NC|;6PJqKI|I%@&cIlwQ@Q^a_+$o){5-^G&VV!U&luoIwXEjYl-;d6+mpLCpxvN} rh+mWe1mo-{024V!j+0UCLB{Ya4Wpv0BKi~#^dEslh0FQ!D- literal 0 HcmV?d00001 diff --git a/src/tools/accessibility.ts b/src/tools/accessibility.ts index 26c698e1..e03c6585 100644 --- a/src/tools/accessibility.ts +++ b/src/tools/accessibility.ts @@ -1056,7 +1056,7 @@ async function fetchAccessibilityIssues( // The memory for this page's rules comes with the issues, so it is applied whether or // not the agent thinks to ask for it. const fp = applyMemory( - await memoryForReport(setupAuth(config), Object.keys(rules ?? {})), + await memoryForReport(setupAuth(config), Object.keys(rules ?? {}), pageUrl), records, pageUrl, ); @@ -1603,7 +1603,11 @@ async function runAccessibilityScan( settings, applied, applyMemory( - await memoryForReport(setupAuth(config), Object.keys(report.rules ?? {})), + await memoryForReport( + setupAuth(config), + Object.keys(report.rules ?? {}), + pageURL, + ), report.records, pageURL, ), diff --git a/src/tools/accessiblity-utils/fp-memory.ts b/src/tools/accessiblity-utils/fp-memory.ts index 91464643..db9f0d86 100644 --- a/src/tools/accessiblity-utils/fp-memory.ts +++ b/src/tools/accessiblity-utils/fp-memory.ts @@ -64,11 +64,14 @@ export interface FpMemory { export async function memoryForReport( auth: { username: string; password: string }, ruleCodes: string[], + page?: string, ): Promise { if (ruleCodes.length === 0) return undefined; + // With the page known, other sites' entries stay out of the page of entries read. + const site = page ? `&site=${encodeURIComponent(page)}` : ""; try { const resp = await apiClient.get({ - url: `${getRegionalHost("accessibilityApi")}/api/fp-memory/v1/memory?ruleCodes=${encodeURIComponent(ruleCodes.join(","))}&limit=${MEMORY_PAGE_SIZE}`, + url: `${getRegionalHost("accessibilityApi")}/api/fp-memory/v1/memory?ruleCodes=${encodeURIComponent(ruleCodes.join(","))}&limit=${MEMORY_PAGE_SIZE}${site}`, headers: { Authorization: "Basic " + @@ -82,7 +85,18 @@ export async function memoryForReport( logger.warn(`False-positive memory not read: HTTP ${resp.status}`); return undefined; } - return resp.data.data as FpMemory; + const memory = resp.data.data; + if (!Array.isArray(memory?.rules)) { + logger.warn("False-positive memory not read: no rules in the response"); + return undefined; + } + return { + nextCursor: memory.nextCursor ?? null, + rules: memory.rules.map((rule: FpRuleMemory) => ({ + ...rule, + entries: Array.isArray(rule.entries) ? rule.entries : [], + })), + }; } catch (error) { logger.warn( `False-positive memory not read: ${error instanceof Error ? error.message : String(error)}`, @@ -214,7 +228,18 @@ export function applyMemory( return { ...record, saved_fp_entries: ids }; }); if (marked === 0) { - return { records, marked, lead: [], notes: [FP_FILING_GUIDE], closing: [] }; + const unread = memory.nextCursor + ? [ + `Not all saved false positives were read here. Before you tell the user an issue is real when they say it is a known false positive, read its rule's entries with the web_accessibility capability ${READ_CAPABILITY} (ruleCodes set to its rule; pass each page's nextCursor as cursor for the next).`, + ] + : []; + return { + records, + marked, + lead: [], + notes: [...unread, FP_FILING_GUIDE], + closing: [], + }; } const shown: FpMemory = { nextCursor: memory.nextCursor, diff --git a/tests/.DS_Store b/tests/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..f27e03dee71814141362efa850f0b5703ed08cf1 GIT binary patch literal 6148 zcmeHKJ5K^Z5T5l=i8h!}SXf+TkJ@_>d{jn5V<~v}NDhyvL<>#sSNMCZ)W+6YI};Os zgWv3~u*(6Z8Z$$7zhfRdbKh}0ED@<%FRl=kh^UCcSeU~!LfFnDCsS5a1}a{o(l|^z z-JG8&gaKjTZ!*B|Za{5Xp>}fYzQ40u14}2Z*IP;2f~NTJ(z;sTd8)eBc!ITi;;zd3 z3^tP^I;Cs6q#m6k_l1{v^<#OIk6Xk2Qv6**Zmmu_sAiI%SJzs|bp@25u9jDg3LXa* zw}#LCehoIx&78M-`BcXFW~f6=Is*$$O1!L8eH*sBxo_isZf^~@f^j}x9kk1JOyfzU z_#{J5Vqx#yx4Jbv-)!G)n%{vF?^ZC{!^>ZO<@_SQ!vK0VOL5+y%))>$APn>w;Qb-Q zV2mtg2KCl~Ngn}#1*o;)TK9Nh$O>R&F*67aM43>a303xpp-edRq4kR_W(G|-DV-VX z*v!hFP?XLNeW=|@MFwRS2802hfxvt%asS^KfByG_ = {}): FpEntry { return { @@ -89,6 +90,40 @@ describe("memoryForReport", () => { expect(call.timeout).toBeGreaterThan(0); }); + it("reads only the page's site (and all-sites entries) when the page is known", async () => { + get.mockResolvedValue({ + ok: true, + status: 200, + data: { success: true, data: memory([entry(1)]) }, + }); + + await memoryForReport(AUTH, ["role-required"], ON_SITE); + + expect(get.mock.calls[0][0].url).toContain( + `&site=${encodeURIComponent(ON_SITE)}`, + ); + }); + + it("answers without memory when the response has no rules, and reads a rule without entries as empty", async () => { + get.mockResolvedValue({ + ok: true, + status: 200, + data: { success: true, data: {} }, + }); + expect(await memoryForReport(AUTH, ["role-required"])).toBeUndefined(); + + get.mockResolvedValue({ + ok: true, + status: 200, + data: { success: true, data: { rules: [{ ruleCode: "role-required" }] } }, + }); + const result = await memoryForReport(AUTH, ["role-required"]); + expect(result).toEqual({ + nextCursor: null, + rules: [{ ruleCode: "role-required", entries: [] }], + }); + }); + it("does not ask when the answer names no rules", async () => { expect(await memoryForReport(AUTH, [])).toBeUndefined(); expect(get).not.toHaveBeenCalled(); @@ -107,7 +142,6 @@ describe("memoryForReport", () => { }); }); -const ON_SITE = "https://www.shop.example/de/angebote"; const issue = (issue_type: string, affected_page = ON_SITE) => ({ issue_type, affected_page, @@ -212,6 +246,17 @@ describe("applyMemory with more entries than one read returns", () => { expect(note).toContain(READ_CAPABILITY); expect(note).toContain("nextCursor"); }); + + it("says so even when no issue here is covered by the page read", () => { + const view = applyMemory( + { ...memory([entry(1, { site: "other.example" })]), nextCursor: "fp_1" }, + [issue("role-required")], + ); + expect(view.marked).toBe(0); + expect(view.notes[0]).toMatch(/Not all saved false positives were read/); + expect(view.notes[0]).toContain(READ_CAPABILITY); + expect(view.notes.at(-1)).toBe(FP_FILING_GUIDE); + }); }); describe("memoryPayload", () => {