mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
merge: integrate main into focus and blur PR
This commit is contained in:
@@ -0,0 +1,75 @@
|
||||
//! Atomic visibility boundary for files received into a same-directory temp.
|
||||
|
||||
use std::path::Path;
|
||||
|
||||
pub fn commit(temp: &Path, out: &Path, overwrite: bool) -> std::io::Result<()> {
|
||||
if !overwrite {
|
||||
std::fs::hard_link(temp, out)?;
|
||||
std::fs::remove_file(temp)?;
|
||||
return Ok(());
|
||||
}
|
||||
replace(temp, out)
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn replace(temp: &Path, out: &Path) -> std::io::Result<()> {
|
||||
// POSIX rename replaces an existing non-directory destination atomically.
|
||||
std::fs::rename(temp, out)
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn replace(temp: &Path, out: &Path) -> std::io::Result<()> {
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use windows_sys::Win32::Storage::FileSystem::{
|
||||
MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH, MoveFileExW,
|
||||
};
|
||||
|
||||
let from: Vec<u16> = temp.as_os_str().encode_wide().chain(Some(0)).collect();
|
||||
let to: Vec<u16> = out.as_os_str().encode_wide().chain(Some(0)).collect();
|
||||
// SAFETY: both buffers are NUL-terminated and remain alive for the call.
|
||||
let ok = unsafe {
|
||||
MoveFileExW(
|
||||
from.as_ptr(),
|
||||
to.as_ptr(),
|
||||
MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH,
|
||||
)
|
||||
};
|
||||
if ok == 0 {
|
||||
Err(std::io::Error::last_os_error())
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(any(unix, windows)))]
|
||||
fn replace(temp: &Path, out: &Path) -> std::io::Result<()> {
|
||||
std::fs::rename(temp, out)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn no_replace_never_overwrites_existing_output() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let temp = dir.path().join("new.part");
|
||||
let out = dir.path().join("out.bin");
|
||||
std::fs::write(&temp, b"new").unwrap();
|
||||
std::fs::write(&out, b"old").unwrap();
|
||||
assert!(commit(&temp, &out, false).is_err());
|
||||
assert_eq!(std::fs::read(&out).unwrap(), b"old");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn overwrite_replaces_existing_output_without_predelete() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let temp = dir.path().join("new.part");
|
||||
let out = dir.path().join("out.bin");
|
||||
std::fs::write(&temp, b"new").unwrap();
|
||||
std::fs::write(&out, b"old").unwrap();
|
||||
commit(&temp, &out, true).unwrap();
|
||||
assert_eq!(std::fs::read(&out).unwrap(), b"new");
|
||||
assert!(!temp.exists());
|
||||
}
|
||||
}
|
||||
@@ -73,6 +73,14 @@ where
|
||||
P: Serialize + Send + 'static,
|
||||
R: DeserializeOwned + Send + 'static,
|
||||
{
|
||||
let parent_cancel = stdin_cancellation();
|
||||
if parent_cancel.as_ref().is_some_and(|rx| *rx.borrow()) {
|
||||
return Err(CliError::from_rpc(RpcError {
|
||||
code: ErrorCode::Cancelled,
|
||||
message: "parent closed the cancellation pipe".into(),
|
||||
data: Some(serde_json::json!({"reason": "cancelled_before_dispatch"})),
|
||||
}));
|
||||
}
|
||||
let rpc_id: RpcId = format!("{}-{}", rpc_id_prefix, random_short_id());
|
||||
let mut client = IpcClient::connect(sock.clone()).await?;
|
||||
let rpc_id_for_call = rpc_id.clone();
|
||||
@@ -88,16 +96,16 @@ where
|
||||
let outcome = tokio::select! {
|
||||
biased;
|
||||
res = &mut main_fut => res?,
|
||||
sig = wait_for_sigint() => {
|
||||
sig.context("install SIGINT handler").map_err(CliError::Local)?;
|
||||
debug!(rpc_id = %rpc_id_for_cancel, "SIGINT: forwarding cancel to daemon");
|
||||
sig = wait_for_cancel(parent_cancel) => {
|
||||
sig.context("wait for cancellation").map_err(CliError::Local)?;
|
||||
debug!(rpc_id = %rpc_id_for_cancel, "forwarding cancel to daemon");
|
||||
let _ = send_cancel(&sock, &rpc_id_for_cancel).await;
|
||||
match tokio::time::timeout(CANCEL_RESPONSE_GRACE, &mut main_fut).await {
|
||||
Ok(res) => res?,
|
||||
Err(_) => {
|
||||
debug!(
|
||||
rpc_id = %rpc_id_for_cancel,
|
||||
"SIGINT: cancel grace elapsed; synthesising cancelled error"
|
||||
"cancel grace elapsed; synthesising cancelled error"
|
||||
);
|
||||
return Err(CliError::from_rpc(RpcError {
|
||||
code: ErrorCode::Cancelled,
|
||||
@@ -112,6 +120,46 @@ where
|
||||
outcome.map_err(CliError::from_rpc)
|
||||
}
|
||||
|
||||
/// Release caller-owned staging even after cancellation. This deliberately
|
||||
/// exposes only transfer.release, not a general bypass for business RPCs.
|
||||
/// The entire batch, including connection setup, shares one bounded budget.
|
||||
pub fn release_transfers<'a>(
|
||||
sock: &Path,
|
||||
transfer_ids: impl IntoIterator<Item = &'a str>,
|
||||
) -> Result<(), CliError> {
|
||||
use bsk_protocol::tools::{TransferIdParams, TransferReleaseResult};
|
||||
const CLEANUP_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
let ids: Vec<_> = transfer_ids.into_iter().collect();
|
||||
if ids.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
let rt = tokio::runtime::Builder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()
|
||||
.context("build runtime for transfer cleanup")?;
|
||||
rt.block_on(async {
|
||||
tokio::time::timeout(CLEANUP_TIMEOUT, async {
|
||||
let mut client = IpcClient::connect(sock).await?;
|
||||
for id in ids {
|
||||
client
|
||||
.call::<_, TransferReleaseResult>(
|
||||
"transfer-release",
|
||||
Method::TransferRelease,
|
||||
Some(TransferIdParams {
|
||||
transfer_id: id.to_string(),
|
||||
}),
|
||||
CLEANUP_TIMEOUT,
|
||||
)
|
||||
.await?
|
||||
.map_err(CliError::from_rpc)?;
|
||||
}
|
||||
Ok::<_, CliError>(())
|
||||
})
|
||||
.await
|
||||
.context("transfer cleanup exceeded its total time budget")?
|
||||
})
|
||||
}
|
||||
|
||||
/// Send a `cancel { rpc_id }` frame over a fresh connection so it
|
||||
/// lands on the daemon while the original call is still parked.
|
||||
///
|
||||
@@ -146,8 +194,57 @@ fn random_short_id() -> String {
|
||||
bytes.iter().map(|b| format!("{b:02x}")).collect()
|
||||
}
|
||||
|
||||
/// Wait for either Ctrl-C or process SIGTERM. Returning here means
|
||||
/// the user has asked us to stop.
|
||||
/// Opt-in control pipe for parent processes that cannot send Ctrl-C (Node on
|
||||
/// Windows). One reader survives the short-lived runtimes used by multi-RPC
|
||||
/// commands such as upload. Ordinary CLI stdin and terminal Ctrl-C are unchanged.
|
||||
fn stdin_cancellation() -> Option<tokio::sync::watch::Receiver<bool>> {
|
||||
if std::env::var("BSK_CANCEL_ON_STDIN_CLOSE").as_deref() != Ok("1") {
|
||||
return None;
|
||||
}
|
||||
static CANCEL: std::sync::OnceLock<tokio::sync::watch::Receiver<bool>> =
|
||||
std::sync::OnceLock::new();
|
||||
Some(
|
||||
CANCEL
|
||||
.get_or_init(|| {
|
||||
let (tx, rx) = tokio::sync::watch::channel(false);
|
||||
std::thread::spawn(move || {
|
||||
use std::io::Read;
|
||||
let mut stdin = std::io::stdin().lock();
|
||||
let mut buf = [0; 64];
|
||||
loop {
|
||||
match stdin.read(&mut buf) {
|
||||
Ok(0) => break,
|
||||
Ok(_) => {}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::Interrupted => continue,
|
||||
Err(_) => break,
|
||||
}
|
||||
}
|
||||
let _ = tx.send(true);
|
||||
});
|
||||
rx
|
||||
})
|
||||
.clone(),
|
||||
)
|
||||
}
|
||||
|
||||
async fn wait_for_cancel(
|
||||
parent_cancel: Option<tokio::sync::watch::Receiver<bool>>,
|
||||
) -> anyhow::Result<()> {
|
||||
let parent_closed = async move {
|
||||
match parent_cancel {
|
||||
Some(mut rx) => {
|
||||
let _ = rx.wait_for(|closed| *closed).await;
|
||||
}
|
||||
None => std::future::pending::<()>().await,
|
||||
}
|
||||
};
|
||||
tokio::select! {
|
||||
result = wait_for_sigint() => result,
|
||||
_ = parent_closed => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Wait for terminal Ctrl-C.
|
||||
async fn wait_for_sigint() -> anyhow::Result<()> {
|
||||
tokio::signal::ctrl_c().await.context("listen for SIGINT")?;
|
||||
Ok(())
|
||||
|
||||
@@ -351,10 +351,10 @@ fn check_version_compatible(status: Option<&StatusResult>) -> CheckResult {
|
||||
}
|
||||
}
|
||||
|
||||
/// `bsk doctor` check: every connected browser should use the same
|
||||
/// protocol version as the daemon. Minor protocol drift is accepted by
|
||||
/// the daemon but flagged here so the user can see who needs updating
|
||||
/// (M10.4).
|
||||
/// `bsk doctor` check: connected browsers should speak a protocol the
|
||||
/// daemon accepts. A different protocol string is still a live
|
||||
/// connection — report Ok so agents keep working. The detail is an
|
||||
/// upgrade reminder, not a blocker.
|
||||
///
|
||||
/// Review M2 (round-1 minor): when no browsers are connected, the
|
||||
/// check has nothing to compare against, so it now reports
|
||||
@@ -396,13 +396,12 @@ fn check_browsers_protocol_compatible(status: Option<&StatusResult>) -> CheckRes
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
CheckResult::fail(
|
||||
CheckResult::ok(
|
||||
name,
|
||||
format!(
|
||||
"{} browser(s) have protocol minor drift from the daemon: {stale}",
|
||||
"{} browser(s) report a different protocol version (still usable — continue, and upgrade soon): {stale}",
|
||||
status.version_skew_browsers.len()
|
||||
),
|
||||
"upgrade the browser-skill extension or bsk CLI so both sides use the same protocol version",
|
||||
)
|
||||
}
|
||||
|
||||
@@ -581,7 +580,7 @@ mod m2_tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn browsers_check_reports_fail_when_skew_present() {
|
||||
fn browsers_check_reports_ok_when_skew_present() {
|
||||
let status = fake_status(
|
||||
vec![BrowserStatusEntry {
|
||||
instance_id: "alpha".into(),
|
||||
@@ -605,9 +604,17 @@ mod m2_tests {
|
||||
}],
|
||||
);
|
||||
let check = check_browsers_protocol_compatible(Some(&status));
|
||||
assert_eq!(check.status, CheckStatus::Fail);
|
||||
assert_eq!(check.status, CheckStatus::Ok);
|
||||
assert!(check.detail.contains("alpha"));
|
||||
assert!(check.hint.is_some());
|
||||
assert!(
|
||||
check
|
||||
.detail
|
||||
.contains("still usable — continue, and upgrade soon")
|
||||
);
|
||||
assert!(
|
||||
check.hint.is_none(),
|
||||
"a protocol-version note must not hint to stop"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -635,7 +642,7 @@ mod m2_tests {
|
||||
}],
|
||||
);
|
||||
let check = check_browsers_protocol_compatible(Some(&status));
|
||||
assert_eq!(check.status, CheckStatus::Fail);
|
||||
assert_eq!(check.status, CheckStatus::Ok);
|
||||
assert!(
|
||||
check
|
||||
.detail
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
//! `bsk download` — capture one browser download and atomically commit it.
|
||||
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Write;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use base64::Engine;
|
||||
use bsk_protocol::Method;
|
||||
use bsk_protocol::tools::{
|
||||
DownloadParams, DownloadResult, TransferChunkParams, TransferChunkResult,
|
||||
};
|
||||
use clap::Args;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::cli::atomic_output;
|
||||
use crate::cli::ensure_daemon::ensure_daemon;
|
||||
use crate::cli::error::{CliError, Format};
|
||||
use crate::cli::interaction::split_target;
|
||||
use crate::cli::navigate::parse_timeout_ms;
|
||||
|
||||
#[derive(Debug, Clone, Args)]
|
||||
pub struct DownloadArgs {
|
||||
/// Snapshot ref (`@e3`) or CSS selector for the download trigger.
|
||||
pub target: Option<String>,
|
||||
#[arg(long = "ref")]
|
||||
pub ref_: Option<String>,
|
||||
#[arg(long = "selector")]
|
||||
pub selector: Option<String>,
|
||||
#[arg(long)]
|
||||
pub out: PathBuf,
|
||||
#[arg(long)]
|
||||
pub session: String,
|
||||
#[arg(long = "tab-id")]
|
||||
pub tab_id: Option<i64>,
|
||||
#[arg(long, default_value = "2m", value_parser = parse_timeout_ms)]
|
||||
pub timeout: u32,
|
||||
#[arg(long)]
|
||||
pub overwrite: bool,
|
||||
}
|
||||
|
||||
pub fn dispatch(args: DownloadArgs, format: Format) -> Result<(), CliError> {
|
||||
if args.out.exists() && !args.overwrite {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"output already exists (pass --overwrite to replace it): {}",
|
||||
args.out.display()
|
||||
)));
|
||||
}
|
||||
let info = ensure_daemon().context("ensure daemon is running")?;
|
||||
let (ref_, selector) = split_target(args.target, args.ref_, args.selector)?;
|
||||
let params = DownloadParams {
|
||||
session_id: args.session,
|
||||
ref_,
|
||||
selector,
|
||||
tab_id: args.tab_id,
|
||||
timeout_ms: Some(args.timeout),
|
||||
browser_relative_dir: None,
|
||||
max_byte_size: None,
|
||||
};
|
||||
let reply: DownloadResult = crate::cli::business_rpc::call(
|
||||
info.sock_path.clone(),
|
||||
"download",
|
||||
Method::ToolDownload,
|
||||
Some(params),
|
||||
ipc_timeout(args.timeout),
|
||||
)?;
|
||||
let transfer_id = reply.transfer_id.clone().ok_or_else(|| {
|
||||
CliError::Local(anyhow::anyhow!("daemon returned no download transfer id"))
|
||||
})?;
|
||||
let write_result = write_transfer(&info.sock_path, &transfer_id, &args.out, args.overwrite);
|
||||
let _ = crate::cli::business_rpc::release_transfers(&info.sock_path, [transfer_id.as_str()]);
|
||||
write_result?;
|
||||
match format {
|
||||
Format::Json => {
|
||||
let mut value = serde_json::to_value(&reply).unwrap();
|
||||
value["path"] = serde_json::json!(args.out.to_string_lossy());
|
||||
if let Some(obj) = value.as_object_mut() {
|
||||
obj.remove("transfer_id");
|
||||
obj.remove("browser_path");
|
||||
}
|
||||
println!("{}", serde_json::to_string_pretty(&value).unwrap());
|
||||
}
|
||||
Format::Human => println!("{}", args.out.display()),
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_transfer(sock: &Path, id: &str, out: &Path, overwrite: bool) -> Result<(), CliError> {
|
||||
let parent = out
|
||||
.parent()
|
||||
.filter(|p| !p.as_os_str().is_empty())
|
||||
.unwrap_or(Path::new("."));
|
||||
let temp = parent.join(format!(".bsk-download-{}.part", Uuid::new_v4().simple()));
|
||||
let result = (|| {
|
||||
let mut file = OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.open(&temp)
|
||||
.with_context(|| format!("create temporary download {}", temp.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
let mut offset = 0u64;
|
||||
loop {
|
||||
let chunk: TransferChunkResult = crate::cli::business_rpc::call(
|
||||
sock.to_path_buf(),
|
||||
"transfer-read",
|
||||
Method::TransferRead,
|
||||
Some(TransferChunkParams {
|
||||
transfer_id: id.to_string(),
|
||||
offset,
|
||||
data_base64: String::new(),
|
||||
}),
|
||||
Duration::from_secs(30),
|
||||
)?;
|
||||
let encoded = chunk.data_base64.as_deref().unwrap_or("");
|
||||
let bytes = base64::engine::general_purpose::STANDARD
|
||||
.decode(encoded)
|
||||
.map_err(|e| CliError::Local(anyhow::anyhow!("decode download chunk: {e}")))?;
|
||||
file.write_all(&bytes)
|
||||
.map_err(|e| CliError::Local(e.into()))?;
|
||||
offset = chunk.next_offset;
|
||||
if chunk.eof {
|
||||
break;
|
||||
}
|
||||
}
|
||||
file.sync_all().map_err(|e| CliError::Local(e.into()))?;
|
||||
drop(file);
|
||||
atomic_output::commit(&temp, out, overwrite)
|
||||
.with_context(|| format!("atomically commit download to {}", out.display()))
|
||||
.map_err(CliError::Local)
|
||||
})();
|
||||
if result.is_err() {
|
||||
let _ = std::fs::remove_file(&temp);
|
||||
}
|
||||
result
|
||||
}
|
||||
|
||||
fn ipc_timeout(timeout_ms: u32) -> Duration {
|
||||
Duration::from_millis(u64::from(timeout_ms) + 5_000)
|
||||
}
|
||||
@@ -73,7 +73,7 @@ pub(crate) fn parse_modifiers(input: &str) -> Result<Vec<KeyModifier>, String> {
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
fn split_target(
|
||||
pub(crate) fn split_target(
|
||||
positional: Option<String>,
|
||||
explicit_ref: Option<String>,
|
||||
explicit_selector: Option<String>,
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use std::time::Duration;
|
||||
|
||||
mod atomic_output;
|
||||
pub mod browser_wait;
|
||||
pub mod browsers;
|
||||
pub mod business_rpc;
|
||||
@@ -9,6 +10,7 @@ pub mod console;
|
||||
pub mod daemon;
|
||||
pub mod dialogs;
|
||||
pub mod doctor;
|
||||
pub mod download;
|
||||
pub mod emulate;
|
||||
pub mod ensure_daemon;
|
||||
pub mod error;
|
||||
@@ -22,6 +24,7 @@ pub mod navigate;
|
||||
pub mod network;
|
||||
pub mod observe;
|
||||
pub mod record;
|
||||
pub mod record_recovery;
|
||||
pub mod record_state;
|
||||
pub mod render_error;
|
||||
pub mod screenshot;
|
||||
@@ -30,6 +33,7 @@ pub mod snapshot;
|
||||
pub mod status;
|
||||
pub mod tab;
|
||||
pub mod update;
|
||||
pub mod upload;
|
||||
pub mod waits;
|
||||
pub mod window;
|
||||
|
||||
@@ -37,6 +41,7 @@ use clap::{Args, Parser, Subcommand};
|
||||
|
||||
use crate::cli::console::ConsoleArgs;
|
||||
use crate::cli::daemon::DaemonCmd;
|
||||
use crate::cli::download::DownloadArgs;
|
||||
use crate::cli::emulate::EmulateArgs;
|
||||
use crate::cli::evaluate::EvaluateArgs;
|
||||
use crate::cli::get_html::GetHtmlArgs;
|
||||
@@ -54,6 +59,7 @@ use crate::cli::session::SessionCmd;
|
||||
use crate::cli::snapshot::SnapshotArgs;
|
||||
use crate::cli::tab::TabCmd;
|
||||
use crate::cli::update::UpdateArgs;
|
||||
use crate::cli::upload::UploadArgs;
|
||||
use crate::cli::waits::{WaitForNavigationArgs, WaitMsArgs};
|
||||
use crate::cli::window::WindowCmd;
|
||||
|
||||
@@ -184,6 +190,12 @@ pub enum Command {
|
||||
/// Set `<select>` option values by `value` attribute.
|
||||
Select(SelectArgs),
|
||||
|
||||
/// Upload files through a page file input or explicit drop target.
|
||||
Upload(UploadArgs),
|
||||
|
||||
/// Capture one browser download and write it to a local path.
|
||||
Download(DownloadArgs),
|
||||
|
||||
/// Evaluate a JavaScript expression inside the Agent Window.
|
||||
Evaluate(EvaluateArgs),
|
||||
|
||||
|
||||
@@ -33,6 +33,12 @@ pub struct ObserveArgs {
|
||||
/// Include conditional surface probe diagnostics in JSON output.
|
||||
#[arg(long = "debug-surfaces")]
|
||||
pub debug_surfaces: bool,
|
||||
|
||||
/// Actively hover page controls to reveal hover-only menus and tooltips.
|
||||
/// Costs seconds of wall clock and touches the live page, so it is off
|
||||
/// unless a static observation looks like it is missing hover content.
|
||||
#[arg(long = "probe-hover")]
|
||||
pub probe_hover: bool,
|
||||
}
|
||||
|
||||
pub fn dispatch(args: ObserveArgs, format: Format) -> Result<(), CliError> {
|
||||
@@ -47,6 +53,7 @@ fn run(sock: PathBuf, args: ObserveArgs, format: Format) -> Result<(), CliError>
|
||||
max_depth: args.max_depth,
|
||||
max_tokens: args.max_tokens,
|
||||
debug_surfaces: args.debug_surfaces,
|
||||
probe_hover: args.probe_hover,
|
||||
};
|
||||
let reply: ObserveResult = call(sock, params)?;
|
||||
match format {
|
||||
|
||||
@@ -1,23 +1,30 @@
|
||||
//! `bsk record start|stop` — capture user actions in the Agent Window.
|
||||
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use bsk_protocol::Method;
|
||||
use bsk_protocol::tools::{
|
||||
RecordAwaitParams, RecordAwaitResult, RecordStartParams, RecordStartResult, RecordStopParams,
|
||||
RecordStopResult, Trace,
|
||||
RecordStopResult, RecordedTrace, TRACE_VERSION_V3,
|
||||
};
|
||||
use bsk_protocol::{ErrorCode, Method};
|
||||
use clap::{Args, Subcommand};
|
||||
|
||||
use crate::cli::TOOL_IPC_TIMEOUT;
|
||||
|
||||
mod export;
|
||||
|
||||
use crate::cli::business_rpc;
|
||||
use crate::cli::ensure_daemon::ensure_daemon;
|
||||
use crate::cli::error::{CliError, Format};
|
||||
use crate::cli::record_recovery;
|
||||
use crate::cli::record_state;
|
||||
use crate::cli::session::{SessionStartOptions, start_session, stop_session};
|
||||
use export::{
|
||||
ExportMeta, export_with_recovery, states_dir_for_output, trace_json_path,
|
||||
validate_record_output,
|
||||
};
|
||||
|
||||
/// Max wait for the user to click 结束 in the browser (24 hours).
|
||||
const RECORD_AWAIT_TIMEOUT_MS: u32 = 86_400_000;
|
||||
@@ -50,7 +57,8 @@ pub struct RecordStartArgs {
|
||||
pub tab_id: Option<i64>,
|
||||
|
||||
/// Navigate to this http(s) URL before recording. When omitted, defaults
|
||||
/// to `https://example.com/`.
|
||||
/// to `https://example.com/`. If that page does not load, pass `--url`
|
||||
/// with a site you can open in this browser.
|
||||
#[arg(long)]
|
||||
pub url: Option<String>,
|
||||
|
||||
@@ -58,15 +66,25 @@ pub struct RecordStartArgs {
|
||||
#[arg(long)]
|
||||
pub purpose: Option<String>,
|
||||
|
||||
/// Output path for the trace JSON (default `./trace.json`).
|
||||
#[arg(long, default_value = "trace.json")]
|
||||
/// Max VOM tokens per page observation file (default 3000).
|
||||
#[arg(long = "max-page-tokens")]
|
||||
pub max_page_tokens: Option<u32>,
|
||||
|
||||
/// Redact all form values in page observations (`[filled]` only).
|
||||
#[arg(long = "redact-values")]
|
||||
pub redact_values: bool,
|
||||
|
||||
/// Output directory for the trace bundle (default `./trace`).
|
||||
/// Writes `<dir>/trace.json` and `<dir>/states/*.txt`.
|
||||
#[arg(long, default_value = "trace")]
|
||||
pub output: PathBuf,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Args)]
|
||||
pub struct RecordStopArgs {
|
||||
/// Output path for the trace JSON (default `./trace.json`).
|
||||
#[arg(long, default_value = "trace.json")]
|
||||
/// Output directory for the trace bundle (default `./trace`).
|
||||
/// Writes `<dir>/trace.json` and `<dir>/states/*.txt`.
|
||||
#[arg(long, default_value = "trace")]
|
||||
pub output: PathBuf,
|
||||
}
|
||||
|
||||
@@ -78,11 +96,7 @@ pub fn dispatch(cmd: RecordCmd, format: Format) -> Result<(), CliError> {
|
||||
}
|
||||
|
||||
fn dispatch_start(args: RecordStartArgs, format: Format) -> Result<(), CliError> {
|
||||
if record_state::read().is_ok() {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"a recording is already in progress; run `bsk record stop` first"
|
||||
)));
|
||||
}
|
||||
prepare_record_start(&args.output)?;
|
||||
|
||||
let info = ensure_daemon().context("ensure daemon is running")?;
|
||||
let session = start_session(
|
||||
@@ -96,8 +110,12 @@ fn dispatch_start(args: RecordStartArgs, format: Format) -> Result<(), CliError>
|
||||
let start_params = RecordStartParams {
|
||||
session_id: session.session_id.clone(),
|
||||
tab_id: args.tab_id,
|
||||
url: args.url,
|
||||
url: args.url.clone(),
|
||||
purpose: args.purpose.clone(),
|
||||
max_page_tokens: args.max_page_tokens,
|
||||
redact_values: Some(args.redact_values),
|
||||
trace_version: Some(TRACE_VERSION_V3),
|
||||
supports_tab_switch_steps: Some(true),
|
||||
};
|
||||
let start_result = business_rpc::call::<RecordStartParams, RecordStartResult>(
|
||||
info.sock_path.clone(),
|
||||
@@ -111,7 +129,7 @@ fn dispatch_start(args: RecordStartArgs, format: Format) -> Result<(), CliError>
|
||||
Ok(result) => result,
|
||||
Err(err) => {
|
||||
let _ = stop_session(info.sock_path, &session.session_id);
|
||||
return Err(err);
|
||||
return Err(annotate_default_start_page_error(err, args.url.as_deref()));
|
||||
}
|
||||
};
|
||||
|
||||
@@ -142,8 +160,8 @@ fn dispatch_start(args: RecordStartArgs, format: Format) -> Result<(), CliError>
|
||||
// Keep write/render inside a Result so `?` cannot skip session teardown.
|
||||
let run_result: Result<(), CliError> = match await_result {
|
||||
Ok(result) => (|| {
|
||||
write_trace_file(&args.output, &result.trace)?;
|
||||
render_finish(&result.trace, &args.output, format)
|
||||
let exported = export_with_recovery(&args.output, &result.trace)?;
|
||||
render_finish(&result.trace, &args.output, &exported, format)
|
||||
})(),
|
||||
Err(err) => Err(err),
|
||||
};
|
||||
@@ -157,11 +175,12 @@ fn dispatch_start(args: RecordStartArgs, format: Format) -> Result<(), CliError>
|
||||
}
|
||||
|
||||
fn dispatch_stop(args: RecordStopArgs, format: Format) -> Result<(), CliError> {
|
||||
let state = record_state::read().map_err(CliError::Local)?;
|
||||
let info = ensure_daemon().context("ensure daemon is running")?;
|
||||
let session_id = state.session_id.clone();
|
||||
validate_record_output(&args.output)?;
|
||||
|
||||
if let Ok(state) = record_state::read() {
|
||||
let info = ensure_daemon().context("ensure daemon is running")?;
|
||||
let session_id = state.session_id.clone();
|
||||
|
||||
let run_result: Result<(), CliError> = (|| {
|
||||
let params = RecordStopParams {
|
||||
session_id: session_id.clone(),
|
||||
};
|
||||
@@ -172,16 +191,85 @@ fn dispatch_stop(args: RecordStopArgs, format: Format) -> Result<(), CliError> {
|
||||
Some(params),
|
||||
TOOL_IPC_TIMEOUT,
|
||||
)?;
|
||||
write_trace_file(&args.output, &result.trace)?;
|
||||
render_stop(&result, &args.output, format)
|
||||
})();
|
||||
|
||||
let session_stop_result = stop_session(info.sock_path, &session_id);
|
||||
record_state::clear();
|
||||
let run_result: Result<(), CliError> = (|| {
|
||||
let exported = export_with_recovery(&args.output, &result.trace)?;
|
||||
render_stop(&result, &args.output, &exported, format)
|
||||
})();
|
||||
|
||||
run_result?;
|
||||
session_stop_result?;
|
||||
Ok(())
|
||||
let session_stop_result = stop_session(info.sock_path, &session_id);
|
||||
record_state::clear();
|
||||
|
||||
run_result?;
|
||||
session_stop_result?;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(trace) = record_recovery::load().map_err(CliError::Local)? else {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"no recording in progress; run `bsk record start` first"
|
||||
)));
|
||||
};
|
||||
|
||||
let exported = export_with_recovery(&args.output, &trace)?;
|
||||
render_finish(&trace, &args.output, &exported, format)
|
||||
}
|
||||
|
||||
/// When `record start` omitted `--url` and the default example.com page
|
||||
/// never loaded, rewrite the RPC error so the CLI hint points at `--url`.
|
||||
fn annotate_default_start_page_error(err: CliError, user_url: Option<&str>) -> CliError {
|
||||
if user_url.is_some() {
|
||||
return err;
|
||||
}
|
||||
let CliError::Rpc {
|
||||
code,
|
||||
message,
|
||||
data,
|
||||
source,
|
||||
} = err
|
||||
else {
|
||||
return err;
|
||||
};
|
||||
if !is_default_start_page_failure(code, &message) {
|
||||
return CliError::Rpc {
|
||||
code,
|
||||
message,
|
||||
data,
|
||||
source,
|
||||
};
|
||||
}
|
||||
CliError::Rpc {
|
||||
code,
|
||||
message,
|
||||
data: Some(serde_json::json!({
|
||||
"reason": crate::cli::render_error::reason::RECORD_START_PAGE_UNREACHABLE
|
||||
})),
|
||||
source,
|
||||
}
|
||||
}
|
||||
|
||||
fn is_default_start_page_failure(code: ErrorCode, message: &str) -> bool {
|
||||
match code {
|
||||
ErrorCode::CdpFailed | ErrorCode::Timeout => true,
|
||||
ErrorCode::InvalidParams => {
|
||||
message.contains("restricted URL") || message.contains("about:blank")
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn prepare_record_start(output: &Path) -> Result<(), CliError> {
|
||||
if record_state::read().is_ok() {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"a recording is already in progress; run `bsk record stop` first"
|
||||
)));
|
||||
}
|
||||
if record_recovery::exists() {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"a previous recording was not exported; run `bsk record stop --output <dir>` to recover it first"
|
||||
)));
|
||||
}
|
||||
validate_record_output(output)
|
||||
}
|
||||
|
||||
fn record_await_ipc_timeout(timeout_ms: u32) -> Duration {
|
||||
@@ -190,39 +278,66 @@ fn record_await_ipc_timeout(timeout_ms: u32) -> Duration {
|
||||
.unwrap_or(Duration::from_secs(u64::from(timeout_ms / 1_000) + 15))
|
||||
}
|
||||
|
||||
fn write_trace_file(output: &PathBuf, trace: &Trace) -> Result<(), CliError> {
|
||||
let json = serde_json::to_string_pretty(trace)
|
||||
.context("serialize trace JSON")
|
||||
.map_err(CliError::Local)?;
|
||||
if let Some(parent) = output.parent() {
|
||||
if !parent.as_os_str().is_empty() {
|
||||
fs::create_dir_all(parent)
|
||||
.with_context(|| format!("create output directory {}", parent.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
}
|
||||
}
|
||||
fs::write(output, format!("{json}\n"))
|
||||
.with_context(|| format!("write trace to {}", output.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn render_finish(trace: &Trace, output: &PathBuf, format: Format) -> Result<(), CliError> {
|
||||
fn render_finish(
|
||||
trace: &RecordedTrace,
|
||||
output_dir: &PathBuf,
|
||||
exported: &ExportMeta,
|
||||
format: Format,
|
||||
) -> Result<(), CliError> {
|
||||
let trace_path = trace_json_path(output_dir);
|
||||
match format {
|
||||
Format::Json => {
|
||||
let payload = match trace {
|
||||
RecordedTrace::V3(t) => serde_json::json!({
|
||||
"output": output_dir,
|
||||
"trace_json": trace_path,
|
||||
"trace_version": exported.trace_version,
|
||||
"states_dir": exported.states_dir,
|
||||
"trace": t,
|
||||
"window_closed": true,
|
||||
}),
|
||||
RecordedTrace::V2(t) => serde_json::json!({
|
||||
"output": output_dir,
|
||||
"trace_json": trace_path,
|
||||
"trace_version": exported.trace_version,
|
||||
"states_dir": null,
|
||||
"trace": t,
|
||||
"window_closed": true,
|
||||
}),
|
||||
};
|
||||
println!(
|
||||
"{}",
|
||||
serde_json::to_string(&serde_json::json!({
|
||||
"output": output,
|
||||
"trace": trace,
|
||||
"window_closed": true,
|
||||
}))
|
||||
.map_err(|e| CliError::Local(anyhow::anyhow!(e)))?
|
||||
serde_json::to_string(&payload).map_err(|e| CliError::Local(anyhow::anyhow!(e)))?
|
||||
);
|
||||
}
|
||||
Format::Human => {
|
||||
println!("saved {} steps to {}", trace.steps.len(), output.display());
|
||||
}
|
||||
Format::Human => match trace {
|
||||
RecordedTrace::V3(t) => {
|
||||
let states_dir = exported
|
||||
.states_dir
|
||||
.as_ref()
|
||||
.map(|p| p.display().to_string())
|
||||
.unwrap_or_else(|| states_dir_for_output(output_dir).display().to_string());
|
||||
println!(
|
||||
"saved {} steps to {} and {} states to {}",
|
||||
t.steps.len(),
|
||||
trace_path.display(),
|
||||
t.states.len(),
|
||||
states_dir
|
||||
);
|
||||
}
|
||||
RecordedTrace::V2(t) => {
|
||||
if exported.v2_fallback {
|
||||
eprintln!(
|
||||
"note: extension returned trace v2 (no page observations); update the BrowserSkill extension for v3 bundles"
|
||||
);
|
||||
}
|
||||
println!(
|
||||
"saved {} steps to {} (trace v2)",
|
||||
t.steps.len(),
|
||||
trace_path.display()
|
||||
);
|
||||
}
|
||||
},
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -230,33 +345,89 @@ fn render_finish(trace: &Trace, output: &PathBuf, format: Format) -> Result<(),
|
||||
fn render_stop(
|
||||
result: &RecordStopResult,
|
||||
output: &PathBuf,
|
||||
exported: &ExportMeta,
|
||||
format: Format,
|
||||
) -> Result<(), CliError> {
|
||||
render_finish(&result.trace, output, format)
|
||||
render_finish(&result.trace, output, exported, format)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn default_output_is_trace_json() {
|
||||
let args = RecordStopArgs {
|
||||
output: PathBuf::from("trace.json"),
|
||||
};
|
||||
assert_eq!(args.output, PathBuf::from("trace.json"));
|
||||
fn with_temp_home<F: FnOnce()>(f: F) {
|
||||
let _lock = record_recovery::test_env_lock();
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
unsafe {
|
||||
std::env::set_var(crate::daemon::paths::BSK_HOME_ENV, tmp.path());
|
||||
}
|
||||
f();
|
||||
record_recovery::clear();
|
||||
record_state::clear();
|
||||
unsafe {
|
||||
std::env::remove_var(crate::daemon::paths::BSK_HOME_ENV);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn start_args_default_output_is_trace_json() {
|
||||
fn prepare_record_start_rejects_existing_trace_json_file() {
|
||||
with_temp_home(|| {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("trace.json");
|
||||
std::fs::write(&output, "{}\n").unwrap();
|
||||
|
||||
let err = prepare_record_start(&output).unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("JSON file") || msg.contains("json file"),
|
||||
"{msg}"
|
||||
);
|
||||
assert!(
|
||||
msg.contains("states/") || msg.contains("--output trace"),
|
||||
"{msg}"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prepare_record_start_rejects_unexported_recovery() {
|
||||
with_temp_home(|| {
|
||||
crate::daemon::paths::ensure_bsk_home().unwrap();
|
||||
std::fs::write(
|
||||
crate::daemon::paths::record_recovery_path().unwrap(),
|
||||
"{}\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let err = prepare_record_start(Path::new("trace")).unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("not exported") || msg.contains("recover"),
|
||||
"{msg}"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_output_is_trace_dir() {
|
||||
let args = RecordStopArgs {
|
||||
output: PathBuf::from("trace"),
|
||||
};
|
||||
assert_eq!(args.output, PathBuf::from("trace"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn start_args_default_output_is_trace_dir() {
|
||||
let args = RecordStartArgs {
|
||||
browser: None,
|
||||
tab_id: None,
|
||||
url: None,
|
||||
purpose: None,
|
||||
output: PathBuf::from("trace.json"),
|
||||
max_page_tokens: None,
|
||||
redact_values: false,
|
||||
output: PathBuf::from("trace"),
|
||||
};
|
||||
assert_eq!(args.output, PathBuf::from("trace.json"));
|
||||
assert_eq!(args.output, PathBuf::from("trace"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -264,4 +435,35 @@ mod tests {
|
||||
let got = record_await_ipc_timeout(RECORD_AWAIT_TIMEOUT_MS);
|
||||
assert!(got >= Duration::from_secs(86_400));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn annotate_default_start_page_error_adds_url_reason() {
|
||||
let err = CliError::from_rpc(bsk_protocol::RpcError {
|
||||
code: ErrorCode::CdpFailed,
|
||||
message: "Page.navigate rejected: net::ERR_ABORTED".into(),
|
||||
data: None,
|
||||
});
|
||||
let annotated = annotate_default_start_page_error(err, None);
|
||||
assert_eq!(
|
||||
crate::cli::render_error::reason_for_data(annotated.data()),
|
||||
Some(crate::cli::render_error::reason::RECORD_START_PAGE_UNREACHABLE)
|
||||
);
|
||||
let info =
|
||||
crate::cli::render_error::info_for_error(annotated.code().unwrap(), annotated.data());
|
||||
assert!(info.hint.unwrap().contains("--url"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn annotate_default_start_page_error_skips_when_user_passed_url() {
|
||||
let err = CliError::from_rpc(bsk_protocol::RpcError {
|
||||
code: ErrorCode::CdpFailed,
|
||||
message: "Page.navigate rejected: net::ERR_ABORTED".into(),
|
||||
data: None,
|
||||
});
|
||||
let annotated = annotate_default_start_page_error(err, Some("https://www.example.org/"));
|
||||
assert_eq!(
|
||||
crate::cli::render_error::reason_for_data(annotated.data()),
|
||||
None
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,976 @@
|
||||
use std::collections::HashSet;
|
||||
use std::fs::{self, OpenOptions};
|
||||
use std::io;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use anyhow::Context;
|
||||
use bsk_protocol::tools::{RecordedTrace, TRACE_VERSION_V2, TRACE_VERSION_V3, TraceV2, TraceV3};
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::cli::error::CliError;
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(super) struct ExportMeta {
|
||||
pub(super) states_dir: Option<PathBuf>,
|
||||
pub(super) trace_version: u32,
|
||||
/// True when the CLI requested v3 but the extension returned legacy v2.
|
||||
pub(super) v2_fallback: bool,
|
||||
}
|
||||
|
||||
pub(super) fn export_recorded_trace(
|
||||
output_dir: &Path,
|
||||
trace: &RecordedTrace,
|
||||
) -> Result<ExportMeta, CliError> {
|
||||
match trace {
|
||||
RecordedTrace::V3(trace) => {
|
||||
let states_dir = write_trace_bundle(output_dir, trace)?;
|
||||
Ok(ExportMeta {
|
||||
states_dir: Some(states_dir),
|
||||
trace_version: TRACE_VERSION_V3,
|
||||
v2_fallback: false,
|
||||
})
|
||||
}
|
||||
RecordedTrace::V2(trace) => {
|
||||
write_trace_v2(output_dir, trace)?;
|
||||
Ok(ExportMeta {
|
||||
states_dir: None,
|
||||
trace_version: TRACE_VERSION_V2,
|
||||
v2_fallback: true,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Save the completed Trace first, then write the bundle. Recovery is
|
||||
/// removed only after the export commits so a bad `--output` cannot drop
|
||||
/// a recording the extension already returned.
|
||||
pub(super) fn export_with_recovery(
|
||||
output_dir: &Path,
|
||||
trace: &RecordedTrace,
|
||||
) -> Result<ExportMeta, CliError> {
|
||||
let save_result = crate::cli::record_recovery::save(trace);
|
||||
match export_recorded_trace(output_dir, trace) {
|
||||
Ok(meta) => {
|
||||
crate::cli::record_recovery::clear();
|
||||
Ok(meta)
|
||||
}
|
||||
Err(export_err) => {
|
||||
let export_err = annotate_recovery_hint(export_err, save_result.is_ok());
|
||||
if let Err(save_err) = save_result {
|
||||
return Err(match export_err {
|
||||
CliError::Local(inner) => CliError::Local(
|
||||
inner.context(format!("also failed to save recovery data: {save_err}")),
|
||||
),
|
||||
other => other,
|
||||
});
|
||||
}
|
||||
Err(export_err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn annotate_recovery_hint(err: CliError, recovery_saved: bool) -> CliError {
|
||||
if !recovery_saved {
|
||||
return err;
|
||||
}
|
||||
match err {
|
||||
CliError::Local(inner) => CliError::Local(inner.context(
|
||||
"export failed; the recorded trace was saved and can be recovered with `bsk record stop --output <dir>`",
|
||||
)),
|
||||
other => other,
|
||||
}
|
||||
}
|
||||
|
||||
fn looks_like_json_output(path: &Path) -> bool {
|
||||
path.extension()
|
||||
.and_then(|ext| ext.to_str())
|
||||
.is_some_and(|ext| ext.eq_ignore_ascii_case("json"))
|
||||
}
|
||||
|
||||
fn legacy_json_output_error(path: &Path) -> CliError {
|
||||
CliError::Local(anyhow::anyhow!(
|
||||
"--output {} is a JSON file path; Trace v3 writes a bundle directory \
|
||||
(`<dir>/trace.json` and `<dir>/states/`), not a single file. \
|
||||
Use `--output trace` or another directory.",
|
||||
path.display()
|
||||
))
|
||||
}
|
||||
|
||||
/// Reject legacy `--output trace.json` (and any existing non-directory)
|
||||
/// before a recording starts, so the user is not blocked only after Finish.
|
||||
pub(super) fn validate_record_output(path: &Path) -> Result<(), CliError> {
|
||||
match fs::symlink_metadata(path) {
|
||||
Ok(metadata) if metadata.file_type().is_symlink() => Err(CliError::Local(anyhow::anyhow!(
|
||||
"--output {} must not be a symlink",
|
||||
path.display()
|
||||
))),
|
||||
Ok(metadata) if metadata.is_dir() => Ok(()),
|
||||
Ok(_) if looks_like_json_output(path) => Err(legacy_json_output_error(path)),
|
||||
Ok(_) => Err(CliError::Local(anyhow::anyhow!(
|
||||
"--output {} is not a directory; Trace v3 writes a bundle directory \
|
||||
(`<dir>/trace.json` and `<dir>/states/`). Use `--output trace`.",
|
||||
path.display()
|
||||
))),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => {
|
||||
if looks_like_json_output(path) {
|
||||
Err(legacy_json_output_error(path))
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
Err(err) => Err(CliError::Local(
|
||||
anyhow::Error::new(err).context(format!("inspect --output {}", path.display())),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
fn acquire_export_lock(output_dir: &Path) -> Result<std::fs::File, CliError> {
|
||||
validate_bundle_directory(output_dir, "output bundle directory")?;
|
||||
fs::create_dir_all(output_dir)
|
||||
.with_context(|| format!("create output dir {}", output_dir.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
let lock_path = output_dir.join(".bsk-record-export.lock");
|
||||
let lock_file = OpenOptions::new()
|
||||
.read(true)
|
||||
.write(true)
|
||||
.create(true)
|
||||
.truncate(false)
|
||||
.open(&lock_path)
|
||||
.with_context(|| format!("open export lock {}", lock_path.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
fs2::FileExt::lock_exclusive(&lock_file)
|
||||
.with_context(|| format!("lock export bundle {}", output_dir.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
Ok(lock_file)
|
||||
}
|
||||
|
||||
pub(super) fn write_trace_v2(output_dir: &Path, trace: &TraceV2) -> Result<(), CliError> {
|
||||
let trace_path = trace_json_path(output_dir);
|
||||
let states_dir = states_dir_for_output(output_dir);
|
||||
let json = serde_json::to_string_pretty(trace)
|
||||
.context("serialize trace JSON")
|
||||
.map_err(CliError::Local)?;
|
||||
|
||||
let _lock = acquire_export_lock(output_dir)?;
|
||||
validate_replaceable_file(&trace_path, "trace JSON")?;
|
||||
|
||||
let transaction_id = uuid::Uuid::new_v4();
|
||||
let staging_dir = output_dir.join(format!(".bsk-record-stage-{transaction_id}"));
|
||||
fs::create_dir_all(&staging_dir)
|
||||
.with_context(|| format!("create staging dir {}", staging_dir.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
|
||||
let staged_trace = staging_dir.join("trace.json");
|
||||
let stage_result: Result<(), CliError> = (|| {
|
||||
fs::write(&staged_trace, format!("{json}\n"))
|
||||
.with_context(|| format!("write staged trace to {}", staged_trace.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
Ok(())
|
||||
})();
|
||||
if let Err(err) = stage_result {
|
||||
let _ = fs::remove_dir_all(&staging_dir);
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
commit_export_transaction(
|
||||
&staging_dir,
|
||||
&states_dir,
|
||||
&HashSet::new(),
|
||||
transaction_id,
|
||||
vec![(
|
||||
staged_trace,
|
||||
trace_path,
|
||||
output_dir.join(format!(".trace.json.{transaction_id}.bak")),
|
||||
)],
|
||||
)?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(super) fn states_dir_for_output(output_dir: &Path) -> PathBuf {
|
||||
output_dir.join("states")
|
||||
}
|
||||
|
||||
pub(super) fn trace_json_path(output_dir: &Path) -> PathBuf {
|
||||
output_dir.join("trace.json")
|
||||
}
|
||||
|
||||
fn is_canonical_state_id(id: &str) -> bool {
|
||||
let Some(number) = id.strip_prefix('s') else {
|
||||
return false;
|
||||
};
|
||||
let mut digits = number.bytes();
|
||||
matches!(digits.next(), Some(b'1'..=b'9')) && digits.all(|byte| byte.is_ascii_digit())
|
||||
}
|
||||
|
||||
fn validate_bundle_directory(path: &Path, description: &str) -> Result<(), CliError> {
|
||||
match fs::symlink_metadata(path) {
|
||||
Ok(metadata) if metadata.file_type().is_symlink() => Err(CliError::Local(anyhow::anyhow!(
|
||||
"{description} {} must not be a symlink",
|
||||
path.display()
|
||||
))),
|
||||
Ok(metadata) if metadata.is_dir() => Ok(()),
|
||||
Ok(_) if looks_like_json_output(path) => Err(legacy_json_output_error(path)),
|
||||
Ok(_) => Err(CliError::Local(anyhow::anyhow!(
|
||||
"{description} {} is not a directory; Trace v3 writes `<dir>/trace.json` and `<dir>/states/`. Use `--output trace`.",
|
||||
path.display()
|
||||
))),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()),
|
||||
Err(err) => Err(CliError::Local(
|
||||
anyhow::Error::new(err).context(format!("inspect {description} {}", path.display())),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_replaceable_file(path: &Path, description: &str) -> Result<(), CliError> {
|
||||
match fs::symlink_metadata(path) {
|
||||
Ok(metadata) if metadata.file_type().is_file() => Ok(()),
|
||||
Ok(_) => Err(CliError::Local(anyhow::anyhow!(
|
||||
"{description} {} is not a regular file",
|
||||
path.display()
|
||||
))),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()),
|
||||
Err(err) => Err(CliError::Local(
|
||||
anyhow::Error::new(err).context(format!("inspect {description} {}", path.display())),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
fn commit_staged_file(staged: &Path, target: &Path, backup: &Path) -> io::Result<Option<PathBuf>> {
|
||||
let old_file = match fs::symlink_metadata(target) {
|
||||
Ok(_) => {
|
||||
fs::rename(target, backup)?;
|
||||
Some(backup.to_path_buf())
|
||||
}
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => None,
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
|
||||
if let Err(err) = fs::rename(staged, target) {
|
||||
if let Some(old_file) = old_file.as_deref() {
|
||||
if let Err(restore_err) = fs::rename(old_file, target) {
|
||||
return Err(io::Error::new(
|
||||
restore_err.kind(),
|
||||
format!(
|
||||
"{err}; additionally failed to restore {}: {restore_err}",
|
||||
target.display()
|
||||
),
|
||||
));
|
||||
}
|
||||
}
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
Ok(old_file)
|
||||
}
|
||||
|
||||
fn rollback_installed_files(installed: &[(PathBuf, Option<PathBuf>)]) -> io::Result<()> {
|
||||
let mut rollback_error = None;
|
||||
for (target, backup) in installed.iter().rev() {
|
||||
match fs::remove_file(target) {
|
||||
Ok(()) => {}
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
|
||||
Err(err) => {
|
||||
rollback_error.get_or_insert(err);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if let Some(backup) = backup {
|
||||
if let Err(err) = fs::rename(backup, target) {
|
||||
rollback_error.get_or_insert(err);
|
||||
}
|
||||
}
|
||||
}
|
||||
match rollback_error {
|
||||
Some(err) => Err(err),
|
||||
None => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
fn is_generated_state_name(name: &str) -> bool {
|
||||
name.strip_suffix(".txt").is_some_and(is_canonical_state_id)
|
||||
}
|
||||
|
||||
fn restore_staged_stale_states(staged: &[(PathBuf, PathBuf)]) -> io::Result<()> {
|
||||
let mut restore_error = None;
|
||||
for (original, backup) in staged.iter().rev() {
|
||||
if let Err(err) = fs::rename(backup, original) {
|
||||
restore_error.get_or_insert(err);
|
||||
}
|
||||
}
|
||||
match restore_error {
|
||||
Some(err) => Err(err),
|
||||
None => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Move stale generated states aside so replacement and cleanup form one
|
||||
/// rollback boundary. User-owned files, including arbitrary non-`sN.txt`, stay.
|
||||
fn stage_stale_states(
|
||||
states_dir: &Path,
|
||||
keep: &HashSet<String>,
|
||||
transaction_id: uuid::Uuid,
|
||||
) -> Result<Vec<(PathBuf, PathBuf)>, CliError> {
|
||||
let entries = match fs::read_dir(states_dir) {
|
||||
Ok(entries) => entries,
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(Vec::new()),
|
||||
Err(err) => {
|
||||
return Err(CliError::Local(
|
||||
anyhow::Error::new(err)
|
||||
.context(format!("read states dir {}", states_dir.display())),
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
let mut staged = Vec::new();
|
||||
for entry in entries {
|
||||
let entry = match entry {
|
||||
Ok(entry) => entry,
|
||||
Err(err) => {
|
||||
let restore_result = restore_staged_stale_states(&staged);
|
||||
let message = match restore_result {
|
||||
Ok(()) => format!("read stale state entry: {err}"),
|
||||
Err(restore_err) => {
|
||||
format!("read stale state entry: {err}; restore also failed: {restore_err}")
|
||||
}
|
||||
};
|
||||
return Err(CliError::Local(anyhow::anyhow!(message)));
|
||||
}
|
||||
};
|
||||
let name = entry.file_name();
|
||||
let Some(name) = name.to_str() else { continue };
|
||||
if !is_generated_state_name(name) || keep.contains(name) {
|
||||
continue;
|
||||
}
|
||||
|
||||
let original = entry.path();
|
||||
let file_type = match entry.file_type() {
|
||||
Ok(file_type) => file_type,
|
||||
Err(err) => {
|
||||
let restore_result = restore_staged_stale_states(&staged);
|
||||
let message = match restore_result {
|
||||
Ok(()) => format!("inspect stale state {}: {err}", original.display()),
|
||||
Err(restore_err) => format!(
|
||||
"inspect stale state {}: {err}; restore also failed: {restore_err}",
|
||||
original.display()
|
||||
),
|
||||
};
|
||||
return Err(CliError::Local(anyhow::anyhow!(message)));
|
||||
}
|
||||
};
|
||||
if file_type.is_dir() {
|
||||
let restore_result = restore_staged_stale_states(&staged);
|
||||
let message = match restore_result {
|
||||
Ok(()) => format!("stale state {} is a directory", original.display()),
|
||||
Err(restore_err) => format!(
|
||||
"stale state {} is a directory; restore also failed: {restore_err}",
|
||||
original.display()
|
||||
),
|
||||
};
|
||||
return Err(CliError::Local(anyhow::anyhow!(message)));
|
||||
}
|
||||
|
||||
let backup = states_dir.join(format!(".{name}.{transaction_id}.stale"));
|
||||
if let Err(err) = fs::rename(&original, &backup) {
|
||||
let restore_result = restore_staged_stale_states(&staged);
|
||||
let message = match restore_result {
|
||||
Ok(()) => format!("stage stale state {}: {err}", original.display()),
|
||||
Err(restore_err) => format!(
|
||||
"stage stale state {}: {err}; restore also failed: {restore_err}",
|
||||
original.display()
|
||||
),
|
||||
};
|
||||
return Err(CliError::Local(anyhow::anyhow!(message)));
|
||||
}
|
||||
staged.push((original, backup));
|
||||
}
|
||||
Ok(staged)
|
||||
}
|
||||
|
||||
fn commit_export_transaction(
|
||||
staging_dir: &Path,
|
||||
states_dir: &Path,
|
||||
keep_states: &HashSet<String>,
|
||||
transaction_id: uuid::Uuid,
|
||||
replacements: Vec<(PathBuf, PathBuf, PathBuf)>,
|
||||
) -> Result<(), CliError> {
|
||||
let mut installed = Vec::with_capacity(replacements.len());
|
||||
for (staged, target, backup) in replacements {
|
||||
match commit_staged_file(&staged, &target, &backup) {
|
||||
Ok(old_file) => installed.push((target, old_file)),
|
||||
Err(err) => {
|
||||
let rollback_result = rollback_installed_files(&installed);
|
||||
let _ = fs::remove_dir_all(staging_dir);
|
||||
let message = match rollback_result {
|
||||
Ok(()) => format!("commit replacement {}: {err}", target.display()),
|
||||
Err(rollback_err) => format!(
|
||||
"commit replacement {}: {err}; rollback also failed: {rollback_err}",
|
||||
target.display()
|
||||
),
|
||||
};
|
||||
return Err(CliError::Local(anyhow::anyhow!(message)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let stale_states = match stage_stale_states(states_dir, keep_states, transaction_id) {
|
||||
Ok(stale) => stale,
|
||||
Err(err) => {
|
||||
let rollback_result = rollback_installed_files(&installed);
|
||||
let _ = fs::remove_dir_all(staging_dir);
|
||||
let message = match rollback_result {
|
||||
Ok(()) => err.to_string(),
|
||||
Err(rollback_err) => {
|
||||
format!("{err}; replacement rollback also failed: {rollback_err}")
|
||||
}
|
||||
};
|
||||
return Err(CliError::Local(anyhow::anyhow!(message)));
|
||||
}
|
||||
};
|
||||
|
||||
for (_, backup) in &installed {
|
||||
if let Some(backup) = backup {
|
||||
let _ = fs::remove_file(backup);
|
||||
}
|
||||
}
|
||||
for (_, backup) in stale_states {
|
||||
let _ = fs::remove_file(backup);
|
||||
}
|
||||
let _ = fs::remove_dir_all(staging_dir);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct BundleState<'a> {
|
||||
id: &'a str,
|
||||
url: &'a str,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
title: Option<&'a str>,
|
||||
page: String,
|
||||
#[serde(skip_serializing_if = "std::ops::Not::not")]
|
||||
truncated: bool,
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct BundleTrace<'a> {
|
||||
version: u32,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
purpose: Option<&'a str>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
started_at: Option<&'a str>,
|
||||
recorded_at: &'a str,
|
||||
stopped_by: &'a bsk_protocol::tools::StopReason,
|
||||
entry: &'a bsk_protocol::tools::TraceEntry,
|
||||
recorder: &'a bsk_protocol::tools::RecorderInfo,
|
||||
states: Vec<BundleState<'a>>,
|
||||
steps: &'a [bsk_protocol::tools::StepV3],
|
||||
}
|
||||
|
||||
pub(super) fn write_trace_bundle(output_dir: &Path, trace: &TraceV3) -> Result<PathBuf, CliError> {
|
||||
let states_dir = states_dir_for_output(output_dir);
|
||||
let trace_path = trace_json_path(output_dir);
|
||||
|
||||
if trace.version != TRACE_VERSION_V3 {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"trace version {} is not supported (expected {})",
|
||||
trace.version,
|
||||
TRACE_VERSION_V3
|
||||
)));
|
||||
}
|
||||
|
||||
let mut state_ids = HashSet::with_capacity(trace.states.len());
|
||||
for state in &trace.states {
|
||||
if !is_canonical_state_id(&state.id) {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"state id {:?} is not canonical (expected s1, s2, ...)",
|
||||
state.id
|
||||
)));
|
||||
}
|
||||
if !state_ids.insert(state.id.as_str()) {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"duplicate state id {:?}",
|
||||
state.id
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
let mut disk_states = Vec::with_capacity(trace.states.len());
|
||||
let mut state_writes = Vec::new();
|
||||
let mut keep = HashSet::new();
|
||||
for state in &trace.states {
|
||||
let filename = format!("{}.txt", state.id);
|
||||
let state_path = states_dir.join(&filename);
|
||||
if state_path.parent() != Some(states_dir.as_path()) {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"state path {} escapes states directory {}",
|
||||
state_path.display(),
|
||||
states_dir.display()
|
||||
)));
|
||||
}
|
||||
keep.insert(filename.clone());
|
||||
state_writes.push((filename.clone(), state.body.as_str()));
|
||||
disk_states.push(BundleState {
|
||||
id: &state.id,
|
||||
url: &state.url,
|
||||
title: state.title.as_deref(),
|
||||
page: filename,
|
||||
truncated: state.truncated,
|
||||
});
|
||||
}
|
||||
|
||||
let disk_trace = BundleTrace {
|
||||
version: trace.version,
|
||||
purpose: trace.purpose.as_deref(),
|
||||
started_at: trace.started_at.as_deref(),
|
||||
recorded_at: &trace.recorded_at,
|
||||
stopped_by: &trace.stopped_by,
|
||||
entry: &trace.entry,
|
||||
recorder: &trace.recorder,
|
||||
states: disk_states,
|
||||
steps: &trace.steps,
|
||||
};
|
||||
let json = serde_json::to_string_pretty(&disk_trace)
|
||||
.context("serialize trace JSON")
|
||||
.map_err(CliError::Local)?;
|
||||
|
||||
validate_bundle_directory(output_dir, "output bundle directory")?;
|
||||
fs::create_dir_all(output_dir)
|
||||
.with_context(|| format!("create output dir {}", output_dir.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
|
||||
validate_bundle_directory(&states_dir, "states directory")?;
|
||||
|
||||
let _lock = acquire_export_lock(output_dir)?;
|
||||
|
||||
fs::create_dir_all(&states_dir)
|
||||
.with_context(|| format!("create states dir {}", states_dir.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
|
||||
for (filename, _) in &state_writes {
|
||||
validate_replaceable_file(&states_dir.join(filename), "state observation")?;
|
||||
}
|
||||
validate_replaceable_file(&trace_path, "trace JSON")?;
|
||||
|
||||
let transaction_id = uuid::Uuid::new_v4();
|
||||
let staging_dir = output_dir.join(format!(".bsk-record-stage-{transaction_id}"));
|
||||
let staging_states = staging_dir.join("states");
|
||||
fs::create_dir_all(&staging_states)
|
||||
.with_context(|| format!("create staging dir {}", staging_states.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
|
||||
let stage_result: Result<(), CliError> = (|| {
|
||||
for (filename, body) in &state_writes {
|
||||
let staged_path = staging_states.join(filename);
|
||||
fs::write(&staged_path, body)
|
||||
.with_context(|| {
|
||||
format!("write staged state observation {}", staged_path.display())
|
||||
})
|
||||
.map_err(CliError::Local)?;
|
||||
}
|
||||
let staged_trace = staging_dir.join("trace.json");
|
||||
fs::write(&staged_trace, format!("{json}\n"))
|
||||
.with_context(|| format!("write staged trace to {}", staged_trace.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
Ok(())
|
||||
})();
|
||||
if let Err(err) = stage_result {
|
||||
let _ = fs::remove_dir_all(&staging_dir);
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
let mut replacements: Vec<(PathBuf, PathBuf, PathBuf)> =
|
||||
Vec::with_capacity(state_writes.len() + 1);
|
||||
for (filename, _) in &state_writes {
|
||||
replacements.push((
|
||||
staging_states.join(filename),
|
||||
states_dir.join(filename),
|
||||
states_dir.join(format!(".{filename}.{transaction_id}.bak")),
|
||||
));
|
||||
}
|
||||
replacements.push((
|
||||
staging_dir.join("trace.json"),
|
||||
trace_path.clone(),
|
||||
output_dir.join(format!(".trace.json.{transaction_id}.bak")),
|
||||
));
|
||||
|
||||
commit_export_transaction(
|
||||
&staging_dir,
|
||||
&states_dir,
|
||||
&keep,
|
||||
transaction_id,
|
||||
replacements,
|
||||
)?;
|
||||
|
||||
Ok(states_dir)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::fs;
|
||||
|
||||
use super::*;
|
||||
use bsk_protocol::tools::{
|
||||
NavigationCause, PageRefV2 as PageRef, RecorderInfo, StepCommonV3 as StepCommon,
|
||||
StepResultV3 as StepResult, StepV3 as Step, StopReason, TRACE_VERSION_V2, TRACE_VERSION_V3,
|
||||
TraceEntry, TraceStateV3, TraceV2, TraceV3, VOM_FORMAT_VERSION,
|
||||
};
|
||||
|
||||
fn sample_trace(state_id: &str, body: &str) -> TraceV3 {
|
||||
TraceV3 {
|
||||
version: TRACE_VERSION_V3,
|
||||
recorded_at: "2026-08-10T02:12:55.360Z".into(),
|
||||
stopped_by: StopReason::UserFinish,
|
||||
entry: TraceEntry {
|
||||
start_url: "https://example.com/".into(),
|
||||
},
|
||||
recorder: RecorderInfo {
|
||||
bsk: "0.1.10".into(),
|
||||
vom: VOM_FORMAT_VERSION,
|
||||
},
|
||||
states: vec![TraceStateV3 {
|
||||
id: state_id.into(),
|
||||
url: "https://example.com/".into(),
|
||||
title: Some("Example".into()),
|
||||
body: body.into(),
|
||||
truncated: false,
|
||||
}],
|
||||
steps: vec![Step::Navigate {
|
||||
common: StepCommon {
|
||||
id: 1,
|
||||
state: state_id.into(),
|
||||
result: StepResult {
|
||||
state: state_id.into(),
|
||||
},
|
||||
},
|
||||
to: "https://example.com/".into(),
|
||||
cause: NavigationCause::UserTyped,
|
||||
}],
|
||||
purpose: None,
|
||||
started_at: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_trace_bundle_writes_dir_layout() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
let trace = sample_trace("s1", "@vom 1\nL1 page");
|
||||
let states_dir = write_trace_bundle(&output, &trace).unwrap();
|
||||
let trace_path = output.join("trace.json");
|
||||
assert!(trace_path.exists());
|
||||
assert_eq!(states_dir, output.join("states"));
|
||||
assert!(states_dir.join("s1.txt").exists());
|
||||
let disk: serde_json::Value =
|
||||
serde_json::from_str(&fs::read_to_string(&trace_path).unwrap()).unwrap();
|
||||
assert_eq!(disk["states"][0]["page"], "s1.txt");
|
||||
assert!(disk["states"][0].get("body").is_none());
|
||||
assert_eq!(
|
||||
fs::read_to_string(states_dir.join("s1.txt")).unwrap(),
|
||||
"@vom 1\nL1 page"
|
||||
);
|
||||
|
||||
// Re-exporting drops states from the previous run but keeps the
|
||||
// directory in place for the concurrent writer.
|
||||
fs::write(states_dir.join("s9.txt"), "stale").unwrap();
|
||||
fs::write(states_dir.join("notes.md"), "keep me").unwrap();
|
||||
fs::write(states_dir.join("reference.txt"), "user file").unwrap();
|
||||
write_trace_bundle(&output, &trace).unwrap();
|
||||
assert!(!states_dir.join("s9.txt").exists());
|
||||
assert!(states_dir.join("s1.txt").exists());
|
||||
assert!(states_dir.join("notes.md").exists());
|
||||
assert_eq!(
|
||||
fs::read_to_string(states_dir.join("reference.txt")).unwrap(),
|
||||
"user file"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_trace_bundle_rejects_traversal_state_id() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
let trace = sample_trace("../escaped", "@vom 1\nL1 page");
|
||||
|
||||
let err = write_trace_bundle(&output, &trace).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("state id"));
|
||||
assert!(!output.join("escaped.txt").exists());
|
||||
assert!(!output.join("trace.json").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_trace_bundle_rejects_duplicate_state_id() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
let mut trace = sample_trace("s1", "@vom 1\nL1 first");
|
||||
trace.states.push(TraceStateV3 {
|
||||
body: "@vom 1\nL1 duplicate".into(),
|
||||
..trace.states[0].clone()
|
||||
});
|
||||
|
||||
let err = write_trace_bundle(&output, &trace).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("duplicate state id"));
|
||||
assert!(!output.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_version_preserves_existing_bundle() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
write_trace_bundle(&output, &sample_trace("s1", "original")).unwrap();
|
||||
let original_json = fs::read(output.join("trace.json")).unwrap();
|
||||
|
||||
let mut replacement = sample_trace("s2", "replacement");
|
||||
replacement.version += 1;
|
||||
let err = write_trace_bundle(&output, &replacement).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("not supported"));
|
||||
assert_eq!(fs::read(output.join("trace.json")).unwrap(), original_json);
|
||||
assert_eq!(
|
||||
fs::read_to_string(output.join("states/s1.txt")).unwrap(),
|
||||
"original"
|
||||
);
|
||||
assert!(!output.join("states/s2.txt").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn state_replacement_failure_preserves_existing_bundle() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
write_trace_bundle(&output, &sample_trace("s1", "original")).unwrap();
|
||||
let original_json = fs::read(output.join("trace.json")).unwrap();
|
||||
fs::write(output.join("states/notes.md"), "keep me").unwrap();
|
||||
fs::create_dir(output.join("states/s2.txt")).unwrap();
|
||||
|
||||
let err = write_trace_bundle(&output, &sample_trace("s2", "replacement")).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("state"));
|
||||
assert_eq!(fs::read(output.join("trace.json")).unwrap(), original_json);
|
||||
assert_eq!(
|
||||
fs::read_to_string(output.join("states/s1.txt")).unwrap(),
|
||||
"original"
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read_to_string(output.join("states/notes.md")).unwrap(),
|
||||
"keep me"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stale_state_cleanup_failure_rolls_back_replacement() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
write_trace_bundle(&output, &sample_trace("s1", "original")).unwrap();
|
||||
let original_json = fs::read(output.join("trace.json")).unwrap();
|
||||
fs::create_dir(output.join("states/s9.txt")).unwrap();
|
||||
|
||||
let err = write_trace_bundle(&output, &sample_trace("s2", "replacement")).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("stale state"));
|
||||
assert_eq!(fs::read(output.join("trace.json")).unwrap(), original_json);
|
||||
assert_eq!(
|
||||
fs::read_to_string(output.join("states/s1.txt")).unwrap(),
|
||||
"original"
|
||||
);
|
||||
assert!(!output.join("states/s2.txt").exists());
|
||||
assert!(output.join("states/s9.txt").is_dir());
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn write_trace_bundle_rejects_symlinked_states_directory() {
|
||||
use std::os::unix::fs::symlink;
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
let external_states = dir.path().join("external-states");
|
||||
fs::create_dir_all(&output).unwrap();
|
||||
fs::create_dir_all(&external_states).unwrap();
|
||||
fs::write(external_states.join("s9.txt"), "external").unwrap();
|
||||
symlink(&external_states, output.join("states")).unwrap();
|
||||
|
||||
let err = write_trace_bundle(&output, &sample_trace("s1", "replacement")).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("symlink"));
|
||||
assert_eq!(
|
||||
fs::read_to_string(external_states.join("s9.txt")).unwrap(),
|
||||
"external"
|
||||
);
|
||||
assert!(!external_states.join("s1.txt").exists());
|
||||
assert!(!output.join("trace.json").exists());
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn write_trace_bundle_rejects_symlinked_output_directory() {
|
||||
use std::os::unix::fs::symlink;
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let external_output = dir.path().join("external-output");
|
||||
let output = dir.path().join("flow");
|
||||
fs::create_dir_all(&external_output).unwrap();
|
||||
fs::write(external_output.join("notes.md"), "external").unwrap();
|
||||
symlink(&external_output, &output).unwrap();
|
||||
|
||||
let err = write_trace_bundle(&output, &sample_trace("s1", "replacement")).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("symlink"));
|
||||
assert_eq!(
|
||||
fs::read_to_string(external_output.join("notes.md")).unwrap(),
|
||||
"external"
|
||||
);
|
||||
assert!(!external_output.join("trace.json").exists());
|
||||
assert!(!external_output.join("states").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_trace_v2_writes_single_json_and_prunes_stale_states() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
fs::create_dir_all(output.join("states")).unwrap();
|
||||
fs::write(output.join("states/s1.txt"), "stale").unwrap();
|
||||
|
||||
let trace = TraceV2 {
|
||||
recorded_at: "2026-08-10T02:12:55.360Z".into(),
|
||||
started_at: None,
|
||||
purpose: None,
|
||||
entry: bsk_protocol::tools::TraceEntry {
|
||||
start_url: "https://example.com/".into(),
|
||||
},
|
||||
pages: vec![PageRef {
|
||||
id: "p1".into(),
|
||||
url: "https://example.com/".into(),
|
||||
title: None,
|
||||
}],
|
||||
steps: vec![],
|
||||
};
|
||||
write_trace_v2(&output, &trace).unwrap();
|
||||
assert!(output.join("trace.json").exists());
|
||||
assert!(!output.join("states/s1.txt").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_trace_v2_stale_cleanup_failure_rolls_back_trace() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
write_trace_bundle(&output, &sample_trace("s1", "original")).unwrap();
|
||||
let original_json = fs::read(output.join("trace.json")).unwrap();
|
||||
fs::create_dir(output.join("states/s9.txt")).unwrap();
|
||||
let trace = TraceV2 {
|
||||
recorded_at: "2026-08-10T02:12:55.360Z".into(),
|
||||
started_at: None,
|
||||
purpose: None,
|
||||
entry: bsk_protocol::tools::TraceEntry {
|
||||
start_url: "https://example.com/".into(),
|
||||
},
|
||||
pages: vec![],
|
||||
steps: vec![],
|
||||
};
|
||||
|
||||
let err = write_trace_v2(&output, &trace).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("stale state"));
|
||||
assert_eq!(fs::read(output.join("trace.json")).unwrap(), original_json);
|
||||
assert_eq!(
|
||||
fs::read_to_string(output.join("states/s1.txt")).unwrap(),
|
||||
"original"
|
||||
);
|
||||
assert!(output.join("states/s9.txt").is_dir());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_recorded_trace_accepts_v2() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("flow");
|
||||
let trace = RecordedTrace::V2(TraceV2 {
|
||||
recorded_at: "2026-08-10T02:12:55.360Z".into(),
|
||||
started_at: None,
|
||||
purpose: None,
|
||||
entry: bsk_protocol::tools::TraceEntry {
|
||||
start_url: "https://example.com/".into(),
|
||||
},
|
||||
pages: vec![],
|
||||
steps: vec![],
|
||||
});
|
||||
let exported = export_recorded_trace(&output, &trace).unwrap();
|
||||
assert_eq!(exported.trace_version, TRACE_VERSION_V2);
|
||||
assert!(exported.states_dir.is_none());
|
||||
assert!(output.join("trace.json").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn existing_trace_json_file_is_rejected_with_bundle_usage() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let output = dir.path().join("trace.json");
|
||||
fs::write(&output, "{}\n").unwrap();
|
||||
|
||||
let err = validate_record_output(&output).unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("JSON file") || msg.contains("json file"),
|
||||
"should name the legacy JSON file usage: {msg}"
|
||||
);
|
||||
assert!(
|
||||
msg.contains("states/") || msg.contains("--output trace"),
|
||||
"should hint at the new bundle directory usage: {msg}"
|
||||
);
|
||||
assert_eq!(fs::read_to_string(&output).unwrap(), "{}\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_trace_json_path_is_rejected_as_legacy_output() {
|
||||
let err = validate_record_output(Path::new("trace.json")).unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("JSON file") || msg.contains("json file"),
|
||||
"{msg}"
|
||||
);
|
||||
assert!(
|
||||
msg.contains("states/") || msg.contains("--output trace"),
|
||||
"{msg}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_failure_keeps_recovery_for_retry() {
|
||||
let _lock = crate::cli::record_recovery::test_env_lock();
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
unsafe {
|
||||
std::env::set_var(crate::daemon::paths::BSK_HOME_ENV, tmp.path());
|
||||
}
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let bad_output = dir.path().join("trace.json");
|
||||
fs::write(&bad_output, "{}\n").unwrap();
|
||||
let trace = RecordedTrace::V3(sample_trace("s1", "@vom 1\nL1 page"));
|
||||
|
||||
let err = export_with_recovery(&bad_output, &trace).unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("recover") || msg.contains("record stop"),
|
||||
"export failure should tell the user how to recover: {msg}"
|
||||
);
|
||||
assert!(
|
||||
crate::cli::record_recovery::exists(),
|
||||
"recovery data must survive a failed bundle export"
|
||||
);
|
||||
let recovered = crate::cli::record_recovery::load()
|
||||
.unwrap()
|
||||
.expect("recovery file");
|
||||
assert_eq!(recovered, trace);
|
||||
assert_eq!(fs::read_to_string(&bad_output).unwrap(), "{}\n");
|
||||
|
||||
let good_output = dir.path().join("bundle");
|
||||
export_with_recovery(&good_output, &recovered).unwrap();
|
||||
assert!(good_output.join("trace.json").exists());
|
||||
assert!(good_output.join("states/s1.txt").exists());
|
||||
assert!(
|
||||
!crate::cli::record_recovery::exists(),
|
||||
"successful export must delete recovery data"
|
||||
);
|
||||
|
||||
unsafe {
|
||||
std::env::remove_var(crate::daemon::paths::BSK_HOME_ENV);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,112 @@
|
||||
//! Persist a completed Trace so a failed bundle export can be retried.
|
||||
|
||||
use std::fs;
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
use bsk_protocol::tools::RecordedTrace;
|
||||
|
||||
use crate::daemon::paths;
|
||||
|
||||
pub fn save(trace: &RecordedTrace) -> Result<()> {
|
||||
let path = paths::record_recovery_path()?;
|
||||
paths::ensure_bsk_home()?;
|
||||
let json = serde_json::to_string_pretty(trace).context("serialize record recovery")?;
|
||||
let tmp = path.with_extension("tmp");
|
||||
fs::write(&tmp, format!("{json}\n")).with_context(|| format!("write {}", tmp.display()))?;
|
||||
fs::rename(&tmp, &path).with_context(|| format!("commit {}", path.display()))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn load() -> Result<Option<RecordedTrace>> {
|
||||
let path = paths::record_recovery_path()?;
|
||||
if !path.exists() {
|
||||
return Ok(None);
|
||||
}
|
||||
let raw = fs::read_to_string(&path).with_context(|| format!("read {}", path.display()))?;
|
||||
let trace = serde_json::from_str(&raw).with_context(|| format!("parse {}", path.display()))?;
|
||||
Ok(Some(trace))
|
||||
}
|
||||
|
||||
pub fn exists() -> bool {
|
||||
paths::record_recovery_path()
|
||||
.map(|path| path.exists())
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
pub fn clear() {
|
||||
if let Ok(path) = paths::record_recovery_path() {
|
||||
let _ = fs::remove_file(path);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn test_env_lock() -> std::sync::MutexGuard<'static, ()> {
|
||||
crate::daemon::paths::test_env_lock()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use bsk_protocol::tools::{
|
||||
RecorderInfo, StopReason, TRACE_VERSION_V3, TraceEntry, TraceStateV3, TraceV3,
|
||||
VOM_FORMAT_VERSION,
|
||||
};
|
||||
|
||||
fn sample_trace() -> RecordedTrace {
|
||||
RecordedTrace::V3(TraceV3 {
|
||||
version: TRACE_VERSION_V3,
|
||||
recorded_at: "2026-08-10T02:12:55.360Z".into(),
|
||||
stopped_by: StopReason::UserFinish,
|
||||
entry: TraceEntry {
|
||||
start_url: "https://example.com/".into(),
|
||||
},
|
||||
recorder: RecorderInfo {
|
||||
bsk: "0.1.10".into(),
|
||||
vom: VOM_FORMAT_VERSION,
|
||||
},
|
||||
states: vec![TraceStateV3 {
|
||||
id: "s1".into(),
|
||||
url: "https://example.com/".into(),
|
||||
title: Some("Example".into()),
|
||||
body: "@vom 1\nL1 page".into(),
|
||||
truncated: false,
|
||||
}],
|
||||
steps: vec![],
|
||||
purpose: None,
|
||||
started_at: None,
|
||||
})
|
||||
}
|
||||
|
||||
fn with_temp_home<F: FnOnce()>(f: F) {
|
||||
let _lock = test_env_lock();
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
unsafe {
|
||||
std::env::set_var(crate::daemon::paths::BSK_HOME_ENV, tmp.path());
|
||||
}
|
||||
f();
|
||||
clear();
|
||||
unsafe {
|
||||
std::env::remove_var(crate::daemon::paths::BSK_HOME_ENV);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_load_round_trips() {
|
||||
with_temp_home(|| {
|
||||
let trace = sample_trace();
|
||||
save(&trace).unwrap();
|
||||
assert!(exists());
|
||||
assert_eq!(load().unwrap(), Some(trace));
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clear_removes_recovery_file() {
|
||||
with_temp_home(|| {
|
||||
save(&sample_trace()).unwrap();
|
||||
clear();
|
||||
assert!(!exists());
|
||||
assert_eq!(load().unwrap(), None);
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -59,15 +59,9 @@ fn started_at_unix_ms() -> String {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::sync::Mutex;
|
||||
|
||||
fn env_guard() -> &'static Mutex<()> {
|
||||
static GUARD: Mutex<()> = Mutex::new(());
|
||||
&GUARD
|
||||
}
|
||||
|
||||
fn with_temp_home<F: FnOnce()>(f: F) {
|
||||
let _lock = env_guard().lock().unwrap_or_else(|e| e.into_inner());
|
||||
let _lock = crate::daemon::paths::test_env_lock();
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
unsafe {
|
||||
std::env::set_var(crate::daemon::paths::BSK_HOME_ENV, tmp.path());
|
||||
|
||||
@@ -43,13 +43,29 @@ pub mod reason {
|
||||
pub const REF_NOT_FOUND: &str = "ref_not_found";
|
||||
pub const SELECTOR_NOT_FOUND: &str = "selector_not_found";
|
||||
pub const TARGET_NOT_FILLABLE: &str = "target_not_fillable";
|
||||
pub const FILL_VALUE_INVALID: &str = "fill_value_invalid";
|
||||
pub const FILL_TARGET_CHANGED: &str = "fill_target_changed";
|
||||
pub const FILL_FOCUS_LOST: &str = "fill_focus_lost";
|
||||
pub const FILL_VALUE_MISMATCH: &str = "fill_value_mismatch";
|
||||
pub const FILL_FAILED: &str = "fill_failed";
|
||||
pub const TARGET_NOT_SELECT: &str = "target_not_select";
|
||||
pub const OPTION_NOT_FOUND: &str = "option_not_found";
|
||||
pub const SINGLE_SELECT_VALUE_COUNT: &str = "single_select_value_count";
|
||||
pub const TAB_NOT_ACTIVE: &str = "tab_not_active";
|
||||
pub const CDP_EXTENSION_ACCESS_DENIED: &str = "cdp_extension_access_denied";
|
||||
pub const BORROW_CONFLICT: &str = "borrow_conflict";
|
||||
pub const SCREENSHOT_CAPTURE_FAILED: &str = "screenshot_capture_failed";
|
||||
pub const FILE_INPUT_PROBE_FAILED: &str = "file_input_probe_failed";
|
||||
pub const FILE_INPUT_NOT_ACTIVATED: &str = "file_input_not_activated";
|
||||
pub const SET_FILE_INPUT_FAILED: &str = "set_file_input_failed";
|
||||
pub const UPLOAD_MECHANISM_UNSUPPORTED: &str = "upload_mechanism_unsupported";
|
||||
pub const FILE_DROP_TARGET_UNAVAILABLE: &str = "file_drop_target_unavailable";
|
||||
pub const FILE_DROP_FAILED: &str = "file_drop_failed";
|
||||
pub const DOWNLOAD_CAPTURE_FAILED: &str = "download_capture_failed";
|
||||
pub const TRANSFER_OUTCOME_UNKNOWN: &str = "transfer_outcome_unknown";
|
||||
pub const TRANSFER_TIMEOUT: &str = "transfer_timeout";
|
||||
pub const SESSION_BUSY: &str = crate::rpc_reason::SESSION_BUSY;
|
||||
pub const RECORD_START_PAGE_UNREACHABLE: &str = "record_start_page_unreachable";
|
||||
}
|
||||
|
||||
/// Per-code rendering metadata. The `summary` is what the CLI prints
|
||||
@@ -196,6 +212,27 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend
|
||||
return base;
|
||||
};
|
||||
match (code, reason) {
|
||||
(ErrorCode::CdpFailed, reason::CDP_EXTENSION_ACCESS_DENIED) => RenderInfo {
|
||||
summary: "Chrome blocked CDP access to another extension's content in this tab",
|
||||
hint: Some(
|
||||
"a web page can contain a restricted extension frame; disable the conflicting extension and reload, or use `bsk navigate <url>` to leave this page; reconnecting BrowserSkill alone does not remove the restriction",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(_, reason::TRANSFER_OUTCOME_UNKNOWN) => RenderInfo {
|
||||
summary: "the file transfer outcome could not be confirmed",
|
||||
hint: Some(
|
||||
"do not retry the transfer: the browser may already have applied it; inspect the page or stop safely",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::Timeout, reason::TRANSFER_TIMEOUT) => RenderInfo {
|
||||
summary: "the file transfer timed out after browser dispatch",
|
||||
hint: Some(
|
||||
"do not retry when effect_state is unknown or committed; inspect the page before taking another action",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::PermissionDenied, reason::ELEMENT_NOT_VISIBLE) => RenderInfo {
|
||||
summary: "target element has no visible geometry",
|
||||
hint: Some(
|
||||
@@ -223,7 +260,42 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend
|
||||
(ErrorCode::InvalidParams, reason::TARGET_NOT_FILLABLE) => RenderInfo {
|
||||
summary: "target element is not fillable",
|
||||
hint: Some(
|
||||
"choose an input, textarea, or contenteditable element from the latest snapshot",
|
||||
"observe the page and choose an enabled, editable text input, textarea, or contenteditable; use the appropriate interaction for other control types",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::InvalidParams, reason::FILL_VALUE_INVALID) => RenderInfo {
|
||||
summary: "the requested text does not fit the field constraints",
|
||||
hint: Some(
|
||||
"check the field's input type and maxlength; use a compatible value without silently truncating or changing the user's intended data",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::CdpFailed, reason::FILL_TARGET_CHANGED) => RenderInfo {
|
||||
summary: "the fill target changed during the action",
|
||||
hint: Some(
|
||||
"observe the current page and field value; if the intended result is missing, use a fresh editable target before retrying",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::CdpFailed, reason::FILL_FOCUS_LOST) => RenderInfo {
|
||||
summary: "the page moved focus away from the fill target",
|
||||
hint: Some(
|
||||
"observe the page for a dialog or rerender and resolve it before retrying; fill manages field focus and does not require bringing the browser to the foreground",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::CdpFailed, reason::FILL_VALUE_MISMATCH) => RenderInfo {
|
||||
summary: "the fill result could not be confirmed",
|
||||
hint: Some(
|
||||
"observe the field before retrying; the page may have formatted the value. Continue if the visible result satisfies the user's intent; otherwise correct the remaining difference. Do not blindly repeat fill or immediately request human help",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::CdpFailed, reason::FILL_FAILED) => RenderInfo {
|
||||
summary: "fill encountered a browser or page-script error",
|
||||
hint: Some(
|
||||
"observe the page before retrying because the field may already have changed; retry only if the intended result is missing and the target is still available",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
@@ -269,6 +341,70 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(
|
||||
ErrorCode::CdpFailed | ErrorCode::Timeout | ErrorCode::InvalidParams,
|
||||
reason::RECORD_START_PAGE_UNREACHABLE,
|
||||
) => RenderInfo {
|
||||
summary: "default start page https://example.com/ could not be opened",
|
||||
hint: Some(
|
||||
"that site may be unreachable from your network. Pass --url with a page you can open, e.g. `bsk record start --url https://<site>/`",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::Unsupported, reason::FILE_INPUT_NOT_ACTIVATED) => RenderInfo {
|
||||
summary: "the upload trigger did not activate a file input",
|
||||
hint: Some(
|
||||
"the page may use a non-input picker such as showOpenFilePicker(); when effect_state is none, re-observe and use `--mode drop` once only for a reliably identified attachment-receiving area — otherwise use `bsk request-help` with the exact original local path",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::Timeout | ErrorCode::CdpFailed, reason::FILE_INPUT_PROBE_FAILED) => {
|
||||
RenderInfo {
|
||||
summary: "the browser upload transaction could not be established",
|
||||
hint: Some(
|
||||
"do not retry the same upload blindly; use `bsk request-help` when human help is available",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
}
|
||||
}
|
||||
(ErrorCode::Timeout | ErrorCode::CdpFailed, reason::SET_FILE_INPUT_FAILED) => RenderInfo {
|
||||
summary: "the browser could not attach the staged file to the input",
|
||||
hint: Some(
|
||||
"check that BrowserSkill has Chrome's 'Allow access to file URLs' permission; otherwise use `bsk request-help`",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::Unsupported, reason::UPLOAD_MECHANISM_UNSUPPORTED) => RenderInfo {
|
||||
summary: "the browser does not support native file-drop upload",
|
||||
hint: Some(
|
||||
"do not retry the same drop; use a standard file-input target or `bsk request-help`",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(
|
||||
ErrorCode::PermissionDenied | ErrorCode::Timeout | ErrorCode::CdpFailed,
|
||||
reason::FILE_DROP_TARGET_UNAVAILABLE,
|
||||
) => RenderInfo {
|
||||
summary: "the file-drop target could not be safely resolved",
|
||||
hint: Some(
|
||||
"rerun observe and select the attachment-receiving editor or drop zone itself; do not guess another target — no file was delivered when effect_state is none",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::Timeout | ErrorCode::CdpFailed, reason::FILE_DROP_FAILED) => RenderInfo {
|
||||
summary: "the browser could not complete the native file drop",
|
||||
hint: Some(
|
||||
"do not retry when effect_state is unknown; observe the page first, otherwise use `bsk request-help`",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
(ErrorCode::CdpFailed, reason::DOWNLOAD_CAPTURE_FAILED) => RenderInfo {
|
||||
summary: "the browser download could not be attributed or completed",
|
||||
hint: Some(
|
||||
"do not retry blindly; the target browser may not expose a reliable download intent signal",
|
||||
),
|
||||
exit_code: base.exit_code,
|
||||
},
|
||||
_ => base,
|
||||
}
|
||||
}
|
||||
@@ -354,6 +490,56 @@ mod tests {
|
||||
ErrorCode::NoBrowserConnected,
|
||||
];
|
||||
|
||||
#[test]
|
||||
fn fill_reasons_provide_recovery_without_changing_exit_codes() {
|
||||
for (code, reason, hint_fragment) in [
|
||||
(
|
||||
ErrorCode::InvalidParams,
|
||||
reason::TARGET_NOT_FILLABLE,
|
||||
"enabled, editable",
|
||||
),
|
||||
(
|
||||
ErrorCode::InvalidParams,
|
||||
reason::FILL_VALUE_INVALID,
|
||||
"maxlength",
|
||||
),
|
||||
(
|
||||
ErrorCode::CdpFailed,
|
||||
reason::FILL_TARGET_CHANGED,
|
||||
"fresh editable target",
|
||||
),
|
||||
(
|
||||
ErrorCode::CdpFailed,
|
||||
reason::FILL_FOCUS_LOST,
|
||||
"does not require bringing the browser",
|
||||
),
|
||||
(
|
||||
ErrorCode::CdpFailed,
|
||||
reason::FILL_VALUE_MISMATCH,
|
||||
"Continue if the visible result satisfies",
|
||||
),
|
||||
(
|
||||
ErrorCode::CdpFailed,
|
||||
reason::FILL_FAILED,
|
||||
"may already have changed",
|
||||
),
|
||||
] {
|
||||
let data = serde_json::json!({ "reason": reason });
|
||||
let info = info_for_error(code, Some(&data));
|
||||
assert!(info.hint.unwrap().contains(hint_fragment), "{reason}");
|
||||
assert_eq!(info.exit_code, info_for(code).exit_code);
|
||||
}
|
||||
let mismatch = serde_json::json!({ "reason": reason::FILL_VALUE_MISMATCH });
|
||||
let info = info_for_error(ErrorCode::CdpFailed, Some(&mismatch));
|
||||
assert_eq!(info.summary, "the fill result could not be confirmed");
|
||||
assert!(info.hint.unwrap().contains("Do not blindly repeat fill"));
|
||||
let unknown = serde_json::json!({ "reason": "future_fill_reason" });
|
||||
assert_eq!(
|
||||
info_for_error(ErrorCode::CdpFailed, Some(&unknown)),
|
||||
info_for(ErrorCode::CdpFailed)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn element_not_visible_overrides_permission_denied_copy() {
|
||||
let data = serde_json::json!({ "reason": reason::ELEMENT_NOT_VISIBLE });
|
||||
@@ -366,6 +552,18 @@ mod tests {
|
||||
assert!(!info.summary.contains("sandbox"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extension_access_denied_explains_the_frame_restriction_and_navigation_recovery() {
|
||||
let data = serde_json::json!({ "reason": reason::CDP_EXTENSION_ACCESS_DENIED });
|
||||
let info = info_for_error(ErrorCode::CdpFailed, Some(&data));
|
||||
assert!(info.summary.contains("another extension"));
|
||||
let hint = info.hint.unwrap();
|
||||
assert!(hint.contains("restricted extension frame"));
|
||||
assert!(hint.contains("bsk navigate <url>"));
|
||||
assert!(hint.contains("disable the conflicting extension"));
|
||||
assert_eq!(info.exit_code, 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_busy_overrides_timeout_copy() {
|
||||
let data = serde_json::json!({ "reason": reason::SESSION_BUSY });
|
||||
@@ -378,6 +576,22 @@ mod tests {
|
||||
assert_eq!(info.exit_code, 4);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn record_start_page_unreachable_overrides_cdp_failed_copy() {
|
||||
let data = serde_json::json!({ "reason": reason::RECORD_START_PAGE_UNREACHABLE });
|
||||
let info = info_for_error(ErrorCode::CdpFailed, Some(&data));
|
||||
assert_eq!(
|
||||
info.summary,
|
||||
"default start page https://example.com/ could not be opened"
|
||||
);
|
||||
assert!(
|
||||
info.hint.unwrap().contains("--url"),
|
||||
"expected --url hint, got {:?}",
|
||||
info.hint
|
||||
);
|
||||
assert_eq!(info.exit_code, 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn screenshot_capture_failed_overrides_cdp_failed_copy() {
|
||||
let data = serde_json::json!({ "reason": reason::SCREENSHOT_CAPTURE_FAILED });
|
||||
@@ -392,6 +606,64 @@ mod tests {
|
||||
assert_eq!(info.exit_code, 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_transfer_reasons_render_actionable_fallbacks() {
|
||||
let unsupported = serde_json::json!({ "reason": reason::FILE_INPUT_NOT_ACTIVATED });
|
||||
let info = info_for_error(ErrorCode::Unsupported, Some(&unsupported));
|
||||
assert_eq!(
|
||||
info.summary,
|
||||
"the upload trigger did not activate a file input"
|
||||
);
|
||||
let hint = info.hint.unwrap();
|
||||
assert!(hint.contains("--mode drop"));
|
||||
assert!(hint.contains("reliably identified attachment-receiving area"));
|
||||
assert!(hint.contains("exact original local path"));
|
||||
|
||||
let control = serde_json::json!({ "reason": reason::FILE_INPUT_PROBE_FAILED });
|
||||
let info = info_for_error(ErrorCode::Timeout, Some(&control));
|
||||
assert!(
|
||||
info.summary
|
||||
.contains("transaction could not be established")
|
||||
);
|
||||
assert!(info.hint.unwrap().contains("do not retry"));
|
||||
|
||||
let unsupported_drop =
|
||||
serde_json::json!({ "reason": reason::UPLOAD_MECHANISM_UNSUPPORTED });
|
||||
let info = info_for_error(ErrorCode::Unsupported, Some(&unsupported_drop));
|
||||
assert_eq!(
|
||||
info.summary,
|
||||
"the browser does not support native file-drop upload"
|
||||
);
|
||||
assert!(info.hint.unwrap().contains("standard file-input"));
|
||||
|
||||
let unavailable_target =
|
||||
serde_json::json!({ "reason": reason::FILE_DROP_TARGET_UNAVAILABLE });
|
||||
let info = info_for_error(ErrorCode::PermissionDenied, Some(&unavailable_target));
|
||||
assert_eq!(
|
||||
info.summary,
|
||||
"the file-drop target could not be safely resolved"
|
||||
);
|
||||
assert!(info.hint.unwrap().contains("effect_state is none"));
|
||||
|
||||
let failed_drop = serde_json::json!({ "reason": reason::FILE_DROP_FAILED });
|
||||
let info = info_for_error(ErrorCode::Timeout, Some(&failed_drop));
|
||||
assert!(info.summary.contains("native file drop"));
|
||||
assert!(info.hint.unwrap().contains("effect_state is unknown"));
|
||||
|
||||
let download = serde_json::json!({ "reason": reason::DOWNLOAD_CAPTURE_FAILED });
|
||||
let info = info_for_error(ErrorCode::CdpFailed, Some(&download));
|
||||
assert!(info.summary.contains("download could not be attributed"));
|
||||
|
||||
let unknown = serde_json::json!({ "reason": reason::TRANSFER_OUTCOME_UNKNOWN });
|
||||
let info = info_for_error(ErrorCode::ProtocolError, Some(&unknown));
|
||||
assert!(info.summary.contains("outcome could not be confirmed"));
|
||||
assert!(info.hint.unwrap().contains("do not retry"));
|
||||
|
||||
let timeout = serde_json::json!({ "reason": reason::TRANSFER_TIMEOUT });
|
||||
let info = info_for_error(ErrorCode::Timeout, Some(&timeout));
|
||||
assert!(info.summary.contains("timed out after browser dispatch"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ref_not_found_overrides_not_found_copy() {
|
||||
let data = serde_json::json!({ "reason": reason::REF_NOT_FOUND });
|
||||
|
||||
@@ -65,7 +65,7 @@ fn render_human(s: &StatusResult) {
|
||||
let bold = "\x1b[1m";
|
||||
let reset = "\x1b[0m";
|
||||
eprintln!(
|
||||
"{bold}{yellow}warning:{reset} {} browser(s) have protocol version drift from the daemon{reset}",
|
||||
"{bold}{yellow}note:{reset} {} browser(s) report a different protocol version; still usable — continue, and run `bsk update` soon{reset}",
|
||||
s.version_skew_browsers.len()
|
||||
);
|
||||
for skew in &s.version_skew_browsers {
|
||||
@@ -75,7 +75,7 @@ fn render_human(s: &StatusResult) {
|
||||
skew.label.clone()
|
||||
};
|
||||
eprintln!(
|
||||
" {} ({}) — protocol ext {} vs daemon {} (app ext v{}, daemon v{}) — please align protocol versions",
|
||||
" {} ({}) — protocol ext {} vs CLI {} (app ext v{}, CLI v{}) — still usable; please upgrade",
|
||||
skew.instance_id,
|
||||
label,
|
||||
display_protocol(&skew.client_protocol_version),
|
||||
|
||||
@@ -6,9 +6,7 @@ use std::path::{Path, PathBuf};
|
||||
use std::time::{Duration, SystemTime};
|
||||
|
||||
#[cfg(windows)]
|
||||
use std::os::windows::process::CommandExt;
|
||||
#[cfg(windows)]
|
||||
use std::process::{Command, Stdio};
|
||||
mod windows;
|
||||
|
||||
use anyhow::{Context, Result, bail};
|
||||
use clap::Args;
|
||||
@@ -363,7 +361,8 @@ fn install_candidate_with_client(
|
||||
crate::daemon::start::run_stop().context("stop bsk daemon before update")?;
|
||||
}
|
||||
|
||||
let action = replace_binary_for_update(&target, &binary, daemon_was_running)?;
|
||||
let restart_args = daemon_was_running.then(StartArgs::default);
|
||||
let action = replace_binary_for_update(&target, &binary, restart_args.as_ref())?;
|
||||
|
||||
if daemon_was_running && matches!(action, InstallAction::Replaced) {
|
||||
crate::daemon::start::run_start(StartArgs::default())
|
||||
@@ -394,8 +393,8 @@ pub(crate) fn download_candidate_binary(
|
||||
|
||||
/// Daemon-side install: download, verify, and replace the executable at
|
||||
/// `target` (on Windows: stage the replacement next to it). Unlike the
|
||||
/// CLI path this never stops or starts the daemon — the daemon drives
|
||||
/// its own restart once the binary is replaced.
|
||||
/// CLI path this never stops the daemon. On Windows the helper starts
|
||||
/// its replacement after exit; elsewhere the daemon drives its own restart.
|
||||
///
|
||||
/// `target` must be captured *before* any replacement happens: on Linux
|
||||
/// `std::env::current_exe` starts returning a ` (deleted)`-suffixed
|
||||
@@ -403,10 +402,11 @@ pub(crate) fn download_candidate_binary(
|
||||
pub(crate) fn self_install_candidate(
|
||||
candidate: &UpdateCandidate,
|
||||
target: &Path,
|
||||
restart_args: &StartArgs,
|
||||
) -> Result<InstallAction> {
|
||||
let client = update_http_client(ARCHIVE_FETCH_TIMEOUT)?;
|
||||
let binary = download_candidate_binary(candidate, &client)?;
|
||||
replace_binary_at_path(target, &binary)
|
||||
replace_binary_for_update(target, &binary, Some(restart_args))
|
||||
}
|
||||
|
||||
/// Outcome of one daemon auto-update step (see [`auto_update_step`]).
|
||||
@@ -423,8 +423,8 @@ pub(crate) enum AutoUpdateOutcome {
|
||||
/// The new binary replaced the old one; the daemon should now
|
||||
/// restart into it.
|
||||
Replaced { latest: String },
|
||||
/// Windows staged the replacement next to the running binary; a
|
||||
/// daemon/terminal restart finishes it.
|
||||
/// Windows started a helper that will replace the executable and
|
||||
/// restart the daemon once the current process exits.
|
||||
Staged { latest: String },
|
||||
}
|
||||
|
||||
@@ -668,6 +668,7 @@ fn render_report(format: Format, report: &UpdateReport) -> Result<()> {
|
||||
}
|
||||
if matches!(report.install_action, Some("staged")) {
|
||||
println!("the detached update helper will apply the replacement after exit");
|
||||
println!("if it fails, see the *.update-*.log file next to the executable");
|
||||
}
|
||||
}
|
||||
Format::Json => {
|
||||
@@ -688,22 +689,22 @@ pub fn extract_bsk_binary(archive_bytes: &[u8], kind: ArchiveKind) -> Result<Vec
|
||||
}
|
||||
|
||||
pub fn replace_binary_at_path(target: &Path, binary: &[u8]) -> Result<InstallAction> {
|
||||
replace_binary_for_update(target, binary, false)
|
||||
replace_binary_for_update(target, binary, None)
|
||||
}
|
||||
|
||||
fn replace_binary_for_update(
|
||||
target: &Path,
|
||||
binary: &[u8],
|
||||
restart_daemon: bool,
|
||||
restart_args: Option<&StartArgs>,
|
||||
) -> Result<InstallAction> {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
stage_windows_replacement(target, binary, restart_daemon)
|
||||
stage_windows_replacement(target, binary, restart_args)
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let _ = restart_daemon;
|
||||
let _ = restart_args;
|
||||
replace_binary_atomically(target, binary)
|
||||
}
|
||||
}
|
||||
@@ -749,33 +750,69 @@ fn replace_binary_atomically(target: &Path, binary: &[u8]) -> Result<InstallActi
|
||||
fn stage_windows_replacement(
|
||||
target: &Path,
|
||||
binary: &[u8],
|
||||
restart_daemon: bool,
|
||||
restart_args: Option<&StartArgs>,
|
||||
) -> Result<InstallAction> {
|
||||
let paths = staged_replacement_paths(target, std::process::id())?;
|
||||
std::fs::write(&paths.binary_path, binary)
|
||||
.with_context(|| format!("write {}", paths.binary_path.display()))?;
|
||||
std::fs::write(
|
||||
&paths.script_path,
|
||||
windows_replacement_script(target, &paths.binary_path, restart_daemon),
|
||||
)
|
||||
.with_context(|| format!("write {}", paths.script_path.display()))?;
|
||||
// The helper owns replacement/restart after this process exits. Do not
|
||||
// report Staged until it has actually begun executing the script.
|
||||
let _child = spawn_windows_replacement(target, binary, restart_args, 120)?;
|
||||
Ok(InstallAction::Staged)
|
||||
}
|
||||
|
||||
const CREATE_NO_WINDOW: u32 = 0x0800_0000;
|
||||
let command = format!("\"{}\"", paths.script_path.display());
|
||||
if let Err(err) = Command::new("cmd.exe")
|
||||
.args(["/D", "/S", "/C"])
|
||||
.arg(command)
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.creation_flags(CREATE_NO_WINDOW)
|
||||
.spawn()
|
||||
{
|
||||
#[cfg(windows)]
|
||||
fn spawn_windows_replacement(
|
||||
target: &Path,
|
||||
binary: &[u8],
|
||||
restart_args: Option<&StartArgs>,
|
||||
attempts: u32,
|
||||
) -> Result<windows::Helper> {
|
||||
let paths = staged_replacement_paths(target, std::process::id())?;
|
||||
let ready_path = paths.script_path.with_extension("ready");
|
||||
let log_path = paths.script_path.with_extension("log");
|
||||
let result = (|| {
|
||||
// A previous failed attempt by this process must not acknowledge a
|
||||
// new helper. All paths are private to this target and process id.
|
||||
match std::fs::remove_file(&ready_path) {
|
||||
Ok(()) => {}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
|
||||
Err(err) => return Err(err.into()),
|
||||
}
|
||||
std::fs::write(&paths.binary_path, binary)
|
||||
.with_context(|| format!("write {}", paths.binary_path.display()))?;
|
||||
std::fs::write(
|
||||
&paths.script_path,
|
||||
windows_replacement_script(restart_args, attempts),
|
||||
)
|
||||
.with_context(|| format!("write {}", paths.script_path.display()))?;
|
||||
let mut child = windows::spawn(
|
||||
&paths.script_path,
|
||||
&paths.binary_path,
|
||||
target,
|
||||
&ready_path,
|
||||
&log_path,
|
||||
)
|
||||
.context("launch detached Windows update helper")?;
|
||||
let deadline = std::time::Instant::now() + Duration::from_secs(5);
|
||||
loop {
|
||||
if std::fs::remove_file(&ready_path).is_ok() {
|
||||
return Ok(child);
|
||||
}
|
||||
if let Some(status) = child.try_wait()? {
|
||||
bail!("Windows update helper exited before becoming ready: {status}");
|
||||
}
|
||||
if std::time::Instant::now() >= deadline {
|
||||
let _ = child.kill();
|
||||
let _ = child.wait();
|
||||
bail!("Windows update helper did not become ready within 5 seconds");
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(20));
|
||||
}
|
||||
})();
|
||||
if result.is_err() {
|
||||
let _ = std::fs::remove_file(&paths.binary_path);
|
||||
let _ = std::fs::remove_file(&paths.script_path);
|
||||
return Err(err).context("launch detached Windows update helper");
|
||||
let _ = std::fs::remove_file(&ready_path);
|
||||
}
|
||||
Ok(InstallAction::Staged)
|
||||
result.with_context(|| format!("Windows update helper failed; see {}", log_path.display()))
|
||||
}
|
||||
|
||||
pub fn staged_replacement_paths(target: &Path, pid: u32) -> Result<StagedReplacementPaths> {
|
||||
@@ -792,25 +829,43 @@ pub fn staged_replacement_paths(target: &Path, pid: u32) -> Result<StagedReplace
|
||||
}
|
||||
|
||||
#[cfg(any(windows, test))]
|
||||
fn windows_replacement_script(target: &Path, staged_binary: &Path, restart_daemon: bool) -> String {
|
||||
let target = target.display().to_string().replace('%', "%%");
|
||||
let staged_binary = staged_binary.display().to_string().replace('%', "%%");
|
||||
let restart = if restart_daemon {
|
||||
format!("start \"\" /B \"{target}\" daemon start >nul 2>nul\r\n")
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
fn windows_replacement_script(restart_args: Option<&StartArgs>, attempts: u32) -> String {
|
||||
// Only fixed switches and numeric values go into the ASCII script. Paths
|
||||
// stay in Unicode environment variables, including %, ! and shell symbols.
|
||||
let restart = restart_args.map_or_else(String::new, |args| {
|
||||
format!(
|
||||
"\"%BSK_UPDATE_TARGET%\" daemon start --port {} --session-idle {}ms --daemon-idle {}ms\r\nif errorlevel 1 goto failed_restart\r\n",
|
||||
args.resolved_port(),
|
||||
args.resolved_session_idle().as_millis(),
|
||||
args.resolved_daemon_idle().as_millis(),
|
||||
)
|
||||
});
|
||||
format!(
|
||||
"@echo off\r\n\
|
||||
setlocal\r\n\
|
||||
setlocal DisableDelayedExpansion\r\n\
|
||||
> \"%BSK_UPDATE_READY%\" echo ready\r\n\
|
||||
if errorlevel 1 exit /b 1\r\n\
|
||||
set /a attempts=0\r\n\
|
||||
:retry\r\n\
|
||||
move /Y \"{staged_binary}\" \"{target}\" >nul 2>nul\r\n\
|
||||
if errorlevel 1 (\r\n\
|
||||
timeout /t 1 /nobreak >nul\r\n\
|
||||
goto retry\r\n\
|
||||
)\r\n\
|
||||
move /Y \"%BSK_UPDATE_SOURCE%\" \"%BSK_UPDATE_TARGET%\" >nul\r\n\
|
||||
if not errorlevel 1 goto replaced\r\n\
|
||||
set /a attempts+=1\r\n\
|
||||
if %attempts% geq {attempts} goto failed_replace\r\n\
|
||||
if not exist \"%BSK_UPDATE_SOURCE%\" goto failed_replace\r\n\
|
||||
ping -n 2 127.0.0.1 >nul\r\n\
|
||||
goto retry\r\n\
|
||||
:failed_replace\r\n\
|
||||
echo Failed to replace the executable after %attempts% attempts.\r\n\
|
||||
goto failed\r\n\
|
||||
:replaced\r\n\
|
||||
{restart}\
|
||||
del /F /Q \"%~f0\" >nul 2>nul\r\n"
|
||||
del /F /Q \"%BSK_UPDATE_LOG%\" >nul 2>nul\r\n\
|
||||
(del /F /Q \"%BSK_UPDATE_SCRIPT%\" >nul 2>nul & exit 0)\r\n\
|
||||
:failed_restart\r\n\
|
||||
echo Replaced the executable but failed to restart the daemon.\r\n\
|
||||
:failed\r\n\
|
||||
del /F /Q \"%BSK_UPDATE_SOURCE%\" >nul 2>nul\r\n\
|
||||
(del /F /Q \"%BSK_UPDATE_SCRIPT%\" >nul 2>nul & exit 1)\r\n"
|
||||
)
|
||||
}
|
||||
|
||||
@@ -964,6 +1019,7 @@ mod tests {
|
||||
assert_eq!(extracted, b"windows binary");
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
#[test]
|
||||
fn replaces_binary_at_path() {
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
@@ -998,29 +1054,139 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_replacement_script_retries_restarts_and_cleans_itself() {
|
||||
let script = windows_replacement_script(
|
||||
Path::new(r"C:\Program Files\bsk.exe"),
|
||||
Path::new(r"C:\Program Files\bsk.exe.update-42"),
|
||||
true,
|
||||
fn windows_replacement_script_preserves_restart_config() {
|
||||
let args = StartArgs {
|
||||
port: Some(54321),
|
||||
session_idle: Some(Duration::from_millis(1234)),
|
||||
daemon_idle: Some(Duration::from_secs(75)),
|
||||
..Default::default()
|
||||
};
|
||||
let script = windows_replacement_script(Some(&args), 120);
|
||||
assert!(script.is_ascii());
|
||||
assert!(
|
||||
script
|
||||
.contains("daemon start --port 54321 --session-idle 1234ms --daemon-idle 75000ms")
|
||||
);
|
||||
|
||||
assert!(script.contains(":retry"));
|
||||
assert!(script.contains("move /Y"));
|
||||
assert!(script.contains(r#"start "" /B "C:\Program Files\bsk.exe" daemon start"#));
|
||||
assert!(script.contains(r#"del /F /Q "%~f0""#));
|
||||
assert!(!windows_replacement_script(None, 120).contains("daemon start"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_replacement_script_omits_restart_when_daemon_was_not_running() {
|
||||
let script = windows_replacement_script(
|
||||
Path::new(r"C:\bsk.exe"),
|
||||
Path::new(r"C:\bsk.exe.update-42"),
|
||||
false,
|
||||
);
|
||||
#[cfg(windows)]
|
||||
fn wait_for_helper(mut child: windows::Helper) -> std::process::ExitStatus {
|
||||
let deadline = std::time::Instant::now() + Duration::from_secs(10);
|
||||
loop {
|
||||
if let Some(status) = child.try_wait().unwrap() {
|
||||
return status;
|
||||
}
|
||||
if std::time::Instant::now() >= deadline {
|
||||
let _ = child.kill();
|
||||
let _ = child.wait();
|
||||
panic!("Windows update helper did not exit");
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(20));
|
||||
}
|
||||
}
|
||||
|
||||
assert!(!script.contains("daemon start"));
|
||||
assert!(script.contains(r#"del /F /Q "%~f0""#));
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_helper_replaces_in_unicode_spaces_and_shell_symbol_paths() {
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
for name in [
|
||||
"ascii",
|
||||
"with space",
|
||||
"中文目录",
|
||||
"literal %PATH% ! & (folder)",
|
||||
] {
|
||||
let dir = tmp.path().join(name);
|
||||
std::fs::create_dir(&dir).unwrap();
|
||||
let target = dir.join("bsk.exe");
|
||||
std::fs::write(&target, b"old binary").unwrap();
|
||||
let child = spawn_windows_replacement(&target, b"new binary", None, 5).unwrap();
|
||||
let paths = staged_replacement_paths(&target, std::process::id()).unwrap();
|
||||
let status = wait_for_helper(child);
|
||||
assert!(
|
||||
status.success(),
|
||||
"{name}: {:?}",
|
||||
std::fs::read_to_string(paths.script_path.with_extension("log"))
|
||||
);
|
||||
assert_eq!(std::fs::read(&target).unwrap(), b"new binary", "{name}");
|
||||
// Successful updates leave no helper, staged binary, ready file or log.
|
||||
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 1, "{name}");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_helper_waits_for_unlock_then_replaces() {
|
||||
use std::os::windows::fs::OpenOptionsExt;
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
let target = tmp.path().join("bsk.exe");
|
||||
std::fs::write(&target, b"old binary").unwrap();
|
||||
// Deny delete sharing, like a running Windows executable.
|
||||
let lock = std::fs::OpenOptions::new()
|
||||
.read(true)
|
||||
.share_mode(1)
|
||||
.open(&target)
|
||||
.unwrap();
|
||||
let mut child = spawn_windows_replacement(&target, b"new binary", None, 5).unwrap();
|
||||
std::thread::sleep(Duration::from_millis(150));
|
||||
assert!(child.try_wait().unwrap().is_none());
|
||||
assert_eq!(std::fs::read(&target).unwrap(), b"old binary");
|
||||
drop(lock);
|
||||
assert!(wait_for_helper(child).success());
|
||||
assert_eq!(std::fs::read(&target).unwrap(), b"new binary");
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_helper_bounds_retries_and_preserves_old_binary_on_failure() {
|
||||
use std::os::windows::fs::OpenOptionsExt;
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
let target = tmp.path().join("bsk.exe");
|
||||
std::fs::write(&target, b"old binary").unwrap();
|
||||
let _lock = std::fs::OpenOptions::new()
|
||||
.read(true)
|
||||
.share_mode(1)
|
||||
.open(&target)
|
||||
.unwrap();
|
||||
let child = spawn_windows_replacement(&target, b"new binary", None, 2).unwrap();
|
||||
assert!(!wait_for_helper(child).success());
|
||||
let paths = staged_replacement_paths(&target, std::process::id()).unwrap();
|
||||
assert_eq!(std::fs::read(&target).unwrap(), b"old binary");
|
||||
assert!(!paths.binary_path.exists());
|
||||
assert!(!paths.script_path.exists());
|
||||
let log_bytes = std::fs::read(paths.script_path.with_extension("log")).unwrap();
|
||||
let log = String::from_utf8_lossy(&log_bytes);
|
||||
assert!(
|
||||
log.contains("Failed to replace the executable after 2 attempts."),
|
||||
"{log}"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_helper_reports_restart_failure_without_losing_replacement() {
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
let target = tmp.path().join("bsk.exe");
|
||||
std::fs::write(&target, b"old binary").unwrap();
|
||||
// Not a valid PE executable: replacement succeeds but restart fails.
|
||||
let child = spawn_windows_replacement(
|
||||
&target,
|
||||
b"invalid executable",
|
||||
Some(&StartArgs::default()),
|
||||
2,
|
||||
)
|
||||
.unwrap();
|
||||
assert!(!wait_for_helper(child).success());
|
||||
let paths = staged_replacement_paths(&target, std::process::id()).unwrap();
|
||||
let bytes = std::fs::read(paths.script_path.with_extension("log")).unwrap();
|
||||
let log = String::from_utf8_lossy(&bytes);
|
||||
assert!(
|
||||
log.contains("Replaced the executable but failed to restart the daemon."),
|
||||
"{log}"
|
||||
);
|
||||
assert_eq!(std::fs::read(&target).unwrap(), b"invalid executable");
|
||||
assert!(!paths.script_path.exists());
|
||||
assert!(!paths.binary_path.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1279,10 +1445,9 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_update_step_staged_outcome_requests_no_restart() {
|
||||
// The Windows shape: the replacement is staged next to the
|
||||
// running binary, so the outcome must not ask for the immediate
|
||||
// self-restart that `Replaced` triggers.
|
||||
fn auto_update_step_reports_staged_handoff() {
|
||||
// The helper, rather than this process, starts the new executable.
|
||||
// The daemon must exit to let that staged handoff finish.
|
||||
let candidate = test_candidate();
|
||||
let outcome =
|
||||
auto_update_step(Some(&candidate), true, 0, |_| Ok(InstallAction::Staged)).unwrap();
|
||||
|
||||
@@ -0,0 +1,203 @@
|
||||
//! Launch the update helper with only its stdio handles inherited.
|
||||
//!
|
||||
//! std::process::Command on Windows also inherits other inheritable handles.
|
||||
//! A daemon's listening socket must not survive in the helper: it would keep
|
||||
//! the port occupied after the daemon exits and prevent its replacement starting.
|
||||
|
||||
use std::ffi::OsStr;
|
||||
use std::fs::File;
|
||||
use std::io;
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle};
|
||||
use std::os::windows::process::ExitStatusExt;
|
||||
use std::path::Path;
|
||||
use std::process::ExitStatus;
|
||||
|
||||
use windows_sys::Win32::Foundation::{
|
||||
HANDLE, HANDLE_FLAG_INHERIT, SetHandleInformation, WAIT_OBJECT_0, WAIT_TIMEOUT,
|
||||
};
|
||||
use windows_sys::Win32::System::Threading::{
|
||||
CREATE_NEW_PROCESS_GROUP, CREATE_NO_WINDOW, CREATE_UNICODE_ENVIRONMENT, CreateProcessW,
|
||||
DeleteProcThreadAttributeList, EXTENDED_STARTUPINFO_PRESENT, GetExitCodeProcess,
|
||||
InitializeProcThreadAttributeList, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, PROCESS_INFORMATION,
|
||||
STARTF_USESTDHANDLES, STARTUPINFOEXW, TerminateProcess, UpdateProcThreadAttribute,
|
||||
WaitForSingleObject,
|
||||
};
|
||||
|
||||
pub(super) struct Helper(OwnedHandle);
|
||||
|
||||
impl Helper {
|
||||
pub(super) fn try_wait(&mut self) -> io::Result<Option<ExitStatus>> {
|
||||
// SAFETY: the owned process handle remains valid for both calls.
|
||||
match unsafe { WaitForSingleObject(self.0.as_raw_handle(), 0) } {
|
||||
WAIT_OBJECT_0 => {
|
||||
let mut code = 0;
|
||||
if unsafe { GetExitCodeProcess(self.0.as_raw_handle(), &mut code) } == 0 {
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
Ok(Some(ExitStatus::from_raw(code)))
|
||||
}
|
||||
WAIT_TIMEOUT => Ok(None),
|
||||
_ => Err(io::Error::last_os_error()),
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn kill(&mut self) -> io::Result<()> {
|
||||
// SAFETY: this handle belongs to the helper we created.
|
||||
if unsafe { TerminateProcess(self.0.as_raw_handle(), 1) } == 0 {
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(super) fn wait(&mut self) -> io::Result<()> {
|
||||
// Only used after kill(); do not leave a live helper on startup failure.
|
||||
if unsafe { WaitForSingleObject(self.0.as_raw_handle(), 5000) } != WAIT_OBJECT_0 {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::TimedOut,
|
||||
"update helper did not exit",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
// The opaque attribute list needs pointer-aligned storage and explicit teardown.
|
||||
struct AttributeList(Vec<usize>);
|
||||
|
||||
impl Drop for AttributeList {
|
||||
fn drop(&mut self) {
|
||||
// SAFETY: this wrapper is constructed only after successful initialization.
|
||||
unsafe { DeleteProcThreadAttributeList(self.0.as_mut_ptr().cast()) };
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn spawn(
|
||||
script: &Path,
|
||||
source: &Path,
|
||||
target: &Path,
|
||||
ready: &Path,
|
||||
log_path: &Path,
|
||||
) -> io::Result<Helper> {
|
||||
let root = std::env::var_os("SystemRoot")
|
||||
.ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "SystemRoot is not set"))?;
|
||||
let application = wide(Path::new(&root).join("System32/cmd.exe").as_os_str());
|
||||
// /S /C strips one outer pair of quotes. Environment expansion keeps paths
|
||||
// Unicode and avoids CRT escaping, batch-file encoding, and CALL expansion.
|
||||
let mut command = wide(OsStr::new(
|
||||
"cmd.exe /D /V:OFF /S /C \"\"%BSK_UPDATE_SCRIPT%\"\"",
|
||||
));
|
||||
let overrides = [
|
||||
("BSK_UPDATE_SCRIPT", script.as_os_str()),
|
||||
("BSK_UPDATE_SOURCE", source.as_os_str()),
|
||||
("BSK_UPDATE_TARGET", target.as_os_str()),
|
||||
("BSK_UPDATE_READY", ready.as_os_str()),
|
||||
("BSK_UPDATE_LOG", log_path.as_os_str()),
|
||||
];
|
||||
let mut env: Vec<_> = std::env::vars_os()
|
||||
.filter(|(key, _)| {
|
||||
let key = key.to_string_lossy();
|
||||
!overrides
|
||||
.iter()
|
||||
.any(|(name, _)| key.eq_ignore_ascii_case(name))
|
||||
&& !key.eq_ignore_ascii_case(crate::daemon::start::DAEMONIZED_ENV)
|
||||
&& !key.eq_ignore_ascii_case(crate::daemon::start::DAEMON_REPLACEMENT_WAIT_ENV)
|
||||
})
|
||||
.collect();
|
||||
env.extend(
|
||||
overrides
|
||||
.into_iter()
|
||||
.map(|(key, value)| (key.into(), value.to_owned())),
|
||||
);
|
||||
env.sort_by_key(|(key, _)| key.to_string_lossy().to_uppercase());
|
||||
let mut environment = Vec::new();
|
||||
for (key, value) in env {
|
||||
environment.extend(key.encode_wide());
|
||||
environment.push(b'=' as u16);
|
||||
environment.extend(value.encode_wide());
|
||||
environment.push(0);
|
||||
}
|
||||
environment.push(0);
|
||||
|
||||
let input = File::open("NUL")?;
|
||||
let log = File::create(log_path)?;
|
||||
let handles: [HANDLE; 2] = [input.as_raw_handle(), log.as_raw_handle()];
|
||||
for &handle in &handles {
|
||||
// SAFETY: these are dedicated helper stdio handles, held until spawn
|
||||
// finishes. The explicit handle list excludes every other parent handle.
|
||||
if unsafe { SetHandleInformation(handle, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT) } == 0 {
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
}
|
||||
|
||||
let mut size = 0;
|
||||
// SAFETY: the first call queries the allocation size, as required by Win32.
|
||||
unsafe { InitializeProcThreadAttributeList(std::ptr::null_mut(), 1, 0, &mut size) };
|
||||
if size == 0 {
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
let mut storage = vec![0usize; size.div_ceil(std::mem::size_of::<usize>())];
|
||||
// SAFETY: storage has the requested size and pointer alignment.
|
||||
if unsafe { InitializeProcThreadAttributeList(storage.as_mut_ptr().cast(), 1, 0, &mut size) }
|
||||
== 0
|
||||
{
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
let mut attributes = AttributeList(storage);
|
||||
// SAFETY: the handle array and attribute storage outlive CreateProcessW.
|
||||
if unsafe {
|
||||
UpdateProcThreadAttribute(
|
||||
attributes.0.as_mut_ptr().cast(),
|
||||
0,
|
||||
PROC_THREAD_ATTRIBUTE_HANDLE_LIST as usize,
|
||||
handles.as_ptr().cast(),
|
||||
std::mem::size_of_val(&handles),
|
||||
std::ptr::null_mut(),
|
||||
std::ptr::null(),
|
||||
)
|
||||
} == 0
|
||||
{
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
// SAFETY: these Win32 structs permit zero initialization; required fields
|
||||
// are populated below before the process is created.
|
||||
let mut startup: STARTUPINFOEXW = unsafe { std::mem::zeroed() };
|
||||
startup.StartupInfo.cb = std::mem::size_of::<STARTUPINFOEXW>() as u32;
|
||||
startup.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
|
||||
startup.StartupInfo.hStdInput = handles[0];
|
||||
startup.StartupInfo.hStdOutput = handles[1];
|
||||
startup.StartupInfo.hStdError = handles[1];
|
||||
startup.lpAttributeList = attributes.0.as_mut_ptr().cast();
|
||||
let mut process: PROCESS_INFORMATION = unsafe { std::mem::zeroed() };
|
||||
// SAFETY: strings are NUL-terminated UTF-16, the environment is double-NUL
|
||||
// terminated, and all buffers/handles remain alive for the duration of spawn.
|
||||
if unsafe {
|
||||
CreateProcessW(
|
||||
application.as_ptr(),
|
||||
command.as_mut_ptr(),
|
||||
std::ptr::null(),
|
||||
std::ptr::null(),
|
||||
1,
|
||||
CREATE_NO_WINDOW
|
||||
| CREATE_NEW_PROCESS_GROUP
|
||||
| CREATE_UNICODE_ENVIRONMENT
|
||||
| EXTENDED_STARTUPINFO_PRESENT,
|
||||
environment.as_ptr().cast(),
|
||||
std::ptr::null(),
|
||||
&startup.StartupInfo,
|
||||
&mut process,
|
||||
)
|
||||
} == 0
|
||||
{
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
// SAFETY: successful CreateProcessW transfers these two handles to us.
|
||||
let _thread = unsafe { OwnedHandle::from_raw_handle(process.hThread) };
|
||||
Ok(Helper(unsafe {
|
||||
OwnedHandle::from_raw_handle(process.hProcess)
|
||||
}))
|
||||
}
|
||||
|
||||
fn wide(value: &OsStr) -> Vec<u16> {
|
||||
value.encode_wide().chain(Some(0)).collect()
|
||||
}
|
||||
@@ -0,0 +1,202 @@
|
||||
//! `bsk upload` — stage caller-readable files and attach them to a page.
|
||||
|
||||
use std::fs::File;
|
||||
use std::io::Read;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use base64::Engine;
|
||||
use bsk_protocol::Method;
|
||||
use bsk_protocol::tools::{
|
||||
TransferBeginParams, TransferBeginResult, TransferChunkParams, TransferChunkResult,
|
||||
TransferIdParams, TransferReadyResult, UploadFile, UploadMode, UploadParams, UploadResult,
|
||||
};
|
||||
use clap::{Args, ValueEnum};
|
||||
|
||||
use crate::cli::ensure_daemon::ensure_daemon;
|
||||
use crate::cli::error::{CliError, Format};
|
||||
use crate::cli::interaction::split_target;
|
||||
use crate::cli::navigate::parse_timeout_ms;
|
||||
|
||||
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, ValueEnum)]
|
||||
pub enum UploadModeArg {
|
||||
#[default]
|
||||
Input,
|
||||
Drop,
|
||||
}
|
||||
|
||||
impl From<UploadModeArg> for UploadMode {
|
||||
fn from(value: UploadModeArg) -> Self {
|
||||
match value {
|
||||
UploadModeArg::Input => Self::Input,
|
||||
UploadModeArg::Drop => Self::Drop,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Args)]
|
||||
pub struct UploadArgs {
|
||||
/// Snapshot ref (`@e3`) or CSS selector for the chooser trigger / drop target.
|
||||
pub target: Option<String>,
|
||||
#[arg(long = "ref")]
|
||||
pub ref_: Option<String>,
|
||||
#[arg(long = "selector")]
|
||||
pub selector: Option<String>,
|
||||
/// Local file to upload. Repeat for a multiple-file input.
|
||||
#[arg(long = "file", required = true)]
|
||||
pub files: Vec<PathBuf>,
|
||||
/// Browser mechanism used to deliver the staged files.
|
||||
#[arg(long, value_enum, default_value_t)]
|
||||
pub mode: UploadModeArg,
|
||||
#[arg(long)]
|
||||
pub session: String,
|
||||
#[arg(long = "tab-id")]
|
||||
pub tab_id: Option<i64>,
|
||||
#[arg(long, default_value = "2m", value_parser = parse_timeout_ms)]
|
||||
pub timeout: u32,
|
||||
}
|
||||
|
||||
pub fn dispatch(args: UploadArgs, format: Format) -> Result<(), CliError> {
|
||||
if args.files.len() > crate::daemon::file_transfer::MAX_UPLOAD_FILES {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"upload accepts at most {} files",
|
||||
crate::daemon::file_transfer::MAX_UPLOAD_FILES
|
||||
)));
|
||||
}
|
||||
let info = ensure_daemon().context("ensure daemon is running")?;
|
||||
let (ref_, selector) = split_target(args.target, args.ref_, args.selector)?;
|
||||
let mut staged = Vec::new();
|
||||
for path in &args.files {
|
||||
if let Err(err) = stage_file(&info.sock_path, &args.session, path, &mut staged) {
|
||||
let _ = crate::cli::business_rpc::release_transfers(
|
||||
&info.sock_path,
|
||||
staged.iter().map(|(id, _)| id.as_str()),
|
||||
);
|
||||
return Err(err);
|
||||
}
|
||||
}
|
||||
let params = UploadParams {
|
||||
session_id: args.session,
|
||||
ref_,
|
||||
selector,
|
||||
tab_id: args.tab_id,
|
||||
files: staged
|
||||
.iter()
|
||||
.map(|(id, name)| UploadFile {
|
||||
transfer_id: id.clone(),
|
||||
name: name.clone(),
|
||||
staged_path: None,
|
||||
})
|
||||
.collect(),
|
||||
mode: args.mode.into(),
|
||||
timeout_ms: Some(args.timeout),
|
||||
};
|
||||
let result = crate::cli::business_rpc::call::<_, UploadResult>(
|
||||
info.sock_path.clone(),
|
||||
"upload",
|
||||
Method::ToolUpload,
|
||||
Some(params),
|
||||
ipc_timeout(args.timeout),
|
||||
);
|
||||
// The helper can reject cancellation before handing any request to the
|
||||
// daemon. In that case staging still belongs to this CLI.
|
||||
if result.as_ref().is_err_and(|err| {
|
||||
err.data()
|
||||
.is_some_and(|data| data["reason"] == "cancelled_before_dispatch")
|
||||
}) {
|
||||
let _ = crate::cli::business_rpc::release_transfers(
|
||||
&info.sock_path,
|
||||
staged.iter().map(|(id, _)| id.as_str()),
|
||||
);
|
||||
}
|
||||
// Once tool.upload is dispatched, staging ownership belongs to the
|
||||
// session. A transport timeout cannot prove that Chrome did not attach
|
||||
// the file, so releasing here could invalidate a late successful attach.
|
||||
// Session teardown remains the single cleanup boundary after dispatch.
|
||||
let reply = result?;
|
||||
match format {
|
||||
Format::Json => println!("{}", serde_json::to_string_pretty(&reply).unwrap()),
|
||||
Format::Human => println!(
|
||||
"upload ok tab={} files={}",
|
||||
reply.tab_id,
|
||||
reply.file_names.join(", ")
|
||||
),
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn stage_file(
|
||||
sock: &Path,
|
||||
session: &str,
|
||||
path: &PathBuf,
|
||||
staged: &mut Vec<(String, String)>,
|
||||
) -> Result<(), CliError> {
|
||||
let mut file = File::open(path)
|
||||
.with_context(|| format!("open upload file {}", path.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
let meta = file
|
||||
.metadata()
|
||||
.with_context(|| format!("read upload file metadata {}", path.display()))
|
||||
.map_err(CliError::Local)?;
|
||||
if !meta.is_file() {
|
||||
return Err(CliError::Local(anyhow::anyhow!(
|
||||
"upload source is not a regular file: {}",
|
||||
path.display()
|
||||
)));
|
||||
}
|
||||
let name = path
|
||||
.file_name()
|
||||
.and_then(|v| v.to_str())
|
||||
.filter(|v| !v.is_empty())
|
||||
.ok_or_else(|| CliError::Local(anyhow::anyhow!("upload file has no valid basename")))?
|
||||
.to_string();
|
||||
let begin: TransferBeginResult = crate::cli::business_rpc::call(
|
||||
sock.to_path_buf(),
|
||||
"transfer-begin",
|
||||
Method::TransferBegin,
|
||||
Some(TransferBeginParams {
|
||||
session_id: session.to_string(),
|
||||
name: name.clone(),
|
||||
byte_size: meta.len(),
|
||||
}),
|
||||
Duration::from_secs(10),
|
||||
)?;
|
||||
// Record ownership immediately so one error path releases both completed
|
||||
// files and the current partially staged file.
|
||||
staged.push((begin.transfer_id.clone(), name));
|
||||
let mut offset = 0u64;
|
||||
let mut buf = vec![0u8; begin.chunk_size as usize];
|
||||
loop {
|
||||
let n = file.read(&mut buf).map_err(|e| CliError::Local(e.into()))?;
|
||||
if n == 0 {
|
||||
break;
|
||||
}
|
||||
let reply: TransferChunkResult = crate::cli::business_rpc::call(
|
||||
sock.to_path_buf(),
|
||||
"transfer-chunk",
|
||||
Method::TransferChunk,
|
||||
Some(TransferChunkParams {
|
||||
transfer_id: begin.transfer_id.clone(),
|
||||
offset,
|
||||
data_base64: base64::engine::general_purpose::STANDARD.encode(&buf[..n]),
|
||||
}),
|
||||
Duration::from_secs(30),
|
||||
)?;
|
||||
offset = reply.next_offset;
|
||||
}
|
||||
let _: TransferReadyResult = crate::cli::business_rpc::call(
|
||||
sock.to_path_buf(),
|
||||
"transfer-finish",
|
||||
Method::TransferFinish,
|
||||
Some(TransferIdParams {
|
||||
transfer_id: begin.transfer_id.clone(),
|
||||
}),
|
||||
Duration::from_secs(10),
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn ipc_timeout(timeout_ms: u32) -> Duration {
|
||||
Duration::from_millis(u64::from(timeout_ms) + 5_000)
|
||||
}
|
||||
@@ -0,0 +1,728 @@
|
||||
//! Session-scoped, content-transparent staging for upload/download tools.
|
||||
//!
|
||||
//! The registry deliberately exposes no arbitrary-path operations. Upload
|
||||
//! bytes arrive from the invoking CLI in bounded chunks; browser downloads
|
||||
//! land in a daemon-minted directory and are read back by the invoking CLI.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::fs::{self, File, OpenOptions};
|
||||
use std::io::{Read, Seek, SeekFrom, Write};
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
use std::sync::Mutex;
|
||||
|
||||
use base64::Engine;
|
||||
use bsk_protocol::tools::{
|
||||
TransferBeginParams, TransferBeginResult, TransferChunkParams, TransferChunkResult,
|
||||
TransferIdParams, TransferReadyResult, TransferReleaseResult,
|
||||
};
|
||||
use bsk_protocol::{ErrorCode, RpcError};
|
||||
use uuid::Uuid;
|
||||
|
||||
use super::paths;
|
||||
|
||||
pub const TRANSFER_CHUNK_SIZE: u32 = 512 * 1024;
|
||||
pub const MAX_TRANSFER_BYTES: u64 = 512 * 1024 * 1024;
|
||||
pub const MAX_UPLOAD_FILES: usize = 20;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum Direction {
|
||||
Upload,
|
||||
Download,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct Entry {
|
||||
session_id: String,
|
||||
direction: Direction,
|
||||
path: PathBuf,
|
||||
expected_size: Option<u64>,
|
||||
written: u64,
|
||||
ready: bool,
|
||||
}
|
||||
|
||||
/// Browser download path after the daemon has verified that it belongs to
|
||||
/// the capability minted for this transfer. From this point onward the daemon
|
||||
/// owns cleanup on every error path; unvalidated arbitrary paths are never
|
||||
/// removed.
|
||||
#[derive(Debug)]
|
||||
struct ValidatedBrowserFile {
|
||||
path: PathBuf,
|
||||
parent: PathBuf,
|
||||
cleanup_armed: bool,
|
||||
}
|
||||
|
||||
impl ValidatedBrowserFile {
|
||||
fn validate(reported_path: &Path, transfer_id: &str) -> Result<Self, RpcError> {
|
||||
if !reported_path.is_absolute() {
|
||||
return Err(permission(
|
||||
"download path is outside its browser capability",
|
||||
));
|
||||
}
|
||||
let expected_parent = Path::new("BrowserSkill").join(transfer_id);
|
||||
let parent = reported_path
|
||||
.parent()
|
||||
.ok_or_else(|| permission("download path has no parent directory"))?;
|
||||
if !parent.ends_with(&expected_parent) {
|
||||
return Err(permission(
|
||||
"download escaped its browser capability directory",
|
||||
));
|
||||
}
|
||||
for path in [reported_path, parent] {
|
||||
if fs::symlink_metadata(path)
|
||||
.map_err(io_error)?
|
||||
.file_type()
|
||||
.is_symlink()
|
||||
{
|
||||
return Err(permission("download capability path contains a symlink"));
|
||||
}
|
||||
}
|
||||
Ok(Self {
|
||||
path: reported_path.to_path_buf(),
|
||||
parent: parent.to_path_buf(),
|
||||
cleanup_armed: true,
|
||||
})
|
||||
}
|
||||
|
||||
fn remove_source(mut self) -> Result<(), RpcError> {
|
||||
remove_if_present(&self.path).map_err(io_error)?;
|
||||
remove_dir_if_present(&self.parent).map_err(io_error)?;
|
||||
self.cleanup_armed = false;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for ValidatedBrowserFile {
|
||||
fn drop(&mut self) {
|
||||
if !self.cleanup_armed {
|
||||
return;
|
||||
}
|
||||
let _ = remove_if_present(&self.path);
|
||||
let _ = remove_dir_if_present(&self.parent);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct DownloadStaging {
|
||||
pub transfer_id: String,
|
||||
pub browser_relative_dir: String,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct TransferRegistry {
|
||||
root: PathBuf,
|
||||
initialized: Mutex<bool>,
|
||||
entries: Mutex<HashMap<String, Entry>>,
|
||||
}
|
||||
|
||||
impl TransferRegistry {
|
||||
pub fn new() -> anyhow::Result<Self> {
|
||||
Ok(Self {
|
||||
root: paths::bsk_home()?.join("run").join("transfers"),
|
||||
initialized: Mutex::new(false),
|
||||
entries: Mutex::new(HashMap::new()),
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn at_root(root: PathBuf) -> anyhow::Result<Self> {
|
||||
let registry = Self {
|
||||
root,
|
||||
initialized: Mutex::new(false),
|
||||
entries: Mutex::new(HashMap::new()),
|
||||
};
|
||||
registry.ensure_root()?;
|
||||
Ok(registry)
|
||||
}
|
||||
|
||||
pub fn initialize(&self) -> anyhow::Result<()> {
|
||||
self.ensure_root()
|
||||
}
|
||||
|
||||
fn ensure_root(&self) -> anyhow::Result<()> {
|
||||
let mut initialized = self.initialized.lock().unwrap();
|
||||
if *initialized {
|
||||
return Ok(());
|
||||
}
|
||||
if self.root.exists() {
|
||||
// Transfers never survive a daemon; removing stale directories on
|
||||
// startup gives crash cleanup without a second persistence model.
|
||||
let _ = fs::remove_dir_all(&self.root);
|
||||
}
|
||||
fs::create_dir_all(&self.root)?;
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
fs::set_permissions(&self.root, fs::Permissions::from_mode(0o700))?;
|
||||
}
|
||||
*initialized = true;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn begin_upload(&self, p: TransferBeginParams) -> Result<TransferBeginResult, RpcError> {
|
||||
self.ensure_root()
|
||||
.map_err(|err| protocol_error(err.to_string()))?;
|
||||
let basename = safe_upload_basename(&p.name)?;
|
||||
if p.byte_size > MAX_TRANSFER_BYTES {
|
||||
return Err(invalid(format!(
|
||||
"file size {} exceeds transfer limit {}",
|
||||
p.byte_size, MAX_TRANSFER_BYTES
|
||||
)));
|
||||
}
|
||||
let mut entries = self.entries.lock().unwrap();
|
||||
let session_bytes: u64 = entries
|
||||
.values()
|
||||
.filter(|entry| entry.session_id == p.session_id)
|
||||
.map(|entry| entry.expected_size.unwrap_or(entry.written))
|
||||
.sum();
|
||||
if session_bytes.saturating_add(p.byte_size) > MAX_TRANSFER_BYTES {
|
||||
return Err(invalid(format!(
|
||||
"session transfer staging exceeds limit {}",
|
||||
MAX_TRANSFER_BYTES
|
||||
)));
|
||||
}
|
||||
let id = new_id();
|
||||
let dir = self.root.join(&id);
|
||||
fs::create_dir(&dir).map_err(io_error)?;
|
||||
set_private_dir(&dir).map_err(io_error)?;
|
||||
let path = dir.join(basename);
|
||||
let file = OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.open(&path)
|
||||
.map_err(io_error)?;
|
||||
set_private_file(&file).map_err(io_error)?;
|
||||
entries.insert(
|
||||
id.clone(),
|
||||
Entry {
|
||||
session_id: p.session_id,
|
||||
direction: Direction::Upload,
|
||||
path,
|
||||
expected_size: Some(p.byte_size),
|
||||
written: 0,
|
||||
ready: false,
|
||||
},
|
||||
);
|
||||
Ok(TransferBeginResult {
|
||||
transfer_id: id,
|
||||
chunk_size: TRANSFER_CHUNK_SIZE,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn write_chunk(&self, p: TransferChunkParams) -> Result<TransferChunkResult, RpcError> {
|
||||
let bytes = base64::engine::general_purpose::STANDARD
|
||||
.decode(p.data_base64)
|
||||
.map_err(|e| invalid(format!("invalid transfer chunk: {e}")))?;
|
||||
if bytes.len() > TRANSFER_CHUNK_SIZE as usize {
|
||||
return Err(invalid("transfer chunk exceeds negotiated size"));
|
||||
}
|
||||
let mut entries = self.entries.lock().unwrap();
|
||||
let entry = entries
|
||||
.get_mut(&p.transfer_id)
|
||||
.ok_or_else(|| not_found("transfer not found"))?;
|
||||
if entry.direction != Direction::Upload || entry.ready {
|
||||
return Err(invalid("transfer is not writable"));
|
||||
}
|
||||
if p.offset != entry.written {
|
||||
return Err(invalid(format!(
|
||||
"non-sequential transfer offset: expected {}, got {}",
|
||||
entry.written, p.offset
|
||||
)));
|
||||
}
|
||||
let next = entry.written.saturating_add(bytes.len() as u64);
|
||||
if next > entry.expected_size.unwrap_or(MAX_TRANSFER_BYTES) {
|
||||
return Err(invalid("transfer exceeds declared byte size"));
|
||||
}
|
||||
let mut file = OpenOptions::new()
|
||||
.append(true)
|
||||
.open(&entry.path)
|
||||
.map_err(io_error)?;
|
||||
file.write_all(&bytes).map_err(io_error)?;
|
||||
entry.written = next;
|
||||
Ok(TransferChunkResult {
|
||||
next_offset: next,
|
||||
eof: false,
|
||||
data_base64: None,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn finish_upload(&self, p: TransferIdParams) -> Result<TransferReadyResult, RpcError> {
|
||||
let mut entries = self.entries.lock().unwrap();
|
||||
let entry = entries
|
||||
.get_mut(&p.transfer_id)
|
||||
.ok_or_else(|| not_found("transfer not found"))?;
|
||||
if entry.direction != Direction::Upload {
|
||||
return Err(invalid("transfer is not an upload"));
|
||||
}
|
||||
if entry.written != entry.expected_size.unwrap_or(entry.written) {
|
||||
return Err(invalid(format!(
|
||||
"incomplete transfer: expected {} bytes, received {}",
|
||||
entry.expected_size.unwrap_or(0),
|
||||
entry.written
|
||||
)));
|
||||
}
|
||||
entry.ready = true;
|
||||
Ok(TransferReadyResult {
|
||||
transfer_id: p.transfer_id,
|
||||
byte_size: entry.written,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn resolve_uploads(
|
||||
&self,
|
||||
session_id: &str,
|
||||
ids: &[String],
|
||||
) -> Result<Vec<PathBuf>, RpcError> {
|
||||
let entries = self.entries.lock().unwrap();
|
||||
ids.iter()
|
||||
.map(|id| {
|
||||
let entry = entries
|
||||
.get(id)
|
||||
.ok_or_else(|| not_found("upload transfer not found"))?;
|
||||
if entry.session_id != session_id
|
||||
|| entry.direction != Direction::Upload
|
||||
|| !entry.ready
|
||||
{
|
||||
return Err(permission(
|
||||
"upload transfer is outside this session or not ready",
|
||||
));
|
||||
}
|
||||
Ok(entry.path.clone())
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn begin_download(&self, session_id: &str) -> Result<DownloadStaging, RpcError> {
|
||||
self.ensure_root()
|
||||
.map_err(|err| protocol_error(err.to_string()))?;
|
||||
let id = new_id();
|
||||
let dir = self.root.join(&id);
|
||||
fs::create_dir(&dir).map_err(io_error)?;
|
||||
set_private_dir(&dir).map_err(io_error)?;
|
||||
self.entries.lock().unwrap().insert(
|
||||
id.clone(),
|
||||
Entry {
|
||||
session_id: session_id.to_string(),
|
||||
direction: Direction::Download,
|
||||
path: dir.clone(),
|
||||
expected_size: None,
|
||||
written: 0,
|
||||
ready: false,
|
||||
},
|
||||
);
|
||||
Ok(DownloadStaging {
|
||||
browser_relative_dir: format!("BrowserSkill/{id}"),
|
||||
transfer_id: id,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn import_download(&self, id: &str, reported_path: &Path) -> Result<u64, RpcError> {
|
||||
let mut entries = self.entries.lock().unwrap();
|
||||
let entry = entries
|
||||
.get_mut(id)
|
||||
.ok_or_else(|| not_found("download transfer not found"))?;
|
||||
if entry.direction != Direction::Download {
|
||||
return Err(permission("transfer is not a download capability"));
|
||||
}
|
||||
let browser_file = ValidatedBrowserFile::validate(reported_path, id)?;
|
||||
let canonical = browser_file.path.canonicalize().map_err(io_error)?;
|
||||
let meta = fs::metadata(&canonical).map_err(io_error)?;
|
||||
if !meta.is_file() || meta.len() > MAX_TRANSFER_BYTES {
|
||||
return Err(invalid(
|
||||
"download is not a regular file or exceeds the transfer limit",
|
||||
));
|
||||
}
|
||||
|
||||
let destination = entry.path.join("payload");
|
||||
let source = File::open(&canonical).map_err(io_error)?;
|
||||
let mut destination_file = OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.open(&destination)
|
||||
.map_err(io_error)?;
|
||||
let copied = std::io::copy(
|
||||
&mut source.take(MAX_TRANSFER_BYTES.saturating_add(1)),
|
||||
&mut destination_file,
|
||||
)
|
||||
.map_err(io_error)?;
|
||||
if copied > MAX_TRANSFER_BYTES {
|
||||
let _ = fs::remove_file(&destination);
|
||||
return Err(invalid("download exceeds the transfer limit"));
|
||||
}
|
||||
entry.path = destination;
|
||||
entry.written = copied;
|
||||
entry.expected_size = Some(copied);
|
||||
entry.ready = true;
|
||||
browser_file.remove_source()?;
|
||||
Ok(copied)
|
||||
}
|
||||
|
||||
pub fn read_chunk(&self, p: TransferChunkParams) -> Result<TransferChunkResult, RpcError> {
|
||||
let entries = self.entries.lock().unwrap();
|
||||
let entry = entries
|
||||
.get(&p.transfer_id)
|
||||
.ok_or_else(|| not_found("transfer not found"))?;
|
||||
if entry.direction != Direction::Download || !entry.ready {
|
||||
return Err(invalid("transfer is not readable"));
|
||||
}
|
||||
if p.offset > entry.written {
|
||||
return Err(invalid("read offset is beyond transfer length"));
|
||||
}
|
||||
let mut file = File::open(&entry.path).map_err(io_error)?;
|
||||
file.seek(SeekFrom::Start(p.offset)).map_err(io_error)?;
|
||||
let mut buf = vec![0u8; TRANSFER_CHUNK_SIZE as usize];
|
||||
let count = file.read(&mut buf).map_err(io_error)?;
|
||||
buf.truncate(count);
|
||||
let next = p.offset + count as u64;
|
||||
Ok(TransferChunkResult {
|
||||
next_offset: next,
|
||||
eof: next >= entry.written,
|
||||
data_base64: Some(base64::engine::general_purpose::STANDARD.encode(buf)),
|
||||
})
|
||||
}
|
||||
|
||||
pub fn release(&self, p: TransferIdParams) -> TransferReleaseResult {
|
||||
let entry = self.entries.lock().unwrap().remove(&p.transfer_id);
|
||||
if let Some(entry) = entry {
|
||||
let dir = if entry.path.is_dir() {
|
||||
entry.path
|
||||
} else {
|
||||
entry.path.parent().unwrap_or(&self.root).to_path_buf()
|
||||
};
|
||||
let _ = fs::remove_dir_all(dir);
|
||||
TransferReleaseResult { released: true }
|
||||
} else {
|
||||
TransferReleaseResult { released: false }
|
||||
}
|
||||
}
|
||||
|
||||
pub fn release_session(&self, session_id: &str) {
|
||||
let ids: Vec<String> = self
|
||||
.entries
|
||||
.lock()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.filter(|(_, entry)| entry.session_id == session_id)
|
||||
.map(|(id, _)| id.clone())
|
||||
.collect();
|
||||
for id in ids {
|
||||
self.release(TransferIdParams { transfer_id: id });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn new_id() -> String {
|
||||
format!("tr_{}", Uuid::new_v4().simple())
|
||||
}
|
||||
|
||||
fn set_private_dir(path: &Path) -> std::io::Result<()> {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
fs::set_permissions(path, fs::Permissions::from_mode(0o700))?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn set_private_file(file: &File) -> std::io::Result<()> {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
file.set_permissions(fs::Permissions::from_mode(0o600))?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_if_present(path: &Path) -> std::io::Result<()> {
|
||||
match fs::remove_file(path) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(()),
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
fn remove_dir_if_present(path: &Path) -> std::io::Result<()> {
|
||||
match fs::remove_dir(path) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(()),
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
fn safe_upload_basename(name: &str) -> Result<&str, RpcError> {
|
||||
if name.is_empty() || name.contains(['/', '\\', '\0']) {
|
||||
return Err(invalid("upload name must be a safe basename"));
|
||||
}
|
||||
let mut components = Path::new(name).components();
|
||||
match (components.next(), components.next()) {
|
||||
(Some(Component::Normal(_)), None) => Ok(name),
|
||||
_ => Err(invalid("upload name must be a safe basename")),
|
||||
}
|
||||
}
|
||||
|
||||
fn invalid(message: impl Into<String>) -> RpcError {
|
||||
RpcError {
|
||||
code: ErrorCode::InvalidParams,
|
||||
message: message.into(),
|
||||
data: None,
|
||||
}
|
||||
}
|
||||
fn not_found(message: impl Into<String>) -> RpcError {
|
||||
RpcError {
|
||||
code: ErrorCode::NotFound,
|
||||
message: message.into(),
|
||||
data: None,
|
||||
}
|
||||
}
|
||||
fn permission(message: impl Into<String>) -> RpcError {
|
||||
RpcError {
|
||||
code: ErrorCode::PermissionDenied,
|
||||
message: message.into(),
|
||||
data: None,
|
||||
}
|
||||
}
|
||||
fn io_error(err: std::io::Error) -> RpcError {
|
||||
RpcError {
|
||||
code: ErrorCode::ProtocolError,
|
||||
message: err.to_string(),
|
||||
data: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn protocol_error(message: impl Into<String>) -> RpcError {
|
||||
RpcError {
|
||||
code: ErrorCode::ProtocolError,
|
||||
message: message.into(),
|
||||
data: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn registry() -> (tempfile::TempDir, TransferRegistry) {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let registry = TransferRegistry::at_root(temp.path().join("transfers")).unwrap();
|
||||
(temp, registry)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upload_is_not_resolvable_until_complete_or_from_another_session() {
|
||||
let (_temp, registry) = registry();
|
||||
let begin = registry
|
||||
.begin_upload(TransferBeginParams {
|
||||
session_id: "s1".into(),
|
||||
name: "image.png".into(),
|
||||
byte_size: 3,
|
||||
})
|
||||
.unwrap();
|
||||
assert!(
|
||||
registry
|
||||
.resolve_uploads("s1", std::slice::from_ref(&begin.transfer_id))
|
||||
.is_err()
|
||||
);
|
||||
registry
|
||||
.write_chunk(TransferChunkParams {
|
||||
transfer_id: begin.transfer_id.clone(),
|
||||
offset: 0,
|
||||
data_base64: base64::engine::general_purpose::STANDARD.encode(b"abc"),
|
||||
})
|
||||
.unwrap();
|
||||
registry
|
||||
.finish_upload(TransferIdParams {
|
||||
transfer_id: begin.transfer_id.clone(),
|
||||
})
|
||||
.unwrap();
|
||||
assert!(
|
||||
registry
|
||||
.resolve_uploads("s2", std::slice::from_ref(&begin.transfer_id))
|
||||
.is_err()
|
||||
);
|
||||
let [path] = registry
|
||||
.resolve_uploads("s1", &[begin.transfer_id])
|
||||
.unwrap()
|
||||
.try_into()
|
||||
.unwrap();
|
||||
assert_eq!(path.file_name().unwrap(), "image.png");
|
||||
assert_eq!(fs::read(path).unwrap(), b"abc");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upload_name_must_be_one_safe_path_component() {
|
||||
let (_temp, registry) = registry();
|
||||
for name in ["", ".", "..", "../secret", "folder/file", "folder\\file"] {
|
||||
let result = registry.begin_upload(TransferBeginParams {
|
||||
session_id: "s1".into(),
|
||||
name: name.into(),
|
||||
byte_size: 0,
|
||||
});
|
||||
assert!(result.is_err(), "accepted unsafe upload name");
|
||||
}
|
||||
assert!(registry.entries.lock().unwrap().is_empty());
|
||||
assert!(fs::read_dir(®istry.root).unwrap().next().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn same_upload_names_are_isolated_by_transfer_directory() {
|
||||
let (_temp, registry) = registry();
|
||||
let first = registry
|
||||
.begin_upload(TransferBeginParams {
|
||||
session_id: "s1".into(),
|
||||
name: "image.png".into(),
|
||||
byte_size: 0,
|
||||
})
|
||||
.unwrap();
|
||||
let second = registry
|
||||
.begin_upload(TransferBeginParams {
|
||||
session_id: "s1".into(),
|
||||
name: "image.png".into(),
|
||||
byte_size: 0,
|
||||
})
|
||||
.unwrap();
|
||||
let entries = registry.entries.lock().unwrap();
|
||||
let first_path = &entries[&first.transfer_id].path;
|
||||
let second_path = &entries[&second.transfer_id].path;
|
||||
assert_ne!(first_path.parent(), second_path.parent());
|
||||
assert_eq!(first_path.file_name().unwrap(), "image.png");
|
||||
assert_eq!(second_path.file_name().unwrap(), "image.png");
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn upload_staging_is_private() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let (_temp, registry) = registry();
|
||||
let begin = registry
|
||||
.begin_upload(TransferBeginParams {
|
||||
session_id: "s1".into(),
|
||||
name: "private.png".into(),
|
||||
byte_size: 0,
|
||||
})
|
||||
.unwrap();
|
||||
let entries = registry.entries.lock().unwrap();
|
||||
let path = &entries[&begin.transfer_id].path;
|
||||
assert_eq!(
|
||||
fs::metadata(path.parent().unwrap())
|
||||
.unwrap()
|
||||
.permissions()
|
||||
.mode()
|
||||
& 0o777,
|
||||
0o700
|
||||
);
|
||||
assert_eq!(
|
||||
fs::metadata(path).unwrap().permissions().mode() & 0o777,
|
||||
0o600
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn download_must_import_from_its_browser_capability_directory() {
|
||||
let (temp, registry) = registry();
|
||||
let staging = registry.begin_download("s1").unwrap();
|
||||
let outside = temp.path().join("outside.bin");
|
||||
fs::write(&outside, b"secret").unwrap();
|
||||
assert!(
|
||||
registry
|
||||
.import_download(&staging.transfer_id, &outside)
|
||||
.is_err()
|
||||
);
|
||||
assert_eq!(fs::read(&outside).unwrap(), b"secret");
|
||||
|
||||
let browser_dir = temp
|
||||
.path()
|
||||
.join("Downloads")
|
||||
.join("BrowserSkill")
|
||||
.join(&staging.transfer_id);
|
||||
fs::create_dir_all(&browser_dir).unwrap();
|
||||
let inside = browser_dir.join("result.bin");
|
||||
fs::write(&inside, b"result").unwrap();
|
||||
assert_eq!(
|
||||
registry
|
||||
.import_download(&staging.transfer_id, &inside)
|
||||
.unwrap(),
|
||||
6
|
||||
);
|
||||
assert!(!inside.exists());
|
||||
let chunk = registry
|
||||
.read_chunk(TransferChunkParams {
|
||||
transfer_id: staging.transfer_id,
|
||||
offset: 0,
|
||||
data_base64: String::new(),
|
||||
})
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
base64::engine::general_purpose::STANDARD
|
||||
.decode(chunk.data_base64.unwrap())
|
||||
.unwrap(),
|
||||
b"result"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validated_oversized_browser_download_is_removed_on_rejection() {
|
||||
let (temp, registry) = registry();
|
||||
let staging = registry.begin_download("s1").unwrap();
|
||||
let browser_dir = temp
|
||||
.path()
|
||||
.join("Downloads")
|
||||
.join("BrowserSkill")
|
||||
.join(&staging.transfer_id);
|
||||
fs::create_dir_all(&browser_dir).unwrap();
|
||||
let inside = browser_dir.join("oversized.bin");
|
||||
File::create(&inside)
|
||||
.unwrap()
|
||||
.set_len(MAX_TRANSFER_BYTES + 1)
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
registry
|
||||
.import_download(&staging.transfer_id, &inside)
|
||||
.is_err()
|
||||
);
|
||||
assert!(!inside.exists());
|
||||
assert!(!browser_dir.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn releasing_a_session_removes_all_staging() {
|
||||
let (_temp, registry) = registry();
|
||||
let first = registry.begin_download("s1").unwrap();
|
||||
let second = registry.begin_download("s2").unwrap();
|
||||
let first_directory = registry.root.join(&first.transfer_id);
|
||||
let second_directory = registry.root.join(&second.transfer_id);
|
||||
registry.release_session("s1");
|
||||
assert!(!first_directory.exists());
|
||||
assert!(second_directory.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upload_staging_is_bounded_across_a_session() {
|
||||
let (_temp, registry) = registry();
|
||||
registry
|
||||
.begin_upload(TransferBeginParams {
|
||||
session_id: "s1".into(),
|
||||
name: "large.bin".into(),
|
||||
byte_size: MAX_TRANSFER_BYTES,
|
||||
})
|
||||
.unwrap();
|
||||
assert!(
|
||||
registry
|
||||
.begin_upload(TransferBeginParams {
|
||||
session_id: "s1".into(),
|
||||
name: "one-more-byte.bin".into(),
|
||||
byte_size: 1,
|
||||
})
|
||||
.is_err()
|
||||
);
|
||||
assert!(
|
||||
registry
|
||||
.begin_upload(TransferBeginParams {
|
||||
session_id: "s2".into(),
|
||||
name: "other-session.bin".into(),
|
||||
byte_size: 1,
|
||||
})
|
||||
.is_ok()
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -27,7 +27,8 @@ use bsk_protocol::system::{
|
||||
VersionSkewEntry,
|
||||
};
|
||||
use bsk_protocol::tools::{
|
||||
HelpOutcome, RequestHelpResult, ReturnFailure, WaitMsParams, WaitMsResult,
|
||||
DownloadParams, DownloadResult, HelpOutcome, RequestHelpResult, ReturnFailure,
|
||||
TransferBeginParams, TransferIdParams, UploadParams, WaitMsParams, WaitMsResult,
|
||||
};
|
||||
use bsk_protocol::{
|
||||
CancelParams, CancelResult, ErrorCode, Method, PingResult, ResponseBody, RpcError, RpcId,
|
||||
@@ -61,6 +62,10 @@ pub type RpcHandler = Arc<
|
||||
>;
|
||||
|
||||
const DEFAULT_RPC_TIMEOUT: Duration = Duration::from_secs(15);
|
||||
// The upload transaction owns its operation deadline and may need a bounded
|
||||
// cleanup before it can return a useful structured error. Keep only that
|
||||
// transport alive slightly longer so it does not replace the result.
|
||||
const EXTENSION_RESPONSE_GRACE: Duration = Duration::from_secs(2);
|
||||
/// Upper bound on `wait_for_browser_ms` accepted over IPC.
|
||||
const MAX_BROWSER_WAIT: Duration = Duration::from_secs(60);
|
||||
// `session.stop` fast-fails while another tool is active; this budget
|
||||
@@ -206,15 +211,15 @@ pub fn full_handler(status: DaemonStatus, state: Arc<DaemonState>) -> RpcHandler
|
||||
Ok(v) => ResponseBody::Ok(v),
|
||||
Err(e) => ResponseBody::Err(e),
|
||||
},
|
||||
Method::SessionStart => match handle_session_start(&state, params).await {
|
||||
Method::SessionStart => match handle_session_start(&state, rpc_id, params).await {
|
||||
Ok(v) => ResponseBody::Ok(v),
|
||||
Err(e) => ResponseBody::Err(e),
|
||||
},
|
||||
Method::SessionStop => match handle_session_stop(&state, params).await {
|
||||
Method::SessionStop => match handle_session_stop(&state, rpc_id, params).await {
|
||||
Ok(v) => ResponseBody::Ok(v),
|
||||
Err(e) => ResponseBody::Err(e),
|
||||
},
|
||||
Method::SessionStopAll => match handle_session_stop_all(&state).await {
|
||||
Method::SessionStopAll => match handle_session_stop_all_rpc(&state, rpc_id).await {
|
||||
Ok(v) => ResponseBody::Ok(v),
|
||||
Err(e) => ResponseBody::Err(e),
|
||||
},
|
||||
@@ -223,6 +228,11 @@ pub fn full_handler(status: DaemonStatus, state: Arc<DaemonState>) -> RpcHandler
|
||||
Ok(v) => ResponseBody::Ok(v),
|
||||
Err(e) => ResponseBody::Err(e),
|
||||
},
|
||||
Method::TransferBegin => handle_transfer_begin(&state, params),
|
||||
Method::TransferChunk => handle_transfer_chunk(&state, params),
|
||||
Method::TransferFinish => handle_transfer_finish(&state, params),
|
||||
Method::TransferRead => handle_transfer_read(&state, params),
|
||||
Method::TransferRelease => handle_transfer_release(&state, params),
|
||||
Method::ToolTabList
|
||||
| Method::ToolTabCreate
|
||||
| Method::ToolTabClose
|
||||
@@ -248,6 +258,8 @@ pub fn full_handler(status: DaemonStatus, state: Arc<DaemonState>) -> RpcHandler
|
||||
| Method::ToolFill
|
||||
| Method::ToolPress
|
||||
| Method::ToolSelect
|
||||
| Method::ToolUpload
|
||||
| Method::ToolDownload
|
||||
| Method::ToolEvaluate
|
||||
| Method::ToolWaitForNavigation
|
||||
| Method::ToolRequestHelp
|
||||
@@ -326,7 +338,7 @@ async fn handle_tool_dispatch(
|
||||
data: None,
|
||||
});
|
||||
}
|
||||
let timeout = match tool_dispatch_timeout(¶ms) {
|
||||
let timeout = match tool_dispatch_transport_timeout(&method, ¶ms) {
|
||||
Ok(timeout) => timeout,
|
||||
Err(err) => return ResponseBody::Err(err),
|
||||
};
|
||||
@@ -340,24 +352,180 @@ async fn handle_tool_dispatch(
|
||||
});
|
||||
}
|
||||
};
|
||||
let mut params = params;
|
||||
let mut download_transfer_id: Option<String> = None;
|
||||
if method == Method::ToolUpload {
|
||||
let mut upload: UploadParams = match serde_json::from_value(params) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(invalid_params(err.to_string())),
|
||||
};
|
||||
if upload.files.is_empty() || upload.files.len() > super::file_transfer::MAX_UPLOAD_FILES {
|
||||
return ResponseBody::Err(invalid_params(format!(
|
||||
"upload requires 1..={} files",
|
||||
super::file_transfer::MAX_UPLOAD_FILES
|
||||
)));
|
||||
}
|
||||
let ids: Vec<String> = upload.files.iter().map(|f| f.transfer_id.clone()).collect();
|
||||
let paths = match state.transfers.resolve_uploads(&session_id.0, &ids) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(err),
|
||||
};
|
||||
for (file, path) in upload.files.iter_mut().zip(paths) {
|
||||
file.staged_path = Some(path.to_string_lossy().into_owned());
|
||||
}
|
||||
params = serde_json::to_value(upload).unwrap_or(Value::Null);
|
||||
} else if method == Method::ToolDownload {
|
||||
let mut download: DownloadParams = match serde_json::from_value(params) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(invalid_params(err.to_string())),
|
||||
};
|
||||
let staging = match state.transfers.begin_download(&session_id.0) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(err),
|
||||
};
|
||||
download.browser_relative_dir = Some(staging.browser_relative_dir);
|
||||
download.max_byte_size = Some(super::file_transfer::MAX_TRANSFER_BYTES);
|
||||
download_transfer_id = Some(staging.transfer_id);
|
||||
params = serde_json::to_value(download).unwrap_or(Value::Null);
|
||||
}
|
||||
let entry = inflight_guard.entry();
|
||||
// `record_stop` must reach the extension while `record_await` holds the
|
||||
// serial busy lock — finishing the recording unblocks await.
|
||||
let outcome = if method == Method::ToolRecordStop {
|
||||
state
|
||||
.tool_queues
|
||||
.dispatch_unlocked(&session_id, method, params, timeout, Some(entry))
|
||||
.dispatch_unlocked(&session_id, method.clone(), params, timeout, Some(entry))
|
||||
.await
|
||||
} else {
|
||||
state
|
||||
.tool_queues
|
||||
.dispatch(&session_id, method, params, timeout, Some(entry))
|
||||
.dispatch(&session_id, method.clone(), params, timeout, Some(entry))
|
||||
.await
|
||||
};
|
||||
drop(inflight_guard);
|
||||
match outcome {
|
||||
Ok(v) if method == Method::ToolDownload => {
|
||||
let id = download_transfer_id.expect("download transfer allocated");
|
||||
let mut result: DownloadResult = match serde_json::from_value(v) {
|
||||
Ok(v) => v,
|
||||
Err(err) => {
|
||||
state
|
||||
.transfers
|
||||
.release(TransferIdParams { transfer_id: id });
|
||||
return ResponseBody::Err(RpcError {
|
||||
code: ErrorCode::ProtocolError,
|
||||
message: format!("invalid tool.download result: {err}"),
|
||||
data: None,
|
||||
});
|
||||
}
|
||||
};
|
||||
let Some(path) = result.browser_path.take() else {
|
||||
state
|
||||
.transfers
|
||||
.release(TransferIdParams { transfer_id: id });
|
||||
return ResponseBody::Err(RpcError {
|
||||
code: ErrorCode::ProtocolError,
|
||||
message: "tool.download returned no browser_path".into(),
|
||||
data: None,
|
||||
});
|
||||
};
|
||||
match state
|
||||
.transfers
|
||||
.import_download(&id, std::path::Path::new(&path))
|
||||
{
|
||||
Ok(size) => {
|
||||
result.byte_size = size;
|
||||
result.transfer_id = Some(id);
|
||||
ResponseBody::Ok(serde_json::to_value(result).unwrap_or(Value::Null))
|
||||
}
|
||||
Err(err) => {
|
||||
state
|
||||
.transfers
|
||||
.release(TransferIdParams { transfer_id: id });
|
||||
ResponseBody::Err(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(v) => ResponseBody::Ok(v),
|
||||
Err(err) => ResponseBody::Err(err.into_rpc()),
|
||||
Err(err) => {
|
||||
if let Some(id) = download_transfer_id {
|
||||
state
|
||||
.transfers
|
||||
.release(TransferIdParams { transfer_id: id });
|
||||
}
|
||||
ResponseBody::Err(err.into_rpc())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_transfer_begin(state: &Arc<DaemonState>, params: Value) -> ResponseBody {
|
||||
let p: TransferBeginParams = match serde_json::from_value(params) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(invalid_params(err.to_string())),
|
||||
};
|
||||
if state
|
||||
.sessions
|
||||
.get(&SessionId(p.session_id.clone()))
|
||||
.is_none()
|
||||
{
|
||||
return ResponseBody::Err(RpcError {
|
||||
code: ErrorCode::NotFound,
|
||||
message: format!("session {} unknown", p.session_id),
|
||||
data: None,
|
||||
});
|
||||
}
|
||||
match state.transfers.begin_upload(p) {
|
||||
Ok(v) => ResponseBody::Ok(serde_json::to_value(v).unwrap_or(Value::Null)),
|
||||
Err(err) => ResponseBody::Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_transfer_chunk(state: &Arc<DaemonState>, params: Value) -> ResponseBody {
|
||||
let p = match serde_json::from_value(params) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(invalid_params(err.to_string())),
|
||||
};
|
||||
match state.transfers.write_chunk(p) {
|
||||
Ok(v) => ResponseBody::Ok(serde_json::to_value(v).unwrap_or(Value::Null)),
|
||||
Err(err) => ResponseBody::Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_transfer_finish(state: &Arc<DaemonState>, params: Value) -> ResponseBody {
|
||||
let p = match serde_json::from_value(params) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(invalid_params(err.to_string())),
|
||||
};
|
||||
match state.transfers.finish_upload(p) {
|
||||
Ok(v) => ResponseBody::Ok(serde_json::to_value(v).unwrap_or(Value::Null)),
|
||||
Err(err) => ResponseBody::Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_transfer_read(state: &Arc<DaemonState>, params: Value) -> ResponseBody {
|
||||
let p = match serde_json::from_value(params) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(invalid_params(err.to_string())),
|
||||
};
|
||||
match state.transfers.read_chunk(p) {
|
||||
Ok(v) => ResponseBody::Ok(serde_json::to_value(v).unwrap_or(Value::Null)),
|
||||
Err(err) => ResponseBody::Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_transfer_release(state: &Arc<DaemonState>, params: Value) -> ResponseBody {
|
||||
let p: TransferIdParams = match serde_json::from_value(params) {
|
||||
Ok(v) => v,
|
||||
Err(err) => return ResponseBody::Err(invalid_params(err.to_string())),
|
||||
};
|
||||
ResponseBody::Ok(serde_json::to_value(state.transfers.release(p)).unwrap_or(Value::Null))
|
||||
}
|
||||
|
||||
fn invalid_params(message: impl Into<String>) -> RpcError {
|
||||
RpcError {
|
||||
code: ErrorCode::InvalidParams,
|
||||
message: message.into(),
|
||||
data: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -436,8 +604,9 @@ async fn handle_wait_ms(
|
||||
/// cancellation surfaces (M10.2 + review C2):
|
||||
///
|
||||
/// 1. [`AbortRegistry`] — answers daemon-local cancellable runners
|
||||
/// (currently `tool.wait_ms`). If a token is registered we trip it
|
||||
/// and stop.
|
||||
/// (`tool.wait_ms` and `session.*` lifecycle calls). If a token is
|
||||
/// registered we trip it and stop. Lifecycle handlers forward their
|
||||
/// own WS cancel after preserving request-before-cancel ordering.
|
||||
/// 2. [`super::inflight::ToolInflightRegistry`] — every IPC-tracked
|
||||
/// `tool.*` RPC, registered the moment the IPC handler accepts the
|
||||
/// request. Trips the entry's cancel token regardless of whether
|
||||
@@ -534,6 +703,16 @@ fn tool_dispatch_timeout(params: &Value) -> Result<Duration, RpcError> {
|
||||
Ok(Duration::from_millis(u64::from(ms)))
|
||||
}
|
||||
|
||||
fn tool_dispatch_transport_timeout(method: &Method, params: &Value) -> Result<Duration, RpcError> {
|
||||
tool_dispatch_timeout(params).map(|timeout| {
|
||||
if matches!(method, Method::ToolUpload | Method::ToolDownload) {
|
||||
timeout.saturating_add(EXTENSION_RESPONSE_GRACE)
|
||||
} else {
|
||||
timeout
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Local CLI-facing shapes. Intentionally distinct from
|
||||
// `bsk_protocol::tools::SessionStart*` (which describes the WS-facing
|
||||
// `tool.session_*` round-trip with the extension) so the two sides can
|
||||
@@ -636,7 +815,20 @@ fn clamp_browser_wait(wait_ms: Option<u64>) -> Option<Duration> {
|
||||
))
|
||||
}
|
||||
|
||||
async fn handle_session_start(state: &Arc<DaemonState>, params: Value) -> Result<Value, RpcError> {
|
||||
async fn handle_session_start(
|
||||
state: &Arc<DaemonState>,
|
||||
rpc_id: RpcId,
|
||||
params: Value,
|
||||
) -> Result<Value, RpcError> {
|
||||
let abort_guard = state
|
||||
.abort_registry
|
||||
.register(rpc_id)
|
||||
.map_err(|err| RpcError {
|
||||
code: ErrorCode::ProtocolError,
|
||||
message: format!("session.start cancellation registration failed: {err:?}"),
|
||||
data: None,
|
||||
})?;
|
||||
let cancel = abort_guard.token().clone();
|
||||
let params: CliSessionStartParams = if params.is_null() {
|
||||
CliSessionStartParams {
|
||||
browser_instance_id: None,
|
||||
@@ -675,6 +867,7 @@ async fn handle_session_start(state: &Arc<DaemonState>, params: Value) -> Result
|
||||
},
|
||||
state.config.extension_connect_wait,
|
||||
DEFAULT_RPC_TIMEOUT,
|
||||
Some(cancel),
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -698,6 +891,8 @@ fn map_start_error(err: StartSessionError) -> RpcError {
|
||||
StartSessionError::AmbiguousBrowserLabel { .. } => ErrorCode::InvalidParams,
|
||||
StartSessionError::IdExhausted => ErrorCode::ProtocolError,
|
||||
StartSessionError::Timeout => ErrorCode::Timeout,
|
||||
StartSessionError::Cancelled => ErrorCode::Cancelled,
|
||||
StartSessionError::CleanupFailed { .. } => ErrorCode::ProtocolError,
|
||||
StartSessionError::TransportClosed => ErrorCode::ProtocolError,
|
||||
StartSessionError::ExtensionError(inner) => inner.code,
|
||||
};
|
||||
@@ -713,6 +908,16 @@ fn map_start_error(err: StartSessionError) -> RpcError {
|
||||
"label": label,
|
||||
"instance_ids": instance_ids,
|
||||
})),
|
||||
StartSessionError::CleanupFailed {
|
||||
session_id,
|
||||
agent_window_id,
|
||||
..
|
||||
} => Some(serde_json::json!({
|
||||
"reason": "cleanup_failed",
|
||||
"session_id": session_id.0,
|
||||
"agent_window_id": agent_window_id,
|
||||
})),
|
||||
StartSessionError::ExtensionError(inner) => inner.data.clone(),
|
||||
_ => None,
|
||||
};
|
||||
RpcError {
|
||||
@@ -722,7 +927,20 @@ fn map_start_error(err: StartSessionError) -> RpcError {
|
||||
}
|
||||
}
|
||||
|
||||
async fn handle_session_stop(state: &Arc<DaemonState>, params: Value) -> Result<Value, RpcError> {
|
||||
async fn handle_session_stop(
|
||||
state: &Arc<DaemonState>,
|
||||
rpc_id: RpcId,
|
||||
params: Value,
|
||||
) -> Result<Value, RpcError> {
|
||||
let abort_guard = state
|
||||
.abort_registry
|
||||
.register(rpc_id)
|
||||
.map_err(|err| RpcError {
|
||||
code: ErrorCode::ProtocolError,
|
||||
message: format!("session.stop cancellation registration failed: {err:?}"),
|
||||
data: None,
|
||||
})?;
|
||||
let cancel = abort_guard.token().clone();
|
||||
let params: CliSessionStopParams = if params.is_null() {
|
||||
CliSessionStopParams {
|
||||
session_id: None,
|
||||
@@ -736,7 +954,7 @@ async fn handle_session_stop(state: &Arc<DaemonState>, params: Value) -> Result<
|
||||
})?
|
||||
};
|
||||
if params.all {
|
||||
return handle_session_stop_all(state).await;
|
||||
return handle_session_stop_all(state, Some(cancel)).await;
|
||||
}
|
||||
let session_id = match params.session_id {
|
||||
Some(s) => SessionId(s),
|
||||
@@ -755,10 +973,12 @@ async fn handle_session_stop(state: &Arc<DaemonState>, params: Value) -> Result<
|
||||
&state.session_interrupts,
|
||||
&session_id,
|
||||
DEFAULT_SESSION_STOP_TIMEOUT,
|
||||
Some(cancel),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(stop) => {
|
||||
state.transfers.release_session(&session_id.0);
|
||||
let result = CliSessionStopResult {
|
||||
stopped: vec![session_id.0],
|
||||
failed: Vec::new(),
|
||||
@@ -806,7 +1026,25 @@ fn map_stop_error(err: StopSessionError) -> RpcError {
|
||||
}
|
||||
}
|
||||
|
||||
async fn handle_session_stop_all(state: &Arc<DaemonState>) -> Result<Value, RpcError> {
|
||||
async fn handle_session_stop_all_rpc(
|
||||
state: &Arc<DaemonState>,
|
||||
rpc_id: RpcId,
|
||||
) -> Result<Value, RpcError> {
|
||||
let abort_guard = state
|
||||
.abort_registry
|
||||
.register(rpc_id)
|
||||
.map_err(|err| RpcError {
|
||||
code: ErrorCode::ProtocolError,
|
||||
message: format!("session.stop_all cancellation registration failed: {err:?}"),
|
||||
data: None,
|
||||
})?;
|
||||
handle_session_stop_all(state, Some(abort_guard.token().clone())).await
|
||||
}
|
||||
|
||||
async fn handle_session_stop_all(
|
||||
state: &Arc<DaemonState>,
|
||||
cancel: Option<super::abort::AbortToken>,
|
||||
) -> Result<Value, RpcError> {
|
||||
let ids: Vec<SessionId> = state
|
||||
.sessions
|
||||
.snapshot()
|
||||
@@ -818,6 +1056,16 @@ async fn handle_session_stop_all(state: &Arc<DaemonState>) -> Result<Value, RpcE
|
||||
let mut returned_tab_ids = Vec::new();
|
||||
let mut return_failures = Vec::new();
|
||||
for id in ids {
|
||||
if cancel
|
||||
.as_ref()
|
||||
.is_some_and(super::abort::AbortToken::is_cancelled)
|
||||
{
|
||||
return Err(RpcError {
|
||||
code: ErrorCode::Cancelled,
|
||||
message: "session.stop_all was cancelled".into(),
|
||||
data: None,
|
||||
});
|
||||
}
|
||||
match stop_session(
|
||||
&state.browsers,
|
||||
&state.sessions,
|
||||
@@ -825,10 +1073,12 @@ async fn handle_session_stop_all(state: &Arc<DaemonState>) -> Result<Value, RpcE
|
||||
&state.session_interrupts,
|
||||
&id,
|
||||
DEFAULT_SESSION_STOP_TIMEOUT,
|
||||
cancel.clone(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(stop) => {
|
||||
state.transfers.release_session(&id.0);
|
||||
stopped.push(id.0);
|
||||
returned_tab_ids.extend(stop.returned_tab_ids);
|
||||
return_failures.extend(stop.return_failures);
|
||||
@@ -850,6 +1100,15 @@ async fn handle_session_stop_all(state: &Arc<DaemonState>) -> Result<Value, RpcE
|
||||
returned_tab_ids.extend(stop.returned_tab_ids);
|
||||
return_failures.extend(stop.return_failures);
|
||||
}
|
||||
Err(err)
|
||||
if matches!(
|
||||
&err,
|
||||
StopSessionError::ExtensionError(inner)
|
||||
if inner.code == ErrorCode::Cancelled
|
||||
) =>
|
||||
{
|
||||
return Err(map_stop_error(err));
|
||||
}
|
||||
Err(err) => {
|
||||
debug!(session = %id, ?err, "session.stop_all: failure (continuing)");
|
||||
let rpc = map_stop_error(err);
|
||||
@@ -1142,6 +1401,28 @@ mod windows {
|
||||
connected = pipe.connect() => {
|
||||
match connected {
|
||||
Ok(()) => {
|
||||
// Keep the connected instance alive until its replacement
|
||||
// exists. Otherwise a fast handler can close the last
|
||||
// instance and make new clients fail with NotFound.
|
||||
loop {
|
||||
match ServerOptions::new()
|
||||
.access_inbound(true)
|
||||
.access_outbound(true)
|
||||
.create(&listener.pipe_name)
|
||||
{
|
||||
Ok(next) => {
|
||||
listener.first = Some(next);
|
||||
break;
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(?err, "create next named-pipe instance failed");
|
||||
tokio::select! {
|
||||
_ = &mut shutdown => return,
|
||||
_ = tokio::time::sleep(std::time::Duration::from_millis(50)) => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
on_open();
|
||||
let handler = handler.clone();
|
||||
let on_act = on_activity.clone();
|
||||
@@ -1210,6 +1491,118 @@ mod windows {
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::sync::Mutex;
|
||||
use std::time::Duration;
|
||||
use tokio::net::windows::named_pipe::ClientOptions;
|
||||
use tokio::sync::oneshot;
|
||||
|
||||
fn isolated_listener() -> NamedPipeListener {
|
||||
let pipe_name = format!(r"\\.\pipe\bsk-test-{}", uuid::Uuid::new_v4());
|
||||
let first = ServerOptions::new()
|
||||
.first_pipe_instance(true)
|
||||
.create(&pipe_name)
|
||||
.unwrap();
|
||||
NamedPipeListener {
|
||||
pipe_name,
|
||||
first: Some(first),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn next_instance_exists_before_connection_is_handed_off() {
|
||||
let listener = isolated_listener();
|
||||
let name = listener.pipe_name.clone();
|
||||
let probe_name = name.clone();
|
||||
let (probe_tx, probe_rx) = oneshot::channel();
|
||||
let probe_tx = Mutex::new(Some(probe_tx));
|
||||
let (stop_tx, stop_rx) = oneshot::channel();
|
||||
let server = tokio::spawn(serve(
|
||||
listener,
|
||||
crate::daemon::ipc::default_ping_handler(),
|
||||
move || {
|
||||
if let Some(tx) = probe_tx.lock().unwrap().take() {
|
||||
// This callback runs before the connection task can finish.
|
||||
// A second client must already have an instance to open.
|
||||
let result = ClientOptions::new().open(&probe_name).map(drop);
|
||||
let _ = tx.send(result);
|
||||
}
|
||||
},
|
||||
|| {},
|
||||
|| {},
|
||||
async {
|
||||
let _ = stop_rx.await;
|
||||
},
|
||||
));
|
||||
let client = ClientOptions::new().open(&name).unwrap();
|
||||
let result = tokio::time::timeout(Duration::from_secs(5), probe_rx).await;
|
||||
drop(client);
|
||||
let _ = stop_tx.send(());
|
||||
server.await.unwrap();
|
||||
result
|
||||
.expect("accept callback ran")
|
||||
.unwrap()
|
||||
.expect("next pipe instance is ready");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
||||
async fn rapid_disconnects_and_concurrent_rpc_connections() {
|
||||
let listener = isolated_listener();
|
||||
let name = listener.pipe_name.clone();
|
||||
let (stop_tx, stop_rx) = oneshot::channel();
|
||||
let server = tokio::spawn(serve(
|
||||
listener,
|
||||
crate::daemon::ipc::default_ping_handler(),
|
||||
|| {},
|
||||
|| {},
|
||||
|| {},
|
||||
async {
|
||||
let _ = stop_rx.await;
|
||||
},
|
||||
));
|
||||
let exercise = async {
|
||||
// Include clients that close without sending any request.
|
||||
for _ in 0..64 {
|
||||
drop(
|
||||
crate::ipc_client::Client::connect_path(name.clone().into())
|
||||
.await
|
||||
.unwrap(),
|
||||
);
|
||||
}
|
||||
let mut clients = tokio::task::JoinSet::new();
|
||||
for _ in 0..8 {
|
||||
let name = name.clone();
|
||||
clients.spawn(async move {
|
||||
for _ in 0..32 {
|
||||
let mut client =
|
||||
crate::ipc_client::Client::connect_path(name.clone().into())
|
||||
.await
|
||||
.unwrap();
|
||||
let reply: bsk_protocol::PingResult = client
|
||||
.call(
|
||||
bsk_protocol::Method::SystemPing,
|
||||
&serde_json::json!({}),
|
||||
Duration::from_secs(5),
|
||||
)
|
||||
.await
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert!(reply.pong);
|
||||
}
|
||||
});
|
||||
}
|
||||
while let Some(result) = clients.join_next().await {
|
||||
result.unwrap();
|
||||
}
|
||||
};
|
||||
let result = tokio::time::timeout(Duration::from_secs(30), exercise).await;
|
||||
let _ = stop_tx.send(());
|
||||
server.await.unwrap();
|
||||
result.expect("connection exercise completed");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
@@ -1252,6 +1645,30 @@ mod tests {
|
||||
assert_eq!(got, std::time::Duration::from_millis(300_000));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extension_transport_outlives_the_operation_deadline() {
|
||||
let params = serde_json::json!({
|
||||
"session_id": "abcd",
|
||||
"timeout_ms": 60_000,
|
||||
});
|
||||
let dispatch_timeout =
|
||||
tool_dispatch_transport_timeout(&Method::ToolUpload, ¶ms).unwrap();
|
||||
|
||||
assert_eq!(dispatch_timeout, Duration::from_secs(62));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extension_response_grace_does_not_change_other_tools() {
|
||||
let params = serde_json::json!({
|
||||
"session_id": "abcd",
|
||||
"timeout_ms": 60_000,
|
||||
});
|
||||
let dispatch_timeout =
|
||||
tool_dispatch_transport_timeout(&Method::ToolClick, ¶ms).unwrap();
|
||||
|
||||
assert_eq!(dispatch_timeout, Duration::from_secs(60));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_dispatch_timeout_rejects_zero_timeout_ms() {
|
||||
let params = serde_json::json!({
|
||||
|
||||
@@ -90,17 +90,21 @@ pub fn pid_alive(pid: u32) -> bool {
|
||||
|
||||
#[cfg(windows)]
|
||||
{
|
||||
use windows_sys::Win32::Foundation::CloseHandle;
|
||||
use windows_sys::Win32::Foundation::{CloseHandle, WAIT_TIMEOUT};
|
||||
use windows_sys::Win32::System::Threading::{
|
||||
OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION,
|
||||
OpenProcess, PROCESS_SYNCHRONIZE, WaitForSingleObject,
|
||||
};
|
||||
unsafe {
|
||||
let handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid);
|
||||
let handle = OpenProcess(PROCESS_SYNCHRONIZE, 0, pid);
|
||||
if handle.is_null() {
|
||||
return false;
|
||||
}
|
||||
// An exited process can still be opened while another handle
|
||||
// keeps its kernel object alive. Poll its termination signal;
|
||||
// unlike checking STILL_ACTIVE, this also handles exit code 259.
|
||||
let alive = WaitForSingleObject(handle, 0) == WAIT_TIMEOUT;
|
||||
CloseHandle(handle);
|
||||
true
|
||||
alive
|
||||
}
|
||||
}
|
||||
|
||||
@@ -110,3 +114,39 @@ pub fn pid_alive(pid: u32) -> bool {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, windows))]
|
||||
mod tests {
|
||||
use super::pid_alive;
|
||||
use std::os::windows::process::CommandExt;
|
||||
use std::process::Command;
|
||||
|
||||
#[test]
|
||||
fn current_process_is_alive() {
|
||||
assert!(pid_alive(std::process::id()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_process_is_not_alive() {
|
||||
assert!(!pid_alive(0));
|
||||
assert!(!pid_alive(u32::MAX));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exited_process_with_retained_handle_is_not_alive() {
|
||||
for exit_code in [0, 259] {
|
||||
let mut child = Command::new("cmd.exe")
|
||||
.args(["/D", "/C", &format!("exit {exit_code}")])
|
||||
.creation_flags(0x0800_0000) // CREATE_NO_WINDOW
|
||||
.spawn()
|
||||
.expect("spawn short-lived process");
|
||||
assert_eq!(child.wait().unwrap().code(), Some(exit_code));
|
||||
// Keep Child (and its process handle) alive during the probe.
|
||||
assert!(
|
||||
!pid_alive(child.id()),
|
||||
"exited process with code {exit_code}"
|
||||
);
|
||||
drop(child);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
pub mod abort;
|
||||
pub mod browsers;
|
||||
mod cancel_forward;
|
||||
pub mod file_transfer;
|
||||
pub mod inflight;
|
||||
pub mod info;
|
||||
pub mod ipc;
|
||||
|
||||
@@ -100,6 +100,14 @@ pub fn record_session_path() -> Result<PathBuf> {
|
||||
Ok(bsk_home()?.join("record-session.json"))
|
||||
}
|
||||
|
||||
/// Path to a completed Trace saved before bundle export (`record-recovery.json`).
|
||||
///
|
||||
/// Kept until export succeeds so a failed `--output` write cannot drop the
|
||||
/// recording the extension already returned.
|
||||
pub fn record_recovery_path() -> Result<PathBuf> {
|
||||
Ok(bsk_home()?.join("record-recovery.json"))
|
||||
}
|
||||
|
||||
/// Windows named-pipe name. Include the resolved `BSK_HOME` path in the
|
||||
/// token so test homes and custom installs do not share a predictable
|
||||
/// per-username pipe.
|
||||
@@ -119,6 +127,13 @@ pub fn pipe_name() -> String {
|
||||
render_pipe_name(&user, &home)
|
||||
}
|
||||
|
||||
/// Process-wide lock for tests that mutate `BSK_HOME`.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn test_env_lock() -> std::sync::MutexGuard<'static, ()> {
|
||||
static GUARD: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
GUARD.lock().unwrap_or_else(|e| e.into_inner())
|
||||
}
|
||||
|
||||
/// Render the Windows named-pipe name for a user/home pair. Kept
|
||||
/// platform-agnostic so unit tests on any host can pin the output
|
||||
/// character set.
|
||||
@@ -136,19 +151,12 @@ fn render_pipe_name(user: &str, home: &str) -> String {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::sync::Mutex;
|
||||
use tempfile::TempDir;
|
||||
|
||||
/// Serialise tests that mutate the global `BSK_HOME` env var.
|
||||
fn env_guard() -> &'static Mutex<()> {
|
||||
static GUARD: Mutex<()> = Mutex::new(());
|
||||
&GUARD
|
||||
}
|
||||
|
||||
fn with_temp_home<F: FnOnce(&Path)>(f: F) {
|
||||
let _lock = env_guard().lock().unwrap_or_else(|e| e.into_inner());
|
||||
let _lock = test_env_lock();
|
||||
let tmp = TempDir::new().unwrap();
|
||||
// SAFETY: serialised by env_guard above.
|
||||
// SAFETY: serialised by test_env_lock above.
|
||||
unsafe {
|
||||
std::env::set_var(BSK_HOME_ENV, tmp.path().join("bsk"));
|
||||
}
|
||||
@@ -187,6 +195,10 @@ mod tests {
|
||||
record_session_path().unwrap(),
|
||||
home.join("record-session.json")
|
||||
);
|
||||
assert_eq!(
|
||||
record_recovery_path().unwrap(),
|
||||
home.join("record-recovery.json")
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -43,6 +43,7 @@ use serde_json::Value;
|
||||
use tokio::sync::{mpsc, oneshot};
|
||||
use tracing::{debug, warn};
|
||||
|
||||
use super::abort::AbortToken;
|
||||
use super::browsers::BrowserRegistry;
|
||||
use super::inflight::{PromoteOutcome, ToolInflightEntry};
|
||||
use super::sessions::{SessionId, SessionRegistry};
|
||||
@@ -83,6 +84,14 @@ pub struct ToolJob {
|
||||
/// IPC request id (e.g. `session.stop`'s queued teardown call —
|
||||
/// those already carry their own bespoke retry / abort path).
|
||||
pub inflight: Option<Arc<ToolInflightEntry>>,
|
||||
/// Cancellation token for session-lifecycle work registered in the
|
||||
/// daemon-local abort registry. Unlike `inflight`, lifecycle callers
|
||||
/// forward their own WS cancel after the request has been queued.
|
||||
pub lifecycle_cancel: Option<AbortToken>,
|
||||
/// How long cancellation keeps the worker busy while the extension
|
||||
/// finishes compensation. Session teardown uses its full RPC budget
|
||||
/// so daemon state is reconciled even after the CLI exits.
|
||||
pub cancel_cleanup_timeout: Duration,
|
||||
/// Owning session — used by `ToolInflightRegistry::cancel_session`
|
||||
/// to drain queued jobs that have not yet been promoted to
|
||||
/// "forwarded". Worker callers also have access to it via the
|
||||
@@ -322,6 +331,8 @@ impl ToolQueueRegistry {
|
||||
timeout,
|
||||
false,
|
||||
inflight,
|
||||
None,
|
||||
CANCEL_CLEANUP_TIMEOUT,
|
||||
)
|
||||
.await;
|
||||
// A long-running request may outlive the idle threshold. Touching
|
||||
@@ -360,6 +371,8 @@ impl ToolQueueRegistry {
|
||||
timeout,
|
||||
respond: respond_tx,
|
||||
inflight,
|
||||
lifecycle_cancel: None,
|
||||
cancel_cleanup_timeout: CANCEL_CLEANUP_TIMEOUT,
|
||||
session_id: sid.clone(),
|
||||
};
|
||||
match forward_one(sid, &self.browsers, &self.sessions, &job).await {
|
||||
@@ -378,6 +391,7 @@ impl ToolQueueRegistry {
|
||||
method: Method,
|
||||
params: Value,
|
||||
timeout: Duration,
|
||||
cancel: Option<AbortToken>,
|
||||
) -> Result<Value, DispatchError> {
|
||||
let (sender, state) = {
|
||||
let mut guard = self.queues.lock().expect("tool queue registry poisoned");
|
||||
@@ -404,6 +418,8 @@ impl ToolQueueRegistry {
|
||||
timeout,
|
||||
true,
|
||||
None,
|
||||
cancel,
|
||||
timeout,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -419,7 +435,10 @@ async fn dispatch_with_sender(
|
||||
timeout: Duration,
|
||||
wait_for_capacity: bool,
|
||||
inflight: Option<Arc<ToolInflightEntry>>,
|
||||
lifecycle_cancel: Option<AbortToken>,
|
||||
cancel_cleanup_timeout: Duration,
|
||||
) -> Result<Value, DispatchError> {
|
||||
let effect_aware_transfer = is_effect_aware_transfer(&method);
|
||||
let (respond_tx, respond_rx) = oneshot::channel();
|
||||
let job = ToolJob {
|
||||
method,
|
||||
@@ -427,8 +446,11 @@ async fn dispatch_with_sender(
|
||||
timeout,
|
||||
respond: respond_tx,
|
||||
inflight,
|
||||
lifecycle_cancel,
|
||||
cancel_cleanup_timeout,
|
||||
session_id,
|
||||
};
|
||||
let lifecycle_cancellable = job.lifecycle_cancel.is_some();
|
||||
if wait_for_capacity {
|
||||
let waited = tokio::time::timeout(
|
||||
timeout.saturating_add(Duration::from_secs(1)),
|
||||
@@ -453,8 +475,21 @@ async fn dispatch_with_sender(
|
||||
mpsc::error::TrySendError::Closed(_) => DispatchError::QueueClosed,
|
||||
});
|
||||
}
|
||||
let waited =
|
||||
tokio::time::timeout(timeout.saturating_add(Duration::from_secs(1)), respond_rx).await;
|
||||
// Lifecycle teardown and effect-aware transfers both keep the worker busy
|
||||
// for bounded compensation after their original deadline. Keep the outer
|
||||
// waiter alive for the same grace period so it cannot abandon reconciliation.
|
||||
let response_grace = if lifecycle_cancellable || effect_aware_transfer {
|
||||
cancel_cleanup_timeout
|
||||
} else {
|
||||
Duration::ZERO
|
||||
};
|
||||
let waited = tokio::time::timeout(
|
||||
timeout
|
||||
.saturating_add(response_grace)
|
||||
.saturating_add(Duration::from_secs(1)),
|
||||
respond_rx,
|
||||
)
|
||||
.await;
|
||||
match waited {
|
||||
Ok(Ok(Ok(v))) => Ok(v),
|
||||
Ok(Ok(Err(rpc))) => Err(DispatchError::Rpc(rpc)),
|
||||
@@ -492,11 +527,17 @@ async fn forward_one(
|
||||
// / browser resolution work, and before any WS frame leaves the
|
||||
// daemon (review C2). We keep the `tokio::select!` later so
|
||||
// cancels arriving mid-WS-hop also unblock the worker.
|
||||
if let Some(entry) = job.inflight.as_ref()
|
||||
&& entry.is_cancelled()
|
||||
if job
|
||||
.inflight
|
||||
.as_ref()
|
||||
.is_some_and(|entry| entry.is_cancelled())
|
||||
|| job
|
||||
.lifecycle_cancel
|
||||
.as_ref()
|
||||
.is_some_and(AbortToken::is_cancelled)
|
||||
{
|
||||
return Err(cancelled_error(
|
||||
Some(entry),
|
||||
job.inflight.as_deref(),
|
||||
"tool dispatch cancelled before forwarding",
|
||||
));
|
||||
}
|
||||
@@ -560,11 +601,20 @@ async fn forward_one(
|
||||
}
|
||||
}
|
||||
None => {
|
||||
// Daemon-internal callers (e.g. queued session.stop drain)
|
||||
// don't carry an inflight entry; they cannot be cancelled
|
||||
// through the IPC `cancel` surface, so a plain sink send
|
||||
// is sufficient and keeps the existing transport-error
|
||||
// behaviour.
|
||||
// Daemon-internal callers do not carry a tool-inflight entry.
|
||||
// Session lifecycle jobs instead use the abort-registry token
|
||||
// below; other internal callers keep the plain send path.
|
||||
if job
|
||||
.lifecycle_cancel
|
||||
.as_ref()
|
||||
.is_some_and(AbortToken::is_cancelled)
|
||||
{
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
return Err(cancelled_error(
|
||||
None,
|
||||
"session lifecycle cancelled before forwarding",
|
||||
));
|
||||
}
|
||||
if client.sink.send(request).is_err() {
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
return Err(RpcError {
|
||||
@@ -573,46 +623,161 @@ async fn forward_one(
|
||||
data: None,
|
||||
});
|
||||
}
|
||||
None
|
||||
job.lifecycle_cancel.clone()
|
||||
}
|
||||
};
|
||||
let forward_lifecycle_cancel = || {
|
||||
if job.lifecycle_cancel.is_none() {
|
||||
return;
|
||||
}
|
||||
let cancel = Frame::Request(RequestFrame {
|
||||
id: format!("cancel-{rpc_id}"),
|
||||
method: Method::Cancel,
|
||||
params: Some(serde_json::json!({ "rpc_id": rpc_id })),
|
||||
});
|
||||
if let Err(err) = client.sink.send(cancel) {
|
||||
warn!(%rpc_id, ?err, "failed to forward session lifecycle cancel");
|
||||
}
|
||||
};
|
||||
let on_abort: Option<&(dyn Fn() + Sync)> = job
|
||||
.lifecycle_cancel
|
||||
.as_ref()
|
||||
.map(|_| &forward_lifecycle_cancel as &(dyn Fn() + Sync));
|
||||
let send_deadline_cancel = || {
|
||||
client
|
||||
.sink
|
||||
.send(Frame::Request(RequestFrame {
|
||||
id: format!("deadline-cancel-{rpc_id}"),
|
||||
method: Method::Cancel,
|
||||
params: Some(serde_json::json!({ "rpc_id": rpc_id })),
|
||||
}))
|
||||
.is_ok()
|
||||
};
|
||||
let deadline_cancel: Option<&(dyn Fn() -> bool + Sync)> =
|
||||
is_effect_aware_transfer(&job.method).then_some(&send_deadline_cancel);
|
||||
let waited = await_with_optional_cancel(
|
||||
job.timeout,
|
||||
CANCEL_CLEANUP_TIMEOUT,
|
||||
job.cancel_cleanup_timeout,
|
||||
waiter,
|
||||
cancel_token.as_ref(),
|
||||
on_abort,
|
||||
deadline_cancel,
|
||||
)
|
||||
.await;
|
||||
let response = match waited {
|
||||
WaitOutcome::Response(resp) => resp,
|
||||
WaitOutcome::CancelledAfterResponse(resp) => {
|
||||
// Cancellation wins the external verdict, but only after the
|
||||
// extension's original RPC has settled. Preserve any non-cancel
|
||||
// error so compensation failures remain explicit instead of
|
||||
// being hidden behind a generic cancelled result.
|
||||
if let ResponseBody::Err(err) = resp.body
|
||||
&& !matches!(err.code, ErrorCode::Cancelled | ErrorCode::UserAborted)
|
||||
if job.method == Method::ToolSessionStop
|
||||
&& let ResponseBody::Ok(value) = &resp.body
|
||||
{
|
||||
// Teardown crossed its final irreversible boundary before
|
||||
// the cancel landed. Commit the real extension result so the
|
||||
// daemon does not retain a session whose window is gone.
|
||||
return Ok(value.clone());
|
||||
}
|
||||
// File transfer commits are irreversible. A late cancel cannot
|
||||
// overwrite a confirmed success, and an unknown transfer effect
|
||||
// must remain explicit so callers do not retry or release upload
|
||||
// staging as though nothing happened.
|
||||
if is_effect_aware_transfer(&job.method) {
|
||||
match &resp.body {
|
||||
ResponseBody::Ok(_) => resp,
|
||||
ResponseBody::Err(err)
|
||||
if transfer_effect(err) == Some("unknown")
|
||||
|| transfer_effect(err) == Some("committed") =>
|
||||
{
|
||||
return Err(err.clone());
|
||||
}
|
||||
ResponseBody::Err(err)
|
||||
if !matches!(err.code, ErrorCode::Cancelled | ErrorCode::UserAborted) =>
|
||||
{
|
||||
return Err(err.clone());
|
||||
}
|
||||
ResponseBody::Err(_) => {
|
||||
return Err(cancelled_error(
|
||||
job.inflight.as_deref(),
|
||||
"tool dispatch cancelled after extension cleanup",
|
||||
));
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// For ordinary tools cancellation keeps the existing verdict,
|
||||
// while non-cancel errors still expose compensation failures.
|
||||
if let ResponseBody::Err(err) = resp.body
|
||||
&& !matches!(err.code, ErrorCode::Cancelled | ErrorCode::UserAborted)
|
||||
{
|
||||
return Err(err);
|
||||
}
|
||||
return Err(cancelled_error(
|
||||
job.inflight.as_deref(),
|
||||
"tool dispatch cancelled after extension cleanup",
|
||||
));
|
||||
}
|
||||
}
|
||||
WaitOutcome::TimedOutAfterResponse(resp) => match resp.body {
|
||||
ResponseBody::Ok(value)
|
||||
if job.method == Method::ToolSessionStop
|
||||
|| is_effect_aware_transfer(&job.method) =>
|
||||
{
|
||||
// The close crossed its irreversible boundary during the
|
||||
// timeout cleanup grace, or the transfer committed before its
|
||||
// deadline cancel settled. Preserve the irreversible result.
|
||||
return Ok(value);
|
||||
}
|
||||
ResponseBody::Err(err)
|
||||
if !matches!(err.code, ErrorCode::Cancelled | ErrorCode::UserAborted) =>
|
||||
{
|
||||
return Err(err);
|
||||
}
|
||||
return Err(cancelled_error(
|
||||
job.inflight.as_deref(),
|
||||
"tool dispatch cancelled after extension cleanup",
|
||||
));
|
||||
}
|
||||
ResponseBody::Err(err) if is_effect_aware_transfer(&job.method) => {
|
||||
return Err(timed_out_transfer_error(&err));
|
||||
}
|
||||
_ => {
|
||||
return Err(RpcError {
|
||||
code: ErrorCode::Timeout,
|
||||
message: format!("tool RPC timed out after {:?}", job.timeout),
|
||||
data: None,
|
||||
});
|
||||
}
|
||||
},
|
||||
WaitOutcome::CleanupTimeout => {
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
if is_effect_aware_transfer(&job.method) {
|
||||
return Err(unknown_transfer_error(
|
||||
ErrorCode::Timeout,
|
||||
"cancelled file transfer did not confirm its outcome before cleanup timed out",
|
||||
"cleanup",
|
||||
true,
|
||||
));
|
||||
}
|
||||
return Err(RpcError {
|
||||
code: ErrorCode::Timeout,
|
||||
message: format!(
|
||||
"cancelled tool did not finish cleanup within {:?}",
|
||||
CANCEL_CLEANUP_TIMEOUT
|
||||
job.cancel_cleanup_timeout
|
||||
),
|
||||
data: Some(serde_json::json!({ "reason": "cancel_cleanup_timeout" })),
|
||||
});
|
||||
}
|
||||
WaitOutcome::TimeoutCleanupFailed => {
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
return Err(unknown_transfer_error(
|
||||
ErrorCode::Timeout,
|
||||
"file transfer timed out and cleanup could not be confirmed",
|
||||
"cleanup",
|
||||
true,
|
||||
));
|
||||
}
|
||||
WaitOutcome::WaiterClosed => {
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
if is_effect_aware_transfer(&job.method) {
|
||||
return Err(unknown_transfer_error(
|
||||
ErrorCode::ProtocolError,
|
||||
"file transfer transport closed after dispatch; outcome is unknown",
|
||||
"transport",
|
||||
false,
|
||||
));
|
||||
}
|
||||
return Err(RpcError {
|
||||
code: ErrorCode::ProtocolError,
|
||||
message: "transport closed mid-call".into(),
|
||||
@@ -621,6 +786,14 @@ async fn forward_one(
|
||||
}
|
||||
WaitOutcome::Timeout => {
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
if is_effect_aware_transfer(&job.method) {
|
||||
return Err(unknown_transfer_error(
|
||||
ErrorCode::Timeout,
|
||||
"file transfer timed out after dispatch; outcome is unknown",
|
||||
"transport",
|
||||
false,
|
||||
));
|
||||
}
|
||||
return Err(RpcError {
|
||||
code: ErrorCode::Timeout,
|
||||
message: format!("tool RPC timed out after {:?}", job.timeout),
|
||||
@@ -634,11 +807,65 @@ async fn forward_one(
|
||||
}
|
||||
}
|
||||
|
||||
fn is_effect_aware_transfer(method: &Method) -> bool {
|
||||
matches!(method, Method::ToolUpload | Method::ToolDownload)
|
||||
}
|
||||
|
||||
fn transfer_effect(err: &RpcError) -> Option<&str> {
|
||||
err.data.as_ref()?.get("effect_state")?.as_str()
|
||||
}
|
||||
|
||||
fn unknown_transfer_error(
|
||||
code: ErrorCode,
|
||||
message: impl Into<String>,
|
||||
phase: &str,
|
||||
cleanup_failed: bool,
|
||||
) -> RpcError {
|
||||
let mut data = serde_json::json!({
|
||||
"reason": "transfer_outcome_unknown",
|
||||
"effect_state": "unknown",
|
||||
"phase": phase,
|
||||
});
|
||||
if cleanup_failed {
|
||||
data["cleanup_state"] = serde_json::json!("failed");
|
||||
}
|
||||
RpcError {
|
||||
code,
|
||||
message: message.into(),
|
||||
data: Some(data),
|
||||
}
|
||||
}
|
||||
|
||||
fn timed_out_transfer_error(err: &RpcError) -> RpcError {
|
||||
let data = err.data.as_ref();
|
||||
RpcError {
|
||||
code: ErrorCode::Timeout,
|
||||
message: "file transfer timed out after dispatch".into(),
|
||||
data: Some(serde_json::json!({
|
||||
"reason": "transfer_timeout",
|
||||
"effect_state": data
|
||||
.and_then(|value| value.get("effect_state"))
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("unknown"),
|
||||
"phase": data
|
||||
.and_then(|value| value.get("phase"))
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("cleanup"),
|
||||
"cleanup_state": data
|
||||
.and_then(|value| value.get("cleanup_state"))
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("complete"),
|
||||
})),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
enum WaitOutcome {
|
||||
Response(bsk_protocol::ResponseFrame),
|
||||
CancelledAfterResponse(bsk_protocol::ResponseFrame),
|
||||
TimedOutAfterResponse(bsk_protocol::ResponseFrame),
|
||||
CleanupTimeout,
|
||||
TimeoutCleanupFailed,
|
||||
WaiterClosed,
|
||||
Timeout,
|
||||
}
|
||||
@@ -648,6 +875,8 @@ async fn await_with_optional_cancel(
|
||||
cleanup_timeout: Duration,
|
||||
mut waiter: oneshot::Receiver<bsk_protocol::ResponseFrame>,
|
||||
cancel: Option<&super::abort::AbortToken>,
|
||||
on_abort: Option<&(dyn Fn() + Sync)>,
|
||||
on_deadline: Option<&(dyn Fn() -> bool + Sync)>,
|
||||
) -> WaitOutcome {
|
||||
match cancel {
|
||||
Some(token) => {
|
||||
@@ -659,6 +888,9 @@ async fn await_with_optional_cancel(
|
||||
// after compensation or the cleanup deadline expires.
|
||||
biased;
|
||||
_ = token.cancelled() => {
|
||||
if let Some(on_abort) = on_abort {
|
||||
on_abort();
|
||||
}
|
||||
match tokio::time::timeout(cleanup_timeout, &mut waiter).await {
|
||||
Ok(Ok(resp)) => WaitOutcome::CancelledAfterResponse(resp),
|
||||
Ok(Err(_)) => WaitOutcome::WaiterClosed,
|
||||
@@ -669,13 +901,42 @@ async fn await_with_optional_cancel(
|
||||
Ok(resp) => WaitOutcome::Response(resp),
|
||||
Err(_) => WaitOutcome::WaiterClosed,
|
||||
},
|
||||
_ = &mut deadline => WaitOutcome::Timeout,
|
||||
_ = &mut deadline => {
|
||||
if let Some(send_cancel) = on_deadline {
|
||||
return if send_cancel() {
|
||||
match tokio::time::timeout(cleanup_timeout, &mut waiter).await {
|
||||
Ok(Ok(resp)) => WaitOutcome::TimedOutAfterResponse(resp),
|
||||
Ok(Err(_)) | Err(_) => WaitOutcome::TimeoutCleanupFailed,
|
||||
}
|
||||
} else {
|
||||
WaitOutcome::TimeoutCleanupFailed
|
||||
};
|
||||
}
|
||||
let Some(on_abort) = on_abort else {
|
||||
return WaitOutcome::Timeout;
|
||||
};
|
||||
on_abort();
|
||||
match tokio::time::timeout(cleanup_timeout, &mut waiter).await {
|
||||
Ok(Ok(resp)) => WaitOutcome::TimedOutAfterResponse(resp),
|
||||
Ok(Err(_)) => WaitOutcome::WaiterClosed,
|
||||
Err(_) => WaitOutcome::CleanupTimeout,
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
None => match tokio::time::timeout(timeout, &mut waiter).await {
|
||||
Ok(Ok(resp)) => WaitOutcome::Response(resp),
|
||||
Ok(Err(_)) => WaitOutcome::WaiterClosed,
|
||||
Err(_) => WaitOutcome::Timeout,
|
||||
Err(_) => match on_deadline {
|
||||
Some(send_cancel) if send_cancel() => {
|
||||
match tokio::time::timeout(cleanup_timeout, &mut waiter).await {
|
||||
Ok(Ok(resp)) => WaitOutcome::TimedOutAfterResponse(resp),
|
||||
Ok(Err(_)) | Err(_) => WaitOutcome::TimeoutCleanupFailed,
|
||||
}
|
||||
}
|
||||
Some(_) => WaitOutcome::TimeoutCleanupFailed,
|
||||
None => WaitOutcome::Timeout,
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -759,6 +1020,8 @@ mod await_with_optional_cancel_tests {
|
||||
Duration::from_secs(1),
|
||||
rx,
|
||||
Some(&token),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
@@ -781,6 +1044,8 @@ mod await_with_optional_cancel_tests {
|
||||
Duration::from_secs(1),
|
||||
rx,
|
||||
Some(&token),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
match outcome {
|
||||
@@ -795,9 +1060,15 @@ mod await_with_optional_cancel_tests {
|
||||
async fn no_cancel_token_still_returns_response() {
|
||||
let (tx, rx) = oneshot::channel();
|
||||
tx.send(dummy_response()).unwrap();
|
||||
let outcome =
|
||||
await_with_optional_cancel(Duration::from_secs(10), Duration::from_secs(1), rx, None)
|
||||
.await;
|
||||
let outcome = await_with_optional_cancel(
|
||||
Duration::from_secs(10),
|
||||
Duration::from_secs(1),
|
||||
rx,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
assert!(matches!(outcome, WaitOutcome::Response(_)));
|
||||
}
|
||||
|
||||
@@ -812,6 +1083,8 @@ mod await_with_optional_cancel_tests {
|
||||
Duration::from_secs(1),
|
||||
rx,
|
||||
Some(&token),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
});
|
||||
@@ -835,10 +1108,75 @@ mod await_with_optional_cancel_tests {
|
||||
Duration::from_millis(10),
|
||||
rx,
|
||||
Some(&token),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
assert!(matches!(outcome, WaitOutcome::CleanupTimeout));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn lifecycle_timeout_forwards_abort_and_waits_for_cleanup_response() {
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
||||
let token = AbortToken::new();
|
||||
let (tx, rx) = oneshot::channel();
|
||||
let abort_forwarded = AtomicBool::new(false);
|
||||
let forward_abort = || abort_forwarded.store(true, Ordering::SeqCst);
|
||||
tokio::spawn(async move {
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
tx.send(dummy_response()).unwrap();
|
||||
});
|
||||
|
||||
let outcome = await_with_optional_cancel(
|
||||
Duration::from_millis(5),
|
||||
Duration::from_millis(100),
|
||||
rx,
|
||||
Some(&token),
|
||||
Some(&forward_abort),
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(abort_forwarded.load(Ordering::SeqCst));
|
||||
assert!(matches!(outcome, WaitOutcome::TimedOutAfterResponse(_)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn transfer_deadline_sends_cancel_and_waits_for_cleanup_response() {
|
||||
let (tx, rx) = oneshot::channel();
|
||||
tokio::spawn(async move {
|
||||
tokio::time::sleep(Duration::from_millis(5)).await;
|
||||
tx.send(dummy_response()).unwrap();
|
||||
});
|
||||
let send_cancel = || true;
|
||||
let outcome = await_with_optional_cancel(
|
||||
Duration::from_millis(1),
|
||||
Duration::from_secs(1),
|
||||
rx,
|
||||
None,
|
||||
None,
|
||||
Some(&send_cancel),
|
||||
)
|
||||
.await;
|
||||
assert!(matches!(outcome, WaitOutcome::TimedOutAfterResponse(_)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn transfer_deadline_is_unknown_when_cancel_cannot_be_sent() {
|
||||
let (_tx, rx) = oneshot::channel();
|
||||
let send_cancel = || false;
|
||||
let outcome = await_with_optional_cancel(
|
||||
Duration::from_millis(1),
|
||||
Duration::from_secs(1),
|
||||
rx,
|
||||
None,
|
||||
None,
|
||||
Some(&send_cancel),
|
||||
)
|
||||
.await;
|
||||
assert!(matches!(outcome, WaitOutcome::TimeoutCleanupFailed));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -901,6 +1239,8 @@ mod tool_job_session_id_tests {
|
||||
timeout: Duration::from_secs(1),
|
||||
respond: tx,
|
||||
inflight: None,
|
||||
lifecycle_cancel: None,
|
||||
cancel_cleanup_timeout: CANCEL_CLEANUP_TIMEOUT,
|
||||
session_id: SessionId("sess-A".into()),
|
||||
};
|
||||
assert_eq!(job.session_id.0, "sess-A");
|
||||
|
||||
@@ -12,13 +12,14 @@ use bsk_protocol::system::{BrowserStatusEntry, SessionStatusEntry};
|
||||
use bsk_protocol::tools::{
|
||||
SessionStartParams, SessionStartResult, SessionStopParams, SessionStopResult,
|
||||
};
|
||||
use bsk_protocol::{Frame, RequestFrame, ResponseBody, RpcError, RpcId};
|
||||
use bsk_protocol::{ErrorCode, Frame, Method, RequestFrame, ResponseBody, RpcError, RpcId};
|
||||
use rand::Rng;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use tokio::time::{Duration, timeout};
|
||||
|
||||
use super::abort::AbortToken;
|
||||
use super::browsers::{BrowserClient, BrowserId, BrowserRegistry, SelectError};
|
||||
use super::queue::{DispatchError, ToolQueueRegistry};
|
||||
use super::queue::{CANCEL_CLEANUP_TIMEOUT, DispatchError, ToolQueueRegistry};
|
||||
use super::session_interrupt::SessionInterruptRegistry;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
|
||||
@@ -339,6 +340,14 @@ pub enum StartSessionError {
|
||||
IdExhausted,
|
||||
#[error("session creation timed out waiting for extension")]
|
||||
Timeout,
|
||||
#[error("session creation was cancelled")]
|
||||
Cancelled,
|
||||
#[error("session creation cancellation cleanup failed for {session_id}: {message}")]
|
||||
CleanupFailed {
|
||||
session_id: SessionId,
|
||||
agent_window_id: Option<i64>,
|
||||
message: String,
|
||||
},
|
||||
#[error("extension rejected tool.session_start: {0:?}")]
|
||||
ExtensionError(RpcError),
|
||||
#[error("transport closed while waiting for extension response")]
|
||||
@@ -354,6 +363,8 @@ impl StartSessionError {
|
||||
StartSessionError::AmbiguousBrowserLabel { .. } => "invalid_params",
|
||||
StartSessionError::IdExhausted => "protocol_error",
|
||||
StartSessionError::Timeout => "timeout",
|
||||
StartSessionError::Cancelled => "cancelled",
|
||||
StartSessionError::CleanupFailed { .. } => "protocol_error",
|
||||
StartSessionError::TransportClosed => "protocol_error",
|
||||
StartSessionError::ExtensionError(err) => match err.code {
|
||||
bsk_protocol::ErrorCode::Timeout => "timeout",
|
||||
@@ -407,6 +418,7 @@ pub struct AgentWindowOptions {
|
||||
/// On success the matching per-session dispatch queue (M6.5,
|
||||
/// design §5) is also spawned so that subsequent `tool.*` RPCs serialise
|
||||
/// against this session's Agent Window / ref-store.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub async fn start_session(
|
||||
registry: &Arc<BrowserRegistry>,
|
||||
sessions: &Arc<SessionRegistry>,
|
||||
@@ -415,24 +427,32 @@ pub async fn start_session(
|
||||
window: AgentWindowOptions,
|
||||
connect_wait: Duration,
|
||||
timeout_dur: Duration,
|
||||
cancel: Option<AbortToken>,
|
||||
) -> Result<Session, StartSessionError> {
|
||||
let client: Arc<BrowserClient> = registry
|
||||
.select_with_connect_wait(requested, connect_wait)
|
||||
.await
|
||||
.map_err(|e| match e {
|
||||
SelectError::NoBrowserConnected => StartSessionError::NoBrowserConnected,
|
||||
SelectError::MultipleBrowsersOnline => StartSessionError::MultipleBrowsersOnline {
|
||||
browsers: snapshot_status_entries(registry, sessions),
|
||||
},
|
||||
SelectError::NotFound => StartSessionError::BrowserNotFound,
|
||||
SelectError::AmbiguousLabel {
|
||||
label,
|
||||
instance_ids,
|
||||
} => StartSessionError::AmbiguousBrowserLabel {
|
||||
label,
|
||||
instance_ids,
|
||||
},
|
||||
})?;
|
||||
let selection = registry.select_with_connect_wait(requested, connect_wait);
|
||||
tokio::pin!(selection);
|
||||
let selected = match cancel.as_ref() {
|
||||
Some(token) => tokio::select! {
|
||||
biased;
|
||||
_ = token.cancelled() => return Err(StartSessionError::Cancelled),
|
||||
selected = &mut selection => selected,
|
||||
},
|
||||
None => selection.await,
|
||||
};
|
||||
let client: Arc<BrowserClient> = selected.map_err(|e| match e {
|
||||
SelectError::NoBrowserConnected => StartSessionError::NoBrowserConnected,
|
||||
SelectError::MultipleBrowsersOnline => StartSessionError::MultipleBrowsersOnline {
|
||||
browsers: snapshot_status_entries(registry, sessions),
|
||||
},
|
||||
SelectError::NotFound => StartSessionError::BrowserNotFound,
|
||||
SelectError::AmbiguousLabel {
|
||||
label,
|
||||
instance_ids,
|
||||
} => StartSessionError::AmbiguousBrowserLabel {
|
||||
label,
|
||||
instance_ids,
|
||||
},
|
||||
})?;
|
||||
let session_id = sessions
|
||||
.reserve_id(client.id.clone(), SESSION_ID_MAX_RESERVE_ATTEMPTS, now_ms)
|
||||
.ok_or(StartSessionError::IdExhausted)?;
|
||||
@@ -453,22 +473,71 @@ pub async fn start_session(
|
||||
let mut pending = client.pending.lock().unwrap();
|
||||
pending.register(rpc_id.clone())
|
||||
};
|
||||
if cancel.as_ref().is_some_and(AbortToken::is_cancelled) {
|
||||
sessions.cancel_reservation(&session_id);
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
return Err(StartSessionError::Cancelled);
|
||||
}
|
||||
if client.sink.send(Frame::Request(request)).is_err() {
|
||||
sessions.cancel_reservation(&session_id);
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
return Err(StartSessionError::TransportClosed);
|
||||
}
|
||||
let response = match timeout(timeout_dur, waiter).await {
|
||||
Ok(Ok(resp)) => resp,
|
||||
Ok(Err(_)) => {
|
||||
let mut waiter = waiter;
|
||||
enum StartWaitOutcome {
|
||||
Response(bsk_protocol::ResponseFrame),
|
||||
WaiterClosed,
|
||||
Cancelled,
|
||||
Timeout,
|
||||
}
|
||||
let wait_outcome = match cancel.as_ref() {
|
||||
Some(token) => {
|
||||
let deadline = tokio::time::sleep(timeout_dur);
|
||||
tokio::pin!(deadline);
|
||||
tokio::select! {
|
||||
biased;
|
||||
_ = token.cancelled() => StartWaitOutcome::Cancelled,
|
||||
response = &mut waiter => match response {
|
||||
Ok(response) => StartWaitOutcome::Response(response),
|
||||
Err(_) => StartWaitOutcome::WaiterClosed,
|
||||
},
|
||||
_ = &mut deadline => StartWaitOutcome::Timeout,
|
||||
}
|
||||
}
|
||||
None => match timeout(timeout_dur, &mut waiter).await {
|
||||
Ok(Ok(response)) => StartWaitOutcome::Response(response),
|
||||
Ok(Err(_)) => StartWaitOutcome::WaiterClosed,
|
||||
Err(_) => StartWaitOutcome::Timeout,
|
||||
},
|
||||
};
|
||||
let response = match wait_outcome {
|
||||
StartWaitOutcome::Response(response) => response,
|
||||
StartWaitOutcome::WaiterClosed => {
|
||||
sessions.cancel_reservation(&session_id);
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
return Err(StartSessionError::TransportClosed);
|
||||
}
|
||||
Err(_) => {
|
||||
sessions.cancel_reservation(&session_id);
|
||||
client.pending.lock().unwrap().cancel(&rpc_id);
|
||||
return Err(StartSessionError::Timeout);
|
||||
StartWaitOutcome::Cancelled => {
|
||||
return Err(finish_aborted_start(
|
||||
&client,
|
||||
sessions,
|
||||
&session_id,
|
||||
&rpc_id,
|
||||
waiter,
|
||||
StartAbortReason::Cancelled,
|
||||
)
|
||||
.await);
|
||||
}
|
||||
StartWaitOutcome::Timeout => {
|
||||
return Err(finish_aborted_start(
|
||||
&client,
|
||||
sessions,
|
||||
&session_id,
|
||||
&rpc_id,
|
||||
waiter,
|
||||
StartAbortReason::Timeout,
|
||||
)
|
||||
.await);
|
||||
}
|
||||
};
|
||||
let agent_window_id = match response.body {
|
||||
@@ -504,6 +573,124 @@ pub async fn start_session(
|
||||
Ok(session)
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum StartAbortReason {
|
||||
Cancelled,
|
||||
Timeout,
|
||||
}
|
||||
|
||||
impl StartAbortReason {
|
||||
fn error(self) -> StartSessionError {
|
||||
match self {
|
||||
Self::Cancelled => StartSessionError::Cancelled,
|
||||
Self::Timeout => StartSessionError::Timeout,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn finish_aborted_start(
|
||||
client: &Arc<BrowserClient>,
|
||||
sessions: &Arc<SessionRegistry>,
|
||||
session_id: &SessionId,
|
||||
rpc_id: &RpcId,
|
||||
mut waiter: tokio::sync::oneshot::Receiver<bsk_protocol::ResponseFrame>,
|
||||
reason: StartAbortReason,
|
||||
) -> StartSessionError {
|
||||
sessions.cancel_reservation(session_id);
|
||||
let cancel = Frame::Request(RequestFrame {
|
||||
id: format!("cancel-{rpc_id}"),
|
||||
method: Method::Cancel,
|
||||
params: Some(serde_json::json!({ "rpc_id": rpc_id })),
|
||||
});
|
||||
if client.sink.send(cancel).is_err() {
|
||||
client.pending.lock().unwrap().cancel(rpc_id);
|
||||
return StartSessionError::TransportClosed;
|
||||
}
|
||||
|
||||
match timeout(CANCEL_CLEANUP_TIMEOUT, &mut waiter).await {
|
||||
Ok(Ok(response)) => match response.body {
|
||||
ResponseBody::Err(err)
|
||||
if matches!(err.code, ErrorCode::Cancelled | ErrorCode::UserAborted) =>
|
||||
{
|
||||
reason.error()
|
||||
}
|
||||
ResponseBody::Err(err) => StartSessionError::ExtensionError(err),
|
||||
ResponseBody::Ok(value) => {
|
||||
let agent_window_id = serde_json::from_value::<SessionStartResult>(value)
|
||||
.ok()
|
||||
.and_then(|result| result.agent_window_id);
|
||||
match rollback_extension_session(client, session_id).await {
|
||||
Ok(()) => reason.error(),
|
||||
Err(message) => StartSessionError::CleanupFailed {
|
||||
session_id: session_id.clone(),
|
||||
agent_window_id,
|
||||
message,
|
||||
},
|
||||
}
|
||||
}
|
||||
},
|
||||
Ok(Err(_)) => {
|
||||
client.pending.lock().unwrap().cancel(rpc_id);
|
||||
StartSessionError::TransportClosed
|
||||
}
|
||||
Err(_) => {
|
||||
// The cancel frame is already queued behind session_start. Even
|
||||
// if chrome.windows.create is still blocked, the extension will
|
||||
// observe the aborted signal and compensate before replying.
|
||||
client.pending.lock().unwrap().cancel(rpc_id);
|
||||
reason.error()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn rollback_extension_session(
|
||||
client: &Arc<BrowserClient>,
|
||||
session_id: &SessionId,
|
||||
) -> Result<(), String> {
|
||||
let rollback_id = next_rpc_id("sess-start-rollback");
|
||||
let waiter = {
|
||||
let mut pending = client.pending.lock().unwrap();
|
||||
pending.register(rollback_id.clone())
|
||||
};
|
||||
let request = RequestFrame {
|
||||
id: rollback_id.clone(),
|
||||
method: Method::ToolSessionStop,
|
||||
params: Some(
|
||||
serde_json::to_value(SessionStopParams {
|
||||
session_id: session_id.0.clone(),
|
||||
})
|
||||
.expect("serialize session_start rollback params"),
|
||||
),
|
||||
};
|
||||
if client.sink.send(Frame::Request(request)).is_err() {
|
||||
client.pending.lock().unwrap().cancel(&rollback_id);
|
||||
return Err("browser disconnected before rollback was queued".into());
|
||||
}
|
||||
let response = match timeout(CANCEL_CLEANUP_TIMEOUT, waiter).await {
|
||||
Ok(Ok(response)) => response,
|
||||
Ok(Err(_)) => return Err("browser disconnected during rollback".into()),
|
||||
Err(_) => {
|
||||
client.pending.lock().unwrap().cancel(&rollback_id);
|
||||
return Err(format!(
|
||||
"extension did not finish rollback within {CANCEL_CLEANUP_TIMEOUT:?}"
|
||||
));
|
||||
}
|
||||
};
|
||||
match response.body {
|
||||
ResponseBody::Ok(value) => {
|
||||
let result: SessionStopResult = serde_json::from_value(value)
|
||||
.map_err(|err| format!("invalid rollback response: {err}"))?;
|
||||
if result.return_failures.is_empty() {
|
||||
Ok(())
|
||||
} else {
|
||||
Err("rollback reported borrowed-tab return failures".into())
|
||||
}
|
||||
}
|
||||
ResponseBody::Err(err) if err.code == ErrorCode::NotFound => Ok(()),
|
||||
ResponseBody::Err(err) => Err(format!("rollback failed: {} ({:?})", err.message, err.code)),
|
||||
}
|
||||
}
|
||||
|
||||
/// Tear down a single session id (round-trips `tool.session_stop` to the
|
||||
/// owning extension and unregisters on success). Also tears down the
|
||||
/// per-session dispatch queue so the worker task exits.
|
||||
@@ -514,6 +701,7 @@ pub async fn stop_session(
|
||||
interrupts: &Arc<SessionInterruptRegistry>,
|
||||
session_id: &SessionId,
|
||||
timeout_dur: Duration,
|
||||
cancel: Option<AbortToken>,
|
||||
) -> Result<SessionStopResult, StopSessionError> {
|
||||
let session = sessions.get(session_id).ok_or(StopSessionError::NotFound)?;
|
||||
if registry.get(&session.browser_id).is_none() {
|
||||
@@ -524,12 +712,17 @@ pub async fn stop_session(
|
||||
session_id: session_id.0.clone(),
|
||||
})
|
||||
.unwrap();
|
||||
// Internal callers (idle reaping and shutdown) do not have a CLI-owned
|
||||
// token, but still need timeout-driven WS cancellation so a late window
|
||||
// close cannot leave daemon state pointing at a vanished session.
|
||||
let lifecycle_cancel = Some(cancel.unwrap_or_default());
|
||||
match queues
|
||||
.dispatch_after_closing(
|
||||
session_id,
|
||||
bsk_protocol::Method::ToolSessionStop,
|
||||
params,
|
||||
timeout_dur,
|
||||
lifecycle_cancel,
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -567,6 +760,10 @@ pub async fn stop_session(
|
||||
drop_session_local(queues, interrupts, session_id);
|
||||
return Ok(SessionStopResult::default());
|
||||
}
|
||||
if err.code == bsk_protocol::ErrorCode::Timeout {
|
||||
queues.reopen(session_id);
|
||||
return Err(StopSessionError::Timeout);
|
||||
}
|
||||
queues.reopen(session_id);
|
||||
Err(StopSessionError::ExtensionError(err))
|
||||
}
|
||||
|
||||
@@ -15,7 +15,6 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr};
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
@@ -27,7 +26,7 @@ use crate::daemon::{
|
||||
browsers::{BROWSER_LIVENESS_TICK, BROWSER_LIVENESS_TIMEOUT, EXTENSION_CONNECT_WAIT},
|
||||
info as daemon_info, ipc, lockfile, paths,
|
||||
sessions::{StopSessionError, forget_session, stop_session},
|
||||
state::DaemonState,
|
||||
state::{DaemonState, PROTOCOL_VERSION},
|
||||
ws,
|
||||
};
|
||||
|
||||
@@ -226,11 +225,15 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> {
|
||||
.with_context(|| format!("bind IPC endpoint {}", sock_path.display()))?;
|
||||
|
||||
let state = Arc::new(DaemonState::new(cfg.clone()));
|
||||
state
|
||||
.transfers
|
||||
.initialize()
|
||||
.context("initialize transfer staging")?;
|
||||
let session_idle_task = spawn_session_idle_reaper(Arc::clone(&state));
|
||||
let browser_liveness_task = spawn_browser_liveness_reaper(Arc::clone(&state));
|
||||
// Fired by the update check task after a successful auto-update:
|
||||
// the replacement daemon has already been spawned, so this
|
||||
// process should shut down and let it take over.
|
||||
// the replacement daemon (or Windows update helper) is ready, so
|
||||
// this process should shut down and let it take over.
|
||||
let restart_notify = Arc::new(tokio::sync::Notify::new());
|
||||
let update_check_task =
|
||||
spawn_update_check_task(Arc::clone(&state), Arc::clone(&restart_notify));
|
||||
@@ -291,15 +294,14 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> {
|
||||
});
|
||||
|
||||
let started_at = Instant::now();
|
||||
let activity: Arc<Mutex<Instant>> = Arc::new(Mutex::new(started_at));
|
||||
let active_ipc_connections = Arc::new(AtomicUsize::new(0));
|
||||
let activity = Arc::new(Mutex::new(IpcActivityState::new(started_at)));
|
||||
let (ipc_shutdown_tx, ipc_shutdown_rx) = tokio::sync::oneshot::channel::<()>();
|
||||
let status = ipc::DaemonStatus {
|
||||
started_at,
|
||||
ws_port,
|
||||
sock_path: sock_path.clone(),
|
||||
daemon_version: env!("CARGO_PKG_VERSION"),
|
||||
protocol_version: "1.0",
|
||||
protocol_version: PROTOCOL_VERSION,
|
||||
};
|
||||
let handler = ipc::full_handler(status, Arc::clone(&state));
|
||||
|
||||
@@ -314,18 +316,14 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> {
|
||||
let handler = handler.clone();
|
||||
let ipc_open = {
|
||||
let activity = activity.clone();
|
||||
let active = active_ipc_connections.clone();
|
||||
move || {
|
||||
active.fetch_add(1, Ordering::SeqCst);
|
||||
record_activity(&activity);
|
||||
record_ipc_open(&activity);
|
||||
}
|
||||
};
|
||||
let ipc_close = {
|
||||
let activity = activity.clone();
|
||||
let active = active_ipc_connections.clone();
|
||||
move || {
|
||||
active.fetch_sub(1, Ordering::SeqCst);
|
||||
record_activity(&activity);
|
||||
record_ipc_close(&activity);
|
||||
}
|
||||
};
|
||||
tokio::spawn(ipc::serve(
|
||||
@@ -344,7 +342,6 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> {
|
||||
let idle_task = {
|
||||
let activity = activity.clone();
|
||||
let state = Arc::clone(&state);
|
||||
let active_ipc_connections = active_ipc_connections.clone();
|
||||
let daemon_idle = cfg.daemon_idle;
|
||||
tokio::spawn(async move {
|
||||
let tick = (daemon_idle / 4).max(Duration::from_millis(250));
|
||||
@@ -352,22 +349,19 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> {
|
||||
ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay);
|
||||
loop {
|
||||
ticker.tick().await;
|
||||
let last = match activity.lock() {
|
||||
Ok(a) => *a,
|
||||
Err(p) => *p.into_inner(),
|
||||
};
|
||||
if last.elapsed() < daemon_idle {
|
||||
let ipc_is_idle =
|
||||
lock_activity(&activity).should_exit_for_idle(Instant::now(), daemon_idle);
|
||||
if !ipc_is_idle {
|
||||
continue;
|
||||
}
|
||||
// Bridge from M2/M3 (which only knew about connection
|
||||
// counts) to M4/M5 (browser + session registries):
|
||||
// hold the daemon alive while any IPC client is
|
||||
// connected, any browser is paired, or any session
|
||||
// is live (design §3.2).
|
||||
if active_ipc_connections.load(Ordering::SeqCst) > 0
|
||||
|| !state.browsers.is_empty()
|
||||
|| !state.sessions.is_empty()
|
||||
{
|
||||
// IPC liveness (open-connection count + idle window) is already
|
||||
// enforced above by `should_exit_for_idle`, which reads
|
||||
// `IpcActivityState` under one lock and returns true only when
|
||||
// there are zero connections AND the idle interval has elapsed.
|
||||
// Here we additionally hold the daemon alive while any browser
|
||||
// is paired or any session is live (design §3.2, M4/M5
|
||||
// registries).
|
||||
if !state.browsers.is_empty() || !state.sessions.is_empty() {
|
||||
continue;
|
||||
}
|
||||
info!(
|
||||
@@ -458,6 +452,7 @@ pub(crate) fn spawn_browser_liveness_reaper(
|
||||
for s in state.sessions.purge_browser(&client.id) {
|
||||
state.tool_queues.remove(&s.id);
|
||||
state.session_interrupts.drop_session(&s.id);
|
||||
state.transfers.release_session(&s.id.0);
|
||||
debug!(session = %s.id, "purged session on browser liveness timeout");
|
||||
}
|
||||
}
|
||||
@@ -491,10 +486,14 @@ pub(crate) fn spawn_session_idle_reaper(state: Arc<DaemonState>) -> tokio::task:
|
||||
&state.session_interrupts,
|
||||
&session_id,
|
||||
Duration::from_secs(10),
|
||||
None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(_) => info!(session = %session_id, "idle session stopped"),
|
||||
Ok(_) => {
|
||||
state.transfers.release_session(&session_id.0);
|
||||
info!(session = %session_id, "idle session stopped");
|
||||
}
|
||||
Err(StopSessionError::SessionBusy | StopSessionError::Stopping) => {
|
||||
debug!(session = %session_id, "idle session still active; retrying later");
|
||||
}
|
||||
@@ -505,6 +504,7 @@ pub(crate) fn spawn_session_idle_reaper(state: Arc<DaemonState>) -> tokio::task:
|
||||
&state.session_interrupts,
|
||||
&session_id,
|
||||
);
|
||||
state.transfers.release_session(&session_id.0);
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(session = %session_id, error = %err, "failed to stop idle session");
|
||||
@@ -539,8 +539,8 @@ pub(crate) fn spawn_session_idle_reaper(state: Arc<DaemonState>) -> tokio::task:
|
||||
/// task spawns the replacement daemon (see
|
||||
/// [`DAEMON_REPLACEMENT_WAIT_ENV`]) and fires `restart` so this process
|
||||
/// shuts down and the new version takes over; on Windows the
|
||||
/// replacement can only be staged, so it just logs that a restart is
|
||||
/// needed.
|
||||
/// helper waits for this process to exit, replaces the binary, and starts
|
||||
/// the new daemon with the same configuration.
|
||||
pub(crate) fn spawn_update_check_task(
|
||||
state: Arc<DaemonState>,
|
||||
restart: Arc<tokio::sync::Notify>,
|
||||
@@ -605,7 +605,11 @@ pub(crate) fn spawn_update_check_task(
|
||||
|candidate| {
|
||||
let target =
|
||||
exe_path.as_deref().context("current executable unknown")?;
|
||||
update::self_install_candidate(candidate, target)
|
||||
update::self_install_candidate(
|
||||
candidate,
|
||||
target,
|
||||
&restart_start_args(&state.config),
|
||||
)
|
||||
},
|
||||
)
|
||||
})
|
||||
@@ -635,10 +639,11 @@ pub(crate) fn spawn_update_check_task(
|
||||
sessions,
|
||||
"auto-update postponed: agent session(s) active; will retry on the next tick"
|
||||
),
|
||||
update::AutoUpdateOutcome::Staged { latest } => warn!(
|
||||
%latest,
|
||||
"auto-update staged the new binary but cannot replace the running daemon in place; restart the daemon (or terminal) to finish the upgrade"
|
||||
),
|
||||
update::AutoUpdateOutcome::Staged { latest } => {
|
||||
info!(%latest, "update helper ready; exiting so it can replace and restart the daemon");
|
||||
restart.notify_one();
|
||||
return;
|
||||
}
|
||||
update::AutoUpdateOutcome::Replaced { latest } => {
|
||||
info!(
|
||||
current = env!("CARGO_PKG_VERSION"),
|
||||
@@ -681,10 +686,55 @@ fn restart_start_args(cfg: &DaemonConfig) -> StartArgs {
|
||||
}
|
||||
}
|
||||
|
||||
fn record_activity(activity: &Arc<Mutex<Instant>>) {
|
||||
if let Ok(mut a) = activity.lock() {
|
||||
*a = Instant::now();
|
||||
#[derive(Debug)]
|
||||
struct IpcActivityState {
|
||||
last_activity: Instant,
|
||||
active_connections: usize,
|
||||
}
|
||||
|
||||
impl IpcActivityState {
|
||||
fn new(now: Instant) -> Self {
|
||||
Self {
|
||||
last_activity: now,
|
||||
active_connections: 0,
|
||||
}
|
||||
}
|
||||
|
||||
fn touch(&mut self, now: Instant) {
|
||||
self.last_activity = now;
|
||||
}
|
||||
|
||||
fn opened(&mut self, now: Instant) {
|
||||
self.active_connections = self.active_connections.saturating_add(1);
|
||||
self.last_activity = now;
|
||||
}
|
||||
|
||||
fn closed(&mut self, now: Instant) {
|
||||
self.active_connections = self.active_connections.saturating_sub(1);
|
||||
self.last_activity = now;
|
||||
}
|
||||
|
||||
fn should_exit_for_idle(&self, now: Instant, idle: Duration) -> bool {
|
||||
self.active_connections == 0 && now.saturating_duration_since(self.last_activity) >= idle
|
||||
}
|
||||
}
|
||||
|
||||
fn lock_activity(
|
||||
activity: &Arc<Mutex<IpcActivityState>>,
|
||||
) -> std::sync::MutexGuard<'_, IpcActivityState> {
|
||||
activity.lock().unwrap_or_else(|poison| poison.into_inner())
|
||||
}
|
||||
|
||||
fn record_activity(activity: &Arc<Mutex<IpcActivityState>>) {
|
||||
lock_activity(activity).touch(Instant::now());
|
||||
}
|
||||
|
||||
fn record_ipc_open(activity: &Arc<Mutex<IpcActivityState>>) {
|
||||
lock_activity(activity).opened(Instant::now());
|
||||
}
|
||||
|
||||
fn record_ipc_close(activity: &Arc<Mutex<IpcActivityState>>) {
|
||||
lock_activity(activity).closed(Instant::now());
|
||||
}
|
||||
|
||||
/// Initialise tracing for the daemon: write a daily-rotated file in
|
||||
@@ -1067,4 +1117,37 @@ mod tests {
|
||||
assert_eq!(args.session_idle, Some(Duration::from_secs(11)));
|
||||
assert_eq!(args.daemon_idle, Some(Duration::from_secs(22)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ipc_close_restarts_the_idle_window_atomically() {
|
||||
let started = Instant::now();
|
||||
let idle = Duration::from_secs(10);
|
||||
let mut activity = IpcActivityState::new(started);
|
||||
|
||||
activity.opened(started);
|
||||
let closed_at = started + Duration::from_secs(30);
|
||||
assert!(
|
||||
!activity.should_exit_for_idle(closed_at, idle),
|
||||
"an open IPC connection must hold the daemon alive"
|
||||
);
|
||||
|
||||
activity.closed(closed_at);
|
||||
assert_eq!(activity.active_connections, 0);
|
||||
assert_eq!(activity.last_activity, closed_at);
|
||||
assert!(!activity.should_exit_for_idle(closed_at, idle));
|
||||
assert!(!activity.should_exit_for_idle(closed_at + idle - Duration::from_millis(1), idle));
|
||||
assert!(activity.should_exit_for_idle(closed_at + idle, idle));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ipc_activity_keeps_the_daemon_alive_until_a_full_idle_interval_passes() {
|
||||
let started = Instant::now();
|
||||
let idle = Duration::from_secs(10);
|
||||
let mut activity = IpcActivityState::new(started);
|
||||
let request_at = started + Duration::from_secs(20);
|
||||
|
||||
activity.touch(request_at);
|
||||
assert!(!activity.should_exit_for_idle(request_at + Duration::from_secs(9), idle));
|
||||
assert!(activity.should_exit_for_idle(request_at + idle, idle));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ use tokio::task::JoinHandle;
|
||||
|
||||
use super::abort::AbortRegistry;
|
||||
use super::browsers::BrowserRegistry;
|
||||
use super::file_transfer::TransferRegistry;
|
||||
use super::inflight::ToolInflightRegistry;
|
||||
use super::ipc::IpcHandle;
|
||||
use super::queue::ToolQueueRegistry;
|
||||
@@ -16,7 +17,7 @@ use super::start::DaemonConfig;
|
||||
use super::ws::WsHandle;
|
||||
|
||||
pub const DAEMON_VERSION: &str = env!("CARGO_PKG_VERSION");
|
||||
pub const PROTOCOL_VERSION: &str = "1.0";
|
||||
pub const PROTOCOL_VERSION: &str = "1.1";
|
||||
/// Lowest **protocol** version peers must speak (e.g. `"1.0"`).
|
||||
pub const MIN_COMPATIBLE_PROTOCOL: &str = "1.0";
|
||||
/// Legacy app-semver floor used only when `HandshakeResult.min_compatible_peer`
|
||||
@@ -34,8 +35,8 @@ pub struct DaemonState {
|
||||
/// `stop_session` / browser disconnect.
|
||||
pub tool_queues: Arc<ToolQueueRegistry>,
|
||||
/// Per-rpc-id cancellation tokens for daemon-side long-runners
|
||||
/// (M9.3 — currently only `tool.wait_ms`). The CLI's `cancel
|
||||
/// { rpc_id }` consults this registry first.
|
||||
/// (`tool.wait_ms` plus `session.*` lifecycle calls). The CLI's
|
||||
/// `cancel { rpc_id }` consults this registry first.
|
||||
pub abort_registry: Arc<AbortRegistry>,
|
||||
/// Tracks `tool.*` RPCs that have been forwarded to an extension
|
||||
/// over WS but have not yet received a response. Indexed by the
|
||||
@@ -51,6 +52,9 @@ pub struct DaemonState {
|
||||
/// `SessionRegistry` because the signal is a transient runtime
|
||||
/// control state.
|
||||
pub session_interrupts: Arc<SessionInterruptRegistry>,
|
||||
/// Operation-scoped local file staging. The extension only sees paths
|
||||
/// minted here; agent-facing RPCs use opaque transfer ids.
|
||||
pub transfers: Arc<TransferRegistry>,
|
||||
}
|
||||
|
||||
impl DaemonState {
|
||||
@@ -64,6 +68,7 @@ impl DaemonState {
|
||||
));
|
||||
let abort_registry = Arc::new(AbortRegistry::new());
|
||||
let session_interrupts = Arc::new(SessionInterruptRegistry::new());
|
||||
let transfers = Arc::new(TransferRegistry::new().expect("initialise transfer staging"));
|
||||
Self {
|
||||
config,
|
||||
browsers,
|
||||
@@ -72,6 +77,7 @@ impl DaemonState {
|
||||
abort_registry,
|
||||
tool_inflight,
|
||||
session_interrupts,
|
||||
transfers,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -295,6 +295,7 @@ async fn drive_connection(
|
||||
for session in state.sessions.purge_browser(&browser_id) {
|
||||
state.tool_queues.remove(&session.id);
|
||||
state.session_interrupts.drop_session(&session.id);
|
||||
state.transfers.release_session(&session.id.0);
|
||||
debug!(session = %session.id, "purged stale session before browser reconnect");
|
||||
}
|
||||
let (tx, mut rx) = mpsc::unbounded_channel::<Frame>();
|
||||
@@ -407,6 +408,7 @@ async fn drive_connection(
|
||||
for s in state.sessions.purge_browser(&browser_id) {
|
||||
state.tool_queues.remove(&s.id);
|
||||
state.session_interrupts.drop_session(&s.id);
|
||||
state.transfers.release_session(&s.id.0);
|
||||
debug!(session = %s.id, "purged session on browser disconnect");
|
||||
}
|
||||
} else {
|
||||
@@ -503,6 +505,7 @@ fn handle_session_window_closed(
|
||||
&state.session_interrupts,
|
||||
&session_id,
|
||||
) {
|
||||
state.transfers.release_session(&session_id.0);
|
||||
info!(session = %session_id, "session removed: user closed Agent Window");
|
||||
} else {
|
||||
debug!(session = %session_id, "session.window_closed for unknown session id");
|
||||
|
||||
@@ -99,6 +99,8 @@ fn dispatch(cli: Cli, format: Format) -> Result<(), CliError> {
|
||||
Command::Fill(args) => cli::interaction::dispatch_fill(args, format),
|
||||
Command::Press(args) => cli::interaction::dispatch_press(args, format),
|
||||
Command::Select(args) => cli::interaction::dispatch_select(args, format),
|
||||
Command::Upload(args) => cli::upload::dispatch(args, format),
|
||||
Command::Download(args) => cli::download::dispatch(args, format),
|
||||
Command::Evaluate(args) => cli::evaluate::dispatch(args, format),
|
||||
Command::WaitForNavigation(args) => cli::waits::dispatch_wait_for_navigation(args, format),
|
||||
Command::WaitMs(args) => cli::waits::dispatch_wait_ms(args, format),
|
||||
|
||||
Reference in New Issue
Block a user