Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/submission-escape-hatch.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Made submitted messages appear before startup work and added a safe short Escape window to return unsent text and images for editing.
123 changes: 115 additions & 8 deletions n00n-agent/src/agent/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use super::history::{History, sanitize_cancelled_history};
use super::instructions::LoadedInstructions;
use super::streaming::stream_with_retry;
use super::tool_dispatch::{self, RecentCalls};
use crate::cancel::{CancelMap, CancelToken};
use crate::cancel::{CancelMap, CancelToken, PreDispatchGate};
use crate::mcp::McpHandle;
use crate::permissions::PermissionManager;
use crate::tools::{Deadline, FileReadTracker, LocalTools, ToolAudience, ToolContext};
Expand Down Expand Up @@ -103,7 +103,9 @@ pub struct Agent<'h> {
recent_calls: RecentCalls,
auto_compact: bool,
loaded_instructions: LoadedInstructions,
rollback_len: usize,
pre_dispatch_rollback_len: Option<usize>,
rollback_len: Option<usize>,
pre_dispatch_gate: Option<Arc<PreDispatchGate>>,
mcp: Option<McpHandle>,
config: AgentConfig,
tool_output_lines: ToolOutputLines,
Expand Down Expand Up @@ -149,7 +151,9 @@ impl<'h> Agent<'h> {
recent_calls: RecentCalls::new(),
auto_compact: compaction::auto_compact_enabled(),
loaded_instructions: LoadedInstructions::new(),
rollback_len: 0,
pre_dispatch_rollback_len: None,
rollback_len: None,
pre_dispatch_gate: None,
mcp: None,
reauth_attempts: 0,
post_tool_empty_retried: false,
Expand Down Expand Up @@ -192,6 +196,18 @@ impl<'h> Agent<'h> {
self
}

#[must_use]
pub fn with_pre_dispatch_gate(mut self, gate: Arc<PreDispatchGate>) -> Self {
self.pre_dispatch_gate = Some(gate);
self
}

#[must_use]
pub fn with_pre_dispatch_rollback_len(mut self, rollback_len: usize) -> Self {
self.pre_dispatch_rollback_len = Some(rollback_len);
self
}

#[must_use]
pub fn with_local_tools(mut self, local_tools: LocalTools) -> Self {
self.local_tools = local_tools;
Expand Down Expand Up @@ -220,7 +236,11 @@ impl<'h> Agent<'h> {
/// Returns an error if the agent loop fails due to provider errors,
/// tool execution failures, or cancellation.
pub async fn run(&mut self, input: AgentInput) -> Result<(), AgentError> {
self.rollback_len = self.history.len();
let rollback_len = self.rollback_len.unwrap_or_else(|| self.history.len());
self.rollback_len = Some(rollback_len);
let pre_dispatch_rollback_len = self
.pre_dispatch_rollback_len
.unwrap_or_else(|| rollback_len);
let msg = Message::user_with_images(input.message.clone(), input.images);
self.history.push(msg);
self.context_size =
Expand All @@ -246,7 +266,16 @@ impl<'h> Agent<'h> {
.await;

if matches!(result, Err(AgentError::Cancelled)) {
sanitize_cancelled_history(self.history, self.rollback_len);
if self
.pre_dispatch_gate
.as_ref()
.is_some_and(|gate| gate.is_cancelled())
{
self.history.truncate(pre_dispatch_rollback_len);
} else {
let rollback_len = self.rollback_len.unwrap_or_else(|| self.history.len());
sanitize_cancelled_history(self.history, rollback_len);
}
}

result
Expand All @@ -270,8 +299,14 @@ impl<'h> Agent<'h> {
}
}

fn commit_pre_dispatch(&self) -> bool {
self.pre_dispatch_gate
.as_ref()
.is_none_or(|gate| gate.try_commit())
}

async fn turn(&mut self) -> Result<TurnOutcome, AgentError> {
if self.cancel.is_cancelled() {
if self.cancel.is_cancelled() || !self.commit_pre_dispatch() {
return Err(AgentError::Cancelled);
}
let response = match stream_with_retry(
Expand Down Expand Up @@ -495,6 +530,9 @@ impl<'h> Agent<'h> {
}

async fn do_compact(&mut self) -> Result<(), AgentError> {
if !self.commit_pre_dispatch() {
return Err(AgentError::Cancelled);
}
let (compact_provider, compact_model) = resolve_compaction_model(
&self.provider,
&self.model,
Expand All @@ -511,7 +549,7 @@ impl<'h> Agent<'h> {
.await?;
let cost = usage.cost(&compact_model.pricing, false);
self.record_usage(usage, cost);
self.rollback_len = self.history.len();
self.rollback_len = Some(self.history.len());
self.event_tx.send(AgentEvent::CompactionDone)?;
self.history
.push(Message::synthetic(CONTINUE_AFTER_COMPACT.into()));
Expand Down Expand Up @@ -593,13 +631,17 @@ pub fn estimate_tool_tokens(tools: &Value) -> u32 {
#[cfg(test)]
mod tests {
use std::collections::VecDeque;
use std::sync::{Arc, Mutex};
use std::sync::{
Arc, Mutex,
atomic::{AtomicUsize, Ordering},
};

use n00n_providers::provider::{BoxFuture, Provider};
use n00n_providers::{
ContentBlock, ImageMediaType, ImageSource, Message, Model, ProviderEvent, RequestOptions,
Role, StopReason, StreamResponse, TokenUsage,
};
use n00n_storage::sessions::TranscriptEntry;
use serde_json::Value;
use test_case::test_case;

Expand Down Expand Up @@ -698,13 +740,17 @@ mod tests {
struct MockProvider {
responses: Mutex<Vec<StreamResponse>>,
requests: Arc<Mutex<Vec<Vec<Message>>>>,
cancel_on_request: Option<usize>,
calls: AtomicUsize,
}

impl MockProvider {
fn new(responses: Vec<StreamResponse>) -> Self {
Self {
responses: Mutex::new(responses),
requests: Arc::new(Mutex::new(Vec::new())),
cancel_on_request: None,
calls: AtomicUsize::new(0),
}
}

Expand All @@ -714,10 +760,21 @@ mod tests {
Self {
responses: Mutex::new(responses),
requests: Arc::clone(&requests),
cancel_on_request: None,
calls: AtomicUsize::new(0),
},
requests,
)
}

fn cancel_on_request(responses: Vec<StreamResponse>, request: usize) -> Self {
Self {
responses: Mutex::new(responses),
requests: Arc::new(Mutex::new(Vec::new())),
cancel_on_request: Some(request),
calls: AtomicUsize::new(0),
}
}
}

impl Provider for MockProvider {
Expand All @@ -732,6 +789,10 @@ mod tests {
_: Option<&'a SessionRef>,
) -> BoxFuture<'a, Result<StreamResponse, AgentError>> {
Box::pin(async {
let request = self.calls.fetch_add(1, Ordering::Relaxed);
if self.cancel_on_request == Some(request) {
return Err(AgentError::Cancelled);
}
self.requests.lock().unwrap().push(messages.to_vec());
let mut responses = self.responses.lock().unwrap();
assert!(!responses.is_empty(), "MockProvider: no more responses");
Expand Down Expand Up @@ -1215,6 +1276,52 @@ mod tests {
});
}

#[test]
fn post_compaction_cancellation_keeps_compaction_transcript_and_cleanup() {
smol::block_on(async {
let mut history = History::new(vec![Message::user("x".repeat(680_000))]);
let (mut agent, _event_rx) = make_agent(
MockProvider::cancel_on_request(vec![text_response(StopReason::EndTurn)], 1),
&mut history,
);
agent.model = Arc::new(small_context_model(200_000, 8_192));

let result = agent.run(default_input()).await;

assert!(matches!(result, Err(AgentError::Cancelled)));
drop(agent);
assert!(
history
.transcript()
.iter()
.any(|entry| matches!(entry, TranscriptEntry::Compaction { .. }))
);
assert_ends_with_cancel_marker(&history);
});
}

#[test]
fn pre_dispatch_cancellation_rolls_back_without_provider_call() {
smol::block_on(async {
let (provider, requests) =
MockProvider::recording(vec![text_response(StopReason::EndTurn)]);
let mut history = History::new(Vec::new());
let (mut agent, _event_rx) = make_agent(provider, &mut history);
let gate = Arc::new(PreDispatchGate::new());
assert!(gate.try_cancel());
agent = agent
.with_pre_dispatch_gate(gate)
.with_pre_dispatch_rollback_len(0);

let result = agent.run(default_input()).await;

assert!(matches!(result, Err(AgentError::Cancelled)));
drop(agent);
assert_eq!(history.len(), 0);
assert_eq!(requests.lock().unwrap().len(), 0);
});
}

#[test_case(
vec![tool_call_response("nonexistent_tool_xyz", "t1"), text_response(StopReason::EndTurn)],
"t1"
Expand Down
101 changes: 100 additions & 1 deletion n00n-agent/src/cancel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use std::collections::HashMap;
use std::future::Future;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::atomic::{AtomicBool, AtomicU8, Ordering};
use std::sync::{Arc, Mutex};

use event_listener::Event;
Expand All @@ -27,6 +27,63 @@ pub struct CancelToken(Arc<Shared>);

pub struct CancelTrigger(Arc<Shared>);

const PRE_DISPATCH_PENDING: u8 = 0;
const PRE_DISPATCH_COMMITTED: u8 = 1;
const PRE_DISPATCH_CANCELLED: u8 = 2;

/// One-shot atomic handoff between UI cancellation and provider dispatch.
pub struct PreDispatchGate(AtomicU8);

impl Default for PreDispatchGate {
fn default() -> Self {
Self::new()
}
}

impl PreDispatchGate {
#[must_use]
pub const fn new() -> Self {
Self(AtomicU8::new(PRE_DISPATCH_PENDING))
}

/// Claims the submission before its first provider request can start.
#[must_use]
pub fn try_cancel(&self) -> bool {
self.0
.compare_exchange(
PRE_DISPATCH_PENDING,
PRE_DISPATCH_CANCELLED,
Ordering::AcqRel,
Ordering::Acquire,
)
.is_ok()
}

/// Commits provider dispatch. Repeated calls support safe provider retries.
#[must_use]
pub fn try_commit(&self) -> bool {
match self.0.compare_exchange(
PRE_DISPATCH_PENDING,
PRE_DISPATCH_COMMITTED,
Ordering::AcqRel,
Ordering::Acquire,
) {
Ok(_) | Err(PRE_DISPATCH_COMMITTED) => true,
Err(_) => false,
}
}

#[must_use]
pub fn is_cancelled(&self) -> bool {
self.0.load(Ordering::Acquire) == PRE_DISPATCH_CANCELLED
}

#[must_use]
pub fn is_committed(&self) -> bool {
self.0.load(Ordering::Acquire) == PRE_DISPATCH_COMMITTED
}
}

impl CancelToken {
#[must_use]
pub fn new() -> (CancelTrigger, Self) {
Expand Down Expand Up @@ -315,4 +372,46 @@ mod tests {
map.cancel_or_precancel("x".to_owned());
assert!(tok2.is_cancelled());
}

#[test]
fn pre_dispatch_cancel_and_commit_are_mutually_exclusive() {
let gate = Arc::new(PreDispatchGate::new());
let barrier = Arc::new(std::sync::Barrier::new(3));
let cancel = {
let gate = Arc::clone(&gate);
let barrier = Arc::clone(&barrier);
std::thread::spawn(move || {
barrier.wait();
gate.try_cancel()
})
};
let commit = {
let gate = Arc::clone(&gate);
let barrier = Arc::clone(&barrier);
std::thread::spawn(move || {
barrier.wait();
gate.try_commit()
})
};
barrier.wait();

let cancel_won = cancel.join().unwrap();
let commit_won = commit.join().unwrap();
assert_ne!(cancel_won, commit_won);
assert_eq!(gate.is_cancelled(), cancel_won);
assert_eq!(gate.is_committed(), commit_won);
}

#[test]
fn pre_dispatch_lifecycle_is_monotonic() {
let cancelled = PreDispatchGate::new();
assert!(cancelled.try_cancel());
assert!(!cancelled.try_cancel());
assert!(!cancelled.try_commit());

let committed = PreDispatchGate::new();
assert!(committed.try_commit());
assert!(committed.try_commit());
assert!(!committed.try_cancel());
}
}
3 changes: 2 additions & 1 deletion n00n-agent/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub use agent::{
Agent, AgentParams, AgentRunParams, History, Instructions, LoadedInstructions, SharedMessages,
SharedTranscript, find_subdirectory_instructions, is_instruction_file,
};
pub use cancel::{CancelMap, CancelToken, CancelTrigger};
pub use cancel::{CancelMap, CancelToken, CancelTrigger, PreDispatchGate};
pub use n00n_config::{AgentConfig, PermissionsConfig, ToolOutputLines};
pub mod command;
pub mod diff;
Expand Down Expand Up @@ -77,6 +77,7 @@ pub struct McpPromptRef {
pub arguments: HashMap<String, String>,
}

#[derive(Clone)]
pub struct AgentInput {
pub message: String,
pub mode: AgentMode,
Expand Down
Loading
Loading