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