feat: pull requests are automatically merged based on their activity

[gentlementlegen]

Resolves https://github.com/ubiquibot/plugins-wishlist/issues/20

[gentlementlegen]

QA run

https://github.com/Meniole/automated-merging/pull/5 (automatically merged after 5 minutes with green CI and one review).

First attempt at a local db

In this repo I wanted to attempt handling the db in a local way, since there is no sensitive data at all. The biggest challenge was to version the db itself. There is an Action uploading the changes every run, but runs are not guaranteed to finish in order so it is very likely that the data will get ovverriden at some point. Also, merge will sometimes fail since the data is not always automatically mergeable without human intervention, when this happens I upload the db as an artifact to be manually merged later.

[whilefoo]

@gentlementlegen have you seen my comments?

[gentlementlegen]

@whilefoo Addressed your comments just now, sorry about the delay.

[rndquu]

@gentlementlegen Check this config.

When running in a worker mode getting this error:

TransformDecodeCheckError: Unable to decode value as it does not match the expected schema
...
{
    schema: {
      type: 'object',
      properties: {
        approvalsRequired: [Object],
        mergeTimeout: [Object],
        databaseUrl: [Object]
      },
      required: [ 'approvalsRequired', 'mergeTimeout', 'databaseUrl'
  ],
      [Symbol(TypeBox.Kind)]: 'Object'
    },
    value: {
      approvalsRequired: { collaborator: 0, contributor: 2 },
      mergeTimeout: { collaborator: '5 minutes', contributor: '7 days' },
      databaseUrl: 'database/sql.db'
    },
    error: {
      type: 39,
      schema: {
        default: 1,
        minimum: 1,
        type: 'number',
        [Symbol(TypeBox.Kind)]: 'Number'
      },
      path: '/approvalsRequired/collaborator',
      value: 0,
      message: 'Expected number to be greater or equal to 1'
    }
  }

Is it expected that approvalsRequired.collaborator can't be set to 0?

[rndquu]

@gentlementlegen One more thing, check this config.

The plugin in the worker mode returns this error:

✘ [ERROR] {

    type: 45,
    schema: { type: 'string', [Symbol(TypeBox.Kind)]: 'String' },
    path: '/workflowName',
    value: undefined,
    message: 'Required property'
  }
  {
    type: 54,
    schema: { type: 'string', [Symbol(TypeBox.Kind)]: 'String' },
    path: '/workflowName',
    value: undefined,
    message: 'Expected string'
  }

What am I doing wrong?

[gentlementlegen]

@rndquu I enforced one review for each following our conversation, I can allow a 0 value for collaborators if you prefer.

Checking about the workflowName issue now.

---

Wrangler is not capable to handle the database as far as I know since the node is polyfilled, and this plugin was not meant to run as a worker, sorry about that. The best way is to test it via Jest or run it withing the repository.

[rndquu]

I enforced one review for each following our https://github.com/ubiquibot/plugins-wishlist/issues/20#issuecomment-2206595217, I can allow a 0 value for collaborators if you prefer.

I meant that we should not set 0 values for the ubiquity organization and simply set approvalsRequired to at least 1 (within our org) since this param is configurable but enforcing it to be none 0 also seems to be fine as a precautious measure

this plugin was not meant to run as a worker

Then we should stop adding worker related files and scripts since it's kind of misleading

[gentlementlegen]

@rndquu Agreed, will remove Worker related files.

[rndquu]

@gentlementlegen Help

Trying to run the plugin with this config and getting this CI error.

The expected behavior is that on any comment this PR will be merged. What am I doing wrong?

P.S. The bot is installed in https://github.com/rndquu/automated-merging with issue and PR read and write permissions.

[gentlementlegen]

@rndquu I changed the token to be the one from the repo itself as @whilefoo seemed to suggest in this comment if I understood properly, please let me know if that fixed your issue.

[rndquu]

@rndquu I changed the token to be the one from the repo itself as @whilefoo seemed to suggest in this comment if I understood properly, please let me know if that fixed your issue.

The error is gone but the PR is not merged.

Check this config and this CI run which prints this log output:

issue_comment.created is not supported, skipping

Why issue_comment.created is not supported? Expected behavior: on this comment this PR is merged.

[whilefoo]

Check this config and this CI run which prints this log output:

issue_comment.created is not supported, skipping

Why issue_comment.created is not supported? Expected behavior: on this comment this PR is merged.

Even though that may seem like an error it isn't, the code should be modified to not look like it failed. I think the PR is not merged because there are no approvals

[gentlementlegen]

@rndquu I changed the token to be the one from the repo itself as @whilefoo seemed to suggest in this comment if I understood properly, please let me know if that fixed your issue.

The error is gone but the PR is not merged.

Check this config and this CI run which prints this log output:

issue_comment.created is not supported, skipping

Why issue_comment.created is not supported? Expected behavior: on this comment this PR is merged.

The comment means there will be no action taken because this event does not trigger any logic inside the plugin. I can change the message, not sure what could make it clearer?

Also the PR didn't close for 2 reasons: one is that I think you opened it before the plugin was properly working on the repo so it is actually not tracked. Two is that there is no reviewer that approved so would be skipped.

My latest test run: https://github.com/Meniole/automated-merging/pull/6

[whilefoo]

The comment means there will be no action taken because this event does not trigger any logic inside the plugin. I can change the message, not sure what could make it clearer?

It looks like the plugin didn't even run but in fact it did run updatePullRequests, just the proxy callback didn't match the event

[gentlementlegen]

@whilefoo Correct, that is the intended behavior. I can try to make a clearer message if that is necessary.

[rndquu]

@gentlementlegen Check this config, this PR and this CI run.

On any comment I'm expecting this PR to be merged but this CI run didn't merge it. What am I doing wrong?

[gentlementlegen]

@rndquu I think the problem is that it only runs on comment_created but it should also run on pull_request.opened. So the PR is never added to the database hence the nothing to do in the logs. https://github.com/ubiquibot/automated-merging/blob/22c79f0126b1cab92b91d07b59054db57f61b927/src/proxy/index.ts#L11

[rndquu]

@rndquu I think the problem is that it only runs on comment_created but it should also run on pull_request.opened. So the PR is never added to the database hence the nothing to do in the logs.

https://github.com/ubiquibot/automated-merging/blob/22c79f0126b1cab92b91d07b59054db57f61b927/src/proxy/index.ts#L11

Could you provide a valid config for QA?

[gentlementlegen]

@rndquu My config is like this:

  - uses:
    - plugin: Meniole/automated-merging
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"

I am using the version of the Kernel that retrieves the data from the manifest.json.

[rndquu]

@rndquu My config is like this:

  - uses:
    - plugin: Meniole/automated-merging
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"

I am using the version of the Kernel that retrieves the data from the manifest.json.

How should the whole config look like with pull_request.opened for the plugin to work as expected?

[gentlementlegen]

@rndquu It can just be like

plugins:
  - uses:
    - plugin: Meniole/automated-merging
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"

and the automated-merging repo should contain a manifest like

{
  "name": "Automated merging",
  "description": "Automatically merge pull-requests.",
  "ubiquity:listeners": [ "push", "pull_request.opened", "pull_request.reopened" ]
}

[rndquu]

@rndquu It can just be like

plugins:
  - uses:
    - plugin: Meniole/automated-merging
      with:
        approvalsRequired:
          collaborator: 1
        mergeTimeout:
          collaborator: "2 minutes"

and the user-activity-watcher repo should contain a manifest like

{
  "name": "Automated merging",
  "description": "Automatically merge pull-requests.",
  "ubiquity:listeners": [ "push", "pull_request.opened", "pull_request.reopened" ]
}

How user-activity-watcher is connected with the automated-merging plugin?

[gentlementlegen]

@rndquu my mistake, got confused while writing it, fixed my comment, meant to say automated-merging.

[0x4007]

As long as you tested it works I think just merge and we can test it out in production.

[gentlementlegen]

@rndquu I'll merge this because it is needed for the v2, but feel free to test and let me know if there is anything wrong, I'll fix within the following pull-request.