fix(config): migrate coreTools setting to tools.core (#27947)

This commit is contained in:
Gal Zahavi
2026-06-16 14:34:08 -07:00
committed by GitHub
parent 97455e5d43
commit 926f3d9b95
10 changed files with 52 additions and 112 deletions
+8 -6
View File
@@ -183,12 +183,14 @@ jobs:
use_gemini_code_assist: '${{ vars.GOOGLE_GENAI_USE_GCA }}'
settings: |-
{
"coreTools": [
"run_shell_command(gh issue list)",
"run_shell_command(gh pr list)",
"run_shell_command(gh search issues)",
"run_shell_command(gh search prs)"
]
"tools": {
"core": [
"run_shell_command(gh issue list)",
"run_shell_command(gh pr list)",
"run_shell_command(gh search issues)",
"run_shell_command(gh search prs)"
]
}
}
prompt: |-
You are a helpful assistant that analyzes community contribution reports.
@@ -109,10 +109,12 @@ jobs:
}
},
"maxSessionTurns": 25,
"coreTools": [
"run_shell_command(echo)",
"run_shell_command(gh issue view)"
],
"tools": {
"core": [
"run_shell_command(echo)",
"run_shell_command(gh issue view)"
]
},
"telemetry": {
"enabled": true,
"target": "gcp"
@@ -170,10 +170,12 @@ jobs:
"enabled": true,
"target": "gcp"
},
"coreTools": [
"run_shell_command(echo)",
"read_file"
]
"tools": {
"core": [
"run_shell_command(echo)",
"read_file"
]
}
}
prompt: |-
## Role
@@ -88,9 +88,11 @@ jobs:
}
},
"maxSessionTurns": 25,
"coreTools": [
"run_shell_command(echo)"
],
"tools": {
"core": [
"run_shell_command(echo)"
]
},
"telemetry": {
"enabled": true,
"target": "gcp"
@@ -192,10 +192,12 @@ jobs:
settings: |-
{
"maxSessionTurns": 25,
"coreTools": [
"run_shell_command(echo)",
"read_file"
],
"tools": {
"core": [
"run_shell_command(echo)",
"read_file"
]
},
"telemetry": {
"enabled": false,
"target": "gcp"
@@ -315,12 +317,14 @@ jobs:
settings: |-
{
"maxSessionTurns": 30,
"coreTools": [
"run_shell_command(echo)",
"grep_search",
"glob",
"read_file"
],
"tools": {
"core": [
"run_shell_command(echo)",
"grep_search",
"glob",
"read_file"
]
},
"telemetry": {
"enabled": false,
"target": "gcp"
+1 -1
View File
@@ -285,7 +285,7 @@ environment to a blocklist.
<!-- prettier-ignore -->
> [!WARNING]
> Blocklisting with `excludeTools` is less secure than
> allowlisting with `coreTools`, as it relies on blocking known-bad commands,
> allowlisting with `tools.core`, as it relies on blocking known-bad commands,
> and clever users may find ways to bypass simple string-based blocks.
> **Allowlisting is the recommended approach.**
+2 -73
View File
@@ -291,9 +291,8 @@ describe('loadConfig', () => {
});
describe('policy engine configuration', () => {
it('should merge V1 and V2 tool settings into policySettings', async () => {
it('should map tool settings into policySettings', async () => {
const settings: Settings = {
allowedTools: ['v1-allowed'],
tools: {
allowed: ['v2-allowed'],
exclude: ['v2-exclude'],
@@ -313,7 +312,7 @@ describe('loadConfig', () => {
tools: {
core: ['v2-core'],
exclude: ['v2-exclude'],
allowed: ['v1-allowed'],
allowed: ['v2-allowed'],
},
mcpServers: settings.mcpServers,
policyPaths: settings.policyPaths,
@@ -324,64 +323,9 @@ describe('loadConfig', () => {
true,
);
});
it('should use V2 tool settings when V1 is missing', async () => {
const settings: Settings = {
tools: {
allowed: ['v2-allowed'],
},
};
await loadConfig(settings, mockExtensionLoader, taskId);
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
expect.objectContaining({
tools: expect.objectContaining({
allowed: ['v2-allowed'],
}),
}),
ApprovalMode.DEFAULT,
undefined,
true,
);
});
it('should use V1 tool settings when V2 is also present', async () => {
const settings: Settings = {
allowedTools: ['v1-allowed'],
tools: {
allowed: ['v2-allowed'],
},
};
await loadConfig(settings, mockExtensionLoader, taskId);
expect(createPolicyEngineConfig).toHaveBeenCalledWith(
expect.objectContaining({
tools: expect.objectContaining({
allowed: ['v1-allowed'],
}),
}),
ApprovalMode.DEFAULT,
undefined,
true,
);
});
});
describe('tool configuration', () => {
it('should pass V1 allowedTools to Config properly', async () => {
const settings: Settings = {
allowedTools: ['shell', 'edit'],
};
await loadConfig(settings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith(
expect.objectContaining({
allowedTools: ['shell', 'edit'],
}),
);
});
it('should pass V2 tools.allowed to Config properly', async () => {
const settings: Settings = {
tools: {
@@ -396,21 +340,6 @@ describe('loadConfig', () => {
);
});
it('should prefer V1 allowedTools over V2 tools.allowed if both present', async () => {
const settings: Settings = {
allowedTools: ['v1-tool'],
tools: {
allowed: ['v2-tool'],
},
};
await loadConfig(settings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith(
expect.objectContaining({
allowedTools: ['v1-tool'],
}),
);
});
it('should pass enableAgents to Config constructor', async () => {
const settings: Settings = {
experimental: {
+6 -6
View File
@@ -69,9 +69,9 @@ export async function loadConfig(
const policySettings: PolicySettings = {
mcpServers: settings.mcpServers,
tools: {
core: settings.coreTools || settings.tools?.core,
exclude: settings.excludeTools || settings.tools?.exclude,
allowed: settings.allowedTools || settings.tools?.allowed,
core: settings.tools?.core,
exclude: settings.tools?.exclude,
allowed: settings.tools?.allowed,
},
policyPaths: settings.policyPaths,
adminPolicyPaths: settings.adminPolicyPaths,
@@ -94,9 +94,9 @@ export async function loadConfig(
debugMode: process.env['DEBUG'] === 'true' || false,
question: '', // Not used in server mode directly like CLI
coreTools: settings.coreTools || settings.tools?.core || undefined,
excludeTools: settings.excludeTools || settings.tools?.exclude || undefined,
allowedTools: settings.allowedTools || settings.tools?.allowed || undefined,
coreTools: settings.tools?.core || undefined,
excludeTools: settings.tools?.exclude || undefined,
allowedTools: settings.tools?.allowed || undefined,
showMemoryUsage: settings.showMemoryUsage || false,
approvalMode,
policyEngineConfig,
@@ -94,7 +94,9 @@ describe('loadSettings', () => {
it('should load other top-level settings correctly', () => {
const settings = {
showMemoryUsage: true,
coreTools: ['tool1', 'tool2'],
tools: {
core: ['tool1', 'tool2'],
},
mcpServers: {
server1: {
command: 'cmd',
@@ -109,7 +111,7 @@ describe('loadSettings', () => {
const result = loadSettings(mockWorkspaceDir);
expect(result.showMemoryUsage).toBe(true);
expect(result.coreTools).toEqual(['tool1', 'tool2']);
expect(result.tools?.core).toEqual(['tool1', 'tool2']);
expect(result.mcpServers).toHaveProperty('server1');
expect(result.fileFiltering?.respectGitIgnore).toBe(true);
});
@@ -27,9 +27,6 @@ export const USER_SETTINGS_PATH = path.join(USER_SETTINGS_DIR, 'settings.json');
// similar to how packages/cli/src/config/settings.ts handles it.
export interface Settings {
mcpServers?: Record<string, MCPServerConfig>;
coreTools?: string[];
excludeTools?: string[];
allowedTools?: string[];
tools?: {
allowed?: string[];
exclude?: string[];