Conversation
Isolate subagent thread context by incorporating parentCallId into agentId. This prevents parallel subagents from sharing the same history and duplicating work. Fixes #25533 TAG=agy CONV=4043d296-611e-448a-a9d9-cd78f122b436
Summary of ChangesHello, I'm Gemini Code Assist1! 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 addresses an issue where parallel subagents were sharing the same history and duplicating work by failing to maintain distinct thread contexts. By modifying the agent ID generation to include the parent call identifier, the system now correctly isolates subagent execution, preventing state collisions and redundant processing. Highlights
Using Gemini Code AssistThe 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
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 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. Footnotes
|
gemini-cli
Bot
added
the
area/agent
|
Size Change: +160 B (0%) Total Size: 33.9 MB
ℹ️ View Unchanged
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the LocalAgentExecutor to include the parentCallId in the generated agentId and adjusts the associated tests to verify this behavior. A security concern was raised regarding potential path traversal attacks if the parentCallId is not sanitized before being used in the agentId, especially if that ID is later used for file-based logging. Additionally, it is recommended to maintain the original entropy of the random suffix to prevent ID collisions when multiple subagents are spawned.
| this.agentId = parentCallId | ||
| ? `${parentCallId}-${Math.random().toString(36).slice(2, 5)}` | ||
| : Math.random().toString(36).slice(2, 8); |
There was a problem hiding this comment.
There are two issues with the new agentId generation logic:
- Security (Path Traversal): parentCallId can originate from a model-provided tool call ID (see processFunctionCalls line 1093). If this ID is used in agentId, and agentId is subsequently used as a filename for session recording or logging, a malicious model could trigger a path traversal attack (e.g., by providing a callId like ../../evil). The parentCallId should be sanitized to include only safe characters (e.g., alphanumeric, hyphens, underscores).
- Entropy Reduction: The random suffix length is reduced from 6 to 3 characters when parentCallId is present. If a single tool call spawns multiple subagents in parallel, the collision risk with only 3 characters (36^3 = 46,656) is significantly higher than with 6 characters (36^6 approx 2.1 billion). Given the goal of this PR is to ensure isolation, maintaining the 6-character suffix is safer.
const sanitizedParentId = parentCallId?.replace(/[^a-zA-Z0-9_-]/g, '_');
this.agentId = sanitizedParentId
? sanitizedParentId + '-' + Math.random().toString(36).slice(2, 8)
: Math.random().toString(36).slice(2, 8);Address security feedback from gemini-code-assist: - Sanitize `parentCallId` to prevent potential path traversal attacks if used in filenames. - Restore random suffix to 6 characters to maintain entropy and avoid collisions. TAG=agy CONV=4043d296-611e-448a-a9d9-cd78f122b436
|
Hi @gemini-code-assist, thanks for the feedback! I have updated the PR to address both points:
The changes have been pushed. |
|
Thank you for the update, @akh64bit. The added sanitization for |
akh64bit
force-pushed
the
fix/subagent-isolation-25533
branch
from
00c3826 to
6b42c28
Compare
1 participant
Isolate subagent thread context by incorporating parentCallId into agentId.
This prevents parallel subagents from sharing the same history and duplicating work.
Fixes #25533