diff --git a/crates/idx-core/src/api/stockbit/browser.rs b/crates/idx-core/src/api/stockbit/browser.rs index 8db593e..3e94ce5 100644 --- a/crates/idx-core/src/api/stockbit/browser.rs +++ b/crates/idx-core/src/api/stockbit/browser.rs @@ -234,6 +234,9 @@ fn wait_for_session( ) -> Result<(), IdxError> { let deadline = Instant::now() + timeout; let mut rejected: Option = None; + // Why Stockbit refused that cookie, so the error that ends the wait says + // so: a polling frontend never saw the warning. + let mut refusal: Option = None; // After a check fails for a passing reason, the next is held back. let mut retry_at: Option = None; let mut last_error: Option = None; @@ -273,6 +276,7 @@ fn wait_for_session( "Stockbit refused the session in idx's browser ({err}); sign in \ again in that window" )); + refusal = Some(err.to_string()); rejected = Some(cookie.value); retry_at = None; } @@ -283,12 +287,18 @@ fn wait_for_session( Ok(_) => last_error = None, Err(err) => last_error = Some(err.to_string()), } + let refused = || { + refusal + .as_ref() + .map(|err| format!("; Stockbit refused the session signed in there ({err})")) + .unwrap_or_default() + }; if !alive() { devtools.close(); - return Err(IdxError::Io( - "the browser exited before idx got a session; run `idx stockbit login` again" - .to_string(), - )); + return Err(IdxError::Io(format!( + "the browser exited before idx got a session{}; run `idx stockbit login` again", + refused() + ))); } match devtools.has_page() { Ok(true) => { @@ -299,11 +309,11 @@ fn wait_for_session( misses += 1; if saw_window && misses >= 2 { devtools.close(); - return Err(IdxError::Io( - "the Stockbit sign-in window was closed before the session arrived; \ - run `idx stockbit login` again" - .to_string(), - )); + return Err(IdxError::Io(format!( + "the Stockbit sign-in window was closed before the session arrived{}; \ + run `idx stockbit login` again", + refused() + ))); } } Err(err) => last_error = Some(err.to_string()), @@ -311,9 +321,10 @@ fn wait_for_session( if Instant::now() >= deadline { devtools.close(); let mut message = format!( - "the Stockbit sign-in timed out after {}s and idx closed its browser window; \ + "the Stockbit sign-in timed out after {}s and idx closed its browser window{}; \ re-run `idx stockbit login`, or add --no-browser for the local page", - timeout.as_secs() + timeout.as_secs(), + refused() ); if let Some(err) = last_error { message.push_str(&format!(" (last DevTools error: {err})")); @@ -993,6 +1004,11 @@ mod tests { ) .unwrap_err(); assert!(err.to_string().contains("timed out"), "{err}"); + assert!( + err.to_string() + .contains("Stockbit refused the session signed in there (auth error: expired)"), + "{err}" + ); assert_eq!(checks.get(), 1, "one refusal per distinct cookie"); browser.finish(); let _ = std::fs::remove_dir_all(&profile); diff --git a/crates/idx-core/src/api/stockbit/signin.rs b/crates/idx-core/src/api/stockbit/signin.rs index b5a8794..fa77057 100644 --- a/crates/idx-core/src/api/stockbit/signin.rs +++ b/crates/idx-core/src/api/stockbit/signin.rs @@ -242,6 +242,10 @@ pub struct Progress { /// Why it failed: the error's code and message. #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, + /// The latest warning while it waits, such as Stockbit refusing the + /// session signed in so far; gone once it signs in. + #[serde(skip_serializing_if = "Option::is_none")] + pub warning: Option, } #[derive(Debug, Clone, Serialize)] @@ -257,6 +261,7 @@ struct Slot { timeout: Duration, state: State, error: Option, + warning: Option, cancel: Arc, thread: Option>, } @@ -273,6 +278,7 @@ impl Slot { expires_in_secs, already_open, error: self.error.clone(), + warning: self.warning.clone(), } } @@ -303,6 +309,7 @@ pub fn start_background(options: &Options) -> Result { expires_in_secs: 0, already_open: false, error: None, + warning: None, }); } let sign_in = SignIn::start(options)?; @@ -315,16 +322,29 @@ pub fn start_background(options: &Options) -> Result { timeout: sign_in.timeout(), state: State::Waiting, error: None, + warning: None, cancel: cancel.clone(), thread: None, }; let progress = open.progress(false); open.thread = Some(std::thread::spawn(move || { + // Nobody reads a polling frontend's stderr, so what the wait warns + // about (a refused session) goes into the progress it polls. + let sink = crate::runtime::on_warning(move |message| { + let mut slot = SLOT.lock().unwrap_or_else(PoisonError::into_inner); + if let Some(open) = slot.as_mut().filter(|open| open.id == id) { + open.warning = Some(message.to_string()); + } + }); let result = sign_in.wait_until(&cancel); + drop(sink); let mut slot = SLOT.lock().unwrap_or_else(PoisonError::into_inner); if let Some(open) = slot.as_mut().filter(|open| open.id == id) { match result { - Ok(_) => open.state = State::SignedIn, + Ok(_) => { + open.state = State::SignedIn; + open.warning = None; + } Err(err) => { open.state = State::Failed; open.error = Some(ProgressError { @@ -398,12 +418,17 @@ mod tests { expires_in_secs: 300, already_open: false, error: None, + warning: None, }; let value = serde_json::to_value(&page).unwrap(); assert_eq!(value["state"], "waiting"); assert_eq!(value["method"], "page"); assert_eq!(value["port"], 7878); - assert!(value.get("reason").is_none() && value.get("error").is_none()); + assert!( + ["reason", "error", "warning"] + .iter() + .all(|key| value.get(key).is_none()) + ); let failed = Progress { state: State::Failed, @@ -416,10 +441,12 @@ mod tests { code: "IO".to_string(), message: "the window was closed".to_string(), }), + warning: Some("Stockbit refused the session".to_string()), }; let value = serde_json::to_value(&failed).unwrap(); assert_eq!(value["method"], "browser"); assert_eq!(value["browser"], "Helium"); assert_eq!(value["error"]["code"], "IO"); + assert_eq!(value["warning"], "Stockbit refused the session"); } } diff --git a/crates/idx-core/src/lib.rs b/crates/idx-core/src/lib.rs index cd49711..dab7f39 100644 --- a/crates/idx-core/src/lib.rs +++ b/crates/idx-core/src/lib.rs @@ -23,12 +23,40 @@ pub mod text; pub use client::{CachePolicy, Fetched, Idx}; pub mod runtime { + use std::cell::RefCell; use std::fmt::Display; use std::sync::atomic::{AtomicBool, Ordering}; static QUIET: AtomicBool = AtomicBool::new(false); static JSON: AtomicBool = AtomicBool::new(false); + type Sink = Box; + + thread_local! { + static SINK: RefCell> = const { RefCell::new(None) }; + } + + /// Sends this thread's warnings to `sink` too, until the guard drops: a + /// frontend whose stderr nobody reads (idx-mcp) hands them on this way. + /// Warnings still go to stderr, and `set_quiet` does not silence `sink`. + pub fn on_warning(sink: impl Fn(&str) + 'static) -> WarningSink { + let previous = SINK.with(|slot| slot.replace(Some(Box::new(sink)))); + WarningSink { previous } + } + + /// Restores the thread's previous sink when dropped. + #[must_use = "warnings reach the sink only while this guard lives"] + pub struct WarningSink { + previous: Option, + } + + impl Drop for WarningSink { + fn drop(&mut self) { + let previous = self.previous.take(); + SINK.with(|slot| *slot.borrow_mut() = previous); + } + } + pub fn set_quiet(quiet: bool) { QUIET.store(quiet, Ordering::Relaxed); } @@ -63,6 +91,40 @@ pub mod runtime { } pub fn warn(message: impl Display) { + let message = message.to_string(); + SINK.with(|slot| { + if let Some(sink) = slot.borrow().as_ref() { + sink(&message); + } + }); emit("warning", message); } + + #[cfg(test)] + mod tests { + use super::*; + use std::rc::Rc; + + /// A sink sees this thread's warnings while its guard lives, and an + /// inner one hands back to the outer when it drops. + #[test] + fn warnings_reach_the_sink_while_its_guard_lives() { + let seen = Rc::new(RefCell::new(Vec::new())); + let outer = { + let seen = seen.clone(); + on_warning(move |m| seen.borrow_mut().push(format!("outer {m}"))) + }; + warn("one"); + { + let seen = seen.clone(); + let _inner = on_warning(move |m| seen.borrow_mut().push(format!("inner {m}"))); + warn("two"); + } + warn("three"); + drop(outer); + warn("four"); + info("not a warning"); + assert_eq!(*seen.borrow(), ["outer one", "inner two", "outer three"]); + } + } } diff --git a/crates/idx-mcp/src/server.rs b/crates/idx-mcp/src/server.rs index 3790249..319e577 100644 --- a/crates/idx-mcp/src/server.rs +++ b/crates/idx-mcp/src/server.rs @@ -190,7 +190,9 @@ IDX_STOCKBIT_REFRESH_TOKEN) and when its tokens expire; never the tokens. Withou get_orderbook fails and stockbit-provider quotes use Stockbit's delayed public page; the \ user adds one with stockbit_login, or `idx stockbit login` in a terminal. After \ stockbit_login, `sign_in.state` says whether that sign-in is waiting, signed_in or failed \ -(with `sign_in.error`).")] +(with `sign_in.error`). While it waits, `sign_in.warning` is the latest problem it hit, such \ +as Stockbit refusing the session the user signed in with: pass it on, since the user may need \ +to sign in again.")] async fn stockbit_status(&self) -> ToolResult { blocking(login_status).await } diff --git a/crates/idx-mcp/tests/golden/tools.json b/crates/idx-mcp/tests/golden/tools.json index 13ecd5e..97e8a9c 100644 --- a/crates/idx-mcp/tests/golden/tools.json +++ b/crates/idx-mcp/tests/golden/tools.json @@ -689,7 +689,7 @@ "name": "stockbit_login" }, { - "description": "Whether a Stockbit session is stored (or seeded by IDX_STOCKBIT_REFRESH_TOKEN) and when its tokens expire; never the tokens. Without one, get_orderbook fails and stockbit-provider quotes use Stockbit's delayed public page; the user adds one with stockbit_login, or `idx stockbit login` in a terminal. After stockbit_login, `sign_in.state` says whether that sign-in is waiting, signed_in or failed (with `sign_in.error`).", + "description": "Whether a Stockbit session is stored (or seeded by IDX_STOCKBIT_REFRESH_TOKEN) and when its tokens expire; never the tokens. Without one, get_orderbook fails and stockbit-provider quotes use Stockbit's delayed public page; the user adds one with stockbit_login, or `idx stockbit login` in a terminal. After stockbit_login, `sign_in.state` says whether that sign-in is waiting, signed_in or failed (with `sign_in.error`). While it waits, `sign_in.warning` is the latest problem it hit, such as Stockbit refusing the session the user signed in with: pass it on, since the user may need to sign in again.", "inputSchema": { "properties": {}, "type": "object" diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f36c6f0..7adf79b 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -217,7 +217,10 @@ WIB as the fallback. - `api/stockbit/signin.rs` is the one sign-in flow; `idx stockbit login` waits on it in the foreground, and idx-mcp's `stockbit_login` runs it on a thread (`start_background`) that `stockbit_status` reports on (`sign_in`: waiting, - signed_in or failed, with the error's code). A thread does not outlive + signed_in or failed, with the error's code). The thread's warnings go into + `sign_in.warning` through `runtime::on_warning`, since nobody reads + idx-mcp's stderr, and the error that ends a browser wait names a refusal + it hit. A thread does not outlive `main`, so idx-mcp calls `signin::shutdown` when stdin closes or on SIGTERM or Ctrl-C, which closes a waiting browser (SIGKILL cannot). `Method::Auto` uses the browser unless the shell is over SSH or has no display, and falls back to the page diff --git a/web/src/content/docs/cli/stockbit.md b/web/src/content/docs/cli/stockbit.md index 3c2493b..d9b9e14 100644 --- a/web/src/content/docs/cli/stockbit.md +++ b/web/src/content/docs/cli/stockbit.md @@ -175,7 +175,9 @@ a browser window opens on Stockbit's sign-in page (`"method": "browser"`) and you sign in there. Over SSH or without a display, the answer carries the page URL and its `ssh -L` forward (`"method": "page"`) for the agent to hand you. The agent then polls `stockbit_status` until `sign_in.state` is `signed_in` -or `failed`. A session that already works answers `signed_in` without opening +or `failed`. While it waits, `sign_in.warning` carries the latest problem, such +as Stockbit refusing the session you signed in with, so the agent can tell you +to sign in again. A session that already works answers `signed_in` without opening anything, a second call while one is waiting returns it again, and `{"method": "page"}` or `{"method": "browser"}` picks the way. If the server stops while the window waits, the window closes with it. diff --git a/web/src/content/docs/mcp/tools.md b/web/src/content/docs/mcp/tools.md index 863bbc6..a35fc5d 100644 --- a/web/src/content/docs/mcp/tools.md +++ b/web/src/content/docs/mcp/tools.md @@ -201,6 +201,6 @@ Start a Stockbit sign-in and return at once; this call does not wait for it. Wit ## `stockbit_status` -Whether a Stockbit session is stored (or seeded by IDX_STOCKBIT_REFRESH_TOKEN) and when its tokens expire; never the tokens. Without one, get_orderbook fails and stockbit-provider quotes use Stockbit's delayed public page; the user adds one with stockbit_login, or `idx stockbit login` in a terminal. After stockbit_login, `sign_in.state` says whether that sign-in is waiting, signed_in or failed (with `sign_in.error`). +Whether a Stockbit session is stored (or seeded by IDX_STOCKBIT_REFRESH_TOKEN) and when its tokens expire; never the tokens. Without one, get_orderbook fails and stockbit-provider quotes use Stockbit's delayed public page; the user adds one with stockbit_login, or `idx stockbit login` in a terminal. After stockbit_login, `sign_in.state` says whether that sign-in is waiting, signed_in or failed (with `sign_in.error`). While it waits, `sign_in.warning` is the latest problem it hit, such as Stockbit refusing the session the user signed in with: pass it on, since the user may need to sign in again. No inputs.