Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -375,7 +383,7 @@ export async function upsertPullRequestFromGitHub(
updatedAt: syncedAt,
},
});
return { ...record, linkedIssueClaimedAt };
return { ...record, linkedIssues, linkedIssueClaimedAt };
}

function resolveLinkedIssueClaimedAt(
Expand Down
42 changes: 42 additions & 0 deletions test/unit/db-parsers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
getRepoAuthorPullRequestHistory,
getLatestScoringModelSnapshot,
getFreshOfficialMinerDetection,
getPullRequest,
listPullRequests,
listPullRequestDetailSyncStates,
listRepoSyncSegments,
Expand Down Expand Up @@ -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" });
Expand Down
Loading