fix(agent): tighten profile discovery and swarm validation
- commit
- bbb672b47d43cd156280960916eeefc67789f4f9
- author
- brice · · 2026-08-04 05:17 UTC
- parent
- 86d3d2d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +69 -6
M docs/subagents.md +1 -1
| @@ -23,7 +23,7 @@ Profiles are read-only inputs. zot does not execute files from these directories | |||
| 23 | 23 | --- | |
| 24 | 24 | name: reviewer | |
| 25 | 25 | description: Read-only code reviewer | |
| 26 | - | tools: read, grep, find, ls, diagnostics, code_search | |
| 26 | + | tools: read, write, edit, bash | |
| 27 | 27 | model: openai-codex/gpt-5.6-luna | |
| 28 | 28 | thinking: max | |
| 29 | 29 | systemPromptMode: replace | |
| @@ -23,7 +23,7 @@ Profiles are read-only inputs. zot does not execute files from these directories | |||||
| 23 | --- | 23 | --- | ||
| 24 | name: reviewer | 24 | name: reviewer | ||
| 25 | description: Read-only code reviewer | 25 | description: Read-only code reviewer | ||
| 26 | - | tools: read, grep, find, ls, diagnostics, code_search | 26 | + | tools: read, write, edit, bash |
| 27 | model: openai-codex/gpt-5.6-luna | 27 | model: openai-codex/gpt-5.6-luna | ||
| 28 | thinking: max | 28 | thinking: max | ||
| 29 | systemPromptMode: replace | 29 | systemPromptMode: replace | ||
M packages/agent/modes/swarm_slash.go +5 -1
| @@ -103,7 +103,11 @@ func (i *Interactive) runSwarm(ctx context.Context, args []string) { | |||
| 103 | 103 | i.swarmStatus("", "/swarm new: missing task (after any spawn flags)") | |
| 104 | 104 | return | |
| 105 | 105 | } | |
| 106 | - | if subagent != "" && i.cfg.ResolveSubagent != nil { | |
| 106 | + | if subagent != "" { | |
| 107 | + | if i.cfg.ResolveSubagent == nil { | |
| 108 | + | i.swarmStatus("", "spawn: named subagent profiles are unavailable") | |
| 109 | + | return | |
| 110 | + | } | |
| 107 | 111 | profile, err := i.cfg.ResolveSubagent(subagent) | |
| 108 | 112 | if err != nil { | |
| 109 | 113 | i.swarmStatus("", "spawn: "+err.Error()) | |
| @@ -103,7 +103,11 @@ func (i *Interactive) runSwarm(ctx context.Context, args []string) { | |||||
| 103 | i.swarmStatus("", "/swarm new: missing task (after any spawn flags)") | 103 | i.swarmStatus("", "/swarm new: missing task (after any spawn flags)") | ||
| 104 | return | 104 | return | ||
| 105 | } | 105 | } | ||
| 106 | - | if subagent != "" && i.cfg.ResolveSubagent != nil { | 106 | + | if subagent != "" { |
| 107 | + | if i.cfg.ResolveSubagent == nil { | |||
| 108 | + | i.swarmStatus("", "spawn: named subagent profiles are unavailable") | |||
| 109 | + | return | |||
| 110 | + | } | |||
| 107 | profile, err := i.cfg.ResolveSubagent(subagent) | 111 | profile, err := i.cfg.ResolveSubagent(subagent) | ||
| 108 | if err != nil { | 112 | if err != nil { | ||
| 109 | i.swarmStatus("", "spawn: "+err.Error()) | 113 | i.swarmStatus("", "spawn: "+err.Error()) | ||
M packages/agent/modes/swarm_slash_test.go +14 -0
| @@ -3,6 +3,7 @@ package modes | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "runtime" | |
| 6 | + | "strings" | |
| 6 | 7 | "testing" | |
| 7 | 8 | "time" | |
| 8 | 9 | ||
| @@ -82,6 +83,19 @@ func TestRunSwarmSubcommandsDoNotPanic(t *testing.T) { | |||
| 82 | 83 | } | |
| 83 | 84 | } | |
| 84 | 85 | ||
| 86 | + | func TestRunSwarmRejectsNamedProfileWithoutResolver(t *testing.T) { | |
| 87 | + | iv, f := newInteractiveForSwarmTest(t) | |
| 88 | + | defer f.StopAll() | |
| 89 | + | ||
| 90 | + | iv.runSwarm(context.Background(), []string{"new", "--agent", "reviewer", "review", "auth"}) | |
| 91 | + | if !strings.Contains(iv.statusErr, "named subagent profiles are unavailable") { | |
| 92 | + | t.Fatalf("status error = %q, want unavailable profile resolver", iv.statusErr) | |
| 93 | + | } | |
| 94 | + | if got := len(f.List()); got != 0 { | |
| 95 | + | t.Fatalf("spawned agents = %d, want 0", got) | |
| 96 | + | } | |
| 97 | + | } | |
| 98 | + | ||
| 85 | 99 | func TestRunSwarmNewSpawnsAgent(t *testing.T) { | |
| 86 | 100 | iv, f := newInteractiveForSwarmTest(t) | |
| 87 | 101 | defer f.StopAll() | |
| @@ -3,6 +3,7 @@ package modes | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "runtime" | 5 | "runtime" | ||
| 6 | + | "strings" | |||
| 6 | "testing" | 7 | "testing" | ||
| 7 | "time" | 8 | "time" | ||
| 8 | 9 | ||||
| @@ -82,6 +83,19 @@ func TestRunSwarmSubcommandsDoNotPanic(t *testing.T) { | |||||
| 82 | } | 83 | } | ||
| 83 | } | 84 | } | ||
| 84 | 85 | ||||
| 86 | + | func TestRunSwarmRejectsNamedProfileWithoutResolver(t *testing.T) { | |||
| 87 | + | iv, f := newInteractiveForSwarmTest(t) | |||
| 88 | + | defer f.StopAll() | |||
| 89 | + | ||||
| 90 | + | iv.runSwarm(context.Background(), []string{"new", "--agent", "reviewer", "review", "auth"}) | |||
| 91 | + | if !strings.Contains(iv.statusErr, "named subagent profiles are unavailable") { | |||
| 92 | + | t.Fatalf("status error = %q, want unavailable profile resolver", iv.statusErr) | |||
| 93 | + | } | |||
| 94 | + | if got := len(f.List()); got != 0 { | |||
| 95 | + | t.Fatalf("spawned agents = %d, want 0", got) | |||
| 96 | + | } | |||
| 97 | + | } | |||
| 98 | + | ||||
| 85 | func TestRunSwarmNewSpawnsAgent(t *testing.T) { | 99 | func TestRunSwarmNewSpawnsAgent(t *testing.T) { | ||
| 86 | iv, f := newInteractiveForSwarmTest(t) | 100 | iv, f := newInteractiveForSwarmTest(t) | ||
| 87 | defer f.StopAll() | 101 | defer f.StopAll() | ||
M packages/agent/subagents/profiles.go +14 -4
| @@ -98,8 +98,9 @@ func Find(profiles []*Profile, name string) *Profile { | |||
| 98 | 98 | } | |
| 99 | 99 | ||
| 100 | 100 | // ModelSelection splits a provider-qualified model value such as | |
| 101 | - | // "openai-codex/gpt-5.6-luna". A model id may itself contain slashes, so only | |
| 102 | - | // the first slash is treated as the provider separator. | |
| 101 | + | // "openai-codex/gpt-5.6-luna". Explicit provider metadata must be paired | |
| 102 | + | // with an unqualified model; model IDs may contain slashes when no provider | |
| 103 | + | // metadata is supplied. | |
| 103 | 104 | func (p *Profile) ModelSelection() (provider, model string) { | |
| 104 | 105 | if p == nil { | |
| 105 | 106 | return "", "" | |
| @@ -171,6 +172,10 @@ func searchDirs(cwd, userHome string) []location { | |||
| 171 | 172 | } | |
| 172 | 173 | if extra := os.Getenv("ZOT_AGENT_PROFILES"); extra != "" { | |
| 173 | 174 | for _, dir := range filepath.SplitList(extra) { | |
| 175 | + | dir = strings.TrimSpace(dir) | |
| 176 | + | if dir != "" && !filepath.IsAbs(dir) && strings.TrimSpace(cwd) != "" { | |
| 177 | + | dir = filepath.Join(cwd, dir) | |
| 178 | + | } | |
| 174 | 179 | add(dir, "configured") | |
| 175 | 180 | } | |
| 176 | 181 | } | |
| @@ -218,13 +223,18 @@ func load(path, source string) (*Profile, error) { | |||
| 218 | 223 | if thinking == "" { | |
| 219 | 224 | thinking = reasoning | |
| 220 | 225 | } | |
| 226 | + | model := strings.TrimSpace(unquote(values["model"])) | |
| 227 | + | provider := strings.TrimSpace(unquote(values["provider"])) | |
| 228 | + | if provider != "" && strings.Contains(model, "/") { | |
| 229 | + | return nil, fmt.Errorf("model must not include a provider when provider is set") | |
| 230 | + | } | |
| 221 | 231 | profile := &Profile{ | |
| 222 | 232 | Name: strings.TrimSpace(name), | |
| 223 | 233 | Description: strings.TrimSpace(unquote(values["description"])), | |
| 224 | 234 | SystemPrompt: strings.TrimSpace(body), | |
| 225 | 235 | Tools: lists["tools"], | |
| 226 | - | Model: unquote(values["model"]), | |
| 227 | - | Provider: unquote(values["provider"]), | |
| 236 | + | Model: model, | |
| 237 | + | Provider: provider, | |
| 228 | 238 | Thinking: thinking, | |
| 229 | 239 | Path: path, | |
| 230 | 240 | Source: source, | |
| @@ -98,8 +98,9 @@ func Find(profiles []*Profile, name string) *Profile { | |||||
| 98 | } | 98 | } | ||
| 99 | 99 | ||||
| 100 | // ModelSelection splits a provider-qualified model value such as | 100 | // ModelSelection splits a provider-qualified model value such as | ||
| 101 | - | // "openai-codex/gpt-5.6-luna". A model id may itself contain slashes, so only | 101 | + | // "openai-codex/gpt-5.6-luna". Explicit provider metadata must be paired |
| 102 | - | // the first slash is treated as the provider separator. | 102 | + | // with an unqualified model; model IDs may contain slashes when no provider |
| 103 | + | // metadata is supplied. | |||
| 103 | func (p *Profile) ModelSelection() (provider, model string) { | 104 | func (p *Profile) ModelSelection() (provider, model string) { | ||
| 104 | if p == nil { | 105 | if p == nil { | ||
| 105 | return "", "" | 106 | return "", "" | ||
| @@ -171,6 +172,10 @@ func searchDirs(cwd, userHome string) []location { | |||||
| 171 | } | 172 | } | ||
| 172 | if extra := os.Getenv("ZOT_AGENT_PROFILES"); extra != "" { | 173 | if extra := os.Getenv("ZOT_AGENT_PROFILES"); extra != "" { | ||
| 173 | for _, dir := range filepath.SplitList(extra) { | 174 | for _, dir := range filepath.SplitList(extra) { | ||
| 175 | + | dir = strings.TrimSpace(dir) | |||
| 176 | + | if dir != "" && !filepath.IsAbs(dir) && strings.TrimSpace(cwd) != "" { | |||
| 177 | + | dir = filepath.Join(cwd, dir) | |||
| 178 | + | } | |||
| 174 | add(dir, "configured") | 179 | add(dir, "configured") | ||
| 175 | } | 180 | } | ||
| 176 | } | 181 | } | ||
| @@ -218,13 +223,18 @@ func load(path, source string) (*Profile, error) { | |||||
| 218 | if thinking == "" { | 223 | if thinking == "" { | ||
| 219 | thinking = reasoning | 224 | thinking = reasoning | ||
| 220 | } | 225 | } | ||
| 226 | + | model := strings.TrimSpace(unquote(values["model"])) | |||
| 227 | + | provider := strings.TrimSpace(unquote(values["provider"])) | |||
| 228 | + | if provider != "" && strings.Contains(model, "/") { | |||
| 229 | + | return nil, fmt.Errorf("model must not include a provider when provider is set") | |||
| 230 | + | } | |||
| 221 | profile := &Profile{ | 231 | profile := &Profile{ | ||
| 222 | Name: strings.TrimSpace(name), | 232 | Name: strings.TrimSpace(name), | ||
| 223 | Description: strings.TrimSpace(unquote(values["description"])), | 233 | Description: strings.TrimSpace(unquote(values["description"])), | ||
| 224 | SystemPrompt: strings.TrimSpace(body), | 234 | SystemPrompt: strings.TrimSpace(body), | ||
| 225 | Tools: lists["tools"], | 235 | Tools: lists["tools"], | ||
| 226 | - | Model: unquote(values["model"]), | 236 | + | Model: model, |
| 227 | - | Provider: unquote(values["provider"]), | 237 | + | Provider: provider, |
| 228 | Thinking: thinking, | 238 | Thinking: thinking, | ||
| 229 | Path: path, | 239 | Path: path, | ||
| 230 | Source: source, | 240 | Source: source, | ||
M packages/agent/subagents/profiles_test.go +35 -0
| @@ -68,6 +68,40 @@ Implement the requested change. | |||
| 68 | 68 | } | |
| 69 | 69 | } | |
| 70 | 70 | ||
| 71 | + | func TestDiscoverResolvesConfiguredRelativePathAgainstCWD(t *testing.T) { | |
| 72 | + | first := t.TempDir() | |
| 73 | + | second := t.TempDir() | |
| 74 | + | t.Setenv("ZOT_AGENT_PROFILES", ".agents") | |
| 75 | + | writeProfile(t, filepath.Join(first, ".agents", "reviewer.md"), `--- | |
| 76 | + | name: reviewer | |
| 77 | + | description: First session profile | |
| 78 | + | --- | |
| 79 | + | First. | |
| 80 | + | `) | |
| 81 | + | writeProfile(t, filepath.Join(second, ".agents", "reviewer.md"), `--- | |
| 82 | + | name: reviewer | |
| 83 | + | description: Second session profile | |
| 84 | + | --- | |
| 85 | + | Second. | |
| 86 | + | `) | |
| 87 | + | ||
| 88 | + | for _, tc := range []struct { | |
| 89 | + | cwd string | |
| 90 | + | description string | |
| 91 | + | }{ | |
| 92 | + | {cwd: first, description: "First session profile"}, | |
| 93 | + | {cwd: second, description: "Second session profile"}, | |
| 94 | + | } { | |
| 95 | + | profiles, errs := Discover(tc.cwd, t.TempDir()) | |
| 96 | + | if len(errs) != 0 { | |
| 97 | + | t.Fatalf("Discover(%q) errors = %v", tc.cwd, errs) | |
| 98 | + | } | |
| 99 | + | if len(profiles) != 1 || profiles[0].Description != tc.description { | |
| 100 | + | t.Fatalf("Discover(%q) = %#v, want %q", tc.cwd, profiles, tc.description) | |
| 101 | + | } | |
| 102 | + | } | |
| 103 | + | } | |
| 104 | + | ||
| 71 | 105 | func TestDiscoverConfiguredDirectoryWinsAndRejectsUnsafeNames(t *testing.T) { | |
| 72 | 106 | configured := t.TempDir() | |
| 73 | 107 | home := t.TempDir() | |
| @@ -117,6 +151,7 @@ func TestLoadRejectsInvalidClosedSchemaMetadata(t *testing.T) { | |||
| 117 | 151 | }{ | |
| 118 | 152 | {name: "invalid-thinking", field: "thinking: extreme"}, | |
| 119 | 153 | {name: "invalid-reasoning", field: "reasoning: extreme"}, | |
| 154 | + | {name: "qualified-model-with-provider", field: "provider: openai\nmodel: openai-codex/gpt-5"}, | |
| 120 | 155 | {name: "invalid-prompt-mode", field: "systemPromptMode: merge"}, | |
| 121 | 156 | {name: "invalid-project-context", field: "inheritProjectContext: sometimes"}, | |
| 122 | 157 | {name: "invalid-skills", field: "inheritSkills: sometimes"}, | |
| @@ -68,6 +68,40 @@ Implement the requested change. | |||||
| 68 | } | 68 | } | ||
| 69 | } | 69 | } | ||
| 70 | 70 | ||||
| 71 | + | func TestDiscoverResolvesConfiguredRelativePathAgainstCWD(t *testing.T) { | |||
| 72 | + | first := t.TempDir() | |||
| 73 | + | second := t.TempDir() | |||
| 74 | + | t.Setenv("ZOT_AGENT_PROFILES", ".agents") | |||
| 75 | + | writeProfile(t, filepath.Join(first, ".agents", "reviewer.md"), `--- | |||
| 76 | + | name: reviewer | |||
| 77 | + | description: First session profile | |||
| 78 | + | --- | |||
| 79 | + | First. | |||
| 80 | + | `) | |||
| 81 | + | writeProfile(t, filepath.Join(second, ".agents", "reviewer.md"), `--- | |||
| 82 | + | name: reviewer | |||
| 83 | + | description: Second session profile | |||
| 84 | + | --- | |||
| 85 | + | Second. | |||
| 86 | + | `) | |||
| 87 | + | ||||
| 88 | + | for _, tc := range []struct { | |||
| 89 | + | cwd string | |||
| 90 | + | description string | |||
| 91 | + | }{ | |||
| 92 | + | {cwd: first, description: "First session profile"}, | |||
| 93 | + | {cwd: second, description: "Second session profile"}, | |||
| 94 | + | } { | |||
| 95 | + | profiles, errs := Discover(tc.cwd, t.TempDir()) | |||
| 96 | + | if len(errs) != 0 { | |||
| 97 | + | t.Fatalf("Discover(%q) errors = %v", tc.cwd, errs) | |||
| 98 | + | } | |||
| 99 | + | if len(profiles) != 1 || profiles[0].Description != tc.description { | |||
| 100 | + | t.Fatalf("Discover(%q) = %#v, want %q", tc.cwd, profiles, tc.description) | |||
| 101 | + | } | |||
| 102 | + | } | |||
| 103 | + | } | |||
| 104 | + | ||||
| 71 | func TestDiscoverConfiguredDirectoryWinsAndRejectsUnsafeNames(t *testing.T) { | 105 | func TestDiscoverConfiguredDirectoryWinsAndRejectsUnsafeNames(t *testing.T) { | ||
| 72 | configured := t.TempDir() | 106 | configured := t.TempDir() | ||
| 73 | home := t.TempDir() | 107 | home := t.TempDir() | ||
| @@ -117,6 +151,7 @@ func TestLoadRejectsInvalidClosedSchemaMetadata(t *testing.T) { | |||||
| 117 | }{ | 151 | }{ | ||
| 118 | {name: "invalid-thinking", field: "thinking: extreme"}, | 152 | {name: "invalid-thinking", field: "thinking: extreme"}, | ||
| 119 | {name: "invalid-reasoning", field: "reasoning: extreme"}, | 153 | {name: "invalid-reasoning", field: "reasoning: extreme"}, | ||
| 154 | + | {name: "qualified-model-with-provider", field: "provider: openai\nmodel: openai-codex/gpt-5"}, | |||
| 120 | {name: "invalid-prompt-mode", field: "systemPromptMode: merge"}, | 155 | {name: "invalid-prompt-mode", field: "systemPromptMode: merge"}, | ||
| 121 | {name: "invalid-project-context", field: "inheritProjectContext: sometimes"}, | 156 | {name: "invalid-project-context", field: "inheritProjectContext: sometimes"}, | ||
| 122 | {name: "invalid-skills", field: "inheritSkills: sometimes"}, | 157 | {name: "invalid-skills", field: "inheritSkills: sometimes"}, | ||