mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Preserve tool authority for TUI delegation prompts (#41046)
## Why Delegated prompts should retain the authority of the TUI tool that sent them instead of being recorded as user input. ## What changed - Start delegated `create_thread` and `send_message_to_thread` turns with a namespaced tool output and an empty user input list. - Recognize delegated outputs from trusted Codex namespaces when building activity summaries. - Render replayed delegation outputs in history and transcripts with their source task attribution, without adding them to composer history. ## Testing - Cover turn request encoding, namespace filtering, activity summaries, and replayed history/transcript rendering. GitOrigin-RevId: aeb2f5c8ad2f2a898c80e8fc791ed879a0ab655d
This commit is contained in:
@@ -1284,14 +1284,18 @@ async fn dynamic_tool_requests_ignore_other_namespaces_and_dispatch_tui_namespac
|
||||
let turn = recorded_params(&requests, "turn/start")
|
||||
.pop()
|
||||
.expect("background task turn request");
|
||||
assert_eq!(turn["input"], serde_json::json!([]));
|
||||
assert_eq!(
|
||||
turn["input"][0]["text"],
|
||||
format!(
|
||||
"<codex_delegation>\n <source_thread_id>{creation_source}</source_thread_id>\n <input>Check <main> & report</input>\n</codex_delegation>"
|
||||
)
|
||||
turn["toolOutput"],
|
||||
serde_json::json!({
|
||||
"name": "create_thread",
|
||||
"namespace": "codex_tui",
|
||||
"output": format!(
|
||||
"<codex_delegation>\n <source_thread_id>{creation_source}</source_thread_id>\n <input>Check <main> & report</input>\n</codex_delegation>"
|
||||
)
|
||||
})
|
||||
);
|
||||
assert_eq!(turn["sandboxPolicy"], source_sandbox);
|
||||
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
codex_app_server_client::AppServerEvent::ServerRequest(Box::new(exec_approval_request(
|
||||
@@ -1353,11 +1357,17 @@ async fn dynamic_tool_requests_ignore_other_namespaces_and_dispatch_tui_namespac
|
||||
panic!("expected a follow-up task completion event")
|
||||
};
|
||||
assert!(response.success, "{response:?}");
|
||||
let turn = &recorded_params(&requests, "turn/start")[1];
|
||||
assert_eq!(turn["input"], serde_json::json!([]));
|
||||
assert_eq!(
|
||||
recorded_params(&requests, "turn/start")[1]["input"][0]["text"],
|
||||
format!(
|
||||
"<codex_delegation>\n <source_thread_id>{thread_id}</source_thread_id>\n <input>Follow <up> & report</input>\n</codex_delegation>"
|
||||
)
|
||||
turn["toolOutput"],
|
||||
serde_json::json!({
|
||||
"name": "send_message_to_thread",
|
||||
"namespace": "codex_tui",
|
||||
"output": format!(
|
||||
"<codex_delegation>\n <source_thread_id>{thread_id}</source_thread_id>\n <input>Follow <up> & report</input>\n</codex_delegation>"
|
||||
)
|
||||
})
|
||||
);
|
||||
|
||||
app.dynamic_tool_tasks.insert(
|
||||
|
||||
@@ -218,7 +218,27 @@ impl ChatWidget {
|
||||
ThreadItem::ContextCompaction { .. } => {
|
||||
self.add_info_message("Context compacted".to_string(), /*hint*/ None);
|
||||
}
|
||||
ThreadItem::HookPrompt { .. } | ThreadItem::FunctionCallOutput { .. } => {}
|
||||
ThreadItem::FunctionCallOutput {
|
||||
name,
|
||||
namespace,
|
||||
output,
|
||||
..
|
||||
} => {
|
||||
if let Some((source_thread_id, prompt)) =
|
||||
crate::dynamic_tools::parse_delegated_tool_output(
|
||||
&name,
|
||||
namespace.as_deref(),
|
||||
&output,
|
||||
)
|
||||
{
|
||||
self.add_to_history(history_cell::PrefixedWrappedHistoryCell::new(
|
||||
format!("Sent by Codex from task {source_thread_id}\n{prompt}"),
|
||||
"• ".dim(),
|
||||
" ",
|
||||
));
|
||||
}
|
||||
}
|
||||
ThreadItem::HookPrompt { .. } => {}
|
||||
ThreadItem::CollabAgentToolCall {
|
||||
id,
|
||||
tool,
|
||||
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/history_replay.rs
|
||||
expression: "lines_to_single_string(&cells[0])"
|
||||
---
|
||||
• Sent by Codex from task source-task
|
||||
Follow <up> & report
|
||||
@@ -2,6 +2,7 @@ use super::*;
|
||||
use crate::app_event::HistoryLookupResponse;
|
||||
use codex_app_server_protocol::NetworkAccess;
|
||||
use codex_app_server_protocol::SandboxPolicy;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::models::ManagedFileSystemPermissions;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
@@ -275,6 +276,49 @@ async fn replayed_review_prompt_does_not_seed_composer_history() {
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_delegated_tool_output_is_attributed_without_seeding_composer_history() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
let item = AppServerThreadItem::FunctionCallOutput {
|
||||
id: "delegation-1".to_string(),
|
||||
name: "send_message_to_thread".to_string(),
|
||||
namespace: Some("codex_tui".to_string()),
|
||||
output: FunctionCallOutputBody::Text(
|
||||
"<codex_delegation>\n <source_thread_id>source-task</source_thread_id>\n <input>Follow <up> & report</input>\n</codex_delegation>".to_string(),
|
||||
),
|
||||
};
|
||||
chat.replay_thread_item(
|
||||
item.clone(),
|
||||
"turn-1".to_string(),
|
||||
ReplayKind::ResumeInitialMessages,
|
||||
);
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1);
|
||||
assert_chatwidget_snapshot!(
|
||||
"replayed_delegated_tool_output",
|
||||
lines_to_single_string(&cells[0])
|
||||
);
|
||||
let projected = crate::thread_transcript::thread_items_to_transcript_cells(
|
||||
/*thread_id*/ None,
|
||||
&chat.config.cwd,
|
||||
[item],
|
||||
crate::thread_transcript::RawReasoningVisibility::Hidden,
|
||||
/*config*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
projected
|
||||
.iter()
|
||||
.map(|cell| lines_to_single_string(&cell.transcript_lines(/*width*/ 80)))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![lines_to_single_string(&cells[0])]
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_nested_review_prompts_do_not_render_or_seed_composer_history() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
@@ -46,8 +46,10 @@ use codex_app_server_protocol::Turn;
|
||||
use codex_app_server_protocol::TurnItemsView;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::TurnStartResponse;
|
||||
use codex_app_server_protocol::TurnToolOutput;
|
||||
use codex_app_server_protocol::UserInput;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use serde::Deserialize;
|
||||
use serde::de::DeserializeOwned;
|
||||
use serde_json::Value;
|
||||
@@ -657,6 +659,7 @@ async fn execute_inner(
|
||||
start_turn(
|
||||
&handle,
|
||||
&thread_id,
|
||||
"create_thread",
|
||||
prompt,
|
||||
/*model*/ None,
|
||||
sandbox_policy,
|
||||
@@ -769,6 +772,7 @@ async fn execute_inner(
|
||||
start_turn(
|
||||
&handle,
|
||||
&arguments.thread_id,
|
||||
"send_message_to_thread",
|
||||
prompt,
|
||||
arguments.model,
|
||||
/*sandbox_policy*/ None,
|
||||
@@ -1167,6 +1171,32 @@ fn delegated_prompt(source_thread_id: &str, prompt: &str) -> String {
|
||||
)
|
||||
}
|
||||
|
||||
fn parse_delegated_prompt(prompt: &str) -> Option<(String, String)> {
|
||||
let delegation = prompt.strip_prefix("<codex_delegation>\n <source_thread_id>")?;
|
||||
let (source, delegated) = delegation.split_once("</source_thread_id>\n <input>")?;
|
||||
let delegated = delegated.strip_suffix("</input>\n</codex_delegation>")?;
|
||||
let unescape = |value: &str| {
|
||||
value
|
||||
.replace("<", "<")
|
||||
.replace(">", ">")
|
||||
.replace("&", "&")
|
||||
};
|
||||
Some((unescape(source), unescape(delegated)))
|
||||
}
|
||||
|
||||
pub(crate) fn parse_delegated_tool_output(
|
||||
name: &str,
|
||||
namespace: Option<&str>,
|
||||
output: &FunctionCallOutputBody,
|
||||
) -> Option<(String, String)> {
|
||||
if !matches!(namespace, Some(NAMESPACE | "codex_app"))
|
||||
|| !matches!(name, "create_thread" | "send_message_to_thread")
|
||||
{
|
||||
return None;
|
||||
}
|
||||
parse_delegated_prompt(output.to_text().as_deref()?)
|
||||
}
|
||||
|
||||
fn same_thread_id(first: &str, second: &str) -> bool {
|
||||
ThreadId::from_string(first)
|
||||
.ok()
|
||||
@@ -1244,6 +1274,7 @@ async fn read_thread(handle: &AppServerRequestHandle, thread_id: &str) -> Result
|
||||
async fn start_turn(
|
||||
handle: &AppServerRequestHandle,
|
||||
thread_id: &str,
|
||||
tool: &str,
|
||||
prompt: String,
|
||||
model: Option<String>,
|
||||
sandbox_policy: Option<SandboxPolicy>,
|
||||
@@ -1252,10 +1283,14 @@ async fn start_turn(
|
||||
request_id,
|
||||
params: TurnStartParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
input: vec![UserInput::Text {
|
||||
text: prompt,
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
input: Vec::new(),
|
||||
// Older app-server/TUI versions are intentionally unsupported:
|
||||
// preserving tool authority takes precedence over legacy fallback.
|
||||
tool_output: Some(Box::new(TurnToolOutput {
|
||||
name: tool.to_string(),
|
||||
namespace: Some(NAMESPACE.to_string()),
|
||||
output: FunctionCallOutputBody::Text(prompt),
|
||||
})),
|
||||
model,
|
||||
sandbox_policy,
|
||||
..TurnStartParams::default()
|
||||
@@ -1294,14 +1329,11 @@ fn turn_summary(turn: &Turn, include_outputs: bool, output_chars: usize) -> Valu
|
||||
"content": content.iter().map(|input| match input {
|
||||
UserInput::Text { text, .. } => {
|
||||
let mut input = json!({"type": "text", "text": truncate(text, DEFAULT_OUTPUT_CHARS)});
|
||||
if let Some(delegation) = text.strip_prefix("<codex_delegation>\n <source_thread_id>")
|
||||
&& let Some((source, delegated)) = delegation.split_once("</source_thread_id>\n <input>")
|
||||
&& let Some(delegated) = delegated.strip_suffix("</input>\n</codex_delegation>")
|
||||
if let Some((source_thread_id, delegated)) = parse_delegated_prompt(text)
|
||||
{
|
||||
let unescape = |value: &str| value.replace("<", "<").replace(">", ">").replace("&", "&");
|
||||
input["codexDelegation"] = json!({
|
||||
"sourceThreadId": unescape(source),
|
||||
"input": truncate(&unescape(delegated), DEFAULT_OUTPUT_CHARS)
|
||||
"sourceThreadId": source_thread_id,
|
||||
"input": truncate(&delegated, DEFAULT_OUTPUT_CHARS)
|
||||
});
|
||||
}
|
||||
input
|
||||
@@ -1326,6 +1358,14 @@ fn turn_summary(turn: &Turn, include_outputs: bool, output_chars: usize) -> Valu
|
||||
let mut item = json!({
|
||||
"type": "functionCallOutput", "id": id, "name": name, "namespace": namespace
|
||||
});
|
||||
if let Some((source_thread_id, delegated)) =
|
||||
parse_delegated_tool_output(name, namespace.as_deref(), output)
|
||||
{
|
||||
item["codexDelegation"] = json!({
|
||||
"sourceThreadId": source_thread_id,
|
||||
"input": truncate(&delegated, DEFAULT_OUTPUT_CHARS)
|
||||
});
|
||||
}
|
||||
if include_outputs {
|
||||
item["output"] = output_summary(
|
||||
output.to_text().as_deref().unwrap_or("[non-text output]"),
|
||||
|
||||
@@ -260,6 +260,17 @@ fn oversized_read_pages_preserve_turns_and_pagination() {
|
||||
|
||||
#[test]
|
||||
fn delegated_prompts_match_desktop_xml_contract() {
|
||||
let output = FunctionCallOutputBody::Text(delegated_prompt("thread-1", "Check status"));
|
||||
for namespace in ["codex_tui", "codex_app"] {
|
||||
assert_eq!(
|
||||
parse_delegated_tool_output("send_message_to_thread", Some(namespace), &output),
|
||||
Some(("thread-1".to_string(), "Check status".to_string()))
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
parse_delegated_tool_output("send_message_to_thread", Some("untrusted"), &output),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
delegated_prompt("thread-1", "Check <main> & report > status"),
|
||||
"<codex_delegation>\n <source_thread_id>thread-1</source_thread_id>\n <input>Check <main> & report > status</input>\n</codex_delegation>"
|
||||
@@ -296,7 +307,9 @@ fn activity_metadata_is_retained_without_including_outputs() -> color_eyre::Resu
|
||||
{"type": "sleep", "id": "sleep-1", "durationMs": 1000},
|
||||
{"type": "imageGeneration", "id": "image-1", "status": "completed",
|
||||
"revisedPrompt": "a cat", "result": "image bytes"},
|
||||
{"type": "enteredReviewMode", "id": "review-1", "review": "review changes"}
|
||||
{"type": "enteredReviewMode", "id": "review-1", "review": "review changes"},
|
||||
{"type": "functionCallOutput", "id": "delegation-1", "name": "send_message_to_thread",
|
||||
"namespace": "codex_tui", "output": delegated_prompt("source-2", "Follow <up> & report")}
|
||||
]
|
||||
}))?;
|
||||
|
||||
@@ -320,7 +333,11 @@ fn activity_metadata_is_retained_without_including_outputs() -> color_eyre::Resu
|
||||
{"type": "sleep", "id": "sleep-1", "durationMs": 1000},
|
||||
{"type": "imageGeneration", "id": "image-1", "status": "completed",
|
||||
"revisedPrompt": "a cat", "savedPath": null},
|
||||
{"type": "enteredReviewMode", "id": "review-1", "review": "review changes"}
|
||||
{"type": "enteredReviewMode", "id": "review-1", "review": "review changes"},
|
||||
{"type": "functionCallOutput", "id": "delegation-1", "name": "send_message_to_thread",
|
||||
"namespace": "codex_tui", "codexDelegation": {
|
||||
"sourceThreadId": "source-2", "input": "Follow <up> & report"
|
||||
}}
|
||||
])
|
||||
);
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ use crate::git_action_directives::parse_assistant_markdown;
|
||||
use crate::history_cell::AgentMarkdownCell;
|
||||
use crate::history_cell::HistoryCell;
|
||||
use crate::history_cell::PlainHistoryCell;
|
||||
use crate::history_cell::PrefixedWrappedHistoryCell;
|
||||
use crate::history_cell::ReasoningSummaryCell;
|
||||
use crate::history_cell::UserHistoryCell;
|
||||
use crate::history_cell::split_reasoning_summary_parts;
|
||||
@@ -134,6 +135,26 @@ pub(crate) fn thread_items_to_transcript_cells(
|
||||
)));
|
||||
}
|
||||
}
|
||||
ThreadItem::FunctionCallOutput {
|
||||
name,
|
||||
namespace,
|
||||
output,
|
||||
..
|
||||
} => {
|
||||
if let Some((source_thread_id, prompt)) =
|
||||
crate::dynamic_tools::parse_delegated_tool_output(
|
||||
&name,
|
||||
namespace.as_deref(),
|
||||
&output,
|
||||
)
|
||||
{
|
||||
cells.push(Arc::new(PrefixedWrappedHistoryCell::new(
|
||||
format!("Sent by Codex from task {source_thread_id}\n{prompt}"),
|
||||
"• ".dim(),
|
||||
" ",
|
||||
)));
|
||||
}
|
||||
}
|
||||
ThreadItem::Plan { text, .. } => {
|
||||
if !text.trim().is_empty() {
|
||||
cells.push(Arc::new(crate::history_cell::new_proposed_plan(
|
||||
|
||||
Reference in New Issue
Block a user