Repository navigation
Python: Extend max output limit to stderr - #9795
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The change breaks existing max_output_bytes callers and leaves subprocess pipe transports open after reader cancellation, allowing descriptors and buffers to accumulate across repeated commands.
2 open issues
What changed in this PR
This PR extends bounded output capture to stderr in Agent Framework’s Python shell tools, covering local and Docker execution.
Changes:
- Adds independent stdout and stderr limits with bounded head-and-tail capture.
- Refactors persistent completion parsing and stateless capture cleanup.
- Expands regression tests and documents output limits and host-memory considerations.
| File | Description |
|---|---|
skills/agent-framework-builder-python/references/harness-agent-builder.md |
Documents output limits and host resource responsibilities. |
python/packages/tools/tests/test_shell_truncate_and_quote.py |
Tests bounded streaming capture and text decoding. |
python/packages/tools/tests/test_shell_parse_rc.py |
Tests incremental completion-marker parsing. |
python/packages/tools/tests/test_local_shell_tool.py |
Adds output-limit, recovery, and cleanup tests. |
python/packages/tools/tests/test_docker_shell_tool.py |
Adds Docker capture and termination tests. |
python/packages/tools/README.md |
Explains independent limits and interruption behavior. |
python/packages/tools/agent_framework_tools/shell/_types.py |
Updates truncation documentation for separate limits. |
python/packages/tools/agent_framework_tools/shell/_truncate.py |
Adds bounded byte and text buffers. |
python/packages/tools/agent_framework_tools/shell/_tool.py |
Exposes and forwards separate local-shell limits. |
python/packages/tools/agent_framework_tools/shell/_session.py |
Uses bounded persistent capture and incremental marker parsing. |
python/packages/tools/agent_framework_tools/shell/_executor.py |
Integrates bounded stateless capture and cleanup. |
python/packages/tools/agent_framework_tools/shell/_docker.py |
Integrates separate limits and refactors container termination. |
python/packages/tools/agent_framework_tools/shell/_capture.py |
Introduces shared concurrent subprocess capture. |
Review effort: Balanced
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): 95d20a6c8b8d
Model: gpt-5.6-sol
Overview
The change adds independent bounded capture for stdout and stderr, with strong coverage for unequal limits, truncation, cancellation, and persistent-session cleanup. Per-command buffers are bounded and released, and no shared-capacity security regression was established. Two compatibility and lifecycle gaps remain: the public constructor migration rejects the former keyword, and local stateless timeout cleanup can abandon a child that inherited the exited shell's pipes.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: python/packages/tools/agent_framework_tools/shell/_capture.py, python/packages/tools/agent_framework_tools/shell/_tool.py


Motivation & Context
The container shell code limits the captured output on stdout, but was missing the same for stderr
Description & Review Guide
Extend max output limit to stderr.
Related Issue
Fixes #
AI Assistance
AI assistance details:
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.