fix(apodex): kill the whole command session when run_shell times out

[Rish-it]

Summary

• kill the command's whole session when run_shell times out, instead of leaving the shell and its children running
• start the command in its own session so the group kill reaches everything it spawned
• bound the post-kill wait so a setsid escapee cannot hold the call open
• cover the backgrounded-subshell case with a regression test

Root cause

run_shell awaited proc.communicate() under a bare asyncio.wait_for. On timeout nothing killed the child, so the model was told the command had stopped while it kept burning CPU and writing the workspace. The same leak happens on cancellation: tool_exec wraps every tool invocation in its own wait_for, so an outer tool timeout or a user interrupt cancels run_shell at the same await.

Terminating only the shell is not enough. A grandchild — anything the command backgrounds — keeps the capture pipes open, and asyncio resolves Process.wait() only once those pipes close (base_subprocess.py: exit waiters are woken from _call_connection_lost). A terminate-then-wait fix therefore leaves the grandchild writing the workspace and blocks the caller until it exits on its own:

terminate-only fix [plain]:      returned in 2.0s, marker leaked: False
terminate-only fix [grandchild]: returned in 2.0s, marker leaked: True

This follows _CurrentCommands.run in plugins/tools/_sandbox.py, which already runs the shell in its own session and SIGKILLs the group on the bare host for exactly this reason, bounded wait included. Only the host/native/container path needed it; the bwrap path reaps through sandbox.commands.run.

Reproduction

The child sleeps, then writes a marker; the caller passes timeout=1. A child that was reaped can never write the marker.

On main @ 9e533db:

[plain]      TimeoutError after 1.0s; marker written later: True -> LEAK
[grandchild] TimeoutError after 1.0s; marker written later: True -> LEAK

On this branch:

[plain]      TimeoutError after 1.0s; marker written later: False -> no leak
[grandchild] TimeoutError after 1.0s; marker written later: False -> no leak

Validation

• uv run pytest -q — 1718 passed, 4 skipped (run twice)
• uv run pytest apodex/tests -q — 823 passed, from the committed branch
• uv run ruff check apodex/sandbox.py apodex/tests/test_native.py — passed
• uv run pyright apodex/sandbox.py apodex/tests/test_native.py — 0 errors
• the new test also passes under -W error::ResourceWarning; before the fix it failed and emitted the leaked-transport warning the issue mentions
• checked by hand: stdout/stderr still captured, exit code 3 preserved, a cancelled call returns in 1.0s with no leak, and a setsid escapee returns after timeout + 5s instead of hanging

Notes

• commands now run without the controlling terminal, so an interactive prompt such as sudo fails fast instead of fighting the TUI; the native backend in plugins/tools/_sandbox.py already behaves this way
• os.killpg is POSIX-only; native Windows is not in the release matrix (docs/install/linux.md), so Windows is covered through WSL2

Fixes #40

[Copilot]

🟡 Changes recommended

Cleanup must handle exited shells with live child processes, and the regression test should cover that case.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR prevents timed-out or cancelled native shell commands from leaving background processes running.

Changes:

• Runs commands in separate sessions and bounds cleanup waits.
• Adds regression coverage for backgrounded shell children.

File summaries

File	Summary
apodex/tests/test_native.py	Adds timeout cleanup coverage, but misses the case where the shell exits while a child holds capture pipes.
apodex/sandbox.py	Adds process-group cleanup, which can be skipped after the shell exits before communicate() completes.

Review details

Suppressed comments (1)

apodex/tests/test_native.py:213

• This regression test keeps the shell alive with wait, so proc.returncode remains None and it does not exercise the case where the shell exits while its background child still holds the capture pipes. Use a background command without wait (or add a separate case) so the test would catch the cleanup guard above.

            "(sleep 2; touch marker) & wait", str(tmp_path), 1,

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.