diff --git a/Cargo.lock b/Cargo.lock index ce8416376..e7470de00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -64,6 +64,7 @@ dependencies = [ "dotenv", "eventsource-stream", "futures", + "imagesize", "reqwest", "schemars", "serde", @@ -2188,6 +2189,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "imagesize" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09e54e57b4c48b40f7aec75635392b12b3421fa26fe8b4332e63138ed278459c" + [[package]] name = "include_dir" version = "0.7.4" diff --git a/crates/agen/Cargo.toml b/crates/agen/Cargo.toml index febf6df18..e01b2ca40 100644 --- a/crates/agen/Cargo.toml +++ b/crates/agen/Cargo.toml @@ -32,6 +32,7 @@ reqwest = { version = "0.13", default-features = false, features = ["stream", "j eventsource-stream = "0.2" zstd = "0.13" base64 = "0.22.1" +imagesize = "0.14" chrono = { version = "0.4", default-features = false, features = ["serde", "clock"], optional = true } agen-macros = { workspace = true } diff --git a/crates/agen/src/engine.rs b/crates/agen/src/engine.rs index decbfbc9b..d35c1778b 100644 --- a/crates/agen/src/engine.rs +++ b/crates/agen/src/engine.rs @@ -626,6 +626,10 @@ pub struct Engine { /// engine history. An error rejects the item and aborts the turn, allowing /// upper layers to make durable storage the commit gate. history_append_cbs: Vec Result<(), String> + Send + Sync>>, + /// Durable host commit gate for image-result corrections. Runs before the + /// correction becomes live or a replacement request can be sent. + image_rejection_handler: + Option, &Item) -> Result<(), String> + Send + Sync>>, /// Request configuration (max_tokens, temperature, etc.) request_config: RequestConfig, /// Cancel notification channel (for interrupting execution) @@ -971,6 +975,16 @@ impl Engine { self.history_append_cbs.push(Box::new(callback)); } + /// Set the host's durable commit gate for correcting a rejected ViewImage + /// result. The original annotation/identity must be preserved. A failed + /// commit leaves both history and the outgoing request untouched. + pub fn set_image_rejection_handler( + &mut self, + handler: impl Fn(&HistoryEntry, &Item) -> Result<(), String> + Send + Sync + 'static, + ) { + self.image_rejection_handler = Some(Box::new(handler)); + } + fn emit_history_append(&self, item: &Item) -> Result<(), EngineError> { for cb in &self.history_append_cbs { cb(item).map_err(EngineError::HistoryAppend)?; @@ -3205,7 +3219,34 @@ impl Engine { "Sending request to LLM" ); - let stream = self.open_stream_with_retry(request, turn, llm_call).await?; + let mut request = request; + let stream = loop { + match self + .open_stream_with_retry(request.clone(), turn, llm_call) + .await + { + Ok(stream) => break stream, + Err(EngineError::Client(error)) if error.is_image_size_rejection() => { + let Some((history_index, request_index)) = + crate::image_recovery::largest_candidate(history, &request) + else { + return Err(EngineError::Client(error)); + }; + let entry = &history.entries()[history_index]; + let replacement = crate::image_recovery::rejected_result(&entry.item); + if let Some(commit) = &self.image_rejection_handler { + commit(entry, &replacement).map_err(EngineError::HistoryAppend)?; + } + history.entries_mut()[history_index].item = replacement.clone(); + // Rebuild the provider request from the corrected projection, + // retaining prior prune/interceptor decisions. No tool runs + // or assistant output have occurred for this failed request. + request.items[request_index] = replacement; + self.emit_warning(crate::image_recovery::REJECTION_MESSAGE); + } + Err(error) => return Err(error), + } + }; self.tool_call_collector.begin_response(); let mut stream = ResponseStreamPump::start(stream); let mut early_tools: Option = None; @@ -3567,6 +3608,7 @@ impl Engine { warning_cbs: Vec::new(), tool_result_cbs: Vec::new(), history_append_cbs: Vec::new(), + image_rejection_handler: None, request_config: RequestConfig::default(), cancel_tx, cancel_rx, @@ -3856,6 +3898,7 @@ impl Engine { warning_cbs: self.warning_cbs, tool_result_cbs: self.tool_result_cbs, history_append_cbs: self.history_append_cbs, + image_rejection_handler: self.image_rejection_handler, request_config: self.request_config, cancel_tx: self.cancel_tx, @@ -4040,6 +4083,7 @@ impl Engine { warning_cbs: self.warning_cbs, tool_result_cbs: self.tool_result_cbs, history_append_cbs: self.history_append_cbs, + image_rejection_handler: self.image_rejection_handler, request_config: self.request_config, cancel_tx: self.cancel_tx, diff --git a/crates/agen/src/image_recovery.rs b/crates/agen/src/image_recovery.rs new file mode 100644 index 000000000..dc38014fa --- /dev/null +++ b/crates/agen/src/image_recovery.rs @@ -0,0 +1,106 @@ +//! Recovery is driven by provider rejection, never a locally chosen image limit. +use crate::{History, Item, Role, ToolResultDisposition, llm_client::Request, tool::Attachment}; + +pub(crate) const REJECTION_MESSAGE: &str = "ViewImage image omitted from model input after the provider rejected image size. This is a recovery candidate, not proof that this particular image caused the rejection. Resize or crop the image and retry ViewImage; do not read the unchanged image again."; + +/// Keep the most recent response's tool results eligible, including early tool +/// results interleaved with that response's later text/calls. Empty boundaries +/// from older versions' failed requests are not evidence of accepted input. +pub(crate) fn accepted_input_end<'a>(items: impl Iterator) -> Option { + let mut response_start = None; + let mut accepted_prefix = None; + for (index, item) in items.enumerate() { + match item { + Item::AssistantResponseBoundary { .. } => response_start = Some(index), + Item::Message { + role: Role::Assistant, + .. + } + | Item::ToolCall { .. } + | Item::Reasoning { .. } => { + accepted_prefix = Some(response_start.unwrap_or(index)); + } + Item::Message { + role: Role::User, .. + } => response_start = None, + _ => {} + } + } + accepted_prefix +} + +/// Select only a ViewImage result actually sent in the rejected request. The +/// tool contract returns exactly one image, so each correction removes one +/// image and preserves the tool call/result pair. Other tools and user uploads +/// are deliberately not rewritten by this recovery policy. +pub(crate) fn largest_candidate( + history: &History, + request: &Request, +) -> Option<(usize, usize)> { + let start = accepted_input_end(history.items()).unwrap_or(0); + let mut largest = None; + for (index, entry) in history.entries().iter().enumerate().skip(start) { + let Item::ToolResult { + call_id, + attachments, + .. + } = &entry.item + else { + continue; + }; + let [Attachment::Image(image)] = attachments.as_slice() else { + continue; + }; + let from_view_image = history.items().any(|item| { + matches!( + item, + Item::ToolCall { call_id: id, name, .. } if id == call_id && name == "ViewImage" + ) + }); + if !from_view_image { + continue; + } + let sent = request.items.iter().position(|item| { + matches!( + item, + Item::ToolResult { call_id: id, attachments: sent, .. } + if id == call_id && sent == attachments + ) + }); + let Some(request_index) = sent else { continue }; + // Header inspection only; no raster allocation or size threshold. + // Unknown dimensions sort by bytes as a deterministic fallback. + let pixels = imagesize::blob_size(image.data()) + .ok() + .map(|size| (size.width as u64).saturating_mul(size.height as u64)) + .unwrap_or(0); + let rank = (pixels, image.data().len(), index); + if largest + .as_ref() + .is_none_or(|(previous, _)| rank > *previous) + { + largest = Some((rank, (index, request_index))); + } + } + largest.map(|(_, candidate)| candidate) +} + +pub(crate) fn rejected_result(item: &Item) -> Item { + let mut item = item.clone(); + if let Item::ToolResult { + summary, + content, + attachments, + disposition, + is_error, + .. + } = &mut item + { + *summary = REJECTION_MESSAGE.to_owned(); + *content = None; + attachments.clear(); + *disposition = ToolResultDisposition::Error; + *is_error = true; + } + item +} diff --git a/crates/agen/src/lib.rs b/crates/agen/src/lib.rs index 869d5c322..fca7069f1 100644 --- a/crates/agen/src/lib.rs +++ b/crates/agen/src/lib.rs @@ -3,6 +3,7 @@ mod engine; mod handler; mod history; +mod image_recovery; mod message; pub(crate) mod callback; diff --git a/crates/agen/src/llm_client/error.rs b/crates/agen/src/llm_client/error.rs index 355940fa1..69334b8a2 100644 --- a/crates/agen/src/llm_client/error.rs +++ b/crates/agen/src/llm_client/error.rs @@ -90,6 +90,35 @@ impl ClientError { } } + /// A provider-declared image size rejection, not a generic invalid input. + /// Some providers (including OpenAI) expose only `invalid_value` here, so + /// narrowly recognize their image-size diagnostic without hardcoded limits. + pub fn is_image_size_rejection(&self) -> bool { + let Self::Api { + status: Some(400 | 413 | 422), + code, + message, + .. + } = self + else { + return false; + }; + if matches!( + code.as_deref(), + Some("image_too_large" | "image_size_exceeded") + ) { + return true; + } + let message = message.to_ascii_lowercase(); + message.contains("image") + && ((message.contains("patches") + && message.contains("exceed") + && message.contains("limit")) + || message.contains("image is too large") + || message.contains("image too large") + || (message.contains("image dimensions") && message.contains("exceed"))) + } + pub fn retry_after(&self) -> Option { match self { ClientError::Api { retry_after, .. } => *retry_after, diff --git a/crates/agen/src/prune.rs b/crates/agen/src/prune.rs index 2ee018904..37af648da 100644 --- a/crates/agen/src/prune.rs +++ b/crates/agen/src/prune.rs @@ -136,14 +136,13 @@ pub fn project(items: &mut [Item], indices: &[usize]) -> usize { /// Remove binary attachments after the first model request that could consume them. /// /// Attachments on tool results following the latest model output are fresh outputs for the -/// immediate follow-up request and remain visible. Once a later assistant/reasoning/tool-call -/// item exists, the model has already had an opportunity to inspect the attachment. Subsequent -/// requests keep the durable tool summary and can call the originating tool again when the image -/// is still needed. Only the request-context clone is modified. +/// immediate follow-up request and remain visible. With early dispatch, later +/// text/calls in the same response have not consumed its tool results: use the +/// confirmed response boundary, not that response's last individual item. +/// Subsequent requests keep the durable tool summary and can call the originating +/// tool again when the image is still needed. Only the request-context clone is modified. pub fn project_consumed_attachments(items: &mut [Item]) -> usize { - let latest_model_output = items.iter().rposition(|item| { - item.is_assistant_message() || item.is_reasoning() || item.is_tool_call() - }); + let latest_model_output = crate::image_recovery::accepted_input_end(items.iter()); let mut count = 0; for (index, item) in items.iter_mut().enumerate() { let is_fresh = latest_model_output.is_some_and(|boundary| index > boundary); diff --git a/crates/agen/tests/image_recovery_test.rs b/crates/agen/tests/image_recovery_test.rs new file mode 100644 index 000000000..2fa11ee06 --- /dev/null +++ b/crates/agen/tests/image_recovery_test.rs @@ -0,0 +1,350 @@ +use agen::llm_client::event::{Event, ResponseStatus, StatusEvent}; +use agen::llm_client::{ClientError, LlmClient, Request, ResponseStream}; +use agen::tool::{Attachment, ImageAttachment}; +use agen::{Engine, EngineRunExit, History, Item, ToolResultDisposition}; +use async_trait::async_trait; +use std::sync::{Arc, Mutex}; + +fn rejection() -> ClientError { + ClientError::Api { status: Some(400), code: Some("invalid_value".into()), message: "The image you provided requires 52150 patches after processing, exceeding the limit of 30000. Please resize the image and try again.".into(), retry_after: None } +} + +fn image_result(id: &str, width: u32, height: u32, padding: usize) -> Item { + // PNG header: dimension inspection never decodes/allocates the pixel plane. + let mut bytes = b"\x89PNG\r\n\x1a\n\0\0\0\rIHDR".to_vec(); + bytes.extend(width.to_be_bytes()); + bytes.extend(height.to_be_bytes()); + bytes.resize(40 + padding, 0); + Item::ToolResult { + id: None, + call_id: id.into(), + summary: "Attached image".into(), + content: None, + disposition: ToolResultDisposition::Success, + is_error: false, + attachments: vec![Attachment::Image(ImageAttachment::new("image/png", bytes))], + } +} +fn history() -> History { + History::from_items(vec![ + Item::user_message("inspect"), + Item::tool_call("small", "ViewImage", "{}"), + Item::tool_call("large", "ViewImage", "{}"), + image_result("small", 10, 10, 2000), + image_result("large", 1600, 33349, 0), + ]) +} +fn images(request: &Request) -> Vec { + request + .items + .iter() + .filter_map(|item| match item { + Item::ToolResult { + call_id, + attachments, + .. + } if !attachments.is_empty() => Some(call_id.clone()), + _ => None, + }) + .collect() +} +#[derive(Clone)] +struct Client { + requests: Arc>>, + failures: usize, + other_error: bool, +} +impl Client { + fn new(failures: usize) -> Self { + Self { + requests: Arc::default(), + failures, + other_error: false, + } + } +} +#[async_trait] +impl LlmClient for Client { + fn clone_boxed(&self) -> Box { + Box::new(self.clone()) + } + async fn stream(&self, request: Request) -> Result { + let mut requests = self.requests.lock().unwrap(); + requests.push(request); + if requests.len() <= self.failures { + return Err(rejection()); + } + if self.other_error { + return Err(ClientError::Api { + status: Some(400), + code: Some("invalid_value".into()), + message: "invalid tool arguments".into(), + retry_after: None, + }); + } + Ok(Box::pin(futures::stream::iter(vec![ + Ok(Event::text_block_start(0)), + Ok(Event::text_delta(0, "resize and retry")), + Ok(Event::text_block_stop(0, None)), + Ok(Event::Status(StatusEvent { + status: ResponseStatus::Completed, + })), + ]))) + } +} + +#[tokio::test] +async fn removes_largest_pixels_not_largest_file_and_keeps_pairing() { + let client = Client::new(1); + let observer = client.clone(); + let mut history = history(); + let mut engine = Engine::new(client); + let committed = Arc::new(Mutex::new(Vec::new())); + let commits = committed.clone(); + engine.set_image_rejection_handler(move |original, replacement| { + commits + .lock() + .unwrap() + .push((original.item.clone(), replacement.clone())); + Ok(()) + }); + let output = engine.run(&mut history, "continue").await; + assert!( + matches!(output.result, EngineRunExit::Finished), + "{:?}", + output.result + ); + let requests = observer.requests.lock().unwrap(); + assert_eq!(requests.len(), 2); + assert_eq!(images(&requests[0]), ["small", "large"]); + assert_eq!(images(&requests[1]), ["small"]); + assert!(requests[1].items.iter().any(|item| matches!(item, Item::ToolResult { call_id, is_error: true, disposition: ToolResultDisposition::Error, summary, .. } if call_id == "large" && summary.contains("retry ViewImage")))); + assert_eq!(committed.lock().unwrap().len(), 1); + assert_eq!( + history + .items() + .filter(|item| matches!(item, Item::ToolResult {call_id, ..} if call_id == "large")) + .count(), + 1 + ); +} + +#[tokio::test] +async fn multiple_rejections_remove_one_image_at_a_time_and_stop_at_exhaustion() { + let client = Client::new(99); + let observer = client.clone(); + let mut history = history(); + let out = Engine::new(client).run(&mut history, "continue").await; + assert!(matches!(out.result, EngineRunExit::Interrupted(_))); + let requests = observer.requests.lock().unwrap(); + assert_eq!(requests.len(), 3); + assert_eq!(images(&requests[1]), ["small"]); + assert!(images(&requests[2]).is_empty()); +} + +#[tokio::test] +async fn unrelated_error_stops_recovery_without_removing_another_image() { + let mut client = Client::new(1); + client.other_error = true; + let observer = client.clone(); + let mut history = history(); + let out = Engine::new(client).run(&mut history, "continue").await; + assert!(matches!(out.result, EngineRunExit::Interrupted(_))); + assert_eq!(observer.requests.lock().unwrap().len(), 2); + assert!(history.items().any(|item| matches!(item, Item::ToolResult {call_id, attachments, ..} if call_id == "small" && !attachments.is_empty()))); +} + +#[tokio::test] +async fn persistence_failure_does_not_change_history_or_retry() { + let client = Client::new(99); + let observer = client.clone(); + let mut history = history(); + let original = history.items_cloned(); + let mut engine = Engine::new(client); + engine.set_image_rejection_handler(|_, _| Err("disk full".into())); + let out = engine.run(&mut history, "continue").await; + assert!(matches!(out.result, EngineRunExit::Interrupted(_))); + assert_eq!( + &history.items_cloned()[..original.len()], + original.as_slice() + ); + assert_eq!(observer.requests.lock().unwrap().len(), 1); +} + +#[tokio::test] +async fn previously_accepted_images_are_not_recovery_candidates() { + let client = Client::new(99); + let observer = client.clone(); + let mut history = history(); + history.push(Item::assistant_message("I saw those images")); + history.push(Item::tool_call("new", "ViewImage", "{}")); + history.push(image_result("new", 1, 1, 0)); + let out = Engine::new(client).run(&mut history, "continue").await; + assert!(matches!(out.result, EngineRunExit::Interrupted(_))); + let requests = observer.requests.lock().unwrap(); + assert_eq!(requests.len(), 2); + // Already-consumed images are projected out, but their durable records + // must not be rewritten as failures. + assert!(images(&requests[1]).is_empty()); + assert_eq!(history.items().filter(|item| matches!(item, Item::ToolResult {call_id, is_error: false, attachments, ..} if (call_id == "small" || call_id == "large") && !attachments.is_empty())).count(), 2); +} + +#[tokio::test] +async fn legacy_empty_failed_request_boundaries_do_not_hide_candidates() { + let client = Client::new(1); + let observer = client.clone(); + let mut history = history(); + history.push(Item::assistant_response_boundary("failed-1")); + history.push(Item::assistant_response_boundary("failed-2")); + let out = Engine::new(client).run(&mut history, "continue").await; + assert!(matches!(out.result, EngineRunExit::Finished)); + assert_eq!(images(&observer.requests.lock().unwrap()[1]), ["small"]); +} + +#[tokio::test] +async fn early_results_remain_candidates_despite_later_text_in_same_response() { + let client = Client::new(1); + let observer = client.clone(); + let mut history = History::from_items(vec![ + Item::user_message("inspect"), + Item::assistant_response_boundary("response-1"), + Item::tool_call("image", "ViewImage", "{}"), + image_result("image", 99, 99, 0), + Item::assistant_message("waiting for image"), + ]); + let out = Engine::new(client).run(&mut history, "continue").await; + assert!( + matches!(out.result, EngineRunExit::Finished), + "{:?}; requests: {:?}", + out.result, + observer.requests.lock().unwrap() + ); + assert!(images(&observer.requests.lock().unwrap()[1]).is_empty()); +} + +#[derive(Clone)] +struct CountingTool { + image: bool, + count: Arc, +} +#[async_trait] +impl agen::tool::Tool for CountingTool { + async fn execute( + &self, + _: &str, + _: agen::tool::ToolExecutionContext, + ) -> Result { + self.count.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + let attachments = if self.image { + let Item::ToolResult { attachments, .. } = image_result("image", 1600, 33349, 0) else { + unreachable!() + }; + attachments + } else { + Vec::new() + }; + Ok(agen::tool::ToolOutput { + summary: "executed once".into(), + content: None, + attachments, + }) + } +} +#[derive(Clone)] +struct ToolClient(Client); +#[async_trait] +impl LlmClient for ToolClient { + fn clone_boxed(&self) -> Box { + Box::new(self.clone()) + } + fn tool_call_completion_support(&self) -> agen::llm_client::ToolCallCompletionSupport { + agen::llm_client::ToolCallCompletionSupport::PerBlock + } + async fn stream(&self, request: Request) -> Result { + if self.0.requests.lock().unwrap().is_empty() { + self.0.requests.lock().unwrap().push(request); + return Ok(Box::pin(futures::stream::iter( + vec![ + Event::tool_use_start(0, "image", "ViewImage"), + Event::tool_input_delta(0, "{}"), + Event::tool_use_stop(0), + Event::tool_use_start(1, "side-effect", "Bash"), + Event::tool_input_delta(1, "{}"), + Event::tool_use_stop(1), + Event::Status(StatusEvent { + status: ResponseStatus::Completed, + }), + ] + .into_iter() + .map(Ok), + ))); + } + self.0.stream(request).await + } +} + +#[tokio::test] +async fn fresh_image_recovery_does_not_repeat_tools_in_either_dispatch_mode() { + for mode in [ + agen::ToolCallDispatchMode::AfterResponse, + agen::ToolCallDispatchMode::OnToolCallComplete, + ] { + let client = Client::new(2); + let mut engine = Engine::new(ToolClient(client.clone())); + engine.set_tool_call_dispatch_mode(mode); + let mut counts = Vec::new(); + for (name, image) in [("ViewImage", true), ("Bash", false)] { + let tool = CountingTool { + image, + count: Arc::default(), + }; + counts.push(tool.count.clone()); + engine.register_tool(Arc::new(move || { + ( + agen::tool::ToolMeta::new(name) + .description("test") + .input_schema(serde_json::json!({"type":"object"})), + Arc::new(tool.clone()), + ) + })); + } + let mut history = History::new(); + let out = engine.run(&mut history, "inspect").await; + assert!( + matches!(out.result, EngineRunExit::Finished), + "{:?}", + out.result + ); + assert!( + counts + .iter() + .all(|count| count.load(std::sync::atomic::Ordering::SeqCst) == 1) + ); + let requests = client.requests.lock().unwrap(); + assert_eq!(requests.len(), 3); + assert_eq!(images(&requests[1]), ["image"]); + assert!(images(&requests[2]).is_empty()); + } +} + +#[test] +fn image_error_classifier_is_not_a_generic_400_retry() { + assert!(rejection().is_image_size_rejection()); + for (status, message) in [ + (400, "invalid image format"), + (400, "image URL not found"), + (400, "context window exceeds token limit"), + (401, "image is too large"), + (500, "image is too large"), + ] { + assert!( + !ClientError::Api { + status: Some(status), + code: Some("invalid_value".into()), + message: message.into(), + retry_after: None + } + .is_image_size_rejection() + ); + } +} diff --git a/crates/session-store/src/public_index.rs b/crates/session-store/src/public_index.rs index d569c09b7..1e1406d16 100644 --- a/crates/session-store/src/public_index.rs +++ b/crates/session-store/src/public_index.rs @@ -1071,6 +1071,7 @@ fn persisted_entry_units(entries: &[LogEntry]) -> usize { | LogEntry::AnnotatedUserInput { history, .. } => history.len(), LogEntry::AnnotatedAssistantItem { .. } | LogEntry::AnnotatedToolResult { .. } + | LogEntry::ToolResultCorrected { .. } | LogEntry::AnnotatedSystemItem { .. } => 1, _ => 0, }; diff --git a/crates/session-store/src/public_snapshot.rs b/crates/session-store/src/public_snapshot.rs index bc7659c12..3f7f8e53c 100644 --- a/crates/session-store/src/public_snapshot.rs +++ b/crates/session-store/src/public_snapshot.rs @@ -7,7 +7,7 @@ use protocol::{ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::Path; use thiserror::Error; @@ -139,6 +139,17 @@ struct RetainedHistoryCursor { before_offset: u64, record_end_offset: Option, seed_entry_index: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + corrections: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +struct ToolCorrectionPosition { + entry_id: String, + segment_id: SegmentId, + start: u64, + end: u64, } #[derive(Debug)] @@ -445,6 +456,7 @@ pub fn read_retained_session_history_page( limits, &mut scanned_bytes, &mut scanned_entries, + &mut Vec::new(), )?; Some(SessionHistoryLineageBoundary { lineage_id: lineage_identity(active.session_id, &lineage[1..]), @@ -456,6 +468,10 @@ pub fn read_retained_session_history_page( } else { None }; + let mut correction_positions = cursor + .as_ref() + .map(|c| c.corrections.clone()) + .unwrap_or_default(); let mut turns = read_history_turns_backward( &store, active.session_id, @@ -465,11 +481,23 @@ pub fn read_retained_session_history_page( limits, &mut scanned_bytes, &mut scanned_entries, + &mut correction_positions, )?; let has_more = turns.len() > page_limit; if has_more { turns.truncate(page_limit); } + // Carry only corrections whose original results are still on older pages. + let returned_ids: HashSet<_> = turns + .iter() + .flat_map(|turn| { + turn.turn + .entries + .iter() + .map(|entry| entry.entry_id.as_str()) + }) + .collect(); + correction_positions.retain(|position| !returned_ids.contains(position.entry_id.as_str())); let next_cursor = if has_more { let oldest = turns .last() @@ -485,6 +513,7 @@ pub fn read_retained_session_history_page( before_offset: oldest.position.before_offset, record_end_offset: oldest.position.record_end_offset, seed_entry_index: oldest.position.seed_entry_index, + corrections: correction_positions, })?) } else { None @@ -702,6 +731,7 @@ fn persisted_entry_units(entries: &[LogEntry]) -> usize { | LogEntry::AnnotatedUserInput { history, .. } => history.len(), LogEntry::AnnotatedAssistantItem { .. } | LogEntry::AnnotatedToolResult { .. } + | LogEntry::ToolResultCorrected { .. } | LogEntry::AnnotatedSystemItem { .. } => 1, _ => 0, }; @@ -814,10 +844,56 @@ fn read_history_turns_backward( limits: RetainedHistoryReadLimits, scanned_bytes: &mut u64, scanned_entries: &mut usize, + correction_positions: &mut Vec, ) -> Result, RetainedHistoryReadError> { let mut turns = Vec::new(); let mut pending_entries = Vec::new(); let mut seen_entries = HashSet::new(); + let mut corrections = HashMap::::new(); + + // Carry only bounded log positions, never correction text or image bodies, + // across pages. Re-read and validate the referenced records from adopted + // lineage rather than rescanning an arbitrarily large newer suffix. + for position in correction_positions.iter() { + let segment = lineage + .iter() + .find(|s| s.segment_id == position.segment_id) + .ok_or(RetainedHistoryReadError::InvalidCursor)?; + let adopted_end = locate_adopted_segment_end( + store, + session_id, + segment, + limits, + scanned_bytes, + scanned_entries, + )?; + if position.start >= position.end || position.end > adopted_end { + return Err(RetainedHistoryReadError::InvalidCursor); + } + let record = store + .read_log_record_range_read_only_bounded( + session_id, + position.segment_id, + position.start, + position.end, + limits.max_scan_bytes.saturating_sub(*scanned_bytes), + ) + .map_err(map_history_cursor_store_error)?; + *scanned_bytes = scanned_bytes + .checked_add(record.end_offset - record.start_offset) + .ok_or(RetainedHistoryReadError::ResourceLimit)?; + *scanned_entries = scanned_entries + .checked_add(persisted_entry_units(std::slice::from_ref(&record.entry))) + .ok_or(RetainedHistoryReadError::ResourceLimit)?; + if *scanned_entries > limits.max_entries { + return Err(RetainedHistoryReadError::ResourceLimit); + } + if !matches!(&record.entry, LogEntry::ToolResultCorrected { entry, .. } if entry.metadata.entry_id.0 == position.entry_id) + || !capture_tool_correction(&record.entry, &mut corrections) + { + return Err(RetainedHistoryReadError::InvalidCursor); + } + } let (start_segment_index, start_position, cursor_turn_id) = match start { Some((position, turn_id)) => { @@ -878,32 +954,6 @@ fn read_history_turns_backward( for (segment_index, segment) in lineage.iter().enumerate().skip(start_segment_index) { let starting_here = segment_index == start_segment_index; - if starting_here - && start_position - .as_ref() - .and_then(|position| position.seed_entry_index) - .is_some() - { - let seed_index = start_position - .as_ref() - .and_then(|position| position.seed_entry_index) - .expect("checked seed cursor"); - if collect_history_entries_backward( - &segment.seed_entries[..seed_index], - segment.segment_id, - 0, - None, - true, - turn_limit, - &mut seen_entries, - &mut pending_entries, - &mut turns, - ) { - return Ok(turns); - } - continue; - } - let adopted_end = locate_adopted_segment_end( store, session_id, @@ -923,6 +973,32 @@ fn read_history_turns_backward( if before_offset > adopted_end { return Err(RetainedHistoryReadError::InvalidCursor); } + if starting_here + && start_position + .as_ref() + .and_then(|position| position.seed_entry_index) + .is_some() + { + let seed_index = start_position + .as_ref() + .and_then(|position| position.seed_entry_index) + .expect("checked seed cursor"); + if collect_history_entries_backward( + &corrected_history_entries(&segment.seed_entries[..seed_index], &corrections), + segment.segment_id, + 0, + None, + true, + turn_limit, + &mut seen_entries, + &mut pending_entries, + &mut turns, + ) { + return Ok(turns); + } + continue; + } + let mut reader = store .open_retained_segment_reader(session_id, segment.segment_id, before_offset) .map_err(map_history_store_error)?; @@ -949,7 +1025,25 @@ fn read_history_turns_backward( } continue; } - let entries = project_history_record(session_id, segment.segment_id, &record.entry); + if capture_tool_correction(&record.entry, &mut corrections) { + let LogEntry::ToolResultCorrected { entry, .. } = &record.entry else { + unreachable!() + }; + let position = ToolCorrectionPosition { + entry_id: entry.metadata.entry_id.0.clone(), + segment_id: segment.segment_id, + start: record.start_offset, + end: record.end_offset, + }; + if !correction_positions.contains(&position) { + correction_positions.push(position); + } + continue; + } + let entries = corrected_history_entries( + &project_history_record(session_id, segment.segment_id, &record.entry), + &corrections, + ); if collect_history_entries_backward( &entries, segment.segment_id, @@ -967,7 +1061,7 @@ fn read_history_turns_backward( if segment_index + 1 == lineage.len() && collect_history_entries_backward( - &segment.seed_entries, + &corrected_history_entries(&segment.seed_entries, &corrections), segment.segment_id, 0, None, @@ -984,6 +1078,38 @@ fn read_history_turns_backward( Ok(turns) } +fn capture_tool_correction( + record: &LogEntry, + corrections: &mut HashMap, +) -> bool { + let LogEntry::ToolResultCorrected { entry, .. } = record else { + return false; + }; + if let Some(data) = project_item(&entry.metadata.entry_id.0, &entry.item) { + // We read backwards: the first correction seen is the latest one. + corrections + .entry(entry.metadata.entry_id.0.clone()) + .or_insert(data); + } + true +} + +fn corrected_history_entries( + entries: &[SessionSnapshotEntry], + corrections: &HashMap, +) -> Vec { + entries + .iter() + .cloned() + .map(|mut entry| { + if let Some(data) = corrections.get(&entry.entry_id) { + entry.data = data.clone(); + } + entry + }) + .collect() +} + #[allow(clippy::too_many_arguments)] fn collect_history_entries_backward( entries: &[SessionSnapshotEntry], @@ -1052,9 +1178,13 @@ fn is_user_entry(entry: &SessionSnapshotEntry) -> bool { fn encode_history_cursor( cursor: &RetainedHistoryCursor, ) -> Result { - serde_json::to_vec(cursor) + let encoded = serde_json::to_vec(cursor) .map(|bytes| URL_SAFE_NO_PAD.encode(bytes)) - .map_err(|_| RetainedHistoryReadError::CorruptLog) + .map_err(|_| RetainedHistoryReadError::CorruptLog)?; + if encoded.len() > 4096 { + return Err(RetainedHistoryReadError::ResourceLimit); + } + Ok(encoded) } fn decode_history_cursor(cursor: &str) -> Result { @@ -1252,6 +1382,19 @@ pub(crate) fn project_session_snapshot_for_segment( entries.push(history_entry(entry, *ts, data)); } } + LogEntry::ToolResultCorrected { ts, entry } => { + if let Some(data) = project_item(&entry.metadata.entry_id.0, &entry.item) { + if let Some(existing) = entries + .iter_mut() + .find(|existing| existing.entry_id == entry.metadata.entry_id.0) + { + // The original result keeps its place/time in the conversation. + existing.data = data; + } else { + entries.push(history_entry(entry, *ts, data)); + } + } + } LogEntry::AnnotatedSystemItem { ts, entry, .. } => entries.push(system_entry( &entry.item, entry.metadata.entry_id.0.clone(), @@ -1782,6 +1925,129 @@ mod tests { .collect() } + #[test] + fn image_correction_keeps_original_position_across_history_pages() { + let session_id = crate::new_session_id(); + let segment_id = crate::new_segment_id(); + let mut image = assistant_message(1, "image"); + image.item = agen::Item::tool_result_item_with_disposition_and_attachments( + "image-call", + "original image", + None, + agen::ToolResultDisposition::Success, + vec![agen::tool::Attachment::Image( + agen::tool::ImageAttachment::new("image/png", vec![1_u8, 2, 3]), + )], + ) + .into(); + let mut corrected = image.clone(); + corrected.item = agen::Item::tool_result_item_with_disposition_and_attachments( + "image-call", + "Resize or crop and retry ViewImage", + None, + agen::ToolResultDisposition::Error, + Vec::new(), + ) + .into(); + let mut log = vec![segment_start(session_id, Vec::new(), None, None)]; + append_turn(&mut log, 1); + log.push(LogEntry::AnnotatedToolResult { + ts: 15, + entry: image.clone(), + }); + append_turn(&mut log, 2); + append_turn(&mut log, 3); + log.push(LogEntry::ToolResultCorrected { + ts: 35, + entry: corrected, + }); + let (_root, aggregate_root) = persist_history_fixture( + "corrected-image", + session_id, + segment_id, + vec![(segment_id, log.clone())], + ); + let first = read_retained_session_history_page( + &aggregate_root, + "corrected-image", + None, + Some(1), + RetainedHistoryReadLimits::default(), + ) + .unwrap(); + assert_eq!(turn_ids(&first), ["user-3"]); + assert!( + !first.turns[0] + .entries + .iter() + .any(|entry| entry.entry_id == image.metadata.entry_id.0) + ); + let second = read_retained_session_history_page( + &aggregate_root, + "corrected-image", + first.next_cursor.as_deref(), + Some(1), + RetainedHistoryReadLimits::default(), + ) + .unwrap(); + assert_eq!(turn_ids(&second), ["user-2"]); + let third = read_retained_session_history_page( + &aggregate_root, + "corrected-image", + second.next_cursor.as_deref(), + Some(1), + RetainedHistoryReadLimits::default(), + ) + .unwrap(); + assert_eq!(turn_ids(&third), ["user-1"]); + let entry = third.turns[0] + .entries + .iter() + .find(|entry| entry.entry_id == image.metadata.entry_id.0) + .unwrap(); + assert_eq!(entry.timestamp, 15); + assert!( + matches!(&entry.data, SessionSnapshotEntryData::ToolResult { is_error: true, attachments, .. } if attachments.is_empty()) + ); + assert_eq!( + *entry, + *project_session_snapshot(session_id, &log) + .entries + .iter() + .find(|entry| entry.entry_id == image.metadata.entry_id.0) + .unwrap() + ); + let restored = crate::collect_state(&log); + assert_eq!( + restored + .annotated_history + .iter() + .filter(|entry| entry.metadata.entry_id == image.metadata.entry_id) + .count(), + 1 + ); + assert!( + matches!(&restored.annotated_history.iter().find(|entry| entry.metadata.entry_id == image.metadata.entry_id).unwrap().item, LoggedItem::ToolResult { is_error: true, attachments, .. } if attachments.is_empty()) + ); + + // Cursor pointers are only hints: the referenced record is read under + // current lineage authority and must actually be a correction record. + let mut cursor = decode_history_cursor(first.next_cursor.as_deref().unwrap()).unwrap(); + cursor.corrections[0].segment_id = crate::new_segment_id(); + let forged = encode_history_cursor(&cursor).unwrap(); + assert_eq!( + read_retained_session_history_page( + &aggregate_root, + "corrected-image", + Some(&forged), + Some(1), + RetainedHistoryReadLimits::default() + ) + .unwrap_err(), + RetainedHistoryReadError::InvalidCursor + ); + } + #[test] fn retained_history_pages_twelve_turns_as_five_five_two_across_compaction() { let worker_name = "worker-history-12"; diff --git a/crates/session-store/src/segment_log.rs b/crates/session-store/src/segment_log.rs index fb61009b0..0201a0b3a 100644 --- a/crates/session-store/src/segment_log.rs +++ b/crates/session-store/src/segment_log.rs @@ -107,6 +107,9 @@ pub enum LogEntry { /// Canonical tool output and metadata committed as one journal record. AnnotatedToolResult { ts: u64, entry: LoggedHistoryEntry }, + /// Append-only correction of a provider-rejected image result. Retains the + /// original stable identity; replay replaces rather than appends the item. + ToolResultCorrected { ts: u64, entry: LoggedHistoryEntry }, /// Canonical typed system event and model-visible metadata committed /// together. @@ -399,6 +402,16 @@ pub fn collect_state(entries: &[LogEntry]) -> RestoredState { state.annotated_history.push(entry.clone()); state.history.push(Item::from(entry.item.clone())); } + LogEntry::ToolResultCorrected { entry, .. } => { + if let Some(index) = state + .annotated_history + .iter() + .position(|existing| existing.metadata.entry_id == entry.metadata.entry_id) + { + state.annotated_history[index] = entry.clone(); + state.history[index] = Item::from(entry.item.clone()); + } + } LogEntry::AnnotatedSystemItem { entry, extensions, .. } => { diff --git a/crates/worker-runtime/src/worker_source.rs b/crates/worker-runtime/src/worker_source.rs index bb700ae41..089ed9f8f 100644 --- a/crates/worker-runtime/src/worker_source.rs +++ b/crates/worker-runtime/src/worker_source.rs @@ -381,10 +381,10 @@ impl RuntimeOwnedWorkspaceClient { self } - fn execute_with_permission( + fn execute_with_optional_timeout( &self, request: WorkspaceRequest, - permission: &'static str, + timeout: Option, ) -> Result { let method = match request.method { WorkspaceRequestMethod::Get => reqwest::Method::GET, @@ -409,9 +409,9 @@ impl RuntimeOwnedWorkspaceClient { path_and_query: request.path, body, headers, - permission: permission.to_string(), + permission: WORKSPACE_REQUEST_PERMISSION.to_string(), worker_id: Some(self.worker_id.clone()), - timeout: self.request_timeout, + timeout, max_response_bytes: 8 * 1024 * 1024, }) .map_err(|error| { @@ -478,7 +478,15 @@ impl WorkspaceClient for RuntimeOwnedWorkspaceClient { &self, request: WorkspaceRequest, ) -> Result { - self.execute_with_permission(request, WORKSPACE_REQUEST_PERMISSION) + self.execute_with_optional_timeout(request, self.request_timeout) + } + + fn execute_with_timeout( + &self, + request: WorkspaceRequest, + timeout: Duration, + ) -> Result { + self.execute_with_optional_timeout(request, Some(timeout)) } fn execute_server_operation( @@ -1341,6 +1349,25 @@ mod tests { server.join().unwrap(); } + #[test] + fn runtime_owned_workspace_client_honors_per_request_timeouts() { + let (base_url, server) = delayed_workspace_response(Duration::from_millis(75)); + let client = + RuntimeOwnedWorkspaceClient::new("workspace-a", base_url, "runtime-a", "worker-a"); + + let error = client + .execute_with_timeout( + WorkspaceRequest::json(WorkspaceRequestMethod::Post, "/api/test", "{}"), + Duration::from_millis(20), + ) + .unwrap_err(); + assert_eq!(client.request_timeout, None); + let message = error.to_string(); + assert!(message.contains("POST /api/test"), "{message}"); + assert!(message.contains("timed out"), "{message}"); + server.join().unwrap(); + } + #[test] fn runtime_owned_workspace_client_reports_request_timeouts() { let (base_url, server) = delayed_workspace_response(Duration::from_millis(75)); diff --git a/crates/worker/src/feature/builtin/memory.rs b/crates/worker/src/feature/builtin/memory.rs index 9a2da3fca..3565e8fcf 100644 --- a/crates/worker/src/feature/builtin/memory.rs +++ b/crates/worker/src/feature/builtin/memory.rs @@ -6,6 +6,7 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex}; +use std::time::Duration; use agen::tool::{Tool, ToolDefinition, ToolError, ToolExecutionContext, ToolMeta, ToolOutput}; use arc_swap::ArcSwap; @@ -157,9 +158,24 @@ impl dyn WorkspaceClient + '_ { } } +const RESIDENT_SUMMARY_REQUEST_TIMEOUT: Duration = Duration::from_secs(5); + async fn execute_subjektiv_memory_operation( client: &dyn WorkspaceClient, operation: server_api::SubjektivMemoryBackendOperation, +) -> Result { + execute_subjektiv_memory_operation_with(client, operation, |client, request| { + client.execute(request) + }) +} + +fn execute_subjektiv_memory_operation_with( + client: &dyn WorkspaceClient, + operation: server_api::SubjektivMemoryBackendOperation, + execute: impl FnOnce( + &dyn WorkspaceClient, + WorkspaceRequest, + ) -> Result, ) -> Result { let workspace_id = client @@ -170,11 +186,14 @@ async fn execute_subjektiv_memory_operation( client.kind() ), })?; - let response = client.execute(WorkspaceRequest::json( - WorkspaceRequestMethod::Post, - format!("/api/w/{workspace_id}/subjektiv/memory"), - serde_json::to_string(&server_api::SubjektivMemoryBackendRequest { operation })?, - ))?; + let response = execute( + client, + WorkspaceRequest::json( + WorkspaceRequestMethod::Post, + format!("/api/w/{workspace_id}/subjektiv/memory"), + serde_json::to_string(&server_api::SubjektivMemoryBackendRequest { operation })?, + ), + )?; let status = reqwest::StatusCode::from_u16(response.status) .unwrap_or(reqwest::StatusCode::INTERNAL_SERVER_ERROR); if !response.is_success() { @@ -712,12 +731,13 @@ impl WorkspaceSubjektivResidentSummarySource { async fn fetch( &self, ) -> Result { - match execute_subjektiv_memory_operation( + match execute_subjektiv_memory_operation_with( self.client.as_ref(), server_api::SubjektivMemoryBackendOperation::ResidentContext(Default::default()), - ) - .await? - { + |client, request| { + client.execute_with_timeout(request, RESIDENT_SUMMARY_REQUEST_TIMEOUT) + }, + )? { server_api::SubjektivMemoryBackendResponse::ResidentContext(output) => Ok(output), other => Err(WorkspaceMemoryBackendError::InvalidResponse(format!( "unexpected subject resident context response: {other:?}" @@ -1291,6 +1311,7 @@ mod tests { scope_allowed: bool, behavior: Mutex<(u64, String)>, paths: Mutex>, + timeouts: Mutex>, } impl SubjectResidentClient { @@ -1304,6 +1325,7 @@ mod tests { scope_allowed: true, behavior: Mutex::new((3, "Be deliberate.".to_string())), paths: Mutex::new(Vec::new()), + timeouts: Mutex::new(Vec::new()), } } @@ -1314,6 +1336,7 @@ mod tests { scope_allowed: false, behavior: Mutex::new((3, "must not be injected".to_string())), paths: Mutex::new(Vec::new()), + timeouts: Mutex::new(Vec::new()), } } } @@ -1367,6 +1390,15 @@ mod tests { .unwrap(), }) } + + fn execute_with_timeout( + &self, + request: WorkspaceRequest, + timeout: Duration, + ) -> Result { + self.timeouts.lock().unwrap().push(timeout); + self.execute(request) + } } fn resident_client(content: &str) -> Arc { @@ -1681,6 +1713,10 @@ permission = "write" ready_client.paths.lock().unwrap().clone(), vec!["/api/w/workspace/subjektiv/memory".to_string()] ); + assert_eq!( + ready_client.timeouts.lock().unwrap().as_slice(), + &[RESIDENT_SUMMARY_REQUEST_TIMEOUT] + ); let ready_empty_client = Arc::new(SubjectResidentClient::new( memory::backend::MemoryResidentSummaryAvailability::Ready, @@ -1790,6 +1826,12 @@ permission = "write" .body .contains("No user-managed behavior is set") ); + // Initial load and every live update/clear/replay probe retain the + // target-side bounded resident request contract. + assert_eq!( + client.timeouts.lock().unwrap().as_slice(), + &[RESIDENT_SUMMARY_REQUEST_TIMEOUT; 7] + ); } #[tokio::test] diff --git a/crates/worker/src/image_recovery_tests.rs b/crates/worker/src/image_recovery_tests.rs new file mode 100644 index 000000000..2c75374b2 --- /dev/null +++ b/crates/worker/src/image_recovery_tests.rs @@ -0,0 +1,221 @@ +//! Recovery tests use the normal Worker execution and append-log commit gate. +use super::*; +use agen::llm_client::event::{Event as LlmEvent, ResponseStatus, StatusEvent}; +use agen::llm_client::{ClientError, Request, ResponseStream}; +use agen::tool::{Attachment, ImageAttachment}; +use async_trait::async_trait; + +#[derive(Clone, Default)] +struct RejectImages { + requests: Arc>>, + cancel_on_retry: Arc>>>, +} +#[async_trait] +impl LlmClient for RejectImages { + fn clone_boxed(&self) -> Box { + Box::new(self.clone()) + } + async fn stream(&self, request: Request) -> Result { + let has_image = request.items.iter().any( + |item| matches!(item, Item::ToolResult { attachments, .. } if !attachments.is_empty()), + ); + self.requests.lock().unwrap().push(request); + if has_image { + return Err(ClientError::Api { + status: Some(400), + code: Some("invalid_value".into()), + message: "The image requires 52150 patches, exceeding the limit of 30000.".into(), + retry_after: None, + }); + } + if let Some(cancel) = self.cancel_on_retry.lock().unwrap().take() { + cancel.try_send(()).unwrap(); + return Ok(Box::pin(futures::stream::pending())); + } + Ok(Box::pin(futures::stream::iter(vec![ + Ok(LlmEvent::text_block_start(0)), + Ok(LlmEvent::text_delta(0, "Resize the source image")), + Ok(LlmEvent::text_block_stop(0, None)), + Ok(LlmEvent::Status(StatusEvent { + status: ResponseStatus::Completed, + })), + ]))) + } +} + +async fn image_worker() -> ( + tempfile::TempDir, + Worker, + RejectImages, +) { + let dir = tempfile::tempdir().unwrap(); + let store = session_store::FsStore::new(dir.path().join("session")).unwrap(); + let manifest = WorkerManifest::from_toml( + r#" +[worker] +name = "image-recovery-test" +[model] +scheme = "anthropic" +model_id = "test" +[engine] +[scope] +allow = [] +"#, + ) + .unwrap(); + let client = RejectImages::default(); + let mut worker = Worker::new( + manifest, + Engine::<_, Mutable, SessionHistoryMetadata>::new_annotated(client.clone()), + store, + WorkerWorkspaceContext::local_filesystem(None), + WorkerFilesystemAuthority::None, + Scope::writable(dir.path()).unwrap(), + ) + .await + .unwrap(); + worker.disable_manifest_lifecycle_features(); + worker.set_history_for_test(vec![ + Item::user_message("inspect this image"), + Item::tool_call("image-call", "ViewImage", "{}"), + Item::tool_result_item_with_disposition_and_attachments( + "image-call", + "Attached image", + None, + agen::ToolResultDisposition::Success, + vec![Attachment::Image(ImageAttachment::new( + "image/png", + vec![1_u8, 2, 3], + ))], + ), + ]); + worker.wire_history_persistence(); + (dir, worker, client) +} +fn rejected(item: &Item) -> bool { + matches!(item, Item::ToolResult { call_id, is_error: true, attachments, .. } if call_id == "image-call" && attachments.is_empty()) +} + +#[tokio::test] +async fn image_correction_is_committed_before_retry_and_survives_restore_and_reseed() { + let (_dir, mut worker, client) = image_worker().await; + assert_eq!( + worker.run_text("continue").await.unwrap(), + WorkerRunResult::Finished + ); + assert_eq!(client.requests.lock().unwrap().len(), 2); + let raw = worker + .store + .read_all(worker.session_id(), worker.segment_id()) + .unwrap(); + let correction = raw + .iter() + .find(|entry| matches!(entry, LogEntry::ToolResultCorrected { .. })) + .unwrap() + .clone(); + let restored = restore_history_entries(worker.session_id(), worker.segment_id(), &raw).unwrap(); + assert_eq!( + restored + .iter() + .filter(|entry| rejected(&entry.item)) + .count(), + 1 + ); + assert_eq!( + session_store::collect_state(&raw) + .history + .iter() + .filter(|item| rejected(item)) + .count(), + 1 + ); + let LogEntry::ToolResultCorrected { + entry: corrected, .. + } = &correction + else { + unreachable!() + }; + let original = match &raw[0] { + LogEntry::AnnotatedSegmentStart { history, .. } => history + .iter() + .find(|entry| entry.metadata.entry_id == corrected.metadata.entry_id) + .unwrap(), + _ => panic!("missing seed"), + }; + assert!( + matches!(&original.item, session_store::LoggedItem::ToolResult { attachments, is_error: false, .. } if attachments.len() == 1) + ); + assert_eq!( + restored + .iter() + .find(|entry| rejected(&entry.item)) + .unwrap() + .annotation, + original.metadata + ); + let snapshot = session_store::public_snapshot::project_current_session_snapshot(&raw); + let projected = snapshot + .entries + .iter() + .find(|entry| entry.entry_id == corrected.metadata.entry_id.0) + .unwrap(); + assert!( + matches!(&projected.data, protocol::SessionSnapshotEntryData::ToolResult { is_error: true, attachments, .. } if attachments.is_empty()) + ); + assert!(matches!( + crate::ipc::protocol_session::live_log_entry_event(correction), + Some(Event::SessionEntryCommitted { .. }) + )); + + // Fork/compact seed writers serialize the effective annotated history. + let seed = LogEntry::AnnotatedSegmentStart { + ts: 1, + session_id: worker.session_id(), + system_prompt: None, + config: agen::llm_client::RequestConfig::default(), + history: restored.iter().map(to_logged_history_entry).collect(), + forked_from: None, + compacted_from: None, + }; + let restored_again = + restore_history_entries(worker.session_id(), worker.segment_id(), &[seed]).unwrap(); + assert_eq!( + restored_again + .iter() + .filter(|entry| rejected(&entry.item)) + .count(), + 1 + ); + let mut history = History::from_entries(restored_again); + let out = Engine::<_, Mutable, SessionHistoryMetadata>::new_annotated(client.clone()) + .run_with_annotation(&mut history, "continue after restart", &mut |_| { + Ok(SessionHistoryMetadata::legacy_unknown()) + }) + .await; + assert!(matches!(out.result, EngineRunExit::Finished)); + assert_eq!(client.requests.lock().unwrap().len(), 3); +} + +#[tokio::test] +async fn cancellation_after_image_correction_does_not_rollback_the_correction() { + let (_dir, mut worker, client) = image_worker().await; + *client.cancel_on_retry.lock().unwrap() = Some(worker.engine().cancel_sender()); + let result = worker.run_text("continue").await.unwrap(); + assert_eq!(result, WorkerRunResult::Cancelled); + let raw = worker + .store + .read_all(worker.session_id(), worker.segment_id()) + .unwrap(); + assert!( + raw.iter() + .any(|entry| matches!(entry, LogEntry::ToolResultCorrected { .. })) + ); + let restored = restore_history_entries(worker.session_id(), worker.segment_id(), &raw).unwrap(); + assert_eq!( + restored + .iter() + .filter(|entry| rejected(&entry.item)) + .count(), + 1 + ); +} diff --git a/crates/worker/src/ipc/protocol_session.rs b/crates/worker/src/ipc/protocol_session.rs index 2e84facd2..a9dd18dd7 100644 --- a/crates/worker/src/ipc/protocol_session.rs +++ b/crates/worker/src/ipc/protocol_session.rs @@ -76,6 +76,7 @@ pub fn live_log_entry_event(entry: LogEntry) -> Option { } entry @ (LogEntry::AnnotatedAssistantItem { .. } | LogEntry::AnnotatedToolResult { .. } + | LogEntry::ToolResultCorrected { .. } | LogEntry::RunYielded { .. } | LogEntry::RunResumed { .. } | LogEntry::RunCancelled { .. } diff --git a/crates/worker/src/segment_log_sink.rs b/crates/worker/src/segment_log_sink.rs index 55da19660..93d0b0c73 100644 --- a/crates/worker/src/segment_log_sink.rs +++ b/crates/worker/src/segment_log_sink.rs @@ -121,6 +121,7 @@ impl SegmentLogSink { | LogEntry::AnnotatedUserInput { .. } | LogEntry::AnnotatedAssistantItem { .. } | LogEntry::AnnotatedToolResult { .. } + | LogEntry::ToolResultCorrected { .. } | LogEntry::AnnotatedSystemItem { .. } | LogEntry::Invoke { .. } | LogEntry::RunYielded { .. } diff --git a/crates/worker/src/session_history.rs b/crates/worker/src/session_history.rs index 420429185..5a5f60d8d 100644 --- a/crates/worker/src/session_history.rs +++ b/crates/worker/src/session_history.rs @@ -116,6 +116,17 @@ pub(crate) fn restore_history_entries( } LogEntry::AnnotatedAssistantItem { entry, .. } | LogEntry::AnnotatedToolResult { entry, .. } => history.push(from_logged(entry)), + LogEntry::ToolResultCorrected { entry, .. } => { + let target = history + .iter_mut() + .find(|existing| existing.annotation.entry_id == entry.metadata.entry_id) + .ok_or_else(|| "tool result correction target is missing".to_string())?; + if !matches!((&target.item, &entry.item), (Item::ToolResult { call_id, .. }, session_store::LoggedItem::ToolResult { call_id: replacement, .. }) if call_id == replacement) + { + return Err("tool result correction changed call identity".to_string()); + } + *target = from_logged(entry); + } LogEntry::AnnotatedSystemItem { entry, .. } => history.push(HistoryEntry::new( entry.item.to_history_item(), entry.metadata.clone(), diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index 3914573d4..08ab40402 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -1,3 +1,7 @@ +#[cfg(test)] +#[path = "image_recovery_tests.rs"] +mod image_recovery_tests; + use std::collections::{HashMap, VecDeque}; #[cfg(test)] use std::path::Path; @@ -734,6 +738,17 @@ pub trait WorkspaceClient: std::fmt::Debug + Send + Sync { fn execute(&self, request: WorkspaceRequest) -> Result; + /// Executes one bounded Workspace request when the caller is on a + /// latency-sensitive lifecycle boundary. Implementations without transport + /// timeout support retain their existing execution semantics. + fn execute_with_timeout( + &self, + request: WorkspaceRequest, + _timeout: Duration, + ) -> Result { + self.execute(request) + } + fn execute_server_operation( &self, operation: WorkspaceServerOperation, @@ -4061,6 +4076,24 @@ impl Worker { St: Clone + 'static, { self.materialize_durable_session_head().await?; + let writer = self.log_writer_handle(); + let activity = self.ai_activity_counter.clone(); + self.engine_mut() + .set_image_rejection_handler(move |original, replacement| { + writer + .append_entry(LogEntry::ToolResultCorrected { + ts: segment_log::now_millis(), + entry: session_store::LoggedHistoryEntry { + item: replacement.clone().into(), + metadata: original.annotation.clone(), + }, + }) + .map_err(|error| error.to_string())?; + // A later Cancel must not roll back this committed correction while + // leaving the in-memory result changed (the original may predate Run). + activity.fetch_add(1, Ordering::SeqCst); + Ok(()) + }); self.ensure_interceptor_installed(); if self.should_pre_run_compact() { self.try_pre_run_compact().await?; diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 0ab083230..75d11d8db 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -162,11 +162,11 @@ use crate::hosts::{ RuntimeRegistryError, RuntimeRegistryUnregisterResult, TicketWorkerRole, WorkerCompletionsRequest, WorkerCompletionsResult, WorkerControlOperation, WorkerCreateBinding, WorkerInputDisposition, WorkerInputKind, WorkerInputRequest, WorkerInputResult, - WorkerLifecycleRequest, WorkerLifecycleResult, WorkerSpawnAcceptanceRequirement, - WorkerSpawnIntent, WorkerSpawnRequest, WorkerSpawnResult, WorkerSpawnWorkingDirectoryRequest, - WorkerTicketAssignmentRequest, WorkspaceRuntimeAuthorization, - is_disallowed_remote_runtime_address, is_loopback_runtime_origin, - worker_spawn_create_fingerprint, workspace_worker_summary, + WorkerLifecycleRequest, WorkerLifecycleResult, WorkerSpawnAcceptanceEvidence, + WorkerSpawnAcceptanceRequirement, WorkerSpawnIntent, WorkerSpawnRequest, WorkerSpawnResult, + WorkerSpawnWorkingDirectoryRequest, WorkerTicketAssignmentRequest, + WorkspaceRuntimeAuthorization, is_disallowed_remote_runtime_address, + is_loopback_runtime_origin, worker_spawn_create_fingerprint, workspace_worker_summary, }; use crate::memory_backend::execute_memory_backend_operation_with_authority; use crate::memory_staging::{ @@ -27712,6 +27712,11 @@ async fn create_workspace_worker_inner( Ok(result) => result, Err(error) => return Err(error.into()), }; + let result = if resolved_subjektiv_attached { + restore_reused_subject_worker(&api, result)? + } else { + result + }; Ok(Json(record_browser_worker_spawn( &api, runtime_id, @@ -27721,6 +27726,52 @@ async fn create_workspace_worker_inner( )?)) } +fn restore_reused_subject_worker( + api: &WorkspaceApi, + mut result: WorkerSpawnResult, +) -> ApiResult { + if result.state != InternalWorkerOperationState::Accepted { + return Ok(result); + } + let Some(worker) = result.worker.as_ref() else { + return Ok(result); + }; + if worker.state != "stopped" { + return Ok(result); + } + + let worker_ref = worker.worker.clone(); + let restored = api.restore_workspace_worker(&worker_ref)?; + if restored.state != server_api::WorkerRestoreState::Accepted { + let message = runtime_diagnostics_message(&restored.diagnostics); + return Err(ApiError::with_diagnostics( + Error::RuntimeOperationFailed { + runtime_id: worker_ref.runtime_id, + code: "subject_worker_restore_rejected".to_string(), + message, + }, + restored.diagnostics, + )); + } + let worker = restored + .worker + .ok_or_else(|| Error::RuntimeOperationFailed { + runtime_id: worker_ref.runtime_id, + code: "subject_worker_restore_missing_summary".to_string(), + message: "Runtime restored the existing subject Worker without returning its summary" + .to_string(), + })?; + result.worker = Some(worker); + result.diagnostics.extend(restored.diagnostics); + result + .acceptance_evidence + .push(WorkerSpawnAcceptanceEvidence { + kind: "subject_worker_restored".to_string(), + detail: "the existing stopped subject Worker was restored".to_string(), + }); + Ok(result) +} + fn record_browser_worker_spawn( api: &WorkspaceApi, requested_runtime_id: String, @@ -39609,7 +39660,7 @@ mod tests { async fn ordinary_worker_launch_validates_optional_subjektiv_connection() { let workspace = tempfile::tempdir().unwrap(); init_clean_git_workspace(workspace.path()); - let api = test_api(workspace.path()).await; + let (api, execution) = test_api_with_recording_backend(workspace.path()).await; let store = open_subjektiv_store(&api).unwrap(); let active = store .create_subject(crate::subjektiv::SubjectRole::new("companion").unwrap()) @@ -39704,6 +39755,31 @@ mod tests { first.worker.singleton_key.as_deref(), Some(format!("subjektiv:{}", active.id).as_str()) ); + + let worker = RuntimeWorkerRef::new(&first.runtime_id, &first.worker_id); + let stopped = api + .runtime + .stop_worker( + &worker, + WorkerLifecycleRequest { + reason: Some("test subject Worker restart".to_string()), + ticket_assignment: None, + }, + ) + .unwrap(); + assert_eq!(stopped.state, InternalWorkerOperationState::Accepted); + assert_eq!(api.runtime.worker(&worker).unwrap().state, "stopped"); + execution.accept_restores(); + + let Json(restored) = create_workspace_worker( + State(api.clone()), + HeaderMap::new(), + Json(request(Some(&active.id), "builtin:companion", None)), + ) + .await + .unwrap(); + assert_eq!(restored.worker_id, first.worker_id); + assert_eq!(restored.worker.state, "idle"); } #[tokio::test] diff --git a/docs/development/ui-ux/subject-memory-information.md b/docs/development/ui-ux/subject-memory-information.md new file mode 100644 index 000000000..2a2ff687c --- /dev/null +++ b/docs/development/ui-ux/subject-memory-information.md @@ -0,0 +1,133 @@ +# Subject / Memory information audit (T-692) + +## Decision and scope + +The previous Subjects index exposed `Store revision` alongside identity, state and +update time; Subject detail led with technical metadata and raw surface states. +These numbers require implementation knowledge before they help a user choose a +Subject or understand what is available. The revised hierarchy starts with +identity, connection, committed records and generated-context availability. + +This is a presentation change, not a change to storage, freshness comparisons, +permissions, generation, or Worker history. T-688's ordinary Worker launch and +T-689's independent Subject creation remain the actual operation paths. T-693 +owns editable user-managed behavior: that content must remain distinct from this +page's context generated from committed Memories. A successful read or a current +surface does not attest what is already in any running Worker's history. + +## Field classification and before/after priorities + +| Information | Classification and reason | Presentation decision | +| --- | --- | --- | +| Subject role and ID | Normal identification. Similar or identical roles need a stable identifier for selection and comparison. | Keep role as heading and the complete wrapping ID visible in index and detail. | +| Subject active/retired state | Normal operation decision: whether a new Worker connection can use the Subject. | Replace raw `active` with `Available`; keep `Retired`. Detail explains connection eligibility, not an assertion that a Worker is running. | +| Current Worker | Normal connection decision, derived from the existing Backend singleton projection. | Promote `Connected` / `Not connected` and display name. Do not infer ownership from store revision or claim all Workers received current Memory. Worker start remains the ordinary launch form. | +| Committed-Memory presence | Normal reading decision, independent from surface generation or candidates. | Show `Available` / `None` only on the successful first bounded page. On later pages say `N on this page`; errors say `Unavailable`. Never infer counts from revision or scan all pages. | +| List counts | Normal navigation context, not a store-wide statistic. | Label `N shown` and `more available`, preserving existing bounded cursor pagination. | +| Resident availability | Normal context decision. A missing surface is not missing committed Memory. | `Ready to use`, `Ready, no context`, `Not generated`, `Needs refresh`, `Generation failed`, and request `Unavailable` are distinct. These describe the current generated store artifact, not content truth or Worker-history propagation. | +| Resident body and generated time | Normal content/read freshness context. | Keep the shared Markdown renderer, generated date and exact snapshot reference count. Ready-empty explains that a valid snapshot has no body; stale/failed do not display a partial or old snapshot. | +| Subject store revision | Diagnostic only: internal committed-Memory change number, not record count, schema version or quality score. | Remove it from the index's normal columns. Retain the exact value under detail's `Technical details`, with its meaning explained. Backend comparisons are untouched. | +| Subject update time | Secondary metadata; not a context-injection or generation completion timestamp. | Label `Last changed`; on detail put it with Subject diagnostics rather than competing with the availability summary. | +| Snapshot ID and built-from revision | Diagnostic correlation, distinct from per-Memory revisions. | Retain under `Surface sources and diagnostics`, labelled `Generated from Subject revision` and explained. | +| Surface Memory references | Detailed audit/reachability. | Preserve reference IDs, immutable revision values and Memory links in the disclosure. | +| Memory claim, kind, lifecycle state, excerpt and body | Normal content decision. Active, resolved and retracted are not interchangeable. | Retain list scanning and body reading. Keep the Markdown renderer and current/historical distinction. | +| Individual Memory revision and history | Detailed content comparison, also useful for identifying a selected version. | Explicit `Memory revision` labels, separate from Subject counters. Preserve immutable history, selected revision links and revision pagination. | +| Candidate provenance, evidence, origin, source refs and derivations | Detailed audit, required to understand why a record exists. | Retain IDs, ranges, summaries, excerpts, origins and derived-revision links. Counts say `shown` except where the API provides exact per-candidate totals. Preserve body and evidence continuation links. | +| Raw field names / duplicated general revision terminology | Unnecessary in the normal hierarchy; multiple unrelated counters are misleading. | Replace bare revision labels and raw surface enums with purpose-specific labels. Do not delete API fields, stored information, diagnostics or their detailed access. | + +No diagnostic datum was removed from Subject detail, surface detail or Memory +provenance/history. The index no longer shows a store revision, but the exact +value remains one click away in Subject detail. No invented regeneration, +editing or recovery action was added; `Needs refresh` does not mean an update job +is running. Creation still creates only a Subject, and selection still uses the +existing ordinary launch contract. + +## Candidate-only verification + +Candidates are not committed records and are not an input to this page. The +Subject loader performs just three bounded reads: Subject, resident surface and +`/memories?limit=100[&cursor=...]`. It does not read or count staging records. + +The existing real-store regression +`subjektiv::tests::candidate_decisions_reject_conflicts_and_roll_back_partial_writes` +stages a candidate and attempts an invalid apply. It asserts the candidate has no +resolution and the committed Memory list remains empty. The component regression +`candidate-only Subjects show no committed Memories, not an inferred candidate count` +uses that read projection (store revision 0, ungenerated surface, empty successful +committed list) and checks `None`, `No committed Memories yet.`, `Not connected` +and `Not generated`, with no resident body. Thus a staged candidate does not turn +into a committed-Memory count or a claim that context has been generated. + +The store test proves the persistence boundary; the component test proves the +rendering of its public projection. These are separate boundary tests, **not** a +live browser-to-Backend candidate-creation E2E. The existing `ungenerated` capture +represents the same visible projection, but does not itself stage a candidate. + +## Visual review and validation evidence + +The restored implementation is in `fbb09e65` and `4cf6c130`; this report/test +supplement does not change rendered markup, styling, copy or data contracts. + +- Scenarios: `tools/web-ux/scenarios/memory-subjects-{light,dark}.json`. +- Before: `target/web-ux/t-692-before-{light,dark}` at `cc979874`. +- After: `target/web-ux/t-692-final2-{light,dark}` at clean `4cf6c130`. +- Comparisons: `target/web-ux/t-692-final2-{light,dark}-comparison`. +- Browser interactions: `target/web-ux/t-692-browser-interactions`. +- Persona: synthetic Workspace fixture; permitted creation and intentional + permission-rejection responses. No real account or private store data used. +- Viewports: 1440, 768, 390 and 320px; light and dark themes. +- States: connected/unconnected, active/retired, successful empty lists, ready + long Markdown, ready-empty, ungenerated, stale, failed, unavailable HTTP503, + create form and create permission error; expanded technical disclosures. +- The takeover implementer personally inspected individual before/after images + for index at 768px, representative detail at desktop/mobile and Memory detail, + then ready-empty mobile, ungenerated narrow, stale desktop, failed mobile, + endpoint error at 320px, mobile surface sources, compact Subject diagnostics + and mobile permission error. Oversized contact sheets were rejected by the + image provider; individual viewport PNGs were inspected instead. +- Findings/result: primary state groups replace counters without losing IDs or + document access; long IDs wrap, status is textual rather than color-only, + mobile groups stack, diagnostics remain readable, and error state regions stay + separate. No new visual defect identified. **Pass for the audited scope.** +- Final bundles contain 64 captures each. `completed-with-errors` is expected: + intentional creation HTTP403, endpoint HTTP503/associated aborted requests, + and the deliberately failed surface's alert are retained as evidence. Healthy + capture states have empty error arrays; this is not a blanket claim that all + captures were error-free. Review-context diagnostics are empty. +- The focused production-shell browser test proves main-owned vertical scrolling + with wheel/keyboard/touch, 320px/mobile shell behavior, no page-wide overflow, + local table/code scroll, Markdown safety, creation/launch separation, bounded + pagination and Memory/source/revision reachability. + +Independent review at source `4cf6c130` ran root `cargo check`, +`cargo fmt --all -- --check`, diff hygiene, Web check (0 errors/warnings), 12 +Memory API tests, 452 full Web unit tests, 134 component tests (8 Memory tests), +production build, the focused production-shell browser test (1), and Web-UX unit +tests (16), all passing. + +The takeover implementer then validated this report/test supplement with: + +- `cargo test -p yoi-workspace-server --lib candidate_decisions_reject_conflicts_and_roll_back_partial_writes`: 1 passed. +- Root `cargo check`, `cargo fmt --all -- --check`, and `git diff --check HEAD`: passed. +- Targeted Memory component tests: 9 passed, including the added candidate-only projection test. +- Web `deno task check`: 0 errors/warnings; `deno task test`: 452 passed; + `deno task test:component`: 135 passed. +- Scoped `deno fmt --check` on the changed TypeScript test: passed. +- JSON audit of both final2 bundles: 64 captures each, exact clean source + `4cf6c130`, and no errors outside the deliberate permission/error/failed routes. + +The supplement changes only a document and a test, so it does not require new +rendered-UI captures. Rereview records and the final approved Ticket handoff link +this report to the new exact source. + +Known non-blocking validation diagnostics: component runs emitted incidental +localhost:3000 connection-refused messages while passing; the production build +warned about a large chunk. A broader Web-UX typecheck fails in unchanged +`tools/web-ux/src/processes.ts:44` on a `TextDecoderStream` generic incompatibility; +this report does not claim that broad check passed. + +No live-Backend browser interactions, real Worker model-request propagation, +regeneration, or server/runtime process E2E were performed. No Rust production +crate changed in T-692; the supplemental store test exercises an existing +contract without changing that contract. Backend counters, comparison logic, +storage schema and feature behavior remain intact. diff --git a/tools/web-ux/browser-tests/console_history_shell_test.ts b/tools/web-ux/browser-tests/console_history_shell_test.ts index f83a5f125..909e8e693 100644 --- a/tools/web-ux/browser-tests/console_history_shell_test.ts +++ b/tools/web-ux/browser-tests/console_history_shell_test.ts @@ -596,6 +596,16 @@ async function checkConsoleHistory(viewportHeight: number): Promise { await touchPage.reload(); await touchPage.getByRole("button", { name: "Show sidebar", exact: true }).waitFor(); assertEquals(await touchPage.evaluate(() => localStorage.getItem("yoi.sidebar.mode.v1")), "pinned"); + // Different-route and same-page links both dismiss the mobile overlay. + for (let visit = 0; visit < 2; visit++) { + await touchPage.getByRole("button", { name: "Show sidebar", exact: true }).tap(); + await touchPage.getByRole("link", { name: "Workspace home", exact: true }).tap(); + await touchPage.waitForURL(`**/w/${workspaceId}`); + await touchPage.getByRole("button", { name: "Show sidebar", exact: true }).waitFor(); + assertEquals(await touchPage.locator(".app-shell__main").evaluate((element) => (element as HTMLElement).inert), false); + assertEquals(await touchPage.evaluate(() => localStorage.getItem("yoi.sidebar.mode.v1")), "pinned"); + } + await touchPage.goto(page.url()); // A wide touchscreen still has a temporary-open control in hover mode. await touchPage.setViewportSize({ width: 1024, height: 600 }); await touchPage.getByRole("button", { name: "Unpin sidebar", exact: true }).tap(); diff --git a/tools/web-ux/browser-tests/console_pending_shell_test.ts b/tools/web-ux/browser-tests/console_pending_shell_test.ts index 7be6c4046..57e6f7a16 100644 --- a/tools/web-ux/browser-tests/console_pending_shell_test.ts +++ b/tools/web-ux/browser-tests/console_pending_shell_test.ts @@ -64,6 +64,104 @@ Deno.test("Console pending scopes are independent and match Tasks row typography `${url}/w/console-history-review/workers/W-900-console-fixture/console?pending=${mode}`, ); await page.locator(".task-mini-row").waitFor(); + const metadata = page.getByLabel("Worker model and context", { exact: true }); + const statusGeometry = await metadata.evaluate((status) => { + const task = document.querySelector(".task-mini-row")!; + const composer = document.querySelector(".console-composer")!; + const metrics = (node: Element) => { + const style = getComputedStyle(node); + return [style.fontFamily, style.fontSize, style.lineHeight]; + }; + const rect = status.getBoundingClientRect(); + return { + typography: metrics(status), + taskTypography: metrics(task), + height: rect.height, + belowComposer: rect.top >= composer.getBoundingClientRect().bottom, + alignedWithTasks: rect.left + parseFloat(getComputedStyle(status).paddingLeft) === task.getBoundingClientRect().left, + visible: rect.bottom <= innerHeight, + wrap: getComputedStyle(status).flexWrap, + pageOverflow: document.documentElement.scrollWidth > innerWidth, + }; + }); + assertEquals(statusGeometry.typography, statusGeometry.taskTypography); + assertEquals(statusGeometry.height, 24); + assertEquals(statusGeometry.belowComposer, true); + assertEquals(statusGeometry.alignedWithTasks, true); + assertEquals(statusGeometry.visible, true); + assertEquals(statusGeometry.wrap, "nowrap"); + assertEquals(statusGeometry.pageOverflow, false); + const details = metadata.getByRole("button", { name: "Details", exact: true }); + assertEquals(await page.locator(".console-header").count(), 0); + const detailsGeometry = await details.evaluate((button) => { + const row = button.closest(".worker-context-status")!; + const rect = button.getBoundingClientRect(); + const rowRect = row.getBoundingClientRect(); + return { + rightAligned: Math.abs(rect.right - (rowRect.right - parseFloat(getComputedStyle(row).paddingRight))) < 1, + centered: Math.abs(rect.top + rect.height / 2 - (rowRect.top + rowRect.height / 2)) < 1, + width: rect.width, + }; + }); + assertEquals(detailsGeometry.rightAligned, true); + assertEquals(detailsGeometry.centered, true); + assertEquals(detailsGeometry.width, 24); + const overview = metadata.getByRole("switch", { name: "Overview", exact: true }); + const controls = await overview.evaluate((group) => { + const row = group.closest(".worker-context-status")!; + const rowRect = row.getBoundingClientRect(); + const buttons = [...row.querySelectorAll("button")]; + return { + buttonsInsideRow: buttons.every((button) => { + const rect = button.getBoundingClientRect(); + return rect.left >= rowRect.left && rect.right <= rowRect.right && + rect.top >= rowRect.top && rect.bottom <= rowRect.bottom; + }), + heights: buttons.map((button) => button.getBoundingClientRect().height), + modeFont: getComputedStyle(buttons[0]).fontFamily, + divider: getComputedStyle(buttons[1]).borderLeftWidth, + beforeDetails: group.nextElementSibling?.getAttribute("aria-label") === "Details", + }; + }); + assertEquals(controls.buttonsInsideRow, true); + assertEquals(controls.heights, [24, 24]); + assertEquals(controls.modeFont.includes("mono"), false); + assertEquals(controls.divider, "0px"); + assertEquals(controls.beforeDetails, true); + const onSize = (await overview.boundingBox())!; + assert(onSize.width < 90, "Switch and fixed label should use less than 90px"); + assertEquals(await overview.getAttribute("aria-checked"), "true"); + await overview.focus(); + assertEquals(await overview.evaluate((button) => getComputedStyle(button).outlineStyle), "solid"); + await page.keyboard.press("Enter"); + assertEquals(await overview.getAttribute("aria-checked"), "false"); + assertEquals((await overview.textContent())!.trim(), "Overview"); + assertEquals((await overview.boundingBox())!.width, onSize.width); + if (mode === "both") { + await Deno.mkdir(join(root, "target/web-ux/console-footer-switch"), { recursive: true }); + await page.screenshot({ path: join(root, `target/web-ux/console-footer-switch/${colorScheme}-${width}-off.png`) }); + } + await page.keyboard.press("Space"); + assertEquals(await overview.getAttribute("aria-checked"), "true"); + assertEquals((await overview.boundingBox())!.width, onSize.width); + await overview.locator("span").last().click(); + assertEquals(await overview.getAttribute("aria-checked"), "false"); + await overview.locator(".console-view-toggle-track").click(); + assertEquals(await overview.getAttribute("aria-checked"), "true"); + await details.focus(); + await page.keyboard.press("Enter"); + assertEquals(await details.getAttribute("aria-expanded"), "true"); + await page.getByRole("button", { name: "Close", exact: true }).waitFor(); + await page.keyboard.press("Space"); + assertEquals(await details.getAttribute("aria-expanded"), "false"); + await details.click(); + assertEquals(await details.getAttribute("aria-expanded"), "true"); + await page.getByRole("button", { name: "Close", exact: true }).click(); + assertEquals(await details.getAttribute("aria-expanded"), "false"); + if (mode === "both") { + await Deno.mkdir(join(root, "target/web-ux/console-footer-switch"), { recursive: true }); + await page.screenshot({ path: join(root, `target/web-ux/console-footer-switch/${colorScheme}-${width}.png`) }); + } const queue = mode === "both" || mode === "queue"; const notifications = ["both", "notifications", "legacy"].includes(mode); assertEquals( diff --git a/tools/web-ux/browser-tests/memory_document_fixture_server.ts b/tools/web-ux/browser-tests/memory_document_fixture_server.ts index 3d2c2314b..4fdb49d4a 100644 --- a/tools/web-ux/browser-tests/memory_document_fixture_server.ts +++ b/tools/web-ux/browser-tests/memory_document_fixture_server.ts @@ -73,6 +73,7 @@ const subjects = [ 42, "Prefer explicit evidence over assumptions.\nAsk before irreversible actions, and state uncertainty plainly.", 3, + connectedWorker("Release coordination Worker with a deliberately long display name"), ), subject(emptySubjectId, "Empty ready subject", "active", 0), subject(staleSubjectId, "Stale surface subject", "active", 19), @@ -82,6 +83,30 @@ const subjects = [ ]; const pagedSubject = subject(pagedSubjectId, "Subject on the next page", "active", 1); +function connectedWorker(displayName: string) { + return { + runtime_id: "embedded-worker-runtime", + worker_id: "release-coordination-worker", + host_id: "fixture-host", + display_name: displayName, + label: displayName, + profile: "builtin:companion", + singleton_key: `subjektiv:${representativeSubjectId}`, + tags: [], + workspace: { + visibility: "workspace", + identity: workspaceId, + workspace_id: workspaceId, + }, + state: "idle", + last_seen_at: "2026-01-02T03:04:05Z", + pinned: false, + retention_state: "retained", + implementation: { kind: "embedded", display_hint: "Fixture Worker" }, + diagnostics: [], + }; +} + function subject( id: string, role: string, @@ -89,6 +114,7 @@ function subject( storeRevision: number, behaviorMd = "", behaviorRevision = 0, + currentWorker?: ReturnType, ) { return { id, @@ -99,6 +125,7 @@ function subject( store_revision: storeRevision, created_at: "2026-01-01T00:00:00Z", updated_at: "2026-01-02T03:04:05Z", + ...(currentWorker ? { current_worker: currentWorker } : {}), }; } @@ -141,7 +168,7 @@ function memoriesFor(subjectId: string) { 1, "working_assumption", "retracted", - "T-672 UNIQUE END MARKER", + "Memory list end marker for scroll validation", ), ]; } @@ -251,7 +278,7 @@ function memoryDetail(memoryId: string, requestedRevision: number | null) { evidence_id: "evidence-message-0001", origin: { kind: "human_input", account_id: "account-fixture" }, evidence_kind: "message", - label: "T-672 product direction", + label: "Memory information design source", summary: "Bounded source reference retained with the committed revision.", }], source_refs_total: 1, diff --git a/tools/web-ux/browser-tests/memory_document_shell_test.ts b/tools/web-ux/browser-tests/memory_document_shell_test.ts index 602cab596..615b280df 100644 --- a/tools/web-ux/browser-tests/memory_document_shell_test.ts +++ b/tools/web-ux/browser-tests/memory_document_shell_test.ts @@ -16,7 +16,7 @@ const memoryId = const subjectPageCursor = "fixture-subject-page-2"; const createInteractionArtifacts = join( repositoryRoot, - "target/web-ux/t-693-subject-behavior", + "target/web-ux/t-692-browser-interactions", ); async function freePort(): Promise { @@ -51,7 +51,7 @@ async function markerIsInsideMain(page: Page): Promise { const document = (globalThis as any).document; const main = document.querySelector("main"); const marker = [...document.querySelectorAll("h3")].find((heading) => - heading.textContent === "T-672 UNIQUE END MARKER" + heading.textContent === "Memory list end marker for scroll validation" ); if (!main || !marker) return false; const mainRect = main.getBoundingClientRect(); @@ -270,7 +270,7 @@ Deno.test("production subject Memory and Worker launch shells preserve exact sco assertEquals(await page.locator('a[href="https://example.com"]').count(), 1); assertEquals(await page.locator('a[href^="javascript:"]').count(), 0); assertEquals( - await page.getByRole("article", { name: "Resident surface" }).locator("img, script") + await page.getByRole("article", { name: "Resident context" }).locator("img, script") .count(), 0, ); @@ -290,7 +290,7 @@ Deno.test("production subject Memory and Worker launch shells preserve exact sco await page.locator("main").hover(); await page.mouse.wheel(0, 100_000); } else if (scenario.label === "keyboard") { - await page.getByText("Surface provenance", { exact: true }).focus(); + await page.getByText("Surface sources and diagnostics", { exact: true }).focus(); await page.keyboard.press("End"); } else { await swipeMainToEnd(page); @@ -321,17 +321,17 @@ Deno.test("production subject Memory and Worker launch shells preserve exact sco level: 1, }).waitFor(); assertEquals(await detailPage.getByText("candidate-release-decision-0001").count(), 1); - assertEquals(await detailPage.getByText("T-672 product direction").count(), 1); + assertEquals(await detailPage.getByText("Memory information design source").count(), 1); assertEquals(await detailPage.getByText("memory-derived-source-0002").count(), 1); assertEquals(await detailPage.getByRole("heading", { name: "Revision history" }).count(), 1); - assertEquals(await detailPage.getByText("Revision 3", { exact: true }).count(), 1); - assertEquals(await detailPage.getByText("Revision 2", { exact: true }).count(), 1); - assertEquals(await detailPage.getByText("Revision 1", { exact: true }).count(), 0); + assertEquals(await detailPage.getByText("Memory revision 3", { exact: true }).count(), 1); + assertEquals(await detailPage.getByText("Memory revision 2", { exact: true }).count(), 1); + assertEquals(await detailPage.getByText("Memory revision 1", { exact: true }).count(), 0); await detailPage.getByRole("navigation", { name: "Revision history pages" }).getByRole( "link", { name: "Next page →" }, ).click(); - await detailPage.getByText("Revision 1", { exact: true }).waitFor(); + await detailPage.getByText("Memory revision 1", { exact: true }).waitFor(); assert(new URL(detailPage.url()).searchParams.has("revision_cursor")); assertEquals( await detailPage.getByRole("navigation", { name: "Revision history pages" }).getByRole( @@ -360,6 +360,15 @@ Deno.test("production subject Memory and Worker launch shells preserve exact sco const paginationPage = await paginationContext.newPage(); await paginationPage.goto(`${baseUrl}/w/${workspaceId}/memory`); await paginationPage.getByRole("heading", { name: "Subjects", level: 1 }).waitFor(); + assertEquals( + await paginationPage.getByText( + "Release coordination Worker with a deliberately long display name", + { exact: true }, + ).count(), + 1, + ); + assertEquals(await paginationPage.getByText("Store revision", { exact: true }).count(), 0); + assert((await paginationPage.getByText("Not connected", { exact: true }).count()) > 0); assertEquals(await paginationPage.getByText("Subject on the next page").count(), 0); await paginationPage.getByRole("navigation", { name: "Subject pages" }).getByRole( "link", @@ -473,10 +482,12 @@ Deno.test("production subject Memory and Worker launch shells preserve exact sco await createPage.locator("code.subject-id", { hasText: "created-subject-0001" }).count(), 1, ); - const currentWorker = createPage.locator(".subject-facts div").filter({ - hasText: "Current worker", - }); - assert((await currentWorker.textContent())?.includes("None")); + const currentWorker = createPage.getByText("Worker connection", { exact: true }).locator( + "..", + ); + assert((await currentWorker.textContent())?.includes("Not connected")); + await createPage.getByText("Resident context has not been generated.").waitFor(); + await createPage.getByText("No committed Memories yet.").waitFor(); await createPage.getByRole("button", { name: "Edit" }).click(); const behaviorEditor = createPage.getByRole("textbox", { name: "Subject behavior" }); await behaviorEditor.fill( @@ -519,21 +530,29 @@ Deno.test("production subject Memory and Worker launch shells preserve exact sco const stateCases = [ { id: "empty-subject", - text: "Resident surface is ready and empty.", + text: "Resident context is current but empty.", + role: "status" as const, + }, + { + id: "stale-subject", + text: "Resident context needs to be refreshed.", role: "status" as const, }, - { id: "stale-subject", text: "Resident surface is stale.", role: "status" as const }, { id: "failed-subject", - text: "Resident surface generation failed.", + text: "Resident context generation failed.", role: "alert" as const, }, { id: "ungenerated-subject", - text: "Resident surface has not been generated.", + text: "Resident context has not been generated.", role: "status" as const, }, - { id: "error-subject", text: "Resident surface unavailable.", role: "alert" as const }, + { + id: "error-subject", + text: "Resident context status unavailable.", + role: "alert" as const, + }, ]; for (const stateCase of stateCases) { const page = await stateContext.newPage(); @@ -541,8 +560,14 @@ Deno.test("production subject Memory and Worker launch shells preserve exact sco const state = page.getByRole(stateCase.role).filter({ hasText: stateCase.text }); await state.waitFor(); assert((await state.textContent())?.includes(stateCase.text)); + if (stateCase.id === "stale-subject") { + assertEquals(await page.getByText("A resolved stale-surface lesson").count(), 1); + } + if (stateCase.id === "failed-subject") { + assertEquals(await page.getByText("A retracted failed-surface constraint").count(), 1); + } if (stateCase.id !== "empty-subject") { - assertEquals(await page.getByRole("article", { name: "Resident surface" }).count(), 0); + assertEquals(await page.getByRole("article", { name: "Resident context" }).count(), 0); } await assertNoPageWideOverflow(page); await page.close(); diff --git a/tools/web-ux/scenarios/memory-subjects-dark.json b/tools/web-ux/scenarios/memory-subjects-dark.json index 7fbdebfb0..bfb3de69b 100644 --- a/tools/web-ux/scenarios/memory-subjects-dark.json +++ b/tools/web-ux/scenarios/memory-subjects-dark.json @@ -122,9 +122,33 @@ "dataState": "Ready generated Markdown surface with long content, unsafe Markdown, wide table/code, provenance refs, and three lifecycle states.", "ready": { "kind": "selector", - "selector": "[data-memory-view='subject'][data-memory-state='ready'] article[aria-label='Resident surface']" + "selector": "[data-memory-view='subject'][data-memory-state='ready'] article[aria-label='Resident context']" }, - "capturePoints": [{ "id": "initial", "label": "Ready subject" }] + "capturePoints": [ + { "id": "initial", "label": "Ready subject" }, + { + "id": "subject-diagnostics", + "label": "Subject diagnostics", + "interaction": [ + { "action": "click", "selector": "details.subject-technical-details summary" } + ], + "ready": { + "kind": "selector", + "selector": "details.subject-technical-details[open]" + } + }, + { + "id": "surface-diagnostics", + "label": "Surface sources and diagnostics", + "interaction": [ + { "action": "click", "selector": "details.surface-details summary" } + ], + "ready": { + "kind": "selector", + "selector": "details.surface-details[open]" + } + } + ] }, { "id": "detail", @@ -150,6 +174,18 @@ }, "capturePoints": [{ "id": "initial", "label": "Ready empty" }] }, + { + "id": "ungenerated", + "label": "Ungenerated subject context", + "path": "/w/memory-review/memory/ungenerated-subject", + "goal": "Distinguish a Subject with no generated resident context from lost committed Memory.", + "dataState": "Ungenerated resident context and a successful empty committed Memory list.", + "ready": { + "kind": "selector", + "selector": "[data-memory-view='subject'][data-memory-state='ungenerated']" + }, + "capturePoints": [{ "id": "initial", "label": "Not generated" }] + }, { "id": "stale", "label": "Stale subject surface", diff --git a/tools/web-ux/scenarios/memory-subjects-light.json b/tools/web-ux/scenarios/memory-subjects-light.json index 9ddf6f803..905f15372 100644 --- a/tools/web-ux/scenarios/memory-subjects-light.json +++ b/tools/web-ux/scenarios/memory-subjects-light.json @@ -122,9 +122,33 @@ "dataState": "Ready generated Markdown surface with long content, unsafe Markdown, wide table/code, provenance refs, and three lifecycle states.", "ready": { "kind": "selector", - "selector": "[data-memory-view='subject'][data-memory-state='ready'] article[aria-label='Resident surface']" + "selector": "[data-memory-view='subject'][data-memory-state='ready'] article[aria-label='Resident context']" }, - "capturePoints": [{ "id": "initial", "label": "Ready subject" }] + "capturePoints": [ + { "id": "initial", "label": "Ready subject" }, + { + "id": "subject-diagnostics", + "label": "Subject diagnostics", + "interaction": [ + { "action": "click", "selector": "details.subject-technical-details summary" } + ], + "ready": { + "kind": "selector", + "selector": "details.subject-technical-details[open]" + } + }, + { + "id": "surface-diagnostics", + "label": "Surface sources and diagnostics", + "interaction": [ + { "action": "click", "selector": "details.surface-details summary" } + ], + "ready": { + "kind": "selector", + "selector": "details.surface-details[open]" + } + } + ] }, { "id": "detail", @@ -150,6 +174,18 @@ }, "capturePoints": [{ "id": "initial", "label": "Ready empty" }] }, + { + "id": "ungenerated", + "label": "Ungenerated subject context", + "path": "/w/memory-review/memory/ungenerated-subject", + "goal": "Distinguish a Subject with no generated resident context from lost committed Memory.", + "dataState": "Ungenerated resident context and a successful empty committed Memory list.", + "ready": { + "kind": "selector", + "selector": "[data-memory-view='subject'][data-memory-state='ungenerated']" + }, + "capturePoints": [{ "id": "initial", "label": "Not generated" }] + }, { "id": "stale", "label": "Stale subject surface", diff --git a/web/workspace/src/lib/workspace/api/workers.test.ts b/web/workspace/src/lib/workspace/api/workers.test.ts index f76b0b404..f00c00189 100644 --- a/web/workspace/src/lib/workspace/api/workers.test.ts +++ b/web/workspace/src/lib/workspace/api/workers.test.ts @@ -33,6 +33,7 @@ import { parseWorkerLaunchOptionsResponse, parseWorkerSummary, } from "./workers.ts"; +import { loadJson } from "./http.ts"; const worker = { runtime_id: "runtime-a", @@ -111,6 +112,92 @@ Deno.test("Worker summary parser enforces observation freshness", () => { assertEquals(observedStopped.state, "stopped"); }); +const workerStates = [ + { kind: "idle" }, + { kind: "busy", state: { kind: "run", state: "running" } }, + { kind: "busy", state: { kind: "run", state: "pausing" } }, + { kind: "busy", state: { kind: "run", state: "paused" } }, + { kind: "busy", state: { kind: "run", state: "cancelling" } }, + { kind: "busy", state: { kind: "maintenance", state: "compacting" } }, +] as const; + +Deno.test("Worker summary parser preserves the optional submission completion fence in every state", () => { + for (const state of workerStates) { + for (const completion of [ + {}, + { last_finished_submission_request_id: null }, + { last_finished_submission_request_id: "completed-submit" }, + ]) { + const snapshot = { last_command_id: 7, ...completion, state }; + const parsed = parseWorkerSummary({ + ...worker, + resource_key: "W-1", + availability: "observed", + worker_state: snapshot, + }); + assertEquals(parsed.worker_state, snapshot); + } + } +}); + +Deno.test("Worker summary parser validates submission completion fences without accepting unknown fields", () => { + for (const state of workerStates) { + for (const invalid of [0, false, {}, [], "x".repeat(65_537)]) { + assertThrows( + () => parseWorkerSummary({ + ...worker, + resource_key: "W-1", + availability: "observed", + worker_state: { + last_command_id: 7, + last_finished_submission_request_id: invalid, + state, + }, + }), + Error, + "worker_state.last_finished_submission_request_id", + ); + } + assertThrows( + () => parseWorkerSummary({ + ...worker, + resource_key: "W-1", + availability: "observed", + worker_state: { + last_command_id: 7, + last_finished_submission_request_id: "completed-submit", + state, + unexpected: true, + }, + }), + Error, + "unknown field unexpected", + ); + } +}); + +Deno.test("Console Worker API loading accepts a completed submission snapshot", async () => { + const snapshot = { + last_command_id: 0, + last_finished_submission_request_id: "completed-submit", + state: { kind: "idle" }, + }; + const result = await loadJson( + () => Promise.resolve(Response.json({ + ...worker, + resource_key: "W-1", + availability: "observed", + worker_state: snapshot, + })), + "/api/w/workspace-a/workers/W-1", + undefined, + parseWorkerSummary, + { diagnosticLabel: "Worker API", maxResponseBytes: 8 * 1024 * 1024 }, + ); + assertEquals(result.error, null); + assertEquals(result.data?.worker_state, snapshot); +}); + Deno.test("Worker summary parser preserves attachment-effective External permissions", () => { const attachment = { alias: "sessions", diff --git a/web/workspace/src/lib/workspace/api/workers.ts b/web/workspace/src/lib/workspace/api/workers.ts index 71c76c9f3..df50de158 100644 --- a/web/workspace/src/lib/workspace/api/workers.ts +++ b/web/workspace/src/lib/workspace/api/workers.ts @@ -262,7 +262,16 @@ function workerStateSnapshot( label: string, ): WorkerStateSnapshot { const snapshot = record(value, label); - exact(snapshot, ["last_command_id", "state"], label); + exact( + snapshot, + ["last_command_id", "last_finished_submission_request_id", "state"], + label, + ); + const lastFinishedSubmissionRequestId = optional( + snapshot.last_finished_submission_request_id, + `${label}.last_finished_submission_request_id`, + string, + ); const state = record(snapshot.state, `${label}.state`); const kind = string(state.kind, `${label}.state.kind`); if (kind === "idle") { @@ -272,6 +281,7 @@ function workerStateSnapshot( snapshot.last_command_id, `${label}.last_command_id`, ), + last_finished_submission_request_id: lastFinishedSubmissionRequestId, state: { kind: "idle" }, }; } @@ -291,6 +301,7 @@ function workerStateSnapshot( snapshot.last_command_id, `${label}.last_command_id`, ), + last_finished_submission_request_id: lastFinishedSubmissionRequestId, state: { kind: "busy", state: busyKind === "run" diff --git a/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.browser.spec.ts b/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.browser.spec.ts new file mode 100644 index 000000000..6355eaa4d --- /dev/null +++ b/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.browser.spec.ts @@ -0,0 +1,231 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, waitFor } from "@testing-library/svelte"; +import { afterEach, beforeEach, expect, test, vi } from "vitest"; +import ConfigSourceEditor from "./ConfigSourceEditor.svelte"; + +const toolchain = vi.hoisted(() => ({ + construct: vi.fn(), + setSnapshot: vi.fn(), + analyze: vi.fn(), + complete: vi.fn(), + close: vi.fn(), +})); +vi.mock("./toolchain.ts", () => ({ + ConfigSourceToolchain: class { + constructor() { + toolchain.construct(); + } + setSnapshot = toolchain.setSnapshot; + analyze = toolchain.analyze; + complete = toolchain.complete; + close = toolchain.close; + }, +})); + +const schemaSource = "schema body\n".repeat(6000); +const tree = { + snapshot: { + revision: 7, + digest: "sha256:tree", + entries: { + "main.dcdl": { + path: "main.dcdl", + content_type: "decodal", + content: "WorkspaceConfigSchema({})\n", + content_digest: "sha256:main", + }, + }, + }, + contract: { + contract_version: 1, + decodal_version: "0.4.0", + schema_version: 1, + entrypoints: ["main.dcdl"], + import_policy_version: 1, + schema_bundle: { + contributions: [{ + provider_id: "builtin", + namespace: "workspace", + version: "1", + source: schemaSource, + source_digest: "sha256:contribution", + }], + source: schemaSource, + fingerprint: "sha256:schema", + }, + fingerprint: "sha256:toolchain", + }, + projection_digest: "sha256:projection", +}; +const fetcher = vi.fn(); +const response = () => new Response(JSON.stringify(tree)); + +beforeEach(() => { + vi.resetAllMocks(); + vi.stubGlobal("fetch", fetcher); + fetcher.mockImplementation(async () => response()); + toolchain.setSnapshot.mockResolvedValue(undefined); + toolchain.analyze.mockResolvedValue([]); + toolchain.complete.mockResolvedValue(null); +}); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +function renderEditor(workspaceId = "w") { + return render(ConfigSourceEditor, { workspaceId }); +} + +function expectNoEditor(view: ReturnType) { + expect(view.queryByText("Select or create a source")).toBeNull(); + expect(view.queryByRole("button", { name: "Create local source" })) + .toBeNull(); + expect(view.queryByRole("button", { name: "Commit" })).toBeNull(); + expect(view.container.querySelector(".cm-editor")).toBeNull(); +} + +async function expectReady(view: ReturnType) { + await waitFor(() => + expect(view.getByRole("button", { name: "main.dcdl entrypoint" })).not + .toBeNull() + ); + expect(view.container.querySelector('.cm-content[contenteditable="true"]')) + .not.toBeNull(); + expect(view.container.textContent).toContain("WorkspaceConfigSchema({})"); + expect(view.queryByRole("alert")).toBeNull(); +} + +test("loading is not an empty source tree; editing waits for schema installation", async () => { + let finishFetch!: (response: Response) => void; + let finishSetup!: () => void; + fetcher.mockReturnValueOnce( + new Promise((resolve) => { + finishFetch = resolve; + }), + ); + toolchain.setSnapshot.mockReturnValueOnce( + new Promise((resolve) => { + finishSetup = resolve; + }), + ); + const view = render(ConfigSourceEditor, { workspaceId: "w/one" }); + expect(view.getByRole("status").textContent).toContain("Loading source tree"); + expectNoEditor(view); + finishFetch(response()); + await waitFor(() => + expect(toolchain.setSnapshot).toHaveBeenCalledWith( + tree.snapshot, + tree.contract.schema_bundle, + ) + ); + expectNoEditor(view); + finishSetup(); + await expectReady(view); + expect(fetcher).toHaveBeenCalledWith( + "/api/w/w%2Fone/config/source-tree", + expect.anything(), + ); +}); + +test.each(["http", "invalid"])( + "%s failure has a manual reload, not creation controls or automatic retries", + async (failure) => { + fetcher.mockResolvedValueOnce( + failure === "http" + ? new Response("Backend unavailable", { status: 503 }) + : new Response(JSON.stringify({ ...tree, unexpected: true })), + ); + const view = renderEditor(); + await waitFor(() => + expect(view.getByRole("alert").textContent).toContain( + "Unable to load workspace configuration", + ) + ); + expect(view.getByRole("alert").textContent).toContain( + failure === "http" ? "Backend unavailable" : "invalid response", + ); + expectNoEditor(view); + expect(toolchain.setSnapshot).not.toHaveBeenCalled(); + await new Promise((resolve) => setTimeout(resolve, 300)); + expect(fetcher).toHaveBeenCalledOnce(); + await fireEvent.click( + view.getByRole("button", { name: "Reload source tree" }), + ); + await expectReady(view); + expect(fetcher).toHaveBeenCalledTimes(2); + }, +); + +test.each(["construct", "setSnapshot"] as const)( + "toolchain %s failure is caught and manual reload initializes a fresh toolchain", + async (phase) => { + if (phase === "construct") { + toolchain.construct.mockImplementationOnce(() => { + throw new Error("Worker unavailable"); + }); + } else {toolchain.setSnapshot.mockRejectedValueOnce( + new Error("Schema setup failed"), + );} + const view = renderEditor(); + await waitFor(() => + expect(view.getByRole("alert").textContent).toContain( + phase === "construct" ? "Worker unavailable" : "Schema setup failed", + ) + ); + expectNoEditor(view); + await fireEvent.click( + view.getByRole("button", { name: "Reload source tree" }), + ); + await expectReady(view); + expect(toolchain.construct).toHaveBeenCalledTimes(2); + }, +); + +test("a successfully loaded empty tree still offers source creation", async () => { + fetcher.mockResolvedValueOnce( + new Response(JSON.stringify({ + ...tree, + snapshot: { ...tree.snapshot, entries: {} }, + })), + ); + const view = renderEditor(); + await waitFor(() => + expect(view.getByText("Select or create a source")).not.toBeNull() + ); + expect(view.getByRole("button", { name: "Create local source" })).not + .toBeNull(); + expect(view.queryByRole("alert")).toBeNull(); +}); + +test("unmount during schema setup closes the toolchain", async () => { + let failSetup!: (error: Error) => void; + toolchain.setSnapshot.mockReturnValueOnce( + new Promise((_, reject) => { + failSetup = reject; + }), + ); + toolchain.close.mockImplementationOnce(() => + failSetup(new Error("toolchain closed")) + ); + const view = renderEditor(); + await waitFor(() => expect(toolchain.setSnapshot).toHaveBeenCalledOnce()); + view.unmount(); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(toolchain.close).toHaveBeenCalledOnce(); +}); + +test("unmount during fetch does not start a toolchain", async () => { + let finishFetch!: (response: Response) => void; + fetcher.mockReturnValueOnce( + new Promise((resolve) => { + finishFetch = resolve; + }), + ); + const view = renderEditor(); + view.unmount(); + finishFetch(response()); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(toolchain.construct).not.toHaveBeenCalled(); +}); diff --git a/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.svelte b/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.svelte index 0cf4471d4..78f9edb44 100644 --- a/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.svelte +++ b/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.svelte @@ -21,6 +21,8 @@ let newPath = $state("module.dcdl"); let diagnostics = $state([]); let status = $state("Loading source tree…"); + let loadState = $state<"loading" | "error" | "ready">("loading"); + let mounted = false; let busy = $state(false); let workingChanges = $state([]); let baseRevision = $state(0); @@ -42,9 +44,12 @@ const dirty = $derived(workingChanges.length > 0 || (selected ? source !== selected.content : source.length > 0)); onMount(() => { - toolchain = new ConfigSourceToolchain(); + mounted = true; void reload(); - return () => toolchain?.close(); + return () => { + mounted = false; + toolchain?.close(); + }; }); $effect(() => { @@ -70,9 +75,20 @@ }); async function reload() { + loadState = "loading"; + status = "Loading source tree…"; analysisReady = false; + analysisGeneration += 1; try { - treeState = await fetchConfigTree(workspaceId); + const remote = await fetchConfigTree(workspaceId); + if (!mounted) return; + toolchain?.close(); + toolchain = null; + toolchain = new ConfigSourceToolchain(); + await toolchain.setSnapshot(remote.snapshot, remote.contract.schema_bundle); + if (!mounted) return; + // Publish the new baseline only after both fetch and toolchain setup succeed. + treeState = remote; if (!selectedPath || !treeState.snapshot.entries[selectedPath]) { selectedPath = Object.keys(treeState.snapshot.entries).toSorted()[0] ?? ""; } @@ -80,15 +96,19 @@ baseSnapshot = $state.snapshot(treeState.snapshot); baseRevision = treeState.snapshot.revision; baseDigest = treeState.snapshot.digest; - await toolchain?.setSnapshot(treeState.snapshot, treeState.contract.schema_bundle); analysisReady = true; workingChanges = []; renamePath = selectedPath; diagnostics = []; conflict = false; status = `Revision ${treeState.snapshot.revision} · ${treeState.snapshot.digest.slice(0, 20)}…`; + loadState = "ready"; } catch (error) { + if (!mounted) return; + toolchain?.close(); + toolchain = null; status = String(error); + loadState = "error"; } } @@ -213,8 +233,6 @@ } async function discardAndReload() { - workingChanges = []; - source = ""; await reload(); } @@ -287,7 +305,18 @@ } -
+
+ {#if loadState === "loading"} +
Loading source tree…
+ {:else if loadState === "error"} +
+
+ Unable to load workspace configuration +

{status}

+
+ +
+ {:else}
+ + diff --git a/web/workspace/src/lib/workspace/config-source/api.ts b/web/workspace/src/lib/workspace/config-source/api.ts index 479ba0b9b..c277ccc5d 100644 --- a/web/workspace/src/lib/workspace/config-source/api.ts +++ b/web/workspace/src/lib/workspace/config-source/api.ts @@ -11,6 +11,9 @@ import type { } from "./types.ts"; const MAX_RESPONSE_BYTES = 8 * 1024 * 1024; +// Schema bodies are documents, not short metadata. Keep them within the +// response budget; readJson also bounds the combined bundle and contributions. +const MAX_SCHEMA_SOURCE_BYTES = MAX_RESPONSE_BYTES; const MAX_ERROR_BYTES = 4 * 1024; const MAX_ENTRY_COUNT = 256; const MAX_ENTRY_BYTES = 256 * 1024; @@ -193,7 +196,7 @@ function parseContribution(value: unknown): ConfigSchemaContribution { provider_id: boundedString(item.provider_id), namespace: boundedString(item.namespace), version: boundedString(item.version), - source: boundedString(item.source), + source: boundedString(item.source, MAX_SCHEMA_SOURCE_BYTES), ...(projection === undefined ? {} : { projection_validator: projection }), source_digest: boundedString(item.source_digest, MAX_DIGEST_BYTES), }; @@ -203,7 +206,7 @@ function parseSchemaBundle(value: unknown): WorkspaceConfigSchemaBundle { const item = record(value, ["contributions", "source", "fingerprint"]); return { contributions: boundedArray(item.contributions).map(parseContribution), - source: boundedString(item.source), + source: boundedString(item.source, MAX_SCHEMA_SOURCE_BYTES), fingerprint: boundedString(item.fingerprint, MAX_DIGEST_BYTES), }; } diff --git a/web/workspace/src/lib/workspace/console/WorkerContextStatus.browser.spec.ts b/web/workspace/src/lib/workspace/console/WorkerContextStatus.browser.spec.ts index f45776f62..fd37c8bcf 100644 --- a/web/workspace/src/lib/workspace/console/WorkerContextStatus.browser.spec.ts +++ b/web/workspace/src/lib/workspace/console/WorkerContextStatus.browser.spec.ts @@ -6,6 +6,23 @@ import WorkerContextStatus from "./WorkerContextStatus.svelte"; afterEach(cleanup); +test("the three-dot Details control invokes the existing toggle and reflects its state", async () => { + let toggles = 0; + const view = render(WorkerContextStatus, { + metadata: null, + detailsOpen: false, + onToggleDetails: () => { toggles++; }, + }); + const button = view.getByRole("button", { name: "Details" }); + expect(button.querySelectorAll("circle")).toHaveLength(3); + expect(button.textContent?.trim()).toBe(""); + expect(button.getAttribute("aria-expanded")).toBe("false"); + button.click(); + expect(toggles).toBe(1); + await view.rerender({ detailsOpen: true }); + expect(button.getAttribute("aria-expanded")).toBe("true"); +}); + test("renders resolved model, reasoning, and measured context accessibly", () => { const view = render(WorkerContextStatus, { metadata: { @@ -19,13 +36,13 @@ test("renders resolved model, reasoning, and measured context accessibly", () => const status = view.getByLabelText("Worker model and context"); expect(status.textContent).toContain("gpt-6-astra · high"); - expect(status.textContent).toContain("Context 142.0k / 272.0k (52%)"); + expect(status.textContent).toContain("142.0k / 272.0k (52%)"); }); test("renders honest unavailable state instead of a fabricated zero percent", async () => { const view = render(WorkerContextStatus, { metadata: null }); expect(view.getByText("Model unavailable")).not.toBeNull(); - expect(view.getByText("Context unavailable")).not.toBeNull(); + expect(view.getByText("unavailable")).not.toBeNull(); expect(view.container.textContent).not.toContain("0%"); await view.rerender({ @@ -38,7 +55,7 @@ test("renders honest unavailable state instead of a fabricated zero percent", as }, }); expect(view.container.textContent).toContain("claude · 8.2k token budget"); - expect(view.container.textContent).toContain("Context ~12.0k / 64.0k (19%)"); + expect(view.container.textContent).toContain("~12.0k / 64.0k (19%)"); await view.rerender({ metadata: { diff --git a/web/workspace/src/lib/workspace/console/WorkerContextStatus.svelte b/web/workspace/src/lib/workspace/console/WorkerContextStatus.svelte index 248cf5025..6f6ad9db1 100644 --- a/web/workspace/src/lib/workspace/console/WorkerContextStatus.svelte +++ b/web/workspace/src/lib/workspace/console/WorkerContextStatus.svelte @@ -1,4 +1,5 @@
{modelSummary} - + {contextSummary} + {#if controls || onToggleDetails} +
+ {@render controls?.()} + {#if onToggleDetails} + + {/if} +
+ {/if}
diff --git a/web/workspace/src/lib/workspace/console/model.test.ts b/web/workspace/src/lib/workspace/console/model.test.ts index 0f6e54dc4..9ba89bec5 100644 --- a/web/workspace/src/lib/workspace/console/model.test.ts +++ b/web/workspace/src/lib/workspace/console/model.test.ts @@ -3769,6 +3769,29 @@ Deno.test("committed tool identities reconcile one live call block", () => { ); }); +Deno.test("image rejection correction updates the existing tool result without duplicate rows", () => { + const result: SessionSnapshotEntry = { + kind: "tool_result", entry_id: "result-image", timestamp: 2, + provenance: "tool_output", call_id: "image", summary: "Attached image", is_error: false, + attachments: [{ attachment_id: "image-body", media_type: "image/png", byte_len: 100 }], + }; + const lines = projectConsole([ + { eventId: "call", event: { event: "session_entry_committed", data: { entry: { + kind: "tool_call", entry_id: "call-image", timestamp: 1, provenance: "model_output", + call_id: "image", name: "ViewImage", arguments: "{}", + } } } }, + { eventId: "original", event: { event: "session_entry_committed", data: { entry: result } } }, + { eventId: "correction", event: { event: "session_entry_committed", data: { entry: { + ...result, timestamp: 3, summary: "Resize or crop and retry ViewImage", is_error: true, attachments: [], + } } } }, + ]).lines; + assertEquals(lines.length, 1); + assertEquals(lines[0].entryId, "result-image"); + assertEquals(lines[0].toolCall?.state, "error"); + assertEquals(lines[0].attachments ?? [], []); + assert(lines[0].body.includes("Resize or crop"), "show the model recovery instruction"); +}); + function readHistory(ids: string[], summary: string): ConsoleLine[] { const entries = ids.flatMap((id): SessionSnapshotEntry[] => [{ kind: "tool_call", entry_id: `call-entry-${id}`, timestamp: 1, diff --git a/web/workspace/src/lib/workspace/console/run-status.test.ts b/web/workspace/src/lib/workspace/console/run-status.test.ts index 29de10947..bc8a9f989 100644 --- a/web/workspace/src/lib/workspace/console/run-status.test.ts +++ b/web/workspace/src/lib/workspace/console/run-status.test.ts @@ -45,7 +45,7 @@ Deno.test("worker metadata formats effort, budget, estimate, and unknown safely" contextTokens: 142_000, contextSource: "estimated", }), - "Context ~142.0k / 272.0k (52%)", + "~142.0k / 272.0k (52%)", ); assertEquals(formatModelSummary(null), "Model unavailable"); assertEquals( @@ -64,7 +64,7 @@ Deno.test("worker metadata formats effort, budget, estimate, and unknown safely" formatReasoning({ kind: "budget_tokens", budget_tokens: -2 }), "reasoning unavailable", ); - assertEquals(formatContextSummary(null), "Context unavailable"); + assertEquals(formatContextSummary(null), "unavailable"); assertEquals( formatContextSummary({ model: null, @@ -73,7 +73,7 @@ Deno.test("worker metadata formats effort, budget, estimate, and unknown safely" contextTokens: 0, contextSource: null, }), - "Context unavailable", + "unavailable", ); }); 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 ba0543d4d..10acf6efa 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 @@ -502,7 +502,9 @@ Deno.test("workspace Memory surfaces use read-only scoped memory APIs", async () subjectIndexLoad.includes('"/subjektiv/subjects"') && subjectIndexLoad.includes("parseSubjektivSubjectListResponse") && subjectIndexPage.includes(">Subjects") && - subjectIndexPage.includes("subject.store_revision") && + subjectIndexPage.includes("Worker connection") && + subjectIndexPage.includes("subject.current_worker") && + !subjectIndexPage.includes("subject.store_revision") && subjectIndexPage.includes('data-memory-view="subjects"'), "Memory product entry should list explicit typed subjects", ); @@ -510,10 +512,13 @@ Deno.test("workspace Memory surfaces use read-only scoped memory APIs", async () subjectPageLoad.includes("`${subjectPath}/surface`") && subjectPageLoad.includes("`${subjectPath}/memories`") && subjectPage.includes("DocumentMarkdown from") && - subjectPage.includes("Resident surface") && + subjectPage.includes("Resident context") && + subjectPage.includes("Subject store revision") && subjectPage.includes("Current Memories") && subjectPage.includes("surface?.availability === 'ready'") && + subjectPage.includes("surface?.availability === 'ungenerated'") && subjectPage.includes("surface?.availability === 'stale'") && + subjectPage.includes("surface?.availability === 'failed'") && subjectPage.includes("data-surface-ready-empty") && !subjectPage.includes("overflow-y"), "Subject page should combine strict resident-surface and current-Memory reads without a nested vertical scroller", @@ -738,6 +743,32 @@ Deno.test("Worker Console renders Edit diffs without preformatted template gaps" ); }); +Deno.test("Model information stays below Composer in one compact monospace row", async () => { + const page = await Deno.readTextFile(new URL( + "./../../../routes/w/[workspaceId]/workers/[workerRef]/console/+page.svelte", + import.meta.url, + )); + const composer = page.indexOf('
", composer); + const status = page.indexOf("= 0 && composerEnd > composer && status > composerEnd, "Model information must follow Composer"); + assert(page.includes("metadata={selectedConsoleProjection.workerMetadata}"), "Keep selected Worker metadata"); + assert(page.indexOf(" status && modes < page.indexOf("", status), "Display toggle must be in the model information row"); + assert(!page.includes("console-header"), "Remove the empty header and its styles"); + assert(component.indexOf("{@render controls?.()}") < component.indexOf('class="details-button"'), "Display controls must precede Details"); + assert(/\.worker-context-actions\s*\{[^}]*flex: 0 0 auto/.test(component), "Keep display controls from shrinking on narrow screens"); + assert(component.includes("margin-left: auto"), "Align footer controls to the right"); +}); + Deno.test("Console spacing and text metrics use existing design tokens", async () => { const files = [ "ComposerInput.svelte", @@ -1551,14 +1582,10 @@ Deno.test("Composer owns TUI-style completion and replaces Compact/Rewind header const input = await Deno.readTextFile( new URL("./ComposerInput.svelte", import.meta.url), ); - const header = page.slice( - page.indexOf('
{"), "Only the mini summary should own Tasks toggling", ); diff --git a/web/workspace/src/lib/workspace/console/worker-metadata.ts b/web/workspace/src/lib/workspace/console/worker-metadata.ts index 03f9618e3..d9217cb61 100644 --- a/web/workspace/src/lib/workspace/console/worker-metadata.ts +++ b/web/workspace/src/lib/workspace/console/worker-metadata.ts @@ -36,11 +36,11 @@ export function formatContextSummary( used === null || used === undefined || limit === null || limit === undefined || !Number.isSafeInteger(used) || !Number.isSafeInteger(limit) || used < 0 || limit <= 0 - ) return "Context unavailable"; + ) return "unavailable"; const percent = Math.round((used / limit) * 100); - if (!Number.isFinite(percent)) return "Context unavailable"; + if (!Number.isFinite(percent)) return "unavailable"; const estimate = metadata?.contextSource === "estimated" ? "~" : ""; - return `Context ${estimate}${formatRunTokens(used)} / ${ + return `${estimate}${formatRunTokens(used)} / ${ formatRunTokens(limit) } (${percent}%)`; } diff --git a/web/workspace/src/lib/workspace/settings/DecodalSourceEditor.browser.spec.ts b/web/workspace/src/lib/workspace/settings/DecodalSourceEditor.browser.spec.ts new file mode 100644 index 000000000..baf7c6a9f --- /dev/null +++ b/web/workspace/src/lib/workspace/settings/DecodalSourceEditor.browser.spec.ts @@ -0,0 +1,153 @@ +// @vitest-environment happy-dom + +import { + type CompletionResult, + completionStatus, + startCompletion, +} from "@codemirror/autocomplete"; +import { EditorView } from "@codemirror/view"; +import { cleanup, fireEvent, render, waitFor } from "@testing-library/svelte"; +import { afterEach, expect, test, vi } from "vitest"; +import DecodalSourceEditor from "./DecodalSourceEditor.svelte"; + +afterEach(cleanup); + +function setup( + value: string, + props: { + readonly?: boolean; + fixedSchemaWrapper?: boolean; + onComplete?: ( + source: string, + offset: number, + explicit: boolean, + ) => Promise; + } = {}, +) { + const onChange = vi.fn(); + const result = render(DecodalSourceEditor, { value, onChange, ...props }); + const content = result.container.querySelector(".cm-content")!; + const editor = EditorView.findFromDOM(content)!; + editor.focus(); + return { ...result, content, editor, onChange }; +} + +test.each([ + { name: "plain line", value: "name = 1", anchor: 8, expected: "name = 1\n" }, + { + name: "object body", + value: "{\n name = 1", + anchor: 12, + expected: "{\n name = 1\n ", + }, + { name: "empty object", value: "{}", anchor: 1, expected: "{\n \n}" }, + { + name: "nested object", + value: "{\n nested = {}\n}", + anchor: 14, + expected: "{\n nested = {\n \n }\n}", + }, +])( + "Enter inserts an indented newline in $name", + async ({ value, anchor, expected }) => { + const { content, editor, onChange } = setup(value); + editor.dispatch({ selection: { anchor } }); + expect(await fireEvent.keyDown(content, { key: "Enter", code: "Enter" })) + .toBe(false); + expect(editor.state.doc.toString()).toBe(expected); + expect(onChange).toHaveBeenLastCalledWith(expected); + }, +); + +test("Enter indents inside the fixed wrapper without altering its assertion", async () => { + const { content, editor } = setup("{} as WorkspaceConfigSchema\n", { + fixedSchemaWrapper: true, + }); + await fireEvent.keyDown(content, { key: "Enter", code: "Enter" }); + expect(editor.state.doc.toString()).toBe( + "{\n \n} as WorkspaceConfigSchema\n", + ); + expect(editor.state.selection.main.head).toBe(4); +}); + +test("Enter does not mutate readonly sources", async () => { + const value = "{\n name = 1"; + const { content, editor, onChange } = setup(value, { readonly: true }); + editor.dispatch({ selection: { anchor: value.length } }); + await fireEvent.keyDown(content, { key: "Enter", code: "Enter" }); + expect(editor.state.doc.toString()).toBe(value); + expect(onChange).not.toHaveBeenCalled(); +}); + +test("Enter adds an indented newline instead of accepting a visible completion", async () => { + const value = "{\n na"; + const { content, editor } = setup(value, { + onComplete: async () => ({ + from: 4, + options: [{ label: "name", type: "property" }], + }), + }); + editor.dispatch({ selection: { anchor: value.length } }); + startCompletion(editor); + await waitFor(() => expect(completionStatus(editor.state)).toBe("active")); + // Wait past CodeMirror's completion interaction delay, so accepting a + // suggestion would be observable rather than ignored as an early keypress. + await new Promise((resolve) => setTimeout(resolve, 100)); + await fireEvent.keyDown(content, { key: "Enter", code: "Enter" }); + expect(editor.state.doc.toString()).toBe("{\n na\n "); + await waitFor(() => expect(completionStatus(editor.state)).toBeNull()); +}); + +test("Tab indents the current line and Shift+Tab removes indentation", async () => { + const { content, editor, onChange } = setup("name = 1"); + editor.dispatch({ selection: { anchor: 4 } }); + expect(await fireEvent.keyDown(content, { key: "Tab", code: "Tab" })).toBe( + false, + ); + expect(editor.state.doc.toString()).toBe(" name = 1"); + expect(onChange).toHaveBeenLastCalledWith(" name = 1"); + expect(editor.hasFocus).toBe(true); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab", shiftKey: true }); + expect(editor.state.doc.toString()).toBe("name = 1"); + expect(onChange).toHaveBeenLastCalledWith("name = 1"); +}); + +test("Tab and Shift+Tab indent every selected line", async () => { + const value = "first = 1\nsecond = 2"; + const { content, editor } = setup(value); + editor.dispatch({ selection: { anchor: 0, head: value.length } }); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab" }); + expect(editor.state.doc.toString()).toBe(" first = 1\n second = 2"); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab", shiftKey: true }); + expect(editor.state.doc.toString()).toBe(value); +}); + +test("Tab inserts indentation on a blank line", async () => { + const { content, editor } = setup(""); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab" }); + expect(editor.state.doc.toString()).toBe(" "); +}); + +test("Tab and Shift+Tab do not mutate readonly sources", async () => { + const value = " name = 1"; + const { content, editor, onChange } = setup(value, { readonly: true }); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab" }); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab", shiftKey: true }); + expect(editor.state.doc.toString()).toBe(value); + expect(onChange).not.toHaveBeenCalled(); +}); + +test("Tab indents the schema body without modifying the fixed wrapper", async () => { + const value = "{\n name = 1\n} as WorkspaceConfigSchema\n"; + const { content, editor } = setup(value, { fixedSchemaWrapper: true }); + editor.dispatch({ selection: { anchor: value.indexOf("name") } }); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab" }); + expect(editor.state.doc.toString()).toBe( + "{\n name = 1\n} as WorkspaceConfigSchema\n", + ); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab", shiftKey: true }); + expect(editor.state.doc.toString()).toBe(value); + editor.dispatch({ selection: { anchor: 0 } }); + await fireEvent.keyDown(content, { key: "Tab", code: "Tab" }); + expect(editor.state.doc.toString()).toBe(value); +}); diff --git a/web/workspace/src/lib/workspace/settings/DecodalSourceEditor.svelte b/web/workspace/src/lib/workspace/settings/DecodalSourceEditor.svelte index 5fd974cec..7bb39e679 100644 --- a/web/workspace/src/lib/workspace/settings/DecodalSourceEditor.svelte +++ b/web/workspace/src/lib/workspace/settings/DecodalSourceEditor.svelte @@ -9,6 +9,7 @@ type CompletionContext, type CompletionResult, } from '@codemirror/autocomplete'; + import { indentWithTab, insertNewlineAndIndent } from '@codemirror/commands'; import { HighlightStyle, syntaxHighlighting } from '@codemirror/language'; import { Compartment, EditorState } from '@codemirror/state'; import { EditorView, keymap, lineNumbers, highlightActiveLine, drawSelection, type ViewUpdate } from '@codemirror/view'; @@ -129,6 +130,8 @@ decodal({ highlight: false }), syntaxHighlighting(syntaxTheme), autocompletion({ + // Use the explicit keymap below so Enter never accepts a suggestion. + defaultKeymap: false, activateOnTyping: false, override: [ async (context: CompletionContext) => { @@ -139,6 +142,10 @@ ], }), keymap.of(completionKeymapWithoutEnter), + keymap.of([ + { key: 'Enter', run: insertNewlineAndIndent }, + indentWithTab, + ]), fixedSchemaWrapperCompartment.of( initialFixedSchemaWrapper ? fixedSchemaWrapperExtension() : [], ), diff --git a/web/workspace/src/lib/workspace/sidebar/SidebarFrame.svelte b/web/workspace/src/lib/workspace/sidebar/SidebarFrame.svelte index 60dbd596e..760759d9e 100644 --- a/web/workspace/src/lib/workspace/sidebar/SidebarFrame.svelte +++ b/web/workspace/src/lib/workspace/sidebar/SidebarFrame.svelte @@ -1,5 +1,6 @@ diff --git a/web/workspace/src/lib/workspace/sidebar/sidebar-navigation.browser.spec.ts b/web/workspace/src/lib/workspace/sidebar/sidebar-navigation.browser.spec.ts new file mode 100644 index 000000000..db602e352 --- /dev/null +++ b/web/workspace/src/lib/workspace/sidebar/sidebar-navigation.browser.spec.ts @@ -0,0 +1,63 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render } from "@testing-library/svelte"; +import { createRawSnippet } from "svelte"; +import { afterEach, beforeEach, expect, test, vi } from "vitest"; +import SidebarFrame from "./SidebarFrame.svelte"; + +// Stop happy-dom navigation after the component has handled the bubbling click. +const preventBrowserNavigation = (event: MouseEvent) => event.preventDefault(); +beforeEach(() => document.addEventListener("click", preventBrowserNavigation)); +afterEach(() => { + cleanup(); + document.removeEventListener("click", preventBrowserNavigation); +}); + +function mount(mobile = true, mode: "pinned" | "hover" = "pinned") { + const onOpenChange = vi.fn(); + const onModeChange = vi.fn(); + const children = createRawSnippet(() => ({ + render: () => ``, + })); + return { ...render(SidebarFrame, { + mobile, mode, open: true, children, onOpenChange, onModeChange, + }), onOpenChange, onModeChange }; +} + +test.each(["pinned", "hover"] as const)("mobile %s closes on nested, current-page and hash links", async (mode) => { + const view = mount(true, mode); + for (const label of ["Workspace home", "Current page", "Section"]) { + view.onOpenChange.mockClear(); + await fireEvent.click(view.getByText(label)); + expect(view.onOpenChange).toHaveBeenCalledExactlyOnceWith(false); + expect(view.onModeChange).not.toHaveBeenCalled(); + } +}); + +test("mobile ignores disclosure buttons, modified clicks, new tabs, downloads and cancelled navigation", async () => { + const view = mount(); + for (const label of ["Expand group", "New tab", "Download"]) { + await fireEvent.click(view.getByText(label)); + } + const link = view.getByText("Workspace home"); + for (const options of [{ ctrlKey: true }, { metaKey: true }, { shiftKey: true }, { altKey: true }, { button: 1 }]) { + await fireEvent.click(link, options); + } + link.addEventListener("click", (event) => event.preventDefault(), { once: true }); + await fireEvent.click(link); + expect(view.onOpenChange).not.toHaveBeenCalled(); +}); + +test.each(["pinned", "hover"] as const)("desktop %s navigation does not change sidebar mode or open state", async (mode) => { + const view = mount(false, mode); + await fireEvent.click(view.getByText("Workspace home")); + expect(view.onOpenChange).not.toHaveBeenCalled(); + expect(view.onModeChange).not.toHaveBeenCalled(); +}); diff --git a/web/workspace/src/routes/w/[workspaceId]/memory/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/memory/+page.svelte index b65e624e3..2f10695ec 100644 --- a/web/workspace/src/routes/w/[workspaceId]/memory/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/memory/+page.svelte @@ -34,6 +34,10 @@ return cursor ? `${path}?cursor=${encodeURIComponent(cursor)}` : path; } + function subjectStateLabel(state: SubjektivSubjectResponse['state']): string { + return state === 'active' ? 'Available' : 'Retired'; + } + async function openCreateForm(): Promise { createExpanded = true; createError = null; @@ -121,7 +125,7 @@
{#if data.subjects.data} - {subjects.length}{data.subjects.data.has_more ? '+' : ''} subject{subjects.length === 1 ? '' : 's'} + {subjects.length} shown{data.subjects.data.has_more ? ' · more available' : ''} {/if}
-
Memory id
{memory.id}
-
Revision
{memory.revision}
-
Updated
+
Memory ID
{memory.id}
+
Memory revision
{memory.revision}
+
Last changed
{/each} @@ -384,25 +441,79 @@ font-size: var(--font-size-compact); } - .subject-facts { - grid-template-columns: repeat(5, minmax(0, 1fr)); - gap: var(--space-3); + .subject-overview { + display: grid; + grid-template-columns: repeat(4, minmax(0, 1fr)); + gap: var(--space-4); + padding: var(--space-4); + background: var(--bg-raised); + border-radius: var(--radius-soft); } - .subject-facts > div { - display: block; + .subject-overview > div { + display: grid; + align-content: start; + gap: var(--space-1); + min-width: 0; } - .subject-facts dt { - white-space: normal; + .subject-overview span, + .subject-overview p, + .subject-technical-details summary, + .subject-technical-details small, + .surface-details small { + color: var(--text-muted); + font-size: var(--font-size-compact); } - .subject-facts dd { - margin-top: var(--space-1); - color: var(--text); + .subject-overview strong { + color: var(--text-strong); overflow-wrap: anywhere; } + .subject-overview p { + margin: 0; + overflow-wrap: anywhere; + } + + .subject-technical-details { + padding-bottom: var(--space-3); + border-bottom: 1px solid var(--line); + } + + .subject-technical-details summary { + width: fit-content; + cursor: pointer; + font-weight: 700; + } + + .subject-technical-details dl, + .surface-details dl { + display: grid; + grid-template-columns: repeat(3, minmax(0, 1fr)); + gap: var(--space-4); + margin-top: var(--space-3); + } + + .subject-technical-details dl > div, + .surface-details dl > div { + display: block; + min-width: 0; + } + + .subject-technical-details dd, + .surface-details dd { + margin-top: var(--space-1); + overflow-wrap: anywhere; + } + + .subject-technical-details small, + .surface-details small { + display: block; + margin-top: var(--space-1); + line-height: 1.4; + } + .subject-state, .availability, .memory-pill, @@ -642,7 +753,7 @@ } @media (max-width: 900px) { - .subject-facts { + .subject-overview { grid-template-columns: repeat(2, minmax(0, 1fr)); } @@ -659,10 +770,16 @@ gap: var(--space-2); } - .subject-facts { + .subject-overview, + .subject-technical-details dl, + .surface-details dl { grid-template-columns: minmax(0, 1fr); } + .subject-overview { + padding: var(--space-3); + } + .memory-row { padding-inline: 0; } diff --git a/web/workspace/src/routes/w/[workspaceId]/memory/[subjectId]/[memoryId]/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/memory/[subjectId]/[memoryId]/+page.svelte index 07a1108fe..71c038627 100644 --- a/web/workspace/src/routes/w/[workspaceId]/memory/[subjectId]/[memoryId]/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/memory/[subjectId]/[memoryId]/+page.svelte @@ -63,6 +63,11 @@ return value.replaceAll('_', ' '); } + function titleLabel(value: string): string { + const normalized = label(value); + return normalized[0].toUpperCase() + normalized.slice(1); + } + function rangeLabel(range: [number, number] | null | undefined): string { return range ? `${range[0]}–${range[1]}` : 'None'; } @@ -98,8 +103,8 @@
- {memory.state} - {label(memory.kind)} + {titleLabel(memory.state)} + {titleLabel(memory.kind)} {#if memory.revision !== memory.current_revision}Historical revision{/if}

{memory.claim}

@@ -109,11 +114,11 @@
-
Revision
{memory.revision} of {memory.current_revision}
-
State
{memory.state}
-
Kind
{label(memory.kind)}
+
Memory revision
{memory.revision} of {memory.current_revision}
+
Status
{titleLabel(memory.state)}
+
Type
{titleLabel(memory.kind)}
Created
-
Updated
+
Last changed
@@ -140,7 +145,7 @@

Audit context

Candidate provenance

- {memory.source_candidates.length} candidate{memory.source_candidates.length === 1 ? '' : 's'} + {memory.source_candidates.length} shown
{#if memory.source_candidates.length === 0} @@ -226,7 +231,11 @@
-

Immutable record

Revision history

{#if data.revisions.data}{revisions.length}{data.revisions.data.has_more ? '+' : ''}{/if}
+
+

Immutable record

Revision history

+ {#if data.revisions.data}{revisions.length} shown{data.revisions.data.has_more ? ' · more available' : ''}{/if} +
+

These are versions of this Memory record, separate from the Subject’s internal update number.

{#if data.revisions.data} {#if revisions.length === 0}
@@ -238,7 +247,7 @@ {#each revisions as revision (revision.revision)}
  • -
    Revision {revision.revision}{revision.state}{label(revision.kind)}
    +
    Memory revision {revision.revision}{titleLabel(revision.state)}{titleLabel(revision.kind)}

    {revision.claim}

    {revision.change_reason} ·     @@ -386,6 +395,7 @@ } .memory-facts { + display: grid; grid-template-columns: repeat(5, minmax(0, 1fr)); gap: var(--space-3); } @@ -414,6 +424,7 @@ .why-useful, .staleness, .change-reason, + .section-intro, .empty-copy, .bounded-note { margin: 0; diff --git a/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts b/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts index ccd3f9c24..5166dc3b0 100644 --- a/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts +++ b/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts @@ -75,6 +75,9 @@ test("renders the subject index as the Memory product entry", () => { expect(screen.getByRole("link", { name: /Release coordinator/ })).not .toBeNull(); expect(screen.getByText("subject-1")).not.toBeNull(); + expect(screen.getByText("Not connected")).not.toBeNull(); + expect(screen.queryByText("Store revision", { exact: true })).toBeNull(); + expect(screen.getByText("1 shown · more available")).not.toBeNull(); expect(screen.getByRole("link", { name: "First page" }).getAttribute("href")) .toBe("/w/workspace-1/memory"); expect(screen.getByRole("link", { name: /Next page/ }).getAttribute("href")) @@ -240,7 +243,10 @@ test("renders a ready resident surface and committed Memory lifecycle states", a workspaceId: "workspace-1", subjectId: "subject-1", cursor: "memory-current", - subject: result(subject()), + subject: result({ + ...subject(), + current_worker: { display_name: "Release Worker" }, + }), surface: result({ subject_id: "subject-1", availability: "ready" as const, @@ -298,9 +304,15 @@ test("renders a ready resident surface and committed Memory lifecycle states", a .textContent, ) .toContain("Prefer explicit evidence."); - expect(screen.getByRole("article", { name: "Resident surface" })).not + expect(screen.getByRole("article", { name: "Resident context" })).not .toBeNull(); - expect(screen.getAllByText("active", { exact: true }).length).toBeGreaterThan( + expect(screen.getByText("Connected", { exact: true })).not.toBeNull(); + expect(screen.getByText("Release Worker", { exact: true })).not.toBeNull(); + expect(screen.getByText("3 on this page", { exact: true })).not.toBeNull(); + expect(screen.getByText("Subject store revision", { exact: true })).not + .toBeNull(); + expect(screen.getByText(/not a Memory count/)).not.toBeNull(); + expect(screen.getAllByText("Active", { exact: true }).length).toBeGreaterThan( 0, ); expect(screen.getByText("Resolved", { exact: true })).not.toBeNull(); @@ -577,6 +589,93 @@ test("a refreshed loader snapshot fences old saves even after returning to the s ).toBeNull(); }); +test("candidate-only Subjects show no committed Memories, not an inferred candidate count", () => { + // The store regression candidate_decisions_reject_conflicts_and_roll_back_partial_writes + // stages a candidate without committing a Memory. Its public read projection is + // the same empty committed list as a new Subject; candidates are not a page input. + render(SubjectPage, { + data: { + workspaceId: "workspace-1", + subjectId: "subject-1", + cursor: null, + subject: result({ ...subject(), store_revision: 0 }), + surface: result({ subject_id: "subject-1", availability: "ungenerated" }), + memories: result({ limit: 100, items: [], has_more: false }), + }, + } as never); + + expect(screen.getByText("None", { exact: true })).not.toBeNull(); + expect(screen.getByText("No committed Memories yet.")).not.toBeNull(); + expect(screen.getByText("Not connected", { exact: true })).not.toBeNull(); + expect(screen.getAllByText("Not generated", { exact: true })).toHaveLength(2); + expect(screen.queryByRole("article", { name: "Resident context" })) + .toBeNull(); + expect( + screen.queryByText( + /candidate count|all Memories lost|automatically updating/i, + ), + ).toBeNull(); +}); + +test.each(["ready", "ungenerated", "stale", "failed"] as const)( + "user behavior and diagnostic disclosures stay independent of %s generated Memory", + async (availability) => { + const record = { ...subject(), store_revision: 0 }; + const view = render(SubjectPage, { + data: { + ...subjectPageData(record), + surface: result({ + subject_id: record.id, + availability, + ...(availability === "ready" + ? { + snapshot: { + snapshot_id: "empty-snapshot", + body_md: "", + memory_refs: [], + built_from_store_revision: 0, + created_at: "2026-01-02T03:04:05Z", + }, + } + : {}), + }), + }, + } as never); + expect( + screen.getByRole("article", { name: "User-managed Subject behavior" }) + .textContent, + ) + .toContain(record.behavior_md); + expect(screen.getByText("Generated Memory context", { exact: true })).not + .toBeNull(); + expect(screen.getByText("No committed Memories yet.")).not.toBeNull(); + expect(screen.getByText("Not connected", { exact: true })).not.toBeNull(); + const technical = view.container.querySelector( + "details.subject-technical-details", + ) as HTMLDetailsElement; + expect(technical.open).toBe(false); + expect(technical.textContent).toContain("Subject store revision"); + if (availability === "ready") { + expect(screen.getByText("Resident context is current but empty.")).not + .toBeNull(); + expect(screen.getByText(/no generated Memory context to show/)).not + .toBeNull(); + expect( + view.container.querySelector("details.surface-details")?.textContent, + ) + .toContain("Generated from Subject revision"); + } + await fireEvent.click(screen.getByRole("button", { name: "Edit" })); + await fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect( + screen.getByRole("article", { name: "User-managed Subject behavior" }) + .textContent, + ) + .toContain(record.behavior_md); + expect(technical.open).toBe(false); + }, +); + test("distinguishes ready-empty, stale, failed, unavailable, and request error surfaces", async () => { const base = { workspaceId: "workspace-1", @@ -600,8 +699,25 @@ test("distinguishes ready-empty, stale, failed, unavailable, and request error s }), }, } as never); - expect(screen.getByText("Resident surface is ready and empty.")).not + expect(screen.getByText("Resident context is current but empty.")).not .toBeNull(); + expect(screen.getByText(/current empty surface exists/)).not.toBeNull(); + expect(screen.getByText("No committed Memories yet.")).not.toBeNull(); + + await view.rerender( + { + data: { + ...base, + surface: result({ + subject_id: "subject-1", + availability: "ungenerated" as const, + }), + }, + } as never, + ); + expect(screen.getByText("Resident context has not been generated.")).not + .toBeNull(); + expect(screen.getAllByText("Not generated", { exact: true })).toHaveLength(2); await view.rerender( { @@ -614,7 +730,8 @@ test("distinguishes ready-empty, stale, failed, unavailable, and request error s }, } as never, ); - expect(screen.getByText("Resident surface is stale.")).not.toBeNull(); + expect(screen.getByText("Resident context needs to be refreshed.")).not + .toBeNull(); await view.rerender( { @@ -630,7 +747,7 @@ test("distinguishes ready-empty, stale, failed, unavailable, and request error s expect(screen.getByRole("alert").textContent).toContain("generation failed"); await view.rerender({ data: { ...base, surface: result(null) } } as never); - expect(screen.getByText("Resident surface data is unavailable.")).not + expect(screen.getByText("Resident context status is unavailable.")).not .toBeNull(); await view.rerender( @@ -639,7 +756,7 @@ test("distinguishes ready-empty, stale, failed, unavailable, and request error s } as never, ); expect(screen.getByRole("alert").textContent).toContain( - "Resident surface unavailable", + "Resident context status unavailable", ); }); diff --git a/web/workspace/src/routes/w/[workspaceId]/workers/[workerRef]/console/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/workers/[workerRef]/console/+page.svelte index 683cd00cb..c3b5713af 100644 --- a/web/workspace/src/routes/w/[workspaceId]/workers/[workerRef]/console/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/workers/[workerRef]/console/+page.svelte @@ -1663,45 +1663,6 @@
    -
    - -
    -
    - - -
    - -
    -
    - {#if rewindTargets.length > 0}

    Rewind targets

    @@ -2106,6 +2067,28 @@
    • + { + workerDetailsOpen = !workerDetailsOpen; + if (workerDetailsOpen) taskPaneOpen = false; + }} + > + {#snippet controls()} + + {/snippet} + diff --git a/web/workspace/src/routes/w/[workspaceId]/workers/[workerRef]/console/console-display.browser.spec.ts b/web/workspace/src/routes/w/[workspaceId]/workers/[workerRef]/console/console-display.browser.spec.ts index fc951de67..125f09ff5 100644 --- a/web/workspace/src/routes/w/[workspaceId]/workers/[workerRef]/console/console-display.browser.spec.ts +++ b/web/workspace/src/routes/w/[workspaceId]/workers/[workerRef]/console/console-display.browser.spec.ts @@ -261,13 +261,28 @@ test.each(["live_protocol", "retained_snapshot"] as const)("overlapping history await screen.findByText("Start of conversation"); expect(view.container.querySelectorAll(".activity-summary")).toHaveLength(1); expect(screen.getAllByText("searched 1 time・ran 1 command")).toHaveLength(1); - await fireEvent.click(screen.getByRole("button", { name: "Normal" })); + const footer = screen.getByLabelText("Worker model and context"); + const overview = screen.getByRole("switch", { name: "Overview" }); + expect(screen.queryByRole("button", { name: "Normal" })).toBeNull(); + const details = screen.getByRole("button", { name: "Details" }); + expect(view.container.querySelector(".console-header")).toBeNull(); + expect(footer.contains(overview)).toBe(true); + expect(overview.nextElementSibling).toBe(details); + expect(overview.closest("form")).toBeNull(); + expect(view.container.querySelector(".console-composer")?.nextElementSibling).toBe(footer); + expect(overview.getAttribute("aria-checked")).toBe("true"); + await fireEvent.click(overview); + expect(overview.getAttribute("aria-checked")).toBe("false"); + expect(overview.textContent?.trim()).toBe("Overview"); const transcript = screen.getByRole("article", { name: "main transcript" }); const ids = [...transcript.querySelectorAll("[data-console-line-id]")].map((line) => line.getAttribute("data-console-line-id")); expect(ids).toHaveLength(3); expect(new Set(ids).size).toBe(ids.length); - await fireEvent.click(screen.getByRole("button", { name: "Overview" })); + await fireEvent.click(overview); + expect(overview.getAttribute("aria-checked")).toBe("true"); + expect(overview.textContent?.trim()).toBe("Overview"); expect(view.container.querySelectorAll(".activity-summary")).toHaveLength(1); + expect(multiplexer.sendWorkerMethod).not.toHaveBeenCalled(); }); test.each([ @@ -473,7 +488,7 @@ test("mini task summary opens and closes details without a header Tasks button", latestListener().onFrame(subscribedFrame(session)); const summary = await screen.findByRole("button", { name: /1 task — pending/ }); expect(screen.queryByRole("complementary", { name: "Worker tasks" })).toBeNull(); - expect(view.container.querySelector(".console-header")?.textContent).not.toContain("Tasks"); + expect(view.container.querySelector(".console-header")).toBeNull(); await fireEvent.click(screen.getByRole("button", { name: "Details" })); const details = await screen.findByRole("complementary", { name: "Worker detail" }); expect(details.textContent).not.toContain("Capabilities"); diff --git a/web/workspace/test/config-source/api.test.ts b/web/workspace/test/config-source/api.test.ts index 6c5771099..d0269c215 100644 --- a/web/workspace/test/config-source/api.test.ts +++ b/web/workspace/test/config-source/api.test.ts @@ -71,18 +71,105 @@ Deno.test("config source API commits directly through the workspace scope", asyn entrypoints: [], }, fetcher); - assertEquals(calls.map((call) => call.url), [ + assertEquals(calls.map((call) => call.url), [ "/api/w/w%2Fone/config/source-tree", "/api/w/w%2Fone/config/source-tree/revisions/7", "/api/w/w%2Fone/config/source-tree/entries/profiles%2Fmain.dcdl", "/api/w/w%2Fone/config/source-tree/commit", ]); - assertEquals(calls[3].init?.method, "POST"); + assertEquals(calls[3].init?.method, "POST"); assert( String(calls[3].init?.body).includes('"base_digest":"sha256:base"'), ); }); +function withSchema(source: unknown, contributionSource: unknown = "builtin") { + return { + ...tree, + contract: { + ...tree.contract, + schema_bundle: { + ...tree.contract.schema_bundle, + source, + contributions: [{ + provider_id: "builtin", + namespace: "workspace", + version: "1", + source: contributionSource, + source_digest: "sha256:contribution", + }], + }, + }, + }; +} + +for (const field of ["bundle", "contribution"] as const) { + Deno.test(`config source API accepts ${field} schema bodies larger than metadata`, async () => { + const source = "schema body\n".repeat(6000); + const body = field === "bundle" + ? withSchema(source) + : withSchema("builtin", source); + const fetcher = (() => Promise.resolve(response(body))) as typeof fetch; + assertEquals(await fetchConfigTree("w", fetcher), body); + assertEquals( + await commitConfigTree("w", { + base_revision: 7, + base_digest: snapshot.digest, + changes: [], + entrypoints: ["main.dcdl"], + }, fetcher), + body, + ); + }); + + Deno.test(`config source API bounds ${field} schema bodies in UTF-8 bytes and checks types`, () => { + const body = (source: unknown) => + field === "bundle" ? withSchema(source) : withSchema("builtin", source); + const limit = 8 * 1024 * 1024; + const atLimit = "é".repeat(limit / 2); + assertEquals( + parseWorkspaceConfigTreeResponse(body(atLimit)), + body(atLimit), + ); + for (const invalid of [atLimit + "x", null, 42, {}]) { + assertThrows( + () => parseWorkspaceConfigTreeResponse(body(invalid)), + ConfigSourceApiError, + ); + } + }); +} + +Deno.test("config source API retains metadata and total response limits", async () => { + const invalid = withSchema("builtin"); + invalid.contract.schema_bundle.contributions[0].provider_id = "x".repeat( + 4097, + ); + assertThrows( + () => parseWorkspaceConfigTreeResponse(invalid), + ConfigSourceApiError, + ); + for ( + const headers of [ + new Headers(), + new Headers({ "content-length": String(8 * 1024 * 1024 + 1) }), + ] + ) { + const fetcher = (() => + Promise.resolve( + new Response( + JSON.stringify(withSchema("x".repeat(8 * 1024 * 1024))), + { headers }, + ), + )) as typeof fetch; + await assertRejects( + () => fetchConfigTree("w", fetcher), + ConfigSourceApiError, + "too large", + ); + } +}); + Deno.test("config source API rejects unknown response fields", async () => { const fetcher = (() => diff --git a/web/workspace/tests/memory-api.test.ts b/web/workspace/tests/memory-api.test.ts index 3e2142035..9d5b460d8 100644 --- a/web/workspace/tests/memory-api.test.ts +++ b/web/workspace/tests/memory-api.test.ts @@ -62,9 +62,7 @@ async function assertSubjectCreateRejects( assertEquals(error.status, expectedStatus); if (!error.message.includes(expectedMessage)) { throw new Error( - `expected ${JSON.stringify(error.message)} to include ${ - JSON.stringify(expectedMessage) - }`, + `expected ${JSON.stringify(error.message)} to include ${JSON.stringify(expectedMessage)}`, ); } return; @@ -85,6 +83,30 @@ function subject(id = "subject-1") { }; } +function currentWorker() { + return { + runtime_id: "runtime-1", + worker_id: "worker-1", + host_id: "host-1", + display_name: "Release Worker", + label: "Release Worker", + profile: "builtin:companion", + singleton_key: "subjektiv:subject-1", + tags: [], + workspace: { + visibility: "workspace", + identity: "workspace-1", + workspace_id: "workspace-1", + }, + state: "idle", + last_seen_at: "2026-09-02T00:00:00Z", + pinned: false, + retention_state: "retained", + implementation: { kind: "embedded", display_hint: "Embedded" }, + diagnostics: [], + }; +} + function queryItem(id = "memory-1") { return { id, @@ -190,14 +212,35 @@ Deno.test("Subject parsers enforce identity, exact enums, safe revisions, and bo () => parseSubjektivSubjectListResponse({ limit: 100, - items: Array.from({ length: 101 }, (_, index) => - subject(`subject-${index}`)), + items: Array.from({ length: 101 }, (_, index) => subject(`subject-${index}`)), has_more: false, }), "bounded array", ); }); +Deno.test("Subject parser preserves live Worker connection state", () => { + assertEquals( + parseSubjektivSubjectResponse({ + ...subject(), + current_worker: currentWorker(), + }), + { ...subject(), current_worker: currentWorker() }, + ); + assertEquals( + parseSubjektivSubjectResponse({ ...subject(), current_worker: null }), + { ...subject(), current_worker: null }, + ); + assertThrows( + () => + parseSubjektivSubjectResponse({ + ...subject(), + current_worker: { display_name: "Incomplete Worker" }, + }), + "current worker.runtime_id", + ); +}); + Deno.test("Subject list parser preserves the exact bounded response", () => { const response = { limit: 100, items: [subject()], has_more: false }; assertEquals(parseSubjektivSubjectListResponse(response), response); @@ -401,8 +444,7 @@ Deno.test("Subject create client distinguishes rejected and unknown outcomes wit throw new TypeError("connection reset"); }) as typeof fetch; await assertSubjectCreateRejects( - () => - createSubjektivSubject(unknownFetch, "workspace-1", { role: "Reviewer" }), + () => createSubjektivSubject(unknownFetch, "workspace-1", { role: "Reviewer" }), "unknown_outcome", "may have been created", ); @@ -506,8 +548,7 @@ Deno.test("Current Memory list parser rejects unknown variants, unsafe integers, assertThrows( () => parseSubjektivMemoryQueryResponse({ - items: Array.from({ length: 101 }, (_, index) => - queryItem(`memory-${index}`)), + items: Array.from({ length: 101 }, (_, index) => queryItem(`memory-${index}`)), has_more: false, }), "bounded array",