-
Notifications
You must be signed in to change notification settings - Fork 749
feat(chat): Add header for execute shell command #6948
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 |
|---|---|---|
|
|
@@ -50,7 +50,12 @@ | |
| import { Change } from 'diff' | ||
| import { FsWriteParams } from '../../../tools/fsWrite' | ||
|
|
||
| export type StaticTextResponseType = 'quick-action-help' | 'onboarding-help' | 'transform' | 'help' | ||
| export type StaticTextResponseType = | ||
| | 'quick-action-help' | ||
| | 'onboarding-help' | ||
| | 'transform' | ||
| | 'help' | ||
| | 'reject-shell-command' | ||
|
|
||
| export type MessengerResponseType = { | ||
| $metadata: { requestId?: string; httpStatusCode?: number } | ||
|
|
@@ -269,12 +274,20 @@ | |
| } | ||
|
|
||
| if (!validation.requiresAcceptance) { | ||
| // Need separate id for read tool and safe bash command execution as 'confirm-tool-use' id is required to change button status from `Confirm` to `Confirmed` state in cwChatConnector.ts which will impact generic tool execution. | ||
| this.dispatcher.sendCustomFormActionMessage( | ||
| new CustomFormActionMessage(tabID, { | ||
| id: 'generic-tool-execution', | ||
| }) | ||
| ) | ||
| // Need separate id for read tool and safe bash command execution as 'run-shell-command' id is required to state in cwChatConnector.ts which will impact generic tool execution. | ||
| if (tool.type === ToolType.ExecuteBash) { | ||
|
Contributor
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.
Contributor
Author
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. can do it in next pr. |
||
| this.dispatcher.sendCustomFormActionMessage( | ||
| new CustomFormActionMessage(tabID, { | ||
| id: 'run-shell-command', | ||
| }) | ||
| ) | ||
| } else { | ||
| this.dispatcher.sendCustomFormActionMessage( | ||
| new CustomFormActionMessage(tabID, { | ||
| id: 'generic-tool-execution', | ||
| }) | ||
| ) | ||
| } | ||
| } | ||
| } else { | ||
| // TODO: Handle the error | ||
|
|
@@ -500,12 +513,28 @@ | |
| ) { | ||
| const buttons: ChatItemButton[] = [] | ||
| let fileList: ChatItemContent['fileList'] = undefined | ||
| if (validation.requiresAcceptance && toolUse?.name === ToolType.ExecuteBash) { | ||
| buttons.push({ | ||
| id: 'confirm-tool-use', | ||
| text: 'Confirm', | ||
| status: 'info', | ||
| }) | ||
| let shellCommandHeader = undefined | ||
| if (toolUse?.name === ToolType.ExecuteBash && message.startsWith('```shell')) { | ||
| if (validation.requiresAcceptance) { | ||
| buttons.push({ | ||
| id: 'run-shell-command', | ||
| text: 'Run', | ||
| status: 'main', | ||
| icon: 'play' as MynahIconsType, | ||
| }) | ||
| buttons.push({ | ||
| id: 'reject-shell-command', | ||
| text: 'Reject', | ||
| status: 'clear', | ||
| icon: 'cancel' as MynahIconsType, | ||
| }) | ||
| } | ||
|
|
||
| shellCommandHeader = { | ||
| icon: 'code-block' as MynahIconsType, | ||
| body: 'shell', | ||
| buttons: buttons, | ||
| } | ||
|
|
||
| if (validation.warning) { | ||
| message = validation.warning + message | ||
|
|
@@ -564,16 +593,23 @@ | |
| codeBlockLanguage: undefined, | ||
| contextList: undefined, | ||
| canBeVoted: false, | ||
| buttons: toolUse?.name === ToolType.FsWrite ? undefined : buttons, | ||
| fullWidth: toolUse?.name === ToolType.FsWrite, | ||
| padding: !(toolUse?.name === ToolType.FsWrite), | ||
| buttons: | ||
|
Contributor
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. We can keep
Contributor
Author
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. you mean the current check is right, but we can simplify it, right?
Contributor
Author
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. if yes, then i can do it in next pr. |
||
| toolUse?.name === ToolType.FsWrite || toolUse?.name === ToolType.ExecuteBash | ||
| ? undefined | ||
| : buttons, | ||
| fullWidth: toolUse?.name === ToolType.FsWrite || toolUse?.name === ToolType.ExecuteBash, | ||
| padding: !(toolUse?.name === ToolType.FsWrite || toolUse?.name === ToolType.ExecuteBash), | ||
| header: | ||
| toolUse?.name === ToolType.FsWrite | ||
| ? { icon: 'code-block' as MynahIconsType, buttons: buttons, fileList: fileList } | ||
| : undefined, | ||
| : toolUse?.name === ToolType.ExecuteBash | ||
| ? shellCommandHeader | ||
| : undefined, | ||
| codeBlockActions: | ||
| // eslint-disable-next-line unicorn/no-null, prettier/prettier | ||
| toolUse?.name === ToolType.FsWrite ? { 'insert-to-cursor': null, copy: null } : undefined, | ||
| toolUse?.name === ToolType.FsWrite || toolUse?.name === ToolType.ExecuteBash | ||
| ? { 'insert-to-cursor': null, copy: null } | ||
|
Check failure on line 611 in packages/core/src/codewhispererChat/controllers/chat/messenger/messenger.ts
|
||
| : undefined, | ||
| }, | ||
| tabID | ||
| ) | ||
|
|
@@ -625,6 +661,10 @@ | |
| ] | ||
| followUpsHeader = 'Try Examples:' | ||
| break | ||
| case 'reject-shell-command': | ||
| // need to update the string later | ||
| message = 'The shell command execution rejected. Abort.' | ||
| break | ||
| } | ||
|
|
||
| this.dispatcher.sendChatMessage( | ||
|
|
||
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.
I think we don't need this icon and body. Please follow same approach as
accept-code-diff.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.
why we don't need body and icon? or i missed some UI?
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.
Use the tyrone StoreItem to get the body and icon