Merge pull request #66 from bnema/fix/web-research-audit
fix(subagents): normalize qualified model IDs
3 files · +58 -0
M packages/agent/subagents/supervisor.go +13 -0
| @@ -374,6 +374,18 @@ func (f *Supervisor) Spawn(ctx context.Context, task string) (*Agent, error) { | |||
| 374 | 374 | return f.SpawnReq(ctx, SpawnRequest{Task: task}) | |
| 375 | 375 | } | |
| 376 | 376 | ||
| 377 | + | // unqualifiedModelID removes a redundant provider prefix from a model ID. | |
| 378 | + | // Model IDs themselves may contain slashes, so only the explicitly selected | |
| 379 | + | // provider's exact prefix is removed. | |
| 380 | + | func unqualifiedModelID(provider, model string) string { | |
| 381 | + | provider = strings.TrimSpace(provider) | |
| 382 | + | model = strings.TrimSpace(model) | |
| 383 | + | if provider == "" { | |
| 384 | + | return model | |
| 385 | + | } | |
| 386 | + | return strings.TrimPrefix(model, provider+"/") | |
| 387 | + | } | |
| 388 | + | ||
| 377 | 389 | // SpawnReq is the full-fat variant of Spawn that accepts a SpawnRequest. | |
| 378 | 390 | // Existing callers can keep using Spawn; new code that wants to pin the | |
| 379 | 391 | // child's model, policy, or workspace uses this. | |
| @@ -388,6 +400,7 @@ func (f *Supervisor) SpawnReq(ctx context.Context, req SpawnRequest) (*Agent, er | |||
| 388 | 400 | if err := f.validateSpawnRequest(req); err != nil { | |
| 389 | 401 | return nil, err | |
| 390 | 402 | } | |
| 403 | + | req.Model = unqualifiedModelID(req.Provider, req.Model) | |
| 391 | 404 | if ctx == nil { | |
| 392 | 405 | ctx = context.Background() | |
| 393 | 406 | } | |
| @@ -374,6 +374,18 @@ func (f *Supervisor) Spawn(ctx context.Context, task string) (*Agent, error) { | |||||
| 374 | return f.SpawnReq(ctx, SpawnRequest{Task: task}) | 374 | return f.SpawnReq(ctx, SpawnRequest{Task: task}) | ||
| 375 | } | 375 | } | ||
| 376 | 376 | ||||
| 377 | + | // unqualifiedModelID removes a redundant provider prefix from a model ID. | |||
| 378 | + | // Model IDs themselves may contain slashes, so only the explicitly selected | |||
| 379 | + | // provider's exact prefix is removed. | |||
| 380 | + | func unqualifiedModelID(provider, model string) string { | |||
| 381 | + | provider = strings.TrimSpace(provider) | |||
| 382 | + | model = strings.TrimSpace(model) | |||
| 383 | + | if provider == "" { | |||
| 384 | + | return model | |||
| 385 | + | } | |||
| 386 | + | return strings.TrimPrefix(model, provider+"/") | |||
| 387 | + | } | |||
| 388 | + | ||||
| 377 | // SpawnReq is the full-fat variant of Spawn that accepts a SpawnRequest. | 389 | // SpawnReq is the full-fat variant of Spawn that accepts a SpawnRequest. | ||
| 378 | // Existing callers can keep using Spawn; new code that wants to pin the | 390 | // Existing callers can keep using Spawn; new code that wants to pin the | ||
| 379 | // child's model, policy, or workspace uses this. | 391 | // child's model, policy, or workspace uses this. | ||
| @@ -388,6 +400,7 @@ func (f *Supervisor) SpawnReq(ctx context.Context, req SpawnRequest) (*Agent, er | |||||
| 388 | if err := f.validateSpawnRequest(req); err != nil { | 400 | if err := f.validateSpawnRequest(req); err != nil { | ||
| 389 | return nil, err | 401 | return nil, err | ||
| 390 | } | 402 | } | ||
| 403 | + | req.Model = unqualifiedModelID(req.Provider, req.Model) | |||
| 391 | if ctx == nil { | 404 | if ctx == nil { | ||
| 392 | ctx = context.Background() | 405 | ctx = context.Background() | ||
| 393 | } | 406 | } | ||
M packages/agent/subagents/supervisor_test.go +20 -0
| @@ -23,6 +23,26 @@ func newTestSupervisor(t *testing.T, mk func(a *Agent) Runner) *Supervisor { | |||
| 23 | 23 | }) | |
| 24 | 24 | } | |
| 25 | 25 | ||
| 26 | + | func TestUnqualifiedModelID(t *testing.T) { | |
| 27 | + | for _, tc := range []struct { | |
| 28 | + | name string | |
| 29 | + | provider string | |
| 30 | + | model string | |
| 31 | + | want string | |
| 32 | + | }{ | |
| 33 | + | {name: "matching provider", provider: "openai-codex", model: "openai-codex/gpt-5.6-sol", want: "gpt-5.6-sol"}, | |
| 34 | + | {name: "matching provider with whitespace", provider: "openai-codex", model: " openai-codex/gpt-5.6-sol ", want: "gpt-5.6-sol"}, | |
| 35 | + | {name: "other slash-containing model ID", provider: "openrouter", model: "anthropic/claude-sonnet-4-5", want: "anthropic/claude-sonnet-4-5"}, | |
| 36 | + | {name: "no provider", model: "/custom-model", want: "/custom-model"}, | |
| 37 | + | } { | |
| 38 | + | t.Run(tc.name, func(t *testing.T) { | |
| 39 | + | if got := unqualifiedModelID(tc.provider, tc.model); got != tc.want { | |
| 40 | + | t.Fatalf("unqualifiedModelID(%q, %q) = %q, want %q", tc.provider, tc.model, got, tc.want) | |
| 41 | + | } | |
| 42 | + | }) | |
| 43 | + | } | |
| 44 | + | } | |
| 45 | + | ||
| 26 | 46 | func TestSpawnRunsAndCompletes(t *testing.T) { | |
| 27 | 47 | ran := make(chan string, 1) | |
| 28 | 48 | f := newTestSupervisor(t, func(a *Agent) Runner { | |
| @@ -23,6 +23,26 @@ func newTestSupervisor(t *testing.T, mk func(a *Agent) Runner) *Supervisor { | |||||
| 23 | }) | 23 | }) | ||
| 24 | } | 24 | } | ||
| 25 | 25 | ||||
| 26 | + | func TestUnqualifiedModelID(t *testing.T) { | |||
| 27 | + | for _, tc := range []struct { | |||
| 28 | + | name string | |||
| 29 | + | provider string | |||
| 30 | + | model string | |||
| 31 | + | want string | |||
| 32 | + | }{ | |||
| 33 | + | {name: "matching provider", provider: "openai-codex", model: "openai-codex/gpt-5.6-sol", want: "gpt-5.6-sol"}, | |||
| 34 | + | {name: "matching provider with whitespace", provider: "openai-codex", model: " openai-codex/gpt-5.6-sol ", want: "gpt-5.6-sol"}, | |||
| 35 | + | {name: "other slash-containing model ID", provider: "openrouter", model: "anthropic/claude-sonnet-4-5", want: "anthropic/claude-sonnet-4-5"}, | |||
| 36 | + | {name: "no provider", model: "/custom-model", want: "/custom-model"}, | |||
| 37 | + | } { | |||
| 38 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 39 | + | if got := unqualifiedModelID(tc.provider, tc.model); got != tc.want { | |||
| 40 | + | t.Fatalf("unqualifiedModelID(%q, %q) = %q, want %q", tc.provider, tc.model, got, tc.want) | |||
| 41 | + | } | |||
| 42 | + | }) | |||
| 43 | + | } | |||
| 44 | + | } | |||
| 45 | + | ||||
| 26 | func TestSpawnRunsAndCompletes(t *testing.T) { | 46 | func TestSpawnRunsAndCompletes(t *testing.T) { | ||
| 27 | ran := make(chan string, 1) | 47 | ran := make(chan string, 1) | ||
| 28 | f := newTestSupervisor(t, func(a *Agent) Runner { | 48 | f := newTestSupervisor(t, func(a *Agent) Runner { | ||
M packages/agent/tools/subagent_spawn_test.go +25 -0
| @@ -389,6 +389,31 @@ func TestSubagentSpawnRejectsUnknownProfile(t *testing.T) { | |||
| 389 | 389 | } | |
| 390 | 390 | } | |
| 391 | 391 | ||
| 392 | + | func TestSubagentSpawnNormalizesMatchingQualifiedModelOverride(t *testing.T) { | |
| 393 | + | tool := &SubagentSpawnTool{ | |
| 394 | + | Supervisor: newTestSupervisor(t), | |
| 395 | + | Enabled: func() bool { return true }, | |
| 396 | + | } | |
| 397 | + | ||
| 398 | + | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"review auth","provider":"openai-codex","model":"openai-codex/gpt-5.6-sol"}`), nil) | |
| 399 | + | if err != nil { | |
| 400 | + | t.Fatal(err) | |
| 401 | + | } | |
| 402 | + | if res.IsError { | |
| 403 | + | t.Fatalf("unexpected tool error: %s", textResult(res.Content)) | |
| 404 | + | } | |
| 405 | + | agents := tool.Supervisor.List() | |
| 406 | + | if len(agents) != 1 { | |
| 407 | + | t.Fatalf("spawned agents = %d, want 1", len(agents)) | |
| 408 | + | } | |
| 409 | + | if got := agents[0].Model; got != "gpt-5.6-sol" { | |
| 410 | + | t.Fatalf("agent model = %q, want unqualified model ID", got) | |
| 411 | + | } | |
| 412 | + | if got := agents[0].Provider; got != "openai-codex" { | |
| 413 | + | t.Fatalf("agent provider = %q, want openai-codex", got) | |
| 414 | + | } | |
| 415 | + | } | |
| 416 | + | ||
| 392 | 417 | func TestSubagentSpawnRejectsPartialModelProviderOverride(t *testing.T) { | |
| 393 | 418 | tool := &SubagentSpawnTool{ | |
| 394 | 419 | Supervisor: newTestSupervisor(t), | |
| @@ -389,6 +389,31 @@ func TestSubagentSpawnRejectsUnknownProfile(t *testing.T) { | |||||
| 389 | } | 389 | } | ||
| 390 | } | 390 | } | ||
| 391 | 391 | ||||
| 392 | + | func TestSubagentSpawnNormalizesMatchingQualifiedModelOverride(t *testing.T) { | |||
| 393 | + | tool := &SubagentSpawnTool{ | |||
| 394 | + | Supervisor: newTestSupervisor(t), | |||
| 395 | + | Enabled: func() bool { return true }, | |||
| 396 | + | } | |||
| 397 | + | ||||
| 398 | + | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"review auth","provider":"openai-codex","model":"openai-codex/gpt-5.6-sol"}`), nil) | |||
| 399 | + | if err != nil { | |||
| 400 | + | t.Fatal(err) | |||
| 401 | + | } | |||
| 402 | + | if res.IsError { | |||
| 403 | + | t.Fatalf("unexpected tool error: %s", textResult(res.Content)) | |||
| 404 | + | } | |||
| 405 | + | agents := tool.Supervisor.List() | |||
| 406 | + | if len(agents) != 1 { | |||
| 407 | + | t.Fatalf("spawned agents = %d, want 1", len(agents)) | |||
| 408 | + | } | |||
| 409 | + | if got := agents[0].Model; got != "gpt-5.6-sol" { | |||
| 410 | + | t.Fatalf("agent model = %q, want unqualified model ID", got) | |||
| 411 | + | } | |||
| 412 | + | if got := agents[0].Provider; got != "openai-codex" { | |||
| 413 | + | t.Fatalf("agent provider = %q, want openai-codex", got) | |||
| 414 | + | } | |||
| 415 | + | } | |||
| 416 | + | ||||
| 392 | func TestSubagentSpawnRejectsPartialModelProviderOverride(t *testing.T) { | 417 | func TestSubagentSpawnRejectsPartialModelProviderOverride(t *testing.T) { | ||
| 393 | tool := &SubagentSpawnTool{ | 418 | tool := &SubagentSpawnTool{ | ||
| 394 | Supervisor: newTestSupervisor(t), | 419 | Supervisor: newTestSupervisor(t), | ||