SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(editor): degrade instead of panicking on host failure paths

Browse source 
Poison-tolerant locking in the ACP transport (a panicking reader task
used to brick every later call), checked thread spawns on UI paths
(thread creation fails under fd/memory pressure and took the editor
down with unsaved work), MCP tools return typed errors for malformed
LLM-controlled args, bounded name-suffix loops, and event-driven chat
stream polling instead of a 20ms sleep loop.
This commit is contained in:
Kayshen-X 2026-07-26 11:23:43 +08:00
parent 5e4c62ef14
commit 839f474a60
21 changed files with 252 additions and 89 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
crates/op-acp/src/client.rs
View file

@ -93,7 +93,11 @@ impl AcpConnection {
// Connection closed: fail every in-flight request now so
// callers get `Closed` immediately instead of stalling
// until the request timeout.
let waiters: Vec<_> = pending.lock().unwrap().drain().collect();
let waiters: Vec<_> = pending
.lock()
.unwrap_or_else(|p| p.into_inner())
.drain()
.collect();
for (_, waiter) in waiters {
let _ = waiter.send(Err(AcpError::Closed));
}
@ -316,7 +320,11 @@ async fn connect_remote(config: &AcpAgentConfig) -> Result<AcpConnection, AcpErr
}
}
// Socket closed: fail in-flight requests immediately.
let waiters: Vec<_> = pending.lock().unwrap().drain().collect();
let waiters: Vec<_> = pending
.lock()
.unwrap_or_else(|p| p.into_inner())
.drain()
.collect();
for (_, waiter) in waiters {
let _ = waiter.send(Err(AcpError::Closed));
}

21
crates/op-acp/src/jsonrpc.rs
View file

@ -63,12 +63,18 @@ impl JsonRpcEngine {
) -> Result<Value, AcpError> {
let id = self.next_id.fetch_add(1, Ordering::Relaxed);
let (tx, rx) = oneshot::channel();
self.pending.lock().unwrap().insert(id, tx);
self.pending
.lock()
.unwrap_or_else(|p| p.into_inner())
.insert(id, tx);
let req = JsonRpcRequest::new(id, method, params);
let frame = serde_json::to_value(&req).map_err(|e| AcpError::Protocol(e.to_string()))?;
if self.out_tx.send(frame).is_err() {
self.pending.lock().unwrap().remove(&id);
self.pending
.lock()
.unwrap_or_else(|p| p.into_inner())
.remove(&id);
return Err(AcpError::Closed);
}
@ -77,7 +83,10 @@ impl JsonRpcEngine {
// The reader task dropped the sender — connection died.
Ok(Err(_)) => Err(AcpError::Closed),
Err(_) => {
self.pending.lock().unwrap().remove(&id);
self.pending
.lock()
.unwrap_or_else(|p| p.into_inner())
.remove(&id);
Err(AcpError::Transport(format!("request '{method}' timed out")))
}
}
@ -117,7 +126,11 @@ pub fn dispatch_inbound(
) {
match classify_inbound(&value) {
Inbound::Response { id, result, error } => {
if let Some(tx) = pending.lock().unwrap().remove(&id) {
if let Some(tx) = pending
.lock()
.unwrap_or_else(|p| p.into_inner())
.remove(&id)
{
let resolved = match error {
Some(e) => Err(AcpError::Rpc {
code: e.code,

27
crates/op-host-desktop/src/chat_session_launch.rs
View file

@ -334,7 +334,7 @@ fn launch_direct_modify_turn(
*current_design = None;
super::finalize_design_session_if_needed(host, current_chat, "teardown-backstop");
*current_chat = Some(ChatSession::from_channels(chat_rx, Some(tool_rx)));
thread::Builder::new()
let spawned = thread::Builder::new()
.name("op-chat-modify".into())
.spawn(move || {
op_host_services::chat_intent::run_modify_turn(
@ -344,8 +344,14 @@ fn launch_direct_modify_turn(
&executor,
target_frame_ids,
);
})
.expect("spawn op-chat-modify thread");
});
if let Err(err) = spawned {
// Worker never started — un-park the session and fall through
// to the honest-error path instead of crashing the UI thread.
eprintln!("openpencil-desktop: spawn op-chat-modify thread failed: {err}");
*current_chat = None;
return false;
}
true
}
@ -465,12 +471,21 @@ fn launch_cli_standard_turn(
abort: design_abort,
model,
};
thread::Builder::new()
let spawned = thread::Builder::new()
.name("op-chat-intent".into())
.spawn(move || {
op_host_services::chat_intent::run_cli_turn(plan, chat_tx, executor, delta_tx, cmd_tx);
})
.expect("spawn op-chat-intent thread");
});
if let Err(err) = spawned {
// Worker never started — un-park both sessions, end the epoch
// begun above, and fall to the honest-error path instead of
// crashing the UI thread.
eprintln!("openpencil-desktop: spawn op-chat-intent thread failed: {err}");
*current_chat = None;
*current_design = None;
op_editor_core::agent_indicators::end_if_epoch(indicator_epoch);
return false;
}
true
}

28
crates/op-host-desktop/src/figma_import_session.rs
View file

@ -179,7 +179,8 @@ pub(crate) fn spawn_approved(
let path_for_thread = path.clone();
let worker_cancellation = cancellation.clone();
thread::Builder::new()
let worker_tx = tx.clone();
let spawned = thread::Builder::new()
.name("op-figma-import".into())
.spawn(move || {
let Some(_permit) = worker_cancellation.worker_permit() else {
@ -189,10 +190,15 @@ pub(crate) fn spawn_approved(
// Recv side may be gone if the user closed the app
// mid-parse; tolerate the SendError silently.
if !worker_cancellation.is_cancelled() {
let _ = tx.send(result);
let _ = worker_tx.send(result);
}
})
.expect("spawn op-figma-import worker");
});
if let Err(err) = spawned {
// Deliver the failure through the normal pump path (error
// dialog + overlay-flag clear) instead of crashing the app.
eprintln!("[import-figma] failed to spawn worker: {err}");
let _ = tx.send(Err(format!("import worker failed to start: {err}")));
}
FigmaImportSession {
path,
@ -288,7 +294,8 @@ fn conversion_receiver(
output_mode: ImportOutputMode,
) -> Receiver<PersistResult> {
let (tx, rx) = mpsc::channel();
thread::Builder::new()
let worker_tx = tx.clone();
let spawned = thread::Builder::new()
.name("op-figma-convert".into())
.spawn(move || {
let Some(_permit) = cancellation.worker_permit() else {
@ -298,10 +305,15 @@ fn conversion_receiver(
persist_import_next_to_source(prepared, &source_path, output_mode, &cancellation)
});
if !cancellation.is_cancelled() {
let _ = tx.send(result);
let _ = worker_tx.send(result);
}
})
.expect("spawn op-figma-convert worker");
});
if let Err(err) = spawned {
// Deliver the failure through the normal pump path (error
// dialog + overlay-flag clear) instead of crashing the app.
eprintln!("[import-figma] failed to spawn convert worker: {err}");
let _ = tx.send(Err(format!("convert worker failed to start: {err}")));
}
rx
}

7
crates/op-host-desktop/src/git_host.rs
View file

@ -913,12 +913,7 @@ impl DesktopApp {
self.git_clone_job = None;
return false;
}
let Some(result) = self
.git_clone_job
.as_mut()
.expect("clone job present (checked above)")
.poll()
else {
let Some(result) = self.git_clone_job.as_mut().and_then(|job| job.poll()) else {
return false;
};
self.git_clone_job = None;

14
crates/op-host-desktop/src/html_import_session.rs
View file

@ -42,13 +42,19 @@ pub fn spawn(host: &mut WidgetHostNative, path: PathBuf) -> HtmlImportSession {
ui.figma_import_in_progress = true;
let path_for_thread = path.clone();
thread::Builder::new()
let worker_tx = tx.clone();
let spawned = thread::Builder::new()
.name("op-html-import".into())
.spawn(move || {
let result = parse_path(&path_for_thread);
let _ = tx.send(result);
})
.expect("spawn op-html-import worker");
let _ = worker_tx.send(result);
});
if let Err(err) = spawned {
// Deliver the failure through the normal pump path (error
// dialog + overlay-flag clear) instead of crashing the app.
eprintln!("[import-html] failed to spawn worker: {err}");
let _ = tx.send(Err(format!("import worker failed to start: {err}")));
}
HtmlImportSession { path, rx }
}

15
crates/op-host-desktop/src/image_decode_host.rs
View file

@ -48,19 +48,26 @@ impl ImageDecodeHost {
let (result_tx, result_rx) = mpsc::channel::<DecodeResult>();
let job_rx = Arc::new(Mutex::new(job_rx));
let wake_proxy = Arc::new(Mutex::new(None::<EventLoopProxy<DesktopEvent>>));
let workers = (0..WORKER_COUNT)
.map(|index| {
let workers: Vec<JoinHandle<()>> = (0..WORKER_COUNT)
.filter_map(|index| {
let job_rx = Arc::clone(&job_rx);
let result_tx = result_tx.clone();
let wake_proxy = Arc::clone(&wake_proxy);
std::thread::Builder::new()
.name(format!("op-image-decode-{index}"))
.spawn(move || decode_worker(job_rx, result_tx, wake_proxy))
.expect("spawn image decode worker")
.map_err(|err| {
eprintln!("openpencil-desktop: spawn image decode worker failed: {err}");
})
.ok()
})
.collect();
// With no worker to drain the queue, disable submission entirely so
// `pump` retires pending decodes instead of parking them forever
// (images stay at thumbnail quality — degraded, not stuck).
let job_tx = (!workers.is_empty()).then_some(job_tx);
Self {
job_tx: Some(job_tx),
job_tx,
result_rx,
workers,
in_flight: 0,

4
crates/op-host-desktop/src/image_decode_stats.rs
View file

@ -109,6 +109,10 @@ impl ImageDecodeStats {
let reporter = std::thread::Builder::new()
.name("op-image-decode-stats".into())
.spawn(move || report_loop(reporter_shared, stop_rx))
.map_err(|err| {
// Opt-in telemetry only — log and run without stats.
eprintln!("openpencil-desktop: spawn image decode stats reporter failed: {err}");
})
.ok()?;
Some(Self {
tally: InstallTally::default(),

26
crates/op-host-desktop/src/save_session.rs
View file

@ -229,7 +229,7 @@ impl SaveSession {
Err("background save worker stopped before reporting a result".to_string())
}
};
Some(self.finish_running(result))
self.finish_running(result)
}
/// Block only for close/reload confirmation. Ordinary Save never calls
@ -239,7 +239,7 @@ impl SaveSession {
let result = self.running.as_ref()?.rx.recv().unwrap_or_else(|_| {
Err("background save worker stopped before reporting a result".to_string())
});
Some(self.finish_running(result))
self.finish_running(result)
}
fn matches_pending(
@ -300,7 +300,7 @@ impl SaveSession {
let worker_path = path.clone();
let worker_snapshot = Arc::clone(&snapshot);
let (tx, rx) = mpsc::channel();
std::thread::Builder::new()
let spawned = std::thread::Builder::new()
.name("op-document-save".to_string())
.spawn(move || {
let started = Instant::now();
@ -334,8 +334,13 @@ impl SaveSession {
if let Some(proxy) = wake {
let _ = proxy.send_event(DesktopEvent::SaveReady);
}
})
.expect("spawn document save worker");
});
if let Err(err) = spawned {
// The worker never started, so `tx` is gone and the rx below is
// disconnected — `poll`/`wait_next` surface that as a failed
// completion instead of the UI thread crashing mid-save.
eprintln!("[save] failed to spawn save worker: {err}");
}
self.running = Some(RunningSave {
snapshot,
path,
@ -347,8 +352,13 @@ impl SaveSession {
});
}
fn finish_running(&mut self, result: Result<(), String>) -> SaveCompletion {
let job = self.running.take().expect("running save exists");
fn finish_running(&mut self, result: Result<(), String>) -> Option<SaveCompletion> {
let Some(job) = self.running.take() else {
// Both callers check `running` before calling; an ordering bug
// here must not panic mid-save. Drop the stray result instead.
eprintln!("[save] finish_running called with no running job; result dropped");
return None;
};
let completion = SaveCompletion {
path: job.path,
set_current_path: job.set_current_path,
@ -360,7 +370,7 @@ impl SaveSession {
if let Some(next) = self.queued.take() {
self.start(next);
}
completion
Some(completion)
}
}

12
crates/op-host-desktop/src/sub_agent_session.rs
View file

@ -111,14 +111,17 @@ pub(crate) fn stash_pending_spawn(specs: Vec<SpawnSpec>, nested: bool) -> bool {
if nested {
return false;
}
*PENDING_SPAWN.lock().unwrap() = Some(specs);
*PENDING_SPAWN.lock().unwrap_or_else(|p| p.into_inner()) = Some(specs);
true
}
/// Take the stashed specs, if any. Called by `app_handler` each frame
/// after the parent pump.
pub(crate) fn take_pending_spawn() -> Option<Vec<SpawnSpec>> {
PENDING_SPAWN.lock().unwrap().take()
PENDING_SPAWN
.lock()
.unwrap_or_else(|p| p.into_inner())
.take()
}
// ---------------------------------------------------------------------------
@ -266,7 +269,10 @@ pub(crate) fn abort_all(subs: &mut Vec<SubAgentSession>, active: &mut usize) {
// A spawn request can be stashed by the parent one frame before the host
// launches it. Stop/New Chat must cancel that queued batch too, otherwise
// it starts after the user has already ended the turn.
PENDING_SPAWN.lock().unwrap().take();
PENDING_SPAWN
.lock()
.unwrap_or_else(|p| p.into_inner())
.take();
SUB_AGENT_ACTIVE.store(false, Ordering::SeqCst);
subs.clear();
*active = 0;

27
crates/op-host-native/src/widget_host.rs
View file

@ -1419,14 +1419,29 @@ impl WidgetHostNative {
// clipboard paste decode) is dropped instead of applied here.
self.document_epoch = self.document_epoch.wrapping_add(1);
let old_scene = std::mem::take(&mut self.layout_scene);
std::thread::Builder::new()
// Take-once slot so the drop work runs exactly once: on the worker
// when the spawn succeeds, inline (blocking the UI thread briefly)
// when thread creation fails under FD/memory pressure.
let work: Box<dyn FnOnce() + Send> = Box::new(move || {
drop(old_state);
drop(old_scene);
after_drop();
});
let work = std::sync::Arc::new(std::sync::Mutex::new(Some(work)));
let worker_slot = std::sync::Arc::clone(&work);
let spawned = std::thread::Builder::new()
.name("op-import-drop".into())
.spawn(move || {
drop(old_state);
drop(old_scene);
after_drop();
})
.expect("spawn op-import-drop worker");
if let Some(f) = worker_slot.lock().unwrap_or_else(|p| p.into_inner()).take() {
f();
}
});
if let Err(err) = spawned {
eprintln!("[widget-host] failed to spawn op-import-drop worker: {err}");
if let Some(f) = work.lock().unwrap_or_else(|p| p.into_inner()).take() {
f();
}
}
// The imported document restarts at revision 0 / page 0, so its
// LayerPanel row-model-cache key aliases the replaced document's.

8
crates/op-host-native/src/widget_host/variables_panel_press.rs
View file

@ -533,11 +533,15 @@ impl WidgetHostNative {
}
fn unique_numbered(base: &str, exists: impl Fn(&str) -> bool) -> String {
for idx in 1.. {
// Bounded scan — 10k collisions on one base never happens in practice,
// but a pathological document must not spin the UI thread forever.
for idx in 1u32..=10_000 {
let candidate = format!("{base}-{idx}");
if !exists(&candidate) {
return candidate;
}
}
unreachable!()
// Deterministic fallback past the bound; a duplicate name is cosmetic
// while an unbounded loop would hang the editor.
format!("{base}-10001")
}

10
crates/op-host-services/src/chat_intent.rs
View file

@ -245,7 +245,7 @@ fn classify_intent_llm_with_timeout(
// after a timeout makes its sends fail and the drain unwind.
let iter = provider.send(req);
let (tx, rx) = mpsc::channel::<ChatDelta>();
std::thread::Builder::new()
let spawned = std::thread::Builder::new()
.name("op-chat-classify".into())
.spawn(move || {
for delta in iter {
@ -253,8 +253,12 @@ fn classify_intent_llm_with_timeout(
return;
}
}
})
.expect("spawn op-chat-classify thread");
});
if let Err(err) = spawned {
// TS: any classification failure falls back to { intent: 'new' }.
eprintln!("[chat-intent] failed to spawn classify drain thread: {err}");
return DesignIntent::New;
}
let deadline = Instant::now() + timeout;
let mut out = String::new();

27
crates/op-host-services/src/chat_runtime.rs
View file

@ -22,6 +22,7 @@
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, OnceLock};
use std::task::{Context, Poll, Wake, Waker};
use std::time::Duration;
use agent::abort::AbortController;
@ -278,6 +279,13 @@ impl<T> Iterator for BlockingRecvIter<T> {
if self.cancel.is_none() {
return self.rx.blocking_recv();
}
// Timed blocking bridge: tokio's mpsc has no blocking recv with a
// timeout, so poll the receiver with a waker that unparks this
// thread. A message (or channel close) ends the park immediately;
// the 20 ms cap keeps the cancel flag observed even when the
// provider is silent.
let waker = Waker::from(Arc::new(ThreadUnparker(std::thread::current())));
let mut cx = Context::from_waker(&waker);
loop {
if self
.cancel
@ -289,17 +297,24 @@ impl<T> Iterator for BlockingRecvIter<T> {
}
return None;
}
match self.rx.try_recv() {
Ok(value) => return Some(value),
Err(mpsc::error::TryRecvError::Disconnected) => return None,
Err(mpsc::error::TryRecvError::Empty) => {
std::thread::sleep(Duration::from_millis(20));
}
match self.rx.poll_recv(&mut cx) {
Poll::Ready(Some(value)) => return Some(value),
Poll::Ready(None) => return None,
Poll::Pending => std::thread::park_timeout(Duration::from_millis(20)),
}
}
}
}
/// Wakes [`BlockingRecvIter::next`]'s park as soon as a message lands.
struct ThreadUnparker(std::thread::Thread);
impl Wake for ThreadUnparker {
fn wake(self: Arc<Self>) {
self.0.unpark();
}
}
impl<T> Drop for BlockingRecvIter<T> {
fn drop(&mut self) {
if let Some(task) = self.task.take() {

29
crates/op-host-services/src/design_session.rs
View file

@ -53,7 +53,8 @@ pub fn start<L: LlmClient + Send + 'static>(
let abort = AbortFlag::new();
let worker_abort = abort.clone();
thread::Builder::new()
let failure_tx = delta_tx.clone();
if let Err(err) = thread::Builder::new()
.name("op-design-turn".into())
.spawn(move || {
run_design_worker(
@ -67,7 +68,17 @@ pub fn start<L: LlmClient + Send + 'static>(
vision_provider,
)
})
.expect("spawn op-design-turn thread");
{
// Thread creation can fail under FD/memory pressure; surface a failed
// turn instead of panicking the UI thread.
eprintln!("[design-session] failed to spawn op-design-turn thread: {err}");
op_editor_core::agent_indicators::end_if_epoch(indicator_epoch);
let _ = failure_tx.send(DesignDelta::Done(Err(
op_orchestrator::OrchestratorError::Internal(format!(
"failed to spawn design worker thread: {err}"
)),
)));
}
DesignSession::from_channels_with_epoch_and_abort(delta_rx, cmd_rx, indicator_epoch, abort)
}
@ -172,7 +183,8 @@ pub fn start_subtask_retry<L: LlmClient + Send + 'static>(
let abort = AbortFlag::new();
let worker_abort = abort.clone();
thread::Builder::new()
let failure_tx = delta_tx.clone();
if let Err(err) = thread::Builder::new()
.name("op-subtask-retry".into())
.spawn(move || {
run_subtask_retry_worker(
@ -186,7 +198,16 @@ pub fn start_subtask_retry<L: LlmClient + Send + 'static>(
worker_abort,
)
})
.expect("spawn op-subtask-retry thread");
{
// Same degrade-instead-of-panic path as `start` above.
eprintln!("[design-session] failed to spawn op-subtask-retry thread: {err}");
op_editor_core::agent_indicators::end_if_epoch(indicator_epoch);
let _ = failure_tx.send(DesignDelta::Done(Err(
op_orchestrator::OrchestratorError::Internal(format!(
"failed to spawn subtask-retry thread: {err}"
)),
)));
}
DesignSession::from_channels_with_epoch_and_abort(delta_rx, cmd_rx, indicator_epoch, abort)
}

8
crates/op-host-web/src/widget_host/variables_panel_press.rs
View file

@ -530,11 +530,15 @@ impl WidgetHost {
}
pub(in crate::widget_host) fn unique_numbered(base: &str, exists: impl Fn(&str) -> bool) -> String {
for idx in 1.. {
// Bounded scan — 10k collisions on one base never happens in practice,
// but a pathological document must not spin the UI thread forever.
for idx in 1u32..=10_000 {
let candidate = format!("{base}-{idx}");
if !exists(&candidate) {
return candidate;
}
}
unreachable!()
// Deterministic fallback past the bound; a duplicate name is cosmetic
// while an unbounded loop would hang the editor.
format!("{base}-10001")
}

23
crates/op-mcp/src/batch_design.rs
View file

@ -134,7 +134,12 @@ pub(crate) fn expand_script_arg(
Ok(BatchInputKind::Operations | BatchInputKind::NodesJson) => return None,
Err((code, message)) => return Some(Err(ToolOutcome::Err(code, message))),
}
let script = args.get("script").expect("selected script exists");
let Some(script) = args.get("script") else {
return Some(Err(ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"script argument is missing".into(),
)));
};
#[cfg(feature = "script")]
{
let program = match crate::script_runner::run_script_to_program(script) {
@ -172,7 +177,12 @@ pub(crate) fn dispatch_batch_design(
};
let page_id = optional_page_id(args);
if input == BatchInputKind::Operations {
let operations = args.get("operations").expect("selected operations exists");
let Some(operations) = args.get("operations") else {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"operations argument is missing".into(),
);
};
if let Some(phase) = phase.filter(|_| is_direct_image_operation(operations)) {
return ToolOutcome::Err(
ToolErrorCode::InvalidArgument,
@ -223,9 +233,12 @@ pub(crate) fn dispatch_batch_design(
Err(e) => ToolOutcome::Err(ToolErrorCode::InvalidArgument, e),
};
}
let raw = args
.get("nodes_json")
.expect("selected nodes_json exists after script expansion");
let Some(raw) = args.get("nodes_json") else {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"nodes_json argument is missing".into(),
);
};
match parse_batch_items(raw) {
Ok(items) if items.is_empty() => ToolOutcome::Err(
ToolErrorCode::InvalidArgument,

9
crates/op-mcp/src/batch_design_result.rs
View file

@ -26,7 +26,7 @@ use super::batch_design::{
use super::batch_direct_ops::is_direct_image_operation;
use super::batch_page::optional_page_id;
use super::read_nodes::{page_nodes_snapshots, PageNodes};
use super::{EditorCommand, McpTool, ToolOutcome};
use super::{EditorCommand, McpTool, ToolErrorCode, ToolOutcome};
/// `batch_design` — insert a node forest in one shot. Holds a document snapshot
/// (page node sets + the doc-wide id state) so the operations path can report
@ -68,7 +68,12 @@ impl McpTool for BatchDesign {
if input == BatchInputKind::NodesJson {
return dispatch_batch_design(args, None);
}
let operations = args.get("operations").expect("selected operations exists");
let Some(operations) = args.get("operations") else {
return ToolOutcome::Err(
ToolErrorCode::MissingArgument,
"operations argument is missing".into(),
);
};
match parse_operations(operations) {
Ok(ParsedOperations::Insert {
parent_id,

5
crates/op-mcp/src/codegen_plan_store_assembly.rs
View file

@ -61,8 +61,9 @@ pub(crate) fn assemble_plan(plan_id: &str, _framework: &str) -> Result<Value, St
.is_some_and(|code| !code.trim().is_empty());
let usable = matches!(status, "done" | "degraded") && has_code;
if usable {
let result = result.expect("usable result exists");
// `usable` implies `has_code`, which implies `result` is Some —
// bind it in the match so the guard is structural, not asserted.
if let (true, Some(result)) = (usable, result) {
chunks_out.push(result.clone());
contracts.push(result.get("contract").cloned().unwrap_or(Value::Null));
} else {

8
crates/op-mcp/src/insert_node_data.rs
View file

@ -58,7 +58,13 @@ pub(super) fn ts_data_node(
let mut nodes = vec![node];
let mut _notes = Vec::new();
super::batch_design::promote_in_slice(&mut nodes, &mut _notes);
Ok(Some(nodes.into_iter().next().expect("single node")))
match nodes.into_iter().next() {
Some(node) => Ok(Some(node)),
None => Err(ToolOutcome::Err(
ToolErrorCode::Internal,
"data node normalization produced no node".into(),
)),
}
}
fn should_preserve_ts_data_node(obj: &serde_json::Map<String, Value>) -> bool {

21
crates/op-mcp/src/write_tools.rs
View file

@ -324,15 +324,18 @@ impl McpTool for UpdateNode {
Ok(v) => v,
Err(e) => return e,
};
let x = parse_opt_i32(&patch_args, "x");
let y = parse_opt_i32(&patch_args, "y");
let width = parse_opt_i32(&patch_args, "width");
let height = parse_opt_i32(&patch_args, "height");
for (lab, v) in [("x", &x), ("y", &y), ("width", &width), ("height", &height)] {
if let Err(e) = v {
return ToolOutcome::Err(ToolErrorCode::InvalidArgument, format!("{lab}: {e}"));
// Parse each dimension, rejecting the first invalid one; collecting
// the validated values here keeps the bindings below unwrap-free.
let mut dims = [None, None, None, None];
for (slot, key) in dims.iter_mut().zip(["x", "y", "width", "height"]) {
match parse_opt_i32(&patch_args, key) {
Ok(v) => *slot = v,
Err(e) => {
return ToolOutcome::Err(ToolErrorCode::InvalidArgument, format!("{key}: {e}"))
}
}
}
let [x, y, width, height] = dims;
let name = patch_args.get("name").cloned();
let fill_hex = match patch_args.get("fill_hex") {
None => None,
@ -344,10 +347,6 @@ impl McpTool for UpdateNode {
}
Some(s) => Some(s.clone()),
};
let x = x.unwrap();
let y = y.unwrap();
let width = width.unwrap();
let height = height.unwrap();
let page_id = args
.get("pageId")
.or_else(|| args.get("page_id"))