fix(bash): kill process group immediately on cancel
Watches for context cancellation in a goroutine and kills the entire process group plus closes the pipe immediately, instead of waiting for cmd.Wait() which deadlocks when child processes hold the pipe open.
- commit
- ba3f3287e9514f73c67c6658142964751c90efc1
- author
- patriceckhart · · 2026-04-23 16:47 UTC
- parent
- daaa062
- signature
- unsigned
1 file · +14 -5
M internal/agent/tools/bash.go +14 -5
| @@ -82,6 +82,20 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 82 | 82 | // Writer to both the buffer (trimmed) and progress callback. | |
| 83 | 83 | captured := &bytes.Buffer{} | |
| 84 | 84 | done := make(chan struct{}) | |
| 85 | + | ||
| 86 | + | // Watch for context cancellation and kill the entire process | |
| 87 | + | // group immediately. exec.CommandContext only kills the direct | |
| 88 | + | // process, but child processes (e.g. grep spawned by the shell) | |
| 89 | + | // keep the output pipe open and block cmd.Wait() indefinitely. | |
| 90 | + | go func() { | |
| 91 | + | select { | |
| 92 | + | case <-runCtx.Done(): | |
| 93 | + | killProcessGroup(cmd) | |
| 94 | + | // Close the write end so the reader goroutine unblocks. | |
| 95 | + | pw.Close() | |
| 96 | + | case <-done: | |
| 97 | + | } | |
| 98 | + | }() | |
| 85 | 99 | go func() { | |
| 86 | 100 | defer close(done) | |
| 87 | 101 | buf := make([]byte, 4096) | |
| @@ -111,11 +125,6 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 111 | 125 | pw.Close() | |
| 112 | 126 | <-done | |
| 113 | 127 | ||
| 114 | - | // On hard cancel, try to kill the process group. | |
| 115 | - | if ctx.Err() != nil && cmd.Process != nil { | |
| 116 | - | killProcessGroup(cmd) | |
| 117 | - | } | |
| 118 | - | ||
| 119 | 128 | output := captured.String() | |
| 120 | 129 | truncBytes := captured.Len() >= maxBashBytes | |
| 121 | 130 | lines := strings.Split(output, "\n") | |
| @@ -82,6 +82,20 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 82 | // Writer to both the buffer (trimmed) and progress callback. | 82 | // Writer to both the buffer (trimmed) and progress callback. | ||
| 83 | captured := &bytes.Buffer{} | 83 | captured := &bytes.Buffer{} | ||
| 84 | done := make(chan struct{}) | 84 | done := make(chan struct{}) | ||
| 85 | + | ||||
| 86 | + | // Watch for context cancellation and kill the entire process | |||
| 87 | + | // group immediately. exec.CommandContext only kills the direct | |||
| 88 | + | // process, but child processes (e.g. grep spawned by the shell) | |||
| 89 | + | // keep the output pipe open and block cmd.Wait() indefinitely. | |||
| 90 | + | go func() { | |||
| 91 | + | select { | |||
| 92 | + | case <-runCtx.Done(): | |||
| 93 | + | killProcessGroup(cmd) | |||
| 94 | + | // Close the write end so the reader goroutine unblocks. | |||
| 95 | + | pw.Close() | |||
| 96 | + | case <-done: | |||
| 97 | + | } | |||
| 98 | + | }() | |||
| 85 | go func() { | 99 | go func() { | ||
| 86 | defer close(done) | 100 | defer close(done) | ||
| 87 | buf := make([]byte, 4096) | 101 | buf := make([]byte, 4096) | ||
| @@ -111,11 +125,6 @@ func (t *BashTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 111 | pw.Close() | 125 | pw.Close() | ||
| 112 | <-done | 126 | <-done | ||
| 113 | 127 | ||||
| 114 | - | // On hard cancel, try to kill the process group. | |||
| 115 | - | if ctx.Err() != nil && cmd.Process != nil { | |||
| 116 | - | killProcessGroup(cmd) | |||
| 117 | - | } | |||
| 118 | - | ||||
| 119 | output := captured.String() | 128 | output := captured.String() | ||
| 120 | truncBytes := captured.Len() >= maxBashBytes | 129 | truncBytes := captured.Len() >= maxBashBytes | ||
| 121 | lines := strings.Split(output, "\n") | 130 | lines := strings.Split(output, "\n") | ||