diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index 205410de..a6d608cb 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -3539,7 +3539,9 @@ mod completion_flow_tests { arguments: args.into(), }); } - assert!(app.task_store.tasks().is_empty()); + let tasks = app.task_store.tasks(); + assert_eq!(tasks.len(), 1); + assert_eq!(tasks[0].status, crate::task::TaskStatus::Completed); } #[test] diff --git a/crates/tui/src/task.rs b/crates/tui/src/task.rs index 537e7853..81e7f931 100644 --- a/crates/tui/src/task.rs +++ b/crates/tui/src/task.rs @@ -28,8 +28,8 @@ pub enum TaskStatus { } impl TaskStatus { - fn is_active(self) -> bool { - matches!(self, Self::Pending | Self::Inprogress) + fn is_retained(self) -> bool { + !matches!(self, Self::Deleted) } } @@ -125,7 +125,7 @@ impl TaskStore { if let Some(d) = p.description { t.description = d; } - if !t.status.is_active() { + if !t.status.is_retained() { self.tasks.remove(task_position); } } @@ -145,7 +145,7 @@ impl TaskStore { fn replace_with(&mut self, tasks: Vec) { let tasks: Vec<_> = tasks .into_iter() - .filter(|task| task.status.is_active()) + .filter(|task| task.status.is_retained()) .collect(); self.next_taskid = tasks .iter() @@ -194,7 +194,7 @@ fn parse_snapshot_text(text: &str) -> Option> { snapshot .tasks .into_iter() - .filter(|task| task.status.is_active()) + .filter(|task| task.status.is_retained()) .collect(), ) } @@ -241,7 +241,7 @@ mod tests { } #[test] - fn counts_tracks_only_active_tasks_after_completion() { + fn counts_retains_completed_tasks_until_deleted() { let mut s = TaskStore::new(); s.apply_tool_call("TaskCreate", r#"{"subject":"a","description":""}"#); s.apply_tool_call("TaskCreate", r#"{"subject":"b","description":""}"#); @@ -251,10 +251,13 @@ mod tests { let c = s.counts(); assert_eq!(c.pending, 1); assert_eq!(c.inprogress, 1); - assert_eq!(c.completed, 0); + assert_eq!(c.completed, 1); assert_eq!(c.deleted, 0); - assert_eq!(c.total(), 2); + assert_eq!(c.total(), 3); assert_eq!(c.active(), 2); + + s.apply_tool_call("TaskUpdate", r#"{"taskid":2,"status":"deleted"}"#); + assert!(s.tasks().iter().all(|task| task.taskid != 2)); } /// Snapshot text matches the wrapping `Worker::try_pre_run_compact` and the @@ -263,7 +266,7 @@ mod tests { fn wrap_snapshot(json_body: &str, overview: &str) -> String { format!( "[Session TaskStore snapshot]\n\n{overview}\n\n```json\n{json_body}\n```\n\n\ - This is the active session task list preserved across compaction. \ + This is the retained session task list preserved across compaction. \ The following TaskList tool result presents the same state through the tool lane." ) } @@ -288,19 +291,21 @@ mod tests { }"#; let text = wrap_snapshot( body, - "TaskStore: 1 active task(s) (pending: 1, inprogress: 0)", + "TaskStore: 2 task(s) (pending: 1, inprogress: 0, completed: 1)", ); let mut s = TaskStore::new(); s.apply_tool_call("TaskCreate", r#"{"subject":"stale","description":""}"#); s.apply_system_message_text(&text); let tasks = s.tasks(); - assert_eq!(tasks.len(), 1); - assert_eq!(tasks[0].taskid, 7); - assert_eq!(tasks[0].status, TaskStatus::Pending); - // Subsequent TaskCreate must continue beyond the highest active taskid + assert_eq!(tasks.len(), 2); + assert_eq!(tasks[0].taskid, 5); + assert_eq!(tasks[0].status, TaskStatus::Completed); + assert_eq!(tasks[1].taskid, 7); + assert_eq!(tasks[1].status, TaskStatus::Pending); + // Subsequent TaskCreate must continue beyond the highest retained taskid // observed in the snapshot. s.apply_tool_call("TaskCreate", r#"{"subject":"new","description":""}"#); - assert_eq!(s.tasks()[1].taskid, 8); + assert_eq!(s.tasks()[2].taskid, 8); } #[test] @@ -324,7 +329,7 @@ mod tests { } ] }"#; - let text = wrap_snapshot(body, "TaskStore: 1 active task(s)"); + let text = wrap_snapshot(body, "TaskStore: 1 task(s)"); let mut s = TaskStore::new(); s.apply_system_message_text(&text); let t = &s.tasks()[0]; @@ -349,13 +354,13 @@ mod snapshot_format_contract { fn wrap_pod_style(snapshot_text: &str) -> String { format!( "[Session TaskStore snapshot]\n\n{snapshot_text}\n\n\ - This is the active session task list preserved across compaction. \ + This is the retained session task list preserved across compaction. \ The following TaskList tool result presents the same state through the tool lane." ) } fn snapshot_fixture() -> &'static str { - r#"TaskStore: 1 active task(s) (pending: 0, inprogress: 1) + r#"TaskStore: 1 task(s) (pending: 0, inprogress: 1, completed: 0) ```json { @@ -372,7 +377,7 @@ mod snapshot_format_contract { } fn empty_snapshot_fixture() -> &'static str { - r#"TaskStore: 0 active task(s) (pending: 0, inprogress: 0) + r#"TaskStore: 0 task(s) (pending: 0, inprogress: 0, completed: 0) ```json { diff --git a/crates/worker/src/feature/builtin/task/mod.rs b/crates/worker/src/feature/builtin/task/mod.rs index 57c5e414..c0317983 100644 --- a/crates/worker/src/feature/builtin/task/mod.rs +++ b/crates/worker/src/feature/builtin/task/mod.rs @@ -15,8 +15,10 @@ mod store; mod tool_impl; pub(crate) use self::tool_impl::task_tools; +#[cfg(test)] +pub(crate) use store::TaskStatus; use store::snapshot_overview; -pub(crate) use store::{TaskEntry, TaskStatus, TaskStore}; +pub(crate) use store::{TaskEntry, TaskStore}; use crate::feature::{ FeatureDescriptor, FeatureHookPoint, FeatureInstallContext, FeatureInstallError, FeatureModule, @@ -76,7 +78,7 @@ impl TaskFeature { /// pointing at the same feature-owned store after rewind. pub fn restore_from_history(&self, history: &[Item]) { let restored = TaskStore::from_history(history); - self.state.task_store.replace_with(restored.list_active()); + self.state.task_store.replace_with(restored.list()); } /// Feature-owned snapshot text used by compaction to preserve Task state. @@ -86,7 +88,7 @@ impl TaskFeature { /// Feature-owned compact summary used for the synthetic TaskList result. pub fn snapshot_overview(&self) -> String { - snapshot_overview(&self.state.task_store.list_active()) + snapshot_overview(&self.state.task_store.list()) } #[cfg(test)] @@ -208,14 +210,8 @@ struct TaskReminderPreRequestHook { #[async_trait] impl Hook for TaskReminderPreRequestHook { async fn call(&self, input: &PreRequestContext) -> HookPreRequestAction { - let active_tasks: Vec = self - .state - .task_store - .list() - .into_iter() - .filter(|task| matches!(task.status, TaskStatus::Pending | TaskStatus::Inprogress)) - .collect(); - if active_tasks.is_empty() { + let tasks = self.state.task_store.list(); + if tasks.is_empty() { return HookPreRequestAction::Continue; } @@ -228,7 +224,7 @@ impl Hook for TaskReminderPreRequestHook { if let Some(system_items) = input.system_items() { self.state.reminder_state.note_reminder(); - system_items.append_task_reminder(render_task_reminder_body(&active_tasks)); + system_items.append_task_reminder(render_task_reminder_body(&tasks)); } HookPreRequestAction::Continue } @@ -252,11 +248,11 @@ fn is_task_management_tool(name: &str) -> bool { TASK_MANAGEMENT_TOOL_NAMES.contains(&name) } -fn render_task_reminder_body(active_tasks: &[TaskEntry]) -> String { +fn render_task_reminder_body(tasks: &[TaskEntry]) -> String { let mut body = String::from( - "Active session tasks are still open. If progress changed, call TaskUpdate.\n", + "Current session steps are listed below. Call TaskUpdate at each natural boundary: keep the current step inprogress, mark it completed when finished, and delete older completed steps once their result is no longer useful context.\n", ); - for task in active_tasks { + for task in tasks { body.push_str(&format!( "- taskid {} ({}) {}\n", task.taskid, task.status, task.subject @@ -433,7 +429,7 @@ mod tests { } #[tokio::test] - async fn task_reminder_is_silent_when_no_active_tasks_exist() { + async fn task_reminder_includes_completed_tasks_until_deleted() { let feature = TaskFeature::new(); let done = feature .task_store() @@ -448,10 +444,16 @@ mod tests { }; let pending = Arc::new(Mutex::new(Vec::new())); - for _ in 0..TASK_REMINDER_REQUEST_THRESHOLD * 2 { + for _ in 0..TASK_REMINDER_REQUEST_THRESHOLD { let _ = hook.call(&pre_request_context(Arc::clone(&pending))).await; - assert!(pending.lock().expect("pending queue poisoned").is_empty()); } + let queued = pending.lock().expect("pending queue poisoned"); + assert_eq!(queued.len(), 1); + let SystemItem::TaskReminder { body, .. } = &queued[0] else { + panic!("unexpected system item: {:?}", queued[0]); + }; + assert!(body.contains("taskid 1 (completed) done")); + assert!(body.contains("delete older completed steps")); } #[tokio::test] diff --git a/crates/worker/src/feature/builtin/task/store.rs b/crates/worker/src/feature/builtin/task/store.rs index 740b4d16..d341811f 100644 --- a/crates/worker/src/feature/builtin/task/store.rs +++ b/crates/worker/src/feature/builtin/task/store.rs @@ -19,8 +19,8 @@ pub enum TaskStatus { } impl TaskStatus { - pub fn is_active(self) -> bool { - matches!(self, Self::Pending | Self::Inprogress) + pub fn is_retained(self) -> bool { + !matches!(self, Self::Deleted) } } @@ -93,13 +93,6 @@ impl TaskStore { .clone() } - pub fn list_active(&self) -> Vec { - self.list() - .into_iter() - .filter(|task| task.status.is_active()) - .collect() - } - pub fn get(&self, taskid: u64) -> Option { self.inner .lock() @@ -137,7 +130,7 @@ impl TaskStore { task.description = description; } let updated = task.clone(); - if !updated.status.is_active() { + if !updated.status.is_retained() { inner.tasks.remove(task_position); } Ok(updated) @@ -149,7 +142,7 @@ impl TaskStore { pub fn snapshot_limited(&self, limit: usize) -> TaskSnapshot { TaskSnapshot { - tasks: self.list_active().into_iter().take(limit).collect(), + tasks: self.list().into_iter().take(limit).collect(), } } @@ -196,7 +189,7 @@ impl TaskStore { pub fn replace_with(&self, tasks: Vec) { let tasks: Vec<_> = tasks .into_iter() - .filter(|task| task.status.is_active()) + .filter(|task| task.status.is_retained()) .collect(); let next_taskid = tasks .iter() @@ -267,24 +260,30 @@ pub fn snapshot_overview(tasks: &[TaskEntry]) -> String { .iter() .filter(|t| t.status == TaskStatus::Inprogress) .count(); - let active = pending + inprogress; - format!("TaskStore: {active} active task(s) (pending: {pending}, inprogress: {inprogress})") + let completed = tasks + .iter() + .filter(|t| t.status == TaskStatus::Completed) + .count(); + format!( + "TaskStore: {} task(s) (pending: {pending}, inprogress: {inprogress}, completed: {completed})", + tasks.len() + ) } pub fn render_snapshot(tasks: &[TaskEntry]) -> String { - let active_tasks: Vec<_> = tasks + let retained_tasks: Vec<_> = tasks .iter() - .filter(|task| task.status.is_active()) + .filter(|task| task.status.is_retained()) .cloned() .collect(); let snapshot = TaskSnapshot { - tasks: active_tasks.clone(), + tasks: retained_tasks.clone(), }; let json = serde_json::to_string_pretty(&snapshot).unwrap_or_else(|_| String::from("{\"tasks\":[]}")); format!( "{}\n\n```json\n{}\n```\n", - snapshot_overview(&active_tasks), + snapshot_overview(&retained_tasks), json ) } @@ -303,7 +302,7 @@ pub(super) fn parse_compact_snapshot_text(text: &str) -> Option> snapshot .tasks .into_iter() - .filter(|task| task.status.is_active()) + .filter(|task| task.status.is_retained()) .collect(), ) } @@ -312,6 +311,22 @@ pub(super) fn parse_compact_snapshot_text(text: &str) -> Option> mod tests { use super::*; + #[test] + fn completed_task_is_retained_until_deleted() { + let store = TaskStore::new(); + store.create("step".into(), "description".into()); + store + .update(1, Some(TaskStatus::Completed), None, None) + .unwrap(); + assert_eq!(store.list()[0].status, TaskStatus::Completed); + + let deleted = store + .update(1, Some(TaskStatus::Deleted), None, None) + .unwrap(); + assert_eq!(deleted.status, TaskStatus::Deleted); + assert!(store.list().is_empty()); + } + #[test] fn replay_history_reconstructs_store_and_ignores_malformed_calls() { let history = vec![ @@ -323,9 +338,11 @@ mod tests { ]; let store = TaskStore::from_history(&history); let tasks = store.list(); - assert_eq!(tasks.len(), 1); + assert_eq!(tasks.len(), 2); assert_eq!(tasks[0].taskid, 1); assert_eq!(tasks[0].status, TaskStatus::Pending); + assert_eq!(tasks[1].taskid, 2); + assert_eq!(tasks[1].status, TaskStatus::Completed); } /// Wrap snapshot text the way `Worker::try_pre_run_compact` does, so tests @@ -333,7 +350,7 @@ mod tests { fn wrap_snapshot_system_message(snapshot: &str) -> String { format!( "[Session TaskStore snapshot]\n\n{snapshot}\n\n\ - This is the active session task list preserved across compaction. \ + This is the retained session task list preserved across compaction. \ The following TaskList tool result presents the same state through the tool lane." ) } @@ -355,9 +372,11 @@ mod tests { ]; let store = TaskStore::from_history(&history); let tasks = store.list(); - assert_eq!(tasks.len(), 1); - assert_eq!(tasks[0].taskid, 2); - assert_eq!(tasks[0].subject, "new"); + assert_eq!(tasks.len(), 2); + assert_eq!(tasks[0].taskid, 1); + assert_eq!(tasks[0].status, TaskStatus::Completed); + assert_eq!(tasks[1].taskid, 2); + assert_eq!(tasks[1].subject, "new"); } #[test] @@ -396,12 +415,15 @@ mod tests { ]; let store = TaskStore::from_history(&history); let tasks = store.list(); - assert_eq!(tasks.len(), 2); - assert_eq!(tasks[0].taskid, 2); - assert_eq!(tasks[0].subject, "B"); - assert_eq!(tasks[0].status, TaskStatus::Inprogress); - assert_eq!(tasks[1].taskid, 3); - assert_eq!(tasks[1].subject, "C"); + assert_eq!(tasks.len(), 3); + assert_eq!(tasks[0].taskid, 1); + assert_eq!(tasks[0].subject, "A"); + assert_eq!(tasks[0].status, TaskStatus::Completed); + assert_eq!(tasks[1].taskid, 2); + assert_eq!(tasks[1].subject, "B"); + assert_eq!(tasks[1].status, TaskStatus::Inprogress); + assert_eq!(tasks[2].taskid, 3); + assert_eq!(tasks[2].subject, "C"); } #[test] @@ -443,10 +465,10 @@ mod tests { let snapshot_text = pre.snapshot_text(); let system = Item::system_message(wrap_snapshot_system_message(&snapshot_text)); let call = Item::tool_call("compact-tasklist", "TaskList", "{}"); - let active_tasks = pre.list_active(); + let retained_tasks = pre.list(); let result = Item::tool_result_with_content( "compact-tasklist", - snapshot_overview(&active_tasks), + snapshot_overview(&retained_tasks), snapshot_text.clone(), ); @@ -455,7 +477,7 @@ mod tests { .as_text() .and_then(parse_compact_snapshot_text) .expect("system message should parse as snapshot"); - assert_eq!(extracted, active_tasks); + assert_eq!(extracted, retained_tasks); // The synthetic call/result pair shares one call_id and carries the // expected tool name + detailed content. @@ -482,6 +504,6 @@ mod tests { // Replaying the full triple reconstructs the same TaskStore. let store = TaskStore::from_history(&[system, call, result]); - assert_eq!(store.list(), active_tasks); + assert_eq!(store.list(), retained_tasks); } } diff --git a/crates/worker/src/feature/builtin/task/tool_impl.rs b/crates/worker/src/feature/builtin/task/tool_impl.rs index f733dc48..931ccbea 100644 --- a/crates/worker/src/feature/builtin/task/tool_impl.rs +++ b/crates/worker/src/feature/builtin/task/tool_impl.rs @@ -18,7 +18,7 @@ struct TaskCreateParams { #[derive(Debug, Deserialize, schemars::JsonSchema)] struct TaskListParams { - /// Maximum number of active tasks to return. Defaults to 20. + /// Maximum number of retained tasks to return. Defaults to 20. #[serde(default)] limit: Option, } @@ -55,23 +55,28 @@ 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 active session-lifetime tasks. Completed and deleted tasks are forgotten and omitted. Defaults to 20 tasks unless `limit` is provided."; +const CREATE_DESCRIPTION: &str = "Before beginning nontrivial work, decompose the predictable \ +portion into a small ordered set of natural, user-visible steps and create one session-lifetime task \ +per step. A natural step has a distinct verifiable outcome, such as investigation, implementation, \ +or validation; do not create a task for every command or mechanical substep. Do not collapse several \ +predictable steps into one umbrella task. Keep future steps pending, mark the current step inprogress, \ +and add or revise tasks when new information changes the plan. Skip task tracking for short questions, \ +quick checks, and one-off commands. Input only `subject` and `description`; `taskid` is assigned \ +automatically and initial `status` is `pending`."; +const LIST_DESCRIPTION: &str = "List retained session-lifetime tasks, including completed steps that \ +have not yet been deleted. Deleted tasks are forgotten and omitted. Defaults to 20 tasks unless \ +`limit` is provided."; 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."; +const UPDATE_DESCRIPTION: &str = "Update task status at each natural step boundary. Mark the \ +finished current step completed before advancing the next pending step to inprogress. Completed tasks \ +remain visible as recent workflow context; at a later natural boundary, delete older completed tasks \ +whose result is no longer useful. Do not jump directly from active to deleted merely to hide progress, \ +and avoid churn for mechanical 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 { @@ -83,7 +88,7 @@ impl Tool for TaskCreateTool { 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_active(); + let tasks = self.store.list(); Ok(task_output( format!( "Created task {} ({})\n{}", @@ -106,10 +111,10 @@ impl Tool for TaskListTool { let params: TaskListParams = serde_json::from_str(input_json) .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskList input: {e}")))?; let limit = params.limit.unwrap_or(DEFAULT_TASK_LIST_LIMIT); - let active_tasks = self.store.list_active(); - let tasks: Vec<_> = active_tasks.iter().take(limit).cloned().collect(); + let retained_tasks = self.store.list(); + let tasks: Vec<_> = retained_tasks.iter().take(limit).cloned().collect(); Ok(ToolOutput { - summary: list_overview(active_tasks.len(), tasks.len()), + summary: list_overview(retained_tasks.len(), tasks.len()), content: Some(render_task_list(&tasks)), attachments: Vec::new(), @@ -157,7 +162,7 @@ impl Tool for TaskUpdateTool { params.description, ) .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; - let tasks = self.store.list_active(); + let tasks = self.store.list(); Ok(task_output( format!( "Updated task {} ({})\n{}", @@ -179,14 +184,14 @@ fn task_output(summary: String, task: &TaskEntry) -> ToolOutput { } } -fn list_overview(total_active: usize, returned: usize) -> String { - if returned < total_active { +fn list_overview(total: usize, returned: usize) -> String { + if returned < total { format!( - "TaskStore: {returned} active task(s) shown; {} omitted.", - total_active - returned + "TaskStore: {returned} task(s) shown; {} omitted.", + total - returned ) } else { - format!("TaskStore: {returned} active task(s)") + format!("TaskStore: {returned} task(s)") } } @@ -310,7 +315,7 @@ mod tests { assert!(out.content.unwrap().contains("implement tasks")); let out = list.execute("{}", Default::default()).await.unwrap(); - assert!(out.summary.contains("1 active task(s)")); + assert!(out.summary.contains("1 task(s)")); let content = out.content.unwrap(); assert!(content.contains("\"taskid\": 1")); assert!(!content.contains("\"limit\"")); @@ -318,7 +323,7 @@ mod tests { } #[tokio::test] - async fn task_list_omits_completed_deleted_and_defaults_to_twenty() { + async fn task_list_retains_completed_omits_deleted_and_defaults_to_twenty() { let store = TaskStore::new(); let create = tool(task_create_tool(store.clone())); let update = tool(task_update_tool(store.clone())); @@ -343,10 +348,7 @@ mod tests { .unwrap(); let out = list.execute("{}", Default::default()).await.unwrap(); - assert_eq!( - out.summary, - "TaskStore: 20 active task(s) shown; 3 omitted." - ); + assert_eq!(out.summary, "TaskStore: 20 task(s) shown; 4 omitted."); let content = out.content.unwrap(); let json: serde_json::Value = serde_json::from_str(&content).unwrap(); let tasks = json.as_array().unwrap(); @@ -355,7 +357,7 @@ mod tests { .iter() .map(|task| task["taskid"].as_u64().unwrap()) .collect(); - assert!(!ids.contains(&1)); + assert!(ids.contains(&1)); assert!(!ids.contains(&2)); assert!(!content.contains("\"limit\"")); assert!(!content.contains("\"total_active\"")); @@ -365,10 +367,7 @@ mod tests { .execute(r#"{"limit":3}"#, Default::default()) .await .unwrap(); - assert_eq!( - out.summary, - "TaskStore: 3 active task(s) shown; 20 omitted." - ); + assert_eq!(out.summary, "TaskStore: 3 task(s) shown; 21 omitted."); let content = out.content.unwrap(); let json: serde_json::Value = serde_json::from_str(&content).unwrap(); assert_eq!(json.as_array().unwrap().len(), 3); @@ -412,6 +411,16 @@ mod tests { assert!(!content.contains("completed task")); } + #[test] + fn task_descriptions_require_natural_steps_and_completed_cleanup() { + assert!(CREATE_DESCRIPTION.contains("Before beginning nontrivial work")); + assert!(CREATE_DESCRIPTION.contains("one session-lifetime task per step")); + assert!(CREATE_DESCRIPTION.contains("Do not collapse several predictable steps")); + assert!(UPDATE_DESCRIPTION.contains("each natural step boundary")); + assert!(UPDATE_DESCRIPTION.contains("remain visible")); + assert!(UPDATE_DESCRIPTION.contains("delete older completed tasks")); + } + #[tokio::test] async fn task_update_validates_existing_and_at_least_one_field() { let store = TaskStore::new(); diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index bab0c157..2b333b7c 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -3347,7 +3347,7 @@ impl Worker { } let task_snapshot_message = Item::system_message(format!( "[Session TaskStore snapshot]\n\n{task_snapshot_text}\n\n\ - This is the active session task list preserved across compaction. \ + This is the retained session task list preserved across compaction. \ The following TaskList tool result presents the same state through the tool lane." )); compact_introduced_system_messages.push(task_snapshot_message.clone()); diff --git a/web/workspace/src/lib/workspace/console/tasks.test.ts b/web/workspace/src/lib/workspace/console/tasks.test.ts index 084453c8..ac7870bb 100644 --- a/web/workspace/src/lib/workspace/console/tasks.test.ts +++ b/web/workspace/src/lib/workspace/console/tasks.test.ts @@ -53,7 +53,7 @@ Deno.test("TaskCreate and TaskUpdate mirror the active TUI task store", () => { assert(counts.inprogress === 1, "one task should be in progress"); }); -Deno.test("completed and deleted tasks remain in the TUI-style store", () => { +Deno.test("completed tasks remain visible until explicitly deleted", () => { let state = emptyConsoleTaskState(); for (const subject of ["complete", "delete"]) { state = applyTaskToolCall( @@ -72,14 +72,11 @@ Deno.test("completed and deleted tasks remain in the TUI-style store", () => { "TaskUpdate", JSON.stringify({ taskid: 2, status: "deleted" }), ); - assert( - state.tasks.length === 2, - "the full TaskStore should retain inactive tasks", - ); + assert(state.tasks.length === 1, "only deleted tasks should be forgotten"); const counts = taskCounts(state.tasks); assert(counts.completed === 1, "completed tasks should remain counted"); - assert(counts.deleted === 1, "deleted tasks should remain counted"); - assert(counts.active === 0, "inactive tasks should not be shown as active"); + assert(counts.deleted === 0, "deleted tasks should be forgotten"); + assert(counts.active === 0, "completed tasks should not be shown as active"); }); Deno.test("session TaskStore snapshot replaces stale state and advances ids", () => { @@ -102,13 +99,13 @@ TaskStore: 1 active task(s) (pending: 0, inprogress: 1) \`\`\` `; state = applyTaskSnapshotText(state, snapshot); + assert(state.tasks.length === 2, "snapshot should restore retained tasks"); + assert(state.tasks[0].taskid === 4, "the completed task should be restored"); + assert(state.tasks[1].taskid === 7, "the active task should be restored"); assert( - state.tasks.length === 2, - "snapshot should restore the full TaskStore", + state.nextTaskId === 8, + "next id should follow the highest retained task id", ); - assert(state.tasks[0].taskid === 4, "completed tasks should remain restored"); - assert(state.tasks[1].taskid === 7, "active tasks should remain restored"); - assert(state.nextTaskId === 8, "next id should follow the highest task id"); state = applyTaskToolCall( state, "TaskCreate", diff --git a/web/workspace/src/lib/workspace/console/tasks.ts b/web/workspace/src/lib/workspace/console/tasks.ts index 8a0fc114..d0b6e606 100644 --- a/web/workspace/src/lib/workspace/console/tasks.ts +++ b/web/workspace/src/lib/workspace/console/tasks.ts @@ -62,6 +62,13 @@ export function applyTaskToolCall( const current = state.tasks[index]; const status = update.status ?? current.status; + if (status === "deleted") { + return { + tasks: state.tasks.filter((_, taskIndex) => taskIndex !== index), + nextTaskId: state.nextTaskId, + }; + } + const tasks = [...state.tasks]; tasks[index] = { taskid: current.taskid, @@ -78,9 +85,10 @@ export function applyTaskSnapshotText( ): ConsoleTaskState { const tasks = parseTaskSnapshotText(text); if (!tasks) return state; + const retained = tasks.filter((task) => task.status !== "deleted"); return { - tasks, - nextTaskId: Math.max(1, ...tasks.map((task) => task.taskid + 1)), + tasks: retained, + nextTaskId: Math.max(1, ...retained.map((task) => task.taskid + 1)), }; } @@ -105,7 +113,9 @@ export function parseTaskSnapshotText(text: string): ConsoleTask[] | null { for (const candidate of value.tasks) { const task = taskEntry(candidate); if (!task) return null; - tasks.push(task); + if (task.status !== "deleted") { + tasks.push(task); + } } return tasks; }