Skip to content

Commit b21e42b

Browse files
Harden LLM inference SDK adapter + WS handler; add unit tests
Review fixes for github/copilot-sdk-internal#88 (Node SDK side). - Honor the runtime's accepted=false ack: the response sink now aborts the provider's signal and stops emitting once the runtime drops the request (I1). - Add a staging backstop in the adapter so a body chunk that arrives before its start frame is buffered and replayed rather than silently dropped (B1). - Run the WebSocket request/response pumps concurrently and race their terminal states, so an upstream-closes-first (or runtime-cancels-first) case tears the other side down instead of hanging on a parked iterator (B2). - Buffer inbound WS frames in wrapGlobalWebSocket until onMessage is registered so the first frames of a fast upstream aren't dropped. - Collapse the dead send branch, hoist TextEncoder/TextDecoder singletons, and correct the LlmWebSocketUpstream.onClose contract doc. - Update CopilotClientOptions.llmInference docs: streaming SSE and WebSocket are intercepted, not bypassed (I6). - Add unit tests: chunk-before-start staging, accepted=false abort, WS upstream-close-first finalisation, and WS upstream-error propagation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 53f00cc commit b21e42b

4 files changed

Lines changed: 478 additions & 77 deletions

File tree

nodejs/src/llmInferenceProvider.ts

Lines changed: 70 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,13 @@ function makeBodyQueue(): BodyQueue {
177177
};
178178
}
179179

180+
const sharedTextEncoder = new TextEncoder();
181+
180182
function decodeChunkData(data: string, binary: boolean): Uint8Array {
181183
if (binary) {
182184
return new Uint8Array(Buffer.from(data, "base64"));
183185
}
184-
return new TextEncoder().encode(data);
186+
return sharedTextEncoder.encode(data);
185187
}
186188

187189
interface PendingState {
@@ -209,9 +211,30 @@ interface PendingState {
209211
*/
210212
export function createLlmInferenceAdapter(
211213
provider: LlmInferenceProvider,
212-
getServerRpc: () => ServerRpc | undefined,
214+
getServerRpc: () => ServerRpc | undefined
213215
): LlmInferenceHandler {
214216
const pending = new Map<string, PendingState>();
217+
// Defense-in-depth backstop: chunks that arrive before their `start`
218+
// frame (a reordering the runtime's single ordered dispatch should make
219+
// impossible) are staged here keyed by requestId and drained the moment
220+
// `httpRequestStart` registers the matching state, so a body byte is
221+
// never silently dropped.
222+
const staged = new Map<string, LlmInferenceHttpRequestChunkRequest[]>();
223+
224+
function routeChunk(state: PendingState, params: LlmInferenceHttpRequestChunkRequest): void {
225+
if (params.cancel) {
226+
state.cancelled = true;
227+
state.abort.abort();
228+
state.queue.push({ cancel: { reason: params.cancelReason } });
229+
return;
230+
}
231+
if (params.data && params.data.length > 0) {
232+
state.queue.push({ chunk: decodeChunkData(params.data, !!params.binary) });
233+
}
234+
if (params.end) {
235+
state.queue.push({ end: true });
236+
}
237+
}
215238

216239
function makeSink(requestId: string, state: PendingState): LlmInferenceResponseSink {
217240
const rpc = (): ServerRpc => {
@@ -221,6 +244,21 @@ export function createLlmInferenceAdapter(
221244
}
222245
return r;
223246
};
247+
// The runtime acknowledges every response frame with `accepted`.
248+
// `accepted: false` means it has dropped the request (e.g. it
249+
// cancelled), so we abort the provider's upstream work and stop
250+
// emitting — there is no consumer for further frames.
251+
const rejectedByRuntime = (): never => {
252+
if (!state.cancelled) {
253+
state.cancelled = true;
254+
state.abort.abort();
255+
}
256+
state.finished = true;
257+
pending.delete(requestId);
258+
throw new Error(
259+
"LLM inference response was rejected by the runtime (request no longer active)."
260+
);
261+
};
224262
return {
225263
async start(init: LlmInferenceResponseInit): Promise<void> {
226264
if (state.started) {
@@ -230,27 +268,38 @@ export function createLlmInferenceAdapter(
230268
throw new Error("LLM inference response sink already finished.");
231269
}
232270
state.started = true;
233-
await rpc().llmInference.httpResponseStart({
271+
const result = await rpc().llmInference.httpResponseStart({
234272
requestId,
235273
status: init.status,
236274
statusText: init.statusText,
237275
headers: init.headers ?? {},
238276
});
277+
if (!result.accepted) {
278+
rejectedByRuntime();
279+
}
239280
},
240281
async write(data: string | Uint8Array): Promise<void> {
282+
if (state.cancelled) {
283+
throw new Error("LLM inference request was cancelled by the runtime.");
284+
}
241285
if (!state.started) {
242286
throw new Error("LLM inference response sink.write() called before start().");
243287
}
244288
if (state.finished) {
245-
throw new Error("LLM inference response sink.write() called after end()/error().");
289+
throw new Error(
290+
"LLM inference response sink.write() called after end()/error()."
291+
);
246292
}
247293
const isString = typeof data === "string";
248-
await rpc().llmInference.httpResponseChunk({
294+
const result = await rpc().llmInference.httpResponseChunk({
249295
requestId,
250296
data: isString ? data : Buffer.from(data).toString("base64"),
251297
binary: !isString,
252298
end: false,
253299
});
300+
if (!result.accepted) {
301+
rejectedByRuntime();
302+
}
254303
},
255304
async end(): Promise<void> {
256305
if (state.finished) {
@@ -283,7 +332,7 @@ export function createLlmInferenceAdapter(
283332
async function failViaSink(
284333
sink: LlmInferenceResponseSink,
285334
state: PendingState,
286-
message: string,
335+
message: string
287336
): Promise<void> {
288337
if (state.finished) {
289338
return;
@@ -300,7 +349,7 @@ export function createLlmInferenceAdapter(
300349

301350
async function finishCancelled(
302351
sink: LlmInferenceResponseSink,
303-
state: PendingState,
352+
state: PendingState
304353
): Promise<void> {
305354
if (state.finished) {
306355
return;
@@ -317,7 +366,7 @@ export function createLlmInferenceAdapter(
317366

318367
return {
319368
async httpRequestStart(
320-
params: LlmInferenceHttpRequestStartRequest,
369+
params: LlmInferenceHttpRequestStartRequest
321370
): Promise<LlmInferenceHttpRequestStartResult> {
322371
const state: PendingState = {
323372
queue: makeBodyQueue(),
@@ -327,6 +376,13 @@ export function createLlmInferenceAdapter(
327376
cancelled: false,
328377
};
329378
pending.set(params.requestId, state);
379+
const stagedChunks = staged.get(params.requestId);
380+
if (stagedChunks) {
381+
staged.delete(params.requestId);
382+
for (const chunk of stagedChunks) {
383+
routeChunk(state, chunk);
384+
}
385+
}
330386
const sink = makeSink(params.requestId, state);
331387
const request: LlmInferenceRequest = {
332388
requestId: params.requestId,
@@ -346,7 +402,7 @@ export function createLlmInferenceAdapter(
346402
await failViaSink(
347403
sink,
348404
state,
349-
"LLM inference provider returned without finalising the response (call responseBody.end() or .error()).",
405+
"LLM inference provider returned without finalising the response (call responseBody.end() or .error())."
350406
);
351407
}
352408
} catch (err) {
@@ -365,24 +421,16 @@ export function createLlmInferenceAdapter(
365421
return {};
366422
},
367423
async httpRequestChunk(
368-
params: LlmInferenceHttpRequestChunkRequest,
424+
params: LlmInferenceHttpRequestChunkRequest
369425
): Promise<LlmInferenceHttpRequestChunkResult> {
370426
const state = pending.get(params.requestId);
371427
if (!state) {
428+
const buffered = staged.get(params.requestId) ?? [];
429+
buffered.push(params);
430+
staged.set(params.requestId, buffered);
372431
return {};
373432
}
374-
if (params.cancel) {
375-
state.cancelled = true;
376-
state.abort.abort();
377-
state.queue.push({ cancel: { reason: params.cancelReason } });
378-
return {};
379-
}
380-
if (params.data && params.data.length > 0) {
381-
state.queue.push({ chunk: decodeChunkData(params.data, !!params.binary) });
382-
}
383-
if (params.end) {
384-
state.queue.push({ end: true });
385-
}
433+
routeChunk(state, params);
386434
return {};
387435
},
388436
};

0 commit comments

Comments
 (0)