Skip to content

Commit 312b04d

Browse files
Randall-JiangRandall-Jiang
authored
fix: show tooltip for warning message and remove the warning text (#1259)
* fix: add tooltip for warning message * fix: fix the string * fix: add enum * fix: remove numbers * fix: the icon for different category icon and reject button
1 parent b0562ca commit 312b04d

File tree

4 files changed

+93
-24
lines changed

4 files changed

+93
-24
lines changed

‎package-lock.json‎

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ import {
121121
} from './constants'
122122
import { URI } from 'vscode-uri'
123123
import { AgenticChatError, customerFacingErrorCodes, unactionableErrorCodes } from './errors'
124+
import { CommandCategory } from './tools/executeBash'
124125

125126
type ChatHandlers = Omit<
126127
LspHandlers<Chat>,
@@ -788,7 +789,7 @@ export class AgenticChatController implements ChatHandlers {
788789
const approvedPaths = session.approvedPaths
789790

790791
// Pass the approved paths to the tool's requiresAcceptance method
791-
const { requiresAcceptance, warning } = await tool.requiresAcceptance(
792+
const { requiresAcceptance, warning, commandCategory } = await tool.requiresAcceptance(
792793
toolUse.input as any,
793794
approvedPaths
794795
)
@@ -798,7 +799,8 @@ export class AgenticChatController implements ChatHandlers {
798799
const confirmationResult = this.#processToolConfirmation(
799800
toolUse,
800801
requiresAcceptance,
801-
warning
802+
warning,
803+
commandCategory
802804
)
803805
cachedButtonBlockId = await chatResultStream.writeResultBlock(confirmationResult)
804806
const isExecuteBash = toolUse.name === 'executeBash'
@@ -1064,6 +1066,22 @@ export class AgenticChatController implements ChatHandlers {
10641066
}
10651067
}
10661068

1069+
/**
1070+
* Get a description for the tooltip based on command category
1071+
* @param commandCategory The category of the command
1072+
* @returns A descriptive message for the tooltip
1073+
*/
1074+
#getCommandCategoryDescription(category: CommandCategory): string | undefined {
1075+
switch (category) {
1076+
case CommandCategory.Mutate:
1077+
return 'This command may modify your code and/or files.'
1078+
case CommandCategory.Destructive:
1079+
return 'This command may cause significant data loss or damage.'
1080+
default:
1081+
return undefined
1082+
}
1083+
}
1084+
10671085
#getWritableStream(chatResultStream: AgenticChatResultStream, toolUse: ToolUse): WritableStream | undefined {
10681086
if (toolUse.name !== 'executeBash') {
10691087
return
@@ -1251,6 +1269,7 @@ export class AgenticChatController implements ChatHandlers {
12511269
toolUse: ToolUse,
12521270
requiresAcceptance: Boolean,
12531271
warning?: string,
1272+
commandCategory?: CommandCategory,
12541273
toolType?: string
12551274
): ChatResult {
12561275
let buttons: Button[] = []
@@ -1280,6 +1299,7 @@ export class AgenticChatController implements ChatHandlers {
12801299
},
12811300
{
12821301
id: 'reject-shell-command',
1302+
status: 'dimmed-clear' as Status,
12831303
text: 'Reject',
12841304
icon: 'cancel',
12851305
},
@@ -1288,10 +1308,22 @@ export class AgenticChatController implements ChatHandlers {
12881308
header = {
12891309
status: requiresAcceptance
12901310
? {
1291-
icon: 'warning',
1292-
status: 'warning',
1311+
icon:
1312+
commandCategory === CommandCategory.Destructive
1313+
? 'warning'
1314+
: commandCategory === CommandCategory.Mutate
1315+
? 'info'
1316+
: 'none',
1317+
status:
1318+
commandCategory === CommandCategory.Destructive
1319+
? 'warning'
1320+
: commandCategory === CommandCategory.Mutate
1321+
? 'info'
1322+
: undefined,
12931323
position: 'left',
1294-
// TODO: Add `description` if necessary to show a tooltip
1324+
description: this.#getCommandCategoryDescription(
1325+
commandCategory ?? CommandCategory.ReadOnly
1326+
),
12951327
}
12961328
: {},
12971329
body: 'shell',
@@ -1347,7 +1379,7 @@ export class AgenticChatController implements ChatHandlers {
13471379
type: 'tool',
13481380
messageId: this.#getMessageIdForToolUse(toolType, toolUse),
13491381
header,
1350-
body: warning ? warning + (toolType === 'executeBash' ? '' : '\n\n') + body : body,
1382+
body: warning ? (toolType === 'executeBash' ? '' : '\n\n') + body : body,
13511383
}
13521384
}
13531385

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/executeBash.ts‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,12 @@ export class ExecuteBash {
208208
allCommands.push(currentCmd)
209209
}
210210

211+
// Track highest command category (ReadOnly < Mutate < Destructive)
212+
let highestCommandCategory = CommandCategory.ReadOnly
213+
211214
for (const cmdArgs of allCommands) {
212215
if (cmdArgs.length === 0) {
213-
return { requiresAcceptance: true }
216+
return { requiresAcceptance: true, commandCategory: highestCommandCategory }
214217
}
215218

216219
// For each command, validate arguments for path safety within workspace
@@ -220,7 +223,11 @@ export class ExecuteBash {
220223
let fullPath: string
221224
if (!IS_WINDOWS_PLATFORM && arg.startsWith('~')) {
222225
// Treat tilde paths as absolute paths (they will be expanded by the shell)
223-
return { requiresAcceptance: true, warning: destructiveCommandWarningMessage }
226+
return {
227+
requiresAcceptance: true,
228+
warning: destructiveCommandWarningMessage,
229+
commandCategory: CommandCategory.Destructive,
230+
}
224231
} else if (!isAbsolute(arg) && params.cwd) {
225232
// If not absolute, resolve using workingDirectory if available
226233
fullPath = join(params.cwd, arg)
@@ -235,23 +242,48 @@ export class ExecuteBash {
235242

236243
const isInWorkspace = workspaceUtils.isInWorkspace(getWorkspaceFolderPaths(this.lsp), fullPath)
237244
if (!isInWorkspace) {
238-
return { requiresAcceptance: true, warning: outOfWorkspaceWarningmessage }
245+
return {
246+
requiresAcceptance: true,
247+
warning: outOfWorkspaceWarningmessage,
248+
commandCategory: highestCommandCategory,
249+
}
239250
}
240251
}
241252
}
242253

243254
const command = cmdArgs[0]
244255
const category = commandCategories.get(command)
245256

257+
// Update the highest command category if current command has higher risk
258+
if (category === CommandCategory.Destructive) {
259+
highestCommandCategory = CommandCategory.Destructive
260+
} else if (
261+
category === CommandCategory.Mutate &&
262+
highestCommandCategory !== CommandCategory.Destructive
263+
) {
264+
highestCommandCategory = CommandCategory.Mutate
265+
}
266+
246267
switch (category) {
247268
case CommandCategory.Destructive:
248-
return { requiresAcceptance: true, warning: destructiveCommandWarningMessage }
269+
return {
270+
requiresAcceptance: true,
271+
warning: destructiveCommandWarningMessage,
272+
commandCategory: CommandCategory.Destructive,
273+
}
249274
case CommandCategory.Mutate:
250-
return { requiresAcceptance: true, warning: mutateCommandWarningMessage }
275+
return {
276+
requiresAcceptance: true,
277+
warning: mutateCommandWarningMessage,
278+
commandCategory: CommandCategory.Mutate,
279+
}
251280
case CommandCategory.ReadOnly:
252281
continue
253282
default:
254-
return { requiresAcceptance: true }
283+
return {
284+
requiresAcceptance: true,
285+
commandCategory: highestCommandCategory,
286+
}
255287
}
256288
}
257289
// Finally, check if the cwd is outside the workspace
@@ -262,7 +294,11 @@ export class ExecuteBash {
262294

263295
// If there are no workspace folders, we can't validate the path
264296
if (!workspaceFolders || workspaceFolders.length === 0) {
265-
return { requiresAcceptance: true, warning: outOfWorkspaceWarningmessage }
297+
return {
298+
requiresAcceptance: true,
299+
warning: outOfWorkspaceWarningmessage,
300+
commandCategory: highestCommandCategory,
301+
}
266302
}
267303

268304
// Normalize paths for consistent comparison
@@ -275,16 +311,20 @@ export class ExecuteBash {
275311
)
276312

277313
if (!isInWorkspace) {
278-
return { requiresAcceptance: true, warning: outOfWorkspaceWarningmessage }
314+
return {
315+
requiresAcceptance: true,
316+
warning: outOfWorkspaceWarningmessage,
317+
commandCategory: highestCommandCategory,
318+
}
279319
}
280320
}
281321
}
282322

283323
// If we've checked all commands and none required acceptance, we're good
284-
return { requiresAcceptance: false }
324+
return { requiresAcceptance: false, commandCategory: highestCommandCategory }
285325
} catch (error) {
286326
this.logging.warn(`Error while checking acceptance: ${(error as Error).message}`)
287-
return { requiresAcceptance: true }
327+
return { requiresAcceptance: true, commandCategory: CommandCategory.ReadOnly }
288328
}
289329
}
290330

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Features } from '@aws/language-server-runtimes/server-interface/server'
22
import { workspaceUtils } from '@aws/lsp-core'
33
import { getWorkspaceFolderPaths } from '@aws/lsp-core/out/util/workspaceUtils'
44
import * as path from 'path'
5+
import { CommandCategory } from './executeBash'
56

67
interface Output<Kind, Content> {
78
kind: Kind
@@ -16,6 +17,7 @@ export interface InvokeOutput {
1617
export interface CommandValidation {
1718
requiresAcceptance: boolean
1819
warning?: string
20+
commandCategory?: CommandCategory
1921
}
2022

2123
export async function validatePath(path: string, exists: (p: string) => Promise<boolean>) {
@@ -28,11 +30,6 @@ export async function validatePath(path: string, exists: (p: string) => Promise<
2830
}
2931
}
3032

31-
export interface CommandValidation {
32-
requiresAcceptance: boolean
33-
warning?: string
34-
}
35-
3633
export class ToolApprovalException extends Error {
3734
public override readonly message: string
3835
public readonly shouldShowMessage: boolean

0 commit comments

Comments
 (0)