worker: track work as natural task steps

This commit is contained in:
2026-08-14 14:23:58 +09:00
parent 93eea24420
commit ecbd003579
8 changed files with 171 additions and 124 deletions
+3 -1
View File
@@ -3539,7 +3539,9 @@ mod completion_flow_tests {
arguments: args.into(), 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] #[test]
+24 -19
View File
@@ -28,8 +28,8 @@ pub enum TaskStatus {
} }
impl TaskStatus { impl TaskStatus {
fn is_active(self) -> bool { fn is_retained(self) -> bool {
matches!(self, Self::Pending | Self::Inprogress) !matches!(self, Self::Deleted)
} }
} }
@@ -125,7 +125,7 @@ impl TaskStore {
if let Some(d) = p.description { if let Some(d) = p.description {
t.description = d; t.description = d;
} }
if !t.status.is_active() { if !t.status.is_retained() {
self.tasks.remove(task_position); self.tasks.remove(task_position);
} }
} }
@@ -145,7 +145,7 @@ impl TaskStore {
fn replace_with(&mut self, tasks: Vec<TaskEntry>) { fn replace_with(&mut self, tasks: Vec<TaskEntry>) {
let tasks: Vec<_> = tasks let tasks: Vec<_> = tasks
.into_iter() .into_iter()
.filter(|task| task.status.is_active()) .filter(|task| task.status.is_retained())
.collect(); .collect();
self.next_taskid = tasks self.next_taskid = tasks
.iter() .iter()
@@ -194,7 +194,7 @@ fn parse_snapshot_text(text: &str) -> Option<Vec<TaskEntry>> {
snapshot snapshot
.tasks .tasks
.into_iter() .into_iter()
.filter(|task| task.status.is_active()) .filter(|task| task.status.is_retained())
.collect(), .collect(),
) )
} }
@@ -241,7 +241,7 @@ mod tests {
} }
#[test] #[test]
fn counts_tracks_only_active_tasks_after_completion() { fn counts_retains_completed_tasks_until_deleted() {
let mut s = TaskStore::new(); let mut s = TaskStore::new();
s.apply_tool_call("TaskCreate", r#"{"subject":"a","description":""}"#); s.apply_tool_call("TaskCreate", r#"{"subject":"a","description":""}"#);
s.apply_tool_call("TaskCreate", r#"{"subject":"b","description":""}"#); s.apply_tool_call("TaskCreate", r#"{"subject":"b","description":""}"#);
@@ -251,10 +251,13 @@ mod tests {
let c = s.counts(); let c = s.counts();
assert_eq!(c.pending, 1); assert_eq!(c.pending, 1);
assert_eq!(c.inprogress, 1); assert_eq!(c.inprogress, 1);
assert_eq!(c.completed, 0); assert_eq!(c.completed, 1);
assert_eq!(c.deleted, 0); assert_eq!(c.deleted, 0);
assert_eq!(c.total(), 2); assert_eq!(c.total(), 3);
assert_eq!(c.active(), 2); 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 /// 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 { fn wrap_snapshot(json_body: &str, overview: &str) -> String {
format!( format!(
"[Session TaskStore snapshot]\n\n{overview}\n\n```json\n{json_body}\n```\n\n\ "[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." The following TaskList tool result presents the same state through the tool lane."
) )
} }
@@ -288,19 +291,21 @@ mod tests {
}"#; }"#;
let text = wrap_snapshot( let text = wrap_snapshot(
body, 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(); let mut s = TaskStore::new();
s.apply_tool_call("TaskCreate", r#"{"subject":"stale","description":""}"#); s.apply_tool_call("TaskCreate", r#"{"subject":"stale","description":""}"#);
s.apply_system_message_text(&text); s.apply_system_message_text(&text);
let tasks = s.tasks(); let tasks = s.tasks();
assert_eq!(tasks.len(), 1); assert_eq!(tasks.len(), 2);
assert_eq!(tasks[0].taskid, 7); assert_eq!(tasks[0].taskid, 5);
assert_eq!(tasks[0].status, TaskStatus::Pending); assert_eq!(tasks[0].status, TaskStatus::Completed);
// Subsequent TaskCreate must continue beyond the highest active taskid 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. // observed in the snapshot.
s.apply_tool_call("TaskCreate", r#"{"subject":"new","description":""}"#); s.apply_tool_call("TaskCreate", r#"{"subject":"new","description":""}"#);
assert_eq!(s.tasks()[1].taskid, 8); assert_eq!(s.tasks()[2].taskid, 8);
} }
#[test] #[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(); let mut s = TaskStore::new();
s.apply_system_message_text(&text); s.apply_system_message_text(&text);
let t = &s.tasks()[0]; let t = &s.tasks()[0];
@@ -349,13 +354,13 @@ mod snapshot_format_contract {
fn wrap_pod_style(snapshot_text: &str) -> String { fn wrap_pod_style(snapshot_text: &str) -> String {
format!( format!(
"[Session TaskStore snapshot]\n\n{snapshot_text}\n\n\ "[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." The following TaskList tool result presents the same state through the tool lane."
) )
} }
fn snapshot_fixture() -> &'static str { 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 ```json
{ {
@@ -372,7 +377,7 @@ mod snapshot_format_contract {
} }
fn empty_snapshot_fixture() -> &'static str { 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 ```json
{ {
+20 -18
View File
@@ -15,8 +15,10 @@ mod store;
mod tool_impl; mod tool_impl;
pub(crate) use self::tool_impl::task_tools; pub(crate) use self::tool_impl::task_tools;
#[cfg(test)]
pub(crate) use store::TaskStatus;
use store::snapshot_overview; use store::snapshot_overview;
pub(crate) use store::{TaskEntry, TaskStatus, TaskStore}; pub(crate) use store::{TaskEntry, TaskStore};
use crate::feature::{ use crate::feature::{
FeatureDescriptor, FeatureHookPoint, FeatureInstallContext, FeatureInstallError, FeatureModule, FeatureDescriptor, FeatureHookPoint, FeatureInstallContext, FeatureInstallError, FeatureModule,
@@ -76,7 +78,7 @@ impl TaskFeature {
/// pointing at the same feature-owned store after rewind. /// pointing at the same feature-owned store after rewind.
pub fn restore_from_history(&self, history: &[Item]) { pub fn restore_from_history(&self, history: &[Item]) {
let restored = TaskStore::from_history(history); 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. /// 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. /// Feature-owned compact summary used for the synthetic TaskList result.
pub fn snapshot_overview(&self) -> String { pub fn snapshot_overview(&self) -> String {
snapshot_overview(&self.state.task_store.list_active()) snapshot_overview(&self.state.task_store.list())
} }
#[cfg(test)] #[cfg(test)]
@@ -208,14 +210,8 @@ struct TaskReminderPreRequestHook {
#[async_trait] #[async_trait]
impl Hook<PreLlmRequest> for TaskReminderPreRequestHook { impl Hook<PreLlmRequest> for TaskReminderPreRequestHook {
async fn call(&self, input: &PreRequestContext) -> HookPreRequestAction { async fn call(&self, input: &PreRequestContext) -> HookPreRequestAction {
let active_tasks: Vec<TaskEntry> = self let tasks = self.state.task_store.list();
.state if tasks.is_empty() {
.task_store
.list()
.into_iter()
.filter(|task| matches!(task.status, TaskStatus::Pending | TaskStatus::Inprogress))
.collect();
if active_tasks.is_empty() {
return HookPreRequestAction::Continue; return HookPreRequestAction::Continue;
} }
@@ -228,7 +224,7 @@ impl Hook<PreLlmRequest> for TaskReminderPreRequestHook {
if let Some(system_items) = input.system_items() { if let Some(system_items) = input.system_items() {
self.state.reminder_state.note_reminder(); 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 HookPreRequestAction::Continue
} }
@@ -252,11 +248,11 @@ fn is_task_management_tool(name: &str) -> bool {
TASK_MANAGEMENT_TOOL_NAMES.contains(&name) 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( 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!( body.push_str(&format!(
"- taskid {} ({}) {}\n", "- taskid {} ({}) {}\n",
task.taskid, task.status, task.subject task.taskid, task.status, task.subject
@@ -433,7 +429,7 @@ mod tests {
} }
#[tokio::test] #[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 feature = TaskFeature::new();
let done = feature let done = feature
.task_store() .task_store()
@@ -448,10 +444,16 @@ mod tests {
}; };
let pending = Arc::new(Mutex::new(Vec::new())); 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; 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] #[tokio::test]
+56 -34
View File
@@ -19,8 +19,8 @@ pub enum TaskStatus {
} }
impl TaskStatus { impl TaskStatus {
pub fn is_active(self) -> bool { pub fn is_retained(self) -> bool {
matches!(self, Self::Pending | Self::Inprogress) !matches!(self, Self::Deleted)
} }
} }
@@ -93,13 +93,6 @@ impl TaskStore {
.clone() .clone()
} }
pub fn list_active(&self) -> Vec<TaskEntry> {
self.list()
.into_iter()
.filter(|task| task.status.is_active())
.collect()
}
pub fn get(&self, taskid: u64) -> Option<TaskEntry> { pub fn get(&self, taskid: u64) -> Option<TaskEntry> {
self.inner self.inner
.lock() .lock()
@@ -137,7 +130,7 @@ impl TaskStore {
task.description = description; task.description = description;
} }
let updated = task.clone(); let updated = task.clone();
if !updated.status.is_active() { if !updated.status.is_retained() {
inner.tasks.remove(task_position); inner.tasks.remove(task_position);
} }
Ok(updated) Ok(updated)
@@ -149,7 +142,7 @@ impl TaskStore {
pub fn snapshot_limited(&self, limit: usize) -> TaskSnapshot { pub fn snapshot_limited(&self, limit: usize) -> TaskSnapshot {
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<TaskEntry>) { pub fn replace_with(&self, tasks: Vec<TaskEntry>) {
let tasks: Vec<_> = tasks let tasks: Vec<_> = tasks
.into_iter() .into_iter()
.filter(|task| task.status.is_active()) .filter(|task| task.status.is_retained())
.collect(); .collect();
let next_taskid = tasks let next_taskid = tasks
.iter() .iter()
@@ -267,24 +260,30 @@ pub fn snapshot_overview(tasks: &[TaskEntry]) -> String {
.iter() .iter()
.filter(|t| t.status == TaskStatus::Inprogress) .filter(|t| t.status == TaskStatus::Inprogress)
.count(); .count();
let active = pending + inprogress; let completed = tasks
format!("TaskStore: {active} active task(s) (pending: {pending}, inprogress: {inprogress})") .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 { pub fn render_snapshot(tasks: &[TaskEntry]) -> String {
let active_tasks: Vec<_> = tasks let retained_tasks: Vec<_> = tasks
.iter() .iter()
.filter(|task| task.status.is_active()) .filter(|task| task.status.is_retained())
.cloned() .cloned()
.collect(); .collect();
let snapshot = TaskSnapshot { let snapshot = TaskSnapshot {
tasks: active_tasks.clone(), tasks: retained_tasks.clone(),
}; };
let json = let json =
serde_json::to_string_pretty(&snapshot).unwrap_or_else(|_| String::from("{\"tasks\":[]}")); serde_json::to_string_pretty(&snapshot).unwrap_or_else(|_| String::from("{\"tasks\":[]}"));
format!( format!(
"{}\n\n```json\n{}\n```\n", "{}\n\n```json\n{}\n```\n",
snapshot_overview(&active_tasks), snapshot_overview(&retained_tasks),
json json
) )
} }
@@ -303,7 +302,7 @@ pub(super) fn parse_compact_snapshot_text(text: &str) -> Option<Vec<TaskEntry>>
snapshot snapshot
.tasks .tasks
.into_iter() .into_iter()
.filter(|task| task.status.is_active()) .filter(|task| task.status.is_retained())
.collect(), .collect(),
) )
} }
@@ -312,6 +311,22 @@ pub(super) fn parse_compact_snapshot_text(text: &str) -> Option<Vec<TaskEntry>>
mod tests { mod tests {
use super::*; 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] #[test]
fn replay_history_reconstructs_store_and_ignores_malformed_calls() { fn replay_history_reconstructs_store_and_ignores_malformed_calls() {
let history = vec![ let history = vec![
@@ -323,9 +338,11 @@ mod tests {
]; ];
let store = TaskStore::from_history(&history); let store = TaskStore::from_history(&history);
let tasks = store.list(); let tasks = store.list();
assert_eq!(tasks.len(), 1); assert_eq!(tasks.len(), 2);
assert_eq!(tasks[0].taskid, 1); assert_eq!(tasks[0].taskid, 1);
assert_eq!(tasks[0].status, TaskStatus::Pending); 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 /// 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 { fn wrap_snapshot_system_message(snapshot: &str) -> String {
format!( format!(
"[Session TaskStore snapshot]\n\n{snapshot}\n\n\ "[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." 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 store = TaskStore::from_history(&history);
let tasks = store.list(); let tasks = store.list();
assert_eq!(tasks.len(), 1); assert_eq!(tasks.len(), 2);
assert_eq!(tasks[0].taskid, 2); assert_eq!(tasks[0].taskid, 1);
assert_eq!(tasks[0].subject, "new"); assert_eq!(tasks[0].status, TaskStatus::Completed);
assert_eq!(tasks[1].taskid, 2);
assert_eq!(tasks[1].subject, "new");
} }
#[test] #[test]
@@ -396,12 +415,15 @@ mod tests {
]; ];
let store = TaskStore::from_history(&history); let store = TaskStore::from_history(&history);
let tasks = store.list(); let tasks = store.list();
assert_eq!(tasks.len(), 2); assert_eq!(tasks.len(), 3);
assert_eq!(tasks[0].taskid, 2); assert_eq!(tasks[0].taskid, 1);
assert_eq!(tasks[0].subject, "B"); assert_eq!(tasks[0].subject, "A");
assert_eq!(tasks[0].status, TaskStatus::Inprogress); assert_eq!(tasks[0].status, TaskStatus::Completed);
assert_eq!(tasks[1].taskid, 3); assert_eq!(tasks[1].taskid, 2);
assert_eq!(tasks[1].subject, "C"); 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] #[test]
@@ -443,10 +465,10 @@ mod tests {
let snapshot_text = pre.snapshot_text(); let snapshot_text = pre.snapshot_text();
let system = Item::system_message(wrap_snapshot_system_message(&snapshot_text)); let system = Item::system_message(wrap_snapshot_system_message(&snapshot_text));
let call = Item::tool_call("compact-tasklist", "TaskList", "{}"); 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( let result = Item::tool_result_with_content(
"compact-tasklist", "compact-tasklist",
snapshot_overview(&active_tasks), snapshot_overview(&retained_tasks),
snapshot_text.clone(), snapshot_text.clone(),
); );
@@ -455,7 +477,7 @@ mod tests {
.as_text() .as_text()
.and_then(parse_compact_snapshot_text) .and_then(parse_compact_snapshot_text)
.expect("system message should parse as snapshot"); .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 // The synthetic call/result pair shares one call_id and carries the
// expected tool name + detailed content. // expected tool name + detailed content.
@@ -482,6 +504,6 @@ mod tests {
// Replaying the full triple reconstructs the same TaskStore. // Replaying the full triple reconstructs the same TaskStore.
let store = TaskStore::from_history(&[system, call, result]); let store = TaskStore::from_history(&[system, call, result]);
assert_eq!(store.list(), active_tasks); assert_eq!(store.list(), retained_tasks);
} }
} }
@@ -18,7 +18,7 @@ struct TaskCreateParams {
#[derive(Debug, Deserialize, schemars::JsonSchema)] #[derive(Debug, Deserialize, schemars::JsonSchema)]
struct TaskListParams { struct TaskListParams {
/// Maximum number of active tasks to return. Defaults to 20. /// Maximum number of retained tasks to return. Defaults to 20.
#[serde(default)] #[serde(default)]
limit: Option<usize>, limit: Option<usize>,
} }
@@ -55,23 +55,28 @@ struct TaskUpdateTool {
store: TaskStore, store: TaskStore,
} }
const CREATE_DESCRIPTION: &str = "Create a session-lifetime task only when user-visible \ const CREATE_DESCRIPTION: &str = "Before beginning nontrivial work, decompose the predictable \
progress tracking is genuinely useful: multiple active tasks must be remembered, or the work \ portion into a small ordered set of natural, user-visible steps and create one session-lifetime task \
will involve long edits, long-running commands, extended investigation, or interruption-prone \ per step. A natural step has a distinct verifiable outcome, such as investigation, implementation, \
coordination. Do not create a task just because a request has several steps, and do not create \ or validation; do not create a task for every command or mechanical substep. Do not collapse several \
one for short questions, quick checks, single reviews, or one-off commands. Prefer updating an \ predictable steps into one umbrella task. Keep future steps pending, mark the current step inprogress, \
existing active task over creating a duplicate. Input only `subject` and `description`; `taskid` \ and add or revise tasks when new information changes the plan. Skip task tracking for short questions, \
is assigned automatically and initial `status` is `pending`."; quick checks, and one-off commands. Input only `subject` and `description`; `taskid` is assigned \
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."; 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 \ 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 \ user-visible real-time status for short-term current-work tracking. Returns an error if the task \
does not exist."; does not exist.";
const UPDATE_DESCRIPTION: &str = "Update an existing session-lifetime task when meaningful \ const UPDATE_DESCRIPTION: &str = "Update task status at each natural step boundary. Mark the \
progress changes between substantial steps. Tasks are user-visible real-time status, so avoid \ finished current step completed before advancing the next pending step to inprogress. Completed tasks \
churn for trivial substeps. Keep status current with `pending`, `inprogress`, `completed`, or \ remain visible as recent workflow context; at a later natural boundary, delete older completed tasks \
`deleted`. Provide `taskid` and at least one of `status`, `subject`, or `description`; deletion is \ whose result is no longer useful. Do not jump directly from active to deleted merely to hide progress, \
logical (`status = deleted`). If an unexpected problem blocks progress, do not force the next \ and avoid churn for mechanical substeps. Keep status current with `pending`, `inprogress`, `completed`, \
step: leave the task as-is, summarize the problem to the user, and end the turn."; 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] #[async_trait]
impl Tool for TaskCreateTool { impl Tool for TaskCreateTool {
@@ -83,7 +88,7 @@ impl Tool for TaskCreateTool {
let params: TaskCreateParams = serde_json::from_str(input_json) let params: TaskCreateParams = serde_json::from_str(input_json)
.map_err(|e| ToolError::InvalidArgument(format!("invalid TaskCreate input: {e}")))?; .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskCreate input: {e}")))?;
let created = self.store.create(params.subject, params.description); let created = self.store.create(params.subject, params.description);
let tasks = self.store.list_active(); let tasks = self.store.list();
Ok(task_output( Ok(task_output(
format!( format!(
"Created task {} ({})\n{}", "Created task {} ({})\n{}",
@@ -106,10 +111,10 @@ impl Tool for TaskListTool {
let params: TaskListParams = serde_json::from_str(input_json) let params: TaskListParams = serde_json::from_str(input_json)
.map_err(|e| ToolError::InvalidArgument(format!("invalid TaskList input: {e}")))?; .map_err(|e| ToolError::InvalidArgument(format!("invalid TaskList input: {e}")))?;
let limit = params.limit.unwrap_or(DEFAULT_TASK_LIST_LIMIT); let limit = params.limit.unwrap_or(DEFAULT_TASK_LIST_LIMIT);
let active_tasks = self.store.list_active(); let retained_tasks = self.store.list();
let tasks: Vec<_> = active_tasks.iter().take(limit).cloned().collect(); let tasks: Vec<_> = retained_tasks.iter().take(limit).cloned().collect();
Ok(ToolOutput { Ok(ToolOutput {
summary: list_overview(active_tasks.len(), tasks.len()), summary: list_overview(retained_tasks.len(), tasks.len()),
content: Some(render_task_list(&tasks)), content: Some(render_task_list(&tasks)),
attachments: Vec::new(), attachments: Vec::new(),
@@ -157,7 +162,7 @@ impl Tool for TaskUpdateTool {
params.description, params.description,
) )
.map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?;
let tasks = self.store.list_active(); let tasks = self.store.list();
Ok(task_output( Ok(task_output(
format!( format!(
"Updated task {} ({})\n{}", "Updated task {} ({})\n{}",
@@ -179,14 +184,14 @@ fn task_output(summary: String, task: &TaskEntry) -> ToolOutput {
} }
} }
fn list_overview(total_active: usize, returned: usize) -> String { fn list_overview(total: usize, returned: usize) -> String {
if returned < total_active { if returned < total {
format!( format!(
"TaskStore: {returned} active task(s) shown; {} omitted.", "TaskStore: {returned} task(s) shown; {} omitted.",
total_active - returned total - returned
) )
} else { } 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")); assert!(out.content.unwrap().contains("implement tasks"));
let out = list.execute("{}", Default::default()).await.unwrap(); 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(); let content = out.content.unwrap();
assert!(content.contains("\"taskid\": 1")); assert!(content.contains("\"taskid\": 1"));
assert!(!content.contains("\"limit\"")); assert!(!content.contains("\"limit\""));
@@ -318,7 +323,7 @@ mod tests {
} }
#[tokio::test] #[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 store = TaskStore::new();
let create = tool(task_create_tool(store.clone())); let create = tool(task_create_tool(store.clone()));
let update = tool(task_update_tool(store.clone())); let update = tool(task_update_tool(store.clone()));
@@ -343,10 +348,7 @@ mod tests {
.unwrap(); .unwrap();
let out = list.execute("{}", Default::default()).await.unwrap(); let out = list.execute("{}", Default::default()).await.unwrap();
assert_eq!( assert_eq!(out.summary, "TaskStore: 20 task(s) shown; 4 omitted.");
out.summary,
"TaskStore: 20 active task(s) shown; 3 omitted."
);
let content = out.content.unwrap(); let content = out.content.unwrap();
let json: serde_json::Value = serde_json::from_str(&content).unwrap(); let json: serde_json::Value = serde_json::from_str(&content).unwrap();
let tasks = json.as_array().unwrap(); let tasks = json.as_array().unwrap();
@@ -355,7 +357,7 @@ mod tests {
.iter() .iter()
.map(|task| task["taskid"].as_u64().unwrap()) .map(|task| task["taskid"].as_u64().unwrap())
.collect(); .collect();
assert!(!ids.contains(&1)); assert!(ids.contains(&1));
assert!(!ids.contains(&2)); assert!(!ids.contains(&2));
assert!(!content.contains("\"limit\"")); assert!(!content.contains("\"limit\""));
assert!(!content.contains("\"total_active\"")); assert!(!content.contains("\"total_active\""));
@@ -365,10 +367,7 @@ mod tests {
.execute(r#"{"limit":3}"#, Default::default()) .execute(r#"{"limit":3}"#, Default::default())
.await .await
.unwrap(); .unwrap();
assert_eq!( assert_eq!(out.summary, "TaskStore: 3 task(s) shown; 21 omitted.");
out.summary,
"TaskStore: 3 active task(s) shown; 20 omitted."
);
let content = out.content.unwrap(); let content = out.content.unwrap();
let json: serde_json::Value = serde_json::from_str(&content).unwrap(); let json: serde_json::Value = serde_json::from_str(&content).unwrap();
assert_eq!(json.as_array().unwrap().len(), 3); assert_eq!(json.as_array().unwrap().len(), 3);
@@ -412,6 +411,16 @@ mod tests {
assert!(!content.contains("completed task")); 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] #[tokio::test]
async fn task_update_validates_existing_and_at_least_one_field() { async fn task_update_validates_existing_and_at_least_one_field() {
let store = TaskStore::new(); let store = TaskStore::new();
+1 -1
View File
@@ -3347,7 +3347,7 @@ impl<C: LlmClient, St: Store> Worker<C, St> {
} }
let task_snapshot_message = Item::system_message(format!( let task_snapshot_message = Item::system_message(format!(
"[Session TaskStore snapshot]\n\n{task_snapshot_text}\n\n\ "[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." The following TaskList tool result presents the same state through the tool lane."
)); ));
compact_introduced_system_messages.push(task_snapshot_message.clone()); compact_introduced_system_messages.push(task_snapshot_message.clone());
@@ -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"); 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(); let state = emptyConsoleTaskState();
for (const subject of ["complete", "delete"]) { for (const subject of ["complete", "delete"]) {
state = applyTaskToolCall( state = applyTaskToolCall(
@@ -72,14 +72,11 @@ Deno.test("completed and deleted tasks remain in the TUI-style store", () => {
"TaskUpdate", "TaskUpdate",
JSON.stringify({ taskid: 2, status: "deleted" }), JSON.stringify({ taskid: 2, status: "deleted" }),
); );
assert( assert(state.tasks.length === 1, "only deleted tasks should be forgotten");
state.tasks.length === 2,
"the full TaskStore should retain inactive tasks",
);
const counts = taskCounts(state.tasks); const counts = taskCounts(state.tasks);
assert(counts.completed === 1, "completed tasks should remain counted"); assert(counts.completed === 1, "completed tasks should remain counted");
assert(counts.deleted === 1, "deleted tasks should remain counted"); assert(counts.deleted === 0, "deleted tasks should be forgotten");
assert(counts.active === 0, "inactive tasks should not be shown as active"); assert(counts.active === 0, "completed tasks should not be shown as active");
}); });
Deno.test("session TaskStore snapshot replaces stale state and advances ids", () => { 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); 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( assert(
state.tasks.length === 2, state.nextTaskId === 8,
"snapshot should restore the full TaskStore", "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 = applyTaskToolCall(
state, state,
"TaskCreate", "TaskCreate",
@@ -62,6 +62,13 @@ export function applyTaskToolCall(
const current = state.tasks[index]; const current = state.tasks[index];
const status = update.status ?? current.status; const status = update.status ?? current.status;
if (status === "deleted") {
return {
tasks: state.tasks.filter((_, taskIndex) => taskIndex !== index),
nextTaskId: state.nextTaskId,
};
}
const tasks = [...state.tasks]; const tasks = [...state.tasks];
tasks[index] = { tasks[index] = {
taskid: current.taskid, taskid: current.taskid,
@@ -78,9 +85,10 @@ export function applyTaskSnapshotText(
): ConsoleTaskState { ): ConsoleTaskState {
const tasks = parseTaskSnapshotText(text); const tasks = parseTaskSnapshotText(text);
if (!tasks) return state; if (!tasks) return state;
const retained = tasks.filter((task) => task.status !== "deleted");
return { return {
tasks, tasks: retained,
nextTaskId: Math.max(1, ...tasks.map((task) => task.taskid + 1)), nextTaskId: Math.max(1, ...retained.map((task) => task.taskid + 1)),
}; };
} }
@@ -105,8 +113,10 @@ export function parseTaskSnapshotText(text: string): ConsoleTask[] | null {
for (const candidate of value.tasks) { for (const candidate of value.tasks) {
const task = taskEntry(candidate); const task = taskEntry(candidate);
if (!task) return null; if (!task) return null;
if (task.status !== "deleted") {
tasks.push(task); tasks.push(task);
} }
}
return tasks; return tasks;
} }