Skip to content

Commit 60be191

Browse files
authored
fix: preserve cook attempts during Lab transport readiness (#9044)
1 parent 83c56d2 commit 60be191

1 file changed

Lines changed: 97 additions & 26 deletions

File tree

  • crates/homeboy-agents/src/agent_task_service

crates/homeboy-agents/src/agent_task_service/cook.rs

Lines changed: 97 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,12 @@ where
624624
} else {
625625
options
626626
};
627+
// Transport readiness can serialize on a reconnect/runtime-promotion
628+
// lease. Complete it before entering the provider-attempt loop so that
629+
// waiting for a shared Lab session never consumes a cook attempt.
630+
if let Some(dispatcher) = &options.attempt_dispatcher {
631+
dispatcher.prepare_for_cook()?;
632+
}
627633
let max_attempts = options.max_attempts.max(1);
628634
let mut attempts = Vec::new();
629635
let mut run_id = options.initial_run_id.clone();
@@ -653,9 +659,6 @@ where
653659
.unwrap_or(true);
654660
if needs_execution {
655661
let execution = (|| {
656-
if let Some(dispatcher) = &options.attempt_dispatcher {
657-
dispatcher.prepare_for_cook()?;
658-
}
659662
let initial_baseline = if attempt == 1 {
660663
materialize_initial_candidate_baseline(
661664
&plan,
@@ -2024,7 +2027,7 @@ mod tests {
20242027
RunLifecycleRecord,
20252028
};
20262029
use std::sync::atomic::{AtomicUsize, Ordering};
2027-
use std::sync::Barrier;
2030+
use std::sync::{Barrier, Condvar};
20282031

20292032
#[test]
20302033
fn cook_service_retry_uses_the_same_passed_context_after_ambient_mutation() {
@@ -2141,6 +2144,13 @@ mod tests {
21412144
failures_remaining: AtomicUsize,
21422145
}
21432146

2147+
#[derive(Debug)]
2148+
struct QueuedPreparationDispatcher {
2149+
barrier: Arc<Barrier>,
2150+
state: Arc<(Mutex<(bool, bool)>, Condvar)>,
2151+
connections: Arc<AtomicUsize>,
2152+
}
2153+
21442154
impl AgentTaskCookAttemptDispatcher for FlakyPreparationDispatcher {
21452155
fn durable_recipe(&self) -> Result<Value> {
21462156
Ok(serde_json::json!({ "kind": "test-flaky-preparation" }))
@@ -2178,6 +2188,46 @@ mod tests {
21782188
}
21792189
}
21802190

2191+
impl AgentTaskCookAttemptDispatcher for QueuedPreparationDispatcher {
2192+
fn durable_recipe(&self) -> Result<Value> {
2193+
Ok(serde_json::json!({ "kind": "test-queued-preparation" }))
2194+
}
2195+
2196+
fn prepare_for_cook(&self) -> Result<()> {
2197+
self.barrier.wait();
2198+
let (state_mutex, ready) = &*self.state;
2199+
let mut state = state_mutex.lock().expect("queued preparation state");
2200+
if state.1 {
2201+
return Ok(());
2202+
}
2203+
if state.0 {
2204+
while !state.1 {
2205+
state = ready.wait(state).expect("queued preparation wait");
2206+
}
2207+
return Ok(());
2208+
}
2209+
state.0 = true;
2210+
drop(state);
2211+
2212+
self.connections.fetch_add(1, Ordering::SeqCst);
2213+
std::thread::sleep(std::time::Duration::from_millis(50));
2214+
2215+
let mut state = state_mutex.lock().expect("queued preparation owner state");
2216+
state.1 = true;
2217+
ready.notify_all();
2218+
Ok(())
2219+
}
2220+
2221+
fn dispatch_attempt(
2222+
&self,
2223+
_plan: AgentTaskPlan,
2224+
_run_id: &str,
2225+
_derived_cook_baseline: Option<&DerivedCookBaselineCapability>,
2226+
) -> Result<()> {
2227+
panic!("transport preparation test does not dispatch a provider attempt")
2228+
}
2229+
}
2230+
21812231
impl AgentTaskCookAttemptDispatcher for AdmissionFailingAttemptDispatcher {
21822232
fn durable_recipe(&self) -> Result<Value> {
21832233
Ok(serde_json::json!({ "kind": "test-admission-failure" }))
@@ -2362,7 +2412,7 @@ mod tests {
23622412
}
23632413

23642414
#[test]
2365-
fn cook_retries_runner_unavailable_under_one_durable_identity() {
2415+
fn cook_transport_preparation_failure_does_not_create_a_provider_attempt() {
23662416
homeboy_core::test_support::with_isolated_home(|_| {
23672417
let cook_id = "cook-runner-unavailable";
23682418
let first_run_id = "cook-runner-unavailable-attempt-1";
@@ -2376,18 +2426,16 @@ mod tests {
23762426
options.initial_run_id = first_run_id.to_string();
23772427
options.max_attempts = 2;
23782428

2379-
let result = run_cook(options, UnusedExecutor).expect("cook recovers runner admission");
2429+
let error = run_cook(options, UnusedExecutor)
2430+
.expect_err("transport preparation is outside the provider-attempt loop");
23802431

2381-
assert_eq!(result.exit_code, 0);
2382-
assert_eq!(result.value.status, "in_flight");
2383-
assert_eq!(result.value.history_run_ids.len(), 2);
2384-
assert_eq!(result.value.history_run_ids[0], first_run_id);
2385-
let failed = agent_task_lifecycle::status(first_run_id).expect("failed attempt");
2386-
assert!(failed.provider_handles.is_empty());
2387-
assert_eq!(failed.metadata["provider_executions_consumed"], 0);
2388-
let resumed =
2389-
agent_task_lifecycle::status(cook_id).expect("cook alias resolves latest");
2390-
assert_eq!(resumed.run_id, result.value.history_run_ids[1]);
2432+
assert!(error.message.contains("fixture runner is unavailable"));
2433+
assert!(!agent_task_lifecycle::run_record_exists(first_run_id)
2434+
.expect("transport failure does not materialize an attempt"));
2435+
assert!(
2436+
agent_task_lifecycle::cook_index(cook_id).is_err(),
2437+
"transport failure must not consume a cook attempt"
2438+
);
23912439
});
23922440
}
23932441

@@ -2416,7 +2464,7 @@ mod tests {
24162464
}
24172465

24182466
#[test]
2419-
fn cook_terminally_exhausts_pre_execution_retries_without_provider_budget() {
2467+
fn cook_transport_preparation_failure_does_not_exhaust_cook_retries() {
24202468
homeboy_core::test_support::with_isolated_home(|_| {
24212469
let cook_id = "cook-runner-exhaustion";
24222470
let mut options = batch_cook_options(
@@ -2429,17 +2477,40 @@ mod tests {
24292477
options.initial_run_id = "cook-runner-exhaustion-attempt-1".to_string();
24302478
options.max_attempts = 2;
24312479

2432-
let result = run_cook(options, UnusedExecutor).expect("cook reports exhaustion");
2480+
let error = run_cook(options, UnusedExecutor)
2481+
.expect_err("transport preparation remains outside cook retries");
24332482

2434-
assert_eq!(result.exit_code, 1);
2435-
assert_eq!(result.value.status, "retries_exhausted");
2436-
assert_eq!(result.value.history_run_ids.len(), 2);
2437-
for run_id in &result.value.history_run_ids {
2438-
let record = agent_task_lifecycle::status(run_id).expect("failed attempt");
2439-
assert!(record.provider_handles.is_empty());
2440-
assert_eq!(record.metadata["provider_executions_consumed"], 0);
2441-
}
2483+
assert!(error.message.contains("fixture runner is unavailable"));
2484+
assert!(
2485+
!agent_task_lifecycle::run_record_exists("cook-runner-exhaustion-attempt-1")
2486+
.expect("transport failure does not materialize an attempt")
2487+
);
2488+
});
2489+
}
2490+
2491+
#[test]
2492+
fn concurrent_cooks_share_transport_readiness_before_first_provider_attempt() {
2493+
const COOKS: usize = 6;
2494+
let connections = Arc::new(AtomicUsize::new(0));
2495+
let dispatcher = Arc::new(QueuedPreparationDispatcher {
2496+
barrier: Arc::new(Barrier::new(COOKS)),
2497+
state: Arc::new((Mutex::new((false, false)), Condvar::new())),
2498+
connections: Arc::clone(&connections),
24422499
});
2500+
let preparations = (0..COOKS)
2501+
.map(|_| {
2502+
let dispatcher = Arc::clone(&dispatcher);
2503+
std::thread::spawn(move || dispatcher.prepare_for_cook())
2504+
})
2505+
.collect::<Vec<_>>();
2506+
2507+
for preparation in preparations {
2508+
preparation
2509+
.join()
2510+
.expect("cook preparation thread")
2511+
.expect("shared transport becomes ready");
2512+
}
2513+
assert_eq!(connections.load(Ordering::SeqCst), 1);
24432514
}
24442515

24452516
#[test]

0 commit comments

Comments
 (0)