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
12 changes: 12 additions & 0 deletions src/agent/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,18 @@ pub async fn build_agent_inner<M: CompletionModel + 'static>(
// was checked, so users couldn't set a default in config.json.
if let Some(temp) = cli.resolve_temperature(cfg) {
let clamped = temp.clamp(0.0, 2.0);
if (clamped - temp).abs() > f64::EPSILON {
// Warn ONCE per process if the user's value was clamped
// — previously silent, so a user with `temperature: 3.5`
// got 2.0 and never knew.
static WARNED: std::sync::OnceLock<()> = std::sync::OnceLock::new();
if WARNED.set(()).is_ok() {
eprintln!(
"warning: temperature {} clamped to {} (valid range 0.0..=2.0)",
temp, clamped,
);
}
}
builder = builder.temperature(clamped);
}

Expand Down
12 changes: 12 additions & 0 deletions src/agent/tools/modified.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ pub static MODIFIED_FILES: LazyLock<Mutex<IndexSet<PathBuf>>> =
LazyLock::new(|| Mutex::new(IndexSet::new()));

/// Record that `path` was modified by a write/edit/apply_patch tool call.
/// Maximum entries retained in the modified-files set. Older entries
/// fall off when the cap is reached so a long session editing many
/// files doesn't grow this set unboundedly. The panel only renders
/// the last few entries anyway, so trimming older ones is invisible
/// to the user.
const MAX_MODIFIED: usize = 256;

/// Best-effort canonicalize; falls back to the path as given when the file
/// doesn't exist yet or canonicalize fails.
pub fn mark_modified(path: &Path) {
Expand All @@ -22,6 +29,11 @@ pub fn mark_modified(path: &Path) {
// IndexSet preserves insertion order and dedups; we want the most-recent
// touch to surface at the end, so re-insert moves the entry.
set.shift_remove(&canonical);
// Cap the set BEFORE inserting so we always have room for the
// freshest entry. Oldest (front) gets evicted.
while set.len() >= MAX_MODIFIED {
set.shift_remove_index(0);
}
set.insert(canonical);
}

Expand Down
14 changes: 14 additions & 0 deletions src/agent/tools/todo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,20 @@ impl Tool for WriteTodoList {
async fn call(&self, args: TodoWriteArgs) -> Result<String, ToolError> {
check_perm(&self.permission, &self.ask_tx, "write_todo_list", "").await?;

// Cap the todo list so a pathological agent can't bloat
// memory + every subsequent prompt by spamming hundreds of
// todos. 50 is generous for any reasonable plan; lists
// longer than that are usually a sign the agent should
// break the task into a separate plan/loop pass.
const MAX_TODOS: usize = 50;
if args.todos.len() > MAX_TODOS {
return Err(ToolError::Msg(format!(
"todo list too long ({} items); cap is {}. Trim the list or split the work across multiple turns.",
args.todos.len(),
MAX_TODOS,
)));
}

let mut list = TODO_LIST.lock().unwrap_or_else(|e| e.into_inner());
*list = args.todos;

Expand Down
7 changes: 6 additions & 1 deletion src/agent/tools/websearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,12 @@ impl Tool for WebSearchTool {
async fn call(&self, args: WebSearchArgs) -> Result<String, ToolError> {
check_perm(&self.permission, &self.ask_tx, "websearch", &args.query).await?;

let client = reqwest::Client::new();
// Match webfetch's 15s timeout — without this, a hung Exa
// endpoint would stall the agent turn indefinitely.
let client = reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(15))
.build()
.map_err(|e| ToolError::Msg(format!("http client init failed: {e}")))?;
let body = ExaRequest {
query: &args.query,
search_type: "auto",
Expand Down
61 changes: 47 additions & 14 deletions src/permission/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,24 @@ impl PermissionChecker {
return CheckResult::Allowed;
}

let mut matched: Vec<Action> = Vec::new();
// Track both the action AND the matching pattern so denial
// messages can name which rule blocked the call (was just
// "Blocked by permission rules", giving the user no way to
// identify and edit the offending rule).
let mut matched: Vec<(Action, String)> = Vec::new();
if let Some(rules) = self.rules.get(tool) {
for (pattern, action) in rules {
if pattern.matches(input) {
matched.push(*action);
matched.push((*action, pattern.original.clone()));
}
}
}

let base = matched.last().copied().unwrap_or(self.default_action);
let base = matched
.last()
.map(|(a, _)| *a)
.unwrap_or(self.default_action);
let last_pat = matched.last().map(|(_, p)| p.clone());
let action = match self.mode {
SecurityMode::Restrictive => {
if matched.is_empty() && self.default_action == Action::Allow {
Expand Down Expand Up @@ -174,9 +182,20 @@ impl PermissionChecker {
if self.is_doom_loop(tool, input) {
match self.doom_loop_action {
Action::Deny => {
return CheckResult::Denied(
"Doom loop: repeated identical tool call".to_string(),
);
// Name the call so the user can identify and
// either fix the LLM's behavior or relax the
// pattern.
let preview: String = input.chars().take(60).collect();
return CheckResult::Denied(format!(
"Doom loop: repeated identical {} call ({}{})",
tool,
preview,
if input.chars().count() > 60 {
"…"
} else {
""
},
));
}
Action::Ask => return CheckResult::Ask,
Action::Allow => {}
Expand All @@ -187,7 +206,10 @@ impl PermissionChecker {
match action {
Action::Allow => CheckResult::Allowed,
Action::Ask => CheckResult::Ask,
Action::Deny => CheckResult::Denied("Blocked by permission rules".to_string()),
Action::Deny => CheckResult::Denied(match last_pat {
Some(pat) => format!("Blocked by rule: {tool} {pat:?} → deny"),
None => format!("Blocked: {tool} denied by default action"),
}),
}
}

Expand All @@ -201,16 +223,20 @@ impl PermissionChecker {
}

let abs_path = resolve_absolute(path, &self.working_dir);
let mut matched: Vec<Action> = Vec::new();
let mut matched: Vec<(Action, String)> = Vec::new();
if let Some(rules) = self.rules.get(tool) {
for (pattern, action) in rules {
if pattern.matches(&abs_path) || pattern.matches(path) {
matched.push(*action);
matched.push((*action, pattern.original.clone()));
}
}
}

let base = matched.last().copied().unwrap_or(self.default_action);
let base = matched
.last()
.map(|(a, _)| *a)
.unwrap_or(self.default_action);
let last_pat = matched.last().map(|(_, p)| p.clone());
let action = match self.mode {
SecurityMode::Restrictive => {
if matched.is_empty() && self.default_action == Action::Allow {
Expand Down Expand Up @@ -245,9 +271,13 @@ impl PermissionChecker {
if self.is_doom_loop(tool, path) {
match self.doom_loop_action {
Action::Deny => {
return CheckResult::Denied(
"Doom loop: repeated identical tool call".to_string(),
);
let preview: String = path.chars().take(80).collect();
return CheckResult::Denied(format!(
"Doom loop: repeated identical {} call ({}{})",
tool,
preview,
if path.chars().count() > 80 { "…" } else { "" },
));
}
Action::Ask => return CheckResult::Ask,
Action::Allow => {}
Expand All @@ -258,7 +288,10 @@ impl PermissionChecker {
match action {
Action::Allow => CheckResult::Allowed,
Action::Ask => CheckResult::Ask,
Action::Deny => CheckResult::Denied("Blocked by permission rules".to_string()),
Action::Deny => CheckResult::Denied(match last_pat {
Some(pat) => format!("Blocked by rule: {tool} {pat:?} → deny"),
None => format!("Blocked: {tool} denied by default action"),
}),
}
}

Expand Down
51 changes: 51 additions & 0 deletions src/tests/checker_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,57 @@ fn deny_rule_not_blocked_by_yolo() {
assert!(matches!(result, CheckResult::Allowed));
}

/// Regression: deny messages must name the rule pattern that matched
/// so the user knows what to edit. Previously bare
/// `"Blocked by permission rules"` left them with no path forward.
#[test]
fn deny_message_names_the_matching_rule() {
let mut checker = make_checker(SecurityMode::Standard);
let result = checker.check("bash", "rm -rf /home/user/project");
match result {
CheckResult::Denied(msg) => {
assert!(
msg.contains("rm") || msg.contains("rule"),
"deny message must reference the rule: {msg}",
);
assert!(
!msg.eq("Blocked by permission rules"),
"deny message must not be generic: {msg}",
);
}
other => panic!("expected Denied; got {other:?}"),
}
}

/// Doom-loop deny message must name the offending tool + call.
#[test]
fn doom_loop_deny_names_the_call() {
use crate::permission::{Action, PermissionConfig};
let config = PermissionConfig {
doom_loop: Some(Action::Deny),
..PermissionConfig::default()
};
let mut checker = PermissionChecker::new(
&config,
SecurityMode::Standard,
Some(std::path::PathBuf::from("/tmp")),
);
// Three identical calls fires the doom-loop deny.
checker.check("bash", "echo hi");
checker.check("bash", "echo hi");
let result = checker.check("bash", "echo hi");
match result {
CheckResult::Denied(msg) => {
assert!(msg.contains("Doom loop"), "must say Doom loop: {msg}");
assert!(
msg.contains("bash") && msg.contains("echo hi"),
"must name tool + call preview: {msg}",
);
}
other => panic!("expected Denied; got {other:?}"),
}
}

// --- Doom loop detection ---

#[test]
Expand Down
Loading