story-kit: merge 231_bug_agent_silently_disappears_when_worktree_creation_fails
This commit is contained in:
@@ -518,15 +518,21 @@ impl AgentPool {
|
|||||||
{
|
{
|
||||||
Ok(wt) => wt,
|
Ok(wt) => wt,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
// Worktree creation failed — mark agent as Failed so the UI shows the error.
|
let error_msg = format!("Failed to create worktree: {e}");
|
||||||
let _ = tx_clone.send(AgentEvent::Error {
|
slog_error!("[agents] {error_msg}");
|
||||||
|
let event = AgentEvent::Error {
|
||||||
story_id: sid.clone(),
|
story_id: sid.clone(),
|
||||||
agent_name: aname.clone(),
|
agent_name: aname.clone(),
|
||||||
message: format!("Failed to create worktree: {e}"),
|
message: error_msg,
|
||||||
});
|
};
|
||||||
if let Ok(mut agents) = agents_ref.lock() {
|
if let Ok(mut log) = log_clone.lock() {
|
||||||
agents.remove(&key_clone);
|
log.push(event.clone());
|
||||||
}
|
}
|
||||||
|
let _ = tx_clone.send(event);
|
||||||
|
if let Ok(mut agents) = agents_ref.lock()
|
||||||
|
&& let Some(agent) = agents.get_mut(&key_clone) {
|
||||||
|
agent.status = AgentStatus::Failed;
|
||||||
|
}
|
||||||
Self::notify_agent_state_changed(&watcher_tx_clone);
|
Self::notify_agent_state_changed(&watcher_tx_clone);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -550,14 +556,21 @@ impl AgentPool {
|
|||||||
) {
|
) {
|
||||||
Ok(result) => result,
|
Ok(result) => result,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let _ = tx_clone.send(AgentEvent::Error {
|
let error_msg = format!("Failed to render agent args: {e}");
|
||||||
|
slog_error!("[agents] {error_msg}");
|
||||||
|
let event = AgentEvent::Error {
|
||||||
story_id: sid.clone(),
|
story_id: sid.clone(),
|
||||||
agent_name: aname.clone(),
|
agent_name: aname.clone(),
|
||||||
message: format!("Failed to render agent args: {e}"),
|
message: error_msg,
|
||||||
});
|
};
|
||||||
if let Ok(mut agents) = agents_ref.lock() {
|
if let Ok(mut log) = log_clone.lock() {
|
||||||
agents.remove(&key_clone);
|
log.push(event.clone());
|
||||||
}
|
}
|
||||||
|
let _ = tx_clone.send(event);
|
||||||
|
if let Ok(mut agents) = agents_ref.lock()
|
||||||
|
&& let Some(agent) = agents.get_mut(&key_clone) {
|
||||||
|
agent.status = AgentStatus::Failed;
|
||||||
|
}
|
||||||
Self::notify_agent_state_changed(&watcher_tx_clone);
|
Self::notify_agent_state_changed(&watcher_tx_clone);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -614,14 +627,20 @@ impl AgentPool {
|
|||||||
Self::notify_agent_state_changed(&watcher_tx_clone);
|
Self::notify_agent_state_changed(&watcher_tx_clone);
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let _ = tx_clone.send(AgentEvent::Error {
|
slog_error!("[agents] Agent process error for {aname} on {sid}: {e}");
|
||||||
|
let event = AgentEvent::Error {
|
||||||
story_id: sid.clone(),
|
story_id: sid.clone(),
|
||||||
agent_name: aname.clone(),
|
agent_name: aname.clone(),
|
||||||
message: e,
|
message: e,
|
||||||
});
|
};
|
||||||
if let Ok(mut agents) = agents_ref.lock() {
|
if let Ok(mut log) = log_clone.lock() {
|
||||||
agents.remove(&key_clone);
|
log.push(event.clone());
|
||||||
}
|
}
|
||||||
|
let _ = tx_clone.send(event);
|
||||||
|
if let Ok(mut agents) = agents_ref.lock()
|
||||||
|
&& let Some(agent) = agents.get_mut(&key_clone) {
|
||||||
|
agent.status = AgentStatus::Failed;
|
||||||
|
}
|
||||||
Self::notify_agent_state_changed(&watcher_tx_clone);
|
Self::notify_agent_state_changed(&watcher_tx_clone);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -5292,15 +5311,28 @@ stage = "qa"
|
|||||||
"agent must transition to Failed after worktree creation error"
|
"agent must transition to Failed after worktree creation error"
|
||||||
);
|
);
|
||||||
|
|
||||||
// The pool must NOT retain a Pending or Running entry for this agent.
|
// The pool must retain a Failed entry (not disappear silently).
|
||||||
let agents = pool.agents.lock().unwrap();
|
let agents = pool.agents.lock().unwrap();
|
||||||
let leaked = agents.values().any(|a| {
|
let failed_entry = agents
|
||||||
a.agent_name == "qa"
|
.values()
|
||||||
&& matches!(a.status, AgentStatus::Pending | AgentStatus::Running)
|
.find(|a| a.agent_name == "qa" && a.status == AgentStatus::Failed);
|
||||||
});
|
|
||||||
assert!(
|
assert!(
|
||||||
!leaked,
|
failed_entry.is_some(),
|
||||||
"agent pool must not retain a Pending/Running entry after worktree creation fails"
|
"agent pool must retain a Failed entry so the UI can show the error state"
|
||||||
|
);
|
||||||
|
drop(agents);
|
||||||
|
|
||||||
|
// The AgentEvent::Error must be persisted in the event_log so late
|
||||||
|
// subscribers / polling clients can see the failure reason.
|
||||||
|
let events = pool
|
||||||
|
.drain_events("50_story_test", "qa")
|
||||||
|
.expect("drain_events should succeed");
|
||||||
|
let has_error_event = events
|
||||||
|
.iter()
|
||||||
|
.any(|e| matches!(e, AgentEvent::Error { .. }));
|
||||||
|
assert!(
|
||||||
|
has_error_event,
|
||||||
|
"event_log must contain AgentEvent::Error after worktree creation fails"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user