Skip to content

ORB approves/merges a PR it just held for manual review (split-brain, PR #3472) #3482

Description

@JSONbored

Summary

PR #3472 was posted a "manual review recommended" comment, assigned to the author, and labeled manual-review by gittensory-orb — then ~4-8 seconds later the same bot approved and merged it.

Evidence

  • pull_requests row for feat(enrichment): leftover console.log / debugger analyzer #3472 stayed labels_json: ["gittensor:bug","manual-review"] even after merge.
  • audit_events shows two overlapping evaluations for the same repo/PR/head:
    • one recorded gateConclusion=neutral, blockerClass=ai_review_inconclusive, planned ["assign","label"] (a manual-review hold)
    • a second recorded agent.action.approve and agent.action.merge with gate passed, CI green
  • Timeline: comment at T+0s ("manual review recommended") → assignee + manual-review label at T+45s → approve at T+49s → merge at T+52s.

Root cause

Two independent plan-and-execute passes (e.g. a webhook-triggered re-review and a regate-repair/sweep pass) can evaluate the same PR head concurrently. The per-PR actuation lock (claimPrActuationLock, #2129) and the AI-review lock (claimAiReviewLock) each serialize part of the pipeline, but neither pass is ever made aware of the other's disposition:

  • The AI-review lock's loser gets an instant ai_review_inconclusive placeholder → gate neutral → the planner holds the PR (assign + manual-review label).
  • The lock's winner keeps running its own (possibly much slower) real AI review. When it later resolves cleanly, that pass's planner independently computes reviewGood && !heldForManualReview from its own plan-time snapshot — which was read before the loser's label ever existed — and stages approve + merge.
  • The executor's existing freshness guard only re-checks head SHA/open state, never the PR's live labels, so nothing stops the winner's approve/merge even though a manual-review hold is already live on GitHub.

Fix

A live, last-moment guard in the executor: immediately before approve/merge, re-check the same live PR fetch already used for the freshness guard for the configured manualReviewLabel. If it's present, deny the action (audited) instead of executing it. Because this reuses the freshness guard's fetch, it is automatically scoped to the same head (a new commit already fails freshness first) and clears itself the moment a maintainer removes the label — no new locking or persistence needed.

Metadata

Metadata

Assignees

No one assigned

    Labels

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

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions