Skip to content

Commit d477dee

Browse files
committed
Drop truncated tool_use blocks before they corrupt the next request and add unit tests for the small helpers introduced in the audit work
1 parent 86b746e commit d477dee

5 files changed

Lines changed: 96 additions & 6 deletions

File tree

src/api/model_info.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,26 @@ pub fn effort_support_error(model: &str, effort: ReasoningEffort) -> Option<Stri
325325
mod tests {
326326
use super::*;
327327

328+
#[test]
329+
fn provider_routes_gpt_and_claude_correctly() {
330+
assert_eq!(provider_for("gpt-5.5"), Provider::OpenAI);
331+
assert_eq!(provider_for("gpt-5.4-codex"), Provider::OpenAI);
332+
assert_eq!(provider_for("GPT-5.5"), Provider::OpenAI);
333+
assert_eq!(provider_for("claude-opus-4-7"), Provider::Anthropic);
334+
assert_eq!(
335+
provider_for("claude-sonnet-4-6-20260301"),
336+
Provider::Anthropic
337+
);
338+
assert_eq!(provider_for("o1-mini"), Provider::OpenAI);
339+
assert_eq!(provider_for("unknown-model"), Provider::Anthropic);
340+
}
341+
342+
#[test]
343+
fn provider_label_is_human_readable() {
344+
assert_eq!(Provider::OpenAI.label(), "OpenAI");
345+
assert_eq!(Provider::Anthropic.label(), "Anthropic");
346+
}
347+
328348
#[test]
329349
fn opus_4_7_has_1m_context_and_server_compaction() {
330350
let info = lookup("claude-opus-4-7");

src/clipboard.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,39 @@ pub fn get_clipboard_image() -> Option<PastedImage> {
9595
base64_data,
9696
})
9797
}
98+
99+
#[cfg(test)]
100+
mod tests {
101+
use super::*;
102+
103+
#[test]
104+
fn marker_for_first_index_returns_circled_one() {
105+
assert_eq!(marker_for_index(0), Some('\u{2460}'));
106+
}
107+
108+
#[test]
109+
fn marker_for_last_supported_index_returns_circled_twenty() {
110+
assert_eq!(
111+
marker_for_index(MAX_PASTED_IMAGES_PER_MESSAGE - 1),
112+
Some('\u{2473}')
113+
);
114+
}
115+
116+
#[test]
117+
fn marker_past_supported_range_returns_none() {
118+
assert_eq!(marker_for_index(MAX_PASTED_IMAGES_PER_MESSAGE), None);
119+
assert_eq!(marker_for_index(100), None);
120+
}
121+
122+
#[test]
123+
fn strip_paste_markers_round_trips_indices_and_text() {
124+
let input = format!(
125+
"look at {} and {}",
126+
marker_for_index(0).unwrap(),
127+
marker_for_index(2).unwrap()
128+
);
129+
let (text, indices) = strip_paste_markers(&input);
130+
assert_eq!(text, "look at and");
131+
assert_eq!(indices, vec![0, 2]);
132+
}
133+
}

src/repl/response_handler.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,24 @@ impl ResponseHandler {
145145
}
146146

147147
if !content_blocks.is_empty() {
148-
let message_blocks: Vec<crate::api::MessageContentBlock> = content_blocks
148+
let mut message_blocks: Vec<crate::api::MessageContentBlock> = content_blocks
149149
.iter()
150150
.map(crate::api::MessageContentBlock::from_content_block_for_api)
151151
.collect();
152+
if truncated_by_max_tokens {
153+
// Drop tool-use blocks from a truncated response.
154+
// Their arguments may be half-formed JSON, and storing
155+
// a `tool_use` without the matching `tool_result`
156+
// leaves the next request in a shape the provider
157+
// will reject.
158+
message_blocks.retain(|block| {
159+
!matches!(
160+
block,
161+
crate::api::MessageContentBlock::ToolUse { .. }
162+
| crate::api::MessageContentBlock::ServerToolUse { .. }
163+
)
164+
});
165+
}
152166
if !message_blocks.is_empty() {
153167
self.conversation.add_assistant_with_blocks(message_blocks);
154168
}
@@ -160,9 +174,6 @@ impl ResponseHandler {
160174
"Consider using --max-tokens with a higher value (current: {})",
161175
self.max_tokens
162176
);
163-
// Skip tool execution: a truncated tool_use carries
164-
// half-formed JSON arguments, so running it would
165-
// either parse-fail or fire on partial input.
166177
return Ok(());
167178
}
168179

src/session/history/manager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ impl HistoryManager {
131131

132132
/// Reject session ids that could escape the sessions directory
133133
/// when interpolated into a path. The generator only produces
134-
/// `session_<timestamp>` strings, so anything containing a path
135-
/// separator or `..` came from an external caller (e.g.
134+
/// `session_<timestamp>_<random>` strings, so anything containing a
135+
/// path separator or `..` came from an external caller (e.g.
136136
/// `--resume <id>`) and must not be trusted.
137137
fn validate_session_id(session_id: &str) -> Result<()> {
138138
if session_id.is_empty()

src/tools/mod.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,26 @@ pub fn read_file_body(output: &str) -> &str {
4343
.map(|(_, body)| body)
4444
.unwrap_or(output)
4545
}
46+
47+
#[cfg(test)]
48+
mod public_helpers_tests {
49+
use super::*;
50+
51+
#[test]
52+
fn format_and_recover_round_trip() {
53+
let payload = format_read_file_output("src/main.rs", "fn main() {}\n");
54+
assert!(payload.starts_with(READ_FILE_HEADER));
55+
assert_eq!(read_file_body(&payload), "fn main() {}\n");
56+
}
57+
58+
#[test]
59+
fn read_file_body_falls_back_when_separator_missing() {
60+
assert_eq!(read_file_body("no separator here"), "no separator here");
61+
}
62+
63+
#[test]
64+
fn read_file_body_handles_empty_body() {
65+
let payload = format_read_file_output("empty.txt", "");
66+
assert_eq!(read_file_body(&payload), "");
67+
}
68+
}

0 commit comments

Comments
 (0)