Extract TaskStorage interface for tasks storage

Follow the same pattern used for todos in PR docker#1960:
- Define a TaskStorage interface with domain-level methods
  (All, Get, Put, Delete) that map to what the toolset does
- Extract file I/O into FileTaskStorage implementation
- Add TaskOption functional options with WithTaskStorage
- Move mutex from TasksTool into FileTaskStorage
- Refactor all handlers to use the interface directly
- Update NewTasksTool to accept variadic options
- Add tests for WithTaskStorage and nil panic

No in-memory storage implementation is needed for tasks.

Assisted-By: cagent

Author: rumpl

pkg/tools/builtin/tasks.go

@@ -87,9 +87,101 @@ type taskStore struct {
 	Tasks map[string]Task `json:"tasks"`
 }
 
-type TasksTool struct {
+// TaskStorage defines the storage layer for task items.
+type TaskStorage interface {
+	// All returns every task keyed by ID.
+	All() map[string]Task
+	// Get returns a single task by ID and whether it was found.
+	Get(id string) (Task, bool)
+	// Put creates or updates a task.
+	Put(task Task) error
+	// Delete removes a task by ID.
+	Delete(id string) error
+}
+
+// FileTaskStorage is a file-backed, concurrency-safe implementation of TaskStorage.
+type FileTaskStorage struct {
 	mu       sync.Mutex
 	filePath string
+}
+
+// NewFileTaskStorage creates a FileTaskStorage that reads/writes to the given path.
+func NewFileTaskStorage(filePath string) *FileTaskStorage {
+	return &FileTaskStorage{filePath: filePath}
+}
+
+func (s *FileTaskStorage) load() map[string]Task {
+	data, err := os.ReadFile(s.filePath)
+	if err != nil {
+		return make(map[string]Task)
+	}
+	var store taskStore
+	if err := json.Unmarshal(data, &store); err != nil {
+		return make(map[string]Task)
+	}
+	if store.Tasks == nil {
+		return make(map[string]Task)
+	}
+	return store.Tasks
+}
+
+func (s *FileTaskStorage) save(tasks map[string]Task) error {
+	if err := os.MkdirAll(filepath.Dir(s.filePath), 0o700); err != nil {
+		return fmt.Errorf("creating storage directory: %w", err)
+	}
+	data, err := json.MarshalIndent(taskStore{Tasks: tasks}, "", "  ")
+	if err != nil {
+		return fmt.Errorf("marshaling task store: %w", err)
+	}
+	return os.WriteFile(s.filePath, data, 0o644)
+}
+
+func (s *FileTaskStorage) All() map[string]Task {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.load()
+}
+
+func (s *FileTaskStorage) Get(id string) (Task, bool) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	tasks := s.load()
+	t, ok := tasks[id]
+	return t, ok
+}
+
+func (s *FileTaskStorage) Put(task Task) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	tasks := s.load()
+	tasks[task.ID] = task
+	return s.save(tasks)
+}
+
+func (s *FileTaskStorage) Delete(id string) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	tasks := s.load()
+	delete(tasks, id)
+	return s.save(tasks)
+}
+
+// TaskOption is a functional option for configuring a TasksTool.
+type TaskOption func(*TasksTool)
+
+// WithTaskStorage sets a custom storage implementation for the TasksTool.
+// The provided storage must not be nil.
+func WithTaskStorage(storage TaskStorage) TaskOption {
+	if storage == nil {
+		panic("tasks: storage must not be nil")
+	}
+	return func(t *TasksTool) {
+		t.storage = storage
+	}
+}
+
+type TasksTool struct {
+	storage  TaskStorage
 	basePath string
 }
 
@@ -98,11 +190,15 @@ var (
 	_ tools.Instructable = (*TasksTool)(nil)
 )
 
-func NewTasksTool(storagePath string) *TasksTool {
-	return &TasksTool{
-		filePath: storagePath,
+func NewTasksTool(storagePath string, opts ...TaskOption) *TasksTool {
+	t := &TasksTool{
+		storage:  NewFileTaskStorage(storagePath),
 		basePath: filepath.Dir(storagePath),
 	}
+	for _, opt := range opts {
+		opt(t)
+	}
+	return t
 }
 
 func (t *TasksTool) Instructions() string {
@@ -115,32 +211,6 @@ Tasks are saved to a JSON file and survive across sessions. A task is automatica
 Workflow: create_task → list_tasks/next_task → update_task as work progresses. Use add_dependency/remove_dependency to manage ordering.`
 }
 
-func (t *TasksTool) load() taskStore {
-	data, err := os.ReadFile(t.filePath)
-	if err != nil {
-		return taskStore{Tasks: make(map[string]Task)}
-	}
-	var store taskStore
-	if err := json.Unmarshal(data, &store); err != nil {
-		return taskStore{Tasks: make(map[string]Task)}
-	}
-	if store.Tasks == nil {
-		store.Tasks = make(map[string]Task)
-	}
-	return store
-}
-
-func (t *TasksTool) save(store taskStore) error {
-	if err := os.MkdirAll(filepath.Dir(t.filePath), 0o700); err != nil {
-		return fmt.Errorf("creating storage directory: %w", err)
-	}
-	data, err := json.MarshalIndent(store, "", "  ")
-	if err != nil {
-		return fmt.Errorf("marshaling task store: %w", err)
-	}
-	return os.WriteFile(t.filePath, data, 0o644)
-}
-
 func effectiveStatus(task Task, tasks map[string]Task) TaskStatus {
 	if task.Status == StatusDone {
 		return StatusDone
@@ -266,22 +336,19 @@ func (t *TasksTool) createTask(_ context.Context, params CreateTaskArgs) (*tools
 		return tools.ResultError(fmt.Sprintf("invalid priority: %s", params.Priority)), nil
 	}
 
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	store := t.load()
+	all := t.storage.All()
 	id := uuid.New().String()
 
 	deps := params.Dependencies
 	if deps == nil {
 		deps = []string{}
 	}
 	for _, depID := range deps {
-		if _, ok := store.Tasks[depID]; !ok {
+		if _, ok := all[depID]; !ok {
 			return tools.ResultError(fmt.Sprintf("dependency task not found: %s", depID)), nil
 		}
 	}
-	if hasCycle(store.Tasks, id, deps) {
+	if hasCycle(all, id, deps) {
 		return tools.ResultError("adding these dependencies would create a cycle"), nil
 	}
 
@@ -296,33 +363,24 @@ func (t *TasksTool) createTask(_ context.Context, params CreateTaskArgs) (*tools
 		UpdatedAt:    now(),
 	}
 
-	store.Tasks[id] = task
-	if err := t.save(store); err != nil {
+	if err := t.storage.Put(task); err != nil {
 		return tools.ResultError(err.Error()), nil
 	}
 
 	return taskResult(task), nil
 }
 
 func (t *TasksTool) getTask(_ context.Context, params GetTaskArgs) (*tools.ToolCallResult, error) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	store := t.load()
-	task, ok := store.Tasks[params.ID]
+	task, ok := t.storage.Get(params.ID)
 	if !ok {
 		return tools.ResultError(fmt.Sprintf("task not found: %s", params.ID)), nil
 	}
 
-	return taskWithEffectiveResult(task, store.Tasks), nil
+	return taskWithEffectiveResult(task, t.storage.All()), nil
 }
 
 func (t *TasksTool) updateTask(_ context.Context, params UpdateTaskArgs) (*tools.ToolCallResult, error) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	store := t.load()
-	task, ok := store.Tasks[params.ID]
+	task, ok := t.storage.Get(params.ID)
 	if !ok {
 		return tools.ResultError(fmt.Sprintf("task not found: %s", params.ID)), nil
 	}
@@ -350,50 +408,53 @@ func (t *TasksTool) updateTask(_ context.Context, params UpdateTaskArgs) (*tools
 		task.Status = TaskStatus(params.Status)
 	}
 	if params.Dependencies != nil {
+		all := t.storage.All()
 		for _, depID := range params.Dependencies {
-			if _, exists := store.Tasks[depID]; !exists {
+			if _, exists := all[depID]; !exists {
 				return tools.ResultError(fmt.Sprintf("dependency task not found: %s", depID)), nil
 			}
 		}
-		if hasCycle(store.Tasks, params.ID, params.Dependencies) {
+		if hasCycle(all, params.ID, params.Dependencies) {
 			return tools.ResultError("adding these dependencies would create a cycle"), nil
 		}
 		task.Dependencies = params.Dependencies
 	}
 
 	task.UpdatedAt = now()
-	store.Tasks[params.ID] = task
 
-	if err := t.save(store); err != nil {
+	if err := t.storage.Put(task); err != nil {
 		return tools.ResultError(err.Error()), nil
 	}
 
 	return taskResult(task), nil
 }
 
 func (t *TasksTool) deleteTask(_ context.Context, params DeleteTaskArgs) (*tools.ToolCallResult, error) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	store := t.load()
-	if _, ok := store.Tasks[params.ID]; !ok {
+	if _, ok := t.storage.Get(params.ID); !ok {
 		return tools.ResultError(fmt.Sprintf("task not found: %s", params.ID)), nil
 	}
 
-	for id, task := range store.Tasks {
+	// Remove the task from other tasks' dependency lists.
+	all := t.storage.All()
+	for id, task := range all {
+		if id == params.ID {
+			continue
+		}
 		filtered := make([]string, 0, len(task.Dependencies))
 		for _, d := range task.Dependencies {
 			if d != params.ID {
 				filtered = append(filtered, d)
 			}
 		}
-		task.Dependencies = filtered
-		store.Tasks[id] = task
+		if len(filtered) != len(task.Dependencies) {
+			task.Dependencies = filtered
+			if err := t.storage.Put(task); err != nil {
+				return tools.ResultError(err.Error()), nil
+			}
+		}
 	}
 
-	delete(store.Tasks, params.ID)
-
-	if err := t.save(store); err != nil {
+	if err := t.storage.Delete(params.ID); err != nil {
 		return tools.ResultError(err.Error()), nil
 	}
 
@@ -405,15 +466,12 @@ func (t *TasksTool) deleteTask(_ context.Context, params DeleteTaskArgs) (*tools
 }
 
 func (t *TasksTool) listTasks(_ context.Context, params ListTasksArgs) (*tools.ToolCallResult, error) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	store := t.load()
+	all := t.storage.All()
 	var tasks []taskWithEffective
-	for _, task := range store.Tasks {
+	for _, task := range all {
 		tasks = append(tasks, taskWithEffective{
 			Task:            task,
-			EffectiveStatus: effectiveStatus(task, store.Tasks),
+			EffectiveStatus: effectiveStatus(task, all),
 		})
 	}
 
@@ -446,15 +504,12 @@ func (t *TasksTool) listTasks(_ context.Context, params ListTasksArgs) (*tools.T
 }
 
 func (t *TasksTool) nextTask(_ context.Context, _ tools.ToolCall) (*tools.ToolCallResult, error) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	store := t.load()
+	all := t.storage.All()
 	var tasks []taskWithEffective
-	for _, task := range store.Tasks {
+	for _, task := range all {
 		tasks = append(tasks, taskWithEffective{
 			Task:            task,
-			EffectiveStatus: effectiveStatus(task, store.Tasks),
+			EffectiveStatus: effectiveStatus(task, all),
 		})
 	}
 	sortTasks(tasks)
@@ -475,43 +530,35 @@ func (t *TasksTool) nextTask(_ context.Context, _ tools.ToolCall) (*tools.ToolCa
 }
 
 func (t *TasksTool) addDependency(_ context.Context, params AddDependencyArgs) (*tools.ToolCallResult, error) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	store := t.load()
-	task, ok := store.Tasks[params.TaskID]
+	task, ok := t.storage.Get(params.TaskID)
 	if !ok {
 		return tools.ResultError(fmt.Sprintf("task not found: %s", params.TaskID)), nil
 	}
-	if _, ok := store.Tasks[params.DependsOnID]; !ok {
+	if _, ok := t.storage.Get(params.DependsOnID); !ok {
 		return tools.ResultError(fmt.Sprintf("dependency task not found: %s", params.DependsOnID)), nil
 	}
 	if slices.Contains(task.Dependencies, params.DependsOnID) {
 		return tools.ResultError("dependency already exists"), nil
 	}
 
 	newDeps := append(task.Dependencies, params.DependsOnID)
-	if hasCycle(store.Tasks, params.TaskID, newDeps) {
+	all := t.storage.All()
+	if hasCycle(all, params.TaskID, newDeps) {
 		return tools.ResultError("adding this dependency would create a cycle"), nil
 	}
 
 	task.Dependencies = newDeps
 	task.UpdatedAt = now()
-	store.Tasks[params.TaskID] = task
 
-	if err := t.save(store); err != nil {
+	if err := t.storage.Put(task); err != nil {
 		return tools.ResultError(err.Error()), nil
 	}
 
 	return taskResult(task), nil
 }
 
 func (t *TasksTool) removeDependency(_ context.Context, params RemoveDependencyArgs) (*tools.ToolCallResult, error) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	store := t.load()
-	task, ok := store.Tasks[params.TaskID]
+	task, ok := t.storage.Get(params.TaskID)
 	if !ok {
 		return tools.ResultError(fmt.Sprintf("task not found: %s", params.TaskID)), nil
 	}
@@ -524,9 +571,8 @@ func (t *TasksTool) removeDependency(_ context.Context, params RemoveDependencyA
 	}
 	task.Dependencies = filtered
 	task.UpdatedAt = now()
-	store.Tasks[params.TaskID] = task
 
-	if err := t.save(store); err != nil {
+	if err := t.storage.Put(task); err != nil {
 		return tools.ResultError(err.Error()), nil
 	}