Skip to content

Commit 5907422

Browse files
authored
feat: annotate conversations with model_provider for filtering (#5658)
Because conversations that use the Responses API can have encrypted reasoning messages, trying to resume a conversation with a different provider could lead to confusing "failed to decrypt" errors. (This is reproducible by starting a conversation using ChatGPT login and resuming it as a conversation that uses OpenAI models via Azure.) This changes `ListConversationsParams` to take a `model_providers: Option<Vec<String>>` and adds `model_provider` on each `ConversationSummary` it returns so these cases can be disambiguated. Note this ended up making changes to `codex-rs/core/src/rollout/tests.rs` because it had a number of cases where it expected `Some` for the value of `next_cursor`, but the list of rollouts was complete, so according to this docstring: https://github.com/openai/codex/blob/bcd64c7e7231d6316a2377d1525a0fa74f21b783/codex-rs/app-server-protocol/src/protocol.rs#L334-L337 If there are no more items to return, then `next_cursor` should be `None`. This PR updates that logic. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/5658). * #5803 * #5793 * __->__ #5658
1 parent f178805 commit 5907422

File tree

14 files changed

+560
-74
lines changed

14 files changed

+560
-74
lines changed

codex-rs/app-server-protocol/src/protocol.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,12 @@ pub struct ListConversationsParams {
327327
/// Opaque pagination cursor returned by a previous call.
328328
#[serde(skip_serializing_if = "Option::is_none")]
329329
pub cursor: Option<String>,
330+
/// Optional model provider filter (matches against session metadata).
331+
/// - None => filter by the server's default model provider
332+
/// - Some([]) => no filtering, include all providers
333+
/// - Some([...]) => only include sessions with one of the specified providers
334+
#[serde(skip_serializing_if = "Option::is_none")]
335+
pub model_providers: Option<Vec<String>>,
330336
}
331337

332338
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
@@ -338,6 +344,8 @@ pub struct ConversationSummary {
338344
/// RFC3339 timestamp string for the session start, if available.
339345
#[serde(skip_serializing_if = "Option::is_none")]
340346
pub timestamp: Option<String>,
347+
/// Model provider recorded for the session (resolved when absent in metadata).
348+
pub model_provider: String,
341349
}
342350

343351
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]

codex-rs/app-server/src/codex_message_processor.rs

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -827,19 +827,38 @@ impl CodexMessageProcessor {
827827
request_id: RequestId,
828828
params: ListConversationsParams,
829829
) {
830-
let page_size = params.page_size.unwrap_or(25);
830+
let ListConversationsParams {
831+
page_size,
832+
cursor,
833+
model_providers: model_provider,
834+
} = params;
835+
let page_size = page_size.unwrap_or(25);
831836
// Decode the optional cursor string to a Cursor via serde (Cursor implements Deserialize from string)
832-
let cursor_obj: Option<RolloutCursor> = match params.cursor {
837+
let cursor_obj: Option<RolloutCursor> = match cursor {
833838
Some(s) => serde_json::from_str::<RolloutCursor>(&format!("\"{s}\"")).ok(),
834839
None => None,
835840
};
836841
let cursor_ref = cursor_obj.as_ref();
842+
let model_provider_filter = match model_provider {
843+
Some(providers) => {
844+
if providers.is_empty() {
845+
None
846+
} else {
847+
Some(providers)
848+
}
849+
}
850+
None => Some(vec![self.config.model_provider_id.clone()]),
851+
};
852+
let model_provider_slice = model_provider_filter.as_deref();
853+
let fallback_provider = self.config.model_provider_id.clone();
837854

838855
let page = match RolloutRecorder::list_conversations(
839856
&self.config.codex_home,
840857
page_size,
841858
cursor_ref,
842859
INTERACTIVE_SESSION_SOURCES,
860+
model_provider_slice,
861+
fallback_provider.as_str(),
843862
)
844863
.await
845864
{
@@ -858,7 +877,7 @@ impl CodexMessageProcessor {
858877
let items = page
859878
.items
860879
.into_iter()
861-
.filter_map(|it| extract_conversation_summary(it.path, &it.head))
880+
.filter_map(|it| extract_conversation_summary(it.path, &it.head, &fallback_provider))
862881
.collect();
863882

864883
// Encode next_cursor as a plain string
@@ -1707,6 +1726,7 @@ async fn on_exec_approval_response(
17071726
fn extract_conversation_summary(
17081727
path: PathBuf,
17091728
head: &[serde_json::Value],
1729+
fallback_provider: &str,
17101730
) -> Option<ConversationSummary> {
17111731
let session_meta = match head.first() {
17121732
Some(first_line) => serde_json::from_value::<SessionMeta>(first_line.clone()).ok()?,
@@ -1731,12 +1751,17 @@ fn extract_conversation_summary(
17311751
} else {
17321752
Some(session_meta.timestamp.clone())
17331753
};
1754+
let conversation_id = session_meta.id;
1755+
let model_provider = session_meta
1756+
.model_provider
1757+
.unwrap_or_else(|| fallback_provider.to_string());
17341758

17351759
Some(ConversationSummary {
1736-
conversation_id: session_meta.id,
1760+
conversation_id,
17371761
timestamp,
17381762
path,
17391763
preview: preview.to_string(),
1764+
model_provider,
17401765
})
17411766
}
17421767

@@ -1760,7 +1785,8 @@ mod tests {
17601785
"cwd": "/",
17611786
"originator": "codex",
17621787
"cli_version": "0.0.0",
1763-
"instructions": null
1788+
"instructions": null,
1789+
"model_provider": "test-provider"
17641790
}),
17651791
json!({
17661792
"type": "message",
@@ -1780,7 +1806,8 @@ mod tests {
17801806
}),
17811807
];
17821808

1783-
let summary = extract_conversation_summary(path.clone(), &head).expect("summary");
1809+
let summary =
1810+
extract_conversation_summary(path.clone(), &head, "test-provider").expect("summary");
17841811

17851812
assert_eq!(summary.conversation_id, conversation_id);
17861813
assert_eq!(
@@ -1789,6 +1816,7 @@ mod tests {
17891816
);
17901817
assert_eq!(summary.path, path);
17911818
assert_eq!(summary.preview, "Count to 5");
1819+
assert_eq!(summary.model_provider, "test-provider");
17921820
Ok(())
17931821
}
17941822
}

codex-rs/app-server/tests/suite/list_resume.rs

Lines changed: 108 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,21 @@ async fn test_list_and_resume_conversations() {
3030
"2025-01-02T12-00-00",
3131
"2025-01-02T12:00:00Z",
3232
"Hello A",
33+
Some("openai"),
3334
);
3435
create_fake_rollout(
3536
codex_home.path(),
3637
"2025-01-01T13-00-00",
3738
"2025-01-01T13:00:00Z",
3839
"Hello B",
40+
Some("openai"),
3941
);
4042
create_fake_rollout(
4143
codex_home.path(),
4244
"2025-01-01T12-00-00",
4345
"2025-01-01T12:00:00Z",
4446
"Hello C",
47+
None,
4548
);
4649

4750
let mut mcp = McpProcess::new(codex_home.path())
@@ -57,6 +60,7 @@ async fn test_list_and_resume_conversations() {
5760
.send_list_conversations_request(ListConversationsParams {
5861
page_size: Some(2),
5962
cursor: None,
63+
model_providers: None,
6064
})
6165
.await
6266
.expect("send listConversations");
@@ -74,6 +78,8 @@ async fn test_list_and_resume_conversations() {
7478
// Newest first; preview text should match
7579
assert_eq!(items[0].preview, "Hello A");
7680
assert_eq!(items[1].preview, "Hello B");
81+
assert_eq!(items[0].model_provider, "openai");
82+
assert_eq!(items[1].model_provider, "openai");
7783
assert!(items[0].path.is_absolute());
7884
assert!(next_cursor.is_some());
7985

@@ -82,6 +88,7 @@ async fn test_list_and_resume_conversations() {
8288
.send_list_conversations_request(ListConversationsParams {
8389
page_size: Some(2),
8490
cursor: next_cursor,
91+
model_providers: None,
8592
})
8693
.await
8794
.expect("send listConversations page 2");
@@ -99,7 +106,88 @@ async fn test_list_and_resume_conversations() {
99106
} = to_response::<ListConversationsResponse>(resp2).expect("deserialize response");
100107
assert_eq!(items2.len(), 1);
101108
assert_eq!(items2[0].preview, "Hello C");
102-
assert!(next2.is_some());
109+
assert_eq!(items2[0].model_provider, "openai");
110+
assert_eq!(next2, None);
111+
112+
// Add a conversation with an explicit non-OpenAI provider for filter tests.
113+
create_fake_rollout(
114+
codex_home.path(),
115+
"2025-01-01T11-30-00",
116+
"2025-01-01T11:30:00Z",
117+
"Hello TP",
118+
Some("test-provider"),
119+
);
120+
121+
// Filtering by model provider should return only matching sessions.
122+
let filter_req_id = mcp
123+
.send_list_conversations_request(ListConversationsParams {
124+
page_size: Some(10),
125+
cursor: None,
126+
model_providers: Some(vec!["test-provider".to_string()]),
127+
})
128+
.await
129+
.expect("send listConversations filtered");
130+
let filter_resp: JSONRPCResponse = timeout(
131+
DEFAULT_READ_TIMEOUT,
132+
mcp.read_stream_until_response_message(RequestId::Integer(filter_req_id)),
133+
)
134+
.await
135+
.expect("listConversations filtered timeout")
136+
.expect("listConversations filtered resp");
137+
let ListConversationsResponse {
138+
items: filtered_items,
139+
next_cursor: filtered_next,
140+
} = to_response::<ListConversationsResponse>(filter_resp).expect("deserialize filtered");
141+
assert_eq!(filtered_items.len(), 1);
142+
assert_eq!(filtered_next, None);
143+
assert_eq!(filtered_items[0].preview, "Hello TP");
144+
assert_eq!(filtered_items[0].model_provider, "test-provider");
145+
146+
// Empty filter should include every session regardless of provider metadata.
147+
let unfiltered_req_id = mcp
148+
.send_list_conversations_request(ListConversationsParams {
149+
page_size: Some(10),
150+
cursor: None,
151+
model_providers: Some(Vec::new()),
152+
})
153+
.await
154+
.expect("send listConversations unfiltered");
155+
let unfiltered_resp: JSONRPCResponse = timeout(
156+
DEFAULT_READ_TIMEOUT,
157+
mcp.read_stream_until_response_message(RequestId::Integer(unfiltered_req_id)),
158+
)
159+
.await
160+
.expect("listConversations unfiltered timeout")
161+
.expect("listConversations unfiltered resp");
162+
let ListConversationsResponse {
163+
items: unfiltered_items,
164+
next_cursor: unfiltered_next,
165+
} = to_response::<ListConversationsResponse>(unfiltered_resp)
166+
.expect("deserialize unfiltered response");
167+
assert_eq!(unfiltered_items.len(), 4);
168+
assert!(unfiltered_next.is_none());
169+
170+
let empty_req_id = mcp
171+
.send_list_conversations_request(ListConversationsParams {
172+
page_size: Some(10),
173+
cursor: None,
174+
model_providers: Some(vec!["other".to_string()]),
175+
})
176+
.await
177+
.expect("send listConversations filtered empty");
178+
let empty_resp: JSONRPCResponse = timeout(
179+
DEFAULT_READ_TIMEOUT,
180+
mcp.read_stream_until_response_message(RequestId::Integer(empty_req_id)),
181+
)
182+
.await
183+
.expect("listConversations filtered empty timeout")
184+
.expect("listConversations filtered empty resp");
185+
let ListConversationsResponse {
186+
items: empty_items,
187+
next_cursor: empty_next,
188+
} = to_response::<ListConversationsResponse>(empty_resp).expect("deserialize filtered empty");
189+
assert!(empty_items.is_empty());
190+
assert!(empty_next.is_none());
103191

104192
// Now resume one of the sessions and expect a SessionConfigured notification and response.
105193
let resume_req_id = mcp
@@ -152,7 +240,13 @@ async fn test_list_and_resume_conversations() {
152240
assert!(!conversation_id.to_string().is_empty());
153241
}
154242

155-
fn create_fake_rollout(codex_home: &Path, filename_ts: &str, meta_rfc3339: &str, preview: &str) {
243+
fn create_fake_rollout(
244+
codex_home: &Path,
245+
filename_ts: &str,
246+
meta_rfc3339: &str,
247+
preview: &str,
248+
model_provider: Option<&str>,
249+
) {
156250
let uuid = Uuid::new_v4();
157251
// sessions/YYYY/MM/DD/ derived from filename_ts (YYYY-MM-DDThh-mm-ss)
158252
let year = &filename_ts[0..4];
@@ -164,18 +258,22 @@ fn create_fake_rollout(codex_home: &Path, filename_ts: &str, meta_rfc3339: &str,
164258
let file_path = dir.join(format!("rollout-{filename_ts}-{uuid}.jsonl"));
165259
let mut lines = Vec::new();
166260
// Meta line with timestamp (flattened meta in payload for new schema)
261+
let mut payload = json!({
262+
"id": uuid,
263+
"timestamp": meta_rfc3339,
264+
"cwd": "/",
265+
"originator": "codex",
266+
"cli_version": "0.0.0",
267+
"instructions": null,
268+
});
269+
if let Some(provider) = model_provider {
270+
payload["model_provider"] = json!(provider);
271+
}
167272
lines.push(
168273
json!({
169274
"timestamp": meta_rfc3339,
170275
"type": "session_meta",
171-
"payload": {
172-
"id": uuid,
173-
"timestamp": meta_rfc3339,
174-
"cwd": "/",
175-
"originator": "codex",
176-
"cli_version": "0.0.0",
177-
"instructions": null
178-
}
276+
"payload": payload
179277
})
180278
.to_string(),
181279
);

0 commit comments

Comments
 (0)