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
15 changes: 15 additions & 0 deletions P2_HISTORY_HARDENING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# P2 history hardening

This branch implements behavior-level hardening for Codex tool continuation.

Implemented:

- helper module for tool output validation
- duplicate tool output detection
- previous response validation against requested call ids
- Chat history legality check before sending to upstream
- Responses-compatible failed response for history errors
- streaming `response.failed` event for early history errors
- regression tests for unknown, duplicate, orphan, and valid tool histories

This is not a full copy of cc-switch `codex_chat_history.rs`.
109 changes: 109 additions & 0 deletions src/codex_chat_history.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
use serde_json::Value;
use std::collections::HashSet;

use crate::state::StoredResponse;

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum HistoryResolutionError {
UnknownToolCall { call_id: String },
DuplicateToolOutput { call_id: String },
InvalidToolHistory { reason: String },
}

impl std::fmt::Display for HistoryResolutionError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::UnknownToolCall { call_id } => write!(f, "unknown tool call id: {call_id}"),
Self::DuplicateToolOutput { call_id } => {
write!(f, "duplicate tool output: {call_id}")
}
Self::InvalidToolHistory { reason } => write!(f, "invalid tool history: {reason}"),
}
}
}

impl std::error::Error for HistoryResolutionError {}

pub type HistoryResult<T> = Result<T, HistoryResolutionError>;

pub fn ensure_no_duplicate_call_outputs(call_ids: &[String]) -> HistoryResult<()> {
let mut seen = HashSet::new();
for call_id in call_ids {
if !seen.insert(call_id.as_str()) {
return Err(HistoryResolutionError::DuplicateToolOutput {
call_id: call_id.clone(),
});
}
}
Ok(())
}

pub fn validate_requested_call_ids(
previous: &StoredResponse,
requested_call_ids: &[String],
) -> HistoryResult<()> {
ensure_no_duplicate_call_outputs(requested_call_ids)?;
let pending: HashSet<&str> = previous
.pending_call_ids
.iter()
.map(String::as_str)
.collect();
for call_id in requested_call_ids {
if !pending.contains(call_id.as_str()) {
return Err(HistoryResolutionError::UnknownToolCall {
call_id: call_id.clone(),
});
}
}
Ok(())
}

pub fn validate_chat_tool_history(messages: &[Value]) -> HistoryResult<()> {
let mut seen_assistant_calls = HashSet::new();
let mut consumed_tool_outputs = HashSet::new();

for message in messages {
let role = message.get("role").and_then(Value::as_str).unwrap_or("");
if role == "assistant" {
if let Some(calls) = message.get("tool_calls").and_then(Value::as_array) {
for call in calls {
if let Some(call_id) = call
.get("id")
.or_else(|| call.get("call_id"))
.and_then(Value::as_str)
.filter(|value| !value.is_empty())
{
seen_assistant_calls.insert(call_id.to_string());
}
}
}
continue;
}

if role == "tool" {
let call_id = message
.get("tool_call_id")
.or_else(|| message.get("call_id"))
.and_then(Value::as_str)
.filter(|value| !value.is_empty())
.ok_or_else(|| HistoryResolutionError::InvalidToolHistory {
reason: "tool message is missing tool_call_id".to_string(),
})?;

if !seen_assistant_calls.contains(call_id) {
return Err(HistoryResolutionError::InvalidToolHistory {
reason: format!(
"tool message references {call_id}, but no preceding assistant tool_call exists"
),
});
}
if !consumed_tool_outputs.insert(call_id.to_string()) {
return Err(HistoryResolutionError::DuplicateToolOutput {
call_id: call_id.to_string(),
});
}
}
}

Ok(())
}
37 changes: 17 additions & 20 deletions src/conversion/responses_to_chat.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::codex_chat_history::{ensure_no_duplicate_call_outputs, validate_chat_tool_history};
use crate::state::StoredResponse;
use serde_json::{json, Value};
use thiserror::Error;
Expand Down Expand Up @@ -61,6 +62,7 @@ pub fn build_chat_payload(
messages.push(json!({"role":"user","content":""}));
}
messages = normalize_upstream_roles(&messages);
validate_chat_tool_history(&messages).map_err(|error| HistoryError::Invalid(error.to_string()))?;

let mut payload = json!({
"model": model_upstream,
Expand Down Expand Up @@ -377,39 +379,34 @@ pub fn repair_history(
let Some(outputs) = tool_outputs else {
return Ok(repaired);
};
let pending: std::collections::HashSet<String> = repaired

let call_ids: Vec<String> = outputs
.iter()
.filter(|m| m.get("role").and_then(Value::as_str) == Some("assistant"))
.flat_map(|m| {
m.get("tool_calls")
.and_then(Value::as_array)
.into_iter()
.flatten()
})
.filter_map(|call| {
call.get("id")
.filter_map(|output| {
output
.get("tool_call_id")
.or_else(|| output.get("call_id"))
.and_then(Value::as_str)
.map(ToString::to_string)
})
.collect();
let mut seen = std::collections::HashSet::new();
ensure_no_duplicate_call_outputs(&call_ids)
.map_err(|error| HistoryError::Invalid(error.to_string()))?;

for output in outputs {
let call_id = output
.get("tool_call_id")
.and_then(Value::as_str)
.unwrap_or("");
if !pending.contains(call_id) {
return Err(HistoryError::Invalid(format!(
"unknown tool call id: {call_id}"
)));
}
if !seen.insert(call_id.to_string()) {
return Err(HistoryError::Invalid(format!(
"duplicate tool output: {call_id}"
)));
if call_id.is_empty() {
return Err(HistoryError::Invalid(
"tool output requires tool_call_id".to_string(),
));
}
repaired.push(output.clone());
}

validate_chat_tool_history(&repaired).map_err(|error| HistoryError::Invalid(error.to_string()))?;
Ok(repaired)
}

Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub mod codex_chat_history;
pub mod config;
pub mod conversion;
pub mod media_guard;
Expand Down
97 changes: 77 additions & 20 deletions src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::sync::Arc;
use tokio::sync::Semaphore;
use uuid::Uuid;

use crate::codex_chat_history::{ensure_no_duplicate_call_outputs, validate_requested_call_ids};
use crate::config::Config;
use crate::conversion::{
build_chat_payload, build_response, function_output_call_ids, StreamAssembler,
Expand Down Expand Up @@ -123,18 +124,23 @@ async fn complete_response(state: AppState, body: Value) -> Response {
let previous = match previous_response(&state, &body) {
Ok(previous) => previous,
Err(message) => {
return error_response(StatusCode::BAD_REQUEST, "invalid_request_error", &message)
return responses_failed_response(&body, &model_alias, "invalid_tool_history", &message)
}
};
let (payload, messages, _reverse, tool_ctx) =
match build_chat_payload(&body, &model_upstream, previous.as_ref(), json!({})) {
Ok(value) => value,
Err(error) => {
return error_response(
StatusCode::BAD_REQUEST,
"invalid_request_error",
&error.to_string(),
)
let message = error.to_string();
if is_history_error_message(&message) {
return responses_failed_response(
&body,
&model_alias,
"invalid_tool_history",
&message,
);
}
return error_response(StatusCode::BAD_REQUEST, "invalid_request_error", &message);
}
};
if let Some(message) = find_unsupported_multimodal_input(&model_upstream, &payload) {
Expand Down Expand Up @@ -210,18 +216,23 @@ async fn stream_response(state: AppState, body: Value) -> Response {
let previous = match previous_response(&state, &body) {
Ok(previous) => previous,
Err(message) => {
return error_response(StatusCode::BAD_REQUEST, "invalid_request_error", &message)
return early_stream_failed_response(body, model_alias, "invalid_tool_history", &message)
}
};
let (payload, messages, _reverse, tool_ctx) =
match build_chat_payload(&body, &model_upstream, previous.as_ref(), json!({})) {
Ok(value) => value,
Err(error) => {
return error_response(
StatusCode::BAD_REQUEST,
"invalid_request_error",
&error.to_string(),
)
let message = error.to_string();
if is_history_error_message(&message) {
return early_stream_failed_response(
body,
model_alias,
"invalid_tool_history",
&message,
);
}
return error_response(StatusCode::BAD_REQUEST, "invalid_request_error", &message);
}
};

Expand Down Expand Up @@ -371,22 +382,41 @@ fn previous_response(
state: &AppState,
body: &Value,
) -> Result<Option<crate::state::StoredResponse>, String> {
let ids = function_output_call_ids(body).map_err(|e| e.to_string())?;
ensure_no_duplicate_call_outputs(&ids).map_err(|e| e.to_string())?;

if let Some(previous_id) = body
.get("previous_response_id")
.and_then(Value::as_str)
.filter(|v| !v.is_empty())
{
return state.state.get(previous_id).map_err(|e| e.to_string());
let previous = state.state.get(previous_id).map_err(|e| e.to_string())?;
if let Some(previous) = previous.as_ref() {
validate_requested_call_ids(previous, &ids).map_err(|e| e.to_string())?;
} else if !ids.is_empty() {
return Err(format!(
"tool output has no matching stored response: {previous_id}"
));
}
return Ok(previous);
}
let ids = function_output_call_ids(body).map_err(|e| e.to_string())?;

if ids.is_empty() {
Ok(None)
} else {
state
.state
.find_by_call_ids(&ids)
.map_err(|e| e.to_string())
return Ok(None);
}

let previous = state
.state
.find_by_call_ids(&ids)
.map_err(|e| e.to_string())?;
let Some(previous) = previous else {
return Err(format!(
"unknown or ambiguous tool call id(s): {}",
ids.join(", ")
));
};
validate_requested_call_ids(&previous, &ids).map_err(|e| e.to_string())?;
Ok(Some(previous))
}

fn authorize(config: &Config, headers: &HeaderMap) -> Result<(), Box<Response>> {
Expand Down Expand Up @@ -455,6 +485,24 @@ fn responses_failed_value(body: &Value, model: &str, kind: &str, message: &str)
})
}

fn early_stream_failed_response(
body: Value,
model_alias: String,
kind: &'static str,
message: &str,
) -> Response {
let (tx, rx) =
tokio::sync::mpsc::unbounded_channel::<Result<axum::response::sse::Event, Infallible>>();
let response = responses_failed_value(&body, &model_alias, kind, message);
let event = json!({"type":"response.failed","response":response});
let _ = tx.send(Ok(axum::response::sse::Event::default()
.event("response.failed")
.data(event.to_string())));
let _ = tx.send(Ok(axum::response::sse::Event::default().data("[DONE]")));
let stream = tokio_stream::wrappers::UnboundedReceiverStream::new(rx);
Sse::new(stream).into_response()
}

fn stream_failed_response(
state: AppState,
body: Value,
Expand Down Expand Up @@ -491,6 +539,15 @@ fn stream_failed_response(
Sse::new(stream).into_response()
}

fn is_history_error_message(message: &str) -> bool {
message.contains("tool output")
|| message.contains("tool call")
|| message.contains("tool history")
|| message.contains("duplicate tool")
|| message.contains("unknown tool")
|| message.contains("invalid tool")
}

fn upstream_stream_error_type(message: &str) -> &'static str {
if is_multimodal_unsupported_error(message) {
"unsupported_multimodal_input"
Expand Down
Loading
Loading