fix(accessibility): dogfooding fixes from 3 Nov — failed runs, build compare, re-scan, Local tunnels, report links by Aaryan430 · Pull Request #553 · browserstack/mcp-server

Aaryan430 · 2026-11-05T08:19:24Z

Stack: #527 → #541 → #553 (this, based on #541). Merge in that order.

Fixes from the accessibility MCP dogfooding on 3–4 Nov, combined into one PR, plus #552's changes (closed in favour of this). Each change is its own commit.

A failed run is no longer read as a clean page

Before: a failed Website Scanner run came back as "0 issues ✅ All issues retrieved". A failed Automated Tests build threw "Failed to fetch report: null". Issues found on an error page (Chrome's, or the BrowserStack Local page shown when the tunnel is down) were counted as the real page's issues. Pages a completed scan could not reach were simply missing from the answer.
After: a failed scan run, build or Workflow Analyzer report returns an error that says nothing was checked, with the scan's own reasons, and no export is started. A cancelled scan run gets the same answer. Pages that showed an error page are set apart and left out of the totals, with a note to check the page or tunnel and scan again. The first page of a read lists the pages that were not scanned, and the last page no longer says every issue was retrieved. Comparing against a failed or cancelled run is refused, because every issue would otherwise read as fixed or new.

Scan failure reasons are quoted as untrusted data

Before: failure reasons and page lines from the scan logs (URLs, HTTP statuses, redirect targets) were printed inline with the agent's instructions.
After: they sit in an untrusted-data block, like report issues already do. The guidance around them stays outside the block.

compareWith on a report that has no compare

Before: compareWith on a Workflow Analyzer report or a manual app report returned an error ("… is neither"), and the agent spent a call working out what to do next.
After: the answer tells the agent to read both reports and match issues by rule and selector on each page (by rule and component for app reports). It comes back as guidance, not as an error.

Compare Automated Tests builds

Before: compareWith on an Automated Tests (web) build was not supported. In dogfooding, all 3 agents passed correct testhub ids and got no comparison.
After: compareWith accepts another build's id, its report link, or "previous". The older build is always treated as "before". When both builds share a build name and project, the dashboard's compare API is used. Otherwise, or when the API refuses or times out, the two exports are matched by rule, element and page. The answer says which method it used and gives new, fixed and still-open lists with counts. Issues found on error pages are set apart. A failed build (no test cases) is reported as such and is not compared.

Re-run an earlier scan instead of creating a new one

Before: startAccessibilityScan always created a new scan. Re-scanning a page after a fix therefore produced a second scan, and comparing it with the first failed with "Scan logs not found".
After: a new optional scanId re-runs that scan with its saved settings, as long as it scans the same page, so compareWith: "previous" works. If the rule settings passed differ from the saved ones, nothing runs and the answer lists each difference. A login (authConfigId) or Local identifier can't be checked, since the API doesn't return them, so the run goes ahead with the saved ones and says each passed one was not used. A scan saved with Local is re-run only when localIdentifier is passed; otherwise a new scan is started, and says why. A local page the MCP tunnelled itself gets no re-run hint, since its tunnel's identifier was never shown. Expected refusals are plain answers, not errors: a run still in progress, an unknown scan, a deleted login config, or the plan's saved-scan limit. Every new scan of a public page says how to re-run it.

Plain answers when a local URL can't be scanned

When the hosted MCP, or USE_OWN_LOCAL_BINARY_PROCESS, can't tunnel to a local URL, the answer explains how to pass your own tunnel's name. It comes back as a plain answer, not an error with a "report on GitHub" line. BrowserStack Local tunnel handling itself is unchanged from #541.

Find older scans from a report link

Before: resolving a Website Scanner dashboard link searched 15 pages (about 300 scans), so a link to an older scan came back "not found".
After: it searches 25 pages (about 500 scans), and a build id it has found once is remembered for the life of the process, so a repeat lookup or compare skips the search. This is a stopgap until the issues API can read by thBuildId.

No 5-minute wait on a finalizing report; account refusals as plain answers; expert questions within 500 characters

Before: fetchAccessibilityIssues could wait up to 5 minutes for a report that was still finalizing. The claude.ai connector cuts off a tool call after about 305 s of silence, so the agent got nothing back. In one case it reported a fix as verified when the fix had added 26 issues. Plan limits, a missing External APIs add-on, and Website Scanner's "user info missing" all read as faults, so agents retried, renamed the scan, or sent submitFeedback. accessibilityExpert questions over 500 characters were refused with "Query is too long".
After:

A report that is still finalizing returns at once with "no issues have been read yet; call again in a minute or two" and the exact call to repeat. It warns not to call the page clean.
The three account refusals each say what to tell the user (delete scans or upgrade, add External APIs, open Website Scanner once).
Long expert questions are shortened to fit, keeping the opening and the final question, and the answer notes that this was done.

Friction found in end-to-end testing of this PR, fixed

84 isolated agent sessions on this branch found 7 places where agents needed extra calls, hit errors, or gave no answer:

A Website Scanner link or build id as compareWith is resolved to its run. The older run is always "before" (this also fixes a silent new/fixed swap). A run from another scan gets plain "read both" guidance instead of "Scan run id must be an Integer" or "Scan logs not found".
A run or build still processing is not compared. The answer says it isn't finished, instead of showing a clean-looking diff.
A bare Website Scanner build id as buildId is found by walking the scan list.
A bare Automate hashed_id (as the SDK prints it) is read as that build's accessibility report.
A build stuck in "running" says "still running since ", or "probably stuck" after 24 h, instead of "Failed to fetch report: null".
compareWith: "previous" on a Workflow Analyzer or manual app report names the earlier report to read.
App report pages stay under about 35,000 characters, so Claude Code shows them inline, and cursor is described as the next_page value.

Re-run: all 13 sessions that had friction are seamless, and 7 regression sessions give the same answers.

Review fixes (6 Nov)

An export started while a report was still processing is not picked up later as the finished report.
A 40-character Automate id that Automate can't answer for (401/403/5xx, or an empty build) falls through to the App Automate check and then a plain not-found, instead of ending the call with an HTTP error.
When a web build's start time is missing, the order falls back to build number (same build name and project); otherwise the passed order is kept and the answer says so.
The saved scan's pages and name are quoted as untrusted data in re-run answers.
Expert questions in Chinese or Japanese are shortened at sentence ends and never mid-character; the note says what was kept.
The plan's scan limit is the number the API gives, not a hard-coded 50.

Testing

Tests for the new behaviour: web-build compare (order, missing start times, refusing processing or failed builds, the compare API and the export-matching fallback), re-runs (same or other page, settings that differ, Local without localIdentifier, the re-run hint), failed and cancelled runs, the 40-character Automate id lookup and its fall-through, export reuse while processing and by build id, the build-id lookup cache, the scan-limit message and expert-question shortening. Tests for code reverted from this PR (Local tunnel handling) are left out.
The new helpers live in new files under src/tools/accessiblity-utils/ (web-build-compare.ts, compare-with.ts, compare-lists.ts, report-answers.ts, scan-rerun.ts, error-messages.ts, expert-query.ts, and shared.ts for small existing helpers they need), so accessibility.ts grows by about 110 lines rather than 1,500.
Merged with main. The new Accessibility API calls (Automated Tests compare, build lookups, scan logs, scan overview, re-run, runs list) go to the account's region (feat(lib): route Automate, App Automate, Accessibility and Observability to the account's GRR region #489), not the US host.
Merged with fix(accessibility): scan fixes from the 2 Nov stability report #541 and fix(accessibility): read any report link directly: scanner links by their run's build id, Workflow Analyzer links #527: a report link read by its build id goes through this PR's compare flow. vitest run: 147 files, 2,205 tests pass. tsc, eslint and prettier --check are clean.
Tool-description budget: within 21,500 characters (the budget test passes).
Live smoke on the stacked build (isolated client, read-only): a Website Scanner link read (61 issues) and an Automated Tests build compare (13 fixed, 6 new, 1 still open) returned the expected answers.
After the review fixes, every earlier end-to-end case was re-run in isolated sessions (91 in all: report reads and links, compares, scans and re-runs, local and hosted). Every answer was correct, none had friction (68 seamless, 23 minor) and none regressed because of these changes. Not reachable live: a cancelled run (none on the test account), a missing build start time, a scan of a local page the MCP tunnelled itself, the scan limit, and the connect retry; these are covered by the tests above.
Build-id lookup cache, checked live: during a re-run the scan list's link (thBuildId) and lastScanDetails.scanRunId never pointed at different runs (25 samples). The link is absent for the run's first ~25 s, then always carries the new run's own id, so a remembered build id → run mapping stays correct.

🤖 Generated with Claude Code

…f reading it as clean fetchAccessibilityIssues read runs that checked nothing as clean or complete reports: - A failed Website Scanner run exported an empty report and read as "0 issues ✅ All issues retrieved"; a failed Automated Tests build (the API's status for one that completed with no test cases) threw "Failed to fetch report: null". A failed scan run, build or Workflow Analyzer report is now an error that says so, with the scan's reasons from its logs, and no export is started. An app build's "failed" is its tests' result, so it is read as before. Comparing scan runs refuses a failed run, whose empty report read every issue of the other run as resolved or new. - A page that did not load had an error page (Chrome's, or BrowserStack Local's privoxy page) scanned in its place, and its issues came back as the page's. The report parser now sets such pages apart, as the comparison already did, keeping their rows out of the issues, totals, summary and duplicates; the read (and startAccessibilityScan) leads with a note. - A completed scan's pages that failed to scan were simply absent. Its scan logs are read with the export and the first page lists them (redirects aside, as those pages were scanned where they redirected to). With pages skipped, the last page no longer claims every issue. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…untrusted data The reasons a run or a page failed, and the pages a completed run could not scan, come from the scan logs and the site: the page's address, its HTTP status, where it redirected. They are now quoted in an untrusted-data block, as the report's issues and page addresses are, with the sentences around them ("N pages were not scanned …:", "Fix the cause and scan again.") left outside it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…eading both compareWith on a Workflow Analyzer report, an Automated Tests build or a manual app report answered with an error ("... is neither"), and agents spent a call working out that they should read both reports and compare them by hand. The answer now says so directly, as guidance rather than an error: read both reports with fetchAccessibilityIssues and match issues by rule and CSS selector on each page (by rule and component for app reports); an issue in the later report only is new, in the earlier only is fixed, in both is still open. compareWith naming a report that is not an app build, for an app build, gets the same guidance. The compareWith description now says which reports it compares (Scanner runs of one scan, app builds) and to read both for any other. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

compareWith on an Automated Tests (web) build answered "can't be compared here yet" and then, since 6a777ac, told the agent to read both builds and compare them by hand. In MCP dogfooding (3 Nov) 3 of 3 agents passed correct testhub ids and got no compare. compareWith on such a build (buildId, or its report link as reportUrl) now takes another build's id, its report link, or "previous", and compares them: - The older build, by start time, is "before" and the newer "after", whichever order they were passed in. When compareWith is the newer one the answer says so in one line. Both builds are named (name, project, #number, id, start), the names quoted as data. - Two builds of one build name and project are compared by the Automated Tests compare API (GET /api/automated-tests/v1/builds/compare/issues, newReport = after, baseReport = before). It takes the builds in the order given and does not check it. "previous" calls it without baseReport, and the answer names the build the API picked. Its issues are keyed by the API's own signatureKey, and the answer says it matches by that, not by rule, element and page alone. The API told apart a css-orientation-lock issue on <html> before and after a fix added lang to it, and listed it as both fixed and new. With a rule|element|page key on both, agents recounted the dashboard's 6/13/1 to 4/11/3. - Any other pair, or a pair the API refuses (issue-count threshold, "TestRun Not Found") or does not answer (504 after its 30 s, or no answer within 40 s), is compared by matching the two builds' issue exports by rule, element and page, as Scanner runs are. The answer says which method was used, in one line. - Either way the answer gives three lists with counts: new (only in after), fixed (only in before), still open (in both). Each list shows at most 50 issues and says how many more. For the API, a list cut short on its first page (100 rows) is read on its own. - Issues found on a BrowserStack Local or Chrome network error page are set apart as "page did not load". The diff also catches a page whose error-page issues are all duplicates of another page's, as a grouped build's are; before, that page's earlier issues read as fixed. - A build the build list calls "failed" (completed with no test cases) has no accessibility results: the answer says so as guidance and does not compare. "previous" with no earlier completed run of the build's name is said plainly, with no request to report it. The compareWith description now names web builds (21,495 of the 21,500 budget). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…of starting a new one startAccessibilityScan always created a new scan, so re-scanning a page after a fix made a second scan whose runs could not be compared with the first (fetchAccessibilityIssues compareWith failed with "Scan logs not found"). - New optional scanId. When that scan is for this same page (compared as startScan saves URLs: local hosts as bs-local.com, host case and a trailing slash ignored), start a new run of it (POST /scans/:id/new_scan) with its saved settings, wait for it like a new scan, and end with how to compare with compareWith "previous". Without scanId a new scan is created as before. - scanId for another page (or several pages): a new scan, saying why. - Settings passed that differ from the scan's saved ones: no run; each difference and the two ways forward are listed. Only settings the call passed are compared; a login or tunnel can only be checked for being saved at all, which is said. - A re-run keeps the scan's name (a different name passed is mentioned), and a Local scan's re-run says the tunnel must run with the saved identifier. - Re-run refusals as plain results: a run still going (with its run id), a deleted login config, an unknown scan, the plan's message. - Every new scan says how to scan the same page again with its scanId. - The 50-scan plan limit on create is said plainly instead of "open an issue on GitHub". - Shorter wording for a few accessibility argument descriptions, to stay within the description budget with the new argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…art; local-URL limits as plain answers Every public-URL accessibility scan and Live session ran `pkill -f BrowserStackLocal` (`taskkill /IM BrowserStackLocal.exe` on Windows), as did starting a tunnel for a local URL, so a tunnel the user had up (their own, or their test suite's) was killed by an unrelated call. - lib/local.ts no longer kills any process by name. A public URL takes no tunnel action at all. - A local-URL scan without localIdentifier runs through this MCP's own tunnel, started through browserstack-local under a unique mcp-<uuid> identifier, kept in module state and reused while it runs. It is replaced (the old one stopped through its own handle) only once it has died or the credentials change, and stopped by its own pid when the process exits or is signalled. The binary runs tunnels with different --local-identifier values side by side, so the user's keeps running; `force` is dropped from the configured options. Should ours fail to start while another Local is running, the error says to pass that tunnel's name as localIdentifier. - Live sessions (the other caller) start their unidentified tunnel only when no other BrowserStack Local is running, and otherwise use the one that is, leaving it alone. - A local URL the MCP cannot tunnel to (the hosted MCP, or USE_OWN_LOCAL_BINARY_PROCESS) is answered as a plain result with #541's guidance (start BrowserStack Local and pass its name as localIdentifier, or scan a public URL), with no GitHub or submitFeedback line. - The accessibility tools' error wrapper adds a full stop only when the message lacks one, so it no longer writes "..". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A Website Scanner dashboard link is resolved by walking the scan list newest first. At the list's default 20 per page, 15 pages reached ~300 scans, so a link to an older scan came back "not found". 25 pages reach ~500, at about 1 s a page (~26 s worst case). Stopgap until the issues API reads by thBuildId (accessibility #5689). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…refusals as plain answers; expert questions within 500 chars fetchAccessibilityIssues waited up to five minutes inside the call for a report still finalizing (or an app build still running its rules). Clients cut off a call that long: the claude.ai connector aborts a tool call silent for 305 s, progress notifications or not, so the agent got nothing, and one reported a fix as verified that had added 26 issues. - A first read of a report that is "finalizing" or "rule_processing" now answers at once. It is not an error: it says no issues were read yet, so the page is not to be called clean nor a fix verified, and gives the exact call to make again in a minute or two (pageUrl and compareWith kept), or to call the report stuck after about 10 minutes. The wait loop and REPORT_STATUS_POLL_MS / REPORT_STATUS_WAIT_MS are gone. A later page, or a report whose tests are still running, is read as before with its "still processing" note. - A cancelled Website Scanner run is answered like a failed one (failedRunResponse): an error saying it has no results and that 0 issues does not mean the page is clean, instead of reading its empty export as "0 issues, all retrieved". - Refusals the user resolves on their own account are explained instead of reported as faults: the plan's saved-scan limit ("make over N scans"), a plan without the External APIs add-on, and Website Scanner's "user info missing" (open Website Scanner once in the dashboard first). Each says what to tell the user and quotes the API's words, with no "open an issue on GitHub" line. They carry NO_FEEDBACK_HINT, a new symbol-keyed flag in lib/feedback-hint.ts that is never serialised, so withFeedbackHint does not send the agent to submitFeedback. (startAccessibilityScan's 402 scan-limit refusal is still answered first by the scanner's ScanRefusedError; this covers the message wherever else it reaches the error handler.) - accessibilityExpert keeps a question within the knowledge base's 500-character limit, which refused longer ones ("Query is too long"). A longer question keeps its last sentence, where agents put the question itself, and as much of its opening as fits, cut at a word break; the answer starts with a note that it was shortened. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-11-05T08:19:36Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Central YAML (base), Organization UI (inherited), Workspace UI (inherited); Enforced overrides: Organization
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 0cd9b1f4-d874-42c0-9d72-0dd196c98bdd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

Trigger review

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Removes the six new test files and the test cases this PR added to existing files. Keeps only the edits that existing tests need because behaviour changed here: no 5-minute wait on a finalizing report, compareWith guidance on reports with no compare, failed-page wording, and no tunnel killing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Resolves report-fetcher.ts against GRR regional routing (#489): keeps the cached build lookup, and sends this branch's new Accessibility API calls (Automated Tests compare, build lookups, scan logs, scan overview, re-run, runs list) to the account's region instead of the US host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…to fix/a11y-dogfooding-3-nov Conflicts: - accessibility.ts (fetchAccessibilityIssues): this branch's flow stays (Automated Tests build compare, readBothToCompare, status read without the 5-minute wait, failed/finalizing answers, unscanned pages). Into it go the report-links pieces: a build id is resolved with orScanRun; under compareWith a scanner run named by its build id is first mapped to its scan's ids (findScanRun), then compared as a scan; otherwise it is read by its build id before the status read, with the scan-list fallback and notes in the preface. The report-links refusal of compareWith on non-app sources is dropped, as this branch now answers those. - scanner.ts (failureDetails): this branch's version, already on scansApi(). - accessibilityScanOutput.test.ts: this branch's imports plus ScanRunNotFoundError; REPORT_STATUS_POLL_MS/REPORT_STATUS_WAIT_MS no longer exist here. MAX_SCAN_LIST_PAGES stays 25. The test files dropped in 7ee4c3d stay dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ocessing report Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…areWith, and compare runs oldest first compareScanRuns took compareWith verbatim as a run id, so a Website Scanner report link or a run's thBuildId failed with "Scan run id must be an Integer", a run id of another scan with "Scan logs not found", and a newer run id swapped new and resolved without a word. compareWith is now resolved as a source is: a link or 40-character id through sourceFromReportUrl / resolveBuild and findScanRun. A run of the same scan is compared with the older run as "before", saying so when the order was swapped. Another scan's run, a build or a report, and a run id the scan's full run list does not hold, get a plain answer naming both reads' arguments. compareWith's description drops "older": the runs are ordered here now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ssing Comparing against a run still processing gave a clean-looking diff ("0 new, 0 resolved, 2 still open") while its issues were still being added, which could read a fix as verified. The Website Scanner, Automated Tests and app build compares now check both sides' statuses first: while either is processing or finalizing, the answer is a plain "not finished yet" naming the run or build and its status, telling the agent to have the user ask again in a few minutes and not to call again in this turn. The Automated Tests compare's "only the issues found so far are compared" warning goes, as such a build is no longer compared. An app build compare is dispatched before the finalizing read, so a build still running its rules gets this answer rather than "call again in a minute or two". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…d in the scan list A Website Scanner run's thBuildId passed as buildId, with no link, was refused whenever the API could not yet read a run by its build id: "pass the scan's report link as reportUrl instead", plus the GitHub and submitFeedback lines. An agent then searched for the run itself, and on an old scan gave up. Such an id now walks the scan list with findScanRun, as a link does, under the same 25-page cap and with the same "found in the scan list instead" note, naming the scan and run it found. Only when the walk finds nothing (or the API that reads runs by build id has none) is the answer a plain not-found, saying what to pass instead, without the GitHub or submitFeedback lines. findScanRun's not-found is now a ScanNotInListError, with the same message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…dId as its accessibility build An SDK run prints the Automate build's hashed_id ("stopBinSession hashed_id=…"), and an agent passing it as buildId was refused, with only a Website Scanner build id offered as an alternative. It then spent recovery calls on fetchBuildInsights and listSessions before finding a session link. A 40-hex id that is neither an Automated Tests nor an App Accessibility build is now tried as an Automate build, then an App Automate build, through the same session mapping their dashboard links use, before the scan-list walk. The answer says which build the id was read as. An id none of them knows says so in the plain not-found answer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…report yet", not "Failed to fetch report: null" A build stuck in "running" since 2025-09-16 was read as processing, so its export ran anyway and failed with error null: "Failed to fetch report: null. Please open an issue on GitHub", and the agent went searching for another product before handing the question back to the user. When a report is still processing and its export fails, the answer is now a plain "still running (status, started <date>), no report yet". If it started more than 24 hours ago it is called probably stuck, to be run again, rather than something to call again for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… for compareWith "previous" compareWith "previous" on a Workflow Analyzer report only said there is no built-in compare and to read both, without saying which report came before. Agents went looking for a way to list reports, hit routing refusals, and the hosted run gave no answer. The earlier report is now found here: the account's report list (newest first) locates this report, and the summaries of the completed reports after it give their start and pages. The newest one before it that scanned one of its pages is named, or else the newest on its site, with its id, its dashboard link and the two reads to make. When none is among the newest reports, the answer says so and to ask the user which report to compare with. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ers, describe cursor, and find its previous report A 70-issue manual report came back as one page of 51,132 characters, over Claude Code's 50,000-character limit, so the client saved it to a file. The page budget counted only the bare issue rows, while the answer prints them as indented JSON with the rules and screens they name. An app page (build or manual report) now counts each issue as printed, and each rule's text and screen's screenshot the first time the page names it, against a 35,000 character budget: with the summary, notes and wrappers the whole answer stays under about 40,000. The cursor still counts the bare issues. Agents then passed cursor as an issue index (40, 35) and got the same page again. cursor is now described as the next_page value from the previous answer, not an issue index. compareWith "previous" on a manual report now names the newest finished manual report of the same app recorded before it (found through the report list, by the report's name and then its app's package), with its id, its dashboard link and the two reads to make, as for a Workflow Analyzer report. Descriptions (total unchanged at 21,500): - cursor: "Pagination offset (default 0); with compareWith, issues already listed." (71) -> "The next_page value from the previous answer (default 0), not an issue index." (77) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… page address that parses Follow-up to the "previous" Workflow Analyzer lookup: a page address that does not parse has no site, and two such addresses no longer count as one site. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

compareWith's description ended "Others: read both", so for a Workflow Analyzer or manual app report agents never passed "previous" and went searching for the earlier report themselves: in re-runs, 4 of 5 sessions skipped it, and one hosted run again gave no answer. "previous" now finds the earlier report for those too, and the tool itself answers any pair it has no compare for with how to read both, so the description says "previous" works for any report. compareWith: 'Runs of one scan, web/app builds: "previous", scanRunId, build id/link. Others: read both.' (90) -> '"previous" (any report), or a run of the same scan or a web/app build: its id or link.' (86). Total 21,500 -> 21,496. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ack hint as they are on #541 Restores src/lib/local.ts, src/lib/feedback-hint.ts and src/tools/live-utils/start-session.ts to their #541 versions, which puts back killExistingBrowserStackLocalProcesses and drops ensureOwnTunnel, stopOwnTunnels, killOwnTunnelsNow and NO_FEEDBACK_HINT. startScan again starts a tunnel with ensureLocalBinarySetup for a local URL and calls killExistingBrowserStackLocalProcesses for a public one. The refusals for the hosted MCP and for USE_OWN_LOCAL_BINARY_PROCESS with a local URL stay ScanRefusedError plain answers, and account refusals keep their wording without the feedback-hint opt-out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…elpers into accessiblity-utils accessibility.ts had grown to 3,337 lines. The helpers this PR added move, unchanged, into accessiblity-utils: - error-messages.ts: asSentence and the account-refusal wording - expert-query.ts: fitting an accessibilityExpert question to 500 characters - report-answers.ts: failed, cancelled, finalizing and no-report-yet answers, unknown build ids, and pages not scanned or that showed an error page - compare-with.ts: what compareWith names (a scan run, the "previous" report), the read-both guidance and the not-finished-yet answer - compare-lists.ts: the export diff, compared lists and not-loaded note that scan-run and web-build comparisons share - web-build-compare.ts: comparing Automated Tests (web) builds - scan-rerun.ts: the saved-settings checks, notes and hints of a re-run shared.ts takes the small helpers of accessibility.ts these need too (the response builders, source names, progress, the scanner client, WCAG names), so neither side imports the other. accessibility.ts re-exports EXPERT_QUERY_MAX_CHARS. No behaviour or tool description changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sarve-shreyas

Reviewed cbe1c17..2649ced (this PR on top of #541). npm run build passes locally on this head: 142 files, 2,103 tests.

Blocking (outside the inline comments): the new behaviour has no new tests. This PR adds web-build compare, scan re-run, compareWith resolution (link / build id / Automate hashed_id), the 40k manual-report paging and expert-question shortening, about 3k lines in all. 7ee4c3d dropped the new test files, and the test diff only rewrites existing cases to match the new behaviour (the suite is one test smaller than #541's). Please bring back tests for at least web-build compare (ordering, refusal of processing/failed builds, the diffExports fallback), rerunAccessibilityScan (same page vs other page, settings mismatch) and the hashed_id path.

The refactor itself looks clean. The moved helpers (sourceArgs, describeSource, exportProgress, scanDashboardNote and others) are unchanged apart from export, and the scanner.ts Local/tunnel code matches #541 after b878a2b.

Non-blocking (outside the diff): stale task id while processing. report-fetcher.ts rememberExport(key, { taskId }) (around L720) stores the task id even when stillProcessing is true. If that export is still pending when the call ends, the next call after the report finishes reuses known.taskId. It then gets the snapshot taken mid-processing and caches it as final. Removing the finalizing wait makes this path more likely. Suggest not remembering the task id while stillProcessing, or storing the flag with it.

Questions I couldn't verify against the API:

While a re-run is in progress, do the scan list's scanUrl (thBuildId) and lastScanDetails.scanRunId point at the same run? If not, findScanRun maps the older run's link to the newer run.
What does Automate return for a 40-hex id it doesn't know, or one from another account (see the hashed_id comment)?
Does the knowledge base count its 500-char limit in UTF-16 units, code points or bytes?

sarve-shreyas · 2026-11-06T05:53:34Z

+      : [
+          `${renameReason(name, startResp.nameChanges ?? ["taken"])}, so this scan was started as "${scanName}".`,
+        ],
+    [rescanHint(String(scanId))],


Blocking: rescanHint is added to every scan, including local URLs the MCP tunnelled itself, and re-running those can't reach the page.

For a local URL with no localIdentifier, startScan saves the scan with randomUUID() as its identifier. That tunnel is killed by the next public-URL scan (killExistingBrowserStackLocalProcesses) or by a restart. rerunScan starts no tunnel, and the API doesn't return the saved identifier, so localRerunNote asks the user for an identifier nobody was ever shown.

Scenario: scan http://localhost:3000 without localIdentifier, scan any public URL, then call startAccessibilityScan with the same scanId. The new run scans the Local error page, and compareWith: "previous" shows the page as not loaded.

Suggestion: any one of these works:

leave rescanHint off MCP-tunnelled local scans;

when the saved scan has localTestingEnabled and no localIdentifier is passed, start a new scan instead of re-running;

give MCP-started tunnels a stable identifier and start that tunnel in rerunAccessibilityScan.

sarve-shreyas · 2026-11-06T05:53:34Z

+    reportFetcher.reportStatus(run(after)),
+    reportFetcher.reportStatus(run(before)),
+  ]);
+  if (status === "failed" || beforeStatus === "failed") {


Non-blocking: only "failed" is refused here. A cancelled run also exports an empty report, and this PR already treats it as failed for reads (the processing.status === "cancelled" branch around L1008). As it stands, comparing against a cancelled run reports every issue of the other run as resolved or new. Suggest treating cancelled the same way and passing the status into failedScanRunResponse.

sarve-shreyas · 2026-11-06T05:53:34Z

+    // id has other letters too), as the SDK prints it at the end of a run: its sessions name
+    // the accessibility build they ran in, as for the build's link. Asked before the scan list.
+    if (/^[0-9a-f]{40}$/.test(source.buildId)) {
+      const session = await this.automateSession(source.buildId);


Non-blocking: this probe ends the call instead of falling through. automateSession throws on any non-404 status and on a 200 with no sessions ("has no sessions"). So a 40-hex id that Automate answers with 401/403/5xx, or an empty Automate build, never reaches the App Automate check or the orScanRun / not-found fallbacks below. Suggest wrapping the probe in try/catch and falling through (you could keep the error as a note if nothing else matches).

sarve-shreyas · 2026-11-06T05:53:34Z

 * at limit=50 or 100 the cursor skips scans, so a larger page cannot be used to go faster.
 */
-export const MAX_SCAN_LIST_PAGES = 15;
+export const MAX_SCAN_LIST_PAGES = 25;


Non-blocking: 25 pages at about 1.2 s each is about 30 s per findScanRun walk. A compare where both sides are scanner links or build ids walks twice, which is about 60 s before the export wait (up to EXPORT_WAIT_MS, 45 s). Web builds can also hit the compare API's 40 s timeout, then the diffExports fallback wait (web-build-compare.ts around L340). The EXPORT_WAIT_MS comment says a call stays well under the 60 s request timeout, which no longer holds. Suggest a shared per-call time budget, a small cache of thBuildId → ids, or returning the pending answer after a compare-API timeout.

sarve-shreyas · 2026-11-06T05:53:35Z

+      : undefined,
+  ]);
+  // The older build is "before": by start, whichever was passed as compareWith.
+  const started = (b: WebBuild) => Date.parse(b.startedAt ?? "");


Non-blocking: when either build has no startedAt, Date.parse("") is NaN, so reversed is always false. An explicit compareWith is then always taken as "before", and new/fixed swap silently if the user passed the newer build there. Suggest falling back to build number / creation order, or saying the order couldn't be determined.

sarve-shreyas · 2026-11-06T05:53:35Z

+ * that identifier, or the run cannot reach the page.
+ */
+export function localRerunNote(localIdentifier: string | undefined): string {
+  return `This scan reaches its page through BrowserStack Local, and the run uses the scan's saved Local settings: a tunnel must be running with the same local identifier the scan was created with${


Non-blocking (wording): the PR body says "If the settings passed differ from the saved ones, nothing runs." But authConfigId (when a login is saved) and localIdentifier (when Local is saved) aren't compared, see uncheckedSettings. The run goes ahead with the saved ones, with only a note. Re-running with a different authConfigId therefore scans with the old login. Either refuse when one of these is passed and can't be checked, or reword the PR body and the scanId description.

sarve-shreyas · 2026-11-06T05:53:35Z

+    return `Scan ${scanId}'s pages could not be read, so it could not be checked to be for ${pageURL}; a new scan was started.`;
+  }
+  if (urlList.length === 1) {
+    return `Scan ${scanId} is for ${urlList[0]}, not ${pageURL}, so a new scan was started.`;


Non-blocking: the saved scan's urlList is printed here without wrapUntrusted, and so is the saved name in the re-run messages in accessibility.ts (the "was not re-run" and "Started a new run" lines). Both come from the account's saved scan, which another user on the account may have created, so they should get the same quoting as the failure reasons. wrapUntrusted itself looks solid (per-call nonce).

sarve-shreyas · 2026-11-06T05:53:35Z

+  if (query.length <= EXPERT_QUERY_MAX_CHARS)
+    return { query, shortened: false };
+  const text = query.trim();
+  const sentences = text.match(/[^.?!]+(?:[.?!]+["')\]]*\s*|$)/g) ?? [text];


Non-blocking: the sentence split only knows ASCII .?!, so CJK text (。？！) is one "sentence" and gets cut at 500, losing the actual question. The same happens whenever the last sentence is over 300 chars. Yet the note in accessibility.ts (~L176) always says the opening and last sentence were sent. Also, .slice on UTF-16 units can split a surrogate pair. Suggest adding the CJK terminators, cutting on a code-point boundary, and making the note match the branch that ran.

sarve-shreyas · 2026-11-06T05:53:35Z

+ */
+const SCAN_LIMIT_REFUSAL = /make over \d+ scans/i;
+
+export const SCAN_LIMIT_MESSAGE =


Nit: SCAN_LIMIT_MESSAGE hard-codes "50", while accountRefusal reads the number from the API's message. Use the captured number so the message stays right if the plan limit differs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…robe falls through - An export started while a report is still processing no longer has its task remembered: continued after the report finished, it handed back the mid-processing snapshot as final. - A run found by its build id is cached (per account, bounded), since a build id names one fixed run: a link read again, or both sides of a compare, skip the scan-list walk. The EXPORT_WAIT_MS comment no longer claims a whole call stays under 60 s. - A 40-hex id that Automate or App Automate can't answer about (an HTTP failure other than 404, or an Automate build with no sessions) moves on to the next API instead of ending the call, and the not-found answer says why. Once a session is found, what it says is still the answer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…; say what a re-run ignores - A local page this MCP tunnelled itself is saved with an identifier nobody was shown, and the tunnel is gone after the next public scan or a restart. Its scan no longer gets the "re-run with scanId" hint, and a re-run asked for a scan saved with Local but no localIdentifier starts a new scan instead, saying why and how to re-run it with its own tunnel. - A re-run can't take a login or a Local identifier: each passed one is now named as not used, with how to scan with it (a new scan), instead of only "could not be checked". - The saved scan's pages and name come from the account, which another user may have set: they are now quoted as untrusted data in re-run answers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ith a failed one A cancelled run has no results and exports an empty report, so compared with it every issue of the other run read as resolved or new. Reads already treat a cancelled run as having no results; compares now do too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…d when start times are missing A build with no start time made the order check always false, so a newer build passed as compareWith was used as "before" and new and fixed issues swapped without a word. The order now falls back to the build number for runs of one build name and project; with neither, the passed order is kept and the answer says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… what was kept The sentence split knew only . ? !, so a Chinese or Japanese question was one "sentence" cut at 500, losing the question at its end. 。？！ now end sentences too. A cut no longer leaves half a surrogate pair, and the note says whether the opening alone or the opening and last sentence were sent, as it happened. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…rd-coded 50 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… in a re-run refusal The quoted name sat between "these differ:" and the list, so it read as one of the differences. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sarve-shreyas

Re-reviewed 2649ced..85f3f09 (eight fix commits plus the #541 merge). npm run build passes locally: 142 files, 2,103 tests.

Resolved:

Local re-run (was blocking): no re-run hint for local pages this MCP tunnelled, and a re-run of a Local scan without localIdentifier starts a new scan and says why (7d73b1e).
Cancelled runs are refused in compares, like failed ones (4c59ace).
The Automate hashed_id probe falls through on a non-404 or an empty build, and the not-found answer names the probe failure (3377151).
Scan-list walks: thBuildId → ids is cached per account, bounded at 200, and the 60 s claim in the EXPORT_WAIT_MS comment is corrected.
The stale task id while processing is no longer remembered.
Web-build order: falls back to build number, otherwise says the order is unknown (cbf63d0).
A passed authConfigId/localIdentifier on a re-run is now named as not used; the saved name and pages are wrapped as untrusted.
Expert questions: CJK terminators, no split surrogate pair, and the note says what was kept.
The scan limit comes from the API message.

Still open (blocking): the new behaviour still has no new tests. The suite is 2,103 on this head, the same as before this round, and one fewer than #541's. This round's fixes (re-run local handling, cancelled-run compare, probe fall-through, the build-id cache, NaN ordering) are also untested. Please add tests for at least web-build compare (ordering including the build-number fallback, refusal of processing/failed builds, the diffExports fallback), rerunAccessibilityScan (same vs other page, settings mismatch, Local without identifier), and the hashed_id probe.

Non-blocking: foundScanRuns caches the thBuildId → scanRunId mapping for the life of the process. That is right only if the scan list's scanUrl and lastScanDetails.scanRunId always describe the same run (my open question from last round). If they can disagree while a re-run is starting, a wrong mapping is now cached instead of being corrected on the next walk.

…d the Automate id probe Brings back the web-build compare, re-run and failed-run tests dropped in 7ee4c3d, updated to this PR's current behaviour, and adds cases for the review fixes: - web-build compare: refusing a build still processing, ordering by build number when a start time is missing, and keeping the passed order with a note otherwise; - re-runs: a Local-saved scan without localIdentifier starts a new scan, a local page the MCP tunnelled gets no re-run hint, a login or tunnel passed is named as not used, the saved pages and name are quoted as untrusted, and the scan limit is the API's number; - a 40-hex Automate or App Automate build id: found, falling through on 403/5xx or an empty build, and naming why Automate could not be asked; - compare refused for a cancelled run; - exports: no task kept while a report is processing, a run read by build id reused for 5 minutes at most, and a link's run found once not looked up again; - expert questions: the note matches what was cut, CJK sentence ends, no split character. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sarve-shreyas

Re-reviewed 85f3f09..7397791 (test-only commit). npm run build passes locally: 147 files, 2,205 tests (+102).

Resolved (was blocking): the new behaviour now has tests. Six files cover:

Web-build compare, in accessibilityWebBuildCompare.test.ts: ordering including the build-number fallback and unknown order, refusing processing/failed builds, the self-matching fallback, list caps and error pages.
Re-runs, in accessibilityRescan.test.ts: same vs other page, settings mismatch, login/tunnel not used, Local without identifier starting a new scan, no hint for MCP-tunnelled pages.
The Automate hashed_id probe and its fall-through, in accessibilityAutomateBuildId.test.ts.
Failed and cancelled runs, including compares.
Expert-question shortening (CJK, surrogate pairs).
The build-id cache, the stale task id, and the 5-minute scanBuild TTL.

I spot-checked the key cases: they assert behaviour (e.g. rerunScan not called, startScan called once, newReport/baseReport order), not only output strings.

No blocking findings left from me. One non-blocking open question from last round remains: foundScanRuns is safe only if the scan list's scanUrl and lastScanDetails.scanRunId always describe the same run.

sarve-shreyas

Approved at 7397791. All findings are fixed and the new behaviour is covered by tests. Build passes locally, 2,205 tests. One non-blocking question is still open: the foundScanRuns cache assumes the scan list's scanUrl and lastScanDetails.scanRunId always describe the same run.

Aaryan430 and others added 8 commits November 5, 2026 13:36

Aaryan430 changed the base branch from fix/a11y-scan-stability to main November 5, 2026 08:45

Aaryan430 mentioned this pull request Nov 5, 2026

fix(accessibility): don't hold calls on finalizing reports; say when a run failed or the plan refuses #552

Closed

Aaryan430 and others added 3 commits November 5, 2026 14:17

docs(accessibility): fetchAccessibilityIssues no longer waits on a pr…

4f9e9e3

…ocessing report Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Aaryan430 changed the base branch from main to fix/a11y-scan-stability November 5, 2026 09:07

This was referenced Nov 5, 2026

fix(accessibility): read any report link directly: scanner links by their run's build id, Workflow Analyzer links #527

Merged

fix(accessibility): scan fixes from the 2 Nov stability report #541

Merged

Aaryan430 and others added 11 commits November 5, 2026 15:50

sarve-shreyas reviewed Nov 6, 2026

View reviewed changes

Aaryan430 and others added 9 commits November 6, 2026 12:21

Merge branch 'stack/541' into stack/553

dbcaab4

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix(accessibility): name the plan's scan limit the API gave, not a ha…

27744bf

…rd-coded 50 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

style: prettier

374c655

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix(accessibility): put the saved scan's name after the settings list…

85f3f09

… in a re-run refusal The quoted name sat between "these differ:" and the list, so it read as one of the differences. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sarve-shreyas reviewed Nov 6, 2026

View reviewed changes

sarve-shreyas approved these changes Nov 6, 2026

View reviewed changes

Dave3130 merged commit 9a7c595 into fix/a11y-scan-stability Nov 6, 2026
3 checks passed

Conversation

Aaryan430 commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A failed run is no longer read as a clean page

Scan failure reasons are quoted as untrusted data

compareWith on a report that has no compare

Compare Automated Tests builds

Re-run an earlier scan instead of creating a new one

Plain answers when a local URL can't be scanned

Find older scans from a report link

No 5-minute wait on a finalizing report; account refusals as plain answers; expert questions within 500 characters

Friction found in end-to-end testing of this PR, fixed

Review fixes (6 Nov)

Testing

Uh oh!

coderabbitai Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

sarve-shreyas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarve-shreyas left a comment

Choose a reason for hiding this comment

Uh oh!

sarve-shreyas left a comment

Choose a reason for hiding this comment

Uh oh!

sarve-shreyas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Aaryan430 commented Nov 5, 2026 •

edited

Loading

coderabbitai Bot commented Nov 5, 2026 •

edited

Loading