Skip to content

Commit aade314

Browse files
jkyberneeesclaude
andcommitted
fix(tools): propagate agent context so Ctrl-C/timeout interrupts tools
Only delegate_tasks honored the loop's SetContext, so a turn timeout or Ctrl-C could not interrupt the other long-running tools — their HTTP requests and subprocesses ran to completion regardless, and the loop's drain blocked on them. Add a small race-safe ctxTool embeddable (SetContext + toolCtx) and wire it through the tools that do unbounded network or subprocess work: - http_batch, browser, web_search → http.NewRequestWithContext - vision (ffprobe/ffmpeg/llama-mtmd-cli), transcribe (ffmpeg/whisper) → exec.CommandContext - shell now uses the shared embed too (replacing its bare ctx field). The mutex in ctxTool matters: when the LLM emits two calls to the same tool in one turn, the loop sets the context on the shared instance from parallel goroutines — an unsynchronised field would race. Tests: ctxTool default/set/concurrent behavior, and http_batch returning a cancellation error when its context is already cancelled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5977e21 commit aade314

8 files changed

Lines changed: 134 additions & 27 deletions

File tree

cmd/odek/browser_tool.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ type browserState struct {
5555
// ── Browser Tool ──────────────────────────────────────────────────────
5656

5757
type browserTool struct {
58+
ctxTool
5859
state *browserState
5960
client *http.Client
6061
dangerousConfig danger.DangerousConfig
@@ -196,7 +197,7 @@ func (t *browserTool) doNavigate(rawURL string) (string, error) {
196197
return jsonError(fmt.Sprintf("invalid URL %q: must start with http:// or https://", rawURL))
197198
}
198199

199-
req, err := http.NewRequest("GET", rawURL, nil)
200+
req, err := http.NewRequestWithContext(t.toolCtx(), "GET", rawURL, nil)
200201
if err != nil {
201202
return jsonError(fmt.Sprintf("cannot create request: %v", err))
202203
}

cmd/odek/perf_tools.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,7 @@ func (t *parallelShellTool) runOne(cmd parallelShellCmd) parallelShellEntry {
412412
const maxHTTPBatchURLs = 10
413413

414414
type httpBatchTool struct {
415+
ctxTool
415416
dangerousConfig danger.DangerousConfig
416417
client *http.Client
417418
}
@@ -556,7 +557,7 @@ func (t *httpBatchTool) fetchOne(r httpBatchReq) httpBatchEntry {
556557
}
557558

558559
entry := httpBatchEntry{URL: r.URL}
559-
httpReq, err := http.NewRequest(method, r.URL, nil)
560+
httpReq, err := http.NewRequestWithContext(t.toolCtx(), method, r.URL, nil)
560561
if err != nil {
561562
entry.Error = err.Error()
562563
return entry

cmd/odek/shell.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,14 @@ type shellTool struct {
7272
// Overridden in tests to mock user input. Only used when approver is nil.
7373
ttyPath string
7474

75-
// ctx, when set via SetContext, ties command execution to the agent's
76-
// lifetime: cancelling it (Ctrl-C, turn timeout) kills the running command.
77-
ctx context.Context
75+
// ctxTool provides SetContext/toolCtx so cancelling the agent context
76+
// (Ctrl-C, turn timeout) kills the running command.
77+
ctxTool
7878

7979
// timeout bounds a single command. Zero falls back to defaultShellTimeout.
8080
timeout time.Duration
8181
}
8282

83-
// SetContext lets the agent loop propagate its context so a running command is
84-
// killed when the turn is cancelled. Implements the loop's optional
85-
// context-aware tool interface.
86-
func (t *shellTool) SetContext(ctx context.Context) { t.ctx = ctx }
87-
8883
func (t *shellTool) Name() string { return "shell" }
8984

9085
func (t *shellTool) Description() string {
@@ -138,10 +133,7 @@ func (t *shellTool) Call(args string) (string, error) {
138133
// Bound execution: cancel with the agent context (Ctrl-C / turn timeout)
139134
// and a generous backstop timeout so a stuck command can never wedge the
140135
// agent forever.
141-
base := t.ctx
142-
if base == nil {
143-
base = context.Background()
144-
}
136+
base := t.toolCtx()
145137
timeout := t.timeout
146138
if timeout <= 0 {
147139
timeout = defaultShellTimeout

cmd/odek/toolctx.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"sync"
6+
)
7+
8+
// ctxTool is embedded by tools that support agent-context cancellation. The
9+
// agent loop calls SetContext on any tool implementing it (see internal/loop)
10+
// right before invoking the tool, so cancelling the agent context — Ctrl-C, a
11+
// turn timeout — interrupts the tool's in-flight network request or subprocess
12+
// instead of letting it run to completion (or hang) unsupervised.
13+
//
14+
// The mutex matters: when the LLM emits two calls to the SAME tool in one
15+
// turn, the loop runs them in parallel goroutines and calls SetContext on the
16+
// shared instance from each. Without synchronisation that is a data race on the
17+
// context field even though the value is identical for the turn.
18+
type ctxTool struct {
19+
mu sync.Mutex
20+
ctx context.Context
21+
}
22+
23+
// SetContext records the agent context for the next Call. Safe for concurrent
24+
// use by parallel invocations of the same tool instance.
25+
func (c *ctxTool) SetContext(ctx context.Context) {
26+
c.mu.Lock()
27+
c.ctx = ctx
28+
c.mu.Unlock()
29+
}
30+
31+
// toolCtx returns the recorded agent context, or context.Background() if none
32+
// was set (e.g. tools invoked directly in tests or outside the agent loop).
33+
func (c *ctxTool) toolCtx() context.Context {
34+
c.mu.Lock()
35+
defer c.mu.Unlock()
36+
if c.ctx == nil {
37+
return context.Background()
38+
}
39+
return c.ctx
40+
}

cmd/odek/toolctx_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"strings"
6+
"sync"
7+
"testing"
8+
)
9+
10+
func TestCtxTool_DefaultsToBackground(t *testing.T) {
11+
var c ctxTool
12+
if c.toolCtx() != context.Background() {
13+
t.Error("unset ctxTool should return context.Background()")
14+
}
15+
}
16+
17+
func TestCtxTool_SetAndGet(t *testing.T) {
18+
var c ctxTool
19+
ctx, cancel := context.WithCancel(context.Background())
20+
defer cancel()
21+
c.SetContext(ctx)
22+
if c.toolCtx() != ctx {
23+
t.Error("toolCtx should return the context set via SetContext")
24+
}
25+
}
26+
27+
// TestCtxTool_ConcurrentSetContext mirrors the loop calling SetContext on a
28+
// shared tool instance from parallel goroutines — it must be race-free.
29+
func TestCtxTool_ConcurrentSetContext(t *testing.T) {
30+
var c ctxTool
31+
ctx := context.Background()
32+
var wg sync.WaitGroup
33+
for i := 0; i < 100; i++ {
34+
wg.Add(1)
35+
go func() {
36+
defer wg.Done()
37+
c.SetContext(ctx)
38+
_ = c.toolCtx()
39+
}()
40+
}
41+
wg.Wait()
42+
}
43+
44+
// TestHTTPBatch_ContextCancelled verifies the propagated context aborts the
45+
// fetch: a cancelled context yields an error entry instead of a real request.
46+
func TestHTTPBatch_ContextCancelled(t *testing.T) {
47+
tool := newHTTPBatchTool(allowAllDanger())
48+
ctx, cancel := context.WithCancel(context.Background())
49+
cancel() // already cancelled
50+
tool.SetContext(ctx)
51+
52+
result := callJSON(t, tool, `{"requests":[{"url":"http://example.com/"}]}`)
53+
var r struct {
54+
Results []struct {
55+
Error string `json:"error"`
56+
} `json:"results"`
57+
}
58+
mustUnmarshal(t, result, &r)
59+
if len(r.Results) != 1 {
60+
t.Fatalf("Results = %d, want 1", len(r.Results))
61+
}
62+
if r.Results[0].Error == "" {
63+
t.Fatal("expected an error from the cancelled context")
64+
}
65+
if !strings.Contains(r.Results[0].Error, "context canceled") {
66+
t.Errorf("error should reflect cancellation, got: %s", r.Results[0].Error)
67+
}
68+
}

cmd/odek/transcribe_tool.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"context"
45
"encoding/json"
56
"fmt"
67
"os"
@@ -20,7 +21,7 @@ import (
2021
// Returns the path to the WAV file (may be the same as input if already WAV/MP3/FLAC
2122
// or if ffmpeg is unavailable/fails — in which case whisper will produce its own error).
2223
// The caller must remove the returned path if it differs from the input path.
23-
func convertToWAV(srcPath string) string {
24+
func convertToWAV(ctx context.Context, srcPath string) string {
2425
ext := strings.ToLower(filepath.Ext(srcPath))
2526
// whisper.cpp supports WAV, MP3, FLAC natively via dr_wav/dr_mp3/dr_flac.
2627
switch ext {
@@ -35,7 +36,7 @@ func convertToWAV(srcPath string) string {
3536

3637
// Convert to WAV using ffmpeg — best-effort, fall through on failure.
3738
dstPath := srcPath + ".wav"
38-
cmd := exec.Command("ffmpeg", "-y", "-i", srcPath, "-acodec", "pcm_s16le", "-ar", "16000", "-ac", "1", dstPath)
39+
cmd := exec.CommandContext(ctx, "ffmpeg", "-y", "-i", srcPath, "-acodec", "pcm_s16le", "-ar", "16000", "-ac", "1", dstPath)
3940
if err := cmd.Run(); err != nil {
4041
// If ffmpeg fails (corrupt file, unsupported codec, etc.),
4142
// just pass the original path — whisper will produce its own error.
@@ -138,6 +139,7 @@ Set "model" in transcription config to change which model is expected.`,
138139
// ═════════════════════════════════════════════════════════════════════════
139140

140141
type transcribeTool struct {
142+
ctxTool
141143
dangerousConfig danger.DangerousConfig
142144
transcriptionCfg config.TranscriptionConfig
143145
}
@@ -224,7 +226,7 @@ func (t *transcribeTool) Call(argsJSON string) (result string, err error) {
224226
f.Close()
225227

226228
// Convert to WAV if needed (whisper.cpp doesn't support OGG Opus natively).
227-
wavPath := convertToWAV(args.Path)
229+
wavPath := convertToWAV(t.toolCtx(), args.Path)
228230
cleanup := func() {
229231
if wavPath != args.Path {
230232
os.Remove(wavPath)
@@ -263,7 +265,7 @@ func (t *transcribeTool) Call(argsJSON string) (result string, err error) {
263265
args2 = append(args2, "--language", lang)
264266
}
265267

266-
cmd := exec.Command(binary, args2...)
268+
cmd := exec.CommandContext(t.toolCtx(), binary, args2...)
267269
output, err := cmd.Output()
268270
if err != nil {
269271
if exitErr, ok := err.(*exec.ExitError); ok {

cmd/odek/vision_tool.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"context"
45
"encoding/json"
56
"fmt"
67
"os"
@@ -89,7 +90,7 @@ Or set models_dir in the vision config.`, mp, dir, dir)
8990
// extractVideoFrames samples n evenly-spaced frames from videoPath into a
9091
// temporary directory. Returns paths to the JPEG frame files; caller must
9192
// remove the directory (filepath.Dir of the first path).
92-
func extractVideoFrames(videoPath string, n int) ([]string, error) {
93+
func extractVideoFrames(ctx context.Context, videoPath string, n int) ([]string, error) {
9394
if _, err := exec.LookPath("ffmpeg"); err != nil {
9495
return nil, fmt.Errorf("ffmpeg not found — required for video frame extraction")
9596
}
@@ -98,7 +99,7 @@ func extractVideoFrames(videoPath string, n int) ([]string, error) {
9899
}
99100

100101
// Get duration with ffprobe
101-
out, err := exec.Command("ffprobe",
102+
out, err := exec.CommandContext(ctx, "ffprobe",
102103
"-v", "error",
103104
"-show_entries", "format=duration",
104105
"-of", "csv=p=0",
@@ -124,7 +125,7 @@ func extractVideoFrames(videoPath string, n int) ([]string, error) {
124125
for i := 1; i <= n; i++ {
125126
ts := interval * float64(i)
126127
out := filepath.Join(tmpDir, fmt.Sprintf("frame_%02d.jpg", i))
127-
cmd := exec.Command("ffmpeg",
128+
cmd := exec.CommandContext(ctx, "ffmpeg",
128129
"-ss", fmt.Sprintf("%.3f", ts),
129130
"-i", videoPath,
130131
"-frames:v", "1",
@@ -146,7 +147,7 @@ func extractVideoFrames(videoPath string, n int) ([]string, error) {
146147

147148
// runLlamaMtmd calls llama-mtmd-cli in single-turn mode with one or more images
148149
// and returns the trimmed stdout response.
149-
func runLlamaMtmd(binary, modelPath, mmprojPath, prompt string, imagePaths []string) (string, error) {
150+
func runLlamaMtmd(ctx context.Context, binary, modelPath, mmprojPath, prompt string, imagePaths []string) (string, error) {
150151
args := []string{
151152
"-m", modelPath,
152153
"--mmproj", mmprojPath,
@@ -162,7 +163,7 @@ func runLlamaMtmd(binary, modelPath, mmprojPath, prompt string, imagePaths []str
162163
args = append(args, "--image", img)
163164
}
164165

165-
cmd := exec.Command(binary, args...)
166+
cmd := exec.CommandContext(ctx, binary, args...)
166167
output, err := cmd.Output()
167168
if err != nil {
168169
if exitErr, ok := err.(*exec.ExitError); ok {
@@ -179,6 +180,7 @@ func runLlamaMtmd(binary, modelPath, mmprojPath, prompt string, imagePaths []str
179180
// ═════════════════════════════════════════════════════════════════════════
180181

181182
type visionTool struct {
183+
ctxTool
182184
dangerousConfig danger.DangerousConfig
183185
visionCfg config.VisionConfig
184186
}
@@ -277,7 +279,7 @@ func (t *visionTool) Call(argsJSON string) (result string, err error) {
277279
}
278280

279281
func (t *visionTool) analyzeImage(binary, modelPath, mmprojPath, imgPath, prompt, source string) (string, error) {
280-
desc, err := runLlamaMtmd(binary, modelPath, mmprojPath, prompt, []string{imgPath})
282+
desc, err := runLlamaMtmd(t.toolCtx(), binary, modelPath, mmprojPath, prompt, []string{imgPath})
281283
if err != nil {
282284
return jsonResult(visionResult{Error: err.Error()})
283285
}
@@ -294,7 +296,7 @@ func (t *visionTool) analyzeVideo(binary, modelPath, mmprojPath, videoPath, prom
294296
n = 8
295297
}
296298

297-
frames, err := extractVideoFrames(videoPath, n)
299+
frames, err := extractVideoFrames(t.toolCtx(), videoPath, n)
298300
if err != nil {
299301
return jsonResult(visionResult{Error: err.Error()})
300302
}
@@ -304,7 +306,7 @@ func (t *visionTool) analyzeVideo(binary, modelPath, mmprojPath, videoPath, prom
304306
"These are %d frames sampled evenly from a video. %s",
305307
len(frames), prompt,
306308
)
307-
desc, err := runLlamaMtmd(binary, modelPath, mmprojPath, videoPrompt, frames)
309+
desc, err := runLlamaMtmd(t.toolCtx(), binary, modelPath, mmprojPath, videoPrompt, frames)
308310
if err != nil {
309311
return jsonResult(visionResult{Error: err.Error()})
310312
}

cmd/odek/web_search_tool.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ var (
3434
// ═════════════════════════════════════════════════════════════════════════
3535

3636
type webSearchTool struct {
37+
ctxTool
3738
dangerousConfig danger.DangerousConfig
3839
cfg config.WebSearchConfig
3940
client *http.Client
@@ -235,7 +236,7 @@ func (t *webSearchTool) query(query, category string) (*searxngResponse, error)
235236
}
236237
endpoint.RawQuery = q.Encode()
237238

238-
req, err := http.NewRequest(http.MethodGet, endpoint.String(), nil)
239+
req, err := http.NewRequestWithContext(t.toolCtx(), http.MethodGet, endpoint.String(), nil)
239240
if err != nil {
240241
return nil, fmt.Errorf("build request: %v", err)
241242
}

0 commit comments

Comments
 (0)