refactor(engine): extract the pull-request-target-key parser into loopover-engine

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 `"<owner>/<repo>#<number>"` 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";

packages/loopover-engine/src/parse-pull-request-target-key.ts

@@ -0,0 +1,23 @@
+/**
+ * Parse a pull-request target key of the form `"<owner>/<repo>#<number>"` 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 };
+}

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;

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,
+    });
+  });
+});