.NET: Require TODO finish reason and rename SubAgents to BackgroundAgents#5902
.NET: Require TODO finish reason and rename SubAgents to BackgroundAgents#5902westey-m wants to merge 1 commit into
Conversation
github-actions
Bot
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 85%
✓ Correctness
No actionable issues found in this dimension.
✓ Security Reliability
No actionable issues found in this dimension.
✓ Test Coverage
The BackgroundAgents rename is thoroughly tested across all tool name references and instruction strings. For the TodoProvider changes, existing tests are updated to use the new TodoCompleteInput type and a new test verifies the Reason parameter is accepted. However, the new CompleteTodos_AcceptsReasonParameterAsync test only asserts that the item is marked complete and the count is correct—it does not verify that the provided Reason is actually used or surfaced in any output, which is a gap given the PR states the reason is meant to 'improve user output'. Since TodoItem.cs is not in the changed file list, the Reason appears not to be stored on the model, making the observable impact of the Reason field untested.
✓ Design Approach
First, the SubAgents→BackgroundAgents rename also changes the persisted state keys, so any existing session that already has queued/running sub-agent work will be treated as empty after upgrade instead of being recoverable or marked lost. Second, the new Todo completion shape does not actually require a completion reason: the new input model makes
reasonnullable and the handler ignores it entirely, so calls without a reason still succeed despite the new contract text. I found two design mismatches in the updated tests. First, the TodoList completion flow is still being specified as succeeding without any finish reason, which conflicts with the PR’s stated goal of requiring one. Second, the BackgroundAgents rename appears incomplete on the custom-instructions surface: the tests still use the old{sub_agents}placeholder, which would leave{background_agents}callers with an unreplaced literal unless the provider supports both tokens.
Automated review by westey-m's agents
| _ => new SubAgentState(), | ||
| this._sessionState = new ProviderSessionState<BackgroundAgentState>( | ||
| _ => new BackgroundAgentState(), | ||
| this.GetType().Name, |
There was a problem hiding this comment.
The rename changes the session-state key from SubAgentsProvider to BackgroundAgentsProvider. Since ProviderSessionState.GetOrInitializeState initializes fresh state when the exact key is missing, an upgraded app will silently lose all previously persisted sub-agent task metadata instead of surfacing it or marking it lost. Consider preserving the legacy key or adding a migration/fallback path.
| } | ||
|
|
||
| /// <summary> | ||
| /// Verify that CompleteTodos accepts an optional reason parameter. |
There was a problem hiding this comment.
This test completes a TODO without providing a reason and asserts success, effectively blessing the old TodoList_Complete(items:[{ id }]) path. If the PR's intent is to require a reason, this test should instead assert that missing reasons are rejected.
There was a problem hiding this comment.
Pull request overview
This PR updates the .NET Harness to (1) rename “SubAgents” to “BackgroundAgents” to better reflect asynchronous background execution, and (2) extend Todo completion calls to include an optional “reason” payload for improved UI output.
Changes:
- Renames SubAgents provider/types/tool names to BackgroundAgents across src, tests, and samples.
- Changes
TodoList_Completeinput shape fromidstoitems[](with optionalreason) and updates tests/JSON source-gen registration accordingly. - Updates Harness sample Step02 and console tool formatters to reflect the new names and richer Todo completion formatting.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/Todo/TodoProviderTests.cs | Updates tests for new TodoList_Complete argument shape and adds reason acceptance test. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/BackgroundAgents/BackgroundAgentsProviderTests.cs | Renames tests and tool names from SubAgents to BackgroundAgents. |
| dotnet/src/Microsoft.Agents.AI/Harness/Todo/TodoProvider.cs | Updates TodoList_Complete tool signature/description and default instructions. |
| dotnet/src/Microsoft.Agents.AI/Harness/Todo/TodoCompleteInput.cs | Adds new input model for todo completion (id + optional reason). |
| dotnet/src/Microsoft.Agents.AI/Harness/BackgroundAgents/BackgroundTaskStatus.cs | Renames status enum to BackgroundTaskStatus and updates docs. |
| dotnet/src/Microsoft.Agents.AI/Harness/BackgroundAgents/BackgroundTaskInfo.cs | Renames task metadata type and updates status type usage. |
| dotnet/src/Microsoft.Agents.AI/Harness/BackgroundAgents/BackgroundAgentState.cs | Renames serializable state container and task list type. |
| dotnet/src/Microsoft.Agents.AI/Harness/BackgroundAgents/BackgroundAgentsProviderOptions.cs | Renames options type and updates docs for BackgroundAgents. |
| dotnet/src/Microsoft.Agents.AI/Harness/BackgroundAgents/BackgroundAgentsProvider.cs | Renames provider and tool names; updates instructions and implementation text. |
| dotnet/src/Microsoft.Agents.AI/Harness/BackgroundAgents/BackgroundAgentRuntimeState.cs | Renames runtime state type and session dictionary name. |
| dotnet/src/Microsoft.Agents.AI/AgentJsonUtilities.cs | Registers new TodoCompleteInput types and renamed BackgroundAgents types for STJ source gen. |
| dotnet/samples/02-agents/Harness/README.md | Updates sample list entry and link to BackgroundAgents sample. |
| dotnet/samples/02-agents/Harness/Harness_Step02_Research_WithBackgroundAgents/README.md | Updates sample docs and instructions to BackgroundAgents terminology. |
| dotnet/samples/02-agents/Harness/Harness_Step02_Research_WithBackgroundAgents/Program.cs | Updates sample code to use BackgroundAgentsProvider. |
| dotnet/samples/02-agents/Harness/Harness_Step02_Research_WithBackgroundAgents/Harness_Step02_Research_WithBackgroundAgents.csproj | Adds the renamed/new sample project file. |
| dotnet/samples/02-agents/Harness/Harness_Shared_Console/ToolFormatters/ToolCallFormatter.cs | Switches default formatter list to BackgroundAgentToolFormatter. |
| dotnet/samples/02-agents/Harness/Harness_Shared_Console/ToolFormatters/TodoToolFormatter.cs | Adds formatting for new TodoList_Complete(items[]) payload including reason. |
| dotnet/samples/02-agents/Harness/Harness_Shared_Console/ToolFormatters/BackgroundAgentToolFormatter.cs | Renames formatter/class and switches matching to BackgroundAgents_*. |
| dotnet/agent-framework-dotnet.slnx | Updates solution to reference the renamed Step02 sample project. |
Comments suppressed due to low confidence (2)
dotnet/src/Microsoft.Agents.AI/Harness/BackgroundAgents/BackgroundAgentsProvider.cs:40
- This change renames/removes the public SubAgentsProvider API (and tool names) in favor of BackgroundAgentsProvider, which is a breaking change for any consumers. Please either preserve backward compatibility (e.g., keep SubAgentsProvider/old tool names as [Obsolete] shims) or mark the PR as breaking (title/release notes) so downstream users can migrate deliberately.
dotnet/src/Microsoft.Agents.AI/Harness/BackgroundAgents/BackgroundAgentsProviderOptions.cs:22 - The instructions/template placeholder is still named
{sub_agents}even though the feature has been renamed to BackgroundAgents. This is likely to confuse users authoring custom instructions. Consider supporting a{background_agents}placeholder (while still honoring{sub_agents}for compatibility), and update the docs/comments accordingly.
| int id = item.TryGetProperty("id", out JsonElement idElement) ? idElement.GetInt32() : 0; | ||
| string? reason = item.TryGetProperty("reason", out JsonElement reasonElement) | ||
| ? reasonElement.GetString() | ||
| : null; |
| 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. |
| }), | ||
|
|
||
| AIFunctionFactory.Create( | ||
| async (List<int> ids) => | ||
| async (List<TodoCompleteInput> items) => | ||
| { | ||
| SemaphoreSlim sessionLock = this.GetSessionLock(session); | ||
| await sessionLock.WaitAsync().ConfigureAwait(false); | ||
| try | ||
| { | ||
| TodoState state = this._sessionState.GetOrInitializeState(session); | ||
| var idSet = new HashSet<int>(ids); | ||
| var idSet = new HashSet<int>(items.Select(i => i.Id)); | ||
| int completed = 0; |
Motivation and Context
SubAgents are not well named, since it can be confused with cases where we merely delegate to a sub agent in a synchronous way.
Description
Contribution Checklist