Skip to content

fix(provider): terminate job before closing terminal to prevent orphaned processes#168

Open
pedropombeiro wants to merge 5 commits intonickjvandyke:mainfrom
pedropombeiro:fix/terminate-job-on-stop
Open

fix(provider): terminate job before closing terminal to prevent orphaned processes#168
pedropombeiro wants to merge 5 commits intonickjvandyke:mainfrom
pedropombeiro:fix/terminate-job-on-stop

Conversation

@pedropombeiro
Copy link

@pedropombeiro pedropombeiro commented Feb 9, 2026

Summary

When Neovim exits, the opencode --port process 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) returns true but doesn't actually terminate the process
  • jobpid(job_id) returns nil - Neovim no longer knows which PID the job maps to

Solution

Capture the PID (not just job ID) when the terminal opens, and use shell kill to terminate it directly during VimLeavePre. This bypasses Neovim's job tracking entirely.

Changes

  • snacks provider: Track _pid at toggle/start time using jobpid(terminal_job_id)
  • snacks provider: Use vim.fn.system('kill -TERM ' .. pid) in stop() for reliable termination
  • plugin/provider.lua: Change autocmd from VimLeave to VimLeavePre for earlier cleanup

Testing

  1. Start opencode via :Opencode
  2. Exit Neovim (:qa)
  3. Verify no orphaned processes: ps aux | grep "opencode --port" should return nothing

@nickjvandyke
Copy link
Owner

Thanks for taking a crack at this!

Is it possible to stop the job and all its children too? The child process (opencode) seems to stick around for me. Not sure if this is specific to fish.

image

@pedropombeiro pedropombeiro force-pushed the fix/terminate-job-on-stop branch from 9a5837e to d41da26 Compare February 9, 2026 23:02
@pedropombeiro pedropombeiro reopened this Feb 9, 2026
@pedropombeiro pedropombeiro force-pushed the fix/terminate-job-on-stop branch from d41da26 to f56624a Compare February 9, 2026 23:34
@pedropombeiro
Copy link
Author

Thanks for taking a crack at this!

Is it possible to stop the job and all its children too? The child process (opencode) seems to stick around for me. Not sure if this is specific to fish.

Can you try again with the latest version?

@pedropombeiro pedropombeiro force-pushed the fix/terminate-job-on-stop branch from f56624a to 733c590 Compare February 9, 2026 23:40
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.
@pedropombeiro pedropombeiro force-pushed the fix/terminate-job-on-stop branch from 733c590 to ca9251c Compare February 9, 2026 23:41
@nickjvandyke
Copy link
Owner

I think the root issue is in opencode. Opened anomalyco/opencode#13001. Will see what happens there first 🙂

Copy link
Owner

@nickjvandyke nickjvandyke left a comment

Choose a reason for hiding this comment

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

Thanks for this clever solution! Just a couple questions 😁

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)
Copy link
Owner

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Comment on lines 51 to 62
-- 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)
Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

-- 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")
Copy link
Owner

Choose a reason for hiding this comment

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

May need an "is unix" check here - I don't think this works on Windows right?

Copy link
Author

Choose a reason for hiding this comment

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

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.
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.

2 participants

Comments