Skip to content

Commit c4f7090

Browse files
refactor(sdk): move session storage to connection-level setStorageProvider
Replace per-session sessionStore config with a connection-level setStorageProvider() method that sets storage once for all sessions. Changes: - Remove sessionStore from SessionConfig, ResumeSessionConfig, and ListSessionsOptions types - Add StorageProviderConfig type alias - Add CopilotClient.setStorageProvider() method that calls session.setStorageProvider RPC - Replace per-session store maps with single storageProviderConfig field - Update createSessionStoreHandler to use connection-level config - Rewrite E2E tests for new setStorageProvider flow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent adeb25d commit c4f7090

8 files changed

Lines changed: 213 additions & 143 deletions

nodejs/src/client.ts

Lines changed: 70 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,8 @@ export class CopilotClient {
178178
private _rpc: ReturnType<typeof createServerRpc> | null = null;
179179
private processExitPromise: Promise<never> | null = null; // Rejects when CLI process exits
180180
private negotiatedProtocolVersion: number | null = null;
181-
/** Per-session store configs, keyed by sessionId. Used to route sessionStore.* RPC requests. */
182-
private sessionStoreConfigs: Map<string, SessionStoreConfig> = new Map();
183-
/**
184-
* Stores used during listSessions() calls. Keyed by an incrementing ID to support concurrent calls.
185-
* The latest registered store is used when the server calls sessionStore.list.
186-
*/
187-
private listSessionsStores: Map<number, SessionStoreConfig> = new Map();
188-
private listSessionsStoreNextId = 0;
181+
/** Connection-level storage provider config set via setStorageProvider(). */
182+
private storageProviderConfig: SessionStoreConfig | null = null;
189183

190184
/**
191185
* Typed server-scoped RPC methods.
@@ -351,6 +345,40 @@ export class CopilotClient {
351345
}
352346
}
353347

348+
/**
349+
* Declare this client as the session data storage provider.
350+
* Must be called before creating any sessions. Only one client can be the
351+
* storage provider at a time.
352+
*
353+
* @param config - The storage provider configuration with callbacks for persistence
354+
* @throws Error if the client is not connected and autoStart is disabled
355+
*
356+
* @example
357+
* ```typescript
358+
* await client.setStorageProvider({
359+
* descriptor: "redis://localhost/sessions",
360+
* onLoad: async (sessionId) => loadEventsFromRedis(sessionId),
361+
* onAppend: async (events, sessionId) => appendEventsToRedis(events, sessionId),
362+
* onTruncate: async (upToEventId, sessionId) => truncateEventsInRedis(upToEventId, sessionId),
363+
* onListSessions: async () => listSessionsFromRedis(),
364+
* onDelete: async (sessionId) => deleteSessionFromRedis(sessionId),
365+
* });
366+
* ```
367+
*/
368+
async setStorageProvider(config: SessionStoreConfig): Promise<void> {
369+
if (!this.connection) {
370+
if (this.options.autoStart) {
371+
await this.start();
372+
} else {
373+
throw new Error("Client not connected. Call start() first.");
374+
}
375+
}
376+
await this.connection!.sendRequest("session.setStorageProvider", {
377+
descriptor: config.descriptor,
378+
});
379+
this.storageProviderConfig = config;
380+
}
381+
354382
/**
355383
* Stops the CLI server and closes all active sessions.
356384
*
@@ -408,8 +436,7 @@ export class CopilotClient {
408436
}
409437
}
410438
this.sessions.clear();
411-
this.sessionStoreConfigs.clear();
412-
this.listSessionsStores.clear();
439+
this.storageProviderConfig = null;
413440

414441
// Close connection
415442
if (this.connection) {
@@ -494,8 +521,7 @@ export class CopilotClient {
494521

495522
// Clear sessions immediately without trying to destroy them
496523
this.sessions.clear();
497-
this.sessionStoreConfigs.clear();
498-
this.listSessionsStores.clear();
524+
this.storageProviderConfig = null;
499525

500526
// Force close connection
501527
if (this.connection) {
@@ -603,12 +629,6 @@ export class CopilotClient {
603629
}
604630
this.sessions.set(sessionId, session);
605631

606-
// Register session store callbacks before the RPC so that
607-
// store requests arriving during session creation are handled.
608-
if (config.sessionStore) {
609-
this.sessionStoreConfigs.set(sessionId, config.sessionStore);
610-
}
611-
612632
try {
613633
const response = await this.connection!.sendRequest("session.create", {
614634
...(await getTraceContext(this.onGetTraceContext)),
@@ -640,9 +660,6 @@ export class CopilotClient {
640660
skillDirectories: config.skillDirectories,
641661
disabledSkills: config.disabledSkills,
642662
infiniteSessions: config.infiniteSessions,
643-
sessionStore: config.sessionStore
644-
? { descriptor: config.sessionStore.descriptor }
645-
: undefined,
646663
});
647664

648665
const { workspacePath } = response as {
@@ -652,7 +669,6 @@ export class CopilotClient {
652669
session["_workspacePath"] = workspacePath;
653670
} catch (e) {
654671
this.sessions.delete(sessionId);
655-
this.sessionStoreConfigs.delete(sessionId);
656672
throw e;
657673
}
658674

@@ -719,11 +735,6 @@ export class CopilotClient {
719735
}
720736
this.sessions.set(sessionId, session);
721737

722-
// Register session store callbacks before the RPC
723-
if (config.sessionStore) {
724-
this.sessionStoreConfigs.set(sessionId, config.sessionStore);
725-
}
726-
727738
try {
728739
const response = await this.connection!.sendRequest("session.resume", {
729740
...(await getTraceContext(this.onGetTraceContext)),
@@ -756,9 +767,6 @@ export class CopilotClient {
756767
disabledSkills: config.disabledSkills,
757768
infiniteSessions: config.infiniteSessions,
758769
disableResume: config.disableResume,
759-
sessionStore: config.sessionStore
760-
? { descriptor: config.sessionStore.descriptor }
761-
: undefined,
762770
});
763771

764772
const { workspacePath } = response as {
@@ -768,7 +776,6 @@ export class CopilotClient {
768776
session["_workspacePath"] = workspacePath;
769777
} catch (e) {
770778
this.sessions.delete(sessionId);
771-
this.sessionStoreConfigs.delete(sessionId);
772779
throw e;
773780
}
774781

@@ -985,7 +992,6 @@ export class CopilotClient {
985992

986993
// Remove from local sessions map if present
987994
this.sessions.delete(sessionId);
988-
this.sessionStoreConfigs.delete(sessionId);
989995
}
990996

991997
/**
@@ -1008,50 +1014,34 @@ export class CopilotClient {
10081014

10091015
// Support both plain filter and full options object
10101016
const options: ListSessionsOptions | undefined =
1011-
filterOrOptions && "sessionStore" in filterOrOptions
1017+
filterOrOptions && "filter" in filterOrOptions
10121018
? (filterOrOptions as ListSessionsOptions)
10131019
: filterOrOptions
10141020
? { filter: filterOrOptions as SessionListFilter }
10151021
: undefined;
10161022

1017-
// Register the store for the duration of this RPC call (supports concurrent calls)
1018-
let storeId: number | undefined;
1019-
if (options?.sessionStore) {
1020-
storeId = this.listSessionsStoreNextId++;
1021-
this.listSessionsStores.set(storeId, options.sessionStore);
1022-
}
1023-
1024-
try {
1025-
const response = await this.connection.sendRequest("session.list", {
1026-
filter: options?.filter,
1027-
sessionStore: options?.sessionStore
1028-
? { descriptor: options.sessionStore.descriptor }
1029-
: undefined,
1030-
});
1031-
const { sessions } = response as {
1032-
sessions: Array<{
1033-
sessionId: string;
1034-
startTime: string;
1035-
modifiedTime: string;
1036-
summary?: string;
1037-
isRemote: boolean;
1038-
context?: SessionContext;
1039-
}>;
1040-
};
1023+
const response = await this.connection.sendRequest("session.list", {
1024+
filter: options?.filter,
1025+
});
1026+
const { sessions } = response as {
1027+
sessions: Array<{
1028+
sessionId: string;
1029+
startTime: string;
1030+
modifiedTime: string;
1031+
summary?: string;
1032+
isRemote: boolean;
1033+
context?: SessionContext;
1034+
}>;
1035+
};
10411036

1042-
return sessions.map((s) => ({
1043-
sessionId: s.sessionId,
1044-
startTime: new Date(s.startTime),
1045-
modifiedTime: new Date(s.modifiedTime),
1046-
summary: s.summary,
1047-
isRemote: s.isRemote,
1048-
context: s.context,
1049-
}));
1050-
} finally {
1051-
if (storeId !== undefined) {
1052-
this.listSessionsStores.delete(storeId);
1053-
}
1054-
}
1037+
return sessions.map((s) => ({
1038+
sessionId: s.sessionId,
1039+
startTime: new Date(s.startTime),
1040+
modifiedTime: new Date(s.modifiedTime),
1041+
summary: s.summary,
1042+
isRemote: s.isRemote,
1043+
context: s.context,
1044+
}));
10551045
}
10561046

10571047
/**
@@ -1533,43 +1523,38 @@ export class CopilotClient {
15331523

15341524
/**
15351525
* Create a {@link SessionStoreHandler} that routes each RPC call to the
1536-
* appropriate {@link SessionStoreConfig} registered for the given session.
1526+
* connection-level storage provider set via {@link setStorageProvider}.
15371527
*/
15381528
private createSessionStoreHandler(): SessionStoreHandler {
1539-
const getStore = (sessionId: string): SessionStoreConfig => {
1540-
const store = this.sessionStoreConfigs.get(sessionId);
1541-
if (!store) {
1542-
throw new Error(`No session store registered for session ${sessionId}`);
1529+
const getStore = (): SessionStoreConfig => {
1530+
if (!this.storageProviderConfig) {
1531+
throw new Error("No storage provider configured. Call setStorageProvider() first.");
15431532
}
1544-
return store;
1533+
return this.storageProviderConfig;
15451534
};
15461535

15471536
return {
15481537
load: async (params) => {
1549-
const store = getStore(params.sessionId);
1538+
const store = getStore();
15501539
const events = await store.onLoad(params.sessionId);
15511540
// Events are opaque on the wire but typed in the SDK consumer API
15521541
return { events: events as Record<string, unknown>[] };
15531542
},
15541543
append: async (params) => {
1555-
const store = getStore(params.sessionId);
1544+
const store = getStore();
15561545
await store.onAppend(params.events as SessionEvent[], params.sessionId);
15571546
},
15581547
truncate: async (params) => {
1559-
const store = getStore(params.sessionId);
1548+
const store = getStore();
15601549
return store.onTruncate(params.upToEventId, params.sessionId);
15611550
},
15621551
list: async () => {
1563-
// Use the most recently registered store for listing.
1564-
const store = Array.from(this.listSessionsStores.values()).pop();
1565-
if (!store) {
1566-
throw new Error("No session store registered for listing");
1567-
}
1552+
const store = getStore();
15681553
const sessions = await store.onListSessions();
15691554
return { sessions };
15701555
},
15711556
delete: async (params) => {
1572-
const store = getStore(params.sessionId);
1557+
const store = getStore();
15731558
await store.onDelete(params.sessionId);
15741559
},
15751560
};

nodejs/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export type {
4545
SessionMetadata,
4646
SessionStoreConfig,
4747
SessionStoreHandler,
48+
StorageProviderConfig,
4849
ClientApiHandlers,
4950
SystemMessageAppendConfig,
5051
SystemMessageConfig,

nodejs/src/types.ts

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -857,15 +857,6 @@ export interface SessionConfig {
857857
* but executes earlier in the lifecycle so no events are missed.
858858
*/
859859
onEvent?: SessionEventHandler;
860-
861-
/**
862-
* Session event store callbacks. When provided, the client handles all
863-
* event persistence instead of the server's default file-based storage.
864-
*
865-
* The server calls back into these callbacks over RPC whenever it needs
866-
* to load, append, truncate, list, or delete session events.
867-
*/
868-
sessionStore?: SessionStoreConfig;
869860
}
870861

871862
/**
@@ -894,8 +885,7 @@ export type ResumeSessionConfig = Pick<
894885
| "disabledSkills"
895886
| "infiniteSessions"
896887
| "onEvent"
897-
| "sessionStore"
898-
> & {
888+
>& {
899889
/**
900890
* When true, skips emitting the session.resume event.
901891
* Useful for reconnecting to a session without triggering resume-related side effects.
@@ -1046,9 +1036,7 @@ export interface SessionContext {
10461036
export interface SessionStoreConfig {
10471037
/**
10481038
* Opaque descriptor identifying this storage backend.
1049-
* Used for UI display (e.g., `"redis://localhost/sessions"`) and to verify
1050-
* resume-consistency — two stores are considered equivalent if and only if
1051-
* their descriptors are identical strings.
1039+
* Used for UI display (e.g., `"redis://localhost/sessions"`).
10521040
*/
10531041
descriptor: string;
10541042

@@ -1074,15 +1062,20 @@ export interface SessionStoreConfig {
10741062
}
10751063

10761064
/**
1077-
* Options for listing sessions, optionally with a session store.
1065+
* Options for listing sessions.
10781066
*/
10791067
export interface ListSessionsOptions {
10801068
/** Filter to narrow down which sessions to return. */
10811069
filter?: SessionListFilter;
1082-
/** When provided, list sessions from this store instead of the server's default. */
1083-
sessionStore?: SessionStoreConfig;
10841070
}
10851071

1072+
/**
1073+
* Configuration for becoming the session data storage provider.
1074+
* Same shape as SessionStoreConfig — call client.setStorageProvider(config)
1075+
* to claim storage before creating sessions.
1076+
*/
1077+
export type StorageProviderConfig = SessionStoreConfig;
1078+
10861079
/**
10871080
* Filter options for listing sessions
10881081
*/

0 commit comments

Comments
 (0)