diff --git a/package.json b/package.json index 4050a1938..dcce4b75f 100644 --- a/package.json +++ b/package.json @@ -102,6 +102,7 @@ "command-redelivery-guards:check": "node --experimental-strip-types scripts/check-command-redelivery-guards.ts", "dispatch-gate-reasons:check": "node --experimental-strip-types scripts/check-dispatch-gate-reasons.ts", "dispatch-provenance:check": "tsx scripts/check-dispatch-provenance-stamped.ts", + "record-mapper-fields:check": "node --experimental-strip-types scripts/check-record-mapper-fields.ts", "replay-runner-manifest": "tsx scripts/replay-runner-image-manifest.ts", "replay-runner-manifest:write": "tsx scripts/replay-runner-image-manifest.ts --write", "replay-runner-manifest:check": "tsx scripts/replay-runner-image-manifest.ts --check", @@ -150,7 +151,7 @@ "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts", "pretest:ci": "npm run check-node-version", - "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run fixture-clock-races:check && npm run typecheck-coverage:check && npm run test-wiring:check && npm run checkers-wired:check && npm run focus-manifest-fields:check && npm run release-commit-parsing:check && npm run releasable-commit-types:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run dispatch-provenance:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run fixture-clock-races:check && npm run typecheck-coverage:check && npm run test-wiring:check && npm run checkers-wired:check && npm run focus-manifest-fields:check && npm run release-commit-parsing:check && npm run releasable-commit-types:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run record-mapper-fields:check && npm run dispatch-provenance:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", "test:watch": "vitest", diff --git a/scripts/check-record-mapper-fields.ts b/scripts/check-record-mapper-fields.ts new file mode 100644 index 000000000..d90f843fd --- /dev/null +++ b/scripts/check-record-mapper-fields.ts @@ -0,0 +1,163 @@ +#!/usr/bin/env node +// #10270: a DB column that is written, declared on the record, and read via `record.field` -- but never +// populated by the row→record mapper. +// +// The instance that prompted this: `visual_capture_unobtainable_sha`. `markPullRequestVisualCaptureUnobtainable` +// wrote it, `PullRequestRecord` declared it, `processors.ts` read it at two sites to decide +// `captureUnobtainable` -- and `mapPullRequestRow` never mapped it. So the read was permanently `undefined`, +// #9881's "this repo has no preview pipeline, degrade the close" path never fired once, and contributor PRs +// kept being closed over evidence they could not produce. One row on the Orb carried the mark; zero degrade +// events in 30 days. +// +// Nothing caught it, and nothing could have: +// +// • TSC cannot: the field is optional (`?:`), so "never populated" and "populated with undefined" are the +// same type. Making it required would catch it, but the engine keeps a structurally-parallel +// `PullRequestRecord` that would then have to gain the field too -- coupling two types that are +// independent on purpose. The type system is the wrong tool here; this is. +// • A test cannot, cheaply: every assertion about the field passes vacuously. A test reading it back gets +// `undefined`, which is indistinguishable from "not set yet" unless the test asserts against the COLUMN. +// +// WHAT THIS CHECKS, and why the predicate is narrow. "Every column must appear in the mapper" is the obvious +// rule and it is wrong: of the 11 unmapped `pull_requests` columns, 10 are legitimate (JSON columns mapped +// under a parsed name, the primary key, columns read through dedicated selects or aggregates). A checker that +// fired on all ten would be muted within a week. The predicate that isolates the real bug is the conjunction: +// +// declared on the record type +// AND backed by a column on the mapper's OWN table +// AND read via `record.field` +// AND absent from the mapper +// +// A field nobody reads off the record is not a bug. A field the mapper populates is not a bug. And a field +// with no column behind it is not a bug either -- `PullRequestRecord.changedFiles` is populated by callers +// that already resolved the diff, and its only same-named column belongs to a DIFFERENT table +// (`recentMergedPullRequests`). Dropping that fourth conjunct makes this checker report it, which is the +// false positive that would get the whole thing muted. Only the intersection of all four is a real defect, +// and on this tree that intersection is exactly the one instance. +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +/** One record type, its mapper, and the sources that read fields off it. */ +type RecordSurface = { + /** Human name, used in the failure message. */ + name: string; + /** File declaring the TS record type. */ + typeFile: string; + /** The `export type = {` / `export interface {` block to read field declarations from. */ + typeName: string; + /** File containing the row→record mapper. */ + mapperFile: string; + /** The `schema.ts` table the mapper reads its row from. A record field with no column here is + * caller-populated, not row-backed, so its absence from the mapper is correct. */ + schemaFile: string; + /** The `export const = sqliteTable(` binding for that table. */ + schemaTable: string; + /** A string unique to the mapper's object literal, used to locate it. */ + mapperAnchor: string; + /** Files whose `.` reads count as "someone reads this off the record". */ + readerFiles: readonly string[]; + /** Bindings that hold a record of this type in the reader files (e.g. `pr.headSha`). */ + readerBindings: readonly string[]; +}; + +const SURFACES: readonly RecordSurface[] = [ + { + name: "PullRequestRecord", + typeFile: "src/types.ts", + typeName: "PullRequestRecord", + mapperFile: "src/db/repositories.ts", + mapperAnchor: "screenshotTablePresenceSatisfied: parseJson<", + schemaFile: "src/db/schema.ts", + schemaTable: "pullRequests", + readerFiles: ["src/queue/processors.ts"], + readerBindings: ["pr", "pullRequest", "storedPr"], + }, +]; + +export type Violation = { surface: string; field: string }; + +/** Field names declared on `typeName`'s object body. */ +export function declaredFields(source: string, typeName: string): string[] { + const start = source.search(new RegExp(`export (?:type ${typeName} = \\{|interface ${typeName} \\{)`)); + if (start === -1) return []; + // The declaration ends at the first line that is a closing brace in column 0 (`};` or `}`), which is how + // every record type in this repo is formatted. + const rest = source.slice(start); + const end = rest.search(/\n\}[;]?\n/); + const body = end === -1 ? rest : rest.slice(0, end); + return [...new Set([...body.matchAll(/^\s{2}(\w+)\??:/gm)].map((match) => match[1]!))]; +} + +/** Field names the mapper's object literal populates. */ +export function mappedFields(source: string, anchor: string): string[] { + const at = source.indexOf(anchor); + if (at === -1) return []; + const open = source.lastIndexOf("return {", at); + if (open === -1) return []; + const close = source.indexOf("\n };", at); + const body = source.slice(open, close === -1 ? source.length : close); + return [...new Set([...body.matchAll(/^\s{4}(\w+):/gm)].map((match) => match[1]!))]; +} + +/** The TS-side column names declared on `tableName`'s Drizzle table body. */ +export function tableColumns(source: string, tableName: string): string[] { + const start = source.indexOf(`export const ${tableName} = `); + if (start === -1) return []; + const rest = source.slice(start); + const end = rest.search(/\n\)/); + const body = end === -1 ? rest : rest.slice(0, end); + return [...new Set([...body.matchAll(/^\s{4}(\w+):\s*\w+\(/gm)].map((match) => match[1]!))]; +} + +/** Fields read as `.` anywhere in `sources`. */ +export function fieldsReadOffRecord(sources: readonly string[], bindings: readonly string[]): Set { + const read = new Set(); + const pattern = new RegExp(`\\b(?:${bindings.join("|")})\\??\\.(\\w+)`, "g"); + for (const source of sources) { + for (const match of source.matchAll(pattern)) read.add(match[1]!); + } + return read; +} + +/** Declared AND read off the record AND not mapped — see the header for why all three conjuncts are needed. */ +export function findViolations( + surface: RecordSurface, + files: { type: string; mapper: string; schema: string; readers: readonly string[] }, +): Violation[] { + const declared = declaredFields(files.type, surface.typeName); + const mapped = new Set(mappedFields(files.mapper, surface.mapperAnchor)); + const columns = new Set(tableColumns(files.schema, surface.schemaTable)); + const read = fieldsReadOffRecord(files.readers, surface.readerBindings); + return declared + .filter((field) => columns.has(field) && read.has(field) && !mapped.has(field)) + .map((field) => ({ surface: surface.name, field })); +} + +function main(): void { + const violations: Violation[] = []; + for (const surface of SURFACES) { + violations.push( + ...findViolations(surface, { + type: readFileSync(surface.typeFile, "utf8"), + mapper: readFileSync(surface.mapperFile, "utf8"), + schema: readFileSync(surface.schemaFile, "utf8"), + readers: surface.readerFiles.map((file) => readFileSync(file, "utf8")), + }), + ); + } + if (violations.length === 0) { + console.log(`check-record-mapper-fields: every declared-and-read record field is populated by its mapper.`); + return; + } + process.stderr.write(`check-record-mapper-fields: ${violations.length} field(s) are declared, read, and never mapped (#10270):\n\n`); + for (const violation of violations) process.stderr.write(` ${violation.surface}.${violation.field}\n`); + process.stderr.write( + "\nEach of these reads `undefined` at every call site, forever, with no compiler error and no failing\n" + + "test -- the field is optional, so 'never populated' and 'populated with undefined' are the same type.\n" + + "That is how #9881's capture-unobtainable degrade sat dead in production while its column was being\n" + + "written. Map the field in the row->record mapper beside its neighbours.\n", + ); + process.exit(1); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) main(); diff --git a/src/db/repositories.ts b/src/db/repositories.ts index dac76652b..7aff4d2c9 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -7506,6 +7506,9 @@ function toPullRequestRecordFromRow(row: typeof pullRequests.$inferSelect): Pull linkedIssueHardRuleViolationIssues: parseJson(row.linkedIssueHardRuleViolationIssuesJson, []), linkedIssueHardRuleViolationReason: row.linkedIssueHardRuleViolationReason, visualCaptureSatisfiedSha: row.visualCaptureSatisfiedSha, + // #10270: this one was missing while its two neighbours were mapped, so markPullRequestVisualCaptureUnobtainable + // wrote a column no reader could ever see and #9881's degrade never fired once in production. + visualCaptureUnobtainableSha: row.visualCaptureUnobtainableSha, visualCaptureRetryPendingSha: row.visualCaptureRetryPendingSha, visualCaptureRetryPendingAt: row.visualCaptureRetryPendingAt, manualReviewLabelAppliedSha: row.manualReviewLabelAppliedSha, diff --git a/src/types.ts b/src/types.ts index c4105b08a..0cd75cbc7 100644 --- a/src/types.ts +++ b/src/types.ts @@ -774,7 +774,15 @@ export type PullRequestRecord = { visualCaptureSatisfiedSha?: string | null | undefined; /** #9881: the head SHA at which the bot PROVED visual capture is structurally unobtainable here -- the * deployments read succeeded, found none at all, and the poll budget is spent. The screenshotTableGate - * degrades its CLOSE to advisory while this equals the current head. */ + * degrades its CLOSE to advisory while this equals the current head. Publish-written; read straight from + * the row. + * + * #10270: this field was WRITTEN and READ for months while the row->record mapper omitted it, so every + * reader saw `undefined` and the degrade never fired once. Making it required would turn an unmapped field + * into a build error, but `?:` is load-bearing here -- the engine's structurally-parallel + * `PullRequestRecord` (packages/loopover-engine/src/types/reward-risk-types.ts) would have to gain the + * field too, coupling two types kept independent on purpose. `scripts/check-record-mapper-fields.ts` + * closes the same hole without that coupling, and closes it for every field rather than this one. */ visualCaptureUnobtainableSha?: string | null | undefined; /** False-positive close guard (#9030): the head SHA a bounded visual-capture recapture retry is currently * scheduled/in-flight for -- set only when the capture pipeline errored, or the preview is still building, diff --git a/test/unit/check-record-mapper-fields.test.ts b/test/unit/check-record-mapper-fields.test.ts new file mode 100644 index 000000000..029675ff3 --- /dev/null +++ b/test/unit/check-record-mapper-fields.test.ts @@ -0,0 +1,149 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; +import { declaredFields, fieldsReadOffRecord, findViolations, mappedFields, tableColumns } from "../../scripts/check-record-mapper-fields"; + +// #10270: the checker's value is entirely in which cases it fires on. A version that missed the real bug is +// useless; a version that fired on the ten legitimate unmapped columns would be muted inside a week. So each +// case below is written as the tree-shape that produces it, and the false-positive cases matter as much as the +// true one. + +const SURFACE = { + name: "PullRequestRecord", + typeFile: "src/types.ts", + typeName: "PullRequestRecord", + mapperFile: "src/db/repositories.ts", + mapperAnchor: "screenshotTablePresenceSatisfied: parseJson<", + schemaFile: "src/db/schema.ts", + schemaTable: "pullRequests", + readerFiles: ["src/queue/processors.ts"], + readerBindings: ["pr", "pullRequest", "storedPr"], +} as const; + +const TYPE = ` +export type PullRequestRecord = { + headSha?: string | null | undefined; + visualCaptureSatisfiedSha?: string | null | undefined; + visualCaptureUnobtainableSha?: string | null | undefined; + changedFiles?: string[] | undefined; +}; + +export type IssueRecord = { + unrelated: string; +}; +`; + +const SCHEMA = ` +export const pullRequests = sqliteTable( + "pull_requests", + { + headSha: text("head_sha"), + visualCaptureSatisfiedSha: text("visual_capture_satisfied_sha"), + visualCaptureUnobtainableSha: text("visual_capture_unobtainable_sha"), + }, +); + +export const recentMergedPullRequests = sqliteTable( + "recent_merged_pull_requests", + { + changedFilesJson: text("changed_files_json"), + }, +); +`; + +const mapper = (fields: readonly string[]) => ` +function mapPullRequestRow(row: Row) { + return { +${fields.map((field) => ` ${field}: row.${field},`).join("\n")} + screenshotTablePresenceSatisfied: parseJson(row.screenshotTablePresenceSatisfiedJson, null), + }; +} +`; + +const READER = ` + const a = pr.headSha; + const b = pr.visualCaptureSatisfiedSha; + const c = Boolean(pr.headSha) && pr.visualCaptureUnobtainableSha === pr.headSha; + const d = pr.changedFiles; +`; + +const run = (fields: readonly string[], type = TYPE, reader = READER) => + findViolations(SURFACE, { type, mapper: mapper(fields), schema: SCHEMA, readers: [reader] }); + +describe("check-record-mapper-fields (#10270)", () => { + it("fires on the real bug: declared, column-backed, read, and unmapped", () => { + expect(run(["headSha", "visualCaptureSatisfiedSha"])).toEqual([{ surface: "PullRequestRecord", field: "visualCaptureUnobtainableSha" }]); + }); + + it("is silent once the field is mapped", () => { + expect(run(["headSha", "visualCaptureSatisfiedSha", "visualCaptureUnobtainableSha"])).toEqual([]); + }); + + it("does NOT fire on a caller-populated field whose only same-named column belongs to another table", () => { + // `changedFiles` is read off the record and never mapped, and that is correct -- callers that already + // resolved the diff set it. Dropping the column-backed conjunct reports it, and this checker would then be + // muted for the one case it exists to catch. + const violations = run(["headSha", "visualCaptureSatisfiedSha", "visualCaptureUnobtainableSha"]); + expect(violations.map((violation) => violation.field)).not.toContain("changedFiles"); + }); + + it("does NOT fire on a column-backed field nobody reads off the record", () => { + // Ten of the eleven unmapped pull_requests columns are this shape. A rule of "every column must be mapped" + // would report all of them. + expect(run(["visualCaptureSatisfiedSha", "visualCaptureUnobtainableSha"], TYPE, "const x = 1;")).toEqual([]); + }); + + it("does NOT fire on a declared field with no column behind it at all", () => { + const typeOnly = TYPE.replace(" changedFiles?: string[] | undefined;", " computedElsewhere?: string | undefined;"); + expect(run(["headSha", "visualCaptureSatisfiedSha", "visualCaptureUnobtainableSha"], typeOnly, "const y = pr.computedElsewhere;")).toEqual([]); + }); + + it("passes on the real tree", () => { + expect( + findViolations(SURFACE, { + type: readFileSync(SURFACE.typeFile, "utf8"), + mapper: readFileSync(SURFACE.mapperFile, "utf8"), + schema: readFileSync(SURFACE.schemaFile, "utf8"), + readers: SURFACE.readerFiles.map((file) => readFileSync(file, "utf8")), + }), + ).toEqual([]); + }); + + describe("helpers", () => { + it("declaredFields reads only the named type's own body", () => { + const fields = declaredFields(TYPE, "PullRequestRecord"); + expect(fields).toContain("visualCaptureUnobtainableSha"); + expect(fields).not.toContain("unrelated"); + expect(declaredFields(TYPE, "NoSuchRecord")).toEqual([]); + }); + + it("declaredFields handles an interface declaration and an unterminated body", () => { + expect(declaredFields("export interface Thing {\n a?: string;\n}\n", "Thing")).toEqual(["a"]); + expect(declaredFields("export type Thing = {\n a?: string;", "Thing")).toEqual(["a"]); + }); + + it("mappedFields reads the mapper's object literal, and returns nothing without an anchor", () => { + expect(mappedFields(mapper(["headSha"]), SURFACE.mapperAnchor)).toEqual(["headSha", "screenshotTablePresenceSatisfied"]); + expect(mappedFields(mapper(["headSha"]), "no-such-anchor")).toEqual([]); + expect(mappedFields("screenshotTablePresenceSatisfied: parseJson<", SURFACE.mapperAnchor)).toEqual([]); + }); + + it("mappedFields runs to end-of-source when the literal is unterminated", () => { + expect(mappedFields("return {\n a: row.a,\n screenshotTablePresenceSatisfied: parseJson<", SURFACE.mapperAnchor)).toEqual(["a", "screenshotTablePresenceSatisfied"]); + }); + + it("tableColumns reads one table's columns, and returns nothing for an unknown table", () => { + expect(tableColumns(SCHEMA, "pullRequests")).toEqual(["headSha", "visualCaptureSatisfiedSha", "visualCaptureUnobtainableSha"]); + expect(tableColumns(SCHEMA, "recentMergedPullRequests")).toEqual(["changedFilesJson"]); + expect(tableColumns(SCHEMA, "noSuchTable")).toEqual([]); + }); + + it("tableColumns runs to end-of-source when the table body is unterminated", () => { + expect(tableColumns('export const t = sqliteTable(\n "t",\n {\n a: text("a"),', "t")).toEqual(["a"]); + }); + + it("fieldsReadOffRecord picks up optional-chained reads and ignores other bindings", () => { + const read = fieldsReadOffRecord(["const a = pr?.headSha; const b = other.notARecordField;"], ["pr"]); + expect([...read]).toEqual(["headSha"]); + }); + }); +}); diff --git a/test/unit/db-persistence.test.ts b/test/unit/db-persistence.test.ts index 49efc6cdb..2ff1d46c6 100644 --- a/test/unit/db-persistence.test.ts +++ b/test/unit/db-persistence.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it } from "vitest"; import { getPullRequest, markPullRequestManualReviewLabelApplied, + markPullRequestVisualCaptureSatisfied, + markPullRequestVisualCaptureUnobtainable, clearPullRequestManualReviewLabelProvenance, getActiveReviewStartedAt, getBounty, @@ -785,3 +787,45 @@ describe("listMergedPullRequestsInWindow (#10168)", () => { expect(await listMergedPullRequestsInWindow(env, "owner/repo", "2026-07-30T00:00:00Z", "2026-07-30T23:59:59Z")).toEqual([]); }); }); + +describe("visual-capture unobtainable provenance (#10270)", () => { + const seed = async (env: ReturnType) => { + await upsertRepositoryFromGitHub(env, { full_name: "owner/repo", name: "repo", id: 1, private: false } as never, 4242); + await upsertPullRequestFromGitHub(env, "owner/repo", { + number: 12, title: "t", state: "open", user: { login: "c" }, head: { sha: "head1" }, labels: [], created_at: "2026-07-05T10:00:00.000Z", + } as never); + }; + + it("REGRESSION: the mark round-trips to the RECORD, not just to the column", async () => { + // The bug: the write worked, the column held the value, and the row->record mapper never populated the + // field -- so `pr.visualCaptureUnobtainableSha` was `undefined` at every reader, `captureUnobtainable` was + // permanently false, and #9881's degrade never fired once in production (0 events in 30 days on the Orb + // while a row carried the mark). Asserting the COLUMN would have passed throughout; only reading it back + // through getPullRequest, the way every real caller does, catches it. + const env = createTestEnv(); + await seed(env); + await markPullRequestVisualCaptureUnobtainable(env, "owner/repo", 12, "head1"); + + const stored = await getPullRequest(env, "owner/repo", 12); + expect(stored?.visualCaptureUnobtainableSha).toBe("head1"); + // The comparison every reader actually performs -- this is what was permanently false. + expect(Boolean(stored?.headSha) && stored?.visualCaptureUnobtainableSha === stored?.headSha).toBe(true); + }); + + it("absent by default, so an unmarked PR never reads as unobtainable", async () => { + const env = createTestEnv(); + await seed(env); + expect((await getPullRequest(env, "owner/repo", 12))?.visualCaptureUnobtainableSha).toBeNull(); + }); + + it("a successful capture CLEARS it -- evidence exists now, so the degrade must stop applying", async () => { + const env = createTestEnv(); + await seed(env); + await markPullRequestVisualCaptureUnobtainable(env, "owner/repo", 12, "head1"); + await markPullRequestVisualCaptureSatisfied(env, "owner/repo", 12, "head1"); + + const stored = await getPullRequest(env, "owner/repo", 12); + expect(stored?.visualCaptureUnobtainableSha).toBeNull(); + expect(stored?.visualCaptureSatisfiedSha).toBe("head1"); + }); +});