fix(relay): don't hijack a user tab on open; surface keydown/keyup + canvas hint
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 <key>` / `keyup <key>` (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.
This commit is contained in:
@@ -2888,7 +2888,32 @@ async fn handle_snapshot(cmd: &Value, state: &mut DaemonState) -> Result<Value,
|
||||
})
|
||||
.collect();
|
||||
|
||||
Ok(json!({ "snapshot": tree, "origin": url, "refs": refs }))
|
||||
let ref_count = refs.len();
|
||||
let mut out = json!({ "snapshot": tree, "origin": url, "refs": refs });
|
||||
|
||||
// Canvas/WebGL apps (games, map/3D viewers, drawing tools) paint to a
|
||||
// <canvas> 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 <canvas> (game / WebGL / editor) and exposes almost no \
|
||||
accessibility tree — refs won't help. Use `screenshot` to see it, coordinate \
|
||||
`click <x> <y>` 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
|
||||
|
||||
@@ -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<String>,
|
||||
) -> 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<Value, String> {
|
||||
// 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
|
||||
|
||||
@@ -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
|
||||
// <canvas>, 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 <sel> <text> Type into element
|
||||
fill <sel> <text> Clear and fill
|
||||
press <key> Press key (Enter, Tab, Control+a)
|
||||
keydown <key> Hold a key down (no auto-release) — for games/shortcuts
|
||||
keyup <key> Release a held key. Pair with keydown to hold-to-move:
|
||||
`keydown d` … `keyup d`
|
||||
keyboard type <text> Type text with real keystrokes (no selector)
|
||||
keyboard inserttext <text> Insert text without key events
|
||||
hover <sel> Hover element
|
||||
|
||||
@@ -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 <select> only
|
||||
@@ -296,6 +299,23 @@ chrome-use click --coords 449,320 # same, explicit flag
|
||||
|
||||
A bare-number argument is always a coordinate, never a selector.
|
||||
|
||||
### Canvas / WebGL apps (games, map & 3D viewers, drawing tools)
|
||||
|
||||
These paint everything to a `<canvas>` and expose **almost no accessibility
|
||||
tree**, so `snapshot` comes back near-empty and refs are a dead end. `snapshot`
|
||||
detects this and prints a one-line hint. Drive them the screenshot way:
|
||||
|
||||
```bash
|
||||
chrome-use screenshot /tmp/s.png # SEE the state (your only read path —
|
||||
# eval/get text return nothing useful)
|
||||
chrome-use click 640 360 # interact by viewport coordinate
|
||||
chrome-use keydown d; sleep 0.6; chrome-use keyup d # hold-to-move
|
||||
chrome-use press Space # discrete actions (jump/attack/confirm)
|
||||
```
|
||||
|
||||
Each command is a ~250ms round-trip, so this is fine for turn-based / canvas
|
||||
*apps* but too slow to play a real-time 60fps action game frame-by-frame.
|
||||
|
||||
## Waiting (read this)
|
||||
|
||||
Agents fail more often from bad waits than from bad selectors. Pick the
|
||||
|
||||
Reference in New Issue
Block a user