Skip to content
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

Resolver workflow: Added support for processing issues/PRs in other repos #5778

Closed
wants to merge 28 commits into from

Conversation

d-walsh
Copy link
Contributor

@d-walsh d-walsh commented Dec 24, 2024

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Give a summary of what the PR does, explaining any non-trivial design decisions

Resolver workflow: Added support for processing issues/PRs in other repos. This enables more powerful usage of this workflow. It also allows the workflow to be triggered not directly by the issues or PRs, which enables support for a scheduler.


Link of any specific issues this addresses

…processing. Added parameters for issue number, type, target repository owner, and name, along with a comment ID. Updated environment variable handling to accommodate new inputs and ensure proper repository context during execution.
… repository context. Updated pull request and branch creation commands to reference TARGET_REPO_OWNER and TARGET_REPO_NAME, ensuring consistent behavior across different repositories. Improved error handling comments with dynamic links to the created branches and workflow logs.
…eferences. Updated pull request creation commands to streamline the process and enhance flexibility by relying on environment variables for repository context. This change improves the maintainability of the workflow and ensures consistent behavior across different repositories.
…or repository context in error handling. Changed hardcoded references to TARGET_REPO_OWNER and TARGET_REPO_NAME, enhancing flexibility and maintainability of the workflow. This aligns with previous refactoring efforts to streamline repository interactions.
…fy the workflow. This change eliminates unnecessary parameters related to issue processing, streamlining the execution process and enhancing maintainability.
…ent. This addition allows manual triggering of the workflow, improving flexibility in issue resolution processes.
… type via workflow_call

Fix comment location

Update openhands-resolver workflow to trigger on 'workflow_call' event.
Copy link
Contributor

@malhotra5 malhotra5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a ton for this! I'd like to understand the motivation behind this PR more deeply -

  1. Do you envision having a dummy repo where you can invoke the resolver on other repos? We're planning on enabling the resolver to run on forked repos via a Github App, but this PR could be super helpful in providing an alternate option. LMK!

  2. It would also be helpful to have an example workflow which calls this one, similar to the example resolver

@@ -69,7 +89,9 @@ jobs:
contains(github.event.review.body, inputs.macro || '@openhands-agent') &&
(github.event.review.author_association == 'OWNER' || github.event.review.author_association == 'COLLABORATOR' || github.event.review.author_association == 'MEMBER')
)
)
) ||
github.workflow_ref != ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little uncertain of including this here; for instance on Openhands we only want maintainers to run the resolver (because of LLM API costs)

Will this be behavior be preserved?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this will only be populated if this workflow is being called by another workflow (and it would be billed to the repo that the calling workflow exists in).

@d-walsh
Copy link
Contributor Author

d-walsh commented Dec 26, 2024

Thanks a ton for this! I'd like to understand the motivation behind this PR more deeply -

  1. Do you envision having a dummy repo where you can invoke the resolver on other repos? We're planning on enabling the resolver to run on forked repos via a Github App, but this PR could be super helpful in providing an alternate option. LMK!

Yes, that's exactly right, I have a queue-manger workflow (because Github only allows you to queue up to 2 workflow runs using the concurrency feature). I need to run the workflows sequentially otherwise I run into rate limits from Anthropic etc.

  1. It would also be helpful to have an example workflow which calls this one, similar to the example resolver

Sure I'm still polishing that but I can share when it's ready!

d-walsh and others added 16 commits December 26, 2024 19:39
…or issue handling. This change improves the logic for setting environment variables by checking for input parameters before handling pull request events, ensuring more accurate issue tracking.
…l group (All-Hands-AI#5820)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@d-walsh d-walsh closed this Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants