Repository navigation
fix(core): give local model discovery 15 seconds instead of 1 - #63700
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works. I ran opencode serve from source on the base and on this branch, against a mock vLLM whose /health and /v1/models each take 3 s:
- Base:
/healthtimes out and no vLLM models are listed. - This branch:
vllm/slow-modelappears.
When I stopped the mock, the branch logged one failed to discover vLLM models WARN with the baseURL and the cause, and did not repeat it on later refreshes. The vLLM, Ollama and LM Studio plugin tests pass. The new test is small and uses the test clock, so it should not be flaky.
Public API: unchanged. There are no new config keys, flags, events or endpoints, only a new log line.
Non-blocking: discoveryLock is shared by every instance of a provider type. A slow or unreachable configured endpoint can now hold it for up to 15 s per request: about 30 s per round for vLLM (/health then /v1/models), and longer for Ollama, which also calls /api/show for each model. While it is held, discovery for that provider in other directories on the same background service waits. The config.updated reload and the account-switch handler also run attempt directly instead of in the background, so they can wait about as long. That is probably fine, but please keep it in mind.
This does not fully fix #53341. Running that issue's repro (Ollama /api/tags taking 1.5 s, then run --standalone -m ollama/qwen3:test) still fails with Unsupported package on this branch, because the prompt starts before discovery finishes.
Related: #53341
There was a problem hiding this comment.
The new commits address both earlier points: the discovery lock is now per endpoint, and the config reload and account-switch handlers no longer wait for discovery. I checked with opencode serve from source against a slow mock vLLM:
- A 3 s endpoint is still discovered.
- I pointed the config at a 10 s endpoint, then back after 2 s. The switch back took effect while the slow request was still in flight.
The vLLM, Ollama and LM Studio plugin tests pass. Public API: unchanged.
One new issue: an old attempt that fails after a reload logs its warning against the new endpoint. See the inline comment.
| yield* ctx.integration.reload() | ||
| yield* ctx.provider.reload() | ||
| yield* refresh().pipe(Effect.ignore) | ||
| yield* FiberSet.run(background, attempt) |
There was a problem hiding this comment.
Now that a reload no longer waits for the attempt already in flight, that old attempt can fail after source.current has moved on. The Effect.catch in attempt reads source.current when the error arrives, so the warning names the wrong endpoint. It also sets loaded.failing for the new source, which hides the warning if the new endpoint really does fail later. I saw this in practice: I switched the config from a 10 s mock on :18002 back to a healthy :18001, and the :18002 timeout logged failed to discover vLLM models baseURL=http://127.0.0.1:18001/v1 cause=TimeoutError. A fix is to capture source.current when the attempt starts, and skip both the warning and the failing update if it is no longer current when the error arrives.
There was a problem hiding this comment.
The new commit fixes the misattributed warning from my last review: a failure from an endpoint that has since been replaced no longer logs a warning or marks the new endpoint as failing.
I checked it with opencode serve from source. I switched from a 20 s mock back to a healthy one; the old request timed out without a warning, where the previous head named the healthy URL. The new test passes 5/5 and fails 3/3 with the old catch put back. The Ollama and LM Studio plugin tests pass.
Public API: unchanged. No new concerns.
Issue for this PR
Closes #58558
Related: #53341
Type of change
What does this PR do?
Local model discovery (Ollama, LM Studio, vLLM) gave each request 1 second. A remote or tunnelled endpoint that takes longer than that to connect got zero models and nothing in the log.
I raised it to 15 seconds per request. Our other model list fetches (models.dev, Azure, GitLab, DigitalOcean) use 10, but the report measured about 9 seconds just to connect, so 10 would still be a coin flip there. Nothing listening on localhost still fails right away with connection refused.
A longer timeout means one slow endpoint can stay busy for a while (about 30 seconds per round for vLLM, which calls
/healththen/v1/models), so discovery now stays out of the way while it waits:config.updatedreload and the account-switch handler start discovery in the background instead of waiting for it, like the periodic refresh already does. A slow endpoint no longer delays the next config or account event.I also log a warning when discovery fails for an endpoint you set in config, once per outage. The default localhost probe stays quiet since most people aren't running all three servers.
Not fixed here
This does not fix #53341. A prompt sent before discovery finishes still fails with
Unsupported package, because the provider has no package until the first discovery lands. Fixing that means the first request either waits for discovery or the configured provider keeps a package before discovery succeeds. That changes how model resolution and provider plugins work together, so it belongs in its own PR.How did you verify your code works?
Added a test with a vLLM server that holds
/v1/modelsopen while the test clock moves 10 seconds, then answers. It passes now and fails if I put the timeout back to 1 second. A second test switches config away from an endpoint while its request is still open, then lets that request time out. The late failure must not log a warning against the new endpoint, and without the check it logs one naming the new URL. Ran the vLLM, Ollama and LM Studio plugin tests 5 times each after the lock and background changes, andbun typecheckinpackages/core.I did not try it against a real slow remote endpoint.
Screenshots / recordings
Not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #53341