From c7e9371501b09b20b1017162f99354eab46a5774 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 31 Aug 2026 20:22:58 +0530 Subject: [PATCH] fix(skills): stop walking the whole tree to answer "does any file match" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `applyPaths` auto-load asks one question per skill — does at least one file in the worktree match this glob — and answered it with `Glob.scan(...).length > 0`. `scan` resolves only once the entire walk has finished, so every skill paid for the full tree even when the first directory already answered. Two builtin skills ship with `applyPaths` (`dbt-develop`, `dbt-schema-verify`), so every session pays this twice, before the first token, with no configuration and no workspace involved. The cost depends entirely on what the worktree resolves to. Inside a git repo it is the repo root and the scans take ~10ms. Outside one, `Project.fromDirectory` returns the global project whose worktree is `/`, and the two scans walk the entire filesystem. Measured from a directory outside a git repo, same binary, same prompt: scan(...).length > 0 48.4 52.9 51.6 s Glob.exists (this change) 9.1 6.8 6.4 s scan removed entirely 7.1 6.8 6.7 s (floor) Inside a repo: 7.5 / 7.3s, unchanged. `Glob.exists` uses `globIterate`, which yields lazily, so the walk is abandoned at the first match. It takes the same options as `scan` — the tests pin `include`, `ignore` and the missing-directory case, and fail if the options stop being forwarded. Also tried and rejected: passing `ignore: Glob.DEFAULT_IGNORE` here, the way #1184 did for the MCP scans. It made this *slower* — 61-82s against a ~51s baseline — because outside a repo the tree is not dependency-heavy, so every candidate path pays 12 minimatch tests and almost nothing gets pruned. Early exit is the right lever for an existence check. Not addressed here: the worktree being `/` outside a git repo. That makes skills auto-load off unrelated files elsewhere on the machine, which is a correctness question for whoever owns project identity. core suite: 1072 pass, 26 fail — the same 26 fail on unmodified main. session/skill suites: 1516 pass, 0 fail. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018fJ3X7pcGT4R9yzjsJnqsV --- packages/core/src/util/glob.ts | 12 +++++++- packages/core/test/util/glob.test.ts | 40 +++++++++++++++++++++++++ packages/opencode/src/session/system.ts | 23 +++++++++----- 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/packages/core/src/util/glob.ts b/packages/core/src/util/glob.ts index cd73b40ff..80878bb9a 100644 --- a/packages/core/src/util/glob.ts +++ b/packages/core/src/util/glob.ts @@ -1,4 +1,4 @@ -import { glob, globSync, type GlobOptions } from "glob" +import { glob, globSync, globIterate, type GlobOptions } from "glob" import { minimatch } from "minimatch" export namespace Glob { @@ -62,6 +62,16 @@ export namespace Glob { } } + // altimate_change start — upstream_fix: existence check that stops at the first match. + // `scan` resolves only once the whole walk is done, so a caller asking "does anything + // match?" pays for the entire tree even when the first directory answers it. `globIterate` + // yields lazily, so this abandons the walk as soon as one path matches. + export async function exists(pattern: string, options: Options = {}): Promise { + for await (const _ of globIterate(pattern, toGlobOptions(options))) return true + return false + } + // altimate_change end + export async function scan(pattern: string, options: Options = {}): Promise { return glob(pattern, toGlobOptions(options)) as Promise } diff --git a/packages/core/test/util/glob.test.ts b/packages/core/test/util/glob.test.ts index d305552b1..c2e8ac346 100644 --- a/packages/core/test/util/glob.test.ts +++ b/packages/core/test/util/glob.test.ts @@ -121,3 +121,43 @@ describe("Glob.DEFAULT_IGNORE", () => { }) }) // altimate_change end + +// altimate_change start — upstream_fix: `exists` must answer without walking the whole tree. +describe("Glob.exists", () => { + test("agrees with scan() on whether anything matched", async () => { + for (const pattern of ["**/*.ts", "**/mcp.json", "**/nothing-matches-this.xyz"]) { + const scanned = await Glob.scan(pattern, { cwd: root, absolute: true }) + const existed = await Glob.exists(pattern, { cwd: root, absolute: true }) + expect(existed, `pattern ${pattern}`).toBe(scanned.length > 0) + } + }) + + test("honours the same options as scan", async () => { + // `include: "file"` must not report a directory match, or a skill whose applyPaths names + // a directory would auto-load on every project that happens to have one. + const dirOnly = await Glob.exists("src", { cwd: root, include: "file" }) + const withDirs = await Glob.exists("src", { cwd: root, include: "all" }) + expect(dirOnly).toBe(false) + expect(withDirs).toBe(true) + }) + + test("prunes with ignore, like scan", async () => { + // Own fixture: the shared `root` has matching files outside the ignored trees too, which + // would make this pass for the wrong reason. + const own = await mkdtemp(path.join(tmpdir(), "glob-exists-")) + try { + await mkdir(path.join(own, "node_modules", "pkg"), { recursive: true }) + await writeFile(path.join(own, "node_modules", "pkg", "only-here.json"), "{}") + + expect(await Glob.exists("**/only-here.json", { cwd: own })).toBe(true) + expect(await Glob.exists("**/only-here.json", { cwd: own, ignore: ["**/node_modules/**"] })).toBe(false) + } finally { + await rm(own, { recursive: true, force: true }) + } + }) + + test("returns false for a directory that does not exist", async () => { + expect(await Glob.exists("**/*", { cwd: path.join(root, "no-such-dir") })).toBe(false) + }) +}) +// altimate_change end diff --git a/packages/opencode/src/session/system.ts b/packages/opencode/src/session/system.ts index 6e035616c..da9692c02 100644 --- a/packages/opencode/src/session/system.ts +++ b/packages/opencode/src/session/system.ts @@ -246,16 +246,23 @@ export namespace SystemPrompt { // catches the file no matter how deep the user's cwd is. // Errors propagate to the caller's try/catch (collectAutoLoadedSkills) // so the warning log there actually fires. + // `Glob.exists` rather than `scan(...).length > 0`: this only needs to know whether any + // file matches, and `scan` walks the whole tree before the caller can look. That cost is + // paid once per `applyPaths` skill — two ship builtin — and the root is the worktree, which + // is `/` for a directory outside any git repo. Measured from such a directory, the two + // scans were ~45s of a ~51s startup, all of it before the first token. const root = Instance.worktree for (const g of globs) { - const matches = await Glob.scan(g, { - cwd: root, - absolute: true, - include: "file", - dot: false, - symlink: false, - }) - if (matches.length > 0) return true + if ( + await Glob.exists(g, { + cwd: root, + absolute: true, + include: "file", + dot: false, + symlink: false, + }) + ) + return true } return false }