mirror of
https://github.com/multica-ai/multica.git
synced 2026-06-17 03:38:32 +02:00
fix(agent): raise pi model-discovery timeout to 15s, stop caching empty results (MUL-2977)
The daemon runs `pi --list-models` to build the model picker. Newer pi (0.78.0) fetches its catalog from each configured provider over the network, so discovery time scales with provider count — a reporter's multi-provider setup measured ~4.6-4.8s across five runs, right at the 5s cap. Network jitter pushed it over, the daemon killed the command before it printed anything, and the picker came back empty while the runtime stayed online and agents ran fine. This is the same class as the opencode timeout in #3627 (fixed in #3689 by raising that path to 15s); the pi path was never raised. Raise pi's discovery timeout 5s -> 15s to match. Also stop `cachedDiscovery` from caching an empty result: zero models is almost always a transient failure (timeout, not-logged-in, network blip), and caching it kept the picker blank for the full 60s TTL even after the cause cleared (which is why restarting/clicking around didn't help). An empty result now retries on the next request; non-empty results are still cached. Fixes #3729. Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -182,6 +182,15 @@ func cachedDiscovery(key string, fn func() ([]Model, error)) ([]Model, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Don't cache an empty result. Zero models is almost always a transient
|
||||
// failure (discovery CLI timeout, not-logged-in, network blip) rather than
|
||||
// a runtime that genuinely has no models; caching it would keep the picker
|
||||
// blank for the full TTL even after the cause clears. Skipping the cache
|
||||
// lets the next request retry immediately. See #3729.
|
||||
if len(models) == 0 {
|
||||
return models, nil
|
||||
}
|
||||
|
||||
modelCacheMu.Lock()
|
||||
modelCache[key] = modelCacheEntry{models: models, expiresAt: time.Now().Add(modelCacheTTL)}
|
||||
modelCacheMu.Unlock()
|
||||
@@ -563,7 +572,13 @@ func discoverPiModels(ctx context.Context, executablePath string) ([]Model, erro
|
||||
if _, err := exec.LookPath(executablePath); err != nil {
|
||||
return []Model{}, nil
|
||||
}
|
||||
runCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
|
||||
// Newer pi fetches its catalog from each configured provider over the
|
||||
// network, so discovery time scales with provider count — a multi-provider
|
||||
// setup measured ~4.6-4.8s, right at the old 5s cap. When jitter pushed it
|
||||
// over, the daemon killed the command before it printed anything and the
|
||||
// model picker came back empty while the runtime stayed online. 15s matches
|
||||
// the opencode discovery cap (see #3729, same class as #3627).
|
||||
runCtx, cancel := context.WithTimeout(ctx, 15*time.Second)
|
||||
defer cancel()
|
||||
cmd := exec.CommandContext(runCtx, executablePath, "--list-models")
|
||||
hideAgentWindow(cmd)
|
||||
|
||||
@@ -407,6 +407,44 @@ exit 1
|
||||
}
|
||||
}
|
||||
|
||||
// TestCachedDiscoveryDoesNotCacheEmpty verifies that an empty discovery result
|
||||
// is not cached, so a transient failure (e.g. a `pi --list-models` timeout)
|
||||
// doesn't keep the model picker blank for the full TTL. A non-empty result is
|
||||
// still cached. See #3729.
|
||||
func TestCachedDiscoveryDoesNotCacheEmpty(t *testing.T) {
|
||||
emptyCalls := 0
|
||||
empty := func() ([]Model, error) {
|
||||
emptyCalls++
|
||||
return []Model{}, nil
|
||||
}
|
||||
for i := 0; i < 2; i++ {
|
||||
got, err := cachedDiscovery("test-cache-empty", empty)
|
||||
if err != nil {
|
||||
t.Fatalf("cachedDiscovery: %v", err)
|
||||
}
|
||||
if len(got) != 0 {
|
||||
t.Fatalf("expected empty result, got %+v", got)
|
||||
}
|
||||
}
|
||||
if emptyCalls != 2 {
|
||||
t.Fatalf("empty result must not be cached: expected fn called 2x, got %d", emptyCalls)
|
||||
}
|
||||
|
||||
nonEmptyCalls := 0
|
||||
nonEmpty := func() ([]Model, error) {
|
||||
nonEmptyCalls++
|
||||
return []Model{{ID: "provider/model"}}, nil
|
||||
}
|
||||
for i := 0; i < 2; i++ {
|
||||
if _, err := cachedDiscovery("test-cache-nonempty", nonEmpty); err != nil {
|
||||
t.Fatalf("cachedDiscovery: %v", err)
|
||||
}
|
||||
}
|
||||
if nonEmptyCalls != 1 {
|
||||
t.Fatalf("non-empty result must be cached: expected fn called 1x, got %d", nonEmptyCalls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParsePiModels(t *testing.T) {
|
||||
input := `openai:gpt-4o
|
||||
anthropic:claude-opus-4-7
|
||||
|
||||
Reference in New Issue
Block a user