Skip to content

Commit 950b0f4

Browse files
committed
fix for stopping bash executions
1 parent ec528bd commit 950b0f4

File tree

7 files changed

+81
-19
lines changed

7 files changed

+81
-19
lines changed

packages/core/src/amazonq/webview/ui/apps/cwChatConnector.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export class Connector extends BaseConnector {
4444
private readonly onChatAnswerUpdated
4545
private readonly onAsyncEventProgress
4646
private chatItems: Map<string, Map<string, ChatItem>> = new Map() // tabId -> messageId -> ChatItem
47+
private tabIdToTriggerId: Map<string, string> = new Map() // tabId -> triggerId
4748

4849
override getTabType(): TabType {
4950
return 'cwc'
@@ -135,6 +136,10 @@ export class Connector extends BaseConnector {
135136
}
136137
}
137138

139+
if (messageData.tabID && messageData.triggerID && messageData.triggerID !== '') {
140+
this.storetriggerId(messageData.tabID, messageData.triggerID)
141+
}
142+
138143
if (answer.messageId) {
139144
this.storeChatItem(messageData.tabID, answer.messageId, answer)
140145
}
@@ -213,6 +218,14 @@ export class Connector extends BaseConnector {
213218
this.chatItems.get(tabId)?.set(messageId, { ...item })
214219
}
215220

221+
private storetriggerId(tabId: string, triggerId: string): void {
222+
this.tabIdToTriggerId.set(tabId, triggerId)
223+
}
224+
225+
private getTriggerId(tabId: string): string | undefined {
226+
return this.tabIdToTriggerId.get(tabId)
227+
}
228+
216229
private getCurrentChatItem(tabId: string, messageId: string | undefined): ChatItem | undefined {
217230
if (!messageId) {
218231
return
@@ -351,7 +364,7 @@ export class Connector extends BaseConnector {
351364
formSelectedValues: action.formItemValues,
352365
tabType: this.getTabType(),
353366
tabID: tabId,
354-
triggerId: triggerId,
367+
triggerId: this.getTriggerId(tabId),
355368
})
356369

357370
if (

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

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

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -709,9 +709,7 @@ export class ChatController {
709709
this.editorContextExtractor
710710
.extractContextForTrigger('ChatMessage')
711711
.then(async (context) => {
712-
console.log('message for tool action:', message)
713712
const triggerID = message.triggerId
714-
715713
// Check if this trigger has already been cancelled
716714
if (this.isTriggerCancelled(triggerID)) {
717715
return

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

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -249,16 +249,13 @@ export class ExecuteBash {
249249
* Check if the trigger has been cancelled using ConversationTracker
250250
*/
251251
private isTriggerCancelled(): boolean {
252-
// console.log('trigger cancellation check is happening')
253252
if (!this.triggerId) {
254253
return false
255254
}
256255

257256
// Import here to avoid circular dependency
258257
const { ConversationTracker } = require('../storages/conversationTracker')
259258
const cancellationtracker = ConversationTracker.getInstance()
260-
// console.log('trigger cancellation status:', cancellationtracker.getTokenForTrigger(this.triggerId))
261-
// console.log('concealltion status:', cancellationtracker.isTriggerCancelled(this.triggerId))
262259
return cancellationtracker.isTriggerCancelled(this.triggerId)
263260
}
264261

@@ -275,24 +272,26 @@ export class ExecuteBash {
275272

276273
// Modify the command to make it more cancellable by using process groups
277274
// This ensures that when we kill the parent process, all child processes are also terminated
278-
// The trap ensures cleanup on SIGTERM/SIGINT
275+
// The trap ensures cleanup on SIGTERM/SIGINT and sends SIGTERM to the child process group
279276
const modifiedCommand = `
280277
exec bash -c "
281278
# Create a new process group
282279
set -m
283280
284281
# Set up trap to kill the entire process group on exit
285-
trap 'exit' TERM INT
282+
trap 'kill -TERM -\\$CMD_PID 2>/dev/null || true; exit' TERM INT
286283
287284
# Run the actual command in background
288-
${this.command} &
285+
# Use '()' to create a subshell which becomes the process group leader
286+
(${this.command}) &
289287
290288
# Store the PID
291289
CMD_PID=\\$!
292290
293291
# Wait for the command to finish
294292
wait \\$CMD_PID
295-
exit \\$?
293+
exit_code=\\$?
294+
exit \\$exit_code
296295
"
297296
`
298297

@@ -343,13 +342,40 @@ exec bash -c "
343342
checkCancellationInterval = setInterval(() => {
344343
if (this.isTriggerCancelled()) {
345344
this.logger.debug('Trigger cancellation detected, killing child process')
346-
this.childProcess?.stop(false, 'SIGTERM')
345+
346+
// First try to kill the entire process group
347+
if (this.childProcess && this.childProcess.pid) {
348+
try {
349+
// On Unix systems, negative PID kills the process group
350+
this.logger.debug(`Sending SIGTERM to process group -${this.childProcess.pid}`)
351+
process.kill(-this.childProcess.pid, 'SIGTERM')
352+
} catch (err) {
353+
this.logger.debug(`Failed to kill process group: ${err}`)
354+
// Fall back to regular process termination
355+
this.childProcess?.stop(false, 'SIGTERM')
356+
}
357+
} else {
358+
this.childProcess?.stop(false, 'SIGTERM')
359+
}
347360

348361
// After a short delay, force kill with SIGKILL if still running
349362
setTimeout(() => {
350363
if (this.childProcess && !this.childProcess.stopped) {
351364
this.logger.debug('Process still running after SIGTERM, sending SIGKILL')
352-
this.childProcess.stop(true, 'SIGKILL')
365+
366+
// Try to kill the process group with SIGKILL
367+
if (this.childProcess.pid) {
368+
try {
369+
this.logger.debug(`Sending SIGKILL to process group -${this.childProcess.pid}`)
370+
process.kill(-this.childProcess.pid, 'SIGKILL')
371+
} catch (err) {
372+
this.logger.debug(`Failed to kill process group with SIGKILL: ${err}`)
373+
// Fall back to regular process termination
374+
this.childProcess.stop(true, 'SIGKILL')
375+
}
376+
} else {
377+
this.childProcess.stop(true, 'SIGKILL')
378+
}
353379
}
354380
}, 500)
355381

@@ -365,7 +391,11 @@ exec bash -c "
365391
cwd: this.workingDirectory,
366392
stdio: ['pipe', 'pipe', 'pipe'],
367393
// Set detached to true to create a new process group
394+
// This allows us to kill the entire process group later
368395
detached: true,
396+
// On Windows, we need to create a new process group
397+
// On Unix, we need to create a new session
398+
...(process.platform === 'win32' ? { windowsVerbatimArguments: true } : {}),
369399
},
370400
collect: false,
371401
waitForStreams: true,

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
fsReadToolResponseSize,
1515
} from './toolShared'
1616
import { ListDirectory, ListDirectoryParams } from './listDirectory'
17-
import { ConversationTracker } from '../storages/conversationTracker'
1817

1918
export enum ToolType {
2019
FsRead = 'fsRead',
@@ -65,8 +64,7 @@ export class ToolUtils {
6564
case ToolType.ExecuteBash:
6665
// If triggerId is provided, update the tool's triggerId
6766
if (triggerId) {
68-
;(tool.tool as ExecuteBash).triggerId =
69-
ConversationTracker.getInstance().getTriggerIdForToolUseId(triggerId)
67+
;(tool.tool as ExecuteBash).triggerId = triggerId
7068
}
7169
return tool.tool.invoke(updates)
7270
case ToolType.ListDirectory:

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

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

520520
public sendCustomFormActionMessage(message: CustomFormActionMessage) {
521-
console.log('sendCustomFormActionMessage', message)
522521
this.appsToWebViewMessagePublisher.publish(message)
523522
}
524523

packages/core/src/shared/utilities/processUtils.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,13 +426,38 @@ export class ChildProcess {
426426
const command = this.#command
427427
const pid = this.pid()
428428
if (!this.stopped) {
429-
child.kill(signal)
429+
// Try to kill the process group if possible (Unix systems only)
430+
if (process.platform !== 'win32' && child.pid && child.spawnargs && child.spawnfile) {
431+
try {
432+
// On Unix systems, negative PID kills the process group
433+
this.#log.debug(`Attempting to kill process group -${child.pid} with ${signal || 'SIGTERM'}`)
434+
process.kill(-child.pid, signal || 'SIGTERM')
435+
} catch (err) {
436+
this.#log.debug(`Failed to kill process group: ${err}, falling back to regular kill`)
437+
child.kill(signal)
438+
}
439+
} else {
440+
child.kill(signal)
441+
}
430442

431443
if (force === true) {
432444
waitUntil(async () => this.stopped, { timeout: ChildProcess.stopTimeout, interval: 200, truthy: true })
433445
.then((stopped) => {
434446
if (!stopped) {
435-
child.kill('SIGKILL')
447+
// Try to kill the process group with SIGKILL if possible
448+
if (process.platform !== 'win32' && child.pid) {
449+
try {
450+
this.#log.debug(`Attempting to kill process group -${child.pid} with SIGKILL`)
451+
process.kill(-child.pid, 'SIGKILL')
452+
} catch (err) {
453+
this.#log.debug(
454+
`Failed to kill process group with SIGKILL: ${err}, falling back to regular kill`
455+
)
456+
child.kill('SIGKILL')
457+
}
458+
} else {
459+
child.kill('SIGKILL')
460+
}
436461
}
437462
})
438463
.catch((e) => {

0 commit comments

Comments
 (0)