Skip to content

Commit a45f897

Browse files
jackfranklindevtools-frontend-scoped@luci-project-accounts.iam.gserviceaccount.com
authored andcommitted
AI: Declare tools unconditionally for imported/non-fresh files
Instead of removing/hiding page-touching tools entirely when dealing with imported Lighthouse reports or non-fresh traces, keep them declared. When AIDA attempts to invoke them, early exit inside their handlers with a concise error message indicating they cannot be used. This also removes the security warning prepended to the prompt preambles, simplifying the initial query. Fixed: 513711812 Change-Id: Ia680e27131d34a6ec97ca9b6bc0e45145af0a27e Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7912246 Auto-Submit: Jack Franklin <jacktfranklin@chromium.org> Commit-Queue: Jack Franklin <jacktfranklin@chromium.org> Reviewed-by: Paul Irish <paulirish@chromium.org>
1 parent 00f3418 commit a45f897

4 files changed

Lines changed: 175 additions & 125 deletions

File tree

front_end/models/ai_assistance/agents/AccessibilityAgent.test.ts

Lines changed: 72 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -417,14 +417,79 @@ describeWithMockConnection('AccessibilityAgent', () => {
417417
sinon.stub(domModel, 'existingDocument').returns(document);
418418

419419
const responses = await Array.fromAsync(agent.run('test', {selected: context}));
420-
const errorResponse = responses.find(response => response.type === AiAssistance.AiAgent.ResponseType.ERROR);
421-
assert.exists(errorResponse);
422-
if (errorResponse && 'error' in errorResponse) {
423-
assert.strictEqual(errorResponse.error, AiAssistance.AiAgent.ErrorType.UNKNOWN);
424-
}
420+
const actionResponse = responses.find(response => response.type === AiAssistance.AiAgent.ResponseType.ACTION);
421+
assert.exists(actionResponse);
422+
assert.strictEqual(actionResponse.output, 'Cannot use this tool on an imported file.');
425423
sinon.assert.notCalled(execJs);
426424
});
427425

426+
it('cannot call runAccessibilityAudits if the report is imported', async () => {
427+
const aidaClient = mockAidaClient([[{
428+
explanation: '',
429+
functionCalls: [{name: 'runAccessibilityAudits', args: {explanation: 'testing'}}],
430+
}]]);
431+
const lighthouseRecording = sinon.stub().resolves(mockReport);
432+
const agent = new AiAssistance.AccessibilityAgent.AccessibilityAgent({
433+
aidaClient,
434+
lighthouseRecording,
435+
});
436+
437+
const importedReport: LHModel.ReporterTypes.ReportJSON = {
438+
...mockReport,
439+
isImported: true,
440+
};
441+
const context = new AiAssistance.AccessibilityAgent.AccessibilityContext(importedReport);
442+
443+
const responses = await Array.fromAsync(agent.run('test', {selected: context}));
444+
const actionResponse = responses.find(response => response.type === AiAssistance.AiAgent.ResponseType.ACTION);
445+
assert.exists(actionResponse);
446+
assert.strictEqual(actionResponse.output, 'Cannot use this tool on an imported file.');
447+
sinon.assert.notCalled(lighthouseRecording);
448+
});
449+
450+
it('cannot call getStyles if the report is imported', async () => {
451+
const aidaClient = mockAidaClient([[{
452+
explanation: '',
453+
functionCalls:
454+
[{name: 'getStyles', args: {path: '1,HTML,1,BODY', styleProperties: ['color'], explanation: 'testing'}}],
455+
}]]);
456+
const agent = new AiAssistance.AccessibilityAgent.AccessibilityAgent({
457+
aidaClient,
458+
});
459+
460+
const importedReport: LHModel.ReporterTypes.ReportJSON = {
461+
...mockReport,
462+
isImported: true,
463+
};
464+
const context = new AiAssistance.AccessibilityAgent.AccessibilityContext(importedReport);
465+
466+
const responses = await Array.fromAsync(agent.run('test', {selected: context}));
467+
const actionResponse = responses.find(response => response.type === AiAssistance.AiAgent.ResponseType.ACTION);
468+
assert.exists(actionResponse);
469+
assert.strictEqual(actionResponse.output, 'Cannot use this tool on an imported file.');
470+
});
471+
472+
it('cannot call getElementAccessibilityDetails if the report is imported', async () => {
473+
const aidaClient = mockAidaClient([[{
474+
explanation: '',
475+
functionCalls: [{name: 'getElementAccessibilityDetails', args: {path: '1,HTML,1,BODY', explanation: 'testing'}}],
476+
}]]);
477+
const agent = new AiAssistance.AccessibilityAgent.AccessibilityAgent({
478+
aidaClient,
479+
});
480+
481+
const importedReport: LHModel.ReporterTypes.ReportJSON = {
482+
...mockReport,
483+
isImported: true,
484+
};
485+
const context = new AiAssistance.AccessibilityAgent.AccessibilityContext(importedReport);
486+
487+
const responses = await Array.fromAsync(agent.run('test', {selected: context}));
488+
const actionResponse = responses.find(response => response.type === AiAssistance.AiAgent.ResponseType.ACTION);
489+
assert.exists(actionResponse);
490+
assert.strictEqual(actionResponse.output, 'Cannot use this tool on an imported file.');
491+
});
492+
428493
it('can still call getLighthouseAudits if the report is imported', async () => {
429494
const aidaClient = mockAidaClient([[{
430495
explanation: '',
@@ -453,33 +518,14 @@ describeWithMockConnection('AccessibilityAgent', () => {
453518
});
454519

455520
describe('enhanceQuery', () => {
456-
it('prepends security warning if the report is imported', async () => {
457-
const agent = new AiAssistance.AccessibilityAgent.AccessibilityAgent({
458-
aidaClient: mockAidaClient([]),
459-
});
460-
461-
const importedReport: LHModel.ReporterTypes.ReportJSON = {
462-
...mockReport,
463-
isImported: true,
464-
};
465-
const context = new AiAssistance.AccessibilityAgent.AccessibilityContext(importedReport);
466-
467-
const enhancedQuery = await agent.enhanceQuery('user query', context);
468-
469-
assert.include(enhancedQuery, 'imported from a file and is static');
470-
assert.include(enhancedQuery, 'user query');
471-
});
472-
473-
it('does not prepend security warning if the report is not imported', async () => {
521+
it('adds the context to the query', async () => {
474522
const agent = new AiAssistance.AccessibilityAgent.AccessibilityAgent({
475523
aidaClient: mockAidaClient([]),
476524
});
477525

478526
const context = new AiAssistance.AccessibilityAgent.AccessibilityContext(mockReport);
479-
480527
const enhancedQuery = await agent.enhanceQuery('user query', context);
481-
482-
assert.notInclude(enhancedQuery, 'imported from a file and is static');
528+
assert.include(enhancedQuery, '# Lighthouse Report');
483529
assert.include(enhancedQuery, 'user query');
484530
});
485531
});

front_end/models/ai_assistance/agents/AccessibilityAgent.ts

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,6 @@ If the user asks a question that requires an investigation of a problem, use thi
8181
- [Suggestion 2]
8282
`;
8383

84-
const SECURITY_WARNING = `**CRITICAL CONSTRAINT**: This Lighthouse report was imported from a file and is static.
85-
You do NOT have access to the inspected page.
86-
Tools like \`executeJavaScript\`, \`getStyles\`, or \`runAccessibilityAudits\` are disabled.
87-
Do NOT attempt to use them or instruct the user that you will use them.
88-
Rely ONLY on the static report data below.`;
89-
9084
export class AccessibilityContext extends ConversationContext<LHModel.ReporterTypes.ReportJSON> {
9185
#lh: LHModel.ReporterTypes.ReportJSON;
9286

@@ -277,11 +271,18 @@ export class AccessibilityAgent extends AiAgent<LHModel.ReporterTypes.ReportJSON
277271
}
278272
});
279273

280-
if (isImported) {
281-
return;
282-
}
283-
284-
this.declareFunction('executeJavaScript', executeJavaScriptFunction(this.#javascriptExecutor));
274+
const executeJsDeclaration = executeJavaScriptFunction(this.#javascriptExecutor);
275+
this.declareFunction('executeJavaScript', {
276+
...executeJsDeclaration,
277+
handler: async (params, options) => {
278+
if (isImported) {
279+
return {
280+
error: 'Cannot use this tool on an imported file.',
281+
};
282+
}
283+
return await executeJsDeclaration.handler(params, options);
284+
},
285+
});
285286

286287
this.declareFunction<{explanation: string}, {audits: string}>('runAccessibilityAudits', {
287288
description:
@@ -308,6 +309,11 @@ export class AccessibilityAgent extends AiAgent<LHModel.ReporterTypes.ReportJSON
308309
},
309310
handler: async params => {
310311
debugLog('Function call: runAccessibilityAudits', params);
312+
if (isImported) {
313+
return {
314+
error: 'Cannot use this tool on an imported file.',
315+
};
316+
}
311317
if (!this.#lighthouseRecording) {
312318
return {error: 'Lighthouse recording is not available.'};
313319
}
@@ -372,6 +378,11 @@ export class AccessibilityAgent extends AiAgent<LHModel.ReporterTypes.ReportJSON
372378
},
373379
handler: async params => {
374380
debugLog('Function call: getStyles', params);
381+
if (isImported) {
382+
return {
383+
error: 'Cannot use this tool on an imported file.',
384+
};
385+
}
375386
const node = await this.#resolvePathToNode(params.path);
376387
if (!node) {
377388
return {error: `Could not find the element with path: ${params.path}`};
@@ -442,6 +453,11 @@ export class AccessibilityAgent extends AiAgent<LHModel.ReporterTypes.ReportJSON
442453
},
443454
handler: async params => {
444455
debugLog('Function call: getElementAccessibilityDetails', params);
456+
if (isImported) {
457+
return {
458+
error: 'Cannot use this tool on an imported file.',
459+
};
460+
}
445461
const node = await this.#resolvePathToNode(params.path);
446462
if (!node) {
447463
return {error: `Could not find the element with path: ${params.path}`};
@@ -519,10 +535,7 @@ export class AccessibilityAgent extends AiAgent<LHModel.ReporterTypes.ReportJSON
519535
if (lhr) {
520536
this.#declareFunctions();
521537
}
522-
let enhancedQuery = lhr ? `${this.#getInitialPayload(lhr)}\n# User request:\n\n` : '';
523-
if (lhr?.getItem().isImported) {
524-
enhancedQuery = `${SECURITY_WARNING}\n\n${enhancedQuery}`;
525-
}
538+
const enhancedQuery = lhr ? `${this.#getInitialPayload(lhr)}\n# User request:\n\n` : '';
526539
return `${enhancedQuery}${query}`;
527540
}
528541

front_end/models/ai_assistance/agents/PerformanceAgent.test.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -663,11 +663,9 @@ code
663663
sinon.stub(Tracing.FreshRecording.Tracker.instance(), 'recordingIsFresh').returns(false);
664664

665665
const responses = await Array.fromAsync(agent.run('test', {selected: context}));
666-
const errorResponse = responses.find(response => response.type === AiAgent.ResponseType.ERROR);
667-
assert.exists(errorResponse);
668-
if (errorResponse && 'error' in errorResponse) {
669-
assert.strictEqual(errorResponse.error, AiAgent.ErrorType.UNKNOWN);
670-
}
666+
const actionResponse = responses.find(response => response.type === AiAgent.ResponseType.ACTION);
667+
assert.exists(actionResponse);
668+
assert.strictEqual(actionResponse.output, 'Cannot use this tool on an imported file.');
671669
});
672670

673671
it('can call getMainThreadTrackSummaryByLabel', async function() {

0 commit comments

Comments
 (0)