feat(core): Implement parallel FC for read only tools. (#18791)

This commit is contained in:
joshualitt
2026-02-19 16:38:22 -08:00
committed by GitHub
parent f4dfbfbb58
commit b11df9d839
11 changed files with 862 additions and 301 deletions
@@ -17,10 +17,12 @@ import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
import type { Config } from '../config/config.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
import type {
DeclarativeTool,
ToolCallConfirmationDetails,
ToolInvocation,
ToolResult,
} from '../tools/tools.js';
import type { ToolRegistry } from 'src/tools/tool-registry.js';
vi.mock('./subagent-tool-wrapper.js');
@@ -274,3 +276,85 @@ describe('SubAgentInvocation', () => {
});
});
});
describe('SubagentTool Read-Only logic', () => {
let mockConfig: Config;
let mockMessageBus: MessageBus;
beforeEach(() => {
vi.clearAllMocks();
mockConfig = makeFakeConfig();
mockMessageBus = createMockMessageBus();
});
it('should be false for remote agents', () => {
const tool = new SubagentTool(
testRemoteDefinition,
mockConfig,
mockMessageBus,
);
expect(tool.isReadOnly).toBe(false);
});
it('should be true for local agent with only read-only tools', () => {
const readOnlyTool = {
name: 'read',
isReadOnly: true,
} as unknown as DeclarativeTool<object, ToolResult>;
const registry = {
getTool: (name: string) => (name === 'read' ? readOnlyTool : undefined),
};
vi.spyOn(mockConfig, 'getToolRegistry').mockReturnValue(
registry as unknown as ToolRegistry,
);
const defWithTools: LocalAgentDefinition = {
...testDefinition,
toolConfig: { tools: ['read'] },
};
const tool = new SubagentTool(defWithTools, mockConfig, mockMessageBus);
expect(tool.isReadOnly).toBe(true);
});
it('should be false for local agent with at least one non-read-only tool', () => {
const readOnlyTool = {
name: 'read',
isReadOnly: true,
} as unknown as DeclarativeTool<object, ToolResult>;
const mutatorTool = {
name: 'write',
isReadOnly: false,
} as unknown as DeclarativeTool<object, ToolResult>;
const registry = {
getTool: (name: string) => {
if (name === 'read') return readOnlyTool;
if (name === 'write') return mutatorTool;
return undefined;
},
};
vi.spyOn(mockConfig, 'getToolRegistry').mockReturnValue(
registry as unknown as ToolRegistry,
);
const defWithTools: LocalAgentDefinition = {
...testDefinition,
toolConfig: { tools: ['read', 'write'] },
};
const tool = new SubagentTool(defWithTools, mockConfig, mockMessageBus);
expect(tool.isReadOnly).toBe(false);
});
it('should be true for local agent with no tools', () => {
const registry = { getTool: () => undefined };
vi.spyOn(mockConfig, 'getToolRegistry').mockReturnValue(
registry as unknown as ToolRegistry,
);
const defNoTools: LocalAgentDefinition = {
...testDefinition,
toolConfig: { tools: [] },
};
const tool = new SubagentTool(defNoTools, mockConfig, mockMessageBus);
expect(tool.isReadOnly).toBe(true);
});
});
+48
View File
@@ -11,6 +11,7 @@ import {
type ToolResult,
BaseToolInvocation,
type ToolCallConfirmationDetails,
isTool,
} from '../tools/tools.js';
import type { AnsiOutput } from '../utils/terminalSerializer.js';
import type { Config } from '../config/config.js';
@@ -48,6 +49,53 @@ export class SubagentTool extends BaseDeclarativeTool<AgentInputs, ToolResult> {
);
}
private _memoizedIsReadOnly: boolean | undefined;
override get isReadOnly(): boolean {
if (this._memoizedIsReadOnly !== undefined) {
return this._memoizedIsReadOnly;
}
// No try-catch here. If getToolRegistry() throws, we let it throw.
// This is an invariant: you can't check read-only status if the system isn't initialized.
this._memoizedIsReadOnly = SubagentTool.checkIsReadOnly(
this.definition,
this.config,
);
return this._memoizedIsReadOnly;
}
private static checkIsReadOnly(
definition: AgentDefinition,
config: Config,
): boolean {
if (definition.kind === 'remote') {
return false;
}
const tools = definition.toolConfig?.tools ?? [];
const registry = config.getToolRegistry();
if (!registry) {
return false;
}
for (const tool of tools) {
if (typeof tool === 'string') {
const resolvedTool = registry.getTool(tool);
if (!resolvedTool || !resolvedTool.isReadOnly) {
return false;
}
} else if (isTool(tool)) {
if (!tool.isReadOnly) {
return false;
}
} else {
// FunctionDeclaration - we don't know, so assume NOT read-only
return false;
}
}
return true;
}
protected createInvocation(
params: AgentInputs,
messageBus: MessageBus,