Skip to content

Commit 18ca133

Browse files
authored
When a task with problem matchers becomes idle, return those instead of using the LLM to evaluate the output (microsoft#261539)
1 parent 533e889 commit 18ca133

File tree

8 files changed

+161
-36
lines changed

8 files changed

+161
-36
lines changed

src/vs/workbench/contrib/tasks/common/taskService.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { Action } from '../../../../base/common/actions.js';
88
import { Event } from '../../../../base/common/event.js';
99
import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js';
1010
import { IDisposable } from '../../../../base/common/lifecycle.js';
11-
1211
import { IWorkspaceFolder, IWorkspace } from '../../../../platform/workspace/common/workspace.js';
1312
import { Task, ContributedTask, CustomTask, ITaskSet, TaskSorter, ITaskEvent, ITaskIdentifier, ConfiguringTask, TaskRunSource } from './tasks.js';
1413
import { ITaskSummary, ITaskTerminateResponse, ITaskSystemInfo } from './taskSystem.js';

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/bufferOutputPolling.ts

Lines changed: 71 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import { IChatService } from '../../../chat/common/chatService.js';
1414
import { ChatMessageRole, ILanguageModelsService } from '../../../chat/common/languageModels.js';
1515
import { IToolInvocationContext } from '../../../chat/common/languageModelToolsService.js';
1616
import type { Terminal as RawXtermTerminal, IMarker as IXtermMarker } from '@xterm/xterm';
17+
import { Task } from '../../../tasks/common/taskService.js';
18+
import { IMarkerData, IMarkerService } from '../../../../../platform/markers/common/markers.js';
19+
import { ProblemMatcher, ProblemMatcherRegistry } from '../../../tasks/common/problemMatcher.js';
1720

1821
export const enum PollingConsts {
1922
MinNoDataEvents = 2, // Minimum number of no data checks before considering the terminal idle
@@ -34,7 +37,8 @@ export async function racePollingOrPrompt(
3437
originalResult: { terminalExecutionIdleBeforeTimeout: boolean; output: string; pollDurationMs?: number; modelOutputEvalResponse?: string },
3538
token: CancellationToken,
3639
languageModelsService: ILanguageModelsService,
37-
execution: { getOutput: () => string; isActive?: () => Promise<boolean> }
40+
markerService: IMarkerService,
41+
execution: { getOutput: () => string; isActive?: () => Promise<boolean>; task?: Task; beginsPattern?: string; endsPattern?: string; dependencyTasks?: Task[] }
3842
): Promise<{ terminalExecutionIdleBeforeTimeout: boolean; output: string; pollDurationMs?: number; modelOutputEvalResponse?: string }> {
3943
const pollPromise = pollFn();
4044
const { promise: promptPromise, part } = promptFn();
@@ -63,7 +67,7 @@ export async function racePollingOrPrompt(
6367
const promptResult = raceResult.result as boolean;
6468
if (promptResult) {
6569
// User accepted, poll again (extended)
66-
return await pollForOutputAndIdle(execution, true, token, languageModelsService);
70+
return await pollForOutputAndIdle(execution, true, token, languageModelsService, markerService);
6771
} else {
6872
return originalResult; // User rejected, return the original result
6973
}
@@ -95,10 +99,12 @@ export function getOutput(terminal?: Pick<RawXtermTerminal, 'buffer'>, startMark
9599
}
96100

97101
export async function pollForOutputAndIdle(
98-
execution: { getOutput: () => string; isActive?: () => Promise<boolean> },
102+
execution: { getOutput: () => string; isActive?: () => Promise<boolean>; task?: Pick<Task, 'configurationProperties'>; dependencyTasks?: Task[] },
99103
extendedPolling: boolean,
100104
token: CancellationToken,
101-
languageModelsService: ILanguageModelsService,
105+
languageModelsService: Pick<ILanguageModelsService, 'selectLanguageModels' | 'sendChatRequest'>,
106+
markerService: Pick<IMarkerService, 'read'>,
107+
knownMatchers?: ProblemMatcher[]
102108
): Promise<{ terminalExecutionIdleBeforeTimeout: boolean; output: string; pollDurationMs?: number; modelOutputEvalResponse?: string }> {
103109
const maxWaitMs = extendedPolling ? PollingConsts.ExtendedPollingMaxDuration : PollingConsts.FirstPollingMaxDuration;
104110
const maxInterval = PollingConsts.MaxPollingIntervalDuration;
@@ -149,10 +155,32 @@ export async function pollForOutputAndIdle(
149155
lastBufferLength = currentBufferLength;
150156
continue;
151157
}
152-
terminalExecutionIdleBeforeTimeout = true;
153-
const modelOutputEvalResponse = await assessOutputForErrors(buffer, token, languageModelsService);
154-
return { modelOutputEvalResponse, terminalExecutionIdleBeforeTimeout, output: buffer, pollDurationMs: Date.now() - pollStartTime + (extendedPolling ? PollingConsts.FirstPollingMaxDuration : 0) };
155158
}
159+
terminalExecutionIdleBeforeTimeout = true;
160+
if (execution.task) {
161+
const problems = await getProblemsForTasks(execution.task, markerService, execution.dependencyTasks, knownMatchers);
162+
if (problems) {
163+
// Problem matchers exist for this task
164+
const problemList: string[] = [];
165+
for (const [, problemArray] of problems.entries()) {
166+
if (problemArray.length) {
167+
for (const p of problemArray) {
168+
problemList.push(`${p.severity}: ${p.message}`);
169+
}
170+
}
171+
}
172+
if (problemList.length === 0) {
173+
return { terminalExecutionIdleBeforeTimeout, output: 'The task succeeded with no problems.', pollDurationMs: Date.now() - pollStartTime + (extendedPolling ? PollingConsts.FirstPollingMaxDuration : 0) };
174+
}
175+
return {
176+
terminalExecutionIdleBeforeTimeout,
177+
output: problemList.join('\n'),
178+
pollDurationMs: Date.now() - pollStartTime + (extendedPolling ? PollingConsts.FirstPollingMaxDuration : 0)
179+
};
180+
}
181+
}
182+
const modelOutputEvalResponse = await assessOutputForErrors(buffer, token, languageModelsService);
183+
return { modelOutputEvalResponse, terminalExecutionIdleBeforeTimeout, output: buffer, pollDurationMs: Date.now() - pollStartTime + (extendedPolling ? PollingConsts.FirstPollingMaxDuration : 0) };
156184
}
157185
return { terminalExecutionIdleBeforeTimeout: false, output: buffer, pollDurationMs: Date.now() - pollStartTime + (extendedPolling ? PollingConsts.FirstPollingMaxDuration : 0) };
158186
}
@@ -168,7 +196,7 @@ export function promptForMorePolling(command: string, token: CancellationToken,
168196
let part: ChatElicitationRequestPart | undefined = undefined;
169197
const promise = new Promise<boolean>(resolve => {
170198
const thePart = part = new ChatElicitationRequestPart(
171-
new MarkdownString(localize('poll.terminal.waiting', "Continue waiting?")),
199+
new MarkdownString(localize('poll.terminal.waiting', "Continue waiting for \`{0}\`?", command)),
172200
new MarkdownString(localize('poll.terminal.polling', "This will continue to poll for output to determine when the terminal becomes idle for up to 2 minutes.")),
173201
'',
174202
localize('poll.terminal.accept', 'Yes'),
@@ -192,7 +220,7 @@ export function promptForMorePolling(command: string, token: CancellationToken,
192220
return { promise: Promise.resolve(false) };
193221
}
194222

195-
export async function assessOutputForErrors(buffer: string, token: CancellationToken, languageModelsService: ILanguageModelsService): Promise<string> {
223+
export async function assessOutputForErrors(buffer: string, token: CancellationToken, languageModelsService: Pick<ILanguageModelsService, 'selectLanguageModels' | 'sendChatRequest'>): Promise<string> {
196224
const models = await languageModelsService.selectLanguageModels({ vendor: 'copilot', family: 'gpt-4o-mini' });
197225
if (!models.length) {
198226
return 'No models available';
@@ -223,3 +251,37 @@ export async function assessOutputForErrors(buffer: string, token: CancellationT
223251
return 'Error occurred ' + err;
224252
}
225253
}
254+
255+
export function getProblemsForTasks(task: Pick<Task, 'configurationProperties'>, markerService: Pick<IMarkerService, 'read'>, dependencyTasks?: Task[], knownMatchers?: ProblemMatcher[]): Map<string, IMarkerData[]> | undefined {
256+
const problemsMap = new Map<string, IMarkerData[]>();
257+
let hadDefinedMatcher = false;
258+
259+
const collectProblems = (t: Pick<Task, 'configurationProperties'>) => {
260+
const matchers = Array.isArray(t.configurationProperties.problemMatchers)
261+
? t.configurationProperties.problemMatchers
262+
: (t.configurationProperties.problemMatchers ? [t.configurationProperties.problemMatchers] : []);
263+
for (const matcherRef of matchers) {
264+
const matcher = typeof matcherRef === 'string'
265+
? ProblemMatcherRegistry.get(matcherRef) ?? knownMatchers?.find(m => m.owner === matcherRef)
266+
: matcherRef;
267+
if (matcher?.owner) {
268+
const markers = markerService.read({ owner: matcher.owner });
269+
hadDefinedMatcher = true;
270+
if (markers.length) {
271+
problemsMap.set(matcher.owner, markers);
272+
}
273+
}
274+
}
275+
};
276+
277+
collectProblems(task);
278+
279+
if (problemsMap.size === 0 && dependencyTasks) {
280+
for (const depTask of dependencyTasks) {
281+
collectProblems(depTask);
282+
}
283+
}
284+
285+
return hadDefinedMatcher ? problemsMap : undefined;
286+
}
287+

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/outputMonitor.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { Disposable } from '../../../../../base/common/lifecycle.js';
99
import { ILanguageModelsService } from '../../../chat/common/languageModels.js';
1010
import { IChatService } from '../../../chat/common/chatService.js';
1111
import { racePollingOrPrompt, promptForMorePolling, pollForOutputAndIdle } from './bufferOutputPolling.js';
12+
import { IMarkerService } from '../../../../../platform/markers/common/markers.js';
13+
import { Task } from '../../../tasks/common/taskService.js';
1214

1315
export interface IOutputMonitor extends Disposable {
1416
readonly isIdle: boolean;
@@ -40,8 +42,9 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
4042
}
4143

4244
constructor(
43-
private readonly _execution: { getOutput: () => string; isActive?: () => Promise<boolean> },
44-
@ILanguageModelsService private readonly _languageModelsService: ILanguageModelsService
45+
private readonly _execution: { getOutput: () => string; isActive?: () => Promise<boolean>; task?: Task; beginsPattern?: string; endsPattern?: string; dependencyTasks?: Task[] },
46+
@ILanguageModelsService private readonly _languageModelsService: ILanguageModelsService,
47+
@IMarkerService private readonly _markerService: IMarkerService
4548
) {
4649
super();
4750
}
@@ -52,15 +55,16 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
5255
invocationContext: any,
5356
token: CancellationToken
5457
): Promise<{ terminalExecutionIdleBeforeTimeout: boolean; output: string; pollDurationMs?: number; modelOutputEvalResponse?: string }> {
55-
let result = await pollForOutputAndIdle(this._execution, false, token, this._languageModelsService);
58+
let result = await pollForOutputAndIdle(this._execution, false, token, this._languageModelsService, this._markerService);
5659

5760
if (!result.terminalExecutionIdleBeforeTimeout) {
5861
result = await racePollingOrPrompt(
59-
() => pollForOutputAndIdle(this._execution, true, token, this._languageModelsService),
62+
() => pollForOutputAndIdle(this._execution, true, token, this._languageModelsService, this._markerService),
6063
() => promptForMorePolling(command, token, invocationContext, chatService),
6164
result,
6265
token,
6366
this._languageModelsService,
67+
this._markerService,
6468
this._execution
6569
);
6670
}

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/taskHelpers.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { IStringDictionary } from '../../../../../base/common/collections.js';
88
import { MarkdownString } from '../../../../../base/common/htmlContent.js';
99
import { URI } from '../../../../../base/common/uri.js';
1010
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
11+
import { IMarkerService } from '../../../../../platform/markers/common/markers.js';
1112
import { IChatService } from '../../../chat/common/chatService.js';
1213
import { ILanguageModelsService } from '../../../chat/common/languageModels.js';
1314
import { ToolProgress } from '../../../chat/common/languageModelToolsService.js';
@@ -140,19 +141,20 @@ export async function resolveDependencyTasks(parentTask: Task, workspaceFolder:
140141
* Collects output, polling duration, and idle status for all terminals.
141142
*/
142143
export async function collectTerminalResults(
143-
terminals: ITerminalInstance[], task: Task, languageModelsService: ILanguageModelsService, chatService: IChatService, invocationContext: any, progress: ToolProgress, token: CancellationToken, isActive?: () => Promise<boolean>): Promise<Array<{ name: string; output: string; pollDurationMs: number; idle: boolean }>> {
144+
terminals: ITerminalInstance[], task: Task, languageModelsService: ILanguageModelsService, markerService: IMarkerService, chatService: IChatService, invocationContext: any, progress: ToolProgress, token: CancellationToken, isActive?: () => Promise<boolean>, dependencyTasks?: Task[]): Promise<Array<{ name: string; output: string; pollDurationMs: number; idle: boolean }>> {
144145
const results: Array<{ name: string; output: string; pollDurationMs: number; idle: boolean }> = [];
145146
for (const terminal of terminals) {
146147
progress.report({ message: new MarkdownString(`Checking output for \`${terminal.shellLaunchConfig.name ?? 'unknown'}\``) });
147-
let outputAndIdle = await pollForOutputAndIdle({ getOutput: () => getOutput(terminal.xterm?.raw), isActive }, false, token, languageModelsService);
148+
let outputAndIdle = await pollForOutputAndIdle({ getOutput: () => getOutput(terminal.xterm?.raw), isActive, task, dependencyTasks }, false, token, languageModelsService, markerService);
148149
if (!outputAndIdle.terminalExecutionIdleBeforeTimeout) {
149150
outputAndIdle = await racePollingOrPrompt(
150-
() => pollForOutputAndIdle({ getOutput: () => getOutput(terminal.xterm?.raw), isActive }, true, token, languageModelsService),
151+
() => pollForOutputAndIdle({ getOutput: () => getOutput(terminal.xterm?.raw), isActive, task, dependencyTasks }, true, token, languageModelsService, markerService),
151152
() => promptForMorePolling(task._label, token, invocationContext, chatService),
152153
outputAndIdle,
153154
token,
154155
languageModelsService,
155-
{ getOutput: () => getOutput(terminal.xterm?.raw), isActive }
156+
markerService,
157+
{ getOutput: () => getOutput(terminal.xterm?.raw), isActive, dependencyTasks },
156158
);
157159
}
158160
results.push({

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/task/createAndRunTaskTool.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { URI } from '../../../../../../../base/common/uri.js';
1818
import { IFileService } from '../../../../../../../platform/files/common/files.js';
1919
import { VSBuffer } from '../../../../../../../base/common/buffer.js';
2020
import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js';
21+
import { IMarkerService } from '../../../../../../../platform/markers/common/markers.js';
2122

2223
type CreateAndRunTaskToolClassification = {
2324
taskLabel: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The label of the task.' };
@@ -54,7 +55,8 @@ export class CreateAndRunTaskTool implements IToolImpl {
5455
@ILanguageModelsService private readonly _languageModelsService: ILanguageModelsService,
5556
@IChatService private readonly _chatService: IChatService,
5657
@IFileService private readonly _fileService: IFileService,
57-
@IConfigurationService private readonly _configurationService: IConfigurationService
58+
@IConfigurationService private readonly _configurationService: IConfigurationService,
59+
@IMarkerService private readonly _markerService: IMarkerService
5860
) { }
5961

6062
async invoke(invocation: IToolInvocation, _countTokens: CountTokensCallback, _progress: ToolProgress, token: CancellationToken): Promise<IToolResult> {
@@ -111,8 +113,8 @@ export class CreateAndRunTaskTool implements IToolImpl {
111113
const raceResult = await Promise.race([this._tasksService.run(task), timeout(3000)]);
112114
const result: ITaskSummary | undefined = raceResult && typeof raceResult === 'object' ? raceResult as ITaskSummary : undefined;
113115

114-
const resolvedDependencyTasks = await resolveDependencyTasks(task, args.workspaceFolder, this._configurationService, this._tasksService);
115-
const resources = this._tasksService.getTerminalsForTasks(resolvedDependencyTasks ?? task);
116+
const dependencyTasks = await resolveDependencyTasks(task, args.workspaceFolder, this._configurationService, this._tasksService);
117+
const resources = this._tasksService.getTerminalsForTasks(dependencyTasks ?? task);
116118
const terminals = resources?.map(resource => this._terminalService.instances.find(t => t.resource.path === resource?.path && t.resource.scheme === resource.scheme)).filter(Boolean) as ITerminalInstance[];
117119
if (!terminals || terminals.length === 0) {
118120
return { content: [{ kind: 'text', value: `Task started but no terminal was found for: ${args.task.label}` }], toolResultMessage: new MarkdownString(localize('copilotChat.noTerminal', 'Task started but no terminal was found for: `{0}`', args.task.label)) };
@@ -122,11 +124,13 @@ export class CreateAndRunTaskTool implements IToolImpl {
122124
terminals,
123125
task,
124126
this._languageModelsService,
127+
this._markerService,
125128
this._chatService,
126129
invocation.context!,
127130
_progress,
128131
token,
129-
() => this._isTaskActive(task)
132+
() => this._isTaskActive(task),
133+
dependencyTasks
130134
);
131135
for (const r of terminalResults) {
132136
this._telemetryService.publicLog2?.<CreateAndRunTaskToolEvent, CreateAndRunTaskToolClassification>('copilotChat.runTaskTool.createAndRunTask', {

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/task/getTaskOutputTool.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { MarkdownString } from '../../../../../../../base/common/htmlContent.js'
88
import { Disposable } from '../../../../../../../base/common/lifecycle.js';
99
import { localize } from '../../../../../../../nls.js';
1010
import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js';
11+
import { IMarkerService } from '../../../../../../../platform/markers/common/markers.js';
1112
import { IChatService } from '../../../../../chat/common/chatService.js';
1213
import { ILanguageModelsService } from '../../../../../chat/common/languageModels.js';
1314
import { ToolDataSource, type CountTokensCallback, type IPreparedToolInvocation, type IToolData, type IToolImpl, type IToolInvocation, type IToolInvocationPreparationContext, type IToolResult, type ToolProgress } from '../../../../../chat/common/languageModelToolsService.js';
@@ -52,6 +53,7 @@ export class GetTaskOutputTool extends Disposable implements IToolImpl {
5253
@IConfigurationService private readonly _configurationService: IConfigurationService,
5354
@ILanguageModelsService private readonly _languageModelsService: ILanguageModelsService,
5455
@IChatService private readonly _chatService: IChatService,
56+
@IMarkerService private readonly _markerService: IMarkerService
5557
) {
5658
super();
5759
}
@@ -83,8 +85,8 @@ export class GetTaskOutputTool extends Disposable implements IToolImpl {
8385
return { content: [{ kind: 'text', value: `Task not found: ${args.id}` }], toolResultMessage: new MarkdownString(localize('copilotChat.taskNotFound', 'Task not found: `{0}`', args.id)) };
8486
}
8587

86-
const resolvedDependencyTasks = await resolveDependencyTasks(task, args.workspaceFolder, this._configurationService, this._tasksService);
87-
const resources = this._tasksService.getTerminalsForTasks(resolvedDependencyTasks ?? task);
88+
const dependencyTasks = await resolveDependencyTasks(task, args.workspaceFolder, this._configurationService, this._tasksService);
89+
const resources = this._tasksService.getTerminalsForTasks(dependencyTasks ?? task);
8890
const taskLabel = task._label;
8991
const terminals = resources?.map(resource => this._terminalService.instances.find(t => t.resource.path === resource?.path && t.resource.scheme === resource.scheme)).filter(t => !!t);
9092
if (!terminals || terminals.length === 0) {
@@ -94,10 +96,13 @@ export class GetTaskOutputTool extends Disposable implements IToolImpl {
9496
terminals,
9597
task,
9698
this._languageModelsService,
99+
this._markerService,
97100
this._chatService,
98101
invocation.context!,
99102
_progress,
100-
token
103+
token,
104+
undefined,
105+
dependencyTasks
101106
);
102107
const details = terminalResults.map(r => `Terminal: ${r.name}\nOutput:\n${r.output}`).join('\n\n');
103108
return {

0 commit comments

Comments
 (0)