From dca2b58cbd2269dbaa020a02f0d75e66e9dc5cf5 Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 22:39:41 +0800 Subject: [PATCH 1/4] fix(doctor): exit nonzero on failed checks --- apps/extension/vitest.config.ts | 4 +--- crates/bsk-cli/src/cli/doctor.rs | 21 +++++++++++++++++++++ crates/bsk-cli/src/cli/error.rs | 27 +++++++++++++++++++++++++-- crates/bsk-cli/src/main.rs | 9 ++++++--- crates/bsk-cli/tests/status_cmd.rs | 8 ++++---- 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/apps/extension/vitest.config.ts b/apps/extension/vitest.config.ts index 2e1c157..00060bf 100644 --- a/apps/extension/vitest.config.ts +++ b/apps/extension/vitest.config.ts @@ -21,9 +21,7 @@ export default defineConfig({ }, define: { __BSK_EXT_VERSION__: JSON.stringify(pkg.version), - __BSK_DAEMON_WS_URL__: JSON.stringify( - process.env.BSK_DAEMON_WS_URL ?? "ws://127.0.0.1:52800", - ), + __BSK_DAEMON_WS_URL__: JSON.stringify(process.env.BSK_DAEMON_WS_URL ?? "ws://127.0.0.1:52800"), }, resolve: { alias: { diff --git a/crates/bsk-cli/src/cli/doctor.rs b/crates/bsk-cli/src/cli/doctor.rs index c4aa2a2..fa33718 100644 --- a/crates/bsk-cli/src/cli/doctor.rs +++ b/crates/bsk-cli/src/cli/doctor.rs @@ -104,6 +104,12 @@ pub fn run(output: Output) -> Result> { Ok(checks) } +/// Whether the rendered doctor report contains an active failure. +/// `NotApplicable` remains informational and must not change the exit code. +pub fn has_failures(checks: &[CheckResult]) -> bool { + checks.iter().any(|check| check.status == CheckStatus::Fail) +} + /// Ensure the daemon is reachable and give the browser extension time to /// connect before checks run. Returns a single [`DaemonState`] snapshot /// for check evaluation. @@ -500,6 +506,21 @@ mod m2_tests { ); } + #[test] + fn only_active_failures_make_doctor_unsuccessful() { + let healthy = vec![ + CheckResult::ok("ok", "ready"), + CheckResult::na("optional", "not connected"), + ]; + assert!(!has_failures(&healthy)); + + let unhealthy = vec![ + CheckResult::ok("ok", "ready"), + CheckResult::fail("broken", "not ready", "repair it"), + ]; + assert!(has_failures(&unhealthy)); + } + #[test] fn browsers_check_reports_na_when_no_browser_connected() { // Review M2: 0 browsers means there is nothing to compare diff --git a/crates/bsk-cli/src/cli/error.rs b/crates/bsk-cli/src/cli/error.rs index 4490a81..f8a7b1e 100644 --- a/crates/bsk-cli/src/cli/error.rs +++ b/crates/bsk-cli/src/cli/error.rs @@ -43,6 +43,12 @@ pub enum CliError { #[error("{message}")] Rendered { code: ErrorCode, message: String }, + /// Command already rendered its result and only needs to communicate + /// a command-specific non-zero status. Unlike `Rendered`, this is not + /// pretending that a daemon protocol error occurred. + #[error("command reported an unsuccessful status")] + RenderedExit { exit_code: u8 }, + /// Local transport / setup failure (e.g. couldn't reach the /// daemon, JSON encode failed). Maps to exit code 2. #[error(transparent)] @@ -63,6 +69,7 @@ impl CliError { match self { CliError::Rpc { code, .. } => Some(*code), CliError::Rendered { code, .. } => Some(*code), + CliError::RenderedExit { .. } => None, CliError::Local(_) => None, } } @@ -76,6 +83,9 @@ impl CliError { /// Map this error to the CLI exit code (ยง3.1). pub fn exit_code(&self) -> u8 { + if let CliError::RenderedExit { exit_code } = self { + return *exit_code; + } match self.code() { Some(code) => exit_code_for(code), None => 2, // local / protocol-level transport failure @@ -207,7 +217,10 @@ pub fn render_with_extras( extras: Option<&dyn RenderExtras>, ) -> ExitCode { let exit = err.exit_code(); - if matches!(err, CliError::Rendered { .. }) { + if matches!( + err, + CliError::Rendered { .. } | CliError::RenderedExit { .. } + ) { return ExitCode::from(exit); } let render_info = render_info_for(err); @@ -259,7 +272,10 @@ pub fn render_with_extras( #[cfg(test)] pub(crate) fn render_human_to_string(err: &CliError, extras: Option<&dyn RenderExtras>) -> String { let mut buf: Vec = Vec::new(); - if matches!(err, CliError::Rendered { .. }) { + if matches!( + err, + CliError::Rendered { .. } | CliError::RenderedExit { .. } + ) { return String::new(); } let render_info = render_info_for(err); @@ -340,6 +356,13 @@ mod tests { assert_eq!(err.exit_code(), 3); } + #[test] + fn rendered_exit_uses_command_specific_status_without_protocol_code() { + let err = CliError::RenderedExit { exit_code: 1 }; + assert_eq!(err.code(), None); + assert_eq!(err.exit_code(), 1); + } + /// Review I2: the `--json` payload must be flat at the top level. /// The previous shape nested everything under `"error"` which was /// awkward for shell scripts (`jq -r '.error.code'`) and contradicted diff --git a/crates/bsk-cli/src/main.rs b/crates/bsk-cli/src/main.rs index ab13d10..8053cd9 100644 --- a/crates/bsk-cli/src/main.rs +++ b/crates/bsk-cli/src/main.rs @@ -56,9 +56,12 @@ fn dispatch(cli: Cli, format: Format) -> Result<(), CliError> { } else { Output::Human }; - cli::doctor::run(output) - .map(|_| ()) - .map_err(CliError::Local) + let checks = cli::doctor::run(output).map_err(CliError::Local)?; + if cli::doctor::has_failures(&checks) { + Err(CliError::RenderedExit { exit_code: 1 }) + } else { + Ok(()) + } } Command::InstallSkill(args) => { let output = if cli.flags.json { diff --git a/crates/bsk-cli/tests/status_cmd.rs b/crates/bsk-cli/tests/status_cmd.rs index 13205d9..0e32914 100644 --- a/crates/bsk-cli/tests/status_cmd.rs +++ b/crates/bsk-cli/tests/status_cmd.rs @@ -92,7 +92,7 @@ fn bsk_doctor_runs_without_running_daemon() { .env("BSK_DOCTOR_BROWSER_WAIT_MS", "200") .output() .expect("run bsk doctor"); - assert!(out.status.success()); + assert_eq!(out.status.code(), Some(1)); let stdout = String::from_utf8(out.stdout).unwrap(); assert!( stdout.contains("bsk home writable"), @@ -137,7 +137,7 @@ fn bsk_doctor_json_returns_structured_checks() { .env("BSK_DOCTOR_BROWSER_WAIT_MS", "200") .output() .expect("run bsk doctor --json"); - assert!(out.status.success()); + assert_eq!(out.status.code(), Some(1)); let stdout = String::from_utf8(out.stdout).unwrap(); let parsed: serde_json::Value = serde_json::from_str(stdout.trim()).expect("doctor --json should be valid JSON"); @@ -203,7 +203,7 @@ fn bsk_doctor_does_not_treat_live_non_daemon_pid_as_running() { .env("RUST_LOG", "warn") .output() .expect("run bsk doctor --json"); - assert!(out.status.success()); + assert_eq!(out.status.code(), Some(1)); let stdout = String::from_utf8(out.stdout).unwrap(); let checks: Vec = serde_json::from_str(stdout.trim()).unwrap(); let daemon = checks @@ -262,7 +262,7 @@ fn bsk_doctor_flags_pid_mismatch_against_running_daemon() { .env("RUST_LOG", "warn") .output() .expect("bsk doctor --json"); - assert!(out.status.success()); + assert_eq!(out.status.code(), Some(1)); let stdout = String::from_utf8(out.stdout).unwrap(); let checks: Vec = serde_json::from_str(stdout.trim()).unwrap(); From d422f655d4a76754d2545b704dae349f4407206b Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 22:50:15 +0800 Subject: [PATCH 2/4] ci: retry stalled Rust job From 2964f6219dfe7649b7bbead57f06c13a350314cc Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 22:59:01 +0800 Subject: [PATCH 3/4] ci: retry stalled Rust job From ec2cde446756b0e16521ac70793eb06aea98038f Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 23:06:02 +0800 Subject: [PATCH 4/4] test: bind daemon ports without probe races --- crates/bsk-cli/tests/browser_list_ordering.rs | 5 +---- crates/bsk-cli/tests/browser_wait.rs | 5 +---- crates/bsk-cli/tests/cancel_forwarding.rs | 5 +---- crates/bsk-cli/tests/handshake_compat.rs | 5 +---- crates/bsk-cli/tests/per_session_queue.rs | 5 +---- crates/bsk-cli/tests/session_user_interrupt.rs | 5 +---- crates/bsk-cli/tests/sessions_ipc.rs | 5 +---- crates/bsk-cli/tests/tools_ipc.rs | 5 +---- crates/bsk-cli/tests/tools_m7_ipc.rs | 5 +---- crates/bsk-cli/tests/tools_m8_ipc.rs | 5 +---- crates/bsk-cli/tests/tools_m9_ipc.rs | 5 +---- crates/bsk-cli/tests/ws_handshake.rs | 5 +---- 12 files changed, 12 insertions(+), 48 deletions(-) diff --git a/crates/bsk-cli/tests/browser_list_ordering.rs b/crates/bsk-cli/tests/browser_list_ordering.rs index 1c5199b..f19f320 100644 --- a/crates/bsk-cli/tests/browser_list_ordering.rs +++ b/crates/bsk-cli/tests/browser_list_ordering.rs @@ -16,7 +16,6 @@ use bsk_protocol::system::BrowserStatusEntry; use bsk_protocol::{ErrorCode, Method}; use rand::Rng; use serde::Deserialize; -use tokio::net::TcpListener; use tokio::sync::mpsc; fn tempfile_path(prefix: &str) -> PathBuf { @@ -30,9 +29,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-browser-list"); diff --git a/crates/bsk-cli/tests/browser_wait.rs b/crates/bsk-cli/tests/browser_wait.rs index 632a7e8..16513e0 100644 --- a/crates/bsk-cli/tests/browser_wait.rs +++ b/crates/bsk-cli/tests/browser_wait.rs @@ -15,7 +15,6 @@ use bsk_protocol::system::{BrowserListParams, HandshakeParams, HandshakeResult, use bsk_protocol::{BrowserPeerInfo, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -77,9 +76,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-browser-wait"); diff --git a/crates/bsk-cli/tests/cancel_forwarding.rs b/crates/bsk-cli/tests/cancel_forwarding.rs index b9913ca..e8a0aaf 100644 --- a/crates/bsk-cli/tests/cancel_forwarding.rs +++ b/crates/bsk-cli/tests/cancel_forwarding.rs @@ -20,7 +20,6 @@ use bsk_protocol::{ use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::json; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -40,9 +39,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-cancel"); diff --git a/crates/bsk-cli/tests/handshake_compat.rs b/crates/bsk-cli/tests/handshake_compat.rs index 07e14e4..596d35e 100644 --- a/crates/bsk-cli/tests/handshake_compat.rs +++ b/crates/bsk-cli/tests/handshake_compat.rs @@ -10,7 +10,6 @@ use bsk_protocol::system::{HandshakeParams, HandshakeResult, StatusResult}; use bsk_protocol::{BrowserPeerInfo, ErrorCode, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -28,9 +27,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-handshake"); diff --git a/crates/bsk-cli/tests/per_session_queue.rs b/crates/bsk-cli/tests/per_session_queue.rs index f548ee6..79b5355 100644 --- a/crates/bsk-cli/tests/per_session_queue.rs +++ b/crates/bsk-cli/tests/per_session_queue.rs @@ -27,7 +27,6 @@ use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde::Deserialize; use serde_json::json; -use tokio::net::TcpListener; use tokio::sync::{Mutex, mpsc}; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; @@ -46,9 +45,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-queue"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/session_user_interrupt.rs b/crates/bsk-cli/tests/session_user_interrupt.rs index 2960814..b7d6e61 100644 --- a/crates/bsk-cli/tests/session_user_interrupt.rs +++ b/crates/bsk-cli/tests/session_user_interrupt.rs @@ -26,7 +26,6 @@ use bsk_protocol::{ use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::json; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -46,9 +45,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-user-interrupt"); diff --git a/crates/bsk-cli/tests/sessions_ipc.rs b/crates/bsk-cli/tests/sessions_ipc.rs index a686520..99b1535 100644 --- a/crates/bsk-cli/tests/sessions_ipc.rs +++ b/crates/bsk-cli/tests/sessions_ipc.rs @@ -15,7 +15,6 @@ use bsk_protocol::tools::{SessionStartParams, SessionStartResult, SessionStopPar use bsk_protocol::{BrowserPeerInfo, Frame, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -39,9 +38,7 @@ async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { } async fn spawn_daemon_with_connect_wait(connect_wait: Duration) -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port).with_extension_connect_wait(connect_wait); let sock = tempfile_path("bsk-test-ipc"); diff --git a/crates/bsk-cli/tests/tools_ipc.rs b/crates/bsk-cli/tests/tools_ipc.rs index 32b7732..2857c62 100644 --- a/crates/bsk-cli/tests/tools_ipc.rs +++ b/crates/bsk-cli/tests/tools_ipc.rs @@ -21,7 +21,6 @@ use futures_util::stream::{SplitSink, SplitStream}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::{Value, json}; -use tokio::net::TcpListener; use tokio::sync::Mutex; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; @@ -40,9 +39,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-tools"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/tools_m7_ipc.rs b/crates/bsk-cli/tests/tools_m7_ipc.rs index acb9481..ee74b5e 100644 --- a/crates/bsk-cli/tests/tools_m7_ipc.rs +++ b/crates/bsk-cli/tests/tools_m7_ipc.rs @@ -25,7 +25,6 @@ use futures_util::stream::{SplitSink, SplitStream}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::{Value, json}; -use tokio::net::TcpListener; use tokio::sync::Mutex; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; @@ -44,9 +43,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-tools-m7"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/tools_m8_ipc.rs b/crates/bsk-cli/tests/tools_m8_ipc.rs index 5308f7b..14e7c72 100644 --- a/crates/bsk-cli/tests/tools_m8_ipc.rs +++ b/crates/bsk-cli/tests/tools_m8_ipc.rs @@ -24,7 +24,6 @@ use futures_util::stream::{SplitSink, SplitStream}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::Value; -use tokio::net::TcpListener; use tokio::sync::Mutex; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; @@ -43,9 +42,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-tools-m8"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/tools_m9_ipc.rs b/crates/bsk-cli/tests/tools_m9_ipc.rs index 6dd5d33..2f8b46d 100644 --- a/crates/bsk-cli/tests/tools_m9_ipc.rs +++ b/crates/bsk-cli/tests/tools_m9_ipc.rs @@ -30,7 +30,6 @@ use rand::Rng; use serde_json::{Value, json}; #[cfg(unix)] use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; -use tokio::net::TcpListener; #[cfg(unix)] use tokio::net::UnixStream; use tokio::sync::Mutex; @@ -53,9 +52,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-tools-m9"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/ws_handshake.rs b/crates/bsk-cli/tests/ws_handshake.rs index 146ef33..c6e2da1 100644 --- a/crates/bsk-cli/tests/ws_handshake.rs +++ b/crates/bsk-cli/tests/ws_handshake.rs @@ -8,7 +8,6 @@ use bsk::daemon::{self, DaemonConfig}; use bsk_protocol::system::{HandshakeParams, HandshakeResult}; use bsk_protocol::{BrowserPeerInfo, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -17,9 +16,7 @@ pub const TEST_EXT_ID: &str = "abcdefghijklmnopabcdefghijklmnop"; // 32 chars in pub async fn spawn_daemon() -> daemon::DaemonHandle { // Bind to any free TCP port. - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); daemon::run(config, None).await.unwrap()