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

Agent #1

Open
wants to merge 12 commits into
base: development
Choose a base branch
from
Open

Agent #1

wants to merge 12 commits into from

Conversation

whilefoo
Copy link
Member

@whilefoo whilefoo commented Feb 3, 2025


- Your plugin config should look similar to this:
- Generate a Github clasic Personal Access Token PAT with access to repositories.
Copy link
Member

@gentlementlegen gentlementlegen Feb 6, 2025

Choose a reason for hiding this comment

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

Suggested change
- Generate a Github clasic Personal Access Token PAT with access to repositories.
- Generate a GitHub classic Personal Access Token PAT with access to repositories.

Copy link
Member

Choose a reason for hiding this comment

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

CSpell should have caught this. Are your pre commit hooks running?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should add markdown files as well!

Copy link
Member Author

Choose a reason for hiding this comment

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

It should've spell checked markdown files because it's not listed in ignorePaths, if we use files instead of ignorePaths then you need to explicitly set all paths

@@ -0,0 +1 @@
GITHUB_PAT_ENCRYPTED: S6K1T2liquv_G9yS2GxGf20NSHMAVDmX7DDx9-hnjG9518z2TKiWuEZrJobREMFzbBPiCvS1R3peatoiyR6phdjVWmeFM4FNAFp6neO5iUu2kVFVU-r2XvXuFAj-rL8RKEF3XC6yInobjcai
Copy link
Member

@0x4007 0x4007 Feb 10, 2025

Choose a reason for hiding this comment

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

Hey actually this should just be in the repository secrets. No need for a new config file or encryption @whilefoo

Copy link
Member Author

@whilefoo whilefoo Feb 10, 2025

Choose a reason for hiding this comment

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

Technically this would be possible if user's personal-agent had the Ubiquity app installed, but the problem would be that the SDK creates an Octokit instance from the authToken received from the kernel/bridge, so if we didn't any token it would fail.

We could modify the SDK to look for env variable if authToken is empty or supply it when creating the plugin like

createActionsPlugin(handler, { customToken: env.GITHUB_PAT })

manifest.json Show resolved Hide resolved
@@ -13,6 +13,6 @@ export default createActionsPlugin<PluginSettings, Env, null, SupportedEvents>(
envSchema: envSchema,
...(process.env.KERNEL_PUBLIC_KEY && { kernelPublicKey: process.env.KERNEL_PUBLIC_KEY }),
postCommentOnError: true,
bypassSignatureVerification: process.env.NODE_ENV === "local",
bypassSignatureVerification: true,
Copy link
Member

Choose a reason for hiding this comment

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

Why true?

Copy link
Member Author

@whilefoo whilefoo Feb 10, 2025

Choose a reason for hiding this comment

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

The bridge doesn't have functionality to generate a signature, I'd have to copy a lot of code from the kernel to the bridge, but then it'd make more sense to just have this as part of the kernel like I suggested before.

Copy link
Member

Choose a reason for hiding this comment

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

Let's go with your suggestion. I've never worked with the kernel code so I have limited insight.

Comment on lines 30 to 33
env:
PLUGIN_GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
KERNEL_PUBLIC_KEY: ${{ secrets.KERNEL_PUBLIC_KEY }}
LOG_LEVEL: ${{ secrets.LOG_LEVEL || 'info' }}
Copy link
Member

Choose a reason for hiding this comment

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

I think there is a way to dynamically check the owner of the repository so that we can automatically catch the correct @xxx name

Copy link
Member

Choose a reason for hiding this comment

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

/ask in a github action workflow .yml file, how can i get my github user name if I own the repository?

Copy link
Member Author

Choose a reason for hiding this comment

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

The bridge only triggers the personal agent if it matches the owner but I can add the check in the personal agent if you think it's worth

Copy link

Choose a reason for hiding this comment

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

/ask in a github action workflow .yml file, how can i get my github user name if I own the repository?

Copy link

ubiquity-os-beta bot commented Feb 10, 2025

Caution

No answer from OpenAI

@0x4007
Copy link
Member

0x4007 commented Feb 10, 2025

UbiquityOS is thinking...

Thinking is not posted in the review thread. Also should have the tip syntax:

Tip

Syntax


Caution

No answer from OpenAI

@shiv810 can you address these

Copy link

ubiquity-os-beta bot commented Feb 14, 2025

@0x4007
Copy link
Member

0x4007 commented Feb 14, 2025

[!CAUTION]
Not Found - https://docs.github.com/rest/pulls/comments#update-a-review-comment-for-a-pull-request

It doesn't post to review threads at all anymore

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.

Personal Agent
4 participants