diff --git a/Cargo.lock b/Cargo.lock index e6038be2..8c6a126e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6650,6 +6650,7 @@ dependencies = [ "serial_test", "session-metrics", "session-store", + "sha2 0.11.0", "tempfile", "thiserror 2.0.18", "ticket", diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 517d6a7c..b760ce00 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -120,12 +120,15 @@ mod tests { let mut client = Client::new(socket); client - .send(&Method::run_text("hello")) + .send(&Method::submit_text( + protocol::new_submission_request_id(), + "hello", + )) .await .expect("send method"); assert!(matches!( decode_method(&client.socket.sent[0]), - Ok(Method::Run { .. }) + Ok(Method::Submit { .. }) )); assert!(matches!( client.next_event().await, diff --git a/crates/client/src/transport/in_process.rs b/crates/client/src/transport/in_process.rs index b1db8e6b..3111808a 100644 --- a/crates/client/src/transport/in_process.rs +++ b/crates/client/src/transport/in_process.rs @@ -89,12 +89,15 @@ mod tests { let mut client = Client::new(socket); client - .send(&Method::run_text("hello")) + .send(&Method::submit_text( + protocol::new_submission_request_id(), + "hello", + )) .await .expect("send method"); assert!(matches!( peer.next().await.as_deref().map(decode_method), - Some(Ok(Method::Run { .. })) + Some(Ok(Method::Submit { .. })) )); peer.send( diff --git a/crates/client/src/transport/unix_socket.rs b/crates/client/src/transport/unix_socket.rs index 0262bff2..089ed83e 100644 --- a/crates/client/src/transport/unix_socket.rs +++ b/crates/client/src/transport/unix_socket.rs @@ -147,12 +147,18 @@ mod tests { let mut client = Client::new(Socket::connect(&socket_path).await.unwrap()); client - .send(&Method::run_text("hello")) + .send(&Method::submit_text( + protocol::new_submission_request_id(), + "hello", + )) .await .expect("send method"); let received = server.await.unwrap().expect("method message"); - assert!(matches!(decode_method(&received), Ok(Method::Run { .. }))); + assert!(matches!( + decode_method(&received), + Ok(Method::Submit { .. }) + )); } #[tokio::test] diff --git a/crates/client/src/transport/websocket.rs b/crates/client/src/transport/websocket.rs index e8640573..b4c1ed84 100644 --- a/crates/client/src/transport/websocket.rs +++ b/crates/client/src/transport/websocket.rs @@ -114,7 +114,7 @@ mod tests { assert!(matches!( message, Message::Text(ref text) - if matches!(decode_method(text), Ok(Method::Run { .. })) + if matches!(decode_method(text), Ok(Method::Submit { .. })) )); let event = encode_event(&Event::Status { status: WorkerStatus::Idle, @@ -126,7 +126,10 @@ mod tests { let request = format!("ws://{address}").into_client_request().unwrap(); let mut client = Client::new(Socket::connect(request).await.unwrap()); client - .send(&Method::run_text("hello")) + .send(&Method::submit_text( + protocol::new_submission_request_id(), + "hello", + )) .await .expect("send method"); assert!(matches!( diff --git a/crates/protocol/src/lib.rs b/crates/protocol/src/lib.rs index 9f0ab12c..7171461c 100644 --- a/crates/protocol/src/lib.rs +++ b/crates/protocol/src/lib.rs @@ -11,6 +11,11 @@ use serde::{Deserialize, Serialize}; pub use identity::{WorkerId, WorkerIdParseError}; +/// Allocate an opaque idempotency key for one client Submit request. +pub fn new_submission_request_id() -> String { + uuid::Uuid::now_v7().to_string() +} + fn default_true() -> bool { true } @@ -31,17 +36,22 @@ fn is_false(value: &bool) -> bool { #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] #[serde(tag = "method", content = "params", rename_all = "snake_case")] pub enum Method { - Run { + /// Durably accept typed input for immediate activation or the session FIFO. + /// + /// `submission_request_id` is generated by the authenticated caller and is + /// used only for idempotent retry. Worker allocates the durable + /// `submission_id` returned by [`Event::SubmissionAccepted`]. + Submit { + submission_request_id: String, input: Vec, }, - /// Runtime-internal Run carrying an opaque correlation id that is committed - /// with the resulting UserInput entry. This variant is not serializable on - /// the public Client → Worker protocol. + /// Runtime-internal Submit with the same request identity contract. This + /// variant is not serializable on the public Client → Worker protocol. #[serde(skip)] #[cfg_attr(feature = "typescript", ts(skip))] - RunTracked { + SubmitTracked { + submission_request_id: String, input: Vec, - submission_id: String, }, /// Human-readable text injected into the target Worker's LLM context /// as a non-blocking system message. `auto_run` controls whether an @@ -50,12 +60,25 @@ pub enum Method { /// No side effects beyond LLM context; use `WorkerEvent` for typed /// lifecycle reports. Notify { + notification_request_id: String, message: String, #[serde(default = "default_true", skip_serializing_if = "is_true")] auto_run: bool, }, /// Typed lifecycle report from a child Worker to its direct parent. WorkerEvent(WorkerEvent), + /// Return the authoritative FIFO summary without exposing queued payloads. + ListPendingSubmissions, + /// Remove one queued submission. Running or already activated submissions + /// are immutable and therefore cannot be cancelled here. + CancelPendingSubmission { + submission_id: String, + }, + /// Remove every queued submission while preserving the active run. + ClearPendingSubmissions, + /// Activate the next queued submission while the Worker is idle. This is an + /// explicit recovery operation and never resumes a paused run implicitly. + ContinuePending, Resume, Cancel, /// Stop the in-flight turn and transition to `Paused`. @@ -68,7 +91,7 @@ pub enum Method { /// Request an explicit compaction while the Worker is otherwise idle. /// /// This is a typed control method: clients must not send `compact` as a - /// `Method::Run` user message. + /// `Method::Submit` user message. Compact, /// Ask the Worker to list valid rewind targets from its authoritative session log. ListRewindTargets, @@ -181,7 +204,7 @@ impl WorkerEvent { /// One typed piece of a user submission. /// -/// `Method::Run` and `Event::UserMessage` carry `Vec`. Dumb +/// `Method::Submit` and `Event::UserMessage` carry `Vec`. Dumb /// clients (CLI piping, scripts) only need to produce a single /// `Segment::Text`; richer clients (TUI / GUI) construct typed atoms /// (paste chips, file refs) and @@ -404,12 +427,13 @@ impl Segment { } impl Method { - /// Convenience: a `Run` carrying a single `Segment::Text`. + /// Convenience: a `Submit` carrying a single `Segment::Text`. /// Used by dumb clients, inter-Worker tools, and tests that only have /// a string to forward. - pub fn run_text(s: impl Into) -> Self { - Self::Run { - input: vec![Segment::text(s)], + pub fn submit_text(submission_request_id: impl Into, text: impl Into) -> Self { + Self::Submit { + submission_request_id: submission_request_id.into(), + input: vec![Segment::text(text)], } } } @@ -503,6 +527,37 @@ pub enum ToolResultDisposition { OutcomeUnknown, } +/// Durable acceptance result for one idempotent Submit request. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(rename_all = "snake_case")] +pub enum SubmissionDisposition { + Started, + Queued, +} + +/// Bounded public projection of one pending submission. Payload segments and +/// provenance remain in the session log and are intentionally not exposed. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +pub struct PendingSubmissionSummary { + pub submission_id: String, + pub accepted_at_ms: u64, + pub segment_count: u32, + pub byte_len: u64, +} + +/// Revisioned session-owned FIFO projection used by snapshots and live events. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +pub struct PendingSubmissionsSnapshot { + pub revision: u64, + #[serde(default)] + pub notification_count: u32, + #[serde(default)] + pub submissions: Vec, +} + /// Canonical, storage-independent projection of committed session history. /// /// Worker protocols expose this DTO instead of append-log records. New @@ -511,6 +566,8 @@ pub enum ToolResultDisposition { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] pub struct SessionSnapshot { + #[serde(default)] + pub pending_submissions: PendingSubmissionsSnapshot, pub entries: Vec, } @@ -609,16 +666,27 @@ pub struct SessionToolAttachment { #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] #[serde(tag = "event", content = "data", rename_all = "snake_case")] pub enum Event { - /// A user input message was accepted, persisted as - /// `LogEntry::AnnotatedUserInput`, and is about to start a new turn. - /// Broadcast to every subscribed client so TUI / GUI instances show - /// the same user line that reconnect snapshots would replay from - /// history; clients must not synthesize a separate pending/fake - /// message for accepted runs. - /// - /// Fires exactly once per committed user input, after - /// `InvokeStart { kind: UserSend }` and before the first - /// `TurnStart`. Rejected runs (e.g. `AlreadyRunning`) do not emit. + /// Durable Submit acceptance. A `Started` receipt follows the atomic + /// UserInput commit; a `Queued` receipt follows the durable FIFO checkpoint. + /// Repeating the same request id and exact payload returns the same receipt + /// without appending or activating twice. + SubmissionAccepted { + submission_request_id: String, + submission_id: String, + disposition: SubmissionDisposition, + }, + /// Correlated rejection before durable acceptance. + SubmissionRejected { + submission_request_id: String, + message: String, + }, + /// Revisioned FIFO replacement following enqueue, activation, cancel, or clear. + PendingSubmissionsChanged { + pending: PendingSubmissionsSnapshot, + }, + /// A user input message persisted as `LogEntry::AnnotatedUserInput` and + /// activated for a turn. Broadcast to every subscribed client so TUI / GUI + /// instances show the same user line that reconnect snapshots replay. UserMessage { segments: Vec, }, @@ -641,7 +709,7 @@ pub enum Event { /// /// Marker event for the start of an Invoke range; the range extends /// implicitly until the next `InvokeStart`. Fires for every accepted - /// `Method::Run` (kind=`UserSend`), `Method::Notify` (kind=`Notify`), + /// `Method::Submit` (kind=`UserSend`), `Method::Notify` (kind=`Notify`), /// `Method::WorkerEvent` re-injection (kind=`WorkerEvent`), and any other /// IDLE-breaking trigger. Mid-run interrupts (e.g. hook output, /// typed system reminder insertion that doesn't break IDLE) do not @@ -1193,7 +1261,7 @@ pub enum TurnResult { #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] #[serde(rename_all = "snake_case")] pub enum InvokeKind { - /// `Method::Run` — a user submission. + /// `Method::Submit` — a user submission. UserSend, /// `Method::Notify` — free-text notification injected into history. Notify, @@ -1216,7 +1284,7 @@ pub enum RunResult { Finished, Paused, LimitReached, - /// The accepted Method::Run produced no assistant/tool output before + /// The accepted Method::Submit produced no assistant/tool output before /// user interruption, so the Worker rolled the submit-time turn state back /// to its pre-submit snapshot. Clients should treat the Worker as Idle and /// restore the just-submitted input into the editable composer if desired. @@ -1285,26 +1353,30 @@ mod tests { use super::*; #[test] - fn method_run_json_roundtrip() { - let json = r#"{"method":"run","params":{"input":[{"kind":"text","content":"Hello"}]}}"#; + fn method_submit_json_roundtrip_and_run_is_rejected() { + let json = r#"{"method":"submit","params":{"submission_request_id":"request-1","input":[{"kind":"text","content":"Hello"}]}}"#; let method: Method = serde_json::from_str(json).unwrap(); match &method { - Method::Run { input } => { + Method::Submit { input, .. } => { assert_eq!(input.len(), 1); match &input[0] { Segment::Text { content } => assert_eq!(content, "Hello"), other => panic!("expected Text, got {other:?}"), } } - other => panic!("expected Run, got {other:?}"), + other => panic!("expected Submit, got {other:?}"), } let serialized = serde_json::to_string(&method).unwrap(); assert_eq!(serialized, json); + assert!( + serde_json::from_str::(r#"{"method":"run","params":{"input":[]}}"#).is_err() + ); } #[test] - fn method_run_paste_segment_roundtrip() { - let method = Method::Run { + fn method_submit_paste_segment_roundtrip() { + let method = Method::Submit { + submission_request_id: "request-1".to_string(), input: vec![ Segment::text("see "), Segment::Paste { @@ -1318,7 +1390,7 @@ mod tests { let json = serde_json::to_string(&method).unwrap(); let decoded: Method = serde_json::from_str(&json).unwrap(); match decoded { - Method::Run { input } => { + Method::Submit { input, .. } => { assert_eq!(input.len(), 2); match &input[1] { Segment::Paste { @@ -1335,7 +1407,7 @@ mod tests { other => panic!("expected Paste, got {other:?}"), } } - other => panic!("expected Run, got {other:?}"), + other => panic!("expected Submit, got {other:?}"), } } @@ -1389,8 +1461,9 @@ mod tests { } #[test] - fn method_run_flow_segment_roundtrip() { - let method = Method::Run { + fn method_submit_flow_segment_roundtrip() { + let method = Method::Submit { + submission_request_id: "request-1".to_string(), input: vec![ Segment::Flow { selector: "builtin:coder-review".to_string(), @@ -1404,7 +1477,7 @@ mod tests { let decoded = serde_json::from_str::(&json).unwrap(); assert!(matches!( decoded, - Method::Run { input } + Method::Submit { input, .. } if matches!( input.as_slice(), [ @@ -1416,15 +1489,15 @@ mod tests { } #[test] - fn runtime_tracked_run_is_not_public_protocol_json() { - let method = Method::RunTracked { + fn runtime_tracked_submit_is_not_public_protocol_json() { + let method = Method::SubmitTracked { input: vec![Segment::text("private")], - submission_id: "submission-1".to_string(), + submission_request_id: "request-1".to_string(), }; assert!(serde_json::to_string(&method).is_err()); assert!( serde_json::from_str::( - r#"{"method":"run_tracked","input":[],"submission_id":"forged"}"#, + r#"{"method":"submit_tracked","input":[],"submission_id":"forged"}"#, ) .is_err() ); @@ -1442,16 +1515,16 @@ mod tests { } #[test] - fn method_run_with_unknown_segment_decodes() { - let json = r#"{"method":"run","params":{"input":[{"kind":"text","content":"hi"},{"kind":"future_thing","x":1}]}}"#; + fn method_submit_with_unknown_segment_decodes() { + let json = r#"{"method":"submit","params":{"submission_request_id":"request-1","input":[{"kind":"text","content":"hi"},{"kind":"future_thing","x":1}]}}"#; let method: Method = serde_json::from_str(json).unwrap(); match method { - Method::Run { input } => { + Method::Submit { input, .. } => { assert_eq!(input.len(), 2); assert!(matches!(input[0], Segment::Text { .. })); assert!(matches!(input[1], Segment::Unknown)); } - other => panic!("expected Run, got {other:?}"), + other => panic!("expected Submit, got {other:?}"), } } @@ -1648,11 +1721,11 @@ mod tests { #[test] fn method_notify_json_roundtrip_defaults_to_auto_run() { - let json = r#"{"method":"notify","params":{"message":"turn done"}}"#; + let json = r#"{"method":"notify","params":{"notification_request_id":"notification-1","message":"turn done"}}"#; let method: Method = serde_json::from_str(json).unwrap(); assert!(matches!( method, - Method::Notify { ref message, auto_run: true } if message == "turn done" + Method::Notify { ref message, auto_run: true, .. } if message == "turn done" )); let serialized = serde_json::to_string(&method).unwrap(); assert_eq!(serialized, json); @@ -1660,11 +1733,11 @@ mod tests { #[test] fn method_notify_weak_json_roundtrip_serializes_auto_run_false() { - let json = r#"{"method":"notify","params":{"message":"progress","auto_run":false}}"#; + let json = r#"{"method":"notify","params":{"notification_request_id":"notification-1","message":"progress","auto_run":false}}"#; let method: Method = serde_json::from_str(json).unwrap(); assert!(matches!( method, - Method::Notify { ref message, auto_run: false } if message == "progress" + Method::Notify { ref message, auto_run: false, .. } if message == "progress" )); assert_eq!(serde_json::to_string(&method).unwrap(), json); } @@ -1725,6 +1798,7 @@ mod tests { fn event_snapshot_format() { let event = Event::Snapshot { session: SessionSnapshot { + pending_submissions: PendingSubmissionsSnapshot::default(), entries: vec![SessionSnapshotEntry { entry_id: "entry-1".into(), timestamp: 1, @@ -1776,6 +1850,7 @@ mod tests { let event = Event::Snapshot { session: SessionSnapshot { + pending_submissions: PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: Greeting { @@ -1844,6 +1919,7 @@ mod tests { fn event_segment_rotated_roundtrip() { let event = Event::SegmentRotated { session: SessionSnapshot { + pending_submissions: PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, }; diff --git a/crates/protocol/src/typescript.rs b/crates/protocol/src/typescript.rs index bdac40ca..bf63c8d6 100644 --- a/crates/protocol/src/typescript.rs +++ b/crates/protocol/src/typescript.rs @@ -8,11 +8,11 @@ use crate::{ CompletionKind, ErrorCode, Event, Greeting, InFlightBlock, InFlightSnapshot, InFlightToolCallState, InternalWorkerKind, InternalWorkerRef, InternalWorkerSnapshot, InvokeKind, MemoryWorkerEvent, Method, PasteArtifactAvailability, PasteArtifactMediaType, - PasteArtifactRef, Permission, RewindSummary, RewindTarget, RewindTargetId, RunResult, - ScopeRule, Segment, SessionContentPart, SessionEntryProvenance, SessionMessageRole, - SessionSnapshot, SessionSnapshotEntry, SessionSnapshotEntryData, SessionToolAttachment, - ToolResultDisposition, TurnResult, UploadedFileAvailability, UploadedFileRef, WorkerEvent, - WorkerStatus, + PasteArtifactRef, PendingSubmissionSummary, PendingSubmissionsSnapshot, Permission, + RewindSummary, RewindTarget, RewindTargetId, RunResult, ScopeRule, Segment, SessionContentPart, + SessionEntryProvenance, SessionMessageRole, SessionSnapshot, SessionSnapshotEntry, + SessionSnapshotEntryData, SessionToolAttachment, SubmissionDisposition, ToolResultDisposition, + TurnResult, UploadedFileAvailability, UploadedFileRef, WorkerEvent, WorkerStatus, subscription::{ EventSubscriptionSelector, SubscriptionEvent, SubscriptionEventPayload, SubscriptionFrame, SubscriptionFramePayload, SubscriptionId, SubscriptionRejectionCode, SubscriptionRequest, @@ -75,6 +75,9 @@ pub fn generated_protocol_types() -> String { push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); + push_decl::(&cfg, &mut output); + push_decl::(&cfg, &mut output); + push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); diff --git a/crates/session-store/src/legacy_session_log.rs b/crates/session-store/src/legacy_session_log.rs index 40dd0fd5..bb9dd1dd 100644 --- a/crates/session-store/src/legacy_session_log.rs +++ b/crates/session-store/src/legacy_session_log.rs @@ -183,6 +183,7 @@ fn canonicalize_history_entry( item, metadata: legacy_metadata(segment_id, line_index, 0), }, + extensions: Vec::new(), }, } } diff --git a/crates/session-store/src/public_snapshot.rs b/crates/session-store/src/public_snapshot.rs index f3b61754..b1793584 100644 --- a/crates/session-store/src/public_snapshot.rs +++ b/crates/session-store/src/public_snapshot.rs @@ -71,7 +71,7 @@ pub fn project_session_snapshot(session_id: SessionId, log: &[LogEntry]) -> Sess entries.push(history_entry(entry, *ts, data)); } } - LogEntry::AnnotatedSystemItem { ts, entry } => entries.push(system_entry( + LogEntry::AnnotatedSystemItem { ts, entry, .. } => entries.push(system_entry( &entry.item, entry.metadata.entry_id.0.clone(), *ts, @@ -100,7 +100,10 @@ pub fn project_session_snapshot(session_id: SessionId, log: &[LogEntry]) -> Sess } } - SessionSnapshot { entries } + SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries, + } } fn extend_history( diff --git a/crates/session-store/src/segment.rs b/crates/session-store/src/segment.rs index 163b2317..75d997c2 100644 --- a/crates/session-store/src/segment.rs +++ b/crates/session-store/src/segment.rs @@ -287,6 +287,7 @@ pub fn append_system_item( LogEntry::AnnotatedSystemItem { ts: segment_log::now_millis(), entry, + extensions: Vec::new(), }, ) } diff --git a/crates/session-store/src/segment_log.rs b/crates/session-store/src/segment_log.rs index 9daf85c2..ee509cbb 100644 --- a/crates/session-store/src/segment_log.rs +++ b/crates/session-store/src/segment_log.rs @@ -112,6 +112,8 @@ pub enum LogEntry { AnnotatedSystemItem { ts: u64, entry: LoggedSystemHistoryEntry, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + extensions: Vec, }, /// Turn boundary. Records the turn count after increment. @@ -312,12 +314,19 @@ pub fn collect_state(entries: &[LogEntry]) -> RestoredState { state.annotated_history.push(entry.clone()); state.history.push(Item::from(entry.item.clone())); } - LogEntry::AnnotatedSystemItem { entry, .. } => { + LogEntry::AnnotatedSystemItem { + entry, extensions, .. + } => { state.annotated_history.push(LoggedHistoryEntry { item: LoggedItem::from(entry.item.to_history_item()), metadata: entry.metadata.clone(), }); state.history.push(entry.item.to_history_item()); + state.extensions.extend( + extensions + .iter() + .map(|extension| (extension.domain.clone(), extension.payload.clone())), + ); } LogEntry::TurnEnd { turn_count, .. } => { if let Some(active_turn_count) = &mut state.active_run_turn_count { diff --git a/crates/standalone/tests/host.rs b/crates/standalone/tests/host.rs index 510cc68c..45c8f620 100644 --- a/crates/standalone/tests/host.rs +++ b/crates/standalone/tests/host.rs @@ -99,7 +99,10 @@ async fn in_process_host_runs_text_and_read_tool_then_shuts_down() { let mut protocol_client = host.connect(); protocol_client - .send(&Method::run_text("read the probe")) + .send(&Method::submit_text( + protocol::new_submission_request_id(), + "read the probe", + )) .await .expect("submit input"); @@ -336,11 +339,15 @@ async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope( let worker_id = host.worker_id(); let mut protocol_client = host.connect(); protocol_client - .send(&Method::run_text("first request")) + .send(&Method::submit_text( + protocol::new_submission_request_id(), + "first request", + )) .await?; wait_for_run_end(&mut protocol_client).await?; protocol_client .send(&Method::Notify { + notification_request_id: protocol::new_submission_request_id(), message: "persisted notification".to_string(), auto_run: true, }) @@ -394,7 +401,10 @@ async fn standalone_restore_preserves_history_tasks_notifications_and_cwd_scope( assert!(snapshot.contains("persisted notification"), "{snapshot}"); protocol_client - .send(&Method::run_text("continue after restore")) + .send(&Method::submit_text( + protocol::new_submission_request_id(), + "continue after restore", + )) .await?; wait_for_run_end(&mut protocol_client).await?; let request = second_inspection diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index b1f4fd04..2fecd888 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -102,23 +102,6 @@ struct RollbackSubmitState { turn_before: usize, } -#[derive(Clone)] -pub struct QueuedInput { - segments: Vec, - preview: String, -} - -impl QueuedInput { - fn new(segments: Vec) -> Self { - let preview = Segment::flatten_to_text(&segments); - Self { segments, preview } - } - - pub fn preview(&self) -> &str { - &self.preview - } -} - struct ComposerInputHistory { entries: VecDeque>, browse: Option, @@ -272,7 +255,7 @@ pub struct App { /// Current transient actionbar notice. Notices are local UI state only: /// they are never appended to transcript/session history or LLM context. actionbar_notice: Option, - /// Normal composer input that is submitted as `Method::Run`. + /// Normal composer input that is submitted as `Method::Submit`. pub input: InputBuffer, /// Separate command-line input. It is never submitted as a user message. pub command_input: InputBuffer, @@ -333,9 +316,8 @@ pub struct App { /// Top entry index of the task pane's visible window. Clamped on /// render so it never points past the end of the list. pub task_pane_scroll: usize, - /// TUI-local FIFO of user inputs submitted while the Worker is already running. - /// Entries have not been sent to the Worker yet, so they remain editable/cancellable locally. - queued_inputs: VecDeque, + /// Authoritative WorkerSession FIFO summary received from snapshot/live events. + pending_submissions: protocol::PendingSubmissionsSnapshot, /// TUI-local readline-style composer input history. This is intentionally /// client-side only: recalled entries are plain drafts until submitted again. input_history: ComposerInputHistory, @@ -395,7 +377,7 @@ impl App { text_selection: TextSelectionState::default(), task_pane_open: false, task_pane_scroll: 0, - queued_inputs: VecDeque::new(), + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), input_history: ComposerInputHistory::new(), input_history_store: None, pending_submit_rollback: None, @@ -768,18 +750,12 @@ impl App { return None; } self.record_input_history(segments.clone()); - if self.running { - self.queued_inputs.push_back(QueuedInput::new(segments)); - self.input.clear(); - self.completion = None; - return None; - } self.input.clear(); Some(self.method_for_run(segments)) } pub fn restore_unsent_run(&mut self, method: &Method) { - let Method::Run { input } = method else { + let Method::Submit { input, .. } = method else { return; }; self.pending_submit_rollback = None; @@ -787,8 +763,9 @@ impl App { self.input.replace_with_segments(input); self.completion = None; } else { - self.queued_inputs - .push_front(QueuedInput::new(input.clone())); + self.push_error( + "Submit transport failed; current Composer was preserved and the unsent input was not queued.", + ); } } @@ -804,7 +781,10 @@ impl App { block_start: self.blocks.len(), turn_before: self.turn_index, }); - Method::Run { input: segments } + Method::Submit { + submission_request_id: protocol::new_submission_request_id(), + input: segments, + } } fn record_input_history(&mut self, segments: Vec) { @@ -825,7 +805,7 @@ impl App { } pub fn queued_input_count(&self) -> usize { - self.queued_inputs.len() + self.pending_submissions.submissions.len() } #[cfg(test)] @@ -911,35 +891,10 @@ impl App { } pub fn next_queued_input_preview(&self) -> Option<&str> { - self.queued_inputs.front().map(QueuedInput::preview) - } - - pub fn clear_queued_inputs(&mut self) -> usize { - let cleared = self.queued_inputs.len(); - self.queued_inputs.clear(); - cleared - } - - pub fn restore_next_queued_input_to_composer(&mut self) -> bool { - if self.queued_inputs.is_empty() { - return false; - } - if !self.input.is_empty() { - self.push_error("Composer is not empty; clear it before editing queued input."); - return false; - } - let Some(queued) = self.queued_inputs.pop_front() else { - return false; - }; - self.input_history.cancel_browse(); - self.input.replace_with_segments(&queued.segments); - self.completion = None; - true - } - - fn pop_next_queued_run(&mut self) -> Option { - let queued = self.queued_inputs.pop_front()?; - Some(self.method_for_run(queued.segments)) + self.pending_submissions + .submissions + .first() + .map(|submission| submission.submission_id.as_str()) } pub fn clear_actionbar_notice(&mut self) { @@ -1123,6 +1078,11 @@ impl App { } match event { + Event::SubmissionAccepted { .. } => {} + Event::SubmissionRejected { message, .. } => self.push_error(message), + Event::PendingSubmissionsChanged { pending } => { + self.pending_submissions = pending; + } Event::UserMessage { segments } => { self.turn_index += 1; self.blocks.push(Block::TurnHeader { @@ -1372,9 +1332,6 @@ impl App { WorkerStatus::Idle } }); - if matches!(result, RunResult::Finished | RunResult::LimitReached) { - return self.pop_next_queued_run(); - } } } Event::CompactStart { .. } => { @@ -1449,6 +1406,7 @@ impl App { internal_workers, } => { self.rewind_refresh_fence = false; + self.pending_submissions = session.pending_submissions.clone(); self.restore_snapshot(&session, greeting, in_flight); self.replace_internal_worker_snapshots(internal_workers); self.set_worker_status(status); @@ -2681,7 +2639,10 @@ mod rewind_refresh_tests { }); app.handle_worker_event(Event::RewindApplied { - session: protocol::SessionSnapshot { entries: vec![] }, + session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries: vec![], + }, input: vec![Segment::text("selected rewind input")], summary: summary(3), }); @@ -2700,7 +2661,10 @@ mod rewind_refresh_tests { }); app.handle_worker_event(Event::RewindApplied { - session: protocol::SessionSnapshot { entries: vec![] }, + session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries: vec![], + }, input: vec![Segment::text("rewound input")], summary: summary(1), }); @@ -2743,7 +2707,10 @@ mod rewind_refresh_tests { }); app.handle_worker_event(Event::RewindApplied { - session: protocol::SessionSnapshot { entries: vec![] }, + session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries: vec![], + }, input: vec![Segment::text("rewound input")], summary: summary(2), }); @@ -2877,7 +2844,7 @@ mod composer_history_persistence_tests { path: "src/lib.rs".into(), }, ]); - assert!(matches!(app.submit_input(), Some(Method::Run { .. }))); + assert!(matches!(app.submit_input(), Some(Method::Submit { .. }))); let mut reloaded = App::new_with_input_history_store("test".into(), store); assert!(reloaded.browse_input_history_older()); @@ -2958,7 +2925,7 @@ mod composer_history_persistence_tests { app.insert_char(c); } match app.submit_input() { - Some(Method::Run { input }) => input, + Some(Method::Submit { input, .. }) => input, other => panic!("expected Run, got {other:?}"), } } @@ -3424,72 +3391,43 @@ mod completion_flow_tests { } #[test] - fn running_submit_is_queued_locally_and_clears_composer() { + fn running_submit_is_sent_to_the_worker_and_not_queued_locally() { let mut app = App::new("test".into()); app.set_worker_status(WorkerStatus::Running); insert_text(&mut app, "queued turn"); - assert!(app.submit_input().is_none()); + let method = app.submit_input(); - assert_eq!(app.queued_input_count(), 1); - assert_eq!(app.next_queued_input_preview(), Some("queued turn")); + assert!(matches!(method, Some(Method::Submit { .. }))); + assert_eq!(app.queued_input_count(), 0); assert_eq!(input_text(&app), ""); } #[test] - fn finished_run_auto_sends_next_queued_input() { + fn pending_submission_projection_is_worker_authoritative() { let mut app = App::new("test".into()); - app.set_worker_status(WorkerStatus::Running); - insert_text(&mut app, "next turn"); - assert!(app.submit_input().is_none()); - - let method = app.handle_worker_event(Event::RunEnd { - result: RunResult::Finished, + app.handle_worker_event(Event::PendingSubmissionsChanged { + pending: protocol::PendingSubmissionsSnapshot { + revision: 3, + notification_count: 0, + submissions: vec![protocol::PendingSubmissionSummary { + submission_id: "submission-1".into(), + accepted_at_ms: 7, + segment_count: 2, + byte_len: 42, + }], + }, }); - match method { - Some(Method::Run { input }) => { - assert_eq!(Segment::flatten_to_text(&input), "next turn"); - } - other => panic!("expected queued Run, got {other:?}"), - } - assert_eq!(app.queued_input_count(), 0); - } - - #[test] - fn limit_reached_run_auto_sends_next_queued_input() { - let mut app = App::new("test".into()); - app.set_worker_status(WorkerStatus::Running); - insert_text(&mut app, "next after limit"); - assert!(app.submit_input().is_none()); - - let method = app.handle_worker_event(Event::RunEnd { - result: RunResult::LimitReached, - }); - - match method { - Some(Method::Run { input }) => { - assert_eq!(Segment::flatten_to_text(&input), "next after limit"); - } - other => panic!("expected queued Run, got {other:?}"), - } - assert_eq!(app.queued_input_count(), 0); - } - - #[test] - fn paused_and_rolled_back_run_do_not_auto_send_queue() { - for result in [RunResult::Paused, RunResult::RolledBack] { - let mut app = App::new("test".into()); - app.set_worker_status(WorkerStatus::Running); - insert_text(&mut app, "held turn"); - assert!(app.submit_input().is_none()); - - let method = app.handle_worker_event(Event::RunEnd { result }); - - assert!(method.is_none()); - assert_eq!(app.queued_input_count(), 1); - assert_eq!(app.next_queued_input_preview(), Some("held turn")); - } + assert_eq!(app.queued_input_count(), 1); + assert_eq!(app.next_queued_input_preview(), Some("submission-1")); + assert!( + app.handle_worker_event(Event::RunEnd { + result: RunResult::Finished, + }) + .is_none() + ); + assert_eq!(app.queued_input_count(), 1); } #[test] @@ -3501,24 +3439,6 @@ mod completion_flow_tests { assert_eq!(app.queued_input_count(), 0); } - #[test] - fn queued_input_can_be_restored_to_composer_or_cleared() { - let mut app = App::new("test".into()); - app.set_worker_status(WorkerStatus::Running); - insert_text(&mut app, "edit me"); - assert!(app.submit_input().is_none()); - - assert!(app.restore_next_queued_input_to_composer()); - assert_eq!(app.queued_input_count(), 0); - assert_eq!(input_text(&app), "edit me"); - - app.input.clear(); - insert_text(&mut app, "clear me"); - assert!(app.submit_input().is_none()); - assert_eq!(app.clear_queued_inputs(), 1); - assert_eq!(app.queued_input_count(), 0); - } - fn insert_text(app: &mut App, text: &str) { for c in text.chars() { app.insert_char(c); @@ -3530,7 +3450,7 @@ mod completion_flow_tests { app.insert_char(c); } match app.submit_input() { - Some(Method::Run { input }) => input, + Some(Method::Submit { input, .. }) => input, other => panic!("expected Run, got {other:?}"), } } @@ -3675,6 +3595,7 @@ mod completion_flow_tests { app.handle_worker_event(Event::Snapshot { greeting: test_greeting(), session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, status: WorkerStatus::Running, @@ -3783,6 +3704,7 @@ mod completion_flow_tests { revision, status: WorkerStatus::Idle, session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, in_flight: protocol::InFlightSnapshot::default(), @@ -4000,6 +3922,7 @@ mod completion_flow_tests { app.handle_worker_event(Event::Snapshot { greeting: test_greeting(), session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, status: WorkerStatus::Idle, @@ -4051,6 +3974,7 @@ mod completion_flow_tests { app.handle_worker_event(Event::Snapshot { greeting: test_greeting(), session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, status: WorkerStatus::Idle, @@ -4064,6 +3988,7 @@ mod completion_flow_tests { }, revision: 4, session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, status: WorkerStatus::Running, @@ -4222,6 +4147,7 @@ mod completion_flow_tests { app.handle_worker_event(Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting, @@ -4437,23 +4363,23 @@ mod completion_flow_tests { } #[test] - fn input_history_records_queued_inputs_and_suppresses_consecutive_duplicates() { + fn input_history_records_running_submits_and_suppresses_consecutive_duplicates() { let mut app = App::new("test".into()); app.running = true; for c in "repeat".chars() { app.insert_char(c); } - assert!(app.submit_input().is_none()); + assert!(app.submit_input().is_some()); assert_eq!(app.input_history_len(), 1); - assert_eq!(app.queued_input_count(), 1); + assert_eq!(app.queued_input_count(), 0); for c in "repeat".chars() { app.insert_char(c); } - assert!(app.submit_input().is_none()); + assert!(app.submit_input().is_some()); assert_eq!(app.input_history_len(), 1); - assert_eq!(app.queued_input_count(), 2); + assert_eq!(app.queued_input_count(), 0); app.insert_char(' '); assert!(app.submit_input().is_none()); @@ -4481,7 +4407,7 @@ mod completion_flow_tests { }, ]; app.input.replace_with_segments(&original); - assert!(matches!(app.submit_input(), Some(Method::Run { .. }))); + assert!(matches!(app.submit_input(), Some(Method::Submit { .. }))); assert!(app.browse_input_history_older()); assert_eq!(app.input.submit_segments(), original); @@ -4493,7 +4419,7 @@ mod completion_flow_tests { for c in "sent".chars() { app.insert_char(c); } - assert!(matches!(app.submit_input(), Some(Method::Run { .. }))); + assert!(matches!(app.submit_input(), Some(Method::Submit { .. }))); for c in "draft".chars() { app.insert_char(c); @@ -4511,7 +4437,7 @@ mod completion_flow_tests { for c in "sent".chars() { app.insert_char(c); } - assert!(matches!(app.submit_input(), Some(Method::Run { .. }))); + assert!(matches!(app.submit_input(), Some(Method::Submit { .. }))); assert!(app.browse_input_history_older()); assert!(app.input_history_is_browsing()); @@ -4528,17 +4454,19 @@ mod completion_flow_tests { for c in "first".chars() { app.insert_char(c); } - assert!(matches!(app.submit_input(), Some(Method::Run { .. }))); + assert!(matches!(app.submit_input(), Some(Method::Submit { .. }))); for c in "second".chars() { app.insert_char(c); } - assert!(matches!(app.submit_input(), Some(Method::Run { .. }))); + assert!(matches!(app.submit_input(), Some(Method::Submit { .. }))); assert!(app.browse_input_history_older()); assert!(app.browse_input_history_older()); let method = app.submit_input(); match method { - Some(Method::Run { input }) => assert_eq!(Segment::flatten_to_text(&input), "first"), + Some(Method::Submit { input, .. }) => { + assert_eq!(Segment::flatten_to_text(&input), "first") + } other => panic!("expected recalled run, got {other:?}"), } assert_eq!(app.input_history_len(), 3); diff --git a/crates/tui/src/console/mod.rs b/crates/tui/src/console/mod.rs index 0106b403..3f0d6ddd 100644 --- a/crates/tui/src/console/mod.rs +++ b/crates/tui/src/console/mod.rs @@ -270,8 +270,8 @@ impl ConsoleConnection { async fn send(&mut self, method: &Method) -> Result<(), Box> { let mut prepared = method.clone(); let carries_attachments = - matches!(prepared, Method::Run { .. }) && !self.pending_attachments.is_empty(); - if let Method::Run { input } = &mut prepared { + matches!(prepared, Method::Submit { .. }) && !self.pending_attachments.is_empty(); + if let Method::Submit { input, .. } = &mut prepared { input.extend( self.pending_attachments .iter() @@ -569,6 +569,7 @@ async fn run_e2e_rewind_fixture( app.connected = true; app.handle_worker_event(Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, status: WorkerStatus::Idle, @@ -697,6 +698,7 @@ async fn run_e2e_rewind_fixture( if submitted_at.elapsed() >= apply_delay { app.handle_worker_event(Event::RewindApplied { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, input: vec![Segment::text("rewind-live-refresh")], @@ -916,7 +918,7 @@ async fn run_loop( } fn attachment_command_path(method: &Method) -> Option { - let Method::Run { input } = method else { + let Method::Submit { input, .. } = method else { return None; }; let [Segment::Text { content }] = input.as_slice() else { @@ -927,7 +929,7 @@ fn attachment_command_path(method: &Method) -> Option { } fn is_clear_attachments_command(method: &Method) -> bool { - let Method::Run { input } = method else { + let Method::Submit { input, .. } = method else { return false; }; matches!( @@ -941,7 +943,7 @@ async fn send_console_method( client: &mut ConsoleConnection, method: &Method, ) -> Result<(), Box> { - if matches!(method, Method::Run { .. }) && client.has_active_uploads() { + if matches!(method, Method::Submit { .. }) && client.has_active_uploads() { app.restore_unsent_run(method); app.flash_actionbar_notice( "Attachment upload is still in progress; wait or use /clear-attachments.", @@ -953,7 +955,7 @@ async fn send_console_method( } let sends_attachments = - matches!(method, Method::Run { .. }) && !client.pending_attachments.is_empty(); + matches!(method, Method::Submit { .. }) && !client.pending_attachments.is_empty(); if let Err(error) = client.send(method).await { if sends_attachments { app.restore_unsent_run(method); @@ -1151,15 +1153,10 @@ fn handle_key(app: &mut App, key: KeyEvent) -> Option { KeyCode::Char(c) if c.eq_ignore_ascii_case(&'q') && alt && !ctrl && !app.is_command_mode() => { - if app.restore_next_queued_input_to_composer() { - Some(app.refresh_completion()) - } else { - Some(None) - } + Some(Some(Method::ContinuePending)) } KeyCode::Char(c) if c.eq_ignore_ascii_case(&'c') && alt && !ctrl => { - app.clear_queued_inputs(); - Some(None) + Some(Some(Method::ClearPendingSubmissions)) } KeyCode::Char('c') if ctrl => Some(handle_pause_or_quit(app)), KeyCode::Char('x') if ctrl => Some(handle_cancel_or_shutdown(app)), @@ -1427,7 +1424,6 @@ fn handle_cancel_or_shutdown(app: &mut App) -> Option { WorkerStatus::Running | WorkerStatus::Paused ) { app.shutdown_confirm = None; - app.clear_queued_inputs(); return Some(Method::Cancel); } if let Some(pressed_at) = app.shutdown_confirm @@ -1450,7 +1446,6 @@ fn handle_cancel_or_shutdown(app: &mut App) -> Option { /// Idle / Paused → 2-tap to quit the TUI (the Worker keeps running). fn handle_pause_or_quit(app: &mut App) -> Option { if app.worker_status == WorkerStatus::Running { - app.clear_queued_inputs(); return Some(Method::Pause); } if let Some(t) = app.quit_confirm @@ -1476,8 +1471,8 @@ mod tests { use crate::text_selection::{HistoryViewport, SelectionRow}; use async_trait::async_trait; use protocol::{ - Event, RewindTarget, RewindTargetId, RunResult, Segment, UploadedFileAvailability, - UploadedFileRef, WorkerStatus, + Event, RewindTarget, RewindTargetId, Segment, UploadedFileAvailability, UploadedFileRef, + WorkerStatus, }; #[test] @@ -1490,7 +1485,8 @@ mod tests { #[test] fn client_local_attachment_commands_are_typed_and_do_not_send_the_path() { - let attach = Method::Run { + let attach = Method::Submit { + submission_request_id: protocol::new_submission_request_id(), input: vec![Segment::text("/attach /tmp/report.md")], }; assert_eq!( @@ -1499,7 +1495,8 @@ mod tests { ); assert!(!is_clear_attachments_command(&attach)); - let clear = Method::Run { + let clear = Method::Submit { + submission_request_id: protocol::new_submission_request_id(), input: vec![Segment::text("/clear-attachments")], }; assert!(is_clear_attachments_command(&clear)); @@ -1605,7 +1602,7 @@ mod tests { } #[tokio::test] - async fn queued_attachment_send_failure_restores_draft_without_exiting_console() { + async fn running_attachment_submit_failure_restores_draft_without_exiting_console() { let file = UploadedFileRef { artifact_id: "artifact-queued".into(), file_name: "queued.txt".into(), @@ -1631,13 +1628,10 @@ mod tests { let mut app = App::new("worker".into()); app.set_worker_status(WorkerStatus::Running); app.input.insert_str("queued inspect"); - assert!(app.submit_input().is_none()); - let method = app - .handle_worker_event(Event::RunEnd { - result: RunResult::Finished, - }) - .expect("queued run must be released"); + .submit_input() + .expect("running Submit is sent immediately"); + send_console_method(&mut app, &mut connection, &method) .await .unwrap(); @@ -1960,7 +1954,7 @@ mod tests { } #[test] - fn running_enter_queues_instead_of_sending_run() { + fn running_enter_sends_submit_to_worker() { let mut app = App::new("agent".to_string()); app.set_worker_status(WorkerStatus::Running); for c in "queued".chars() { @@ -1973,102 +1967,80 @@ mod tests { ); } - assert!(handle_key(&mut app, KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)).is_none()); + assert!(matches!( + handle_key(&mut app, KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), + Some(Method::Submit { .. }) + )); - assert_eq!(app.queued_input_count(), 1); - assert_eq!(app.next_queued_input_preview(), Some("queued")); + assert_eq!(app.queued_input_count(), 0); assert_eq!(input_text(&app), ""); } #[test] - fn queued_input_keybindings_restore_and_clear() { - let mut app = App::new("agent".to_string()); - app.set_worker_status(WorkerStatus::Running); - for c in "edit queued".chars() { - assert!( - handle_key( - &mut app, - KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE) - ) - .is_none() - ); - } - assert!(handle_key(&mut app, KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)).is_none()); + fn pending_queue_shortcuts_send_worker_operations() { + let mut app = App::new("test".into()); + app.handle_worker_event(Event::PendingSubmissionsChanged { + pending: protocol::PendingSubmissionsSnapshot { + revision: 2, + notification_count: 0, + submissions: vec![protocol::PendingSubmissionSummary { + submission_id: "submission-1".into(), + accepted_at_ms: 1, + segment_count: 1, + byte_len: 6, + }], + }, + }); - assert!( - handle_key( - &mut app, - KeyEvent::new(KeyCode::Char('q'), KeyModifiers::ALT) - ) - .is_none() + let continue_next = handle_key( + &mut app, + KeyEvent::new(KeyCode::Char('q'), KeyModifiers::ALT), ); - assert_eq!(app.queued_input_count(), 0); - assert_eq!(input_text(&app), "edit queued"); - - app.input.clear(); - for c in "clear queued".chars() { - assert!( - handle_key( - &mut app, - KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE) - ) - .is_none() - ); - } - assert!(handle_key(&mut app, KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)).is_none()); + assert!(matches!(continue_next, Some(Method::ContinuePending))); assert_eq!(app.queued_input_count(), 1); - assert!( - handle_key( - &mut app, - KeyEvent::new(KeyCode::Char('c'), KeyModifiers::ALT) - ) - .is_none() + let clear = handle_key( + &mut app, + KeyEvent::new(KeyCode::Char('c'), KeyModifiers::ALT), ); - assert_eq!(app.queued_input_count(), 0); + assert!(matches!(clear, Some(Method::ClearPendingSubmissions))); + assert_eq!(app.queued_input_count(), 1); } #[test] - fn pause_and_cancel_clear_queued_input() { - let mut app = App::new("agent".to_string()); + fn pause_and_cancel_preserve_authoritative_pending_queue() { + let mut app = App::new("test".into()); + app.handle_worker_event(Event::PendingSubmissionsChanged { + pending: protocol::PendingSubmissionsSnapshot { + revision: 2, + notification_count: 0, + submissions: vec![protocol::PendingSubmissionSummary { + submission_id: "submission-1".into(), + accepted_at_ms: 1, + segment_count: 1, + byte_len: 6, + }], + }, + }); app.set_worker_status(WorkerStatus::Running); - for c in "queued".chars() { - assert!( - handle_key( - &mut app, - KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE) - ) - .is_none() - ); - } - assert!(handle_key(&mut app, KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)).is_none()); + assert!(matches!( + handle_key( + &mut app, + KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL), + ), + Some(Method::Pause) + )); assert_eq!(app.queued_input_count(), 1); - let pause = handle_key( - &mut app, - KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL), - ); - assert!(matches!(pause, Some(Method::Pause))); - assert_eq!(app.queued_input_count(), 0); - - for c in "queued again".chars() { - assert!( - handle_key( - &mut app, - KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE) - ) - .is_none() - ); - } - assert!(handle_key(&mut app, KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)).is_none()); + app.set_worker_status(WorkerStatus::Running); + assert!(matches!( + handle_key( + &mut app, + KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL), + ), + Some(Method::Cancel) + )); assert_eq!(app.queued_input_count(), 1); - - let cancel = handle_key( - &mut app, - KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL), - ); - assert!(matches!(cancel, Some(Method::Cancel))); - assert_eq!(app.queued_input_count(), 0); } #[test] @@ -2535,13 +2507,19 @@ mod tests { let mut app = App::new("agent".to_string()); app.handle_worker_event(Event::Snapshot { greeting: test_greeting(), - session: protocol::SessionSnapshot { entries: vec![] }, + session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries: vec![], + }, status: WorkerStatus::Idle, in_flight: Default::default(), internal_workers: Vec::new(), }); app.handle_worker_event(Event::RewindApplied { - session: protocol::SessionSnapshot { entries: vec![] }, + session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries: vec![], + }, input: vec![Segment::Text { content: "retry this".into(), }], @@ -2562,7 +2540,10 @@ mod tests { let mut app = App::new("agent".to_string()); app.handle_worker_event(Event::Snapshot { greeting: test_greeting(), - session: protocol::SessionSnapshot { entries: vec![] }, + session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries: vec![], + }, status: WorkerStatus::Idle, in_flight: Default::default(), internal_workers: Vec::new(), @@ -2570,7 +2551,10 @@ mod tests { type_keys(&mut app, "draft"); app.handle_worker_event(Event::RewindApplied { - session: protocol::SessionSnapshot { entries: vec![] }, + session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries: vec![], + }, input: vec![Segment::Text { content: "retry this".into(), }], @@ -2918,12 +2902,12 @@ mod tests { type_keys(&mut app, "first"); assert!(matches!( handle_key(&mut app, key(KeyCode::Enter)), - Some(Method::Run { .. }) + Some(Method::Submit { .. }) )); type_keys(&mut app, "second"); assert!(matches!( handle_key(&mut app, key(KeyCode::Enter)), - Some(Method::Run { .. }) + Some(Method::Submit { .. }) )); assert_eq!(input_text(&app), ""); @@ -2954,7 +2938,7 @@ mod tests { type_keys(&mut app, "sent"); assert!(matches!( handle_key(&mut app, key(KeyCode::Enter)), - Some(Method::Run { .. }) + Some(Method::Submit { .. }) )); type_keys(&mut app, "draft\nbody"); app.move_cursor_start(); diff --git a/crates/tui/src/ui.rs b/crates/tui/src/ui.rs index 35b4fcbc..577fafe9 100644 --- a/crates/tui/src/ui.rs +++ b/crates/tui/src/ui.rs @@ -1880,7 +1880,7 @@ fn actionbar_left_item(app: &App, now: Instant) -> Option<(String, Style)> { } if app.queued_input_count() > 0 { return Some(( - "Alt-q edit queued Alt-c clear queued".to_string(), + "Alt-q continue queued Alt-c clear queued".to_string(), Style::default().fg(Color::DarkGray), )); } @@ -2136,9 +2136,24 @@ mod tests { use super::*; use crate::app::{ActionbarNoticeLevel, ActionbarNoticeSource, App}; use crate::block::{ToolCallBlock, ToolCallState}; - use protocol::WorkerStatus; + use protocol::Event; use std::time::{Duration, Instant}; + fn set_pending_submission(app: &mut App, id: &str) { + app.handle_worker_event(Event::PendingSubmissionsChanged { + pending: protocol::PendingSubmissionsSnapshot { + revision: 1, + notification_count: 0, + submissions: vec![protocol::PendingSubmissionSummary { + submission_id: id.into(), + accepted_at_ms: 1, + segment_count: 1, + byte_len: 1, + }], + }, + }); + } + #[test] fn run_status_line_matches_console_metrics_and_spinner_frame() { let now = Instant::now(); @@ -2251,15 +2266,11 @@ mod tests { #[test] fn queue_status_text_includes_count_and_preview() { let mut app = App::new("test".into()); - app.set_worker_status(WorkerStatus::Running); - for c in "queued preview".chars() { - app.insert_char(c); - } - assert!(app.submit_input().is_none()); + set_pending_submission(&mut app, "submission-1"); assert_eq!( queue_status_text(&app), - Some("queued: 1 — queued preview".to_string()) + Some("queued: 1 — submission-1".to_string()) ); } @@ -2289,14 +2300,10 @@ mod tests { Some("Worker keeps running. Press Ctrl-C again to exit TUI.".into()) ); - app.set_worker_status(WorkerStatus::Running); - for c in "queued turn".chars() { - app.insert_char(c); - } - assert!(app.submit_input().is_none()); + set_pending_submission(&mut app, "submission-1"); assert_eq!( actionbar_left_item(&app, now).map(|(text, _)| text), - Some("Alt-q edit queued Alt-c clear queued".into()) + Some("Alt-q continue queued Alt-c clear queued".into()) ); app.enter_command_mode(); diff --git a/crates/worker-runtime/src/execution.rs b/crates/worker-runtime/src/execution.rs index 47660977..c85e9f29 100644 --- a/crates/worker-runtime/src/execution.rs +++ b/crates/worker-runtime/src/execution.rs @@ -41,14 +41,12 @@ pub enum WorkerExecutionOperation { Cancel, } -/// Evidence that a user input reached the durable Worker session boundary. -/// -/// This is intentionally distinct from accepting a method on the Worker's -/// in-memory channel. For Flow submissions, the committed UserInput entry also -/// carries the initial Flow runtime-state extension. +/// Evidence that a Submit request reached the durable Worker session boundary. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct WorkerInputCommitAck { +pub struct WorkerSubmissionAck { + pub submission_request_id: String, pub submission_id: String, + pub disposition: protocol::SubmissionDisposition, } /// Typed execution result class. Results are transient operation outcomes and @@ -61,7 +59,7 @@ pub struct WorkerExecutionResult { #[serde(default, skip_serializing_if = "Option::is_none")] pub message: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub input_commit: Option, + pub submission: Option, } /// Backend result class for a Worker execution operation. @@ -85,22 +83,26 @@ impl WorkerExecutionResult { outcome: WorkerExecutionOutcome::Accepted, run_state, message: None, - input_commit: None, + submission: None, } } - pub fn accepted_input_committed( + pub fn accepted_submission( operation: WorkerExecutionOperation, run_state: WorkerExecutionRunState, + submission_request_id: impl Into, submission_id: impl Into, + disposition: protocol::SubmissionDisposition, ) -> Self { Self { operation, outcome: WorkerExecutionOutcome::Accepted, run_state, message: None, - input_commit: Some(WorkerInputCommitAck { + submission: Some(WorkerSubmissionAck { + submission_request_id: submission_request_id.into(), submission_id: submission_id.into(), + disposition, }), } } @@ -111,7 +113,7 @@ impl WorkerExecutionResult { outcome: WorkerExecutionOutcome::Busy, run_state: WorkerExecutionRunState::Busy, message: Some(message.into()), - input_commit: None, + submission: None, } } @@ -121,7 +123,7 @@ impl WorkerExecutionResult { outcome: WorkerExecutionOutcome::Rejected, run_state: WorkerExecutionRunState::Stopped, message: Some(message.into()), - input_commit: None, + submission: None, } } @@ -131,7 +133,7 @@ impl WorkerExecutionResult { outcome: WorkerExecutionOutcome::Errored, run_state: WorkerExecutionRunState::Errored, message: Some(message.into()), - input_commit: None, + submission: None, } } @@ -141,7 +143,7 @@ impl WorkerExecutionResult { outcome: WorkerExecutionOutcome::Unsupported, run_state: WorkerExecutionRunState::Stopped, message: Some(message.into()), - input_commit: None, + submission: None, } } @@ -618,14 +620,17 @@ mod tests { use super::*; #[test] - fn input_commit_ack_survives_json_round_trip() { - let result = WorkerExecutionResult::accepted_input_committed( + fn submission_ack_survives_json_round_trip() { + let result = WorkerExecutionResult::accepted_submission( WorkerExecutionOperation::Input, WorkerExecutionRunState::Busy, + "request-1", "submission-1", + protocol::SubmissionDisposition::Started, ); let json = serde_json::to_string(&result).unwrap(); + assert!(json.contains("\"submission_request_id\":\"request-1\"")); assert!(json.contains("\"submission_id\":\"submission-1\"")); assert_eq!( serde_json::from_str::(&json).unwrap(), diff --git a/crates/worker-runtime/src/http_server.rs b/crates/worker-runtime/src/http_server.rs index 2217e973..575df269 100644 --- a/crates/worker-runtime/src/http_server.rs +++ b/crates/worker-runtime/src/http_server.rs @@ -2735,11 +2735,13 @@ mod tests { _handle: &WorkerExecutionHandle, input: WorkerInput, ) -> WorkerExecutionResult { - if let Some(submission_id) = input.submission_id { - WorkerExecutionResult::accepted_input_committed( + if let Some(submission_id) = input.submission_request_id { + WorkerExecutionResult::accepted_submission( WorkerExecutionOperation::Input, WorkerExecutionRunState::Idle, + submission_id.clone(), submission_id, + protocol::SubmissionDisposition::Started, ) } else { WorkerExecutionResult::accepted( @@ -3059,11 +3061,13 @@ mod ws_tests { _handle: &WorkerExecutionHandle, input: WorkerInput, ) -> WorkerExecutionResult { - if let Some(submission_id) = input.submission_id { - WorkerExecutionResult::accepted_input_committed( + if let Some(submission_id) = input.submission_request_id { + WorkerExecutionResult::accepted_submission( WorkerExecutionOperation::Input, WorkerExecutionRunState::Idle, + submission_id.clone(), submission_id, + protocol::SubmissionDisposition::Started, ) } else { WorkerExecutionResult::accepted( diff --git a/crates/worker-runtime/src/interaction.rs b/crates/worker-runtime/src/interaction.rs index d76c675d..52e0168e 100644 --- a/crates/worker-runtime/src/interaction.rs +++ b/crates/worker-runtime/src/interaction.rs @@ -25,10 +25,10 @@ impl WorkerInputKind { pub struct WorkerInput { pub kind: WorkerInputKind, pub content: String, - /// Runtime-generated correlation id. This is never accepted from public - /// JSON input and is consumed only by the execution backend. - #[serde(skip)] - pub submission_id: Option, + /// Authenticated client-generated idempotency key. Runtime generates one + /// only for trusted internal callers that omit it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub submission_request_id: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub segments: Option>, } @@ -38,7 +38,7 @@ impl WorkerInput { Self { kind: WorkerInputKind::User, content: content.into(), - submission_id: None, + submission_request_id: None, segments: None, } } @@ -47,7 +47,7 @@ impl WorkerInput { Self { kind: WorkerInputKind::Notify, content: content.into(), - submission_id: None, + submission_request_id: None, segments: None, } } @@ -57,6 +57,21 @@ impl WorkerInput { mod tests { use super::WorkerInput; + #[test] + fn submission_request_id_round_trips_for_authenticated_client_retry() { + let input: WorkerInput = serde_json::from_value(serde_json::json!({ + "kind": "user", + "content": "message", + "submission_request_id": "request-1" + })) + .unwrap(); + assert_eq!(input.submission_request_id.as_deref(), Some("request-1")); + assert_eq!( + serde_json::to_value(input).unwrap()["submission_request_id"], + "request-1" + ); + } + #[test] fn notify_is_an_operation_and_legacy_system_kind_is_rejected() { assert_eq!( @@ -78,4 +93,7 @@ mod tests { pub struct WorkerInteractionAck { pub worker_ref: WorkerRef, pub status: WorkerStatus, + /// Present for User Submit and absent for non-Submit interactions. + #[serde(skip_serializing_if = "Option::is_none")] + pub submission: Option, } diff --git a/crates/worker-runtime/src/runtime.rs b/crates/worker-runtime/src/runtime.rs index fd5a0b16..9701aae1 100644 --- a/crates/worker-runtime/src/runtime.rs +++ b/crates/worker-runtime/src/runtime.rs @@ -748,8 +748,12 @@ impl Runtime { let state = self.lock()?; state.worker(&worker_ref)?.request.initial_input.clone() } { - let expected_submission_id = Uuid::now_v7().to_string(); - initial_input.submission_id = Some(expected_submission_id.clone()); + let expected_submission_id = initial_input + .submission_request_id + .clone() + .filter(|request_id| !request_id.trim().is_empty()) + .unwrap_or_else(|| Uuid::now_v7().to_string()); + initial_input.submission_request_id = Some(expected_submission_id.clone()); let dispatch_result = backend.dispatch_input(&handle, initial_input.clone()); if !dispatch_result.is_accepted() { let _ = backend.stop_worker(&handle); @@ -763,9 +767,9 @@ impl Runtime { }); } let has_commit_ack = dispatch_result - .input_commit + .submission .as_ref() - .is_some_and(|ack| ack.submission_id == expected_submission_id); + .is_some_and(|ack| ack.submission_request_id == expected_submission_id); if !has_commit_ack { let _ = backend.stop_worker(&handle); self.rollback_failed_create(&worker_ref)?; @@ -1146,13 +1150,18 @@ impl Runtime { mut input: WorkerInput, ) -> Result { validate_worker_input(&input)?; - let expected_submission_id = if input.kind == WorkerInputKind::User { - let submission_id = Uuid::now_v7().to_string(); - input.submission_id = Some(submission_id.clone()); - Some(submission_id) - } else { - None - }; + let expected_submission_id = + if matches!(input.kind, WorkerInputKind::User | WorkerInputKind::Notify) { + let submission_id = input + .submission_request_id + .clone() + .filter(|request_id| !request_id.trim().is_empty()) + .unwrap_or_else(|| Uuid::now_v7().to_string()); + input.submission_request_id = Some(submission_id.clone()); + Some(submission_id) + } else { + None + }; self.ensure_worker_execution(worker_ref)?; let (backend, handle) = { let state = self.lock()?; @@ -1191,13 +1200,13 @@ impl Runtime { } if let Some(expected_submission_id) = expected_submission_id && dispatch_result - .input_commit + .submission .as_ref() - .is_none_or(|ack| ack.submission_id != expected_submission_id) + .is_none_or(|ack| ack.submission_request_id != expected_submission_id) { let result = WorkerExecutionResult::rejected( WorkerExecutionOperation::Input, - "execution backend did not acknowledge the committed Runtime submission id", + "execution backend did not acknowledge the committed Runtime submission request id", ); self.record_execution_result(worker_ref, result.clone())?; return Err(RuntimeError::WorkerExecutionRejected { @@ -1209,6 +1218,7 @@ impl Runtime { }); } + let submission = dispatch_result.submission.clone(); let mut state = self.lock()?; state.ensure_running()?; let worker = state.worker_mut(worker_ref)?; @@ -1225,6 +1235,7 @@ impl Runtime { Ok(WorkerInteractionAck { worker_ref: worker_ref.clone(), status, + submission, }) } @@ -1706,6 +1717,7 @@ impl Runtime { } Ok(protocol::Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: protocol::Greeting { @@ -3250,17 +3262,9 @@ fn validate_worker_input(input: &WorkerInput) -> Result<(), RuntimeError> { #[cfg(feature = "ws-server")] fn input_protocol_event(input: &WorkerInput) -> Option { match input.kind { - WorkerInputKind::User => Some(protocol::Event::UserMessage { - segments: input.segments.clone().unwrap_or_else(|| { - vec![protocol::Segment::Text { - content: input.content.clone(), - }] - }), - }), - // The committed `SystemItem::Notification` is the sole agent-visible - // and Console-visible authority for Notify. A synthetic observation - // here would display the same notification twice. - WorkerInputKind::Notify => None, + // Submit is projected only after the Worker commits UserInput. Queued + // payloads must never become model- or client-visible history early. + WorkerInputKind::User | WorkerInputKind::Notify => None, WorkerInputKind::Compact | WorkerInputKind::ListRewindTargets | WorkerInputKind::RegisterPeer => Some(protocol::Event::SystemItem { @@ -3435,6 +3439,7 @@ mod tests { ); let snapshot = protocol::Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: protocol::Greeting { @@ -3475,7 +3480,7 @@ mod tests { let input = WorkerInput { kind: WorkerInputKind::User, content: String::new(), - submission_id: None, + submission_request_id: None, segments: Some(vec![protocol::Segment::Flow { selector: "builtin:coder-review".to_string(), }]), @@ -3488,7 +3493,7 @@ mod tests { let input = WorkerInput { kind: WorkerInputKind::User, content: String::new(), - submission_id: None, + submission_request_id: None, segments: Some(Vec::new()), }; assert!(matches!( @@ -3503,7 +3508,7 @@ mod tests { request.initial_input = Some(WorkerInput { kind: WorkerInputKind::User, content: String::new(), - submission_id: None, + submission_request_id: None, segments: Some(vec![protocol::Segment::Flow { selector: "builtin:coder-review".to_string(), }]), @@ -4005,7 +4010,7 @@ mod tests { _handle: &WorkerExecutionHandle, input: WorkerInput, ) -> WorkerExecutionResult { - let submission_id = input.submission_id.clone(); + let submission_id = input.submission_request_id.clone(); self.dispatched_inputs.lock().unwrap().push(input); let mut result = self .dispatch_result @@ -4013,19 +4018,21 @@ mod tests { .unwrap() .clone() .unwrap_or_else(|| { - WorkerExecutionResult::accepted_input_committed( + WorkerExecutionResult::accepted_submission( WorkerExecutionOperation::Input, WorkerExecutionRunState::Idle, + "request-test", "test-submission", + protocol::SubmissionDisposition::Started, ) }); if !self .preserve_commit_ack_submission_id .load(Ordering::SeqCst) && let (Some(ack), Some(submission_id)) = - (result.input_commit.as_mut(), submission_id) + (result.submission.as_mut(), submission_id) { - ack.submission_id = submission_id; + ack.submission_request_id = submission_id; } result } @@ -4717,10 +4724,12 @@ mod tests { #[test] fn create_worker_uses_committed_input_ack_run_state() { let (runtime, backend) = runtime_and_backend(); - backend.set_dispatch_result(WorkerExecutionResult::accepted_input_committed( + backend.set_dispatch_result(WorkerExecutionResult::accepted_submission( WorkerExecutionOperation::Input, WorkerExecutionRunState::Idle, + "request-test", "test-submission", + protocol::SubmissionDisposition::Started, )); let mut request = task_request("committed initial input is already idle"); request.initial_input = Some(WorkerInput::user("start the ticket")); @@ -4731,13 +4740,15 @@ mod tests { } #[test] - fn create_worker_rejects_mismatched_input_commit_acknowledgement() { + fn create_worker_rejects_mismatched_submission_acknowledgement() { let (runtime, backend) = runtime_and_backend(); backend.preserve_commit_ack_submission_id(); - backend.set_dispatch_result(WorkerExecutionResult::accepted_input_committed( + backend.set_dispatch_result(WorkerExecutionResult::accepted_submission( WorkerExecutionOperation::Input, WorkerExecutionRunState::Busy, + "request-test", "forged-submission", + protocol::SubmissionDisposition::Started, )); let mut request = task_request("mismatched initial input commit ack"); request.initial_input = Some(WorkerInput::user("start the ticket")); @@ -4866,6 +4877,7 @@ mod tests { &detail.worker_ref, protocol::Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: vec![protocol::SessionSnapshotEntry { entry_id: "restored-log-entry".to_owned(), timestamp: 1, @@ -4937,10 +4949,14 @@ mod tests { _handle: &WorkerExecutionHandle, input: WorkerInput, ) -> WorkerExecutionResult { - WorkerExecutionResult::accepted_input_committed( + WorkerExecutionResult::accepted_submission( WorkerExecutionOperation::Input, WorkerExecutionRunState::Idle, - input.submission_id.expect("Runtime submission id"), + "request-test", + input + .submission_request_id + .expect("Runtime submission request id"), + protocol::SubmissionDisposition::Started, ) } } @@ -5030,7 +5046,7 @@ mod tests { request.initial_input = Some(WorkerInput { kind: WorkerInputKind::User, content: String::new(), - submission_id: None, + submission_request_id: None, segments: Some(vec![ protocol::Segment::Flow { selector: "builtin:coder-review".to_string(), @@ -5068,7 +5084,7 @@ mod tests { let input = WorkerInput { kind: WorkerInputKind::User, content: String::new(), - submission_id: None, + submission_request_id: None, segments: Some(vec![protocol::Segment::Flow { selector: "builtin:coder-review".to_string(), }]), @@ -5083,11 +5099,11 @@ mod tests { assert_eq!(dispatched[0].kind, input.kind); assert_eq!(dispatched[0].content, input.content); assert_eq!(dispatched[0].segments, input.segments); - let submission_id = dispatched[0] - .submission_id + let submission_request_id = dispatched[0] + .submission_request_id .as_deref() - .expect("Runtime submission id"); - Uuid::parse_str(submission_id).expect("submission id UUID"); + .expect("Runtime submission request id"); + Uuid::parse_str(submission_request_id).expect("submission request id UUID"); } #[cfg(feature = "ws-server")] @@ -5113,11 +5129,7 @@ mod tests { let observations = runtime .read_worker_observation_events(&detail.worker_ref, WorkerObservationCursor::zero()) .unwrap(); - assert_eq!(observations.len(), 1); - assert!(matches!( - observations[0].payload, - protocol::Event::UserMessage { .. } - )); + assert!(observations.is_empty()); runtime .observe_worker_event( @@ -5135,8 +5147,8 @@ mod tests { let observations = runtime .read_worker_observation_events(&detail.worker_ref, WorkerObservationCursor::zero()) .unwrap(); - assert_eq!(observations.len(), 2); - let protocol::Event::SystemItem { item } = &observations[1].payload else { + assert_eq!(observations.len(), 1); + let protocol::Event::SystemItem { item } = &observations[0].payload else { panic!("committed notification observation must be a system item"); }; assert_eq!(item["kind"], "notification"); diff --git a/crates/worker-runtime/src/worker_backend.rs b/crates/worker-runtime/src/worker_backend.rs index 95d0c941..07800e82 100644 --- a/crates/worker-runtime/src/worker_backend.rs +++ b/crates/worker-runtime/src/worker_backend.rs @@ -39,7 +39,7 @@ use crate::working_directory::{ }; use async_trait::async_trait; use protocol::{ErrorCode, Event, Method, Segment, WorkerStatus}; -use session_store::{CombinedStore, LogEntry, WorkerAggregateStore, WorkerSessionStore}; +use session_store::{CombinedStore, WorkerAggregateStore, WorkerSessionStore}; #[cfg(test)] use session_store::{FsStore, FsWorkerStore}; use tokio::runtime::Runtime; @@ -57,11 +57,10 @@ use worker::feature::builtin::{ #[cfg(feature = "ws-server")] use worker::ipc::protocol_session::{live_log_entry_event, subscribe_worker_protocol_session}; use worker::{ - PreparedWorker, PromptCatalogSource, SegmentLogSink, WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN, - Worker, WorkerBootstrap, WorkerBootstrapError, WorkerBootstrapLayout, - WorkerControllerTransport, WorkerError, WorkerFilesystemAuthority, WorkerHandle, - WorkerSharedState, WorkerWorkspaceContext, WorkspaceClient, WorkspaceId, - bash_output_dir_for_worker_id, + PreparedWorker, PromptCatalogSource, SegmentLogSink, Worker, WorkerBootstrap, + WorkerBootstrapError, WorkerBootstrapLayout, WorkerControllerTransport, WorkerError, + WorkerFilesystemAuthority, WorkerHandle, WorkerSharedState, WorkerWorkspaceContext, + WorkspaceClient, WorkspaceId, bash_output_dir_for_worker_id, }; const DEFAULT_BACKEND_ID: &str = "worker-crate"; @@ -70,17 +69,6 @@ const RUNTIME_TASK_TIMEOUT: Duration = Duration::from_secs(10); // returns a typed execution error instead of leaving the outer waiter to time out. const USER_INPUT_COMMIT_TIMEOUT: Duration = Duration::from_secs(9); -fn user_input_has_submission(entry: &LogEntry, submission_id: &str) -> bool { - let extensions = match entry { - LogEntry::AnnotatedUserInput { extensions, .. } => extensions, - _ => return false, - }; - extensions.iter().any(|extension| { - extension.domain == WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN - && extension.payload["submission_id"].as_str() == Some(submission_id) - }) -} - pub struct RuntimeWorkerController { pub handle: WorkerHandle, pub shutdown: Arc>>, @@ -1342,126 +1330,75 @@ where .unwrap_or_else(|message| WorkerExecutionResult::errored(operation, message)) } - fn send_user_input_and_wait_for_commit( + fn send_submit_and_wait_for_acceptance( &self, operation: WorkerExecutionOperation, worker: WorkerHandle, method: Method, - submission_id: String, + submission_request_id: String, accepted_run_state: WorkerExecutionRunState, ) -> WorkerExecutionResult { - let acknowledged_submission_id = submission_id.clone(); + let request_id = submission_request_id.clone(); self.run_on_adapter_runtime(async move { - // Subscribe before enqueueing the input so the acknowledgement cannot - // race with a fast Worker commit. The opaque submission id is stored in - // the same UserInput entry as the transformed Flow input and its state. - let (_, mut committed_entries) = worker.sink.subscribe_with_snapshot(); - let committed_probe = worker.clone(); + // Subscribe before enqueueing so a fast durable acceptance cannot + // race the Runtime acknowledgement. let mut events = worker.subscribe(); worker .send(method) .await .map_err(|err| format!("failed to send Worker method: {err}"))?; - let timeout_probe = committed_probe.clone(); - let timeout_submission_id = submission_id.clone(); - let acknowledgement = tokio::time::timeout(USER_INPUT_COMMIT_TIMEOUT, async move { - let input_was_committed = || { - committed_probe - .committed_entries() - .iter() - .any(|entry| user_input_has_submission(entry, &submission_id)) - }; + tokio::time::timeout(USER_INPUT_COMMIT_TIMEOUT, async move { loop { - tokio::select! { - entry = committed_entries.recv() => { - match entry { - Ok(entry) if user_input_has_submission(&entry, &submission_id) => { - return Ok(()); - } - Ok(_) => {} - Err(tokio::sync::broadcast::error::RecvError::Lagged(skipped)) => { - if input_was_committed() { - return Ok(()); - } - return Err(format!( - "worker input commit acknowledgement lagged by {skipped} entry event(s)" - )); - } - Err(tokio::sync::broadcast::error::RecvError::Closed) => { - if input_was_committed() { - return Ok(()); - } - return Err( - "worker entry stream closed before user input was committed" - .to_string(), - ); - } - } + match events.recv().await { + Ok(Event::SubmissionAccepted { + submission_request_id, + submission_id, + disposition, + }) if submission_request_id == request_id => { + return Ok((submission_id, disposition)); } - event = events.recv() => { - match event { - Ok(Event::Error { message, .. }) => { - if input_was_committed() { - return Ok(()); - } - return Err(format!( - "worker rejected user input before session commit: {message}" - )); - } - Ok(Event::Shutdown) => { - if input_was_committed() { - return Ok(()); - } - return Err( - "worker shut down before user input was committed".to_string() - ); - } - Ok(_) => {} - Err(tokio::sync::broadcast::error::RecvError::Lagged(skipped)) => { - if input_was_committed() { - return Ok(()); - } - return Err(format!( - "worker input commit acknowledgement lagged by {skipped} protocol event(s)" - )); - } - Err(tokio::sync::broadcast::error::RecvError::Closed) => { - if input_was_committed() { - return Ok(()); - } - return Err( - "worker event stream closed before user input was committed" - .to_string(), - ); - } - } + Ok(Event::SubmissionRejected { + submission_request_id, + message, + }) if submission_request_id == request_id => { + return Err(format!("worker rejected Submit: {message}")); + } + Ok(Event::Error { message, .. }) => { + return Err(format!( + "worker rejected Submit before durable acceptance: {message}" + )); + } + Ok(Event::Shutdown) => { + return Err( + "worker shut down before Submit was durably accepted".to_string() + ); + } + Ok(_) => {} + Err(tokio::sync::broadcast::error::RecvError::Lagged(skipped)) => { + return Err(format!( + "worker Submit acknowledgement lagged by {skipped} protocol event(s)" + )); + } + Err(tokio::sync::broadcast::error::RecvError::Closed) => { + return Err( + "worker event stream closed before Submit was durably accepted" + .to_string(), + ); } } } }) - .await; - - match acknowledgement { - Ok(result) => result, - Err(_) => { - if timeout_probe - .committed_entries() - .iter() - .any(|entry| user_input_has_submission(entry, &timeout_submission_id)) - { - Ok(()) - } else { - Err("timed out waiting for worker user input commit".to_string()) - } - } - } + .await + .map_err(|_| "timed out waiting for durable Worker Submit acceptance".to_string())? }) - .map(|_| { - WorkerExecutionResult::accepted_input_committed( + .map(|(submission_id, disposition)| { + WorkerExecutionResult::accepted_submission( operation, accepted_run_state, - acknowledged_submission_id, + submission_request_id, + submission_id, + disposition, ) }) .unwrap_or_else(|message| WorkerExecutionResult::errored(operation, message)) @@ -1582,8 +1519,8 @@ impl Drop for WorkerRuntimeExecutionBackend { fn method_starts_turn(method: &Method) -> bool { matches!( method, - Method::Run { .. } - | Method::RunTracked { .. } + Method::Submit { .. } + | Method::SubmitTracked { .. } | Method::Notify { auto_run: true, .. } | Method::Resume | Method::Compact @@ -1609,8 +1546,8 @@ fn accepted_notify_run_state(status: WorkerStatus, auto_run: bool) -> WorkerExec fn accepted_run_state_for_method(method: &Method) -> WorkerExecutionRunState { match method { - Method::Run { .. } - | Method::RunTracked { .. } + Method::Submit { .. } + | Method::SubmitTracked { .. } | Method::Notify { auto_run: true, .. } | Method::Resume | Method::Compact => WorkerExecutionRunState::Busy, @@ -1963,6 +1900,9 @@ where WorkerExecutionOperation::Input, worker, Method::Notify { + notification_request_id: input + .submission_request_id + .unwrap_or_else(protocol::new_submission_request_id), message: input.content, auto_run: true, }, @@ -1975,21 +1915,23 @@ where return result; } - if worker.shared_state.get_status() != WorkerStatus::Idle - || busy + let is_user_submit = input.kind == WorkerInputKind::User; + let status = worker.shared_state.get_status(); + let claimed_here = status == WorkerStatus::Idle + && busy .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) - .is_err() - { + .is_ok(); + if !is_user_submit && !claimed_here { return WorkerExecutionResult::busy( WorkerExecutionOperation::Input, - "Worker is already running; runtime adapter v0 does not queue input", + "Worker is already running", ); } - let (method, submission_id) = match input.kind { + let (method, submission_request_id) = match input.kind { WorkerInputKind::User => { let Some(submission_id) = input - .submission_id + .submission_request_id .filter(|submission_id| !submission_id.trim().is_empty()) else { busy.store(false, Ordering::SeqCst); @@ -1999,11 +1941,11 @@ where ); }; ( - Method::RunTracked { + Method::SubmitTracked { + submission_request_id: submission_id.clone(), input: input.segments.unwrap_or_else(|| { vec![Segment::text(input.content.trim().to_string())] }), - submission_id: submission_id.clone(), }, Some(submission_id), ) @@ -2021,21 +1963,21 @@ where ), }; let accepted_run_state = match method { - Method::Run { .. } - | Method::RunTracked { .. } + Method::Submit { .. } + | Method::SubmitTracked { .. } | Method::Notify { .. } | Method::Compact => WorkerExecutionRunState::Busy, _ => WorkerExecutionRunState::Idle, }; let accepted_is_idle = accepted_run_state == WorkerExecutionRunState::Idle; - let waits_for_user_input_commit = submission_id.is_some(); + let waits_for_submission_acceptance = submission_request_id.is_some(); - let result = if waits_for_user_input_commit { - self.send_user_input_and_wait_for_commit( + let result = if waits_for_submission_acceptance { + self.send_submit_and_wait_for_acceptance( WorkerExecutionOperation::Input, worker, method, - submission_id.expect("tracked Run has submission id"), + submission_request_id.expect("Submit must have a submission request id"), accepted_run_state, ) } else { @@ -2046,7 +1988,9 @@ where accepted_run_state, ) }; - if accepted_is_idle || result.outcome != crate::execution::WorkerExecutionOutcome::Accepted + if accepted_is_idle + || (claimed_here + && result.outcome != crate::execution::WorkerExecutionOutcome::Accepted) { busy.store(false, Ordering::SeqCst); } @@ -3400,6 +3344,60 @@ mod tests { ); } + #[test] + fn running_worker_accepts_a_second_submit_as_queued() { + let client = MockClient::sequential(vec![MockResponse::Hang(vec![])]); + let runtime_base = tempfile::tempdir().unwrap(); + let cwd = tempfile::tempdir().unwrap(); + let store = tempfile::tempdir().unwrap(); + let factory = MockFactory { + client, + runtime_base: runtime_base.path().to_path_buf(), + cwd: cwd.path().to_path_buf(), + store_dir: store.path().join("sessions"), + worker_metadata_dir: store.path().join("workers"), + observed_cwds: Arc::new(Mutex::new(Vec::new())), + observed_workspace_clients: Arc::new(Mutex::new(Vec::new())), + }; + let backend = Arc::new(WorkerRuntimeExecutionBackend::new(factory).unwrap()); + let runtime = + EmbeddedRuntime::with_execution_backend(RuntimeOptions::default(), backend).unwrap(); + runtime.store_config_bundle(test_bundle()).unwrap(); + let detail = runtime + .create_worker(create_request("queued-submit")) + .unwrap(); + + let mut first_input = WorkerInput::user("first"); + first_input.submission_request_id = Some("request-first".into()); + let first = runtime + .send_input(&detail.worker_ref, first_input.clone()) + .unwrap(); + assert_eq!( + first.submission.as_ref().map(|ack| ack.disposition), + Some(protocol::SubmissionDisposition::Started) + ); + let retry = runtime.send_input(&detail.worker_ref, first_input).unwrap(); + assert_eq!(retry.submission, first.submission); + let mut conflicting_retry = WorkerInput::user("different"); + conflicting_retry.submission_request_id = Some("request-first".into()); + assert!( + runtime + .send_input(&detail.worker_ref, conflicting_retry) + .is_err(), + "same request id with a different payload must fail" + ); + + let mut second_input = WorkerInput::user("second"); + second_input.submission_request_id = Some("request-second".into()); + let second = runtime + .send_input(&detail.worker_ref, second_input) + .unwrap(); + assert_eq!( + second.submission.as_ref().map(|ack| ack.disposition), + Some(protocol::SubmissionDisposition::Queued) + ); + } + #[test] fn create_with_initial_input_returns_after_session_commit() { let client = MockClient::new(simple_text_events()); @@ -3449,8 +3447,10 @@ mod tests { }; extensions .iter() - .find(|extension| extension.domain == WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN) - .and_then(|extension| extension.payload["submission_id"].as_str()) + .find(|extension| extension.domain == "worker.pending_activations.v1") + .and_then(|extension| { + extension.payload["receipts"][0]["submission_id"].as_str() + }) }) .expect("committed input submission id"); uuid::Uuid::parse_str(submission_id).expect("opaque submission id is a UUID"); diff --git a/crates/worker/Cargo.toml b/crates/worker/Cargo.toml index 87437d17..87046834 100644 --- a/crates/worker/Cargo.toml +++ b/crates/worker/Cargo.toml @@ -20,6 +20,7 @@ protocol = { workspace = true, features = ["json-schema"] } client = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +sha2 = { workspace = true } reqwest = { version = "0.13", default-features = false, features = ["blocking", "native-tls"] } thiserror = { workspace = true } tokio = { workspace = true, features = ["fs", "io-util", "macros", "net", "process", "rt-multi-thread", "signal", "sync", "time"] } diff --git a/crates/worker/examples/worker_protocol.rs b/crates/worker/examples/worker_protocol.rs index 1e2c618f..cd00370a 100644 --- a/crates/worker/examples/worker_protocol.rs +++ b/crates/worker/examples/worker_protocol.rs @@ -101,7 +101,10 @@ async fn main() -> Result<(), Box> { // Send a run method handle - .send(Method::run_text("What is the capital of France?")) + .send(Method::submit_text( + protocol::new_submission_request_id(), + "What is the capital of France?", + )) .await?; // Wait for completion diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index 4e431d3d..2794b35c 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -5,7 +5,7 @@ use std::sync::atomic::Ordering; use agen::EngineError; use agen::llm_client::client::LlmClient; use session_store::WorkerMetadataStore; -use session_store::{LogEntry, SessionExtension, Store}; +use session_store::{LogEntry, Store}; use tokio::sync::{broadcast, mpsc, oneshot}; use crate::discovery::WorkerDiscovery; @@ -23,16 +23,12 @@ use crate::shutdown_after_idle::{ }; use crate::spawn::registry::SpawnedWorkerRegistry; use crate::spawn::tool::sub_worker_spawn_tool; -use crate::worker::{ - SystemItemCommitter, WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN, Worker, WorkerError, - WorkerRunResult, -}; +use crate::worker::{SystemItemCommitter, Worker, WorkerError, WorkerRunResult}; use protocol::{ AlertLevel, AlertSource, CommandEvent as ProtocolCommandEvent, CommandSnapshot as ProtocolCommandSnapshot, CommandStatus as ProtocolCommandStatus, CommandStream as ProtocolCommandStream, CommandStreamSlice as ProtocolCommandStreamSlice, - ErrorCode, Event, Method, RewindTargetId, RunResult, Segment, TurnResult, UploadedFileRef, - WorkerStatus, + ErrorCode, Event, Method, RewindTargetId, RunResult, TurnResult, UploadedFileRef, WorkerStatus, }; use workdir::{ CommandEvent as WorkdirCommandEvent, CommandSnapshot as WorkdirCommandSnapshot, @@ -58,6 +54,7 @@ pub struct WorkerHandle { spawned_registry: Arc, artifact_store: Arc, session_id: session_store::SessionId, + pending_activations: Arc>, } impl WorkerHandle { @@ -131,8 +128,15 @@ impl WorkerHandle { let in_flight = snapshot_from_guard(&in_flight_guard); (entries, entry_rx, in_flight) }; + let mut session = + session_store::public_snapshot::project_current_session_snapshot(&entries); + session.pending_submissions = self + .pending_activations + .lock() + .expect("pending activation state poisoned") + .snapshot(); let event = Event::Snapshot { - session: session_store::public_snapshot::project_current_session_snapshot(&entries), + session, greeting: self.shared_state.greeting.clone(), status: self.shared_state.get_status(), in_flight, @@ -213,21 +217,41 @@ async fn finish_controller_run( /// `Worker::*` entry point — `RunForNotification` carries none because /// `worker.run_for_notification()` drains the NotifyBuffer on its own. enum PendingRun { - Run(Vec), - RunTracked { - input: Vec, - extension: SessionExtension, - }, + Submit(crate::worker::PendingSubmission), /// Self-initiated turn kicked from the notify buffer. The carried /// `InvokeKind` is the trigger that flipped the Worker from IDLE /// (Notify or WorkerEvent) and is recorded by the Invoke marker /// committed at the start of `worker.run_for_notification`. - RunForNotification(protocol::InvokeKind), + RunForNotification { + invoke_kind: protocol::InvokeKind, + notification_request_id: Option, + }, Resume, } +fn prepare_pending_run( + pending_submissions: &crate::worker::PendingSubmissionHandle, + notify_buffer: &NotifyBuffer, +) -> Result, crate::worker::PendingSubmissionError> { + Ok(match pending_submissions.prepare_next_activation()? { + Some(crate::worker::PendingActivation::Submission(submission)) => { + Some(PendingRun::Submit(submission)) + } + Some(crate::worker::PendingActivation::Notification(notification)) => { + let extension = pending_submissions.notification_activation_extension(); + let notification_request_id = notification.notification_request_id.clone(); + notify_buffer.push_durable_notify(notification.message, extension); + Some(PendingRun::RunForNotification { + invoke_kind: protocol::InvokeKind::Notify, + notification_request_id: Some(notification_request_id), + }) + } + None => None, + }) +} + impl PendingRun { - /// Whether this turn was kicked off by the parent (via `Method::Run` + /// Whether this turn was kicked off by the parent (via `Method::Submit` /// or `Method::Resume`). Used by [`drive_turn`] to gate upward /// `WorkerEvent::TurnEnded` / `WorkerEvent::Errored` reports so the parent /// only sees completion signals for work it actually delegated. @@ -235,16 +259,12 @@ impl PendingRun { /// notify buffer (Notify / inbound WorkerEvent) and stays silent. fn is_parent_originated(&self) -> bool { match self { - PendingRun::Run(_) | PendingRun::RunTracked { .. } | PendingRun::Resume => true, - PendingRun::RunForNotification(_) => false, + PendingRun::Submit(_) | PendingRun::Resume => true, + PendingRun::RunForNotification { .. } => false, } } } -fn should_auto_run_notification(status: WorkerStatus, auto_run: bool) -> bool { - auto_run && status == WorkerStatus::Idle -} - // --------------------------------------------------------------------------- // WorkerController — actor that owns a Worker // --------------------------------------------------------------------------- @@ -552,6 +572,7 @@ impl WorkerController { let artifact_store: Arc = Arc::new(worker.store().clone()); let session_id = worker.session_id(); + let pending_activations = worker.pending_activation_state(); let handle = WorkerHandle { method_tx, working_event_tx: working_event_tx.clone(), @@ -563,6 +584,7 @@ impl WorkerController { spawned_registry: spawned_registry.clone(), artifact_store, session_id, + pending_activations, }; let socket_server = match transport { @@ -1291,6 +1313,7 @@ async fn controller_loop( spawned_registry.clone(), ); let mut pending: Option = None; + let pending_submissions = worker.pending_submission_handle(); loop { // Top-of-iteration: if an event handler staged a run, fire it @@ -1307,8 +1330,8 @@ async fn controller_loop( // interrupted/error turn from being carried into the next snapshot. worker.clear_in_flight_events(); let parent_originated = run.is_parent_originated(); - let user_input_run = matches!(&run, PendingRun::Run(_) | PendingRun::RunTracked { .. }); - if !user_input_run { + let user_input_submit = matches!(&run, PendingRun::Submit(_)); + if !user_input_submit { set_controller_status( &shared_state, &runtime_dir, @@ -1317,37 +1340,21 @@ async fn controller_loop( ) .await; } - let (mut new_status, shutdown) = match run { - PendingRun::Run(input) => { + let notification_request_id = match &run { + PendingRun::RunForNotification { + notification_request_id, + .. + } => notification_request_id.clone(), + _ => None, + }; + let (mut new_status, shutdown, may_drain_pending) = match run { + PendingRun::Submit(submission) => { let (input_commit_tx, input_commit_rx) = oneshot::channel(); + let committed_submission = submission.clone(); + let extension = pending_submissions.activation_extension(); drive_turn( worker.run_with_input_extensions_and_commit_hook( - input, - Vec::new(), - move || { - let _ = input_commit_tx.send(()); - }, - ), - &mut method_rx, - &working_event_tx, - &cancel_tx, - &pause_tx, - &shared_state, - &runtime_dir, - Some(input_commit_rx), - ¬ify_buffer, - self_parent_socket.as_ref(), - &spawner_name, - &spawned_registry, - parent_originated, - ) - .await - } - PendingRun::RunTracked { input, extension } => { - let (input_commit_tx, input_commit_rx) = oneshot::channel(); - drive_turn( - worker.run_with_input_extensions_and_commit_hook( - input, + submission.input, vec![extension], move || { let _ = input_commit_tx.send(()); @@ -1359,8 +1366,9 @@ async fn controller_loop( &pause_tx, &shared_state, &runtime_dir, - Some(input_commit_rx), + Some((input_commit_rx, committed_submission)), ¬ify_buffer, + &pending_submissions, self_parent_socket.as_ref(), &spawner_name, &spawned_registry, @@ -1368,9 +1376,9 @@ async fn controller_loop( ) .await } - PendingRun::RunForNotification(kind) => { + PendingRun::RunForNotification { invoke_kind, .. } => { drive_turn( - worker.run_for_notification(kind), + worker.run_for_notification(invoke_kind), &mut method_rx, &working_event_tx, &cancel_tx, @@ -1379,6 +1387,7 @@ async fn controller_loop( &runtime_dir, None, ¬ify_buffer, + &pending_submissions, self_parent_socket.as_ref(), &spawner_name, &spawned_registry, @@ -1397,6 +1406,7 @@ async fn controller_loop( &runtime_dir, None, ¬ify_buffer, + &pending_submissions, self_parent_socket.as_ref(), &spawner_name, &spawned_registry, @@ -1405,10 +1415,32 @@ async fn controller_loop( .await } }; - if !shutdown && new_status == WorkerStatus::Idle && notify_buffer.has_auto_run_pending() - { - pending = Some(PendingRun::RunForNotification(protocol::InvokeKind::Notify)); - new_status = WorkerStatus::Running; + if let Some(notification_request_id) = notification_request_id { + pending_submissions.finish_notification_activation(¬ification_request_id); + } + + if !shutdown && may_drain_pending && new_status == WorkerStatus::Idle { + match prepare_pending_run(&pending_submissions, ¬ify_buffer) { + Ok(Some(next)) => { + pending = Some(next); + new_status = WorkerStatus::Running; + } + Ok(None) => { + if notify_buffer.has_auto_run_pending() { + pending = Some(PendingRun::RunForNotification { + invoke_kind: protocol::InvokeKind::Notify, + notification_request_id: None, + }); + new_status = WorkerStatus::Running; + } + } + Err(error) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::Internal, + message: error.to_string(), + }); + } + } } finish_controller_run( &mut worker, @@ -1435,61 +1467,118 @@ async fn controller_loop( }; match method { - Method::Run { input } => { - if shared_state.get_status() == WorkerStatus::Running { - // Defensive: the inner select! inside drive_turn - // already rejects `Run` while a turn is live, so - // this branch is only reachable across a race window - // around status flips. - let _ = working_event_tx.send(Event::Error { - code: ErrorCode::AlreadyRunning, - message: "Worker is already executing a turn".into(), - }); - continue; - } - // Stage the run without a speculative user-message echo. - // `Worker::run` validates the input, commits - // `LogEntry::AnnotatedUserInput`, and the session-log sink turns that - // committed entry into the live `Event::UserMessage`. That - // keeps every client ordered against `SegmentStart` replay and - // makes persisted history the single source of visible user - // input. Paused→Run cleanup (orphan tool_result closure + - // interrupt system note) is applied inside `Worker::run` itself - // when the worker's `last_run_interrupted` flag is set. - pending = Some(PendingRun::Run(input)); - } - - Method::RunTracked { + Method::Submit { + submission_request_id, input, - submission_id, - } => { - // Runtime-correlated submissions retain their opaque id in the - // same durable UserInput record used for Flow state. - let extension = SessionExtension::new( - WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN, - serde_json::json!({ "submission_id": submission_id }), - ); - pending = Some(PendingRun::RunTracked { input, extension }); } - - Method::Notify { message, auto_run } => { - // Client-side live echo is delivered as `Event::SystemItem` - // once the interceptor commits the corresponding - // `LogEntry::AnnotatedSystemItem` entry — drained out of the - // notify buffer + broadcast through the sink. No - // separate echo here. - 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)); + | Method::SubmitTracked { + submission_request_id, + input, + } => { + let request_id = submission_request_id.clone(); + match pending_submissions.accept(submission_request_id, input, true) { + Ok(acceptance) => { + if let Some(activation) = acceptance.activation { + pending = Some(PendingRun::Submit(activation)); + } else { + let _ = working_event_tx.send(Event::SubmissionAccepted { + submission_request_id: acceptance.submission_request_id, + submission_id: acceptance.submission_id, + disposition: acceptance.disposition, + }); + } + } + Err(error) => { + let _ = working_event_tx.send(Event::SubmissionRejected { + submission_request_id: request_id, + message: error.to_string(), + }); + } } } + Method::Notify { + notification_request_id, + message, + auto_run, + } => { + if auto_run { + match pending_submissions.accept_notification(notification_request_id, message) + { + Ok(true) => match prepare_pending_run(&pending_submissions, ¬ify_buffer) + { + Ok(Some(next)) => pending = Some(next), + Ok(None) => {} + Err(error) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::Internal, + message: error.to_string(), + }); + } + }, + Ok(false) => {} + Err(error) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::InvalidRequest, + message: error.to_string(), + }); + } + } + } else { + worker.push_notify(message, false); + } + } + + Method::ListPendingSubmissions => { + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { + pending: pending_submissions.snapshot(), + }); + } + Method::CancelPendingSubmission { submission_id } => { + match pending_submissions.cancel(&submission_id) { + Ok(pending_snapshot) => { + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { + pending: pending_snapshot, + }); + } + Err(error) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::InvalidRequest, + message: error.to_string(), + }); + } + } + } + Method::ClearPendingSubmissions => match pending_submissions.clear() { + Ok(pending_snapshot) => { + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { + pending: pending_snapshot, + }); + } + Err(error) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::Internal, + message: error.to_string(), + }); + } + }, + Method::ContinuePending => { + match prepare_pending_run(&pending_submissions, ¬ify_buffer) { + Ok(Some(next)) => pending = Some(next), + Ok(None) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::InvalidRequest, + message: "pending activation queue is empty".into(), + }); + } + Err(error) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::Internal, + message: error.to_string(), + }); + } + } + } Method::Resume => { if shared_state.get_status() != WorkerStatus::Paused { let _ = working_event_tx.send(Event::Error { @@ -1703,9 +1792,10 @@ async fn controller_loop( // notification is not stranded. Matches the // `Method::Notify` idle path. if shared_state.get_status() == WorkerStatus::Idle { - pending = Some(PendingRun::RunForNotification( - protocol::InvokeKind::WorkerEvent, - )); + pending = Some(PendingRun::RunForNotification { + invoke_kind: protocol::InvokeKind::WorkerEvent, + notification_request_id: None, + }); } } } @@ -1788,12 +1878,12 @@ async fn handle_inbound_worker_event( /// as `Errored` — only the worker-execution `Err` branch below fires. /// /// `parent_originated` further restricts both upward reports to turns -/// the parent actually delegated (`Method::Run` / `Method::Resume`). +/// the parent actually delegated (`Method::Submit` / `Method::Resume`). /// `Method::Notify` / inbound `WorkerEvent` auto-kicks complete silently /// so the parent's history does not get flooded with child-internal /// turn boundaries. #[allow(clippy::too_many_arguments)] -async fn drive_turn( +async fn drive_turn( worker_future: F, method_rx: &mut mpsc::Receiver, working_event_tx: &broadcast::Sender, @@ -1801,15 +1891,17 @@ async fn drive_turn( pause_tx: &mpsc::Sender<()>, shared_state: &Arc, runtime_dir: &RuntimeDir, - mut input_commit_rx: Option>, + mut input_commit: Option<(oneshot::Receiver<()>, crate::worker::PendingSubmission)>, notify_buffer: &NotifyBuffer, + pending_submissions: &crate::worker::PendingSubmissionHandle, parent_socket: Option<&PathBuf>, self_name: &str, spawned_registry: &Arc, parent_originated: bool, -) -> (WorkerStatus, bool) +) -> (WorkerStatus, bool, bool) where F: std::future::Future>, + St: Store + Clone, { tokio::pin!(worker_future); let mut shutdown_requested = false; @@ -1822,13 +1914,25 @@ where // Running snapshot contract deterministic even for immediate clients. biased; committed = async { - input_commit_rx + input_commit .as_mut() + .map(|(receiver, _)| receiver) .expect("input commit receiver guarded by select condition") .await - }, if input_commit_rx.is_some() => { - input_commit_rx = None; + }, if input_commit.is_some() => { + let submission = input_commit.take().map(|(_, submission)| submission); if committed.is_ok() { + if let Some(submission) = submission { + pending_submissions.finish_activation(&submission.submission_id); + let _ = working_event_tx.send(Event::SubmissionAccepted { + submission_request_id: submission.submission_request_id, + submission_id: submission.submission_id, + disposition: protocol::SubmissionDisposition::Started, + }); + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { + pending: pending_submissions.snapshot(), + }); + } set_controller_status( shared_state, runtime_dir, @@ -1836,11 +1940,33 @@ where WorkerStatus::Running, ) .await; + } else if let Some(submission) = submission { + pending_submissions.abort_activation(submission); } } result = &mut worker_future => { + if let Some((mut receiver, submission)) = input_commit.take() { + match receiver.try_recv() { + Ok(()) => { + pending_submissions.finish_activation(&submission.submission_id); + let _ = working_event_tx.send(Event::SubmissionAccepted { + submission_request_id: submission.submission_request_id, + submission_id: submission.submission_id, + disposition: protocol::SubmissionDisposition::Started, + }); + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { + pending: pending_submissions.snapshot(), + }); + } + Err(_) => pending_submissions.abort_activation(submission), + } + } return match result { Ok(r) => { + let may_drain_pending = matches!( + &r, + WorkerRunResult::Finished | WorkerRunResult::LimitReached + ); let (status, run_result) = match r { WorkerRunResult::Finished if pause_requested => { (WorkerStatus::Paused, RunResult::Paused) @@ -1851,7 +1977,7 @@ where WorkerRunResult::RolledBack => (WorkerStatus::Idle, RunResult::RolledBack), WorkerRunResult::Interrupted { .. } if pause_requested => { let _ = working_event_tx.send(Event::RunEnd { result: RunResult::Paused }); - return (WorkerStatus::Paused, shutdown_requested); + return (WorkerStatus::Paused, shutdown_requested, false); } WorkerRunResult::Interrupted { code, message } => { let _ = working_event_tx.send(Event::Error { @@ -1867,7 +1993,7 @@ where }, ); } - return (WorkerStatus::Idle, shutdown_requested); + return (WorkerStatus::Idle, shutdown_requested, false); } }; let _ = working_event_tx.send(Event::RunEnd { result: run_result }); @@ -1879,7 +2005,7 @@ where }, ); } - (status, shutdown_requested) + (status, shutdown_requested, may_drain_pending) } Err(WorkerError::Engine(EngineError::Cancelled)) if pause_requested => { // User-initiated Pause. Report the transition to @@ -1888,7 +2014,7 @@ where // that channel is reserved for worker runtime // failures, not deliberate interruptions. let _ = working_event_tx.send(Event::RunEnd { result: RunResult::Paused }); - (WorkerStatus::Paused, shutdown_requested) + (WorkerStatus::Paused, shutdown_requested, false) } Err(e) => { let code = worker_error_code(&e); @@ -1906,11 +2032,11 @@ where }, ); } - (WorkerStatus::Idle, shutdown_requested) + (WorkerStatus::Idle, shutdown_requested, false) } }; } - method = method_rx.recv() => { + method = method_rx.recv(), if input_commit.is_none() => { match method { Some(Method::Cancel) => { let _ = cancel_tx.try_send(()); @@ -1923,12 +2049,71 @@ where shutdown_requested = true; let _ = cancel_tx.try_send(()); } - Some(Method::Run { .. } | Method::RunTracked { .. } | Method::Resume) => { + Some(Method::Submit { + submission_request_id, + input, + } + | Method::SubmitTracked { + submission_request_id, + input, + }) => { + let request_id = submission_request_id.clone(); + match pending_submissions.accept(submission_request_id, input, false) { + Ok(acceptance) => { + let _ = working_event_tx.send(Event::SubmissionAccepted { + submission_request_id: acceptance.submission_request_id, + submission_id: acceptance.submission_id, + disposition: acceptance.disposition, + }); + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { + pending: pending_submissions.snapshot(), + }); + } + Err(error) => { + let _ = working_event_tx.send(Event::SubmissionRejected { + submission_request_id: request_id, + message: error.to_string(), + }); + } + } + } + Some(Method::Resume | Method::ContinuePending) => { let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, message: "Worker is already executing a turn".into(), }); } + Some(Method::ListPendingSubmissions) => { + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { + pending: pending_submissions.snapshot(), + }); + } + Some(Method::CancelPendingSubmission { submission_id }) => { + match pending_submissions.cancel(&submission_id) { + Ok(pending) => { + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { pending }); + } + Err(error) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::InvalidRequest, + message: error.to_string(), + }); + } + } + } + Some(Method::ClearPendingSubmissions) => { + match pending_submissions.clear() { + Ok(pending) => { + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { pending }); + } + Err(error) => { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::Internal, + message: error.to_string(), + }); + } + } + } Some(Method::Compact | Method::ListRewindTargets | Method::RewindTo { .. }) => { let _ = working_event_tx.send(Event::Error { code: ErrorCode::AlreadyRunning, @@ -1936,11 +2121,28 @@ where .into(), }); } - 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, auto_run); + Some(Method::Notify { + notification_request_id, + message, + auto_run, + }) => { + if auto_run { + if let Err(error) = pending_submissions.accept_notification( + notification_request_id, + message, + ) { + let _ = working_event_tx.send(Event::Error { + code: ErrorCode::InvalidRequest, + message: error.to_string(), + }); + } else { + let _ = working_event_tx.send(Event::PendingSubmissionsChanged { + pending: pending_submissions.snapshot(), + }); + } + } else { + notify_buffer.push_notify(message, false); + } } Some(Method::ListCompletions { .. }) => {} Some(Method::ListWorkers | Method::RestoreWorker { .. } | Method::RegisterPeer { .. }) => { @@ -1969,7 +2171,7 @@ where None => { let _ = cancel_tx.try_send(()); shared_state.set_status(WorkerStatus::Idle); - return (WorkerStatus::Idle, false); + return (WorkerStatus::Idle, false, false); } } } @@ -2134,21 +2336,16 @@ mod tests { #[test] fn pending_run_parent_origin_table() { - assert!(PendingRun::Run(Vec::new()).is_parent_originated()); assert!(PendingRun::Resume.is_parent_originated()); assert!( - !PendingRun::RunForNotification(protocol::InvokeKind::Notify).is_parent_originated() + !PendingRun::RunForNotification { + invoke_kind: protocol::InvokeKind::Notify, + notification_request_id: None, + } + .is_parent_originated() ); } - #[test] - fn notification_auto_run_gate_only_allows_idle_auto_run() { - assert!(should_auto_run_notification(WorkerStatus::Idle, true)); - assert!(!should_auto_run_notification(WorkerStatus::Idle, false)); - assert!(!should_auto_run_notification(WorkerStatus::Running, true)); - assert!(!should_auto_run_notification(WorkerStatus::Paused, true)); - } - struct DriveTurnEnv { // Held to keep the channel alive; without this `method_rx.recv()` // would observe channel-closed and confuse the select! arm. @@ -2161,6 +2358,7 @@ mod tests { _pause_rx: mpsc::Receiver<()>, shared_state: Arc, notify_buffer: NotifyBuffer, + pending_submissions: crate::worker::PendingSubmissionHandle, spawned_registry: Arc, parent_socket_path: PathBuf, runtime_dir: Arc, @@ -2194,6 +2392,8 @@ mod tests { }, )); let notify_buffer = NotifyBuffer::new(); + let pending_submissions = + crate::worker::PendingSubmissionHandle::for_test(&temp.path().join("pending-sessions")); let spawned_registry = SpawnedWorkerRegistry::new(runtime_dir.clone()); let parent_socket_path = temp.path().join("parent.sock"); @@ -2207,6 +2407,7 @@ mod tests { _pause_rx: pause_rx, shared_state, notify_buffer, + pending_submissions, spawned_registry, parent_socket_path, runtime_dir, @@ -2225,6 +2426,7 @@ mod tests { writer .write(&Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: protocol::Greeting { @@ -2259,7 +2461,7 @@ mod tests { let recv = tokio::spawn(recv_worker_event(listener, Duration::from_secs(2))); let worker_future = async { Ok::<_, WorkerError>(WorkerRunResult::Finished) }; - let (status, shutdown) = drive_turn( + let (status, shutdown, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2269,6 +2471,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, Some(&env.parent_socket_path), "child-worker", &env.spawned_registry, @@ -2302,7 +2505,7 @@ mod tests { Ok::<_, WorkerError>(WorkerRunResult::Finished) }; let started_at = std::time::Instant::now(); - let (status, shutdown) = drive_turn( + let (status, shutdown, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2312,6 +2515,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, None, "child-worker", &env.spawned_registry, @@ -2332,7 +2536,7 @@ mod tests { let listener = UnixListener::bind(&env.parent_socket_path).expect("bind listener"); let worker_future = async { Ok::<_, WorkerError>(WorkerRunResult::Finished) }; - let (status, _) = drive_turn( + let (status, _, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2342,6 +2546,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, Some(&env.parent_socket_path), "child-worker", &env.spawned_registry, @@ -2370,7 +2575,7 @@ mod tests { "boom from test".into(), ))) }; - let (status, _) = drive_turn( + let (status, _, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2380,6 +2585,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, Some(&env.parent_socket_path), "child-worker", &env.spawned_registry, @@ -2414,7 +2620,7 @@ mod tests { "boom from notify".into(), ))) }; - let (status, _) = drive_turn( + let (status, _, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2424,6 +2630,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, Some(&env.parent_socket_path), "child-worker", &env.spawned_registry, @@ -2456,7 +2663,7 @@ mod tests { tokio::time::sleep(Duration::from_millis(50)).await; Ok::<_, WorkerError>(WorkerRunResult::Finished) }; - let (status, shutdown) = drive_turn( + let (status, shutdown, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2466,6 +2673,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, Some(&env.parent_socket_path), "parent", &env.spawned_registry, @@ -2495,7 +2703,7 @@ mod tests { tokio::time::sleep(Duration::from_millis(50)).await; Ok::<_, WorkerError>(WorkerRunResult::Finished) }; - let (status, shutdown) = drive_turn( + let (status, shutdown, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2505,6 +2713,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, Some(&env.parent_socket_path), "parent", &env.spawned_registry, @@ -2522,6 +2731,7 @@ mod tests { let mut env = make_env().await; env._method_tx .send(Method::Notify { + notification_request_id: protocol::new_submission_request_id(), message: "continue".into(), auto_run: true, }) @@ -2532,7 +2742,7 @@ mod tests { tokio::time::sleep(Duration::from_millis(50)).await; Ok::<_, WorkerError>(WorkerRunResult::Finished) }; - let (status, shutdown) = drive_turn( + let (status, shutdown, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2542,6 +2752,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, Some(&env.parent_socket_path), "parent", &env.spawned_registry, @@ -2551,8 +2762,8 @@ mod tests { assert_eq!(status, WorkerStatus::Idle); assert!(!shutdown); - assert_eq!(env.notify_buffer.len(), 1); - assert!(env.notify_buffer.has_auto_run_pending()); + assert_eq!(env.notify_buffer.len(), 0); + assert_eq!(env.pending_submissions.snapshot().notification_count, 1); } #[tokio::test] @@ -2568,7 +2779,7 @@ mod tests { tokio::time::sleep(Duration::from_millis(50)).await; Ok::<_, WorkerError>(WorkerRunResult::Finished) }; - let (status, shutdown) = drive_turn( + let (status, shutdown, _) = drive_turn( worker_future, &mut env.method_rx, &env.working_event_tx, @@ -2578,6 +2789,7 @@ mod tests { &env.runtime_dir, None, &env.notify_buffer, + &env.pending_submissions, Some(&env.parent_socket_path), "child-worker", &env.spawned_registry, diff --git a/crates/worker/src/discovery.rs b/crates/worker/src/discovery.rs index 32da104d..effc1f6f 100644 --- a/crates/worker/src/discovery.rs +++ b/crates/worker/src/discovery.rs @@ -1012,7 +1012,15 @@ async fn send_peer_notify(socket_path: &Path, message: String) -> io::Result<()> } async fn send_notify(socket_path: &Path, message: String, auto_run: bool) -> io::Result<()> { - connect_and_send(socket_path, &Method::Notify { message, auto_run }).await + connect_and_send( + socket_path, + &Method::Notify { + notification_request_id: protocol::new_submission_request_id(), + message, + auto_run, + }, + ) + .await } fn json_content(value: &T) -> Result { @@ -1482,6 +1490,7 @@ mod tests { writer .write(&Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: protocol::Greeting { @@ -1517,6 +1526,7 @@ mod tests { writer .write(&Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: protocol::Greeting { @@ -1536,7 +1546,10 @@ mod tests { .await .unwrap(); let method = reader.next::().await.unwrap().unwrap(); - if let Method::Notify { message, auto_run } = method { + if let Method::Notify { + message, auto_run, .. + } = method + { assert!(auto_run); tx.send(message).await.unwrap(); } else { @@ -1608,6 +1621,7 @@ mod tests { writer .write(&Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: protocol::Greeting { @@ -1634,6 +1648,7 @@ mod tests { writer .write(&Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: protocol::Greeting { @@ -1653,7 +1668,10 @@ mod tests { .await .unwrap(); let method = reader.next::().await.unwrap().unwrap(); - if let Method::Notify { message, auto_run } = method { + if let Method::Notify { + message, auto_run, .. + } = method + { assert!(!auto_run); tx.send(message).await.unwrap(); } else { @@ -1738,6 +1756,7 @@ mod tests { writer .write(&Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: Vec::new(), }, greeting: protocol::Greeting { @@ -1790,6 +1809,8 @@ mod tests { let _ = writer .write(&Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default( + ), entries: Vec::new(), }, greeting: protocol::Greeting { diff --git a/crates/worker/src/feature/builtin/worker_observation.rs b/crates/worker/src/feature/builtin/worker_observation.rs index 2dc059f6..bda05e09 100644 --- a/crates/worker/src/feature/builtin/worker_observation.rs +++ b/crates/worker/src/feature/builtin/worker_observation.rs @@ -803,7 +803,10 @@ mod tests { .collect(); Ok(WorkerSessionCapture { segment_id: "segment".to_string(), - session: protocol::SessionSnapshot { entries }, + session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), + entries, + }, }) } } diff --git a/crates/worker/src/ipc/interceptor.rs b/crates/worker/src/ipc/interceptor.rs index 30bf4e12..5853a2f7 100644 --- a/crates/worker/src/ipc/interceptor.rs +++ b/crates/worker/src/ipc/interceptor.rs @@ -176,12 +176,16 @@ impl WorkerInterceptor { /// `Item::system_message`s reach the worker via /// `ContinueWith` / `pending_history_appends`, so on-disk order /// matches worker-history order. - fn commit_system_items(&self, items: &[SystemItem]) -> Result<(), session_store::StoreError> { + fn commit_system_items_with_extensions( + &self, + items: &[(SystemItem, Vec)], + ) -> Result<(), session_store::StoreError> { let Some(writer) = self.log_writer.as_ref() else { return Ok(()); }; - for item in items { - let entry = writer.commit_system_item(item.clone())?; + for (item, extensions) in items { + let entry = + writer.commit_system_item_with_extensions(item.clone(), extensions.clone())?; self.pending_committed_history .lock() .expect("pending committed history poisoned") @@ -190,6 +194,16 @@ impl WorkerInterceptor { Ok(()) } + fn commit_system_items(&self, items: &[SystemItem]) -> Result<(), session_store::StoreError> { + self.commit_system_items_with_extensions( + &items + .iter() + .cloned() + .map(|item| (item, Vec::new())) + .collect::>(), + ) + } + fn current_turn_index(&self) -> usize { self.next_turn_index .load(Ordering::Relaxed) @@ -327,7 +341,8 @@ impl Interceptor for WorkerInterceptor { projection_digest: projection.catalog_digest.clone(), logical_name: "internal.notify_wrapper".to_string(), }; - let mut system_items: Vec = Vec::with_capacity(drained.len()); + let mut system_items: Vec<(SystemItem, Vec)> = + Vec::with_capacity(drained.len()); let mut items: Vec = Vec::with_capacity(drained.len()); for entry in &drained { let system_item = match build_system_item_with_provenance( @@ -345,9 +360,9 @@ impl Interceptor for WorkerInterceptor { } }; items.push(system_item.to_history_item()); - system_items.push(system_item); + system_items.push((system_item, entry.extensions())); } - if let Err(error) = self.commit_system_items(&system_items) { + if let Err(error) = self.commit_system_items_with_extensions(&system_items) { self.pending_notifies.requeue_front(drained); return Err(InterceptorError::new( InterceptorErrorCategory::Dependency, diff --git a/crates/worker/src/ipc/notify_buffer.rs b/crates/worker/src/ipc/notify_buffer.rs index 7b97e4ed..cbaedc5d 100644 --- a/crates/worker/src/ipc/notify_buffer.rs +++ b/crates/worker/src/ipc/notify_buffer.rs @@ -25,7 +25,7 @@ use std::collections::VecDeque; use std::sync::{Arc, Mutex}; use protocol::WorkerEvent; -use session_store::SystemItem; +use session_store::{SessionExtension, SystemItem}; use tracing::warn; use crate::prompt::catalog::{CatalogError, PromptCatalog}; @@ -41,8 +41,23 @@ const CAPACITY: usize = 128; /// is available. #[derive(Debug, Clone)] pub enum PendingNotify { - Notify { message: String, auto_run: bool }, - WorkerEvent { event: WorkerEvent }, + Notify { + message: String, + auto_run: bool, + extensions: Vec, + }, + WorkerEvent { + event: WorkerEvent, + }, +} + +impl PendingNotify { + pub(crate) fn extensions(&self) -> Vec { + match self { + PendingNotify::Notify { extensions, .. } => extensions.clone(), + PendingNotify::WorkerEvent { .. } => Vec::new(), + } + } } /// Shared, mutex-guarded buffer of pending entries. @@ -62,7 +77,19 @@ impl NotifyBuffer { /// 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, auto_run: bool) { - self.push_entry(PendingNotify::Notify { message, auto_run }); + self.push_entry(PendingNotify::Notify { + message, + auto_run, + extensions: Vec::new(), + }); + } + + pub fn push_durable_notify(&self, message: String, extension: SessionExtension) { + self.push_entry(PendingNotify::Notify { + message, + auto_run: true, + extensions: vec![extension], + }); } /// Push a typed worker-event entry onto the queue. @@ -202,6 +229,7 @@ mod tests { let entry = PendingNotify::Notify { message: "hello".into(), auto_run: false, + extensions: Vec::new(), }; let catalog = PromptCatalog::builtins_only().unwrap(); let item = build_system_item(&entry, &catalog).unwrap(); diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index d5f7f8d1..c83ef56a 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -57,9 +57,9 @@ pub use session_history::{ }; pub use shared_state::WorkerSharedState; pub use worker::{ - LocalWorkingDirectory, WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN, Worker, WorkerError, - WorkerFilesystemAuthority, WorkerRunResult, WorkerWorkspaceContext, WorkspaceClient, - WorkspaceClientError, WorkspaceId, WorkspaceIdError, WorkspacePromptCatalogResolution, - WorkspaceRequest, WorkspaceRequestMethod, WorkspaceResponse, WorkspaceWorkerDiscoveryRequest, - apply_worker_manifest, marker_workspace_client, unavailable_workspace_client, + LocalWorkingDirectory, Worker, WorkerError, WorkerFilesystemAuthority, WorkerRunResult, + WorkerWorkspaceContext, WorkspaceClient, WorkspaceClientError, WorkspaceId, WorkspaceIdError, + WorkspacePromptCatalogResolution, WorkspaceRequest, WorkspaceRequestMethod, WorkspaceResponse, + WorkspaceWorkerDiscoveryRequest, apply_worker_manifest, marker_workspace_client, + unavailable_workspace_client, }; diff --git a/crates/worker/src/segment_log_sink.rs b/crates/worker/src/segment_log_sink.rs index 91c2792b..20233983 100644 --- a/crates/worker/src/segment_log_sink.rs +++ b/crates/worker/src/segment_log_sink.rs @@ -291,6 +291,7 @@ mod tests { prompt_provenance: None, }, ), + extensions: Vec::new(), } } diff --git a/crates/worker/src/spawn/comm_tools.rs b/crates/worker/src/spawn/comm_tools.rs index 27e6845f..0389f288 100644 --- a/crates/worker/src/spawn/comm_tools.rs +++ b/crates/worker/src/spawn/comm_tools.rs @@ -72,6 +72,7 @@ mod tests { fn snapshot(entries: Vec) -> Event { Event::Snapshot { session: protocol::SessionSnapshot { + pending_submissions: protocol::PendingSubmissionsSnapshot::default(), entries: entries .into_iter() .enumerate() diff --git a/crates/worker/src/spawn/tool.rs b/crates/worker/src/spawn/tool.rs index c2498874..7fab828e 100644 --- a/crates/worker/src/spawn/tool.rs +++ b/crates/worker/src/spawn/tool.rs @@ -58,7 +58,7 @@ struct SubWorkerSpawnInput { /// a host path and grants no authority. When omitted, the Workdir root is used. #[serde(default)] cwd: Option, - /// First message sent to the spawned SubWorker via `Method::Run`. + /// First message sent to the spawned SubWorker via `Method::Submit`. task: String, /// Allow rules delegated to the spawned SubWorker. Must be a subset of the /// spawner's explicit delegation authority; direct tool scope alone is not @@ -235,7 +235,11 @@ impl ParentNotificationTarget { }; tokio::spawn(async move { if let Err(error) = parent_method_tx - .send(Method::Notify { message, auto_run }) + .send(Method::Notify { + notification_request_id: protocol::new_submission_request_id(), + message, + auto_run, + }) .await { tracing::warn!( @@ -1267,6 +1271,7 @@ enabled = false Method::Notify { message, auto_run: true, + .. } if message.contains("SubWorker `reviewer-child` turn ended with status Idle") )); assert!(!runtime.path().join("reviewer-child/sock").exists()); diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index d4a3b85c..cc93bb3a 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -1,3 +1,4 @@ +use std::collections::VecDeque; #[cfg(test)] use std::path::Path; use std::path::PathBuf; @@ -68,6 +69,130 @@ const LARGE_PASTE_INLINE_MAX_BYTES: usize = 32 * 1024; const WORKER_ORCHESTRATION_INSTRUCTION_ID: &str = "worker.orchestration"; const WORKER_ORCHESTRATION_PROMPT_REF: &str = "common.worker_orchestration"; const FEATURE_HOOK_CHAIN_TIMEOUT: Duration = Duration::from_secs(30); +const SESSION_PENDING_ACTIVATIONS_EXTENSION_DOMAIN: &str = "worker.pending_activations.v1"; +const MAX_PENDING_SUBMISSIONS: usize = 32; +const MAX_PENDING_SUBMISSION_BYTES: u64 = 1024 * 1024; +const MAX_PENDING_ARTIFACT_REFS: usize = 64; +const MAX_ACTIVATION_REQUEST_ID_BYTES: usize = 128; +const MAX_SUBMISSION_RECEIPTS: usize = 128; + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub(crate) struct PendingSubmission { + pub(crate) submission_request_id: String, + pub(crate) submission_id: String, + payload_digest: String, + accepted_at_ms: u64, + activation_sequence: u64, + provenance: WorkerHistoryProvenance, + #[serde(default)] + was_queued: bool, + pub(crate) input: Vec, +} + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +struct SubmissionReceipt { + submission_request_id: String, + submission_id: String, + payload_digest: String, + disposition: protocol::SubmissionDisposition, +} + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub(crate) struct PendingNotification { + pub(crate) notification_request_id: String, + pub(crate) message: String, + payload_digest: String, + accepted_at_ms: u64, + activation_sequence: u64, + provenance: WorkerHistoryProvenance, +} + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +struct NotificationReceipt { + notification_request_id: String, + payload_digest: String, +} + +#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] +pub(crate) struct PendingActivationState { + revision: u64, + next_activation_sequence: u64, + /// A prepared activation remains in checkpoints until the same atomic + /// UserInput record commits the clearing checkpoint. Restore puts it back + /// at the FIFO head. + activating: Option, + activating_notification: Option, + pending: VecDeque, + pending_notifications: VecDeque, + receipts: VecDeque, + notification_receipts: VecDeque, +} + +impl PendingActivationState { + pub(crate) fn snapshot(&self) -> protocol::PendingSubmissionsSnapshot { + protocol::PendingSubmissionsSnapshot { + revision: self.revision, + notification_count: u32::try_from(self.pending_notifications.len()).unwrap_or(u32::MAX), + submissions: self + .pending + .iter() + .map(|pending| protocol::PendingSubmissionSummary { + submission_id: pending.submission_id.clone(), + accepted_at_ms: pending.accepted_at_ms, + segment_count: u32::try_from(pending.input.len()).unwrap_or(u32::MAX), + byte_len: submission_payload_len(&pending.input), + }) + .collect(), + } + } + + fn remember_notification_receipt(&mut self, receipt: NotificationReceipt) { + self.notification_receipts.push_back(receipt); + while self.notification_receipts.len() > MAX_SUBMISSION_RECEIPTS { + self.notification_receipts.pop_front(); + } + } + + fn remember_receipt(&mut self, receipt: SubmissionReceipt) { + self.receipts.push_back(receipt); + while self.receipts.len() > MAX_SUBMISSION_RECEIPTS { + self.receipts.pop_front(); + } + } +} + +fn submission_payload_len(input: &[Segment]) -> u64 { + serde_json::to_vec(input) + .map(|bytes| u64::try_from(bytes.len()).unwrap_or(u64::MAX)) + .unwrap_or(u64::MAX) +} + +fn submission_payload_digest(input: &[Segment]) -> String { + use sha2::Digest as _; + sha2::Sha256::digest(serde_json::to_vec(input).unwrap_or_default()) + .iter() + .map(|byte| format!("{byte:02x}")) + .collect() +} + +fn submission_artifact_ref_count(input: &[Segment]) -> usize { + input + .iter() + .filter(|segment| { + matches!( + segment, + Segment::PasteArtifact { .. } | Segment::UploadedFile { .. } + ) + }) + .count() +} + +fn pending_activation_extension(state: &PendingActivationState) -> SessionExtension { + SessionExtension { + domain: SESSION_PENDING_ACTIVATIONS_EXTENSION_DOMAIN.to_owned(), + payload: serde_json::to_value(state).expect("pending activation state must serialize"), + } +} fn hook_run_exit(exit: &EngineRunExit) -> RunCommittedExit { match exit { @@ -970,15 +1095,489 @@ where } } -/// Type-erased commit handle for the interceptor. Lets the -/// interceptor commit `SystemItem`s without being generic over the +#[derive(Debug, Clone)] +pub(crate) enum PendingActivation { + Submission(PendingSubmission), + Notification(PendingNotification), +} + +#[derive(Debug, Clone)] +pub(crate) struct SubmissionAcceptance { + pub(crate) submission_request_id: String, + pub(crate) submission_id: String, + pub(crate) disposition: protocol::SubmissionDisposition, + pub(crate) activation: Option, +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum PendingSubmissionError { + #[error("submission_request_id must not be empty")] + EmptyRequestId, + #[error("activation request id exceeds {MAX_ACTIVATION_REQUEST_ID_BYTES} bytes")] + RequestIdLimit, + #[error("submission input must contain at least one typed segment")] + EmptyInput, + #[error("submission request id was already used with a different payload")] + IdempotencyConflict, + #[error("pending submission queue is full (maximum {MAX_PENDING_SUBMISSIONS})")] + CountLimit, + #[error("pending submission bytes exceed {MAX_PENDING_SUBMISSION_BYTES}")] + ByteLimit, + #[error("pending submission artifact references exceed {MAX_PENDING_ARTIFACT_REFS}")] + ArtifactLimit, + #[error("pending submission not found: {0}")] + NotFound(String), + #[error("pending submission state persistence failed: {0}")] + Store(#[from] StoreError), +} + +#[derive(Clone)] +pub(crate) struct PendingSubmissionHandle { + state: Arc>, + writer: LogWriterHandle, +} + +impl PendingSubmissionHandle +where + St: Store + Clone, +{ + fn persist_locked(&self, state: &PendingActivationState) -> Result<(), PendingSubmissionError> { + self.writer.append_entry_locked(LogEntry::Extension { + ts: segment_log::now_millis(), + domain: SESSION_PENDING_ACTIVATIONS_EXTENSION_DOMAIN.to_owned(), + payload: serde_json::to_value(state).expect("pending activation state must serialize"), + })?; + Ok(()) + } + + pub(crate) fn accept( + &self, + submission_request_id: String, + input: Vec, + activate_now: bool, + ) -> Result { + if submission_request_id.trim().is_empty() { + return Err(PendingSubmissionError::EmptyRequestId); + } + if submission_request_id.len() > MAX_ACTIVATION_REQUEST_ID_BYTES { + return Err(PendingSubmissionError::RequestIdLimit); + } + if input.is_empty() { + return Err(PendingSubmissionError::EmptyInput); + } + let payload_digest = submission_payload_digest(&input); + let _append_guard = self + .writer + .state + .append_lock + .lock() + .expect("segment append lock poisoned"); + let mut current = self + .state + .lock() + .expect("pending activation state poisoned"); + let original = current.clone(); + if let Some(receipt) = current + .receipts + .iter() + .find(|receipt| receipt.submission_request_id == submission_request_id) + { + if receipt.payload_digest != payload_digest { + return Err(PendingSubmissionError::IdempotencyConflict); + } + return Ok(SubmissionAcceptance { + submission_request_id, + submission_id: receipt.submission_id.clone(), + disposition: receipt.disposition, + activation: None, + }); + } + + let submission_id = uuid::Uuid::now_v7().to_string(); + let pending = PendingSubmission { + submission_request_id: submission_request_id.clone(), + submission_id: submission_id.clone(), + payload_digest: payload_digest.clone(), + accepted_at_ms: segment_log::now_millis(), + activation_sequence: current.next_activation_sequence, + provenance: WorkerHistoryProvenance::LegacyUnknown, + was_queued: !activate_now, + input, + }; + current.next_activation_sequence = current.next_activation_sequence.saturating_add(1); + let disposition = if activate_now { + protocol::SubmissionDisposition::Started + } else { + protocol::SubmissionDisposition::Queued + }; + current.remember_receipt(SubmissionReceipt { + submission_request_id: submission_request_id.clone(), + submission_id: submission_id.clone(), + payload_digest, + disposition, + }); + current.revision = current.revision.saturating_add(1); + + if activate_now { + current.activating = Some(pending.clone()); + } else { + let count = current + .pending + .len() + .saturating_add(current.pending_notifications.len()) + .saturating_add(1); + if count > MAX_PENDING_SUBMISSIONS { + *current = original; + return Err(PendingSubmissionError::CountLimit); + } + let bytes = current + .pending + .iter() + .map(|pending| submission_payload_len(&pending.input)) + .sum::() + .saturating_add( + current + .pending_notifications + .iter() + .map(|pending| u64::try_from(pending.message.len()).unwrap_or(u64::MAX)) + .sum::(), + ) + .saturating_add(submission_payload_len(&pending.input)); + if bytes > MAX_PENDING_SUBMISSION_BYTES { + *current = original; + return Err(PendingSubmissionError::ByteLimit); + } + let artifact_refs = current + .pending + .iter() + .map(|pending| submission_artifact_ref_count(&pending.input)) + .sum::() + .saturating_add(submission_artifact_ref_count(&pending.input)); + if artifact_refs > MAX_PENDING_ARTIFACT_REFS { + *current = original; + return Err(PendingSubmissionError::ArtifactLimit); + } + current.pending.push_back(pending.clone()); + if let Err(error) = self.persist_locked(¤t) { + *current = original; + return Err(error); + } + } + Ok(SubmissionAcceptance { + submission_request_id, + submission_id, + disposition, + activation: activate_now.then_some(pending), + }) + } + + pub(crate) fn accept_notification( + &self, + notification_request_id: String, + message: String, + ) -> Result { + if notification_request_id.trim().is_empty() { + return Err(PendingSubmissionError::EmptyRequestId); + } + if notification_request_id.len() > MAX_ACTIVATION_REQUEST_ID_BYTES { + return Err(PendingSubmissionError::RequestIdLimit); + } + let payload_digest = submission_payload_digest(&[Segment::text(message.clone())]); + let _append_guard = self + .writer + .state + .append_lock + .lock() + .expect("segment append lock poisoned"); + let mut state = self + .state + .lock() + .expect("pending activation state poisoned"); + if let Some(receipt) = state + .notification_receipts + .iter() + .find(|receipt| receipt.notification_request_id == notification_request_id) + { + if receipt.payload_digest != payload_digest { + return Err(PendingSubmissionError::IdempotencyConflict); + } + return Ok(false); + } + if state + .pending + .len() + .saturating_add(state.pending_notifications.len()) + >= MAX_PENDING_SUBMISSIONS + { + return Err(PendingSubmissionError::CountLimit); + } + let queued_bytes = state + .pending + .iter() + .map(|pending| submission_payload_len(&pending.input)) + .sum::() + .saturating_add( + state + .pending_notifications + .iter() + .map(|pending| u64::try_from(pending.message.len()).unwrap_or(u64::MAX)) + .sum::(), + ) + .saturating_add(u64::try_from(message.len()).unwrap_or(u64::MAX)); + if queued_bytes > MAX_PENDING_SUBMISSION_BYTES { + return Err(PendingSubmissionError::ByteLimit); + } + let original = state.clone(); + let activation_sequence = state.next_activation_sequence; + state.next_activation_sequence = state.next_activation_sequence.saturating_add(1); + state.pending_notifications.push_back(PendingNotification { + notification_request_id: notification_request_id.clone(), + message, + payload_digest: payload_digest.clone(), + accepted_at_ms: segment_log::now_millis(), + activation_sequence, + provenance: WorkerHistoryProvenance::BackendInstruction { + operation_id: Some(notification_request_id.clone()), + }, + }); + state.remember_notification_receipt(NotificationReceipt { + notification_request_id, + payload_digest, + }); + state.revision = state.revision.saturating_add(1); + if let Err(error) = self.persist_locked(&state) { + *state = original; + return Err(error); + } + Ok(true) + } + + pub(crate) fn prepare_next_activation( + &self, + ) -> Result, PendingSubmissionError> { + let _append_guard = self + .writer + .state + .append_lock + .lock() + .expect("segment append lock poisoned"); + let mut state = self + .state + .lock() + .expect("pending activation state poisoned"); + if state.activating.is_some() || state.activating_notification.is_some() { + return Ok(None); + } + let submission_sequence = state.pending.front().map(|item| item.activation_sequence); + let notification_sequence = state + .pending_notifications + .front() + .map(|item| item.activation_sequence); + if notification_sequence.is_some() + && (submission_sequence.is_none() || notification_sequence < submission_sequence) + { + let notification = state + .pending_notifications + .pop_front() + .expect("notification sequence came from queue head"); + state.activating_notification = Some(notification.clone()); + state.revision = state.revision.saturating_add(1); + return Ok(Some(PendingActivation::Notification(notification))); + } + if submission_sequence.is_some() { + let pending = state + .pending + .pop_front() + .expect("submission sequence came from queue head"); + state.activating = Some(pending.clone()); + state.revision = state.revision.saturating_add(1); + return Ok(Some(PendingActivation::Submission(pending))); + } + Ok(None) + } + + pub(crate) fn abort_activation(&self, pending: PendingSubmission) { + let mut state = self + .state + .lock() + .expect("pending activation state poisoned"); + if state.activating.as_ref().map(|item| &item.submission_id) != Some(&pending.submission_id) + { + return; + } + state.activating = None; + if pending.was_queued { + state.pending.push_front(pending); + } else { + state + .receipts + .retain(|receipt| receipt.submission_id != pending.submission_id); + } + state.revision = state.revision.saturating_add(1); + } + + pub(crate) fn activation_extension(&self) -> SessionExtension { + let state = self + .state + .lock() + .expect("pending activation state poisoned"); + let mut committed = state.clone(); + if let Some(activating) = &committed.activating + && let Some(receipt) = committed + .receipts + .iter_mut() + .find(|receipt| receipt.submission_id == activating.submission_id) + { + receipt.disposition = protocol::SubmissionDisposition::Started; + } + committed.activating = None; + committed.revision = committed.revision.saturating_add(1); + pending_activation_extension(&committed) + } + + pub(crate) fn finish_activation(&self, submission_id: &str) { + let mut state = self + .state + .lock() + .expect("pending activation state poisoned"); + if state + .activating + .as_ref() + .map(|item| item.submission_id.as_str()) + == Some(submission_id) + { + if let Some(receipt) = state + .receipts + .iter_mut() + .find(|receipt| receipt.submission_id == submission_id) + { + receipt.disposition = protocol::SubmissionDisposition::Started; + } + state.activating = None; + state.revision = state.revision.saturating_add(1); + } + } + + pub(crate) fn notification_activation_extension(&self) -> SessionExtension { + let state = self + .state + .lock() + .expect("pending activation state poisoned"); + let mut committed = state.clone(); + committed.activating_notification = None; + committed.revision = committed.revision.saturating_add(1); + pending_activation_extension(&committed) + } + + pub(crate) fn finish_notification_activation(&self, notification_request_id: &str) { + let mut state = self + .state + .lock() + .expect("pending activation state poisoned"); + if state + .activating_notification + .as_ref() + .map(|item| item.notification_request_id.as_str()) + == Some(notification_request_id) + { + state.activating_notification = None; + state.revision = state.revision.saturating_add(1); + } + } + + pub(crate) fn snapshot(&self) -> protocol::PendingSubmissionsSnapshot { + self.state + .lock() + .expect("pending activation state poisoned") + .snapshot() + } + + pub(crate) fn cancel( + &self, + submission_id: &str, + ) -> Result { + let _append_guard = self + .writer + .state + .append_lock + .lock() + .expect("segment append lock poisoned"); + let mut state = self + .state + .lock() + .expect("pending activation state poisoned"); + let original = state.clone(); + let Some(index) = state + .pending + .iter() + .position(|pending| pending.submission_id == submission_id) + else { + return Err(PendingSubmissionError::NotFound(submission_id.to_owned())); + }; + state.pending.remove(index); + state.revision = state.revision.saturating_add(1); + if let Err(error) = self.persist_locked(&state) { + *state = original; + return Err(error); + } + Ok(state.snapshot()) + } + + pub(crate) fn clear( + &self, + ) -> Result { + let _append_guard = self + .writer + .state + .append_lock + .lock() + .expect("segment append lock poisoned"); + let mut state = self + .state + .lock() + .expect("pending activation state poisoned"); + let original = state.clone(); + state.pending.clear(); + state.pending_notifications.clear(); + state.revision = state.revision.saturating_add(1); + if let Err(error) = self.persist_locked(&state) { + *state = original; + return Err(error); + } + Ok(state.snapshot()) + } +} + +impl PendingSubmissionHandle { + #[cfg(test)] + pub(crate) fn for_test(root: &std::path::Path) -> Self { + let store = session_store::FsStore::new(root).expect("test session store"); + let session_id = session_store::new_session_id(); + let segment_id = session_store::new_segment_id(); + store + .create_segment(session_id, segment_id, &[]) + .expect("test session segment"); + Self { + state: Arc::new(Mutex::new(PendingActivationState::default())), + writer: LogWriterHandle { + store, + state: SegmentState::new(session_id, segment_id, 0), + sink: SegmentLogSink::new(), + in_flight: None, + }, + } + } +} + +/// Type-erased commit handle for the interceptor. Lets the interceptor commit `SystemItem`s without being generic over the /// concrete `Store` type. pub trait SystemItemCommitter: Send + Sync { fn commit_log_entry(&self, entry: LogEntry) -> Result<(), StoreError>; - fn commit_system_item( + fn commit_system_item_with_extensions( &self, item: SystemItem, + extensions: Vec, ) -> Result, StoreError> { let metadata = new_history_metadata( WorkerHistoryProvenance::BackendInstruction { operation_id: None }, @@ -991,6 +1590,7 @@ pub trait SystemItemCommitter: Send + Sync { item, metadata: metadata.clone(), }, + extensions, })?; Ok(HistoryEntry::new(history_item, metadata)) } @@ -1027,8 +1627,6 @@ where } } -pub const WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN: &str = "worker.input-submission.v1"; - #[derive(Clone)] struct PreparedFlowProjection { selector: String, @@ -1049,6 +1647,7 @@ pub struct WorkerSession { session_id: SessionId, revision: u64, history: History, + pending_activations: Arc>, } impl WorkerSession { @@ -1058,9 +1657,39 @@ impl WorkerSession { session_id, revision, history: History::from_entries(entries), + pending_activations: Arc::new(Mutex::new(PendingActivationState::default())), } } + fn restore_pending_activations(&mut self, extensions: &[(String, serde_json::Value)]) { + let Some(payload) = extensions.iter().rev().find_map(|(domain, payload)| { + (domain == SESSION_PENDING_ACTIVATIONS_EXTENSION_DOMAIN).then_some(payload) + }) else { + return; + }; + if let Ok(mut state) = serde_json::from_value::(payload.clone()) { + if let Some(activating) = state.activating.take() { + state.pending.push_front(activating); + state.revision = state.revision.saturating_add(1); + } + if let Some(activating) = state.activating_notification.take() { + state.pending_notifications.push_front(activating); + state.revision = state.revision.saturating_add(1); + } + *self + .pending_activations + .lock() + .expect("pending activation state poisoned") = state; + } + } + + pub fn pending_submissions(&self) -> protocol::PendingSubmissionsSnapshot { + self.pending_activations + .lock() + .expect("pending activation state poisoned") + .snapshot() + } + pub fn session_id(&self) -> SessionId { self.session_id } @@ -1309,6 +1938,21 @@ impl Worker { } } + pub(crate) fn pending_activation_state(&self) -> Arc> { + self.session.pending_activations.clone() + } + + pub(crate) fn pending_submission_handle(&self) -> PendingSubmissionHandle { + PendingSubmissionHandle { + state: self.session.pending_activations.clone(), + writer: self.log_writer_handle(), + } + } + + pub fn pending_submissions(&self) -> protocol::PendingSubmissionsSnapshot { + self.session.pending_submissions() + } + /// Attach a type-erased system-item commit handle. The controller /// calls this once during spawn so the interceptor can commit /// `SystemItem`s directly without owning a generic store handle. @@ -1670,6 +2314,7 @@ impl Worker { }, metadata: skill_metadata.clone(), }, + extensions: Vec::new(), })?; let history_entry = HistoryEntry::new(agen::Item::system_message(body), skill_metadata); let mut annotate = history_annotator( @@ -1960,6 +2605,28 @@ impl Worker { .truncate(loc.session_id, loc.segment_id, truncate_entries)?; self.segment_state.set_entries_written(truncate_entries); self.sink.truncate_silent(truncate_entries); + let pending_state = self + .session + .pending_activations + .lock() + .expect("pending activation state poisoned") + .clone(); + if !pending_state.pending.is_empty() + || pending_state.activating.is_some() + || pending_state.activating_notification.is_some() + || !pending_state.receipts.is_empty() + { + let checkpoint = LogEntry::Extension { + ts: segment_log::now_millis(), + domain: SESSION_PENDING_ACTIVATIONS_EXTENSION_DOMAIN.to_owned(), + payload: serde_json::to_value(&pending_state).map_err(|error| { + RewindError::Invalid(format!( + "serialize pending submissions during rewind: {error}" + )) + })?, + }; + self.commit_entry(checkpoint)?; + } let history_entries = restore_history_entries(loc.session_id, loc.segment_id, &retained) .map_err(|error| RewindError::Invalid(error.into()))?; @@ -2525,7 +3192,7 @@ impl Worker { /// Convenience: run with a single `Segment::Text`. /// /// Equivalent to `run(vec![Segment::text(s)])`. The dumb-client - /// counterpart of [`protocol::Method::run_text`]; primarily for + /// counterpart of [`protocol::Method::submit_text`]; primarily for /// tests and tools that have only a string in hand. pub async fn run_text(&mut self, s: impl Into) -> Result where @@ -3042,6 +3709,7 @@ impl Worker { }, metadata: interrupt_metadata.clone(), }, + extensions: Vec::new(), })?; let interrupt_entry = HistoryEntry::new(agen::Item::system_message(system_note), interrupt_metadata); @@ -4428,6 +5096,22 @@ impl Worker { { initial_entries.push(checkpoint); } + initial_entries.push(LogEntry::Extension { + ts: segment_log::now_millis(), + domain: SESSION_PENDING_ACTIVATIONS_EXTENSION_DOMAIN.to_owned(), + payload: serde_json::to_value( + &*self + .session + .pending_activations + .lock() + .expect("pending activation state poisoned"), + ) + .map_err(|error| { + WorkerError::InvalidState(format!( + "serialize pending submissions during compaction: {error}" + )) + })?, + }); if let Some(flow_state) = self .flow_runtime_state .lock() @@ -5248,6 +5932,9 @@ where history_persistence_wired: false, log_writer: None, }; + worker + .session + .restore_pending_activations(&state.extensions); worker.apply_permissions_from_manifest(); worker.apply_prune_from_manifest(); worker.write_worker_metadata_active(SegmentLocation { @@ -8453,6 +9140,164 @@ mod build_summary_prompt_tests { ); } + #[test] + fn pending_submission_queue_is_durable_idempotent_and_bounded() { + let temp = tempfile::tempdir().unwrap(); + let handle = PendingSubmissionHandle::for_test(temp.path()); + let input = vec![Segment::text("queued")]; + let accepted = handle + .accept("request-1".into(), input.clone(), false) + .unwrap(); + assert_eq!( + accepted.disposition, + protocol::SubmissionDisposition::Queued + ); + assert_eq!(handle.snapshot().submissions.len(), 1); + + let replay = handle + .accept("request-1".into(), input.clone(), false) + .unwrap(); + assert_eq!(replay.submission_id, accepted.submission_id); + assert!(replay.activation.is_none()); + assert_eq!(handle.snapshot().submissions.len(), 1); + assert!(matches!( + handle.accept("request-1".into(), vec![Segment::text("different")], false), + Err(PendingSubmissionError::IdempotencyConflict) + )); + + let entries = handle + .writer + .store + .read_all( + handle.writer.state.session_id(), + handle.writer.state.segment_id(), + ) + .unwrap(); + let payload = entries + .iter() + .rev() + .find_map(|entry| match entry { + LogEntry::Extension { + domain, payload, .. + } if domain == SESSION_PENDING_ACTIVATIONS_EXTENSION_DOMAIN => { + Some(payload.clone()) + } + _ => None, + }) + .unwrap(); + let restored: PendingActivationState = serde_json::from_value(payload).unwrap(); + assert_eq!(restored.pending.len(), 1); + assert_eq!(restored.pending[0].submission_id, accepted.submission_id); + + let snapshot = handle.cancel(&accepted.submission_id).unwrap(); + assert!(snapshot.submissions.is_empty()); + assert!(matches!( + handle.cancel(&accepted.submission_id), + Err(PendingSubmissionError::NotFound(_)) + )); + + for index in 0..MAX_PENDING_SUBMISSIONS { + handle + .accept( + format!("limit-{index}"), + vec![Segment::text(format!("value-{index}"))], + false, + ) + .unwrap(); + } + assert!(matches!( + handle.accept("over-limit".into(), vec![Segment::text("too much")], false), + Err(PendingSubmissionError::CountLimit) + )); + assert_eq!(handle.snapshot().submissions.len(), MAX_PENDING_SUBMISSIONS); + let cleared = handle.clear().unwrap(); + assert!(cleared.submissions.is_empty()); + assert_eq!(cleared.notification_count, 0); + } + + #[test] + fn notification_and_submit_share_activation_order_and_notification_dedupes() { + let temp = tempfile::tempdir().unwrap(); + let handle = PendingSubmissionHandle::for_test(temp.path()); + assert!( + handle + .accept_notification("notification-1".into(), "notice".into()) + .unwrap() + ); + assert!( + !handle + .accept_notification("notification-1".into(), "notice".into()) + .unwrap() + ); + assert!(matches!( + handle.accept_notification("notification-1".into(), "different".into()), + Err(PendingSubmissionError::IdempotencyConflict) + )); + handle + .accept("request-1".into(), vec![Segment::text("submit")], false) + .unwrap(); + + let first = handle.prepare_next_activation().unwrap().unwrap(); + assert!(matches!( + first, + PendingActivation::Notification(PendingNotification { ref message, .. }) + if message == "notice" + )); + let committed = handle.notification_activation_extension(); + let committed_state: PendingActivationState = + serde_json::from_value(committed.payload).unwrap(); + assert!(committed_state.pending_notifications.is_empty()); + assert!(committed_state.activating_notification.is_none()); + handle.finish_notification_activation("notification-1"); + let second = handle.prepare_next_activation().unwrap().unwrap(); + assert!(matches!(second, PendingActivation::Submission(_))); + } + + #[test] + fn restoring_an_in_flight_activation_requeues_it_at_the_fifo_head() { + let mut session = WorkerSession::new(session_store::new_session_id(), Vec::new()); + let state = PendingActivationState { + revision: 4, + next_activation_sequence: 2, + activating: Some(PendingSubmission { + submission_request_id: "request-1".into(), + submission_id: "submission-1".into(), + payload_digest: submission_payload_digest(&[Segment::text("first")]), + accepted_at_ms: 1, + activation_sequence: 0, + provenance: WorkerHistoryProvenance::LegacyUnknown, + was_queued: false, + input: vec![Segment::text("first")], + }), + activating_notification: None, + pending: VecDeque::from([PendingSubmission { + submission_request_id: "request-2".into(), + submission_id: "submission-2".into(), + payload_digest: submission_payload_digest(&[Segment::text("second")]), + accepted_at_ms: 2, + activation_sequence: 1, + provenance: WorkerHistoryProvenance::LegacyUnknown, + was_queued: true, + input: vec![Segment::text("second")], + }]), + pending_notifications: VecDeque::new(), + receipts: VecDeque::new(), + notification_receipts: VecDeque::new(), + }; + session.restore_pending_activations(&[( + SESSION_PENDING_ACTIVATIONS_EXTENSION_DOMAIN.into(), + serde_json::to_value(state).unwrap(), + )]); + let state = session + .pending_activations + .lock() + .expect("pending activation state poisoned"); + assert!(state.activating.is_none()); + assert_eq!(state.pending.len(), 2); + assert_eq!(state.pending[0].submission_id, "submission-1"); + assert_eq!(state.pending[1].submission_id, "submission-2"); + } + fn minimal_manifest() -> WorkerManifest { let toml_str = r#" [worker] diff --git a/crates/worker/tests/compact_events_test.rs b/crates/worker/tests/compact_events_test.rs index 47fbd351..8265f9e2 100644 --- a/crates/worker/tests/compact_events_test.rs +++ b/crates/worker/tests/compact_events_test.rs @@ -630,7 +630,10 @@ async fn controller_compact_method_emits_start_and_done() { let mut rx = handle.subscribe(); handle - .send(Method::run_text("seed history")) + .send(Method::submit_text( + protocol::new_submission_request_id(), + "seed history", + )) .await .expect("send run"); loop { diff --git a/crates/worker/tests/controller_test.rs b/crates/worker/tests/controller_test.rs index 403ca3cb..6faa2fdd 100644 --- a/crates/worker/tests/controller_test.rs +++ b/crates/worker/tests/controller_test.rs @@ -617,7 +617,13 @@ async fn feature_flags_default_to_core_tool_surface_only() { let worker = make_worker(client).await; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Idle).await; let request = wait_for_captured_request(&client_for_assert).await; @@ -672,7 +678,13 @@ permission = "write" let worker = make_worker_with_pwd_and_manifest(client, manifest).await.0; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Idle).await; let request = wait_for_captured_request(&client_for_assert).await; @@ -758,7 +770,13 @@ permission = "write" let worker = make_worker_with_pwd_and_manifest(client, &manifest).await.0; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Idle).await; let request = wait_for_captured_request(&client_for_assert).await; @@ -814,7 +832,13 @@ async fn builtin_orchestrator_exposes_worker_remove_and_workdir_delete() { .await; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Idle).await; let request = wait_for_captured_request(&client_for_assert).await; let installed = request_tool_names(&request); @@ -863,7 +887,13 @@ permission = "write" .0; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Idle).await; let request = wait_for_captured_request(&client_for_assert).await; @@ -916,7 +946,13 @@ permission = "write" ) .await; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Idle).await; let request = wait_for_captured_request(&client_for_assert).await; let names = request_tool_names(&request); @@ -963,7 +999,13 @@ async fn run_end_returns_to_idle_without_busy_status() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); let mut saw_run_end = false; let mut saw_idle_status = false; @@ -1005,7 +1047,13 @@ async fn provider_stream_error_records_run_errored() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("ping")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "ping", + )) + .await + .unwrap(); assert!( drain_until(&mut rx, std::time::Duration::from_secs(2), |e| matches!( @@ -1054,7 +1102,10 @@ async fn snapshot_includes_user_input_for_in_flight_turn() { let mut events = handle.subscribe(); handle - .send(Method::run_text("hello in-flight")) + .send(Method::submit_text( + protocol::new_submission_request_id(), + "hello in-flight", + )) .await .unwrap(); tokio::time::timeout(std::time::Duration::from_secs(2), async { @@ -1119,7 +1170,13 @@ async fn attach_snapshot_includes_current_status() { let worker = make_worker(client).await; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Running).await; let stream = tokio::net::UnixStream::connect(handle.runtime_dir.socket_path()) @@ -1157,7 +1214,13 @@ async fn run_updates_shared_state_to_idle_after_completion() { let worker = make_worker(client).await; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); // Wait for the run to complete tokio::time::sleep(std::time::Duration::from_millis(100)).await; @@ -1171,7 +1234,13 @@ async fn run_populates_history() { let worker = make_worker(client).await; let handle = spawn_controller(worker).await; - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); tokio::time::sleep(std::time::Duration::from_millis(100)).await; @@ -1189,7 +1258,13 @@ async fn events_are_broadcast() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("Hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); let mut saw_turn_start = false; let mut saw_text_delta = false; @@ -1224,10 +1299,8 @@ async fn events_are_broadcast() { } #[tokio::test] -async fn double_run_returns_error() { - // Keep the first turn in-flight until the test drops the handle. A - // finite stream can finish before the second Method reaches the - // controller in the full test suite, making this assertion racy. +async fn submit_while_running_is_durably_queued() { + // Keep the first turn in-flight until the second Submit is accepted. let events = vec![ LlmEvent::text_block_start(0), LlmEvent::text_delta(0, "slow..."), @@ -1237,35 +1310,44 @@ async fn double_run_returns_error() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - // Send first run and wait until the controller has entered Running. - handle.send(Method::run_text("first")).await.unwrap(); + handle + .send(Method::submit_text("request-first", "first")) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Running).await; + handle + .send(Method::submit_text("request-second", "second")) + .await + .unwrap(); - // Now the second run must be rejected by drive_turn's live Method arm. - handle.send(Method::run_text("second")).await.unwrap(); - - // Look for the error event - let mut saw_already_running = false; let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(2); - loop { - tokio::select! { - event = rx.recv() => { - match event { - Ok(Event::Error { code, .. }) => { - if code == worker::ErrorCode::AlreadyRunning { - saw_already_running = true; - break; - } - } - Err(_) => break, - _ => {} - } + let mut accepted = None; + let mut pending_count = None; + while tokio::time::Instant::now() < deadline { + match tokio::time::timeout(std::time::Duration::from_millis(100), rx.recv()).await { + Ok(Ok(Event::SubmissionAccepted { + submission_request_id, + disposition, + .. + })) if submission_request_id == "request-second" => accepted = Some(disposition), + Ok(Ok(Event::PendingSubmissionsChanged { pending })) + if pending.submissions.len() == 1 => + { + pending_count = Some(1) } - _ = tokio::time::sleep_until(deadline) => break, + Ok(Ok(Event::Error { code, message })) if code == worker::ErrorCode::AlreadyRunning => { + panic!("Submit was busy-rejected: {message}") + } + _ => {} + } + if accepted.is_some() && pending_count.is_some() { + break; } } - assert!(saw_already_running, "should see already_running error"); + assert_eq!(accepted, Some(protocol::SubmissionDisposition::Queued)); + assert_eq!(pending_count, Some(1)); + handle.send(Method::Pause).await.unwrap(); } #[tokio::test] @@ -1353,7 +1435,8 @@ async fn run_with_paste_segment_inlines_content_and_emits_typed_user_message() { protocol::Segment::text(" thanks"), ]; handle - .send(Method::Run { + .send(Method::Submit { + submission_request_id: protocol::new_submission_request_id(), input: segments.clone(), }) .await @@ -1425,7 +1508,13 @@ async fn run_with_resolvable_file_ref_attaches_system_message_after_user() { path: "notes.md".into(), }, ]; - handle.send(Method::Run { input: segments }).await.unwrap(); + handle + .send(Method::Submit { + submission_request_id: protocol::new_submission_request_id(), + input: segments, + }) + .await + .unwrap(); // Wait for the turn to complete. let mut rx = handle.subscribe(); @@ -1473,7 +1562,8 @@ async fn run_with_file_ref_uses_manifest_file_upload_limit() { let handle = spawn_controller(worker).await; handle - .send(Method::Run { + .send(Method::Submit { + submission_request_id: protocol::new_submission_request_id(), input: vec![protocol::Segment::FileRef { path: "long.txt".into(), }], @@ -1526,7 +1616,13 @@ async fn run_with_unresolved_segment_emits_alert_and_placeholder() { path: "src/lib.rs".into(), }, ]; - handle.send(Method::Run { input: segments }).await.unwrap(); + handle + .send(Method::Submit { + submission_request_id: protocol::new_submission_request_id(), + input: segments, + }) + .await + .unwrap(); let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(2); let mut saw_alert_for_file_ref = false; @@ -1574,6 +1670,7 @@ async fn notify_while_idle_auto_starts_turn_and_injects_system_message() { handle .send(Method::Notify { + notification_request_id: protocol::new_submission_request_id(), message: "turn finished".into(), auto_run: true, }) @@ -1614,6 +1711,19 @@ async fn notify_while_idle_auto_starts_turn_and_injects_system_message() { saw_notify_in_mirror, "Method::Notify should commit a SystemItem::Notification entry; mirror = {entries:?}" ); + let queue_checkpoint_is_atomic = entries.iter().any(|entry| match entry { + LogEntry::AnnotatedSystemItem { extensions, .. } => extensions.iter().any(|extension| { + extension.domain == "worker.pending_activations.v1" + && extension.payload["pending_notifications"] + .as_array() + .is_some_and(Vec::is_empty) + }), + _ => false, + }); + assert!( + queue_checkpoint_is_atomic, + "notification history and queue claim must share one log entry" + ); // Exactly one request was made; it must contain the formatted // notification as one of the items (committed to history by @@ -1662,6 +1772,7 @@ async fn notify_while_idle_with_auto_run_false_waits_for_explicit_run() { handle .send(Method::Notify { + notification_request_id: protocol::new_submission_request_id(), message: "progress snapshot".into(), auto_run: false, }) @@ -1675,7 +1786,13 @@ async fn notify_while_idle_with_auto_run_false_waits_for_explicit_run() { "weak Notify must not stage RunForNotification while idle" ); - handle.send(Method::run_text("continue")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "continue", + )) + .await + .unwrap(); let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(2); loop { if !client_for_assert.captured_requests().is_empty() { @@ -1855,9 +1972,16 @@ async fn notify_while_running_does_not_emit_already_running_error() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("start")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "start", + )) + .await + .unwrap(); handle .send(Method::Notify { + notification_request_id: protocol::new_submission_request_id(), message: "ping".into(), auto_run: true, }) @@ -1924,7 +2048,13 @@ async fn socket_run_receives_events() { let mut writer = JsonLineWriter::new(writer); // Send run method via socket - writer.write(&Method::run_text("Hello")).await.unwrap(); + writer + .write(&Method::submit_text( + protocol::new_submission_request_id(), + "Hello", + )) + .await + .unwrap(); // Collect events let mut saw_turn_start = false; @@ -2231,7 +2361,13 @@ async fn pause_then_resume_transitions_and_preserves_history_consistency() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("hello")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "hello", + )) + .await + .unwrap(); // Wait for the partial text_delta to confirm the first stream is // live before we pause. @@ -2320,7 +2456,7 @@ async fn pause_then_resume_transitions_and_preserves_history_consistency() { assert!(!has_tool_call, "no orphan tool_call in history"); } -/// Paused with an orphan `tool_use` in history + a fresh `Method::Run` +/// Paused with an orphan `tool_use` in history + a fresh `Method::Submit` /// must produce a wire-valid next LLM request: the orphan is closed /// with a synthetic `tool_result`, a system note is inserted, and the /// new user input is appended. @@ -2357,7 +2493,13 @@ async fn paused_then_run_closes_orphan_tool_use_for_next_request() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("first")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "first", + )) + .await + .unwrap(); // Wait for ToolCallDone — the ToolCall is committed to history // right before the Engine enters tool execution and pends. @@ -2388,7 +2530,13 @@ async fn paused_then_run_closes_orphan_tool_use_for_next_request() { // `last_run_interrupted` and runs its interrupt-prep step, which // closes the orphan + injects a system note before the fresh user // message. - handle.send(Method::run_text("new request")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "new request", + )) + .await + .unwrap(); assert!( drain_until(&mut rx, std::time::Duration::from_secs(2), |e| matches!( e, @@ -2519,7 +2667,13 @@ async fn paused_cancel_abandons_resume_and_next_input_is_fresh_run() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("first")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "first", + )) + .await + .unwrap(); assert!( drain_until(&mut rx, std::time::Duration::from_secs(2), |e| matches!( e, @@ -2587,7 +2741,10 @@ async fn paused_cancel_abandons_resume_and_next_input_is_fresh_run() { ); handle - .send(Method::run_text("fresh request")) + .send(Method::submit_text( + protocol::new_submission_request_id(), + "fresh request", + )) .await .unwrap(); assert!( @@ -2676,7 +2833,13 @@ async fn empty_turn_cancel_rolls_back_submit_entries_and_emits_signal() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("rollback me")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "rollback me", + )) + .await + .unwrap(); wait_for_status(&handle, WorkerStatus::Running).await; handle.send(Method::Cancel).await.unwrap(); @@ -2709,7 +2872,10 @@ async fn empty_turn_pause_rolls_back_and_snapshot_does_not_restore_input() { let mut rx = handle.subscribe(); handle - .send(Method::run_text("pause rollback")) + .send(Method::submit_text( + protocol::new_submission_request_id(), + "pause rollback", + )) .await .unwrap(); wait_for_status(&handle, WorkerStatus::Running).await; @@ -2743,7 +2909,13 @@ async fn empty_turn_rollback_removes_only_the_most_recent_turn() { let handle = spawn_controller(worker).await; let mut rx = handle.subscribe(); - handle.send(Method::run_text("first kept")).await.unwrap(); + handle + .send(Method::submit_text( + protocol::new_submission_request_id(), + "first kept", + )) + .await + .unwrap(); assert!( drain_until(&mut rx, std::time::Duration::from_secs(2), |e| matches!( e, @@ -2757,7 +2929,10 @@ async fn empty_turn_rollback_removes_only_the_most_recent_turn() { wait_for_status(&handle, WorkerStatus::Idle).await; handle - .send(Method::run_text("second rolled back")) + .send(Method::submit_text( + protocol::new_submission_request_id(), + "second rolled back", + )) .await .unwrap(); wait_for_status(&handle, WorkerStatus::Running).await; @@ -2804,7 +2979,10 @@ async fn pause_after_assistant_token_does_not_rollback() { let mut rx = handle.subscribe(); handle - .send(Method::run_text("keep this turn")) + .send(Method::submit_text( + protocol::new_submission_request_id(), + "keep this turn", + )) .await .unwrap(); assert!( diff --git a/crates/workspace-server/src/hosts.rs b/crates/workspace-server/src/hosts.rs index 7f530a8d..f86a1d00 100644 --- a/crates/workspace-server/src/hosts.rs +++ b/crates/workspace-server/src/hosts.rs @@ -533,7 +533,7 @@ fn initial_worker_input(segments: &[Segment]) -> Option { Some(EmbeddedWorkerInput { kind: EmbeddedWorkerInputKind::User, content: Segment::flatten_to_text(segments), - submission_id: None, + submission_request_id: None, segments: Some(segments.to_vec()), }) } @@ -2625,7 +2625,7 @@ impl WorkspaceWorkerRuntime for EmbeddedWorkerRuntime { WorkerInputKind::RegisterPeer => EmbeddedWorkerInputKind::RegisterPeer, }, content: request.content, - submission_id: None, + submission_request_id: None, segments: request.segments, }; match self.runtime.send_input(&worker_ref, input) { @@ -3726,7 +3726,7 @@ impl WorkspaceWorkerRuntime for RemoteWorkerRuntime { WorkerInputKind::RegisterPeer => EmbeddedWorkerInputKind::RegisterPeer, }, content: request.content, - submission_id: None, + submission_request_id: None, segments: request.segments, }; match self.post_json::<_, RuntimeHttpWorkerInputResponse>( diff --git a/docs/development/work-items.md b/docs/development/work-items.md index 61c174ff..34bec74f 100644 --- a/docs/development/work-items.md +++ b/docs/development/work-items.md @@ -286,7 +286,7 @@ User triggers a Ticket action in yoi panel -> client Ticket role launcher reads .yoi/workspace.toml [ticket] settings -> launcher selects the role Profile -> launcher spawns the role Worker - -> launcher sends Method::Run with Text segments + -> launcher sends Method::Submit with Text segments -> launcher waits for run-acceptance evidence -> Dashboard reports success/failure ``` diff --git a/docs/report/test-validity-20260612/pod.md b/docs/report/test-validity-20260612/pod.md index d5c5cfc8..087ea4d2 100644 --- a/docs/report/test-validity-20260612/pod.md +++ b/docs/report/test-validity-20260612/pod.md @@ -34,7 +34,7 @@ - いくつかの async/socket tests はまだ fixed sleeps と process-wide environment mutation (`YOI_RUNTIME_DIR`, `YOI_HOME`, `XDG_RUNTIME_DIR`) に依存している。一部ファイルでは env-changing tests を guards で serialize しているが、fixed timing と global env は高負荷 CI や parallel execution 下で flakiness risk のまま。 - Real provider wire behavior は意図的に `pod` の外側だが、crate と streaming edge cases の interaction はまだ大部分が mock されている。tests は重要な `Worker` outcomes をカバーしているが、malformed/partial provider streams を real provider adapter 経由では exercise していない。 - Prompt tests は数が多く有用だが、一部は behavior-coupled というより prose-coupled である。critical safety wording に対して、その文字列を意図的に stable contract として扱う場合だけこれは許容できる。そうでなければ maintenance noise になる。 -- startup profile resolution、socket server、`Method::Run`、session persistence、shutdown、restore を跨ぐ full lifecycle integration は slice ごとにしかカバーされておらず、1 つの scenario としてはカバーされていない。 +- startup profile resolution、socket server、`Method::Submit`、session persistence、shutdown、restore を跨ぐ full lifecycle integration は slice ごとにしかカバーされておらず、1 つの scenario としてはカバーされていない。 ## 追加を提案するもの diff --git a/web/workspace/src/lib/generated/protocol.ts b/web/workspace/src/lib/generated/protocol.ts index 8cd81901..1e744d20 100644 --- a/web/workspace/src/lib/generated/protocol.ts +++ b/web/workspace/src/lib/generated/protocol.ts @@ -103,7 +103,13 @@ entry_id: string, */ timestamp: number, provenance: SessionEntryProvenance, derived_from?: Array, } & ({ "kind": "user_input", segments: Array, } | { "kind": "message", role: SessionMessageRole, content: Array, } | { "kind": "tool_call", call_id: string, name: string, arguments: string, } | { "kind": "tool_result", call_id: string, summary: string, content?: string | null, is_error: boolean, attachments?: Array, } | { "kind": "system_item", item_kind: string, content: string, data?: unknown, } | { "kind": "run_error", message: string, }); -export type SessionSnapshot = { entries: Array, }; +export type PendingSubmissionSummary = { submission_id: string, accepted_at_ms: number, segment_count: number, byte_len: number, }; + +export type PendingSubmissionsSnapshot = { revision: number, notification_count: number, submissions: Array, }; + +export type SubmissionDisposition = "started" | "queued"; + +export type SessionSnapshot = { pending_submissions: PendingSubmissionsSnapshot, entries: Array, }; export type InternalWorkerKind = "sub_worker" | { "service": { kind: string, } }; @@ -225,9 +231,9 @@ export type SubscriptionFramePayload = { "frame": "request", "message": Subscrip export type SubscriptionFrame = { protocol_version: number, } & ({ "frame": "request", "message": SubscriptionRequest } | { "frame": "response", "message": SubscriptionResponse } | { "frame": "event", "message": SubscriptionEvent } | { "frame": "worker_protocol", "message": SubscriptionWorkerProtocolMethod }); -export type Method = { "method": "run", "params": { input: Array, } } | { "method": "notify", "params": { message: string, auto_run?: boolean, } } | { "method": "worker_event", "params": WorkerEvent } | { "method": "resume" } | { "method": "cancel" } | { "method": "pause" } | { "method": "compact" } | { "method": "list_rewind_targets" } | { "method": "rewind_to", "params": { target: RewindTargetId, expected_head_entries: number, } } | { "method": "shutdown" } | { "method": "list_completions", "params": { kind: CompletionKind, prefix: string, } } | { "method": "list_workers" } | { "method": "restore_worker", "params": { name: string, } } | { "method": "register_peer", "params": { name: string, } }; +export type Method = { "method": "submit", "params": { submission_request_id: string, input: Array, } } | { "method": "notify", "params": { notification_request_id: string, message: string, auto_run?: boolean, } } | { "method": "worker_event", "params": WorkerEvent } | { "method": "list_pending_submissions" } | { "method": "cancel_pending_submission", "params": { submission_id: string, } } | { "method": "clear_pending_submissions" } | { "method": "continue_pending" } | { "method": "resume" } | { "method": "cancel" } | { "method": "pause" } | { "method": "compact" } | { "method": "list_rewind_targets" } | { "method": "rewind_to", "params": { target: RewindTargetId, expected_head_entries: number, } } | { "method": "shutdown" } | { "method": "list_completions", "params": { kind: CompletionKind, prefix: string, } } | { "method": "list_workers" } | { "method": "restore_worker", "params": { name: string, } } | { "method": "register_peer", "params": { name: string, } }; -export type Event = { "event": "user_message", "data": { segments: Array, } } | { "event": "system_item", "data": { item: unknown, } } | { "event": "invoke_start", "data": { kind: InvokeKind, } } | { "event": "turn_start", "data": { turn: number, } } | { "event": "turn_end", "data": { turn: number, result: TurnResult, } } | { "event": "llm_call_start", "data": { llm_call: number, } } | { "event": "llm_call_end", "data": { llm_call: number, } } | { "event": "llm_retry", "data": { llm_call: number, +export type Event = { "event": "submission_accepted", "data": { submission_request_id: string, submission_id: string, disposition: SubmissionDisposition, } } | { "event": "submission_rejected", "data": { submission_request_id: string, message: string, } } | { "event": "pending_submissions_changed", "data": { pending: PendingSubmissionsSnapshot, } } | { "event": "user_message", "data": { segments: Array, } } | { "event": "system_item", "data": { item: unknown, } } | { "event": "invoke_start", "data": { kind: InvokeKind, } } | { "event": "turn_start", "data": { turn: number, } } | { "event": "turn_end", "data": { turn: number, result: TurnResult, } } | { "event": "llm_call_start", "data": { llm_call: number, } } | { "event": "llm_call_end", "data": { llm_call: number, } } | { "event": "llm_retry", "data": { llm_call: number, /** * The attempt that just failed. 1 origin. */ diff --git a/web/workspace/src/lib/workspace/console/model.test.ts b/web/workspace/src/lib/workspace/console/model.test.ts index e2dfdceb..da4f70d5 100644 --- a/web/workspace/src/lib/workspace/console/model.test.ts +++ b/web/workspace/src/lib/workspace/console/model.test.ts @@ -2150,6 +2150,7 @@ Deno.test("snapshot restores TaskStore state from system history", () => { const event = snapshotEvent("/repo"); if (event.event !== "snapshot") throw new Error("snapshot fixture expected"); event.data.session = { + pending_submissions: { revision: 0, notification_count: 0, submissions: [] }, entries: [{ entry_id: "task-reminder-1", timestamp: 1, diff --git a/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts b/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts index 869603f3..b031f1d0 100644 --- a/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts +++ b/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts @@ -1059,3 +1059,25 @@ Deno.test("Web Console switches main and direct SubWorker views from the Tasks r "Worker view selection should expose only direct SubWorker session identities with main fallback", ); }); + +Deno.test("Web Console uses Notify while running and exposes durable pending controls", async () => { + const consolePage = await Deno.readTextFile( + new URL( + "./../../../routes/w/[workspaceId]/runtimes/[runtimeId]/workers/[workerId]/console/+page.svelte", + import.meta.url, + ), + ); + + for (const token of [ + 'method: "submit"', + 'method: "notify"', + "notification_request_id: crypto.randomUUID()", + "submission_request_id: crypto.randomUUID()", + 'payload.event === "pending_submissions_changed"', + 'method: "cancel_pending_submission"', + 'method: "clear_pending_submissions"', + 'method: "continue_pending"', + ]) { + assert(consolePage.includes(token), `missing durable pending control token: ${token}`); + } +}); diff --git a/web/workspace/src/routes/w/[workspaceId]/runtimes/[runtimeId]/workers/[workerId]/console/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/runtimes/[runtimeId]/workers/[workerId]/console/+page.svelte index 03efcf8d..6c50e2ae 100644 --- a/web/workspace/src/routes/w/[workspaceId]/runtimes/[runtimeId]/workers/[workerId]/console/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/runtimes/[runtimeId]/workers/[workerId]/console/+page.svelte @@ -31,7 +31,13 @@ type ConsoleViewMode, type ConsoleViewScroll, } from "$lib/workspace/console/model"; - import type { Event as ProtocolEvent, Method as ProtocolMethod, RewindTarget, Segment } from "$lib/generated/protocol"; + import type { + Event as ProtocolEvent, + Method as ProtocolMethod, + PendingSubmissionsSnapshot, + RewindTarget, + Segment, + } from "$lib/generated/protocol"; import { MAX_FILES_PER_SUBMISSION, uploadAttachment, @@ -152,6 +158,12 @@ "connecting", ); let protocolSubscription: WorkspaceMultiplexerSubscription | null = null; + let pendingSubmissions = $state({ + revision: 0, + notification_count: 0, + submissions: [], + }); + let pendingSubmissionItems = $derived(pendingSubmissions.submissions ?? []); let pendingCompletionRequest: { resolve: (entries: ComposerCompletionEntry[]) => void; reject: (error: Error) => void; @@ -334,6 +346,13 @@ function handleIncomingProtocolEvent(payload: ProtocolEvent) { handleProtocolCommandEvent(payload); + if (payload.event === "snapshot") { + pendingSubmissions = payload.data.session.pending_submissions; + } else if (payload.event === "segment_rotated") { + pendingSubmissions = payload.data.session.pending_submissions; + } else if (payload.event === "pending_submissions_changed") { + pendingSubmissions = payload.data.pending; + } if (payload.event === "error") { queueObservationDiagnostic({ code: payload.data.code, @@ -555,9 +574,20 @@ ): ProtocolMethod { switch (request.kind) { case "user": + if (workerRunning) { + return { + method: "notify", + params: { + notification_request_id: crypto.randomUUID(), + message: request.content, + auto_run: true, + }, + }; + } return { - method: "run", + method: "submit", params: { + submission_request_id: crypto.randomUUID(), input: request.segments ?? [ { kind: "text", content: request.content }, ], @@ -566,7 +596,11 @@ case "notify": return { method: "notify", - params: { message: request.content, auto_run: true }, + params: { + notification_request_id: crypto.randomUUID(), + message: request.content, + auto_run: true, + }, }; case "compact": return { method: "compact" }; @@ -779,7 +813,7 @@ composerInputElement?.recordHistory(value); composerInputElement?.clear(); attachments = []; - if (method.method === "run" || method.method === "notify") { + if (method.method === "submit" || method.method === "notify") { liveWorkerState = "running"; } composerNotice = "Sent through Worker protocol."; @@ -1722,6 +1756,50 @@ {/if} + {#if pendingSubmissionItems.length > 0 || pendingSubmissions.notification_count > 0} +
+ + Pending activations ({pendingSubmissionItems.length} submissions · {pendingSubmissions.notification_count} notifications) + +
    + {#each pendingSubmissionItems as submission (submission.submission_id)} +
  1. + {submission.submission_id} + {submission.segment_count} segments · {submission.byte_len} bytes + +
    2. + {/each} +
+ + +
+ {/if} + {#if workerRunning}