fix(review): reflect a live manual-review label in the public PR comment#8068
Conversation
Closes #8067. heldForReview only checked for a hard-guardrail-path hit, so a PR whose manual-review label came from elsewhere (a since-resolved gate blocker on a protected author, a migration collision, ...) kept headlining "approve/merge recommended" while the executor's live-label guard silently denied every merge/approve attempt, indefinitely, with no visible explanation on the PR. Confirmed live on PR #7994, stuck 3+ hours despite fully green CI, before being merged manually. Broaden the check to also fire on the live, configured manual-review label being present — this only changes what the comment says, not the "only a maintainer removes the label" safety invariant itself.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-22 18:37:08 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8068 +/- ##
==========================================
+ Coverage 91.82% 95.78% +3.96%
==========================================
Files 734 1 -733
Lines 75019 3135 -71884
Branches 23037 1115 -21922
==========================================
- Hits 68885 3003 -65882
+ Misses 5034 45 -4989
+ Partials 1100 87 -1013
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Closes #8067.
Summary
manual-reviewapplied as a fallback hold (src/settings/agent-actions.ts's genericmanualHoldReasonfallback). That label is deliberately maintainer-only-removable (src/services/agent-action-executor.ts's live-label guard, the "feat(enrichment): leftover console.log / debugger analyzer #3472 split-brain" check) — correct, since the bot must never silently override a real human freeze.derivePublicCommentMergeFacts'sheldForReviewflag only ever checked for a hard-guardrail-path hit. It never checked the live PR for the manual-review label itself, so once the ORIGINAL triggering blocker cleared on a later pass, the public comment happily said "✅ approve/merge recommended, no blockers" while the executor kept silently denying every merge/approve attempt because the (now-stale, but still live) label was still on the PR — with zero visible explanation anywhere on the PR.missing_linked_issueearly from a wrong closing-keyword phrasing ("Implements Fix bare-word hotkey/coldkey false positives in containsSecretLikeText + static-message redaction bug #7981" instead of "Closes Fix bare-word hotkey/coldkey false positives in containsSecretLikeText + static-message redaction bug #7981"), gotmanual-reviewapplied as the owner-fallback hold, the wording was fixed and the blocker cleared — but the label stuck around, and the PR sat fully green (CI + codecov + bot-approved) for 3+ hours before being merged manually. Full root-cause trail (audit_events, executor logs) is in fix(review): stale manual-review label silently blocks merge with no visible explanation #8067.heldForReviewto also gotruewhenever the configured manual-review label (default"manual-review", honors a repo'smanualReviewLabeloverride/disable) is present on the live PR — regardless of which hold path originally applied it. Visibility-only change: the comment now correctly says "manual review recommended" instead of a misleading "ready"; the actual "only a maintainer can remove this label" safety behavior is untouched.Test plan
npm run typechecktest/unit/processors-public-comment-merge-facts.test.ts: label present → held; case-insensitive match; label absent → unaffected; custom configured label name;manualReviewLabel: nulldisables the check entirelynpm run test:coverage: 1085/1085 files, 20278 tests, 0 failuresprocessors.tsaggregate: 97.36% stmts / 96.76% branch, no new lines in the uncovered set)