From 7d8f97159826dd397663c1615c2ed87c111f0366 Mon Sep 17 00:00:00 2001 From: Quickbeard Date: Fri, 31 Jul 2026 18:05:18 +0700 Subject: [PATCH] fix(core): make Windows file-search fallback resilient On Windows fff is disabled by default, so the @ mention popup and file search depend entirely on the ripgrep fallback. A missing rg.exe plus one failed download left the index permanently empty with no signal: the background walk died silently (Effect.orDie in a forked fiber) and Effect.cached memoized the failed binary resolution for the process lifetime. - Retry the index walk forever (1s exponential backoff capped at 30s), log each failure, and reset partial state per attempt so a mid-session ripgrep install self-heals without duplicating entries. - Cache only successful ripgrep binary resolution; retry failed downloads on next use. - Suffix and detect directory entries with "/" instead of path.sep in search, fs.list, and the Read tool listing, and "/"-normalize path.relative output, so entry paths are "/"-normalized on every platform (fixes endsWith("/") directory-mime detection and tab drill-down on Windows). - Normalize "\" and doubled slashes in fallback find queries; they can never match the "/"-normalized index. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 16 ++++ DIVERGENCES.md | 1 + packages/core/src/filesystem.ts | 6 +- packages/core/src/filesystem/search.ts | 32 ++++++-- packages/core/src/ripgrep/binary.ts | 69 +++++++++-------- packages/core/src/tool/read-filesystem.ts | 2 +- packages/core/test/filesystem/search.test.ts | 75 +++++++++++++++++-- .../core/test/location-filesystem.test.ts | 2 +- 8 files changed, 153 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf2a616725..e38188c721 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ file is the human-readable summary of what changed and why. ## Unreleased +### Core + +- **`@` file mentions work on Windows** — on Windows the `@` popup is powered + by a ripgrep-based fallback (the native fuzzy finder is disabled there), and + that fallback could end up permanently empty with no error shown: if + `rg.exe` was missing and the one-time automatic download failed (offline, + blocked proxy), the failure was silently cached for the whole session and + every `@` query showed "No matching items". The file index scan now logs its + failures and keeps retrying (so installing ripgrep mid-session starts + working without a restart), and a failed download is retried on next use. + Directory results also no longer carry a trailing `\` on Windows — they are + `/`-suffixed on every platform, which fixes drilling into folders with tab + (the old mixed-separator paths produced queries that could never match) and + directory-mention detection in `codev run`. Queries containing `\` are now + matched as `/`. + ### TUI - **`ctrl+v` pastes into dialog prompts** — dialog text fields (the API key and diff --git a/DIVERGENCES.md b/DIVERGENCES.md index 214d9b9511..deab0678de 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -167,6 +167,7 @@ from the squash message first, or use a plain merge. | `packages/tui/src/component/prompt/autocomplete.tsx`, `packages/app/src/components/prompt-input.tsx`, `packages/opencode/src/cli/cmd/run/footer.prompt.tsx`, `packages/core/src/filesystem/search.ts`, `packages/tui/test/fixture/tui-sdk.ts`, `packages/opencode/test/cli/run/footer.view.test.tsx`, `packages/core/test/filesystem/search.test.ts` | `@` mention autocomplete lists file/directory results before references/agents/MCP resources; TUI and run-CLI menus cap the leading files so up to 3 non-file matches stay in the first visible page; web app group rank reordered to recent → file → reference → agent → resource (extracted into fork-only `prompt-input/at-options.ts`). TUI and run-CLI file resources track menu visibility so opening on an empty query fetches (previously nothing refetched until the query changed); core's ripgrep-fallback `find` returns its first entries for an empty query to mirror fff. Tests: tui-sdk fixture's `FetchHandler` gains an optional `method` arg, `renderFooter` accepts `findFiles`/`agents` overrides + a mention-menu ordering test, core search tests cover the fallback's empty query (fork-only test files: `packages/tui/test/prompt-autocomplete.test.tsx`, `packages/app/test-browser/prompt-at-options.test.ts`, `packages/app/src/components/prompt-input/at-options{,.test}.ts`) | Files are the dominant `@` use case; they should rank first and appear on a bare `@`. | Keep our ordering/reserve logic, resource source tracking, empty-query branch, fixture/test additions; take upstream's surrounding changes. | | `packages/slack/.gitignore`, `sdks/vscode/.gitignore` | Added the missing trailing newline (content unchanged) | Upstream's copies end without a newline; POSIX text files should end with one, and tools that concatenate or append to these files misbehave without it. | Whitespace-only: keep the trailing newline; take upstream's entries. | | `packages/tui/src/config/keybind.ts`, `packages/tui/src/component/prompt/index.tsx`, `packages/tui/src/app.tsx` | Permission-mode indicator + shift+tab cycle. keybind.ts: added `permission_mode_cycle: keybind("shift+tab", ...)` + its `permission.mode` CommandMap entry; `agent_cycle_reverse` default `"shift+tab"` → `"none"` (tab still cycles agents forward; previous-agent stays rebindable). prompt/index.tsx: the footer badge next to the agent name always renders in prompt mode — gray `manual` (`theme.textMuted`) in normal mode, red `auto` (`theme.error`) in auto mode (upstream only showed gray `auto`); the idle footer's shortcut hints gain a `shift+tab modes` text (via a `modeShortcut` `useCommandShortcut("permission.mode")` memo) left of `tab agents`, in the same no-usage `Match` arm. app.tsx: `"permission.mode"` added to `appBindingCommands` so the binding is gathered. | The permission mode is always visible (manual/auto) with auto in red as a danger signal, and shift+tab toggles it without opening the palette. | Keep the `permission_mode_cycle` definition + mapping, the `agent_cycle_reverse` `"none"` default, the badge `Show` block, the `modeShortcut` memo + hint text, and the `appBindingCommands` entry; take upstream's surrounding changes. If upstream reworks the auto badge or rebinds shift+tab, re-apply on top and re-audit the shift+tab conflict. | +| `packages/core/src/filesystem/search.ts`, `packages/core/src/ripgrep/binary.ts`, `packages/core/src/filesystem.ts`, `packages/core/src/tool/read-filesystem.ts`, `packages/core/test/filesystem/search.test.ts`, `packages/core/test/location-filesystem.test.ts` | Windows file-search fallback robustness. search.ts: the background ripgrep index walk no longer dies silently (`Effect.orDie` in a forked fiber) — each attempt resets the accumulated file/directory state, failures log a warning ("file index scan failed, file search may be empty"), and the walk retries forever on a 1s exponential backoff capped at 30s spacing, so installing rg mid-session self-heals; directory entries are suffixed and detected with `"/"` instead of `path.sep` (both ripgrep and fff layers), and fallback `find` queries normalize `\` to `/` and collapse doubled slashes (directory tab-expansion appends `/` to an already-`/`-suffixed entry). binary.ts: `Effect.cached` on ripgrep binary resolution (which memoizes failure exits for the process lifetime) replaced with a success-only cache behind a semaphore, so a failed rg download (offline, blocked proxy) retries on the next use instead of bricking search until restart. filesystem.ts `list` + read-filesystem.ts `list`: directory suffix `path.sep` → `"/"`; `fs.list` also `/`-normalizes `path.relative` output. Tests: search.test.ts pins `"/"`-suffixed directories, adds query-normalization coverage, and a stub-Ripgrep scan-retry test asserting the partial first walk is discarded; location-filesystem.test.ts pins `"src/"`. | On Windows fff is disabled by default (upstream `e4300e9b74`), so the `@` mention popup and file search depend entirely on the ripgrep fallback; a missing `rg.exe` plus one failed download left the index permanently empty with no user-visible signal (the CI-workflows row's choco preinstall papered over the same swallowed failure). `\`-suffixed directory entries also broke `endsWith("/")` directory-mime detection in the run CLI and poisoned drill-down queries with backslashes that can never match the `/`-normalized index. | Keep our retry/reset scan, success-only binary cache, `"/"` suffixes, and query normalization; take upstream's surrounding changes. If upstream reworks the fallback or re-enables fff on win32, preserve the invariants: entry paths are `/`-normalized with `/`-suffixed directories on every platform, index-scan failures are logged and retried, and a failed binary download is never cached. | | `packages/tui/src/context/permission.tsx`, `packages/tui/src/app.tsx`, `packages/tui/src/ui/dialog-confirm.tsx`, `packages/tui/src/ui/toast.tsx` | Auto-approve consent gate. permission.tsx: mode always starts `"normal"` (upstream initialized it from `args.auto`); `toggle()` replaced by `set(mode)` plus a per-run `consented` flag + `consent()`. app.tsx: shared `enableAutoApprove()` (title in the `AUTO_APPROVE_TITLE` module const, body from an `autoApproveMessage` accessor, and a shorter one-line `autoApproveToast` accessor for the toast, which only has a few seconds on screen; both redden the word "dangerous" with `theme.error` and are passed unevaluated because renderables can only be created during render) shows a `DialogConfirm` opened on cancel and, on confirm, a warning toast under the same title, then sets the mode; both entry points funnel through it — an `onMount` when `args.auto` is set, and the `permission.mode` command (shift+tab / palette), which enables via the gate and disables directly. Consent is asked once per run; later toggles are silent. dialog-confirm.tsx: optional `defaultChoice` prop + 5th `DialogConfirm.show` arg selecting the initially-active button (omitted keeps upstream's confirm-first default); `message` widened to `JSX.Element \| (() => JSX.Element)` and called at the render site. toast.tsx: same `message` widening, resolved by a `message()` helper in `Toast()` (fork-only test file: `packages/tui/test/auto-approve-consent.view.test.tsx`). | Auto-approve is dangerous, so `--auto` (and its hidden aliases) and the first shift+tab into auto require explicit confirmation; declining or dismissing the dialog stays in manual mode, opening on cancel keeps a stray return from granting it, and the risk word is red like the `auto` badge. | Keep the consent flag + gate flow, the `toggle()` removal, the `defaultChoice` prop, and the accessor-aware `message` on both dialog and toast; take upstream's surrounding changes. If upstream adds new writers of the permission mode, route their enable path through `enableAutoApprove()`. | ## Known rebrand gaps diff --git a/packages/core/src/filesystem.ts b/packages/core/src/filesystem.ts index 8470134ff7..4473dfd4ac 100644 --- a/packages/core/src/filesystem.ts +++ b/packages/core/src/filesystem.ts @@ -95,10 +95,12 @@ const baseLayer = Layer.effect( .flatMap((item) => { if (item.type !== "file" && item.type !== "directory") return [] const absolute = path.join(target.absolute, item.name) - const relative = path.relative(target.directory, absolute) + // Entry paths are "/"-normalized on every platform; path.relative + // yields "\" separators on Windows. + const relative = path.relative(target.directory, absolute).split(path.sep).join("/") return [ Entry.make({ - path: RelativePath.make(relative + (item.type === "directory" ? path.sep : "")), + path: RelativePath.make(relative + (item.type === "directory" ? "/" : "")), type: item.type, }), ] diff --git a/packages/core/src/filesystem/search.ts b/packages/core/src/filesystem/search.ts index 6cc836ef44..b689d995ad 100644 --- a/packages/core/src/filesystem/search.ts +++ b/packages/core/src/filesystem/search.ts @@ -2,7 +2,7 @@ export * as FileSystemSearch from "./search" import { makeLocationNode } from "../effect/app-node" import path from "path" -import { Context, Effect, Layer, Scope } from "effect" +import { Context, Effect, Layer, Schedule, Scope } from "effect" import { Fff } from "#fff" import fuzzysort from "fuzzysort" import { FileSystem } from "../filesystem" @@ -32,8 +32,12 @@ export const ripgrepLayer = Layer.effect( directories: [] as string[], } const directories = new Set() - yield* ripgrep - .find({ + const scan = Effect.suspend(() => { + // Reset so a retried scan does not duplicate entries from a partial walk. + state.files = [] + state.directories = [] + directories.clear() + return ripgrep.find({ cwd: location.directory, pattern: "*", limit: location.vcs ? Number.MAX_SAFE_INTEGER : 100_000, @@ -41,11 +45,20 @@ export const ripgrepLayer = Layer.effect( Effect.sync(() => { state.files.push(entry.path) const parts = entry.path.split("/") - parts.slice(0, -1).forEach((_, index) => directories.add(parts.slice(0, index + 1).join("/") + path.sep)) + parts.slice(0, -1).forEach((_, index) => directories.add(parts.slice(0, index + 1).join("/") + "/")) state.directories = Array.from(directories) }), }) - .pipe(Effect.orDie, Effect.asVoid, Effect.forkIn(scope)) + }) + // A dead scan leaves find() permanently empty (the "@" mention popup shows + // nothing), so log failures and keep retrying: ripgrep may need a binary + // download that failed transiently or an install that happens mid-session. + yield* scan.pipe( + Effect.tapError((error) => Effect.logWarning("file index scan failed, file search may be empty", { error })), + Effect.retry(Schedule.exponential("1 second").pipe(Schedule.either(Schedule.spaced("30 seconds")))), + Effect.asVoid, + Effect.forkIn(scope), + ) return Service.of({ glob: (input) => Effect.gen(function* () { @@ -109,12 +122,15 @@ export const ripgrepLayer = Layer.effect( const entry = (relative: string) => FileSystem.Entry.make({ path: RelativePath.make(relative), - type: relative.endsWith(path.sep) ? "directory" : "file", + type: relative.endsWith("/") ? "directory" : "file", }) // fuzzysort matches nothing on an empty needle; mirror fff, which // returns its top-ranked entries for an empty query. if (!input.query) return items.slice(0, input.limit ?? 50).map(entry) - return fuzzysort.go(input.query, items, { limit: input.limit ?? 50 }).map((item) => entry(item.target)) + // Windows habits and directory tab-expansion produce "\" and "//" in + // queries; the index is "/"-normalized, so those could never match. + const query = input.query.split("\\").join("/").replace(/\/{2,}/g, "/") + return fuzzysort.go(query, items, { limit: input.limit ?? 50 }).map((item) => entry(item.target)) }), }) }), @@ -224,7 +240,7 @@ export const fffLayer = Layer.effect( .map((item) => { const relative = item.path.replaceAll("\\", "/").replace(/\/$/, "") return FileSystem.Entry.make({ - path: RelativePath.make(relative + (item.type === "directory" ? path.sep : "")), + path: RelativePath.make(relative + (item.type === "directory" ? "/" : "")), type: item.type, }) }) diff --git a/packages/core/src/ripgrep/binary.ts b/packages/core/src/ripgrep/binary.ts index 762a8e613e..b81c24113f 100644 --- a/packages/core/src/ripgrep/binary.ts +++ b/packages/core/src/ripgrep/binary.ts @@ -1,5 +1,5 @@ import path from "path" -import { Context, Effect, Layer, Stream } from "effect" +import { Context, Effect, Layer, Semaphore, Stream } from "effect" import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http" import { ChildProcess } from "effect/unstable/process" import { ChildProcessSpawner } from "effect/unstable/process/ChildProcessSpawner" @@ -88,36 +88,45 @@ export namespace RipgrepBinary { if (process.platform !== "win32") yield* fs.chmod(target, 0o755) }, Effect.scoped) - return Service.of({ - filepath: yield* Effect.cached( - Effect.gen(function* () { - const system = yield* Effect.sync(() => which(process.platform === "win32" ? "rg.exe" : "rg")) - if (system && (yield* fs.isFile(system).pipe(Effect.orDie))) return system - - const target = path.join(Global.Path.bin, `rg${process.platform === "win32" ? ".exe" : ""}`) - if (yield* fs.isFile(target).pipe(Effect.orDie)) return target - - const platformKey = `${process.arch}-${process.platform}` as keyof typeof PLATFORM - const config = PLATFORM[platformKey] - if (!config) throw new Error(`unsupported platform for ripgrep: ${platformKey}`) - - const filename = `ripgrep-${VERSION}-${config.platform}.${config.extension}` - const url = `https://github.com/BurntSushi/ripgrep/releases/download/${VERSION}/${filename}` - const archive = path.join(Global.Path.bin, filename) - - yield* Effect.logInfo("downloading ripgrep", { url }) - yield* fs.ensureDir(Global.Path.bin).pipe(Effect.orDie) - const bytes = yield* HttpClientRequest.get(url).pipe( - http.execute, - Effect.flatMap((response) => response.arrayBuffer), - Effect.mapError((cause) => (cause instanceof Error ? cause : new Error(String(cause)))), - ) - if (bytes.byteLength === 0) throw new Error(`failed to download ripgrep from ${url}`) + // Effect.cached memoizes failures for the process lifetime: one failed + // download (offline, blocked proxy) would leave every ripgrep consumer + // broken until restart. Cache successful resolution only. + const resolved = { filepath: undefined as string | undefined } + const semaphore = Semaphore.makeUnsafe(1) + const resolve = Effect.gen(function* () { + const system = yield* Effect.sync(() => which(process.platform === "win32" ? "rg.exe" : "rg")) + if (system && (yield* fs.isFile(system).pipe(Effect.orDie))) return system + + const target = path.join(Global.Path.bin, `rg${process.platform === "win32" ? ".exe" : ""}`) + if (yield* fs.isFile(target).pipe(Effect.orDie)) return target + + const platformKey = `${process.arch}-${process.platform}` as keyof typeof PLATFORM + const config = PLATFORM[platformKey] + if (!config) throw new Error(`unsupported platform for ripgrep: ${platformKey}`) + + const filename = `ripgrep-${VERSION}-${config.platform}.${config.extension}` + const url = `https://github.com/BurntSushi/ripgrep/releases/download/${VERSION}/${filename}` + const archive = path.join(Global.Path.bin, filename) + + yield* Effect.logInfo("downloading ripgrep", { url }) + yield* fs.ensureDir(Global.Path.bin).pipe(Effect.orDie) + const bytes = yield* HttpClientRequest.get(url).pipe( + http.execute, + Effect.flatMap((response) => response.arrayBuffer), + Effect.mapError((cause) => (cause instanceof Error ? cause : new Error(String(cause)))), + ) + if (bytes.byteLength === 0) throw new Error(`failed to download ripgrep from ${url}`) - yield* fs.writeWithDirs(archive, new Uint8Array(bytes)) - yield* extract(archive, config, target) - yield* fs.remove(archive, { force: true }).pipe(Effect.ignore) - return target + yield* fs.writeWithDirs(archive, new Uint8Array(bytes)) + yield* extract(archive, config, target) + yield* fs.remove(archive, { force: true }).pipe(Effect.ignore) + return target + }) + return Service.of({ + filepath: semaphore.withPermit( + Effect.suspend(() => { + if (resolved.filepath) return Effect.succeed(resolved.filepath) + return resolve.pipe(Effect.tap((filepath) => Effect.sync(() => (resolved.filepath = filepath)))) }), ), }) diff --git a/packages/core/src/tool/read-filesystem.ts b/packages/core/src/tool/read-filesystem.ts index e325a83edd..0164bb37a1 100644 --- a/packages/core/src/tool/read-filesystem.ts +++ b/packages/core/src/tool/read-filesystem.ts @@ -337,7 +337,7 @@ export const list = Effect.fn("ReadTool.list")(function* (fs: FSUtil.Interface, const type = info?.type === "Directory" ? "directory" : info?.type === "File" ? "file" : undefined if (!type) return return FileSystem.Entry.make({ - path: RelativePath.make(item.name + (type === "directory" ? path.sep : "")), + path: RelativePath.make(item.name + (type === "directory" ? "/" : "")), type, }) }), diff --git a/packages/core/test/filesystem/search.test.ts b/packages/core/test/filesystem/search.test.ts index 24ed52eb4c..4a8e11f2d6 100644 --- a/packages/core/test/filesystem/search.test.ts +++ b/packages/core/test/filesystem/search.test.ts @@ -81,16 +81,11 @@ describe("FileSystemSearch ripgrep fallback", () => { yield* Effect.promise(() => fs.writeFile(path.join(cwd, "src", "match.ts"), "needle\n")) yield* Effect.promise(() => fs.writeFile(path.join(cwd, "README.md"), "# readme\n")) const entries = yield* findSettled("") - // Directory entries carry the OS path separator while ripgrep returns files - // with forward slashes, so sort both sides for a platform-independent compare. + // Directory entries are "/"-suffixed and "/"-normalized on every platform. expect(entries.map((entry) => entry.path).sort()).toEqual( - [ - RelativePath.make("README.md"), - RelativePath.make("src" + path.sep), - RelativePath.make("src/match.ts"), - ].sort(), + [RelativePath.make("README.md"), RelativePath.make("src/"), RelativePath.make("src/match.ts")].sort(), ) - expect(entries.find((entry) => entry.path === "src" + path.sep)?.type).toBe("directory") + expect(entries.find((entry) => entry.path === "src/")?.type).toBe("directory") }).pipe(Effect.provide(searchLayer(cwd))), ), ) @@ -118,4 +113,68 @@ describe("FileSystemSearch ripgrep fallback", () => { }).pipe(Effect.provide(searchLayer(cwd))), ), ) + + it.live("normalizes backslashes and doubled slashes in queries", () => + withTmp((cwd) => + Effect.gen(function* () { + yield* Effect.promise(() => fs.mkdir(path.join(cwd, "src"))) + yield* Effect.promise(() => fs.writeFile(path.join(cwd, "src", "match.ts"), "needle\n")) + const backslash = yield* findSettled("src\\match") + expect(backslash.map((entry) => entry.path)).toContain(RelativePath.make("src/match.ts")) + // Directory tab-expansion appends "/" to the "/"-suffixed entry, producing "//". + const doubled = yield* findSettled("src//match") + expect(doubled.map((entry) => entry.path)).toContain(RelativePath.make("src/match.ts")) + }).pipe(Effect.provide(searchLayer(cwd))), + ), + ) + + it.live("retries the scan after a failed walk without duplicating entries", () => + Effect.gen(function* () { + const entry = FileSystem.Entry.make({ path: RelativePath.make("src/match.ts"), type: "file" }) + const stale = FileSystem.Entry.make({ path: RelativePath.make("stale.ts"), type: "file" }) + const attempts = { count: 0 } + const ripgrep = Ripgrep.Service.of({ + find: (input) => + Effect.gen(function* () { + attempts.count++ + if (attempts.count === 1) { + yield* input.onEntry?.(stale) ?? Effect.void + return yield* new Ripgrep.Error({ message: "walk failed" }) + } + yield* input.onEntry?.(entry) ?? Effect.void + return [entry] + }), + glob: () => Effect.succeed([]), + grep: () => Effect.succeed([]), + }) + const layer = FileSystemSearch.ripgrepLayer.pipe( + Layer.provide( + Layer.mergeAll( + LayerNode.compile(FSUtil.node), + Layer.succeed(Ripgrep.Service, ripgrep), + Layer.succeed( + Location.Service, + Location.Service.of(location({ directory: AbsolutePath.make(process.cwd()) })), + ), + ), + ), + ) + yield* Effect.gen(function* () { + const search = yield* FileSystemSearch.Service + // The first walk emits a partial entry and dies; the retry (1s backoff) + // must reset that partial state and repopulate the index. + for (let i = 0; i < 100; i++) { + const found = yield* search.find(FileSystem.FindInput.make({ query: "" })) + if (found.some((item) => item.path === "src/match.ts")) break + yield* Effect.sleep("100 millis") + } + const found = yield* search.find(FileSystem.FindInput.make({ query: "" })) + expect(found.map((item) => item.path).sort()).toEqual([ + RelativePath.make("src/"), + RelativePath.make("src/match.ts"), + ]) + expect(attempts.count).toBe(2) + }).pipe(Effect.provide(layer)) + }), + ) }) diff --git a/packages/core/test/location-filesystem.test.ts b/packages/core/test/location-filesystem.test.ts index f0075db245..3de6c4738f 100644 --- a/packages/core/test/location-filesystem.test.ts +++ b/packages/core/test/location-filesystem.test.ts @@ -49,7 +49,7 @@ describe("FileSystem", () => { yield* Effect.promise(() => fs.writeFile(path.join(directory, "README.md"), "# Test")) const entries = yield* (yield* FileSystem.Service).list() expect(entries.map((entry) => ({ path: entry.path, type: entry.type }))).toEqual([ - { path: RelativePath.make("src" + path.sep), type: "directory" }, + { path: RelativePath.make("src/"), type: "directory" }, { path: RelativePath.make("README.md"), type: "file" }, ]) }).pipe(provide(directory)),