Skip to content

Commit f09ecb2

Browse files
anandgupta42claude
andcommitted
fix: add 30s timeout to glob tool to prevent indefinite hangs (#636)
- Add `abortAfter` timeout that kills the `rg` process after 30 seconds - Return partial results on timeout with actionable guidance message - Fix error handling: only treat `AbortError` from timeout signal as timeout — properly re-throw ENOENT, permission, and user abort errors - Add `localAbort` controller to kill `rg` process on early loop exit (e.g., 100-file limit hit), preventing background resource waste - Add comprehensive glob tool tests (6 tests covering basic matching, truncation, user abort, nested patterns, and custom path) Closes #636 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 31d163e commit f09ecb2

2 files changed

Lines changed: 186 additions & 17 deletions

File tree

packages/opencode/src/tool/glob.ts

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import DESCRIPTION from "./glob.txt"
66
import { Ripgrep } from "../file/ripgrep"
77
import { Instance } from "../project/instance"
88
import { assertExternalDirectory } from "./external-directory"
9+
import { abortAfter } from "../util/abort"
10+
11+
const GLOB_TIMEOUT_MS = 30_000
912

1013
export const GlobTool = Tool.define("glob", {
1114
description: DESCRIPTION,
@@ -36,29 +39,60 @@ export const GlobTool = Tool.define("glob", {
3639
const limit = 100
3740
const files = []
3841
let truncated = false
39-
for await (const file of Ripgrep.files({
40-
cwd: search,
41-
glob: [params.pattern],
42-
signal: ctx.abort,
43-
})) {
44-
if (files.length >= limit) {
45-
truncated = true
46-
break
42+
let timedOut = false
43+
44+
const timeout = abortAfter(GLOB_TIMEOUT_MS)
45+
const localAbort = new AbortController()
46+
const parentSignals = ctx.abort ? [ctx.abort] : []
47+
const signal = AbortSignal.any([timeout.signal, localAbort.signal, ...parentSignals])
48+
49+
try {
50+
for await (const file of Ripgrep.files({
51+
cwd: search,
52+
glob: [params.pattern],
53+
signal,
54+
})) {
55+
if (files.length >= limit) {
56+
truncated = true
57+
break
58+
}
59+
const full = path.resolve(search, file)
60+
const stats = Filesystem.stat(full)?.mtime.getTime() ?? 0
61+
files.push({
62+
path: full,
63+
mtime: stats,
64+
})
65+
}
66+
} catch (err: any) {
67+
if (timeout.signal.aborted) {
68+
// Our timeout fired — return partial results
69+
timedOut = true
70+
} else {
71+
// User cancellation, ENOENT, permission error, etc. — propagate
72+
throw err
4773
}
48-
const full = path.resolve(search, file)
49-
const stats = Filesystem.stat(full)?.mtime.getTime() ?? 0
50-
files.push({
51-
path: full,
52-
mtime: stats,
53-
})
74+
} finally {
75+
localAbort.abort()
76+
timeout.clearTimeout()
5477
}
5578
files.sort((a, b) => b.mtime - a.mtime)
5679

5780
const output = []
58-
if (files.length === 0) output.push("No files found")
81+
if (files.length === 0 && timedOut) {
82+
output.push(
83+
`Glob search timed out after ${GLOB_TIMEOUT_MS / 1000}s with no results. The search directory "${search}" is too broad for the pattern "${params.pattern}". Use a more specific \`path\` parameter to narrow the search scope.`,
84+
)
85+
} else if (files.length === 0) {
86+
output.push("No files found")
87+
}
5988
if (files.length > 0) {
6089
output.push(...files.map((f) => f.path))
61-
if (truncated) {
90+
if (timedOut) {
91+
output.push("")
92+
output.push(
93+
`(Search timed out after ${GLOB_TIMEOUT_MS / 1000}s: only partial results shown. Use a more specific \`path\` parameter to narrow the search scope.)`,
94+
)
95+
} else if (truncated) {
6296
output.push("")
6397
output.push(
6498
`(Results are truncated: showing first ${limit} results. Consider using a more specific path or pattern.)`,
@@ -70,7 +104,7 @@ export const GlobTool = Tool.define("glob", {
70104
title: path.relative(Instance.worktree, search),
71105
metadata: {
72106
count: files.length,
73-
truncated,
107+
truncated: truncated || timedOut,
74108
},
75109
output: output.join("\n"),
76110
}
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { describe, expect, test } from "bun:test"
2+
import path from "path"
3+
import fs from "fs/promises"
4+
import { GlobTool } from "../../src/tool/glob"
5+
import { Instance } from "../../src/project/instance"
6+
import { tmpdir } from "../fixture/fixture"
7+
import { SessionID, MessageID } from "../../src/session/schema"
8+
9+
const ctx = {
10+
sessionID: SessionID.make("ses_test"),
11+
messageID: MessageID.make(""),
12+
callID: "",
13+
agent: "build",
14+
abort: AbortSignal.any([]),
15+
messages: [],
16+
metadata: () => {},
17+
ask: async () => {},
18+
}
19+
20+
describe("tool.glob", () => {
21+
test("finds files matching pattern", async () => {
22+
await using tmp = await tmpdir({
23+
init: async (dir) => {
24+
await Bun.write(path.join(dir, "a.txt"), "hello")
25+
await Bun.write(path.join(dir, "b.txt"), "world")
26+
await Bun.write(path.join(dir, "c.md"), "readme")
27+
},
28+
})
29+
await Instance.provide({
30+
directory: tmp.path,
31+
fn: async () => {
32+
const glob = await GlobTool.init()
33+
const result = await glob.execute({ pattern: "*.txt" }, ctx)
34+
expect(result.metadata.count).toBe(2)
35+
expect(result.output).toContain("a.txt")
36+
expect(result.output).toContain("b.txt")
37+
expect(result.output).not.toContain("c.md")
38+
},
39+
})
40+
})
41+
42+
test("returns 'No files found' when no matches", async () => {
43+
await using tmp = await tmpdir()
44+
await Instance.provide({
45+
directory: tmp.path,
46+
fn: async () => {
47+
const glob = await GlobTool.init()
48+
const result = await glob.execute({ pattern: "*.nonexistent" }, ctx)
49+
expect(result.metadata.count).toBe(0)
50+
expect(result.output).toBe("No files found")
51+
},
52+
})
53+
})
54+
55+
test("truncates results at 100 files", async () => {
56+
await using tmp = await tmpdir({
57+
init: async (dir) => {
58+
for (let i = 0; i < 110; i++) {
59+
await Bun.write(path.join(dir, `file${String(i).padStart(3, "0")}.txt`), "")
60+
}
61+
},
62+
})
63+
await Instance.provide({
64+
directory: tmp.path,
65+
fn: async () => {
66+
const glob = await GlobTool.init()
67+
const result = await glob.execute({ pattern: "*.txt" }, ctx)
68+
expect(result.metadata.count).toBe(100)
69+
expect(result.metadata.truncated).toBe(true)
70+
expect(result.output).toContain("Results are truncated")
71+
},
72+
})
73+
})
74+
75+
test("respects user abort signal", async () => {
76+
await using tmp = await tmpdir({
77+
init: async (dir) => {
78+
await Bun.write(path.join(dir, "a.txt"), "hello")
79+
},
80+
})
81+
const controller = new AbortController()
82+
controller.abort()
83+
const abortCtx = { ...ctx, abort: controller.signal }
84+
85+
await Instance.provide({
86+
directory: tmp.path,
87+
fn: async () => {
88+
const glob = await GlobTool.init()
89+
await expect(glob.execute({ pattern: "*.txt" }, abortCtx)).rejects.toThrow()
90+
},
91+
})
92+
})
93+
94+
test("finds nested files with ** pattern", async () => {
95+
await using tmp = await tmpdir({
96+
init: async (dir) => {
97+
await fs.mkdir(path.join(dir, "sub", "deep"), { recursive: true })
98+
await Bun.write(path.join(dir, "sub", "deep", "target.yml"), "")
99+
await Bun.write(path.join(dir, "other.txt"), "")
100+
},
101+
})
102+
await Instance.provide({
103+
directory: tmp.path,
104+
fn: async () => {
105+
const glob = await GlobTool.init()
106+
const result = await glob.execute({ pattern: "**/target.yml" }, ctx)
107+
expect(result.metadata.count).toBe(1)
108+
expect(result.output).toContain("target.yml")
109+
},
110+
})
111+
})
112+
113+
test("uses custom path parameter", async () => {
114+
await using tmp = await tmpdir({
115+
init: async (dir) => {
116+
await fs.mkdir(path.join(dir, "subdir"))
117+
await Bun.write(path.join(dir, "subdir", "inner.txt"), "")
118+
await Bun.write(path.join(dir, "outer.txt"), "")
119+
},
120+
})
121+
await Instance.provide({
122+
directory: tmp.path,
123+
fn: async () => {
124+
const glob = await GlobTool.init()
125+
const result = await glob.execute(
126+
{ pattern: "*.txt", path: path.join(tmp.path, "subdir") },
127+
ctx,
128+
)
129+
expect(result.metadata.count).toBe(1)
130+
expect(result.output).toContain("inner.txt")
131+
expect(result.output).not.toContain("outer.txt")
132+
},
133+
})
134+
})
135+
})

0 commit comments

Comments
 (0)