-
Notifications
You must be signed in to change notification settings - Fork 6.9k
cloud: default to current branch in cloud exec #7460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cloud: default to current branch in cloud exec #7460
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds branch override functionality for cloud-executed tasks, allowing users to specify a custom git reference via a --branch flag. The implementation introduces a shared git reference resolver that consolidates duplicate logic previously used in both codex cloud exec and TUI task submission.
Key Changes:
- Introduced a
resolve_git_reffunction with testable abstraction for git operations - Added
--branchflag to theExecCommandCLI - Replaced duplicate git reference resolution logic with the new shared function
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| codex-rs/cloud-tasks/src/lib.rs | Adds GitInfoProvider trait, resolve_git_ref functions, updates run_exec_command to use branch override, refactors TUI submission to use shared resolver, and adds comprehensive unit tests |
| codex-rs/cloud-tasks/src/cli.rs | Adds optional --branch parameter to ExecCommand struct |
| codex-rs/cloud-tasks/Cargo.toml | Moves async-trait from dev-dependencies to regular dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tamird
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please give me some context? I'm not sure why I was chosen as a reviewer for this PR. Thanks!
|
@tamird github thought you were relevant. |
|
It thought wrong! |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@squinlan-oai, PR has been reviewed and approved. Please address the merge conflicts, and then it should be ready. |
codex-rs/cloud-tasks/src/lib.rs
Outdated
| if let Some(branch) = git_info.default_branch_name(&cwd).await { | ||
| branch | ||
| } else if let Some(branch) = git_info.current_branch_name(&cwd).await { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably be the other way around? i.e. current branch first, otherwise default branch, otherwise "main"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codex address this + fix any build errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codex couldn't complete this request. Try again later.
Summary
codex cloud execand TUI task submission--branchflag to override the git ref passed to cloud tasksTesting
Codex Task