Skip to content

fix(review-evasion): bounded retry + audit visibility for every one-shot close's explanation comment#8814

Merged
JSONbored merged 1 commit into
mainfrom
fix/unexplained-close-backfill
Jul 26, 2026
Merged

fix(review-evasion): bounded retry + audit visibility for every one-shot close's explanation comment#8814
JSONbored merged 1 commit into
mainfrom
fix/unexplained-close-backfill

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Round-2 audit fix (#8802): all 7 one-shot close-enforcement guards posted the contributor's only explanation via createIssueComment(...).catch(() => undefined) — a transient failure left a permanently closed PR with no visible reason and nothing ever backfilled it.

Changes

  • postCloseExplanation (exported, review-evasion.ts): bounded in-handler retry — three spaced attempts (0/400/1600ms) — still fail-safe toward the close itself; exhaustion emits a level:error close_explanation_post_failed log (reaches the Sentry forwarder).
  • All 7 guards route through it, and each guard's audit row now records explanationPosted: true / false / null (repo opted out of the comment — the five settings-gated sites hoist a tri-state so opt-out is distinguishable from delivery failure). An operator can now enumerate any unexplained close straight from audit_events.
  • Deliberate scope call (the issue offered retry OR a backfill sweep): the sweep needs the explanation body persisted, marker-detection, and a new always-on cron surface — real machinery that's only worth building if retry residue proves nonzero. The audit field added here is exactly the evidence for that follow-up decision; noted on the issue.

Test plan

  • npm run typecheck — clean
  • New helper suite (fake timers): first-try success, retry-through-transients, all-fail → false + loud residue with the exact event shape, delay-ladder pin
  • Guards suite extended: the draft-dodge success path asserts explanationPosted: true in the audit metadata — 226/226 + queue-2 green
  • Every changed line (21 hunks, script-verified against the diff): 0 uncovered statements/branches

Closes #8802

@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored JSONbored self-assigned this Jul 26, 2026
…hot close's explanation comment (#8802)

All 7 close-enforcement guards posted the contributor's ONLY explanation via
a swallowed, never-retried createIssueComment — a secondary-rate-limit/5xx
left a permanently closed one-shot PR with zero visible reason, forever (no
sweep backfills a missing explanation on a closed PR).

- postCloseExplanation: three spaced attempts (0/400/1600ms — a transient
  window rarely spans them), still fail-safe toward the close, with a LOUD
  level:error residue log (close_explanation_post_failed) when exhausted so
  the structured-log Sentry forwarder surfaces any remaining gap.
- Every guard's audit row now records explanationPosted (true / false /
  null = the repo opted out of the comment), so an operator can enumerate
  unexplained closes directly from the audit trail — the evidence that
  decides whether a marker-based backfill sweep is ever actually needed.
@JSONbored
JSONbored force-pushed the fix/unexplained-close-backfill branch from fe1bc7b to 771d5ab Compare July 26, 2026 07:33
@JSONbored
JSONbored merged commit 18daff1 into main Jul 26, 2026
3 of 4 checks passed
@JSONbored
JSONbored deleted the fix/unexplained-close-backfill branch July 26, 2026 07:41
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 26, 2026
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
21263 3 21260 21
View the top 3 failed test(s) by shortest run time
test/unit/queue.test.ts > queue processors > linked-issue hard-rule violation persistence (#linked-issue-hard-rule-persistence) > REGRESSION (live-issue-state-change-before-re-evaluation): Pass 1 flags a real violation; Pass 2's live re-parse re-evaluates the SAME issue as clean (assignee removed) but the persisted violation still closes the PR
Stack Traces | 0.234s run time
AssertionError: expected 'error' to be 'completed' // Object.is equality

Expected: "completed"
Received: "error"

 ❯ test/unit/queue.test.ts:2843:32
test/unit/queue-2.test.ts > queue processors > #4603: the SAME sub-floor defect one-shot-closes when aiReviewLowConfidenceDisposition is explicitly one_shot
Stack Traces | 0.35s run time
AssertionError: expected 'error' to be 'completed' // Object.is equality

Expected: "completed"
Received: "error"

 ❯ test/unit/queue-2.test.ts:361:28
test/unit/queue.test.ts > queue processors > linked-issue hard-rule violation persistence (#linked-issue-hard-rule-persistence) > REGRESSION (body-edit-during-grace-window): Pass 1 flags a real owner-assigned violation; Pass 2's live re-parse finds NO linked issues (body edited) but the persisted violation still closes the PR
Stack Traces | 0.424s run time
AssertionError: expected 'error' to be 'completed' // Object.is equality

Expected: "completed"
Received: "error"

 ❯ test/unit/queue.test.ts:2798:32

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

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.

fix(review-evasion): a one-shot-closed PR's only explanation comment is swallowed on failure — closed with no visible reason, forever

1 participant