Wire session-aware backend routing in proxy transports (RC-12) by yrobla · Pull Request #4318 · stacklok/toolhive

yrobla · 2026-03-23T13:17:00Z

Summary

Implements RC-12 from the horizontal scaling RFC (THV-0047, epic #263):

Adds WithSessionStorage(session.Storage) Option to all three proxy transports (streamable, HTTP-SSE, transparent), allowing a Redis-backed session store to be injected for multi-replica deployments
Transparent proxy now stores backend_url = targetURI in session metadata when an initialize response returns Mcp-Session-Id, and routes subsequent requests to the originating pod via the Rewrite closure
Non-initialize requests with an unknown session ID are rejected with HTTP 400 rather than silently forwarded

Fixes #4212

Type of change

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

Large PR Justification

This is a complete PR with an isolated functionality and complete tests. Cannot be split.

github-actions

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

This review will be automatically dismissed once you add the justification section.

PR size has been reduced below the XL threshold. Thank you for splitting this up!

github-actions · 2026-03-23T13:18:13Z

✅ PR size has been reduced below the XL threshold. The size review has been dismissed and this PR can now proceed with normal review. Thank you for splitting this up!

codecov · 2026-03-23T13:22:24Z

Codecov Report

❌ Patch coverage is 68.21705% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.58%. Comparing base (344907c) to head (41ad948).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
...g/transport/proxy/transparent/transparent_proxy.go	77.41%	8 Missing and 6 partials ⚠️
pkg/transport/proxy/httpsse/http_proxy.go	69.04%	8 Missing and 5 partials ⚠️
pkg/transport/stdio.go	0.00%	11 Missing ⚠️
pkg/transport/proxy/streamable/streamable_proxy.go	70.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4318      +/-   ##
==========================================
+ Coverage   69.53%   69.58%   +0.05%     
==========================================
  Files         480      480              
  Lines       48641    48718      +77     
==========================================
+ Hits        33823    33902      +79     
+ Misses      12215    12207       -8     
- Partials     2603     2609       +6

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

chatgpt-codex-connector · 2026-03-23T13:24:49Z

💡 Codex Review

toolhive/pkg/transport/proxy/transparent/transparent_proxy.go

Line 554 in a58edb9

pr.Out.URL = parsed

Preserve path/query when rewriting to backend_url

In Rewrite, assigning pr.Out.URL = parsed overwrites the URL that pr.SetURL(targetURL) already built from the inbound request, which drops the original path/query (for example /mcp). Once a session has backend_url metadata, routed requests can be sent to the backend root instead of the intended MCP endpoint, causing 404s or wrong-handler behavior. Keep the inbound path/query and only swap scheme/host (or re-run SetURL with the backend target) when applying session-aware routing.

toolhive/pkg/api/server.go

Line 623 in a58edb9

    
           provider, err := registry.GetDefaultProviderWithConfig(config.NewDefaultProvider())

Use non-interactive registry lookup in serve mode

lazySkillLookup.SearchSkills creates the registry provider without registry.WithInteractive(false), unlike serve-mode registry routes (pkg/api/v1/registry.go) which explicitly disable browser OAuth flows. In API/server deployments with OAuth-protected registries and no cached token, plain-name install lookup can trigger interactive auth behavior (or fail indirectly) instead of returning a deterministic auth-required response, making registry-backed installs unreliable in headless environments.

toolhive/pkg/skills/skillsvc/skillsvc.go

Line 265 in a58edb9

return result, s.registerSkillInGroup(ctx, opts.Group, opts.Name)

Add resolved registry installs to groups by skill name

After resolving from registry, opts.Name is rewritten to the OCI reference (ref.String()), and this value is then passed to registerSkillInGroup. Group filtering later matches group entries against InstalledSkill.Metadata.Name, so storing the OCI reference causes registry-resolved installs to be omitted from group-scoped results and can leave stale group entries on uninstall. Register the extracted skill name (for example from result.Skill.Metadata.Name) instead of the OCI reference string.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copilot

Pull request overview

This PR implements RC-12 session-sticky routing capabilities in the proxy transports by allowing an injectable session storage backend and using session metadata in the transparent proxy to route follow-up requests back to the originating backend.

Changes:

Add WithSessionStorage(session.Storage) option support to the Streamable HTTP, HTTP-SSE, and Transparent proxies (and adjust constructors to accept options).
Transparent proxy now records backend_url in session metadata on initialize and uses it during ReverseProxy.Rewrite to steer subsequent requests.
Add/extend unit/integration tests for session storage injection and transparent backend routing behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
pkg/transport/stdio.go	Updates proxy constructor calls for new middleware parameter shape; currently no plumbing for session storage injection.
pkg/transport/proxy/transparent/transparent_proxy.go	Adds session storage option, unknown-session guard, session metadata persistence, and Rewrite-based backend routing.
pkg/transport/proxy/transparent/backend_routing_test.go	New tests covering backend routing via `backend_url` and 400s for unknown sessions.
pkg/transport/proxy/transparent/transparent_test.go	Adds a unit test ensuring `WithSessionStorage` initializes a session manager.
pkg/transport/proxy/streamable/streamable_proxy.go	Introduces options pattern and `WithSessionStorage` for Streamable proxy; updates constructor signature.
pkg/transport/proxy/streamable/streamable_proxy_test.go	Updates constructor call sites and adds a `WithSessionStorage` unit test.
pkg/transport/proxy/streamable/streamable_proxy_spec_test.go	Updates constructor call sites for new signature.
pkg/transport/proxy/streamable/streamable_proxy_mcp_client_integration_test.go	Updates constructor call sites for new signature.
pkg/transport/proxy/streamable/streamable_proxy_integration_test.go	Updates constructor call sites for new signature.
pkg/transport/proxy/httpsse/http_proxy.go	Introduces options pattern and `WithSessionStorage` for HTTP-SSE proxy; updates constructor signature.
pkg/transport/proxy/httpsse/http_proxy_test.go	Updates constructor call sites and adds a `WithSessionStorage` unit test.
pkg/transport/proxy/httpsse/http_proxy_integration_test.go	Updates constructor call sites for new signature.

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

pkg/transport/proxy/transparent/transparent_proxy.go

pkg/transport/stdio.go

pkg/transport/proxy/transparent/backend_routing_test.go

pkg/transport/proxy/streamable/streamable_proxy.go

pkg/transport/proxy/httpsse/http_proxy.go

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

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

pkg/transport/proxy/transparent/backend_routing_test.go

pkg/skills/skillsvc/skillsvc.go

pkg/transport/proxy/transparent/transparent_proxy.go

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

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

pkg/transport/proxy/transparent/transparent_proxy.go

pkg/transport/proxy/httpsse/http_proxy.go

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

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

pkg/transport/proxy/transparent/transparent_proxy.go

pkg/transport/proxy/httpsse/http_proxy.go

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

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

pkg/transport/proxy/transparent/transparent_proxy.go

pkg/transport/proxy/httpsse/http_proxy.go

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

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

pkg/transport/proxy/httpsse/http_proxy.go

pkg/transport/proxy/transparent/transparent_proxy.go

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

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

pkg/transport/proxy/transparent/transparent_proxy.go

pkg/transport/proxy/httpsse/http_proxy_integration_test.go

…smatch The CI Go build cache is keyed on go.sum, so source-only changes do not bust it. With -coverpkg=./..., every test binary instruments all packages; if any binary was compiled from a stale cached artifact whose coverage statement count differs from the current source, go tool cover -func errors with "inconsistent NumStmt: changed from N to M". Replace go clean -testcache with go clean -cache -testcache so that both the test-result cache and the compiled artifact cache are wiped before each coverage run, guaranteeing every package is instrumented from fresh source. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Adds Option/WithSessionStorage functional option to the streamable HTTP proxy, enabling injection of a custom session storage backend for multi-replica deployments. Updates NewHTTPProxy signature from variadic middlewares to explicit slice + variadic opts, and fixes all call sites. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…-12) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… in Rewrite (RC-12) On initialize responses that return Mcp-Session-Id, RoundTrip now creates the session via AddSession and stores backend_url=targetURI in metadata. The Rewrite closure in Start looks up that metadata to override the outbound URL, enabling session-aware routing to originating backend pods. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…wn session (RC-12) Guard added in RoundTrip: requests carrying an Mcp-Session-Id that is not found in the session store are rejected with HTTP 400 immediately, unless the request is an initialize call. Two tests added to cover both branches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

yrobla requested review from ChrisJBurns, JAORMX, amirejaz, blkt, jerm-dro, jhrozek, lujunsan and rdimitrov as code owners March 23, 2026 13:17

github-actions bot previously requested changes Mar 23, 2026

View reviewed changes

github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Mar 23, 2026

yrobla force-pushed the issue-4212 branch from a58edb9 to cbf41f8 Compare March 23, 2026 13:17

github-actions bot removed the size/XL Extra large PR: 1000+ lines changed label Mar 23, 2026

github-actions bot added the size/L Large PR: 600-999 lines changed label Mar 23, 2026

yrobla force-pushed the issue-4212 branch from cbf41f8 to f65c121 Compare March 23, 2026 13:22

yrobla requested a review from Copilot March 23, 2026 13:23

github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 23, 2026

Copilot started reviewing on behalf of yrobla March 23, 2026 13:24 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 23, 2026

yrobla requested a review from Copilot March 23, 2026 15:01

Copilot started reviewing on behalf of yrobla March 23, 2026 15:01 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

pkg/transport/proxy/transparent/backend_routing_test.go Outdated Show resolved Hide resolved

pkg/skills/skillsvc/skillsvc.go Show resolved Hide resolved

pkg/transport/proxy/transparent/transparent_proxy.go Outdated Show resolved Hide resolved

yrobla force-pushed the issue-4212 branch from 356dfc8 to 045c6a4 Compare March 23, 2026 15:14

yrobla requested a review from Copilot March 23, 2026 15:14

github-actions bot added the size/L Large PR: 600-999 lines changed label Mar 25, 2026

Copilot AI reviewed Mar 25, 2026

View reviewed changes

pkg/transport/proxy/transparent/transparent_proxy.go Show resolved Hide resolved

pkg/transport/proxy/httpsse/http_proxy.go Show resolved Hide resolved

yrobla force-pushed the issue-4212 branch from 0057261 to a391738 Compare March 25, 2026 08:13

yrobla requested a review from Copilot March 25, 2026 08:13

github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 25, 2026

Copilot started reviewing on behalf of yrobla March 25, 2026 08:13 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

pkg/transport/proxy/transparent/transparent_proxy.go Show resolved Hide resolved

pkg/transport/proxy/httpsse/http_proxy.go Outdated Show resolved Hide resolved

yrobla force-pushed the issue-4212 branch from a391738 to c49568a Compare March 25, 2026 08:23

yrobla requested a review from Copilot March 25, 2026 08:23

github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 25, 2026

Copilot started reviewing on behalf of yrobla March 25, 2026 08:24 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

pkg/transport/proxy/transparent/transparent_proxy.go Outdated Show resolved Hide resolved

pkg/transport/proxy/httpsse/http_proxy.go Outdated Show resolved Hide resolved

yrobla force-pushed the issue-4212 branch from c49568a to 2c7716c Compare March 25, 2026 09:18

yrobla requested a review from Copilot March 25, 2026 09:18

github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 25, 2026

Copilot started reviewing on behalf of yrobla March 25, 2026 09:19 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

pkg/transport/proxy/httpsse/http_proxy.go Outdated Show resolved Hide resolved

pkg/transport/proxy/transparent/transparent_proxy.go Show resolved Hide resolved

Copilot AI reviewed Mar 25, 2026

View reviewed changes

pkg/transport/proxy/transparent/transparent_proxy.go Outdated Show resolved Hide resolved

pkg/transport/proxy/httpsse/http_proxy_integration_test.go Outdated Show resolved Hide resolved

taskbot and others added 8 commits March 25, 2026 11:31

feat(session): add WithSessionStorage option to HTTP-SSE proxy (RC-12)

15c6185

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(session): add WithSessionStorage option to transparent proxy (RC…

e15b3c9

…-12) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fixes from review

671764c

Merge branch 'main' into issue-4212

41ad948

jerm-dro approved these changes Mar 25, 2026

View reviewed changes

Conversation

yrobla commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of change

Test plan

Changes

Does this introduce a user-facing change?

Special notes for reviewers

Large PR Justification

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Large PR Detected

How to unblock this PR:

Alternative:

Uh oh!

github-actions bot commented Mar 23, 2026

Uh oh!

codecov bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

chatgpt-codex-connector bot commented Mar 23, 2026

💡 Codex Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

yrobla commented Mar 23, 2026 •

edited

Loading

codecov bot commented Mar 23, 2026 •

edited

Loading