diff --git a/agent-schema.json b/agent-schema.json index 21864e933..cd14a43fe 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -157,14 +157,14 @@ }, "sub_agents": { "type": "array", - "description": "List of sub-agents. Can be names of agents defined in this config or external references (OCI images like 'namespace/repo' or URLs).", + "description": "List of sub-agents. Can be names of agents defined in this config, external references (OCI images like 'namespace/repo' or URLs), or named external references using 'name:reference' syntax (e.g. 'reviewer:agentcatalog/review-pr'). External agents without an explicit name are named after their last path segment.", "items": { "type": "string" } }, "handoffs": { "type": "array", - "description": "List of agents this agent can hand off the conversation to. Can be names of agents defined in this config or external references (OCI images like 'namespace/repo' or URLs).", + "description": "List of agents this agent can hand off the conversation to. Can be names of agents defined in this config, external references (OCI images like 'namespace/repo' or URLs), or named external references using 'name:reference' syntax (e.g. 'reviewer:agentcatalog/review-pr'). External agents without an explicit name are named after their last path segment.", "items": { "type": "string" } diff --git a/examples/sub-agents-from-catalog.yaml b/examples/sub-agents-from-catalog.yaml index dcd3bf3b9..fd4b394f4 100644 --- a/examples/sub-agents-from-catalog.yaml +++ b/examples/sub-agents-from-catalog.yaml @@ -3,6 +3,11 @@ # This example demonstrates using agents from the catalog as sub-agents. # Sub-agents can be defined locally in the same config, or referenced from # external sources such as OCI registries (e.g., the Docker agent catalog). +# +# External sub-agents are automatically named after their last path segment +# (e.g., "agentcatalog/pirate" becomes "pirate"). You can also give them +# an explicit name using the "name:reference" syntax: +# - reviewer:agentcatalog/review-pr models: model: @@ -17,7 +22,7 @@ agents: You are a coordinator agent. You have access to both local and external sub-agents. - Use the "local_helper" agent for simple tasks. - - Use the "agentcatalog/pirate" agent when users want responses in a pirate style. + - Use the "pirate" agent when users want responses in a pirate style. Delegate tasks to the most appropriate sub-agent based on the user's request. sub_agents: diff --git a/pkg/config/config.go b/pkg/config/config.go index 8017d4233..a618fc7ec 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -130,15 +130,27 @@ func validateConfig(cfg *latest.Config) error { } for _, agent := range cfg.Agents { - for _, subAgentName := range agent.SubAgents { - if _, exists := allNames[subAgentName]; !exists && !IsExternalReference(subAgentName) { - return fmt.Errorf("agent '%s' references non-existent sub-agent '%s'", agent.Name, subAgentName) + for _, subAgentRef := range agent.SubAgents { + if _, exists := allNames[subAgentRef]; !exists && !IsExternalReference(subAgentRef) { + return fmt.Errorf("agent '%s' references non-existent sub-agent '%s'", agent.Name, subAgentRef) + } + if IsExternalReference(subAgentRef) { + name, _ := ParseExternalAgentRef(subAgentRef) + if allNames[name] { + return fmt.Errorf("agent '%s': external sub-agent '%s' resolves to name '%s' which conflicts with a locally-defined agent", agent.Name, subAgentRef, name) + } } } - for _, handoffName := range agent.Handoffs { - if _, exists := allNames[handoffName]; !exists && !IsExternalReference(handoffName) { - return fmt.Errorf("agent '%s' references non-existent handoff agent '%s'", agent.Name, handoffName) + for _, handoffRef := range agent.Handoffs { + if _, exists := allNames[handoffRef]; !exists && !IsExternalReference(handoffRef) { + return fmt.Errorf("agent '%s' references non-existent handoff agent '%s'", agent.Name, handoffRef) + } + if IsExternalReference(handoffRef) { + name, _ := ParseExternalAgentRef(handoffRef) + if allNames[name] { + return fmt.Errorf("agent '%s': external handoff '%s' resolves to name '%s' which conflicts with a locally-defined agent", agent.Name, handoffRef, name) + } } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index f0ea9ecbe..38f5517c9 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -533,6 +533,60 @@ func TestValidateConfig_ExternalSubAgentReferences(t *testing.T) { }, wantErr: "non-existent handoff agent 'does_not_exist'", }, + { + name: "named OCI reference in sub_agents is allowed", + cfg: &latest.Config{ + Agents: []latest.AgentConfig{ + {Name: "root", Model: "openai/gpt-4o", SubAgents: []string{"reviewer:agentcatalog/review-pr"}}, + }, + }, + }, + { + name: "named URL reference in sub_agents is allowed", + cfg: &latest.Config{ + Agents: []latest.AgentConfig{ + {Name: "root", Model: "openai/gpt-4o", SubAgents: []string{"myagent:https://example.com/agent.yaml"}}, + }, + }, + }, + { + name: "named OCI reference in handoffs is allowed", + cfg: &latest.Config{ + Agents: []latest.AgentConfig{ + {Name: "root", Model: "openai/gpt-4o", Handoffs: []string{"reviewer:agentcatalog/review-pr"}}, + }, + }, + }, + { + name: "external sub-agent name collides with local agent", + cfg: &latest.Config{ + Agents: []latest.AgentConfig{ + {Name: "root", Model: "openai/gpt-4o", SubAgents: []string{"pirate", "agentcatalog/pirate"}}, + {Name: "pirate", Model: "openai/gpt-4o"}, + }, + }, + wantErr: "conflicts with a locally-defined agent", + }, + { + name: "named external sub-agent collides with local agent", + cfg: &latest.Config{ + Agents: []latest.AgentConfig{ + {Name: "root", Model: "openai/gpt-4o", SubAgents: []string{"helper", "helper:agentcatalog/review-pr"}}, + {Name: "helper", Model: "openai/gpt-4o"}, + }, + }, + wantErr: "conflicts with a locally-defined agent", + }, + { + name: "external handoff name collides with local agent", + cfg: &latest.Config{ + Agents: []latest.AgentConfig{ + {Name: "root", Model: "openai/gpt-4o", Handoffs: []string{"agentcatalog/pirate"}}, + {Name: "pirate", Model: "openai/gpt-4o"}, + }, + }, + wantErr: "conflicts with a locally-defined agent", + }, { name: "local handoff to another agent passes", cfg: &latest.Config{ diff --git a/pkg/config/resolve.go b/pkg/config/resolve.go index a4cca4afa..dd84bcc27 100644 --- a/pkg/config/resolve.go +++ b/pkg/config/resolve.go @@ -216,6 +216,83 @@ func fileNameWithoutExt(path string) string { // (OCI image or URL) rather than a local agent name defined in the same config. // Local agent names never contain "/", so the slash check distinguishes them // from OCI references like "agentcatalog/pirate" or "docker.io/org/agent:v1". +// It also handles the "name:ref" syntax (e.g. "reviewer:agentcatalog/review-pr"). func IsExternalReference(input string) bool { + _, ref := ParseExternalAgentRef(input) + return isExternalRef(ref) +} + +// ParseExternalAgentRef parses an external agent reference that may include an +// explicit name prefix. The syntax is "name:reference" where name is a simple +// identifier (no slashes) and reference is an OCI reference or URL. +// +// If no explicit name is provided, the base name is derived from the reference: +// - OCI refs: last path segment without tag (e.g. "agentcatalog/review-pr" → "review-pr") +// - URLs: filename without extension (e.g. "https://example.com/agent.yaml" → "agent") +// +// Examples: +// +// ParseExternalAgentRef("reviewer:agentcatalog/review-pr") → ("reviewer", "agentcatalog/review-pr") +// ParseExternalAgentRef("agentcatalog/review-pr") → ("review-pr", "agentcatalog/review-pr") +// ParseExternalAgentRef("docker.io/myorg/myagent:v1") → ("myagent", "docker.io/myorg/myagent:v1") +// ParseExternalAgentRef("https://example.com/agent.yaml") → ("agent", "https://example.com/agent.yaml") +func ParseExternalAgentRef(input string) (agentName, ref string) { + // If the whole input is already a valid external reference, derive the name + // from it without trying to split on ":". + if isExternalRef(input) { + return externalRefBaseName(input), input + } + + // Check for explicit "name:reference" syntax. + // A name prefix is identified by not containing "/" (distinguishing it from + // OCI references or URLs which always contain slashes). + if i := strings.Index(input, ":"); i > 0 { + candidate := input[:i] + if !strings.Contains(candidate, "/") { + remainder := input[i+1:] + if isExternalRef(remainder) { + return candidate, remainder + } + } + } + + // Fallback: return input as both name and ref (for local agent names). + return input, input +} + +// isExternalRef is the core check for whether a string is an external reference. +// It is used by both IsExternalReference and ParseExternalAgentRef to avoid +// circular dependencies. +func isExternalRef(input string) bool { return IsURLReference(input) || (strings.Contains(input, "/") && IsOCIReference(input)) } + +// externalRefBaseName extracts a short agent name from an external reference. +// +// - OCI: last path segment, tag/digest stripped +// "agentcatalog/review-pr" → "review-pr" +// "docker.io/myorg/myagent:v1" → "myagent" +// +// - URL: filename without extension +// "https://example.com/agent.yaml" → "agent" +func externalRefBaseName(ref string) string { + if IsURLReference(ref) { + return fileNameWithoutExt(ref) + } + + // OCI reference: strip tag or digest, then take last path segment. + base := ref + if i := strings.LastIndex(base, "@"); i >= 0 { + base = base[:i] + } + if i := strings.LastIndex(base, ":"); i >= 0 { + // Only strip if the colon is after the last slash (i.e. it's a tag, not a port). + if j := strings.LastIndex(base, "/"); j < i { + base = base[:i] + } + } + if i := strings.LastIndex(base, "/"); i >= 0 { + base = base[i+1:] + } + return base +} diff --git a/pkg/config/resolve_test.go b/pkg/config/resolve_test.go index 6fc8fff2e..592b42810 100644 --- a/pkg/config/resolve_test.go +++ b/pkg/config/resolve_test.go @@ -665,6 +665,16 @@ func TestIsExternalReference(t *testing.T) { input: "", expected: false, }, + { + name: "named OCI reference is external", + input: "reviewer:agentcatalog/review-pr", + expected: true, + }, + { + name: "named URL reference is external", + input: "myagent:https://example.com/agent.yaml", + expected: true, + }, } for _, tt := range tests { @@ -676,3 +686,85 @@ func TestIsExternalReference(t *testing.T) { }) } } + +func TestParseExternalAgentRef(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + expectedName string + expectedRef string + }{ + { + name: "simple OCI reference derives base name", + input: "agentcatalog/pirate", + expectedName: "pirate", + expectedRef: "agentcatalog/pirate", + }, + { + name: "OCI reference with tag derives base name without tag", + input: "docker.io/myorg/myagent:v1", + expectedName: "myagent", + expectedRef: "docker.io/myorg/myagent:v1", + }, + { + name: "OCI reference with digest derives base name", + input: "docker.io/myorg/myagent@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + expectedName: "myagent", + expectedRef: "docker.io/myorg/myagent@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + }, + { + name: "explicit name prefix", + input: "reviewer:agentcatalog/review-pr", + expectedName: "reviewer", + expectedRef: "agentcatalog/review-pr", + }, + { + name: "explicit name with tagged OCI ref", + input: "myreviewer:docker.io/myorg/review-pr:v2", + expectedName: "myreviewer", + expectedRef: "docker.io/myorg/review-pr:v2", + }, + { + name: "URL reference derives filename", + input: "https://example.com/agent.yaml", + expectedName: "agent", + expectedRef: "https://example.com/agent.yaml", + }, + { + name: "named URL reference", + input: "myagent:https://example.com/agent.yaml", + expectedName: "myagent", + expectedRef: "https://example.com/agent.yaml", + }, + { + name: "simple name without slash is not split", + input: "my_agent", + expectedName: "my_agent", + expectedRef: "my_agent", + }, + { + name: "OCI ref with registry port is not confused with name prefix", + input: "localhost:5000/test/agent", + expectedName: "agent", + expectedRef: "localhost:5000/test/agent", + }, + { + name: "deeply nested OCI path", + input: "registry.example.com/org/sub/agent:latest", + expectedName: "agent", + expectedRef: "registry.example.com/org/sub/agent:latest", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + name, ref := ParseExternalAgentRef(tt.input) + assert.Equal(t, tt.expectedName, name) + assert.Equal(t, tt.expectedRef, ref) + }) + } +} diff --git a/pkg/teamloader/teamloader.go b/pkg/teamloader/teamloader.go index c538e9066..f0639d2d7 100644 --- a/pkg/teamloader/teamloader.go +++ b/pkg/teamloader/teamloader.go @@ -511,6 +511,8 @@ func configNameFromSource(sourceName string) string { // References that match a locally-defined agent name are looked up directly. // References that are external (OCI or URL) are loaded on-demand and cached // in externalAgents so the same reference isn't loaded twice. +// External references may include an explicit name prefix ("name:ref") or +// derive a short name from the reference (e.g. "agentcatalog/review-pr" → "review-pr"). func resolveAgentRefs( ctx context.Context, refs []string, @@ -538,12 +540,25 @@ func resolveAgentRefs( continue } - a, err := loadExternalAgent(ctx, ref, runConfig, loadOpts) + agentName, externalRef := config.ParseExternalAgentRef(ref) + + // Check for name collisions before loading the external agent. + if existing, ok := agentsByName[agentName]; ok { + return nil, fmt.Errorf("external agent %q resolves to name %q which conflicts with agent %q", ref, agentName, existing.Name()) + } + + a, err := loadExternalAgent(ctx, externalRef, runConfig, loadOpts) if err != nil { - return nil, fmt.Errorf("loading %q: %w", ref, err) + return nil, fmt.Errorf("loading %q: %w", externalRef, err) } + + // Rename the external agent so it doesn't collide with locally-defined + // agents (external agents typically have the name "root"). + agent.WithName(agentName)(a) + *agents = append(*agents, a) externalAgents[ref] = a + agentsByName[agentName] = a resolved = append(resolved, a) } return resolved, nil