Skip to content

Commit d58befa

Browse files
Empty mode: wire safe defaults for ambient session knobs
Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a41b260 commit d58befa

2 files changed

Lines changed: 306 additions & 1 deletion

File tree

nodejs/src/client.ts

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ import type {
5454
ResumeSessionConfig,
5555
SectionTransformFn,
5656
SessionConfig,
57+
SessionConfigBase,
58+
SystemMessageConfig,
5759
SessionCapabilities,
5860
SessionEvent,
5961
SessionFsConfig,
@@ -886,11 +888,77 @@ export class CopilotClient {
886888
return { availableTools, excludedTools, toolFilterPrecedence: "excluded" };
887889
}
888890

891+
/** Mode-specific defaults spread under the caller's config (app values win). */
892+
private configDefaultsForMode(): Partial<SessionConfigBase> {
893+
if (this.options.mode === "empty") {
894+
return { enableSessionTelemetry: false };
895+
}
896+
return {};
897+
}
898+
899+
/**
900+
* Returns the systemMessage config to use, adjusted for the current mode.
901+
* In empty mode we ensure the environment_context section is removed
902+
* unless the app has already taken control of it; append mode is rejected
903+
* because it would leave environment info in the prompt.
904+
*/
905+
private getSystemMessageConfigForMode(
906+
supplied: SystemMessageConfig | undefined
907+
): SystemMessageConfig | undefined {
908+
if (this.options.mode !== "empty") return supplied;
909+
if (!supplied) {
910+
return {
911+
mode: "customize",
912+
sections: { environment_context: { action: "remove" } },
913+
};
914+
}
915+
switch (supplied.mode) {
916+
case "replace":
917+
return supplied;
918+
case "customize":
919+
if (supplied.sections?.environment_context) return supplied;
920+
return {
921+
...supplied,
922+
sections: {
923+
...supplied.sections,
924+
environment_context: { action: "remove" },
925+
},
926+
};
927+
case "append":
928+
case undefined:
929+
// Promote to customize so we can also strip environment_context.
930+
// The runtime appends `content` to additional instructions in
931+
// both customize and append modes, so the caller's text is
932+
// preserved verbatim.
933+
return {
934+
mode: "customize",
935+
content: supplied.content,
936+
sections: { environment_context: { action: "remove" } },
937+
};
938+
}
939+
}
940+
941+
/** Mode-specific options applied via session.options.update after create/resume. */
942+
private async updateSessionOptionsForMode(session: CopilotSession): Promise<void> {
943+
if (this.options.mode === "empty") {
944+
await session.rpc.options.update({
945+
skipCustomInstructions: true,
946+
customAgentsLocalOnly: true,
947+
coauthorEnabled: false,
948+
manageScheduleEnabled: false,
949+
installedPlugins: [],
950+
});
951+
}
952+
}
953+
889954
async createSession(config: SessionConfig): Promise<CopilotSession> {
890955
if (!this.connection) {
891956
await this.start();
892957
}
893958

959+
config = { ...this.configDefaultsForMode(), ...config };
960+
config.systemMessage = this.getSystemMessageConfigForMode(config.systemMessage);
961+
894962
const sessionId = config.sessionId ?? randomUUID();
895963

896964
// Create and register the session before issuing the RPC so that
@@ -998,6 +1066,8 @@ export class CopilotClient {
9981066
};
9991067
session["_workspacePath"] = workspacePath;
10001068
session.setCapabilities(capabilities);
1069+
1070+
await this.updateSessionOptionsForMode(session);
10011071
} catch (e) {
10021072
this.sessions.delete(sessionId);
10031073
throw e;
@@ -1063,7 +1133,9 @@ export class CopilotClient {
10631133
session.registerHooks(config.hooks);
10641134
}
10651135

1066-
// Extract transform callbacks from system message config before serialization.
1136+
config = { ...this.configDefaultsForMode(), ...config };
1137+
config.systemMessage = this.getSystemMessageConfigForMode(config.systemMessage);
1138+
10671139
const { wirePayload: wireSystemMessage, transformCallbacks } = extractTransformCallbacks(
10681140
config.systemMessage
10691141
);
@@ -1145,6 +1217,8 @@ export class CopilotClient {
11451217
session["_workspacePath"] = workspacePath;
11461218
session.setCapabilities(capabilities);
11471219
session.setOpenCanvases(openCanvases ?? []);
1220+
1221+
await this.updateSessionOptionsForMode(session);
11481222
} catch (e) {
11491223
this.sessions.delete(sessionId);
11501224
throw e;

nodejs/test/toolSet.test.ts

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ describe("Tool filter wiring", () => {
129129
if (method === "session.create" || method === "session.resume") {
130130
return { sessionId: params.sessionId };
131131
}
132+
if (method === "session.options.update") {
133+
return { success: true };
134+
}
132135
throw new Error(`Unexpected method: ${method}`);
133136
});
134137
return { client, spy };
@@ -209,3 +212,231 @@ describe("Tool filter wiring", () => {
209212
expect(payload.toolFilterPrecedence).toBe("excluded");
210213
});
211214
});
215+
216+
describe("Empty-mode safe defaults", () => {
217+
async function setupClient(mode: "empty" | "copilot-cli" = "empty") {
218+
const client = new CopilotClient({
219+
mode,
220+
baseDirectory: mode === "empty" ? "/tmp/copilot-test" : undefined,
221+
});
222+
await client.start();
223+
onTestFinished(() => client.forceStop());
224+
const spy = vi
225+
.spyOn((client as any).connection!, "sendRequest")
226+
.mockImplementation(async (method: string, params: any) => {
227+
if (method === "session.create" || method === "session.resume") {
228+
return { sessionId: params.sessionId };
229+
}
230+
if (method === "session.options.update") {
231+
return { success: true };
232+
}
233+
throw new Error(`Unexpected method: ${method}`);
234+
});
235+
return { client, spy };
236+
}
237+
238+
function createPayload(spy: ReturnType<typeof vi.spyOn>) {
239+
return (spy as any).mock.calls.find(([m]: [string]) => m === "session.create")![1] as any;
240+
}
241+
242+
function patchCall(spy: ReturnType<typeof vi.spyOn>) {
243+
return (spy as any).mock.calls.find(
244+
([m]: [string]) => m === "session.options.update"
245+
)![1] as any;
246+
}
247+
248+
it("forces enableSessionTelemetry=false when app didn't opt in", async () => {
249+
const { client, spy } = await setupClient();
250+
await client.createSession({
251+
onPermissionRequest: approveAll,
252+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
253+
});
254+
expect(createPayload(spy).enableSessionTelemetry).toBe(false);
255+
});
256+
257+
it("respects app-supplied enableSessionTelemetry=true override", async () => {
258+
const { client, spy } = await setupClient();
259+
await client.createSession({
260+
onPermissionRequest: approveAll,
261+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
262+
enableSessionTelemetry: true,
263+
});
264+
expect(createPayload(spy).enableSessionTelemetry).toBe(true);
265+
});
266+
267+
it("injects environment_context removal when app didn't pass systemMessage", async () => {
268+
const { client, spy } = await setupClient();
269+
await client.createSession({
270+
onPermissionRequest: approveAll,
271+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
272+
});
273+
const payload = createPayload(spy);
274+
expect(payload.systemMessage).toEqual({
275+
mode: "customize",
276+
sections: { environment_context: { action: "remove" } },
277+
});
278+
});
279+
280+
it("passes through app-supplied systemMessage in replace mode", async () => {
281+
const { client, spy } = await setupClient();
282+
await client.createSession({
283+
onPermissionRequest: approveAll,
284+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
285+
systemMessage: { mode: "replace", content: "you are a haiku bot" },
286+
});
287+
expect(createPayload(spy).systemMessage).toEqual({
288+
mode: "replace",
289+
content: "you are a haiku bot",
290+
});
291+
});
292+
293+
it("promotes append-mode systemMessage to customize with env_context removal in empty mode", async () => {
294+
const { client, spy } = await setupClient();
295+
await client.createSession({
296+
onPermissionRequest: approveAll,
297+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
298+
systemMessage: { mode: "append", content: "extra rules" },
299+
});
300+
expect(createPayload(spy).systemMessage).toEqual({
301+
mode: "customize",
302+
content: "extra rules",
303+
sections: { environment_context: { action: "remove" } },
304+
});
305+
});
306+
307+
it("promotes default-mode (append) systemMessage in empty mode", async () => {
308+
const { client, spy } = await setupClient();
309+
await client.createSession({
310+
onPermissionRequest: approveAll,
311+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
312+
systemMessage: { content: "extra rules" },
313+
});
314+
expect(createPayload(spy).systemMessage).toEqual({
315+
mode: "customize",
316+
content: "extra rules",
317+
sections: { environment_context: { action: "remove" } },
318+
});
319+
});
320+
321+
it("adds environment_context removal to customize mode when app didn't set it", async () => {
322+
const { client, spy } = await setupClient();
323+
await client.createSession({
324+
onPermissionRequest: approveAll,
325+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
326+
systemMessage: {
327+
mode: "customize",
328+
sections: { tool_use: { action: "remove" } },
329+
},
330+
});
331+
expect(createPayload(spy).systemMessage).toEqual({
332+
mode: "customize",
333+
sections: {
334+
tool_use: { action: "remove" },
335+
environment_context: { action: "remove" },
336+
},
337+
});
338+
});
339+
340+
it("leaves customize-mode systemMessage alone when app set environment_context", async () => {
341+
const { client, spy } = await setupClient();
342+
const supplied = {
343+
mode: "customize" as const,
344+
sections: {
345+
environment_context: { action: "replace" as const, content: "custom env" },
346+
},
347+
};
348+
await client.createSession({
349+
onPermissionRequest: approveAll,
350+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
351+
systemMessage: supplied,
352+
});
353+
expect(createPayload(spy).systemMessage).toEqual(supplied);
354+
});
355+
356+
it("sends session.options.update with safe defaults after session.create", async () => {
357+
const { client, spy } = await setupClient();
358+
await client.createSession({
359+
onPermissionRequest: approveAll,
360+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
361+
});
362+
const patch = patchCall(spy);
363+
expect(patch).toMatchObject({
364+
skipCustomInstructions: true,
365+
customAgentsLocalOnly: true,
366+
coauthorEnabled: false,
367+
manageScheduleEnabled: false,
368+
installedPlugins: [],
369+
});
370+
expect(patch.sessionId).toBeDefined();
371+
});
372+
373+
it("sends the patch AFTER session.create succeeds (order matters)", async () => {
374+
const { client, spy } = await setupClient();
375+
await client.createSession({
376+
onPermissionRequest: approveAll,
377+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
378+
});
379+
const methods = spy.mock.calls.map(([m]) => m);
380+
const createIdx = methods.indexOf("session.create");
381+
const patchIdx = methods.indexOf("session.options.update");
382+
expect(createIdx).toBeGreaterThanOrEqual(0);
383+
expect(patchIdx).toBeGreaterThan(createIdx);
384+
});
385+
386+
it("does NOT send patch or systemMessage override in copilot-cli mode", async () => {
387+
const { client, spy } = await setupClient("copilot-cli");
388+
await client.createSession({
389+
onPermissionRequest: approveAll,
390+
availableTools: ["builtin:bash"],
391+
});
392+
const methods = spy.mock.calls.map(([m]) => m);
393+
expect(methods).not.toContain("session.options.update");
394+
expect(createPayload(spy).systemMessage).toBeUndefined();
395+
expect(createPayload(spy).enableSessionTelemetry).toBeUndefined();
396+
});
397+
398+
it("tears the session down if the post-create patch fails", async () => {
399+
const client = new CopilotClient({ mode: "empty", baseDirectory: "/tmp/copilot-test" });
400+
await client.start();
401+
onTestFinished(() => client.forceStop());
402+
vi.spyOn((client as any).connection!, "sendRequest").mockImplementation(
403+
async (method: string, params: any) => {
404+
if (method === "session.create") return { sessionId: params.sessionId };
405+
if (method === "session.options.update") {
406+
throw new Error("update rejected");
407+
}
408+
throw new Error(`Unexpected method: ${method}`);
409+
}
410+
);
411+
await expect(
412+
client.createSession({
413+
onPermissionRequest: approveAll,
414+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
415+
})
416+
).rejects.toThrowError(/update rejected/);
417+
// Session must not remain registered after the failed patch.
418+
expect((client as any).sessions.size).toBe(0);
419+
});
420+
421+
it("also applies overrides on session.resume", async () => {
422+
const { client, spy } = await setupClient();
423+
// First create so we have a session id to resume.
424+
const session = await client.createSession({
425+
onPermissionRequest: approveAll,
426+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
427+
});
428+
spy.mockClear();
429+
await client.resumeSession(session.sessionId, {
430+
onPermissionRequest: approveAll,
431+
availableTools: new ToolSet().addBuiltIn(BuiltInTools.Isolated),
432+
});
433+
const resumePayload = spy.mock.calls.find(([m]) => m === "session.resume")![1] as any;
434+
expect(resumePayload.enableSessionTelemetry).toBe(false);
435+
expect(resumePayload.systemMessage).toEqual({
436+
mode: "customize",
437+
sections: { environment_context: { action: "remove" } },
438+
});
439+
const patch = spy.mock.calls.find(([m]) => m === "session.options.update")![1] as any;
440+
expect(patch.skipCustomInstructions).toBe(true);
441+
});
442+
});

0 commit comments

Comments
 (0)