Skip to content

[Bugfix #389] Fix: Kill shellper processes during af cleanup - #390

Merged
waleedkadous merged 1 commit into
mainfrom
builder/bugfix-389-af-cleanup-doesn-t-kill-shellp
Feb 17, 2026
Merged

[Bugfix #389] Fix: Kill shellper processes during af cleanup#390
waleedkadous merged 1 commit into
mainfrom
builder/bugfix-389-af-cleanup-doesn-t-kill-shellp

Conversation

@waleedkadous

Copy link
Copy Markdown
Contributor

Summary

Fixes #389

Root Cause

af cleanup relied exclusively on the Tower REST API (TowerClient.killTerminal()) to kill shellper processes. This silently fails when:

  • Tower is not running (exception caught and swallowed)
  • The terminal was already removed from Tower's registry (e.g., process exited naturally)
  • Tower was restarted and lost the shellper session mapping

In all these cases, the shellper process and its PTY children (Claude, bash) are left running as orphans.

Fix

Added killShellperProcesses(worktreePath) in cleanup.ts that:

  1. Searches ps -ww -eo pid,args output for shellper-main.js processes
  2. Matches by exact worktree path in the JSON config cwd field (prevents false positives from partial path prefixes)
  3. Kills via process group signal (SIGTERM to -pid) to also terminate PTY children
  4. Falls back to individual PID kill if process group kill fails
  5. Runs after the Tower API kill attempt as a reliable fallback for all non-shell builders
  6. Non-fatal: silently handles cases where ps isn't available or no processes are found

Test Plan

  • Added regression test (cleanup-shellper-kill.test.ts, 9 tests)
  • Tests cover: exact cwd matching, no false positives on different/partial paths, multiple kills, process group fallback, ps failure handling, self-process exclusion
  • Existing cleanup tests pass (cleanup-task-builders.test.ts, 13 tests)
  • TypeScript type-check passes
  • Build passes

Previously, af cleanup relied solely on the Tower REST API to kill
shellper processes. This silently failed when Tower wasn't running,
the terminal was already removed from Tower's registry, or Tower was
restarted — leaving shellper processes (and their PTY children like
Claude) orphaned and consuming resources.

Added killShellperProcesses() that searches ps output for
shellper-main.js processes whose JSON config cwd matches the
builder's worktree path, and kills them via process group signal
(SIGTERM to -pid). This runs after the Tower API kill attempt
as a reliable fallback for all non-shell builders.
@waleedkadous
waleedkadous merged commit 44ea268 into main Feb 17, 2026
3 of 7 checks passed
@waleedkadous
waleedkadous deleted the builder/bugfix-389-af-cleanup-doesn-t-kill-shellp branch February 17, 2026 09:27
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.

af cleanup doesn't kill shellper processes — orphaned processes accumulate

1 participant