From 7978760316319d22670cd6369c15b68c649761bc Mon Sep 17 00:00:00 2001
From: Peter Stone <thepeterstone@gmail.com>
Date: Sat, 11 Jul 2026 08:32:45 +0000
Subject: fix(scheduler): retry role-typed FAILED tasks even with no active
 role_configs

Before this, a role with no seeded role_configs (every role except
builder/planner) got no retry safety net at all: processTask returned
immediately on GetActiveRoleConfig's error, leaving a task FAILED forever
after a single transient infra blip. retryWithoutLadder mirrors
escalateAskUserTimeout's existing 'no active role config -- still
resuming, just without a provider/model change' fallback, bounded by
maxRetriesNoRoleConfig (2) before declining final=true. Same treatment
now applies to a decoded role config with an empty EscalationLadder.
---
 internal/scheduler/scheduler_test.go | 94 ++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

(limited to 'internal/scheduler/scheduler_test.go')

diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go
index 258bb5d..5e71d34 100644
--- a/internal/scheduler/scheduler_test.go
+++ b/internal/scheduler/scheduler_test.go
@@ -417,6 +417,100 @@ func TestScheduler_DeclinesFinal_WhenLadderExhausted(t *testing.T) {
 	}
 }
 
+// TestScheduler_RetriesWithoutRoleConfig_WhenNoActiveConfig proves the
+// 2026-07-11 fix: a role-typed task whose role has no active role_configs
+// row at all must still get a bounded retry at its existing Agent.Type/
+// Model, not be left FAILED forever with nothing ever retrying it. Before
+// this fix, processTask returned immediately on GetActiveRoleConfig's
+// error, so a role like "evaluator_quality" (which, unlike builder/planner,
+// has no seeded role_configs) would never recover from a single transient
+// infra failure.
+func TestScheduler_RetriesWithoutRoleConfig_WhenNoActiveConfig(t *testing.T) {
+	store := newFakeStore()
+	// Deliberately no seedRoleConfig call -- this role has no active config.
+
+	tk := failedTask("t1", "evaluator_quality", "claude")
+	store.tasks[tk.ID] = tk
+	store.execsByTask[tk.ID] = []*storage.Execution{failedExec(0)}
+
+	pool := &fakePool{}
+	sch := &Scheduler{Store: store, Pool: pool}
+	sch.Tick(context.Background())
+
+	if pool.submitCount() != 1 {
+		t.Fatalf("expected 1 retry submission, got %d", pool.submitCount())
+	}
+	resubmitted := pool.submitted[0]
+	if resubmitted.Agent.Type != "claude" {
+		t.Errorf("retry without a role config should keep the existing provider unchanged: got %q", resubmitted.Agent.Type)
+	}
+	if resubmitted.State != task.StateQueued {
+		t.Errorf("resubmitted task should be QUEUED, got %v", resubmitted.State)
+	}
+	if len(store.eventsOfKind(event.KindEscalated)) != 0 {
+		t.Errorf("a bounded retry (not yet exhausted) should not emit a KindEscalated event")
+	}
+}
+
+// TestScheduler_DeclinesFinal_WhenNoRoleConfigRetriesExhausted proves the
+// safety net on the fallback path above: once maxRetriesNoRoleConfig
+// consecutive failures have happened with no role config to consult, the
+// scheduler stops retrying and declines with final=true, exactly like the
+// ladder-exhaustion case does for a role that does have a config.
+func TestScheduler_DeclinesFinal_WhenNoRoleConfigRetriesExhausted(t *testing.T) {
+	store := newFakeStore()
+
+	tk := failedTask("t1", "evaluator_quality", "claude")
+	store.tasks[tk.ID] = tk
+	execs := make([]*storage.Execution, maxRetriesNoRoleConfig)
+	for i := range execs {
+		execs[i] = failedExec(0)
+	}
+	store.execsByTask[tk.ID] = execs
+
+	pool := &fakePool{}
+	sch := &Scheduler{Store: store, Pool: pool}
+	sch.Tick(context.Background())
+
+	if pool.submitCount() != 0 {
+		t.Fatalf("exhausted no-role-config retries should not resubmit, got %d submissions", pool.submitCount())
+	}
+	evs := store.eventsOfKind(event.KindEscalated)
+	if len(evs) != 1 {
+		t.Fatalf("expected 1 KindEscalated event, got %d", len(evs))
+	}
+	var payload struct {
+		Final bool `json:"final"`
+	}
+	if err := json.Unmarshal(evs[0].Payload, &payload); err != nil {
+		t.Fatalf("unmarshal event payload: %v", err)
+	}
+	if !payload.Final {
+		t.Errorf("exhausted no-role-config retries should emit final=true")
+	}
+}
+
+// TestScheduler_RetriesWithoutRoleConfig_EmptyLadder proves the same
+// fallback applies when a role_configs row does exist and is active but its
+// EscalationLadder is empty -- mirroring escalateAskUserTimeout's identical
+// treatment of "no active config" and "empty ladder" as the same case.
+func TestScheduler_RetriesWithoutRoleConfig_EmptyLadder(t *testing.T) {
+	store := newFakeStore()
+	seedRoleConfig(t, store, role.RoleConfig{Role: "coder"}) // no EscalationLadder set
+
+	tk := failedTask("t1", "coder", "claude")
+	store.tasks[tk.ID] = tk
+	store.execsByTask[tk.ID] = []*storage.Execution{failedExec(0)}
+
+	pool := &fakePool{}
+	sch := &Scheduler{Store: store, Pool: pool}
+	sch.Tick(context.Background())
+
+	if pool.submitCount() != 1 {
+		t.Fatalf("expected 1 retry submission for an empty ladder, got %d", pool.submitCount())
+	}
+}
+
 // TestScheduler_Convergence_DoesNotReprocessSameExecution proves the
 // double-processing guard: ticking twice against the same unchanged FAILED
 // execution only acts once (no duplicate submissions or events), which is
-- 
cgit v1.2.3