fix: reject failed worker restores before attach
This commit is contained in:
@@ -16,6 +16,7 @@ pub use workspace_api::{
|
|||||||
WorkerLaunchOptionsResponse as BackendWorkerLaunchOptions,
|
WorkerLaunchOptionsResponse as BackendWorkerLaunchOptions,
|
||||||
WorkerLaunchProfileCandidate as BackendWorkerLaunchProfileCandidate,
|
WorkerLaunchProfileCandidate as BackendWorkerLaunchProfileCandidate,
|
||||||
WorkerLaunchRuntimeOption as BackendWorkerLaunchRuntimeOption,
|
WorkerLaunchRuntimeOption as BackendWorkerLaunchRuntimeOption,
|
||||||
|
WorkerOperationState as BackendWorkerOperationState,
|
||||||
WorkerRestoreResponse as BackendWorkerRestoreResponse,
|
WorkerRestoreResponse as BackendWorkerRestoreResponse,
|
||||||
WorkerRestoreResult as BackendWorkerRestoreResult, WorkerSummary as BackendWorkerSummary,
|
WorkerRestoreResult as BackendWorkerRestoreResult, WorkerSummary as BackendWorkerSummary,
|
||||||
WorkerWorkspaceSummary as BackendWorkerWorkspaceSummary,
|
WorkerWorkspaceSummary as BackendWorkerWorkspaceSummary,
|
||||||
|
|||||||
@@ -26,10 +26,11 @@ pub use backend_runtime::{
|
|||||||
BackendRuntimeListTarget, BackendRuntimeSummary, BackendRuntimeTarget,
|
BackendRuntimeListTarget, BackendRuntimeSummary, BackendRuntimeTarget,
|
||||||
BackendWorkerCapabilitySummary, BackendWorkerImplementationSummary, BackendWorkerLaunchOptions,
|
BackendWorkerCapabilitySummary, BackendWorkerImplementationSummary, BackendWorkerLaunchOptions,
|
||||||
BackendWorkerLaunchProfileCandidate, BackendWorkerLaunchRuntimeOption,
|
BackendWorkerLaunchProfileCandidate, BackendWorkerLaunchRuntimeOption,
|
||||||
BackendWorkerLaunchTarget, BackendWorkerRestoreResponse, BackendWorkerRestoreResult,
|
BackendWorkerLaunchTarget, BackendWorkerOperationState, BackendWorkerRestoreResponse,
|
||||||
BackendWorkerSummary, BackendWorkerWorkspaceSummary, BackendWorkingDirectorySummary,
|
BackendWorkerRestoreResult, BackendWorkerSummary, BackendWorkerWorkspaceSummary,
|
||||||
connect_backend_runtime, create_backend_worker, get_backend_worker_launch_options,
|
BackendWorkingDirectorySummary, connect_backend_runtime, create_backend_worker,
|
||||||
list_backend_stopped_workers, list_backend_workers, restore_backend_worker,
|
get_backend_worker_launch_options, list_backend_stopped_workers, list_backend_workers,
|
||||||
|
restore_backend_worker,
|
||||||
};
|
};
|
||||||
pub use backend_workspace::{
|
pub use backend_workspace::{
|
||||||
BackendWorkspace, BackendWorkspaceCatalogTarget, BackendWorkspaceClientError,
|
BackendWorkspace, BackendWorkspaceCatalogTarget, BackendWorkspaceClientError,
|
||||||
|
|||||||
@@ -3,8 +3,9 @@ use std::io;
|
|||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
|
|
||||||
use client::{
|
use client::{
|
||||||
BackendRuntimeListTarget, BackendWorkerSummary, list_backend_stopped_workers,
|
BackendRuntimeListTarget, BackendWorkerOperationState, BackendWorkerRestoreResponse,
|
||||||
list_backend_workers, restore_backend_worker,
|
BackendWorkerSummary, list_backend_stopped_workers, list_backend_workers,
|
||||||
|
restore_backend_worker,
|
||||||
};
|
};
|
||||||
use crossterm::event::{self, Event as TermEvent, KeyCode, KeyEventKind, KeyModifiers};
|
use crossterm::event::{self, Event as TermEvent, KeyCode, KeyEventKind, KeyModifiers};
|
||||||
use ratatui::Frame;
|
use ratatui::Frame;
|
||||||
@@ -84,17 +85,20 @@ pub(crate) async fn run(
|
|||||||
let restore_target = target
|
let restore_target = target
|
||||||
.runtime_target(selected.runtime_id.clone(), selected.worker_id.clone())
|
.runtime_target(selected.runtime_id.clone(), selected.worker_id.clone())
|
||||||
.map_err(|error| io::Error::other(error.to_string()))?;
|
.map_err(|error| io::Error::other(error.to_string()))?;
|
||||||
restore_backend_worker(&restore_target)
|
let restore = restore_backend_worker(&restore_target)
|
||||||
.await
|
.await
|
||||||
.map_err(|error| {
|
.map_err(|error| {
|
||||||
io::Error::other(format!(
|
io::Error::other(format!(
|
||||||
"failed to restore Backend worker {}/{}: {error}",
|
"failed to restore Backend worker {}/{}: {error}",
|
||||||
selected.runtime_id, selected.worker_id
|
selected.runtime_id, selected.worker_id
|
||||||
))
|
))
|
||||||
})?
|
})?;
|
||||||
.result
|
restored_worker(restore).map_err(|error| {
|
||||||
.worker
|
io::Error::other(format!(
|
||||||
.unwrap_or(selected)
|
"failed to restore Backend worker {}/{}: {error}",
|
||||||
|
selected.runtime_id, selected.worker_id
|
||||||
|
))
|
||||||
|
})?
|
||||||
} else {
|
} else {
|
||||||
selected
|
selected
|
||||||
};
|
};
|
||||||
@@ -105,6 +109,33 @@ pub(crate) async fn run(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn restored_worker(response: BackendWorkerRestoreResponse) -> Result<BackendWorkerSummary, String> {
|
||||||
|
if response.result.state != BackendWorkerOperationState::Accepted {
|
||||||
|
let diagnostics = response
|
||||||
|
.result
|
||||||
|
.diagnostics
|
||||||
|
.iter()
|
||||||
|
.map(|diagnostic| format!("{}: {}", diagnostic.code, diagnostic.message))
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join("; ");
|
||||||
|
let state = match response.result.state {
|
||||||
|
BackendWorkerOperationState::Accepted => unreachable!(),
|
||||||
|
BackendWorkerOperationState::Rejected => "rejected",
|
||||||
|
BackendWorkerOperationState::Unsupported => "unsupported",
|
||||||
|
};
|
||||||
|
return Err(if diagnostics.is_empty() {
|
||||||
|
format!("restore was {state} without a diagnostic")
|
||||||
|
} else {
|
||||||
|
format!("restore was {state}: {diagnostics}")
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
response
|
||||||
|
.result
|
||||||
|
.worker
|
||||||
|
.ok_or_else(|| "restore was accepted without a Worker snapshot".to_string())
|
||||||
|
}
|
||||||
|
|
||||||
fn dedup_workers(workers: &mut Vec<BackendWorkerSummary>) {
|
fn dedup_workers(workers: &mut Vec<BackendWorkerSummary>) {
|
||||||
let mut seen = std::collections::HashSet::new();
|
let mut seen = std::collections::HashSet::new();
|
||||||
workers.retain(|worker| seen.insert((worker.runtime_id.clone(), worker.worker_id.clone())));
|
workers.retain(|worker| seen.insert((worker.runtime_id.clone(), worker.worker_id.clone())));
|
||||||
@@ -405,7 +436,8 @@ fn working_directory_text(worker: &BackendWorkerSummary) -> String {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use client::{
|
use client::{
|
||||||
BackendWorkerCapabilitySummary, BackendWorkerImplementationSummary,
|
BackendDiagnostic, BackendDiagnosticSeverity, BackendWorkerCapabilitySummary,
|
||||||
|
BackendWorkerImplementationSummary, BackendWorkerRestoreResult,
|
||||||
BackendWorkerWorkspaceSummary,
|
BackendWorkerWorkspaceSummary,
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -463,6 +495,67 @@ mod tests {
|
|||||||
text_width(&text[..byte_offset])
|
text_width(&text[..byte_offset])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn restore_response(
|
||||||
|
state: BackendWorkerOperationState,
|
||||||
|
worker: Option<BackendWorkerSummary>,
|
||||||
|
diagnostics: Vec<BackendDiagnostic>,
|
||||||
|
) -> BackendWorkerRestoreResponse {
|
||||||
|
BackendWorkerRestoreResponse {
|
||||||
|
workspace_id: "workspace-a".to_string(),
|
||||||
|
runtime_id: "runtime-a".to_string(),
|
||||||
|
worker_id: "worker-a".to_string(),
|
||||||
|
result: BackendWorkerRestoreResult {
|
||||||
|
state,
|
||||||
|
worker,
|
||||||
|
diagnostics,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rejected_restore_surfaces_diagnostic_instead_of_attaching_selected_worker() {
|
||||||
|
let error = restored_worker(restore_response(
|
||||||
|
BackendWorkerOperationState::Rejected,
|
||||||
|
None,
|
||||||
|
vec![BackendDiagnostic {
|
||||||
|
code: "working_directory_not_found".to_string(),
|
||||||
|
severity: BackendDiagnosticSeverity::Error,
|
||||||
|
message: "working directory was not found".to_string(),
|
||||||
|
}],
|
||||||
|
))
|
||||||
|
.expect_err("rejected restore must not produce a Worker to attach");
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
error,
|
||||||
|
"restore was rejected: working_directory_not_found: working directory was not found"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn accepted_restore_requires_returned_worker_snapshot() {
|
||||||
|
let error = restored_worker(restore_response(
|
||||||
|
BackendWorkerOperationState::Accepted,
|
||||||
|
None,
|
||||||
|
Vec::new(),
|
||||||
|
))
|
||||||
|
.expect_err("accepted restore without a Worker must not attach the stale selection");
|
||||||
|
|
||||||
|
assert_eq!(error, "restore was accepted without a Worker snapshot");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn accepted_restore_returns_authoritative_worker_snapshot() {
|
||||||
|
let worker = worker("runtime-a", "worker-a", Some("builtin:companion"));
|
||||||
|
let restored = restored_worker(restore_response(
|
||||||
|
BackendWorkerOperationState::Accepted,
|
||||||
|
Some(worker.clone()),
|
||||||
|
Vec::new(),
|
||||||
|
))
|
||||||
|
.expect("accepted restore should return its Worker snapshot");
|
||||||
|
|
||||||
|
assert_eq!(restored, worker);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn worker_row_orders_and_simplifies_columns() {
|
fn worker_row_orders_and_simplifies_columns() {
|
||||||
let mut worker = worker("runtime-a", "worker-b", Some("builtin:coder"));
|
let mut worker = worker("runtime-a", "worker-b", Some("builtin:coder"));
|
||||||
|
|||||||
Reference in New Issue
Block a user