diff --git a/src/DevtoolsUtils.ts b/src/DevtoolsUtils.ts index 997cb86a..eb7ad365 100644 --- a/src/DevtoolsUtils.ts +++ b/src/DevtoolsUtils.ts @@ -6,11 +6,17 @@ import { type Issue, + type AggregatedIssue, type IssuesManagerEventTypes, + MarkdownIssueDescription, + Marked, Common, I18n, } from '../node_modules/chrome-devtools-frontend/mcp/mcp.js'; +import {ISSUE_UTILS} from './issue-descriptions.js'; +import {logger} from './logger.js'; + export function extractUrlLikeFromDevToolsTitle( title: string, ): string | undefined { @@ -69,6 +75,48 @@ export class FakeIssuesManager extends Common.ObjectWrapper } } +export function mapIssueToMessageObject(issue: AggregatedIssue) { + const count = issue.getAggregatedIssuesCount(); + const markdownDescription = issue.getDescription(); + const filename = markdownDescription?.file; + if (!markdownDescription) { + logger(`no description found for issue:` + issue.code); + return null; + } + const rawMarkdown = filename + ? ISSUE_UTILS.getIssueDescription(filename) + : null; + if (!rawMarkdown) { + logger(`no markdown ${filename} found for issue:` + issue.code); + return null; + } + let processedMarkdown: string; + let title: string | null; + + try { + processedMarkdown = MarkdownIssueDescription.substitutePlaceholders( + rawMarkdown, + markdownDescription.substitutions, + ); + const markdownAst = Marked.Marked.lexer(processedMarkdown); + title = MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst); + } catch { + logger('error parsing markdown for issue ' + issue.code()); + return null; + } + if (!title) { + logger('cannot read issue title from ' + filename); + return null; + } + return { + type: 'issue', + item: issue, + message: title, + count, + description: processedMarkdown, + }; +} + I18n.DevToolsLocale.DevToolsLocale.instance({ create: true, data: { diff --git a/src/McpContext.ts b/src/McpContext.ts index 22d54c94..7536fe78 100644 --- a/src/McpContext.ts +++ b/src/McpContext.ts @@ -199,8 +199,13 @@ export class McpContext implements Context { this.logger('no cdpBackendNodeId'); return; } + if (this.#textSnapshot === null) + throw new Error( + "The snapshot is not defined, can't resolve backendNodeId: " + + cdpBackendNodeId, + ); // TODO: index by backendNodeId instead. - const queue = [this.#textSnapshot?.root]; + const queue = [this.#textSnapshot.root]; while (queue.length) { const current = queue.pop()!; if (current.backendNodeId === cdpBackendNodeId) { diff --git a/src/McpResponse.ts b/src/McpResponse.ts index 3fb83986..b7de0440 100644 --- a/src/McpResponse.ts +++ b/src/McpResponse.ts @@ -4,12 +4,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { - AggregatedIssue, - Marked, - MarkdownIssueDescription, -} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js'; +import {AggregatedIssue} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js'; +import {mapIssueToMessageObject} from './DevtoolsUtils.js'; import type {ConsoleMessageData} from './formatters/consoleFormatter.js'; import { formatConsoleEventShort, @@ -23,8 +20,6 @@ import { getStatusFromRequest, } from './formatters/networkFormatter.js'; import {formatSnapshotNode} from './formatters/snapshotFormatter.js'; -import {getIssueDescription} from './issue-descriptions.js'; -import {logger} from './logger.js'; import type {McpContext} from './McpContext.js'; import type { ConsoleMessage, @@ -256,6 +251,16 @@ export class McpResponse implements Response { }), ), }; + } else if (message instanceof AggregatedIssue) { + const mappedIssueMessage = mapIssueToMessageObject(message); + if (!mappedIssueMessage) + throw new Error( + "Can't prpovide detals for the msgid " + consoleMessageStableId, + ); + consoleData = { + consoleMessageStableId, + ...mappedIssueMessage, + }; } else { consoleData = { consoleMessageStableId, @@ -309,29 +314,11 @@ export class McpResponse implements Response { }; } if (item instanceof AggregatedIssue) { - const count = item.getAggregatedIssuesCount(); - const filename = item.getDescription()?.file; - const rawMarkdown = filename - ? getIssueDescription(filename) - : null; - if (!rawMarkdown) { - logger(`no markdown ${filename} found for issue:` + item.code); - return null; - } - const markdownAst = Marked.Marked.lexer(rawMarkdown); - const title = - MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst); - if (!title) { - logger('cannot read issue title from ' + filename); - return null; - } + const mappedIssueMessage = mapIssueToMessageObject(item); + if (!mappedIssueMessage) return null; return { consoleMessageStableId, - type: 'issue', - item, - message: title, - count, - args: [], + ...mappedIssueMessage, }; } return { diff --git a/src/formatters/consoleFormatter.ts b/src/formatters/consoleFormatter.ts index e4eca9b1..0e4672aa 100644 --- a/src/formatters/consoleFormatter.ts +++ b/src/formatters/consoleFormatter.ts @@ -4,12 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ +import type {AggregatedIssue} from '../../node_modules/chrome-devtools-frontend/mcp/mcp.js'; + export interface ConsoleMessageData { consoleMessageStableId: number; type?: string; - item?: unknown; + item?: AggregatedIssue; message?: string; count?: number; + description?: string; args?: string[]; } @@ -34,12 +37,12 @@ function getArgs(msg: ConsoleMessageData) { // The verbose format for a console message, including all details. export function formatConsoleEventVerbose(msg: ConsoleMessageData): string { + const aggregatedIssue = msg.item; const result = [ `ID: ${msg.consoleMessageStableId}`, - `Message: ${msg.type}> ${msg.message}`, - formatArgs(msg), + `Message: ${msg.type}> ${aggregatedIssue ? formatIssue(aggregatedIssue, msg.description) : msg.message}`, + aggregatedIssue ? undefined : formatArgs(msg), ].filter(line => !!line); - return result.join('\n'); } @@ -62,3 +65,29 @@ function formatArgs(consoleData: ConsoleMessageData): string { return result.join('\n'); } + +export function formatIssue( + issue: AggregatedIssue, + description?: string, +): string { + const result: string[] = []; + + let processedMarkdown = description?.trim(); + // Remove heading in order not to conflict with the whole console message response markdown + if (processedMarkdown?.startsWith('# ')) { + processedMarkdown = processedMarkdown.substring(2).trimStart(); + } + if (processedMarkdown) result.push(processedMarkdown); + + const links = issue.getDescription()?.links; + if (links && links.length > 0) { + result.push('Learn more:'); + for (const link of links) { + result.push(`[${link.linkTitle}](${link.link})`); + } + } + + if (result.length === 0) + return 'No details provided for the issue ' + issue.code(); + return result.join('\n'); +} diff --git a/src/issue-descriptions.ts b/src/issue-descriptions.ts index 21485e4b..c13e405e 100644 --- a/src/issue-descriptions.ts +++ b/src/issue-descriptions.ts @@ -47,3 +47,8 @@ export async function loadIssueDescriptions(): Promise { export function getIssueDescription(fileName: string): string | null { return issueDescriptions[fileName] ?? null; } + +export const ISSUE_UTILS = { + loadIssueDescriptions, + getIssueDescription, +}; diff --git a/tests/DevtoolsUtils.test.ts b/tests/DevtoolsUtils.test.ts index 40ecf2c3..fd1569d5 100644 --- a/tests/DevtoolsUtils.test.ts +++ b/tests/DevtoolsUtils.test.ts @@ -5,12 +5,17 @@ */ import assert from 'node:assert'; -import {describe, it} from 'node:test'; +import {afterEach, describe, it} from 'node:test'; +import sinon from 'sinon'; + +import {AggregatedIssue} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js'; import { extractUrlLikeFromDevToolsTitle, urlsEqual, + mapIssueToMessageObject, } from '../src/DevtoolsUtils.js'; +import {ISSUE_UTILS} from '../src/issue-descriptions.js'; describe('extractUrlFromDevToolsTitle', () => { it('deals with no trailing /', () => { @@ -70,3 +75,104 @@ describe('urlsEqual', () => { ); }); }); + +describe('mapIssueToMessageObject', () => { + const mockDescription = { + file: 'mock-issue.md', + substitutions: new Map([['PLACEHOLDER_VALUE', 'substitution value']]), + links: [ + {link: 'http://example.com/learnmore', linkTitle: 'Learn more'}, + { + link: 'http://example.com/another-learnmore', + linkTitle: 'Learn more 2', + }, + ], + }; + + afterEach(() => { + sinon.restore(); + }); + + it('maps aggregated issue with substituted description', () => { + const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue); + mockAggregatedIssue.getDescription.returns(mockDescription); + mockAggregatedIssue.getAggregatedIssuesCount.returns(1); + + const getIssueDescriptionStub = sinon.stub( + ISSUE_UTILS, + 'getIssueDescription', + ); + + getIssueDescriptionStub + .withArgs('mock-issue.md') + .returns( + '# Mock Issue Title\n\nThis is a mock issue description with a {PLACEHOLDER_VALUE}.', + ); + + const result = mapIssueToMessageObject(mockAggregatedIssue); + const expected = { + type: 'issue', + item: mockAggregatedIssue, + message: 'Mock Issue Title', + count: 1, + description: + '# Mock Issue Title\n\nThis is a mock issue description with a substitution value.', + }; + assert.deepStrictEqual(result, expected); + }); + + it('returns null for the issue with no description', () => { + const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue); + mockAggregatedIssue.getDescription.returns(null); + + const result = mapIssueToMessageObject(mockAggregatedIssue); + assert.equal(result, null); + }); + + it('returns null if there is no desciption file', () => { + const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue); + mockAggregatedIssue.getDescription.returns(mockDescription); + mockAggregatedIssue.getAggregatedIssuesCount.returns(1); + + const getIssueDescriptionStub = sinon.stub( + ISSUE_UTILS, + 'getIssueDescription', + ); + + getIssueDescriptionStub.withArgs('mock-issue.md').returns(null); + const result = mapIssueToMessageObject(mockAggregatedIssue); + assert.equal(result, null); + }); + + it("returns null if can't parse the title", () => { + const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue); + mockAggregatedIssue.getDescription.returns(mockDescription); + mockAggregatedIssue.getAggregatedIssuesCount.returns(1); + + const getIssueDescriptionStub = sinon.stub( + ISSUE_UTILS, + 'getIssueDescription', + ); + + getIssueDescriptionStub + .withArgs('mock-issue.md') + .returns('No title test {PLACEHOLDER_VALUE}'); + assert.deepStrictEqual(mapIssueToMessageObject(mockAggregatedIssue), null); + }); + + it('returns null if devtools utill function throws an error', () => { + const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue); + mockAggregatedIssue.getDescription.returns(mockDescription); + mockAggregatedIssue.getAggregatedIssuesCount.returns(1); + + const getIssueDescriptionStub = sinon.stub( + ISSUE_UTILS, + 'getIssueDescription', + ); + // An error will be thrown if placeholder doesn't start from PLACEHOLDER_ + getIssueDescriptionStub + .withArgs('mock-issue.md') + .returns('No title test {WRONG_PLACEHOLDER}'); + assert.deepStrictEqual(mapIssueToMessageObject(mockAggregatedIssue), null); + }); +}); diff --git a/tests/McpResponse.test.ts b/tests/McpResponse.test.ts index 2c9516bc..b045454d 100644 --- a/tests/McpResponse.test.ts +++ b/tests/McpResponse.test.ts @@ -10,6 +10,10 @@ import {tmpdir} from 'node:os'; import {join} from 'node:path'; import {describe, it} from 'node:test'; +import sinon from 'sinon'; + +import {AggregatedIssue} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js'; + import { getMockRequest, getMockResponse, @@ -295,6 +299,46 @@ describe('McpResponse', () => { t.assert.snapshot?.(result[0].text); }); }); + + it("doesn't list the issue message if mapping returns null", async () => { + await withBrowser(async (response, context) => { + const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue); + const mockDescription = { + file: 'not-existing-description-file.md', + links: [], + }; + mockAggregatedIssue.getDescription.returns(mockDescription); + response.setIncludeConsoleData(true); + context.getConsoleData = () => { + return [mockAggregatedIssue]; + }; + + const result = await response.handle('test', context); + const text = (result[0].text as string).toString(); + assert.ok(text.includes('')); + }); + }); + + it('throws error if mapping returns null on get issue details', async () => { + await withBrowser(async (response, context) => { + const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue); + const mockDescription = { + file: 'not-existing-description-file.md', + links: [], + }; + mockAggregatedIssue.getDescription.returns(mockDescription); + response.attachConsoleMessage(1); + context.getConsoleMessageById = () => { + return mockAggregatedIssue; + }; + + try { + await response.handle('test', context); + } catch (e) { + assert.ok(e.message.includes("Can't prpovide detals for the msgid 1")); + } + }); + }); }); describe('McpResponse network request filtering', () => { diff --git a/tests/formatters/consoleFormatter.test.js.snapshot b/tests/formatters/consoleFormatter.test.js.snapshot index 7e6d5d65..d6803572 100644 --- a/tests/formatters/consoleFormatter.test.js.snapshot +++ b/tests/formatters/consoleFormatter.test.js.snapshot @@ -34,3 +34,13 @@ Message: log> Processing file: ### Arguments Arg #0: file.txt `; + +exports[`consoleFormatter > formats a console.log message with issue type 1`] = ` +ID: 5 +Message: issue> Mock Issue Title + +This is a mock issue description +Learn more: +[Learn more](http://example.com/learnmore) +[Learn more 2](http://example.com/another-learnmore) +`; diff --git a/tests/formatters/consoleFormatter.test.ts b/tests/formatters/consoleFormatter.test.ts index d5e71ba4..eabe2a44 100644 --- a/tests/formatters/consoleFormatter.test.ts +++ b/tests/formatters/consoleFormatter.test.ts @@ -6,6 +6,9 @@ import {describe, it} from 'node:test'; +import sinon from 'sinon'; + +import {AggregatedIssue} from '../../node_modules/chrome-devtools-frontend/mcp/mcp.js'; import type {ConsoleMessageData} from '../../src/formatters/consoleFormatter.js'; import { formatConsoleEventShort, @@ -92,4 +95,31 @@ describe('consoleFormatter', () => { t.assert.snapshot?.(result); }); }); + + it('formats a console.log message with issue type', t => { + const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue); + const mockDescription = { + file: 'mock.md', + links: [ + {link: 'http://example.com/learnmore', linkTitle: 'Learn more'}, + { + link: 'http://example.com/another-learnmore', + linkTitle: 'Learn more 2', + }, + ], + }; + mockAggregatedIssue.getDescription.returns(mockDescription); + const mockDescriptionFileContent = + '# Mock Issue Title\n\nThis is a mock issue description'; + + const message: ConsoleMessageData = { + consoleMessageStableId: 5, + type: 'issue', + description: mockDescriptionFileContent, + item: mockAggregatedIssue, + }; + + const result = formatConsoleEventVerbose(message); + t.assert.snapshot?.(result); + }); }); diff --git a/tests/tools/console.test.ts b/tests/tools/console.test.ts index c7f6a97d..0724a503 100644 --- a/tests/tools/console.test.ts +++ b/tests/tools/console.test.ts @@ -9,6 +9,7 @@ import {afterEach, before, beforeEach, describe, it} from 'node:test'; import {setIssuesEnabled} from '../../src/features.js'; import {loadIssueDescriptions} from '../../src/issue-descriptions.js'; +import {McpResponse} from '../../src/McpResponse.js'; import { getConsoleMessage, listConsoleMessages, @@ -150,5 +151,45 @@ describe('console', () => { ); }); }); + + describe('issues type', () => { + beforeEach(() => { + setIssuesEnabled(true); + }); + afterEach(() => { + setIssuesEnabled(false); + }); + + it('gets issue details', async () => { + await withBrowser(async (response, context) => { + const page = await context.newPage(); + const issuePromise = new Promise(resolve => { + page.once('issue', () => { + resolve(); + }); + }); + await page.setContent(''); + await issuePromise; + await listConsoleMessages.handler({params: {}}, response, context); + const response2 = new McpResponse(); + await getConsoleMessage.handler( + {params: {msgid: 1}}, + response2, + context, + ); + const formattedResponse = await response2.handle('test', context); + const textContent = formattedResponse[0] as {text: string}; + const learnMoreLinks = + '[HTML attribute: autocomplete](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete#values)'; + const detailsDescription = + "A form field has an `id` or `name` attribute that the browser's autofill recognizes. However, it doesn't have an `autocomplete` attribute assigned. This might prevent the browser from correctly autofilling the form.\n\nTo fix this issue, provide an `autocomplete` attribute."; + const title = + "Message: issue> An element doesn't have an autocomplete attribute"; + assert.ok(textContent.text.includes(title)); + assert.ok(textContent.text.includes(detailsDescription)); + assert.ok(textContent.text.includes(learnMoreLinks)); + }); + }); + }); }); });