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

feat: correct context and skip issues outside of the org #134

Merged
merged 11 commits into from
Feb 3, 2025

Conversation

whilefoo
Copy link
Member

Resolves #131

The issue is that start function was getting the context associated with the PR's repo but the issue was from another repo. I fixed it by creating a new context from the old context and replacing issue and repository from the API, but this feels more like a hack because the config will be from the original repo.

Another problem is that the PR's repo and issue's repo have to be in the same organization because the plugin only has installation token for the org and it doesn't have write permissions in other orgs. For example I made a PR in ubiquity-os-marketplace/text-conversation-rewards and linking to ubiquity-os/plugins-wishlist, this won't work because of the permissions.
A solution would be that we somehow move the logic so that it happens in the issue instead of the PR, but I couldn't find any webhook that would be triggered when an issue got a PR linked, cross-referenced or something similar to that so that we could trigger it based on that.

@whilefoo whilefoo requested review from gentlementlegen and 0x4007 and removed request for gentlementlegen January 29, 2025 22:37
@gentlementlegen
Copy link
Member

gentlementlegen commented Jan 30, 2025

What about creating a new Octokit instance with the proper credentials for the other organization from this plugin? It is possible because we have the APP_ID and the APP_PRIVATE_KEY, and this technique has been used in other plugins to solve authentication issue. Maybe it would be worth having this function exposed from the SDK.

Copy link
Contributor

Unlisted dependencies (1)

Filename unlisted
src/handlers/user-start-stop.ts @octokit/auth-app

@whilefoo whilefoo marked this pull request as ready for review January 30, 2025 22:01
@whilefoo
Copy link
Member Author

whilefoo commented Jan 30, 2025

Enabled support for cross-org with APP_ID and APP_PRIVATE_KEY and added tests

cross-org QA: PR and issue

@0x4007
Copy link
Member

0x4007 commented Jan 31, 2025

QA isn't 100% clear to me I left a comment on QA PR

@whilefoo
Copy link
Member Author

Made another PR so the errors aren't present

@gentlementlegen
Copy link
Member

gentlementlegen commented Feb 2, 2025

Tested it, not sure if I did something wrong but the issues doesn't appear to be fixed:

The version I used is a deployed worker created from your PR changes.

Edit: it works fine, the deployed code was not taken from the proper branch. @whilefoo Could you please fix the worker deployment script and change the checkout step to:

      - uses: actions/checkout@v4
        with:
          ref: ${{ github.event.workflow_run.head_branch || github.ref }}

so the proper code source is pulled during build?

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.

Assign user to the proper linked issue
3 participants