Skip to content

Comments

fix(opencode): run tui worker shutdown on signal/terminal-exit paths#14238

Closed
s2bomb wants to merge 1 commit intoanomalyco:devfrom
s2bomb:fix/tui-signal-shutdown-leak
Closed

fix(opencode): run tui worker shutdown on signal/terminal-exit paths#14238
s2bomb wants to merge 1 commit intoanomalyco:devfrom
s2bomb:fix/tui-signal-shutdown-leak

Conversation

@s2bomb
Copy link

@s2bomb s2bomb commented Feb 19, 2026

Fixes #14237

What does this PR do?

Ensures the TUI command routes process-exit signals through the existing worker
shutdown path, instead of only cleaning up during normal UI exit.

Why this change

thread.ts calls client.call("shutdown") only inside onExit. There are no
handlers for SIGINT, SIGTERM, or SIGHUP, so signal-based exits skip worker
cleanup and child resources can survive.

worker.ts already has the canonical cleanup (Instance.disposeAll() with timeout

  • server.stop(true)). This patch reliably invokes that existing logic from all
    exit paths.

Changes

packages/opencode/src/cli/cmd/tui/thread.ts — one idempotent shutdown wrapper
with a 5s timeout, signal handlers for SIGINT/SIGTERM/SIGHUP, and onExit
routed through the same path. Mirrors signal-driven shutdown already used in other
command entrypoints. No new cleanup semantics introduced.

How did you verify your code works?

Reproduced leak before fix using a deterministic harness (local fake MCP server
under script -q -c), then verified clean exit after fix:

# start TUI under pseudo-terminal host
script -q -c "opencode /tmp/repro-project" /tmp/tui.log &
# send SIGHUP to TUI process
kill -HUP "$(pgrep -f 'opencode /tmp/repro-project')"
# confirm clean exit (exit code 0, no surviving children)

Full test suite: bun test --timeout 30000 in packages/opencode — 1095 pass,
5 skip, 0 fail.

@github-actions github-actions bot added the needs:compliance This means the issue will auto-close after 2 hours. label Feb 19, 2026
@github-actions
Copy link
Contributor

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

@github-actions
Copy link
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Contributor

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate PRs Found

Based on the search results, here are related PRs that may be addressing similar issues:

  1. fix(tui): add signal handlers to prevent orphaned processes on terminal close #13848 - fix(tui): add signal handlers to prevent orphaned processes on terminal close

    • Directly related: Adds signal handlers to TUI to prevent orphaned processes, which is very similar to the current PR's goal of handling SIGINT/SIGTERM/SIGHUP
  2. fix(cli): handle SIGHUP to prevent orphaned processes on terminal close #12718 - fix(cli): handle SIGHUP to prevent orphaned processes on terminal close

    • Related: Specifically handles SIGHUP in CLI to prevent orphaned processes, overlaps with signal handling approach in current PR
  3. fix: resolve memory leaks and zombie processes from missing cleanup handlers #13186 - fix: resolve memory leaks and zombie processes from missing cleanup handlers

    • Related: Addresses the same problem of missing cleanup handlers causing zombie processes and resource leaks
  4. fix: prevent orphaned worker process when killed #11603 - fix: prevent orphaned worker process when killed

    • Related: Directly addresses preventing orphaned worker processes

Recommendation: Check PR #13848 first, as it appears to be the most directly related to TUI signal handling. Verify if it already covers the current PR's scope or if this PR addresses remaining gaps.

@s2bomb
Copy link
Author

s2bomb commented Feb 19, 2026

Closing — this duplicates #13848 (and partially #12718), which I discovered after submission. Will comment there instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs:compliance This means the issue will auto-close after 2 hours.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TUI: signal exits leak child processes

1 participant