fix: address Copilot review round 5 on #2318
RemoteClientTransport keeps its open-stream bookkeeping whether or not the consumer logs requests, so onFetchStreamUpdate works on its own and close() can synthesize the terminal update. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APKSYHGxXBUpnR7ErFiLJn Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
abc4967298
commit
a38f1322c1
2 changed files with 37 additions and 5 deletions
|
|
@ -432,6 +432,38 @@ describe("RemoteClientTransport (focused branch coverage)", () => {
|
||||||
expect(onFetchStreamUpdate).toHaveBeenCalledTimes(1);
|
expect(onFetchStreamUpdate).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("synthesizes the close on close() for a consumer that registered only onFetchStreamUpdate", async () => {
|
||||||
|
const onFetchStreamUpdate = vi.fn();
|
||||||
|
const { response, push } = createPushableEventStream();
|
||||||
|
const t = makeTransport(
|
||||||
|
{ events: () => response },
|
||||||
|
{ onFetchStreamUpdate },
|
||||||
|
);
|
||||||
|
await t.start();
|
||||||
|
push({
|
||||||
|
type: "fetch_request",
|
||||||
|
data: {
|
||||||
|
id: "stream",
|
||||||
|
method: "GET",
|
||||||
|
url: "http://x/mcp",
|
||||||
|
timestamp: new Date().toISOString(),
|
||||||
|
requestHeaders: {},
|
||||||
|
responseStatus: 200,
|
||||||
|
responseHeaders: { "content-type": "text/event-stream" },
|
||||||
|
} as never,
|
||||||
|
});
|
||||||
|
push({
|
||||||
|
type: "fetch_stream_update",
|
||||||
|
data: { id: "stream", eventCount: 3 },
|
||||||
|
});
|
||||||
|
await tick();
|
||||||
|
await t.close();
|
||||||
|
expect(onFetchStreamUpdate).toHaveBeenLastCalledWith("stream", {
|
||||||
|
eventCount: 3,
|
||||||
|
closedAt: expect.any(Date),
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("drops fetch_stream_update events when no handler is registered", async () => {
|
it("drops fetch_stream_update events when no handler is registered", async () => {
|
||||||
const onFetchRequest = vi.fn();
|
const onFetchRequest = vi.fn();
|
||||||
const t = makeTransport(
|
const t = makeTransport(
|
||||||
|
|
|
||||||
|
|
@ -552,11 +552,11 @@ export class RemoteClientTransport implements Transport {
|
||||||
this.resetSseResponseWait(progressToken);
|
this.resetSseResponseWait(progressToken);
|
||||||
}
|
}
|
||||||
this.onmessage?.(msg, undefined);
|
this.onmessage?.(msg, undefined);
|
||||||
} else if (
|
} else if (parsed.type === "fetch_request") {
|
||||||
parsed.type === "fetch_request" &&
|
|
||||||
this.options.onFetchRequest
|
|
||||||
) {
|
|
||||||
const entry = parsed.data;
|
const entry = parsed.data;
|
||||||
|
// Stream bookkeeping runs whether or not the consumer logs
|
||||||
|
// requests: `onFetchStreamUpdate` is documented to work on its
|
||||||
|
// own, and `close()` needs to know which streams are open.
|
||||||
if (
|
if (
|
||||||
isLongLivedStreamResponse(
|
isLongLivedStreamResponse(
|
||||||
entry.method,
|
entry.method,
|
||||||
|
|
@ -565,7 +565,7 @@ export class RemoteClientTransport implements Transport {
|
||||||
) {
|
) {
|
||||||
this.openStreams.set(entry.id, 0);
|
this.openStreams.set(entry.id, 0);
|
||||||
}
|
}
|
||||||
this.options.onFetchRequest({
|
this.options.onFetchRequest?.({
|
||||||
...entry,
|
...entry,
|
||||||
timestamp:
|
timestamp:
|
||||||
typeof entry.timestamp === "string"
|
typeof entry.timestamp === "string"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue