From 86d8c1ac937d864d14e62b447c70a4625c3faeb5 Mon Sep 17 00:00:00 2001 From: Hikari Date: Fri, 20 Feb 2026 19:31:03 -0800 Subject: [PATCH] feat: add auto-merge for non-major dependency updates Minori now automatically merges dependency update PRs when: - The update is NOT a major version bump (to avoid breaking changes) - The CI checks pass (status = "success") - An existing PR for the dependency update is found This reduces manual work for safe, non-breaking dependency updates whilst still requiring human review for potentially breaking changes. Changes: - Add version comparison utility to detect major version bumps - Add Gitea service methods for commit status and PR merging - Add auto-merge logic to update orchestrator - Add comprehensive tests for new functionality --- src/services/giteaService.ts | 78 ++++++++++++ src/services/updateOrchestratorService.ts | 120 ++++++++++++++++-- src/types/gitea.types.ts | 29 ++++- src/utils/versionComparison.ts | 64 ++++++++++ test/services/giteaService.spec.ts | 83 +++++++++++- .../updateOrchestratorService.spec.ts | 11 ++ test/utils/versionComparison.spec.ts | 108 ++++++++++++++++ 7 files changed, 479 insertions(+), 14 deletions(-) create mode 100644 src/utils/versionComparison.ts create mode 100644 test/utils/versionComparison.spec.ts diff --git a/src/services/giteaService.ts b/src/services/giteaService.ts index 86432c6..7f76b2e 100644 --- a/src/services/giteaService.ts +++ b/src/services/giteaService.ts @@ -7,6 +7,7 @@ import axios, { isAxiosError, type AxiosInstance } from "axios"; import { config } from "../config.js"; import type { + GiteaCombinedStatus, GiteaFile, GiteaPullRequest, GiteaRepository, @@ -142,6 +143,83 @@ class GiteaService { ); return data; } + + /** + * Gets the combined commit status for a specific commit. + * @param owner - The repository owner. + * @param repo - The repository name. + * @param sha - The commit SHA. + * @returns The combined status of all checks for the commit. + */ + public async getCommitStatus( + owner: string, + repo: string, + sha: string, + ): Promise { + const { data } = await this.client.get( + `/repos/${owner}/${repo}/commits/${sha}/status`, + ); + return data; + } + + /** + * Merges a pull request. + * @param owner - The repository owner. + * @param repo - The repository name. + * @param index - The pull request index number. + * @returns True if the merge was successful, false otherwise. + */ + public async mergePullRequest( + owner: string, + repo: string, + index: number, + ): Promise { + try { + await this.client.post( + `/repos/${owner}/${repo}/pulls/${String(index)}/merge`, + { + /* eslint-disable @typescript-eslint/naming-convention -- Gitea API uses snake_case */ + Do: "merge", + MergeMessageField: "", + MergeTitleField: "", + delete_branch_after_merge: true, + force_merge: false, + head_commit_id: "", + merge_when_checks_succeed: false, + /* eslint-enable @typescript-eslint/naming-convention -- End Gitea API */ + }, + ); + return true; + } catch (error) { + if (isAxiosError(error)) { + return false; + } + throw error; + } + } + + /** + * Deletes a repository branch by name. + * @param owner - The repository owner. + * @param repo - The repository name. + * @param branch - The branch name to remove. + * @returns True if successful, false otherwise. + */ + public async deleteBranch( + owner: string, + repo: string, + branch: string, + ): Promise { + try { + await this.client.delete(`/repos/${owner}/${repo}/branches/${branch}`); + return true; + } catch (error) { + if (isAxiosError(error)) { + return false; + } + throw error; + } + } } export { GiteaService }; diff --git a/src/services/updateOrchestratorService.ts b/src/services/updateOrchestratorService.ts index 5af57a8..55760c7 100644 --- a/src/services/updateOrchestratorService.ts +++ b/src/services/updateOrchestratorService.ts @@ -6,6 +6,10 @@ import { config } from "../config.js"; import { logger } from "../utils/logger.js"; +import { + isMajorVersionBump, + stripVersionPrefix, +} from "../utils/versionComparison.js"; import { DependencyAnalyzerService } from "./dependencyAnalyzerService.js"; import { GiteaService } from "./giteaService.js"; import { @@ -17,15 +21,6 @@ import { NpmService } from "./npmService.js"; import type { GiteaRepository } from "../types/gitea.types.js"; import type { DependencyUpdate, PackageJson } from "../types/package.types.js"; -/** - * Strips version prefix characters from a version string. - * @param version - The version string with potential prefixes. - * @returns The version without prefix characters. - */ -const stripVersionPrefix = (version: string): string => { - return version.replace(/^[<=>^~]*/, ""); -}; - /** * Generates the body content for a PR. * @param update - The dependency update information. @@ -142,6 +137,103 @@ class UpdateOrchestratorService { await logger.log("info", "Dependency update check complete!"); } + /** + * Attempts to merge an existing PR after checking CI status. + * @param repo - The repository information. + * @param update - The dependency update details. + * @param matchingPR - The existing PR with head SHA and number. + * @param matchingPR.head - The PR head information. + * @param matchingPR.head.sha - The commit SHA to check. + * @param matchingPR.number - The PR number for merging. + * @returns True if merge was attempted, false otherwise. + */ + private async attemptPRMerge( + repo: GiteaRepository, + update: DependencyUpdate, + matchingPR: { head: { sha: string }; number: number }, + ): Promise { + const commitStatus = await this.giteaService.getCommitStatus( + config.giteaOrg, + repo.name, + matchingPR.head.sha, + ); + + if (commitStatus.state !== "success") { + await logger.log( + "info", + ` PR exists for ${update.packageName} but CI status is ${commitStatus.state}, skipping auto-merge...`, + ); + return true; + } + + await logger.log( + "info", + ` Auto-merging PR for ${update.packageName} (CI passed, non-major bump)...`, + ); + + const merged = await this.giteaService.mergePullRequest( + config.giteaOrg, + repo.name, + matchingPR.number, + ); + + if (merged) { + await logger.log( + "info", + ` Successfully merged PR for ${update.packageName}`, + ); + return true; + } + + await logger.log( + "warn", + ` Failed to merge PR for ${update.packageName}`, + ); + return true; + } + + /** + * Checks if an existing PR can be auto-merged based on CI status and version bump type. + * @param repo - The repository information. + * @param update - The dependency update details. + * @param branchName - The branch name for the PR. + * @returns True if the PR exists and was handled, false otherwise. + */ + private async checkAndMergeExistingPR( + repo: GiteaRepository, + update: DependencyUpdate, + branchName: string, + ): Promise { + const existingPRs = await this.giteaService.listPullRequests( + config.giteaOrg, + repo.name, + "open", + ); + + const matchingPR = existingPRs.find((pr) => { + return pr.head.ref === branchName; + }); + + if (matchingPR === undefined) { + return false; + } + + const isMajorBump = isMajorVersionBump( + update.currentVersion, + update.latestVersion, + ); + + if (isMajorBump) { + await logger.log( + "info", + ` PR exists for ${update.packageName} but is a major version bump, skipping auto-merge...`, + ); + return true; + } + + return await this.attemptPRMerge(repo, update, matchingPR); + } + /** * Creates or updates a PR for a dependency update. * @param repo - The repository information. @@ -157,6 +249,16 @@ class UpdateOrchestratorService { = `${config.prBranchPrefix}${update.packageName.replaceAll(/[/@]/g, "-")}`; try { + const existingPRMerged = await this.checkAndMergeExistingPR( + repo, + update, + branchName, + ); + + if (existingPRMerged) { + return; + } + const result = await createOrUpdateBranch({ branchName: branchName, clonedRepo: clonedRepo, diff --git a/src/types/gitea.types.ts b/src/types/gitea.types.ts index d598599..7412cba 100644 --- a/src/types/gitea.types.ts +++ b/src/types/gitea.types.ts @@ -39,6 +39,33 @@ interface GiteaPullRequest { state: "closed" | "open"; title: string; } + +interface GiteaCommitStatus { + context: string; + created_at: string; + description: string; + id: number; + state: "error" | "failure" | "pending" | "success" | "warning"; + target_url: string; + updated_at: string; + url: string; +} + +interface GiteaCombinedStatus { + commit_url: string; + repository: GiteaRepository; + sha: string; + state: "error" | "failure" | "pending" | "success" | "warning"; + statuses: Array; + total_count: number; + url: string; +} /* eslint-enable @typescript-eslint/naming-convention -- End Gitea API types */ -export type { GiteaFile, GiteaPullRequest, GiteaRepository }; +export type { + GiteaCombinedStatus, + GiteaCommitStatus, + GiteaFile, + GiteaPullRequest, + GiteaRepository, +}; diff --git a/src/utils/versionComparison.ts b/src/utils/versionComparison.ts new file mode 100644 index 0000000..a6a467b --- /dev/null +++ b/src/utils/versionComparison.ts @@ -0,0 +1,64 @@ +/** + * @copyright NHCarrigan + * @license Naomi's Public License + * @author Naomi Carrigan + */ + +/** + * Strips version prefix characters from a version string. + * @param version - The version string with potential prefixes. + * @returns The version without prefix characters. + */ +const stripVersionPrefix = (version: string): string => { + return version.replace(/^[<=>^~]*/, ""); +}; + +/** + * Parses a semantic version string into its components. + * @param version - The version string to parse (e.g., "1.2.3"). + * @returns An object with major, minor, and patch numbers, or null if invalid. + */ +const parseVersion = ( + version: string, +): { major: number; minor: number; patch: number } | null => { + const cleaned = stripVersionPrefix(version); + const parts = cleaned.split("."); + + if (parts.length < 3) { + return null; + } + + const major = Number.parseInt(parts[0] ?? "0", 10); + const minor = Number.parseInt(parts[1] ?? "0", 10); + const patchPart = parts[2] ?? "0"; + const patch = Number.parseInt(patchPart.split("-")[0] ?? "0", 10); + + if (Number.isNaN(major) || Number.isNaN(minor) || Number.isNaN(patch)) { + return null; + } + + return { major, minor, patch }; +}; + +/** + * Determines if a version update is a major version bump. + * A major bump occurs when the major version number increases. + * @param fromVersion - The current version. + * @param toVersion - The target version. + * @returns True if this is a major version bump, false otherwise. + */ +const isMajorVersionBump = ( + fromVersion: string, + toVersion: string, +): boolean => { + const from = parseVersion(fromVersion); + const to = parseVersion(toVersion); + + if (from === null || to === null) { + return false; + } + + return to.major > from.major; +}; + +export { isMajorVersionBump, stripVersionPrefix }; diff --git a/test/services/giteaService.spec.ts b/test/services/giteaService.spec.ts index 5e03157..8344000 100644 --- a/test/services/giteaService.spec.ts +++ b/test/services/giteaService.spec.ts @@ -6,11 +6,16 @@ /* eslint-disable vitest/valid-expect -- Test expectations don't need messages */ /* eslint-disable max-lines-per-function -- Test suites require many test cases */ +/* eslint-disable max-lines -- Test suites naturally have many cases */ +/* eslint-disable max-statements -- Test suites naturally have many statements */ /* eslint-disable @typescript-eslint/consistent-type-assertions -- Required for mocking */ /* eslint-disable @typescript-eslint/consistent-type-imports -- Dynamic imports */ /* eslint-disable @typescript-eslint/naming-convention -- Environment variables and Gitea API format */ /* eslint-disable max-nested-callbacks -- Vitest structure requires nested callbacks */ /* eslint-disable vitest/require-to-throw-message -- Generic throw assertion */ +/* eslint-disable vitest/prefer-to-be-truthy -- toBe(true) is clearer for boolean functions */ +/* eslint-disable vitest/prefer-to-be-falsy -- toBe(false) is clearer for boolean functions */ +/* eslint-disable stylistic/max-len -- Test files have long object literals */ import axios, { AxiosError, type AxiosResponse } from "axios"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -24,8 +29,9 @@ vi.mock("axios", async() => { default: { create: vi.fn(() => { return { - get: vi.fn(), - post: vi.fn(), + delete: vi.fn(), + get: vi.fn(), + post: vi.fn(), }; }), }, @@ -69,6 +75,8 @@ describe("giteaService", () => { let mockGet: ReturnType; // eslint-disable-next-line @typescript-eslint/init-declarations -- Reassigned in beforeEach let mockPost: ReturnType; + // eslint-disable-next-line @typescript-eslint/init-declarations -- Reassigned in beforeEach + let mockDelete: ReturnType; const originalEnvironment = process.env; beforeEach(() => { @@ -77,9 +85,11 @@ describe("giteaService", () => { mockGet = vi.fn(); mockPost = vi.fn(); + mockDelete = vi.fn(); vi.mocked(axios.create).mockReturnValue({ - get: mockGet, - post: mockPost, + delete: mockDelete, + get: mockGet, + post: mockPost, } as unknown as ReturnType); giteaService = new GiteaService(); @@ -306,4 +316,69 @@ describe("giteaService", () => { { params: { state: "closed" } }, ); }); + + it("should get commit status", async() => { + expect.assertions(2); + const mockStatus = { + commit_url: "https://git.nhcarrigan.com/api/v1/repos/owner/repo/commits/abc123", + repository: createMockRepository({ id: 1, name: "test-repo" }), + sha: "abc123", + state: "success", + statuses: [], + total_count: 0, + url: "https://git.nhcarrigan.com/api/v1/repos/owner/repo/commits/abc123/status", + }; + mockGet.mockResolvedValueOnce({ data: mockStatus }); + const result = await giteaService.getCommitStatus("owner", "repo", "abc123"); + expect(result).toStrictEqual(mockStatus); + expect(mockGet).toHaveBeenCalledWith("/repos/owner/repo/commits/abc123/status"); + }); + + it("should merge a pull request successfully", async() => { + expect.assertions(2); + mockPost.mockResolvedValueOnce({ data: {} }); + const result = await giteaService.mergePullRequest("owner", "repo", 1); + expect(result).toBe(true); + expect(mockPost).toHaveBeenCalledWith("/repos/owner/repo/pulls/1/merge", { + Do: "merge", + MergeMessageField: "", + MergeTitleField: "", + delete_branch_after_merge: true, + force_merge: false, + head_commit_id: "", + merge_when_checks_succeed: false, + }); + }); + + it("should return false when merge fails", async() => { + expect.assertions(1); + const axiosError = new AxiosError("Merge conflict"); + mockPost.mockRejectedValueOnce(axiosError); + const result = await giteaService.mergePullRequest("owner", "repo", 1); + expect(result).toBe(false); + }); + + it("should delete a branch successfully", async() => { + expect.assertions(2); + mockDelete.mockResolvedValueOnce({ data: {} }); + const result = await giteaService.deleteBranch( + "owner", + "repo", + "feature-branch", + ); + expect(result).toBe(true); + expect(mockDelete).toHaveBeenCalledWith("/repos/owner/repo/branches/feature-branch"); + }); + + it("should return false when branch deletion fails", async() => { + expect.assertions(1); + const axiosError = new AxiosError("Branch not found"); + mockDelete.mockRejectedValueOnce(axiosError); + const result = await giteaService.deleteBranch( + "owner", + "repo", + "nonexistent-branch", + ); + expect(result).toBe(false); + }); }); diff --git a/test/services/updateOrchestratorService.spec.ts b/test/services/updateOrchestratorService.spec.ts index ac3b0ac..f681d19 100644 --- a/test/services/updateOrchestratorService.spec.ts +++ b/test/services/updateOrchestratorService.spec.ts @@ -18,6 +18,8 @@ const mockGiteaGetFileContent = vi.fn(); const mockGiteaListOrgRepositories = vi.fn(); const mockGiteaCreatePullRequest = vi.fn(); const mockGiteaListPullRequests = vi.fn(); +const mockGiteaGetCommitStatus = vi.fn(); +const mockGiteaMergePullRequest = vi.fn(); const mockNpmGetPackageChangelog = vi.fn(); const mockNpmGetPackageInfo = vi.fn(); const mockAnalyzePackageJson = vi.fn(); @@ -39,9 +41,11 @@ vi.mock("../../src/services/giteaService.js", () => { GiteaService: class MockGiteaService { public createPullRequest = mockGiteaCreatePullRequest; + public getCommitStatus = mockGiteaGetCommitStatus; public getFileContent = mockGiteaGetFileContent; public listOrgRepositories = mockGiteaListOrgRepositories; public listPullRequests = mockGiteaListPullRequests; + public mergePullRequest = mockGiteaMergePullRequest; }, }; }); @@ -221,6 +225,7 @@ describe("updateOrchestratorService", () => { const mockUpdates = [ createMockUpdate() ]; mockGiteaListOrgRepositories.mockResolvedValue(mockRepos); mockGiteaGetFileContent.mockResolvedValue(mockFileContent); + mockGiteaListPullRequests.mockResolvedValue([]); mockAnalyzePackageJson.mockResolvedValue(mockUpdates); mockNpmGetPackageChangelog.mockResolvedValue("## Changelog"); mockCloneRepository.mockResolvedValue(createMockClonedRepo()); @@ -267,6 +272,7 @@ describe("updateOrchestratorService", () => { const mockUpdates = [ createMockUpdate() ]; mockGiteaListOrgRepositories.mockResolvedValue(mockRepos); mockGiteaGetFileContent.mockResolvedValue(mockFileContent); + mockGiteaListPullRequests.mockResolvedValue([]); mockAnalyzePackageJson.mockResolvedValue(mockUpdates); mockCloneRepository.mockResolvedValue(createMockClonedRepo()); mockCreateOrUpdateBranch.mockResolvedValue({ @@ -289,6 +295,7 @@ describe("updateOrchestratorService", () => { const mockUpdates = [ createMockUpdate() ]; mockGiteaListOrgRepositories.mockResolvedValue(mockRepos); mockGiteaGetFileContent.mockResolvedValue(mockFileContent); + mockGiteaListPullRequests.mockResolvedValue([]); mockAnalyzePackageJson.mockResolvedValue(mockUpdates); mockCloneRepository.mockResolvedValue(createMockClonedRepo()); mockCreateOrUpdateBranch.mockResolvedValue({ @@ -312,6 +319,7 @@ describe("updateOrchestratorService", () => { const mockUpdates = [ createMockUpdate() ]; mockGiteaListOrgRepositories.mockResolvedValue(mockRepos); mockGiteaGetFileContent.mockResolvedValue(mockFileContent); + mockGiteaListPullRequests.mockResolvedValue([]); mockAnalyzePackageJson.mockResolvedValue(mockUpdates); mockCloneRepository.mockResolvedValue(createMockClonedRepo()); mockCreateOrUpdateBranch.mockResolvedValue({ @@ -354,6 +362,7 @@ describe("updateOrchestratorService", () => { const mockUpdates = [ createMockUpdate() ]; mockGiteaListOrgRepositories.mockResolvedValue(mockRepos); mockGiteaGetFileContent.mockResolvedValue(mockFileContent); + mockGiteaListPullRequests.mockResolvedValue([]); mockAnalyzePackageJson.mockResolvedValue(mockUpdates); mockCloneRepository.mockResolvedValue(createMockClonedRepo()); mockCreateOrUpdateBranch.mockResolvedValue({ @@ -377,6 +386,7 @@ describe("updateOrchestratorService", () => { const mockUpdates = [ createMockUpdate() ]; mockGiteaListOrgRepositories.mockResolvedValue(mockRepos); mockGiteaGetFileContent.mockResolvedValue(mockFileContent); + mockGiteaListPullRequests.mockResolvedValue([]); mockAnalyzePackageJson.mockResolvedValue(mockUpdates); mockNpmGetPackageChangelog.mockResolvedValue("## Changelog"); mockCloneRepository.mockResolvedValue(createMockClonedRepo()); @@ -426,6 +436,7 @@ describe("updateOrchestratorService", () => { const mockCleanup = vi.fn(); mockGiteaListOrgRepositories.mockResolvedValue(mockRepos); mockGiteaGetFileContent.mockResolvedValue(mockFileContent); + mockGiteaListPullRequests.mockResolvedValue([]); mockAnalyzePackageJson.mockResolvedValue(mockUpdates); mockCloneRepository.mockResolvedValue(createMockClonedRepo(mockCleanup)); mockCreateOrUpdateBranch.mockResolvedValue({ diff --git a/test/utils/versionComparison.spec.ts b/test/utils/versionComparison.spec.ts new file mode 100644 index 0000000..1e9c62f --- /dev/null +++ b/test/utils/versionComparison.spec.ts @@ -0,0 +1,108 @@ +/** + * @copyright NHCarrigan + * @license Naomi's Public License + * @author Naomi Carrigan + */ + +/* eslint-disable vitest/valid-expect -- Test expectations don't need messages */ +/* eslint-disable max-lines-per-function -- Test suites naturally have many cases */ +/* eslint-disable max-nested-callbacks -- Vitest structure requires nesting */ +/* eslint-disable vitest/prefer-to-be-truthy -- toBe(true) is clearer for boolean functions */ +/* eslint-disable vitest/prefer-to-be-falsy -- toBe(false) is clearer for boolean functions */ + +import { describe, expect, it } from "vitest"; +import { + isMajorVersionBump, + stripVersionPrefix, +} from "../../src/utils/versionComparison.js"; + +describe("versionComparison", () => { + describe("stripVersionPrefix", () => { + it("should strip caret prefix", () => { + expect.assertions(1); + expect(stripVersionPrefix("^1.2.3")).toBe("1.2.3"); + }); + + it("should strip tilde prefix", () => { + expect.assertions(1); + expect(stripVersionPrefix("~1.2.3")).toBe("1.2.3"); + }); + + it("should strip greater than prefix", () => { + expect.assertions(1); + expect(stripVersionPrefix(">1.2.3")).toBe("1.2.3"); + }); + + it("should strip less than prefix", () => { + expect.assertions(1); + expect(stripVersionPrefix("<1.2.3")).toBe("1.2.3"); + }); + + it("should strip equals prefix", () => { + expect.assertions(1); + expect(stripVersionPrefix("=1.2.3")).toBe("1.2.3"); + }); + + it("should strip multiple prefix characters", () => { + expect.assertions(1); + expect(stripVersionPrefix(">=1.2.3")).toBe("1.2.3"); + }); + + it("should return version without prefix unchanged", () => { + expect.assertions(1); + expect(stripVersionPrefix("1.2.3")).toBe("1.2.3"); + }); + }); + + describe("isMajorVersionBump", () => { + it("should detect major version bump", () => { + expect.assertions(1); + expect(isMajorVersionBump("1.2.3", "2.0.0")).toBe(true); + }); + + it("should detect major version bump with prefixes", () => { + expect.assertions(1); + expect(isMajorVersionBump("^1.2.3", "^2.0.0")).toBe(true); + }); + + it("should not detect minor version bump as major", () => { + expect.assertions(1); + expect(isMajorVersionBump("1.2.3", "1.3.0")).toBe(false); + }); + + it("should not detect patch version bump as major", () => { + expect.assertions(1); + expect(isMajorVersionBump("1.2.3", "1.2.4")).toBe(false); + }); + + it("should handle version with pre-release tags", () => { + expect.assertions(1); + expect(isMajorVersionBump("1.2.3", "2.0.0-beta.1")).toBe(true); + }); + + it("should return false for invalid from version", () => { + expect.assertions(1); + expect(isMajorVersionBump("invalid", "2.0.0")).toBe(false); + }); + + it("should return false for invalid to version", () => { + expect.assertions(1); + expect(isMajorVersionBump("1.2.3", "invalid")).toBe(false); + }); + + it("should return false for both invalid versions", () => { + expect.assertions(1); + expect(isMajorVersionBump("invalid", "also-invalid")).toBe(false); + }); + + it("should handle 0.x.x to 1.x.x as major bump", () => { + expect.assertions(1); + expect(isMajorVersionBump("0.9.5", "1.0.0")).toBe(true); + }); + + it("should not detect same version as major bump", () => { + expect.assertions(1); + expect(isMajorVersionBump("1.2.3", "1.2.3")).toBe(false); + }); + }); +});