Skip to content

Commit ec528bd

Browse files
committed
fix for stop button for execute bash
1 parent 3b71170 commit ec528bd

File tree

5 files changed

+114
-36
lines changed

5 files changed

+114
-36
lines changed

packages/core/src/amazonq/webview/ui/connector.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,7 @@ export class Connector {
744744
tabType: 'cwc',
745745
})
746746
} else {
747+
console.log('messageId:', messageId, 'event Id:', eventId)
747748
this.cwChatConnector.onCustomFormAction(tabId, messageId ?? '', action, messageId ?? '')
748749
}
749750
break

packages/core/src/codewhispererChat/controllers/chat/controller.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,7 @@ export class ChatController {
709709
this.editorContextExtractor
710710
.extractContextForTrigger('ChatMessage')
711711
.then(async (context) => {
712+
console.log('message for tool action:', message)
712713
const triggerID = message.triggerId
713714

714715
// Check if this trigger has already been cancelled
@@ -786,11 +787,7 @@ export class ChatController {
786787
return
787788
}
788789

789-
const output = await ToolUtils.invoke(
790-
tool,
791-
chatStream,
792-
ConversationTracker.getInstance().getTokenForTrigger(triggerID)
793-
)
790+
const output = await ToolUtils.invoke(tool, chatStream, triggerID)
794791
ToolUtils.validateOutput(output, tool.type)
795792

796793
let status: ToolResultStatus = ToolResultStatus.SUCCESS
@@ -1632,7 +1629,7 @@ export class ChatController {
16321629
})}`
16331630
)
16341631
let response: MessengerResponseType | undefined = undefined
1635-
session.createNewTokenSource()
1632+
// session.createNewTokenSource()
16361633
// TODO: onProfileChanged, abort previous response?
16371634
try {
16381635
if (!session.context && triggerPayload.context.length) {

packages/core/src/codewhispererChat/tools/executeBash.ts

Lines changed: 101 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ export interface ExecuteBashParams {
114114
command: string
115115
cwd?: string
116116
explanation?: string
117+
triggerId?: string
117118
}
118119

119120
export interface CommandValidation {
@@ -134,10 +135,22 @@ export class ExecuteBash {
134135
private readonly workingDirectory?: string
135136
private readonly logger = getLogger('executeBash')
136137
private childProcess?: ChildProcess
138+
// Make triggerId writable so it can be set after construction
139+
private _triggerId?: string
137140

138141
constructor(params: ExecuteBashParams) {
139142
this.command = params.command
140143
this.workingDirectory = params.cwd ? sanitizePath(params.cwd) : fs.getUserHomeDir()
144+
this._triggerId = params.triggerId
145+
}
146+
147+
// Getter and setter for triggerId
148+
get triggerId(): string | undefined {
149+
return this._triggerId
150+
}
151+
152+
set triggerId(id: string | undefined) {
153+
this._triggerId = id
141154
}
142155

143156
public async validate(): Promise<void> {
@@ -232,18 +245,60 @@ export class ExecuteBash {
232245
}
233246
}
234247

235-
public async invoke(updates?: Writable, cancellationToken?: vscode.CancellationToken): Promise<InvokeOutput> {
248+
/**
249+
* Check if the trigger has been cancelled using ConversationTracker
250+
*/
251+
private isTriggerCancelled(): boolean {
252+
// console.log('trigger cancellation check is happening')
253+
if (!this.triggerId) {
254+
return false
255+
}
256+
257+
// Import here to avoid circular dependency
258+
const { ConversationTracker } = require('../storages/conversationTracker')
259+
const cancellationtracker = ConversationTracker.getInstance()
260+
// console.log('trigger cancellation status:', cancellationtracker.getTokenForTrigger(this.triggerId))
261+
// console.log('concealltion status:', cancellationtracker.isTriggerCancelled(this.triggerId))
262+
return cancellationtracker.isTriggerCancelled(this.triggerId)
263+
}
264+
265+
public async invoke(updates?: Writable): Promise<InvokeOutput> {
236266
this.logger.info(`Invoking bash command: "${this.command}" in cwd: "${this.workingDirectory}"`)
237267

238268
return new Promise(async (resolve, reject) => {
239-
// Check if cancelled before starting
240-
if (cancellationToken?.isCancellationRequested) {
269+
// Check if cancelled before starting using triggerId
270+
if (this.isTriggerCancelled()) {
241271
this.logger.debug('Bash command execution cancelled before starting')
242272
reject(new Error('Command execution cancelled'))
243273
return
244274
}
245275

246-
this.logger.debug(`Spawning process with command: bash -c "${this.command}" (cwd=${this.workingDirectory})`)
276+
// Modify the command to make it more cancellable by using process groups
277+
// This ensures that when we kill the parent process, all child processes are also terminated
278+
// The trap ensures cleanup on SIGTERM/SIGINT
279+
const modifiedCommand = `
280+
exec bash -c "
281+
# Create a new process group
282+
set -m
283+
284+
# Set up trap to kill the entire process group on exit
285+
trap 'exit' TERM INT
286+
287+
# Run the actual command in background
288+
${this.command} &
289+
290+
# Store the PID
291+
CMD_PID=\\$!
292+
293+
# Wait for the command to finish
294+
wait \\$CMD_PID
295+
exit \\$?
296+
"
297+
`
298+
299+
this.logger.debug(
300+
`Spawning process with modified command for better cancellation support (cwd=${this.workingDirectory})`
301+
)
247302

248303
const stdoutBuffer: string[] = []
249304
const stderrBuffer: string[] = []
@@ -282,16 +337,41 @@ export class ExecuteBash {
282337
}
283338
}
284339

340+
// Setup a periodic check for trigger cancellation
341+
let checkCancellationInterval: NodeJS.Timeout | undefined
342+
if (this.triggerId) {
343+
checkCancellationInterval = setInterval(() => {
344+
if (this.isTriggerCancelled()) {
345+
this.logger.debug('Trigger cancellation detected, killing child process')
346+
this.childProcess?.stop(false, 'SIGTERM')
347+
348+
// After a short delay, force kill with SIGKILL if still running
349+
setTimeout(() => {
350+
if (this.childProcess && !this.childProcess.stopped) {
351+
this.logger.debug('Process still running after SIGTERM, sending SIGKILL')
352+
this.childProcess.stop(true, 'SIGKILL')
353+
}
354+
}, 500)
355+
356+
if (checkCancellationInterval) {
357+
clearInterval(checkCancellationInterval)
358+
}
359+
}
360+
}, 100) // Check every 100ms
361+
}
362+
285363
const childProcessOptions: ChildProcessOptions = {
286364
spawnOptions: {
287365
cwd: this.workingDirectory,
288366
stdio: ['pipe', 'pipe', 'pipe'],
367+
// Set detached to true to create a new process group
368+
detached: true,
289369
},
290370
collect: false,
291371
waitForStreams: true,
292372
onStdout: async (chunk: string) => {
293-
if (cancellationToken?.isCancellationRequested) {
294-
this.logger.debug('Bash command execution cancelled during stderr processing')
373+
if (this.isTriggerCancelled()) {
374+
this.logger.debug('Bash command execution cancelled during stdout processing')
295375
return
296376
}
297377
const isFirst = getAndSetFirstChunk(false)
@@ -305,7 +385,7 @@ export class ExecuteBash {
305385
processQueue()
306386
},
307387
onStderr: async (chunk: string) => {
308-
if (cancellationToken?.isCancellationRequested) {
388+
if (this.isTriggerCancelled()) {
309389
this.logger.debug('Bash command execution cancelled during stderr processing')
310390
return
311391
}
@@ -321,21 +401,19 @@ export class ExecuteBash {
321401
},
322402
}
323403

324-
this.childProcess = new ChildProcess('bash', ['-c', this.command], childProcessOptions)
325-
326-
// Set up cancellation listener
327-
if (cancellationToken) {
328-
cancellationToken.onCancellationRequested(() => {
329-
this.logger.debug('Cancellation requested, killing child process')
330-
this.childProcess?.stop()
331-
})
332-
}
404+
// Use bash directly with the modified command
405+
this.childProcess = new ChildProcess('bash', ['-c', modifiedCommand], childProcessOptions)
333406

334407
try {
335408
const result = await this.childProcess.run()
336409

410+
// Clean up the interval if it exists
411+
if (checkCancellationInterval) {
412+
clearInterval(checkCancellationInterval)
413+
}
414+
337415
// Check if cancelled after execution
338-
if (cancellationToken?.isCancellationRequested) {
416+
if (this.isTriggerCancelled()) {
339417
this.logger.debug('Bash command execution cancelled after completion')
340418
reject(new Error('Command execution cancelled'))
341419
return
@@ -368,8 +446,13 @@ export class ExecuteBash {
368446
},
369447
})
370448
} catch (err: any) {
449+
// Clean up the interval if it exists
450+
if (checkCancellationInterval) {
451+
clearInterval(checkCancellationInterval)
452+
}
453+
371454
// Check if this was due to cancellation
372-
if (cancellationToken?.isCancellationRequested) {
455+
if (this.isTriggerCancelled()) {
373456
reject(new Error('Command execution cancelled'))
374457
} else {
375458
this.logger.error(`Failed to execute bash command '${this.command}': ${err.message}`)

packages/core/src/codewhispererChat/tools/toolUtils.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
5-
import * as vscode from 'vscode'
65
import { Writable } from 'stream'
76
import { FsRead, FsReadParams } from './fsRead'
87
import { FsWrite, FsWriteParams } from './fsWrite'
@@ -15,6 +14,7 @@ import {
1514
fsReadToolResponseSize,
1615
} from './toolShared'
1716
import { ListDirectory, ListDirectoryParams } from './listDirectory'
17+
import { ConversationTracker } from '../storages/conversationTracker'
1818

1919
export enum ToolType {
2020
FsRead = 'fsRead',
@@ -56,23 +56,19 @@ export class ToolUtils {
5656
}
5757
}
5858

59-
static async invoke(
60-
tool: Tool,
61-
updates?: Writable,
62-
cancellationToken?: vscode.CancellationToken
63-
): Promise<InvokeOutput> {
64-
// Check if cancelled before executing
65-
if (cancellationToken?.isCancellationRequested) {
66-
throw new Error('Tool execution cancelled')
67-
}
68-
59+
static async invoke(tool: Tool, updates?: Writable, triggerId?: string): Promise<InvokeOutput> {
6960
switch (tool.type) {
7061
case ToolType.FsRead:
7162
return tool.tool.invoke(updates)
7263
case ToolType.FsWrite:
7364
return tool.tool.invoke(updates)
7465
case ToolType.ExecuteBash:
75-
return tool.tool.invoke(updates ?? undefined, cancellationToken)
66+
// If triggerId is provided, update the tool's triggerId
67+
if (triggerId) {
68+
;(tool.tool as ExecuteBash).triggerId =
69+
ConversationTracker.getInstance().getTriggerIdForToolUseId(triggerId)
70+
}
71+
return tool.tool.invoke(updates)
7672
case ToolType.ListDirectory:
7773
return tool.tool.invoke(updates)
7874
}

packages/core/src/codewhispererChat/view/connector/connector.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ export class AppToWebViewMessageDispatcher {
518518
}
519519

520520
public sendCustomFormActionMessage(message: CustomFormActionMessage) {
521+
console.log('sendCustomFormActionMessage', message)
521522
this.appsToWebViewMessagePublisher.publish(message)
522523
}
523524

0 commit comments

Comments
 (0)