From ea03f36f3a105a04f9062518c03d547c6037b539 Mon Sep 17 00:00:00 2001
From: drakezhang <drakezhang@tencent.com>
Date: Wed, 22 Jul 2026 12:17:15 +0800
Subject: [PATCH] Widen session.start browser-connect wait for cold reconnects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After a long idle the service worker is only revived on its next
keepalive-alarm tick (up to 30s away), so the previous 5s wait made
the first command after idle fail with `no_browser_connected` even
though the extension was about to reconnect.

- Raise the daemon's `EXTENSION_CONNECT_WAIT` (used by `session.start`)
  to 35s so it comfortably covers one alarm period plus handshake slack.
- Bump the CLI `session start` IPC timeout to that window plus slack so
  it never gives up before the daemon answers, and print a one-line
  "waiting for browser extension to connect…" hint when it actually
  waits.
- Keep `bsk status` / `browsers` on their own short 5s wait so quick
  informational commands are not slowed down.

Co-authored-by: Cursor <cursoragent@cursor.com>
---
 crates/bsk-cli/src/cli/browser_wait.rs | 12 ++++++++---
 crates/bsk-cli/src/cli/session.rs      | 30 +++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/crates/bsk-cli/src/cli/browser_wait.rs b/crates/bsk-cli/src/cli/browser_wait.rs
index 56ef246..1ac4767 100644
--- a/crates/bsk-cli/src/cli/browser_wait.rs
+++ b/crates/bsk-cli/src/cli/browser_wait.rs
@@ -2,10 +2,16 @@
 
 use std::time::Duration;
 
-use crate::daemon::browsers::EXTENSION_CONNECT_WAIT;
-
 /// Default wait passed to daemon status/list RPCs when the registry is empty.
-pub const DEFAULT_BROWSER_CONNECT_WAIT: Duration = EXTENSION_CONNECT_WAIT;
+///
+/// Deliberately shorter than the daemon's [`EXTENSION_CONNECT_WAIT`]
+/// (used by `session.start`): `bsk status` / `bsk browsers` are quick
+/// informational snapshots, so blocking them for a full keepalive-alarm
+/// period would hurt more than it helps. The long wait is reserved for
+/// the command path that actually needs the extension online.
+///
+/// [`EXTENSION_CONNECT_WAIT`]: crate::daemon::browsers::EXTENSION_CONNECT_WAIT
+pub const DEFAULT_BROWSER_CONNECT_WAIT: Duration = Duration::from_secs(5);
 /// Keep CLI-side waits aligned with the daemon's IPC clamp so a bad env value
 /// cannot inflate read timeouts after the daemon has already returned.
 const MAX_BROWSER_CONNECT_WAIT: Duration = Duration::from_secs(60);
diff --git a/crates/bsk-cli/src/cli/session.rs b/crates/bsk-cli/src/cli/session.rs
index 0f1d94a..20b6764 100644
--- a/crates/bsk-cli/src/cli/session.rs
+++ b/crates/bsk-cli/src/cli/session.rs
@@ -6,6 +6,8 @@
 //! structured JSON instead.
 
 use std::path::PathBuf;
+use std::sync::Arc;
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::time::Duration;
 
 use anyhow::Context;
@@ -17,9 +19,18 @@ use serde::{Deserialize, Serialize};
 
 use crate::cli::ensure_daemon::ensure_daemon;
 use crate::cli::error::{self, CliError, Format, RenderExtras};
+use crate::daemon::browsers::EXTENSION_CONNECT_WAIT;
 
 const SESSION_STOP_IPC_TIMEOUT: Duration = Duration::from_secs(60 * 60);
 
+/// IPC read budget for `session.start`. The daemon holds this RPC open
+/// while it polls for the extension to (re)connect for up to
+/// [`EXTENSION_CONNECT_WAIT`], so the CLI-side timeout must exceed that
+/// window plus scheduling slack — otherwise the CLI would give up and
+/// surface a spurious timeout before the daemon ever answers.
+const SESSION_START_IPC_TIMEOUT: Duration =
+    EXTENSION_CONNECT_WAIT.saturating_add(Duration::from_secs(10));
+
 /// `bsk session …` subcommand tree.
 #[derive(Debug, Clone, Args)]
 pub struct SessionCmd {
@@ -113,14 +124,31 @@ pub fn dispatch(cmd: SessionCmd, format: Format) -> Result<(), CliError> {
 }
 
 fn run_start(sock: PathBuf, args: SessionStartArgs, format: Format) -> Result<(), CliError> {
+    // The daemon may hold `session.start` open for up to
+    // `EXTENSION_CONNECT_WAIT` while a just-woken service worker
+    // reconnects. Let the user know we are waiting rather than hung —
+    // but only if it actually takes a moment, so the common (already
+    // connected) fast path stays silent. Human output only; the hint
+    // goes to stderr so it never contaminates the session id on stdout.
+    let waited = Arc::new(AtomicBool::new(false));
+    if matches!(format, Format::Human) {
+        let waited = Arc::clone(&waited);
+        std::thread::spawn(move || {
+            std::thread::sleep(Duration::from_millis(750));
+            if !waited.swap(true, Ordering::SeqCst) {
+                eprintln!("waiting for browser extension to connect…");
+            }
+        });
+    }
     let result: Result<StartReply, CliError> = call(
         sock,
         Method::SessionStart,
         Some(StartParams {
             browser_instance_id: args.browser,
         }),
-        Duration::from_secs(30),
+        SESSION_START_IPC_TIMEOUT,
     );
+    waited.store(true, Ordering::SeqCst);
     match result {
         Ok(reply) => match format {
             Format::Json => {