diff --git a/crates/core/src/private_file.rs b/crates/core/src/private_file.rs index 76b170e..248a182 100644 --- a/crates/core/src/private_file.rs +++ b/crates/core/src/private_file.rs @@ -20,9 +20,12 @@ pub(crate) fn open_private_lock(path: &Path, create: bool) -> std::io::Result std::io::Result { let mut options = OpenOptions::new(); - options.create(true).append(true); + options.read(true).create(true).append(true); configure_unix(&mut options, 0o600); let file = options.open(path)?; validate_private_regular(&file)?; @@ -299,3 +302,7 @@ pub(super) fn permission_denied(message: &'static str) -> std::io::Error { #[cfg(test)] #[path = "private_file_tests.rs"] mod tests; + +#[cfg(test)] +#[path = "private_file_lock_tests.rs"] +mod lock_tests; diff --git a/crates/core/src/private_file_lock_tests.rs b/crates/core/src/private_file_lock_tests.rs new file mode 100644 index 0000000..d5e5d69 --- /dev/null +++ b/crates/core/src/private_file_lock_tests.rs @@ -0,0 +1,37 @@ +use super::*; + +fn scratch_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "agent-desktop-{name}-{}-{}", + std::process::id(), + crate::refs::new_snapshot_id() + )); + crate::private_file_parent::ensure_private(&dir).expect("private scratch directory"); + dir +} + +#[test] +fn appended_private_files_are_lockable() { + let path = scratch_dir("append-lock").join("trace.jsonl"); + + let file = open_private_append(&path).expect("append handle"); + file.lock() + .expect("append handles must be lockable on every platform"); + file.unlock().expect("unlock"); + + drop(file); + let _ = std::fs::remove_file(&path); +} + +#[test] +fn append_handles_reopen_and_relock_after_close() { + let path = scratch_dir("append-relock").join("trace.jsonl"); + + for _ in 0..2 { + let file = open_private_append(&path).expect("append handle"); + file.lock().expect("lock"); + file.unlock().expect("unlock"); + } + + let _ = std::fs::remove_file(&path); +} diff --git a/crates/core/src/refs_lock.rs b/crates/core/src/refs_lock.rs index f5a90f9..d68bc56 100644 --- a/crates/core/src/refs_lock.rs +++ b/crates/core/src/refs_lock.rs @@ -32,11 +32,13 @@ mod tests { use super::*; fn lock_path(name: &str) -> std::path::PathBuf { - std::env::temp_dir().join(format!( - "agent-desktop-{name}-{}-{}.lock", - std::process::id(), - crate::refs::new_snapshot_id() - )) + std::env::temp_dir() + .join(format!( + "agent-desktop-{name}-{}-{}", + std::process::id(), + crate::refs::new_snapshot_id() + )) + .join("store.lock") } #[test] diff --git a/crates/ffi/src/adapter.rs b/crates/ffi/src/adapter.rs index 8f5c183..8c7aef8 100644 --- a/crates/ffi/src/adapter.rs +++ b/crates/ffi/src/adapter.rs @@ -81,9 +81,9 @@ pub(crate) use acquire_adapter; /// /// Every method delegates to the `PlatformAdapter` trait defaults, all of /// which return `not_supported()` errors. `permission_report()` returns -/// `Denied` via the trait default, so `ad_check_permissions` yields -/// `ErrPermDenied` on a stub build — the expected signal for a CI runner -/// that has no OS accessibility permission. +/// `Unknown` via the trait default, so `ad_check_permissions` yields +/// `ErrPlatformNotSupported` on a stub build — the honest signal that the +/// adapter never probed permissions, rather than that one was denied. #[cfg(feature = "stub-adapter")] struct StubAdapter; diff --git a/crates/ffi/tests/c_abi_passthrough.rs b/crates/ffi/tests/c_abi_passthrough.rs index 361dea5..f4b31b5 100644 --- a/crates/ffi/tests/c_abi_passthrough.rs +++ b/crates/ffi/tests/c_abi_passthrough.rs @@ -93,27 +93,27 @@ fn stub_ad_version_always_succeeds() { } } -/// `ad_check_permissions` maps the stub adapter's `Denied` permission state -/// to `ErrPermDenied (-1)`, not `ErrPlatformNotSupported (-8)`. This is the -/// documented exception. Cross-platform callers must treat both codes as -/// "adapter not operational here". +/// `ad_check_permissions` reports `ErrPlatformNotSupported (-8)` on an adapter +/// that never probes permissions. Not having measured a permission is a +/// different fact from a user having denied one, and `ErrPermDenied (-1)` sends +/// callers looking for a grant that does not exist on that platform. #[cfg(feature = "stub-adapter")] #[test] -fn stub_ad_check_permissions_returns_err_perm_denied() { +fn stub_ad_check_permissions_returns_err_platform_not_supported() { with_adapter(|adapter| unsafe { let rc = ad_check_permissions(adapter); assert_eq!( rc, - AdResult::ErrPermDenied, - "stub adapter permission_report() returns Denied → ErrPermDenied (-1), \ - not ErrPlatformNotSupported (-8). Both mean the adapter is not operational." + AdResult::ErrPlatformNotSupported, + "an adapter that does not probe permissions reports Unknown, which must \ + surface as ErrPlatformNotSupported (-8), never ErrPermDenied (-1)." ); let msg = ad_last_error_message(); assert!( !msg.is_null(), - "last-error message must be set on ErrPermDenied" + "last-error message must be set on ErrPlatformNotSupported" ); - assert_eq!(ad_last_error_code(), AdResult::ErrPermDenied); + assert_eq!(ad_last_error_code(), AdResult::ErrPlatformNotSupported); }); }