Skip to content

fix: log warning when string timeout cannot be parsed in mcp_scripts_parser.go #28797

@github-actions

Description

@github-actions

Problem

In pkg/workflow/mcp_scripts_parser.go at lines 184 and 355, string-typed timeout values are parsed via:

case string:
    // Try to parse string as integer
    _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout)

Both return values — including the error — are silently discarded. If a user writes timeout: "abc" or timeout: "5m" in their mcp-scripts config, the parse fails silently and the timeout stays at the 60-second default, giving no diagnostic feedback.

This is inconsistent with how fmt.Sscanf is used everywhere else in the codebase (>10 call sites in pkg/) where the error is always checked. For instance, time_delta.go:432 does:

_, err := fmt.Sscanf(numStr, "%d", &num)
if err != nil || num <= 0 {
    timeDeltaLog.Printf("Invalid expires time spec number: %s", spec)

Recommendation

Replace the silent fmt.Sscanf with strconv.Atoi and emit a warning on failure, consistent with codebase patterns:

Before (mcp_scripts_parser.go:183):

case string:
    // Try to parse string as integer
    _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout)

After:

case string:
    if n, err := strconv.Atoi(t); err == nil {
        toolConfig.Timeout = n
    } else {
        mcpScriptsLog.Printf("Warning: invalid timeout value %q for tool %q, using default 60s", t, toolName)
    }

Apply the same fix at both occurrences (lines 184 and 355, inside parseMCPScriptsMap and mergeMCPScripts respectively). If issue #28796 is resolved first by extracting the helper, only one site will need updating.

Impact

  • Severity: Medium
  • Affected file: pkg/workflow/mcp_scripts_parser.go (lines 184, 355)
  • Risk: Users with misconfigured string timeouts (e.g. "5m", "two minutes") receive no feedback and silently get the 60-second default.

Validation

  • Add a test case: timeout: "not-a-number" results in default 60s AND emits a warning log
  • Add a test case: timeout: "120" results in 120s timeout
  • All existing timeout tests continue to pass

Estimated Effort: Small

Generated by Sergo - Serena Go Expert · ● 475.4K ·

  • expires on May 4, 2026, 8:44 PM UTC

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions