[Repo Assist] fix(mcp): replace hardcoded 30s with defaultConnectTimeout constant, fix <= 0 guard

[github-actions]

🤖 This is a Repo Assist automated pull request.

Summary

Closes #3933.

Two locations in internal/mcp/connection.go used the magic literal 30 * time.Second for the HTTP connect-timeout fallback. A canonical constant config.DefaultConnectTimeout = 30 already exists in internal/config/config_core.go, but importing that package into internal/mcp would pull in init() functions with side effects. Instead, this PR introduces a local unexported constant to be the single source of truth within the package.

It also fixes a latent bug: reconnectSDKTransport guarded with == 0, which left negative values through as-is. The guard is now <= 0, consistent with NewHTTPConnection.

Root cause

Issue #3933 (Duplicate Code Detector) identified:

• NewHTTPConnection (line ~202): if connectTimeout <= 0 { connectTimeout = 30 * time.Second }
• reconnectSDKTransport (line ~386): if timeout == 0 { timeout = 30 * time.Second } — note inconsistent guard

Both hardcode 30 s instead of using a named constant, and the guards differ.

Changes

File	Change
internal/mcp/connection.go	Add const defaultConnectTimeout = 30 * time.Second; replace both magic literals with the constant; fix == 0 → <= 0 guard in reconnectSDKTransport

Trade-offs

• Why a local constant, not importing config? internal/config has init() functions that register flags and load schemas. Importing it into internal/mcp just for one constant would be a heavyweight dependency and could cause ordering surprises. A comment on the constant explicitly documents the sync requirement.
• Behavioural change: A negative connectTimeout passed to reconnectSDKTransport previously slipped through; it now falls back to 30 s. This is the correct behaviour (negative durations are invalid) and consistent with NewHTTPConnection.

Test Status

⚠️ Infrastructure constraint: proxy.golang.org is blocked by the environment firewall, preventing go test from downloading the required Go toolchain (go1.25.0). The change is a pure constant-introduction refactor with no logic change for valid inputs; the == 0 → <= 0 guard fix is covered by the companion PR adding dedicated tests (repo-assist/test-connect-timeout-defaults).

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Repo Assist · ● 6.1M · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@851905c06e905bf362a9f6cc54f912e3df747d55

[Copilot]

Pull request overview

Refactors MCP HTTP connection timeout defaults in internal/mcp to remove duplicated magic literals and aligns reconnect behavior with the existing “non-positive means default” semantics used elsewhere.

Changes:

• Introduce a package-local defaultConnectTimeout constant (30s) to replace hardcoded 30 * time.Second usages without importing internal/config.
• Apply the constant in NewHTTPConnection and reconnectSDKTransport.
• Fix reconnect timeout guard from == 0 to <= 0 to correctly default negative durations.

Show a summary per file

File	Description
internal/mcp/connection.go	Centralizes the 30s connect-timeout fallback and makes reconnect timeout validation consistent (<= 0).

Copilot's findings

Tip

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

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