diff --git a/crates/client/src/backend_api.rs b/crates/client/src/backend_api.rs index be4c7519..7e96d189 100644 --- a/crates/client/src/backend_api.rs +++ b/crates/client/src/backend_api.rs @@ -192,6 +192,32 @@ impl BackendApiClient { format!("Bearer {}", self.access_token.0) } + pub async fn require_success( + &self, + response: reqwest::Response, + ) -> Result { + let status = response.status(); + match status { + StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN => { + self.check_status(status)?; + } + status if !status.is_success() => { + let detail = response + .bytes() + .await + .ok() + .and_then(|body| backend_error_detail(&body)); + return Err(BackendApiClientError::BackendResponse { + origin: self.origin.clone(), + status: status.as_u16(), + detail, + }); + } + _ => {} + } + Ok(response) + } + pub fn check_status(&self, status: StatusCode) -> Result<(), BackendApiClientError> { match status { StatusCode::UNAUTHORIZED => Err(BackendApiClientError::Unauthorized { @@ -235,6 +261,18 @@ fn redirect_policy(origin: BackendOrigin) -> redirect::Policy { }) } +#[derive(Deserialize)] +struct BackendErrorBody { + message: String, +} + +fn backend_error_detail(body: &[u8]) -> Option { + serde_json::from_slice::(body) + .ok() + .map(|body| body.message) + .filter(|message| !message.trim().is_empty()) +} + #[derive(Debug)] pub enum BackendApiClientError { InvalidBackendOrigin(String), @@ -266,6 +304,11 @@ pub enum BackendApiClientError { origin: BackendOrigin, status: u16, }, + BackendResponse { + origin: BackendOrigin, + status: u16, + detail: Option, + }, Io { path: PathBuf, source: std::io::Error, @@ -312,6 +355,17 @@ impl fmt::Display for BackendApiClientError { Self::BackendStatus { origin, status } => { write!(f, "Backend {origin} returned HTTP {status}") } + Self::BackendResponse { + origin, + status, + detail, + } => { + write!(f, "Backend {origin} returned HTTP {status}")?; + if let Some(detail) = detail { + write!(f, ": {detail}")?; + } + Ok(()) + } Self::Io { path, source } => { write!(f, "failed to access {}: {source}", path.display()) } @@ -584,6 +638,23 @@ mod tests { ); } + #[test] + fn backend_error_detail_preserves_public_server_message() { + let detail = backend_error_detail( + br#"{"error":"Bad Request","message":"working_directory_runtime_mismatch: Working directory is owned by a different Runtime","diagnostics":[{"code":"working_directory_runtime_mismatch"}]}"#, + ); + let error = BackendApiClientError::BackendResponse { + origin: BackendOrigin::parse("http://127.0.0.1:8787").unwrap(), + status: 400, + detail, + }; + + assert_eq!( + error.to_string(), + "Backend http://127.0.0.1:8787 returned HTTP 400: working_directory_runtime_mismatch: Working directory is owned by a different Runtime" + ); + } + #[test] fn backend_origin_rejects_unsafe_authority_changes() { for invalid in [ diff --git a/crates/client/src/backend_runtime.rs b/crates/client/src/backend_runtime.rs index 0d84072f..a3cf0609 100644 --- a/crates/client/src/backend_runtime.rs +++ b/crates/client/src/backend_runtime.rs @@ -387,7 +387,7 @@ pub async fn restore_backend_worker( .json(&serde_json::json!({})) .send() .await?; - api.check_status(response.status())?; + let response = api.require_success(response).await?; Ok(response.json::().await?) } diff --git a/crates/config-source-wasm/src/lib.rs b/crates/config-source-wasm/src/lib.rs index 6df1e834..5752b64a 100644 --- a/crates/config-source-wasm/src/lib.rs +++ b/crates/config-source-wasm/src/lib.rs @@ -101,20 +101,24 @@ pub fn complete_current( let utf8_byte_offset = utf16_to_utf8_offset(&source, utf16_offset)?; let result = session_environment(snapshot.clone()) .complete_config(&entrypoint, &source, utf8_byte_offset, explicit) - .map_err(|error| JsValue::from_str(&format!("{error:?}")))? - .map(|result| WasmCompletionResult { - from: result.from, - items: result - .items - .into_iter() - .map(|item| WasmCompletionItem { - label: item.label, - kind: format!("{:?}", item.kind).to_lowercase(), - detail: item.detail, - priority: item.priority, - }) - .collect(), - }); + .map_err(|error| JsValue::from_str(&format!("{error:?}")))?; + let result = result + .map(|result| { + Ok::(WasmCompletionResult { + from: utf8_to_utf16_offset(&source, result.from)?, + items: result + .items + .into_iter() + .map(|item| WasmCompletionItem { + label: item.label, + kind: format!("{:?}", item.kind).to_lowercase(), + detail: item.detail, + priority: item.priority, + }) + .collect(), + }) + }) + .transpose()?; encode(result) }) } @@ -177,6 +181,16 @@ fn utf16_to_utf8_offset(source: &str, utf16_offset: usize) -> Result Result { + if utf8_offset > source.len() { + return Err(JsValue::from_str("UTF-8 offset is outside the source")); + } + if !source.is_char_boundary(utf8_offset) { + return Err(JsValue::from_str("UTF-8 offset splits a character")); + } + Ok(source[..utf8_offset].encode_utf16().count()) +} + fn decode(value: JsValue) -> Result { from_value(value).map_err(|error| JsValue::from_str(&error.to_string())) } diff --git a/crates/config-source/src/lib.rs b/crates/config-source/src/lib.rs index 0adc87e4..a5df85dc 100644 --- a/crates/config-source/src/lib.rs +++ b/crates/config-source/src/lib.rs @@ -1203,6 +1203,9 @@ impl SnapshotEnvironment { { let mut member_source = format!("{WORKSPACE_CONFIG_SCHEMA_GLOBAL}."); member_source.push_str(&context.schema_path.join(".")); + if !context.schema_path.is_empty() && context.from == utf8_byte_offset { + member_source.push('.'); + } let mut completion = LanguageService::new(self).complete( entrypoint.as_str(), &member_source, @@ -1961,6 +1964,31 @@ mod tests { .iter() .any(|item| item.label == "default_profile") ); + + let blank_nested_source = "{ profile = { } } as WorkspaceConfigSchema"; + let blank_nested_cursor = blank_nested_source.find("{ }").unwrap() + 2; + let blank_nested = environment + .complete_config( + &path("main.dcdl"), + blank_nested_source, + blank_nested_cursor, + true, + ) + .unwrap() + .unwrap(); + assert_eq!(blank_nested.from, blank_nested_cursor); + assert!( + blank_nested + .items + .iter() + .any(|item| item.label == "default_profile") + ); + assert!( + !blank_nested + .items + .iter() + .any(|item| item.label == "profile") + ); } #[test] diff --git a/crates/tui/src/backend_worker_picker.rs b/crates/tui/src/backend_worker_picker.rs index 54c7b6e4..a59891ad 100644 --- a/crates/tui/src/backend_worker_picker.rs +++ b/crates/tui/src/backend_worker_picker.rs @@ -7,15 +7,15 @@ use client::{ list_backend_workers, restore_backend_worker, }; use crossterm::event::{self, Event as TermEvent, KeyCode, KeyEventKind, KeyModifiers}; -use ratatui::backend::CrosstermBackend; +use ratatui::Frame; use ratatui::layout::{Constraint, Layout}; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; -use ratatui::{Frame, Terminal, TerminalOptions, Viewport}; use crate::backend_workspace_picker::select_backend_workspace; use crate::console; +use crate::inline_terminal::with_inline_terminal; const MAX_ROWS: usize = 10; const VIEWPORT_LINES: u16 = MAX_ROWS as u16 + 4; @@ -127,31 +127,32 @@ fn pick_worker( workers.truncate(MAX_ROWS); let mut state = BackendWorkerPickerState::new(target, workers); - let mut terminal = make_inline_terminal()?; - loop { - terminal.draw(|frame| draw(frame, &state))?; - match poll_event()? { - None => continue, - Some(Action::Up) => state.previous(), - Some(Action::Down) => state.next(), - Some(Action::Submit) => { - close_viewport(&mut terminal)?; - return Ok(WorkerPickerResult::Selected( - state.selected_worker().clone(), - )); + with_inline_terminal( + VIEWPORT_LINES, + |terminal| -> Result<_, Box> { + loop { + terminal.draw(|frame| draw(frame, &state))?; + match poll_event()? { + None => continue, + Some(Action::Up) => state.previous(), + Some(Action::Down) => state.next(), + Some(Action::Submit) => { + return Ok(WorkerPickerResult::Selected( + state.selected_worker().clone(), + )); + } + Some(Action::SwitchWorkspace) => { + return Ok(WorkerPickerResult::SwitchWorkspace); + } + Some(Action::Cancel) => { + return Err(Box::new(io::Error::other( + "Backend worker picker cancelled", + ))); + } + } } - Some(Action::SwitchWorkspace) => { - close_viewport(&mut terminal)?; - return Ok(WorkerPickerResult::SwitchWorkspace); - } - Some(Action::Cancel) => { - close_viewport(&mut terminal)?; - return Err(Box::new(io::Error::other( - "Backend worker picker cancelled", - ))); - } - } - } + }, + ) } struct BackendWorkerPickerState { @@ -184,27 +185,6 @@ impl BackendWorkerPickerState { } } -fn make_inline_terminal() -> io::Result>> { - let backend = CrosstermBackend::new(io::stdout()); - Terminal::with_options( - backend, - TerminalOptions { - viewport: Viewport::Inline(VIEWPORT_LINES), - }, - ) -} - -fn close_viewport(terminal: &mut Terminal>) -> io::Result<()> { - let area = terminal.get_frame().area(); - let last_row = area.bottom().saturating_sub(1); - terminal.set_cursor_position((0, last_row))?; - use std::io::Write; - let mut out = io::stdout(); - out.write_all(b"\r\n")?; - out.flush()?; - Ok(()) -} - enum Action { Up, Down, diff --git a/crates/tui/src/inline_terminal.rs b/crates/tui/src/inline_terminal.rs new file mode 100644 index 00000000..86093d5d --- /dev/null +++ b/crates/tui/src/inline_terminal.rs @@ -0,0 +1,109 @@ +use std::io::{self, Stdout, Write}; + +use ratatui::Terminal; +use ratatui::backend::CrosstermBackend; +use ratatui::{TerminalOptions, Viewport}; + +pub(crate) type InlineTerminal = Terminal>; + +struct InlineTerminalGuard { + terminal: InlineTerminal, + closed: bool, +} + +impl InlineTerminalGuard { + fn open(height: u16) -> io::Result { + let terminal = Terminal::with_options( + CrosstermBackend::new(io::stdout()), + TerminalOptions { + viewport: Viewport::Inline(height), + }, + )?; + Ok(Self { + terminal, + closed: false, + }) + } + + fn close(&mut self) -> io::Result<()> { + if self.closed { + return Ok(()); + } + self.closed = true; + + let area = self.terminal.get_frame().area(); + let last_row = area.bottom().saturating_sub(1); + let cursor_result = self.terminal.set_cursor_position((0, last_row)); + let output_result = write_viewport_terminator(&mut io::stdout()); + cursor_result?; + output_result + } +} + +impl Drop for InlineTerminalGuard { + fn drop(&mut self) { + let _ = self.close(); + } +} + +pub(crate) fn with_inline_terminal( + height: u16, + run: impl FnOnce(&mut InlineTerminal) -> Result, +) -> Result +where + E: From, +{ + let mut guard = InlineTerminalGuard::open(height).map_err(E::from)?; + let result = run(&mut guard.terminal); + let close_result = guard.close(); + match result { + Ok(value) => { + close_result.map_err(E::from)?; + Ok(value) + } + Err(error) => Err(error), + } +} + +fn write_viewport_terminator(output: &mut impl Write) -> io::Result<()> { + output.write_all(b"\r\n")?; + output.flush() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn viewport_terminator_moves_following_output_to_a_fresh_line() { + let mut output = Vec::new(); + + write_viewport_terminator(&mut output).unwrap(); + + assert_eq!(output, b"\r\n"); + } + + #[test] + fn inline_viewport_construction_is_owned_by_this_module() { + fn assert_shared_owner(path: &std::path::Path) { + for entry in std::fs::read_dir(path).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + assert_shared_owner(&path); + } else if path.extension().and_then(|value| value.to_str()) == Some("rs") + && path.file_name().and_then(|value| value.to_str()) + != Some("inline_terminal.rs") + { + let source = std::fs::read_to_string(&path).unwrap(); + assert!( + !source.contains("Viewport::Inline"), + "{} constructs an inline viewport outside its shared owner", + path.display() + ); + } + } + } + + assert_shared_owner(&std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src")); + } +} diff --git a/crates/tui/src/keys.rs b/crates/tui/src/keys.rs index 0b2c69f7..e9acc181 100644 --- a/crates/tui/src/keys.rs +++ b/crates/tui/src/keys.rs @@ -1,17 +1,17 @@ -use std::io::{self, Stdout, Write}; use std::process::ExitCode; use std::time::Duration; use crossterm::event::{self, Event, KeyCode, KeyEventKind, KeyModifiers}; use crossterm::terminal::{disable_raw_mode, enable_raw_mode}; -use ratatui::backend::CrosstermBackend; +use ratatui::Frame; use ratatui::layout::{Constraint, Layout}; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; -use ratatui::{Frame, Terminal, TerminalOptions, Viewport}; use secrets::{SecretStore, SecretValue}; +use crate::inline_terminal::{InlineTerminal, with_inline_terminal}; + #[derive(Debug, Clone, PartialEq, Eq)] enum Mode { Normal, @@ -235,7 +235,6 @@ pub async fn launch() -> ExitCode { } type UiResult = Result>; -type InlineTerminal = Terminal>; const MAX_ROWS: usize = 10; const VIEWPORT_LINES: u16 = MAX_ROWS as u16 + 5; @@ -270,37 +269,9 @@ impl Drop for RawModeGuard { fn run(store: SecretStore) -> UiResult<()> { enable_raw_mode()?; let guard = RawModeGuard::new(); - let mut terminal = make_inline_terminal()?; - let result = run_loop(&mut terminal, store); - let close_result = close_viewport(&mut terminal); - drop(terminal); + let result = with_inline_terminal(VIEWPORT_LINES, |terminal| run_loop(terminal, store)); guard.restore(); - result?; - close_result?; - Ok(()) -} - -fn make_inline_terminal() -> io::Result { - let backend = CrosstermBackend::new(io::stdout()); - Terminal::with_options( - backend, - TerminalOptions { - viewport: Viewport::Inline(VIEWPORT_LINES), - }, - ) -} - -/// Park the cursor at the very bottom of the inline viewport and emit one -/// newline before dropping the terminal. This matches the resume picker and -/// keeps the shell prompt (or a later inline viewport) from drawing over rows. -fn close_viewport(terminal: &mut InlineTerminal) -> io::Result<()> { - let area = terminal.get_frame().area(); - let last_row = area.bottom().saturating_sub(1); - terminal.set_cursor_position((0, last_row))?; - let mut out = io::stdout(); - out.write_all(b"\r\n")?; - out.flush()?; - Ok(()) + result } fn run_loop(terminal: &mut InlineTerminal, store: SecretStore) -> UiResult<()> { diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index 699da9a7..2cd9f031 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -10,6 +10,7 @@ mod composer_keys; mod console; #[cfg(feature = "e2e-test")] mod e2e_observer; +mod inline_terminal; mod input; pub mod keys; mod markdown; diff --git a/crates/tui/src/standalone_picker.rs b/crates/tui/src/standalone_picker.rs index 35ccbb5f..9d3dfcf3 100644 --- a/crates/tui/src/standalone_picker.rs +++ b/crates/tui/src/standalone_picker.rs @@ -3,15 +3,14 @@ use std::time::Duration; use client::{StandaloneWorkerListIntent, StandaloneWorkerResumeIntent, Target}; use crossterm::event::{self, Event as TermEvent, KeyCode, KeyEventKind, KeyModifiers}; -use ratatui::Terminal; -use ratatui::backend::CrosstermBackend; use ratatui::layout::{Constraint, Layout}; use ratatui::prelude::{Color, Line, Modifier, Span, Style}; use ratatui::widgets::Paragraph; -use ratatui::{TerminalOptions, Viewport}; use standalone::{StandaloneListScope, StandaloneWorkerRecord, StandaloneWorkerStore}; use thiserror::Error; +use crate::inline_terminal::with_inline_terminal; + const LIMIT: usize = 100; pub(crate) fn pick( @@ -57,41 +56,36 @@ fn run_picker( records: Vec, ) -> Result, StandalonePickerError> { let height = u16::try_from(records.len().saturating_add(3).min(20)).unwrap_or(20); - let mut terminal = Terminal::with_options( - CrosstermBackend::new(io::stdout()), - TerminalOptions { - viewport: Viewport::Inline(height), - }, - ) - .map_err(StandalonePickerError::Io)?; - let mut selected = 0usize; - loop { - terminal - .draw(|frame| draw(frame, &records, selected)) - .map_err(StandalonePickerError::Io)?; - if !event::poll(Duration::from_millis(100)).map_err(StandalonePickerError::Io)? { - continue; - } - let TermEvent::Key(key) = event::read().map_err(StandalonePickerError::Io)? else { - continue; - }; - if key.kind == KeyEventKind::Release { - continue; - } - let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); - match key.code { - KeyCode::Up | KeyCode::Char('k') if !ctrl => { - selected = selected.saturating_sub(1); + with_inline_terminal(height, |terminal| { + let mut selected = 0usize; + loop { + terminal + .draw(|frame| draw(frame, &records, selected)) + .map_err(StandalonePickerError::Io)?; + if !event::poll(Duration::from_millis(100)).map_err(StandalonePickerError::Io)? { + continue; } - KeyCode::Down | KeyCode::Char('j') if !ctrl => { - selected = (selected + 1).min(records.len() - 1); + let TermEvent::Key(key) = event::read().map_err(StandalonePickerError::Io)? else { + continue; + }; + if key.kind == KeyEventKind::Release { + continue; + } + let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); + match key.code { + KeyCode::Up | KeyCode::Char('k') if !ctrl => { + selected = selected.saturating_sub(1); + } + KeyCode::Down | KeyCode::Char('j') if !ctrl => { + selected = (selected + 1).min(records.len() - 1); + } + KeyCode::Enter => return Ok(Some(records[selected].clone())), + KeyCode::Esc => return Ok(None), + KeyCode::Char('c') if ctrl => return Ok(None), + _ => {} } - KeyCode::Enter => return Ok(Some(records[selected].clone())), - KeyCode::Esc => return Ok(None), - KeyCode::Char('c') if ctrl => return Ok(None), - _ => {} } - } + }) } fn draw(frame: &mut ratatui::Frame<'_>, records: &[StandaloneWorkerRecord], selected: usize) { @@ -145,11 +139,11 @@ pub(crate) enum StandalonePickerError { #[error("standalone Worker state is unavailable: {0}")] StateStore(#[source] standalone::StandaloneStoreError), #[error( - "no standalone Workers found for this cwd; use `yoi --local --resume --all` to include all cwd identities" + "no standalone Workers found for this cwd; use `yoi --local resume --all` to include all cwd identities" )] NoWorkers { include_all: bool }, #[error("standalone Worker picker I/O failed: {0}")] - Io(#[source] io::Error), + Io(#[from] io::Error), } #[cfg(test)] diff --git a/crates/tui/src/standalone_spawn.rs b/crates/tui/src/standalone_spawn.rs index 1880803d..b6b78fe1 100644 --- a/crates/tui/src/standalone_spawn.rs +++ b/crates/tui/src/standalone_spawn.rs @@ -1,17 +1,17 @@ -use std::io::{self, Stdout}; +use std::io; use std::path::Path; use std::time::Duration; use crossterm::event::{self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; use manifest::ProfileDiscovery; -use ratatui::Terminal; -use ratatui::backend::CrosstermBackend; use ratatui::layout::{Constraint, Direction, Layout}; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::Paragraph; use thiserror::Error; +use crate::inline_terminal::{InlineTerminal, with_inline_terminal}; + const VIEWPORT_HEIGHT: u16 = 6; const FALLBACK_WORKER_NAME: &str = "worker"; @@ -182,15 +182,16 @@ pub(crate) fn select( return Err(StandaloneSpawnError::NoProfiles); } - let terminal = open_inline_terminal()?; - run_picker( - terminal, - SpawnForm::new(worker_name, default_worker_name, choices), - ) + with_inline_terminal(VIEWPORT_HEIGHT, |terminal| { + run_picker( + terminal, + SpawnForm::new(worker_name, default_worker_name, choices), + ) + }) } fn run_picker( - mut terminal: Terminal>, + terminal: &mut InlineTerminal, mut form: SpawnForm, ) -> Result, StandaloneSpawnError> { loop { @@ -222,13 +223,6 @@ fn run_picker( } } -fn open_inline_terminal() -> io::Result>> { - let options = ratatui::TerminalOptions { - viewport: ratatui::Viewport::Inline(VIEWPORT_HEIGHT), - }; - Terminal::with_options(CrosstermBackend::new(io::stdout()), options) -} - fn profile_choices(registry: &manifest::ProfileRegistry) -> Vec { registry .entries() diff --git a/crates/worker-runtime/src/worker_source.rs b/crates/worker-runtime/src/worker_source.rs index ed485ab0..1905e233 100644 --- a/crates/worker-runtime/src/worker_source.rs +++ b/crates/worker-runtime/src/worker_source.rs @@ -134,6 +134,8 @@ pub trait EmbeddedWorkerMutationDispatcher: Send + Sync { enum RuntimeWorkerMutationTransport { Remote { base_url: String, + request_source_signer: RuntimeRequestSourceSigner, + request_source_audience: String, }, Embedded { dispatcher: Arc, @@ -157,10 +159,12 @@ impl RuntimeWorkerMutationForwarder { ) -> Self { Self { authority: RuntimeWorkerMutationSourceAuthority::remote(identity), - scope, + scope: scope.clone(), source_worker_id: source_worker_id.into(), transport: RuntimeWorkerMutationTransport::Remote { base_url: base_url.into().trim_end_matches('/').to_string(), + request_source_signer: RuntimeRequestSourceSigner::from_identity(identity), + request_source_audience: scope.server_id, }, } } @@ -197,11 +201,18 @@ impl RuntimeWorkerMutationForwarder { )?; match (&self.transport, proof) { ( - RuntimeWorkerMutationTransport::Remote { base_url }, + RuntimeWorkerMutationTransport::Remote { + base_url, + request_source_signer, + request_source_audience, + }, RuntimeOwnedWorkerMutationProof::Remote(token), ) => execute_remote_worker_remove_http(RemoteWorkerRemoveHttpRequest { base_url: base_url.clone(), workspace_id: self.scope.workspace_id.clone(), + source_worker_id: self.source_worker_id.clone(), + request_source_signer: request_source_signer.clone(), + request_source_audience: request_source_audience.clone(), token, target_runtime_id: target_runtime_id.to_string(), target_worker_id: target_worker_id.to_string(), @@ -224,6 +235,9 @@ impl RuntimeWorkerMutationForwarder { struct RemoteWorkerRemoveHttpRequest { base_url: String, workspace_id: String, + source_worker_id: String, + request_source_signer: RuntimeRequestSourceSigner, + request_source_audience: String, token: String, target_runtime_id: String, target_worker_id: String, @@ -256,23 +270,35 @@ fn execute_remote_worker_remove_http( fn execute_remote_worker_remove_http_blocking( request: RemoteWorkerRemoveHttpRequest, ) -> Result { - let url = format!( - "{}/api/w/{}/workers/remove", - request.base_url, request.workspace_id - ); - let body = serde_json::json!({ + let path = format!("/api/w/{}/workers/remove", request.workspace_id); + let url = format!("{}{}", request.base_url, path); + let body = serde_json::to_string(&serde_json::json!({ "target_runtime_id": request.target_runtime_id, "target_worker_id": request.target_worker_id, "reason": request.reason, - }); + })) + .map_err(|error| RuntimeWorkerMutationForwardError::Transport(error.to_string()))?; + let request_source_proof = request.request_source_signer.issue( + &request.request_source_audience, + &request.workspace_id, + Some(&request.source_worker_id), + WORKSPACE_REQUEST_PERMISSION, + "POST", + &path, + body.as_bytes(), + i64::try_from(unix_now_seconds()).unwrap_or(i64::MAX), + 30, + )?; let client = reqwest::blocking::Client::new(); let response = client .post(url) + .header(RUNTIME_REQUEST_SOURCE_PROOF_HEADER, request_source_proof) .header( crate::auth::WORKER_MUTATION_SOURCE_PROOF_HEADER, request.token, ) - .json(&body) + .header(reqwest::header::CONTENT_TYPE, "application/json") + .body(body) .send() .map_err(|error| RuntimeWorkerMutationForwardError::Transport(error.to_string()))?; let status = response.status().as_u16(); @@ -697,7 +723,8 @@ mod tests { use super::*; use crate::auth::{ WorkerMutationSourceExpectation, decode_runtime_request_source_claims, - decode_worker_mutation_source_claims, verify_worker_mutation_source_proof, + decode_worker_mutation_source_claims, request_body_digest, + verify_worker_mutation_source_proof, }; #[test] @@ -1119,6 +1146,41 @@ mod tests { assert!(request.contains("\"target_worker_id\":\"worker-target\"")); assert!(!request.contains("expected_worker_revision")); assert!(request.contains("\"reason\":\"retire obsolete Worker\"")); + let request_source_token = request + .lines() + .find_map(|line| { + line.split_once(':').and_then(|(name, value)| { + name.eq_ignore_ascii_case(RUNTIME_REQUEST_SOURCE_PROOF_HEADER) + .then(|| value.trim()) + }) + }) + .expect("runtime request source proof header"); + let request_source_claims = + decode_runtime_request_source_claims(request_source_token).unwrap(); + assert_eq!(request_source_claims.iss, "runtime-a"); + assert_eq!(request_source_claims.aud, "server-a"); + assert_eq!(request_source_claims.workspace_id, "workspace-a"); + assert_eq!( + request_source_claims.worker_id.as_deref(), + Some("worker-source") + ); + assert_eq!( + request_source_claims.permission, + WORKSPACE_REQUEST_PERMISSION + ); + assert_eq!(request_source_claims.method, "POST"); + assert_eq!( + request_source_claims.path, + "/api/w/workspace-a/workers/remove" + ); + let request_body = request + .split_once("\r\n\r\n") + .map(|(_, body)| body) + .expect("WorkerRemove request body"); + assert_eq!( + request_source_claims.body_digest, + request_body_digest(request_body.as_bytes()) + ); let token = request .lines() .find_map(|line| { diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index e8f6d1ba..2377ee59 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -1990,16 +1990,18 @@ impl WorkspaceApi { .list_worker_workdir_links(&self.config.workspace_id, worker)? .into_iter() .find(|link| link.unlinked_at.is_none()) - && let Some(access) = repository_access_request_for_workdir( + { + let workdir_runtime_id = registered_workdir_runtime_id(self, &link.workdir_id)?; + if let Some(access) = repository_access_request_for_workdir( self, - &worker.runtime_id, + &workdir_runtime_id, &link.workdir_id, &format!("worker-restore:{}", WorkerId::now_v7()), - )? - { - self.runtime - .authorize_working_directory_repository_access(&worker.runtime_id, access) - .map_err(RuntimeRegistryError::into_error)?; + )? { + self.runtime + .authorize_working_directory_repository_access(&workdir_runtime_id, access) + .map_err(RuntimeRegistryError::into_error)?; + } } let binding = self .runtime @@ -22720,6 +22722,119 @@ mod tests { assert!(!route_body.contains("source")); assert!(!route_body.contains("proof")); + let outer_path = format!("/api/w/{TEST_WORKSPACE_ID}/workers/remove"); + let outer_request_body = r#"{"target_runtime_id":"runtime-target","target_worker_id":"target-worker","reason":"retire target Worker"}"#; + let outer_mutation_token = signer + .issue_worker_remove( + "server-main", + &api.config.workspace_id, + "7", + "runtime-target", + "target-worker", + 60, + ) + .unwrap(); + let outer_request_token = + worker_runtime::auth::RuntimeRequestSourceSigner::from_identity(&identity) + .issue( + "server-main", + &api.config.workspace_id, + Some("7"), + worker_runtime::auth::WORKSPACE_REQUEST_PERMISSION, + "POST", + &outer_path, + outer_request_body.as_bytes(), + i64::try_from(worker_runtime::auth::unix_now_seconds()).unwrap_or(i64::MAX), + 30, + ) + .unwrap(); + let outer_response = build_router(api.clone()) + .oneshot( + Request::builder() + .method("POST") + .uri(&outer_path) + .header(CONTENT_TYPE, "application/json") + .header( + worker_runtime::auth::RUNTIME_REQUEST_SOURCE_PROOF_HEADER, + outer_request_token, + ) + .header( + worker_runtime::auth::WORKER_MUTATION_SOURCE_PROOF_HEADER, + outer_mutation_token, + ) + .body(Body::from(outer_request_body)) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(outer_response.status(), StatusCode::NOT_FOUND); + let outer_response_body = axum::body::to_bytes(outer_response.into_body(), usize::MAX) + .await + .unwrap(); + assert!( + String::from_utf8(outer_response_body.to_vec()) + .unwrap() + .contains("unknown_worker") + ); + + let missing_outer_mutation_token = signer + .issue_worker_remove( + "server-main", + &api.config.workspace_id, + "7", + "runtime-target", + "target-worker", + 60, + ) + .unwrap(); + let missing_outer_response = build_router(api.clone()) + .oneshot( + Request::builder() + .method("POST") + .uri(&outer_path) + .header(CONTENT_TYPE, "application/json") + .header( + worker_runtime::auth::WORKER_MUTATION_SOURCE_PROOF_HEADER, + missing_outer_mutation_token, + ) + .body(Body::from(outer_request_body)) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(missing_outer_response.status(), StatusCode::UNAUTHORIZED); + + let missing_mutation_request_token = + worker_runtime::auth::RuntimeRequestSourceSigner::from_identity(&identity) + .issue( + "server-main", + &api.config.workspace_id, + Some("7"), + worker_runtime::auth::WORKSPACE_REQUEST_PERMISSION, + "POST", + &outer_path, + outer_request_body.as_bytes(), + i64::try_from(worker_runtime::auth::unix_now_seconds()).unwrap_or(i64::MAX), + 30, + ) + .unwrap(); + let missing_mutation_response = build_router(api.clone()) + .oneshot( + Request::builder() + .method("POST") + .uri(&outer_path) + .header(CONTENT_TYPE, "application/json") + .header( + worker_runtime::auth::RUNTIME_REQUEST_SOURCE_PROOF_HEADER, + missing_mutation_request_token, + ) + .body(Body::from(outer_request_body)) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(missing_mutation_response.status(), StatusCode::UNAUTHORIZED); + let mut revoked = trust; revoked.revoked_at = Some("2026-08-11T00:01:00Z".to_string()); let authority = SqliteWorkspaceStore::open(api.config.database_path.clone()).unwrap(); @@ -26480,7 +26595,7 @@ mod tests { ) .await .unwrap(); - let app = build_inner_router(api); + let app = build_inner_router(api.clone()); let runtimes = get_json(app.clone(), "/api/runtimes").await; let embedded_summary = runtimes["items"] @@ -26535,6 +26650,37 @@ mod tests { "embedded_worker_runtime" ); + let workdir_id = "external-workdir"; + api.store + .upsert_workdir_registry(&WorkdirRegistryRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + workdir_id: workdir_id.to_string(), + runtime_id: "external-workdir-runtime".to_string(), + repository_id: "main".to_string(), + creation_selector: None, + creation_ref: None, + creation_tree: None, + current_selector: None, + current_ref: None, + current_tree: None, + observed_at_epoch_seconds: None, + materialization_status: "present".to_string(), + cleanliness: "clean".to_string(), + created_at: "1".to_string(), + updated_at: "1".to_string(), + }) + .unwrap(); + api.store + .attach_worker_workdir(&WorkerWorkdirLinkRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + worker: RuntimeWorkerRef::new("embedded-worker-runtime", &worker_id), + workdir_id: workdir_id.to_string(), + role: "attachment".to_string(), + linked_at: "2".to_string(), + unlinked_at: None, + }) + .unwrap(); + let worker = get_json( app.clone(), &format!("/api/runtimes/embedded-worker-runtime/workers/{worker_id}"), diff --git a/crates/yoi/src/main.rs b/crates/yoi/src/main.rs index 8700b670..6ae7d313 100644 --- a/crates/yoi/src/main.rs +++ b/crates/yoi/src/main.rs @@ -571,20 +571,10 @@ fn parse_console_options( let mut socket_override = None; let mut runtime_id = None; let mut worker_id = None; - let mut standalone_resume = false; - let mut standalone_all = false; let mut i = 0; while i < args.len() { let arg = &args[i]; match arg.as_str() { - "--resume" => { - standalone_resume = true; - i += 1; - } - "--all" => { - standalone_all = true; - i += 1; - } "--worker" => { let value = args .get(i + 1) @@ -766,29 +756,6 @@ fn parse_console_options( &workspace_root, )?; - if standalone_all && !standalone_resume { - return Err(ParseError("--all requires --resume".to_string())); - } - if standalone_resume { - if target.kind() != TargetKind::Standalone { - return Err(ParseError( - "--resume is a Standalone option and requires --local".to_string(), - )); - } - if worker_name.is_some() - || profile.is_some() - || session.is_some() - || socket_override.is_some() - || runtime_id.is_some() - || worker_id.is_some() - { - return Err(ParseError( - "--local --resume cannot be combined with Worker, profile, session, socket, or Runtime selectors" - .to_string(), - )); - } - } - if target.kind() == TargetKind::Standalone { if runtime_id.is_some() || worker_id.is_some() { return Err(ParseError( @@ -798,7 +765,7 @@ fn parse_console_options( } if session.is_some() { return Err(ParseError( - "--local does not accept legacy --session; use --local --resume for Standalone Worker restore" + "--local does not accept legacy --session; use `yoi --local resume` for Standalone Worker restore" .to_string(), )); } @@ -834,16 +801,12 @@ fn parse_console_options( if target.kind() == TargetKind::Standalone && (session.is_some() || socket_override.is_some()) { return Err(ParseError( - "Standalone does not accept legacy Worker session or socket selectors; use --resume for the standalone Worker store" + "Standalone does not accept legacy Worker session or socket selectors; use `yoi --local resume` for the standalone Worker store" .to_string(), )); } - let mode = if standalone_resume { - LaunchMode::StandaloneResume { - include_all: standalone_all, - } - } else if target.kind() == TargetKind::Standalone { + let mode = if target.kind() == TargetKind::Standalone { LaunchMode::Spawn { worker_name, profile, @@ -951,7 +914,7 @@ fn parse_workers_args( )?; if target.kind() != TargetKind::Backend { return Err(ParseError( - "yoi workers requires a Backend connection target; use yoi --local --resume for Standalone Workers" + "yoi workers requires a Backend connection target; use yoi --local resume for Standalone Workers" .to_string(), )); } @@ -1741,7 +1704,6 @@ const TOP_LEVEL_HELP: &str = r#"yoi Usage: yoi [TARGET] - yoi --local --resume [--all] yoi [TARGET] workers [-r|--stopped] [--runtime-id ] yoi [TARGET] resume [--all] [--runtime-id ] yoi --backend [--workspace-id ] panel @@ -1750,8 +1712,6 @@ Usage: Target selection: --local Use the client-owned one-process Standalone host - --resume With --local, restore from the Standalone Worker store - --all With Standalone restore, include Workers from every cwd identity --backend Use a Workspace Backend explicitly --workspace-id Scope Backend routes to a Workspace id @@ -1801,7 +1761,7 @@ Usage: Authority: Lists Workers from the selected Backend Workspace. Standalone Workers are restored with - `yoi --local --resume` and are not part of the Workspace Worker catalog. + `yoi --local resume` and are not part of the Workspace Worker catalog. Options: --backend Use this Workspace Backend @@ -2212,35 +2172,14 @@ backend = "shared" } #[test] - fn parser_local_resume_uses_standalone_picker_scope() { - let mode = parse_args_from(["--local", "--resume"]).unwrap(); - let Mode::Tui { target, mode, .. } = mode else { - panic!("expected TUI mode") - }; - assert_eq!(target.kind(), TargetKind::Standalone); - assert!(matches!( - mode, - LaunchMode::StandaloneResume { include_all: false } - )); - let intent = target.standalone_worker_list(false).unwrap(); - assert!(intent.state_dir.ends_with("client/standalone/workers")); - assert!(!intent.include_all); - - let mode = parse_args_from(["--local", "--resume", "--all"]).unwrap(); - let Mode::Tui { mode, .. } = mode else { - panic!("expected TUI mode") - }; - assert!(matches!( - mode, - LaunchMode::StandaloneResume { include_all: true } - )); - - assert_eq!( - parse_args_from(["--local", "--all"]) - .unwrap_err() - .to_string(), - "--all requires --resume" - ); + fn parser_rejects_removed_top_level_resume_flags() { + for (args, expected) in [ + (vec!["--resume"], "unknown argument: --resume"), + (vec!["--local", "--resume"], "unknown argument: --resume"), + (vec!["--all"], "unknown argument: --all"), + ] { + assert_eq!(parse_args_from(args).unwrap_err().to_string(), expected); + } } #[test] @@ -2264,7 +2203,7 @@ backend = "shared" let err = parse_args_slice_with_connection_resolver(&session_args, &resolver).unwrap_err(); assert_eq!( err.0, - "--local does not accept legacy --session; use --local --resume for Standalone Worker restore" + "--local does not accept legacy --session; use `yoi --local resume` for Standalone Worker restore" ); let socket_args = [ @@ -2897,7 +2836,7 @@ backend = "shared" other => panic!("expected WorkersHelp mode, got {other:?}"), } assert!(WORKERS_HELP.contains("selected Backend Workspace")); - assert!(WORKERS_HELP.contains("--local --resume")); + assert!(WORKERS_HELP.contains("--local resume")); assert!(!WORKERS_HELP.contains("[--local|--backend")); assert!(!WORKERS_HELP.contains("local Worker records")); } diff --git a/docs/README.md b/docs/README.md index 534bf851..954e6cdc 100644 --- a/docs/README.md +++ b/docs/README.md @@ -7,17 +7,17 @@ It is not a dumping ground for external research, old plans, API inventories, or ## Reading order 1. [`design/overview.md`](design/overview.md) — the system map. -2. [`design/context-history.md`](design/context-history.md) — the highest-risk invariant: inputs that affect the model must be committed to history before they enter context. -3. [`design/worker-session-state.md`](design/worker-session-state.md) — Worker identity, replayable session logs, current metadata, and live process hints. -4. [`design/session-observation.md`](design/session-observation.md) — common session captures, `SessionEntryRef`, Memory evidence, and host-authorized Worker observation. -5. [`design/flow-state-graph.md`](design/flow-state-graph.md) — Workspace Flow sources, immutable revisions, transition attempts, and bounded internal verification. -6. [`design/profiles-manifests-prompts.md`](design/profiles-manifests-prompts.md) — reusable Profiles, resolved Manifests, and prompt resources. -7. [`design/tool-permissions-scope.md`](design/tool-permissions-scope.md) — tool policy and filesystem scope. -8. [`design/plugin-packages.md`](design/plugin-packages.md) — plugin package distribution, discovery, and enablement boundaries. -9. [`development/plugin-development.md`](development/plugin-development.md) — how to build, package, enable, and inspect Yoi Plugins. -10. [`design/memory-knowledge.md`](design/memory-knowledge.md) — generated memory and audit records. -11. [`design/workspace-kanban-orchestrator-runtime.md`](design/workspace-kanban-orchestrator-runtime.md) — how Kanban operations become durable orchestration events and backend-internal routing decisions. -12. [`design/durable-operations.md`](design/durable-operations.md) — durable Backend intents that cross Runtime/provider side-effect boundaries, including Workdir removal. +2. [`design/durable-operations.md`](design/durable-operations.md) — cross-domain operation identity, checkpoints, retries, child operations, and disposition, including durable Workdir removal. +3. [`design/context-history.md`](design/context-history.md) — the highest-risk invariant: inputs that affect the model must be committed to history before they enter context. +4. [`design/worker-session-state.md`](design/worker-session-state.md) — Worker identity, replayable session logs, current metadata, and live process hints. +5. [`design/session-observation.md`](design/session-observation.md) — common session captures, `SessionEntryRef`, Memory evidence, and host-authorized Worker observation. +6. [`design/flow-state-graph.md`](design/flow-state-graph.md) — Workspace Flow sources, immutable revisions, transition attempts, and bounded internal verification. +7. [`design/profiles-manifests-prompts.md`](design/profiles-manifests-prompts.md) — reusable Profiles, resolved Manifests, and prompt resources. +8. [`design/tool-permissions-scope.md`](design/tool-permissions-scope.md) — tool policy and filesystem scope. +9. [`design/plugin-packages.md`](design/plugin-packages.md) — plugin package distribution, discovery, and enablement boundaries. +10. [`development/plugin-development.md`](development/plugin-development.md) — how to build, package, enable, and inspect Yoi Plugins. +11. [`design/memory-knowledge.md`](design/memory-knowledge.md) — generated memory and audit records. +12. [`design/workspace-kanban-orchestrator-runtime.md`](design/workspace-kanban-orchestrator-runtime.md) — how Kanban operations become durable orchestration events and backend-internal routing decisions. 13. [`design/workspace-runtime-docker.md`](design/workspace-runtime-docker.md) — the WebUI / Backend / Runtime split, Docker image layout, worker launch path, and workdir materialization boundary. 14. [`development/server-runtime-auth.md`](development/server-runtime-auth.md) — manual Workspace Server / Runtime public-key exchange and authenticated Runtime startup checks. 15. [`development/work-items.md`](development/work-items.md) — how project work is recorded and reviewed. diff --git a/docs/design/durable-operations.md b/docs/design/durable-operations.md index a582e63f..315e93a6 100644 --- a/docs/design/durable-operations.md +++ b/docs/design/durable-operations.md @@ -1,45 +1,335 @@ -# Durable side-effect operations +# Durable operation design -A durable side-effect operation is a Backend-owned intent whose execution crosses an authority boundary, such as a Runtime/provider mutation, and must converge across duplicate requests and bounded recovery. The durable record is not a trace of Rust control flow. +Durable operation records make retries converge on the same authorized intent +and preserve the facts needed to explain an externally visible result. They are +not execution traces and must not mirror every Rust function or implementation +step as persisted state. -## Durable authority +This document defines the cross-domain rules for operation identity, state, +checkpoints, child operations, failure evidence, and terminal disposition. +Domain code may use different records where its atomicity boundary differs, but +it should classify the operation before choosing a schema. -The record stores only facts that affect identity, authorization, replay, or the final domain result: +## Core rule -- a stable operation identity and request fingerprint derived from caller intent; -- the Workspace resource and the resolved authority that exact retries must keep; -- `pending`, `failed`, or `completed` lifecycle state; -- attempt count and timestamps as operational evidence; -- explicit retryability, bounded failure category, and bounded disposition; -- a factual checkpoint only when a non-idempotent provider effect cannot be safely re-observed or repeated. +Persist authority and non-reconstructable facts, not control flow. -A fingerprint excludes Server-generated result identifiers, attempt data, diagnostics, and fresh observations. Reusing one operation identity with a different fingerprint is an error. A completed exact retry replays the committed bounded result. A durable one-pending-operation constraint plus an atomic attempt claim prevents concurrent callers from entering the provider side effect for the same Workdir; the in-process resource lock is an additional serialization layer, not the sole authority. Each active attempt persists the Server process ID and process-start marker. Recovery reclaims only an owner proven missing or replaced; a live or unobservable owner is never stolen. The reclaim transaction CAS-checks the exact proved owner snapshot and attempt count so a stale orphan proof cannot overwrite a newer live claim. +A value belongs in durable operation state only when at least one of the +following is true: -`pending` means only that the intent remains open. Function names, validation steps, and provider-call positions are not persisted as lifecycle stages. `failed` records the latest terminal attempt outcome; retryability remains separate metadata. `completed` means the required domain result and disposition are durably committed. +- it identifies the caller's stable intent and detects conflicting reuse; +- it freezes authority or configuration that an exact retry must continue to + use; +- it binds a preallocated or created resource to the operation; +- it records an externally visible side effect that cannot be safely derived or + repeated; +- it records the final domain result or disposition; +- it provides bounded evidence needed for retry, reconciliation, or audit. -## Live authority and provider evidence - -Before every attempt, the Backend rereads current Workspace ownership and guards. A previous observation that a resource was detached, unblocked, or clean is not authorization for a later retry. - -Provider timeout, unavailability, an empty response, or an unknown outcome is not authoritative absence. Registry cleanup may use only an explicit provider success contract or the provider's exact not-found evidence. If a provider effect is idempotent and exact not-found can be re-observed, arbitrary execution stages and crash-window checkpoints are unnecessary: recovery repeats observation and converges from last committed facts. - -## Workdir removal - -Workdir removal is one durable side-effect operation in the Workspace Server DB. It binds the Workspace, Workdir, owning Runtime, Repository/materialization identity, source actor, stable intent fingerprint, lifecycle, retry metadata, and bounded result. Runtime URL, provider handle, host path, credentials, and caller-selected Runtime are not operation inputs. - -Each attempt: - -1. resolves or revalidates the persisted same-Workspace Workdir, Runtime, Repository, and materialization identity; -2. checks current attachments, attachment reservations, current assignment occupancy, retention/cleanup holds, and pending materialization authority; a failed Workdir-create retry must atomically return to `pending` before provider work and is rejected while removal is pending; -3. retains dirty, occupied, blocked, or otherwise unknown Workdirs without detaching a Worker or forcing deletion; -4. observes the owning Runtime/provider and calls its existing Workdir cleanup only for an eligible clean Workdir; -5. treats only successful provider cleanup or exact `working_directory_not_found` as removal evidence; -6. deletes the Backend Workdir registry row and commits the operation's `completed`/`removed` result in one SQLite transaction. - -A provider error leaves the registry intact and records a bounded `attention_required` result with explicit retryability. Startup recovery lists `pending` and retryable `failed` operations, then executes this same path after rereading live authority. `WorkdirDelete`, Workspace REST removal, Runtime cleanup execution, and recovery must not maintain separate inline provider-delete paths. - -The public request contains only `working_directory_id` plus a bounded reason. The public result contains only the Workdir ID, `removed | retained | attention_required`, retryability, and an optional bounded failure category. Internal operation identifiers, checkpoints, provider paths, and credentials are not public DTO fields. +A local step does not become durable merely because it occurs before or after +another function call. If current authority can be reread or the step can be +repeated safely, derive or repeat it instead of adding a stage. ## Resilience boundary -This pattern covers duplicate requests, returned failures, timeouts and unknown outcomes, known partial-completion contracts, and restart recovery from committed facts. It does not provide general exactly-once execution or claim recovery from every instruction-boundary panic, process kill, machine loss, or power failure. A stronger provider guarantee requires a separately specified protocol, checkpoint ordering, reconciliation evidence, and tests. +These rules cover normal product retry and recovery boundaries: duplicate +requests, returned errors, timeouts, known partial-completion outcomes, process +restart from the last committed facts, and retries of provider operations with +an explicit idempotency or observation contract. + +They do not require the system to survive an unexpected stop at every point +while Rust code is executing. A panic, abort, process kill, machine loss, or +power failure may occur between an external side effect and its next durable +checkpoint. Yoi does not attempt to close every such instruction-level crash +window by writing a stage before and after every `await`, function, or provider +call. + +Consequently: + +- do not claim general exactly-once execution; +- do not introduce write-ahead stages solely to model arbitrary Rust + control-flow interruption; +- rely on SQLite transaction atomicity for work inside one database transaction; +- prefer provider idempotency, compare-and-swap, stable resource identity, and + authoritative observation for external effects; +- when an uncovered crash window cannot be reconciled automatically, retain the + last committed facts and surface an `unknown` or attention-required + disposition rather than guessing that the side effect did or did not happen. + +A domain may require a stronger crash-consistency contract for a specific +destructive or security-sensitive effect. That requirement must be explicit and +must define the provider protocol, checkpoint ordering, replay behavior, and +reconciliation evidence. It is not implied by calling a record a durable +operation. + +## Classify the record before adding state + +Not every record containing an `operation_id` is a state machine. Use one of the +following shapes. + +### Atomic idempotency ledger + +Use an idempotency ledger when all authoritative mutations and result recording +commit in one database transaction. + +The record normally contains: + +- Workspace and operation identity; +- a fingerprint of stable caller intent; +- the created resource or result identity; +- the committed revision and timestamp where relevant. + +It does not need `pending`, `executing`, or intermediate stages. An exact retry +returns the recorded result. Reusing the same operation identity with a +different fingerprint fails. + +Repository secret mutation results, Workspace resource creation results, and +transactionally appended domain events are examples of this shape. + +### Reservation + +Use a reservation when an identity or exclusive right must exist before a later +binding can complete. + +Persist factual transitions such as: + +- the reserved resource identity; +- the immutable request fingerprint and authority snapshot; +- the concrete resource or assignment bound to the reservation; +- reservation expiry or release evidence when the contract requires it. + +Do not model internal dispatch, validation, construction, or callback steps as +reservation states. A nullable result binding or a small `reserved | created` +state can be sufficient when those values correspond to real authority facts. + +### Durable side-effect operation + +Use a durable side-effect operation when work crosses a database/provider +boundary and a retry needs durable intent or result evidence. + +The default lifecycle is deliberately small: + +```text +pending -> completed +pending -> failed +failed -> pending # only when the domain explicitly permits retry +``` + +Existing code may use `succeeded` for the successful terminal value; new naming +should prefer `completed`. Do not rewrite applied migrations or historical audit +text only to normalize that word. + +The operation should contain: + +- stable operation identity and request fingerprint; +- immutable resolved authority needed by an exact retry; +- preallocated resource identity where it prevents duplicate creation; +- only the necessary irreversible checkpoints; +- bounded failure evidence; +- the final result and domain disposition. + +`pending` means that the intent remains open and current authority must be +reread before progress. It does not identify which Rust function should execute +next. `failed` records the latest terminal attempt outcome; retryability is an +explicit domain rule, not something inferred from the word. `completed` means +the operation's required result and evidence are durably committed. + +### Parent workflow + +A parent workflow coordinates domain operations but does not duplicate their +lifecycle. + +Persist: + +- the parent intent and fencing authority; +- stable child operation identities; +- the final workflow result or disposition; +- bounded attention or decision evidence. + +Read child state from the child authority. Do not copy child states, provider +stages, Worker status, attachment status, or Workdir status into a second parent +state machine. A parent cleanup workflow will often need only +`pending | completed`; child failure remains on the child operation and appears +in the parent as current attention metadata. + +Creating or binding a child must itself be idempotent. Prefer a deterministic +child operation identity or persist the child reference atomically with the +parent decision so a retry cannot create siblings for one intent. + +## Checkpoint rules + +A checkpoint records a fact that changes retry semantics. It is not a progress +notification. + +Add a checkpoint only when all of the following hold: + +1. A side effect may already have occurred outside the current transaction. +2. Current authority cannot derive the fact reliably enough for safe retry, or + repeating the effect is not safe under the provider contract. +3. The retry algorithm changes after the fact is committed. +4. Tests can exercise behavior before and after the checkpoint. + +Prefer factual fields over stage names: + +- `provider_deleted_at` is evidence that provider deletion succeeded; +- `child_operation_id` binds delegated work; +- `result_revision` identifies the committed result; +- `target_ref_after` records verified merge evidence. + +Avoid fields such as `validating`, `closing_session`, `detaching`, +`deleting_registry`, or `finalizing`. Those names describe code location, not +durable authority. If those steps are safe to rerun or their result can be read +from Worker, attachment, Workdir, repository, or provider authority, they are +not checkpoints. + +A checkpoint must never claim more than the authority that produced it. For +example, sending a provider request is not proof that provider deletion +completed, and receiving a Worker notification is not proof that a Ticket or +cleanup workflow completed. + +## State, failure, blockers, and disposition are separate + +Do not overload one enum with unrelated dimensions. + +- **Operation state** says whether the intent is open, completed, or has a + recorded failed attempt. +- **Failure evidence** records a bounded category, timestamp, and safe + diagnostic detail for the latest failure. +- **Blockers and eligibility** are normally derived by rereading current + authority. Persist them only as audit or attention evidence, not as a + substitute for live validation. +- **Disposition** records what the domain decided to retain, delete, release, + tombstone, abandon, or leave unknown. +- **Attention metadata** explains why automated progress currently cannot + continue and what authority must change. + +Values such as `blocked`, `executing`, `stale`, `dirty`, `retained`, and +`deleted` therefore do not all belong in one operation-state enum. Some are +derived conditions, some describe transient execution, and some are domain +results. + +Before every retry or side effect, reread live authority and revalidate its +fence. A previously recorded blocker does not prove that the operation remains +blocked, and a previously unblocked operation does not retain permission after +assignment, ownership, revision, or attachment authority changes. + +## Identity and fingerprinting + +Every externally retryable operation has a stable identity in its owning +Workspace or authority scope. The operation fingerprint represents stable caller +intent, not generated results or mutable observations. + +Include inputs whose change would mean a different requested operation. Exclude: + +- generated resource IDs when the Server allocates and persists them as the + result; +- timestamps assigned by the Server; +- retry counters and diagnostics; +- current provider observations that are expected to change; +- secret bytes and credential material. + +Resolved authority snapshots may be stored separately from the caller +fingerprint. An exact retry uses the persisted snapshot where replay convergence +requires it; a new operation resolves current authority. Unknown, foreign, or +conflicting operation identity fails closed. + +## Transactions and external providers + +Keep database work in one transaction whenever the owning authority and result +live in the same database. Do not create a durable operation merely to split a +transaction that can remain atomic. + +When an external provider is involved: + +1. reserve stable intent and identity if retry needs them; +2. invoke the provider with the strongest available idempotency, expected-old + revision, or stable resource key; +3. verify the provider result through authoritative response or observation; +4. commit only the checkpoint or result evidence that changes retry behavior; +5. on retry, reread both the operation and current domain/provider authority + before acting. + +Compensation is a domain operation, not an invisible `finally` block. If +compensation has its own external side effects or retry lifecycle, give it a +stable child operation identity rather than expanding the parent into a list of +cleanup stages. + +## Workdir removal application + +Workdir removal is one durable side-effect operation in the Workspace Server +DB. It binds the Workspace, Workdir, owning Runtime, +Repository/materialization identity, source actor, stable intent fingerprint, +lifecycle, retry metadata, and bounded result. Runtime URL, provider handle, +host path, credentials, and caller-selected Runtime are not operation inputs. + +A durable one-pending-operation constraint plus an atomic attempt claim prevents +concurrent callers from entering the provider side effect for the same Workdir; +the in-process resource lock is an additional serialization layer, not the sole +authority. Each active attempt persists the Server process ID and process-start +marker. Recovery reclaims only an owner proven missing or replaced; a live or +unobservable owner is never stolen. The reclaim transaction compare-and-set +checks the exact proved owner snapshot and attempt count so stale orphan proof +cannot overwrite a newer live claim. + +Each attempt: + +1. resolves or revalidates the persisted same-Workspace Workdir, Runtime, + Repository, and materialization identity; +2. checks current attachments, attachment reservations, current assignment + occupancy, retention/cleanup holds, and pending materialization authority; a + failed Workdir-create retry must atomically return to `pending` before + provider work and is rejected while removal is pending; +3. retains dirty, occupied, blocked, or otherwise unknown Workdirs without + detaching a Worker or forcing deletion; +4. observes the owning Runtime/provider and calls its existing Workdir cleanup + only for an eligible clean Workdir; +5. treats only successful provider cleanup or exact + `working_directory_not_found` as removal evidence; +6. deletes the Backend Workdir registry row and commits the operation's + `completed`/`removed` result in one SQLite transaction. + +A provider error leaves the registry intact and records a bounded +`attention_required` result with explicit retryability. Startup recovery lists +`pending` and retryable `failed` operations, then executes this same path after +rereading live authority. `WorkdirDelete`, Workspace REST removal, Runtime +cleanup execution, and recovery must not maintain separate inline +provider-delete paths. + +The public request contains only `working_directory_id` plus a bounded reason. +The public result contains only the Workdir ID, +`removed | retained | attention_required`, retryability, and an optional bounded +failure category. Internal operation identifiers, checkpoints, provider paths, +and credentials are not public DTO fields. + +## Diagnostics and audit + +Persist bounded error categories and identifiers needed to investigate or retry. +Do not persist credentials, provider handles, raw command output, raw prompts, +full session transcripts, or host paths in ordinary operation diagnostics. + +Attempt counts, last-attempt timestamps, and safe provider categories may help +operations, but they are telemetry and evidence rather than lifecycle authority. +Logs may describe detailed execution stages; the durable record should remain +centered on intent, checkpoints, result, and disposition. + +## Applying this rule + +For a new or materially changed operation: + +1. identify the owning authority and transaction boundary; +2. classify it as an atomic ledger, reservation, durable side-effect operation, + or parent workflow; +3. define stable identity, fingerprint, and exact-retry behavior; +4. list external side effects and decide which are idempotent or authoritatively + observable; +5. add only checkpoints that change retry behavior; +6. keep child operation state in the child authority; +7. separate failure, blocker, attention, and disposition from lifecycle state; +8. state the unsupported crash windows honestly; +9. test fingerprint conflict, exact retry, authority revalidation, checkpoint + replay, and result/disposition projection as applicable. + +Existing operation schemas need not be rewritten solely for vocabulary +consistency. When an operation is changed for functional reasons, use this +classification to remove derived or control-flow stages rather than adding +another special-case lifecycle. diff --git a/web/workspace/deno.json b/web/workspace/deno.json index f6bb8c2a..28c50342 100644 --- a/web/workspace/deno.json +++ b/web/workspace/deno.json @@ -6,7 +6,7 @@ "dev": "deno run -A npm:vite@7.2.7 dev", "dev:backend": "cd ../.. && cargo run -p yoi-workspace-server --bin yoi-server -- serve --listen 127.0.0.1:8787", "check": "deno run -A npm:@sveltejs/kit@2.49.4 sync && deno run -A npm:svelte-check@4.3.4 --tsconfig ./tsconfig.json", - "test": "deno test --allow-read=src,test,tests --allow-env=LOG,VSCODE_TEXTMATE_DEBUG,NODE_ENV tests/workspace-model.test.ts tests/workspace-catalog.test.ts tests/profile-api.test.ts src/lib/workspace/auth/model.test.ts src/lib/workspace/api/http.test.ts src/lib/workspace/header/breadcrumb-model.test.ts src/lib/workspace/console/chat-submit.test.ts tests/composer-paste.test.ts src/lib/workspace/console/composer-command.test.ts src/lib/workspace/console/composer-draft.test.ts src/lib/workspace/console/composer-completion.test.ts src/lib/workspace/console/markdown.test.ts test/console/ansi.test.ts src/lib/workspace/console/model.test.ts src/lib/workspace/companion/api.test.ts tests/workdir-api.test.ts src/lib/workspace/console/tasks.test.ts test/ticket-detail-route-reuse.test.ts test/repositories/ui.test.ts src/lib/workspace/console/worker-console.ui.test.ts src/lib/workspace/settings/model.test.ts src/lib/workspace/sidebar/override-stack.test.ts src/lib/workspace/sidebar/workers.test.ts src/lib/workspace/sidebar/workspace-switcher.test.ts src/lib/workspace/sidebar/worker-subscription.test.ts src/lib/workspace/sidebar/worker-launch.test.ts src/lib/workspace/tickets/merge-request-resources.test.ts src/lib/workspace/tickets/ticket-panel.test.ts test/merge-request-status.test.ts test/config-source/decodal-grammar.test.ts test/config-source/editor-state.test.ts test/config-source/fixed-schema-wrapper.test.ts test/config-source/toolchain.test.ts test/config-source/wasm-parity.test.ts test/repository-access/api.test.ts test/repository-access/loader.test.ts test/repository-access/ui.test.ts", + "test": "deno test --allow-read=src,test,tests --allow-env=LOG,VSCODE_TEXTMATE_DEBUG,NODE_ENV tests/workspace-model.test.ts tests/workspace-catalog.test.ts tests/profile-api.test.ts src/lib/workspace/auth/model.test.ts src/lib/workspace/api/http.test.ts src/lib/workspace/header/breadcrumb-model.test.ts src/lib/workspace/console/chat-submit.test.ts test/composer-history.test.ts tests/composer-paste.test.ts src/lib/workspace/console/composer-command.test.ts src/lib/workspace/console/composer-draft.test.ts src/lib/workspace/console/composer-completion.test.ts src/lib/workspace/console/markdown.test.ts test/console/ansi.test.ts src/lib/workspace/console/model.test.ts src/lib/workspace/companion/api.test.ts tests/workdir-api.test.ts src/lib/workspace/console/tasks.test.ts test/ticket-detail-route-reuse.test.ts test/repositories/ui.test.ts src/lib/workspace/console/worker-console.ui.test.ts src/lib/workspace/settings/model.test.ts src/lib/workspace/sidebar/override-stack.test.ts src/lib/workspace/sidebar/workers.test.ts src/lib/workspace/sidebar/workspace-switcher.test.ts src/lib/workspace/sidebar/worker-subscription.test.ts src/lib/workspace/sidebar/worker-launch.test.ts test/sidebar/worker-actions.test.ts src/lib/workspace/tickets/merge-request-resources.test.ts src/lib/workspace/tickets/ticket-panel.test.ts test/merge-request-status.test.ts test/config-source/decodal-grammar.test.ts test/config-source/editor-state.test.ts test/config-source/fixed-schema-wrapper.test.ts test/config-source/toolchain.test.ts test/config-source/wasm-parity.test.ts test/repository-access/api.test.ts test/repository-access/loader.test.ts test/repository-access/ui.test.ts", "build": "deno run -A npm:vite@7.2.7 build", "preview": "deno run -A npm:vite@7.2.7 preview" }, @@ -20,6 +20,7 @@ "@codemirror/language": "npm:@codemirror/language@6.12.4", "@codemirror/state": "npm:@codemirror/state@6.7.1", "@codemirror/view": "npm:@codemirror/view@6.43.8", + "@lezer/common": "npm:@lezer/common@1.5.2", "@lezer/highlight": "npm:@lezer/highlight@1.2.3", "decodal-codemirror": "npm:decodal-codemirror@0.3.0", "clsx": "npm:clsx@2.1.1", diff --git a/web/workspace/deno.lock b/web/workspace/deno.lock index 71b8b841..dc902d3a 100644 --- a/web/workspace/deno.lock +++ b/web/workspace/deno.lock @@ -9,6 +9,7 @@ "npm:@codemirror/state@6.7.1": "6.7.1", "npm:@codemirror/view@6.43.8": "6.43.8", "npm:@fontsource/ibm-plex-mono@5.3.0": "5.3.0", + "npm:@lezer/common@1.5.2": "1.5.2", "npm:@lezer/highlight@1.2.3": "1.2.3", "npm:@sveltejs/adapter-static@3.0.9": "3.0.9_@sveltejs+kit@2.49.4__@sveltejs+vite-plugin-svelte@6.2.1___svelte@5.45.6___vite@7.2.7____yaml@2.9.0___yaml@2.9.0__svelte@5.45.6__typescript@5.9.3__vite@7.2.7___yaml@2.9.0__yaml@2.9.0_yaml@2.9.0", "npm:@sveltejs/kit@2.49.4": "2.49.4_@sveltejs+vite-plugin-svelte@6.2.1__svelte@5.45.6__vite@7.2.7___yaml@2.9.0__yaml@2.9.0_svelte@5.45.6_typescript@5.9.3_vite@7.2.7__yaml@2.9.0_yaml@2.9.0", @@ -1026,6 +1027,7 @@ "npm:@codemirror/language@6.12.4", "npm:@codemirror/state@6.7.1", "npm:@codemirror/view@6.43.8", + "npm:@lezer/common@1.5.2", "npm:@lezer/highlight@1.2.3", "npm:@sveltejs/adapter-static@3.0.9", "npm:@sveltejs/kit@2.49.4", diff --git a/web/workspace/src/lib/workspace/api/http.test.ts b/web/workspace/src/lib/workspace/api/http.test.ts index 93521aa6..eefa309f 100644 --- a/web/workspace/src/lib/workspace/api/http.test.ts +++ b/web/workspace/src/lib/workspace/api/http.test.ts @@ -43,17 +43,21 @@ Deno.test("root layout leaves Workspace selection explicit", async () => { new URL("./../../../routes/+layout.ts", import.meta.url), ); assert( - !layout.includes("/api/workspace") && + !layout.includes('"/api/workspace"') && !layout.includes("redirect(") && - layout.includes("Workspace selection is explicit"), - "root layout must not infer or redirect to a singleton Workspace", + layout.includes("listWorkspaces(fetch)") && + layout.includes("accessibleWorkspaces"), + "root layout may list accessible Workspaces but must not infer or redirect to a singleton Workspace", ); }); Deno.test("Workspace route changes dispose old multiplexed subscription state", async () => { const [layout, multiplexer] = await Promise.all([ Deno.readTextFile( - new URL("./../../../routes/w/[workspaceId]/+layout.svelte", import.meta.url), + new URL( + "./../../../routes/w/[workspaceId]/+layout.svelte", + import.meta.url, + ), ), Deno.readTextFile(new URL("./../multiplexer.ts", import.meta.url)), ]); diff --git a/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.svelte b/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.svelte index b3d6cfa9..433efc62 100644 --- a/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.svelte +++ b/web/workspace/src/lib/workspace/config-source/ConfigSourceEditor.svelte @@ -328,10 +328,12 @@ source = value} - onComplete={(value, offset, explicit) => toolchain?.complete(selectedPath, value, offset, explicit) ?? Promise.resolve(null)} + onComplete={(value, offset, explicit) => analysisReady && toolchain + ? toolchain.complete(selectedPath, value, offset, explicit) + : Promise.resolve(null)} />

{status}

{#if conflict} diff --git a/web/workspace/src/lib/workspace/config-source/completion.ts b/web/workspace/src/lib/workspace/config-source/completion.ts index 8467bdb2..81ad4888 100644 --- a/web/workspace/src/lib/workspace/config-source/completion.ts +++ b/web/workspace/src/lib/workspace/config-source/completion.ts @@ -12,13 +12,18 @@ type ConfigSourceCompletionItem = { priority: number; }; +export function shouldStartCompletionAfterTyping( + insertedText: string, +): boolean { + return /\S/u.test(insertedText); +} + export function toCodeMirrorCompletion( - source: string, result: ConfigSourceCompletionResult | null, ): CompletionResult | null { if (!result) return null; return { - from: utf8ByteOffsetToUtf16(source, result.from), + from: result.from, options: result.items.map((item) => ({ label: item.label, type: item.kind, @@ -27,31 +32,3 @@ export function toCodeMirrorCompletion( })), }; } - -function utf8ByteOffsetToUtf16(source: string, byteOffset: number): number { - if (!Number.isSafeInteger(byteOffset) || byteOffset < 0) { - throw new RangeError( - "completion byte offset must be a non-negative integer", - ); - } - - let bytes = 0; - let utf16 = 0; - for (const character of source) { - if (bytes === byteOffset) return utf16; - const codePoint = character.codePointAt(0)!; - bytes += codePoint <= 0x7f - ? 1 - : codePoint <= 0x7ff - ? 2 - : codePoint <= 0xffff - ? 3 - : 4; - utf16 += character.length; - if (bytes > byteOffset) { - throw new RangeError("completion byte offset splits a UTF-8 code point"); - } - } - if (bytes === byteOffset) return utf16; - throw new RangeError("completion byte offset is outside the source"); -} diff --git a/web/workspace/src/lib/workspace/config-source/generated/config_source_wasm_bg.wasm b/web/workspace/src/lib/workspace/config-source/generated/config_source_wasm_bg.wasm index e2f01466..4662c4ac 100644 Binary files a/web/workspace/src/lib/workspace/config-source/generated/config_source_wasm_bg.wasm and b/web/workspace/src/lib/workspace/config-source/generated/config_source_wasm_bg.wasm differ diff --git a/web/workspace/src/lib/workspace/config-source/toolchain.ts b/web/workspace/src/lib/workspace/config-source/toolchain.ts index 510b4a00..53969a0f 100644 --- a/web/workspace/src/lib/workspace/config-source/toolchain.ts +++ b/web/workspace/src/lib/workspace/config-source/toolchain.ts @@ -79,7 +79,7 @@ export class ConfigSourceToolchain { utf16Offset, explicit, }); - return toCodeMirrorCompletion(source, result); + return toCodeMirrorCompletion(result); } format(source: string): Promise { return this.#request({ kind: "format", source }); diff --git a/web/workspace/src/lib/workspace/config-source/vite-dedupe.ts b/web/workspace/src/lib/workspace/config-source/vite-dedupe.ts new file mode 100644 index 00000000..c41ab5ba --- /dev/null +++ b/web/workspace/src/lib/workspace/config-source/vite-dedupe.ts @@ -0,0 +1,7 @@ +export const CODEMIRROR_VITE_DEDUPE = [ + "@codemirror/autocomplete", + "@codemirror/language", + "@codemirror/state", + "@codemirror/view", + "@lezer/common", +]; diff --git a/web/workspace/src/lib/workspace/console/ComposerInput.svelte b/web/workspace/src/lib/workspace/console/ComposerInput.svelte index 36672a8c..9bb6235c 100644 --- a/web/workspace/src/lib/workspace/console/ComposerInput.svelte +++ b/web/workspace/src/lib/workspace/console/ComposerInput.svelte @@ -37,12 +37,21 @@ type ComposerPaste, type ComposerTextPaste, } from "$lib/workspace/console/composer-draft.ts"; + import { + ComposerHistory, + loadComposerHistory, + saveComposerHistory, + shouldBrowseComposerHistory, + type ComposerHistoryDirection, + type ComposerHistoryEntry, + } from "$lib/workspace/console/composer-history.ts"; import { shouldSubmitChatKey } from "$lib/workspace/console/chat-submit.ts"; interface Props { disabled?: boolean; ariaLabel?: string; ariaKeyShortcuts?: string; + historyScope: string; onchange?: (snapshot: ComposerDraftSnapshot) => void; onkeydown?: (event: KeyboardEvent) => void; onsubmit?: () => void; @@ -52,6 +61,7 @@ disabled = false, ariaLabel = "Message", ariaKeyShortcuts = "Meta+Enter Control+Enter", + historyScope, onchange, onkeydown, onsubmit, @@ -59,6 +69,8 @@ let mountElement: HTMLDivElement; let view: EditorView | null = null; + let composerHistory = new ComposerHistory(); + let restoringHistory = false; let nextPasteId = 1; let nextPasteKey = 1; const editable = new Compartment(); @@ -179,6 +191,40 @@ onchange?.(currentSnapshot()); } + function historyEntry(state: EditorState): ComposerHistoryEntry { + const snapshot = currentSnapshot(state); + return { + segments: snapshot.segments, + preserveExactText: snapshot.textPastes.length > 0, + }; + } + + function browseHistory(currentView: EditorView, direction: ComposerHistoryDirection): boolean { + const selection = currentView.state.selection.main; + const cursorLine = currentView.state.doc.lineAt(selection.head).number; + if (!shouldBrowseComposerHistory({ + direction, + cursorLine, + lineCount: currentView.state.doc.lines, + selectionEmpty: selection.empty, + readOnly: currentView.state.readOnly, + composing: currentView.composing, + })) return false; + + const entry = direction === "older" + ? composerHistory.previous(historyEntry(currentView.state)) + : composerHistory.next(); + if (!entry) return false; + + restoringHistory = true; + try { + restoreSegments(entry.segments, entry.preserveExactText); + } finally { + restoringHistory = false; + } + return true; + } + function insertPasteChip(content: string, measurement: ComposerPasteMeasurement): void { if (!view) return; const selection = view.state.selection.main; @@ -273,6 +319,10 @@ return true; } + $effect(() => { + composerHistory = loadComposerHistory(localStorage, historyScope); + }); + onMount(() => { view = new EditorView({ parent: mountElement, @@ -292,6 +342,14 @@ key: "Mod-y", run: (currentView) => currentView.state.readOnly, }, + { + key: "ArrowUp", + run: (currentView) => browseHistory(currentView, "older"), + }, + { + key: "ArrowDown", + run: (currentView) => browseHistory(currentView, "newer"), + }, { key: "Backspace", run: (currentView) => @@ -319,6 +377,7 @@ spellcheck: "true", }), EditorView.updateListener.of((update) => { + if (update.docChanged && !restoringHistory) composerHistory.cancelNavigation(); if (update.docChanged || update.transactions.some((tx) => tx.effects.length > 0)) { emitChange(); } @@ -396,6 +455,16 @@ return currentSnapshot(); } + export function recordHistory(value: ComposerDraftSnapshot): void { + const entry: ComposerHistoryEntry = { + segments: value.segments, + preserveExactText: value.textPastes.length > 0, + }; + if (composerHistory.record(entry)) { + saveComposerHistory(localStorage, historyScope, composerHistory); + } + } + export function focus(): void { view?.focus(); } diff --git a/web/workspace/src/lib/workspace/console/composer-history.ts b/web/workspace/src/lib/workspace/console/composer-history.ts new file mode 100644 index 00000000..62698094 --- /dev/null +++ b/web/workspace/src/lib/workspace/console/composer-history.ts @@ -0,0 +1,193 @@ +import type { Segment } from "$lib/generated/protocol"; + +export const COMPOSER_HISTORY_LIMIT = 30; +const COMPOSER_HISTORY_VERSION = 1; +const COMPOSER_HISTORY_KEY_PREFIX = "yoi.composer-history.v1.workspace."; + +export type ComposerHistoryDirection = "older" | "newer"; + +export type ComposerHistoryCursor = { + direction: ComposerHistoryDirection; + cursorLine: number; + lineCount: number; + selectionEmpty: boolean; + readOnly: boolean; + composing: boolean; +}; + +export type ComposerHistoryEntry = { + segments: Segment[]; + preserveExactText: boolean; +}; + +type StoredComposerHistory = { + version: typeof COMPOSER_HISTORY_VERSION; + entries: ComposerHistoryEntry[]; +}; + +type ComposerHistoryStorage = Pick; + +function cloneEntry(entry: ComposerHistoryEntry): ComposerHistoryEntry { + return { + segments: entry.segments.map((segment) => ({ ...segment })) as Segment[], + preserveExactText: entry.preserveExactText, + }; +} + +function isSegment(value: unknown): value is Segment { + if (!value || typeof value !== "object") return false; + const segment = value as Record; + if (segment.kind === "text") return typeof segment.content === "string"; + if (segment.kind === "paste") { + return typeof segment.content === "string" && + typeof segment.id === "number" && + typeof segment.chars === "number" && + typeof segment.lines === "number"; + } + if (segment.kind === "file_ref") return typeof segment.path === "string"; + return false; +} + +function isHistoryEntry(value: unknown): value is ComposerHistoryEntry { + if (!value || typeof value !== "object") return false; + const entry = value as Record; + return Array.isArray(entry.segments) && + entry.segments.every(isSegment) && + typeof entry.preserveExactText === "boolean"; +} + +function isBlankEntry(entry: ComposerHistoryEntry): boolean { + return entry.segments.length === 0 || + entry.segments.every((segment) => + segment.kind === "text" && segment.content.trim().length === 0 + ); +} + +function sameEntry( + left: ComposerHistoryEntry, + right: ComposerHistoryEntry, +): boolean { + return JSON.stringify(left) === JSON.stringify(right); +} + +export function shouldBrowseComposerHistory( + cursor: ComposerHistoryCursor, +): boolean { + if (cursor.readOnly || cursor.composing || !cursor.selectionEmpty) { + return false; + } + return cursor.direction === "older" + ? cursor.cursorLine === 1 + : cursor.cursorLine === cursor.lineCount; +} + +export function composerHistoryStorageKey(workspaceId: string): string { + return `${COMPOSER_HISTORY_KEY_PREFIX}${encodeURIComponent(workspaceId)}`; +} + +export class ComposerHistory { + #entries: ComposerHistoryEntry[]; + #index: number | null = null; + #draft: ComposerHistoryEntry | null = null; + + constructor(entries: ComposerHistoryEntry[] = []) { + this.#entries = []; + for (const entry of entries) this.record(entry); + } + + get entries(): ComposerHistoryEntry[] { + return this.#entries.map(cloneEntry); + } + + get browsing(): boolean { + return this.#index !== null; + } + + record(entry: ComposerHistoryEntry): boolean { + if (isBlankEntry(entry)) { + this.cancelNavigation(); + return false; + } + const last = this.#entries.at(-1); + if (last && sameEntry(last, entry)) { + this.cancelNavigation(); + return false; + } + this.#entries.push(cloneEntry(entry)); + if (this.#entries.length > COMPOSER_HISTORY_LIMIT) { + this.#entries.splice(0, this.#entries.length - COMPOSER_HISTORY_LIMIT); + } + this.cancelNavigation(); + return true; + } + + previous(draft: ComposerHistoryEntry): ComposerHistoryEntry | null { + if (this.#entries.length === 0) return null; + if (this.#index === null) { + this.#draft = cloneEntry(draft); + this.#index = this.#entries.length - 1; + } else if (this.#index > 0) { + this.#index -= 1; + } + return cloneEntry(this.#entries[this.#index]); + } + + next(): ComposerHistoryEntry | null { + if (this.#index === null) return null; + if (this.#index < this.#entries.length - 1) { + this.#index += 1; + return cloneEntry(this.#entries[this.#index]); + } + const draft = this.#draft + ? cloneEntry(this.#draft) + : { segments: [], preserveExactText: false }; + this.cancelNavigation(); + return draft; + } + + cancelNavigation(): void { + this.#index = null; + this.#draft = null; + } +} + +export function loadComposerHistory( + storage: ComposerHistoryStorage, + workspaceId: string, +): ComposerHistory { + try { + const raw = storage.getItem(composerHistoryStorageKey(workspaceId)); + if (!raw) return new ComposerHistory(); + const value = JSON.parse(raw) as unknown; + if (!value || typeof value !== "object") return new ComposerHistory(); + const stored = value as Partial; + if ( + stored.version !== COMPOSER_HISTORY_VERSION || + !Array.isArray(stored.entries) + ) { + return new ComposerHistory(); + } + return new ComposerHistory(stored.entries.filter(isHistoryEntry)); + } catch { + return new ComposerHistory(); + } +} + +export function saveComposerHistory( + storage: ComposerHistoryStorage, + workspaceId: string, + history: ComposerHistory, +): void { + const value: StoredComposerHistory = { + version: COMPOSER_HISTORY_VERSION, + entries: history.entries, + }; + try { + storage.setItem( + composerHistoryStorageKey(workspaceId), + JSON.stringify(value), + ); + } catch { + // History is an optional convenience; storage failures must not block input submission. + } +} 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 cd71662f..869603f3 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 @@ -358,9 +358,10 @@ Deno.test("root layout keeps Workspace selection explicit", async () => { assert( layoutLoad.includes("export const load") && - layoutLoad.includes("() => ({})") && + layoutLoad.includes("listWorkspaces(fetch)") && + layoutLoad.includes("accessibleWorkspaces") && !layoutLoad.includes("scopedCompatibilityRoute") && - !layoutLoad.includes("/api/workspace") && + !layoutLoad.includes('"/api/workspace"') && !layoutLoad.includes("workspaceRoute") && !layoutLoad.includes("redirect("), "root layout should not infer, bootstrap, or redirect through a singleton Workspace", @@ -812,6 +813,9 @@ Deno.test("Account UI owns browser passkey session state without workspace autho const globalSidebar = await Deno.readTextFile( new URL("../sidebar/GlobalSidebar.svelte", import.meta.url), ); + const globalNavSections = await Deno.readTextFile( + new URL("../sidebar/GlobalNavSections.svelte", import.meta.url), + ); const sidebarFrame = await Deno.readTextFile( new URL("../sidebar/SidebarFrame.svelte", import.meta.url), ); @@ -874,13 +878,23 @@ Deno.test("Account UI owns browser passkey session state without workspace autho "Root layout chrome should keep GlobalSidebar as the root slot owner while account navigation stays in the header", ); assert( - globalSidebar.includes('aria-label="Global pages"') && - !globalSidebar.includes(' diff --git a/web/workspace/src/lib/workspace/sidebar/GlobalSidebar.svelte b/web/workspace/src/lib/workspace/sidebar/GlobalSidebar.svelte index 2a704ed8..fcf7fc02 100644 --- a/web/workspace/src/lib/workspace/sidebar/GlobalSidebar.svelte +++ b/web/workspace/src/lib/workspace/sidebar/GlobalSidebar.svelte @@ -1,20 +1,22 @@ {#if content} @@ -22,18 +24,7 @@ {:else} {/if} diff --git a/web/workspace/src/lib/workspace/sidebar/WorkersNavSection.svelte b/web/workspace/src/lib/workspace/sidebar/WorkersNavSection.svelte index a954f46a..c50a4220 100644 --- a/web/workspace/src/lib/workspace/sidebar/WorkersNavSection.svelte +++ b/web/workspace/src/lib/workspace/sidebar/WorkersNavSection.svelte @@ -1,6 +1,12 @@ + +