feat!: tracking from config

[gentlementlegen]

Resolves #2

[gentlementlegen]

Major changes

To get rid of the database which sole purpose was to track pull-requests, I changed the way this works to instead rely on the configuration to know what to parse. To avoid using too much resources, it actually leverages the search API to directly fetch all open pull requests within orgs and repos.

New items in the configuration:

 - uses: ubiquibot/automated-merging
   with:
+    watch: [ "ubiquity" ]
-    database: "sql.db"

The accepted values are the following:

• an organization (e.g. ubiquity)
• a specific repo (e.g. ubiquibot/automated-merging)
• a repo to exclude (e.g. the name starting with a minus character like -ubiquibot/conversation-rewards)

QA run

Automatically merged PR

[0x4007]

Concerns On This Approach

The only thing that makes sense to me is: when an event occurs within an organization, the plugin can search all open pulls across the entire organization, and not across other organizations. I think this also simplifies the implementation quite a bit.

In this case, consider something like:

 - uses: ubiquibot/automated-merging
   with:
    repos_monitor: [ "ubiquibot" ] 
    repos_ignore: []

Concerns Regarding Property Names

I'm not super confident in picking property names because it is somewhat subjective, but the goal is to make development as unambiguous as possible.

 - uses: ubiquibot/automated-merging
   with:
+    watch: [ "ubiquity" ]
-    database: "sql.db"

I am very cautious to use similar property names for different purposes in our technology stack. We use listeners in our plugin manifests to subscribe to webhook events. This can confuse future developers if they see something similar (like watch) in the config, but used differently. Consider using a more descriptive property name specifically to describe this feature's behavior in the context of my concern and the manifest's behavior. We can consider using a qualifier word and a synonym¹ to further remove associations. Reducing the association will lead to less confusion.

Perhaps we should rename the manifest one to ubiquity:subscribe? https://github.com/ubiquibot/automated-merging/blob/development/manifest.json#L4

Footnotes

1. repos_monitor is an example of a unique qualifier word (repos) with a synonym (monitor) ↩

[gentlementlegen]

I can limit to the same organization eventually, but that actually complexifies the code because I should always check the org it comes from, and also it would not be possible anymore to target repositories located outside and organization.

Also, it is currently not needed to have two fields monitor and ignore, both these cases can be handled within the same array:

watch:
  - "ubiquibot"
  - "-ubiquibot/automated-merging"

would watch all Ubiquibot repos except automated-merging. Also I can rename watch to anything else, it's less cumbersome than having to rename all the manifests and change the kernel's code.

[0x4007]

I should always check the org it comes from

I presume that the authentication is inherited from the original request? Meaning that the payload includes the installation ID, so then you can just use the same installation ID for any future read operations?

Also, it is currently not needed to have two fields monitor and ignore, both these cases can be handled within the same array:

The intent is to make the config as expressive as possible for ease of use. repo_ignore is more expressive than - "-ubiquibot/automated-merging"

Also I can rename watch to anything else, it's less cumbersome than having to rename all the manifests and change the kernel's code.

We can handle config renames over time. The priority now is just to get v2 functional (wen) so we can demo/promote at abs.io

[gentlementlegen]

Then it would only be safe to run it within the org it is installed, I might be able to find that information somewhere in the payload. I will also create two fields for repositories then.

[gentlementlegen]

@gentlementlegen I'm unsure what I'm doing wrong here.

I have an up to date fork and my plugin: url is being parsed correctly in the kernel it seems

Dispatching workflow {
  owner: 'ubq-testing',
  repository: 'automated-merging',
  workflowId: 'compute.yml',
  ref: 'test',
  }

// error log url:
  'https://api.github.com/repos/ubq-testing/automated-merging/actions/workflows/compute.yml/dispatches'

I have also tried ref: development and every time I get the same Not found error using the below config. Any ideas?

      - plugin: "ubq-testing/automated-merging:compute.yml@development"
        runsOn: [ "push" ]
        with:
          approvalsRequired:
            collaborator: 1
            contributor: 2
          mergeTimeoutSchema:
            collaborator: "2 days"
            contributor: "5 days"
          repos: 
            monitor: 
              - "ubq-testing/command-start-stop"
            ignore:
              - "bot-ai"
              

Is the workflow present on the default branch?

[Keyrxng]

present on both branches yes, I haven't worked with workflows in ages it feels but everything appears right

I have the correct bot permissions set, I have not changed logic in either the kernel or this plugin and the last time I ran a workflow plugin this format did work (I just comment them out in my config)

I could only find two wf runs from this PR branch when I was trying to debug but they ran on issue.labeled events (easy testing I guess) tho they both failed and push is the only supported event.

[gentlementlegen]

I need a computer to test myself but I checked the repo and it seems right. Dunno if the kernel you're using is the latest version, but yesterday it was working fine with a specific branch for me. I will come back to you if I make some findings. In the meantime what you can try is adding a workflow_dispatch to try to manually trigger it and see if there is issues.

---

@Keyrxng I tried using a specific branch and had no issue running it so far: https://github.com/Meniole/automated-merging/actions/runs/10295280986 with configuration looking like Meniole/automated-merging:compute.yml@test

[Keyrxng]

I had issues running the workflow for this repo for ages, I had to refork the repo and it allowed me to in the new fork

---

I'm repeatedly being secondary rate limited running this workflow. Any ideas why this is happening? Below is the run and my rate usage after 4 attempts with time between each.

The only repo secret I need to define is workflowName right?

https://github.com/ubq-testing/automated-merging-qa/actions/runs/10368619910/job/28702614645

'x-ratelimit-limit': '6100',
'x-ratelimit-remaining': '6024',
'x-ratelimit-reset': '1723550724',
'x-ratelimit-resource': 'core',
'x-ratelimit-used': '76',
'x-xss-protection': '0'

With config:

      - plugin: ubq-testing/automated-merging-qa:compute.yml@test
        runsOn: [ "push" ]
        with:
          approvalsRequired:
            collaborator: 1
            contributor: 2
          mergeTimeoutSchema:
            collaborator: "2 days"
            contributor: "5 days"
          repos: 
            monitor: 
              - "ubq-testing/command-start-stop"
            ignore:
              - "bot-ai"

---

request: {
method: 'GET',
url: 'https://api.github.com/search/issues?q=is%3Apr+is%3Aopen+draft%3Afalse+&page=7',

This doesn't seem right to me that it's on page 7 when searching for PRs in my QA org specifically only one repo I think is what should be happening because I've specified only one repo to monitor?

[Keyrxng]

typebox enforces that approvalsRequired > 0 but mergeTimeout can be 0, is this intentional or should it also be validated to be > 0?

"org/repo" works but "repo" does not appear to

this is a dangerous plugin in that only authorized users should be able to make changes I guess this would be covered by config-protection as well?

cspell failed locally running yarn format but doesn't appear to have affected CI. We have the workflow file but it's not recognized in the Actions tab

[gentlementlegen]

typebox enforces that approvalsRequired > 0 but mergeTimeout can be 0, is this intentional or should it also be validated to be > 0?

"org/repo" works but "repo" does not appear to

this is a dangerous plugin in that only authorized users should be able to make changes I guess this would be covered by config-protection as well?

cspell failed locally running yarn format but doesn't appear to have affected CI. We have the workflow file but it's not recognized in the Actions tab

I think it is okay because the merge timeout is represented as a string like "1 day", so errors would be easy to spot.

"org/repo" or "org" are the only accepted patterns. You need to fully qualify repos to avoid errors when two orgs have the same repo name.

I will check the Cspell problem.

[whilefoo]

"org/repo" or "org" are the only accepted patterns. You need to fully qualify repos to avoid errors when two orgs have the same repo name.

this plugin only works within an org so "org" is useless, and "org/repo" can be just "repo"

[gentlementlegen]

"org/repo" or "org" are the only accepted patterns. You need to fully qualify repos to avoid errors when two orgs have the same repo name.

this plugin only works within an org so "org" is useless, and "org/repo" can be just "repo"

Makes sense, will do the necessary changes.

[0x4007]

We should use issues_comment.created as the clock because it is the most frequent event. What is the reason you chose push?

[gentlementlegen]

QA run: Meniole#10 (review)