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
#9881 added an enforcement degrade to the screenshot-table gate: when the bot has proved it can never
produce a capture for this repo, a close must not fire. Both the engine and the call site say, explicitly,
that the finding survives the degrade even though the enforcement does not.
/**#9881:setwhentheviolationstandsbutENFORCEMENTmustnot.Thebotprovedthisrepoproducesno*previewdeploymentatall,so`review.visual.enabled`can never satisfythegatehereandaCLOSEwould*destroyPRsoverevidencenocontributoractioncouldsupply.Thefinding is stillraised--thePR*reallydoeslackvisualevidence--butthecallerdegrades`action: "close"`toadvisoryandsaysthis.
src/queue/processors.ts:3572
// #9881: the bot proved this repo produces no preview deployment at all, so `review.visual.enabled` can// never satisfy this gate here. The violation stands and still surfaces as a finding, but a CLOSE would// destroy a contributor's PR over a maintainer-side pipeline gap no contributor action can close, and// which nothing had ever told the maintainer about. Degrade to advisory and say so.
And the reason string itself claims to be contributor-visible — packages/loopover-engine/src/review/screenshot-table-gate.ts:404:
/** #9881: the reason attached when enforcement is degraded. Stated in the contributor-visible comment, so * the PR author learns the gate could not be satisfied here rather than being left to guess. */exportconstCAPTURE_UNOBTAINABLE_REASON=
None of that is true. There is exactly one code path in this repo that turns a screenshot-table violation
into a finding, and it refuses to run in precisely the degraded case:
The degrade only ever applies when the configured action is close or block — with action: "advisory"
there is no enforcement to degrade. So maybeAddScreenshotTableAdvisoryFinding always returns early on the
degraded path and never appends screenshot_table_missing. Meanwhile in the planner input:
screenshotTableMatch is undefined (src/queue/processors.ts:3579, gated on !screenshotTableEnforcementDegraded),
screenshotEvidenceHold is undefined (src/queue/processors.ts:3600, same gate),
screenshotTableEvidenceUnresolved is false (src/queue/processors.ts:3619, same gate),
so planAgentMaintenanceActions (src/settings/agent-actions.ts:1029–1044) falls straight through to
ordinary merit/CI evaluation. CAPTURE_UNOBTAINABLE_REASON reaches exactly one place — an internal audit
event's detail at src/queue/processors.ts:3613 — and nothing else. It is grep-confirmed unused outside packages/loopover-engine/src/review/screenshot-table-gate.ts:420 and that audit call.
What breaks in practice: on a repo with requireScreenshotTable.action: "close" (the default action) whose
preview pipeline is proved unobtainable, a PR with no visual evidence is silently merged as if the gate had
never been configured. The maintainer is never told their gate is unsatisfiable, the contributor is never
told anything, and the only trace is an audit row nobody reads. The exact failure #9881's own migration
comment names — "nothing had ever told the maintainer about" the gap — is reproduced by the fix for it.
test/unit/screenshot-table-gate.test.ts:909 covers the pure evaluator only (that enforcementDegradedReason is set and violated stays true); nothing tests the wiring.
Requirements
When evaluateScreenshotTableGate returns an enforcementDegradedReason, a warning-severity advisory
finding MUST be appended to the PR's advisory findings regardless of the configured action, carrying both
the gate's own reason and CAPTURE_UNOBTAINABLE_REASON so the maintainer reads the remedy.
maybeAddScreenshotTableAdvisoryFinding (src/queue/processors.ts:8748) is the function that must be
extended — it must accept the captureUnobtainable fact and pass it into evaluateScreenshotTableGate,
and its action !== "advisory" early return must become "return only when the action is not advisory
AND enforcement was not degraded". Do NOT add a second, parallel finding-emitting function.
The call site at src/queue/processors.ts:12119 must pass the same Boolean(pr.headSha) && pr.visualCaptureUnobtainableSha === pr.headSha value the enforcement call site at src/queue/processors.ts:3535 computes, so the two evaluations of the same pure check cannot disagree.
Behaviour that must NOT change: action: "advisory" with no degrade must keep emitting exactly the finding
it emits today, with the same code, severity, title, action and publicText; a degraded gate must
still produce NO close, NO hold, and NO blocker; VISUAL/gate blocker classification must not change
(screenshot_table_missing must remain unrecognized by isConfiguredGateBlocker); and a non-degraded close/block violation must keep producing no advisory finding (its close/hold comment already says it).
The finding emitted on the degraded path must be distinguishable from the ordinary advisory one — it must
state that enforcement was degraded and why — so a maintainer can tell "you configured advisory" apart from
"your gate cannot be satisfied here".
⚠️ Required pattern: mirror maybeAddLockfileTamperFinding (src/queue/processors.ts, immediately above maybeAddScreenshotTableAdvisoryFinding) — one function, one advisory.findings.push, all evaluation
errors swallowed so the gate can never be destabilized. What does NOT satisfy this issue: (a) rewriting screenshotTableGateConfig.action to "advisory" somewhere upstream so the existing early return happens
to pass — that silently changes what GET/settings surfaces report the repo's action to be; (b) emitting
the notice as a GitHub comment or check-run annotation instead of an advisory finding — the advisory
pipeline is the surface every sibling gate uses; (c) making the degraded case produce a hold or a close,
which is the exact behaviour #9881 removed; (d) a test-only PR asserting today's silence.
Deliverables
maybeAddScreenshotTableAdvisoryFinding (src/queue/processors.ts:8748) takes a captureUnobtainable: boolean argument, threads it into its evaluateScreenshotTableGate call, and no
longer early-returns when result.enforcementDegradedReason !== undefined.
The degraded path pushes a finding whose detail/publicText contains CAPTURE_UNOBTAINABLE_REASON
(imported from packages/loopover-engine/src/review/screenshot-table-gate.ts) in addition to the gate's
own reason.
The call site at src/queue/processors.ts:12119 passes Boolean(pr.headSha) && pr.visualCaptureUnobtainableSha === pr.headSha.
Test in test/unit/screenshot-table-gate.test.ts: action: "close", gate violated, captureUnobtainable: true ⇒ maybeAddScreenshotTableAdvisoryFinding appends exactly one finding whose detail includes the CAPTURE_UNOBTAINABLE_REASON text, and the plan built from the same inputs
contains no close and no manual-review hold.
Test in test/unit/screenshot-table-gate.test.ts: action: "block", gate violated, captureUnobtainable: true ⇒ the same finding is appended and no hold is planned.
Test in test/unit/screenshot-table-gate.test.ts: action: "close", gate violated, captureUnobtainable: false ⇒ NO advisory finding is appended (today's behaviour, pinned).
Test in test/unit/screenshot-table-gate.test.ts: action: "advisory", gate violated, captureUnobtainable: false ⇒ byte-identical finding to today.
Regression test in test/unit/screenshot-table-gate.test.ts named for this bug: a degraded action: "close" evaluation never produces a completely silent pass (assert advisory.findings is
non-empty).
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that only removes the action !== "advisory" early return without threading captureUnobtainable from the
call site (so the degrade is never detected there at all), or one that adds the finding but not the captureUnobtainable: false pinning 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 — the touched file
(src/queue/processors.ts) is under src/**, so it is measured and gated. Every branch needs both arms:
the rewritten early return (!enabled, action !== "advisory" with degrade, action !== "advisory" without
degrade, action === "advisory"); the degraded vs non-degraded finding-shape branch; and the Boolean(pr.headSha) && pr.visualCaptureUnobtainableSha === pr.headSha conjunction at the call site (headSha
absent, headSha present and matching, headSha present and not matching). If the PR also touches packages/loopover-engine/src/review/screenshot-table-gate.ts, engine lines are credited by two uploads
whose hits are unioned — add the test to packages/loopover-engine/test/** as well as any root test/**
coverage, or the patch gate can still fail.
Expected Outcome
A repo whose screenshot-table gate cannot be satisfied stops failing silently. The contributor's PR is still
never closed for evidence the pipeline cannot produce, but the PR now carries a visible, non-blocking finding
naming the exact maintainer-side remedy — which is what both #9881's code comments and its migration already
claim happens.
Links & Resources
src/queue/processors.ts:3572-3617 — the degrade decision and the audit-only surfacing
src/queue/processors.ts:8748-8794 — maybeAddScreenshotTableAdvisoryFinding and its early return
src/queue/processors.ts:12117-12128 — the advisory call site
src/settings/agent-actions.ts:1013-1045 — the planner block that is skipped entirely on the degraded path
packages/loopover-engine/src/review/screenshot-table-gate.ts:404-421 — CAPTURE_UNOBTAINABLE_REASON and the degrade
test/unit/screenshot-table-gate.test.ts:909-949 — the existing pure-evaluator coverage
Context
#9881 added an enforcement degrade to the screenshot-table gate: when the bot has proved it can never
produce a capture for this repo, a
closemust not fire. Both the engine and the call site say, explicitly,that the finding survives the degrade even though the enforcement does not.
packages/loopover-engine/src/review/screenshot-table-gate.ts:343src/queue/processors.ts:3572And the reason string itself claims to be contributor-visible —
packages/loopover-engine/src/review/screenshot-table-gate.ts:404:None of that is true. There is exactly one code path in this repo that turns a screenshot-table violation
into a finding, and it refuses to run in precisely the degraded case:
src/queue/processors.ts:8761The degrade only ever applies when the configured action is
closeorblock— withaction: "advisory"there is no enforcement to degrade. So
maybeAddScreenshotTableAdvisoryFindingalways returns early on thedegraded path and never appends
screenshot_table_missing. Meanwhile in the planner input:screenshotTableMatchisundefined(src/queue/processors.ts:3579, gated on!screenshotTableEnforcementDegraded),screenshotEvidenceHoldisundefined(src/queue/processors.ts:3600, same gate),screenshotTableEvidenceUnresolvedisfalse(src/queue/processors.ts:3619, same gate),so
planAgentMaintenanceActions(src/settings/agent-actions.ts:1029–1044) falls straight through toordinary merit/CI evaluation.
CAPTURE_UNOBTAINABLE_REASONreaches exactly one place — an internal auditevent's
detailatsrc/queue/processors.ts:3613— and nothing else. It is grep-confirmed unused outsidepackages/loopover-engine/src/review/screenshot-table-gate.ts:420and that audit call.What breaks in practice: on a repo with
requireScreenshotTable.action: "close"(the default action) whosepreview pipeline is proved unobtainable, a PR with no visual evidence is silently merged as if the gate had
never been configured. The maintainer is never told their gate is unsatisfiable, the contributor is never
told anything, and the only trace is an audit row nobody reads. The exact failure #9881's own migration
comment names — "nothing had ever told the maintainer about" the gap — is reproduced by the fix for it.
test/unit/screenshot-table-gate.test.ts:909covers the pure evaluator only (thatenforcementDegradedReasonis set andviolatedstaystrue); nothing tests the wiring.Requirements
evaluateScreenshotTableGatereturns anenforcementDegradedReason, a warning-severity advisoryfinding MUST be appended to the PR's advisory findings regardless of the configured
action, carrying boththe gate's own
reasonandCAPTURE_UNOBTAINABLE_REASONso the maintainer reads the remedy.maybeAddScreenshotTableAdvisoryFinding(src/queue/processors.ts:8748) is the function that must beextended — it must accept the
captureUnobtainablefact and pass it intoevaluateScreenshotTableGate,and its
action !== "advisory"early return must become "return only when the action is notadvisoryAND enforcement was not degraded". Do NOT add a second, parallel finding-emitting function.
src/queue/processors.ts:12119must pass the sameBoolean(pr.headSha) && pr.visualCaptureUnobtainableSha === pr.headShavalue the enforcement call site atsrc/queue/processors.ts:3535computes, so the two evaluations of the same pure check cannot disagree.action: "advisory"with no degrade must keep emitting exactly the findingit emits today, with the same
code,severity,title,actionandpublicText; a degraded gate muststill produce NO close, NO hold, and NO blocker;
VISUAL/gate blocker classification must not change(
screenshot_table_missingmust remain unrecognized byisConfiguredGateBlocker); and a non-degradedclose/blockviolation must keep producing no advisory finding (its close/hold comment already says it).state that enforcement was degraded and why — so a maintainer can tell "you configured advisory" apart from
"your gate cannot be satisfied here".
Deliverables
maybeAddScreenshotTableAdvisoryFinding(src/queue/processors.ts:8748) takes acaptureUnobtainable: booleanargument, threads it into itsevaluateScreenshotTableGatecall, and nolonger early-returns when
result.enforcementDegradedReason !== undefined.detail/publicTextcontainsCAPTURE_UNOBTAINABLE_REASON(imported from
packages/loopover-engine/src/review/screenshot-table-gate.ts) in addition to the gate'sown
reason.src/queue/processors.ts:12119passesBoolean(pr.headSha) && pr.visualCaptureUnobtainableSha === pr.headSha.test/unit/screenshot-table-gate.test.ts:action: "close", gate violated,captureUnobtainable: true⇒maybeAddScreenshotTableAdvisoryFindingappends exactly one finding whosedetailincludes theCAPTURE_UNOBTAINABLE_REASONtext, and the plan built from the same inputscontains no
closeand no manual-review hold.test/unit/screenshot-table-gate.test.ts:action: "block", gate violated,captureUnobtainable: true⇒ the same finding is appended and no hold is planned.test/unit/screenshot-table-gate.test.ts:action: "close", gate violated,captureUnobtainable: false⇒ NO advisory finding is appended (today's behaviour, pinned).test/unit/screenshot-table-gate.test.ts:action: "advisory", gate violated,captureUnobtainable: false⇒ byte-identical finding to today.test/unit/screenshot-table-gate.test.tsnamed for this bug: a degradedaction: "close"evaluation never produces a completely silent pass (assertadvisory.findingsisnon-empty).
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that only removes the
action !== "advisory"early return without threadingcaptureUnobtainablefrom thecall site (so the degrade is never detected there at all), or one that adds the finding but not the
captureUnobtainable: falsepinning tests — 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— the touched file(
src/queue/processors.ts) is undersrc/**, so it is measured and gated. Every branch needs both arms:the rewritten early return (
!enabled,action !== "advisory"with degrade,action !== "advisory"withoutdegrade,
action === "advisory"); the degraded vs non-degraded finding-shape branch; and theBoolean(pr.headSha) && pr.visualCaptureUnobtainableSha === pr.headShaconjunction at the call site (headShaabsent, headSha present and matching, headSha present and not matching). If the PR also touches
packages/loopover-engine/src/review/screenshot-table-gate.ts, engine lines are credited by two uploadswhose hits are unioned — add the test to
packages/loopover-engine/test/**as well as any roottest/**coverage, or the patch gate can still fail.
Expected Outcome
A repo whose screenshot-table gate cannot be satisfied stops failing silently. The contributor's PR is still
never closed for evidence the pipeline cannot produce, but the PR now carries a visible, non-blocking finding
naming the exact maintainer-side remedy — which is what both #9881's code comments and its migration already
claim happens.
Links & Resources
src/queue/processors.ts:3572-3617— the degrade decision and the audit-only surfacingsrc/queue/processors.ts:8748-8794—maybeAddScreenshotTableAdvisoryFindingand its early returnsrc/queue/processors.ts:12117-12128— the advisory call sitesrc/settings/agent-actions.ts:1013-1045— the planner block that is skipped entirely on the degraded pathpackages/loopover-engine/src/review/screenshot-table-gate.ts:404-421—CAPTURE_UNOBTAINABLE_REASONand the degradetest/unit/screenshot-table-gate.test.ts:909-949— the existing pure-evaluator coverage