Files
claw/docs/superpowers/plans/2026-04-14-helper-page-lifecycle-hidden-domain-plan.md
2026-04-14 09:01:59 +08:00

17 KiB

Helper Page Lifecycle Fix & Hidden Domain Support — Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Fix duplicate browser-helper.html pages caused by WebSocket reconnections, add cleanup on Drop, and introduce a config switch for hidden-domain page opening.

Architecture: Three changes: (1) lift cached_host from serve_client() to the outer run() loop so reconnections share one host, (2) enhance Drop for LiveBrowserCallbackHost to send a close-page command via browser WS, (3) add use_hidden_domain: bool parameter that selects between sgBrowerserOpenPage/sgHideBrowerserOpenPage and their corresponding close APIs.

Tech Stack: Rust, tungstenite WebSocket crate, SuperRPA browser WS protocol


Task 1: Add use_hidden_domain field and update bootstrap_helper_page

Files:

  • Modify: src/browser/callback_host.rs:28 (constant), :44-51 (struct), :215-252 (constructor), :340-359 (bootstrap fn)

  • Step 1: Change HELPER_BOOTSTRAP_ACTION from constant to a function of use_hidden_domain

Replace the constant and update bootstrap_helper_page to accept and use the flag:

// DELETE this line:
// const HELPER_BOOTSTRAP_ACTION: &str = "sgBrowerserOpenPage";

// REPLACE bootstrap_helper_page signature and body:
fn bootstrap_helper_page(
    browser_ws_url: &str,
    request_url: &str,
    helper_url: &str,
    use_hidden_domain: bool,
) -> Result<(), PipeError> {
    let (mut websocket, _) = connect(browser_ws_url)
        .map_err(|err| PipeError::Protocol(format!("browser websocket connect failed: {err}")))?;
    configure_bootstrap_socket(&mut websocket)?;
    websocket
        .send(Message::Text(
            r#"{"type":"register","role":"web"}"#.to_string().into(),
        ))
        .map_err(|err| PipeError::Protocol(format!("browser websocket register failed: {err}")))?;
    let _ = recv_bootstrap_prelude(&mut websocket);
    let open_action = if use_hidden_domain {
        "sgHideBrowerserOpenPage"
    } else {
        "sgBrowerserOpenPage"
    };
    let payload = json!([
        request_url,
        open_action,
        helper_url,
    ])
    .to_string();
    websocket
        .send(Message::Text(payload.into()))
        .map_err(|err| PipeError::Protocol(format!("helper bootstrap send failed: {err}")))?;
    Ok(())
}
  • Step 2: Add new fields to LiveBrowserCallbackHost
#[derive(Debug)]
pub(crate) struct LiveBrowserCallbackHost {
    host: Arc<BrowserCallbackHost>,
    shutdown: Arc<AtomicBool>,
    server_thread: Mutex<Option<JoinHandle<()>>>,
    command_lock: Mutex<()>,
    result_timeout: Duration,
    browser_ws_url: String,
    use_hidden_domain: bool,
}
  • Step 3: Update start_with_browser_ws_url to accept and store the new parameter
impl LiveBrowserCallbackHost {
    pub(crate) fn start_with_browser_ws_url(
        browser_ws_url: &str,
        bootstrap_request_url: &str,
        ready_timeout: Duration,
        result_timeout: Duration,
        use_hidden_domain: bool,
    ) -> Result<Self, PipeError> {
        let listener = TcpListener::bind("127.0.0.1:0").map_err(|err| {
            PipeError::Protocol(format!("failed to bind callback host listener: {err}"))
        })?;
        listener.set_nonblocking(true).map_err(|err| {
            PipeError::Protocol(format!("failed to configure callback host listener: {err}"))
        })?;
        let origin = format!(
            "http://{}",
            listener.local_addr().map_err(|err| {
                PipeError::Protocol(format!(
                    "failed to resolve callback host listener address: {err}"
                ))
            })?
        );
        let host = Arc::new(BrowserCallbackHost::with_urls(&origin, browser_ws_url));
        let shutdown = Arc::new(AtomicBool::new(false));
        let thread_host = host.clone();
        let thread_shutdown = shutdown.clone();
        let server_thread = thread::spawn(move || serve_loop(listener, thread_host, thread_shutdown));

        bootstrap_helper_page(browser_ws_url, bootstrap_request_url, host.helper_url(), use_hidden_domain)?;
        wait_for_helper_ready(host.as_ref(), ready_timeout)?;

        let live_host = Self {
            host,
            shutdown,
            server_thread: Mutex::new(Some(server_thread)),
            command_lock: Mutex::new(()),
            result_timeout,
            browser_ws_url: browser_ws_url.to_string(),
            use_hidden_domain,
        };
        Ok(live_host)
    }
  • Step 4: Fix the inline test struct literal that constructs LiveBrowserCallbackHost directly

In the live_callback_host_treats_simulated_mouse_command_as_fire_and_forget test (around line 1110), add the new fields:

        let host = LiveBrowserCallbackHost {
            host: Arc::new(BrowserCallbackHost::new()),
            shutdown: Arc::new(AtomicBool::new(false)),
            server_thread: Mutex::new(None),
            command_lock: Mutex::new(()),
            result_timeout: Duration::from_millis(10),
            browser_ws_url: "ws://127.0.0.1:12345".to_string(),
            use_hidden_domain: false,
        };
  • Step 5: Run build to verify compilation

Run: cargo build 2>&1 Expected: 0 errors. The HELPER_BOOTSTRAP_ACTION constant removal and signature changes should all be internally consistent.

  • Step 6: Run tests to verify existing behavior is preserved

Run: cargo test -- callback_host 2>&1 Expected: All existing callback_host tests pass (including live_callback_host_sends_bootstrap_open_page_command which still checks for sgBrowerserOpenPage since no caller passes true yet).

  • Step 7: Commit
git add src/browser/callback_host.rs
git commit -m "feat(callback_host): add use_hidden_domain param to bootstrap_helper_page"

Task 2: Enhance Drop to close the helper page

Files:

  • Modify: src/browser/callback_host.rs:321-328 (Drop impl)

  • Step 1: Add close_helper_page helper function

Add this function near bootstrap_helper_page (after line ~360):

/// Best-effort attempt to close the helper page tab via browser WebSocket.
/// Silently ignores all errors — this runs during Drop and must not panic.
fn close_helper_page(browser_ws_url: &str, helper_url: &str, use_hidden_domain: bool) {
    let close_action = if use_hidden_domain {
        "sgHideBrowerserClosePage"
    } else {
        "sgBrowserClosePage"
    };

    let result: Result<(), Box<dyn std::error::Error>> = (|| {
        // Use a raw TcpStream with timeouts instead of tungstenite::connect
        // which does not expose a connection timeout.
        let addr = browser_ws_url
            .trim_start_matches("ws://")
            .trim_start_matches("wss://");
        let stream = TcpStream::connect_timeout(
            &addr.parse().map_err(|e| format!("addr parse: {e}"))?,
            Duration::from_millis(100),
        )?;
        stream.set_read_timeout(Some(Duration::from_millis(200)))?;
        stream.set_write_timeout(Some(Duration::from_millis(200)))?;
        let (mut websocket, _) = tungstenite::client(
            browser_ws_url,
            stream,
        )?;
        websocket.send(Message::Text(
            r#"{"type":"register","role":"web"}"#.to_string().into(),
        ))?;
        // Drain the welcome prelude (best-effort, ignore timeout).
        let _ = websocket.read();
        let payload = json!([helper_url, close_action, helper_url]).to_string();
        websocket.send(Message::Text(payload.into()))?;
        Ok(())
    })();

    if let Err(err) = result {
        eprintln!("close_helper_page best-effort failed (harmless): {err}");
    }
}
  • Step 2: Update Drop for LiveBrowserCallbackHost to call close_helper_page
impl Drop for LiveBrowserCallbackHost {
    fn drop(&mut self) {
        // Best-effort: tell the browser to close the helper page tab.
        close_helper_page(
            &self.browser_ws_url,
            self.host.helper_url(),
            self.use_hidden_domain,
        );

        self.shutdown.store(true, Ordering::Relaxed);
        if let Some(handle) = self.server_thread.lock().unwrap().take() {
            let _ = handle.join();
        }
    }
}
  • Step 3: Run build to verify compilation

Run: cargo build 2>&1 Expected: 0 errors. close_helper_page uses types already imported (TcpStream, Duration, json!, Message).

  • Step 4: Run tests

Run: cargo test -- callback_host 2>&1 Expected: All pass. The Drop enhancement is best-effort and the test helper constructs hosts with server_thread: Mutex::new(None), so Drop completes cleanly.

  • Step 5: Commit
git add src/browser/callback_host.rs
git commit -m "feat(callback_host): close helper page on Drop via browser WS"

Task 3: Lift cached_host to outer loop and update serve_client signature

Files:

  • Modify: src/service/mod.rs:72-96 (run loop)

  • Modify: src/service/server.rs:232-241 (serve_client signature and cached_host init)

  • Step 1: Change serve_client to accept cached_host as a parameter

In src/service/server.rs, change the function signature and remove the local cached_host variable:

pub fn serve_client(
    context: &AgentRuntimeContext,
    session: &ServiceSession,
    sink: Arc<ServiceEventSink>,
    browser_ws_url: &str,
    mac_policy: &MacPolicy,
    cached_host: &mut Option<Arc<LiveBrowserCallbackHost>>,
) -> Result<(), PipeError> {
    // DELETE the line: let mut cached_host: Option<Arc<LiveBrowserCallbackHost>> = None;

    loop {
        // ... rest of function body unchanged, `cached_host` is now the parameter

The body references to cached_host remain identical — they just operate on the borrowed mutable reference instead of a local variable.

  • Step 2: Update start_with_browser_ws_url call to pass false for use_hidden_domain

In src/service/server.rs, at the LiveBrowserCallbackHost::start_with_browser_ws_url call (around line 288), add the false argument:

                    match LiveBrowserCallbackHost::start_with_browser_ws_url(
                        browser_ws_url,
                        &bootstrap_url,
                        Duration::from_secs(15),
                        BROWSER_RESPONSE_TIMEOUT,
                        false, // use_hidden_domain: visible tab for now
                    ) {
  • Step 3: Lift cached_host into run() in mod.rs

In src/service/mod.rs, declare cached_host before the loop and pass it to serve_client:

    // Add this import at the top of the function or file:
    use crate::browser::callback_host::LiveBrowserCallbackHost;

    // Before the loop (after line 64, after `let session = ...`):
    let mut cached_host: Option<Arc<LiveBrowserCallbackHost>> = None;

    loop {
        let (stream, _) = listener.accept()?;
        let websocket = accept(stream)
            .map_err(|err| PipeError::Protocol(format!("service websocket accept failed: {err}")))?;
        let sink = Arc::new(ServiceEventSink::from_websocket(websocket));
        match session.try_attach_client() {
            Ok(()) => {
                let result = serve_client(
                    &runtime_context,
                    &session,
                    sink.clone(),
                    browser_ws_url,
                    &mac_policy,
                    &mut cached_host,
                );
                session.detach_client();
                match result {
                    Ok(()) | Err(PipeError::PipeClosed) => {}
                    Err(err) => return Err(err),
                }
            }
            Err(message) => {
                sink.send_service_message(message)?;
            }
        }
    }
  • Step 4: Update the pub use export if needed

Check src/service/mod.rs:17:

pub use server::{serve_client, ServiceEventSink, ServiceSession};

The signature change is compatible — serve_client is still public with an added parameter. Any external callers will get a compile error guiding them to add the parameter, which is the desired behavior.

  • Step 5: Run build to verify compilation

Run: cargo build 2>&1 Expected: 0 errors. If there are external test files calling serve_client, they will fail here and need the new parameter added.

  • Step 6: Run full test suite

Run: cargo test 2>&1 Expected: All tests pass. External test files that call serve_client indirectly through the service protocol tests should still work because they use the WS protocol layer, not serve_client directly. (Verified: grep found 0 test files referencing serve_client or LiveBrowserCallbackHost.)

  • Step 7: Commit
git add src/service/mod.rs src/service/server.rs
git commit -m "fix(service): lift cached_host to outer loop to prevent duplicate helper pages"

Task 4: Add tests for hidden domain bootstrap

Files:

  • Modify: src/browser/callback_host.rs (inline tests module, around line 1071)

  • Step 1: Update existing live_callback_host_sends_bootstrap_open_page_command test

The test currently calls start_with_browser_ws_url with 4 args. Add the 5th arg false:

    #[test]
    fn live_callback_host_sends_bootstrap_open_page_command() {
        let (ws_url, frames, handle) = start_fake_browser_status_server();

        let result = LiveBrowserCallbackHost::start_with_browser_ws_url(
            &ws_url,
            "https://www.zhihu.com",
            Duration::from_millis(100),
            Duration::from_millis(50),
            false,
        );
        assert!(result.is_err(), "expected timeout because no real helper page loads");
        drop(result);
        handle.join().unwrap();

        let sent = frames.lock().unwrap().clone();
        assert!(
            sent.iter().any(|frame| frame.contains("sgBrowerserOpenPage")),
            "bootstrap should send sgBrowerserOpenPage to the browser WS; sent frames: {sent:?}"
        );
        assert!(
            sent.iter().any(|frame| frame.contains("/sgclaw/browser-helper.html")),
            "bootstrap should include the helper page URL; sent frames: {sent:?}"
        );
        assert!(
            sent.iter().any(|frame| frame.contains("https://www.zhihu.com")),
            "bootstrap requestUrl should be the provided page URL; sent frames: {sent:?}"
        );
    }
  • Step 2: Add new test for hidden domain bootstrap

Add this test after the existing one:

    #[test]
    fn live_callback_host_hidden_domain_sends_hide_open_page_command() {
        let (ws_url, frames, handle) = start_fake_browser_status_server();

        let result = LiveBrowserCallbackHost::start_with_browser_ws_url(
            &ws_url,
            "https://www.zhihu.com",
            Duration::from_millis(100),
            Duration::from_millis(50),
            true,
        );
        assert!(result.is_err(), "expected timeout because no real helper page loads");
        drop(result);
        handle.join().unwrap();

        let sent = frames.lock().unwrap().clone();
        assert!(
            sent.iter().any(|frame| frame.contains("sgHideBrowerserOpenPage")),
            "hidden domain bootstrap should send sgHideBrowerserOpenPage; sent frames: {sent:?}"
        );
        assert!(
            !sent.iter().any(|frame| {
                frame.contains("\"sgBrowerserOpenPage\"")
            }),
            "hidden domain bootstrap should NOT send visible sgBrowerserOpenPage; sent frames: {sent:?}"
        );
        assert!(
            sent.iter().any(|frame| frame.contains("/sgclaw/browser-helper.html")),
            "bootstrap should include the helper page URL; sent frames: {sent:?}"
        );
    }
  • Step 3: Run all callback_host tests

Run: cargo test -- callback_host 2>&1 Expected: All 3 tests pass:

  • live_callback_host_sends_bootstrap_open_page_command — regression, visible domain

  • live_callback_host_hidden_domain_sends_hide_open_page_command — new, hidden domain

  • live_callback_host_treats_simulated_mouse_command_as_fire_and_forget — unchanged

  • Step 4: Run full test suite

Run: cargo test 2>&1 Expected: All tests pass.

  • Step 5: Commit
git add src/browser/callback_host.rs
git commit -m "test(callback_host): add hidden domain bootstrap test"

Task 5: Full build verification

Files: None (verification only)

  • Step 1: Clean build

Run: cargo build 2>&1 Expected: 0 errors. Warnings about dead code in unrelated modules are acceptable.

  • Step 2: Full test suite

Run: cargo test 2>&1 Expected: All tests pass. The pre-existing lineloss_period_resolver_prompts_for_missing_period failure (from previous work) is known and unrelated.

  • Step 3: Verify the key behavioral changes in code

Manually confirm:

  1. src/service/mod.rscached_host is declared BEFORE the loop, not inside serve_client
  2. src/browser/callback_host.rsDrop::drop calls close_helper_page before shutdown
  3. src/browser/callback_host.rsbootstrap_helper_page uses "sgHideBrowerserOpenPage" when use_hidden_domain == true and "sgBrowerserOpenPage" when false
  4. src/service/server.rsstart_with_browser_ws_url call passes false as use_hidden_domain