diff --git a/actions/setup/js/start_mcp_gateway.cjs b/actions/setup/js/start_mcp_gateway.cjs index d9fea7dc3e6..ea637a0d28c 100644 --- a/actions/setup/js/start_mcp_gateway.cjs +++ b/actions/setup/js/start_mcp_gateway.cjs @@ -83,8 +83,10 @@ async function retryGatewayStartup(probe, retryConfig, deadline, label, shouldRe throw err; } delayMs = Math.min(delayMs * retryConfig.backoffMultiplier, retryConfig.maxDelayMs); - const sleepMs = Math.min(delayMs, deadline - nowMs()); - core.warning(`${label}: retrying after attempt ${attempt} in ${sleepMs}ms`); + const sleepMs = Math.min(Math.ceil(delayMs), deadline - nowMs()); + if (attempt === 1 || (attempt & (attempt - 1)) === 0) { + core.warning(`${label}: retrying after attempt ${attempt} in ${sleepMs}ms`); + } await sleep(sleepMs); } } @@ -114,7 +116,7 @@ const MCP_GATEWAY_HEALTH_RETRY_ENV = Object.freeze({ /** Default MCP backend startup timeout the retry budget must outlast. */ const MCP_GATEWAY_BACKEND_STARTUP_TIMEOUT_DEFAULT_MS = 120_000; -/** Upper bound on configurable health-check attempts, to keep the wait finite. */ +/** Upper bound on explicit and automatically sized health-check attempts. */ const MCP_GATEWAY_HEALTH_MAX_ATTEMPTS_LIMIT = 10_000; /** @@ -131,24 +133,25 @@ const MCP_GATEWAY_HEALTH_REGISTRATION_MARGIN_MS = 25_000; * @param {number} defaultValue * @param {NodeJS.ProcessEnv} env * @param {{ integer?: boolean }} [options] - * @returns {number} + * @returns {{ value: number, wasExplicit: boolean }} */ function readPositiveEnvNumber(envName, defaultValue, env, options = {}) { const raw = env[envName]; if (raw === undefined || !raw.trim()) { - return defaultValue; + return { value: defaultValue, wasExplicit: false }; } const parsed = Number(raw.trim()); const isValid = Number.isFinite(parsed) && parsed > 0 && (!options.integer || Number.isSafeInteger(parsed)); if (!isValid) { core.warning(`Ignoring invalid ${envName}='${raw}': expected a positive ${options.integer ? "integer" : "number"}, using ${defaultValue}`); - return defaultValue; + return { value: defaultValue, wasExplicit: false }; } - return parsed; + return { value: parsed, wasExplicit: true }; } /** - * Computes the cumulative startup retry delay before exhausting its attempts. + * Computes a lower bound on the cumulative startup retry delay before exhausting + * its attempts. Actual sleeps round up to avoid Node truncating fractional delays. * The retry loop applies the backoff multiplier before its first sleep, * so the first retry waits initialDelayMs * backoffMultiplier. * @@ -156,13 +159,20 @@ function readPositiveEnvNumber(envName, defaultValue, env, options = {}) { * @returns {number} */ function computeHealthRetryBudgetMs(config) { - let delayMs = config.initialDelayMs; - let budgetMs = 0; - for (let retry = 1; retry < config.maxTotalAttempts; retry += 1) { - delayMs = Math.min(delayMs * config.backoffMultiplier, config.maxDelayMs); - budgetMs += delayMs; + const retries = config.maxTotalAttempts - 1; + if (retries <= 0) { + return 0; + } + if (config.backoffMultiplier === 1) { + return Math.min(config.initialDelayMs, config.maxDelayMs) * retries; } - return budgetMs; + + // Sum the exponential ramp and capped tail without iterating over potentially + // millions of attempts. expm1 preserves precision for multipliers close to 1. + const logMultiplier = Math.log(config.backoffMultiplier); + const rampRetries = Math.min(retries, Math.max(0, Math.ceil(Math.log(config.maxDelayMs / config.initialDelayMs) / logMultiplier) - 1)); + const rampBudgetMs = rampRetries === 0 ? 0 : (config.initialDelayMs * config.backoffMultiplier * Math.expm1(rampRetries * logMultiplier)) / (config.backoffMultiplier - 1); + return rampBudgetMs + (retries - rampRetries) * config.maxDelayMs; } /** @@ -173,11 +183,13 @@ function computeHealthRetryBudgetMs(config) { * @returns {{ maxTotalAttempts: number, initialDelayMs: number, maxDelayMs: number, backoffMultiplier: number, backendStartupTimeoutMs: number, retryBudgetMs: number }} */ function resolveGatewayHealthRetryConfig(env = process.env) { - let maxTotalAttempts = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.maxTotalAttempts, MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.maxTotalAttempts, env, { integer: true }); - const initialDelayMs = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.initialDelayMs, MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.initialDelayMs, env, { integer: true }); - let maxDelayMs = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.maxDelayMs, MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.maxDelayMs, env, { integer: true }); - let backoffMultiplier = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.backoffMultiplier, MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.backoffMultiplier, env); - const backendStartupTimeoutMs = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs, MCP_GATEWAY_BACKEND_STARTUP_TIMEOUT_DEFAULT_MS, env, { integer: true }); + const maxAttemptsOverride = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.maxTotalAttempts, MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.maxTotalAttempts, env, { integer: true }); + let maxTotalAttempts = maxAttemptsOverride.value; + const hasExplicitAttempts = maxAttemptsOverride.wasExplicit; + const initialDelayMs = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.initialDelayMs, MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.initialDelayMs, env, { integer: true }).value; + let maxDelayMs = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.maxDelayMs, MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.maxDelayMs, env, { integer: true }).value; + let backoffMultiplier = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.backoffMultiplier, MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.backoffMultiplier, env).value; + const backendStartupTimeoutMs = readPositiveEnvNumber(MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs, MCP_GATEWAY_BACKEND_STARTUP_TIMEOUT_DEFAULT_MS, env, { integer: true }).value; if (maxTotalAttempts > MCP_GATEWAY_HEALTH_MAX_ATTEMPTS_LIMIT) { core.warning(`${MCP_GATEWAY_HEALTH_RETRY_ENV.maxTotalAttempts}=${maxTotalAttempts} exceeds the supported maximum, using ${MCP_GATEWAY_HEALTH_MAX_ATTEMPTS_LIMIT}`); @@ -199,9 +211,25 @@ function resolveGatewayHealthRetryConfig(env = process.env) { } const config = { maxTotalAttempts, initialDelayMs, maxDelayMs, backoffMultiplier, backendStartupTimeoutMs }; - const retryBudgetMs = computeHealthRetryBudgetMs(config); - const requiredBudgetMs = backendStartupTimeoutMs + MCP_GATEWAY_HEALTH_REGISTRATION_MARGIN_MS; + let retryBudgetMs = computeHealthRetryBudgetMs(config); + if (!hasExplicitAttempts && retryBudgetMs < requiredBudgetMs) { + const firstDelayMs = Math.min(initialDelayMs * backoffMultiplier, maxDelayMs); + let lower = maxTotalAttempts; + // Bound automatic sizing to prevent very frequent polling from exploding. + let upper = MCP_GATEWAY_HEALTH_MAX_ATTEMPTS_LIMIT; + while (lower < upper) { + const middle = lower + Math.floor((upper - lower) / 2); + if (computeHealthRetryBudgetMs({ ...config, maxTotalAttempts: middle }) < requiredBudgetMs) { + lower = middle + 1; + } else { + upper = middle; + } + } + config.maxTotalAttempts = lower; + retryBudgetMs = computeHealthRetryBudgetMs(config); + } + if (retryBudgetMs < requiredBudgetMs) { core.warning( `MCP gateway health retry budget is ${Math.round(retryBudgetMs / 1000)}s, which is below the ${Math.round(requiredBudgetMs / 1000)}s needed for the ${Math.round(backendStartupTimeoutMs / 1000)}s backend startup timeout plus cleanup and registration; the health check may give up before the gateway is ready` @@ -1113,7 +1141,8 @@ async function main() { }, healthRetryConfig, startupDeadline, - "MCP gateway health check" + "MCP gateway health check", + () => isProcessAlive(gatewayPid) ); succeeded = true; } catch { diff --git a/actions/setup/js/start_mcp_gateway.test.cjs b/actions/setup/js/start_mcp_gateway.test.cjs index 40b0c8fd3a4..9414d21b53c 100644 --- a/actions/setup/js/start_mcp_gateway.test.cjs +++ b/actions/setup/js/start_mcp_gateway.test.cjs @@ -35,6 +35,7 @@ import { httpGet, } from "./start_mcp_gateway.cjs"; import { writeMcpConfiguration } from "./mcp_configuration.cjs"; +import { applyGatewayRuntimeTimeouts } from "./tool_timeouts.cjs"; describe("agenticworkflows backend ownership", () => { it.each(["localhost/gh-aw:dev", "alpine:3.24"])("uses the runner identity without widening private artifact modes (%s)", container => { @@ -136,6 +137,13 @@ describe("start_mcp_gateway logging", () => { expect(source).not.toContain("/tmp/gh-aw/mcp-logs/start-gateway.log"); }); + it("stops health polling when the gateway process exits", () => { + const source = fs.readFileSync(new URL("./start_mcp_gateway.cjs", import.meta.url), "utf8"); + const healthCheckStart = source.indexOf('core.info("=== Health Check Progress ===")'); + const healthCheckEnd = source.indexOf("succeeded = true;", healthCheckStart); + expect(source.slice(healthCheckStart, healthCheckEnd)).toContain("() => isProcessAlive(gatewayPid)"); + }); + it("captures the gateway child process stderr outside the artifact directory", () => { const source = fs.readFileSync(new URL("./start_mcp_gateway.cjs", import.meta.url), "utf8"); expect(source).toContain(`stdio: ["pipe", outputFd, stderrFd]`); @@ -190,6 +198,11 @@ describe("start_mcp_gateway logging", () => { }); describe("start_mcp_gateway health timeout", () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + it("allows cleanup and registration time after the backend startup timeout", () => { const config = resolveGatewayHealthRetryConfig({}); @@ -211,6 +224,166 @@ describe("start_mcp_gateway health timeout", () => { expect(config.retryBudgetMs).toBe(computeHealthRetryBudgetMs(config)); }); + it("sizes default health retries for tools.startup-timeout", () => { + const env = { GH_AW_STARTUP_TIMEOUT: "300" }; + const gatewayConfig = { gateway: {} }; + applyGatewayRuntimeTimeouts(gatewayConfig, env); + const warning = vi.spyOn(global.core, "warning").mockImplementation(() => {}); + + const config = resolveGatewayHealthRetryConfig(env); + + expect(gatewayConfig.gateway.startupTimeout).toBe(300); + expect(config.backendStartupTimeoutMs).toBe(300_000); + expect(config.maxTotalAttempts).toBe(327); + expect(config.retryBudgetMs).toBeGreaterThanOrEqual(325_000); + expect(computeHealthRetryBudgetMs({ ...config, maxTotalAttempts: config.maxTotalAttempts - 1 })).toBeLessThan(325_000); + expect(warning).not.toHaveBeenCalled(); + }); + + it.each([{}, { initialDelayMs: "10", maxDelayMs: "100", backoffMultiplier: "1.5" }, { initialDelayMs: "250", maxDelayMs: "1000", backoffMultiplier: "1" }, { initialDelayMs: "2000", maxDelayMs: "500", backoffMultiplier: "0.5" }])( + "covers the startup window with normalized custom retry delays: %j", + overrides => { + const env = { [MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs]: "300000" }; + for (const [key, value] of Object.entries(overrides)) { + env[MCP_GATEWAY_HEALTH_RETRY_ENV[key]] = value; + } + + const config = resolveGatewayHealthRetryConfig(env); + + expect(config.retryBudgetMs).toBeGreaterThanOrEqual(325_000); + expect(config.maxTotalAttempts).toBeGreaterThanOrEqual(MCP_GATEWAY_HEALTH_RETRY_DEFAULTS.maxTotalAttempts); + } + ); + + it.each(["", " ", "not-a-number", "0", "-1", "1.5"])("automatically sizes attempts when the override is unusable: %j", override => { + const config = resolveGatewayHealthRetryConfig({ + [MCP_GATEWAY_HEALTH_RETRY_ENV.maxTotalAttempts]: override, + [MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs]: "300000", + }); + + expect(config.retryBudgetMs).toBeGreaterThanOrEqual(325_000); + }); + + it.each(["10", "150", "330"])("preserves an explicit attempt limit: %s", attempts => { + const warning = vi.spyOn(global.core, "warning").mockImplementation(() => {}); + const config = resolveGatewayHealthRetryConfig({ + [MCP_GATEWAY_HEALTH_RETRY_ENV.maxTotalAttempts]: attempts, + [MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs]: "300000", + }); + + expect(config.maxTotalAttempts).toBe(Number(attempts)); + if (Number(attempts) < 327) { + expect(warning).toHaveBeenCalledWith(expect.stringContaining("health check may give up")); + } else { + expect(warning).not.toHaveBeenCalled(); + } + }); + + it("caps automatic attempts when short delays cannot cover the startup window", () => { + const warning = vi.spyOn(global.core, "warning").mockImplementation(() => {}); + const config = resolveGatewayHealthRetryConfig({ + [MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs]: "300000", + [MCP_GATEWAY_HEALTH_RETRY_ENV.initialDelayMs]: "1", + [MCP_GATEWAY_HEALTH_RETRY_ENV.maxDelayMs]: "1", + [MCP_GATEWAY_HEALTH_RETRY_ENV.backoffMultiplier]: "1", + }); + + expect(config.maxTotalAttempts).toBe(MCP_GATEWAY_HEALTH_MAX_ATTEMPTS_LIMIT); + expect(config.retryBudgetMs).toBe(9_999); + expect(warning).toHaveBeenCalledWith(expect.stringContaining("health retry budget is")); + }); + + it.each(["1", "1.0000000000000002", "2"])("bounds automatically sized attempts for large startup windows (%s)", backoffMultiplier => { + const warning = vi.spyOn(global.core, "warning").mockImplementation(() => {}); + const config = resolveGatewayHealthRetryConfig({ + [MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs]: "2147483000", + [MCP_GATEWAY_HEALTH_RETRY_ENV.initialDelayMs]: "1", + [MCP_GATEWAY_HEALTH_RETRY_ENV.backoffMultiplier]: backoffMultiplier, + }); + + expect(config.maxTotalAttempts).toBe(MCP_GATEWAY_HEALTH_MAX_ATTEMPTS_LIMIT); + expect(config.retryBudgetMs).toBeLessThan(2147483000 + MCP_GATEWAY_HEALTH_REGISTRATION_MARGIN_MS); + expect(warning).toHaveBeenCalledWith(expect.stringContaining("health retry budget is")); + }); + + it("stops retrying when the gateway process is no longer alive", async () => { + vi.useFakeTimers(); + vi.setSystemTime(0); + const probe = vi.fn(async () => { + throw new Error("connection refused"); + }); + const isGatewayAlive = vi.fn(() => false); + const config = { maxTotalAttempts: 10, initialDelayMs: 1, maxDelayMs: 1, backoffMultiplier: 1 }; + + await expect(retryGatewayStartup(probe, config, 1000, "health", isGatewayAlive)).rejects.toThrow("connection refused"); + + expect(probe).toHaveBeenCalledTimes(1); + expect(isGatewayAlive).toHaveBeenCalledTimes(1); + expect(Date.now()).toBe(0); + }); + + it("limits retry warnings to exponentially spaced progress reports", async () => { + vi.useFakeTimers(); + vi.setSystemTime(0); + const warning = vi.spyOn(global.core, "warning").mockImplementation(() => {}); + const probe = vi.fn(async () => { + throw new Error("backend unavailable"); + }); + const config = { maxTotalAttempts: 20, initialDelayMs: 1, maxDelayMs: 1, backoffMultiplier: 1 }; + const wait = retryGatewayStartup(probe, config, 1000, "health"); + const rejection = expect(wait).rejects.toThrow("backend unavailable"); + + await vi.runAllTimersAsync(); + await rejection; + + expect(probe).toHaveBeenCalledTimes(20); + expect(warning).toHaveBeenCalledTimes(5); + }); + + it.each([ + { backendStartupTimeoutMs: 300_000, readyMs: 300_000, backoffMultiplier: "2" }, + { backendStartupTimeoutMs: 1000, readyMs: 25_000, initialDelayMs: "3", backoffMultiplier: "1.00001" }, + ])("keeps polling through backend startup and registration: %j", async ({ backendStartupTimeoutMs, readyMs, initialDelayMs, backoffMultiplier }) => { + vi.useFakeTimers({ loopLimit: 50_000 }); + vi.setSystemTime(0); + vi.spyOn(global.core, "warning").mockImplementation(() => {}); + const config = resolveGatewayHealthRetryConfig({ + [MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs]: String(backendStartupTimeoutMs), + [MCP_GATEWAY_HEALTH_RETRY_ENV.initialDelayMs]: initialDelayMs, + [MCP_GATEWAY_HEALTH_RETRY_ENV.backoffMultiplier]: backoffMultiplier, + }); + const deadline = backendStartupTimeoutMs + MCP_GATEWAY_HEALTH_REGISTRATION_MARGIN_MS; + const probe = vi.fn(async () => { + if (Date.now() < readyMs) throw new Error("backend still starting"); + }); + const wait = retryGatewayStartup(probe, config, deadline, "health"); + + await vi.runAllTimersAsync(); + await wait; + + expect(probe.mock.calls.length).toBeGreaterThan(150); + expect(Date.now()).toBeGreaterThanOrEqual(readyMs); + expect(Date.now()).toBeLessThan(deadline); + }); + + it("stops an unavailable gateway at the shared startup deadline", async () => { + vi.useFakeTimers(); + vi.setSystemTime(0); + vi.spyOn(global.core, "warning").mockImplementation(() => {}); + const config = resolveGatewayHealthRetryConfig({ [MCP_GATEWAY_HEALTH_RETRY_ENV.backendStartupTimeoutMs]: "300000" }); + const probe = vi.fn(async () => { + throw new Error("backend unavailable"); + }); + const wait = retryGatewayStartup(probe, config, 325_000, "health"); + const rejection = expect(wait).rejects.toThrow("startup deadline exceeded"); + + await vi.runAllTimersAsync(); + await rejection; + + expect(probe.mock.calls.length).toBeGreaterThan(150); + expect(Date.now()).toBe(325_000); + }); + it("falls back to defaults for invalid overrides", () => { const config = resolveGatewayHealthRetryConfig({ [MCP_GATEWAY_HEALTH_RETRY_ENV.maxTotalAttempts]: "not-a-number", @@ -251,6 +424,23 @@ describe("start_mcp_gateway health timeout", () => { // The retry loop multiplies before its first sleep: 500ms, then 1000ms capped. expect(computeHealthRetryBudgetMs({ maxTotalAttempts: 3, initialDelayMs: 250, maxDelayMs: 1000, backoffMultiplier: 2 })).toBe(1500); }); + + it.each([ + { maxTotalAttempts: 1, initialDelayMs: 250, maxDelayMs: 1000, backoffMultiplier: 2 }, + { maxTotalAttempts: 500, initialDelayMs: 100, maxDelayMs: 400, backoffMultiplier: 1.5 }, + { maxTotalAttempts: 500, initialDelayMs: 250, maxDelayMs: 1000, backoffMultiplier: 1 }, + { maxTotalAttempts: 500, initialDelayMs: 1, maxDelayMs: 1000, backoffMultiplier: 1.0000000000000002 }, + { maxTotalAttempts: 500, initialDelayMs: 250, maxDelayMs: 1000, backoffMultiplier: Number.MAX_VALUE }, + ])("matches the unrounded backoff schedule: %j", config => { + let delayMs = config.initialDelayMs; + let budgetMs = 0; + for (let attempt = 1; attempt < config.maxTotalAttempts; attempt += 1) { + delayMs = Math.min(delayMs * config.backoffMultiplier, config.maxDelayMs); + budgetMs += delayMs; + } + + expect(computeHealthRetryBudgetMs(config)).toBeCloseTo(budgetMs, 5); + }); }); describe("start_mcp_gateway startup diagnostics", () => { diff --git a/docs/src/content/docs/reference/environment-variables.md b/docs/src/content/docs/reference/environment-variables.md index 7f2f5933aae..7f97d14a7fd 100644 --- a/docs/src/content/docs/reference/environment-variables.md +++ b/docs/src/content/docs/reference/environment-variables.md @@ -221,7 +221,7 @@ The MCP gateway startup step polls the gateway `/health` endpoint with exponenti | Variable | Default | Applies to | | --- | --- | --- | -| `GH_AW_MCP_GATEWAY_HEALTH_MAX_ATTEMPTS` | `150` | Total health-check attempts, including the first one. | +| `GH_AW_MCP_GATEWAY_HEALTH_MAX_ATTEMPTS` | At least `150`, automatically sized up to `10000` | Total health-check attempts, including the first one. | | `GH_AW_MCP_GATEWAY_HEALTH_INITIAL_DELAY_MS` | `250` | Delay the backoff starts from. | | `GH_AW_MCP_GATEWAY_HEALTH_MAX_DELAY_MS` | `1000` | Cap applied to every retry delay. | | `GH_AW_MCP_GATEWAY_HEALTH_BACKOFF_MULTIPLIER` | `2` | Multiplier applied to the delay before every retry. | @@ -231,7 +231,7 @@ These values are cross-validated at startup: - A backoff multiplier below `1` is raised to `1` (constant delay). - A delay cap below the initial delay is raised to the initial delay. -- The cumulative retry delay must cover the backend startup timeout plus 25 seconds for backend cleanup and final server registration; otherwise a warning is emitted that the health check may give up before the gateway is ready. +- Unless a valid attempt count is explicitly configured, the attempt count increases so the cumulative retry delay covers the backend startup timeout (including `tools.startup-timeout`) plus 25 seconds for backend cleanup and final server registration. Explicit and automatically sized attempt counts are capped at `10000`; an insufficient retry budget emits a warning. Health and readiness probes share the startup deadline, so increasing attempts does not extend that deadline. ## CLI Configuration Variables