From 3904672c47330115be808a3ab68bf971f22ec602 Mon Sep 17 00:00:00 2001 From: OpSpawn Date: Thu, 12 Feb 2026 10:19:44 +0000 Subject: [PATCH 1/2] fix: correct recursion depth tracking in agent validation The tState.with() method mutated shared state (incrementing depth and appending to the same visitedAgents slice) instead of creating copies. This caused flat agent tool lists to incorrectly hit the recursion limit because each sibling tool incremented the same depth counter, making 10+ sibling agent tools appear as 10+ levels of nesting. Replace with copy-on-write semantics: with() now returns a new tState with an independent depth counter and a copied visitedAgents slice. Fixes #1287 Signed-off-by: opspawn Signed-off-by: OpSpawn --- .../translator/agent/adk_api_translator.go | 10 +- .../agent/adk_api_translator_test.go | 317 ++++++++++++++++++ 2 files changed, 324 insertions(+), 3 deletions(-) diff --git a/go/internal/controller/translator/agent/adk_api_translator.go b/go/internal/controller/translator/agent/adk_api_translator.go index eb76a108c..dbc6fd7dd 100644 --- a/go/internal/controller/translator/agent/adk_api_translator.go +++ b/go/internal/controller/translator/agent/adk_api_translator.go @@ -130,9 +130,13 @@ type tState struct { } func (s *tState) with(agent *v1alpha2.Agent) *tState { - s.depth++ - s.visitedAgents = append(s.visitedAgents, utils.GetObjectRef(agent)) - return s + visited := make([]string, len(s.visitedAgents), len(s.visitedAgents)+1) + copy(visited, s.visitedAgents) + visited = append(visited, utils.GetObjectRef(agent)) + return &tState{ + depth: s.depth + 1, + visitedAgents: visited, + } } func (t *tState) isVisited(agentName string) bool { diff --git a/go/internal/controller/translator/agent/adk_api_translator_test.go b/go/internal/controller/translator/agent/adk_api_translator_test.go index 32fc350e6..b7940d7e8 100644 --- a/go/internal/controller/translator/agent/adk_api_translator_test.go +++ b/go/internal/controller/translator/agent/adk_api_translator_test.go @@ -2,6 +2,7 @@ package agent_test import ( "context" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -596,3 +597,319 @@ func Test_AdkApiTranslator_ServiceAccountNameOverride(t *testing.T) { }) } } + +// Test_AdkApiTranslator_RecursionDepthTracking validates that the with() method +// correctly tracks nesting depth independently per branch, fixing issue #1287 +// where shared state mutation caused flat agent tool lists to hit the recursion limit. +func Test_AdkApiTranslator_RecursionDepthTracking(t *testing.T) { + scheme := schemev1.Scheme + require.NoError(t, v1alpha2.AddToScheme(scheme)) + + namespace := "default" + + // Helper: create a leaf agent (no sub-agent tools) + leafAgent := func(name string) *v1alpha2.Agent { + return &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Leaf agent " + name, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are " + name, + ModelConfig: "test-model", + }, + }, + } + } + + modelConfig := &v1alpha2.ModelConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-model", + Namespace: namespace, + }, + Spec: v1alpha2.ModelConfigSpec{ + Model: "gpt-4", + Provider: v1alpha2.ModelProviderOpenAI, + }, + } + + defaultModel := types.NamespacedName{ + Namespace: namespace, + Name: "test-model", + } + + t.Run("flat list of 12 agent tools should pass", func(t *testing.T) { + // Root agent references 12 leaf agents as tools (all siblings, depth=1). + // Before the fix, this would fail because with() mutated shared state, + // incrementing depth for each sibling instead of each nesting level. + var leafAgents [](*v1alpha2.Agent) + var tools []*v1alpha2.Tool + for i := 0; i < 12; i++ { + name := fmt.Sprintf("leaf-%02d", i) + leafAgents = append(leafAgents, leafAgent(name)) + tools = append(tools, &v1alpha2.Tool{ + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: name, + }, + }) + } + + root := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "root", + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Root agent", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are root", + ModelConfig: "test-model", + Tools: tools, + }, + }, + } + + builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(modelConfig, root) + for _, la := range leafAgents { + builder = builder.WithObjects(la) + } + kubeClient := builder.Build() + + trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "") + _, err := trans.TranslateAgent(context.Background(), root) + require.NoError(t, err, "flat list of 12 agent tools should not hit recursion limit") + }) + + t.Run("deep nesting of 10 levels should pass", func(t *testing.T) { + // Chain: chain-0 -> chain-1 -> ... -> chain-9 (leaf) + // Depth from root's perspective: chain-0 calls validateAgent on chain-1 at depth=1, etc. + // chain-9 is validated at depth=9 which is <= MAX_DEPTH (10). + agents := make([]*v1alpha2.Agent, 10) + for i := 0; i < 10; i++ { + name := fmt.Sprintf("chain-%d", i) + agents[i] = &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Chain agent " + name, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are " + name, + ModelConfig: "test-model", + }, + }, + } + if i < 9 { + agents[i].Spec.Declarative.Tools = []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: fmt.Sprintf("chain-%d", i+1), + }, + }, + } + } + } + + builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(modelConfig) + for _, a := range agents { + builder = builder.WithObjects(a) + } + kubeClient := builder.Build() + + trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "") + _, err := trans.TranslateAgent(context.Background(), agents[0]) + require.NoError(t, err, "deep nesting of 10 levels should pass") + }) + + t.Run("deep nesting of 12 levels should fail with recursion limit", func(t *testing.T) { + // Chain: deep-0 -> deep-1 -> ... -> deep-11 (leaf) + // deep-11 is validated at depth=11 which exceeds MAX_DEPTH (10). + agents := make([]*v1alpha2.Agent, 12) + for i := 0; i < 12; i++ { + name := fmt.Sprintf("deep-%d", i) + agents[i] = &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Deep agent " + name, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are " + name, + ModelConfig: "test-model", + }, + }, + } + if i < 11 { + agents[i].Spec.Declarative.Tools = []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: fmt.Sprintf("deep-%d", i+1), + }, + }, + } + } + } + + builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(modelConfig) + for _, a := range agents { + builder = builder.WithObjects(a) + } + kubeClient := builder.Build() + + trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "") + _, err := trans.TranslateAgent(context.Background(), agents[0]) + require.Error(t, err, "deep nesting of 12 levels should fail") + assert.Contains(t, err.Error(), "recursion limit reached") + }) + + t.Run("true cycle A->B->A should fail with cycle detection", func(t *testing.T) { + agentA := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cycle-a", + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent A", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are A", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "cycle-b", + }, + }, + }, + }, + }, + } + agentB := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cycle-b", + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent B", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are B", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "cycle-a", + }, + }, + }, + }, + }, + } + + kubeClient := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(modelConfig, agentA, agentB).Build() + + trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "") + _, err := trans.TranslateAgent(context.Background(), agentA) + require.Error(t, err, "cycle A->B->A should be detected") + assert.Contains(t, err.Error(), "cycle detected") + }) + + t.Run("diamond pattern A->B,C B->D C->D should pass", func(t *testing.T) { + // A has tools B and C. B has tool D. C has tool D. + // D is visited twice but via different branches — this is NOT a cycle. + agentD := leafAgent("diamond-d") + agentB := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "diamond-b", + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent B", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are B", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "diamond-d", + }, + }, + }, + }, + }, + } + agentC := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "diamond-c", + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent C", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are C", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "diamond-d", + }, + }, + }, + }, + }, + } + agentA := &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "diamond-a", + Namespace: namespace, + }, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Description: "Agent A", + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "You are A", + ModelConfig: "test-model", + Tools: []*v1alpha2.Tool{ + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "diamond-b", + }, + }, + { + Type: v1alpha2.ToolProviderType_Agent, + Agent: &v1alpha2.TypedLocalReference{ + Name: "diamond-c", + }, + }, + }, + }, + }, + } + + kubeClient := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(modelConfig, agentA, agentB, agentC, agentD).Build() + + trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "") + _, err := trans.TranslateAgent(context.Background(), agentA) + require.NoError(t, err, "diamond pattern should pass — D is not a cycle, just shared") + }) +} From 7d98e36c4f1c0475b68d79507b83c413676149c0 Mon Sep 17 00:00:00 2001 From: OpSpawn Date: Thu, 12 Feb 2026 15:15:15 +0000 Subject: [PATCH 2/2] fix: modernize for-loops to range-over-int syntax (go-lint) Co-Authored-By: Claude Opus 4.6 Signed-off-by: OpSpawn --- .../controller/translator/agent/adk_api_translator_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/internal/controller/translator/agent/adk_api_translator_test.go b/go/internal/controller/translator/agent/adk_api_translator_test.go index b7940d7e8..7fa2fb345 100644 --- a/go/internal/controller/translator/agent/adk_api_translator_test.go +++ b/go/internal/controller/translator/agent/adk_api_translator_test.go @@ -647,7 +647,7 @@ func Test_AdkApiTranslator_RecursionDepthTracking(t *testing.T) { // incrementing depth for each sibling instead of each nesting level. var leafAgents [](*v1alpha2.Agent) var tools []*v1alpha2.Tool - for i := 0; i < 12; i++ { + for i := range 12 { name := fmt.Sprintf("leaf-%02d", i) leafAgents = append(leafAgents, leafAgent(name)) tools = append(tools, &v1alpha2.Tool{ @@ -690,7 +690,7 @@ func Test_AdkApiTranslator_RecursionDepthTracking(t *testing.T) { // Depth from root's perspective: chain-0 calls validateAgent on chain-1 at depth=1, etc. // chain-9 is validated at depth=9 which is <= MAX_DEPTH (10). agents := make([]*v1alpha2.Agent, 10) - for i := 0; i < 10; i++ { + for i := range 10 { name := fmt.Sprintf("chain-%d", i) agents[i] = &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{ @@ -733,7 +733,7 @@ func Test_AdkApiTranslator_RecursionDepthTracking(t *testing.T) { // Chain: deep-0 -> deep-1 -> ... -> deep-11 (leaf) // deep-11 is validated at depth=11 which exceeds MAX_DEPTH (10). agents := make([]*v1alpha2.Agent, 12) - for i := 0; i < 12; i++ { + for i := range 12 { name := fmt.Sprintf("deep-%d", i) agents[i] = &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{