From 5469335de930443c57bf083d9efaad4d71d8e8d8 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 5 Jun 2026 12:12:19 +0900 Subject: [PATCH] refactor: move task feature into pod --- Cargo.lock | 1 - .../feature/builtin/{task.rs => task/mod.rs} | 45 +-- .../src/feature/builtin/task/store.rs} | 287 ++---------------- .../pod/src/feature/builtin/task/tool_impl.rs | 285 +++++++++++++++++ crates/pod/src/ipc/interceptor.rs | 16 +- crates/tools/src/lib.rs | 18 +- crates/tools/src/tracker.rs | 5 +- crates/tools/tests/edge_cases.rs | 5 +- crates/tools/tests/integration.rs | 26 +- crates/tui/Cargo.toml | 1 - crates/tui/src/task.rs | 143 ++++----- package.nix | 18 +- 12 files changed, 429 insertions(+), 421 deletions(-) rename crates/pod/src/feature/builtin/{task.rs => task/mod.rs} (93%) rename crates/{tools/src/task.rs => pod/src/feature/builtin/task/store.rs} (58%) create mode 100644 crates/pod/src/feature/builtin/task/tool_impl.rs diff --git a/Cargo.lock b/Cargo.lock index c4d472fe..3aad1a8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3937,7 +3937,6 @@ dependencies = [ "tempfile", "tokio", "toml", - "tools", "unicode-width", "uuid", ] diff --git a/crates/pod/src/feature/builtin/task.rs b/crates/pod/src/feature/builtin/task/mod.rs similarity index 93% rename from crates/pod/src/feature/builtin/task.rs rename to crates/pod/src/feature/builtin/task/mod.rs index e7bd776c..d6d8c725 100644 --- a/crates/pod/src/feature/builtin/task.rs +++ b/crates/pod/src/feature/builtin/task/mod.rs @@ -1,10 +1,9 @@ //! Task tools built-in feature module. //! -//! The built-in Task feature owns the session-lifetime [`tools::TaskStore`] -//! shared by the Task tools and reminder hooks. Pod hosts install this module -//! through the feature contribution boundary and use its narrow snapshot surface -//! for restore/rewind/compaction compatibility; Pod does not own Task-specific -//! store or reminder state. +//! The built-in Task feature owns the session-lifetime [`TaskStore`] shared by +//! the Task tools and reminder hooks. Pod hosts install this module through the +//! feature contribution boundary and use its narrow snapshot surface for +//! restore/rewind/compaction compatibility. use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -12,6 +11,13 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use async_trait::async_trait; use llm_worker::Item; +mod store; +mod tool_impl; + +pub(crate) use self::tool_impl::task_tools; +use store::snapshot_overview; +pub(crate) use store::{TaskEntry, TaskStatus, TaskStore}; + use crate::feature::{ FeatureDescriptor, FeatureHookPoint, FeatureInstallContext, FeatureInstallError, FeatureModule, HookDeclaration, ToolContribution, ToolDeclaration, @@ -44,20 +50,20 @@ pub struct TaskFeature { #[derive(Debug)] struct TaskFeatureState { - task_store: tools::TaskStore, + task_store: TaskStore, reminder_state: TaskReminderState, } impl TaskFeature { pub fn new() -> Self { - Self::from_store(tools::TaskStore::new()) + Self::from_store(TaskStore::new()) } pub fn from_history(history: &[Item]) -> Self { - Self::from_store(tools::TaskStore::from_history(history)) + Self::from_store(TaskStore::from_history(history)) } - fn from_store(task_store: tools::TaskStore) -> Self { + fn from_store(task_store: TaskStore) -> Self { Self { state: Arc::new(TaskFeatureState { task_store, @@ -70,7 +76,7 @@ impl TaskFeature { /// existing shared store handle. Existing Task tool instances and hooks keep /// pointing at the same feature-owned store after rewind. pub fn restore_from_history(&self, history: &[Item]) { - let restored = tools::TaskStore::from_history(history); + let restored = TaskStore::from_history(history); self.state.task_store.replace_with(restored.list()); } @@ -81,11 +87,11 @@ impl TaskFeature { /// Feature-owned compact summary used for the synthetic TaskList result. pub fn snapshot_overview(&self) -> String { - tools::task::snapshot_overview(&self.state.task_store.list()) + snapshot_overview(&self.state.task_store.list()) } #[cfg(test)] - fn task_store(&self) -> tools::TaskStore { + fn task_store(&self) -> TaskStore { self.state.task_store.clone() } } @@ -130,7 +136,7 @@ impl FeatureModule for TaskFeature { let names = ["TaskCreate", "TaskList", "TaskGet", "TaskUpdate"]; for (name, definition) in names .into_iter() - .zip(tools::task_tools(self.state.task_store.clone())) + .zip(task_tools(self.state.task_store.clone())) { context .tools() @@ -203,17 +209,12 @@ struct TaskReminderPreRequestHook { #[async_trait] impl Hook for TaskReminderPreRequestHook { async fn call(&self, input: &PreRequestContext) -> HookPreRequestAction { - let active_tasks: Vec = self + let active_tasks: Vec = self .state .task_store .list() .into_iter() - .filter(|task| { - matches!( - task.status, - tools::TaskStatus::Pending | tools::TaskStatus::Inprogress - ) - }) + .filter(|task| matches!(task.status, TaskStatus::Pending | TaskStatus::Inprogress)) .collect(); if active_tasks.is_empty() { return HookPreRequestAction::Continue; @@ -252,7 +253,7 @@ fn is_task_management_tool(name: &str) -> bool { TASK_MANAGEMENT_TOOL_NAMES.contains(&name) } -fn render_task_reminder_body(active_tasks: &[tools::TaskEntry]) -> String { +fn render_task_reminder_body(active_tasks: &[TaskEntry]) -> String { let mut body = String::from( "Active session tasks are still open. If progress changed, call TaskUpdate.\n", ); @@ -441,7 +442,7 @@ mod tests { .taskid; feature .task_store() - .update(done, Some(tools::TaskStatus::Completed), None, None) + .update(done, Some(TaskStatus::Completed), None, None) .expect("complete task"); let hook = TaskReminderPreRequestHook { state: Arc::clone(&feature.state), diff --git a/crates/tools/src/task.rs b/crates/pod/src/feature/builtin/task/store.rs similarity index 58% rename from crates/tools/src/task.rs rename to crates/pod/src/feature/builtin/task/store.rs index 1a9e1478..dee5e53c 100644 --- a/crates/tools/src/task.rs +++ b/crates/pod/src/feature/builtin/task/store.rs @@ -1,15 +1,12 @@ -//! Session-lifetime TaskStore and builtin task tools. +//! Task domain state and snapshot/replay support. //! -//! The store survives compaction and Pod restart — it is reconstructed -//! on resume by replaying TaskCreate / TaskUpdate tool-call arguments -//! from persisted history, so its effective lifetime is the -//! [`session_store::SessionId`] (the conversation), not the Pod process. +//! The store survives compaction and Pod restart by replaying TaskCreate / +//! TaskUpdate tool-call arguments and compacted TaskStore snapshots from +//! persisted history. use std::sync::{Arc, Mutex}; -use async_trait::async_trait; use llm_worker::Item; -use llm_worker::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] @@ -145,12 +142,16 @@ impl TaskStore { name, arguments, .. } => match name.as_str() { "TaskCreate" => { - if let Ok(params) = serde_json::from_str::(arguments) { + if let Ok(params) = + serde_json::from_str::(arguments) + { let _ = self.create(params.subject, params.description); } } "TaskUpdate" => { - if let Ok(params) = serde_json::from_str::(arguments) { + if let Ok(params) = + serde_json::from_str::(arguments) + { let _ = self.update( params.taskid, params.status, @@ -186,7 +187,8 @@ impl TaskStore { } pub fn snapshot_text(&self) -> String { - render_snapshot(&self.list()) + let snapshot = self.snapshot(); + render_snapshot(&snapshot.tasks) } } @@ -209,24 +211,14 @@ impl std::fmt::Display for TaskStoreError { impl std::error::Error for TaskStoreError {} -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct TaskCreateParams { - /// One-line task subject. +#[derive(Debug, Deserialize)] +struct ReplayTaskCreateParams { subject: String, - /// Detailed task description. description: String, } -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct TaskListParams {} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct TaskGetParams { - taskid: u64, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct TaskUpdateParams { +#[derive(Debug, Deserialize)] +struct ReplayTaskUpdateParams { taskid: u64, #[serde(default)] status: Option, @@ -236,130 +228,6 @@ struct TaskUpdateParams { description: Option, } -struct TaskCreateTool { - store: TaskStore, -} - -struct TaskListTool { - store: TaskStore, -} - -struct TaskGetTool { - store: TaskStore, -} - -struct TaskUpdateTool { - store: TaskStore, -} - -const CREATE_DESCRIPTION: &str = "Create a session-lifetime task only when user-visible \ -progress tracking is genuinely useful: multiple active tasks must be remembered, or the work \ -will involve long edits, long-running commands, extended investigation, or interruption-prone \ -coordination. Do not create a task just because a request has several steps, and do not create \ -one for short questions, quick checks, single reviews, or one-off commands. Prefer updating an \ -existing active task over creating a duplicate. Input only `subject` and `description`; `taskid` \ -is assigned automatically and initial `status` is `pending`."; -const LIST_DESCRIPTION: &str = "List every session-lifetime task, including completed and \ -deleted entries. Tasks are user-visible real-time status for short-term current-work tracking. \ -Takes an empty object as input."; -const GET_DESCRIPTION: &str = "Get one session-lifetime task by `taskid`. Tasks are \ -user-visible real-time status for short-term current-work tracking. Returns an error if the task \ -does not exist."; -const UPDATE_DESCRIPTION: &str = "Update an existing session-lifetime task when meaningful \ -progress changes between substantial steps. Tasks are user-visible real-time status, so avoid \ -churn for trivial substeps. Keep status current with `pending`, `inprogress`, `completed`, or \ -`deleted`. Provide `taskid` and at least one of `status`, `subject`, or `description`; deletion is \ -logical (`status = deleted`). If an unexpected problem blocks progress, do not force the next \ -step: leave the task as-is, summarize the problem to the user, and end the turn."; - -#[async_trait] -impl Tool for TaskCreateTool { - async fn execute(&self, input_json: &str) -> Result { - let params: TaskCreateParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskCreate input: {e}")))?; - let created = self.store.create(params.subject, params.description); - let tasks = self.store.list(); - Ok(task_output( - format!( - "Created task {} ({})\n{}", - created.taskid, - created.status, - snapshot_overview(&tasks) - ), - &created, - &tasks, - )) - } -} - -#[async_trait] -impl Tool for TaskListTool { - async fn execute(&self, input_json: &str) -> Result { - let _: TaskListParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskList input: {e}")))?; - let tasks = self.store.list(); - Ok(ToolOutput { - summary: snapshot_overview(&tasks), - content: Some(render_snapshot(&tasks)), - }) - } -} - -#[async_trait] -impl Tool for TaskGetTool { - async fn execute(&self, input_json: &str) -> Result { - let params: TaskGetParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskGet input: {e}")))?; - let task = self.store.get(params.taskid).ok_or_else(|| { - ToolError::ExecutionFailed(format!("taskid {} not found", params.taskid)) - })?; - let content = serde_json::to_string_pretty(&task).unwrap_or_else(|_| format!("{task:?}")); - Ok(ToolOutput { - summary: format!("Task {} ({}) {}", task.taskid, task.status, task.subject), - content: Some(content), - }) - } -} - -#[async_trait] -impl Tool for TaskUpdateTool { - async fn execute(&self, input_json: &str) -> Result { - let params: TaskUpdateParams = serde_json::from_str(input_json) - .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskUpdate input: {e}")))?; - let updated = self - .store - .update( - params.taskid, - params.status, - params.subject, - params.description, - ) - .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; - let tasks = self.store.list(); - Ok(task_output( - format!( - "Updated task {} ({})\n{}", - updated.taskid, - updated.status, - snapshot_overview(&tasks) - ), - &updated, - &tasks, - )) - } -} - -fn task_output(summary: String, task: &TaskEntry, tasks: &[TaskEntry]) -> ToolOutput { - let content = serde_json::json!({ - "task": task, - "snapshot": { "tasks": tasks }, - }); - ToolOutput { - summary, - content: Some(serde_json::to_string_pretty(&content).unwrap_or_default()), - } -} - pub fn snapshot_overview(tasks: &[TaskEntry]) -> String { let pending = tasks .iter() @@ -392,7 +260,7 @@ pub fn render_snapshot(tasks: &[TaskEntry]) -> String { format!("{}\n\n```json\n{}\n```\n", snapshot_overview(tasks), json) } -fn parse_compact_snapshot_text(text: &str) -> Option> { +pub(super) fn parse_compact_snapshot_text(text: &str) -> Option> { if !text.starts_with("[Session TaskStore snapshot]") { return None; } @@ -405,131 +273,10 @@ fn parse_compact_snapshot_text(text: &str) -> Option> { Some(snapshot.tasks) } -fn task_create_tool(store: TaskStore) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(TaskCreateParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("TaskCreate") - .description(CREATE_DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(TaskCreateTool { - store: store.clone(), - }); - (meta, tool) - }) -} - -fn task_list_tool(store: TaskStore) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(TaskListParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("TaskList") - .description(LIST_DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(TaskListTool { - store: store.clone(), - }); - (meta, tool) - }) -} - -fn task_get_tool(store: TaskStore) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(TaskGetParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("TaskGet") - .description(GET_DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(TaskGetTool { - store: store.clone(), - }); - (meta, tool) - }) -} - -fn task_update_tool(store: TaskStore) -> ToolDefinition { - Arc::new(move || { - let schema = schemars::schema_for!(TaskUpdateParams); - let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); - let meta = ToolMeta::new("TaskUpdate") - .description(UPDATE_DESCRIPTION) - .input_schema(schema_value); - let tool: Arc = Arc::new(TaskUpdateTool { - store: store.clone(), - }); - (meta, tool) - }) -} - -pub fn task_tools(store: TaskStore) -> Vec { - vec![ - task_create_tool(store.clone()), - task_list_tool(store.clone()), - task_get_tool(store.clone()), - task_update_tool(store), - ] -} - #[cfg(test)] mod tests { use super::*; - fn tool(def: ToolDefinition) -> Arc { - let (_, tool) = def(); - tool - } - - #[tokio::test] - async fn task_tools_create_list_get_update() { - let store = TaskStore::new(); - let create = tool(task_create_tool(store.clone())); - let list = tool(task_list_tool(store.clone())); - let get = tool(task_get_tool(store.clone())); - let update = tool(task_update_tool(store.clone())); - - let out = create - .execute(r#"{"subject":"implement","description":"write code"}"#) - .await - .unwrap(); - assert!(out.summary.contains("Created task 1")); - assert_eq!(store.get(1).unwrap().status, TaskStatus::Pending); - - let out = update - .execute(r#"{"taskid":1,"status":"inprogress","subject":"implement tasks"}"#) - .await - .unwrap(); - assert!(out.summary.contains("Updated task 1")); - let task = store.get(1).unwrap(); - assert_eq!(task.status, TaskStatus::Inprogress); - assert_eq!(task.subject, "implement tasks"); - - let out = get.execute(r#"{"taskid":1}"#).await.unwrap(); - assert!(out.summary.contains("Task 1 (inprogress)")); - assert!(out.content.unwrap().contains("implement tasks")); - - let out = list.execute("{}").await.unwrap(); - assert!(out.summary.contains("1 task(s)")); - let content = out.content.unwrap(); - assert!(content.contains("\"taskid\": 1")); - assert!(content.contains("```json")); - } - - #[tokio::test] - async fn task_update_validates_existing_and_at_least_one_field() { - let store = TaskStore::new(); - store.create("s".into(), "d".into()); - let update = tool(task_update_tool(store)); - - let err = update.execute(r#"{"taskid":1}"#).await.unwrap_err(); - assert!(err.to_string().contains("at least one")); - - let err = update - .execute(r#"{"taskid":99,"status":"deleted"}"#) - .await - .unwrap_err(); - assert!(err.to_string().contains("taskid 99 not found")); - } - #[test] fn replay_history_reconstructs_store_and_ignores_malformed_calls() { let history = vec![ diff --git a/crates/pod/src/feature/builtin/task/tool_impl.rs b/crates/pod/src/feature/builtin/task/tool_impl.rs new file mode 100644 index 00000000..9ef9e221 --- /dev/null +++ b/crates/pod/src/feature/builtin/task/tool_impl.rs @@ -0,0 +1,285 @@ +//! Task built-in tool implementations. + +use std::sync::Arc; + +use async_trait::async_trait; +use llm_worker::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; +use serde::Deserialize; + +use super::store::{TaskEntry, TaskStatus, TaskStore, render_snapshot, snapshot_overview}; + +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TaskCreateParams { + /// One-line task subject. + subject: String, + /// Detailed task description. + description: String, +} + +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TaskListParams {} + +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TaskGetParams { + taskid: u64, +} + +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TaskUpdateParams { + taskid: u64, + #[serde(default)] + status: Option, + #[serde(default)] + subject: Option, + #[serde(default)] + description: Option, +} + +struct TaskCreateTool { + store: TaskStore, +} + +struct TaskListTool { + store: TaskStore, +} + +struct TaskGetTool { + store: TaskStore, +} + +struct TaskUpdateTool { + store: TaskStore, +} + +const CREATE_DESCRIPTION: &str = "Create a session-lifetime task only when user-visible \ +progress tracking is genuinely useful: multiple active tasks must be remembered, or the work \ +will involve long edits, long-running commands, extended investigation, or interruption-prone \ +coordination. Do not create a task just because a request has several steps, and do not create \ +one for short questions, quick checks, single reviews, or one-off commands. Prefer updating an \ +existing active task over creating a duplicate. Input only `subject` and `description`; `taskid` \ +is assigned automatically and initial `status` is `pending`."; +const LIST_DESCRIPTION: &str = "List every session-lifetime task, including completed and \ +deleted entries. Tasks are user-visible real-time status for short-term current-work tracking. \ +Takes an empty object as input."; +const GET_DESCRIPTION: &str = "Get one session-lifetime task by `taskid`. Tasks are \ +user-visible real-time status for short-term current-work tracking. Returns an error if the task \ +does not exist."; +const UPDATE_DESCRIPTION: &str = "Update an existing session-lifetime task when meaningful \ +progress changes between substantial steps. Tasks are user-visible real-time status, so avoid \ +churn for trivial substeps. Keep status current with `pending`, `inprogress`, `completed`, or \ +`deleted`. Provide `taskid` and at least one of `status`, `subject`, or `description`; deletion is \ +logical (`status = deleted`). If an unexpected problem blocks progress, do not force the next \ +step: leave the task as-is, summarize the problem to the user, and end the turn."; + +#[async_trait] +impl Tool for TaskCreateTool { + async fn execute(&self, input_json: &str) -> Result { + let params: TaskCreateParams = serde_json::from_str(input_json) + .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskCreate input: {e}")))?; + let created = self.store.create(params.subject, params.description); + let tasks = self.store.list(); + Ok(task_output( + format!( + "Created task {} ({})\n{}", + created.taskid, + created.status, + snapshot_overview(&tasks) + ), + &created, + &tasks, + )) + } +} + +#[async_trait] +impl Tool for TaskListTool { + async fn execute(&self, input_json: &str) -> Result { + let _: TaskListParams = serde_json::from_str(input_json) + .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskList input: {e}")))?; + let tasks = self.store.list(); + Ok(ToolOutput { + summary: snapshot_overview(&tasks), + content: Some(render_snapshot(&tasks)), + }) + } +} + +#[async_trait] +impl Tool for TaskGetTool { + async fn execute(&self, input_json: &str) -> Result { + let params: TaskGetParams = serde_json::from_str(input_json) + .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskGet input: {e}")))?; + let task = self.store.get(params.taskid).ok_or_else(|| { + ToolError::ExecutionFailed(format!("taskid {} not found", params.taskid)) + })?; + let content = serde_json::to_string_pretty(&task).unwrap_or_else(|_| format!("{task:?}")); + Ok(ToolOutput { + summary: format!("Task {} ({}) {}", task.taskid, task.status, task.subject), + content: Some(content), + }) + } +} + +#[async_trait] +impl Tool for TaskUpdateTool { + async fn execute(&self, input_json: &str) -> Result { + let params: TaskUpdateParams = serde_json::from_str(input_json) + .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskUpdate input: {e}")))?; + let updated = self + .store + .update( + params.taskid, + params.status, + params.subject, + params.description, + ) + .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; + let tasks = self.store.list(); + Ok(task_output( + format!( + "Updated task {} ({})\n{}", + updated.taskid, + updated.status, + snapshot_overview(&tasks) + ), + &updated, + &tasks, + )) + } +} + +fn task_output(summary: String, task: &TaskEntry, tasks: &[TaskEntry]) -> ToolOutput { + let content = serde_json::json!({ + "task": task, + "snapshot": { "tasks": tasks }, + }); + ToolOutput { + summary, + content: Some(serde_json::to_string_pretty(&content).unwrap_or_default()), + } +} +fn task_create_tool(store: TaskStore) -> ToolDefinition { + Arc::new(move || { + let schema = schemars::schema_for!(TaskCreateParams); + let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); + let meta = ToolMeta::new("TaskCreate") + .description(CREATE_DESCRIPTION) + .input_schema(schema_value); + let tool: Arc = Arc::new(TaskCreateTool { + store: store.clone(), + }); + (meta, tool) + }) +} + +fn task_list_tool(store: TaskStore) -> ToolDefinition { + Arc::new(move || { + let schema = schemars::schema_for!(TaskListParams); + let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); + let meta = ToolMeta::new("TaskList") + .description(LIST_DESCRIPTION) + .input_schema(schema_value); + let tool: Arc = Arc::new(TaskListTool { + store: store.clone(), + }); + (meta, tool) + }) +} + +fn task_get_tool(store: TaskStore) -> ToolDefinition { + Arc::new(move || { + let schema = schemars::schema_for!(TaskGetParams); + let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); + let meta = ToolMeta::new("TaskGet") + .description(GET_DESCRIPTION) + .input_schema(schema_value); + let tool: Arc = Arc::new(TaskGetTool { + store: store.clone(), + }); + (meta, tool) + }) +} + +fn task_update_tool(store: TaskStore) -> ToolDefinition { + Arc::new(move || { + let schema = schemars::schema_for!(TaskUpdateParams); + let schema_value = serde_json::to_value(schema).unwrap_or(serde_json::json!({})); + let meta = ToolMeta::new("TaskUpdate") + .description(UPDATE_DESCRIPTION) + .input_schema(schema_value); + let tool: Arc = Arc::new(TaskUpdateTool { + store: store.clone(), + }); + (meta, tool) + }) +} + +pub(crate) fn task_tools(store: TaskStore) -> Vec { + vec![ + task_create_tool(store.clone()), + task_list_tool(store.clone()), + task_get_tool(store.clone()), + task_update_tool(store), + ] +} + +#[cfg(test)] +mod tests { + use super::*; + + fn tool(def: ToolDefinition) -> Arc { + let (_, tool) = def(); + tool + } + + #[tokio::test] + async fn task_tools_create_list_get_update() { + let store = TaskStore::new(); + let create = tool(task_create_tool(store.clone())); + let list = tool(task_list_tool(store.clone())); + let get = tool(task_get_tool(store.clone())); + let update = tool(task_update_tool(store.clone())); + + let out = create + .execute(r#"{"subject":"implement","description":"write code"}"#) + .await + .unwrap(); + assert!(out.summary.contains("Created task 1")); + assert_eq!(store.get(1).unwrap().status, TaskStatus::Pending); + + let out = update + .execute(r#"{"taskid":1,"status":"inprogress","subject":"implement tasks"}"#) + .await + .unwrap(); + assert!(out.summary.contains("Updated task 1")); + let task = store.get(1).unwrap(); + assert_eq!(task.status, TaskStatus::Inprogress); + assert_eq!(task.subject, "implement tasks"); + + let out = get.execute(r#"{"taskid":1}"#).await.unwrap(); + assert!(out.summary.contains("Task 1 (inprogress)")); + assert!(out.content.unwrap().contains("implement tasks")); + + let out = list.execute("{}").await.unwrap(); + assert!(out.summary.contains("1 task(s)")); + let content = out.content.unwrap(); + assert!(content.contains("\"taskid\": 1")); + assert!(content.contains("```json")); + } + + #[tokio::test] + async fn task_update_validates_existing_and_at_least_one_field() { + let store = TaskStore::new(); + store.create("s".into(), "d".into()); + let update = tool(task_update_tool(store)); + + let err = update.execute(r#"{"taskid":1}"#).await.unwrap_err(); + assert!(err.to_string().contains("at least one")); + + let err = update + .execute(r#"{"taskid":99,"status":"deleted"}"#) + .await + .unwrap_err(); + assert!(err.to_string().contains("taskid 99 not found")); + } +} diff --git a/crates/pod/src/ipc/interceptor.rs b/crates/pod/src/ipc/interceptor.rs index 74ba4280..a73aeafb 100644 --- a/crates/pod/src/ipc/interceptor.rs +++ b/crates/pod/src/ipc/interceptor.rs @@ -473,13 +473,15 @@ mod tests { } fn task_tool_call_info(name: &str, input: serde_json::Value) -> ToolCallInfo { - let def = tools::task_tools(tools::TaskStore::new()) - .into_iter() - .find(|def| { - let (meta, _) = def(); - meta.name == name - }) - .expect("task tool definition"); + let def = crate::feature::builtin::task::task_tools( + crate::feature::builtin::task::TaskStore::new(), + ) + .into_iter() + .find(|def| { + let (meta, _) = def(); + meta.name == name + }) + .expect("task tool definition"); let (meta, tool) = def(); ToolCallInfo { call: llm_worker::tool::ToolCall { diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index 7f76e4d2..fe2af16a 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -12,7 +12,7 @@ //! Recreated fresh on each Pod start (including resume). //! //! The Pod layer owns both instances and passes them to -//! [`builtin_tools`] when registering tools on a `Worker`. +//! [`core_builtin_tools`] when registering tools on a `Worker`. //! //! `Bash` is the lone exception — its child processes bypass `ScopedFs` //! entirely. Safety for arbitrary command execution is delegated to the @@ -20,7 +20,6 @@ pub mod error; pub mod scoped_fs; -pub mod task; pub mod tracker; mod bash; @@ -38,7 +37,6 @@ pub use glob::glob_tool; pub use grep::grep_tool; pub use read::read_tool; pub use scoped_fs::ScopedFs; -pub use task::{TaskEntry, TaskSnapshot, TaskStatus, TaskStore, task_tools}; pub use tracker::Tracker; pub use web::{web_fetch_tool, web_search_tool}; pub use write::write_tool; @@ -72,17 +70,3 @@ pub fn core_builtin_tools( web_fetch_tool(web::WebTools::new(web_config)), ] } - -/// Register all builtin tools, including task tools, for callers that are not -/// using the Pod feature registry path. -pub fn builtin_tools( - fs: ScopedFs, - tracker: Tracker, - task_store: TaskStore, - bash_output_dir: std::path::PathBuf, - web_config: Option, -) -> Vec { - let mut defs = core_builtin_tools(fs, tracker, bash_output_dir, web_config); - defs.extend(task_tools(task_store)); - defs -} diff --git a/crates/tools/src/tracker.rs b/crates/tools/src/tracker.rs index a79d91c4..fcfdbe2e 100644 --- a/crates/tools/src/tracker.rs +++ b/crates/tools/src/tracker.rs @@ -29,13 +29,12 @@ //! ```no_run //! # use std::path::PathBuf; //! # use manifest::Scope; -//! # use tools::{ScopedFs, Tracker, builtin_tools}; +//! # use tools::{ScopedFs, Tracker, core_builtin_tools}; //! let scope = Scope::writable("/workspace").unwrap(); //! let fs = ScopedFs::new(scope, PathBuf::from("/workspace")); // pod lifetime //! let tracker = Tracker::new(); // session lifetime //! let bash_outputs = PathBuf::from("/run/yoi/bash-output"); -//! let task_store = tools::TaskStore::new(); -//! let defs = builtin_tools(fs, tracker, task_store, bash_outputs, None); +//! let defs = core_builtin_tools(fs, tracker, bash_outputs, None); //! ``` use std::collections::{HashMap, VecDeque}; diff --git a/crates/tools/tests/edge_cases.rs b/crates/tools/tests/edge_cases.rs index cc9afa96..181e2f83 100644 --- a/crates/tools/tests/edge_cases.rs +++ b/crates/tools/tests/edge_cases.rs @@ -6,7 +6,7 @@ use llm_worker::tool::{Tool, ToolDefinition}; use manifest::{Permission, Scope, ScopeConfig, ScopeRule}; use serde_json::json; use tempfile::TempDir; -use tools::{ScopedFs, TaskStore, Tracker, builtin_tools}; +use tools::{ScopedFs, Tracker, core_builtin_tools}; struct Registry { entries: Vec<(llm_worker::tool::ToolMeta, Arc)>, @@ -43,10 +43,9 @@ fn setup() -> (TempDir, TempDir, Registry) { let scope = Scope::from_config(&config).unwrap(); let fs = ScopedFs::new(scope, dir.path().to_path_buf()); let tracker = Tracker::new(); - let reg = Registry::new(builtin_tools( + let reg = Registry::new(core_builtin_tools( fs, tracker, - TaskStore::new(), spill.path().to_path_buf(), None, )); diff --git a/crates/tools/tests/integration.rs b/crates/tools/tests/integration.rs index 4ec21598..13cef5df 100644 --- a/crates/tools/tests/integration.rs +++ b/crates/tools/tests/integration.rs @@ -1,4 +1,4 @@ -//! Cross-tool integration tests exercising `builtin_tools()` end-to-end. +//! Cross-tool integration tests exercising `core_builtin_tools()` end-to-end. //! //! `ToolServerHandle::register_tool` / `flush_pending` are `pub(crate)` in //! llm-worker, so from here we exercise the factories directly — the same @@ -11,7 +11,7 @@ use llm_worker::tool::{Tool, ToolDefinition, ToolMeta}; use manifest::{Permission, Scope, ScopeConfig, ScopeRule}; use serde_json::json; use tempfile::TempDir; -use tools::{ScopedFs, TaskStore, Tracker, builtin_tools}; +use tools::{ScopedFs, Tracker, core_builtin_tools}; fn scope_with_spill(workspace: &Path, spill: &Path) -> Scope { let base = Scope::writable(workspace).unwrap(); @@ -56,10 +56,9 @@ fn setup() -> (TempDir, TempDir, Registry) { let scope = scope_with_spill(dir.path(), spill.path()); let fs = ScopedFs::new(scope, dir.path().to_path_buf()); let tracker = Tracker::new(); - let reg = Registry::new(builtin_tools( + let reg = Registry::new(core_builtin_tools( fs, tracker, - TaskStore::new(), spill.path().to_path_buf(), None, )); @@ -79,7 +78,7 @@ async fn call_err(tool: &Arc, input: serde_json::Value) -> llm_worker: } #[test] -fn builtin_tools_registers_full_set() { +fn core_builtin_tools_registers_full_set() { let (_dir, _spill, reg) = setup(); let mut names = reg.names(); names.sort(); @@ -91,10 +90,6 @@ fn builtin_tools_registers_full_set() { "Glob", "Grep", "Read", - "TaskCreate", - "TaskGet", - "TaskList", - "TaskUpdate", "WebFetch", "WebSearch", "Write" @@ -292,7 +287,7 @@ async fn edit_requires_read_across_tools() { #[tokio::test] async fn deterministic_tool_order_is_registration_order() { let (_dir, _spill, reg) = setup(); - // Registration order from builtin_tools(): Read, Write, Edit, Glob, Grep, Bash, WebSearch, WebFetch, TaskCreate, TaskList, TaskGet, TaskUpdate + // Registration order from core_builtin_tools(): Read, Write, Edit, Glob, Grep, Bash, WebSearch, WebFetch let names: Vec<&str> = reg.entries.iter().map(|(m, _)| m.name.as_str()).collect(); assert_eq!( names, @@ -305,10 +300,6 @@ async fn deterministic_tool_order_is_registration_order() { "Bash", "WebSearch", "WebFetch", - "TaskCreate", - "TaskList", - "TaskGet", - "TaskUpdate" ] ); } @@ -326,10 +317,6 @@ fn tool_names_match_reference_spec() { "Bash", "WebSearch", "WebFetch", - "TaskCreate", - "TaskList", - "TaskGet", - "TaskUpdate", ] { assert!( reg.entries.iter().any(|(m, _)| m.name == expected), @@ -346,10 +333,9 @@ async fn tracker_recent_files_tracks_read_write_edit() { let scope = scope_with_spill(dir.path(), spill.path()); let fs = ScopedFs::new(scope, dir.path().to_path_buf()); let tracker = Tracker::new(); - let reg = Registry::new(builtin_tools( + let reg = Registry::new(core_builtin_tools( fs, tracker.clone(), - TaskStore::new(), spill.path().to_path_buf(), None, )); diff --git a/crates/tui/Cargo.toml b/crates/tui/Cargo.toml index 28ee5c62..d34cce3d 100644 --- a/crates/tui/Cargo.toml +++ b/crates/tui/Cargo.toml @@ -25,4 +25,3 @@ llm-worker.workspace = true [dev-dependencies] tempfile = { workspace = true } -tools = { workspace = true } diff --git a/crates/tui/src/task.rs b/crates/tui/src/task.rs index 32cf6902..69ce9bf5 100644 --- a/crates/tui/src/task.rs +++ b/crates/tui/src/task.rs @@ -1,11 +1,11 @@ //! In-TUI mirror of the session-lifetime task store. //! -//! This deliberately does NOT depend on `tools::TaskStore`. The TUI is a +//! This deliberately does NOT depend on the Pod TaskStore. The TUI is a //! presentation layer; pulling in `tools` would drag along `llm-worker` //! and the whole tool surface. Instead we mirror the small subset we //! need: //! -//! - `TaskEntry` / `TaskStatus`: shaped to round-trip with `tools`'s JSON +//! - `TaskEntry` / `TaskStatus`: shaped to round-trip with Pod Task JSON //! serialization (`#[serde(rename_all = "lowercase")]` on the status, //! matching field names on the entry). //! - Just enough state machine to apply `TaskCreate` / `TaskUpdate` @@ -90,7 +90,7 @@ impl TaskStore { /// Apply a completed `TaskCreate` / `TaskUpdate` tool_call. Other /// tool names and unparseable JSON are silent no-ops, matching the - /// resilience of `tools::TaskStore::replay_history`. + /// resilience of the Pod TaskStore history replay. pub fn apply_tool_call(&mut self, name: &str, arguments: &str) { match name { "TaskCreate" => { @@ -313,22 +313,15 @@ mod tests { } } -/// Cross-crate contract tests. The TUI deliberately re-implements a -/// stripped-down mirror of `tools::TaskStore` instead of depending on -/// the real one (see `tickets/tui-task-display.md`). That decoupling -/// means a format change on the tools side — a renamed field on -/// `TaskEntry`, a different fence syntax in `render_snapshot`, a new -/// JSON wrapper — would silently leave the TUI parsing nothing instead -/// of failing loudly. -/// -/// These tests pull `tools` in as a dev-dependency so the contract is -/// exercised at CI time. If they fail, either the format genuinely -/// changed (update both sides) or the TUI mirror has drifted (re-sync -/// it). +/// Snapshot format compatibility tests. The TUI deliberately re-implements a +/// stripped-down TaskStore mirror instead of depending on the Pod Task feature; +/// it only consumes task tool calls and `[Session TaskStore snapshot]` system +/// messages. These fixtures encode the Pod-owned Task snapshot JSON/text shape +/// so accidental TUI parser drift still fails locally without making `tui` +/// depend on `pod` or `tools`. #[cfg(test)] -mod cross_format_contract { +mod snapshot_format_contract { use super::*; - use tools::task::{TaskStatus as ToolsTaskStatus, TaskStore as ToolsTaskStore}; /// Mirrors the envelope `Pod::try_pre_run_compact` wraps the raw /// snapshot text in. Hand-rolled here so the test fails loudly if @@ -341,16 +334,40 @@ mod cross_format_contract { ) } - fn tools_status_label(s: ToolsTaskStatus) -> &'static str { - match s { - ToolsTaskStatus::Pending => "pending", - ToolsTaskStatus::Inprogress => "inprogress", - ToolsTaskStatus::Completed => "completed", - ToolsTaskStatus::Deleted => "deleted", - } + fn snapshot_fixture() -> &'static str { + r#"TaskStore: 2 task(s) (pending: 0, inprogress: 1, completed: 1, deleted: 0) + +```json +{ + "tasks": [ + { + "taskid": 1, + "status": "inprogress", + "subject": "first", + "description": "first desc" + }, + { + "taskid": 2, + "status": "completed", + "subject": "second", + "description": "second desc with\nnewline" + } + ] +} +```"# } - fn tui_status_label(s: TaskStatus) -> &'static str { + fn empty_snapshot_fixture() -> &'static str { + r#"TaskStore: 0 task(s) (pending: 0, inprogress: 0, completed: 0, deleted: 0) + +```json +{ + "tasks": [] +} +```"# + } + + fn status_label(s: TaskStatus) -> &'static str { match s { TaskStatus::Pending => "pending", TaskStatus::Inprogress => "inprogress", @@ -360,61 +377,49 @@ mod cross_format_contract { } #[test] - fn tools_snapshot_text_round_trips_into_tui_store() { - let upstream = ToolsTaskStore::new(); - upstream.create("first".into(), "first desc".into()); - upstream.create("second".into(), "second desc with\nnewline".into()); - upstream - .update(1, Some(ToolsTaskStatus::Inprogress), None, None) - .expect("update 1"); - upstream - .update(2, Some(ToolsTaskStatus::Completed), None, None) - .expect("update 2"); - - let envelope = wrap_pod_style(&upstream.snapshot_text()); + fn pod_snapshot_text_round_trips_into_tui_store() { + let envelope = wrap_pod_style(snapshot_fixture()); let mut downstream = TaskStore::new(); downstream.apply_system_message_text(&envelope); - let upstream_tasks = upstream.list(); - let downstream_tasks = downstream.tasks(); - assert_eq!( - downstream_tasks.len(), - upstream_tasks.len(), - "TUI parsed wrong number of tasks — `tools::render_snapshot` shape may have shifted" - ); - for (u, d) in upstream_tasks.iter().zip(downstream_tasks.iter()) { - assert_eq!(d.taskid, u.taskid); - assert_eq!(d.subject, u.subject); - assert_eq!(d.description, u.description); - assert_eq!(tui_status_label(d.status), tools_status_label(u.status)); - } + let tasks = downstream.tasks(); + assert_eq!(tasks.len(), 2, "TUI parsed wrong number of tasks"); + assert_eq!(tasks[0].taskid, 1); + assert_eq!(tasks[0].subject, "first"); + assert_eq!(tasks[0].description, "first desc"); + assert_eq!(status_label(tasks[0].status), "inprogress"); + assert_eq!(tasks[1].taskid, 2); + assert_eq!(tasks[1].subject, "second"); + assert_eq!(tasks[1].description, "second desc with\nnewline"); + assert_eq!(status_label(tasks[1].status), "completed"); } #[test] - fn tools_taskentry_field_shape_deserializes_into_tui_taskentry() { - // A single `tools::TaskEntry` round-tripped through JSON. Field - // renames like `taskid` → `task_id` or status case changes on - // the tools side would surface here as a serde failure or a - // wrong-status assertion. - let upstream = ToolsTaskStore::new(); - let created = upstream.create("subj".into(), "desc".into()); - let json = serde_json::to_string(&created).expect("serialize tools::TaskEntry"); + fn taskentry_field_shape_deserializes_into_tui_taskentry() { + // A single Pod TaskEntry as JSON. Field renames like `taskid` → + // `task_id` or status case changes surface here as serde failures or + // wrong-status assertions. + let json = r#"{ + "taskid": 7, + "status": "pending", + "subject": "subj", + "description": "desc" +}"#; let parsed: TaskEntry = - serde_json::from_str(&json).expect("deserialize into tui::task::TaskEntry"); - assert_eq!(parsed.taskid, created.taskid); - assert_eq!(parsed.subject, created.subject); - assert_eq!(parsed.description, created.description); - assert_eq!(tui_status_label(parsed.status), "pending"); + serde_json::from_str(json).expect("deserialize into tui::task::TaskEntry"); + assert_eq!(parsed.taskid, 7); + assert_eq!(parsed.subject, "subj"); + assert_eq!(parsed.description, "desc"); + assert_eq!(status_label(parsed.status), "pending"); } #[test] - fn empty_tools_store_snapshot_is_recognised_by_tui() { - // Edge case: a freshly initialised TaskStore still produces a - // valid snapshot envelope. The TUI must parse it as "zero - // tasks", not silently fall through to no-op. - let upstream = ToolsTaskStore::new(); - let envelope = wrap_pod_style(&upstream.snapshot_text()); + fn empty_pod_task_snapshot_is_recognised_by_tui() { + // Edge case: a freshly initialised TaskStore still produces a valid + // snapshot envelope. The TUI must parse it as "zero tasks", not + // silently fall through to no-op. + let envelope = wrap_pod_style(empty_snapshot_fixture()); // Seed the TUI store with stale state to confirm replacement. let mut downstream = TaskStore::new(); diff --git a/package.nix b/package.nix index f44a6ce5..8d3dea1b 100644 --- a/package.nix +++ b/package.nix @@ -40,13 +40,13 @@ rustPlatform.buildRustPackage rec { filter = sourceFilter; }; - cargoHash = "sha256-f4/oOuPv4dUiwznX+popMjjDCXZQPBvqWRYmlJDyKkE="; + cargoHash = "sha256-iickLtGGmqc0raCZp7giowKajAMLn5+jwtQ9c5hZmhA="; depsExtraArgs = { - # nixpkgs 25.11's fetchCargoVendor still uses crates.io's API - # download endpoint in this environment, which returns 403 while the - # immutable static CDN endpoint works. Keep this local package build on - # static.crates.io until the upstream fetcher is fixed in our nixpkgs pin. + # Older fetchCargoVendor utilities used crates.io's API download endpoint, + # which returns 403 in this environment while the immutable static CDN + # endpoint works. Newer utilities already use static.crates.io, so patch + # only when the legacy endpoint is still present. buildPhase = '' runHook preBuild @@ -56,9 +56,11 @@ rustPlatform.buildRustPackage rec { vendor_util="$(command -v fetch-cargo-vendor-util-v2 || command -v fetch-cargo-vendor-util)" cp "$vendor_util" ./fetch-cargo-vendor-util-static - substituteInPlace ./fetch-cargo-vendor-util-static \ - --replace-fail 'https://crates.io/api/v1/crates/{pkg["name"]}/{pkg["version"]}/download' \ - 'https://static.crates.io/crates/{pkg["name"]}/{pkg["version"]}/download' + if grep -q 'https://crates.io/api/v1/crates/{pkg\["name"\]}/{pkg\["version"\]}/download' ./fetch-cargo-vendor-util-static; then + substituteInPlace ./fetch-cargo-vendor-util-static \ + --replace-fail 'https://crates.io/api/v1/crates/{pkg["name"]}/{pkg["version"]}/download' \ + 'https://static.crates.io/crates/{pkg["name"]}/{pkg["version"]}/download' + fi ./fetch-cargo-vendor-util-static create-vendor-staging ./Cargo.lock "$out" runHook postBuild