feat(proxyrunner): wire SIGTERM-aware graceful shutdown (RC-14)#4241
feat(proxyrunner): wire SIGTERM-aware graceful shutdown (RC-14)#4241
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the thv-proxyrunner entrypoint to propagate a signal-cancellable root context into Cobra execution so Kubernetes termination signals trigger the existing graceful drain path (context cancellation → runner stop → transportHandler.Stop) rather than abrupt termination.
Changes:
- Add
signal.NotifyContextincmd/thv-proxyrunner/main.goto cancel onos.Interrupt,SIGTERM, andSIGQUIT. - Switch from
Execute()toExecuteContext(ctx)and log/exit on command execution errors.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4241 +/- ##
==========================================
- Coverage 69.29% 68.95% -0.35%
==========================================
Files 470 473 +3
Lines 47197 47854 +657
==========================================
+ Hits 32706 32997 +291
- Misses 11957 12266 +309
- Partials 2534 2591 +57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replace Execute() with signal.NotifyContext + ExecuteContext() in cmd/thv-proxyrunner/main.go, mirroring cmd/vmcp/main.go. SIGTERM, SIGQUIT, and os.Interrupt now cancel the root context so in-flight MCP proxy connections drain via transportHandler.Stop instead of being dropped abruptly during Kubernetes pod termination. Closes #4207
There was a problem hiding this comment.
Pull request overview
Adds SIGTERM-aware graceful shutdown handling to the thv-proxyrunner entrypoint so Kubernetes pod termination signals cancel the root context and allow the runner’s existing drain/stop path to execute.
Changes:
- Update
cmd/thv-proxyrunner/main.goto usesignal.NotifyContext(...)andExecuteContext(ctx)(SIGTERM/SIGQUIT/Interrupt cancel the root context). - Add a new e2e test that starts
thv-proxyrunner, waits for the proxy port to accept connections, sends SIGTERM, and asserts shutdown.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/thv-proxyrunner/main.go | Wires OS-signal cancellation into the Cobra root command via ExecuteContext. |
| test/e2e/proxyrunner_graceful_shutdown_test.go | Adds an e2e scenario for SIGTERM-driven shutdown behavior of thv-proxyrunner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* feat(proxyrunner): wire SIGTERM-aware graceful shutdown (RC-14) Replace Execute() with signal.NotifyContext + ExecuteContext() in cmd/thv-proxyrunner/main.go, mirroring cmd/vmcp/main.go. SIGTERM, SIGQUIT, and os.Interrupt now cancel the root context so in-flight MCP proxy connections drain via transportHandler.Stop instead of being dropped abruptly during Kubernetes pod termination. Closes #4207 * add e2e test --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
Summary
Replace Execute() with signal.NotifyContext + ExecuteContext() in cmd/thv-proxyrunner/main.go, mirroring cmd/vmcp/main.go. SIGTERM, SIGQUIT, and os.Interrupt now cancel the root context so in-flight MCP proxy connections drain via transportHandler.Stop instead of being dropped abruptly during Kubernetes pod termination.
Fixes #4207
Type of change
Test plan
task test)task test-e2e)task lint-fix)Changes
Does this introduce a user-facing change?
Special notes for reviewers