From daeba45c771282adf9d7b92943498e8833523792 Mon Sep 17 00:00:00 2001 From: J Date: Thu, 4 Jun 2026 14:06:00 +0800 Subject: [PATCH] fix(agent): keep resolvable models when CLI discovery exits non-zero (MUL-2977) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Model discovery shells out to each runtime's CLI (`pi --list-models`, `opencode models`, `cursor-agent --list-models`, `openclaw agents list`) and parses the output. The bail-out was gated only on `cmd.Output()` returning an error, so any non-zero exit discarded the already-captured output and returned an empty list — even when the CLI had printed a perfectly good catalog. Pi exits non-zero (with a `No models match pattern "..."` warning) when an agent config still references stale provider/model patterns that no longer match the local catalog. The daemon threw the whole stdout away, so the UI model picker came back empty while the runtime stayed online and agents ran fine on their configured model. See GitHub #3729. - Only treat a non-zero exit as failure when there is also no usable output (stdout/stderr empty), so a partial-but-valid catalog is still parsed. - Guard `parsePiModels` so a stray `Warning:` line that shares the stream with the table (older pi prints the catalog to stderr) cannot become a bogus `Warning/` entry in the picker. - Regression test covers both shapes: catalog on stdout and catalog on stderr, each with a non-zero exit. Co-authored-by: multica-agent --- server/pkg/agent/models.go | 28 +++++++++----- server/pkg/agent/models_test.go | 65 +++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/server/pkg/agent/models.go b/server/pkg/agent/models.go index b16df1627..5c0fe3ed7 100644 --- a/server/pkg/agent/models.go +++ b/server/pkg/agent/models.go @@ -362,11 +362,13 @@ func discoverOpenCodeModels(ctx context.Context, executablePath string) ([]Model hideAgentWindow(cmd) out, err := cmd.Output() if err != nil { - cmd = exec.CommandContext(runCtx, executablePath, "models") - hideAgentWindow(cmd) - out, err = cmd.Output() - if err != nil { - return []Model{}, nil + if len(out) == 0 { + cmd = exec.CommandContext(runCtx, executablePath, "models") + hideAgentWindow(cmd) + out, err = cmd.Output() + if err != nil && len(out) == 0 { + return []Model{}, nil + } } } return parseOpenCodeModels(string(out)), nil @@ -562,7 +564,7 @@ func discoverPiModels(ctx context.Context, executablePath string) ([]Model, erro var stderr strings.Builder cmd.Stderr = &stderr stdout, err := cmd.Output() - if err != nil { + if err != nil && len(stdout) == 0 && stderr.Len() == 0 { return []Model{}, nil } text := string(stdout) @@ -606,6 +608,14 @@ func parsePiModels(output string) []Model { } else { continue } + // A real row resolves to `provider/model` with both halves present. + // Drop anything else — e.g. a stray `Warning: No models match pattern + // "..."` line, which pi can interleave with the catalog when an agent + // config holds stale patterns (#3729). Without this guard the leading + // `Warning:` token becomes a bogus `Warning/` model in the picker. + if slash := strings.Index(id, "/"); slash <= 0 || slash == len(id)-1 { + continue + } if seen[id] { continue } @@ -942,7 +952,7 @@ func discoverCursorModels(ctx context.Context, executablePath string) ([]Model, cmd := exec.CommandContext(runCtx, executablePath, "--list-models") hideAgentWindow(cmd) out, err := cmd.Output() - if err != nil { + if err != nil && len(out) == 0 { return cursorStaticModels(), nil } models := parseCursorModels(string(out)) @@ -1039,7 +1049,7 @@ func discoverOpenclawAgents(ctx context.Context, executablePath string) ([]Model cmd := exec.CommandContext(runCtx, executablePath, jsonArgs...) hideAgentWindow(cmd) out, err := cmd.Output() - if err != nil { + if err != nil && len(out) == 0 { continue } if models, ok := parseOpenclawAgentsJSON(out); ok { @@ -1053,7 +1063,7 @@ func discoverOpenclawAgents(ctx context.Context, executablePath string) ([]Model cmd := exec.CommandContext(runCtx, executablePath, "agents", "list") hideAgentWindow(cmd) out, err := cmd.Output() - if err != nil { + if err != nil && len(out) == 0 { return []Model{}, nil } return parseOpenclawAgents(string(out)), nil diff --git a/server/pkg/agent/models_test.go b/server/pkg/agent/models_test.go index ba6987c7d..a1611c2dc 100644 --- a/server/pkg/agent/models_test.go +++ b/server/pkg/agent/models_test.go @@ -4,6 +4,7 @@ import ( "context" "path/filepath" "runtime" + "strconv" "strings" "testing" ) @@ -450,6 +451,70 @@ bareword-only-line } } +// TestDiscoverPiModelsNonZeroExit verifies that discoverPiModels still returns +// the resolvable catalog when `pi --list-models` exits non-zero. Pi exits +// non-zero (and warns) when an agent config references stale provider/model +// patterns that no longer match the local catalog. Before the fix the daemon +// discarded the populated output on any non-zero exit and returned an empty +// list, so the UI model picker was blank even though the runtime was online and +// agents ran fine. See GitHub #3729. +func TestDiscoverPiModelsNonZeroExit(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake pi binary is a /bin/sh script") + } + + const table = "provider model context max-out thinking images\n" + + "glm-coding-plan glm-4.7 202.8K 16.4K no no" + const warning = `Warning: No models match pattern "opencode-go/mimo-v2-omni"` + + cases := []struct { + name string + script string + }{ + { + // Newer pi prints the catalog to stdout; the stale-pattern + // warning goes to stderr and the process exits non-zero. + name: "catalog on stdout", + script: "#!/bin/sh\n" + + "cat <<'EOF'\n" + table + "\nEOF\n" + + "echo " + strconv.Quote(warning) + " >&2\n" + + "exit 1\n", + }, + { + // Older pi prints the catalog (and the warning) to stderr; same + // non-zero exit. The stderr fallback must still parse the catalog. + name: "catalog on stderr", + script: "#!/bin/sh\n" + + "cat >&2 <<'EOF'\n" + table + "\n" + warning + "\nEOF\n" + + "exit 1\n", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fakePath := filepath.Join(t.TempDir(), "pi") + writeTestExecutable(t, fakePath, []byte(tc.script)) + + models, err := discoverPiModels(context.Background(), fakePath) + if err != nil { + t.Fatalf("discoverPiModels: %v", err) + } + var found bool + for _, m := range models { + if m.ID == "glm-coding-plan/glm-4.7" { + found = true + } + if m.ID == "Warning/" || m.Provider == "Warning" { + t.Errorf("warning line leaked into the catalog as a bogus model: %+v", m) + } + } + if !found { + t.Fatalf("expected glm-coding-plan/glm-4.7 despite non-zero exit, got %+v", models) + } + }) + } +} + func TestParseOpenclawAgents(t *testing.T) { input := `deepseek-v4 deepseek-v4 claude-sonnet claude-sonnet-4-6