Skip to content

fix(review): keep missing required CI pending - #3947

Merged
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-missing-required-ci-context
Jul 7, 2026
Merged

fix(review): keep missing required CI pending#3947
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-missing-required-ci-context

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent a short-cap path from publishing a passing Gittensory review when a branch-protection/required CI context never appeared, which could make the review check appear green before expected CI has run.

Description

  • When the live CI aggregate reports hasMissingRequiredContext, keep the PR deferred instead of falling through to finalization, by recording a defer audit event and returning false in prReadyForReview (change in src/queue/processors.ts).
  • Update the regression test to assert no gate check is published and that a defer audit event is recorded when a required context remains missing past the short surfacing cap (change in test/unit/queue.test.ts).

Testing

  • Ran the focused unit tests with npx vitest run test/unit/queue.test.ts -t "missing-required-context", and the two relevant tests passed.
  • Ran git diff --check locally to validate whitespace/conflict checks before committing the change.

Codex Task

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-07 09:02:50 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI failing · unstable

🛑 Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR closes a real gap: previously, when live CI reported `hasMissingRequiredContext` past the short surfacing cap, `prReadyForReview` fell through to the stuck-CI finalize path and could publish a passing gate before a required branch-protection context ever ran. The new `if (ci.hasMissingRequiredContext) { ...; return false; }` block in src/queue/processors.ts intercepts that fall-through, records a `review_deferred_ci_pending` audit event, and keeps the PR deferred indefinitely instead of finalizing — a fail-closed choice consistent with the stated motivation. The updated test correctly flips the assertions (gateChecks===0, deferred audit recorded, finalized audit not recorded) to match the new behavior. One side effect: `MISSING_REQUIRED_CONTEXT_DEFER_MS`/`deferCapMs` still gates the earlier `hasVisiblePending || ...` block, but now that hasMissingRequiredContext always re-defers past that cap too, the shorter-cap distinction for this class is functionally inert for this branch — the diff strips the comment explaining why the shorter cap existed but leaves the now-largely-decorative constant/branch split in place.

Nits — 5 non-blocking
  • src/queue/processors.ts: with the new unconditional `if (ci.hasMissingRequiredContext) return false`, `MISSING_REQUIRED_CONTEXT_DEFER_MS`/`deferCapMs`'s shorter-cap-vs-STUCK_CI_DEFER_MS distinction no longer changes the outcome for this class (both within-cap and past-cap now return false) — worth a comment or removal so a future reader doesn't assume the cap still causes eventual finalization for missing-required-context.
  • test/unit/queue.test.ts: the new `.catch(() => undefined)` on the added `recordAuditEvent` call for `review_deferred_ci_pending` isn't exercised on its error path, and codecov/patch is reported failing at 75% vs the 99% target — worth adding/checking a case that covers the catch arm or otherwise closing the patch-coverage gap.
  • Consider clarifying in a comment why a required context that can never post is now deferred forever rather than eventually escalated/alerted, since operators may want visibility into a PR stuck in this state indefinitely rather than silent perpetual deferral.
  • src/queue/processors.ts: either fold the missing-required-context case into the same cap-driven branch above (avoiding two separate `return false` paths with overlapping audit semantics) or explicitly document that the cap is now purely used to select which of two identically-outcome branches fires, for future maintainability.
  • Given this changes long-standing behavior from 'eventually finalize with review_finalized_ci_stuck' to 'defer forever' for this CI class, consider whether ops needs a way to detect/alert on PRs permanently stuck in this state (e.g. a dashboard/audit query), since #ci-stuck-repeat's original concern (repeat AI-review spend) is now replaced by a different concern (PRs that never get reviewed).

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

CI checks failing

  • codecov/patch — 75.00% of diff hit (target 99.00%)
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 569 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 569 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 569 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 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.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.66%. Comparing base (1ce4364) to head (6b978a6).
⚠️ Report is 15 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/queue/processors.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3947      +/-   ##
==========================================
- Coverage   93.66%   93.66%   -0.01%     
==========================================
  Files         372      372              
  Lines       34856    34860       +4     
  Branches    12743    12744       +1     
==========================================
+ Hits        32649    32652       +3     
- Misses       1588     1589       +1     
  Partials      619      619              
Files with missing lines Coverage Δ
src/queue/processors.ts 94.51% <75.00%> (-0.03%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 7, 2026
@JSONbored
JSONbored merged commit 38ef5a2 into main Jul 7, 2026
9 of 10 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-missing-required-ci-context branch July 7, 2026 09:51
andriypolanski pushed a commit to andriypolanski/gittensory that referenced this pull request Jul 9, 2026
…trust

gittensor:priority propagation required the PR author to be the linked issue's
own author or a GitHub assignee of it, with no relaxation -- unlike bug/feature,
which already trust a maintainer-authored issue without formal assignment
(trustMaintainerAuthoredIssue). But GitHub silently refuses to assign a
contributor lacking push/triage access to the repo, and our issues are almost
always maintainer-authored for open pickup, rarely formally assigned to a
specific person. That combination meant the reward label could structurally
never reach the external contributors it exists to reward, no matter how
correctly the rest of the propagation pipeline behaved -- confirmed live on
metagraphed PR JSONbored#4554 (issue JSONbored#3947 carried both gittensor:bug and
gittensor:priority; only bug ever propagated).

Adds a new, distinctly-named, opt-in LinkedIssueLabelPropagationMapping flag,
trustMaintainerAuthoredIssueForReward, so a repo whose priority label is meant
to incentivize open pickup (the maintainer's hand-picking already happened when
the issue was labeled, not gated on which contributor later closes it) can
extend the SAME maintainer-authored-issue relaxation to its reward mapping,
while a repo that wants the strict author-or-assignee bar preserved for its
reward label keeps that behavior unchanged by default. Enabled it for
gittensor:priority in this repo's own config, matching the enrichment-issue
open-pickup policy this label already implements for metagraphed.
JSONbored added a commit that referenced this pull request Jul 12, 2026
…s confirmed (#5482)

* fix(review): stop charging a regate-repair attempt before readiness is confirmed

regatePullRequest recorded a REGATE_REPAIR_ATTEMPT_EVENT_TYPE audit event as
soon as the job cleared GitHub rate-limit admission, before ever calling
reReviewStoredPullRequest -- so a PR legitimately still waiting on a missing
branch-protection-required check (which prReadyForReview defers
unconditionally and indefinitely by design, #3947) got charged a full repair
attempt on every ~2-minute sweep tick even though nothing was actually wrong.
The fixed 5-attempt cap exhausted in ~10 minutes, an order of magnitude
shorter than realistic required-CI latency, firing a false
"repair exhausted" alert for a review that was never broken.

reReviewStoredPullRequest now returns whether it actually got past
prReadyForReview (there is no early return anywhere after that point, so
every exit before it returns false and the function's natural end returns
true); regatePullRequest only records the repair attempt when that's true.
A swallowed (non-retryable) exception past that point still counts as a real
executed attempt, same as before -- only the clean "declined before ever
reaching readiness" path is now excluded.

Fixes GITTENSORY-1E

* fix(review): charge the repair attempt even when readiness is reached then throws

Gittensory review flagged that regatePullRequest only recorded
REGATE_REPAIR_ATTEMPT_EVENT_TYPE after reReviewStoredPullRequest
returns, so a retryable error (GitHub rate limit / actuation-lock
contention) surfacing from real post-readiness work skipped the
charge entirely and could let a stuck repair reselect indefinitely.

reReviewStoredPullRequest now exposes an onReachedReadiness callback
that fires the instant its own readiness gate passes, before any
further throwable work -- a side channel that survives a later throw,
unlike the boolean return value. regatePullRequest uses it in a
try/catch/finally: the retryable branch still rethrows for the
queue's own retry, but the finally block charges the attempt first
whenever readiness was genuinely reached, regardless of whether the
call returned or threw. This also closes the symmetric gap the
review's own nit raised: an error thrown BEFORE readiness (the
callback never fires) correctly still does NOT charge the budget.
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant