test(sdk): improve tool control docs and add pattern matching tests#2644
test(sdk): improve tool control docs and add pattern matching tests#2644
Conversation
Remove excludeTools and allowedTools configurations from the test as coreTools is sufficient for limiting available tools. Update canUseTool expectation to verify write_file is properly called. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…with permissions reference Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
📋 Review SummaryThis PR updates SDK documentation to align with the 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
- Use includePartialMessages and isSDKPartialAssistantMessage for more reliable abort triggering - Remove flaky tool name assertions that could fail when tools aren't registered (issue #2653) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…ring with itself Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
TLDR
Updates the SDK documentation and adds comprehensive E2E tests for tool control parameters (
coreTools,excludeTools,allowedTools). Documentation now references thepermissionsconfiguration system with proper tool name aliases and pattern matching syntax. Tests cover path-based blocking, shell command prefix matching, permission priority interactions, andcanUseToolcallback behavior.Screenshots / Video Demo
N/A — no user-facing change. This PR focuses on documentation improvements and test coverage for SDK tool control features.
Dive Deeper
Documentation Updates
Updated
coreTools,excludeTools,allowedToolsdescriptions to reference the correctpermissionsconfiguration system (permissions.allow/permissions.deny) instead of the deprecatedtool.*settings.Added tip note after the QueryOptions table recommending users read the permissions configuration documentation for detailed information on tool name aliases (e.g.,
Bash,Read,Edit) and pattern matching syntax (e.g.,Bash(git *),Read(.env),Edit(/src/**)).Improved Permission Priority Chain section to align with the actual implementation and documented format.
Test Coverage
Added comprehensive E2E tests in
tool-control.test.ts:excludeToolswith patterns likeRead(.env),Edit(/src/**)excludeToolswith patterns likeBash(rm *)allowedToolswith patterns likeRead(.json),Read(.txt)allowedToolswith patterns likeShellTool(echo *)permissionMode: 'plan'blocks write tools even withallowedTools, andexcludeToolsblocks tools even inyolomodeupdatedInputhandling and confirmingcanUseToolis not called for allowed toolscoreToolsas a whitelist takes precedence overallowedToolsandexcludeToolsblocks tools even if present incoreToolsMinor Fixes
test-helper.ts: Addedmkdirwithrecursive: truebefore writing files to ensure parent directories existabort-and-lifecycle.test.ts: Updated test prompt to handle exceptions gracefullyReviewer Test Plan
Review documentation changes:
packages/sdk-typescript/README.mdcorrectly documentscoreTools,excludeTools, andallowedToolsRun SDK integration tests:
cd integration-tests/sdk-typescript npm run test:e2e:sdkVerify test coverage for new patterns:
excludeToolswith path patterns:Read(.env),Edit(/src/**)excludeToolswith shell prefixes:Bash(rm *)allowedToolswith path patterns and shell prefixescanUseToolcallback not called for allowed toolscoreToolswhitelist precedenceTesting Matrix
Linked issues / bugs