diff --git a/kernel/relayflowd-core/src/machine/parallel_tests.rs b/kernel/relayflowd-core/src/machine/parallel_tests.rs index c5ec62137..c058bda81 100644 --- a/kernel/relayflowd-core/src/machine/parallel_tests.rs +++ b/kernel/relayflowd-core/src/machine/parallel_tests.rs @@ -248,11 +248,32 @@ fn overlapping_agent_surfaces_are_serialized_in_authored_order() { let mut recovering_entries = entries.clone(); recovering_entries.extend(appended_entries(&recovery_actions(&running, 12))); let recovering = RunState::fold("run", spec.clone(), &recovering_entries).unwrap(); + // The invariant is that the sibling does not pass the unfinished lane. It + // used to be spelled "every action is an ArmTimer", which held only because + // a dead leased attempt served a retry delay there was a timer to arm for. + // Recovery no longer imposes that delay (see `abandonment_actions`), so the + // lane is due at once and the pass wakes it instead of sleeping. Assert the + // invariant itself rather than the mechanism, which is strictly stronger: + // no action may start the conflicting sibling, whether or not a timer is + // involved. + let recovering_actions = next_actions(&recovering, 12); assert!( - next_actions(&recovering, 12) - .iter() - .all(|action| matches!(action, Action::ArmTimer { .. })), - "the conflicting sibling must not pass an unfinished lane in retry backoff" + !recovering_actions.iter().any(|action| matches!( + action, + Action::Append(entry) + if entry.entry_type == EntryType::StepAttemptStarted + && entry.step_id.as_deref() == Some("lane-a") + )), + "the conflicting sibling must not pass an unfinished lane being recovered" + ); + assert!( + recovering_actions.iter().any(|action| matches!( + action, + Action::Append(entry) + if entry.entry_type == EntryType::WaitCompleted + && entry.step_id.as_deref() == Some("lane-b") + )), + "the recovered lane must be woken before any sibling is considered" ); entries.push(agent_success(&spec, "lane-b", "rB", 12)); @@ -432,3 +453,4 @@ fn failed_run_drains_open_siblings_before_terminal_entry() { Err(StateError::EntryAfterRunCompleted { .. }) )); } + diff --git a/kernel/relayflowd-core/src/machine/recovery.rs b/kernel/relayflowd-core/src/machine/recovery.rs index 85e697ba6..201d084f3 100644 --- a/kernel/relayflowd-core/src/machine/recovery.rs +++ b/kernel/relayflowd-core/src/machine/recovery.rs @@ -4,7 +4,7 @@ use serde_json::Value; -use super::{Action, backoff_delay_ms, deterministic_ulid, idempotency_key, retry_wait_id}; +use super::{Action, deterministic_ulid, retry_wait_id}; use crate::{ entry::{ Budget, CompletionReason, Disposition, EntryType, JournalEntry, Pins, SleepUntilPayload, @@ -86,13 +86,27 @@ pub fn abandonment_actions( } ); let may_retry = runtime.semantic_executions < spec.max_iterations; - let next_attempt_at_ms = (may_retry && !manual).then(|| { - now_ms.saturating_add(backoff_delay_ms( - &spec.retry, - &idempotency_key(&state.run_id, step_id), - attempt, - ) as i64) - }); + // No retry delay for a dead leased attempt. This function records an + // attempt that died WITHOUT producing a result a gate could judge -- which + // is why, as the doc comment above says, it does not charge a semantic + // iteration either. Rate-limiting it is the same category error: the + // backoff curve exists to damp a step that keeps failing on its own merits, + // not one whose worker was killed. + // + // Leaving the delay in place also made recovery order a race, which is + // issue #155. The dead lane sat in `Backoff` with `wake_at_ms` a few + // milliseconds in the future; the scheduler's `due_waits` only fires once + // that passes, and `due_waits` is returned ahead of any `starts`. So + // whether the recovered lane or an idle sibling claimed the one free worker + // depended purely on when the next pass landed relative to that deadline: + // + // PROBE pass now=1788503129020 + // states=[("lane-b", Backoff { wake_at_ms: 1788503129025 }), + // ("lane-a", Runnable)] due_waits=0 + // + // Five milliseconds decided it, and `worker_capacity.rs:149` saw `lane-a` + // instead of the retried `lane-b` in roughly 15% of runs. + let next_attempt_at_ms = (may_retry && !manual).then_some(now_ms); let mut actions = vec![Action::Append(JournalEntry::new( EntryType::StepCompleted, state.run_id.clone(),