Skip to content

Deduplicate MCP_GATEWAY_SESSION_TIMEOUT env-var lookup into shared getSessionTimeout()#5100

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-session-timeout-fix
May 4, 2026
Merged

Deduplicate MCP_GATEWAY_SESSION_TIMEOUT env-var lookup into shared getSessionTimeout()#5100
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-session-timeout-fix

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 4, 2026

envutil.GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 6*time.Hour) was inlined in transport.go despite routed.go already owning an identical wrapper — two places to update if the env-var key or default ever changes.

Changes

  • routed.go — Rename getRoutedSessionTimeout()getSessionTimeout(); update its doc comment to reflect shared ownership
  • transport.go — Replace inline literal with getSessionTimeout(); drop now-unused envutil and time imports
  • routed_test.go — Update TestRoutedModeSessionTimeout_ReadsEnvVar call sites to the new name; test coverage is unchanged
// Before — transport.go inlined the literal independently
SessionTimeout: envutil.GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 6*time.Hour),

// After — both callers share one function
SessionTimeout: getSessionTimeout(),

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build602771692/b513/launcher.test /tmp/go-build602771692/b513/launcher.test -test.testlogfile=/tmp/go-build602771692/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build602771692/b428/vet.cfg ache/go/1.25.9/x64/src/runtime/cgo /tmp/go-build3583632305/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet go_.�� 64/src/net -I x_amd64/vet --gdwarf-5 ions =0 x_amd64/vet (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build602771692/b495/config.test /tmp/go-build602771692/b495/config.test -test.testlogfile=/tmp/go-build602771692/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build602771692/b365/vet.cfg Ua_e/htfOW-jx_xLgo1.25.9 ache/go/1.25.9/x-c=4 x_amd64/vet 0276810/b172/ go.opentelemetry-atomic (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build602771692/b513/launcher.test /tmp/go-build602771692/b513/launcher.test -test.testlogfile=/tmp/go-build602771692/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build602771692/b428/vet.cfg ache/go/1.25.9/x64/src/runtime/cgo /tmp/go-build3583632305/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet go_.�� 64/src/net -I x_amd64/vet --gdwarf-5 ions =0 x_amd64/vet (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build602771692/b513/launcher.test /tmp/go-build602771692/b513/launcher.test -test.testlogfile=/tmp/go-build602771692/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build602771692/b428/vet.cfg ache/go/1.25.9/x64/src/runtime/cgo /tmp/go-build3583632305/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet go_.�� 64/src/net -I x_amd64/vet --gdwarf-5 ions =0 x_amd64/vet (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build602771692/b522/mcp.test /tmp/go-build602771692/b522/mcp.test -test.testlogfile=/tmp/go-build602771692/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Fix duplicate env-var literal for session timeout Deduplicate MCP_GATEWAY_SESSION_TIMEOUT env-var lookup into shared getSessionTimeout() May 4, 2026
Copilot AI requested a review from lpcox May 4, 2026 17:19
Copilot finished work on behalf of lpcox May 4, 2026 17:19
@lpcox lpcox marked this pull request as ready for review May 4, 2026 17:20
Copilot AI review requested due to automatic review settings May 4, 2026 17:20
Copy link
Copy Markdown
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 removes duplication of the MCP_GATEWAY_SESSION_TIMEOUT env-var lookup by consolidating it into a single package-level helper (getSessionTimeout()), so both routed and unified (transport) server creation paths share the same default and env wiring.

Changes:

  • Renamed getRoutedSessionTimeout() to getSessionTimeout() and updated its comment to reflect shared usage.
  • Updated unified transport server creation to use getSessionTimeout() instead of inlining envutil.GetEnvDuration(..., 6*time.Hour), allowing removal of now-unused imports.
  • Updated routed-mode tests to call the renamed helper.
Show a summary per file
File Description
internal/server/transport.go Switches unified-mode StreamableHTTP SessionTimeout to shared getSessionTimeout() and removes unused imports.
internal/server/routed.go Renames and repurposes the session timeout helper as the shared implementation for both modes.
internal/server/routed_test.go Updates tests to call getSessionTimeout() after the rename.

Copilot's findings

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment thread internal/server/routed.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox lpcox merged commit 056b712 into main May 4, 2026
14 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-session-timeout-fix branch May 4, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code Pattern: Session Timeout Env-Var Literal

3 participants