Skip to content

gate(visual): capture-retry marker never cleared + pending marker skips instead of holds → screenshot gate silently disabled (false merge) #9462

Description

@JSONbored

Parent: #9461

Summary

The screenshot-table gate's close-deferral marker (visualCaptureRetryPendingSha, added by #9030) is never cleared when the capture retry budget is exhausted, and a head carrying a stale marker causes the gate to be skipped entirely rather than held. Combined, a PR whose preview never renders can auto-merge in violation of a gate that is configured to close.

This is two defects that compound; both need fixing.

Part A — the marker is never cleared on exhaustion

src/queue/processors.ts:9469-9491 (scheduleVisualCaptureRetry):

if (args.previewPollAttempt >= MAX_PREVIEW_POLL_ATTEMPTS) return;
if (args.pr.headSha) {
  await markPullRequestVisualCaptureRetryPending(env, args.repoFullName, args.pr.number, args.pr.headSha)...
}

The doc comment at :9464-9467 claims:

once MAX_PREVIEW_POLL_ATTEMPTS is reached, the marker is deliberately left unset so the gate falls through to its normal (accurate) evaluation on this final attempt

This is false in code. The early return only skips setting the marker again; the marker written by the previous attempt is still there. Nothing clears it.

Exhaustive verification — grep for every reader/writer of the column across src/:

  • One clear: src/db/repositories.ts:4489markPullRequestVisualCaptureSatisfied sets visualCaptureRetryPendingSha: null, and it runs only on a successful capture.
  • One setter: src/db/repositories.ts:4504.
  • No other writer anywhere.

So if capture never succeeds for a head, pr.visualCaptureRetryPendingSha === pr.headSha holds permanently for that head. The same permanent state arises from a lost recapture-preview job (crash between the marker write at :9481 and the enqueue at :9492, or a dropped queue message) — with zero attempts actually spent.

Part B — a pending marker skips the gate instead of holding

src/queue/processors.ts:3449-3453:

const botCaptureRetryPending = Boolean(pr.headSha) && pr.visualCaptureRetryPendingSha === pr.headSha;
const screenshotTableMatch =
  screenshotTableGateResult.violated && screenshotTableGateConfig.action === "close" && !botCaptureRetryPending
    ? { matched: true, reason: screenshotTableGateResult.reason }
    : undefined;

In close mode the planner's screenshotTableMatch close is the gate's only enforcement (src/settings/agent-actions.ts:977-992 is the sole consumer). When botCaptureRetryPending is true, screenshotTableMatch is left undefined and the plan falls through to ordinary merit/CI evaluation — a green PR plans a merge, with no finding, no hold, and no blocker representing "screenshot evidence unresolved".

Contrast the correct precedent in the same codebase: an unresolvable enforced pre-merge check yields PRE_MERGE_CHECK_UNRESOLVED_CODE → NEUTRAL → held, documented as "never silently skipping a hard requirement (auto-merge bypass)" (packages/loopover-engine/src/review/pre-merge-checks.ts:9-13,38-49). Same "cannot evaluate the evidence" state, opposite resolution — a disposition inversion.

Why this is live

Verified on edge-nl-01 (2026-07-27): screenshotTableGate.action: close is set in the private per-repo config for all three repos (/opt/loopover/loopover-config/jsonbored__{loopover,metagraphed,awesome-claude}/.loopover.yml), and the gate was re-enabled two commits before this audit in 73d4e94 (#9455).

Note this defect makes a PR merge; its sibling (#9464) makes a PR close. They are different failure classes of the same subsystem.

Requirements

  1. An exhausted retry budget must restore accurate gate evaluation, exactly as the doc comment already promises.
  2. A gate that is violated and cannot be evaluated must hold, never silently skip.
  3. A lost retry job must not be able to neutralise the gate indefinitely.

Deliverables

  • Clear visualCaptureRetryPendingSha in the exhausted branch of scheduleVisualCaptureRetry (or wherever previewFailed resolves on the final attempt), so the marker cannot outlive the retry budget.
  • Belt-and-braces: bound the marker by age at the read site (processors.ts:3449) so a marker that survives any future code path cannot neutralise the gate forever. This needs a timestamp — either add a column alongside the SHA or probe the corresponding audit event.
  • While botCaptureRetryPending && violated && action === "close", emit a neutral hold (a finding mirroring pre_merge_check_unresolved) instead of leaving screenshotTableMatch undefined. The deferral must suppress the close and the merge.
  • Correct the two doc comments (processors.ts:9464-9467 and :3444-3448) — both currently assert a self-healing property the code does not have.

Tests (must fail against current main)

  • Capture fails for all MAX_PREVIEW_POLL_ATTEMPTS; assert visualCaptureRetryPendingSha is null afterwards.
  • A PR with a stale marker for the current head, gate violated, action: close ⇒ plan contains neither merge nor close, and carries an explicit unresolved-evidence finding.
  • Marker set but retry job never runs (simulate drop) ⇒ gate still enforces after the age bound elapses.
  • Happy path unchanged: successful capture clears the marker and the gate evaluates normally.

Expected outcome

The screenshot-table gate can be deferred by a transient capture failure, but never disabled by one. A PR that violates the gate either closes (when evidence is genuinely absent) or holds (when evidence cannot be determined) — it never merges.

Metadata

Metadata

Assignees

Labels

gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.maintainer-onlyOwner-only work — yields no Gittensor points.orbGittensory Orb related - maintainer self-hosting analytics.visualUI/web visual work — owner-led, NOT for Gittensor contributors (extensions excepted)

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions