Skip to content

Commit fabb03e

Browse files
committed
Enhance loop recovery hints and streamline error handling in tool execution
1 parent 2602a7f commit fabb03e

File tree

5 files changed

+222
-18
lines changed

5 files changed

+222
-18
lines changed

src/agent/runloop/tool_output/mod.rs

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,28 @@ use mcp::{
3535
use streams::render_stream_section;
3636
use styles::{GitStyles, LsStyles};
3737

38+
fn tool_recovery_hint(val: &Value) -> Option<&'static str> {
39+
if !val
40+
.get("loop_detected")
41+
.and_then(Value::as_bool)
42+
.unwrap_or(false)
43+
{
44+
return None;
45+
}
46+
if val.get("spool_path").and_then(Value::as_str).is_some() {
47+
return Some("Loop detected; continue from spooled output.");
48+
}
49+
if val.get("fallback_tool").and_then(Value::as_str).is_some() {
50+
return Some("Loop detected; fallback is available.");
51+
}
52+
Some("Loop detected; change approach before retrying.")
53+
}
54+
3855
fn tool_follow_up_hints(val: &Value) -> Vec<String> {
39-
let mut hints = Vec::with_capacity(2);
56+
let mut hints = Vec::with_capacity(3);
57+
if let Some(hint) = tool_recovery_hint(val) {
58+
hints.push(hint.to_string());
59+
}
4060
if let Some(path) = val.get("spool_path").and_then(Value::as_str) {
4161
hints.push(format!(
4262
"Large output was spooled to \"{}\". Use read_file/grep_file to inspect details.",
@@ -759,6 +779,81 @@ mod tests {
759779
assert!(inline_output.contains("Large output was spooled to"));
760780
}
761781

782+
#[tokio::test]
783+
async fn render_tool_output_renders_loop_recovery_hint_from_structured_fields() {
784+
let (sender, mut receiver) = tokio::sync::mpsc::unbounded_channel();
785+
let mut renderer =
786+
AnsiRenderer::with_inline_ui(InlineHandle::new_for_tests(sender), Default::default());
787+
let payload = json!({
788+
"loop_detected": true,
789+
"fallback_tool": vtcode_core::config::constants::tools::UNIFIED_SEARCH
790+
});
791+
792+
render_tool_output(
793+
&mut renderer,
794+
Some(vtcode_core::config::constants::tools::UNIFIED_SEARCH),
795+
&payload,
796+
None,
797+
)
798+
.await
799+
.expect("loop recovery hint payload should render");
800+
801+
let inline_output = collect_inline_output(&mut receiver);
802+
assert!(inline_output.contains("Loop detected; fallback is available."));
803+
}
804+
805+
#[tokio::test]
806+
async fn render_tool_output_renders_spooled_loop_recovery_hint() {
807+
let (sender, mut receiver) = tokio::sync::mpsc::unbounded_channel();
808+
let mut renderer =
809+
AnsiRenderer::with_inline_ui(InlineHandle::new_for_tests(sender), Default::default());
810+
let payload = json!({
811+
"loop_detected": true,
812+
"spool_path": ".vtcode/context/tool_outputs/readme.txt",
813+
"next_read_args": {
814+
"path": ".vtcode/context/tool_outputs/readme.txt",
815+
"offset": 81,
816+
"limit": 40
817+
}
818+
});
819+
820+
render_tool_output(
821+
&mut renderer,
822+
Some(vtcode_core::config::constants::tools::READ_FILE),
823+
&payload,
824+
None,
825+
)
826+
.await
827+
.expect("spooled loop recovery hint payload should render");
828+
829+
let inline_output = collect_inline_output(&mut receiver);
830+
assert!(inline_output.contains("Loop detected; continue from spooled output."));
831+
}
832+
833+
#[tokio::test]
834+
async fn render_tool_output_does_not_duplicate_loop_recovery_hint() {
835+
let (sender, mut receiver) = tokio::sync::mpsc::unbounded_channel();
836+
let mut renderer =
837+
AnsiRenderer::with_inline_ui(InlineHandle::new_for_tests(sender), Default::default());
838+
let payload = json!({
839+
"loop_detected": true,
840+
"fallback_tool": vtcode_core::config::constants::tools::UNIFIED_SEARCH,
841+
"output": "Loop detected; fallback is available."
842+
});
843+
844+
render_tool_output(&mut renderer, Some("custom_tool"), &payload, None)
845+
.await
846+
.expect("duplicate hint payload should render");
847+
848+
let inline_output = collect_inline_output(&mut receiver);
849+
assert_eq!(
850+
inline_output
851+
.matches("Loop detected; fallback is available.")
852+
.count(),
853+
1
854+
);
855+
}
856+
762857
#[test]
763858
fn tracker_summary_lines_include_progress_and_active_items() {
764859
let payload = json!({

src/agent/runloop/unified/turn/tool_outcomes/execution_result.rs

Lines changed: 100 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ pub(crate) fn build_error_content(
283283
inline_full_args,
284284
);
285285
}
286-
payload
286+
compact_model_tool_payload(payload)
287287
} else {
288288
let mut payload = serde_json::json!({
289289
"error": error_text,
@@ -293,7 +293,7 @@ pub(crate) fn build_error_content(
293293
"next_action": next_action,
294294
});
295295
push_error_truncation_flag(&mut payload, error_truncated);
296-
payload
296+
compact_model_tool_payload(payload)
297297
}
298298
}
299299

@@ -492,9 +492,9 @@ pub(crate) async fn handle_tool_execution_result<'a>(
492492
Ok(None)
493493
}
494494

495-
pub(super) fn maybe_inline_spooled(_tool_name: &str, output: &serde_json::Value) -> String {
495+
pub(crate) fn compact_model_tool_payload(output: serde_json::Value) -> serde_json::Value {
496496
let Some(obj) = output.as_object() else {
497-
return serialize_output(output);
497+
return output;
498498
};
499499

500500
let next_continue_args = obj
@@ -510,6 +510,10 @@ pub(super) fn maybe_inline_spooled(_tool_name: &str, output: &serde_json::Value)
510510
.map(|next_continue| next_continue.session_id.as_str())
511511
});
512512
let has_spool_path = obj.contains_key("spool_path");
513+
let loop_detected = obj
514+
.get("loop_detected")
515+
.and_then(serde_json::Value::as_bool)
516+
.unwrap_or(false);
513517
let is_exec_like = obj.contains_key("command")
514518
|| obj.contains_key("working_directory")
515519
|| session_id.is_some()
@@ -538,26 +542,35 @@ pub(super) fn maybe_inline_spooled(_tool_name: &str, output: &serde_json::Value)
538542
"spool_hint" | "spooled_bytes" | "spooled_to_file" => has_spool_path,
539543
"id" => session_id.is_some_and(|sid| value == sid),
540544
"working_directory" => is_exec_like,
545+
"next_action" => true,
541546
"has_more" | "preferred_next_action" => {
542547
next_continue_args.is_some() || next_read_args.is_some() || is_false_bool(value)
543548
}
544549
"session_id" | "command" => is_exec_like,
545-
"spool_path" => next_read_args
546-
.as_ref()
547-
.is_some_and(|next_read| value == next_read.path.as_str()),
550+
"spool_path" => {
551+
!loop_detected
552+
&& next_read_args
553+
.as_ref()
554+
.is_some_and(|next_read| value == next_read.path.as_str())
555+
}
548556
"next_offset" => next_read_args
549557
.as_ref()
550558
.is_some_and(|next_read| value_matches_usize(value, next_read.offset)),
551559
"chunk_limit" => next_read_args
552560
.as_ref()
553561
.is_some_and(|next_read| value_matches_usize(value, next_read.limit)),
554562
"stderr_preview" => has_stderr,
555-
"truncated"
556-
| "auto_recovered"
563+
"loop_detected_note"
564+
| "spool_ref_only"
565+
| "result_ref_only"
557566
| "reused_spooled_output"
558567
| "reused_recent_result"
559-
| "loop_detected"
560-
| "query_truncated" => is_false_bool(value),
568+
| "repeat_count"
569+
| "tool" => loop_detected,
570+
"limit" => loop_detected && obj.get("tool").is_some(),
571+
"truncated" | "auto_recovered" | "loop_detected" | "query_truncated" => {
572+
is_false_bool(value)
573+
}
561574
"stdout" => output_value.is_some_and(|output| output == value),
562575
"process_id" => is_exec_like || process_session_id.is_some_and(|sid| value == sid),
563576
"is_exited" => {
@@ -581,7 +594,11 @@ pub(super) fn maybe_inline_spooled(_tool_name: &str, output: &serde_json::Value)
581594
sanitized.insert(key.clone(), cloned_value);
582595
}
583596

584-
serialize_output(&serde_json::Value::Object(sanitized))
597+
serde_json::Value::Object(sanitized)
598+
}
599+
600+
pub(super) fn maybe_inline_spooled(_tool_name: &str, output: &serde_json::Value) -> String {
601+
serialize_output(&compact_model_tool_payload(output.clone()))
585602
}
586603

587604
fn compact_next_continue_args(value: &serde_json::Value) -> serde_json::Value {
@@ -1118,6 +1135,31 @@ mod tests {
11181135
.and_then(|v| v.as_str())
11191136
.is_some()
11201137
);
1138+
assert!(payload.get("next_action").is_none());
1139+
}
1140+
1141+
#[test]
1142+
fn build_error_content_keeps_structured_fallback_fields_only() {
1143+
let payload = build_error_content(
1144+
"boom".to_string(),
1145+
Some(tool_names::UNIFIED_SEARCH.to_string()),
1146+
Some(serde_json::json!({"action":"list","path":"."})),
1147+
"execution",
1148+
);
1149+
1150+
assert_eq!(
1151+
payload.get("fallback_tool"),
1152+
Some(&serde_json::json!(tool_names::UNIFIED_SEARCH))
1153+
);
1154+
assert_eq!(
1155+
payload.get("fallback_tool_args"),
1156+
Some(&serde_json::json!({"action":"list","path":"."}))
1157+
);
1158+
assert_eq!(
1159+
payload.get("is_recoverable").and_then(|v| v.as_bool()),
1160+
Some(true)
1161+
);
1162+
assert!(payload.get("next_action").is_none());
11211163
}
11221164

11231165
#[test]
@@ -1257,6 +1299,52 @@ mod tests {
12571299
);
12581300
}
12591301

1302+
#[test]
1303+
fn maybe_inline_spooled_keeps_loop_recovery_fields_and_drops_notes() {
1304+
let serialized = maybe_inline_spooled(
1305+
tool_names::READ_FILE,
1306+
&serde_json::json!({
1307+
"loop_detected": true,
1308+
"spool_path": ".vtcode/context/tool_outputs/unified_exec_loop.txt",
1309+
"next_read_args": {
1310+
"path": ".vtcode/context/tool_outputs/unified_exec_loop.txt",
1311+
"offset": 81,
1312+
"limit": 40
1313+
},
1314+
"reused_spooled_output": true,
1315+
"spool_ref_only": true,
1316+
"loop_detected_note": "Read the spool file instead of re-running this call.",
1317+
"repeat_count": 4,
1318+
"limit": 3,
1319+
"tool": "read_file"
1320+
}),
1321+
);
1322+
1323+
let parsed: serde_json::Value =
1324+
serde_json::from_str(&serialized).expect("serialized JSON payload");
1325+
assert_eq!(parsed.get("loop_detected"), Some(&serde_json::json!(true)));
1326+
assert_eq!(
1327+
parsed.get("spool_path"),
1328+
Some(&serde_json::json!(
1329+
".vtcode/context/tool_outputs/unified_exec_loop.txt"
1330+
))
1331+
);
1332+
assert_eq!(
1333+
parsed.get("next_read_args"),
1334+
Some(&serde_json::json!({
1335+
"path": ".vtcode/context/tool_outputs/unified_exec_loop.txt",
1336+
"offset": 81,
1337+
"limit": 40
1338+
}))
1339+
);
1340+
assert!(parsed.get("reused_spooled_output").is_none());
1341+
assert!(parsed.get("spool_ref_only").is_none());
1342+
assert!(parsed.get("loop_detected_note").is_none());
1343+
assert!(parsed.get("repeat_count").is_none());
1344+
assert!(parsed.get("limit").is_none());
1345+
assert!(parsed.get("tool").is_none());
1346+
}
1347+
12601348
#[test]
12611349
fn maybe_inline_spooled_drops_terminal_exec_metadata_without_continuation() {
12621350
let serialized = maybe_inline_spooled(

src/agent/runloop/unified/turn/tool_outcomes/handlers/fallbacks.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use vtcode_core::tools::registry::ToolPreflightOutcome;
55
use vtcode_core::tools::tool_intent;
66

77
use crate::agent::runloop::unified::turn::context::TurnProcessingContext;
8+
use crate::agent::runloop::unified::turn::tool_outcomes::execution_result::compact_model_tool_payload;
89

910
pub(super) fn recovery_fallback_for_tool(tool_name: &str, args: &Value) -> Option<(String, Value)> {
1011
match tool_name {
@@ -64,6 +65,7 @@ pub(super) fn build_validation_error_content_with_fallback(
6465
fallback_tool_args: Option<Value>,
6566
) -> String {
6667
let is_recoverable = fallback_tool.is_some();
68+
let loop_detected = validation_stage == "loop_detection";
6769
let next_action = if is_recoverable {
6870
"Retry with fallback_tool_args."
6971
} else {
@@ -76,6 +78,7 @@ pub(super) fn build_validation_error_content_with_fallback(
7678
"validation_stage": validation_stage,
7779
"retryable": false,
7880
"is_recoverable": is_recoverable,
81+
"loop_detected": loop_detected,
7982
"next_action": next_action,
8083
});
8184
if let Some(obj) = payload.as_object_mut() {
@@ -86,7 +89,7 @@ pub(super) fn build_validation_error_content_with_fallback(
8689
obj.insert("fallback_tool_args".to_string(), args);
8790
}
8891
}
89-
payload.to_string()
92+
compact_model_tool_payload(payload).to_string()
9093
}
9194

9295
pub(super) fn preflight_validation_fallback(

src/agent/runloop/unified/turn/tool_outcomes/handlers/tests.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,24 @@ fn validation_error_payload_includes_fallback_metadata() {
651651
assert_eq!(parsed["is_recoverable"], true);
652652
assert_eq!(parsed["fallback_tool"], tool_names::UNIFIED_SEARCH);
653653
assert_eq!(parsed["fallback_tool_args"]["action"], "grep");
654+
assert!(parsed.get("next_action").is_none());
655+
assert!(parsed.get("loop_detected").is_none());
656+
}
657+
658+
#[test]
659+
fn validation_error_payload_marks_loop_detection_without_prose_hint() {
660+
let payload = build_validation_error_content_with_fallback(
661+
"Tool 'read_file' is blocked due to excessive repetition (Loop Detected).".to_string(),
662+
"loop_detection",
663+
Some(tool_names::UNIFIED_SEARCH.to_string()),
664+
Some(json!({"action":"list","path":"."})),
665+
);
666+
let parsed: serde_json::Value =
667+
serde_json::from_str(&payload).expect("validation payload should be json");
668+
assert_eq!(parsed.get("loop_detected"), Some(&json!(true)));
669+
assert_eq!(parsed["fallback_tool"], tool_names::UNIFIED_SEARCH);
670+
assert_eq!(parsed["fallback_tool_args"]["action"], "list");
671+
assert!(parsed.get("next_action").is_none());
654672
}
655673

656674
#[test]

vtcode.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ read_file = true
5959
# Environment variable that stores the API key for the active provider
6060
# Default: "OPENAI_API_KEY"
6161
# Type: `string`
62-
api_key_env = "OPENROUTER_API_KEY"
62+
api_key_env = "MINIMAX_API_KEY"
6363

6464
# Enable autonomous mode - auto-approve safe tools with reduced HITL prompts When true, the agent
6565
# operates with fewer confirmation prompts for safe tools.
@@ -86,7 +86,7 @@ default_editing_mode = "edit"
8686
# Default model to use
8787
# Default: "gpt-5.3-codex"
8888
# Type: `string`
89-
default_model = "nvidia/nemotron-3-super-120b-a12b:free"
89+
default_model = "MiniMax-M2.5"
9090

9191
# Enable an extra self-review pass to refine final responses
9292
# Default: false
@@ -148,7 +148,7 @@ project_doc_max_bytes = 16384
148148
# Possible values: anthropic, gemini, openai, openrouter, zai
149149
# Default: "openai"
150150
# Type: `string`
151-
provider = "openrouter"
151+
provider = "minimax"
152152

153153
# Reasoning effort level for models that support it (none, minimal, low, medium, high, xhigh) Applies
154154
# to: Claude, GPT-5 family, Gemini, Qwen3, DeepSeek with reasoning capability
@@ -209,7 +209,7 @@ temporal_context_use_utc = false
209209
# UI theme identifier controlling ANSI styling
210210
# Default: "ciapre"
211211
# Type: `string`
212-
theme = "ciapre"
212+
theme = "ansi-classic"
213213

214214
# Enable TODO planning helper mode for structured task management
215215
# Default: true

0 commit comments

Comments
 (0)