Skip to content

fix(tmux): reap pane descendants on destroy#2650

Open
VenkataSakethDakuri wants to merge 2 commits into
AgentWrapper:mainfrom
VenkataSakethDakuri:fix/session-orphan-process-reaper
Open

fix(tmux): reap pane descendants on destroy#2650
VenkataSakethDakuri wants to merge 2 commits into
AgentWrapper:mainfrom
VenkataSakethDakuri:fix/session-orphan-process-reaper

Conversation

@VenkataSakethDakuri

@VenkataSakethDakuri VenkataSakethDakuri commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • list tmux pane session PIDs before destroying a session
  • after kill-session, SIGTERM/SIGKILL those pane sessions to reap background descendants like dev servers
  • add unit coverage for pane PID discovery and reaper invocation

Scope

  • This runs only when AO explicitly tears down the runtime, such as ao session kill, cleanup/reconcile paths, or other callers of Runtime.Destroy.
  • It does not make lifecycle-manager terminal transitions call runtime teardown. That is intentional: lifecycle terminal state is a durable fact, not a resource-destruction command.
  • Normal agent exit still leaves the tmux session available for user monitoring and terminal inspection; this change only tightens cleanup once teardown is explicitly requested.

Validation

  • go test ./internal/adapters/runtime/tmux

Refs #2523

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens tmux runtime teardown so that Runtime.Destroy not only kills the tmux session but also reaps pane descendant processes (e.g., backgrounded dev servers) that can otherwise survive tmux kill-session and keep ports/resources allocated.

Changes:

  • Capture tmux pane PIDs (session leaders) before teardown and reap those sessions after kill-session via SIGTERM→grace→SIGKILL.
  • Add Options.ReapGrace / default grace timing for the reap wait.
  • Add unit tests covering pane PID discovery and destroy-time reaper invocation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
backend/internal/adapters/runtime/tmux/tmux.go Adds pane PID discovery and post-destroy session reaping with configurable grace period.
backend/internal/adapters/runtime/tmux/commands.go Adds listPanePIDsArgs command builder for tmux list-panes PID discovery.
backend/internal/adapters/runtime/tmux/tmux_test.go Extends Destroy tests and introduces a reaper seam for safe unit testing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread backend/internal/adapters/runtime/tmux/tmux.go
Comment thread backend/internal/adapters/runtime/tmux/commands.go
@somewherelostt

Copy link
Copy Markdown
Collaborator

Thanks for contributing to Agent Orchestrator.

This PR is being picked up by the current external contributor on-call pair:

If someone is already working on this, please continue as usual.
The on-call pair is added for visibility, tracking, and support, not to take over the work.
If you need help with review, direction, reproduction, or next steps, please tag @neversettle17-101 and @somewherelostt here.
You can also join the AO Discord if you want faster context, want to ask questions live, or just want to follow what the community is building.

You are also welcome to join the AO Discord community.

We do daily Discord calls at 10 PM IST, except Saturday.

Join the session here:
https://discord.gg/H6ZDcUXmq

Come by if you want to see what is being built, ask questions, or just hang around with the community.

@harshitsinghbhandari harshitsinghbhandari left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small things:

  1. Bump defaultReapGrace from 2s to 5s so background processes get a real chance to release ports before SIGKILL.
  2. The grace wait is paid on every Destroy (a live session always has panes), even when nothing survived the SIGTERM. Consider a pgrep -s survivor check to skip the wait + SIGKILL when the sessions are already empty.

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.

5 participants