Skip to content

Commit 51fe4cf

Browse files
committed
fix(infinity-slack-bot): finalize the active stream when user input interrupts a turn
When a user message lands mid-turn, the daemon cancels the in-flight step and settles unanswered tool calls with *history-only* synthetic results (`interrupt_pending_tool_call` → `append_unvalidated`, no `ToolResult` broadcast), then emits exactly one `ResponseDone` for the cancelled step. The Slack dataflow swallowed that `ResponseDone` via the had-tool-call keep-open rule, so: * the old stream message was never finalized — its task cards spun forever (the live incident: `execute_command` / `sleep_until_event_or_input` cards stuck "in progress"), and * the next turn's chunks appended to the still-open message *above* the user's message (discontinuity). The chat-message branch now emits a `StreamStop { end_of_response: true }` for the targeted stream (root, or the subagent's stream for replies inside a dedicated subagent thread) before the "is thinking" status, finalizing the interrupted message — completing its open cards — so the new turn starts fresh below the user's message. Ordering is causal: the new turn's output can only exist after the daemon receives `SendInput`, which is emitted in the same tick as the stop. `clear_interrupted_turn_state()` also discards the stream's never-to-be- resolved tool-task queue entries and stale orphan-result sentinels so they cannot mislabel the next turn's completions (FIFO pairing). `had_tool_call` is deliberately left for the cancelled step's `ResponseDone` to consume — swallowed harmlessly — instead of letting it emit a second stop that would wipe the fresh status. New sim test drives the full incident shape (mid-turn message, swallowed cancel done, new tool round pairing with the right card) plus a unit test for the per-stream-key scoping.
1 parent fe4f611 commit 51fe4cf

2 files changed

Lines changed: 317 additions & 15 deletions

File tree

‎crates/infinity-slack-dataflow/src/flow.rs‎

Lines changed: 157 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -546,23 +546,55 @@ pub fn slack_dataflow<'a, P: 'a>(
546546
*/),
547547
);
548548

549-
// Set an "is thinking" status as soon as a chat message arrives. Replies
550-
// inside a subagent thread set the status on the root conversation thread
551-
// (that is where the session-level status indicator lives).
552-
let status_actions = chat_messages.map(q!(|event: SlackEvent| {
549+
// Finalize the interrupted stream (if any) and set an "is thinking"
550+
// status as soon as a chat message arrives. The daemon cancels an
551+
// in-flight turn when user input lands, but it broadcasts no ToolResult
552+
// events for the interrupted calls, and the cancelled turn's ResponseDone
553+
// is swallowed by the had-tool-call keep-open rule — so without an
554+
// explicit stop the old stream message would stay active (its task cards
555+
// spinning forever) and the next turn would continue streaming *above*
556+
// the user's message. Emitting the stop here (same branch, before the
557+
// status, so in-branch ordering holds) finalizes the old message; the new
558+
// turn's output can only be produced after the daemon receives SendInput,
559+
// so it starts a fresh stream below. Replies inside a subagent's
560+
// dedicated thread finalize that subagent's stream; the status indicator
561+
// always lives on the root conversation thread.
562+
let status_actions = chat_messages.flat_map_ordered(q!(|event: SlackEvent| {
553563
let rt = crate::runtime::get();
554-
let thread_ts = {
555-
let routes = rt.subagent_routes.lock().expect("bug: lock poisoned");
556-
routes
557-
.get(&event.thread_ts)
558-
.map(|r| r.root_thread_ts.clone())
559-
.unwrap_or(event.thread_ts)
564+
let route = rt
565+
.subagent_routes
566+
.lock()
567+
.expect("bug: lock poisoned")
568+
.get(&event.thread_ts)
569+
.cloned();
570+
let (root_ts, subagent) = match route {
571+
Some(r) => (r.root_thread_ts, Some(r.subagent_id)),
572+
None => (event.thread_ts, None),
560573
};
561-
SlackAction::SetStatus {
574+
let mut actions = Vec::new();
575+
let has_session = {
576+
let sessions = rt.sessions.lock().expect("bug: lock poisoned");
577+
sessions.get(&root_ts).is_some()
578+
};
579+
// Only finalize for known sessions (a brand-new thread has nothing to
580+
// stop). Stale tool bookkeeping must be cleared even when no output
581+
// was delivered yet, so the clear runs unconditionally for the key.
582+
let was_in_flight =
583+
crate::flow::clear_interrupted_turn_state(rt, &root_ts, subagent.as_deref());
584+
if has_session && was_in_flight {
585+
actions.push(SlackAction::StreamStop {
586+
channel: event.channel.clone(),
587+
thread_ts: root_ts.clone(),
588+
subagent,
589+
end_of_response: true,
590+
});
591+
}
592+
actions.push(SlackAction::SetStatus {
562593
channel: event.channel,
563-
thread_ts,
594+
thread_ts: root_ts,
564595
status: "is thinking".to_owned(),
565-
}
596+
});
597+
actions
566598
}));
567599

568600
// --- Daemon events → Slack actions (streaming responses) ---
@@ -1052,6 +1084,56 @@ pub fn pair_tool_result(
10521084
.and_then(|q| q.pop_front())
10531085
}
10541086

1087+
/// Clear the per-stream bookkeeping of a turn the user just interrupted by
1088+
/// sending a message, returning whether anything suggested a response was in
1089+
/// flight.
1090+
///
1091+
/// When user input arrives mid-turn, the daemon cancels the in-flight step
1092+
/// and settles unanswered tool calls with *synthetic* results that are
1093+
/// written to history only — no `ToolResult` events are broadcast — so the
1094+
/// stream's FIFO queue entries for them would never be popped and would
1095+
/// mislabel the next turn's completions. The orphan-result sentinels from a
1096+
/// resume are stale for the same reason.
1097+
///
1098+
/// `had_tool_call` is deliberately **left alone**: the cancelled step still
1099+
/// broadcasts exactly one `ResponseDone`, which consumes the flag (and is
1100+
/// swallowed by the keep-open rule — harmless, since the stream was already
1101+
/// finalized by the caller's `StreamStop`). Clearing it here would instead
1102+
/// let that `ResponseDone` emit a second stop that wipes the fresh
1103+
/// "is thinking" status.
1104+
pub fn clear_interrupted_turn_state(
1105+
rt: &crate::runtime::Runtime,
1106+
thread_ts: &str,
1107+
subagent: Option<&str>,
1108+
) -> bool {
1109+
let skey = crate::slack::stream_key(thread_ts, subagent);
1110+
let had_tasks = rt
1111+
.tool_tasks
1112+
.lock()
1113+
.expect("bug: lock poisoned")
1114+
.remove(&skey)
1115+
.map(|q| !q.is_empty())
1116+
.unwrap_or(false);
1117+
let orphans = rt
1118+
.resume_orphan_results
1119+
.lock()
1120+
.expect("bug: lock poisoned")
1121+
.remove(&skey)
1122+
.unwrap_or(0);
1123+
let delivering = rt
1124+
.stream_progress
1125+
.lock()
1126+
.expect("bug: lock poisoned")
1127+
.contains_key(&(thread_ts.to_owned(), subagent.map(str::to_owned)));
1128+
if had_tasks || orphans > 0 {
1129+
tracing::debug!(
1130+
"cleared interrupted-turn state for {skey} (open tasks: {had_tasks}, \
1131+
orphan sentinels: {orphans})"
1132+
);
1133+
}
1134+
had_tasks || orphans > 0 || delivering
1135+
}
1136+
10551137
pub fn subagent_of(
10561138
msg: &infinity_protocol::DaemonMessage,
10571139
root: Option<&infinity_protocol::ThreadRef>,
@@ -1305,6 +1387,68 @@ mod tests {
13051387
assert_eq!(subagent_of(&welcome, Some(&root)), None);
13061388
}
13071389

1390+
#[test]
1391+
fn clear_interrupted_turn_state_scopes_to_the_stream_key() {
1392+
crate::runtime::ensure_test_init();
1393+
let rt = crate::runtime::get();
1394+
// Unique ids: the runtime is shared process-wide across tests.
1395+
let thread_ts = "991.1";
1396+
let root_key = crate::slack::stream_key(thread_ts, None);
1397+
let sub_key = crate::slack::stream_key(thread_ts, Some("subA"));
1398+
1399+
// Idle thread: nothing to clear, nothing in flight.
1400+
assert!(!clear_interrupted_turn_state(rt, thread_ts, None));
1401+
1402+
// Root turn in flight (open tool task + delivery cursor + a stale
1403+
// orphan sentinel); the subagent's stream has its own open task.
1404+
rt.tool_tasks.lock().expect("lock").insert(
1405+
root_key.clone(),
1406+
[("t_root".to_owned(), "bash".to_owned(), "{}".to_owned())]
1407+
.into_iter()
1408+
.collect(),
1409+
);
1410+
rt.tool_tasks.lock().expect("lock").insert(
1411+
sub_key.clone(),
1412+
[("t_sub".to_owned(), "grep".to_owned(), "{}".to_owned())]
1413+
.into_iter()
1414+
.collect(),
1415+
);
1416+
rt.resume_orphan_results
1417+
.lock()
1418+
.expect("lock")
1419+
.insert(root_key.clone(), 2);
1420+
rt.stream_progress
1421+
.lock()
1422+
.expect("lock")
1423+
.insert((thread_ts.to_owned(), None), Default::default());
1424+
1425+
// Clearing the ROOT stream reports in-flight and leaves the subagent
1426+
// stream untouched.
1427+
assert!(clear_interrupted_turn_state(rt, thread_ts, None));
1428+
assert!(!rt.tool_tasks.lock().expect("lock").contains_key(&root_key));
1429+
assert!(!rt
1430+
.resume_orphan_results
1431+
.lock()
1432+
.expect("lock")
1433+
.contains_key(&root_key));
1434+
assert!(rt.tool_tasks.lock().expect("lock").contains_key(&sub_key));
1435+
1436+
// The delivery cursor alone (text-only turn) still reports in-flight;
1437+
// the cursor itself is owned by the I/O layer's end-of-response path
1438+
// and is not removed here.
1439+
assert!(clear_interrupted_turn_state(rt, thread_ts, None));
1440+
1441+
// The subagent stream clears independently.
1442+
assert!(clear_interrupted_turn_state(rt, thread_ts, Some("subA")));
1443+
assert!(!rt.tool_tasks.lock().expect("lock").contains_key(&sub_key));
1444+
1445+
// Clean up shared runtime state.
1446+
rt.stream_progress
1447+
.lock()
1448+
.expect("lock")
1449+
.remove(&(thread_ts.to_owned(), None));
1450+
}
1451+
13081452
#[test]
13091453
fn pair_tool_result_consumes_sentinels_before_the_queue() {
13101454
crate::runtime::ensure_test_init();

‎crates/infinity-slack-dataflow/tests/dataflow_sim.rs‎

Lines changed: 160 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,10 @@ fn choice_required(id: &str, prompt: &str) -> infinity_protocol::DaemonMessage {
182182
}
183183
}
184184

185-
/// The non-`SetStatus` actions, which all originate from totally-ordered
186-
/// branches and therefore have a deterministic sequence.
185+
/// The non-`SetStatus` actions. In most tests these all originate from the
186+
/// totally-ordered daemon branch; tests that send a chat message while a turn
187+
/// is in flight additionally filter out `StreamStop` (the message-triggered
188+
/// finalize rides the slack-events branch and may interleave).
187189
fn non_status(actions: &[SlackAction]) -> Vec<&SlackAction> {
188190
actions
189191
.iter()
@@ -664,6 +666,162 @@ fn session_stop_interrupts_daemon_generation() {
664666
});
665667
}
666668

669+
/// A user message that interrupts an in-flight turn must finalize the active
670+
/// stream (so the old message stops and its cards complete) BEFORE the new
671+
/// turn streams — and the interrupted turn's never-resolved tool entries must
672+
/// not desync the next turn's completion pairing.
673+
///
674+
/// Mirrors the live incident: the daemon settles interrupted tool calls with
675+
/// history-only synthetic results (no ToolResult broadcast) and its cancel
676+
/// ResponseDone is swallowed by the keep-open rule, so without the
677+
/// message-triggered finalize the old stream stayed open forever.
678+
#[test]
679+
fn user_message_mid_turn_finalizes_the_stream() {
680+
let mut flow = FlowBuilder::new();
681+
let (slack_tx, daemon_tx, actions_out, commands_out) = build_sim(&mut flow);
682+
683+
flow.sim().unit_test_fuzz_iterations(256).fuzz(async || {
684+
let thread_ts = fresh("interrupt");
685+
let channel = fresh("C");
686+
let session_id = fresh("sess");
687+
688+
// Establish the session.
689+
slack_tx.send(chat("U1", "go", &channel, &thread_ts));
690+
assert!(matches!(
691+
commands_out.next().await.unwrap(),
692+
DaemonCommand::CreateSession { .. }
693+
));
694+
daemon_tx.send(devent(
695+
&thread_ts,
696+
infinity_protocol::DaemonMessage::Connected {
697+
root_thread_id: session_id.clone().into(),
698+
thread_id: session_id.clone().into(),
699+
model_name: String::new(),
700+
context_window: 0,
701+
title: Some("T".to_owned()),
702+
total_tokens_used: 0,
703+
provider_id: String::new(),
704+
},
705+
));
706+
loop {
707+
match actions_out.next().await.unwrap() {
708+
SlackAction::SetThreadTitle { .. } => break,
709+
SlackAction::SetStatus { .. } => {}
710+
other => panic!("expected SetThreadTitle, got {other:?}"),
711+
}
712+
}
713+
714+
// A turn is in flight: text + a tool call whose result will never be
715+
// broadcast (the user's message interrupts it daemon-side).
716+
daemon_tx.send_many([
717+
devent(&thread_ts, text_chunk("part one")),
718+
devent(&thread_ts, tool_call("execute_command")),
719+
]);
720+
721+
// Mid-turn user message → SendInput (and, in the same slack-branch
722+
// tick, the finalize StreamStop).
723+
slack_tx.send(chat(
724+
"U1",
725+
"actually, do this instead",
726+
&channel,
727+
&thread_ts,
728+
));
729+
match commands_out.next().await.unwrap() {
730+
DaemonCommand::SendInput { thread_ts: t, .. } => assert_eq!(t, thread_ts),
731+
other => panic!("expected SendInput, got {other:?}"),
732+
}
733+
734+
// The daemon's post-interrupt behavior: the cancelled step's
735+
// ResponseDone (swallowed: the old turn had a tool call), then the new
736+
// turn with its own tool round.
737+
daemon_tx.send_many([
738+
devent(&thread_ts, response_done()), // cancel notification
739+
devent(&thread_ts, tool_call("bash")), // new turn
740+
devent(&thread_ts, response_done()), // round end (swallowed)
741+
devent(&thread_ts, tool_result()), // bash completes
742+
devent(&thread_ts, text_chunk("done")),
743+
devent(&thread_ts, response_done()), // final → stop
744+
]);
745+
746+
let actions: Vec<SlackAction> = actions_out.collect().await;
747+
748+
// The daemon-branch sequence is totally ordered; the slack-branch
749+
// finalize stop may interleave anywhere, so pull the stops out and
750+
// assert the rest by order.
751+
let stops: Vec<_> = actions
752+
.iter()
753+
.filter(|a| matches!(a, SlackAction::StreamStop { .. }))
754+
.collect();
755+
assert_eq!(
756+
stops.len(),
757+
2,
758+
"expected the message-triggered finalize stop and the final \
759+
response stop, got {stops:?}"
760+
);
761+
for stop in &stops {
762+
match stop {
763+
SlackAction::StreamStop {
764+
thread_ts: t,
765+
subagent,
766+
end_of_response,
767+
..
768+
} => {
769+
assert_eq!(t, &thread_ts);
770+
assert!(subagent.is_none());
771+
assert!(*end_of_response, "both stops end their response");
772+
}
773+
_ => unreachable!(),
774+
}
775+
}
776+
777+
// Everything else (appends and task updates) is daemon-branch ordered.
778+
let core: Vec<_> = non_status(&actions)
779+
.into_iter()
780+
.filter(|a| !matches!(a, SlackAction::StreamStop { .. }))
781+
.collect();
782+
match &core[..] {
783+
[SlackAction::StreamAppend { text: t1, .. }, SlackAction::StreamTaskUpdate {
784+
task_id: old_task,
785+
title: old_title,
786+
status: s1,
787+
..
788+
}, SlackAction::StreamTaskUpdate {
789+
task_id: new_task,
790+
title: new_title,
791+
status: s2,
792+
..
793+
}, SlackAction::StreamTaskUpdate {
794+
task_id: done_task,
795+
status: s3,
796+
..
797+
}, SlackAction::StreamAppend { text: t2, .. }] => {
798+
assert_eq!(t1, "part one");
799+
assert_eq!(old_title, "execute_command");
800+
assert_eq!(s1, "in_progress");
801+
assert_eq!(new_title, "bash");
802+
assert_eq!(s2, "in_progress");
803+
assert_eq!(s3, "complete");
804+
// The completion pairs with the NEW turn's call: the
805+
// interrupted execute_command entry was cleared when the user
806+
// message arrived, so it cannot swallow bash's result.
807+
assert_eq!(
808+
done_task, new_task,
809+
"completion must pair with the new turn's call"
810+
);
811+
assert_ne!(
812+
done_task, old_task,
813+
"completion must not pair with the interrupted call"
814+
);
815+
assert_eq!(t2, "done");
816+
}
817+
other => panic!(
818+
"expected [append, task(old in_progress), task(new in_progress), \
819+
task(new complete), append], got {other:?}"
820+
),
821+
}
822+
});
823+
}
824+
667825
#[test]
668826
fn subagent_output_is_tagged_and_tracked_separately() {
669827
let mut flow = FlowBuilder::new();

0 commit comments

Comments
 (0)