Skip to content

Commit dc2f2fc

Browse files
yh928user
andauthored
feat(message): carry a tool result's trusted-verbatim request into the transcript (#77)
* feat(message): carry a tool result's trusted-verbatim request into the transcript A `ToolResult` can hold structured metadata in `raw`, but folding it into the transcript goes through `Message::tool(call_id, content)`, which keeps only those two fields. Everything else is dropped at that boundary, so a host has no way to learn anything the tool wanted to say about its own output. That matters for one property in particular: whether the content may be reshaped. Hosts summarise, truncate, batch, and re-frame tool output, and usually should — but some results are only useful intact. An input schema whose argument names the model must copy exactly, a signature, a diff: rewriting those produces content that still reads well and is wrong. Today a host can only guess from the text, and guessing wrong is silent. Add an explicit, opt-in request: - `ToolResult::mark_trusted_verbatim()` / `is_trusted_verbatim()`, backed by `raw` under `TRUSTED_VERBATIM_KEY`, so the opt-in costs nothing for results that never use it and round-trips through a host's own serialization. - `ToolMessage.trusted_verbatim: bool`, `#[serde(default)]` so transcripts persisted before this field still deserialize (and read as not-trusted). - `Message::tool_from_result(&ToolResult)`, which carries the flag across; the agent-loop fold site now uses it. The flag is advisory: the crate transports it, honouring it is the host's job. `Message::tool` is unchanged and still sets `false`, so nothing moves for callers that do not opt in. Tests cover the opt-in being off by default, idempotent marking that preserves unrelated `raw` metadata, non-boolean or non-object `raw` never tripping it, `tool_from_result` matching `tool` for an unmarked result, and a pre-field transcript still loading. Verified: `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, `cargo build --all-targets`, `cargo test` (1205 lib tests + suites), and `cargo run --example basic_graph`. * fix(tool): report a refused trusted-verbatim request; omit the unset flag Both review bots flagged the same defect, and they were right: when `raw` already held a non-object value, `mark_trusted_verbatim` did nothing at all. `get_or_insert_with` returns the existing scalar, `as_object_mut()` gives `None`, and the insert was skipped — with no return value, no panic, and no diagnostic. `is_trusted_verbatim()` then kept answering `false` while the tool author believed the opt-in had taken effect. The flag lives under a key in `raw`, so a scalar genuinely has nowhere to put it, and clobbering a value the tool deliberately set would be worse. So the request is now *refused and reported* rather than silently lost: `mark_trusted_verbatim` returns `bool`, and is `#[must_use]` precisely because the failure is otherwise invisible. A tool that wants both can move its value under a key of its own and call again. Second finding: the field serialized as `"trusted_verbatim":false` on every tool message, which contradicts this change's own "costs nothing unless you opt in" claim and diverges from the rest of the crate. Added `skip_serializing_if`, reusing the `is_false` helper convention already used by `graph::export::types` rather than an inline `std::ops::Not::not`. Tests: a new regression case walks a string, an array, and a number `raw`, asserting each refuses, leaves the tool's own value untouched, and that the call succeeds once room is made; a new wire test pins that an unset flag is omitted and a set one is present. The existing tests now assert the return value instead of discarding it — `#[must_use]` caught all three call sites. Verified: fmt, clippy (both feature sets), build (both), `cargo test` (1207), `cargo test --all-features`, and `cargo run --example basic_graph`. --------- Co-authored-by: user <user@localhost.localdomain>
1 parent d591107 commit dc2f2fc

6 files changed

Lines changed: 278 additions & 4 deletions

File tree

‎src/harness/agent_loop/tools.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,10 @@ impl<State: Send + Sync, Ctx: Send + Sync> AgentHarness<State, Ctx> {
369369
});
370370
status.set_last_event(record.id);
371371

372-
messages.push(Message::tool(
373-
result.call_id.clone(),
374-
result.content.clone(),
375-
));
372+
// `tool_from_result`, not `tool`: a result that asked to reach the model
373+
// byte-for-byte must carry that request into the transcript, or the host
374+
// has no way to tell it apart from output it may freely reshape.
375+
messages.push(Message::tool_from_result(&result));
376376
Ok(())
377377
}
378378

‎src/harness/message/mod.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,30 @@ impl Message {
125125
}
126126

127127
/// Creates a tool result message for the given tool call id.
128+
///
129+
/// Leaves [`ToolMessage::trusted_verbatim`] unset. Use
130+
/// [`Message::tool_from_result`] to fold a real
131+
/// [`crate::harness::tool::ToolResult`], which carries the flag across.
128132
pub fn tool(tool_call_id: impl Into<String>, content: impl Into<String>) -> Self {
129133
Message::Tool(ToolMessage {
130134
tool_call_id: tool_call_id.into(),
131135
content: vec![ContentBlock::Text(content.into())],
136+
trusted_verbatim: false,
137+
})
138+
}
139+
140+
/// Folds a [`crate::harness::tool::ToolResult`] into the transcript message
141+
/// that answers its call.
142+
///
143+
/// Preferred over [`Message::tool`] on the agent-loop path: a `ToolResult`
144+
/// carries structured metadata the message shape would otherwise drop, and
145+
/// [`ToolMessage::trusted_verbatim`] is the part a host must not lose — it
146+
/// is what tells the host this content may not be reshaped.
147+
pub fn tool_from_result(result: &crate::harness::tool::ToolResult) -> Self {
148+
Message::Tool(ToolMessage {
149+
tool_call_id: result.call_id.clone(),
150+
content: vec![ContentBlock::Text(result.content.clone())],
151+
trusted_verbatim: result.is_trusted_verbatim(),
132152
})
133153
}
134154

‎src/harness/message/test.rs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,72 @@ fn legacy_content_without_thinking_still_parses() {
208208
assert_eq!(blocks[0].as_text(), Some("hello"));
209209
assert!(!blocks[1].is_reasoning());
210210
}
211+
212+
#[test]
213+
fn tool_from_result_carries_trusted_verbatim_and_tool_does_not() {
214+
use crate::harness::tool::ToolResult;
215+
216+
let mut result = ToolResult {
217+
call_id: "call-7".into(),
218+
name: "read_schema".into(),
219+
content: "verbatim body".into(),
220+
raw: None,
221+
error: None,
222+
elapsed_ms: 0,
223+
};
224+
225+
// The plain constructor never claims the flag — nothing changes for the
226+
// callers that do not opt in.
227+
let Message::Tool(plain) = Message::tool("call-7", "verbatim body") else {
228+
panic!("expected a tool message");
229+
};
230+
assert!(!plain.trusted_verbatim);
231+
232+
// Folding an unmarked result matches the plain constructor.
233+
let Message::Tool(unmarked) = Message::tool_from_result(&result) else {
234+
panic!("expected a tool message");
235+
};
236+
assert!(!unmarked.trusted_verbatim);
237+
assert_eq!(unmarked, plain);
238+
239+
// Folding a marked result carries the request across, which is the whole
240+
// point: without it the host cannot tell this content apart from output it
241+
// may freely summarise or re-frame.
242+
assert!(result.mark_trusted_verbatim());
243+
let Message::Tool(marked) = Message::tool_from_result(&result) else {
244+
panic!("expected a tool message");
245+
};
246+
assert!(marked.trusted_verbatim);
247+
assert_eq!(marked.tool_call_id, "call-7");
248+
assert_eq!(marked.content, plain.content);
249+
}
250+
251+
#[test]
252+
fn a_transcript_stored_before_the_flag_existed_still_deserializes() {
253+
// Additive field: `#[serde(default)]` keeps older persisted transcripts
254+
// loadable, and they read as not-trusted rather than failing.
255+
let legacy = json!({ "tool_call_id": "call-1", "content": [{ "text": "ok" }] });
256+
let msg: ToolMessage = serde_json::from_value(legacy).unwrap();
257+
assert_eq!(msg.tool_call_id, "call-1");
258+
assert!(!msg.trusted_verbatim);
259+
}
260+
261+
#[test]
262+
fn an_unset_flag_is_omitted_from_the_wire() {
263+
// The opt-in must cost nothing for the messages that never use it: a tool
264+
// message without the flag serializes exactly as it did before the field
265+
// existed, matching how the rest of the crate omits unset fields.
266+
let plain = Message::tool("call-1", "ok");
267+
let wire = serde_json::to_value(&plain).unwrap();
268+
assert_eq!(
269+
wire,
270+
json!({ "tool": { "tool_call_id": "call-1", "content": [{ "text": "ok" }] } })
271+
);
272+
273+
let Message::Tool(mut msg) = plain else {
274+
panic!("expected a tool message");
275+
};
276+
msg.trusted_verbatim = true;
277+
let wire = serde_json::to_value(Message::Tool(msg)).unwrap();
278+
assert_eq!(wire["tool"]["trusted_verbatim"], json!(true));
279+
}

‎src/harness/message/types.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,26 @@ pub struct ToolMessage {
9999
pub tool_call_id: String,
100100
/// Ordered content blocks.
101101
pub content: Vec<ContentBlock>,
102+
/// The producing tool asked for this content to reach the model
103+
/// byte-for-byte, so a host that rewrites tool output must leave it alone.
104+
///
105+
/// Set from [`ToolResult::is_trusted_verbatim`] by
106+
/// [`Message::tool_from_result`]; `false` for every other constructor, so
107+
/// nothing changes unless a tool opts in.
108+
///
109+
/// A host is free to summarise, truncate, batch, or re-frame tool output —
110+
/// and normally should. But some results are only useful intact: an input
111+
/// schema the model must copy argument names out of, a signature, a diff.
112+
/// Rewriting those silently produces content that reads fine and is wrong.
113+
/// The flag lets a tool say "this one is not safe to reshape" without the
114+
/// host having to guess from the text.
115+
///
116+
/// `#[serde(default)]` so transcripts persisted before this field existed
117+
/// still deserialise, and `skip_serializing_if` so a message that never
118+
/// opted in stays byte-identical on the wire — matching how the rest of the
119+
/// crate omits unset fields.
120+
#[serde(default, skip_serializing_if = "is_false")]
121+
pub trusted_verbatim: bool,
102122
}
103123

104124
/// A structured conversation message.
@@ -154,3 +174,8 @@ impl MessageDelta {
154174
}
155175
}
156176
}
177+
178+
/// Serde helper: skip serializing `false` booleans.
179+
fn is_false(value: &bool) -> bool {
180+
!*value
181+
}

‎src/harness/tool/test.rs‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,3 +384,104 @@ fn schema_validation_rejects_wrong_types_and_extra_fields() {
384384
let err = schema.validate_call(&extra).expect_err("extra field");
385385
assert!(err.to_string().contains("extra"));
386386
}
387+
388+
#[test]
389+
fn trusted_verbatim_is_opt_in_and_round_trips_through_raw() {
390+
let mut result = ToolResult {
391+
call_id: "call-1".into(),
392+
name: "read_schema".into(),
393+
content: "{\n \"a\": 1\n}".into(),
394+
raw: None,
395+
error: None,
396+
elapsed_ms: 0,
397+
};
398+
assert!(
399+
!result.is_trusted_verbatim(),
400+
"a plain result must not claim the flag"
401+
);
402+
403+
assert!(result.mark_trusted_verbatim(), "an absent `raw` makes room");
404+
assert!(result.is_trusted_verbatim());
405+
406+
// Stored under a known key in `raw`, so a host can round-trip it through its
407+
// own serialization without a schema change.
408+
assert_eq!(
409+
result
410+
.raw
411+
.as_ref()
412+
.and_then(|r| r.get(TRUSTED_VERBATIM_KEY)),
413+
Some(&json!(true))
414+
);
415+
416+
// Marking twice is idempotent and preserves unrelated metadata.
417+
result.raw.as_mut().unwrap()["other"] = json!("kept");
418+
assert!(result.mark_trusted_verbatim());
419+
assert!(result.is_trusted_verbatim());
420+
assert_eq!(result.raw.as_ref().unwrap()["other"], json!("kept"));
421+
}
422+
423+
#[test]
424+
fn unrelated_raw_metadata_never_sets_the_flag() {
425+
// The opt-in must not be trippable by a tool that happens to return raw
426+
// metadata — including a value at the key that is not `true`.
427+
let with_other = ToolResult {
428+
call_id: "c".into(),
429+
name: "t".into(),
430+
content: "x".into(),
431+
raw: Some(json!({ "rows": 3 })),
432+
error: None,
433+
elapsed_ms: 0,
434+
};
435+
assert!(!with_other.is_trusted_verbatim());
436+
437+
let wrong_type = ToolResult {
438+
raw: Some(json!({ TRUSTED_VERBATIM_KEY: "yes" })),
439+
..with_other.clone()
440+
};
441+
assert!(!wrong_type.is_trusted_verbatim());
442+
443+
let not_an_object = ToolResult {
444+
raw: Some(json!("opaque")),
445+
..with_other
446+
};
447+
assert!(!not_an_object.is_trusted_verbatim());
448+
}
449+
450+
#[test]
451+
fn marking_is_refused_rather_than_silently_lost_when_raw_is_not_an_object() {
452+
// Regression: the flag lives under a key in `raw`, so a scalar `raw` has
453+
// nowhere to put it. Clobbering a value the tool deliberately set would be
454+
// worse, so the request is refused — and the caller is told, because a
455+
// silent failure leaves `is_trusted_verbatim()` answering `false` forever
456+
// while the author believes the opt-in took effect.
457+
for raw in [json!("opaque"), json!([1, 2, 3]), json!(7)] {
458+
let mut result = ToolResult {
459+
call_id: "c".into(),
460+
name: "t".into(),
461+
content: "x".into(),
462+
raw: Some(raw.clone()),
463+
error: None,
464+
elapsed_ms: 0,
465+
};
466+
assert!(
467+
!result.mark_trusted_verbatim(),
468+
"a non-object `raw` ({raw}) must refuse the request"
469+
);
470+
assert!(!result.is_trusted_verbatim());
471+
// The tool's own value is left exactly as it was.
472+
assert_eq!(result.raw, Some(raw));
473+
}
474+
475+
// Once the tool makes room, the same call succeeds.
476+
let mut moved = ToolResult {
477+
call_id: "c".into(),
478+
name: "t".into(),
479+
content: "x".into(),
480+
raw: Some(json!({ "value": "opaque" })),
481+
error: None,
482+
elapsed_ms: 0,
483+
};
484+
assert!(moved.mark_trusted_verbatim());
485+
assert!(moved.is_trusted_verbatim());
486+
assert_eq!(moved.raw.as_ref().unwrap()["value"], json!("opaque"));
487+
}

‎src/harness/tool/types.rs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,65 @@ pub struct ToolResult {
109109
pub elapsed_ms: u64,
110110
}
111111

112+
/// `raw` key under which [`ToolResult::mark_trusted_verbatim`] records the
113+
/// request that this result reach the model byte-for-byte.
114+
///
115+
/// Kept in `raw` rather than as a dedicated field so the opt-in costs nothing
116+
/// for the results that never use it, and so a host can round-trip the flag
117+
/// through its own serialisation without a schema change.
118+
pub const TRUSTED_VERBATIM_KEY: &str = "trusted_verbatim";
119+
120+
impl ToolResult {
121+
/// Asks hosts to deliver this result's content to the model unchanged.
122+
///
123+
/// A host is free to summarise, truncate, batch, or re-frame tool output —
124+
/// and usually should. This marks the results where that is not safe: an
125+
/// input schema whose argument names the model must copy exactly, a
126+
/// signature, a diff. Reshaping those yields content that still reads well
127+
/// and is wrong.
128+
///
129+
/// Advisory: the crate carries the flag to [`super::super::message::ToolMessage`]
130+
/// via [`crate::harness::message::Message::tool_from_result`]; honouring it
131+
/// is the host's job.
132+
/// # Returns
133+
///
134+
/// `true` when the request was recorded. `false` when [`Self::raw`] already
135+
/// holds a **non-object** value (a string, array, or number): the flag lives
136+
/// under a key in `raw`, and a scalar has nowhere to put a key. The crate
137+
/// will not clobber a value the tool deliberately set, so the request is
138+
/// refused rather than silently lost — make room by moving that value under
139+
/// a key of its own, then call this again.
140+
///
141+
/// `#[must_use]` precisely because the failure is otherwise invisible:
142+
/// [`Self::is_trusted_verbatim`] would keep answering `false` with no
143+
/// diagnostic, and the caller would believe the opt-in took effect.
144+
#[must_use = "returns false when `raw` holds a non-object value and the request was refused"]
145+
pub fn mark_trusted_verbatim(&mut self) -> bool {
146+
let raw = self
147+
.raw
148+
.get_or_insert_with(|| Value::Object(Default::default()));
149+
match raw.as_object_mut() {
150+
Some(map) => {
151+
map.insert(TRUSTED_VERBATIM_KEY.to_string(), Value::Bool(true));
152+
true
153+
}
154+
None => false,
155+
}
156+
}
157+
158+
/// Whether [`Self::mark_trusted_verbatim`] was set on this result.
159+
///
160+
/// `false` when `raw` is absent, is not an object, or holds anything other
161+
/// than `true` — an opt-in that cannot be tripped by unrelated metadata.
162+
pub fn is_trusted_verbatim(&self) -> bool {
163+
self.raw
164+
.as_ref()
165+
.and_then(|raw| raw.get(TRUSTED_VERBATIM_KEY))
166+
.and_then(Value::as_bool)
167+
.unwrap_or(false)
168+
}
169+
}
170+
112171
/// Live run context visible to a tool invoked by an agent loop.
113172
///
114173
/// The legacy [`Tool::call`] entry point remains available for direct calls and

0 commit comments

Comments
 (0)