fix(interaction): honor scroll cancellation and visible bounds

crates/bsk-cli/skill/SKILL.md

@@ -59,7 +59,12 @@ bsk click|hover|focus|blur|fill|select|press ... --session <id>
 bsk observe --session <id>             # after navigation or a meaningful DOM change
 ```
 
-`bsk scroll-to <ref-or-selector>` scrolls an element and its frame owners into the visible viewport.
+`bsk scroll-to <ref-or-selector> --session <id>` scrolls an element and its frame owners into view.
+Use a fresh element ref for iframe/shadow-root targets; CSS selectors search the main document.
+The result is the visible border-box portion's bounds in top-level viewport CSS pixels after
+ancestor clipping. Partial visibility is enough; hidden or fully clipped targets fail with
+`element_not_visible`. The result does not guarantee that other elements do not cover the target.
+For a specific tab or deadline: `bsk scroll-to @e3 --session <id> --tab-id 42 --timeout 5s`.
 
 `bsk focus <ref>` explicitly focuses a target; `bsk blur <ref>` removes focus and reports whether
 it was focused. Use these for UI states triggered by focus changes.

crates/bsk-cli/src/cli/scroll.rs

@@ -7,9 +7,10 @@ use bsk_protocol::Method;
 use bsk_protocol::tools::{ScrollToParams, ScrollToResult};
 use clap::Args;
 
+use crate::cli::dialogs::print_dialog_summaries;
 use crate::cli::ensure_daemon::ensure_daemon;
 use crate::cli::error::{CliError, Format};
-use crate::cli::interaction::looks_like_ref;
+use crate::cli::interaction::split_target;
 use crate::cli::navigate::parse_timeout_ms;
 
 #[derive(Debug, Clone, Args)]
@@ -34,8 +35,8 @@ pub struct ScrollToArgs {
 }
 
 pub fn dispatch(args: ScrollToArgs, format: Format) -> Result<(), CliError> {
-    let info = ensure_daemon().context("ensure daemon is running")?;
     let (ref_, selector) = split_target(args.target, args.ref_, args.selector)?;
+    let info = ensure_daemon().context("ensure daemon is running")?;
     let params = ScrollToParams {
         session_id: args.session,
         ref_,
@@ -63,30 +64,12 @@ pub fn dispatch(args: ScrollToArgs, format: Format) -> Result<(), CliError> {
                 "scroll-to ok tab={} target={target} bounds=({}, {}, {}, {})",
                 reply.tab_id, reply.x, reply.y, reply.width, reply.height
             );
+            print_dialog_summaries(&reply.dialogs);
         }
     }
     Ok(())
 }
 
-fn split_target(
-    positional: Option<String>,
-    explicit_ref: Option<String>,
-    explicit_selector: Option<String>,
-) -> Result<(Option<String>, Option<String>), CliError> {
-    match (positional, explicit_ref, explicit_selector) {
-        (None, None, None) => Err(CliError::Local(anyhow::anyhow!(
-            "missing target: pass <ref-or-selector>, --ref @eN, or --selector <css>"
-        ))),
-        (None, Some(r), None) => Ok((Some(r), None)),
-        (None, None, Some(s)) => Ok((None, Some(s))),
-        (Some(target), None, None) if looks_like_ref(&target) => Ok((Some(target), None)),
-        (Some(target), None, None) => Ok((None, Some(target))),
-        _ => Err(CliError::Local(anyhow::anyhow!(
-            "pass exactly one of: <target>, --ref, or --selector"
-        ))),
-    }
-}
-
 fn format_used_target(used_ref: Option<&str>, used_selector: Option<&str>) -> String {
     used_ref
         .map(|r| format!("@{r}"))

crates/bsk-cli/tests/cli_parse.rs

@@ -264,6 +264,38 @@ fn parses_scroll_to_target() {
     assert_eq!(args.target.as_deref(), Some("@e2"));
 }
 
+#[test]
+fn parses_scroll_to_explicit_target_tab_and_timeout() {
+    for flag in ["--ref", "--selector"] {
+        let cli = parse(&[
+            "bsk",
+            "scroll-to",
+            flag,
+            "e3",
+            "--session",
+            "s1",
+            "--tab-id",
+            "42",
+            "--timeout",
+            "5s",
+        ]);
+        let Command::ScrollTo(args) = cli.command else {
+            panic!("expected scroll-to command");
+        };
+        assert_eq!(args.tab_id, Some(42));
+        assert_eq!(args.timeout, 5_000);
+        assert_eq!(
+            if flag == "--ref" {
+                args.ref_
+            } else {
+                args.selector
+            }
+            .as_deref(),
+            Some("e3")
+        );
+    }
+}
+
 #[test]
 fn parses_focus_target() {
     let cli = parse(&["bsk", "focus", "@e2", "--session", "s1"]);

crates/bsk-cli/tests/session_user_interrupt.rs

@@ -293,6 +293,15 @@ async fn session_user_interrupt_event_cancels_inflight_with_user_aborted() {
 /// catch a tool dispatched arbitrarily later.
 #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
 async fn user_interrupt_rejects_next_mutating_tool_call_when_session_was_idle() {
+    assert_idle_interrupt_rejects(Method::ToolClick).await;
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+async fn user_interrupt_rejects_scroll_to_before_forwarding() {
+    assert_idle_interrupt_rejects(Method::ToolScrollTo).await;
+}
+
+async fn assert_idle_interrupt_rejects(method: Method) {
     let (handle, sock) = spawn_daemon().await;
     let mut ws = connect_ext(handle.ws_addr()).await;
     let _ = handshake_as_ext(&mut ws).await;
@@ -301,10 +310,11 @@ async fn user_interrupt_rejects_next_mutating_tool_call_when_session_was_idle()
 
     // Fake extension: answer tool.session_start, observe other
     // requests but never reply. We expect the daemon to reject the
-    // tool call BEFORE it ever forwards a tool.click frame.
+    // tool call BEFORE it ever forwards a browser-input frame.
     let ws_sink_for_responder = Arc::clone(&ws_sink);
-    let observed_click_count = Arc::new(std::sync::atomic::AtomicUsize::new(0));
-    let observed_click_count_clone = Arc::clone(&observed_click_count);
+    let observed_call_count = Arc::new(std::sync::atomic::AtomicUsize::new(0));
+    let observed_call_count_clone = Arc::clone(&observed_call_count);
+    let forwarded_method = method.clone();
     let responder = tokio::spawn(async move {
         let mut ws_stream = ws_stream;
         while let Some(Ok(msg)) = ws_stream.next().await {
@@ -329,8 +339,8 @@ async fn user_interrupt_rejects_next_mutating_tool_call_when_session_was_idle()
                     g.send(Message::Text(serde_json::to_string(&reply).unwrap()))
                         .await
                         .unwrap();
-                } else if req.method == Method::ToolClick {
-                    observed_click_count_clone.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
+                } else if req.method == forwarded_method {
+                    observed_call_count_clone.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
                     // Deliberately do NOT reply — daemon should reject before
                     // ever forwarding to us.
                     let _ = req;
@@ -377,12 +387,12 @@ async fn user_interrupt_rejects_next_mutating_tool_call_when_session_was_idle()
     let state = handle.state();
     wait_for_session_interrupt_pending(&state, &start.session_id).await;
 
-    // The CLI now issues a tool.click (mutating). The daemon must
+    // The CLI now issues a browser-input (mutating). The daemon must
     // reject it WITHOUT forwarding to the extension.
-    let click_outcome = ipc
+    let outcome = ipc
         .call::<_, serde_json::Value>(
-            "click-after-interrupt",
-            Method::ToolClick,
+            "input-after-interrupt",
+            method,
             Some(json!({
                 "session_id": start.session_id,
                 "ref": "fake-ref-1",
@@ -391,7 +401,7 @@ async fn user_interrupt_rejects_next_mutating_tool_call_when_session_was_idle()
         )
         .await
         .unwrap();
-    let err = click_outcome.expect_err("tool.click must be rejected");
+    let err = outcome.expect_err("browser-input must be rejected");
     assert_eq!(
         err.code,
         ErrorCode::UserAborted,
@@ -399,11 +409,11 @@ async fn user_interrupt_rejects_next_mutating_tool_call_when_session_was_idle()
         err
     );
 
-    // Crucially: the extension never saw a tool.click frame.
+    // Crucially: the extension never saw a browser-input frame.
     assert_eq!(
-        observed_click_count.load(std::sync::atomic::Ordering::SeqCst),
+        observed_call_count.load(std::sync::atomic::Ordering::SeqCst),
         0,
-        "tool.click must NOT have been forwarded to the extension"
+        "browser-input must NOT have been forwarded to the extension"
     );
 
     responder.abort();