add basic regex auto-reply support #257691
Conversation
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/bufferOutputPolling.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/bufferOutputPolling.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/bufferOutputPolling.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/bufferOutputPolling.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/bufferOutputPolling.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalTool.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalTool.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalTool.ts
Outdated
Show resolved
Hide resolved
| const userInputKind = await getExpectedUserInputKind(outputAndIdle.output); | ||
| if (userInputKind) { | ||
| const handleResult = await handleYesNoUserPrompt( | ||
| userInputKind, | ||
| invocation.context, | ||
| this._chatService, | ||
| terminal, | ||
| async () => await pollForOutputAndIdle({ getOutput: () => getOutput(terminal), isActive: () => this._isTaskActive(task) }, true, token, this._languageModelsService), | ||
| ); | ||
| if (handleResult.handled) { | ||
| if (handleResult.outputAndIdle) { | ||
| outputAndIdle = handleResult.outputAndIdle; | ||
| } | ||
| } else { | ||
| return { content: [{ kind: 'text', value: `The task is still running and requires user input of ${userInputKind}.` }], toolResultMessage: new MarkdownString(localize('copilotChat.taskRequiresUserInput', 'The task `{0}` is still running and requires user input. {1}', task._label, userInputKind)) }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Sharing this between the 3 tools would be good. This could live in elicitation.ts with the prompt calls too?
There was a problem hiding this comment.
can't put that in elicitation.ts as there are very specific terminal and task things in this function like outputAndIdle, but moved to bufferOutputPolling
There was a problem hiding this comment.
done though in terms of consolidating them
There was a problem hiding this comment.
Pushed 98e45b3 to add the events we want to collect for this. From my testing the userResponse variable doesn't actually work currently.
When I pulled the code I also found it pretty hard to follow. I think it's a little late to push something so large that has the potential to hang the tool due to async logic, especially without any tests.
Since it's an edge case (bg terminals), doesn't handle multiple questions and just does basic regex, I recommend waiting and implementing it in a more robust way next month. It's not worth the risk considering how little this feature actually covers in its current state. What we need is:
- Some output monitoring class that
RunInTerminalToolhands ownership of the execution to and then it has a loop that does all the idle/question polling and handling in a central place (not insideRunInTerminalTool.invoke) - This component must have a good set of unit tests
- It should return a well structured return object as
handleTerminalUserInputPrompt's return result is quite messy right now.
If anything the hint with an a11y alert as I suggested earlier is the safer approach as it's just adding a low risk hint to tell the user the terminal is awaiting input.
| token: CancellationToken, | ||
| languageModelsService: ILanguageModelsService | ||
| ): Promise<{ handled: boolean; userResponse?: 'accepted' | 'rejected'; outputAndIdle: { output: string; terminalExecutionIdleBeforeTimeout: boolean; pollDurationMs?: number; modelOutputEvalResponse?: string }; message?: IToolResult }> { | ||
| const userInputKind = await getExpectedUserInputKind(outputAndIdle.output); |
There was a problem hiding this comment.
| const userInputKind = await getExpectedUserInputKind(outputAndIdle.output); | |
| const userInputKind = getExpectedUserInputKind(outputAndIdle.output); |
fixes https://github.com/microsoft/vscode-copilot/issues/16430
Have a new issue #257726 to track: