-
Notifications
You must be signed in to change notification settings - Fork 73
feat(amazonq): initial UI for execute bash chat message #1041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
* Will be deleted or merged. | ||
*/ | ||
|
||
import * as path from 'path' | ||
Check warning on line 6 in server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts
|
||
import { | ||
ChatTriggerType, | ||
GenerateAssistantResponseCommandInput, | ||
|
@@ -16,6 +16,7 @@ | |
ToolUse, | ||
} from '@amzn/codewhisperer-streaming' | ||
import { | ||
Button, | ||
ButtonClickParams, | ||
ButtonClickResult, | ||
ChatMessage, | ||
|
@@ -97,7 +98,9 @@ | |
import { workspaceUtils } from '@aws/lsp-core' | ||
import { FsReadParams } from './tools/fsRead' | ||
import { ListDirectoryParams } from './tools/listDirectory' | ||
import { FsWriteParams, getDiffChanges } from './tools/fsWrite' | ||
import { FsWrite, FsWriteParams, getDiffChanges } from './tools/fsWrite' | ||
import { ExecuteBash, ExecuteBashOutput, ExecuteBashParams } from './tools/executeBash' | ||
import { InvokeOutput } from './tools/toolShared' | ||
|
||
type ChatHandlers = Omit< | ||
LspHandlers<Chat>, | ||
|
@@ -147,10 +150,28 @@ | |
} | ||
|
||
async onButtonClick(params: ButtonClickParams): Promise<ButtonClickResult> { | ||
this.#log(`onButtonClick event with params: ${JSON.stringify(params)}`) | ||
return { | ||
success: false, | ||
failureReason: 'not implemented', | ||
if (params.buttonId === 'run-shell-command' || params.buttonId === 'reject-shell-command') { | ||
const session = this.#chatSessionManagementService.getSession(params.tabId) | ||
if (!session.data) { | ||
return { success: false, failureReason: `could not find chat session for tab: ${params.tabId} ` } | ||
} | ||
const handler = session.data.getDeferredToolExecution(params.messageId) | ||
if (!handler?.reject || !handler.resolve) { | ||
return { | ||
success: false, | ||
failureReason: `could not find deferred tool execution for message: ${params.messageId} `, | ||
} | ||
} | ||
params.buttonId === 'reject-shell-command' ? handler.reject() : handler.resolve() | ||
return { | ||
success: true, | ||
} | ||
} else { | ||
this.#log(`onButtonClick event with params: ${JSON.stringify(params)}`) | ||
return { | ||
success: false, | ||
failureReason: 'not implemented', | ||
} | ||
} | ||
} | ||
|
||
|
@@ -361,7 +382,7 @@ | |
const currentMessage = currentRequestInput.conversationState?.currentMessage | ||
|
||
// Process tool uses and update the request input for the next iteration | ||
const toolResults = await this.#processToolUses(pendingToolUses, chatResultStream) | ||
const toolResults = await this.#processToolUses(pendingToolUses, chatResultStream, session) | ||
currentRequestInput = this.#updateRequestInputWithToolResults(currentRequestInput, toolResults) | ||
|
||
if (!currentRequestInput.conversationState!.history) { | ||
|
@@ -414,13 +435,15 @@ | |
*/ | ||
async #processToolUses( | ||
toolUses: Array<ToolUse & { stop: boolean }>, | ||
chatResultStream: AgenticChatResultStream | ||
chatResultStream: AgenticChatResultStream, | ||
session: ChatSessionService | ||
): Promise<ToolResult[]> { | ||
const results: ToolResult[] = [] | ||
|
||
for (const toolUse of toolUses) { | ||
if (!toolUse.name || !toolUse.toolUseId) continue | ||
this.#triggerContext.getToolUseLookup().set(toolUse.toolUseId, toolUse) | ||
let needsConfirmation | ||
|
||
try { | ||
switch (toolUse.name) { | ||
|
@@ -439,6 +462,15 @@ | |
.set(toolUse.toolUseId, { ...toolUse, oldContent: document?.getText() }) | ||
break | ||
case 'executeBash': | ||
const bashTool = new ExecuteBash(this.#features) | ||
const { requiresAcceptance, warning } = await bashTool.requiresAcceptance( | ||
toolUse.input as unknown as ExecuteBashParams | ||
) | ||
if (requiresAcceptance) { | ||
needsConfirmation = true | ||
const confirmationResult = this.#processExecuteBashConfirmation(toolUse, warning) | ||
await chatResultStream.writeResultBlock(confirmationResult) | ||
} | ||
break | ||
default: | ||
await chatResultStream.writeResultBlock({ | ||
|
@@ -449,6 +481,18 @@ | |
break | ||
} | ||
|
||
if (needsConfirmation) { | ||
const deferred = this.#createDeferred() | ||
session.setDeferredToolExecution(toolUse.toolUseId, deferred.resolve, deferred.reject) | ||
|
||
// the below line was commented out for now because | ||
// the partial result block from above is not streamed to chat window yet at this point | ||
// so the buttons are not in the window for the promise to be rejected/resolved | ||
// this can to be brought back once intermediate messages are shown | ||
|
||
// await deferred.promise | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the client-side changes should be merged now. What other changes do we need before this can be enabled? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this also needs improvement for intermediate chat result to show up |
||
} | ||
|
||
const result = await this.#features.agent.runTool(toolUse.name, toolUse.input) | ||
let toolResultContent: ToolResultContentBlock | ||
|
||
|
@@ -475,6 +519,9 @@ | |
const chatResult = await this.#getFsWriteChatResult(toolUse) | ||
await chatResultStream.writeResultBlock(chatResult) | ||
break | ||
case 'executeBash': | ||
const bashToolResult = this.#getBashExecutionChatResult(toolUse, result) | ||
await chatResultStream.writeResultBlock(bashToolResult) | ||
default: | ||
await chatResultStream.writeResultBlock({ | ||
type: 'tool', | ||
|
@@ -500,6 +547,45 @@ | |
return results | ||
} | ||
|
||
#getBashExecutionChatResult(toolUse: ToolUse, result: InvokeOutput): ChatResult { | ||
const outputString = result.output.success | ||
? (result.output.content as ExecuteBashOutput).stdout | ||
: (result.output.content as ExecuteBashOutput).stderr | ||
return { | ||
type: 'tool', | ||
messageId: toolUse.toolUseId, | ||
body: outputString, | ||
} | ||
} | ||
|
||
#processExecuteBashConfirmation(toolUse: ToolUse, warning?: string): ChatResult { | ||
const buttons: Button[] = [ | ||
{ | ||
id: 'reject-shell-command', | ||
text: 'Reject', | ||
icon: 'cancel', | ||
}, | ||
{ | ||
id: 'run-shell-command', | ||
text: 'Run', | ||
icon: 'play', | ||
}, | ||
] | ||
const header = { | ||
body: 'shell', | ||
buttons, | ||
} | ||
|
||
const commandString = (toolUse.input as unknown as ExecuteBashParams).command | ||
const body = '```shell\n' + commandString + '\n```' | ||
return { | ||
type: 'tool', | ||
messageId: toolUse.toolUseId, | ||
header, | ||
body: warning ? warning + body : body, | ||
} | ||
} | ||
|
||
async #getFsWriteChatResult(toolUse: ToolUse): Promise<ChatMessage> { | ||
const input = toolUse.input as unknown as FsWriteParams | ||
const oldContent = this.#triggerContext.getToolUseLookup().get(toolUse.toolUseId!)?.oldContent ?? '' | ||
|
@@ -1135,6 +1221,16 @@ | |
return tools | ||
} | ||
|
||
#createDeferred() { | ||
let resolve | ||
let reject | ||
const promise = new Promise((res, rej) => { | ||
resolve = res | ||
reject = rej | ||
}) | ||
return { promise, resolve, reject } | ||
} | ||
|
||
#log(...messages: string[]) { | ||
this.#features.logging.log(messages.join(' ')) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want this to be conditional on the tool type when we move towards granular permissions