[Bug]: manual review retrigger evaluates slop/manifest gates against stale PR files

[galuis116]

[Bug]: manual "Re-run Gittensory review" evaluates the slop + manifest-policy gates against STALE PR files

Summary

PRs #866 ("refresh PR files before policy evaluation") and #925 ("refresh PR
files for merge readiness") established the contract that the Gate must evaluate
the slop and focus-manifest-policy gates against the PR's current changed
files. The refresh is the caller's responsibility — maybePublishPrPublicSurface
itself only reads the cached files (listPullRequestFiles). #866 wired the
refresh into the webhook path, but the manual panel-retrigger path
(maybeProcessPrPanelRetrigger, the "Re-run Gittensory review" checkbox) was
never given the same refresh. So when a contributor re-runs the review after
pushing new commits, the slop gate and manifest-policy gate are computed from
stale (or empty, if never synced) cached files, and the published blocking
Gate check reflects the wrong file set.

This is exactly the failure mode the retrigger exists to avoid: the checkbox is
there so a user can re-evaluate after the PR changed.

Evidence

// src/queue/processors.ts — WEBHOOK path: refreshes before publishing (added by #866)
if (installationId && shouldProcessPullRequestPublicSurface(payload.action)) {
  if (shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off") {
    await refreshPullRequestDetails(env, repoFullName, pr.number);   // ← fresh files
  }
  const gate = await maybePublishPrPublicSurface(env, installationId, repoFullName, pr, repo, settings, advisory, { ... });
}

// src/queue/processors.ts — maybePublishPrPublicSurface reads the CACHE; it does NOT refresh
// "Slop (#530) and focus-manifest-policy (#555) gates both need the PR's changed files. Load ONCE..."
let gateFiles: Awaited<ReturnType<typeof listPullRequestFiles>> | null = null;
if (shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off") {
  gateFiles = await listPullRequestFiles(env, repoFullName, pr.number);   // ← cached files only
}
// ...gateFiles feeds the slop gate + manifest-policy findings, which feed createOrUpdateGateCheckRun (the blocking gate)

// src/queue/processors.ts — maybeProcessPrPanelRetrigger: NO refresh before publishing
const { repo, advisory } = await buildAuthorizedPrActionAdvisory(env, repoFullName, pr, settings); // does not refresh files
await persistAdvisory(env, advisory);
await recordAuditEvent(env, { eventType: "github_app.pr_panel_retriggered", ... });
await maybePublishPrPublicSurface(env, installationId, repoFullName, pr, repo, settings, advisory, {
  deliveryId,
  action: "manual_retrigger",
});   // ← gate evaluated against STALE cached files

refreshPullRequestDetails is called in exactly one place in processors.ts
(the webhook path, line 960). maybePublishPrPublicSurface has two callers —
the webhook (refreshes) and the retrigger (does not). buildAuthorizedPrActionAdvisory
(called on the retrigger path) contains no refreshPullRequestDetails /
listPullRequestFiles call.

Reachability

A real issue_comment (created/edited) webhook whose body has the
panel-retrigger checkbox checked → the dispatcher routes it to
maybeProcessPrPanelRetrigger → after author authorization →
maybePublishPrPublicSurface with no preceding refresh. This is a separate
GitHub delivery from any pull_request/synchronize event, so no concurrent
webhook refreshes the files for it. With slopGateMode or manifestPolicyGateMode
enabled, the re-published required Gate check passes/fails the PR against whatever
pullRequestFiles rows are cached — which after new commits are stale, and on a
PR that was never file-synced are empty (so the slop/manifest gate sees zero
changed files and silently "passes").

Why it's wrong

The retrigger is a manual re-evaluation surface; its entire purpose is to
recompute the gate against the latest PR state. Skipping the refresh makes it
evaluate against an outdated file set, producing a blocking check-run conclusion
that does not reflect the PR — the exact class of stale-gate defect #866/#925
were created to eliminate, left unaddressed on the second caller.

Suggested fix

Mirror the webhook guard in maybeProcessPrPanelRetrigger, immediately before the
maybePublishPrPublicSurface call:

if (shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off") {
  await refreshPullRequestDetails(env, repoFullName, pr.number);
}
await maybePublishPrPublicSurface(env, installationId, repoFullName, pr, repo, settings, advisory, {
  deliveryId,
  action: "manual_retrigger",
});

refreshPullRequestDetails is already imported and is fail-safe (#866: on a
file-fetch failure it preserves the cached files and returns status: "partial"),
so this never makes the retrigger worse on an API hiccup. The slop/manifest
findings are computed inside maybePublishPrPublicSurface after the refresh, so
ordering is correct — identical to the webhook path.

Test status

Not covered. The #866/#925 tests assert the refresh on the webhook
synchronize path only; no test stubs /pulls/<n>/files or asserts file
freshness on the retrigger path. A regression test should drive
maybeProcessPrPanelRetrigger with slop/manifest gating enabled and assert
refreshPullRequestDetails runs (e.g. the files endpoint is hit) before the Gate
check is published.

Confidence note

High. Two callers of the same publish function; the webhook one refreshes and
the retrigger one does not, against an in-code contract (#866/#925) that the gate
must see current files. Reachable from a normal panel interaction, with a
two-line fix that mirrors the existing guard. The only nuance is severity: it
mis-evaluates a re-run (the first, webhook-driven evaluation is correct), and a
later push re-syncs — but a user explicitly re-running to get a fresh verdict gets
a stale one.

Distinct from prior reports

Builds on #866/#925 but is a different, unaddressed caller of the same gate
path (the manual retrigger vs. the webhook). Unrelated to predicted-gate,
gate-completion-403, manifest cache poisoning (#922), or BYOK (#921).