fix(desktop): kill orphaned terminal daemon processes during teardown

[Kitenite]

Summary

• Electron terminal-host.js and pty-subprocess.js processes spawned from a worktree's node_modules persist as orphans after the worktree is deleted
• Found 14 orphaned processes from 9 deleted worktrees lingering on the system
• Adds a teardown step that kills these processes by matching the worktree path before the directory is removed

Changes

• .superset/teardown.sh: Added step_kill_terminal_daemons function that uses pgrep -f to find Electron processes matching <worktree_path>/.*terminal-host.js and <worktree_path>/.*pty-subprocess.js, then sends SIGTERM to each
• Runs as Step 3 (before Electric SQL and Neon cleanup) so daemons are killed early in the teardown sequence

Test Plan

• Verify a worktree with running terminal-host processes shows them in ps aux | grep terminal-host
• Delete that worktree via the desktop app
• Confirm teardown logs show "Killed N terminal daemon process(es)"
• Confirm those specific processes are gone after deletion

Summary by CodeRabbit

• Chores
  • Improved the application shutdown process to properly clean up background processes before stopping dependent services, ensuring more complete and reliable cleanup.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added a new step_kill_terminal_daemons() function to .superset/teardown.sh that searches for and terminates terminal daemon processes matching the current worktree path. The function is integrated into the teardown flow as a pre-stop step before stopping Electric SQL.

Changes

Cohort / File(s)	Summary
Terminal Daemon Teardown .superset/teardown.sh	Added step_kill_terminal_daemons() function to search and terminate terminal daemon processes (terminal-host.js, pty-subprocess.js) by worktree path. Integrated as a pre-stop step in the main teardown flow before Electric SQL shutdown, with updated step numbering and error tracking labels.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix(desktop): remove auto-restart of terminal daemon in dev mode #897: Modifies terminal daemon lifecycle and auto-restart behavior, directly related to the daemon processes being killed in this PR.
• Storing process logs #89: Adds explicit terminal process termination logic, similar code-level concern for cleaning up terminal sessions.
• fix(desktop): prevent env var leak in persistent terminal subprocess #785: Handles environment configuration for the same terminal daemon subprocesses affected by the teardown cleanup.

Poem

🐰 A script with a purpose so keen,
Hunts down daemons, oh what a scene!
Terminal hosts and pty's in the way—
Cleaned up and killed before we dismay!
Worktrees rejoice, the cleanup's divine! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (54 files): ⚔️ .superset/setup.sh (content) ⚔️ .superset/teardown.sh (content) ⚔️ apps/api/src/app/api/github/install/route.ts (content) ⚔️ apps/api/src/app/api/integrations/linear/connect/route.ts (content) ⚔️ apps/api/src/app/api/integrations/slack/connect/route.ts (content) ⚔️ apps/api/src/app/api/integrations/slack/events/process-app-home-opened/process-app-home-opened.ts (content) ⚔️ apps/api/src/app/api/integrations/slack/events/process-assistant-message/process-assistant-message.ts (content) ⚔️ apps/api/src/app/api/integrations/slack/events/process-mention/process-mention.ts (content) ⚔️ apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts (content) ⚔️ apps/api/src/app/api/integrations/slack/link/route.ts (content) ⚔️ apps/desktop/package.json (content) ⚔️ apps/desktop/scripts/patch-dev-protocol.ts (content) ⚔️ apps/desktop/src/lib/trpc/routers/ports/ports.ts (content) ⚔️ apps/desktop/src/lib/trpc/routers/terminal/terminal.ts (content) ⚔️ apps/desktop/src/main/lib/terminal/daemon/daemon-manager.ts (content) ⚔️ apps/desktop/src/main/lib/terminal/env.test.ts (content) ⚔️ apps/desktop/src/main/lib/terminal/env.ts (content) ⚔️ apps/desktop/src/main/lib/terminal/session.ts (content) ⚔️ apps/desktop/src/main/lib/terminal/types.ts (content) ⚔️ apps/desktop/src/renderer/react-query/workspaces/useOpenExternalWorktree.ts (content) ⚔️ apps/desktop/src/renderer/react-query/workspaces/useOpenWorktree.ts (content) ⚔️ apps/desktop/src/renderer/routes/_authenticated/_dashboard/project/$projectId/page.tsx (content) ⚔️ apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/PropertiesSidebar.tsx (content) ⚔️ apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/components/AssigneeCell/AssigneeCell.tsx (content) ⚔️ apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/components/PriorityCell/PriorityCell.tsx (content) ⚔️ apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/components/StatusCell/StatusCell.tsx (content) ⚔️ apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx (content) ⚔️ apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/PortsList/hooks/usePortsData.ts (content) ⚔️ apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx (content) ⚔️ apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/mosaic-theme.css (content) ⚔️ apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalConnection.ts (content) ⚔️ apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/types.ts (content) ⚔️ apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PresetsBar/PresetsBar.tsx (content) ⚔️ apps/desktop/src/renderer/stores/theme/utils/index.ts (content) ⚔️ apps/desktop/src/shared/utils/branch.test.ts (content) ⚔️ apps/desktop/src/shared/utils/branch.ts (content) ⚔️ apps/desktop/src/shared/worktree-id.ts (content) ⚔️ apps/electric-proxy/src/index.ts (content) ⚔️ apps/electric-proxy/wrangler.jsonc (content) ⚔️ apps/marketing/src/lib/blog-utils.ts (content) ⚔️ apps/marketing/src/lib/blog.ts (content) ⚔️ apps/marketing/src/lib/changelog-utils.ts (content) ⚔️ apps/marketing/src/lib/changelog.ts (content) ⚔️ apps/marketing/src/lib/compare-utils.ts (content) ⚔️ apps/marketing/src/lib/compare.ts (content) ⚔️ apps/web/src/app/(dashboard)/settings/billing/page.tsx (content) ⚔️ packages/db/drizzle/meta/_journal.json (content) ⚔️ packages/db/src/utils/index.ts (content) ⚔️ packages/mcp/package.json (content) ⚔️ packages/mcp/src/tools/devices/start-claude-session/start-claude-session.ts (content) ⚔️ packages/shared/package.json (content) ⚔️ packages/trpc/src/router/integration/utils.ts (content) ⚔️ packages/trpc/src/router/organization/organization.ts (content) ⚔️ packages/trpc/src/router/user/user.ts (content) These conflicts must be resolved before merging into main.	Resolve conflicts locally and push changes to this branch.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: killing orphaned terminal daemon processes during teardown, which matches the core objective of the PR.
Description check	✅ Passed	The PR description is comprehensive and covers all essential sections: summary of the problem, specific changes made, and a detailed test plan. All key information is present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/teardown-kill-terminal-daemons

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch fix/teardown-kill-terminal-daemons
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

.superset/teardown.sh (2)

120-128: ((killed++)) returns exit code 1 on first successful kill when killed=0.

When killed is 0, ((killed++)) evaluates the pre-increment value (0), which is falsy in bash arithmetic, giving exit code 1. The || true saves you here, but the intent is obscured — the first successful kill takes the || true path rather than the && path.

A clearer alternative avoids mixing arithmetic truthiness with control flow:

Suggested fix

-        kill "$pid" 2>/dev/null && ((killed++)) || true
+        if kill "$pid" 2>/dev/null; then
+          killed=$((killed + 1))
+        fi

Also, pgrep -f interprets the pattern as a regex — special characters in worktree_path (e.g., ., +) won't be escaped. Typical worktree paths should be fine, but worth noting if paths ever contain regex metacharacters.

---

113-137: step_kill_terminal_daemons always returns 0, so the if ! guard is dead code.

The function unconditionally returns 0 on line 136, meaning step_failed "Kill terminal daemons" on line 226 can never be reached. This is harmless and defensive, but if you ever want to surface failures (e.g., kill failing on all PIDs), you'd need to return non-zero in that case.

Also applies to: 224-227

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ✅ Neon database branch
• ⚠️ Electric Fly.io app
• ⚠️ Streams Fly.io app

Thank you for your contribution! 🎉