[quality] add unit tests for 6 untested compat/delegation files in pkg/agent

pkg/agent/config_compat_test.go

@@ -0,0 +1,100 @@
+package agent
+
+import (
+	"testing"
+)
+
+func TestGetConfigManager(t *testing.T) {
+	// GetConfigManager should return a non-nil *ConfigManager singleton
+	cm := GetConfigManager()
+	if cm == nil {
+		t.Fatal("GetConfigManager() returned nil")
+	}
+
+	// Calling again should return the same singleton
+	cm2 := GetConfigManager()
+	if cm != cm2 {
+		t.Error("GetConfigManager() did not return the same singleton on second call")
+	}
+}
+
+func TestGetBaseURLEnvKeyForProvider(t *testing.T) {
+	tests := []struct {
+		provider string
+		wantNon  bool // expect non-empty result
+	}{
+		{"ollama", true},
+		{"llamacpp", true},
+		{"localai", true},
+		{"vllm", true},
+		{"lmstudio", true},
+		{"nonexistent-provider", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.provider, func(t *testing.T) {
+			got := getBaseURLEnvKeyForProvider(tt.provider)
+			if tt.wantNon && got == "" {
+				t.Errorf("getBaseURLEnvKeyForProvider(%q) = empty, want non-empty", tt.provider)
+			}
+			if !tt.wantNon && got != "" {
+				t.Errorf("getBaseURLEnvKeyForProvider(%q) = %q, want empty", tt.provider, got)
+			}
+		})
+	}
+}
+
+func TestGetEnvKeyForProvider(t *testing.T) {
+	tests := []struct {
+		provider string
+		wantNon  bool
+	}{
+		{"claude", true},
+		{"openai", true},
+		{"gemini", true},
+		{"nonexistent-provider", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.provider, func(t *testing.T) {
+			got := getEnvKeyForProvider(tt.provider)
+			if tt.wantNon && got == "" {
+				t.Errorf("getEnvKeyForProvider(%q) = empty, want non-empty", tt.provider)
+			}
+			if !tt.wantNon && got != "" {
+				t.Errorf("getEnvKeyForProvider(%q) = %q, want empty", tt.provider, got)
+			}
+		})
+	}
+}
+
+func TestGetModelEnvKeyForProvider(t *testing.T) {
+	tests := []struct {
+		provider string
+		wantNon  bool
+	}{
+		{"ollama", true},
+		{"openai", true},
+		{"nonexistent-provider", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.provider, func(t *testing.T) {
+			got := getModelEnvKeyForProvider(tt.provider)
+			if tt.wantNon && got == "" {
+				t.Errorf("getModelEnvKeyForProvider(%q) = empty, want non-empty", tt.provider)
+			}
+			if !tt.wantNon && got != "" {
+				t.Errorf("getModelEnvKeyForProvider(%q) = %q, want empty", tt.provider, got)
+			}
+		})
+	}
+}
+
+func TestIsolateConfigManager(t *testing.T) {
+	// isolateConfigManager should return a non-nil *ConfigManager for test isolation
+	cm := isolateConfigManager(t)
+	if cm == nil {
+		t.Fatal("isolateConfigManager(t) returned nil")
+	}
+}

pkg/agent/kagent_compat_test.go

@@ -0,0 +1,62 @@
+package agent
+
+import (
+	"testing"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+func TestKagentGVRsNonEmpty(t *testing.T) {
+	// All kagent GVR delegations should resolve to non-empty GroupVersionResource
+	gvrs := map[string]schema.GroupVersionResource{
+		"agentGVR":               agentGVR,
+		"modelConfigGVR":         modelConfigGVR,
+		"modelProviderConfigGVR": modelProviderConfigGVR,
+		"toolServerGVR":          toolServerGVR,
+		"remoteMCPServerGVR":     remoteMCPServerGVR,
+		"memoryGVR":              memoryGVR,
+	}
+
+	for name, gvr := range gvrs {
+		t.Run(name, func(t *testing.T) {
+			if gvr.Group == "" {
+				t.Errorf("%s has empty Group", name)
+			}
+			if gvr.Version == "" {
+				t.Errorf("%s has empty Version", name)
+			}
+			if gvr.Resource == "" {
+				t.Errorf("%s has empty Resource", name)
+			}
+		})
+	}
+}
+
+func TestKagentTypeAliases(t *testing.T) {
+	// Verify type aliases compile and can hold zero values
+	var agent kagentCRDAgent
+	if agent.Name != "" {
+		t.Error("zero-value kagentCRDAgent should have empty Name")
+	}
+
+	var tool kagentCRDTool
+	_ = tool
+
+	var model kagentCRDModel
+	_ = model
+
+	var memory kagentCRDMemory
+	_ = memory
+
+	var iAgent kagentiAgent
+	_ = iAgent
+
+	var build kagentiBuild
+	_ = build
+
+	var card kagentiCard
+	_ = card
+
+	var iTool kagentiTool
+	_ = iTool
+}

pkg/agent/procgroup_compat_test.go

@@ -0,0 +1,26 @@
+package agent
+
+import (
+	"os/exec"
+	"testing"
+)
+
+func TestConfigureProcessGroup(t *testing.T) {
+	// configureProcessGroup should not panic on a valid Cmd
+	cmd := exec.Command("echo", "test")
+	configureProcessGroup(cmd)
+
+	// Verify SysProcAttr was set (on Unix, this sets Setpgid)
+	if cmd.SysProcAttr == nil {
+		t.Error("configureProcessGroup did not set SysProcAttr")
+	}
+}
+
+func TestConfigureProcessGroupNilFields(t *testing.T) {
+	// Should handle a freshly created Cmd without panicking
+	cmd := &exec.Cmd{Path: "/bin/echo"}
+	configureProcessGroup(cmd)
+	if cmd.SysProcAttr == nil {
+		t.Error("configureProcessGroup did not set SysProcAttr on bare Cmd")
+	}
+}

pkg/agent/provider_test.go

@@ -0,0 +1,72 @@
+package agent
+
+import (
+	"testing"
+)
+
+func TestProviderTypeAliases(t *testing.T) {
+	// Verify the capability constants are correctly re-exported
+	if CapabilityChat == 0 {
+		t.Error("CapabilityChat should be non-zero")
+	}
+	if CapabilityToolExec == 0 {
+		t.Error("CapabilityToolExec should be non-zero")
+	}
+	if CapabilityChat == CapabilityToolExec {
+		t.Error("CapabilityChat and CapabilityToolExec should be distinct")
+	}
+}
+
+func TestMaxStderrBytes(t *testing.T) {
+	// maxStderrBytes should be 1MB
+	if maxStderrBytes != 1<<20 {
+		t.Errorf("maxStderrBytes = %d, want %d (1MB)", maxStderrBytes, 1<<20)
+	}
+}
+
+func TestMixedModeConfigZeroValue(t *testing.T) {
+	// MixedModeConfig should be usable with zero values
+	var cfg MixedModeConfig
+	if cfg.Enabled {
+		t.Error("zero-value MixedModeConfig should have Enabled=false")
+	}
+	if cfg.ThinkingAgent != "" {
+		t.Error("zero-value MixedModeConfig should have empty ThinkingAgent")
+	}
+	if cfg.ExecutionAgent != "" {
+		t.Error("zero-value MixedModeConfig should have empty ExecutionAgent")
+	}
+}
+
+func TestMixedModeConfigFields(t *testing.T) {
+	cfg := MixedModeConfig{
+		ThinkingAgent:  "claude",
+		ExecutionAgent: "codex",
+		Enabled:        true,
+	}
+	if cfg.ThinkingAgent != "claude" {
+		t.Errorf("ThinkingAgent = %q, want %q", cfg.ThinkingAgent, "claude")
+	}
+	if cfg.ExecutionAgent != "codex" {
+		t.Errorf("ExecutionAgent = %q, want %q", cfg.ExecutionAgent, "codex")
+	}
+	if !cfg.Enabled {
+		t.Error("Enabled should be true")
+	}
+}
+
+func TestChatRequestZeroValue(t *testing.T) {
+	// ChatRequest type alias should compile and instantiate
+	var req ChatRequest
+	if req.Messages != nil {
+		t.Error("zero-value ChatRequest should have nil Messages")
+	}
+}
+
+func TestChatResponseZeroValue(t *testing.T) {
+	// ChatResponse type alias should compile and instantiate
+	var resp ChatResponse
+	if resp.Content != "" {
+		t.Error("zero-value ChatResponse should have empty Content")
+	}
+}

pkg/agent/providers_compat_test.go

@@ -0,0 +1,105 @@
+package agent
+
+import (
+	"testing"
+	"time"
+)
+
+func TestProviderConstants(t *testing.T) {
+	// Verify re-exported provider key constants are non-empty
+	constants := map[string]string{
+		"ProviderKeyOllama":   ProviderKeyOllama,
+		"ProviderKeyLlamaCpp": ProviderKeyLlamaCpp,
+		"ProviderKeyLocalAI":  ProviderKeyLocalAI,
+		"ProviderKeyVLLM":     ProviderKeyVLLM,
+		"ProviderKeyLMStudio": ProviderKeyLMStudio,
+		"ProviderKeyRHAIIS":   ProviderKeyRHAIIS,
+	}
+
+	for name, val := range constants {
+		if val == "" {
+			t.Errorf("constant %s is empty", name)
+		}
+	}
+}
+
+func TestProviderVars(t *testing.T) {
+	// Verify re-exported URL defaults are non-empty
+	if defaultOllamaURL == "" {
+		t.Error("defaultOllamaURL is empty")
+	}
+	if defaultLMStudioURL == "" {
+		t.Error("defaultLMStudioURL is empty")
+	}
+	if claudeAPIVersion == "" {
+		t.Error("claudeAPIVersion is empty")
+	}
+	if geminiAPIBaseURL == "" {
+		t.Error("geminiAPIBaseURL is empty")
+	}
+}
+
+func TestGroqValidationURL(t *testing.T) {
+	url := groqValidationURL()
+	if url == "" {
+		t.Error("groqValidationURL() returned empty string")
+	}
+	if len(url) < 10 {
+		t.Errorf("groqValidationURL() too short: %q", url)
+	}
+}
+
+func TestTruncateString(t *testing.T) {
+	tests := []struct {
+		name   string
+		input  string
+		maxLen int
+		want   string
+	}{
+		{"empty", "", 10, ""},
+		{"within limit", "hello", 10, "hello"},
+		{"at limit", "hello", 5, "hello"},
+		{"over limit", "hello world", 5, "hello"},
+		{"zero max", "hello", 0, ""},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := truncateString(tt.input, tt.maxLen)
+			if got != tt.want {
+				t.Errorf("truncateString(%q, %d) = %q, want %q", tt.input, tt.maxLen, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestNewRestrictedAIProviderHTTPClient(t *testing.T) {
+	client := newRestrictedAIProviderHTTPClient(30 * time.Second)
+	if client == nil {
+		t.Fatal("newRestrictedAIProviderHTTPClient returned nil")
+	}
+	if client.Timeout != 30*time.Second {
+		t.Errorf("expected timeout 30s, got %v", client.Timeout)
+	}
+
+	// Verify transport has restricted dial (doesn't allow private networks)
+	if client.Transport == nil {
+		t.Error("expected non-nil Transport with restricted dialer")
+	}
+}
+
+func TestMaxLLMResponseBytes(t *testing.T) {
+	// Should be a reasonable value (> 0, < 100MB)
+	if maxLLMResponseBytes <= 0 {
+		t.Error("maxLLMResponseBytes should be > 0")
+	}
+	if maxLLMResponseBytes > 100*1024*1024 {
+		t.Errorf("maxLLMResponseBytes unexpectedly large: %d", maxLLMResponseBytes)
+	}
+}
+
+func TestSetAllowLoopbackForTests(t *testing.T) {
+	// Should not panic
+	SetAllowLoopbackForTests(true)
+	SetAllowLoopbackForTests(false)
+}