Dotnet: Fixing FoundryToolboxMcp sample to use created toolbox#5786
Conversation
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR adds a new first-party Python tools package (agent-framework-tools) focused on cross-platform shell execution, and updates the Foundry Toolbox MCP .NET sample to create/connect to a toolbox endpoint programmatically instead of relying on a pre-specified endpoint.
Changes:
- Introduces
python/packages/tools(agent-framework-tools) withLocalShellTool,DockerShellTool,ShellPolicy, andShellEnvironmentProvider, plus unit tests and docs. - Wires Python workspace packaging/lockfiles to include the new package, and updates the OpenAI local-shell sample to use
LocalShellTool. - Updates the .NET Foundry Toolbox MCP sample to create a toolbox at startup and connect via Streamable HTTP with required preview headers.
Show a summary per file
| File | Description |
|---|---|
| python/uv.lock | Adds agent-framework-tools to the uv workspace lock. |
| python/pyproject.toml | Registers agent-framework-tools as a workspace package. |
| python/samples/02-agents/tools/local_shell_with_environment_provider.py | New sample showing ShellEnvironmentProvider with LocalShellTool. |
| python/samples/02-agents/tools/local_shell_with_allowlist.py | New sample showing strict allow-list usage via ShellPolicy. |
| python/samples/02-agents/providers/openai/client_with_local_shell.py | Refactors sample to use LocalShellTool instead of ad-hoc subprocess execution. |
| python/packages/tools/README.md | Documents agent-framework-tools, safety model, and usage examples. |
| python/packages/tools/LICENSE | Adds MIT license file for the new package. |
| python/packages/tools/pyproject.toml | Defines the new package metadata, deps (incl. psutil), and tooling config. |
| python/packages/tools/agent_framework_tools/init.py | Package init with version discovery. |
| python/packages/tools/agent_framework_tools/py.typed | Marks the package as typed. |
| python/packages/tools/agent_framework_tools/shell/init.py | Public exports for shell tools, policy, provider, and helpers. |
| python/packages/tools/agent_framework_tools/shell/_types.py | Defines ShellResult and shell-related exception types. |
| python/packages/tools/agent_framework_tools/shell/_truncate.py | Implements shared head/tail truncation helpers. |
| python/packages/tools/agent_framework_tools/shell/_tool.py | Implements LocalShellTool facade and agent-framework tool wiring. |
| python/packages/tools/agent_framework_tools/shell/_session.py | Implements persistent shell session protocol via sentinels/readers. |
| python/packages/tools/agent_framework_tools/shell/_resolve.py | Cross-platform shell discovery and argv resolution. |
| python/packages/tools/agent_framework_tools/shell/_policy.py | Implements ShellPolicy allow/deny/custom filtering. |
| python/packages/tools/agent_framework_tools/shell/_killtree.py | Implements cross-OS process-tree termination (psutil-backed). |
| python/packages/tools/agent_framework_tools/shell/_executor.py | Stateless subprocess executor with timeout + truncation. |
| python/packages/tools/agent_framework_tools/shell/_executor_base.py | Defines the ShellExecutor protocol. |
| python/packages/tools/agent_framework_tools/shell/_environment.py | Implements ShellEnvironmentProvider and default prompt formatter. |
| python/packages/tools/agent_framework_tools/shell/_docker.py | Implements DockerShellTool plus pure argv builders and availability check. |
| python/packages/tools/tests/init.py | Initializes the new test package. |
| python/packages/tools/tests/test_shell_truncate_and_quote.py | Tests truncation and quoting helpers. |
| python/packages/tools/tests/test_shell_result.py | Tests ShellResult.format_for_model() behavior. |
| python/packages/tools/tests/test_shell_parse_rc.py | Tests sentinel rc parsing for persistent sessions. |
| python/packages/tools/tests/test_shell_environment_provider.py | Tests provider probing, caching, concurrency behavior, and formatting. |
| python/packages/tools/tests/test_security.py | Adds security regression tests and documented residual-risk surface. |
| python/packages/tools/tests/test_policy.py | Tests ShellPolicy default/deny/allow/custom behavior. |
| python/packages/tools/tests/test_local_shell_tool.py | Tests local tool stateless/persistent behavior, policy, timeouts, and concurrency. |
| python/packages/tools/tests/test_docker_shell_tool.py | Tests docker argv builders and docker tool behavior (with gated integration tests). |
| dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step25_FoundryToolboxMcp/README.md | Updates sample docs to describe startup creation + endpoint connection flow. |
| dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step25_FoundryToolboxMcp/Program.cs | Updates sample to create toolbox and connect via Streamable HTTP with headers. |
Copilot's findings
- Files reviewed: 31/33 changed files
- Comments generated: 4
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 78%
✓ Correctness
The .NET sample change in
CreateSampleToolboxAsyncconstructs the toolbox MCP endpoint URL usingapi-version=v{created.Version}, which incorrectly substitutes the toolbox version (e.g.,"2025-09-01") for the Azure API version (should be"2025-05-01-preview"). This is confirmed byFoundryToolboxOptions.cs:26which definesApiVersion = "2025-05-01-preview"andFoundryToolboxService.cs:195which constructs the same class of URL withapi-version={this._options.ApiVersion}. The existing test atFoundryAIToolExtensionsTests.cs:55confirmsToolboxVersion.Versionis a toolbox revision identifier like"2025-09-01", not an API version string. The Python shell tools package appears correct.
✓ Security Reliability
No actionable issues found in this dimension.
✓ Test Coverage
No actionable issues found in this dimension.
✗ Design Approach
It tells readers that the deny-list overrides the allow-list and that disabling approval is safe because the allow-list does the gating, but the framework’s own
ShellPolicycontract says the opposite: allow rules short-circuit deny rules, and pattern matching is not the security boundary. That makes the sample teach the wrong deployment model for unattended local execution.
Flagged Issues
- The
local_shell_with_allowlist.pysample documents an unsafe policy model: it says 'the deny-list still wins' and justifiesapproval_mode="never_require"on that basis, but the framework'sShellPolicycontract explicitly states that allow patterns short-circuit deny patterns and that policy regexes are only a UX pre-filter, not a security control (ShellPolicy.cs:127-132, 178-205).
Automated review by alliscode's agents
|
@alliscode is the PR description accurate? "This pull request introduces a new Python package, agent-framework-tools"?
|
This pull request introduces a new Python package,
agent-framework-tools, providing built-in tools for the Microsoft Agent Framework, with a primary focus on robust, cross-platform shell command execution. It also updates the .NET sample for Foundry Toolbox MCP to simplify toolbox endpoint management and clarify documentation.Contribution Checklist