fix: recover run agents after restart
This commit is contained in:
@@ -139,6 +139,8 @@ pub enum PendingToolCallState {
|
||||
decision: PermissionDecision,
|
||||
},
|
||||
Executing,
|
||||
/// External work survived a restart and must be reattached before it can complete.
|
||||
RecoveryPending,
|
||||
Resolved {
|
||||
result: ToolResult,
|
||||
},
|
||||
@@ -151,7 +153,8 @@ impl PendingToolCallState {
|
||||
Self::Proposed
|
||||
| Self::PermissionPending { .. }
|
||||
| Self::Approved { .. }
|
||||
| Self::Executing => None,
|
||||
| Self::Executing
|
||||
| Self::RecoveryPending => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -161,6 +164,7 @@ impl PendingToolCallState {
|
||||
Self::PermissionPending { .. } => "permission_pending",
|
||||
Self::Approved { .. } => "approved",
|
||||
Self::Executing => "executing",
|
||||
Self::RecoveryPending => "recovery_pending",
|
||||
Self::Resolved { .. } => "resolved",
|
||||
}
|
||||
}
|
||||
@@ -300,6 +304,7 @@ pub enum ModelFailureDisposition {
|
||||
pub struct ProviderRunRestoreNormalization {
|
||||
pub permission_call_ids_reset: Vec<String>,
|
||||
pub interrupted_call_ids: Vec<String>,
|
||||
pub recovery_call_ids: Vec<String>,
|
||||
pub committed_tool_batch: bool,
|
||||
}
|
||||
|
||||
@@ -499,6 +504,13 @@ impl ProviderRun {
|
||||
|
||||
pub fn normalize_after_restore(
|
||||
&mut self,
|
||||
) -> Result<ProviderRunRestoreNormalization, ProviderRunProtocolError> {
|
||||
self.normalize_after_restore_with_recoverable_calls(&HashSet::new())
|
||||
}
|
||||
|
||||
pub fn normalize_after_restore_with_recoverable_calls(
|
||||
&mut self,
|
||||
recoverable_call_ids: &HashSet<String>,
|
||||
) -> Result<ProviderRunRestoreNormalization, ProviderRunProtocolError> {
|
||||
let ProviderRunState::AwaitingTools { batch } = &mut self.state else {
|
||||
return Ok(ProviderRunRestoreNormalization::default());
|
||||
@@ -513,6 +525,19 @@ impl ProviderRun {
|
||||
.push(pending.call.id.clone());
|
||||
pending.state = PendingToolCallState::Proposed;
|
||||
}
|
||||
PendingToolCallState::Executing
|
||||
if recoverable_call_ids.contains(&pending.call.id) =>
|
||||
{
|
||||
normalization
|
||||
.recovery_call_ids
|
||||
.push(pending.call.id.clone());
|
||||
pending.state = PendingToolCallState::RecoveryPending;
|
||||
}
|
||||
PendingToolCallState::RecoveryPending => {
|
||||
normalization
|
||||
.recovery_call_ids
|
||||
.push(pending.call.id.clone());
|
||||
}
|
||||
PendingToolCallState::Approved { .. } | PendingToolCallState::Executing => {
|
||||
normalization
|
||||
.interrupted_call_ids
|
||||
@@ -719,7 +744,8 @@ impl ProviderRun {
|
||||
}
|
||||
PendingToolCallState::PermissionPending { .. }
|
||||
| PendingToolCallState::Approved { .. }
|
||||
| PendingToolCallState::Executing => {
|
||||
| PendingToolCallState::Executing
|
||||
| PendingToolCallState::RecoveryPending => {
|
||||
Err(invalid_tool_transition(call, "permission request"))
|
||||
}
|
||||
}
|
||||
@@ -742,7 +768,8 @@ impl ProviderRun {
|
||||
}
|
||||
PendingToolCallState::Proposed
|
||||
| PendingToolCallState::Approved { .. }
|
||||
| PendingToolCallState::Executing => {
|
||||
| PendingToolCallState::Executing
|
||||
| PendingToolCallState::RecoveryPending => {
|
||||
return Err(invalid_tool_transition(call, "permission resolution"));
|
||||
}
|
||||
};
|
||||
@@ -782,7 +809,9 @@ impl ProviderRun {
|
||||
) -> Result<(), ProviderRunProtocolError> {
|
||||
let call = self.pending_tool_call_mut(work_id, call_id)?;
|
||||
match &call.state {
|
||||
PendingToolCallState::Proposed | PendingToolCallState::Approved { .. } => {
|
||||
PendingToolCallState::Proposed
|
||||
| PendingToolCallState::Approved { .. }
|
||||
| PendingToolCallState::RecoveryPending => {
|
||||
call.state = PendingToolCallState::Executing;
|
||||
Ok(())
|
||||
}
|
||||
@@ -806,7 +835,8 @@ impl ProviderRun {
|
||||
match &call.state {
|
||||
PendingToolCallState::Proposed
|
||||
| PendingToolCallState::Approved { .. }
|
||||
| PendingToolCallState::Executing => {
|
||||
| PendingToolCallState::Executing
|
||||
| PendingToolCallState::RecoveryPending => {
|
||||
call.state = PendingToolCallState::Resolved { result };
|
||||
Ok(())
|
||||
}
|
||||
@@ -837,7 +867,8 @@ impl ProviderRun {
|
||||
PendingToolCallState::Proposed
|
||||
| PendingToolCallState::PermissionPending { .. }
|
||||
| PendingToolCallState::Approved { .. }
|
||||
| PendingToolCallState::Executing => {
|
||||
| PendingToolCallState::Executing
|
||||
| PendingToolCallState::RecoveryPending => {
|
||||
call.state = PendingToolCallState::Resolved {
|
||||
result: ToolResult {
|
||||
call_id: call_id.to_string(),
|
||||
|
||||
@@ -758,6 +758,73 @@ fn restore_normalization_reproposes_permissions_and_interrupts_unsafe_tools() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_normalization_preserves_selected_executing_tools_for_recovery() {
|
||||
let mut run = run();
|
||||
let batch = accept_tool_turn(
|
||||
&mut run,
|
||||
tool_turn(
|
||||
vec![
|
||||
tool_call("run-agents", "run_agents"),
|
||||
tool_call("read", "read_files"),
|
||||
],
|
||||
&["run_agents", "read_files"],
|
||||
),
|
||||
);
|
||||
run.start_tool(&batch.work_id, "run-agents").unwrap();
|
||||
run.start_tool(&batch.work_id, "read").unwrap();
|
||||
|
||||
let normalization = run
|
||||
.normalize_after_restore_with_recoverable_calls(&HashSet::from(["run-agents".to_string()]))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(normalization.recovery_call_ids, vec!["run-agents"]);
|
||||
assert_eq!(normalization.interrupted_call_ids, vec!["read"]);
|
||||
let ProviderRunState::AwaitingTools { batch } = run.state() else {
|
||||
panic!("recovered tool batch should remain pending");
|
||||
};
|
||||
assert!(matches!(
|
||||
batch.calls[0].state,
|
||||
PendingToolCallState::RecoveryPending
|
||||
));
|
||||
assert_eq!(
|
||||
batch.calls[1].state.result().unwrap().status,
|
||||
ToolResultStatus::Error
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recovery_pending_tool_survives_another_restore_and_completes_once() {
|
||||
let mut run = run();
|
||||
let batch = accept_tool_turn(
|
||||
&mut run,
|
||||
tool_turn(vec![tool_call("run-agents", "run_agents")], &["run_agents"]),
|
||||
);
|
||||
run.start_tool(&batch.work_id, "run-agents").unwrap();
|
||||
run.normalize_after_restore_with_recoverable_calls(&HashSet::from(["run-agents".to_string()]))
|
||||
.unwrap();
|
||||
let serialized = serde_json::to_string(&run).unwrap();
|
||||
let mut restored: ProviderRun = serde_json::from_str(&serialized).unwrap();
|
||||
|
||||
let normalization = restored.normalize_after_restore().unwrap();
|
||||
assert_eq!(normalization.recovery_call_ids, vec!["run-agents"]);
|
||||
restored
|
||||
.complete_tool(
|
||||
&batch.work_id,
|
||||
successful_result("run-agents", "children completed"),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(matches!(
|
||||
restored.complete_tool(
|
||||
&batch.work_id,
|
||||
successful_result("run-agents", "duplicate completion"),
|
||||
),
|
||||
Err(ProviderRunProtocolError::DuplicateToolUpdate { .. })
|
||||
));
|
||||
restored.commit_tool_batch(&batch.work_id).unwrap();
|
||||
assert_eq!(restored.state().phase(), ProviderRunPhase::ReadyToCallModel);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_normalization_commits_a_fully_resolved_batch() {
|
||||
let mut run = run();
|
||||
|
||||
Reference in New Issue
Block a user