Skip to content

Commit a7fe733

Browse files
agentzeroOpSpawnclaudeEItanya
authored
fix: correct recursion depth tracking in agent validation (#1288)
## Summary Fixes #1287 The `tState.with()` method in `adk_api_translator.go` mutated shared state instead of creating independent copies. This caused two bugs: 1. **Depth counter counted siblings instead of nesting depth**: Each call to `with()` incremented the same `depth` field on the shared `tState`, so an agent with 11+ flat sibling agent tools would hit the `MAX_DEPTH=10` recursion limit even though the actual nesting depth was only 1. 2. **Visited agents list leaked across branches**: `append()` on the shared slice meant that agents visited in one branch of the tool tree could appear as "visited" in unrelated branches, potentially causing false cycle detection in diamond-shaped agent graphs. ### The Fix Replace the mutating `with()` method with a copy-on-write version that returns a new `tState` with an independent depth counter and a copied `visitedAgents` slice. ### Test Cases Added - **Flat list of 12 agent tools** (all leaf agents) — PASSES (was failing before fix) - **Deep nesting of 10 levels** — PASSES (within limit) - **Deep nesting of 12 levels** — correctly FAILS with recursion error - **True cycle A->B->A** — correctly FAILS with cycle detection - **Diamond pattern A->B, A->C, B->D, C->D** — PASSES (shared agent D is not a cycle) ## Test plan - [x] All new test cases pass - [x] All existing tests pass (golden test failures are pre-existing on main, unrelated to this change) - [x] Diff is minimal — only `with()` method changed + tests added --------- Signed-off-by: opspawn <opspawnhq@gmail.com> Signed-off-by: OpSpawn <opspawn@users.noreply.github.com> Co-authored-by: OpSpawn <opspawn@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Eitan Yarmush <eitan.yarmush@solo.io>
1 parent dce1702 commit a7fe733

2 files changed

Lines changed: 324 additions & 3 deletions

File tree

‎go/internal/controller/translator/agent/adk_api_translator.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,13 @@ type tState struct {
130130
}
131131

132132
func (s *tState) with(agent *v1alpha2.Agent) *tState {
133-
s.depth++
134-
s.visitedAgents = append(s.visitedAgents, utils.GetObjectRef(agent))
135-
return s
133+
visited := make([]string, len(s.visitedAgents), len(s.visitedAgents)+1)
134+
copy(visited, s.visitedAgents)
135+
visited = append(visited, utils.GetObjectRef(agent))
136+
return &tState{
137+
depth: s.depth + 1,
138+
visitedAgents: visited,
139+
}
136140
}
137141

138142
func (t *tState) isVisited(agentName string) bool {

‎go/internal/controller/translator/agent/adk_api_translator_test.go‎

Lines changed: 317 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package agent_test
22

33
import (
44
"context"
5+
"fmt"
56
"testing"
67

78
"github.com/stretchr/testify/assert"
@@ -596,3 +597,319 @@ func Test_AdkApiTranslator_ServiceAccountNameOverride(t *testing.T) {
596597
})
597598
}
598599
}
600+
601+
// Test_AdkApiTranslator_RecursionDepthTracking validates that the with() method
602+
// correctly tracks nesting depth independently per branch, fixing issue #1287
603+
// where shared state mutation caused flat agent tool lists to hit the recursion limit.
604+
func Test_AdkApiTranslator_RecursionDepthTracking(t *testing.T) {
605+
scheme := schemev1.Scheme
606+
require.NoError(t, v1alpha2.AddToScheme(scheme))
607+
608+
namespace := "default"
609+
610+
// Helper: create a leaf agent (no sub-agent tools)
611+
leafAgent := func(name string) *v1alpha2.Agent {
612+
return &v1alpha2.Agent{
613+
ObjectMeta: metav1.ObjectMeta{
614+
Name: name,
615+
Namespace: namespace,
616+
},
617+
Spec: v1alpha2.AgentSpec{
618+
Type: v1alpha2.AgentType_Declarative,
619+
Description: "Leaf agent " + name,
620+
Declarative: &v1alpha2.DeclarativeAgentSpec{
621+
SystemMessage: "You are " + name,
622+
ModelConfig: "test-model",
623+
},
624+
},
625+
}
626+
}
627+
628+
modelConfig := &v1alpha2.ModelConfig{
629+
ObjectMeta: metav1.ObjectMeta{
630+
Name: "test-model",
631+
Namespace: namespace,
632+
},
633+
Spec: v1alpha2.ModelConfigSpec{
634+
Model: "gpt-4",
635+
Provider: v1alpha2.ModelProviderOpenAI,
636+
},
637+
}
638+
639+
defaultModel := types.NamespacedName{
640+
Namespace: namespace,
641+
Name: "test-model",
642+
}
643+
644+
t.Run("flat list of 12 agent tools should pass", func(t *testing.T) {
645+
// Root agent references 12 leaf agents as tools (all siblings, depth=1).
646+
// Before the fix, this would fail because with() mutated shared state,
647+
// incrementing depth for each sibling instead of each nesting level.
648+
var leafAgents [](*v1alpha2.Agent)
649+
var tools []*v1alpha2.Tool
650+
for i := range 12 {
651+
name := fmt.Sprintf("leaf-%02d", i)
652+
leafAgents = append(leafAgents, leafAgent(name))
653+
tools = append(tools, &v1alpha2.Tool{
654+
Type: v1alpha2.ToolProviderType_Agent,
655+
Agent: &v1alpha2.TypedLocalReference{
656+
Name: name,
657+
},
658+
})
659+
}
660+
661+
root := &v1alpha2.Agent{
662+
ObjectMeta: metav1.ObjectMeta{
663+
Name: "root",
664+
Namespace: namespace,
665+
},
666+
Spec: v1alpha2.AgentSpec{
667+
Type: v1alpha2.AgentType_Declarative,
668+
Description: "Root agent",
669+
Declarative: &v1alpha2.DeclarativeAgentSpec{
670+
SystemMessage: "You are root",
671+
ModelConfig: "test-model",
672+
Tools: tools,
673+
},
674+
},
675+
}
676+
677+
builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(modelConfig, root)
678+
for _, la := range leafAgents {
679+
builder = builder.WithObjects(la)
680+
}
681+
kubeClient := builder.Build()
682+
683+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
684+
_, err := trans.TranslateAgent(context.Background(), root)
685+
require.NoError(t, err, "flat list of 12 agent tools should not hit recursion limit")
686+
})
687+
688+
t.Run("deep nesting of 10 levels should pass", func(t *testing.T) {
689+
// Chain: chain-0 -> chain-1 -> ... -> chain-9 (leaf)
690+
// Depth from root's perspective: chain-0 calls validateAgent on chain-1 at depth=1, etc.
691+
// chain-9 is validated at depth=9 which is <= MAX_DEPTH (10).
692+
agents := make([]*v1alpha2.Agent, 10)
693+
for i := range 10 {
694+
name := fmt.Sprintf("chain-%d", i)
695+
agents[i] = &v1alpha2.Agent{
696+
ObjectMeta: metav1.ObjectMeta{
697+
Name: name,
698+
Namespace: namespace,
699+
},
700+
Spec: v1alpha2.AgentSpec{
701+
Type: v1alpha2.AgentType_Declarative,
702+
Description: "Chain agent " + name,
703+
Declarative: &v1alpha2.DeclarativeAgentSpec{
704+
SystemMessage: "You are " + name,
705+
ModelConfig: "test-model",
706+
},
707+
},
708+
}
709+
if i < 9 {
710+
agents[i].Spec.Declarative.Tools = []*v1alpha2.Tool{
711+
{
712+
Type: v1alpha2.ToolProviderType_Agent,
713+
Agent: &v1alpha2.TypedLocalReference{
714+
Name: fmt.Sprintf("chain-%d", i+1),
715+
},
716+
},
717+
}
718+
}
719+
}
720+
721+
builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(modelConfig)
722+
for _, a := range agents {
723+
builder = builder.WithObjects(a)
724+
}
725+
kubeClient := builder.Build()
726+
727+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
728+
_, err := trans.TranslateAgent(context.Background(), agents[0])
729+
require.NoError(t, err, "deep nesting of 10 levels should pass")
730+
})
731+
732+
t.Run("deep nesting of 12 levels should fail with recursion limit", func(t *testing.T) {
733+
// Chain: deep-0 -> deep-1 -> ... -> deep-11 (leaf)
734+
// deep-11 is validated at depth=11 which exceeds MAX_DEPTH (10).
735+
agents := make([]*v1alpha2.Agent, 12)
736+
for i := range 12 {
737+
name := fmt.Sprintf("deep-%d", i)
738+
agents[i] = &v1alpha2.Agent{
739+
ObjectMeta: metav1.ObjectMeta{
740+
Name: name,
741+
Namespace: namespace,
742+
},
743+
Spec: v1alpha2.AgentSpec{
744+
Type: v1alpha2.AgentType_Declarative,
745+
Description: "Deep agent " + name,
746+
Declarative: &v1alpha2.DeclarativeAgentSpec{
747+
SystemMessage: "You are " + name,
748+
ModelConfig: "test-model",
749+
},
750+
},
751+
}
752+
if i < 11 {
753+
agents[i].Spec.Declarative.Tools = []*v1alpha2.Tool{
754+
{
755+
Type: v1alpha2.ToolProviderType_Agent,
756+
Agent: &v1alpha2.TypedLocalReference{
757+
Name: fmt.Sprintf("deep-%d", i+1),
758+
},
759+
},
760+
}
761+
}
762+
}
763+
764+
builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(modelConfig)
765+
for _, a := range agents {
766+
builder = builder.WithObjects(a)
767+
}
768+
kubeClient := builder.Build()
769+
770+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
771+
_, err := trans.TranslateAgent(context.Background(), agents[0])
772+
require.Error(t, err, "deep nesting of 12 levels should fail")
773+
assert.Contains(t, err.Error(), "recursion limit reached")
774+
})
775+
776+
t.Run("true cycle A->B->A should fail with cycle detection", func(t *testing.T) {
777+
agentA := &v1alpha2.Agent{
778+
ObjectMeta: metav1.ObjectMeta{
779+
Name: "cycle-a",
780+
Namespace: namespace,
781+
},
782+
Spec: v1alpha2.AgentSpec{
783+
Type: v1alpha2.AgentType_Declarative,
784+
Description: "Agent A",
785+
Declarative: &v1alpha2.DeclarativeAgentSpec{
786+
SystemMessage: "You are A",
787+
ModelConfig: "test-model",
788+
Tools: []*v1alpha2.Tool{
789+
{
790+
Type: v1alpha2.ToolProviderType_Agent,
791+
Agent: &v1alpha2.TypedLocalReference{
792+
Name: "cycle-b",
793+
},
794+
},
795+
},
796+
},
797+
},
798+
}
799+
agentB := &v1alpha2.Agent{
800+
ObjectMeta: metav1.ObjectMeta{
801+
Name: "cycle-b",
802+
Namespace: namespace,
803+
},
804+
Spec: v1alpha2.AgentSpec{
805+
Type: v1alpha2.AgentType_Declarative,
806+
Description: "Agent B",
807+
Declarative: &v1alpha2.DeclarativeAgentSpec{
808+
SystemMessage: "You are B",
809+
ModelConfig: "test-model",
810+
Tools: []*v1alpha2.Tool{
811+
{
812+
Type: v1alpha2.ToolProviderType_Agent,
813+
Agent: &v1alpha2.TypedLocalReference{
814+
Name: "cycle-a",
815+
},
816+
},
817+
},
818+
},
819+
},
820+
}
821+
822+
kubeClient := fake.NewClientBuilder().WithScheme(scheme).
823+
WithObjects(modelConfig, agentA, agentB).Build()
824+
825+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
826+
_, err := trans.TranslateAgent(context.Background(), agentA)
827+
require.Error(t, err, "cycle A->B->A should be detected")
828+
assert.Contains(t, err.Error(), "cycle detected")
829+
})
830+
831+
t.Run("diamond pattern A->B,C B->D C->D should pass", func(t *testing.T) {
832+
// A has tools B and C. B has tool D. C has tool D.
833+
// D is visited twice but via different branches — this is NOT a cycle.
834+
agentD := leafAgent("diamond-d")
835+
agentB := &v1alpha2.Agent{
836+
ObjectMeta: metav1.ObjectMeta{
837+
Name: "diamond-b",
838+
Namespace: namespace,
839+
},
840+
Spec: v1alpha2.AgentSpec{
841+
Type: v1alpha2.AgentType_Declarative,
842+
Description: "Agent B",
843+
Declarative: &v1alpha2.DeclarativeAgentSpec{
844+
SystemMessage: "You are B",
845+
ModelConfig: "test-model",
846+
Tools: []*v1alpha2.Tool{
847+
{
848+
Type: v1alpha2.ToolProviderType_Agent,
849+
Agent: &v1alpha2.TypedLocalReference{
850+
Name: "diamond-d",
851+
},
852+
},
853+
},
854+
},
855+
},
856+
}
857+
agentC := &v1alpha2.Agent{
858+
ObjectMeta: metav1.ObjectMeta{
859+
Name: "diamond-c",
860+
Namespace: namespace,
861+
},
862+
Spec: v1alpha2.AgentSpec{
863+
Type: v1alpha2.AgentType_Declarative,
864+
Description: "Agent C",
865+
Declarative: &v1alpha2.DeclarativeAgentSpec{
866+
SystemMessage: "You are C",
867+
ModelConfig: "test-model",
868+
Tools: []*v1alpha2.Tool{
869+
{
870+
Type: v1alpha2.ToolProviderType_Agent,
871+
Agent: &v1alpha2.TypedLocalReference{
872+
Name: "diamond-d",
873+
},
874+
},
875+
},
876+
},
877+
},
878+
}
879+
agentA := &v1alpha2.Agent{
880+
ObjectMeta: metav1.ObjectMeta{
881+
Name: "diamond-a",
882+
Namespace: namespace,
883+
},
884+
Spec: v1alpha2.AgentSpec{
885+
Type: v1alpha2.AgentType_Declarative,
886+
Description: "Agent A",
887+
Declarative: &v1alpha2.DeclarativeAgentSpec{
888+
SystemMessage: "You are A",
889+
ModelConfig: "test-model",
890+
Tools: []*v1alpha2.Tool{
891+
{
892+
Type: v1alpha2.ToolProviderType_Agent,
893+
Agent: &v1alpha2.TypedLocalReference{
894+
Name: "diamond-b",
895+
},
896+
},
897+
{
898+
Type: v1alpha2.ToolProviderType_Agent,
899+
Agent: &v1alpha2.TypedLocalReference{
900+
Name: "diamond-c",
901+
},
902+
},
903+
},
904+
},
905+
},
906+
}
907+
908+
kubeClient := fake.NewClientBuilder().WithScheme(scheme).
909+
WithObjects(modelConfig, agentA, agentB, agentC, agentD).Build()
910+
911+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
912+
_, err := trans.TranslateAgent(context.Background(), agentA)
913+
require.NoError(t, err, "diamond pattern should pass — D is not a cycle, just shared")
914+
})
915+
}

0 commit comments

Comments
 (0)