Skip to content

Commit 40d7ada

Browse files
refactor: code quality cleanup — DRY schemas, consolidate dispatch, remove dead code (#46)
- Add createResponseSchema() factory to eliminate ~26 repeated z.union() blocks - Extract shared dispatchNotification() from duplicated protocol/client logic - Remove orphaned aggregateMessages() and isPartialMessage() (knip-verified) - Fix misleading @deprecated on LEGACY_FACTORY_API_VERSION (it's required by wire protocol) - Remove ~70 name-restating JSDoc comments that add no information beyond type names - Remove stale internal monorepo path references from enums.ts header - Add 11 tests for createResponseSchema and dispatchNotification Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 parent 70396a2 commit 40d7ada

13 files changed

Lines changed: 336 additions & 406 deletions

File tree

src/client.ts

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ import type { z } from 'zod';
22

33
import { ConnectionError, SessionError } from './errors.js';
44
import {
5+
dispatchNotification,
56
ProtocolEngine,
67
type AskUserHandler,
78
type NotificationCallback,
89
type NotificationFilter,
10+
type NotificationListener,
911
type PermissionHandler,
1012
} from './protocol.js';
1113
import type {
@@ -93,7 +95,6 @@ import {
9395
SESSION_INIT_TIMEOUT,
9496
} from './schemas/constants.js';
9597
import { DroidServerMethod, ToolConfirmationOutcome } from './schemas/enums.js';
96-
import { SessionNotificationParamsSchema } from './schemas/server.js';
9798
import type {
9899
AskUserRequestParams,
99100
AskUserResult,
@@ -106,11 +107,6 @@ export type ClientPermissionHandler = PermissionHandler;
106107

107108
export type ClientAskUserHandler = AskUserHandler;
108109

109-
interface ClientNotificationListener {
110-
readonly callback: NotificationCallback;
111-
readonly filter?: NotificationFilter;
112-
}
113-
114110
export interface DroidClientOptions {
115111
/** A connected DroidClientTransport implementation. */
116112
transport: DroidClientTransport;
@@ -124,11 +120,7 @@ export class DroidClient {
124120
private _sessionId: string | null = null;
125121
private _closed = false;
126122

127-
/**
128-
* Client-level notification listeners.
129-
* Each entry is [callback, optional notification filter].
130-
*/
131-
private readonly _notificationListeners: ClientNotificationListener[] = [];
123+
private readonly _notificationListeners: NotificationListener[] = [];
132124

133125
/** Client-level permission handler. */
134126
private _permissionHandler: ClientPermissionHandler | null = null;
@@ -463,7 +455,7 @@ export class DroidClient {
463455
callback: NotificationCallback,
464456
filter?: NotificationFilter
465457
): () => void {
466-
const entry: ClientNotificationListener = { callback, filter };
458+
const entry: NotificationListener = { callback, filter };
467459
this._notificationListeners.push(entry);
468460

469461
let unsubscribed = false;
@@ -508,29 +500,7 @@ export class DroidClient {
508500
}
509501

510502
private _dispatchNotification(notification: Record<string, unknown>): void {
511-
let notificationType: string | undefined;
512-
const parsed = SessionNotificationParamsSchema.safeParse(
513-
notification['params']
514-
);
515-
if (parsed.success) {
516-
notificationType = parsed.data.notification.type;
517-
}
518-
519-
const listeners = [...this._notificationListeners];
520-
for (const listener of listeners) {
521-
if (
522-
listener.filter?.type != null &&
523-
listener.filter.type !== notificationType
524-
) {
525-
continue;
526-
}
527-
528-
try {
529-
listener.callback(notification);
530-
} catch {
531-
// Notification listener raised — don't crash the client
532-
}
533-
}
503+
dispatchNotification(notification, [...this._notificationListeners]);
534504
}
535505

536506
private _dispatchPermissionRequest(

src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ export * from './errors.js';
77
export * from './types.js';
88
export { ProcessTransport } from './transport.js';
99

10-
export { ProtocolEngine } from './protocol.js';
10+
export { dispatchNotification, ProtocolEngine } from './protocol.js';
1111
export type {
1212
AskUserHandler,
1313
NotificationCallback,
1414
NotificationFilter,
15+
NotificationListener,
1516
PermissionHandler,
1617
} from './protocol.js';
1718

src/protocol.ts

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,40 @@ interface PendingRequest {
6060
readonly timer: ReturnType<typeof setTimeout>;
6161
}
6262

63-
interface NotificationListener {
63+
export interface NotificationListener {
6464
readonly callback: NotificationCallback;
6565
readonly filter?: NotificationFilter;
6666
}
6767

68+
/** Dispatch a notification to matching listeners, swallowing listener errors. */
69+
export function dispatchNotification(
70+
notification: Record<string, unknown>,
71+
listeners: Iterable<NotificationListener>
72+
): void {
73+
let notificationType: string | undefined;
74+
const parsed = SessionNotificationParamsSchema.safeParse(
75+
notification['params']
76+
);
77+
if (parsed.success) {
78+
notificationType = parsed.data.notification.type;
79+
}
80+
81+
for (const listener of listeners) {
82+
if (
83+
listener.filter?.type != null &&
84+
listener.filter.type !== notificationType
85+
) {
86+
continue;
87+
}
88+
89+
try {
90+
listener.callback(notification);
91+
} catch {
92+
// Notification listener raised — don't crash the dispatch loop
93+
}
94+
}
95+
}
96+
6897
export class ProtocolEngine {
6998
private readonly _transport: DroidClientTransport;
7099
private readonly _defaultTimeout: number;
@@ -277,28 +306,7 @@ export class ProtocolEngine {
277306
}
278307

279308
private _handleNotification(notification: Record<string, unknown>): void {
280-
let notificationType: string | undefined;
281-
const parsed = SessionNotificationParamsSchema.safeParse(
282-
notification['params']
283-
);
284-
if (parsed.success) {
285-
notificationType = parsed.data.notification.type;
286-
}
287-
288-
for (const listener of this._notificationListeners) {
289-
if (
290-
listener.filter?.type != null &&
291-
listener.filter.type !== notificationType
292-
) {
293-
continue;
294-
}
295-
296-
try {
297-
listener.callback(notification);
298-
} catch {
299-
// Notification listener raised — don't crash the engine
300-
}
301-
}
309+
dispatchNotification(notification, this._notificationListeners);
302310
}
303311

304312
private async _handleServerRequest(

0 commit comments

Comments
 (0)