Repository navigation
fix(core): retry the ripgrep binary resolution after a failure - #63424
Anthony14FR wants to merge 2 commits into
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks good to merge. A failed ripgrep resolution is no longer cached for the life of the process, and a resolved path is still cached forever.
How I checked: I ran the reproduction from #51022 from source with rg off PATH and GitHub unreachable. On the base, the second grep still returned ripgrep execution failed after a working rg was placed in the cache bin folder. With this PR, the second grep finds src/value.ts. I also read cachedWithTTL in the pinned Effect 4.1.0. A zero TTL on a failure makes the next call run the resolver again, concurrent callers still share one run, and errors thrown inside the resolver are covered too. test/ripgrep-binary.test.ts (fails on the base) and test/ripgrep.test.ts pass, and tsc -b in packages/core is clean.
Public API: unchanged.
The new test is small and uses a stub HTTP client with no real network, so it's fine. It isn't needed for a fix this size, though, and can be dropped (or cut to one case) to keep the change minimal.
Related: #40623
There was a problem hiding this comment.
My earlier note is addressed: the test is now a single focused case, and it still fails on the base and passes here. The source change is the same one I verified before. With rg off PATH and GitHub unreachable, the second grep now picks up a binary placed in the cache bin folder. test/ripgrep.test.ts still passes.
Public API: unchanged.
No further concerns.
Issue for this PR
Fixes #51022
Type of change
What does this PR do?
The ripgrep binary resolver was wrapped in
Effect.cached, which also caches failures. One failed download (offline, proxy, empty body) left file search broken until OpenCode restarted, even after ripgrep became available.It now uses
Effect.cachedWithTTLwith an exit-based TTL: a resolved path is kept forever, a failure is not cached, so the next call resolves again. It stays single-flight, and it covers both typed failures and errors thrown inside the resolver. Failures retry on the next call with no backoff on purpose, so the workaround from the issue (droppingrginto the cache dir) works right away.#51192 tried this before and was closed by the stale-PR cleanup because it had no linked issue. It used
tapError, which doesn't see the errors thrown inside the resolver.The Windows system-proxy part of the issue is not addressed here.
How did you verify your code works?
packages/core/test/ripgrep-binary.test.ts: after a 503 and after an empty 200 response, a second call finds a binary placed in the bin dir. Both cases fail ondevand pass with this change (also with--rerun-each 5).bun test test/ripgrep.test.ts,bun typecheckinpackages/core, oxlint and prettier pass on Windows 10. Not run on Linux or macOS.Screenshots / recordings
Not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #40623