diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 99446933e8..871997fa81 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -180,7 +180,16 @@ import { decryptSecret, encryptSecret, sha256Hex } from "../utils/crypto"; import { errorMessage, jsonString, nowIso, parseJson, repoParts } from "../utils/json"; import { PUBLIC_LOCAL_PATH_SCRUB_PATTERN } from "../signals/redaction"; -const MAX_STORED_BODY_CHARS = 4000; +// GitHub's own documented issue/PR body character limit -- this is a defensive backstop, never an +// intended-to-fire cap. (2026-07-10 incident: the prior 4000-char value silently truncated any body over +// that length before every body-content-dependent check ever saw it -- screenshotTableGate's viewport/theme +// matrix parser, linked-issue-satisfaction, slop keyword matching, etc. -- with zero indication anything was +// cut. Confirmed live on metagraphed#4682: a genuinely complete 12-image Phase C2 table (5160 real chars) got +// closed for "missing before/after screenshot table" because only the first ~4000 chars (one row) were ever +// stored. A cap this repo's own contributor-facing evidence format was never sized against is not a safety +// margin, it's a landmine -- 65536 matches what GitHub itself would already reject, so this can now only ever +// bind on content GitHub was never going to accept in the first place.) +const MAX_STORED_BODY_CHARS = 65536; const SIGNAL_FRESHNESS_LOOKBACK_MS = 14 * 24 * 60 * 60 * 1000; const MAX_SIGNAL_FRESHNESS_TARGETS = 200; const MAX_SIGNAL_FRESHNESS_TARGET_KEY_CHARS = 256; @@ -343,6 +352,7 @@ export async function upsertPullRequestFromGitHub( const existingPayload = preserveSparseBody ? parseJson<{ body?: string | null }>(existingClaimRow.payloadJson, {}) : undefined; const existingBody = existingPayload?.body ?? null; const body = preserveSparseBody ? existingBody : record.body; + logIfBodyTruncated("pull_request", repoFullName, pr.number, preserveSparseBody ? existingBody : pr.body); const payload = preserveSparseBody ? compactGitHubPayload({ ...pr, body: existingBody }) : compactGitHubPayload(pr); const linkedIssues = preserveSparseBody ? parseLinkedIssuesJson(existingClaimRow.linkedIssuesJson) : record.linkedIssues; const linkedIssuesJson = preserveSparseBody ? existingClaimRow.linkedIssuesJson : jsonString(linkedIssues); @@ -436,6 +446,7 @@ export async function upsertIssueFromGitHub(env: Env, repoFullName: string, issu const record = toIssueRecord(repoFullName, issue); const db = getDb(env.DB); const lastSeenOpenAt = issue.state === "open" ? (options.seenOpenAt ?? nowIso()) : null; + logIfBodyTruncated("issue", repoFullName, issue.number, issue.body); await db .insert(issues) .values({ @@ -6091,6 +6102,27 @@ function truncateBody(body: string | null | undefined): string | null { return body.length > MAX_STORED_BODY_CHARS ? body.slice(0, MAX_STORED_BODY_CHARS) : body; } +/** Structured, greppable trace the instant a body-content check (screenshotTableGate, linked-issue + * satisfaction, slop keyword matching, ...) could see a truncated body instead of the real one -- the #4682 + * incident's entire failure mode was that this was previously SILENT (no log, no audit row, nothing), so a + * 4000-char cap quietly corrupted every check reading `pr.body`/`issue.body` for months before anyone + * noticed. MAX_STORED_BODY_CHARS now matches GitHub's own issue/PR body limit, so this should never actually + * fire in practice -- if it ever does, that fact belongs in the logs immediately, not rediscovered later via + * manual DB archaeology. */ +function logIfBodyTruncated(kind: "pull_request" | "issue", repoFullName: string, number: number, body: string | null | undefined): void { + if (!body || body.length <= MAX_STORED_BODY_CHARS) return; + console.log( + JSON.stringify({ + event: "github_app.body_truncated_on_store", + kind, + repoFullName, + number, + originalLength: body.length, + storedLength: MAX_STORED_BODY_CHARS, + }), + ); +} + function toIssueRecordFromRow(row: typeof issues.$inferSelect): IssueRecord { const payload = parseJson<{ body?: string | null; created_at?: string | null; updated_at?: string | null; closed_at?: string | null }>(row.payloadJson, {}); return { diff --git a/test/unit/backfill.test.ts b/test/unit/backfill.test.ts index 2a06df3fd9..054fe59b76 100644 --- a/test/unit/backfill.test.ts +++ b/test/unit/backfill.test.ts @@ -21,6 +21,7 @@ import { upsertInstallationHealth, upsertRepoSyncSegment, upsertRepoSyncState, + getPullRequest, upsertPullRequestFile, upsertPullRequestFromGitHub, upsertIssueFromGitHub, @@ -63,6 +64,125 @@ import { persistRegistrySnapshot } from "../../src/registry/sync"; import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; import { createTestEnv } from "../helpers/d1"; +// #4682 incident (2026-07-10): the stored-body cap used to be 4000 chars -- well under what a compliant +// screenshot-evidence table (or any sufficiently detailed PR/issue) actually needs -- and every body-content +// check (screenshotTableGate's matrix parser included) reads the STORED copy, not a live GitHub fetch, so a +// silently truncated body produced a false "missing evidence" close for a PR that had genuinely complete +// evidence. The cap now matches GitHub's own issue/PR body limit (65536) so it can only ever bind on content +// GitHub itself was never going to accept. +describe("pull request / issue body storage cap (#4682 regression)", () => { + it("stores a body well past the OLD 4000-char cap in full, unmangled", async () => { + const env = createTestEnv(); + const longBody = "x".repeat(5160); // matches the real metagraphed#4682 body length that got truncated + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 4682, + title: "Long body PR", + state: "open", + user: { login: "nickmopen" }, + head: { sha: "abc4682" }, + labels: [], + body: longBody, + }); + const stored = await getPullRequest(env, "JSONbored/gittensory", 4682); + expect(stored?.body).toBe(longBody); + expect(stored?.body?.length).toBe(5160); + }); + + it("still caps a body at GitHub's own 65536-char issue/PR body limit, not unboundedly", async () => { + const env = createTestEnv(); + const oversizedBody = "y".repeat(70000); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 4683, + title: "Oversized body PR", + state: "open", + user: { login: "nickmopen" }, + head: { sha: "abc4683" }, + labels: [], + body: oversizedBody, + }); + const stored = await getPullRequest(env, "JSONbored/gittensory", 4683); + expect(stored?.body?.length).toBe(65536); + expect(stored?.body).toBe(oversizedBody.slice(0, 65536)); + }); + + it("stores an issue body well past the OLD 4000-char cap in full too (compactGitHubPayload is shared)", async () => { + const env = createTestEnv(); + const longBody = "z".repeat(4500); + await upsertIssueFromGitHub(env, "JSONbored/gittensory", { + number: 9001, + title: "Long issue body", + state: "open", + user: { login: "nickmopen" }, + labels: [], + body: longBody, + }); + const stored = await listIssues(env, "JSONbored/gittensory"); + const issue = stored.find((i) => i.number === 9001); + expect(issue?.body).toBe(longBody); + }); + + it("logs a structured, greppable trace the instant a PR body actually gets truncated -- the #4682 failure mode was total silence", async () => { + const env = createTestEnv(); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 4684, + title: "Body past the real GitHub limit", + state: "open", + user: { login: "nickmopen" }, + head: { sha: "abc4684" }, + labels: [], + body: "w".repeat(70000), + }); + const traceLine = logSpy.mock.calls.map((c) => String(c[0])).find((line) => line.includes("github_app.body_truncated_on_store")); + expect(traceLine).toBeDefined(); + const parsed = JSON.parse(traceLine as string) as Record; + expect(parsed).toMatchObject({ event: "github_app.body_truncated_on_store", kind: "pull_request", repoFullName: "JSONbored/gittensory", number: 4684, originalLength: 70000, storedLength: 65536 }); + } finally { + logSpy.mockRestore(); + } + }); + + it("never logs the truncation trace for a body within the cap (the common case stays silent)", async () => { + const env = createTestEnv(); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 4685, + title: "Ordinary body", + state: "open", + user: { login: "nickmopen" }, + head: { sha: "abc4685" }, + labels: [], + body: "normal PR body", + }); + expect(logSpy.mock.calls.map((c) => String(c[0])).some((line) => line.includes("github_app.body_truncated_on_store"))).toBe(false); + } finally { + logSpy.mockRestore(); + } + }); + + it("logs the truncation trace for issue bodies too (compactGitHubPayload is shared)", async () => { + const env = createTestEnv(); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + try { + await upsertIssueFromGitHub(env, "JSONbored/gittensory", { + number: 9002, + title: "Oversized issue body", + state: "open", + user: { login: "nickmopen" }, + labels: [], + body: "v".repeat(70000), + }); + const traceLine = logSpy.mock.calls.map((c) => String(c[0])).find((line) => line.includes("github_app.body_truncated_on_store")); + expect(traceLine).toBeDefined(); + expect(JSON.parse(traceLine as string)).toMatchObject({ kind: "issue", repoFullName: "JSONbored/gittensory", number: 9002 }); + } finally { + logSpy.mockRestore(); + } + }); +}); + describe("GitHub backfill", () => { afterEach(() => { vi.useRealTimers(); diff --git a/test/unit/data-spine.test.ts b/test/unit/data-spine.test.ts index 851e4f40eb..b10c22cd37 100644 --- a/test/unit/data-spine.test.ts +++ b/test/unit/data-spine.test.ts @@ -473,7 +473,9 @@ describe("data spine repositories", () => { expect(await getPullRequest(env, "owner/repo", 1)).toMatchObject({ labels: ["bug"], linkedIssues: [10, 11] }); expect(await getIssue(env, "owner/repo", 10)).toMatchObject({ labels: ["bug"], linkedPrs: [1, 2] }); - expect((await getIssue(env, "owner/repo", 11))?.body).toHaveLength(4000); + // #4682 regression: the stored-body cap is GitHub's own 65536-char issue/PR limit, not the old 4000 -- + // a 5000-char body (well within a real screenshot-evidence table's length) must round-trip in full. + expect((await getIssue(env, "owner/repo", 11))?.body).toHaveLength(5000); expect(await countOpenIssues(env, "owner/repo")).toBe(2); expect(await listOpenIssues(env, "owner/repo")).toEqual(expect.arrayContaining([expect.objectContaining({ number: 10 }), expect.objectContaining({ number: 11 })])); expect(await listIssueSignalSample(env, "owner/repo", 1)).toHaveLength(1);