diff --git a/actions/setup/js/mcp_cli_bridge.cjs b/actions/setup/js/mcp_cli_bridge.cjs index 9d56d00f17c..668938e7e19 100644 --- a/actions/setup/js/mcp_cli_bridge.cjs +++ b/actions/setup/js/mcp_cli_bridge.cjs @@ -45,6 +45,16 @@ const DEFAULT_HTTP_TIMEOUT_MS = 15000; /** Timeout (ms) for tool invocation calls (may be long-running) */ const TOOL_CALL_TIMEOUT_MS = 120000; +/** Default run count for logs MCP calls when count is not provided (mirrors server default) */ +const LOGS_TOOL_DEFAULT_COUNT = 100; +/** Number of runs per timeout minute for logs auto-scaling (mirrors server: ceil(count/40)) */ +const LOGS_TOOL_RUNS_PER_TIMEOUT_MINUTE = 40; +/** Minimum fallback timeout (minutes) for unfiltered logs calls (no workflow_name) */ +const LOGS_TOOL_MIN_TIMEOUT_MINUTES_NO_FILTER = 5; +/** Maximum allowed explicit timeout (minutes) for logs calls to prevent bridge-resource exhaustion */ +const LOGS_TOOL_MAX_EXPLICIT_TIMEOUT_MINUTES = 60; +/** Extra time (ms) to allow response marshalling/transport after tool execution */ +const TOOL_CALL_TIMEOUT_BUFFER_MS = 15000; /** Timeout (ms) for the notifications/initialized handshake step */ const NOTIFY_TIMEOUT_MS = 10000; @@ -331,6 +341,7 @@ async function mcpToolsCall(serverUrl, apiKey, sessionId, toolName, toolArgs, se headers["Mcp-Session-Id"] = sessionId; } + const callTimeoutMs = getToolCallTimeoutMs(toolName, toolArgs); const resp = await httpPostJSON( serverUrl, headers, @@ -340,7 +351,7 @@ async function mcpToolsCall(serverUrl, apiKey, sessionId, toolName, toolArgs, se method: "tools/call", params: { name: toolName, arguments: toolArgs }, }, - TOOL_CALL_TIMEOUT_MS + callTimeoutMs ); const elapsedMs = Date.now() - startMs; @@ -357,6 +368,56 @@ async function mcpToolsCall(serverUrl, apiKey, sessionId, toolName, toolArgs, se return resp; } +/** + * Resolve MCP bridge timeout for a tool call. + * + * The logs tool may legitimately run for multiple minutes. The bridge always + * computes a count-derived floor that mirrors the server's own auto-scaling + * (`ceil(count / 40)` minutes, with a 5-minute minimum when no `workflow_name` + * filter is provided — matching `effectiveMCPLogsToolTimeoutMinutes` in + * `mcp_tools_privileged.go`). This floor applies to both the implicit path and + * any explicit `timeout` argument, so callers that pass a small positive value + * still receive a bridge deadline that is at least as long as the server's own + * expected runtime. Explicit timeouts are capped at + * `LOGS_TOOL_MAX_EXPLICIT_TIMEOUT_MINUTES` to prevent bridge-resource + * exhaustion from adversarial or misconfigured calls. The result is always + * at least `TOOL_CALL_TIMEOUT_MS` (120 s). + * + * @param {string} toolName + * @param {Record} toolArgs + * @returns {number} Timeout in ms. Always at least TOOL_CALL_TIMEOUT_MS (120 s). + */ +function getToolCallTimeoutMs(toolName, toolArgs) { + if (toolName !== "logs") { + return TOOL_CALL_TIMEOUT_MS; + } + + // Compute the count-derived floor that mirrors the server's auto-scaling. + // Reject non-numeric types to avoid implicit coercion of booleans/strings. + const countCandidate = typeof toolArgs?.count === "number" ? toolArgs.count : NaN; + const effectiveCount = Number.isFinite(countCandidate) && countCandidate > 0 ? countCandidate : LOGS_TOOL_DEFAULT_COUNT; + const baseMinutes = Math.ceil(effectiveCount / LOGS_TOOL_RUNS_PER_TIMEOUT_MINUTE); + // Without a workflow filter the GitHub API scans all runs and is substantially + // slower for large repositories — apply the same 5-minute floor as the server. + const floorMinutes = toolArgs?.workflow_name ? baseMinutes : Math.max(LOGS_TOOL_MIN_TIMEOUT_MINUTES_NO_FILTER, baseMinutes); + const floorMs = Math.ceil(floorMinutes * 60 * 1000) + TOOL_CALL_TIMEOUT_BUFFER_MS; + + // Honor an explicit timeout when present. Reject non-numeric types (e.g. + // booleans, strings) so only real numeric values are accepted, and cap at + // LOGS_TOOL_MAX_EXPLICIT_TIMEOUT_MINUTES to bound bridge-resource use. + const timeoutCandidate = typeof toolArgs?.timeout === "number" ? toolArgs.timeout : NaN; + if (Number.isFinite(timeoutCandidate) && timeoutCandidate > 0) { + const clampedMinutes = Math.min(timeoutCandidate, LOGS_TOOL_MAX_EXPLICIT_TIMEOUT_MINUTES); + const explicitMs = Math.ceil(clampedMinutes * 60 * 1000) + TOOL_CALL_TIMEOUT_BUFFER_MS; + // Floor to the count-derived minimum so tiny explicit values do not produce + // a misleadingly short bridge deadline; always at least TOOL_CALL_TIMEOUT_MS. + return Math.max(TOOL_CALL_TIMEOUT_MS, floorMs, explicitMs); + } + + // No valid explicit timeout: use the count-derived floor directly. + return Math.max(TOOL_CALL_TIMEOUT_MS, floorMs); +} + /** * Start periodic MCP ping requests to keep a session alive while a tool call runs. * @@ -1341,5 +1402,6 @@ module.exports = { hasStdinJsonPayload, readStdinSync, ensureSafeOutputsTools, + getToolCallTimeoutMs, main, }; diff --git a/actions/setup/js/mcp_cli_bridge.test.cjs b/actions/setup/js/mcp_cli_bridge.test.cjs index edfddc2fd69..1478efe4b6f 100644 --- a/actions/setup/js/mcp_cli_bridge.test.cjs +++ b/actions/setup/js/mcp_cli_bridge.test.cjs @@ -3,7 +3,7 @@ import fs from "fs"; import os from "os"; import path from "path"; -import { ensureSafeOutputsTools, formatResponse, hasStdinJsonPayload, parseToolArgs, readStdinSync, shouldShowToolHelpForEmptyArgs, showHelp, showToolHelp, writeStdoutAndFlush } from "./mcp_cli_bridge.cjs"; +import { ensureSafeOutputsTools, formatResponse, getToolCallTimeoutMs, hasStdinJsonPayload, parseToolArgs, readStdinSync, shouldShowToolHelpForEmptyArgs, showHelp, showToolHelp, writeStdoutAndFlush } from "./mcp_cli_bridge.cjs"; describe("mcp_cli_bridge.cjs", () => { let originalCore; @@ -214,6 +214,59 @@ describe("mcp_cli_bridge.cjs", () => { }); }); + it("uses default 120s timeout for non-logs tools", () => { + expect(getToolCallTimeoutMs("audit", {})).toBe(120000); + }); + + it("uses a longer timeout for logs calls without explicit timeout (default count=100, no filter)", () => { + // effectiveCount=100, base=ceil(100/40)=3, no workflow_name → max(5,3)=5 minutes + expect(getToolCallTimeoutMs("logs", {})).toBe(315000); + }); + + it("scales logs timeout from count when no explicit timeout is set (count=250, no filter)", () => { + // effectiveCount=250, base=ceil(250/40)=7, no workflow_name → max(5,7)=7 minutes + expect(getToolCallTimeoutMs("logs", { count: 250 })).toBe(435000); + }); + + it("scales logs timeout from count with workflow_name filter (count=250, filtered)", () => { + // effectiveCount=250, base=ceil(250/40)=7, workflow_name present → 7 minutes (no min floor applied) + expect(getToolCallTimeoutMs("logs", { count: 250, workflow_name: "ci" })).toBe(435000); + }); + + it("clamps count-based timeout to global minimum for small filtered counts", () => { + // effectiveCount=40, base=ceil(40/40)=1, workflow_name present → 1 minute → 75000ms < 120000ms → clamped + expect(getToolCallTimeoutMs("logs", { count: 40, workflow_name: "ci" })).toBe(120000); + }); + + it("applies 5-minute no-filter floor for small unfiltered counts", () => { + // effectiveCount=40, base=1, no workflow_name → max(5,1)=5 minutes + expect(getToolCallTimeoutMs("logs", { count: 40 })).toBe(315000); + }); + + it("uses logs timeout argument with bridge buffer when provided", () => { + // timeout=10min, floor=5min (default count=100, no filter) → max(120000, 315000, 615000) = 615000 + expect(getToolCallTimeoutMs("logs", { timeout: 10 })).toBe(615000); + }); + + it("floors small explicit timeout to the count-derived minimum", () => { + // timeout=2.5min → explicit=165000ms; floor=5min → 315000ms; floor wins + expect(getToolCallTimeoutMs("logs", { timeout: 2.5 })).toBe(315000); + }); + + it("caps explicit timeout at LOGS_TOOL_MAX_EXPLICIT_TIMEOUT_MINUTES (60)", () => { + // timeout=999min → clamped to 60min → 3615000ms; floor=315000ms; capped value wins + expect(getToolCallTimeoutMs("logs", { timeout: 999 })).toBe(3615000); + }); + + it("rejects non-numeric timeout types and falls back to count-derived timeout", () => { + // typeof-check rejects strings and booleans even when Number() would accept them + expect(getToolCallTimeoutMs("logs", { timeout: 0 })).toBe(315000); + expect(getToolCallTimeoutMs("logs", { timeout: -5 })).toBe(315000); + expect(getToolCallTimeoutMs("logs", { timeout: "invalid" })).toBe(315000); + expect(getToolCallTimeoutMs("logs", { timeout: "5" })).toBe(315000); + expect(getToolCallTimeoutMs("logs", { timeout: true })).toBe(315000); + }); + it("treats MCP result envelopes with isError=true as errors", async () => { await formatResponse( {