Skip to content

Commit 534805a

Browse files
SteveSandersonMSSteveSandersonMS
committed
Move createSessionFsHandler onto SessionConfig
1 parent 47bf24e commit 534805a

3 files changed

Lines changed: 112 additions & 94 deletions

File tree

‎nodejs/src/client.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,13 @@ export class CopilotClient {
678678
}
679679
this.sessions.set(sessionId, session);
680680
if (this.sessionFsConfig) {
681-
session.clientSessionApis.sessionFs = this.sessionFsConfig.createHandler(session);
681+
if (config.createSessionFsHandler) {
682+
session.clientSessionApis.sessionFs = config.createSessionFsHandler(session);
683+
} else {
684+
throw new Error(
685+
"createSessionFsHandler is required in session config when sessionFs is enabled in client options."
686+
);
687+
}
682688
}
683689

684690
try {
@@ -803,7 +809,13 @@ export class CopilotClient {
803809
}
804810
this.sessions.set(sessionId, session);
805811
if (this.sessionFsConfig) {
806-
session.clientSessionApis.sessionFs = this.sessionFsConfig.createHandler(session);
812+
if (config.createSessionFsHandler) {
813+
session.clientSessionApis.sessionFs = config.createSessionFsHandler(session);
814+
} else {
815+
throw new Error(
816+
"createSessionFsHandler is required in session config when sessionFs is enabled in client options."
817+
);
818+
}
807819
}
808820

809821
try {

‎nodejs/src/types.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type { SessionEvent as GeneratedSessionEvent } from "./generated/session-
1111
export type SessionEvent = GeneratedSessionEvent;
1212

1313
import type { SessionFsHandler } from "./generated/rpc.js";
14-
export type { SessionFsHandler } from "./generated/rpc.js";
14+
import type { CopilotSession } from "./session.js";
1515

1616
/**
1717
* Options for creating a CopilotClient
@@ -623,7 +623,6 @@ export interface PermissionRequest {
623623
}
624624

625625
import type { SessionPermissionsHandlePendingPermissionRequestParams } from "./generated/rpc.js";
626-
import { CopilotSession } from "./session.js";
627626

628627
export type PermissionRequestResult =
629628
| SessionPermissionsHandlePendingPermissionRequestParams["result"]
@@ -1160,6 +1159,12 @@ export interface SessionConfig {
11601159
* but executes earlier in the lifecycle so no events are missed.
11611160
*/
11621161
onEvent?: SessionEventHandler;
1162+
1163+
/**
1164+
* Supplies a handler for session filesystem operations. This takes effect
1165+
* only if {@link CopilotClientOptions.sessionFs} is configured.
1166+
*/
1167+
createSessionFsHandler?: (session: CopilotSession) => SessionFsHandler;
11631168
}
11641169

11651170
/**
@@ -1189,6 +1194,7 @@ export type ResumeSessionConfig = Pick<
11891194
| "disabledSkills"
11901195
| "infiniteSessions"
11911196
| "onEvent"
1197+
| "createSessionFsHandler"
11921198
> & {
11931199
/**
11941200
* When true, skips emitting the session.resume event.
@@ -1349,11 +1355,6 @@ export interface SessionFsConfig {
13491355
* Path conventions used by this filesystem provider.
13501356
*/
13511357
conventions: "windows" | "linux";
1352-
1353-
/**
1354-
* Supplies a handler for session filesystem operations.
1355-
*/
1356-
createHandler: (session: CopilotSession) => SessionFsHandler;
13571358
}
13581359

13591360
/**

‎nodejs/test/e2e/session_fs.test.ts‎

Lines changed: 90 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,42 @@
22
* Copyright (c) Microsoft Corporation. All rights reserved.
33
*--------------------------------------------------------------------------------------------*/
44

5-
import { MemoryProvider } from "@platformatic/vfs";
5+
import { MemoryProvider, VirtualProvider } from "@platformatic/vfs";
66
import { describe, expect, it, onTestFinished } from "vitest";
77
import { CopilotClient } from "../../src/client.js";
8-
import { approveAll, defineTool, SessionEvent, type SessionFsConfig } from "../../src/index.js";
8+
import { SessionFsHandler } from "../../src/generated/rpc.js";
9+
import {
10+
approveAll,
11+
CopilotSession,
12+
defineTool,
13+
SessionEvent,
14+
type SessionFsConfig,
15+
} from "../../src/index.js";
916
import { createSdkTestContext } from "./harness/sdkTestContext.js";
1017

18+
process.env.COPILOT_CLI_PATH =
19+
"c:\\Users\\stevesa\\.copilot\\worktrees\\copilot-agent-runtime\\amber-aura\\dist-cli\\index.js";
20+
1121
describe("Session Fs", async () => {
1222
// Single provider for the describe block — session IDs are unique per test,
1323
// so no cross-contamination between tests.
1424
const provider = new MemoryProvider();
15-
const { config } = createMemorySessionFs("/projects/test", "/session-state", provider);
25+
const createSessionFsHandler = (session: CopilotSession) =>
26+
createTestSessionFsHandler(session, provider);
1627

1728
// Helpers to build session-namespaced paths for direct provider assertions
1829
const p = (sessionId: string, path: string) =>
1930
`/${sessionId}${path.startsWith("/") ? path : "/" + path}`;
2031

2132
const { copilotClient: client, env } = await createSdkTestContext({
22-
copilotClientOptions: {
23-
sessionFs: config,
24-
},
33+
copilotClientOptions: { sessionFs: sessionFsConfig },
2534
});
2635

2736
it("should route file operations through the session fs provider", async () => {
28-
const session = await client.createSession({ onPermissionRequest: approveAll });
37+
const session = await client.createSession({
38+
onPermissionRequest: approveAll,
39+
createSessionFsHandler,
40+
});
2941

3042
const msg = await session.sendAndWait({ prompt: "What is 100 + 200?" });
3143
expect(msg?.data.content).toContain("300");
@@ -37,7 +49,10 @@ describe("Session Fs", async () => {
3749
});
3850

3951
it("should load session data from fs provider on resume", async () => {
40-
const session1 = await client.createSession({ onPermissionRequest: approveAll });
52+
const session1 = await client.createSession({
53+
onPermissionRequest: approveAll,
54+
createSessionFsHandler,
55+
});
4156
const sessionId = session1.sessionId;
4257

4358
const msg = await session1.sendAndWait({ prompt: "What is 50 + 50?" });
@@ -49,6 +64,7 @@ describe("Session Fs", async () => {
4964

5065
const session2 = await client.resumeSession(sessionId, {
5166
onPermissionRequest: approveAll,
67+
createSessionFsHandler,
5268
});
5369

5470
// Send another message to verify the session is functional after resume
@@ -62,23 +78,18 @@ describe("Session Fs", async () => {
6278
useStdio: false, // Use TCP so we can connect from a second client
6379
env,
6480
});
65-
await client.createSession({ onPermissionRequest: approveAll });
81+
await client.createSession({ onPermissionRequest: approveAll, createSessionFsHandler });
6682

6783
// Get the port the first client's runtime is listening on
6884
const port = (client as unknown as { actualPort: number }).actualPort;
6985

7086
// Second client tries to connect with a session fs — should fail
7187
// because sessions already exist on the runtime.
72-
const { config: config2 } = createMemorySessionFs(
73-
"/projects/test",
74-
"/session-state",
75-
new MemoryProvider()
76-
);
7788
const client2 = new CopilotClient({
7889
env,
7990
logLevel: "error",
8091
cliUrl: `localhost:${port}`,
81-
sessionFs: config2,
92+
sessionFs: sessionFsConfig,
8293
});
8394
onTestFinished(() => client2.forceStop());
8495

@@ -89,6 +100,7 @@ describe("Session Fs", async () => {
89100
const suppliedFileContent = "x".repeat(100_000);
90101
const session = await client.createSession({
91102
onPermissionRequest: approveAll,
103+
createSessionFsHandler,
92104
tools: [
93105
defineTool("get_big_string", {
94106
description: "Returns a large string",
@@ -132,78 +144,71 @@ function findToolName(messages: SessionEvent[], toolCallId: string): string | un
132144
}
133145
}
134146

135-
/**
136-
* Builds a SessionFsConfig backed by a @platformatic/vfs MemoryProvider.
137-
* Each sessionId is namespaced under `/<sessionId>/` in the provider's tree.
138-
* Tests can assert directly against the returned MemoryProvider instance.
139-
*/
140-
function createMemorySessionFs(
141-
initialCwd: string,
142-
sessionStatePath: string,
143-
provider: MemoryProvider
144-
): { config: SessionFsConfig } {
147+
const sessionFsConfig: SessionFsConfig = {
148+
initialCwd: "/",
149+
sessionStatePath: "/session-state",
150+
conventions: "linux",
151+
};
152+
153+
function createTestSessionFsHandler(
154+
session: CopilotSession,
155+
provider: VirtualProvider
156+
): SessionFsHandler {
145157
const sp = (sessionId: string, path: string) =>
146158
`/${sessionId}${path.startsWith("/") ? path : "/" + path}`;
147159

148-
const config: SessionFsConfig = {
149-
initialCwd,
150-
sessionStatePath,
151-
conventions: "linux",
152-
createHandler: (session) => ({
153-
readFile: async ({ path }) => {
154-
const content = await provider.readFile(sp(session.sessionId, path), "utf8");
155-
return { content: content as string };
156-
},
157-
writeFile: async ({ path, content }) => {
158-
await provider.writeFile(sp(session.sessionId, path), content);
159-
},
160-
appendFile: async ({ path, content }) => {
161-
await provider.appendFile(sp(session.sessionId, path), content);
162-
},
163-
exists: async ({ path }) => {
164-
return { exists: await provider.exists(sp(session.sessionId, path)) };
165-
},
166-
stat: async ({ path }) => {
167-
const st = await provider.stat(sp(session.sessionId, path));
168-
return {
169-
isFile: st.isFile(),
170-
isDirectory: st.isDirectory(),
171-
size: st.size,
172-
mtime: new Date(st.mtimeMs).toISOString(),
173-
birthtime: new Date(st.birthtimeMs).toISOString(),
174-
};
175-
},
176-
mkdir: async ({ path, recursive, mode }) => {
177-
await provider.mkdir(sp(session.sessionId, path), {
178-
recursive: recursive ?? false,
179-
mode,
180-
});
181-
},
182-
readdir: async ({ path }) => {
183-
const entries = await provider.readdir(sp(session.sessionId, path));
184-
return { entries: entries as string[] };
185-
},
186-
readdirWithTypes: async ({ path }) => {
187-
const names = (await provider.readdir(sp(session.sessionId, path))) as string[];
188-
const entries = await Promise.all(
189-
names.map(async (name) => {
190-
const st = await provider.stat(sp(session.sessionId, `${path}/${name}`));
191-
return {
192-
name,
193-
type: st.isDirectory() ? ("directory" as const) : ("file" as const),
194-
};
195-
})
196-
);
197-
return { entries };
198-
},
199-
rm: async ({ path }) => {
200-
await provider.unlink(sp(session.sessionId, path));
201-
},
202-
rename: async ({ src, dest }) => {
203-
await provider.rename(sp(session.sessionId, src), sp(session.sessionId, dest));
204-
},
205-
}),
160+
return {
161+
readFile: async ({ path }) => {
162+
const content = await provider.readFile(sp(session.sessionId, path), "utf8");
163+
return { content: content as string };
164+
},
165+
writeFile: async ({ path, content }) => {
166+
await provider.writeFile(sp(session.sessionId, path), content);
167+
},
168+
appendFile: async ({ path, content }) => {
169+
await provider.appendFile(sp(session.sessionId, path), content);
170+
},
171+
exists: async ({ path }) => {
172+
return { exists: await provider.exists(sp(session.sessionId, path)) };
173+
},
174+
stat: async ({ path }) => {
175+
const st = await provider.stat(sp(session.sessionId, path));
176+
return {
177+
isFile: st.isFile(),
178+
isDirectory: st.isDirectory(),
179+
size: st.size,
180+
mtime: new Date(st.mtimeMs).toISOString(),
181+
birthtime: new Date(st.birthtimeMs).toISOString(),
182+
};
183+
},
184+
mkdir: async ({ path, recursive, mode }) => {
185+
await provider.mkdir(sp(session.sessionId, path), {
186+
recursive: recursive ?? false,
187+
mode,
188+
});
189+
},
190+
readdir: async ({ path }) => {
191+
const entries = await provider.readdir(sp(session.sessionId, path));
192+
return { entries: entries as string[] };
193+
},
194+
readdirWithTypes: async ({ path }) => {
195+
const names = (await provider.readdir(sp(session.sessionId, path))) as string[];
196+
const entries = await Promise.all(
197+
names.map(async (name) => {
198+
const st = await provider.stat(sp(session.sessionId, `${path}/${name}`));
199+
return {
200+
name,
201+
type: st.isDirectory() ? ("directory" as const) : ("file" as const),
202+
};
203+
})
204+
);
205+
return { entries };
206+
},
207+
rm: async ({ path }) => {
208+
await provider.unlink(sp(session.sessionId, path));
209+
},
210+
rename: async ({ src, dest }) => {
211+
await provider.rename(sp(session.sessionId, src), sp(session.sessionId, dest));
212+
},
206213
};
207-
208-
return { config };
209214
}

0 commit comments

Comments
 (0)