From 997cd8b56bc086a02b9c7c006dd62b07b9fcd2f3 Mon Sep 17 00:00:00 2001 From: Claude Sonnet 5 Date: Fri, 3 Jul 2026 23:22:42 +0000 Subject: feat(executor): add DAG auto-cascade-fail + role-typed subtask spawning (Phase 6) Two prerequisites for safe parallel evaluator fan-out (later phase): 1. Auto-cascade-fail: previously, a failed task's dependents just sat PENDING/QUEUED forever (or until something eventually tried to dispatch them and discovered the dependency was dead). Pool.cascadeFail now fires right after a task lands in a terminal failure state (FAILED/TIMED_OUT/ CANCELLED/BUDGET_EXCEEDED, from handleRunResult, the budget-gate reject path, and the checkDepsReady dependency-failure path), recursively cancelling every not-yet-run dependent (PENDING/QUEUED only -- RUNNING and terminal states are left alone) with a message referencing the upstream failure. A visited-set guards recursion, which turned out to be load-bearing rather than defense-in-depth: task creation does not prevent dependency cycles anywhere in this codebase. Correction to an earlier assumption: internal/executor's waitForDependencies is dead code, never called. The live mechanism is checkDepsReady, invoked synchronously in execute() with a self-requeue via time.AfterFunc. Added a freshness re-check (GetTask, bail if no longer QUEUED) at the top of that block so a task cascade-cancelled while sitting in the requeue loop stops silently instead of hitting an invalid CANCELLED->CANCELLED transition or, worse, still getting dispatched. 2. storeChannel.SpawnSubtask hardcoded Agent.Type: "claude" on every spawned child regardless of what role it should play -- a hard blocker for a Planner/Builder task spawning role-typed evaluator subtasks. SubtaskSpec (internal/agentchannel) gains a Role field; when set, the child task gets Agent.Role instead of a hardcoded Type, so Phase 5's role-resolution picks provider/model from that role's escalation ladder. spec.Role == "" (every existing caller) preserves today's exact behavior byte-for-byte -- proven by an explicit regression test, not just new-feature coverage. Threaded the new `role` parameter through both spawn_subtask transports: the native tool-use loop (internal/agentloop/tools.go) and the MCP tool exposed to ContainerRunner-driven claude/gemini agents (internal/executor/agentmcp.go). go build/vet/test -race -count=1 all pass, full suite (20 packages). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01V1moSNCJRcP6kykA4tyUSs --- internal/executor/channel_test.go | 82 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) (limited to 'internal/executor/channel_test.go') diff --git a/internal/executor/channel_test.go b/internal/executor/channel_test.go index 250e8eb..157cb0c 100644 --- a/internal/executor/channel_test.go +++ b/internal/executor/channel_test.go @@ -97,6 +97,88 @@ func TestStoreChannel_SpawnSubtask_CreatesChildWithParent(t *testing.T) { } } +// TestStoreChannel_SpawnSubtask_RoleSet verifies that when spec.Role is set, +// the child task gets Agent.Role populated and Agent.Type/Model left empty +// so executor.Pool.execute()'s role-based dispatch (Phase 5) resolves them +// from the role's escalation ladder, exactly like any other role-typed task. +func TestStoreChannel_SpawnSubtask_RoleSet(t *testing.T) { + store := &fakeChannelStore{} + ch := newStoreChannel(store, "parent-1") + id, err := ch.SpawnSubtask(context.Background(), SubtaskSpec{ + Name: "evaluator", + Instructions: "evaluate the change", + Model: "should-be-ignored", + MaxBudgetUSD: 2.0, + Role: "evaluator_quality", + }) + if err != nil { + t.Fatalf("SpawnSubtask: %v", err) + } + if len(store.createdTasks) != 1 { + t.Fatalf("expected 1 created task, got %d", len(store.createdTasks)) + } + ct := store.createdTasks[0] + if ct.ID != id { + t.Errorf("returned id %q != created task id %q", id, ct.ID) + } + if ct.Agent.Role != "evaluator_quality" { + t.Errorf("Agent.Role: want evaluator_quality, got %q", ct.Agent.Role) + } + if ct.Agent.Type != "" { + t.Errorf("Agent.Type: want empty (resolved later by role dispatch), got %q", ct.Agent.Type) + } + if ct.Agent.Model != "" { + t.Errorf("Agent.Model: want empty (resolved later by role dispatch), got %q", ct.Agent.Model) + } + if ct.Agent.Instructions != "evaluate the change" || ct.Agent.MaxBudgetUSD != 2.0 { + t.Errorf("non-role fields not propagated: %+v", ct.Agent) + } +} + +// TestStoreChannel_SpawnSubtask_NoRole_RegressionUnchanged is a regression +// test proving that spec.Role == "" (every pre-Phase-6 caller) still +// produces exactly today's behavior: Agent.Type hardcoded to "claude" and +// Agent.Model set from spec.Model. This must keep passing unmodified by any +// future change to role-typed spawning. +func TestStoreChannel_SpawnSubtask_NoRole_RegressionUnchanged(t *testing.T) { + store := &fakeChannelStore{} + ch := newStoreChannel(store, "parent-1") + id, err := ch.SpawnSubtask(context.Background(), SubtaskSpec{ + Name: "child", + Instructions: "do it", + Model: "sonnet", + MaxBudgetUSD: 1.5, + }) + if err != nil { + t.Fatalf("SpawnSubtask: %v", err) + } + if len(store.createdTasks) != 1 { + t.Fatalf("expected 1 created task, got %d", len(store.createdTasks)) + } + ct := store.createdTasks[0] + if ct.ID != id { + t.Errorf("returned id %q != created task id %q", id, ct.ID) + } + if ct.ParentTaskID != "parent-1" { + t.Errorf("ParentTaskID: got %q want parent-1", ct.ParentTaskID) + } + if ct.Agent.Type != "claude" { + t.Errorf("Agent.Type: want claude (unchanged backward-compat default), got %q", ct.Agent.Type) + } + if ct.Agent.Model != "sonnet" { + t.Errorf("Agent.Model: want sonnet, got %q", ct.Agent.Model) + } + if ct.Agent.Role != "" { + t.Errorf("Agent.Role: want empty, got %q", ct.Agent.Role) + } + if ct.Agent.Instructions != "do it" || ct.Agent.MaxBudgetUSD != 1.5 { + t.Errorf("agent config not propagated: %+v", ct.Agent) + } + if ct.State != task.StatePending { + t.Errorf("expected PENDING child, got %s", ct.State) + } +} + func TestStoreChannel_SpawnSubtask_PropagatesError(t *testing.T) { store := &fakeChannelStore{createTaskErr: errors.New("boom")} ch := newStoreChannel(store, "parent-1") -- cgit v1.2.3