[test] Add tests for server.enforceToolCallLimit#6598
Merged
Conversation
Comprehensive test coverage for the enforceToolCallLimit method in UnifiedServer, which previously had 0% test coverage. The tests cover all branches and edge cases: - No session: always allowed - Session exists, no GuardInit for server: always allowed - GuardSessionState has nil ToolCallLimits: always allowed - ToolCallLimits is empty map: always allowed - Tool not present in ToolCallLimits: always allowed - Tool limit is 0: always allowed - First call within budget: allowed, counter incremented - Calls up to limit: each allowed and increments counter - Call at limit: returns descriptive error, counter not incremented - Error message contains tool name and max count - Nil ToolCallCounts lazily initialised on first use - Limits are independent per tool - Limits are independent per server - Concurrent calls: no data race, exactly limit calls allowed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 28, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds focused unit coverage for UnifiedServer.enforceToolCallLimit, validating the per-session/per-server/per-tool budget enforcement behavior used by backend tool calls.
Changes:
- Adds table-style subtests for guard clauses, counter increments, exhausted limits, lazy counter initialization, and independence across tools/servers.
- Adds a concurrency test to verify limit enforcement remains race-safe and caps allowed calls exactly.
Show a summary per file
| File | Description |
|---|---|
internal/server/enforce_tool_call_limit_test.go |
Adds direct unit tests for enforceToolCallLimit behavior and concurrency safety. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test Coverage Improvement:
server.enforceToolCallLimitFunction Analyzed
internal/server(*UnifiedServer).enforceToolCallLimitinternal/server/unified.goWhy This Function?
enforceToolCallLimitis a security-relevant budget-enforcement function that capshow many times an agent may invoke a given tool per session per backend server. Despite
being on the critical path of every
callBackendToolinvocation, the function had zerotest coverage: no existing test file exercised it directly or indirectly.
The function contains several non-trivial branches:
ToolCallCountsTests Added
GuardSessionState.ToolCallLimitsis nil → always allowedToolCallLimitsis empty map → always allowedToolCallLimits→ always allowed(max: N)textToolCallCountslazily initialised on first in-budget calllimitcalls allowed (race-detector safe)Coverage Report
Test File
internal/server/enforce_tool_call_limit_test.go— 200 lines, 14 table-driven sub-testsGenerated by Test Coverage Improver
Next run will target the next most complex under-tested function
Warning
Firewall blocked 5 domains
The following domains were blocked by the firewall during workflow execution:
172.30.0.50goproxy.ioproxy.golang.orgreleaseassets.githubusercontent.comsum.golang.orgSee Network Configuration for more information.