mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-07 03:00:29 +00:00
Merge pull request #605 from fabro-sh/fix/remove-dead-agent-turn-limits
Remove dead agent execution limits
This commit is contained in:
commit
db473da2f3
19 changed files with 206 additions and 226 deletions
|
|
@ -1252,26 +1252,6 @@ Emitted when the agent detects a tool-use loop.
|
|||
|
||||
No properties.
|
||||
|
||||
### `agent.turn.limit`
|
||||
|
||||
Emitted when the agent reaches its maximum turn count.
|
||||
|
||||
```json
|
||||
{
|
||||
"id": "...", "ts": "...", "run_id": "...",
|
||||
"event": "agent.turn.limit",
|
||||
"node_id": "code", "node_label": "code",
|
||||
"session_id": "ses_abc",
|
||||
"properties": {
|
||||
"max_turns": 25
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
| Property | Type | Description |
|
||||
|----------|------|-------------|
|
||||
| `max_turns` | number | Maximum turns allowed |
|
||||
|
||||
### `agent.skill.expanded`
|
||||
|
||||
```json
|
||||
|
|
|
|||
|
|
@ -374,7 +374,6 @@ V2 keeps the current durable family surface broadly intact.
|
|||
- `agent.error`
|
||||
- `agent.warning`
|
||||
- `agent.loop.detected`
|
||||
- `agent.turn.limit`
|
||||
- `agent.steering.injected`
|
||||
- `agent.compaction.started`
|
||||
- `agent.compaction.completed`
|
||||
|
|
|
|||
|
|
@ -44,7 +44,6 @@ Sub-agent failures do not automatically fail the parent stage. The parent receiv
|
|||
|
||||
Common cases:
|
||||
|
||||
- **Hits `max_turns`** -- returns normally with its last output
|
||||
- **Panics or errors** -- returned as a failed `wait` result
|
||||
- **`spawn_agent` fails** -- returned immediately as a tool result
|
||||
|
||||
|
|
@ -97,5 +96,5 @@ Sub-agents are most useful for:
|
|||
Use [child runs](/execution/child-runs) instead when the delegated work should be a separate Fabro run with its own workflow, lifecycle, sandbox, checkpoints, and outputs.
|
||||
|
||||
<Note>
|
||||
Sub-agents run with no turn limit by default. Pass `max_turns` when you want predictable cost or time bounds. All active sub-agents are cleaned up automatically when the parent session closes.
|
||||
Sub-agents run until they complete, fail, are cancelled, or hit the session's wall-clock timeout. All active sub-agents are cleaned up automatically when the parent session closes.
|
||||
</Note>
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ date: "2026-03-23"
|
|||
|
||||
## Unlimited agent tool rounds
|
||||
|
||||
Agent stages no longer have an artificial cap on tool rounds per input or subagent turns. Previously, the default limits could cause agents to stop mid-task on complex operations. Both `max_tool_rounds_per_input` and subagent `max_turns` now default to unlimited, so agents run until they complete their work or hit the context window.
|
||||
Agent stages no longer have an artificial cap on tool rounds per input or subagent turns, so agents run until they complete their work or are interrupted.
|
||||
|
||||
## More
|
||||
|
||||
|
|
|
|||
|
|
@ -84,7 +84,7 @@ pub fn new(
|
|||
| Method | Description |
|
||||
|---|---|
|
||||
| `initialize().await` | Discovers project docs, skills, and MCP servers. Call before `process_input`. |
|
||||
| `process_input(input).await` | Sends user input and runs the agent loop until the model stops or a limit is hit. |
|
||||
| `process_input(input).await` | Sends user input and runs the agent loop until the model stops, the session is interrupted, or an error occurs. |
|
||||
| `close()` | Ends the session and emits `SessionEnded`. |
|
||||
| `interrupt()` | Cancels the current `process_input` call. |
|
||||
| `cancel_token()` | Returns a `CancellationToken` for external cancellation. |
|
||||
|
|
@ -110,8 +110,6 @@ All fields are public. Key settings with their defaults:
|
|||
|
||||
| Field | Default | Description |
|
||||
|---|---|---|
|
||||
| `max_turns` | `0` (unlimited) | Maximum conversation turns before stopping. |
|
||||
| `max_tool_rounds_per_input` | `200` | Maximum tool execution rounds per `process_input` call. |
|
||||
| `default_command_timeout_ms` | `10,000` | Default timeout for Bash tool commands. |
|
||||
| `max_command_timeout_ms` | `600,000` | Maximum allowed timeout for Bash tool commands. |
|
||||
| `enable_loop_detection` | `true` | Detect and break out of repetitive tool call patterns. |
|
||||
|
|
@ -221,7 +219,6 @@ Key `AgentEvent` variants:
|
|||
| `ToolCallCompleted { tool_name, tool_call_id, output, is_error }` | A tool call finished. |
|
||||
| `Error { error }` | An `AgentError` occurred. |
|
||||
| `LoopDetected` | The agent is repeating itself. |
|
||||
| `TurnLimitReached { max_turns }` | Turn limit hit. |
|
||||
| `CompactionStarted` / `CompactionCompleted` | Context window compaction. |
|
||||
| `SubAgentSpawned` / `SubAgentCompleted` | Sub-agent lifecycle. |
|
||||
| `McpServerReady` / `McpServerFailed` | MCP server connection status. |
|
||||
|
|
@ -296,7 +293,7 @@ All fallible `Session` methods return `Result<T, AgentError>`:
|
|||
| `SessionClosed` | `process_input` was called on a closed session. |
|
||||
| `InvalidState(String)` | The session is in an unexpected state. |
|
||||
| `ToolExecution(String)` | A tool execution failed. |
|
||||
| `Interrupted(InterruptReason)` | The session was cancelled (`Cancelled`) or timed out (`WallClockTimeout`). |
|
||||
| `Interrupted(InterruptReason)` | The session was cancelled or timed out. |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -98,8 +98,6 @@ pub trait Sandbox: Send + Sync {
|
|||
|
||||
```rust
|
||||
pub struct SessionConfig {
|
||||
pub max_turns: usize, // 0 = unlimited
|
||||
pub max_tool_rounds_per_input: usize, // default: 200
|
||||
pub default_command_timeout_ms: u64, // default: 10s
|
||||
pub max_command_timeout_ms: u64, // default: 600s
|
||||
pub enable_loop_detection: bool, // default: true
|
||||
|
|
@ -134,7 +132,6 @@ let env = Arc::new(LocalSandbox::new(
|
|||
|
||||
// 4. Configure the session
|
||||
let config = SessionConfig {
|
||||
max_tool_rounds_per_input: 50,
|
||||
enable_loop_detection: true,
|
||||
user_instructions: Some("Always write tests first".into()),
|
||||
..SessionConfig::default()
|
||||
|
|
|
|||
|
|
@ -106,8 +106,6 @@ pub struct ToolSecrets {
|
|||
|
||||
#[derive(Clone)]
|
||||
pub struct SessionOptions {
|
||||
pub max_turns: usize,
|
||||
pub max_tool_rounds_per_input: usize,
|
||||
pub default_command_timeout_ms: u64,
|
||||
pub max_command_timeout_ms: u64,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
|
|
@ -149,8 +147,6 @@ pub struct SessionOptions {
|
|||
impl std::fmt::Debug for SessionOptions {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("SessionOptions")
|
||||
.field("max_turns", &self.max_turns)
|
||||
.field("max_tool_rounds_per_input", &self.max_tool_rounds_per_input)
|
||||
.field(
|
||||
"default_command_timeout_ms",
|
||||
&self.default_command_timeout_ms,
|
||||
|
|
@ -196,8 +192,6 @@ impl std::fmt::Debug for SessionOptions {
|
|||
impl Default for SessionOptions {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
max_turns: 0,
|
||||
max_tool_rounds_per_input: 0,
|
||||
default_command_timeout_ms: 10_000,
|
||||
max_command_timeout_ms: 600_000,
|
||||
max_tokens: None,
|
||||
|
|
@ -280,8 +274,6 @@ mod tests {
|
|||
#[test]
|
||||
fn default_config_values() {
|
||||
let config = SessionOptions::default();
|
||||
assert_eq!(config.max_turns, 0);
|
||||
assert_eq!(config.max_tool_rounds_per_input, 0);
|
||||
assert_eq!(config.default_command_timeout_ms, 10_000);
|
||||
assert_eq!(config.max_command_timeout_ms, 600_000);
|
||||
assert!(config.reasoning_effort.is_none());
|
||||
|
|
@ -328,13 +320,10 @@ mod tests {
|
|||
#[test]
|
||||
fn config_with_custom_values() {
|
||||
let config = SessionOptions {
|
||||
max_turns: 50,
|
||||
reasoning_effort: Some(ReasoningEffort::High),
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(config.max_turns, 50);
|
||||
assert_eq!(config.reasoning_effort, Some(ReasoningEffort::High));
|
||||
assert_eq!(config.max_tool_rounds_per_input, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -1195,10 +1195,6 @@ impl Session {
|
|||
self.config.speed = speed;
|
||||
}
|
||||
|
||||
pub const fn set_max_turns(&mut self, max_turns: usize) {
|
||||
self.config.max_turns = max_turns;
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub const fn history(&self) -> &History {
|
||||
&self.history
|
||||
|
|
@ -1210,7 +1206,14 @@ impl Session {
|
|||
}
|
||||
|
||||
pub async fn process_input(&mut self, input: &str) -> Result<(), Error> {
|
||||
self.process_input_with_runtime(input, AgentToolRuntime::default())
|
||||
self.process_input_with_output(input).await.map(drop)
|
||||
}
|
||||
|
||||
pub(crate) async fn process_input_with_output(
|
||||
&mut self,
|
||||
input: &str,
|
||||
) -> Result<Option<String>, Error> {
|
||||
self.process_input_with_runtime_and_output(input, AgentToolRuntime::default())
|
||||
.await
|
||||
}
|
||||
|
||||
|
|
@ -1237,6 +1240,16 @@ impl Session {
|
|||
input: &str,
|
||||
agent_tool_runtime: AgentToolRuntime,
|
||||
) -> Result<(), Error> {
|
||||
self.process_input_with_runtime_and_output(input, agent_tool_runtime)
|
||||
.await
|
||||
.map(drop)
|
||||
}
|
||||
|
||||
async fn process_input_with_runtime_and_output(
|
||||
&mut self,
|
||||
input: &str,
|
||||
agent_tool_runtime: AgentToolRuntime,
|
||||
) -> Result<Option<String>, Error> {
|
||||
let mut timing = SessionInputTiming::default();
|
||||
let mut usage = TokenCounts::default();
|
||||
let mut cost = None;
|
||||
|
|
@ -1322,7 +1335,7 @@ impl Session {
|
|||
timing: &mut SessionInputTiming,
|
||||
usage_accumulator: &mut TokenCounts,
|
||||
cost_accumulator: &mut Option<UsdMicros>,
|
||||
) -> Result<(), Error> {
|
||||
) -> Result<Option<String>, Error> {
|
||||
const STREAM_CONSUME_RETRIES: usize = 3;
|
||||
|
||||
if self.state == SessionState::Closed {
|
||||
|
|
@ -1360,8 +1373,6 @@ impl Session {
|
|||
text: expanded_input.clone(),
|
||||
});
|
||||
|
||||
let mut round_count: usize = 0;
|
||||
|
||||
loop {
|
||||
// Top-of-loop: if the previous round's interrupt token fired,
|
||||
// swap in a fresh one before draining and rebuilding state.
|
||||
|
|
@ -1394,26 +1405,6 @@ impl Session {
|
|||
self.wait_for_steer_if_needed().await?;
|
||||
self.drain_steering();
|
||||
|
||||
// Check max_tool_rounds_per_input
|
||||
if self.config.max_tool_rounds_per_input > 0
|
||||
&& round_count >= self.config.max_tool_rounds_per_input
|
||||
{
|
||||
self.event_emitter
|
||||
.emit(self.id.clone(), AgentEvent::TurnLimitReached {
|
||||
max_turns: self.config.max_tool_rounds_per_input,
|
||||
});
|
||||
break;
|
||||
}
|
||||
|
||||
// Check max_turns
|
||||
if self.config.max_turns > 0 && self.history.turns().len() >= self.config.max_turns {
|
||||
self.event_emitter
|
||||
.emit(self.id.clone(), AgentEvent::TurnLimitReached {
|
||||
max_turns: self.config.max_turns,
|
||||
});
|
||||
break;
|
||||
}
|
||||
|
||||
// Snapshot the per-round token; it stays stable for this iteration.
|
||||
let round_token = self
|
||||
.round_token
|
||||
|
|
@ -1773,11 +1764,9 @@ impl Session {
|
|||
if should_continue {
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
return Ok((!text.trim().is_empty()).then_some(text));
|
||||
}
|
||||
|
||||
round_count += 1;
|
||||
|
||||
// Build a composite cancellation token covering both terminal
|
||||
// cancel and round (steer) interrupt. Tools observe it
|
||||
// cooperatively — they synthesize "Cancelled" results rather
|
||||
|
|
@ -1859,8 +1848,6 @@ impl Session {
|
|||
.emit(self.id.clone(), AgentEvent::LoopDetected);
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn compact_if_needed(&mut self) {
|
||||
|
|
@ -2259,8 +2246,9 @@ mod tests {
|
|||
#[tokio::test]
|
||||
async fn text_only_response_natural_completion() {
|
||||
let mut session = make_session(vec![text_response("Hello there!")]).await;
|
||||
session.process_input("Hi").await.unwrap();
|
||||
let output = session.process_input_with_output("Hi").await.unwrap();
|
||||
|
||||
assert_eq!(output.as_deref(), Some("Hello there!"));
|
||||
assert_eq!(session.state(), SessionState::Idle);
|
||||
let turns = session.history().turns();
|
||||
// UserTurn + AssistantTurn = 2
|
||||
|
|
@ -2445,55 +2433,12 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn max_tool_rounds_enforced() {
|
||||
let mut registry = ToolRegistry::new();
|
||||
registry.register(make_echo_tool());
|
||||
async fn empty_natural_completion_has_no_output() {
|
||||
let mut session = make_session(vec![text_response(" ")]).await;
|
||||
|
||||
// Respond with tool calls indefinitely
|
||||
let responses = vec![
|
||||
tool_call_response("echo", "call_1", serde_json::json!({"text": "a"})),
|
||||
tool_call_response("echo", "call_2", serde_json::json!({"text": "b"})),
|
||||
tool_call_response("echo", "call_3", serde_json::json!({"text": "c"})),
|
||||
];
|
||||
let output = session.process_input_with_output("Hi").await.unwrap();
|
||||
|
||||
let config = SessionOptions {
|
||||
max_tool_rounds_per_input: 2,
|
||||
enable_loop_detection: false,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let mut session = make_session_with_tools_and_config(responses, registry, config).await;
|
||||
session.process_input("Keep using tools").await.unwrap();
|
||||
|
||||
// Should stop after 2 rounds: User + (Asst+ToolResult) * 2 = 5 turns
|
||||
assert_eq!(session.state(), SessionState::Idle);
|
||||
let turns = session.history().turns();
|
||||
assert_eq!(turns.len(), 5);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn max_turns_enforced() {
|
||||
let responses = vec![
|
||||
text_response("first"),
|
||||
text_response("second"),
|
||||
text_response("should not reach"),
|
||||
];
|
||||
|
||||
let config = SessionOptions {
|
||||
max_turns: 3,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let mut session = make_session_with_config(responses, config).await;
|
||||
|
||||
// First input: adds User + Assistant = 2 turns
|
||||
session.process_input("one").await.unwrap();
|
||||
assert_eq!(session.history().turns().len(), 2);
|
||||
|
||||
// Second input: adds User (now 3 turns), then max_turns check triggers
|
||||
session.process_input("two").await.unwrap();
|
||||
// Should have 3 turns total (User + Asst + User), max_turns hit before LLM call
|
||||
assert_eq!(session.history().turns().len(), 3);
|
||||
assert_eq!(output, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ use crate::error::Error;
|
|||
use crate::session::Session;
|
||||
use crate::tool_registry::{RegisteredTool, ToolSource};
|
||||
use crate::tools::required_str;
|
||||
use crate::types::{AgentEvent, Message, SessionEvent};
|
||||
use crate::types::{AgentEvent, SessionEvent};
|
||||
|
||||
pub type SessionFactory = Arc<dyn Fn() -> Session + Send + Sync>;
|
||||
|
||||
|
|
@ -112,15 +112,18 @@ impl SubAgentManager {
|
|||
let task_prompt_for_spawn = task_prompt.clone();
|
||||
let task = tokio::spawn(async move {
|
||||
session.initialize().await?;
|
||||
session.process_input(&task_prompt_for_spawn).await?;
|
||||
let output = session
|
||||
.process_input_with_output(&task_prompt_for_spawn)
|
||||
.await?
|
||||
.ok_or_else(|| {
|
||||
Error::InvalidState(
|
||||
"Subagent completed without a non-empty final response".to_string(),
|
||||
)
|
||||
})?;
|
||||
let turns = session.history().turns();
|
||||
let last_text = turns.iter().rev().find_map(|t| match t {
|
||||
Message::Assistant { content, .. } => Some(content.clone()),
|
||||
_ => None,
|
||||
});
|
||||
Ok(SubAgentResult {
|
||||
output: last_text.unwrap_or_default(),
|
||||
success: true,
|
||||
output,
|
||||
success: true,
|
||||
turns_used: turns.len(),
|
||||
})
|
||||
});
|
||||
|
|
@ -314,18 +317,6 @@ pub fn make_spawn_agent_tool(
|
|||
"task": {
|
||||
"type": "string",
|
||||
"description": "The task description for the subagent"
|
||||
},
|
||||
"working_dir": {
|
||||
"type": "string",
|
||||
"description": "Working directory for the subagent"
|
||||
},
|
||||
"model": {
|
||||
"type": "string",
|
||||
"description": "Model to use for the subagent"
|
||||
},
|
||||
"max_turns": {
|
||||
"type": "integer",
|
||||
"description": "Maximum number of turns for the subagent"
|
||||
}
|
||||
},
|
||||
"required": ["task"]
|
||||
|
|
@ -337,13 +328,6 @@ pub fn make_spawn_agent_tool(
|
|||
Box::pin(async move {
|
||||
let task = required_str(&args, "task")?;
|
||||
|
||||
// Extract optional max_turns parameter
|
||||
let max_turns = args
|
||||
.get("max_turns")
|
||||
.and_then(serde_json::Value::as_u64)
|
||||
.map(|v| usize::try_from(v).unwrap_or(usize::MAX));
|
||||
|
||||
// Note: working_dir and model require session factory changes to wire through
|
||||
let mut session = session_factory();
|
||||
// Inherit the parent agent's root session ID so todo tools
|
||||
// that scope by root (e.g. Anthropic tasks) share one list
|
||||
|
|
@ -351,9 +335,6 @@ pub fn make_spawn_agent_tool(
|
|||
if let Some(root) = ctx.root_session_id.as_ref().or(ctx.session_id.as_ref()) {
|
||||
session.set_root_session_id(root.clone());
|
||||
}
|
||||
// Default subagent max_turns is 0 (unlimited) per spec (overridable via
|
||||
// parameter)
|
||||
session.set_max_turns(max_turns.unwrap_or(0));
|
||||
let mut mgr = manager.lock().await;
|
||||
mgr.spawn(session, task.to_string(), current_depth)
|
||||
.map_err(|e| e.to_string())
|
||||
|
|
@ -619,7 +600,11 @@ mod tests {
|
|||
|
||||
let spawn_tool = make_spawn_agent_tool(manager.clone(), factory, 0);
|
||||
assert_eq!(spawn_tool.definition.name, "spawn_agent");
|
||||
assert!(spawn_tool.definition.parameters["properties"]["task"].is_object());
|
||||
let spawn_properties = spawn_tool.definition.parameters["properties"]
|
||||
.as_object()
|
||||
.unwrap();
|
||||
assert_eq!(spawn_properties.len(), 1);
|
||||
assert!(spawn_properties["task"].is_object());
|
||||
let spawn_required = spawn_tool.definition.parameters["required"]
|
||||
.as_array()
|
||||
.unwrap();
|
||||
|
|
@ -831,6 +816,21 @@ mod tests {
|
|||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn empty_final_response_is_not_reported_as_success() {
|
||||
let mut manager = SubAgentManager::new(3);
|
||||
let session = make_session(vec![text_response("")]).await;
|
||||
let agent_id = manager.spawn(session, "Do something".into(), 0).unwrap();
|
||||
|
||||
let result = manager.wait(&agent_id).await;
|
||||
|
||||
assert!(
|
||||
matches!(result, Err(Error::InvalidState(message)) if message.contains(
|
||||
"without a non-empty final response"
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn send_input_to_completed_agent_errors() {
|
||||
let mut manager = SubAgentManager::new(3);
|
||||
|
|
|
|||
|
|
@ -284,9 +284,6 @@ pub enum AgentEvent {
|
|||
details: serde_json::Value,
|
||||
},
|
||||
LoopDetected,
|
||||
TurnLimitReached {
|
||||
max_turns: usize,
|
||||
},
|
||||
SteeringInjected {
|
||||
text: String,
|
||||
/// Principal that authored the steer. Lifted to top-level
|
||||
|
|
@ -464,9 +461,6 @@ impl AgentEvent {
|
|||
Self::LoopDetected => {
|
||||
warn!(session_id, "Loop detected");
|
||||
}
|
||||
Self::TurnLimitReached { max_turns } => {
|
||||
warn!(session_id, max_turns, "Message limit reached");
|
||||
}
|
||||
Self::SteeringInjected { text, .. } => {
|
||||
debug!(session_id, text_len = text.len(), "Steering injected");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -53,7 +53,6 @@ fn make_openai_session(cwd: &Path, base_url: String, api_key: String) -> Session
|
|||
let profile: Arc<dyn AgentProfile> = Arc::new(OpenAiProfile::new(MODEL));
|
||||
let sandbox = Arc::new(LocalSandbox::new(cwd.to_path_buf()));
|
||||
let options = SessionOptions {
|
||||
max_turns: 20,
|
||||
enable_context_compaction: true,
|
||||
compaction_threshold_percent: 80,
|
||||
compaction_preserve_turns: 6,
|
||||
|
|
|
|||
|
|
@ -126,11 +126,7 @@ async fn make_session(
|
|||
profile.register_subagent_tools(manager, factory, 0);
|
||||
|
||||
let profile: Arc<dyn AgentProfile> = Arc::from(profile);
|
||||
let config = SessionOptions {
|
||||
max_turns: 20,
|
||||
..SessionOptions::default()
|
||||
};
|
||||
Session::new(client, profile, env, config, None)
|
||||
Session::new(client, profile, env, SessionOptions::default(), None)
|
||||
}
|
||||
|
||||
async fn make_session_with_config(
|
||||
|
|
@ -315,20 +311,17 @@ async fn openai_compatible_twin_uses_json_edit_file_tool() {
|
|||
"old_string": "old",
|
||||
"new_string": "new"
|
||||
}),
|
||||
)),
|
||||
))
|
||||
.text("Done."),
|
||||
)
|
||||
.load(twin_openai().await)
|
||||
.await;
|
||||
|
||||
let config = SessionOptions {
|
||||
max_turns: 2,
|
||||
..SessionOptions::default()
|
||||
};
|
||||
let mut session = make_openai_compatible_twin_session(
|
||||
ProviderId::new("litellm"),
|
||||
"gpt-5.4-mini",
|
||||
tmp.path(),
|
||||
config,
|
||||
SessionOptions::default(),
|
||||
&twin,
|
||||
);
|
||||
session.initialize().await.unwrap();
|
||||
|
|
@ -767,7 +760,6 @@ macro_rules! reasoning_effort_tests {
|
|||
async fn $test_name() {
|
||||
let tmp = tempfile::tempdir().expect("failed to create tempdir");
|
||||
let config = SessionOptions {
|
||||
max_turns: 20,
|
||||
reasoning_effort: Some(fabro_llm::types::ReasoningEffort::Low),
|
||||
..SessionOptions::default()
|
||||
};
|
||||
|
|
@ -846,7 +838,6 @@ macro_rules! loop_detection_tests {
|
|||
async fn $test_name() {
|
||||
let tmp = tempfile::tempdir().expect("failed to create tempdir");
|
||||
let config = SessionOptions {
|
||||
max_turns: 20,
|
||||
loop_detection_window: 3,
|
||||
..SessionOptions::default()
|
||||
};
|
||||
|
|
|
|||
|
|
@ -90,8 +90,6 @@ const BUDGET_EXHAUSTED_HINTS: &[&str] = &[
|
|||
"context length",
|
||||
"budget",
|
||||
"quota exceeded",
|
||||
"max_turns",
|
||||
"max turns",
|
||||
"max_tokens",
|
||||
"max tokens",
|
||||
"context window exceeded",
|
||||
|
|
@ -1233,7 +1231,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn budget_exhausted_hints_count() {
|
||||
assert_eq!(BUDGET_EXHAUSTED_HINTS.len(), 12);
|
||||
assert_eq!(BUDGET_EXHAUSTED_HINTS.len(), 10);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -1313,22 +1311,6 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_reason_max_turns() {
|
||||
assert_eq!(
|
||||
classify_failure_reason("hit max_turns limit"),
|
||||
FailureCategory::BudgetExhausted
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_reason_max_turns_space() {
|
||||
assert_eq!(
|
||||
classify_failure_reason("max turns reached"),
|
||||
FailureCategory::BudgetExhausted
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_reason_max_tokens() {
|
||||
assert_eq!(
|
||||
|
|
|
|||
|
|
@ -674,12 +674,6 @@ fn event_body_from_event(event: &Event) -> EventBody {
|
|||
AgentEvent::LoopDetected => {
|
||||
EventBody::AgentLoopDetected(fabro_types::AgentLoopDetectedProps { visit: *visit })
|
||||
}
|
||||
AgentEvent::TurnLimitReached { max_turns } => {
|
||||
EventBody::AgentTurnLimitReached(fabro_types::AgentTurnLimitReachedProps {
|
||||
max_turns: *max_turns,
|
||||
visit: *visit,
|
||||
})
|
||||
}
|
||||
AgentEvent::SteeringInjected { text, .. } => {
|
||||
EventBody::AgentSteeringInjected(fabro_types::AgentSteeringInjectedProps {
|
||||
text: text.clone(),
|
||||
|
|
|
|||
|
|
@ -81,7 +81,6 @@ pub fn event_name(event: &Event) -> &'static str {
|
|||
AgentEvent::Error { .. } => "agent.error",
|
||||
AgentEvent::Warning { .. } => "agent.warning",
|
||||
AgentEvent::LoopDetected => "agent.loop.detected",
|
||||
AgentEvent::TurnLimitReached { .. } => "agent.turn.limit",
|
||||
AgentEvent::SteeringInjected { .. } => "agent.steering.injected",
|
||||
AgentEvent::CompactionStarted { .. } => "agent.compaction.started",
|
||||
AgentEvent::CompactionCompleted { .. } => "agent.compaction.completed",
|
||||
|
|
|
|||
|
|
@ -195,12 +195,6 @@ pub struct AgentLoopDetectedProps {
|
|||
pub visit: u32,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub struct AgentTurnLimitReachedProps {
|
||||
pub max_turns: usize,
|
||||
pub visit: u32,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub struct AgentSteeringInjectedProps {
|
||||
pub text: String,
|
||||
|
|
|
|||
|
|
@ -220,8 +220,6 @@ pub enum EventBody {
|
|||
AgentWarning(AgentWarningProps),
|
||||
#[serde(rename = "agent.loop.detected")]
|
||||
AgentLoopDetected(AgentLoopDetectedProps),
|
||||
#[serde(rename = "agent.turn.limit")]
|
||||
AgentTurnLimitReached(AgentTurnLimitReachedProps),
|
||||
#[serde(rename = "agent.steering.injected")]
|
||||
AgentSteeringInjected(AgentSteeringInjectedProps),
|
||||
#[serde(rename = "agent.pair.user_message")]
|
||||
|
|
@ -499,7 +497,6 @@ impl EventBody {
|
|||
Self::AgentError(_) => "agent.error",
|
||||
Self::AgentWarning(_) => "agent.warning",
|
||||
Self::AgentLoopDetected(_) => "agent.loop.detected",
|
||||
Self::AgentTurnLimitReached(_) => "agent.turn.limit",
|
||||
Self::AgentSteeringInjected(_) => "agent.steering.injected",
|
||||
Self::AgentPairUserMessage(_) => "agent.pair.user_message",
|
||||
Self::AgentPairSystemMessage(_) => "agent.pair.system_message",
|
||||
|
|
@ -671,7 +668,6 @@ fn is_known_event_name(event: &str) -> bool {
|
|||
| "agent.error"
|
||||
| "agent.warning"
|
||||
| "agent.loop.detected"
|
||||
| "agent.turn.limit"
|
||||
| "agent.steering.injected"
|
||||
| "agent.pair.user_message"
|
||||
| "agent.pair.system_message"
|
||||
|
|
|
|||
|
|
@ -625,15 +625,34 @@ impl ChatCompletionsRequest {
|
|||
#[derive(Clone, Debug, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct ChatMessage {
|
||||
pub role: String,
|
||||
pub content: Value,
|
||||
pub role: String,
|
||||
pub content: Option<Value>,
|
||||
pub reasoning_content: Option<String>,
|
||||
pub tool_call_id: Option<String>,
|
||||
pub tool_calls: Option<Vec<ChatMessageToolCall>>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct ChatMessageToolCall {
|
||||
pub id: String,
|
||||
#[serde(rename = "type")]
|
||||
pub kind: String,
|
||||
pub function: ChatMessageToolCallFunction,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct ChatMessageToolCallFunction {
|
||||
pub name: String,
|
||||
pub arguments: String,
|
||||
}
|
||||
|
||||
impl ChatMessage {
|
||||
fn extract_texts(&self) -> Vec<String> {
|
||||
match &self.content {
|
||||
Value::String(text) => vec![normalize_whitespace(text)],
|
||||
Value::Array(parts) => parts
|
||||
match self.content.as_ref() {
|
||||
Some(Value::String(text)) => vec![normalize_whitespace(text)],
|
||||
Some(Value::Array(parts)) => parts
|
||||
.iter()
|
||||
.filter_map(|part| {
|
||||
part.get("text")
|
||||
|
|
@ -647,13 +666,21 @@ impl ChatMessage {
|
|||
|
||||
fn contains_reasoning_content(&self) -> bool {
|
||||
self.role == "assistant"
|
||||
&& self.content.as_array().is_some_and(|parts| {
|
||||
parts.iter().any(|part| {
|
||||
part.get("type")
|
||||
.and_then(Value::as_str)
|
||||
.is_some_and(|kind| kind == "reasoning")
|
||||
})
|
||||
})
|
||||
&& (self
|
||||
.reasoning_content
|
||||
.as_deref()
|
||||
.is_some_and(|reasoning| !reasoning.trim().is_empty())
|
||||
|| self
|
||||
.content
|
||||
.as_ref()
|
||||
.and_then(Value::as_array)
|
||||
.is_some_and(|parts| {
|
||||
parts.iter().any(|part| {
|
||||
part.get("type")
|
||||
.and_then(Value::as_str)
|
||||
.is_some_and(|kind| kind == "reasoning")
|
||||
})
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -680,14 +707,24 @@ fn validate_chat_message(message: &ChatMessage) -> Result<(), OpenAiError> {
|
|||
));
|
||||
}
|
||||
|
||||
match &message.content {
|
||||
Value::String(_) => Ok(()),
|
||||
Value::Array(parts) if !parts.is_empty() => {
|
||||
validate_chat_tool_fields(message)?;
|
||||
|
||||
match message.content.as_ref() {
|
||||
Some(Value::String(_)) => Ok(()),
|
||||
Some(Value::Array(parts)) if !parts.is_empty() => {
|
||||
for part in parts {
|
||||
validate_chat_message_part(part, &message.role)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
None if message.role == "assistant"
|
||||
&& message
|
||||
.tool_calls
|
||||
.as_ref()
|
||||
.is_some_and(|tool_calls| !tool_calls.is_empty()) =>
|
||||
{
|
||||
Ok(())
|
||||
}
|
||||
_ => Err(OpenAiError::invalid_request(
|
||||
"messages",
|
||||
"unsupported message content shape",
|
||||
|
|
@ -695,6 +732,56 @@ fn validate_chat_message(message: &ChatMessage) -> Result<(), OpenAiError> {
|
|||
}
|
||||
}
|
||||
|
||||
fn validate_chat_tool_fields(message: &ChatMessage) -> Result<(), OpenAiError> {
|
||||
if message.role == "tool" {
|
||||
if message
|
||||
.tool_call_id
|
||||
.as_deref()
|
||||
.is_none_or(|tool_call_id| tool_call_id.trim().is_empty())
|
||||
{
|
||||
return Err(OpenAiError::invalid_request(
|
||||
"messages",
|
||||
"tool messages require a tool_call_id",
|
||||
));
|
||||
}
|
||||
} else if message.tool_call_id.is_some() {
|
||||
return Err(OpenAiError::invalid_request(
|
||||
"messages",
|
||||
"tool_call_id is only supported on tool messages",
|
||||
));
|
||||
}
|
||||
|
||||
let Some(tool_calls) = &message.tool_calls else {
|
||||
return Ok(());
|
||||
};
|
||||
if message.role != "assistant" {
|
||||
return Err(OpenAiError::invalid_request(
|
||||
"messages",
|
||||
"tool_calls are only supported on assistant messages",
|
||||
));
|
||||
}
|
||||
if tool_calls.is_empty() {
|
||||
return Err(OpenAiError::invalid_request(
|
||||
"messages",
|
||||
"tool_calls must not be empty",
|
||||
));
|
||||
}
|
||||
for tool_call in tool_calls {
|
||||
if tool_call.id.trim().is_empty()
|
||||
|| tool_call.kind != "function"
|
||||
|| tool_call.function.name.trim().is_empty()
|
||||
|| tool_call.function.arguments.trim().is_empty()
|
||||
{
|
||||
return Err(OpenAiError::invalid_request(
|
||||
"messages",
|
||||
"invalid assistant tool call",
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_chat_message_part(part: &Value, role: &str) -> Result<(), OpenAiError> {
|
||||
let Some(object) = part.as_object() else {
|
||||
return Err(OpenAiError::invalid_request(
|
||||
|
|
|
|||
|
|
@ -90,6 +90,44 @@ async fn chat_completions_accepts_supported_openai_compatible_fields() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn chat_completions_accepts_tool_call_history() {
|
||||
let server = common::spawn_server().await.expect("server should start");
|
||||
|
||||
let response = server
|
||||
.post_chat(json!({
|
||||
"model": "gpt-test",
|
||||
"messages": [
|
||||
{ "role": "user", "content": "replace old with new" },
|
||||
{
|
||||
"role": "assistant",
|
||||
"tool_calls": [{
|
||||
"id": "call_edit",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "edit_file",
|
||||
"arguments": "{\"old\":\"old\",\"new\":\"new\"}"
|
||||
}
|
||||
}]
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"content": "Updated data.txt",
|
||||
"tool_call_id": "call_edit"
|
||||
}
|
||||
],
|
||||
"stream": false
|
||||
}))
|
||||
.await;
|
||||
|
||||
assert_eq!(response.status(), 200);
|
||||
let body = response.json::<serde_json::Value>().await.expect("json");
|
||||
assert_eq!(
|
||||
body["choices"][0]["message"]["content"],
|
||||
"deterministic: replace old with new"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn chat_completions_supports_scripted_tool_call_and_json_schema() {
|
||||
let server = common::spawn_server().await.expect("server should start");
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue