Skip to content

Commit 6c2e04d

Browse files
olddonkeyclaude
andcommitted
refactor(responses): converge the two opaque-blob recovery call sites
Two units landed separately and left duplication. The recovery unit was written on a branch without the compaction-provenance change, so it degraded compaction items itself by rewriting `parsed._rawBody.input` in place. Once both are merged that walk is redundant: it sets `_stripReasoningEncryptedContent`, which is exactly the signal the adapter's own compaction scrub consumes. Verified rather than assumed, since the two call sites rebuild through different adapters. Both reach `openai-responses` (the recovery predicate restricts to it), whose `buildRequest` consumes `_rawBody` and runs `scrubOcxCompactionItems`; the native passthrough site resolves a passthrough retry adapter, the generic site rebuilds through the retained `activeAdapter`. So the manual walk changes no outbound body on either path, and dropping it removes a mutation whose side effect outlived the request. The native Responses branch returns before the generic `recovery:` loop, so the recovery block was also written out twice. Whoever next adds a recovery kind to the generic loop would not know a second loop exists. Extract the shared predicate, guard, preparation, body cancellation and rebuild into one `attemptOpaqueBlobRecovery` helper both sites call, each keeping its own control flow and its site-specific rebuild — the generic one still invalidates the same-target request. Cross-reference comments on both loops name the other. No outbound behaviour changes. Existing recovery tests are untouched; added coverage for routed compaction recovery through the generic loop. Co-Authored-By: Claude Fable 5 <[email protected]>
1 parent 1bbb63b commit 6c2e04d

2 files changed

Lines changed: 110 additions & 64 deletions

File tree

‎src/server/responses/core.ts‎

Lines changed: 73 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
updateReasoningReplayServingIdentity,
2525
} from "../../responses/reasoning-replay-cache";
2626
import { awaitThoughtSignatureDurability, thoughtSignatureReplaySalt } from "../../responses/thought-signature-replay";
27-
import { buildCompactV1Output, COMPACT_PROMPT, compactionItemToText, decodeCompactionSummary, extractCompactUserMessages, isCompactionItemType } from "../../responses/compaction";
27+
import { buildCompactV1Output, COMPACT_PROMPT, decodeCompactionSummary, extractCompactUserMessages } from "../../responses/compaction";
2828
import { FORWARD_HEADERS, sanitizeReasoningInputContent } from "../../adapters/openai-responses";
2929
import {
3030
expandPreviousResponseInput,
@@ -516,27 +516,50 @@ async function opaqueBlobRejectionBodyForRecovery(
516516

517517
function prepareOpaqueBlobRecovery(parsed: OcxParsedRequest): void {
518518
parsed._stripReasoningEncryptedContent = true;
519-
const body = parsed._rawBody;
520-
if (!body || typeof body !== "object" || Array.isArray(body)) return;
521-
const input = (body as { input?: unknown }).input;
522-
if (!Array.isArray(input)) return;
523-
let changed = false;
524-
const recoveredInput = input.map(item => {
525-
if (!item || typeof item !== "object" || Array.isArray(item)) return item;
526-
const candidate = item as { type?: unknown; encrypted_content?: unknown };
527-
if (!isCompactionItemType(candidate.type) || typeof candidate.encrypted_content !== "string") {
528-
return item;
529-
}
530-
changed = true;
531-
return {
532-
type: "message",
533-
role: "user",
534-
content: [{ type: "input_text", text: compactionItemToText(candidate.encrypted_content) }],
535-
};
536-
});
537-
// Mutate only the raw input carrier so the adapter rebuild sees the same degradation as its
538-
// ordinary compaction scrub. Keeping the body object preserves non-enumerable persistence marks.
539-
if (changed) (body as { input: unknown[] }).input = recoveredInput;
519+
}
520+
521+
type OpaqueBlobRecoveryGuard = { attempted: boolean };
522+
523+
type OpaqueBlobRecoveryResult =
524+
| { kind: "skipped" }
525+
| { kind: "recovered"; response: Response }
526+
| { kind: "failed"; response: Response };
527+
528+
async function attemptOpaqueBlobRecovery(
529+
args: {
530+
response: Response;
531+
outboundBody?: string;
532+
adapterName: string;
533+
parsed: OcxParsedRequest;
534+
guard: OpaqueBlobRecoveryGuard;
535+
signal: AbortSignal;
536+
},
537+
rebuild: (kind: AttemptRecoveryKind) => Promise<Response | { failed: Response }>,
538+
): Promise<OpaqueBlobRecoveryResult> {
539+
const errorBody = await opaqueBlobRejectionBodyForRecovery(
540+
args.response,
541+
args.outboundBody,
542+
args.adapterName,
543+
args.guard.attempted,
544+
args.signal,
545+
);
546+
if (errorBody === undefined || !shouldAttemptOpaqueBlobRecovery({
547+
status: args.response.status,
548+
adapterName: args.adapterName,
549+
outboundBody: args.outboundBody,
550+
errorBody,
551+
alreadyAttempted: args.guard.attempted,
552+
})) {
553+
return { kind: "skipped" };
554+
}
555+
556+
args.guard.attempted = true;
557+
prepareOpaqueBlobRecovery(args.parsed);
558+
try { void args.response.body?.cancel().catch(() => {}); } catch { /* already consumed/closed */ }
559+
const result = await rebuild("opaque-blob-rejection");
560+
return "failed" in result
561+
? { kind: "failed", response: result.failed }
562+
: { kind: "recovered", response: result };
540563
}
541564

542565
function nonEmptyProviderApiKey(provider: OcxProviderConfig): string | undefined {
@@ -2875,7 +2898,7 @@ async function handleResponsesInner(
28752898
request.releaseBodyObservation?.();
28762899
}
28772900

2878-
let opaqueBlobRecoveryAttempted = false;
2901+
const opaqueBlobRecoveryGuard: OpaqueBlobRecoveryGuard = { attempted: false };
28792902
let oauth401ReplayAttempted = false;
28802903
const rateLimitPolicy = rateLimitRetryPolicyFor(route.provider);
28812904
let rateLimitRetries = 0;
@@ -2943,6 +2966,7 @@ async function handleResponsesInner(
29432966
}
29442967
};
29452968

2969+
// Keep recovery kinds in sync with the generic `recovery:` loop below.
29462970
passthroughRecovery: for (;;) {
29472971

29482972
// Native Responses providers return before the generic adapter recovery loop below. Keep
@@ -3150,33 +3174,24 @@ async function handleResponsesInner(
31503174
// eviction, or an older transcript). Inspect only a bounded clone of a 4xx whose exact outbound
31513175
// Responses body still carries opaque state, then rebuild once through the ordinary adapter
31523176
// sanitation path. A second rejection falls through unchanged because the guard stays armed.
3153-
const opaqueBlobErrorBody = await opaqueBlobRejectionBodyForRecovery(
3154-
upstreamResponse,
3155-
request.body,
3156-
adapter.name,
3157-
opaqueBlobRecoveryAttempted,
3158-
upstream.signal,
3159-
);
3160-
if (opaqueBlobErrorBody !== undefined && shouldAttemptOpaqueBlobRecovery({
3161-
status: upstreamResponse.status,
3162-
adapterName: adapter.name,
3177+
const opaqueBlobRecovery = await attemptOpaqueBlobRecovery({
3178+
response: upstreamResponse,
31633179
outboundBody: request.body,
3164-
errorBody: opaqueBlobErrorBody,
3165-
alreadyAttempted: opaqueBlobRecoveryAttempted,
3166-
})) {
3167-
opaqueBlobRecoveryAttempted = true;
3168-
prepareOpaqueBlobRecovery(parsed);
3169-
try { void upstreamResponse.body?.cancel().catch(() => {}); } catch { /* already consumed/closed */ }
3170-
const result = await rebuildAndRefetch("opaque-blob-rejection");
3171-
if ("failed" in result) return result.failed;
3172-
upstreamResponse = result;
3180+
adapterName: adapter.name,
3181+
parsed,
3182+
guard: opaqueBlobRecoveryGuard,
3183+
signal: upstream.signal,
3184+
}, rebuildAndRefetch);
3185+
if (opaqueBlobRecovery.kind === "failed") return opaqueBlobRecovery.response;
3186+
if (opaqueBlobRecovery.kind === "recovered") {
3187+
upstreamResponse = opaqueBlobRecovery.response;
31733188
continue passthroughRecovery;
31743189
}
31753190
break;
31763191
}
31773192
// Binding normally records before the first send. Repeat it only after a successful recovery
31783193
// so an eviction during the extra round trip cannot leave the next cross-route turn cold.
3179-
if (opaqueBlobRecoveryAttempted && upstreamResponse.ok) {
3194+
if (opaqueBlobRecoveryGuard.attempted && upstreamResponse.ok) {
31803195
updateReasoningReplayServingIdentity(parsed._reasoningReplayScope);
31813196
}
31823197
const headers = sanitizePassthroughHeaders(upstreamResponse.headers);
@@ -4243,7 +4258,7 @@ async function handleResponsesInner(
42434258
// adapter, and imageTierBias — once armed — rides EVERY subsequent rebuild so a
42444259
// 413→429 rotation cannot silently undo the tightening.
42454260
let imageRetryAttempted = false;
4246-
let opaqueBlobRecoveryAttempted = false;
4261+
const opaqueBlobRecoveryGuard: OpaqueBlobRecoveryGuard = { attempted: false };
42474262
let oauth401ReplayAttempted = false;
42484263
/**
42494264
* Rebuild the request from the current parsed input (and any image-tier bias) and refetch
@@ -4321,6 +4336,7 @@ async function handleResponsesInner(
43214336
return { failed: formatErrorResponse(502, "upstream_error", msg) };
43224337
}
43234338
};
4339+
// Keep recovery kinds in sync with the native Responses `passthroughRecovery:` loop above.
43244340
recovery: for (;;) {
43254341
if (
43264342
upstreamResponse.status === 401
@@ -4481,27 +4497,20 @@ async function handleResponsesInner(
44814497
// the missing authoritative signal. Rebuild once through the same sanitation path used by a
44824498
// known route switch; invalidating is mandatory because `parsed` mutates in place and the
44834499
// same-target cache would otherwise replay the rejected bytes verbatim.
4484-
const opaqueBlobErrorBody = await opaqueBlobRejectionBodyForRecovery(
4485-
upstreamResponse,
4486-
sameTargetRequest?.body,
4487-
activeAdapter.name,
4488-
opaqueBlobRecoveryAttempted,
4489-
upstream.signal,
4490-
);
4491-
if (opaqueBlobErrorBody !== undefined && shouldAttemptOpaqueBlobRecovery({
4492-
status: upstreamResponse.status,
4493-
adapterName: activeAdapter.name,
4500+
const opaqueBlobRecovery = await attemptOpaqueBlobRecovery({
4501+
response: upstreamResponse,
44944502
outboundBody: sameTargetRequest?.body,
4495-
errorBody: opaqueBlobErrorBody,
4496-
alreadyAttempted: opaqueBlobRecoveryAttempted,
4497-
})) {
4498-
opaqueBlobRecoveryAttempted = true;
4499-
prepareOpaqueBlobRecovery(parsed);
4503+
adapterName: activeAdapter.name,
4504+
parsed,
4505+
guard: opaqueBlobRecoveryGuard,
4506+
signal: upstream.signal,
4507+
}, recovery => {
45004508
invalidateSameTargetRequest();
4501-
try { void upstreamResponse.body?.cancel().catch(() => {}); } catch { /* already consumed/closed */ }
4502-
const result = await rebuildAndRefetch("opaque-blob-rejection");
4503-
if ("failed" in result) return result.failed;
4504-
upstreamResponse = result;
4509+
return rebuildAndRefetch(recovery);
4510+
});
4511+
if (opaqueBlobRecovery.kind === "failed") return opaqueBlobRecovery.response;
4512+
if (opaqueBlobRecovery.kind === "recovered") {
4513+
upstreamResponse = opaqueBlobRecovery.response;
45054514
continue recovery;
45064515
}
45074516
// Anthropic 413 request_too_large: rebuild once with every image one tier lower
@@ -4525,7 +4534,7 @@ async function handleResponsesInner(
45254534
}
45264535
// Binding normally records before the first send. Repeat it only after a successful recovery
45274536
// so an eviction during the extra round trip cannot leave the next cross-route turn cold.
4528-
if (opaqueBlobRecoveryAttempted && upstreamResponse.ok) {
4537+
if (opaqueBlobRecoveryGuard.attempted && upstreamResponse.ok) {
45294538
updateReasoningReplayServingIdentity(parsed._reasoningReplayScope);
45304539
}
45314540
if (!upstreamResponse.ok) {

‎tests/responses-opaque-blob-recovery.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,43 @@ describe("opaque blob recovery through /v1/responses", () => {
202202
expect(logCtx.activeAttempt?.recoveryKinds).toEqual(["opaque-blob-rejection"]);
203203
});
204204

205+
test("degrades a compaction blob through the generic routed-compaction recovery resend", async () => {
206+
const outbound: Array<Record<string, unknown>> = [];
207+
globalThis.fetch = (async (_input: RequestInfo | URL, init?: RequestInit) => {
208+
outbound.push(JSON.parse(String(init?.body)) as Record<string, unknown>);
209+
return outbound.length === 1 ? rejection(XAI_DECODE_ERROR) : success("resp-compact-recovered");
210+
}) as typeof fetch;
211+
const body = {
212+
model: "first/model-a",
213+
stream: false,
214+
store: false,
215+
input: [...reasoningReplayInput(), { type: "compaction_trigger" }],
216+
};
217+
const logCtx: RequestLogContext = { model: "", provider: "" };
218+
219+
const response = await handleResponses(new Request("http://localhost/v1/responses", {
220+
method: "POST",
221+
headers: {
222+
"content-type": "application/json",
223+
"x-codex-parent-thread-id": "thread-routed-compaction-recovery",
224+
},
225+
body: JSON.stringify(body),
226+
}), config(), logCtx);
227+
expect(response.status).toBe(200);
228+
await response.text();
229+
230+
expect(outbound).toHaveLength(2);
231+
expect(hasBlob(outbound[0]!)).toBe(true);
232+
expect(hasBlob(outbound[1]!)).toBe(false);
233+
expect(outbound[1]!.input).toContainEqual({
234+
type: "message",
235+
role: "user",
236+
content: [{ type: "input_text", text: OPAQUE_COMPACTION_NOTE }],
237+
});
238+
expect(logCtx.activeAttempt?.sendCount).toBe(2);
239+
expect(logCtx.activeAttempt?.recoveryKinds).toEqual(["opaque-blob-rejection"]);
240+
});
241+
205242
test("surfaces the second rejection after exactly one recovery attempt", async () => {
206243
let upstreamCalls = 0;
207244
globalThis.fetch = (async () => {

0 commit comments

Comments
 (0)