Skip to content

Commit 4fcdcd1

Browse files
0xJingwei0xJingwei
committed
fix: close responses repair review gaps
1 parent d219dbc commit 4fcdcd1

4 files changed

Lines changed: 30 additions & 14 deletions

File tree

‎devlog/_plan/260731_macos_rss_retention/060_impl_roadmap.md‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,19 +82,20 @@ tail loses reset semantics for opted-in clients). 110 is the terminal gate.
8282
queue. Suppress when `sawTerminal()` is true. Do not feed the tail through
8383
the inspector.
8484

85-
## Design decisions locked for 100 (from 052 risk table)
85+
## Design decisions locked for 100 (from 052 risk table; rewrite scope superseded by 100)
8686

87-
- Gate change only: `(win32 || darwin) && !needsClientRewrite`, and darwin
88-
additionally requires the EXPLICIT `streamMode: "eager-relay"` decision
89-
(`reason: "config-eager"`). `auto` on darwin stays tee while
90-
`MIN_FIXED_BUN_VERSION` is null. No rewrite-path widening.
87+
- Windows keeps its existing routing policy. Darwin requires the EXPLICIT
88+
`streamMode: "eager-relay"` decision (`reason: "config-eager"`) for both
89+
no-rewrite and client-rewrite traffic; rewrites run inline after the eager
90+
single reader. `auto` on darwin stays tee while `MIN_FIXED_BUN_VERSION` is
91+
null. See `100_darwin_eager_optin.md` for the superseding gate matrix.
9192
- Bun#32111 posture per 052: eager avoids the known reproducer shape but is
9293
unproven on 1.3.14 — opt-in only, never default.
9394

9495
## Out of scope for this unit's implementation phases
9596

9697
- Allocator residual (053): documented NOOP until a stable Bun ships the
9798
allocator train. No forced GC, no FFI purge, no restart-as-fix.
98-
- Rewrite-traffic eager migration (052 steps 4–6): future unit.
99+
- Default eager migration beyond the explicit Darwin opt-in: future unit.
99100
- `auto` default flip: blocked on a released Bun carrying #32120 (UNSAFE
100101
boundary per goalplan).

‎src/server/responses-snapshot-repair.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,13 @@ function repairOutputItem(
8484
: [];
8585
changed = !Array.isArray(rawContent)
8686
|| content.some((part, index) => part !== rawContent[index]);
87-
const role = item.role === "assistant" ? item.role : "assistant";
87+
// Responses output-message roles are the literal "assistant"; input roles are not valid here.
88+
const role = "assistant";
8889
changed = changed || role !== item.role;
8990
if (changed) repaired = { ...repaired, content, role };
9091
}
9192

92-
if (inferredStatus && typeof repaired.status !== "string") {
93+
if (inferredStatus && (typeof repaired.status !== "string" || repaired.status.trim().length === 0)) {
9394
repaired = { ...repaired, status: inferredStatus };
9495
}
9596
return repaired;
@@ -132,7 +133,7 @@ function repairResponseSnapshot(
132133
repaired.tools = defaults.tools;
133134
changed = true;
134135
}
135-
if (typeof repaired.status !== "string") {
136+
if (typeof repaired.status !== "string" || repaired.status.trim().length === 0) {
136137
repaired.status = defaultStatus;
137138
changed = true;
138139
}

‎src/server/sse-payload-rewrite.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import type { TranslatorBudget } from "../lib/translator-budget";
88
*/
99

1010
export type SsePayloadRewrite = ((payload: string) => string) & {
11-
/** Release state retained across events when the stream ends or is cancelled. */
11+
/**
12+
* Release state retained across events when the stream ends or is cancelled. Implementations
13+
* are idempotent. A composed rewrite attempts every child and then throws the first child error,
14+
* so callers on stream-teardown paths must guard this call.
15+
*/
1216
dispose?: () => void;
1317
};
1418

‎tests/responses-snapshot-repair.test.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
} from "../src/server/responses-snapshot-repair";
66
import { handleResponses } from "../src/server/responses";
77
import { createTranslatorBudget } from "../src/lib/translator-budget";
8-
import { isEagerRelaySseResponse } from "../src/server/relay";
8+
import { isEagerRelaySseResponse, MAX_COMPLETED_OUTPUT_ITEMS } from "../src/server/relay";
99
import type { OcxConfig } from "../src/types";
1010

1111
function parsedSseEvents(text: string): Array<Record<string, unknown>> {
@@ -112,7 +112,7 @@ describe("Responses passthrough sparse-snapshot repair", () => {
112112

113113
test("repairs output-item status and nested reasoning summary parts", () => {
114114
const rewrite = createResponsesSnapshotPayloadRewrite();
115-
for (const invalidStatus of [undefined, null, 42]) {
115+
for (const invalidStatus of [undefined, null, 42, ""]) {
116116
const added = JSON.parse(rewrite(JSON.stringify({
117117
type: "response.output_item.added",
118118
output_index: 0,
@@ -166,7 +166,7 @@ describe("Responses passthrough sparse-snapshot repair", () => {
166166
});
167167

168168
test("replaces structurally invalid response statuses with the event status", () => {
169-
for (const invalidStatus of [null, 42]) {
169+
for (const invalidStatus of [null, 42, ""]) {
170170
const rewrite = createResponsesSnapshotPayloadRewrite();
171171
const event = JSON.parse(rewrite(JSON.stringify({
172172
type: "response.completed",
@@ -194,6 +194,16 @@ describe("Responses passthrough sparse-snapshot repair", () => {
194194
expect(event.response.output[1]?.status).toBeUndefined();
195195
});
196196

197+
test("repairs non-assistant output message roles to the canonical Responses role", () => {
198+
const rewrite = createResponsesSnapshotPayloadRewrite();
199+
const event = JSON.parse(rewrite(JSON.stringify({
200+
type: "response.output_item.added",
201+
item: { id: "msg_role", type: "message", role: "user", content: [] },
202+
}))) as { item: Record<string, unknown> };
203+
204+
expect(event.item.role).toBe("assistant");
205+
});
206+
197207
test("reconstructs a sparse completed output from prior done items", () => {
198208
for (const sparseOutput of [undefined, null, [], "invalid"]) {
199209
const rewrite = createResponsesSnapshotPayloadRewrite();
@@ -309,7 +319,7 @@ describe("Responses passthrough sparse-snapshot repair", () => {
309319

310320
test("does not reconstruct a partial terminal after the retained-item cap is exceeded", () => {
311321
const rewrite = createResponsesSnapshotPayloadRewrite();
312-
for (let outputIndex = 0; outputIndex <= 256; outputIndex += 1) {
322+
for (let outputIndex = 0; outputIndex <= MAX_COMPLETED_OUTPUT_ITEMS; outputIndex += 1) {
313323
rewrite(JSON.stringify({
314324
type: "response.output_item.done",
315325
output_index: outputIndex,

0 commit comments

Comments
 (0)