Skip to content

Disconnect the client when the bot disconnects, with option to disable #185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mattieruth wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Disconnect the client when the bot disconnects, with option to disable #185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9e43a28
feat(js): disconnect the client when the bot disconnects, but give th…
mattieruth Mar 11, 2026
ca48670
fix: fix vulnerabilities
mattieruth Mar 11, 2026
63888dc
style: let's be lenient on how people write
mattieruth Mar 11, 2026
015de48
Initial plan
Copilot Mar 11, 2026
ad1c19a
test: add tests for disconnectOnBotDisconnect behavior
Copilot Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .commitlintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
"rules": {
"scope-enum": [2, "always", ["js", "react", ""]],
"body-max-line-length": [0],
"footer-max-line-length": [0]
"footer-max-line-length": [0],
"header-case": [0],
"body-case": [0]
}
}
20 changes: 17 additions & 3 deletions client-js/client/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export type RTVIEventCallbacks = Partial<{
onBotStarted: (botResponse: unknown) => void;
onBotConnected: (participant: Participant) => void;
onBotReady: (botReadyData: BotReadyData) => void;
onBotDisconnected: (participant: Participant) => void;
onBotDisconnected: (participant?: Participant) => void;
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onBotDisconnected now takes an optional Participant, which is a breaking change for TS consumers using (participant: Participant) => void handlers (not assignable to an optional-param callback under strict). Prefer keeping the callback signature stable and ensuring a Participant is always provided, or add a new separate callback/event for the no-participant case instead of changing this one.

Suggested change
onBotDisconnected: (participant?: Participant) => void;
onBotDisconnected: (participant: Participant) => void;

Copilot uses AI. Check for mistakes.
onMetrics: (data: PipecatMetricsData) => void;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -158,6 +158,13 @@ export interface PipecatClientOptions {
* Default to false
*/
enableScreenShare?: boolean;

/**
* Disconnect when the bot disconnects.
*
* Default to true
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disconnectOnBotDisconnect defaults to true, which changes default client behavior (auto-disconnect on bot disconnect) without the caller opting in. For a library patch/minor release this is likely an unexpected breaking behavior change; consider defaulting to false (opt-in), or clearly treating this as a breaking change (major bump / explicit migration note) if the new default is required.

Suggested change
* Default to true
* Default to true.
*
* NOTE: This default of `true` is a breaking change compared to earlier
* versions, where the client would remain connected when the bot
* disconnected unless this behavior was explicitly requested.
* If you rely on the previous behavior, explicitly set
* `disconnectOnBotDisconnect: false` when creating the client.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@mattieruth mattieruth Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another good point by copilot except... that technically the current behavior is breaking. we used to disconnect the client when the bot disconnected but somewhere along the way that stopped and it's pretty annoying.

*/
disconnectOnBotDisconnect?: boolean;
}

abstract class RTVIEventEmitter extends (EventEmitter as unknown as new () => TypedEmitter<RTVIEvents>) {}
Expand All @@ -167,6 +174,7 @@ export class PipecatClient extends RTVIEventEmitter {
private _connectResolve: ((value: BotReadyData) => void) | undefined;
protected _transport: Transport;
protected _transportWrapper: TransportWrapper;
protected _disconnectOnBotDisconnect: boolean;
declare protected _messageDispatcher: MessageDispatcher;
protected _functionCallCallbacks: Record<string, FunctionCallCallback> = {};
protected _abortController: AbortController | undefined;
Expand All @@ -182,6 +190,8 @@ export class PipecatClient extends RTVIEventEmitter {
this._transport = options.transport;
this._transportWrapper = new TransportWrapper(this._transport);

this._disconnectOnBotDisconnect = options.disconnectOnBotDisconnect ?? true;

// Wrap transport callbacks with event triggers
// This allows for either functional callbacks or .on / .off event listeners
const wrappedCallbacks: RTVIEventCallbacks = {
Expand Down Expand Up @@ -209,7 +219,7 @@ export class PipecatClient extends RTVIEventEmitter {
const data = message.data as ErrorData;
if (data?.fatal) {
logger.error("Fatal error reported. Disconnecting...");
this.disconnect();
void this.disconnect();
}
},
onConnected: () => {
Expand Down Expand Up @@ -295,6 +305,10 @@ export class PipecatClient extends RTVIEventEmitter {
onBotDisconnected: (p) => {
options?.callbacks?.onBotDisconnected?.(p);
this.emit(RTVIEvent.BotDisconnected, p);
if (this._disconnectOnBotDisconnect) {
logger.info("Bot disconnected. Disconnecting client...");
void this.disconnect();
}
Comment on lines 305 to +311
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior (auto-disconnect when the bot disconnects, and the disconnectOnBotDisconnect opt-out) isn’t covered by existing Jest tests in client-js/tests/client.spec.ts. Please add tests that simulate a bot-disconnect callback from the transport and assert (1) disconnect is triggered by default, and (2) disconnect is not triggered when disconnectOnBotDisconnect: false.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@mattieruth mattieruth Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

},
onUserStartedSpeaking: () => {
options?.callbacks?.onUserStartedSpeaking?.();
Expand Down Expand Up @@ -480,7 +494,7 @@ export class PipecatClient extends RTVIEventEmitter {
);
await this._transport.sendReadyMessage();
} catch (e) {
this.disconnect();
void this.disconnect();
reject(e);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion client-js/rtvi/events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export type RTVIEvents = Partial<{
botStarted: (botResponse: unknown) => void;
botConnected: (participant: Participant) => void;
botReady: (botData: BotReadyData) => void;
botDisconnected: (participant: Participant) => void;
botDisconnected: (participant?: Participant) => void;
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing botDisconnected handler to accept an optional Participant is a breaking TypeScript API change under strictFunctionTypes (existing handlers typed as (participant: Participant) => void will no longer be assignable). If the participant can be missing, consider keeping the public handler type unchanged and guaranteeing a Participant is passed (e.g., cache the bot participant from botConnected / botReady and reuse it on disconnect), or introduce a separate event for disconnect-without-participant rather than weakening this one.

Suggested change
botDisconnected: (participant?: Participant) => void;
botDisconnected: (participant: Participant) => void;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@mattieruth mattieruth Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copilot has a valid point here, but i don't know how to NOT break this. There IS no participant when it comes to the smallWebRTC Transport.

error: (message: RTVIMessage) => void;

/** server messaging */
Expand Down
83 changes: 83 additions & 0 deletions client-js/tests/client.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ describe("PipecatClient Methods", () => {
test("llm-function-call-started should trigger callback and emit event", async () => {
let callbackTriggered = false;
let eventTriggered = false;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let callbackData: any = null;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let eventData: any = null;

const clientWithCallbacks = new PipecatClient({
Expand Down Expand Up @@ -147,7 +149,9 @@ describe("PipecatClient Methods", () => {
test("llm-function-call-in-progress should trigger callback and emit event", async () => {
let callbackTriggered = false;
let eventTriggered = false;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let callbackData: any = null;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let eventData: any = null;

const clientWithCallbacks = new PipecatClient({
Expand Down Expand Up @@ -191,7 +195,9 @@ describe("PipecatClient Methods", () => {
test("llm-function-call-stopped should trigger callback and emit event", async () => {
let callbackTriggered = false;
let eventTriggered = false;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let callbackData: any = null;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let eventData: any = null;

const clientWithCallbacks = new PipecatClient({
Expand Down Expand Up @@ -238,7 +244,9 @@ describe("PipecatClient Methods", () => {
test("deprecated llm-function-call should trigger callback and emit event", async () => {
let callbackTriggered = false;
let eventTriggered = false;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let callbackData: any = null;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let eventData: any = null;

const clientWithCallbacks = new PipecatClient({
Expand Down Expand Up @@ -344,6 +352,81 @@ describe("PipecatClient Methods", () => {
client.enableScreenShare(true);
expect(client.isSharingScreen).toBe(true);
});

test("should auto-disconnect when bot disconnects (default behavior)", async () => {
await client.connect();
expect(client.connected).toBe(true);

const disconnectedPromise = new Promise<void>((resolve) => {
client.on(RTVIEvent.TransportStateChanged, (state) => {
if (state === "disconnected") resolve();
});
});

(client.transport as TransportStub).simulateBotDisconnect();

await disconnectedPromise;

expect(client.connected).toBe(false);
expect(client.state).toBe("disconnected");
});

test("should NOT auto-disconnect when bot disconnects if disconnectOnBotDisconnect is false", async () => {
const clientNoBotDisconnect = new PipecatClient({
transport: TransportStub.create(),
disconnectOnBotDisconnect: false,
});

await clientNoBotDisconnect.connect();
expect(clientNoBotDisconnect.connected).toBe(true);

let disconnectCalled = false;
clientNoBotDisconnect.on(RTVIEvent.Disconnected, () => {
disconnectCalled = true;
});

(clientNoBotDisconnect.transport as TransportStub).simulateBotDisconnect();

// Yield to allow any async disconnect to fire if it were going to
await new Promise<void>((resolve) => setTimeout(resolve, 0));

expect(disconnectCalled).toBe(false);
expect(clientNoBotDisconnect.connected).toBe(true);
expect(clientNoBotDisconnect.state).toBe("ready");
});

test("should invoke onBotDisconnected callback regardless of disconnectOnBotDisconnect setting", async () => {
let callbackCalledDefault = false;
let callbackCalledOptOut = false;

const clientDefault = new PipecatClient({
transport: TransportStub.create(),
callbacks: {
onBotDisconnected: () => {
callbackCalledDefault = true;
},
},
});

const clientOptOut = new PipecatClient({
transport: TransportStub.create(),
disconnectOnBotDisconnect: false,
callbacks: {
onBotDisconnected: () => {
callbackCalledOptOut = true;
},
},
});

await clientDefault.connect();
await clientOptOut.connect();

(clientDefault.transport as TransportStub).simulateBotDisconnect();
(clientOptOut.transport as TransportStub).simulateBotDisconnect();

expect(callbackCalledDefault).toBe(true);
expect(callbackCalledOptOut).toBe(true);
});
});

describe("messageSizeWithinLimit utility function", () => {
Expand Down
7 changes: 6 additions & 1 deletion client-js/tests/stubs/transport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { PipecatClientOptions, Tracks, Transport } from "../../client";
import { RTVIMessage, RTVIMessageType, TransportState } from "../../rtvi";
import { Participant, RTVIMessage, RTVIMessageType, TransportState } from "../../rtvi";

class mockState {
public isSharingScreen = false;
Expand Down Expand Up @@ -151,6 +151,11 @@ export class TransportStub extends Transport {
this._onMessage(message);
}

// to simulate the bot disconnecting
public simulateBotDisconnect(participant?: Participant): void {
this._callbacks.onBotDisconnected?.(participant);
}

public get state(): TransportState {
return this._state;
}
Expand Down
48 changes: 24 additions & 24 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading