Skip to content

Review-E dispatch deny-list — rc#1893

Decision: ship a pure-Core deny-list (ReviewEDispatchSurface) and apply it at both ReviewScanService dispatch paths. Initial entry: Stig-Johnny/claude-3.

Problem

review-e-bot filed seven consecutive stuck reports against issues for the same repo (Stig-Johnny/claude-3) — issue-797, issue-806, issue-802, issue-795, issue-809, and two more before that — all with the same failure shape:

$ task-workspace create Stig-Johnny/claude-3 issue-XYZ
fatal: repository 'https://github.com/Stig-Johnny/claude-3.git/' not found
$ gh api repos/Stig-Johnny/claude-3
HTTP 404

The orchestrator's consolidated triage on rc#1893 pinned two compounding facts:

  1. Access. The review-e-bot GitHub App installation does not include Stig-Johnny/claude-3. Confirmed by review-e's own /installation/repositories check. claude-3 is a live private repo in the legacy Stig-Johnny org, structurally outside the rig tenant surface (no CODEOWNERS, required_approving_review_count=0, inert conductor webhooks, not in any managed_repos).

  2. No pre-check, no loop bound. ReviewScanService emits ReviewAssigned without verifying the installation can resolve the target, and an assignment that can never complete (can't clone, can't post a review, can't close) re-dispatches indefinitely.

Per BRAIN, these assignments should never have been created — review is opt-in for non-agent authors (needs-review label) and claude-3 PRs are Stig-Johnny-authored in a non-tenant legacy org.

Decision

Ship a pure-Core deny-list at the dispatch boundary. Two layers of gating:

Layer Path Behaviour
ReviewScanService.StigJohnnyRepos GitHub-poll Stig-Johnny/claude-3 removed from the static list — the poll no longer fetches its PRs at all.
ReviewEDispatchSurface.CanDispatchReviewE(repo) both Pure predicate. Skips dispatch (with a LogWarning) for any repo on the deny-list, regardless of which path surfaced it.

The state-based path (ScanConductorStateAsync) is gated as well because in-flight in_review issues created before this change would otherwise keep re-dispatching forever — the repo will never become reachable.

Why a deny-list and not a live installation pre-check?

The orchestrator's triage names two decision-independent fixes:

  • A. Repo-resolvability pre-check against the review-e-bot App installation before emitting ReviewAssigned.
  • B. Enforce the BRAIN routing boundary (don't dispatch for repos outside the rig tenant surface).

Both are correct. This PR ships neither — it ships a third, smaller fix that stops the active loop today without depending on either:

  • The deny-list is pure: zero network, no App credentials, fully unit-testable.
  • It composes with both A and B. Once a live pre-check lands, the deny-list becomes the belt to its braces for known-bad repos that should remain skipped even if the installation lookup transiently 200s.
  • It is the minimum change that survives the next conductor restart.

The full A+B work is intentionally out of scope here — see the rc#1893 comments for the broader architectural plan.

Scope discipline (when to extend the deny-list)

Add a new entry only when:

  1. An unreachable repo has produced a repeating stuck loop (≥ 2 occurrences), and
  2. Removing it from ReviewScanService.StigJohnnyRepos is insufficient on its own — i.e. the conductor already has in-flight in_review issues whose state-based scan would otherwise infinite-loop.

For one-off repo deletions or transient access failures, closing the issues is the right fix, not extending this list.

Removing an entry

Remove Stig-Johnny/claude-3 from the deny-list only when one of:

  • review-e-bot is installed on Stig-Johnny/claude-3 (Option A from the rc#1893 runbook), confirmed by /installation/repositories.
  • A live installation pre-check lands and supersedes this guard (Option C).

Locked-in via ReviewEDispatchSurfaceTests.UnreachableRepos_contains_claude_3 so the removal is an intentional, reviewable change.

Files

  • src/ConductorE.Core/Domain/ReviewEDispatchSurface.cs — new pure predicate.
  • src/ConductorE.Api/Services/ReviewScanService.cs — gating at both paths; StigJohnnyRepos slimmed.
  • tests/ConductorE.Core.Tests/Domain/ReviewEDispatchSurfaceTests.cs — predicate tests.
  • tests/ConductorE.Api.Tests/ReviewScanServiceTests.cs — three integration tests covering state-based path, mixed-batch (don't block siblings), and GitHub-poll path.

References

  • Issue: dashecorp/rig-conductor#1893
  • Prior issue (broader claude-3 access discussion): rig-conductor#1455
  • Runbook PRs in rig-docs (#318–#323): superseded by this PR per the orchestrator's triage comment; closing on merge.