diff --git a/migrations/0094_pull_request_reviews_invalidated.sql b/migrations/0094_pull_request_reviews_invalidated.sql new file mode 100644 index 0000000000..12673eebfb --- /dev/null +++ b/migrations/0094_pull_request_reviews_invalidated.sql @@ -0,0 +1,16 @@ +-- PR reviews cache invalidation marker (#2537): bumped by a `pull_request_review` webhook +-- (submitted/dismissed/edited) to signal the cached `pull_request_reviews` rows are stale. NULL (the +-- default) means no invalidating event has been recorded, so a subsequent fetchAndStorePullRequestDetails +-- pass can skip the `GET /pulls/{n}/reviews` call when reviews_synced_at already covers it. +ALTER TABLE pull_request_detail_sync_state ADD COLUMN reviews_invalidated_at TEXT; + +-- One-time cleanup (gate review finding): reviews_synced_at has existed since migration 0006, long before +-- today it gains any cache-skip meaning -- every sync pass since then has stamped it UNCONDITIONALLY, +-- including passes whose review fetch itself failed (there was no per-segment success timestamp before this +-- PR). A `status != 'complete'` row is exactly the set where SOME segment failed on its last sync (`status` +-- only reads 'complete' when that pass recorded zero warnings across files/reviews/checks, which is a +-- reliable historical guarantee reviews specifically succeeded); for every other status the failure could +-- have been reviews, so trusting a stale reviews_synced_at there the moment this cache-skip logic goes live +-- would silently skip re-fetching reviews that were never actually captured. Reset ONLY the ambiguous rows; +-- a genuinely 'complete' row's reviews_synced_at is trustworthy as-is and is left untouched. +UPDATE pull_request_detail_sync_state SET reviews_synced_at = NULL WHERE status != 'complete' AND reviews_synced_at IS NOT NULL; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index c54c3fe6b8..6126a0b420 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -1152,6 +1152,7 @@ export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequ headSha: state.headSha, filesSyncedAt: state.filesSyncedAt, reviewsSyncedAt: state.reviewsSyncedAt, + reviewsInvalidatedAt: state.reviewsInvalidatedAt, checksSyncedAt: state.checksSyncedAt, lastSyncedAt: state.lastSyncedAt, errorSummary: state.errorSummary, @@ -1164,6 +1165,7 @@ export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequ headSha: state.headSha, filesSyncedAt: state.filesSyncedAt, reviewsSyncedAt: state.reviewsSyncedAt, + reviewsInvalidatedAt: state.reviewsInvalidatedAt, checksSyncedAt: state.checksSyncedAt, lastSyncedAt: state.lastSyncedAt, errorSummary: state.errorSummary, @@ -1172,6 +1174,50 @@ export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequ }); } +/** Reviews-cache invalidation stamp (#2537): a pure single-field bump of `reviewsInvalidatedAt`, leaving + * every other column (`headSha`/`filesSyncedAt`/`reviewsSyncedAt`/`checksSyncedAt`/...) untouched when the + * row already exists — mirrors the narrow single-field touches on `pull_requests` (markPullRequestApproved, + * markPullRequestRegated). Creates the row (all other columns default/NULL) if this repo+PR has never been + * synced yet, so an early review webhook is not silently dropped. + * + * Unlike its siblings above (advisory/reporting markers with other fallback signals), this write is the SOLE + * source of the reviews-cache invalidation signal (#2537 gate finding) — a single failed attempt loses that + * PR's specific "reviews changed" event permanently, with nothing to naturally re-trigger it until some LATER + * invalidation happens to succeed. The caller already treats this as best-effort (never blocks the webhook), + * so a short bounded retry absorbs a transient D1 blip in-process rather than needing a durable retry queue + * for what is still, even after this, a best-effort write. */ +export async function markPullRequestReviewsInvalidated(env: Env, repoFullName: string, pullNumber: number): Promise { + const db = getDb(env.DB); + const now = nowIso(); + const MAX_ATTEMPTS = 3; + let lastError: unknown; + for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt += 1) { + try { + await db + .insert(pullRequestDetailSyncState) + .values({ + id: `${repoFullName}#${pullNumber}`, + repoFullName, + pullNumber, + status: "never_synced", + reviewsInvalidatedAt: now, + updatedAt: now, + }) + .onConflictDoUpdate({ + target: [pullRequestDetailSyncState.repoFullName, pullRequestDetailSyncState.pullNumber], + set: { + reviewsInvalidatedAt: now, + updatedAt: now, + }, + }); + return; + } catch (error) { + lastError = error; + } + } + throw lastError; +} + export async function getPullRequestDetailSyncState(env: Env, fullName: string, pullNumber: number): Promise { const db = getDb(env.DB); const [row] = await db @@ -4226,6 +4272,7 @@ function toPullRequestDetailSyncStateRecord(row: typeof pullRequestDetailSyncSta headSha: row.headSha, filesSyncedAt: row.filesSyncedAt, reviewsSyncedAt: row.reviewsSyncedAt, + reviewsInvalidatedAt: row.reviewsInvalidatedAt, checksSyncedAt: row.checksSyncedAt, lastSyncedAt: row.lastSyncedAt, errorSummary: row.errorSummary, diff --git a/src/db/schema.ts b/src/db/schema.ts index 4dca4013e4..31f9f9d372 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -231,6 +231,12 @@ export const pullRequestDetailSyncState = sqliteTable( headSha: text("head_sha"), filesSyncedAt: text("files_synced_at"), reviewsSyncedAt: text("reviews_synced_at"), + // Bumped by a `pull_request_review` webhook (submitted/dismissed/edited) to signal the cached reviews are + // stale. NULL, or a value <= reviewsSyncedAt, means the last sync already covers every invalidating event, + // so fetchAndStorePullRequestDetails can skip the `GET /pulls/{n}/reviews` call. Reviews are independent of + // headSha (a new commit alone does not invalidate existing review state; only an actual review-webhook event + // does) -- unlike the files cache, which is why this uses its own timestamp column instead of headSha matching. + reviewsInvalidatedAt: text("reviews_invalidated_at"), checksSyncedAt: text("checks_synced_at"), lastSyncedAt: text("last_synced_at"), errorSummary: text("error_summary"), diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 4158a2e0e7..07cc777742 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -613,7 +613,7 @@ export async function backfillOpenPullRequestDetails( await mapWithConcurrency(batch, 2, async (pr) => { await upsertPullRequestDetailSyncState(env, { repoFullName: repo.fullName, pullNumber: pr.number, status: "running" }); const before = warnings.length; - await fetchAndStorePullRequestDetails(env, repo.fullName, pr, token, warnings, admissionKey, "backfill_open_pr_details"); + const { reviewsSyncedAt } = await fetchAndStorePullRequestDetails(env, repo.fullName, pr, token, warnings, admissionKey, "backfill_open_pr_details"); const syncedAt = nowIso(); const newWarnings = warnings.slice(before); await upsertPullRequestDetailSyncState(env, { @@ -622,7 +622,7 @@ export async function backfillOpenPullRequestDetails( status: newWarnings.length > 0 ? "partial" : "complete", headSha: pr.headSha, filesSyncedAt: syncedAt, - reviewsSyncedAt: syncedAt, + reviewsSyncedAt, checksSyncedAt: syncedAt, lastSyncedAt: syncedAt, errorSummary: newWarnings.at(-1), @@ -689,7 +689,7 @@ export async function refreshPullRequestDetails( const admissionKey = repoAdmissionKeyForToken(env, repo, token); const warnings: string[] = []; await upsertPullRequestDetailSyncState(env, { repoFullName, pullNumber, status: "running" }); - await fetchAndStorePullRequestDetails(env, repoFullName, pr, token, warnings, admissionKey, "live_review", { forceFiles: options.force }); + const { reviewsSyncedAt } = await fetchAndStorePullRequestDetails(env, repoFullName, pr, token, warnings, admissionKey, "live_review", { forceFiles: options.force }); const syncedAt = nowIso(); const status: PullRequestDetailSyncStateRecord["status"] = warnings.length > 0 ? "partial" : "complete"; await upsertPullRequestDetailSyncState(env, { @@ -698,7 +698,7 @@ export async function refreshPullRequestDetails( status, headSha: pr.headSha, filesSyncedAt: syncedAt, - reviewsSyncedAt: syncedAt, + reviewsSyncedAt, checksSyncedAt: syncedAt, lastSyncedAt: syncedAt, errorSummary: warnings.at(-1), @@ -1807,7 +1807,7 @@ async function backfillRepository(env: Env, repo: RepositoryRecord, limits: Back const detailWarningStart = warnings.length; await mapWithConcurrency(detailTargets, limits.detailConcurrency, async (pr) => { const before = warnings.length; - await fetchAndStorePullRequestDetails(env, repo.fullName, pr, token, warnings, admissionKey, "backfill_open_pr_details"); + const { reviewsSyncedAt } = await fetchAndStorePullRequestDetails(env, repo.fullName, pr, token, warnings, admissionKey, "backfill_open_pr_details"); // Persist the repo+PR+headSha snapshot marker (#audit-rate-headroom) so a later call through ANY // cache-aware path (open-PR convergence, live review) can skip refetching this PR's files while its // head is unchanged — without this write, fetchAndStorePullRequestDetails's cache check always misses @@ -1820,7 +1820,7 @@ async function backfillRepository(env: Env, repo: RepositoryRecord, limits: Back status: newWarnings.length > 0 ? "partial" : "complete", headSha: pr.headSha, filesSyncedAt: syncedAt, - reviewsSyncedAt: syncedAt, + reviewsSyncedAt, checksSyncedAt: syncedAt, lastSyncedAt: syncedAt, errorSummary: newWarnings.at(-1), @@ -1978,20 +1978,54 @@ async function fetchAndStorePullRequestDetails( admissionKey: GitHubRateLimitAdmissionKey | undefined, caller: PullRequestFilesFetchCaller, options: { forceFiles?: boolean | undefined } = {}, -): Promise { +): Promise<{ reviewsSyncedAt: string | null | undefined }> { // Durable repo+PR+headSha file snapshot (#audit-rate-headroom): a bare URL cache is insufficient because // `/pulls/{n}/files` has the SAME url across different heads. Reuse the stored `pull_request_files` rows // instead of refetching when the last successful files sync already covered the PR's CURRENT head SHA — - // only files are cached here; reviews/checks are more volatile at a fixed head and still refresh every call. - const existingState = !options.forceFiles && pr.headSha ? await getPullRequestDetailSyncState(env, repoFullName, pr.number) : null; - const filesUpToDate = Boolean(existingState?.headSha) && existingState?.headSha === pr.headSha && Boolean(existingState?.filesSyncedAt); + // only files are cached here; checks are more volatile at a fixed head and still refresh every call. + // + // The row is now fetched UNCONDITIONALLY (the original files-only cache gated this on `!options.forceFiles`, + // skipping the read entirely on a forced refresh) because reviews caching (#2537) reuses this SAME row and + // does not depend on `headSha` or `forceFiles` at all -- `forceFiles` only ever forces a FILES re-fetch (see + // its name and its only caller, refreshPullRequestDetails's manual "force" option), so a caller asking to + // force-refresh files must not ALSO force an unrelated reviews refetch (gate review finding: the previous + // version skipped the row entirely on `forceFiles && headSha`, which zeroed out `reviewsUpToDate` too). + // `forceFiles` is applied ONLY to `filesUpToDate` below, never to `reviewsUpToDate`. + const existingState = await getPullRequestDetailSyncState(env, repoFullName, pr.number); + const filesUpToDate = !options.forceFiles && Boolean(existingState?.headSha) && existingState?.headSha === pr.headSha && Boolean(existingState?.filesSyncedAt); + // Reviews cache (#2537): independent of headSha — a new commit alone does not invalidate existing review + // state, only an actual `pull_request_review` webhook (submitted/dismissed/edited) does, via + // markPullRequestReviewsInvalidated. Up to date when a prior sync recorded reviewsSyncedAt and either no + // invalidation has been recorded since, or the invalidation predates that sync. STRICTLY greater-than (not + // >=): millisecond-resolution ISO timestamps can tie when a sync and a racing invalidation land in the same + // millisecond, and sub-millisecond ordering is unknowable from the stored strings — a tie must fail toward + // "still needs a refetch," never toward silently trusting a possibly-stale cache. + const reviewsSyncedAtBefore = existingState?.reviewsSyncedAt; + const reviewsUpToDate = + Boolean(reviewsSyncedAtBefore) && + (!existingState?.reviewsInvalidatedAt || (reviewsSyncedAtBefore ?? "") > existingState.reviewsInvalidatedAt); + // Gate review finding (TOCTOU race): `existingState` above is a snapshot read at the TOP of this call. If a + // `pull_request_review` webhook races in AFTER that read but BEFORE this function returns, an unconditional + // "stamp reviewsSyncedAt to now" on the CALLER's side (the old design) would advance the timestamp PAST that + // webhook's invalidation without ever having fetched the reviews it invalidated -- the cache would then + // permanently believe it's fresh through an event it never actually observed. Captured HERE, before the + // fetch even starts, so it's safe: any invalidation racing in from this instant onward still leaves + // `reviewsInvalidatedAt` newer than whatever we return below, forcing a correct refetch on the NEXT pass. + const reviewFetchStartedAt = nowIso(); const warningStart = warnings.length; const [files, reviews, checks] = await Promise.all([ filesUpToDate ? Promise.resolve([]) : fetchPullRequestFiles(env, repoFullName, pr.number, token, warnings, admissionKey, caller), - fetchPullRequestReviews(env, repoFullName, pr.number, token, warnings, admissionKey), + reviewsUpToDate ? Promise.resolve([]) : fetchPullRequestReviews(env, repoFullName, pr.number, token, warnings, admissionKey), fetchPullRequestChecks(env, repoFullName, pr, token, warnings, admissionKey), ]); const fileSyncFailed = warnings.slice(warningStart).some((warning) => warning.startsWith(`File sync failed for #${pr.number}:`)); + // reviewsSyncedAt only ever ADVANCES on a genuine success in THIS call -- never on a cache-hit skip, and + // never on a failed fetch attempt. This is what makes a stored reviewsSyncedAt a trustworthy "last confirmed + // successful sync" marker on its own (no separate errorSummary string-matching needed: a failed or skipped + // pass simply preserves whatever was already known, which -- being unchanged -- correctly keeps comparing as + // stale against reviewsInvalidatedAt on the next pass until a real fetch actually succeeds). + const reviewSyncFailedThisCall = !reviewsUpToDate && warnings.slice(warningStart).some((warning) => warning.startsWith(`Review sync failed for #${pr.number}:`)); + const reviewsSyncedAtResult = !reviewsUpToDate && !reviewSyncFailedThisCall ? reviewFetchStartedAt : reviewsSyncedAtBefore; if (!filesUpToDate && !fileSyncFailed) { await deletePullRequestFiles(env, repoFullName, pr.number); @@ -2036,6 +2070,7 @@ async function fetchAndStorePullRequestDetails( payload: check as unknown as Record, }); } + return { reviewsSyncedAt: reviewsSyncedAtResult }; } // GitHub caps list endpoints at 100 items/page, so a single `per_page=100` fetch silently truncates a diff --git a/src/queue/processors.ts b/src/queue/processors.ts index ffbb345d69..d051075233 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -43,6 +43,7 @@ import { getCachedAiReview, putCachedAiReview, markPullRequestsRegated, + markPullRequestReviewsInvalidated, markPullRequestSurfacePublished, getLatestRegatedAt, claimRegateFanoutSlot, @@ -4220,6 +4221,29 @@ async function processGitHubWebhook( }), ); }); + // Reviews-cache invalidation (#2537): a `pull_request_review` webhook (submitted/dismissed/edited) is + // the ONLY event that can change the set of reviews GitHub reports for this PR, so it is the sole signal + // fetchAndStorePullRequestDetails's reviewsUpToDate check needs to know the cached reviews are stale. + // Independent of, and does not gate, any downstream processing below — best-effort like the outcome/ + // reversal recording above, so a transient D1 failure here never blocks the webhook. + if ( + eventName === "pull_request_review" && + (payload.action === "submitted" || payload.action === "dismissed" || payload.action === "edited") + ) { + await markPullRequestReviewsInvalidated(env, repoFullName, payloadPullRequest.number).catch((error) => { + /* v8 ignore next -- best-effort: cache-invalidation stamping never blocks the webhook. */ + console.warn( + JSON.stringify({ + level: "warn", + event: "pull_request_reviews_invalidate_failed", + deliveryId, + repository: repoFullName, + pullNumber: payloadPullRequest.number, + error: errorMessage(error), + }), + ); + }); + } const pr = await upsertPullRequestFromGitHub( env, repoFullName, diff --git a/src/types.ts b/src/types.ts index b8b6e57b70..aa0fb8870c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -870,6 +870,7 @@ export type PullRequestDetailSyncStateRecord = { headSha?: string | null | undefined; filesSyncedAt?: string | null | undefined; reviewsSyncedAt?: string | null | undefined; + reviewsInvalidatedAt?: string | null | undefined; checksSyncedAt?: string | null | undefined; lastSyncedAt?: string | null | undefined; errorSummary?: string | null | undefined; diff --git a/test/unit/backfill-reviews-cache-scoping.test.ts b/test/unit/backfill-reviews-cache-scoping.test.ts new file mode 100644 index 0000000000..067ec195e1 --- /dev/null +++ b/test/unit/backfill-reviews-cache-scoping.test.ts @@ -0,0 +1,569 @@ +import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { DatabaseSync } from "node:sqlite"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + getPullRequestDetailSyncState, + listPullRequestReviews, + markPullRequestReviewsInvalidated, + upsertPullRequestDetailSyncState, + upsertPullRequestFromGitHub, + upsertPullRequestReview, +} from "../../src/db/repositories"; +import { refreshPullRequestDetails } from "../../src/github/backfill"; +import { clearGitHubResponseCacheForTest } from "../../src/github/client"; +import { resetMetrics } from "../../src/selfhost/metrics"; +import { normalizeRegistryPayload } from "../../src/registry/normalize"; +import { persistRegistrySnapshot } from "../../src/registry/sync"; +import { createD1Adapter, nodeSqliteDriver } from "../../src/selfhost/d1-adapter"; +import { runSelfHostMigrations } from "../../src/selfhost/migrate"; +import { createTestEnv } from "../helpers/d1"; + +describe("GitHub PR reviews cache scoping (#2537)", () => { + afterEach(() => { + clearGitHubResponseCacheForTest(); + resetMetrics(); + vi.unstubAllGlobals(); + }); + + async function seedRegisteredRepo(env: Env) { + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0, trusted_label_pipeline: true, label_multipliers: {} } }, + { kind: "raw-github", url: "https://example.test/master_repositories.json" }, + "2026-05-23T00:00:00.000Z", + ), + ); + } + + function stubFetchTracking(handler: (url: string, init?: RequestInit) => Response | Promise): string[] { + const urls: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + urls.push(url); + return handler(url, init); + }); + return urls; + } + + it("fetches and stores reviews on first sync when no sync-state row exists (cache miss)", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 60, + title: "Open PR, never synced", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-60" }, + labels: [], + body: "", + }); + const urls = stubFetchTracking((url) => + url.includes("/pulls/60/reviews") + ? Response.json([{ id: 1, user: { login: "maintainer" }, state: "APPROVED", author_association: "OWNER", submitted_at: "2026-05-20T00:00:00.000Z" }]) + : Response.json([]), + ); + + const result = await refreshPullRequestDetails(env, "JSONbored/gittensory", 60); + + expect(result).toMatchObject({ status: "complete" }); + expect(urls.some((url) => url.includes("/pulls/60/reviews"))).toBe(true); + expect(await listPullRequestReviews(env, "JSONbored/gittensory", 60)).toEqual([expect.objectContaining({ reviewerLogin: "maintainer", state: "APPROVED" })]); + expect(await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 60)).toMatchObject({ status: "complete" }); + }); + + it("does not re-fetch reviews when reviewsSyncedAt is set and no invalidation has been recorded (cache hit), and leaves stored rows untouched", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 61, + title: "Open PR, reviews already synced", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-61" }, + labels: [], + body: "", + }); + await upsertPullRequestReview(env, { + id: "JSONbored/gittensory#61#1", + repoFullName: "JSONbored/gittensory", + pullNumber: 61, + reviewerLogin: "maintainer", + state: "APPROVED", + authorAssociation: "OWNER", + submittedAt: "2026-05-19T00:00:00.000Z", + payload: { id: 1 }, + }); + await upsertPullRequestDetailSyncState(env, { + repoFullName: "JSONbored/gittensory", + pullNumber: 61, + status: "complete", + headSha: "head-61", + filesSyncedAt: "2026-05-20T00:00:00.000Z", + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + }); + const urls = stubFetchTracking((url) => (url.includes("/reviews") ? new Response("must not be called", { status: 500 }) : Response.json([]))); + + const result = await refreshPullRequestDetails(env, "JSONbored/gittensory", 61); + + expect(result).toMatchObject({ status: "complete", warnings: [] }); + expect(urls.some((url) => url.includes("/pulls/61/reviews"))).toBe(false); + expect(await listPullRequestReviews(env, "JSONbored/gittensory", 61)).toEqual([expect.objectContaining({ reviewerLogin: "maintainer", state: "APPROVED" })]); + }); + + it("does not re-fetch reviews when reviewsInvalidatedAt predates reviewsSyncedAt (stale invalidation, still a cache hit)", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 62, + title: "Open PR, invalidation predates sync", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-62" }, + labels: [], + body: "", + }); + await upsertPullRequestDetailSyncState(env, { + repoFullName: "JSONbored/gittensory", + pullNumber: 62, + status: "complete", + headSha: "head-62", + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + reviewsInvalidatedAt: "2026-05-19T00:00:00.000Z", + }); + const urls = stubFetchTracking((url) => (url.includes("/reviews") ? new Response("must not be called", { status: 500 }) : Response.json([]))); + + const result = await refreshPullRequestDetails(env, "JSONbored/gittensory", 62); + + expect(result).toMatchObject({ status: "complete" }); + expect(urls.some((url) => url.includes("/pulls/62/reviews"))).toBe(false); + }); + + it("re-fetches reviews on the next sync after markPullRequestReviewsInvalidated bumps reviewsInvalidatedAt past reviewsSyncedAt (cache invalidation)", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 63, + title: "Open PR, invalidated after sync", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-63" }, + labels: [], + body: "", + }); + await upsertPullRequestDetailSyncState(env, { + repoFullName: "JSONbored/gittensory", + pullNumber: 63, + status: "complete", + headSha: "head-63", + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + }); + + await markPullRequestReviewsInvalidated(env, "JSONbored/gittensory", 63); + expect(await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 63)).toMatchObject({ + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + status: "complete", + headSha: "head-63", + }); + + const urls = stubFetchTracking((url) => + url.includes("/pulls/63/reviews") + ? Response.json([{ id: 2, user: { login: "second-reviewer" }, state: "CHANGES_REQUESTED", author_association: "NONE", submitted_at: "2026-05-21T00:00:00.000Z" }]) + : Response.json([]), + ); + + const result = await refreshPullRequestDetails(env, "JSONbored/gittensory", 63); + + expect(result).toMatchObject({ status: "complete" }); + expect(urls.some((url) => url.includes("/pulls/63/reviews"))).toBe(true); + expect(await listPullRequestReviews(env, "JSONbored/gittensory", 63)).toEqual([expect.objectContaining({ reviewerLogin: "second-reviewer", state: "CHANGES_REQUESTED" })]); + }); + + it("REGRESSION (gate finding): a FAILED review fetch never advances reviewsSyncedAt, so the next sync retries instead of trusting a false cache hit", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 65, + title: "Open PR, review fetch fails on first sync", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-65" }, + labels: [], + body: "", + }); + // First pass: reviews REST + GraphQL fallback both fail (mirrors backfill.test.ts's "review failure, 503" + // stub — any unstubbed URL, including the GraphQL fallback, falls through to a 404). + const firstPassUrls = stubFetchTracking((url) => (url.includes("/pulls/65/reviews") ? new Response("review failure", { status: 503 }) : Response.json([]))); + + const firstResult = await refreshPullRequestDetails(env, "JSONbored/gittensory", 65); + + expect(firstResult.status).toBe("partial"); + expect(firstPassUrls.some((url) => url.includes("/pulls/65/reviews"))).toBe(true); + // The FAILED attempt must NOT advance reviewsSyncedAt — a stored value here (as the pre-fix code produced, + // stamping it unconditionally regardless of success) would let the next pass wrongly treat the failed + // fetch as a valid cache hit and never retry. + expect((await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 65))?.reviewsSyncedAt).toBeFalsy(); + + // Second pass: reviews now succeed — since reviewsSyncedAt is still unset, this MUST be treated as a cache + // miss and genuinely refetched (not skipped). + const secondPassUrls = stubFetchTracking((url) => + url.includes("/pulls/65/reviews") + ? Response.json([{ id: 3, user: { login: "late-reviewer" }, state: "APPROVED", author_association: "NONE", submitted_at: "2026-05-22T00:00:00.000Z" }]) + : Response.json([]), + ); + + const secondResult = await refreshPullRequestDetails(env, "JSONbored/gittensory", 65); + + expect(secondResult.status).toBe("complete"); + expect(secondPassUrls.some((url) => url.includes("/pulls/65/reviews"))).toBe(true); + expect(await listPullRequestReviews(env, "JSONbored/gittensory", 65)).toEqual([expect.objectContaining({ reviewerLogin: "late-reviewer" })]); + // The now-successful sync DOES advance reviewsSyncedAt. + expect((await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 65))?.reviewsSyncedAt).toBeTruthy(); + }); + + it("REGRESSION (gate finding, TOCTOU race): a pull_request_review webhook racing in DURING a sync still forces a retry on the next pass", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 68, + title: "Open PR, invalidation races in mid-sync", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-68" }, + labels: [], + body: "", + }); + // No existing sync state — first-ever sync, so a real reviews fetch happens. The /reviews handler itself + // calls markPullRequestReviewsInvalidated mid-flight, simulating a `pull_request_review` webhook landing + // AFTER fetchAndStorePullRequestDetails already read `existingState` but BEFORE it (and the caller's final + // write) complete — exactly the race the gate flagged: a naive "stamp reviewsSyncedAt to now, once the + // whole call finishes" would land AFTER this invalidation and wrongly look like it already covers it. + let racingInvalidationDone = false; + const urls = stubFetchTracking(async (url) => { + if (url.includes("/pulls/68/reviews")) { + await markPullRequestReviewsInvalidated(env, "JSONbored/gittensory", 68); + racingInvalidationDone = true; + return Response.json([{ id: 1, user: { login: "reviewer" }, state: "APPROVED", author_association: "NONE", submitted_at: "2026-05-22T00:00:00.000Z" }]); + } + return Response.json([]); + }); + + await refreshPullRequestDetails(env, "JSONbored/gittensory", 68); + + expect(racingInvalidationDone).toBe(true); + expect(urls.some((url) => url.includes("/pulls/68/reviews"))).toBe(true); + const stateAfterRace = await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 68); + expect(stateAfterRace?.reviewsSyncedAt).toBeTruthy(); + expect(stateAfterRace?.reviewsInvalidatedAt).toBeTruthy(); + // The stored reviewsSyncedAt was captured BEFORE the fetch started (and therefore no later than the + // race). Millisecond-resolution timestamps can tie in a fast test run, so allow equality here — the + // production `reviewsUpToDate` check uses a STRICT `>` specifically so a tie still forces a refetch. + expect(stateAfterRace!.reviewsSyncedAt! <= stateAfterRace!.reviewsInvalidatedAt!).toBe(true); + + // A follow-up pass must therefore still see this as stale and genuinely refetch — not trust the sync that + // raced against (and missed) the invalidating event. + const followUpUrls = stubFetchTracking((url) => + url.includes("/pulls/68/reviews") + ? Response.json([ + { id: 1, user: { login: "reviewer" }, state: "APPROVED", author_association: "NONE", submitted_at: "2026-05-22T00:00:00.000Z" }, + { id: 2, user: { login: "second-reviewer" }, state: "CHANGES_REQUESTED", author_association: "NONE", submitted_at: "2026-05-23T00:00:00.000Z" }, + ]) + : Response.json([]), + ); + + await refreshPullRequestDetails(env, "JSONbored/gittensory", 68); + + expect(followUpUrls.some((url) => url.includes("/pulls/68/reviews"))).toBe(true); + expect(await listPullRequestReviews(env, "JSONbored/gittensory", 68)).toEqual( + expect.arrayContaining([expect.objectContaining({ reviewerLogin: "second-reviewer", state: "CHANGES_REQUESTED" })]), + ); + }); + + it("does not treat a FILES-only failure as a reason to re-fetch reviews (only a review-specific failure forces a retry)", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 66, + title: "Open PR, prior FILES failure only", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-66" }, + labels: [], + body: "", + }); + await upsertPullRequestReview(env, { + id: "JSONbored/gittensory#66#1", + repoFullName: "JSONbored/gittensory", + pullNumber: 66, + reviewerLogin: "maintainer", + state: "APPROVED", + authorAssociation: "OWNER", + submittedAt: "2026-05-19T00:00:00.000Z", + payload: { id: 1 }, + }); + await upsertPullRequestDetailSyncState(env, { + repoFullName: "JSONbored/gittensory", + pullNumber: 66, + status: "partial", + // headSha intentionally omitted/mismatched so files remain "not up to date" too — the point of this + // test is only that the FILES failure text in errorSummary must not be mistaken for a reviews failure. + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + errorSummary: "File sync failed for #66: GitHub REST and GraphQL detail fetches failed.", + }); + const urls = stubFetchTracking((url) => (url.includes("/pulls/66/reviews") ? new Response("must not be called", { status: 500 }) : Response.json([]))); + + const result = await refreshPullRequestDetails(env, "JSONbored/gittensory", 66); + + expect(result).toMatchObject({ status: "complete" }); + expect(urls.some((url) => url.includes("/pulls/66/reviews"))).toBe(false); + expect(await listPullRequestReviews(env, "JSONbored/gittensory", 66)).toEqual([expect.objectContaining({ reviewerLogin: "maintainer" })]); + }); + + it("REGRESSION: a head SHA change alone does not invalidate cached reviews (reviews are independent of the head, unlike files)", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 64, + title: "Open PR, new commit pushed", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-new" }, + labels: [], + body: "", + }); + await upsertPullRequestReview(env, { + id: "JSONbored/gittensory#64#1", + repoFullName: "JSONbored/gittensory", + pullNumber: 64, + reviewerLogin: "maintainer", + state: "APPROVED", + authorAssociation: "OWNER", + submittedAt: "2026-05-19T00:00:00.000Z", + payload: { id: 1 }, + }); + // Sync state was stamped for a DIFFERENT (older) head SHA — files caching would treat this as stale, but + // reviews caching must not, since reviews.reviewsSyncedAt has no head-SHA gate at all. + await upsertPullRequestDetailSyncState(env, { + repoFullName: "JSONbored/gittensory", + pullNumber: 64, + status: "complete", + headSha: "head-old", + filesSyncedAt: "2026-05-20T00:00:00.000Z", + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + }); + const urls = stubFetchTracking((url) => + url.includes("/pulls/64/files") + ? Response.json([{ filename: "src/new.ts", status: "added", additions: 3, deletions: 0, changes: 3 }]) + : url.includes("/reviews") + ? new Response("must not be called", { status: 500 }) + : Response.json([]), + ); + + const result = await refreshPullRequestDetails(env, "JSONbored/gittensory", 64); + + expect(result).toMatchObject({ status: "complete" }); + // Files WERE refetched (head changed)... + expect(urls.some((url) => url.includes("/pulls/64/files"))).toBe(true); + // ...but reviews were NOT — the core distinction from the files cache. + expect(urls.some((url) => url.includes("/pulls/64/reviews"))).toBe(false); + expect(await listPullRequestReviews(env, "JSONbored/gittensory", 64)).toEqual([expect.objectContaining({ reviewerLogin: "maintainer", state: "APPROVED" })]); + }); + + it("REGRESSION (gate finding): a manual force-files refresh does not also force an unrelated reviews refetch", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + await seedRegisteredRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 67, + title: "Open PR, manual force-files refresh", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "head-67" }, + labels: [], + body: "", + }); + await upsertPullRequestReview(env, { + id: "JSONbored/gittensory#67#1", + repoFullName: "JSONbored/gittensory", + pullNumber: 67, + reviewerLogin: "maintainer", + state: "APPROVED", + authorAssociation: "OWNER", + submittedAt: "2026-05-19T00:00:00.000Z", + payload: { id: 1 }, + }); + // Same head SHA + a fresh reviewsSyncedAt — reviews ARE cache-current; `force: true` must only re-fetch + // files (its own documented purpose), never reviews (an earlier version of this cache accidentally + // skipped the whole sync-state row lookup whenever `forceFiles && headSha`, which zeroed out + // `reviewsUpToDate` too and forced an unrelated reviews refetch on every manual files-only force). + await upsertPullRequestDetailSyncState(env, { + repoFullName: "JSONbored/gittensory", + pullNumber: 67, + status: "complete", + headSha: "head-67", + filesSyncedAt: "2026-05-20T00:00:00.000Z", + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + }); + const urls = stubFetchTracking((url) => + url.includes("/pulls/67/files") + ? Response.json([{ filename: "src/refreshed.ts", status: "modified", additions: 1, deletions: 1, changes: 2 }]) + : url.includes("/reviews") + ? new Response("must not be called", { status: 500 }) + : Response.json([]), + ); + + const result = await refreshPullRequestDetails(env, "JSONbored/gittensory", 67, { force: true }); + + expect(result).toMatchObject({ status: "complete" }); + // Files WERE refetched (force: true)... + expect(urls.some((url) => url.includes("/pulls/67/files"))).toBe(true); + // ...but reviews were NOT — forceFiles must never bleed into the (unrelated) reviews cache decision. + expect(urls.some((url) => url.includes("/pulls/67/reviews"))).toBe(false); + expect(await listPullRequestReviews(env, "JSONbored/gittensory", 67)).toEqual([expect.objectContaining({ reviewerLogin: "maintainer", state: "APPROVED" })]); + }); + + describe("markPullRequestReviewsInvalidated", () => { + it("creates a sync-state row if none exists yet", async () => { + const env = createTestEnv(); + expect(await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 70)).toBeNull(); + + await markPullRequestReviewsInvalidated(env, "JSONbored/gittensory", 70); + + const state = await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 70); + expect(state).not.toBeNull(); + expect(state?.reviewsInvalidatedAt).toBeTruthy(); + }); + + it("updates ONLY reviewsInvalidatedAt when a row already exists, leaving filesSyncedAt/reviewsSyncedAt/checksSyncedAt/headSha unchanged", async () => { + const env = createTestEnv(); + await upsertPullRequestDetailSyncState(env, { + repoFullName: "JSONbored/gittensory", + pullNumber: 71, + status: "complete", + headSha: "sha-preserved", + filesSyncedAt: "2026-05-20T00:00:00.000Z", + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + checksSyncedAt: "2026-05-20T00:00:00.000Z", + errorSummary: "prior warning", + }); + + await markPullRequestReviewsInvalidated(env, "JSONbored/gittensory", 71); + + const state = await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 71); + expect(state).toMatchObject({ + status: "complete", + headSha: "sha-preserved", + filesSyncedAt: "2026-05-20T00:00:00.000Z", + reviewsSyncedAt: "2026-05-20T00:00:00.000Z", + checksSyncedAt: "2026-05-20T00:00:00.000Z", + errorSummary: "prior warning", + }); + expect(state?.reviewsInvalidatedAt).toBeTruthy(); + expect(state?.reviewsInvalidatedAt).not.toBe("2026-05-20T00:00:00.000Z"); + }); + + it("REGRESSION (gate finding): retries a transient D1 write failure instead of losing the sole invalidation signal", async () => { + const env = createTestEnv(); + const realPrepare = env.DB.prepare.bind(env.DB); + let calls = 0; + // Fail the first 2 attempts (a transient blip), succeed on the 3rd (within MAX_ATTEMPTS). + vi.spyOn(env.DB, "prepare").mockImplementation((sql: string) => { + calls += 1; + if (calls <= 2) { + return { + bind: () => ({ + run: () => Promise.reject(new Error("d1 transient error")), + all: () => Promise.reject(new Error("d1 transient error")), + first: () => Promise.reject(new Error("d1 transient error")), + }), + } as unknown as ReturnType; + } + return realPrepare(sql); + }); + + await markPullRequestReviewsInvalidated(env, "JSONbored/gittensory", 72); + + expect(calls).toBeGreaterThan(2); + const state = await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 72); + expect(state?.reviewsInvalidatedAt).toBeTruthy(); + }); + + it("REGRESSION (gate finding): still throws (bounded, not infinite) once every retry attempt fails", async () => { + const env = createTestEnv(); + vi.spyOn(env.DB, "prepare").mockImplementation( + () => + ({ + bind: () => ({ + run: () => Promise.reject(new Error("d1 permanently down")), + all: () => Promise.reject(new Error("d1 permanently down")), + first: () => Promise.reject(new Error("d1 permanently down")), + }), + }) as unknown as ReturnType, + ); + + await expect(markPullRequestReviewsInvalidated(env, "JSONbored/gittensory", 73)).rejects.toThrow(); + }); + }); + + describe("migration 0094 legacy-row cleanup (gate review finding)", () => { + it("clears reviews_synced_at ONLY for rows whose last sync was not 'complete', leaving genuinely-complete rows untouched", async () => { + // Applies the REAL migration 0094 SQL (read straight off disk, not a hand-copied duplicate) against a + // scratch table shaped like the pre-#2537 schema (reviews_synced_at has existed since migration 0006, + // long before it gained any cache-skip meaning), seeded with rows exactly as years of pre-#2537 code + // would have unconditionally stamped reviews_synced_at regardless of whether that sync actually + // succeeded -- this is what a real production database looks like on the day this migration runs. + const dir = mkdtempSync(join(tmpdir(), "gtmig-reviews-")); + writeFileSync( + join(dir, "0001_base.sql"), + `CREATE TABLE pull_request_detail_sync_state ( + id TEXT PRIMARY KEY, + repo_full_name TEXT NOT NULL, + pull_number INTEGER NOT NULL, + status TEXT NOT NULL DEFAULT 'never_synced', + files_synced_at TEXT, + reviews_synced_at TEXT, + checks_synced_at TEXT, + last_synced_at TEXT, + error_summary TEXT, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP + );`, + ); + const db = createD1Adapter(nodeSqliteDriver(new DatabaseSync(":memory:") as never)); + await runSelfHostMigrations(db, dir); + + await db + .prepare( + "insert into pull_request_detail_sync_state (id, repo_full_name, pull_number, status, reviews_synced_at, error_summary, updated_at) values (?, ?, ?, ?, ?, ?, ?)", + ) + .bind("owner/repo#1", "owner/repo", 1, "complete", "2026-05-20T00:00:00.000Z", null, "2026-05-20T00:00:00.000Z") + .run(); + await db + .prepare( + "insert into pull_request_detail_sync_state (id, repo_full_name, pull_number, status, reviews_synced_at, error_summary, updated_at) values (?, ?, ?, ?, ?, ?, ?)", + ) + .bind("owner/repo#2", "owner/repo", 2, "partial", "2026-05-20T00:00:00.000Z", "Review sync failed for #2: GitHub REST and GraphQL detail fetches failed.", "2026-05-20T00:00:00.000Z") + .run(); + await db + .prepare( + "insert into pull_request_detail_sync_state (id, repo_full_name, pull_number, status, reviews_synced_at, error_summary, updated_at) values (?, ?, ?, ?, ?, ?, ?)", + ) + .bind("owner/repo#3", "owner/repo", 3, "partial", "2026-05-20T00:00:00.000Z", "File sync failed for #3: GitHub REST and GraphQL detail fetches failed.", "2026-05-20T00:00:00.000Z") + .run(); + + writeFileSync(join(dir, "0002_reviews_invalidated.sql"), readFileSync("migrations/0094_pull_request_reviews_invalidated.sql", "utf8")); + await runSelfHostMigrations(db, dir); + + const rows = (await db.prepare("select id, status, reviews_synced_at from pull_request_detail_sync_state order by pull_number").all()).results as Array<{ + id: string; + status: string; + reviews_synced_at: string | null; + }>; + expect(rows).toEqual([ + { id: "owner/repo#1", status: "complete", reviews_synced_at: "2026-05-20T00:00:00.000Z" }, // untouched + { id: "owner/repo#2", status: "partial", reviews_synced_at: null }, // reset — reviews specifically failed + { id: "owner/repo#3", status: "partial", reviews_synced_at: null }, // reset — ambiguous (files failed, reviews unverifiable) + ]); + }); + }); +}); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index c5ccf7b9d2..6b29f55f83 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -18,6 +18,7 @@ import { getInstallation, getLatestUpstreamRulesetSnapshot, getPullRequest, + getPullRequestDetailSyncState, getRepository, listUpstreamDriftReports, listInstallationHealth, @@ -12413,6 +12414,100 @@ describe("queue processors", () => { expect(permissionCalls).toEqual([]); }); + it.each(["submitted", "dismissed", "edited"] as const)( + "bumps reviewsInvalidatedAt for the right repo+PR on a pull_request_review '%s' webhook (#2537)", + async (action) => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + JOBS: { async send() {} } as unknown as Queue, + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + return new Response("not found", { status: 404 }); + }); + // Seed an existing sync-state row so the assertion can confirm ONLY reviewsInvalidatedAt moved. + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 42, + title: "Add feature", + state: "open", + user: { login: "contributor" }, + head: { sha: "sha-42" }, + labels: [], + body: "", + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: `review-invalidate-${action}`, + eventName: "pull_request_review", + payload: { + action, + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { + number: 42, + title: "Add feature", + state: "open", + user: { login: "contributor", type: "User" }, + html_url: "https://github.com/JSONbored/gittensory/pull/42", + }, + review: { + state: action === "dismissed" ? "DISMISSED" : "APPROVED", + user: { login: "maintainer", type: "User" }, + submitted_at: "2026-05-28T12:00:00.000Z", + html_url: "https://github.com/JSONbored/gittensory/pull/42#pullrequestreview-1", + }, + sender: { login: "maintainer", type: "User" }, + }, + }); + + const state = await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 42); + expect(state?.reviewsInvalidatedAt).toBeTruthy(); + }, + ); + + it("does not bump reviewsInvalidatedAt for a pull_request_review action outside submitted/dismissed/edited", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + JOBS: { async send() {} } as unknown as Queue, + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "review-invalidate-unsupported-action", + eventName: "pull_request_review", + payload: { + // "submitted" | "dismissed" | "edited" are the only invalidating actions; GitHub also emits others + // (e.g. review comments carry their own event) that must NOT stamp the cache marker. + action: "unrecognized_action" as unknown as "submitted", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { + number: 43, + title: "Add feature", + state: "open", + user: { login: "contributor", type: "User" }, + html_url: "https://github.com/JSONbored/gittensory/pull/43", + }, + review: { + state: "APPROVED", + user: { login: "maintainer", type: "User" }, + submitted_at: "2026-05-28T12:00:00.000Z", + html_url: "https://github.com/JSONbored/gittensory/pull/43#pullrequestreview-1", + }, + sender: { login: "maintainer", type: "User" }, + }, + }); + + expect(await getPullRequestDetailSyncState(env, "JSONbored/gittensory", 43)).toBeNull(); + }); + it("notifies issue-watchers when a new grabbable maintainer-created issue opens (#699 path B)", async () => { const enqueued: Array<{ type: string; event?: { eventType: string; recipientLogin: string; pullNumber: number } }> = []; const env = createTestEnv({ JOBS: { async send(message: { type: string }) { enqueued.push(message); } } as unknown as Queue });