From 525e0d7f3c66edb5012db99b6a88349970972305 Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Wed, 28 Jan 2026 16:37:57 -0800 Subject: [PATCH] Rename plan tool types to todo and dual-emit todo/plan --- .../app-server-protocol/src/protocol/v2.rs | 23 +-- codex-rs/app-server/README.md | 2 +- .../app-server/src/bespoke_event_handling.rs | 36 ++-- codex-rs/core/src/tools/handlers/plan.rs | 14 +- codex-rs/core/tests/suite/tool_harness.rs | 15 +- .../src/event_processor_with_human_output.rs | 14 +- .../src/event_processor_with_jsonl_output.rs | 12 +- .../tests/event_processor_with_json_output.rs | 58 +++---- codex-rs/protocol/src/lib.rs | 1 + codex-rs/protocol/src/plan_tool.rs | 38 +---- codex-rs/protocol/src/protocol.rs | 7 +- codex-rs/protocol/src/todo_tool.rs | 158 ++++++++++++++++++ codex-rs/tui/src/chatwidget.rs | 4 +- codex-rs/tui/src/chatwidget/tests.rs | 46 ++--- codex-rs/tui/src/history_cell.rs | 61 ++++--- 15 files changed, 313 insertions(+), 176 deletions(-) create mode 100644 codex-rs/protocol/src/todo_tool.rs diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 51f5682ead..05bafa55dd 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -17,8 +17,6 @@ use codex_protocol::items::TurnItem as CoreTurnItem; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand; -use codex_protocol::plan_tool::PlanItemArg as CorePlanItemArg; -use codex_protocol::plan_tool::StepStatus as CorePlanStepStatus; use codex_protocol::protocol::AgentStatus as CoreAgentStatus; use codex_protocol::protocol::AskForApproval as CoreAskForApproval; use codex_protocol::protocol::CodexErrorInfo as CoreCodexErrorInfo; @@ -36,6 +34,8 @@ use codex_protocol::protocol::SkillToolDependency as CoreSkillToolDependency; use codex_protocol::protocol::SubAgentSource as CoreSubAgentSource; use codex_protocol::protocol::TokenUsage as CoreTokenUsage; use codex_protocol::protocol::TokenUsageInfo as CoreTokenUsageInfo; +use codex_protocol::todo_tool::TodoItemArg as CoreTodoItemArg; +use codex_protocol::todo_tool::TodoStatus as CoreTodoStatus; use codex_protocol::user_input::ByteRange as CoreByteRange; use codex_protocol::user_input::TextElement as CoreTextElement; use codex_protocol::user_input::UserInput as CoreUserInput; @@ -2256,11 +2256,14 @@ pub struct TurnDiffUpdatedNotification { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] -/// Todo list update from the todo_write tool. The plan name is legacy. +/// Todo list update from the todo_write tool. `todo` is preferred; `plan` is a legacy alias. pub struct TurnPlanUpdatedNotification { pub thread_id: String, pub turn_id: String, pub explanation: Option, + #[serde(default)] + pub todo: Vec, + #[serde(default)] pub plan: Vec, } @@ -2281,8 +2284,8 @@ pub enum TurnPlanStepStatus { Completed, } -impl From for TurnPlanStep { - fn from(value: CorePlanItemArg) -> Self { +impl From for TurnPlanStep { + fn from(value: CoreTodoItemArg) -> Self { Self { step: value.step, status: value.status.into(), @@ -2290,12 +2293,12 @@ impl From for TurnPlanStep { } } -impl From for TurnPlanStepStatus { - fn from(value: CorePlanStepStatus) -> Self { +impl From for TurnPlanStepStatus { + fn from(value: CoreTodoStatus) -> Self { match value { - CorePlanStepStatus::Pending => Self::Pending, - CorePlanStepStatus::InProgress => Self::InProgress, - CorePlanStepStatus::Completed => Self::Completed, + CoreTodoStatus::Pending => Self::Pending, + CoreTodoStatus::InProgress => Self::InProgress, + CoreTodoStatus::Completed => Self::Completed, } } } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index ad1f0790fd..a8116c20c1 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -433,7 +433,7 @@ The app-server streams JSON-RPC notifications while a turn is running. Each turn - `turn/started` — `{ turn }` with the turn id, empty `items`, and `status: "inProgress"`. - `turn/completed` — `{ turn }` where `turn.status` is `completed`, `interrupted`, or `failed`; failures carry `{ error: { message, codexErrorInfo?, additionalDetails? } }`. - `turn/diff/updated` — `{ threadId, turnId, diff }` represents the up-to-date snapshot of the turn-level unified diff, emitted after every FileChange item. `diff` is the latest aggregated unified diff across every file change in the turn. UIs can render this to show the full "what changed" view without stitching individual `fileChange` items. -- `turn/plan/updated` — `{ turnId, explanation?, plan }` whenever the agent shares or changes its todo list; each `plan` entry is `{ step, status }` with `status` in `pending`, `inProgress`, or `completed`. (The plan-named event and field are legacy and are unrelated to Plan mode.) +- `turn/plan/updated` — `{ turnId, explanation?, todo, plan }` whenever the agent shares or changes its todo list; each entry is `{ step, status }` with `status` in `pending`, `inProgress`, or `completed`. `todo` is preferred and `plan` is a deprecated legacy alias; both are emitted for compatibility. (The plan-named event and field are legacy and are unrelated to Plan mode.) Today both notifications carry an empty `items` array even when item events were streamed; rely on `item/*` notifications for the canonical item list until this is fixed. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index e95a236d46..625832d6ca 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -87,10 +87,10 @@ use codex_core::review_format::format_review_findings_block; use codex_core::review_prompts; use codex_protocol::ThreadId; use codex_protocol::dynamic_tools::DynamicToolResponse as CoreDynamicToolResponse; -use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::ReviewOutputEvent; use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUserInputAnswer; use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse; +use codex_protocol::todo_tool::UpdateTodoArgs; use std::collections::HashMap; use std::convert::TryFrom; use std::path::PathBuf; @@ -1143,21 +1143,20 @@ async fn handle_turn_diff( async fn handle_turn_todo_update( conversation_id: ThreadId, event_turn_id: &str, - todo_update_event: UpdatePlanArgs, + todo_update_event: UpdateTodoArgs, api_version: ApiVersion, outgoing: &OutgoingMessageSender, ) { if let ApiVersion::V2 = api_version { // NOTE: The protocol keeps plan-named fields/events for compatibility; this is the todo list update. + let (explanation, todo_items) = todo_update_event.into_parts(); + let steps: Vec = todo_items.into_iter().map(TurnPlanStep::from).collect(); let notification = TurnPlanUpdatedNotification { thread_id: conversation_id.to_string(), turn_id: event_turn_id.to_string(), - explanation: todo_update_event.explanation, - plan: todo_update_event - .plan - .into_iter() - .map(TurnPlanStep::from) - .collect(), + explanation, + todo: steps.clone(), + plan: steps, }; outgoing .send_server_notification(ServerNotification::TurnPlanUpdated(notification)) @@ -1813,8 +1812,8 @@ mod tests { use codex_core::protocol::RateLimitWindow; use codex_core::protocol::TokenUsage; use codex_core::protocol::TokenUsageInfo; - use codex_protocol::plan_tool::PlanItemArg; - use codex_protocol::plan_tool::StepStatus; + use codex_protocol::todo_tool::TodoItemArg; + use codex_protocol::todo_tool::TodoStatus; use mcp_types::CallToolResult; use mcp_types::ContentBlock; use mcp_types::TextContent; @@ -1992,19 +1991,19 @@ mod tests { async fn test_handle_turn_todo_update_emits_notification_for_v2() -> Result<()> { let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY); let outgoing = OutgoingMessageSender::new(tx); - let update = UpdatePlanArgs { - explanation: Some("need todo list".to_string()), - plan: vec![ - PlanItemArg { + let update = UpdateTodoArgs::new( + Some("need todo list".to_string()), + vec![ + TodoItemArg { step: "first".to_string(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }, - PlanItemArg { + TodoItemArg { step: "second".to_string(), - status: StepStatus::Completed, + status: TodoStatus::Completed, }, ], - }; + ); let conversation_id = ThreadId::new(); @@ -2026,6 +2025,7 @@ mod tests { assert_eq!(n.thread_id, conversation_id.to_string()); assert_eq!(n.turn_id, "turn-123"); assert_eq!(n.explanation.as_deref(), Some("need todo list")); + assert_eq!(n.todo, n.plan); assert_eq!(n.plan.len(), 2); assert_eq!(n.plan[0].step, "first"); assert_eq!(n.plan[0].status, TurnPlanStepStatus::Pending); diff --git a/codex-rs/core/src/tools/handlers/plan.rs b/codex-rs/core/src/tools/handlers/plan.rs index 621fd4c825..16152d6b9c 100644 --- a/codex-rs/core/src/tools/handlers/plan.rs +++ b/codex-rs/core/src/tools/handlers/plan.rs @@ -10,8 +10,8 @@ use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::tools::spec::JsonSchema; use async_trait::async_trait; -use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::EventMsg; +use codex_protocol::todo_tool::UpdateTodoArgs; use std::collections::BTreeMap; use std::sync::LazyLock; @@ -41,20 +41,22 @@ pub static TODO_WRITE_TOOL: LazyLock = LazyLock::new(|| { "explanation".to_string(), JsonSchema::String { description: None }, ); - // NOTE: The "plan" field name is legacy tool payload shape, unrelated to Plan mode. + properties.insert("todo".to_string(), todo_items_schema.clone()); + // NOTE: The "plan" field name is a deprecated legacy alias, unrelated to Plan mode. properties.insert("plan".to_string(), todo_items_schema); ToolSpec::Function(ResponsesApiTool { name: "todo_write".to_string(), description: r#"Updates the task list. -Provide an optional explanation and a list of todo items, each with a step and status. +Provide an optional explanation and a list of todo items in `todo`, each with a step and status. +The legacy `plan` field is still accepted. At most one step can be in_progress at a time. "# .to_string(), strict: false, parameters: JsonSchema::Object { properties, - required: Some(vec!["plan".to_string()]), + required: Some(vec!["todo".to_string()]), additional_properties: Some(false.into()), }, }) @@ -112,8 +114,8 @@ pub(crate) async fn handle_todo_write( Ok("Todo list updated".to_string()) } -fn parse_todo_write_arguments(arguments: &str) -> Result { - serde_json::from_str::(arguments).map_err(|e| { +fn parse_todo_write_arguments(arguments: &str) -> Result { + serde_json::from_str::(arguments).map_err(|e| { FunctionCallError::RespondToModel(format!("failed to parse function arguments: {e}")) }) } diff --git a/codex-rs/core/tests/suite/tool_harness.rs b/codex-rs/core/tests/suite/tool_harness.rs index f35809ca5e..cf4e651293 100644 --- a/codex-rs/core/tests/suite/tool_harness.rs +++ b/codex-rs/core/tests/suite/tool_harness.rs @@ -9,7 +9,7 @@ use codex_core::protocol::EventMsg; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; use codex_protocol::config_types::ReasoningSummary; -use codex_protocol::plan_tool::StepStatus; +use codex_protocol::todo_tool::TodoStatus; use codex_protocol::user_input::UserInput; use core_test_support::assert_regex_match; use core_test_support::responses; @@ -124,7 +124,7 @@ async fn todo_write_tool_emits_todo_update_event() -> anyhow::Result<()> { let call_id = "todo-tool-call"; let todo_args = json!({ "explanation": "Tool harness check", - "plan": [ + "todo": [ {"step": "Inspect workspace", "status": "in_progress"}, {"step": "Report results", "status": "pending"}, ], @@ -169,11 +169,12 @@ async fn todo_write_tool_emits_todo_update_event() -> anyhow::Result<()> { EventMsg::PlanUpdate(update) => { saw_todo_update = true; assert_eq!(update.explanation.as_deref(), Some("Tool harness check")); - assert_eq!(update.plan.len(), 2); - assert_eq!(update.plan[0].step, "Inspect workspace"); - assert_matches!(update.plan[0].status, StepStatus::InProgress); - assert_eq!(update.plan[1].step, "Report results"); - assert_matches!(update.plan[1].status, StepStatus::Pending); + let todo_items = update.todo_items(); + assert_eq!(todo_items.len(), 2); + assert_eq!(todo_items[0].step, "Inspect workspace"); + assert_matches!(todo_items[0].status, TodoStatus::InProgress); + assert_eq!(todo_items[1].step, "Report results"); + assert_matches!(todo_items[1].status, TodoStatus::Pending); false } EventMsg::TurnComplete(_) => true, diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 0c24639228..2c8b92ee59 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -45,8 +45,7 @@ use crate::event_processor::CodexStatus; use crate::event_processor::EventProcessor; use crate::event_processor::handle_last_message; use codex_common::create_config_summary_entries; -use codex_protocol::plan_tool::StepStatus; -use codex_protocol::plan_tool::UpdatePlanArgs; +use codex_protocol::todo_tool::TodoStatus; /// This should be configurable. When used in CI, users may not want to impose /// a limit so they can see the full transcript. @@ -539,10 +538,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { eprintln!(); } EventMsg::PlanUpdate(todo_update_event) => { - let UpdatePlanArgs { - explanation, - plan: todo_items, - } = todo_update_event; + let (explanation, todo_items) = todo_update_event.into_parts(); // Header ts_msg!(self, "{}", "Todo list update".style(self.magenta)); @@ -557,13 +553,13 @@ impl EventProcessor for EventProcessorWithHumanOutput { // Pretty-print the todo items with simple status markers. for todo_item in todo_items { match todo_item.status { - StepStatus::Completed => { + TodoStatus::Completed => { ts_msg!(self, " {} {}", "✓".style(self.green), todo_item.step); } - StepStatus::InProgress => { + TodoStatus::InProgress => { ts_msg!(self, " {} {}", "→".style(self.cyan), todo_item.step); } - StepStatus::Pending => { + TodoStatus::Pending => { ts_msg!( self, " {} {}", diff --git a/codex-rs/exec/src/event_processor_with_jsonl_output.rs b/codex-rs/exec/src/event_processor_with_jsonl_output.rs index e8a3779d23..25b02c99b9 100644 --- a/codex-rs/exec/src/event_processor_with_jsonl_output.rs +++ b/codex-rs/exec/src/event_processor_with_jsonl_output.rs @@ -50,8 +50,8 @@ use codex_core::protocol::CollabCloseEndEvent; use codex_core::protocol::CollabWaitingBeginEvent; use codex_core::protocol::CollabWaitingEndEvent; use codex_protocol::models::WebSearchAction; -use codex_protocol::plan_tool::StepStatus; -use codex_protocol::plan_tool::UpdatePlanArgs; +use codex_protocol::todo_tool::TodoStatus; +use codex_protocol::todo_tool::UpdateTodoArgs; use serde_json::Value as JsonValue; use tracing::error; use tracing::warn; @@ -700,17 +700,17 @@ impl EventProcessorWithJsonOutput { vec![ThreadEvent::ItemCompleted(ItemCompletedEvent { item })] } - fn todo_items_from_update(&self, args: &UpdatePlanArgs) -> Vec { - args.plan + fn todo_items_from_update(&self, args: &UpdateTodoArgs) -> Vec { + args.todo_items() .iter() .map(|p| TodoItem { text: p.step.clone(), - completed: matches!(p.status, StepStatus::Completed), + completed: matches!(p.status, TodoStatus::Completed), }) .collect() } - fn handle_todo_update(&mut self, args: &UpdatePlanArgs) -> Vec { + fn handle_todo_update(&mut self, args: &UpdateTodoArgs) -> Vec { let items = self.todo_items_from_update(args); if let Some(running) = &mut self.running_todo_list { diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index 3168035da0..74339679c9 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -56,12 +56,12 @@ use codex_exec::exec_events::Usage; use codex_exec::exec_events::WebSearchItem; use codex_protocol::ThreadId; use codex_protocol::models::WebSearchAction; -use codex_protocol::plan_tool::PlanItemArg; -use codex_protocol::plan_tool::StepStatus; -use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::CodexErrorInfo; use codex_protocol::protocol::ExecCommandOutputDeltaEvent; use codex_protocol::protocol::ExecOutputStream; +use codex_protocol::todo_tool::TodoItemArg; +use codex_protocol::todo_tool::TodoStatus; +use codex_protocol::todo_tool::UpdateTodoArgs; use mcp_types::CallToolResult; use mcp_types::ContentBlock; use mcp_types::TextContent; @@ -226,19 +226,19 @@ fn todo_update_emits_todo_list_started_updated_and_completed() { // First todo_write update (PlanUpdate event) => item.started (todo_list) let first = event( "p1", - EventMsg::PlanUpdate(UpdatePlanArgs { - explanation: None, - plan: vec![ - PlanItemArg { + EventMsg::PlanUpdate(UpdateTodoArgs::new( + None, + vec![ + TodoItemArg { step: "step one".to_string(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }, - PlanItemArg { + TodoItemArg { step: "step two".to_string(), - status: StepStatus::InProgress, + status: TodoStatus::InProgress, }, ], - }), + )), ); let out_first = ep.collect_thread_events(&first); assert_eq!( @@ -265,19 +265,19 @@ fn todo_update_emits_todo_list_started_updated_and_completed() { // Second todo_write update (PlanUpdate event) => item.updated (same id) let second = event( "p2", - EventMsg::PlanUpdate(UpdatePlanArgs { - explanation: None, - plan: vec![ - PlanItemArg { + EventMsg::PlanUpdate(UpdateTodoArgs::new( + None, + vec![ + TodoItemArg { step: "step one".to_string(), - status: StepStatus::Completed, + status: TodoStatus::Completed, }, - PlanItemArg { + TodoItemArg { step: "step two".to_string(), - status: StepStatus::InProgress, + status: TodoStatus::InProgress, }, ], - }), + )), ); let out_second = ep.collect_thread_events(&second); assert_eq!( @@ -666,13 +666,13 @@ fn todo_update_after_complete_starts_new_todo_list_with_new_id() { // First turn: start + complete let start = event( "t1", - EventMsg::PlanUpdate(UpdatePlanArgs { - explanation: None, - plan: vec![PlanItemArg { + EventMsg::PlanUpdate(UpdateTodoArgs::new( + None, + vec![TodoItemArg { step: "only".to_string(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }], - }), + )), ); let _ = ep.collect_thread_events(&start); let complete = event( @@ -686,13 +686,13 @@ fn todo_update_after_complete_starts_new_todo_list_with_new_id() { // Second turn: a new todo list should have a new id let start_again = event( "t3", - EventMsg::PlanUpdate(UpdatePlanArgs { - explanation: None, - plan: vec![PlanItemArg { + EventMsg::PlanUpdate(UpdateTodoArgs::new( + None, + vec![TodoItemArg { step: "again".to_string(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }], - }), + )), ); let out = ep.collect_thread_events(&start_again); diff --git a/codex-rs/protocol/src/lib.rs b/codex-rs/protocol/src/lib.rs index 60b01bbd73..59fdd3e060 100644 --- a/codex-rs/protocol/src/lib.rs +++ b/codex-rs/protocol/src/lib.rs @@ -14,4 +14,5 @@ pub mod parse_command; pub mod plan_tool; pub mod protocol; pub mod request_user_input; +pub mod todo_tool; pub mod user_input; diff --git a/codex-rs/protocol/src/plan_tool.rs b/codex-rs/protocol/src/plan_tool.rs index 9f01111b9d..b8122da2d8 100644 --- a/codex-rs/protocol/src/plan_tool.rs +++ b/codex-rs/protocol/src/plan_tool.rs @@ -1,30 +1,8 @@ -use schemars::JsonSchema; -use serde::Deserialize; -use serde::Serialize; -use ts_rs::TS; - -// Types for the todo_write tool arguments matching codex-vscode/todo-mcp/src/main.rs. -// The plan-named types and fields are legacy payload shape and are not related to Plan mode. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, TS)] -#[serde(rename_all = "snake_case")] -pub enum StepStatus { - Pending, - InProgress, - Completed, -} - -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, TS)] -#[serde(deny_unknown_fields)] -pub struct PlanItemArg { - pub step: String, - pub status: StepStatus, -} - -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, TS)] -#[serde(deny_unknown_fields)] -pub struct UpdatePlanArgs { - #[serde(default)] - pub explanation: Option, - /// Legacy field name for todo items; kept for compatibility. - pub plan: Vec, -} +// Deprecated compatibility layer for the old plan_* names. These are todo list +// types and not related to Plan mode. +#[deprecated(note = "use codex_protocol::todo_tool::TodoItemArg")] +pub use crate::todo_tool::TodoItemArg as PlanItemArg; +#[deprecated(note = "use codex_protocol::todo_tool::TodoStatus")] +pub use crate::todo_tool::TodoStatus as StepStatus; +#[deprecated(note = "use codex_protocol::todo_tool::UpdateTodoArgs")] +pub use crate::todo_tool::UpdateTodoArgs as UpdatePlanArgs; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 400566e771..b28a99fd02 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -29,8 +29,8 @@ use crate::models::WebSearchAction; use crate::num_format::format_with_separators; use crate::openai_models::ReasoningEffort as ReasoningEffortConfig; use crate::parse_command::ParsedCommand; -use crate::plan_tool::UpdatePlanArgs; use crate::request_user_input::RequestUserInputResponse; +use crate::todo_tool::UpdateTodoArgs; use crate::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; use mcp_types::CallToolResult; @@ -810,8 +810,9 @@ pub enum EventMsg { /// Notification that skill data may have been updated and clients may want to reload. SkillsUpdateAvailable, - /// Todo list update from the todo_write tool. The plan name is legacy. - PlanUpdate(UpdatePlanArgs), + /// Todo list update from the todo_write tool. Payload uses `todo` (preferred) + /// and also emits `plan` as a legacy alias. + PlanUpdate(UpdateTodoArgs), TurnAborted(TurnAbortedEvent), diff --git a/codex-rs/protocol/src/todo_tool.rs b/codex-rs/protocol/src/todo_tool.rs new file mode 100644 index 0000000000..37d660a495 --- /dev/null +++ b/codex-rs/protocol/src/todo_tool.rs @@ -0,0 +1,158 @@ +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Deserializer; +use serde::Serialize; +use serde::Serializer; +use serde::de; +use serde::ser::SerializeStruct; +use ts_rs::TS; + +// Types for the todo_write tool arguments matching codex-vscode/todo-mcp/src/main.rs. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, TS, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum TodoStatus { + Pending, + InProgress, + Completed, +} + +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, TS, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct TodoItemArg { + pub step: String, + pub status: TodoStatus, +} + +/// Arguments for the todo_write tool. +/// +/// The `plan` field is a deprecated legacy alias for `todo`, but is still emitted for +/// backward compatibility. +#[derive(Debug, Clone, JsonSchema, TS, PartialEq, Eq, Default)] +pub struct UpdateTodoArgs { + pub explanation: Option, + pub todo: Vec, + pub plan: Vec, +} + +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct UpdateTodoArgsRaw { + #[serde(default)] + explanation: Option, + #[serde(default)] + todo: Option>, + #[serde(default)] + plan: Option>, +} + +impl UpdateTodoArgs { + pub fn new(explanation: Option, todo: Vec) -> Self { + let plan = todo.clone(); + Self { + explanation, + todo, + plan, + } + } + + pub fn todo_items(&self) -> &[TodoItemArg] { + if self.todo.is_empty() { + &self.plan + } else { + &self.todo + } + } + + pub fn into_todo_items(self) -> Vec { + if self.todo.is_empty() { + self.plan + } else { + self.todo + } + } + + pub fn into_parts(self) -> (Option, Vec) { + let UpdateTodoArgs { + explanation, + todo, + plan, + } = self; + let items = if todo.is_empty() { plan } else { todo }; + (explanation, items) + } +} + +impl Serialize for UpdateTodoArgs { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let items = self.todo_items(); + let mut state = serializer.serialize_struct("UpdateTodoArgs", 3)?; + if let Some(explanation) = &self.explanation { + state.serialize_field("explanation", explanation)?; + } + state.serialize_field("todo", items)?; + state.serialize_field("plan", items)?; + state.end() + } +} + +impl<'de> Deserialize<'de> for UpdateTodoArgs { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let raw = UpdateTodoArgsRaw::deserialize(deserializer)?; + let items = match (raw.todo, raw.plan) { + (Some(todo), Some(plan)) => { + if todo.is_empty() { + plan + } else { + todo + } + } + (Some(todo), None) => todo, + (None, Some(plan)) => plan, + (None, None) => return Err(de::Error::missing_field("todo")), + }; + + Ok(UpdateTodoArgs::new(raw.explanation, items)) + } +} + +#[cfg(test)] +mod tests { + use super::TodoItemArg; + use super::TodoStatus; + use super::UpdateTodoArgs; + use pretty_assertions::assert_eq; + + #[test] + fn deserializes_legacy_plan_field() { + let args: UpdateTodoArgs = + serde_json::from_str(r#"{"explanation":"x","plan":[{"step":"a","status":"pending"}]}"#) + .expect("legacy plan field should parse"); + let expected = UpdateTodoArgs::new( + Some("x".to_string()), + vec![TodoItemArg { + step: "a".to_string(), + status: TodoStatus::Pending, + }], + ); + assert_eq!(args, expected); + } + + #[test] + fn serializes_todo_and_plan_fields() { + let args = UpdateTodoArgs::new( + None, + vec![TodoItemArg { + step: "a".to_string(), + status: TodoStatus::Completed, + }], + ); + let value = serde_json::to_value(&args).expect("args should serialize"); + assert_eq!(value.get("todo"), value.get("plan")); + } +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c09466af53..1356f87d51 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -208,7 +208,7 @@ use codex_core::protocol::SandboxPolicy; use codex_file_search::FileMatch; use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; -use codex_protocol::plan_tool::UpdatePlanArgs; +use codex_protocol::todo_tool::UpdateTodoArgs; use strum::IntoEnumIterator; const USER_SHELL_COMMAND_HELP_TITLE: &str = "Prefix a command with ! to run it locally"; @@ -1324,7 +1324,7 @@ impl ChatWidget { Some(combined) } - fn on_todo_update(&mut self, update: UpdatePlanArgs) { + fn on_todo_update(&mut self, update: UpdateTodoArgs) { self.saw_todo_update_this_turn = true; self.add_to_history(history_cell::new_todo_update(update)); } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 211d7c63ab..b860430140 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -70,10 +70,10 @@ use codex_protocol::config_types::Settings; use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::parse_command::ParsedCommand; -use codex_protocol::plan_tool::PlanItemArg; -use codex_protocol::plan_tool::StepStatus; -use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::CodexErrorInfo; +use codex_protocol::todo_tool::TodoItemArg; +use codex_protocol::todo_tool::TodoStatus; +use codex_protocol::todo_tool::UpdateTodoArgs; use codex_protocol::user_input::TextElement; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; @@ -1279,13 +1279,13 @@ async fn plan_implementation_popup_shows_on_todo_update_without_message() { chat.set_collaboration_mask(plan_mask); chat.on_task_started(); - chat.on_todo_update(UpdatePlanArgs { - explanation: None, - plan: vec![PlanItemArg { + chat.on_todo_update(UpdateTodoArgs::new( + None, + vec![TodoItemArg { step: "First".to_string(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }], - }); + )); chat.on_task_complete(None, false); let popup = render_bottom_popup(&chat, 80); @@ -1307,13 +1307,13 @@ async fn plan_implementation_popup_skips_when_rate_limit_prompt_pending() { chat.set_collaboration_mask(plan_mask); chat.on_task_started(); - chat.on_todo_update(UpdatePlanArgs { - explanation: None, - plan: vec![PlanItemArg { + chat.on_todo_update(UpdateTodoArgs::new( + None, + vec![TodoItemArg { step: "First".to_string(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }], - }); + )); chat.on_rate_limit_snapshot(Some(snapshot(92.0))); chat.on_task_complete(None, false); @@ -4406,23 +4406,23 @@ async fn apply_patch_request_shows_diff_summary() -> anyhow::Result<()> { #[tokio::test] async fn todo_update_renders_history_cell() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; - let update = UpdatePlanArgs { - explanation: Some("Adapting plan".to_string()), - plan: vec![ - PlanItemArg { + let update = UpdateTodoArgs::new( + Some("Adapting plan".to_string()), + vec![ + TodoItemArg { step: "Explore codebase".into(), - status: StepStatus::Completed, + status: TodoStatus::Completed, }, - PlanItemArg { + TodoItemArg { step: "Implement feature".into(), - status: StepStatus::InProgress, + status: TodoStatus::InProgress, }, - PlanItemArg { + TodoItemArg { step: "Write tests".into(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }, ], - }; + ); chat.handle_codex_event(Event { id: "sub-1".into(), msg: EventMsg::PlanUpdate(update), diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 79c62813b6..48bb560ff0 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -46,9 +46,9 @@ use codex_core::protocol::SessionConfiguredEvent; use codex_core::web_search::web_search_detail; use codex_protocol::models::WebSearchAction; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; -use codex_protocol::plan_tool::PlanItemArg; -use codex_protocol::plan_tool::StepStatus; -use codex_protocol::plan_tool::UpdatePlanArgs; +use codex_protocol::todo_tool::TodoItemArg; +use codex_protocol::todo_tool::TodoStatus; +use codex_protocol::todo_tool::UpdateTodoArgs; use codex_protocol::user_input::TextElement; use image::DynamicImage; use image::ImageReader; @@ -1728,11 +1728,8 @@ pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { } /// Render a user-friendly todo list update styled like a checkbox list. -pub(crate) fn new_todo_update(update: UpdatePlanArgs) -> TodoUpdateCell { - let UpdatePlanArgs { - explanation, - plan: todo_items, - } = update; +pub(crate) fn new_todo_update(update: UpdateTodoArgs) -> TodoUpdateCell { + let (explanation, todo_items) = update.into_parts(); TodoUpdateCell { explanation, todo_items, @@ -1742,7 +1739,7 @@ pub(crate) fn new_todo_update(update: UpdatePlanArgs) -> TodoUpdateCell { #[derive(Debug)] pub(crate) struct TodoUpdateCell { explanation: Option, - todo_items: Vec, + todo_items: Vec, } impl HistoryCell for TodoUpdateCell { @@ -1755,11 +1752,11 @@ impl HistoryCell for TodoUpdateCell { .collect() }; - let render_step = |status: &StepStatus, text: &str| -> Vec> { + let render_step = |status: &TodoStatus, text: &str| -> Vec> { let (box_str, step_style) = match status { - StepStatus::Completed => ("✔ ", Style::default().crossed_out().dim()), - StepStatus::InProgress => ("□ ", Style::default().cyan().bold()), - StepStatus::Pending => ("□ ", Style::default().dim()), + TodoStatus::Completed => ("✔ ", Style::default().crossed_out().dim()), + TodoStatus::InProgress => ("□ ", Style::default().cyan().bold()), + TodoStatus::Pending => ("□ ", Style::default().dim()), }; let wrap_width = (width as usize) .saturating_sub(4) @@ -1789,7 +1786,7 @@ impl HistoryCell for TodoUpdateCell { if self.todo_items.is_empty() { indented_lines.push(Line::from("(no steps provided)".dim().italic())); } else { - for PlanItemArg { step, status } in self.todo_items.iter() { + for TodoItemArg { step, status } in self.todo_items.iter() { indented_lines.extend(render_step(status, step)); } } @@ -2895,26 +2892,26 @@ mod tests { #[test] fn plan_update_with_note_and_wrapping_snapshot() { // Long explanation forces wrapping; include long step text to verify step wrapping and alignment. - let update = UpdatePlanArgs { - explanation: Some( + let update = UpdateTodoArgs::new( + Some( "I’ll update Grafana call error handling by adding retries and clearer messages when the backend is unreachable." .to_string(), ), - plan: vec![ - PlanItemArg { + vec![ + TodoItemArg { step: "Investigate existing error paths and logging around HTTP timeouts".into(), - status: StepStatus::Completed, + status: TodoStatus::Completed, }, - PlanItemArg { + TodoItemArg { step: "Harden Grafana client error handling with retry/backoff and user‑friendly messages".into(), - status: StepStatus::InProgress, + status: TodoStatus::InProgress, }, - PlanItemArg { + TodoItemArg { step: "Add tests for transient failure scenarios and surfacing to the UI".into(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }, ], - }; + ); let cell = new_todo_update(update); // Narrow width to force wrapping for both the note and steps @@ -2925,19 +2922,19 @@ mod tests { #[test] fn plan_update_without_note_snapshot() { - let update = UpdatePlanArgs { - explanation: None, - plan: vec![ - PlanItemArg { + let update = UpdateTodoArgs::new( + None, + vec![ + TodoItemArg { step: "Define error taxonomy".into(), - status: StepStatus::InProgress, + status: TodoStatus::InProgress, }, - PlanItemArg { + TodoItemArg { step: "Implement mapping to user messages".into(), - status: StepStatus::Pending, + status: TodoStatus::Pending, }, ], - }; + ); let cell = new_todo_update(update); let lines = cell.display_lines(40);