diff options
| -rw-r--r-- | internal/scheduler/scheduler.go | 48 | ||||
| -rw-r--r-- | internal/scheduler/scheduler_test.go | 94 |
2 files changed, 136 insertions, 6 deletions
diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 6df854b..cc6fa3a 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -205,24 +205,26 @@ func (s *Scheduler) processTask(ctx context.Context, t *task.Task) { return // already decided for this execution — converged, nothing to do. } + currentRung := latest.EscalationRung + if currentRung < 0 { + currentRung = 0 + } + row, err := s.Store.GetActiveRoleConfig(t.Agent.Role) if err != nil { - s.logf("scheduler: no active role config for role", "role", t.Agent.Role, "taskID", t.ID, "error", err) + s.retryWithoutLadder(ctx, t, execs, currentRung, "no active role config for role "+t.Agent.Role) return } var rc role.RoleConfig if err := json.Unmarshal([]byte(row.ConfigJSON), &rc); err != nil { - s.logf("scheduler: decode role config", "role", t.Agent.Role, "taskID", t.ID, "error", err) + s.retryWithoutLadder(ctx, t, execs, currentRung, "failed to decode role config") return } if len(rc.EscalationLadder) == 0 { + s.retryWithoutLadder(ctx, t, execs, currentRung, "empty escalation ladder") return } - currentRung := latest.EscalationRung - if currentRung < 0 { - currentRung = 0 - } if currentRung >= len(rc.EscalationLadder) { // Ladder already exhausted (e.g. the ladder was shortened after this // task started climbing it) — nothing more to do. @@ -291,6 +293,40 @@ func (s *Scheduler) retrySameRung(ctx context.Context, t *task.Task, rung int) { } } +// maxRetriesNoRoleConfig bounds retryWithoutLadder's same-tier retries when +// a role-typed task has no usable escalation ladder to consult -- a simple, +// hardcoded safety net (mirrors maxFixAttempts in +// internal/scheduler/story_orchestrator.go), not real ladder-based tiering. +const maxRetriesNoRoleConfig = 2 + +// retryWithoutLadder handles a role-typed task whose role has no usable +// escalation ladder to consult -- no active role_configs row at all, a +// corrupt one, or one with an empty ladder -- by retrying at the task's +// existing Agent.Type/Model (unchanged, since there's no ladder to pick a +// tier from) up to maxRetriesNoRoleConfig times before giving up. Mirrors +// escalateAskUserTimeout's own "no active role config... still resuming, +// just without a provider/model change" fallback below, applied here to the +// FAILED-task retry path instead of the ask_user-timeout path. +// +// Added 2026-07-11: before this, a role with no seeded role_configs (every +// role except builder/planner, as of this fix -- see +// internal/storage.SeedRoleConfigs) got zero retry safety net at all: the +// very first transient infra failure (a generic "container execution failed: +// exit status 1", the same symptom repeatedly traced to disk-full or other +// non-deterministic causes this session) left the task FAILED forever with +// nothing ever looking at it again -- a real production incident (see +// internal/scheduler/story_orchestrator.go's finalizeArbitration doc +// comment for the related arbitration-side half of this same root cause). +func (s *Scheduler) retryWithoutLadder(ctx context.Context, t *task.Task, execs []*storage.Execution, rung int, reason string) { + attempts := attemptsAtRung(execs, rung) + if attempts < maxRetriesNoRoleConfig { + s.logf("scheduler: retrying without a usable escalation ladder", "role", t.Agent.Role, "taskID", t.ID, "reason", reason, "attempt", attempts+1, "maxAttempts", maxRetriesNoRoleConfig) + s.retrySameRung(ctx, t, rung) + return + } + s.decline(ctx, t, rung, "", reason+"; retries exhausted") +} + func (s *Scheduler) escalate(ctx context.Context, t *task.Task, fromRung, toRung int, target role.Rung) { fromProvider := t.Agent.Type newAgent := t.Agent 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 |