fix(router): stop counting rotation as a retry (B4)

Router.attempts (checked against RetryPolicy.MaxAttempts) advanced on
every TaskReleased and again on every subsequent lease. Rotation is
TaskReleased carrying a valid handoff_ref (spec §5.3: "rotation =
intra-task lease transfer"), not a failure — so a task healthy enough to
rotate twice hit the default MaxAttempts=3 and was killed by the retry
limit meant for genuine failures (expiry, crash).

HandleEvent now only advances attempts (and applies retry backoff) for a
TaskReleased whose payload lacks a handoff_ref. AssignPending no longer
increments attempts at lease time at all — that was double-counting
against the same failure that a subsequent non-rotation release already
counts.

TestRotationDoesNotCountAgainstRetryLimit drives a task through 5
rotate-and-release cycles with MaxAttempts=3 and asserts it never reaches
TaskFailed.

AUDIT.md B4.
This commit is contained in:
kami
2026-07-27 19:09:48 +04:00
parent 8af6bbdc8e
commit 7211238590
2 changed files with 84 additions and 4 deletions
+18 -4
View File
@@ -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 {
+66
View File
@@ -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