Skip to content

Commit 1585eb0

Browse files
bugerclaude
andauthored
fix: Respect allowedTools configuration in validTools array (#294)
The `disableTools: true` option was not fully respected - tool definitions were correctly filtered from the system message, but the agentic loop still accepted and executed tool calls because the `validTools` array was hardcoded and didn't respect the `allowedTools` configuration. Changes: - Replace hardcoded `validTools` array with dynamic filtering using `allowedTools.isEnabled()` pattern (same as `getSystemMessage()`) - Edit tools now require both `allowEdit` flag AND `allowedTools` permission - Bash tool requires both `enableBash` flag AND `allowedTools` permission - Delegate tool requires both `enableDelegate` flag AND `allowedTools` permission - Add comprehensive tests to verify validTools respects allowedTools config Fixes #293 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9814f13 commit 1585eb0

2 files changed

Lines changed: 108 additions & 6 deletions

File tree

npm/src/agent/ProbeAgent.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,16 +1706,25 @@ When troubleshooting:
17061706
}
17071707

17081708
// Parse tool call from response with valid tools list
1709-
const validTools = [
1710-
'search', 'query', 'extract', 'listFiles', 'searchFiles', 'attempt_completion'
1711-
];
1712-
if (this.allowEdit) {
1709+
// Build validTools based on allowedTools configuration (same pattern as getSystemMessage)
1710+
const validTools = [];
1711+
if (this.allowedTools.isEnabled('search')) validTools.push('search');
1712+
if (this.allowedTools.isEnabled('query')) validTools.push('query');
1713+
if (this.allowedTools.isEnabled('extract')) validTools.push('extract');
1714+
if (this.allowedTools.isEnabled('listFiles')) validTools.push('listFiles');
1715+
if (this.allowedTools.isEnabled('searchFiles')) validTools.push('searchFiles');
1716+
if (this.allowedTools.isEnabled('attempt_completion')) validTools.push('attempt_completion');
1717+
1718+
// Edit tools (require both allowEdit flag AND allowedTools permission)
1719+
if (this.allowEdit && this.allowedTools.isEnabled('implement')) {
17131720
validTools.push('implement', 'edit', 'create');
17141721
}
1715-
if (this.enableBash) {
1722+
// Bash tool (require both enableBash flag AND allowedTools permission)
1723+
if (this.enableBash && this.allowedTools.isEnabled('bash')) {
17161724
validTools.push('bash');
17171725
}
1718-
if (this.enableDelegate) {
1726+
// Delegate tool (require both enableDelegate flag AND allowedTools permission)
1727+
if (this.enableDelegate && this.allowedTools.isEnabled('delegate')) {
17191728
validTools.push('delegate');
17201729
}
17211730

npm/tests/unit/allowed-tools.test.js

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,99 @@ describe('ProbeAgent allowedTools option', () => {
248248
});
249249
});
250250

251+
describe('validTools array in agentic loop', () => {
252+
test('should respect allowedTools when building validTools array', async () => {
253+
const agent = new ProbeAgent({
254+
path: process.cwd(),
255+
allowedTools: ['search', 'extract', 'attempt_completion']
256+
});
257+
await agent.initialize();
258+
259+
// Verify allowedTools configuration
260+
expect(agent.allowedTools.isEnabled('search')).toBe(true);
261+
expect(agent.allowedTools.isEnabled('extract')).toBe(true);
262+
expect(agent.allowedTools.isEnabled('attempt_completion')).toBe(true);
263+
expect(agent.allowedTools.isEnabled('query')).toBe(false);
264+
expect(agent.allowedTools.isEnabled('listFiles')).toBe(false);
265+
});
266+
267+
test('should build empty validTools array when disableTools is true', async () => {
268+
const agent = new ProbeAgent({
269+
path: process.cwd(),
270+
disableTools: true
271+
});
272+
await agent.initialize();
273+
274+
// Verify that no tools are enabled
275+
expect(agent.allowedTools.mode).toBe('none');
276+
expect(agent.allowedTools.isEnabled('search')).toBe(false);
277+
expect(agent.allowedTools.isEnabled('query')).toBe(false);
278+
expect(agent.allowedTools.isEnabled('extract')).toBe(false);
279+
expect(agent.allowedTools.isEnabled('attempt_completion')).toBe(false);
280+
});
281+
282+
test('should respect both enableBash flag and allowedTools in validTools', async () => {
283+
const agent = new ProbeAgent({
284+
path: process.cwd(),
285+
enableBash: true,
286+
allowedTools: ['search', 'bash', 'attempt_completion']
287+
});
288+
await agent.initialize();
289+
290+
// Bash should be enabled because both enableBash=true AND it's in allowedTools
291+
expect(agent.allowedTools.isEnabled('bash')).toBe(true);
292+
expect(agent.allowedTools.isEnabled('search')).toBe(true);
293+
expect(agent.allowedTools.isEnabled('attempt_completion')).toBe(true);
294+
// Query is not in allowedTools
295+
expect(agent.allowedTools.isEnabled('query')).toBe(false);
296+
});
297+
298+
test('should not include bash in validTools when not in allowedTools', async () => {
299+
const agent = new ProbeAgent({
300+
path: process.cwd(),
301+
enableBash: true,
302+
allowedTools: ['search', 'query', 'attempt_completion']
303+
});
304+
await agent.initialize();
305+
306+
// Bash should NOT be enabled even though enableBash=true
307+
// because it's not in the allowedTools list
308+
expect(agent.allowedTools.isEnabled('bash')).toBe(false);
309+
expect(agent.allowedTools.isEnabled('search')).toBe(true);
310+
expect(agent.allowedTools.isEnabled('query')).toBe(true);
311+
});
312+
313+
test('should not include edit tools in validTools when not in allowedTools', async () => {
314+
const agent = new ProbeAgent({
315+
path: process.cwd(),
316+
allowEdit: true,
317+
allowedTools: ['search', 'query', 'attempt_completion']
318+
});
319+
await agent.initialize();
320+
321+
// Edit tools should NOT be enabled even though allowEdit=true
322+
// because they're not in the allowedTools list
323+
expect(agent.allowedTools.isEnabled('implement')).toBe(false);
324+
expect(agent.allowedTools.isEnabled('edit')).toBe(false);
325+
expect(agent.allowedTools.isEnabled('create')).toBe(false);
326+
expect(agent.allowedTools.isEnabled('search')).toBe(true);
327+
});
328+
329+
test('should include edit tools in validTools when in allowedTools', async () => {
330+
const agent = new ProbeAgent({
331+
path: process.cwd(),
332+
allowEdit: true,
333+
allowedTools: ['search', 'implement', 'attempt_completion']
334+
});
335+
await agent.initialize();
336+
337+
// Implement should be enabled because both allowEdit=true AND it's in allowedTools
338+
expect(agent.allowedTools.isEnabled('implement')).toBe(true);
339+
expect(agent.allowedTools.isEnabled('search')).toBe(true);
340+
expect(agent.allowedTools.isEnabled('attempt_completion')).toBe(true);
341+
});
342+
});
343+
251344
describe('disableTools convenience flag', () => {
252345
test('should disable all tools when disableTools is true', () => {
253346
const agent = new ProbeAgent({

0 commit comments

Comments
 (0)