Skip to content

refactor(core): consolidate execute() arguments into ExecuteOptions #25101

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
mbleigh merged 5 commits into from
Apr 10, 2026
Merged

refactor(core): consolidate execute() arguments into ExecuteOptions #25101

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
760c539
refactor(core): consolidate execute() arguments into ExecuteOptions
mbleigh Apr 10, 2026
4ab03f1
refactor: rename ExecuteOptions.signal to abortSignal
mbleigh Apr 10, 2026
cccc25d
fix: integration tests
mbleigh Apr 10, 2026
b2ee6d9
fix: integration tests again
mbleigh Apr 10, 2026
023f108
Merge branch 'main' into mb/execute-options
mbleigh Apr 10, 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
6 changes: 4 additions & 2 deletions packages/cli/src/acp/acpClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,9 @@ export class Session {
});
}

const toolResult: ToolResult = await invocation.execute(abortSignal);
const toolResult: ToolResult = await invocation.execute({
signal: abortSignal,
});
const content = toToolCallContent(toolResult);

const updateContent: acp.ToolCallContent[] = content ? [content] : [];
Expand Down Expand Up @@ -1671,7 +1673,7 @@ export class Session {
kind: toAcpToolKind(readManyFilesTool.kind),
});

const result = await invocation.execute(abortSignal);
const result = await invocation.execute({ signal: abortSignal });
const content = toToolCallContent(result) || {
type: 'content',
content: {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/ui/hooks/atCommandProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ async function readLocalFiles(
let invocation: AnyToolInvocation | undefined = undefined;
try {
invocation = readManyFilesTool.build(toolArgs);
const result = await invocation.execute(signal);
const result = await invocation.execute({ signal });
const display: IndividualToolCallDisplay = {
callId: `client-read-${userMessageTimestamp}`,
name: readManyFilesTool.displayName,
Expand Down
10 changes: 4 additions & 6 deletions packages/core/src/agents/agent-tool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
type ToolResult,
BaseToolInvocation,
type ToolCallConfirmationDetails,
type ToolLiveOutput,
type ExecuteOptions,
} from '../tools/tools.js';
import { type AgentLoopContext } from '../config/agent-loop-context.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
Expand Down Expand Up @@ -185,10 +185,8 @@ class DelegateInvocation extends BaseToolInvocation<
return invocation.shouldConfirmExecute(abortSignal);
}

async execute(
signal: AbortSignal,
updateOutput?: (output: ToolLiveOutput) => void,
): Promise<ToolResult> {
async execute(options: ExecuteOptions): Promise<ToolResult> {
const { signal, updateOutput } = options;
const hintedParams = this.withUserHints(this.mappedInputs);
const invocation = this.buildChildInvocation(hintedParams);

Expand All @@ -204,7 +202,7 @@ class DelegateInvocation extends BaseToolInvocation<
},
async ({ metadata }) => {
metadata.input = this.params;
const result = await invocation.execute(signal, updateOutput);
const result = await invocation.execute({ signal, updateOutput });
metadata.output = result;
return result;
},
Expand Down
26 changes: 19 additions & 7 deletions packages/core/src/agents/browser/analyzeScreenshot.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ describe('analyzeScreenshot', () => {
const invocation = tool.build({
instruction: 'Find the blue submit button',
});
const result = await invocation.execute(new AbortController().signal);
const result = await invocation.execute({
signal: new AbortController().signal,
});

// Verify screenshot was captured
expect(browserManager.callTool).toHaveBeenCalledWith(
Expand Down Expand Up @@ -165,7 +167,7 @@ describe('analyzeScreenshot', () => {
const invocation = tool.build({
instruction: 'Find the search bar',
});
await invocation.execute(new AbortController().signal);
await invocation.execute({ signal: new AbortController().signal });

const contentGenerator = config.getContentGenerator();
expect(contentGenerator.generateContent).toHaveBeenCalledWith(
Expand Down Expand Up @@ -194,7 +196,9 @@ describe('analyzeScreenshot', () => {
const invocation = tool.build({
instruction: 'Find the button',
});
const result = await invocation.execute(new AbortController().signal);
const result = await invocation.execute({
signal: new AbortController().signal,
});

expect(result.error).toBeDefined();
expect(result.llmContent).toContain('Failed to capture screenshot');
Expand All @@ -217,7 +221,9 @@ describe('analyzeScreenshot', () => {
const invocation = tool.build({
instruction: 'Check the layout',
});
const result = await invocation.execute(new AbortController().signal);
const result = await invocation.execute({
signal: new AbortController().signal,
});

expect(result.error).toBeDefined();
expect(result.llmContent).toContain('Visual model returned no analysis');
Expand All @@ -238,7 +244,9 @@ describe('analyzeScreenshot', () => {
const invocation = tool.build({
instruction: 'Find the red error',
});
const result = await invocation.execute(new AbortController().signal);
const result = await invocation.execute({
signal: new AbortController().signal,
});

expect(result.error).toBeDefined();
expect(result.llmContent).toContain(
Expand All @@ -261,7 +269,9 @@ describe('analyzeScreenshot', () => {
const invocation = tool.build({
instruction: 'Identify the element',
});
const result = await invocation.execute(new AbortController().signal);
const result = await invocation.execute({
signal: new AbortController().signal,
});

expect(result.error).toBeDefined();
expect(result.llmContent).toContain(
Expand All @@ -281,7 +291,9 @@ describe('analyzeScreenshot', () => {
const invocation = tool.build({
instruction: 'Find something',
});
const result = await invocation.execute(new AbortController().signal);
const result = await invocation.execute({
signal: new AbortController().signal,
});

expect(result.error).toBeDefined();
expect(result.llmContent).toContain('Visual analysis failed');
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/agents/browser/analyzeScreenshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
Kind,
type ToolResult,
type ToolInvocation,
type ExecuteOptions,
} from '../../tools/tools.js';
import { Environment } from '@google/genai';
import type { MessageBus } from '../../confirmation-bus/message-bus.js';
Expand Down Expand Up @@ -80,7 +81,7 @@ class AnalyzeScreenshotInvocation extends BaseToolInvocation<
return `Visual analysis: "${instruction}"`;
}

async execute(signal: AbortSignal): Promise<ToolResult> {
async execute({ signal }: ExecuteOptions): Promise<ToolResult> {
try {
const instruction = String(this.params['instruction'] ?? '');

Expand Down
84 changes: 51 additions & 33 deletions packages/core/src/agents/browser/browserAgentInvocation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,10 @@ describe('BrowserAgentInvocation', () => {
const controller = new AbortController();
const updateOutput: (output: ToolLiveOutput) => void = vi.fn();

const result = await invocation.execute(controller.signal, updateOutput);
const result = await invocation.execute({
signal: controller.signal,
updateOutput,
});

expect(Array.isArray(result.llmContent)).toBe(true);
expect((result.llmContent as Array<{ text: string }>)[0].text).toContain(
Expand All @@ -242,7 +245,7 @@ describe('BrowserAgentInvocation', () => {
const controller = new AbortController();
// Should not throw even with no updateOutput
await expect(
invocation.execute(controller.signal),
invocation.execute({ signal: controller.signal }),
).resolves.toBeDefined();
});

Expand All @@ -256,7 +259,7 @@ describe('BrowserAgentInvocation', () => {
);

const controller = new AbortController();
const result = await invocation.execute(controller.signal);
const result = await invocation.execute({ signal: controller.signal });

expect(result.error).toBeDefined();
expect(removeInputBlocker).toHaveBeenCalled();
Expand Down Expand Up @@ -298,7 +301,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

await invocation.execute(new AbortController().signal, updateOutput);
await invocation.execute({
signal: new AbortController().signal,
updateOutput,
});

const firstCall = updateOutput.mock.calls[0]?.[0] as SubagentProgress;
expect(firstCall.isSubagentProgress).toBe(true);
Expand All @@ -315,7 +321,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

await invocation.execute(new AbortController().signal, updateOutput);
await invocation.execute({
signal: new AbortController().signal,
updateOutput,
});

const lastCall = updateOutput.mock.calls[
updateOutput.mock.calls.length - 1
Expand All @@ -334,10 +343,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

const executePromise = invocation.execute(
new AbortController().signal,
const executePromise = invocation.execute({
signal: new AbortController().signal,
updateOutput,
);
});

// Allow createBrowserAgentDefinition to resolve and onActivity to be registered
await Promise.resolve();
Expand Down Expand Up @@ -377,10 +386,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

const executePromise = invocation.execute(
new AbortController().signal,
const executePromise = invocation.execute({
signal: new AbortController().signal,
updateOutput,
);
});

// Allow createBrowserAgentDefinition to resolve and onActivity to be registered
await Promise.resolve();
Expand Down Expand Up @@ -424,10 +433,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

const executePromise = invocation.execute(
new AbortController().signal,
const executePromise = invocation.execute({
signal: new AbortController().signal,
updateOutput,
);
});

await Promise.resolve();
await Promise.resolve();
Expand Down Expand Up @@ -475,10 +484,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

const executePromise = invocation.execute(
new AbortController().signal,
const executePromise = invocation.execute({
signal: new AbortController().signal,
updateOutput,
);
});

await Promise.resolve();
await Promise.resolve();
Expand Down Expand Up @@ -519,10 +528,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

const executePromise = invocation.execute(
new AbortController().signal,
const executePromise = invocation.execute({
signal: new AbortController().signal,
updateOutput,
);
});

await Promise.resolve();
await Promise.resolve();
Expand Down Expand Up @@ -564,10 +573,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

const executePromise = invocation.execute(
new AbortController().signal,
const executePromise = invocation.execute({
signal: new AbortController().signal,
updateOutput,
);
});

await Promise.resolve();
await Promise.resolve();
Expand Down Expand Up @@ -604,10 +613,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

const executePromise = invocation.execute(
new AbortController().signal,
const executePromise = invocation.execute({
signal: new AbortController().signal,
updateOutput,
);
});

await Promise.resolve();
await Promise.resolve();
Expand Down Expand Up @@ -647,10 +656,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

const executePromise = invocation.execute(
new AbortController().signal,
const executePromise = invocation.execute({
signal: new AbortController().signal,
updateOutput,
);
});

await Promise.resolve();
await Promise.resolve();
Expand Down Expand Up @@ -703,7 +712,10 @@ describe('BrowserAgentInvocation', () => {
mockParams,
mockMessageBus,
);
await invocation.execute(new AbortController().signal, vi.fn());
await invocation.execute({
signal: new AbortController().signal,
updateOutput: vi.fn(),
});

expect(recordBrowserAgentTaskOutcome).toHaveBeenCalledWith(
mockConfig,
Expand Down Expand Up @@ -731,7 +743,10 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

await invocation.execute(new AbortController().signal, updateOutput);
await invocation.execute({
signal: new AbortController().signal,
updateOutput,
});

expect(recordBrowserAgentTaskOutcome).toHaveBeenCalledWith(
mockConfig,
Expand All @@ -751,7 +766,10 @@ describe('BrowserAgentInvocation', () => {
mockParams,
mockMessageBus,
);
await invocation.execute(new AbortController().signal, vi.fn());
await invocation.execute({
signal: new AbortController().signal,
updateOutput: vi.fn(),
});

expect(cleanupBrowserAgent).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -807,7 +825,7 @@ describe('BrowserAgentInvocation', () => {
mockMessageBus,
);

await invocation.execute(new AbortController().signal);
await invocation.execute({ signal: new AbortController().signal });

// Verify list_pages was called
expect(mockBrowserManager.callTool).toHaveBeenCalledWith(
Expand Down
8 changes: 3 additions & 5 deletions packages/core/src/agents/browser/browserAgentInvocation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { LocalAgentExecutor } from '../local-executor.js';
import {
BaseToolInvocation,
type ToolResult,
type ToolLiveOutput,
type ExecuteOptions,
} from '../../tools/tools.js';
import { ToolErrorType } from '../../tools/tool-error.js';
import {
Expand Down Expand Up @@ -107,10 +107,8 @@ export class BrowserAgentInvocation extends BaseToolInvocation<
* 3. Runs the agent via LocalAgentExecutor
* 4. Cleans up browser resources
*/
async execute(
signal: AbortSignal,
updateOutput?: (output: ToolLiveOutput) => void,
): Promise<ToolResult> {
async execute(options: ExecuteOptions): Promise<ToolResult> {
const { signal, updateOutput } = options;
const invocationStartMs = Date.now();
let browserManager;
let recentActivity: SubagentActivityItem[] = [];
Expand Down
Loading
Loading