stacklok / toolhive / 20038516308

60%

Build Type

push

github

Committed by web-flow

Commit Message

Gracefully Shutdown Foreground Server on Interrupt (#2927)

Summary
This pull request is a second attempt at #2863: Gracefully shutting down foreground servers when receiving a SIGINT.

This PR takes greater care to fix tests which became flaky with the original change.

Fixes #1855.

Fixed Tests and Their Changes
osv_mcp_server_test.go
This file has a test that became flaky, specifically the Running OSV MCP server in the foreground" tests. This is the only e2e test that exercises the foreground server like a user would. I made the following changes to the test/code to ensure it passes:

Update the test to document how SIGINT should behave rather than using thv stop/rm.
Simplify the handling of context cancellation in runForeground. Previously, we had two goroutines doing parallel shutdown work: workloadManager.RunWorkload and runForeground. runForeground calls RunWorkload so it's natural to block within runForeground until RunWorload returns. Previously, the two shutdown routines could race on modifying/deleting the workload.
Screenshot 2025-12-07 at 3 32 00 PM
fetch_mcp_server_test.go
This file and likely others exercise --foreground indirectly. The fetch tests are structured to stop a server with a shared name in between tests. Previously, thv stop sent a SIGINT to the background process thv restart --foreground that was spawned by thv run. When the background process received that signal, the signal handler we removed called os.Exit causing the process to exit and no longer babysit the workload.

The problem and solution

thv restart was intentionally coded to ignore context cancellation because we wanted a timeout around the restart's initial startup. Because thv restart ignore all context cancellation, it effectively ignored all SIGINTs and the process would continually resuscitate the server with stale state we were attempting to change across tests. The solution preserves the timeout on startup but ensures post-startup context cancellation is respected.

Run Details

31 of 62 new or added lines in 4 files covered. (50.0%)

6 existing lines in 3 files now uncovered.

26441 of 53256 relevant lines covered (49.65%)

59.03 hits per line