Skip to content

Commit 8c2cab6

Browse files
dungdong-awsdungdong-aws
authored
fix(amazonq): revert commit f17b631 (aws#1965)
1 parent 519f75d commit 8c2cab6

File tree

10 files changed

+94
-186
lines changed

10 files changed

+94
-186
lines changed

‎chat-client/src/client/tabs/tabFactory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ export class TabFactory {
178178
tabBarButtons.push({
179179
id: McpServerTabButtonId,
180180
icon: MynahIcons.TOOLS,
181-
description: 'Configure MCP servers and Built-in tools',
181+
description: 'Configure MCP servers',
182182
})
183183
}
184184

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

Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,6 @@ import {
185185
DEFAULT_WINDOW_REJECT_SHORTCUT,
186186
DEFAULT_MACOS_STOP_SHORTCUT,
187187
DEFAULT_WINDOW_STOP_SHORTCUT,
188-
OUT_OF_WORKSPACE_WARNING_MSG,
189-
CREDENTIAL_FILE_WARNING_MSG,
190-
BINARY_FILE_WARNING_MSG,
191188
} from './constants/constants'
192189
import {
193190
AgenticChatError,
@@ -1695,9 +1692,9 @@ export class AgenticChatController implements ChatHandlers {
16951692
const tool = new Tool(this.#features)
16961693

16971694
// For MCP tools, get the permission from McpManager
1698-
const permission = McpManager.instance.getToolPerm('Built-in', toolUse.name)
1695+
// const permission = McpManager.instance.getToolPerm('Built-in', toolUse.name)
16991696
// If permission is 'alwaysAllow', we don't need to ask for acceptance
1700-
const builtInPermission = permission !== 'alwaysAllow'
1697+
// const builtInPermission = permission !== 'alwaysAllow'
17011698

17021699
// Get the approved paths from the session
17031700
const approvedPaths = session.approvedPaths
@@ -1708,40 +1705,19 @@ export class AgenticChatController implements ChatHandlers {
17081705
approvedPaths
17091706
)
17101707

1711-
const isExecuteBash = toolUse.name === EXECUTE_BASH
1712-
1713-
// check if tool execution's path is out of workspace
1714-
const isOutOfWorkSpace = warning === OUT_OF_WORKSPACE_WARNING_MSG
1715-
// check if tool involved secured files
1716-
const isSecuredFilesInvoled =
1717-
warning === BINARY_FILE_WARNING_MSG || warning === CREDENTIAL_FILE_WARNING_MSG
1718-
17191708
// Honor built-in permission if available, otherwise use tool's requiresAcceptance
1720-
let toolRequiresAcceptance =
1721-
(builtInPermission || isOutOfWorkSpace || isSecuredFilesInvoled) ?? requiresAcceptance
1722-
1723-
// if the command is read-only and in-workspace --> flip back to no approval needed
1724-
if (
1725-
isExecuteBash &&
1726-
commandCategory === CommandCategory.ReadOnly &&
1727-
!isOutOfWorkSpace &&
1728-
!requiresAcceptance
1729-
) {
1730-
toolRequiresAcceptance = false
1731-
}
1709+
// const requiresAcceptance = builtInPermission || toolRequiresAcceptance
17321710

1733-
if (toolRequiresAcceptance || isExecuteBash) {
1711+
if (requiresAcceptance || toolUse.name === EXECUTE_BASH) {
17341712
// for executeBash, we till send the confirmation message without action buttons
17351713
const confirmationResult = this.#processToolConfirmation(
17361714
toolUse,
1737-
toolRequiresAcceptance,
1715+
requiresAcceptance,
17381716
warning,
1739-
commandCategory,
1740-
toolUse.name,
1741-
builtInPermission
1717+
commandCategory
17421718
)
17431719
cachedButtonBlockId = await chatResultStream.writeResultBlock(confirmationResult)
1744-
1720+
const isExecuteBash = toolUse.name === EXECUTE_BASH
17451721
if (isExecuteBash) {
17461722
this.#telemetryController.emitInteractWithAgenticChat(
17471723
'GeneratedCommand',
@@ -1752,7 +1728,7 @@ export class AgenticChatController implements ChatHandlers {
17521728
this.#abTestingAllocation?.userVariation
17531729
)
17541730
}
1755-
if (toolRequiresAcceptance) {
1731+
if (requiresAcceptance) {
17561732
await this.waitForToolApproval(
17571733
toolUse,
17581734
chatResultStream,
@@ -2723,7 +2699,7 @@ export class AgenticChatController implements ChatHandlers {
27232699
body = builtInPermission
27242700
? `I need permission to read files.\n${formattedPaths.join('\n')}`
27252701
: `I need permission to read files outside the workspace.\n${formattedPaths.join('\n')}`
2726-
} else if (toolName === 'listDirectory') {
2702+
} else {
27272703
const readFilePath = (toolUse.input as unknown as ListDirectoryParams).path
27282704

27292705
// Validate the path using our synchronous utility
@@ -2733,11 +2709,6 @@ export class AgenticChatController implements ChatHandlers {
27332709
body = builtInPermission
27342710
? `I need permission to list directories.\n\`${readFilePath}\``
27352711
: `I need permission to list directories outside the workspace.\n\`${readFilePath}\``
2736-
} else {
2737-
const readFilePath = (toolUse.input as unknown as ListDirectoryParams).path
2738-
body = builtInPermission
2739-
? `I need permission to search files.\n\`${readFilePath}\``
2740-
: `I need permission to search files outside the workspace.\n\`${readFilePath}\``
27412712
}
27422713
break
27432714
}

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,3 @@ export const DEFAULT_LINUX_STOP_SHORTCUT = 'Meta + ⇧ + ⌫'
8888
export const DEFAULT_MACOS_REJECT_SHORTCUT = '⇧ ⌘ R'
8989
export const DEFAULT_WINDOW_REJECT_SHORTCUT = 'Ctrl + ⇧ + R'
9090
export const DEFAULT_LINUX_REJECT_SHORTCUT = 'Meta + ⇧ + R'
91-
92-
// Warning Message Constants
93-
export const DESTRUCTIVE_COMMAND_WARNING_MSG = 'WARNING: Potentially destructive command detected:\n\n'
94-
export const MUTATE_COMMAND_WARNING_MSG = 'Mutation command:\n\n'
95-
export const OUT_OF_WORKSPACE_WARNING_MSG = 'Execution out of workspace scope:\n\n'
96-
export const CREDENTIAL_FILE_WARNING_MSG =
97-
'WARNING: Command involves credential files that require secure permissions:\n\n'
98-
export const BINARY_FILE_WARNING_MSG = 'WARNING: Command involves binary files that require secure permissions:\n\n'

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

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,6 @@ import { getWorkspaceFolderPaths } from '@aws/lsp-core/out/util/workspaceUtils'
1313
// eslint-disable-next-line import/no-nodejs-modules
1414
import { existsSync, statSync } from 'fs'
1515

16-
// Warning message
17-
import {
18-
BINARY_FILE_WARNING_MSG,
19-
CREDENTIAL_FILE_WARNING_MSG,
20-
DESTRUCTIVE_COMMAND_WARNING_MSG,
21-
MUTATE_COMMAND_WARNING_MSG,
22-
OUT_OF_WORKSPACE_WARNING_MSG,
23-
} from '../constants/constants'
24-
2516
export enum CommandCategory {
2617
ReadOnly,
2718
Mutate,
@@ -117,6 +108,12 @@ export const commandCategories = new Map<string, CommandCategory>([
117108
])
118109
export const maxToolResponseSize: number = 1024 * 1024 // 1MB
119110
export const lineCount: number = 1024
111+
export const destructiveCommandWarningMessage = 'WARNING: Potentially destructive command detected:\n\n'
112+
export const mutateCommandWarningMessage = 'Mutation command:\n\n'
113+
export const outOfWorkspaceWarningmessage = 'Execution out of workspace scope:\n\n'
114+
export const credentialFileWarningMessage =
115+
'WARNING: Command involves credential files that require secure permissions:\n\n'
116+
export const binaryFileWarningMessage = 'WARNING: Command involves binary files that require secure permissions:\n\n'
120117

121118
/**
122119
* Parameters for executing a command on the system shell.
@@ -235,7 +232,7 @@ export class ExecuteBash {
235232
// Treat tilde paths as absolute paths (they will be expanded by the shell)
236233
return {
237234
requiresAcceptance: true,
238-
warning: DESTRUCTIVE_COMMAND_WARNING_MSG,
235+
warning: destructiveCommandWarningMessage,
239236
commandCategory: CommandCategory.Destructive,
240237
}
241238
} else if (!isAbsolute(arg) && params.cwd) {
@@ -258,7 +255,7 @@ export class ExecuteBash {
258255
this.logging.info(`Detected credential file in command: ${fullPath}`)
259256
return {
260257
requiresAcceptance: true,
261-
warning: CREDENTIAL_FILE_WARNING_MSG,
258+
warning: credentialFileWarningMessage,
262259
commandCategory: CommandCategory.Mutate,
263260
}
264261
}
@@ -268,7 +265,7 @@ export class ExecuteBash {
268265
this.logging.info(`Detected binary file in command: ${fullPath}`)
269266
return {
270267
requiresAcceptance: true,
271-
warning: BINARY_FILE_WARNING_MSG,
268+
warning: binaryFileWarningMessage,
272269
commandCategory: CommandCategory.Mutate,
273270
}
274271
}
@@ -285,7 +282,7 @@ export class ExecuteBash {
285282
if (!isInWorkspace) {
286283
return {
287284
requiresAcceptance: true,
288-
warning: OUT_OF_WORKSPACE_WARNING_MSG,
285+
warning: outOfWorkspaceWarningmessage,
289286
commandCategory: highestCommandCategory,
290287
}
291288
}
@@ -309,13 +306,13 @@ export class ExecuteBash {
309306
case CommandCategory.Destructive:
310307
return {
311308
requiresAcceptance: true,
312-
warning: DESTRUCTIVE_COMMAND_WARNING_MSG,
309+
warning: destructiveCommandWarningMessage,
313310
commandCategory: CommandCategory.Destructive,
314311
}
315312
case CommandCategory.Mutate:
316313
return {
317314
requiresAcceptance: true,
318-
warning: MUTATE_COMMAND_WARNING_MSG,
315+
warning: mutateCommandWarningMessage,
319316
commandCategory: CommandCategory.Mutate,
320317
}
321318
case CommandCategory.ReadOnly:
@@ -337,7 +334,7 @@ export class ExecuteBash {
337334
if (!workspaceFolders || workspaceFolders.length === 0) {
338335
return {
339336
requiresAcceptance: true,
340-
warning: OUT_OF_WORKSPACE_WARNING_MSG,
337+
warning: outOfWorkspaceWarningmessage,
341338
commandCategory: highestCommandCategory,
342339
}
343340
}
@@ -354,7 +351,7 @@ export class ExecuteBash {
354351
if (!isInWorkspace) {
355352
return {
356353
requiresAcceptance: true,
357-
warning: OUT_OF_WORKSPACE_WARNING_MSG,
354+
warning: outOfWorkspaceWarningmessage,
358355
commandCategory: highestCommandCategory,
359356
}
360357
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { ChildProcess, ChildProcessOptions } from '@aws/lsp-core/out/util/proces
77
import path = require('path')
88
import { dirname } from 'path'
99
import { pathToFileURL } from 'url'
10-
import { OUT_OF_WORKSPACE_WARNING_MSG } from '../constants/constants'
1110

1211
export interface GrepSearchParams {
1312
path?: string
@@ -82,8 +81,7 @@ export class GrepSearch {
8281

8382
public async requiresAcceptance(params: GrepSearchParams): Promise<CommandValidation> {
8483
const path = this.getSearchDirectory(params.path)
85-
const isInWorkspace = workspaceUtils.isInWorkspace(getWorkspaceFolderPaths(this.workspace), path)
86-
return { requiresAcceptance: !isInWorkspace, warning: !isInWorkspace ? OUT_OF_WORKSPACE_WARNING_MSG : '' }
84+
return { requiresAcceptance: !workspaceUtils.isInWorkspace(getWorkspaceFolderPaths(this.workspace), path) }
8785
}
8886

8987
public async invoke(params: GrepSearchParams, token?: CancellationToken): Promise<InvokeOutput> {

‎server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpEventHandler.test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ describe('McpEventHandler error handling', () => {
4949
},
5050
agent: {
5151
getTools: sinon.stub().returns([]),
52-
getBuiltInToolNames: sinon.stub().returns([]),
5352
},
5453
lsp: {},
5554
telemetry: {

0 commit comments

Comments
 (0)