fix(backport): do not push if there's no differences and fix error catch and replace ts-node by tsx

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>

Author: skjnldsv

.vscode/launch.json

@@ -5,14 +5,14 @@
 	"version": "1.0.0",
 	"configurations": [
 		{
-		"name": "TS-Node",
+		"name": "TSX Debug",
 		"type": "node",
 		"request": "launch",
-		"runtimeExecutable": "${workspaceRoot}/node_modules/.bin/ts-node",
+		"runtimeExecutable": "${workspaceRoot}/node_modules/.bin/tsx",
 		"program": "src/index.ts",
 		"cwd": "${workspaceRoot}",
 		"internalConsoleOptions": "openOnSessionStart",
 		"skipFiles": ["<node_internals>/**", "node_modules/**"]
 		}
 	]
-}
+}

docker-compose.yml

@@ -0,0 +1,19 @@
+version: "3.7"
+
+services:
+  app:
+    image: node:lts
+    working_dir: "/app"
+    command: sh -c "npm i && npm run serve"
+
+    ports:
+      - "3000:3000"
+
+    environment:
+      APP_ID: 12345
+      WEBHOOK_SECRET: 123456789abcdefghijklmnop
+      PRIVATE_KEY_PATH: /private-key.pem
+
+    volumes:
+      - ./backportbot:/app
+      - ./private-key.pem:/private-key.pem:ro

package.json

@@ -4,7 +4,7 @@
   "description": "A bot that backport commits to other branches",
   "main": "index.ts",
   "scripts": {
-    "serve": "ts-node src/index.ts",
+    "serve": "tsx src/index.ts",
     "lint": "eslint src/**/*.ts",
     "lint:fix": "eslint src/**/*.ts --fix"
   },
@@ -28,11 +28,10 @@
   "dependencies": {
     "@octokit/app": "^14.0.2",
     "@octokit/rest": "^20.0.2",
-    "@octokit/webhooks": "^12.0.11",
-    "octokit": "^3.1.2",
     "p-queue": "^8.0.1",
     "simple-git": "^3.22.0",
-    "ts-node": "^10.9.2"
+    "ts-node": "^10.9.2",
+    "tsx": "^4.7.0"
   },
   "devDependencies": {
     "@tsconfig/recommended": "^1.0.3",
@@ -41,5 +40,9 @@
     "@typescript-eslint/parser": "^6.18.1",
     "eslint": "^8.56.0",
     "typescript": "^5.3.3"
+  },
+  "engines": {
+    "node": "^20.0.0",
+    "npm": "^9.0.0"
   }
 }

src/appUtils.ts

@@ -1,11 +1,10 @@
 import { App } from '@octokit/app'
-import { join } from 'node:path'
 import { readFileSync } from 'node:fs'
 
-import { APP_ID, PRIVATE_KEY_FILENAME, ROOT_DIR, WEBHOOK_SECRET } from './constants'
+import { APP_ID, PRIVATE_KEY_PATH, WEBHOOK_SECRET } from './constants'
 
 const initApp = (): App => {
-	const privateKey = readFileSync(join(ROOT_DIR, PRIVATE_KEY_FILENAME), 'utf-8').toString()
+	const privateKey = readFileSync(PRIVATE_KEY_PATH, 'utf-8').toString()
 	return new App({
 		appId: APP_ID,
 		privateKey,

src/backport.ts

@@ -1,7 +1,7 @@
 import { existsSync, rmSync } from 'node:fs'
 import { Octokit } from '@octokit/rest'
 
-import { cherryPickCommits, cloneAndCacheRepo, hasEmptyCommits, hasSkipCiCommits, pushBranch } from './gitUtils'
+import { cherryPickCommits, cloneAndCacheRepo, hasDiff, hasEmptyCommits, hasSkipCiCommits, pushBranch } from './gitUtils'
 import { CherryPickResult, Task } from './constants'
 import { debug, error, info, warn } from './logUtils'
 import { Reaction, addReaction, getAuthToken, getAvailableLabels, getLabelsFromPR, getAvailableMilestones, requestReviewers, getReviewers, createBackportPullRequest, setPRLabels, setPRMilestone, getChangesFromPR, updatePRBody, commentOnPR, assignToPR } from './githubUtils'
@@ -27,8 +27,7 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 				tmpDir = await cloneAndCacheRepo(task, backportBranch)
 				info(task, `Cloned to ${tmpDir}`)
 			} catch (e) {
-				reject(`Failed to clone repository: ${e.message}`)
-				throw e
+				throw new Error(`Failed to clone repository: ${e.message}`)
 			}
 
 			// Cherry pick the commits
@@ -40,17 +39,20 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 					info(task, `Cherry picking commits successful`)
 				}
 			} catch (e) {
-				reject(`Failed to cherry pick commits: ${e.message}`)
-				throw e
+				throw new Error(`Failed to cherry pick commits: ${e.message}`)
+			}
+
+			// Check if there are any changes to backport
+			if (!await hasDiff(tmpDir, task.branch, backportBranch)) {
+				throw new Error(`No changes found in backport branch`)
 			}
 
 			// Push the branch
 			try {
 				await pushBranch(task, tmpDir, token)
 				info(task, `Pushed branch ${backportBranch}`)
 			} catch (e) {
-				reject(`Failed to push branch: ${e.message}`)
-				throw e
+				throw new Error(`Failed to push branch: ${e.message}`)
 			}
 
 			// Create the pull request
@@ -71,11 +73,10 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 					await requestReviewers(octokit, task, prNumber, [task.author])
 					info(task, `Requested reviews from ${[...reviewers, task.author].join(', ')}`)
 				} catch (e) {
-					reject(`Failed to request reviews: ${e.message}`)
+					throw new Error(`Failed to request reviews: ${e.message}`)
 				}
 			} catch (e) {
-				reject(`Failed to create pull request: ${e.message}`)
-				throw e
+				throw new Error(`Failed to create pull request: ${e.message}`)
 			}
 
 			// Get labels from original PR and set them on the new PR
@@ -86,7 +87,8 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 				await setPRLabels(octokit, task, prNumber, labels)
 				info(task, `Set labels: ${labels.join(', ')}`)
 			} catch (e) {
-				reject(`Failed to get labels: ${e.message}`)
+				error(task, `Failed to get and set labels: ${e.message}`)
+				// continue, this is not a fatal error
 			}
 
 			// Find new appropriate Milestone and set it on the new PR
@@ -96,15 +98,17 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 				await setPRMilestone(octokit, task, prNumber, milestone)
 				info(task, `Set milestone: ${milestone.title}`)
 			} catch (e) {
-				warn(task, `Failed to find appropriate milestone: ${e.message}`)
+				error(task, `Failed to find appropriate milestone: ${e.message}`)
+				// continue, this is not a fatal error
 			}
 
 			// Assign the PR to the author of the original PR
 			try {
 				await assignToPR(octokit, task, prNumber, [task.author])
 				info(task, `Assigned original author: ${task.author}`)
 			} catch (e) {
-				reject(`Failed to assign PR: ${e.message}`)
+				error(task, `Failed to assign PR: ${e.message}`)
+				// continue, this is not a fatal error
 			}
 
 			// Compare the original PR with the new PR
@@ -125,10 +129,12 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 						await updatePRBody(octokit, task, prNumber, newBody)
 					}
 				} catch (e) {
-					reject(`Failed to update PR body: ${e.message}`)
+					error(task, `Failed to update PR body: ${e.message}`)
+					// continue, this is not a fatal error
 				}
 			} catch (e) {
-				reject(`Failed to compare changes: ${e.message}`)
+				error(task, `Failed to compare changes: ${e.message}`)
+				// continue, this is not a fatal error
 			}
 
 			// Success! We're done here
@@ -140,8 +146,11 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 				const failureComment = getFailureCommentBody(task, backportBranch, e?.message)
 				await commentOnPR(octokit, task, failureComment)
 			} catch (e) {
-				reject(`Failed to comment failure on PR: ${e.message}`)
+				error(task, `Failed to comment failure on PR: ${e.message}`)
+				// continue, this is not a fatal error
 			}
+
+			reject(`Failed to backport: ${e.message}`)
 		}
 
 		// Remove the temp dir if it exists

src/constants.ts

@@ -1,21 +1,20 @@
-import { resolve } from 'node:path'
+import { join, resolve } from 'node:path'
 
 export const SERVE_PORT = 8123
 export const SERVE_HOST = '0.0.0.0'
 
-
-export const APP_ID = process.env.APP_ID
-export const WEBHOOK_SECRET = process.env.WEBHOOK_SECRET
-export const PRIVATE_KEY_FILENAME = 'private-key.pem'
-
 export const ROOT_DIR = resolve(__dirname + '/../')
 export const CACHE_DIRNAME = 'cache' // relative to the root dir
 export const WORK_DIRNAME = 'work' // relative to the root dir
 
+export const APP_ID = process.env.APP_ID || 0
+export const WEBHOOK_SECRET = process.env.WEBHOOK_SECRET || ''
+export const PRIVATE_KEY_FILENAME = 'private-key.pem'
+export const PRIVATE_KEY_PATH = process.env.PRIVATE_KEY_PATH || join(ROOT_DIR, PRIVATE_KEY_FILENAME)
+
 export const LOG_FILE = 'backport.log'
-export const QUEUE_FILE = 'queue.json'
 
-export const COMMAND_PREFIX = '/skjnldsv-backport'
+export const COMMAND_PREFIX = '/backport'
 export const TO_SEPARATOR = ' to '
 export const COMMIT_REGEX = /^\b[0-9a-f]{7,40}$\b/i
 export const BRANCH_REGEX = /^\b[a-z0-9-_./]{1,100}\b$/i

src/gitUtils.ts

@@ -156,16 +156,19 @@ export const hasSkipCiCommits = async (repoRoot: string, commits: number): Promi
 	return commitMessages.some(message => message.includes('[skip ci]'))
 }
 
+export const hasDiff = async (repoRoot: string, base: string, head: string): Promise<boolean> => {
+	const git = simpleGit(repoRoot)
+	const diff = await git.raw(['diff', '--stats', base, head])
+	return diff !== ''
+}
 
 export const hasEmptyCommits = async (repoRoot: string, commits: number): Promise<boolean> => {
-	const git = simpleGit(repoRoot)
 	let hasEmptyCommits = false
 	for (let count = 0; count < commits; count++) {
-		const diff = await git.raw(['diff', '--stat', `HEAD~${count}`, `HEAD~${count + 1}`])
-		if (diff === '') {
+		if (!await hasDiff(repoRoot, `HEAD~${count}`, `HEAD~${count + 1}`)) {
 			hasEmptyCommits = true
 			break
 		}
 	}
 	return hasEmptyCommits
-}
+}

src/index.ts

@@ -4,7 +4,7 @@ import { info, error } from 'node:console'
 import { Octokit } from '@octokit/rest'
 
 import { addToQueue } from './queue'
-import { CACHE_DIRNAME, COMMAND_PREFIX, LABEL_BACKPORT, ROOT_DIR, SERVE_HOST, SERVE_PORT, TO_SEPARATOR, Task } from './constants'
+import { CACHE_DIRNAME, COMMAND_PREFIX, LABEL_BACKPORT, PRIVATE_KEY_PATH, ROOT_DIR, SERVE_HOST, SERVE_PORT, TO_SEPARATOR, Task, WEBHOOK_SECRET } from './constants'
 import { extractBranchFromPayload, extractCommitsFromPayload } from './payloadUtils'
 import { getApp } from './appUtils'
 import { Reaction, addPRLabel, addReaction, getAuthToken, getBackportRequestsFromPR, getCommitsForPR, removePRLabel, setPRLabels } from './githubUtils'
@@ -227,14 +227,17 @@ app.webhooks.on(['issue_comment.created'], async ({ payload }) => {
 
 			// If the PR is already merged, we can start the backport right away
 			if (isMerged) {
-				addToQueue(task).then(async () => {
+				try {
+					await addToQueue(task)
 					// Remove the backport label from the PR on success
 					try {
 						await removePRLabel(authOctokit, task, prNumber, LABEL_BACKPORT)
 					} catch (e) {
 						error(`\nFailed to remove backport label from PR ${htmlUrl}: ${e.message}`)
 					}
-				})
+				} catch (e) {
+					// Safely ignore
+				}
 			}
 		} catch (e) {
 			// This should really not happen, but if it does, we want to know about it
@@ -256,11 +259,14 @@ Subscribed events: ${data.events}`)
 		process.exit(1)
 	}
 
+	const obfuscatedWebhookSecret = WEBHOOK_SECRET.slice(0, 8) + '*'.repeat(WEBHOOK_SECRET.length - 8)
 	info(`Listening on ${SERVE_HOST}:${SERVE_PORT}`)
 	info(`├ Authenticated as ${data.name}`)
 	info(`├ Monitoring events`, data.events)
 	info(`├ Command prefix: ${COMMAND_PREFIX}`)
 	info(`├ Root dir: ${ROOT_DIR}`)
-	info(`└ Cache dir: ${ROOT_DIR}/${CACHE_DIRNAME}`)
+	info(`├ Cache dir: ${ROOT_DIR}/${CACHE_DIRNAME}`)
+	info(`├ Private key in ${PRIVATE_KEY_PATH}`)
+	info(`└ Webhook secret is ${obfuscatedWebhookSecret}`)
 	createServer(createNodeMiddleware(app.webhooks)).listen(SERVE_PORT, SERVE_HOST)
 })

tsconfig.json

@@ -1,6 +1,6 @@
 {
 	"extends": "@tsconfig/recommended",
-	"include": ["src/**/*"],
+	"include": ["src/**/*.ts"],
 	"exclude": ["node_modules"],
 	"compilerOptions": {
 		"allowJs": true