todo: extract storage interface with in-memory implementation

Introduce TodoStorage interface to decouple the todo toolset from its
storage layer. Add MemoryTodoStorage as the default in-memory
implementation and a WithStorage functional option so callers can
provide their own.

Assisted-By: cagent

Author: rumpl

pkg/tools/builtin/todo.go

@@ -50,18 +50,84 @@ type UpdateTodosArgs struct {
 	Updates []TodoUpdate `json:"updates" jsonschema:"List of todo updates"`
 }
 
-type todoHandler struct {
+// TodoStorage defines the storage layer for todo items.
+type TodoStorage interface {
+	// Add appends a new todo item.
+	Add(todo Todo)
+	// All returns a copy of all todo items.
+	All() []Todo
+	// Len returns the number of todo items.
+	Len() int
+	// FindByID returns the index of the todo with the given ID, or -1 if not found.
+	FindByID(id string) int
+	// Update modifies the todo at the given index using the provided function.
+	Update(index int, fn func(Todo) Todo)
+	// Clear removes all todo items.
+	Clear()
+}
+
+// MemoryTodoStorage is an in-memory, concurrency-safe implementation of TodoStorage.
+type MemoryTodoStorage struct {
 	todos *concurrent.Slice[Todo]
 }
 
-var NewSharedTodoTool = sync.OnceValue(NewTodoTool)
+func NewMemoryTodoStorage() *MemoryTodoStorage {
+	return &MemoryTodoStorage{
+		todos: concurrent.NewSlice[Todo](),
+	}
+}
+
+func (s *MemoryTodoStorage) Add(todo Todo) {
+	s.todos.Append(todo)
+}
+
+func (s *MemoryTodoStorage) All() []Todo {
+	return s.todos.All()
+}
+
+func (s *MemoryTodoStorage) Len() int {
+	return s.todos.Length()
+}
+
+func (s *MemoryTodoStorage) FindByID(id string) int {
+	_, idx := s.todos.Find(func(t Todo) bool { return t.ID == id })
+	return idx
+}
+
+func (s *MemoryTodoStorage) Update(index int, fn func(Todo) Todo) {
+	s.todos.Update(index, fn)
+}
+
+func (s *MemoryTodoStorage) Clear() {
+	s.todos.Clear()
+}
+
+// TodoOption is a functional option for configuring a TodoTool.
+type TodoOption func(*TodoTool)
+
+// WithStorage sets a custom storage implementation for the TodoTool.
+func WithStorage(storage TodoStorage) TodoOption {
+	return func(t *TodoTool) {
+		t.handler.storage = storage
+	}
+}
 
-func NewTodoTool() *TodoTool {
-	return &TodoTool{
+type todoHandler struct {
+	storage TodoStorage
+}
+
+var NewSharedTodoTool = sync.OnceValue(func() *TodoTool { return NewTodoTool() })
+
+func NewTodoTool(opts ...TodoOption) *TodoTool {
+	t := &TodoTool{
 		handler: &todoHandler{
-			todos: concurrent.NewSlice[Todo](),
+			storage: NewMemoryTodoStorage(),
 		},
 	}
+	for _, opt := range opts {
+		opt(t)
+	}
+	return t
 }
 
 func (t *TodoTool) Instructions() string {
@@ -86,25 +152,25 @@ This toolset is REQUIRED for maintaining task state and ensuring all steps are c
 }
 
 func (h *todoHandler) createTodo(_ context.Context, params CreateTodoArgs) (*tools.ToolCallResult, error) {
-	id := fmt.Sprintf("todo_%d", h.todos.Length()+1)
+	id := fmt.Sprintf("todo_%d", h.storage.Len()+1)
 	todo := Todo{
 		ID:          id,
 		Description: params.Description,
 		Status:      "pending",
 	}
-	h.todos.Append(todo)
+	h.storage.Add(todo)
 
 	return &tools.ToolCallResult{
 		Output: fmt.Sprintf("Created todo [%s]: %s", id, params.Description),
-		Meta:   h.todos.All(),
+		Meta:   h.storage.All(),
 	}, nil
 }
 
 func (h *todoHandler) createTodos(_ context.Context, params CreateTodosArgs) (*tools.ToolCallResult, error) {
-	start := h.todos.Length()
+	start := h.storage.Len()
 	for i, desc := range params.Descriptions {
 		id := fmt.Sprintf("todo_%d", start+i+1)
-		h.todos.Append(Todo{
+		h.storage.Add(Todo{
 			ID:          id,
 			Description: desc,
 			Status:      "pending",
@@ -122,7 +188,7 @@ func (h *todoHandler) createTodos(_ context.Context, params CreateTodosArgs) (*t
 
 	return &tools.ToolCallResult{
 		Output: output.String(),
-		Meta:   h.todos.All(),
+		Meta:   h.storage.All(),
 	}, nil
 }
 
@@ -131,13 +197,13 @@ func (h *todoHandler) updateTodos(_ context.Context, params UpdateTodosArgs) (*t
 	var updated []string
 
 	for _, update := range params.Updates {
-		_, idx := h.todos.Find(func(t Todo) bool { return t.ID == update.ID })
+		idx := h.storage.FindByID(update.ID)
 		if idx == -1 {
 			notFound = append(notFound, update.ID)
 			continue
 		}
 
-		h.todos.Update(idx, func(t Todo) Todo {
+		h.storage.Update(idx, func(t Todo) Todo {
 			t.Status = update.Status
 			return t
 		})
@@ -159,44 +225,40 @@ func (h *todoHandler) updateTodos(_ context.Context, params UpdateTodosArgs) (*t
 		return tools.ResultError(output.String()), nil
 	}
 
-	// Clear all todos if all are completed
 	if h.allCompleted() {
-		h.todos.Clear()
+		h.storage.Clear()
 	}
 
 	return &tools.ToolCallResult{
 		Output: output.String(),
-		Meta:   h.todos.All(),
+		Meta:   h.storage.All(),
 	}, nil
 }
 
 func (h *todoHandler) allCompleted() bool {
-	if h.todos.Length() == 0 {
+	all := h.storage.All()
+	if len(all) == 0 {
 		return false
 	}
-	allDone := true
-	h.todos.Range(func(_ int, todo Todo) bool {
+	for _, todo := range all {
 		if todo.Status != "completed" {
-			allDone = false
 			return false
 		}
-		return true
-	})
-	return allDone
+	}
+	return true
 }
 
 func (h *todoHandler) listTodos(_ context.Context, _ tools.ToolCall) (*tools.ToolCallResult, error) {
 	var output strings.Builder
 	output.WriteString("Current todos:\n")
 
-	h.todos.Range(func(_ int, todo Todo) bool {
+	for _, todo := range h.storage.All() {
 		fmt.Fprintf(&output, "- [%s] %s (Status: %s)\n", todo.ID, todo.Description, todo.Status)
-		return true
-	})
+	}
 
 	return &tools.ToolCallResult{
 		Output: output.String(),
-		Meta:   h.todos.All(),
+		Meta:   h.storage.All(),
 	}, nil
 }
 

pkg/tools/builtin/todo_test.go

@@ -22,7 +22,8 @@ func TestTodoTool_DisplayNames(t *testing.T) {
 }
 
 func TestTodoTool_CreateTodo(t *testing.T) {
-	tool := NewTodoTool()
+	storage := NewMemoryTodoStorage()
+	tool := NewTodoTool(WithStorage(storage))
 
 	result, err := tool.handler.createTodo(t.Context(), CreateTodoArgs{
 		Description: "Test todo item",
@@ -31,8 +32,8 @@ func TestTodoTool_CreateTodo(t *testing.T) {
 	require.NoError(t, err)
 	assert.Contains(t, result.Output, "Created todo [todo_1]: Test todo item")
 
-	assert.Equal(t, 1, tool.handler.todos.Length())
-	todos := tool.handler.todos.All()
+	assert.Equal(t, 1, storage.Len())
+	todos := storage.All()
 	require.Len(t, todos, 1)
 	assert.Equal(t, "todo_1", todos[0].ID)
 	assert.Equal(t, "Test todo item", todos[0].Description)
@@ -48,7 +49,8 @@ func TestTodoTool_CreateTodo(t *testing.T) {
 }
 
 func TestTodoTool_CreateTodos(t *testing.T) {
-	tool := NewTodoTool()
+	storage := NewMemoryTodoStorage()
+	tool := NewTodoTool(WithStorage(storage))
 
 	result, err := tool.handler.createTodos(t.Context(), CreateTodosArgs{
 		Descriptions: []string{
@@ -64,7 +66,7 @@ func TestTodoTool_CreateTodos(t *testing.T) {
 	assert.Contains(t, result.Output, "todo_2")
 	assert.Contains(t, result.Output, "todo_3")
 
-	assert.Equal(t, 3, tool.handler.todos.Length())
+	assert.Equal(t, 3, storage.Len())
 
 	// Verify Meta contains all todos (order not guaranteed from map)
 	metaTodos, ok := result.Meta.([]Todo)
@@ -85,7 +87,7 @@ func TestTodoTool_CreateTodos(t *testing.T) {
 	require.NoError(t, err)
 	assert.Contains(t, result.Output, "Created 1 todos:")
 	assert.Contains(t, result.Output, "todo_4")
-	assert.Equal(t, 4, tool.handler.todos.Length())
+	assert.Equal(t, 4, storage.Len())
 
 	// Verify Meta for second call contains all 4 todos
 	metaTodos, ok = result.Meta.([]Todo)
@@ -126,7 +128,8 @@ func TestTodoTool_ListTodos(t *testing.T) {
 }
 
 func TestTodoTool_UpdateTodos(t *testing.T) {
-	tool := NewTodoTool()
+	storage := NewMemoryTodoStorage()
+	tool := NewTodoTool(WithStorage(storage))
 
 	// Create multiple todos first
 	_, err := tool.handler.createTodos(t.Context(), CreateTodosArgs{
@@ -152,7 +155,7 @@ func TestTodoTool_UpdateTodos(t *testing.T) {
 	assert.Contains(t, result.Output, "todo_3 -> in-progress")
 
 	// Verify the todos were updated
-	todos := tool.handler.todos.All()
+	todos := storage.All()
 	require.Len(t, todos, 3)
 	assert.Equal(t, "completed", todos[0].Status)
 	assert.Equal(t, "pending", todos[1].Status)
@@ -165,7 +168,8 @@ func TestTodoTool_UpdateTodos(t *testing.T) {
 }
 
 func TestTodoTool_UpdateTodos_PartialFailure(t *testing.T) {
-	tool := NewTodoTool()
+	storage := NewMemoryTodoStorage()
+	tool := NewTodoTool(WithStorage(storage))
 
 	// Create two todos so we can complete one without clearing the list
 	_, err := tool.handler.createTodos(t.Context(), CreateTodosArgs{
@@ -186,7 +190,7 @@ func TestTodoTool_UpdateTodos_PartialFailure(t *testing.T) {
 	assert.Contains(t, result.Output, "Not found: nonexistent")
 
 	// Verify the existing todo was updated (list not cleared because todo_2 still pending)
-	todos := tool.handler.todos.All()
+	todos := storage.All()
 	require.Len(t, todos, 2)
 	assert.Equal(t, "completed", todos[0].Status)
 	assert.Equal(t, "pending", todos[1].Status)
@@ -208,7 +212,8 @@ func TestTodoTool_UpdateTodos_AllNotFound(t *testing.T) {
 }
 
 func TestTodoTool_UpdateTodos_ClearsWhenAllCompleted(t *testing.T) {
-	tool := NewTodoTool()
+	storage := NewMemoryTodoStorage()
+	tool := NewTodoTool(WithStorage(storage))
 
 	// Create multiple todos
 	_, err := tool.handler.createTodos(t.Context(), CreateTodosArgs{
@@ -227,7 +232,7 @@ func TestTodoTool_UpdateTodos_ClearsWhenAllCompleted(t *testing.T) {
 	assert.Contains(t, result.Output, "Updated 2 todos")
 
 	// Verify all todos were cleared
-	todos := tool.handler.todos.All()
+	todos := storage.All()
 	assert.Empty(t, todos)
 
 	// Verify Meta is also empty
@@ -236,6 +241,20 @@ func TestTodoTool_UpdateTodos_ClearsWhenAllCompleted(t *testing.T) {
 	assert.Empty(t, metaTodos)
 }
 
+func TestTodoTool_WithStorage(t *testing.T) {
+	storage := NewMemoryTodoStorage()
+	tool := NewTodoTool(WithStorage(storage))
+
+	_, err := tool.handler.createTodo(t.Context(), CreateTodoArgs{
+		Description: "Test item",
+	})
+	require.NoError(t, err)
+
+	// Verify the custom storage received the item
+	assert.Equal(t, 1, storage.Len())
+	assert.Equal(t, "Test item", storage.All()[0].Description)
+}
+
 func TestTodoTool_OutputSchema(t *testing.T) {
 	tool := NewTodoTool()