Merge pull request docker#1330 from dgageot/todo

Single tool call to update multiple TODOs

Author: rumpl

pkg/acp/agent.go

@@ -698,7 +698,7 @@ func isTodoTool(toolName string) bool {
 	return slices.Contains([]string{
 		builtin.ToolNameCreateTodo,
 		builtin.ToolNameCreateTodos,
-		builtin.ToolNameUpdateTodo,
+		builtin.ToolNameUpdateTodos,
 		builtin.ToolNameListTodos,
 	}, toolName)
 }

pkg/tools/builtin/todo.go

@@ -13,7 +13,7 @@ import (
 const (
 	ToolNameCreateTodo  = "create_todo"
 	ToolNameCreateTodos = "create_todos"
-	ToolNameUpdateTodo  = "update_todo"
+	ToolNameUpdateTodos = "update_todos"
 	ToolNameListTodos   = "list_todos"
 )
 
@@ -39,11 +39,15 @@ type CreateTodosArgs struct {
 	Descriptions []string `json:"descriptions" jsonschema:"Descriptions of the todo items"`
 }
 
-type UpdateTodoArgs struct {
+type TodoUpdate struct {
 	ID     string `json:"id" jsonschema:"ID of the todo item"`
 	Status string `json:"status" jsonschema:"New status (pending, in-progress,completed)"`
 }
 
+type UpdateTodosArgs struct {
+	Updates []TodoUpdate `json:"updates" jsonschema:"List of todo updates"`
+}
+
 type todoHandler struct {
 	todos *concurrent.Slice[Todo]
 }
@@ -120,19 +124,41 @@ func (h *todoHandler) createTodos(_ context.Context, params CreateTodosArgs) (*t
 	}, nil
 }
 
-func (h *todoHandler) updateTodo(_ context.Context, params UpdateTodoArgs) (*tools.ToolCallResult, error) {
-	_, idx := h.todos.Find(func(t Todo) bool { return t.ID == params.ID })
-	if idx == -1 {
-		return tools.ResultError(fmt.Sprintf("todo [%s] not found", params.ID)), nil
+func (h *todoHandler) updateTodos(_ context.Context, params UpdateTodosArgs) (*tools.ToolCallResult, error) {
+	var notFound []string
+	var updated []string
+
+	for _, update := range params.Updates {
+		_, idx := h.todos.Find(func(t Todo) bool { return t.ID == update.ID })
+		if idx == -1 {
+			notFound = append(notFound, update.ID)
+			continue
+		}
+
+		h.todos.Update(idx, func(t Todo) Todo {
+			t.Status = update.Status
+			return t
+		})
+		updated = append(updated, fmt.Sprintf("%s -> %s", update.ID, update.Status))
 	}
 
-	h.todos.Update(idx, func(t Todo) Todo {
-		t.Status = params.Status
-		return t
-	})
+	var output strings.Builder
+	if len(updated) > 0 {
+		fmt.Fprintf(&output, "Updated %d todos: %s", len(updated), strings.Join(updated, ", "))
+	}
+	if len(notFound) > 0 {
+		if output.Len() > 0 {
+			output.WriteString("; ")
+		}
+		fmt.Fprintf(&output, "Not found: %s", strings.Join(notFound, ", "))
+	}
+
+	if len(notFound) > 0 && len(updated) == 0 {
+		return tools.ResultError(output.String()), nil
+	}
 
 	return &tools.ToolCallResult{
-		Output: fmt.Sprintf("Updated todo [%s] to status: [%s]", params.ID, params.Status),
+		Output: output.String(),
 		Meta:   h.todos.All(),
 	}, nil
 }
@@ -179,14 +205,14 @@ func (t *TodoTool) Tools(context.Context) ([]tools.Tool, error) {
 			},
 		},
 		{
-			Name:         ToolNameUpdateTodo,
+			Name:         ToolNameUpdateTodos,
 			Category:     "todo",
-			Description:  "Update the status of a todo item",
-			Parameters:   tools.MustSchemaFor[UpdateTodoArgs](),
+			Description:  "Update the status of one or more todo item(s)",
+			Parameters:   tools.MustSchemaFor[UpdateTodosArgs](),
 			OutputSchema: tools.MustSchemaFor[string](),
-			Handler:      NewHandler(t.handler.updateTodo),
+			Handler:      NewHandler(t.handler.updateTodos),
 			Annotations: tools.ToolAnnotations{
-				Title:        "Update TODO",
+				Title:        "Update TODOs",
 				ReadOnlyHint: true, // Technically not read-only but has practically no destructive side effects.
 			},
 		},

pkg/tools/builtin/todo_test.go

@@ -93,34 +93,6 @@ func TestTodoTool_CreateTodos(t *testing.T) {
 	require.Len(t, metaTodos, 4)
 }
 
-func TestTodoTool_UpdateTodo(t *testing.T) {
-	tool := NewTodoTool()
-
-	_, err := tool.handler.createTodo(t.Context(), CreateTodoArgs{
-		Description: "Test todo item",
-	})
-	require.NoError(t, err)
-
-	result, err := tool.handler.updateTodo(t.Context(), UpdateTodoArgs{
-		ID:     "todo_1",
-		Status: "completed",
-	})
-	require.NoError(t, err)
-	assert.Contains(t, result.Output, "Updated todo [todo_1] to status: [completed]")
-
-	todos := tool.handler.todos.All()
-	require.Len(t, todos, 1)
-	assert.Equal(t, "completed", todos[0].Status)
-
-	// Verify Meta contains all todos with updated status
-	metaTodos, ok := result.Meta.([]Todo)
-	require.True(t, ok, "Meta should be []Todo")
-	require.Len(t, metaTodos, 1)
-	assert.Equal(t, "todo_1", metaTodos[0].ID)
-	assert.Equal(t, "Test todo item", metaTodos[0].Description)
-	assert.Equal(t, "completed", metaTodos[0].Status)
-}
-
 func TestTodoTool_ListTodos(t *testing.T) {
 	tool := NewTodoTool()
 
@@ -153,15 +125,85 @@ func TestTodoTool_ListTodos(t *testing.T) {
 	require.Len(t, metaTodos, 3)
 }
 
-func TestTodoTool_UpdateNonexistentTodo(t *testing.T) {
+func TestTodoTool_UpdateTodos(t *testing.T) {
+	tool := NewTodoTool()
+
+	// Create multiple todos first
+	_, err := tool.handler.createTodos(t.Context(), CreateTodosArgs{
+		Descriptions: []string{
+			"First todo item",
+			"Second todo item",
+			"Third todo item",
+		},
+	})
+	require.NoError(t, err)
+
+	// Update multiple todos in one call
+	result, err := tool.handler.updateTodos(t.Context(), UpdateTodosArgs{
+		Updates: []TodoUpdate{
+			{ID: "todo_1", Status: "completed"},
+			{ID: "todo_3", Status: "in-progress"},
+		},
+	})
+	require.NoError(t, err)
+	assert.False(t, result.IsError)
+	assert.Contains(t, result.Output, "Updated 2 todos")
+	assert.Contains(t, result.Output, "todo_1 -> completed")
+	assert.Contains(t, result.Output, "todo_3 -> in-progress")
+
+	// Verify the todos were updated
+	todos := tool.handler.todos.All()
+	require.Len(t, todos, 3)
+	assert.Equal(t, "completed", todos[0].Status)
+	assert.Equal(t, "pending", todos[1].Status)
+	assert.Equal(t, "in-progress", todos[2].Status)
+
+	// Verify Meta contains all todos with updated status
+	metaTodos, ok := result.Meta.([]Todo)
+	require.True(t, ok, "Meta should be []Todo")
+	require.Len(t, metaTodos, 3)
+}
+
+func TestTodoTool_UpdateTodos_PartialFailure(t *testing.T) {
 	tool := NewTodoTool()
 
-	res, err := tool.handler.updateTodo(t.Context(), UpdateTodoArgs{
-		ID:     "nonexistent_todo",
-		Status: "completed",
+	// Create a single todo
+	_, err := tool.handler.createTodo(t.Context(), CreateTodoArgs{
+		Description: "Test todo item",
+	})
+	require.NoError(t, err)
+
+	// Try to update one existing and one non-existing todo
+	result, err := tool.handler.updateTodos(t.Context(), UpdateTodosArgs{
+		Updates: []TodoUpdate{
+			{ID: "todo_1", Status: "completed"},
+			{ID: "nonexistent", Status: "completed"},
+		},
+	})
+	require.NoError(t, err)
+	assert.False(t, result.IsError) // Not an error because at least one succeeded
+	assert.Contains(t, result.Output, "Updated 1 todos")
+	assert.Contains(t, result.Output, "Not found: nonexistent")
+
+	// Verify the existing todo was updated
+	todos := tool.handler.todos.All()
+	require.Len(t, todos, 1)
+	assert.Equal(t, "completed", todos[0].Status)
+}
+
+func TestTodoTool_UpdateTodos_AllNotFound(t *testing.T) {
+	tool := NewTodoTool()
+
+	// Try to update non-existing todos
+	result, err := tool.handler.updateTodos(t.Context(), UpdateTodosArgs{
+		Updates: []TodoUpdate{
+			{ID: "nonexistent1", Status: "completed"},
+			{ID: "nonexistent2", Status: "completed"},
+		},
 	})
 	require.NoError(t, err)
-	require.True(t, res.IsError)
+	assert.True(t, result.IsError) // Error because all failed
+	assert.Contains(t, result.Output, "Not found: nonexistent1, nonexistent2")
 }
 
 func TestTodoTool_OutputSchema(t *testing.T) {

pkg/tui/components/tool/factory.go

@@ -83,7 +83,7 @@ func newDefaultRegistry() *Registry {
 			[]string{
 				builtin.ToolNameCreateTodo,
 				builtin.ToolNameCreateTodos,
-				builtin.ToolNameUpdateTodo,
+				builtin.ToolNameUpdateTodos,
 				builtin.ToolNameListTodos,
 			},
 			todotool.New,