mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 21:32:20 +03:00
fix(responses-continuation): chain off the effective post-reconstruction input, not the pre-reconstruction client bytes (#12641)
Validado em lote numa worktree combinada com os 3 PRs desta leva sobre o tip de `release/v3.8.51`: os três boardaram sem conflito, `typecheck:core` limpo e **22/22** nos arquivos de teste que trazem. O crescimento de `src/sse/handlers/chat.ts` (2450 → 2454) é do #12641 e vai num PR de rebaseline próprio. Obrigado, @hartmark.
This commit is contained in:
@@ -28,7 +28,12 @@ export type RequestPipelinePayloads = {
|
||||
|
||||
type RequestLogger = {
|
||||
sessionPath: null;
|
||||
logClientRawRequest: (endpoint: unknown, body: unknown, headers?: HeaderInput) => void;
|
||||
logClientRawRequest: (
|
||||
endpoint: unknown,
|
||||
body: unknown,
|
||||
headers?: HeaderInput,
|
||||
effectiveInput?: unknown
|
||||
) => void;
|
||||
logRouteDecision: (decision: unknown) => void;
|
||||
logOpenAIRequest: (body: unknown) => void;
|
||||
logTargetRequest: (url: unknown, headers: HeaderInput, body: unknown) => void;
|
||||
@@ -392,12 +397,26 @@ export async function createRequestLogger(
|
||||
return {
|
||||
sessionPath: null,
|
||||
|
||||
logClientRawRequest(endpoint, body, headers = {}) {
|
||||
logClientRawRequest(endpoint, body, headers = {}, effectiveInput) {
|
||||
payloads.clientRawRequest = {
|
||||
timestamp: new Date().toISOString(),
|
||||
endpoint,
|
||||
headers: maskSensitiveHeaders(headers),
|
||||
body: cloneBoundedForLog(body),
|
||||
// The actual `input` this request dispatched with, captured AFTER
|
||||
// OmniRoute's own previous_response_id reconstruction (see
|
||||
// src/sse/handlers/chat.ts) -- `body` above is deliberately the
|
||||
// pre-reconstruction raw client bytes (captureDeferredClientRawBody's
|
||||
// whole point) and is NOT what got sent for a continued turn.
|
||||
// resolvePreviousResponseState must chain off this field, not
|
||||
// `body.input`: reading the raw pre-reconstruction input for a
|
||||
// request that was itself a continuation compounds into progressively
|
||||
// truncated history a few hops deep (live incident 2026-09-03,
|
||||
// manifested as a malformed request with no leading system/user
|
||||
// message rejected by the upstream provider).
|
||||
...(effectiveInput !== undefined
|
||||
? { effectiveInput: cloneBoundedForLog(effectiveInput) }
|
||||
: {}),
|
||||
};
|
||||
},
|
||||
|
||||
|
||||
@@ -81,7 +81,8 @@ export function resolvePreviousResponseState(
|
||||
const { artifact, state } = readCallArtifact(row.artifact_relpath);
|
||||
if (state !== "ready" || !artifact?.pipeline) return null;
|
||||
|
||||
const clientRawRequest = artifact.pipeline.clientRawRequest as { body?: unknown } | undefined;
|
||||
const clientRawRequest = artifact.pipeline.clientRawRequest as
|
||||
{ body?: unknown; effectiveInput?: unknown } | undefined;
|
||||
const clientResponse = artifact.pipeline.clientResponse as
|
||||
{ output?: unknown; summary?: { output?: unknown } } | undefined;
|
||||
|
||||
@@ -94,7 +95,22 @@ export function resolvePreviousResponseState(
|
||||
// unconditionally unresolvable for every translate-mode/auto-routed
|
||||
// connection (previous_response_not_found on every attempt, regardless of
|
||||
// whether the id was real and the artifact was otherwise 'ready').
|
||||
const input = isPlainRecord(clientRawRequest?.body) ? clientRawRequest.body.input : undefined;
|
||||
//
|
||||
// effectiveInput first, body.input as a compat fallback for artifacts
|
||||
// logged before this field existed: `body` is captureDeferredClientRawBody's
|
||||
// deliberately pre-reconstruction snapshot of the raw client bytes. For a
|
||||
// turn that was ITSELF a continuation, that's just the client's own trimmed
|
||||
// delta, not the full input that actually dispatched -- chaining off it
|
||||
// compounds into a progressively truncated reconstruction a few hops deep
|
||||
// (live incident 2026-09-03: a malformed request with no leading
|
||||
// system/user message, rejected by the upstream provider). effectiveInput
|
||||
// is captured AFTER reconstruction runs (chat.ts) and is what this function
|
||||
// must chain off so a multi-hop continuation stays accurate.
|
||||
const input = Array.isArray(clientRawRequest?.effectiveInput)
|
||||
? clientRawRequest.effectiveInput
|
||||
: isPlainRecord(clientRawRequest?.body)
|
||||
? clientRawRequest.body.input
|
||||
: undefined;
|
||||
// A streaming clientResponse is clientPayloadCollector.build()'s output, which
|
||||
// always nests the caller's summary under `.summary` (see
|
||||
// createStructuredSSECollector in streamPayloadCollector.ts) -- a non-streaming
|
||||
|
||||
@@ -105,10 +105,16 @@ interface ClientRawRequestLike {
|
||||
endpoint: unknown;
|
||||
body: unknown;
|
||||
headers?: unknown;
|
||||
effectiveInput?: unknown;
|
||||
}
|
||||
|
||||
interface RequestLoggerLike {
|
||||
logClientRawRequest: (endpoint: unknown, body: unknown, headers?: unknown) => void;
|
||||
logClientRawRequest: (
|
||||
endpoint: unknown,
|
||||
body: unknown,
|
||||
headers?: unknown,
|
||||
effectiveInput?: unknown
|
||||
) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -130,7 +136,8 @@ export function logClientRawRequestRedacted(
|
||||
videoBridgeObserved
|
||||
? redactVideoTranscriptFieldsForLog(clientRawRequest.body)
|
||||
: clientRawRequest.body,
|
||||
clientRawRequest.headers
|
||||
clientRawRequest.headers,
|
||||
clientRawRequest.effectiveInput
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -753,6 +753,17 @@ async function handleChatImplementation(
|
||||
clientRawRequest = chatAdmission.resolveClientRawAfterAdmission(clientRawRequest, () =>
|
||||
deferredClientRawBody.withClientBody((clientBody) => buildClientRawRequest(request, clientBody))
|
||||
);
|
||||
// Sibling of clientRawRequest.body, not a replacement: .body stays the raw
|
||||
// pre-reconstruction client bytes (see captureDeferredClientRawBody), while
|
||||
// this is the `input` actually dispatched with -- after the
|
||||
// previous_response_id reconstruction above ran, when it applies. A future
|
||||
// continuation lookup against THIS response must resolve from this field,
|
||||
// not the raw one. See the logClientRawRequest doc comment in requestLogger.ts.
|
||||
if (clientRawRequest && Array.isArray((body as { input?: unknown }).input)) {
|
||||
(clientRawRequest as { effectiveInput?: unknown }).effectiveInput = (
|
||||
body as { input: unknown[] }
|
||||
).input;
|
||||
}
|
||||
|
||||
// Guardrail pre-call pipeline — prompt injection, PII masking, and future custom rules.
|
||||
telemetry.startPhase("validate");
|
||||
|
||||
@@ -131,6 +131,85 @@ test("resolvePreviousResponseState reads output from a wrapped (streaming) clien
|
||||
});
|
||||
});
|
||||
|
||||
test("resolvePreviousResponseState chains off effectiveInput, not the pre-reconstruction clientRawRequest.body", () => {
|
||||
// Live incident (2026-09-03): clientRawRequest.body is deliberately captured
|
||||
// BEFORE chat.ts's own previous_response_id reconstruction runs
|
||||
// (captureDeferredClientRawBody's whole point -- it must reflect the raw
|
||||
// client bytes for audit/guardrail purposes, not what OmniRoute rewrote the
|
||||
// request into). For a turn that was ITSELF a continuation, body.input is
|
||||
// just the client's own trimmed delta -- a handful of tool-call items with
|
||||
// no leading system/user message. Chaining a LATER continuation off that
|
||||
// instead of the request's real effective input compounds into a
|
||||
// progressively truncated reconstruction, which the upstream provider then
|
||||
// rejects outright ("Please ensure that function call turn comes
|
||||
// immediately after a user turn..."). effectiveInput is captured AFTER
|
||||
// reconstruction and must be what this function chains off.
|
||||
insertCallLog({
|
||||
id: "log-continued-turn",
|
||||
responseId: "resp_continued",
|
||||
apiKeyId: "key-1",
|
||||
detailState: "ready",
|
||||
artifactRelPath: "2026-01-01/log-continued-turn.json",
|
||||
});
|
||||
writeArtifact("2026-01-01/log-continued-turn.json", {
|
||||
clientRawRequest: {
|
||||
// What the client actually sent this turn: just the new delta, relying
|
||||
// on OmniRoute to have reconstructed full history server-side.
|
||||
body: {
|
||||
input: [{ type: "function_call_output", call_id: "call_1", output: "42" }],
|
||||
},
|
||||
// What this request ACTUALLY dispatched with, after chat.ts's own
|
||||
// reconstruction expanded the prior turn's stored input+output back in.
|
||||
effectiveInput: [
|
||||
{ type: "message", role: "user", content: "hi" },
|
||||
{ type: "message", role: "assistant", content: "calling a tool" },
|
||||
{ type: "function_call", call_id: "call_1", name: "get_answer", arguments: "{}" },
|
||||
{ type: "function_call_output", call_id: "call_1", output: "42" },
|
||||
],
|
||||
},
|
||||
providerRequest: { body: { input: [] } },
|
||||
clientResponse: {
|
||||
id: "resp_continued",
|
||||
output: [{ type: "message", role: "assistant", content: "the answer is 42" }],
|
||||
},
|
||||
});
|
||||
|
||||
const result = store.resolvePreviousResponseState("resp_continued", "key-1");
|
||||
assert.deepEqual(result, {
|
||||
input: [
|
||||
{ type: "message", role: "user", content: "hi" },
|
||||
{ type: "message", role: "assistant", content: "calling a tool" },
|
||||
{ type: "function_call", call_id: "call_1", name: "get_answer", arguments: "{}" },
|
||||
{ type: "function_call_output", call_id: "call_1", output: "42" },
|
||||
],
|
||||
output: [{ type: "message", role: "assistant", content: "the answer is 42" }],
|
||||
});
|
||||
});
|
||||
|
||||
test("resolvePreviousResponseState falls back to clientRawRequest.body.input when effectiveInput is absent (pre-fix artifacts)", () => {
|
||||
insertCallLog({
|
||||
id: "log-legacy-no-effective-input",
|
||||
responseId: "resp_legacy",
|
||||
apiKeyId: "key-1",
|
||||
detailState: "ready",
|
||||
artifactRelPath: "2026-01-01/log-legacy-no-effective-input.json",
|
||||
});
|
||||
writeArtifact("2026-01-01/log-legacy-no-effective-input.json", {
|
||||
clientRawRequest: { body: { input: [{ type: "message", role: "user", content: "hi" }] } },
|
||||
providerRequest: { body: { input: [{ type: "message", role: "user", content: "hi" }] } },
|
||||
clientResponse: {
|
||||
id: "resp_legacy",
|
||||
output: [{ type: "message", role: "assistant", content: "hello" }],
|
||||
},
|
||||
});
|
||||
|
||||
const result = store.resolvePreviousResponseState("resp_legacy", "key-1");
|
||||
assert.deepEqual(result, {
|
||||
input: [{ type: "message", role: "user", content: "hi" }],
|
||||
output: [{ type: "message", role: "assistant", content: "hello" }],
|
||||
});
|
||||
});
|
||||
|
||||
test("resolvePreviousResponseState returns null for an unknown response id", () => {
|
||||
const result = store.resolvePreviousResponseState("resp_does_not_exist", "key-1");
|
||||
assert.equal(result, null);
|
||||
|
||||
Reference in New Issue
Block a user