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
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,23 @@ fn image_blocks_preserve_text_order() {
assert_eq!(content[4]["text"], " after");
}

#[test]
fn literal_native_image_marker_stays_text() {
let s = String::from_utf8(build_stdin(
&[ChatMessage::user(
"literal [OH_IMAGE:data:image/png;base64,QUJD] then [OH_IMAGE_LITERAL:data:image/png;base64,REVG]",
)],
true,
))
.unwrap();
let row: Value = serde_json::from_str(s.lines().next().unwrap()).unwrap();
let content = row["message"]["content"].as_array().unwrap();
assert_eq!(content[0]["text"], "literal ");
assert_eq!(content[1]["type"], "image");
assert_eq!(content[2]["text"], " then ");
assert_eq!(content[3]["text"], "[OH_IMAGE:data:image/png;base64,REVG]");
}

#[test]
fn literal_file_marker_is_not_read() {
let s = String::from_utf8(build_stdin(
Expand Down
37 changes: 34 additions & 3 deletions crates/tinyagents-harness/src/providers/claude_code/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,9 @@ fn response_format_instruction(format: Option<&ResponseFormat>) -> Option<String
}

/// Flattens a message's content blocks to plain text: text and reasoning
/// blocks pass through, JSON/extension blocks are stringified, an image
/// becomes an `[OH_IMAGE:<url>]` marker `input_builder` later rehydrates,
/// blocks pass through (with internal image-looking text escaped),
/// JSON/extension blocks are stringified, an image becomes an
/// `[OH_IMAGE:<url>]` marker `input_builder` later rehydrates,
/// and redacted-thinking blocks are dropped (nothing to show).
fn render_content(content: &[ContentBlock]) -> String {
content
Expand All @@ -406,7 +407,7 @@ fn render_content(content: &[ContentBlock]) -> String {
// and rehydrated by `input_builder::content_blocks`. Escape the
// same marker when it occurs in ordinary text so user-authored
// prose can never be mistaken for an attachment.
ContentBlock::Text(text) => Some(text.replace("[OH_IMAGE:", "[OH_IMAGE_LITERAL:")),
ContentBlock::Text(text) => Some(escape_native_image_markers(text)),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium critique confident

Escape image markers in reasoning blocks

The updated documentation says text and reasoning blocks pass through with internal image-looking text escaped, but only ContentBlock::Text calls escape_native_image_markers. A reasoning block containing [OH_IMAGE:data:image/png;base64,QUJD] reaches content_blocks unchanged and is converted into an image block, changing the reasoning text and potentially causing an unintended attachment to be sent. Apply the same escaping to reasoning text (and any other textual block that can contain user/provider data) before flattening it.

[RULE] incomplete-escaping ·

ContentBlock::Image(image) => Some(format!("[OH_IMAGE:{}]", image.url)),
ContentBlock::Json(value) | ContentBlock::ProviderExtension(value) => {
Some(value.to_string())
Expand All @@ -424,6 +425,36 @@ fn render_content(content: &[ContentBlock]) -> String {
.join("")
}

/// Escapes complete private image markers in user-authored text.
///
/// An unmatched `[OH_IMAGE:` prefix is ordinary prose and must remain byte-for-byte
/// unchanged; `input_builder::content_blocks` only interprets bracket-terminated
/// markers, so escaping an unmatched prefix would make that malformed prose visible
/// to the user as `[OH_IMAGE_LITERAL:`.
fn escape_native_image_markers(text: &str) -> String {
const IMAGE_PREFIX: &str = "[OH_IMAGE:";
const LITERAL_IMAGE_PREFIX: &str = "[OH_IMAGE_LITERAL:";

let mut escaped = String::with_capacity(text.len());
let mut cursor = 0;
while let Some(relative_start) = text[cursor..].find(IMAGE_PREFIX) {
let start = cursor + relative_start;
let marker_body = start + IMAGE_PREFIX.len();
let Some(relative_end) = text[marker_body..].find(']') else {
escaped.push_str(&text[cursor..]);
return escaped;
};
let end = marker_body + relative_end + 1;

escaped.push_str(&text[cursor..start]);
escaped.push_str(LITERAL_IMAGE_PREFIX);
escaped.push_str(&text[marker_body..end]);
cursor = end;
}
escaped.push_str(&text[cursor..]);
escaped
}

/// Converts an internal [`ChatResponse`] into the harness's `ModelResponse`.
/// `tool_calls` is always empty (see `event_mapper`); cost is surfaced via
/// `raw` only when the CLI reported a non-zero charge, since `finish_reason`
Expand Down
14 changes: 14 additions & 0 deletions crates/tinyagents-harness/src/providers/claude_code/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,20 @@ fn request_rendering_keeps_private_image_marker_text_literal() {
assert_eq!(content[1]["text"], "[OH_IMAGE:data:image/png;base64,QUJD]");
}

#[test]
fn request_rendering_preserves_unclosed_private_image_marker_text() {
let text = "literal [OH_IMAGE:data:image/png;base64,QUJD";
let request = ModelRequest::new(vec![Message::user(text)]);
let row: serde_json::Value =
serde_json::from_slice(&render_request_stdin(&request, true)).expect("stream-json row");
let content = row["message"]["content"]
.as_array()
.expect("content blocks");

assert_eq!(content.len(), 1);
assert_eq!(content[0]["text"], text);
}

#[test]
fn request_messages_filter_host_custom_records() {
let request = ModelRequest::new(vec![
Expand Down
20 changes: 12 additions & 8 deletions crates/tinyagents-integration-tests/tests/dependency_boundary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,39 +99,43 @@ const KNOWN_GENERIC_CLAUDE_CODE_CHAT_MESSAGE_DEBT: &[(&str, usize)] = &[
),
(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high critique confident

Synchronize every tracked ChatMessage reference

These line numbers do not match the current input_builder_tests.rs: the literal-marker test references ChatMessage at line 167, and the file-marker test references it at line 185, while this inventory records 184 and omits 167 and 185. The same mismatch leaves the inventory inconsistent with the source, so the dependency-boundary guard will report stale and missing debt entries and fail the integration test. Update the full inventory from the actual source references rather than applying only the apparent line shifts.

[RULE] synchronized-line-inventory ·

"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
178,
184,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
179,
195,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
180,
196,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
197,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
198,
214,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
199,
215,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
200,
216,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
210,
217,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
225,
227,
),
(
"crates/tinyagents-harness/src/providers/claude_code/input_builder_tests.rs",
242,
),
(
"crates/tinyagents-harness/src/providers/claude_code/mod.rs",
Expand Down
Loading