diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 1890d68381..242296373b 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -320,15 +320,23 @@ export async function upsertPullRequestFromGitHub( const db = getDb(env.DB); const syncedAt = nowIso(); const lastSeenOpenAt = pr.state === "open" ? (options.seenOpenAt ?? syncedAt) : null; - const linkedIssuesJson = jsonString(record.linkedIssues); - const observedLinkedIssueClaimedAt = record.linkedIssues.length > 0 ? syncedAt : null; const existingClaimRows = await db .select({ linkedIssuesJson: pullRequests.linkedIssuesJson, linkedIssueClaimedAt: pullRequests.linkedIssueClaimedAt }) .from(pullRequests) .where(and(eq(pullRequests.repoFullName, repoFullName), eq(pullRequests.number, pr.number))) .limit(1); + // A sparse GitHub payload (e.g. a narrower webhook event's embedded pull_request sub-object, as opposed to a + // full `GET /pulls/{n}` read) can omit `body` entirely (`undefined`) rather than reporting it as explicitly + // empty (`null`/`""`) — `GitHubPullRequestPayload.body` is typed `string | null` precisely because a caller + // may not have it at all. Re-deriving linked issues from an ABSENT body would silently wipe an + // already-correctly-claimed linked issue (and reset its claim timestamp via resolveLinkedIssueClaimedAt's own + // `linkedIssues.length === 0` branch) on any such upsert. Fall back to whatever is already stored in that + // case; only a genuinely observed (possibly empty) body updates the claim. (#linked-issue-sparse-payload-preserve) + const linkedIssues = pr.body === undefined && existingClaimRows[0] ? parseLinkedIssuesJson(existingClaimRows[0].linkedIssuesJson) : record.linkedIssues; + const linkedIssuesJson = pr.body === undefined && existingClaimRows[0] ? existingClaimRows[0].linkedIssuesJson : jsonString(linkedIssues); + const observedLinkedIssueClaimedAt = linkedIssues.length > 0 ? syncedAt : null; const linkedIssueClaimedAt = resolveLinkedIssueClaimedAt( - record.linkedIssues, + linkedIssues, linkedIssuesJson, existingClaimRows[0], observedLinkedIssueClaimedAt, @@ -375,7 +383,7 @@ export async function upsertPullRequestFromGitHub( updatedAt: syncedAt, }, }); - return { ...record, linkedIssueClaimedAt }; + return { ...record, linkedIssues, linkedIssueClaimedAt }; } function resolveLinkedIssueClaimedAt( diff --git a/test/unit/db-parsers.test.ts b/test/unit/db-parsers.test.ts index 533cd9a9ee..6b27e4968d 100644 --- a/test/unit/db-parsers.test.ts +++ b/test/unit/db-parsers.test.ts @@ -9,6 +9,7 @@ import { getRepoAuthorPullRequestHistory, getLatestScoringModelSnapshot, getFreshOfficialMinerDetection, + getPullRequest, listPullRequests, listPullRequestDetailSyncStates, listRepoSyncSegments, @@ -363,6 +364,47 @@ describe("database row parser hardening", () => { expect(resynced?.lastRegatedAt).toBe(stamped); // but the marker survived }); + it("REGRESSION: a sparse sync (payload.body absent) does NOT wipe an already-claimed linked issue (#linked-issue-sparse-payload-preserve)", async () => { + const env = createTestEnv(); + // A full sync (e.g. pull_request.opened) correctly claims the linked issue. + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "Fix the bug", state: "open", user: { login: "bob" }, head: { sha: "a1" }, labels: [], body: "Closes #42" }); + const claimed = await getPullRequest(env, "owner/repo", 7); + expect(claimed?.linkedIssues).toEqual([42]); + expect(typeof claimed?.linkedIssueClaimedAt).toBe("string"); + + // A NARROWER event's embedded pull_request sub-object (e.g. a pull_request_review payload shape) can omit + // `body` entirely -- `undefined`, not an explicit empty string/null. This upsert must not re-derive an + // empty linkedIssues from the absent body and clobber the already-correct claim. + const resynced = await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "Fix the bug", state: "open", user: { login: "bob" }, head: { sha: "a1" }, labels: [] }); + expect(resynced.linkedIssues).toEqual([42]); // the function's own return value is corrected too + expect(resynced.linkedIssueClaimedAt).toBe(claimed?.linkedIssueClaimedAt); // claim timestamp is untouched + + const stored = await getPullRequest(env, "owner/repo", 7); + expect(stored?.linkedIssues).toEqual([42]); + expect(stored?.linkedIssueClaimedAt).toBe(claimed?.linkedIssueClaimedAt); + }); + + it("a genuinely empty body (explicit null/\"\", not absent) DOES clear a previously-claimed linked issue", async () => { + const env = createTestEnv(); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 8, title: "Fix the bug", state: "open", user: { login: "bob" }, head: { sha: "a1" }, labels: [], body: "Closes #42" }); + // The contributor genuinely deleted their PR description -- GitHub reports this as body: null, a real signal + // distinct from a sparse payload's absent field, and it must still update the stored claim. + const cleared = await upsertPullRequestFromGitHub(env, "owner/repo", { number: 8, title: "Fix the bug", state: "open", user: { login: "bob" }, head: { sha: "a1" }, labels: [], body: null }); + expect(cleared.linkedIssues).toEqual([]); + const stored = await getPullRequest(env, "owner/repo", 8); + expect(stored?.linkedIssues).toEqual([]); + expect(stored?.linkedIssueClaimedAt).toBeNull(); + }); + + it("a sparse sync on a brand-new PR (no existing row to preserve) falls through to the empty default", async () => { + const env = createTestEnv(); + // No prior row exists for PR #9 -- the sparse-preserve branch has nothing to preserve, so this must behave + // exactly as it always has: derive from the (absent) body, yielding no linked issues. + const created = await upsertPullRequestFromGitHub(env, "owner/repo", { number: 9, title: "New PR", state: "open", user: { login: "bob" }, labels: [] }); + expect(created.linkedIssues).toEqual([]); + expect(created.linkedIssueClaimedAt).toBeNull(); + }); + it("countRecentDeadLetters counts github_app.dlq_dead_lettered audits since a cutoff, independent of any ops flag (#1276)", async () => { const env = createTestEnv(); await recordAuditEvent(env, { eventType: "github_app.dlq_dead_lettered", actor: "gittensory", targetKey: "dlq:github-webhook:a", outcome: "error", createdAt: "2026-06-24T10:00:00.000Z" });