Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
galdawave merged 4 commits into from
Jun 16, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0646cb6
fix(config): migrate coreTools setting to tools.core
galdawave Jun 15, 2026
4d91e11
fix(a2a-server): resolve unsafe assignment lint error for coreTools
galdawave Jun 15, 2026
6b2092d
refactor(config): remove deprecated tools fallback logic and fix tests
galdawave Jun 15, 2026
d7fa933
chore: fix lint
galdawave Jun 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions .github/workflows/community-report.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,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.
Expand Down
10 changes: 6 additions & 4 deletions .github/workflows/gemini-automated-issue-dedup.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,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"
Expand Down
10 changes: 6 additions & 4 deletions .github/workflows/gemini-automated-issue-triage.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,12 @@ jobs:
"enabled": true,
"target": "gcp"
},
"coreTools": [
"run_shell_command(echo)",
"read_file"
]
"tools": {
"core": [
"run_shell_command(echo)",
"read_file"
]
}
}
prompt: |-
## Role
Expand Down
8 changes: 5 additions & 3 deletions .github/workflows/gemini-scheduled-issue-dedup.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,11 @@ jobs:
}
},
"maxSessionTurns": 25,
"coreTools": [
"run_shell_command(echo)"
],
"tools": {
"core": [
"run_shell_command(echo)"
]
},
"telemetry": {
"enabled": true,
"target": "gcp"
Expand Down
24 changes: 14 additions & 10 deletions .github/workflows/gemini-scheduled-issue-triage.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,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"
Expand Down Expand Up @@ -313,12 +315,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"
Expand Down
2 changes: 1 addition & 1 deletion docs/cli/enterprise.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.**

Expand Down
75 changes: 2 additions & 73 deletions packages/a2a-server/src/config/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,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'],
Expand All @@ -312,7 +311,7 @@ describe('loadConfig', () => {
tools: {
core: ['v2-core'],
exclude: ['v2-exclude'],
allowed: ['v1-allowed'],
allowed: ['v2-allowed'],
},
mcpServers: settings.mcpServers,
policyPaths: settings.policyPaths,
Expand All @@ -323,64 +322,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: {
Expand All @@ -395,21 +339,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: {
Expand Down
12 changes: 6 additions & 6 deletions packages/a2a-server/src/config/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,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,
Expand All @@ -92,9 +92,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,
Expand Down
6 changes: 4 additions & 2 deletions packages/a2a-server/src/config/settings.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'],

@gemini-code-assist gemini-code-assist Bot Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Since settings.tools.core is now used instead of coreTools, the assertion on line 114 of this file should be updated to use the new nested configuration format. This aligns with removing backward compatibility logic for deprecated settings.

References
  1. Instead of maintaining backward compatibility for deprecated or unshipped configuration settings, remove them and their associated logic entirely to avoid confusion.

},
mcpServers: {
server1: {
command: 'cmd',
Expand All @@ -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);
});
Expand Down
3 changes: 0 additions & 3 deletions packages/a2a-server/src/config/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[];

@gemini-code-assist gemini-code-assist Bot Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Removing coreTools from the Settings interface is correct as part of the migration, but there are still references to settings.coreTools in the codebase that will now cause TypeScript compilation errors. Instead of maintaining backward compatibility for deprecated configuration settings, we should remove them and their associated logic entirely to avoid confusion.

References
  1. Instead of maintaining backward compatibility for deprecated or unshipped configuration settings, remove them and their associated logic entirely to avoid confusion.

excludeTools?: string[];
allowedTools?: string[];
tools?: {
allowed?: string[];
exclude?: string[];
Expand Down
Loading