Skip to content

Commit df202b5

Browse files
joshspicerCopilot
andcommitted
Address review: drop managedPermissions schema default and stale doc
The `managedPermissions` root config key follows omit semantics: it is absent when no restrictive enterprise policy applies. Its schema property carried a `default: {}`, which is seeded into stored root config values by `registerProviderConfiguration` and would let the launcher forward an empty (and misleading) `managedSettings` even with no policy. Remove the default so the key stays `undefined` and `managedSettings` is omitted entirely. Also remove the stale `chat.tools.eligibleForAutoApproval` reference from the `AgentHostManagedPermissionsConfigKey` doc comment (eligibility is no longer synthesized after the runtime-contract correction). Add a launcher test asserting neither create nor resume config carries `managedSettings` when the root value is unset. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c111c62a-eff3-4ff6-bb8a-8436a1b4babe
1 parent 99133df commit df202b5

2 files changed

Lines changed: 59 additions & 3 deletions

File tree

src/vs/platform/agentHost/common/agentHostSchema.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,10 @@ const managedPermissionsProperty = schemaProperty<IManagedPermissions>({
406406
items: { type: 'string', title: localize('agentHost.config.managedPermissions.rule', "Permission rule") },
407407
},
408408
},
409-
default: {},
409+
// Intentionally NO `default`: the key follows omit semantics. When no
410+
// restrictive policy applies the renderer forwards `undefined`, so
411+
// `getRootValue` stays `undefined` and the launcher omits `managedSettings`
412+
// entirely rather than forwarding an empty (and misleading) object.
410413
});
411414

412415
/**
@@ -557,8 +560,8 @@ export const GLOBAL_AUTO_APPROVE_SETTING_ID = 'chat.tools.global.autoApprove';
557560
/**
558561
* Root config key forwarded from the renderer holding the enterprise-policy-derived
559562
* {@link IManagedPermissions} object. Synthesized by VS Code exclusively from managed
560-
* (policy) values of `chat.tools.global.autoApprove`, `chat.tools.eligibleForAutoApproval`,
561-
* and `chat.tools.terminal.enableAutoApprove`, and forwarded to the runtime as
563+
* (policy) values of `chat.tools.global.autoApprove` and
564+
* `chat.tools.terminal.enableAutoApprove`, and forwarded to the runtime as
562565
* `managedSettings.permissions` at SDK session startup. Absent when no policy applies.
563566
*/
564567
export const AgentHostManagedPermissionsConfigKey = 'managedPermissions';

src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,59 @@ suite('CopilotSessionLauncher client identity', () => {
454454
await launcher.disposeByokProxyHandle();
455455
}
456456
});
457+
458+
test('omits managedSettings from create and resume configs when no policy is set', async () => {
459+
const createConfigs: Parameters<CopilotClient['createSession']>[0][] = [];
460+
const resumeConfigs: Parameters<CopilotClient['resumeSession']>[1][] = [];
461+
const session = {
462+
sessionId: 'session-1',
463+
on: () => () => { },
464+
disconnect: async () => { },
465+
} as unknown as CopilotSession;
466+
const client = {
467+
createSession: async (config: Parameters<CopilotClient['createSession']>[0]) => {
468+
createConfigs.push(config);
469+
return session;
470+
},
471+
resumeSession: async (_sessionId: string, config: Parameters<CopilotClient['resumeSession']>[1]) => {
472+
resumeConfigs.push(config);
473+
return session;
474+
},
475+
};
476+
// Root value unset (default launcher's getRootValue returns undefined).
477+
const launcher = createTestLauncher();
478+
const basePlan = {
479+
client,
480+
sessionId: 'session-1',
481+
workingDirectory: testWorkingDirectory,
482+
resolvedAgentName: undefined,
483+
snapshot: { tools: [], plugins: [], mcpServers: {} },
484+
activeClientToolSet: new ActiveClientToolSet(),
485+
shellManager: undefined,
486+
githubToken: undefined,
487+
};
488+
const createPlan: CopilotSessionLaunchPlan = { ...basePlan, kind: 'create', model: undefined };
489+
const resumePlan: CopilotSessionLaunchPlan = { ...basePlan, kind: 'resume', fallback: { model: undefined } };
490+
491+
const sessions = new DisposableStore();
492+
try {
493+
sessions.add(await launcher.launch(createPlan, testRuntime));
494+
sessions.add(await launcher.launch(resumePlan, testRuntime));
495+
496+
assert.deepStrictEqual({
497+
create: (createConfigs[0] as { managedSettings?: unknown }).managedSettings,
498+
createEnable: (createConfigs[0] as { enableManagedSettings?: boolean }).enableManagedSettings,
499+
resume: (resumeConfigs[0] as { managedSettings?: unknown }).managedSettings,
500+
}, {
501+
create: undefined,
502+
createEnable: true,
503+
resume: undefined,
504+
});
505+
} finally {
506+
sessions.dispose();
507+
await launcher.disposeByokProxyHandle();
508+
}
509+
});
457510
});
458511

459512
suite('CopilotSessionLauncher resume fallback', () => {

0 commit comments

Comments
 (0)