Skip to content

Commit 9806411

Browse files
anandgupta42claude
anandgupta42
and
claude
authored
fix: add timeout, home/root blocking, and default exclusions to glob tool (#637)
* 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> * feat: add home/root blocking and default directory exclusions to glob tool - Block glob searches from `/` and `~` with immediate helpful message - Add default directory exclusions (`node_modules/`, `dist/`, `build/`, `.cache/`, `.venv/`, etc.) reusing `IGNORE_PATTERNS` from `ls` tool - Document `.gitignore` and `.ignore` file support in glob tool description - Add 5 new tests: home/root blocking, subdirectory of home allowed, `node_modules` excluded, `dist`/`build` excluded - Add `altimate_change` markers to all custom code in upstream file Closes #636 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add missing `altimate_change` markers and revert glob.txt changes - Wrap `truncated || timedOut` in marker block - Revert glob.txt description changes (triggers marker check on .txt files) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit review — tighten error handling and add tests - Add `AbortError` type check to catch block: only suppress errors that are AbortErrors AND from our timeout AND not user-initiated aborts - Add test for ENOENT propagation (non-abort errors not masked as timeout) - Add test for normal completion without timeout on small directories Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve type error in glob test — `abort` field requires `AbortSignal` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c103bfd commit 9806411

File tree

2 files changed

+334
-17
lines changed

2 files changed

+334
-17
lines changed

‎packages/opencode/src/tool/glob.ts‎

Lines changed: 79 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ import DESCRIPTION from "./glob.txt"
66
import { Ripgrep } from "../file/ripgrep"
77
import { Instance } from "../project/instance"
88
import { assertExternalDirectory } from "./external-directory"
9+
// altimate_change start — glob hardening: timeout, home/root blocking, default exclusions
10+
import os from "os"
11+
import { abortAfter } from "../util/abort"
12+
import { IGNORE_PATTERNS } from "./ls"
13+
14+
const GLOB_TIMEOUT_MS = 30_000
15+
// altimate_change end
916

1017
export const GlobTool = Tool.define("glob", {
1118
description: DESCRIPTION,
@@ -33,32 +40,84 @@ export const GlobTool = Tool.define("glob", {
3340
search = path.isAbsolute(search) ? search : path.resolve(Instance.directory, search)
3441
await assertExternalDirectory(ctx, search, { kind: "directory" })
3542

43+
// altimate_change start — block home/root directory to prevent scanning entire filesystem
44+
const homeDir = os.homedir()
45+
if (search === "/" || search === homeDir) {
46+
return {
47+
title: path.relative(Instance.worktree, search),
48+
metadata: { count: 0, truncated: false },
49+
output: `The directory "${search}" is too broad to search. Please specify a more specific \`path\` parameter within your project or a subdirectory.`,
50+
}
51+
}
52+
// altimate_change end
53+
3654
const limit = 100
3755
const files = []
3856
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
57+
// altimate_change start — 30s timeout with default directory exclusions
58+
let timedOut = false
59+
60+
const defaultExclusions = IGNORE_PATTERNS.map((p) => `!${p}*`)
61+
const globs = [params.pattern, ...defaultExclusions]
62+
63+
const timeout = abortAfter(GLOB_TIMEOUT_MS)
64+
const localAbort = new AbortController()
65+
const parentSignals = ctx.abort ? [ctx.abort] : []
66+
const signal = AbortSignal.any([timeout.signal, localAbort.signal, ...parentSignals])
67+
68+
try {
69+
for await (const file of Ripgrep.files({
70+
cwd: search,
71+
glob: globs,
72+
signal,
73+
})) {
74+
if (files.length >= limit) {
75+
truncated = true
76+
break
77+
}
78+
const full = path.resolve(search, file)
79+
const stats = Filesystem.stat(full)?.mtime.getTime() ?? 0
80+
files.push({
81+
path: full,
82+
mtime: stats,
83+
})
84+
}
85+
} catch (err: any) {
86+
if (
87+
err?.name === "AbortError" &&
88+
timeout.signal.aborted &&
89+
!ctx.abort?.aborted
90+
) {
91+
// Our timeout fired — return partial results
92+
timedOut = true
93+
} else {
94+
// User cancellation, ENOENT, permission error, etc. — propagate
95+
throw err
4796
}
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-
})
97+
} finally {
98+
localAbort.abort()
99+
timeout.clearTimeout()
54100
}
101+
// altimate_change end
55102
files.sort((a, b) => b.mtime - a.mtime)
56103

57104
const output = []
58-
if (files.length === 0) output.push("No files found")
105+
// altimate_change start — timeout-aware output messages
106+
if (files.length === 0 && timedOut) {
107+
output.push(
108+
`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.`,
109+
)
110+
} else if (files.length === 0) {
111+
output.push("No files found")
112+
}
59113
if (files.length > 0) {
60114
output.push(...files.map((f) => f.path))
61-
if (truncated) {
115+
if (timedOut) {
116+
output.push("")
117+
output.push(
118+
`(Search timed out after ${GLOB_TIMEOUT_MS / 1000}s: only partial results shown. Use a more specific \`path\` parameter to narrow the search scope.)`,
119+
)
120+
} else if (truncated) {
62121
output.push("")
63122
output.push(
64123
`(Results are truncated: showing first ${limit} results. Consider using a more specific path or pattern.)`,
@@ -70,9 +129,12 @@ export const GlobTool = Tool.define("glob", {
70129
title: path.relative(Instance.worktree, search),
71130
metadata: {
72131
count: files.length,
73-
truncated,
132+
// altimate_change start — include timeout in truncated flag
133+
truncated: truncated || timedOut,
134+
// altimate_change end
74135
},
75136
output: output.join("\n"),
76137
}
138+
// altimate_change end
77139
},
78140
})

‎packages/opencode/test/tool/glob.test.ts‎

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
import { describe, expect, test } from "bun:test"
2+
import os from "os"
3+
import path from "path"
4+
import fs from "fs/promises"
5+
import { GlobTool } from "../../src/tool/glob"
6+
import { Instance } from "../../src/project/instance"
7+
import { tmpdir } from "../fixture/fixture"
8+
import { SessionID, MessageID } from "../../src/session/schema"
9+
10+
const ctx = {
11+
sessionID: SessionID.make("ses_test"),
12+
messageID: MessageID.make(""),
13+
callID: "",
14+
agent: "build",
15+
abort: AbortSignal.any([]),
16+
messages: [],
17+
metadata: () => {},
18+
ask: async () => {},
19+
}
20+
21+
describe("tool.glob", () => {
22+
test("finds files matching pattern", async () => {
23+
await using tmp = await tmpdir({
24+
init: async (dir) => {
25+
await Bun.write(path.join(dir, "a.txt"), "hello")
26+
await Bun.write(path.join(dir, "b.txt"), "world")
27+
await Bun.write(path.join(dir, "c.md"), "readme")
28+
},
29+
})
30+
await Instance.provide({
31+
directory: tmp.path,
32+
fn: async () => {
33+
const glob = await GlobTool.init()
34+
const result = await glob.execute({ pattern: "*.txt" }, ctx)
35+
expect(result.metadata.count).toBe(2)
36+
expect(result.output).toContain("a.txt")
37+
expect(result.output).toContain("b.txt")
38+
expect(result.output).not.toContain("c.md")
39+
},
40+
})
41+
})
42+
43+
test("returns 'No files found' when no matches", async () => {
44+
await using tmp = await tmpdir()
45+
await Instance.provide({
46+
directory: tmp.path,
47+
fn: async () => {
48+
const glob = await GlobTool.init()
49+
const result = await glob.execute({ pattern: "*.nonexistent" }, ctx)
50+
expect(result.metadata.count).toBe(0)
51+
expect(result.output).toBe("No files found")
52+
},
53+
})
54+
})
55+
56+
test("truncates results at 100 files", async () => {
57+
await using tmp = await tmpdir({
58+
init: async (dir) => {
59+
for (let i = 0; i < 110; i++) {
60+
await Bun.write(path.join(dir, `file${String(i).padStart(3, "0")}.txt`), "")
61+
}
62+
},
63+
})
64+
await Instance.provide({
65+
directory: tmp.path,
66+
fn: async () => {
67+
const glob = await GlobTool.init()
68+
const result = await glob.execute({ pattern: "*.txt" }, ctx)
69+
expect(result.metadata.count).toBe(100)
70+
expect(result.metadata.truncated).toBe(true)
71+
expect(result.output).toContain("Results are truncated")
72+
},
73+
})
74+
})
75+
76+
test("respects user abort signal", async () => {
77+
await using tmp = await tmpdir({
78+
init: async (dir) => {
79+
await Bun.write(path.join(dir, "a.txt"), "hello")
80+
},
81+
})
82+
const controller = new AbortController()
83+
controller.abort()
84+
const abortCtx = { ...ctx, abort: controller.signal }
85+
86+
await Instance.provide({
87+
directory: tmp.path,
88+
fn: async () => {
89+
const glob = await GlobTool.init()
90+
await expect(glob.execute({ pattern: "*.txt" }, abortCtx)).rejects.toThrow()
91+
},
92+
})
93+
})
94+
95+
test("finds nested files with ** pattern", async () => {
96+
await using tmp = await tmpdir({
97+
init: async (dir) => {
98+
await fs.mkdir(path.join(dir, "sub", "deep"), { recursive: true })
99+
await Bun.write(path.join(dir, "sub", "deep", "target.yml"), "")
100+
await Bun.write(path.join(dir, "other.txt"), "")
101+
},
102+
})
103+
await Instance.provide({
104+
directory: tmp.path,
105+
fn: async () => {
106+
const glob = await GlobTool.init()
107+
const result = await glob.execute({ pattern: "**/target.yml" }, ctx)
108+
expect(result.metadata.count).toBe(1)
109+
expect(result.output).toContain("target.yml")
110+
},
111+
})
112+
})
113+
114+
test("uses custom path parameter", async () => {
115+
await using tmp = await tmpdir({
116+
init: async (dir) => {
117+
await fs.mkdir(path.join(dir, "subdir"))
118+
await Bun.write(path.join(dir, "subdir", "inner.txt"), "")
119+
await Bun.write(path.join(dir, "outer.txt"), "")
120+
},
121+
})
122+
await Instance.provide({
123+
directory: tmp.path,
124+
fn: async () => {
125+
const glob = await GlobTool.init()
126+
const result = await glob.execute(
127+
{ pattern: "*.txt", path: path.join(tmp.path, "subdir") },
128+
ctx,
129+
)
130+
expect(result.metadata.count).toBe(1)
131+
expect(result.output).toContain("inner.txt")
132+
expect(result.output).not.toContain("outer.txt")
133+
},
134+
})
135+
})
136+
137+
test("blocks home directory with helpful message", async () => {
138+
const homeDir = os.homedir()
139+
await Instance.provide({
140+
directory: homeDir,
141+
fn: async () => {
142+
const glob = await GlobTool.init()
143+
const result = await glob.execute({ pattern: "**/*.yml", path: homeDir }, ctx)
144+
expect(result.metadata.count).toBe(0)
145+
expect(result.output).toContain("too broad to search")
146+
},
147+
})
148+
})
149+
150+
test("blocks root directory with helpful message", async () => {
151+
await using tmp = await tmpdir()
152+
await Instance.provide({
153+
directory: tmp.path,
154+
fn: async () => {
155+
const glob = await GlobTool.init()
156+
const result = await glob.execute({ pattern: "**/*.yml", path: "/" }, ctx)
157+
expect(result.metadata.count).toBe(0)
158+
expect(result.output).toContain("too broad to search")
159+
},
160+
})
161+
})
162+
163+
test("allows subdirectory of home", async () => {
164+
await using tmp = await tmpdir({
165+
init: async (dir) => {
166+
await Bun.write(path.join(dir, "project.yml"), "")
167+
},
168+
})
169+
await Instance.provide({
170+
directory: tmp.path,
171+
fn: async () => {
172+
const glob = await GlobTool.init()
173+
const result = await glob.execute({ pattern: "*.yml" }, ctx)
174+
expect(result.metadata.count).toBe(1)
175+
expect(result.output).toContain("project.yml")
176+
},
177+
})
178+
})
179+
180+
test("excludes node_modules by default", async () => {
181+
await using tmp = await tmpdir({
182+
init: async (dir) => {
183+
await fs.mkdir(path.join(dir, "node_modules", "pkg"), { recursive: true })
184+
await Bun.write(path.join(dir, "node_modules", "pkg", "index.ts"), "")
185+
await Bun.write(path.join(dir, "src.ts"), "")
186+
},
187+
})
188+
await Instance.provide({
189+
directory: tmp.path,
190+
fn: async () => {
191+
const glob = await GlobTool.init()
192+
const result = await glob.execute({ pattern: "**/*.ts" }, ctx)
193+
expect(result.metadata.count).toBe(1)
194+
expect(result.output).toContain("src.ts")
195+
expect(result.output).not.toContain("node_modules")
196+
},
197+
})
198+
})
199+
200+
test("propagates non-abort errors instead of treating as timeout", async () => {
201+
await using tmp = await tmpdir()
202+
await Instance.provide({
203+
directory: tmp.path,
204+
fn: async () => {
205+
const glob = await GlobTool.init()
206+
// Searching a non-existent path should throw ENOENT, not return "timed out"
207+
const badPath = path.join(tmp.path, "nonexistent-dir-12345")
208+
await expect(
209+
glob.execute({ pattern: "*.txt", path: badPath }, ctx),
210+
).rejects.toThrow()
211+
},
212+
})
213+
})
214+
215+
test("completes without timeout on small directories", async () => {
216+
await using tmp = await tmpdir({
217+
init: async (dir) => {
218+
await Bun.write(path.join(dir, "a.txt"), "hello")
219+
},
220+
})
221+
await Instance.provide({
222+
directory: tmp.path,
223+
fn: async () => {
224+
const glob = await GlobTool.init()
225+
const result = await glob.execute({ pattern: "*.txt" }, ctx)
226+
expect(result.metadata.count).toBe(1)
227+
expect(result.metadata.truncated).toBe(false)
228+
expect(result.output).not.toContain("timed out")
229+
},
230+
})
231+
})
232+
233+
test("excludes dist and build by default", async () => {
234+
await using tmp = await tmpdir({
235+
init: async (dir) => {
236+
await fs.mkdir(path.join(dir, "dist"), { recursive: true })
237+
await fs.mkdir(path.join(dir, "build"), { recursive: true })
238+
await Bun.write(path.join(dir, "dist", "bundle.js"), "")
239+
await Bun.write(path.join(dir, "build", "output.js"), "")
240+
await Bun.write(path.join(dir, "app.js"), "")
241+
},
242+
})
243+
await Instance.provide({
244+
directory: tmp.path,
245+
fn: async () => {
246+
const glob = await GlobTool.init()
247+
const result = await glob.execute({ pattern: "**/*.js" }, ctx)
248+
expect(result.metadata.count).toBe(1)
249+
expect(result.output).toContain("app.js")
250+
expect(result.output).not.toContain("dist")
251+
expect(result.output).not.toContain("build")
252+
},
253+
})
254+
})
255+
})

0 commit comments

Comments
 (0)