diff --git a/cmd/odek/subagent_model_integration_test.go b/cmd/odek/subagent_model_integration_test.go new file mode 100644 index 0000000..35a9d65 --- /dev/null +++ b/cmd/odek/subagent_model_integration_test.go @@ -0,0 +1,86 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "math" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// TestE2E_SubagentTaskModelReachesProvider verifies the task envelope's model +// survives the real subprocess boundary and is used in the provider request. +// This is intentionally E2E-gated because it builds/runs the odek binary. +func TestE2E_SubagentTaskModelReachesProvider(t *testing.T) { + skipIfNoE2E(t) + requestModels := make(chan string, 16) + provider := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/models") { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":[]}`)) + return + } + var body struct { + Model string `json:"model"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + requestModels <- body.Model + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"choices":[{"message":{"content":"done"},"finish_reason":"stop"}],"usage":{"prompt_tokens":100,"completion_tokens":20}}`)) + })) + defer provider.Close() + + fixture := t.TempDir() + configDir := filepath.Join(fixture, ".odek") + if err := os.Mkdir(configDir, 0700); err != nil { + t.Fatal(err) + } + config := `{"model":"operator-model","memory":{"enabled":false},"limits":{"model_prices":{"task-model":{"input_cost_per_million_usd":2,"output_cost_per_million_usd":4},"operator-model":{"input_cost_per_million_usd":20,"output_cost_per_million_usd":40}}}}` + if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(config), 0600); err != nil { + t.Fatal(err) + } + taskPath := filepath.Join(fixture, "task.json") + spec := taskFileSpec{Goal: "reply briefly", Provider: "deepseek", Model: "task-model", BaseURL: provider.URL} + b, err := json.Marshal(spec) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(taskPath, b, 0600); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, e2eBinary, "subagent", "--task", taskPath, "--quiet") + cmd.Env = append(os.Environ(), "ODEK_API_KEY=test-key", "ODEK_NO_SANDBOX=1", "HOME="+fixture, "USERPROFILE="+fixture) + cmd.Dir = fixture + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + if err := cmd.Run(); err != nil { + t.Fatalf("subagent failed: %v\nstdout=%s\nstderr=%s", err, stdout.String(), stderr.String()) + } + select { + case requestModel := <-requestModels: + if requestModel != "task-model" { + t.Fatalf("provider received model %q, want task-model", requestModel) + } + default: + t.Fatal("no provider request received") + } + var result subagentResult + if err := json.Unmarshal(stdout.Bytes(), &result); err != nil { + t.Fatalf("invalid result: %v: %s", err, stdout.String()) + } + if result.Status != "success" { + t.Fatalf("child failed: %+v", result) + } + if math.Abs(result.CostUSD-0.00028) > 1e-10 { + t.Fatalf("child cost = %g, want 0.00028 from selected model prices; result=%+v", result.CostUSD, result) + } +} diff --git a/cmd/odek/subagent_model_test.go b/cmd/odek/subagent_model_test.go new file mode 100644 index 0000000..52e7183 --- /dev/null +++ b/cmd/odek/subagent_model_test.go @@ -0,0 +1,104 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" +) + +func TestDelegateTasksPerTaskModelInTaskEnvelope(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, "envelopes") + if err := os.Mkdir(marker, 0700); err != nil { + t.Fatal(err) + } + t.Setenv("ODEK_MODEL_MARKER", marker) + script := filepath.Join(dir, "child") + const body = `#!/bin/sh +task="" +while [ "$#" -gt 0 ]; do + if [ "$1" = "--task" ]; then shift; task="$1"; fi + shift +done +id=$(sed -n 's/.*"task_id":"\([^"]*\)".*/\1/p' "$task") +cp "$task" "$ODEK_MODEL_MARKER/$id.json" +echo '{"status":"success","summary":"ok"}' +` + if err := os.WriteFile(script, []byte(body), 0755); err != nil { + t.Fatal(err) + } + tool := &delegateTasksTool{maxConcurrency: 3, odekPath: script, timeout: 10 * time.Second, provider: "parent-provider", model: "parent-model", baseURL: "https://provider.invalid/v1"} + args := `{"tasks":[{"goal":"inherited","provider":"evil","base_url":"https://evil.invalid"},{"goal":"chosen","model":"fast-child"},{"goal":"another","model":"cheap-child"}]}` + if _, err := tool.Call(args); err != nil { + t.Fatalf("delegate call failed: %v", err) + } + entries, err := os.ReadDir(marker) + if err != nil { + t.Fatal(err) + } + if len(entries) != 3 { + t.Fatalf("captured %d child envelopes, want 3", len(entries)) + } + models := make(map[string]bool) + for _, entry := range entries { + data, err := os.ReadFile(filepath.Join(marker, entry.Name())) + if err != nil { + t.Fatal(err) + } + var envelope taskEnvelope + if err := json.Unmarshal(data, &envelope); err != nil { + t.Fatal(err) + } + if envelope.Provider != "parent-provider" || envelope.BaseURL != "https://provider.invalid/v1" { + t.Errorf("parent connection fields changed: %+v", envelope) + } + var expected string + switch envelope.Goal { + case "inherited": + expected = "parent-model" + case "chosen": + expected = "fast-child" + case "another": + expected = "cheap-child" + default: + t.Errorf("unexpected child goal %q", envelope.Goal) + } + if envelope.Model != expected { + t.Errorf("goal %q got model %q, want %q", envelope.Goal, envelope.Model, expected) + } + models[envelope.Model] = true + } + for _, model := range []string{"parent-model", "fast-child", "cheap-child"} { + if !models[model] { + t.Errorf("child envelopes missing model %q: %v", model, models) + } + } + if tool.model != "parent-model" { + t.Fatalf("parent model mutated: %q", tool.model) + } +} + +func TestDelegateTasksRejectsInvalidModelNamesBeforeSpawn(t *testing.T) { + var spawned atomic.Int32 + tool := &delegateTasksTool{maxConcurrency: 1, runTaskFn: func(int, string, string, string, string, string, string, string, string) string { + spawned.Add(1) + return `{"status":"success"}` + }} + for _, model := range []string{"", " ", "\nfast", "\tfast", "bad\x00name", strings.Repeat("x", 257)} { + argsBytes, err := json.Marshal(map[string]any{"tasks": []any{map[string]any{"goal": "test", "model": model}}}) + if err != nil { + t.Fatal(err) + } + result, callErr := tool.Call(string(argsBytes)) + if callErr != nil || !strings.Contains(result, `"error"`) { + t.Errorf("model %q: result=%s err=%v", model, result, callErr) + } + } + if spawned.Load() != 0 { + t.Fatalf("invalid model names spawned %d children", spawned.Load()) + } +} diff --git a/cmd/odek/subagent_tool.go b/cmd/odek/subagent_tool.go index 54ed95c..8cce1fb 100644 --- a/cmd/odek/subagent_tool.go +++ b/cmd/odek/subagent_tool.go @@ -17,6 +17,7 @@ import ( "strings" "sync" "time" + "unicode" "github.com/BackendStack21/odek" "github.com/BackendStack21/odek/internal/artifact" @@ -204,7 +205,7 @@ func (t *delegateTasksTool) SetEventEmitter(fn func(events.Event)) { } func (t *delegateTasksTool) Description() string { - return `Spawn sub-agent processes for independent sub-tasks. Each child has a fresh context — put everything it needs in goal/context. Children never prompt for approvals (denials are listed). Trust never increases downward. Depth is capped. + return `Spawn sub-agent processes for independent sub-tasks. Each child has a fresh context — put everything it needs in goal/context. Children never prompt for approvals (denials are listed). Trust never increases downward. Depth is capped. Set a task's optional model to select another model from the same configured provider (for example, a faster model for a simple reviewer); omitted inherits the parent model. Result delivery — two channels per sub-agent: - Headline: the sub-agent's final answer, capped at ~2000 characters keeping the END (verdicts, next actions) — a leading … marks the cut. @@ -249,6 +250,12 @@ func (t *delegateTasksTool) Schema() any { "type": "string", "description": "Optional. Operator-defined capability profile name (profiles config); its max_risk/allowlist/tool-filter override global config. Call list_subagent_profiles first to pick one. Omitted = subagent.default_profile.", }, + "model": map[string]any{ + "type": "string", + "minLength": 1, + "maxLength": 256, + "description": "Optional. Model name for this child task. Omitted = inherit the parent model.", + }, }, "required": []string{"goal"}, }, @@ -270,12 +277,13 @@ func (t *delegateTasksTool) Call(args string) (string, error) { var input struct { Tasks []struct { - Goal string `json:"goal"` - Context string `json:"context"` - Guidance string `json:"guidance,omitempty"` - TrustLevel string `json:"trust_level,omitempty"` - MaxRisk string `json:"max_risk,omitempty"` - Profile string `json:"profile,omitempty"` + Goal string `json:"goal"` + Context string `json:"context"` + Guidance string `json:"guidance,omitempty"` + TrustLevel string `json:"trust_level,omitempty"` + MaxRisk string `json:"max_risk,omitempty"` + Profile string `json:"profile,omitempty"` + Model *string `json:"model,omitempty"` } `json:"tasks"` Description string `json:"description,omitempty"` } @@ -288,6 +296,17 @@ func (t *delegateTasksTool) Call(args string) (string, error) { if len(input.Tasks) > 8 { return `{"error":"max 8 tasks per call"}`, nil } + selectedModels := make([]string, len(input.Tasks)) + for i, task := range input.Tasks { + if task.Model == nil { + continue + } + model, err := validateSubagentModel(*task.Model) + if err != nil { + return fmt.Sprintf(`{"error":"task %d model: %v"}`, i+1, err), nil + } + selectedModels[i] = model + } // Trust is provenance-derived, not model-declarable. Once this run has // ingested external content, a tool call cannot label attacker-derived // goal/context as trusted to recover MCP access or a looser child policy. @@ -342,11 +361,9 @@ func (t *delegateTasksTool) Call(args string) (string, error) { } t.acquireSem(sem, emitFn, i) run := t.runTaskFn - if run == nil { - run = t.runTask - } + model := selectedModels[i] wg.Add(1) - go func(i int, taskID, goal, ctx, guidance, trust, maxRisk, profile, artifactDir string) { + go func(i int, taskID, goal, ctx, guidance, trust, maxRisk, profile, artifactDir, model string) { defer wg.Done() defer func() { <-sem }() defer func() { @@ -356,11 +373,16 @@ func (t *delegateTasksTool) Call(args string) (string, error) { mu.Unlock() } }() - r := run(i, taskID, goal, ctx, guidance, trust, maxRisk, profile, artifactDir) + var r string + if run != nil { + r = run(i, taskID, goal, ctx, guidance, trust, maxRisk, profile, artifactDir) + } else { + r = t.runTaskWithModel(i, taskID, goal, ctx, guidance, trust, maxRisk, profile, artifactDir, model) + } mu.Lock() results[i] = r mu.Unlock() - }(i, taskID, task.Goal, task.Context, task.Guidance, task.TrustLevel, task.MaxRisk, task.Profile, dirs[i]) + }(i, taskID, task.Goal, task.Context, task.Guidance, task.TrustLevel, task.MaxRisk, task.Profile, dirs[i], model) } // Wait for every goroutine. Never refill a shared limiter's slots to @@ -432,6 +454,10 @@ func (t *delegateTasksTool) chargeParentUsage(tokens int64) { } func (t *delegateTasksTool) runTask(taskIdx int, taskID, goal, taskContext, guidance, trustLevel, maxRisk, profile, artifactDir string) string { + return t.runTaskWithModel(taskIdx, taskID, goal, taskContext, guidance, trustLevel, maxRisk, profile, artifactDir, "") +} + +func (t *delegateTasksTool) runTaskWithModel(taskIdx int, taskID, goal, taskContext, guidance, trustLevel, maxRisk, profile, artifactDir, model string) string { // Parent-side fail-closed validation: an unknown profile name must // fail the task BEFORE a child is spawned — the tool schema promises // "unknown names fail the task", and a silently-bare child would run @@ -480,6 +506,9 @@ func (t *delegateTasksTool) runTask(taskIdx int, taskID, goal, taskContext, guid task.ArtifactRoot = artifactDir task.Provider = t.provider task.Model = t.model + if model != "" { + task.Model = model + } task.BaseURL = t.baseURL if err := json.NewEncoder(taskFile).Encode(task); err != nil { taskFile.Close() @@ -1080,6 +1109,22 @@ func newTaskEnvelope(taskID, goal, context, guidance, trustLevel, maxRisk, profi } } +func validateSubagentModel(model string) (string, error) { + for _, r := range model { + if r < 0x20 || r == 0x7f || unicode.IsControl(r) { + return "", errors.New("contains control characters") + } + } + model = strings.TrimSpace(model) + if model == "" { + return "", errors.New("must be non-empty") + } + if len([]rune(model)) > 256 { + return "", errors.New("is too long (maximum 256 characters)") + } + return model, nil +} + // subagentDeniedEvent is emitted on the runtime event stream for every // policy denial observed by a child sub-agent. // subagentWaitEventThreshold is how long a task may queue on the shared diff --git a/docs/SUBAGENTS.md b/docs/SUBAGENTS.md index ec0f6ad..7f567b5 100644 --- a/docs/SUBAGENTS.md +++ b/docs/SUBAGENTS.md @@ -94,10 +94,14 @@ The `delegate_tasks` tool is available in CLI, REPL, Web UI, Telegram, and headl // accepts persistence, unknown, and unread_exec // (validated at load; unread_exec is enforced by the // trust lockdown, not this clamp). - "profile": { "type": "string" } // Optional. Operator-defined capability profile name + "profile": { "type": "string" }, // Optional. Operator-defined capability profile name // (top-level `profiles` config). Its max_risk, allowlist, // and tool filter OVERRIDE the operator's global config // for this sub-agent. Unknown names fail the task. + "model": { "type": "string", "minLength": 1, "maxLength": 256 } + // Optional. Model ID supported by the parent's provider; + // omitted inherits the parent's model. Provider, endpoint, + // and credentials always remain those of the parent. }, "required": ["goal"] } @@ -227,7 +231,11 @@ For large prompts that exceed CLI argument length limits, use the `--task` flag } ``` -All keys except `goal` are optional. `trust_level` / `max_risk` / `profile` mirror the `delegate_tasks` task fields; `parent_trust` records the spawning agent's trust; `provider` / `model` / `base_url` inherit from the parent run (`delegate_tasks` stamps them — they are not model-controlled tool args) and bind the FD-handed API key to that provider; `budget` carries the parent's remaining budget when `subagent.budget_inherit` is `"share"` — the child enforces `min(operator limits, inherited values)` (zero values are ignored). +All keys except `goal` are optional. `trust_level`, `max_risk`, `profile`, and `model` mirror the `delegate_tasks` task fields; `parent_trust` records the spawning agent's trust. Omitted models inherit the parent's model. Model availability is validated by the provider. + +Through `delegate_tasks`, the model can select only a model ID on the parent's provider. The tool stamps the inherited provider and base URL and passes the API key through the existing file descriptor channel. Provider, endpoint, and credentials are not selectable tool arguments. + +In share mode, `budget` carries the parent's remaining budget; the child enforces `min(operator limits, inherited values)` (zero values are ignored). A shared cost budget requires both input and output prices for the selected model, resolved through an exact `limits.model_prices` match with per-field flat-price fallback; missing prices fail closed. The `delegate_tasks` tool always uses this file-based approach internally. @@ -453,9 +461,55 @@ The sub-agent system has three test layers: E2E tests: - Build the `odek` binary once via `TestMain` - Test the full pipeline: `tool.Call()` → `exec.Command("odek", "subagent", ...)` → JSON stdout → parse -- Require no LLM provider (sub-agent fails on setup, producing JSON error — which is the exact contract verified) +- Require no live LLM provider: setup-failure tests verify error contracts, and a local mock provider verifies selected model IDs and model-specific cost accounting. - Validate: binary exists, stderr emoji protocol, quiet mode, 100KB+ task files via temp files, missing binary graceful degradation +## Choosing a task model + +The main agent can set `tasks[].model` in `delegate_tasks` independently for +each child. Omit it to inherit the parent's current model. A single batch can +mix explicit model selections and inherited models without changing the +parent or sibling tasks. + +For example, ask the main agent: “Run an adversarial review with a Flash model, +and have another subagent review the test coverage using your current model.” +For a parent configured to use DeepSeek, the tool arguments can be: + +```json +{ + "tasks": [ + { + "goal": "Review the authentication change for bypasses and report findings", + "model": "deepseek-v4-flash", + "max_risk": "safe", + "context": "Review the implementation and tests. Do not modify files." + }, + { + "goal": "Review authentication test coverage and report missing cases", + "max_risk": "safe", + "context": "Review the tests independently. Do not modify files." + } + ] +} +``` + +The first child requests `deepseek-v4-flash`; the second inherits the parent's +model. Use an exact model ID supported by your provider. “Flash” and “cheaper” +are task guidance, not built-in model aliases or automatic price selectors. +The provider validates model availability; odek does not discover available +models or switch providers for a task. Provider, endpoint, and credentials +remain inherited from the parent. + +Explicit model names must contain 1–256 characters after trimming surrounding +spaces and must not contain control characters. Empty or invalid names reject +the batch before any child starts. Omit `model` to request inheritance. + +Model selection does not change capability profiles, trust restrictions, or +execution limits. With a shared cost budget, both input and output prices must +resolve for the selected model through `limits.model_prices` or the flat-price +fallback; otherwise the child fails before making an LLM call. Configure +per-model prices when comparing spend across different models. + ## Example: End-to-end flow ```