Skip to content

fix(gate): refresh PR files on manual review rerun - #928

Merged
JSONbored merged 5 commits into
JSONbored:mainfrom
philluiz2323:fix/retrigger-refresh-pr-files
Jun 20, 2026
Merged

fix(gate): refresh PR files on manual review rerun#928
JSONbored merged 5 commits into
JSONbored:mainfrom
philluiz2323:fix/retrigger-refresh-pr-files

Conversation

@philluiz2323

Copy link
Copy Markdown
Contributor

Summary

The webhook gate path refreshes the PR's changed files before evaluating the slop and focus-manifest-policy gates (#866/#925) — the refresh is the caller's job, since maybePublishPrPublicSurface only reads the cached files (listPullRequestFiles):

// webhook path — refreshes first (added by #866)
if (shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off") {
  await refreshPullRequestDetails(env, repoFullName, pr.number);
}
const gate = await maybePublishPrPublicSurface(env, installationId, repoFullName, pr, repo, settings, advisory, { ... });

But maybePublishPrPublicSurface has two callers. The manual "Re-run Gittensory review" retrigger (maybeProcessPrPanelRetrigger) called it with no refresh (and buildAuthorizedPrActionAdvisory doesn't refresh either):

const { repo, advisory } = await buildAuthorizedPrActionAdvisory(env, repoFullName, pr, settings);
await persistAdvisory(env, advisory);
await recordAuditEvent(env, { eventType: "github_app.pr_panel_retriggered", ... });
await maybePublishPrPublicSurface(env, ..., { deliveryId, action: "manual_retrigger" }); // gate over STALE files

Impact: the retrigger is a re-evaluation surface — a user clicks it after the PR changed. Without the refresh, the slop gate (file additions/paths) and manifest-policy gate (blocked paths, missing-tests over changed paths) compute against stale (or empty, if never synced) cached files, so the re-published required Gate check reflects the wrong diff — the exact stale-gate class #866/#925 set out to eliminate, left unaddressed on the second caller.

Fix

Mirror the webhook guard in maybeProcessPrPanelRetrigger, before the publish:

if (shouldCollectSlopEvidence(settings) || settings.manifestPolicyGateMode !== "off") {
  await refreshPullRequestDetails(env, repoFullName, pr.number);
}

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

Tests

Added a regression test: a maintainer rerun with slopGateMode enabled now fetches /pulls/<n>/files (the refresh) before publishing, and the retrigger still audits completed. The existing rerun test (gates off → guard not taken) is unchanged, so both arms of the new guard are exercised.

Full unit suite green locally (2219 passed; only the known local-only CRLF gittensory-focus-manifest test fails locally, passes in CI).

Closes #927

The webhook gate path refreshes the PR's files before evaluating the slop and
manifest-policy gates (JSONbored#866/JSONbored#925), but the manual 'Re-run Gittensory review'
retrigger called the same publish path without refreshing. A user re-running
after pushing commits got the slop/manifest gate evaluated against stale (or
empty) cached files, so the re-published Gate check reflected the wrong diff.

Mirror the webhook guard: refresh the PR files before publishing on the
retrigger path too. refreshPullRequestDetails is fail-safe (preserves the
cache on a fetch error).

Closes JSONbored#927
@philluiz2323
philluiz2323 requested a review from JSONbored as a code owner June 19, 2026 20:28
@dosubot dosubot Bot added the size:XS label Jun 19, 2026
@ghost

ghost commented Jun 19, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #928 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 19, 2026
@ghost

ghost commented Jun 19, 2026

Copy link
Copy Markdown

🔍 Maintainer review

Reviewed 2 file(s) — ✅ all CI green.

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The change correctly adds a refresh of PR details before publishing the panel when a manual rerun occurs and slop or manifest policy gates are active. The added test validates this behavior by asserting a fetch of the PR files endpoint. No public‑facing output is altered, no security boundaries are weakened, and the code follows existing patterns. The modification is limited to backend queue processing logic and is well‑documented.

Suggestions

  • Consider adding a unit test for the case when both gates are off to ensure the refresh is not called unnecessarily.
  • Document the new refresh behavior in the developer guide or changelog for future maintainers.

Worth double-checking

  • Potential increase in GitHub API calls on manual reruns could affect rate limits if reruns are frequent.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The change adds a refresh of PR files on manual retrigger when slop or manifest policy gates are active, ensuring gates evaluate the current diff. This mirrors the webhook path behavior and is logically sound. The test thoroughly verifies the refresh occurs and audit events are recorded. No security, correctness, or maintainability issues are observed.

No blocking issues spotted.

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.61%. Comparing base (4d748c4) to head (d381d64).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #928   +/-   ##
=======================================
  Coverage   96.60%   96.61%           
=======================================
  Files         110      110           
  Lines       14926    14928    +2     
  Branches     5398     5399    +1     
=======================================
+ Hits        14420    14422    +2     
  Misses        104      104           
  Partials      402      402           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 20, 2026
@JSONbored
JSONbored merged commit d0e549f into JSONbored:main Jun 20, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants