fix: CJK browser_search panic and multi-tab tab_close crash (#257, #258)

* Fix UTF-8 char boundary panic in browser_search

The search snippet window sliced body[..start] / body[end..] at byte
offsets derived from char counts and needle.len(), which can land inside
a multi-byte (CJK) character and panic, killing the MCP stdio transport.
Snap start/end to char boundaries before slicing and advance past
whitespace by the actual char width. Closes #257.

* Add setLineDash and friends to the canvas 2D mock

Charting libraries (Highcharts, ECharts) call setLineDash/getLineDash/
ellipse every animation frame. The missing setLineDash threw 'is not a
function' from a timer each tick, spamming the console. Adds the
line-dash methods plus ellipse/roundRect/createConicGradient/
getContextAttributes stubs. Addresses the error spam in #258; the
multi-tab isolate-teardown crash is a separate deeper issue.

* Fix multi-tab tab_close V8 isolate abort

rusty_v8 enters each isolate on creation and requires them dropped in
reverse order of creation. The MCP gave each tab its own isolate and
dropped them in arbitrary (user-chosen) order on tab_close, so closing a
non-newest tab aborted the process (Check failed: heap->isolate() ==
Isolate::TryGetCurrent()). Pages with active timers (Highcharts/ECharts)
made it easy to hit.

Enforce the CDP server's single-live-isolate discipline in the MCP: at
most one tab's isolate is ever live. page_mut calls a new activate() that
suspends every other tab (drops its isolate, keeps its DOM) and resumes
the active one. close_tab drops the closed tab's isolate via suspend_js
while it is still the only entered isolate, and tool_close suspends all
tabs before clearing. Confined to obscura-mcp; the engine, CDP server and
obstacle course are untouched, and peak memory drops to one isolate
instead of N. Closes #258.
This commit is contained in:
SGavrl authored and GitHub committed 2026-06-07 11:44:49 +02:00
1 parent c1e0b0fb5a
commit fb2841b7ab
2 files changed
+59 -9

No files matched your search

+50 -9
View File
@@ -102,6 +102,7 @@ impl BrowserState {
self.active_tab = Some(id);
}
let id = self.active_tab.as_ref().unwrap().clone();
self.activate(&id);
self.tabs.get_mut(&id).expect("active tab must exist")
}
@@ -114,6 +115,34 @@ impl BrowserState {
id
}
/// Enforce the single-live-isolate invariant. rusty_v8 enters each V8
/// isolate on creation and requires isolates be dropped in reverse order of
/// creation, so keeping more than one tab's isolate live at once and then
/// dropping a non-newest one aborts the whole process (#258). Suspend every
/// other tab (drops its isolate, keeps its DOM in self.dom) and make the
/// active tab the only live isolate, mirroring the CDP server's
/// Dispatcher::get_session_page_mut.
fn activate(&mut self, tab_id: &str) {
for (id, page) in self.tabs.iter_mut() {
if id.as_str() != tab_id && page.has_js() {
page.suspend_js();
}
}
if let Some(page) = self.tabs.get_mut(tab_id) {
page.resume_js();
}
}
/// Close a tab without breaking the LIFO isolate-drop rule. suspend_js drops
/// this tab's isolate (if it is the live one) while it is still the only
/// entered isolate, so the following remove disposes no isolate (#258).
fn close_tab(&mut self, tab_id: &str) -> bool {
if let Some(page) = self.tabs.get_mut(tab_id) {
page.suspend_js();
}
self.tabs.remove(tab_id).is_some()
}
/// Resolve `ref=eN` to a CSS selector that uniquely targets the
/// element. Snapshot writes `data-obscura-ref="eN"` onto every
/// interactable, so the attribute survives across calls as long as
@@ -903,6 +932,12 @@ fn tool_console_messages(state: &BrowserState) -> Result<String, String> {
}
fn tool_close(state: &mut BrowserState) -> Result<String, String> {
// Drop the one live isolate (if any) via suspend_js before clearing, so the
// map drop disposes no isolate and the LIFO rule holds regardless of the
// BTreeMap's ascending drop order (#258).
for page in state.tabs.values_mut() {
page.suspend_js();
}
state.tabs.clear();
state.active_tab = None;
state.console_messages.clear();
@@ -1513,7 +1548,7 @@ fn tool_tab_close(args: &Value, state: &mut BrowserState) -> Result<String, Stri
.map(String::from)
.or_else(|| state.active_tab.clone())
.ok_or("No tab to close")?;
if state.tabs.remove(&tab_id).is_none() {
if !state.close_tab(&tab_id) {
return Err(format!("No such tab: {tab_id}"));
}
if state.active_tab.as_deref() == Some(&tab_id) {
@@ -1608,14 +1643,20 @@ fn tool_search(args: &Value, state: &mut BrowserState) -> Result<String, String>
let mut idx = 0;
while let Some(pos) = haystack[idx..].find(&needle) {
let abs = idx + pos;
let start = abs.saturating_sub(context);
let end = (abs + needle.len() + context).min(body.len());
// body and haystack share byte offsets even when lowercased (only ASCII fold).
// For safety on non-ASCII, fall back to char-boundary trimming.
let start = body[..start].rfind(|c: char| c.is_whitespace())
.map(|i| i + 1).unwrap_or(start);
let end = body[end..].find(|c: char| c.is_whitespace())
.map(|i| end + i).unwrap_or(end);
let mut start = abs.saturating_sub(context);
let mut end = (abs + needle.len() + context).min(body.len());
// start/end are byte offsets derived from char counts and needle.len(),
// so they can land inside a multi-byte (CJK) character. Snap to char
// boundaries before slicing or body[..start] panics (#257).
while start > 0 && !body.is_char_boundary(start) { start -= 1; }
while end < body.len() && !body.is_char_boundary(end) { end += 1; }
// Trim inward to the nearest whitespace so snippets start/end on words.
if let Some(i) = body[..start].rfind(|c: char| c.is_whitespace()) {
start = i + body[i..].chars().next().map_or(1, char::len_utf8);
}
if let Some(i) = body[end..].find(|c: char| c.is_whitespace()) {
end += i;
}
let snippet = body.get(start..end).unwrap_or("").trim().replace('\n', " ");
out.push(json!({
"offset": abs,