mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-06 14:10:43 +00:00
fix: session-affinity hook defaults to not_supported and is Send+Sync
open_session silently returned Ok(None) instead of following the
blanket not_supported convention every other adapter default uses,
AdapterSession had lost its Sync bound, and the SessionAffinity
parameter the plan specified never shipped. Introduces
session_affinity.rs (SessionAffinity { session_id }), restores
AdapterSession: Send + Sync, and changes open_session's signature to
open_session(&SessionAffinity) -> Result<Box<dyn AdapterSession>,
AdapterError> with a default Err(not_supported("open_session")).
This commit is contained in:
parent
c1810539bf
commit
b5c595468d
7 changed files with 147 additions and 3 deletions
|
|
@ -82,10 +82,23 @@ pub trait SystemOps: Send + Sync {
|
|||
Err(AdapterError::not_supported("capture_signal_baseline"))
|
||||
}
|
||||
|
||||
/// Opens adapter-native session affinity for a host that outlives a
|
||||
/// single command (an FFI embedder, a future daemon) — the landing zone
|
||||
/// for a Windows COM-MTA apartment thread or a Linux D-Bus connection
|
||||
/// before those adapters exist. `affinity.session_id` lets the caller
|
||||
/// tie the native connection's lifetime to a CLI-level session (see
|
||||
/// [`crate::session::SessionManifest`]). The returned session may hold
|
||||
/// native connection state but must never hold a resolved element
|
||||
/// handle — commands keep resolving elements per call from a
|
||||
/// `RefEntry`, exactly as they do today. Nothing in the CLI/dispatch
|
||||
/// path calls this yet; the stateless request-per-command flow is
|
||||
/// unaffected until a persistent host opts in. Adapters with no native
|
||||
/// connection state to manage return `not_supported`.
|
||||
fn open_session(
|
||||
&self,
|
||||
) -> Result<Option<Box<dyn crate::adapter_session::AdapterSession>>, AdapterError> {
|
||||
Ok(None)
|
||||
_affinity: &crate::session_affinity::SessionAffinity,
|
||||
) -> Result<Box<dyn crate::adapter_session::AdapterSession>, AdapterError> {
|
||||
Err(AdapterError::not_supported("open_session"))
|
||||
}
|
||||
|
||||
fn close_app(&self, _id: &str, _force: bool) -> Result<(), AdapterError> {
|
||||
|
|
@ -188,3 +201,7 @@ pub trait SystemOps: Send + Sync {
|
|||
Err(AdapterError::not_supported("notification_action"))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "system_tests.rs"]
|
||||
mod tests;
|
||||
|
|
|
|||
21
crates/core/src/adapter/system_tests.rs
Normal file
21
crates/core/src/adapter/system_tests.rs
Normal file
|
|
@ -0,0 +1,21 @@
|
|||
use super::*;
|
||||
use crate::error::ErrorCode;
|
||||
use crate::session_affinity::SessionAffinity;
|
||||
|
||||
struct DefaultOnly;
|
||||
impl SystemOps for DefaultOnly {}
|
||||
|
||||
#[test]
|
||||
fn default_open_session_is_not_supported() {
|
||||
let result = DefaultOnly.open_session(&SessionAffinity::default());
|
||||
let Err(err) = result else {
|
||||
panic!("expected open_session default to return an error");
|
||||
};
|
||||
|
||||
assert_eq!(err.code, ErrorCode::PlatformNotSupported);
|
||||
assert!(
|
||||
err.message.contains("open_session"),
|
||||
"not_supported message should name the method, got: {}",
|
||||
err.message
|
||||
);
|
||||
}
|
||||
|
|
@ -1,5 +1,22 @@
|
|||
use crate::error::AdapterError;
|
||||
|
||||
pub trait AdapterSession: Send {
|
||||
/// A live handle to adapter-native connection affinity opened by
|
||||
/// [`crate::adapter::SystemOps::open_session`] for a host that outlives a
|
||||
/// single command (an FFI embedder, a future daemon) — the landing zone for
|
||||
/// state like a Windows COM-MTA apartment thread or a Linux D-Bus
|
||||
/// connection. It may hold that native connection state for as long as the
|
||||
/// session stays open, but it must never hold a resolved element handle:
|
||||
/// commands keep resolving elements per call from a `RefEntry` (the
|
||||
/// resolve-then-release RAII boundary), and a session must not become a
|
||||
/// second place stale identity can hide.
|
||||
///
|
||||
/// `Send + Sync` so a persistent host can hand the boxed session across
|
||||
/// threads, matching every other adapter capability trait
|
||||
/// (`ObservationOps`, `ActionOps`, `InputOps`, `SystemOps`).
|
||||
pub trait AdapterSession: Send + Sync {
|
||||
fn close(self: Box<Self>) -> Result<(), AdapterError>;
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "adapter_session_tests.rs"]
|
||||
mod tests;
|
||||
|
|
|
|||
36
crates/core/src/adapter_session_tests.rs
Normal file
36
crates/core/src/adapter_session_tests.rs
Normal file
|
|
@ -0,0 +1,36 @@
|
|||
use super::*;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
||||
struct FlagSession {
|
||||
closed: Arc<AtomicBool>,
|
||||
}
|
||||
|
||||
impl AdapterSession for FlagSession {
|
||||
fn close(self: Box<Self>) -> Result<(), AdapterError> {
|
||||
self.closed.store(true, Ordering::SeqCst);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn boxed_adapter_session_close_runs_through_dyn_dispatch() {
|
||||
let closed = Arc::new(AtomicBool::new(false));
|
||||
let session: Box<dyn AdapterSession> = Box::new(FlagSession {
|
||||
closed: closed.clone(),
|
||||
});
|
||||
|
||||
session.close().unwrap();
|
||||
|
||||
assert!(
|
||||
closed.load(Ordering::SeqCst),
|
||||
"close() must run through Box<dyn AdapterSession> dispatch, proving the trait is object-safe"
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_send_sync<T: Send + Sync>() {}
|
||||
|
||||
#[test]
|
||||
fn boxed_adapter_session_is_send_and_sync() {
|
||||
assert_send_sync::<Box<dyn AdapterSession>>();
|
||||
}
|
||||
|
|
@ -43,6 +43,7 @@ pub mod roles;
|
|||
pub mod screenshot_target;
|
||||
pub(crate) mod search_text;
|
||||
pub mod session;
|
||||
pub mod session_affinity;
|
||||
pub mod signals;
|
||||
pub mod snapshot;
|
||||
pub mod snapshot_ref;
|
||||
|
|
|
|||
12
crates/core/src/session_affinity.rs
Normal file
12
crates/core/src/session_affinity.rs
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
/// Which CLI-level session (see [`crate::session::SessionManifest`]) a
|
||||
/// long-lived adapter session should be affiliated with. Extends the
|
||||
/// manifest's `id` vocabulary into the adapter layer so a persistent host
|
||||
/// (an FFI embedder, a future daemon) can scope native connection affinity
|
||||
/// — a Windows COM-MTA apartment thread, a Linux D-Bus connection — to the
|
||||
/// same lifetime as the caller's session. `None` means no session is known;
|
||||
/// `open_session` implementations remain free to open an unaffiliated
|
||||
/// connection in that case.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct SessionAffinity {
|
||||
pub session_id: Option<String>,
|
||||
}
|
||||
|
|
@ -280,3 +280,43 @@ fn adapter_contract_wait_predicates_cover_live_state_paths() {
|
|||
assert_eq!(actionable["observed"]["actionable"], true);
|
||||
assert_eq!(value["observed"]["matched"], true);
|
||||
}
|
||||
|
||||
/// U16/KTD15: `open_session` is a contract-only hook — its doc says the CLI
|
||||
/// path stays stateless and nothing calls it yet. This scans the actual
|
||||
/// CLI/dispatch surface (not just today's known-empty call list) so a future
|
||||
/// PR that wires a call site without revisiting that contract fails loudly
|
||||
/// here instead of silently drifting from the plan.
|
||||
#[test]
|
||||
fn open_session_has_no_cli_dispatch_call_site() {
|
||||
let src_root = std::path::Path::new(env!("CARGO_MANIFEST_DIR"));
|
||||
let scanned_dirs = [
|
||||
src_root.join("dispatch"),
|
||||
src_root.join("batch"),
|
||||
src_root.join("cli"),
|
||||
src_root.join("cli_args"),
|
||||
src_root.join("command_policy"),
|
||||
src_root.join("../crates/core/src/commands"),
|
||||
];
|
||||
|
||||
for dir in scanned_dirs {
|
||||
for path in rust_files_in(&dir) {
|
||||
let source =
|
||||
std::fs::read_to_string(&path).expect("scanned source file should be readable");
|
||||
assert!(
|
||||
!source.contains(".open_session("),
|
||||
"{} calls open_session; U16/KTD15 keeps this hook uncalled by the CLI/\
|
||||
dispatch path until a persistent host (FFI/daemon) opts in",
|
||||
path.display()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn rust_files_in(dir: &std::path::Path) -> Vec<std::path::PathBuf> {
|
||||
std::fs::read_dir(dir)
|
||||
.unwrap_or_else(|e| panic!("{} should be readable: {e}", dir.display()))
|
||||
.filter_map(Result::ok)
|
||||
.map(|entry| entry.path())
|
||||
.filter(|path| path.extension().is_some_and(|ext| ext == "rs"))
|
||||
.collect()
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue