Skip to content

Commit

Permalink
Command bot - named arguments (#208)
Browse files Browse the repository at this point in the history
Share credits for idea & part of implementation with @mutantcornholio 
- Fixes #160
- Related #174
  - Implements named arguments for #174 + handy "guess" helper which translates old to a new syntax
  - Improved docs which now filterable by repo and have better command arguments description
- Made "bot ..." configurable in env vars, so we can run local bots on **paritytech-stg** in same repo alongside with staging bots and not interfere
  • Loading branch information
mordamax authored Jul 3, 2023
1 parent 28f4ecb commit 1a093fd
Show file tree
Hide file tree
Showing 45 changed files with 1,578 additions and 403 deletions.
2 changes: 2 additions & 0 deletions .env.example.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,5 @@ process.env.MASTER_TOKEN ??= "placeholder"
process.env.CMD_BOT_URL ??= "http://localhost:3000/"

process.env.MIN_LOG_LEVEL ??= "debug"

process.env.BOT_PR_COMMENT_MENTION ??= "localbot"
1 change: 1 addition & 0 deletions helm/values-parity-prod.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ common:
DB_CLIENT: postgres
DB_PORT: 5432
CMD_BOT_URL: https://command-bot.parity-prod.parity.io/
BOT_PR_COMMENT_MENTION: bot
secrets:
ALLOWED_ORGANIZATIONS: ref+vault://kv/gitlab/parity/mirrors/command-bot/devops-parity-prod#ALLOWED_ORGANIZATIONS
APP_ID: ref+vault://kv/gitlab/parity/mirrors/command-bot/devops-parity-prod#APP_ID
Expand Down
2 changes: 2 additions & 0 deletions helm/values-parity-stg.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ common:
DB_CLIENT: postgres
DB_PORT: 5432
CMD_BOT_URL: https://command-bot.parity-stg.parity.io/
PIPELINE_SCRIPTS_REF: monorepo
BOT_PR_COMMENT_MENTION: stgbot
secrets:
ALLOWED_ORGANIZATIONS: ref+vault://kv/gitlab/parity/mirrors/command-bot/devops-parity-stg#ALLOWED_ORGANIZATIONS
APP_ID: ref+vault://kv/gitlab/parity/mirrors/command-bot/devops-parity-stg#APP_ID
Expand Down
1 change: 1 addition & 0 deletions helm/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ common:
GITLAB_JOB_IMAGE: paritytech/ci-linux:production
GITLAB_DOMAIN: gitlab.parity.io
PIPELINE_SCRIPTS_REPOSITORY: https://github.com/paritytech/command-bot-scripts/
PIPELINE_SCRIPTS_REF: main
# We only want one instance at all times so that the execution queue can be
# sanely tracked across multiple MRs.
autoscaling:
Expand Down
2 changes: 1 addition & 1 deletion nodemon.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
"verbose": false,
"watch": ["src/", ".env.cjs"],
"ignore": ["src/test/", "node_modules/", ".git", "src/schema/*.ts"],
"ext": ".ts",
"ext": ".ts,.pug",
"exec": "node -r ts-node/register -r ./.env.cjs"
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"@types/node-fetch": "^2.6.2",
"@types/pug": "^2.0.6",
"async-mutex": "0.3.2",
"commander": "^10.0.1",
"date-fns": "2.28.0",
"express-prom-bundle": "^6.6.0",
"glob": "^8.0.3",
Expand Down
1 change: 0 additions & 1 deletion src/bot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { onIssueCommentCreated } from "src/bot/events/onIssueCommentCreated";
import { setupEvent } from "src/bot/setupEvent";
import { Context } from "src/types";

export const botPullRequestCommentMention = "bot";
// just do nothing, completely skip
export const botPullRequestIgnoreCommands = ["merge", "rebase"];
export const botPullRequestCommentSubcommands: {
Expand Down
27 changes: 18 additions & 9 deletions src/bot/events/onIssueCommentCreated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ import {
removeReactionToComment,
updateComment,
} from "src/github";
import { counters, getMetricsPrData } from "src/metrics";
import { cancelTask, getNextTaskId, PullRequestTask, queueTask, serializeTaskQueuedDate } from "src/task";
import { getLines } from "src/utils";

const eventName = "issue_comment.created";

export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = async (ctx, octokit, payload) => {
const { repositoryCloneDirectory, gitlab, logger } = ctx;
const { issue, comment, repository, installation } = payload;
Expand Down Expand Up @@ -48,14 +51,15 @@ export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = as
let getError = (body: string) => new PullRequestError(pr, { body, requester, requesterCommentId: comment.id });

try {
const commands: ParsedCommand[] = [];
const commands: (ParsedCommand | SkipEvent)[] = [];

if (!(await isRequesterAllowed(ctx, octokit, requester))) {
return getError("Requester could not be detected as a member of an allowed organization.");
}

for (const line of getLines(comment.body)) {
const parsedCommand = await parsePullRequestBotCommandLine(line, ctx, pr.repo);

if (parsedCommand instanceof SkipEvent) {
return parsedCommand;
}

if (parsedCommand instanceof Error) {
return getError(parsedCommand.message);
}
Expand All @@ -67,13 +71,18 @@ export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = as
return new SkipEvent("No commands found within a comment");
}

if (!(await isRequesterAllowed(ctx, octokit, requester))) {
return getError("Requester could not be detected as a member of an allowed organization.");
}

for (const parsedCommand of commands) {
logger.debug({ parsedCommand }, "Processing parsed command");

if (parsedCommand instanceof SkipEvent) {
const skip = getMetricsPrData("skip", eventName, pr, parsedCommand.reason);
counters.commandsRun.inc({ ...skip });
logger.debug(
{ command: comment.body, payload, ...skip },
`Skip command with reason: "${parsedCommand.reason}"`,
);
}

if (parsedCommand instanceof HelpCommand) {
await createComment(ctx, octokit, {
...commentParams,
Expand Down
78 changes: 78 additions & 0 deletions src/bot/parse/guessCommand.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { jest } from "@jest/globals";

import { guessCommand } from "src/bot/parse/guessCommand";
import { logger } from "src/logger";

jest.mock("src/command-configs/fetchCommandsConfiguration");

logger.options.minLogLevel = "fatal";

type DataProvider = {
suitName: string;
command: string;
repo: string;
result: string;
};

const dataProvider: DataProvider[] = [
{ suitName: "no args", command: "help", repo: "polkadot", result: "" },
{ suitName: "sample", command: "sample $ args", repo: "polkadot", result: "bot sample --input=args" },
{
suitName: "sample in wrong repo",
command: "sample $ args",
repo: "polkadot-sdk",
result: "bot sample --input=args",
},
{
suitName: "cumulus assets polkadot",
command: "bench $ pallet asset-hub-polkadot assets pallet_xz",
repo: "polkadot-sdk",
result: "bot bench cumulus-assets --pallet=pallet_xz",
},
{
suitName: "cumulus assets kusama",
command: "bench $ pallet asset-hub-kusama assets pallet_xz",
repo: "polkadot-sdk",
result: "bot bench cumulus-assets --runtime=asset-hub-kusama --pallet=pallet_xz",
},
{
suitName: "cumulus assets old kusama: will endup with default, as can't find `statemine`",
command: "bench $ pallet statemine assets pallet_xz",
repo: "polkadot-sdk",
result: "bot bench cumulus-assets --pallet=pallet_name",
},
{
suitName: "polkadot runtime",
command: "bench $ pallet dev pallet_contracts",
repo: "polkadot-sdk",
result: "bot bench substrate-pallet --pallet=pallet_contracts",
},
{
suitName: "substrate pallet",
command: "bench $ runtime polkadot pallet_contracts",
repo: "polkadot-sdk",
result: "bot bench polkadot-pallet --pallet=pallet_contracts",
},
{ suitName: "polkadot all", command: "bench-all $ kusama", repo: "polkadot", result: "bot bench-all polkadot" },
{
suitName: "cumulus bridge-hubs",
command: "bench $ xcm bridge-hub-kusama bridge-hubs pallet_name",
repo: "cumulus",
result: "bot bench cumulus-bridge-hubs --subcommand=xcm --runtime=bridge-hub-kusama --pallet=pallet_name",
},
{
suitName: "try-runtime default",
command: "try-runtime $ westend",
repo: "polkadot",
result: "bot try-runtime --chain=westend",
},
{ suitName: "try-runtime default", command: "try-runtime $ polkadot", repo: "polkadot", result: "bot try-runtime" },
];

describe("guessCommand", () => {
for (const { suitName, result, command, repo } of dataProvider) {
test(`test: [${suitName}]: "bot ${command}"`, async () => {
expect(await guessCommand({ logger }, command, repo)).toEqual(result);
});
}
});
144 changes: 144 additions & 0 deletions src/bot/parse/guessCommand.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { fetchCommandsConfiguration } from "src/command-configs/fetchCommandsConfiguration";
import { optionValuesToFlags } from "src/commander/commander";
import { config } from "src/config";
import { LoggerContext } from "src/logger";
import { CmdJson } from "src/schema/schema.cmd";

/**
* DISCLAIMER: don't even try to understand this code :D it's a mess
* This is throw-away shitcode💩, to simplify the migration messaging
* to be deleted soon after migration 🙏
*
* This wunderwaffe is heuristically guessing the command to be run
* helping the user to migrate to the new syntax
*/

export async function guessCommand(ctx: LoggerContext, command: string, repo: string): Promise<string> {
const { commandConfigs } = await fetchCommandsConfiguration(ctx, undefined, repo);
const { botPullRequestCommentMention } = config;

// extract first word from command
const commandName = command.split(" ")[0];
// extract args from command
const [_, args] = command.split("$ ");

const presets = commandConfigs[commandName]?.command?.presets || {};

if (Object.keys(presets)?.length > 0) {
const relatedPresets = Object.entries(presets).filter((preset) => preset[1].repos?.includes(repo));
const commonPresets = Object.entries(presets).filter((preset) => !preset[1].repos?.includes(repo));

if (relatedPresets.length > 0 || commonPresets.length > 0) {
const guessPreset = getWinnerPreset(args, relatedPresets) || getWinnerPreset(args, commonPresets);

if (guessPreset) {
const presetName = guessPreset.name === "default" ? "" : `${guessPreset.name} `;
return `${botPullRequestCommentMention} ${commandName} ${presetName}${optionValuesToFlags(
guessPreset.argsValues,
)}`.trim();
}
} else {
return `This command doesn't exist in "${repo}" repository.`;
}
}

return "";
}

function getWinnerPreset(
args: string,
presets: [string, NonNullable<CmdJson["command"]["presets"]>[keyof NonNullable<CmdJson["command"]["presets"]>]][],
):
| {
name: string;
argsValues: { [key: string]: string };
}
| undefined {
let winnerPreset: { name: string; argsValues: { [key: string]: string } } | undefined = undefined;
if (presets?.length > 0) {
if (presets?.length === 1) {
winnerPreset = { name: presets[0][0], argsValues: buildGuessedCommandArgs(presets[0][1], args) };
} else {
const bestMatch = presets.reduce((acc, presetEntry) => {
const [presetName, preset] = presetEntry;
if (preset.args && Object.values(preset.args).length > 0) {
const matchedCount = Object.entries(preset.args).reduce((a, argEntry) => {
const [_, arg] = argEntry;
if (Array.isArray(arg.type_one_of)) {
const match = args
.split(" ")
.find((argToMatch) => (arg.type_one_of as string[])?.find((type) => argToMatch.includes(type)));
if (match) {
a = a + 1;
}
}

if (typeof arg.type_string === "string" && args.includes(arg.type_string)) {
a = a + 1;
}
return a;
}, 0);

acc[presetName] = { rank: matchedCount, preset };
}

return acc;
}, {} as { [key: string]: { rank: number; preset: typeof presets[0][1] } });

const [winnerPresetId, winnerGuess] = Object.entries(bestMatch).sort((a, b) => b[1].rank - a[1].rank)[0];

if (Object.values(winnerGuess).length > 0) {
winnerPreset = { argsValues: buildGuessedCommandArgs(winnerGuess.preset, args), name: winnerPresetId };
}
}

return winnerPreset;
}
}

function buildGuessedCommandArgs(
preset: NonNullable<CmdJson["command"]["presets"]>[keyof NonNullable<CmdJson["command"]["presets"]>],
args: string,
): { [key: string]: string } {
// sort preset.args so that type_rule is the last one

return Object.entries(preset.args || {})
.sort(([, a], [, b]) => {
if (typeof a.type_rule === "string") {
return 1;
}
if (typeof b.type_rule === "string") {
return -1;
}
return 0;
})
.reduce((a, argEntry) => {
const [argName, arg] = argEntry;
if (Array.isArray(arg.type_one_of)) {
const match = args
.split(" ")
.find((argToMatch) => (arg.type_one_of as string[])?.find((type) => argToMatch.includes(type)));

if (match) {
// add to recommendation only if it's not 1st, because it'd be default anyway
if (arg.type_one_of.indexOf(match) > 0) {
a[argName] = match;
}
args = args.replace(match, "").trim();
}
} else if (typeof arg.type_string === "string") {
// we won't add to recommendation, because it's default anyway
// but clear from the args, so we could find the type_rule easier
if (args.includes(arg.type_string)) {
args = args.replace(arg.type_string, "").trim();
}
} else if (typeof arg.type_rule === "string") {
// assume that this arg is provided anyway
const isTheLastOne = args.split(" ").length === 1;
a[argName] = isTheLastOne ? args : (arg.example as string) || "custom_string";
} else {
console.log("unknown arg type", arg);
}
return a;
}, {} as { [key: string]: string });
}
Loading

0 comments on commit 1a093fd

Please sign in to comment.