Skip to content

Commit 70396a2

Browse files
fix: unblock active streams on session close (#43)
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 parent dd9e8e3 commit 70396a2

2 files changed

Lines changed: 195 additions & 1 deletion

File tree

src/session.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ export class DroidSession {
140140
private _closed = false;
141141
private _cleanupAbortSignal: (() => void) | null = null;
142142
private _cleanupCallbacks: Array<() => Promise<void> | void> = [];
143+
private readonly _activeBridges = new Set<MessageBridge>();
143144

144145
/** @internal */
145146
constructor(
@@ -188,12 +189,17 @@ export class DroidSession {
188189
throwIfAborted(options?.abortSignal);
189190

190191
const startedAt = Date.now();
191-
const bridge = new MessageBridge(undefined, {
192+
let resolveDone: () => void = () => {};
193+
const donePromise = new Promise<void>((resolve) => {
194+
resolveDone = resolve;
195+
});
196+
const bridge = new MessageBridge(resolveDone, {
192197
includePartialMessages: options?.includePartialMessages,
193198
sessionId: this._sessionId,
194199
startedAt,
195200
outputFormat: options?.outputFormat,
196201
});
202+
this._activeBridges.add(bridge);
197203
const unsubscribe = this._client.onNotification(bridge.notificationHandler);
198204
let resolveAbort: () => void = () => {};
199205
const abortPromise = new Promise<void>((resolve) => {
@@ -213,6 +219,7 @@ export class DroidSession {
213219
files: options?.files,
214220
outputFormat: options?.outputFormat,
215221
}),
222+
donePromise,
216223
abortPromise,
217224
]);
218225
throwIfAborted(options?.abortSignal);
@@ -225,6 +232,7 @@ export class DroidSession {
225232
} finally {
226233
cleanupAbortSignal();
227234
unsubscribe();
235+
this._activeBridges.delete(bridge);
228236
}
229237
}
230238

@@ -240,6 +248,9 @@ export class DroidSession {
240248
this._closed = true;
241249
this._cleanupAbortSignal?.();
242250
this._cleanupAbortSignal = null;
251+
for (const bridge of this._activeBridges) {
252+
bridge.signalDone();
253+
}
243254

244255
try {
245256
await this._client.closeSession({ reason: 'other' }).catch(() => {});

tests/session.test.ts

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,27 @@ async function expectStreamToThrow(
7373
}).rejects.toThrow(ConnectionError);
7474
}
7575

76+
async function expectToResolveWithin<T>(
77+
promise: Promise<T>,
78+
timeoutMs: number
79+
): Promise<T> {
80+
let timeout: ReturnType<typeof setTimeout> | undefined;
81+
try {
82+
return await Promise.race([
83+
promise,
84+
new Promise<never>((_, reject) => {
85+
timeout = setTimeout(() => {
86+
reject(new Error(`Promise did not resolve within ${timeoutMs}ms`));
87+
}, timeoutMs);
88+
}),
89+
]);
90+
} finally {
91+
if (timeout !== undefined) {
92+
clearTimeout(timeout);
93+
}
94+
}
95+
}
96+
7697
/**
7798
* Set up transport to auto-respond to loadSession.
7899
*/
@@ -691,6 +712,168 @@ describe('DroidSession', () => {
691712
expect(closeMessage?.['params']).toEqual({ reason: 'other' });
692713
expect(transport.isConnected).toBe(false);
693714
});
715+
716+
it('unblocks an active stream with no completion notification when closed', async () => {
717+
const transport = new InMemoryTransport();
718+
await transport.connect();
719+
720+
let resolveAddSent: () => void = () => {};
721+
const addSent = new Promise<void>((resolve) => {
722+
resolveAddSent = resolve;
723+
});
724+
725+
wireTransportSend(transport, ({ method, id }) => {
726+
if (method === DroidServerMethod.INITIALIZE_SESSION) {
727+
queueMicrotask(() => {
728+
transport.injectMessage(
729+
makeSuccessResponse(id, {
730+
sessionId: 'sess-close-active-stream',
731+
session: {},
732+
settings: {
733+
modelId: 'test-model',
734+
reasoningEffort: 'medium',
735+
},
736+
})
737+
);
738+
});
739+
} else if (method === DroidServerMethod.ADD_USER_MESSAGE) {
740+
resolveAddSent();
741+
queueMicrotask(() => {
742+
transport.injectMessage(makeSuccessResponse(id, {}));
743+
});
744+
} else if (method === DroidServerMethod.CLOSE_SESSION) {
745+
queueMicrotask(() => {
746+
transport.injectMessage(makeSuccessResponse(id, {}));
747+
});
748+
}
749+
});
750+
751+
const session = await createSession({ transport });
752+
const messages: DroidMessage[] = [];
753+
const streamPromise = (async () => {
754+
for await (const msg of session.stream('wait forever')) {
755+
messages.push(msg);
756+
}
757+
})();
758+
759+
await addSent;
760+
761+
const closePromise = session.close();
762+
763+
await expectToResolveWithin(streamPromise, 100);
764+
await closePromise;
765+
expect(messages).toEqual([]);
766+
});
767+
768+
it('unblocks an active stream even when addUserMessage has no response', async () => {
769+
const transport = new InMemoryTransport();
770+
await transport.connect();
771+
772+
let resolveAddSent: () => void = () => {};
773+
const addSent = new Promise<void>((resolve) => {
774+
resolveAddSent = resolve;
775+
});
776+
777+
wireTransportSend(transport, ({ method, id }) => {
778+
if (method === DroidServerMethod.INITIALIZE_SESSION) {
779+
queueMicrotask(() => {
780+
transport.injectMessage(
781+
makeSuccessResponse(id, {
782+
sessionId: 'sess-close-pending-add',
783+
session: {},
784+
settings: {
785+
modelId: 'test-model',
786+
reasoningEffort: 'medium',
787+
},
788+
})
789+
);
790+
});
791+
} else if (method === DroidServerMethod.ADD_USER_MESSAGE) {
792+
resolveAddSent();
793+
} else if (method === DroidServerMethod.CLOSE_SESSION) {
794+
queueMicrotask(() => {
795+
transport.injectMessage(makeSuccessResponse(id, {}));
796+
});
797+
}
798+
});
799+
800+
const session = await createSession({ transport });
801+
const streamPromise = (async () => {
802+
const collected: DroidMessage[] = [];
803+
for await (const msg of session.stream('pending add')) {
804+
collected.push(msg);
805+
}
806+
return collected;
807+
})();
808+
809+
await addSent;
810+
811+
const closePromise = session.close();
812+
const messages = await expectToResolveWithin(streamPromise, 100);
813+
await closePromise;
814+
815+
expect(messages).toEqual([]);
816+
});
817+
818+
it('unblocks all active streams when closed', async () => {
819+
const transport = new InMemoryTransport();
820+
await transport.connect();
821+
822+
let addSentCount = 0;
823+
let resolveBothAddSent: () => void = () => {};
824+
const bothAddSent = new Promise<void>((resolve) => {
825+
resolveBothAddSent = resolve;
826+
});
827+
828+
wireTransportSend(transport, ({ method, id }) => {
829+
if (method === DroidServerMethod.INITIALIZE_SESSION) {
830+
queueMicrotask(() => {
831+
transport.injectMessage(
832+
makeSuccessResponse(id, {
833+
sessionId: 'sess-close-all-active-streams',
834+
session: {},
835+
settings: {
836+
modelId: 'test-model',
837+
reasoningEffort: 'medium',
838+
},
839+
})
840+
);
841+
});
842+
} else if (method === DroidServerMethod.ADD_USER_MESSAGE) {
843+
addSentCount += 1;
844+
if (addSentCount === 2) {
845+
resolveBothAddSent();
846+
}
847+
} else if (method === DroidServerMethod.CLOSE_SESSION) {
848+
queueMicrotask(() => {
849+
transport.injectMessage(makeSuccessResponse(id, {}));
850+
});
851+
}
852+
});
853+
854+
const session = await createSession({ transport });
855+
const collect = async (prompt: string): Promise<DroidMessage[]> => {
856+
const messages: DroidMessage[] = [];
857+
for await (const msg of session.stream(prompt)) {
858+
messages.push(msg);
859+
}
860+
return messages;
861+
};
862+
const firstStream = collect('first pending add');
863+
const secondStream = collect('second pending add');
864+
865+
await bothAddSent;
866+
867+
const closePromise = session.close();
868+
const [firstMessages, secondMessages] = await expectToResolveWithin(
869+
Promise.all([firstStream, secondStream]),
870+
100
871+
);
872+
await closePromise;
873+
874+
expect(firstMessages).toEqual([]);
875+
expect(secondMessages).toEqual([]);
876+
});
694877
});
695878

696879
describe('MCP methods (VAL-API-011)', () => {

0 commit comments

Comments
 (0)