Bot config #38

whilefoo · 2023-11-03T19:28:30Z

I still have some issues because of circular dependencies

…eans

…of the issue is

…or/bot-config

whilefoo · 2023-11-12T09:41:06Z

@pavlovcik can you take a look?

0x4007 · 2023-11-12T11:34:30Z

If it's stable let's just merge it and I'll continue working on it on my branch

whilefoo · 2023-11-13T13:39:47Z

ok lets merge then

0x4007 · 2023-11-13T17:01:04Z

src/utils/ajv.ts

+  // }
+  return { valid: true, error: null };
+  // } catch (error) {
+  // throw console.trace(error);


Are errors still logging?

Actually this code is not used anymore, I have to remove it

0x4007 · 2023-11-13T17:01:47Z

src/types/payload.ts

@@ -33,7 +33,7 @@ export enum GitHubEvent {
 export enum UserType {
  User = "User",
  Bot = "Bot",
-  // Organization = "Organization",
+  Organization = "Organization",


I thought this is a hallucination

Yeah it was failing the payload validation

0x4007 · 2023-11-13T17:02:30Z

src/handlers/processors.ts

@@ -79,7 +78,7 @@ export const processors: Record<string, Handler> = {
    post: [],
  },
  [GitHubEvent.PUSH_EVENT]: {
-    pre: [validateConfigChange],
+    pre: [],


Why is this removed?

Because it needs to execute before loading the config because loading it will fail if it's invalid. It's here now

Do you have another approach for reimplementation of this feature? The purpose of it as you may recall is to immediately diagnose broken configs. Which can happen regularly with partners as they update them.

I just moved it here, it's still working.
Now thinking about it, it might make sense to merge it with loadConfiguration

src/bindings/env.ts

0x4007 · 2023-11-13T17:05:07Z

package.json

@@ -63,7 +63,8 @@
    "prettier": "^2.7.1",
    "probot": "^12.2.4",
    "tsx": "^3.12.7",
-    "yaml": "^2.2.2"
+    "yaml": "^2.2.2",
+    "zod": "^3.22.4"


I thought you said you had to remove zod

Yes, my bad

0x4007

Merging anyways but address these comments and make adjustments in a new pull request if necessary. Thanks!

whilefoo · 2023-11-13T17:21:01Z

src/utils/generate-configuration.ts

Hmm I don't remembering removing that, it might be an accident

0x4007 · 2023-11-14T06:29:29Z

From the Telegram conversation

🖼 https://github.com/ubiquity-whilefoo/ubiquibot-config/blob/main/.github/ubiquibot-config.yml

i cross referenced your org config. its not clear to me how to enable the slash commands

ill dig through the source code in a bit

🖼 let me know how to fix my config

https://github.com/pavlovcik/ubiquibot-config/blob/development/.github/ubiquibot-config.yml

#40 (comment)

chatgpt says my yaml is correct so perhaps your code isnt properly implemented

also it should be throwing errors if my config is wrong so you need to restore that capability

also i dont think you're loading from the ubiquibot-config-default.ts anymore? that seems wrong

i think i found the issue:

i had an almost empty repo config. im pretty sure that becuase i had no properties for commands, it overwrote and set them all to false. of course this must be fixed.

i just deleted the local repo config and now the start command seems to work.

🖼 also trying to understand why wallet is false when its set to true in the config

https://github.com/pavlovcik/ubiquibot-config/blob/d69bdff0ad050c1cf89b2fe6272c2965b7475671/.github/ubiquibot-config.yml#L57-L73

ubiquibot-pavlovcik · 2023-11-14T06:29:31Z

# I do not understand how to respond to that command

0x4007 · 2023-11-14T06:30:51Z

Some additional points:

OpenAI is not initialized and doesn't work. You should test permit generation before submitting fixes.
Keys should be in .env or GitHub Secrets. It doesn't make sense to expose the OpenAI key in the config.

ubiquibot-pavlovcik · 2023-11-14T06:30:54Z

# I do not understand how to respond to that command

ubiquibot-pavlovcik · 2023-11-14T06:49:29Z

# I do not understand how to respond to that command

whilefoo · 2023-11-14T11:19:13Z

🖼 also trying to understand why wallet is false when its set to true in the config

https://github.com/pavlovcik/ubiquibot-config/blob/d69bdff0ad050c1cf89b2fe6272c2965b7475671/.github/ubiquibot-config.yml#L57-L73

The problem was that repo and org were validated and defaults were added automatically and after that the lodash merged them so repo defaults overwrote org properties, so the fix is to first merge the configs and then validate and add defaults.

However I'm still not sure if lodash can merge arrays correctly because I don't know how it chooses the key to merge by or it just joins them, so I propose we change it to:

commands:
  start: true
  stop: false

OR

commands:
  start:
    enabled: true
    # ...other future properties
  stop:
    enabled: false
    # ...other future properties

This will also prevent duplicates and Typescript can detect if some are missing.

0x4007 · 2023-11-14T11:31:31Z

Perhaps we shouldn’t use lodash if it has this limitation. We should aim to make the configuration expressive and partner friendly.

I think it’s more intuitive to simplify further such as

commands: 
   | start
   | stop
   | query

etc. which to my understanding represents an array of strings.

reference: 0x4007#38 (comment)

whilefoo · 2023-11-14T11:49:31Z

It seems it replaces by index.
So we have to find another library that handles this or make our own version

whilefoo · 2023-11-14T12:02:21Z

Actually lodash has a mergeWith function that accepts a custom function, I'll try that first

whilefoo · 2023-11-14T12:25:41Z

I can make it merge object arrays by a certain property such as name but the problem is that the algorithm needs to be generic thus any array will need to have the same field. Or I can make it that repo arrays completely overwrite the org arrays without trying to merge them.
For string arrays I can just merge them and remove duplicates.

0x4007 · 2023-11-14T23:23:33Z

Taking a step back, I feel the only part of the configuration that is a headache to deal with / is not super intuitive are the commands being enabled.

Perhaps we can focus on simplifying or taking a different approach related to those?

Otherwise I think that object assign should be pretty much fine for all primitive values. For example, if a key exists on an org and repo config in the same place, we clobber and take the repo config string.

I wonder what structure makes the most sense for commands. I suppose that an array of strings isn't a primitive data type.

This is verbose but what about new root-level property names that are the command names?

Another idea is that we take the opposite approach with our commands and enable all by default BUT have disables on the repository (or even org) level?

In this case we can have that array of strings:

disabled:
   | start
   | stop
   | query

Given that we rarely disable commands, I feel that this might be the right approach.

Not to complicate things further but when I disabled on ubiquibot the other week, I wished that we were able to also leave an automated message for why to efficiently explain to the contributors. That structure may look more like this:

disabled:
   start: A large refactor is happening at https://github.com/ubiquity/ubiquibot/pull/644 please standby until this is completed. 
   query: User privacy is prioritized in this organization.

whilefoo · 2023-11-15T09:55:04Z

If we use disabled then commands disabled in org config can't be enabled in repo config (you can only disable more commands).
Maybe instead of trying to merge we just overwrite the whole array, so if repo doesn't have disabled array it will use the org's array but if it does then it will ignore org's array and just use repo

Another case with this problem are the labels because they are array of objects:

labels:
  time:
    - name: a
    - name: b
  priority:
    - name: a

0x4007 · 2023-11-15T22:16:03Z

If we use disabled then commands disabled in org config can't be enabled in repo config (you can only disable more commands).

I think this might be okay. At least for our purposes. I can't think of a time where I needed the capability to selectively enable for a repository while the command was disabled on all the others.

Maybe instead of trying to merge we just overwrite the whole array, so if repo doesn't have disabled array it will use the org's array but if it does then it will ignore org's array and just use repo

From the partner's perspective this does not seem intuitive.

The last idea I have for now is to manage this in Supabase with a new command. This is probably reasonable to do if we can't find a good, partner friendly, config schema for these commands.

However I have mixed feelings on it because I think it makes the most sense to be able to manage the repository settings in one place (instead of now potentially three places- two yml and one database)

ubiquibot-pavlovcik · 2023-11-15T23:53:40Z

# I do not understand how to respond to that command

ubiquibot-pavlovcik · 2023-11-15T23:56:43Z

# I do not understand how to respond to that command

0x4007 and others added 13 commits October 27, 2023 18:03

chore: init refactor bot config

6cf0f77

refactor(config): ajv broken cant figure out what the error message m…

69a29b7

…eans

chore(ajv): had to disable strict validation no idea what the origin …

bcb35cd

…of the issue is

fix(ajv): loading default config is stable

b414d45

chore: this is all broken

bdf0ba4

feat: tried zod

f6e23eb

Merge remote-tracking branch 'pavlovcik/refactor/general' into refact…

f5da0ab

…or/bot-config

feat: new config

85a3bf4

fix: circular dependencies

3e939d5

fix: schema and decode

4926daf

fix: payload user type

776938a

fix: transform/decode, config validation at the start

3c67919

feat: error messages, probot context in adapters

33b955c

0x4007 changed the base branch from refactor/general to feat/contributor-incentives-total-scoring November 11, 2023 00:25

0x4007 changed the base branch from feat/contributor-incentives-total-scoring to refactor/general November 11, 2023 00:25

whilefoo marked this pull request as ready for review November 12, 2023 09:25

whilefoo added 2 commits November 12, 2023 12:41

fix: tests

8897087

fix: added comment

1100888

0x4007 changed the base branch from refactor/general to feat/contributor-incentives-total-scoring-initialization November 13, 2023 16:59

0x4007 changed the base branch from feat/contributor-incentives-total-scoring-initialization to feat/contributor-incentives-total-scoring November 13, 2023 16:59

0x4007 reviewed Nov 13, 2023

View reviewed changes

src/bindings/env.ts Show resolved Hide resolved

0x4007 reviewed Nov 13, 2023

View reviewed changes

whilefoo commented Nov 13, 2023

View reviewed changes

src/utils/generate-configuration.ts

Copy link

Author

whilefoo Nov 13, 2023

Choose a reason for hiding this comment

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

Hmm I don't remembering removing that, it might be an accident

0x4007 merged commit d057dc6 into 0x4007:feat/contributor-incentives-total-scoring Nov 13, 2023
3 of 5 checks passed

whilefoo added a commit to ubiquity-whilefoo/ubiquibot that referenced this pull request Nov 14, 2023

fix: merge before validation/defaults

4cc7866

reference: 0x4007#38 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bot config #38

Bot config #38

whilefoo commented Nov 3, 2023

whilefoo commented Nov 12, 2023

0x4007 commented Nov 12, 2023

whilefoo commented Nov 13, 2023

0x4007 Nov 13, 2023

whilefoo Nov 13, 2023

0x4007 Nov 13, 2023

whilefoo Nov 13, 2023

0x4007 Nov 13, 2023

whilefoo Nov 13, 2023 •

edited

Loading

0x4007 Nov 13, 2023

whilefoo Nov 13, 2023

0x4007 Nov 13, 2023

whilefoo Nov 13, 2023

0x4007 left a comment

whilefoo Nov 13, 2023

0x4007 commented Nov 14, 2023

ubiquibot-pavlovcik bot commented Nov 14, 2023

0x4007 commented Nov 14, 2023 •

edited

Loading

ubiquibot-pavlovcik bot commented Nov 14, 2023

ubiquibot-pavlovcik bot commented Nov 14, 2023

whilefoo commented Nov 14, 2023 •

edited

Loading

0x4007 commented Nov 14, 2023 •

edited

Loading

whilefoo commented Nov 14, 2023 •

edited

Loading

whilefoo commented Nov 14, 2023

whilefoo commented Nov 14, 2023

0x4007 commented Nov 14, 2023 •

edited

Loading

whilefoo commented Nov 15, 2023

0x4007 commented Nov 15, 2023 •

edited

Loading

ubiquibot-pavlovcik bot commented Nov 15, 2023

ubiquibot-pavlovcik bot commented Nov 15, 2023

Bot config #38

Bot config #38

Conversation

whilefoo commented Nov 3, 2023

whilefoo commented Nov 12, 2023

0x4007 commented Nov 12, 2023

whilefoo commented Nov 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whilefoo Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0x4007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0x4007 commented Nov 14, 2023

ubiquibot-pavlovcik bot commented Nov 14, 2023

0x4007 commented Nov 14, 2023 • edited Loading

ubiquibot-pavlovcik bot commented Nov 14, 2023

ubiquibot-pavlovcik bot commented Nov 14, 2023

whilefoo commented Nov 14, 2023 • edited Loading

0x4007 commented Nov 14, 2023 • edited Loading

whilefoo commented Nov 14, 2023 • edited Loading

whilefoo commented Nov 14, 2023

whilefoo commented Nov 14, 2023

0x4007 commented Nov 14, 2023 • edited Loading

whilefoo commented Nov 15, 2023

0x4007 commented Nov 15, 2023 • edited Loading

ubiquibot-pavlovcik bot commented Nov 15, 2023

ubiquibot-pavlovcik bot commented Nov 15, 2023

whilefoo Nov 13, 2023 •

edited

Loading

0x4007 commented Nov 14, 2023 •

edited

Loading

whilefoo commented Nov 14, 2023 •

edited

Loading

0x4007 commented Nov 14, 2023 •

edited

Loading

whilefoo commented Nov 14, 2023 •

edited

Loading

0x4007 commented Nov 14, 2023 •

edited

Loading

0x4007 commented Nov 15, 2023 •

edited

Loading