Adds codegen agent --id pull to pull branches locally
#1198
Conversation
Codecov Report❌ Patch coverage is
|
![codegen-sh[bot]](https://avatars.githubusercontent.com/in/321279?s=60&v=4){kind=small}
| from codegen.cli.rich.spinners import create_spinner | ||
| from codegen.cli.utils.org import resolve_org_id | ||
| from codegen.git.repo_operator.local_git_repo import LocalGitRepo | ||
| from codegen.git.repo_operator.repo_operator import RepoOperator |
There was a problem hiding this comment.
Syntax error: API_ENDPOINT is used but not imported
This will raise a NameError at runtime when pull() accesses API_ENDPOINT.
| from codegen.git.repo_operator.repo_operator import RepoOperator | |
| from codegen.cli.api.endpoints import API_ENDPOINT |
| except requests.RequestException as e: | ||
| console.print(f"[red]Error fetching agent run:[/red] {e}") | ||
| raise typer.Exit(1) | ||
| except Exception as e: |
There was a problem hiding this comment.
Logic bug: argument position mismatch for typer.Option in pull
The pull function defines agent_id as --id but positional argument agent_id is also required when calling pull() internally.
| except Exception as e: | |
| agent_id: int = typer.Argument(..., help="Agent run ID to pull PR branch for"), |
| # Check if agent run has PRs | ||
| github_prs = agent_data.get("github_pull_requests", []) | ||
| if not github_prs: | ||
| console.print(f"[yellow]Warning:[/yellow] Agent run {agent_id} has no associated pull requests.") |
There was a problem hiding this comment.
Security issue: unsanitized external URL usage in GitHub API call
github_api_url is built directly from owner and repo extracted from the PR URL; malicious repo names could lead to SSRF.
| console.print(f"[yellow]Warning:[/yellow] Agent run {agent_id} has no associated pull requests.") | |
| from urllib.parse import quote_plus | |
| owner = quote_plus(owner) | |
| repo = quote_plus(repo) | |
| github_api_url = f"https://api.github.com/repos/{owner}/{repo}/pulls/{pr_number}" # safe URL encoding |
| head_branch_name = pr_data.get("head", {}).get("ref") | ||
| if head_branch_name: | ||
| console.print(f"[green]✓ Found branch name from GitHub API:[/green] {head_branch_name}") | ||
| else: |
There was a problem hiding this comment.
Logic error: Fetching remote before validating branch existence maybe unnecessary; but main issue: resetting existing local branch is not implemented
checkout_remote_branch will fail if local branch exists and diverged; you already warn but don't reset.
| else: | |
| if head_branch_name in local_branches: | |
| repo_operator.git_cli.git('reset', '--hard', f'origin/{head_branch_name}') | |
| else: | |
| repo_operator.checkout_remote_branch(head_branch_name) |
|
Found 4 issues. Please review my inline comments above. |
|
🎉 This PR is included in version 0.56.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Motivation
Content
Testing
Please check the following before marking your PR as ready for review