Skip to content

feat(proxyrunner): wire SIGTERM-aware graceful shutdown (RC-14)#4241

Merged
yrobla merged 2 commits intomainfrom
issue-4207
Mar 23, 2026
Merged

feat(proxyrunner): wire SIGTERM-aware graceful shutdown (RC-14)#4241
yrobla merged 2 commits intomainfrom
issue-4207

Conversation

@yrobla
Copy link
Contributor

@yrobla yrobla commented Mar 19, 2026

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

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

Changes

File Change

Does this introduce a user-facing change?

Special notes for reviewers

@yrobla yrobla requested a review from Copilot March 19, 2026 15:35
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Mar 19, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.NotifyContext in cmd/thv-proxyrunner/main.go to cancel on os.Interrupt, SIGTERM, and SIGQUIT.
  • Switch from Execute() to ExecuteContext(ctx) and log/exit on command execution errors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.95%. Comparing base (3ae086f) to head (c7b5470).
⚠️ Report is 21 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
@yrobla yrobla requested review from Copilot and jerm-dro March 20, 2026 09:09
@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Mar 20, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.go to use signal.NotifyContext(...) and ExecuteContext(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.

@yrobla yrobla requested a review from Copilot March 20, 2026 09:30
@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Mar 20, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Mar 20, 2026
@yrobla yrobla merged commit d96dbac into main Mar 23, 2026
70 of 71 checks passed
@yrobla yrobla deleted the issue-4207 branch March 23, 2026 10:12
yrobla added a commit that referenced this pull request Mar 23, 2026
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wire SIGTERM-aware graceful shutdown in thv-proxyrunner entrypoint

4 participants