Skip to content

Commit 2f2b49f

Browse files
committed
fix(runtime): provenance-gate media markers in the dispatcher + turn-loop result path
The provenance-aware canonicalization helper (`canonicalize_tool_result_media_markers_for`) was previously applied only in the old monolithic `run_tool_call_loop`. After the turn-engine refactor (#7969) the loop's canonicalization moved to `turn::results_collect`, and the `ToolDispatcher::format_results` path (XmlToolDispatcher / NativeToolDispatcher) still used the provenance-blind `canonicalize_tool_result_media_markers`. Route every tool-name-aware canonicalization site through the single shared helper so a search/listing tool (content_search, glob_search) that merely lists a local image path is never rewritten into a routable `[IMAGE:…]` marker — which falsely triggers vision routing and a provider-capability error on a text-only provider — while a genuine image-producing tool (image_gen) is still canonicalized. - turn::results_collect::collect_tool_results: canonicalize via `_for(tool_name)` - XmlToolDispatcher::format_results: canonicalize via `_for(result.name)` - NativeToolDispatcher::format_results: canonicalize via `_for(result.name)` Adds dispatcher regression tests asserting content_search/glob_search image paths are not promoted in either dispatcher, and that image_gen output still is. See PR #7345.
1 parent 8fa7022 commit 2f2b49f

3 files changed

Lines changed: 157 additions & 11 deletions

File tree

crates/zeroclaw-runtime/src/agent/dispatcher.rs

Lines changed: 142 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use super::history::canonicalize_tool_result_media_markers;
1+
use super::history::{
2+
canonicalize_tool_result_media_markers, canonicalize_tool_result_media_markers_for,
3+
};
24
use crate::tools::{Tool, ToolSpec};
35
use serde_json::Value;
46
use std::fmt::Write;
@@ -129,7 +131,12 @@ impl ToolDispatcher for XmlToolDispatcher {
129131
let mut content = String::new();
130132
for result in results {
131133
let status = if result.success { "ok" } else { "error" };
132-
let output = canonicalize_tool_result_media_markers(&result.output);
134+
// Provenance-gated: search/listing tools (content_search,
135+
// glob_search) must not have incidental image paths promoted to
136+
// routable [IMAGE:...] markers (PR #7345). The producing tool name is
137+
// known here, so canonicalize through the same shared helper the
138+
// turn loop uses.
139+
let output = canonicalize_tool_result_media_markers_for(&result.name, &result.output);
133140
let _ = writeln!(
134141
content,
135142
"<tool_result name=\"{}\" status=\"{}\">\n{}\n</tool_result>",
@@ -212,7 +219,8 @@ impl ToolDispatcher for NativeToolDispatcher {
212219
.tool_call_id
213220
.clone()
214221
.unwrap_or_else(|| "unknown".to_string()),
215-
content: canonicalize_tool_result_media_markers(&result.output),
222+
// Provenance-gated (PR #7345): see the XML dispatcher above.
223+
content: canonicalize_tool_result_media_markers_for(&result.name, &result.output),
216224
})
217225
.collect();
218226
ConversationMessage::ToolResults(messages)
@@ -386,6 +394,137 @@ mod tests {
386394
}
387395
}
388396

397+
// ═══════════════════════════════════════════════════════════════════════
398+
// provenance-gated media-marker canonicalization (PR #7345)
399+
// ═══════════════════════════════════════════════════════════════════════
400+
// The dispatcher result-formatting path is reachable from `Agent::turn`
401+
// / `Agent::turn_streamed` (ACP, gateway WebSocket + RPC). A search/listing
402+
// tool that merely *lists* a local image path must NOT have that path
403+
// rewritten into a routable `[IMAGE:...]` marker - otherwise it falsely
404+
// triggers vision routing and a provider-capability error on a text-only
405+
// provider. A genuine image-producing tool (e.g. `image_gen`) MUST still be
406+
// canonicalized. Both dispatchers gate via the shared
407+
// `canonicalize_tool_result_media_markers_for(tool_name, ...)` helper.
408+
409+
/// Write a throwaway PNG and return its absolute path string. An existing
410+
/// local image path is required for canonicalization to fire at all.
411+
fn write_temp_image(dir: &std::path::Path, name: &str) -> String {
412+
let image = dir.join(name);
413+
std::fs::write(&image, [0x89, b'P', b'N', b'G', b'\r', b'\n', 0x1a, b'\n']).unwrap();
414+
image.display().to_string()
415+
}
416+
417+
fn xml_format_results_text(
418+
dispatcher: &XmlToolDispatcher,
419+
result: ToolExecutionResult,
420+
) -> String {
421+
match dispatcher.format_results(&[result]) {
422+
ConversationMessage::Chat(chat) => chat.content,
423+
_ => panic!("XmlToolDispatcher::format_results must return a Chat message"),
424+
}
425+
}
426+
427+
fn native_format_results_content(
428+
dispatcher: &NativeToolDispatcher,
429+
result: ToolExecutionResult,
430+
) -> String {
431+
match dispatcher.format_results(&[result]) {
432+
ConversationMessage::ToolResults(results) => results[0].content.clone(),
433+
_ => panic!("NativeToolDispatcher::format_results must return ToolResults"),
434+
}
435+
}
436+
437+
#[test]
438+
fn xml_format_results_does_not_promote_search_tool_image_paths() {
439+
let dir = tempfile::tempdir().unwrap();
440+
let path = write_temp_image(dir.path(), "hit.png");
441+
let xml = XmlToolDispatcher;
442+
443+
for tool in ["content_search", "glob_search"] {
444+
let rendered = xml_format_results_text(
445+
&xml,
446+
ToolExecutionResult {
447+
name: tool.into(),
448+
output: format!("match: {path}"),
449+
success: true,
450+
tool_call_id: None,
451+
},
452+
);
453+
assert!(
454+
!rendered.contains("[IMAGE:"),
455+
"{tool} output must not be promoted to an image marker"
456+
);
457+
assert!(
458+
rendered.contains(&path),
459+
"{tool} output must still carry the literal path text"
460+
);
461+
}
462+
}
463+
464+
#[test]
465+
fn native_format_results_does_not_promote_search_tool_image_paths() {
466+
let dir = tempfile::tempdir().unwrap();
467+
let path = write_temp_image(dir.path(), "hit.png");
468+
let native = NativeToolDispatcher;
469+
470+
for tool in ["content_search", "glob_search"] {
471+
let content = native_format_results_content(
472+
&native,
473+
ToolExecutionResult {
474+
name: tool.into(),
475+
output: format!("found: {path}"),
476+
success: true,
477+
tool_call_id: Some("tc1".into()),
478+
},
479+
);
480+
assert!(
481+
!content.contains("[IMAGE:"),
482+
"{tool} output must not be promoted to an image marker"
483+
);
484+
assert!(content.contains(&path));
485+
}
486+
}
487+
488+
#[test]
489+
fn format_results_still_promotes_image_producing_tool_paths() {
490+
// Default-allow: a genuinely image-producing tool keeps canonicalization
491+
// in BOTH dispatchers, so real tool-produced images still route to a
492+
// vision provider.
493+
let dir = tempfile::tempdir().unwrap();
494+
let path = write_temp_image(dir.path(), "generated.png");
495+
let expected = format!("[IMAGE:{path}]");
496+
497+
let xml = XmlToolDispatcher;
498+
let rendered = xml_format_results_text(
499+
&xml,
500+
ToolExecutionResult {
501+
name: "image_gen".into(),
502+
output: format!("saved to {path}"),
503+
success: true,
504+
tool_call_id: None,
505+
},
506+
);
507+
assert!(
508+
rendered.contains(&expected),
509+
"image_gen output must be canonicalized into a marker (XML)"
510+
);
511+
512+
let native = NativeToolDispatcher;
513+
let content = native_format_results_content(
514+
&native,
515+
ToolExecutionResult {
516+
name: "image_gen".into(),
517+
output: format!("saved to {path}"),
518+
success: true,
519+
tool_call_id: Some("tc1".into()),
520+
},
521+
);
522+
assert!(
523+
content.contains(&expected),
524+
"image_gen output must be canonicalized into a marker (native)"
525+
);
526+
}
527+
389528
// ═══════════════════════════════════════════════════════════════════════
390529
// reasoning_content pass-through tests
391530
// ═══════════════════════════════════════════════════════════════════════

crates/zeroclaw-runtime/src/agent/history.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -230,15 +230,15 @@ pub fn canonicalize_tool_result_media_markers(output: &str) -> String {
230230
/// Tools whose output merely *lists* or *quotes* local filesystem paths
231231
/// (search hits, glob matches) rather than presenting an image as visual
232232
/// content. Their incidental image-file paths must NOT be auto-promoted to
233-
/// `[IMAGE:]` markers: the agent loop counts the current iteration's
233+
/// `[IMAGE:...]` markers: the agent loop counts the current iteration's
234234
/// tool-result markers (`multimodal::count_image_markers`) when deciding
235235
/// whether to switch to a vision provider, so a path echo here falsely
236-
/// triggers vision routing producing a provider-capability error on a
236+
/// triggers vision routing - producing a provider-capability error on a
237237
/// text-only provider. See PR #7345.
238238
///
239-
/// This is a denylist (default-allow): any other tool including ones that
239+
/// This is a denylist (default-allow): any other tool - including ones that
240240
/// genuinely *generate* or *fetch* an image and print its path (e.g.
241-
/// `image_gen`, `file_download`) keeps canonicalization, so real
241+
/// `image_gen`, `file_download`) - keeps canonicalization, so real
242242
/// tool-produced images still route to a configured vision provider.
243243
fn is_path_listing_tool(tool_name: &str) -> bool {
244244
matches!(
@@ -250,7 +250,7 @@ fn is_path_listing_tool(tool_name: &str) -> bool {
250250
/// Provenance-aware wrapper around [`canonicalize_tool_result_media_markers`].
251251
///
252252
/// Returns the output unchanged for path-listing tools ([`is_path_listing_tool`])
253-
/// so their incidental image paths never become routable `[IMAGE:]` markers;
253+
/// so their incidental image paths never become routable `[IMAGE:...]` markers;
254254
/// all other tools are canonicalized exactly as before.
255255
pub fn canonicalize_tool_result_media_markers_for(tool_name: &str, output: &str) -> String {
256256
if is_path_listing_tool(tool_name) {
@@ -567,7 +567,7 @@ mod tests {
567567
#[test]
568568
fn canonicalize_for_skips_path_listing_tools() {
569569
// A search/listing tool that surfaces a real image path must be left
570-
// untouched promoting it to [IMAGE:] would falsely trigger vision
570+
// untouched - promoting it to [IMAGE:...] would falsely trigger vision
571571
// routing (PR #7345).
572572
let dir = tempfile::tempdir().unwrap();
573573
let image = dir.path().join("hit.png");

crates/zeroclaw-runtime/src/agent/turn/results_collect.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
//! identical-output abort.
44
55
use crate::agent::history::{
6-
append_or_merge_system_message, canonicalize_tool_result_media_markers, truncate_tool_result,
6+
append_or_merge_system_message, canonicalize_tool_result_media_markers_for,
7+
truncate_tool_result,
78
};
89
use crate::agent::loop_detector::LoopDetector;
910
use crate::agent::tool_execution::ToolExecutionOutcome;
@@ -112,7 +113,13 @@ pub(crate) fn collect_tool_results(
112113
}
113114
}
114115
}
115-
let canonical_output = canonicalize_tool_result_media_markers(&outcome.output);
116+
// Provenance-gated: search/listing tools (content_search, glob_search)
117+
// must not have incidental image paths promoted to routable [IMAGE:...]
118+
// markers, or they falsely trigger vision routing on a text-only
119+
// provider. Image-producing/fetching tools keep canonicalization.
120+
// See PR #7345.
121+
let canonical_output =
122+
canonicalize_tool_result_media_markers_for(&tool_name, &outcome.output);
116123
let mut result_output = truncate_tool_result(&canonical_output, max_tool_result_chars);
117124
// Append HMAC receipt to tool result when receipts are enabled
118125
if let Some(ref receipt) = outcome.receipt {

0 commit comments

Comments
 (0)