From 5159846971937949ff0a993d5674ac471a1c6212 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Sat, 24 Jan 2026 17:32:23 +0100 Subject: [PATCH 01/19] feat: add tasks tool with dependency management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new 'tasks' toolset separate from 'todo' that provides dependency-aware task management inspired by Claude Code's task system: Features: - BlockedBy/Blocks relationships between tasks - Automatic enforcement: cannot start blocked tasks - Visual indicators: ✓=done, ■=in-progress, □=pending, ⚠=blocked - Owner assignment for tasks - Circular dependency detection - Unblock notifications when dependencies complete New tools: - create_task: Create task with optional blocked_by and owner - create_tasks: Batch create tasks with dependencies - update_tasks: Update status with dependency enforcement - list_tasks: List with stats and blocked indicators - add_task_dependency: Add blockers to existing task - remove_task_dependency: Remove blockers - get_blocked_tasks: Query blocked tasks Usage in config: toolsets: - type: tasks shared: true # Optional: share across agents The original 'todo' tool remains unchanged for simple use cases. Assisted-By: cagent --- pkg/teamloader/registry.go | 8 + pkg/tools/builtin/tasks.go | 588 +++++++++++++++++++++++++++++++++++++ 2 files changed, 596 insertions(+) create mode 100644 pkg/tools/builtin/tasks.go diff --git a/pkg/teamloader/registry.go b/pkg/teamloader/registry.go index 7374605cc..c218fdd13 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -59,6 +59,7 @@ func NewDefaultToolsetRegistry() *ToolsetRegistry { r := NewToolsetRegistry() // Register all built-in toolset creators r.Register("todo", createTodoTool) + r.Register("tasks", createTasksTool) r.Register("memory", createMemoryTool) r.Register("think", createThinkTool) r.Register("shell", createShellTool) @@ -80,6 +81,13 @@ func createTodoTool(_ context.Context, toolset latest.Toolset, _ string, _ *conf return builtin.NewTodoTool(), nil } +func createTasksTool(_ context.Context, toolset latest.Toolset, _ string, _ *config.RuntimeConfig) (tools.ToolSet, error) { + if toolset.Shared { + return builtin.NewSharedTasksTool(), nil + } + return builtin.NewTasksTool(), nil +} + func createMemoryTool(_ context.Context, toolset latest.Toolset, parentDir string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { var memoryPath string if filepath.IsAbs(toolset.Path) { diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go new file mode 100644 index 000000000..1ad682941 --- /dev/null +++ b/pkg/tools/builtin/tasks.go @@ -0,0 +1,588 @@ +package builtin + +import ( + "context" + "fmt" + "strings" + "sync" + + "github.com/docker/cagent/pkg/concurrent" + "github.com/docker/cagent/pkg/tools" +) + +const ( + ToolNameCreateTask = "create_task" + ToolNameCreateTasks = "create_tasks" + ToolNameUpdateTasks = "update_tasks" + ToolNameListTasks = "list_tasks" + ToolNameAddTaskDep = "add_task_dependency" + ToolNameRemoveTaskDep = "remove_task_dependency" + ToolNameGetBlockedTasks = "get_blocked_tasks" +) + +type TasksTool struct { + tools.BaseToolSet + handler *tasksHandler +} + +var _ tools.ToolSet = (*TasksTool)(nil) + +// Task represents a task with optional dependencies +type Task struct { + ID string `json:"id" jsonschema:"ID of the task"` + Description string `json:"description" jsonschema:"Description of the task"` + Status string `json:"status" jsonschema:"Status: pending, in-progress, or completed"` + BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"IDs of tasks that must be completed before this one can start"` + Blocks []string `json:"blocks,omitempty" jsonschema:"IDs of tasks that are waiting for this one to complete"` + Owner string `json:"owner,omitempty" jsonschema:"Owner/assignee of this task"` +} + +type CreateTaskArgs struct { + Description string `json:"description" jsonschema:"Description of the task,required"` + BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"IDs of tasks that must be completed before this one can start"` + Owner string `json:"owner,omitempty" jsonschema:"Owner/assignee of this task"` +} + +type CreateTaskItem struct { + Description string `json:"description" jsonschema:"Description of the task,required"` + BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"IDs of tasks that must be completed before this one can start"` + Owner string `json:"owner,omitempty" jsonschema:"Owner/assignee of this task"` +} + +type CreateTasksArgs struct { + Tasks []CreateTaskItem `json:"tasks" jsonschema:"List of tasks to create,required"` +} + +type TaskUpdate struct { + ID string `json:"id" jsonschema:"ID of the task,required"` + Status string `json:"status,omitempty" jsonschema:"New status: pending, in-progress, or completed"` + Owner string `json:"owner,omitempty" jsonschema:"New owner/assignee"` +} + +type UpdateTasksArgs struct { + Updates []TaskUpdate `json:"updates" jsonschema:"List of task updates,required"` +} + +type AddTaskDependencyArgs struct { + TaskID string `json:"task_id" jsonschema:"ID of the task to add dependencies to,required"` + BlockedBy []string `json:"blocked_by" jsonschema:"IDs of tasks that must be completed first,required"` +} + +type RemoveTaskDependencyArgs struct { + TaskID string `json:"task_id" jsonschema:"ID of the task to remove dependencies from,required"` + BlockedBy []string `json:"blocked_by" jsonschema:"IDs of blocking tasks to remove,required"` +} + +type GetBlockedTasksArgs struct { + BlockedBy string `json:"blocked_by,omitempty" jsonschema:"Filter by specific blocker ID (optional)"` +} + +type tasksHandler struct { + tasks *concurrent.Slice[Task] +} + +var NewSharedTasksTool = sync.OnceValue(NewTasksTool) + +func NewTasksTool() *TasksTool { + return &TasksTool{ + handler: &tasksHandler{ + tasks: concurrent.NewSlice[Task](), + }, + } +} + +func (t *TasksTool) Instructions() string { + return `## Using the Tasks Tools + +IMPORTANT: Use these tools to track tasks with dependencies: + +1. Before starting complex work: + - Create tasks using create_task with blocked_by for dependencies + - Break down work into smaller tasks + +2. Dependencies: + - Tasks with blocked_by cannot start until blockers are completed + - Completing a task unblocks dependent tasks + - Use list_tasks to see blocked status + +3. While working: + - Use list_tasks to see available tasks + - Mark tasks as "in-progress" when starting + - Mark as "completed" when done + +4. Visual indicators in list_tasks: + - ✓ = completed, ■ = in-progress, □ = pending, ⚠ = blocked` +} + +func (h *tasksHandler) canStart(taskID string) (bool, []string) { + task, idx := h.tasks.Find(func(t Task) bool { return t.ID == taskID }) + if idx == -1 { + return false, []string{"task not found"} + } + if len(task.BlockedBy) == 0 { + return true, nil + } + var pendingBlockers []string + for _, blockerID := range task.BlockedBy { + blocker, blockerIdx := h.tasks.Find(func(t Task) bool { return t.ID == blockerID }) + if blockerIdx != -1 && blocker.Status != "completed" { + pendingBlockers = append(pendingBlockers, blockerID) + } + } + return len(pendingBlockers) == 0, pendingBlockers +} + +func (h *tasksHandler) findTask(id string) (*Task, int) { + task, idx := h.tasks.Find(func(t Task) bool { return t.ID == id }) + if idx == -1 { + return nil, -1 + } + return &task, idx +} + +func (h *tasksHandler) taskExists(id string) bool { + _, idx := h.findTask(id) + return idx != -1 +} + +func (h *tasksHandler) hasCircularDependency(taskID string, newBlockedBy []string) bool { + blocked := make(map[string]bool) + var collectBlocked func(id string) + collectBlocked = func(id string) { + task, idx := h.findTask(id) + if idx == -1 { + return + } + for _, blockedID := range task.Blocks { + if !blocked[blockedID] { + blocked[blockedID] = true + collectBlocked(blockedID) + } + } + } + collectBlocked(taskID) + for _, blockerID := range newBlockedBy { + if blocked[blockerID] || blockerID == taskID { + return true + } + } + return false +} + +func (h *tasksHandler) getUnblockedTasks(completedID string) []string { + var unblocked []string + h.tasks.Range(func(_ int, task Task) bool { + for _, blockerID := range task.BlockedBy { + if blockerID == completedID { + if canStart, _ := h.canStart(task.ID); canStart && task.Status == "pending" { + unblocked = append(unblocked, task.ID) + } + break + } + } + return true + }) + return unblocked +} + +func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*tools.ToolCallResult, error) { + for _, blockerID := range params.BlockedBy { + if !h.taskExists(blockerID) { + return tools.ResultError(fmt.Sprintf("invalid blocked_by reference: %s not found", blockerID)), nil + } + } + id := fmt.Sprintf("task_%d", h.tasks.Length()+1) + task := Task{ + ID: id, + Description: params.Description, + Status: "pending", + BlockedBy: params.BlockedBy, + Owner: params.Owner, + } + h.tasks.Append(task) + for _, blockerID := range params.BlockedBy { + _, idx := h.findTask(blockerID) + if idx != -1 { + h.tasks.Update(idx, func(t Task) Task { + t.Blocks = append(t.Blocks, id) + return t + }) + } + } + var output strings.Builder + fmt.Fprintf(&output, "Created task [%s]: %s", id, params.Description) + if len(params.BlockedBy) > 0 { + fmt.Fprintf(&output, " (blocked by %s)", strings.Join(params.BlockedBy, ", ")) + } + return &tools.ToolCallResult{Output: output.String(), Meta: h.tasks.All()}, nil +} + +func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (*tools.ToolCallResult, error) { + start := h.tasks.Length() + var createdIDs []string + for i, item := range params.Tasks { + for _, blockerID := range item.BlockedBy { + if !h.taskExists(blockerID) { + isEarlierInBatch := false + for j := 0; j < i; j++ { + if fmt.Sprintf("task_%d", start+j+1) == blockerID { + isEarlierInBatch = true + break + } + } + if !isEarlierInBatch { + return tools.ResultError(fmt.Sprintf("invalid blocked_by reference: %s not found", blockerID)), nil + } + } + } + id := fmt.Sprintf("task_%d", start+i+1) + task := Task{ + ID: id, + Description: item.Description, + Status: "pending", + BlockedBy: item.BlockedBy, + Owner: item.Owner, + } + h.tasks.Append(task) + createdIDs = append(createdIDs, id) + for _, blockerID := range item.BlockedBy { + _, idx := h.findTask(blockerID) + if idx != -1 { + h.tasks.Update(idx, func(t Task) Task { + t.Blocks = append(t.Blocks, id) + return t + }) + } + } + } + return &tools.ToolCallResult{ + Output: fmt.Sprintf("Created %d tasks: %s", len(params.Tasks), strings.Join(createdIDs, ", ")), + Meta: h.tasks.All(), + }, nil +} + +func (h *tasksHandler) updateTasks(_ context.Context, params UpdateTasksArgs) (*tools.ToolCallResult, error) { + var notFound, updated, blocked, newlyUnblocked []string + for _, update := range params.Updates { + task, idx := h.findTask(update.ID) + if idx == -1 { + notFound = append(notFound, update.ID) + continue + } + if update.Status == "in-progress" && task.Status == "pending" { + if canStart, blockers := h.canStart(update.ID); !canStart { + blocked = append(blocked, fmt.Sprintf("cannot start %s: blocked by %s", update.ID, strings.Join(blockers, ", "))) + continue + } + } + wasCompleting := update.Status == "completed" && task.Status != "completed" + h.tasks.Update(idx, func(t Task) Task { + if update.Status != "" { + t.Status = update.Status + } + if update.Owner != "" { + t.Owner = update.Owner + } + return t + }) + updated = append(updated, fmt.Sprintf("%s -> %s", update.ID, update.Status)) + if wasCompleting { + newlyUnblocked = append(newlyUnblocked, h.getUnblockedTasks(update.ID)...) + } + } + var output strings.Builder + if len(updated) > 0 { + fmt.Fprintf(&output, "Updated %d tasks: %s", len(updated), strings.Join(updated, ", ")) + } + for _, id := range newlyUnblocked { + if output.Len() > 0 { + output.WriteString("; ") + } + fmt.Fprintf(&output, "%s is now unblocked", id) + } + if len(blocked) > 0 { + if output.Len() > 0 { + output.WriteString("; ") + } + output.WriteString(strings.Join(blocked, "; ")) + } + if len(notFound) > 0 { + if output.Len() > 0 { + output.WriteString("; ") + } + fmt.Fprintf(&output, "Not found: %s", strings.Join(notFound, ", ")) + } + if len(updated) == 0 && (len(notFound) > 0 || len(blocked) > 0) { + return tools.ResultError(output.String()), nil + } + if h.allCompleted() { + h.tasks.Clear() + } + return &tools.ToolCallResult{Output: output.String(), Meta: h.tasks.All()}, nil +} + +func (h *tasksHandler) allCompleted() bool { + if h.tasks.Length() == 0 { + return false + } + allDone := true + h.tasks.Range(func(_ int, task Task) bool { + if task.Status != "completed" { + allDone = false + return false + } + return true + }) + return allDone +} + +func (h *tasksHandler) listTasks(_ context.Context, _ tools.ToolCall) (*tools.ToolCallResult, error) { + var output strings.Builder + var completed, inProgress, pending, blockedCount int + h.tasks.Range(func(_ int, task Task) bool { + switch task.Status { + case "completed": + completed++ + case "in-progress": + inProgress++ + default: + pending++ + if canStart, _ := h.canStart(task.ID); !canStart { + blockedCount++ + } + } + return true + }) + if h.tasks.Length() == 0 { + return &tools.ToolCallResult{Output: "No tasks.\n", Meta: h.tasks.All()}, nil + } + fmt.Fprintf(&output, "Tasks (%d done, %d in progress, %d pending", completed, inProgress, pending) + if blockedCount > 0 { + fmt.Fprintf(&output, ", %d blocked", blockedCount) + } + output.WriteString(")\n\n") + h.tasks.Range(func(_ int, task Task) bool { + var icon, suffix string + switch task.Status { + case "completed": + icon = "✓" + case "in-progress": + icon = "■" + default: + if canStart, blockers := h.canStart(task.ID); canStart { + icon = "□" + } else { + icon = "⚠" + suffix = fmt.Sprintf(" → blocked by: %s", strings.Join(blockers, ", ")) + } + } + fmt.Fprintf(&output, "%s [%s] %s", icon, task.ID, task.Description) + if task.Owner != "" { + fmt.Fprintf(&output, " (%s)", task.Owner) + } + output.WriteString(suffix + "\n") + return true + }) + return &tools.ToolCallResult{Output: output.String(), Meta: h.tasks.All()}, nil +} + +func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependencyArgs) (*tools.ToolCallResult, error) { + task, idx := h.findTask(params.TaskID) + if idx == -1 { + return tools.ResultError(fmt.Sprintf("task not found: %s", params.TaskID)), nil + } + if task.Status == "completed" { + return tools.ResultError(fmt.Sprintf("cannot add dependency to completed task: %s", params.TaskID)), nil + } + for _, blockerID := range params.BlockedBy { + if !h.taskExists(blockerID) { + return tools.ResultError(fmt.Sprintf("blocker not found: %s", blockerID)), nil + } + if blockerID == params.TaskID { + return tools.ResultError(fmt.Sprintf("task cannot depend on itself: %s", params.TaskID)), nil + } + } + if h.hasCircularDependency(params.TaskID, params.BlockedBy) { + return tools.ResultError("circular dependency detected"), nil + } + existingBlockers := make(map[string]bool) + for _, b := range task.BlockedBy { + existingBlockers[b] = true + } + var added, alreadyExists []string + for _, blockerID := range params.BlockedBy { + if existingBlockers[blockerID] { + alreadyExists = append(alreadyExists, blockerID) + } else { + added = append(added, blockerID) + } + } + if len(added) == 0 { + return &tools.ToolCallResult{ + Output: fmt.Sprintf("Dependency already exists: %s is already blocked by %s", params.TaskID, strings.Join(alreadyExists, ", ")), + Meta: h.tasks.All(), + }, nil + } + h.tasks.Update(idx, func(t Task) Task { + t.BlockedBy = append(t.BlockedBy, added...) + return t + }) + for _, blockerID := range added { + _, blockerIdx := h.findTask(blockerID) + if blockerIdx != -1 { + h.tasks.Update(blockerIdx, func(t Task) Task { + t.Blocks = append(t.Blocks, params.TaskID) + return t + }) + } + } + return &tools.ToolCallResult{ + Output: fmt.Sprintf("Added dependency: %s is now blocked by %s", params.TaskID, strings.Join(added, ", ")), + Meta: h.tasks.All(), + }, nil +} + +func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDependencyArgs) (*tools.ToolCallResult, error) { + task, idx := h.findTask(params.TaskID) + if idx == -1 { + return tools.ResultError(fmt.Sprintf("task not found: %s", params.TaskID)), nil + } + toRemove := make(map[string]bool) + for _, b := range params.BlockedBy { + toRemove[b] = true + } + var removed, newBlockedBy []string + for _, blockerID := range task.BlockedBy { + if toRemove[blockerID] { + removed = append(removed, blockerID) + } else { + newBlockedBy = append(newBlockedBy, blockerID) + } + } + if len(removed) == 0 { + return &tools.ToolCallResult{ + Output: fmt.Sprintf("No matching dependencies found to remove from %s", params.TaskID), + Meta: h.tasks.All(), + }, nil + } + h.tasks.Update(idx, func(t Task) Task { + t.BlockedBy = newBlockedBy + return t + }) + for _, blockerID := range removed { + _, blockerIdx := h.findTask(blockerID) + if blockerIdx != -1 { + h.tasks.Update(blockerIdx, func(t Task) Task { + var newBlocks []string + for _, b := range t.Blocks { + if b != params.TaskID { + newBlocks = append(newBlocks, b) + } + } + t.Blocks = newBlocks + return t + }) + } + } + return &tools.ToolCallResult{ + Output: fmt.Sprintf("Removed dependency: %s is no longer blocked by %s", params.TaskID, strings.Join(removed, ", ")), + Meta: h.tasks.All(), + }, nil +} + +func (h *tasksHandler) getBlockedTasks(_ context.Context, params GetBlockedTasksArgs) (*tools.ToolCallResult, error) { + var output strings.Builder + output.WriteString("Blocked tasks:\n") + found := false + h.tasks.Range(func(_ int, task Task) bool { + if len(task.BlockedBy) == 0 || task.Status == "completed" { + return true + } + if params.BlockedBy != "" { + hasBlocker := false + for _, b := range task.BlockedBy { + if b == params.BlockedBy { + hasBlocker = true + break + } + } + if !hasBlocker { + return true + } + } + if canStart, blockers := h.canStart(task.ID); !canStart { + found = true + fmt.Fprintf(&output, "- [%s] %s → blocked by: %s\n", task.ID, task.Description, strings.Join(blockers, ", ")) + } + return true + }) + if !found { + output.Reset() + output.WriteString("No blocked tasks") + if params.BlockedBy != "" { + fmt.Fprintf(&output, " (filtered by %s)", params.BlockedBy) + } + output.WriteString(".\n") + } + return &tools.ToolCallResult{Output: output.String(), Meta: h.tasks.All()}, nil +} + +func (t *TasksTool) Tools(context.Context) ([]tools.Tool, error) { + return []tools.Tool{ + { + Name: ToolNameCreateTask, + Category: "tasks", + Description: "Create a new task. Use blocked_by to specify dependencies on other tasks.", + Parameters: tools.MustSchemaFor[CreateTaskArgs](), + Handler: tools.NewHandler(t.handler.createTask), + Annotations: tools.ToolAnnotations{Title: "Create Task", ReadOnlyHint: true}, + }, + { + Name: ToolNameCreateTasks, + Category: "tasks", + Description: "Create multiple tasks at once with dependencies.", + Parameters: tools.MustSchemaFor[CreateTasksArgs](), + Handler: tools.NewHandler(t.handler.createTasks), + Annotations: tools.ToolAnnotations{Title: "Create Tasks", ReadOnlyHint: true}, + }, + { + Name: ToolNameUpdateTasks, + Category: "tasks", + Description: "Update the status of tasks. Cannot start a task blocked by incomplete dependencies.", + Parameters: tools.MustSchemaFor[UpdateTasksArgs](), + Handler: tools.NewHandler(t.handler.updateTasks), + Annotations: tools.ToolAnnotations{Title: "Update Tasks", ReadOnlyHint: true}, + }, + { + Name: ToolNameListTasks, + Category: "tasks", + Description: "List all tasks with status and dependencies. Visual indicators: ✓=done, ■=in-progress, □=available, ⚠=blocked", + Handler: t.handler.listTasks, + Annotations: tools.ToolAnnotations{Title: "List Tasks", ReadOnlyHint: true}, + }, + { + Name: ToolNameAddTaskDep, + Category: "tasks", + Description: "Add a dependency to an existing task.", + Parameters: tools.MustSchemaFor[AddTaskDependencyArgs](), + Handler: tools.NewHandler(t.handler.addDependency), + Annotations: tools.ToolAnnotations{Title: "Add Task Dependency", ReadOnlyHint: true}, + }, + { + Name: ToolNameRemoveTaskDep, + Category: "tasks", + Description: "Remove a dependency from a task.", + Parameters: tools.MustSchemaFor[RemoveTaskDependencyArgs](), + Handler: tools.NewHandler(t.handler.removeDependency), + Annotations: tools.ToolAnnotations{Title: "Remove Task Dependency", ReadOnlyHint: true}, + }, + { + Name: ToolNameGetBlockedTasks, + Category: "tasks", + Description: "Get a list of all blocked tasks and what is blocking them.", + Parameters: tools.MustSchemaFor[GetBlockedTasksArgs](), + Handler: tools.NewHandler(t.handler.getBlockedTasks), + Annotations: tools.ToolAnnotations{Title: "Get Blocked Tasks", ReadOnlyHint: true}, + }, + }, nil +} From 11add000fb6a0d01b8b0418dd18b0243271ce8b2 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Sat, 24 Jan 2026 17:38:20 +0100 Subject: [PATCH 02/19] feat: add tests and TUI support for tasks tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unit tests (tasks_test.go): - Test task creation with/without dependencies - Test canStart logic for blocked tasks - Test update enforcement (cannot start blocked) - Test completion unblocking dependents - Test list_tasks output with visual indicators - Test add/remove dependencies - Test circular dependency detection - Test shared instance behavior - Test cross-agent sharing simulation TUI components: - Add taskstool package with sidebar component - Display tasks with dependency indicators in sidebar - Show stats: X done · Y active · Z pending · W blocked - Visual icons: ✓ □ ■ ⚠ - Owner display and blocked-by indicators - Update sidebar to render both todos and tasks E2E test configs: - tasks_dependencies.yaml: single agent with tasks - shared_tasks.yaml: multi-agent with shared tasks Assisted-By: cagent --- e2e/testdata/shared_tasks.yaml | 36 ++ e2e/testdata/tasks_dependencies.yaml | 16 + pkg/tools/builtin/tasks_test.go | 592 ++++++++++++++++++ pkg/tui/components/sidebar/sidebar.go | 11 + pkg/tui/components/tool/taskstool/sidebar.go | 155 +++++ .../components/tool/taskstool/taskstool.go | 20 + pkg/tui/page/chat/runtime_events.go | 11 +- 7 files changed, 838 insertions(+), 3 deletions(-) create mode 100644 e2e/testdata/shared_tasks.yaml create mode 100644 e2e/testdata/tasks_dependencies.yaml create mode 100644 pkg/tools/builtin/tasks_test.go create mode 100644 pkg/tui/components/tool/taskstool/sidebar.go create mode 100644 pkg/tui/components/tool/taskstool/taskstool.go diff --git a/e2e/testdata/shared_tasks.yaml b/e2e/testdata/shared_tasks.yaml new file mode 100644 index 000000000..97b8c2b67 --- /dev/null +++ b/e2e/testdata/shared_tasks.yaml @@ -0,0 +1,36 @@ +version: "2" + +agents: + root: + model: openai/gpt-5-mini + description: Coordinator that delegates to specialized agents + instruction: | + You coordinate work using a shared task list. + Use transfer_task to delegate work to sub-agents. + sub_agents: + - backend + - frontend + toolsets: + - type: tasks + shared: true + - type: transfer_task + + backend: + model: openai/gpt-5-mini + description: Backend developer + instruction: | + You handle backend tasks. + You share a task list with other agents. + toolsets: + - type: tasks + shared: true + + frontend: + model: openai/gpt-5-mini + description: Frontend developer + instruction: | + You handle frontend tasks. + You share a task list with other agents. + toolsets: + - type: tasks + shared: true diff --git a/e2e/testdata/tasks_dependencies.yaml b/e2e/testdata/tasks_dependencies.yaml new file mode 100644 index 000000000..23bfa4803 --- /dev/null +++ b/e2e/testdata/tasks_dependencies.yaml @@ -0,0 +1,16 @@ +version: "2" + +agents: + root: + model: openai/gpt-5-mini + description: Test agent for tasks with dependencies + instruction: | + You are a helpful assistant that uses tasks tools with dependencies. + + When creating tasks: + - Use blocked_by to specify dependencies + - Use owner to assign tasks + + Always use the tasks tools to track work. + toolsets: + - type: tasks diff --git a/pkg/tools/builtin/tasks_test.go b/pkg/tools/builtin/tasks_test.go new file mode 100644 index 000000000..9974381e8 --- /dev/null +++ b/pkg/tools/builtin/tasks_test.go @@ -0,0 +1,592 @@ +package builtin + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/cagent/pkg/tools" +) + +// ============================================================================= +// Unit Tests: Task Creation with Dependencies +// ============================================================================= + +func TestTasksTool_CreateTask_Basic(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + result, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Setup database", + }) + + require.NoError(t, err) + assert.Contains(t, result.Output, "Created task [task_1]: Setup database") + + tasks := tool.handler.tasks.All() + require.Len(t, tasks, 1) + assert.Equal(t, "task_1", tasks[0].ID) + assert.Equal(t, "pending", tasks[0].Status) +} + +func TestTasksTool_CreateTask_WithBlockedBy(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + // Create prerequisite tasks first + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Task 1"}) + require.NoError(t, err) + _, err = tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Task 2"}) + require.NoError(t, err) + + // Create a task that depends on both + result, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Task 3", + BlockedBy: []string{"task_1", "task_2"}, + }) + + require.NoError(t, err) + assert.Contains(t, result.Output, "Created task [task_3]: Task 3") + assert.Contains(t, result.Output, "blocked by task_1, task_2") + + tasks := tool.handler.tasks.All() + require.Len(t, tasks, 3) + assert.Equal(t, []string{"task_1", "task_2"}, tasks[2].BlockedBy) +} + +func TestTasksTool_CreateTask_WithInvalidBlockedBy(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + result, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Some task", + BlockedBy: []string{"task_999"}, + }) + + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "invalid blocked_by reference: task_999 not found") +} + +func TestTasksTool_CreateTask_WithOwner(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + result, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Backend task", + Owner: "backend-dev", + }) + + require.NoError(t, err) + assert.Contains(t, result.Output, "Created task [task_1]: Backend task") + + tasks := tool.handler.tasks.All() + require.Len(t, tasks, 1) + assert.Equal(t, "backend-dev", tasks[0].Owner) +} + +func TestTasksTool_CreateTasks_Batch(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + result, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Research"}, + {Description: "Design", BlockedBy: []string{"task_1"}}, + {Description: "Implement", BlockedBy: []string{"task_2"}}, + }, + }) + + require.NoError(t, err) + assert.Contains(t, result.Output, "Created 3 tasks") + + tasks := tool.handler.tasks.All() + require.Len(t, tasks, 3) + assert.Empty(t, tasks[0].BlockedBy) + assert.Equal(t, []string{"task_1"}, tasks[1].BlockedBy) + assert.Equal(t, []string{"task_2"}, tasks[2].BlockedBy) +} + +// ============================================================================= +// Unit Tests: canStart Logic +// ============================================================================= + +func TestTasksTool_CanStart_NoDependencies(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Independent task"}) + require.NoError(t, err) + + canStart, blockers := tool.handler.canStart("task_1") + assert.True(t, canStart) + assert.Empty(t, blockers) +} + +func TestTasksTool_CanStart_WithPendingBlockers(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Blocker"}) + require.NoError(t, err) + _, err = tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Dependent", + BlockedBy: []string{"task_1"}, + }) + require.NoError(t, err) + + canStart, blockers := tool.handler.canStart("task_2") + assert.False(t, canStart) + assert.Equal(t, []string{"task_1"}, blockers) +} + +func TestTasksTool_CanStart_WithCompletedBlockers(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Blocker"}) + require.NoError(t, err) + _, err = tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Dependent", + BlockedBy: []string{"task_1"}, + }) + require.NoError(t, err) + + // Complete the blocker + _, err = tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_1", Status: "completed"}}, + }) + require.NoError(t, err) + + canStart, blockers := tool.handler.canStart("task_2") + assert.True(t, canStart) + assert.Empty(t, blockers) +} + +func TestTasksTool_CanStart_MultipleBlockers_PartiallyCompleted(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Blocker 1"}, + {Description: "Blocker 2"}, + {Description: "Dependent", BlockedBy: []string{"task_1", "task_2"}}, + }, + }) + require.NoError(t, err) + + // Complete only one blocker + _, err = tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_1", Status: "completed"}}, + }) + require.NoError(t, err) + + canStart, blockers := tool.handler.canStart("task_3") + assert.False(t, canStart) + assert.Equal(t, []string{"task_2"}, blockers) +} + +// ============================================================================= +// Unit Tests: Update with Dependency Enforcement +// ============================================================================= + +func TestTasksTool_UpdateTasks_CannotStartBlocked(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Blocker"}) + require.NoError(t, err) + _, err = tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Blocked", + BlockedBy: []string{"task_1"}, + }) + require.NoError(t, err) + + result, err := tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_2", Status: "in-progress"}}, + }) + + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "cannot start task_2: blocked by task_1") +} + +func TestTasksTool_UpdateTasks_CanStartAfterBlockerCompleted(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Blocker"}) + require.NoError(t, err) + _, err = tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Blocked", + BlockedBy: []string{"task_1"}, + }) + require.NoError(t, err) + + // Complete blocker first + _, err = tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_1", Status: "completed"}}, + }) + require.NoError(t, err) + + // Now can start the dependent + result, err := tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_2", Status: "in-progress"}}, + }) + + require.NoError(t, err) + assert.False(t, result.IsError) + assert.Contains(t, result.Output, "task_2 -> in-progress") +} + +func TestTasksTool_UpdateTasks_CompletionUnblocks(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "First"}, + {Description: "Second", BlockedBy: []string{"task_1"}}, + {Description: "Third", BlockedBy: []string{"task_2"}}, + }, + }) + require.NoError(t, err) + + result, err := tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_1", Status: "completed"}}, + }) + + require.NoError(t, err) + assert.Contains(t, result.Output, "task_1 -> completed") + assert.Contains(t, result.Output, "task_2 is now unblocked") + + // task_3 should still be blocked + canStart, blockers := tool.handler.canStart("task_3") + assert.False(t, canStart) + assert.Equal(t, []string{"task_2"}, blockers) +} + +// ============================================================================= +// Unit Tests: List Tasks +// ============================================================================= + +func TestTasksTool_ListTasks_Empty(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) + + require.NoError(t, err) + assert.Contains(t, result.Output, "No tasks") +} + +func TestTasksTool_ListTasks_WithDependencies(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Research"}, + {Description: "Design", BlockedBy: []string{"task_1"}}, + {Description: "Implement", BlockedBy: []string{"task_2"}}, + }, + }) + require.NoError(t, err) + + result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) + + require.NoError(t, err) + assert.Contains(t, result.Output, "□ [task_1] Research") + assert.Contains(t, result.Output, "⚠ [task_2] Design → blocked by: task_1") + assert.Contains(t, result.Output, "⚠ [task_3] Implement → blocked by: task_2") +} + +func TestTasksTool_ListTasks_StatusIcons(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Done"}, + {Description: "Active"}, + {Description: "Pending"}, + }, + }) + require.NoError(t, err) + + _, err = tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{ + {ID: "task_1", Status: "completed"}, + {ID: "task_2", Status: "in-progress"}, + }, + }) + require.NoError(t, err) + + result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) + + require.NoError(t, err) + assert.Contains(t, result.Output, "✓ [task_1] Done") + assert.Contains(t, result.Output, "■ [task_2] Active") + assert.Contains(t, result.Output, "□ [task_3] Pending") +} + +func TestTasksTool_ListTasks_ShowsOwner(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Backend work", + Owner: "backend-dev", + }) + require.NoError(t, err) + + result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) + + require.NoError(t, err) + assert.Contains(t, result.Output, "(backend-dev)") +} + +func TestTasksTool_ListTasks_Stats(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Task 1"}, + {Description: "Task 2"}, + {Description: "Task 3"}, + {Description: "Task 4", BlockedBy: []string{"task_1"}}, + }, + }) + require.NoError(t, err) + + _, err = tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{ + {ID: "task_1", Status: "completed"}, + {ID: "task_2", Status: "in-progress"}, + }, + }) + require.NoError(t, err) + + result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) + + require.NoError(t, err) + assert.Contains(t, result.Output, "1 done") + assert.Contains(t, result.Output, "1 in progress") + assert.Contains(t, result.Output, "2 pending") +} + +// ============================================================================= +// Unit Tests: Add/Remove Dependencies +// ============================================================================= + +func TestTasksTool_AddDependency(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "First"}, + {Description: "Second"}, + }, + }) + require.NoError(t, err) + + result, err := tool.handler.addDependency(t.Context(), AddTaskDependencyArgs{ + TaskID: "task_2", + BlockedBy: []string{"task_1"}, + }) + + require.NoError(t, err) + assert.Contains(t, result.Output, "Added dependency: task_2 is now blocked by task_1") + + tasks := tool.handler.tasks.All() + assert.Equal(t, []string{"task_1"}, tasks[1].BlockedBy) + assert.Contains(t, tasks[0].Blocks, "task_2") +} + +func TestTasksTool_AddDependency_PreventCircular(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "First"}) + require.NoError(t, err) + _, err = tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Second", + BlockedBy: []string{"task_1"}, + }) + require.NoError(t, err) + + // Try circular: task_1 blocked by task_2 + result, err := tool.handler.addDependency(t.Context(), AddTaskDependencyArgs{ + TaskID: "task_1", + BlockedBy: []string{"task_2"}, + }) + + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "circular dependency detected") +} + +func TestTasksTool_AddDependency_PreventSelfDependency(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Task"}) + require.NoError(t, err) + + result, err := tool.handler.addDependency(t.Context(), AddTaskDependencyArgs{ + TaskID: "task_1", + BlockedBy: []string{"task_1"}, + }) + + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "cannot depend on itself") +} + +func TestTasksTool_RemoveDependency(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "First"}) + require.NoError(t, err) + _, err = tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Second", + BlockedBy: []string{"task_1"}, + }) + require.NoError(t, err) + + result, err := tool.handler.removeDependency(t.Context(), RemoveTaskDependencyArgs{ + TaskID: "task_2", + BlockedBy: []string{"task_1"}, + }) + + require.NoError(t, err) + assert.Contains(t, result.Output, "Removed dependency") + + tasks := tool.handler.tasks.All() + assert.Empty(t, tasks[1].BlockedBy) + assert.NotContains(t, tasks[0].Blocks, "task_2") +} + +// ============================================================================= +// Unit Tests: Get Blocked Tasks +// ============================================================================= + +func TestTasksTool_GetBlockedTasks(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Root"}, + {Description: "Child 1", BlockedBy: []string{"task_1"}}, + {Description: "Child 2", BlockedBy: []string{"task_1"}}, + }, + }) + require.NoError(t, err) + + result, err := tool.handler.getBlockedTasks(t.Context(), GetBlockedTasksArgs{}) + + require.NoError(t, err) + assert.Contains(t, result.Output, "task_2") + assert.Contains(t, result.Output, "task_3") + assert.Contains(t, result.Output, "blocked by: task_1") +} + +func TestTasksTool_GetBlockedTasks_FilterByBlocker(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Blocker A"}, + {Description: "Blocker B"}, + {Description: "Blocked by A", BlockedBy: []string{"task_1"}}, + {Description: "Blocked by B", BlockedBy: []string{"task_2"}}, + }, + }) + require.NoError(t, err) + + result, err := tool.handler.getBlockedTasks(t.Context(), GetBlockedTasksArgs{ + BlockedBy: "task_1", + }) + + require.NoError(t, err) + assert.Contains(t, result.Output, "task_3") + assert.NotContains(t, result.Output, "task_4") +} + +// ============================================================================= +// Unit Tests: Shared Instance +// ============================================================================= + +func TestTasksTool_SharedInstance(t *testing.T) { + t.Parallel() + + shared1 := NewSharedTasksTool() + shared2 := NewSharedTasksTool() + assert.Same(t, shared1, shared2, "NewSharedTasksTool should return same instance") + + nonShared1 := NewTasksTool() + nonShared2 := NewTasksTool() + assert.NotSame(t, nonShared1, nonShared2, "NewTasksTool should return different instances") +} + +func TestTasksTool_CrossAgentSharing(t *testing.T) { + // Simulates two agents sharing a task list + tool := NewTasksTool() + + // Agent A creates a task + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Task from Agent A", + Owner: "agent-a", + }) + require.NoError(t, err) + + // Agent B creates a dependent task + _, err = tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Task from Agent B", + Owner: "agent-b", + BlockedBy: []string{"task_1"}, + }) + require.NoError(t, err) + + // Both tasks visible + tasks := tool.handler.tasks.All() + require.Len(t, tasks, 2) + assert.Equal(t, "agent-a", tasks[0].Owner) + assert.Equal(t, "agent-b", tasks[1].Owner) + + // Agent A completes their task + result, err := tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_1", Status: "completed"}}, + }) + require.NoError(t, err) + assert.Contains(t, result.Output, "task_2 is now unblocked") + + // Agent B can now start + canStart, _ := tool.handler.canStart("task_2") + assert.True(t, canStart) +} + +// ============================================================================= +// Unit Tests: Schema +// ============================================================================= + +func TestTasksTool_Schema(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + allTools, err := tool.Tools(t.Context()) + require.NoError(t, err) + require.Len(t, allTools, 7) + + // Verify all tools have correct category + for _, tt := range allTools { + assert.Equal(t, "tasks", tt.Category) + } +} diff --git a/pkg/tui/components/sidebar/sidebar.go b/pkg/tui/components/sidebar/sidebar.go index c9ee159dc..526952b44 100644 --- a/pkg/tui/components/sidebar/sidebar.go +++ b/pkg/tui/components/sidebar/sidebar.go @@ -20,6 +20,7 @@ import ( "github.com/docker/cagent/pkg/tui/components/scrollbar" "github.com/docker/cagent/pkg/tui/components/spinner" "github.com/docker/cagent/pkg/tui/components/tab" + "github.com/docker/cagent/pkg/tui/components/tool/taskstool" "github.com/docker/cagent/pkg/tui/components/tool/todotool" "github.com/docker/cagent/pkg/tui/components/toolcommon" "github.com/docker/cagent/pkg/tui/core/layout" @@ -43,6 +44,7 @@ type Model interface { SetTokenUsage(event *runtime.TokenUsageEvent) SetTodos(result *tools.ToolCallResult) error + SetTasks(result *tools.ToolCallResult) error SetMode(mode Mode) SetAgentInfo(agentName, model, description string) SetTeamInfo(availableAgents []runtime.AgentDetails) @@ -73,6 +75,7 @@ type model struct { sessionUsage map[string]*runtime.Usage // sessionID -> latest usage snapshot sessionAgent map[string]string // sessionID -> agent name todoComp *todotool.SidebarComponent + tasksComp *taskstool.SidebarComponent mcpInit bool ragIndexing map[string]*ragIndexingState // strategy name -> indexing state spinner spinner.Spinner @@ -112,6 +115,7 @@ func New(sessionState *service.SessionState, opts ...Option) Model { sessionUsage: make(map[string]*runtime.Usage), sessionAgent: make(map[string]string), todoComp: todotool.NewSidebarComponent(), + tasksComp: taskstool.NewSidebarComponent(), spinner: spinner.New(spinner.ModeSpinnerOnly, styles.SpinnerDotsHighlightStyle), sessionTitle: "New session", ragIndexing: make(map[string]*ragIndexingState), @@ -148,6 +152,10 @@ func (m *model) SetTodos(result *tools.ToolCallResult) error { return m.todoComp.SetTodos(result) } +func (m *model) SetTasks(result *tools.ToolCallResult) error { + return m.tasksComp.SetTasks(result) +} + // SetAgentInfo sets the current agent information and updates the model in availableAgents func (m *model) SetAgentInfo(agentName, modelID, description string) { m.currentAgent = agentName @@ -526,6 +534,9 @@ func (m *model) renderSections(contentWidth int) []string { m.todoComp.SetSize(contentWidth) appendSection(strings.TrimSuffix(m.todoComp.Render(), "\n")) + m.tasksComp.SetSize(contentWidth) + appendSection(strings.TrimSuffix(m.tasksComp.Render(), "\n")) + return lines } diff --git a/pkg/tui/components/tool/taskstool/sidebar.go b/pkg/tui/components/tool/taskstool/sidebar.go new file mode 100644 index 000000000..dd7d613c3 --- /dev/null +++ b/pkg/tui/components/tool/taskstool/sidebar.go @@ -0,0 +1,155 @@ +package taskstool + +import ( + "fmt" + "strings" + + "charm.land/lipgloss/v2" + + "github.com/docker/cagent/pkg/tools" + "github.com/docker/cagent/pkg/tools/builtin" + "github.com/docker/cagent/pkg/tui/components/tab" + "github.com/docker/cagent/pkg/tui/components/toolcommon" + "github.com/docker/cagent/pkg/tui/styles" +) + +// SidebarComponent represents the tasks display component for the sidebar +type SidebarComponent struct { + tasks []builtin.Task + width int +} + +func NewSidebarComponent() *SidebarComponent { + return &SidebarComponent{ + width: 20, + } +} + +func (c *SidebarComponent) SetSize(width int) { + c.width = width +} + +func (c *SidebarComponent) SetTasks(result *tools.ToolCallResult) error { + if result == nil || result.Meta == nil { + return nil + } + + tasks, ok := result.Meta.([]builtin.Task) + if !ok { + return nil + } + + c.tasks = tasks + return nil +} + +func (c *SidebarComponent) Render() string { + if len(c.tasks) == 0 { + return "" + } + + var lines []string + + // Add summary stats + lines = append(lines, c.renderStats()) + lines = append(lines, "") + + // Render each task + for _, task := range c.tasks { + lines = append(lines, c.renderTaskLine(task)) + } + + return c.renderTab("Tasks", strings.Join(lines, "\n")) +} + +func (c *SidebarComponent) renderStats() string { + var completed, inProgress, pending, blocked int + for _, task := range c.tasks { + switch task.Status { + case "completed": + completed++ + case "in-progress": + inProgress++ + default: + pending++ + if len(task.BlockedBy) > 0 && !c.allBlockersCompleted(task.BlockedBy) { + blocked++ + } + } + } + + var parts []string + if completed > 0 { + parts = append(parts, fmt.Sprintf("%d done", completed)) + } + if inProgress > 0 { + parts = append(parts, fmt.Sprintf("%d active", inProgress)) + } + if pending > 0 { + parts = append(parts, fmt.Sprintf("%d pending", pending)) + } + if blocked > 0 { + parts = append(parts, styles.WarningStyle.Render(fmt.Sprintf("%d blocked", blocked))) + } + + return strings.Join(parts, " · ") +} + +func (c *SidebarComponent) allBlockersCompleted(blockerIDs []string) bool { + for _, blockerID := range blockerIDs { + for _, task := range c.tasks { + if task.ID == blockerID && task.Status != "completed" { + return false + } + } + } + return true +} + +func (c *SidebarComponent) renderTaskLine(task builtin.Task) string { + icon, iconStyle := renderTaskIcon(task.Status) + + // Check if blocked + isBlocked := len(task.BlockedBy) > 0 && !c.allBlockersCompleted(task.BlockedBy) + if isBlocked && task.Status == "pending" { + icon = "⚠" + iconStyle = styles.WarningStyle + } + + // Build the line + prefix := iconStyle.Render(icon) + " " + prefixWidth := lipgloss.Width(prefix) + + // Calculate available width for description + maxDescWidth := c.width - prefixWidth + + // Add owner suffix if present + var ownerSuffix string + if task.Owner != "" { + ownerSuffix = styles.MutedStyle.Render(" (" + task.Owner + ")") + maxDescWidth -= lipgloss.Width(ownerSuffix) + } + + description := toolcommon.TruncateText(task.Description, maxDescWidth) + + // Apply strikethrough for completed items + if task.Status == "completed" { + description = styles.CompletedStyle.Strikethrough(true).Render(description) + } else { + description = styles.TabPrimaryStyle.Render(description) + } + + line := prefix + description + ownerSuffix + + // Add blocked-by indicator on next line if blocked + if isBlocked { + blockerText := styles.MutedStyle.Render(" → blocked by: " + strings.Join(task.BlockedBy, ", ")) + line += "\n" + toolcommon.TruncateText(blockerText, c.width) + } + + return line +} + +func (c *SidebarComponent) renderTab(title, content string) string { + return tab.Render(title, content, c.width) +} diff --git a/pkg/tui/components/tool/taskstool/taskstool.go b/pkg/tui/components/tool/taskstool/taskstool.go new file mode 100644 index 000000000..773dcc54b --- /dev/null +++ b/pkg/tui/components/tool/taskstool/taskstool.go @@ -0,0 +1,20 @@ +package taskstool + +import ( + "charm.land/lipgloss/v2" + + "github.com/docker/cagent/pkg/tui/styles" +) + +func renderTaskIcon(status string) (string, lipgloss.Style) { + switch status { + case "pending": + return "□", styles.ToBeDoneStyle + case "in-progress": + return "■", styles.InProgressStyle + case "completed": + return "✓", styles.CompletedStyle + default: + return "?", styles.ToBeDoneStyle + } +} diff --git a/pkg/tui/page/chat/runtime_events.go b/pkg/tui/page/chat/runtime_events.go index 0adf10625..5e68ff005 100644 --- a/pkg/tui/page/chat/runtime_events.go +++ b/pkg/tui/page/chat/runtime_events.go @@ -219,9 +219,14 @@ func (p *chatPage) handleToolCallResponse(msg *runtime.ToolCallResponseEvent) te } toolCmd := p.messages.AddToolResult(msg, status) - // Update todo sidebar if this is a todo tool - if msg.ToolDefinition.Category == "todo" && !msg.Result.IsError { - _ = p.sidebar.SetTodos(msg.Result) + // Update sidebar for todo/tasks tools + if !msg.Result.IsError { + switch msg.ToolDefinition.Category { + case "todo": + _ = p.sidebar.SetTodos(msg.Result) + case "tasks": + _ = p.sidebar.SetTasks(msg.Result) + } } return tea.Batch(toolCmd, p.messages.ScrollToBottom(), spinnerCmd) From ad708d721f058fe27f4464a7c8d2dc8dc14e78eb Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Sat, 24 Jan 2026 17:41:17 +0100 Subject: [PATCH 03/19] chore: use tasks tool in golang_developer agent Replace todo with tasks to leverage dependency management for more structured development workflows. Assisted-By: cagent --- golang_developer.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/golang_developer.yaml b/golang_developer.yaml index 0749a0b63..a9efd1ee6 100755 --- a/golang_developer.yaml +++ b/golang_developer.yaml @@ -97,7 +97,7 @@ agents: toolsets: - type: filesystem - type: shell - - type: todo + - type: tasks - type: fetch sub_agents: - librarian From 67ff0c235a5c6bdf4a77898db018f7c755b0eb71 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Sat, 24 Jan 2026 17:51:11 +0100 Subject: [PATCH 04/19] fix: address lint issues in tasks tool - Fix gci import formatting in tasks.go - Combine append calls in taskstool/sidebar.go (gocritic) - Use integer range syntax for loop (intrange) Assisted-By: cagent --- pkg/tools/builtin/tasks.go | 16 ++++++++-------- pkg/tui/components/tool/taskstool/sidebar.go | 3 +-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index 1ad682941..470d4fde3 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -11,13 +11,13 @@ import ( ) const ( - ToolNameCreateTask = "create_task" - ToolNameCreateTasks = "create_tasks" - ToolNameUpdateTasks = "update_tasks" - ToolNameListTasks = "list_tasks" - ToolNameAddTaskDep = "add_task_dependency" - ToolNameRemoveTaskDep = "remove_task_dependency" - ToolNameGetBlockedTasks = "get_blocked_tasks" + ToolNameCreateTask = "create_task" + ToolNameCreateTasks = "create_tasks" + ToolNameUpdateTasks = "update_tasks" + ToolNameListTasks = "list_tasks" + ToolNameAddTaskDep = "add_task_dependency" + ToolNameRemoveTaskDep = "remove_task_dependency" + ToolNameGetBlockedTasks = "get_blocked_tasks" ) type TasksTool struct { @@ -224,7 +224,7 @@ func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (* for _, blockerID := range item.BlockedBy { if !h.taskExists(blockerID) { isEarlierInBatch := false - for j := 0; j < i; j++ { + for j := range i { if fmt.Sprintf("task_%d", start+j+1) == blockerID { isEarlierInBatch = true break diff --git a/pkg/tui/components/tool/taskstool/sidebar.go b/pkg/tui/components/tool/taskstool/sidebar.go index dd7d613c3..141408fd9 100644 --- a/pkg/tui/components/tool/taskstool/sidebar.go +++ b/pkg/tui/components/tool/taskstool/sidebar.go @@ -51,8 +51,7 @@ func (c *SidebarComponent) Render() string { var lines []string // Add summary stats - lines = append(lines, c.renderStats()) - lines = append(lines, "") + lines = append(lines, c.renderStats(), "") // Render each task for _, task := range c.tasks { From 26b45aaaf2a33e9726bbfb1cc27451a731ed4cdf Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Sun, 25 Jan 2026 18:56:39 +0100 Subject: [PATCH 05/19] fix(tui): show blocker descriptions instead of IDs in tasks sidebar Assisted-By: cagent --- pkg/tui/components/tool/taskstool/sidebar.go | 27 +++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/tui/components/tool/taskstool/sidebar.go b/pkg/tui/components/tool/taskstool/sidebar.go index 141408fd9..5a0b6e61a 100644 --- a/pkg/tui/components/tool/taskstool/sidebar.go +++ b/pkg/tui/components/tool/taskstool/sidebar.go @@ -142,7 +142,8 @@ func (c *SidebarComponent) renderTaskLine(task builtin.Task) string { // Add blocked-by indicator on next line if blocked if isBlocked { - blockerText := styles.MutedStyle.Render(" → blocked by: " + strings.Join(task.BlockedBy, ", ")) + blockerNames := c.getBlockerDescriptions(task.BlockedBy) + blockerText := styles.MutedStyle.Render(" → blocked by: " + strings.Join(blockerNames, ", ")) line += "\n" + toolcommon.TruncateText(blockerText, c.width) } @@ -152,3 +153,27 @@ func (c *SidebarComponent) renderTaskLine(task builtin.Task) string { func (c *SidebarComponent) renderTab(title, content string) string { return tab.Render(title, content, c.width) } + +// getBlockerDescriptions returns short descriptions for the given blocker IDs +func (c *SidebarComponent) getBlockerDescriptions(blockerIDs []string) []string { + result := make([]string, 0, len(blockerIDs)) + for _, id := range blockerIDs { + found := false + for _, task := range c.tasks { + if task.ID == id { + // Truncate description to keep it short + desc := task.Description + if len(desc) > 20 { + desc = desc[:17] + "..." + } + result = append(result, desc) + found = true + break + } + } + if !found { + result = append(result, id) // Fallback to ID if not found + } + } + return result +} From 7633b5d111bbb9428f17aea81de07dfb84f74f22 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Sun, 25 Jan 2026 18:56:46 +0100 Subject: [PATCH 06/19] feat(tasks): add cross-session persistence Add file-based persistence for tasks with --task-list flag or CAGENT_TASK_LIST_ID env var. Tasks are stored in ~/.cagent/tasks/.json. - Add TaskStore interface with FileTaskStore and MemoryTaskStore - Add GetTasksDir() to paths package - Add TaskListID to RuntimeConfig - Add --task-list flag and CAGENT_TASK_LIST_ID env var support - Lazy loading: tasks loaded on first access - Atomic writes with temp file + rename Without --task-list flag, tasks remain in-memory only (no persistence). Assisted-By: cagent --- cmd/root/flags.go | 22 ++ cmd/root/flags_test.go | 50 ++++ pkg/config/runtime.go | 1 + pkg/paths/paths.go | 5 + pkg/teamloader/registry.go | 13 +- pkg/tools/builtin/tasks.go | 75 +++++- pkg/tools/builtin/tasks_store.go | 133 +++++++++ pkg/tools/builtin/tasks_store_test.go | 268 +++++++++++++++++++ pkg/tui/components/tool/taskstool/sidebar.go | 34 +-- 9 files changed, 583 insertions(+), 18 deletions(-) create mode 100644 pkg/tools/builtin/tasks_store.go create mode 100644 pkg/tools/builtin/tasks_store_test.go diff --git a/cmd/root/flags.go b/cmd/root/flags.go index b441ef217..416bc47e0 100644 --- a/cmd/root/flags.go +++ b/cmd/root/flags.go @@ -16,10 +16,13 @@ import ( const ( flagModelsGateway = "models-gateway" envModelsGateway = "CAGENT_MODELS_GATEWAY" + flagTaskList = "task-list" + envTaskListID = "CAGENT_TASK_LIST_ID" ) func addRuntimeConfigFlags(cmd *cobra.Command, runConfig *config.RuntimeConfig) { addGatewayFlags(cmd, runConfig) + addTaskListFlags(cmd, runConfig) cmd.PersistentFlags().StringSliceVar(&runConfig.EnvFiles, "env-from-file", nil, "Set environment variables from file") cmd.PersistentFlags().BoolVar(&runConfig.GlobalCodeMode, "code-mode-tools", false, "Provide a single tool to call other tools via Javascript") cmd.PersistentFlags().StringVar(&runConfig.WorkingDir, "working-dir", "", "Set the working directory for the session (applies to tools and relative paths)") @@ -94,3 +97,22 @@ func addGatewayFlags(cmd *cobra.Command, runConfig *config.RuntimeConfig) { return nil } } + +func addTaskListFlags(cmd *cobra.Command, runConfig *config.RuntimeConfig) { + cmd.PersistentFlags().StringVar(&runConfig.TaskListID, flagTaskList, "", "Use a persistent task list with the given ID") + + persistentPreRunE := cmd.PersistentPreRunE + cmd.PersistentPreRunE = func(c *cobra.Command, args []string) error { + // Precedence: CLI flag > environment variable + if runConfig.TaskListID != "" { + logFlagShadowing(os.Getenv(envTaskListID), envTaskListID, flagTaskList) + } else if taskListID := os.Getenv(envTaskListID); taskListID != "" { + runConfig.TaskListID = taskListID + } + + if persistentPreRunE != nil { + return persistentPreRunE(c, args) + } + return nil + } +} diff --git a/cmd/root/flags_test.go b/cmd/root/flags_test.go index b2a9c330e..fb7809021 100644 --- a/cmd/root/flags_test.go +++ b/cmd/root/flags_test.go @@ -152,3 +152,53 @@ func TestCanonize(t *testing.T) { }) } } + +func TestTaskListLogic(t *testing.T) { + tests := []struct { + name string + env string + args []string + expected string + }{ + { + name: "empty_by_default", + expected: "", + }, + { + name: "from_env", + env: "my-project", + expected: "my-project", + }, + { + name: "from_cli", + args: []string{"--task-list", "cli-project"}, + expected: "cli-project", + }, + { + name: "cli_overrides_env", + env: "env-project", + args: []string{"--task-list", "cli-project"}, + expected: "cli-project", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("CAGENT_TASK_LIST_ID", tt.env) + + cmd := &cobra.Command{ + RunE: func(*cobra.Command, []string) error { + return nil + }, + } + runConfig := config.RuntimeConfig{} + addTaskListFlags(cmd, &runConfig) + + cmd.SetArgs(tt.args) + err := cmd.Execute() + + require.NoError(t, err) + assert.Equal(t, tt.expected, runConfig.TaskListID) + }) + } +} diff --git a/pkg/config/runtime.go b/pkg/config/runtime.go index dd64bcbb4..12673beb4 100644 --- a/pkg/config/runtime.go +++ b/pkg/config/runtime.go @@ -20,6 +20,7 @@ type Config struct { ModelsGateway string GlobalCodeMode bool WorkingDir string + TaskListID string // ID for persistent task list (from --task-list or CAGENT_TASK_LIST_ID) } func (runConfig *RuntimeConfig) Clone() *RuntimeConfig { diff --git a/pkg/paths/paths.go b/pkg/paths/paths.go index e4b20230e..535a8d0d3 100644 --- a/pkg/paths/paths.go +++ b/pkg/paths/paths.go @@ -41,3 +41,8 @@ func GetHomeDir() string { } return filepath.Clean(homeDir) } + +// GetTasksDir returns the directory for storing task lists. +func GetTasksDir() string { + return filepath.Join(GetDataDir(), "tasks") +} diff --git a/pkg/teamloader/registry.go b/pkg/teamloader/registry.go index c218fdd13..5ba00748a 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -81,10 +81,21 @@ func createTodoTool(_ context.Context, toolset latest.Toolset, _ string, _ *conf return builtin.NewTodoTool(), nil } -func createTasksTool(_ context.Context, toolset latest.Toolset, _ string, _ *config.RuntimeConfig) (tools.ToolSet, error) { +func createTasksTool(_ context.Context, toolset latest.Toolset, _ string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { + // Determine task list ID: CLI/env takes precedence + listID := runConfig.TaskListID + if toolset.Shared { + // Shared mode uses in-memory storage (no persistence) return builtin.NewSharedTasksTool(), nil } + + if listID != "" { + // Use file-based persistence with the specified list ID + return builtin.NewTasksToolWithStore(builtin.NewFileTaskStore(listID)), nil + } + + // Default: in-memory only (no persistence) return builtin.NewTasksTool(), nil } diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index 470d4fde3..9be95cb13 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -3,6 +3,7 @@ package builtin import ( "context" "fmt" + "log/slog" "strings" "sync" @@ -78,19 +79,60 @@ type GetBlockedTasksArgs struct { } type tasksHandler struct { - tasks *concurrent.Slice[Task] + tasks *concurrent.Slice[Task] + store TaskStore + loaded bool } -var NewSharedTasksTool = sync.OnceValue(NewTasksTool) +// Shared instance for shared: true (no persistence) +var NewSharedTasksTool = sync.OnceValue(func() *TasksTool { + return NewTasksToolWithStore(NewMemoryTaskStore()) +}) +// NewTasksTool creates a new TasksTool with in-memory storage only func NewTasksTool() *TasksTool { + return NewTasksToolWithStore(NewMemoryTaskStore()) +} + +// NewTasksToolWithStore creates a new TasksTool with the specified store +func NewTasksToolWithStore(store TaskStore) *TasksTool { return &TasksTool{ handler: &tasksHandler{ tasks: concurrent.NewSlice[Task](), + store: store, }, } } +// ensureLoaded loads tasks from store on first access (lazy loading) +func (h *tasksHandler) ensureLoaded() { + if h.loaded { + return + } + h.loaded = true + + tasks, err := h.store.Load() + if err != nil { + slog.Error("Failed to load tasks from store", "error", err) + return + } + + for _, task := range tasks { + h.tasks.Append(task) + } + + if len(tasks) > 0 { + slog.Debug("Loaded tasks from store", "count", len(tasks)) + } +} + +// save persists tasks to store +func (h *tasksHandler) save() { + if err := h.store.Save(h.tasks.All()); err != nil { + slog.Error("Failed to save tasks to store", "error", err) + } +} + func (t *TasksTool) Instructions() string { return `## Using the Tasks Tools @@ -186,6 +228,8 @@ func (h *tasksHandler) getUnblockedTasks(completedID string) []string { } func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*tools.ToolCallResult, error) { + h.ensureLoaded() + for _, blockerID := range params.BlockedBy { if !h.taskExists(blockerID) { return tools.ResultError(fmt.Sprintf("invalid blocked_by reference: %s not found", blockerID)), nil @@ -209,6 +253,9 @@ func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*to }) } } + + h.save() + var output strings.Builder fmt.Fprintf(&output, "Created task [%s]: %s", id, params.Description) if len(params.BlockedBy) > 0 { @@ -218,6 +265,8 @@ func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*to } func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (*tools.ToolCallResult, error) { + h.ensureLoaded() + start := h.tasks.Length() var createdIDs []string for i, item := range params.Tasks { @@ -255,6 +304,9 @@ func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (* } } } + + h.save() + return &tools.ToolCallResult{ Output: fmt.Sprintf("Created %d tasks: %s", len(params.Tasks), strings.Join(createdIDs, ", ")), Meta: h.tasks.All(), @@ -262,6 +314,8 @@ func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (* } func (h *tasksHandler) updateTasks(_ context.Context, params UpdateTasksArgs) (*tools.ToolCallResult, error) { + h.ensureLoaded() + var notFound, updated, blocked, newlyUnblocked []string for _, update := range params.Updates { task, idx := h.findTask(update.ID) @@ -318,6 +372,9 @@ func (h *tasksHandler) updateTasks(_ context.Context, params UpdateTasksArgs) (* if h.allCompleted() { h.tasks.Clear() } + + h.save() + return &tools.ToolCallResult{Output: output.String(), Meta: h.tasks.All()}, nil } @@ -337,6 +394,8 @@ func (h *tasksHandler) allCompleted() bool { } func (h *tasksHandler) listTasks(_ context.Context, _ tools.ToolCall) (*tools.ToolCallResult, error) { + h.ensureLoaded() + var output strings.Builder var completed, inProgress, pending, blockedCount int h.tasks.Range(func(_ int, task Task) bool { @@ -387,6 +446,8 @@ func (h *tasksHandler) listTasks(_ context.Context, _ tools.ToolCall) (*tools.To } func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependencyArgs) (*tools.ToolCallResult, error) { + h.ensureLoaded() + task, idx := h.findTask(params.TaskID) if idx == -1 { return tools.ResultError(fmt.Sprintf("task not found: %s", params.TaskID)), nil @@ -436,6 +497,9 @@ func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependency }) } } + + h.save() + return &tools.ToolCallResult{ Output: fmt.Sprintf("Added dependency: %s is now blocked by %s", params.TaskID, strings.Join(added, ", ")), Meta: h.tasks.All(), @@ -443,6 +507,8 @@ func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependency } func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDependencyArgs) (*tools.ToolCallResult, error) { + h.ensureLoaded() + task, idx := h.findTask(params.TaskID) if idx == -1 { return tools.ResultError(fmt.Sprintf("task not found: %s", params.TaskID)), nil @@ -484,6 +550,9 @@ func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDepe }) } } + + h.save() + return &tools.ToolCallResult{ Output: fmt.Sprintf("Removed dependency: %s is no longer blocked by %s", params.TaskID, strings.Join(removed, ", ")), Meta: h.tasks.All(), @@ -491,6 +560,8 @@ func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDepe } func (h *tasksHandler) getBlockedTasks(_ context.Context, params GetBlockedTasksArgs) (*tools.ToolCallResult, error) { + h.ensureLoaded() + var output strings.Builder output.WriteString("Blocked tasks:\n") found := false diff --git a/pkg/tools/builtin/tasks_store.go b/pkg/tools/builtin/tasks_store.go new file mode 100644 index 000000000..bb76c5890 --- /dev/null +++ b/pkg/tools/builtin/tasks_store.go @@ -0,0 +1,133 @@ +package builtin + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sync" + + "github.com/docker/cagent/pkg/paths" +) + +// TaskListFile represents the JSON structure for persisted task lists +type TaskListFile struct { + Version int `json:"version"` + Tasks []Task `json:"tasks"` +} + +const taskListFileVersion = 1 + +// TaskStore defines the interface for task persistence +type TaskStore interface { + // Load loads tasks from the store. Returns empty slice if not found. + Load() ([]Task, error) + // Save persists tasks to the store. + Save(tasks []Task) error +} + +// FileTaskStore implements TaskStore using a JSON file +type FileTaskStore struct { + listID string + baseDir string + mu sync.RWMutex +} + +// NewFileTaskStore creates a new file-based task store +func NewFileTaskStore(listID string) *FileTaskStore { + return &FileTaskStore{ + listID: listID, + baseDir: paths.GetTasksDir(), + } +} + +// NewFileTaskStoreWithDir creates a file-based task store with a custom directory (for testing) +func NewFileTaskStoreWithDir(listID, baseDir string) *FileTaskStore { + return &FileTaskStore{ + listID: listID, + baseDir: baseDir, + } +} + +func (s *FileTaskStore) filePath() string { + // Sanitize listID to be safe as filename + safeID := filepath.Base(s.listID) + return filepath.Join(s.baseDir, safeID+".json") +} + +// Load loads tasks from the JSON file +func (s *FileTaskStore) Load() ([]Task, error) { + s.mu.RLock() + defer s.mu.RUnlock() + + path := s.filePath() + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + // File doesn't exist yet - return empty list + return []Task{}, nil + } + return nil, fmt.Errorf("reading task file: %w", err) + } + + var file TaskListFile + if err := json.Unmarshal(data, &file); err != nil { + return nil, fmt.Errorf("parsing task file: %w", err) + } + + return file.Tasks, nil +} + +// Save persists tasks to the JSON file +func (s *FileTaskStore) Save(tasks []Task) error { + s.mu.Lock() + defer s.mu.Unlock() + + path := s.filePath() + + // Ensure directory exists + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("creating tasks directory: %w", err) + } + + file := TaskListFile{ + Version: taskListFileVersion, + Tasks: tasks, + } + + data, err := json.MarshalIndent(file, "", " ") + if err != nil { + return fmt.Errorf("marshaling tasks: %w", err) + } + + // Write atomically using temp file + rename + tmpPath := path + ".tmp" + if err := os.WriteFile(tmpPath, data, 0o600); err != nil { + return fmt.Errorf("writing task file: %w", err) + } + + if err := os.Rename(tmpPath, path); err != nil { + os.Remove(tmpPath) // Clean up on failure + return fmt.Errorf("renaming task file: %w", err) + } + + return nil +} + +// MemoryTaskStore implements TaskStore with in-memory storage only (no persistence) +// Used when no listID is provided +type MemoryTaskStore struct{} + +func NewMemoryTaskStore() *MemoryTaskStore { + return &MemoryTaskStore{} +} + +func (s *MemoryTaskStore) Load() ([]Task, error) { + return []Task{}, nil +} + +func (s *MemoryTaskStore) Save(_ []Task) error { + // No-op for memory store + return nil +} diff --git a/pkg/tools/builtin/tasks_store_test.go b/pkg/tools/builtin/tasks_store_test.go new file mode 100644 index 000000000..12046ed95 --- /dev/null +++ b/pkg/tools/builtin/tasks_store_test.go @@ -0,0 +1,268 @@ +package builtin + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/cagent/pkg/tools" +) + +func TestFileTaskStore_SaveAndLoad(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + store := NewFileTaskStoreWithDir("test-project", tmpDir) + + // Initially empty + tasks, err := store.Load() + require.NoError(t, err) + assert.Empty(t, tasks) + + // Save some tasks + tasksToSave := []Task{ + {ID: "task_1", Description: "First task", Status: "pending"}, + {ID: "task_2", Description: "Second task", Status: "in-progress", BlockedBy: []string{"task_1"}}, + } + err = store.Save(tasksToSave) + require.NoError(t, err) + + // Load them back + loaded, err := store.Load() + require.NoError(t, err) + require.Len(t, loaded, 2) + assert.Equal(t, "task_1", loaded[0].ID) + assert.Equal(t, "task_2", loaded[1].ID) + assert.Equal(t, []string{"task_1"}, loaded[1].BlockedBy) +} + +func TestFileTaskStore_FileCreatedOnSave(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + store := NewFileTaskStoreWithDir("my-project", tmpDir) + expectedPath := filepath.Join(tmpDir, "my-project.json") + + // File should not exist yet + _, err := os.Stat(expectedPath) + assert.True(t, os.IsNotExist(err)) + + // Save creates the file + err = store.Save([]Task{{ID: "task_1", Description: "Test", Status: "pending"}}) + require.NoError(t, err) + + // File should now exist + _, err = os.Stat(expectedPath) + assert.NoError(t, err) +} + +func TestFileTaskStore_LoadNonExistentFile(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + store := NewFileTaskStoreWithDir("nonexistent", tmpDir) + + // Should return empty list, not error + tasks, err := store.Load() + require.NoError(t, err) + assert.Empty(t, tasks) +} + +func TestFileTaskStore_SanitizesListID(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + // Try to use path traversal - should be sanitized + store := NewFileTaskStoreWithDir("../../../etc/passwd", tmpDir) + + err := store.Save([]Task{{ID: "task_1", Description: "Test", Status: "pending"}}) + require.NoError(t, err) + + // File should be created in tmpDir with sanitized name, not elsewhere + expectedPath := filepath.Join(tmpDir, "passwd.json") + _, err = os.Stat(expectedPath) + assert.NoError(t, err) +} + +func TestMemoryTaskStore_NoOp(t *testing.T) { + t.Parallel() + + store := NewMemoryTaskStore() + + // Load always returns empty + tasks, err := store.Load() + require.NoError(t, err) + assert.Empty(t, tasks) + + // Save is a no-op + err = store.Save([]Task{{ID: "task_1", Description: "Test", Status: "pending"}}) + require.NoError(t, err) + + // Still empty after save + tasks, err = store.Load() + require.NoError(t, err) + assert.Empty(t, tasks) +} + +func TestTasksToolWithStore_Persistence(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + + // Create first tool instance and add a task + store1 := NewFileTaskStoreWithDir("persistent-test", tmpDir) + tool1 := NewTasksToolWithStore(store1) + + _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Persistent task", + }) + require.NoError(t, err) + + // Create second tool instance with same store ID + store2 := NewFileTaskStoreWithDir("persistent-test", tmpDir) + tool2 := NewTasksToolWithStore(store2) + + // Should load the task from the first instance + result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) + require.NoError(t, err) + assert.Contains(t, result.Output, "Persistent task") +} + +func TestTasksToolWithStore_LazyLoading(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + + // Pre-populate a task file + store := NewFileTaskStoreWithDir("lazy-test", tmpDir) + err := store.Save([]Task{ + {ID: "task_1", Description: "Pre-existing task", Status: "pending"}, + }) + require.NoError(t, err) + + // Create tool - should not load yet + tool := NewTasksToolWithStore(NewFileTaskStoreWithDir("lazy-test", tmpDir)) + assert.False(t, tool.handler.loaded) + + // First operation triggers load + result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) + require.NoError(t, err) + assert.True(t, tool.handler.loaded) + assert.Contains(t, result.Output, "Pre-existing task") +} + +func TestTasksToolWithStore_PersistsDependencies(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + + // Create tasks with dependencies + store1 := NewFileTaskStoreWithDir("deps-test", tmpDir) + tool1 := NewTasksToolWithStore(store1) + + // Create first task + _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Setup database", + }) + require.NoError(t, err) + + // Create second task that depends on first + _, err = tool1.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Run migrations", + BlockedBy: []string{"task_1"}, + }) + require.NoError(t, err) + + // Load in new instance + store2 := NewFileTaskStoreWithDir("deps-test", tmpDir) + tool2 := NewTasksToolWithStore(store2) + + result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) + require.NoError(t, err) + assert.Contains(t, result.Output, "Setup database") + assert.Contains(t, result.Output, "Run migrations") + assert.Contains(t, result.Output, "blocked by") +} + +func TestTasksToolWithStore_PersistsStatusChanges(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + + // Create and complete a task + store1 := NewFileTaskStoreWithDir("status-test", tmpDir) + tool1 := NewTasksToolWithStore(store1) + + _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Task to complete", + }) + require.NoError(t, err) + + _, err = tool1.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_1", Status: "in-progress"}}, + }) + require.NoError(t, err) + + // Load in new instance - should see in-progress status + store2 := NewFileTaskStoreWithDir("status-test", tmpDir) + tool2 := NewTasksToolWithStore(store2) + + result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) + require.NoError(t, err) + assert.Contains(t, result.Output, "■") // in-progress icon + assert.Contains(t, result.Output, "1 in progress") +} + +func TestTasksToolWithStore_ClearsOnAllCompleted(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + + store := NewFileTaskStoreWithDir("clear-test", tmpDir) + tool := NewTasksToolWithStore(store) + + // Create and complete a task + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: "Single task", + }) + require.NoError(t, err) + + _, err = tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: "task_1", Status: "completed"}}, + }) + require.NoError(t, err) + + // Load in new instance - should be empty (cleared when all completed) + store2 := NewFileTaskStoreWithDir("clear-test", tmpDir) + tool2 := NewTasksToolWithStore(store2) + + result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) + require.NoError(t, err) + assert.Contains(t, result.Output, "No tasks") +} + +func TestFileTaskStore_AtomicWrite(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + store := NewFileTaskStoreWithDir("atomic-test", tmpDir) + + // Save initial tasks + err := store.Save([]Task{ + {ID: "task_1", Description: "Initial", Status: "pending"}, + }) + require.NoError(t, err) + + // Verify no .tmp file left behind + tmpFile := filepath.Join(tmpDir, "atomic-test.json.tmp") + _, err = os.Stat(tmpFile) + assert.True(t, os.IsNotExist(err), "temp file should not exist after save") + + // Verify main file exists + mainFile := filepath.Join(tmpDir, "atomic-test.json") + _, err = os.Stat(mainFile) + assert.NoError(t, err, "main file should exist") +} diff --git a/pkg/tui/components/tool/taskstool/sidebar.go b/pkg/tui/components/tool/taskstool/sidebar.go index 5a0b6e61a..86920ec9b 100644 --- a/pkg/tui/components/tool/taskstool/sidebar.go +++ b/pkg/tui/components/tool/taskstool/sidebar.go @@ -158,22 +158,26 @@ func (c *SidebarComponent) renderTab(title, content string) string { func (c *SidebarComponent) getBlockerDescriptions(blockerIDs []string) []string { result := make([]string, 0, len(blockerIDs)) for _, id := range blockerIDs { - found := false - for _, task := range c.tasks { - if task.ID == id { - // Truncate description to keep it short - desc := task.Description - if len(desc) > 20 { - desc = desc[:17] + "..." - } - result = append(result, desc) - found = true - break - } - } - if !found { - result = append(result, id) // Fallback to ID if not found + desc := c.findTaskDescription(id) + if desc == "" { + desc = id // Fallback to ID if not found } + result = append(result, desc) } return result } + +// findTaskDescription finds and returns a truncated description for a task ID +func (c *SidebarComponent) findTaskDescription(id string) string { + for _, task := range c.tasks { + if task.ID != id { + continue + } + desc := task.Description + if len(desc) > 20 { + desc = desc[:17] + "..." + } + return desc + } + return "" +} From 50f899967e85839df4480dfffa372f9ae2a78bc8 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 10:09:11 +0100 Subject: [PATCH 07/19] feat(tasks): always persist with auto-detected list ID from git repo - Remove MemoryTaskStore usage, always use FileTaskStore - Auto-detect task list ID from git common dir (shared across worktrees) - Format: - (e.g., 'cagent-a1b2c3d4') - Enables coordination across worktrees of the same repo - --task-list flag still overrides auto-detection if needed - shared: true now also persists (uses same store) Assisted-By: cagent --- pkg/teamloader/registry.go | 18 +++---- pkg/tools/builtin/tasks.go | 15 ++++++ pkg/tools/builtin/tasks_store.go | 72 +++++++++++++++++++++++++++ pkg/tools/builtin/tasks_store_test.go | 16 ++++++ 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/pkg/teamloader/registry.go b/pkg/teamloader/registry.go index 5ba00748a..3b3e4060d 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -82,21 +82,19 @@ func createTodoTool(_ context.Context, toolset latest.Toolset, _ string, _ *conf } func createTasksTool(_ context.Context, toolset latest.Toolset, _ string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { - // Determine task list ID: CLI/env takes precedence + // Determine task list ID: CLI/env takes precedence, otherwise auto-detect from git repo listID := runConfig.TaskListID - - if toolset.Shared { - // Shared mode uses in-memory storage (no persistence) - return builtin.NewSharedTasksTool(), nil + if listID == "" { + listID = builtin.DefaultTaskListID() } - if listID != "" { - // Use file-based persistence with the specified list ID - return builtin.NewTasksToolWithStore(builtin.NewFileTaskStore(listID)), nil + store := builtin.NewFileTaskStore(listID) + + if toolset.Shared { + return builtin.NewSharedTasksToolWithStore(store), nil } - // Default: in-memory only (no persistence) - return builtin.NewTasksTool(), nil + return builtin.NewTasksToolWithStore(store), nil } func createMemoryTool(_ context.Context, toolset latest.Toolset, parentDir string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index 9be95cb13..fc744582a 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -89,6 +89,21 @@ var NewSharedTasksTool = sync.OnceValue(func() *TasksTool { return NewTasksToolWithStore(NewMemoryTaskStore()) }) +// sharedTasksToolWithStore holds the shared instance when using a custom store +var ( + sharedTasksToolWithStore *TasksTool + sharedTasksToolWithStoreOnce sync.Once +) + +// NewSharedTasksToolWithStore creates or returns a shared TasksTool instance with the given store. +// The first call sets the store; subsequent calls return the same instance. +func NewSharedTasksToolWithStore(store TaskStore) *TasksTool { + sharedTasksToolWithStoreOnce.Do(func() { + sharedTasksToolWithStore = NewTasksToolWithStore(store) + }) + return sharedTasksToolWithStore +} + // NewTasksTool creates a new TasksTool with in-memory storage only func NewTasksTool() *TasksTool { return NewTasksToolWithStore(NewMemoryTaskStore()) diff --git a/pkg/tools/builtin/tasks_store.go b/pkg/tools/builtin/tasks_store.go index bb76c5890..908068629 100644 --- a/pkg/tools/builtin/tasks_store.go +++ b/pkg/tools/builtin/tasks_store.go @@ -1,10 +1,14 @@ package builtin import ( + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "os" + "os/exec" "path/filepath" + "strings" "sync" "github.com/docker/cagent/pkg/paths" @@ -131,3 +135,71 @@ func (s *MemoryTaskStore) Save(_ []Task) error { // No-op for memory store return nil } + +// DefaultTaskListID returns a default task list ID based on the git repository. +// It uses the git common dir (shared across worktrees) to generate a deterministic ID. +// Format: - (e.g., "cagent-a1b2c3d4") +// Falls back to working directory if not in a git repo. +func DefaultTaskListID() string { + // Try to get the git common directory (shared across worktrees) + repoPath := getGitCommonDir() + if repoPath == "" { + // Fallback to current working directory + var err error + repoPath, err = os.Getwd() + if err != nil { + return "default" + } + } + + // Get the directory name + dirName := filepath.Base(repoPath) + // Remove .git suffix if present (for bare repos or .git dirs) + dirName = strings.TrimSuffix(dirName, ".git") + if dirName == "" || dirName == "." { + dirName = "project" + } + + // Generate short hash of the full path for uniqueness + hash := sha256.Sum256([]byte(repoPath)) + shortHash := hex.EncodeToString(hash[:])[:8] + + return fmt.Sprintf("%s-%s", dirName, shortHash) +} + +// getGitCommonDir returns the path to the git common directory. +// This is the main .git directory shared across all worktrees. +// Returns empty string if not in a git repository. +func getGitCommonDir() string { + // First check if we're in a git repo at all + cmd := exec.Command("git", "rev-parse", "--git-common-dir") + output, err := cmd.Output() + if err != nil { + return "" + } + + gitCommonDir := strings.TrimSpace(string(output)) + if gitCommonDir == "" { + return "" + } + + // Convert to absolute path if relative + if !filepath.IsAbs(gitCommonDir) { + cwd, err := os.Getwd() + if err != nil { + return "" + } + gitCommonDir = filepath.Join(cwd, gitCommonDir) + } + + // Clean and return the parent of .git (the repo root) + gitCommonDir = filepath.Clean(gitCommonDir) + + // If it ends with .git, return the parent directory + if filepath.Base(gitCommonDir) == ".git" { + return filepath.Dir(gitCommonDir) + } + + // For bare repos or other cases, return as-is + return gitCommonDir +} diff --git a/pkg/tools/builtin/tasks_store_test.go b/pkg/tools/builtin/tasks_store_test.go index 12046ed95..2e21c0644 100644 --- a/pkg/tools/builtin/tasks_store_test.go +++ b/pkg/tools/builtin/tasks_store_test.go @@ -266,3 +266,19 @@ func TestFileTaskStore_AtomicWrite(t *testing.T) { _, err = os.Stat(mainFile) assert.NoError(t, err, "main file should exist") } + +func TestDefaultTaskListID(t *testing.T) { + // This test runs in the cagent repo, so it should detect the git repo + listID := DefaultTaskListID() + + // Should be non-empty + assert.NotEmpty(t, listID) + + // Should contain "cagent" (the repo name) and a hash + assert.Contains(t, listID, "cagent") + assert.Contains(t, listID, "-") // separator between name and hash + + // Should be deterministic (same result on multiple calls) + listID2 := DefaultTaskListID() + assert.Equal(t, listID, listID2) +} From 5e86780681ed47bd10f115f0c8de49514f136b02 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 10:11:38 +0100 Subject: [PATCH 08/19] test(tasks): add tests for worktree and cross-repo task list IDs - TestDefaultTaskListID_Worktrees: verify same ID across main repo and worktree - TestDefaultTaskListID_DifferentRepos: verify different IDs for different repos - TestDefaultTaskListID_NotGitRepo: verify fallback to directory name - Fix symlink resolution for macOS (/tmp -> /private/tmp) Assisted-By: cagent --- pkg/tools/builtin/tasks_store.go | 9 ++- pkg/tools/builtin/tasks_store_test.go | 97 +++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/pkg/tools/builtin/tasks_store.go b/pkg/tools/builtin/tasks_store.go index 908068629..c40f7f9bb 100644 --- a/pkg/tools/builtin/tasks_store.go +++ b/pkg/tools/builtin/tasks_store.go @@ -192,9 +192,16 @@ func getGitCommonDir() string { gitCommonDir = filepath.Join(cwd, gitCommonDir) } - // Clean and return the parent of .git (the repo root) + // Clean the path gitCommonDir = filepath.Clean(gitCommonDir) + // Resolve symlinks to get canonical path (important for macOS where /tmp -> /private/tmp) + gitCommonDir, err = filepath.EvalSymlinks(gitCommonDir) + if err != nil { + // If we can't resolve symlinks, use the cleaned path + gitCommonDir = filepath.Clean(gitCommonDir) + } + // If it ends with .git, return the parent directory if filepath.Base(gitCommonDir) == ".git" { return filepath.Dir(gitCommonDir) diff --git a/pkg/tools/builtin/tasks_store_test.go b/pkg/tools/builtin/tasks_store_test.go index 2e21c0644..a9a9b6e7e 100644 --- a/pkg/tools/builtin/tasks_store_test.go +++ b/pkg/tools/builtin/tasks_store_test.go @@ -2,6 +2,7 @@ package builtin import ( "os" + "os/exec" "path/filepath" "testing" @@ -282,3 +283,99 @@ func TestDefaultTaskListID(t *testing.T) { listID2 := DefaultTaskListID() assert.Equal(t, listID, listID2) } + +func TestDefaultTaskListID_Worktrees(t *testing.T) { + // Create a temp directory for our test repos + tmpDir := t.TempDir() + + // Create main repo + mainRepo := filepath.Join(tmpDir, "main-repo") + require.NoError(t, os.MkdirAll(mainRepo, 0o755)) + + // Initialize git repo + cmd := exec.Command("git", "init") + cmd.Dir = mainRepo + require.NoError(t, cmd.Run()) + + // Configure git user for commits + cmd = exec.Command("git", "config", "user.email", "test@test.com") + cmd.Dir = mainRepo + require.NoError(t, cmd.Run()) + cmd = exec.Command("git", "config", "user.name", "Test") + cmd.Dir = mainRepo + require.NoError(t, cmd.Run()) + + // Create initial commit (required for worktrees) + testFile := filepath.Join(mainRepo, "test.txt") + require.NoError(t, os.WriteFile(testFile, []byte("test"), 0o644)) + cmd = exec.Command("git", "add", ".") + cmd.Dir = mainRepo + require.NoError(t, cmd.Run()) + cmd = exec.Command("git", "commit", "-m", "initial") + cmd.Dir = mainRepo + require.NoError(t, cmd.Run()) + + // Create a worktree + worktree := filepath.Join(tmpDir, "worktree-feature") + cmd = exec.Command("git", "worktree", "add", "-b", "feature", worktree) + cmd.Dir = mainRepo + require.NoError(t, cmd.Run()) + + // Get task list ID from main repo + t.Chdir(mainRepo) + mainListID := DefaultTaskListID() + + // Get task list ID from worktree + t.Chdir(worktree) + worktreeListID := DefaultTaskListID() + + // Both should return the same ID (same underlying repo) + assert.Equal(t, mainListID, worktreeListID, "main repo and worktree should share the same task list ID") + + // ID should contain the repo name + assert.Contains(t, mainListID, "main-repo") +} + +func TestDefaultTaskListID_DifferentRepos(t *testing.T) { + tmpDir := t.TempDir() + + // Create two separate repos + repo1 := filepath.Join(tmpDir, "project-alpha") + repo2 := filepath.Join(tmpDir, "project-beta") + require.NoError(t, os.MkdirAll(repo1, 0o755)) + require.NoError(t, os.MkdirAll(repo2, 0o755)) + + // Initialize both repos + for _, repo := range []string{repo1, repo2} { + cmd := exec.Command("git", "init") + cmd.Dir = repo + require.NoError(t, cmd.Run()) + } + + // Get task list IDs + t.Chdir(repo1) + id1 := DefaultTaskListID() + + t.Chdir(repo2) + id2 := DefaultTaskListID() + + // Should be different + assert.NotEqual(t, id1, id2, "different repos should have different task list IDs") + assert.Contains(t, id1, "project-alpha") + assert.Contains(t, id2, "project-beta") +} + +func TestDefaultTaskListID_NotGitRepo(t *testing.T) { + // Create a temp directory that's not a git repo + tmpDir := t.TempDir() + notGitDir := filepath.Join(tmpDir, "not-a-repo") + require.NoError(t, os.MkdirAll(notGitDir, 0o755)) + + t.Chdir(notGitDir) + listID := DefaultTaskListID() + + // Should fallback to directory name + hash + assert.NotEmpty(t, listID) + assert.Contains(t, listID, "not-a-repo") + assert.Contains(t, listID, "-") // should still have hash +} From 2aa86d1ac39043432e41de68a85928f437881fe1 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 10:21:09 +0100 Subject: [PATCH 09/19] docs: add tasks tool documentation - Add tasks tool section to USAGE.md with features, tools, and examples - Update tool Instructions() to mention persistence and git-aware sharing - Add tasks to the toolsets configuration example Assisted-By: cagent --- docs/USAGE.md | 67 ++++++++++++++++++++++++++++++++++++-- pkg/tools/builtin/tasks.go | 6 +++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index 7275a219e..662a61af3 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -859,8 +859,10 @@ Included in `cagent` are a series of built-in tools that can greatly enhance the toolsets: - type: filesystem # Grants the agent filesystem access - type: think # Enables the think tool - - type: todo # Enable the todo list tool + - type: todo # Enable the simple todo list tool shared: boolean # Should the todo list be shared between agents (optional) + - type: tasks # Enable the tasks tool with dependencies and persistence + shared: boolean # Should tasks be shared between agents (optional) - type: memory # Allows the agent to store memories to a local sqlite db path: ./mem.db # Path to the sqlite database for memory storage (optional) ``` @@ -881,7 +883,7 @@ agents: ### Todo Tool -The todo tool helps agents manage task lists: +The todo tool helps agents manage simple task lists: ```yaml agents: @@ -891,6 +893,67 @@ agents: - type: todo ``` +### Tasks Tool + +The tasks tool provides advanced task management with dependencies, blocking relationships, and automatic persistence: + +```yaml +agents: + root: + # ... other config + toolsets: + - type: tasks +``` + +**Features:** +- **Dependencies**: Tasks can be blocked by other tasks (`blocked_by`) +- **Status tracking**: `pending`, `in-progress`, `completed` +- **Blocking enforcement**: Cannot start a blocked task until dependencies are completed +- **Cycle detection**: Prevents circular dependencies +- **Automatic persistence**: Tasks persist across sessions, stored in `~/.cagent/tasks/` +- **Git-aware**: Tasks are shared across all worktrees of the same git repository + +**Available tools:** +- `create_task` - Create a single task with optional dependencies +- `create_tasks` - Create multiple tasks at once +- `update_tasks` - Update task status or owner +- `list_tasks` - List all tasks with visual indicators (✓ done, ■ in-progress, □ pending, ⚠ blocked) +- `add_task_dependency` - Add dependencies to an existing task +- `remove_task_dependency` - Remove dependencies from a task +- `get_blocked_tasks` - List tasks that are currently blocked + +**Multi-agent sharing:** + +```yaml +agents: + coordinator: + toolsets: + - type: tasks + shared: true # Share task list across all agents + sub_agents: [backend, frontend] + backend: + toolsets: + - type: tasks + shared: true + frontend: + toolsets: + - type: tasks + shared: true +``` + +**Custom task list ID:** + +By default, tasks are stored based on the git repository. You can override this with a custom ID: + +```bash +# Via CLI flag +cagent run agent.yaml --task-list my-project + +# Via environment variable +export CAGENT_TASK_LIST_ID="my-project" +cagent run agent.yaml +``` + ### Memory Tool The memory tool provides persistent storage: diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index fc744582a..5e998cdd1 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -168,7 +168,11 @@ IMPORTANT: Use these tools to track tasks with dependencies: - Mark as "completed" when done 4. Visual indicators in list_tasks: - - ✓ = completed, ■ = in-progress, □ = pending, ⚠ = blocked` + - ✓ = completed, ■ = in-progress, □ = pending, ⚠ = blocked + +5. Persistence: + - Tasks are automatically saved and persist across sessions + - Tasks are shared across all worktrees of the same git repository` } func (h *tasksHandler) canStart(taskID string) (bool, []string) { From 0be70921120be7b88875017d270a544880e8c812 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 10:55:22 +0100 Subject: [PATCH 10/19] fix(tasks): add handler-level mutex to prevent race conditions Add sync.RWMutex to tasksHandler to protect compound operations in shared mode. This fixes several concurrency bugs: 1. Race in ensureLoaded() - multiple agents loading duplicate tasks 2. Race in task ID generation - concurrent creates generating duplicates 3. Race in Clear operation - newly created tasks incorrectly deleted Use sync.Once for ensureLoaded() to allow read operations (listTasks, getBlockedTasks) to use RLock() for better concurrency, while write operations use Lock(). Added concurrent tests with -race flag to verify the fix. Assisted-By: cagent --- pkg/tools/builtin/tasks.go | 59 +++++++++++++------- pkg/tools/builtin/tasks_store_test.go | 6 +- pkg/tools/builtin/tasks_test.go | 80 +++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 22 deletions(-) diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index 5e998cdd1..3a4c86583 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -79,9 +79,10 @@ type GetBlockedTasksArgs struct { } type tasksHandler struct { - tasks *concurrent.Slice[Task] - store TaskStore - loaded bool + mu sync.RWMutex + tasks *concurrent.Slice[Task] + store TaskStore + loadOnce sync.Once } // Shared instance for shared: true (no persistence) @@ -120,28 +121,27 @@ func NewTasksToolWithStore(store TaskStore) *TasksTool { } // ensureLoaded loads tasks from store on first access (lazy loading) +// Thread-safe via sync.Once func (h *tasksHandler) ensureLoaded() { - if h.loaded { - return - } - h.loaded = true - - tasks, err := h.store.Load() - if err != nil { - slog.Error("Failed to load tasks from store", "error", err) - return - } + h.loadOnce.Do(func() { + tasks, err := h.store.Load() + if err != nil { + slog.Error("Failed to load tasks from store", "error", err) + return + } - for _, task := range tasks { - h.tasks.Append(task) - } + for _, task := range tasks { + h.tasks.Append(task) + } - if len(tasks) > 0 { - slog.Debug("Loaded tasks from store", "count", len(tasks)) - } + if len(tasks) > 0 { + slog.Debug("Loaded tasks from store", "count", len(tasks)) + } + }) } // save persists tasks to store +// Must be called with h.mu held (write lock) func (h *tasksHandler) save() { if err := h.store.Save(h.tasks.All()); err != nil { slog.Error("Failed to save tasks to store", "error", err) @@ -249,6 +249,9 @@ func (h *tasksHandler) getUnblockedTasks(completedID string) []string { func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*tools.ToolCallResult, error) { h.ensureLoaded() + h.mu.Lock() + defer h.mu.Unlock() + for _, blockerID := range params.BlockedBy { if !h.taskExists(blockerID) { return tools.ResultError(fmt.Sprintf("invalid blocked_by reference: %s not found", blockerID)), nil @@ -286,6 +289,9 @@ func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*to func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (*tools.ToolCallResult, error) { h.ensureLoaded() + h.mu.Lock() + defer h.mu.Unlock() + start := h.tasks.Length() var createdIDs []string for i, item := range params.Tasks { @@ -335,6 +341,9 @@ func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (* func (h *tasksHandler) updateTasks(_ context.Context, params UpdateTasksArgs) (*tools.ToolCallResult, error) { h.ensureLoaded() + h.mu.Lock() + defer h.mu.Unlock() + var notFound, updated, blocked, newlyUnblocked []string for _, update := range params.Updates { task, idx := h.findTask(update.ID) @@ -415,6 +424,9 @@ func (h *tasksHandler) allCompleted() bool { func (h *tasksHandler) listTasks(_ context.Context, _ tools.ToolCall) (*tools.ToolCallResult, error) { h.ensureLoaded() + h.mu.RLock() + defer h.mu.RUnlock() + var output strings.Builder var completed, inProgress, pending, blockedCount int h.tasks.Range(func(_ int, task Task) bool { @@ -467,6 +479,9 @@ func (h *tasksHandler) listTasks(_ context.Context, _ tools.ToolCall) (*tools.To func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependencyArgs) (*tools.ToolCallResult, error) { h.ensureLoaded() + h.mu.Lock() + defer h.mu.Unlock() + task, idx := h.findTask(params.TaskID) if idx == -1 { return tools.ResultError(fmt.Sprintf("task not found: %s", params.TaskID)), nil @@ -528,6 +543,9 @@ func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependency func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDependencyArgs) (*tools.ToolCallResult, error) { h.ensureLoaded() + h.mu.Lock() + defer h.mu.Unlock() + task, idx := h.findTask(params.TaskID) if idx == -1 { return tools.ResultError(fmt.Sprintf("task not found: %s", params.TaskID)), nil @@ -581,6 +599,9 @@ func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDepe func (h *tasksHandler) getBlockedTasks(_ context.Context, params GetBlockedTasksArgs) (*tools.ToolCallResult, error) { h.ensureLoaded() + h.mu.RLock() + defer h.mu.RUnlock() + var output strings.Builder output.WriteString("Blocked tasks:\n") found := false diff --git a/pkg/tools/builtin/tasks_store_test.go b/pkg/tools/builtin/tasks_store_test.go index a9a9b6e7e..d02914bb1 100644 --- a/pkg/tools/builtin/tasks_store_test.go +++ b/pkg/tools/builtin/tasks_store_test.go @@ -144,14 +144,14 @@ func TestTasksToolWithStore_LazyLoading(t *testing.T) { }) require.NoError(t, err) - // Create tool - should not load yet + // Create tool - tasks slice should be empty before first operation tool := NewTasksToolWithStore(NewFileTaskStoreWithDir("lazy-test", tmpDir)) - assert.False(t, tool.handler.loaded) + assert.Equal(t, 0, tool.handler.tasks.Length()) // First operation triggers load result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) require.NoError(t, err) - assert.True(t, tool.handler.loaded) + assert.Equal(t, 1, tool.handler.tasks.Length()) assert.Contains(t, result.Output, "Pre-existing task") } diff --git a/pkg/tools/builtin/tasks_test.go b/pkg/tools/builtin/tasks_test.go index 9974381e8..84254ad83 100644 --- a/pkg/tools/builtin/tasks_test.go +++ b/pkg/tools/builtin/tasks_test.go @@ -1,6 +1,7 @@ package builtin import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -590,3 +591,82 @@ func TestTasksTool_Schema(t *testing.T) { assert.Equal(t, "tasks", tt.Category) } } + +// ============================================================================= +// Unit Tests: Concurrency +// ============================================================================= + +func TestTasksTool_ConcurrentCreates(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + const numGoroutines = 10 + done := make(chan bool, numGoroutines) + + for i := range numGoroutines { + go func(idx int) { + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: fmt.Sprintf("Task from goroutine %d", idx), + }) + assert.NoError(t, err) + done <- true + }(i) + } + + // Wait for all goroutines + for range numGoroutines { + <-done + } + + // Verify all tasks were created with unique IDs + tasks := tool.handler.tasks.All() + assert.Len(t, tasks, numGoroutines) + + ids := make(map[string]bool) + for _, task := range tasks { + assert.False(t, ids[task.ID], "duplicate task ID: %s", task.ID) + ids[task.ID] = true + } +} + +func TestTasksTool_ConcurrentUpdates(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + // Create initial tasks + for i := range 5 { + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ + Description: fmt.Sprintf("Task %d", i+1), + }) + require.NoError(t, err) + } + + const numGoroutines = 10 + done := make(chan bool, numGoroutines) + + // Concurrent updates and reads + for i := range numGoroutines { + go func(idx int) { + if idx%2 == 0 { + // Update a task + taskID := fmt.Sprintf("task_%d", (idx%5)+1) + _, _ = tool.handler.updateTasks(t.Context(), UpdateTasksArgs{ + Updates: []TaskUpdate{{ID: taskID, Status: "in-progress"}}, + }) + } else { + // List tasks + _, _ = tool.handler.listTasks(t.Context(), tools.ToolCall{}) + } + done <- true + }(i) + } + + // Wait for all goroutines + for range numGoroutines { + <-done + } + + // Verify tasks are still consistent + tasks := tool.handler.tasks.All() + assert.Len(t, tasks, 5) +} From 1b1c06e6559f7a0fa006d888bc0852b60387bc39 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 11:24:05 +0100 Subject: [PATCH 11/19] fix(tasks): add circular dependency check in batch creation and save warnings - Add validation for self-dependency and ordering in createTasks batch - Tasks can only depend on earlier tasks in the same batch - save() now returns warning message appended to output on failure - Document NewSharedTasksToolWithStore singleton behavior - Add tests for circular/self/mutual dependencies in batch Assisted-By: cagent --- pkg/tools/builtin/tasks.go | 68 +++++++++++++++++++++++---------- pkg/tools/builtin/tasks_test.go | 60 +++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 20 deletions(-) diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index 3a4c86583..ae92eb9c4 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -97,7 +97,8 @@ var ( ) // NewSharedTasksToolWithStore creates or returns a shared TasksTool instance with the given store. -// The first call sets the store; subsequent calls return the same instance. +// Note: Only the first call's store is used; subsequent calls return the same instance. +// This is safe because all agents in a shared context use the same listID. func NewSharedTasksToolWithStore(store TaskStore) *TasksTool { sharedTasksToolWithStoreOnce.Do(func() { sharedTasksToolWithStore = NewTasksToolWithStore(store) @@ -142,10 +143,13 @@ func (h *tasksHandler) ensureLoaded() { // save persists tasks to store // Must be called with h.mu held (write lock) -func (h *tasksHandler) save() { +// Returns an error message to append to output if save fails, empty string on success +func (h *tasksHandler) save() string { if err := h.store.Save(h.tasks.All()); err != nil { slog.Error("Failed to save tasks to store", "error", err) + return fmt.Sprintf(" (warning: failed to persist - %v)", err) } + return "" } func (t *TasksTool) Instructions() string { @@ -276,13 +280,14 @@ func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*to } } - h.save() + saveWarning := h.save() var output strings.Builder fmt.Fprintf(&output, "Created task [%s]: %s", id, params.Description) if len(params.BlockedBy) > 0 { fmt.Fprintf(&output, " (blocked by %s)", strings.Join(params.BlockedBy, ", ")) } + output.WriteString(saveWarning) return &tools.ToolCallResult{Output: output.String(), Meta: h.tasks.All()}, nil } @@ -293,22 +298,44 @@ func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (* defer h.mu.Unlock() start := h.tasks.Length() - var createdIDs []string + + // Build a map of task IDs that will be created in this batch + batchIDs := make(map[string]int) // ID -> index in batch + for i := range params.Tasks { + batchIDs[fmt.Sprintf("task_%d", start+i+1)] = i + } + + // Validate all tasks before creating any for i, item := range params.Tasks { + taskID := fmt.Sprintf("task_%d", start+i+1) for _, blockerID := range item.BlockedBy { - if !h.taskExists(blockerID) { - isEarlierInBatch := false - for j := range i { - if fmt.Sprintf("task_%d", start+j+1) == blockerID { - isEarlierInBatch = true - break - } + // Check for self-dependency + if blockerID == taskID { + return tools.ResultError(fmt.Sprintf("task cannot depend on itself: %s", taskID)), nil + } + + // Check if blocker is in this batch + if blockerIdx, inBatch := batchIDs[blockerID]; inBatch { + // Must be earlier in the batch (can't depend on later tasks) + if blockerIdx >= i { + return tools.ResultError(fmt.Sprintf("invalid blocked_by reference: %s must be created before %s", blockerID, taskID)), nil } - if !isEarlierInBatch { - return tools.ResultError(fmt.Sprintf("invalid blocked_by reference: %s not found", blockerID)), nil + // Check for mutual dependency (direct cycle in batch) + for _, blockerBlockedBy := range params.Tasks[blockerIdx].BlockedBy { + if blockerBlockedBy == taskID { + return tools.ResultError(fmt.Sprintf("circular dependency detected: %s and %s block each other", taskID, blockerID)), nil + } } + } else if !h.taskExists(blockerID) { + // Not in batch and doesn't exist in store + return tools.ResultError(fmt.Sprintf("invalid blocked_by reference: %s not found", blockerID)), nil } } + } + + // All validations passed, create the tasks + var createdIDs []string + for i, item := range params.Tasks { id := fmt.Sprintf("task_%d", start+i+1) task := Task{ ID: id, @@ -330,10 +357,10 @@ func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (* } } - h.save() + saveWarning := h.save() return &tools.ToolCallResult{ - Output: fmt.Sprintf("Created %d tasks: %s", len(params.Tasks), strings.Join(createdIDs, ", ")), + Output: fmt.Sprintf("Created %d tasks: %s%s", len(params.Tasks), strings.Join(createdIDs, ", "), saveWarning), Meta: h.tasks.All(), }, nil } @@ -401,7 +428,8 @@ func (h *tasksHandler) updateTasks(_ context.Context, params UpdateTasksArgs) (* h.tasks.Clear() } - h.save() + saveWarning := h.save() + output.WriteString(saveWarning) return &tools.ToolCallResult{Output: output.String(), Meta: h.tasks.All()}, nil } @@ -532,10 +560,10 @@ func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependency } } - h.save() + saveWarning := h.save() return &tools.ToolCallResult{ - Output: fmt.Sprintf("Added dependency: %s is now blocked by %s", params.TaskID, strings.Join(added, ", ")), + Output: fmt.Sprintf("Added dependency: %s is now blocked by %s%s", params.TaskID, strings.Join(added, ", "), saveWarning), Meta: h.tasks.All(), }, nil } @@ -588,10 +616,10 @@ func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDepe } } - h.save() + saveWarning := h.save() return &tools.ToolCallResult{ - Output: fmt.Sprintf("Removed dependency: %s is no longer blocked by %s", params.TaskID, strings.Join(removed, ", ")), + Output: fmt.Sprintf("Removed dependency: %s is no longer blocked by %s%s", params.TaskID, strings.Join(removed, ", "), saveWarning), Meta: h.tasks.All(), }, nil } diff --git a/pkg/tools/builtin/tasks_test.go b/pkg/tools/builtin/tasks_test.go index 84254ad83..cae1db5c7 100644 --- a/pkg/tools/builtin/tasks_test.go +++ b/pkg/tools/builtin/tasks_test.go @@ -109,6 +109,66 @@ func TestTasksTool_CreateTasks_Batch(t *testing.T) { assert.Equal(t, []string{"task_2"}, tasks[2].BlockedBy) } +func TestTasksTool_CreateTasks_CircularDependency(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + // Try to create tasks with circular dependency + result, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Task A", BlockedBy: []string{"task_2"}}, + {Description: "Task B", BlockedBy: []string{"task_1"}}, + }, + }) + + require.NoError(t, err) + assert.True(t, result.IsError) + // First task depends on second task which comes later in batch - invalid order + assert.Contains(t, result.Output, "task_2 must be created before task_1") + + // No tasks should have been created + tasks := tool.handler.tasks.All() + assert.Empty(t, tasks) +} + +func TestTasksTool_CreateTasks_MutualDependency(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + // Create a task first + _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Existing task"}) + require.NoError(t, err) + + // Try to create tasks where second depends on first, and first depends on second + // This is a real circular dependency since task_1 exists + result, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Task A", BlockedBy: []string{"task_1"}}, // task_2 blocked by existing task_1 + {Description: "Task B", BlockedBy: []string{"task_2"}}, // task_3 blocked by task_2 (in batch) + }, + }) + + require.NoError(t, err) + assert.False(t, result.IsError) // This should work - it's a valid chain + assert.Contains(t, result.Output, "Created 2 tasks") +} + +func TestTasksTool_CreateTasks_SelfDependency(t *testing.T) { + t.Parallel() + tool := NewTasksTool() + + // Try to create a task that depends on itself + result, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ + Tasks: []CreateTaskItem{ + {Description: "Self-referential", BlockedBy: []string{"task_1"}}, + }, + }) + + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "cannot depend on itself") +} + // ============================================================================= // Unit Tests: canStart Logic // ============================================================================= From b2f3c6b970977ad500552dfd4f37857d111556bf Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 14:11:41 +0100 Subject: [PATCH 12/19] fix(tasks): fail fast on load error to prevent data loss ensureLoaded() now captures and returns the load error. All handlers check this error and refuse to operate on empty state, preventing accidental overwrite of corrupted files. Added test: TestTasksToolWithStore_LoadError Assisted-By: cagent --- pkg/tools/builtin/tasks.go | 35 ++++++++++++++++++++------- pkg/tools/builtin/tasks_store_test.go | 28 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index ae92eb9c4..b963589c6 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -83,6 +83,7 @@ type tasksHandler struct { tasks *concurrent.Slice[Task] store TaskStore loadOnce sync.Once + loadErr error // Captured error from initial load } // Shared instance for shared: true (no persistence) @@ -122,11 +123,12 @@ func NewTasksToolWithStore(store TaskStore) *TasksTool { } // ensureLoaded loads tasks from store on first access (lazy loading) -// Thread-safe via sync.Once -func (h *tasksHandler) ensureLoaded() { +// Thread-safe via sync.Once. Returns error if load failed. +func (h *tasksHandler) ensureLoaded() error { h.loadOnce.Do(func() { tasks, err := h.store.Load() if err != nil { + h.loadErr = fmt.Errorf("failed to load tasks: %w", err) slog.Error("Failed to load tasks from store", "error", err) return } @@ -139,6 +141,7 @@ func (h *tasksHandler) ensureLoaded() { slog.Debug("Loaded tasks from store", "count", len(tasks)) } }) + return h.loadErr } // save persists tasks to store @@ -251,7 +254,9 @@ func (h *tasksHandler) getUnblockedTasks(completedID string) []string { } func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*tools.ToolCallResult, error) { - h.ensureLoaded() + if err := h.ensureLoaded(); err != nil { + return tools.ResultError(fmt.Sprintf("cannot create task: %v", err)), nil + } h.mu.Lock() defer h.mu.Unlock() @@ -292,7 +297,9 @@ func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*to } func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (*tools.ToolCallResult, error) { - h.ensureLoaded() + if err := h.ensureLoaded(); err != nil { + return tools.ResultError(fmt.Sprintf("cannot create tasks: %v", err)), nil + } h.mu.Lock() defer h.mu.Unlock() @@ -366,7 +373,9 @@ func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (* } func (h *tasksHandler) updateTasks(_ context.Context, params UpdateTasksArgs) (*tools.ToolCallResult, error) { - h.ensureLoaded() + if err := h.ensureLoaded(); err != nil { + return tools.ResultError(fmt.Sprintf("cannot update tasks: %v", err)), nil + } h.mu.Lock() defer h.mu.Unlock() @@ -450,7 +459,9 @@ func (h *tasksHandler) allCompleted() bool { } func (h *tasksHandler) listTasks(_ context.Context, _ tools.ToolCall) (*tools.ToolCallResult, error) { - h.ensureLoaded() + if err := h.ensureLoaded(); err != nil { + return tools.ResultError(fmt.Sprintf("cannot list tasks: %v", err)), nil + } h.mu.RLock() defer h.mu.RUnlock() @@ -505,7 +516,9 @@ func (h *tasksHandler) listTasks(_ context.Context, _ tools.ToolCall) (*tools.To } func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependencyArgs) (*tools.ToolCallResult, error) { - h.ensureLoaded() + if err := h.ensureLoaded(); err != nil { + return tools.ResultError(fmt.Sprintf("cannot add dependency: %v", err)), nil + } h.mu.Lock() defer h.mu.Unlock() @@ -569,7 +582,9 @@ func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependency } func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDependencyArgs) (*tools.ToolCallResult, error) { - h.ensureLoaded() + if err := h.ensureLoaded(); err != nil { + return tools.ResultError(fmt.Sprintf("cannot remove dependency: %v", err)), nil + } h.mu.Lock() defer h.mu.Unlock() @@ -625,7 +640,9 @@ func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDepe } func (h *tasksHandler) getBlockedTasks(_ context.Context, params GetBlockedTasksArgs) (*tools.ToolCallResult, error) { - h.ensureLoaded() + if err := h.ensureLoaded(); err != nil { + return tools.ResultError(fmt.Sprintf("cannot get blocked tasks: %v", err)), nil + } h.mu.RLock() defer h.mu.RUnlock() diff --git a/pkg/tools/builtin/tasks_store_test.go b/pkg/tools/builtin/tasks_store_test.go index d02914bb1..b3b2d3fb7 100644 --- a/pkg/tools/builtin/tasks_store_test.go +++ b/pkg/tools/builtin/tasks_store_test.go @@ -379,3 +379,31 @@ func TestDefaultTaskListID_NotGitRepo(t *testing.T) { assert.Contains(t, listID, "not-a-repo") assert.Contains(t, listID, "-") // should still have hash } + +func TestTasksToolWithStore_LoadError(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + taskFile := filepath.Join(tmpDir, "corrupted.json") + + // Write corrupted JSON + err := os.WriteFile(taskFile, []byte("not valid json{"), 0o644) + require.NoError(t, err) + + // Create store pointing to corrupted file + store := NewFileTaskStoreWithDir("corrupted", tmpDir) + tool := NewTasksToolWithStore(store) + + // All operations should fail with load error + result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "cannot list tasks") + assert.Contains(t, result.Output, "failed to load tasks") + + // Create should also fail - prevents overwriting corrupted file + result, err = tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "test"}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "cannot create task") +} From 2b66cc6b551312e67c1da74b0ce1ea3510482268 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 14:25:51 +0100 Subject: [PATCH 13/19] docs: add tasks tool guidance to golang_developer agent prompt Encourage the agent to use the tasks tool for complex multi-step work. Assisted-By: cagent --- golang_developer.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/golang_developer.yaml b/golang_developer.yaml index a9efd1ee6..d1388b43c 100755 --- a/golang_developer.yaml +++ b/golang_developer.yaml @@ -63,6 +63,8 @@ agents: The agent follows a deliberate approach to code changes. It begins by understanding what the user needs and searching for relevant code files and functions. Once it has a clear picture of the codebase structure, it makes necessary modifications while ensuring changes follow best practices and maintain consistency with existing code style. + For complex tasks with multiple steps, the agent uses the tasks tool to break down work, track dependencies, and monitor progress. + After making changes, the agent validates its work by running linters and tests. If issues arise, it returns to modification and continues this loop until all requirements are met and the code passes validation. From 33e5e798dec551a9668d65a8e464666bc290c098 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 14:46:51 +0100 Subject: [PATCH 14/19] refactor(tasks): simplify to always-shared singleton with file persistence Breaking changes: - Remove MemoryTaskStore (no longer needed) - Remove shared option from tasks toolset (always shared now) - Remove NewSharedTasksTool, NewSharedTasksToolWithStore, NewTasksToolWithStore (use NewTasksTool) Improvements: - NewTasksTool() returns singleton instance - SetTaskListID() allows CLI override before first use - NewFileTaskStore uses option pattern (WithBaseDir) instead of separate constructor - Factored addBlockerLink/removeBlockerLink helpers - Consistent use of findTask() method - noopTaskStore for unit tests avoids singleton interference Tasks are now: - Always persisted to ~/.cagent/tasks/.json - Always shared across all agents in the same process - Auto-detected from git repo (shared across worktrees) Assisted-By: cagent --- docs/USAGE.md | 7 +- pkg/teamloader/registry.go | 19 ++--- pkg/tools/builtin/tasks.go | 113 +++++++++++++------------- pkg/tools/builtin/tasks_store.go | 41 ++++------ pkg/tools/builtin/tasks_store_test.go | 70 ++++++---------- pkg/tools/builtin/tasks_test.go | 85 +++++++++++-------- 6 files changed, 155 insertions(+), 180 deletions(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index 662a61af3..0549d9fc3 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -861,8 +861,7 @@ toolsets: - type: think # Enables the think tool - type: todo # Enable the simple todo list tool shared: boolean # Should the todo list be shared between agents (optional) - - type: tasks # Enable the tasks tool with dependencies and persistence - shared: boolean # Should tasks be shared between agents (optional) + - type: tasks # Enable the tasks tool with dependencies and persistence (always shared) - type: memory # Allows the agent to store memories to a local sqlite db path: ./mem.db # Path to the sqlite database for memory storage (optional) ``` @@ -924,17 +923,17 @@ agents: **Multi-agent sharing:** +Tasks are automatically shared across all agents - no configuration needed: + ```yaml agents: coordinator: toolsets: - type: tasks - shared: true # Share task list across all agents sub_agents: [backend, frontend] backend: toolsets: - type: tasks - shared: true frontend: toolsets: - type: tasks diff --git a/pkg/teamloader/registry.go b/pkg/teamloader/registry.go index 3b3e4060d..d76626f25 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -81,20 +81,13 @@ func createTodoTool(_ context.Context, toolset latest.Toolset, _ string, _ *conf return builtin.NewTodoTool(), nil } -func createTasksTool(_ context.Context, toolset latest.Toolset, _ string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { - // Determine task list ID: CLI/env takes precedence, otherwise auto-detect from git repo - listID := runConfig.TaskListID - if listID == "" { - listID = builtin.DefaultTaskListID() +func createTasksTool(_ context.Context, _ latest.Toolset, _ string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { + // Override list ID if provided via CLI flag or env var + if runConfig.TaskListID != "" { + builtin.SetTaskListID(runConfig.TaskListID) } - - store := builtin.NewFileTaskStore(listID) - - if toolset.Shared { - return builtin.NewSharedTasksToolWithStore(store), nil - } - - return builtin.NewTasksToolWithStore(store), nil + // Tasks are always shared and persisted - singleton based on git repo + return builtin.NewTasksTool(), nil } func createMemoryTool(_ context.Context, toolset latest.Toolset, parentDir string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index b963589c6..b627d361b 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -86,34 +86,37 @@ type tasksHandler struct { loadErr error // Captured error from initial load } -// Shared instance for shared: true (no persistence) -var NewSharedTasksTool = sync.OnceValue(func() *TasksTool { - return NewTasksToolWithStore(NewMemoryTaskStore()) -}) - -// sharedTasksToolWithStore holds the shared instance when using a custom store +// tasksTool holds the singleton instance var ( - sharedTasksToolWithStore *TasksTool - sharedTasksToolWithStoreOnce sync.Once + tasksTool *TasksTool + tasksToolOnce sync.Once + tasksToolOpts struct { + listID string + } ) -// NewSharedTasksToolWithStore creates or returns a shared TasksTool instance with the given store. -// Note: Only the first call's store is used; subsequent calls return the same instance. -// This is safe because all agents in a shared context use the same listID. -func NewSharedTasksToolWithStore(store TaskStore) *TasksTool { - sharedTasksToolWithStoreOnce.Do(func() { - sharedTasksToolWithStore = NewTasksToolWithStore(store) - }) - return sharedTasksToolWithStore +// SetTaskListID allows overriding the default task list ID (call before NewTasksTool) +// This is used by the --task-list CLI flag. +func SetTaskListID(id string) { + tasksToolOpts.listID = id } -// NewTasksTool creates a new TasksTool with in-memory storage only +// NewTasksTool returns the shared TasksTool instance. +// All agents share the same task list, persisted to ~/.cagent/tasks/.json func NewTasksTool() *TasksTool { - return NewTasksToolWithStore(NewMemoryTaskStore()) + tasksToolOnce.Do(func() { + listID := tasksToolOpts.listID + if listID == "" { + listID = DefaultTaskListID() + } + store := NewFileTaskStore(listID) + tasksTool = newTasksToolWithStore(store) + }) + return tasksTool } -// NewTasksToolWithStore creates a new TasksTool with the specified store -func NewTasksToolWithStore(store TaskStore) *TasksTool { +// newTasksToolWithStore creates a new TasksTool with the specified store (for testing) +func newTasksToolWithStore(store TaskStore) *TasksTool { return &TasksTool{ handler: &tasksHandler{ tasks: concurrent.NewSlice[Task](), @@ -183,7 +186,7 @@ IMPORTANT: Use these tools to track tasks with dependencies: } func (h *tasksHandler) canStart(taskID string) (bool, []string) { - task, idx := h.tasks.Find(func(t Task) bool { return t.ID == taskID }) + task, idx := h.findTask(taskID) if idx == -1 { return false, []string{"task not found"} } @@ -192,7 +195,7 @@ func (h *tasksHandler) canStart(taskID string) (bool, []string) { } var pendingBlockers []string for _, blockerID := range task.BlockedBy { - blocker, blockerIdx := h.tasks.Find(func(t Task) bool { return t.ID == blockerID }) + blocker, blockerIdx := h.findTask(blockerID) if blockerIdx != -1 && blocker.Status != "completed" { pendingBlockers = append(pendingBlockers, blockerID) } @@ -213,6 +216,34 @@ func (h *tasksHandler) taskExists(id string) bool { return idx != -1 } +// addBlockerLink adds taskID to the Blocks list of blockerID +func (h *tasksHandler) addBlockerLink(blockerID, taskID string) { + _, idx := h.findTask(blockerID) + if idx != -1 { + h.tasks.Update(idx, func(t Task) Task { + t.Blocks = append(t.Blocks, taskID) + return t + }) + } +} + +// removeBlockerLink removes taskID from the Blocks list of blockerID +func (h *tasksHandler) removeBlockerLink(blockerID, taskID string) { + _, idx := h.findTask(blockerID) + if idx != -1 { + h.tasks.Update(idx, func(t Task) Task { + var newBlocks []string + for _, b := range t.Blocks { + if b != taskID { + newBlocks = append(newBlocks, b) + } + } + t.Blocks = newBlocks + return t + }) + } +} + func (h *tasksHandler) hasCircularDependency(taskID string, newBlockedBy []string) bool { blocked := make(map[string]bool) var collectBlocked func(id string) @@ -276,13 +307,7 @@ func (h *tasksHandler) createTask(_ context.Context, params CreateTaskArgs) (*to } h.tasks.Append(task) for _, blockerID := range params.BlockedBy { - _, idx := h.findTask(blockerID) - if idx != -1 { - h.tasks.Update(idx, func(t Task) Task { - t.Blocks = append(t.Blocks, id) - return t - }) - } + h.addBlockerLink(blockerID, id) } saveWarning := h.save() @@ -354,13 +379,7 @@ func (h *tasksHandler) createTasks(_ context.Context, params CreateTasksArgs) (* h.tasks.Append(task) createdIDs = append(createdIDs, id) for _, blockerID := range item.BlockedBy { - _, idx := h.findTask(blockerID) - if idx != -1 { - h.tasks.Update(idx, func(t Task) Task { - t.Blocks = append(t.Blocks, id) - return t - }) - } + h.addBlockerLink(blockerID, id) } } @@ -564,13 +583,7 @@ func (h *tasksHandler) addDependency(_ context.Context, params AddTaskDependency return t }) for _, blockerID := range added { - _, blockerIdx := h.findTask(blockerID) - if blockerIdx != -1 { - h.tasks.Update(blockerIdx, func(t Task) Task { - t.Blocks = append(t.Blocks, params.TaskID) - return t - }) - } + h.addBlockerLink(blockerID, params.TaskID) } saveWarning := h.save() @@ -616,19 +629,7 @@ func (h *tasksHandler) removeDependency(_ context.Context, params RemoveTaskDepe return t }) for _, blockerID := range removed { - _, blockerIdx := h.findTask(blockerID) - if blockerIdx != -1 { - h.tasks.Update(blockerIdx, func(t Task) Task { - var newBlocks []string - for _, b := range t.Blocks { - if b != params.TaskID { - newBlocks = append(newBlocks, b) - } - } - t.Blocks = newBlocks - return t - }) - } + h.removeBlockerLink(blockerID, params.TaskID) } saveWarning := h.save() diff --git a/pkg/tools/builtin/tasks_store.go b/pkg/tools/builtin/tasks_store.go index c40f7f9bb..82ceb71e3 100644 --- a/pkg/tools/builtin/tasks_store.go +++ b/pkg/tools/builtin/tasks_store.go @@ -37,20 +37,26 @@ type FileTaskStore struct { mu sync.RWMutex } -// NewFileTaskStore creates a new file-based task store -func NewFileTaskStore(listID string) *FileTaskStore { - return &FileTaskStore{ - listID: listID, - baseDir: paths.GetTasksDir(), +// FileTaskStoreOption configures a FileTaskStore +type FileTaskStoreOption func(*FileTaskStore) + +// WithBaseDir sets a custom base directory (for testing) +func WithBaseDir(dir string) FileTaskStoreOption { + return func(s *FileTaskStore) { + s.baseDir = dir } } -// NewFileTaskStoreWithDir creates a file-based task store with a custom directory (for testing) -func NewFileTaskStoreWithDir(listID, baseDir string) *FileTaskStore { - return &FileTaskStore{ +// NewFileTaskStore creates a new file-based task store +func NewFileTaskStore(listID string, opts ...FileTaskStoreOption) *FileTaskStore { + s := &FileTaskStore{ listID: listID, - baseDir: baseDir, + baseDir: paths.GetTasksDir(), } + for _, opt := range opts { + opt(s) + } + return s } func (s *FileTaskStore) filePath() string { @@ -119,23 +125,6 @@ func (s *FileTaskStore) Save(tasks []Task) error { return nil } -// MemoryTaskStore implements TaskStore with in-memory storage only (no persistence) -// Used when no listID is provided -type MemoryTaskStore struct{} - -func NewMemoryTaskStore() *MemoryTaskStore { - return &MemoryTaskStore{} -} - -func (s *MemoryTaskStore) Load() ([]Task, error) { - return []Task{}, nil -} - -func (s *MemoryTaskStore) Save(_ []Task) error { - // No-op for memory store - return nil -} - // DefaultTaskListID returns a default task list ID based on the git repository. // It uses the git common dir (shared across worktrees) to generate a deterministic ID. // Format: - (e.g., "cagent-a1b2c3d4") diff --git a/pkg/tools/builtin/tasks_store_test.go b/pkg/tools/builtin/tasks_store_test.go index b3b2d3fb7..985ab6bf5 100644 --- a/pkg/tools/builtin/tasks_store_test.go +++ b/pkg/tools/builtin/tasks_store_test.go @@ -16,7 +16,7 @@ func TestFileTaskStore_SaveAndLoad(t *testing.T) { t.Parallel() tmpDir := t.TempDir() - store := NewFileTaskStoreWithDir("test-project", tmpDir) + store := NewFileTaskStore("test-project", WithBaseDir(tmpDir)) // Initially empty tasks, err := store.Load() @@ -44,7 +44,7 @@ func TestFileTaskStore_FileCreatedOnSave(t *testing.T) { t.Parallel() tmpDir := t.TempDir() - store := NewFileTaskStoreWithDir("my-project", tmpDir) + store := NewFileTaskStore("my-project", WithBaseDir(tmpDir)) expectedPath := filepath.Join(tmpDir, "my-project.json") // File should not exist yet @@ -64,7 +64,7 @@ func TestFileTaskStore_LoadNonExistentFile(t *testing.T) { t.Parallel() tmpDir := t.TempDir() - store := NewFileTaskStoreWithDir("nonexistent", tmpDir) + store := NewFileTaskStore("nonexistent", WithBaseDir(tmpDir)) // Should return empty list, not error tasks, err := store.Load() @@ -77,7 +77,7 @@ func TestFileTaskStore_SanitizesListID(t *testing.T) { tmpDir := t.TempDir() // Try to use path traversal - should be sanitized - store := NewFileTaskStoreWithDir("../../../etc/passwd", tmpDir) + store := NewFileTaskStore("../../../etc/passwd", WithBaseDir(tmpDir)) err := store.Save([]Task{{ID: "task_1", Description: "Test", Status: "pending"}}) require.NoError(t, err) @@ -88,34 +88,14 @@ func TestFileTaskStore_SanitizesListID(t *testing.T) { assert.NoError(t, err) } -func TestMemoryTaskStore_NoOp(t *testing.T) { - t.Parallel() - - store := NewMemoryTaskStore() - - // Load always returns empty - tasks, err := store.Load() - require.NoError(t, err) - assert.Empty(t, tasks) - - // Save is a no-op - err = store.Save([]Task{{ID: "task_1", Description: "Test", Status: "pending"}}) - require.NoError(t, err) - - // Still empty after save - tasks, err = store.Load() - require.NoError(t, err) - assert.Empty(t, tasks) -} - func TestTasksToolWithStore_Persistence(t *testing.T) { t.Parallel() tmpDir := t.TempDir() // Create first tool instance and add a task - store1 := NewFileTaskStoreWithDir("persistent-test", tmpDir) - tool1 := NewTasksToolWithStore(store1) + store1 := NewFileTaskStore("persistent-test", WithBaseDir(tmpDir)) + tool1 := newTasksToolWithStore(store1) _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ Description: "Persistent task", @@ -123,8 +103,8 @@ func TestTasksToolWithStore_Persistence(t *testing.T) { require.NoError(t, err) // Create second tool instance with same store ID - store2 := NewFileTaskStoreWithDir("persistent-test", tmpDir) - tool2 := NewTasksToolWithStore(store2) + store2 := NewFileTaskStore("persistent-test", WithBaseDir(tmpDir)) + tool2 := newTasksToolWithStore(store2) // Should load the task from the first instance result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) @@ -138,14 +118,14 @@ func TestTasksToolWithStore_LazyLoading(t *testing.T) { tmpDir := t.TempDir() // Pre-populate a task file - store := NewFileTaskStoreWithDir("lazy-test", tmpDir) + store := NewFileTaskStore("lazy-test", WithBaseDir(tmpDir)) err := store.Save([]Task{ {ID: "task_1", Description: "Pre-existing task", Status: "pending"}, }) require.NoError(t, err) // Create tool - tasks slice should be empty before first operation - tool := NewTasksToolWithStore(NewFileTaskStoreWithDir("lazy-test", tmpDir)) + tool := newTasksToolWithStore(NewFileTaskStore("lazy-test", WithBaseDir(tmpDir))) assert.Equal(t, 0, tool.handler.tasks.Length()) // First operation triggers load @@ -161,8 +141,8 @@ func TestTasksToolWithStore_PersistsDependencies(t *testing.T) { tmpDir := t.TempDir() // Create tasks with dependencies - store1 := NewFileTaskStoreWithDir("deps-test", tmpDir) - tool1 := NewTasksToolWithStore(store1) + store1 := NewFileTaskStore("deps-test", WithBaseDir(tmpDir)) + tool1 := newTasksToolWithStore(store1) // Create first task _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ @@ -178,8 +158,8 @@ func TestTasksToolWithStore_PersistsDependencies(t *testing.T) { require.NoError(t, err) // Load in new instance - store2 := NewFileTaskStoreWithDir("deps-test", tmpDir) - tool2 := NewTasksToolWithStore(store2) + store2 := NewFileTaskStore("deps-test", WithBaseDir(tmpDir)) + tool2 := newTasksToolWithStore(store2) result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) require.NoError(t, err) @@ -194,8 +174,8 @@ func TestTasksToolWithStore_PersistsStatusChanges(t *testing.T) { tmpDir := t.TempDir() // Create and complete a task - store1 := NewFileTaskStoreWithDir("status-test", tmpDir) - tool1 := NewTasksToolWithStore(store1) + store1 := NewFileTaskStore("status-test", WithBaseDir(tmpDir)) + tool1 := newTasksToolWithStore(store1) _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ Description: "Task to complete", @@ -208,8 +188,8 @@ func TestTasksToolWithStore_PersistsStatusChanges(t *testing.T) { require.NoError(t, err) // Load in new instance - should see in-progress status - store2 := NewFileTaskStoreWithDir("status-test", tmpDir) - tool2 := NewTasksToolWithStore(store2) + store2 := NewFileTaskStore("status-test", WithBaseDir(tmpDir)) + tool2 := newTasksToolWithStore(store2) result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) require.NoError(t, err) @@ -222,8 +202,8 @@ func TestTasksToolWithStore_ClearsOnAllCompleted(t *testing.T) { tmpDir := t.TempDir() - store := NewFileTaskStoreWithDir("clear-test", tmpDir) - tool := NewTasksToolWithStore(store) + store := NewFileTaskStore("clear-test", WithBaseDir(tmpDir)) + tool := newTasksToolWithStore(store) // Create and complete a task _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ @@ -237,8 +217,8 @@ func TestTasksToolWithStore_ClearsOnAllCompleted(t *testing.T) { require.NoError(t, err) // Load in new instance - should be empty (cleared when all completed) - store2 := NewFileTaskStoreWithDir("clear-test", tmpDir) - tool2 := NewTasksToolWithStore(store2) + store2 := NewFileTaskStore("clear-test", WithBaseDir(tmpDir)) + tool2 := newTasksToolWithStore(store2) result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) require.NoError(t, err) @@ -249,7 +229,7 @@ func TestFileTaskStore_AtomicWrite(t *testing.T) { t.Parallel() tmpDir := t.TempDir() - store := NewFileTaskStoreWithDir("atomic-test", tmpDir) + store := NewFileTaskStore("atomic-test", WithBaseDir(tmpDir)) // Save initial tasks err := store.Save([]Task{ @@ -391,8 +371,8 @@ func TestTasksToolWithStore_LoadError(t *testing.T) { require.NoError(t, err) // Create store pointing to corrupted file - store := NewFileTaskStoreWithDir("corrupted", tmpDir) - tool := NewTasksToolWithStore(store) + store := NewFileTaskStore("corrupted", WithBaseDir(tmpDir)) + tool := newTasksToolWithStore(store) // All operations should fail with load error result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) diff --git a/pkg/tools/builtin/tasks_test.go b/pkg/tools/builtin/tasks_test.go index cae1db5c7..be943b29c 100644 --- a/pkg/tools/builtin/tasks_test.go +++ b/pkg/tools/builtin/tasks_test.go @@ -10,13 +10,24 @@ import ( "github.com/docker/cagent/pkg/tools" ) +// newTestTasksTool creates a new TasksTool for testing (not the singleton) +func newTestTasksTool() *TasksTool { + return newTasksToolWithStore(&noopTaskStore{}) +} + +// noopTaskStore is a TaskStore that doesn't persist anything (for unit tests) +type noopTaskStore struct{} + +func (s *noopTaskStore) Load() ([]Task, error) { return []Task{}, nil } +func (s *noopTaskStore) Save(_ []Task) error { return nil } + // ============================================================================= // Unit Tests: Task Creation with Dependencies // ============================================================================= func TestTasksTool_CreateTask_Basic(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() result, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ Description: "Setup database", @@ -33,7 +44,7 @@ func TestTasksTool_CreateTask_Basic(t *testing.T) { func TestTasksTool_CreateTask_WithBlockedBy(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() // Create prerequisite tasks first _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Task 1"}) @@ -58,7 +69,7 @@ func TestTasksTool_CreateTask_WithBlockedBy(t *testing.T) { func TestTasksTool_CreateTask_WithInvalidBlockedBy(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() result, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ Description: "Some task", @@ -72,7 +83,7 @@ func TestTasksTool_CreateTask_WithInvalidBlockedBy(t *testing.T) { func TestTasksTool_CreateTask_WithOwner(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() result, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ Description: "Backend task", @@ -89,7 +100,7 @@ func TestTasksTool_CreateTask_WithOwner(t *testing.T) { func TestTasksTool_CreateTasks_Batch(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() result, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -111,7 +122,7 @@ func TestTasksTool_CreateTasks_Batch(t *testing.T) { func TestTasksTool_CreateTasks_CircularDependency(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() // Try to create tasks with circular dependency result, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ @@ -133,7 +144,7 @@ func TestTasksTool_CreateTasks_CircularDependency(t *testing.T) { func TestTasksTool_CreateTasks_MutualDependency(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() // Create a task first _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Existing task"}) @@ -155,7 +166,7 @@ func TestTasksTool_CreateTasks_MutualDependency(t *testing.T) { func TestTasksTool_CreateTasks_SelfDependency(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() // Try to create a task that depends on itself result, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ @@ -175,7 +186,7 @@ func TestTasksTool_CreateTasks_SelfDependency(t *testing.T) { func TestTasksTool_CanStart_NoDependencies(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Independent task"}) require.NoError(t, err) @@ -187,7 +198,7 @@ func TestTasksTool_CanStart_NoDependencies(t *testing.T) { func TestTasksTool_CanStart_WithPendingBlockers(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Blocker"}) require.NoError(t, err) @@ -204,7 +215,7 @@ func TestTasksTool_CanStart_WithPendingBlockers(t *testing.T) { func TestTasksTool_CanStart_WithCompletedBlockers(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Blocker"}) require.NoError(t, err) @@ -227,7 +238,7 @@ func TestTasksTool_CanStart_WithCompletedBlockers(t *testing.T) { func TestTasksTool_CanStart_MultipleBlockers_PartiallyCompleted(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -255,7 +266,7 @@ func TestTasksTool_CanStart_MultipleBlockers_PartiallyCompleted(t *testing.T) { func TestTasksTool_UpdateTasks_CannotStartBlocked(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Blocker"}) require.NoError(t, err) @@ -276,7 +287,7 @@ func TestTasksTool_UpdateTasks_CannotStartBlocked(t *testing.T) { func TestTasksTool_UpdateTasks_CanStartAfterBlockerCompleted(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Blocker"}) require.NoError(t, err) @@ -304,7 +315,7 @@ func TestTasksTool_UpdateTasks_CanStartAfterBlockerCompleted(t *testing.T) { func TestTasksTool_UpdateTasks_CompletionUnblocks(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -335,7 +346,7 @@ func TestTasksTool_UpdateTasks_CompletionUnblocks(t *testing.T) { func TestTasksTool_ListTasks_Empty(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) @@ -345,7 +356,7 @@ func TestTasksTool_ListTasks_Empty(t *testing.T) { func TestTasksTool_ListTasks_WithDependencies(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -366,7 +377,7 @@ func TestTasksTool_ListTasks_WithDependencies(t *testing.T) { func TestTasksTool_ListTasks_StatusIcons(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -395,7 +406,7 @@ func TestTasksTool_ListTasks_StatusIcons(t *testing.T) { func TestTasksTool_ListTasks_ShowsOwner(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ Description: "Backend work", @@ -411,7 +422,7 @@ func TestTasksTool_ListTasks_ShowsOwner(t *testing.T) { func TestTasksTool_ListTasks_Stats(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -445,7 +456,7 @@ func TestTasksTool_ListTasks_Stats(t *testing.T) { func TestTasksTool_AddDependency(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -470,7 +481,7 @@ func TestTasksTool_AddDependency(t *testing.T) { func TestTasksTool_AddDependency_PreventCircular(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "First"}) require.NoError(t, err) @@ -493,7 +504,7 @@ func TestTasksTool_AddDependency_PreventCircular(t *testing.T) { func TestTasksTool_AddDependency_PreventSelfDependency(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "Task"}) require.NoError(t, err) @@ -510,7 +521,7 @@ func TestTasksTool_AddDependency_PreventSelfDependency(t *testing.T) { func TestTasksTool_RemoveDependency(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{Description: "First"}) require.NoError(t, err) @@ -539,7 +550,7 @@ func TestTasksTool_RemoveDependency(t *testing.T) { func TestTasksTool_GetBlockedTasks(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -560,7 +571,7 @@ func TestTasksTool_GetBlockedTasks(t *testing.T) { func TestTasksTool_GetBlockedTasks_FilterByBlocker(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() _, err := tool.handler.createTasks(t.Context(), CreateTasksArgs{ Tasks: []CreateTaskItem{ @@ -588,18 +599,20 @@ func TestTasksTool_GetBlockedTasks_FilterByBlocker(t *testing.T) { func TestTasksTool_SharedInstance(t *testing.T) { t.Parallel() - shared1 := NewSharedTasksTool() - shared2 := NewSharedTasksTool() - assert.Same(t, shared1, shared2, "NewSharedTasksTool should return same instance") + // NewTasksTool is now a singleton + shared1 := NewTasksTool() + shared2 := NewTasksTool() + assert.Same(t, shared1, shared2, "NewTasksTool should return same instance") - nonShared1 := NewTasksTool() - nonShared2 := NewTasksTool() - assert.NotSame(t, nonShared1, nonShared2, "NewTasksTool should return different instances") + // newTestTasksTool creates separate instances for testing + nonShared1 := newTestTasksTool() + nonShared2 := newTestTasksTool() + assert.NotSame(t, nonShared1, nonShared2, "newTestTasksTool should return different instances") } func TestTasksTool_CrossAgentSharing(t *testing.T) { // Simulates two agents sharing a task list - tool := NewTasksTool() + tool := newTestTasksTool() // Agent A creates a task _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ @@ -640,7 +653,7 @@ func TestTasksTool_CrossAgentSharing(t *testing.T) { func TestTasksTool_Schema(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() allTools, err := tool.Tools(t.Context()) require.NoError(t, err) @@ -658,7 +671,7 @@ func TestTasksTool_Schema(t *testing.T) { func TestTasksTool_ConcurrentCreates(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() const numGoroutines = 10 done := make(chan bool, numGoroutines) @@ -691,7 +704,7 @@ func TestTasksTool_ConcurrentCreates(t *testing.T) { func TestTasksTool_ConcurrentUpdates(t *testing.T) { t.Parallel() - tool := NewTasksTool() + tool := newTestTasksTool() // Create initial tasks for i := range 5 { From 3455cd296597df40232e89777595652d021d68eb Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 14:48:16 +0100 Subject: [PATCH 15/19] docs: update tasks tool documentation for always-shared behavior - Update Instructions() to mention all agents share the same task list - Update USAGE.md to remove shared option examples - Add 'Always shared' to features list - Update PR description Assisted-By: cagent --- docs/USAGE.md | 2 +- pkg/tools/builtin/tasks.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index 0549d9fc3..22d3652f8 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -911,6 +911,7 @@ agents: - **Cycle detection**: Prevents circular dependencies - **Automatic persistence**: Tasks persist across sessions, stored in `~/.cagent/tasks/` - **Git-aware**: Tasks are shared across all worktrees of the same git repository +- **Always shared**: All agents automatically share the same task list **Available tools:** - `create_task` - Create a single task with optional dependencies @@ -937,7 +938,6 @@ agents: frontend: toolsets: - type: tasks - shared: true ``` **Custom task list ID:** diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index b627d361b..68933e32d 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -182,7 +182,8 @@ IMPORTANT: Use these tools to track tasks with dependencies: 5. Persistence: - Tasks are automatically saved and persist across sessions - - Tasks are shared across all worktrees of the same git repository` + - Tasks are shared across all worktrees of the same git repository + - All agents share the same task list automatically` } func (h *tasksHandler) canStart(taskID string) (bool, []string) { From e4a2b57b2c36c68fda69f9ef9b995a70e6d3fb84 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 14:51:18 +0100 Subject: [PATCH 16/19] refactor(tasks): replace singleton with dependency injection via registry - Remove global singleton pattern from tasks.go - NewTasksTool(store) now takes store as parameter (pure function) - ToolsetRegistry holds shared instance via GetOrCreateTasksTool() - Sharing happens at registry level, not via global state - Better testability: each test can create isolated instances Assisted-By: cagent --- pkg/teamloader/registry.go | 26 ++++++++++++++------- pkg/tools/builtin/tasks.go | 33 ++------------------------- pkg/tools/builtin/tasks_store_test.go | 20 ++++++++-------- pkg/tools/builtin/tasks_test.go | 20 ++++++++-------- 4 files changed, 40 insertions(+), 59 deletions(-) diff --git a/pkg/teamloader/registry.go b/pkg/teamloader/registry.go index d76626f25..bff148c6a 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -25,7 +25,8 @@ type ToolsetCreator func(ctx context.Context, toolset latest.Toolset, parentDir // ToolsetRegistry manages the registration of toolset creators by type type ToolsetRegistry struct { - creators map[string]ToolsetCreator + creators map[string]ToolsetCreator + sharedTasksTool *builtin.TasksTool // Shared instance for all agents } // NewToolsetRegistry creates a new empty toolset registry @@ -48,6 +49,11 @@ func (r *ToolsetRegistry) Get(toolsetType string) (ToolsetCreator, bool) { // CreateTool creates a toolset using the registered creator for the given type func (r *ToolsetRegistry) CreateTool(ctx context.Context, toolset latest.Toolset, parentDir string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { + // Special case for tasks - always returns shared instance + if toolset.Type == "tasks" { + return r.GetOrCreateTasksTool(runConfig), nil + } + creator, ok := r.Get(toolset.Type) if !ok { return nil, fmt.Errorf("unknown toolset type: %s", toolset.Type) @@ -59,7 +65,7 @@ func NewDefaultToolsetRegistry() *ToolsetRegistry { r := NewToolsetRegistry() // Register all built-in toolset creators r.Register("todo", createTodoTool) - r.Register("tasks", createTasksTool) + // Note: "tasks" is handled specially in CreateTool - no registration needed r.Register("memory", createMemoryTool) r.Register("think", createThinkTool) r.Register("shell", createShellTool) @@ -81,13 +87,17 @@ func createTodoTool(_ context.Context, toolset latest.Toolset, _ string, _ *conf return builtin.NewTodoTool(), nil } -func createTasksTool(_ context.Context, _ latest.Toolset, _ string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { - // Override list ID if provided via CLI flag or env var - if runConfig.TaskListID != "" { - builtin.SetTaskListID(runConfig.TaskListID) +// GetOrCreateTasksTool returns the shared TasksTool instance, creating it if needed +func (r *ToolsetRegistry) GetOrCreateTasksTool(runConfig *config.RuntimeConfig) *builtin.TasksTool { + if r.sharedTasksTool == nil { + listID := runConfig.TaskListID + if listID == "" { + listID = builtin.DefaultTaskListID() + } + store := builtin.NewFileTaskStore(listID) + r.sharedTasksTool = builtin.NewTasksTool(store) } - // Tasks are always shared and persisted - singleton based on git repo - return builtin.NewTasksTool(), nil + return r.sharedTasksTool } func createMemoryTool(_ context.Context, toolset latest.Toolset, parentDir string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index 68933e32d..5aee912d6 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -86,37 +86,8 @@ type tasksHandler struct { loadErr error // Captured error from initial load } -// tasksTool holds the singleton instance -var ( - tasksTool *TasksTool - tasksToolOnce sync.Once - tasksToolOpts struct { - listID string - } -) - -// SetTaskListID allows overriding the default task list ID (call before NewTasksTool) -// This is used by the --task-list CLI flag. -func SetTaskListID(id string) { - tasksToolOpts.listID = id -} - -// NewTasksTool returns the shared TasksTool instance. -// All agents share the same task list, persisted to ~/.cagent/tasks/.json -func NewTasksTool() *TasksTool { - tasksToolOnce.Do(func() { - listID := tasksToolOpts.listID - if listID == "" { - listID = DefaultTaskListID() - } - store := NewFileTaskStore(listID) - tasksTool = newTasksToolWithStore(store) - }) - return tasksTool -} - -// newTasksToolWithStore creates a new TasksTool with the specified store (for testing) -func newTasksToolWithStore(store TaskStore) *TasksTool { +// NewTasksTool creates a new TasksTool with the specified store +func NewTasksTool(store TaskStore) *TasksTool { return &TasksTool{ handler: &tasksHandler{ tasks: concurrent.NewSlice[Task](), diff --git a/pkg/tools/builtin/tasks_store_test.go b/pkg/tools/builtin/tasks_store_test.go index 985ab6bf5..9e59a5fbe 100644 --- a/pkg/tools/builtin/tasks_store_test.go +++ b/pkg/tools/builtin/tasks_store_test.go @@ -95,7 +95,7 @@ func TestTasksToolWithStore_Persistence(t *testing.T) { // Create first tool instance and add a task store1 := NewFileTaskStore("persistent-test", WithBaseDir(tmpDir)) - tool1 := newTasksToolWithStore(store1) + tool1 := NewTasksTool(store1) _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ Description: "Persistent task", @@ -104,7 +104,7 @@ func TestTasksToolWithStore_Persistence(t *testing.T) { // Create second tool instance with same store ID store2 := NewFileTaskStore("persistent-test", WithBaseDir(tmpDir)) - tool2 := newTasksToolWithStore(store2) + tool2 := NewTasksTool(store2) // Should load the task from the first instance result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) @@ -125,7 +125,7 @@ func TestTasksToolWithStore_LazyLoading(t *testing.T) { require.NoError(t, err) // Create tool - tasks slice should be empty before first operation - tool := newTasksToolWithStore(NewFileTaskStore("lazy-test", WithBaseDir(tmpDir))) + tool := NewTasksTool(NewFileTaskStore("lazy-test", WithBaseDir(tmpDir))) assert.Equal(t, 0, tool.handler.tasks.Length()) // First operation triggers load @@ -142,7 +142,7 @@ func TestTasksToolWithStore_PersistsDependencies(t *testing.T) { // Create tasks with dependencies store1 := NewFileTaskStore("deps-test", WithBaseDir(tmpDir)) - tool1 := newTasksToolWithStore(store1) + tool1 := NewTasksTool(store1) // Create first task _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ @@ -159,7 +159,7 @@ func TestTasksToolWithStore_PersistsDependencies(t *testing.T) { // Load in new instance store2 := NewFileTaskStore("deps-test", WithBaseDir(tmpDir)) - tool2 := newTasksToolWithStore(store2) + tool2 := NewTasksTool(store2) result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) require.NoError(t, err) @@ -175,7 +175,7 @@ func TestTasksToolWithStore_PersistsStatusChanges(t *testing.T) { // Create and complete a task store1 := NewFileTaskStore("status-test", WithBaseDir(tmpDir)) - tool1 := newTasksToolWithStore(store1) + tool1 := NewTasksTool(store1) _, err := tool1.handler.createTask(t.Context(), CreateTaskArgs{ Description: "Task to complete", @@ -189,7 +189,7 @@ func TestTasksToolWithStore_PersistsStatusChanges(t *testing.T) { // Load in new instance - should see in-progress status store2 := NewFileTaskStore("status-test", WithBaseDir(tmpDir)) - tool2 := newTasksToolWithStore(store2) + tool2 := NewTasksTool(store2) result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) require.NoError(t, err) @@ -203,7 +203,7 @@ func TestTasksToolWithStore_ClearsOnAllCompleted(t *testing.T) { tmpDir := t.TempDir() store := NewFileTaskStore("clear-test", WithBaseDir(tmpDir)) - tool := newTasksToolWithStore(store) + tool := NewTasksTool(store) // Create and complete a task _, err := tool.handler.createTask(t.Context(), CreateTaskArgs{ @@ -218,7 +218,7 @@ func TestTasksToolWithStore_ClearsOnAllCompleted(t *testing.T) { // Load in new instance - should be empty (cleared when all completed) store2 := NewFileTaskStore("clear-test", WithBaseDir(tmpDir)) - tool2 := newTasksToolWithStore(store2) + tool2 := NewTasksTool(store2) result, err := tool2.handler.listTasks(t.Context(), tools.ToolCall{}) require.NoError(t, err) @@ -372,7 +372,7 @@ func TestTasksToolWithStore_LoadError(t *testing.T) { // Create store pointing to corrupted file store := NewFileTaskStore("corrupted", WithBaseDir(tmpDir)) - tool := newTasksToolWithStore(store) + tool := NewTasksTool(store) // All operations should fail with load error result, err := tool.handler.listTasks(t.Context(), tools.ToolCall{}) diff --git a/pkg/tools/builtin/tasks_test.go b/pkg/tools/builtin/tasks_test.go index be943b29c..42621e146 100644 --- a/pkg/tools/builtin/tasks_test.go +++ b/pkg/tools/builtin/tasks_test.go @@ -10,9 +10,9 @@ import ( "github.com/docker/cagent/pkg/tools" ) -// newTestTasksTool creates a new TasksTool for testing (not the singleton) +// newTestTasksTool creates a new TasksTool for testing (not shared) func newTestTasksTool() *TasksTool { - return newTasksToolWithStore(&noopTaskStore{}) + return NewTasksTool(&noopTaskStore{}) } // noopTaskStore is a TaskStore that doesn't persist anything (for unit tests) @@ -599,15 +599,15 @@ func TestTasksTool_GetBlockedTasks_FilterByBlocker(t *testing.T) { func TestTasksTool_SharedInstance(t *testing.T) { t.Parallel() - // NewTasksTool is now a singleton - shared1 := NewTasksTool() - shared2 := NewTasksTool() - assert.Same(t, shared1, shared2, "NewTasksTool should return same instance") + // NewTasksTool with same store returns different instances + // Sharing is handled at the registry level, not in the constructor + store := &noopTaskStore{} + tool1 := NewTasksTool(store) + tool2 := NewTasksTool(store) + assert.NotSame(t, tool1, tool2, "NewTasksTool should return different instances") - // newTestTasksTool creates separate instances for testing - nonShared1 := newTestTasksTool() - nonShared2 := newTestTasksTool() - assert.NotSame(t, nonShared1, nonShared2, "newTestTasksTool should return different instances") + // But they share the same store, so changes are visible + // (in production, registry ensures single instance) } func TestTasksTool_CrossAgentSharing(t *testing.T) { From 7a96bbba9cf23ce244b96c60063f7fb0dd78ca86 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 15:17:10 +0100 Subject: [PATCH 17/19] fix(tasks): address PR review comments 1. Race condition in atomic write: use unique temp filename with PID and nanosecond timestamp to prevent corruption when multiple processes write simultaneously 2. Empty/dot listID validation: filePath() now handles empty, '.', and '..' values by falling back to 'default' 3. Git symlink resolution: check for .git basename BEFORE resolving symlinks to handle cases where .git is symlinked to a different path Added TestFileTaskStore_EmptyListID test. Assisted-By: cagent --- pkg/tools/builtin/tasks_store.go | 26 ++++++++++++++-------- pkg/tools/builtin/tasks_store_test.go | 32 +++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/pkg/tools/builtin/tasks_store.go b/pkg/tools/builtin/tasks_store.go index 82ceb71e3..0dd8c8ec6 100644 --- a/pkg/tools/builtin/tasks_store.go +++ b/pkg/tools/builtin/tasks_store.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" "sync" + "time" "github.com/docker/cagent/pkg/paths" ) @@ -62,6 +63,10 @@ func NewFileTaskStore(listID string, opts ...FileTaskStoreOption) *FileTaskStore func (s *FileTaskStore) filePath() string { // Sanitize listID to be safe as filename safeID := filepath.Base(s.listID) + // Handle empty, ".", or ".." values + if safeID == "" || safeID == "." || safeID == ".." { + safeID = "default" + } return filepath.Join(s.baseDir, safeID+".json") } @@ -112,7 +117,8 @@ func (s *FileTaskStore) Save(tasks []Task) error { } // Write atomically using temp file + rename - tmpPath := path + ".tmp" + // Use unique temp filename to avoid race conditions between processes + tmpPath := fmt.Sprintf("%s.%d.%d.tmp", path, os.Getpid(), time.Now().UnixNano()) if err := os.WriteFile(tmpPath, data, 0o600); err != nil { return fmt.Errorf("writing task file: %w", err) } @@ -160,7 +166,6 @@ func DefaultTaskListID() string { // This is the main .git directory shared across all worktrees. // Returns empty string if not in a git repository. func getGitCommonDir() string { - // First check if we're in a git repo at all cmd := exec.Command("git", "rev-parse", "--git-common-dir") output, err := cmd.Output() if err != nil { @@ -181,18 +186,21 @@ func getGitCommonDir() string { gitCommonDir = filepath.Join(cwd, gitCommonDir) } - // Clean the path + // Clean the path first gitCommonDir = filepath.Clean(gitCommonDir) + // Check for .git BEFORE resolving symlinks to handle cases where + // .git is a symlink to a path that doesn't end with .git + isGitDir := filepath.Base(gitCommonDir) == ".git" + // Resolve symlinks to get canonical path (important for macOS where /tmp -> /private/tmp) - gitCommonDir, err = filepath.EvalSymlinks(gitCommonDir) - if err != nil { - // If we can't resolve symlinks, use the cleaned path - gitCommonDir = filepath.Clean(gitCommonDir) + resolved, err := filepath.EvalSymlinks(gitCommonDir) + if err == nil { + gitCommonDir = resolved } - // If it ends with .git, return the parent directory - if filepath.Base(gitCommonDir) == ".git" { + // If it was a .git directory, return the parent + if isGitDir { return filepath.Dir(gitCommonDir) } diff --git a/pkg/tools/builtin/tasks_store_test.go b/pkg/tools/builtin/tasks_store_test.go index 9e59a5fbe..3477f7b34 100644 --- a/pkg/tools/builtin/tasks_store_test.go +++ b/pkg/tools/builtin/tasks_store_test.go @@ -88,6 +88,38 @@ func TestFileTaskStore_SanitizesListID(t *testing.T) { assert.NoError(t, err) } +func TestFileTaskStore_EmptyListID(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + + tests := []struct { + name string + listID string + }{ + {"empty string", ""}, + {"dot", "."}, + {"double dot", ".."}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := NewFileTaskStore(tt.listID, WithBaseDir(tmpDir)) + + err := store.Save([]Task{{ID: "task_1", Description: "Test", Status: "pending"}}) + require.NoError(t, err) + + // Should use "default" as filename + expectedPath := filepath.Join(tmpDir, "default.json") + _, err = os.Stat(expectedPath) + require.NoError(t, err) + + // Cleanup for next test + os.Remove(expectedPath) + }) + } +} + func TestTasksToolWithStore_Persistence(t *testing.T) { t.Parallel() From e66f9811dd293b48fc3a67e329da2c9b6fb935a7 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 26 Jan 2026 15:23:53 +0100 Subject: [PATCH 18/19] fix(fake): prevent panic in StreamCopy when client disconnects - Add recover() in goroutine to handle panics when ResponseWriter becomes invalid after client disconnect - Check context before starting new goroutines - Add type assertion with fallback for writers without ReaderFrom Assisted-By: cagent --- pkg/fake/proxy.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/fake/proxy.go b/pkg/fake/proxy.go index 5b03a09b9..18ad91772 100644 --- a/pkg/fake/proxy.go +++ b/pkg/fake/proxy.go @@ -395,7 +395,12 @@ type streamReadResult struct { // It properly handles context cancellation during blocking reads. func StreamCopy(c echo.Context, resp *http.Response) error { ctx := c.Request().Context() - writer := c.Response().Writer.(io.ReaderFrom) + writer, ok := c.Response().Writer.(io.ReaderFrom) + if !ok { + // Fallback to io.Copy if writer doesn't implement ReaderFrom + _, err := io.Copy(c.Response().Writer, resp.Body) + return err + } // Use a channel to receive read results from a goroutine. // This allows us to properly select on context cancellation @@ -403,8 +408,20 @@ func StreamCopy(c echo.Context, resp *http.Response) error { resultCh := make(chan streamReadResult, 1) for { + // Check context before starting new goroutine + if ctx.Err() != nil { + return nil + } + // Start a goroutine to perform the blocking read go func() { + defer func() { + // Recover from panic if writer becomes invalid + if r := recover(); r != nil { + slog.Warn("StreamCopy recovered from panic", "panic", r) + resultCh <- streamReadResult{n: 0, err: io.EOF} + } + }() n, err := writer.ReadFrom(io.LimitReader(resp.Body, 256)) resultCh <- streamReadResult{n: n, err: err} }() From 0c9bef0783bfb378c7af0c83a31485eaea1213fb Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Tue, 27 Jan 2026 10:52:26 +0100 Subject: [PATCH 19/19] docs(tasks): clarify task ID format in tool descriptions Make it explicit that blocked_by expects task IDs in the format task_1, task_2, etc. This helps LLMs use the correct format on first try. Assisted-By: cagent --- pkg/tools/builtin/tasks.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/tools/builtin/tasks.go b/pkg/tools/builtin/tasks.go index 5aee912d6..6145aacfd 100644 --- a/pkg/tools/builtin/tasks.go +++ b/pkg/tools/builtin/tasks.go @@ -33,20 +33,20 @@ type Task struct { ID string `json:"id" jsonschema:"ID of the task"` Description string `json:"description" jsonschema:"Description of the task"` Status string `json:"status" jsonschema:"Status: pending, in-progress, or completed"` - BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"IDs of tasks that must be completed before this one can start"` - Blocks []string `json:"blocks,omitempty" jsonschema:"IDs of tasks that are waiting for this one to complete"` + BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"Task IDs (e.g. task_1, task_2) that must be completed before this one can start"` + Blocks []string `json:"blocks,omitempty" jsonschema:"Task IDs that are waiting for this one to complete"` Owner string `json:"owner,omitempty" jsonschema:"Owner/assignee of this task"` } type CreateTaskArgs struct { Description string `json:"description" jsonschema:"Description of the task,required"` - BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"IDs of tasks that must be completed before this one can start"` + BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"Task IDs (e.g. task_1, task_2) that must be completed first"` Owner string `json:"owner,omitempty" jsonschema:"Owner/assignee of this task"` } type CreateTaskItem struct { Description string `json:"description" jsonschema:"Description of the task,required"` - BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"IDs of tasks that must be completed before this one can start"` + BlockedBy []string `json:"blocked_by,omitempty" jsonschema:"Task IDs (e.g. task_1, task_2) that must be completed first. For batch creation, use task_N where N is the 1-based position in the final list."` Owner string `json:"owner,omitempty" jsonschema:"Owner/assignee of this task"` } @@ -661,7 +661,7 @@ func (t *TasksTool) Tools(context.Context) ([]tools.Tool, error) { { Name: ToolNameCreateTask, Category: "tasks", - Description: "Create a new task. Use blocked_by to specify dependencies on other tasks.", + Description: "Create a new task. Use blocked_by with task IDs (e.g. [\"task_1\", \"task_2\"]) to specify dependencies.", Parameters: tools.MustSchemaFor[CreateTaskArgs](), Handler: tools.NewHandler(t.handler.createTask), Annotations: tools.ToolAnnotations{Title: "Create Task", ReadOnlyHint: true}, @@ -669,7 +669,7 @@ func (t *TasksTool) Tools(context.Context) ([]tools.Tool, error) { { Name: ToolNameCreateTasks, Category: "tasks", - Description: "Create multiple tasks at once with dependencies.", + Description: "Create multiple tasks at once with dependencies. Task IDs are assigned as task_1, task_2, etc. Use these IDs in blocked_by.", Parameters: tools.MustSchemaFor[CreateTasksArgs](), Handler: tools.NewHandler(t.handler.createTasks), Annotations: tools.ToolAnnotations{Title: "Create Tasks", ReadOnlyHint: true},