runtime: route notifications through worker inbox
This commit is contained in:
@@ -285,6 +285,7 @@ impl WorkerController {
|
||||
worker.push_notify(
|
||||
"Restored Worker state contained unreachable delegated child Workers; their delegated write scopes were reclaimed before resume."
|
||||
.to_string(),
|
||||
false,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -883,7 +884,7 @@ async fn controller_loop<C, St>(
|
||||
)
|
||||
.await;
|
||||
let parent_originated = run.is_parent_originated();
|
||||
let (new_status, shutdown) = match run {
|
||||
let (mut new_status, shutdown) = match run {
|
||||
PendingRun::Run(input) => {
|
||||
drive_turn(
|
||||
worker.run(input),
|
||||
@@ -930,6 +931,11 @@ async fn controller_loop<C, St>(
|
||||
.await
|
||||
}
|
||||
};
|
||||
if !shutdown && new_status == WorkerStatus::Idle && notify_buffer.has_auto_run_pending()
|
||||
{
|
||||
pending = Some(PendingRun::RunForNotification(protocol::InvokeKind::Notify));
|
||||
new_status = WorkerStatus::Running;
|
||||
}
|
||||
finish_controller_run(
|
||||
&mut worker,
|
||||
&shared_state,
|
||||
@@ -985,13 +991,13 @@ async fn controller_loop<C, St>(
|
||||
// `LogEntry::SystemItem` entry — drained out of the
|
||||
// notify buffer + broadcast through the sink. No
|
||||
// separate echo here.
|
||||
worker.push_notify(message);
|
||||
// RUNNING / Paused: the buffer push is the entire
|
||||
// operation; an in-flight turn (or the next
|
||||
// Resume/Run) will drain it at its next
|
||||
// pending_history_appends. IDLE: only `auto_run`
|
||||
// notifications stage RunForNotification; weak progress
|
||||
// notices stay queued until an explicit run/resume.
|
||||
worker.push_notify(message, auto_run);
|
||||
// RUNNING: the in-flight turn drains the buffer at its next
|
||||
// pending_history_appends; if an auto-run notification remains
|
||||
// at turn end, the Controller stages a follow-up notification
|
||||
// turn. Paused notifications remain queued until Resume/Run.
|
||||
// IDLE: `auto_run` notifications stage RunForNotification;
|
||||
// weak progress notices stay queued until an explicit run.
|
||||
if should_auto_run_notification(shared_state.get_status(), auto_run) {
|
||||
pending = Some(PendingRun::RunForNotification(protocol::InvokeKind::Notify));
|
||||
}
|
||||
@@ -1385,11 +1391,11 @@ where
|
||||
.into(),
|
||||
});
|
||||
}
|
||||
Some(Method::Notify { message, .. }) => {
|
||||
Some(Method::Notify { message, auto_run }) => {
|
||||
// Live echo arrives via `Event::SystemItem` once
|
||||
// the in-flight turn's next `pending_history_appends`
|
||||
// drains this entry through the interceptor.
|
||||
notify_buffer.push_notify(message);
|
||||
notify_buffer.push_notify(message, auto_run);
|
||||
}
|
||||
Some(Method::ListCompletions { .. }) => {}
|
||||
Some(Method::ListWorkers | Method::RestoreWorker { .. } | Method::RegisterPeer { .. }) => {
|
||||
@@ -1904,6 +1910,41 @@ mod tests {
|
||||
assert_eq!(env.notify_buffer.len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn running_auto_run_notify_remains_staged_for_followup_turn() {
|
||||
let mut env = make_env().await;
|
||||
env._method_tx
|
||||
.send(Method::Notify {
|
||||
message: "continue".into(),
|
||||
auto_run: true,
|
||||
})
|
||||
.await
|
||||
.expect("send notify");
|
||||
|
||||
let worker_future = async {
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
Ok::<_, WorkerError>(WorkerRunResult::Finished)
|
||||
};
|
||||
let (status, shutdown) = drive_turn(
|
||||
worker_future,
|
||||
&mut env.method_rx,
|
||||
&env.event_tx,
|
||||
&env.cancel_tx,
|
||||
&env.shared_state,
|
||||
&env.notify_buffer,
|
||||
Some(&env.parent_socket_path),
|
||||
"parent",
|
||||
&env.spawned_registry,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(status, WorkerStatus::Idle);
|
||||
assert!(!shutdown);
|
||||
assert_eq!(env.notify_buffer.len(), 1);
|
||||
assert!(env.notify_buffer.has_auto_run_pending());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn compact_method_is_rejected_while_running() {
|
||||
let mut env = make_env().await;
|
||||
|
||||
@@ -220,7 +220,9 @@ impl Interceptor for WorkerInterceptor {
|
||||
// simply be skipped from the SystemItem batch.
|
||||
warn!(error = %e, "failed to render notify_wrapper; using raw message");
|
||||
let fallback = match &entry {
|
||||
super::notify_buffer::PendingNotify::Notify { message } => message.clone(),
|
||||
super::notify_buffer::PendingNotify::Notify { message, .. } => {
|
||||
message.clone()
|
||||
}
|
||||
super::notify_buffer::PendingNotify::WorkerEvent { event } => {
|
||||
session_store::render_worker_event(event)
|
||||
}
|
||||
@@ -1019,8 +1021,8 @@ mod tests {
|
||||
async fn pending_history_appends_drains_buffer_into_items() {
|
||||
let registry = Arc::new(HookRegistryBuilder::new().build());
|
||||
let buffer = NotifyBuffer::new();
|
||||
buffer.push_notify("first".into());
|
||||
buffer.push_notify("second".into());
|
||||
buffer.push_notify("first".into(), false);
|
||||
buffer.push_notify("second".into(), false);
|
||||
|
||||
let interceptor = WorkerInterceptor::new(
|
||||
registry,
|
||||
@@ -1057,7 +1059,7 @@ mod tests {
|
||||
// anything itself.
|
||||
let registry = Arc::new(HookRegistryBuilder::new().build());
|
||||
let buffer = NotifyBuffer::new();
|
||||
buffer.push_notify("msg".into());
|
||||
buffer.push_notify("msg".into(), false);
|
||||
|
||||
let interceptor = WorkerInterceptor::new(
|
||||
registry,
|
||||
|
||||
@@ -41,7 +41,7 @@ const CAPACITY: usize = 128;
|
||||
/// is available.
|
||||
#[derive(Debug, Clone)]
|
||||
pub enum PendingNotify {
|
||||
Notify { message: String },
|
||||
Notify { message: String, auto_run: bool },
|
||||
WorkerEvent { event: WorkerEvent },
|
||||
}
|
||||
|
||||
@@ -61,8 +61,8 @@ impl NotifyBuffer {
|
||||
/// Push a notify entry onto the queue. If the queue is full, the
|
||||
/// oldest entry is dropped and a `tracing::warn` is emitted — the
|
||||
/// caller should never hit this in normal operation.
|
||||
pub fn push_notify(&self, message: String) {
|
||||
self.push_entry(PendingNotify::Notify { message });
|
||||
pub fn push_notify(&self, message: String, auto_run: bool) {
|
||||
self.push_entry(PendingNotify::Notify { message, auto_run });
|
||||
}
|
||||
|
||||
/// Push a typed worker-event entry onto the queue.
|
||||
@@ -89,6 +89,15 @@ impl NotifyBuffer {
|
||||
q.drain(..).collect()
|
||||
}
|
||||
|
||||
/// Whether an undrained `Method::Notify { auto_run: true }` remains.
|
||||
pub fn has_auto_run_pending(&self) -> bool {
|
||||
self.inner
|
||||
.lock()
|
||||
.expect("notify buffer poisoned")
|
||||
.iter()
|
||||
.any(|entry| matches!(entry, PendingNotify::Notify { auto_run: true, .. }))
|
||||
}
|
||||
|
||||
/// Number of pending entries. Primarily for tests.
|
||||
pub fn len(&self) -> usize {
|
||||
self.inner.lock().expect("notify buffer poisoned").len()
|
||||
@@ -107,7 +116,7 @@ pub(crate) fn build_system_item(
|
||||
prompts: &PromptCatalog,
|
||||
) -> Result<SystemItem, CatalogError> {
|
||||
match entry {
|
||||
PendingNotify::Notify { message } => {
|
||||
PendingNotify::Notify { message, .. } => {
|
||||
let body = prompts.notify_wrapper(message)?;
|
||||
Ok(SystemItem::Notification {
|
||||
message: message.clone(),
|
||||
@@ -132,12 +141,15 @@ mod tests {
|
||||
#[test]
|
||||
fn push_then_drain_preserves_order() {
|
||||
let buf = NotifyBuffer::new();
|
||||
buf.push_notify("one".into());
|
||||
buf.push_notify("two".into());
|
||||
buf.push_notify("one".into(), false);
|
||||
assert!(!buf.has_auto_run_pending());
|
||||
buf.push_notify("two".into(), true);
|
||||
assert!(buf.has_auto_run_pending());
|
||||
let drained = buf.drain();
|
||||
assert!(!buf.has_auto_run_pending());
|
||||
assert_eq!(drained.len(), 2);
|
||||
match &drained[0] {
|
||||
PendingNotify::Notify { message } => assert_eq!(message, "one"),
|
||||
PendingNotify::Notify { message, .. } => assert_eq!(message, "one"),
|
||||
other => panic!("unexpected: {other:?}"),
|
||||
}
|
||||
assert!(buf.is_empty());
|
||||
@@ -147,12 +159,12 @@ mod tests {
|
||||
fn capacity_drops_oldest() {
|
||||
let buf = NotifyBuffer::new();
|
||||
for i in 0..(CAPACITY + 5) {
|
||||
buf.push_notify(format!("msg{i}"));
|
||||
buf.push_notify(format!("msg{i}"), false);
|
||||
}
|
||||
let drained = buf.drain();
|
||||
assert_eq!(drained.len(), CAPACITY);
|
||||
match &drained[0] {
|
||||
PendingNotify::Notify { message } => assert_eq!(message, "msg5"),
|
||||
PendingNotify::Notify { message, .. } => assert_eq!(message, "msg5"),
|
||||
other => panic!("unexpected: {other:?}"),
|
||||
}
|
||||
}
|
||||
@@ -161,6 +173,7 @@ mod tests {
|
||||
fn build_system_item_for_notify_carries_wrapper_body() {
|
||||
let entry = PendingNotify::Notify {
|
||||
message: "hello".into(),
|
||||
auto_run: false,
|
||||
};
|
||||
let catalog = PromptCatalog::builtins_only().unwrap();
|
||||
let item = build_system_item(&entry, &catalog).unwrap();
|
||||
|
||||
@@ -1597,8 +1597,8 @@ impl<C: LlmClient, St: Store> Worker<C, St> {
|
||||
/// `Item::system_message` just before the next LLM request, via
|
||||
/// `WorkerInterceptor::pending_history_appends`. See [`NotifyBuffer`]
|
||||
/// for overflow behaviour and the lane-of-record rationale.
|
||||
pub fn push_notify(&self, message: String) {
|
||||
self.pending_notifies.push_notify(message);
|
||||
pub fn push_notify(&self, message: String, auto_run: bool) {
|
||||
self.pending_notifies.push_notify(message, auto_run);
|
||||
}
|
||||
|
||||
/// Push an agent-visible typed `WorkerEvent` entry onto the pending buffer.
|
||||
@@ -4364,6 +4364,7 @@ where
|
||||
self.push_notify(
|
||||
"Restored Worker state contained missing or unreachable delegated child Workers; their delegated write scopes were reclaimed before resume."
|
||||
.to_string(),
|
||||
false,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user