From a085157c390f177e1706ccf07bfdbf317adc2722 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 19 May 2026 10:25:29 +0000 Subject: [PATCH 1/5] feat(todo): require reason in TodoList_Complete aligned with .NET PR #5902 Port the upstream .NET change that replaces the plain-ID slice in TodoList_Complete with a structured CompleteInput{ID, Reason} slice. Agents are now prompted to include a completion reason, which improves auditability of todo lifecycle events. Changes: - Add CompleteInput struct (id + reason fields) - Update completeTool to accept []CompleteInput instead of []int - Update tool description to mention the reason field - Update default instructions to ask agents to include a reason - Update all existing tests to use the new input format - Add TestCompleteTodos_WithReason and TestCompleteToolDescription_MentionsReason Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- agent/harness/todo/todo.go | 19 ++++++---- agent/harness/todo/todo_test.go | 63 +++++++++++++++++++++++++++++---- 2 files changed, 69 insertions(+), 13 deletions(-) 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..43819d77 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,52 @@ 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") +} From e5cb60d63203b6f4c4215125f6c29ff3d7e80dce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 19 May 2026 13:41:08 +0000 Subject: [PATCH 2/5] Validate reason field in TodoList_Complete and add negative tests --- agent/harness/todo/todo.go | 5 +++ agent/harness/todo/todo_test.go | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/agent/harness/todo/todo.go b/agent/harness/todo/todo.go index 25b15370..17042e48 100644 --- a/agent/harness/todo/todo.go +++ b/agent/harness/todo/todo.go @@ -245,6 +245,11 @@ func (p *Provider) createTools(opts []agent.Option) []tool.FuncTool { 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, items []CompleteInput) (int, error) { + for _, item := range items { + if strings.TrimSpace(item.Reason) == "" { + return 0, fmt.Errorf("item %d is missing a completion reason", item.ID) + } + } mu := p.getSessionLock(opts) mu.Lock() defer mu.Unlock() diff --git a/agent/harness/todo/todo_test.go b/agent/harness/todo/todo_test.go index 43819d77..8777a360 100644 --- a/agent/harness/todo/todo_test.go +++ b/agent/harness/todo/todo_test.go @@ -630,3 +630,57 @@ func TestCompleteToolDescription_MentionsReason(t *testing.T) { } t.Error("TodoList_Complete tool not found") } + +// TestCompleteTodos_EmptyReasonIsRejected verifies that TodoList_Complete +// returns an error when a completion entry has an empty or whitespace-only reason. +func TestCompleteTodos_EmptyReasonIsRejected(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 Y"}]}`) + items := p.GetAllItems(opts...) + if len(items) != 1 { + t.Fatalf("expected 1 item, got %d", len(items)) + } + + argsJSON := fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":%q}]}`, items[0].ID, tc.reason) + + var completeTool tool.FuncTool + for _, opt := range outOpts { + if tt, ok := opt.Value().(tool.Tool); ok && tt.Name() == "TodoList_Complete" { + completeTool = tt.(tool.FuncTool) + break + } + } + if completeTool == nil { + t.Fatal("TodoList_Complete tool not found") + } + + _, err = completeTool.Call(tool.Context{Context: context.Background()}, argsJSON) + if err == nil { + t.Errorf("expected error for %q reason, got nil", tc.reason) + } + + // The item must not have been marked complete. + all := p.GetAllItems(opts...) + if all[0].IsComplete { + t.Error("item should not be marked complete when reason is invalid") + } + }) + } +} From 86c5f112d12688f8303c8161323056fcf5d3b36f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 19 May 2026 14:46:28 +0000 Subject: [PATCH 3/5] Remove strict reason validation to match .NET behavior --- agent/harness/todo/todo.go | 5 --- agent/harness/todo/todo_test.go | 54 --------------------------------- 2 files changed, 59 deletions(-) diff --git a/agent/harness/todo/todo.go b/agent/harness/todo/todo.go index 17042e48..25b15370 100644 --- a/agent/harness/todo/todo.go +++ b/agent/harness/todo/todo.go @@ -245,11 +245,6 @@ func (p *Provider) createTools(opts []agent.Option) []tool.FuncTool { 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, items []CompleteInput) (int, error) { - for _, item := range items { - if strings.TrimSpace(item.Reason) == "" { - return 0, fmt.Errorf("item %d is missing a completion reason", item.ID) - } - } mu := p.getSessionLock(opts) mu.Lock() defer mu.Unlock() diff --git a/agent/harness/todo/todo_test.go b/agent/harness/todo/todo_test.go index 8777a360..43819d77 100644 --- a/agent/harness/todo/todo_test.go +++ b/agent/harness/todo/todo_test.go @@ -630,57 +630,3 @@ func TestCompleteToolDescription_MentionsReason(t *testing.T) { } t.Error("TodoList_Complete tool not found") } - -// TestCompleteTodos_EmptyReasonIsRejected verifies that TodoList_Complete -// returns an error when a completion entry has an empty or whitespace-only reason. -func TestCompleteTodos_EmptyReasonIsRejected(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 Y"}]}`) - items := p.GetAllItems(opts...) - if len(items) != 1 { - t.Fatalf("expected 1 item, got %d", len(items)) - } - - argsJSON := fmt.Sprintf(`{"Arg0":[{"id":%d,"reason":%q}]}`, items[0].ID, tc.reason) - - var completeTool tool.FuncTool - for _, opt := range outOpts { - if tt, ok := opt.Value().(tool.Tool); ok && tt.Name() == "TodoList_Complete" { - completeTool = tt.(tool.FuncTool) - break - } - } - if completeTool == nil { - t.Fatal("TodoList_Complete tool not found") - } - - _, err = completeTool.Call(tool.Context{Context: context.Background()}, argsJSON) - if err == nil { - t.Errorf("expected error for %q reason, got nil", tc.reason) - } - - // The item must not have been marked complete. - all := p.GetAllItems(opts...) - if all[0].IsComplete { - t.Error("item should not be marked complete when reason is invalid") - } - }) - } -} From e2ab007ff7a57fe143f93bd01966fdc6c533070f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 19 May 2026 14:48:58 +0000 Subject: [PATCH 4/5] Add test documenting empty-reason acceptance matching .NET behavior --- agent/harness/todo/todo_test.go | 41 +++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/agent/harness/todo/todo_test.go b/agent/harness/todo/todo_test.go index 43819d77..61ae7951 100644 --- a/agent/harness/todo/todo_test.go +++ b/agent/harness/todo/todo_test.go @@ -630,3 +630,44 @@ func TestCompleteToolDescription_MentionsReason(t *testing.T) { } 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) + } + }) + } +} From fabb28536a141ff2dc9682ec393b67c9c9d3fdcd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 08:32:59 +0000 Subject: [PATCH 5/5] Fix gofumpt formatting in todo_test.go --- agent/harness/todo/todo_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/harness/todo/todo_test.go b/agent/harness/todo/todo_test.go index 61ae7951..45bda317 100644 --- a/agent/harness/todo/todo_test.go +++ b/agent/harness/todo/todo_test.go @@ -636,8 +636,8 @@ func TestCompleteToolDescription_MentionsReason(t *testing.T) { // field is prompted for but not enforced at runtime. func TestCompleteTodos_EmptyReasonIsAccepted(t *testing.T) { cases := []struct { - name string - reason string + name string + reason string }{ {"empty reason", ""}, {"whitespace reason", " "},