From 23ab4ce68fded7c99fe618aec9b345ad9fd6a311 Mon Sep 17 00:00:00 2001 From: leeguooooo Date: Sat, 13 Jun 2026 23:27:17 +0900 Subject: [PATCH] fix(relay): don't hijack a user tab on open; surface keydown/keyup + canvas hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dogfooding a canvas game over the extension relay surfaced three issues: 1. (serious) A fresh relay session's first `open` navigated one of the USER's existing tabs instead of opening its own — in testing it replaced a half-filled form with the target site. On connect the daemon passively attaches to the user's tabs and pinned one as active; navigate() then drove it. Now: on the relay (agent_group set), if the active tab isn't one this session created, navigate() opens its own tab in the session's group first. Off the relay (a browser we launched) reusing the active tab stays correct. Pure helper active_index_is_owned() + regression tests. 2. (discoverability) `keydown ` / `keyup ` (hold-to-move, essential for games/shortcuts) already existed as commands+daemon handlers but were absent from --help and the skill, so they were undiscoverable. Documented in --help, the core skill, and the canvas-app hint. 3. (UX) Canvas/WebGL pages expose almost no a11y tree, so `snapshot` is empty and agents get stuck hunting refs. snapshot now detects a viewport-dominating canvas with a sparse tree and prints a hint pointing at the screenshot + coordinate-click + keydown/keyup path. Verified live over the relay: `open` now lands the game in its own new tab with the user's tabs (incl. the Rakuten recovery form) untouched; the canvas hint fires on the game page; `close` cleans up only the session's own tab. --- cli/src/native/actions.rs | 27 +++++++++++++- cli/src/native/browser.rs | 74 +++++++++++++++++++++++++++++++++++++++ cli/src/output.rs | 8 +++++ skill-data/core/SKILL.md | 22 +++++++++++- 4 files changed, 129 insertions(+), 2 deletions(-) diff --git a/cli/src/native/actions.rs b/cli/src/native/actions.rs index f675946..646cc46 100644 --- a/cli/src/native/actions.rs +++ b/cli/src/native/actions.rs @@ -2888,7 +2888,32 @@ async fn handle_snapshot(cmd: &Value, state: &mut DaemonState) -> Result and expose almost no accessibility tree, so `snapshot` comes back + // near-empty and agents get stuck looking for refs that will never exist + // (dogfood: the Dead Cell game). When the tree is sparse but a canvas + // dominates the viewport, tell them to switch to the screenshot-driven path. + if ref_count < 3 { + let canvas_js = + "(() => { const c = document.querySelector('canvas'); if (!c) return false; \ + const r = c.getBoundingClientRect(); \ + return r.width * r.height > innerWidth * innerHeight * 0.5; })()"; + if let Ok(v) = mgr.evaluate(canvas_js, None).await { + if v.as_bool() == Some(true) { + out["note"] = json!( + "This page renders to a (game / WebGL / editor) and exposes almost no \ + accessibility tree — refs won't help. Use `screenshot` to see it, coordinate \ + `click ` to interact, and `keydown`/`keyup`/`press` for keyboard \ + (hold-to-move: `keydown d` … `keyup d`)." + ); + } + } + } + + Ok(out) } /// Resolve a (possibly relative) saved-file path to an absolute one so the CLI diff --git a/cli/src/native/browser.rs b/cli/src/native/browser.rs index 65c3c12..d697220 100644 --- a/cli/src/native/browser.rs +++ b/cli/src/native/browser.rs @@ -166,6 +166,25 @@ fn resolve_active_index( active_page_index } +/// Whether the resolved active page is a tab the session created (its target_id +/// is in `created_targets`). Pure core of [`BrowserManager::active_is_session_owned`] +/// so the relay no-hijack rule is unit-testable without a live browser. +fn active_index_is_owned( + pages: &[PageInfo], + active_target_id: Option<&str>, + active_page_index: usize, + created_targets: &HashSet, +) -> bool { + pages + .get(resolve_active_index( + pages, + active_target_id, + active_page_index, + )) + .map(|p| created_targets.contains(&p.target_id)) + .unwrap_or(false) +} + /// Converts common error messages into AI-friendly, actionable descriptions. pub fn to_ai_friendly_error(error: &str) -> String { let lower = error.to_lowercase(); @@ -799,6 +818,20 @@ impl BrowserManager { ) } + /// Whether the resolved active page is a tab THIS session created (via + /// `Target.createTarget` — `tab new`, `ensure_page`, or the first `open`). + /// On the shared real browser a fresh session also passively attaches to the + /// user's existing tabs; those are NOT owned, and navigating one would + /// clobber the user's page. Used to gate `navigate` on the relay. + fn active_is_session_owned(&self) -> bool { + active_index_is_owned( + &self.pages, + self.active_target_id.as_deref(), + self.active_page_index, + &self.created_targets, + ) + } + /// Pin the current active page by target_id so later commands stick to it. /// Call after any explicit open / tab new / tab switch. fn pin_active_target(&mut self) { @@ -816,6 +849,18 @@ impl BrowserManager { } pub async fn navigate(&mut self, url: &str, wait_until: WaitUntil) -> Result { + // On the shared real browser (extension relay), a fresh session only + // passively attached to the user's existing tabs — it doesn't own any. The + // pre-fix code made one of those the active tab, so the first `open` then + // navigated (clobbered) the user's page: in dogfooding an `open` replaced a + // half-filled form with the target site. If the active tab isn't one we + // created, open our own tab in this session's group and navigate THAT, so + // the user's (and other sessions') tabs are never hijacked. Off the relay + // (a browser we launched) reusing the active tab is correct, so this is + // gated on `agent_group()`. + if self.agent_group().is_some() && !self.active_is_session_owned() { + self.tab_new(None, None).await?; + } let session_id = self.active_session_id()?.to_string(); let mut lifecycle_rx = self.client.subscribe(); @@ -2431,6 +2476,35 @@ mod tests { assert_eq!(resolve_active_index(&pages, Some("CLOSED"), 1), 1); } + // --- issue: `open` must not hijack a user's tab on the relay (dogfood) --- + + #[test] + fn active_not_owned_when_only_user_tabs_discovered() { + // A fresh relay session passively attached to the user's tabs but created + // none — so navigate must NOT reuse the active tab (it'd clobber the + // user's page); it has to open its own first. + let pages = vec![page("USER_A"), page("USER_B")]; + let created = HashSet::new(); + assert!(!active_index_is_owned(&pages, Some("USER_A"), 0, &created)); + } + + #[test] + fn active_owned_when_session_created_the_tab() { + let pages = vec![page("USER_A"), page("OURS")]; + let mut created = HashSet::new(); + created.insert("OURS".to_string()); + // Active pinned to the tab we created → safe to navigate it. + assert!(active_index_is_owned(&pages, Some("OURS"), 1, &created)); + // But pinned to the user's tab → not owned, even though we own another. + assert!(!active_index_is_owned(&pages, Some("USER_A"), 0, &created)); + } + + #[test] + fn active_not_owned_when_no_pages() { + let created = HashSet::new(); + assert!(!active_index_is_owned(&[], None, 0, &created)); + } + #[test] fn resolve_active_index_pin_survives_passive_background_tab() { // A foreign tab ("Z") gets appended by passive discovery after we pinned diff --git a/cli/src/output.rs b/cli/src/output.rs index db6ba4a..30d48b5 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -297,6 +297,11 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou // Snapshot if let Some(snapshot) = data.get("snapshot").and_then(|v| v.as_str()) { print_with_boundaries(snapshot, origin, opts); + // Canvas-app hint: the tree was near-empty but the page paints to a + // , so refs are a dead end — point at the screenshot path. + if let Some(note) = data.get("note").and_then(|v| v.as_str()) { + eprintln!("{}", color::dim(note)); + } return; } // Title @@ -3066,6 +3071,9 @@ Core Commands: type Type into element fill Clear and fill press Press key (Enter, Tab, Control+a) + keydown Hold a key down (no auto-release) — for games/shortcuts + keyup Release a held key. Pair with keydown to hold-to-move: + `keydown d` … `keyup d` keyboard type Type text with real keystrokes (no selector) keyboard inserttext Insert text without key events hover Hover element diff --git a/skill-data/core/SKILL.md b/skill-data/core/SKILL.md index 060c0e1..fb07e9e 100644 --- a/skill-data/core/SKILL.md +++ b/skill-data/core/SKILL.md @@ -226,8 +226,11 @@ chrome-use hover @e1 # hover chrome-use focus @e1 # focus (useful before keyboard input) chrome-use fill @e2 "hello" # clear then type chrome-use type @e2 " world" # type without clearing -chrome-use press Enter # press a key at current focus +chrome-use press Enter # press a key at current focus (down+up) chrome-use press Control+a # key combination +chrome-use keydown d # HOLD a key down (no auto-release) +chrome-use keyup d # release it — pair them to hold-to-move + # in a game: `keydown d; sleep; keyup d` chrome-use check @e3 # check checkbox chrome-use uncheck @e3 # uncheck chrome-use select @e4 "option-value" # native