You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
visual_capture_unobtainable_sha (migration 0207_visual_capture_unobtainable.sql) exists to record one
very specific, PROVEN fact. The migration states the contract exactly:
This column records the head SHA at which the bot PROVED the distinction: the deployments read succeeded,
reported no deployment whatsoever (not a failed one, not an API error), and the poll budget is spent.
The same contract is restated in two more places:
src/types.ts:765
/** #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. */
visualCaptureUnobtainableSha?: string|null|undefined;
src/db/repositories.ts:4715
/**Structurallyunobtainablecapture(#9881):recordthatfor`headSha`thebotPROVEDtherepoproducesno*previewdeploymentatall--thedeploymentsreadSUCCEEDED(notanAPIerror,notafaileddeploy)and*returnednothing,andthepreview-pollbudgetforthishead is spent.
The code does not enforce that contract. previewUnobtainable is set from getPreviewBuildState's "absent" return, and that function returns "absent" on any read failure by design:
src/review/visual/preview-url.ts:310
*failedcard)and"no preview build at all"(don't poll). Returns 'absent' on any read failure (fail-safe:*neveraninfinitepollonatransienterror).
src/review/visual/preview-url.ts:334
returnstate??"absent";}catch{return"absent";}
The deployments read on the same path is swallowed identically — src/review/visual/capture.ts:720:
So at src/review/visual/capture.ts:748 the absent branch cannot distinguish "GitHub answered, there is
no preview check-run" from "the check-runs request threw (5xx, timeout, secondary rate limit, revoked
token)" — and once the poll budget is spent it records the terminal, PROVEN state anyway:
src/review/visual/capture.ts:760
constattempts=awaitpreviewPollAttemptCount(env,target.headSha);if(attempts>=MAX_PREVIEW_POLL_ATTEMPTS){previewFailed=true;// #9881: `absent` means no preview check-run was ever found, and the budget is now spent -- so// this is not a late or broken deploy, it is a repo with no preview pipeline. Recording it is// what lets the screenshot-table gate decline to CLOSE on evidence it could never obtain.previewUnobtainable=true;
src/queue/processors.ts:13010 then persists it, and src/queue/processors.ts:3535 reads it back and
degrades the screenshot-table gate's enforcement for that head:
What breaks in practice: a GitHub API outage (or a token that stops working) lasting across the head's MAX_PREVIEW_POLL_ATTEMPTS poll attempts makes every getPreviewBuildState call return "absent". The
bot then writes visual_capture_unobtainable_sha = headSha for a repo that has a perfectly working preview
pipeline, and the screenshot-table gate stops enforcing — close and block both degrade to nothing (see src/queue/processors.ts:3578–3606) — for that head, permanently, with no path that ever re-checks or
clears the column short of a new commit. The one state the whole feature is built on being proved is
instead inferred from an error the code deliberately made indistinguishable from a fact.
No test asserts previewUnobtainable: true from buildCapture at all — every occurrence in test/ is a false literal in a buildCapture mock (test/unit/queue-3.test.ts:2097, :2176, :2505, :2524).
Requirements
buildCapture (src/review/visual/capture.ts) must set previewUnobtainable: true only when the
check-run read that produced "absent" actually succeeded. A read that threw must leave previewUnobtainable at false.
getPreviewBuildState (src/review/visual/preview-url.ts:313) must keep returning a value on error — its
"never an infinite poll on a transient error" contract at :310 is load-bearing and must NOT change.
Distinguish the two cases by adding a new terminal member to its return union (e.g. "unreadable") that buildCapture treats exactly like "absent" for polling/previewFailed purposes but that never sets previewUnobtainable. Do NOT make getPreviewBuildState throw, and do NOT change what it returns when the
read succeeds and finds no matching check-run (that must stay "absent").
The getLatestDeploymentStatus failure at src/review/visual/capture.ts:720–725 must likewise be
recorded, and a buildCapture call in which that read threw must not set previewUnobtainable: true for
that call even if the later check-run read succeeds and reports "absent" — the recorded state claims the
deployments read succeeded, so a call where it did not must not produce it.
previewPending, previewFailed, renderFailed, and the poll-budget increment behaviour must be
byte-identical to today for every case where both reads succeeded. Adding a new failure signal must not
change how many times a head is polled.
markPullRequestVisualCaptureUnobtainable (src/db/repositories.ts:4726) and the gate evaluator
(packages/loopover-engine/src/review/screenshot-table-gate.ts:417) must NOT change — the defect is
entirely in what evidence produces the flag, not in what the flag does.
⚠️ Required pattern: mirror how getPreviewBuildState already distinguishes "failed" from "building"
from "absent" — one extra member on the same union, resolved at the same call site
(src/review/visual/capture.ts:748), not a second out-of-band return value. What does NOT satisfy this
issue: (a) deleting the previewUnobtainable assignment or the whole #9881 degrade path — the degrade is
correct behaviour for a genuinely pipeline-less repo and must keep working; (b) adding a retry loop or a
second GitHub read inside getPreviewBuildState instead of reporting the failure to its caller; (c) a
PR that only edits the migration/JSDoc comments so they describe the current (wrong) behaviour; (d) a
test-only PR that asserts today's behaviour.
Deliverables
getPreviewBuildState (src/review/visual/preview-url.ts:313) returns a distinct value on a thrown
read instead of "absent", with its doc comment updated to say which value means "read failed" and
which means "read succeeded, no preview build".
buildCapture (src/review/visual/capture.ts) sets previewUnobtainable = true only on the
read-succeeded-and-empty path; the read-failed path yields previewUnobtainable: false while producing
the same previewFailed/poll-budget outcome it produces today.
buildCapture records that the getLatestDeploymentStatus call at src/review/visual/capture.ts:720
threw, and suppresses previewUnobtainable for that call.
Test in test/unit/visual-capture.test.ts: with the check-runs fetch rejecting on every attempt and the
poll budget already at MAX_PREVIEW_POLL_ATTEMPTS, buildCapture returns previewUnobtainable: false and previewPending: false.
Test in test/unit/visual-capture.test.ts: with getLatestDeploymentStatus rejecting but the
check-runs read succeeding and empty at an exhausted budget, buildCapture returns previewUnobtainable: false.
Test in test/unit/visual-capture.test.ts named for this bug (regression test): a sustained GitHub read
failure across the whole poll budget never produces previewUnobtainable: true.
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that fixes getPreviewBuildState's return but leaves the getLatestDeploymentStatus catch at src/review/visual/capture.ts:724 still indistinguishable from a successful empty read, or one that adds
only the two getPreviewBuildState tests — does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts and packages/loopover-engine/src/**/*.ts — both touched files
(src/review/visual/capture.ts, src/review/visual/preview-url.ts) are under src/**, so both are
measured and gated. Every branch this change introduces or touches needs both arms tested: the new
read-failed vs read-succeeded discriminator in getPreviewBuildState's catch; the new union member's arm
at src/review/visual/capture.ts:748 and the existing "absent" arm beside it; the attempts >= MAX_PREVIEW_POLL_ATTEMPTS true and false arms on the new path; and the deployments-read
threw / did-not-throw arms around src/review/visual/capture.ts:720. No change lands in packages/loopover-engine/src/**, so no engine-suite upload is involved.
Expected Outcome
visual_capture_unobtainable_sha means what the migration, the type doc, and the repository function all say
it means: the bot read GitHub successfully, GitHub reported no preview deployment, and the poll budget is
spent. A GitHub outage no longer silently switches a repo's screenshot-table gate off for a head, and a repo
that genuinely has no preview pipeline still gets the #9881 degrade.
Links & Resources
src/review/visual/capture.ts:711-771 — the preview-discovery chain and the previewUnobtainable assignment
src/review/visual/preview-url.ts:307-338 — getPreviewBuildState and its fail-safe "absent" return
src/queue/processors.ts:13010-13029 — where previewUnobtainable is persisted
src/queue/processors.ts:3535 — where it is read back into the gate
src/db/repositories.ts:4715-4732 — markPullRequestVisualCaptureUnobtainable and its stated contract
Context
visual_capture_unobtainable_sha(migration0207_visual_capture_unobtainable.sql) exists to record onevery specific, PROVEN fact. The migration states the contract exactly:
The same contract is restated in two more places:
src/types.ts:765src/db/repositories.ts:4715The code does not enforce that contract.
previewUnobtainableis set fromgetPreviewBuildState's"absent"return, and that function returns"absent"on any read failure by design:src/review/visual/preview-url.ts:310src/review/visual/preview-url.ts:334The deployments read on the same path is swallowed identically —
src/review/visual/capture.ts:720:So at
src/review/visual/capture.ts:748theabsentbranch cannot distinguish "GitHub answered, there isno preview check-run" from "the check-runs request threw (5xx, timeout, secondary rate limit, revoked
token)" — and once the poll budget is spent it records the terminal, PROVEN state anyway:
src/review/visual/capture.ts:760src/queue/processors.ts:13010then persists it, andsrc/queue/processors.ts:3535reads it back anddegrades the screenshot-table gate's enforcement for that head:
What breaks in practice: a GitHub API outage (or a token that stops working) lasting across the head's
MAX_PREVIEW_POLL_ATTEMPTSpoll attempts makes everygetPreviewBuildStatecall return"absent". Thebot then writes
visual_capture_unobtainable_sha = headShafor a repo that has a perfectly working previewpipeline, and the screenshot-table gate stops enforcing —
closeandblockboth degrade to nothing (seesrc/queue/processors.ts:3578–3606) — for that head, permanently, with no path that ever re-checks orclears the column short of a new commit. The one state the whole feature is built on being proved is
instead inferred from an error the code deliberately made indistinguishable from a fact.
No test asserts
previewUnobtainable: truefrombuildCaptureat all — every occurrence intest/is afalseliteral in abuildCapturemock (test/unit/queue-3.test.ts:2097,:2176,:2505,:2524).Requirements
buildCapture(src/review/visual/capture.ts) must setpreviewUnobtainable: trueonly when thecheck-run read that produced
"absent"actually succeeded. A read that threw must leavepreviewUnobtainableatfalse.getPreviewBuildState(src/review/visual/preview-url.ts:313) must keep returning a value on error — its"never an infinite poll on a transient error" contract at
:310is load-bearing and must NOT change.Distinguish the two cases by adding a new terminal member to its return union (e.g.
"unreadable") thatbuildCapturetreats exactly like"absent"for polling/previewFailedpurposes but that never setspreviewUnobtainable. Do NOT makegetPreviewBuildStatethrow, and do NOT change what it returns when theread succeeds and finds no matching check-run (that must stay
"absent").getLatestDeploymentStatusfailure atsrc/review/visual/capture.ts:720–725must likewise berecorded, and a
buildCapturecall in which that read threw must not setpreviewUnobtainable: trueforthat call even if the later check-run read succeeds and reports
"absent"— the recorded state claims thedeployments read succeeded, so a call where it did not must not produce it.
previewPending,previewFailed,renderFailed, and the poll-budget increment behaviour must bebyte-identical to today for every case where both reads succeeded. Adding a new failure signal must not
change how many times a head is polled.
markPullRequestVisualCaptureUnobtainable(src/db/repositories.ts:4726) and the gate evaluator(
packages/loopover-engine/src/review/screenshot-table-gate.ts:417) must NOT change — the defect isentirely in what evidence produces the flag, not in what the flag does.
Deliverables
getPreviewBuildState(src/review/visual/preview-url.ts:313) returns a distinct value on a thrownread instead of
"absent", with its doc comment updated to say which value means "read failed" andwhich means "read succeeded, no preview build".
buildCapture(src/review/visual/capture.ts) setspreviewUnobtainable = trueonly on theread-succeeded-and-empty path; the read-failed path yields
previewUnobtainable: falsewhile producingthe same
previewFailed/poll-budget outcome it produces today.buildCapturerecords that thegetLatestDeploymentStatuscall atsrc/review/visual/capture.ts:720threw, and suppresses
previewUnobtainablefor that call.test/unit/visual-capture.test.ts: with the check-runs fetch rejecting on every attempt and thepoll budget already at
MAX_PREVIEW_POLL_ATTEMPTS,buildCapturereturnspreviewUnobtainable: falseandpreviewPending: false.test/unit/visual-capture.test.ts: with the check-runs fetch succeeding and returning zeromatching check-runs, and the poll budget already at
MAX_PREVIEW_POLL_ATTEMPTS,buildCapturereturnspreviewUnobtainable: true(the behaviour The screenshot-table gate can close PRs for evidence the bot is structurally unable to produce #9881 shipped, now pinned).test/unit/visual-capture.test.ts: withgetLatestDeploymentStatusrejecting but thecheck-runs read succeeding and empty at an exhausted budget,
buildCapturereturnspreviewUnobtainable: false.test/unit/visual-capture.test.tsnamed for this bug (regression test): a sustained GitHub readfailure across the whole poll budget never produces
previewUnobtainable: true.All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that fixes
getPreviewBuildState's return but leaves thegetLatestDeploymentStatuscatch atsrc/review/visual/capture.ts:724still indistinguishable from a successful empty read, or one that addsonly the two
getPreviewBuildStatetests — does not resolve this issue.Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted.
vitest.config.ts'scoverage.includecovers
src/**/*.tsandpackages/loopover-engine/src/**/*.ts— both touched files(
src/review/visual/capture.ts,src/review/visual/preview-url.ts) are undersrc/**, so both aremeasured and gated. Every branch this change introduces or touches needs both arms tested: the new
read-failed vs read-succeeded discriminator in
getPreviewBuildState'scatch; the new union member's armat
src/review/visual/capture.ts:748and the existing"absent"arm beside it; theattempts >= MAX_PREVIEW_POLL_ATTEMPTStrue and false arms on the new path; and the deployments-readthrew / did-not-throw arms around
src/review/visual/capture.ts:720. No change lands inpackages/loopover-engine/src/**, so no engine-suite upload is involved.Expected Outcome
visual_capture_unobtainable_shameans what the migration, the type doc, and the repository function all sayit means: the bot read GitHub successfully, GitHub reported no preview deployment, and the poll budget is
spent. A GitHub outage no longer silently switches a repo's screenshot-table gate off for a head, and a repo
that genuinely has no preview pipeline still gets the #9881 degrade.
Links & Resources
src/review/visual/capture.ts:711-771— the preview-discovery chain and thepreviewUnobtainableassignmentsrc/review/visual/preview-url.ts:307-338—getPreviewBuildStateand its fail-safe"absent"returnsrc/queue/processors.ts:13010-13029— wherepreviewUnobtainableis persistedsrc/queue/processors.ts:3535— where it is read back into the gatesrc/db/repositories.ts:4715-4732—markPullRequestVisualCaptureUnobtainableand its stated contractsrc/types.ts:765-768—visualCaptureUnobtainableSha's doc commentmigrations/0207_visual_capture_unobtainable.sql— the column and the contract it was added under