Skip to content

fix(sandbox): terminate POSIX shell children on timeout - #3508

Draft
BichengWang wants to merge 1 commit into
ag2ai:mainfrom
BichengWang:fix/local-shell-timeout-cleanup
Draft

BichengWang wants to merge 1 commit into
ag2ai:mainfrom
BichengWang:fix/local-shell-timeout-cleanup

Conversation

@BichengWang

Copy link
Copy Markdown
Contributor

Why are these changes needed?

A local shell command can return timeout exit code 124 while its child keeps running and writing files. Killing only sh leaves the actual command alive. This also happens when the shell has exited but a background child still holds the output pipes open.

Start local commands in their own POSIX session and kill their process group on timeout, then reap the direct process. Other platforms retain direct-process termination. The timeout message and exit code stay the same. The sandbox docstring describes the POSIX behavior.

The regression starts a child that waits for a release file. After the timeout returns, releasing it must not create its completion file. Both the waiting-shell and exited-shell cases fail on unchanged upstream and pass with the fix.

Related issue number

No existing issue found for this timeout defect. Cancellation of an executing call is a separate defect and remains outside this fix.

Checks

  • I've included any doc changes needed for https://docs.ag2.ai/. The sandbox timeout docstring describes the behavior; no public signature changes.
  • I've added tests (if relevant) corresponding to the changes introduced in this PR.
  • I've made sure all auto checks have passed. Hosted CI is pending.

Local validation on macOS / Python 3.14.7:

  • just test constrained to sandbox tests, local shell tests, and sandbox code-tool tests: 169 passed.
  • Both regression cases fail on unchanged upstream at d08d75c.
  • Repository-wide Ruff 0.16.10 lint and format checks passed (1,222 files checked for formatting).
  • Mypy 2.4.0 passed for ag2/tools/sandbox/local.py; license checks and git diff --check passed.
  • An unconstrained test collection stopped on missing optional dependencies, including referencing, jsonschema, and respx; the full suite was not validated. The dependency-installing lint/type hook wrappers were replaced with the direct checks above. Windows was not exercised; the process-group regression is POSIX-only.
  • No model API calls were used.

AI assistance

The reproduction, implementation, regression and description were checked against the actual diff and local validation above.

  • I understand the changes in this PR and can explain them in my own words.
  • I have verified that the PR description accurately reflects the actual diff.
  • If AI assistance was used, I reviewed, tested, and validated the generated code/text before submitting.

@github-actions github-actions Bot added the area:tools Tool system, builtin tools, subagents (ag2/tools) label Oct 26, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:tools Tool system, builtin tools, subagents (ag2/tools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant