Repository navigation
Add in-tool retries with backoff for create_zstream_branch git operations - #581
Conversation
…ions The SSH clone, push, and ls_remote calls to bastion/GitLab can fail transiently due to network blips (connection closed, connection reset, failed to push some refs). Previously these raised ToolError immediately, relying on the beeai framework's LLM-driven retry which has no backoff and is non-deterministic. Now the three git operations retry up to 3 times with exponential backoff (5s, 10s, 20s). Transient errors (identified by stderr patterns) are retried; permanent errors (permission denied, authentication failed, etc.) fail immediately without wasting attempts. Assisted-by: Claude Opus 4.6
![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 introduces a retry mechanism for transient Git errors during clone, push, and remote operations, along with comprehensive unit tests. The review feedback suggests wrapping synchronous file system operations (os.path.exists and shutil.rmtree) in asyncio.to_thread to prevent blocking the main event loop.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| async def _clone(): | ||
| if os.path.exists(clone_dest): | ||
| shutil.rmtree(clone_dest) | ||
| return await asyncio.to_thread(git.Repo.clone_from, clone_url, clone_dest) |
There was a problem hiding this comment.
The helper function _clone performs synchronous file system operations (os.path.exists and shutil.rmtree) directly on the main event loop. For repositories with many files, shutil.rmtree can block the event loop for a significant duration, degrading the performance of other concurrent tasks. Running these operations in a separate thread using asyncio.to_thread avoids blocking the event loop.
| async def _clone(): | |
| if os.path.exists(clone_dest): | |
| shutil.rmtree(clone_dest) | |
| return await asyncio.to_thread(git.Repo.clone_from, clone_url, clone_dest) | |
| async def _clone(): | |
| if await asyncio.to_thread(os.path.exists, clone_dest): | |
| await asyncio.to_thread(shutil.rmtree, clone_dest) | |
| return await asyncio.to_thread(git.Repo.clone_from, clone_url, clone_dest) |
| _TRANSIENT_MAX_RETRIES = 3 | ||
| _TRANSIENT_BASE_DELAY = 5 # seconds | ||
|
|
||
| _TRANSIENT_STDERR_PATTERNS = ( |
There was a problem hiding this comment.
This seems like a pretty ad-hoc heuristic list. I personally would say it's simpler to just retry on all errors; it's unlikely but possible that some of these error strings change eventually.
There was a problem hiding this comment.
these are some of the SSH/network error messages we saw in production last wee. My concern with retrying everything is adding unnecessary load on the infra for permanent errors
| return await fn() | ||
| except Exception as e: | ||
| if attempt < max_retries - 1 and _is_transient_git_error(e): | ||
| backoff = base_delay * 2**attempt |
There was a problem hiding this comment.
Usually best practice for this is to add an additional randomized delay to reduce "thundering herd".
Review feedback: use randomized delay (full jitter) to prevent concurrent agents from retrying in lockstep after a shared outage. Assisted-by: Claude Opus 4.6
TomasTomecek
left a comment
TomasTomecek
left a comment
There was a problem hiding this comment.
LGTM, very nice improvements! I look forward to gather these so we can have concrete numbers and see the state of infra real time
3 participants
No description provided.