From 3faa392628926db8ced18bf1c6d98157c2691bfa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 03:54:25 +0000 Subject: [PATCH 1/5] Initial plan From 763e71aa9d1526a53925385409f0c3b078ecf481 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 04:03:19 +0000 Subject: [PATCH 2/5] Fix Copilot proxy listener readiness and first ECONNREFUSED retry path Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/awf_reflect.cjs | 73 +++++++++++++++++++++++ actions/setup/js/awf_reflect.test.cjs | 71 ++++++++++++++++++++++ actions/setup/js/copilot_harness.cjs | 37 ++++++++++++ actions/setup/js/copilot_harness.test.cjs | 12 ++++ 4 files changed, 193 insertions(+) diff --git a/actions/setup/js/awf_reflect.cjs b/actions/setup/js/awf_reflect.cjs index 9e57aca4be6..b2f895b2faf 100644 --- a/actions/setup/js/awf_reflect.cjs +++ b/actions/setup/js/awf_reflect.cjs @@ -19,6 +19,7 @@ require("./shim.cjs"); const fs = require("fs"); const path = require("path"); +const net = require("net"); const { withRetry, sleep } = require("./error_recovery.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); @@ -33,6 +34,10 @@ const AWF_REFLECT_OUTPUT_PATH = "/tmp/gh-aw/sandbox/firewall/awf-reflect.json"; const AWF_REFLECT_TIMEOUT_MS = 60000; // Milliseconds to wait for each models_url fallback fetch (shorter than the main reflect timeout). const AWF_MODELS_URL_TIMEOUT_MS = 3000; +// Milliseconds to wait for an api-proxy provider listener to accept a real TCP connection. +const AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS = 15000; +// Delay between provider-listener readiness probes. +const AWF_PROVIDER_LISTENER_READY_RETRY_MS = 250; // Maximum attempts for models_url fallback fetches when the proxy is not yet ready. const AWF_MODELS_URL_MAX_ATTEMPTS = 5; // Base delay between models_url fallback retries. Uses exponential backoff. @@ -323,6 +328,7 @@ async function fetchAWFReflect(options) { status: res.status, }; } + /** @type {any} */ const reflectData = await res.json(); // Attempt to fill in null models for configured providers by fetching directly @@ -364,6 +370,70 @@ async function fetchAWFReflect(options) { } } +/** + * Wait until a provider listener (e.g. http://api-proxy:10002) accepts a real TCP + * connection, or time out. + * + * This guards against startup races where /reflect is available but a per-provider + * listener has not yet bound/started accepting connections. + * + * @param {{ + * baseUrl: string, + * timeoutMs?: number, + * retryDelayMs?: number, + * logger?: (msg: string) => void, + * connectImpl?: typeof net.connect, + * }} options + * @returns {Promise<{ ok: true } | { ok: false, reason: "invalid_base_url" | "timeout", error: string }>} + */ +async function waitForProviderListenerReady(options) { + const logger = options?.logger ?? DEFAULT_REFLECT_LOGGER; + const timeoutMs = options?.timeoutMs ?? AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS; + const retryDelayMs = options?.retryDelayMs ?? AWF_PROVIDER_LISTENER_READY_RETRY_MS; + const connectImpl = options?.connectImpl ?? net.connect; + const baseUrl = String(options?.baseUrl ?? "").trim(); + if (!baseUrl) { + return { ok: false, reason: "invalid_base_url", error: "baseUrl is empty" }; + } + + let parsed; + try { + parsed = new URL(baseUrl); + } catch { + return { ok: false, reason: "invalid_base_url", error: `invalid baseUrl: ${baseUrl}` }; + } + const host = parsed.hostname; + const port = parsed.port ? Number.parseInt(parsed.port, 10) : parsed.protocol === "https:" ? 443 : 80; + if (!host || !Number.isFinite(port) || port <= 0) { + return { ok: false, reason: "invalid_base_url", error: `baseUrl missing host/port: ${baseUrl}` }; + } + + logger(`awf-reflect: waiting for provider listener readiness at ${host}:${port} (timeout=${timeoutMs}ms)`); + const startedAt = Date.now(); + let lastError = "connection not ready"; + while (Date.now() - startedAt < timeoutMs) { + const ready = await new Promise(resolve => { + const socket = connectImpl({ host, port }); + socket.once("connect", () => { + socket.end(); + resolve(true); + }); + socket.once("error", err => { + lastError = getErrorMessage(err); + socket.destroy(); + resolve(false); + }); + }); + if (ready) { + logger(`awf-reflect: provider listener is accepting connections at ${host}:${port}`); + return { ok: true }; + } + await sleep(retryDelayMs); + } + logger(`awf-reflect: provider listener readiness timed out for ${host}:${port} (${lastError})`); + return { ok: false, reason: "timeout", error: lastError }; +} + /** * Returns true when the model name matches well-known Anthropic naming patterns: * "claude-*" prefix, or "-opus", "-haiku", or "-sonnet" as a segment or suffix. @@ -793,11 +863,14 @@ if (typeof module !== "undefined" && module.exports) { AWF_MODELS_URL_MAX_ATTEMPTS, AWF_MODELS_URL_RETRY_BASE_MS, AWF_MODELS_URL_RETRY_MAX_MS, + AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS, + AWF_PROVIDER_LISTENER_READY_RETRY_MS, GEMINI_MODEL_NAME_PREFIX, enrichReflectModels, extractModelIds, fetchAWFReflect, fetchModelsFromUrl, + waitForProviderListenerReady, getCatalogModelEntry, inferProviderTypeForModel, inferWireApiForModel, diff --git a/actions/setup/js/awf_reflect.test.cjs b/actions/setup/js/awf_reflect.test.cjs index 657dceb70e4..67535a22ed1 100644 --- a/actions/setup/js/awf_reflect.test.cjs +++ b/actions/setup/js/awf_reflect.test.cjs @@ -13,11 +13,14 @@ const { AWF_MODELS_URL_MAX_ATTEMPTS, AWF_MODELS_URL_RETRY_BASE_MS, AWF_MODELS_URL_RETRY_MAX_MS, + AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS, + AWF_PROVIDER_LISTENER_READY_RETRY_MS, GEMINI_MODEL_NAME_PREFIX, enrichReflectModels, extractModelIds, fetchAWFReflect, fetchModelsFromUrl, + waitForProviderListenerReady, getCatalogModelEntry, inferProviderTypeForModel, inferWireApiForModel, @@ -36,10 +39,78 @@ describe("awf_reflect.cjs", () => { expect(AWF_MODELS_URL_MAX_ATTEMPTS).toBe(5); expect(AWF_MODELS_URL_RETRY_BASE_MS).toBe(250); expect(AWF_MODELS_URL_RETRY_MAX_MS).toBe(2000); + expect(AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS).toBe(15000); + expect(AWF_PROVIDER_LISTENER_READY_RETRY_MS).toBe(250); expect(GEMINI_MODEL_NAME_PREFIX).toBe("models/"); }); }); + describe("waitForProviderListenerReady", () => { + it("returns ok when listener accepts a connection", async () => { + const probingConnect = vi.fn().mockImplementation(() => { + const listeners = {}; + queueMicrotask(() => listeners.connect && listeners.connect()); + return { + once(event, cb) { + listeners[event] = cb; + return this; + }, + end() {}, + destroy() {}, + }; + }); + + const result = await waitForProviderListenerReady({ + baseUrl: "http://api-proxy:10002", + timeoutMs: 500, + retryDelayMs: 1, + connectImpl: probingConnect, + logger: () => {}, + }); + expect(result).toEqual({ ok: true }); + expect(probingConnect).toHaveBeenCalled(); + }); + + it("returns timeout when listener keeps refusing connections", async () => { + const probingConnect = vi.fn().mockImplementation(() => { + const listeners = {}; + queueMicrotask(() => listeners.error && listeners.error(new Error("connect ECONNREFUSED"))); + return { + once(event, cb) { + listeners[event] = cb; + return this; + }, + end() {}, + destroy() {}, + }; + }); + + const result = await waitForProviderListenerReady({ + baseUrl: "http://api-proxy:10002", + timeoutMs: 20, + retryDelayMs: 1, + connectImpl: probingConnect, + logger: () => {}, + }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe("timeout"); + expect(result.error).toContain("ECONNREFUSED"); + } + }); + + it("returns invalid_base_url for malformed baseUrl", async () => { + const result = await waitForProviderListenerReady({ + baseUrl: "not a url", + logger: () => {}, + }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe("invalid_base_url"); + } + }); + }); + describe("extractModelIds", () => { it("returns null for null input", () => { expect(extractModelIds(null)).toBeNull(); diff --git a/actions/setup/js/copilot_harness.cjs b/actions/setup/js/copilot_harness.cjs index b485c7b3243..24d7cbb6aa0 100644 --- a/actions/setup/js/copilot_harness.cjs +++ b/actions/setup/js/copilot_harness.cjs @@ -65,6 +65,8 @@ const { AWF_REFLECT_TIMEOUT_MS, AWF_MODELS_URL_TIMEOUT_MS, GEMINI_MODEL_NAME_PREFIX, + AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS, + waitForProviderListenerReady, enrichReflectModels, extractModelIds, fetchAWFReflect, @@ -162,6 +164,8 @@ const SDK_SESSION_IDLE_TIMEOUT_PATTERN = /Timeout after \d+ms waiting for sessio // avoid false positives from any process that logs "Gateway shutdown initiated" // as plain text. const MCP_GATEWAY_SHUTDOWN_PATTERN = /"message"\s*:\s*"Gateway shutdown initiated"/; +const CONNECTION_REFUSED_ERROR_PATTERN = /connection refused|ECONNREFUSED/i; +const FIRST_CONNECTION_REFUSED_RETRY_DELAY_MS = 1000; // Pattern to detect null-type tool_call error that poisons conversation history. // Matches the Copilot API 400 error: @@ -488,6 +492,15 @@ function isMCPGatewayShutdownError(output) { return MCP_GATEWAY_SHUTDOWN_PATTERN.test(output); } +/** + * Determine whether output contains a connection-refused signal. + * @param {string} output + * @returns {boolean} + */ +function isConnectionRefusedError(output) { + return CONNECTION_REFUSED_ERROR_PATTERN.test(output); +} + /** * Extract a compact tail preview from combined process output for failure logs. * @param {string} output @@ -1017,6 +1030,20 @@ async function main() { } log(`copilot-sdk driver mode: multi-provider config resolved (${multiProvider.providers.length} providers, ${multiProvider.models.length} models, model=${resolvedModel})`); + + const uniqueProviderBaseUrls = [...new Set(multiProvider.providers.map(provider => String(provider.baseUrl || "").trim()).filter(Boolean))]; + for (const providerBaseUrlToProbe of uniqueProviderBaseUrls) { + const readiness = await waitForProviderListenerReady({ + baseUrl: providerBaseUrlToProbe, + timeoutMs: AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS, + logger: log, + }); + if (!readiness.ok) { + emitInfrastructureIncomplete(`api-proxy provider listener was not ready at ${providerBaseUrlToProbe} before first Copilot SDK request (${readiness.error}).`); + log(`copilot-sdk driver mode: provider listener readiness probe failed for ${providerBaseUrlToProbe}: ${readiness.error}`); + process.exit(1); + } + } } // Merge SDK env additions into the child process env only when the SDK helper @@ -1179,6 +1206,7 @@ async function main() { const isNullTypeToolCall = isNullTypeToolCallError(result.output); const isSDKSessionIdleTimeout = isSDKSessionIdleTimeoutError(result.output); const isMCPGatewayShutdown = isMCPGatewayShutdownError(result.output); + const isConnectionRefused = isConnectionRefusedError(result.output); const permissionDeniedCount = countPermissionDeniedIssues(result.output); const hasNumerousPermissionDenied = hasNumerousPermissionDeniedIssues(result.output); const nonRetryableGuard = detectNonRetryableHarnessGuard(result.output); @@ -1215,6 +1243,7 @@ async function main() { ` isNullTypeToolCallError=${isNullTypeToolCall}` + ` isSDKSessionIdleTimeoutError=${isSDKSessionIdleTimeout}` + ` isMCPGatewayShutdownError=${isMCPGatewayShutdown}` + + ` isConnectionRefusedError=${isConnectionRefused}` + ` isAuthError=${isAuthErr}` + ` isAuthenticationFailedError=${isAuthenticationFailed}` + ` permissionDeniedCount=${permissionDeniedCount}` + @@ -1363,6 +1392,13 @@ async function main() { } } + if (attempt === 0 && isConnectionRefused && attempt < maxRetries) { + useContinueOnRetry = false; + continueDisabledPermanently = true; + log(`attempt ${attempt + 1}: connection refused on first request path — retrying as fresh run with short backoff (${FIRST_CONNECTION_REFUSED_RETRY_DELAY_MS}ms) (attempt ${attempt + 2}/${maxRetries + 1})`); + return { action: "retry", nextDelayMs: FIRST_CONNECTION_REFUSED_RETRY_DELAY_MS }; + } + if (isStartupRetryEligible && isStartupNoOutputRetryCandidate(result) && scheduledExit2Retries < MAX_SCHEDULED_EXIT2_RETRIES && attempt < maxRetries) { scheduledExit2Retries += 1; scheduledExit2RetryAttempted = true; @@ -1468,6 +1504,7 @@ if (typeof module !== "undefined" && module.exports) { AGENTIC_ENGINE_TIMEOUT_PATTERN, buildMissingToolPermissionIssuePayload, isAuthenticationFailedError, + isConnectionRefusedError, isMCPGatewayShutdownError, isSDKSessionIdleTimeoutError, startCopilotSDKServer, diff --git a/actions/setup/js/copilot_harness.test.cjs b/actions/setup/js/copilot_harness.test.cjs index dacbe470e76..26ea2133a17 100644 --- a/actions/setup/js/copilot_harness.test.cjs +++ b/actions/setup/js/copilot_harness.test.cjs @@ -35,6 +35,7 @@ const { AGENTIC_ENGINE_TIMEOUT_PATTERN, isDetectionPhase, isAuthenticationFailedError, + isConnectionRefusedError, isRetryableProxyAuthenticationFailure, isMCPGatewayShutdownError, isModelAvailableInReflectData, @@ -125,6 +126,17 @@ describe("copilot_harness.cjs", () => { expect(CAPI_ERROR_400_PATTERN.test(errorOutput)).toBe(true); }); + describe("connection refused detection", () => { + it("detects ECONNREFUSED signals in SDK driver output", () => { + const output = "Failed native model HTTP request: error sending request for url (http://api-proxy:10002/chat/completions): " + "client error (Connect): tcp connect error: Connection refused (os error 111) [ECONNREFUSED]"; + expect(isConnectionRefusedError(output)).toBe(true); + }); + + it("does not match unrelated output", () => { + expect(isConnectionRefusedError("CAPIError: 400 bad request")).toBe(false); + }); + }); + describe("CAPI quota-exceeded detection pattern", () => { it("matches the observed CAPIError 429 quota exceeded error", () => { expect(isCAPIQuotaExceededError("CAPIError: 429 429 quota exceeded")).toBe(true); From 0b6bf813c5767fa90a79282d5de5529fddf1f772 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 04:08:26 +0000 Subject: [PATCH 3/5] Harden Copilot provider listener readiness probe behavior Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/awf_reflect.cjs | 25 +++++++++++++++++++++++-- actions/setup/js/awf_reflect.test.cjs | 12 +++++------- actions/setup/js/copilot_harness.cjs | 3 +-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/actions/setup/js/awf_reflect.cjs b/actions/setup/js/awf_reflect.cjs index b2f895b2faf..fcc1c4d3308 100644 --- a/actions/setup/js/awf_reflect.cjs +++ b/actions/setup/js/awf_reflect.cjs @@ -38,6 +38,8 @@ const AWF_MODELS_URL_TIMEOUT_MS = 3000; const AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS = 15000; // Delay between provider-listener readiness probes. const AWF_PROVIDER_LISTENER_READY_RETRY_MS = 250; +// Per-attempt connect timeout while probing provider listener readiness. +const AWF_PROVIDER_LISTENER_READY_PROBE_TIMEOUT_MS = 2000; // Maximum attempts for models_url fallback fetches when the proxy is not yet ready. const AWF_MODELS_URL_MAX_ATTEMPTS = 5; // Base delay between models_url fallback retries. Uses exponential backoff. @@ -381,6 +383,7 @@ async function fetchAWFReflect(options) { * baseUrl: string, * timeoutMs?: number, * retryDelayMs?: number, + * perAttemptTimeoutMs?: number, * logger?: (msg: string) => void, * connectImpl?: typeof net.connect, * }} options @@ -390,6 +393,8 @@ async function waitForProviderListenerReady(options) { const logger = options?.logger ?? DEFAULT_REFLECT_LOGGER; const timeoutMs = options?.timeoutMs ?? AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS; const retryDelayMs = options?.retryDelayMs ?? AWF_PROVIDER_LISTENER_READY_RETRY_MS; + const perAttemptTimeoutMsRaw = options?.perAttemptTimeoutMs ?? AWF_PROVIDER_LISTENER_READY_PROBE_TIMEOUT_MS; + const perAttemptTimeoutMs = Number.isFinite(perAttemptTimeoutMsRaw) && perAttemptTimeoutMsRaw > 0 ? perAttemptTimeoutMsRaw : AWF_PROVIDER_LISTENER_READY_PROBE_TIMEOUT_MS; const connectImpl = options?.connectImpl ?? net.connect; const baseUrl = String(options?.baseUrl ?? "").trim(); if (!baseUrl) { @@ -414,14 +419,29 @@ async function waitForProviderListenerReady(options) { while (Date.now() - startedAt < timeoutMs) { const ready = await new Promise(resolve => { const socket = connectImpl({ host, port }); + let settled = false; + const settle = value => { + if (settled) return; + settled = true; + resolve(value); + }; + const timer = setTimeout(() => { + clear(); + lastError = `connect attempt timed out after ${perAttemptTimeoutMs}ms`; + socket.destroy(); + settle(false); + }, perAttemptTimeoutMs); + const clear = () => clearTimeout(timer); socket.once("connect", () => { + clear(); socket.end(); - resolve(true); + settle(true); }); socket.once("error", err => { + clear(); lastError = getErrorMessage(err); socket.destroy(); - resolve(false); + settle(false); }); }); if (ready) { @@ -865,6 +885,7 @@ if (typeof module !== "undefined" && module.exports) { AWF_MODELS_URL_RETRY_MAX_MS, AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS, AWF_PROVIDER_LISTENER_READY_RETRY_MS, + AWF_PROVIDER_LISTENER_READY_PROBE_TIMEOUT_MS, GEMINI_MODEL_NAME_PREFIX, enrichReflectModels, extractModelIds, diff --git a/actions/setup/js/awf_reflect.test.cjs b/actions/setup/js/awf_reflect.test.cjs index 67535a22ed1..3972fc5436f 100644 --- a/actions/setup/js/awf_reflect.test.cjs +++ b/actions/setup/js/awf_reflect.test.cjs @@ -15,6 +15,7 @@ const { AWF_MODELS_URL_RETRY_MAX_MS, AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS, AWF_PROVIDER_LISTENER_READY_RETRY_MS, + AWF_PROVIDER_LISTENER_READY_PROBE_TIMEOUT_MS, GEMINI_MODEL_NAME_PREFIX, enrichReflectModels, extractModelIds, @@ -41,6 +42,7 @@ describe("awf_reflect.cjs", () => { expect(AWF_MODELS_URL_RETRY_MAX_MS).toBe(2000); expect(AWF_PROVIDER_LISTENER_READY_TIMEOUT_MS).toBe(15000); expect(AWF_PROVIDER_LISTENER_READY_RETRY_MS).toBe(250); + expect(AWF_PROVIDER_LISTENER_READY_PROBE_TIMEOUT_MS).toBe(2000); expect(GEMINI_MODEL_NAME_PREFIX).toBe("models/"); }); }); @@ -93,10 +95,8 @@ describe("awf_reflect.cjs", () => { logger: () => {}, }); expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("timeout"); - expect(result.error).toContain("ECONNREFUSED"); - } + expect(result.reason).toBe("timeout"); + expect(result.error).toContain("ECONNREFUSED"); }); it("returns invalid_base_url for malformed baseUrl", async () => { @@ -105,9 +105,7 @@ describe("awf_reflect.cjs", () => { logger: () => {}, }); expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("invalid_base_url"); - } + expect(result.reason).toBe("invalid_base_url"); }); }); diff --git a/actions/setup/js/copilot_harness.cjs b/actions/setup/js/copilot_harness.cjs index 24d7cbb6aa0..1c3d876dfb6 100644 --- a/actions/setup/js/copilot_harness.cjs +++ b/actions/setup/js/copilot_harness.cjs @@ -1392,9 +1392,8 @@ async function main() { } } - if (attempt === 0 && isConnectionRefused && attempt < maxRetries) { + if (attempt === 0 && isConnectionRefused && maxRetries > 0) { useContinueOnRetry = false; - continueDisabledPermanently = true; log(`attempt ${attempt + 1}: connection refused on first request path — retrying as fresh run with short backoff (${FIRST_CONNECTION_REFUSED_RETRY_DELAY_MS}ms) (attempt ${attempt + 2}/${maxRetries + 1})`); return { action: "retry", nextDelayMs: FIRST_CONNECTION_REFUSED_RETRY_DELAY_MS }; } From 12d84418b0c749e463336b83eb2edd311e097477 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 06:37:14 +0000 Subject: [PATCH 4/5] Address review feedback: TLS-aware readiness, noop ordering, deadline budget, tests Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- actions/setup/js/awf_reflect.cjs | 34 ++++-- actions/setup/js/awf_reflect.test.cjs | 120 ++++++++++++++++++++++ actions/setup/js/copilot_harness.cjs | 27 +++-- actions/setup/js/copilot_harness.test.cjs | 39 +++++++ 4 files changed, 205 insertions(+), 15 deletions(-) diff --git a/actions/setup/js/awf_reflect.cjs b/actions/setup/js/awf_reflect.cjs index fcc1c4d3308..e3ce9e8f1f6 100644 --- a/actions/setup/js/awf_reflect.cjs +++ b/actions/setup/js/awf_reflect.cjs @@ -20,6 +20,7 @@ require("./shim.cjs"); const fs = require("fs"); const path = require("path"); const net = require("net"); +const tls = require("tls"); const { withRetry, sleep } = require("./error_recovery.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); @@ -379,13 +380,17 @@ async function fetchAWFReflect(options) { * This guards against startup races where /reflect is available but a per-provider * listener has not yet bound/started accepting connections. * + * For "https:" baseUrls, the probe performs a full TLS handshake (via `tls.connect`) rather + * than a bare TCP connect, since a raw TCP accept can succeed well before the TLS listener + * is actually able to negotiate a secure session and serve requests. + * * @param {{ * baseUrl: string, * timeoutMs?: number, * retryDelayMs?: number, * perAttemptTimeoutMs?: number, * logger?: (msg: string) => void, - * connectImpl?: typeof net.connect, + * connectImpl?: (opts: { host: string, port: number }) => import("net").Socket, * }} options * @returns {Promise<{ ok: true } | { ok: false, reason: "invalid_base_url" | "timeout", error: string }>} */ @@ -395,7 +400,6 @@ async function waitForProviderListenerReady(options) { const retryDelayMs = options?.retryDelayMs ?? AWF_PROVIDER_LISTENER_READY_RETRY_MS; const perAttemptTimeoutMsRaw = options?.perAttemptTimeoutMs ?? AWF_PROVIDER_LISTENER_READY_PROBE_TIMEOUT_MS; const perAttemptTimeoutMs = Number.isFinite(perAttemptTimeoutMsRaw) && perAttemptTimeoutMsRaw > 0 ? perAttemptTimeoutMsRaw : AWF_PROVIDER_LISTENER_READY_PROBE_TIMEOUT_MS; - const connectImpl = options?.connectImpl ?? net.connect; const baseUrl = String(options?.baseUrl ?? "").trim(); if (!baseUrl) { return { ok: false, reason: "invalid_base_url", error: "baseUrl is empty" }; @@ -412,11 +416,19 @@ async function waitForProviderListenerReady(options) { if (!host || !Number.isFinite(port) || port <= 0) { return { ok: false, reason: "invalid_base_url", error: `baseUrl missing host/port: ${baseUrl}` }; } + // For https:// providers, a bare TCP accept does not prove the listener can complete a TLS + // handshake. Probe with tls.connect and wait for "secureConnect" so the readiness gate lines + // up with the actual failure mode (handshake/startup errors), not just an open port. + const isHttps = parsed.protocol === "https:"; + const readyEvent = isHttps ? "secureConnect" : "connect"; + const connectImpl = options?.connectImpl ?? (isHttps ? opts => tls.connect({ ...opts, servername: opts.host }) : opts => net.connect(opts)); logger(`awf-reflect: waiting for provider listener readiness at ${host}:${port} (timeout=${timeoutMs}ms)`); const startedAt = Date.now(); let lastError = "connection not ready"; while (Date.now() - startedAt < timeoutMs) { + const remainingBudgetMs = timeoutMs - (Date.now() - startedAt); + const attemptTimeoutMs = Math.max(1, Math.min(perAttemptTimeoutMs, remainingBudgetMs)); const ready = await new Promise(resolve => { const socket = connectImpl({ host, port }); let settled = false; @@ -427,14 +439,18 @@ async function waitForProviderListenerReady(options) { }; const timer = setTimeout(() => { clear(); - lastError = `connect attempt timed out after ${perAttemptTimeoutMs}ms`; + lastError = `connect attempt timed out after ${attemptTimeoutMs}ms`; socket.destroy(); settle(false); - }, perAttemptTimeoutMs); + }, attemptTimeoutMs); const clear = () => clearTimeout(timer); - socket.once("connect", () => { + socket.once(readyEvent, () => { clear(); - socket.end(); + // Remove the error listener before tearing down the socket: a successful handshake has + // already settled this probe as ready, and a late/trailing error (e.g. an abrupt RST) + // must not overwrite lastError or otherwise affect the already-settled result. + socket.removeAllListeners("error"); + socket.destroy(); settle(true); }); socket.once("error", err => { @@ -448,7 +464,11 @@ async function waitForProviderListenerReady(options) { logger(`awf-reflect: provider listener is accepting connections at ${host}:${port}`); return { ok: true }; } - await sleep(retryDelayMs); + const remainingAfterAttemptMs = timeoutMs - (Date.now() - startedAt); + if (remainingAfterAttemptMs <= 0) { + break; + } + await sleep(Math.min(retryDelayMs, remainingAfterAttemptMs)); } logger(`awf-reflect: provider listener readiness timed out for ${host}:${port} (${lastError})`); return { ok: false, reason: "timeout", error: lastError }; diff --git a/actions/setup/js/awf_reflect.test.cjs b/actions/setup/js/awf_reflect.test.cjs index 3972fc5436f..e690c94eeaa 100644 --- a/actions/setup/js/awf_reflect.test.cjs +++ b/actions/setup/js/awf_reflect.test.cjs @@ -57,6 +57,9 @@ describe("awf_reflect.cjs", () => { listeners[event] = cb; return this; }, + removeAllListeners() { + return this; + }, end() {}, destroy() {}, }; @@ -107,6 +110,123 @@ describe("awf_reflect.cjs", () => { expect(result.ok).toBe(false); expect(result.reason).toBe("invalid_base_url"); }); + + it("returns timeout when the per-attempt timer fires (hung connect)", async () => { + const probingConnect = vi.fn().mockImplementation(() => { + // Never fire "connect" or "error" — simulates a hung connection attempt. + return { + once() { + return this; + }, + end() {}, + destroy() {}, + removeAllListeners() { + return this; + }, + }; + }); + + const result = await waitForProviderListenerReady({ + baseUrl: "http://api-proxy:10002", + timeoutMs: 50, + retryDelayMs: 1, + perAttemptTimeoutMs: 5, + connectImpl: probingConnect, + logger: () => {}, + }); + expect(result.ok).toBe(false); + expect(result.reason).toBe("timeout"); + expect(result.error).toContain("timed out after"); + }); + + it("uses a TLS secureConnect handshake for https:// baseUrls", async () => { + const probingConnect = vi.fn().mockImplementation(() => { + const listeners = {}; + queueMicrotask(() => listeners.secureConnect && listeners.secureConnect()); + return { + once(event, cb) { + listeners[event] = cb; + return this; + }, + removeAllListeners() { + return this; + }, + end() {}, + destroy() {}, + }; + }); + + const result = await waitForProviderListenerReady({ + baseUrl: "https://api-proxy:10443", + timeoutMs: 500, + retryDelayMs: 1, + connectImpl: probingConnect, + logger: () => {}, + }); + expect(result).toEqual({ ok: true }); + }); + + it("does not report ready on a bare TCP connect for https:// baseUrls", async () => { + const probingConnect = vi.fn().mockImplementation(() => { + const listeners = {}; + // Only fires "connect" (bare TCP), never "secureConnect" (TLS handshake complete). + queueMicrotask(() => listeners.connect && listeners.connect()); + return { + once(event, cb) { + listeners[event] = cb; + return this; + }, + removeAllListeners() { + return this; + }, + end() {}, + destroy() {}, + }; + }); + + const result = await waitForProviderListenerReady({ + baseUrl: "https://api-proxy:10443", + timeoutMs: 20, + retryDelayMs: 1, + perAttemptTimeoutMs: 5, + connectImpl: probingConnect, + logger: () => {}, + }); + expect(result.ok).toBe(false); + expect(result.reason).toBe("timeout"); + }); + + it("clears a late error after a successful connect without affecting the result", async () => { + const probingConnect = vi.fn().mockImplementation(() => { + const listeners = {}; + queueMicrotask(() => listeners.connect && listeners.connect()); + return { + once(event, cb) { + listeners[event] = cb; + return this; + }, + removeAllListeners(event) { + delete listeners[event]; + return this; + }, + end() {}, + destroy() { + // Simulate a trailing error emitted after destroy(); it must be a no-op + // since the "error" listener was removed on successful connect. + if (listeners.error) listeners.error(new Error("late ECONNRESET")); + }, + }; + }); + + const result = await waitForProviderListenerReady({ + baseUrl: "http://api-proxy:10002", + timeoutMs: 500, + retryDelayMs: 1, + connectImpl: probingConnect, + logger: () => {}, + }); + expect(result).toEqual({ ok: true }); + }); }); describe("extractModelIds", () => { diff --git a/actions/setup/js/copilot_harness.cjs b/actions/setup/js/copilot_harness.cjs index 1c3d876dfb6..8139b771f17 100644 --- a/actions/setup/js/copilot_harness.cjs +++ b/actions/setup/js/copilot_harness.cjs @@ -994,6 +994,17 @@ async function main() { applyCopilotModelAliasResolution({ awfReflectData, logger: log }); applyCopilotWireAPI({ modelsJson: loadModelsJson(), logger: log }); + // Pre-flight: skip the agent entirely when a noop has already been written by a prior step. + // A noop indicates the work is complete or there is nothing to do — starting the agent + // (and, in SDK mode, probing provider listener readiness below) would be wasteful and + // potentially harmful. This must run before the provider-listener readiness gate so a + // legitimate noop exit is never turned into an infrastructure-incomplete failure by an + // unrelated listener being unavailable. + const safeOutputsPath = process.env.GH_AW_SAFE_OUTPUTS || ""; + if (shouldSkipForNoopSafeOutputs({ safeOutputsPath, hasNoopInSafeOutputs, log })) { + process.exit(0); + } + // Resolve BYOK provider from live reflect data (SDK mode only). // Multi-provider BYOK is the only supported mode — fail immediately if the // provider cannot be resolved so retries are not wasted on a misconfigured environment. @@ -1032,6 +1043,9 @@ async function main() { log(`copilot-sdk driver mode: multi-provider config resolved (${multiProvider.providers.length} providers, ${multiProvider.models.length} models, model=${resolvedModel})`); const uniqueProviderBaseUrls = [...new Set(multiProvider.providers.map(provider => String(provider.baseUrl || "").trim()).filter(Boolean))]; + if (uniqueProviderBaseUrls.length === 0) { + log("copilot-sdk driver mode: no provider baseUrls to probe — skipping listener readiness check"); + } for (const providerBaseUrlToProbe of uniqueProviderBaseUrls) { const readiness = await waitForProviderListenerReady({ baseUrl: providerBaseUrlToProbe, @@ -1070,14 +1084,6 @@ async function main() { }); const childEnv = Object.keys(sdkChildEnv).length > 0 ? { ...process.env, ...sdkChildEnv } : undefined; - // Pre-flight: skip the agent entirely when a noop has already been written by a prior step. - // A noop indicates the work is complete or there is nothing to do — starting the agent - // would be wasteful and potentially harmful. - const safeOutputsPath = process.env.GH_AW_SAFE_OUTPUTS || ""; - if (shouldSkipForNoopSafeOutputs({ safeOutputsPath, hasNoopInSafeOutputs, log })) { - process.exit(0); - } - let delay = initialDelayMs; let lastExitCode = 1; let lastHasOutput = false; @@ -1392,6 +1398,11 @@ async function main() { } } + // The listener readiness probe above ensures the api-proxy provider listener is + // accepting connections before attempt 0 is sent. A ECONNREFUSED here means the + // provider died in the narrow window between the probe and the first request — retry + // once as a fresh run. Later attempts (attempt > 0) fall through to the generic retry + // handling below instead of taking this one-shot path. if (attempt === 0 && isConnectionRefused && maxRetries > 0) { useContinueOnRetry = false; log(`attempt ${attempt + 1}: connection refused on first request path — retrying as fresh run with short backoff (${FIRST_CONNECTION_REFUSED_RETRY_DELAY_MS}ms) (attempt ${attempt + 2}/${maxRetries + 1})`); diff --git a/actions/setup/js/copilot_harness.test.cjs b/actions/setup/js/copilot_harness.test.cjs index 26ea2133a17..1c6b560b7c1 100644 --- a/actions/setup/js/copilot_harness.test.cjs +++ b/actions/setup/js/copilot_harness.test.cjs @@ -1788,6 +1788,45 @@ describe("copilot_harness.cjs", () => { }); }); + describe("connection-refused retries once on first attempt", () => { + // Inline the same decision as the driver's ECONNREFUSED handling: retry once as a fresh + // run with a short backoff only on the very first attempt, and only when a retry budget + // remains. Later attempts (attempt > 0) and an exhausted retry budget must not take this + // one-shot path. + const FIRST_CONNECTION_REFUSED_RETRY_DELAY_MS = 1000; + + /** + * @param {boolean} isConnectionRefused + * @param {number} attempt + * @param {number} maxRetries + * @returns {{ action: "retry" | "continue", nextDelayMs?: number }} + */ + function decideConnectionRefusedRetry(isConnectionRefused, attempt, maxRetries) { + if (attempt === 0 && isConnectionRefused && maxRetries > 0) { + return { action: "retry", nextDelayMs: FIRST_CONNECTION_REFUSED_RETRY_DELAY_MS }; + } + return { action: "continue" }; + } + + it("retries once as a fresh run with a 1s delay on the first attempt", () => { + const decision = decideConnectionRefusedRetry(true, 0, 3); + expect(decision).toEqual({ action: "retry", nextDelayMs: 1000 }); + }); + + it("does not take this path on later attempts", () => { + expect(decideConnectionRefusedRetry(true, 1, 3)).toEqual({ action: "continue" }); + expect(decideConnectionRefusedRetry(true, 2, 3)).toEqual({ action: "continue" }); + }); + + it("does not take this path when the retry budget is zero", () => { + expect(decideConnectionRefusedRetry(true, 0, 0)).toEqual({ action: "continue" }); + }); + + it("does not take this path when the error is not a connection refusal", () => { + expect(decideConnectionRefusedRetry(false, 0, 3)).toEqual({ action: "continue" }); + }); + }); + describe("permanent --continue disable guard", () => { // Inline retry logic to verify that once continueDisabledPermanently is set, // subsequent partial-execution retries never re-enable --continue. From ddb0b3888641eb0a58b7a97b71099a8ce4aa9d6f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 06:38:24 +0000 Subject: [PATCH 5/5] Clarify connectImpl/readyEvent contract in JSDoc per code review Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- actions/setup/js/awf_reflect.cjs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/actions/setup/js/awf_reflect.cjs b/actions/setup/js/awf_reflect.cjs index e3ce9e8f1f6..f9bc34e6ef6 100644 --- a/actions/setup/js/awf_reflect.cjs +++ b/actions/setup/js/awf_reflect.cjs @@ -391,7 +391,11 @@ async function fetchAWFReflect(options) { * perAttemptTimeoutMs?: number, * logger?: (msg: string) => void, * connectImpl?: (opts: { host: string, port: number }) => import("net").Socket, - * }} options + * }} options - `connectImpl`, when provided, overrides the default connect implementation for + * both http:// and https:// baseUrls (test-only hook). The readiness event awaited is still + * derived from the baseUrl's protocol: for `https:` baseUrls the returned socket must emit + * `"secureConnect"` (not `"connect"`) to be treated as ready, matching the real `tls.connect` + * behavior; for `http:` baseUrls it must emit `"connect"`. * @returns {Promise<{ ok: true } | { ok: false, reason: "invalid_base_url" | "timeout", error: string }>} */ async function waitForProviderListenerReady(options) {