diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index b06237fac9..eb72e78e57 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -371,6 +371,7 @@ import { responsesJsonToSseStream } from "../responses-json-events"; import { guardTerminalEventStream } from "./terminal-guard"; import { emptyCompletionRetryEnabled, + emptyCompletionNotice, observeEmptyCompletion, guardEmptyCompletionEventStream, } from "./empty-completion-guard"; @@ -5271,10 +5272,7 @@ async function handleResponsesInner( // result (#2472). Retrying by default would re-send a turn that may already have had // billable side effects, so the honest default is observability, not recovery. : observeEmptyCompletion(eventSource, () => { - console.warn( - `[opencodex] ${route.providerName}/${route.modelId} completed with no output text ` - + "and no tool call. Set \"emptyCompletionRetry\": true to retry such turns once.", - ); + console.warn(emptyCompletionNotice(route.providerName, route.modelId)); }); const sseStream = bridgeToResponsesSSE( guardedSource, parsed._responseModelId ?? parsed.modelId, toolNsMap, freeformToolNames, toolSearchToolNames, diff --git a/src/server/responses/empty-completion-guard.ts b/src/server/responses/empty-completion-guard.ts index 8d9d7dae5b..352d4a9eaf 100644 --- a/src/server/responses/empty-completion-guard.ts +++ b/src/server/responses/empty-completion-guard.ts @@ -1,4 +1,5 @@ import type { AdapterEvent, OcxConfig, OcxUsage } from "../../types"; +import { sanitizeLogMetadataString } from "../../lib/redact"; /** * Empty-completion guard for Responses turns (port of codex-router's @@ -21,6 +22,21 @@ import type { AdapterEvent, OcxConfig, OcxUsage } from "../../types"; */ export const EMPTY_COMPLETION_RETRY_ENV = "OCX_EMPTY_COMPLETION_RETRY"; +/** + * The observability notice for a turn that ended empty with the retry guard off. + * + * Both labels are caller-supplied: the request names its provider and model. Interpolated raw, + * a model name carrying newlines or terminal escapes writes additional lines into whatever + * reads this warning, so a caller could forge log records it never produced. Both are reduced + * to bounded single-line metadata first. + */ +export function emptyCompletionNotice(providerName: unknown, modelId: unknown): string { + const provider = sanitizeLogMetadataString(providerName) ?? "unknown"; + const model = sanitizeLogMetadataString(modelId) ?? "unknown"; + return `[opencodex] ${provider}/${model} completed with no output text and no tool call. ` + + "Set \"emptyCompletionRetry\": true to retry such turns once."; +} + /** Retained pre-content events are bounded independently by count and encoded size. */ export const EMPTY_COMPLETION_MAX_BUFFERED_EVENTS = 1_024; export const EMPTY_COMPLETION_MAX_BUFFERED_BYTES = 1_048_576; diff --git a/tests/empty-completion-guard.test.ts b/tests/empty-completion-guard.test.ts index 773042fb82..18d3a32f5f 100644 --- a/tests/empty-completion-guard.test.ts +++ b/tests/empty-completion-guard.test.ts @@ -3,6 +3,7 @@ import { EMPTY_COMPLETION_RETRY_ENV, EMPTY_COMPLETION_RETRY_FAILED_CODE, emptyCompletionRetryEnabled, + emptyCompletionNotice, guardEmptyCompletionEventStream, isContentEvent, observeEmptyCompletion, @@ -443,5 +444,28 @@ describe("#2472 an empty turn is observable even when the retry guard is off", ( const { empties } = await drain([]); expect(empties).toBe(1); }); -}); + test("the notice cannot be forged through the caller-supplied provider or model label", () => { + // Both labels come from the request. Interpolated raw, a model name carrying newlines or + // terminal escapes writes extra lines into whatever reads this warning, so a caller could + // fabricate log records it never produced. + const notice = emptyCompletionNotice( + "fixture", + "model\r\n[opencodex] forged: injected\u001b[31m", + ); + + expect(notice).not.toContain("\n"); + expect(notice).not.toContain("\r"); + expect(notice).not.toContain("\u001b"); + expect(notice).toContain("completed with no output text and no tool call"); + // The forged text may survive as inert characters; what must not survive is its ability to + // become a separate record, so the notice stays exactly one line. + expect(notice.split(/\r|\n|\u2028|\u2029/)).toHaveLength(1); + }); + + test("the notice still names an ordinary route and degrades to a stated placeholder", () => { + expect(emptyCompletionNotice("fixture", "gpt-5.4")).toContain("fixture/gpt-5.4"); + // An unusable label must not silently vanish into an empty slot in the sentence. + expect(emptyCompletionNotice(undefined, "")).toContain("unknown/unknown"); + }); +});