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
Estimated Effort: Small
Generated by Sergo - Serena Go Expert · ● 475.4K · ◷
Problem
In
pkg/workflow/mcp_scripts_parser.goat lines 184 and 355, string-typed timeout values are parsed via:Both return values — including the error — are silently discarded. If a user writes
timeout: "abc"ortimeout: "5m"in theirmcp-scriptsconfig, the parse fails silently and the timeout stays at the 60-second default, giving no diagnostic feedback.This is inconsistent with how
fmt.Sscanfis used everywhere else in the codebase (>10 call sites inpkg/) where the error is always checked. For instance,time_delta.go:432does:Recommendation
Replace the silent
fmt.Sscanfwithstrconv.Atoiand emit a warning on failure, consistent with codebase patterns:Before (
mcp_scripts_parser.go:183):After:
Apply the same fix at both occurrences (lines 184 and 355, inside
parseMCPScriptsMapandmergeMCPScriptsrespectively). If issue #28796 is resolved first by extracting the helper, only one site will need updating.Impact
pkg/workflow/mcp_scripts_parser.go(lines 184, 355)"5m","two minutes") receive no feedback and silently get the 60-second default.Validation
timeout: "not-a-number"results in default 60s AND emits a warning logtimeout: "120"results in 120s timeoutEstimated Effort: Small