Skip to content

fix(skills): clean up bash process groups on timeout and cancellation - #1146

Merged
zakahan merged 1 commit into
volcengine:mainfrom
zakahan:fix/bash-tool-timeout-cleanup
Sep 22, 2026
Merged

zakahan merged 1 commit into
volcengine:mainfrom
zakahan:fix/bash-tool-timeout-cleanup

Conversation

@zakahan

@zakahan zakahan commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Calling bash_tool(command="sleep 900 | cat", timeout=5, ...) kills only the outer shell on timeout. The pipeline children retain the output pipes, so the subsequent unbounded process.wait() can prevent the tool from returning its timeout error.

Start commands in a separate session on POSIX and kill their process group on timeout or cancellation, including when the shell has already exited. Retain ownership when cancellation arrives during process creation. Limit cleanup waits to 5 seconds and close local transports if descendants still hold the pipes open. Preserve the tool signature, output formatting, timeout error, and cancellation propagation; non-POSIX systems retain direct-process termination.

The regression tests verify the original pipeline command returns a timeout error, its children stop, and the next call outputs TOOL_TIMEOUT_RECOVERED. They also cover cancellation during execution and spawning, an already-exited shell, cleanup deadlines, process-exit races, spawn failures, and normal output compatibility. Process-group cleanup does not claim to terminate descendants that explicitly detach into another session.

Validation:

  • 77 tests passed: tests/tools/test_skills_bash_tool.py, tests/tools/test_skills_session_path.py, and tests/skills.
  • pre-commit passed for both changed files (Ruff lint, formatting, and gitleaks).

@zakahan
zakahan merged commit ce27142 into volcengine:main Sep 22, 2026
15 of 16 checks passed
PolarisCzh pushed a commit to PolarisCzh/veadk-python that referenced this pull request Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants