fix(test): prevent agent CLI execution in default tests (#5789)

server/internal/daemon/agent_command_names_test.go

@@ -4,7 +4,12 @@ import (
 	"go/ast"
 	"go/parser"
 	"go/token"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
 	"sort"
+	"strings"
 	"testing"
 )
 
@@ -63,3 +68,82 @@ func TestDefaultAgentCommandNamesCoversAllProbes(t *testing.T) {
 			"add them so GUI-launched daemons can resolve these agents via the login shell", missing)
 	}
 }
+
+func TestAgentCLIGuardCoversDefaultCommands(t *testing.T) {
+	data, err := os.ReadFile(filepath.Join("..", "..", "..", "scripts", "agent-cli-command-names.txt"))
+	if err != nil {
+		t.Fatalf("read agent CLI guard names: %v", err)
+	}
+	guarded := map[string]bool{}
+	for lineNumber, line := range strings.Split(string(data), "\n") {
+		if line != strings.TrimSpace(line) {
+			t.Fatalf("agent CLI guard name on line %d has surrounding whitespace", lineNumber+1)
+		}
+		if line != "" && !strings.HasPrefix(line, "#") {
+			if !isSafeAgentCLICommandName(line) {
+				t.Fatalf("agent CLI guard name on line %d contains unsafe characters: %q", lineNumber+1, line)
+			}
+			guarded[line] = true
+		}
+	}
+	for _, name := range defaultAgentCommandNames {
+		if !guarded[name] {
+			t.Errorf("default agent command %q is not covered by the test guard", name)
+		}
+	}
+	if !guarded["qodercli"] {
+		t.Error("default qoder command \"qodercli\" is not covered by the test guard")
+	}
+}
+
+func isSafeAgentCLICommandName(name string) bool {
+	for _, char := range name {
+		if (char >= 'a' && char <= 'z') ||
+			(char >= 'A' && char <= 'Z') ||
+			(char >= '0' && char <= '9') ||
+			char == '.' || char == '_' || char == '-' {
+			continue
+		}
+		return false
+	}
+	return name != ""
+}
+
+func TestAgentCLIGuardDetectsSwallowedFailure(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("the full guarded backend suite runs on Linux/macOS")
+	}
+	script := filepath.Join("..", "..", "..", "scripts", "go-test-with-agent-cli-guard.sh")
+	cmd := exec.Command(script, "--", "/bin/sh", "-c", "claude --version --token super-secret >/dev/null 2>&1 || true")
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatalf("guard succeeded after a swallowed agent CLI failure: %s", out)
+	}
+	if !strings.Contains(string(out), "unexpected agent CLI invocation: claude [arguments redacted]") {
+		t.Fatalf("guard diagnostic missing invocation: %s", out)
+	}
+	if strings.Contains(string(out), "super-secret") {
+		t.Fatalf("guard diagnostic exposed command arguments: %s", out)
+	}
+}
+
+func TestAgentCLIGuardFailsClosedWhenSetupFails(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("the full guarded backend suite runs on Linux/macOS")
+	}
+	invalidTempDir := filepath.Join(t.TempDir(), "not-a-directory")
+	if err := os.WriteFile(invalidTempDir, []byte("fixture"), 0o600); err != nil {
+		t.Fatalf("write invalid temp directory fixture: %v", err)
+	}
+	executedMarker := filepath.Join(t.TempDir(), "executed")
+	script := filepath.Join("..", "..", "..", "scripts", "go-test-with-agent-cli-guard.sh")
+	cmd := exec.Command(script, "--", "/bin/sh", "-c", "printf ran >\"$1\"", "sh", executedMarker)
+	cmd.Env = append(os.Environ(), "TMPDIR="+invalidTempDir)
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatalf("guard succeeded after setup failure: %s", out)
+	}
+	if _, statErr := os.Stat(executedMarker); !os.IsNotExist(statErr) {
+		t.Fatalf("wrapped command ran after guard setup failure: %v", statErr)
+	}
+}

server/pkg/agent/agent_test_executable_test.go

@@ -0,0 +1,11 @@
+package agent
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+func missingAgentExecutable(tb testing.TB, name string) string {
+	tb.Helper()
+	return filepath.Join(tb.TempDir(), name)
+}

server/pkg/agent/grok_integration_test.go

@@ -0,0 +1,64 @@
+//go:build agentintegration
+
+package agent
+
+import (
+	"context"
+	"log/slog"
+	"os/exec"
+	"strings"
+	"testing"
+	"time"
+)
+
+// TestGrokRealACPSmoke drives the real `grok agent stdio` binary end-to-end.
+func TestGrokRealACPSmoke(t *testing.T) {
+	requireRealAgentSmoke(t)
+	if testing.Short() {
+		t.Skip("skipping real-binary smoke test in -short mode")
+	}
+	path, err := exec.LookPath("grok")
+	if err != nil {
+		t.Skip("grok not on PATH; skipping real-binary smoke test")
+	}
+	if version, err := exec.Command(path, "--version").CombinedOutput(); err == nil {
+		t.Logf("grok CLI version: %s", strings.TrimSpace(string(version)))
+	} else {
+		t.Logf("grok CLI version unavailable: %v (%s)", err, strings.TrimSpace(string(version)))
+	}
+
+	backend, err := New("grok", Config{ExecutablePath: path, Logger: slog.Default()})
+	if err != nil {
+		t.Fatalf("new grok backend: %v", err)
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second)
+	defer cancel()
+
+	session, err := backend.Execute(ctx, "Reply with exactly one word: pong. Do not use any tools.", ExecOptions{
+		Cwd:     t.TempDir(),
+		Timeout: 80 * time.Second,
+	})
+	if err != nil {
+		t.Fatalf("execute: %v", err)
+	}
+	go func() {
+		for range session.Messages {
+		}
+	}()
+
+	select {
+	case result := <-session.Result:
+		if result.Status != "completed" {
+			t.Fatalf("real grok run did not complete: status=%q error=%q", result.Status, result.Error)
+		}
+		if !strings.Contains(strings.ToLower(result.Output), "pong") {
+			t.Fatalf("expected real grok output to contain 'pong', got %q", result.Output)
+		}
+		if result.SessionID == "" {
+			t.Error("expected a non-empty session id from real grok")
+		}
+		t.Logf("real grok smoke OK: session=%s output=%q", result.SessionID, result.Output)
+	case <-time.After(90 * time.Second):
+		t.Fatal("timeout waiting for real grok result")
+	}
+}

server/pkg/agent/grok_test.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"log/slog"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -835,59 +834,3 @@ func TestGrokIsKnownThinkingValue(t *testing.T) {
 		}
 	}
 }
-
-// TestGrokRealACPSmoke drives the REAL `grok agent stdio` binary end-to-end
-// when it is installed and authenticated. Skipped automatically when grok is
-// not on PATH or the session cannot be created, so CI stays green.
-func TestGrokRealACPSmoke(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping real-binary smoke test in -short mode")
-	}
-	path, err := exec.LookPath("grok")
-	if err != nil {
-		t.Skip("grok not on PATH; skipping real-binary smoke test")
-	}
-	if version, err := exec.Command(path, "--version").CombinedOutput(); err == nil {
-		t.Logf("grok CLI version: %s", strings.TrimSpace(string(version)))
-	} else {
-		t.Logf("grok CLI version unavailable: %v (%s)", err, strings.TrimSpace(string(version)))
-	}
-
-	backend, err := New("grok", Config{ExecutablePath: path, Logger: slog.Default()})
-	if err != nil {
-		t.Fatalf("new grok backend: %v", err)
-	}
-	ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second)
-	defer cancel()
-
-	session, err := backend.Execute(ctx, "Reply with exactly one word: pong. Do not use any tools.", ExecOptions{
-		Cwd:     t.TempDir(),
-		Timeout: 80 * time.Second,
-	})
-	if err != nil {
-		t.Fatalf("execute: %v", err)
-	}
-	go func() {
-		for range session.Messages {
-		}
-	}()
-
-	select {
-	case result := <-session.Result:
-		if result.Status == "failed" && (strings.Contains(result.Error, "session/new") || strings.Contains(result.Error, "initialize")) {
-			t.Skipf("grok not authenticated or ACP unavailable: %v", result.Error)
-		}
-		if result.Status != "completed" {
-			t.Fatalf("real grok run did not complete: status=%q error=%q", result.Status, result.Error)
-		}
-		if !strings.Contains(strings.ToLower(result.Output), "pong") {
-			t.Fatalf("expected real grok output to contain 'pong', got %q", result.Output)
-		}
-		if result.SessionID == "" {
-			t.Error("expected a non-empty session id from real grok")
-		}
-		t.Logf("real grok smoke OK: session=%s output=%q", result.SessionID, result.Output)
-	case <-time.After(90 * time.Second):
-		t.Fatal("timeout waiting for real grok result")
-	}
-}

server/pkg/agent/models_test.go

@@ -9,28 +9,30 @@ import (
 	"testing"
 )
 
-func TestListModelsStaticProviders(t *testing.T) {
-	ctx := context.Background()
-	for _, provider := range []string{"claude", "codex", "cursor"} {
-		got, err := ListModels(ctx, provider, "")
-		if err != nil {
-			t.Fatalf("ListModels(%q) error: %v", provider, err)
+func TestStaticModelCatalogsHaveValidEntries(t *testing.T) {
+	t.Parallel()
+	catalogs := map[string][]Model{
+		"claude": claudeStaticModels(),
+		"codex":  codexStaticModels(),
+		"cursor": cursorStaticModels(),
+	}
+	for provider, models := range catalogs {
+		if len(models) == 0 {
+			t.Errorf("%s static catalog returned no models", provider)
 		}
-		if len(got) == 0 {
-			t.Errorf("ListModels(%q) returned no models", provider)
-		}
-		for i, m := range got {
-			if m.ID == "" {
-				t.Errorf("ListModels(%q)[%d] has empty ID", provider, i)
+		for i, model := range models {
+			if model.ID == "" {
+				t.Errorf("%s static catalog[%d] has empty ID", provider, i)
 			}
-			if m.Label == "" {
-				t.Errorf("ListModels(%q)[%d] has empty Label", provider, i)
+			if model.Label == "" {
+				t.Errorf("%s static catalog[%d] has empty Label", provider, i)
 			}
 		}
 	}
 }
 
 func TestListModelsQwenUsesRuntimeDefaultAndManualEntry(t *testing.T) {
+	// Qwen returns its manual-entry catalog without resolving or executing a CLI.
 	got, err := ListModels(context.Background(), "qwen", "")
 	if err != nil {
 		t.Fatalf("ListModels(qwen) error: %v", err)
@@ -51,7 +53,7 @@ func TestListModelsCopilotFallsBackToStatic(t *testing.T) {
 	delete(modelCache, "copilot")
 	modelCacheMu.Unlock()
 
-	got, err := ListModels(ctx, "copilot", "/nonexistent/copilot-cli")
+	got, err := ListModels(ctx, "copilot", missingAgentExecutable(t, "copilot"))
 	if err != nil {
 		t.Fatalf("ListModels(copilot) error: %v", err)
 	}
@@ -321,7 +323,7 @@ func TestListModelsHermesWithoutBinary(t *testing.T) {
 	delete(modelCache, "hermes")
 	modelCacheMu.Unlock()
 
-	got, err := ListModels(ctx, "hermes", "/nonexistent/hermes")
+	got, err := ListModels(ctx, "hermes", missingAgentExecutable(t, "hermes"))
 	if err != nil {
 		t.Fatalf("ListModels(hermes) error: %v", err)
 	}
@@ -336,7 +338,7 @@ func TestListModelsKiroWithoutBinary(t *testing.T) {
 	delete(modelCache, "kiro")
 	modelCacheMu.Unlock()
 
-	got, err := ListModels(ctx, "kiro", "/nonexistent/kiro-cli")
+	got, err := ListModels(ctx, "kiro", missingAgentExecutable(t, "kiro-cli"))
 	if err != nil {
 		t.Fatalf("ListModels(kiro) error: %v", err)
 	}
@@ -351,7 +353,7 @@ func TestListModelsQoderWithoutBinary(t *testing.T) {
 	delete(modelCache, "qoder")
 	modelCacheMu.Unlock()
 
-	got, err := ListModels(ctx, "qoder", "/nonexistent/qodercli")
+	got, err := ListModels(ctx, "qoder", missingAgentExecutable(t, "qodercli"))
 	if err != nil {
 		t.Fatalf("ListModels(qoder) error: %v", err)
 	}

server/pkg/agent/real_agent_smoke_integration_test.go

@@ -0,0 +1,16 @@
+//go:build agentintegration
+
+package agent
+
+import (
+	"os"
+	"testing"
+)
+
+func requireRealAgentSmoke(t *testing.T) {
+	t.Helper()
+	if os.Getenv("MULTICA_RUN_REAL_AGENT_SMOKE") != "1" {
+		t.Skip("set MULTICA_RUN_REAL_AGENT_SMOKE=1 to allow real agent CLI and account access")
+	}
+	t.Log("REAL AGENT SMOKE TEST: this test may access an authenticated account and consume quota")
+}

server/pkg/agent/traecli_integration_test.go

@@ -0,0 +1,59 @@
+//go:build agentintegration
+
+package agent
+
+import (
+	"context"
+	"log/slog"
+	"os/exec"
+	"strings"
+	"testing"
+	"time"
+)
+
+// TestTraecliRealACPSmoke drives the real `traecli acp serve` binary end-to-end.
+func TestTraecliRealACPSmoke(t *testing.T) {
+	requireRealAgentSmoke(t)
+	if testing.Short() {
+		t.Skip("skipping real-binary smoke test in -short mode")
+	}
+	path, err := exec.LookPath("traecli")
+	if err != nil {
+		t.Skip("traecli not on PATH; skipping real-binary smoke test")
+	}
+
+	backend, err := New("traecli", Config{ExecutablePath: path, Logger: slog.Default()})
+	if err != nil {
+		t.Fatalf("new traecli backend: %v", err)
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second)
+	defer cancel()
+
+	session, err := backend.Execute(ctx, "Reply with exactly one word: pong. Do not use any tools.", ExecOptions{
+		Cwd:     t.TempDir(),
+		Timeout: 80 * time.Second,
+	})
+	if err != nil {
+		t.Fatalf("execute: %v", err)
+	}
+	go func() {
+		for range session.Messages {
+		}
+	}()
+
+	select {
+	case result := <-session.Result:
+		if result.Status != "completed" {
+			t.Fatalf("real traecli run did not complete: status=%q error=%q", result.Status, result.Error)
+		}
+		if !strings.Contains(strings.ToLower(result.Output), "pong") {
+			t.Fatalf("expected real traecli output to contain 'pong', got %q", result.Output)
+		}
+		if result.SessionID == "" {
+			t.Error("expected a non-empty session id from real traecli")
+		}
+		t.Logf("real traecli smoke OK: session=%s output=%q", result.SessionID, result.Output)
+	case <-time.After(90 * time.Second):
+		t.Fatal("timeout waiting for real traecli result")
+	}
+}

server/pkg/agent/traecli_test.go

@@ -3,7 +3,6 @@ package agent
 import (
 	"context"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -288,61 +287,3 @@ func TestTraecliUsesSessionLoadForResume(t *testing.T) {
 		t.Fatalf("traecli must use session/load (loadSession:true), not session/resume:\n%s", requests)
 	}
 }
-
-// TestTraecliRealACPSmoke drives the REAL official `traecli acp serve` binary
-// end-to-end when it is installed and logged in. It is the live counterpart to
-// the fake-ACP tests above: it proves the backend's initialize → session/new →
-// session/prompt flow works against the actual binary and the user's account.
-//
-// Skipped automatically when traecli is not on PATH or the session cannot be
-// created (not logged in), so CI — which has neither — stays green. Run locally
-// with a logged-in traecli to exercise it.
-func TestTraecliRealACPSmoke(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping real-binary smoke test in -short mode")
-	}
-	path, err := exec.LookPath("traecli")
-	if err != nil {
-		t.Skip("traecli not on PATH; skipping real-binary smoke test")
-	}
-
-	backend, err := New("traecli", Config{ExecutablePath: path, Logger: slog.Default()})
-	if err != nil {
-		t.Fatalf("new traecli backend: %v", err)
-	}
-	ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second)
-	defer cancel()
-
-	session, err := backend.Execute(ctx, "Reply with exactly one word: pong. Do not use any tools.", ExecOptions{
-		Cwd:     t.TempDir(),
-		Timeout: 80 * time.Second,
-	})
-	if err != nil {
-		t.Fatalf("execute: %v", err)
-	}
-	go func() {
-		for range session.Messages {
-		}
-	}()
-
-	select {
-	case result := <-session.Result:
-		// "session/new" panics on a NOT-logged-in traecli (no models); treat
-		// that as a skip so the test only fails for real protocol regressions.
-		if result.Status == "failed" && strings.Contains(result.Error, "session/new") {
-			t.Skipf("traecli not logged in (session/new failed): %v", result.Error)
-		}
-		if result.Status != "completed" {
-			t.Fatalf("real traecli run did not complete: status=%q error=%q", result.Status, result.Error)
-		}
-		if !strings.Contains(strings.ToLower(result.Output), "pong") {
-			t.Fatalf("expected real traecli output to contain 'pong', got %q", result.Output)
-		}
-		if result.SessionID == "" {
-			t.Error("expected a non-empty session id from real traecli")
-		}
-		t.Logf("real traecli smoke OK: session=%s output=%q", result.SessionID, result.Output)
-	case <-time.After(90 * time.Second):
-		t.Fatal("timeout waiting for real traecli result")
-	}
-}