From 7fc8ac63b8bb51da4cd0395b96aef725618a3cb2 Mon Sep 17 00:00:00 2001
From: J <j@multica.ai>
Date: Fri, 5 Jun 2026 14:33:29 +0800
Subject: [PATCH] fix(agent): raise pi model-discovery timeout to 15s, stop
 caching empty results (MUL-2977)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 server/pkg/agent/models.go      | 17 ++++++++++++++-
 server/pkg/agent/models_test.go | 38 +++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/server/pkg/agent/models.go b/server/pkg/agent/models.go
index faded3e5d..ef9e4f6ca 100644
--- a/server/pkg/agent/models.go
+++ b/server/pkg/agent/models.go
@@ -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)
diff --git a/server/pkg/agent/models_test.go b/server/pkg/agent/models_test.go
index 47c12dce1..c1123a81d 100644
--- a/server/pkg/agent/models_test.go
+++ b/server/pkg/agent/models_test.go
@@ -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