fix(provider): terminate job before closing terminal to prevent orphaned processes#168
fix(provider): terminate job before closing terminal to prevent orphaned processes#168pedropombeiro wants to merge 5 commits intonickjvandyke:mainfrom
Conversation
9a5837e to
d41da26
Compare
d41da26 to
f56624a
Compare
Can you try again with the latest version? |
f56624a to
733c590
Compare
Track the terminal's PID at toggle/start time and use shell 'kill' to terminate it during VimLeavePre. This is more reliable than jobstop() because snacks.terminal cleanup can invalidate the job before our stop() is called.
733c590 to
ca9251c
Compare
|
I think the root issue is in |
nickjvandyke
left a comment
There was a problem hiding this comment.
Thanks for this clever solution! Just a couple questions 😁
lua/opencode/provider/terminal.lua
Outdated
| if self.bufnr ~= nil and vim.api.nvim_buf_is_valid(self.bufnr) then | ||
| local job_id = vim.b[self.bufnr].terminal_job_id | ||
| if job_id then | ||
| vim.fn.jobstop(job_id) |
There was a problem hiding this comment.
I see we don't use the same workaround here - does that mean it doesn't suffer the same order-of-operations issues that snacks.terminal does?
There was a problem hiding this comment.
Good catch — I tested this with the terminal provider and it does in fact leave orphaned processes. The issue isn't order-of-operations (we control the lifecycle here), but rather that vim.fn.jobstop() sends SIGHUP which causes the opencode process to daemonize/respawn instead of terminating cleanly.
I've now applied the same PID-based termination to the terminal provider, and extracted the shared kill logic into opencode.provider.util so both providers use the same code path. A key finding: we must skip jobstop entirely when the PID kill succeeds — otherwise jobstop's SIGHUP undoes the clean termination.
Also discovered that vim.fn.system() is unreliable during VimLeavePre (it spawns a child job that Neovim kills during shutdown), so switched to os.execute() which is synchronous.
lua/opencode/provider/snacks.lua
Outdated
| -- Capture the job ID and PID after terminal opens (may be a new terminal) | ||
| vim.defer_fn(function() | ||
| local win = self:get() | ||
| if win and win.buf and vim.api.nvim_buf_is_valid(win.buf) then | ||
| self._job_id = vim.b[win.buf].terminal_job_id | ||
| if self._job_id then | ||
| pcall(function() | ||
| self._pid = vim.fn.jobpid(self._job_id) | ||
| end) | ||
| end | ||
| end | ||
| end, 100) |
There was a problem hiding this comment.
I wonder if we could add this to one of the snacks.terminal callbacks in self.opts? Like on_buf (not sure if that's the right one/at the right time). To reduce duplication and ensure it's always called at the right time, no matter how we start it.
There was a problem hiding this comment.
Done — moved the PID capture into an on_buf callback set up in Snacks.new(). It wraps any user-provided on_buf (e.g., the default keymaps callback from config) so existing behavior is preserved. The vim.defer_fn is still needed since on_buf fires before the terminal job is fully started, but it now lives in a single place and fires automatically for both toggle() and open().
The core capture logic is extracted into opencode.provider.util.capture_pid(), shared with the terminal provider.
lua/opencode/provider/snacks.lua
Outdated
| -- Kill via PID using shell kill (most reliable during VimLeavePre, | ||
| -- as vim.uv.kill and jobstop may not work when Neovim is shutting down) | ||
| if self._pid then | ||
| vim.fn.system("kill -TERM " .. self._pid .. " 2>/dev/null") |
There was a problem hiding this comment.
May need an "is unix" check here - I don't think this works on Windows right?
There was a problem hiding this comment.
Added a vim.fn.has("unix") guard. On non-Unix systems, falls back to pcall(vim.uv.kill, pid, "sigterm") — libuv's kill is cross-platform. Note that vim.uv.kill only targets the individual PID (not the process group), so on Windows the child processes may not be cleaned up. I couldn't test this on Windows, so it may need further iteration.
… Windows support Move PID capture logic into an on_buf callback in the constructor so it fires automatically for both toggle() and open(), eliminating duplicated code. Wrap the user's existing on_buf callback to preserve custom behavior (e.g., keymap application). Guard the shell 'kill -TERM' call with a Unix platform check and fall back to vim.uv.kill() on non-Unix systems for cross-platform compatibility.
…l provider Extract process kill and PID capture logic into opencode.provider.util, shared by both snacks and terminal providers. This eliminates duplicated code and ensures consistent behavior. Key fix: use os.execute() instead of vim.fn.system() for process group kill during VimLeavePre, and skip jobstop when PID kill succeeds to prevent SIGHUP from causing the process to respawn. Apply the same PID-based termination to the terminal provider, which suffered from the same orphaned process issue as the snacks provider.
Add diagnostic disable/enable annotations around private field access in the on_buf closure, which is part of the constructor but lua-ls treats as an external context.
…haned processes Add PID-based process group kill to the tmux provider, matching the approach used by the snacks and terminal providers. Capture the pane PID via tmux display-message and kill the process group before tmux kill-pane. This is a workaround for anomalyco/opencode#13001 (opencode does not handle SIGHUP gracefully). Document the upstream issue in provider.util so the workaround is easy to identify and remove later.

Summary
When Neovim exits, the
opencode --portprocess stays orphaned in memory instead of being terminated.Root Cause
During
VimLeavePre, snacks.terminal has already cleaned up its internal state, which invalidates the Neovim job. This means:jobstop(job_id)returnstruebut doesn't actually terminate the processjobpid(job_id)returnsnil- Neovim no longer knows which PID the job maps toSolution
Capture the PID (not just job ID) when the terminal opens, and use shell
killto terminate it directly duringVimLeavePre. This bypasses Neovim's job tracking entirely.Changes
_pidat toggle/start time usingjobpid(terminal_job_id)vim.fn.system('kill -TERM ' .. pid)instop()for reliable terminationVimLeavetoVimLeavePrefor earlier cleanupTesting
:Opencode:qa)ps aux | grep "opencode --port"should return nothing