From c8c6e5a990b2c94e933bb65220df7a1a2a35f3eb Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Wed, 26 Aug 2026 09:34:48 -0700 Subject: [PATCH] fix(qa): align repeated-request proof with provider timeout (#129851) --- .../mock-openai/mock-openai-contracts.ts | 1 + .../src/providers/mock-openai/server.ts | 26 +++++-- ...eway-repeated-request-recovery.e2e.test.ts | 67 +++++++++++++------ 3 files changed, 67 insertions(+), 27 deletions(-) diff --git a/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts b/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts index 99f6661dbd82..8da1b37d89a5 100644 --- a/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts +++ b/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts @@ -371,6 +371,7 @@ export type MockScenarioState = { subagentFanoutCompletedWorkers: Set<"alpha" | "beta">; subagentFanoutPhase: number; subagentHandoffSpawned: boolean; + repeatedRequestRecoveryAttempts: number; toolLoopReadAttempts: number; }; diff --git a/extensions/qa-lab/src/providers/mock-openai/server.ts b/extensions/qa-lab/src/providers/mock-openai/server.ts index 983b189ba19d..c1c44a42f80e 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.ts @@ -296,9 +296,12 @@ const QA_FAILED_TOOL_TERMINAL_RECOVERY_PROMPT_RE = /failed tool terminal recover const QA_TELEGRAM_VISIBLE_PARTIAL_FAILURE_PROMPT_RE = /telegram visible partial failure qa check/i; const QA_TELEGRAM_UNSENT_FAILURE_PROMPT_RE = /telegram unsent failure qa check/i; const QA_TELEGRAM_VISIBLE_PARTIAL_FAILURE_MARKER = "TELEGRAM-VISIBLE-PARTIAL-BEFORE-FAILURE"; -// Keep each real provider request active long enough for retries to span the -// unchanged five-minute recovery bound while remaining below first-byte timeout. -const QA_REPEATED_REQUEST_RESPONSE_PAUSE_MS = 110_000; +// Complete ordinary retries inside their diagnostic request allowance, then +// leave the fifth request active so recovery can honor both the cumulative +// no-progress bound and the current request's own allowance. +const QA_REPEATED_REQUEST_RESPONSE_PAUSE_MS = 80_000; +const QA_REPEATED_REQUEST_STALLED_RESPONSE_PAUSE_MS = 180_000; +const QA_REPEATED_REQUEST_STALL_ATTEMPT = 5; function isStreamingToolProgressContinuationText(text: string) { const trimmed = text.trim(); @@ -2509,6 +2512,7 @@ export async function startQaMockOpenAiServer(params?: { subagentFanoutCompletedWorkers: new Set<"alpha" | "beta">(), subagentFanoutPhase: 0, subagentHandoffSpawned: false, + repeatedRequestRecoveryAttempts: 0, toolLoopReadAttempts: 0, }; scenarioStates.set(key, state); @@ -2698,6 +2702,12 @@ export async function startQaMockOpenAiServer(params?: { toolOutputCallId: extractToolOutputCallId(input) || undefined, ...(extractToolOutputStructuredError(input) ? { toolOutputStructuredError: true } : {}), }); + const repeatedRequestRecovery = + QA_REPEATED_REQUEST_RECOVERY_PROMPT_RE.test(allInputText) && + !QA_REPEATED_REQUEST_QUEUED_REPLY_PROMPT_RE.test(prompt); + if (repeatedRequestRecovery) { + scenarioState.repeatedRequestRecoveryAttempts += 1; + } return { events, model, @@ -2714,9 +2724,13 @@ export async function startQaMockOpenAiServer(params?: { ...(QA_FINAL_ONLY_MARKER_STREAMING_PROMPT_RE.test(allInputText) ? { previewPauseMs: finalOnlyMarkerPauseMs } : {}), - ...(QA_REPEATED_REQUEST_RECOVERY_PROMPT_RE.test(allInputText) && - !QA_REPEATED_REQUEST_QUEUED_REPLY_PROMPT_RE.test(prompt) - ? { responsePauseMs: QA_REPEATED_REQUEST_RESPONSE_PAUSE_MS } + ...(repeatedRequestRecovery + ? { + responsePauseMs: + scenarioState.repeatedRequestRecoveryAttempts >= QA_REPEATED_REQUEST_STALL_ATTEMPT + ? QA_REPEATED_REQUEST_STALLED_RESPONSE_PAUSE_MS + : QA_REPEATED_REQUEST_RESPONSE_PAUSE_MS, + } : {}), }; }; diff --git a/test/e2e/qa-lab/runtime/gateway-repeated-request-recovery.e2e.test.ts b/test/e2e/qa-lab/runtime/gateway-repeated-request-recovery.e2e.test.ts index 88317cdb8504..084ce59b68db 100644 --- a/test/e2e/qa-lab/runtime/gateway-repeated-request-recovery.e2e.test.ts +++ b/test/e2e/qa-lab/runtime/gateway-repeated-request-recovery.e2e.test.ts @@ -11,6 +11,8 @@ type StabilityEvent = { reason?: unknown; outcome?: unknown; ageMs?: unknown; + durationMs?: unknown; + failureKind?: unknown; queueDepth?: unknown; source?: unknown; }; @@ -56,6 +58,7 @@ const QUEUED_PROMPT = const QUEUED_REPLY_MARKER = "GATEWAY_REPEATED_REQUEST_QUEUED_OK"; const RECOVERY_REASON = "repeated_model_requests_without_progress"; const PRODUCTION_RECOVERY_BOUND_MS = 360_000; +const MODEL_REQUEST_ALLOWANCE_SECONDS = 90; const RECOVERY_PROGRESS_INTERVAL_MS = 60_000; const HISTORY_RETRY_TIMEOUT_MS = 60_000; const HISTORY_RETRY_INTERVAL_MS = 250; @@ -276,9 +279,9 @@ async function readFailureEvidence(params: { return JSON.stringify({ stability, requests, gatewayLogs }); } -describe("Gateway repeated-request recovery", () => { +describe("Gateway repeated-request provider timeout", () => { it( - "aborts the real stalled owner once and releases one queued followup", + "lets the provider timeout terminate the stalled attempt before draining one queued followup", { timeout: 510_000 }, async () => { harness = await startQaLiveLaneGateway({ @@ -294,7 +297,27 @@ describe("Gateway repeated-request recovery", () => { }, transportBaseUrl: "http://127.0.0.1", controlUiEnabled: false, - mutateConfig: (config) => ({ ...config, diagnostics: { enabled: true } }), + mutateConfig: (config) => { + const models = config.models; + const provider = models?.providers?.["mock-openai"]; + if (!models || !provider) { + throw new Error("mock-openai provider config unavailable"); + } + return { + ...config, + diagnostics: { enabled: true }, + models: { + ...models, + providers: { + ...models.providers, + "mock-openai": { + ...provider, + timeoutSeconds: MODEL_REQUEST_ALLOWANCE_SECONDS, + }, + }, + }, + }; + }, }); const { gateway } = harness; @@ -338,7 +361,14 @@ describe("Gateway repeated-request recovery", () => { const events = await waitForStability( gateway, baselineSeq, - (records) => records.some((event) => event.type === "session.recovery.completed"), + (records) => + records.some( + (event) => + event.type === "model.call.error" && + event.failureKind === "timeout" && + typeof event.durationMs === "number" && + event.durationMs >= MODEL_REQUEST_ALLOWANCE_SECONDS * 1_000, + ), 350_000, ); const stalled = events.filter( @@ -352,15 +382,11 @@ describe("Gateway repeated-request recovery", () => { expect(stalled).toHaveLength(1); expect(stalled[0]?.ageMs).toEqual(expect.any(Number)); expect(stalled[0]?.ageMs as number).toBeGreaterThanOrEqual(PRODUCTION_RECOVERY_BOUND_MS); - expect(requested).toEqual([ - expect.objectContaining({ action: "abort", reason: RECOVERY_REASON }), - ]); - expect(completed).toEqual([ - expect.objectContaining({ action: "abort_embedded_run", outcome: "aborted" }), - ]); + expect(requested).toEqual([]); + expect(completed).toEqual([]); expect( events.filter((event) => event.type === "model.call.started").length, - ).toBeGreaterThanOrEqual(4); + ).toBeGreaterThanOrEqual(5); const activeTerminal = (await gateway.call( "agent.wait", @@ -369,13 +395,6 @@ describe("Gateway repeated-request recovery", () => { )) as GatewayChatRun; expect(activeTerminal.status).not.toBe("ok"); - const queuedTerminal = (await gateway.call( - "agent.wait", - { runId: queued.runId, timeoutMs: 30_000 }, - { timeoutMs: 35_000 }, - )) as GatewayChatRun; - expect(queuedTerminal.status).toBe("ok"); - const history = await waitForQueuedReply(gateway, sessionKey).catch( async (error: unknown) => { const evidence = await readFailureEvidence({ @@ -387,6 +406,12 @@ describe("Gateway repeated-request recovery", () => { }, ); expect(historyContainsQueuedReply(history)).toBe(true); + const queuedTerminal = (await gateway.call( + "agent.wait", + { runId: queued.runId, timeoutMs: 30_000 }, + { timeoutMs: 35_000 }, + )) as GatewayChatRun; + expect(queuedTerminal.status).toBe("ok"); const mockBaseUrl = harness?.mock?.baseUrl; if (!mockBaseUrl) { throw new Error("mock provider request evidence unavailable"); @@ -394,7 +419,7 @@ describe("Gateway repeated-request recovery", () => { const requests = await readClassifiedMockRequests(mockBaseUrl); expect( requests.filter((request) => request.prompt === "recovery").length, - ).toBeGreaterThanOrEqual(4); + ).toBeGreaterThanOrEqual(5); expect(requests.filter((request) => request.prompt === "queued")).toEqual([ expect.objectContaining({ outcome: "success" }), ]); @@ -402,10 +427,10 @@ describe("Gateway repeated-request recovery", () => { const finalEvents = (await readStability(gateway, baselineSeq)).events ?? []; expect( finalEvents.filter((event) => event.type === "session.recovery.requested"), - ).toHaveLength(1); + ).toHaveLength(0); expect( finalEvents.filter((event) => event.type === "session.recovery.completed"), - ).toHaveLength(1); + ).toHaveLength(0); }, ); });