From 68749cb903c4493803f652dea48e3534f2ca0d9f Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 17 Mar 2026 12:26:07 +0000 Subject: [PATCH 1/5] feat: add session store support to TypeScript SDK client - Add SessionStoreConfig interface with onLoad/onAppend/onTruncate/onListSessions/onDelete callbacks - Add sessionStore to SessionConfig and ResumeSessionConfig - Add ListSessionsOptions type for listSessions with session store - Update CopilotClient.createSession/resumeSession to send sessionStore descriptor and register RPC handlers - Update listSessions to accept ListSessionsOptions - Register sessionStore.* RPC request handlers on connection setup - Add E2E tests verifying persist, resume, listSessions, and descriptor mismatch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 155 +++++++++++++-- nodejs/src/index.ts | 2 + nodejs/src/types.ts | 55 ++++++ nodejs/test/e2e/session_store.test.ts | 177 ++++++++++++++++++ package-lock.json | 6 + ...rom_a_client_supplied_store_on_resume.yaml | 14 ++ ...ist_events_to_a_client_supplied_store.yaml | 10 + ...lient_supplied_store_for_listsessions.yaml | 10 + 8 files changed, 409 insertions(+), 20 deletions(-) create mode 100644 nodejs/test/e2e/session_store.test.ts create mode 100644 package-lock.json create mode 100644 test/snapshots/session_store/should_load_events_from_a_client_supplied_store_on_resume.yaml create mode 100644 test/snapshots/session_store/should_persist_events_to_a_client_supplied_store.yaml create mode 100644 test/snapshots/session_store/should_use_client_supplied_store_for_listsessions.yaml diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index b8e7b31dc..708025be1 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -33,6 +33,7 @@ import type { ForegroundSessionInfo, GetAuthStatusResponse, GetStatusResponse, + ListSessionsOptions, ModelInfo, ResumeSessionConfig, SessionConfig, @@ -43,6 +44,7 @@ import type { SessionLifecycleHandler, SessionListFilter, SessionMetadata, + SessionStoreConfig, TelemetryConfig, Tool, ToolCallRequestPayload, @@ -175,6 +177,10 @@ export class CopilotClient { private _rpc: ReturnType | null = null; private processExitPromise: Promise | null = null; // Rejects when CLI process exits private negotiatedProtocolVersion: number | null = null; + /** Per-session store configs, keyed by sessionId. Used to route sessionStore.* RPC requests. */ + private sessionStoreConfigs: Map = new Map(); + /** Temporary store used during listSessions() calls with a session store. */ + private listSessionsStore: SessionStoreConfig | null = null; /** * Typed server-scoped RPC methods. @@ -588,6 +594,12 @@ export class CopilotClient { } this.sessions.set(sessionId, session); + // Register session store callbacks before the RPC so that + // store requests arriving during session creation are handled. + if (config.sessionStore) { + this.sessionStoreConfigs.set(sessionId, config.sessionStore); + } + try { const response = await this.connection!.sendRequest("session.create", { ...(await getTraceContext(this.onGetTraceContext)), @@ -619,6 +631,9 @@ export class CopilotClient { skillDirectories: config.skillDirectories, disabledSkills: config.disabledSkills, infiniteSessions: config.infiniteSessions, + sessionStore: config.sessionStore + ? { descriptor: config.sessionStore.descriptor } + : undefined, }); const { workspacePath } = response as { @@ -628,6 +643,7 @@ export class CopilotClient { session["_workspacePath"] = workspacePath; } catch (e) { this.sessions.delete(sessionId); + this.sessionStoreConfigs.delete(sessionId); throw e; } @@ -694,6 +710,11 @@ export class CopilotClient { } this.sessions.set(sessionId, session); + // Register session store callbacks before the RPC + if (config.sessionStore) { + this.sessionStoreConfigs.set(sessionId, config.sessionStore); + } + try { const response = await this.connection!.sendRequest("session.resume", { ...(await getTraceContext(this.onGetTraceContext)), @@ -726,6 +747,9 @@ export class CopilotClient { disabledSkills: config.disabledSkills, infiniteSessions: config.infiniteSessions, disableResume: config.disableResume, + sessionStore: config.sessionStore + ? { descriptor: config.sessionStore.descriptor } + : undefined, }); const { workspacePath } = response as { @@ -735,6 +759,7 @@ export class CopilotClient { session["_workspacePath"] = workspacePath; } catch (e) { this.sessions.delete(sessionId); + this.sessionStoreConfigs.delete(sessionId); throw e; } @@ -951,6 +976,7 @@ export class CopilotClient { // Remove from local sessions map if present this.sessions.delete(sessionId); + this.sessionStoreConfigs.delete(sessionId); } /** @@ -966,31 +992,55 @@ export class CopilotClient { * // List sessions for a specific repository * const sessions = await client.listSessions({ repository: "owner/repo" }); */ - async listSessions(filter?: SessionListFilter): Promise { + async listSessions(filterOrOptions?: SessionListFilter | ListSessionsOptions): Promise { if (!this.connection) { throw new Error("Client not connected"); } - const response = await this.connection.sendRequest("session.list", { filter }); - const { sessions } = response as { - sessions: Array<{ - sessionId: string; - startTime: string; - modifiedTime: string; - summary?: string; - isRemote: boolean; - context?: SessionContext; - }>; - }; + // Support both plain filter and full options object + const options: ListSessionsOptions | undefined = + filterOrOptions && "sessionStore" in filterOrOptions + ? (filterOrOptions as ListSessionsOptions) + : filterOrOptions + ? { filter: filterOrOptions as SessionListFilter } + : undefined; - return sessions.map((s) => ({ - sessionId: s.sessionId, - startTime: new Date(s.startTime), - modifiedTime: new Date(s.modifiedTime), - summary: s.summary, - isRemote: s.isRemote, - context: s.context, - })); + // Temporarily register the store for the duration of this RPC call + if (options?.sessionStore) { + this.listSessionsStore = options.sessionStore; + } + + try { + const response = await this.connection.sendRequest("session.list", { + filter: options?.filter, + sessionStore: options?.sessionStore + ? { descriptor: options.sessionStore.descriptor } + : undefined, + }); + const { sessions } = response as { + sessions: Array<{ + sessionId: string; + startTime: string; + modifiedTime: string; + summary?: string; + isRemote: boolean; + context?: SessionContext; + }>; + }; + + return sessions.map((s) => ({ + sessionId: s.sessionId, + startTime: new Date(s.startTime), + modifiedTime: new Date(s.modifiedTime), + summary: s.summary, + isRemote: s.isRemote, + context: s.context, + })); + } finally { + if (options?.sessionStore) { + this.listSessionsStore = null; + } + } } /** @@ -1455,6 +1505,71 @@ export class CopilotClient { }): Promise<{ output?: unknown }> => await this.handleHooksInvoke(params) ); + // Register session store RPC handlers (server→client callbacks for event persistence) + this.connection.onRequest( + "sessionStore.load", + async (params: { sessionId: string }): Promise<{ events: SessionEvent[] }> => { + const store = this.sessionStoreConfigs.get(params.sessionId); + if (!store) { + throw new Error(`No session store registered for session ${params.sessionId}`); + } + const events = await store.onLoad(params.sessionId); + return { events }; + } + ); + + this.connection.onRequest( + "sessionStore.append", + async (params: { sessionId: string; events: SessionEvent[] }): Promise => { + const store = this.sessionStoreConfigs.get(params.sessionId); + if (!store) { + throw new Error(`No session store registered for session ${params.sessionId}`); + } + await store.onAppend(params.events, params.sessionId); + } + ); + + this.connection.onRequest( + "sessionStore.truncate", + async (params: { + sessionId: string; + upToEventId: string; + }): Promise<{ eventsRemoved: number; eventsKept: number }> => { + const store = this.sessionStoreConfigs.get(params.sessionId); + if (!store) { + throw new Error(`No session store registered for session ${params.sessionId}`); + } + return await store.onTruncate(params.upToEventId, params.sessionId); + } + ); + + this.connection.onRequest( + "sessionStore.list", + async (): Promise<{ + sessions: Array<{ sessionId: string; mtime: string; birthtime: string }>; + }> => { + // Use the most recently registered store for listing. + // In practice, listSessions() registers a temporary store before the RPC call. + const store = this.listSessionsStore; + if (!store) { + throw new Error("No session store registered for listing"); + } + const sessions = await store.onListSessions(); + return { sessions }; + } + ); + + this.connection.onRequest( + "sessionStore.delete", + async (params: { sessionId: string }): Promise => { + const store = this.sessionStoreConfigs.get(params.sessionId); + if (!store) { + throw new Error(`No session store registered for session ${params.sessionId}`); + } + await store.onDelete(params.sessionId); + } + ); + this.connection.onClose(() => { this.state = "disconnected"; }); diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index 214b80050..11a394430 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -19,6 +19,7 @@ export type { GetAuthStatusResponse, GetStatusResponse, InfiniteSessionConfig, + ListSessionsOptions, MCPLocalServerConfig, MCPRemoteServerConfig, MCPServerConfig, @@ -42,6 +43,7 @@ export type { SessionContext, SessionListFilter, SessionMetadata, + SessionStoreConfig, SystemMessageAppendConfig, SystemMessageConfig, SystemMessageReplaceConfig, diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 9576b6925..aed6a450b 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -844,6 +844,15 @@ export interface SessionConfig { * but executes earlier in the lifecycle so no events are missed. */ onEvent?: SessionEventHandler; + + /** + * Session event store callbacks. When provided, the client handles all + * event persistence instead of the server's default file-based storage. + * + * The server calls back into these callbacks over RPC whenever it needs + * to load, append, truncate, list, or delete session events. + */ + sessionStore?: SessionStoreConfig; } /** @@ -872,6 +881,7 @@ export type ResumeSessionConfig = Pick< | "disabledSkills" | "infiniteSessions" | "onEvent" + | "sessionStore" > & { /** * When true, skips emitting the session.resume event. @@ -1007,6 +1017,51 @@ export interface SessionContext { branch?: string; } +/** + * Callbacks for a session event store. When provided to {@link SessionConfig.sessionStore}, + * the server routes all event persistence through these callbacks over RPC instead + * of using its default file-based storage. + */ +export interface SessionStoreConfig { + /** + * Opaque descriptor identifying this storage backend. + * Used for UI display (e.g., `"redis://localhost/sessions"`) and to verify + * resume-consistency — two stores are considered equivalent if and only if + * their descriptors are identical strings. + */ + descriptor: string; + + /** Load all events for a session. Return an empty array if the session doesn't exist. */ + onLoad: (sessionId: string) => Promise; + + /** Append events to a session's event log. */ + onAppend: (events: SessionEvent[], sessionId: string) => Promise; + + /** Truncate events up to (and including) the given event ID. */ + onTruncate: ( + upToEventId: string, + sessionId: string, + ) => Promise<{ eventsRemoved: number; eventsKept: number }>; + + /** List all sessions in this store. */ + onListSessions: () => Promise< + Array<{ sessionId: string; mtime: string; birthtime: string }> + >; + + /** Delete a session from this store. */ + onDelete: (sessionId: string) => Promise; +} + +/** + * Options for listing sessions, optionally with a session store. + */ +export interface ListSessionsOptions { + /** Filter to narrow down which sessions to return. */ + filter?: SessionListFilter; + /** When provided, list sessions from this store instead of the server's default. */ + sessionStore?: SessionStoreConfig; +} + /** * Filter options for listing sessions */ diff --git a/nodejs/test/e2e/session_store.test.ts b/nodejs/test/e2e/session_store.test.ts new file mode 100644 index 000000000..062a32803 --- /dev/null +++ b/nodejs/test/e2e/session_store.test.ts @@ -0,0 +1,177 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +import { describe, expect, it, vi } from "vitest"; +import { approveAll, type SessionEvent, type SessionStoreConfig } from "../../src/index.js"; +import { createSdkTestContext } from "./harness/sdkTestContext.js"; +import { getFinalAssistantMessage } from "./harness/sdkTestHelper.js"; + +/** + * In-memory session event store for testing. + * Stores events in a Map keyed by sessionId, and tracks call counts + * for each operation so tests can assert they were invoked. + */ +class InMemorySessionStore { + private sessions = new Map(); + readonly calls = { + load: 0, + append: 0, + truncate: 0, + listSessions: 0, + delete: 0, + }; + + toConfig(descriptor: string): SessionStoreConfig { + return { + descriptor, + onLoad: async (sessionId) => { + this.calls.load++; + return this.sessions.get(sessionId) ?? []; + }, + onAppend: async (events, sessionId) => { + this.calls.append++; + const existing = this.sessions.get(sessionId) ?? []; + existing.push(...events); + this.sessions.set(sessionId, existing); + }, + onTruncate: async (upToEventId, sessionId) => { + this.calls.truncate++; + const existing = this.sessions.get(sessionId) ?? []; + const idx = existing.findIndex((e) => e.id === upToEventId); + if (idx === -1) { + return { eventsRemoved: 0, eventsKept: existing.length }; + } + const kept = existing.slice(idx + 1); + this.sessions.set(sessionId, kept); + return { eventsRemoved: idx + 1, eventsKept: kept.length }; + }, + onListSessions: async () => { + this.calls.listSessions++; + const now = new Date().toISOString(); + return Array.from(this.sessions.keys()).map((sessionId) => ({ + sessionId, + mtime: now, + birthtime: now, + })); + }, + onDelete: async (sessionId) => { + this.calls.delete++; + this.sessions.delete(sessionId); + }, + }; + } + + getEvents(sessionId: string): SessionEvent[] { + return this.sessions.get(sessionId) ?? []; + } + + hasSession(sessionId: string): boolean { + return this.sessions.has(sessionId); + } + + get sessionCount(): number { + return this.sessions.size; + } +} + +describe("Session Store", async () => { + const { copilotClient: client } = await createSdkTestContext(); + + it("should persist events to a client-supplied store", async () => { + const store = new InMemorySessionStore(); + const session = await client.createSession({ + onPermissionRequest: approveAll, + sessionStore: store.toConfig("memory://test-persist"), + }); + + // Send a message and wait for the response + const msg = await session.sendAndWait({ prompt: "What is 100 + 200?" }); + expect(msg?.data.content).toContain("300"); + + // Verify onAppend was called — events should have been routed to our store. + // The SessionWriter uses debounced flushing, so poll until events arrive. + await vi.waitFor( + () => { + const events = store.getEvents(session.sessionId); + const eventTypes = events.map((e) => e.type); + expect(eventTypes).toContain("session.start"); + expect(eventTypes).toContain("user.message"); + expect(eventTypes).toContain("assistant.message"); + }, + { timeout: 10_000, interval: 200 }, + ); + expect(store.calls.append).toBeGreaterThan(0); + }); + + it("should load events from a client-supplied store on resume", async () => { + const store = new InMemorySessionStore(); + const storeConfig = store.toConfig("memory://test-resume"); + + // Create a session and send a message + const session1 = await client.createSession({ + onPermissionRequest: approveAll, + sessionStore: storeConfig, + }); + const sessionId = session1.sessionId; + + const msg1 = await session1.sendAndWait({ prompt: "What is 50 + 50?" }); + expect(msg1?.data.content).toContain("100"); + await session1.disconnect(); + + // Verify onLoad is called when resuming + const loadCountBefore = store.calls.load; + const session2 = await client.resumeSession(sessionId, { + onPermissionRequest: approveAll, + sessionStore: storeConfig, + }); + + expect(store.calls.load).toBeGreaterThan(loadCountBefore); + + // Send another message to verify the session is functional + const msg2 = await session2.sendAndWait({ prompt: "What is that times 3?" }); + expect(msg2?.data.content).toContain("300"); + }); + + it("should use client-supplied store for listSessions", async () => { + const store = new InMemorySessionStore(); + const storeConfig = store.toConfig("memory://test-list"); + + // Create a session and send a message to trigger event flushing + const session = await client.createSession({ + onPermissionRequest: approveAll, + sessionStore: storeConfig, + }); + await session.sendAndWait({ prompt: "What is 10 + 10?" }); + + // Wait for events to be flushed (debounced) + await vi.waitFor( + () => expect(store.hasSession(session.sessionId)).toBe(true), + { timeout: 10_000, interval: 200 }, + ); + + // List sessions using the same store + const sessions = await client.listSessions({ sessionStore: storeConfig }); + expect(store.calls.listSessions).toBeGreaterThan(0); + expect(sessions.some((s) => s.sessionId === session.sessionId)).toBe(true); + }); + + it("should reject resume with a different store descriptor", async () => { + const store1 = new InMemorySessionStore(); + const session = await client.createSession({ + onPermissionRequest: approveAll, + sessionStore: store1.toConfig("memory://store-alpha"), + }); + const sessionId = session.sessionId; + + // Don't disconnect — session is still active. + // Try to resume with a different descriptor. + const store2 = new InMemorySessionStore(); + await expect( + client.resumeSession(sessionId, { + onPermissionRequest: approveAll, + sessionStore: store2.toConfig("memory://store-beta"), + }), + ).rejects.toThrow(/Cannot change storage backend/); + }); +}); diff --git a/package-lock.json b/package-lock.json new file mode 100644 index 000000000..c3f458113 --- /dev/null +++ b/package-lock.json @@ -0,0 +1,6 @@ +{ + "name": "azure-otter", + "lockfileVersion": 3, + "requires": true, + "packages": {} +} diff --git a/test/snapshots/session_store/should_load_events_from_a_client_supplied_store_on_resume.yaml b/test/snapshots/session_store/should_load_events_from_a_client_supplied_store_on_resume.yaml new file mode 100644 index 000000000..4744667cd --- /dev/null +++ b/test/snapshots/session_store/should_load_events_from_a_client_supplied_store_on_resume.yaml @@ -0,0 +1,14 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: What is 50 + 50? + - role: assistant + content: 50 + 50 = 100 + - role: user + content: What is that times 3? + - role: assistant + content: 100 × 3 = 300 diff --git a/test/snapshots/session_store/should_persist_events_to_a_client_supplied_store.yaml b/test/snapshots/session_store/should_persist_events_to_a_client_supplied_store.yaml new file mode 100644 index 000000000..455652bfd --- /dev/null +++ b/test/snapshots/session_store/should_persist_events_to_a_client_supplied_store.yaml @@ -0,0 +1,10 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: What is 100 + 200? + - role: assistant + content: 100 + 200 = 300 diff --git a/test/snapshots/session_store/should_use_client_supplied_store_for_listsessions.yaml b/test/snapshots/session_store/should_use_client_supplied_store_for_listsessions.yaml new file mode 100644 index 000000000..3461d8aee --- /dev/null +++ b/test/snapshots/session_store/should_use_client_supplied_store_for_listsessions.yaml @@ -0,0 +1,10 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: What is 10 + 10? + - role: assistant + content: 10 + 10 = 20 From 1bea8933ffea4746069c0ca5f92d0450b0998b64 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 24 Mar 2026 12:38:07 +0000 Subject: [PATCH 2/5] feat: extend codegen to generate client API handler types Add support for the 'client' section of api.schema.json: - Generate typed handler interfaces (e.g., SessionStoreHandler) - Generate ClientApiHandlers composite interface - Generate registerClientApiHandlers() registration function - Handle methods with no result type (void return) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/generated/rpc.ts | 554 ++++++++++++++++++++++++++++++++++ scripts/codegen/typescript.ts | 126 +++++++- scripts/codegen/utils.ts | 3 +- 3 files changed, 675 insertions(+), 8 deletions(-) diff --git a/nodejs/src/generated/rpc.ts b/nodejs/src/generated/rpc.ts index e5ba9ad4c..80211c6c1 100644 --- a/nodejs/src/generated/rpc.ts +++ b/nodejs/src/generated/rpc.ts @@ -452,6 +452,274 @@ export interface SessionAgentDeselectParams { sessionId: string; } +export interface SessionAgentReloadResult { + /** + * Reloaded custom agents + */ + agents: { + /** + * Unique identifier of the custom agent + */ + name: string; + /** + * Human-readable display name + */ + displayName: string; + /** + * Description of the agent's purpose + */ + description: string; + }[]; +} + +export interface SessionAgentReloadParams { + /** + * Target session identifier + */ + sessionId: string; +} + +export interface SessionSkillsListResult { + /** + * Available skills + */ + skills: { + /** + * Unique identifier for the skill + */ + name: string; + /** + * Description of what the skill does + */ + description: string; + /** + * Source location type (e.g., project, personal, plugin) + */ + source: string; + /** + * Whether the skill can be invoked by the user as a slash command + */ + userInvocable: boolean; + /** + * Whether the skill is currently enabled + */ + enabled: boolean; + /** + * Absolute path to the skill file + */ + path?: string; + }[]; +} + +export interface SessionSkillsListParams { + /** + * Target session identifier + */ + sessionId: string; +} + +export interface SessionSkillsEnableResult {} + +export interface SessionSkillsEnableParams { + /** + * Target session identifier + */ + sessionId: string; + /** + * Name of the skill to enable + */ + name: string; +} + +export interface SessionSkillsDisableResult {} + +export interface SessionSkillsDisableParams { + /** + * Target session identifier + */ + sessionId: string; + /** + * Name of the skill to disable + */ + name: string; +} + +export interface SessionSkillsReloadResult {} + +export interface SessionSkillsReloadParams { + /** + * Target session identifier + */ + sessionId: string; +} + +export interface SessionMcpListResult { + /** + * Configured MCP servers + */ + servers: { + /** + * Server name (config key) + */ + name: string; + /** + * Connection status: connected, failed, pending, disabled, or not_configured + */ + status: "connected" | "failed" | "pending" | "disabled" | "not_configured"; + /** + * Configuration source: user, workspace, plugin, or builtin + */ + source?: string; + /** + * Error message if the server failed to connect + */ + error?: string; + }[]; +} + +export interface SessionMcpListParams { + /** + * Target session identifier + */ + sessionId: string; +} + +export interface SessionMcpEnableResult {} + +export interface SessionMcpEnableParams { + /** + * Target session identifier + */ + sessionId: string; + /** + * Name of the MCP server to enable + */ + serverName: string; +} + +export interface SessionMcpDisableResult {} + +export interface SessionMcpDisableParams { + /** + * Target session identifier + */ + sessionId: string; + /** + * Name of the MCP server to disable + */ + serverName: string; +} + +export interface SessionMcpReloadResult {} + +export interface SessionMcpReloadParams { + /** + * Target session identifier + */ + sessionId: string; +} + +export interface SessionPluginsListResult { + /** + * Installed plugins + */ + plugins: { + /** + * Plugin name + */ + name: string; + /** + * Marketplace the plugin came from + */ + marketplace: string; + /** + * Installed version + */ + version?: string; + /** + * Whether the plugin is currently enabled + */ + enabled: boolean; + }[]; +} + +export interface SessionPluginsListParams { + /** + * Target session identifier + */ + sessionId: string; +} + +export interface SessionExtensionsListResult { + /** + * Discovered extensions and their current status + */ + extensions: { + /** + * Source-qualified ID (e.g., 'project:my-ext', 'user:auth-helper') + */ + id: string; + /** + * Extension name (directory name) + */ + name: string; + /** + * Discovery source: project (.github/extensions/) or user (~/.copilot/extensions/) + */ + source: "project" | "user"; + /** + * Current status: running, disabled, failed, or starting + */ + status: "running" | "disabled" | "failed" | "starting"; + /** + * Process ID if the extension is running + */ + pid?: number; + }[]; +} + +export interface SessionExtensionsListParams { + /** + * Target session identifier + */ + sessionId: string; +} + +export interface SessionExtensionsEnableResult {} + +export interface SessionExtensionsEnableParams { + /** + * Target session identifier + */ + sessionId: string; + /** + * Source-qualified extension ID to enable + */ + id: string; +} + +export interface SessionExtensionsDisableResult {} + +export interface SessionExtensionsDisableParams { + /** + * Target session identifier + */ + sessionId: string; + /** + * Source-qualified extension ID to disable + */ + id: string; +} + +export interface SessionExtensionsReloadResult {} + +export interface SessionExtensionsReloadParams { + /** + * Target session identifier + */ + sessionId: string; +} + export interface SessionCompactionCompactResult { /** * Whether compaction completed successfully @@ -500,6 +768,135 @@ export interface SessionToolsHandlePendingToolCallParams { error?: string; } +export interface SessionCommandsHandlePendingCommandResult { + success: boolean; +} + +export interface SessionCommandsHandlePendingCommandParams { + /** + * Target session identifier + */ + sessionId: string; + /** + * Request ID from the command invocation event + */ + requestId: string; + /** + * Error message if the command handler failed + */ + error?: string; +} + +export interface SessionUiElicitationResult { + /** + * The user's response: accept (submitted), decline (rejected), or cancel (dismissed) + */ + action: "accept" | "decline" | "cancel"; + /** + * The form values submitted by the user (present when action is 'accept') + */ + content?: { + [k: string]: string | number | boolean | string[]; + }; +} + +export interface SessionUiElicitationParams { + /** + * Target session identifier + */ + sessionId: string; + /** + * Message describing what information is needed from the user + */ + message: string; + /** + * JSON Schema describing the form fields to present to the user + */ + requestedSchema: { + /** + * Schema type indicator (always 'object') + */ + type: "object"; + /** + * Form field definitions, keyed by field name + */ + properties: { + [k: string]: + | { + type: "string"; + title?: string; + description?: string; + enum: string[]; + enumNames?: string[]; + default?: string; + } + | { + type: "string"; + title?: string; + description?: string; + oneOf: { + const: string; + title: string; + }[]; + default?: string; + } + | { + type: "array"; + title?: string; + description?: string; + minItems?: number; + maxItems?: number; + items: { + type: "string"; + enum: string[]; + }; + default?: string[]; + } + | { + type: "array"; + title?: string; + description?: string; + minItems?: number; + maxItems?: number; + items: { + anyOf: { + const: string; + title: string; + }[]; + }; + default?: string[]; + } + | { + type: "boolean"; + title?: string; + description?: string; + default?: boolean; + } + | { + type: "string"; + title?: string; + description?: string; + minLength?: number; + maxLength?: number; + format?: "email" | "uri" | "date" | "date-time"; + default?: string; + } + | { + type: "number" | "integer"; + title?: string; + description?: string; + minimum?: number; + maximum?: number; + default?: number; + }; + }; + /** + * List of required field names + */ + required?: string[]; + }; +} + export interface SessionPermissionsHandlePendingPermissionRequestResult { /** * Whether the permission request was handled successfully @@ -559,6 +956,10 @@ export interface SessionLogParams { * When true, the message is transient and not persisted to the session event log on disk */ ephemeral?: boolean; + /** + * Optional URL the user can open in their browser for more details + */ + url?: string; } export interface SessionShellExecResult { @@ -609,6 +1010,78 @@ export interface SessionShellKillParams { signal?: "SIGTERM" | "SIGKILL" | "SIGINT"; } +export interface SessionStoreLoadResult { + /** + * All persisted events for the session, in order + */ + events: { + [k: string]: unknown; + }[]; +} + +export interface SessionStoreLoadParams { + /** + * The session to load events for + */ + sessionId: string; +} + +export interface SessionStoreAppendParams { + /** + * The session to append events to + */ + sessionId: string; + /** + * Events to append, in order + */ + events: { + [k: string]: unknown; + }[]; +} + +export interface SessionStoreTruncateResult { + /** + * Number of events removed + */ + eventsRemoved: number; + /** + * Number of events kept + */ + eventsKept: number; +} + +export interface SessionStoreTruncateParams { + /** + * The session to truncate + */ + sessionId: string; + /** + * Event ID marking the truncation boundary (excluded) + */ + upToEventId: string; +} + +export interface SessionStoreListResult { + sessions: { + sessionId: string; + /** + * ISO 8601 timestamp of last modification + */ + mtime: string; + /** + * ISO 8601 timestamp of creation + */ + birthtime: string; + }[]; +} + +export interface SessionStoreDeleteParams { + /** + * The session to delete + */ + sessionId: string; +} + /** Create typed server-scoped RPC methods (no session required). */ export function createServerRpc(connection: MessageConnection) { return { @@ -673,6 +1146,42 @@ export function createSessionRpc(connection: MessageConnection, sessionId: strin connection.sendRequest("session.agent.select", { sessionId, ...params }), deselect: async (): Promise => connection.sendRequest("session.agent.deselect", { sessionId }), + reload: async (): Promise => + connection.sendRequest("session.agent.reload", { sessionId }), + }, + skills: { + list: async (): Promise => + connection.sendRequest("session.skills.list", { sessionId }), + enable: async (params: Omit): Promise => + connection.sendRequest("session.skills.enable", { sessionId, ...params }), + disable: async (params: Omit): Promise => + connection.sendRequest("session.skills.disable", { sessionId, ...params }), + reload: async (): Promise => + connection.sendRequest("session.skills.reload", { sessionId }), + }, + mcp: { + list: async (): Promise => + connection.sendRequest("session.mcp.list", { sessionId }), + enable: async (params: Omit): Promise => + connection.sendRequest("session.mcp.enable", { sessionId, ...params }), + disable: async (params: Omit): Promise => + connection.sendRequest("session.mcp.disable", { sessionId, ...params }), + reload: async (): Promise => + connection.sendRequest("session.mcp.reload", { sessionId }), + }, + plugins: { + list: async (): Promise => + connection.sendRequest("session.plugins.list", { sessionId }), + }, + extensions: { + list: async (): Promise => + connection.sendRequest("session.extensions.list", { sessionId }), + enable: async (params: Omit): Promise => + connection.sendRequest("session.extensions.enable", { sessionId, ...params }), + disable: async (params: Omit): Promise => + connection.sendRequest("session.extensions.disable", { sessionId, ...params }), + reload: async (): Promise => + connection.sendRequest("session.extensions.reload", { sessionId }), }, compaction: { compact: async (): Promise => @@ -682,6 +1191,14 @@ export function createSessionRpc(connection: MessageConnection, sessionId: strin handlePendingToolCall: async (params: Omit): Promise => connection.sendRequest("session.tools.handlePendingToolCall", { sessionId, ...params }), }, + commands: { + handlePendingCommand: async (params: Omit): Promise => + connection.sendRequest("session.commands.handlePendingCommand", { sessionId, ...params }), + }, + ui: { + elicitation: async (params: Omit): Promise => + connection.sendRequest("session.ui.elicitation", { sessionId, ...params }), + }, permissions: { handlePendingPermissionRequest: async (params: Omit): Promise => connection.sendRequest("session.permissions.handlePendingPermissionRequest", { sessionId, ...params }), @@ -696,3 +1213,40 @@ export function createSessionRpc(connection: MessageConnection, sessionId: strin }, }; } + +/** + * Handler interface for the `sessionStore` client API group. + * Implement this to provide a custom sessionStore backend. + */ +export interface SessionStoreHandler { + load(params: SessionStoreLoadParams): Promise; + append(params: SessionStoreAppendParams): Promise; + truncate(params: SessionStoreTruncateParams): Promise; + list(): Promise; + delete(params: SessionStoreDeleteParams): Promise; +} + +/** All client API handler groups. Each group is optional. */ +export interface ClientApiHandlers { + sessionStore?: SessionStoreHandler; +} + +/** + * Register client API handlers on a JSON-RPC connection. + * The server calls these methods to delegate work to the client. + * Methods for unregistered groups will respond with a standard JSON-RPC + * method-not-found error. + */ +export function registerClientApiHandlers( + connection: MessageConnection, + handlers: ClientApiHandlers, +): void { + if (handlers.sessionStore) { + const h = handlers.sessionStore!; + connection.onRequest("sessionStore.load", (params: SessionStoreLoadParams) => h.load(params)); + connection.onRequest("sessionStore.append", (params: SessionStoreAppendParams) => h.append(params)); + connection.onRequest("sessionStore.truncate", (params: SessionStoreTruncateParams) => h.truncate(params)); + connection.onRequest("sessionStore.list", () => h.list()); + connection.onRequest("sessionStore.delete", (params: SessionStoreDeleteParams) => h.delete(params)); + } +} diff --git a/scripts/codegen/typescript.ts b/scripts/codegen/typescript.ts index 77c31019a..5a5d8c4f2 100644 --- a/scripts/codegen/typescript.ts +++ b/scripts/codegen/typescript.ts @@ -85,14 +85,17 @@ import type { MessageConnection } from "vscode-jsonrpc/node.js"; `); const allMethods = [...collectRpcMethods(schema.server || {}), ...collectRpcMethods(schema.session || {})]; + const clientMethods = collectRpcMethods(schema.client || {}); - for (const method of allMethods) { - const compiled = await compile(method.result, resultTypeName(method.rpcMethod), { - bannerComment: "", - additionalProperties: false, - }); - lines.push(compiled.trim()); - lines.push(""); + for (const method of [...allMethods, ...clientMethods]) { + if (method.result) { + const compiled = await compile(method.result, resultTypeName(method.rpcMethod), { + bannerComment: "", + additionalProperties: false, + }); + lines.push(compiled.trim()); + lines.push(""); + } if (method.params?.properties && Object.keys(method.params.properties).length > 0) { const paramsCompiled = await compile(method.params, paramsTypeName(method.rpcMethod), { @@ -125,6 +128,11 @@ import type { MessageConnection } from "vscode-jsonrpc/node.js"; lines.push(""); } + // Generate client API handler interfaces and registration function + if (schema.client) { + lines.push(...emitClientApiHandlers(schema.client)); + } + const outPath = await writeGeneratedFile("nodejs/src/generated/rpc.ts", lines.join("\n")); console.log(` ✓ ${outPath}`); } @@ -171,6 +179,110 @@ function emitGroup(node: Record, indent: string, isSession: boo return lines; } +// ── Client API Handler Generation ─────────────────────────────────────────── + +/** + * Collect client API methods grouped by their top-level namespace. + * Returns a map like: { sessionStore: [{ rpcMethod, params, result }, ...] } + */ +function collectClientGroups(node: Record): Map { + const groups = new Map(); + for (const [groupName, groupNode] of Object.entries(node)) { + if (typeof groupNode === "object" && groupNode !== null) { + groups.set(groupName, collectRpcMethods(groupNode as Record)); + } + } + return groups; +} + +/** + * Derive the handler method name from the full RPC method name. + * e.g., "sessionStore.load" → "load" + */ +function handlerMethodName(rpcMethod: string): string { + const parts = rpcMethod.split("."); + return parts[parts.length - 1]; +} + +/** + * Generate handler interfaces and a registration function for client API groups. + */ +function emitClientApiHandlers(clientSchema: Record): string[] { + const lines: string[] = []; + const groups = collectClientGroups(clientSchema); + + // Emit a handler interface per group + for (const [groupName, methods] of groups) { + const interfaceName = toPascalCase(groupName) + "Handler"; + lines.push(`/**`); + lines.push(` * Handler interface for the \`${groupName}\` client API group.`); + lines.push(` * Implement this to provide a custom ${groupName} backend.`); + lines.push(` */`); + lines.push(`export interface ${interfaceName} {`); + + for (const method of methods) { + const name = handlerMethodName(method.rpcMethod); + const hasParams = method.params?.properties && Object.keys(method.params.properties).length > 0; + const pType = hasParams ? paramsTypeName(method.rpcMethod) : ""; + const rType = method.result ? resultTypeName(method.rpcMethod) : "void"; + + const sig = hasParams + ? ` ${name}(params: ${pType}): Promise<${rType}>;` + : ` ${name}(): Promise<${rType}>;`; + lines.push(sig); + } + + lines.push(`}`); + lines.push(""); + } + + // Emit combined ClientApiHandlers type + lines.push(`/** All client API handler groups. Each group is optional. */`); + lines.push(`export interface ClientApiHandlers {`); + for (const [groupName] of groups) { + const interfaceName = toPascalCase(groupName) + "Handler"; + lines.push(` ${groupName}?: ${interfaceName};`); + } + lines.push(`}`); + lines.push(""); + + // Emit registration function + lines.push(`/**`); + lines.push(` * Register client API handlers on a JSON-RPC connection.`); + lines.push(` * The server calls these methods to delegate work to the client.`); + lines.push(` * Methods for unregistered groups will respond with a standard JSON-RPC`); + lines.push(` * method-not-found error.`); + lines.push(` */`); + lines.push(`export function registerClientApiHandlers(`); + lines.push(` connection: MessageConnection,`); + lines.push(` handlers: ClientApiHandlers,`); + lines.push(`): void {`); + + for (const [groupName, methods] of groups) { + lines.push(` if (handlers.${groupName}) {`); + lines.push(` const h = handlers.${groupName}!;`); + + for (const method of methods) { + const name = handlerMethodName(method.rpcMethod); + const hasParams = method.params?.properties && Object.keys(method.params.properties).length > 0; + const pType = hasParams ? paramsTypeName(method.rpcMethod) : ""; + + if (hasParams) { + lines.push(` connection.onRequest("${method.rpcMethod}", (params: ${pType}) => h.${name}(params));`); + } else { + lines.push(` connection.onRequest("${method.rpcMethod}", () => h.${name}());`); + } + } + + lines.push(` }`); + } + + lines.push(`}`); + lines.push(""); + + return lines; +} + // ── Main ──────────────────────────────────────────────────────────────────── async function generate(sessionSchemaPath?: string, apiSchemaPath?: string): Promise { diff --git a/scripts/codegen/utils.ts b/scripts/codegen/utils.ts index 88ca68de8..ddc6ea883 100644 --- a/scripts/codegen/utils.ts +++ b/scripts/codegen/utils.ts @@ -125,12 +125,13 @@ export async function writeGeneratedFile(relativePath: string, content: string): export interface RpcMethod { rpcMethod: string; params: JSONSchema7 | null; - result: JSONSchema7; + result: JSONSchema7 | null; } export interface ApiSchema { server?: Record; session?: Record; + client?: Record; } export function isRpcMethod(node: unknown): node is RpcMethod { From a0daf7cff0aab61ce566832affd4e7cf98544f7e Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 24 Mar 2026 12:43:28 +0000 Subject: [PATCH 3/5] refactor: use generated types for session store RPC handlers - Replace hand-written onRequest registrations with generated registerClientApiHandlers() and a routing SessionStoreHandler - Re-export SessionStoreHandler and ClientApiHandlers from index.ts - Fix listSessions concurrency: replace singleton listSessionsStore with a Map keyed by incrementing ID - Add cleanup of sessionStoreConfigs and listSessionsStores on stop() and forceStop() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 132 +++++++++++++++++++++---------------------- nodejs/src/index.ts | 2 + nodejs/src/types.ts | 21 +++++++ 3 files changed, 87 insertions(+), 68 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 708025be1..eb6a8eed3 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -23,7 +23,8 @@ import { StreamMessageReader, StreamMessageWriter, } from "vscode-jsonrpc/node.js"; -import { createServerRpc } from "./generated/rpc.js"; +import { createServerRpc, registerClientApiHandlers } from "./generated/rpc.js"; +import type { SessionStoreHandler } from "./generated/rpc.js"; import { getSdkProtocolVersion } from "./sdkProtocolVersion.js"; import { CopilotSession, NO_RESULT_PERMISSION_V2_ERROR } from "./session.js"; import { getTraceContext } from "./telemetry.js"; @@ -179,8 +180,12 @@ export class CopilotClient { private negotiatedProtocolVersion: number | null = null; /** Per-session store configs, keyed by sessionId. Used to route sessionStore.* RPC requests. */ private sessionStoreConfigs: Map = new Map(); - /** Temporary store used during listSessions() calls with a session store. */ - private listSessionsStore: SessionStoreConfig | null = null; + /** + * Stores used during listSessions() calls. Keyed by an incrementing ID to support concurrent calls. + * The latest registered store is used when the server calls sessionStore.list. + */ + private listSessionsStores: Map = new Map(); + private listSessionsStoreNextId = 0; /** * Typed server-scoped RPC methods. @@ -403,6 +408,8 @@ export class CopilotClient { } } this.sessions.clear(); + this.sessionStoreConfigs.clear(); + this.listSessionsStores.clear(); // Close connection if (this.connection) { @@ -487,6 +494,8 @@ export class CopilotClient { // Clear sessions immediately without trying to destroy them this.sessions.clear(); + this.sessionStoreConfigs.clear(); + this.listSessionsStores.clear(); // Force close connection if (this.connection) { @@ -1005,9 +1014,11 @@ export class CopilotClient { ? { filter: filterOrOptions as SessionListFilter } : undefined; - // Temporarily register the store for the duration of this RPC call + // Register the store for the duration of this RPC call (supports concurrent calls) + let storeId: number | undefined; if (options?.sessionStore) { - this.listSessionsStore = options.sessionStore; + storeId = this.listSessionsStoreNextId++; + this.listSessionsStores.set(storeId, options.sessionStore); } try { @@ -1037,8 +1048,8 @@ export class CopilotClient { context: s.context, })); } finally { - if (options?.sessionStore) { - this.listSessionsStore = null; + if (storeId !== undefined) { + this.listSessionsStores.delete(storeId); } } } @@ -1505,78 +1516,63 @@ export class CopilotClient { }): Promise<{ output?: unknown }> => await this.handleHooksInvoke(params) ); - // Register session store RPC handlers (server→client callbacks for event persistence) - this.connection.onRequest( - "sessionStore.load", - async (params: { sessionId: string }): Promise<{ events: SessionEvent[] }> => { - const store = this.sessionStoreConfigs.get(params.sessionId); - if (!store) { - throw new Error(`No session store registered for session ${params.sessionId}`); - } - const events = await store.onLoad(params.sessionId); - return { events }; - } - ); + // Register session store RPC handlers via generated registration function. + // The handler routes each call to the appropriate SessionStoreConfig based on sessionId. + registerClientApiHandlers(this.connection, { + sessionStore: this.createSessionStoreHandler(), + }); - this.connection.onRequest( - "sessionStore.append", - async (params: { sessionId: string; events: SessionEvent[] }): Promise => { - const store = this.sessionStoreConfigs.get(params.sessionId); - if (!store) { - throw new Error(`No session store registered for session ${params.sessionId}`); - } - await store.onAppend(params.events, params.sessionId); - } - ); + this.connection.onClose(() => { + this.state = "disconnected"; + }); - this.connection.onRequest( - "sessionStore.truncate", - async (params: { - sessionId: string; - upToEventId: string; - }): Promise<{ eventsRemoved: number; eventsKept: number }> => { - const store = this.sessionStoreConfigs.get(params.sessionId); - if (!store) { - throw new Error(`No session store registered for session ${params.sessionId}`); - } - return await store.onTruncate(params.upToEventId, params.sessionId); + this.connection.onError((_error) => { + this.state = "disconnected"; + }); + } + + /** + * Create a {@link SessionStoreHandler} that routes each RPC call to the + * appropriate {@link SessionStoreConfig} registered for the given session. + */ + private createSessionStoreHandler(): SessionStoreHandler { + const getStore = (sessionId: string): SessionStoreConfig => { + const store = this.sessionStoreConfigs.get(sessionId); + if (!store) { + throw new Error(`No session store registered for session ${sessionId}`); } - ); + return store; + }; - this.connection.onRequest( - "sessionStore.list", - async (): Promise<{ - sessions: Array<{ sessionId: string; mtime: string; birthtime: string }>; - }> => { + return { + load: async (params) => { + const store = getStore(params.sessionId); + const events = await store.onLoad(params.sessionId); + // Events are opaque on the wire but typed in the SDK consumer API + return { events: events as Record[] }; + }, + append: async (params) => { + const store = getStore(params.sessionId); + await store.onAppend(params.events as SessionEvent[], params.sessionId); + }, + truncate: async (params) => { + const store = getStore(params.sessionId); + return store.onTruncate(params.upToEventId, params.sessionId); + }, + list: async () => { // Use the most recently registered store for listing. - // In practice, listSessions() registers a temporary store before the RPC call. - const store = this.listSessionsStore; + const store = Array.from(this.listSessionsStores.values()).pop(); if (!store) { throw new Error("No session store registered for listing"); } const sessions = await store.onListSessions(); return { sessions }; - } - ); - - this.connection.onRequest( - "sessionStore.delete", - async (params: { sessionId: string }): Promise => { - const store = this.sessionStoreConfigs.get(params.sessionId); - if (!store) { - throw new Error(`No session store registered for session ${params.sessionId}`); - } + }, + delete: async (params) => { + const store = getStore(params.sessionId); await store.onDelete(params.sessionId); - } - ); - - this.connection.onClose(() => { - this.state = "disconnected"; - }); - - this.connection.onError((_error) => { - this.state = "disconnected"; - }); + }, + }; } private handleSessionEventNotification(notification: unknown): void { diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index 11a394430..98bbfe620 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -44,6 +44,8 @@ export type { SessionListFilter, SessionMetadata, SessionStoreConfig, + SessionStoreHandler, + ClientApiHandlers, SystemMessageAppendConfig, SystemMessageConfig, SystemMessageReplaceConfig, diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index aed6a450b..6bec9c622 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -10,6 +10,19 @@ import type { SessionEvent as GeneratedSessionEvent } from "./generated/session-events.js"; export type SessionEvent = GeneratedSessionEvent; +// Re-export generated client API types +export type { + SessionStoreHandler, + SessionStoreLoadParams, + SessionStoreLoadResult, + SessionStoreAppendParams, + SessionStoreTruncateParams, + SessionStoreTruncateResult, + SessionStoreListResult, + SessionStoreDeleteParams, + ClientApiHandlers, +} from "./generated/rpc.js"; + /** * Options for creating a CopilotClient */ @@ -1022,6 +1035,14 @@ export interface SessionContext { * the server routes all event persistence through these callbacks over RPC instead * of using its default file-based storage. */ +/** + * Configuration for a custom session store backend. + * + * The `descriptor` identifies the storage backend. The callback methods + * correspond to the generated {@link SessionStoreHandler} interface but + * use a consumer-friendly signature where `sessionId` is a plain argument + * rather than embedded in a params object. + */ export interface SessionStoreConfig { /** * Opaque descriptor identifying this storage backend. From adeb25d175a20707b0905ee89477064ccc142b96 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 24 Mar 2026 12:52:55 +0000 Subject: [PATCH 4/5] test: add E2E test for session store delete and remove unused import - Add test verifying onDelete is called when deleting a session - Remove unused getFinalAssistantMessage import Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/e2e/session_store.test.ts | 30 ++++++++++++++++++- ...call_ondelete_when_deleting_a_session.yaml | 10 +++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/snapshots/session_store/should_call_ondelete_when_deleting_a_session.yaml diff --git a/nodejs/test/e2e/session_store.test.ts b/nodejs/test/e2e/session_store.test.ts index 062a32803..26e8f6854 100644 --- a/nodejs/test/e2e/session_store.test.ts +++ b/nodejs/test/e2e/session_store.test.ts @@ -5,7 +5,6 @@ import { describe, expect, it, vi } from "vitest"; import { approveAll, type SessionEvent, type SessionStoreConfig } from "../../src/index.js"; import { createSdkTestContext } from "./harness/sdkTestContext.js"; -import { getFinalAssistantMessage } from "./harness/sdkTestHelper.js"; /** * In-memory session event store for testing. @@ -174,4 +173,33 @@ describe("Session Store", async () => { }), ).rejects.toThrow(/Cannot change storage backend/); }); + + it("should call onDelete when deleting a session", async () => { + const store = new InMemorySessionStore(); + const storeConfig = store.toConfig("memory://test-delete"); + + const session = await client.createSession({ + onPermissionRequest: approveAll, + sessionStore: storeConfig, + }); + const sessionId = session.sessionId; + + // Send a message to create some events + await session.sendAndWait({ prompt: "What is 7 + 7?" }); + + // Wait for events to flush + await vi.waitFor( + () => expect(store.hasSession(sessionId)).toBe(true), + { timeout: 10_000, interval: 200 }, + ); + + expect(store.calls.delete).toBe(0); + + // Delete the session + await client.deleteSession(sessionId); + + // Verify onDelete was called and the session was removed from our store + expect(store.calls.delete).toBeGreaterThan(0); + expect(store.hasSession(sessionId)).toBe(false); + }); }); diff --git a/test/snapshots/session_store/should_call_ondelete_when_deleting_a_session.yaml b/test/snapshots/session_store/should_call_ondelete_when_deleting_a_session.yaml new file mode 100644 index 000000000..2081e76aa --- /dev/null +++ b/test/snapshots/session_store/should_call_ondelete_when_deleting_a_session.yaml @@ -0,0 +1,10 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: What is 7 + 7? + - role: assistant + content: 7 + 7 = 14 From c4f70904c816681cf8a2474a40e0b3d77915e696 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Wed, 25 Mar 2026 14:14:03 +0000 Subject: [PATCH 5/5] 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> --- nodejs/src/client.ts | 155 ++++++++---------- nodejs/src/index.ts | 1 + nodejs/src/types.ts | 27 ++- nodejs/test/e2e/session_store.test.ts | 107 +++++++----- ..._support_multiple_concurrent_sessions.yaml | 8 +- ...st_sessions_from_the_storage_provider.yaml | 10 ++ ...ould_load_events_from_store_on_resume.yaml | 14 ++ ...etstorageprovider_when_sessions_exist.yaml | 34 ++++ 8 files changed, 213 insertions(+), 143 deletions(-) create mode 100644 test/snapshots/session_store/should_list_sessions_from_the_storage_provider.yaml create mode 100644 test/snapshots/session_store/should_load_events_from_store_on_resume.yaml create mode 100644 test/snapshots/session_store/should_reject_setstorageprovider_when_sessions_exist.yaml diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index eb6a8eed3..fdf691c15 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -178,14 +178,8 @@ export class CopilotClient { private _rpc: ReturnType | null = null; private processExitPromise: Promise | null = null; // Rejects when CLI process exits private negotiatedProtocolVersion: number | null = null; - /** Per-session store configs, keyed by sessionId. Used to route sessionStore.* RPC requests. */ - private sessionStoreConfigs: Map = new Map(); - /** - * Stores used during listSessions() calls. Keyed by an incrementing ID to support concurrent calls. - * The latest registered store is used when the server calls sessionStore.list. - */ - private listSessionsStores: Map = new Map(); - private listSessionsStoreNextId = 0; + /** Connection-level storage provider config set via setStorageProvider(). */ + private storageProviderConfig: SessionStoreConfig | null = null; /** * Typed server-scoped RPC methods. @@ -351,6 +345,40 @@ export class CopilotClient { } } + /** + * Declare this client as the session data storage provider. + * Must be called before creating any sessions. Only one client can be the + * storage provider at a time. + * + * @param config - The storage provider configuration with callbacks for persistence + * @throws Error if the client is not connected and autoStart is disabled + * + * @example + * ```typescript + * await client.setStorageProvider({ + * descriptor: "redis://localhost/sessions", + * onLoad: async (sessionId) => loadEventsFromRedis(sessionId), + * onAppend: async (events, sessionId) => appendEventsToRedis(events, sessionId), + * onTruncate: async (upToEventId, sessionId) => truncateEventsInRedis(upToEventId, sessionId), + * onListSessions: async () => listSessionsFromRedis(), + * onDelete: async (sessionId) => deleteSessionFromRedis(sessionId), + * }); + * ``` + */ + async setStorageProvider(config: SessionStoreConfig): Promise { + if (!this.connection) { + if (this.options.autoStart) { + await this.start(); + } else { + throw new Error("Client not connected. Call start() first."); + } + } + await this.connection!.sendRequest("session.setStorageProvider", { + descriptor: config.descriptor, + }); + this.storageProviderConfig = config; + } + /** * Stops the CLI server and closes all active sessions. * @@ -408,8 +436,7 @@ export class CopilotClient { } } this.sessions.clear(); - this.sessionStoreConfigs.clear(); - this.listSessionsStores.clear(); + this.storageProviderConfig = null; // Close connection if (this.connection) { @@ -494,8 +521,7 @@ export class CopilotClient { // Clear sessions immediately without trying to destroy them this.sessions.clear(); - this.sessionStoreConfigs.clear(); - this.listSessionsStores.clear(); + this.storageProviderConfig = null; // Force close connection if (this.connection) { @@ -603,12 +629,6 @@ export class CopilotClient { } this.sessions.set(sessionId, session); - // Register session store callbacks before the RPC so that - // store requests arriving during session creation are handled. - if (config.sessionStore) { - this.sessionStoreConfigs.set(sessionId, config.sessionStore); - } - try { const response = await this.connection!.sendRequest("session.create", { ...(await getTraceContext(this.onGetTraceContext)), @@ -640,9 +660,6 @@ export class CopilotClient { skillDirectories: config.skillDirectories, disabledSkills: config.disabledSkills, infiniteSessions: config.infiniteSessions, - sessionStore: config.sessionStore - ? { descriptor: config.sessionStore.descriptor } - : undefined, }); const { workspacePath } = response as { @@ -652,7 +669,6 @@ export class CopilotClient { session["_workspacePath"] = workspacePath; } catch (e) { this.sessions.delete(sessionId); - this.sessionStoreConfigs.delete(sessionId); throw e; } @@ -719,11 +735,6 @@ export class CopilotClient { } this.sessions.set(sessionId, session); - // Register session store callbacks before the RPC - if (config.sessionStore) { - this.sessionStoreConfigs.set(sessionId, config.sessionStore); - } - try { const response = await this.connection!.sendRequest("session.resume", { ...(await getTraceContext(this.onGetTraceContext)), @@ -756,9 +767,6 @@ export class CopilotClient { disabledSkills: config.disabledSkills, infiniteSessions: config.infiniteSessions, disableResume: config.disableResume, - sessionStore: config.sessionStore - ? { descriptor: config.sessionStore.descriptor } - : undefined, }); const { workspacePath } = response as { @@ -768,7 +776,6 @@ export class CopilotClient { session["_workspacePath"] = workspacePath; } catch (e) { this.sessions.delete(sessionId); - this.sessionStoreConfigs.delete(sessionId); throw e; } @@ -985,7 +992,6 @@ export class CopilotClient { // Remove from local sessions map if present this.sessions.delete(sessionId); - this.sessionStoreConfigs.delete(sessionId); } /** @@ -1008,50 +1014,34 @@ export class CopilotClient { // Support both plain filter and full options object const options: ListSessionsOptions | undefined = - filterOrOptions && "sessionStore" in filterOrOptions + filterOrOptions && "filter" in filterOrOptions ? (filterOrOptions as ListSessionsOptions) : filterOrOptions ? { filter: filterOrOptions as SessionListFilter } : undefined; - // Register the store for the duration of this RPC call (supports concurrent calls) - let storeId: number | undefined; - if (options?.sessionStore) { - storeId = this.listSessionsStoreNextId++; - this.listSessionsStores.set(storeId, options.sessionStore); - } - - try { - const response = await this.connection.sendRequest("session.list", { - filter: options?.filter, - sessionStore: options?.sessionStore - ? { descriptor: options.sessionStore.descriptor } - : undefined, - }); - const { sessions } = response as { - sessions: Array<{ - sessionId: string; - startTime: string; - modifiedTime: string; - summary?: string; - isRemote: boolean; - context?: SessionContext; - }>; - }; + const response = await this.connection.sendRequest("session.list", { + filter: options?.filter, + }); + const { sessions } = response as { + sessions: Array<{ + sessionId: string; + startTime: string; + modifiedTime: string; + summary?: string; + isRemote: boolean; + context?: SessionContext; + }>; + }; - return sessions.map((s) => ({ - sessionId: s.sessionId, - startTime: new Date(s.startTime), - modifiedTime: new Date(s.modifiedTime), - summary: s.summary, - isRemote: s.isRemote, - context: s.context, - })); - } finally { - if (storeId !== undefined) { - this.listSessionsStores.delete(storeId); - } - } + return sessions.map((s) => ({ + sessionId: s.sessionId, + startTime: new Date(s.startTime), + modifiedTime: new Date(s.modifiedTime), + summary: s.summary, + isRemote: s.isRemote, + context: s.context, + })); } /** @@ -1533,43 +1523,38 @@ export class CopilotClient { /** * Create a {@link SessionStoreHandler} that routes each RPC call to the - * appropriate {@link SessionStoreConfig} registered for the given session. + * connection-level storage provider set via {@link setStorageProvider}. */ private createSessionStoreHandler(): SessionStoreHandler { - const getStore = (sessionId: string): SessionStoreConfig => { - const store = this.sessionStoreConfigs.get(sessionId); - if (!store) { - throw new Error(`No session store registered for session ${sessionId}`); + const getStore = (): SessionStoreConfig => { + if (!this.storageProviderConfig) { + throw new Error("No storage provider configured. Call setStorageProvider() first."); } - return store; + return this.storageProviderConfig; }; return { load: async (params) => { - const store = getStore(params.sessionId); + const store = getStore(); const events = await store.onLoad(params.sessionId); // Events are opaque on the wire but typed in the SDK consumer API return { events: events as Record[] }; }, append: async (params) => { - const store = getStore(params.sessionId); + const store = getStore(); await store.onAppend(params.events as SessionEvent[], params.sessionId); }, truncate: async (params) => { - const store = getStore(params.sessionId); + const store = getStore(); return store.onTruncate(params.upToEventId, params.sessionId); }, list: async () => { - // Use the most recently registered store for listing. - const store = Array.from(this.listSessionsStores.values()).pop(); - if (!store) { - throw new Error("No session store registered for listing"); - } + const store = getStore(); const sessions = await store.onListSessions(); return { sessions }; }, delete: async (params) => { - const store = getStore(params.sessionId); + const store = getStore(); await store.onDelete(params.sessionId); }, }; diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index 98bbfe620..9ad9dc6f6 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -45,6 +45,7 @@ export type { SessionMetadata, SessionStoreConfig, SessionStoreHandler, + StorageProviderConfig, ClientApiHandlers, SystemMessageAppendConfig, SystemMessageConfig, diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 6bec9c622..75b0cf5eb 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -857,15 +857,6 @@ export interface SessionConfig { * but executes earlier in the lifecycle so no events are missed. */ onEvent?: SessionEventHandler; - - /** - * Session event store callbacks. When provided, the client handles all - * event persistence instead of the server's default file-based storage. - * - * The server calls back into these callbacks over RPC whenever it needs - * to load, append, truncate, list, or delete session events. - */ - sessionStore?: SessionStoreConfig; } /** @@ -894,8 +885,7 @@ export type ResumeSessionConfig = Pick< | "disabledSkills" | "infiniteSessions" | "onEvent" - | "sessionStore" -> & { +>& { /** * When true, skips emitting the session.resume event. * Useful for reconnecting to a session without triggering resume-related side effects. @@ -1046,9 +1036,7 @@ export interface SessionContext { export interface SessionStoreConfig { /** * Opaque descriptor identifying this storage backend. - * Used for UI display (e.g., `"redis://localhost/sessions"`) and to verify - * resume-consistency — two stores are considered equivalent if and only if - * their descriptors are identical strings. + * Used for UI display (e.g., `"redis://localhost/sessions"`). */ descriptor: string; @@ -1074,15 +1062,20 @@ export interface SessionStoreConfig { } /** - * Options for listing sessions, optionally with a session store. + * Options for listing sessions. */ export interface ListSessionsOptions { /** Filter to narrow down which sessions to return. */ filter?: SessionListFilter; - /** When provided, list sessions from this store instead of the server's default. */ - sessionStore?: SessionStoreConfig; } +/** + * Configuration for becoming the session data storage provider. + * Same shape as SessionStoreConfig — call client.setStorageProvider(config) + * to claim storage before creating sessions. + */ +export type StorageProviderConfig = SessionStoreConfig; + /** * Filter options for listing sessions */ diff --git a/nodejs/test/e2e/session_store.test.ts b/nodejs/test/e2e/session_store.test.ts index 26e8f6854..cff00d795 100644 --- a/nodejs/test/e2e/session_store.test.ts +++ b/nodejs/test/e2e/session_store.test.ts @@ -2,9 +2,10 @@ * Copyright (c) Microsoft Corporation. All rights reserved. *--------------------------------------------------------------------------------------------*/ -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it, onTestFinished, vi } from "vitest"; +import { CopilotClient } from "../../src/client.js"; import { approveAll, type SessionEvent, type SessionStoreConfig } from "../../src/index.js"; -import { createSdkTestContext } from "./harness/sdkTestContext.js"; +import { createSdkTestContext, isCI } from "./harness/sdkTestContext.js"; /** * In-memory session event store for testing. @@ -74,14 +75,15 @@ class InMemorySessionStore { } } -describe("Session Store", async () => { - const { copilotClient: client } = await createSdkTestContext(); +describe("Session Store (setStorageProvider)", async () => { + const { copilotClient: client, env } = await createSdkTestContext(); it("should persist events to a client-supplied store", async () => { const store = new InMemorySessionStore(); + await client.setStorageProvider(store.toConfig("memory://test-persist")); + const session = await client.createSession({ onPermissionRequest: approveAll, - sessionStore: store.toConfig("memory://test-persist"), }); // Send a message and wait for the response @@ -103,14 +105,23 @@ describe("Session Store", async () => { expect(store.calls.append).toBeGreaterThan(0); }); - it("should load events from a client-supplied store on resume", async () => { + it("should load events from store on resume", async () => { const store = new InMemorySessionStore(); const storeConfig = store.toConfig("memory://test-resume"); + const client2 = new CopilotClient({ + env, + logLevel: "error", + cliPath: process.env.COPILOT_CLI_PATH, + githubToken: isCI ? "fake-token-for-e2e-tests" : undefined, + }); + onTestFinished(() => client2.forceStop()); + + await client2.setStorageProvider(storeConfig); + // Create a session and send a message - const session1 = await client.createSession({ + const session1 = await client2.createSession({ onPermissionRequest: approveAll, - sessionStore: storeConfig, }); const sessionId = session1.sessionId; @@ -120,9 +131,8 @@ describe("Session Store", async () => { // Verify onLoad is called when resuming const loadCountBefore = store.calls.load; - const session2 = await client.resumeSession(sessionId, { + const session2 = await client2.resumeSession(sessionId, { onPermissionRequest: approveAll, - sessionStore: storeConfig, }); expect(store.calls.load).toBeGreaterThan(loadCountBefore); @@ -132,14 +142,23 @@ describe("Session Store", async () => { expect(msg2?.data.content).toContain("300"); }); - it("should use client-supplied store for listSessions", async () => { + it("should list sessions from the storage provider", async () => { const store = new InMemorySessionStore(); const storeConfig = store.toConfig("memory://test-list"); + const client3 = new CopilotClient({ + env, + logLevel: "error", + cliPath: process.env.COPILOT_CLI_PATH, + githubToken: isCI ? "fake-token-for-e2e-tests" : undefined, + }); + onTestFinished(() => client3.forceStop()); + + await client3.setStorageProvider(storeConfig); + // Create a session and send a message to trigger event flushing - const session = await client.createSession({ + const session = await client3.createSession({ onPermissionRequest: approveAll, - sessionStore: storeConfig, }); await session.sendAndWait({ prompt: "What is 10 + 10?" }); @@ -149,38 +168,28 @@ describe("Session Store", async () => { { timeout: 10_000, interval: 200 }, ); - // List sessions using the same store - const sessions = await client.listSessions({ sessionStore: storeConfig }); + // List sessions — should come from our store + const sessions = await client3.listSessions(); expect(store.calls.listSessions).toBeGreaterThan(0); expect(sessions.some((s) => s.sessionId === session.sessionId)).toBe(true); }); - it("should reject resume with a different store descriptor", async () => { - const store1 = new InMemorySessionStore(); - const session = await client.createSession({ - onPermissionRequest: approveAll, - sessionStore: store1.toConfig("memory://store-alpha"), - }); - const sessionId = session.sessionId; - - // Don't disconnect — session is still active. - // Try to resume with a different descriptor. - const store2 = new InMemorySessionStore(); - await expect( - client.resumeSession(sessionId, { - onPermissionRequest: approveAll, - sessionStore: store2.toConfig("memory://store-beta"), - }), - ).rejects.toThrow(/Cannot change storage backend/); - }); - it("should call onDelete when deleting a session", async () => { const store = new InMemorySessionStore(); const storeConfig = store.toConfig("memory://test-delete"); - const session = await client.createSession({ + const client4 = new CopilotClient({ + env, + logLevel: "error", + cliPath: process.env.COPILOT_CLI_PATH, + githubToken: isCI ? "fake-token-for-e2e-tests" : undefined, + }); + onTestFinished(() => client4.forceStop()); + + await client4.setStorageProvider(storeConfig); + + const session = await client4.createSession({ onPermissionRequest: approveAll, - sessionStore: storeConfig, }); const sessionId = session.sessionId; @@ -196,10 +205,34 @@ describe("Session Store", async () => { expect(store.calls.delete).toBe(0); // Delete the session - await client.deleteSession(sessionId); + await client4.deleteSession(sessionId); // Verify onDelete was called and the session was removed from our store expect(store.calls.delete).toBeGreaterThan(0); expect(store.hasSession(sessionId)).toBe(false); }); + + it("should reject setStorageProvider when sessions exist", async () => { + const client5 = new CopilotClient({ + env, + logLevel: "error", + cliPath: process.env.COPILOT_CLI_PATH, + githubToken: isCI ? "fake-token-for-e2e-tests" : undefined, + }); + onTestFinished(() => client5.forceStop()); + + // Create a session first (without storage provider) + const session = await client5.createSession({ + onPermissionRequest: approveAll, + }); + + // Send a message so the session is fully established + await session.sendAndWait({ prompt: "Hello" }); + + // Try setting a storage provider after sessions exist — should fail + const store = new InMemorySessionStore(); + await expect( + client5.setStorageProvider(store.toConfig("memory://too-late")), + ).rejects.toThrow(); + }); }); diff --git a/test/snapshots/session_lifecycle/should_support_multiple_concurrent_sessions.yaml b/test/snapshots/session_lifecycle/should_support_multiple_concurrent_sessions.yaml index cf55fcc17..fdb7ebca0 100644 --- a/test/snapshots/session_lifecycle/should_support_multiple_concurrent_sessions.yaml +++ b/test/snapshots/session_lifecycle/should_support_multiple_concurrent_sessions.yaml @@ -5,13 +5,13 @@ conversations: - role: system content: ${system} - role: user - content: What is 3+3? Reply with just the number. + content: What is 1+1? Reply with just the number. - role: assistant - content: "6" + content: "2" - messages: - role: system content: ${system} - role: user - content: What is 1+1? Reply with just the number. + content: What is 3+3? Reply with just the number. - role: assistant - content: "2" + content: "6" diff --git a/test/snapshots/session_store/should_list_sessions_from_the_storage_provider.yaml b/test/snapshots/session_store/should_list_sessions_from_the_storage_provider.yaml new file mode 100644 index 000000000..3461d8aee --- /dev/null +++ b/test/snapshots/session_store/should_list_sessions_from_the_storage_provider.yaml @@ -0,0 +1,10 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: What is 10 + 10? + - role: assistant + content: 10 + 10 = 20 diff --git a/test/snapshots/session_store/should_load_events_from_store_on_resume.yaml b/test/snapshots/session_store/should_load_events_from_store_on_resume.yaml new file mode 100644 index 000000000..4744667cd --- /dev/null +++ b/test/snapshots/session_store/should_load_events_from_store_on_resume.yaml @@ -0,0 +1,14 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: What is 50 + 50? + - role: assistant + content: 50 + 50 = 100 + - role: user + content: What is that times 3? + - role: assistant + content: 100 × 3 = 300 diff --git a/test/snapshots/session_store/should_reject_setstorageprovider_when_sessions_exist.yaml b/test/snapshots/session_store/should_reject_setstorageprovider_when_sessions_exist.yaml new file mode 100644 index 000000000..09d01531f --- /dev/null +++ b/test/snapshots/session_store/should_reject_setstorageprovider_when_sessions_exist.yaml @@ -0,0 +1,34 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: Hello + - role: assistant + content: >- + Hello! I'm GitHub Copilot CLI, powered by claude-sonnet-4.5. I'm here to help you with software engineering + tasks in this repository. + + + I can see you're working in the **copilot-sdk/nodejs** directory, which is part of a monorepo that implements + language SDKs for connecting to the Copilot CLI via JSON-RPC. + + + How can I help you today? I can: + + - Build, test, or lint the codebase + + - Add new SDK features or E2E tests + + - Debug issues or investigate bugs + + - Explore the codebase structure + + - Generate types or run other scripts + + - And more! + + + What would you like to work on?