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

[joshualitt]

Fixes #17549

[gemini-code-assist]

Summary of Changes

Hello @joshualitt, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a fundamental improvement to the tool execution pipeline by enabling parallel processing of read-only operations. This change enhances efficiency by allowing multiple non-mutating tasks to run concurrently, while maintaining sequential execution for tools that modify the system state. The core mechanism involves a new isReadOnly property on tools and a refactored scheduler to leverage this property for optimized execution flow.

Highlights

• Parallel Execution for Read-Only Tools: The CoreToolScheduler has been enhanced to execute multiple read-only tool calls in parallel, significantly improving performance for operations that do not modify state. Write operations will continue to be executed sequentially.
• Introduction of isReadOnly Property: A new isReadOnly property has been added to the DeclarativeTool interface and base class, allowing tools to explicitly declare whether they have side effects. This property is crucial for enabling parallel execution.
• Subagent Read-Only Status Determination: Subagents now dynamically determine their isReadOnly status based on the isReadOnly property of all the tools they encapsulate. If any encapsulated tool is not read-only, the subagent itself is considered not read-only.
• Explicitly Marked Read-Only Tools: Several core tools, including file system operations (e.g., ReadFileTool, GlobTool, GrepTool, LSTool, RipGrepTool), web interaction tools (e.g., WebFetchTool, WebSearchTool), and internal workflow tools (e.g., ActivateSkillTool, EnterPlanModeTool, ExitPlanModeTool, GetInternalDocsTool), have been updated to explicitly declare themselves as read-only.

Changelog

• packages/core/src/agents/subagent-tool.test.ts
  • Added new test suite for SubagentTool's read-only logic.
  • Included tests for remote agents, local agents with only read-only tools, local agents with mixed tools, and local agents with no tools to verify isReadOnly behavior.
• packages/core/src/agents/subagent-tool.ts
  • Imported isTool utility.
  • Implemented isReadOnly getter with memoization to determine if a SubagentTool is read-only.
  • Added a static checkIsReadOnly method to evaluate a subagent's read-only status based on its definition and constituent tools.
• packages/core/src/core/coreToolScheduler.parallel.test.ts
  • Added a new test file to verify the parallel execution flow of the CoreToolScheduler.
  • Included tests to ensure correct wave-based execution for mixed read-only and write tools.
  • Verified parallelization of DiscoveredMCPTools when marked as read-only.
• packages/core/src/core/coreToolScheduler.ts
  • Refactored tool validation and scheduling logic into a new validateAndSchedule private method.
  • Modified _processNextInQueue to identify and batch contiguous read-only tool calls for parallel execution.
  • Updated attemptExecutionOfScheduledCalls to use Promise.all for concurrent execution of scheduled tools.
  • Adjusted checkAndNotifyCompletion to correctly handle completion logic for multiple active tool calls in a parallel batch.
• packages/core/src/tools/activate-skill.ts
  • Updated ActivateSkillTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/enter-plan-mode.ts
  • Updated EnterPlanModeTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/exit-plan-mode.ts
  • Updated ExitPlanModeTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/get-internal-docs.ts
  • Updated GetInternalDocsTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/glob.ts
  • Updated GlobTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/grep.ts
  • Updated GrepTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/ls.ts
  • Updated LSTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/mcp-tool.ts
  • Modified DiscoveredMCPTool constructor to accept and pass isReadOnly to the base DeclarativeTool.
• packages/core/src/tools/read-file.ts
  • Updated ReadFileTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/read-many-files.ts
  • Updated ReadManyFilesTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/ripGrep.ts
  • Updated RipGrepTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/tools.test.ts
  • Added new tests to verify the isReadOnly property for ReadFileTool and generic DeclarativeTool instances.
• packages/core/src/tools/tools.ts
  • Added isReadOnly property to the ToolBuilder interface.
  • Implemented isReadOnly getter and private field in DeclarativeTool, initialized via constructor.
  • Updated isTool type guard to include a check for the isReadOnly property.
• packages/core/src/tools/web-fetch.ts
  • Updated WebFetchTool constructor to explicitly set isReadOnly to true.
• packages/core/src/tools/web-search.ts
  • Updated WebSearchTool constructor to explicitly set isReadOnly to true.

Activity

• The pull request introduces a new feature to implement parallel function calls for read-only tools.
• New tests were added to validate the isReadOnly logic for SubagentTool and the parallel execution flow in CoreToolScheduler.
• Existing tools were updated to correctly declare their isReadOnly status.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces parallel execution for read-only tools, which is a great feature for improving performance. The implementation correctly identifies read-only tools and batches them for concurrent execution. However, I've identified a critical race condition in the CoreToolScheduler that could lead to incorrect state management when parallel tool calls complete. My review includes a detailed explanation of the issue. Addressing this is crucial for the stability of the new parallel execution logic.

[github-actions]

Size Change: -9.53 kB (-0.04%)

Total Size: 24.5 MB

Filename	Size	Change
./bundle/gemini.js	24.5 MB	-9.53 kB (-0.04%)

ℹ️ View Unchanged

Filename	Size
./bundle/sandbox-macos-permissive-open.sb	890 B
./bundle/sandbox-macos-permissive-proxied.sb	1.31 kB
./bundle/sandbox-macos-restrictive-open.sb	3.36 kB
./bundle/sandbox-macos-restrictive-proxied.sb	3.56 kB
./bundle/sandbox-macos-strict-open.sb	4.82 kB
./bundle/sandbox-macos-strict-proxied.sb	5.02 kB

_{compressed-size-action}

[gundermanc]

LGTM though I think we should reconsider whether the 'isReadonly' check is necessary. We'll need the ability to have subagents that write in parallel. I'd rather us figure out what's needed to make that work and skip the intermediate step.

[abhipatel12]

LGTM!