diff --git a/packages/loopover-engine/src/index.ts b/packages/loopover-engine/src/index.ts index d5bd07604c..df3c09ba4d 100644 --- a/packages/loopover-engine/src/index.ts +++ b/packages/loopover-engine/src/index.ts @@ -707,3 +707,7 @@ export { // hashing both Orb's self-host collector and AMS's export path use before repo/PR identifiers leave the // instance. export { generateAnonSecret, hmacAnonymize } from "./telemetry/anonymize.js"; + +// Pure PR-target-key parser (#4882) -- parses `"/#"` into its parts; extracted so the +// D1-heavy repositories access layer no longer carries this stranded pure logic. +export { parsePullRequestTargetKey } from "./parse-pull-request-target-key.js"; diff --git a/packages/loopover-engine/src/parse-pull-request-target-key.ts b/packages/loopover-engine/src/parse-pull-request-target-key.ts new file mode 100644 index 0000000000..adffc086d1 --- /dev/null +++ b/packages/loopover-engine/src/parse-pull-request-target-key.ts @@ -0,0 +1,23 @@ +/** + * Parse a pull-request target key of the form `"/#"` into its repo + * full-name and pull-number parts. Pure string parsing: returns `null` for any malformed + * input -- falsy input, a missing / leading / trailing `#`, a repo half without a `/`, or a + * non-integer / non-positive pull number. + */ +export function parsePullRequestTargetKey( + targetKey: string | null | undefined, +): { repoFullName: string; pullNumber: number } | null { + if (!targetKey) return null; + const delimiter = targetKey.lastIndexOf("#"); + if (delimiter <= 0 || delimiter === targetKey.length - 1) return null; + const repoFullName = targetKey.slice(0, delimiter); + const pullNumber = Number(targetKey.slice(delimiter + 1)); + if ( + !repoFullName.includes("/") || + !Number.isInteger(pullNumber) || + pullNumber <= 0 + ) { + return null; + } + return { repoFullName, pullNumber }; +} diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 4c101417c6..442180f583 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -1,3 +1,4 @@ +import { parsePullRequestTargetKey } from "@loopover/engine"; import { and, asc, desc, eq, gte, inArray, not, or, sql, type SQL } from "drizzle-orm"; import { getDb } from "./client"; import { @@ -3354,16 +3355,6 @@ function uniqueRepoNames(values: string[]): string[] { return result; } -function parsePullRequestTargetKey(targetKey: string | null | undefined): { repoFullName: string; pullNumber: number } | null { - if (!targetKey) return null; - const delimiter = targetKey.lastIndexOf("#"); - if (delimiter <= 0 || delimiter === targetKey.length - 1) return null; - const repoFullName = targetKey.slice(0, delimiter); - const pullNumber = Number(targetKey.slice(delimiter + 1)); - if (!repoFullName.includes("/") || !Number.isInteger(pullNumber) || pullNumber <= 0) return null; - return { repoFullName, pullNumber }; -} - function maxIso(left: string | null | undefined, right: string | null | undefined): string | null { if (!left) return right ?? null; if (!right) return left; diff --git a/test/unit/parse-pull-request-target-key.test.ts b/test/unit/parse-pull-request-target-key.test.ts new file mode 100644 index 0000000000..ea0788a111 --- /dev/null +++ b/test/unit/parse-pull-request-target-key.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; +// #4882: the pure PR-target-key parser now lives in loopover-engine, extracted out of the D1-heavy +// repositories access layer. This suite drives it through the engine's public barrel and exercises every +// branch so the moved code carries its own coverage. +import { parsePullRequestTargetKey } from "../../packages/loopover-engine/src/index"; + +describe("parsePullRequestTargetKey (#4882)", () => { + it("returns null for falsy input", () => { + expect(parsePullRequestTargetKey(null)).toBeNull(); + expect(parsePullRequestTargetKey(undefined)).toBeNull(); + expect(parsePullRequestTargetKey("")).toBeNull(); + }); + + it("returns null when there is no '#', or it is leading or trailing", () => { + expect(parsePullRequestTargetKey("owner/repo")).toBeNull(); + expect(parsePullRequestTargetKey("#5")).toBeNull(); + expect(parsePullRequestTargetKey("owner/repo#")).toBeNull(); + }); + + it("returns null when the repo half has no '/'", () => { + expect(parsePullRequestTargetKey("owner#5")).toBeNull(); + }); + + it("returns null when the pull number is not a positive integer", () => { + expect(parsePullRequestTargetKey("owner/repo#abc")).toBeNull(); + expect(parsePullRequestTargetKey("owner/repo#1.5")).toBeNull(); + expect(parsePullRequestTargetKey("owner/repo#0")).toBeNull(); + expect(parsePullRequestTargetKey("owner/repo#-3")).toBeNull(); + }); + + it("parses a well-formed target key", () => { + expect(parsePullRequestTargetKey("owner/repo#42")).toEqual({ + repoFullName: "owner/repo", + pullNumber: 42, + }); + }); + + it("splits on the last '#' so a repo name containing '#' still parses", () => { + expect(parsePullRequestTargetKey("owner/re#po#7")).toEqual({ + repoFullName: "owner/re#po", + pullNumber: 7, + }); + }); +});