diff --git a/internal/router/router.go b/internal/router/router.go index 1d7bd7f..39a74c4 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -124,9 +124,20 @@ func (r *Router) HandleEvent(e domain.Event) ([]domain.Event, error) { return nil, nil } if e.Type == "TaskReleased" { - r.attempts[e.TaskID]++ - if r.Retry.Backoff > 0 { - r.backoff[e.TaskID] = r.Now().Add(r.Retry.Backoff) + // Rotation *is* TaskReleased (spec §5.3: "rotation = intra-task + // lease transfer") — a task healthy enough to rotate repeatedly + // must not be killed by the retry limit meant for genuine failures + // (expiry, crash). Only a release without a valid handoff_ref + // (expiry/crash) counts against MaxAttempts. + var p struct { + HandoffRef string `json:"handoff_ref"` + } + isRotation := json.Unmarshal(e.Payload, &p) == nil && p.HandoffRef != "" + if !isRotation { + r.attempts[e.TaskID]++ + if r.Retry.Backoff > 0 { + r.backoff[e.TaskID] = r.Now().Add(r.Retry.Backoff) + } } } return r.AssignPending() @@ -166,7 +177,10 @@ func (r *Router) AssignPending() ([]domain.Event, error) { if err != nil { continue } - r.attempts[t.ID]++ + // attempts is the failure counter checked against MaxAttempts + // above; it advances only on a non-rotation TaskReleased (see + // HandleEvent), not here, so a task that leases and rotates + // repeatedly is not double-counted toward the retry limit. out = append(out, e) if r.OnLease != nil { if err := r.OnLease(e); err != nil { diff --git a/internal/router/router_test.go b/internal/router/router_test.go index d0e49c6..09df11a 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -45,6 +45,72 @@ func TestAssignsByAffinityCapabilityAndConcurrency(t *testing.T) { } } +// TestRotationDoesNotCountAgainstRetryLimit guards B4: rotation is +// TaskReleased carrying a valid handoff_ref (spec §5.3: "rotation = +// intra-task lease transfer"), never a failure. A task healthy enough to +// rotate repeatedly must survive past MaxAttempts, which is a retry policy +// for genuine failures (expiry/crash releases without a handoff_ref), not +// for lease transfers. +func TestRotationDoesNotCountAgainstRetryLimit(t *testing.T) { + s, err := store.Open(t.TempDir()) + if err != nil { + t.Fatal(err) + } + r, err := registry.New(registry.Config{ + Projects: []registry.Project{{ID: "p", MachineAffinity: []string{"m"}}}, + Machines: []registry.Machine{{ID: "m", Address: "unused"}}, + Herdrs: []registry.Herdr{{ID: "h", MachineID: "m", Capabilities: []string{"go"}, Concurrency: 1}}, + }) + if err != nil { + t.Fatal(err) + } + b, _ := json.Marshal(map[string]any{"source": "test", "external_id": "a", "project": "p", "capability": []string{"go"}}) + if err := s.Append(domain.Event{ID: "a", TaskID: "a", Type: "TaskCreated", Version: 1, Payload: b, Surface: string(authz.System)}); err != nil { + t.Fatal(err) + } + rt := Router{Store: s, Registry: r, Reachability: reachable{}, Retry: RetryPolicy{MaxAttempts: 3}} + handoffRef, err := s.PutArtifact([]byte("handoff")) + if err != nil { + t.Fatal(err) + } + + for i := 0; i < 5; i++ { + got, err := rt.AssignPending() + if err != nil { + t.Fatal(err) + } + task, ok := s.Task("a") + if !ok { + t.Fatal("task missing") + } + if task.State == domain.StateFailed { + t.Fatalf("task failed after %d rotations, retry limit wrongly counted rotation as a failure", i) + } + if task.State != domain.StateLeased { + if len(got) == 0 { + t.Fatalf("round %d: task not leased and nothing assigned (state=%v)", i, task.State) + } + continue + } + rb, _ := json.Marshal(map[string]string{ + "handoff_ref": handoffRef, + "reason": "threshold", + "anchor_sha": "0123456789abcdef0123456789abcdef01234567", + }) + release := domain.Event{ID: domain.NewID(), Type: "TaskReleased", TaskID: "a", Version: task.Version + 1, Payload: rb, Surface: string(authz.System)} + if err := s.Append(release); err != nil { + t.Fatal(err) + } + if _, err := rt.HandleEvent(release); err != nil { + t.Fatal(err) + } + } + task, _ := s.Task("a") + if task.State == domain.StateFailed { + t.Fatal("task failed after 5 rotations, want still alive") + } +} + // TestQuotaWindowsAreIndependent proves the 5-hour rolling window and the // weekly window (spec §7.2, §9 item 1) are each conservative-80%-full gates // on their own — a harness can be fine on one window and excluded by the