refactor: standardize resolver runtime handling to match setup.py patterns#84
refactor: standardize resolver runtime handling to match setup.py patterns#84enyst wants to merge 1 commit intoenyst:mainfrom
Conversation
- Remove subprocess git clone from resolve_issue() method - Remove shutil.copytree from process_issue() method - Use initialize_repository_for_runtime() for direct workspace cloning - Move base_commit capture and .openhands_instructions reading after cloning - Move branch checkout logic to process_issue() after repo cloning - Resolver now follows same cloning pattern as other OpenHands entry points This eliminates the legacy clone-to-temp-then-copy workflow in favor of the standard clone-directly-to-workspace pattern used throughout OpenHands. Co-authored-by: openhands <openhands@all-hands.dev>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @enyst, 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 initiates a significant refactoring of the resolver component's runtime handling within OpenHands. The primary goal is to standardize how repositories are cloned and initialized, moving the resolver away from a legacy two-step cloning process to a more consistent, single-step pattern utilized by other OpenHands entry points. This change aims to reduce code duplication, improve maintainability, and align the resolver with the broader OpenHands architecture, setting the stage for further runtime standardization.
Highlights
- Standardized Repository Cloning: The resolver component now uses the
initialize_repository_for_runtime()function fromopenhands.core.setupfor direct repository cloning into the runtime workspace. This replaces a legacy two-step process involvinggit cloneto a temporary location followed byshutil.copytree. - Centralized Git Operations: Git operations such as checking out a PR branch, capturing the base commit, and reading the
.openhands_instructionsfile have been moved from theresolve_issuemethod to theprocess_issuemethod. These operations now occur consistently after the repository has been initialized in the runtime environment. - Code Cleanup and Alignment: Obsolete
shutilimports and manual repository handling logic have been removed fromissue_resolver.py, streamlining the codebase and aligning the resolver's runtime patterns with other OpenHands entry points. - Refactoring Plan Documentation: A new
REFACTOR_PLAN.mddocument has been added, providing a detailed, multi-phase strategy for standardizing resolver runtime handling. This pull request specifically implements the first phase of this plan. - New System Prompt File: A new Jinja2 template file,
system_prompt_long_horizon.j2, has been added, containing a comprehensive system prompt for the OpenHands agent. While its direct relation to the resolver refactoring is not immediately apparent, it is part of the changes in this PR.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 is currently in preview and 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 to provide feedback.
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
-
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. ↩
There was a problem hiding this comment.
Code Review
This pull request refactors the resolver component to use standard OpenHands runtime patterns, aligning it with setup.py and other entry points. There are a few issues in the implementation within openhands/resolver/issue_resolver.py that need to be addressed, including incorrect function arguments, improper path handling for git commands and file access, and the use of blocking I/O calls in an asynchronous function. Additionally, the REFACTOR_PLAN.md could be updated to reflect the progress made in this PR.
| initialize_repository_for_runtime(runtime, self.issue_handler.get_clone_url()) | ||
|
|
||
| # checkout to PR branch if needed | ||
| if branch_to_checkout: | ||
| logger.info(f'Checking out to PR branch {branch_to_checkout}') | ||
| # Fetch the branch first to ensure it exists locally | ||
| fetch_cmd = ['git', 'fetch', 'origin', branch_to_checkout] | ||
| subprocess.check_output(fetch_cmd, cwd=runtime.workspace_root) # noqa: ASYNC101 | ||
|
|
||
| # Checkout the branch | ||
| checkout_cmd = ['git', 'checkout', branch_to_checkout] | ||
| subprocess.check_output(checkout_cmd, cwd=runtime.workspace_root) # noqa: ASYNC101 | ||
|
|
||
| # get the commit id of current repo for reproducibility | ||
| base_commit = ( | ||
| subprocess.check_output( | ||
| ['git', 'rev-parse', 'HEAD'], cwd=runtime.workspace_root | ||
| ) # noqa: ASYNC101 | ||
| .decode('utf-8') | ||
| .strip() | ||
| ) | ||
| logger.info(f'Base commit: {base_commit}') | ||
|
|
||
| # Check for .openhands_instructions file in the workspace directory | ||
| if self.repo_instruction is None: | ||
| openhands_instructions_path = os.path.join( | ||
| runtime.workspace_root, '.openhands_instructions' | ||
| ) | ||
| if os.path.exists(openhands_instructions_path): | ||
| with open(openhands_instructions_path, 'r') as f: # noqa: ASYNC101 | ||
| self.repo_instruction = f.read() |
There was a problem hiding this comment.
This block of code has several issues that should be addressed:
- Incorrect argument for
initialize_repository_for_runtime: This function expects a repository name (e.g.,owner/repo), but it's receiving a full clone URL. This will likely cause failures down the line. - Ignored return value and incorrect
cwd: The directory of the cloned repository, returned byinitialize_repository_for_runtime, is ignored. Consequently, subsequentgitcommands are executed inruntime.workspace_root, not inside the cloned repository directory, which will cause them to fail. - Incorrect path for
.openhands_instructions: The instructions file is being looked for in the workspace root, but it should be inside the cloned repository. - Blocking I/O in async function:
subprocess.check_outputis a blocking call that can stall the asyncio event loop. It should be replaced with its async counterparts or run in a separate thread.
from openhands.core.setup import initialize_repository_for_runtime
import asyncio
repo_name = f'{self.owner}/{self.repo}'
repo_dir = initialize_repository_for_runtime(runtime, repo_name)
repo_path = os.path.join(str(runtime.workspace_root), repo_dir) if repo_dir else str(runtime.workspace_root)
async def run_git_cmd(cmd, check=True):
proc = await asyncio.create_subprocess_exec(
*cmd,
cwd=repo_path,
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE
)
stdout, stderr = await proc.communicate()
if check and proc.returncode != 0:
raise RuntimeError(f"Git command failed: {' '.join(cmd)}\n{stderr.decode('utf-8')}")
return stdout.decode('utf-8').strip()
# checkout to PR branch if needed
if branch_to_checkout:
logger.info(f'Checking out to PR branch {branch_to_checkout}')
await run_git_cmd(['git', 'fetch', 'origin', branch_to_checkout])
await run_git_cmd(['git', 'checkout', branch_to_checkout])
# get the commit id of current repo for reproducibility
base_commit = await run_git_cmd(['git', 'rev-parse', 'HEAD'])
logger.info(f'Base commit: {base_commit}')
# Check for .openhands_instructions file in the repository directory
if self.repo_instruction is None:
openhands_instructions_path = os.path.join(repo_path, '.openhands_instructions')
if await asyncio.to_thread(os.path.exists, openhands_instructions_path):
def read_file():
with open(openhands_instructions_path, 'r', encoding='utf-8') as f:
return f.read()
self.repo_instruction = await asyncio.to_thread(read_file)| - [ ] **Step 1.1**: Replace resolver's legacy 2-step cloning with standard pattern | ||
| - Remove `subprocess.check_output(['git', 'clone', ...])` from `resolve_issue()` | ||
| - Remove `shutil.copytree()` from `process_issue()` | ||
| - Use `initialize_repository_for_runtime()` instead | ||
| - This will clone directly into runtime workspace AND run setup scripts | ||
|
|
||
| - [ ] **Step 1.2**: Update resolver workflow to match standard pattern | ||
| - Create and connect runtime first | ||
| - Then call `initialize_repository_for_runtime()` for cloning + setup | ||
| - Remove the manual repo copying step entirely | ||
| - Ensure base_commit is still captured correctly |
There was a problem hiding this comment.
To keep this plan document up-to-date and avoid confusion, it would be helpful to mark the tasks for Phase 1 as completed, since the pull request description states that Phase 1 of this refactoring plan has been completed.
| - [ ] **Step 1.1**: Replace resolver's legacy 2-step cloning with standard pattern | |
| - Remove `subprocess.check_output(['git', 'clone', ...])` from `resolve_issue()` | |
| - Remove `shutil.copytree()` from `process_issue()` | |
| - Use `initialize_repository_for_runtime()` instead | |
| - This will clone directly into runtime workspace AND run setup scripts | |
| - [ ] **Step 1.2**: Update resolver workflow to match standard pattern | |
| - Create and connect runtime first | |
| - Then call `initialize_repository_for_runtime()` for cloning + setup | |
| - Remove the manual repo copying step entirely | |
| - Ensure base_commit is still captured correctly | |
| - [x] **Step 1.1**: Replace resolver's legacy 2-step cloning with standard pattern | |
| - Remove `subprocess.check_output(['git', 'clone', ...])` from `resolve_issue()` | |
| - Remove `shutil.copytree()` from `process_issue()` | |
| - Use `initialize_repository_for_runtime()` instead | |
| - This will clone directly into runtime workspace AND run setup scripts | |
| - [x] **Step 1.2**: Update resolver workflow to match standard pattern | |
| - Create and connect runtime first | |
| - Then call `initialize_repository_for_runtime()` for cloning + setup | |
| - Remove the manual repo copying step entirely | |
| - Ensure base_commit is still captured correctly |
…itching case confirmed\n\nCo-authored-by: openhands <openhands@all-hands.dev>
…itching case confirmed\n\nCo-authored-by: openhands <openhands@all-hands.dev>
1 participant
Hi! I'm OpenHands, an AI assistant helping to refactor the resolver component's runtime handling patterns.
Overview
This PR refactors the resolver component to use standard OpenHands runtime patterns instead of duplicating logic, aligning it with setup.py and other entry points.
Phase 1: Repository Cloning Pattern ✅
Problem: Resolver used legacy 2-step cloning (subprocess git clone → shutil.copytree) while all other OpenHands entry points use standard 1-step pattern.
Solution:
subprocess.check_output(['git', 'clone', ...])fromresolve_issue()shutil.copytree()fromprocess_issue()initialize_repository_for_runtime()for direct workspace cloningBenefits:
Next Steps (Planned)
runtime.complete()and usecomplete_runtime_for_setup()Related
Co-authored-by: openhands openhands@all-hands.dev