boos
5501558354
The previous approval registry used a oneshot channel, so two independent signals decided an approval: the HTTP handler's result (from Sender::send's bool) and the agent loop's allow/deny (from which timeout arm fired). At the 300s boundary these could disagree — a late /approve could see send() return Ok (receiver not yet dropped) and report ok:true to the user, while the timeout's final poll simultaneously fired and denied the tool. The frontend was told "approved" while nothing ran. The mirror case (ok:false but allowed) was impossible, so it never executed an unapproved tool, but the user-facing signal could lie. Replace the channel with a single source of truth: each entry is an AtomicU8 state machine (Pending -> Approved/Denied/TimedOut) plus a Notify. The decision is made by exactly one compare_exchange from Pending — whoever wins (approver or timeout) settles it, everyone else observes a losing CAS and no-ops. resolve() now returns whether *its* CAS won, so /approve tells the frontend the truth; the loop decides purely from the settled atomic. They read the same atomic and cannot disagree. A PendingGuard removes the registry entry on drop (normal completion or aborted/disconnected session), so there is no leak on any path. The callback also emits an approval_resolved SSE event with the final decision so a late click — already told ok:false — can dismiss its modal with the real outcome. Verified with an integration test that races resolve() against a near-zero timeout 5000x and asserts ok==true iff the executed decision is Approved (never a mixed state). Run via tests/ because the in-crate lib test binary is currently blocked by unrelated AppConfig test fixtures in other modules.
84 lines
3.1 KiB
Rust
84 lines
3.1 KiB
Rust
//! Integration tests for the Agent Workbench approval registry.
|
|
//!
|
|
//! These run against the library's public API (not the in-crate `#[cfg(test)]`
|
|
//! modules), so they compile and execute independently of unrelated test
|
|
//! fixtures elsewhere in the crate. They pin the core safety invariant of the
|
|
//! atomic approval state machine:
|
|
//!
|
|
//! the value `/api/agent/approve` reports back (`resolve` -> bool) and the
|
|
//! decision the agent loop executes (`wait_for_decision` -> SettledState)
|
|
//! are derived from the *same* atomic, so they can never disagree.
|
|
|
|
use boosapi::services::agent_workbench::approval::{
|
|
register, resolve, wait_for_decision, ApprovalOutcome, SettledState,
|
|
};
|
|
use std::time::Duration;
|
|
|
|
#[tokio::test]
|
|
async fn approve_then_wait_observes_approved() {
|
|
let (entry, _guard) = register("it-approve");
|
|
assert!(resolve("it-approve", ApprovalOutcome::Approve));
|
|
assert_eq!(
|
|
wait_for_decision(&entry, Duration::from_secs(5)).await,
|
|
SettledState::Approved
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn deny_then_wait_observes_denied() {
|
|
let (entry, _guard) = register("it-deny");
|
|
assert!(resolve("it-deny", ApprovalOutcome::Deny));
|
|
assert_eq!(
|
|
wait_for_decision(&entry, Duration::from_secs(5)).await,
|
|
SettledState::Denied
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn timeout_denies_and_late_click_is_truthfully_rejected() {
|
|
let (entry, _guard) = register("it-timeout");
|
|
// No resolve arrives before the (tiny) timeout.
|
|
assert_eq!(
|
|
wait_for_decision(&entry, Duration::from_millis(20)).await,
|
|
SettledState::TimedOut
|
|
);
|
|
// A late click must be told ok:false — the timeout already claimed it.
|
|
assert!(!resolve("it-timeout", ApprovalOutcome::Approve));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn unknown_id_resolves_false() {
|
|
assert!(!resolve("it-unknown", ApprovalOutcome::Approve));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn guard_drop_cleans_up_registry() {
|
|
{
|
|
let (_entry, _guard) = register("it-guard");
|
|
}
|
|
assert!(!resolve("it-guard", ApprovalOutcome::Approve));
|
|
}
|
|
|
|
/// The invariant under contention: race a `resolve(Approve)` against a
|
|
/// near-zero timeout thousands of times. The frontend-reported success
|
|
/// (`resolve` -> true) must hold if and only if the executed decision is
|
|
/// `Approved`; otherwise the decision must be `TimedOut`. The mixed state
|
|
/// (frontend told "approved" while the loop denied, or vice versa) must never
|
|
/// occur — that was the bug the atomic state machine fixes.
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
|
|
async fn resolve_and_timeout_never_disagree() {
|
|
for i in 0..5000 {
|
|
let id = format!("it-race-{i}");
|
|
let (entry, _guard) = register(&id);
|
|
let id2 = id.clone();
|
|
let resolver = tokio::spawn(async move { resolve(&id2, ApprovalOutcome::Approve) });
|
|
let decided = wait_for_decision(&entry, Duration::from_nanos(1)).await;
|
|
let frontend_ok = resolver.await.unwrap();
|
|
match (frontend_ok, decided) {
|
|
(true, SettledState::Approved) => {}
|
|
(false, SettledState::TimedOut) => {}
|
|
other => panic!("inconsistent outcome at i={i}: {other:?}"),
|
|
}
|
|
}
|
|
}
|