Skip to content

orb(disposition): manual-review label has no autonomous removal path — every transient hold becomes a permanent human-only stuck state #9009

Description

@JSONbored

The single highest-leverage fix in the system

The manual-review label has no autonomous removal path anywhere in the codebase. Verified exhaustively: grep -rn 'labelOp: "remove"' src/ returns exactly two sites in src/settings/agent-actions.ts (~1308, ~1337) — the disposition-sibling cleanup (readyToMerge, migrationCollision, changesRequested) and the pending-closure flag. Neither touches manualReview. The exclusion is deliberate and the comment states the real blocker:

Intentionally do NOT remove manualReview here: that same label is also the live maintainer safety hold/freeze, and the planner has no provenance bit proving it was only a stale bot disposition rather than a human-applied hold. Only a maintainer removing the label should lift it.

Meanwhile, while that label is live: the executor denies approve AND merge for all authors (src/services/agent-action-executor.ts step 7b, ~392-399), and contributor AI review is frozen (src/queue/processors.ts ~9846-9851). The label survives new pushes. So a contributor has zero autonomous recovery.

The result: every reason that applies the label becomes permanent, including reasons that are purely transient. Confirmed entry points that are transient:

Entry Where Why it's transient
AI-review lock contention ai-review-orchestration.ts ~151-172 the racing pass finishes in seconds (this is #8999)
Merge-precision circuit breaker agent-actions.ts ~537-559 the fleet flag auto-clears (maybeAutoClearHoldOnly, outcomes-wire.ts ~824-843)
Close-precision circuit breaker agent-actions.ts ~599-640 same — auto-clears
mergeable_state === "unstable" pr-disposition.ts ~99 → agent-actions.ts ~1208-1214 usually a pending non-required check that later passes
Migration collision agent-actions.ts ~1160-1166 contributor rebases and it's gone
ciUnverified / checks-API hiccup generic manual-hold fallback, agent-actions.ts ~1523-1546 a 5xx from the checks API

So a statistical dip in fleet precision, or one flaky non-required check, permanently strands every PR that passed through the window — each requiring a human to unlabel. The codebase already knows: processors.ts ~1348-1352 refers to "a maintainer-only manual-review hold that nothing auto-clears", and ~2036-2042 memorializes PR #7994 stuck 3+ hours this exact way.

Note forceAiReview (the re-run checkbox) does not fix this: it unfreezes the review but does not remove the label, so the executor still denies the merge.

Fix — add the missing provenance bit

  1. Record provenance when the bot applies the label: who applied it (bot vs human), the specific reason code, and the head SHA — a DB column on the PR row or a dedicated audit event the planner can read back.
  2. Let the planner autonomously remove a bot-applied transient hold once its own recorded reason re-evaluates clean at the same-or-newer head: breaker flag cleared, mergeable_state now clean, collision resolved, lock no longer contended, checks API healthy.
  3. Never touch a human-applied label, and never auto-remove a substantive hold (guardrail-path hit, advisory check-run, unlinked-issue-match) — those stay exactly as sticky as today.
  4. Make the executor's step-7b guard defer to a fresh disposition that can plan the removal, rather than hard-denying regardless.

This one change dissolves #8999 plus the five other transient entries above.

Acceptance

  • Reproduce each transient entry, clear its cause, and confirm the label is removed and the PR proceeds autonomously — zero human action.
  • A guardrail hold and a human-applied label both remain sticky (regression-covered).

Refs #8999, #9008.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.orbGittensory Orb related - maintainer self-hosting analytics.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions