diff --git a/agent/harness/todo/todo.go b/agent/harness/todo/todo.go index 17ca2a14..25b15370 100644 --- a/agent/harness/todo/todo.go +++ b/agent/harness/todo/todo.go @@ -36,7 +36,7 @@ When a user changes the topic or changes their mind, ensure that you update the Use these tools to manage your tasks: - Use TodoList_Add to break down complex work into trackable items (supports adding one or many at once). -- Use TodoList_Complete to mark items as done when finished (supports one or many at once). +- Use TodoList_Complete to mark items as done when finished (supports one or many at once). Include a reason describing how the items were completed. - Use TodoList_GetRemaining to check what work is still pending. - Use TodoList_GetAll to review the full list including completed items. - Use TodoList_Remove to remove items that are no longer needed (supports one or many at once).` @@ -55,6 +55,13 @@ type ItemInput struct { Description string `json:"description,omitempty"` } +// CompleteInput is the input structure for completing a single todo item. +// It carries the item ID and a reason describing how or why the item was completed. +type CompleteInput struct { + ID int `json:"id"` + Reason string `json:"reason"` +} + type state struct { NextID int `json:"nextId"` Items []Item `json:"items"` @@ -235,16 +242,16 @@ func (p *Provider) createTools(opts []agent.Option) []tool.FuncTool { completeTool := functool.MustNew( functool.Config{ Name: "TodoList_Complete", - Description: "Mark one or more todo items as complete by their IDs. Returns the number of items that were found and marked complete.", + Description: "Mark one or more todo items as complete. Each entry has an ID and a reason describing how/why the item was completed. Returns the number of items that were found and marked complete.", }, - func(ctx tool.Context, ids []int) (int, error) { + func(ctx tool.Context, items []CompleteInput) (int, error) { mu := p.getSessionLock(opts) mu.Lock() defer mu.Unlock() st := p.loadState(opts) - idSet := make(map[int]struct{}, len(ids)) - for _, id := range ids { - idSet[id] = struct{}{} + idSet := make(map[int]struct{}, len(items)) + for _, item := range items { + idSet[item.ID] = struct{}{} } completed := 0 for i := range st.Items { diff --git a/agent/harness/todo/todo_test.go b/agent/harness/todo/todo_test.go index 0c0136ac..45bda317 100644 --- a/agent/harness/todo/todo_test.go +++ b/agent/harness/todo/todo_test.go @@ -155,7 +155,7 @@ func TestCompleteTodos_MarksItemComplete(t *testing.T) { items := p.GetAllItems(opts...) id := items[0].ID - result := callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[%d]}`, id)) + result := callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":"done"}]}`, id)) if !strings.Contains(result, "1") { t.Errorf("expected 1 completed, got %s", result) } @@ -179,7 +179,7 @@ func TestCompleteTodos_MarksMultipleComplete(t *testing.T) { callTool(t, outOpts, "TodoList_Add", `{"Arg0":[{"title":"A"},{"title":"B"},{"title":"C"}]}`) items := p.GetAllItems(opts...) - callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[%d,%d]}`, items[0].ID, items[1].ID)) + callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":"done"},{"id":%d,"reason":"done"}]}`, items[0].ID, items[1].ID)) remaining := p.GetRemainingItems(opts...) if len(remaining) != 1 { @@ -200,7 +200,7 @@ func TestCompleteTodos_ReturnsZeroForMissingIds(t *testing.T) { t.Fatal(err) } - result := callTool(t, outOpts, "TodoList_Complete", `{"Arg0":[999]}`) + result := callTool(t, outOpts, "TodoList_Complete", `{"Arg0":[{"id":999,"reason":"done"}]}`) if !strings.Contains(result, "0") { t.Errorf("expected 0 completed for missing ID, got %s", result) } @@ -280,7 +280,7 @@ func TestGetRemainingTodos_ReturnsOnlyIncomplete(t *testing.T) { callTool(t, outOpts, "TodoList_Add", `{"Arg0":[{"title":"Done"},{"title":"Pending"}]}`) items := p.GetAllItems(opts...) - callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[%d]}`, items[0].ID)) + callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":"done"}]}`, items[0].ID)) remaining := p.GetRemainingItems(opts...) if len(remaining) != 1 { @@ -303,7 +303,7 @@ func TestGetAllTodos_ReturnsAllItems(t *testing.T) { callTool(t, outOpts, "TodoList_Add", `{"Arg0":[{"title":"Done"},{"title":"Pending"}]}`) items := p.GetAllItems(opts...) - callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[%d]}`, items[0].ID)) + callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":"done"}]}`, items[0].ID)) all := p.GetAllItems(opts...) if len(all) != 2 { @@ -369,7 +369,7 @@ func TestPublicGetRemainingTodos_ReturnsOnlyIncomplete(t *testing.T) { callTool(t, outOpts, "TodoList_Add", `{"Arg0":[{"title":"Done"},{"title":"Open"}]}`) items := p.GetAllItems(opts...) - callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[%d]}`, items[0].ID)) + callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":"done"}]}`, items[0].ID)) remaining := p.GetRemainingItems(opts...) if len(remaining) != 1 { @@ -459,7 +459,7 @@ func TestProvide_InjectsTodoListMessage(t *testing.T) { } callTool(t, outOpts, "TodoList_Add", `{"Arg0":[{"title":"Task A"},{"title":"Task B"}]}`) items := p.GetAllItems(opts...) - callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[%d]}`, items[0].ID)) + callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":"done"}]}`, items[0].ID)) // Second call should inject todo list message. outMessages, _, err := p.BeforeRun(context.Background(), newMessages("hi"), opts...) @@ -581,3 +581,93 @@ func TestToolNames(t *testing.T) { } } } + +// Verify CompleteInput with reason is accepted and items are marked complete. +func TestCompleteTodos_WithReason(t *testing.T) { + p := todo.New(nil) + opts := sessionOpts() + + _, outOpts, err := p.BeforeRun(context.Background(), newMessages("hi"), opts...) + if err != nil { + t.Fatal(err) + } + + callTool(t, outOpts, "TodoList_Add", `{"Arg0":[{"title":"Task X"}]}`) + items := p.GetAllItems(opts...) + if len(items) != 1 { + t.Fatalf("expected 1 item, got %d", len(items)) + } + + result := callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":"completed successfully"}]}`, items[0].ID)) + if !strings.Contains(result, "1") { + t.Errorf("expected 1 completed, got %s", result) + } + + all := p.GetAllItems(opts...) + if !all[0].IsComplete { + t.Error("expected item to be complete after providing reason") + } +} + +// Verify that TodoList_Complete description mentions reason. +func TestCompleteToolDescription_MentionsReason(t *testing.T) { + p := todo.New(nil) + opts := sessionOpts() + + _, outOpts, err := p.BeforeRun(context.Background(), newMessages("hi"), opts...) + if err != nil { + t.Fatal(err) + } + + tools := collectTools(outOpts) + for _, tt := range tools { + if tt.Name() == "TodoList_Complete" { + if !strings.Contains(tt.Description(), "reason") { + t.Errorf("expected TodoList_Complete description to mention 'reason', got: %s", tt.Description()) + } + return + } + } + t.Error("TodoList_Complete tool not found") +} + +// TestCompleteTodos_EmptyReasonIsAccepted verifies that TodoList_Complete allows +// an empty or omitted reason, matching the upstream .NET behavior where the reason +// field is prompted for but not enforced at runtime. +func TestCompleteTodos_EmptyReasonIsAccepted(t *testing.T) { + cases := []struct { + name string + reason string + }{ + {"empty reason", ""}, + {"whitespace reason", " "}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p := todo.New(nil) + opts := sessionOpts() + + _, outOpts, err := p.BeforeRun(context.Background(), newMessages("hi"), opts...) + if err != nil { + t.Fatal(err) + } + + callTool(t, outOpts, "TodoList_Add", `{"Arg0":[{"title":"Task Z"}]}`) + items := p.GetAllItems(opts...) + if len(items) != 1 { + t.Fatalf("expected 1 item, got %d", len(items)) + } + + result := callTool(t, outOpts, "TodoList_Complete", fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":%q}]}`, items[0].ID, tc.reason)) + if !strings.Contains(result, "1") { + t.Errorf("expected 1 completed with %q reason, got %s", tc.reason, result) + } + + all := p.GetAllItems(opts...) + if !all[0].IsComplete { + t.Errorf("item should be complete even with %q reason", tc.reason) + } + }) + } +}