Repository navigation
Python: Preserve shell outcome fidelity - #8934
Evan Mattson (moonbox3) merged 3 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The conversions preserve compatibility while regression tests cover successful, failed, and timed-out shell executions.
Review effort: Balanced
Findings: None
What changed in this PR
Preserves structured shell outcomes across local tools, OpenAI Responses, and Foundry hosting.
Changes:
- Retains stdout, stderr, exit codes, and timeout state alongside model-facing text.
- Correctly converts OpenAI and Foundry shell outcomes.
- Adds regression tests and updates the function-loop specification.
| File | Description |
|---|---|
python/packages/tools/tests/test_local_shell_tool.py |
Tests structured local-shell results. |
python/packages/tools/tests/test_docker_shell_tool.py |
Tests structured Docker-shell results. |
python/packages/tools/agent_framework_tools/shell/_types.py |
Converts shell results to structured content. |
python/packages/tools/agent_framework_tools/shell/_tool.py |
Returns structured local-shell content. |
python/packages/tools/agent_framework_tools/shell/_docker.py |
Returns structured Docker-shell content. |
python/packages/openai/tests/openai/test_openai_chat_client.py |
Covers OpenAI shell serialization. |
python/packages/openai/agent_framework_openai/_chat_client.py |
Preserves structured OpenAI shell outcomes. |
python/packages/foundry_hosting/tests/test_responses.py |
Covers Foundry outcome round-trips. |
python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py |
Maps Foundry exit and timeout outcomes. |
docs/specs/004-python-function-calling-loop.md |
Documents the restored fidelity invariant. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): abccb3e779c7
Model: gpt-5.6-sol
Overview
The change preserves structured shell outcomes through the local tool, OpenAI, and Foundry boundaries while retaining the existing model-facing text, with focused coverage for success, non-zero exit, timeout, and persistence paths. Type guards and provider-specific outcome conversion constrain malformed metadata and keep timeout handling distinct from normal exits. However, returning Content directly from the shell wrappers breaks raw-value integrations such as Monty, where commands now execute but their results are rejected instead of reaching sandbox code.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/tools/agent_framework_tools/shell/_tool.py
|
One regression I hit while checking this against the original repro: when a command's output is truncated, the OpenAI |
|
Manjunath Janardhan (@manjunathshiva) Addressed in |
b7fe5ee to
8315fc4
Compare
|
Thanks, verified on |
Motivation & Context
Shell execution outcomes were crossing Python provider boundaries as display text or default exit outcomes, causing non-zero exits, stderr, and timeouts to be reported as successful commands. This restores structured shell result fidelity for local OpenAI Responses execution and Foundry hosting.
Description & Review Guide
Related Issue
Fixes #8929
Fixes #8930
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.