fix(delegation): résout idea_ask_agent quand la cible revient au prompt sans idea_reply
Introduit le seam TurnResolution : sur prompt-ready, après un délai de grâce, la délégation en attente est résolue de façon typée plutôt que de rester bloquée indéfiniment (jusquà 24h). Couvre domain (mailbox), infrastructure (mailbox/input) et application (orchestrator/error), avec tests étendus. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@ -57,6 +57,18 @@ pub enum AppError {
|
||||
node_id: NodeId,
|
||||
},
|
||||
|
||||
/// The delegation target finished its turn and **returned to its prompt without
|
||||
/// calling `idea_reply`**, so the rendezvous could produce no answer. Surfaced
|
||||
/// promptly (after a short grace window) instead of blocking the caller until the
|
||||
/// long safety timeout. **Retryable**: the caller may re-ask, ideally reminding the
|
||||
/// target to render its result via `idea_reply` (never plain text). Carries the
|
||||
/// target's display name.
|
||||
#[error(
|
||||
"agent {0} returned to its prompt without calling idea_reply (no answer was \
|
||||
rendered); retry the request and ensure the agent replies via idea_reply"
|
||||
)]
|
||||
TargetReturnedNoReply(String),
|
||||
|
||||
/// An unexpected internal error.
|
||||
#[error("internal error: {0}")]
|
||||
Internal(String),
|
||||
@ -76,6 +88,7 @@ impl AppError {
|
||||
Self::Git(_) => "GIT",
|
||||
Self::Remote(_) => "REMOTE",
|
||||
Self::AgentAlreadyRunning { .. } => "AGENT_ALREADY_RUNNING",
|
||||
Self::TargetReturnedNoReply(_) => "TARGET_RETURNED_NO_REPLY",
|
||||
Self::Internal(_) => "INTERNAL",
|
||||
}
|
||||
}
|
||||
|
||||
@ -22,7 +22,7 @@ use tokio::sync::Mutex as AsyncMutex;
|
||||
use domain::conversation::{ConversationParty, ConversationRegistry, SessionRef, WaitForGraph};
|
||||
use domain::conversation_log::{ConversationTurn, TurnId, TurnRole};
|
||||
use domain::input::{InputMediator, InputSource, SubmitConfig};
|
||||
use domain::mailbox::{Ticket, TicketId};
|
||||
use domain::mailbox::{Ticket, TicketId, TurnResolution};
|
||||
use domain::ports::{Clock, EventBus, ProfileStore, PtyHandle};
|
||||
use domain::project::ProjectPath;
|
||||
use domain::{
|
||||
@ -1063,7 +1063,7 @@ impl OrchestratorService {
|
||||
// 3. Attendre la réponse, bornée. Timeout/canal fermé ⇒ le garde retire le ticket
|
||||
// (cible laissée vivante) et la ramène `Idle` au Drop ; renvoie une erreur typée.
|
||||
match tokio::time::timeout(turn_timeout, pending).await {
|
||||
Ok(Ok(result)) => {
|
||||
Ok(Ok(TurnResolution::Replied(result))) => {
|
||||
// Checkpoint Response (P6b, best-effort) : persister la réponse AVANT de
|
||||
// déplacer `result` dans `reply_outcome`. Source = la **cible** (c'est
|
||||
// elle qui a rendu le tour) ; même conversation que le Prompt.
|
||||
@ -1093,6 +1093,21 @@ impl OrchestratorService {
|
||||
);
|
||||
Ok(self.reply_outcome(agent_id, &target, result))
|
||||
}
|
||||
// La cible est revenue à son prompt SANS `idea_reply` : la fenêtre de grâce a
|
||||
// expiré et le médiateur a complété le tour « sans réponse ». Le ticket est
|
||||
// déjà retiré (`complete_without_reply`) et la cible déjà `Idle` (prompt-ready
|
||||
// `mark_idle`) ⇒ on **désarme** le garde (pas de `cancel_head`/`mark_idle`
|
||||
// redondant ni de beacon « busy-guard freed » trompeur) et on renvoie une
|
||||
// erreur typée claire/retryable, en ~G au lieu du timeout long.
|
||||
Ok(Ok(TurnResolution::ReturnedToPromptNoReply)) => {
|
||||
busy_guard.disarm();
|
||||
crate::diag!(
|
||||
"[rendezvous] ask returned-to-prompt-no-reply: target={target} \
|
||||
(agent {agent_id}) ticket={ticket_id} after_ms={}",
|
||||
started.elapsed().as_millis(),
|
||||
);
|
||||
Err(AppError::TargetReturnedNoReply(target))
|
||||
}
|
||||
// Erreur / timeout : on laisse le garde faire `cancel_head` + `mark_idle` au
|
||||
// Drop (retrait des `cancel_head` redondants — `cancel_head` reste idempotent).
|
||||
Ok(Err(_cancelled)) => {
|
||||
@ -1232,7 +1247,7 @@ impl OrchestratorService {
|
||||
},
|
||||
// Issue alternative : un `idea_reply` explicite a résolu le ticket d'abord.
|
||||
replied = pending => match replied {
|
||||
Ok(content) => {
|
||||
Ok(TurnResolution::Replied(content)) => {
|
||||
// La session draine encore en arrière-plan ; on s'assure que la FIFO
|
||||
// avance même si le `Final` n'est pas encore observé.
|
||||
input.mark_idle(agent_id);
|
||||
@ -1244,6 +1259,19 @@ impl OrchestratorService {
|
||||
);
|
||||
content
|
||||
}
|
||||
// Retour au prompt sans `idea_reply` : très rare sur le chemin structuré
|
||||
// (la cible n'a pas de PTY/watcher), mais on traite la résolution comme
|
||||
// sur le chemin PTY — erreur typée claire/retryable, garde désarmé.
|
||||
Ok(TurnResolution::ReturnedToPromptNoReply) => {
|
||||
input.mark_idle(agent_id);
|
||||
busy_guard.disarm();
|
||||
crate::diag!(
|
||||
"[rendezvous] ask returned-to-prompt-no-reply (structured): \
|
||||
target={target} (agent {agent_id}) ticket={ticket_id} after_ms={}",
|
||||
started.elapsed().as_millis(),
|
||||
);
|
||||
return Err(AppError::TargetReturnedNoReply(target.to_owned()));
|
||||
}
|
||||
// Canal fermé : le garde fait `cancel_head` + `mark_idle` au Drop.
|
||||
Err(_cancelled) => {
|
||||
crate::diag!(
|
||||
|
||||
@ -861,7 +861,7 @@ use domain::conversation::{
|
||||
SessionRef,
|
||||
};
|
||||
use domain::input::{AgentBusyState, InputMediator};
|
||||
use domain::mailbox::{AgentMailbox, MailboxError, PendingReply, Ticket, TicketId};
|
||||
use domain::mailbox::{AgentMailbox, MailboxError, PendingReply, Ticket, TicketId, TurnResolution};
|
||||
use tokio::time::{timeout, Duration};
|
||||
|
||||
/// Safety guard: no awaited rendezvous in these tests should ever exceed this.
|
||||
@ -872,7 +872,8 @@ const TEST_GUARD: Duration = Duration::from_secs(10);
|
||||
/// would be a dependency cycle). Per-agent FIFO of `(Ticket, oneshot::Sender)`.
|
||||
#[derive(Default)]
|
||||
struct TestMailbox {
|
||||
queues: Mutex<HashMap<AgentId, VecDeque<(Ticket, tokio::sync::oneshot::Sender<String>)>>>,
|
||||
queues:
|
||||
Mutex<HashMap<AgentId, VecDeque<(Ticket, tokio::sync::oneshot::Sender<TurnResolution>)>>>,
|
||||
}
|
||||
impl TestMailbox {
|
||||
fn new() -> Self {
|
||||
@ -899,7 +900,7 @@ impl TestMailbox {
|
||||
}
|
||||
impl AgentMailbox for TestMailbox {
|
||||
fn enqueue(&self, agent: AgentId, ticket: Ticket) -> PendingReply {
|
||||
let (tx, rx) = tokio::sync::oneshot::channel::<String>();
|
||||
let (tx, rx) = tokio::sync::oneshot::channel::<TurnResolution>();
|
||||
self.queues
|
||||
.lock()
|
||||
.unwrap()
|
||||
@ -919,7 +920,7 @@ impl AgentMailbox for TestMailbox {
|
||||
.ok_or(MailboxError::NoPendingRequest(agent))?;
|
||||
queue.pop_front().expect("non-empty")
|
||||
};
|
||||
let _ = slot.1.send(result);
|
||||
let _ = slot.1.send(TurnResolution::Replied(result));
|
||||
Ok(())
|
||||
}
|
||||
fn resolve_ticket(
|
||||
@ -940,7 +941,7 @@ impl AgentMailbox for TestMailbox {
|
||||
.ok_or(MailboxError::NoPendingRequest(agent))?;
|
||||
queue.remove(pos).expect("found position")
|
||||
};
|
||||
let _ = slot.1.send(result);
|
||||
let _ = slot.1.send(TurnResolution::Replied(result));
|
||||
Ok(())
|
||||
}
|
||||
fn cancel_head(&self, agent: AgentId, ticket_id: TicketId) {
|
||||
@ -951,6 +952,20 @@ impl AgentMailbox for TestMailbox {
|
||||
}
|
||||
}
|
||||
}
|
||||
fn complete_without_reply(&self, agent: AgentId, ticket_id: TicketId) {
|
||||
// Mirror the production adapter: head-only, idempotent, fire-and-forget-safe.
|
||||
let mut q = self.queues.lock().unwrap();
|
||||
if let Some(queue) = q.get_mut(&agent) {
|
||||
if queue.front().map(|(t, _)| t.id) != Some(ticket_id) {
|
||||
return;
|
||||
}
|
||||
if queue.front().is_some_and(|(_, tx)| tx.is_closed()) {
|
||||
return; // receiver gone (human submit / timed-out caller): preserve head.
|
||||
}
|
||||
let (_, tx) = queue.pop_front().expect("head just matched");
|
||||
let _ = tx.send(TurnResolution::ReturnedToPromptNoReply);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A delivering [`InputMediator`] for the application tests: it composes the
|
||||
@ -1287,6 +1302,63 @@ async fn ask_live_pty_target_writes_task_and_returns_reply() {
|
||||
assert_eq!(fx.mailbox.pending(&aid(1)), 0, "ticket drained on reply");
|
||||
}
|
||||
|
||||
/// The target returned to its prompt **without** `idea_reply`: the mediator's grace
|
||||
/// window expired and completed the turn "no reply". The awaiting ask must error out
|
||||
/// promptly with the typed, retryable [`AppError::TargetReturnedNoReply`] — never hang
|
||||
/// until the long safety timeout. (The grace **timer** lives in the infrastructure
|
||||
/// mediator; here we drive the resulting mailbox completion directly.)
|
||||
#[tokio::test]
|
||||
async fn ask_target_returns_to_prompt_without_reply_is_a_typed_error() {
|
||||
let agent = scratch_agent(aid(1), "architect", "agents/architect.md");
|
||||
let fx = ask_fixture(FakeContexts::with_agent(&agent, "# persona"));
|
||||
seed_live_pty(&fx.sessions, aid(1), sid(800));
|
||||
|
||||
let svc = Arc::clone(&fx.service);
|
||||
let ask = tokio::spawn(async move { svc.dispatch(&project(), cmd(ASK_JSON)).await });
|
||||
await_until(|| fx.mailbox.pending(&aid(1)) == 1).await;
|
||||
|
||||
// Simulate the grace-window completion (target back at prompt, no idea_reply).
|
||||
let ticket = fx.mailbox.ticket_ids(&aid(1))[0];
|
||||
fx.mailbox.complete_without_reply(aid(1), ticket);
|
||||
|
||||
let err = timeout(TEST_GUARD, ask)
|
||||
.await
|
||||
.expect("ask completes promptly, not after the long timeout")
|
||||
.expect("join ok")
|
||||
.expect_err("a no-reply turn is a typed error");
|
||||
assert_eq!(err.code(), "TARGET_RETURNED_NO_REPLY", "got {err:?}");
|
||||
assert_eq!(fx.mailbox.pending(&aid(1)), 0, "head retired by completion");
|
||||
}
|
||||
|
||||
/// An `idea_reply` that lands **first** still wins over a (later) no-reply completion:
|
||||
/// the completion then finds the head gone and is a no-op (idempotent race).
|
||||
#[tokio::test]
|
||||
async fn ask_reply_wins_then_late_completion_is_noop() {
|
||||
let agent = scratch_agent(aid(1), "architect", "agents/architect.md");
|
||||
let fx = ask_fixture(FakeContexts::with_agent(&agent, "# persona"));
|
||||
seed_live_pty(&fx.sessions, aid(1), sid(800));
|
||||
|
||||
let svc = Arc::clone(&fx.service);
|
||||
let ask = tokio::spawn(async move { svc.dispatch(&project(), cmd(ASK_JSON)).await });
|
||||
await_until(|| fx.mailbox.pending(&aid(1)) == 1).await;
|
||||
|
||||
let ticket = fx.mailbox.ticket_ids(&aid(1))[0];
|
||||
// idea_reply resolves the head first…
|
||||
fx.service
|
||||
.dispatch(&project(), reply_cmd(aid(1), "the real answer"))
|
||||
.await
|
||||
.expect("reply ok");
|
||||
// …then a late grace completion for the same ticket is a no-op.
|
||||
fx.mailbox.complete_without_reply(aid(1), ticket);
|
||||
|
||||
let out = timeout(TEST_GUARD, ask)
|
||||
.await
|
||||
.expect("ask completes")
|
||||
.expect("join ok")
|
||||
.expect("ask ok");
|
||||
assert_eq!(out.reply.as_deref(), Some("the real answer"));
|
||||
}
|
||||
|
||||
// --- Lot E1: auto-memory harvest on a successful ask reply -----------------
|
||||
|
||||
/// A reply carrying an `idea-memory` block is harvested **after** the reply
|
||||
|
||||
@ -13,7 +13,7 @@ use application::{
|
||||
TerminalSessions, TicketWorkSource, TicketWorkStatus,
|
||||
};
|
||||
use domain::mailbox::{
|
||||
AgentQueueSnapshot, MailboxError, PendingReply, QueuedTicketSnapshot, Ticket,
|
||||
AgentQueueSnapshot, MailboxError, PendingReply, QueuedTicketSnapshot, Ticket, TurnResolution,
|
||||
};
|
||||
use domain::ports::{
|
||||
AgentContextStore, AgentSession, AgentSessionError, PtyHandle, ReplyStream, StoreError,
|
||||
@ -123,7 +123,7 @@ impl FakeInput {
|
||||
|
||||
impl InputMediator for FakeInput {
|
||||
fn enqueue(&self, _agent: AgentId, _ticket: Ticket) -> PendingReply {
|
||||
let fut: Pin<Box<dyn Future<Output = Result<String, MailboxError>> + Send>> =
|
||||
let fut: Pin<Box<dyn Future<Output = Result<TurnResolution, MailboxError>> + Send>> =
|
||||
Box::pin(async { Err(MailboxError::Cancelled) });
|
||||
PendingReply::new(fut)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user