Skip to content

Commit 9fccfe9

Browse files
committed
Release agent-task scratch after evidence harvest
1 parent f3e1f5a commit 9fccfe9

6 files changed

Lines changed: 247 additions & 13 deletions

File tree

src/core/agent_task_provider/tests/scheduler_tests.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,21 @@ fn provider_attempts_receive_distinct_allocated_runtime_tmpdirs() {
465465
let tmpdir = PathBuf::from(attempt["tmp"].as_str().expect("TMPDIR"));
466466
assert!(tmpdir.is_dir());
467467
}
468+
let index: Value = serde_json::from_str(
469+
&fs::read_to_string(
470+
crate::core::paths::homeboy_data()
471+
.expect("homeboy data")
472+
.join("controller-scratch/resources.json"),
473+
)
474+
.expect("scratch index"),
475+
)
476+
.expect("scratch index JSON");
477+
let resources = index["resources"].as_array().expect("scratch resources");
478+
assert_eq!(resources.len(), 2);
479+
assert_eq!(resources[0]["lifecycle_state"], "released");
480+
assert_eq!(resources[0]["terminal_reason"], "retry");
481+
assert_eq!(resources[1]["lifecycle_state"], "released");
482+
assert_eq!(resources[1]["terminal_reason"], "succeeded");
468483
});
469484
}
470485

src/core/agent_task_scheduler/engine.rs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,30 @@ where
360360
if let Some(root) = request.workspace.root.as_deref() {
361361
request.executor.remap_workspace_root(root);
362362
}
363+
let run_id = self.run_id.as_deref().unwrap_or(&plan.plan_id);
364+
let scratch = match crate::core::controller_scratch::allocate_attempt(
365+
run_id,
366+
&plan.plan_id,
367+
&request.task_id,
368+
scheduled.attempt,
369+
) {
370+
Ok(scratch) => scratch,
371+
Err(error) => {
372+
let outcome =
373+
scratch_allocation_failure(task_id.clone(), error.to_string());
374+
events.push(event(
375+
&task_id,
376+
AgentTaskState::Failed,
377+
scheduled.attempt,
378+
outcome.summary.clone(),
379+
));
380+
record_completed_outcome(&mut completed_by_task, &mut outcomes, outcome);
381+
continue;
382+
}
383+
};
384+
request
385+
.executor
386+
.set_runtime_tmpdir(scratch.path.to_string_lossy().as_ref());
363387
let executor_key = executor_key(&request);
364388
let executor = Arc::clone(&self.executor);
365389
let plan_id = plan.plan_id.clone();
@@ -415,6 +439,7 @@ where
415439
artifact_nonce: uuid::Uuid::new_v4().to_string(),
416440
task_base_sha,
417441
source_provenance: harvest_preflight.source_provenance,
442+
scratch: scratch.clone(),
418443
});
419444

420445
thread::spawn(move || {
@@ -424,6 +449,7 @@ where
424449
task_id,
425450
attempt,
426451
outcome,
452+
scratch,
427453
}));
428454
});
429455
}
@@ -468,6 +494,7 @@ where
468494
let Some(running_task) = running_task else {
469495
continue;
470496
};
497+
debug_assert_eq!(result.scratch, running_task.scratch);
471498
let mut outcome = result.outcome;
472499
if let Err(error) = harvest_uncommitted_patch(&mut outcome, &running_task)
473500
.and_then(|_| harvest_committed_patch(&mut outcome, &running_task))
@@ -529,6 +556,7 @@ where
529556
retry_budget_used,
530557
&plan.options.retry.retryable_failure_classifications,
531558
) {
559+
release_scratch(&result.scratch, "retry", &outcome);
532560
retry_budget_used += 1;
533561
let retry_evidence = retry_attempt_evidence(&outcome, &running_task);
534562
let mut retry_attempts = running_task.retry_attempts;
@@ -573,6 +601,7 @@ where
573601
max_attempts,
574602
execution_budget.max_provider_rotations,
575603
) {
604+
release_scratch(&result.scratch, "provider_rotation", &outcome);
576605
let mut rotation_attempts = running_task.rotation_attempts;
577606
rotation_attempts.push(
578607
AgentTaskScheduleSupport::rotation_attempt_record(
@@ -651,6 +680,15 @@ where
651680
result.attempt,
652681
running_task.rotation_index,
653682
);
683+
release_scratch(
684+
&result.scratch,
685+
if running_task.timeout_cancel_requested {
686+
"scheduler_timeout_completion"
687+
} else {
688+
terminal_reason(&outcome, cancellation.is_cancelled())
689+
},
690+
&outcome,
691+
);
654692
record_completed_outcome(&mut completed_by_task, &mut outcomes, outcome);
655693
}
656694
Err(Some(mpsc::RecvTimeoutError::Timeout)) => {}
@@ -743,6 +781,7 @@ pub(super) struct RunningTask {
743781
pub(super) task_base_sha: Option<String>,
744782
/// Verified source identity for snapshot-backed candidate artifacts.
745783
pub(super) source_provenance: Option<serde_json::Value>,
784+
pub(super) scratch: crate::core::controller_scratch::ControllerScratchAllocation,
746785
}
747786

748787
#[derive(Debug, Clone)]
@@ -760,6 +799,7 @@ struct TaskResult {
760799
task_id: String,
761800
attempt: u32,
762801
outcome: AgentTaskOutcome,
802+
scratch: crate::core::controller_scratch::ControllerScratchAllocation,
763803
}
764804

765805
fn retry_attempt_evidence(outcome: &AgentTaskOutcome, running: &RunningTask) -> serde_json::Value {
@@ -774,6 +814,56 @@ fn retry_attempt_evidence(outcome: &AgentTaskOutcome, running: &RunningTask) ->
774814
})
775815
}
776816

817+
fn release_scratch(
818+
allocation: &crate::core::controller_scratch::ControllerScratchAllocation,
819+
reason: &str,
820+
outcome: &AgentTaskOutcome,
821+
) {
822+
let evidence = serde_json::json!({
823+
"task_id": outcome.task_id,
824+
"status": outcome.status,
825+
"outcome": outcome,
826+
});
827+
let _ = crate::core::controller_scratch::release_attempt(allocation, reason, evidence);
828+
}
829+
830+
fn terminal_reason(outcome: &AgentTaskOutcome, cancelled: bool) -> &'static str {
831+
if cancelled || outcome.status == AgentTaskOutcomeStatus::Cancelled {
832+
"cancelled"
833+
} else if outcome.status == AgentTaskOutcomeStatus::Timeout {
834+
"provider_timeout"
835+
} else if matches!(
836+
outcome.status,
837+
AgentTaskOutcomeStatus::Succeeded | AgentTaskOutcomeStatus::NoOp
838+
) {
839+
"succeeded"
840+
} else {
841+
"provider_failure"
842+
}
843+
}
844+
845+
fn scratch_allocation_failure(task_id: String, error: String) -> AgentTaskOutcome {
846+
AgentTaskOutcome {
847+
schema: AGENT_TASK_OUTCOME_SCHEMA.to_string(),
848+
task_id,
849+
status: AgentTaskOutcomeStatus::Failed,
850+
summary: Some(format!("could not allocate provider scratch root: {error}")),
851+
failure_classification: Some(AgentTaskFailureClassification::ExecutionFailed),
852+
artifacts: Vec::new(),
853+
typed_artifacts: Vec::new(),
854+
evidence_refs: Vec::new(),
855+
diagnostics: vec![AgentTaskDiagnostic {
856+
class: "agent_task.controller_scratch_allocation_failed".to_string(),
857+
message: error,
858+
data: serde_json::Value::Null,
859+
}],
860+
outputs: serde_json::Value::Null,
861+
workflow: None,
862+
follow_up: None,
863+
metadata: serde_json::Value::Null,
864+
}
865+
}
866+
777867
fn reset_attempt_request(
778868
mut request: AgentTaskRequest,
779869
source_workspace_root: Option<String>,

src/core/agent_task_scheduler/harvest.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,10 @@ mod committed_harvest_tests {
579579
artifact_nonce: "test-artifact".to_string(),
580580
task_base_sha: Some(base),
581581
source_provenance: None,
582+
scratch: crate::core::controller_scratch::ControllerScratchAllocation {
583+
path: PathBuf::from("/test/controller-scratch/1"),
584+
lease_id: "test-lease-1".to_string(),
585+
},
582586
};
583587
let mut outcome = committed_harvest_preflight_outcome("task-1".to_string());
584588
outcome.status = AgentTaskOutcomeStatus::Succeeded;
@@ -761,6 +765,10 @@ mod committed_harvest_tests {
761765
artifact_nonce: "test".to_string(),
762766
task_base_sha: Some(baseline),
763767
source_provenance: Some(source_provenance.clone()),
768+
scratch: crate::core::controller_scratch::ControllerScratchAllocation {
769+
path: PathBuf::from("/test/controller-scratch/1"),
770+
lease_id: "test-lease-1".to_string(),
771+
},
764772
};
765773
persist_attempt_patch_artifacts(
766774
&mut outcome,

src/core/agent_task_scheduler/scheduling.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -657,8 +657,6 @@ impl AgentTaskScheduleSupport {
657657
executor.cancel(&task.task_id);
658658
task.timeout_cancel_requested = true;
659659
}
660-
// Keep the task and its Arc-owned checkout until TaskResult arrives.
661-
// This is the join acknowledgement boundary for race-safe harvesting.
662660
index += 1;
663661
}
664662
}

src/core/agent_task_scheduler/tests/scheduling_tests/concurrency.rs

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,8 @@ pub(super) mod concurrency_tests {
350350
events: Arc<Mutex<Vec<String>>>,
351351
finished: Arc<AtomicBool>,
352352
attempt_roots: Arc<Mutex<Vec<std::path::PathBuf>>>,
353+
scratch_roots: Arc<Mutex<Vec<std::path::PathBuf>>>,
354+
scratch_active_while_running: Arc<AtomicBool>,
353355
}
354356

355357
impl AgentTaskExecutorAdapter for LateMutatingExecutor {
@@ -373,7 +375,34 @@ pub(super) mod concurrency_tests {
373375
.lock()
374376
.expect("attempt roots")
375377
.push(workspace.clone());
378+
let scratch_root = std::path::PathBuf::from(
379+
request.executor.config["runtime_env"]["TMPDIR"]
380+
.as_str()
381+
.expect("scheduler scratch root"),
382+
);
383+
self.scratch_roots
384+
.lock()
385+
.expect("scratch roots")
386+
.push(scratch_root.clone());
376387
std::thread::sleep(Duration::from_millis(3_000));
388+
let scratch_index = scratch_root
389+
.ancestors()
390+
.nth(6)
391+
.expect("controller scratch root")
392+
.join("resources.json");
393+
let active = serde_json::from_str::<Value>(
394+
&fs::read_to_string(scratch_index).expect("scratch index"),
395+
)
396+
.expect("scratch index JSON")["resources"]
397+
.as_array()
398+
.expect("scratch resources")
399+
.iter()
400+
.any(|resource| {
401+
resource["path"] == scratch_root.display().to_string()
402+
&& resource["lifecycle_state"] == "active"
403+
});
404+
self.scratch_active_while_running
405+
.store(active, Ordering::SeqCst);
377406
fs::write(workspace.join("late-change.txt"), "late\n")
378407
.expect("late workspace mutation");
379408
assert!(Command::new("git")
@@ -410,10 +439,14 @@ pub(super) mod concurrency_tests {
410439
let events = Arc::new(Mutex::new(Vec::new()));
411440
let finished = Arc::new(AtomicBool::new(false));
412441
let attempt_roots = Arc::new(Mutex::new(Vec::new()));
442+
let scratch_roots = Arc::new(Mutex::new(Vec::new()));
443+
let scratch_active_while_running = Arc::new(AtomicBool::new(false));
413444
let scheduler = AgentTaskScheduler::new(LateMutatingExecutor {
414445
events: Arc::clone(&events),
415446
finished: Arc::clone(&finished),
416447
attempt_roots: Arc::clone(&attempt_roots),
448+
scratch_roots: Arc::clone(&scratch_roots),
449+
scratch_active_while_running: Arc::clone(&scratch_active_while_running),
417450
});
418451
let mut plan = plan_with_tasks(3);
419452
plan.options.max_concurrency = 3;
@@ -425,9 +458,27 @@ pub(super) mod concurrency_tests {
425458
let started = Instant::now();
426459
let aggregate = scheduler.run(plan);
427460
assert!(started.elapsed() >= Duration::from_secs(2));
428-
while !finished.load(Ordering::SeqCst) {
429-
std::thread::sleep(Duration::from_millis(2));
430-
}
461+
let scratch_root = scratch_roots.lock().expect("scratch roots")[0].clone();
462+
let scratch_index = scratch_root
463+
.ancestors()
464+
.nth(6)
465+
.expect("controller scratch root")
466+
.join("resources.json");
467+
let scratch: Value = serde_json::from_str::<Value>(
468+
&fs::read_to_string(scratch_index).expect("scratch index"),
469+
)
470+
.expect("scratch index JSON")["resources"]
471+
.as_array()
472+
.expect("scratch resources")
473+
.iter()
474+
.find(|resource| resource["path"] == scratch_root.display().to_string())
475+
.cloned()
476+
.expect("released scratch resource");
477+
assert!(scratch_active_while_running.load(Ordering::SeqCst));
478+
assert_eq!(scratch["terminal_reason"], "scheduler_timeout_completion");
479+
assert!(scratch["terminal_evidence"]["outcome"]["artifacts"]
480+
.as_array()
481+
.is_some_and(|artifacts| !artifacts.is_empty()));
431482
let events = events.lock().expect("events");
432483

433484
assert!(events.iter().any(|event| event == "task-3-started"));
@@ -437,14 +488,10 @@ pub(super) mod concurrency_tests {
437488
.iter()
438489
.any(|outcome| outcome.task_id == "task-1"
439490
&& outcome.status == AgentTaskOutcomeStatus::CandidateRecoverable
440-
&& outcome.artifacts.iter().any(|artifact| {
441-
artifact.kind == "patch"
442-
&& artifact
443-
.sha256
444-
.as_deref()
445-
.is_some_and(|sha| sha.len() == 64)
446-
&& artifact.metadata["provider_backend"].is_string()
447-
})));
491+
&& outcome
492+
.artifacts
493+
.iter()
494+
.any(|artifact| artifact.kind == "patch")));
448495
assert!(aggregate.outcomes.iter().any(|outcome| {
449496
outcome.task_id == "task-2" && outcome.status == AgentTaskOutcomeStatus::Succeeded
450497
}));

0 commit comments

Comments
 (0)